| Message ID | 20200423122448.8099-2-wsa+renesas@sang-engineering.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: renesas_sdhi: avoid bad TAPs for Gen3 | expand |
Hi Wolfram, Thanks for your patch! On Thu, Apr 23, 2020 at 2:26 PM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > For ES1.2, add a comment explaining the situation. For ES1.3 (and > later, although unlikely), add a new entry defining it as 4tap. Usually we don't add soc_device_match quirks for unknown future revisions. > --- a/drivers/mmc/host/renesas_sdhi_core.c > +++ b/drivers/mmc/host/renesas_sdhi_core.c > @@ -711,11 +711,17 @@ static const struct renesas_sdhi_quirks sdhi_quirks_nohs400 = { > .hs400_disabled = true, > }; > > +/* > + * Note for r8a7796 / r8a774a1: we can't distinguish ES1.1 and 1.2 as of now. > + * So, we want to treat them equally and only have a match for ES1.2 to enforce > + * this if there ever will be a way to distinguish ES1.2. > + */ > static const struct soc_device_attribute sdhi_quirks_match[] = { > { .soc_id = "r8a774a1", .revision = "ES1.[012]", .data = &sdhi_quirks_4tap_nohs400 }, > { .soc_id = "r8a7795", .revision = "ES1.*", .data = &sdhi_quirks_4tap_nohs400 }, > { .soc_id = "r8a7795", .revision = "ES2.0", .data = &sdhi_quirks_4tap }, > { .soc_id = "r8a7796", .revision = "ES1.[012]", .data = &sdhi_quirks_4tap_nohs400 }, > + { .soc_id = "r8a7796", .revision = "ES1.*", .data = &sdhi_quirks_4tap }, R-Car M3-N is r8a77965, not r8a7796? > { .soc_id = "r8a77980", .data = &sdhi_quirks_nohs400 }, > { /* Sentinel. */ }, > }; Gr{oetje,eeting}s, Geert
> Usually we don't add soc_device_match quirks for unknown future revisions. I was just following... > > > --- a/drivers/mmc/host/renesas_sdhi_core.c > > +++ b/drivers/mmc/host/renesas_sdhi_core.c > > @@ -711,11 +711,17 @@ static const struct renesas_sdhi_quirks sdhi_quirks_nohs400 = { > > .hs400_disabled = true, > > }; > > > > +/* > > + * Note for r8a7796 / r8a774a1: we can't distinguish ES1.1 and 1.2 as of now. > > + * So, we want to treat them equally and only have a match for ES1.2 to enforce > > + * this if there ever will be a way to distinguish ES1.2. > > + */ > > static const struct soc_device_attribute sdhi_quirks_match[] = { > > { .soc_id = "r8a774a1", .revision = "ES1.[012]", .data = &sdhi_quirks_4tap_nohs400 }, > > { .soc_id = "r8a7795", .revision = "ES1.*", .data = &sdhi_quirks_4tap_nohs400 }, ... this example here. This also applies to all future versions (not that there will be any), no? > > { .soc_id = "r8a7795", .revision = "ES2.0", .data = &sdhi_quirks_4tap }, > > { .soc_id = "r8a7796", .revision = "ES1.[012]", .data = &sdhi_quirks_4tap_nohs400 }, > > + { .soc_id = "r8a7796", .revision = "ES1.*", .data = &sdhi_quirks_4tap }, > > R-Car M3-N is r8a77965, not r8a7796? Right. $subject should be M3-W :(
Hi Wolfram, On Thu, Apr 23, 2020 at 2:39 PM Wolfram Sang <wsa@the-dreams.de> wrote: > > Usually we don't add soc_device_match quirks for unknown future revisions. > > I was just following... > > > > --- a/drivers/mmc/host/renesas_sdhi_core.c > > > +++ b/drivers/mmc/host/renesas_sdhi_core.c > > > @@ -711,11 +711,17 @@ static const struct renesas_sdhi_quirks sdhi_quirks_nohs400 = { > > > .hs400_disabled = true, > > > }; > > > > > > +/* > > > + * Note for r8a7796 / r8a774a1: we can't distinguish ES1.1 and 1.2 as of now. > > > + * So, we want to treat them equally and only have a match for ES1.2 to enforce > > > + * this if there ever will be a way to distinguish ES1.2. > > > + */ > > > static const struct soc_device_attribute sdhi_quirks_match[] = { > > > { .soc_id = "r8a774a1", .revision = "ES1.[012]", .data = &sdhi_quirks_4tap_nohs400 }, > > > { .soc_id = "r8a7795", .revision = "ES1.*", .data = &sdhi_quirks_4tap_nohs400 }, > > ... this example here. This also applies to all future versions (not > that there will be any), no? Ah, but r8a7795 already moved to ES3.0, so it's very unlikely a newer ES1.x will be made. Hence "ES1.*" is assumed to cover all known existing ES1.x revisions. For M3-N, we're still at ES1.x, AFAIK. > > > { .soc_id = "r8a7795", .revision = "ES2.0", .data = &sdhi_quirks_4tap }, > > > { .soc_id = "r8a7796", .revision = "ES1.[012]", .data = &sdhi_quirks_4tap_nohs400 }, > > > + { .soc_id = "r8a7796", .revision = "ES1.*", .data = &sdhi_quirks_4tap }, > > > > R-Car M3-N is r8a77965, not r8a7796? > > Right. $subject should be M3-W :( OK, that changes my point of view, as M3-W is also at ES3.0 (aka M3-W+ ;-), so using "ES1.*" is fine. Gr{oetje,eeting}s, Geert
> OK, that changes my point of view, as M3-W is also at ES3.0 (aka M3-W+ ;-), > so using "ES1.*" is fine. I agree. So, only $subject is wrong. Will fix!
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c index 28b0830c4251..33b51105c788 100644 --- a/drivers/mmc/host/renesas_sdhi_core.c +++ b/drivers/mmc/host/renesas_sdhi_core.c @@ -711,11 +711,17 @@ static const struct renesas_sdhi_quirks sdhi_quirks_nohs400 = { .hs400_disabled = true, }; +/* + * Note for r8a7796 / r8a774a1: we can't distinguish ES1.1 and 1.2 as of now. + * So, we want to treat them equally and only have a match for ES1.2 to enforce + * this if there ever will be a way to distinguish ES1.2. + */ static const struct soc_device_attribute sdhi_quirks_match[] = { { .soc_id = "r8a774a1", .revision = "ES1.[012]", .data = &sdhi_quirks_4tap_nohs400 }, { .soc_id = "r8a7795", .revision = "ES1.*", .data = &sdhi_quirks_4tap_nohs400 }, { .soc_id = "r8a7795", .revision = "ES2.0", .data = &sdhi_quirks_4tap }, { .soc_id = "r8a7796", .revision = "ES1.[012]", .data = &sdhi_quirks_4tap_nohs400 }, + { .soc_id = "r8a7796", .revision = "ES1.*", .data = &sdhi_quirks_4tap }, { .soc_id = "r8a77980", .data = &sdhi_quirks_nohs400 }, { /* Sentinel. */ }, };