| Message ID | 1449553911-21412-2-git-send-email-horms+renesas@verge.net.au (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
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 <horms+renesas@verge.net.au> > --- (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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 08, 2015 at 06:05:51AM +0000, 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 <horms+renesas@verge.net.au> > > --- > (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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <horms+renesas@verge.net.au> > > > --- > > (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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <horms+renesas@verge.net.au> > --- > 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-<soctype>", "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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Dec 08, 2015 at 08:32:49AM +0000, 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 <horms+renesas@verge.net.au> > > > > --- > > > (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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <horms+renesas@verge.net.au> > >--- > > 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-<soctype>", "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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 09, 2015 at 04:48:47AM +0000, 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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Dec 09, 2015 at 06:29:07AM +0000, 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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-<soctype>", "renesas,rcar-usbhs" as fallback. - "renesas,usbhs-r8a7790" - "renesas,usbhs-r8a7791" - "renesas,usbhs-r8a7794" @@ -22,7 +22,7 @@ Optional properties: Example: usbhs: usb@e6590000 { - compatible = "renesas,usbhs-r8a7790"; + compatible = "renesas,usbhs-r8a7790", "renesas,rcar-usbhs"; reg = <0 0xe6590000 0 0x100>; interrupts = <0 107 IRQ_TYPE_LEVEL_HIGH>; clocks = <&mstp7_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);
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 <horms+renesas@verge.net.au> --- Documentation/devicetree/bindings/usb/renesas_usbhs.txt | 4 ++-- drivers/usb/renesas_usbhs/common.c | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-)