public inbox for linux-mmc@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mmc: dw_mmc: clear INSTS register when initialize
@ 2013-04-18  5:21 Joonyoung Shim
  2013-04-18  6:59 ` Jaehoon Chung
  2013-04-23  9:57 ` Seungwon Jeon
  0 siblings, 2 replies; 11+ messages in thread
From: Joonyoung Shim @ 2013-04-18  5:21 UTC (permalink / raw)
  To: linux-mmc; +Cc: cjb, will.newton, jh80.chung

If pending interrupt for IDMAC exists when probe, it will call interrupt
handler unnecessarily.

Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
 drivers/mmc/host/dw_mmc.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 323c502..b0057a2 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -2192,6 +2192,7 @@ int dw_mci_probe(struct dw_mci *host)
 
 	/* Clear the interrupts for the host controller */
 	mci_writel(host, RINTSTS, 0xFFFFFFFF);
+	mci_writel(host, IDSTS, 0xFFFFFFFF);
 	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
 
 	/* Put in max timeout */
@@ -2243,6 +2244,7 @@ int dw_mci_probe(struct dw_mci *host)
 	 * receive ready and error such as transmit, receive timeout, crc error
 	 */
 	mci_writel(host, RINTSTS, 0xFFFFFFFF);
+	mci_writel(host, IDSTS, 0xFFFFFFFF);
 	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
 		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
 		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
@@ -2393,6 +2395,7 @@ int dw_mci_resume(struct dw_mci *host)
 	mci_writel(host, FIFOTH, host->fifoth_val);
 
 	mci_writel(host, RINTSTS, 0xFFFFFFFF);
+	mci_writel(host, IDSTS, 0xFFFFFFFF);
 	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
 		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
 		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: dw_mmc: clear INSTS register when initialize
  2013-04-18  5:21 [PATCH] mmc: dw_mmc: clear INSTS register when initialize Joonyoung Shim
@ 2013-04-18  6:59 ` Jaehoon Chung
  2013-04-23  9:57 ` Seungwon Jeon
  1 sibling, 0 replies; 11+ messages in thread
From: Jaehoon Chung @ 2013-04-18  6:59 UTC (permalink / raw)
  To: Joonyoung Shim; +Cc: linux-mmc, cjb, will.newton, jh80.chung, Seungwon Jeon

Subject of this patch needs to modify from "INSTS" to "IDSTS".
add CC'd Seungwon

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

On 04/18/2013 02:21 PM, Joonyoung Shim wrote:
> If pending interrupt for IDMAC exists when probe, it will call interrupt
> handler unnecessarily.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 323c502..b0057a2 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2192,6 +2192,7 @@ int dw_mci_probe(struct dw_mci *host)
>  
>  	/* Clear the interrupts for the host controller */
>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
>  	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
>  
>  	/* Put in max timeout */
> @@ -2243,6 +2244,7 @@ int dw_mci_probe(struct dw_mci *host)
>  	 * receive ready and error such as transmit, receive timeout, crc error
>  	 */
>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
>  	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
>  		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>  		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
> @@ -2393,6 +2395,7 @@ int dw_mci_resume(struct dw_mci *host)
>  	mci_writel(host, FIFOTH, host->fifoth_val);
>  
>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
>  	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
>  		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>  		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] mmc: dw_mmc: clear INSTS register when initialize
  2013-04-18  5:21 [PATCH] mmc: dw_mmc: clear INSTS register when initialize Joonyoung Shim
  2013-04-18  6:59 ` Jaehoon Chung
@ 2013-04-23  9:57 ` Seungwon Jeon
  2013-04-23 10:23   ` Jaehoon Chung
  1 sibling, 1 reply; 11+ messages in thread
From: Seungwon Jeon @ 2013-04-23  9:57 UTC (permalink / raw)
  To: 'Joonyoung Shim', linux-mmc; +Cc: cjb, will.newton, jh80.chung

Hi,

On Thursday, April 18, 2013, Joonyoung Shim wrote:
> If pending interrupt for IDMAC exists when probe, it will call interrupt
> handler unnecessarily.
> 
> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 323c502..b0057a2 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2192,6 +2192,7 @@ int dw_mci_probe(struct dw_mci *host)
> 
>  	/* Clear the interrupts for the host controller */
>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
0x337 is correct for bits. Could you check the bit filed?

>  	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
> 
>  	/* Put in max timeout */
> @@ -2243,6 +2244,7 @@ int dw_mci_probe(struct dw_mci *host)
>  	 * receive ready and error such as transmit, receive timeout, crc error
>  	 */
>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
No need, it's already done above.

>  	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
>  		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>  		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
> @@ -2393,6 +2395,7 @@ int dw_mci_resume(struct dw_mci *host)
>  	mci_writel(host, FIFOTH, host->fifoth_val);
> 
>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
Same, incorrect bits.

Thanks,
Seungwon Jeon

>  	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
>  		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>  		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
> --
> 1.7.9.5
> 
> --
> 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: dw_mmc: clear INSTS register when initialize
  2013-04-23  9:57 ` Seungwon Jeon
@ 2013-04-23 10:23   ` Jaehoon Chung
  2013-04-24  1:34     ` Seungwon Jeon
  0 siblings, 1 reply; 11+ messages in thread
From: Jaehoon Chung @ 2013-04-23 10:23 UTC (permalink / raw)
  To: Seungwon Jeon; +Cc: 'Joonyoung Shim', linux-mmc, cjb, will.newton

On 04/23/2013 06:57 PM, Seungwon Jeon wrote:
> Hi,
> 
> On Thursday, April 18, 2013, Joonyoung Shim wrote:
>> If pending interrupt for IDMAC exists when probe, it will call interrupt
>> handler unnecessarily.
>>
>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>> ---
>>  drivers/mmc/host/dw_mmc.c |    3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 323c502..b0057a2 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -2192,6 +2192,7 @@ int dw_mci_probe(struct dw_mci *host)
>>
>>  	/* Clear the interrupts for the host controller */
>>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
> 0x337 is correct for bits. Could you check the bit filed?
I think that don't care which reset value used.
"It is recommended that you write 0xffff_ffff to the Raw Interrupt register @0x044 and IDSTS @0x8C
in order to clear any pending interrupts before setting the int_enable bit."
This boot mode case is also used the 0xffff_ffff.
> 
>>  	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
>>
>>  	/* Put in max timeout */
>> @@ -2243,6 +2244,7 @@ int dw_mci_probe(struct dw_mci *host)
>>  	 * receive ready and error such as transmit, receive timeout, crc error
>>  	 */
>>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
> No need, it's already done above.
> 
>>  	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
>>  		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>>  		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
>> @@ -2393,6 +2395,7 @@ int dw_mci_resume(struct dw_mci *host)
>>  	mci_writel(host, FIFOTH, host->fifoth_val);
>>
>>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
> Same, incorrect bits.
> 
> Thanks,
> Seungwon Jeon
> 
>>  	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
>>  		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>>  		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
>> --
>> 1.7.9.5
>>
>> --
>> 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
> 
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] mmc: dw_mmc: clear INSTS register when initialize
  2013-04-23 10:23   ` Jaehoon Chung
@ 2013-04-24  1:34     ` Seungwon Jeon
  2013-04-24  3:05       ` Joonyoung Shim
  0 siblings, 1 reply; 11+ messages in thread
From: Seungwon Jeon @ 2013-04-24  1:34 UTC (permalink / raw)
  To: 'Jaehoon Chung'
  Cc: 'Joonyoung Shim', linux-mmc, cjb, will.newton

On Tuesday, April 23, 2013, Jaehoon Chung wrote:
> On 04/23/2013 06:57 PM, Seungwon Jeon wrote:
> > Hi,
> >
> > On Thursday, April 18, 2013, Joonyoung Shim wrote:
> >> If pending interrupt for IDMAC exists when probe, it will call interrupt
> >> handler unnecessarily.
> >>
> >> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> >> ---
> >>  drivers/mmc/host/dw_mmc.c |    3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> >> index 323c502..b0057a2 100644
> >> --- a/drivers/mmc/host/dw_mmc.c
> >> +++ b/drivers/mmc/host/dw_mmc.c
> >> @@ -2192,6 +2192,7 @@ int dw_mci_probe(struct dw_mci *host)
> >>
> >>  	/* Clear the interrupts for the host controller */
> >>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
> >> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
> > 0x337 is correct for bits. Could you check the bit filed?
> I think that don't care which reset value used.
> "It is recommended that you write 0xffff_ffff to the Raw Interrupt register @0x044 and IDSTS @0x8C
> in order to clear any pending interrupts before setting the int_enable bit."
> This boot mode case is also used the 0xffff_ffff.
In case IDSTS all 32bit are not for interrupt status unlike RINTSTS.
IDSTS[31:17] is reserved and IDSTS[16:0] also contains 'reserved' and 'read-only' field.
Correct use would be needed.

Thanks,
Seungwon Jeon
> >
> >>  	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
> >>
> >>  	/* Put in max timeout */
> >> @@ -2243,6 +2244,7 @@ int dw_mci_probe(struct dw_mci *host)
> >>  	 * receive ready and error such as transmit, receive timeout, crc error
> >>  	 */
> >>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
> >> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
> > No need, it's already done above.
> >
> >>  	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
> >>  		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
> >>  		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
> >> @@ -2393,6 +2395,7 @@ int dw_mci_resume(struct dw_mci *host)
> >>  	mci_writel(host, FIFOTH, host->fifoth_val);
> >>
> >>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
> >> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
> > Same, incorrect bits.
> >
> > Thanks,
> > Seungwon Jeon
> >
> >>  	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
> >>  		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
> >>  		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
> >> --
> >> 1.7.9.5
> >>
> >> --
> >> 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
> >
> >


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: dw_mmc: clear INSTS register when initialize
  2013-04-24  1:34     ` Seungwon Jeon
@ 2013-04-24  3:05       ` Joonyoung Shim
  2013-04-25  0:45         ` Jaehoon Chung
  0 siblings, 1 reply; 11+ messages in thread
From: Joonyoung Shim @ 2013-04-24  3:05 UTC (permalink / raw)
  To: Seungwon Jeon; +Cc: 'Jaehoon Chung', linux-mmc, cjb, will.newton

On 04/24/2013 10:34 AM, Seungwon Jeon wrote:
> On Tuesday, April 23, 2013, Jaehoon Chung wrote:
>> On 04/23/2013 06:57 PM, Seungwon Jeon wrote:
>>> Hi,
>>>
>>> On Thursday, April 18, 2013, Joonyoung Shim wrote:
>>>> If pending interrupt for IDMAC exists when probe, it will call interrupt
>>>> handler unnecessarily.
>>>>
>>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>>>> ---
>>>>  drivers/mmc/host/dw_mmc.c |    3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>> index 323c502..b0057a2 100644
>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>> @@ -2192,6 +2192,7 @@ int dw_mci_probe(struct dw_mci *host)
>>>>
>>>>  	/* Clear the interrupts for the host controller */
>>>>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>>>> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
>>> 0x337 is correct for bits. Could you check the bit filed?

I feel it's better to use already existing defines than 0x337.

>> I think that don't care which reset value used.
>> "It is recommended that you write 0xffff_ffff to the Raw Interrupt register @0x044 and IDSTS @0x8C
>> in order to clear any pending interrupts before setting the int_enable bit."
>> This boot mode case is also used the 0xffff_ffff.

Jaehoon, can i get this sentence from which document?

> In case IDSTS all 32bit are not for interrupt status unlike RINTSTS.
> IDSTS[31:17] is reserved and IDSTS[16:0] also contains 'reserved' and 'read-only' field.
> Correct use would be needed.
>
> Thanks,
> Seungwon Jeon
>>>>  	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
>>>>
>>>>  	/* Put in max timeout */
>>>> @@ -2243,6 +2244,7 @@ int dw_mci_probe(struct dw_mci *host)
>>>>  	 * receive ready and error such as transmit, receive timeout, crc error
>>>>  	 */
>>>>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>>>> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
>>> No need, it's already done above.

OK.

>>>
>>>>  	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
>>>>  		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>>>>  		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
>>>> @@ -2393,6 +2395,7 @@ int dw_mci_resume(struct dw_mci *host)
>>>>  	mci_writel(host, FIFOTH, host->fifoth_val);
>>>>
>>>>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>>>> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
>>> Same, incorrect bits.
>>>
>>> Thanks,
>>> Seungwon Jeon
>>>
>>>>  	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
>>>>  		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>>>>  		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
>>>> --
>>>> 1.7.9.5
>>>>
>>>> --
>>>> 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
>>>

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: dw_mmc: clear INSTS register when initialize
  2013-04-24  3:05       ` Joonyoung Shim
@ 2013-04-25  0:45         ` Jaehoon Chung
  2013-04-25  7:49           ` Joonyoung Shim
  0 siblings, 1 reply; 11+ messages in thread
From: Jaehoon Chung @ 2013-04-25  0:45 UTC (permalink / raw)
  To: Joonyoung Shim
  Cc: Seungwon Jeon, 'Jaehoon Chung', linux-mmc, cjb,
	will.newton

On 04/24/2013 12:05 PM, Joonyoung Shim wrote:
> On 04/24/2013 10:34 AM, Seungwon Jeon wrote:
>> On Tuesday, April 23, 2013, Jaehoon Chung wrote:
>>> On 04/23/2013 06:57 PM, Seungwon Jeon wrote:
>>>> Hi,
>>>>
>>>> On Thursday, April 18, 2013, Joonyoung Shim wrote:
>>>>> If pending interrupt for IDMAC exists when probe, it will call interrupt
>>>>> handler unnecessarily.
>>>>>
>>>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>>>>> ---
>>>>>  drivers/mmc/host/dw_mmc.c |    3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>>> index 323c502..b0057a2 100644
>>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>>> @@ -2192,6 +2192,7 @@ int dw_mci_probe(struct dw_mci *host)
>>>>>
>>>>>  	/* Clear the interrupts for the host controller */
>>>>>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>>>>> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
>>>> 0x337 is correct for bits. Could you check the bit filed?
> 
> I feel it's better to use already existing defines than 0x337.
> 
>>> I think that don't care which reset value used.
>>> "It is recommended that you write 0xffff_ffff to the Raw Interrupt register @0x044 and IDSTS @0x8C
>>> in order to clear any pending interrupts before setting the int_enable bit."
>>> This boot mode case is also used the 0xffff_ffff.
> 
> Jaehoon, can i get this sentence from which document?
That comment is existed at "Synopsys DesignWare Cores mobile storage host databook".
So we can use the 0xffff_ffff.

Best Regards,
Jaehoon Chung
> 
>> In case IDSTS all 32bit are not for interrupt status unlike RINTSTS.
>> IDSTS[31:17] is reserved and IDSTS[16:0] also contains 'reserved' and 'read-only' field.
>> Correct use would be needed.
>>
>> Thanks,
>> Seungwon Jeon
>>>>>  	mci_writel(host, INTMASK, 0); /* disable all mmc interrupt first */
>>>>>
>>>>>  	/* Put in max timeout */
>>>>> @@ -2243,6 +2244,7 @@ int dw_mci_probe(struct dw_mci *host)
>>>>>  	 * receive ready and error such as transmit, receive timeout, crc error
>>>>>  	 */
>>>>>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>>>>> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
>>>> No need, it's already done above.
> 
> OK.
> 
>>>>
>>>>>  	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
>>>>>  		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>>>>>  		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
>>>>> @@ -2393,6 +2395,7 @@ int dw_mci_resume(struct dw_mci *host)
>>>>>  	mci_writel(host, FIFOTH, host->fifoth_val);
>>>>>
>>>>>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>>>>> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
>>>> Same, incorrect bits.
>>>>
>>>> Thanks,
>>>> Seungwon Jeon
>>>>
>>>>>  	mci_writel(host, INTMASK, SDMMC_INT_CMD_DONE | SDMMC_INT_DATA_OVER |
>>>>>  		   SDMMC_INT_TXDR | SDMMC_INT_RXDR |
>>>>>  		   DW_MCI_ERROR_FLAGS | SDMMC_INT_CD);
>>>>> --
>>>>> 1.7.9.5
>>>>>
>>>>> --
>>>>> 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
>>>>
> 
> Thanks.
> --
> 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
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: dw_mmc: clear INSTS register when initialize
  2013-04-25  0:45         ` Jaehoon Chung
@ 2013-04-25  7:49           ` Joonyoung Shim
  2013-04-26  4:24             ` Seungwon Jeon
  0 siblings, 1 reply; 11+ messages in thread
From: Joonyoung Shim @ 2013-04-25  7:49 UTC (permalink / raw)
  To: Seungwon Jeon; +Cc: Jaehoon Chung, linux-mmc, cjb, will.newton

On 04/25/2013 09:45 AM, Jaehoon Chung wrote:
> On 04/24/2013 12:05 PM, Joonyoung Shim wrote:
>> On 04/24/2013 10:34 AM, Seungwon Jeon wrote:
>>> On Tuesday, April 23, 2013, Jaehoon Chung wrote:
>>>> On 04/23/2013 06:57 PM, Seungwon Jeon wrote:
>>>>> Hi,
>>>>>
>>>>> On Thursday, April 18, 2013, Joonyoung Shim wrote:
>>>>>> If pending interrupt for IDMAC exists when probe, it will call interrupt
>>>>>> handler unnecessarily.
>>>>>>
>>>>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>>>>>> ---
>>>>>>  drivers/mmc/host/dw_mmc.c |    3 +++
>>>>>>  1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>>>> index 323c502..b0057a2 100644
>>>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>>>> @@ -2192,6 +2192,7 @@ int dw_mci_probe(struct dw_mci *host)
>>>>>>
>>>>>>  	/* Clear the interrupts for the host controller */
>>>>>>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>>>>>> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
>>>>> 0x337 is correct for bits. Could you check the bit filed?
>> I feel it's better to use already existing defines than 0x337.
>>
>>>> I think that don't care which reset value used.
>>>> "It is recommended that you write 0xffff_ffff to the Raw Interrupt register @0x044 and IDSTS @0x8C
>>>> in order to clear any pending interrupts before setting the int_enable bit."
>>>> This boot mode case is also used the 0xffff_ffff.
>> Jaehoon, can i get this sentence from which document?
> That comment is existed at "Synopsys DesignWare Cores mobile storage host databook".
> So we can use the 0xffff_ffff.
>
> Best Regards,
> Jaehoon Chung
>>> In case IDSTS all 32bit are not for interrupt status unlike RINTSTS.
>>> IDSTS[31:17] is reserved and IDSTS[16:0] also contains 'reserved' and 'read-only' field.
>>> Correct use would be needed.

I know, but as Jaehoon said, it recommands to write 0xFFFFFFFF "Synopsys
DesignWare Cores mobile storage host databook" document, so i also think
it's ok.

Nevertheless if you want to use strict bits set, i can modify it. What
is your idea?

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] mmc: dw_mmc: clear INSTS register when initialize
  2013-04-25  7:49           ` Joonyoung Shim
@ 2013-04-26  4:24             ` Seungwon Jeon
  2013-04-26  5:18               ` Jaehoon Chung
  0 siblings, 1 reply; 11+ messages in thread
From: Seungwon Jeon @ 2013-04-26  4:24 UTC (permalink / raw)
  To: 'Joonyoung Shim'
  Cc: 'Jaehoon Chung', linux-mmc, cjb, will.newton

On Thursday, April 25, 2013, Joonyoung Shim wrote:
> On 04/25/2013 09:45 AM, Jaehoon Chung wrote:
> > On 04/24/2013 12:05 PM, Joonyoung Shim wrote:
> >> On 04/24/2013 10:34 AM, Seungwon Jeon wrote:
> >>> On Tuesday, April 23, 2013, Jaehoon Chung wrote:
> >>>> On 04/23/2013 06:57 PM, Seungwon Jeon wrote:
> >>>>> Hi,
> >>>>>
> >>>>> On Thursday, April 18, 2013, Joonyoung Shim wrote:
> >>>>>> If pending interrupt for IDMAC exists when probe, it will call interrupt
> >>>>>> handler unnecessarily.
> >>>>>>
> >>>>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> >>>>>> ---
> >>>>>>  drivers/mmc/host/dw_mmc.c |    3 +++
> >>>>>>  1 file changed, 3 insertions(+)
> >>>>>>
> >>>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> >>>>>> index 323c502..b0057a2 100644
> >>>>>> --- a/drivers/mmc/host/dw_mmc.c
> >>>>>> +++ b/drivers/mmc/host/dw_mmc.c
> >>>>>> @@ -2192,6 +2192,7 @@ int dw_mci_probe(struct dw_mci *host)
> >>>>>>
> >>>>>>  	/* Clear the interrupts for the host controller */
> >>>>>>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
> >>>>>> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
> >>>>> 0x337 is correct for bits. Could you check the bit filed?
> >> I feel it's better to use already existing defines than 0x337.
> >>
> >>>> I think that don't care which reset value used.
> >>>> "It is recommended that you write 0xffff_ffff to the Raw Interrupt register @0x044 and IDSTS
> @0x8C
> >>>> in order to clear any pending interrupts before setting the int_enable bit."
> >>>> This boot mode case is also used the 0xffff_ffff.
> >> Jaehoon, can i get this sentence from which document?
> > That comment is existed at "Synopsys DesignWare Cores mobile storage host databook".
> > So we can use the 0xffff_ffff.
> >
> > Best Regards,
> > Jaehoon Chung
> >>> In case IDSTS all 32bit are not for interrupt status unlike RINTSTS.
> >>> IDSTS[31:17] is reserved and IDSTS[16:0] also contains 'reserved' and 'read-only' field.
> >>> Correct use would be needed.
> 
> I know, but as Jaehoon said, it recommands to write 0xFFFFFFFF "Synopsys
> DesignWare Cores mobile storage host databook" document, so i also think
> it's ok.
> 
> Nevertheless if you want to use strict bits set, i can modify it. What
> is your idea?
Jaehoon,
Thank you for information. I also checked that description, 
but I still feel it doesn't make sense a little bit.
It seems that manual mentions both RINTSTS and IDSTS at a time while focusing the RINTSTS.
In a general approach, it's expected to touch relevant fields.

Thanks,
Seungwon Jeon
> 
> Thanks.
> --
> 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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mmc: dw_mmc: clear INSTS register when initialize
  2013-04-26  4:24             ` Seungwon Jeon
@ 2013-04-26  5:18               ` Jaehoon Chung
  2013-04-26  5:35                 ` Seungwon Jeon
  0 siblings, 1 reply; 11+ messages in thread
From: Jaehoon Chung @ 2013-04-26  5:18 UTC (permalink / raw)
  To: Seungwon Jeon
  Cc: 'Joonyoung Shim', 'Jaehoon Chung', linux-mmc, cjb,
	will.newton

