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

Liam Breck
2017-03-20T18:19:10Z

On Mon, Mar 20, 2017 at 11:01 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> 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"

Just if (!pdata->x) register(battery). Eventually I'll have to move
those properties to charger, as we added a fuel gauge after charger
driver was written.

Or change bq24190_battery_desc.type to not be _BATTERY?