[PATCH v5 1/2] dt-bindings: usb: add analogix,anx7688.yaml
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Mar 31 20:40:46 UTC 2021
On Tue, Mar 30, 2021 at 05:14:44PM +0200, Enric Balletbo i Serra wrote:
> On 30/3/21 15:35, Dafna Hirschfeld wrote:
> > On 05.03.21 16:19, Laurent Pinchart wrote:
> >> On Fri, Mar 05, 2021 at 04:14:03PM +0100, Dafna Hirschfeld wrote:
> >>> On 05.03.21 15:34, Laurent Pinchart wrote:
> >>>> 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.
> >>>>>
> >>>>> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld at collabora.com>
> >>>>> ---
> >>>>> .../bindings/usb/analogix,anx7688.yaml | 177 ++++++++++++++++++
> >>>>> 1 file changed, 177 insertions(+)
> >>>>> create mode 100644
> >>>>> Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> >>>>> b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> >>>>> new file mode 100644
> >>>>> index 000000000000..6c4dd6b4b28b
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> >>>>> @@ -0,0 +1,177 @@
> >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>>> +%YAML 1.2
> >>>>> +---
> >>>>> +$id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml#
> >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>> +
> >>>>> +title: Analogix ANX7688 Type-C Port Controller with HDMI to DP conversion
> >>>>> +
> >>>>> +maintainers:
> >>>>> + - Nicolas Boichat <drinkcat at chromium.org>
> >>>>> + - Enric Balletbo i Serra <enric.balletbo at collabora.com>
> >>>>> +
> >>>>> +description: |
> >>>>> + 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.
> >>>>> +
> >>>>> +
> >>>>
> >>>> Extra blank line ?
> >>>>
> >>>>> +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.
> >
> > Can you explain what do you mean by 'grouped' ?
> > Do you mean that instead of having two properties dvdd18-supply and avdd18-supply
> > I have only one property vdd18-supply?
>
> You can simplify all this with vdd33, vdd18 vdd10. For the Chromebook case all
> the analogic and digital part are the same regulator just filtered. That's a
> common configuration and if there is some hardware that needs it we can extend
> later.
That's the idea, yes. If in a typical use case multiple supplies are
provided by a single regulator (for some devices that datasheet strongly
recommends that, or event mandates it), then it makes sense to group
those supplies in a single DT supply property. It can always be extended
later indeed, without any backward compatibility issue.
> >>>>> + 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
> >>>>> +
> >>>>> + 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
> >>>>> +
> >>>>> + 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
> >>>>> +
> >>>>> + 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
> >>>>> +
> >>>>> + 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).
>
> I think that for the Chromebook you can assume that these supplies are a
> fixed-regulator that are always on.
>
> >> This may not be a popular opinion, but if the ANX7688 is fully
> >> controlled by the EC, I wonder if we shouldn't have an "EC DRM bridge"
> >> driver that would interrogate the EC instead :-)
>
> We can do this, and tbh will be more easy for us, but we were already asked to
> do it generic, so ...
It's hardly generic if the "generic" driver assumes that there's an EC
controlling the device, isn't it ?
> > But the driver in patch 2/2 do access the anx7688 device with I2C.
> > It does interrogate the EC.
> >
> >> Is there a risk of bus conflicts if the EC and the main SoC try to
> >> access the ANX7688 over I2C at the same time ?
>
> No, there is a kind of i2c tunnel but you don't talk directly with the ANX7688.
> The EC exposes the anx bus control to the AP.
Maybe an additional reason to talk to the EC directly instead ? Won't
upstreaming an ANX7688 driver that hardcodes assumptions about an EC
being present cause issues in the future, when someone will want to
extend the driver for a standalone ANX7688 ? The driver will start
programming the ANX7688, and the EC won't like it. We would have to add
a "this is a real ANX7688" DT property later, which would really not be
nice.
> > The SoC access the anx7688 though something called 'i2c-tunnel' (see
> > google,cros-ec-i2c-tunnel.yaml)
> > So the I2C tunnels though the EC (I don't know how this is really implemented to
> > be honest).
> >
> >>>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >>>>
> >>>>> +
> >>>>> +additionalProperties: false
> >>>>> +
> >>>>> +examples:
> >>>>> + - |
> >>>>> + #include <dt-bindings/gpio/gpio.h>
> >>>>> + #include <dt-bindings/interrupt-controller/irq.h>
> >>>>> +
> >>>>> + i2c0 {
> >>>>> + #address-cells = <1>;
> >>>>> + #size-cells = <0>;
> >>>>> +
> >>>>> + anx7688: anx7688 at 2c {
> >>>>> + compatible = "analogix,anx7688";
> >>>>> + reg = <0x2c>;
> >>>>> + avdd33-supply = <®_dcdc1>;
> >>>>> + dvdd18-supply = <®_ldo_io1>;
> >>>>> + avdd18-supply = <®_ldo_io1>;
> >>>>> + avdd10-supply = <®_anx1v0>;
> >>>>> + dvdd10-supply = <®_anx1v0>;
> >>>>> + hdmi_vt-supply = <®_dldo1>;
> >>>>> + enable-gpios = <&pio 3 10 GPIO_ACTIVE_LOW>; /* PD10 */
> >>>>> + reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
> >>>>> + interrupt-parent = <&r_pio>;
> >>>>> + interrupts = <0 11 IRQ_TYPE_EDGE_FALLING>; /* PL11 */
> >>>>> + cabledet-gpios = <&r_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
> >>>>> + vconn-en1-gpios = <&pio 3 9 GPIO_ACTIVE_LOW>; /* PD9 */
> >>>>> + vconn-en2-gpios = <&pio 3 9 GPIO_ACTIVE_LOW>; /* PD9 */
> >>>>> + ports {
> >>>>> + #address-cells = <1>;
> >>>>> + #size-cells = <0>;
> >>>>> +
> >>>>> + port at 0 {
> >>>>> + reg = <0>;
> >>>>> + anx7688_in0: endpoint {
> >>>>> + remote-endpoint = <&hdmi0_out>;
> >>>>> + };
> >>>>> + };
> >>>>> +
> >>>>> + port at 1 {
> >>>>> + reg = <1>;
> >>>>> + anx7688_in1: endpoint {
> >>>>> + remote-endpoint = <&usbdrd_phy_ss>;
> >>>>> + };
> >>>>> + };
> >>>>> + port at 2 {
> >>>>> + reg = <2>;
> >>>>> + anx7688_out: endpoint {
> >>>>> + remote-endpoint = <&typec_connector>;
> >>>>> + };
> >>>>> + };
> >>>>> + };
> >>>>> + };
> >>>>> + };
--
Regards,
Laurent Pinchart
More information about the dri-devel
mailing list