| Message ID | 1438103991-19008-1-git-send-email-andre.przywara@arm.com |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Andre, On 28 July 2015 at 11:19, Andre Przywara <andre.przywara@arm.com> wrote: > So far every MMC driver is relying on a board specific init routine, > but as the Rockchip MMC driver is the first MMC device model user, > it requires a generic DM MMC init routine. > > Add that to the MMC init path to enable the Rockchip driver. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > Hi Simon, > > this goes on top of your rockchip-working branch in the u-boot-dm > repo. > Not fully satisfied with this one, but the best I could come up with > without reading the whole of the device model code ;-) > This makes the SD card visible to U-Boot on my RK3288 dev board, I > can see partitions and load files from it. It gives me information > about the eMMC as well, but I haven't tested this so far except for > that mmc info. > > Cheers, > Andre. > > drivers/mmc/mmc.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) Thanks for the patch and sorry for ignoring it so long. I'm finally getting close to getting the rockchip patches applied. See also this one: http://patchwork.ozlabs.org/patch/507840/ > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > index da47037..52e82b5 100644 > --- a/drivers/mmc/mmc.c > +++ b/drivers/mmc/mmc.c > @@ -17,6 +17,7 @@ > #include <linux/list.h> > #include <div64.h> > #include "mmc_private.h" > +#include <dm.h> > > static struct list_head mmc_devices; > static int cur_dev_num = -1; > @@ -1765,7 +1766,13 @@ int mmc_initialize(bd_t *bis) > INIT_LIST_HEAD (&mmc_devices); > cur_dev_num = 0; > > -#ifndef CONFIG_DM_MMC > +#ifdef CONFIG_DM_MMC > + struct udevice *udev; > + > + for (;;cur_dev_num++) > + if (uclass_get_device(UCLASS_MMC, cur_dev_num, &udev)) > + break; Do you have to increase cur_dev_num? I thought it might happen automatically when a device is probed and calls mmc_create(). Do you think it would work if instead we changed get_mmc_num() to do this? With driver model we are trying to make it so that devices a probed when they are used, not before. I.e. mmc_initialize() should ideally be a nop. > +#else > if (board_mmc_init(bis) < 0) > cpu_mmc_init(bis); > #endif > -- > 2.3.5 > Regards, Simon
On Sun, 2015-08-23 at 15:22 -0600, Simon Glass wrote: > Hi Andre, > > On 28 July 2015 at 11:19, Andre Przywara <andre.przywara@arm.com> > wrote: > > So far every MMC driver is relying on a board specific init > > routine, > > but as the Rockchip MMC driver is the first MMC device model user, > > it requires a generic DM MMC init routine. > > > > Add that to the MMC init path to enable the Rockchip driver. > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > > --- > > Hi Simon, > > > > this goes on top of your rockchip-working branch in the u-boot-dm > > repo. > > Not fully satisfied with this one, but the best I could come up > > with > > without reading the whole of the device model code ;-) > > This makes the SD card visible to U-Boot on my RK3288 dev board, I > > can see partitions and load files from it. It gives me information > > about the eMMC as well, but I haven't tested this so far except for > > that mmc info. > > > > Cheers, > > Andre. > > > > drivers/mmc/mmc.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > Thanks for the patch and sorry for ignoring it so long. I'm finally > getting close to getting the rockchip patches applied. > > See also this one: > > http://patchwork.ozlabs.org/patch/507840/ As the author of that one, i missed this one :/ > > > > diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c > > index da47037..52e82b5 100644 > > --- a/drivers/mmc/mmc.c > > +++ b/drivers/mmc/mmc.c > > @@ -17,6 +17,7 @@ > > #include <linux/list.h> > > #include <div64.h> > > #include "mmc_private.h" > > +#include <dm.h> > > > > static struct list_head mmc_devices; > > static int cur_dev_num = -1; > > @@ -1765,7 +1766,13 @@ int mmc_initialize(bd_t *bis) > > INIT_LIST_HEAD (&mmc_devices); > > cur_dev_num = 0; > > > > -#ifndef CONFIG_DM_MMC > > +#ifdef CONFIG_DM_MMC > > + struct udevice *udev; > > + > > + for (;;cur_dev_num++) > > + if (uclass_get_device(UCLASS_MMC, cur_dev_num, > > &udev)) > > + break; > > Do you have to increase cur_dev_num? I thought it might happen > automatically when a device is probed and calls mmc_create(). Yes mmc_create increases it so this way is buggy.. I think my patch is nicer here wrt. using uclass_foreach_device. Also i was under the impression that in principle there could be gaps in the device numbers in DM ? (maybe thats not the case for the MMC ones though) > Do you think it would work if instead we changed get_mmc_num() to do > this? > With driver model we are trying to make it so that devices a probed > when they are used, not before. I.e. mmc_initialize() should ideally > be a nop. The key here is that functions like print_mmc_devices and find_mmc_device simply iterate the mmc_devices list, so i don't think you can key on get_mmc_num at the moment. Ofcourse a nicer long-term solution would be to make those functions be DM aware so that the mmc_devices list can disappear, but either this patch or mine seems like a reasonable first small step :) > > +#else > > if (board_mmc_init(bis) < 0) > > cpu_mmc_init(bis); > > #endif > > -- > > 2.3.5 > > > > Regards, > Simon > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c index da47037..52e82b5 100644 --- a/drivers/mmc/mmc.c +++ b/drivers/mmc/mmc.c @@ -17,6 +17,7 @@ #include <linux/list.h> #include <div64.h> #include "mmc_private.h" +#include <dm.h> static struct list_head mmc_devices; static int cur_dev_num = -1; @@ -1765,7 +1766,13 @@ int mmc_initialize(bd_t *bis) INIT_LIST_HEAD (&mmc_devices); cur_dev_num = 0; -#ifndef CONFIG_DM_MMC +#ifdef CONFIG_DM_MMC + struct udevice *udev; + + for (;;cur_dev_num++) + if (uclass_get_device(UCLASS_MMC, cur_dev_num, &udev)) + break; +#else if (board_mmc_init(bis) < 0) cpu_mmc_init(bis); #endif
So far every MMC driver is relying on a board specific init routine, but as the Rockchip MMC driver is the first MMC device model user, it requires a generic DM MMC init routine. Add that to the MMC init path to enable the Rockchip driver. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- Hi Simon, this goes on top of your rockchip-working branch in the u-boot-dm repo. Not fully satisfied with this one, but the best I could come up with without reading the whole of the device model code ;-) This makes the SD card visible to U-Boot on my RK3288 dev board, I can see partitions and load files from it. It gives me information about the eMMC as well, but I haven't tested this so far except for that mmc info. Cheers, Andre. drivers/mmc/mmc.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)