diff mbox series

[6/6] mmc: renesas_sdhi: make 'dmac_only_one_rx' a quirk

Message ID 20220320123016.57991-7-wsa+renesas@sang-engineering.com
State New
Headers show
Series mmc: renesas_sdhi: internal_dmac: updates after refactoring | expand

Commit Message

Wolfram Sang March 20, 2022, 12:30 p.m. UTC 
After Shimoda-san's much appreciated refactoring of the quirk handling,
we can convert now 'dmac_only_one_rx' from an ugly global flag to a
regular quirk. This makes quirk handling more consistent and easier to
maintain. After this patch, soc_dma_quirks is completely gone, hooray!

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/mmc/host/renesas_sdhi.h               |  1 +
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 28 ++++++++-----------
 2 files changed, 12 insertions(+), 17 deletions(-)

Comments

Geert Uytterhoeven April 1, 2022, 2:13 p.m. UTC | #1 
Hi Wolfram,

On Mon, Mar 21, 2022 at 6:33 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> After Shimoda-san's much appreciated refactoring of the quirk handling,
> we can convert now 'dmac_only_one_rx' from an ugly global flag to a
> regular quirk. This makes quirk handling more consistent and easier to
> maintain. After this patch, soc_dma_quirks is completely gone, hooray!
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Thanks for your patch!

> --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> @@ -372,7 +378,7 @@ renesas_sdhi_internal_dmac_start_dma(struct tmio_mmc_host *host,
>
>         if (data->flags & MMC_DATA_READ) {
>                 dtran_mode |= DTRAN_MODE_CH_NUM_CH1;
> -               if (test_bit(SDHI_INTERNAL_DMAC_ONE_RX_ONLY, &global_flags) &&
> +               if (priv->quirks->dma_one_rx_only &&

If there are no quirks (yes, we do have such systems ;-), this will
crash with a NULL-pointer dereference.  Actually patch 5/6 has the
same problem.

As I could reproduce the issue, and Ulf has already applied
this series, I've sent a patch:
https://lore.kernel.org/r/cc3178c2ff60f640f4d5a071d51f6b0b1db37656.1648822020.git.geert+renesas@glider.be

>                     test_and_set_bit(SDHI_INTERNAL_DMAC_RX_IN_USE, &global_flags))
>                         goto force_pio_with_unmap;
>         } else {

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Wolfram Sang April 1, 2022, 3 p.m. UTC | #2 
> If there are no quirks (yes, we do have such systems ;-), this will

Wow, seems that slipped my reality :D Thanks for catching!
diff mbox series

Patch

diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
index e6b1395e99a3..1a1e3e020a8c 100644
--- a/drivers/mmc/host/renesas_sdhi.h
+++ b/drivers/mmc/host/renesas_sdhi.h
@@ -42,6 +42,7 @@  struct renesas_sdhi_quirks {
 	bool hs400_disabled;
 	bool hs400_4taps;
 	bool fixed_addr_mode;
+	bool dma_one_rx_only;
 	u32 hs400_bad_taps;
 	const u8 (*hs400_calib_table)[SDHI_CALIB_TABLE_MAX];
 };
diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index 4d8df61657cd..1497a46260d4 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -78,8 +78,7 @@  static unsigned long global_flags;
  * stored into the system memory even if the DMAC interrupt happened.
  * So, this driver then uses one RX DMAC channel only.
  */
-#define SDHI_INTERNAL_DMAC_ONE_RX_ONLY	0
-#define SDHI_INTERNAL_DMAC_RX_IN_USE	1
+#define SDHI_INTERNAL_DMAC_RX_IN_USE	0
 
 /* Definitions for sampling clocks */
 static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = {
@@ -162,6 +161,12 @@  static const struct renesas_sdhi_quirks sdhi_quirks_4tap_nohs400 = {
 	.hs400_4taps = true,
 };
 
+static const struct renesas_sdhi_quirks sdhi_quirks_4tap_nohs400_one_rx = {
+	.hs400_disabled = true,
+	.hs400_4taps = true,
+	.dma_one_rx_only = true,
+};
+
 static const struct renesas_sdhi_quirks sdhi_quirks_4tap = {
 	.hs400_4taps = true,
 	.hs400_bad_taps = BIT(2) | BIT(3) | BIT(6) | BIT(7),
@@ -205,9 +210,10 @@  static const struct renesas_sdhi_quirks sdhi_quirks_r8a77990 = {
  */
 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 = "ES1.*", .data = &sdhi_quirks_4tap_nohs400_one_rx },
 	{ .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.0", .data = &sdhi_quirks_4tap_nohs400_one_rx },
+	{ .soc_id = "r8a7796", .revision = "ES1.[12]", .data = &sdhi_quirks_4tap_nohs400 },
 	{ .soc_id = "r8a7796", .revision = "ES1.*", .data = &sdhi_quirks_r8a7796_es13 },
 	{ /* Sentinel. */ }
 };
@@ -372,7 +378,7 @@  renesas_sdhi_internal_dmac_start_dma(struct tmio_mmc_host *host,
 
 	if (data->flags & MMC_DATA_READ) {
 		dtran_mode |= DTRAN_MODE_CH_NUM_CH1;
-		if (test_bit(SDHI_INTERNAL_DMAC_ONE_RX_ONLY, &global_flags) &&
+		if (priv->quirks->dma_one_rx_only &&
 		    test_and_set_bit(SDHI_INTERNAL_DMAC_RX_IN_USE, &global_flags))
 			goto force_pio_with_unmap;
 	} else {
@@ -524,14 +530,6 @@  static const struct tmio_mmc_dma_ops renesas_sdhi_internal_dmac_dma_ops = {
 	.end = renesas_sdhi_internal_dmac_end_dma,
 };
 
-static const struct soc_device_attribute soc_dma_quirks[] = {
-	{ .soc_id = "r8a7795", .revision = "ES1.*",
-	  .data = (void *)BIT(SDHI_INTERNAL_DMAC_ONE_RX_ONLY) },
-	{ .soc_id = "r8a7796", .revision = "ES1.0",
-	  .data = (void *)BIT(SDHI_INTERNAL_DMAC_ONE_RX_ONLY) },
-	{ /* sentinel */ }
-};
-
 static int renesas_sdhi_internal_dmac_probe(struct platform_device *pdev)
 {
 	const struct soc_device_attribute *attr;
@@ -542,10 +540,6 @@  static int renesas_sdhi_internal_dmac_probe(struct platform_device *pdev)
 	of_data_quirks = of_device_get_match_data(&pdev->dev);
 	quirks = of_data_quirks->quirks;
 
-	attr = soc_device_match(soc_dma_quirks);
-	if (attr)
-		global_flags |= (unsigned long)attr->data;
-
 	attr = soc_device_match(sdhi_quirks_match);
 	if (attr)
 		quirks = attr->data;