On Wed, 21 Nov 2018 09:52:08 +0100 Lukasz Majewski <lu...@denx.de> wrote:
> On Wed, 07 Nov 2018 16:03:08 +0100
> Marek Szyprowski <m.szyprow...@samsung.com> wrote:
>
> > From: Lukasz Majewski <l.majew...@samsung.com>
> >
> > DW-MMC module in Samsung Exynos5433 requires 64bit DMA descriptors.
> > This patch adds code for handling them.
> >
> > Signed-off-by: Lukasz Majewski <l.majew...@samsung.com>
> > [extracted from old sources and adapted to mainline u-boot, minor
> > fixes] Signed-off-by: Marek Szyprowski <m.szyprow...@samsung.com>
> > ---
> > drivers/mmc/dw_mmc.c | 53
> > ++++++++++++++++++++++++++++++------- drivers/mmc/exynos_dw_mmc.c |
> > 6 +++++ include/dwmmc.h | 25 +++++++++++++++++
> > 3 files changed, 74 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> > index 3c702b3ed8..f50eb29ac9 100644
> > --- a/drivers/mmc/dw_mmc.c
> > +++ b/drivers/mmc/dw_mmc.c
> > @@ -30,8 +30,8 @@ static int dwmci_wait_reset(struct dwmci_host
> > *host, u32 value) return 0;
> > }
> >
> > -static void dwmci_set_idma_desc(struct dwmci_idmac *idmac,
> > - u32 desc0, u32 desc1, u32 desc2)
> > +static void dwmci_set_idma_desc_32bit(void *idmac,
> > + u32 desc0, u32 desc1, u32
> > desc2) {
> > struct dwmci_idmac *desc = idmac;
> >
> > @@ -41,11 +41,27 @@ static void dwmci_set_idma_desc(struct
> > dwmci_idmac *idmac, desc->next_addr = (ulong)desc + sizeof(struct
> > dwmci_idmac); }
> >
> > +static void dwmci_set_idma_desc_64bit(void *idmac,
> > + u32 desc0, u32 desc1, u32
> > desc2) +{
> > + struct dwmci_idmac_64addr *desc = idmac;
> > +
> > + desc->flags = desc0;
> > + desc->_res1 = 0;
> > + desc->cnt = desc1;
> > + desc->_res2 = 0;
> > + desc->addrl = desc2;
> > + desc->addrh = 0;
> > + desc->next_addrl = (ulong)desc + sizeof(struct
> > dwmci_idmac_64addr);
> > + desc->next_addrh = 0;
> > +}
> > +
> > static void dwmci_prepare_data(struct dwmci_host *host,
> > struct mmc_data *data,
> > - struct dwmci_idmac *cur_idmac,
> > + struct dwmci_idmac_64addr
> > *cur_idmac64, void *bounce_buffer)
>
> This looks strange - why the core dwmci_prepare_data() has this
> change?
>
> Above, the *idmac has been changed to void* from struct dwmci_idmac
> *idmac to handle 64bit DMA descriptors.
>
> I think that it shall be done in the same way here.
>
> > {
> > + struct dwmci_idmac *cur_idmac = (struct dwmci_idmac
> > *)cur_idmac64; unsigned long ctrl;
> > unsigned int i = 0, flags, cnt, blk_cnt;
> > ulong data_start, data_end;
> > @@ -56,7 +72,12 @@ static void dwmci_prepare_data(struct dwmci_host
> > *host, dwmci_wait_reset(host, DWMCI_CTRL_FIFO_RESET);
> >
> > data_start = (ulong)cur_idmac;
> > - dwmci_writel(host, DWMCI_DBADDR, (ulong)cur_idmac);
> > +
> > + if (host->dma_64bit_address) {
> > + dwmci_writel(host, DWMCI_DBADDRU, 0);
> > + dwmci_writel(host, DWMCI_DBADDRL,
> > (ulong)cur_idmac64);
>
> This is even more strange - the upper part of 32 bit address is 0, so
> we pass only 32 bit address. Is this SoC working in armv7 mode (32
> bit) and the dwmmc is expecting 64bit descriptors?
>
>
> > + } else
> > + dwmci_writel(host, DWMCI_DBADDR, (ulong)cur_idmac);
> >
> > do {
> > flags = DWMCI_IDMAC_OWN | DWMCI_IDMAC_CH ;
> > @@ -67,17 +88,27 @@ static void dwmci_prepare_data(struct dwmci_host
> > *host, } else
> > cnt = data->blocksize * 8;
> >
> > - dwmci_set_idma_desc(cur_idmac, flags, cnt,
> > - (ulong)bounce_buffer + (i *
> > PAGE_SIZE));
> > + if (host->dma_64bit_address)
> > + dwmci_set_idma_desc_64bit(cur_idmac64,
> ^^^^^^ here a void
> pointer with a
> static cast would be enough.
>
> > flags, cnt,
> > +
> > (ulong)bounce_buffer +
> > + (i * PAGE_SIZE));
> > + else
> > + dwmci_set_idma_desc_32bit(cur_idmac, flags,
> > cnt,
> > +
> > (ulong)bounce_buffer +
> > + (i * PAGE_SIZE));
> >
> > if (blk_cnt <= 8)
> > break;
> > blk_cnt -= 8;
> > cur_idmac++;
> > + cur_idmac64++;
>
> This looks like a quick hack - the if (host->dma_64bit_address) / else
> is missing. Otherwise it would break the boards which use this code
> already.
>
> > i++;
> > } while(1);
> >
> > - data_end = (ulong)cur_idmac;
> > + if (host->dma_64bit_address)
> > + data_end = (ulong)cur_idmac64;
> > + else
> > + data_end = (ulong)cur_idmac;
> > flush_dcache_range(data_start, data_end +
> > ARCH_DMA_MINALIGN);
> > ctrl = dwmci_readl(host, DWMCI_CTRL);
> > @@ -222,7 +253,7 @@ static int dwmci_send_cmd(struct mmc *mmc,
> > struct mmc_cmd *cmd, {
> > #endif
> > struct dwmci_host *host = mmc->priv;
> > - ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac, cur_idmac,
> > + ALLOC_CACHE_ALIGN_BUFFER(struct dwmci_idmac_64addr,
>
> Isn't this change causing the boards with 32 bits addressing
> malfunctioning?
>
> #ifdef would be necessary - as struct dwmci_idmac / dwmci_idmac_64addr
> have different sizes and fields.
>
> > cur_idmac64, data ? DIV_ROUND_UP(data->blocks, 8) : 0);
> > int ret = 0, flags = 0, i;
> > unsigned int timeout = 500;
> > @@ -256,7 +287,7 @@ static int dwmci_send_cmd(struct mmc *mmc,
> > struct mmc_cmd *cmd, data->blocksize *
> > data->blocks,
> > GEN_BB_READ); }
> > - dwmci_prepare_data(host, data, cur_idmac,
> > + dwmci_prepare_data(host, data, cur_idmac64,
> > bbstate.bounce_buffer);
>
> The above comment also applies here.
>
> > }
> > }
> > @@ -474,7 +505,9 @@ static int dwmci_init(struct mmc *mmc)
> >
> > dwmci_writel(host, DWMCI_TMOUT, 0xFFFFFFFF);
> >
> > - dwmci_writel(host, DWMCI_IDINTEN, 0);
> > + dwmci_writel(host, host->dma_64bit_address ?
> > + DWMCI_IDINTEN64 : DWMCI_IDINTEN, 0);
> > +
> > dwmci_writel(host, DWMCI_BMOD, 1);
> >
> > if (!host->fifoth_val) {
> > diff --git a/drivers/mmc/exynos_dw_mmc.c
> > b/drivers/mmc/exynos_dw_mmc.c index 435ccac594..3e9d47538c 100644
> > --- a/drivers/mmc/exynos_dw_mmc.c
> > +++ b/drivers/mmc/exynos_dw_mmc.c
> > @@ -98,6 +98,7 @@ static void exynos_dwmci_board_init(struct
> > dwmci_host *host)
> > static int exynos_dwmci_core_init(struct dwmci_host *host)
> > {
> > + unsigned int addr_config;
> > unsigned int div;
> > unsigned long freq, sclk;
> >
> > @@ -122,6 +123,11 @@ static int exynos_dwmci_core_init(struct
> > dwmci_host *host) host->clksel = exynos_dwmci_clksel;
> > host->get_mmc_clk = exynos_dwmci_get_clk;
> >
> > + addr_config = DWMCI_GET_ADDR_CONFIG(dwmci_readl(host,
> > DWMCI_HCON));
> > + if (addr_config == 1)
> > + /* host supports IDMAC in 64-bit address mode */
> > + host->dma_64bit_address = 1;
> > +
> > #ifndef CONFIG_DM_MMC
> > /* Add the mmc channel to be registered with mmc core */
> > if (add_dwmci(host, DWMMC_MAX_FREQ, DWMMC_MIN_FREQ)) {
> > diff --git a/include/dwmmc.h b/include/dwmmc.h
> > index 0f9d51b557..14db03d7d4 100644
> > --- a/include/dwmmc.h
> > +++ b/include/dwmmc.h
> > @@ -48,6 +48,17 @@
> > #define DWMCI_DSCADDR 0x094
> > #define DWMCI_BUFADDR 0x098
> > #define DWMCI_DATA 0x200
> > +/*
> > + * Registers to support idmac 64-bit address mode
> > + */
> > +#define DWMCI_DBADDRL 0x088
> > +#define DWMCI_DBADDRU 0x08c
> > +#define DWMCI_IDSTS64 0x090
> > +#define DWMCI_IDINTEN64 0x094
> > +#define DWMCI_DSCADDRL 0x098
> > +#define DWMCI_DSCADDRU 0x09c
> > +#define DWMCI_BUFADDRL 0x0A0
> > +#define DWMCI_BUFADDRU 0x0A4
> >
> > /* Interrupt Mask register */
> > #define DWMCI_INTMSK_ALL 0xffffffff
> > @@ -132,6 +143,7 @@
> > /* quirks */
> > #define DWMCI_QUIRK_DISABLE_SMU (1 << 0)
> >
> > +#define DWMCI_GET_ADDR_CONFIG(x) (((x)>>27) & 0x1)
> > /**
> > * struct dwmci_host - Information about a designware MMC host
> > *
> > @@ -145,6 +157,7 @@
> > * @dev_id: Arbitrary device ID for use by controller
> > * @buswidth: Bus width in bits (8 or 4)
> > * @fifoth_val: Value for FIFOTH register (or 0 to leave
> > unset)
> > + * @dma_64bit_address: True only for devices supporting 64
> > bit DMA
> > * @mmc: Pointer to generic MMC structure for this device
> > * @priv: Private pointer for use by controller
> > */
> > @@ -161,6 +174,7 @@ struct dwmci_host {
> > int dev_id;
> > int buswidth;
> > u32 fifoth_val;
> > + int dma_64bit_address;
> > struct mmc *mmc;
> > void *priv;
> >
> > @@ -196,6 +210,17 @@ struct dwmci_idmac {
> > u32 next_addr;
> > } __aligned(ARCH_DMA_MINALIGN);
> >
> > +struct dwmci_idmac_64addr {
> > + u32 flags;
> > + u32 _res1;
> > + u32 cnt;
> > + u32 _res2;
> > + u32 addrl;
> > + u32 addrh;
> > + u32 next_addrl;
> > + u32 next_addrh;
> > +} __aligned(ARCH_DMA_MINALIGN);
> > +
> > static inline void dwmci_writel(struct dwmci_host *host, int reg,
> > u32 val) {
> > writel(val, host->ioaddr + reg);
>
>
And just to mention - for such change I would like to have a tested-by:
tag on the SoC (Odroid XU3/4), which uses the 32 bit DMA descriptors.
>
>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lu...@denx.de
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lu...@denx.de
pgp1CnjKnhLiI.pgp
Description: OpenPGP digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