On 04/26/2013 01:24 PM, Seungwon Jeon wrote:
> On Thursday, April 25, 2013, Joonyoung Shim wrote:
>> On 04/25/2013 09:45 AM, Jaehoon Chung wrote:
>>> On 04/24/2013 12:05 PM, Joonyoung Shim wrote:
>>>> On 04/24/2013 10:34 AM, Seungwon Jeon wrote:
>>>>> On Tuesday, April 23, 2013, Jaehoon Chung wrote:
>>>>>> On 04/23/2013 06:57 PM, Seungwon Jeon wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Thursday, April 18, 2013, Joonyoung Shim wrote:
>>>>>>>> If pending interrupt for IDMAC exists when probe, it will call interrupt
>>>>>>>> handler unnecessarily.
>>>>>>>>
>>>>>>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
>>>>>>>> ---
>>>>>>>>  drivers/mmc/host/dw_mmc.c |    3 +++
>>>>>>>>  1 file changed, 3 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>>>>>> index 323c502..b0057a2 100644
>>>>>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>>>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>>>>>> @@ -2192,6 +2192,7 @@ int dw_mci_probe(struct dw_mci *host)
>>>>>>>>
>>>>>>>>  	/* Clear the interrupts for the host controller */
>>>>>>>>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
>>>>>>>> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
>>>>>>> 0x337 is correct for bits. Could you check the bit filed?
>>>> I feel it's better to use already existing defines than 0x337.
>>>>
>>>>>> I think that don't care which reset value used.
>>>>>> "It is recommended that you write 0xffff_ffff to the Raw Interrupt register @0x044 and IDSTS
>> @0x8C
>>>>>> in order to clear any pending interrupts before setting the int_enable bit."
>>>>>> This boot mode case is also used the 0xffff_ffff.
>>>> Jaehoon, can i get this sentence from which document?
>>> That comment is existed at "Synopsys DesignWare Cores mobile storage host databook".
>>> So we can use the 0xffff_ffff.
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>>> In case IDSTS all 32bit are not for interrupt status unlike RINTSTS.
>>>>> IDSTS[31:17] is reserved and IDSTS[16:0] also contains 'reserved' and 'read-only' field.
>>>>> Correct use would be needed.
>>
>> I know, but as Jaehoon said, it recommands to write 0xFFFFFFFF "Synopsys
>> DesignWare Cores mobile storage host databook" document, so i also think
>> it's ok.
>>
>> Nevertheless if you want to use strict bits set, i can modify it. What
>> is your idea?
> Jaehoon,
> Thank you for information. I also checked that description, 
> but I still feel it doesn't make sense a little bit.
> It seems that manual mentions both RINTSTS and IDSTS at a time while focusing the RINTSTS.
> In a general approach, it's expected to touch relevant fields.
Dear Seungwon,

I don't mind which reset value is used. Then we can use the define macro like IDSTS_RESET_VALUE.
It's important that reset the IDSTS register to prevent pending interrupt.

Best Regards,
Jaehoon Chung
> 
> Thanks,
> Seungwon Jeon
>>
>> Thanks.
>> --
>> 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
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] mmc: dw_mmc: clear INSTS register when initialize
  2013-04-26  5:18               ` Jaehoon Chung
@ 2013-04-26  5:35                 ` Seungwon Jeon
  0 siblings, 0 replies; 11+ messages in thread
From: Seungwon Jeon @ 2013-04-26  5:35 UTC (permalink / raw)
  To: 'Jaehoon Chung'
  Cc: 'Joonyoung Shim', linux-mmc, cjb, will.newton

