Re: [PATCH 1/4 v5] dt-bindings: Add Rockchip rk817 battery charger support

[Chris Morgan <macroalpha82@gmail.com>]

On Tue, Apr 05, 2022 at 03:35:04PM +0200, Krzysztof Kozlowski wrote:
> On 05/04/2022 15:12, Chris Morgan wrote:
> > On Tue, Apr 05, 2022 at 01:16:55PM +0200, Krzysztof Kozlowski wrote:
> >> On 04/04/2022 23:57, Chris Morgan wrote:
> >>> From: Chris Morgan <macromorgan@hotmail.com>
> >>>
> >>> Create dt-binding documentation to document rk817 battery and charger
> >>> usage. New device-tree properties have been added.
> >>>
> >>> - rockchip,resistor-sense-micro-ohms: The value in microohms of the
> >>>                                       sample resistor.
> >>> - rockchip,sleep-enter-current-microamp: The value in microamps of the
> >>>                                          sleep enter current.
> >>> - rockchip,sleep-filter-current: The value in microamps of the sleep
> >>>                                  filter current.
> >>>
> >>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> >>> Signed-off-by: Maya Matuszczyk <maccraft123mc@gmail.com>
> >>> ---
> >>>  .../bindings/mfd/rockchip,rk817.yaml          | 48 +++++++++++++++++++
> >>>  1 file changed, 48 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> >>> index bfc1720adc43..b949d406a487 100644
> >>> --- a/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> >>> +++ b/Documentation/devicetree/bindings/mfd/rockchip,rk817.yaml
> >>> @@ -117,6 +117,47 @@ properties:
> >>>          description:
> >>>            Describes if the microphone uses differential mode.
> >>>  
> >>> +  battery:
> >>
> >> I wonder why do you call it a batter while it is a charger, isn't it?
> > 
> > It is a driver for both the battery and charger. I'd argue about 95% of
> > it is battery functions and the other 5% is managing the IRQs for plug
> > removal/insertion and capturing the incoming voltage and current. In
> > the BSP kernel these were two seperate drivers, but there was so little
> > that needed to be done for the charger (and users probably don't need
> > plug IRQs if they aren't using a battery anyway since the system will
> > shut off on a plug out event due to no power...).
> 
> What do you mean by driver for "battery"? Like some smart-battery
> system? with embedded battery (RK817 comes with embedded battery) Or a
> fuel gauge? Judging by power supply properties it looks like fuel gauge.

It's basically just additional registers in the rk817 PMIC. The PMIC is
controlled via I2C and contains multiple regulators, an audio codec,
an RTC, some GPIO, some nvram, a columb counter for the battery, and an
ADC for measuring input current/voltage/temperature (for battery or
charger). This driver deals with the columb counter, the ADC, and a few
bits of nvram for storing battery values to retain backwards
compatibility with the BSP kernel and bootloader, and handling IRQs
related to inserting or removing the charger (curiously my
implementation ALSO has a charger sense GPIO in addition to the PMIC
being able to sense that).

The driver itself is named rk817_charger. If you think I should change
this from battery to "fuel gauge" or "charger" let me know and I can
resubmit. Whatever makes it clearer for everyone.

Thank you.

> 
> "Battery" should rather be used for the node referenced by
> "monitored-battery"...
> 
> Best regards,
> Krzysztof