Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756810AbcDLKj3 (ORCPT ); Tue, 12 Apr 2016 06:39:29 -0400 Received: from down.free-electrons.com ([37.187.137.238]:60265 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756090AbcDLKj2 (ORCPT ); Tue, 12 Apr 2016 06:39:28 -0400 Date: Tue, 12 Apr 2016 12:39:16 +0200 From: Boris Brezillon To: Mark Brown , Liam Girdwood Cc: linux-kernel@vger.kernel.org, Caesar Wang , Heiko =?UTF-8?B?U3TDvGJuZXI=?= , Stephen Barber , Douglas Anderson , Brian Norris Subject: Re: [PATCH] regulator: reorder initialization steps in regulator_register() Message-ID: <20160412123916.54a0428e@bbrezillon> In-Reply-To: <1460457060-3946-1-git-send-email-boris.brezillon@free-electrons.com> References: <1460457060-3946-1-git-send-email-boris.brezillon@free-electrons.com> X-Mailer: Claws Mail 3.12.0 (GTK+ 2.24.28; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2462 Lines: 83 On Tue, 12 Apr 2016 12:31:00 +0200 Boris Brezillon wrote: > device_register() is calling ->get_voltage() as part of is sysfs attribute ^ its > initialization process, and this functions might need to know the regulator > constraints to return a valid value. > This is at least true for the pwm regulator driver (when operating in > continuous mode) which needs to know the minimum and maximum voltage values > to calculate the current voltage: > > min_uV + (((max_uV - min_uV) * dutycycle) / 100); > > Move device_register() after set_machine_constraints() to make sure those > constraints are correctly initialized when ->get_voltage() is called. > > Signed-off-by: Boris Brezillon > Reported-by: Stephen Barber > --- > drivers/regulator/core.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c > index e0b7642..8258568 100644 > --- a/drivers/regulator/core.c > +++ b/drivers/regulator/core.c > @@ -3950,13 +3950,6 @@ regulator_register(const struct regulator_desc *regulator_desc, > rdev->dev.parent = dev; > dev_set_name(&rdev->dev, "regulator.%lu", > (unsigned long) atomic_inc_return(®ulator_no)); > - ret = device_register(&rdev->dev); > - if (ret != 0) { > - put_device(&rdev->dev); > - goto wash; > - } > - > - dev_set_drvdata(&rdev->dev, rdev); > > /* set regulator constraints */ > if (init_data) > @@ -3964,7 +3957,15 @@ regulator_register(const struct regulator_desc *regulator_desc, > > ret = set_machine_constraints(rdev, constraints); > if (ret < 0) > - goto scrub; > + goto wash; > + > + ret = device_register(&rdev->dev); > + if (ret != 0) { > + put_device(&rdev->dev); > + goto wash; > + } > + > + dev_set_drvdata(&rdev->dev, rdev); > > if (init_data && init_data->supply_regulator) > rdev->supply_name = init_data->supply_regulator; > @@ -3993,8 +3994,6 @@ out: > > unset_supplies: > unset_regulator_supplies(rdev); > - > -scrub: > regulator_ena_gpio_free(rdev); > device_unregister(&rdev->dev); > /* device core frees rdev */ > @@ -4002,6 +4001,7 @@ scrub: > goto out; > > wash: > + kfree(rdev->constraints); > regulator_ena_gpio_free(rdev); > clean: > kfree(rdev); -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com