All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] env: Merge Rockchip, Sunxi, Zynq and ZynqMP
@ 2018-07-19  6:45 Michal Simek
  2018-07-19 11:13 ` Maxime Ripard
  2018-08-19 19:14 ` [U-Boot] " Tom Rini
  0 siblings, 2 replies; 10+ messages in thread
From: Michal Simek @ 2018-07-19  6:45 UTC (permalink / raw)
  To: u-boot

There is no reason to have the same Kconfig options for different SoCs
separately. The patch is merging them together.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

Patch is based on
https://lists.denx.de/pipermail/u-boot/2018-July/335126.html

I have ENV_SECT_SIZE just for zynq/zynqmp because rockchip and sunxi
have this in their configs. When they decide to move then can enable
that option for them too.
I expect when more platforms extend this we will have less constrain
Kconfig setup.

---
 env/Kconfig | 66 ++++++++++++++++---------------------------------------------
 1 file changed, 17 insertions(+), 49 deletions(-)

diff --git a/env/Kconfig b/env/Kconfig
index b37dcd78eb75..0ded003d7d41 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -431,23 +431,37 @@ config ENV_EXT4_FILE
 	  It's a string of the EXT4 file name. This file use to store the
 	  environment (explicit path to the file)
 
-if ARCH_SUNXI
+if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP
 
 config ENV_OFFSET
 	hex "Environment Offset"
 	depends on !ENV_IS_IN_UBI
 	depends on !ENV_IS_NOWHERE
+	default 0x3f8000 if ARCH_ROCKCHIP
 	default 0x88000 if ARCH_SUNXI
+	default 0xE0000 if ARCH_ZYNQ
+	default 0x1E00000 if ARCH_ZYNQMP
 	help
 	  Offset from the start of the device (or partition)
 
 config ENV_SIZE
 	hex "Environment Size"
-	depends on !ENV_IS_NOWHERE
-	default 0x20000 if ARCH_SUNXI
+	default 0x8000 if ARCH_ROCKCHIP && !ENV_IS_NOWHERE
+	default 0x20000 if ARCH_SUNXI && !ENV_IS_NOWHERE
+	default 0x20000 if ARCH_ZYNQ
+	default 0x40000 if ENV_IS_IN_SPI_FLASH && ARCH_ZYNQMP
+	default 0x8000 if ARCH_ZYNQMP
 	help
 	  Size of the environment storage area
 
+config ENV_SECT_SIZE
+	hex "Environment Sector-Size"
+	depends on !ENV_IS_NOWHERE && (ARCH_ZYNQ || ARCH_ZYNQMP)
+	default 0x40000 if ARCH_ZYNQMP
+	default 0x20000 if ARCH_ZYNQ
+	help
+	  Size of the sector containing the environment.
+
 config ENV_UBI_PART
 	string "UBI partition name"
 	depends on ENV_IS_IN_UBI
@@ -462,52 +476,6 @@ config ENV_UBI_VOLUME
 
 endif
 
-if ARCH_ROCKCHIP
-
-config ENV_OFFSET
-	hex
-	depends on !ENV_IS_IN_UBI
-	depends on !ENV_IS_NOWHERE
-	default 0x3f8000
-	help
-	  Offset from the start of the device (or partition)
-
-config ENV_SIZE
-	hex
-	default 0x8000
-	help
-	  Size of the environment storage area
-
-endif
-
-if ARCH_ZYNQMP || ARCH_ZYNQ
-
-config ENV_OFFSET
-	hex "Environment Offset"
-	depends on !ENV_IS_NOWHERE
-	default 0x1E00000 if ARCH_ZYNQMP
-	default 0xE0000 if ARCH_ZYNQ
-	help
-	  Offset from the start of the device (or partition)
-
-config ENV_SIZE
-	hex "Environment Size"
-	default 0x40000 if ENV_IS_IN_SPI_FLASH && ARCH_ZYNQMP
-	default 0x8000 if ARCH_ZYNQMP
-	default 0x20000 if ARCH_ZYNQ
-	help
-	  Size of the environment storage area.
-
-config ENV_SECT_SIZE
-	hex "Environment Sector-Size"
-	depends on !ENV_IS_NOWHERE
-	default 0x40000 if ARCH_ZYNQMP
-	default 0x20000 if ARCH_ZYNQ
-	help
-	  Size of the sector containing the environment.
-
-endif
-
 config USE_DEFAULT_ENV_FILE
 	bool "Create default environment from file"
 	help
