linux-kernel - Re: [PATCH v4 7/8] power: supply: rt5033_battery: Adopt status property from charger

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fb554a6f-6058-a9b3-b39b-eb9f634a212c@rocketmail.com>
Date:   Wed, 10 May 2023 01:00:13 +0200
From:   Jakob Hauser <jahau@...ketmail.com>
To:     Sebastian Reichel <sebastian.reichel@...labora.com>
Cc:     Lee Jones <lee@...nel.org>, Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Beomho Seo <beomho.seo@...sung.com>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Stephan Gerhold <stephan@...hold.net>,
        Raymond Hackley <raymondhackley@...tonmail.com>,
        Pavel Machek <pavel@....cz>, Axel Lin <axel.lin@...ics.com>,
        ChiYuan Huang <cy_huang@...htek.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Henrik Grimler <henrik@...mler.se>, linux-pm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        phone-devel@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht
Subject: Re: [PATCH v4 7/8] power: supply: rt5033_battery: Adopt status
 property from charger

Hi Sebastian,

On 09.05.23 09:25, Sebastian Reichel wrote:
> On Tue, May 09, 2023 at 03:01:32AM +0200, Jakob Hauser wrote:
>> On 09.05.23 00:06, Sebastian Reichel wrote:

...

>>> I suppose your DT is missing the connection between the charger and
>>> the battery:
>>>
>>> rt5033_charger: charger {
>>>       compatible = "rt5033-charger";
>>>       ...
>>> }
>>>
>>> fuel-gauge {
>>>       compatible = "rt5033-battery";
>>>       ...
>>>       power-supplies = <&rt5033_charger>; // you are probably missing this
>>> };
>>>
>>> See also Documentation/devicetree/bindings/power/supply/power-supply.yaml
>>
>> ...
>>
>> Thanks for the hints.
>>
>> This leads to updating the dt-bindings because adding the "power-supplies"
>> property is important to be aware of.
> 
> It should already be part of the binding, because richtek,rt5033-battery.yaml has
> 
> allOf:
>    - $ref: power-supply.yaml#

Uh, I see, you're two steps ahead ;)

>> Btw. first it didn't work. It took me quite some time to debug. I needed to
>> add "psy_cfg.of_node = client->dev.of_node;" to the rt5033-battery probe
>> function.
>>
>> Now it works. However, there is a new problem. The battery driver gets
>> probed first. The charger driver a bit later. In the meantime the battery
>> driver spams dmesg with several "Failed to register power supply" because
>> the charger driver isn't available yet. Once the charger driver is there, it
>> works fine and dmesg becomes silent.
>>
>> With the current state of the patchset:
>> dmesg | grep rt5033
>> [   13.628030] rt5033 6-0034: Device found (rev. 6)
>> [   13.633552] rt5033-led: Failed to locate of_node [id: -1]
>> [   13.790478] rt5033-charger rt5033-charger: DMA mask not set
>>
>> With the changes discussed here:
>> dmesg | grep rt5033
>> [   15.741915] rt5033-battery 4-0035: Failed to register power supply
>> [   15.752894] rt5033-battery 4-0035: Failed to register power supply
>> [   15.795458] rt5033-battery 4-0035: Failed to register power supply
>> [   15.910760] rt5033-battery 4-0035: Failed to register power supply
>> [   15.913187] rt5033 6-0034: Device found (rev. 6)
>> [   15.914341] rt5033-led: Failed to locate of_node [id: -1]
>> [   15.920052] rt5033-battery 4-0035: Failed to register power supply
>> [   15.927262] rt5033-battery 4-0035: Failed to register power supply
>> [   16.017131] rt5033-battery 4-0035: Failed to register power supply
>> [   16.017401] rt5033-charger rt5033-charger: DMA mask not set
>>
>> The message is comming from the rt5033-battery probe function, it's the
>> power_supply_register() that fails.
>>
>> Any ideas what could be done about this?
> 
> Replace the dev_err() with dev_err_probe():
> 
> if (IS_ERR(battery->psy))
>      return dev_err_probe(&client->dev, PTR_ERR(battery->psy), "Failed to register power supply\n");
> 
> That will avoid printing an error for -EPROBE_DEFER.

Confirming, that works. Thanks!

Kind regards,
Jakob