| Message ID | 1525031296-16512-3-git-send-email-ykaneko0929@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On Mon, Apr 30, 2018 at 04:48:15AM +0900, Yoshihiro Kaneko wrote: > From: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com> > > This patch adds r8a77965 support in SDHI. > > Signed-off-by: Masaharu Hayakawa <masaharu.hayakawa.ry@renesas.com> > Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com> > --- > Documentation/devicetree/bindings/mmc/tmio_mmc.txt | 1 + > drivers/mmc/host/renesas_sdhi_internal_dmac.c | 2 ++ > 2 files changed, 3 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > index ba38252..ee978c9 100644 > --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt > @@ -26,6 +26,7 @@ Required properties: > "renesas,sdhi-r8a7794" - SDHI IP on R8A7794 SoC > "renesas,sdhi-r8a7795" - SDHI IP on R8A7795 SoC > "renesas,sdhi-r8a7796" - SDHI IP on R8A7796 SoC > + "renesas,sdhi-r8a77965" - SDHI IP on R8A77965 SoC > "renesas,sdhi-r8a77980" - SDHI IP on R8A77980 SoC > "renesas,sdhi-r8a77995" - SDHI IP on R8A77995 SoC > "renesas,sdhi-shmobile" - a generic sh-mobile SDHI controller > diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c > index a6bf123..733ea8e 100644 > --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c > +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c > @@ -99,6 +99,7 @@ > static const struct of_device_id renesas_sdhi_internal_dmac_of_match[] = { > { .compatible = "renesas,sdhi-r8a7795", .data = &of_rcar_gen3_compatible, }, > { .compatible = "renesas,sdhi-r8a7796", .data = &of_rcar_gen3_compatible, }, > + { .compatible = "renesas,sdhi-r8a77965", .data = &of_rcar_gen3_compatible, }, > { .compatible = "renesas,rcar-gen3-sdhi", .data = &of_rcar_gen3_compatible, }, > {}, > }; > @@ -276,6 +277,7 @@ static void renesas_sdhi_internal_dmac_complete_tasklet_fn(unsigned long arg) > /* generic ones */ > { .soc_id = "r8a7795" }, > { .soc_id = "r8a7796" }, > + { .soc_id = "r8a77965", .revision = "ES1.0" }, I think we can drop .revision = "ES1.0" to be in keeping with 349936fcdaf8 ("mmc: renesas_sdhi_internal_dmac: use more generic whitelisting"). With that fixed: Reviewed-by: Simon Horman <horms+renesas@verge.net.au> > { .soc_id = "r8a77980" }, > { .soc_id = "r8a77995" }, > { /* sentinel */ } > -- > 1.9.1 >
> > + { .compatible = "renesas,sdhi-r8a77965", .data = &of_rcar_gen3_compatible, }, Do we need this line... > > { .compatible = "renesas,rcar-gen3-sdhi", .data = &of_rcar_gen3_compatible, }, ... with this generic fallback in place? > > @@ -276,6 +277,7 @@ static void renesas_sdhi_internal_dmac_complete_tasklet_fn(unsigned long arg) > > /* generic ones */ > > { .soc_id = "r8a7795" }, > > { .soc_id = "r8a7796" }, > > + { .soc_id = "r8a77965", .revision = "ES1.0" }, > > I think we can drop .revision = "ES1.0" > > to be in keeping with 349936fcdaf8 ("mmc: renesas_sdhi_internal_dmac: use > more generic whitelisting"). Ack.
On Wed, May 02, 2018 at 07:32:19AM +0200, Wolfram Sang wrote: > > > > + { .compatible = "renesas,sdhi-r8a77965", .data = &of_rcar_gen3_compatible, }, > > Do we need this line... > > > > { .compatible = "renesas,rcar-gen3-sdhi", .data = &of_rcar_gen3_compatible, }, > > ... with this generic fallback in place? Sorry, I missed that in my review. I agree that the renesas,sdhi-r8a77965 is not needed. > > > > @@ -276,6 +277,7 @@ static void renesas_sdhi_internal_dmac_complete_tasklet_fn(unsigned long arg) > > > /* generic ones */ > > > { .soc_id = "r8a7795" }, > > > { .soc_id = "r8a7796" }, > > > + { .soc_id = "r8a77965", .revision = "ES1.0" }, > > > > I think we can drop .revision = "ES1.0" > > > > to be in keeping with 349936fcdaf8 ("mmc: renesas_sdhi_internal_dmac: use > > more generic whitelisting"). > > Ack. One more consideration. With the current state of the driver this patch should be fine, modulo the changes suggested above. But once HS400 support is merged some logic will be required to disable that feature for the r8a77965 until HS400 support for that SoC is explicitly added. Or conversely, perhaps when HS400 is added it should only be enabled in the driver for SoCs that are known to work: r8a7796 and r8a7795. In the case of the latter perhaps only ES2.0. Wolfram, what do you think?
> With the current state of the driver this patch should be fine, > modulo the changes suggested above. But once HS400 support is merged > some logic will be required to disable that feature for the r8a77965 > until HS400 support for that SoC is explicitly added. > > Or conversely, perhaps when HS400 is added it should only be enabled > in the driver for SoCs that are known to work: r8a7796 and r8a7795. In the > case of the latter perhaps only ES2.0. > > Wolfram, what do you think? M3-N (and future SoCs as it seems) has 8 taps while the others have 4. Maybe we should have something already in place to distinguish 8 taps and 4 taps and leave the 8 taps part for "to be added later"?
Hello, 2018-05-02 17:14 GMT+09:00 Wolfram Sang <wsa@the-dreams.de>: > >> With the current state of the driver this patch should be fine, >> modulo the changes suggested above. But once HS400 support is merged >> some logic will be required to disable that feature for the r8a77965 >> until HS400 support for that SoC is explicitly added. >> >> Or conversely, perhaps when HS400 is added it should only be enabled >> in the driver for SoCs that are known to work: r8a7796 and r8a7795. In the >> case of the latter perhaps only ES2.0. >> >> Wolfram, what do you think? > > M3-N (and future SoCs as it seems) has 8 taps while the others have 4. > Maybe we should have something already in place to distinguish 8 taps > and 4 taps and leave the 8 taps part for "to be added later"? > Thanks for your review. I will drop .revision = "ES1.0" and renesas,sdhi-r8a77965 in V2. Best regards, Kaneko
On Wed, May 02, 2018 at 10:14:15AM +0200, Wolfram Sang wrote: > > > With the current state of the driver this patch should be fine, > > modulo the changes suggested above. But once HS400 support is merged > > some logic will be required to disable that feature for the r8a77965 > > until HS400 support for that SoC is explicitly added. > > > > Or conversely, perhaps when HS400 is added it should only be enabled > > in the driver for SoCs that are known to work: r8a7796 and r8a7795. In the > > case of the latter perhaps only ES2.0. > > > > Wolfram, what do you think? > > M3-N (and future SoCs as it seems) has 8 taps while the others have 4. > Maybe we should have something already in place to distinguish 8 taps > and 4 taps and leave the 8 taps part for "to be added later"? Yes, that is one option.
diff --git a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt index ba38252..ee978c9 100644 --- a/Documentation/devicetree/bindings/mmc/tmio_mmc.txt +++ b/Documentation/devicetree/bindings/mmc/tmio_mmc.txt @@ -26,6 +26,7 @@ Required properties: "renesas,sdhi-r8a7794" - SDHI IP on R8A7794 SoC "renesas,sdhi-r8a7795" - SDHI IP on R8A7795 SoC "renesas,sdhi-r8a7796" - SDHI IP on R8A7796 SoC + "renesas,sdhi-r8a77965" - SDHI IP on R8A77965 SoC "renesas,sdhi-r8a77980" - SDHI IP on R8A77980 SoC "renesas,sdhi-r8a77995" - SDHI IP on R8A77995 SoC "renesas,sdhi-shmobile" - a generic sh-mobile SDHI controller diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c index a6bf123..733ea8e 100644 --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c @@ -99,6 +99,7 @@ static const struct of_device_id renesas_sdhi_internal_dmac_of_match[] = { { .compatible = "renesas,sdhi-r8a7795", .data = &of_rcar_gen3_compatible, }, { .compatible = "renesas,sdhi-r8a7796", .data = &of_rcar_gen3_compatible, }, + { .compatible = "renesas,sdhi-r8a77965", .data = &of_rcar_gen3_compatible, }, { .compatible = "renesas,rcar-gen3-sdhi", .data = &of_rcar_gen3_compatible, }, {}, }; @@ -276,6 +277,7 @@ static void renesas_sdhi_internal_dmac_complete_tasklet_fn(unsigned long arg) /* generic ones */ { .soc_id = "r8a7795" }, { .soc_id = "r8a7796" }, + { .soc_id = "r8a77965", .revision = "ES1.0" }, { .soc_id = "r8a77980" }, { .soc_id = "r8a77995" }, { /* sentinel */ }