-- 
1.9.1

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

* [U-Boot] [PATCH] env: Merge Rockchip, Sunxi, Zynq and ZynqMP
  2018-07-19  6:45 [U-Boot] [PATCH] env: Merge Rockchip, Sunxi, Zynq and ZynqMP Michal Simek
@ 2018-07-19 11:13 ` Maxime Ripard
  2018-07-19 13:45   ` Michal Simek
  2018-08-19 19:14 ` [U-Boot] " Tom Rini
  1 sibling, 1 reply; 10+ messages in thread
From: Maxime Ripard @ 2018-07-19 11:13 UTC (permalink / raw)
  To: u-boot

Hi,

On Thu, Jul 19, 2018 at 08:45:45AM +0200, Michal Simek wrote:
> There is no reason to have the same Kconfig options for different SoCs
> separately. The patch is merging them together.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> 
> Patch is based on
> https://lists.denx.de/pipermail/u-boot/2018-July/335126.html
> 
> I have ENV_SECT_SIZE just for zynq/zynqmp because rockchip and sunxi
> have this in their configs. When they decide to move then can enable
> that option for them too.
> I expect when more platforms extend this we will have less constrain
> Kconfig setup.
> 
> ---
>  env/Kconfig | 66 ++++++++++++++++---------------------------------------------
>  1 file changed, 17 insertions(+), 49 deletions(-)
> 
> diff --git a/env/Kconfig b/env/Kconfig
> index b37dcd78eb75..0ded003d7d41 100644
> --- a/env/Kconfig
> +++ b/env/Kconfig
> @@ -431,23 +431,37 @@ config ENV_EXT4_FILE
>  	  It's a string of the EXT4 file name. This file use to store the
>  	  environment (explicit path to the file)
>  
> -if ARCH_SUNXI
> +if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP

Can we have a depends on instead? That would be more flexible.

>  config ENV_OFFSET
>  	hex "Environment Offset"
>  	depends on !ENV_IS_IN_UBI
>  	depends on !ENV_IS_NOWHERE
> +	default 0x3f8000 if ARCH_ROCKCHIP
>  	default 0x88000 if ARCH_SUNXI
> +	default 0xE0000 if ARCH_ZYNQ
> +	default 0x1E00000 if ARCH_ZYNQMP
>  	help
>  	  Offset from the start of the device (or partition)
>  
>  config ENV_SIZE
>  	hex "Environment Size"
> -	depends on !ENV_IS_NOWHERE
> -	default 0x20000 if ARCH_SUNXI
> +	default 0x8000 if ARCH_ROCKCHIP && !ENV_IS_NOWHERE
> +	default 0x20000 if ARCH_SUNXI && !ENV_IS_NOWHERE

I'm not sure why you removed the depends on !ENV_IS_NOWHERE. Do you
have a case where the environment is not store anywhere but still need
a size?

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180719/d416eb39/attachment.sig>

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

* [U-Boot] [PATCH] env: Merge Rockchip, Sunxi, Zynq and ZynqMP
  2018-07-19 11:13 ` Maxime Ripard
@ 2018-07-19 13:45   ` Michal Simek
  2018-07-19 14:53     ` Tom Rini
  0 siblings, 1 reply; 10+ messages in thread
From: Michal Simek @ 2018-07-19 13:45 UTC (permalink / raw)
  To: u-boot

