| Message ID | 20221110195015.207-6-jonathan.derrick@linux.dev (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCIe Hotplug Slot Emulation driver | expand |
On Thursday 10 November 2022 12:50:13 Jonathan Derrick wrote: > Add a handler to set behavior of a PCI or PCIe register. Add the > appropriate enums to specify the register's Read-Only, Read-Write, and > Write-1-to-Clear behaviors. > > Signed-off-by: Jonathan Derrick <jonathan.derrick@linux.dev> I do not think that this is the correct way. Drivers should not need to tell bridge emulator that some register is read-only or read-write. Bridge emulator has already all required information in its internal structures. If there is a need to tell bridge emulator to "emulate" some optional feature, for example hot plug capabilities, then it would be better to extend pci_bridge_emul_init() flags and let init function to correctly fill all behavior bits. There is already PCI_BRIDGE_EMUL_NO_PREFMEM_FORWARD flag which modify bridge prefetchable bits. In my opinion, all standard PCI/PCIe behavior bits should be in pci-bridge-emul.c source file and drivers should not modify them. I think that this approach makes implementation lot of cleaner. > --- > drivers/pci/pci-bridge-emul.c | 19 +++++++++++++++++++ > drivers/pci/pci-bridge-emul.h | 10 ++++++++++ > 2 files changed, 29 insertions(+) > > diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c > index 9334b2dd4764..3c1a683ece66 100644 > --- a/drivers/pci/pci-bridge-emul.c > +++ b/drivers/pci/pci-bridge-emul.c > @@ -46,6 +46,25 @@ struct pci_bridge_reg_behavior { > u32 w1c; > }; > > +void pci_bridge_emul_set_reg_behavior(struct pci_bridge_emul *bridge, > + bool pcie, int reg, u32 val, > + enum pci_bridge_emul_reg_behavior type) > +{ > + struct pci_bridge_reg_behavior *behavior; > + > + if (pcie) > + behavior = &bridge->pcie_cap_regs_behavior[reg / 4]; > + else > + behavior = &bridge->pci_regs_behavior[reg / 4]; > + > + if (type == PCI_BRIDGE_EMUL_REG_BEHAVIOR_RO) > + behavior->ro = val; > + else if (type == PCI_BRIDGE_EMUL_REG_BEHAVIOR_RW) > + behavior->rw = val; > + else /* PCI_BRIDGE_EMUL_REG_BEHAVIOR_W1C */ > + behavior->w1c = val; > +} > + > static const > struct pci_bridge_reg_behavior pci_regs_behavior[PCI_STD_HEADER_SIZEOF / 4] = { > [PCI_VENDOR_ID / 4] = { .ro = ~0 }, > diff --git a/drivers/pci/pci-bridge-emul.h b/drivers/pci/pci-bridge-emul.h > index 2a0e59c7f0d9..b2401d58518c 100644 > --- a/drivers/pci/pci-bridge-emul.h > +++ b/drivers/pci/pci-bridge-emul.h > @@ -72,6 +72,12 @@ struct pci_bridge_emul; > typedef enum { PCI_BRIDGE_EMUL_HANDLED, > PCI_BRIDGE_EMUL_NOT_HANDLED } pci_bridge_emul_read_status_t; > > +enum pci_bridge_emul_reg_behavior { > + PCI_BRIDGE_EMUL_REG_BEHAVIOR_RO, > + PCI_BRIDGE_EMUL_REG_BEHAVIOR_RW, > + PCI_BRIDGE_EMUL_REG_BEHAVIOR_W1C, > +}; > + > struct pci_bridge_emul_ops { > /* > * Called when reading from the regular PCI bridge > @@ -161,4 +167,8 @@ int pci_bridge_emul_conf_read(struct pci_bridge_emul *bridge, int where, > int pci_bridge_emul_conf_write(struct pci_bridge_emul *bridge, int where, > int size, u32 value); > > +void pci_bridge_emul_set_reg_behavior(struct pci_bridge_emul *bridge, > + bool pcie, int reg, u32 val, > + enum pci_bridge_emul_reg_behavior type); > + > #endif /* __PCI_BRIDGE_EMUL_H__ */ > -- > 2.30.2 >
diff --git a/drivers/pci/pci-bridge-emul.c b/drivers/pci/pci-bridge-emul.c index 9334b2dd4764..3c1a683ece66 100644 --- a/drivers/pci/pci-bridge-emul.c +++ b/drivers/pci/pci-bridge-emul.c @@ -46,6 +46,25 @@ struct pci_bridge_reg_behavior { u32 w1c; }; +void pci_bridge_emul_set_reg_behavior(struct pci_bridge_emul *bridge, + bool pcie, int reg, u32 val, + enum pci_bridge_emul_reg_behavior type) +{ + struct pci_bridge_reg_behavior *behavior; + + if (pcie) + behavior = &bridge->pcie_cap_regs_behavior[reg / 4]; + else + behavior = &bridge->pci_regs_behavior[reg / 4]; + + if (type == PCI_BRIDGE_EMUL_REG_BEHAVIOR_RO) + behavior->ro = val; + else if (type == PCI_BRIDGE_EMUL_REG_BEHAVIOR_RW) + behavior->rw = val; + else /* PCI_BRIDGE_EMUL_REG_BEHAVIOR_W1C */ + behavior->w1c = val; +} + static const struct pci_bridge_reg_behavior pci_regs_behavior[PCI_STD_HEADER_SIZEOF / 4] = { [PCI_VENDOR_ID / 4] = { .ro = ~0 }, diff --git a/drivers/pci/pci-bridge-emul.h b/drivers/pci/pci-bridge-emul.h index 2a0e59c7f0d9..b2401d58518c 100644 --- a/drivers/pci/pci-bridge-emul.h +++ b/drivers/pci/pci-bridge-emul.h @@ -72,6 +72,12 @@ struct pci_bridge_emul; typedef enum { PCI_BRIDGE_EMUL_HANDLED, PCI_BRIDGE_EMUL_NOT_HANDLED } pci_bridge_emul_read_status_t; +enum pci_bridge_emul_reg_behavior { + PCI_BRIDGE_EMUL_REG_BEHAVIOR_RO, + PCI_BRIDGE_EMUL_REG_BEHAVIOR_RW, + PCI_BRIDGE_EMUL_REG_BEHAVIOR_W1C, +}; + struct pci_bridge_emul_ops { /* * Called when reading from the regular PCI bridge @@ -161,4 +167,8 @@ int pci_bridge_emul_conf_read(struct pci_bridge_emul *bridge, int where, int pci_bridge_emul_conf_write(struct pci_bridge_emul *bridge, int where, int size, u32 value); +void pci_bridge_emul_set_reg_behavior(struct pci_bridge_emul *bridge, + bool pcie, int reg, u32 val, + enum pci_bridge_emul_reg_behavior type); + #endif /* __PCI_BRIDGE_EMUL_H__ */
Add a handler to set behavior of a PCI or PCIe register. Add the appropriate enums to specify the register's Read-Only, Read-Write, and Write-1-to-Clear behaviors. Signed-off-by: Jonathan Derrick <jonathan.derrick@linux.dev> --- drivers/pci/pci-bridge-emul.c | 19 +++++++++++++++++++ drivers/pci/pci-bridge-emul.h | 10 ++++++++++ 2 files changed, 29 insertions(+)