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

[Nathan Fontenot]

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 

Reviewed-by: Nathan Fontenot 

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

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

[Bharata B Rao]

On Wed, Feb 21, 2018 at 10:48:18PM +1100, Michael Ellerman wrote:
> Bharata B Rao  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 
> 
> Presumably:
> 
>   Fixes: 2b31e3aec1db ("powerpc/drmem: Add support for ibm, dynamic-memory-v2 
> property")

Yes.

Regards,
Bharata.

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

[Michael Ellerman]

Bharata B Rao  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 

Presumably:

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

?

cheers

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

[Bharata B Rao]

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 
---
 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++;
-- 
2.7.4