diff mbox

mmc: dw_mmc: clear INSTS register when initialize

Message ID 1366262497-22560-1-git-send-email-jy0922.shim@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joonyoung Shim April 18, 2013, 5:21 a.m. UTC
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(+)

Comments

Jaehoon Chung April 18, 2013, 6:59 a.m. UTC | #1
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);
> 

--
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
Seungwon Jeon April 23, 2013, 9:57 a.m. UTC | #2
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

--
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
Jaehoon Chung April 23, 2013, 10:23 a.m. UTC | #3
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
> 
> 

--
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
Seungwon Jeon April 24, 2013, 1:34 a.m. UTC | #4
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
> >
> >

--
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
Joonyoung Shim April 24, 2013, 3:05 a.m. UTC | #5
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.
--
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
Jaehoon Chung April 25, 2013, 12:45 a.m. UTC | #6
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
> 

--
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
Joonyoung Shim April 25, 2013, 7:49 a.m. UTC | #7
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.
--
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
Seungwon Jeon April 26, 2013, 4:24 a.m. UTC | #8
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

--
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
Jaehoon Chung April 26, 2013, 5:18 a.m. UTC | #9
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
> 

--
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
Seungwon Jeon April 26, 2013, 5:35 a.m. UTC | #10
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

--
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
diff mbox

Patch

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);