[PATCH v5 1/2] dt-bindings: usb: add analogix,anx7688.yaml
Ondřej Jirman
megous at megous.com
Fri Mar 5 17:24:37 UTC 2021
Hello Dafna,
On Fri, Mar 05, 2021 at 04:14:03PM +0100, Dafna Hirschfeld wrote:
> Hi
>
> On 05.03.21 15:34, Laurent Pinchart wrote:
> > Hi Dafna,
> >
> > Thank you for the patch.
> >
> > On Fri, Mar 05, 2021 at 01:43:50PM +0100, Dafna Hirschfeld wrote:
> > > ANX7688 is a USB Type-C port controller with a MUX. It converts HDMI 2.0 to
> > > DisplayPort 1.3 Ultra-HDi (4096x2160p60).
> > > The integrated crosspoint switch (the MUX) supports USB 3.1 data transfer
> > > along with the DisplayPort Alternate Mode signaling over USB Type-C.
> > > Additionally, an on-chip microcontroller (OCM) is available to manage the
> > > signal switching, Channel Configuration (CC) detection, USB Power
> > > Delivery (USB-PD), Vendor Defined Message (VDM) protocol support and other
> > > functions as defined in the USB TypeC and USB Power Delivery
> > > specifications.
> > >
> > > ANX7688 is found on Acer Chromebook R13 (elm) and on
> > > Pine64 PinePhone.
Thanks for your work on the bindings. :) It would be great to find something
acceptable for mainlining.
A few comments based on my experience implementing the USB-PD part for PinePhone
are bellow.
> > > +properties:
> > > + compatible:
> > > + const: analogix,anx7688
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + avdd33-supply:
> > > + description: 3.3V Analog core supply voltage.
> > > +
> > > + dvdd18-supply:
> > > + description: 1.8V Digital I/O supply voltage.
> > > +
> > > + avdd18-supply:
> > > + description: 1.8V Analog core power supply voltage.
> > > +
> > > + avdd10-supply:
> > > + description: 1.0V Analog core power supply voltage.
> > > +
> > > + dvdd10-supply:
> > > + description: 1.0V Digital core supply voltage.
> > > +
> >
> > That's lots of supplies. If there's a reasonable chance that some of
> > them will always be driven by the same regulator (especially if the
> > ANX7688 documentation requires that), then they could be grouped. For
> > instance dvdd18-supply and avdd18-supply could be grouped into
> > vdd18-supply. It would still allow us to extend the bindings in a
> > backward compatible way later if a system uses different regulators. You
> > have more information about the hardware than I do, so it's your call.
On PinePhone, AVDD and DVDD for 1.0V are just separately fitlered outputs
from the same regulator. So it would work there to just use vdd18 and
vdd10. The same is true for reference design, so it's probably safe
to assume this can be simplified.
> > > + hdmi5v-supply:
> > > + description: 5V power supply for the HDMI.
> > > +
> > > + hdmi_vt-supply:
> > > + description: Termination voltage for HDMI input.
> >
> > Maybe hdmi-vt-supply ?
> >
> > > +
> > > + clocks:
> > > + description: The input clock specifier.
> > > + maxItems: 1
> >
> > How about
> >
> > items:
> > - description: The input clock specifier.
> >
> > > +
> > > + clock-names:
> > > + items:
> > > + - const: xtal
> > > +
> > > + hpd-gpios:
> > > + description: |
> > > + In USB Type-C applications, DP_HPD has no use. In standard DisplayPort
> > > + applications, DP_HPD is used as DP hot-plug.
> > > + maxItems: 1
On PinePhone this is wired to a HDMI port on the SoC, and HPD is handled by the
sun4i HDMI DRM driver. Seems like HPD will be handled by HDMI controller on
many/all? other platforms too. Why have it here?
> > > + enable-gpios:
> > > + description: Chip power down control. No internal pull-down or pull-up resistor.
> > > + maxItems: 1
> > > +
> > > + reset-gpios:
> > > + description: Reset input signal. Active low.
> > > + maxItems: 1
> > > +
> > > + vbus-det-gpios:
> > > + description: |
> > > + An input gpio for VBUS detection and high voltage detection,
> > > + external resistance divide VBUS voltage to 1/8.
> > > + maxItems: 1
Why have this in the bindings? It seems that this is handled internally by the
ANX7688 via OCM firmware. And it's not really gpio either, it's an analog input
with AD converter hooked to it internally.
> > > + interrupts:
> > > + description: |
> > > + The interrupt notifies 4 possible events - TCPC ALERT int, PD int, DP int, HDMI int.
> > > + maxItems: 1
> > > +
> > > + cabledet-gpios:
> > > + description: An output gpio, indicates by the device that a cable is plugged.
> > > + maxItems: 1
> > > +
> > > + vbus-ctrl-gpios:
> > > + description:
> > > + External VBUS power path. Enable VBUS source and disable VBUS sink or vice versa.
> > > + maxItems: 1
VBUS control seems to be already modelled by the usb-connector bindings. Why
have this here?
> > > + vconn-en1-gpios:
> > > + description: Controls the VCONN switch on the CC1 pin.
> > > + maxItems: 1
> > > +
> > > + vconn-en2-gpios:
> > > + description: Controls the VCONN switch on the CC2 pin.
> > > + maxItems: 1
VCONN is a voltage regulator that can be enabled either on CC1 or CC2
pin, or disabled completely. This control is *partially* performed in reference
design directly by the OCM. OCM only decides which CC pin to switch
the VCONN regulator to, and informs the SoC when the base VCONN regulator
for the switches needs to be enabled.
So vconn-en1/2 gpios are irrelevant to the driver, but the driver needs
to control VCONN power supply somehow and defer control over it to the OCM.
I don't know how to express it in the bindings. Maybe a vconn-supply here
on the anx7688 node?
It may also be part of the usb-connector binding, but this is really when it
comes to anx7688 a parent regulator for the switches, and switches are not
controlled by this driver, just their parent regulator is.
Any ideas?
kind regards,
o.
> > > + ports:
> > > + $ref: /schemas/graph.yaml#/properties/ports
> > > +
> > > + properties:
> > > + port at 0:
> > > + $ref: /schemas/graph.yaml#/properties/port
> > > + description: Video port for HDMI input.
> > > +
> > > + port at 1:
> > > + $ref: /schemas/graph.yaml#/properties/port
> > > + description: USB port for the USB3 input.
> > > +
> > > + port at 2:
> > > + $ref: /schemas/graph.yaml#/properties/port
> > > + description: USB Type-c connector, see connector/usb-connector.yaml.
> > > +
> > > + required:
> > > + - port at 0
> >
> > As all the ports exist at the hardware level, should they always be
> > present ? The endpoints are optional of course, in case a port isn't
> > connected on a particular system.
> >
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> >
> > Shouldn't clocks and regulators be also required ?
>
> hmm, theoretically yes. Practically, in the case of Acer R13 (mediatek elm) device,
> the ANX7688 is powered up and controlled by the Embedded Controller. The kernel only
> needs to read few registers from that device for the drm bridge driver.
> (also mentioned that in the cover letter).
>
> Thanks,
> Dafna
More information about the dri-devel
mailing list