On Tue, Feb 15, 2022 at 04:54:00PM +0000, Lee Jones wrote:
> On Mon, 31 Jan 2022, Andy Shevchenko wrote:
Thank you for the review, my answers below.
...
> > +static struct resource apl_gpio_resources[APL_GPIO_NR_DEVICES][2] = {
> > + [APL_GPIO_NORTH] = {
> > + DEFINE_RES_MEM(APL_GPIO_NORTH_OFFSET, 0x1000),
>
> Are these 0x1000's being over-written in lpc_ich_init_pinctrl()?
>
> If so, why pre-initialise?
You mean to pre-initialize the offsets, but leave the length to be added
in the function? It can be done, but it feels inconsistent, since we would
have offsets and lengths in different places for the same thingy. That said,
I prefer current way for the sake of consistency.
> > + DEFINE_RES_IRQ(APL_GPIO_IRQ),
> > + },
...
> > +/* The order must be in sync with apl_pinctrl_soc_data */
>
> Why does the order matter if you've pre-enumerated them all?
Indeed. I will drop the confusing comment in the next version.
--
With Best Regards,
Andy Shevchenko
