[U-Boot] [PATCH v2] common/memsize.c: Increase save array for supporting memory size > 4GB

[U-Boot] [PATCH v2] common/memsize.c: Increase save array for supporting memory size > 4GB

[tien.fong.chee at intel.com]

From: Tien Fong Chee <tien.fong.chee@intel.com>

In ARM 64-bits, memory size can be supported is more than 4GB,
hence increasing save array is needed to cope with testing larger memory.

Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
---

Changes in v2:
- Change save array size to save[BITS_PER_LONG - 1]
---
 common/memsize.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/memsize.c b/common/memsize.c
index 5670e95..13b0047 100644
--- a/common/memsize.c
+++ b/common/memsize.c
@@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR;
 long get_ram_size(long *base, long maxsize)
 {
 	volatile long *addr;
-	long           save[31];
+	long           save[BITS_PER_LONG - 1];
 	long           save_base;
 	long           cnt;
 	long           val;
-- 
2.2.0

[U-Boot] [PATCH v2] common/memsize.c: Increase save array for supporting memory size > 4GB

[Marek Vasut]

On 06/20/2018 09:06 AM, tien.fong.chee at intel.com wrote:
> From: Tien Fong Chee <tien.fong.chee@intel.com>
> 
> In ARM 64-bits, memory size can be supported is more than 4GB,
> hence increasing save array is needed to cope with testing larger memory.
> 
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> ---
> 
> Changes in v2:
> - Change save array size to save[BITS_PER_LONG - 1]
> ---
>  common/memsize.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/memsize.c b/common/memsize.c
> index 5670e95..13b0047 100644
> --- a/common/memsize.c
> +++ b/common/memsize.c
> @@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR;
>  long get_ram_size(long *base, long maxsize)
>  {
>  	volatile long *addr;
> -	long           save[31];
> +	long           save[BITS_PER_LONG - 1];
>  	long           save_base;
>  	long           cnt;
>  	long           val;
> 
Does this work with LPAE systems, where bits per long == 32 and the
address space is bigger ? Or with similar setups ? I mean, you can have
> 4 GiB of RAM on 32bit system ...

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2] common/memsize.c: Increase save array for supporting memory size > 4GB

[Chee, Tien Fong]

On Thu, 2018-06-21 at 06:34 +0200, Marek Vasut wrote:
> On 06/20/2018 09:06 AM, tien.fong.chee at intel.com wrote:
> > 
> > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > 
> > In ARM 64-bits, memory size can be supported is more than 4GB,
> > hence increasing save array is needed to cope with testing larger
> > memory.
> > 
> > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > ---
> > 
> > Changes in v2:
> > - Change save array size to save[BITS_PER_LONG - 1]
> > ---
> >  common/memsize.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/common/memsize.c b/common/memsize.c
> > index 5670e95..13b0047 100644
> > --- a/common/memsize.c
> > +++ b/common/memsize.c
> > @@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR;
> >  long get_ram_size(long *base, long maxsize)
> >  {
> >  	volatile long *addr;
> > -	long           save[31];
> > +	long           save[BITS_PER_LONG - 1];
> >  	long           save_base;
> >  	long           cnt;
> >  	long           val;
> > 
> Does this work with LPAE systems, where bits per long == 32 and the
> address space is bigger ? Or with similar setups ? I mean, you can
> have
> > 
> > 4 GiB of RAM on 32bit system ...
This function is designed to work with 32bit or 64 bits system, for
example the argument such as maxsize can be 32bit or 64 bit, if value
larger than 4GiB is passed as maxsize argument with 32bit system, the
whole thing will go wrong.

[U-Boot] [PATCH v2] common/memsize.c: Increase save array for supporting memory size > 4GB

[Marek Vasut]

On 06/21/2018 08:31 AM, Chee, Tien Fong wrote:
> On Thu, 2018-06-21 at 06:34 +0200, Marek Vasut wrote:
>> On 06/20/2018 09:06 AM, tien.fong.chee at intel.com wrote:
>>>
>>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>>
>>> In ARM 64-bits, memory size can be supported is more than 4GB,
>>> hence increasing save array is needed to cope with testing larger
>>> memory.
>>>
>>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
>>> ---
>>>
>>> Changes in v2:
>>> - Change save array size to save[BITS_PER_LONG - 1]
>>> ---
>>>  common/memsize.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/common/memsize.c b/common/memsize.c
>>> index 5670e95..13b0047 100644
>>> --- a/common/memsize.c
>>> +++ b/common/memsize.c
>>> @@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR;
>>>  long get_ram_size(long *base, long maxsize)
>>>  {
>>>  	volatile long *addr;
>>> -	long           save[31];
>>> +	long           save[BITS_PER_LONG - 1];
>>>  	long           save_base;
>>>  	long           cnt;
>>>  	long           val;
>>>
>> Does this work with LPAE systems, where bits per long == 32 and the
>> address space is bigger ? Or with similar setups ? I mean, you can
>> have
>>>
>>> 4 GiB of RAM on 32bit system ...
> This function is designed to work with 32bit or 64 bits system, for
> example the argument such as maxsize can be 32bit or 64 bit, if value
> larger than 4GiB is passed as maxsize argument with 32bit system, the
> whole thing will go wrong.

And what do you do when you identify a problem in mainline ? :-)

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH v2] common/memsize.c: Increase save array for supporting memory size > 4GB

[Chee, Tien Fong]

On Sat, 2018-06-23 at 05:29 +0200, Marek Vasut wrote:
> On 06/21/2018 08:31 AM, Chee, Tien Fong wrote:
> > 
> > On Thu, 2018-06-21 at 06:34 +0200, Marek Vasut wrote:
> > > 
> > > On 06/20/2018 09:06 AM, tien.fong.chee at intel.com wrote:
> > > > 
> > > > 
> > > > From: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > 
> > > > In ARM 64-bits, memory size can be supported is more than 4GB,
> > > > hence increasing save array is needed to cope with testing
> > > > larger
> > > > memory.
> > > > 
> > > > Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > > > ---
> > > > 
> > > > Changes in v2:
> > > > - Change save array size to save[BITS_PER_LONG - 1]
> > > > ---
> > > >  common/memsize.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/common/memsize.c b/common/memsize.c
> > > > index 5670e95..13b0047 100644
> > > > --- a/common/memsize.c
> > > > +++ b/common/memsize.c
> > > > @@ -26,7 +26,7 @@ DECLARE_GLOBAL_DATA_PTR;
> > > >  long get_ram_size(long *base, long maxsize)
> > > >  {
> > > >  	volatile long *addr;
> > > > -	long           save[31];
> > > > +	long           save[BITS_PER_LONG - 1];
> > > >  	long           save_base;
> > > >  	long           cnt;
> > > >  	long           val;
> > > > 
> > > Does this work with LPAE systems, where bits per long == 32 and
> > > the
> > > address space is bigger ? Or with similar setups ? I mean, you
> > > can
> > > have
> > > > 
> > > > 
> > > > 4 GiB of RAM on 32bit system ...
> > This function is designed to work with 32bit or 64 bits system, for
> > example the argument such as maxsize can be 32bit or 64 bit, if
> > value
> > larger than 4GiB is passed as maxsize argument with 32bit system,
> > the
> > whole thing will go wrong.
> And what do you do when you identify a problem in mainline ? :-)
> 
I have no idea at this moment, how about fixing the issue for save
array in 64-bit 1st?

[U-Boot] [U-Boot, v2] common/memsize.c: Increase save array for supporting memory size > 4GB

[Tom Rini]

On Wed, Jun 20, 2018 at 03:06:20PM +0800, tien.fong.chee at intel.com wrote:

> From: Tien Fong Chee <tien.fong.chee@intel.com>
> 
> In ARM 64-bits, memory size can be supported is more than 4GB,
> hence increasing save array is needed to cope with testing larger memory.
> 
> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>

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/20180711/8d4c4a4e/attachment.sig>

[U-Boot] [U-Boot, v2] common/memsize.c: Increase save array for supporting memory size > 4GB

[Marek Vasut]

On 07/11/2018 02:42 PM, Tom Rini wrote:
> On Wed, Jun 20, 2018 at 03:06:20PM +0800, tien.fong.chee at intel.com wrote:
> 
>> From: Tien Fong Chee <tien.fong.chee@intel.com>
>>
>> In ARM 64-bits, memory size can be supported is more than 4GB,
>> hence increasing save array is needed to cope with testing larger memory.
>>
>> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> 
> Applied to u-boot/master, thanks!

There were review comments on this patch which were not addressed,
please drop.

-- 
Best regards,
Marek Vasut

[U-Boot] [U-Boot, v2] common/memsize.c: Increase save array for supporting memory size > 4GB

[Tom Rini]

On Wed, Jul 11, 2018 at 02:48:34PM +0200, Marek Vasut wrote:
> On 07/11/2018 02:42 PM, Tom Rini wrote:
> > On Wed, Jun 20, 2018 at 03:06:20PM +0800, tien.fong.chee at intel.com wrote:
> > 
> >> From: Tien Fong Chee <tien.fong.chee@intel.com>
> >>
> >> In ARM 64-bits, memory size can be supported is more than 4GB,
> >> hence increasing save array is needed to cope with testing larger memory.
> >>
> >> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> > 
> > Applied to u-boot/master, thanks!
> 
> There were review comments on this patch which were not addressed,
> please drop.

I thought I had put them in my message here, sorry.  I agree with Tien
that we should fix the issue we have now and address the enhancement in
a follow up patch.

-- 
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/20180711/c5ba83a2/attachment.sig>