All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yixun Lan <dlan@kernel.org>
To: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Matthias Kaehlcke <mka@chromium.org>,
	Junzhong Pan <panjunzhong@linux.spacemit.com>,
	Inochi Amaoto <inochiama@gmail.com>,
	spacemit@lists.linux.dev, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: usb: Add support for Terminus FE1.1s USB2.0 Hub controller
Date: Thu, 19 Mar 2026 11:43:59 +0800	[thread overview]
Message-ID: <20260319034359-GKC489299@kernel.org> (raw)
In-Reply-To: <20260318-original-cockle-from-jupiter-a2c9b0@quoll>

Hi Krzysztof, 

I will address all comments for other patches in this series, so no reply
to all of them..

On 09:36 Wed 18 Mar     , Krzysztof Kozlowski wrote:
> On Tue, Mar 17, 2026 at 08:55:03AM +0000, Yixun Lan wrote:
> > Terminus FE1.1s is USB2.0 protocol compliant 4-port USB HUB, It support
> > MTT (Multiple Transaction Translator) mode, the upstream port supports
> > high-speed 480MHz and full-speed 12MHz modes, also has integrated 5V to
> > 3.3V, 1.8V regulator and Power-On-Reset circuit.
> > 
> > Introduce the DT binding for it.
> > 
> > Link: https://terminus-usa.com/wp-content/uploads/2024/06/FE1.1s-Product-Brief-Rev.-2.0-2023.pdf [1]
> > Signed-off-by: Yixun Lan <dlan@kernel.org>
> > ---
> >  .../devicetree/bindings/usb/terminus,fe11.yaml     | 61 ++++++++++++++++++++++
> >  1 file changed, 61 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/usb/terminus,fe11.yaml b/Documentation/devicetree/bindings/usb/terminus,fe11.yaml
> > new file mode 100644
> > index 000000000000..93bb4066f851
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/terminus,fe11.yaml
> > @@ -0,0 +1,61 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/terminus,fe11.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Terminus FE1.1/1.1S USB 2.0 Hub Controller
> > +
> > +maintainers:
> > +  - Yixun Lan <dlan@kernel.org>
> > +
> > +allOf:
> > +  - $ref: usb-hub.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - usb1a40,0101
> 
> I do not see vendor prefix used anywhere. Drop that patch, because I
> think we do not reserve them for file names or schema IDs.
> 
Ok, will drop
> > +
> > +  reg: true
> > +
> > +  reset-gpios:
> > +    description:
> > +      GPIO controlling the RESET#, but the reset line can be optional.
> 
> Don't repeat constraints in free form text. Schema tells what can be
> optional by not requiring it.
> 
Ok
> > +
> > +  vdd-supply:
> > +    description:
> > +      Regulator supply to the hub, one of 3.3V or 5V can be chosen.
> 
> And this cannot be optional.
> 
Ok, will add it to 'required' section
> > +
> > +  ports:
> > +    $ref: /schemas/graph.yaml#/properties/ports
> > +
> > +    patternProperties:
> > +      '^port@':
> > +        $ref: /schemas/graph.yaml#/properties/port
> > +
> > +        properties:
> > +          reg:
> > +            minimum: 1
> > +            maximum: 4
> > +
> > +required:
> > +  - compatible
> > +  - reg
> 
> Missing supply.
> 
ditto
> > +
> > +additionalProperties: false
> 
> First, radxa never built tested the DTS when posted it. Yeah, why would
> they care...
> 
> Second, now you added schema which is nice, but you still did not verify
> it with the DTS.
> 
> That's the point of DT schema. To verify the DTS. You MUST do it when
> you post the binding. Why? To see the errors you have here. See other
> hub schemas, like genesys, what goes above - unevaluated.
> 
Ok, I did the binding check but only for arch/riscv/, will cover all in
future..

> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    usb {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        hub: hub@1 {
> 
> Drop unused label
> 
Ok

> > +            compatible = "usb1a40,0101";
> > +            reg = <1>;
> > +            reset-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
> > +            vdd-supply = <&vcc_5v>;
> > +        };
> > +    };
> > 
> > -- 
> > 2.53.0
> > 
> 

-- 
Yixun Lan (dlan)

  reply	other threads:[~2026-03-19  3:44 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-17  8:55 [PATCH 0/3] usb: misc: Add Terminus FE1.1 USB2.0 Hub support Yixun Lan
2026-03-17  8:55 ` [PATCH 1/3] dt-bindings: vendor-prefixes: Add Terminus Yixun Lan
2026-03-18  8:27   ` Krzysztof Kozlowski
2026-03-17  8:55 ` [PATCH 2/3] dt-bindings: usb: Add support for Terminus FE1.1s USB2.0 Hub controller Yixun Lan
2026-03-18  8:36   ` Krzysztof Kozlowski
2026-03-19  3:43     ` Yixun Lan [this message]
2026-03-17  8:55 ` [PATCH 3/3] usb: misc: onboard_usb_dev: Add Terminus FE1.1s USB2.0 Hub (1a40:0101) Yixun Lan
2026-03-18  8:40   ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260319034359-GKC489299@kernel.org \
    --to=dlan@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=inochiama@gmail.com \
    --cc=krzk+dt@kernel.org \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mka@chromium.org \
    --cc=panjunzhong@linux.spacemit.com \
    --cc=robh@kernel.org \
    --cc=spacemit@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.