On 19.7.2018 13:13, Maxime Ripard wrote:
> Hi,
> 
> On Thu, Jul 19, 2018 at 08:45:45AM +0200, Michal Simek wrote:
>> There is no reason to have the same Kconfig options for different SoCs
>> separately. The patch is merging them together.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> Patch is based on
>> https://lists.denx.de/pipermail/u-boot/2018-July/335126.html
>>
>> I have ENV_SECT_SIZE just for zynq/zynqmp because rockchip and sunxi
>> have this in their configs. When they decide to move then can enable
>> that option for them too.
>> I expect when more platforms extend this we will have less constrain
>> Kconfig setup.
>>
>> ---
>>  env/Kconfig | 66 ++++++++++++++++---------------------------------------------
>>  1 file changed, 17 insertions(+), 49 deletions(-)
>>
>> diff --git a/env/Kconfig b/env/Kconfig
>> index b37dcd78eb75..0ded003d7d41 100644
>> --- a/env/Kconfig
>> +++ b/env/Kconfig
>> @@ -431,23 +431,37 @@ config ENV_EXT4_FILE
>>  	  It's a string of the EXT4 file name. This file use to store the
>>  	  environment (explicit path to the file)
>>  
>> -if ARCH_SUNXI
>> +if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP
> 
> Can we have a depends on instead? That would be more flexible.

In what sense? If depends is used below then the same 4 platforms will
be listed on all options below. (I want to also add ZYNQMP_R5 there too)
And changing this in one place seems to me better then on four.


>>  config ENV_OFFSET
>>  	hex "Environment Offset"
>>  	depends on !ENV_IS_IN_UBI
>>  	depends on !ENV_IS_NOWHERE
>> +	default 0x3f8000 if ARCH_ROCKCHIP
>>  	default 0x88000 if ARCH_SUNXI
>> +	default 0xE0000 if ARCH_ZYNQ
>> +	default 0x1E00000 if ARCH_ZYNQMP
>>  	help
>>  	  Offset from the start of the device (or partition)
>>  
>>  config ENV_SIZE
>>  	hex "Environment Size"
>> -	depends on !ENV_IS_NOWHERE
>> -	default 0x20000 if ARCH_SUNXI
>> +	default 0x8000 if ARCH_ROCKCHIP && !ENV_IS_NOWHERE
>> +	default 0x20000 if ARCH_SUNXI && !ENV_IS_NOWHERE
> 
> I'm not sure why you removed the depends on !ENV_IS_NOWHERE. Do you
> have a case where the environment is not store anywhere but still need
> a size?

yes, I had a compilation warning for that case.

in include/environment.h at line 145 it is written this
#define ENV_SIZE (CONFIG_ENV_SIZE - ENV_HEADER_SIZE)

ENV_SIZE is also used in typedef struct environment_s some lines below.
And this structure is used a lot.

How did you find out that this can't be used for ENV_IS_NOWHERE?

Thanks,
Michal

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

* [U-Boot] [PATCH] env: Merge Rockchip, Sunxi, Zynq and ZynqMP
  2018-07-19 13:45   ` Michal Simek
@ 2018-07-19 14:53     ` Tom Rini
  2018-07-19 15:50       ` Maxime Ripard
  0 siblings, 1 reply; 10+ messages in thread
From: Tom Rini @ 2018-07-19 14:53 UTC (permalink / raw)
  To: u-boot

On Thu, Jul 19, 2018 at 03:45:11PM +0200, Michal Simek wrote:
> On 19.7.2018 13:13, Maxime Ripard wrote:
> > Hi,
> > 
> > On Thu, Jul 19, 2018 at 08:45:45AM +0200, Michal Simek wrote:
> >> There is no reason to have the same Kconfig options for different SoCs
> >> separately. The patch is merging them together.
> >>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >> ---
> >>
> >> Patch is based on
> >> https://lists.denx.de/pipermail/u-boot/2018-July/335126.html
> >>
> >> I have ENV_SECT_SIZE just for zynq/zynqmp because rockchip and sunxi
> >> have this in their configs. When they decide to move then can enable
> >> that option for them too.
> >> I expect when more platforms extend this we will have less constrain
> >> Kconfig setup.
> >>
> >> ---
> >>  env/Kconfig | 66 ++++++++++++++++---------------------------------------------
> >>  1 file changed, 17 insertions(+), 49 deletions(-)
> >>
> >> diff --git a/env/Kconfig b/env/Kconfig
> >> index b37dcd78eb75..0ded003d7d41 100644
> >> --- a/env/Kconfig
> >> +++ b/env/Kconfig
> >> @@ -431,23 +431,37 @@ config ENV_EXT4_FILE
> >>  	  It's a string of the EXT4 file name. This file use to store the
> >>  	  environment (explicit path to the file)
> >>  
> >> -if ARCH_SUNXI
> >> +if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP
> > 
> > Can we have a depends on instead? That would be more flexible.
> 
> In what sense? If depends is used below then the same 4 platforms will
> be listed on all options below. (I want to also add ZYNQMP_R5 there too)
> And changing this in one place seems to me better then on four.

For now I like the "if" method for now as we can't (or couldn't a while
ago) globally migrate everyone over.  I think trying to move everyone
over again is something I should give another try.

> >>  config ENV_OFFSET
> >>  	hex "Environment Offset"
> >>  	depends on !ENV_IS_IN_UBI
> >>  	depends on !ENV_IS_NOWHERE
> >> +	default 0x3f8000 if ARCH_ROCKCHIP
> >>  	default 0x88000 if ARCH_SUNXI
> >> +	default 0xE0000 if ARCH_ZYNQ
> >> +	default 0x1E00000 if ARCH_ZYNQMP
> >>  	help
> >>  	  Offset from the start of the device (or partition)
> >>  
> >>  config ENV_SIZE
> >>  	hex "Environment Size"
> >> -	depends on !ENV_IS_NOWHERE
> >> -	default 0x20000 if ARCH_SUNXI
> >> +	default 0x8000 if ARCH_ROCKCHIP && !ENV_IS_NOWHERE
> >> +	default 0x20000 if ARCH_SUNXI && !ENV_IS_NOWHERE
> > 
> > I'm not sure why you removed the depends on !ENV_IS_NOWHERE. Do you
> > have a case where the environment is not store anywhere but still need
> > a size?
> 
> yes, I had a compilation warning for that case.
> 
> in include/environment.h at line 145 it is written this
> #define ENV_SIZE (CONFIG_ENV_SIZE - ENV_HEADER_SIZE)
> 
> ENV_SIZE is also used in typedef struct environment_s some lines below.
> And this structure is used a lot.
> 
> How did you find out that this can't be used for ENV_IS_NOWHERE?

I would have sworn that ENV_SIZE is used for ENV_IS_NOWHERE as that's
how much space we have for environment when it's in memory as well.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180719/28a1d927/attachment.sig>

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

* [U-Boot] [PATCH] env: Merge Rockchip, Sunxi, Zynq and ZynqMP
  2018-07-19 14:53     ` Tom Rini
@ 2018-07-19 15:50       ` Maxime Ripard
  2018-07-20  6:28         ` Michal Simek
  0 siblings, 1 reply; 10+ messages in thread
From: Maxime Ripard @ 2018-07-19 15:50 UTC (permalink / raw)
  To: u-boot

On Thu, Jul 19, 2018 at 10:53:44AM -0400, Tom Rini wrote:
> On Thu, Jul 19, 2018 at 03:45:11PM +0200, Michal Simek wrote:
> > On 19.7.2018 13:13, Maxime Ripard wrote:
> > > Hi,
> > > 
> > > On Thu, Jul 19, 2018 at 08:45:45AM +0200, Michal Simek wrote:
> > >> There is no reason to have the same Kconfig options for different SoCs
> > >> separately. The patch is merging them together.
> > >>
> > >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> > >> ---
> > >>
> > >> Patch is based on
> > >> https://lists.denx.de/pipermail/u-boot/2018-July/335126.html
> > >>
> > >> I have ENV_SECT_SIZE just for zynq/zynqmp because rockchip and sunxi
> > >> have this in their configs. When they decide to move then can enable
> > >> that option for them too.
> > >> I expect when more platforms extend this we will have less constrain
> > >> Kconfig setup.
> > >>
> > >> ---
> > >>  env/Kconfig | 66 ++++++++++++++++---------------------------------------------
> > >>  1 file changed, 17 insertions(+), 49 deletions(-)
> > >>
> > >> diff --git a/env/Kconfig b/env/Kconfig
> > >> index b37dcd78eb75..0ded003d7d41 100644
> > >> --- a/env/Kconfig
> > >> +++ b/env/Kconfig
> > >> @@ -431,23 +431,37 @@ config ENV_EXT4_FILE
> > >>  	  It's a string of the EXT4 file name. This file use to store the
> > >>  	  environment (explicit path to the file)
> > >>  
> > >> -if ARCH_SUNXI
> > >> +if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP
> > > 
> > > Can we have a depends on instead? That would be more flexible.
> > 
> > In what sense? If depends is used below then the same 4 platforms will
> > be listed on all options below. (I want to also add ZYNQMP_R5 there too)
> > And changing this in one place seems to me better then on four.
> 
> For now I like the "if" method for now as we can't (or couldn't a while
> ago) globally migrate everyone over.  I think trying to move everyone
> over again is something I should give another try.

Ack.

> > >>  config ENV_OFFSET
> > >>  	hex "Environment Offset"
> > >>  	depends on !ENV_IS_IN_UBI
> > >>  	depends on !ENV_IS_NOWHERE
> > >> +	default 0x3f8000 if ARCH_ROCKCHIP
> > >>  	default 0x88000 if ARCH_SUNXI
> > >> +	default 0xE0000 if ARCH_ZYNQ
> > >> +	default 0x1E00000 if ARCH_ZYNQMP
> > >>  	help
> > >>  	  Offset from the start of the device (or partition)
> > >>  
> > >>  config ENV_SIZE
> > >>  	hex "Environment Size"
> > >> -	depends on !ENV_IS_NOWHERE
> > >> -	default 0x20000 if ARCH_SUNXI
> > >> +	default 0x8000 if ARCH_ROCKCHIP && !ENV_IS_NOWHERE
> > >> +	default 0x20000 if ARCH_SUNXI && !ENV_IS_NOWHERE
> > > 
> > > I'm not sure why you removed the depends on !ENV_IS_NOWHERE. Do you
> > > have a case where the environment is not store anywhere but still need
> > > a size?
> > 
> > yes, I had a compilation warning for that case.
> > 
> > in include/environment.h at line 145 it is written this
> > #define ENV_SIZE (CONFIG_ENV_SIZE - ENV_HEADER_SIZE)
> > 
> > ENV_SIZE is also used in typedef struct environment_s some lines below.
> > And this structure is used a lot.
> > 
> > How did you find out that this can't be used for ENV_IS_NOWHERE?
> 
> I would have sworn that ENV_SIZE is used for ENV_IS_NOWHERE as that's
> how much space we have for environment when it's in memory as well.

Argh, sorry for that I was abused by sunxi-common still having that:
https://git.denx.de/?p=u-boot.git;a=blob;f=include/configs/sunxi-common.h#l161

While i was convinced that we were relying solely on Kconfig. I'll
send a subsequent patch, that one works for me.

Sorry,
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180719/5127dc2c/attachment.sig>

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

