On Thu, Mar 4, 2021 at 10:57 AM Álvaro Fernández Rojas
<[email protected]> 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.
Also, why is it inside a function? It's anyway global and constant.
> + };
...
> + regmap_update_bits(pc->regs, bcm6328_mux_off(pin),
> + 3UL << ((pin % 16) * 2),
3UL => #define BLABLA GENMASK(1, 0)
> + mux << ((pin % 16) * 2));
...
> +static const struct of_device_id bcm6328_pinctrl_match[] = {
> + { .compatible = "brcm,bcm6328-pinctrl", },
> + { },
No comma.
> +};
Above comments to all your patches.
--
With Best Regards,
Andy Shevchenko
Hi Andy,
> El 4 mar 2021, a las 11:49, Andy Shevchenko <[email protected]> escribió:
>
> On Thu, Mar 4, 2021 at 10:57 AM Álvaro Fernández Rojas
> <[email protected]> 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