[Skiboot] [PATCH 02/12] cpu: use dt accessor device tree access
Oliver O'Halloran
oohall at gmail.com
Tue Oct 1 13:03:32 AEST 2019
On Sun, 2019-09-29 at 17:46 +1000, Nicholas Piggin wrote:
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
> ---
> core/cpu.c | 2 +-
> core/device.c | 1 +
> core/fdt.c | 4 ++--
> core/interrupts.c | 4 ++--
> core/pci.c | 28 +++++++++++++---------------
> hdata/fsp.c | 7 ++++---
> hdata/iohub.c | 20 ++++++++++----------
> hdata/paca.c | 10 +++++-----
> hw/fsp/fsp-sysparam.c | 4 ++--
> hw/imc.c | 4 ++--
> hw/lpc.c | 6 +++---
> hw/psi.c | 8 ++++----
> hw/vas.c | 7 ++++---
> 13 files changed, 53 insertions(+), 52 deletions(-)
>
> diff --git a/core/pci.c b/core/pci.c
> index 9ee70f4fd..6c5c83bea 100644
> --- a/core/pci.c
> +++ b/core/pci.c
> @@ -1549,16 +1547,16 @@ static void __noinline pci_add_one_device_node(struct phb *phb,
> char name[MAX_NAME];
> char compat[MAX_NAME];
> uint32_t rev_class, vdid;
> - uint32_t reg[5];
> + __be32 reg[5];
> uint8_t intpin;
> bool is_pcie;
> - const uint32_t ranges_direct[] = {
> + const __be32 ranges_direct[] = {
> /* 64-bit direct mapping. We know the bridges
> * don't cover the entire address space so
> * use 0xf00... as a good compromise. */
> - 0x02000000, 0x0, 0x0,
> - 0x02000000, 0x0, 0x0,
> - 0xf0000000, 0x0};
> + cpu_to_be32(0x02000000), 0x0, 0x0,
> + cpu_to_be32(0x02000000), 0x0, 0x0,
> + cpu_to_be32(0xf0000000), 0x0};
>
> pci_cfg_read32(phb, pd->bdfn, 0, &vdid);
> pci_cfg_read32(phb, pd->bdfn, PCI_CFG_REV_ID, &rev_class);
> @@ -1635,7 +1633,7 @@ static void __noinline pci_add_one_device_node(struct phb *phb,
> * entry in the "reg" property. That's enough for Linux and we might
> * even want to make this legit in future ePAPR
> */
> - reg[0] = pd->bdfn << 8;
> + reg[0] = cpu_to_be32(pd->bdfn << 8);
> reg[1] = reg[2] = reg[3] = reg[4] = 0;
> dt_add_property(np, "reg", reg, sizeof(reg));
I think we can use dt_add_property_cells() here instead of constructing
reg manually.
> diff --git a/hdata/iohub.c b/hdata/iohub.c
> index 6921d95ce..2af040a2f 100644
> --- a/hdata/iohub.c
> +++ b/hdata/iohub.c
> @@ -109,12 +109,12 @@ static struct dt_node *io_add_phb3(const struct cechub_io_hub *hub,
> /* "reg" property contains in order the PE, PCI and SPCI XSCOM
> * addresses
> */
> - reg[0] = pe_xscom;
> - reg[1] = 0x20;
> - reg[2] = pci_xscom;
> - reg[3] = 0x05;
> - reg[4] = spci_xscom;
> - reg[5] = 0x15;
> + reg[0] = cpu_to_be32(pe_xscom);
> + reg[1] = cpu_to_be32(0x20);
> + reg[2] = cpu_to_be32(pci_xscom);
> + reg[3] = cpu_to_be32(0x05);
> + reg[4] = cpu_to_be32(spci_xscom);
> + reg[5] = cpu_to_be32(0x15);
> dt_add_property(pbcq, "reg", reg, sizeof(reg));
Same here.
>
> /* A couple more things ... */
> @@ -214,10 +214,10 @@ static struct dt_node *io_add_phb4(const struct cechub_io_hub *hub,
> return NULL;
>
> /* "reg" property contains (in order) the PE and PCI XSCOM addresses */
> - reg[0] = pe_xscom;
> - reg[1] = 0x100;
> - reg[2] = pci_xscom;
> - reg[3] = 0x200;
> + reg[0] = cpu_to_be32(pe_xscom);
> + reg[1] = cpu_to_be32(0x100);
> + reg[2] = cpu_to_be32(pci_xscom);
> + reg[3] = cpu_to_be32(0x200);
> dt_add_property(pbcq, "reg", reg, sizeof(reg));
And here.
> diff --git a/hdata/paca.c b/hdata/paca.c
> index 28025b0cd..6ebb75320 100644
> --- a/hdata/paca.c
> +++ b/hdata/paca.c
> @@ -174,8 +174,8 @@ static void add_xics_icps(void)
> "IBM,ppc-xicp",
> "IBM,power7-xicp");
>
> - irange[0] = dt_property_get_cell(intsrv, 0); /* Index */
> - irange[1] = num_threads; /* num servers */
> + irange[0] = cpu_to_be32(dt_property_get_cell(intsrv, 0)); /* Index */
> + irange[1] = cpu_to_be32(num_threads); /* num servers */
> dt_add_property(icp, "ibm,interrupt-server-ranges",
> irange, sizeof(irange));
> dt_add_property(icp, "interrupt-controller", NULL, 0);
> @@ -183,10 +183,10 @@ static void add_xics_icps(void)
> dt_add_property_string(icp, "device_type",
> "PowerPC-External-Interrupt-Presentation");
> for (i = 0; i < num_threads*2; i += 2) {
> - reg[i] = ibase;
> + reg[i] = cpu_to_be64(ibase);
> /* One page is enough for a handful of regs. */
> - reg[i+1] = 4096;
> - ibase += reg[i+1];
> + reg[i+1] = cpu_to_be64(4096);
> + ibase += be64_to_cpu(reg[i+1]);
> }
> dt_add_property(icp, "reg", reg, sizeof(reg));
> }
Looks like this code is used as a fallback if we can't find the XICS
address ranges in the PACA. However, it's also got:
num_threads = intsrv->len / sizeof(u32);
assert(num_threads <= PACA_MAX_THREADS);
icp = dt_new_addr(dt_root, "interrupt-controller", ibase);
if (!icp)
continue;
dt_add_property_strings(icp, "compatible",
"IBM,ppc-xicp",
"IBM,power7-xicp");
and:
#define PACA_MAX_THREADS 4
So I'm guessing this is all dead P7 code that we can delete.
> diff --git a/hw/vas.c b/hw/vas.c
> index 212da0ec1..3c5ebc920 100644
> --- a/hw/vas.c
> +++ b/hw/vas.c
> @@ -375,7 +375,7 @@ static struct vas *alloc_vas(uint32_t chip_id, uint32_t vas_id, uint64_t base)
>
> static void create_mm_dt_node(struct proc_chip *chip)
> {
> - int gcid;
> + uint32_t gcid, vas_id;
> struct dt_node *dn;
> struct vas *vas;
> uint64_t hvwc_start, hvwc_len;
> @@ -384,7 +384,8 @@ static void create_mm_dt_node(struct proc_chip *chip)
> uint64_t pbf_start, pbf_nbits;
>
> vas = chip->vas;
> - gcid = chip->id;
> + vas_id = cpu_to_be32(vas->vas_id);
> + gcid = cpu_to_be32(chip->id);
> get_hvwc_mmio_bar(chip->id, &hvwc_start, &hvwc_len);
> get_uwc_mmio_bar(chip->id, &uwc_start, &uwc_len);
> get_paste_bar(chip->id, &pbar_start, &pbar_len);
> @@ -400,7 +401,7 @@ static void create_mm_dt_node(struct proc_chip *chip)
> pbar_start, pbar_len,
> pbf_start, pbf_nbits);
>
> - dt_add_property(dn, "ibm,vas-id", &vas->vas_id, sizeof(vas->vas_id));
> + dt_add_property(dn, "ibm,vas-id", &vas_id, sizeof(vas_id));
> dt_add_property(dn, "ibm,chip-id", &gcid, sizeof(gcid));
> }
Urgh, why is it use dt_add_property() directly... Might as well convert
these to use dt_add_property_cells() too
Other hunks looked fine.
More information about the Skiboot
mailing list