From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754716AbbISKNg (ORCPT ); Sat, 19 Sep 2015 06:13:36 -0400 Received: from mail-wi0-f177.google.com ([209.85.212.177]:33819 "EHLO mail-wi0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751372AbbISKNe (ORCPT ); Sat, 19 Sep 2015 06:13:34 -0400 Date: Sat, 19 Sep 2015 11:13:29 +0100 From: Lee Jones To: Andy Shevchenko Cc: Aaron Sierra , linux-kernel , Peter Tyser , Guenter Roeck , Jean Delvare , Samuel Ortiz , Mika Westerberg , Matt Fleming Subject: Re: [PATCH] mfd: lpc_ich: Separate device cells for clarity Message-ID: <20150919101329.GM3218@x1> References: <1595954137.78926.1441307640528.JavaMail.zimbra@xes-inc.com> <1442506679.8361.85.camel@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1442506679.8361.85.camel@linux.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > --- > > 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