[lm-sensors] [PATCH] hwmon: (lm90) Fix driver to work on standard PCs with CONFIG_REGULATOR enabled

[lm-sensors] [PATCH] hwmon: (lm90) Fix driver to work on standard PCs with CONFIG_REGULATOR enabled

[Guenter Roeck]

On Ubuntu systems with CONFIG_REGULATOR enabled, the lm90 driver fails to load
with an error mesage such as

i2c 1-0018: Driver lm90 requests probe deferral

This is a result of commit 3e0f964f2ad (hwmon: (lm90) Add power control)
which adds mandatory regulator support to the lm90 driver. On non-dt systems
with CONFIG_REGULATOR enabled, this fails if regulators are not fully
specified. This is the case on a standard PC system.

To fix the problem, make the regulator on non-dt systems optional.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/lm90.c |   30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
index bc41682..51932da 100644
--- a/drivers/hwmon/lm90.c
+++ b/drivers/hwmon/lm90.c
@@ -1520,15 +1520,23 @@ static int lm90_probe(struct i2c_client *client,
 	struct regulator *regulator;
 	int err;
 
+	/*
+	 * On non-dt systems, the regulator subsystem expects regulators
+	 * to be fully constrained if enabled, which is an unreasonable
+	 * expectation from the perspective of this driver.
+	 * Therefore, ignore errors on such systems.
+	 */
 	regulator = devm_regulator_get(dev, "vcc");
-	if (IS_ERR(regulator))
-		return PTR_ERR(regulator);
-
-	err = regulator_enable(regulator);
-	if (err < 0) {
-		dev_err(&client->dev,
-			"Failed to enable regulator: %d\n", err);
-		return err;
+	if (IS_ERR(regulator)) {
+		if (IS_ENABLED(CONFIG_OF))
+			return PTR_ERR(regulator);
+	} else {
+		err = regulator_enable(regulator);
+		if (err < 0) {
+			dev_err(&client->dev,
+				"Failed to enable regulator: %d\n", err);
+			return err;
+		}
 	}
 
 	data = devm_kzalloc(&client->dev, sizeof(struct lm90_data), GFP_KERNEL);
@@ -1621,7 +1629,8 @@ exit_remove_files:
 	lm90_remove_files(client, data);
 exit_restore:
 	lm90_restore_conf(client, data);
-	regulator_disable(data->regulator);
+	if (!IS_ERR_OR_NULL(data->regulator))
+		regulator_disable(data->regulator);
 
 	return err;
 }
@@ -1633,7 +1642,8 @@ static int lm90_remove(struct i2c_client *client)
 	hwmon_device_unregister(data->hwmon_dev);
 	lm90_remove_files(client, data);
 	lm90_restore_conf(client, data);
-	regulator_disable(data->regulator);
+	if (!IS_ERR_OR_NULL(data->regulator))
+		regulator_disable(data->regulator);
 
 	return 0;
 }
-- 
1.7.9.7


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (lm90) Fix driver to work on standard PCs with CONFIG_REGULATOR enab

[Guenter Roeck]

On 01/26/2014 03:08 PM, Guenter Roeck wrote:
> On Ubuntu systems with CONFIG_REGULATOR enabled, the lm90 driver fails to load
> with an error mesage such as
>
> i2c 1-0018: Driver lm90 requests probe deferral
>
> This is a result of commit 3e0f964f2ad (hwmon: (lm90) Add power control)
> which adds mandatory regulator support to the lm90 driver. On non-dt systems
> with CONFIG_REGULATOR enabled, this fails if regulators are not fully
> specified. This is the case on a standard PC system.
>
Mark appears to object to that idea, so maybe we should just just revert
3e0f964f2ad until a more acceptable solution is found.

Guenter

> To fix the problem, make the regulator on non-dt systems optional.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
>   drivers/hwmon/lm90.c |   30 ++++++++++++++++++++----------
>   1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
> index bc41682..51932da 100644
> --- a/drivers/hwmon/lm90.c
> +++ b/drivers/hwmon/lm90.c
> @@ -1520,15 +1520,23 @@ static int lm90_probe(struct i2c_client *client,
>   	struct regulator *regulator;
>   	int err;
>
> +	/*
> +	 * On non-dt systems, the regulator subsystem expects regulators
> +	 * to be fully constrained if enabled, which is an unreasonable
> +	 * expectation from the perspective of this driver.
> +	 * Therefore, ignore errors on such systems.
> +	 */
>   	regulator = devm_regulator_get(dev, "vcc");
> -	if (IS_ERR(regulator))
> -		return PTR_ERR(regulator);
> -
> -	err = regulator_enable(regulator);
> -	if (err < 0) {
> -		dev_err(&client->dev,
> -			"Failed to enable regulator: %d\n", err);
> -		return err;
> +	if (IS_ERR(regulator)) {
> +		if (IS_ENABLED(CONFIG_OF))
> +			return PTR_ERR(regulator);
> +	} else {
> +		err = regulator_enable(regulator);
> +		if (err < 0) {
> +			dev_err(&client->dev,
> +				"Failed to enable regulator: %d\n", err);
> +			return err;
> +		}
>   	}
>
>   	data = devm_kzalloc(&client->dev, sizeof(struct lm90_data), GFP_KERNEL);
> @@ -1621,7 +1629,8 @@ exit_remove_files:
>   	lm90_remove_files(client, data);
>   exit_restore:
>   	lm90_restore_conf(client, data);
> -	regulator_disable(data->regulator);
> +	if (!IS_ERR_OR_NULL(data->regulator))
> +		regulator_disable(data->regulator);
>
>   	return err;
>   }
> @@ -1633,7 +1642,8 @@ static int lm90_remove(struct i2c_client *client)
>   	hwmon_device_unregister(data->hwmon_dev);
>   	lm90_remove_files(client, data);
>   	lm90_restore_conf(client, data);
> -	regulator_disable(data->regulator);
> +	if (!IS_ERR_OR_NULL(data->regulator))
> +		regulator_disable(data->regulator);
>
>   	return 0;
>   }
>


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (lm90) Fix driver to work on standard PCs with CONFIG_REGULATOR enab

[Jean Delvare]

On Sun, 26 Jan 2014 20:21:00 -0800, Guenter Roeck wrote:
> On 01/26/2014 03:08 PM, Guenter Roeck wrote:
> > On Ubuntu systems with CONFIG_REGULATOR enabled, the lm90 driver fails to load
> > with an error mesage such as
> >
> > i2c 1-0018: Driver lm90 requests probe deferral
> >
> > This is a result of commit 3e0f964f2ad (hwmon: (lm90) Add power control)
> > which adds mandatory regulator support to the lm90 driver. On non-dt systems
> > with CONFIG_REGULATOR enabled, this fails if regulators are not fully
> > specified. This is the case on a standard PC system.
> >
> Mark appears to object to that idea, so maybe we should just just revert
> 3e0f964f2ad until a more acceptable solution is found.

Well, I object as well, so I'm happy to not apply it. I hate seeing
IS_ENABLED() creep into driver code. Any issue like that should be
handled as a Kconfig dependency or with a better API.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (lm90) Fix driver to work on standard PCs with CONFIG_REGULATOR enab

[Guenter Roeck]

On 01/27/2014 12:14 AM, Jean Delvare wrote:
> On Sun, 26 Jan 2014 20:21:00 -0800, Guenter Roeck wrote:
>> On 01/26/2014 03:08 PM, Guenter Roeck wrote:
>>> On Ubuntu systems with CONFIG_REGULATOR enabled, the lm90 driver fails to load
>>> with an error mesage such as
>>>
>>> i2c 1-0018: Driver lm90 requests probe deferral
>>>
>>> This is a result of commit 3e0f964f2ad (hwmon: (lm90) Add power control)
>>> which adds mandatory regulator support to the lm90 driver. On non-dt systems
>>> with CONFIG_REGULATOR enabled, this fails if regulators are not fully
>>> specified. This is the case on a standard PC system.
>>>
>> Mark appears to object to that idea, so maybe we should just just revert
>> 3e0f964f2ad until a more acceptable solution is found.
>
> Well, I object as well, so I'm happy to not apply it. I hate seeing
> IS_ENABLED() creep into driver code. Any issue like that should be
> handled as a Kconfig dependency or with a better API.
>



Going back to the original problem, lm90 no longer works in any non-DT
system which has CONFIG_REGULATORS enabled, such as any Ubuntu distribution.

I agree that it would have been better if the regulator subsystem would deal
with the issue, but that seems unlikely to happen. If anything, it will get
worse with the assumption that regulators are fully declared in any ACPI
based system (such a system for sure won't know about an USB-I2C adapter
with an LM90 connected to it). So what do you want to do ?

Guenter


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] hwmon: (lm90) Fix driver to work on standard PCs with CONFIG_REGULATOR enab

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1044 bytes --]

On Mon, Jan 27, 2014 at 06:39:39AM -0800, Guenter Roeck wrote:

> Going back to the original problem, lm90 no longer works in any non-DT
> system which has CONFIG_REGULATORS enabled, such as any Ubuntu distribution.

*sigh*  To repeat yet again this is not associated with DT in the way
that you seem to believe it is.

> I agree that it would have been better if the regulator subsystem would deal
> with the issue, but that seems unlikely to happen. If anything, it will get
> worse with the assumption that regulators are fully declared in any ACPI
> based system (such a system for sure won't know about an USB-I2C adapter
> with an LM90 connected to it). So what do you want to do ?

I don't think you understand what _has_full_constraints() does, the way
that you're still going on about DT seems to suggest that this is the
case.  A USB-I2C adaptor will work in exactly the same way as anything
on the motherboard in an ACPI system unless it explicitly ensures that
it registers any software controllable supplies which also works fine.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors