| Message ID | cd3bd35a570064e1b03054bab73e29aa1578bd24.1741596512.git.mykyta_poturai@epam.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for R-Car Gen4 PCI host controller | expand |
On 11.03.25 12:24, Mykyta Poturai wrote: > From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > > Some of the PCI host bridges require private data. Create a generic > approach for that, so such bridges may request the private data to > be allocated during initialization. > > Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> > Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com> > --- > v1->v2: > * no change > --- > xen/arch/arm/include/asm/pci.h | 4 +++- > xen/arch/arm/pci/pci-host-common.c | 18 +++++++++++++++++- > xen/arch/arm/pci/pci-host-generic.c | 2 +- > xen/arch/arm/pci/pci-host-zynqmp.c | 2 +- > 4 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/asm/pci.h > index 7f77226c9b..44344ac8c1 100644 > --- a/xen/arch/arm/include/asm/pci.h > +++ b/xen/arch/arm/include/asm/pci.h > @@ -66,6 +66,7 @@ struct pci_host_bridge { > uint16_t segment; /* Segment number */ > struct pci_config_window* cfg; /* Pointer to the bridge config window */ > const struct pci_ops *ops; > + void *priv; /* Private data of the bridge. */ > }; > > struct pci_ops { > @@ -95,7 +96,8 @@ struct pci_ecam_ops { > extern const struct pci_ecam_ops pci_generic_ecam_ops; > > int pci_host_common_probe(struct dt_device_node *dev, > - const struct pci_ecam_ops *ops); > + const struct pci_ecam_ops *ops, > + size_t priv_sz); > int pci_generic_config_read(struct pci_host_bridge *bridge, pci_sbdf_t sbdf, > uint32_t reg, uint32_t len, uint32_t *value); > int pci_generic_config_write(struct pci_host_bridge *bridge, pci_sbdf_t sbdf, > diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/pci-host-common.c > index c0faf0f436..be7e6c3510 100644 > --- a/xen/arch/arm/pci/pci-host-common.c > +++ b/xen/arch/arm/pci/pci-host-common.c > @@ -209,7 +209,8 @@ static int pci_bus_find_domain_nr(struct dt_device_node *dev) > } > > int pci_host_common_probe(struct dt_device_node *dev, > - const struct pci_ecam_ops *ops) > + const struct pci_ecam_ops *ops, > + size_t priv_sz) > { > struct pci_host_bridge *bridge; > struct pci_config_window *cfg; > @@ -241,11 +242,26 @@ int pci_host_common_probe(struct dt_device_node *dev, > printk(XENLOG_ERR "Inconsistent \"linux,pci-domain\" property in DT\n"); > BUG(); > } > + > bridge->segment = domain; > + > + if ( priv_sz ) > + { > + bridge->priv = xzalloc_bytes(priv_sz); > + if ( !bridge->priv ) > + { > + err = -ENOMEM; > + goto err_priv; > + } > + } > + I'd like to propose to move allocation into pci_alloc_host_bridge() to keep mallocs in one place and do it earlier, before proceeding with other initialization steps. Also the pci_alloc_host_bridge() could be made static, seems. > pci_add_host_bridge(bridge); > > return 0; > > +err_priv: > + xfree(bridge->priv); > + > err_exit: > xfree(bridge); > [...] -grygorii
Hi, On 11/03/2025 15:05, Grygorii Strashko wrote: > > > On 11.03.25 12:24, Mykyta Poturai wrote: >> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> >> Some of the PCI host bridges require private data. Create a generic >> approach for that, so such bridges may request the private data to >> be allocated during initialization. >> >> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> Signed-off-by: Mykyta Poturai <mykyta_poturai@epam.com> >> --- >> v1->v2: >> * no change >> --- >> xen/arch/arm/include/asm/pci.h | 4 +++- >> xen/arch/arm/pci/pci-host-common.c | 18 +++++++++++++++++- >> xen/arch/arm/pci/pci-host-generic.c | 2 +- >> xen/arch/arm/pci/pci-host-zynqmp.c | 2 +- >> 4 files changed, 22 insertions(+), 4 deletions(-) >> >> diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/ >> asm/pci.h >> index 7f77226c9b..44344ac8c1 100644 >> --- a/xen/arch/arm/include/asm/pci.h >> +++ b/xen/arch/arm/include/asm/pci.h >> @@ -66,6 +66,7 @@ struct pci_host_bridge { >> uint16_t segment; /* Segment number */ >> struct pci_config_window* cfg; /* Pointer to the bridge config >> window */ >> const struct pci_ops *ops; >> + void *priv; /* Private data of the bridge. */ >> }; >> struct pci_ops { >> @@ -95,7 +96,8 @@ struct pci_ecam_ops { >> extern const struct pci_ecam_ops pci_generic_ecam_ops; >> int pci_host_common_probe(struct dt_device_node *dev, >> - const struct pci_ecam_ops *ops); >> + const struct pci_ecam_ops *ops, >> + size_t priv_sz); >> int pci_generic_config_read(struct pci_host_bridge *bridge, >> pci_sbdf_t sbdf, >> uint32_t reg, uint32_t len, uint32_t >> *value); >> int pci_generic_config_write(struct pci_host_bridge *bridge, >> pci_sbdf_t sbdf, >> diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/ >> pci-host-common.c >> index c0faf0f436..be7e6c3510 100644 >> --- a/xen/arch/arm/pci/pci-host-common.c >> +++ b/xen/arch/arm/pci/pci-host-common.c >> @@ -209,7 +209,8 @@ static int pci_bus_find_domain_nr(struct >> dt_device_node *dev) >> } >> int pci_host_common_probe(struct dt_device_node *dev, >> - const struct pci_ecam_ops *ops) >> + const struct pci_ecam_ops *ops, >> + size_t priv_sz) >> { >> struct pci_host_bridge *bridge; >> struct pci_config_window *cfg; >> @@ -241,11 +242,26 @@ int pci_host_common_probe(struct dt_device_node >> *dev, >> printk(XENLOG_ERR "Inconsistent \"linux,pci-domain\" >> property in DT\n"); >> BUG(); >> } >> + >> bridge->segment = domain; >> + >> + if ( priv_sz ) >> + { >> + bridge->priv = xzalloc_bytes(priv_sz); >> + if ( !bridge->priv ) >> + { >> + err = -ENOMEM; >> + goto err_priv; >> + } >> + } >> + > > I'd like to propose to move allocation into pci_alloc_host_bridge() > to keep mallocs in one place and do it earlier, before proceeding > with other initialization steps. In both cases, we need to pass the size of the structure in parameter and the structure is not initialized until later. I would rather prefer if the allocation is done by each PCI controller that needs it close to the initialization of "priv". Cheers,
diff --git a/xen/arch/arm/include/asm/pci.h b/xen/arch/arm/include/asm/pci.h index 7f77226c9b..44344ac8c1 100644 --- a/xen/arch/arm/include/asm/pci.h +++ b/xen/arch/arm/include/asm/pci.h @@ -66,6 +66,7 @@ struct pci_host_bridge { uint16_t segment; /* Segment number */ struct pci_config_window* cfg; /* Pointer to the bridge config window */ const struct pci_ops *ops; + void *priv; /* Private data of the bridge. */ }; struct pci_ops { @@ -95,7 +96,8 @@ struct pci_ecam_ops { extern const struct pci_ecam_ops pci_generic_ecam_ops; int pci_host_common_probe(struct dt_device_node *dev, - const struct pci_ecam_ops *ops); + const struct pci_ecam_ops *ops, + size_t priv_sz); int pci_generic_config_read(struct pci_host_bridge *bridge, pci_sbdf_t sbdf, uint32_t reg, uint32_t len, uint32_t *value); int pci_generic_config_write(struct pci_host_bridge *bridge, pci_sbdf_t sbdf, diff --git a/xen/arch/arm/pci/pci-host-common.c b/xen/arch/arm/pci/pci-host-common.c index c0faf0f436..be7e6c3510 100644 --- a/xen/arch/arm/pci/pci-host-common.c +++ b/xen/arch/arm/pci/pci-host-common.c @@ -209,7 +209,8 @@ static int pci_bus_find_domain_nr(struct dt_device_node *dev) } int pci_host_common_probe(struct dt_device_node *dev, - const struct pci_ecam_ops *ops) + const struct pci_ecam_ops *ops, + size_t priv_sz) { struct pci_host_bridge *bridge; struct pci_config_window *cfg; @@ -241,11 +242,26 @@ int pci_host_common_probe(struct dt_device_node *dev, printk(XENLOG_ERR "Inconsistent \"linux,pci-domain\" property in DT\n"); BUG(); } + bridge->segment = domain; + + if ( priv_sz ) + { + bridge->priv = xzalloc_bytes(priv_sz); + if ( !bridge->priv ) + { + err = -ENOMEM; + goto err_priv; + } + } + pci_add_host_bridge(bridge); return 0; +err_priv: + xfree(bridge->priv); + err_exit: xfree(bridge); diff --git a/xen/arch/arm/pci/pci-host-generic.c b/xen/arch/arm/pci/pci-host-generic.c index 46de6e43cc..cc4bc70684 100644 --- a/xen/arch/arm/pci/pci-host-generic.c +++ b/xen/arch/arm/pci/pci-host-generic.c @@ -29,7 +29,7 @@ static const struct dt_device_match __initconstrel gen_pci_dt_match[] = static int __init pci_host_generic_probe(struct dt_device_node *dev, const void *data) { - return pci_host_common_probe(dev, &pci_generic_ecam_ops); + return pci_host_common_probe(dev, &pci_generic_ecam_ops, 0); } DT_DEVICE_START(pci_gen, "PCI HOST GENERIC", DEVICE_PCI_HOSTBRIDGE) diff --git a/xen/arch/arm/pci/pci-host-zynqmp.c b/xen/arch/arm/pci/pci-host-zynqmp.c index 101edb8593..985a43c516 100644 --- a/xen/arch/arm/pci/pci-host-zynqmp.c +++ b/xen/arch/arm/pci/pci-host-zynqmp.c @@ -47,7 +47,7 @@ static const struct dt_device_match __initconstrel nwl_pcie_dt_match[] = static int __init pci_host_generic_probe(struct dt_device_node *dev, const void *data) { - return pci_host_common_probe(dev, &nwl_pcie_ops); + return pci_host_common_probe(dev, &nwl_pcie_ops, 0); } DT_DEVICE_START(pci_gen, "PCI HOST ZYNQMP", DEVICE_PCI_HOSTBRIDGE)