* [U-Boot] [PATCH] env: Merge Rockchip, Sunxi, Zynq and ZynqMP
  2018-07-19 15:50       ` Maxime Ripard
@ 2018-07-20  6:28         ` Michal Simek
  2018-07-20  7:45           ` Maxime Ripard
  0 siblings, 1 reply; 10+ messages in thread
From: Michal Simek @ 2018-07-20  6:28 UTC (permalink / raw)
  To: u-boot

On 19.7.2018 17:50, Maxime Ripard wrote:
> On Thu, Jul 19, 2018 at 10:53:44AM -0400, Tom Rini wrote:
>> On Thu, Jul 19, 2018 at 03:45:11PM +0200, Michal Simek wrote:
>>> On 19.7.2018 13:13, Maxime Ripard wrote:
>>>> Hi,
>>>>
>>>> On Thu, Jul 19, 2018 at 08:45:45AM +0200, Michal Simek wrote:
>>>>> There is no reason to have the same Kconfig options for different SoCs
>>>>> separately. The patch is merging them together.
>>>>>
>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>> ---
>>>>>
>>>>> Patch is based on
>>>>> https://lists.denx.de/pipermail/u-boot/2018-July/335126.html
>>>>>
>>>>> I have ENV_SECT_SIZE just for zynq/zynqmp because rockchip and sunxi
>>>>> have this in their configs. When they decide to move then can enable
>>>>> that option for them too.
>>>>> I expect when more platforms extend this we will have less constrain
>>>>> Kconfig setup.
>>>>>
>>>>> ---
>>>>>  env/Kconfig | 66 ++++++++++++++++---------------------------------------------
>>>>>  1 file changed, 17 insertions(+), 49 deletions(-)
>>>>>
>>>>> diff --git a/env/Kconfig b/env/Kconfig
>>>>> index b37dcd78eb75..0ded003d7d41 100644
>>>>> --- a/env/Kconfig
>>>>> +++ b/env/Kconfig
>>>>> @@ -431,23 +431,37 @@ config ENV_EXT4_FILE
>>>>>  	  It's a string of the EXT4 file name. This file use to store the
>>>>>  	  environment (explicit path to the file)
>>>>>  
>>>>> -if ARCH_SUNXI
>>>>> +if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP
>>>>
>>>> Can we have a depends on instead? That would be more flexible.
>>>
>>> In what sense? If depends is used below then the same 4 platforms will
>>> be listed on all options below. (I want to also add ZYNQMP_R5 there too)
>>> And changing this in one place seems to me better then on four.
>>
>> For now I like the "if" method for now as we can't (or couldn't a while
>> ago) globally migrate everyone over.  I think trying to move everyone
>> over again is something I should give another try.
> 
> Ack.
> 
>>>>>  config ENV_OFFSET
>>>>>  	hex "Environment Offset"
>>>>>  	depends on !ENV_IS_IN_UBI
>>>>>  	depends on !ENV_IS_NOWHERE
>>>>> +	default 0x3f8000 if ARCH_ROCKCHIP
>>>>>  	default 0x88000 if ARCH_SUNXI
>>>>> +	default 0xE0000 if ARCH_ZYNQ
>>>>> +	default 0x1E00000 if ARCH_ZYNQMP
>>>>>  	help
>>>>>  	  Offset from the start of the device (or partition)
>>>>>  
>>>>>  config ENV_SIZE
>>>>>  	hex "Environment Size"
>>>>> -	depends on !ENV_IS_NOWHERE
>>>>> -	default 0x20000 if ARCH_SUNXI
>>>>> +	default 0x8000 if ARCH_ROCKCHIP && !ENV_IS_NOWHERE
>>>>> +	default 0x20000 if ARCH_SUNXI && !ENV_IS_NOWHERE
>>>>
>>>> I'm not sure why you removed the depends on !ENV_IS_NOWHERE. Do you
>>>> have a case where the environment is not store anywhere but still need
>>>> a size?
>>>
>>> yes, I had a compilation warning for that case.
>>>
>>> in include/environment.h at line 145 it is written this
>>> #define ENV_SIZE (CONFIG_ENV_SIZE - ENV_HEADER_SIZE)
>>>
>>> ENV_SIZE is also used in typedef struct environment_s some lines below.
>>> And this structure is used a lot.
>>>
>>> How did you find out that this can't be used for ENV_IS_NOWHERE?
>>
>> I would have sworn that ENV_SIZE is used for ENV_IS_NOWHERE as that's
>> how much space we have for environment when it's in memory as well.
> 
> Argh, sorry for that I was abused by sunxi-common still having that:
> https://git.denx.de/?p=u-boot.git;a=blob;f=include/configs/sunxi-common.h#l161
> 
> While i was convinced that we were relying solely on Kconfig. I'll
> send a subsequent patch, that one works for me.

Ok. Can you please convert this to any official tag which I can include?

Thanks,
Michal

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

* [U-Boot] [PATCH] env: Merge Rockchip, Sunxi, Zynq and ZynqMP
  2018-07-20  6:28         ` Michal Simek
@ 2018-07-20  7:45           ` Maxime Ripard
  0 siblings, 0 replies; 10+ messages in thread
From: Maxime Ripard @ 2018-07-20  7:45 UTC (permalink / raw)
  To: u-boot

On Fri, Jul 20, 2018 at 08:28:07AM +0200, Michal Simek wrote:
> On 19.7.2018 17:50, Maxime Ripard wrote:
> > On Thu, Jul 19, 2018 at 10:53:44AM -0400, Tom Rini wrote:
> >> On Thu, Jul 19, 2018 at 03:45:11PM +0200, Michal Simek wrote:
> >>> On 19.7.2018 13:13, Maxime Ripard wrote:
> >>>> Hi,
> >>>>
> >>>> On Thu, Jul 19, 2018 at 08:45:45AM +0200, Michal Simek wrote:
> >>>>> There is no reason to have the same Kconfig options for different SoCs
> >>>>> separately. The patch is merging them together.
> >>>>>
> >>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >>>>> ---
> >>>>>
> >>>>> Patch is based on
> >>>>> https://lists.denx.de/pipermail/u-boot/2018-July/335126.html
> >>>>>
> >>>>> I have ENV_SECT_SIZE just for zynq/zynqmp because rockchip and sunxi
> >>>>> have this in their configs. When they decide to move then can enable
> >>>>> that option for them too.
> >>>>> I expect when more platforms extend this we will have less constrain
> >>>>> Kconfig setup.
> >>>>>
> >>>>> ---
> >>>>>  env/Kconfig | 66 ++++++++++++++++---------------------------------------------
> >>>>>  1 file changed, 17 insertions(+), 49 deletions(-)
> >>>>>
> >>>>> diff --git a/env/Kconfig b/env/Kconfig
> >>>>> index b37dcd78eb75..0ded003d7d41 100644
> >>>>> --- a/env/Kconfig
> >>>>> +++ b/env/Kconfig
> >>>>> @@ -431,23 +431,37 @@ config ENV_EXT4_FILE
> >>>>>  	  It's a string of the EXT4 file name. This file use to store the
> >>>>>  	  environment (explicit path to the file)
> >>>>>  
> >>>>> -if ARCH_SUNXI
> >>>>> +if ARCH_ROCKCHIP || ARCH_SUNXI || ARCH_ZYNQ || ARCH_ZYNQMP
> >>>>
> >>>> Can we have a depends on instead? That would be more flexible.
> >>>
> >>> In what sense? If depends is used below then the same 4 platforms will
> >>> be listed on all options below. (I want to also add ZYNQMP_R5 there too)
> >>> And changing this in one place seems to me better then on four.
> >>
> >> For now I like the "if" method for now as we can't (or couldn't a while
> >> ago) globally migrate everyone over.  I think trying to move everyone
> >> over again is something I should give another try.
> > 
> > Ack.
> > 
> >>>>>  config ENV_OFFSET
> >>>>>  	hex "Environment Offset"
> >>>>>  	depends on !ENV_IS_IN_UBI
> >>>>>  	depends on !ENV_IS_NOWHERE
> >>>>> +	default 0x3f8000 if ARCH_ROCKCHIP
> >>>>>  	default 0x88000 if ARCH_SUNXI
> >>>>> +	default 0xE0000 if ARCH_ZYNQ
> >>>>> +	default 0x1E00000 if ARCH_ZYNQMP
> >>>>>  	help
> >>>>>  	  Offset from the start of the device (or partition)
> >>>>>  
> >>>>>  config ENV_SIZE
> >>>>>  	hex "Environment Size"
> >>>>> -	depends on !ENV_IS_NOWHERE
> >>>>> -	default 0x20000 if ARCH_SUNXI
> >>>>> +	default 0x8000 if ARCH_ROCKCHIP && !ENV_IS_NOWHERE
> >>>>> +	default 0x20000 if ARCH_SUNXI && !ENV_IS_NOWHERE
> >>>>
> >>>> I'm not sure why you removed the depends on !ENV_IS_NOWHERE. Do you
> >>>> have a case where the environment is not store anywhere but still need
> >>>> a size?
> >>>
> >>> yes, I had a compilation warning for that case.
> >>>
> >>> in include/environment.h at line 145 it is written this
> >>> #define ENV_SIZE (CONFIG_ENV_SIZE - ENV_HEADER_SIZE)
> >>>
> >>> ENV_SIZE is also used in typedef struct environment_s some lines below.
> >>> And this structure is used a lot.
> >>>
> >>> How did you find out that this can't be used for ENV_IS_NOWHERE?
> >>
> >> I would have sworn that ENV_SIZE is used for ENV_IS_NOWHERE as that's
> >> how much space we have for environment when it's in memory as well.
> > 
> > Argh, sorry for that I was abused by sunxi-common still having that:
> > https://git.denx.de/?p=u-boot.git;a=blob;f=include/configs/sunxi-common.h#l161
> > 
> > While i was convinced that we were relying solely on Kconfig. I'll
> > send a subsequent patch, that one works for me.
> 
> Ok. Can you please convert this to any official tag which I can include?

Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180720/4f8127ef/attachment.sig>

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

* [U-Boot] env: Merge Rockchip, Sunxi, Zynq and ZynqMP
  2018-07-19  6:45 [U-Boot] [PATCH] env: Merge Rockchip, Sunxi, Zynq and ZynqMP Michal Simek
  2018-07-19 11:13 ` Maxime Ripard
@ 2018-08-19 19:14 ` Tom Rini
  2018-08-20  6:42   ` Michal Simek
  1 sibling, 1 reply; 10+ messages in thread
From: Tom Rini @ 2018-08-19 19:14 UTC (permalink / raw)
  To: u-boot

On Thu, Jul 19, 2018 at 08:45:45AM +0200, Michal Simek wrote:

> There is no reason to have the same Kconfig options for different SoCs
> separately. The patch is merging them together.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>

After doing a little work around the ENV_IS_NOWHERE parts and
size-testing, applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180819/975ff2a2/attachment-0001.sig>

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

* [U-Boot] env: Merge Rockchip, Sunxi, Zynq and ZynqMP
  2018-08-19 19:14 ` [U-Boot] " Tom Rini
@ 2018-08-20  6:42   ` Michal Simek
  2018-08-20 11:56     ` Tom Rini
  0 siblings, 1 reply; 10+ messages in thread
From: Michal Simek @ 2018-08-20  6:42 UTC (permalink / raw)
  To: u-boot

On 19.8.2018 21:14, Tom Rini wrote:
> On Thu, Jul 19, 2018 at 08:45:45AM +0200, Michal Simek wrote:
> 
>> There is no reason to have the same Kconfig options for different SoCs
>> separately. The patch is merging them together.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
> 
> After doing a little work around the ENV_IS_NOWHERE parts and
> size-testing, applied to u-boot/master, thanks!

I expect you forget to push.

Thanks,
Michal

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

* [U-Boot] env: Merge Rockchip, Sunxi, Zynq and ZynqMP
  2018-08-20  6:42   ` Michal Simek
@ 2018-08-20 11:56     ` Tom Rini
  0 siblings, 0 replies; 10+ messages in thread
From: Tom Rini @ 2018-08-20 11:56 UTC (permalink / raw)
  To: u-boot

On Mon, Aug 20, 2018 at 08:42:07AM +0200, Michal Simek wrote:

> On 19.8.2018 21:14, Tom Rini wrote:
> > On Thu, Jul 19, 2018 at 08:45:45AM +0200, Michal Simek wrote:
> > 
> >> There is no reason to have the same Kconfig options for different SoCs
> >> separately. The patch is merging them together.
> >>
> >> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> >> Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > 
> > After doing a little work around the ENV_IS_NOWHERE parts and
> > size-testing, applied to u-boot/master, thanks!
> 
> I expect you forget to push.

Sadly the sync out to the public side of git.denx.de is lagging behind again.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180820/75a3448b/attachment.sig>

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

end of thread, other threads:[~2018-08-20 11:56 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-19  6:45 [U-Boot] [PATCH] env: Merge Rockchip, Sunxi, Zynq and ZynqMP Michal Simek
2018-07-19 11:13 ` Maxime Ripard
2018-07-19 13:45   ` Michal Simek
2018-07-19 14:53     ` Tom Rini
2018-07-19 15:50       ` Maxime Ripard
2018-07-20  6:28         ` Michal Simek
2018-07-20  7:45           ` Maxime Ripard
2018-08-19 19:14 ` [U-Boot] " Tom Rini
2018-08-20  6:42   ` Michal Simek
2018-08-20 11:56     ` Tom Rini

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.