Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Kuninori Morimoto]

Hi Simon

> > > Add fallback compatibility string.
> > > This is in keeping with the fallback scheme being adopted wherever
> > > appropriate for drivers for Renesas SoCs.
> > > 
> > > Signed-off-by: Simon Horman 
> > > ---
> > (snip)
> > > + {
> > > + .compatible = "renesas,usbhs",
> > > + .data = (void *)USBHS_TYPE_RCAR_GEN2,
> > > + },
> > >   { },
> > >  };
> > 
> > I think this is too much. This driver is used not only from R-Car Gen2.
> > It will work as normal mode if .data was 0.
> > see usbhs_parse_dt()
> 
> Are you suggesting that we remove USBHS_TYPE_RCAR_GEN2 from the driver?

I mean this

+   {
+   .compatible = "renesas,usbhs",
+   },

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Sergei Shtylyov]

On 12/08/2015 08:51 AM, Simon Horman wrote:


Add fallback compatibility string.
This is in keeping with the fallback scheme being adopted wherever
appropriate for drivers for Renesas SoCs.

Signed-off-by: Simon Horman 
---
  Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 4 ++--
  drivers/usb/renesas_usbhs/common.c  | 4 
  2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt 
b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
index 7d48f63db44e..8c50df8441c9 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
@@ -1,7 +1,7 @@
  Renesas Electronics USBHS driver

  Required properties:
-  - compatible: Must contain one of the following:
+  - compatible: "renesas,usbhs-", "renesas,rcar-usbhs" as fallback.
- "renesas,usbhs-r8a7790"
- "renesas,usbhs-r8a7791"
- "renesas,usbhs-r8a7794"

[...]

diff --git a/drivers/usb/renesas_usbhs/common.c 
b/drivers/usb/renesas_usbhs/common.c
index d82fa36c3465..2a9d4f405f30 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -481,6 +481,10 @@ static const struct of_device_id usbhs_of_match[] = {
.compatible = "renesas,usbhs-r8a7795",
.data = (void *)USBHS_TYPE_RCAR_GEN2,
},
+   {
+   .compatible = "renesas,usbhs",


   You just documented "renesas,rcar-usbhs".

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Simon Horman]

On Tue, Dec 08, 2015 at 05:35:28PM +0300, Sergei Shtylyov wrote:
> On 12/08/2015 08:51 AM, Simon Horman wrote:
> 
> >Add fallback compatibility string.
> >This is in keeping with the fallback scheme being adopted wherever
> >appropriate for drivers for Renesas SoCs.
> >
> >Signed-off-by: Simon Horman 
> >---
> >  Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 4 ++--
> >  drivers/usb/renesas_usbhs/common.c  | 4 
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> >
> >diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt 
> >b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> >index 7d48f63db44e..8c50df8441c9 100644
> >--- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> >+++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
> >@@ -1,7 +1,7 @@
> >  Renesas Electronics USBHS driver
> >
> >  Required properties:
> >-  - compatible: Must contain one of the following:
> >+  - compatible: "renesas,usbhs-", "renesas,rcar-usbhs" as fallback.
> > - "renesas,usbhs-r8a7790"
> > - "renesas,usbhs-r8a7791"
> > - "renesas,usbhs-r8a7794"
> [...]
> >diff --git a/drivers/usb/renesas_usbhs/common.c 
> >b/drivers/usb/renesas_usbhs/common.c
> >index d82fa36c3465..2a9d4f405f30 100644
> >--- a/drivers/usb/renesas_usbhs/common.c
> >+++ b/drivers/usb/renesas_usbhs/common.c
> >@@ -481,6 +481,10 @@ static const struct of_device_id usbhs_of_match[] = {
> > .compatible = "renesas,usbhs-r8a7795",
> > .data = (void *)USBHS_TYPE_RCAR_GEN2,
> > },
> >+{
> >+.compatible = "renesas,usbhs",
> 
>You just documented "renesas,rcar-usbhs".

Thanks. I meant to use "renesas,rcar-usbhs" throughout the patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Simon Horman]

