mbox series

[v1,0/4] Enable rk356x PCIe controller

Message ID 20220412185751.124783-1-pgwipeout@gmail.com
Headers show
Series Enable rk356x PCIe controller | expand

Message

Peter Geis April 12, 2022, 6:57 p.m. UTC 
This series enables the DesignWare based PCIe controller on the rk356x
series of chips.
We drop the fallback to the core driver due to compatibility issues.
We add support for legacy interrupts for cards that lack MSI support
(which is partially broken currently).
We then add the device tree nodes to enable PCIe on the Quartz64 Model
A.

Patch 1 drops the snps,dw,pcie fallback from the dt-binding
Patch 2 adds legacy interrupt support to the driver
Patch 3 adds the device tree binding to the rk356x.dtsi
Patch 4 enables the PCIe controller on the Quartz64-A

Peter Geis (4):
  dt-bindings: pci: remove fallback from Rockchip DesignWare binding
  PCI: dwc: rockchip: add legacy interrupt support
  arm64: dts: rockchip: add rk3568 pcie2x1 controller
  arm64: dts: rockchip: enable pcie controller on quartz64-a

 .../bindings/pci/rockchip-dw-pcie.yaml        |  3 +-
 .../boot/dts/rockchip/rk3566-quartz64-a.dts   | 34 +++++++
 arch/arm64/boot/dts/rockchip/rk356x.dtsi      | 68 +++++++++++++-
 drivers/pci/controller/dwc/pcie-dw-rockchip.c | 92 ++++++++++++++++++-
 4 files changed, 189 insertions(+), 8 deletions(-)

Comments

Shawn Lin April 13, 2022, 2:54 a.m. UTC | #1 
Hi Peter,

在 2022/4/13 2:57, Peter Geis 写道:
> The legacy interrupts on the rk356x pcie controller are handled by a
> single muxed interrupt. Add irq domain support to the pcie-dw-rockchip >
> +static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
> +{
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	struct rockchip_pcie *rockchip = irq_desc_get_handler_data(desc);
> +	struct device *dev = rockchip->pci.dev;
> +	u32 reg;
> +	u32 hwirq;
> +	u32 virq;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	reg = rockchip_pcie_readl_apb(rockchip, 0x8);
> +


Overall it looks good except that 0x8 has a name,
PCIE_CLIENT_INTR_STATUS_LEGACY. BTW, If you consider adding more support
to it, for instance, enable/disable/affinity?  The downstream kernel
finished these for better fitting for function driver usage.
Peter Geis April 13, 2022, 12:09 p.m. UTC | #2 
On Tue, Apr 12, 2022 at 10:54 PM Shawn Lin <shawn.lin@rock-chips.com> wrote:
>
> Hi Peter,
>
> 在 2022/4/13 2:57, Peter Geis 写道:
> > The legacy interrupts on the rk356x pcie controller are handled by a
> > single muxed interrupt. Add irq domain support to the pcie-dw-rockchip >
> > +static void rockchip_pcie_legacy_int_handler(struct irq_desc *desc)
> > +{
> > +     struct irq_chip *chip = irq_desc_get_chip(desc);
> > +     struct rockchip_pcie *rockchip = irq_desc_get_handler_data(desc);
> > +     struct device *dev = rockchip->pci.dev;
> > +     u32 reg;
> > +     u32 hwirq;
> > +     u32 virq;
> > +
> > +     chained_irq_enter(chip, desc);
> > +
> > +     reg = rockchip_pcie_readl_apb(rockchip, 0x8);
> > +
>
>
> Overall it looks good except that 0x8 has a name,
> PCIE_CLIENT_INTR_STATUS_LEGACY. BTW, If you consider adding more support
> to it, for instance, enable/disable/affinity?  The downstream kernel
> finished these for better fitting for function driver usage.

Good catch, thanks.

This patch has remained largely unchanged from when I first created it
prior to asking for Rockchip to include support for it in the initial
series.
I would have left it out based on the original counter arguments,
except in testing we have discovered two issues:
A surprising number of cards do not support MSIs.
The current MSI implementation has poor compatibility.

I will look at the downstream implementation and consider possible
changes, but for the time being this does the job.