diff mbox

[v10,01/15] usb: doc: phy-mxs: Add more compatible strings

Message ID 1392873284-9386-2-git-send-email-peter.chen@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

Peter Chen Feb. 20, 2014, 5:14 a.m. UTC
Add "fsl,imx6q-usbphy" for imx6dq and imx6dl, add
"fsl,imx6sl-usbphy" for imx6sl.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 Documentation/devicetree/bindings/usb/mxs-phy.txt |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Comments

Peter Chen Feb. 21, 2014, 9:05 a.m. UTC | #1
On Fri, Feb 21, 2014 at 10:46:41AM +0100, Marc Kleine-Budde wrote:
> On 02/21/2014 10:40 AM, Peter Chen wrote:
> >  
> >>>
> >>>  Required properties:
> >>> -- compatible: Should be "fsl,imx23-usbphy"
> >>> +- compatible: "fsl,imx23-usbphy" for imx23 and imx28, "fsl,imx6q-
> >> usbphy"
> >>> +  for imx6dq and imx6dl, "fsl,imx6sl-usbphy" for imx6sl
> >>
> >> Minor nit, but could we restructure this as something like the following,
> >> with each string on a new line:
> >>
> >> - compatible: should contain:
> >>     * "fsl,imx23-usbphy" for imx23 and imx28
> >>     * "fsl,imx6q-usbphy" for imx6dq and imx6dl
> >>     * "fsl,imx6sl-usbphy" for imx6sl
> >>
> >> It makes it a bit easier to read.
> > 
> > Thanks, will change like above.
> > 
> >>
> >> I see the existing "fsl,imx23-usbphy" is used as a fallback for
> >> "fsl,imx28-usbphy", "fsl,imx6q-usbphy", and "fsl,imx6sl-usbphy" in
> >> existing DTs.
> >>
> >> Is this expected going forward? It might be worth mentioning.
> >>
> > 
> > These SoCs used the same FSL imx PHY, but different versions.
> > imx23/imx28 are the first version, more improvements are at
> > later SoCs (like imx6x) version. Keep "fsl,imx23-usbphy" at
> > imx6 dts will be user know it is from imx23's. If you think
> > it does not need, I can delete "fsl,imx23-usbphy" from imx6 dts.
> 
> You should go after compatibility here. List (all) phys that are
> comaptible, start with most specific end with most generic.
> 
> Marc
> 

Then, I should keep imx6 dts unchanging. Then, what I need to
mention at this binding doc?
Mark Rutland Feb. 21, 2014, 9:13 a.m. UTC | #2
On Thu, Feb 20, 2014 at 05:14:30AM +0000, Peter Chen wrote:
> Add "fsl,imx6q-usbphy" for imx6dq and imx6dl, add
> "fsl,imx6sl-usbphy" for imx6sl.
> 
> Signed-off-by: Peter Chen <peter.chen@freescale.com>
> ---
>  Documentation/devicetree/bindings/usb/mxs-phy.txt |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> index 5835b27..b43d4c9e 100644
> --- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
> +++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
> @@ -1,7 +1,8 @@
>  * Freescale MXS USB Phy Device
>  
>  Required properties:
> -- compatible: Should be "fsl,imx23-usbphy"
> +- compatible: "fsl,imx23-usbphy" for imx23 and imx28, "fsl,imx6q-usbphy"
> +  for imx6dq and imx6dl, "fsl,imx6sl-usbphy" for imx6sl

Minor nit, but could we restructure this as something like the
following, with each string on a new line:

- compatible: should contain:
    * "fsl,imx23-usbphy" for imx23 and imx28
    * "fsl,imx6q-usbphy" for imx6dq and imx6dl
    * "fsl,imx6sl-usbphy" for imx6sl

It makes it a bit easier to read.

I see the existing "fsl,imx23-usbphy" is used as a fallback for
"fsl,imx28-usbphy", "fsl,imx6q-usbphy", and "fsl,imx6sl-usbphy" in
existing DTs.

Is this expected going forward? It might be worth mentioning.

Otherwise this looks fine to me.

Thanks,
Mark.
Peter Chen Feb. 21, 2014, 9:40 a.m. UTC | #3
> >
> >  Required properties:
> > -- compatible: Should be "fsl,imx23-usbphy"
> > +- compatible: "fsl,imx23-usbphy" for imx23 and imx28, "fsl,imx6q-
> usbphy"
> > +  for imx6dq and imx6dl, "fsl,imx6sl-usbphy" for imx6sl
> 
> Minor nit, but could we restructure this as something like the following,
> with each string on a new line:
> 
> - compatible: should contain:
>     * "fsl,imx23-usbphy" for imx23 and imx28
>     * "fsl,imx6q-usbphy" for imx6dq and imx6dl
>     * "fsl,imx6sl-usbphy" for imx6sl
> 
> It makes it a bit easier to read.

Thanks, will change like above.

