| 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
| ||
|
Date: Wed, 15 Oct 2014 17:35:44 +0530
From: Vivek Gautam <gautam.vivek@...sung.com>
To: Alim Akhtar <alim.akhtar@...il.com>
Cc: Prabu Thangamuthu <Prabu.T@...opsys.com>,
Seungwon Jeon <tgih.jun@...sung.com>,
Jaehoon Chung <jh80.chung@...sung.com>,
Chris Ball <chris@...ntf.net>,
Ulf Hansson <ulf.hansson@...aro.org>,
"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Manjunath M Bettegowda <Manjunath.MB@...opsys.com>
Subject: Re: [PATCH v6] mmc: dw_mmc: Add IDMAC 64-bit address mode support
Hi Prabu,
On Tue, Oct 14, 2014 at 5:41 PM, Alim Akhtar <alim.akhtar@...il.com> wrote:
> Hi Prahu,
> Thanks for a quick re-spin o the patch.
> One last comment, this is more of a information seek.
> On Thu, Oct 9, 2014 at 1:09 PM, Prabu Thangamuthu <Prabu.T@...opsys.com> wrote:
>> Synopsys DW_MMC IP core supports Internal DMA Controller with 64-bit address mode from IP version 2.70a onwards.
>> Updated the driver to support IDMAC 64-bit addressing mode.
>>
>> Tested the features in DW_MMC core v2.70a and v2.40a with HAPS-51 setup and driver is working fine.
>>
>> Signed-off-by: Prabu Thangamuthu <prabu.t@...opsys.com>
>> ---
>> Change log v6:
>> - Updated with review comment.
>>
>> Change log v5:
>> - Recreated the patch against linux-next as this patch is required for another patch http://www.spinics.net/lists/arm-kernel/msg357985.html
>>
>> Change log v4:
>> - Add the dynamic support for 32-bit and 64-bit address mode based on hw configuration.
>> - Removed the CONFIG_MMC_DW_IDMAC_64BIT_ADDRESS macro.
>>
>> drivers/mmc/host/dw_mmc.c | 195 +++++++++++++++++++++++++++++++++++---------
>> drivers/mmc/host/dw_mmc.h | 11 +++
>> include/linux/mmc/dw_mmc.h | 2 +
>> 3 files changed, 170 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 69f0cc6..c3b75a5 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -62,6 +62,24 @@
>> SDMMC_IDMAC_INT_FBE | SDMMC_IDMAC_INT_RI | \
>> SDMMC_IDMAC_INT_TI)
>>
>> +struct idmac_desc_64addr {
>> + u32 des0; /* Control Descriptor */
>> +
>> + u32 des1; /* Reserved */
> What is the default value of these _Reserved_ descriptors? As these
> are pointers, do you thing initializing then to zero make sense?
I think the default reset value of these descriptors will depend on
how dwmmc is integrated in h/w and how these descriptors are then
configured.
So it makes sense to initialize these descriptors to zero before use.
We have seen on our exynos7 system, that we need to initialize the
descriptors des1 and des2 to zero before we use them further.
>> +
>> + u32 des2; /*Buffer sizes */
>> +#define IDMAC_64ADDR_SET_BUFFER1_SIZE(d, s) \
>> + ((d)->des2 = ((d)->des2 & 0x03ffe000) | ((s) & 0x1fff))
>> +
>> + u32 des3; /* Reserved */
>> +
>> + u32 des4; /* Lower 32-bits of Buffer Address Pointer 1*/
>> + u32 des5; /* Upper 32-bits of Buffer Address Pointer 1*/
>> +
>> + u32 des6; /* Lower 32-bits of Next Descriptor Address */
>> + u32 des7; /* Upper 32-bits of Next Descriptor Address */
>> +};
>> +
>> struct idmac_desc {
>> u32 des0; /* Control Descriptor */
>> #define IDMAC_DES0_DIC BIT(1)
>> @@ -414,30 +432,66 @@ static void dw_mci_translate_sglist(struct dw_mci *host, struct mmc_data *data,
>> unsigned int sg_len)
>> {
>> int i;
>> - struct idmac_desc *desc = host->sg_cpu;
>> + if (host->dma_64bit_address == 1) {
>> + struct idmac_desc_64addr *desc = host->sg_cpu;
>>
>> - for (i = 0; i < sg_len; i++, desc++) {
>> - unsigned int length = sg_dma_len(&data->sg[i]);
>> - u32 mem_addr = sg_dma_address(&data->sg[i]);
>> + for (i = 0; i < sg_len; i++, desc++) {
>> + unsigned int length = sg_dma_len(&data->sg[i]);
>> + u64 mem_addr = sg_dma_address(&data->sg[i]);
>>
>> - /* Set the OWN bit and disable interrupts for this descriptor */
>> - desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC | IDMAC_DES0_CH;
>> + /*
>> + * Set the OWN bit and disable interrupts for this
>> + * descriptor
>> + */
>> + desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC |
>> + IDMAC_DES0_CH;
>> + /* Buffer length */
>> + IDMAC_64ADDR_SET_BUFFER1_SIZE(desc, length);
>> +
>> + /* Physical address to DMA to/from */
>> + desc->des4 = mem_addr & 0xffffffff;
>> + desc->des5 = mem_addr >> 32;
>> + }
>>
>> - /* Buffer length */
>> - IDMAC_SET_BUFFER1_SIZE(desc, length);
>> + /* Set first descriptor */
>> + desc = host->sg_cpu;
>> + desc->des0 |= IDMAC_DES0_FD;
>>
>> - /* Physical address to DMA to/from */
>> - desc->des2 = mem_addr;
>> - }
>> + /* Set last descriptor */
>> + desc = host->sg_cpu + (i - 1) *
>> + sizeof(struct idmac_desc_64addr);
>> + desc->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC);
>> + desc->des0 |= IDMAC_DES0_LD;
>> +
>> + } else {
>> + struct idmac_desc *desc = host->sg_cpu;
>> +
>> + for (i = 0; i < sg_len; i++, desc++) {
>> + unsigned int length = sg_dma_len(&data->sg[i]);
>> + u32 mem_addr = sg_dma_address(&data->sg[i]);
>> +
>> + /*
>> + * Set the OWN bit and disable interrupts for this
>> + * descriptor
>> + */
>> + desc->des0 = IDMAC_DES0_OWN | IDMAC_DES0_DIC |
>> + IDMAC_DES0_CH;
>> + /* Buffer length */
>> + IDMAC_SET_BUFFER1_SIZE(desc, length);
>> +
>> + /* Physical address to DMA to/from */
>> + desc->des2 = mem_addr;
>> + }
>>
>> - /* Set first descriptor */
>> - desc = host->sg_cpu;
>> - desc->des0 |= IDMAC_DES0_FD;
>> + /* Set first descriptor */
>> + desc = host->sg_cpu;
>> + desc->des0 |= IDMAC_DES0_FD;
>>
>> - /* Set last descriptor */
>> - desc = host->sg_cpu + (i - 1) * sizeof(struct idmac_desc);
>> - desc->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC);
>> - desc->des0 |= IDMAC_DES0_LD;
>> + /* Set last descriptor */
>> + desc = host->sg_cpu + (i - 1) * sizeof(struct idmac_desc);
>> + desc->des0 &= ~(IDMAC_DES0_CH | IDMAC_DES0_DIC);
>> + desc->des0 |= IDMAC_DES0_LD;
>> + }
>>
>> wmb();
>> }
>> @@ -466,29 +520,67 @@ static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>>
>> static int dw_mci_idmac_init(struct dw_mci *host)
>> {
>> - struct idmac_desc *p;
>> int i;
>>
>> - /* Number of descriptors in the ring buffer */
>> - host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
>> + if (host->dma_64bit_address == 1) {
>> + struct idmac_desc_64addr *p;
>> + /* Number of descriptors in the ring buffer */
>> + host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc_64addr);
>> +
>> + /* Forward link the descriptor list */
>> + for (i = 0, p = host->sg_cpu; i < host->ring_size - 1;
>> + i++, p++) {
>> + p->des6 = (host->sg_dma +
>> + (sizeof(struct idmac_desc_64addr) *
>> + (i + 1))) & 0xffffffff;
>> +
>> + p->des7 = (host->sg_dma +
>> + (sizeof(struct idmac_desc_64addr) *
>> + (i + 1))) >> 32;
>> + }
>> +
>> + /* Set the last descriptor as the end-of-ring descriptor */
>> + p->des6 = host->sg_dma & 0xffffffff;
>> + p->des7 = host->sg_dma >> 32;
>> + p->des0 = IDMAC_DES0_ER;
>> +
>> + } else {
>> + struct idmac_desc *p;
>> + /* Number of descriptors in the ring buffer */
>> + host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
>>
>> - /* Forward link the descriptor list */
>> - for (i = 0, p = host->sg_cpu; i < host->ring_size - 1; i++, p++)
>> - p->des3 = host->sg_dma + (sizeof(struct idmac_desc) * (i + 1));
>> + /* Forward link the descriptor list */
>> + for (i = 0, p = host->sg_cpu; i < host->ring_size - 1; i++, p++)
>> + p->des3 = host->sg_dma + (sizeof(struct idmac_desc) *
>> + (i + 1));
>>
>> - /* Set the last descriptor as the end-of-ring descriptor */
>> - p->des3 = host->sg_dma;
>> - p->des0 = IDMAC_DES0_ER;
>> + /* Set the last descriptor as the end-of-ring descriptor */
>> + p->des3 = host->sg_dma;
>> + p->des0 = IDMAC_DES0_ER;
>> + }
>>
>> dw_mci_idmac_reset(host);
>>
>> - /* Mask out interrupts - get Tx & Rx complete only */
>> - mci_writel(host, IDSTS, IDMAC_INT_CLR);
>> - mci_writel(host, IDINTEN, SDMMC_IDMAC_INT_NI | SDMMC_IDMAC_INT_RI |
>> - SDMMC_IDMAC_INT_TI);
>> + if (host->dma_64bit_address == 1) {
>> + /* Mask out interrupts - get Tx & Rx complete only */
>> + mci_writel(host, IDSTS64, IDMAC_INT_CLR);
>> + mci_writel(host, IDINTEN64, SDMMC_IDMAC_INT_NI |
>> + SDMMC_IDMAC_INT_RI | SDMMC_IDMAC_INT_TI);
>> +
>> + /* Set the descriptor base address */
>> + mci_writel(host, DBADDRL, host->sg_dma & 0xffffffff);
>> + mci_writel(host, DBADDRU, host->sg_dma >> 32);
>> +
>> + } else {
>> + /* Mask out interrupts - get Tx & Rx complete only */
>> + mci_writel(host, IDSTS, IDMAC_INT_CLR);
>> + mci_writel(host, IDINTEN, SDMMC_IDMAC_INT_NI |
>> + SDMMC_IDMAC_INT_RI | SDMMC_IDMAC_INT_TI);
>> +
>> + /* Set the descriptor base address */
>> + mci_writel(host, DBADDR, host->sg_dma);
>> + }
>>
>> - /* Set the descriptor base address */
>> - mci_writel(host, DBADDR, host->sg_dma);
>> return 0;
>> }
>>
>> @@ -2045,11 +2137,22 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id)
>>
>> #ifdef CONFIG_MMC_DW_IDMAC
>> /* Handle DMA interrupts */
>> - 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->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);
>> + }
>> }
>> #endif
>>
>> @@ -2309,6 +2412,22 @@ 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;
>> + /* Check ADDR_CONFIG bit in HCON to find IDMAC address bus width */
>> + addr_config = (mci_readl(host, HCON) >> 27) & 0x01;
>> +
>> + if (addr_config == 1) {
>> + /* host supports IDMAC in 64-bit address mode */
>> + host->dma_64bit_address = 1;
>> + dev_info(host->dev, "IDMAC supports 64-bit address mode.\n");
>> + if (!dma_set_mask(host->dev, DMA_BIT_MASK(64)))
>> + dma_set_coherent_mask(host->dev, DMA_BIT_MASK(64));
>> + } else {
>> + /* host supports IDMAC in 32-bit address mode */
>> + host->dma_64bit_address = 0;
>> + dev_info(host->dev, "IDMAC supports 32-bit address mode.\n");
>> + }
>> +
>> /* Alloc memory for sg translation */
>> host->sg_cpu = dmam_alloc_coherent(host->dev, PAGE_SIZE,
>> &host->sg_dma, GFP_KERNEL);
>> diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
>> index 01b99e8..64b04aa 100644
>> --- a/drivers/mmc/host/dw_mmc.h
>> +++ b/drivers/mmc/host/dw_mmc.h
>> @@ -55,6 +55,17 @@
>> #define SDMMC_BUFADDR 0x098
>> #define SDMMC_CDTHRCTL 0x100
>> #define SDMMC_DATA(x) (x)
>> +/*
>> +* Registers to support idmac 64-bit address mode
>> +*/
>> +#define SDMMC_DBADDRL 0x088
>> +#define SDMMC_DBADDRU 0x08c
>> +#define SDMMC_IDSTS64 0x090
>> +#define SDMMC_IDINTEN64 0x094
>> +#define SDMMC_DSCADDRL 0x098
>> +#define SDMMC_DSCADDRU 0x09c
>> +#define SDMMC_BUFADDRL 0x0A0
>> +#define SDMMC_BUFADDRU 0x0A4
>>
>> /*
>> * Data offset is difference according to Version
>> diff --git a/include/linux/mmc/dw_mmc.h b/include/linux/mmc/dw_mmc.h
>> index 0013669..9e6eabb 100644
>> --- a/include/linux/mmc/dw_mmc.h
>> +++ b/include/linux/mmc/dw_mmc.h
>> @@ -54,6 +54,7 @@ struct mmc_data;
>> * transfer is in progress.
>> * @use_dma: Whether DMA channel is initialized or not.
>> * @using_dma: Whether DMA is in use for the current transfer.
>> + * @dma_64bit_address: Whether DMA supports 64-bit address mode or not.
>> * @sg_dma: Bus address of DMA buffer.
>> * @sg_cpu: Virtual address of DMA buffer.
>> * @dma_ops: Pointer to platform-specific DMA callbacks.
>> @@ -140,6 +141,7 @@ struct dw_mci {
>> /* DMA interface members*/
>> int use_dma;
>> int using_dma;
>> + int dma_64bit_address;
>>
>> dma_addr_t sg_dma;
>> void *sg_cpu;
>> --
>> 1.7.6.5
>>
>
>
>
> --
> Regards,
> Alim
> --
> 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/
--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India
--
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