| Message ID | 20131018141221.GH2721@beef (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Friday 18 October 2013 07:42 PM, Matt Porter wrote: > This is a summary of an unresolved issue resulting from this thread: > http://www.spinics.net/lists/arm-kernel/msg277700.html > > The BCM281xx family of SoCs contain an OTG subsystem consisting of a > DWC2 HSOTG controller and an internal UTMI PHY. This is appears as > follows (monospace font requirement ahead): > > +----------------+ +-----------------+ > | | | | > | | 8 | | > | DWC2 |<------/----->| BCM Kona | > | | UTMI | UTMI PHY | > | | | | > +----------------+ +-----------------+ > > The internal UTMI phy is connected via an 8-bit data path. There is > no way to autodetect whether the data path is 8-bit or 16-bit. As such, > it was determined that a DT property is necessary to reflect this. > > In the original patch submitted this property was offered as an > additional optional dwc2 property: > > --- a/Documentation/devicetree/bindings/staging/dwc2.txt > +++ b/Documentation/devicetree/bindings/staging/dwc2.txt > @@ -6,10 +6,14 @@ Required properties: > - reg : Should contain 1 register range (address and length) > - interrupts : Should contain 1 interrupt > > +Optional properties: > +- snps,phy-utmi-width: Must contain the UTMI data width (either 8 or 16) > + > Example: > > usb@101c0000 { > compatible = "ralink,rt3050-usb, snps,dwc2"; > reg = <0x101c0000 40000>; > interrupts = <18>; > + snps,phy-utmi-width = <8>; > }; > > The open question is whether this required hardware property belongs to > the DWC2 controller or the PHY itself. I think it makes sense to keep the data width property in the dwc2 node itself. I mean it describes how the dwc2 IP is configured in that particular SoC (given that it can be either <8> or <16>). Thanks Kishon > > If the UTMI data path width is considered to be a property of the PHY > then this will impact both the generic PHY framework and the PHY device > node (producer) binding. The binding would need to be extended to carry > the data path width property. In addition, the generic PHY framework > would need to allow for this information to be gathered in some manner > for use by the controller driver (PHY consumer). In the case of DWC2, > the driver needs to know whether to program the phy interface for 8 or > 16 bit UTMI communication. > > Thanks, > Matt > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Hi Kishon, On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote: > I think it makes sense to keep the data width property in the dwc2 node itself. > I mean it describes how the dwc2 IP is configured in that particular SoC (given > that it can be either <8> or <16>). If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP can be configured for 8, 16 or 8 _and_ 16. In the latter case, the "8 and 16 supported" would make sense as a property of dwc2 (though this value should be autodetectable through GHWCFG4), while the actual 8 or 16 supported by the PHY would make sense as property of a phy. Note sure if this is really useful in practice as well, or if just setting the actual width to use on dwc2 makes more sense... Gr. Matthijs
On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote: > Hi Kishon, > > On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote: > > I think it makes sense to keep the data width property in the dwc2 node itself. > > I mean it describes how the dwc2 IP is configured in that particular SoC (given > > that it can be either <8> or <16>). > If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP > can be configured for 8, 16 or 8 _and_ 16. In the latter case, the "8 > and 16 supported" would make sense as a property of dwc2 (though this > value should be autodetectable through GHWCFG4), while the actual 8 or > 16 supported by the PHY would make sense as property of a phy. There would be no value in adding a property for an already detectable value to dwc2's binding. To be honest, it's pretty much useless information due to the existence of the "8 and 16" option. > Note sure if this is really useful in practice as well, or if just > setting the actual width to use on dwc2 makes more sense... The GHWCFG4 information itself is not useful in practice, as described in the original thread: https://lkml.org/lkml/2013/10/10/477 It's certainly useful in practice to have this width property in either the dwc2 or the phy binding. One can make a case for either. As I mentioned in the original post, if we put it in the phy binding we'll be updating the generic phy binding. We'll then need an api added into the generic phy framework to fetch the width of a phy. Both cases are doable and trivial, we just need the canonical decision from a DT maintainer as to where the property belongs. Given that they are in ARM ksummit, I'm not expecting to hear anything right this moment. :) -Matt
On 10/22/2013 06:25 AM, Matt Porter wrote: > On Tue, Oct 22, 2013 at 12:48:29PM +0200, Matthijs Kooijman wrote: >> Hi Kishon, >> >> On Mon, Oct 21, 2013 at 02:57:26PM +0530, Kishon Vijay Abraham I wrote: >>> I think it makes sense to keep the data width property in the dwc2 node itself. >>> I mean it describes how the dwc2 IP is configured in that particular SoC (given >>> that it can be either <8> or <16>). >> If I'm reading the RT3052 datasheet correctly (GHWCFG4 register), the IP >> can be configured for 8, 16 or 8 _and_ 16. In the latter case, the "8 >> and 16 supported" would make sense as a property of dwc2 (though this >> value should be autodetectable through GHWCFG4), while the actual 8 or >> 16 supported by the PHY would make sense as property of a phy. > > There would be no value in adding a property for an already detectable > value to dwc2's binding. To be honest, it's pretty much useless > information due to the existence of the "8 and 16" option. > >> Note sure if this is really useful in practice as well, or if just >> setting the actual width to use on dwc2 makes more sense... > > The GHWCFG4 information itself is not useful in practice, as described > in the original thread: https://lkml.org/lkml/2013/10/10/477 > > It's certainly useful in practice to have this width property in either > the dwc2 or the phy binding. One can make a case for either. As I > mentioned in the original post, if we put it in the phy binding we'll be > updating the generic phy binding. We'll then need an api added into the > generic phy framework to fetch the width of a phy. > > Both cases are doable and trivial, we just need the canonical decision > from a DT maintainer as to where the property belongs. Given that they > are in ARM ksummit, I'm not expecting to hear anything right this > moment. :) The host can support both, so it is not a property of the host and is a property of the phy. It is no different than what mode a SPI slave requires or whether an i2c slave supports 8 or 10-bit addressing. Those examples are all 1 to many rather than 1 to 1 where it doesn't really matter, but the same logic applies. Rob
--- a/Documentation/devicetree/bindings/staging/dwc2.txt +++ b/Documentation/devicetree/bindings/staging/dwc2.txt @@ -6,10 +6,14 @@ Required properties: - reg : Should contain 1 register range (address and length) - interrupts : Should contain 1 interrupt +Optional properties: +- snps,phy-utmi-width: Must contain the UTMI data width (either 8 or 16) + Example: usb@101c0000 { compatible = "ralink,rt3050-usb, snps,dwc2"; reg = <0x101c0000 40000>; interrupts = <18>; + snps,phy-utmi-width = <8>; };