diff mbox

[U-Boot,v2,1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY for CMD with busy response

Message ID 1470129653-15854-1-git-send-email-yangbo.lu@nxp.com
State Changes Requested
Delegated to: York Sun
Headers show

Commit Message

Yangbo Lu Aug. 2, 2016, 9:20 a.m. UTC 
For CMD with busy response, the eSDHC driver would poll DAT0 until
CMD completion rather than polling IRQSTAT. So, don't set
XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v2:
	- None
---
 drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

York Sun Sept. 12, 2016, 7:23 p.m. UTC | #1 
Panto,

On 08/02/2016 02:32 AM, Yangbo Lu wrote:
> For CMD with busy response, the eSDHC driver would poll DAT0 until
> CMD completion rather than polling IRQSTAT. So, don't set
> XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.
>
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
> Changes for v2:
> 	- None
> ---
>  drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>

I noticed this set wasn't CC'ing you. Can you check and comment/ack this 
set?

York
Yangbo Lu Sept. 13, 2016, 7:30 a.m. UTC | #2 
Hi York,

I found the MMC maintainer had been changed to Jeahoon.
It seems Panto haven't commented on upstream for a very long time.

MMC
M:      Jaehoon Chung <jh80.chung@samsung.com>
S:      Maintained
T:      git git://git.denx.de/u-boot-mmc.git
F:      drivers/mmc/

Thanks.


Best regards,
Yangbo Lu


> -----Original Message-----
> From: york sun
> Sent: Tuesday, September 13, 2016 3:24 AM
> To: Y.B. Lu; u-boot@lists.denx.de; Pantelis Antoniou
> Cc: Jaehoon Chung
> Subject: Re: [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY
> for CMD with busy response
> 
> Panto,
> 
> On 08/02/2016 02:32 AM, Yangbo Lu wrote:
> > For CMD with busy response, the eSDHC driver would poll DAT0 until CMD
> > completion rather than polling IRQSTAT. So, don't set
> > XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.
> >
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > ---
> > Changes for v2:
> > 	- None
> > ---
> >  drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> 
> I noticed this set wasn't CC'ing you. Can you check and comment/ack this
> set?
> 
> York
Jaehoon Chung Sept. 19, 2016, 12:06 a.m. UTC | #3 
Hi Yangbo,

On 08/02/2016 06:20 PM, Yangbo Lu wrote:
> For CMD with busy response, the eSDHC driver would poll DAT0 until
> CMD completion rather than polling IRQSTAT. So, don't set
> XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.

Sorry for late.. I missed your patchset.

> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
> Changes for v2:
> 	- None
> ---
>  drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> index a865c7b..b23845d 100644
> --- a/drivers/mmc/fsl_esdhc.c
> +++ b/drivers/mmc/fsl_esdhc.c
> @@ -136,8 +136,16 @@ static uint esdhc_xfertyp(struct mmc_cmd *cmd, struct mmc_data *data)
>  		xfertyp |= XFERTYP_CICEN;
>  	if (cmd->resp_type & MMC_RSP_136)
>  		xfertyp |= XFERTYP_RSPTYP_136;
> -	else if (cmd->resp_type & MMC_RSP_BUSY)
> -		xfertyp |= XFERTYP_RSPTYP_48_BUSY;
> +	/*
> +	 * For CMD with busy response, the eSDHC driver would poll DAT0
> +	 * until CMD completion rather than polling IRQSTAT. So, don't
> +	 * set XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC)
> +	 * in IRQSTAT.
> +	 *
> +	 * Remove:
> +	 * else if (cmd->resp_type & MMC_RSP_BUSY)
> +	 *      xfertyp |= XFERTYP_RSPTYP_48_BUSY;
> +	 */

I don't have the board that is using the fsl_esdhc driver.
I wonder that it doesn't need to set XFERTYP_RSPTYP_48_BUSY in future.
If so be, is it possible to remove this comments?

Why add this comment?

Best Regards,
Jaehoon Chung

>  	else if (cmd->resp_type & MMC_RSP_PRESENT)
>  		xfertyp |= XFERTYP_RSPTYP_48;
>  
>
Jaehoon Chung Sept. 19, 2016, 12:16 a.m. UTC | #4 
On 09/13/2016 04:30 PM, Y.B. Lu wrote:
> Hi York,
> 
> I found the MMC maintainer had been changed to Jeahoon.
> It seems Panto haven't commented on upstream for a very long time.
> 
> MMC
> M:      Jaehoon Chung <jh80.chung@samsung.com>
> S:      Maintained
> T:      git git://git.denx.de/u-boot-mmc.git
> F:      drivers/mmc/
> 
> Thanks.

Thanks for reminding. I missed these patchset.
Sorry.

Best Regards,
Jaehoon Chung

> 
> 
> Best regards,
> Yangbo Lu
> 
> 
>> -----Original Message-----
>> From: york sun
>> Sent: Tuesday, September 13, 2016 3:24 AM
>> To: Y.B. Lu; u-boot@lists.denx.de; Pantelis Antoniou
>> Cc: Jaehoon Chung
>> Subject: Re: [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY
>> for CMD with busy response
>>
>> Panto,
>>
>> On 08/02/2016 02:32 AM, Yangbo Lu wrote:
>>> For CMD with busy response, the eSDHC driver would poll DAT0 until CMD
>>> completion rather than polling IRQSTAT. So, don't set
>>> XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.
>>>
>>> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
>>> ---
>>> Changes for v2:
>>> 	- None
>>> ---
>>>  drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
>>>  1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>
>> I noticed this set wasn't CC'ing you. Can you check and comment/ack this
>> set?
>>
>> York
> 
> 
> 
>
Yangbo Lu Sept. 23, 2016, 7:34 a.m. UTC | #5 
> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
> Sent: Monday, September 19, 2016 8:07 AM
> To: Y.B. Lu; u-boot@lists.denx.de
> Cc: york sun
> Subject: Re: [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY
> for CMD with busy response
> 
> Hi Yangbo,
> 
> On 08/02/2016 06:20 PM, Yangbo Lu wrote:
> > For CMD with busy response, the eSDHC driver would poll DAT0 until CMD
> > completion rather than polling IRQSTAT. So, don't set
> > XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.
> 
> Sorry for late.. I missed your patchset.
> 
> >
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > ---
> > Changes for v2:
> > 	- None
> > ---
> >  drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index
> > a865c7b..b23845d 100644
> > --- a/drivers/mmc/fsl_esdhc.c
> > +++ b/drivers/mmc/fsl_esdhc.c
> > @@ -136,8 +136,16 @@ static uint esdhc_xfertyp(struct mmc_cmd *cmd,
> struct mmc_data *data)
> >  		xfertyp |= XFERTYP_CICEN;
> >  	if (cmd->resp_type & MMC_RSP_136)
> >  		xfertyp |= XFERTYP_RSPTYP_136;
> > -	else if (cmd->resp_type & MMC_RSP_BUSY)
> > -		xfertyp |= XFERTYP_RSPTYP_48_BUSY;
> > +	/*
> > +	 * For CMD with busy response, the eSDHC driver would poll DAT0
> > +	 * until CMD completion rather than polling IRQSTAT. So, don't
> > +	 * set XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC)
> > +	 * in IRQSTAT.
> > +	 *
> > +	 * Remove:
> > +	 * else if (cmd->resp_type & MMC_RSP_BUSY)
> > +	 *      xfertyp |= XFERTYP_RSPTYP_48_BUSY;
> > +	 */
> 
> I don't have the board that is using the fsl_esdhc driver.
> I wonder that it doesn't need to set XFERTYP_RSPTYP_48_BUSY in future.
> If so be, is it possible to remove this comments?
> 
> Why add this comment?

[Lu Yangbo-B47093] I added this comment to explain why there isn't XFERTYP_RSPTYP_48_BUSY setting in esdhc_xfertyp().
Because usually the xfertyp should be XFERTYP_RSPTYP_48_BUSY for cmd with busy response.

Although we don't need to set XFERTYP_RSPTYP_48_BUSY in the future, I'd like to keep an explain comment here if possible :)

> 
> Best Regards,
> Jaehoon Chung
> 
> >  	else if (cmd->resp_type & MMC_RSP_PRESENT)
> >  		xfertyp |= XFERTYP_RSPTYP_48;
> >
> >
Yangbo Lu Sept. 23, 2016, 7:59 a.m. UTC | #6 
> -----Original Message-----
> From: Jaehoon Chung [mailto:jh80.chung@samsung.com]
> Sent: Monday, September 19, 2016 8:17 AM
> To: Y.B. Lu; york sun; u-boot@lists.denx.de; Pantelis Antoniou
> Subject: Re: [v2, 1/5] mmc: fsl_esdhc: don't set XFERTYP_RSPTYP_48_BUSY
> for CMD with busy response
> 
> On 09/13/2016 04:30 PM, Y.B. Lu wrote:
> > Hi York,
> >
> > I found the MMC maintainer had been changed to Jeahoon.
> > It seems Panto haven't commented on upstream for a very long time.
> >
> > MMC
> > M:      Jaehoon Chung <jh80.chung@samsung.com>
> > S:      Maintained
> > T:      git git://git.denx.de/u-boot-mmc.git
> > F:      drivers/mmc/
> >
> > Thanks.
> 
> Thanks for reminding. I missed these patchset.
> Sorry.
> 

[Lu Yangbo-B47093] That's ok. Thank you for your comments :)

> Best Regards,
> Jaehoon Chung
> 
> >
> >
> > Best regards,
> > Yangbo Lu
> >
> >
> >> -----Original Message-----
> >> From: york sun
> >> Sent: Tuesday, September 13, 2016 3:24 AM
> >> To: Y.B. Lu; u-boot@lists.denx.de; Pantelis Antoniou
> >> Cc: Jaehoon Chung
> >> Subject: Re: [v2, 1/5] mmc: fsl_esdhc: don't set
> >> XFERTYP_RSPTYP_48_BUSY for CMD with busy response
> >>
> >> Panto,
> >>
> >> On 08/02/2016 02:32 AM, Yangbo Lu wrote:
> >>> For CMD with busy response, the eSDHC driver would poll DAT0 until
> >>> CMD completion rather than polling IRQSTAT. So, don't set
> >>> XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC) in IRQSTAT.
> >>>
> >>> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> >>> ---
> >>> Changes for v2:
> >>> 	- None
> >>> ---
> >>>  drivers/mmc/fsl_esdhc.c | 12 ++++++++++--
> >>>  1 file changed, 10 insertions(+), 2 deletions(-)
> >>>
> >>
> >> I noticed this set wasn't CC'ing you. Can you check and comment/ack
> >> this set?
> >>
> >> York
> >
> >
> >
> >
diff mbox

Patch

diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
index a865c7b..b23845d 100644
--- a/drivers/mmc/fsl_esdhc.c
+++ b/drivers/mmc/fsl_esdhc.c
@@ -136,8 +136,16 @@  static uint esdhc_xfertyp(struct mmc_cmd *cmd, struct mmc_data *data)
 		xfertyp |= XFERTYP_CICEN;
 	if (cmd->resp_type & MMC_RSP_136)
 		xfertyp |= XFERTYP_RSPTYP_136;
-	else if (cmd->resp_type & MMC_RSP_BUSY)
-		xfertyp |= XFERTYP_RSPTYP_48_BUSY;
+	/*
+	 * For CMD with busy response, the eSDHC driver would poll DAT0
+	 * until CMD completion rather than polling IRQSTAT. So, don't
+	 * set XFERTYP_RSPTYP_48_BUSY to avoid interrupts (DTOE or TC)
+	 * in IRQSTAT.
+	 *
+	 * Remove:
+	 * else if (cmd->resp_type & MMC_RSP_BUSY)
+	 *      xfertyp |= XFERTYP_RSPTYP_48_BUSY;
+	 */
 	else if (cmd->resp_type & MMC_RSP_PRESENT)
 		xfertyp |= XFERTYP_RSPTYP_48;