linux-kernel - Re: [RFC PATCH v4 1/9] mmc: dw_mmc: Add external dma interface support

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-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGhQ9VxUYvgGD=K3J3cQ0CNpCC-RfGE9cxhBHnTeAU2CmWkiKA@mail.gmail.com>
Date:	Fri, 7 Aug 2015 23:32:05 +0200
From:	Joachim Eastwood <manabian@...il.com>
To:	Shawn Lin <shawn.lin@...k-chips.com>
Cc:	jh80.chung@...sung.com, Ulf Hansson <ulf.hansson@...aro.org>,
	Heiko Stübner <heiko@...ech.de>,
	Doug Anderson <dianders@...omium.org>,
	Vineet.Gupta1@...opsys.com, Wei Xu <xuwei5@...ilicon.com>,
	Alexey Brodkin <abrodkin@...opsys.com>,
	Kukjin Kim <kgene@...nel.org>,
	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	Russell King <linux@....linux.org.uk>,
	Jun Nie <jun.nie@...aro.org>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Govindraj Raja <govindraj.raja@...tec.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	linux-samsung-soc@...r.kernel.org, linux-mips@...ux-mips.org,
	linux-mmc@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-rockchip@...ts.infradead.org, devicetree@...r.kernel.org
Subject: Re: [RFC PATCH v4 1/9] mmc: dw_mmc: Add external dma interface support

Hi Shawn,

On 6 August 2015 at 08:44, Shawn Lin <shawn.lin@...k-chips.com> wrote:
> DesignWare MMC Controller can supports two types of DMA
> mode: external dma and internal dma. We get a RK312x platform
> integrated dw_mmc and ARM pl330 dma controller. This patch add
> edmac ops to support these platforms. I've tested it on RK312x
> platform with edmac mode and RK3288 platform with idmac mode.
>
> Signed-off-by: Shawn Lin <shawn.lin@...k-chips.com>

> @@ -2256,26 +2373,30 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>
>         }
>
> -#ifdef CONFIG_MMC_DW_IDMAC
> -       /* Handle DMA interrupts */
> -       if (host->dma_64bit_address == 1) {
> -               pending = mci_readl(host, IDSTS64);
> -               if (pending & (SDMMC_IDMAC_INT_TI | SDMMC_IDMAC_INT_RI)) {
> -                       mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_TI |
> -                                                       SDMMC_IDMAC_INT_RI);
> -                       mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_NI);
> -                       host->dma_ops->complete(host);
> -               }
> -       } else {
> -               pending = mci_readl(host, IDSTS);
> -               if (pending & (SDMMC_IDMAC_INT_TI | SDMMC_IDMAC_INT_RI)) {
> -                       mci_writel(host, IDSTS, SDMMC_IDMAC_INT_TI |
> -                                                       SDMMC_IDMAC_INT_RI);
> -                       mci_writel(host, IDSTS, SDMMC_IDMAC_INT_NI);
> -                       host->dma_ops->complete(host);
> +       if (host->use_dma == TRANS_MODE_IDMAC) {

Doing:
if (host->use_dma != TRANS_MODE_IDMAC)
    return IRQ_HANDLED;

Could save you the extra level of identation you add below.

> +               /* Handle DMA interrupts */
> +               if (host->dma_64bit_address == 1) {
> +                       pending = mci_readl(host, IDSTS64);
> +                       if (pending & (SDMMC_IDMAC_INT_TI |
> +                                      SDMMC_IDMAC_INT_RI)) {
> +                               mci_writel(host, IDSTS64,
> +                                          SDMMC_IDMAC_INT_TI |
> +                                          SDMMC_IDMAC_INT_RI);
> +                               mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_NI);
> +                               host->dma_ops->complete((void *)host);
> +                       }
> +               } else {
> +                       pending = mci_readl(host, IDSTS);
> +                       if (pending & (SDMMC_IDMAC_INT_TI |
> +                                      SDMMC_IDMAC_INT_RI)) {
> +                               mci_writel(host, IDSTS,
> +                                          SDMMC_IDMAC_INT_TI |
> +                                          SDMMC_IDMAC_INT_RI);
> +                               mci_writel(host, IDSTS, SDMMC_IDMAC_INT_NI);
> +                               host->dma_ops->complete((void *)host);
> +                       }
>                 }
>         }
> -#endif
>
>         return IRQ_HANDLED;
>  }


> @@ -2437,6 +2567,21 @@ static void dw_mci_cleanup_slot(struct dw_mci_slot *slot, unsigned int id)
>  static void dw_mci_init_dma(struct dw_mci *host)
>  {
>         int addr_config;
> +       int trans_mode;
> +       struct device *dev = host->dev;
> +       struct device_node *np = dev->of_node;
> +
> +       /* Check tansfer mode */
> +       trans_mode = (mci_readl(host, HCON) >> 16) & 0x3;

I think it would be nice if you could add some defines for 16 and 0x03
or add a macro like SDMMC_GET_FCNT() that is in dw_mmc.h.

> +       if (trans_mode == 0) {
> +               trans_mode = TRANS_MODE_IDMAC;
> +       } else if (trans_mode == 1 || trans_mode == 2) {
> +               trans_mode = TRANS_MODE_EDMAC;
> +       } else {
> +               trans_mode = TRANS_MODE_PIO;
> +               goto no_dma;
> +       }
> +
>         /* Check ADDR_CONFIG bit in HCON to find IDMAC address bus width */
>         addr_config = (mci_readl(host, HCON) >> 27) & 0x01;

I'll try to get this patch tested on my lpc18xx platform soon.
btw, the HCON reg on lpc18xx reads as 0x00e42cc1 (address 0x40004070).


regard,
Joachim Eastwood
--
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/