On 13/12/2019 13:00, Greg Kurz wrote:
> Since pnv_dt_xscom() is called from chip specific dt_populate() hooks,
> it shouldn't have to guess the chip type in order to populate the
> "compatible" property. Just pass the compat string and its size as
> arguments.
Yeah. That is where I think a PnXscom model and class could be a little
cleaner. This is minor.
> Signed-off-by: Greg Kurz <gr...@kaod.org>
Reviewed-by: Cédric Le Goater <c...@kaod.org>
> ---
> hw/ppc/pnv.c | 12 +++++++++---
> hw/ppc/pnv_xscom.c | 20 +++-----------------
> include/hw/ppc/pnv_xscom.h | 3 ++-
> 3 files changed, 14 insertions(+), 21 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index c532e98e752a..0447b534b8c5 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -280,11 +280,13 @@ static void pnv_dt_icp(PnvChip *chip, void *fdt,
> uint32_t pir,
>
> static void pnv_chip_power8_dt_populate(PnvChip *chip, void *fdt)
> {
> + static const char compat[] = "ibm,power8-xscom\0ibm,xscom";
> int i;
>
> pnv_dt_xscom(chip, fdt, 0,
> cpu_to_be64(PNV_XSCOM_BASE(chip)),
> - cpu_to_be64(PNV_XSCOM_SIZE));
> + cpu_to_be64(PNV_XSCOM_SIZE),
> + compat, sizeof(compat));
>
> for (i = 0; i < chip->nr_cores; i++) {
> PnvCore *pnv_core = chip->cores[i];
> @@ -302,11 +304,13 @@ static void pnv_chip_power8_dt_populate(PnvChip *chip,
> void *fdt)
>
> static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
> {
> + static const char compat[] = "ibm,power9-xscom\0ibm,xscom";
> int i;
>
> pnv_dt_xscom(chip, fdt, 0,
> cpu_to_be64(PNV9_XSCOM_BASE(chip)),
> - cpu_to_be64(PNV9_XSCOM_SIZE));
> + cpu_to_be64(PNV9_XSCOM_SIZE),
> + compat, sizeof(compat));
>
> for (i = 0; i < chip->nr_cores; i++) {
> PnvCore *pnv_core = chip->cores[i];
> @@ -323,11 +327,13 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip,
> void *fdt)
>
> static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
> {
> + static const char compat[] = "ibm,power10-xscom\0ibm,xscom";
> int i;
>
> pnv_dt_xscom(chip, fdt, 0,
> cpu_to_be64(PNV10_XSCOM_BASE(chip)),
> - cpu_to_be64(PNV10_XSCOM_SIZE));
> + cpu_to_be64(PNV10_XSCOM_SIZE),
> + compat, sizeof(compat));
>
> for (i = 0; i < chip->nr_cores; i++) {
> PnvCore *pnv_core = chip->cores[i];
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index 8189767eb0bb..5ae9dfbb88ad 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -282,12 +282,9 @@ static int xscom_dt_child(Object *child, void *opaque)
> return 0;
> }
>
> -static const char compat_p8[] = "ibm,power8-xscom\0ibm,xscom";
> -static const char compat_p9[] = "ibm,power9-xscom\0ibm,xscom";
> -static const char compat_p10[] = "ibm,power10-xscom\0ibm,xscom";
> -
> int pnv_dt_xscom(PnvChip *chip, void *fdt, int root_offset,
> - uint64_t xscom_base, uint64_t xscom_size)
> + uint64_t xscom_base, uint64_t xscom_size,
> + const char *compat, int compat_size)
> {
> uint64_t reg[] = { xscom_base, xscom_size };
> int xscom_offset;
> @@ -302,18 +299,7 @@ int pnv_dt_xscom(PnvChip *chip, void *fdt, int
> root_offset,
> _FDT((fdt_setprop_cell(fdt, xscom_offset, "#address-cells", 1)));
> _FDT((fdt_setprop_cell(fdt, xscom_offset, "#size-cells", 1)));
> _FDT((fdt_setprop(fdt, xscom_offset, "reg", reg, sizeof(reg))));
> -
> - if (pnv_chip_is_power10(chip)) {
> - _FDT((fdt_setprop(fdt, xscom_offset, "compatible", compat_p10,
> - sizeof(compat_p10))));
> - } else if (pnv_chip_is_power9(chip)) {
> - _FDT((fdt_setprop(fdt, xscom_offset, "compatible", compat_p9,
> - sizeof(compat_p9))));
> - } else {
> - _FDT((fdt_setprop(fdt, xscom_offset, "compatible", compat_p8,
> - sizeof(compat_p8))));
> - }
> -
> + _FDT((fdt_setprop(fdt, xscom_offset, "compatible", compat,
> compat_size)));
> _FDT((fdt_setprop(fdt, xscom_offset, "scom-controller", NULL, 0)));
>
> args.fdt = fdt;
> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
> index ad53f788b44c..f74c81a980f3 100644
> --- a/include/hw/ppc/pnv_xscom.h
> +++ b/include/hw/ppc/pnv_xscom.h
> @@ -115,7 +115,8 @@ typedef struct PnvXScomInterfaceClass {
>
> void pnv_xscom_realize(PnvChip *chip, uint64_t size, Error **errp);
> int pnv_dt_xscom(PnvChip *chip, void *fdt, int root_offset,
> - uint64_t xscom_base, uint64_t xscom_size);
> + uint64_t xscom_base, uint64_t xscom_size,
> + const char *compat, int compat_size);
>
> void pnv_xscom_add_subregion(PnvChip *chip, hwaddr offset,
> MemoryRegion *mr);
>
