[U-Boot] [v2, 1/3] mmc: fsl_esdhc: add esdhc_imx flag

[Lukasz Majewski <lukma@denx.de>]

Hi "Y.b. Lu",

> Hi Lukasz,
> 
> > -----Original Message-----
> > From: Lukasz Majewski <lukma@denx.de>
> > Sent: Monday, February 18, 2019 7:12 AM
> > To: Y.b. Lu <yangbo.lu@nxp.com>
> > Cc: u-boot at lists.denx.de
> > Subject: Re: [U-Boot] [v2, 1/3] mmc: fsl_esdhc: add esdhc_imx flag
> > 
> > Dear "Y.b. Lu",
> >   
> > > The fsl_esdhc driver was for Freescale eSDHC on MPC83XX/MPC85XX
> > > initially. The later QoriQ series processors (which were
> > > evolutions of MPC83XX/MPC85XX) and i.MX series processors were
> > > using this driver for their eSDHCs too.
> > >
> > > So there are two evolution directions for eSDHC now. For the two
> > > series processors, the eSDHCs are becoming more and more
> > > different. We should have split it into two drivers, like them
> > > (sdhci-of-esdhc.c/sdhci-esdhc-imx.c) in linux kernel.  
> > 
> > Why we cannot do it right just from the very beginning?
> > 
> > In the end of the day we will try to mimic Linux kernel anyway,
> > IMHO it is better to start the split now and save some effort on a
> > half-step solution. 
> 
> [Y.b. Lu] I understand. The perfect way is to split them.
> However, if you grep 'CONFIG_FSL_ESDHC' in the whole u-boot source,
> you would find there are 607 results. 

Those are boards, which use the driver. The modification would be done
in one or two files (fsl_esdhc.c|h).

> There will be so many companies
> boards affected. 

I guess that the task of your patch set is to separate imx6q and ppc
specific code for those IP blocks.

> I just don't know who could make all of these boards
> tested.

Your patch set also makes some changes in the generic driver depending
on the priv->esdhc_imx flag. Those changes also need to be tested on
all before mentioned boards.

In the end you logically split the driver,
having it in a single file, which IMHO is not proper long-term solution.

> 
> My current patch-set is also to cleaning up the driver waiting for
> splitting them someday on the other hand. After you check
> CONFIG_FSL_ESDHC in u-boot source, if you think it's better we should
> split them right now, I could work on the driver splitting.

You can think about having common part (in one file - fsl_esdhc.c) and
then separate files with code specific to imx and ppc. This would
reduce the number of changes needed.

> 
> Thanks a lot.
> 
> > > But it seemed
> > > to be a lot of work now. So let's keep as it is. Be very careful
> > > to change the driver if the changes are not common for all
> > > eSDHCs, and clarify i.MX eSDHC specific things to distingush them
> > > with QorIQ eSDHC.
> > >
> > > This patch is to added an esdhc_imx flag for the development of
> > > i.MX eSDHC, to distinguish it with QoriQ eSDHC.
> > >
> > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > > ---
> > > Changes for v2:
> > > 	- Converted to use device_is_compatible().
> > > ---
> > >  drivers/mmc/fsl_esdhc.c | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >
> > > diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c
> > > index 21fa2ab1d4..a647bc7f1c 100644
> > > --- a/drivers/mmc/fsl_esdhc.c
> > > +++ b/drivers/mmc/fsl_esdhc.c
> > > @@ -147,6 +147,7 @@ struct fsl_esdhc_priv {
> > >  	struct gpio_desc cd_gpio;
> > >  	struct gpio_desc wp_gpio;
> > >  #endif
> > > +	bool esdhc_imx;
> > >  };
> > >
> > >  /* Return the XFERTYP flags for a given command and data packet
> > > */ @@ -1462,6 +1463,16 @@ static int fsl_esdhc_probe(struct
> > > udevice *dev) priv->caps = data->caps;
> > >  	}
> > >
> > > +	/*
> > > +	 * This is to specify whether current eSDHC is an i.MX
> > > eSDHC,
> > > +	 * since the i.MX eSDHC has been becoming more and more
> > > different
> > > +	 * with QorIQ eSDHC and initial MPC83XX/MPC85XX.
> > > +	 */
> > > +	if (!device_is_compatible(dev, "fsl,esdhc"))
> > > +		priv->esdhc_imx = true;
> > > +	else
> > > +		priv->esdhc_imx = false;
> > > +
> > >  	val = dev_read_u32_default(dev, "bus-width", -1);
> > >  	if (val == 8)
> > >  		priv->bus_width = 8;  
> > 
> > 
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lukma at denx.de  




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190219/4cb801b8/attachment.sig>