Hi Andy,
> El 4 mar 2021, a las 11:49, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> escribió:
>
> On Thu, Mar 4, 2021 at 10:57 AM Álvaro Fernández Rojas
> <noltari@xxxxxxxxx> wrote:
>>
>> 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.
>
> ...
>
>> +static inline unsigned int bcm6328_mux_off(unsigned int pin)
>> +{
>> + static const unsigned int bcm6328_mux[] = {
>> + BCM6328_MUX_LO_REG,
>> + BCM6328_MUX_HI_REG,
>
>> + 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 :)
>
>> + };
>
> ...
>
>> + 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.
>
>> + mux << ((pin % 16) * 2));
>
> ...
>
>> +static const struct of_device_id bcm6328_pinctrl_match[] = {
>> + { .compatible = "brcm,bcm6328-pinctrl", },
>> + { },
>
> No comma.
Ok, I will remove the comma.
>
>> +};
>
> Above comments to all your patches.
Ok, I will try to address all of this comments in v5.
>
> --
> With Best Regards,
> Andy Shevchenko
