-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at intel.com]
Sent: Friday, February 23, 2018 1:04 PM
To: Manish Narani <redacted>; michal.simek at xilinx.com;
ulf.hansson at linaro.org; linux-arm-kernel at lists.infradead.org; linux-
mmc at vger.kernel.org; linux-kernel at vger.kernel.org;
devicetree at vger.kernel.org; mark.rutland at arm.com; robh+dt at kernel.org
Cc: Anirudha Sarangi <redacted>; Srinivas Goud
[off-list ref]
Subject: Re: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning support for
ZynqMP Platform

On 23/02/18 09:29, Manish Narani wrote:

quoted

quoted

-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at intel.com]
Sent: Thursday, February 22, 2018 1:50 PM
To: Manish Narani <redacted>; michal.simek at xilinx.com;
ulf.hansson at linaro.org; linux-arm-kernel at lists.infradead.org; linux-
mmc at vger.kernel.org; linux-kernel at vger.kernel.org;
devicetree at vger.kernel.org; mark.rutland at arm.com;

robh+dt at kernel.org

quoted

quoted

Cc: Anirudha Sarangi <redacted>; Srinivas Goud
[off-list ref]
Subject: Re: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning
support for ZynqMP Platform

On 21/02/18 17:00, Manish Narani wrote:

quoted

Hi Adrian,

quoted

-----Original Message-----
From: Manish Narani
Sent: Wednesday, February 21, 2018 11:39 AM
To: Adrian Hunter <adrian.hunter@intel.com>;
michal.simek at xilinx.com; ulf.hansson at linaro.org;
linux-arm-kernel at lists.infradead.org; linux- mmc at vger.kernel.org;
linux-kernel at vger.kernel.org; devicetree at vger.kernel.org;
mark.rutland at arm.com;

robh+dt at kernel.org

quoted

quoted

Cc: Anirudha Sarangi <redacted>; Srinivas Goud
[off-list ref]
Subject: RE: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning
support for ZynqMP Platform

Hi Adrian,

quoted

-----Original Message-----
From: Adrian Hunter [mailto:adrian.hunter at intel.com]
Sent: Friday, February 16, 2018 7:37 PM
To: Manish Narani <redacted>;

michal.simek at xilinx.com;

quoted

quoted

quoted

quoted

quoted

ulf.hansson at linaro.org; linux-arm-kernel at lists.infradead.org;
linux- mmc at vger.kernel.org; linux-kernel at vger.kernel.org;
devicetree at vger.kernel.org; mark.rutland at arm.com;

robh+dt at kernel.org

quoted

quoted

quoted

Cc: Anirudha Sarangi <redacted>; Srinivas Goud
[off-list ref]; Manish Narani [off-list ref]
Subject: Re: [RFC PATCH] mmc: sdhci-of-arasan: Add auto tuning
support for ZynqMP Platform

On 30/01/18 20:14, Manish Narani wrote:

quoted

This patch adds support of SD auto tuning for ZynqMP platform.
Auto tuning sequence sends tuning block to card when operating in
UHS-1 modes. This resets the DLL and sends CMD19/CMD21 as a part
of the auto tuning process. Once the auto tuning process gets
completed, reset the DLL to load the newly obtained SDHC tuned tap

value.

quoted

quoted

quoted

quoted

quoted

How is this different from:
	1. reset the dll
	2. call sdhci_execute_tuning
	3. reset the dll

Below is my take on your above comments:
- 'Reset the dll' is a platform specific call inside
'arasan_zynqmp_execute_tuning' which is implemented in
sdhci-of-arasan.c
- 'arasan_zynqmp_execute_tuning' is called from

'sdhci_execute_tuning'

quoted

quoted

quoted

as a platform_execute_tuning routine
- So to keep 'DLL reset' routine called from sdhci-of-arasan.c, I
have implemented the execute_tuning in sdhci-of-arasan.c

I meant something like:

	if (of_device_is_compatible(pdev->dev.of_node, "xlnx,zynqmp-
8.9a"))
		host->mmc_host_ops.execute_tuning =

arasan_zynqmp_execute_tuning;

quoted

quoted

This will need the removal of 'const' from 'static const struct

mmc_host_ops sdhci_ops = {}' in sdhci.c file. Please confirm.

No, it is not const.  You are not looking at the right place. i.e.

$ grep mmc_host_ops drivers/mmc/host/sdhci.h
        struct mmc_host_ops mmc_host_ops;       /* MMC host ops */

[Manish] 
This looks fair. I got your point. I will send another patch for the same.

quoted

Thanks,
Manish

quoted

static int arasan_zynqmp_execute_tuning(struct mmc_host *mmc, u32
opcode) {
	struct sdhci_host *host = mmc_priv(mmc);
	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
	struct sdhci_arasan_data *sdhci_arasan =
sdhci_pltfm_priv(pltfm_host);
	int err;

	arasan_zynqmp_dll_reset(host, sdhci_arasan->device_id);

	err = sdhci_execute_tuning(mmc, opcode);
	if (err)
		return err;

	arasan_zynqmp_dll_reset(host, sdhci_arasan->device_id);

	return 0;
}

quoted

Alternative way (Please review):
- Define a host->quirk2 bit (SDHCI_QUIRK2_DLL_RESET_NEEDED) in
sdhci-of-arasan.c indicating DLL reset needed while tuning operation
- Call 'dll reset' routine before and after __sdhci_execute_tuning()
in sdhci.c when a host->quirk2 bit (SDHCI_QUIRK2_DLL_RESET_NEEDED)
is set

We should try to avoid quirks.