Re: [PATCH] mtd: sf: Set SF parameters as env variables
On 10/7/21 8:40 AM, Pratyush Yadav wrote: On 23/09/21 10:00PM, Marek Vasut wrote: On 9/23/21 8:53 PM, Pratyush Yadav wrote: > On 14/09/21 05:28AM, Marek Vasut wrote: > > Set the SF page size, erase block size and total size as an environment > > variable after "sf probe". This lets us discern boards with multiple > > distinct SPI flash options and also e.g. set mtdparts accordingly. > > I don't quite follow the rationale for making these environment > variables. Wouldn't you be better off finding this info out via mtd or > sf command? I need to use that info in a script, that's why these env vars. Ok. Honestly, it doesn't feel quite right to me. I haven't played with U-Boot's shell too much but is there no way to assign variables from command outputs? For example, can we do something like `foo=$(cat a.txt)`? If that is possible, maybe add some new subcommands to "sf" that return this information? There is not. IMO the current solution (exporting to environment variables in an ad-hoc manner) is rather ugly, but fixing it is difficult :) --Sean
Re: [PATCH] mtd: sf: Set SF parameters as env variables
Hi Marek, On 28/11/21 10:56PM, Marek Vasut wrote: > On 10/7/21 2:46 PM, Marek Vasut wrote: > > On 10/7/21 2:40 PM, Pratyush Yadav wrote: > > > On 23/09/21 10:00PM, Marek Vasut wrote: > > > > On 9/23/21 8:53 PM, Pratyush Yadav wrote: > > > > > On 14/09/21 05:28AM, Marek Vasut wrote: > > > > > > Set the SF page size, erase block size and total size as > > > > > > an environment > > > > > > variable after "sf probe". This lets us discern boards with multiple > > > > > > distinct SPI flash options and also e.g. set mtdparts accordingly. > > > > > > > > > > I don't quite follow the rationale for making these environment > > > > > variables. Wouldn't you be better off finding this info out via mtd or > > > > > sf command? > > > > > > > > I need to use that info in a script, that's why these env vars. > > > > > > Ok. Honestly, it doesn't feel quite right to me. > > > > > > I haven't played with U-Boot's shell too much but is there no way to > > > assign variables from command outputs? > > > > Not that I know of. > > > > > For example, can we do something > > > like `foo=$(cat a.txt)`? If that is possible, maybe add some new > > > subcommands to "sf" that return this information? > > > > What you would want to have is per-device runtime properties, kind-of > > like reduced sysfs, which would need VFS in U-Boot, and we do not have > > that yet. > > Are there any news on this patch ? All I would say is that I do not particularly like what this patch is doing, but I also can't give you an alternative solution to the problem unfortunately. So I drop my $0.02 here and leave it up to you and Jagan to figure it out. -- Regards, Pratyush Yadav Texas Instruments Inc.
Re: [PATCH] mtd: sf: Set SF parameters as env variables
On 12/2/21 06:57, Jagan Teki wrote:
On Thu, Dec 2, 2021 at 11:19 AM Marek Vasut wrote:
On 12/2/21 06:47, Jagan Teki wrote:
On Tue, Sep 14, 2021 at 8:59 AM Marek Vasut wrote:
Set the SF page size, erase block size and total size as an environment
variable after "sf probe". This lets us discern boards with multiple
distinct SPI flash options and also e.g. set mtdparts accordingly.
Signed-off-by: Marek Vasut
Cc: Jagan Teki
Cc: Vignesh R
---
drivers/mtd/spi/spi-nor-core.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index d5d905fa5a1..448653b9931 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -3829,8 +3830,11 @@ int spi_nor_scan(struct spi_nor *nor)
#ifndef CONFIG_SPL_BUILD
printf("SF: Detected %s with page size ", nor->name);
print_size(nor->page_size, ", erase size ");
+ env_set_hex("sf_pagesize", nor->page_size);
print_size(nor->erase_size, ", total ");
+ env_set_hex("sf_erasesize", nor->erase_size);
print_size(nor->size, "");
+ env_set_hex("sf_size", nor->size);
puts("\n");
I feel this unnecessarily add env variables which are not needed for
those SF enabled boards, isn't it?
So, how do you determine current SPI NOR size from U-Boot shell ?
I think we need to know the size for those who enabled mtd command (or
mtdparts), any way to preserve it via mtd command or set the env via
mtd command side.
I need to know the size of SPI NOR flash to use it in an U-Boot script
which handle different board population options. This has nothing to do
with mtd command.
Re: [PATCH] mtd: sf: Set SF parameters as env variables
On Thu, Dec 2, 2021 at 11:19 AM Marek Vasut wrote:
>
> On 12/2/21 06:47, Jagan Teki wrote:
> > On Tue, Sep 14, 2021 at 8:59 AM Marek Vasut wrote:
> >>
> >> Set the SF page size, erase block size and total size as an environment
> >> variable after "sf probe". This lets us discern boards with multiple
> >> distinct SPI flash options and also e.g. set mtdparts accordingly.
> >>
> >> Signed-off-by: Marek Vasut
> >> Cc: Jagan Teki
> >> Cc: Vignesh R
> >> ---
> >> drivers/mtd/spi/spi-nor-core.c | 4
> >> 1 file changed, 4 insertions(+)
> >>
> >> diff --git a/drivers/mtd/spi/spi-nor-core.c
> >> b/drivers/mtd/spi/spi-nor-core.c
> >> index d5d905fa5a1..448653b9931 100644
> >> --- a/drivers/mtd/spi/spi-nor-core.c
> >> +++ b/drivers/mtd/spi/spi-nor-core.c
> >> @@ -15,6 +15,7 @@
> >> #include
> >> #include
> >> #include
> >> +#include
> >> #include
> >> #include
> >> #include
> >> @@ -3829,8 +3830,11 @@ int spi_nor_scan(struct spi_nor *nor)
> >> #ifndef CONFIG_SPL_BUILD
> >> printf("SF: Detected %s with page size ", nor->name);
> >> print_size(nor->page_size, ", erase size ");
> >> + env_set_hex("sf_pagesize", nor->page_size);
> >> print_size(nor->erase_size, ", total ");
> >> + env_set_hex("sf_erasesize", nor->erase_size);
> >> print_size(nor->size, "");
> >> + env_set_hex("sf_size", nor->size);
> >> puts("\n");
> >
> > I feel this unnecessarily add env variables which are not needed for
> > those SF enabled boards, isn't it?
>
> So, how do you determine current SPI NOR size from U-Boot shell ?
I think we need to know the size for those who enabled mtd command (or
mtdparts), any way to preserve it via mtd command or set the env via
mtd command side.
Jagan.
Re: [PATCH] mtd: sf: Set SF parameters as env variables
On 12/2/21 06:47, Jagan Teki wrote:
On Tue, Sep 14, 2021 at 8:59 AM Marek Vasut wrote:
Set the SF page size, erase block size and total size as an environment
variable after "sf probe". This lets us discern boards with multiple
distinct SPI flash options and also e.g. set mtdparts accordingly.
Signed-off-by: Marek Vasut
Cc: Jagan Teki
Cc: Vignesh R
---
drivers/mtd/spi/spi-nor-core.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index d5d905fa5a1..448653b9931 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -3829,8 +3830,11 @@ int spi_nor_scan(struct spi_nor *nor)
#ifndef CONFIG_SPL_BUILD
printf("SF: Detected %s with page size ", nor->name);
print_size(nor->page_size, ", erase size ");
+ env_set_hex("sf_pagesize", nor->page_size);
print_size(nor->erase_size, ", total ");
+ env_set_hex("sf_erasesize", nor->erase_size);
print_size(nor->size, "");
+ env_set_hex("sf_size", nor->size);
puts("\n");
I feel this unnecessarily add env variables which are not needed for
those SF enabled boards, isn't it?
So, how do you determine current SPI NOR size from U-Boot shell ?
Re: [PATCH] mtd: sf: Set SF parameters as env variables
On Tue, Sep 14, 2021 at 8:59 AM Marek Vasut wrote:
>
> Set the SF page size, erase block size and total size as an environment
> variable after "sf probe". This lets us discern boards with multiple
> distinct SPI flash options and also e.g. set mtdparts accordingly.
>
> Signed-off-by: Marek Vasut
> Cc: Jagan Teki
> Cc: Vignesh R
> ---
> drivers/mtd/spi/spi-nor-core.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index d5d905fa5a1..448653b9931 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -3829,8 +3830,11 @@ int spi_nor_scan(struct spi_nor *nor)
> #ifndef CONFIG_SPL_BUILD
> printf("SF: Detected %s with page size ", nor->name);
> print_size(nor->page_size, ", erase size ");
> + env_set_hex("sf_pagesize", nor->page_size);
> print_size(nor->erase_size, ", total ");
> + env_set_hex("sf_erasesize", nor->erase_size);
> print_size(nor->size, "");
> + env_set_hex("sf_size", nor->size);
> puts("\n");
I feel this unnecessarily add env variables which are not needed for
those SF enabled boards, isn't it?
Jagan.
Re: [PATCH] mtd: sf: Set SF parameters as env variables
On 10/7/21 2:46 PM, Marek Vasut wrote: On 10/7/21 2:40 PM, Pratyush Yadav wrote: On 23/09/21 10:00PM, Marek Vasut wrote: On 9/23/21 8:53 PM, Pratyush Yadav wrote: On 14/09/21 05:28AM, Marek Vasut wrote: Set the SF page size, erase block size and total size as an environment variable after "sf probe". This lets us discern boards with multiple distinct SPI flash options and also e.g. set mtdparts accordingly. I don't quite follow the rationale for making these environment variables. Wouldn't you be better off finding this info out via mtd or sf command? I need to use that info in a script, that's why these env vars. Ok. Honestly, it doesn't feel quite right to me. I haven't played with U-Boot's shell too much but is there no way to assign variables from command outputs? Not that I know of. For example, can we do something like `foo=$(cat a.txt)`? If that is possible, maybe add some new subcommands to "sf" that return this information? What you would want to have is per-device runtime properties, kind-of like reduced sysfs, which would need VFS in U-Boot, and we do not have that yet. Are there any news on this patch ?
Re: [PATCH] mtd: sf: Set SF parameters as env variables
On 10/7/21 2:40 PM, Pratyush Yadav wrote: On 23/09/21 10:00PM, Marek Vasut wrote: On 9/23/21 8:53 PM, Pratyush Yadav wrote: On 14/09/21 05:28AM, Marek Vasut wrote: Set the SF page size, erase block size and total size as an environment variable after "sf probe". This lets us discern boards with multiple distinct SPI flash options and also e.g. set mtdparts accordingly. I don't quite follow the rationale for making these environment variables. Wouldn't you be better off finding this info out via mtd or sf command? I need to use that info in a script, that's why these env vars. Ok. Honestly, it doesn't feel quite right to me. I haven't played with U-Boot's shell too much but is there no way to assign variables from command outputs? Not that I know of. For example, can we do something like `foo=$(cat a.txt)`? If that is possible, maybe add some new subcommands to "sf" that return this information? What you would want to have is per-device runtime properties, kind-of like reduced sysfs, which would need VFS in U-Boot, and we do not have that yet.
Re: [PATCH] mtd: sf: Set SF parameters as env variables
On 23/09/21 10:00PM, Marek Vasut wrote: > On 9/23/21 8:53 PM, Pratyush Yadav wrote: > > On 14/09/21 05:28AM, Marek Vasut wrote: > > > Set the SF page size, erase block size and total size as an environment > > > variable after "sf probe". This lets us discern boards with multiple > > > distinct SPI flash options and also e.g. set mtdparts accordingly. > > > > I don't quite follow the rationale for making these environment > > variables. Wouldn't you be better off finding this info out via mtd or > > sf command? > > I need to use that info in a script, that's why these env vars. Ok. Honestly, it doesn't feel quite right to me. I haven't played with U-Boot's shell too much but is there no way to assign variables from command outputs? For example, can we do something like `foo=$(cat a.txt)`? If that is possible, maybe add some new subcommands to "sf" that return this information? -- Regards, Pratyush Yadav Texas Instruments Inc.
Re: [PATCH] mtd: sf: Set SF parameters as env variables
On 9/23/21 8:53 PM, Pratyush Yadav wrote: On 14/09/21 05:28AM, Marek Vasut wrote: Set the SF page size, erase block size and total size as an environment variable after "sf probe". This lets us discern boards with multiple distinct SPI flash options and also e.g. set mtdparts accordingly. I don't quite follow the rationale for making these environment variables. Wouldn't you be better off finding this info out via mtd or sf command? I need to use that info in a script, that's why these env vars.
Re: [PATCH] mtd: sf: Set SF parameters as env variables
On 14/09/21 05:28AM, Marek Vasut wrote:
> Set the SF page size, erase block size and total size as an environment
> variable after "sf probe". This lets us discern boards with multiple
> distinct SPI flash options and also e.g. set mtdparts accordingly.
I don't quite follow the rationale for making these environment
variables. Wouldn't you be better off finding this info out via mtd or
sf command?
>
> Signed-off-by: Marek Vasut
> Cc: Jagan Teki
> Cc: Vignesh R
> ---
> drivers/mtd/spi/spi-nor-core.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index d5d905fa5a1..448653b9931 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -3829,8 +3830,11 @@ int spi_nor_scan(struct spi_nor *nor)
> #ifndef CONFIG_SPL_BUILD
> printf("SF: Detected %s with page size ", nor->name);
> print_size(nor->page_size, ", erase size ");
> + env_set_hex("sf_pagesize", nor->page_size);
> print_size(nor->erase_size, ", total ");
> + env_set_hex("sf_erasesize", nor->erase_size);
> print_size(nor->size, "");
> + env_set_hex("sf_size", nor->size);
> puts("\n");
> #endif
>
> --
> 2.33.0
>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
[PATCH] mtd: sf: Set SF parameters as env variables
Set the SF page size, erase block size and total size as an environment
variable after "sf probe". This lets us discern boards with multiple
distinct SPI flash options and also e.g. set mtdparts accordingly.
Signed-off-by: Marek Vasut
Cc: Jagan Teki
Cc: Vignesh R
---
drivers/mtd/spi/spi-nor-core.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index d5d905fa5a1..448653b9931 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -3829,8 +3830,11 @@ int spi_nor_scan(struct spi_nor *nor)
#ifndef CONFIG_SPL_BUILD
printf("SF: Detected %s with page size ", nor->name);
print_size(nor->page_size, ", erase size ");
+ env_set_hex("sf_pagesize", nor->page_size);
print_size(nor->erase_size, ", total ");
+ env_set_hex("sf_erasesize", nor->erase_size);
print_size(nor->size, "");
+ env_set_hex("sf_size", nor->size);
puts("\n");
#endif
--
2.33.0
