Re: [PATCH] mmc: dw_mmc: clear INSTS register when initialize

[Jaehoon Chung <jh80.chung@samsung.com>]

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
>