On Thu, Mar 28, 2019 at 12:48 PM Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> wrote: > > On Thu, Mar 28, 2019 at 10:33:53AM -0500, Rob Herring wrote: > > On Fri, Mar 15, 2019 at 12:51:46PM +0200, Heikki Krogerus wrote: > > > Thanks, > > > > > > On Fri, Mar 15, 2019 at 09:08:19AM +0000, Biju Das wrote: > > > > > Subject: Re: [PATCH v2 2/7] dt-bindings: usb: renesas_usb3: add usb-role- > > > > > > > > a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > index 35039e7..eecaf4c 100644 > > > > > > > > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt > > > > > > > > @@ -22,6 +22,7 @@ Required properties: > > > > > > > > Optional properties: > > > > > > > > - phys: phandle + phy specifier pair > > > > > > > > - phy-names: must be "usb" > > > > > > > > + - usb-role-switch: use USB role switch to support dual-role > > > > > > > > + switch > > > > > > > > > > > > > > I don't think we can add such a property. At least, we have to add > > > > > "renesas," > > > > > > > prefix. > > > > > > > > > > > > usb_role_switch_get api uses "usb-role-switch" property to get role > > > > > switch linked with the device. > > > > > > > > > > > > HD3SS3220 port controller driver gets role switch handle linked with the > > > > > device using usb_role_switch_get api. > > > > > > That is the reason, I have added " usb-role-switch" property here. > > > > > > > > > > > > Do you have any other suggestion to get usb role switch handle? > > > > > > > > > > We can still change the API. Can we use the compatible for this? > > > > > > > > Do you mean usb_role_switch_get API needs to handle compatible "usb-x-connector" wherex= a,b ,c ? > > > > Then it uses the graphs api to get the device linked with it and return the usb role switch handle. > > > > In that case, no need to add generic "usb-role-switch" property here. > > > > > > > > Can you please confirm my understanding is correct? > > > > > > No, I meant the compatible property would have the value > > > "usb-role-switch". Your compatible would probable look something > > > like this: > > > > > > compatible = "renesas,r8a774c0-usb3-peri", > > > "usb-role-switch"; > > > > > > So the idea would be that the device supplying USB role switch > > > functionality, in your case the USB controller, would need to have the > > > compatible property containing "usb-role-switch". > > > > That's not really something a driver could bind to nor provides any info > > as to what the h/w is (and how to interact with it). > > Fair enough. As I said, I don't know much about DT. > > The problem we are trying to solve here is, how to identify the USB > role switch from graph. It's primarily the USB Type-C connector > drivers that need to be able to get a handle to the role switch device > so they can tell it what to do. Basically the caller of > usb_role_switch_get() will have the compatible "usb-c-connector", so > it's of no use to us. We need the other endpoint, the role switch. > > Note: The USB role switch device will be a discrete (or integrated) > mux on platforms that have separate USB host and USB device > controllers, and on platforms with dual-role capable USB controller > (like this one) the USB controller will represent it. Either way, it should just be the device connected to the connector's USB data port in the graph. Though maybe if USB2 and USB3 are different controllers that would complicate things. > At the moment the function usb_role_switch_get() walks trough the > graph of the caller (so the connector) and expects that the > remote-endpoint representing the mux will have a boolean property > "usb-role-switch". Assuming we have that I think it should be in the device (controller/mux) node rather than the endpoint node itself. It could also be outside of DT in that the controller driver will know the h/w can support role switch and can register that capability. IOW, it can be implied by the compatible string. > > I do agree that Documentation/devicetree/bindings/usb/renesas_usb3.txt > should not be the file describing that boolean property, but if we had > a separate file describing the bindings for just the USB role switch > devices, would using it still be OK? It should be described somewhere common, yes, but the property itself belongs in controller nodes. Generally, we still document where common properties are used. Though, I prefer the implied by the compatible option. This should work unless there's a strong need to find the switch in DT without the switch driver being probed or if this is board specific (I wouldn't think so). > > If not, then can you propose something else? How do we identify the > USB role switch endpoint from the graph? Would it be OK to expect the > the endpoint subnode to have that boolean property instead? > > > thanks, > > -- > heikki
