* [U-Boot] [PATCH v2] misc: fs_loader: Add support for initializing MMC
@ 2018-12-30 1:36 tien.fong.chee at intel.com
2019-01-03 7:20 ` Chee, Tien Fong
2019-01-09 16:19 ` Simon Glass
0 siblings, 2 replies; 5+ messages in thread
From: tien.fong.chee at intel.com @ 2018-12-30 1:36 UTC (permalink / raw)
To: u-boot
From: Tien Fong Chee <tien.fong.chee@intel.com>
Firmware loader would encounter problem if the MMC is accessed before
initializing it. This patch would adding the support of probing block
device and initializing MMC before the MMC is accessed by firmware loader.
Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---
Changes in v2:
- Initializing MMC through probing the blk device
---
drivers/misc/fs_loader.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fs_loader.c
index 57a14a3..5858073 100644
--- a/drivers/misc/fs_loader.c
+++ b/drivers/misc/fs_loader.c
@@ -12,6 +12,7 @@
#include <linux/string.h>
#include <mapmem.h>
#include <malloc.h>
+#include <mmc.h>
#include <spl.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -252,6 +253,38 @@ static int fs_loader_ofdata_to_platdata(struct udevice *dev)
static int fs_loader_probe(struct udevice *dev)
{
+#if defined(CONFIG_DM_MMC) && defined(CONFIG_BLK)
+ int ret;
+ struct device_platdata *plat = dev->platdata;
+
+ ret = mmc_initialize(NULL);
+ if (ret) {
+ debug("MMC: could not initialize mmc. error: %d\n", ret);
+
+ return ret;
+ }
+
+ if (plat->phandlepart.phandle) {
+ ofnode node = ofnode_get_by_phandle(plat->phandlepart.phandle);
+
+ struct udevice *mmc_dev = NULL;
+
+ ret = device_get_global_by_ofnode(node, &mmc_dev);
+ if (!ret) {
+ struct mmc *mmc = mmc_get_mmc_dev(mmc_dev);
+ struct udevice *dev;
+
+ ret = blk_get_from_parent(mmc->dev, &dev);
+ if (ret) {
+ debug("MMC: No block device: %d\n",
+ ret);
+
+ return ret;
+ }
+ }
+ }
+#endif
+
return 0;
};
--
2.2.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH v2] misc: fs_loader: Add support for initializing MMC
2018-12-30 1:36 [U-Boot] [PATCH v2] misc: fs_loader: Add support for initializing MMC tien.fong.chee at intel.com
@ 2019-01-03 7:20 ` Chee, Tien Fong
2019-01-09 16:19 ` Simon Glass
1 sibling, 0 replies; 5+ messages in thread
From: Chee, Tien Fong @ 2019-01-03 7:20 UTC (permalink / raw)
To: u-boot
On Sun, 2018-12-30 at 09:36 +0800, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
>
> Firmware loader would encounter problem if the MMC is accessed before
> initializing it. This patch would adding the support of probing block
> device and initializing MMC before the MMC is accessed by firmware
> loader.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
>
> Changes in v2:
> - Initializing MMC through probing the blk device
Any comment for this patch?
> ---
> drivers/misc/fs_loader.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fs_loader.c
> index 57a14a3..5858073 100644
> --- a/drivers/misc/fs_loader.c
> +++ b/drivers/misc/fs_loader.c
> @@ -12,6 +12,7 @@
> #include <linux/string.h>
> #include <mapmem.h>
> #include <malloc.h>
> +#include <mmc.h>
> #include <spl.h>
>
> DECLARE_GLOBAL_DATA_PTR;
> @@ -252,6 +253,38 @@ static int fs_loader_ofdata_to_platdata(struct
> udevice *dev)
>
> static int fs_loader_probe(struct udevice *dev)
> {
> +#if defined(CONFIG_DM_MMC) && defined(CONFIG_BLK)
> + int ret;
> + struct device_platdata *plat = dev->platdata;
> +
> + ret = mmc_initialize(NULL);
> + if (ret) {
> + debug("MMC: could not initialize mmc. error: %d\n",
> ret);
> +
> + return ret;
> + }
> +
> + if (plat->phandlepart.phandle) {
> + ofnode node = ofnode_get_by_phandle(plat-
> >phandlepart.phandle);
> +
> + struct udevice *mmc_dev = NULL;
> +
> + ret = device_get_global_by_ofnode(node, &mmc_dev);
> + if (!ret) {
> + struct mmc *mmc = mmc_get_mmc_dev(mmc_dev);
> + struct udevice *dev;
> +
> + ret = blk_get_from_parent(mmc->dev, &dev);
> + if (ret) {
> + debug("MMC: No block device: %d\n",
> + ret);
> +
> + return ret;
> + }
> + }
> + }
> +#endif
> +
> return 0;
> };
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH v2] misc: fs_loader: Add support for initializing MMC
2018-12-30 1:36 [U-Boot] [PATCH v2] misc: fs_loader: Add support for initializing MMC tien.fong.chee at intel.com
2019-01-03 7:20 ` Chee, Tien Fong
@ 2019-01-09 16:19 ` Simon Glass
2019-01-10 3:35 ` Chee, Tien Fong
1 sibling, 1 reply; 5+ messages in thread
From: Simon Glass @ 2019-01-09 16:19 UTC (permalink / raw)
To: u-boot
Hi Tien Fong,
On Sat, 29 Dec 2018 at 18:36, <tien.fong.chee@intel.com> wrote:
>
> From: Tien Fong Chee <tien.fong.chee@intel.com>
>
> Firmware loader would encounter problem if the MMC is accessed before
> initializing it. This patch would adding the support of probing block
> device and initializing MMC before the MMC is accessed by firmware loader.
>
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
>
> Changes in v2:
> - Initializing MMC through probing the blk device
> ---
> drivers/misc/fs_loader.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fs_loader.c
> index 57a14a3..5858073 100644
> --- a/drivers/misc/fs_loader.c
> +++ b/drivers/misc/fs_loader.c
> @@ -12,6 +12,7 @@
> #include <linux/string.h>
> #include <mapmem.h>
> #include <malloc.h>
> +#include <mmc.h>
> #include <spl.h>
>
> DECLARE_GLOBAL_DATA_PTR;
> @@ -252,6 +253,38 @@ static int fs_loader_ofdata_to_platdata(struct udevice *dev)
>
> static int fs_loader_probe(struct udevice *dev)
> {
> +#if defined(CONFIG_DM_MMC) && defined(CONFIG_BLK)
> + int ret;
> + struct device_platdata *plat = dev->platdata;
> +
> + ret = mmc_initialize(NULL);
You should not need this since the MMC device should init itself when probed.
> + if (ret) {
> + debug("MMC: could not initialize mmc. error: %d\n", ret);
> +
> + return ret;
> + }
> +
> + if (plat->phandlepart.phandle) {
> + ofnode node = ofnode_get_by_phandle(plat->phandlepart.phandle);
> +
> + struct udevice *mmc_dev = NULL;
> +
> + ret = device_get_global_by_ofnode(node, &mmc_dev);
You could use this?
uclass_get_device_by_ofnode(UCLASS_MMC, node, &mmc_dev)
But actually I think you should drop any mention of MMC so that your
code can work for any storage uclass.
> + if (!ret) {
> + struct mmc *mmc = mmc_get_mmc_dev(mmc_dev);
> + struct udevice *dev;
> +
> + ret = blk_get_from_parent(mmc->dev, &dev);
You are going from mmc_dev -> mmc -> mmc->dev, but unless I am
mistaken, mmc_dev == mmc->dev. So you should be able to simplify this
to:
ret = blk_get_from_parent(mmc_dev, &dev);
Also you should be able to rename mmc to something more generic, e.g.
to make this work with SATA.
> + if (ret) {
> + debug("MMC: No block device: %d\n",
> + ret);
> +
> + return ret;
> + }
> + }
> + }
> +#endif
> +
> return 0;
> };
>
> --
> 2.2.0
>
Regards,
Simon
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH v2] misc: fs_loader: Add support for initializing MMC
2019-01-09 16:19 ` Simon Glass
@ 2019-01-10 3:35 ` Chee, Tien Fong
2019-01-10 12:57 ` Simon Glass
0 siblings, 1 reply; 5+ messages in thread
From: Chee, Tien Fong @ 2019-01-10 3:35 UTC (permalink / raw)
To: u-boot
On Wed, 2019-01-09 at 09:19 -0700, Simon Glass wrote:
> Hi Tien Fong,
>
> On Sat, 29 Dec 2018 at 18:36, <tien.fong.chee@intel.com> wrote:
> >
> >
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> >
> > Firmware loader would encounter problem if the MMC is accessed
> > before
> > initializing it. This patch would adding the support of probing
> > block
> > device and initializing MMC before the MMC is accessed by firmware
> > loader.
> >
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > ---
> >
> > Changes in v2:
> > - Initializing MMC through probing the blk device
> > ---
> > drivers/misc/fs_loader.c | 33 +++++++++++++++++++++++++++++++++
> > 1 file changed, 33 insertions(+)
> >
> > diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fs_loader.c
> > index 57a14a3..5858073 100644
> > --- a/drivers/misc/fs_loader.c
> > +++ b/drivers/misc/fs_loader.c
> > @@ -12,6 +12,7 @@
> > #include <linux/string.h>
> > #include <mapmem.h>
> > #include <malloc.h>
> > +#include <mmc.h>
> > #include <spl.h>
> >
> > DECLARE_GLOBAL_DATA_PTR;
> > @@ -252,6 +253,38 @@ static int fs_loader_ofdata_to_platdata(struct
> > udevice *dev)
> >
> > static int fs_loader_probe(struct udevice *dev)
> > {
> > +#if defined(CONFIG_DM_MMC) && defined(CONFIG_BLK)
> > + int ret;
> > + struct device_platdata *plat = dev->platdata;
> > +
> > + ret = mmc_initialize(NULL);
> You should not need this since the MMC device should init itself when
> probed.
Noted.
>
> >
> > + if (ret) {
> > + debug("MMC: could not initialize mmc. error: %d\n",
> > ret);
> > +
> > + return ret;
> > + }
> > +
> > + if (plat->phandlepart.phandle) {
> > + ofnode node = ofnode_get_by_phandle(plat-
> > >phandlepart.phandle);
> > +
> > + struct udevice *mmc_dev = NULL;
> > +
> > + ret = device_get_global_by_ofnode(node, &mmc_dev);
> You could use this?
>
> uclass_get_device_by_ofnode(UCLASS_MMC, node, &mmc_dev)
Okay.
>
> But actually I think you should drop any mention of MMC so that your
> code can work for any storage uclass.
I agree with you, we should made the probing generic to any storage
uclass.
But now i'm not familiar with all storage uclass yet, so my strategy is
getting one storage uclass in at a time, and restructure the probing if
there is commonality for the next storage uclass. With that, we can
control the quality and achieving the goal step by step.
What do you think?
>
> >
> > + if (!ret) {
> > + struct mmc *mmc = mmc_get_mmc_dev(mmc_dev);
> > + struct udevice *dev;
> > +
> > + ret = blk_get_from_parent(mmc->dev, &dev);
> You are going from mmc_dev -> mmc -> mmc->dev, but unless I am
> mistaken, mmc_dev == mmc->dev. So you should be able to simplify this
> to:
>
> ret = blk_get_from_parent(mmc_dev, &dev);
Okay,noted.
>
> Also you should be able to rename mmc to something more generic, e.g.
> to make this work with SATA.
Replied in above.
>
> >
> > + if (ret) {
> > + debug("MMC: No block device: %d\n",
> > + ret);
> > +
> > + return ret;
> > + }
> > + }
> > + }
> > +#endif
> > +
> > return 0;
> > };
> >
> > --
> > 2.2.0
> >
> Regards,
> Simon
^ permalink raw reply [flat|nested] 5+ messages in thread
* [U-Boot] [PATCH v2] misc: fs_loader: Add support for initializing MMC
2019-01-10 3:35 ` Chee, Tien Fong
@ 2019-01-10 12:57 ` Simon Glass
0 siblings, 0 replies; 5+ messages in thread
From: Simon Glass @ 2019-01-10 12:57 UTC (permalink / raw)
To: u-boot
Hi Tien Fong,
On Wed, 9 Jan 2019 at 20:35, Chee, Tien Fong <tien.fong.chee@intel.com> wrote:
>
> On Wed, 2019-01-09 at 09:19 -0700, Simon Glass wrote:
> > Hi Tien Fong,
> >
> > On Sat, 29 Dec 2018 at 18:36, <tien.fong.chee@intel.com> wrote:
> > >
> > >
> > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > >
> > > Firmware loader would encounter problem if the MMC is accessed
> > > before
> > > initializing it. This patch would adding the support of probing
> > > block
> > > device and initializing MMC before the MMC is accessed by firmware
> > > loader.
> > >
> > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > > ---
> > >
> > > Changes in v2:
> > > - Initializing MMC through probing the blk device
> > > ---
> > > drivers/misc/fs_loader.c | 33 +++++++++++++++++++++++++++++++++
> > > 1 file changed, 33 insertions(+)
> > >
> > > diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fs_loader.c
> > > index 57a14a3..5858073 100644
> > > --- a/drivers/misc/fs_loader.c
> > > +++ b/drivers/misc/fs_loader.c
> > > @@ -12,6 +12,7 @@
> > > #include <linux/string.h>
> > > #include <mapmem.h>
> > > #include <malloc.h>
> > > +#include <mmc.h>
> > > #include <spl.h>
> > >
> > > DECLARE_GLOBAL_DATA_PTR;
> > > @@ -252,6 +253,38 @@ static int fs_loader_ofdata_to_platdata(struct
> > > udevice *dev)
> > >
> > > static int fs_loader_probe(struct udevice *dev)
> > > {
> > > +#if defined(CONFIG_DM_MMC) && defined(CONFIG_BLK)
> > > + int ret;
> > > + struct device_platdata *plat = dev->platdata;
> > > +
> > > + ret = mmc_initialize(NULL);
> > You should not need this since the MMC device should init itself when
> > probed.
> Noted.
> >
> > >
> > > + if (ret) {
> > > + debug("MMC: could not initialize mmc. error: %d\n",
> > > ret);
> > > +
> > > + return ret;
> > > + }
> > > +
> > > + if (plat->phandlepart.phandle) {
> > > + ofnode node = ofnode_get_by_phandle(plat-
> > > >phandlepart.phandle);
> > > +
> > > + struct udevice *mmc_dev = NULL;
> > > +
> > > + ret = device_get_global_by_ofnode(node, &mmc_dev);
> > You could use this?
> >
> > uclass_get_device_by_ofnode(UCLASS_MMC, node, &mmc_dev)
> Okay.
> >
> > But actually I think you should drop any mention of MMC so that your
> > code can work for any storage uclass.
> I agree with you, we should made the probing generic to any storage
> uclass.
>
> But now i'm not familiar with all storage uclass yet, so my strategy is
> getting one storage uclass in at a time, and restructure the probing if
> there is commonality for the next storage uclass. With that, we can
> control the quality and achieving the goal step by step.
>
> What do you think?
Well I don't really like it, sorry.
You can call blk_get_from_parent() to get a block device from any
storage device. So if you make your code more generic you can test
that it still works for your case (MMC), and then we should be fine.
Does fs_loader have tests?
> >
> > >
> > > + if (!ret) {
> > > + struct mmc *mmc = mmc_get_mmc_dev(mmc_dev);
> > > + struct udevice *dev;
> > > +
> > > + ret = blk_get_from_parent(mmc->dev, &dev);
> > You are going from mmc_dev -> mmc -> mmc->dev, but unless I am
> > mistaken, mmc_dev == mmc->dev. So you should be able to simplify this
> > to:
> >
> > ret = blk_get_from_parent(mmc_dev, &dev);
> Okay,noted.
> >
> > Also you should be able to rename mmc to something more generic, e.g.
> > to make this work with SATA.
> Replied in above.
> >
> > >
> > > + if (ret) {
> > > + debug("MMC: No block device: %d\n",
> > > + ret);
> > > +
> > > + return ret;
> > > + }
> > > + }
> > > + }
> > > +#endif
> > > +
> > > return 0;
> > > };
> > >
> > > --
> > > 2.2.0
> > >
> > Regards,
> > Simon
Regards,
Simon
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-10 12:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-30 1:36 [U-Boot] [PATCH v2] misc: fs_loader: Add support for initializing MMC tien.fong.chee at intel.com
2019-01-03 7:20 ` Chee, Tien Fong
2019-01-09 16:19 ` Simon Glass
2019-01-10 3:35 ` Chee, Tien Fong
2019-01-10 12:57 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.