On 07/12/2023 13:33, Anand Moon wrote: > Hi Krzysztof > > On Thu, 7 Dec 2023 at 14:00, Krzysztof Kozlowski > <krzysztof.kozlowski@xxxxxxxxxx> wrote: >> >> On 06/12/2023 18:14, Anand Moon wrote: >>> Hi Rob, >>> >>> On Wed, 6 Dec 2023 at 19:23, Rob Herring <robh@xxxxxxxxxx> wrote: >>>> >>>> On Mon, Dec 04, 2023 at 08:14:25PM +0530, Anand Moon wrote: >>>>> Add the binding example for the USB3.1 Genesys Logic GL3523 >>>>> integrates with USB 3.1 Gen 1 Super Speed and USB 2.0 High-Speed >>>>> hub. >>>>> >>>>> For onboard hub controllers that support USB 3.x and USB 2.0 hubs >>>>> with shared resets and power supplies, this property is used to identify >>>>> the hubs with which these are shared. >>>>> >>>>> GL3523 has built-in 5V to 3.3V and 5V to 1.2V regulators, which serves >>>>> power to the USB HUB, it uses 5V power regulator. >>>>> >>>>> Reviewed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx> >>>>> Signed-off-by: Anand Moon <linux.amoon@xxxxxxxxx> >>>>> --- >>>>> V6: fix the description of the regulators >>>>> Updated the commit message for regulator updates. >>>>> add reviewed by Conor Dooley >>>>> [1] https://lore.kernel.org/all/20231130053130.21966-2-linux.amoon@xxxxxxxxx/ >>>>> v5: upgrade peer-hub description : Conor Dooley >>>>> [0] https://www.genesyslogic.com.tw/en/product_view.php?show=67 [Block Diagram] >>>>> v4: Fix the description of peer-hub and update the commit message. >>>>> Schematics of the Odroid N2+ >>>>> https://dn.odroid.com/S922X/ODROID-N2/Schematic/odroid-n2_rev0.6_20210121.pdf >>>>> V3: fix the dt_binding_check error, added new example for Genesys GL3523 >>>>> v2: added Genesys GL3523 binding >>>>> v1: none >>>>> --- >>>>> .../bindings/usb/genesys,gl850g.yaml | 65 +++++++++++++++++-- >>>>> 1 file changed, 61 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml >>>>> index ee08b9c3721f..c6f63a69396d 100644 >>>>> --- a/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml >>>>> +++ b/Documentation/devicetree/bindings/usb/genesys,gl850g.yaml >>>>> @@ -9,9 +9,6 @@ title: Genesys Logic USB hub controller >>>>> maintainers: >>>>> - Icenowy Zheng <uwu@xxxxxxxxxx> >>>>> >>>>> -allOf: >>>>> - - $ref: usb-device.yaml# >>>>> - >>>>> properties: >>>>> compatible: >>>>> enum: >>>>> @@ -27,12 +24,46 @@ properties: >>>>> >>>>> vdd-supply: >>>>> description: >>>>> - the regulator that provides 3.3V core power to the hub. >>>>> + The regulator that provides 3.3V or 5.0V core power to the hub. >>>>> + >>>>> + peer-hub: >>>>> + $ref: /schemas/types.yaml#/definitions/phandle >>>>> + description: >>>>> + For onboard hub controllers that support USB 3.x and USB 2.0 hubs >>>>> + with shared resets and power supplies, this property is used to identify >>>>> + the hubs with which these are shared. >>>>> >>>>> required: >>>>> - compatible >>>>> - reg >>>>> >>>>> +allOf: >>>>> + - $ref: usb-device.yaml# >>>>> + - if: >>>>> + properties: >>>>> + compatible: >>>>> + contains: >>>>> + enum: >>>>> + - usb5e3,608 >>>>> + then: >>>>> + properties: >>>>> + peer-hub: false >>>>> + vdd-supply: false >>>>> + reset-gpios: true >>>>> + >>>>> + - if: >>>>> + properties: >>>>> + compatible: >>>>> + contains: >>>>> + enum: >>>>> + - usb5e3,610 >>>>> + - usb5e3,620 >>>>> + then: >>>>> + properties: >>>>> + peer-hub: true >>>>> + vdd-supply: true >>>>> + reset-gpios: true >>>> >>>> No need for this if schema. The default is they are allowed. >>>> >>> >>> If I move reset-gpios to required, I observe the below warning. >>> >>> DTC_CHK Documentation/devicetree/bindings/usb/maxim,max33359.example.dtb >>> /home/alarm/linux-amlogic-5.y-devel/Documentation/devicetree/bindings/usb/usb-device.example.dtb: >>> hub@1: 'reset-gpio' is a required property >>> from schema $id: http://devicetree.org/schemas/usb/genesys,gl850g.yaml# >> >> Where are the properties defined? If you open the binding you see: >> nowhere. You cannot define properties in some variant with "true". >> Please define all of them in top-level and only narrow/constrain when >> applicable. >> > What I meant is the example below, required meant applicable for both > the binding > But it shows me the above warning. My explanation stands... So again: >> Please define all of them in top-level and only narrow/constrain when >> applicable. Best regards, Krzysztof