[PATCH] mmc: zynq_sdhci: Dll reset only for ZynqMP platform

[Michal Simek]

Hi,

On 7/11/23 07:00, Jaehoon Chung wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Michal Simek <michal.simek at amd.com>
>> Sent: Monday, July 10, 2023 9:12 PM
>> To: u-boot at lists.denx.de; git at xilinx.com
>> Cc: Ashok Reddy Soma <ashok.reddy.soma at amd.com>; Jaehoon Chung <jh80.chung at samsung.com>; Peng Fan
>> <peng.fan at nxp.com>
>> Subject: [PATCH] mmc: zynq_sdhci: Dll reset only for ZynqMP platform
>>
>> From: Ashok Reddy Soma <ashok.reddy.soma at amd.com>
>>
>> Dll reset is needed only for ZynqMP platforms, add condition in tuning
>> to call arasan_zynqmp_dll_reset() just for ZynqMP platforms.
>>
>> On other platforms like Versal NET, If this condition is not added, we
>> see PLM error messages when dll reset smc is called.
>>
>> Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma at amd.com>
>> Signed-off-by: Michal Simek <michal.simek at amd.com>
>> ---
>>
>>   drivers/mmc/zynq_sdhci.c | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
>> index e779251ce34f..935540d17194 100644
>> --- a/drivers/mmc/zynq_sdhci.c
>> +++ b/drivers/mmc/zynq_sdhci.c
>> @@ -422,7 +422,8 @@ static int arasan_sdhci_execute_tuning(struct mmc *mmc, u8 opcode)
>>
>>   	mdelay(1);
>>
>> -	arasan_zynqmp_dll_reset(host, priv->node_id);
>> +	if (device_is_compatible(mmc->dev, "xlnx,zynqmp-8.9a"))
>> +		arasan_zynqmp_dll_reset(host, priv->node_id);
> 
> How about using local variable to check whether it needs to reset or not?
> It's not efficient to call device_is_compatible() everytime.
> (I'm not sure that it will be added more in future.)
> 
> e.g)
> bool reset = device_is_compatible(mmc->dev, "xlx,zynmp-8.8a");
> 
> if (reset)
> 	arasan_zynqmp_dll_reset(host, priv->node_id);
> 
> ..
> 
> If (reset)
> 	arasan_zynqmp_dll_reset(host, priv->node_id);

This is very valid request and TBH I have already added this to our TODO list to 
convert all device_is_compatible() to flags because over time the driver was 
extended and this construct is used more than it should be.

This is going to be the last device_is_compatible() patch.
Is it fine for you?

Thanks,
Michal