Re: [PATCH v3 1/6] pinctrl: Add pingroup and define PINCTRL_GRP

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Tue, May 31, 2022 at 02:13:17PM +0530, Basavaraj Natikar wrote:
> Add 'struct pingroup' to represent pingroup and 'PINCTRL_GRP' macro for
> inline use. Both are used to manage and represent larger number
> of pingroups.


Thanks! My comments below.

...

> +/**
> + * struct pingroups - provides information on pingroup

Try `make W=1` against each of your patches from the series. Here is the kernel
doc issue.

> + * @name: a name for pingroup
> + * @pins: an array of pins in the pingroup
> + * @npins: number of pins in the pingroup
> + */
> +struct pingroup {
> +	const char *name;
> +	const unsigned int *pins;

> +	unsigned int npins;

size_t probably would be better.

> +};
> +
> +/* Convenience macro to define a single named or anonymous pingroup */
> +#define PINCTRL_GRP(_name, _pins, _npins)	\

I think PINCTRL_PINGROUP would be more precise.

> +((struct pingroup) {				\

No need to have space before { and compound literal means that it's not a GCC
expression, i.e. drop outer parentheses ().

> +	.name = _name,				\
> +	.pins = _pins,				\
> +	.npins = _npins,			\
> +})

-- 
With Best Regards,
Andy Shevchenko