On Thu, Feb 11, 2021 at 3:35 AM Michal Simek <michal.simek@xxxxxxxxxx> wrote: > > Hi Rob, > > On 2/10/21 11:22 PM, Rob Herring wrote: > > On Tue, Feb 09, 2021 at 11:48:09AM +0100, Michal Simek wrote: > >> From: Piyush Mehta <piyush.mehta@xxxxxxxxxx> > >> > >> Added dt binding for usb5744 driver. > >> > >> Signed-off-by: Piyush Mehta <piyush.mehta@xxxxxxxxxx> > >> Signed-off-by: Michal Simek <michal.simek@xxxxxxxxxx> > >> --- > >> > >> Changes in v2: None > >> > >> .../bindings/usb/microchip,usb5744.yaml | 56 +++++++++++++++++++ > >> MAINTAINERS | 1 + > >> 2 files changed, 57 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/usb/microchip,usb5744.yaml > >> > >> diff --git a/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml > >> new file mode 100644 > >> index 000000000000..fe222f6db81d > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/usb/microchip,usb5744.yaml > >> @@ -0,0 +1,56 @@ > >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >> +%YAML 1.2 > >> +--- > >> +$id: "http://devicetree.org/schemas/usb/microchip,usb5744.yaml#"; > >> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"; > >> + > >> +title: Bindings for the Microchip USB5744 4-port Hub Controller > >> + > >> +description: > >> + Microchip’s USB5744 SmartHub™ IC is a 4 port, SuperSpeed (SS)/Hi-Speed (HS), > >> + low power, low pin count configurable and fully compliant with the USB 3.1 > >> + Gen 1 specification. The USB5744 also supports Full Speed (FS) and Low Speed > >> + (LS) USB signaling, offering complete coverage of all defined USB operating > >> + speeds. The new SuperSpeed hubs operate in parallel with the USB 2.0 > >> + controller, so 5 Gbps SuperSpeed data transfers are not affected by slower > >> + USB 2.0 traffic. > >> + > >> +maintainers: > >> + - Piyush Mehta <piyush.mehta@xxxxxxxxxx> > >> + - Michal Simek <michal.simek@xxxxxxxxxx> > >> + > >> +properties: > >> + compatible: > >> + const: microchip,usb5744 > >> + > >> + reg: > >> + maxItems: 1 > >> + description: | > >> + Specifies the i2c slave address, it is required and should be 0x2d > >> + if I2C is used. > > > > If I2C is not used, then this should be underneath the USB host as a USB > > device. That also implies a different compatible string. I'd suggest you > > just say I2C is required if that's your use. > > We can't say that i2c is required because we have both cases. One is > really usb hub connected over i2c which at least requires to send one > smbus command to start operate. But it can be extended to add more > features - limit speeds, disable ports, etc. > > And the second is really the same usb hub without i2c connected which > runs in default mode. But reset is required to ensure proper reset > sequence. > Hub also have external clock chip which is not handled now because it is > just crystal on the board but if you want I can also model it via fixed > clock and call clock enable for it. > > It is the same use case as is with > Documentation/devicetree/bindings/usb/usb3503.txt Yes, there are examples of how we don't want to do it. > Can you please elaborate why different compatible string should be used? > It is still the same device and not quite sure why different compatible > string should be used. > > Do you also want to example where this node is the part of usb node? See usb/usb-device.txt. And there is this[1] under review. For these cases with I2C, I'd really rather see the hub always under the USB bus with a link to the I2C bus when connected. Rob [1] https://lore.kernel.org/linux-devicetree/20210210091015.v5.1.I248292623d3d0f6a4f0c5bc58478ca3c0062b49a@changeid/
