[PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Seungwon Jeon]

This reverts commit 94c6cee91(Add check for IDMAC configuration).
Synopsys says that only if internal dmac is not present, optional
external dma interface is present. When internal dmac is present,
'0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
indicates external dma interface. And idmac initialization is
prohibited now. So, let's revert this commit.

CC: Girish K S <girish.shivananjappa@linaro.org>
Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
---
 drivers/mmc/host/dw_mmc.c |   15 ++-------------
 1 files changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 36f98c0..dcbe9aa 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -405,23 +405,11 @@ 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, dma_support;
+	int i;
 
 	/* Number of descriptors in the ring buffer */
 	host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
 
-	/* Check if Hardware Configuration Register has support for DMA */
-	dma_support = (mci_readl(host, HCON) >> 16) & 0x3;
-
-	if (!dma_support || dma_support > 2) {
-		dev_err(&host->dev,
-			"Host Controller does not support IDMA Tx.\n");
-		host->dma_ops = NULL;
-		return -ENODEV;
-	}
-
-	dev_info(&host->dev, "Using internal DMA controller.\n");
-
 	/* 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));
@@ -1895,6 +1883,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
 	/* Determine which DMA interface to use */
 #ifdef CONFIG_MMC_DW_IDMAC
 	host->dma_ops = &dw_mci_idmac_ops;
+	dev_info(&host->dev, "Using internal DMA controller.\n");
 #endif
 
 	if (!host->dma_ops)
-- 
1.7.0.4

Re: [PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Girish K S]

On 11 September 2012 07:53, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> This reverts commit 94c6cee91(Add check for IDMAC configuration).
> Synopsys says that only if internal dmac is not present, optional
> external dma interface is present. When internal dmac is present,
> '0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
> indicates external dma interface. And idmac initialization is
> prohibited now. So, let's revert this commit.

There is no register, or a field in any register which says IDMAC present.
I can see only use_idmac bit field in CTRL register.
So in first place how will i know whether IDMAC is present? Have you
assumed that IDMAC is present.
I would like to know without assuming, whether  IDMAC is present.
If i have missed out something let me know.  i will check and revert back

>
> CC: Girish K S <girish.shivananjappa@linaro.org>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c |   15 ++-------------
>  1 files changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 36f98c0..dcbe9aa 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -405,23 +405,11 @@ 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, dma_support;
> +       int i;
>
>         /* Number of descriptors in the ring buffer */
>         host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
>
> -       /* Check if Hardware Configuration Register has support for DMA */
> -       dma_support = (mci_readl(host, HCON) >> 16) & 0x3;
> -
> -       if (!dma_support || dma_support > 2) {
> -               dev_err(&host->dev,
> -                       "Host Controller does not support IDMA Tx.\n");
> -               host->dma_ops = NULL;
> -               return -ENODEV;
> -       }
> -
> -       dev_info(&host->dev, "Using internal DMA controller.\n");
> -
>         /* 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));
> @@ -1895,6 +1883,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
>         /* Determine which DMA interface to use */
>  #ifdef CONFIG_MMC_DW_IDMAC
>         host->dma_ops = &dw_mci_idmac_ops;
> +       dev_info(&host->dev, "Using internal DMA controller.\n");
>  #endif
>
>         if (!host->dma_ops)
> --
> 1.7.0.4
>
>

Re: [PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Jaehoon Chung]

On 09/11/2012 12:46 PM, Girish K S wrote:
> On 11 September 2012 07:53, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> This reverts commit 94c6cee91(Add check for IDMAC configuration).
>> Synopsys says that only if internal dmac is not present, optional
>> external dma interface is present. When internal dmac is present,
>> '0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
>> indicates external dma interface. And idmac initialization is
>> prohibited now. So, let's revert this commit.
> 
> There is no register, or a field in any register which says IDMAC present.
> I can see only use_idmac bit field in CTRL register.
> So in first place how will i know whether IDMAC is present? Have you
> assumed that IDMAC is present.
> I would like to know without assuming, whether  IDMAC is present.
> If i have missed out something let me know.  i will check and revert back
I understood that if INTERNAL_DMAC at HCON register is set to 1, we didn't consdier the DMA_INTERFACE's value.
So although dma_interface is set to 0 or others, host can use the internal DMA.

Best Regards,
Jaehoon Chung
> 
>>
>> CC: Girish K S <girish.shivananjappa@linaro.org>
>> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>> ---
>>  drivers/mmc/host/dw_mmc.c |   15 ++-------------
>>  1 files changed, 2 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 36f98c0..dcbe9aa 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -405,23 +405,11 @@ 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, dma_support;
>> +       int i;
>>
>>         /* Number of descriptors in the ring buffer */
>>         host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
>>
>> -       /* Check if Hardware Configuration Register has support for DMA */
>> -       dma_support = (mci_readl(host, HCON) >> 16) & 0x3;
>> -
>> -       if (!dma_support || dma_support > 2) {
>> -               dev_err(&host->dev,
>> -                       "Host Controller does not support IDMA Tx.\n");
>> -               host->dma_ops = NULL;
>> -               return -ENODEV;
>> -       }
>> -
>> -       dev_info(&host->dev, "Using internal DMA controller.\n");
>> -
>>         /* 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));
>> @@ -1895,6 +1883,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
>>         /* Determine which DMA interface to use */
>>  #ifdef CONFIG_MMC_DW_IDMAC
>>         host->dma_ops = &dw_mci_idmac_ops;
>> +       dev_info(&host->dev, "Using internal DMA controller.\n");
>>  #endif
>>
>>         if (!host->dma_ops)
>> --
>> 1.7.0.4
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Girish K S]

On 11 September 2012 10:06, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 09/11/2012 12:46 PM, Girish K S wrote:
>> On 11 September 2012 07:53, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>>> This reverts commit 94c6cee91(Add check for IDMAC configuration).
>>> Synopsys says that only if internal dmac is not present, optional
>>> external dma interface is present. When internal dmac is present,
>>> '0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
>>> indicates external dma interface. And idmac initialization is
>>> prohibited now. So, let's revert this commit.
>>
>> There is no register, or a field in any register which says IDMAC present.
>> I can see only use_idmac bit field in CTRL register.
>> So in first place how will i know whether IDMAC is present? Have you
>> assumed that IDMAC is present.
>> I would like to know without assuming, whether  IDMAC is present.
>> If i have missed out something let me know.  i will check and revert back
> I understood that if INTERNAL_DMAC at HCON register is set to 1, we didn't consdier the DMA_INTERFACE's value.
I cannot see any field by name INTERNAL_DMAC in the HCON: can you
please tell me the bit position of it.
I think INTERNAL_DMAC is the signal interface to AXI master.
> So although dma_interface is set to 0 or others, host can use the internal DMA.
>
> Best Regards,
> Jaehoon Chung
>>
>>>
>>> CC: Girish K S <girish.shivananjappa@linaro.org>
>>> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>>> ---
>>>  drivers/mmc/host/dw_mmc.c |   15 ++-------------
>>>  1 files changed, 2 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> index 36f98c0..dcbe9aa 100644
>>> --- a/drivers/mmc/host/dw_mmc.c
>>> +++ b/drivers/mmc/host/dw_mmc.c
>>> @@ -405,23 +405,11 @@ 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, dma_support;
>>> +       int i;
>>>
>>>         /* Number of descriptors in the ring buffer */
>>>         host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
>>>
>>> -       /* Check if Hardware Configuration Register has support for DMA */
>>> -       dma_support = (mci_readl(host, HCON) >> 16) & 0x3;
>>> -
>>> -       if (!dma_support || dma_support > 2) {
>>> -               dev_err(&host->dev,
>>> -                       "Host Controller does not support IDMA Tx.\n");
>>> -               host->dma_ops = NULL;
>>> -               return -ENODEV;
>>> -       }
>>> -
>>> -       dev_info(&host->dev, "Using internal DMA controller.\n");
>>> -
>>>         /* 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));
>>> @@ -1895,6 +1883,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
>>>         /* Determine which DMA interface to use */
>>>  #ifdef CONFIG_MMC_DW_IDMAC
>>>         host->dma_ops = &dw_mci_idmac_ops;
>>> +       dev_info(&host->dev, "Using internal DMA controller.\n");
>>>  #endif
>>>
>>>         if (!host->dma_ops)
>>> --
>>> 1.7.0.4
>>>
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>

Re: [PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Jaehoon Chung]

On 09/11/2012 01:40 PM, Girish K S wrote:
> On 11 September 2012 10:06, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> On 09/11/2012 12:46 PM, Girish K S wrote:
>>> On 11 September 2012 07:53, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>>>> This reverts commit 94c6cee91(Add check for IDMAC configuration).
>>>> Synopsys says that only if internal dmac is not present, optional
>>>> external dma interface is present. When internal dmac is present,
>>>> '0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
>>>> indicates external dma interface. And idmac initialization is
>>>> prohibited now. So, let's revert this commit.
>>>
>>> There is no register, or a field in any register which says IDMAC present.
>>> I can see only use_idmac bit field in CTRL register.
>>> So in first place how will i know whether IDMAC is present? Have you
>>> assumed that IDMAC is present.
>>> I would like to know without assuming, whether  IDMAC is present.
>>> If i have missed out something let me know.  i will check and revert back
>> I understood that if INTERNAL_DMAC at HCON register is set to 1, we didn't consdier the DMA_INTERFACE's value.
> I cannot see any field by name INTERNAL_DMAC in the HCON: can you
> please tell me the bit position of it.
> I think INTERNAL_DMAC is the signal interface to AXI master.
Sorry, You're right. there is no field for INTERNAL_DMAC at HCON register.
>> So although dma_interface is set to 0 or others, host can use the internal DMA.
>>
>> Best Regards,
>> Jaehoon Chung
>>>
>>>>
>>>> CC: Girish K S <girish.shivananjappa@linaro.org>
>>>> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>>>> ---
>>>>  drivers/mmc/host/dw_mmc.c |   15 ++-------------
>>>>  1 files changed, 2 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>> index 36f98c0..dcbe9aa 100644
>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>> @@ -405,23 +405,11 @@ 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, dma_support;
>>>> +       int i;
>>>>
>>>>         /* Number of descriptors in the ring buffer */
>>>>         host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
>>>>
>>>> -       /* Check if Hardware Configuration Register has support for DMA */
>>>> -       dma_support = (mci_readl(host, HCON) >> 16) & 0x3;
>>>> -
>>>> -       if (!dma_support || dma_support > 2) {
>>>> -               dev_err(&host->dev,
>>>> -                       "Host Controller does not support IDMA Tx.\n");
>>>> -               host->dma_ops = NULL;
>>>> -               return -ENODEV;
>>>> -       }
>>>> -
>>>> -       dev_info(&host->dev, "Using internal DMA controller.\n");
>>>> -
>>>>         /* 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));
>>>> @@ -1895,6 +1883,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
>>>>         /* Determine which DMA interface to use */
>>>>  #ifdef CONFIG_MMC_DW_IDMAC
>>>>         host->dma_ops = &dw_mci_idmac_ops;
>>>> +       dev_info(&host->dev, "Using internal DMA controller.\n");
>>>>  #endif
>>>>
>>>>         if (!host->dma_ops)
>>>> --
>>>> 1.7.0.4
>>>>
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

RE: [PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Seungwon Jeon]

On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
> On 11 September 2012 07:53, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> > This reverts commit 94c6cee91(Add check for IDMAC configuration).
> > Synopsys says that only if internal dmac is not present, optional
> > external dma interface is present. When internal dmac is present,
> > '0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
> > indicates external dma interface. And idmac initialization is
> > prohibited now. So, let's revert this commit.
> 
> There is no register, or a field in any register which says IDMAC present.
> I can see only use_idmac bit field in CTRL register.
> So in first place how will i know whether IDMAC is present? Have you
> assumed that IDMAC is present.
> I would like to know without assuming, whether  IDMAC is present.
> If i have missed out something let me know.  i will check and revert back
There is no way to see the presence of idmac from host controller on the runtime.
But enabling idmac is not selected in the menuconfig just with assuming its presence.
User can find the support of idmac from manual at least, I  know the lack of this information in manual though.

Thanks,
Seungwon Jeon

> 
> >
> > CC: Girish K S <girish.shivananjappa@linaro.org>
> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> > ---
> >  drivers/mmc/host/dw_mmc.c |   15 ++-------------
> >  1 files changed, 2 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> > index 36f98c0..dcbe9aa 100644
> > --- a/drivers/mmc/host/dw_mmc.c
> > +++ b/drivers/mmc/host/dw_mmc.c
> > @@ -405,23 +405,11 @@ 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, dma_support;
> > +       int i;
> >
> >         /* Number of descriptors in the ring buffer */
> >         host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
> >
> > -       /* Check if Hardware Configuration Register has support for DMA */
> > -       dma_support = (mci_readl(host, HCON) >> 16) & 0x3;
> > -
> > -       if (!dma_support || dma_support > 2) {
> > -               dev_err(&host->dev,
> > -                       "Host Controller does not support IDMA Tx.\n");
> > -               host->dma_ops = NULL;
> > -               return -ENODEV;
> > -       }
> > -
> > -       dev_info(&host->dev, "Using internal DMA controller.\n");
> > -
> >         /* 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));
> > @@ -1895,6 +1883,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
> >         /* Determine which DMA interface to use */
> >  #ifdef CONFIG_MMC_DW_IDMAC
> >         host->dma_ops = &dw_mci_idmac_ops;
> > +       dev_info(&host->dev, "Using internal DMA controller.\n");
> >  #endif
> >
> >         if (!host->dma_ops)
> > --
> > 1.7.0.4
> >
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Girish K S]

On 11 September 2012 11:16, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
>> On 11 September 2012 07:53, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> > This reverts commit 94c6cee91(Add check for IDMAC configuration).
>> > Synopsys says that only if internal dmac is not present, optional
>> > external dma interface is present. When internal dmac is present,
>> > '0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
>> > indicates external dma interface. And idmac initialization is
>> > prohibited now. So, let's revert this commit.
>>
>> There is no register, or a field in any register which says IDMAC present.
>> I can see only use_idmac bit field in CTRL register.
>> So in first place how will i know whether IDMAC is present? Have you
>> assumed that IDMAC is present.
>> I would like to know without assuming, whether  IDMAC is present.
>> If i have missed out something let me know.  i will check and revert back
> There is no way to see the presence of idmac from host controller on the runtime.
> But enabling idmac is not selected in the menuconfig just with assuming its presence.
> User can find the support of idmac from manual at least, I  know the lack of this information in manual though.
I cannot find any line in the manual saying "Support IDMAC". Keeping
this in mind and the with the literal meaning what the HCON field
DMA_INTERFACE means, the above patch was made.
i had posted a patch for adding quirk to this (was waiting for some dt
patches to be accepted to resend it). Once that patch gets merged it
will be helpful for the user to use it comfortably. If QUIRK is
enabled it means there is IDMAC and the driver user is intentionally
enabling it.

>
> Thanks,
> Seungwon Jeon
>
>>
>> >
>> > CC: Girish K S <girish.shivananjappa@linaro.org>
>> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>> > ---
>> >  drivers/mmc/host/dw_mmc.c |   15 ++-------------
>> >  1 files changed, 2 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> > index 36f98c0..dcbe9aa 100644
>> > --- a/drivers/mmc/host/dw_mmc.c
>> > +++ b/drivers/mmc/host/dw_mmc.c
>> > @@ -405,23 +405,11 @@ 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, dma_support;
>> > +       int i;
>> >
>> >         /* Number of descriptors in the ring buffer */
>> >         host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
>> >
>> > -       /* Check if Hardware Configuration Register has support for DMA */
>> > -       dma_support = (mci_readl(host, HCON) >> 16) & 0x3;
>> > -
>> > -       if (!dma_support || dma_support > 2) {
>> > -               dev_err(&host->dev,
>> > -                       "Host Controller does not support IDMA Tx.\n");
>> > -               host->dma_ops = NULL;
>> > -               return -ENODEV;
>> > -       }
>> > -
>> > -       dev_info(&host->dev, "Using internal DMA controller.\n");
>> > -
>> >         /* 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));
>> > @@ -1895,6 +1883,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
>> >         /* Determine which DMA interface to use */
>> >  #ifdef CONFIG_MMC_DW_IDMAC
>> >         host->dma_ops = &dw_mci_idmac_ops;
>> > +       dev_info(&host->dev, "Using internal DMA controller.\n");
>> >  #endif
>> >
>> >         if (!host->dma_ops)
>> > --
>> > 1.7.0.4
>> >
>> >
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

RE: [PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Seungwon Jeon]

On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
> On 11 September 2012 11:16, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> > On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
> >> On 11 September 2012 07:53, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> >> > This reverts commit 94c6cee91(Add check for IDMAC configuration).
> >> > Synopsys says that only if internal dmac is not present, optional
> >> > external dma interface is present. When internal dmac is present,
> >> > '0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
> >> > indicates external dma interface. And idmac initialization is
> >> > prohibited now. So, let's revert this commit.
> >>
> >> There is no register, or a field in any register which says IDMAC present.
> >> I can see only use_idmac bit field in CTRL register.
> >> So in first place how will i know whether IDMAC is present? Have you
> >> assumed that IDMAC is present.
> >> I would like to know without assuming, whether  IDMAC is present.
> >> If i have missed out something let me know.  i will check and revert back
> > There is no way to see the presence of idmac from host controller on the runtime.
> > But enabling idmac is not selected in the menuconfig just with assuming its presence.
> > User can find the support of idmac from manual at least, I  know the lack of this information in
> manual though.
> I cannot find any line in the manual saying "Support IDMAC". Keeping
> this in mind and the with the literal meaning what the HCON field
> DMA_INTERFACE means, the above patch was made.
> i had posted a patch for adding quirk to this (was waiting for some dt
> patches to be accepted to resend it). Once that patch gets merged it
> will be helpful for the user to use it comfortably. If QUIRK is
> enabled it means there is IDMAC and the driver user is intentionally
> enabling it.
I mentioned the lack of description of feature in manual.
When we consider the real meaning of DMA_INTERFACE, '0' value doen't indicate the absence of idmac.
So, we cannot decide  the support of idmac with the DMA_INTERFACE.

Do you mean this patch:"mmc: dwmmc: Add quirk for broken Hardware Config"?
HCON register in the original Synopsys doesn't include information of internal dmac.
Do you think it's also broken?

Thanks,
Seungwon Jeon
> 
> >
> > Thanks,
> > Seungwon Jeon
> >
> >>
> >> >
> >> > CC: Girish K S <girish.shivananjappa@linaro.org>
> >> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> >> > ---
> >> >  drivers/mmc/host/dw_mmc.c |   15 ++-------------
> >> >  1 files changed, 2 insertions(+), 13 deletions(-)
> >> >
> >> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> >> > index 36f98c0..dcbe9aa 100644
> >> > --- a/drivers/mmc/host/dw_mmc.c
> >> > +++ b/drivers/mmc/host/dw_mmc.c
> >> > @@ -405,23 +405,11 @@ 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, dma_support;
> >> > +       int i;
> >> >
> >> >         /* Number of descriptors in the ring buffer */
> >> >         host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
> >> >
> >> > -       /* Check if Hardware Configuration Register has support for DMA */
> >> > -       dma_support = (mci_readl(host, HCON) >> 16) & 0x3;
> >> > -
> >> > -       if (!dma_support || dma_support > 2) {
> >> > -               dev_err(&host->dev,
> >> > -                       "Host Controller does not support IDMA Tx.\n");
> >> > -               host->dma_ops = NULL;
> >> > -               return -ENODEV;
> >> > -       }
> >> > -
> >> > -       dev_info(&host->dev, "Using internal DMA controller.\n");
> >> > -
> >> >         /* 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));
> >> > @@ -1895,6 +1883,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
> >> >         /* Determine which DMA interface to use */
> >> >  #ifdef CONFIG_MMC_DW_IDMAC
> >> >         host->dma_ops = &dw_mci_idmac_ops;
> >> > +       dev_info(&host->dev, "Using internal DMA controller.\n");
> >> >  #endif
> >> >
> >> >         if (!host->dma_ops)
> >> > --
> >> > 1.7.0.4
> >> >
> >> >
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Girish K S]

