[RFC,v2,0/4] power: supply: Add Support for RK817 Charger

Message

Chris Morgan July 26, 2021, 7:01 p.m. UTC

From: Chris Morgan <[e-mail]>

As this is my first charger driver, I'm submitting it as an RFC so I
can get input from the community on what else is needed to make the
driver consistent with the quality upstream expects.

This series is to add support for the Rockchip rk817 battery charger
which is present in all Rockchip RK817 PMICs. The driver was written
as a joint effort by Maya Matuszczyk <[e-mail]> and
myself Chris Morgan <[e-mail]>.

The driver requires some basic parameters be described about the
battery in the devicetree such as the maximum capacity, the minimum
and maximum voltage for the battery, the maximum charge current, the
maximum charge voltage, and the value of sample resistors and sleep
currents.

The hardware itself contains an ADC capable of measuring the voltage,
current, and temperature of the battery (though my implementation of an
Odroid Go Advance lacks a thermistor for temperature). It also contains
a columb counter, registers for tracking the measured voltage and
current at boot, and a few bytes of nvram for storing data.

Changes from V1:

 - Fixed a CLANG warning regarding an uninitalized variable.
 - Fixed a CLANG warning regarding a pointer as a bool value always
   returning as true.
 - Added Maya Matuszczyk to the Signed-off-by.

Chris Morgan (4):
  [RFC v2 1/4] dt-bindings: Add Rockchip rk817 battery charger support
  [RFC v2 2/4] mfd: Add Rockchip rk817 battery charger support
  [RFC v2 3/4] power: supply: Add charger driver for Rockchip RK817
  [RFC 4/4] arm64: dts: rockchip: add rk817 charger to Odroid Go Advance

 .../devicetree/bindings/mfd/rk808.txt         |  35 +-
 .../boot/dts/rockchip/rk3326-odroid-go2.dts   |  25 +
 drivers/mfd/rk808.c                           |  16 +-
 drivers/power/supply/Kconfig                  |   6 +
 drivers/power/supply/Makefile                 |   1 +
 drivers/power/supply/rk817_charger.c          | 932 ++++++++++++++++++
 include/linux/mfd/rk808.h                     |  87 ++
 7 files changed, 1099 insertions(+), 3 deletions(-)
 create mode 100644 drivers/power/supply/rk817_charger.c

Comments

Peter Geis July 28, 2021, 12:57 p.m. UTC | #1

On Mon, Jul 26, 2021 at 3:02 PM Chris Morgan <[e-mail]> wrote:
>
> From: Chris Morgan <[e-mail]>
>
> As this is my first charger driver, I'm submitting it as an RFC so I
> can get input from the community on what else is needed to make the
> driver consistent with the quality upstream expects.
>
> This series is to add support for the Rockchip rk817 battery charger
> which is present in all Rockchip RK817 PMICs. The driver was written
> as a joint effort by Maya Matuszczyk <[e-mail]> and
> myself Chris Morgan <[e-mail]>.
>
> The driver requires some basic parameters be described about the
> battery in the devicetree such as the maximum capacity, the minimum
> and maximum voltage for the battery, the maximum charge current, the
> maximum charge voltage, and the value of sample resistors and sleep
> currents.
>
> The hardware itself contains an ADC capable of measuring the voltage,
> current, and temperature of the battery (though my implementation of an
> Odroid Go Advance lacks a thermistor for temperature). It also contains
> a columb counter, registers for tracking the measured voltage and
> current at boot, and a few bytes of nvram for storing data.

Feel free to add my Tested-by: Peter Geis <[e-mail]>

Tested on the Pine64 Quartz64 Model A.
We can't go as low on the battery voltage, otherwise the SOC becomes
unstable before it can shut down.
<3625000 0> seems sufficient for us though (adding this for the
benefit of anyone else testing).

Voltages are accurate as measured at the battery.
Are there plans to expose the thermal sensor (if that's possible)?

>
> Changes from V1:
>
>  - Fixed a CLANG warning regarding an uninitalized variable.
>  - Fixed a CLANG warning regarding a pointer as a bool value always
>    returning as true.
>  - Added Maya Matuszczyk to the Signed-off-by.
>
> Chris Morgan (4):
>   [RFC v2 1/4] dt-bindings: Add Rockchip rk817 battery charger support
>   [RFC v2 2/4] mfd: Add Rockchip rk817 battery charger support
>   [RFC v2 3/4] power: supply: Add charger driver for Rockchip RK817
>   [RFC 4/4] arm64: dts: rockchip: add rk817 charger to Odroid Go Advance
>
>  .../devicetree/bindings/mfd/rk808.txt         |  35 +-
>  .../boot/dts/rockchip/rk3326-odroid-go2.dts   |  25 +
>  drivers/mfd/rk808.c                           |  16 +-
>  drivers/power/supply/Kconfig                  |   6 +
>  drivers/power/supply/Makefile                 |   1 +
>  drivers/power/supply/rk817_charger.c          | 932 ++++++++++++++++++
>  include/linux/mfd/rk808.h                     |  87 ++
>  7 files changed, 1099 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/power/supply/rk817_charger.c
>
> --
> 2.25.1
>
>
> _______________________________________________
> Linux-rockchip mailing list
> [e-mail]
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

Chris Morgan July 28, 2021, 2:15 p.m. UTC | #2

On Wed, Jul 28, 2021 at 08:57:52AM -0400, Peter Geis wrote:
> On Mon, Jul 26, 2021 at 3:02 PM Chris Morgan <[e-mail]> wrote:
> >
> > From: Chris Morgan <[e-mail]>
> >
> > As this is my first charger driver, I'm submitting it as an RFC so I
> > can get input from the community on what else is needed to make the
> > driver consistent with the quality upstream expects.
> >
> > This series is to add support for the Rockchip rk817 battery charger
> > which is present in all Rockchip RK817 PMICs. The driver was written
> > as a joint effort by Maya Matuszczyk <[e-mail]> and
> > myself Chris Morgan <[e-mail]>.
> >
> > The driver requires some basic parameters be described about the
> > battery in the devicetree such as the maximum capacity, the minimum
> > and maximum voltage for the battery, the maximum charge current, the
> > maximum charge voltage, and the value of sample resistors and sleep
> > currents.
> >
> > The hardware itself contains an ADC capable of measuring the voltage,
> > current, and temperature of the battery (though my implementation of an
> > Odroid Go Advance lacks a thermistor for temperature). It also contains
> > a columb counter, registers for tracking the measured voltage and
> > current at boot, and a few bytes of nvram for storing data.
> 
> Feel free to add my Tested-by: Peter Geis <[e-mail]>

Thank you. Have you checked out the audio driver yet? Would be nice to
see how it works on a device other than the Odroid Go Advance.

> 
> Tested on the Pine64 Quartz64 Model A.
> We can't go as low on the battery voltage, otherwise the SOC becomes
> unstable before it can shut down.
> <3625000 0> seems sufficient for us though (adding this for the
> benefit of anyone else testing).
> 
> Voltages are accurate as measured at the battery.
> Are there plans to expose the thermal sensor (if that's possible)?

Unfortunately it might be more than we are capable of. The output of
the thermal sensor from the ADC is stored in register 0x7c and 0x7d,
however the datasheet doesn't tell me the scale/how to translate it
to degrees. For the other ADC values there were coefficients in the
BSP kernel, but there is nothing for the temperature unfortunately.
Additionally, my hardware doesn't have a thermistor, so even if I
wanted to reverse engineer it some way I don't have the means. For
me those registers just read 0xff 0xff, which I assume is what it
reads when the values are not present.

> 
> >
> > Changes from V1:
> >
> >  - Fixed a CLANG warning regarding an uninitalized variable.
> >  - Fixed a CLANG warning regarding a pointer as a bool value always
> >    returning as true.
> >  - Added Maya Matuszczyk to the Signed-off-by.
> >
> > Chris Morgan (4):
> >   [RFC v2 1/4] dt-bindings: Add Rockchip rk817 battery charger support
> >   [RFC v2 2/4] mfd: Add Rockchip rk817 battery charger support
> >   [RFC v2 3/4] power: supply: Add charger driver for Rockchip RK817
> >   [RFC 4/4] arm64: dts: rockchip: add rk817 charger to Odroid Go Advance
> >
> >  .../devicetree/bindings/mfd/rk808.txt         |  35 +-
> >  .../boot/dts/rockchip/rk3326-odroid-go2.dts   |  25 +
> >  drivers/mfd/rk808.c                           |  16 +-
> >  drivers/power/supply/Kconfig                  |   6 +
> >  drivers/power/supply/Makefile                 |   1 +
> >  drivers/power/supply/rk817_charger.c          | 932 ++++++++++++++++++
> >  include/linux/mfd/rk808.h                     |  87 ++
> >  7 files changed, 1099 insertions(+), 3 deletions(-)
> >  create mode 100644 drivers/power/supply/rk817_charger.c
> >
> > --
> > 2.25.1
> >
> >
> > _______________________________________________
> > Linux-rockchip mailing list
> > [e-mail]
> > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-rockchip&amp;data=04%7C01%7C%7Ca5cf5e4a240e4f76b67508d951c75713%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637630738863903897%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=driHUKoWSfpQ5yV7E3Jo7pyQDwoxZ7wl56A8OB0XwyA%3D&amp;reserved=0

Peter Geis July 28, 2021, 2:26 p.m. UTC | #3

On Wed, Jul 28, 2021 at 10:15 AM Chris Morgan <[e-mail]> wrote:
>
> On Wed, Jul 28, 2021 at 08:57:52AM -0400, Peter Geis wrote:
> > On Mon, Jul 26, 2021 at 3:02 PM Chris Morgan <[e-mail]> wrote:
> > >
> > > From: Chris Morgan <[e-mail]>
> > >
> > > As this is my first charger driver, I'm submitting it as an RFC so I
> > > can get input from the community on what else is needed to make the
> > > driver consistent with the quality upstream expects.
> > >
> > > This series is to add support for the Rockchip rk817 battery charger
> > > which is present in all Rockchip RK817 PMICs. The driver was written
> > > as a joint effort by Maya Matuszczyk <[e-mail]> and
> > > myself Chris Morgan <[e-mail]>.
> > >
> > > The driver requires some basic parameters be described about the
> > > battery in the devicetree such as the maximum capacity, the minimum
> > > and maximum voltage for the battery, the maximum charge current, the
> > > maximum charge voltage, and the value of sample resistors and sleep
> > > currents.
> > >
> > > The hardware itself contains an ADC capable of measuring the voltage,
> > > current, and temperature of the battery (though my implementation of an
> > > Odroid Go Advance lacks a thermistor for temperature). It also contains
> > > a columb counter, registers for tracking the measured voltage and
> > > current at boot, and a few bytes of nvram for storing data.
> >
> > Feel free to add my Tested-by: Peter Geis <[e-mail]>
>
> Thank you. Have you checked out the audio driver yet? Would be nice to
> see how it works on a device other than the Odroid Go Advance.

I would love to, but we still need to port the i2s-tdm driver over for
that functionality in the rk356x.
Once we have that, I'll be happy to test it.

>
> >
> > Tested on the Pine64 Quartz64 Model A.
> > We can't go as low on the battery voltage, otherwise the SOC becomes
> > unstable before it can shut down.
> > <3625000 0> seems sufficient for us though (adding this for the
> > benefit of anyone else testing).
> >
> > Voltages are accurate as measured at the battery.
> > Are there plans to expose the thermal sensor (if that's possible)?
>
> Unfortunately it might be more than we are capable of. The output of
> the thermal sensor from the ADC is stored in register 0x7c and 0x7d,
> however the datasheet doesn't tell me the scale/how to translate it
> to degrees. For the other ADC values there were coefficients in the
> BSP kernel, but there is nothing for the temperature unfortunately.
> Additionally, my hardware doesn't have a thermistor, so even if I
> wanted to reverse engineer it some way I don't have the means. For
> me those registers just read 0xff 0xff, which I assume is what it
> reads when the values are not present.

Okay, we have the thermistor connected so I can look into this.

>
> >
> > >
> > > Changes from V1:
> > >
> > >  - Fixed a CLANG warning regarding an uninitalized variable.
> > >  - Fixed a CLANG warning regarding a pointer as a bool value always
> > >    returning as true.
> > >  - Added Maya Matuszczyk to the Signed-off-by.
> > >
> > > Chris Morgan (4):
> > >   [RFC v2 1/4] dt-bindings: Add Rockchip rk817 battery charger support
> > >   [RFC v2 2/4] mfd: Add Rockchip rk817 battery charger support
> > >   [RFC v2 3/4] power: supply: Add charger driver for Rockchip RK817
> > >   [RFC 4/4] arm64: dts: rockchip: add rk817 charger to Odroid Go Advance
> > >
> > >  .../devicetree/bindings/mfd/rk808.txt         |  35 +-
> > >  .../boot/dts/rockchip/rk3326-odroid-go2.dts   |  25 +
> > >  drivers/mfd/rk808.c                           |  16 +-
> > >  drivers/power/supply/Kconfig                  |   6 +
> > >  drivers/power/supply/Makefile                 |   1 +
> > >  drivers/power/supply/rk817_charger.c          | 932 ++++++++++++++++++
> > >  include/linux/mfd/rk808.h                     |  87 ++
> > >  7 files changed, 1099 insertions(+), 3 deletions(-)
> > >  create mode 100644 drivers/power/supply/rk817_charger.c
> > >
> > > --
> > > 2.25.1
> > >
> > >
> > > _______________________________________________
> > > Linux-rockchip mailing list
> > > [e-mail]
> > > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists.infradead.org%2Fmailman%2Flistinfo%2Flinux-rockchip&amp;data=04%7C01%7C%7Ca5cf5e4a240e4f76b67508d951c75713%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637630738863903897%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=driHUKoWSfpQ5yV7E3Jo7pyQDwoxZ7wl56A8OB0XwyA%3D&amp;reserved=0