| Message ID | 22b4c393bf5074b53791d2797d8fe74deb8ea9a7.1623315732.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | arm64: renesas: Add support for R Car H3e 2G-and M3e-2G | expand |
Hi Geert, Thank you for the patch. On Thu, Jun 10, 2021 at 11:37:17AM +0200, Geert Uytterhoeven wrote: > Add support for the R-Car H3e-2G (R8A779M1) and M3e-2G (R8A779M3) SoCs. > These are different gradings of the R-Car H3 ES3.0 (R8A77951) and M3-W+ > (R8A77961) SoCs, and thus subject to the same quirks. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > drivers/mmc/host/renesas_sdhi_core.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c > index b719eda6b8619453..0478c9d58b965392 100644 > --- a/drivers/mmc/host/renesas_sdhi_core.c > +++ b/drivers/mmc/host/renesas_sdhi_core.c > @@ -943,6 +943,8 @@ static const struct soc_device_attribute sdhi_quirks_match[] = { > { .soc_id = "r8a77965", .data = &sdhi_quirks_r8a77965 }, > { .soc_id = "r8a77980", .data = &sdhi_quirks_nohs400 }, > { .soc_id = "r8a77990", .data = &sdhi_quirks_r8a77990 }, > + { .soc_id = "r8a779m1", .data = &sdhi_quirks_bad_taps2367 }, > + { .soc_id = "r8a779m3", .data = &sdhi_quirks_bad_taps1357 }, Could we reuse the entries for H3 and M3 instead, by dropping the "ES3.*" revision ? > { /* Sentinel. */ }, > }; >
Hi Laurent, On Mon, Jun 14, 2021 at 8:42 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Thu, Jun 10, 2021 at 11:37:17AM +0200, Geert Uytterhoeven wrote: > > Add support for the R-Car H3e-2G (R8A779M1) and M3e-2G (R8A779M3) SoCs. > > These are different gradings of the R-Car H3 ES3.0 (R8A77951) and M3-W+ > > (R8A77961) SoCs, and thus subject to the same quirks. > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > --- a/drivers/mmc/host/renesas_sdhi_core.c > > +++ b/drivers/mmc/host/renesas_sdhi_core.c > > @@ -943,6 +943,8 @@ static const struct soc_device_attribute sdhi_quirks_match[] = { > > { .soc_id = "r8a77965", .data = &sdhi_quirks_r8a77965 }, > > { .soc_id = "r8a77980", .data = &sdhi_quirks_nohs400 }, > > { .soc_id = "r8a77990", .data = &sdhi_quirks_r8a77990 }, > > + { .soc_id = "r8a779m1", .data = &sdhi_quirks_bad_taps2367 }, > > + { .soc_id = "r8a779m3", .data = &sdhi_quirks_bad_taps1357 }, > > Could we reuse the entries for H3 and M3 instead, by dropping the > "ES3.*" revision ? We cannot reuse the H3 ES3.0 entry, as soc_device_match() works differently than of_machine_is_compatible(): the former doesn't consider "r8a779m1" and "r8a7795" equivalent, the latter does. Same for M3-W+ (no explicit ES3.0 there) and M3e-2G. It's a pity we still don't have a "quirk-free" SDHI version on H3 and M3-W class SoCs (waiting for ES4.0?), as that would allow us to just match on "renesas,sdhi-r8a7795" resp. "renesas,sdhi-r8a77961" through the driver's .of_match_table[] instead, which would work for H3e-2G and M3e-2G, too. Gr{oetje,eeting}s, Geert
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Tuesday, June 15, 2021 4:32 AM > > Hi Laurent, > > On Mon, Jun 14, 2021 at 8:42 PM Laurent Pinchart wrote: > > On Thu, Jun 10, 2021 at 11:37:17AM +0200, Geert Uytterhoeven wrote: > > > Add support for the R-Car H3e-2G (R8A779M1) and M3e-2G (R8A779M3) SoCs. > > > These are different gradings of the R-Car H3 ES3.0 (R8A77951) and M3-W+ > > > (R8A77961) SoCs, and thus subject to the same quirks. > > > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > --- a/drivers/mmc/host/renesas_sdhi_core.c > > > +++ b/drivers/mmc/host/renesas_sdhi_core.c > > > @@ -943,6 +943,8 @@ static const struct soc_device_attribute sdhi_quirks_match[] = { > > > { .soc_id = "r8a77965", .data = &sdhi_quirks_r8a77965 }, > > > { .soc_id = "r8a77980", .data = &sdhi_quirks_nohs400 }, > > > { .soc_id = "r8a77990", .data = &sdhi_quirks_r8a77990 }, > > > + { .soc_id = "r8a779m1", .data = &sdhi_quirks_bad_taps2367 }, > > > + { .soc_id = "r8a779m3", .data = &sdhi_quirks_bad_taps1357 }, > > > > Could we reuse the entries for H3 and M3 instead, by dropping the > > "ES3.*" revision ? > > We cannot reuse the H3 ES3.0 entry, as soc_device_match() > works differently than of_machine_is_compatible(): the former doesn't > consider "r8a779m1" and "r8a7795" equivalent, the latter does. > Same for M3-W+ (no explicit ES3.0 there) and M3e-2G. > > It's a pity we still don't have a "quirk-free" SDHI version on H3 > and M3-W class SoCs (waiting for ES4.0?), as that would allow us to > just match on "renesas,sdhi-r8a7795" resp. "renesas,sdhi-r8a77961" > through the driver's .of_match_table[] instead, which would work for > H3e-2G and M3e-2G, too. Perhaps, ES4.0 will not be released. So, we can refactor the driver's .of_match_table[] now. I investigated this a little, and it seems we need many renesas_sdhi_of_data for each SoC instead of of_rcar_gen3_compatible. But, I guess such modification is better than adding sdhi_quirks_match entries. Wolfram-san, what do you thinks? Best regards, Yoshihiro Shimoda
Hi all, > > > > --- a/drivers/mmc/host/renesas_sdhi_core.c > > > > +++ b/drivers/mmc/host/renesas_sdhi_core.c > > > > @@ -943,6 +943,8 @@ static const struct soc_device_attribute sdhi_quirks_match[] = { > > > > { .soc_id = "r8a77965", .data = &sdhi_quirks_r8a77965 }, > > > > { .soc_id = "r8a77980", .data = &sdhi_quirks_nohs400 }, > > > > { .soc_id = "r8a77990", .data = &sdhi_quirks_r8a77990 }, > > > > + { .soc_id = "r8a779m1", .data = &sdhi_quirks_bad_taps2367 }, > > > > + { .soc_id = "r8a779m3", .data = &sdhi_quirks_bad_taps1357 }, > > > > > > Could we reuse the entries for H3 and M3 instead, by dropping the > > > "ES3.*" revision ? > > > > We cannot reuse the H3 ES3.0 entry, as soc_device_match() > > works differently than of_machine_is_compatible(): the former doesn't > > consider "r8a779m1" and "r8a7795" equivalent, the latter does. > > Same for M3-W+ (no explicit ES3.0 there) and M3e-2G. > > > > It's a pity we still don't have a "quirk-free" SDHI version on H3 > > and M3-W class SoCs (waiting for ES4.0?), as that would allow us to > > just match on "renesas,sdhi-r8a7795" resp. "renesas,sdhi-r8a77961" > > through the driver's .of_match_table[] instead, which would work for > > H3e-2G and M3e-2G, too. > > Perhaps, ES4.0 will not be released. So, we can refactor the driver's > .of_match_table[] now. I investigated this a little, and it seems > we need many renesas_sdhi_of_data for each SoC instead of > of_rcar_gen3_compatible. But, I guess such modification is better > than adding sdhi_quirks_match entries. > > Wolfram-san, what do you thinks? I don't fully understand how the refactoring should look like? Is it moving 'struct renesas_sdhi_quirks' to renesas_sdhi_internal_dmac.c and merge it there with renesas_sdhi_of_data? Is it really better to copy this struct per SoC? Most of the data is the same. Thanks, Wolfram
Hi Wolfram-san, > From: Wolfram Sang, Sent: Thursday, June 24, 2021 3:15 PM > > Hi all, > > > > > > --- a/drivers/mmc/host/renesas_sdhi_core.c > > > > > +++ b/drivers/mmc/host/renesas_sdhi_core.c > > > > > @@ -943,6 +943,8 @@ static const struct soc_device_attribute sdhi_quirks_match[] = { > > > > > { .soc_id = "r8a77965", .data = &sdhi_quirks_r8a77965 }, > > > > > { .soc_id = "r8a77980", .data = &sdhi_quirks_nohs400 }, > > > > > { .soc_id = "r8a77990", .data = &sdhi_quirks_r8a77990 }, > > > > > + { .soc_id = "r8a779m1", .data = &sdhi_quirks_bad_taps2367 }, > > > > > + { .soc_id = "r8a779m3", .data = &sdhi_quirks_bad_taps1357 }, > > > > > > > > Could we reuse the entries for H3 and M3 instead, by dropping the > > > > "ES3.*" revision ? > > > > > > We cannot reuse the H3 ES3.0 entry, as soc_device_match() > > > works differently than of_machine_is_compatible(): the former doesn't > > > consider "r8a779m1" and "r8a7795" equivalent, the latter does. > > > Same for M3-W+ (no explicit ES3.0 there) and M3e-2G. > > > > > > It's a pity we still don't have a "quirk-free" SDHI version on H3 > > > and M3-W class SoCs (waiting for ES4.0?), as that would allow us to > > > just match on "renesas,sdhi-r8a7795" resp. "renesas,sdhi-r8a77961" > > > through the driver's .of_match_table[] instead, which would work for > > > H3e-2G and M3e-2G, too. > > > > Perhaps, ES4.0 will not be released. So, we can refactor the driver's > > .of_match_table[] now. I investigated this a little, and it seems > > we need many renesas_sdhi_of_data for each SoC instead of > > of_rcar_gen3_compatible. But, I guess such modification is better > > than adding sdhi_quirks_match entries. > > > > Wolfram-san, what do you thinks? > > I don't fully understand how the refactoring should look like? Is it > moving 'struct renesas_sdhi_quirks' to renesas_sdhi_internal_dmac.c and > merge it there with renesas_sdhi_of_data? Is it really better to copy > this struct per SoC? Most of the data is the same. I also have the same concern. But, I guess we can refactor the renesas_sdhi_of_data like below to avoid increasing data size: struct renesas_sdhi_of_data_with_quirks { const struct renesas_sdhi_of_data *of_data; const struct renesas_sdhi_quirks *quirks; }; And then, we can keep of_rcar_gen3_compatible and we can add each SoC's renesas_sdhi_of_data_with_quirks and set it to the .data. Best regards, Yoshihiro Shimoda > Thanks, > > Wolfram
> > I don't fully understand how the refactoring should look like? Is it > > moving 'struct renesas_sdhi_quirks' to renesas_sdhi_internal_dmac.c and > > merge it there with renesas_sdhi_of_data? Is it really better to copy > > this struct per SoC? Most of the data is the same. > > I also have the same concern. But, I guess we can refactor > the renesas_sdhi_of_data like below to avoid increasing data size: > > struct renesas_sdhi_of_data_with_quirks { > const struct renesas_sdhi_of_data *of_data; > const struct renesas_sdhi_quirks *quirks; > }; > > And then, we can keep of_rcar_gen3_compatible and > we can add each SoC's renesas_sdhi_of_data_with_quirks > and set it to the .data. That sounds like a reasonable approach to me. This would also allow us to merge the quirks from sdhi_core with the quirks from sdhi_internal_dmac.
Hi Wolfram-san, > From: Wolfram Sang, Sent: Friday, June 25, 2021 12:20 AM > > > > I don't fully understand how the refactoring should look like? Is it > > > moving 'struct renesas_sdhi_quirks' to renesas_sdhi_internal_dmac.c and > > > merge it there with renesas_sdhi_of_data? Is it really better to copy > > > this struct per SoC? Most of the data is the same. > > > > I also have the same concern. But, I guess we can refactor > > the renesas_sdhi_of_data like below to avoid increasing data size: > > > > struct renesas_sdhi_of_data_with_quirks { > > const struct renesas_sdhi_of_data *of_data; > > const struct renesas_sdhi_quirks *quirks; > > }; > > > > And then, we can keep of_rcar_gen3_compatible and > > we can add each SoC's renesas_sdhi_of_data_with_quirks > > and set it to the .data. > > That sounds like a reasonable approach to me. This would also allow us > to merge the quirks from sdhi_core with the quirks from > sdhi_internal_dmac. Thank you for your reply! So, I'll try to make such a patch. Best regards, Yoshihiro Shimoda
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c index b719eda6b8619453..0478c9d58b965392 100644 --- a/drivers/mmc/host/renesas_sdhi_core.c +++ b/drivers/mmc/host/renesas_sdhi_core.c @@ -943,6 +943,8 @@ static const struct soc_device_attribute sdhi_quirks_match[] = { { .soc_id = "r8a77965", .data = &sdhi_quirks_r8a77965 }, { .soc_id = "r8a77980", .data = &sdhi_quirks_nohs400 }, { .soc_id = "r8a77990", .data = &sdhi_quirks_r8a77990 }, + { .soc_id = "r8a779m1", .data = &sdhi_quirks_bad_taps2367 }, + { .soc_id = "r8a779m3", .data = &sdhi_quirks_bad_taps1357 }, { /* Sentinel. */ }, };
Add support for the R-Car H3e-2G (R8A779M1) and M3e-2G (R8A779M3) SoCs. These are different gradings of the R-Car H3 ES3.0 (R8A77951) and M3-W+ (R8A77961) SoCs, and thus subject to the same quirks. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- drivers/mmc/host/renesas_sdhi_core.c | 2 ++ 1 file changed, 2 insertions(+)