On 11 September 2012 12:09, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
>> On 11 September 2012 11:16, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> > On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
>> >> On 11 September 2012 07:53, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> >> > This reverts commit 94c6cee91(Add check for IDMAC configuration).
>> >> > Synopsys says that only if internal dmac is not present, optional
>> >> > external dma interface is present. When internal dmac is present,
>> >> > '0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
>> >> > indicates external dma interface. And idmac initialization is
>> >> > prohibited now. So, let's revert this commit.
>> >>
>> >> There is no register, or a field in any register which says IDMAC present.
>> >> I can see only use_idmac bit field in CTRL register.
>> >> So in first place how will i know whether IDMAC is present? Have you
>> >> assumed that IDMAC is present.
>> >> I would like to know without assuming, whether  IDMAC is present.
>> >> If i have missed out something let me know.  i will check and revert back
>> > There is no way to see the presence of idmac from host controller on the runtime.
>> > But enabling idmac is not selected in the menuconfig just with assuming its presence.
>> > User can find the support of idmac from manual at least, I  know the lack of this information in
>> manual though.
>> I cannot find any line in the manual saying "Support IDMAC". Keeping
>> this in mind and the with the literal meaning what the HCON field
>> DMA_INTERFACE means, the above patch was made.
>> i had posted a patch for adding quirk to this (was waiting for some dt
>> patches to be accepted to resend it). Once that patch gets merged it
>> will be helpful for the user to use it comfortably. If QUIRK is
>> enabled it means there is IDMAC and the driver user is intentionally
>> enabling it.
> I mentioned the lack of description of feature in manual.
> When we consider the real meaning of DMA_INTERFACE, '0' value doen't indicate the absence of idmac.
> So, we cannot decide  the support of idmac with the DMA_INTERFACE.
>
> Do you mean this patch:"mmc: dwmmc: Add quirk for broken Hardware Config"?
> HCON register in the original Synopsys doesn't include information of internal dmac.
> Do you think it's also broken?
As per the current available information from the manual i think the
HCON is broken.
May be the synopsys is the right person to clarify this.
>
> Thanks,
> Seungwon Jeon
>>
>> >
>> > Thanks,
>> > Seungwon Jeon
>> >
>> >>
>> >> >
>> >> > CC: Girish K S <girish.shivananjappa@linaro.org>
>> >> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>> >> > ---
>> >> >  drivers/mmc/host/dw_mmc.c |   15 ++-------------
>> >> >  1 files changed, 2 insertions(+), 13 deletions(-)
>> >> >
>> >> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> >> > index 36f98c0..dcbe9aa 100644
>> >> > --- a/drivers/mmc/host/dw_mmc.c
>> >> > +++ b/drivers/mmc/host/dw_mmc.c
>> >> > @@ -405,23 +405,11 @@ 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, dma_support;
>> >> > +       int i;
>> >> >
>> >> >         /* Number of descriptors in the ring buffer */
>> >> >         host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
>> >> >
>> >> > -       /* Check if Hardware Configuration Register has support for DMA */
>> >> > -       dma_support = (mci_readl(host, HCON) >> 16) & 0x3;
>> >> > -
>> >> > -       if (!dma_support || dma_support > 2) {
>> >> > -               dev_err(&host->dev,
>> >> > -                       "Host Controller does not support IDMA Tx.\n");
>> >> > -               host->dma_ops = NULL;
>> >> > -               return -ENODEV;
>> >> > -       }
>> >> > -
>> >> > -       dev_info(&host->dev, "Using internal DMA controller.\n");
>> >> > -
>> >> >         /* 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));
>> >> > @@ -1895,6 +1883,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
>> >> >         /* Determine which DMA interface to use */
>> >> >  #ifdef CONFIG_MMC_DW_IDMAC
>> >> >         host->dma_ops = &dw_mci_idmac_ops;
>> >> > +       dev_info(&host->dev, "Using internal DMA controller.\n");
>> >> >  #endif
>> >> >
>> >> >         if (!host->dma_ops)
>> >> > --
>> >> > 1.7.0.4
>> >> >
>> >> >
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> >> the body of a message to majordomo@vger.kernel.org
>> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Girish K S]

On 11 September 2012 12:41, Girish K S <girish.shivananjappa@linaro.org> wrote:
> On 11 September 2012 12:09, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
>>> On 11 September 2012 11:16, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>>> > On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
>>> >> On 11 September 2012 07:53, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>>> >> > This reverts commit 94c6cee91(Add check for IDMAC configuration).
>>> >> > Synopsys says that only if internal dmac is not present, optional
>>> >> > external dma interface is present. When internal dmac is present,
>>> >> > '0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
>>> >> > indicates external dma interface. And idmac initialization is
>>> >> > prohibited now. So, let's revert this commit.
>>> >>
>>> >> There is no register, or a field in any register which says IDMAC present.
>>> >> I can see only use_idmac bit field in CTRL register.
>>> >> So in first place how will i know whether IDMAC is present? Have you
>>> >> assumed that IDMAC is present.
>>> >> I would like to know without assuming, whether  IDMAC is present.
>>> >> If i have missed out something let me know.  i will check and revert back
>>> > There is no way to see the presence of idmac from host controller on the runtime.
>>> > But enabling idmac is not selected in the menuconfig just with assuming its presence.
>>> > User can find the support of idmac from manual at least, I  know the lack of this information in
>>> manual though.
>>> I cannot find any line in the manual saying "Support IDMAC". Keeping
>>> this in mind and the with the literal meaning what the HCON field
>>> DMA_INTERFACE means, the above patch was made.
>>> i had posted a patch for adding quirk to this (was waiting for some dt
>>> patches to be accepted to resend it). Once that patch gets merged it
>>> will be helpful for the user to use it comfortably. If QUIRK is
>>> enabled it means there is IDMAC and the driver user is intentionally
>>> enabling it.
>> I mentioned the lack of description of feature in manual.
>> When we consider the real meaning of DMA_INTERFACE, '0' value doen't indicate the absence of idmac.
>> So, we cannot decide  the support of idmac with the DMA_INTERFACE.
>>
>> Do you mean this patch:"mmc: dwmmc: Add quirk for broken Hardware Config"?
>> HCON register in the original Synopsys doesn't include information of internal dmac.
>> Do you think it's also broken?
> As per the current available information from the manual i think the
> HCON is broken.
> May be the synopsys is the right person to clarify this.
If you have some confirmation regarding the DMA_INTERFACE from the
synopsys. please post it, It will be really helpful in correcting my
understanding abt the dma interface
>>
>> Thanks,
>> Seungwon Jeon
>>>
>>> >
>>> > Thanks,
>>> > Seungwon Jeon
>>> >
>>> >>
>>> >> >
>>> >> > CC: Girish K S <girish.shivananjappa@linaro.org>
>>> >> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>>> >> > ---
>>> >> >  drivers/mmc/host/dw_mmc.c |   15 ++-------------
>>> >> >  1 files changed, 2 insertions(+), 13 deletions(-)
>>> >> >
>>> >> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>> >> > index 36f98c0..dcbe9aa 100644
>>> >> > --- a/drivers/mmc/host/dw_mmc.c
>>> >> > +++ b/drivers/mmc/host/dw_mmc.c
>>> >> > @@ -405,23 +405,11 @@ 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, dma_support;
>>> >> > +       int i;
>>> >> >
>>> >> >         /* Number of descriptors in the ring buffer */
>>> >> >         host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
>>> >> >
>>> >> > -       /* Check if Hardware Configuration Register has support for DMA */
>>> >> > -       dma_support = (mci_readl(host, HCON) >> 16) & 0x3;
>>> >> > -
>>> >> > -       if (!dma_support || dma_support > 2) {
>>> >> > -               dev_err(&host->dev,
>>> >> > -                       "Host Controller does not support IDMA Tx.\n");
>>> >> > -               host->dma_ops = NULL;
>>> >> > -               return -ENODEV;
>>> >> > -       }
>>> >> > -
>>> >> > -       dev_info(&host->dev, "Using internal DMA controller.\n");
>>> >> > -
>>> >> >         /* 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));
>>> >> > @@ -1895,6 +1883,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
>>> >> >         /* Determine which DMA interface to use */
>>> >> >  #ifdef CONFIG_MMC_DW_IDMAC
>>> >> >         host->dma_ops = &dw_mci_idmac_ops;
>>> >> > +       dev_info(&host->dev, "Using internal DMA controller.\n");
>>> >> >  #endif
>>> >> >
>>> >> >         if (!host->dma_ops)
>>> >> > --
>>> >> > 1.7.0.4
>>> >> >
>>> >> >
>>> >> --
>>> >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> >> the body of a message to majordomo@vger.kernel.org
>>> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>> >
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>

