On Wed, Jun 18, 2014 at 09:11:59AM +0200, Boris BREZILLON wrote: > > On 17/06/2014 22:44, Maxime Ripard wrote: > > On Tue, Jun 17, 2014 at 09:38:40AM +0200, Boris BREZILLON wrote: > >> The init_data and of_node fields of the axp2xx_matches tables are filled > >> at each device probe by the axp20x_regulator_parse_dt function (which then > >> calls the of_regulator_match function). > >> This means we can probe a new device and consider data initialized during > >> the probe of another device as valid. > >> > >> Reset init_data and of_node field to NULL before each probe in order to > >> avoid this kind of issue. > >> > >> Signed-off-by: Boris BREZILLON <boris.brezillon@xxxxxxxxxxxxxxxxxx> > >> --- > >> drivers/regulator/axp20x-regulator.c | 9 +++++++++ > >> 1 file changed, 9 insertions(+) > >> > >> diff --git a/drivers/regulator/axp20x-regulator.c b/drivers/regulator/axp20x-regulator.c > >> index 7a30f49..d42bf6d 100644 > >> --- a/drivers/regulator/axp20x-regulator.c > >> +++ b/drivers/regulator/axp20x-regulator.c > >> @@ -324,6 +324,15 @@ static int axp20x_regulator_probe(struct platform_device *pdev) > >> nregulators = AXP20X_REG_ID_MAX; > >> } > >> > >> + /* > >> + * Reset matches table (this table might have been modified by a > >> + * previous AXP2xx device probe). > >> + */ > >> + for (i = 0; i < nmatches; i++) { > >> + matches[i].init_data = NULL; > >> + matches[i].of_node = NULL; > >> + } > >> + > > That looks rather hackish, especially since we've never been in such a > > case yet, since we have a single PMIC in our system. > > Even if something is unlikely to happen, it doesn't mean it's impossible. > I'm pretty sure there are (or will be) some systems containing several > identical PMICs in the wild, and fixing this possible bug now prevents > us (or other developers) from having a big headache debugging this in > the future. > > BTW, what is hackish in this code ? Pretty what Hans was saying, either you think that there will only be one single instance of the driver, and using a global definition is fine, or you can have several instances of the driver, and in this case you'll use a dynamic allocation, but you seem to be stuck in between. I understand that you might not want to redeclare by hand the whole match content, so maybe you can just use memcpy from the global definition then. Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
Attachment:
signature.asc
Description: Digital signature