On Friday, April 26, 2013, Jaehoon Chung wrote:
> On 04/26/2013 01:24 PM, Seungwon Jeon wrote:
> > On Thursday, April 25, 2013, Joonyoung Shim wrote:
> >> On 04/25/2013 09:45 AM, Jaehoon Chung wrote:
> >>> On 04/24/2013 12:05 PM, Joonyoung Shim wrote:
> >>>> On 04/24/2013 10:34 AM, Seungwon Jeon wrote:
> >>>>> On Tuesday, April 23, 2013, Jaehoon Chung wrote:
> >>>>>> On 04/23/2013 06:57 PM, Seungwon Jeon wrote:
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> On Thursday, April 18, 2013, Joonyoung Shim wrote:
> >>>>>>>> If pending interrupt for IDMAC exists when probe, it will call interrupt
> >>>>>>>> handler unnecessarily.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
> >>>>>>>> ---
> >>>>>>>>  drivers/mmc/host/dw_mmc.c |    3 +++
> >>>>>>>>  1 file changed, 3 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> >>>>>>>> index 323c502..b0057a2 100644
> >>>>>>>> --- a/drivers/mmc/host/dw_mmc.c
> >>>>>>>> +++ b/drivers/mmc/host/dw_mmc.c
> >>>>>>>> @@ -2192,6 +2192,7 @@ int dw_mci_probe(struct dw_mci *host)
> >>>>>>>>
> >>>>>>>>  	/* Clear the interrupts for the host controller */
> >>>>>>>>  	mci_writel(host, RINTSTS, 0xFFFFFFFF);
> >>>>>>>> +	mci_writel(host, IDSTS, 0xFFFFFFFF);
> >>>>>>> 0x337 is correct for bits. Could you check the bit filed?
> >>>> I feel it's better to use already existing defines than 0x337.
> >>>>
> >>>>>> I think that don't care which reset value used.
> >>>>>> "It is recommended that you write 0xffff_ffff to the Raw Interrupt register @0x044 and IDSTS
> >> @0x8C
> >>>>>> in order to clear any pending interrupts before setting the int_enable bit."
> >>>>>> This boot mode case is also used the 0xffff_ffff.
> >>>> Jaehoon, can i get this sentence from which document?
> >>> That comment is existed at "Synopsys DesignWare Cores mobile storage host databook".
> >>> So we can use the 0xffff_ffff.
> >>>
> >>> Best Regards,
> >>> Jaehoon Chung
> >>>>> In case IDSTS all 32bit are not for interrupt status unlike RINTSTS.
> >>>>> IDSTS[31:17] is reserved and IDSTS[16:0] also contains 'reserved' and 'read-only' field.
> >>>>> Correct use would be needed.
> >>
> >> I know, but as Jaehoon said, it recommands to write 0xFFFFFFFF "Synopsys
> >> DesignWare Cores mobile storage host databook" document, so i also think
> >> it's ok.
> >>
> >> Nevertheless if you want to use strict bits set, i can modify it. What
> >> is your idea?
> > Jaehoon,
> > Thank you for information. I also checked that description,
> > but I still feel it doesn't make sense a little bit.
> > It seems that manual mentions both RINTSTS and IDSTS at a time while focusing the RINTSTS.
> > In a general approach, it's expected to touch relevant fields.
> Dear Seungwon,
> 
> I don't mind which reset value is used. Then we can use the define macro like IDSTS_RESET_VALUE.
> It's important that reset the IDSTS register to prevent pending interrupt.
Sure!
I guess Joonyoung will update the patch soon.

Thanks,
Seungwon Jeon


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-04-26  5:35 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-18  5:21 [PATCH] mmc: dw_mmc: clear INSTS register when initialize Joonyoung Shim
2013-04-18  6:59 ` Jaehoon Chung
2013-04-23  9:57 ` Seungwon Jeon
2013-04-23 10:23   ` Jaehoon Chung
2013-04-24  1:34     ` Seungwon Jeon
2013-04-24  3:05       ` Joonyoung Shim
2013-04-25  0:45         ` Jaehoon Chung
2013-04-25  7:49           ` Joonyoung Shim
2013-04-26  4:24             ` Seungwon Jeon
2013-04-26  5:18               ` Jaehoon Chung
2013-04-26  5:35                 ` Seungwon Jeon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox