[PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[Tim Bird]

Add a binding for the regulator which controls the OTG chargepath switch.
The OTG switch gets its power from pm8941_5vs1, and that should be
expressed as a usb-otg-in-supply property in the DT node for the
charger driver.  The regulator name is "otg".

Signed-off-by: Tim Bird <tim.bird@sonymobile.com>
---
Changes since v1
 - switch supply name to have dashes instead of underscores
 - remove superfluous DT explanations in the otg node description
---
 .../devicetree/bindings/power_supply/qcom_smbb.txt    | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt b/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
index 65b88fa..28b6da7 100644
--- a/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
+++ b/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
@@ -105,6 +105,22 @@ PROPERTIES
                regulation must be done externally to fully comply with
                the JEITA safety guidelines if this flag is set.
 
+- usb-otg-in-supply:
+  Usage: optional
+  Value type: <phandle>
+  Description: Reference to the regulator supplying power to the USB_OTG_IN
+               pin.
+
+child nodes:
+- otg:
+  Usage: optional
+  Description: This node defines a regulator used to control the direction
+               of VBUS voltage - specifically: whether to supply voltage
+               to VBUS for host mode operation of the OTG port, or allow
+               input voltage from external VBUS for charging.  In the
+               hardware, the supply for this regulator comes from
+               usb-otg-in-supply.
+
 EXAMPLE
 charger@1000 {
        compatible = "qcom,pm8941-charger";
@@ -128,4 +144,7 @@ charger@1000 {
 
        qcom,fast-charge-current-limit = <1000000>;
        qcom,dc-charge-current-limit = <1000000>;
+       usb-otg-in-supply = <&pm8941_5vs1>;
+
+       otg {};
 };
-- 
2.4.2

[PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[Tim Bird]

Add a binding for the regulator which controls the OTG chargepath switch.
The OTG switch gets its power from pm8941_5vs1, and that should be
expressed as a usb-otg-in-supply property in the DT node for the
charger driver.  The regulator name is "otg".

Signed-off-by: Tim Bird <tim.bird@sonymobile.com>
---
Changes since v1
 - switch supply name to have dashes instead of underscores
 - remove superfluous DT explanations in the otg node description
---
 .../devicetree/bindings/power_supply/qcom_smbb.txt    | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt b/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
index 65b88fa..28b6da7 100644
--- a/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
+++ b/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
@@ -105,6 +105,22 @@ PROPERTIES
                regulation must be done externally to fully comply with
                the JEITA safety guidelines if this flag is set.
 
+- usb-otg-in-supply:
+  Usage: optional
+  Value type: <phandle>
+  Description: Reference to the regulator supplying power to the USB_OTG_IN
+               pin.
+
+child nodes:
+- otg:
+  Usage: optional
+  Description: This node defines a regulator used to control the direction
+               of VBUS voltage - specifically: whether to supply voltage
+               to VBUS for host mode operation of the OTG port, or allow
+               input voltage from external VBUS for charging.  In the
+               hardware, the supply for this regulator comes from
+               usb-otg-in-supply.
+
 EXAMPLE
 charger@1000 {
        compatible = "qcom,pm8941-charger";
@@ -128,4 +144,7 @@ charger@1000 {
 
        qcom,fast-charge-current-limit = <1000000>;
        qcom,dc-charge-current-limit = <1000000>;
+       usb-otg-in-supply = <&pm8941_5vs1>;
+
+       otg {};
 };
-- 
2.4.2

[PATCH v2 2/3] power: qcom_smbb: Add otg regulator for control of vbus

[Tim Bird]

Add a regulator to control the OTG chargepath switch.  This
is used by USB code to control VBUS direction - out for host mode
on the OTG port, and in for charging mode.

Signed-off-by: Tim Bird <tim.bird@sonymobile.com>
---
Changes since v1:
 - changed name of supply to remove underscores
---
 drivers/power/qcom_smbb.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/drivers/power/qcom_smbb.c b/drivers/power/qcom_smbb.c
index 5eb1e9e..2f1394c 100644
--- a/drivers/power/qcom_smbb.c
+++ b/drivers/power/qcom_smbb.c
@@ -34,6 +34,9 @@
 #include <linux/power_supply.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
 
 #define SMBB_CHG_VMAX		0x040
 #define SMBB_CHG_VSAFE		0x041
@@ -71,6 +74,8 @@
 #define BTC_CTRL_HOT_EXT_N	BIT(0)
 
 #define SMBB_USB_IMAX		0x344
+#define SMBB_USB_OTG_CTL	0x348
+#define OTG_CTL_EN		BIT(0)
 #define SMBB_USB_ENUM_TIMER_STOP 0x34e
 #define ENUM_TIMER_STOP		BIT(0)
 #define SMBB_USB_SEC_ACCESS	0x3d0
@@ -123,6 +128,9 @@ struct smbb_charger {
 	struct power_supply *dc_psy;
 	struct power_supply *bat_psy;
 	struct regmap *regmap;
+
+	struct regulator_desc otg_rdesc;
+	struct regulator_dev *otg_reg;
 };
 
 static int smbb_vbat_weak_fn(unsigned int index)
@@ -778,12 +786,56 @@ static const struct power_supply_desc dc_psy_desc = {
 	.property_is_writeable = smbb_charger_writable_property,
 };
 
+static int smbb_chg_otg_enable(struct regulator_dev *rdev)
+{
+	struct smbb_charger *chg = rdev_get_drvdata(rdev);
+	int rc;
+
+	rc = regmap_update_bits(chg->regmap, chg->addr + SMBB_USB_OTG_CTL,
+				OTG_CTL_EN, OTG_CTL_EN);
+	if (rc)
+		dev_err(chg->dev, "failed to update OTG_CTL\n");
+	return rc;
+}
+
+static int smbb_chg_otg_disable(struct regulator_dev *rdev)
+{
+	struct smbb_charger *chg = rdev_get_drvdata(rdev);
+	int rc;
+
+	rc = regmap_update_bits(chg->regmap, chg->addr + SMBB_USB_OTG_CTL,
+				OTG_CTL_EN, 0);
+	if (rc)
+		dev_err(chg->dev, "failed to update OTG_CTL\n");
+	return rc;
+}
+
+static int smbb_chg_otg_is_enabled(struct regulator_dev *rdev)
+{
+	struct smbb_charger *chg = rdev_get_drvdata(rdev);
+	unsigned int value = 0;
+	int rc;
+
+	rc = regmap_read(chg->regmap, chg->addr + SMBB_USB_OTG_CTL, &value);
+	if (rc)
+		dev_err(chg->dev, "failed to read OTG_CTL\n");
+
+	return !!(value & OTG_CTL_EN);
+}
+
+static struct regulator_ops smbb_chg_otg_ops = {
+	.enable = smbb_chg_otg_enable,
+	.disable = smbb_chg_otg_disable,
+	.is_enabled = smbb_chg_otg_is_enabled,
+};
+
 static int smbb_charger_probe(struct platform_device *pdev)
 {
 	struct power_supply_config bat_cfg = {};
 	struct power_supply_config usb_cfg = {};
 	struct power_supply_config dc_cfg = {};
 	struct smbb_charger *chg;
+	struct regulator_config config = { };
 	int rc, i;
 
 	chg = devm_kzalloc(&pdev->dev, sizeof(*chg), GFP_KERNEL);
@@ -884,6 +936,28 @@ static int smbb_charger_probe(struct platform_device *pdev)
 		}
 	}
 
+	/*
+	 * otg regulator is used to control VBUS voltage direction
+	 * when USB switches between host and gadget mode
+	 */
+	chg->otg_rdesc.id = -1;
+	chg->otg_rdesc.name = "otg";
+	chg->otg_rdesc.ops = &smbb_chg_otg_ops;
+	chg->otg_rdesc.owner = THIS_MODULE;
+	chg->otg_rdesc.type = REGULATOR_VOLTAGE;
+	chg->otg_rdesc.supply_name = "usb-otg-in";
+	chg->otg_rdesc.fixed_uV = 5000000;
+	chg->otg_rdesc.n_voltages = 1;
+	chg->otg_rdesc.of_match = "otg";
+
+	config.dev = &pdev->dev;
+	config.driver_data = chg;
+
+	chg->otg_reg = devm_regulator_register(&pdev->dev, &chg->otg_rdesc,
+					       &config);
+	if (IS_ERR(chg->otg_reg))
+		return PTR_ERR(chg->otg_reg);
+
 	chg->jeita_ext_temp = of_property_read_bool(pdev->dev.of_node,
 			"qcom,jeita-extended-temp-range");
 
-- 
2.4.2

[PATCH v2 2/3] power: qcom_smbb: Add otg regulator for control of vbus

[Tim Bird]

Add a regulator to control the OTG chargepath switch.  This
is used by USB code to control VBUS direction - out for host mode
on the OTG port, and in for charging mode.

Signed-off-by: Tim Bird <tim.bird@sonymobile.com>
---
Changes since v1:
 - changed name of supply to remove underscores
---
 drivers/power/qcom_smbb.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/drivers/power/qcom_smbb.c b/drivers/power/qcom_smbb.c
index 5eb1e9e..2f1394c 100644
--- a/drivers/power/qcom_smbb.c
+++ b/drivers/power/qcom_smbb.c
@@ -34,6 +34,9 @@
 #include <linux/power_supply.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
 
 #define SMBB_CHG_VMAX		0x040
 #define SMBB_CHG_VSAFE		0x041
@@ -71,6 +74,8 @@
 #define BTC_CTRL_HOT_EXT_N	BIT(0)
 
 #define SMBB_USB_IMAX		0x344
+#define SMBB_USB_OTG_CTL	0x348
+#define OTG_CTL_EN		BIT(0)
 #define SMBB_USB_ENUM_TIMER_STOP 0x34e
 #define ENUM_TIMER_STOP		BIT(0)
 #define SMBB_USB_SEC_ACCESS	0x3d0
@@ -123,6 +128,9 @@ struct smbb_charger {
 	struct power_supply *dc_psy;
 	struct power_supply *bat_psy;
 	struct regmap *regmap;
+
+	struct regulator_desc otg_rdesc;
+	struct regulator_dev *otg_reg;
 };
 
 static int smbb_vbat_weak_fn(unsigned int index)
@@ -778,12 +786,56 @@ static const struct power_supply_desc dc_psy_desc = {
 	.property_is_writeable = smbb_charger_writable_property,
 };
 
+static int smbb_chg_otg_enable(struct regulator_dev *rdev)
+{
+	struct smbb_charger *chg = rdev_get_drvdata(rdev);
+	int rc;
+
+	rc = regmap_update_bits(chg->regmap, chg->addr + SMBB_USB_OTG_CTL,
+				OTG_CTL_EN, OTG_CTL_EN);
+	if (rc)
+		dev_err(chg->dev, "failed to update OTG_CTL\n");
+	return rc;
+}
+
+static int smbb_chg_otg_disable(struct regulator_dev *rdev)
+{
+	struct smbb_charger *chg = rdev_get_drvdata(rdev);
+	int rc;
+
+	rc = regmap_update_bits(chg->regmap, chg->addr + SMBB_USB_OTG_CTL,
+				OTG_CTL_EN, 0);
+	if (rc)
+		dev_err(chg->dev, "failed to update OTG_CTL\n");
+	return rc;
+}
+
+static int smbb_chg_otg_is_enabled(struct regulator_dev *rdev)
+{
+	struct smbb_charger *chg = rdev_get_drvdata(rdev);
+	unsigned int value = 0;
+	int rc;
+
+	rc = regmap_read(chg->regmap, chg->addr + SMBB_USB_OTG_CTL, &value);
+	if (rc)
+		dev_err(chg->dev, "failed to read OTG_CTL\n");
+
+	return !!(value & OTG_CTL_EN);
+}
+
+static struct regulator_ops smbb_chg_otg_ops = {
+	.enable = smbb_chg_otg_enable,
+	.disable = smbb_chg_otg_disable,
+	.is_enabled = smbb_chg_otg_is_enabled,
+};
+
 static int smbb_charger_probe(struct platform_device *pdev)
 {
 	struct power_supply_config bat_cfg = {};
 	struct power_supply_config usb_cfg = {};
 	struct power_supply_config dc_cfg = {};
 	struct smbb_charger *chg;
+	struct regulator_config config = { };
 	int rc, i;
 
 	chg = devm_kzalloc(&pdev->dev, sizeof(*chg), GFP_KERNEL);
@@ -884,6 +936,28 @@ static int smbb_charger_probe(struct platform_device *pdev)
 		}
 	}
 
+	/*
+	 * otg regulator is used to control VBUS voltage direction
+	 * when USB switches between host and gadget mode
+	 */
+	chg->otg_rdesc.id = -1;
+	chg->otg_rdesc.name = "otg";
+	chg->otg_rdesc.ops = &smbb_chg_otg_ops;
+	chg->otg_rdesc.owner = THIS_MODULE;
+	chg->otg_rdesc.type = REGULATOR_VOLTAGE;
+	chg->otg_rdesc.supply_name = "usb-otg-in";
+	chg->otg_rdesc.fixed_uV = 5000000;
+	chg->otg_rdesc.n_voltages = 1;
+	chg->otg_rdesc.of_match = "otg";
+
+	config.dev = &pdev->dev;
+	config.driver_data = chg;
+
+	chg->otg_reg = devm_regulator_register(&pdev->dev, &chg->otg_rdesc,
+					       &config);
+	if (IS_ERR(chg->otg_reg))
+		return PTR_ERR(chg->otg_reg);
+
 	chg->jeita_ext_temp = of_property_read_bool(pdev->dev.of_node,
 			"qcom,jeita-extended-temp-range");
 
-- 
2.4.2

[PATCH v2 3/3] ARM: dts: qcom: add charger otg regulator

[Tim Bird]

Add the otg regulator provided by the charger block.

Signed-off-by: Tim Bird <tim.bird@sonymobile.com>
---
 arch/arm/boot/dts/qcom-pm8941.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi
index b0d4439..1b80100 100644
--- a/arch/arm/boot/dts/qcom-pm8941.dtsi
+++ b/arch/arm/boot/dts/qcom-pm8941.dtsi
@@ -45,6 +45,10 @@
 					  "chg-gone",
 					  "usb-valid",
 					  "dc-valid";
+
+			usb-otg-in-supply = <&pm8941_5vs1>;
+
+			chg_otg: otg { };
 		};
 
 		pm8941_gpios: gpios@c000 {
-- 
2.4.2

[PATCH v2 3/3] ARM: dts: qcom: add charger otg regulator

[Tim Bird]

Add the otg regulator provided by the charger block.

Signed-off-by: Tim Bird <tim.bird@sonymobile.com>
---
 arch/arm/boot/dts/qcom-pm8941.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-pm8941.dtsi b/arch/arm/boot/dts/qcom-pm8941.dtsi
index b0d4439..1b80100 100644
--- a/arch/arm/boot/dts/qcom-pm8941.dtsi
+++ b/arch/arm/boot/dts/qcom-pm8941.dtsi
@@ -45,6 +45,10 @@
 					  "chg-gone",
 					  "usb-valid",
 					  "dc-valid";
+
+			usb-otg-in-supply = <&pm8941_5vs1>;
+
+			chg_otg: otg { };
 		};
 
 		pm8941_gpios: gpios@c000 {
-- 
2.4.2

Re: [PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[Rob Herring]

On Tue, Dec 08, 2015 at 04:40:16PM -0800, Tim Bird wrote:
> Add a binding for the regulator which controls the OTG chargepath switch.
> The OTG switch gets its power from pm8941_5vs1, and that should be
> expressed as a usb-otg-in-supply property in the DT node for the
> charger driver.  The regulator name is "otg".
> 
> Signed-off-by: Tim Bird <tim.bird@sonymobile.com>
> ---
> Changes since v1
>  - switch supply name to have dashes instead of underscores
>  - remove superfluous DT explanations in the otg node description
> ---
>  .../devicetree/bindings/power_supply/qcom_smbb.txt    | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt b/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
> index 65b88fa..28b6da7 100644
> --- a/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
> +++ b/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
> @@ -105,6 +105,22 @@ PROPERTIES
>                 regulation must be done externally to fully comply with
>                 the JEITA safety guidelines if this flag is set.
>  
> +- usb-otg-in-supply:
> +  Usage: optional
> +  Value type: <phandle>
> +  Description: Reference to the regulator supplying power to the USB_OTG_IN
> +               pin.
> +
> +child nodes:
> +- otg:
> +  Usage: optional
> +  Description: This node defines a regulator used to control the direction
> +               of VBUS voltage - specifically: whether to supply voltage
> +               to VBUS for host mode operation of the OTG port, or allow
> +               input voltage from external VBUS for charging.  In the
> +               hardware, the supply for this regulator comes from
> +               usb-otg-in-supply.

Doesn't this regulator need to have a name defined?

Disabling this regulator (along with other setup) will enable charging?

Rob

Re: [PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[Tim Bird]

On 12/08/2015 08:11 PM, Rob Herring wrote:
> On Tue, Dec 08, 2015 at 04:40:16PM -0800, Tim Bird wrote:
>> Add a binding for the regulator which controls the OTG chargepath switch.
>> The OTG switch gets its power from pm8941_5vs1, and that should be
>> expressed as a usb-otg-in-supply property in the DT node for the
>> charger driver.  The regulator name is "otg".
>>
>> Signed-off-by: Tim Bird <tim.bird-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
>> ---
>> Changes since v1
>>  - switch supply name to have dashes instead of underscores
>>  - remove superfluous DT explanations in the otg node description
>> ---
>>  .../devicetree/bindings/power_supply/qcom_smbb.txt    | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt b/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
>> index 65b88fa..28b6da7 100644
>> --- a/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
>> +++ b/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
>> @@ -105,6 +105,22 @@ PROPERTIES
>>                 regulation must be done externally to fully comply with
>>                 the JEITA safety guidelines if this flag is set.
>>  
>> +- usb-otg-in-supply:
>> +  Usage: optional
>> +  Value type: <phandle>
>> +  Description: Reference to the regulator supplying power to the USB_OTG_IN
>> +               pin.
>> +
>> +child nodes:
>> +- otg:
>> +  Usage: optional
>> +  Description: This node defines a regulator used to control the direction
>> +               of VBUS voltage - specifically: whether to supply voltage
>> +               to VBUS for host mode operation of the OTG port, or allow
>> +               input voltage from external VBUS for charging.  In the
>> +               hardware, the supply for this regulator comes from
>> +               usb-otg-in-supply.
> 
> Doesn't this regulator need to have a name defined?

I'm not sure what you mean.  The regulator name is "otg", defined by the DT node
name. The code requires that the DT node name be "otg", and defines a regulator
with the same name.

As far as I know, you have to define a DT label for the node, in order
to reference this regulator with a phandle.  Is that what you are referring to?
I usually use "chg_otg" as the label.  Are you asking that this be reflected
in the example?

> Disabling this regulator (along with other setup) will enable charging?

Yes.  Enabling it allows the device to power the USB VBUS line for host
mode, and disabling it allows power to flow the other way (into the device)
for charging, when the USB port is in gadget mode.
 -- Tim

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[Tim Bird]

On 12/08/2015 08:11 PM, Rob Herring wrote:
> On Tue, Dec 08, 2015 at 04:40:16PM -0800, Tim Bird wrote:
>> Add a binding for the regulator which controls the OTG chargepath switch.
>> The OTG switch gets its power from pm8941_5vs1, and that should be
>> expressed as a usb-otg-in-supply property in the DT node for the
>> charger driver.  The regulator name is "otg".
>>
>> Signed-off-by: Tim Bird <tim.bird@sonymobile.com>
>> ---
>> Changes since v1
>>  - switch supply name to have dashes instead of underscores
>>  - remove superfluous DT explanations in the otg node description
>> ---
>>  .../devicetree/bindings/power_supply/qcom_smbb.txt    | 19 +++++++++++++++++++
>>  1 file changed, 19 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt b/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
>> index 65b88fa..28b6da7 100644
>> --- a/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
>> +++ b/Documentation/devicetree/bindings/power_supply/qcom_smbb.txt
>> @@ -105,6 +105,22 @@ PROPERTIES
>>                 regulation must be done externally to fully comply with
>>                 the JEITA safety guidelines if this flag is set.
>>  
>> +- usb-otg-in-supply:
>> +  Usage: optional
>> +  Value type: <phandle>
>> +  Description: Reference to the regulator supplying power to the USB_OTG_IN
>> +               pin.
>> +
>> +child nodes:
>> +- otg:
>> +  Usage: optional
>> +  Description: This node defines a regulator used to control the direction
>> +               of VBUS voltage - specifically: whether to supply voltage
>> +               to VBUS for host mode operation of the OTG port, or allow
>> +               input voltage from external VBUS for charging.  In the
>> +               hardware, the supply for this regulator comes from
>> +               usb-otg-in-supply.
> 
> Doesn't this regulator need to have a name defined?

I'm not sure what you mean.  The regulator name is "otg", defined by the DT node
name. The code requires that the DT node name be "otg", and defines a regulator
with the same name.

As far as I know, you have to define a DT label for the node, in order
to reference this regulator with a phandle.  Is that what you are referring to?
I usually use "chg_otg" as the label.  Are you asking that this be reflected
in the example?

> Disabling this regulator (along with other setup) will enable charging?

Yes.  Enabling it allows the device to power the USB VBUS line for host
mode, and disabling it allows power to flow the other way (into the device)
for charging, when the USB port is in gadget mode.
 -- Tim

Re: [PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[Rob Herring]

On Wed, Dec 9, 2015 at 6:55 AM, Tim Bird <tim.bird@sonymobile.com> wrote:
> On 12/08/2015 08:11 PM, Rob Herring wrote:
>> On Tue, Dec 08, 2015 at 04:40:16PM -0800, Tim Bird wrote:
>>> Add a binding for the regulator which controls the OTG chargepath switch.
>>> The OTG switch gets its power from pm8941_5vs1, and that should be
>>> expressed as a usb-otg-in-supply property in the DT node for the
>>> charger driver.  The regulator name is "otg".

[...]

>>> +child nodes:
>>> +- otg:
>>> +  Usage: optional
>>> +  Description: This node defines a regulator used to control the direction
>>> +               of VBUS voltage - specifically: whether to supply voltage
>>> +               to VBUS for host mode operation of the OTG port, or allow
>>> +               input voltage from external VBUS for charging.  In the
>>> +               hardware, the supply for this regulator comes from
>>> +               usb-otg-in-supply.
>>
>> Doesn't this regulator need to have a name defined?
>
> I'm not sure what you mean.  The regulator name is "otg", defined by the DT node
> name. The code requires that the DT node name be "otg", and defines a regulator
> with the same name.
>
> As far as I know, you have to define a DT label for the node, in order
> to reference this regulator with a phandle.  Is that what you are referring to?
> I usually use "chg_otg" as the label.  Are you asking that this be reflected
> in the example?

You need a regulator-name property. Also, should should define valid
values for regulator-min-microvolt and regulator-max-microvolt.

Thinking about this some more, the node name should be generic, so
just "regulator". The label does not need to be generic.

Rob

Re: [PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[Tim Bird]

On 12/09/2015 06:36 AM, Rob Herring wrote:
> On Wed, Dec 9, 2015 at 6:55 AM, Tim Bird <tim.bird@sonymobile.com> wrote:
>> On 12/08/2015 08:11 PM, Rob Herring wrote:
>>> On Tue, Dec 08, 2015 at 04:40:16PM -0800, Tim Bird wrote:
>>>> Add a binding for the regulator which controls the OTG chargepath switch.
>>>> The OTG switch gets its power from pm8941_5vs1, and that should be
>>>> expressed as a usb-otg-in-supply property in the DT node for the
>>>> charger driver.  The regulator name is "otg".
> 
> [...]
> 
>>>> +child nodes:
>>>> +- otg:
>>>> +  Usage: optional
>>>> +  Description: This node defines a regulator used to control the direction
>>>> +               of VBUS voltage - specifically: whether to supply voltage
>>>> +               to VBUS for host mode operation of the OTG port, or allow
>>>> +               input voltage from external VBUS for charging.  In the
>>>> +               hardware, the supply for this regulator comes from
>>>> +               usb-otg-in-supply.
>>>
>>> Doesn't this regulator need to have a name defined?
>>
>> I'm not sure what you mean.  The regulator name is "otg", defined by the DT node
>> name. The code requires that the DT node name be "otg", and defines a regulator
>> with the same name.
>>
>> As far as I know, you have to define a DT label for the node, in order
>> to reference this regulator with a phandle.  Is that what you are referring to?
>> I usually use "chg_otg" as the label.  Are you asking that this be reflected
>> in the example?
> 
> You need a regulator-name property. Also, should should define valid
> values for regulator-min-microvolt and regulator-max-microvolt.

All of those are listed as optional properties in
Documentation/devicetree/bindings/regulator/regulator.txt.

The regulator-name seems redundant, since the name of the DT
node becomes the regulator name in the Linux system.  This is
not currently used anywhere, as the node is "connected up" via
phandle, which uses the label.  That is, there's no code in
the kernel that looks up this regulator by name, although it
does get a name.

As for the microvolt references, I'm a little stumped what
to do with those.  I could use regulator-min-microvolt of 0
and a regulator-max-microvolt of 500000, but that's not
exactly right.

This is actually representing a voltage switch, which is being
represented in the regulator framework in Linux, because it
seems to fit there best.  It has regulators it's related
to, which should be adjusted when this one is.  However, it
intrinsically does not have a voltage of it's own, so it's a
bit of a weird case.  And the voltage can either be going
"out" or "in".

I'm CC-ing Mark Brown, who may know how these types of things
are supposed to be expressed.  I believe he saw an earlier version
of this patch last year.  Or maybe our power maintainers will
chime in with some wisdom.  I can't be the first person to
be adding a charge pathway switch to mainline, so I'm open to
doing it the "right way". :-)

> Thinking about this some more, the node name should be generic, so
> just "regulator". The label does not need to be generic.

There are other switches in the charger block that are not
exposed yet.  This one handles the the OTG (vbus) charge pathway.
Others handle other charge pathways (some of which are used on phones
and some are not - they're used, e.g., on the dragonboard).  I'd
rather not give it a generic name, because eventually the driver
should expose those other switches as something as well.

 -- Tim

Re: [PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 2431 bytes --]

On Wed, Dec 09, 2015 at 09:42:09AM -0800, Tim Bird wrote:
> On 12/09/2015 06:36 AM, Rob Herring wrote:

> > You need a regulator-name property. Also, should should define valid
> > values for regulator-min-microvolt and regulator-max-microvolt.

> All of those are listed as optional properties in
> Documentation/devicetree/bindings/regulator/regulator.txt.

I have to say I'm also a bit confused about why a binding using a
regulator would need to do anything other than reference that document
except in cases where using some functionality enabled by the generic
bindings is a key part of the functionality of the device (which do tend
to be rare).

> The regulator-name seems redundant, since the name of the DT
> node becomes the regulator name in the Linux system.  This is

The purpose of the regulator-name property is to allow you to put in a
descriptive name for the supply which would normally correspond to the
name the supply is given on the schematic and may not fit into the DT
conventions.  It is entirely optional but strongly recommended to aid
people in understanding how both the DT and the running kernel relate to
the schematic.

> not currently used anywhere, as the node is "connected up" via
> phandle, which uses the label.  That is, there's no code in
> the kernel that looks up this regulator by name, although it
> does get a name.

The name is purely for use by humans in logs and/or userspace, it is not
used in the kernel.

> As for the microvolt references, I'm a little stumped what
> to do with those.  I could use regulator-min-microvolt of 0
> and a regulator-max-microvolt of 500000, but that's not
> exactly right.

You should only specify voltages if the intention is to either allow
consumers to vary voltages at runtime or have the kernel set a specific
voltage for the regulator (eg, if the hardware defaults are wrong).

> This is actually representing a voltage switch, which is being
> represented in the regulator framework in Linux, because it
> seems to fit there best.  It has regulators it's related
> to, which should be adjusted when this one is.  However, it
> intrinsically does not have a voltage of it's own, so it's a
> bit of a weird case.  And the voltage can either be going
> "out" or "in".

If you want consumers of the supply to be able to vary the voltage you
need to permit that on the child supply as well as the parent supply so
we know we can pass requests up.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[Bjorn Andersson]

On Wed 09 Dec 06:36 PST 2015, Rob Herring wrote:

> On Wed, Dec 9, 2015 at 6:55 AM, Tim Bird <tim.bird@sonymobile.com> wrote:
> > On 12/08/2015 08:11 PM, Rob Herring wrote:
> >> On Tue, Dec 08, 2015 at 04:40:16PM -0800, Tim Bird wrote:
> >>> Add a binding for the regulator which controls the OTG chargepath switch.
> >>> The OTG switch gets its power from pm8941_5vs1, and that should be
> >>> expressed as a usb-otg-in-supply property in the DT node for the
> >>> charger driver.  The regulator name is "otg".
> 
> [...]
> 
> >>> +child nodes:
> >>> +- otg:
> >>> +  Usage: optional
> >>> +  Description: This node defines a regulator used to control the direction
> >>> +               of VBUS voltage - specifically: whether to supply voltage
> >>> +               to VBUS for host mode operation of the OTG port, or allow
> >>> +               input voltage from external VBUS for charging.  In the
> >>> +               hardware, the supply for this regulator comes from
> >>> +               usb-otg-in-supply.
> >>
> >> Doesn't this regulator need to have a name defined?
> >
> > I'm not sure what you mean.  The regulator name is "otg", defined by the DT node
> > name. The code requires that the DT node name be "otg", and defines a regulator
> > with the same name.
> >
> > As far as I know, you have to define a DT label for the node, in order
> > to reference this regulator with a phandle.  Is that what you are referring to?
> > I usually use "chg_otg" as the label.  Are you asking that this be reflected
> > in the example?
> 
> You need a regulator-name property. Also, should should define valid
> values for regulator-min-microvolt and regulator-max-microvolt.
> 

The regulator has a name, derived from the node name, and this is
significant. If the developer wants an additional human readable name
for some reason they can use the optional regulator-name property.

The regulator is a simple switch and as such inherits voltage properties
from its supply. It should therefor not have any specified voltage
range.

> Thinking about this some more, the node name should be generic, so
> just "regulator". The label does not need to be generic.
> 

The name of the node is significant, as it's used for matching the
regulator to an implementation.

Regards,
Bjorn

Re: [PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[Rob Herring]

On Wed, Dec 9, 2015 at 1:59 PM, Bjorn Andersson
<bjorn.andersson@sonymobile.com> wrote:
> On Wed 09 Dec 06:36 PST 2015, Rob Herring wrote:
>
>> On Wed, Dec 9, 2015 at 6:55 AM, Tim Bird <tim.bird@sonymobile.com> wrote:
>> > On 12/08/2015 08:11 PM, Rob Herring wrote:
>> >> On Tue, Dec 08, 2015 at 04:40:16PM -0800, Tim Bird wrote:
>> >>> Add a binding for the regulator which controls the OTG chargepath switch.
>> >>> The OTG switch gets its power from pm8941_5vs1, and that should be
>> >>> expressed as a usb-otg-in-supply property in the DT node for the
>> >>> charger driver.  The regulator name is "otg".
>>
>> [...]
>>
>> >>> +child nodes:
>> >>> +- otg:
>> >>> +  Usage: optional
>> >>> +  Description: This node defines a regulator used to control the direction
>> >>> +               of VBUS voltage - specifically: whether to supply voltage
>> >>> +               to VBUS for host mode operation of the OTG port, or allow
>> >>> +               input voltage from external VBUS for charging.  In the
>> >>> +               hardware, the supply for this regulator comes from
>> >>> +               usb-otg-in-supply.
>> >>
>> >> Doesn't this regulator need to have a name defined?
>> >
>> > I'm not sure what you mean.  The regulator name is "otg", defined by the DT node
>> > name. The code requires that the DT node name be "otg", and defines a regulator
>> > with the same name.
>> >
>> > As far as I know, you have to define a DT label for the node, in order
>> > to reference this regulator with a phandle.  Is that what you are referring to?
>> > I usually use "chg_otg" as the label.  Are you asking that this be reflected
>> > in the example?
>>
>> You need a regulator-name property. Also, should should define valid
>> values for regulator-min-microvolt and regulator-max-microvolt.
>>
>
> The regulator has a name, derived from the node name, and this is
> significant. If the developer wants an additional human readable name
> for some reason they can use the optional regulator-name property.
>
> The regulator is a simple switch and as such inherits voltage properties
> from its supply. It should therefor not have any specified voltage
> range.
>
>> Thinking about this some more, the node name should be generic, so
>> just "regulator". The label does not need to be generic.
>>
>
> The name of the node is significant, as it's used for matching the
> regulator to an implementation.

Ah yes, you are right. I forget what an oddball the regulator binding is.

And if voltage switches don't need min and max properties, then it is
fine as is. Still, the empty node with no properties seems odd to me.

Rob

Re: [PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

Hi,

On Wed, Dec 09, 2015 at 09:42:09AM -0800, Tim Bird wrote:
> Or maybe our power maintainers will chime in with some wisdom. I
> can't be the first person to be adding a charge pathway switch to
> mainline, so I'm open to doing it the "right way". :-)

I don't think there is a standard way for this so far. Another
otg charger coming to my mindis bq2415x, which basically just
exposes the enable bit via sysfs.

Exposing the switch as regulator would be fine for me.

> > Thinking about this some more, the node name should be generic, so
> > just "regulator". The label does not need to be generic.
> 
> There are other switches in the charger block that are not
> exposed yet.  This one handles the the OTG (vbus) charge pathway.
> Others handle other charge pathways (some of which are used on phones
> and some are not - they're used, e.g., on the dragonboard).  I'd
> rather not give it a generic name, because eventually the driver
> should expose those other switches as something as well.

otg_regulator: regulator@0 { }
other_regulator: regulator@1 { }

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 828 bytes --]

On Wed, Dec 09, 2015 at 11:20:45PM +0100, Sebastian Reichel wrote:
> On Wed, Dec 09, 2015 at 09:42:09AM -0800, Tim Bird wrote:

> > There are other switches in the charger block that are not
> > exposed yet.  This one handles the the OTG (vbus) charge pathway.
> > Others handle other charge pathways (some of which are used on phones
> > and some are not - they're used, e.g., on the dragonboard).  I'd
> > rather not give it a generic name, because eventually the driver
> > should expose those other switches as something as well.

> otg_regulator: regulator@0 { }
> other_regulator: regulator@1 { }

No, if this is a device with multiple regulators we have a standard way
of exposing that - have a regulators subnode with a collection of named
regulator subnodes in that.  There's support for parsing this in the
framework.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v2 1/3] dt-binding: power: Add otg regulator binding

[Rob Herring]

On Wed, Dec 9, 2015 at 3:08 PM, Rob Herring <robh@kernel.org> wrote:
> On Wed, Dec 9, 2015 at 1:59 PM, Bjorn Andersson
> <bjorn.andersson@sonymobile.com> wrote:
>> On Wed 09 Dec 06:36 PST 2015, Rob Herring wrote:
>>
>>> On Wed, Dec 9, 2015 at 6:55 AM, Tim Bird <tim.bird@sonymobile.com> wrote:
>>> > On 12/08/2015 08:11 PM, Rob Herring wrote:
>>> >> On Tue, Dec 08, 2015 at 04:40:16PM -0800, Tim Bird wrote:
>>> >>> Add a binding for the regulator which controls the OTG chargepath switch.
>>> >>> The OTG switch gets its power from pm8941_5vs1, and that should be
>>> >>> expressed as a usb-otg-in-supply property in the DT node for the
>>> >>> charger driver.  The regulator name is "otg".
>>>
>>> [...]
>>>
>>> >>> +child nodes:
>>> >>> +- otg:
>>> >>> +  Usage: optional
>>> >>> +  Description: This node defines a regulator used to control the direction
>>> >>> +               of VBUS voltage - specifically: whether to supply voltage
>>> >>> +               to VBUS for host mode operation of the OTG port, or allow
>>> >>> +               input voltage from external VBUS for charging.  In the
>>> >>> +               hardware, the supply for this regulator comes from
>>> >>> +               usb-otg-in-supply.
>>> >>
>>> >> Doesn't this regulator need to have a name defined?
>>> >
>>> > I'm not sure what you mean.  The regulator name is "otg", defined by the DT node
>>> > name. The code requires that the DT node name be "otg", and defines a regulator
>>> > with the same name.
>>> >
>>> > As far as I know, you have to define a DT label for the node, in order
>>> > to reference this regulator with a phandle.  Is that what you are referring to?
>>> > I usually use "chg_otg" as the label.  Are you asking that this be reflected
>>> > in the example?
>>>
>>> You need a regulator-name property. Also, should should define valid
>>> values for regulator-min-microvolt and regulator-max-microvolt.
>>>
>>
>> The regulator has a name, derived from the node name, and this is
>> significant. If the developer wants an additional human readable name
>> for some reason they can use the optional regulator-name property.
>>
>> The regulator is a simple switch and as such inherits voltage properties
>> from its supply. It should therefor not have any specified voltage
>> range.
>>
>>> Thinking about this some more, the node name should be generic, so
>>> just "regulator". The label does not need to be generic.
>>>
>>
>> The name of the node is significant, as it's used for matching the
>> regulator to an implementation.
>
> Ah yes, you are right. I forget what an oddball the regulator binding is.
>
> And if voltage switches don't need min and max properties, then it is
> fine as is. Still, the empty node with no properties seems odd to me.

Thinking some more about this, what bothers me is just having "otg
{};" gives no clue as to what the node is for w/o the documentation.
"otg" alone could mean several things. I think minimally, it needs
either a node name that gives some indication it is a regulator or
Vbus supply, a required regulator-name or even a comment.

Rob