All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info
@ 2015-04-29 10:40 Haikun Wang
  2015-05-01  1:54 ` Simon Glass
  0 siblings, 1 reply; 13+ messages in thread
From: Haikun Wang @ 2015-04-29 10:40 UTC (permalink / raw)
  To: u-boot

Add command "sf info" to show the information of the current SPI flash device.

Signed-off-by: Haikun Wang <haikun.wang@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".

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(-)

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info
  2015-04-29 10:40 [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info Haikun Wang
@ 2015-05-01  1:54 ` Simon Glass
  2015-05-05 11:37   ` Haikun.Wang at freescale.com
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2015-05-01  1:54 UTC (permalink / raw)
  To: u-boot

Hi,

On 29 April 2015 at 04:40, Haikun Wang <haikun.wang@freescale.com> wrote:
> Add command "sf info" to show the information of the current SPI flash device.
>
> Signed-off-by: Haikun Wang <haikun.wang@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".
>
> 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?

>
> 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
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info
  2015-05-01  1:54 ` Simon Glass
@ 2015-05-05 11:37   ` Haikun.Wang at freescale.com
  2015-05-05 21:00     ` Simon Glass
  0 siblings, 1 reply; 13+ messages in thread
From: Haikun.Wang at freescale.com @ 2015-05-05 11:37 UTC (permalink / raw)
  To: u-boot

On 5/1/2015 9:54 AM, Simon Glass wrote:
> Hi,
>
> On 29 April 2015 at 04:40, Haikun Wang <haikun.wang@freescale.com> wrote:
>> Add command "sf info" to show the information of the current SPI flash device.
>>
>> Signed-off-by: Haikun Wang <haikun.wang@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".
>>
>> 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.

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
>>
>
> Regards,
> Simon
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info
  2015-05-05 11:37   ` Haikun.Wang at freescale.com
@ 2015-05-05 21:00     ` Simon Glass
  2015-05-07 11:44       ` Jagan Teki
  0 siblings, 1 reply; 13+ messages in thread
From: Simon Glass @ 2015-05-05 21:00 UTC (permalink / raw)
  To: u-boot

On 5 May 2015 at 05:37, Haikun.Wang at freescale.com
<Haikun.Wang@freescale.com> wrote:
> On 5/1/2015 9:54 AM, Simon Glass wrote:
>> Hi,
>>
>> On 29 April 2015 at 04:40, Haikun Wang <haikun.wang@freescale.com> wrote:
>>> Add command "sf info" to show the information of the current SPI flash device.
>>>
>>> Signed-off-by: Haikun Wang <haikun.wang@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".
>>>
>>> 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 <sjg@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
>>>
>>
>> Regards,
>> Simon
>>
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info
  2015-05-05 21:00     ` Simon Glass
@ 2015-05-07 11:44       ` Jagan Teki
  2015-05-08  2:44         ` Haikun.Wang at freescale.com
  0 siblings, 1 reply; 13+ messages in thread
From: Jagan Teki @ 2015-05-07 11:44 UTC (permalink / raw)
  To: u-boot

On 6 May 2015 at 02:30, Simon Glass <sjg@chromium.org> wrote:
> On 5 May 2015 at 05:37, Haikun.Wang at freescale.com
> <Haikun.Wang@freescale.com> wrote:
>> On 5/1/2015 9:54 AM, Simon Glass wrote:
>>> Hi,
>>>
>>> On 29 April 2015 at 04:40, Haikun Wang <haikun.wang@freescale.com> wrote:
>>>> Add command "sf info" to show the information of the current SPI flash device.
>>>>
>>>> Signed-off-by: Haikun Wang <haikun.wang@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?

>>>>
>>>> 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 <sjg@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
>>>>
>>>
>>> Regards,
>>> Simon
>>>
>>

thaks!

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info
  2015-05-07 11:44       ` Jagan Teki
@ 2015-05-08  2:44         ` Haikun.Wang at freescale.com
  2015-05-08  5:53           ` Jagan Teki
  0 siblings, 1 reply; 13+ messages in thread
From: Haikun.Wang at freescale.com @ 2015-05-08  2:44 UTC (permalink / raw)
  To: u-boot

On 5/7/2015 7:44 PM, Jagan Teki wrote:
> On 6 May 2015 at 02:30, Simon Glass <sjg@chromium.org> wrote:
>> On 5 May 2015 at 05:37, Haikun.Wang at freescale.com
>> <Haikun.Wang@freescale.com> wrote:
>>> On 5/1/2015 9:54 AM, Simon Glass wrote:
>>>> Hi,
>>>>
>>>> On 29 April 2015 at 04:40, Haikun Wang <haikun.wang@freescale.com> wrote:
>>>>> Add command "sf info" to show the information of the current SPI flash device.
>>>>>
>>>>> Signed-off-by: Haikun Wang <haikun.wang@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.
>
>>>>>
>>>>> 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 <sjg@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
>>>>>
>>>>
>>>> Regards,
>>>> Simon
>>>>
>>>
>
> thaks!
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info
  2015-05-08  2:44         ` Haikun.Wang at freescale.com
@ 2015-05-08  5:53           ` Jagan Teki
  2015-05-08  8:21             ` Haikun.Wang at freescale.com
  0 siblings, 1 reply; 13+ messages in thread
From: Jagan Teki @ 2015-05-08  5:53 UTC (permalink / raw)
  To: u-boot

On 8 May 2015 at 08:14, Haikun.Wang at freescale.com
<Haikun.Wang@freescale.com> wrote:
> On 5/7/2015 7:44 PM, Jagan Teki wrote:
>> On 6 May 2015 at 02:30, Simon Glass <sjg@chromium.org> wrote:
>>> On 5 May 2015 at 05:37, Haikun.Wang at freescale.com
>>> <Haikun.Wang@freescale.com> wrote:
>>>> On 5/1/2015 9:54 AM, Simon Glass wrote:
>>>>> Hi,
>>>>>
>>>>> On 29 April 2015 at 04:40, Haikun Wang <haikun.wang@freescale.com> wrote:
>>>>>> Add command "sf info" to show the information of the current SPI flash device.
>>>>>>
>>>>>> Signed-off-by: Haikun Wang <haikun.wang@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.

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).

>>
>>>>>>
>>>>>> 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 <sjg@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
>>>>>>
>>>>>
>>>>> Regards,
>>>>> Simon
>>>>>
>>>>
>>
>> thaks!

thanks!
-- 
Jagan Teki,
Openedev.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info
  2015-05-08  5:53           ` Jagan Teki
@ 2015-05-08  8:21             ` Haikun.Wang at freescale.com
  2015-05-10 13:04               ` Jagan Teki
  0 siblings, 1 reply; 13+ messages in thread
From: Haikun.Wang at freescale.com @ 2015-05-08  8:21 UTC (permalink / raw)
  To: u-boot

On 5/8/2015 1:53 PM, Jagan Teki wrote:
> On 8 May 2015 at 08:14, Haikun.Wang at freescale.com
> <Haikun.Wang@freescale.com> wrote:
>> On 5/7/2015 7:44 PM, Jagan Teki wrote:
>>> On 6 May 2015 at 02:30, Simon Glass <sjg@chromium.org> wrote:
>>>> On 5 May 2015 at 05:37, Haikun.Wang at freescale.com
>>>> <Haikun.Wang@freescale.com> wrote:
>>>>> On 5/1/2015 9:54 AM, Simon Glass wrote:
>>>>>> Hi,
>>>>>>
>>>>>> On 29 April 2015 at 04:40, Haikun Wang <haikun.wang@freescale.com> wrote:
>>>>>>> Add command "sf info" to show the information of the current SPI flash device.
>>>>>>>
>>>>>>> Signed-off-by: Haikun Wang <haikun.wang@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.

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 <sjg@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
>>>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Simon
>>>>>>
>>>>>
>>>
>>> thaks!
>
> thanks!
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info
  2015-05-08  8:21             ` Haikun.Wang at freescale.com
@ 2015-05-10 13:04               ` Jagan Teki
  2015-05-11  2:46                 ` Haikun.Wang at freescale.com
  2015-05-11  2:59                 ` Simon Glass
  0 siblings, 2 replies; 13+ messages in thread
From: Jagan Teki @ 2015-05-10 13:04 UTC (permalink / raw)
  To: u-boot

On 8 May 2015 at 13:51, Haikun.Wang at freescale.com
<Haikun.Wang@freescale.com> wrote:
> On 5/8/2015 1:53 PM, Jagan Teki wrote:
>> On 8 May 2015 at 08:14, Haikun.Wang at freescale.com
>> <Haikun.Wang@freescale.com> wrote:
>>> On 5/7/2015 7:44 PM, Jagan Teki wrote:
>>>> On 6 May 2015 at 02:30, Simon Glass <sjg@chromium.org> wrote:
>>>>> On 5 May 2015 at 05:37, Haikun.Wang at freescale.com
>>>>> <Haikun.Wang@freescale.com> wrote:
>>>>>> On 5/1/2015 9:54 AM, Simon Glass wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 29 April 2015 at 04:40, Haikun Wang <haikun.wang@freescale.com> wrote:
>>>>>>>> Add command "sf info" to show the information of the current SPI flash device.
>>>>>>>>
>>>>>>>> Signed-off-by: Haikun Wang <haikun.wang@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.

>>>>
>>>>>>>>
>>>>>>>> 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 <sjg@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!
-- 
Jagan Teki,
Openedev.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info
  2015-05-10 13:04               ` Jagan Teki
@ 2015-05-11  2:46                 ` Haikun.Wang at freescale.com
  2015-05-11  2:59                 ` Simon Glass
  1 sibling, 0 replies; 13+ messages in thread
From: Haikun.Wang at freescale.com @ 2015-05-11  2:46 UTC (permalink / raw)
  To: u-boot

On 5/10/2015 9:04 PM, Jagan Teki wrote:
> On 8 May 2015 at 13:51, Haikun.Wang at freescale.com
> <Haikun.Wang@freescale.com> wrote:
>> On 5/8/2015 1:53 PM, Jagan Teki wrote:
>>> On 8 May 2015 at 08:14, Haikun.Wang at freescale.com
>>> <Haikun.Wang@freescale.com> wrote:
>>>> On 5/7/2015 7:44 PM, Jagan Teki wrote:
>>>>> On 6 May 2015 at 02:30, Simon Glass <sjg@chromium.org> wrote:
>>>>>> On 5 May 2015 at 05:37, Haikun.Wang at freescale.com
>>>>>> <Haikun.Wang@freescale.com> wrote:
>>>>>>> On 5/1/2015 9:54 AM, Simon Glass wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 29 April 2015 at 04:40, Haikun Wang <haikun.wang@freescale.com> wrote:
>>>>>>>>> Add command "sf info" to show the information of the current SPI flash device.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Haikun Wang <haikun.wang@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 <sjg@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!
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info
  2015-05-10 13:04               ` Jagan Teki
  2015-05-11  2:46                 ` Haikun.Wang at freescale.com
@ 2015-05-11  2:59                 ` Simon Glass
  2015-07-20  5:55                   ` Wang Haikun
  1 sibling, 1 reply; 13+ messages in thread
From: Simon Glass @ 2015-05-11  2:59 UTC (permalink / raw)
  To: u-boot

Hi,

On 10 May 2015 at 07:04, Jagan Teki <jteki@openedev.com> wrote:
> On 8 May 2015 at 13:51, Haikun.Wang at freescale.com
> <Haikun.Wang@freescale.com> wrote:
>> On 5/8/2015 1:53 PM, Jagan Teki wrote:
>>> On 8 May 2015 at 08:14, Haikun.Wang at freescale.com
>>> <Haikun.Wang@freescale.com> wrote:
>>>> On 5/7/2015 7:44 PM, Jagan Teki wrote:
>>>>> On 6 May 2015 at 02:30, Simon Glass <sjg@chromium.org> wrote:
>>>>>> On 5 May 2015 at 05:37, Haikun.Wang at freescale.com
>>>>>> <Haikun.Wang@freescale.com> wrote:
>>>>>>> On 5/1/2015 9:54 AM, Simon Glass wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On 29 April 2015 at 04:40, Haikun Wang <haikun.wang@freescale.com> wrote:
>>>>>>>>> Add command "sf info" to show the information of the current SPI flash device.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Haikun Wang <haikun.wang@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.

We have an 'info' command for usb, mmc, scsi, etc. and they don't have
side-effects like re-probing the device. I think it makes sense to
support something like this for sf, at least for driver model.

Also, with driver model it would be good if the sf could automatically
probe when used, rather than needing to probe it explicitly. We could
also support more than one active flash, and a command to switch
between them (like mmc dev and the like). Even better if we could
specific the device in the 'sf read/write/erase' commands.

[snip]

