linux-kernel - Re: [PATCH V3] regulator: palmas: add support for external control of rails

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <52139DC9.4040803@wwwdotorg.org>
Date:	Tue, 20 Aug 2013 10:48:09 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Laxman Dewangan <ldewangan@...dia.com>
CC:	broonie@...nel.org, sameo@...ux.intel.com, rob.herring@...xeda.com,
	pawel.moll@....com, mark.rutland@....com, rob@...dley.net,
	lee.jones@...aro.org, grant.likely@...aro.org,
	devicetree@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org, gg@...mlogic.co.uk, kishon@...com,
	j-keerthy@...com, ian.campbell@...rix.com
Subject: Re: [PATCH V3] regulator: palmas: add support for external control
 of rails

On 08/20/2013 07:14 AM, Laxman Dewangan wrote:
> Palmas rails like LDOs, SMPSs, REGENs, SYSENs can be enable and disable
> by register programming through I2C communication as well as it can be
> enable/disable with the external control input ENABLE1, ENABLE2 and NSLEEP.
> 
> Add support for configuring these rails to be controlled by external control
> inputs. This is require to configure the rail's control register as well as
> configuration of resource register.
> 
> Provide the external input names through parameter "roof-floor". Updated the
> DT binding document to details different value of the roof-floor.

> diff --git a/Documentation/devicetree/bindings/regulator/palmas-pmic.txt b/Documentation/devicetree/bindings/regulator/palmas-pmic.txt

The binding changes look reasonable and backwards-compatible, so they're
good. Just one nit:

(In the example, I assume:)
>  			regulator-always-on;
>  			regulator-boot-on;
>  			ti,warm-reset;
> -			ti,roof-floor;
> +			ti,roof-floor = 0x1; /* ENABLE1 control */

You need <> around the value, so better:

			ti,roof-floor = <0x1>; /* ENABLE1 control */

or better:

			ti,roof-floor = <1>; /* ENABLE1 control */

(even better might be a header file that defines names for those
constants...)

> diff --git a/drivers/regulator/palmas-regulator.c b/drivers/regulator/palmas-regulator.c

> @@ -746,9 +854,24 @@ static void palmas_dt_to_pdata(struct device *dev,
>  			of_property_read_bool(palmas_matches[idx].of_node,
>  					     "ti,warm-reset");
>  
> -		pdata->reg_init[idx]->roof_floor =
> -			of_property_read_bool(palmas_matches[idx].of_node,
> -					      "ti,roof-floor");
> +		ret = of_property_read_u32(palmas_matches[idx].of_node,
> +					      "ti,roof-floor", &prop);
> +		/* EINVAL: Property not found */
> +		if (ret != -EINVAL) {
> +			int econtrol;
> +
> +			/* use default value, when no value is specified */
> +			econtrol = PALMAS_EXT_CONTROL_NSLEEP;
> +			if (!ret) {
> +				if (prop == 1)
> +					econtrol = PALMAS_EXT_CONTROL_ENABLE1;
> +				else if (prop == 2)
> +					econtrol = PALMAS_EXT_CONTROL_ENABLE2;
> +				else if (prop == 3)
> +					econtrol = PALMAS_EXT_CONTROL_NSLEEP;

Should probably at least WARN() here if the value is invalid?

> +			}
> +			pdata->reg_init[idx]->roof_floor = econtrol;
> +		}

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/