Messages in this thread | | | Date | Wed, 20 Oct 2021 08:21:21 +0200 | Subject | Re: [PATCH v16 0/7] usb: misc: Add onboard_usb_hub driver | From | Michal Simek <> |
| |
On 10/19/21 18:04, Fabrice Gasnier wrote: > On 10/15/21 8:39 AM, Greg Kroah-Hartman wrote: >> On Thu, Oct 14, 2021 at 02:38:55PM -0700, Doug Anderson wrote: >>> Hi, >>> >>> On Tue, Sep 21, 2021 at 10:09 AM Matthias Kaehlcke <mka@chromium.org> wrote: >>>> >>>> Hi Greg, >>>> >>>> are there any actions pending or can this land in usb-testing? >>>> >>>> I confirmed that this series can be rebased on top of v5.15-rc2 >>>> without conflicts. >>> >>> I'm quite interested to know what the next action items are, too. This >>> is one of the very few patches we have for trogdor (excluding MIPI >>> camera, which is a long story) that we're carrying downstream, so I'm >>> keenly interested in making sure it's unblocked (if, indeed, it's >>> blocked on anything). >>> >>> If folks feel that this needs more review eyes before landing again >>> then I'll try to find some time in the next week or two. If it's just >>> waiting for the merge window to open/close so it can have maximal bake >>> time, that's cool too. Please yell if there's something that I can do >>> to help, though! :-) >> >> I would love more review-eyes on this please. >> > > Hi, > > I noticed this series some time ago, and wanted to take a closer look. > > The same issue this series address is seen on stm32 board for instance. > (arch/arm/boot/dts/stm32mp15xx-dkx.dtsi). On board HUB (not described in > the DT) is supplied by an always-on regulator. > So it could could be interesting/useful to address the same case , > on stm32 boards, where USB2 (ehci-platform driver) is used currently. > > I noticed a few things, especially on the dt-bindings. I've some > questions here. > > In this series, RTS5411 is used. The dt-bindings documents it as a child > node of the USB controller. E.g. > > &usb { > usb_hub_2_0: hub@1 { > ... > }; > > usb_hub_3_0: hub@2 { > }; > } > > I had a quick look at RTS5411 datasheet. It looks like there's an i2c > interface too. > - I guess the I2C interface isn't used in your case ? > (I haven't checked what it could be used for...) > > In the stm32 boards (stm32mp15xx-dkx), there's an usb2514b chip > - that also could be wired on I2C interface (0R mount option) > - unused on stm32 boards by default > > usb2514b chip already has a dt-bindings (with compatible), and a driver: > - drivers/usb/misc/usb251xb.c > - Documentation/devicetree/bindings/usb/usb251xb.txt > > It is defined more as an i2c chip, so I'd expect it as an i2c child, > e.g. like: > > &i2c { > usb2514b@2c { > compatible = "microchip,usb2514b"; > ... > }; > }; > > > This way, I don't see how it could be used together with onboard_usb_hub > driver ? (But I may have missed it) > Is it possible to use a phandle, instead of a child node ? > > However, in the stm32mp15xx-dkx case, i2c interface isn't wired/used by > default. So obviously the i2c driver isn't used. In this case, could the > "microchip,usb2514b" be listed in onboard_usb_hub driver ? > (wouldn't it be redundant ?) > > In this case it would be a child node of the usb DT node... Maybe that's > more a question for Rob: would it be "legal" regarding existing > dt-bindings ?
We wanted to upstream driver for microchip usb5744 and based on this thread with Rob
https://lore.kernel.org/all/CAL_JsqJZBbu+UXqUNdZwg-uv0PAsNg55026PTwhKr5wQtxCjVQ@mail.gmail.com/
the recommendation was to use i2c-bus link. And in our usb5744 case where usb hub has only one i2c address we just hardcoded it in the driver. I should be pushing this driver to xilinx soc tree soon if you want to take a look.
Thanks, Michal
| |