RE: [PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Seungwon Jeon]

On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
> On 11 September 2012 12:41, Girish K S <girish.shivananjappa@linaro.org> wrote:
> > On 11 September 2012 12:09, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> >> On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
> >>> On 11 September 2012 11:16, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> >>> > On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
> >>> >> On 11 September 2012 07:53, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> >>> >> > This reverts commit 94c6cee91(Add check for IDMAC configuration).
> >>> >> > Synopsys says that only if internal dmac is not present, optional
> >>> >> > external dma interface is present. When internal dmac is present,
> >>> >> > '0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
> >>> >> > indicates external dma interface. And idmac initialization is
> >>> >> > prohibited now. So, let's revert this commit.
> >>> >>
> >>> >> There is no register, or a field in any register which says IDMAC present.
> >>> >> I can see only use_idmac bit field in CTRL register.
> >>> >> So in first place how will i know whether IDMAC is present? Have you
> >>> >> assumed that IDMAC is present.
> >>> >> I would like to know without assuming, whether  IDMAC is present.
> >>> >> If i have missed out something let me know.  i will check and revert back
> >>> > There is no way to see the presence of idmac from host controller on the runtime.
> >>> > But enabling idmac is not selected in the menuconfig just with assuming its presence.
> >>> > User can find the support of idmac from manual at least, I  know the lack of this information in
> >>> manual though.
> >>> I cannot find any line in the manual saying "Support IDMAC". Keeping
> >>> this in mind and the with the literal meaning what the HCON field
> >>> DMA_INTERFACE means, the above patch was made.
> >>> i had posted a patch for adding quirk to this (was waiting for some dt
> >>> patches to be accepted to resend it). Once that patch gets merged it
> >>> will be helpful for the user to use it comfortably. If QUIRK is
> >>> enabled it means there is IDMAC and the driver user is intentionally
> >>> enabling it.
> >> I mentioned the lack of description of feature in manual.
> >> When we consider the real meaning of DMA_INTERFACE, '0' value doen't indicate the absence of idmac.
> >> So, we cannot decide  the support of idmac with the DMA_INTERFACE.
> >>
> >> Do you mean this patch:"mmc: dwmmc: Add quirk for broken Hardware Config"?
> >> HCON register in the original Synopsys doesn't include information of internal dmac.
> >> Do you think it's also broken?
> > As per the current available information from the manual i think the
> > HCON is broken.
> > May be the synopsys is the right person to clarify this.
> If you have some confirmation regarding the DMA_INTERFACE from the
> synopsys. please post it, It will be really helpful in correcting my
> understanding abt the dma interface
Information regarding DMA_INTERFACE can be referred enough in Synopsys manual.
As mentioned above commit message, DMA_INTERFACE specifies the external dma interface.
'0'(DMA_INTERFACE) means that there is no external dma interface in current host controller.
Synopsys may supply the idmac for host controller basically.
If Soc's vendor decides to use external dma interface instead of idmac, 
then DMA_INTERFACE parameter should be configured before synthesizing RTL.
I hope it helpful to you. If there is something wrong, please let me know.

Thanks,
Seungwon Jeon
> >>> >
> >>> >>
> >>> >> >
> >>> >> > CC: Girish K S <girish.shivananjappa@linaro.org>
> >>> >> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> >>> >> > ---
> >>> >> >  drivers/mmc/host/dw_mmc.c |   15 ++-------------
> >>> >> >  1 files changed, 2 insertions(+), 13 deletions(-)
> >>> >> >
> >>> >> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> >>> >> > index 36f98c0..dcbe9aa 100644
> >>> >> > --- a/drivers/mmc/host/dw_mmc.c
> >>> >> > +++ b/drivers/mmc/host/dw_mmc.c
> >>> >> > @@ -405,23 +405,11 @@ 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, dma_support;
> >>> >> > +       int i;
> >>> >> >
> >>> >> >         /* Number of descriptors in the ring buffer */
> >>> >> >         host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
> >>> >> >
> >>> >> > -       /* Check if Hardware Configuration Register has support for DMA */
> >>> >> > -       dma_support = (mci_readl(host, HCON) >> 16) & 0x3;
> >>> >> > -
> >>> >> > -       if (!dma_support || dma_support > 2) {
> >>> >> > -               dev_err(&host->dev,
> >>> >> > -                       "Host Controller does not support IDMA Tx.\n");
> >>> >> > -               host->dma_ops = NULL;
> >>> >> > -               return -ENODEV;
> >>> >> > -       }
> >>> >> > -
> >>> >> > -       dev_info(&host->dev, "Using internal DMA controller.\n");
> >>> >> > -
> >>> >> >         /* 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));
> >>> >> > @@ -1895,6 +1883,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
> >>> >> >         /* Determine which DMA interface to use */
> >>> >> >  #ifdef CONFIG_MMC_DW_IDMAC
> >>> >> >         host->dma_ops = &dw_mci_idmac_ops;
> >>> >> > +       dev_info(&host->dev, "Using internal DMA controller.\n");
> >>> >> >  #endif
> >>> >> >
> >>> >> >         if (!host->dma_ops)
> >>> >> > --
> >>> >> > 1.7.0.4
> >>> >> >
> >>> >> >
> >>> >> --
> >>> >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> >>> >> the body of a message to majordomo@vger.kernel.org
> >>> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>> >
> >>> --
> >>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> >>> the body of a message to majordomo@vger.kernel.org
> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Girish K S]

On 12 September 2012 07:51, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
>> On 11 September 2012 12:41, Girish K S <girish.shivananjappa@linaro.org> wrote:
>> > On 11 September 2012 12:09, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> >> On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
>> >>> On 11 September 2012 11:16, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> >>> > On Tuesday, September 11, 2012, Girish K S <girish.shivananjappa@linaro.org> wrote:
>> >>> >> On 11 September 2012 07:53, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> >>> >> > This reverts commit 94c6cee91(Add check for IDMAC configuration).
>> >>> >> > Synopsys says that only if internal dmac is not present, optional
>> >>> >> > external dma interface is present. When internal dmac is present,
>> >>> >> > '0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
>> >>> >> > indicates external dma interface. And idmac initialization is
>> >>> >> > prohibited now. So, let's revert this commit.
>> >>> >>
>> >>> >> There is no register, or a field in any register which says IDMAC present.
>> >>> >> I can see only use_idmac bit field in CTRL register.
>> >>> >> So in first place how will i know whether IDMAC is present? Have you
>> >>> >> assumed that IDMAC is present.
>> >>> >> I would like to know without assuming, whether  IDMAC is present.
>> >>> >> If i have missed out something let me know.  i will check and revert back
>> >>> > There is no way to see the presence of idmac from host controller on the runtime.
>> >>> > But enabling idmac is not selected in the menuconfig just with assuming its presence.
>> >>> > User can find the support of idmac from manual at least, I  know the lack of this information in
>> >>> manual though.
>> >>> I cannot find any line in the manual saying "Support IDMAC". Keeping
>> >>> this in mind and the with the literal meaning what the HCON field
>> >>> DMA_INTERFACE means, the above patch was made.
>> >>> i had posted a patch for adding quirk to this (was waiting for some dt
>> >>> patches to be accepted to resend it). Once that patch gets merged it
>> >>> will be helpful for the user to use it comfortably. If QUIRK is
>> >>> enabled it means there is IDMAC and the driver user is intentionally
>> >>> enabling it.
>> >> I mentioned the lack of description of feature in manual.
>> >> When we consider the real meaning of DMA_INTERFACE, '0' value doen't indicate the absence of idmac.
>> >> So, we cannot decide  the support of idmac with the DMA_INTERFACE.
>> >>
>> >> Do you mean this patch:"mmc: dwmmc: Add quirk for broken Hardware Config"?
>> >> HCON register in the original Synopsys doesn't include information of internal dmac.
>> >> Do you think it's also broken?
>> > As per the current available information from the manual i think the
>> > HCON is broken.
>> > May be the synopsys is the right person to clarify this.
>> If you have some confirmation regarding the DMA_INTERFACE from the
>> synopsys. please post it, It will be really helpful in correcting my
>> understanding abt the dma interface
> Information regarding DMA_INTERFACE can be referred enough in Synopsys manual.
> As mentioned above commit message, DMA_INTERFACE specifies the external dma interface.
If this is true (because my discussion is based on the information
available in the SOc's manual). then i go with the decision of
reverting.
> '0'(DMA_INTERFACE) means that there is no external dma interface in current host controller.
> Synopsys may supply the idmac for host controller basically.
> If Soc's vendor decides to use external dma interface instead of idmac,
> then DMA_INTERFACE parameter should be configured before synthesizing RTL.
> I hope it helpful to you. If there is something wrong, please let me know.
>
> Thanks,
> Seungwon Jeon
>> >>> >
>> >>> >>
>> >>> >> >
>> >>> >> > CC: Girish K S <girish.shivananjappa@linaro.org>
>> >>> >> > Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>> >>> >> > ---
>> >>> >> >  drivers/mmc/host/dw_mmc.c |   15 ++-------------
>> >>> >> >  1 files changed, 2 insertions(+), 13 deletions(-)
>> >>> >> >
>> >>> >> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> >>> >> > index 36f98c0..dcbe9aa 100644
>> >>> >> > --- a/drivers/mmc/host/dw_mmc.c
>> >>> >> > +++ b/drivers/mmc/host/dw_mmc.c
>> >>> >> > @@ -405,23 +405,11 @@ 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, dma_support;
>> >>> >> > +       int i;
>> >>> >> >
>> >>> >> >         /* Number of descriptors in the ring buffer */
>> >>> >> >         host->ring_size = PAGE_SIZE / sizeof(struct idmac_desc);
>> >>> >> >
>> >>> >> > -       /* Check if Hardware Configuration Register has support for DMA */
>> >>> >> > -       dma_support = (mci_readl(host, HCON) >> 16) & 0x3;
>> >>> >> > -
>> >>> >> > -       if (!dma_support || dma_support > 2) {
>> >>> >> > -               dev_err(&host->dev,
>> >>> >> > -                       "Host Controller does not support IDMA Tx.\n");
>> >>> >> > -               host->dma_ops = NULL;
>> >>> >> > -               return -ENODEV;
>> >>> >> > -       }
>> >>> >> > -
>> >>> >> > -       dev_info(&host->dev, "Using internal DMA controller.\n");
>> >>> >> > -
>> >>> >> >         /* 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));
>> >>> >> > @@ -1895,6 +1883,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
>> >>> >> >         /* Determine which DMA interface to use */
>> >>> >> >  #ifdef CONFIG_MMC_DW_IDMAC
>> >>> >> >         host->dma_ops = &dw_mci_idmac_ops;
>> >>> >> > +       dev_info(&host->dev, "Using internal DMA controller.\n");
>> >>> >> >  #endif
>> >>> >> >
>> >>> >> >         if (!host->dma_ops)
>> >>> >> > --
>> >>> >> > 1.7.0.4
>> >>> >> >
>> >>> >> >
>> >>> >> --
>> >>> >> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> >>> >> the body of a message to majordomo@vger.kernel.org
>> >>> >> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >>> >
>> >>> --
>> >>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> >>> the body of a message to majordomo@vger.kernel.org
>> >>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> >>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Will Newton]

On Tue, Sep 11, 2012 at 3:23 AM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> This reverts commit 94c6cee91(Add check for IDMAC configuration).
> Synopsys says that only if internal dmac is not present, optional
> external dma interface is present. When internal dmac is present,
> '0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
> indicates external dma interface. And idmac initialization is
> prohibited now. So, let's revert this commit.
>
> CC: Girish K S <girish.shivananjappa@linaro.org>
> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c |   15 ++-------------
>  1 files changed, 2 insertions(+), 13 deletions(-)

I think this revert is a good idea. The original patch tries to check
whether IDMAC is supported and falls back to PIO, but it is not clear
to me that HCON gives you the appropriate information to do this (at
least in my version of the manual, I have not tried to clarify this
with Synopsys). Also falling back to PIO mode seems like it would
provide pretty awful performance, I would not expect to see instances
of this block with no DMA support anywhere other than FPGA so
configuring with IDMAC without it's presence would be a serious
misconfiguration in any case.

Acked-by: Will Newton <will.newton@imgtec.com>

Re: [PATCH] Revert "mmc: dw_mmc: Add check for IDMAC configuration"

[Chris Ball]

Hi,

On Mon, Sep 17 2012, Will Newton wrote:
> On Tue, Sep 11, 2012 at 3:23 AM, Seungwon Jeon <tgih.jun@samsung.com> wrote:
>> This reverts commit 94c6cee91(Add check for IDMAC configuration).
>> Synopsys says that only if internal dmac is not present, optional
>> external dma interface is present. When internal dmac is present,
>> '0' value in DMA_INTERFACE of HCON is reasonable. DMA_INTERFACE
>> indicates external dma interface. And idmac initialization is
>> prohibited now. So, let's revert this commit.
>>
>> CC: Girish K S <girish.shivananjappa@linaro.org>
>> Signed-off-by: Seungwon Jeon <tgih.jun@samsung.com>
>> ---
>>  drivers/mmc/host/dw_mmc.c |   15 ++-------------
>>  1 files changed, 2 insertions(+), 13 deletions(-)
>
> I think this revert is a good idea. The original patch tries to check
> whether IDMAC is supported and falls back to PIO, but it is not clear
> to me that HCON gives you the appropriate information to do this (at
> least in my version of the manual, I have not tried to clarify this
> with Synopsys). Also falling back to PIO mode seems like it would
> provide pretty awful performance, I would not expect to see instances
> of this block with no DMA support anywhere other than FPGA so
> configuring with IDMAC without it's presence would be a serious
> misconfiguration in any case.
>
> Acked-by: Will Newton <will.newton@imgtec.com>

Thanks, pushed to mmc-next for 3.7.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child