| Message ID | 20230125212631.749094-1-trix@redhat.com |
|---|---|
| State | Accepted |
| Commit | 2e539b735d8683097846b486c0fb093da5f27fbb |
| Headers | show |
| Series | gpio: tegra186: remove unneeded loop in tegra186_gpio_init_route_mapping() | expand |
On Wed, Jan 25, 2023 at 01:26:31PM -0800, Tom Rix wrote: > Reviewing the j loop over num_irqs_per_bank, in the code previous > to the fixes: commit, every j was used. now only when j == 0. > If only j == 0 is used, there is no need for the loop. > > Fixes: 210386804745 ("gpio: tegra186: Support multiple interrupts per bank") > Signed-off-by: Tom Rix <trix@redhat.com> > --- > drivers/gpio/gpio-tegra186.c | 40 ++++++++++++++++-------------------- > 1 file changed, 18 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c > index 9941f35af823..14c872b6ad05 100644 > --- a/drivers/gpio/gpio-tegra186.c > +++ b/drivers/gpio/gpio-tegra186.c > @@ -677,7 +677,7 @@ static const struct of_device_id tegra186_pmc_of_match[] = { > static void tegra186_gpio_init_route_mapping(struct tegra_gpio *gpio) > { > struct device *dev = gpio->gpio.parent; > - unsigned int i, j; > + unsigned int i; > u32 value; > > for (i = 0; i < gpio->soc->num_ports; i++) { > @@ -699,27 +699,23 @@ static void tegra186_gpio_init_route_mapping(struct tegra_gpio *gpio) > * On Tegra194 and later, each pin can be routed to one or more > * interrupts. > */ > - for (j = 0; j < gpio->num_irqs_per_bank; j++) { > - dev_dbg(dev, "programming default interrupt routing for port %s\n", > - port->name); > - > - offset = TEGRA186_GPIO_INT_ROUTE_MAPPING(p, j); > - > - /* > - * By default we only want to route GPIO pins to IRQ 0. This works > - * only under the assumption that we're running as the host kernel > - * and hence all GPIO pins are owned by Linux. > - * > - * For cases where Linux is the guest OS, the hypervisor will have > - * to configure the interrupt routing and pass only the valid > - * interrupts via device tree. > - */ > - if (j == 0) { > - value = readl(base + offset); > - value = BIT(port->pins) - 1; > - writel(value, base + offset); > - } > - } > + dev_dbg(dev, "programming default interrupt routing for port %s\n", > + port->name); > + > + offset = TEGRA186_GPIO_INT_ROUTE_MAPPING(p, 0); > + > + /* > + * By default we only want to route GPIO pins to IRQ 0. This works > + * only under the assumption that we're running as the host kernel > + * and hence all GPIO pins are owned by Linux. > + * > + * For cases where Linux is the guest OS, the hypervisor will have > + * to configure the interrupt routing and pass only the valid > + * interrupts via device tree. > + */ > + value = readl(base + offset); > + value = BIT(port->pins) - 1; > + writel(value, base + offset); This was supposed to be a placeholder so that a more complex routing could be added later on. Maybe adding "j" to the debug message would have made that a bit clearer. Anyway, no complex routing has been needed so far, so I don't have a strong objection to this. We can always add it back if we ever need it. On the other hand, we don't do much in the loop, so leaving it as-is wouldn't be harmful either. I'll leave it up to Linus and Bartosz to decide. If they want to pick it up: Acked-by: Thierry Reding <treding@nvidia.com>
diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c index 9941f35af823..14c872b6ad05 100644 --- a/drivers/gpio/gpio-tegra186.c +++ b/drivers/gpio/gpio-tegra186.c @@ -677,7 +677,7 @@ static const struct of_device_id tegra186_pmc_of_match[] = { static void tegra186_gpio_init_route_mapping(struct tegra_gpio *gpio) { struct device *dev = gpio->gpio.parent; - unsigned int i, j; + unsigned int i; u32 value; for (i = 0; i < gpio->soc->num_ports; i++) { @@ -699,27 +699,23 @@ static void tegra186_gpio_init_route_mapping(struct tegra_gpio *gpio) * On Tegra194 and later, each pin can be routed to one or more * interrupts. */ - for (j = 0; j < gpio->num_irqs_per_bank; j++) { - dev_dbg(dev, "programming default interrupt routing for port %s\n", - port->name); - - offset = TEGRA186_GPIO_INT_ROUTE_MAPPING(p, j); - - /* - * By default we only want to route GPIO pins to IRQ 0. This works - * only under the assumption that we're running as the host kernel - * and hence all GPIO pins are owned by Linux. - * - * For cases where Linux is the guest OS, the hypervisor will have - * to configure the interrupt routing and pass only the valid - * interrupts via device tree. - */ - if (j == 0) { - value = readl(base + offset); - value = BIT(port->pins) - 1; - writel(value, base + offset); - } - } + dev_dbg(dev, "programming default interrupt routing for port %s\n", + port->name); + + offset = TEGRA186_GPIO_INT_ROUTE_MAPPING(p, 0); + + /* + * By default we only want to route GPIO pins to IRQ 0. This works + * only under the assumption that we're running as the host kernel + * and hence all GPIO pins are owned by Linux. + * + * For cases where Linux is the guest OS, the hypervisor will have + * to configure the interrupt routing and pass only the valid + * interrupts via device tree. + */ + value = readl(base + offset); + value = BIT(port->pins) - 1; + writel(value, base + offset); } } }
Reviewing the j loop over num_irqs_per_bank, in the code previous to the fixes: commit, every j was used. now only when j == 0. If only j == 0 is used, there is no need for the loop. Fixes: 210386804745 ("gpio: tegra186: Support multiple interrupts per bank") Signed-off-by: Tom Rix <trix@redhat.com> --- drivers/gpio/gpio-tegra186.c | 40 ++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 22 deletions(-)