On Tue, Apr 05, 2022 at 04:00:09PM +0200, Krzysztof Kozlowski wrote: > On 05/04/2022 15:54, Chris Morgan wrote: > > 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@xxxxxxxxxxx> > >>>>> > >>>>> 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@xxxxxxxxxxx> > >>>>> Signed-off-by: Maya Matuszczyk <maccraft123mc@xxxxxxxxx> > >>>>> --- > >>>>> .../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). > > Actually you mentioned it in the cover letter - it's a PMIC, not a > battery. I doubt that it's embedded with a battery. :) > > > 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. > > Yeah, the property name and bindings should describe the hardware, so in > such case the hardware is rather a "charger" or "fuel-gauge". Your > "battery-cell" from DTS is probably just a "battery" (unless you expect > multiple cells?). > > Best regards, > Krzysztof Okay, when v6 comes around I'll change it to be "charger" instead of "battery" to make it more clear. There should only be a single battery instead of multiple cells, and according to the documentation I should be okay with describing the battery in the devicetree since it's not something easy for the end-user to change. I'd like to get someone to look at the meat and potatoes of the series before I submit a v6... I did a fairly substantial rewrite of the actual rk817_charger.c to solve for several problems and fix several bugs I found in extended testing. One of the major changes was to mirror the BSP in that I poll the PMIC every 8 seconds for updates and then store it in the driver struct rather than pull each value on demand as requested. I see other drivers doing this but I want to make sure that's acceptable upstream. Thank you.