Regards,
Simon

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info
  2015-05-11  2:59                 ` Simon Glass
@ 2015-07-20  5:55                   ` Wang Haikun
  2015-08-07  8:17                     ` Jagan Teki
  0 siblings, 1 reply; 13+ messages in thread
From: Wang Haikun @ 2015-07-20  5:55 UTC (permalink / raw)
  To: u-boot

On 5/11/2015 10:59 AM, Simon Glass wrote:
> Hi,
>
> On 10 May 2015 at 07:04, Jagan Teki <jteki@openedev.com> wrote:
>> On 8 May 2015 at 13:51, Haikun.Wang at freescale.com
>> <Haikun.Wang@freescale.com> wrote:
>>> On 5/8/2015 1:53 PM, Jagan Teki wrote:
>>>> On 8 May 2015 at 08:14, Haikun.Wang at freescale.com
>>>> <Haikun.Wang@freescale.com> wrote:
>>>>> On 5/7/2015 7:44 PM, Jagan Teki wrote:
>>>>>> On 6 May 2015 at 02:30, Simon Glass <sjg@chromium.org> wrote:
>>>>>>> On 5 May 2015 at 05:37, Haikun.Wang at freescale.com
>>>>>>> <Haikun.Wang@freescale.com> wrote:
>>>>>>>> On 5/1/2015 9:54 AM, Simon Glass wrote:
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> On 29 April 2015 at 04:40, Haikun Wang <haikun.wang@freescale.com> wrote:
>>>>>>>>>> Add command "sf info" to show the information of the current SPI flash device.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Haikun Wang <haikun.wang@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.
>
> We have an 'info' command for usb, mmc, scsi, etc. and they don't have
> side-effects like re-probing the device. I think it makes sense to
> support something like this for sf, at least for driver model.
>
> Also, with driver model it would be good if the sf could automatically
> probe when used, rather than needing to probe it explicitly. We could
> also support more than one active flash, and a command to switch
> between them (like mmc dev and the like). Even better if we could
> specific the device in the 'sf read/write/erase' commands.
>
> [snip]
>
> Regards,
> Simon
>
Update my email address.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info
  2015-07-20  5:55                   ` Wang Haikun
@ 2015-08-07  8:17                     ` Jagan Teki
  0 siblings, 0 replies; 13+ messages in thread
From: Jagan Teki @ 2015-08-07  8:17 UTC (permalink / raw)
  To: u-boot

On 20 July 2015 at 11:25, Wang Haikun <Haikun.Wang@freescale.com> wrote:
> On 5/11/2015 10:59 AM, Simon Glass wrote:
>> Hi,
>>
>> On 10 May 2015 at 07:04, Jagan Teki <jteki@openedev.com> wrote:
>>> On 8 May 2015 at 13:51, Haikun.Wang at freescale.com
>>> <Haikun.Wang@freescale.com> wrote:
>>>> On 5/8/2015 1:53 PM, Jagan Teki wrote:
>>>>> On 8 May 2015 at 08:14, Haikun.Wang at freescale.com
>>>>> <Haikun.Wang@freescale.com> wrote:
>>>>>> On 5/7/2015 7:44 PM, Jagan Teki wrote:
>>>>>>> On 6 May 2015 at 02:30, Simon Glass <sjg@chromium.org> wrote:
>>>>>>>> On 5 May 2015 at 05:37, Haikun.Wang at freescale.com
>>>>>>>> <Haikun.Wang@freescale.com> wrote:
>>>>>>>>> On 5/1/2015 9:54 AM, Simon Glass wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> On 29 April 2015 at 04:40, Haikun Wang <haikun.wang@freescale.com> wrote:
>>>>>>>>>>> Add command "sf info" to show the information of the current SPI flash device.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Haikun Wang <haikun.wang@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.
>>
>> We have an 'info' command for usb, mmc, scsi, etc. and they don't have
>> side-effects like re-probing the device. I think it makes sense to
>> support something like this for sf, at least for driver model.
>>
>> Also, with driver model it would be good if the sf could automatically
>> probe when used, rather than needing to probe it explicitly. We could
>> also support more than one active flash, and a command to switch
>> between them (like mmc dev and the like). Even better if we could
>> specific the device in the 'sf read/write/erase' commands.
>>
>> [snip]
>>
>> Regards,
>> Simon
>>
> Update my email address.

Pls- send the next version by add some text on commit message.

thanks!
-- 
Jagan | openedev.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-08-07  8:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-29 10:40 [U-Boot] [PATCH v1 1/2] cmd_sf: Add command "sf info" to show current device info Haikun Wang
2015-05-01  1:54 ` Simon Glass
2015-05-05 11:37   ` Haikun.Wang at freescale.com
2015-05-05 21:00     ` Simon Glass
2015-05-07 11:44       ` Jagan Teki
2015-05-08  2:44         ` Haikun.Wang at freescale.com
2015-05-08  5:53           ` Jagan Teki
2015-05-08  8:21             ` Haikun.Wang at freescale.com
2015-05-10 13:04               ` Jagan Teki
2015-05-11  2:46                 ` Haikun.Wang at freescale.com
2015-05-11  2:59                 ` Simon Glass
2015-07-20  5:55                   ` Wang Haikun
2015-08-07  8:17                     ` Jagan Teki

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.