2024-02-09 19:52:17

by Pavel Machek

[permalink] [raw]
Subject: [PATCH] dt-bindings: usb: typec: anx7688: start a binding document

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 <[email protected]>

diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
new file mode 100644
index 000000000000..b9d60586937f
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
@@ -0,0 +1,140 @@
+# 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 controller
+
+maintainers:
+ - Pavel Machek <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - analogix,anx7688
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+ enable-gpios:
+ maxItems: 1
+ cabledet-gpios:
+ maxItems: 1
+
+ avdd10-supply:
+ description:
+ 1.0V power supply
+ dvdd10-supply:
+ description:
+ 1.0V power supply
+ avdd18-supply:
+ description:
+ 1.8V power supply
+ dvdd18-supply:
+ description:
+ 1.8V power supply
+ avdd33-supply:
+ description:
+ 3.3V power supply
+ i2c-supply:
+ description:
+ Power supply
+ vconn-supply:
+ description:
+ Power supply
+ hdmi_vt-supply:
+ description:
+ Power supply
+
+ vbus-supply:
+ description:
+ Power supply
+ vbus_in-supply:
+ description:
+ Power supply
+
+ connector:
+ type: object
+ $ref: ../connector/usb-connector.yaml
+ unevaluatedProperties: false
+
+ description:
+ Properties for usb c connector.
+
+ properties:
+ compatible:
+ const: usb-c-connector
+
+ power-role: true
+
+ data-role: true
+
+ try-power-role: true
+
+ required:
+ - compatible
+
+required:
+ - compatible
+ - reg
+ - connector
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ typec@2c {
+ compatible = "analogix,anx7688";
+ reg = <0x2c>;
+ interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-parent = <&gpio0>;
+
+ enable-gpios = <&pio 3 10 GPIO_ACTIVE_LOW>; /* PD10 */
+ reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
+ cabledet-gpios = <&r_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
+
+ avdd10-supply = <&reg_anx1v0>;
+ dvdd10-supply = <&reg_anx1v0>;
+ avdd18-supply = <&reg_ldo_io1>;
+ dvdd18-supply = <&reg_ldo_io1>;
+ avdd33-supply = <&reg_dcdc1>;
+ i2c-supply = <&reg_ldo_io0>;
+ vconn-supply = <&reg_vconn5v0>;
+ hdmi_vt-supply = <&reg_dldo1>;
+
+ vbus-supply = <&reg_usb_5v>;
+ vbus_in-supply = <&usb_power_supply>;
+
+ typec_con: connector {
+ compatible = "usb-c-connector";
+ power-role = "dual";
+ data-role = "dual";
+ try-power-role = "source";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ typec_con_ep: endpoint {
+ remote-endpoint = <&usbotg_hs_ep>;
+ };
+ };
+ };
+ };
+ };
+ };
+...
diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
index 594ebb3ee432..6ceafa4af292 100644
--- a/Documentation/devicetree/bindings/usb/generic-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
@@ -9,9 +9,6 @@ title: USB xHCI Controller
maintainers:
- Mathias Nyman <[email protected]>

-allOf:
- - $ref: usb-xhci.yaml#
-
properties:
compatible:
oneOf:
@@ -25,6 +22,11 @@ properties:
- marvell,armada-380-xhci
- marvell,armada-8k-xhci
- const: generic-xhci
+ - description: Broadcom SoCs with power domains
+ items:
+ - enum:
+ - brcm,bcm2711-xhci
+ - const: brcm,xhci-brcm-v2
- description: Broadcom STB SoCs with xHCI
enum:
- brcm,xhci-brcm-v2
@@ -49,6 +51,9 @@ properties:
- const: core
- const: reg

+ power-domains:
+ maxItems: 1
+
unevaluatedProperties: false

required:
@@ -56,6 +61,20 @@ required:
- reg
- interrupts

+allOf:
+ - $ref: usb-xhci.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: brcm,bcm2711-xhci
+ then:
+ required:
+ - power-domains
+ else:
+ properties:
+ power-domains: false
+
examples:
- |
usb@f0931000 {
diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
index ee08b9c3721f..37cf5249e526 100644
--- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
+++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml
@@ -29,6 +29,11 @@ properties:
description:
the regulator that provides 3.3V core power to the hub.

+ peer-hub:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ phandle to the peer hub on the controller.
+
required:
- compatible
- reg
diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
index e9644e333d78..924fd3d748a8 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
@@ -124,6 +124,17 @@ properties:
defined in the xHCI spec on MTK's controller.
default: 5000

+ rx-fifo-depth:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ It is a quirk used to work around Gen1 isoc-in endpoint transfer issue
+ that still send out unexpected ACK after device finishes the burst
+ transfer with a short packet and cause an exception, specially on a 4K
+ camera device, it happens on controller before about IPM v1.6.0;
+ the side-effect is that it may cause performance drop about 10%,
+ including bulk transfer, prefer to use 3k here. The size is in bytes.
+ enum: [1024, 2048, 3072, 4096]
+
# the following properties are only used for case 1
wakeup-source:
description: enable USB remote wakeup, see power/wakeup-source.txt
diff --git a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml
index 28eb25ecba74..eaedb4cc6b6c 100644
--- a/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml
+++ b/Documentation/devicetree/bindings/usb/nxp,ptn5110.yaml
@@ -4,7 +4,7 @@
$id: http://devicetree.org/schemas/usb/nxp,ptn5110.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#

-title: NXP PTN5110 Typec Port Cotroller
+title: NXP PTN5110 Type-C Port Controller

maintainers:
- Li Jun <[email protected]>
diff --git a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
index 915c8205623b..63d150b216c5 100644
--- a/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,dwc3.yaml
@@ -46,6 +46,8 @@ properties:
- qcom,sm8350-dwc3
- qcom,sm8450-dwc3
- qcom,sm8550-dwc3
+ - qcom,sm8650-dwc3
+ - qcom,x1e80100-dwc3
- const: qcom,dwc3

reg:
@@ -97,12 +99,29 @@ properties:
- const: apps-usb

interrupts:
- minItems: 1
- maxItems: 4
+ description: |
+ Different types of interrupts are used based on HS PHY used on target:
+ - pwr_event: Used for wakeup based on other power events.
+ - hs_phY_irq: Apart from DP/DM/QUSB2 PHY interrupts, there is
+ hs_phy_irq which is not triggered by default and its
+ functionality is mutually exclusive to that of
+ {dp/dm}_hs_phy_irq and qusb2_phy_irq.
+ - qusb2_phy: SoCs with QUSB2 PHY do not have separate DP/DM IRQs and
+ expose only a single IRQ whose behavior can be modified
+ by the QUSB2PHY_INTR_CTRL register. The required DPSE/
+ DMSE configuration is done in QUSB2PHY_INTR_CTRL register
+ of PHY address space.
+ - {dp/dm}_hs_phy_irq: These IRQ's directly reflect changes on the DP/
+ DM pads of the SoC. These are used for wakeup
+ only on SoCs with non-QUSB2 targets with
+ exception of SDM670/SDM845/SM6350.
+ - ss_phy_irq: Used for remote wakeup in Super Speed mode of operation.
+ minItems: 2
+ maxItems: 5

interrupt-names:
- minItems: 1
- maxItems: 4
+ minItems: 2
+ maxItems: 5

qcom,select-utmi-as-pipe-clk:
description:
@@ -263,6 +282,7 @@ allOf:
contains:
enum:
- qcom,sc8280xp-dwc3
+ - qcom,x1e80100-dwc3
then:
properties:
clocks:
@@ -288,8 +308,8 @@ allOf:
then:
properties:
clocks:
- minItems: 5
- maxItems: 6
+ minItems: 4
+ maxItems: 5
clock-names:
oneOf:
- items:
@@ -298,13 +318,11 @@ allOf:
- const: iface
- const: sleep
- const: mock_utmi
- - const: bus
- items:
- const: cfg_noc
- const: core
- const: sleep
- const: mock_utmi
- - const: bus

- if:
properties:
@@ -318,6 +336,7 @@ allOf:
- qcom,sm8250-dwc3
- qcom,sm8450-dwc3
- qcom,sm8550-dwc3
+ - qcom,sm8650-dwc3
then:
properties:
clocks:
@@ -357,131 +376,96 @@ allOf:
compatible:
contains:
enum:
- - qcom,ipq4019-dwc3
+ - qcom,ipq5018-dwc3
- qcom,ipq6018-dwc3
- - qcom,ipq8064-dwc3
- qcom,ipq8074-dwc3
- - qcom,msm8994-dwc3
+ - qcom,msm8953-dwc3
+ - qcom,msm8998-dwc3
+ then:
+ properties:
+ interrupts:
+ minItems: 2
+ maxItems: 3
+ interrupt-names:
+ items:
+ - const: pwr_event
+ - const: qusb2_phy
+ - const: ss_phy_irq
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,msm8996-dwc3
- qcom,qcs404-dwc3
+ - qcom,sdm660-dwc3
+ - qcom,sm6115-dwc3
+ - qcom,sm6125-dwc3
+ then:
+ properties:
+ interrupts:
+ minItems: 3
+ maxItems: 4
+ interrupt-names:
+ items:
+ - const: pwr_event
+ - const: qusb2_phy
+ - const: hs_phy_irq
+ - const: ss_phy_irq
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,ipq5332-dwc3
+ - qcom,x1e80100-dwc3
+ then:
+ properties:
+ interrupts:
+ maxItems: 4
+ interrupt-names:
+ items:
+ - const: pwr_event
+ - const: dp_hs_phy_irq
+ - const: dm_hs_phy_irq
+ - const: ss_phy_irq
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,ipq4019-dwc3
+ - qcom,ipq8064-dwc3
+ - qcom,msm8994-dwc3
+ - qcom,sa8775p-dwc3
- qcom,sc7180-dwc3
+ - qcom,sc7280-dwc3
+ - qcom,sc8280xp-dwc3
- qcom,sdm670-dwc3
- qcom,sdm845-dwc3
- qcom,sdx55-dwc3
- qcom,sdx65-dwc3
- qcom,sdx75-dwc3
- qcom,sm4250-dwc3
- - qcom,sm6125-dwc3
- qcom,sm6350-dwc3
- qcom,sm8150-dwc3
- qcom,sm8250-dwc3
- qcom,sm8350-dwc3
- qcom,sm8450-dwc3
- qcom,sm8550-dwc3
+ - qcom,sm8650-dwc3
then:
properties:
interrupts:
- items:
- - description: The interrupt that is asserted
- when a wakeup event is received on USB2 bus.
- - description: The interrupt that is asserted
- when a wakeup event is received on USB3 bus.
- - description: Wakeup event on DM line.
- - description: Wakeup event on DP line.
- interrupt-names:
- items:
- - const: hs_phy_irq
- - const: ss_phy_irq
- - const: dm_hs_phy_irq
- - const: dp_hs_phy_irq
-
- - if:
- properties:
- compatible:
- contains:
- enum:
- - qcom,msm8953-dwc3
- - qcom,msm8996-dwc3
- - qcom,msm8998-dwc3
- - qcom,sm6115-dwc3
- then:
- properties:
- interrupts:
- maxItems: 2
- interrupt-names:
- items:
- - const: hs_phy_irq
- - const: ss_phy_irq
-
- - if:
- properties:
- compatible:
- contains:
- enum:
- - qcom,ipq5018-dwc3
- - qcom,ipq5332-dwc3
- - qcom,sdm660-dwc3
- then:
- properties:
- interrupts:
- minItems: 1
- maxItems: 2
- interrupt-names:
- minItems: 1
- items:
- - const: hs_phy_irq
- - const: ss_phy_irq
-
- - if:
- properties:
- compatible:
- contains:
- enum:
- - qcom,sc7280-dwc3
- then:
- properties:
- interrupts:
- minItems: 3
- maxItems: 4
- interrupt-names:
- minItems: 3
- items:
- - const: hs_phy_irq
- - const: dp_hs_phy_irq
- - const: dm_hs_phy_irq
- - const: ss_phy_irq
-
- - if:
- properties:
- compatible:
- contains:
- enum:
- - qcom,sc8280xp-dwc3
- then:
- properties:
- interrupts:
- maxItems: 4
+ minItems: 4
+ maxItems: 5
interrupt-names:
items:
- const: pwr_event
- - const: dp_hs_phy_irq
- - const: dm_hs_phy_irq
- - const: ss_phy_irq
-
- - if:
- properties:
- compatible:
- contains:
- enum:
- - qcom,sa8775p-dwc3
- then:
- properties:
- interrupts:
- minItems: 3
- maxItems: 4
- interrupt-names:
- minItems: 3
- items:
- - const: pwr_event
+ - const: hs_phy_irq
- const: dp_hs_phy_irq
- const: dm_hs_phy_irq
- const: ss_phy_irq
@@ -519,12 +503,13 @@ examples:
<&gcc GCC_USB30_PRIM_MASTER_CLK>;
assigned-clock-rates = <19200000>, <150000000>;

- interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
- <GIC_SPI 486 IRQ_TYPE_LEVEL_HIGH>,
+ interrupts = <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 489 IRQ_TYPE_EDGE_BOTH>,
<GIC_SPI 488 IRQ_TYPE_EDGE_BOTH>,
- <GIC_SPI 489 IRQ_TYPE_EDGE_BOTH>;
- interrupt-names = "hs_phy_irq", "ss_phy_irq",
- "dm_hs_phy_irq", "dp_hs_phy_irq";
+ <GIC_SPI 486 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "pwr_event", "hs_phy_irq",
+ "dp_hs_phy_irq", "dm_hs_phy_irq", "ss_phy_irq";

power-domains = <&gcc USB30_PRIM_GDSC>;

diff --git a/Documentation/devicetree/bindings/usb/qcom,wcd939x-usbss.yaml b/Documentation/devicetree/bindings/usb/qcom,wcd939x-usbss.yaml
new file mode 100644
index 000000000000..7ddfd3313a18
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/qcom,wcd939x-usbss.yaml
@@ -0,0 +1,102 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/qcom,wcd939x-usbss.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm WCD9380/WCD9385 USB SubSystem Altmode/Analog Audio Switch
+
+maintainers:
+ - Neil Armstrong <[email protected]>
+
+description:
+ Qualcomm WCD9390/WCD9395 is a standalone Hi-Fi audio codec IC with a
+ functionally separate USB SubSystem for Altmode/Analog Audio Switch
+ accessible over an I2C interface.
+ The Audio Headphone and Microphone data path between the Codec and the
+ USB-C Mux subsystems are external to the IC, thus requiring DT port-endpoint
+ graph description to handle USB-C altmode & orientation switching for Audio
+ Accessory Mode.
+
+properties:
+ compatible:
+ oneOf:
+ - const: qcom,wcd9390-usbss
+ - items:
+ - const: qcom,wcd9395-usbss
+ - const: qcom,wcd9390-usbss
+
+ reg:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ vdd-supply:
+ description: USBSS VDD power supply
+
+ mode-switch:
+ description: Flag the port as possible handle of altmode switching
+ type: boolean
+
+ orientation-switch:
+ description: Flag the port as possible handler of orientation switching
+ type: boolean
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/properties/port
+ description:
+ A port node to link the WCD939x USB SubSystem to a TypeC controller for the
+ purpose of handling altmode muxing and orientation switching.
+
+ port@1:
+ $ref: /schemas/graph.yaml#/properties/port
+ description:
+ A port node to link the WCD939x USB SubSystem to the Codec SubSystem for the
+ purpose of handling USB-C Audio Accessory Mode muxing and orientation switching.
+
+required:
+ - compatible
+ - reg
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ typec-mux@42 {
+ compatible = "qcom,wcd9390-usbss";
+ reg = <0x42>;
+
+ vdd-supply = <&vreg_bob>;
+
+ mode-switch;
+ orientation-switch;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ wcd9390_usbss_sbu: endpoint {
+ remote-endpoint = <&typec_sbu>;
+ };
+ };
+ port@1 {
+ reg = <1>;
+ wcd9390_usbss_codec: endpoint {
+ remote-endpoint = <&wcd9390_codec_usbss>;
+ };
+ };
+ };
+ };
+ };
+...
diff --git a/Documentation/devicetree/bindings/usb/renesas,usbhs.yaml b/Documentation/devicetree/bindings/usb/renesas,usbhs.yaml
index bad55dfb2fa0..40ada78f2328 100644
--- a/Documentation/devicetree/bindings/usb/renesas,usbhs.yaml
+++ b/Documentation/devicetree/bindings/usb/renesas,usbhs.yaml
@@ -19,7 +19,7 @@ properties:
- items:
- enum:
- renesas,usbhs-r7s9210 # RZ/A2
- - renesas,usbhs-r9a07g043 # RZ/G2UL
+ - renesas,usbhs-r9a07g043 # RZ/G2UL and RZ/Five
- renesas,usbhs-r9a07g044 # RZ/G2{L,LC}
- renesas,usbhs-r9a07g054 # RZ/V2L
- const: renesas,rza2-usbhs
diff --git a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
index ee5af4b381b1..203a1eb66691 100644
--- a/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
+++ b/Documentation/devicetree/bindings/usb/snps,dwc3.yaml
@@ -432,6 +432,10 @@ properties:
items:
enum: [1, 4, 8, 16, 32, 64, 128, 256]

