On Mon, Jul 15, 2024 at 11:13:30AM -0700, Mayank Rana wrote:
> PCIe ECAM driver do not have dw_pcie data structure populated and DBI
> access related APIs. Hence add msi_ops as part of dw_msi structure to
> allow populating DBI based MSI register access.
>
> Signed-off-by: Mayank Rana <quic_mrana@xxxxxxxxxxx>
> ---
> drivers/pci/controller/dwc/pcie-designware-host.c | 20 ++++++++++++-
> drivers/pci/controller/dwc/pcie-designware-msi.c | 36 +++++++++++++----------
> drivers/pci/controller/dwc/pcie-designware-msi.h | 10 +++++--
> 3 files changed, 47 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 3dcf88a..7a1eb1f 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -47,6 +47,16 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
> }
> }
>
> +static u32 dw_pcie_readl_msi_dbi(void *pci, u32 reg)
> +{
> + return dw_pcie_readl_dbi((struct dw_pcie *)pci, reg);
> +}
> +
> +static void dw_pcie_writel_msi_dbi(void *pci, u32 reg, u32 val)
> +{
> + dw_pcie_writel_dbi((struct dw_pcie *)pci, reg, val);
> +}
> +
There is nothing MSI specific in dw_pcie_{writel/readl}_msi_dbi(). This just
writes/reads the registers. So this should be called as dw_pcie_write_reg()/
dw_pcie_write_reg().
> int dw_pcie_host_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -55,6 +65,7 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> struct platform_device *pdev = to_platform_device(dev);
> struct resource_entry *win;
> struct pci_host_bridge *bridge;
> + struct dw_msi_ops *msi_ops;
> struct resource *res;
> bool has_msi_ctrl;
> int ret;
> @@ -124,7 +135,14 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
> if (ret < 0)
> goto err_deinit_host;
> } else if (has_msi_ctrl) {
> - pp->msi = dw_pcie_msi_host_init(pdev, pp, pp->num_vectors);
> + msi_ops = devm_kzalloc(dev, sizeof(*msi_ops), GFP_KERNEL);
> + if (msi_ops == NULL)
> + goto err_deinit_host;
> +
> + msi_ops->pp = pci;
Stuffing private data inside ops structure looks weird. Also the fact that
allocating memory for ops...
> + msi_ops->readl_msi = dw_pcie_readl_msi_dbi,
> + msi_ops->writel_msi = dw_pcie_writel_msi_dbi,
Same for the callback name.
> + pp->msi = dw_pcie_msi_host_init(pdev, msi_ops, pp->num_vectors);
> if (IS_ERR(pp->msi)) {
> ret = PTR_ERR(pp->msi);
> goto err_deinit_host;
> diff --git a/drivers/pci/controller/dwc/pcie-designware-msi.c b/drivers/pci/controller/dwc/pcie-designware-msi.c
> index 39fe5be..dbfffce 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-msi.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-msi.c
> @@ -44,6 +44,16 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {
> .chip = &dw_pcie_msi_irq_chip,
> };
>
> +static u32 dw_msi_readl(struct dw_msi *msi, u32 reg)
> +{
> + return msi->msi_ops->readl_msi(msi->msi_ops->pp, reg);
> +}
> +
> +static void dw_msi_writel(struct dw_msi *msi, u32 reg, u32 val)
> +{
> + msi->msi_ops->writel_msi(msi->msi_ops->pp, reg, val);
> +}
> +
These could be:
dw_msi_read_reg()
dw_msi_write_reg()
- Mani
> /* MSI int handler */
> irqreturn_t dw_handle_msi_irq(struct dw_msi *msi)
> {
> @@ -51,13 +61,11 @@ irqreturn_t dw_handle_msi_irq(struct dw_msi *msi)
> unsigned long val;
> u32 status, num_ctrls;
> irqreturn_t ret = IRQ_NONE;
> - struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
>
> num_ctrls = msi->num_vectors / MAX_MSI_IRQS_PER_CTRL;
>
> for (i = 0; i < num_ctrls; i++) {
> - status = dw_pcie_readl_dbi(pci, PCIE_MSI_INTR0_STATUS +
> - (i * MSI_REG_CTRL_BLOCK_SIZE));
> + status = dw_msi_readl(msi, PCIE_MSI_INTR0_STATUS + (i * MSI_REG_CTRL_BLOCK_SIZE));
> if (!status)
> continue;
>
> @@ -115,7 +123,6 @@ static int dw_pci_msi_set_affinity(struct irq_data *d,
> static void dw_pci_bottom_mask(struct irq_data *d)
> {
> struct dw_msi *msi = irq_data_get_irq_chip_data(d);
> - struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
> unsigned int res, bit, ctrl;
> unsigned long flags;
>
> @@ -126,7 +133,7 @@ static void dw_pci_bottom_mask(struct irq_data *d)
> bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>
> msi->irq_mask[ctrl] |= BIT(bit);
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
> + dw_msi_writel(msi, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
>
> raw_spin_unlock_irqrestore(&msi->lock, flags);
> }
> @@ -134,7 +141,6 @@ static void dw_pci_bottom_mask(struct irq_data *d)
> static void dw_pci_bottom_unmask(struct irq_data *d)
> {
> struct dw_msi *msi = irq_data_get_irq_chip_data(d);
> - struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
> unsigned int res, bit, ctrl;
> unsigned long flags;
>
> @@ -145,7 +151,7 @@ static void dw_pci_bottom_unmask(struct irq_data *d)
> bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>
> msi->irq_mask[ctrl] &= ~BIT(bit);
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
> + dw_msi_writel(msi, PCIE_MSI_INTR0_MASK + res, msi->irq_mask[ctrl]);
>
> raw_spin_unlock_irqrestore(&msi->lock, flags);
> }
> @@ -153,14 +159,13 @@ static void dw_pci_bottom_unmask(struct irq_data *d)
> static void dw_pci_bottom_ack(struct irq_data *d)
> {
> struct dw_msi *msi = irq_data_get_irq_chip_data(d);
> - struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
> unsigned int res, bit, ctrl;
>
> ctrl = d->hwirq / MAX_MSI_IRQS_PER_CTRL;
> res = ctrl * MSI_REG_CTRL_BLOCK_SIZE;
> bit = d->hwirq % MAX_MSI_IRQS_PER_CTRL;
>
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_STATUS + res, BIT(bit));
> + dw_msi_writel(msi, PCIE_MSI_INTR0_STATUS + res, BIT(bit));
> }
>
> static struct irq_chip dw_pci_msi_bottom_irq_chip = {
> @@ -262,7 +267,6 @@ void dw_pcie_free_msi(struct dw_msi *msi)
>
> void dw_pcie_msi_init(struct dw_msi *msi)
> {
> - struct dw_pcie *pci = to_dw_pcie_from_pp(msi->pp);
> u32 ctrl, num_ctrls;
> u64 msi_target;
>
> @@ -273,16 +277,16 @@ void dw_pcie_msi_init(struct dw_msi *msi)
> num_ctrls = msi->num_vectors / MAX_MSI_IRQS_PER_CTRL;
> /* Initialize IRQ Status array */
> for (ctrl = 0; ctrl < num_ctrls; ctrl++) {
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK +
> + dw_msi_writel(msi, PCIE_MSI_INTR0_MASK +
> (ctrl * MSI_REG_CTRL_BLOCK_SIZE),
> msi->irq_mask[ctrl]);
> - dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_ENABLE +
> + dw_msi_writel(msi, PCIE_MSI_INTR0_ENABLE +
> (ctrl * MSI_REG_CTRL_BLOCK_SIZE), ~0);
> }
>
> /* Program the msi_data */
> - dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
> - dw_pcie_writel_dbi(pci, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
> + dw_msi_writel(msi, PCIE_MSI_ADDR_LO, lower_32_bits(msi_target));
> + dw_msi_writel(msi, PCIE_MSI_ADDR_HI, upper_32_bits(msi_target));
> }
>
> static int dw_pcie_parse_split_msi_irq(struct dw_msi *msi, struct platform_device *pdev)
> @@ -324,7 +328,7 @@ static int dw_pcie_parse_split_msi_irq(struct dw_msi *msi, struct platform_devic
> }
>
> struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
> - void *pp, u32 num_vectors)
> + struct dw_msi_ops *ops, u32 num_vectors)
> {
> struct device *dev = &pdev->dev;
> u64 *msi_vaddr = NULL;
> @@ -341,7 +345,7 @@ struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
>
> raw_spin_lock_init(&msi->lock);
> msi->dev = dev;
> - msi->pp = pp;
> + msi->msi_ops = ops;
> msi->has_msi_ctrl = true;
> msi->num_vectors = num_vectors;
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-msi.h b/drivers/pci/controller/dwc/pcie-designware-msi.h
> index 633156e..cf5c612 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-msi.h
> +++ b/drivers/pci/controller/dwc/pcie-designware-msi.h
> @@ -18,8 +18,15 @@
> #define MSI_REG_CTRL_BLOCK_SIZE 12
> #define MSI_DEF_NUM_VECTORS 32
>
> +struct dw_msi_ops {
> + void *pp;
> + u32 (*readl_msi)(void *pp, u32 reg);
> + void (*writel_msi)(void *pp, u32 reg, u32 val);
> +};
> +
> struct dw_msi {
> struct device *dev;
> + struct dw_msi_ops *msi_ops;
> struct irq_domain *irq_domain;
> struct irq_domain *msi_domain;
> struct irq_chip *msi_irq_chip;
> @@ -31,11 +38,10 @@ struct dw_msi {
> DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
> bool has_msi_ctrl;
> void *private_data;
> - void *pp;
> };
>
> struct dw_msi *dw_pcie_msi_host_init(struct platform_device *pdev,
> - void *pp, u32 num_vectors);
> + struct dw_msi_ops *ops, u32 num_vectors);
> int dw_pcie_allocate_domains(struct dw_msi *msi);
> void dw_pcie_msi_init(struct dw_msi *msi);
> void dw_pcie_free_msi(struct dw_msi *msi);
> --
> 2.7.4
>
--
மணிவண்ணன் சதாசிவம்
