diff mbox

[6/8] ARM: dts: axp209: Add usb_power_supply child node to the ax209 node

Message ID 1433885881-19809-7-git-send-email-hdegoede@redhat.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Hans de Goede June 9, 2015, 9:37 p.m. UTC
Add a node representing the usb power supply part of the axp209 pmic, note
that the usb power supply and the (to be added later) ac power supply will
each have their own child-node, so that they can be separately specified
as power-supply for other nodes using a power-supply property with a
phandle pointing to the right axp209 child-node.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 arch/arm/boot/dts/axp209.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Chen-Yu Tsai July 31, 2015, 5:31 a.m. UTC | #1
Hi Hans,

On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Add a node representing the usb power supply part of the axp209 pmic, note
> that the usb power supply and the (to be added later) ac power supply will
> each have their own child-node, so that they can be separately specified
> as power-supply for other nodes using a power-supply property with a
> phandle pointing to the right axp209 child-node.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  arch/arm/boot/dts/axp209.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
> index 24c935c..051ab3b 100644
> --- a/arch/arm/boot/dts/axp209.dtsi
> +++ b/arch/arm/boot/dts/axp209.dtsi
> @@ -89,4 +89,9 @@
>                         regulator-name = "ldo5";
>                 };
>         };
> +
> +       usb_power_supply: usb_power_supply {
> +               compatible = "x-powers,axp202-usb-power-supply";
> +               status = "disabled";

Is there any reason to have this disabled by default?
AFAIK this is not something configurable in hardware,
and the driver just gives readouts and status updates.

ChenYu

> +       };
>  };
> --
> 2.3.6
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bruno Prémont July 31, 2015, 5:51 a.m. UTC | #2
Hi ChenYu,

