Re: [PATCH v1 1/1] mfd: intel_soc_pmic_core: Use explicit name of GPIO controller

From: Andy Shevchenko
Date: Thu Oct 20 2016 - 07:49:09 EST


On Thu, 2016-10-20 at 14:13 +0300, Ville SyrjÃlà wrote:
> On Thu, Oct 20, 2016 at 12:36:13PM +0300, Andy Shevchenko wrote:
> > The Crystal Cove GPIO driver has its explicit name, i.e.
> > crystal_cove_gpio.
> >
> > Use it explicitly to be in align with the rest of code, pretty
> > grepping, and
> > prevent potential breakage if GPIO driver file name ever changed.
> >
> > Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
> > Cc: Ville SyrjÃlà <ville.syrjala@xxxxxxxxxxxxxxx>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > ---
> > Âdrivers/mfd/intel_soc_pmic_core.c | 2 +-
> > Â1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mfd/intel_soc_pmic_core.c
> > b/drivers/mfd/intel_soc_pmic_core.c
> > index 12d6ebb4..7fd8e87 100644
> > --- a/drivers/mfd/intel_soc_pmic_core.c
> > +++ b/drivers/mfd/intel_soc_pmic_core.c
> > @@ -34,7 +34,7 @@ static struct gpiod_lookup_table panel_gpio_table
> > = {
> > Â .dev_id = "0000:00:02.0",
> > Â .table = {
> > Â /* Panel EN/DISABLE */
> > - GPIO_LOOKUP("gpio_crystalcove", 94, "panel",
> > GPIO_ACTIVE_HIGH),
> > + GPIO_LOOKUP("crystal_cove_gpio", 94, "panel",
> > GPIO_ACTIVE_HIGH),
>
> This looks like it would break things.

Yeah. Good catch.

> The gpio_chip->label which is
> used for the lookup is based on KBUILD_MODNAME and the moduled is
> called gpio-crystalcove.

...which I think is wrong. Only few drivers are binding to file / module
name.

Perhaps it should be fixed all together.

Naming a bit chaotic in drivers/gpio/*::chip.label, though majority of
them are using dev_name() of actual device.

Linus, what is your opinion?


--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy