* [linux-sunxi] Re: PATH[1/3] ARM: axp20x_usb_power.c add device tree configuration options for REG 30H: VBUS-IPSOUT
[not found] ` <671c7471-b63a-40cc-989f-6b20f00571ee@googlegroups.com>
@ 2016-05-17 23:15 ` Julian Calaby
[not found] ` <35a39bdc-1304-403d-b56a-a45bb8debf5e@googlegroups.com>
1 sibling, 0 replies; 4+ messages in thread
From: Julian Calaby @ 2016-05-17 23:15 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ene,
On Wed, May 18, 2016 at 3:47 AM, Ene Alexandru <ene.alexandru@gmail.com> wrote:
> axp20x_usb_power.c is modified to read those parameters from the device tree
> configuration.
> if a configuration value is not found then the corresponding register value
> is not changed.
>
> also, debug messages are added, controlled by "CONFIG_POWER_SUPPLY_DEBUG" :
>
> Signed-off-by: Ene Alexandru <ene.alexandru@gmail.com>
>
>
> ---
> diff -uprN -X linux-sunxi-original/Documentation/dontdiff
> linux-sunxi-original/drivers/power/axp20x_usb_power.c
> linux-sunxi/drivers/power/axp20x_usb_power.c
> --- linux-sunxi-original/drivers/power/axp20x_usb_power.c 2016-05-09
> 16:51:44.000000000 +0200
> +++ linux-sunxi/drivers/power/axp20x_usb_power.c 2016-05-11
> 13:26:24.444681579 +0200
> @@ -41,6 +41,19 @@
> #define AXP20X_VBUS_MON_VBUS_VALID BIT(3)
> +/* bit defines for REG 30H: VBUS-IPSOUT Power Path Management */
> +/* VBUS VHOLD voltage limiting control */
> +#define AXP20X_VBUS_IPSOUT_MGMT_VHOLD BIT(6)
> +#define AXP20X_VBUS_IPSOUT_MGMT_VHOLD_ENA BIT(6)
> +#define AXP20X_VBUS_IPSOUT_MGMT_VHOLD_DIS BIT(0)
> +/* VHOLD Set voltage */
> +#define AXP20X_VBUS_IPSOUT_MGMT_VHOLD_SET_MASK (BIT(5)|BIT(4)|BIT(3))
> +#define AXP20X_VBUS_IPSOUT_MGMT_VHOLD_SET_SHIFT (3)
> +/* VBUS current-limit selection */
> +#define AXP20X_VBUS_IPSOUT_MGMT_IBUS_MASK (BIT(1) | BIT(0))
> +
> +
> +
Drop the two extra empty lines here.
> struct axp20x_usb_power {
> struct regmap *regmap;
> struct power_supply *supply;
> @@ -164,6 +177,93 @@ static const struct power_supply_desc ax
> .get_property = axp20x_usb_power_get_property,
> };
> +
> +static int axp20x_usb_power_read_params(const struct device_node *node,
> + struct axp20x_usb_power *power, struct
> platform_device *pdev)
> +{
> + const u32 *prop;
> + int ret;
> +
> + /*
> + * configurable parameters are:
> + * register VBUS-IPSOUT
> + * bit 6: VBUS VHOLD voltage limiting control
> + * 0: No voltage drop limit
> + * 1: Limit the voltage drop
> + * bit 5-3 VHOLD Set VHOLD = [4.0+ (Bit5-3) * 0.1] V
> + * bit 1-0 VBUS current-limit selection
> + * 00:900mA
> + * 01:500mA
> + * 10:100mA
> + * 11:no limit
> + */
> +
> + prop = of_get_property(node, "vhold-enable", NULL);
> + if (prop) {
> + /* either 1 or 0 */
> +#ifdef DEBUG
> + dev_info(&pdev->dev, "set vhold-enable property to
> %d",
> + !!(*prop));
> +#endif
Use dev_dbg() instead of wrapping the dev_info() calls in #ifdefs.
> + if (!!(*prop)) {
> + ret = regmap_update_bits(power->regmap,
> +
> AXP20X_VBUS_IPSOUT_MGMT,
> +
> AXP20X_VBUS_IPSOUT_MGMT_VHOLD,
> +
> AXP20X_VBUS_IPSOUT_MGMT_VHOLD_ENA);
> + } else {
> + ret = regmap_update_bits(power->regmap,
> +
> AXP20X_VBUS_IPSOUT_MGMT,
> +
> AXP20X_VBUS_IPSOUT_MGMT_VHOLD,
> +
> AXP20X_VBUS_IPSOUT_MGMT_VHOLD_DIS);
> + }
> + if (ret)
> + return ret;
> + } else {
> +#ifdef DEBUG
> + dev_info(&pdev->dev, "no vhold-enable property
> found");
> +#endif
Ditto.
> + }
> +
> + prop = of_get_property(node, "vhold-set", NULL);
> + if (prop) {
> + /* from 0b000 to 0b111 */
> +#ifdef DEBUG
> + dev_info(&pdev->dev, "set vhold-set property to
> %02X",
> + ((*prop)>>24));
> +#endif
Ditto.
> + ret = regmap_update_bits(power->regmap,
> + AXP20X_VBUS_IPSOUT_MGMT,
> +
> AXP20X_VBUS_IPSOUT_MGMT_VHOLD_SET_MASK,
> + ((*prop)>>24) <<
> AXP20X_VBUS_IPSOUT_MGMT_VHOLD_SET_SHIFT);
> + if (ret)
> + return ret;
> + } else {
> +#ifdef DEBUG
> + dev_info(&pdev->dev, "no vhold-set property found");
> +#endif
Ditto.
> + }
> +
> + prop = of_get_property(node, "ibus-limit", NULL);
> + if (prop) {
> + /* from 0b0 to 0b11 */
> +#ifdef DEBUG
> + dev_info(&pdev->dev, "set ibus-limit property to
> %02X",
> + ((*prop)>>24));
> +#endif
Ditto.
> + ret = regmap_update_bits(power->regmap,
> AXP20X_VBUS_IPSOUT_MGMT,
> +
> AXP20X_VBUS_IPSOUT_MGMT_IBUS_MASK,
> + ((*prop)>>24));
> + if (ret)
> + return ret;
> + } else {
> +#ifdef DEBUG
> + dev_info(&pdev->dev, "no ibus-limit property found");
> +#endif
Ditto.
> + }
> +
> + return 0;
> +}
> +
> static int axp20x_usb_power_probe(struct platform_device *pdev)
> {
> struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
> @@ -172,6 +272,7 @@ static int axp20x_usb_power_probe(struct
> static const char * const irq_names[] = { "VBUS_PLUGIN",
> "VBUS_REMOVAL", "VBUS_VALID", "VBUS_NOT_VALID" };
> int i, irq, ret;
> + struct device_node *node;
> if (!of_device_is_available(pdev->dev.of_node))
> return -ENODEV;
> @@ -208,6 +309,11 @@ static int axp20x_usb_power_probe(struct
> if (IS_ERR(power->supply))
> return PTR_ERR(power->supply);
> +
> + /* read DT configurations parameters, if available */
> + for_each_compatible_node(node, NULL,
> "x-powers,axp202-usb-power-supply")
> + axp20x_usb_power_read_params(node, power,
> pdev);
> +
> /* Request irqs after registering, as irqs may trigger
> immediately */
> for (i = 0; i < ARRAY_SIZE(irq_names); i++) {
> irq = platform_get_irq_byname(pdev, irq_names[i]);
> ---
Thanks,
--
Julian Calaby
Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
^ permalink raw reply [flat|nested] 4+ messages in thread
* [linux-sunxi] Re: PATH[1/3] ARM: axp20x_usb_power.c add device tree configuration options for REG 30H: VBUS-IPSOUT
[not found] ` <35a39bdc-1304-403d-b56a-a45bb8debf5e@googlegroups.com>
@ 2016-05-17 23:21 ` Julian Calaby
0 siblings, 0 replies; 4+ messages in thread
From: Julian Calaby @ 2016-05-17 23:21 UTC (permalink / raw)
To: linux-arm-kernel
Hi Ene,
On Wed, May 18, 2016 at 3:50 AM, Ene Alexandru <ene.alexandru@gmail.com> wrote:
> the documentation is updated to describe how to convert actual values to the
> provided parameters
>
> ---
> diff -uprN -X linux-sunxi-original/Documentation/dontdiff
> linux-sunxi-original/Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
> linux-sunxi/Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
> ---
> linux-sunxi-original/Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
> 2016-05-09 16:48:46.000000000 +0200
> +++
> linux-sunxi/Documentation/devicetree/bindings/power_supply/axp20x_usb_power.txt
> 2016-05-11 13:05:58.995873267 +0200
> @@ -5,6 +5,26 @@ Required Properties:
> This node is a subnode of the axp20x PMIC.
> +The following 3 parameters are configurable from the device-tree:
> + - vhold-enable
> + - bit 6 of register REG 30H: VBUS-IPSOUT Power Path
> Management
I don't think it's necessary to go into this much detail here.
> + - available values are:
> + - 0x00 : VBUS VHOLD voltage limiting
> control disabled
> + - 0x01 : VBUS VHOLD voltage limiting
> control enabled
If these are the only options available, why not make this property a boolean?
> + - vhold-set
> + - bits <5-3> of register REG 30H: VBUS-IPSOUT Power
> Path Management
> + - available values are from 0x00 to 0x07
> + - VHOLD = [4000000 + ( vhold-set & 0x07 ) * 100000]
> in uV
What does this actually do? Is this the lower limit on the voltage? Is
this the voltage it'll try to maintain?
> + - ibus-limit
> + - bits <1-0> of register REG 30H: VBUS-IPSOUT Power
> Path Management
> + - available values are :
> + - 0x00 : 900000uA
> + - 0x01 : 500000uA
> + - 0x02 : 100000uA
> + - 0x03 : unlimited
Again, what does this actually do? It looks like the current limit?
> +
> +
Extra blank line.
> Example:
> axp209: pmic at 34 {
> @@ -30,5 +50,8 @@ axp209: pmic at 34 {
> usb-power-supply: usb-power-supply {
> compatible = "x-powers,axp202-usb-power-supply";
> + vhold-enable = <0x01>;
> + vhold-set = <0x04>;
> + ibus-limit = <0x01>;
> };
> };
Thanks,
--
Julian Calaby
Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
^ permalink raw reply [flat|nested] 4+ messages in thread
* PATH[1/3] ARM: axp20x_usb_power.c add device tree configuration options for REG 30H: VBUS-IPSOUT
[not found] <0f58d2b3-f3d3-45a3-a307-e54df26c1ab1@googlegroups.com>
[not found] ` <671c7471-b63a-40cc-989f-6b20f00571ee@googlegroups.com>
@ 2016-05-18 7:21 ` Lee Jones
2016-06-10 9:56 ` [linux-sunxi] " Hans de Goede
2 siblings, 0 replies; 4+ messages in thread
From: Lee Jones @ 2016-05-18 7:21 UTC (permalink / raw)
To: linux-arm-kernel
What on earth are all these emails you're sending to the list?
Please read the following documentation to ensure you're using correct
processes to send patches. If you require any help with a particular
aspect, please feel free to ask.
Documentation/HOWTO
Documentation/CodingSytle
Documentation/email-clients.txt
Documentation/SubmittingPatches
The last one being the most prevalent in this case.
NB: If you use Git to create your patches `git format-patch` and to
email your patches out `git send-email`, then you'll be most of the
way there.
> The AXP20X chips have a configuration register "REG 30H: VBUS-IPSOUT Power
> Path Management" that provide USB power supply protection.
> The current axp20x_usb_power.c provides support only for reading those
> values.
>
> From the datasheet:
> In order not to affect the USB communication, VBUS is always working under
> Voltage-Limit mode by default. In this mode, AXP209 ensures that VBUS
> voltage remains above a configurable reference voltage VHOLD which can meet
> the USB specification. The default VHOLD is 4.4V, adjustable in Reg30H
> [5:3] register. If the system has limit on current obtained from USB VBUS,
> a current-limit mode is provided (See REG30H[1] register), with
> 900mA/500mA/100mA (Reg30H [0]) selectable.
>
> The following entries are added in the axp209.dtsi
> file: vhold-enable, vhold-set and ibus-limit
> The chosen default values are the AXP20X reset values, as described in the
> data sheet.
>
> Signed-off-by: Ene Alexandru <ene.alexandru@gmail.com>
>
>
> ---
> diff -uprN -X linux-sunxi-original/Documentation/dontdiff
> linux-sunxi-original/arch/arm/boot/dts/axp209.dtsi
> linux-sunxi/arch/arm/boot/dts/axp209.dtsi
> --- linux-sunxi-original/arch/arm/boot/dts/axp209.dtsi 2016-05-11
> 09:21:47.470923679 +0200
> +++ linux-sunxi/arch/arm/boot/dts/axp209.dtsi 2016-05-10
> 18:45:32.795158651 +0200
> @@ -98,5 +98,8 @@
> usb_power_supply: usb_power_supply {
> compatible = "x-powers,axp202-usb-power-supply";
> status = "disabled";
> + vhold-enable = <0x01>;
> + vhold-set = <0x04>;
> + ibus-limit = <0x01>;
> };
> };
> ---
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 4+ messages in thread
* [linux-sunxi] PATH[1/3] ARM: axp20x_usb_power.c add device tree configuration options for REG 30H: VBUS-IPSOUT
[not found] <0f58d2b3-f3d3-45a3-a307-e54df26c1ab1@googlegroups.com>
[not found] ` <671c7471-b63a-40cc-989f-6b20f00571ee@googlegroups.com>
2016-05-18 7:21 ` Lee Jones
@ 2016-06-10 9:56 ` Hans de Goede
2 siblings, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2016-06-10 9:56 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On 17-05-16 19:41, Ene Alexandru wrote:
> The AXP20X chips have a configuration register "REG 30H: VBUS-IPSOUT Power Path Management" that provide USB power supply protection.
> The current axp20x_usb_power.c provides support only for reading those values.
>
> From the datasheet:
> In order not to affect the USB communication, VBUS is always working under Voltage-Limit mode by default. In this mode, AXP209 ensures that VBUS voltage remains above a configurable reference voltage VHOLD which can meet the USB specification. The default VHOLD is 4.4V, adjustable in Reg30H [5:3] register. If the system has limit on current obtained from USB VBUS, a current-limit mode is provided (See REG30H[1] register), with 900mA/500mA/100mA (Reg30H [0]) selectable.
>
> The following entries are added in the axp209.dtsi file: vhold-enable, vhold-set and ibus-limit
> The chosen default values are the AXP20X reset values, as described in the data sheet.
>
> Signed-off-by: Ene Alexandru <ene.alexandru@gmail.com>
>
>
> ---
> diff -uprN -X linux-sunxi-original/Documentation/dontdiff linux-sunxi-original/arch/arm/boot/dts/axp209.dtsi linux-sunxi/arch/arm/boot/dts/axp209.dtsi
> --- linux-sunxi-original/arch/arm/boot/dts/axp209.dtsi 2016-05-11 09:21:47.470923679 +0200
> +++ linux-sunxi/arch/arm/boot/dts/axp209.dtsi 2016-05-10 18:45:32.795158651 +0200
> @@ -98,5 +98,8 @@
> usb_power_supply: usb_power_supply {
> compatible = "x-powers,axp202-usb-power-supply";
> status = "disabled";
> + vhold-enable = <0x01>;
> + vhold-set = <0x04>;
> + ibus-limit = <0x01>;
> };
> };
Please do not use raw register values like this, instead make the device-tree property take
microvolts / microamps like e.g. the regulator bindings do, and translate these in the
driver.
Regards,
Hans
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-06-10 9:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <0f58d2b3-f3d3-45a3-a307-e54df26c1ab1@googlegroups.com>
[not found] ` <671c7471-b63a-40cc-989f-6b20f00571ee@googlegroups.com>
2016-05-17 23:15 ` [linux-sunxi] Re: PATH[1/3] ARM: axp20x_usb_power.c add device tree configuration options for REG 30H: VBUS-IPSOUT Julian Calaby
[not found] ` <35a39bdc-1304-403d-b56a-a45bb8debf5e@googlegroups.com>
2016-05-17 23:21 ` Julian Calaby
2016-05-18 7:21 ` Lee Jones
2016-06-10 9:56 ` [linux-sunxi] " Hans de Goede
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).