[PATCH] mmc: fsl_esdhc_imx: add wait_dat0() support

[Jaehoon Chung]

On 11/3/20 6:03 PM, Bough Chen wrote:
> 
>> -----Original Message-----
>> From: Peng Fan
>> Sent: 2020年11月3日 16:17
>> To: Jaehoon Chung <jh80.chung at samsung.com>; Bough Chen
>> <haibo.chen at nxp.com>; u-boot at lists.denx.de
>> Cc: dl-uboot-imx <uboot-imx at nxp.com>
>> Subject: RE: [PATCH] mmc: fsl_esdhc_imx: add wait_dat0() support
>>
>>> Subject: Re: [PATCH] mmc: fsl_esdhc_imx: add wait_dat0() support
>>>
>>> On 11/2/20 8:17 PM, haibo.chen at nxp.com wrote:
>>>> From: Haibo Chen <haibo.chen at nxp.com>
>>>>
>>>> Add wait_dat0() support, upper layer will use this callback.
>>>>
>>>> Signed-off-by: Haibo Chen <haibo.chen at nxp.com>
>>>> ---
>>>>  drivers/mmc/fsl_esdhc_imx.c | 23 +++++++++++++++++++++++
>>>>  1 file changed, 23 insertions(+)
>>>>
>>>> diff --git a/drivers/mmc/fsl_esdhc_imx.c
>>>> b/drivers/mmc/fsl_esdhc_imx.c index 22040c67a8..dc6a6006fa 100644
>>>> --- a/drivers/mmc/fsl_esdhc_imx.c
>>>> +++ b/drivers/mmc/fsl_esdhc_imx.c
>>>> @@ -1646,6 +1646,28 @@ static int
>>>> fsl_esdhc_set_enhanced_strobe(struct
>>>> udevice *dev)  }  #endif
>>>>
>>>> +static int fsl_esdhc_wait_dat0(struct udevice *dev, int state,
>>>> +				int timeout_us)
>>>> +{
>>>> +	int ret = -ETIMEDOUT;
>>>> +	bool dat0_high;
>>>> +	bool target_dat0_high = !!state;
>>>> +	struct fsl_esdhc_priv *priv = dev_get_priv(dev);
>>>> +	struct fsl_esdhc *regs = priv->esdhc_regs;
>>>> +
>>>> +	timeout_us = DIV_ROUND_UP(timeout_us, 10); /* check every 10
>> us. */
>>>> +	while (timeout_us--) {
>>>> +		dat0_high = !!(esdhc_read32(&regs->prsstat) &
>> PRSSTAT_DAT0);
>>>> +		if (dat0_high == target_dat0_high) {
>>>> +			ret = 0;
>>>> +			break;
>>>> +		}
>>>> +	udelay(10);
>>>
>>> Fix indent.
>>> And can't use wait_for_bit_xx()?
>>
>> +1, read_poll_timeout or similar should be used here.
> 
> Do not use read_poll_timeout here is because the parameter 'state' can be 0 or 1.
> if use read_poll_timeout, should like this:
> read_poll_timeout(esdhc_read32, &regs->prsstat, tmp, !!(tmp & PRSSTAT_DAT0) == !!state, 10, timeout_us);> 
> I think the condition is a little bit complicated, not that readable, but if you prefer this, I can do that.

I think it's not complicated.
I don't have any objection but you can follow Peng's opinion. :)
		
Best Regards,
Jaehoon Chung

> 
> Best Regards
> Haibo Chen
>>
>> Thanks,
>> Peng.
>>
>>>
>>> Best  Regards,
>>> Jaehoon Chung
>>>
>>>> +	}
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>>  static const struct dm_mmc_ops fsl_esdhc_ops = {
>>>>  	.get_cd		= fsl_esdhc_get_cd,
>>>>  	.send_cmd	= fsl_esdhc_send_cmd,
>>>> @@ -1656,6 +1678,7 @@ static const struct dm_mmc_ops
>> fsl_esdhc_ops =
>>> {
>>>> #if CONFIG_IS_ENABLED(MMC_HS400_ES_SUPPORT)
>>>>  	.set_enhanced_strobe = fsl_esdhc_set_enhanced_strobe,  #endif
>>>> +	.wait_dat0 = fsl_esdhc_wait_dat0,
>>>>  };
>>>>  #endif
>>>>
>>>>
>