On Mon, 21 Oct 2019, Daniel Thompson wrote: > On Mon, Oct 21, 2019 at 11:58:15AM +0100, Lee Jones wrote: > > The current implementation abuses the platform 'id' mfd_cell member > > to index into the correct resources entry. If we place all cells > > into their numbered slots, we can cycle through all the cell entries > > and only process the populated ones which avoids this behaviour. > > > > Signed-off-by: Lee Jones <[email protected]> > > --- > > drivers/mfd/cs5535-mfd.c | 31 +++++++++++++------------------ > > 1 file changed, 13 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/mfd/cs5535-mfd.c b/drivers/mfd/cs5535-mfd.c > > index 2c47afc22d24..9ce6bbcdbda1 100644 > > --- a/drivers/mfd/cs5535-mfd.c > > +++ b/drivers/mfd/cs5535-mfd.c > > @@ -62,26 +62,22 @@ static int cs5535_mfd_res_disable(struct > > platform_device *pdev) > > static struct resource cs5535_mfd_resources[NR_BARS]; > > > > static struct mfd_cell cs5535_mfd_cells[] = { > > This array is sized from the initializer... > > > - { > > - .id = SMB_BAR, > > + [SMB_BAR] = { > > .name = "cs5535-smb", > > .num_resources = 1, > > .resources = &cs5535_mfd_resources[SMB_BAR], > > }, > > - { > > - .id = GPIO_BAR, > > + [GPIO_BAR] = { > > .name = "cs5535-gpio", > > .num_resources = 1, > > .resources = &cs5535_mfd_resources[GPIO_BAR], > > }, > > - { > > - .id = MFGPT_BAR, > > + [MFGPT_BAR] = { > > .name = "cs5535-mfgpt", > > .num_resources = 1, > > .resources = &cs5535_mfd_resources[MFGPT_BAR], > > }, > > - { > > - .id = PMS_BAR, > > + [PMS_BAR] = { > > .name = "cs5535-pms", > > .num_resources = 1, > > .resources = &cs5535_mfd_resources[PMS_BAR], > > @@ -89,8 +85,7 @@ static struct mfd_cell cs5535_mfd_cells[] = { > > .enable = cs5535_mfd_res_enable, > > .disable = cs5535_mfd_res_disable, > > }, > > - { > > - .id = ACPI_BAR, > > + [ACPI_BAR] = { > > .name = "cs5535-acpi", > > .num_resources = 1, > > .resources = &cs5535_mfd_resources[ACPI_BAR], > > @@ -115,16 +110,16 @@ static int cs5535_mfd_probe(struct pci_dev *pdev, > > return err; > > > > /* fill in IO range for each cell; subdrivers handle the region */ > > - for (i = 0; i < ARRAY_SIZE(cs5535_mfd_cells); i++) { > > - int bar = cs5535_mfd_cells[i].id; > > - struct resource *r = &cs5535_mfd_resources[bar]; > > + for (i = 0; i < NR_BARS; i++) { > > ... which means this translation from ARRAY_SIZE() to NR_BARS > is rather odd. > > I don't care whether the array is sized using NR_BARS or the loop > uses ARRAY_SIZE() but IMHO the loop boundary condition must match > the array declaration.
Sounds reasonable. > With that fixed free to throw the following onto the next rev: > Reviewed-by: Daniel Thompson <[email protected]> Ta. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
