RE: [PATCH RFC 3/4] mmc: host: renesas_sdhi_sys_dmac: add DMACR setting

From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
To: Ulrich Hecht <uli@fpond.eu>
Cc: "linux-renesas-soc@vger.kernel.org"  <linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH RFC 3/4] mmc: host: renesas_sdhi_sys_dmac: add DMACR setting
Date: Mon, 2 Dec 2019 08:19:44 +0000	[thread overview]
Message-ID: <TYAPR01MB454428AA3B0D7538EF98A6F2D8430@TYAPR01MB4544.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <837048895.1354220.1574765168549@webmail.strato.com>

Hi Ulrich-san,

Thank you for your review!

> From: Ulrich Hecht, Sent: Tuesday, November 26, 2019 7:46 PM
<snip>
> > +#define DMACR_SDR104		0x192
> > +#define DMACR_SDR104_32BYTE	0x0004
> > +#define DMACR_SDR50		0xe4
> > +#define DMACR_SDR50_32BYTE	0x000a
> > +#define DMACR_2_OR_4BYTE	0x0000
> > +
> 
> Could you give me a pointer to where these magic numbers are documented? In my (rather old) SDHI docs the register addresses
> don't match.

R-Car Gen2 has 2 types of SDHI controller from renesas_sdhi_core.c:
---
#define SDHI_VER_GEN2_SDR50	0x490c
<snip>
#define SDHI_VER_GEN2_SDR104	0xcb0d
---

- SDR50/104 supported controller's bus_shift will be 1, so that DMACR_SDR104 is 0x192 (actual address is 0x324).
- SDR50 supported controller's bus_shift will be 0, so that DMACR_SDR50 is 0xe6 (actual address is 0xe6).

So, I should fix the "#define DMACR_SDR50" value to 0xe6, not 0xe4 :)

Best regards,
Yoshihiro Shimoda

> >  struct renesas_sdhi_quirks {
> >  	bool hs400_disabled;
> >  	bool hs400_4taps;
> > @@ -604,6 +611,32 @@ static int renesas_sdhi_multi_io_quirk(struct mmc_card *card,
> >  	return blk_size;
> >  }
> >
> > +static void renesas_sdhi_set_dmacr(struct tmio_mmc_host *host)
> > +{
> > +	struct renesas_sdhi *priv = host_to_priv(host);
> > +	u16 val = DMACR_2_OR_4BYTE;
> > +	u16 reg;
> > +	enum dma_slave_buswidth width = priv->dma_priv.dma_buswidth;
> > +
> > +	switch (sd_ctrl_read16(host, CTL_VERSION)) {
> > +	case SDHI_VER_GEN2_SDR50:
> > +		if (width == DMA_SLAVE_BUSWIDTH_32_BYTES)
> > +			val = DMACR_SDR50_32BYTE;
> > +		reg = DMACR_SDR50;
> > +		break;
> > +	case SDHI_VER_GEN2_SDR104:
> > +		if (width == DMA_SLAVE_BUSWIDTH_32_BYTES)
> > +			val = DMACR_SDR104_32BYTE;
> > +		reg = DMACR_SDR104;
> > +		break;
> > +	default:
> > +		/* nothing to do */
> > +		return;
> > +	}
> > +
> > +	sd_ctrl_write16(host, reg, val);
> > +}
> > +
> >  static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
> >  {
> >  	/* Iff regs are 8 byte apart, sdbuf is 64 bit. Otherwise always 32. */
> > @@ -611,6 +644,8 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
> >
> >  	sd_ctrl_write16(host, CTL_DMA_ENABLE, enable ? DMA_ENABLE_DMASDRW : 0);
> >  	renesas_sdhi_sdbuf_width(host, enable ? width : 16);
> > +
> > +	renesas_sdhi_set_dmacr(host);
> >  }
> >
> >  static const struct renesas_sdhi_quirks sdhi_quirks_4tap_nohs400 = {
> > --
> > 2.7.4
> >
> 
> Assuming that the register addresses are correct,
> Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
> 
> CU
> Uli