* [U-Boot] [PATCH 1/2] fsl: esdhc: consolidate fsl_esdhc_cfg structure @ 2016-03-10 8:57 Peng Fan 2016-03-10 8:57 ` [U-Boot] [PATCH 2/2] fsl: esdhc: support driver model Peng Fan 0 siblings, 1 reply; 6+ messages in thread From: Peng Fan @ 2016-03-10 8:57 UTC (permalink / raw) To: u-boot We can use phys_addr_to for esdhc_base to discard the #ifdef. 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> --- include/fsl_esdhc.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h index 073048f..fa760a5 100644 --- a/include/fsl_esdhc.h +++ b/include/fsl_esdhc.h @@ -168,11 +168,7 @@ #define ESDHC_VENDORSPEC_VSELECT 0x00000002 /* Use 1.8V */ struct fsl_esdhc_cfg { -#ifdef CONFIG_FSL_LAYERSCAPE - u64 esdhc_base; -#else - u32 esdhc_base; -#endif + phys_addr_t esdhc_base; u32 sdhc_clk; u8 max_bus_width; struct mmc_config cfg; -- 2.6.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] fsl: esdhc: support driver model 2016-03-10 8:57 [U-Boot] [PATCH 1/2] fsl: esdhc: consolidate fsl_esdhc_cfg structure Peng Fan @ 2016-03-10 8:57 ` Peng Fan 2016-03-12 0:33 ` Simon Glass 0 siblings, 1 reply; 6+ messages in thread From: Peng Fan @ 2016-03-10 8:57 UTC (permalink / raw) To: u-boot Support Driver Model for fsl esdhc driver. In order to minimize the change, reuse the fsl_esdhc_initialize function. This new way is to fill an fsl_esdhc_cfg struture and pass it to fsl_esdhc_initialize, just like the code in different board codes. Introduce a 'struct mmc *mmc' entry in fsl_esdhc_cfg structure, otherwise fsl_esdhc_initialize may need to be restructured which will cause lots changes for board code. Since clk driver is not implemented, use mxc_get_clock here to fill cfg->sdhc_clk. Anyway we can utilize the pinctrl imx driver now, except the SPL part, we can drop the pinmux settings from board file for mmc. There are so many "ifdef" in the file, maybe we can use driver data or quirks to cover these. But to minimize changes for this patch, these are not included. Later we can try to discard the nasty ifdef. Has been tested on i.MX6UL 9x9 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> --- drivers/mmc/fsl_esdhc.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++ include/fsl_esdhc.h | 1 + 2 files changed, 82 insertions(+) diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index ea5f4bf..1bd2174 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -20,6 +20,8 @@ #include <fsl_esdhc.h> #include <fdt_support.h> #include <asm/io.h> +#include <dm.h> +#include <asm/arch/clock.h> DECLARE_GLOBAL_DATA_PTR; @@ -739,6 +741,8 @@ int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg) if (mmc == NULL) return -1; + cfg->mmc = mmc; + return 0; } @@ -819,3 +823,80 @@ void fdt_fixup_esdhc(void *blob, bd_t *bd) 4 + 1, 1); } #endif + +#ifdef CONFIG_DM_MMC +static int fsl_esdhc_probe(struct udevice *dev) +{ + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev); + struct fsl_esdhc_cfg *cfg = dev_get_priv(dev); + const void *fdt = gd->fdt_blob; + fdt_addr_t addr; + unsigned int val; + int node = dev->of_offset; + int ret; + + addr = dev_get_addr(dev); + if (addr == FDT_ADDR_T_NONE) + return -EINVAL; + + cfg->esdhc_base = (phys_addr_t)addr; + + val = fdtdec_get_int(fdt, node, "bus-width", -1); + if (val == 8) + cfg->max_bus_width = 8; + else if (val == 4) + cfg->max_bus_width = 4; + else + cfg->max_bus_width = 1; + + /* + * 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. + */ + cfg->sdhc_clk = mxc_get_clock(MXC_ESDHC_CLK + dev->seq); + if (cfg->sdhc_clk <= 0) { + dev_err(dev, "Unable to get clk for %s\n", dev->name); + return -EINVAL; + } + + ret = fsl_esdhc_initialize(NULL, cfg); + if (ret) + return ret; + + upriv->mmc = cfg->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_cfg), +}; +#endif diff --git a/include/fsl_esdhc.h b/include/fsl_esdhc.h index fa760a5..e0d3265 100644 --- a/include/fsl_esdhc.h +++ b/include/fsl_esdhc.h @@ -172,6 +172,7 @@ struct fsl_esdhc_cfg { u32 sdhc_clk; u8 max_bus_width; struct mmc_config cfg; + struct mmc *mmc; }; /* Select the correct accessors depending on endianess */ -- 2.6.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] fsl: esdhc: support driver model 2016-03-10 8:57 ` [U-Boot] [PATCH 2/2] fsl: esdhc: support driver model Peng Fan @ 2016-03-12 0:33 ` Simon Glass 2016-03-12 4:21 ` Peng Fan 0 siblings, 1 reply; 6+ messages in thread From: Simon Glass @ 2016-03-12 0:33 UTC (permalink / raw) To: u-boot Hi Peng, On 10 March 2016 at 01:57, Peng Fan <van.freenix@gmail.com> wrote: > Support Driver Model for fsl esdhc driver. > > In order to minimize the change, reuse the fsl_esdhc_initialize function. > This new way is to fill an fsl_esdhc_cfg struture and pass it > to fsl_esdhc_initialize, just like the code in different board codes. > > Introduce a 'struct mmc *mmc' entry in fsl_esdhc_cfg structure, > otherwise fsl_esdhc_initialize may need to be restructured which will > cause lots changes for board code. > > Since clk driver is not implemented, use mxc_get_clock here to > fill cfg->sdhc_clk. Anyway we can utilize the pinctrl imx driver > now, except the SPL part, we can drop the pinmux settings from > board file for mmc. > > There are so many "ifdef" in the file, maybe we can use driver data > or quirks to cover these. But to minimize changes for this patch, > these are not included. Later we can try to discard the nasty > ifdef. > > Has been tested on i.MX6UL 9x9 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> > --- > drivers/mmc/fsl_esdhc.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++ > include/fsl_esdhc.h | 1 + > 2 files changed, 82 insertions(+) I'm nervous about this patch. It is calling board code from a driver, which we should avoid. Perhaps it would be better to start by refactoring things to fix the problems you mention, and then come back to this patch? I'm worried we'll end up with a driver model conversion that never gets properly finished if we do things in the wrong order. Regards, Simon ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] fsl: esdhc: support driver model 2016-03-12 0:33 ` Simon Glass @ 2016-03-12 4:21 ` Peng Fan 2016-03-13 2:51 ` Simon Glass 0 siblings, 1 reply; 6+ messages in thread From: Peng Fan @ 2016-03-12 4:21 UTC (permalink / raw) To: u-boot Hi Simon, On Fri, Mar 11, 2016 at 05:33:05PM -0700, Simon Glass wrote: >Hi Peng, > >On 10 March 2016 at 01:57, Peng Fan <van.freenix@gmail.com> wrote: >> Support Driver Model for fsl esdhc driver. >> >> In order to minimize the change, reuse the fsl_esdhc_initialize function. >> This new way is to fill an fsl_esdhc_cfg struture and pass it >> to fsl_esdhc_initialize, just like the code in different board codes. >> >> Introduce a 'struct mmc *mmc' entry in fsl_esdhc_cfg structure, >> otherwise fsl_esdhc_initialize may need to be restructured which will >> cause lots changes for board code. >> >> Since clk driver is not implemented, use mxc_get_clock here to >> fill cfg->sdhc_clk. Anyway we can utilize the pinctrl imx driver >> now, except the SPL part, we can drop the pinmux settings from >> board file for mmc. >> >> There are so many "ifdef" in the file, maybe we can use driver data >> or quirks to cover these. But to minimize changes for this patch, >> these are not included. Later we can try to discard the nasty >> ifdef. >> >> Has been tested on i.MX6UL 9x9 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> >> --- >> drivers/mmc/fsl_esdhc.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++ >> include/fsl_esdhc.h | 1 + >> 2 files changed, 82 insertions(+) > >I'm nervous about this patch. It is calling board code from a driver, >which we should avoid. No. I may need to write more in the commit log. The current way without driver model is board code fill fsl_esdhc_cfg strucure and then call fsl_esdhc_initialize which is in driver/mmc/fsl_esdhc.c. In order to minimize changes, in this patch, the probe function will fill fsl_esdhc_cfg structure and call fsl_esdhc_initialize. Then we can drop the function call to fsl_esdhc_initialize in different board code. I originally want to rewrite fsl_esdhc_cfg structure, may give it a new name such as fsl_esdhc_platdata or priv, but this may incur lots changes in board code, because board code also refers to this structure. So I keep in. >Perhaps it would be better to start by refactoring things to fix the >problems you mention, and then come back to this patch? I'm worried >we'll end up with a driver model conversion that never gets properly >finished if we do things in the wrong order. Please kind take my upper words. I am still thinking add "non-removable", "cd-gpios" related code in this patch, so I'll write a new version patch. Thanks, Peng. > >Regards, >Simon ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] fsl: esdhc: support driver model 2016-03-12 4:21 ` Peng Fan @ 2016-03-13 2:51 ` Simon Glass 2016-03-13 9:22 ` Peng Fan 0 siblings, 1 reply; 6+ messages in thread From: Simon Glass @ 2016-03-13 2:51 UTC (permalink / raw) To: u-boot Hi Peng, On 11 March 2016 at 21:21, Peng Fan <van.freenix@gmail.com> wrote: > Hi Simon, > > On Fri, Mar 11, 2016 at 05:33:05PM -0700, Simon Glass wrote: >>Hi Peng, >> >>On 10 March 2016 at 01:57, Peng Fan <van.freenix@gmail.com> wrote: >>> Support Driver Model for fsl esdhc driver. >>> >>> In order to minimize the change, reuse the fsl_esdhc_initialize function. >>> This new way is to fill an fsl_esdhc_cfg struture and pass it >>> to fsl_esdhc_initialize, just like the code in different board codes. >>> >>> Introduce a 'struct mmc *mmc' entry in fsl_esdhc_cfg structure, >>> otherwise fsl_esdhc_initialize may need to be restructured which will >>> cause lots changes for board code. >>> >>> Since clk driver is not implemented, use mxc_get_clock here to >>> fill cfg->sdhc_clk. Anyway we can utilize the pinctrl imx driver >>> now, except the SPL part, we can drop the pinmux settings from >>> board file for mmc. >>> >>> There are so many "ifdef" in the file, maybe we can use driver data >>> or quirks to cover these. But to minimize changes for this patch, >>> these are not included. Later we can try to discard the nasty >>> ifdef. >>> >>> Has been tested on i.MX6UL 9x9 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> >>> --- >>> drivers/mmc/fsl_esdhc.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++ >>> include/fsl_esdhc.h | 1 + >>> 2 files changed, 82 insertions(+) >> >>I'm nervous about this patch. It is calling board code from a driver, >>which we should avoid. > > No. I may need to write more in the commit log. > The current way without driver model is > board code fill fsl_esdhc_cfg strucure and then call fsl_esdhc_initialize > which is in driver/mmc/fsl_esdhc.c. > > In order to minimize changes, in this patch, the probe function > will fill fsl_esdhc_cfg structure and call fsl_esdhc_initialize. > > Then we can drop the function call to fsl_esdhc_initialize in different board code. > > I originally want to rewrite fsl_esdhc_cfg structure, may give it a new name > such as fsl_esdhc_platdata or priv, but this may incur lots changes in > board code, because board code also refers to this structure. So I keep in. > >>Perhaps it would be better to start by refactoring things to fix the >>problems you mention, and then come back to this patch? I'm worried >>we'll end up with a driver model conversion that never gets properly >>finished if we do things in the wrong order. > > Please kind take my upper words. I am still thinking add "non-removable", > "cd-gpios" related code in this patch, so I'll write a new version patch. When will this work be finished, so that the driver does not call board code? At least, we should have a TODO here Regards, Simon ^ permalink raw reply [flat|nested] 6+ messages in thread
* [U-Boot] [PATCH 2/2] fsl: esdhc: support driver model 2016-03-13 2:51 ` Simon Glass @ 2016-03-13 9:22 ` Peng Fan 0 siblings, 0 replies; 6+ messages in thread From: Peng Fan @ 2016-03-13 9:22 UTC (permalink / raw) To: u-boot Hi Simon, On Sat, Mar 12, 2016 at 07:51:53PM -0700, Simon Glass wrote: >Hi Peng, > >On 11 March 2016 at 21:21, Peng Fan <van.freenix@gmail.com> wrote: >> Hi Simon, >> >> On Fri, Mar 11, 2016 at 05:33:05PM -0700, Simon Glass wrote: >>>Hi Peng, >>> >>>On 10 March 2016 at 01:57, Peng Fan <van.freenix@gmail.com> wrote: >>>> Support Driver Model for fsl esdhc driver. >>>> >>>> In order to minimize the change, reuse the fsl_esdhc_initialize function. >>>> This new way is to fill an fsl_esdhc_cfg struture and pass it >>>> to fsl_esdhc_initialize, just like the code in different board codes. >>>> >>>> Introduce a 'struct mmc *mmc' entry in fsl_esdhc_cfg structure, >>>> otherwise fsl_esdhc_initialize may need to be restructured which will >>>> cause lots changes for board code. >>>> >>>> Since clk driver is not implemented, use mxc_get_clock here to >>>> fill cfg->sdhc_clk. Anyway we can utilize the pinctrl imx driver >>>> now, except the SPL part, we can drop the pinmux settings from >>>> board file for mmc. >>>> >>>> There are so many "ifdef" in the file, maybe we can use driver data >>>> or quirks to cover these. But to minimize changes for this patch, >>>> these are not included. Later we can try to discard the nasty >>>> ifdef. >>>> >>>> Has been tested on i.MX6UL 9x9 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> >>>> --- >>>> drivers/mmc/fsl_esdhc.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++ >>>> include/fsl_esdhc.h | 1 + >>>> 2 files changed, 82 insertions(+) >>> >>>I'm nervous about this patch. It is calling board code from a driver, >>>which we should avoid. >> >> No. I may need to write more in the commit log. >> The current way without driver model is >> board code fill fsl_esdhc_cfg strucure and then call fsl_esdhc_initialize >> which is in driver/mmc/fsl_esdhc.c. >> >> In order to minimize changes, in this patch, the probe function >> will fill fsl_esdhc_cfg structure and call fsl_esdhc_initialize. >> >> Then we can drop the function call to fsl_esdhc_initialize in different board code. >> >> I originally want to rewrite fsl_esdhc_cfg structure, may give it a new name >> such as fsl_esdhc_platdata or priv, but this may incur lots changes in >> board code, because board code also refers to this structure. So I keep in. >> >>>Perhaps it would be better to start by refactoring things to fix the >>>problems you mention, and then come back to this patch? I'm worried >>>we'll end up with a driver model conversion that never gets properly >>>finished if we do things in the wrong order. >> >> Please kind take my upper words. I am still thinking add "non-removable", >> "cd-gpios" related code in this patch, so I'll write a new version patch. > >When will this work be finished, so that the driver does not call board code? Yeah. The driver does not call board code. My commit log may be misleading. I have restructured my patch. After do some basic test, will send out v2 version tommorow or some day:) Thanks, Peng. > >At least, we should have a TODO here > >Regards, >Simon ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-13 9:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-03-10 8:57 [U-Boot] [PATCH 1/2] fsl: esdhc: consolidate fsl_esdhc_cfg structure Peng Fan 2016-03-10 8:57 ` [U-Boot] [PATCH 2/2] fsl: esdhc: support driver model Peng Fan 2016-03-12 0:33 ` Simon Glass 2016-03-12 4:21 ` Peng Fan 2016-03-13 2:51 ` Simon Glass 2016-03-13 9:22 ` Peng Fan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox