Hi Andy,

El 4 mar 2021, a las 11:49, Andy Shevchenko [off-list ref] escribió:

On Thu, Mar 4, 2021 at 10:57 AM Álvaro Fernández Rojas
[off-list ref] wrote:

quoted

Add a pincontrol driver for BCM6328. BCM628 supports muxing 32 pins as
GPIOs, as LEDs for the integrated LED controller, or various other
functions. Its pincontrol mux registers also control other aspects, like
switching the second USB port between host and device mode.

...

quoted

+static inline unsigned int bcm6328_mux_off(unsigned int pin)
+{
+       static const unsigned int bcm6328_mux[] = {
+               BCM6328_MUX_LO_REG,
+               BCM6328_MUX_HI_REG,

quoted

+               BCM6328_MUX_OTHER_REG

When it's not terminator add a comma, otherwise remove a comma.

… so you want me to add a comma or not?

Also, why is it inside a function? It's anyway global and constant.

Because this is only used by this function and I’m used to doing this to comply with MISRA C 2004 at work :)

quoted

+       };

...

quoted

+       regmap_update_bits(pc->regs, bcm6328_mux_off(pin),
+                          3UL << ((pin % 16) * 2),

3UL => #define BLABLA  GENMASK(1, 0)

Ok, I will add a define for this.

quoted

+                          mux << ((pin % 16) * 2));

...

quoted

+static const struct of_device_id bcm6328_pinctrl_match[] = {
+       { .compatible = "brcm,bcm6328-pinctrl", },
+       { },

No comma.

Ok, I will remove the comma.

quoted

+};

Above comments to all your patches.

Ok, I will try to address all of this comments in v5.

-- 
With Best Regards,
Andy Shevchenko