> 
> I see the existing "fsl,imx23-usbphy" is used as a fallback for
> "fsl,imx28-usbphy", "fsl,imx6q-usbphy", and "fsl,imx6sl-usbphy" in
> existing DTs.
> 
> Is this expected going forward? It might be worth mentioning.
> 

These SoCs used the same FSL imx PHY, but different versions.
imx23/imx28 are the first version, more improvements are at
later SoCs (like imx6x) version. Keep "fsl,imx23-usbphy" at
imx6 dts will be user know it is from imx23's. If you think
it does not need, I can delete "fsl,imx23-usbphy" from imx6 dts.

Peter
Marc Kleine-Budde Feb. 21, 2014, 9:46 a.m. UTC | #4
On 02/21/2014 10:40 AM, Peter Chen wrote:
>  
>>>
>>>  Required properties:
>>> -- compatible: Should be "fsl,imx23-usbphy"
>>> +- compatible: "fsl,imx23-usbphy" for imx23 and imx28, "fsl,imx6q-
>> usbphy"
>>> +  for imx6dq and imx6dl, "fsl,imx6sl-usbphy" for imx6sl
>>
>> Minor nit, but could we restructure this as something like the following,
>> with each string on a new line:
>>
>> - compatible: should contain:
>>     * "fsl,imx23-usbphy" for imx23 and imx28
>>     * "fsl,imx6q-usbphy" for imx6dq and imx6dl
>>     * "fsl,imx6sl-usbphy" for imx6sl
>>
>> It makes it a bit easier to read.
> 
> Thanks, will change like above.
> 
>>
>> I see the existing "fsl,imx23-usbphy" is used as a fallback for
>> "fsl,imx28-usbphy", "fsl,imx6q-usbphy", and "fsl,imx6sl-usbphy" in
>> existing DTs.
>>
>> Is this expected going forward? It might be worth mentioning.
>>
> 
> These SoCs used the same FSL imx PHY, but different versions.
> imx23/imx28 are the first version, more improvements are at
> later SoCs (like imx6x) version. Keep "fsl,imx23-usbphy" at
> imx6 dts will be user know it is from imx23's. If you think
> it does not need, I can delete "fsl,imx23-usbphy" from imx6 dts.

You should go after compatibility here. List (all) phys that are
comaptible, start with most specific end with most generic.

Marc
Mark Rutland Feb. 21, 2014, 12:38 p.m. UTC | #5
On Fri, Feb 21, 2014 at 09:40:29AM +0000, Peter Chen wrote:
>  
> > >
> > >  Required properties:
> > > -- compatible: Should be "fsl,imx23-usbphy"
> > > +- compatible: "fsl,imx23-usbphy" for imx23 and imx28, "fsl,imx6q-
> > usbphy"
> > > +  for imx6dq and imx6dl, "fsl,imx6sl-usbphy" for imx6sl
> > 
> > Minor nit, but could we restructure this as something like the following,
> > with each string on a new line:
> > 
> > - compatible: should contain:
> >     * "fsl,imx23-usbphy" for imx23 and imx28
> >     * "fsl,imx6q-usbphy" for imx6dq and imx6dl
> >     * "fsl,imx6sl-usbphy" for imx6sl
> > 
> > It makes it a bit easier to read.
> 
> Thanks, will change like above.
> 
> > 
> > I see the existing "fsl,imx23-usbphy" is used as a fallback for
> > "fsl,imx28-usbphy", "fsl,imx6q-usbphy", and "fsl,imx6sl-usbphy" in
> > existing DTs.
> > 
> > Is this expected going forward? It might be worth mentioning.
> > 
> 
> These SoCs used the same FSL imx PHY, but different versions.
> imx23/imx28 are the first version, more improvements are at
> later SoCs (like imx6x) version. Keep "fsl,imx23-usbphy" at
> imx6 dts will be user know it is from imx23's. If you think
> it does not need, I can delete "fsl,imx23-usbphy" from imx6 dts.

I'm not arguing to remove it, I'm suggesting it might be worth
mentioning that it's not mutually exclusive, and can be a fallback for
the other strings.

Cheers,
Mark.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/usb/mxs-phy.txt b/Documentation/devicetree/bindings/usb/mxs-phy.txt
index 5835b27..b43d4c9e 100644
--- a/Documentation/devicetree/bindings/usb/mxs-phy.txt
+++ b/Documentation/devicetree/bindings/usb/mxs-phy.txt
@@ -1,7 +1,8 @@ 
 * Freescale MXS USB Phy Device
 
 Required properties:
-- compatible: Should be "fsl,imx23-usbphy"
+- compatible: "fsl,imx23-usbphy" for imx23 and imx28, "fsl,imx6q-usbphy"
+  for imx6dq and imx6dl, "fsl,imx6sl-usbphy" for imx6sl
 - reg: Should contain registers location and length
 - interrupts: Should contain phy interrupt