On 5/10/2015 9:04 PM, Jagan Teki wrote: > On 8 May 2015 at 13:51, haikun.w...@freescale.com > <haikun.w...@freescale.com> wrote: >> On 5/8/2015 1:53 PM, Jagan Teki wrote: >>> On 8 May 2015 at 08:14, haikun.w...@freescale.com >>> <haikun.w...@freescale.com> wrote: >>>> On 5/7/2015 7:44 PM, Jagan Teki wrote: >>>>> On 6 May 2015 at 02:30, Simon Glass <s...@chromium.org> wrote: >>>>>> On 5 May 2015 at 05:37, haikun.w...@freescale.com >>>>>> <haikun.w...@freescale.com> wrote: >>>>>>> On 5/1/2015 9:54 AM, Simon Glass wrote: >>>>>>>> Hi, >>>>>>>> >>>>>>>> On 29 April 2015 at 04:40, Haikun Wang <haikun.w...@freescale.com> >>>>>>>> wrote: >>>>>>>>> Add command "sf info" to show the information of the current SPI >>>>>>>>> flash device. >>>>>>>>> >>>>>>>>> Signed-off-by: Haikun Wang <haikun.w...@freescale.com> >>>>>>>>> --- >>>>>>>>> In current sf driver, we show the debug information during the flash >>>>>>>>> probe >>>>>>>>> period. >>>>>>>>> >>>>>>>>> In case of without DM SPI, we need to run command "sf probe" to get >>>>>>>>> the debug >>>>>>>>> information of the current SPI flash device. "sf probe" will >>>>>>>>> re-identify the >>>>>>>>> device every time and it reduce the efficiency. We can get the debug >>>>>>>>> information >>>>>>>>> without any re-identify process using "sf info". >>>>> >>>>> So for non-dm case, sf probe and sf info does same? >>>> For non-dm case, sf info only show the information store in the current >>>> struct spi_flash, sf_probe will re-probe the SPI flash and create a new >>>> struct spi-flash. >>> >>> How could it be, in non-dm case sf probe will detect the flash and >>> print the info >>> from drivers/mtd/spi/sf_probe.c So sf probe every-time will >>> re-identify the device >>> and print the info. >> I approve of what you said. >> We don't have divergence about the difference between "sf probe" and "sf >> info". >> I just want to emphasize that "sf probe" re-identify the device, create >> a new "spi_flash" and print the info, "sf info" only print the current info. >>> >>> BTW: regarding this patch, unlike any command in u-boot (for my >>> understanding) >>> sf probe has a run-time capable detection option based on bus:cs hz mode >>> from user. So it better to skip the re-identify the same fitlash if >>> the flash is identified before >>> in sf probe logic and try to print the info in it instead of going >>> another stale command just >>> by doing print using earlier commands settings (sf probe). >>> >> I think we get to a common view that it is unnecessary to re-identify >> the device if it has been identified. >> Divergence is that you think this should be completed through updating >> the sf probe logic and I think we should add the new command "sf info". >> "sf info" resolves the problem in a more simpler way. >> User will be more clearly about the functions of the sf commands if we >> add a new command "sf info". >> From the literal meaning of the command "sf probe" should identify the >> device and the command "sf info" show the information of current device. > > Yes, I agree that 'sf info' simply show the current device info in a > simpler way. > But, being with my previous statement it simply print the info which > is derived from > 'sf probe' So why should we go and do the same thing in 'sf probe' > with increasing > one more command which does part of same as before. > > Yes, 'sf probe' is doing unnecessary re-identify the device so may be we can > fix that, ie going to be a real fix other than adding new command. > > Please think in that direction, adding new command in u-boot is really a big > step to think in whole u-boot development point of view. Fine, I will only enable this command in DM model.
Best regards, Wang Haikun > >>>>> >>>>>>>>> >>>>>>>>> In case of using DM SPI, if we disable CONFIG_DM_DEVICE_REMOVE "sf >>>>>>>>> probe" will >>>>>>>>> only call the flash driver's probe function the first time you run it >>>>>>>>> and no >>>>>>>>> information will show after the first. It is recommended that only >>>>>>>>> call the >>>>>>>>> flash driver's probe function once during u-boot period. You can get >>>>>>>>> the debug >>>>>>>>> information using "sf info" in this case. >>>>>>>>> >>>>>>>>> Changes in v1: None. >>>>>>>>> >>>>>>>>> common/cmd_sf.c | 43 ++++++++++++++++++++++++++++++++++++++++++- >>>>>>>>> 1 file changed, 42 insertions(+), 1 deletion(-) >>>>>>>> >>>>>>>> I wonder if you should enable this command only when driver model is >>>>>>>> used? >>>>>>> You mean I should only enable it when driver model is used? >>>>>>> I think it is also useful in NO-DM model. >>>>>>> We can get the device information without data transfer using this >>>>>>> command. >>>>>> >>>>>> OK. >>>>>> >>>>>> Acked-by: Simon Glass <s...@chromium.org> >>>>>> >>>>>>> >>>>>>> Best regards, >>>>>>> Wang Haikun >>>>>>>> >>>>>>>>> >>>>>>>>> diff --git a/common/cmd_sf.c b/common/cmd_sf.c >>>>>>>>> index 6aabf39..38841fa 100644 >>>>>>>>> --- a/common/cmd_sf.c >>>>>>>>> +++ b/common/cmd_sf.c >>>>>>>>> @@ -503,6 +503,44 @@ static int do_spi_flash_test(int argc, char * >>>>>>>>> const argv[]) >>>>>>>>> } >>>>>>>>> #endif /* CONFIG_CMD_SF_TEST */ >>>>>>>>> >>>>>>>>> +static int do_spi_flash_info(struct spi_flash *flash, bool >>>>>>>>> dm_column_style) >>>>>>>>> +{ >>>>>>>>> + if (dm_column_style) { >>>>>>>>> + struct udevice *bus; >>>>>>>>> + struct udevice *dev; >>>>>>>>> + struct dm_spi_slave_platdata *plat; >>>>>>>>> + >>>>>>>>> + dev = flash->dev; >>>>>>>>> + bus = dev->parent; >>>>>>>>> + plat = dev_get_parent_platdata(dev); >>>>>>>>> + >>>>>>>>> + printf("Device: %s\n", dev->name); >>>>>>>>> + printf("Chipselect: %d\n", plat->cs); >>>>>>>>> + printf("Bind Driver: %s\n", dev->driver->name); >>>>>>>>> + printf("SPI bus: %s\n", bus->name); >>>>>>>>> + printf("SPI bus number: %d\n", bus->seq); >>>>>>>>> + printf("Flash type: %s\n", flash->name); >>>>>>>>> + printf("Page size: "); >>>>>>>>> + print_size(flash->page_size, "\n"); >>>>>>>>> + printf("Erase size: "); >>>>>>>>> + print_size(flash->erase_size, "\n"); >>>>>>>>> + printf("Total size: "); >>>>>>>>> + print_size(flash->size, "\n"); >>>>>>>>> + if (flash->memory_map) >>>>>>>>> + printf("Mapped at %p\n", flash->memory_map); >>>>>>>>> + } else { >>>>>>>>> + printf("SF: Detected %s with page size ", >>>>>>>>> flash->name); >>>>>>>>> + print_size(flash->page_size, ", erase size "); >>>>>>>>> + print_size(flash->erase_size, ", total "); >>>>>>>>> + print_size(flash->size, ""); >>>>>>>>> + if (flash->memory_map) >>>>>>>>> + printf(", mapped at %p", flash->memory_map); >>>>>>>>> + puts("\n"); >>>>>>>>> + } >>>>>>>>> + >>>>>>>>> + return 0; >>>>>>>>> +} >>>>>>>>> + >>>>>>>>> static int do_spi_flash(cmd_tbl_t *cmdtp, int flag, int argc, >>>>>>>>> char * const argv[]) >>>>>>>>> { >>>>>>>>> @@ -537,6 +575,8 @@ static int do_spi_flash(cmd_tbl_t *cmdtp, int >>>>>>>>> flag, int argc, >>>>>>>>> else if (!strcmp(cmd, "test")) >>>>>>>>> ret = do_spi_flash_test(argc, argv); >>>>>>>>> #endif >>>>>>>>> + else if (!strcmp(cmd, "info")) >>>>>>>>> + ret = do_spi_flash_info(flash, >>>>>>>>> IS_ENABLED(CONFIG_DM_SPI_FLASH)); >>>>>>>>> else >>>>>>>>> ret = -1; >>>>>>>>> >>>>>>>>> @@ -567,6 +607,7 @@ U_BOOT_CMD( >>>>>>>>> "sf erase offset [+]len - erase `len' bytes from >>>>>>>>> `offset'\n" >>>>>>>>> " `+len' round up `len' >>>>>>>>> to block size\n" >>>>>>>>> "sf update addr offset len - erase and write `len' >>>>>>>>> bytes from memory\n" >>>>>>>>> - " at `addr' to flash at >>>>>>>>> `offset'" >>>>>>>>> + " at `addr' to flash at >>>>>>>>> `offset'\n" >>>>>>>>> + "sf info - display info of the current SPI Flash device\n" >>>>>>>>> SF_TEST_HELP >>>>>>>>> ); >>>>>>>>> -- >>>>>>>>> 2.1.0.27.g96db324 > > thanks! > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot