Re: [PATCH v3 03/14] PCI: rcar: Convert to MSI domains

2021-04-01 Thread Lorenzo Pieralisi
On Thu, Apr 01, 2021 at 11:38:19AM +0100, Marc Zyngier wrote:
> On Thu, 01 Apr 2021 11:19:57 +0100,
> Lorenzo Pieralisi  wrote:
> > 
> > On Tue, Mar 30, 2021 at 04:11:34PM +0100, Marc Zyngier wrote:
> > 
> > [...]
> > 
> > > +static void rcar_compose_msi_msg(struct irq_data *data, struct msi_msg 
> > > *msg)
> > > +{
> > > + struct rcar_msi *msi = irq_data_get_irq_chip_data(data);
> > > + unsigned long pa = virt_to_phys(msi);
> > >  
> > > - hwirq = rcar_msi_alloc_region(msi, nvec);
> > > - if (hwirq < 0)
> > > - return -ENOSPC;
> > > + /* Use the msi structure as the PA for the MSI doorbell */
> > > + msg->address_lo = lower_32_bits(pa);
> > > + msg->address_hi = upper_32_bits(pa);
> > 
> > I don't think this change is aligned with the previous patch (is it ?),
> > the PA address we are using here is different from the one programmed
> > into the controller registers - either that or I am missing something,
> > please let me know.
> 
> Err. You are right. This looks like a bad case of broken conflict
> resolution on my part.
> 
> The following snippet should fix it. Let me know if you want me to
> resend the whole thing or whether you are OK with applying this by
> hand.

I will apply it and merge the whole series into -next, thanks for
implementing it !

Lorenzo

> Thanks,
> 
>   M.
> 
> diff --git a/drivers/pci/controller/pcie-rcar-host.c 
> b/drivers/pci/controller/pcie-rcar-host.c
> index f7331ad0d6dc..765cf2b45e24 100644
> --- a/drivers/pci/controller/pcie-rcar-host.c
> +++ b/drivers/pci/controller/pcie-rcar-host.c
> @@ -573,11 +573,10 @@ static int rcar_msi_set_affinity(struct irq_data *d, 
> const struct cpumask *mask,
>  static void rcar_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>  {
>   struct rcar_msi *msi = irq_data_get_irq_chip_data(data);
> - unsigned long pa = virt_to_phys(msi);
> + struct rcar_pcie *pcie = _to_host(msi)->pcie;
>  
> - /* Use the msi structure as the PA for the MSI doorbell */
> - msg->address_lo = lower_32_bits(pa);
> - msg->address_hi = upper_32_bits(pa);
> + msg->address_lo = rcar_pci_read_reg(pcie, PCIEMSIALR) & ~MSIFE;
> + msg->address_hi = rcar_pci_read_reg(pcie, PCIEMSIAUR);
>   msg->data = data->hwirq;
>  }
>  
> 
> -- 
> Without deviation from the norm, progress is not possible.


Re: [PATCH v3 03/14] PCI: rcar: Convert to MSI domains

2021-04-01 Thread Marc Zyngier
On Thu, 01 Apr 2021 11:19:57 +0100,
Lorenzo Pieralisi  wrote:
> 
> On Tue, Mar 30, 2021 at 04:11:34PM +0100, Marc Zyngier wrote:
> 
> [...]
> 
> > +static void rcar_compose_msi_msg(struct irq_data *data, struct msi_msg 
> > *msg)
> > +{
> > +   struct rcar_msi *msi = irq_data_get_irq_chip_data(data);
> > +   unsigned long pa = virt_to_phys(msi);
> >  
> > -   hwirq = rcar_msi_alloc_region(msi, nvec);
> > -   if (hwirq < 0)
> > -   return -ENOSPC;
> > +   /* Use the msi structure as the PA for the MSI doorbell */
> > +   msg->address_lo = lower_32_bits(pa);
> > +   msg->address_hi = upper_32_bits(pa);
> 
> I don't think this change is aligned with the previous patch (is it ?),
> the PA address we are using here is different from the one programmed
> into the controller registers - either that or I am missing something,
> please let me know.

Err. You are right. This looks like a bad case of broken conflict
resolution on my part.

The following snippet should fix it. Let me know if you want me to
resend the whole thing or whether you are OK with applying this by
hand.

Thanks,

M.

diff --git a/drivers/pci/controller/pcie-rcar-host.c 
b/drivers/pci/controller/pcie-rcar-host.c
index f7331ad0d6dc..765cf2b45e24 100644
--- a/drivers/pci/controller/pcie-rcar-host.c
+++ b/drivers/pci/controller/pcie-rcar-host.c
@@ -573,11 +573,10 @@ static int rcar_msi_set_affinity(struct irq_data *d, 
const struct cpumask *mask,
 static void rcar_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 {
struct rcar_msi *msi = irq_data_get_irq_chip_data(data);
-   unsigned long pa = virt_to_phys(msi);
+   struct rcar_pcie *pcie = _to_host(msi)->pcie;
 
-   /* Use the msi structure as the PA for the MSI doorbell */
-   msg->address_lo = lower_32_bits(pa);
-   msg->address_hi = upper_32_bits(pa);
+   msg->address_lo = rcar_pci_read_reg(pcie, PCIEMSIALR) & ~MSIFE;
+   msg->address_hi = rcar_pci_read_reg(pcie, PCIEMSIAUR);
msg->data = data->hwirq;
 }
 

-- 
Without deviation from the norm, progress is not possible.


Re: [PATCH v3 03/14] PCI: rcar: Convert to MSI domains

2021-04-01 Thread Lorenzo Pieralisi
On Tue, Mar 30, 2021 at 04:11:34PM +0100, Marc Zyngier wrote:

[...]

> +static void rcar_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> +{
> + struct rcar_msi *msi = irq_data_get_irq_chip_data(data);
> + unsigned long pa = virt_to_phys(msi);
>  
> - hwirq = rcar_msi_alloc_region(msi, nvec);
> - if (hwirq < 0)
> - return -ENOSPC;
> + /* Use the msi structure as the PA for the MSI doorbell */
> + msg->address_lo = lower_32_bits(pa);
> + msg->address_hi = upper_32_bits(pa);

I don't think this change is aligned with the previous patch (is it ?),
the PA address we are using here is different from the one programmed
into the controller registers - either that or I am missing something,
please let me know.

Thanks,
Lorenzo

> + msg->data = data->hwirq;
> +}
>  
> - irq = irq_find_mapping(msi->domain, hwirq);
> - if (!irq)
> - return -ENOSPC;
> +static struct irq_chip rcar_msi_bottom_chip = {
> + .name   = "Rcar MSI",
> + .irq_ack= rcar_msi_irq_ack,
> + .irq_mask   = rcar_msi_irq_mask,
> + .irq_unmask = rcar_msi_irq_unmask,
> + .irq_set_affinity   = rcar_msi_set_affinity,
> + .irq_compose_msi_msg= rcar_compose_msi_msg,
> +};
>  
> - for (i = 0; i < nvec; i++) {
> - /*
> -  * irq_create_mapping() called from rcar_pcie_probe() pre-
> -  * allocates descs,  so there is no need to allocate descs here.
> -  * We can therefore assume that if irq_find_mapping() above
> -  * returns non-zero, then the descs are also successfully
> -  * allocated.
> -  */
> - if (irq_set_msi_desc_off(irq, i, desc)) {
> - /* TODO: clear */
> - return -EINVAL;
> - }
> - }
> +static int rcar_msi_domain_alloc(struct irq_domain *domain, unsigned int 
> virq,
> +   unsigned int nr_irqs, void *args)
> +{
> + struct rcar_msi *msi = domain->host_data;
> + unsigned int i;
> + int hwirq;
> +
> + mutex_lock(>map_lock);
>  
> - desc->nvec_used = nvec;
> - desc->msi_attrib.multiple = order_base_2(nvec);
> + hwirq = bitmap_find_free_region(msi->used, INT_PCI_MSI_NR, 
> order_base_2(nr_irqs));
>  
> - msg.address_lo = rcar_pci_read_reg(pcie, PCIEMSIALR) & ~MSIFE;
> - msg.address_hi = rcar_pci_read_reg(pcie, PCIEMSIAUR);
> - msg.data = hwirq;
> + mutex_unlock(>map_lock);
> +
> + if (hwirq < 0)
> + return -ENOSPC;
>  
> - pci_write_msi_msg(irq, );
> + for (i = 0; i < nr_irqs; i++)
> + irq_domain_set_info(domain, virq + i, hwirq + i,
> + _msi_bottom_chip, domain->host_data,
> + handle_edge_irq, NULL, NULL);
>  
>   return 0;
>  }
>  
> -static void rcar_msi_teardown_irq(struct msi_controller *chip, unsigned int 
> irq)
> +static void rcar_msi_domain_free(struct irq_domain *domain, unsigned int 
> virq,
> +   unsigned int nr_irqs)
>  {
> - struct rcar_msi *msi = to_rcar_msi(chip);
> - struct irq_data *d = irq_get_irq_data(irq);
> + struct irq_data *d = irq_domain_get_irq_data(domain, virq);
> + struct rcar_msi *msi = domain->host_data;
>  
> - rcar_msi_free(msi, d->hwirq);
> -}
> -
> -static struct irq_chip rcar_msi_irq_chip = {
> - .name = "R-Car PCIe MSI",
> - .irq_enable = pci_msi_unmask_irq,
> - .irq_disable = pci_msi_mask_irq,
> - .irq_mask = pci_msi_mask_irq,
> - .irq_unmask = pci_msi_unmask_irq,
> -};
> + mutex_lock(>map_lock);
>  
> -static int rcar_msi_map(struct irq_domain *domain, unsigned int irq,
> - irq_hw_number_t hwirq)
> -{
> - irq_set_chip_and_handler(irq, _msi_irq_chip, handle_simple_irq);
> - irq_set_chip_data(irq, domain->host_data);
> + bitmap_release_region(msi->used, d->hwirq, order_base_2(nr_irqs));
>  
> - return 0;
> + mutex_unlock(>map_lock);
>  }
>  
> -static const struct irq_domain_ops msi_domain_ops = {
> - .map = rcar_msi_map,
> +static const struct irq_domain_ops rcar_msi_domain_ops = {
> + .alloc  = rcar_msi_domain_alloc,
> + .free   = rcar_msi_domain_free,
> +};
> +
> +static struct msi_domain_info rcar_msi_info = {
> + .flags  = (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> +MSI_FLAG_MULTI_PCI_MSI),
> + .chip   = _msi_top_chip,
>  };
>  
> -static void rcar_pcie_unmap_msi(struct rcar_pcie_host *host)
> +static int rcar_allocate_domains(struct rcar_msi *msi)
>  {
> - struct rcar_msi *msi = >msi;
> - int i, irq;
> + struct rcar_pcie *pcie = _to_host(msi)->pcie;
> + struct fwnode_handle *fwnode = dev_fwnode(pcie->dev);
> + struct irq_domain *parent;
> +
> + parent = irq_domain_create_linear(fwnode, INT_PCI_MSI_NR,
> + 

[PATCH v3 03/14] PCI: rcar: Convert to MSI domains

2021-03-30 Thread Marc Zyngier
In anticipation of the removal of the msi_controller structure, convert
the Rcar host controller driver to MSI domains.

We end-up with the usual two domain structure, the top one being a
generic PCI/MSI domain, the bottom one being Rcar-specific and handling
the actual HW interrupt allocation.

Acked-by: Bjorn Helgaas 
Tested-by: Marek Vasut 
Signed-off-by: Marc Zyngier 
---
 drivers/pci/controller/Kconfig  |   1 -
 drivers/pci/controller/pcie-rcar-host.c | 352 
 2 files changed, 170 insertions(+), 183 deletions(-)

diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index be8f9ff512a0..5cc07d28a3a0 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -58,7 +58,6 @@ config PCIE_RCAR_HOST
bool "Renesas R-Car PCIe host controller"
depends on ARCH_RENESAS || COMPILE_TEST
depends on PCI_MSI_IRQ_DOMAIN
-   select PCI_MSI_ARCH_FALLBACKS
help
  Say Y here if you want PCIe controller support on R-Car SoCs in host
  mode.
diff --git a/drivers/pci/controller/pcie-rcar-host.c 
b/drivers/pci/controller/pcie-rcar-host.c
index ce952403e22c..f7331ad0d6dc 100644
--- a/drivers/pci/controller/pcie-rcar-host.c
+++ b/drivers/pci/controller/pcie-rcar-host.c
@@ -35,17 +35,12 @@
 struct rcar_msi {
DECLARE_BITMAP(used, INT_PCI_MSI_NR);
struct irq_domain *domain;
-   struct msi_controller chip;
-   struct mutex lock;
+   struct mutex map_lock;
+   spinlock_t mask_lock;
int irq1;
int irq2;
 };
 
-static inline struct rcar_msi *to_rcar_msi(struct msi_controller *chip)
-{
-   return container_of(chip, struct rcar_msi, chip);
-}
-
 /* Structure representing the PCIe interface */
 struct rcar_pcie_host {
struct rcar_pciepcie;
@@ -55,6 +50,11 @@ struct rcar_pcie_host {
int (*phy_init_fn)(struct rcar_pcie_host *host);
 };
 
+static struct rcar_pcie_host *msi_to_host(struct rcar_msi *msi)
+{
+   return container_of(msi, struct rcar_pcie_host, msi);
+}
+
 static u32 rcar_read_conf(struct rcar_pcie *pcie, int where)
 {
unsigned int shift = BITS_PER_BYTE * (where & 3);
@@ -291,8 +291,6 @@ static int rcar_pcie_enable(struct rcar_pcie_host *host)
 
bridge->sysdata = host;
bridge->ops = _pcie_ops;
-   if (IS_ENABLED(CONFIG_PCI_MSI))
-   bridge->msi = >msi.chip;
 
return pci_host_probe(bridge);
 }
@@ -472,42 +470,6 @@ static int rcar_pcie_phy_init_gen3(struct rcar_pcie_host 
*host)
return err;
 }
 
-static int rcar_msi_alloc(struct rcar_msi *chip)
-{
-   int msi;
-
-   mutex_lock(>lock);
-
-   msi = find_first_zero_bit(chip->used, INT_PCI_MSI_NR);
-   if (msi < INT_PCI_MSI_NR)
-   set_bit(msi, chip->used);
-   else
-   msi = -ENOSPC;
-
-   mutex_unlock(>lock);
-
-   return msi;
-}
-
-static int rcar_msi_alloc_region(struct rcar_msi *chip, int no_irqs)
-{
-   int msi;
-
-   mutex_lock(>lock);
-   msi = bitmap_find_free_region(chip->used, INT_PCI_MSI_NR,
- order_base_2(no_irqs));
-   mutex_unlock(>lock);
-
-   return msi;
-}
-
-static void rcar_msi_free(struct rcar_msi *chip, unsigned long irq)
-{
-   mutex_lock(>lock);
-   clear_bit(irq, chip->used);
-   mutex_unlock(>lock);
-}
-
 static irqreturn_t rcar_pcie_msi_irq(int irq, void *data)
 {
struct rcar_pcie_host *host = data;
@@ -526,18 +488,13 @@ static irqreturn_t rcar_pcie_msi_irq(int irq, void *data)
unsigned int index = find_first_bit(, 32);
unsigned int msi_irq;
 
-   /* clear the interrupt */
-   rcar_pci_write_reg(pcie, 1 << index, PCIEMSIFR);
-
-   msi_irq = irq_find_mapping(msi->domain, index);
+   msi_irq = irq_find_mapping(msi->domain->parent, index);
if (msi_irq) {
-   if (test_bit(index, msi->used))
-   generic_handle_irq(msi_irq);
-   else
-   dev_info(dev, "unhandled MSI\n");
+   generic_handle_irq(msi_irq);
} else {
/* Unknown MSI, just clear it */
dev_dbg(dev, "unexpected MSI\n");
+   rcar_pci_write_reg(pcie, BIT(index), PCIEMSIFR);
}
 
/* see if there's any more pending in this vector */
@@ -547,150 +504,170 @@ static irqreturn_t rcar_pcie_msi_irq(int irq, void 
*data)
return IRQ_HANDLED;
 }
 
-static int rcar_msi_setup_irq(struct msi_controller *chip, struct pci_dev 
*pdev,
- struct msi_desc *desc)
+static void rcar_msi_top_irq_ack(struct irq_data *d)
 {
-   struct rcar_msi *msi = to_rcar_msi(chip);
-   struct rcar_pcie_host *host = container_of(chip, struct rcar_pcie_host,
-