Re: [04/15] power: supply: bq24190_charger: Add no_register_reset pdata flag

Hans de Goede
2017-03-20T18:02:15Z

Hi,

On 20-03-17 18:51, Liam Breck wrote:
> On Mon, Mar 20, 2017 at 10:04 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>>
>> On 20-03-17 14:54, Hans de Goede wrote:
>>>
>>> Hi,
>>>
>>> On 20-03-17 06:27, Sebastian Reichel wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Sun, Mar 19, 2017 at 10:42:00AM +0100, Hans de Goede wrote:
>>>
>>>
>>> <snip>
>>>
>>>>> We want one driver which is solidly in control of the charger
>>>>> since getting that wrong is quite bad and we want to extend
>>>>> the information it is exporting to userspace in the form of
>>>>> power_supply properties with some extra info.
>>>>>
>>>>> So taking that as a starting point and generalizing that,
>>>>> I think that if we want we can make something more generic then
>>>>> my original patch for this. My idea is to introduce something
>>>>> called a power_supply_properties_provider, with an API
>>>>> like this:
>>>>
>>>>
>>>> Thanks for thinking about this. From what I can see it should
>>>> be enough to just avoid exposing a battery device at all in
>>>> the charger driver, though. It does not really provide useful
>>>> information anyways.
>>>
>>>
>>> Interesting suggestion, 2 remarks / questions:
>>>
>>> 1) Given that we do not want to break existing setups
>>> which may depend on the existing battery interface on the bq24190
>>> driver and add a flag to not register it, or are you suggesting
>>> to simply remove it all together? Removing it all together would
>>> be better from a maintenance pov. Liam, you indicated that you
>>> are the main user of the bq24190 driver currently, do you need
>>> the battery power_supply interface for your use case ?
>>>
>>> 2) Not using the battery interface of the bq24190 driver at all
>>> means that the fuelgauge driver needs to grow some extra
>>> properties, specifically it will need to start reporting status,
>>> something which the bq24190 driver really has a better idea
>>> of the the fuel gauge.
>
> Our userspace looks at bq24190-battery & -charger. If your userspace
> can work with /sys/class/power_supply/whisky_cove-battery/* can't you
> leave .bq24190-battery as is?

No, my userspace is a generic distro which uses upower, which as I
explained before will look at *all* battery type power_supply devices
and will consider each of them being a separate battery. There *must*
be only one battery type power_supply per physical battery, that is
simply how the userspace ABI works.

> bq24190-battery's properties could move to -charger, tho health &
> online would have to change name, replace same in -charger, or be
> dropped.

That is one option, I can also simply make the registration dependent
on a device-property, then you do not need to change your userspace.

In that case I would prefer for the behavior to be to not register
the battery power_supply device unless the boolean device(tree)-property
named "linux,register-battery-power-supply" is present. But if you
need things to keep working the same with older dtb files which
will not contain that property, we can also go for:
"linux,disable-battery-power-supply"

Regards,

Hans