Re: [PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding document

Krzysztof Kozlowski
2024-04-11T19:59:39Z

On 10/04/2024 04:20, Ondřej Jirman wrote:
> On Mon, Apr 08, 2024 at 10:12:30PM GMT, Krzysztof Kozlowski wrote:
>> On 08/04/2024 17:17, Ondřej Jirman wrote:
>>>
>>> Now for things to not fail during suspend/resume based on PM callbacks
>>> invocation order, anx7688 driver needs to enable this regulator too, as long
>>> as it needs it.
>>
>> No, the I2C bus driver needs to manage it. Not one individual I2C
>> device. Again, why anx7688 is specific? If you next phone has anx8867,
>> using different driver, you also add there i2c-supply? And if it is
>> nxp,ptn5100 as well?
>
> Yes, that could work, if I2C core would manage this.

Either I don't understand about which I2C regulator you speak or this is
not I2C core regulator. This is a regulator to be managed by the I2C
controller, not by I2C core.

>
>>>
>>> I can put bus-supply to I2C controller node, and read it from the ANX7688 driver
>>> I guess, by going up a DT node. Whether that's going to be acceptable, I don't
>>> know.
>>>
>>>
>>> VCONN regulator I don't know where else to put either. It doesn't seem to belong
>>> anywhere. It's not something directly connected to Type-C connector, so
>>> not part of connector bindings, and there's nothing else I can see, other
>>> than anx7688 device which needs it for core functionality.
>>
>> That sounds like a GPIO, not regulator. anx7688 has GPIOs, right? On
>> Pinephone they go to regulator, but on FooPhone also using anx7688 they
>> go somewhere else, so why this anx7688 assumes this is a regulator?
>
> CC1/CC2_VCONN control pins are "GPIO" of anx7688, sort of. They have fixed
> purpose of switching external 5V regulator output to one of the CC pins
> on type-c port. I don't care what other purpose with some other firmware
> someone puts to those pins. It's irrelevant to the use case of anx7688
> as a type-c controller/HDMI bridge, which we're describing here.
>
> VCONN regulator is an actual GPIO controlled regulator on the board, and
> needs to be controlled by the anx7688 driver. So that CC1/CC2_VCONN control
> pins driven by the firmware actually do what they're supposed to do.
>
> Not sure why it would be a business of anything else but anx7688 driver
> enabling this regulator, because only this driver knows and cares about this.
> If some other board doesn't have the need to manually enable the regulator, or
> doesn't have the regulator, it can simply be optional.
>
> There are also some other funky supplies in the bindings, that are not connected
> to the chip in any way, but need to be controlled by the driver:
>
> + vbus-supply: true
> + vbus-in-supply: true

Yeah, the vconn looks reasonable. Just provide description of the
supply, so it will be obvious.

>

Best regards,
Krzysztof