| Message ID | 20210305124351.15079-2-dafna.hirschfeld@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for ANX7688 | expand |
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. > > Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@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@chromium.org> > + - Enric Balletbo i Serra <enric.balletbo@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. > + 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@0: > + $ref: /schemas/graph.yaml#/properties/port > + description: Video port for HDMI input. > + > + port@1: > + $ref: /schemas/graph.yaml#/properties/port > + description: USB port for the USB3 input. > + > + port@2: > + $ref: /schemas/graph.yaml#/properties/port > + description: USB Type-c connector, see connector/usb-connector.yaml. > + > + required: > + - port@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 ? Reviewed-by: Laurent Pinchart <laurent.pinchart@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@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@0 { > + reg = <0>; > + anx7688_in0: endpoint { > + remote-endpoint = <&hdmi0_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + anx7688_in1: endpoint { > + remote-endpoint = <&usbdrd_phy_ss>; > + }; > + }; > + port@2 { > + reg = <2>; > + anx7688_out: endpoint { > + remote-endpoint = <&typec_connector>; > + }; > + }; > + }; > + }; > + };
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. >> >> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@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@chromium.org> >> + - Enric Balletbo i Serra <enric.balletbo@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. > >> + 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@0: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: Video port for HDMI input. >> + >> + port@1: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: USB port for the USB3 input. >> + >> + port@2: >> + $ref: /schemas/graph.yaml#/properties/port >> + description: USB Type-c connector, see connector/usb-connector.yaml. >> + >> + required: >> + - port@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 > > Reviewed-by: Laurent Pinchart <laurent.pinchart@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@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@0 { >> + reg = <0>; >> + anx7688_in0: endpoint { >> + remote-endpoint = <&hdmi0_out>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + anx7688_in1: endpoint { >> + remote-endpoint = <&usbdrd_phy_ss>; >> + }; >> + }; >> + port@2 { >> + reg = <2>; >> + anx7688_out: endpoint { >> + remote-endpoint = <&typec_connector>; >> + }; >> + }; >> + }; >> + }; >> + }; >
Hi, On 05.03.21 16:19, Laurent Pinchart wrote: > Hi Dafna, > > 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@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@chromium.org> >>>> + - Enric Balletbo i Serra <enric.balletbo@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? >>> >>>> + 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@0: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: Video port for HDMI input. >>>> + >>>> + port@1: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: USB port for the USB3 input. >>>> + >>>> + port@2: >>>> + $ref: /schemas/graph.yaml#/properties/port >>>> + description: USB Type-c connector, see connector/usb-connector.yaml. >>>> + >>>> + required: >>>> + - port@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). > > 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 :-) 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 ? 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). Thanks, Dafna > >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@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@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@0 { >>>> + reg = <0>; >>>> + anx7688_in0: endpoint { >>>> + remote-endpoint = <&hdmi0_out>; >>>> + }; >>>> + }; >>>> + >>>> + port@1 { >>>> + reg = <1>; >>>> + anx7688_in1: endpoint { >>>> + remote-endpoint = <&usbdrd_phy_ss>; >>>> + }; >>>> + }; >>>> + port@2 { >>>> + reg = <2>; >>>> + anx7688_out: endpoint { >>>> + remote-endpoint = <&typec_connector>; >>>> + }; >>>> + }; >>>> + }; >>>> + }; >>>> + }; >
Hi Dafna, Many thanks to take this challenge and work on this. On 30/3/21 15:35, Dafna Hirschfeld wrote: > Hi, > > On 05.03.21 16:19, Laurent Pinchart wrote: >> Hi Dafna, >> >> 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@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@chromium.org> >>>>> + - Enric Balletbo i Serra <enric.balletbo@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. >>>> >>>>> + 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@0: >>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>> + description: Video port for HDMI input. >>>>> + >>>>> + port@1: >>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>> + description: USB port for the USB3 input. >>>>> + >>>>> + port@2: >>>>> + $ref: /schemas/graph.yaml#/properties/port >>>>> + description: USB Type-c connector, see connector/usb-connector.yaml. >>>>> + >>>>> + required: >>>>> + - port@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 ... > 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. > 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). > > > Thanks, > Dafna > >> >>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@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@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@0 { >>>>> + reg = <0>; >>>>> + anx7688_in0: endpoint { >>>>> + remote-endpoint = <&hdmi0_out>; >>>>> + }; >>>>> + }; >>>>> + >>>>> + port@1 { >>>>> + reg = <1>; >>>>> + anx7688_in1: endpoint { >>>>> + remote-endpoint = <&usbdrd_phy_ss>; >>>>> + }; >>>>> + }; >>>>> + port@2 { >>>>> + reg = <2>; >>>>> + anx7688_out: endpoint { >>>>> + remote-endpoint = <&typec_connector>; >>>>> + }; >>>>> + }; >>>>> + }; >>>>> + }; >>>>> + }; >>
On Wed, Mar 31, 2021 at 07:16:40PM +0200, Dafna Hirschfeld wrote: > Hi, > > On 05.03.21 18:24, Ondřej Jirman wrote: > > Hello Dafna, > > [...] > > > > > + 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? > > Looking at the diagram in the schematics, I see that both vbus-supply and vconn-supply > come directly from the PMIC so similarly to the vbus-supply, the vconn-supply > can be part of the usb-connector. Then a driver can access the vconn-supply from the remote usb > connector. Is there a problem with that? No problem with that. usb-c-connector binding would just have to be extended. I just don't see any need for these `vconn-en*-gpios`, because the control is performed directly by the OCM firmware via GPIOs in the ANX7688 chip, outside of the control of the Linux driver, and the driver doesn't even care about the status of these pins. And if it will ever care, the status can be read via I2C from the ANX7688 chip directly. kind regards, o. > Thanks a lot for the review, I am not very familiar with usb and type-c, so your review helps a lot. > I will send v6 soon. > > Thanks, > Dafna
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@chromium.org> + - Enric Balletbo i Serra <enric.balletbo@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. + + +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. + + hdmi5v-supply: + description: 5V power supply for the HDMI. + + hdmi_vt-supply: + description: Termination voltage for HDMI input. + + clocks: + description: The input clock specifier. + maxItems: 1 + + 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@0: + $ref: /schemas/graph.yaml#/properties/port + description: Video port for HDMI input. + + port@1: + $ref: /schemas/graph.yaml#/properties/port + description: USB port for the USB3 input. + + port@2: + $ref: /schemas/graph.yaml#/properties/port + description: USB Type-c connector, see connector/usb-connector.yaml. + + required: + - port@0 + +required: + - compatible + - reg + +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@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@0 { + reg = <0>; + anx7688_in0: endpoint { + remote-endpoint = <&hdmi0_out>; + }; + }; + + port@1 { + reg = <1>; + anx7688_in1: endpoint { + remote-endpoint = <&usbdrd_phy_ss>; + }; + }; + port@2 { + reg = <2>; + anx7688_out: endpoint { + remote-endpoint = <&typec_connector>; + }; + }; + }; + }; + };
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@collabora.com> --- .../bindings/usb/analogix,anx7688.yaml | 177 ++++++++++++++++++ 1 file changed, 177 insertions(+) create mode 100644 Documentation/devicetree/bindings/usb/analogix,anx7688.yaml