lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 21 Aug 2012 10:12:34 +0530
From:	viresh kumar <viresh.kumar@...aro.org>
To:	Hein Tibosch <hein_tibosch@...oo.es>
Cc:	Hans-Christian Egtvedt <hans-christian.egtvedt@...el.com>,
	Nicolas Ferre <nicolas.ferre@...el.com>,
	Havard Skinnemoen <havard@...nnemoen.net>,
	"ludovic.desroches" <ludovic.desroches@...el.com>,
	linux-kernel@...r.kernel.org, spear-devel <spear-devel@...t.st.com>
Subject: Re: [PATCH] Fixes for dw_dmac and atmel-mci for AP700x

Hi Hein,

I have added linux-kernel list in cc as there might be other users of
this patch.
Also, please try to keep spear-devel list in cc for dw_dmac as we are using this
driver for SPEAr.

On Sun, Aug 19, 2012 at 9:36 PM, Hein Tibosch <hein_tibosch@...oo.es> wrote:
> Hi Hans-Christian,
>
> Do you know to which list I could send the patch(es) below?
>
> Viresh Kumar: did you have an urgent reason to replace __raw_readl
> and __raw_writel with readl/writel in the dw_dmac module?
> (see http://lists.infradead.org/pipermail/linux-arm-kernel/2011-March/044799.html)
>
> Here's a draft:
>
> Because of the changes since 2.6.38 to dw_dmac.c and atmel-mci.c,
> the MCI driver on the AP700x platforms didn't work anymore using DMA.
> The PIO method still worked well.
>
> I found several problems. The patch below makes it work again.
> It was tested on an atngw100 board.
>
> atmel-mci:
> - The AP700x has an MCI controller with version 0x210
>   Therefor it was supposed to work the PDC (peripheral DMA
>   controller). However, in this CPU the PDC is not connected
>   to the MCI.
> - When using DMA, the driver was accessing a register ATMCI_DMA
>   at offset 0x50, which is undefined for the AP700x. But it
>   probably doesn't hurt either...
>
> dw_dmac:
> - After 2.6.39, the registers were accessed using readl/writel
>   in stead of the __raw_readl and __raw_writel causing a 16-bit
>   swap of all values (little endian access)

Ahhhh!! Firstly we can't use __raw* for architectures >= ARMv6. It is not
only for endianess but for memory barriers. Why are they getting swapped
for your case? Does your processor and dw_dmac have different endianess?

And if i am not wrong, we should always try not to use __raw* variants just
due to endianess things... instead use either readl/writel OR
readl_/writel_ relaxed.
I am not sure if relaxed versions are available for architectures
other than ARM.

> - Access to memory was sometimes done in chunks of 64-bits,
>   which gives an undefined value of 0x03 for SRC/DST_TR_WIDTH
>   field in the CTLxL register

Looks fine. But there should be a separate patch for this.

> - The SMS field in the CTLxL register received the wrong value:
>   0 in stead of 1

I believe it is not for dw_dmac?

--
viresh

> Signed-off-by: Hein Tibosch <hein_tibosch@...oo.es>
> ---
>  arch/avr32/mach-at32ap/at32ap700x.c |    6 ++++++
>  drivers/dma/dw_dmac.c               |    8 ++++++++
>  drivers/dma/dw_dmac_regs.h          |    8 ++++----
>  drivers/mmc/host/atmel-mci.c        |   19 ++++++++++++++++---
>  4 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/arch/avr32/mach-at32ap/at32ap700x.c b/arch/avr32/mach-at32ap/at32ap700x.c
> index 0445c4f..06b6eff 100644
> --- a/arch/avr32/mach-at32ap/at32ap700x.c
> +++ b/arch/avr32/mach-at32ap/at32ap700x.c
> @@ -1355,6 +1355,12 @@ at32_add_device_mci(unsigned int id, struct mci_platform_data *data)
>                                 | DWC_CFGH_DST_PER(1));
>         slave->sdata.cfg_lo &= ~(DWC_CFGL_HS_DST_POL
>                                 | DWC_CFGL_HS_SRC_POL);
> +       /* value for SMS: Source Master Select */
> +       slave->sdata.src_master = 1;
> +       /* value for DMS: Destination Master Select */
> +       slave->sdata.dst_master = 0;
> +       /* SRC/DST_TR_WIDTH register only accepts 0,1,2 */
> +       slave->sdata.max_mem_width = 2;
>         data->dma_slave = slave;
>  diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index 7212961..a4bdf1d 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -754,6 +754,10 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>                                 mem_width = 1;
>                         else
>                                 mem_width = 0;
> +                       /* Some controllers don't support 64-bits mem access */
> +                       if (dws->max_mem_width &&
> +                               mem_width > dws->max_mem_width)
> +                               mem_width = dws->max_mem_width;
>   slave_sg_todev_fill_desc:
>                         desc = dwc_desc_get(dwc);
> @@ -821,6 +825,10 @@ slave_sg_todev_fill_desc:
>                                 mem_width = 1;
>                         else
>                                 mem_width = 0;
> +                       /* Some controllers don't support 64-bits mem access */
> +                       if (dws->max_mem_width &&
> +                               mem_width > dws->max_mem_width)
> +                               mem_width = dws->max_mem_width;
>   slave_sg_fromdev_fill_desc:
>                         desc = dwc_desc_get(dwc);
> diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
> index f298f69..b380d5f 100644
> --- a/drivers/dma/dw_dmac_regs.h
> +++ b/drivers/dma/dw_dmac_regs.h
> @@ -176,9 +176,9 @@ __dwc_regs(struct dw_dma_chan *dwc)
>  }
>   #define channel_readl(dwc, name) \
> -       readl(&(__dwc_regs(dwc)->name))
> +       __raw_readl(&(__dwc_regs(dwc)->name))
>  #define channel_writel(dwc, name, val) \
> -       writel((val), &(__dwc_regs(dwc)->name))
> +       __raw_writel((val), &(__dwc_regs(dwc)->name))
>   static inline struct dw_dma_chan *to_dw_dma_chan(struct dma_chan *chan)
>  {
> @@ -202,9 +202,9 @@ static inline struct dw_dma_regs __iomem *__dw_regs(struct dw_dma *dw)
>  }
>   #define dma_readl(dw, name) \
> -       readl(&(__dw_regs(dw)->name))
> +       __raw_readl(&(__dw_regs(dw)->name))
>  #define dma_writel(dw, name, val) \
> -       writel((val), &(__dw_regs(dw)->name))
> +       __raw_writel((val), &(__dw_regs(dw)->name))
>   #define channel_set_bit(dw, reg, mask) \
>         dma_writel(dw, reg, ((mask) << 8) | (mask))
> diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
> index f2c115e..b78a674 100644
> --- a/drivers/mmc/host/atmel-mci.c
> +++ b/drivers/mmc/host/atmel-mci.c
> @@ -75,6 +75,7 @@ struct atmel_mci_caps {
>         bool    has_pdc;
>         bool    has_cfg_reg;
>         bool    has_cstor_reg;
> +       bool    has_dma_reg;
>         bool    has_highspeed;
>         bool    has_rwproof;
>         bool    has_odd_clk_div;
> @@ -411,7 +412,7 @@ static int atmci_regs_show(struct seq_file *s, void *v)
>         atmci_show_status_reg(s, "SR", buf[ATMCI_SR / 4]);
>         atmci_show_status_reg(s, "IMR", buf[ATMCI_IMR / 4]);
>  -      if (host->caps.has_dma) {
> +       if (host->caps.has_dma_reg) {
>                 u32 val;
>                 val = buf[ATMCI_DMA / 4];
> @@ -767,7 +768,7 @@ static void atmci_dma_complete(void *arg)
>         dev_vdbg(&host->pdev->dev, "DMA complete\n");
>  -      if (host->caps.has_dma)
> +       if (host->caps.has_dma_reg)
>                 /* Disable DMA hardware handshaking on MCI */
>                 atmci_writel(host, ATMCI_DMA, atmci_readl(host, ATMCI_DMA) & ~ATMCI_DMAEN);
>  @@ -954,7 +955,9 @@ atmci_prepare_data_dma(struct atmel_mci *host, struct mmc_data *data)
>                 maxburst = atmci_convert_chksize(host->dma_conf.dst_maxburst);
>         }
>  -      atmci_writel(host, ATMCI_DMA, ATMCI_DMA_CHKSIZE(maxburst) | ATMCI_DMAEN);
> +       if (host->caps.has_dma_reg)
> +               atmci_writel(host, ATMCI_DMA,
> +                       ATMCI_DMA_CHKSIZE(maxburst) | ATMCI_DMAEN);
>         sglen = dma_map_sg(chan->device->dev, data->sg,
>                         data->sg_len, direction);
> @@ -2203,7 +2206,11 @@ static void __init atmci_get_cap(struct atmel_mci *host)
>                         "version: 0x%x\n", version);
>         host->caps.has_dma = 0;
> +#if defined(CONFIG_CPU_AT32AP700X)
> +       host->caps.has_pdc = 0;
> +#else
>         host->caps.has_pdc = 1;
> +#endif
>         host->caps.has_cfg_reg = 0;
>         host->caps.has_cstor_reg = 0;
>         host->caps.has_highspeed = 0;
> @@ -2221,6 +2228,7 @@ static void __init atmci_get_cap(struct atmel_mci *host)
>         case 0x300:
>  #ifdef CONFIG_AT_HDMAC
>                 host->caps.has_dma = 1;
> +               host->caps.has_dma_reg = 1;
>  #else
>                 dev_info(&host->pdev->dev,
>                         "has dma capability but dma engine is not selected, then use pio\n");
> @@ -2230,6 +2238,11 @@ static void __init atmci_get_cap(struct atmel_mci *host)
>                 host->caps.has_cstor_reg = 1;
>                 host->caps.has_highspeed = 1;
>         case 0x200:
> +#if defined(CONFIG_DW_DMAC) && defined(CONFIG_MMC_ATMELMCI_DMA)
> +               host->caps.has_dma = 1;
> +               dev_info(&host->pdev->dev,
> +                       "using dma (AP7000)\n");
> +#endif
>                 host->caps.has_rwproof = 1;
>                 host->caps.need_blksz_mul_4 = 0;
>         case 0x100:
> --
> 1.7.8.0
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