Re: [PATCH v5 1/2] dt-bindings: usb: add analogix,anx7688.yaml

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxx>
>>>>> ---
>>>>>    .../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@xxxxxxxxxxxx>
>>>>> +  - Enric Balletbo i Serra <enric.balletbo@xxxxxxxxxxxxx>
>>>>> +
>>>>> +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@xxxxxxxxxxxxxxxx>
>>>>
>>>>> +
>>>>> +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 = <&reg_dcdc1>;
>>>>> +            dvdd18-supply = <&reg_ldo_io1>;
>>>>> +            avdd18-supply = <&reg_ldo_io1>;
>>>>> +            avdd10-supply = <&reg_anx1v0>;
>>>>> +            dvdd10-supply = <&reg_anx1v0>;
>>>>> +            hdmi_vt-supply = <&reg_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>;
>>>>> +                    };
>>>>> +                };
>>>>> +            };
>>>>> +        };
>>>>> +    };
>>



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux