On Tue, 15 Feb 2022, Andy Shevchenko wrote:
> 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.
Don't you over-write this entry entirely?
for (i = 0; i < ARRAY_SIZE(apl_gpio_devices); i++) {
struct resource *mem = &apl_gpio_resources[i][0];
/* Fill MEM resource */
mem->start += base.start;
mem->end += base.start;
mem->flags = base.flags;
}
Oh wait, you're just adding the base value to the offsets.
In which case that comment is also confusing!
--
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