On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
> Hi Hans,
> 
> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> > Add a node representing the usb power supply part of the axp209 pmic, note
> > that the usb power supply and the (to be added later) ac power supply will
> > each have their own child-node, so that they can be separately specified
> > as power-supply for other nodes using a power-supply property with a
> > phandle pointing to the right axp209 child-node.
> >
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > ---
> >  arch/arm/boot/dts/axp209.dtsi | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
> > index 24c935c..051ab3b 100644
> > --- a/arch/arm/boot/dts/axp209.dtsi
> > +++ b/arch/arm/boot/dts/axp209.dtsi
> > @@ -89,4 +89,9 @@
> >                         regulator-name = "ldo5";
> >                 };
> >         };
> > +
> > +       usb_power_supply: usb_power_supply {
> > +               compatible = "x-powers,axp202-usb-power-supply";
> > +               status = "disabled";
> 
> Is there any reason to have this disabled by default?
> AFAIK this is not something configurable in hardware,
> and the driver just gives readouts and status updates.

There are some devices that only have 'AC-IN' instead of usb/otg power
(set-top boxes) so having the devices tell what power sources they
have makes sense (otherwise those that have ac-in instead of otg would
have to disable usb_power_supply) and avoids confusion by explicitly
stating presence in device dts.

NB: I should hopefully get my battery supply driver ported this WE,
ac-in/rtc are done, this WE. I'm going to default-off for them as well
(note that for some extra details should be provided in dts).

Bruno

> ChenYu
> 
> > +       };
> >  };
> > --
> > 2.3.6
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chen-Yu Tsai July 31, 2015, 6:14 a.m. UTC | #3
On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont
<bonbons@linux-vserver.org> wrote:
> Hi ChenYu,
>
> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>> Hi Hans,
>>
>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> > Add a node representing the usb power supply part of the axp209 pmic, note
>> > that the usb power supply and the (to be added later) ac power supply will
>> > each have their own child-node, so that they can be separately specified
>> > as power-supply for other nodes using a power-supply property with a
>> > phandle pointing to the right axp209 child-node.
>> >
>> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> > ---
>> >  arch/arm/boot/dts/axp209.dtsi | 5 +++++
>> >  1 file changed, 5 insertions(+)
>> >
>> > diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
>> > index 24c935c..051ab3b 100644
>> > --- a/arch/arm/boot/dts/axp209.dtsi
>> > +++ b/arch/arm/boot/dts/axp209.dtsi
>> > @@ -89,4 +89,9 @@
>> >                         regulator-name = "ldo5";
>> >                 };
>> >         };
>> > +
>> > +       usb_power_supply: usb_power_supply {
>> > +               compatible = "x-powers,axp202-usb-power-supply";
>> > +               status = "disabled";
>>
>> Is there any reason to have this disabled by default?
>> AFAIK this is not something configurable in hardware,
>> and the driver just gives readouts and status updates.
>
> There are some devices that only have 'AC-IN' instead of usb/otg power
> (set-top boxes) so having the devices tell what power sources they
> have makes sense (otherwise those that have ac-in instead of otg would
> have to disable usb_power_supply) and avoids confusion by explicitly
> stating presence in device dts.

That's true. But this is a bit of hardware inside the AXP.
If it's power isn't routed, it should report it, shouldn't it?

And normal users would probably expect to read the input
voltage/current/status through sysfs (or maybe hwmon?),
instead of not finding the device and having to look through
the DTS.

If you're using a distro, you might not have the kernel sources
around, and digging through the in-system device tree isn't that
straightforward if you don't know where to look.

> NB: I should hopefully get my battery supply driver ported this WE,
> ac-in/rtc are done, this WE. I'm going to default-off for them as well
> (note that for some extra details should be provided in dts).

I'm guessing we won't be using the axp288 one?

Thanks
ChenYu

> Bruno
>
>> ChenYu
>>
>> > +       };
>> >  };
>> > --
>> > 2.3.6
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bruno Prémont July 31, 2015, 6:35 a.m. UTC | #4
On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont wrote:
> > Hi ChenYu,
> >
> > On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
> >> Hi Hans,
> >>
> >> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
> >> > Add a node representing the usb power supply part of the axp209 pmic, note
> >> > that the usb power supply and the (to be added later) ac power supply will
> >> > each have their own child-node, so that they can be separately specified
> >> > as power-supply for other nodes using a power-supply property with a
> >> > phandle pointing to the right axp209 child-node.
> >> >
> >> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >> > ---
> >> >  arch/arm/boot/dts/axp209.dtsi | 5 +++++
> >> >  1 file changed, 5 insertions(+)
> >> >
> >> > diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
> >> > index 24c935c..051ab3b 100644
> >> > --- a/arch/arm/boot/dts/axp209.dtsi
> >> > +++ b/arch/arm/boot/dts/axp209.dtsi
> >> > @@ -89,4 +89,9 @@
> >> >                         regulator-name = "ldo5";
> >> >                 };
> >> >         };
> >> > +
> >> > +       usb_power_supply: usb_power_supply {
> >> > +               compatible = "x-powers,axp202-usb-power-supply";
> >> > +               status = "disabled";
> >>
> >> Is there any reason to have this disabled by default?
> >> AFAIK this is not something configurable in hardware,
> >> and the driver just gives readouts and status updates.
> >
> > There are some devices that only have 'AC-IN' instead of usb/otg power
> > (set-top boxes) so having the devices tell what power sources they
> > have makes sense (otherwise those that have ac-in instead of otg would
> > have to disable usb_power_supply) and avoids confusion by explicitly
> > stating presence in device dts.
> 
> That's true. But this is a bit of hardware inside the AXP.
> If it's power isn't routed, it should report it, shouldn't it?

If the device does not use the input there are two cases:
- ac-in and usb/otg in short-circuited (there is a bit in AXP registers
  to provide this indication)
- the unused one is just not connected/connectable

In either case the voltage of the unused/unusable branch is not really
useful, nor is it useful to have it show up in sysfs.

> And normal users would probably expect to read the input
> voltage/current/status through sysfs (or maybe hwmon?),
> instead of not finding the device and having to look through
> the DTS.

Only those supplies effectively present (in-use or with connectors
to eventually use them) should be visible in sysfs.
Having more of them will just confuse the users, they would be searching
how they can connect the supply while there is no means to do so
except soldering to the tiny AXP pins.

hwmon makes more sense for output voltages of the AXP or temperature
of AXP chip itself, not so much for the input side if that side is
covered by power_supply devices (otherwise the information is being
duplicated and if so should only be via power_supply class, not
individual drivers).

> If you're using a distro, you might not have the kernel sources
> around, and digging through the in-system device tree isn't that
> straightforward if you don't know where to look.

If the dts is incomplete that's another issue.

> > NB: I should hopefully get my battery supply driver ported this WE,
> > ac-in/rtc are done, this WE. I'm going to default-off for them as well
> > (note that for some extra details should be provided in dts).
> 
> I'm guessing we won't be using the axp288 one?

In a first step I won't be using it.

There are a few reasons for that:
- axp288 is based on ACPI/platform to provide configuration information
  instead of dts
- it makes extensive use of OCV and registers that are possibly present
  on axp20x but undocumented there.

Bruno
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Maxime Ripard July 31, 2015, 7:57 a.m. UTC | #5
On Fri, Jul 31, 2015 at 08:35:41AM +0200, Bruno Prémont wrote:
> > > NB: I should hopefully get my battery supply driver ported this WE,
> > > ac-in/rtc are done, this WE. I'm going to default-off for them as well
> > > (note that for some extra details should be provided in dts).
> > 
> > I'm guessing we won't be using the axp288 one?
> 
> In a first step I won't be using it.
> 
> There are a few reasons for that:
> - axp288 is based on ACPI/platform to provide configuration information
>   instead of dts

That's not an issue. You have an API to handle lookup in both DT and
ACPI (the fwnode_* stuff).

> - it makes extensive use of OCV and registers that are possibly present
>   on axp20x but undocumented there.

That will be more problematic. Still I don't see why we wouldn't use a
similar setup to what they did.

Maxime
Hans de Goede July 31, 2015, 8:31 a.m. UTC | #6
Hi,

On 31-07-15 08:35, Bruno Prémont wrote:
> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont wrote:
>>> Hi ChenYu,
>>>
>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>> Hi Hans,
>>>>
>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>> Add a node representing the usb power supply part of the axp209 pmic, note
>>>>> that the usb power supply and the (to be added later) ac power supply will
>>>>> each have their own child-node, so that they can be separately specified
>>>>> as power-supply for other nodes using a power-supply property with a
>>>>> phandle pointing to the right axp209 child-node.
>>>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>> ---
>>>>>   arch/arm/boot/dts/axp209.dtsi | 5 +++++
>>>>>   1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
>>>>> index 24c935c..051ab3b 100644
>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>> @@ -89,4 +89,9 @@
>>>>>                          regulator-name = "ldo5";
>>>>>                  };
>>>>>          };
>>>>> +
>>>>> +       usb_power_supply: usb_power_supply {
>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>> +               status = "disabled";
>>>>
>>>> Is there any reason to have this disabled by default?
>>>> AFAIK this is not something configurable in hardware,
>>>> and the driver just gives readouts and status updates.
>>>
>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>> (set-top boxes) so having the devices tell what power sources they
>>> have makes sense (otherwise those that have ac-in instead of otg would
>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>> stating presence in device dts.
>>
>> That's true. But this is a bit of hardware inside the AXP.
>> If it's power isn't routed, it should report it, shouldn't it?
>
> If the device does not use the input there are two cases:
> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>    to provide this indication)
> - the unused one is just not connected/connectable
>
> In either case the voltage of the unused/unusable branch is not really
> useful, nor is it useful to have it show up in sysfs.

Exactly my thinking on this, +1 .

>> And normal users would probably expect to read the input
>> voltage/current/status through sysfs (or maybe hwmon?),
>> instead of not finding the device and having to look through
>> the DTS.
>
> Only those supplies effectively present (in-use or with connectors
> to eventually use them) should be visible in sysfs.
> Having more of them will just confuse the users, they would be searching
> how they can connect the supply while there is no means to do so
> except soldering to the tiny AXP pins.

+1

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chen-Yu Tsai July 31, 2015, 9 a.m. UTC | #7
On Fri, Jul 31, 2015 at 4:31 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 31-07-15 08:35, Bruno Prémont wrote:
>>
>> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>>>
>>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont wrote:
>>>>
>>>> Hi ChenYu,
>>>>
>>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>>>
>>>>> Hi Hans,
>>>>>
>>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>>>
>>>>>> Add a node representing the usb power supply part of the axp209 pmic,
>>>>>> note
>>>>>> that the usb power supply and the (to be added later) ac power supply
>>>>>> will
>>>>>> each have their own child-node, so that they can be separately
>>>>>> specified
>>>>>> as power-supply for other nodes using a power-supply property with a
>>>>>> phandle pointing to the right axp209 child-node.
>>>>>>
>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>> ---
>>>>>>   arch/arm/boot/dts/axp209.dtsi | 5 +++++
>>>>>>   1 file changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/arch/arm/boot/dts/axp209.dtsi
>>>>>> b/arch/arm/boot/dts/axp209.dtsi
>>>>>> index 24c935c..051ab3b 100644
>>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>>> @@ -89,4 +89,9 @@
>>>>>>                          regulator-name = "ldo5";
>>>>>>                  };
>>>>>>          };
>>>>>> +
>>>>>> +       usb_power_supply: usb_power_supply {
>>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>>> +               status = "disabled";
>>>>>
>>>>>
>>>>> Is there any reason to have this disabled by default?
>>>>> AFAIK this is not something configurable in hardware,
>>>>> and the driver just gives readouts and status updates.
>>>>
>>>>
>>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>>> (set-top boxes) so having the devices tell what power sources they
>>>> have makes sense (otherwise those that have ac-in instead of otg would
>>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>>> stating presence in device dts.
>>>
>>>
>>> That's true. But this is a bit of hardware inside the AXP.
>>> If it's power isn't routed, it should report it, shouldn't it?
>>
>>
>> If the device does not use the input there are two cases:
>> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>>    to provide this indication)
>> - the unused one is just not connected/connectable
>>
>> In either case the voltage of the unused/unusable branch is not really
>> useful, nor is it useful to have it show up in sysfs.
>
>
> Exactly my thinking on this, +1 .
>
>>> And normal users would probably expect to read the input
>>> voltage/current/status through sysfs (or maybe hwmon?),
>>> instead of not finding the device and having to look through
>>> the DTS.
>>
>>
>> Only those supplies effectively present (in-use or with connectors
>> to eventually use them) should be visible in sysfs.
>> Having more of them will just confuse the users, they would be searching
>> how they can connect the supply while there is no means to do so
>> except soldering to the tiny AXP pins.
>
>
> +1

OK. So mostly tablets and dev boards would benefit from having both
power supplies. Is there a preference for usb-power-supply over
vbus-det-gpio if both are available? Or just list them both and
let the driver sort it out?

Thanks.


ChenYu
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hans de Goede July 31, 2015, 9:11 a.m. UTC | #8
Hi,

On 31-07-15 11:00, Chen-Yu Tsai wrote:
> On Fri, Jul 31, 2015 at 4:31 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 31-07-15 08:35, Bruno Prémont wrote:
>>>
>>> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>>>>
>>>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont wrote:
>>>>>
>>>>> Hi ChenYu,
>>>>>
>>>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>>>>
>>>>>> Hi Hans,
>>>>>>
>>>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>>>>
>>>>>>> Add a node representing the usb power supply part of the axp209 pmic,
>>>>>>> note
>>>>>>> that the usb power supply and the (to be added later) ac power supply
>>>>>>> will
>>>>>>> each have their own child-node, so that they can be separately
>>>>>>> specified
>>>>>>> as power-supply for other nodes using a power-supply property with a
>>>>>>> phandle pointing to the right axp209 child-node.
>>>>>>>
>>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>>> ---
>>>>>>>    arch/arm/boot/dts/axp209.dtsi | 5 +++++
>>>>>>>    1 file changed, 5 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/axp209.dtsi
>>>>>>> b/arch/arm/boot/dts/axp209.dtsi
>>>>>>> index 24c935c..051ab3b 100644
>>>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>>>> @@ -89,4 +89,9 @@
>>>>>>>                           regulator-name = "ldo5";
>>>>>>>                   };
>>>>>>>           };
>>>>>>> +
>>>>>>> +       usb_power_supply: usb_power_supply {
>>>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>>>> +               status = "disabled";
>>>>>>
>>>>>>
>>>>>> Is there any reason to have this disabled by default?
>>>>>> AFAIK this is not something configurable in hardware,
>>>>>> and the driver just gives readouts and status updates.
>>>>>
>>>>>
>>>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>>>> (set-top boxes) so having the devices tell what power sources they
>>>>> have makes sense (otherwise those that have ac-in instead of otg would
>>>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>>>> stating presence in device dts.
>>>>
>>>>
>>>> That's true. But this is a bit of hardware inside the AXP.
>>>> If it's power isn't routed, it should report it, shouldn't it?
>>>
>>>
>>> If the device does not use the input there are two cases:
>>> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>>>     to provide this indication)
>>> - the unused one is just not connected/connectable
>>>
>>> In either case the voltage of the unused/unusable branch is not really
>>> useful, nor is it useful to have it show up in sysfs.
>>
>>
>> Exactly my thinking on this, +1 .
>>
>>>> And normal users would probably expect to read the input
>>>> voltage/current/status through sysfs (or maybe hwmon?),
>>>> instead of not finding the device and having to look through
>>>> the DTS.
>>>
>>>
>>> Only those supplies effectively present (in-use or with connectors
>>> to eventually use them) should be visible in sysfs.
>>> Having more of them will just confuse the users, they would be searching
>>> how they can connect the supply while there is no means to do so
>>> except soldering to the tiny AXP pins.
>>
>>
>> +1
>
> OK. So mostly tablets and dev boards would benefit from having both
> power supplies. Is there a preference for usb-power-supply over
> vbus-det-gpio if both are available? Or just list them both and
> let the driver sort it out?

So far I've simply stuck with whatever the fex file uses, if both
are specified the driver will prefer the gpio, but that is not
documented behavior. IMHO it would be better to pick and specify
only one.

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hans de Goede July 31, 2015, 9:14 a.m. UTC | #9
Hi,

On 31-07-15 11:00, Chen-Yu Tsai wrote:
> On Fri, Jul 31, 2015 at 4:31 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 31-07-15 08:35, Bruno Prémont wrote:
>>>
>>> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>>>>
>>>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont wrote:
>>>>>
>>>>> Hi ChenYu,
>>>>>
>>>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>>>>
>>>>>> Hi Hans,
>>>>>>
>>>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>>>>
>>>>>>> Add a node representing the usb power supply part of the axp209 pmic,
>>>>>>> note
>>>>>>> that the usb power supply and the (to be added later) ac power supply
>>>>>>> will
>>>>>>> each have their own child-node, so that they can be separately
>>>>>>> specified
>>>>>>> as power-supply for other nodes using a power-supply property with a
>>>>>>> phandle pointing to the right axp209 child-node.
>>>>>>>
>>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>>> ---
>>>>>>>    arch/arm/boot/dts/axp209.dtsi | 5 +++++
>>>>>>>    1 file changed, 5 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/arm/boot/dts/axp209.dtsi
>>>>>>> b/arch/arm/boot/dts/axp209.dtsi
>>>>>>> index 24c935c..051ab3b 100644
>>>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>>>> @@ -89,4 +89,9 @@
>>>>>>>                           regulator-name = "ldo5";
>>>>>>>                   };
>>>>>>>           };
>>>>>>> +
>>>>>>> +       usb_power_supply: usb_power_supply {
>>>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>>>> +               status = "disabled";
>>>>>>
>>>>>>
>>>>>> Is there any reason to have this disabled by default?
>>>>>> AFAIK this is not something configurable in hardware,
>>>>>> and the driver just gives readouts and status updates.
>>>>>
>>>>>
>>>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>>>> (set-top boxes) so having the devices tell what power sources they
>>>>> have makes sense (otherwise those that have ac-in instead of otg would
>>>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>>>> stating presence in device dts.
>>>>
>>>>
>>>> That's true. But this is a bit of hardware inside the AXP.
>>>> If it's power isn't routed, it should report it, shouldn't it?
>>>
>>>
>>> If the device does not use the input there are two cases:
>>> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>>>     to provide this indication)
>>> - the unused one is just not connected/connectable
>>>
>>> In either case the voltage of the unused/unusable branch is not really
>>> useful, nor is it useful to have it show up in sysfs.
>>
>>
>> Exactly my thinking on this, +1 .
>>
>>>> And normal users would probably expect to read the input
>>>> voltage/current/status through sysfs (or maybe hwmon?),
>>>> instead of not finding the device and having to look through
>>>> the DTS.
>>>
>>>
>>> Only those supplies effectively present (in-use or with connectors
>>> to eventually use them) should be visible in sysfs.
>>> Having more of them will just confuse the users, they would be searching
>>> how they can connect the supply while there is no means to do so
>>> except soldering to the tiny AXP pins.
>>
>>
>> +1
>
> OK. So mostly tablets and dev boards would benefit from having both
> power supplies. Is there a preference for usb-power-supply over
> vbus-det-gpio if both are available? Or just list them both and
> let the driver sort it out?

p.s.

Note that the driver can even deal with not having a vbus-det at
all. It will then simulate a vbus drop for long enough to terminate
the current session when ever the id pin changes. This is necessary
for the original cubieboard which has its vbus wired hard to 5V.

This MUST only be used on boards woth broken vbus-det though, having
vbus-det is important to avoid feeding power to the outside when
a charger is plugged in (a charge cable should never pull the id
pin low, but you never know).

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Chen-Yu Tsai July 31, 2015, 10:06 a.m. UTC | #10
On Fri, Jul 31, 2015 at 5:11 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
>
> On 31-07-15 11:00, Chen-Yu Tsai wrote:
>>
>> On Fri, Jul 31, 2015 at 4:31 PM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>>
>>> Hi,
>>>
>>>
>>> On 31-07-15 08:35, Bruno Prémont wrote:
>>>>
>>>>
>>>> On Fri, 31 Jul 2015 14:14:28 +0800 Chen-Yu Tsai wrote:
>>>>>
>>>>>
>>>>> On Fri, Jul 31, 2015 at 1:51 PM, Bruno Prémont wrote:
>>>>>>
>>>>>>
>>>>>> Hi ChenYu,
>>>>>>
>>>>>> On Fri, 31 Jul 2015 13:31:53 +0800 Chen-Yu Tsai wrote:
>>>>>>>
>>>>>>>
>>>>>>> Hi Hans,
>>>>>>>
>>>>>>> On Wed, Jun 10, 2015 at 5:37 AM, Hans de Goede wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> Add a node representing the usb power supply part of the axp209
>>>>>>>> pmic,
>>>>>>>> note
>>>>>>>> that the usb power supply and the (to be added later) ac power
>>>>>>>> supply
>>>>>>>> will
>>>>>>>> each have their own child-node, so that they can be separately
>>>>>>>> specified
>>>>>>>> as power-supply for other nodes using a power-supply property with a
>>>>>>>> phandle pointing to the right axp209 child-node.
>>>>>>>>
>>>>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>>>>> ---
>>>>>>>>    arch/arm/boot/dts/axp209.dtsi | 5 +++++
>>>>>>>>    1 file changed, 5 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/arch/arm/boot/dts/axp209.dtsi
>>>>>>>> b/arch/arm/boot/dts/axp209.dtsi
>>>>>>>> index 24c935c..051ab3b 100644
>>>>>>>> --- a/arch/arm/boot/dts/axp209.dtsi
>>>>>>>> +++ b/arch/arm/boot/dts/axp209.dtsi
>>>>>>>> @@ -89,4 +89,9 @@
>>>>>>>>                           regulator-name = "ldo5";
>>>>>>>>                   };
>>>>>>>>           };
>>>>>>>> +
>>>>>>>> +       usb_power_supply: usb_power_supply {
>>>>>>>> +               compatible = "x-powers,axp202-usb-power-supply";
>>>>>>>> +               status = "disabled";
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Is there any reason to have this disabled by default?
>>>>>>> AFAIK this is not something configurable in hardware,
>>>>>>> and the driver just gives readouts and status updates.
>>>>>>
>>>>>>
>>>>>>
>>>>>> There are some devices that only have 'AC-IN' instead of usb/otg power
>>>>>> (set-top boxes) so having the devices tell what power sources they
>>>>>> have makes sense (otherwise those that have ac-in instead of otg would
>>>>>> have to disable usb_power_supply) and avoids confusion by explicitly
>>>>>> stating presence in device dts.
>>>>>
>>>>>
>>>>>
>>>>> That's true. But this is a bit of hardware inside the AXP.
>>>>> If it's power isn't routed, it should report it, shouldn't it?
>>>>
>>>>
>>>>
>>>> If the device does not use the input there are two cases:
>>>> - ac-in and usb/otg in short-circuited (there is a bit in AXP registers
>>>>     to provide this indication)
>>>> - the unused one is just not connected/connectable
>>>>
>>>> In either case the voltage of the unused/unusable branch is not really
>>>> useful, nor is it useful to have it show up in sysfs.
>>>
>>>
>>>
>>> Exactly my thinking on this, +1 .
>>>
>>>>> And normal users would probably expect to read the input
>>>>> voltage/current/status through sysfs (or maybe hwmon?),
>>>>> instead of not finding the device and having to look through
>>>>> the DTS.
>>>>
>>>>
>>>>
>>>> Only those supplies effectively present (in-use or with connectors
>>>> to eventually use them) should be visible in sysfs.
>>>> Having more of them will just confuse the users, they would be searching
>>>> how they can connect the supply while there is no means to do so
>>>> except soldering to the tiny AXP pins.
>>>
>>>
>>>
>>> +1
>>
>>
>> OK. So mostly tablets and dev boards would benefit from having both
>> power supplies. Is there a preference for usb-power-supply over
>> vbus-det-gpio if both are available? Or just list them both and
>> let the driver sort it out?
>
>
> So far I've simply stuck with whatever the fex file uses, if both
> are specified the driver will prefer the gpio, but that is not
> documented behavior. IMHO it would be better to pick and specify
> only one.

From a functional standpoint, if usb-power-supply is available,
using it would be better because:

a) VBUS detection should be much better (IIRC cutoff at 4.4V)
b) Controllable max current draw
c) Can see the current draw

Of course not all boards would work. I think I even have a board
that is only half powered on when only USB VBUS is used.

ChenYu
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/axp209.dtsi b/arch/arm/boot/dts/axp209.dtsi
index 24c935c..051ab3b 100644
--- a/arch/arm/boot/dts/axp209.dtsi
+++ b/arch/arm/boot/dts/axp209.dtsi
@@ -89,4 +89,9 @@ 
 			regulator-name = "ldo5";
 		};
 	};
+
+	usb_power_supply: usb_power_supply {
+		compatible = "x-powers,axp202-usb-power-supply";
+		status = "disabled";
+	};
 };