diff mbox series

[RFC,v0,1/2] powerpc, drmem: Fix unexpected flag value in ibm, dynamic-memory-v2

Message ID 1519209387-29767-2-git-send-email-bharata@linux.vnet.ibm.com (mailing list archive)
State Accepted
Commit 2f7d03e0511991f124455682cc94094eaa0981ea
Headers show
Series ibm,dynamic-memory-v2 fix | expand

Commit Message

Bharata B Rao Feb. 21, 2018, 10:36 a.m. UTC
Memory addtion and removal by count and indexed-count methods
temporarily mark the LMBs that are being added/removed by a special
flag value DRMEM_LMB_RESERVED. Accessing flags value directly at
a few places without proper accessor method is causing two unexpected
side-effects:

- DRMEM_LMB_RESERVED bit is becoming part of the flags word of
  drconf_cell_v2 entries in ibm,dynamic-memory-v2 DT property.
- This results in extra drconf_cell entries in ibm,dynamic-memory-v2.
  For example if 1G memory is added, it leads to one entry for 3 LMBs
  and 1 separate entry for the last LMB. All the 4 LMBs should be
  defined by one entry here.

Fix this by always accessing the flags by its accessor method
drmem_lmb_flags().

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 arch/powerpc/mm/drmem.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Michael Ellerman Feb. 21, 2018, 11:48 a.m. UTC | #1
Bharata B Rao <bharata@linux.vnet.ibm.com> writes:

> Memory addtion and removal by count and indexed-count methods
> temporarily mark the LMBs that are being added/removed by a special
> flag value DRMEM_LMB_RESERVED. Accessing flags value directly at
> a few places without proper accessor method is causing two unexpected
> side-effects:
>
> - DRMEM_LMB_RESERVED bit is becoming part of the flags word of
>   drconf_cell_v2 entries in ibm,dynamic-memory-v2 DT property.
> - This results in extra drconf_cell entries in ibm,dynamic-memory-v2.
>   For example if 1G memory is added, it leads to one entry for 3 LMBs
>   and 1 separate entry for the last LMB. All the 4 LMBs should be
>   defined by one entry here.
>
> Fix this by always accessing the flags by its accessor method
> drmem_lmb_flags().
>
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

Presumably:

  Fixes: 2b31e3aec1db ("powerpc/drmem: Add support for ibm, dynamic-memory-v2 property")

?

cheers
Bharata B Rao Feb. 21, 2018, 1:36 p.m. UTC | #2
On Wed, Feb 21, 2018 at 10:48:18PM +1100, Michael Ellerman wrote:
> Bharata B Rao <bharata@linux.vnet.ibm.com> writes:
> 
> > Memory addtion and removal by count and indexed-count methods
> > temporarily mark the LMBs that are being added/removed by a special
> > flag value DRMEM_LMB_RESERVED. Accessing flags value directly at
> > a few places without proper accessor method is causing two unexpected
> > side-effects:
> >
> > - DRMEM_LMB_RESERVED bit is becoming part of the flags word of
> >   drconf_cell_v2 entries in ibm,dynamic-memory-v2 DT property.
> > - This results in extra drconf_cell entries in ibm,dynamic-memory-v2.
> >   For example if 1G memory is added, it leads to one entry for 3 LMBs
> >   and 1 separate entry for the last LMB. All the 4 LMBs should be
> >   defined by one entry here.
> >
> > Fix this by always accessing the flags by its accessor method
> > drmem_lmb_flags().
> >
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> 
> Presumably:
> 
>   Fixes: 2b31e3aec1db ("powerpc/drmem: Add support for ibm, dynamic-memory-v2 property")

Yes.

Regards,
Bharata.
Nathan Fontenot Feb. 22, 2018, 1:51 p.m. UTC | #3
On 02/21/2018 04:36 AM, Bharata B Rao wrote:
> Memory addtion and removal by count and indexed-count methods
> temporarily mark the LMBs that are being added/removed by a special
> flag value DRMEM_LMB_RESERVED. Accessing flags value directly at
> a few places without proper accessor method is causing two unexpected
> side-effects:
> 
> - DRMEM_LMB_RESERVED bit is becoming part of the flags word of
>   drconf_cell_v2 entries in ibm,dynamic-memory-v2 DT property.
> - This results in extra drconf_cell entries in ibm,dynamic-memory-v2.
>   For example if 1G memory is added, it leads to one entry for 3 LMBs
>   and 1 separate entry for the last LMB. All the 4 LMBs should be
>   defined by one entry here.
> 
> Fix this by always accessing the flags by its accessor method
> drmem_lmb_flags().
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

Reviewed-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>

> ---
>  arch/powerpc/mm/drmem.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
> index 916844f..3f18036 100644
> --- a/arch/powerpc/mm/drmem.c
> +++ b/arch/powerpc/mm/drmem.c
> @@ -98,7 +98,7 @@ static void init_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>  	dr_cell->base_addr = cpu_to_be64(lmb->base_addr);
>  	dr_cell->drc_index = cpu_to_be32(lmb->drc_index);
>  	dr_cell->aa_index = cpu_to_be32(lmb->aa_index);
> -	dr_cell->flags = cpu_to_be32(lmb->flags);
> +	dr_cell->flags = cpu_to_be32(drmem_lmb_flags(lmb));
>  }
> 
>  static int drmem_update_dt_v2(struct device_node *memory,
> @@ -121,7 +121,7 @@ static int drmem_update_dt_v2(struct device_node *memory,
>  		}
> 
>  		if (prev_lmb->aa_index != lmb->aa_index ||
> -		    prev_lmb->flags != lmb->flags)
> +		    drmem_lmb_flags(prev_lmb) != drmem_lmb_flags(lmb))
>  			lmb_sets++;
> 
>  		prev_lmb = lmb;
> @@ -150,7 +150,7 @@ static int drmem_update_dt_v2(struct device_node *memory,
>  		}
> 
>  		if (prev_lmb->aa_index != lmb->aa_index ||
> -		    prev_lmb->flags != lmb->flags) {
> +		    drmem_lmb_flags(prev_lmb) != drmem_lmb_flags(lmb)) {
>  			/* end of one set, start of another */
>  			dr_cell->seq_lmbs = cpu_to_be32(seq_lmbs);
>  			dr_cell++;
>
Michael Ellerman Feb. 26, 2018, 5:35 a.m. UTC | #4
On Wed, 2018-02-21 at 10:36:26 UTC, Bharata B Rao wrote:
> Memory addtion and removal by count and indexed-count methods
> temporarily mark the LMBs that are being added/removed by a special
> flag value DRMEM_LMB_RESERVED. Accessing flags value directly at
> a few places without proper accessor method is causing two unexpected
> side-effects:
> 
> - DRMEM_LMB_RESERVED bit is becoming part of the flags word of
>   drconf_cell_v2 entries in ibm,dynamic-memory-v2 DT property.
> - This results in extra drconf_cell entries in ibm,dynamic-memory-v2.
>   For example if 1G memory is added, it leads to one entry for 3 LMBs
>   and 1 separate entry for the last LMB. All the 4 LMBs should be
>   defined by one entry here.
> 
> Fix this by always accessing the flags by its accessor method
> drmem_lmb_flags().
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> Reviewed-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/2f7d03e0511991f124455682cc9409

cheers
diff mbox series

Patch

diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
index 916844f..3f18036 100644
--- a/arch/powerpc/mm/drmem.c
+++ b/arch/powerpc/mm/drmem.c
@@ -98,7 +98,7 @@  static void init_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
 	dr_cell->base_addr = cpu_to_be64(lmb->base_addr);
 	dr_cell->drc_index = cpu_to_be32(lmb->drc_index);
 	dr_cell->aa_index = cpu_to_be32(lmb->aa_index);
-	dr_cell->flags = cpu_to_be32(lmb->flags);
+	dr_cell->flags = cpu_to_be32(drmem_lmb_flags(lmb));
 }
 
 static int drmem_update_dt_v2(struct device_node *memory,
@@ -121,7 +121,7 @@  static int drmem_update_dt_v2(struct device_node *memory,
 		}
 
 		if (prev_lmb->aa_index != lmb->aa_index ||
-		    prev_lmb->flags != lmb->flags)
+		    drmem_lmb_flags(prev_lmb) != drmem_lmb_flags(lmb))
 			lmb_sets++;
 
 		prev_lmb = lmb;
@@ -150,7 +150,7 @@  static int drmem_update_dt_v2(struct device_node *memory,
 		}
 
 		if (prev_lmb->aa_index != lmb->aa_index ||
-		    prev_lmb->flags != lmb->flags) {
+		    drmem_lmb_flags(prev_lmb) != drmem_lmb_flags(lmb)) {
 			/* end of one set, start of another */
 			dr_cell->seq_lmbs = cpu_to_be32(seq_lmbs);
 			dr_cell++;