[U-Boot] [PATCH V3] fsl: esdhc: support driver model

[Peng Fan <van.freenix@gmail.com>]

Hi York,

On Mon, Mar 28, 2016 at 05:55:39PM +0000, york sun wrote:
>On 03/27/2016 08:02 AM, Peng Fan wrote:
>> Hi York,
>> 
>> Could you test this patch for PPC and layerscape platform? Since I
>> only test this on i.MX6 platform, I do not want to break PPC or else.
>> 
>> Thanks,
>> Peng.
>> 
>> On Fri, Mar 25, 2016 at 02:16:56PM +0800, Peng Fan wrote:
>>> Support Driver Model for fsl esdhc driver.
>>>
>>> 1. Introduce a new structure struct fsl_esdhc_priv
>>> 2. Refactor fsl_esdhc_initialize which is originally used by board code.
>>>   - Introduce fsl_esdhc_init to be common usage for DM and non-DM
>>>   - Introduce fsl_esdhc_cfg_to_priv to build the bridge for non-DM part.
>>>   - The original API for board code is still there, but we use
>>>     'fsl_esdhc_cfg_to_priv' and 'fsl_esdhc_init' to serve it.
>>> 3. All the functions are changed to use 'struct fsl_esdhc_priv', except
>>>   fsl_esdhc_initialize.
>>> 4. Since clk driver is not implemented, use mxc_get_clock to geth
>>>   the clk and fill 'priv->sdhc_clk'.
>>>
>>> Has been tested on i.MX6UL 14X14 EVK board:
>>> "
>>> =>dm tree
>>> ....
>>> simple_bus  [ + ]    |   `-- aips-bus at 02100000
>>>  mmc        [ + ]    |       |-- usdhc at 02190000
>>>  mmc        [ + ]    |       |-- usdhc at 02194000
>>> ....
>>> => mmc list
>>> FSL_SDHC: 0 (SD)
>>> FSL_SDHC: 1 (SD)
>>> "
>>>
>>> Signed-off-by: Peng Fan <van.freenix@gmail.com>
>>> Cc: York Sun <york.sun@nxp.com>
>>> Cc: Yangbo Lu <yangbo.lu@nxp.com>
>>> Cc: Hector Palacios <hector.palacios@digi.com>
>>> Cc: Eric Nelson <eric@nelint.com>
>>> Cc: Stefano Babic <sbabic@denx.de>
>>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
>>> Cc: Pantelis Antoniou <panto@antoniou-consulting.com>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>> V3:
>>> Fix build error reported by York for PPC.
>>>
>>> V2:
>>> restructure the V1 patch.
>>> Introduce fsl_esdhc_priv structure.
>>> Introduce code to handle cd-gpios and non-removable.
>
><snip>
>
>>> +
>>> +#ifdef CONFIG_DM_MMC
>>> +#include <asm/arch/clock.h>
>>> +static int fsl_esdhc_probe(struct udevice *dev)
>>> +{
>>> +	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>>> +	struct fsl_esdhc_priv *priv = dev_get_priv(dev);
>>> +	const void *fdt = gd->fdt_blob;
>>> +	int node = dev->of_offset;
>>> +	fdt_addr_t addr;
>>> +	unsigned int val;
>>> +	int ret;
>>> +
>>> +	addr = dev_get_addr(dev);
>>> +	if (addr == FDT_ADDR_T_NONE)
>>> +		return -EINVAL;
>>> +
>>> +	priv->esdhc_regs = (struct fsl_esdhc *)addr;
>>> +	priv->dev = dev;
>>> +
>>> +	val = fdtdec_get_int(fdt, node, "bus-width", -1);
>>> +	if (val == 8)
>>> +		priv->bus_width = 8;
>>> +	else if (val == 4)
>>> +		priv->bus_width = 4;
>>> +	else
>>> +		priv->bus_width = 1;
>>> +
>>> +	if (fdt_get_property(fdt, node, "non-removable", NULL)) {
>>> +		priv->non_removable = 1;
>>> +	 } else {
>>> +		priv->non_removable = 0;
>>> +		gpio_request_by_name_nodev(fdt, node, "cd-gpios", 0,
>>> +					   &priv->cd_gpio, GPIOD_IS_IN);
>>> +	}
>>> +
>>> +	/*
>>> +	 * TODO:
>>> +	 * Because lack of clk driver, if SDHC clk is not enabled,
>>> +	 * need to enable it first before this driver is invoked.
>>> +	 *
>>> +	 * we use MXC_ESDHC_CLK to get clk freq.
>>> +	 * If one would like to make this function work,
>>> +	 * the aliases should be provided in dts as this:
>>> +	 *
>>> +	 *  aliases {
>>> +	 *	mmc0 = &usdhc1;
>>> +	 *	mmc1 = &usdhc2;
>>> +	 *	mmc2 = &usdhc3;
>>> +	 *	mmc3 = &usdhc4;
>>> +	 *	};
>>> +	 * Then if your board only supports mmc2 and mmc3, but we can
>>> +	 * correctly get the seq as 2 and 3, then let mxc_get_clock
>>> +	 * work as expected.
>>> +	 */
>>> +	priv->sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK + dev->seq);
>>> +	if (priv->sdhc_clk <= 0) {
>>> +		dev_err(dev, "Unable to get clk for %s\n", dev->name);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	ret = fsl_esdhc_init(priv);
>>> +	if (ret) {
>>> +		dev_err(dev, "fsl_esdhc_init failure\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	upriv->mmc = priv->mmc;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct udevice_id fsl_esdhc_ids[] = {
>>> +	{ .compatible = "fsl,imx6ul-usdhc", },
>>> +	{ .compatible = "fsl,imx6sx-usdhc", },
>>> +	{ .compatible = "fsl,imx6sl-usdhc", },
>>> +	{ .compatible = "fsl,imx6q-usdhc", },
>>> +	{ .compatible = "fsl,imx7d-usdhc", },
>>> +	{ /* sentinel */ }
>>> +};
>>> +
>>> +U_BOOT_DRIVER(fsl_esdhc) = {
>>> +	.name	= "fsl-esdhc-mmc",
>>> +	.id	= UCLASS_MMC,
>>> +	.of_match = fsl_esdhc_ids,
>>> +	.probe	= fsl_esdhc_probe,
>>> +	.priv_auto_alloc_size = sizeof(struct fsl_esdhc_priv),
>>> +};
>>> +#endif
>>> -- 
>>> 2.6.2
>>>
>> 
>
>Peng,
>
>You are not breaking PPC yet. The new function you added fsl_esdhc_probe() is
>gated by CONFIG_DM_MMC. This macro is not defined for any PPC platform.

Thanks for the test. If CONFIG_DM_MMC is enabled for PPC, I think the
only thing that may break PPC is mxc_get_clock, because this function
is only defined for i.MX.

I am not very sure whether we need a more complicated clk driver
following linux, since it may make board/soc bringup work more
complicated. Anyway, later when PPC want to enable CONFIG_DM_MMC,
mxc_get_clock should be refined to support i.MX and PPC or we develop
a general function to support both.

Thanks,
Peng.
>
>York