[U-Boot] [PATCH v2] power: extend prefix match to regulator-name property

Jaehoon Chung jh80.chung at samsung.com
Fri Oct 20 13:34:04 UTC 2017


On 10/18/2017 06:39 PM, Felix Brack wrote:
> This patch extends pmic_bind_children prefix matching. In addition to
> the node name the property regulator-name is used while trying to match
> prefixes. This allows assigning different drivers to regulator nodes
> named regulator at 1 and regulator at 10 for example.
> I have discarded the idea of using other properties then regulator-name
> as I do not see any benefit in using property compatible or even
> regulator-compatible. Of course I am open to change this if there are
> good reasons to do so.
> 
> Signed-off-by: Felix Brack <fb at ltec.ch>
> ---
> 
> Changes in v2:
> - add documentation
> - add a regulator to the sandbox for testing
> - extend the test for the new sandbox regulator
> 
>  arch/sandbox/dts/sandbox_pmic.dtsi               |  6 ++++++
>  doc/device-tree-bindings/regulator/regulator.txt | 16 ++++++++++++++--
>  drivers/power/pmic/pmic-uclass.c                 | 15 +++++++++++++--
>  include/power/sandbox_pmic.h                     |  5 ++++-
>  test/dm/regulator.c                              |  2 ++
>  5 files changed, 39 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/sandbox/dts/sandbox_pmic.dtsi b/arch/sandbox/dts/sandbox_pmic.dtsi
> index ce261b9..acb4799 100644
> --- a/arch/sandbox/dts/sandbox_pmic.dtsi
> +++ b/arch/sandbox/dts/sandbox_pmic.dtsi
> @@ -75,4 +75,10 @@
>  		regulator-min-microvolt = <3300000>;
>  		regulator-max-microvolt = <3300000>;
>  	};
> +
> +	no_match_by_nodename {
> +		regulator-name = "buck_SUPPLY_1.5V";
> +		regulator-min-microvolt = <1500000>;
> +		regulator-max-microvolt = <1500000>;
> +	};
>  };
> diff --git a/doc/device-tree-bindings/regulator/regulator.txt b/doc/device-tree-bindings/regulator/regulator.txt
> index 918711e..65b69c4 100644
> --- a/doc/device-tree-bindings/regulator/regulator.txt
> +++ b/doc/device-tree-bindings/regulator/regulator.txt
> @@ -2,7 +2,8 @@ Voltage/Current regulator
>  
>  Binding:
>  The regulator devices don't use the "compatible" property. The binding is done
> -by the prefix of regulator node's name. Usually the pmic I/O driver will provide
> +by the prefix of regulator node's name, or, if this fails, by the prefix of the
> +regulator's "regulator-name" property. Usually the pmic I/O driver will provide
>  the array of 'struct pmic_child_info' with the prefixes and compatible drivers.
>  The bind is done by calling function: pmic_bind_childs().
>  Example drivers:
> @@ -15,8 +16,19 @@ For the node name e.g.: "prefix[:alpha:]num { ... }":
>  
>  Example the prefix "ldo" will pass for: "ldo1", "ldo at 1", "ldoreg at 1, ...
>  
> +Binding by means of the node's name is preferred. However if the node names
> +would produce ambiguous prefixes (like "regulator at 1" and "regualtor at 11") and you
> +can't or do not want to change them then binding against the "regulator-name"
> +property is possible. The syntax for the prefix of the "regulator-name" property
> +is the same as the one for the regulator's node name.
> +Use case: a regulator named "regulator at 1" to be bound to a driver named
> +"LDO_DRV" and a regulator named "regualator at 11" to be bound to an other driver
> +named "BOOST_DRV". Using prefix "regualtor at 1" for driver matching would load
> +the same driver for both regulators, hence the prefix is ambiguous.
> +
>  Optional properties:
> -- regulator-name: a string, required by the regulator uclass
> +- regulator-name: a string, required by the regulator uclass, used for driver
> +                  binding if binding by node's name prefix fails
>  - regulator-min-microvolt: a minimum allowed Voltage value
>  - regulator-max-microvolt: a maximum allowed Voltage value
>  - regulator-min-microamp: a minimum allowed Current value
> diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c
> index 64964e4..5a034f0 100644
> --- a/drivers/power/pmic/pmic-uclass.c
> +++ b/drivers/power/pmic/pmic-uclass.c
> @@ -26,6 +26,7 @@ int pmic_bind_children(struct udevice *pmic, ofnode parent,
>  	struct driver *drv;
>  	struct udevice *child;
>  	const char *node_name;
> +	const char *reg_name;
>  	int bind_count = 0;
>  	ofnode node;
>  	int prefix_len;
> @@ -44,8 +45,18 @@ int pmic_bind_children(struct udevice *pmic, ofnode parent,
>  			debug("  - compatible prefix: '%s'\n", info->prefix);
>  
>  			prefix_len = strlen(info->prefix);
> -			if (strncmp(info->prefix, node_name, prefix_len))
> -				continue;
> +			if (strncmp(info->prefix, node_name, prefix_len)) {
> +				reg_name = ofnode_read_string(node,
> +							      "regulator-name");
> +				if (reg_name) {
> +					if (strncmp(info->prefix, reg_name,
> +						    prefix_len)) {
> +						continue;
> +					}
> +				} else {
> +					continue;
> +				}The below code is more readable?

if (!reg_name)
	continue;

if (strncmp())
	continue;

...

how about?


> +			}
>  
>  			drv = lists_driver_lookup_name(info->driver);
>  			if (!drv) {
> diff --git a/include/power/sandbox_pmic.h b/include/power/sandbox_pmic.h
> index 7fdbfb9..da2c296 100644
> --- a/include/power/sandbox_pmic.h
> +++ b/include/power/sandbox_pmic.h
> @@ -13,7 +13,7 @@
>  #define SANDBOX_BUCK_DRIVER		"sandbox_buck"
>  #define SANDBOX_OF_BUCK_PREFIX		"buck"
>  
> -#define SANDBOX_BUCK_COUNT	2
> +#define SANDBOX_BUCK_COUNT	3
>  #define SANDBOX_LDO_COUNT	2
>  /*
>   * Sandbox PMIC registers:
> @@ -114,6 +114,9 @@ enum {
>  #define SANDBOX_LDO1_PLATNAME	"VDD_EMMC_1.8V"
>  #define SANDBOX_LDO2_DEVNAME	"ldo2"
>  #define SANDBOX_LDO2_PLATNAME	"VDD_LCD_3.3V"
> +/* BUCK3: for testing fallback regulator prefix matching during bind */
> +#define SANDBOX_BUCK3_DEVNAME	"no_match_by_nodename"
> +#define SANDBOX_BUCK3_PLATNAME	"buck_SUPPLY_1.5V"

These defined might be located on above point. (SANDBOX_BUCK2_xxxx)

Best Regards,
Jaehoon Chung

>  
>  /*
>   * Expected regulators setup after call of:
> diff --git a/test/dm/regulator.c b/test/dm/regulator.c
> index 3d0056f..395381d 100644
> --- a/test/dm/regulator.c
> +++ b/test/dm/regulator.c
> @@ -27,6 +27,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  enum {
>  	BUCK1,
>  	BUCK2,
> +	BUCK3,
>  	LDO1,
>  	LDO2,
>  	OUTPUT_COUNT,
> @@ -42,6 +43,7 @@ static const char *regulator_names[OUTPUT_COUNT][OUTPUT_NAME_COUNT] = {
>  	/* devname, platname */
>  	{ SANDBOX_BUCK1_DEVNAME, SANDBOX_BUCK1_PLATNAME },
>  	{ SANDBOX_BUCK2_DEVNAME, SANDBOX_BUCK2_PLATNAME },
> +	{ SANDBOX_BUCK3_DEVNAME, SANDBOX_BUCK3_PLATNAME },
>  	{ SANDBOX_LDO1_DEVNAME, SANDBOX_LDO1_PLATNAME},
>  	{ SANDBOX_LDO2_DEVNAME, SANDBOX_LDO2_PLATNAME},
>  };
> 



More information about the U-Boot mailing list