| Message ID | 1376995992-20870-3-git-send-email-p.marczak@samsung.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Minkyu Kang |
| Headers | show |
Hello Marek, > This change allows using every mmc device instance with ums, > like eMMC or SD cards. > > Example: ums <device_number> for mmc devices. Could you review this patch, please? > > Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > CC: Marek Vasut <marek.vasut@gmail.com> > --- > common/cmd_usb_mass_storage.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c > index 33a4715..4181d3a 100644 > --- a/common/cmd_usb_mass_storage.c > +++ b/common/cmd_usb_mass_storage.c > @@ -14,6 +14,7 @@ > int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag, > int argc, char * const argv[]) > { > + struct mmc *mmc = NULL; > char *ep; > unsigned int dev_num = 0, offset = 0, part_size = 0; > int rc; > @@ -28,8 +29,10 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag, > > dev_num = (int)simple_strtoul(argv[1], &ep, 16); > > - if (dev_num) { > - puts("\nSet eMMC device to 0! - e.g. ums 0\n"); > + mmc = find_mmc_device(dev_num); > + > + if (!mmc) { > + printf("\neMMC device: %d not found! Try ums 0.\n", dev_num); > goto fail; > } > >
Dear Przemyslaw Marczak, > This change allows using every mmc device instance with ums, > like eMMC or SD cards. > > Example: ums <device_number> for mmc devices. > > Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > CC: Marek Vasut <marek.vasut@gmail.com> > --- > common/cmd_usb_mass_storage.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c > index 33a4715..4181d3a 100644 > --- a/common/cmd_usb_mass_storage.c > +++ b/common/cmd_usb_mass_storage.c > @@ -14,6 +14,7 @@ > int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag, > int argc, char * const argv[]) > { > + struct mmc *mmc = NULL; > char *ep; > unsigned int dev_num = 0, offset = 0, part_size = 0; > int rc; > @@ -28,8 +29,10 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag, > > dev_num = (int)simple_strtoul(argv[1], &ep, 16); > > - if (dev_num) { > - puts("\nSet eMMC device to 0! - e.g. ums 0\n"); > + mmc = find_mmc_device(dev_num); > + > + if (!mmc) { > + printf("\neMMC device: %d not found! Try ums 0.\n", dev_num); > goto fail; > } You still use dev_num further in the code, why not convert the whole thing to use *mmc ? Best regards, Marek Vasut
On 08/28/2013 10:44 AM, Marek Vasut wrote: > You still use dev_num further in the code, why not convert the whole thing to > use *mmc ? > > Best regards, > Marek Vasut. > Hello, I don't know exactly what do you mean. I suppose that putting "mmc device number" as a parameter is easy to maintain at command code and also intuitive to use at prompt. Do you prefer set mmc dev number by command "mmc dev <num>" ? Should we depends one command on another? If yes, then what about e.g. fat or ext command set? Thank you.
Dear Przemyslaw Marczak, > On 08/28/2013 10:44 AM, Marek Vasut wrote: > > You still use dev_num further in the code, why not convert the whole > > thing to use *mmc ? > > > > Best regards, > > Marek Vasut. > > Hello, > I don't know exactly what do you mean. > I suppose that putting "mmc device number" as a parameter is easy to > maintain at command code and also intuitive to use at prompt. > Do you prefer set mmc dev number by command "mmc dev <num>" ? > Should we depends one command on another? If yes, then what about e.g. > fat or ext command set? What I mean is ... for example, you pass dev_num to board_ums_init() . See the trats board, find_mmc_device() is called again in there. Why not just pass the *mmc directly? Best regards, Marek Vasut
On 08/28/2013 01:03 PM, Marek Vasut wrote: > Dear Przemyslaw Marczak, > >> On 08/28/2013 10:44 AM, Marek Vasut wrote: >>> You still use dev_num further in the code, why not convert the whole >>> thing to use *mmc ? >>> >>> Best regards, >>> Marek Vasut. >> Hello, >> I don't know exactly what do you mean. >> I suppose that putting "mmc device number" as a parameter is easy to >> maintain at command code and also intuitive to use at prompt. >> Do you prefer set mmc dev number by command "mmc dev <num>" ? >> Should we depends one command on another? If yes, then what about e.g. >> fat or ext command set? > What I mean is ... for example, you pass dev_num to board_ums_init() . See the > trats board, find_mmc_device() is called again in there. Why not just pass the > *mmc directly? > > Best regards, > Marek Vasut. > Oh, now I see. You are absolutely right, it was just code duplication. I will change it and resend patch. Thank you,
Dear Przemyslaw Marczak, > On 08/28/2013 01:03 PM, Marek Vasut wrote: > > Dear Przemyslaw Marczak, > > > >> On 08/28/2013 10:44 AM, Marek Vasut wrote: > >>> You still use dev_num further in the code, why not convert the whole > >>> thing to use *mmc ? > >>> > >>> Best regards, > >>> Marek Vasut. > >> > >> Hello, > >> I don't know exactly what do you mean. > >> I suppose that putting "mmc device number" as a parameter is easy to > >> maintain at command code and also intuitive to use at prompt. > >> Do you prefer set mmc dev number by command "mmc dev <num>" ? > >> Should we depends one command on another? If yes, then what about e.g. > >> fat or ext command set? > > > > What I mean is ... for example, you pass dev_num to board_ums_init() . > > See the trats board, find_mmc_device() is called again in there. Why not > > just pass the *mmc directly? > > > > Best regards, > > Marek Vasut. > > Oh, now I see. You are absolutely right, it was just code duplication. > I will change it and resend patch. Try and see if *mmc can not be passed all around instead of the dev_num too. Best regards, Marek Vasut
diff --git a/common/cmd_usb_mass_storage.c b/common/cmd_usb_mass_storage.c index 33a4715..4181d3a 100644 --- a/common/cmd_usb_mass_storage.c +++ b/common/cmd_usb_mass_storage.c @@ -14,6 +14,7 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { + struct mmc *mmc = NULL; char *ep; unsigned int dev_num = 0, offset = 0, part_size = 0; int rc; @@ -28,8 +29,10 @@ int do_usb_mass_storage(cmd_tbl_t *cmdtp, int flag, dev_num = (int)simple_strtoul(argv[1], &ep, 16); - if (dev_num) { - puts("\nSet eMMC device to 0! - e.g. ums 0\n"); + mmc = find_mmc_device(dev_num); + + if (!mmc) { + printf("\neMMC device: %d not found! Try ums 0.\n", dev_num); goto fail; }