| Message ID | 20181010150642.73890-3-chris.brandt@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | mmc: tmio_mmc: Add support for RZ/A2 | expand |
Hi Chris, > +/* RZ/A2 does not have the ADRR_MODE bit */ > +#define SDHI_INTERNAL_DMAC_ADRR_MODE_FIXED 2 First, there is a typo: s/ADRR/ADDR/g Also, I think it would make the code much more comprehensible if this macro was named SDHI_INTERNAL_DMAC_ADDR_MODE_BROKEN. Or maybe SDHI_INTERNAL_DMAC_FIXED_ADDR_MODE_BROKEN. Currently, on first read I thought this mode was fixed on this SoC and broken on all the others and was confused. What do you think? Rest looks okay to me! Regards, Wolfram
Hi Wolfram, On Friday, October 12, 2018, Wolfram Sang wrote: > > +/* RZ/A2 does not have the ADRR_MODE bit */ > > +#define SDHI_INTERNAL_DMAC_ADRR_MODE_FIXED 2 > > First, there is a typo: s/ADRR/ADDR/g Thanks! Even after you pointed that out...I had to stare real hard to see it. I guess the brain corrects what you see. > Also, I think it would make the code much more comprehensible if this > macro was named SDHI_INTERNAL_DMAC_ADDR_MODE_BROKEN. Or maybe > SDHI_INTERNAL_DMAC_FIXED_ADDR_MODE_BROKEN. Currently, on first read I > thought this mode was fixed on this SoC and broken on all the others and > was confused. To be honest, I was not able to fully understand the issue in detail. I was getting information through someone that was not in the design team. So to be fair to the chip design guys, I want to avoid the word "broken". They took the bit out of the hardware manual, and since the HW was designed to work either way (and the default register setting is for fixed), I consider it an 'unsupported feature'. So, how about a compromise of: #define SDHI_INTERNAL_DMAC_ADDR_MODE_FIXED_ONLY 2 From your other email: > > As you can imagine, it does have this bit. And it worked fine from me. > > But the chip guys said they found something not right with it, so they > > removed it from the v1.0 Hardware Manual. > > Do you happen to know if this applies for Gen3 SDHI as well? I don't think this applies to Gen3. This is just a RZ/A2 thing. With that said, there are some Gen3 HW bugs that are in RZ/A2 HW, like the QSPI read bug. Since the RZ/A2 has the exact same HW as Gen3, it has the same HW bug. Now that one is a "bug", and I don't mind calling that one broken. Chris
> Even after you pointed that out...I had to stare real hard to see it. I > guess the brain corrects what you see. Yes. c57d3e7a9391 ("i2c-dev: Fix typo in ioctl name reference") fixed something in 2015 which was around forever. I had to look twice at this patch as well. > I was getting information through someone that was not in the design > team. So to be fair to the chip design guys, I want to avoid the word > "broken". They took the bit out of the hardware manual, and since the > HW was designed to work either way (and the default register setting > is for fixed), I consider it an 'unsupported feature'. > > So, how about a compromise of: > > #define SDHI_INTERNAL_DMAC_ADDR_MODE_FIXED_ONLY 2 Perfect. Now we nailed it, I think. > I don't think this applies to Gen3. This is just a RZ/A2 thing. OK. Thanks for the heads up!
diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig index cf984f0f0246..b69d5cd45d0f 100644 --- a/drivers/mmc/host/Kconfig +++ b/drivers/mmc/host/Kconfig @@ -636,13 +636,14 @@ config MMC_SDHI_SYS_DMAC config MMC_SDHI_INTERNAL_DMAC tristate "DMA for SDHI SD/SDIO controllers using on-chip bus mastering" - depends on ARM64 || COMPILE_TEST + depends on ARM64 || ARCH_R7S9210 || COMPILE_TEST depends on MMC_SDHI default MMC_SDHI if ARM64 help This provides DMA support for SDHI SD/SDIO controllers using on-chip bus mastering. This supports the controllers - found in arm64 based SoCs. + found in arm64 based SoCs. This controller is also found in + RZ/A2 series SoCs. config MMC_UNIPHIER tristate "UniPhier SD/eMMC Host Controller support" diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c index e5e5015ca680..e3c4fa764609 100644 --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c @@ -34,7 +34,7 @@ #define DTRAN_MODE_CH_NUM_CH0 0 /* "downstream" = for write commands */ #define DTRAN_MODE_CH_NUM_CH1 BIT(16) /* "upstream" = for read commands */ #define DTRAN_MODE_BUS_WIDTH (BIT(5) | BIT(4)) -#define DTRAN_MODE_ADDR_MODE BIT(0) /* 1 = Increment address */ +#define DTRAN_MODE_ADDR_MODE BIT(0) /* 1 = Increment address, 0 = Fixed */ /* DM_CM_DTRAN_CTRL */ #define DTRAN_CTRL_DM_START BIT(0) @@ -73,6 +73,9 @@ static unsigned long global_flags; #define SDHI_INTERNAL_DMAC_ONE_RX_ONLY 0 #define SDHI_INTERNAL_DMAC_RX_IN_USE 1 +/* RZ/A2 does not have the ADRR_MODE bit */ +#define SDHI_INTERNAL_DMAC_ADRR_MODE_FIXED 2 + /* Definitions for sampling clocks */ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = { { @@ -81,6 +84,21 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = { }, }; +static const struct renesas_sdhi_of_data of_rza2_compatible = { + .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL | + TMIO_MMC_HAVE_CBSY, + .tmio_ocr_mask = MMC_VDD_32_33, + .capabilities = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ | + MMC_CAP_CMD23, + .bus_shift = 2, + .scc_offset = 0 - 0x1000, + .taps = rcar_gen3_scc_taps, + .taps_num = ARRAY_SIZE(rcar_gen3_scc_taps), + /* DMAC can handle 0xffffffff blk count but only 1 segment */ + .max_blk_count = 0xffffffff, + .max_segs = 1, +}; + static const struct renesas_sdhi_of_data of_rcar_r8a7795_compatible = { .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL | TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2 | @@ -113,6 +131,7 @@ static const struct renesas_sdhi_of_data of_rcar_gen3_compatible = { }; static const struct of_device_id renesas_sdhi_internal_dmac_of_match[] = { + { .compatible = "renesas,sdhi-r7s9210", .data = &of_rza2_compatible, }, { .compatible = "renesas,sdhi-r8a7795", .data = &of_rcar_r8a7795_compatible, }, { .compatible = "renesas,sdhi-r8a7796", .data = &of_rcar_r8a7795_compatible, }, { .compatible = "renesas,rcar-gen3-sdhi", .data = &of_rcar_gen3_compatible, }, @@ -171,7 +190,10 @@ renesas_sdhi_internal_dmac_start_dma(struct tmio_mmc_host *host, struct mmc_data *data) { struct scatterlist *sg = host->sg_ptr; - u32 dtran_mode = DTRAN_MODE_BUS_WIDTH | DTRAN_MODE_ADDR_MODE; + u32 dtran_mode = DTRAN_MODE_BUS_WIDTH; + + if (!test_bit(SDHI_INTERNAL_DMAC_ADRR_MODE_FIXED, &global_flags)) + dtran_mode |= DTRAN_MODE_ADDR_MODE; if (!dma_map_sg(&host->pdev->dev, sg, host->sg_len, mmc_get_dma_dir(data))) @@ -290,6 +312,8 @@ static const struct tmio_mmc_dma_ops renesas_sdhi_internal_dmac_dma_ops = { */ static const struct soc_device_attribute gen3_soc_whitelist[] = { /* specific ones */ + { .soc_id = "r7s9210", + .data = (void *)BIT(SDHI_INTERNAL_DMAC_ADRR_MODE_FIXED) }, { .soc_id = "r8a7795", .revision = "ES1.*", .data = (void *)BIT(SDHI_INTERNAL_DMAC_ONE_RX_ONLY) }, { .soc_id = "r8a7796", .revision = "ES1.0",
The SDHI/MMC controller in the RZ/A2 is almost the same as R-Car gen3, but with some minor differences. Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- v3: * Removed extra space in Kconfig * Removed unneeded parentheses v2: * Made comment clearer --- drivers/mmc/host/Kconfig | 5 +++-- drivers/mmc/host/renesas_sdhi_internal_dmac.c | 28 +++++++++++++++++++++++++-- 2 files changed, 29 insertions(+), 4 deletions(-)