Re: [PATCH V3 4/4] mmc: sdhci-msm: Rectify DLL programming sequence for SDCC

[Ram Prakash Gupta]

Hi Dmitry,

I will start on this with addressing your comments in previous version as
suggested.

Thanks,
Ram

On 1/22/2025 3:30 PM, Dmitry Baryshkov wrote:
> On Wed, Jan 22, 2025 at 03:17:07PM +0530, Sachin Gupta wrote:
>> With the current DLL sequence stability issues for data
>> transfer seen in HS400 and HS200 modes.
>>
>> "mmc0: cqhci: error IRQ status: 0x00000000 cmd error -84
>> data error 0"
>>
>> Rectify the DLL programming sequence as per latest hardware
>> programming guide
>>
>> Signed-off-by: Sachin Gupta <quic_sachgupt@xxxxxxxxxxx>
>> ---
>>  drivers/mmc/host/sdhci-msm.c | 270 ++++++++++++++++++++++++++++++++---
>>  1 file changed, 252 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index cc7756a59c55..17f17a635d83 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -28,6 +28,7 @@
>>  #define CORE_VERSION_MAJOR_SHIFT	28
>>  #define CORE_VERSION_MAJOR_MASK		(0xf << CORE_VERSION_MAJOR_SHIFT)
>>  #define CORE_VERSION_MINOR_MASK		0xff
>> +#define SDHCI_MSM_MIN_V_7FF		0x6e
>>  
>>  #define CORE_MCI_GENERICS		0x70
>>  #define SWITCHABLE_SIGNALING_VOLTAGE	BIT(29)
>> @@ -118,7 +119,8 @@
>>  #define CORE_PWRSAVE_DLL	BIT(3)
>>  
>>  #define DDR_CONFIG_POR_VAL	0x80040873
>> -
>> +#define DLL_CONFIG_3_POR_VAL	0x10
>> +#define TCXO_FREQ               19200000
>>  
>>  #define INVALID_TUNING_PHASE	-1
>>  #define SDHCI_MSM_MIN_CLOCK	400000
>> @@ -309,6 +311,16 @@ struct sdhci_msm_host {
>>  	bool artanis_dll;
>>  };
>>  
>> +enum dll_init_context {
>> +	DLL_INIT_NORMAL,
>> +	DLL_INIT_FROM_CX_COLLAPSE_EXIT,
>> +};
>> +
>> +enum mode {
>> +	HS400, // equivalent to SDR104 mode for DLL.
>> +	HS200, // equivalent to SDR50 mode for DLL.
>> +};
>> +
>>  static const struct sdhci_msm_offset *sdhci_priv_msm_offset(struct sdhci_host *host)
>>  {
>>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> @@ -793,6 +805,211 @@ static int msm_init_cm_dll(struct sdhci_host *host)
>>  	return 0;
>>  }
>>  
>> +static unsigned int sdhci_msm_get_min_clock(struct sdhci_host *host)
>> +{
>> +	return SDHCI_MSM_MIN_CLOCK;
>> +}
>> +
>> +static unsigned int sdhci_msm_get_clk_rate(struct sdhci_host *host, u32 req_clk)
>> +{
>> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> +	struct sdhci_msm_host *msm_host = sdhci_pltfm_priv(pltfm_host);
>> +	struct clk *core_clk = msm_host->bulk_clks[0].clk;
>> +	unsigned int sup_clk;
>> +
>> +	if (req_clk < sdhci_msm_get_min_clock(host))
>> +		return sdhci_msm_get_min_clock(host);
>> +
>> +	sup_clk = clk_get_rate(core_clk);
>> +
>> +	if (host->clock != msm_host->clk_rate)
>> +		sup_clk = sup_clk / 2;
> Please resolve previous discussions before sending new versions. Just
> sending a response and then sending next iteration of the patchset is
> not a proper way to communicate.
>
> NAK until the discussion is resolved in the previous thread.
>
>> +
>> +	return sup_clk;
>> +}
>> +