On Tue, May 16, 2023 at 4:05 PM Esteban Blanc <eblanc@xxxxxxxxxxxx> wrote: > On Fri May 12, 2023 at 7:07 PM CEST, wrote: > > Fri, May 12, 2023 at 04:17:54PM +0200, Esteban Blanc kirjoitti: ... > > > +config PINCTRL_TPS6594 > > > + tristate "Pinctrl and GPIO driver for TI TPS6594 PMIC" > > > + depends on MFD_TPS6594 > > > + default MFD_TPS6594 > > > + select PINMUX > > > + select GPIOLIB > > > + select REGMAP > > > + select GPIO_REGMAP > > > + help > > > + This driver supports GPIOs and pinmuxing for the TPS6594 > > > + PMICs chip family. > > > > Module name? > > I'm not sure to understand what you are looking for. Something like this > ?: > > To compile this driver as a module, choose M here: the > module will be called pinctrl-tps6594. Yes. ... > > > + pinctrl->pctl_dev = > > > + devm_pinctrl_register(&pdev->dev, pctrl_desc, pinctrl); > > > > One line? > > I use clang-format quite extensively so I would rather keep it like > that to still be able to just run it over the whole file without having > to fix this line every time. > If you feel like I should not respect the 80 columns recommendation, I > will fix it. As you wish. ... > > > -#define TPS6594_REG_GPIOX_CONF(gpio_inst) (0x31 + (gpio_inst)) > > > +#define TPS6594_REG_GPIO1_CONF 0x31 > > > +#define TPS6594_REG_GPIOX_CONF(gpio_inst) (TPS6594_REG_GPIO1_CONF + (gpio_inst)) > > > > Why? The original code with parameter 0 will issue the same. > > I felt that replacing 0x31 with a constant would make the computation > in TPS6594_REG_GPIOX_CONFIG more understandable. What do you think? The question is why that register is so special that you need to have it as a constant explicitly? -- With Best Regards, Andy Shevchenko