On Tue, Dec 08, 2015 at 08:32:49AM +, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > > > Add fallback compatibility string.
> > > > This is in keeping with the fallback scheme being adopted wherever
> > > > appropriate for drivers for Renesas SoCs.
> > > > 
> > > > Signed-off-by: Simon Horman 
> > > > ---
> > > (snip)
> > > > +   {
> > > > +   .compatible = "renesas,usbhs",
> > > > +   .data = (void *)USBHS_TYPE_RCAR_GEN2,
> > > > +   },
> > > > { },
> > > >  };
> > > 
> > > I think this is too much. This driver is used not only from R-Car Gen2.
> > > It will work as normal mode if .data was 0.
> > > see usbhs_parse_dt()
> > 
> > Are you suggesting that we remove USBHS_TYPE_RCAR_GEN2 from the driver?
> 
> I mean this
> 
> + {
> + .compatible = "renesas,usbhs",
> + },

(As Sergei noted elsewhere, "renesas,rcar-usbhs" is consistent
 with what I documented elsewhere in the patch)

My understanding is that will cause the driver to operate in a different manner
to if USBHS_TYPE_RCAR_GEN2 was set. And thus meaning
of the generic and SoC-specifc compatibility strings will cause
the driver to operate differently.

Currently the only compat strings present are as follows:

* renesas,usbhs-r8a7790
* renesas,usbhs-r8a7791
* renesas,usbhs-r8a7794
* renesas,usbhs-r8a7795

And they all set USBHS_TYPE_RCAR_GEN2 (even though r8a7795 is a Gen3 SoC).

What I am aiming is either:
* A compatibility string for R-Car or;
* Two compatibility strings, one for R-Car Gen2 and one for R-Car Gen3

And for this new compatibility string or strings to operate the same
way as the current compatibility strings.


Looking over the driver its not clear to me why the
non-USBHS_TYPE_RCAR_GEN2 case is handled by the code as
there are no compatibility strings present that trigger that mode of
operation. I feel that I must be missing something.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Kuninori Morimoto]

Hi Simon

> > > (As Sergei noted elsewhere, "renesas,rcar-usbhs" is consistent
> > >  with what I documented elsewhere in the patch)
> > 
> > "renesas,rcar-usbhs" is better,
> > but I guess you want to have "renesas,rcar-gen2-usbhs" ?
> > 
> > My understanding is these
> > 
> >  * renesas,usbhs-r8a77xx# SoC specific
> >  * renesas,rcar-usbhs   # R-Car common
> >  * renesas,rcar-gen2-usbhs  # R-Car Gen2 common
> >  * renesas,rcar-gen3-usbhs  # R-Car Gen3 common
> >  * renesas,usbhs# Renesas USBHS common
> > 
> 
> I was intentionally including gen3 as well. So I think we have two options:
> 
> 1. renesas,rcar-usbhs
> 2. renesas,rcar-gen2-usbhs and renesas,rcar-gen3-usbhs

Renesas USB always have pick feature/settings.
Thus, generic name (= "renesas,rcar-usbhs") is very risky IMO.
I think 2 is more safety.

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Simon Horman]

On Wed, Dec 09, 2015 at 06:29:07AM +, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > > > (As Sergei noted elsewhere, "renesas,rcar-usbhs" is consistent
> > > >  with what I documented elsewhere in the patch)
> > > 
> > > "renesas,rcar-usbhs" is better,
> > > but I guess you want to have "renesas,rcar-gen2-usbhs" ?
> > > 
> > > My understanding is these
> > > 
> > >  * renesas,usbhs-r8a77xx  # SoC specific
> > >  * renesas,rcar-usbhs # R-Car common
> > >  * renesas,rcar-gen2-usbhs# R-Car Gen2 common
> > >  * renesas,rcar-gen3-usbhs# R-Car Gen3 common
> > >  * renesas,usbhs  # Renesas USBHS common
> > > 
> > 
> > I was intentionally including gen3 as well. So I think we have two options:
> > 
> > 1. renesas,rcar-usbhs
> > 2. renesas,rcar-gen2-usbhs and renesas,rcar-gen3-usbhs
> 
> Renesas USB always have pick feature/settings.
> Thus, generic name (= "renesas,rcar-usbhs") is very risky IMO.
> I think 2 is more safety.

Sure, better safe than sorry.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Kuninori Morimoto]

Hi Simon

> (As Sergei noted elsewhere, "renesas,rcar-usbhs" is consistent
>  with what I documented elsewhere in the patch)

"renesas,rcar-usbhs" is better,
but I guess you want to have "renesas,rcar-gen2-usbhs" ?

My understanding is these

 * renesas,usbhs-r8a77xx# SoC specific
 * renesas,rcar-usbhs   # R-Car common
 * renesas,rcar-gen2-usbhs  # R-Car Gen2 common
 * renesas,rcar-gen3-usbhs  # R-Car Gen3 common
 * renesas,usbhs# Renesas USBHS common
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Simon Horman]