+ num-hc-interrupters:
+ maximum: 8
+ default: 1
+
port:
$ref: /schemas/graph.yaml#/properties/port
description:
diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
index 323d664ae06a..1745e28b3110 100644
--- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
+++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
@@ -38,6 +38,10 @@ properties:
- const: main
- const: patch-address

+ reset-gpios:
+ description: GPIO used for the HRESET pin.
+ maxItems: 1
+
wakeup-source: true

interrupts:
@@ -90,6 +94,7 @@ additionalProperties: false

examples:
- |
+ #include <dt-bindings/gpio/gpio.h>
#include <dt-bindings/interrupt-controller/irq.h>
i2c {
#address-cells = <1>;
@@ -106,6 +111,7 @@ examples:

pinctrl-names = "default";
pinctrl-0 = <&typec_pins>;
+ reset-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;

typec_con: connector {
compatible = "usb-c-connector";
diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.yaml b/Documentation/devicetree/bindings/usb/usb-xhci.yaml
index 180a261c3e8f..4238ae896ef6 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.yaml
@@ -29,6 +29,12 @@ properties:
description: Interrupt moderation interval
default: 5000

+ num-hc-interrupters:
+ description: Maximum number of interrupters to allocate
+ $ref: /schemas/types.yaml#/definitions/uint16
+ minimum: 1
+ maximum: 1024
+
additionalProperties: true

examples:

--
People of Russia, stop Putin before his war on Ukraine escalates.


Attachments:
(No filename) (22.44 kB)
signature.asc (201.00 B)
Download all attachments

2024-02-11 13:56:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: usb: typec: anx7688: start a binding document

On 09/02/2024 20: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 <[email protected]>
>

You miss proper diffstat which makes reviewing difficult.

Actually entire patch is corrupted and impossible to apply.

Anyway, where is any user of this? Nothing in commit msg explains this.


> diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> new file mode 100644
> index 000000000000..b9d60586937f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> @@ -0,0 +1,140 @@
> +# 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 controller
> +
> +maintainers:
> + - Pavel Machek <[email protected]>
> +
> +properties:
> + compatible:
> + enum:
> + - analogix,anx7688
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1

blank line

> + enable-gpios:
> + maxItems: 1
> + cabledet-gpios:
> + maxItems: 1
> +
> + avdd10-supply:
> + description:
> + 1.0V power supply

Keep description in one line and add a blank line. This code is not that
readable.

> + dvdd10-supply:
> + description:
> + 1.0V power supply
> + avdd18-supply:
> + description:
> + 1.8V power supply
> + dvdd18-supply:
> + description:
> + 1.8V power supply
> + avdd33-supply:
> + description:
> + 3.3V power supply
> + i2c-supply:
> + description:
> + Power supply

Drop all useless descriptions (so : true)

> + vconn-supply:
> + description:
> + Power supply
> + hdmi_vt-supply:
> + description:
> + Power supply
> +
> + vbus-supply:
> + description:
> + Power supply
> + vbus_in-supply:

No underscores in property names.

> + description:
> + Power supply
> +
> + connector:
> + type: object
> + $ref: ../connector/usb-connector.yaml

Full path, so /schemas/connector/......

> + unevaluatedProperties: false

Drop

> +
> + description:
> + Properties for usb c connector.

> +
> + properties:
> + compatible:
> + const: usb-c-connector
> +
> + power-role: true
> +
> + data-role: true
> +
> + try-power-role: true

I don't understand why do you have here properties. Do you see any
binding like this?

> +
> + required:
> + - compatible

Drop, why is it needed?

> +
> +required:
> + - compatible
> + - reg
> + - connector
> +
> +additionalProperti

I don't know what's further but this patch is not a patch... Please read
submitting-patches, organize your patchset correctly into manageable
logical chunks and send them as proper patchset, not one junk.

b4 helps here a lot...

>

Best regards,
Krzysztof


2024-02-12 11:02:39

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: usb: typec: anx7688: start a binding document

Hi!
> > 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 <[email protected]>
> >
>
> You miss proper diffstat which makes reviewing difficult.

> Actually entire patch is corrupted and impossible to apply.

Sorry about that.

> Anyway, where is any user of this? Nothing in commit msg explains
> this.

User being is worked on:

https://lore.kernel.org/lkml/2024020126-emote-unsubtly-3394@gregkh/T/

Thanks for comments. I'll go through them and try to improve things.

Best regards,
Pavel

>
> > diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> > new file mode 100644
> > index 000000000000..b9d60586937f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> > @@ -0,0 +1,140 @@
> > +# 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 controller
> > +
> > +maintainers:
> > + - Pavel Machek <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - analogix,anx7688
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + reset-gpios:
> > + maxItems: 1
>
> blank line
>
> > + enable-gpios:
> > + maxItems: 1
> > + cabledet-gpios:
> > + maxItems: 1
> > +
> > + avdd10-supply:
> > + description:
> > + 1.0V power supply
>
> Keep description in one line and add a blank line. This code is not that
> readable.
>
> > + dvdd10-supply:
> > + description:
> > + 1.0V power supply
> > + avdd18-supply:
> > + description:
> > + 1.8V power supply
> > + dvdd18-supply:
> > + description:
> > + 1.8V power supply
> > + avdd33-supply:
> > + description:
> > + 3.3V power supply
> > + i2c-supply:
> > + description:
> > + Power supply
>
> Drop all useless descriptions (so : true)
>
> > + vconn-supply:
> > + description:
> > + Power supply
> > + hdmi_vt-supply:
> > + description:
> > + Power supply
> > +
> > + vbus-supply:
> > + description:
> > + Power supply
> > + vbus_in-supply:
>
> No underscores in property names.
>
> > + description:
> > + Power supply
> > +
> > + connector:
> > + type: object
> > + $ref: ../connector/usb-connector.yaml
>
> Full path, so /schemas/connector/......
>
> > + unevaluatedProperties: false
>
> Drop
>
> > +
> > + description:
> > + Properties for usb c connector.
>
> > +
> > + properties:
> > + compatible:
> > + const: usb-c-connector
> > +
> > + power-role: true
> > +
> > + data-role: true
> > +
> > + try-power-role: true
>
> I don't understand why do you have here properties. Do you see any
> binding like this?
>
> > +
> > + required:
> > + - compatible
>
> Drop, why is it needed?
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - connector
> > +
> > +additionalProperti
>
> I don't know what's further but this patch is not a patch... Please read
> submitting-patches, organize your patchset correctly into manageable
> logical chunks and send them as proper patchset, not one junk.
>
> b4 helps here a lot...
>
> >
>
> Best regards,
> Krzysztof

--
People of Russia, stop Putin before his war on Ukraine escalates.


Attachments:
(No filename) (3.61 kB)
signature.asc (201.00 B)
Download all attachments

2024-02-12 11:16:57

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: usb: typec: anx7688: start a binding document

On 12/02/2024 12:02, Pavel Machek wrote:
> Hi!
>>> 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 <[email protected]>
>>>
>>
>> You miss proper diffstat which makes reviewing difficult.
>
>> Actually entire patch is corrupted and impossible to apply.
>
> Sorry about that.
>
>> Anyway, where is any user of this? Nothing in commit msg explains
>> this.
>
> User being is worked on:
>
> https://lore.kernel.org/lkml/2024020126-emote-unsubtly-3394@gregkh/T/
>
> Thanks for comments. I'll go through them and try to improve things.

OK, please send bindings patch in the same patchset as the driver.
Preferably first bindings patch, then driver patch(es). The bindings
document ABI exposed by driver, so we expect to see and apply both of
them together (apply through subsystem, so USB in this case).

Best regards,
Krzysztof


2024-02-21 22:47:35

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: usb: typec: anx7688: start a binding document

Hi!

> > + reset-gpios:
> > + maxItems: 1
>
> blank line
>
> > + avdd10-supply:
> > + description:
> > + 1.0V power supply
>
> Keep description in one line and add a blank line. This code is not that
> readable.
>
> > + i2c-supply:
> > + description:
> > + Power supply
>
> Drop all useless descriptions (so : true)

Ok, fixed, I hope I got it right.

> > + vbus-supply:
> > + description:
> > + Power supply
> > + vbus_in-supply:
>
> No underscores in property names.

Ok.

> > + connector:
> > + type: object
> > + $ref: ../connector/usb-connector.yaml
>
> Full path, so /schemas/connector/......
>
> > + unevaluatedProperties: false
>
> Drop

> > +
> > + description:
> > + Properties for usb c connector.
>
> > +
> > + properties:
> > + compatible:
> > + const: usb-c-connector
> > +
> > + power-role: true
> > +
> > + data-role: true
> > +
> > + try-power-role: true
>
> I don't understand why do you have here properties. Do you see any
> binding like this?

Well, it looks like I copied these mistakes from analogix,anx7411.yaml .

> > +
> > + required:
> > + - compatible
>
> Drop, why is it needed?

Again, copy from analogix,anx7411.yaml .

Should I try to fix up analogix,anx7411.yaml , and submit that, first,
or is it easier if you do that?

Thanks and best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.


Attachments:
(No filename) (1.50 kB)
signature.asc (201.00 B)
Download all attachments

2024-02-22 08:27:06

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: usb: typec: anx7688: start a binding document

On 21/02/2024 23:45, Pavel Machek wrote:
>>> + properties:
>>> + compatible:
>>> + const: usb-c-connector
>>> +
>>> + power-role: true
>>> +
>>> + data-role: true
>>> +
>>> + try-power-role: true
>>
>> I don't understand why do you have here properties. Do you see any
>> binding like this?
>
> Well, it looks like I copied these mistakes from analogix,anx7411.yaml .
>
>>> +
>>> + required:
>>> + - compatible
>>
>> Drop, why is it needed?
>
> Again, copy from analogix,anx7411.yaml .
>
> Should I try to fix up analogix,anx7411.yaml , and submit that, first,
> or is it easier if you do that?

I'll fix it up, thanks.

Best regards,
Krzysztof