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

Krzysztof Kozlowski
2024-04-08T12:01:14Z

On 08/04/2024 13:51, Ondřej Jirman wrote:
> Hi Krzysztof,
>
> On Mon, Apr 08, 2024 at 01:17:32PM GMT, Krzysztof Kozlowski wrote:
>> On 08/04/2024 12:51, Pavel Machek wrote:
>>> Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
>>> but I did best I could.
>>>
>>> Signed-off-by: Pavel Machek <pavel@ucw.cz>
>>
>> ...
>>
>>> + cabledet-gpios:
>>> + maxItems: 1
>>> + description: GPIO controlling CABLE_DET (C3) pin.
>>> +
>>> + avdd10-supply:
>>> + description: 1.0V power supply going to AVDD10 (A4, ...) pins
>>> +
>>> + dvdd10-supply:
>>> + description: 1.0V power supply going to DVDD10 (D6, ...) pins
>>> +
>>> + avdd18-supply:
>>> + description: 1.8V power supply going to AVDD18 (E3, ...) pins
>>> +
>>> + dvdd18-supply:
>>> + description: 1.8V power supply going to DVDD18 (G4, ...) pins
>>> +
>>> + avdd33-supply:
>>> + description: 3.3V power supply going to AVDD33 (C4, ...) pins
>>> +
>>> + i2c-supply: true
>>> + vconn-supply: true
>>
>> There are no such supplies like i2c and vconn on the schematics.
>
> Which schematics?
>
> ANX7688 has VCONN1/2_EN GPIOs that control a switching of VCONN power supply
> to resective CCx pins. That's just a switch signal. Power for VCONN needs
> to come from somewhere and the driver needs to enable the regulator at
> the appropriate time only.
>
> On Pinephone it can't be an always on power supply and needs to be enabled
> only when used due to HW design of the circuit. (default without ANX driver
> initialized would be to shove 5V to both CC pins, which breaks Type-C spec)
>
> I2C supply is needed for I2C bus to work, apparently. There's nothing
> that says that I2C pull-ups supply has to come from supplies powering the
> chip. I2C I/O is open drain and the device needs to enable a bus supply
> in order to communicate.

No, that's misunderstanding of DT. These are not supplies to anx7688.

Bus supply is not related to anx7688.

>
> You can say that bus master should be managing the bus supply, but you'd still
> have a problem that each device may be behind a voltage translator, and
> logically, bus master driver should care only about its side of the bus then.
> Both side of level shifter need the pull-up power enabled.

Again, that's nothing related to anx7688. If you want to add it here,
why not to every device everywhere?

>
> You can also make an argument that bus supply can be always on, but that
> causes several other issues on Pinephone due to shared nature of most
> resources like these. There are other devices on shared power rails, that
> need to be turned off during sleep, etc.
>

No, do not add non-existing properties on this device as workaround for
other issues.

Please drop these two supplies *and all other which do not exist* on
anx7688.

Best regards,
Krzysztof