The ACOK pin on the bq24735 is active-high, of course meaning that when
AC is OK the pin is high. However, all Tegra dts files have incorrectly
specified active-high even though the signal is inverted on the Tegra
boards. This has worked since the Linux driver has also inverted the
meaning of the GPIO. Fix this situation by simply specifying in the
bindings what everybody else agrees on; that the ti,ac-detect-gpios is
active on AC adapter absence.
Signed-off-by: Peter Rosin <[email protected]>
---
Hi!
This patch is the result of this discussion:
http://marc.info/?t=148152531800002
I don't like how it changes the one thing that is seems correct, but
what to do?
Cheers,
Peter
Documentation/devicetree/bindings/power/supply/ti,bq24735.txt | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
index 3bf55757ceec..43d56c49455b 100644
--- a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
+++ b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
@@ -8,8 +8,9 @@ Optional properties :
- interrupts : Specify the interrupt to be used to trigger when the AC
adapter is either plugged in or removed.
- ti,ac-detect-gpios : This GPIO is optionally used to read the AC adapter
- presence. This is a Host GPIO that is configured as an input and
- connected to the bq24735.
+ status. This is a Host GPIO that is configured as an input and
+ connected to the bq24735, typically the ACOK pin (note: the GPIO should
+ be active on AC adapter absence).
- ti,charge-current : Used to control and set the charging current. This value
must be between 128mA and 8.128A with a 64mA step resolution. The POR value
is 0x0000h. This number is in mA (e.g. 8192), see spec for more information
--
2.1.4
On 12/15/2016 05:21 AM, Peter Rosin wrote:
> The ACOK pin on the bq24735 is active-high, of course meaning that when
> AC is OK the pin is high. However, all Tegra dts files have incorrectly
> specified active-high even though the signal is inverted on the Tegra
> boards. This has worked since the Linux driver has also inverted the
> meaning of the GPIO. Fix this situation by simply specifying in the
> bindings what everybody else agrees on; that the ti,ac-detect-gpios is
> active on AC adapter absence.
>
> Signed-off-by: Peter Rosin <[email protected]>
> ---
>
> Hi!
>
> This patch is the result of this discussion:
> http://marc.info/?t=148152531800002
>
> I don't like how it changes the one thing that is seems correct, but
> what to do?
I haven't followed this thread so hopefully what I say is relevant. My
take is:
If the DT binding is correct or reasonable, keep it.
If the Tegra DTs contain incorrect content, and never worked correctly
in this aspect, then fix them. We do need to maintain DT
ABI/compatibility, but I believe only with stuff that actually worked
correctly. If the DT has a bug, just fix it.
That said, if ti,ac-detect-gpios is describing a host GPIO, then it's
entirely arbitrary which polarity it should have, i.e. the polarity is
not something specified by the bq24735 HW. In that case, feel free to
change either the binding to match the DT or the DT to match the
binding. Changing the DT to match the binding might still be better
since there could be other users you're not aware of, and they might
have written their DT correctly, and you don't want to break them.
On 2016-12-15 18:32, Stephen Warren wrote:
> On 12/15/2016 05:21 AM, Peter Rosin wrote:
>> The ACOK pin on the bq24735 is active-high, of course meaning that when
>> AC is OK the pin is high. However, all Tegra dts files have incorrectly
>> specified active-high even though the signal is inverted on the Tegra
>> boards. This has worked since the Linux driver has also inverted the
>> meaning of the GPIO. Fix this situation by simply specifying in the
>> bindings what everybody else agrees on; that the ti,ac-detect-gpios is
>> active on AC adapter absence.
>>
>> Signed-off-by: Peter Rosin <[email protected]>
>> ---
>>
>> Hi!
>>
>> This patch is the result of this discussion:
>> http://marc.info/?t=148152531800002
>>
>> I don't like how it changes the one thing that is seems correct, but
>> what to do?
>
> I haven't followed this thread so hopefully what I say is relevant. My
> take is:
>
> If the DT binding is correct or reasonable, keep it.
>
> If the Tegra DTs contain incorrect content, and never worked correctly
> in this aspect, then fix them. We do need to maintain DT
> ABI/compatibility, but I believe only with stuff that actually worked
> correctly. If the DT has a bug, just fix it.
>
> That said, if ti,ac-detect-gpios is describing a host GPIO, then it's
> entirely arbitrary which polarity it should have, i.e. the polarity is
> not something specified by the bq24735 HW. In that case, feel free to
> change either the binding to match the DT or the DT to match the
> binding. Changing the DT to match the binding might still be better
> since there could be other users you're not aware of, and they might
> have written their DT correctly, and you don't want to break them.
The bindings are fine.
The Tegra dts files are buggy, but the driver is also buggy, so those
two bugs cancel each other. So, the option is to either introduce
regressions by fixing the two bugs thus creating a flag day where
the kernel and dt needs to match. Or, just document what is going on
and change the bindings even if they are not wrong.
I suggest you read the discussion. We covered all this already, and it
is also in the commit message.
Cheers,
peda
On 15/12/16 12:21, Peter Rosin wrote:
> The ACOK pin on the bq24735 is active-high, of course meaning that when
> AC is OK the pin is high. However, all Tegra dts files have incorrectly
> specified active-high even though the signal is inverted on the Tegra
> boards. This has worked since the Linux driver has also inverted the
> meaning of the GPIO. Fix this situation by simply specifying in the
> bindings what everybody else agrees on; that the ti,ac-detect-gpios is
> active on AC adapter absence.
>
> Signed-off-by: Peter Rosin <[email protected]>
> ---
>
> Hi!
>
> This patch is the result of this discussion:
> http://marc.info/?t=148152531800002
>
> I don't like how it changes the one thing that is seems correct, but
> what to do?
>
> Cheers,
> Peter
>
> Documentation/devicetree/bindings/power/supply/ti,bq24735.txt | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
> index 3bf55757ceec..43d56c49455b 100644
> --- a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
> +++ b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
> @@ -8,8 +8,9 @@ Optional properties :
> - interrupts : Specify the interrupt to be used to trigger when the AC
> adapter is either plugged in or removed.
> - ti,ac-detect-gpios : This GPIO is optionally used to read the AC adapter
> - presence. This is a Host GPIO that is configured as an input and
> - connected to the bq24735.
> + status. This is a Host GPIO that is configured as an input and
> + connected to the bq24735, typically the ACOK pin (note: the GPIO should
> + be active on AC adapter absence).
> - ti,charge-current : Used to control and set the charging current. This value
> must be between 128mA and 8.128A with a 64mA step resolution. The POR value
> is 0x0000h. This number is in mA (e.g. 8192), see spec for more information
I don't think you need the 'typically' above, and I may have added ...
"the GPIO must be active on AC adapter absence despite the ACOK being
active-high on AC presence to be compatible with legacy boards".
... but otherwise ...
Acked-by: Jon Hunter <[email protected]>
Cheers
Jon
--
nvpublic