Messages in this thread | | | Date | Sat, 19 Sep 2015 11:13:29 +0100 | From | Lee Jones <> | Subject | Re: [PATCH] mfd: lpc_ich: Separate device cells for clarity |
| |
On Thu, 17 Sep 2015, Andy Shevchenko wrote:
> On Thu, 2015-09-03 at 14:14 -0500, Aaron Sierra wrote: > > The lpc_ich_cells array gives the wrong impression about the > > relationship between the watchdog and GPIO devices. They are > > completely distinct devices, so this patch separates the > > array into distinct mfd_cell structs per device. > > > > A side effect of removing the array, is that the lpc_cells enum > > is no longer needed. > > > > Looks good for me.
Is that an Ack? If so, can you formally provide one please?
> > Signed-off-by: Aaron Sierra <asierra@xes-inc.com> > > --- > > drivers/mfd/lpc_ich.c | 42 ++++++++++++++++++----------------------- > > - > > 1 file changed, 18 insertions(+), 24 deletions(-) > > > > diff --git a/drivers/mfd/lpc_ich.c b/drivers/mfd/lpc_ich.c > > index 8de3439..7a01d430 100644 > > --- a/drivers/mfd/lpc_ich.c > > +++ b/drivers/mfd/lpc_ich.c > > @@ -131,24 +131,18 @@ static struct resource gpio_ich_res[] = { > > }, > > }; > > > > -enum lpc_cells { > > - LPC_WDT = 0, > > - LPC_GPIO, > > +static struct mfd_cell lpc_ich_wdt_cell = { > > + .name = "iTCO_wdt", > > + .num_resources = ARRAY_SIZE(wdt_ich_res), > > + .resources = wdt_ich_res, > > + .ignore_resource_conflicts = true, > > }; > > > > -static struct mfd_cell lpc_ich_cells[] = { > > - [LPC_WDT] = { > > - .name = "iTCO_wdt", > > - .num_resources = ARRAY_SIZE(wdt_ich_res), > > - .resources = wdt_ich_res, > > - .ignore_resource_conflicts = true, > > - }, > > - [LPC_GPIO] = { > > - .name = "gpio_ich", > > - .num_resources = ARRAY_SIZE(gpio_ich_res), > > - .resources = gpio_ich_res, > > - .ignore_resource_conflicts = true, > > - }, > > +static struct mfd_cell lpc_ich_gpio_cell = { > > + .name = "gpio_ich", > > + .num_resources = ARRAY_SIZE(gpio_ich_res), > > + .resources = gpio_ich_res, > > + .ignore_resource_conflicts = true, > > }; > > > > /* chipset related info */ > > @@ -881,7 +875,7 @@ static int lpc_ich_init_gpio(struct pci_dev *dev) > > base_addr = base_addr_cfg & 0x0000ff80; > > if (!base_addr) { > > dev_notice(&dev->dev, "I/O space for ACPI > > uninitialized\n"); > > - lpc_ich_cells[LPC_GPIO].num_resources--; > > + lpc_ich_gpio_cell.num_resources--; > > goto gpe0_done; > > } > > > > @@ -895,7 +889,7 @@ static int lpc_ich_init_gpio(struct pci_dev *dev) > > * the platform_device subsystem doesn't see this > > resource > > * or it will register an invalid region. > > */ > > - lpc_ich_cells[LPC_GPIO].num_resources--; > > + lpc_ich_gpio_cell.num_resources--; > > acpi_conflict = true; > > } else { > > lpc_ich_enable_acpi_space(dev); > > @@ -933,14 +927,14 @@ gpe0_done: > > lpc_chipset_info[priv->chipset].use_gpio = ret; > > lpc_ich_enable_gpio_space(dev); > > > > - lpc_ich_finalize_cell(dev, &lpc_ich_cells[LPC_GPIO]); > > + lpc_ich_finalize_cell(dev, &lpc_ich_gpio_cell); > > ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO, > > - &lpc_ich_cells[LPC_GPIO], 1, NULL, 0, > > NULL); > > + &lpc_ich_gpio_cell, 1, NULL, 0, NULL); > > > > gpio_done: > > if (acpi_conflict) > > pr_warn("Resource conflict(s) found affecting %s\n", > > - lpc_ich_cells[LPC_GPIO].name); > > + lpc_ich_gpio_cell.name); > > return ret; > > } > > > > @@ -984,7 +978,7 @@ static int lpc_ich_init_wdt(struct pci_dev *dev) > > */ > > if (lpc_chipset_info[priv->chipset].iTCO_version == 1) { > > /* Don't register iomem for TCO ver 1 */ > > - lpc_ich_cells[LPC_WDT].num_resources--; > > + lpc_ich_wdt_cell.num_resources--; > > } else if (lpc_chipset_info[priv->chipset].iTCO_version == > > 2) { > > pci_read_config_dword(dev, RCBABASE, > > &base_addr_cfg); > > base_addr = base_addr_cfg & 0xffffc000; > > @@ -1007,9 +1001,9 @@ static int lpc_ich_init_wdt(struct pci_dev > > *dev) > > res->end = base_addr + ACPIBASE_PMC_END; > > } > > > > - lpc_ich_finalize_cell(dev, &lpc_ich_cells[LPC_WDT]); > > + lpc_ich_finalize_cell(dev, &lpc_ich_wdt_cell); > > ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO, > > - &lpc_ich_cells[LPC_WDT], 1, NULL, 0, > > NULL); > > + &lpc_ich_wdt_cell, 1, NULL, 0, NULL); > > > > wdt_done: > > return ret; >
-- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
| |