Re: [PATCH 1/2] power: supply: bq25890: Add CC voltage to ADC properties

[Marek Vasut <marex@denx.de>]

On 10/11/22 09:38, Hans de Goede wrote:
> Hi,

Hi,

>>>> Though I think there is no proper property
>>>> for the VSYS value that is currently occupying VOLTAGE_NOW - this
>>>> might be better modelled as a separate regulator maybe?
>>>
>>> Ack, see above.
>>
>> We already do have a regulator in the bq25890 driver. The regulator is used as a switch to toggle OTG boost mode (supply from battery to VBUS), but I don't see any users of this functionality, and I cannot imagine how this would be modeled in DT. (Hans, can you clarify?)
> 
> Ah, with the Ack I meant ack for the "I think there is no proper property for the VSYS value" I did not meant to ack the regulator bit, I don't directly see how having registering a regulator device for Vsys would be useful, sorry. As mentioned in my original email I believe that just adding a new property for Vsys makes the most sense.

I disagree here, have a look at the new series I posted that adds the 
Vsys regulator, esp. patch 7/7 . I think the Vsys regulator does make 
sense, since it can be the supply for PMIC, which would let us model 
that hardware connection between the charger and PMIC properly, and even 
in DT.

> Note the OTG regulator is useful to enable/disable 5V boost output when used with e.g. a micro-usb connector and that micro-usb connector is used with an micro-USB OTG host-mode cable / dongle.
> 
>> There is the usb_work (usb_register_notifier()) which triggers workqueue which does the same, toggles OTG boost mode, but this is only used in case a valid USB PHY is found. I didn't find any users of this either.
> 
> This is used, but the use is hidden away pretty well I admit. Some X86 tablets from the Cherry Trail era have ACPI tables where the charging / fuel-gauge ICs are not handled in ACPI (as they typically are on x86) so we need to do it ourselves.
> 
> Specifically the charger IC is connected to a special I2c bus of the PMIC (which itself is an i2c-device so we have an i2c-attached i2c-controller, crazy ...) the driver for this special PMIC embedded i2c-controller is also responsible for instantiating the charger chip i2c-client and this setup uses the OTG regulator device, see: drivers/i2c/busses/i2c-cht-wc.c specifically these bits:

[...]

> And then the drivers/extcon/extcon-intel-cht-wc.c uses
> the vbus regulator to control the 5V out on the micro-USB.

Uh, now I understand, thanks for clarification.

> (also note these devices do not instantiate a usb_phy device anywhere, which is why the V5 boost is handled through the regulator framework)
> 
> TL;DR: the regulator device for the V5 boost output is used, please don't remove it :)

I won't, I only extended the current regulator registration in the end.

>> Anyway, maybe we can extend the regulator to report VBus and register another one to report VSys, where the VSys one can be plugged e.g. as supply for PMIC in DT ?
> 
> I'm not sure if a regulator device for Vsys is really useful, AFAIK Vsys can not be turned off, nor can the voltage level be controlled...

It can be used as a supply for system PMIC, so I think it is actually 
useful. Sure, you cannot turn the Vsys on/off, but that's not the sole 
purpose of the regulator. The PMIC can get its input voltage and 
configure itself accordingly, which without the Vsys regulator providing 
its current voltage is not possible.