Messages in this thread | | | Date | Sun, 20 Sep 2015 05:20:03 +0100 | From | Lee Jones <> | Subject | Re: [PATCH] mfd: tps6105x: Fix NULL pointer access. |
| |
On Fri, 04 Sep 2015, Grigoryev Denis wrote:
> When tps6105x used in TPS6105X_MODE_SHUTDOWN mode the driver calls > mfd_add_devices() with mfd_cell->name == NULL, that causes an ooops in > platform_device_register() later. > > This patch reorders mfd_cell structures and makes code to call > mfd_add_devices() with proper number of cells. > > Signed-off-by: Denis Grigoryev <grigoryev@spb.prosoft.ru> > --- > drivers/mfd/tps6105x.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/drivers/mfd/tps6105x.c b/drivers/mfd/tps6105x.c > index 5de95c2..be2c286 100644 > --- a/drivers/mfd/tps6105x.c > +++ b/drivers/mfd/tps6105x.c > @@ -124,11 +124,11 @@ static int tps6105x_startup(struct tps6105x *tps6105x) > */ > static struct mfd_cell tps6105x_cells[] = { > { > - /* name will be runtime assigned */ > + .name = "tps6105x-gpio", > .id = -1, > }, > { > - .name = "tps6105x-gpio", > + /* name will be runtime assigned */ > .id = -1, > }, > };
So you have 2 cells with identical .name's and identical .id's.
How does that work?
> @@ -138,6 +138,7 @@ static int tps6105x_probe(struct i2c_client *client, > { > struct tps6105x *tps6105x; > struct tps6105x_platform_data *pdata; > + int n_cells = ARRAY_SIZE(tps6105x_cells); > int ret; > int i; > > @@ -162,33 +163,34 @@ static int tps6105x_probe(struct i2c_client *client, > case TPS6105X_MODE_SHUTDOWN: > dev_info(&client->dev, > "present, not used for anything, only GPIO\n"); > + n_cells = 1; > break; > case TPS6105X_MODE_TORCH: > - tps6105x_cells[0].name = "tps6105x-leds"; > + tps6105x_cells[1].name = "tps6105x-leds";
Now you're over-writing cell names! I'd say this was a hack.
Please just create an entry for each device, like usual.
> dev_warn(&client->dev, > "torch mode is unsupported\n"); > break; > case TPS6105X_MODE_TORCH_FLASH: > - tps6105x_cells[0].name = "tps6105x-flash"; > + tps6105x_cells[1].name = "tps6105x-flash"; > dev_warn(&client->dev, > "flash mode is unsupported\n"); > break; > case TPS6105X_MODE_VOLTAGE: > - tps6105x_cells[0].name ="tps6105x-regulator"; > + tps6105x_cells[1].name = "tps6105x-regulator"; > break; > default: > break; > } > > /* Set up and register the platform devices. */ > - for (i = 0; i < ARRAY_SIZE(tps6105x_cells); i++) { > + for (i = 0; i < n_cells; i++) { > /* One state holder for all drivers, this is simple */ > tps6105x_cells[i].platform_data = tps6105x; > tps6105x_cells[i].pdata_size = sizeof(*tps6105x); > } > > return mfd_add_devices(&client->dev, 0, tps6105x_cells, > - ARRAY_SIZE(tps6105x_cells), NULL, 0, NULL); > + n_cells, NULL, 0, NULL); > } > > static int tps6105x_remove(struct i2c_client *client)
-- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
| |