| Message ID | 20180124143517.18469-4-andre.przywara@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | ARM: vGICv3: clean up optional DT properties | expand |
Hi Andre, On 24/01/18 14:35, Andre Przywara wrote: > The ARM GICv3 DT property "#redistributor-regions" is optional and only > useful if it has any other values than the architected "1". > Keep our generated DT node clean by emitting this property only if we > actually need more than one region. I really don't see the benefits of this patch. It is fine to have #redistributor-regions property in the DT. You might "clean" the DT but make the code a bit more complex. Cheers, > > Signed-off-by: Andre Przywara <andre.przywara@linaro.org> > --- > xen/arch/arm/gic-v3.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index a0d290b55c..9ad0cd19ef 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -1168,10 +1168,13 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d, > if ( res ) > return res; > > - res = fdt_property_cell(fdt, "#redistributor-regions", > - d->arch.vgic.nr_regions); > - if ( res ) > - return res; > + if ( d->arch.vgic.nr_regions > 1 ) > + { > + res = fdt_property_cell(fdt, "#redistributor-regions", > + d->arch.vgic.nr_regions); > + if ( res ) > + return res; > + } > > len = dt_cells_to_size(dt_n_addr_cells(gic) + dt_n_size_cells(gic)); > /* >
Hi, On 24/01/18 16:32, Julien Grall wrote: > Hi Andre, > > On 24/01/18 14:35, Andre Przywara wrote: >> The ARM GICv3 DT property "#redistributor-regions" is optional and only >> useful if it has any other values than the architected "1". >> Keep our generated DT node clean by emitting this property only if we >> actually need more than one region. > > I really don't see the benefits of this patch. It is fine to have > #redistributor-regions property in the DT. You might "clean" the DT but > make the code a bit more complex. Fair enough, I can easily drop this patch. Was a victim patch anyway to give reviewers some satisfaction :-D Cheers, Andre. > > Cheers, > >> >> Signed-off-by: Andre Przywara <andre.przywara@linaro.org> >> --- >> xen/arch/arm/gic-v3.c | 11 +++++++---- >> 1 file changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c >> index a0d290b55c..9ad0cd19ef 100644 >> --- a/xen/arch/arm/gic-v3.c >> +++ b/xen/arch/arm/gic-v3.c >> @@ -1168,10 +1168,13 @@ static int gicv3_make_hwdom_dt_node(const >> struct domain *d, >> if ( res ) >> return res; >> - res = fdt_property_cell(fdt, "#redistributor-regions", >> - d->arch.vgic.nr_regions); >> - if ( res ) >> - return res; >> + if ( d->arch.vgic.nr_regions > 1 ) >> + { >> + res = fdt_property_cell(fdt, "#redistributor-regions", >> + d->arch.vgic.nr_regions); >> + if ( res ) >> + return res; >> + } >> len = dt_cells_to_size(dt_n_addr_cells(gic) + >> dt_n_size_cells(gic)); >> /* >> >
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c index a0d290b55c..9ad0cd19ef 100644 --- a/xen/arch/arm/gic-v3.c +++ b/xen/arch/arm/gic-v3.c @@ -1168,10 +1168,13 @@ static int gicv3_make_hwdom_dt_node(const struct domain *d, if ( res ) return res; - res = fdt_property_cell(fdt, "#redistributor-regions", - d->arch.vgic.nr_regions); - if ( res ) - return res; + if ( d->arch.vgic.nr_regions > 1 ) + { + res = fdt_property_cell(fdt, "#redistributor-regions", + d->arch.vgic.nr_regions); + if ( res ) + return res; + } len = dt_cells_to_size(dt_n_addr_cells(gic) + dt_n_size_cells(gic)); /*
The ARM GICv3 DT property "#redistributor-regions" is optional and only useful if it has any other values than the architected "1". Keep our generated DT node clean by emitting this property only if we actually need more than one region. Signed-off-by: Andre Przywara <andre.przywara@linaro.org> --- xen/arch/arm/gic-v3.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)