On Wed, Dec 09, 2015 at 04:48:47AM +, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > (As Sergei noted elsewhere, "renesas,rcar-usbhs" is consistent
> >  with what I documented elsewhere in the patch)
> 
> "renesas,rcar-usbhs" is better,
> but I guess you want to have "renesas,rcar-gen2-usbhs" ?
> 
> My understanding is these
> 
>  * renesas,usbhs-r8a77xx  # SoC specific
>  * renesas,rcar-usbhs # R-Car common
>  * renesas,rcar-gen2-usbhs# R-Car Gen2 common
>  * renesas,rcar-gen3-usbhs# R-Car Gen3 common
>  * renesas,usbhs  # Renesas USBHS common
> 

I was intentionally including gen3 as well. So I think we have two options:

1. renesas,rcar-usbhs
2. renesas,rcar-gen2-usbhs and renesas,rcar-gen3-usbhs

Which do you prefer?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Simon Horman]

On Tue, Dec 08, 2015 at 06:05:51AM +, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > Add fallback compatibility string.
> > This is in keeping with the fallback scheme being adopted wherever
> > appropriate for drivers for Renesas SoCs.
> > 
> > Signed-off-by: Simon Horman 
> > ---
> (snip)
> > +   {
> > +   .compatible = "renesas,usbhs",
> > +   .data = (void *)USBHS_TYPE_RCAR_GEN2,
> > +   },
> > { },
> >  };
> 
> I think this is too much. This driver is used not only from R-Car Gen2.
> It will work as normal mode if .data was 0.
> see usbhs_parse_dt()

Are you suggesting that we remove USBHS_TYPE_RCAR_GEN2 from the driver?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Kuninori Morimoto]

Hi Simon

> Add fallback compatibility string.
> This is in keeping with the fallback scheme being adopted wherever
> appropriate for drivers for Renesas SoCs.
> 
> Signed-off-by: Simon Horman 
> ---
(snip)
> + {
> + .compatible = "renesas,usbhs",
> + .data = (void *)USBHS_TYPE_RCAR_GEN2,
> + },
>   { },
>  };

I think this is too much. This driver is used not only from R-Car Gen2.
It will work as normal mode if .data was 0.
see usbhs_parse_dt()
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] usb: renesas_usbhs: add fallback compatibility string

[Simon Horman]

Add fallback compatibility string.
This is in keeping with the fallback scheme being adopted wherever
appropriate for drivers for Renesas SoCs.

Signed-off-by: Simon Horman 
---
 Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 4 ++--
 drivers/usb/renesas_usbhs/common.c  | 4 
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt 
b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
index 7d48f63db44e..8c50df8441c9 100644
--- a/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
+++ b/Documentation/devicetree/bindings/usb/renesas_usbhs.txt
@@ -1,7 +1,7 @@
 Renesas Electronics USBHS driver
 
 Required properties:
-  - compatible: Must contain one of the following:
+  - compatible: "renesas,usbhs-", "renesas,rcar-usbhs" as fallback.
- "renesas,usbhs-r8a7790"
- "renesas,usbhs-r8a7791"
- "renesas,usbhs-r8a7794"
@@ -22,7 +22,7 @@ Optional properties:
 
 Example:
usbhs: usb@e659 {
-   compatible = "renesas,usbhs-r8a7790";
+   compatible = "renesas,usbhs-r8a7790", "renesas,rcar-usbhs";
reg = <0 0xe659 0 0x100>;
interrupts = <0 107 IRQ_TYPE_LEVEL_HIGH>;
clocks = <_clks R8A7790_CLK_HSUSB>;
diff --git a/drivers/usb/renesas_usbhs/common.c 
b/drivers/usb/renesas_usbhs/common.c
index d82fa36c3465..2a9d4f405f30 100644
--- a/drivers/usb/renesas_usbhs/common.c
+++ b/drivers/usb/renesas_usbhs/common.c
@@ -481,6 +481,10 @@ static const struct of_device_id usbhs_of_match[] = {
.compatible = "renesas,usbhs-r8a7795",
.data = (void *)USBHS_TYPE_RCAR_GEN2,
},
+   {
+   .compatible = "renesas,usbhs",
+   .data = (void *)USBHS_TYPE_RCAR_GEN2,
+   },
{ },
 };
 MODULE_DEVICE_TABLE(of, usbhs_of_match);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html