Re: [PATCH] PCI: tegra: Restore MSI enable state on resume
On Tue, Apr 20, 2021 at 02:05:26PM +0100, Marc Zyngier wrote:
> When going into suspend, the Tegra MSI controller loses its
> state. Restore the MSI allocation on resume so that PCI devices
> are usable again.
>
> Reported-by: Jon Hunter
> Tested-by: Jon Hunter
> Fixes: 973a28677e39 ("PCI: tegra: Convert to MSI domains")
> Signed-off-by: Marc Zyngier
> Cc: Lorenzo Pieralisi
> Cc: Bjorn Helgaas
> Cc: Thierry Reding
> ---
> drivers/pci/controller/pci-tegra.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
Squashed with the Fixes: commit, updated my pci/msi branch with it
and pushed out.
Thanks,
Lorenzo
> diff --git a/drivers/pci/controller/pci-tegra.c
> b/drivers/pci/controller/pci-tegra.c
> index eaba7b2fab4a..507b23d43ad1 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -1802,13 +1802,19 @@ static void tegra_pcie_enable_msi(struct tegra_pcie
> *pcie)
> {
> const struct tegra_pcie_soc *soc = pcie->soc;
> struct tegra_msi *msi = >msi;
> - u32 reg;
> + u32 reg, msi_state[INT_PCI_MSI_NR / 32];
> + int i;
>
> afi_writel(pcie, msi->phys >> soc->msi_base_shift, AFI_MSI_FPCI_BAR_ST);
> afi_writel(pcie, msi->phys, AFI_MSI_AXI_BAR_ST);
> /* this register is in 4K increments */
> afi_writel(pcie, 1, AFI_MSI_BAR_SZ);
>
> + /* Restore the MSI allocation state */
> + bitmap_to_arr32(msi_state, msi->used, INT_PCI_MSI_NR);
> + for (i = 0; i < ARRAY_SIZE(msi_state); i++)
> + afi_writel(pcie, msi_state[i], AFI_MSI_EN_VEC(i));
> +
> /* and unmask the MSI interrupt */
> reg = afi_readl(pcie, AFI_INTR_MASK);
> reg |= AFI_INTR_MASK_MSI_MASK;
> --
> 2.30.2
>
Re: [PATCH v10 0/7] PCI: mediatek: Add new generation controller support
On Tue, 20 Apr 2021 14:17:16 +0800, Jianjun Wang wrote: > These series patches add pcie-mediatek-gen3.c and dt-bindings file to > support new generation PCIe controller. > > Changes in v10: > 1. Fix the subject line format in commit message; > 2. Use EXPORT_SYMBOL_GPL() to export pci_pio_to_address(). > > [...] Applied to pci/mediatek, thanks! [1/7] dt-bindings: PCI: mediatek-gen3: Add YAML schema https://git.kernel.org/lpieralisi/pci/c/07ca255e3d [2/7] PCI: Export pci_pio_to_address() for module use https://git.kernel.org/lpieralisi/pci/c/9cc742078c [3/7] PCI: mediatek-gen3: Add MediaTek Gen3 driver for MT8192 https://git.kernel.org/lpieralisi/pci/c/441903d9e8 [4/7] PCI: mediatek-gen3: Add INTx support https://git.kernel.org/lpieralisi/pci/c/c58148e657 [5/7] PCI: mediatek-gen3: Add MSI support https://git.kernel.org/lpieralisi/pci/c/e0282a61f4 [6/7] PCI: mediatek-gen3: Add system PM support https://git.kernel.org/lpieralisi/pci/c/a7583c42f4 [7/7] MAINTAINERS: Add Jianjun Wang as MediaTek PCI co-maintainer https://git.kernel.org/lpieralisi/pci/c/fcf132f196 Thanks, Lorenzo
Re: [v9,0/7] PCI: mediatek: Add new generation controller support
On Fri, Apr 16, 2021 at 02:21:00PM -0500, Bjorn Helgaas wrote: > On Wed, Mar 24, 2021 at 11:05:03AM +0800, Jianjun Wang wrote: > > These series patches add pcie-mediatek-gen3.c and dt-bindings file to > > support new generation PCIe controller. > > Incidental: b4 doesn't work on this thread, I suspect because the > usual subject line format is: > > [PATCH v9 9/7] > > instead of: > > [v9,0/7] > > For b4 info, see > https://git.kernel.org/pub/scm/utils/b4/b4.git/tree/README.rst Jianjun will update the series accordingly (and please add to v10 the review tags you received. Lorenzo
Re: [PATCH] PCI: dwc: remove unused function
On Thu, 15 Apr 2021 16:32:57 +0800, Jiapeng Chong wrote: > Fix the following clang warning: > > drivers/pci/controller/dwc/pcie-intel-gw.c:84:19: warning: unused > function 'pcie_app_rd' [-Wunused-function]. Applied to pci/dwc, thanks! [1/1] PCI: dwc/intel-gw: Remove unused function https://git.kernel.org/lpieralisi/pci/c/9534286be3 Thanks, Lorenzo
Re: [PATCH v3] PCI: dwc: move dw_pcie_msi_init() to dw_pcie_setup_rc()
On Thu, 25 Mar 2021 15:26:04 +0800, Jisheng Zhang wrote: > If the host which makes use of IP's integrated MSI Receiver losts > power during suspend, we need to reinit the RC and MSI Receiver in > resume. But after we move dw_pcie_msi_init() into the core, we have no > API to do so. Usually the dwc users need to call dw_pcie_setup_rc() to > reinit the RC, we can solve this problem by moving dw_pcie_msi_init() > to dw_pcie_setup_rc(). Applied to pci/dwc, thanks! [1/1] PCI: dwc: Move dw_pcie_msi_init() to dw_pcie_setup_rc() https://git.kernel.org/lpieralisi/pci/c/fe08db2835 Thanks, Lorenzo
Re: [PATCH -next] PCI: altera-msi: Remove redundant dev_err call in altera_msi_probe()
On Fri, 9 Apr 2021 15:57:48 +0800, Chen Hui wrote: > There is a error message within devm_ioremap_resource > already, so remove the dev_err call to avoid redundant > error message. Applied to pci/altera-msi, thanks! [1/1] PCI: altera-msi: Remove redundant dev_err call in altera_msi_probe() https://git.kernel.org/lpieralisi/pci/c/b1160a06e0 Thanks, Lorenzo
Re: [PATCH -next] PCI: endpoint: fix missing destroy_workqueue()
On Wed, 31 Mar 2021 16:40:12 +0800, Yang Yingliang wrote: > Add the missing destroy_workqueue() before return from > pci_epf_test_init() in the error handling case and add > destroy_workqueue() in pci_epf_test_exit(). Applied to pci/endpoint, thanks! [1/1] PCI: endpoint: Fix missing destroy_workqueue() https://git.kernel.org/lpieralisi/pci/c/acaef7981a Thanks, Lorenzo
Re: [PATCH] PCI: endpoint: remove redundant initialization of pointer dev
On Fri, 26 Mar 2021 19:09:09 +, Colin King wrote: > The pointer dev is being initialized with a value that is > never read and it is being updated later with a new value. The > initialization is redundant and can be removed. Applied to pci/endpoint, thanks! [1/1] PCI: endpoint: Remove redundant initialization of pointer dev https://git.kernel.org/lpieralisi/pci/c/80c253bd7f Thanks, Lorenzo
Re: [v9,2/7] PCI: Export pci_pio_to_address() for module use
On Wed, Mar 24, 2021 at 10:09:42AM +0100, Pali Rohár wrote:
> On Wednesday 24 March 2021 11:05:05 Jianjun Wang wrote:
> > This interface will be used by PCI host drivers for PIO translation,
> > export it to support compiling those drivers as kernel modules.
> >
> > Signed-off-by: Jianjun Wang
> > ---
> > drivers/pci/pci.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 16a17215f633..12bba221c9f2 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -4052,6 +4052,7 @@ phys_addr_t pci_pio_to_address(unsigned long pio)
> >
> > return address;
> > }
> > +EXPORT_SYMBOL(pci_pio_to_address);
>
> Hello! I'm not sure if EXPORT_SYMBOL is correct because file has GPL-2.0
> header. Should not be in this case used only EXPORT_SYMBOL_GPL? Maybe
> other people would know what is correct?
I think this should be EXPORT_SYMBOL_GPL(), I can make this change
but this requires Bjorn's ACK to go upstream (Bjorn, it is my fault,
it was assigned to me on patchwork, now updated, please have a look).
Thanks,
Lorenzo
> >
> > unsigned long __weak pci_address_to_pio(phys_addr_t address)
> > {
> > --
> > 2.25.1
> >
Re: [PATCH] PCI: dwc/intel-gw: Fix enabling the legacy PCI interrupt lines
On Fri, Apr 09, 2021 at 10:17:12AM +, Rahul Tanwar wrote:
> On 9/4/2021 4:40 am, Martin Blumenstingl wrote:
> > This email was sent from outside of MaxLinear.
> >
> > Hi Lorenzo,
> >
> > On Tue, Mar 23, 2021 at 12:36 PM Lorenzo Pieralisi
> > wrote:
> > >
> > > On Wed, Jan 06, 2021 at 02:55:40PM +0100, Martin Blumenstingl wrote:
> > > > The legacy PCI interrupt lines need to be enabled using PCIE_APP_IRNEN
> > > > bits 13 (INTA), 14 (INTB), 15 (INTC) and 16 (INTD). The old code
> > however
> > > > was taking (for example) "13" as raw value instead of taking BIT(13).
> > > > Define the legacy PCI interrupt bits using the BIT() macro and then use
> > > > these in PCIE_APP_IRN_INT.
> > > >
> > > > Fixes: ed22aaaede44 ("PCI: dwc: intel: PCIe RC controller driver")
> > > > Signed-off-by: Martin Blumenstingl
> > > > ---
> > > > drivers/pci/controller/dwc/pcie-intel-gw.c | 10 ++
> > > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c
> > b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > > index 0cedd1f95f37..ae96bfbb6c83 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> > > > @@ -39,6 +39,10 @@
> > > > #define PCIE_APP_IRN_PM_TO_ACK BIT(9)
> > > > #define PCIE_APP_IRN_LINK_AUTO_BW_STAT BIT(11)
> > > > #define PCIE_APP_IRN_BW_MGT BIT(12)
> > > > +#define PCIE_APP_IRN_INTA BIT(13)
> > > > +#define PCIE_APP_IRN_INTB BIT(14)
> > > > +#define PCIE_APP_IRN_INTC BIT(15)
> > > > +#define PCIE_APP_IRN_INTD BIT(16)
> > > > #define PCIE_APP_IRN_MSG_LTR BIT(18)
> > > > #define PCIE_APP_IRN_SYS_ERR_RC BIT(29)
> > > > #define PCIE_APP_INTX_OFST 12
> > > > @@ -48,10 +52,8 @@
> > > > PCIE_APP_IRN_RX_VDM_MSG | PCIE_APP_IRN_SYS_ERR_RC | \
> > > > PCIE_APP_IRN_PM_TO_ACK | PCIE_APP_IRN_MSG_LTR | \
> > > > PCIE_APP_IRN_BW_MGT | PCIE_APP_IRN_LINK_AUTO_BW_STAT | \
> > > > - (PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTA) | \
> > > > - (PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTB) | \
> > > > - (PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTC) | \
> > > > - (PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTD))
> > > > + PCIE_APP_IRN_INTA | PCIE_APP_IRN_INTB | \
> > > > + PCIE_APP_IRN_INTC | PCIE_APP_IRN_INTD)
> > > >
> > > > #define BUS_IATU_OFFSET SZ_256M
> > > > #define RESET_INTERVAL_MS 100
> > >
> > > This looks like a significant bug - which in turn raises the question
> > > on how well this driver has been tested.
> > to give them the benefit of doubt: maybe only MSIs were tested
> >
> > > Dilip, can you review and ACK asap please ?
> > From "Re: MaxLinear, please maintain your drivers was Re: [PATCH]
> > leds: lgm: fix gpiolib dependency" [0]:
> > > Please send any Lightning Mountain SoC related issues email to Rahul
> > > Tanwar (rtan...@maxlinear.com) and I will ensure that I address the
> > > issues in a timely manner.
> > so I added rtan...@maxlinear.com to this email
> >
> >
> > Best regards,
> > Martin
> >
> >
> > [0] https://lkml.org/lkml/2021/3/16/282
> > <https://lkml.org/lkml/2021/3/16/282>
>
>
> Dilip has left the org. So not sure how exactly he tested it (maybe only
> MSIs). But i have confirmed it to be a bug. Thanks Martin for fixing it.
Can you take on maintainership for this driver please ?
If yes please send a MAINTAINERS file patch.
Thanks,
Lorenzo
> Acked-by: Rahul Tanwar
>
> Regards,
> Rahul
>
>
>
>
>
Re: [PATCH v5 0/6] Add SiFive FU740 PCIe host controller driver support
On Tue, 6 Apr 2021 17:26:28 +0800, Greentime Hu wrote: > This patchset includes SiFive FU740 PCIe host controller driver. We also > add pcie_aux clock and pcie_power_on_reset controller to prci driver for > PCIe driver to use it. > > This is tested with e1000e: Intel(R) PRO/1000 Network Card, AMD Radeon R5 > 230 graphics card and SP M.2 PCIe Gen 3 SSD in SiFive Unmatched based on > v5.11 Linux kernel. > > [...] Applied to pci/dwc [dropped patch 6], thanks! [1/6] clk: sifive: Add pcie_aux clock in prci driver for PCIe driver https://git.kernel.org/lpieralisi/pci/c/f3ce593b1a [2/6] clk: sifive: Use reset-simple in prci driver for PCIe driver https://git.kernel.org/lpieralisi/pci/c/0a78fcfd3d [3/6] MAINTAINERS: Add maintainers for SiFive FU740 PCIe driver https://git.kernel.org/lpieralisi/pci/c/8bb1c66a90 [4/6] dt-bindings: PCI: Add SiFive FU740 PCIe host controller https://git.kernel.org/lpieralisi/pci/c/b86d55c107 [5/6] PCI: fu740: Add SiFive FU740 PCIe host controller driver https://git.kernel.org/lpieralisi/pci/c/327c333a79 Thanks, Lorenzo
Re: [PATCH] PCI: tegra: Fix runtime PM imbalance in pex_ep_event_pex_rst_deassert
On Thu, 8 Apr 2021 15:26:58 +0800, Dinghao Liu wrote: > pm_runtime_get_sync() will increase the runtime PM counter > even it returns an error. Thus a pairing decrement is needed > to prevent refcount leak. Fix this by replacing this API with > pm_runtime_resume_and_get(), which will not change the runtime > PM counter on error. Applied to pci/tegra, thanks! [1/1] PCI: tegra: Fix runtime PM imbalance in pex_ep_event_pex_rst_deassert https://git.kernel.org/lpieralisi/pci/c/571cdd5294 Thanks, Lorenzo
Re: [PATCH v5 0/6] Add SiFive FU740 PCIe host controller driver support
On Tue, Apr 06, 2021 at 05:26:28PM +0800, Greentime Hu wrote: > This patchset includes SiFive FU740 PCIe host controller driver. We also > add pcie_aux clock and pcie_power_on_reset controller to prci driver for > PCIe driver to use it. > > This is tested with e1000e: Intel(R) PRO/1000 Network Card, AMD Radeon R5 > 230 graphics card and SP M.2 PCIe Gen 3 SSD in SiFive Unmatched based on > v5.11 Linux kernel. > > Changes in v5: > - Fix typo in comments > - Keep comments style consistent > - Refine some error handling codes > - Remove unneeded header file including > - Merge fu740_pcie_ltssm_enable implementation to fu740_pcie_start_link > > Changes in v4: > - Fix Wunused-but-set-variable warning in prci driver > > Changes in v3: > - Remove items that has been defined > - Refine format of sifive,fu740-pcie.yaml > - Replace perstn-gpios with the common one > - Change DBI mapping space to 2GB from 4GB > - Refine drivers/reset/Kconfig > > Changes in v2: > - Refine codes based on reviewers' feedback > - Remove define and use the common one > - Replace __raw_writel with writel_relaxed > - Split fu740_phyregreadwrite to write function > - Use readl_poll_timeout in stead of while loop checking > - Use dwc common codes > - Use gpio descriptors and the gpiod_* api. > - Replace devm_ioremap_resource with devm_platform_ioremap_resource_byname > - Replace devm_reset_control_get with devm_reset_control_get_exclusive > - Add more comments for delay and sleep > - Remove "phy ? x : y" expressions > - Refine code logic to remove possible infinite loop > - Replace magic number with meaningful define > - Remove fu740_pcie_pm_ops > - Use builtin_platform_driver > > Greentime Hu (5): > clk: sifive: Add pcie_aux clock in prci driver for PCIe driver > clk: sifive: Use reset-simple in prci driver for PCIe driver > MAINTAINERS: Add maintainers for SiFive FU740 PCIe driver > dt-bindings: PCI: Add SiFive FU740 PCIe host controller > riscv: dts: Add PCIe support for the SiFive FU740-C000 SoC > > Paul Walmsley (1): > PCI: fu740: Add SiFive FU740 PCIe host controller driver I can pull the patches above into the PCI tree (but will drop patch 6 - dts changes), is it OK for you ? Please let me know how you would like to upstream it. Lorenzo > .../bindings/pci/sifive,fu740-pcie.yaml | 113 +++ > MAINTAINERS | 8 + > arch/riscv/boot/dts/sifive/fu740-c000.dtsi| 33 ++ > drivers/clk/sifive/Kconfig| 2 + > drivers/clk/sifive/fu740-prci.c | 11 + > drivers/clk/sifive/fu740-prci.h | 2 +- > drivers/clk/sifive/sifive-prci.c | 54 +++ > drivers/clk/sifive/sifive-prci.h | 13 + > drivers/pci/controller/dwc/Kconfig| 9 + > drivers/pci/controller/dwc/Makefile | 1 + > drivers/pci/controller/dwc/pcie-fu740.c | 308 ++ > drivers/reset/Kconfig | 1 + > include/dt-bindings/clock/sifive-fu740-prci.h | 1 + > 13 files changed, 555 insertions(+), 1 deletion(-) > create mode 100644 > Documentation/devicetree/bindings/pci/sifive,fu740-pcie.yaml > create mode 100644 drivers/pci/controller/dwc/pcie-fu740.c > > -- > 2.30.2 >
Re: [PATCH v3 1/2] PCI: xilinx-nwl: Enable coherent PCIe DMA traffic using CCI
On Mon, 22 Feb 2021 14:17:31 +0530, Bharat Kumar Gogada wrote: > Add support for routing PCIe DMA traffic coherently when > Cache Coherent Interconnect (CCI) is enabled in the system. > The "dma-coherent" property is used to determine if CCI is enabled > or not. > Refer to https://developer.arm.com/documentation/ddi0470/k/preface > for the CCI specification. Applied to pci/xilinx, thanks! [1/2] PCI: xilinx-nwl: Enable coherent PCIe DMA traffic using CCI https://git.kernel.org/lpieralisi/pci/c/213e122052 [2/2] PCI: xilinx-nwl: Add optional "dma-coherent" property https://git.kernel.org/lpieralisi/pci/c/1c4422f226 Thanks, Lorenzo
Re: [PATCH v5 0/2] ata: ahci_brcm: Fix use of BCM7216 reset controller
On Fri, 12 Mar 2021 15:45:53 -0500, Jim Quinlan wrote: > v5 -- Improved (I hope) commit description (Bjorn). >-- Rnamed error labels (Krzyszt). >-- Fixed typos. > > v4 -- does not rely on a pending commit, unlike v3. > > v3 -- discard commit from v2; instead rely on the new function > reset_control_rearm provided in a recent commit [1] applied > to reset/next. >-- New commit to correct pcie-brcmstb.c usage of a reset controller > to use reset/rearm verses deassert/assert. > > [...] Applied to pci/brcmstb, thanks! [1/2] ata: ahci_brcm: Fix use of BCM7216 reset controller https://git.kernel.org/lpieralisi/pci/c/92b9cb55a9 [2/2] PCI: brcmstb: Use reset/rearm instead of deassert/assert https://git.kernel.org/lpieralisi/pci/c/a24fd1d646 Thanks, Lorenzo
Re: [PATCH v5 1/2] ata: ahci_brcm: Fix use of BCM7216 reset controller
On Fri, Mar 12, 2021 at 03:45:54PM -0500, Jim Quinlan wrote:
> This driver may use one of two resets controllers. Keep them in separate
> variables to keep things simple. The reset controller "rescal" is shared
> between the AHCI driver and the PCIe driver for the BrcmSTB 7216 chip. Use
> devm_reset_control_get_optional_shared() to handle this sharing.
>
> Fixes: 272ecd60a636 ("ata: ahci_brcm: BCM7216 reset is self de-asserting")
> Fixes: c345ec6a50e9 ("ata: ahci_brcm: Support BCM7216 reset controller name")
> Signed-off-by: Jim Quinlan
> Acked-by: Florian Fainelli
> ---
> drivers/ata/ahci_brcm.c | 46 -
> 1 file changed, 23 insertions(+), 23 deletions(-)
Hi Jens,
I am happy to take this series via the PCI tree but I'd need your
ACK on this patch, please let me know if you are OK with it.
Thanks,
Lorenzo
> diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c
> index 5b32df5d33ad..6e9c5ade4c2e 100644
> --- a/drivers/ata/ahci_brcm.c
> +++ b/drivers/ata/ahci_brcm.c
> @@ -86,7 +86,8 @@ struct brcm_ahci_priv {
> u32 port_mask;
> u32 quirks;
> enum brcm_ahci_version version;
> - struct reset_control *rcdev;
> + struct reset_control *rcdev_rescal;
> + struct reset_control *rcdev_ahci;
> };
>
> static inline u32 brcm_sata_readreg(void __iomem *addr)
> @@ -352,8 +353,8 @@ static int brcm_ahci_suspend(struct device *dev)
> else
> ret = 0;
>
> - if (priv->version != BRCM_SATA_BCM7216)
> - reset_control_assert(priv->rcdev);
> + reset_control_assert(priv->rcdev_ahci);
> + reset_control_rearm(priv->rcdev_rescal);
>
> return ret;
> }
> @@ -365,10 +366,10 @@ static int __maybe_unused brcm_ahci_resume(struct
> device *dev)
> struct brcm_ahci_priv *priv = hpriv->plat_data;
> int ret = 0;
>
> - if (priv->version == BRCM_SATA_BCM7216)
> - ret = reset_control_reset(priv->rcdev);
> - else
> - ret = reset_control_deassert(priv->rcdev);
> + ret = reset_control_deassert(priv->rcdev_ahci);
> + if (ret)
> + return ret;
> + ret = reset_control_reset(priv->rcdev_rescal);
> if (ret)
> return ret;
>
> @@ -434,7 +435,6 @@ static int brcm_ahci_probe(struct platform_device *pdev)
> {
> const struct of_device_id *of_id;
> struct device *dev = >dev;
> - const char *reset_name = NULL;
> struct brcm_ahci_priv *priv;
> struct ahci_host_priv *hpriv;
> struct resource *res;
> @@ -456,15 +456,15 @@ static int brcm_ahci_probe(struct platform_device *pdev)
> if (IS_ERR(priv->top_ctrl))
> return PTR_ERR(priv->top_ctrl);
>
> - /* Reset is optional depending on platform and named differently */
> - if (priv->version == BRCM_SATA_BCM7216)
> - reset_name = "rescal";
> - else
> - reset_name = "ahci";
> -
> - priv->rcdev = devm_reset_control_get_optional(>dev, reset_name);
> - if (IS_ERR(priv->rcdev))
> - return PTR_ERR(priv->rcdev);
> + if (priv->version == BRCM_SATA_BCM7216) {
> + priv->rcdev_rescal = devm_reset_control_get_optional_shared(
> + >dev, "rescal");
> + if (IS_ERR(priv->rcdev_rescal))
> + return PTR_ERR(priv->rcdev_rescal);
> + }
> + priv->rcdev_ahci = devm_reset_control_get_optional(>dev, "ahci");
> + if (IS_ERR(priv->rcdev_ahci))
> + return PTR_ERR(priv->rcdev_ahci);
>
> hpriv = ahci_platform_get_resources(pdev, 0);
> if (IS_ERR(hpriv))
> @@ -485,10 +485,10 @@ static int brcm_ahci_probe(struct platform_device *pdev)
> break;
> }
>
> - if (priv->version == BRCM_SATA_BCM7216)
> - ret = reset_control_reset(priv->rcdev);
> - else
> - ret = reset_control_deassert(priv->rcdev);
> + ret = reset_control_reset(priv->rcdev_rescal);
> + if (ret)
> + return ret;
> + ret = reset_control_deassert(priv->rcdev_ahci);
> if (ret)
> return ret;
>
> @@ -539,8 +539,8 @@ static int brcm_ahci_probe(struct platform_device *pdev)
> out_disable_clks:
> ahci_platform_disable_clks(hpriv);
> out_reset:
> - if (priv->version != BRCM_SATA_BCM7216)
> - reset_control_assert(priv->rcdev);
> + reset_control_assert(priv->rcdev_ahci);
> + reset_control_rearm(priv->rcdev_rescal);
> return ret;
> }
>
> --
> 2.17.1
>
Re: [PATCH v3 1/2] PCI: xilinx-nwl: Enable coherent PCIe DMA traffic using CCI
[+ Rob, Robin] On Mon, Feb 22, 2021 at 02:17:31PM +0530, Bharat Kumar Gogada wrote: > Add support for routing PCIe DMA traffic coherently when > Cache Coherent Interconnect (CCI) is enabled in the system. > The "dma-coherent" property is used to determine if CCI is enabled > or not. > Refer to https://developer.arm.com/documentation/ddi0470/k/preface > for the CCI specification. > > Signed-off-by: Bharat Kumar Gogada > --- > drivers/pci/controller/pcie-xilinx-nwl.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c > b/drivers/pci/controller/pcie-xilinx-nwl.c > index 07e36661bbc2..8689311c5ef6 100644 > --- a/drivers/pci/controller/pcie-xilinx-nwl.c > +++ b/drivers/pci/controller/pcie-xilinx-nwl.c > @@ -26,6 +26,7 @@ > > /* Bridge core config registers */ > #define BRCFG_PCIE_RX0 0x > +#define BRCFG_PCIE_RX1 0x0004 > #define BRCFG_INTERRUPT 0x0010 > #define BRCFG_PCIE_RX_MSG_FILTER 0x0020 > > @@ -128,6 +129,7 @@ > #define NWL_ECAM_VALUE_DEFAULT 12 > > #define CFG_DMA_REG_BAR GENMASK(2, 0) > +#define CFG_PCIE_CACHE GENMASK(7, 0) > > #define INT_PCI_MSI_NR (2 * 32) > > @@ -675,6 +677,11 @@ static int nwl_pcie_bridge_init(struct nwl_pcie *pcie) > nwl_bridge_writel(pcie, CFG_ENABLE_MSG_FILTER_MASK, > BRCFG_PCIE_RX_MSG_FILTER); > > + /* This routes the PCIe DMA traffic to go through CCI path */ > + if (of_dma_is_coherent(dev->of_node)) > + nwl_bridge_writel(pcie, nwl_bridge_readl(pcie, BRCFG_PCIE_RX1) | > + CFG_PCIE_CACHE, BRCFG_PCIE_RX1); > + This is weird. FW is telling us that the RC is DMA coherent hence we have to program the RC so that it is indeed DMA coherent. It does not make much sense. I think this is a set-up that should be programmed by firmware and reported to the kernel via the standard "dma-coherent" property. The kernel can read that register to check the HW configuration complies with the DT property. I'd like to get RobH/Robin thoughts on this before proceeding - they have more insights about the DT dma-coherent usage/bindings and expected behaviour. Thanks, Lorenzo > err = nwl_wait_for_link(pcie); > if (err) > return err; > -- > 2.17.1 >
Re: [PATCH v3 00/14] PCI/MSI: Getting rid of msi_controller, and other cleanups
On Tue, 30 Mar 2021 16:11:31 +0100, Marc Zyngier wrote: > This is a respin of the series described at [1]. > > * From v2 [2]: > - Fixed the Xilinx driver, thanks to Bharat for testing it > - Dropped the no_msi attribute, and solely rely on msi_domain, which > has the same effect for the only platform that was using it. > - Fixed compilation on architectures that do not select the generic > MSI support > > [...] I have applied it to pci/msi and should be moved into -next shortly for further testing/visibility, thanks a lot for putting it together. [01/14] PCI: tegra: Convert to MSI domains https://git.kernel.org/lpieralisi/pci/c/973a28677e [02/14] PCI: rcar: Don't allocate extra memory for the MSI capture address https://git.kernel.org/lpieralisi/pci/c/c244dc15dc [03/14] PCI: rcar: Convert to MSI domains https://git.kernel.org/lpieralisi/pci/c/516286287d [04/14] PCI: xilinx: Don't allocate extra memory for the MSI capture address https://git.kernel.org/lpieralisi/pci/c/cc8cf90738 [05/14] PCI: xilinx: Convert to MSI domains https://git.kernel.org/lpieralisi/pci/c/b66873599e [06/14] PCI: hv: Drop msi_controller structure https://git.kernel.org/lpieralisi/pci/c/65b131816a [07/14] PCI/MSI: Drop use of msi_controller from core code https://git.kernel.org/lpieralisi/pci/c/54729d2a7a [08/14] PCI/MSI: Kill msi_controller structure https://git.kernel.org/lpieralisi/pci/c/27278a3fac [09/14] PCI/MSI: Kill default_teardown_msi_irqs() https://git.kernel.org/lpieralisi/pci/c/f68f571db9 [10/14] PCI/MSI: Let PCI host bridges declare their reliance on MSI domains https://git.kernel.org/lpieralisi/pci/c/419150a4ff [11/14] PCI/MSI: Make pci_host_common_probe() declare its reliance on MSI domains https://git.kernel.org/lpieralisi/pci/c/98be0634c8 [12/14] PCI: mediatek: Advertise lack of built-in MSI handling https://git.kernel.org/lpieralisi/pci/c/77cbd88c90 [13/14] PCI/MSI: Document the various ways of ending up with NO_MSI https://git.kernel.org/lpieralisi/pci/c/44ec480daf [14/14] PCI: Refactor HT advertising of NO_MSI flag https://git.kernel.org/lpieralisi/pci/c/18d56e5afe Thanks, Lorenzo
Re: [PATCH v3 03/14] PCI: rcar: Convert to MSI domains
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
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,
> +
Re: [PATCH] PCI: xgene: fix a mistake about cfg address
On Tue, Mar 30, 2021 at 02:19:26PM -0500, Bjorn Helgaas wrote:
> On Sun, Mar 28, 2021 at 10:41:18PM +0800, Dejin Zheng wrote:
> > It has a wrong modification to the xgene driver by the commit
> > e2dcd20b1645a. it use devm_platform_ioremap_resource_byname() to
> > simplify codes and remove the res variable, But the following code
> > needs to use this res variable, So after this commit, the port->cfg_addr
> > will get a wrong address. Now, revert it.
> >
> > Fixes: e2dcd20b1645a ("PCI: controller: Convert to
> > devm_platform_ioremap_resource_byname()")
> > Reported-by: dann.fraz...@canonical.com
> > Signed-off-by: Dejin Zheng
>
> This looks right to me, but since e2dcd20b1645a appeared in v5.9-rc1,
> I think it should have:
>
> Cc: sta...@vger.kernel.org # v5.9+
Fixed and pushed out in my pci/xgene branch.
Thanks,
Lorenzo
Re: [PATCH v3 02/14] PCI: rcar: Don't allocate extra memory for the MSI capture address
On Tue, Mar 30, 2021 at 04:11:33PM +0100, Marc Zyngier wrote:
> A long cargo-culted behaviour of PCI drivers is to allocate memory
> to obtain an address that is fed to the controller as the MSI
> capture address (i.e. the MSI doorbell).
>
> But there is no actual requirement for this address to be RAM.
> All it needs to be is a suitable aligned address that will
> *not* be DMA'd to.
>
> Since the rcar platform already has a requirement that this
> address should be in the first 4GB of the physical address space,
> use the controller's own base address as the capture address.
>
> Signed-off-by: Marc Zyngier
> ---
> drivers/pci/controller/pcie-rcar-host.c | 18 +++---
> 1 file changed, 7 insertions(+), 11 deletions(-)
Marek, Yoshihiro,
can you test this patch please and report back ? It is not fundamental
for the rest of the series (ie the rest of the series does not depend on
it) and we can still merge the series without it but it would be good if
you can review and test anyway.
I'd like to merge this series into -next shortly.
Thanks,
Lorenzo
> diff --git a/drivers/pci/controller/pcie-rcar-host.c
> b/drivers/pci/controller/pcie-rcar-host.c
> index a728e8f9ad3c..ce952403e22c 100644
> --- a/drivers/pci/controller/pcie-rcar-host.c
> +++ b/drivers/pci/controller/pcie-rcar-host.c
> @@ -36,7 +36,6 @@ struct rcar_msi {
> DECLARE_BITMAP(used, INT_PCI_MSI_NR);
> struct irq_domain *domain;
> struct msi_controller chip;
> - unsigned long pages;
> struct mutex lock;
> int irq1;
> int irq2;
> @@ -680,14 +679,15 @@ static void rcar_pcie_unmap_msi(struct rcar_pcie_host
> *host)
> static void rcar_pcie_hw_enable_msi(struct rcar_pcie_host *host)
> {
> struct rcar_pcie *pcie = >pcie;
> - struct rcar_msi *msi = >msi;
> - unsigned long base;
> + struct device *dev = pcie->dev;
> + struct resource res;
>
> - /* setup MSI data target */
> - base = virt_to_phys((void *)msi->pages);
> + if (WARN_ON(of_address_to_resource(dev->of_node, 0, )))
> + return;
>
> - rcar_pci_write_reg(pcie, lower_32_bits(base) | MSIFE, PCIEMSIALR);
> - rcar_pci_write_reg(pcie, upper_32_bits(base), PCIEMSIAUR);
> + /* setup MSI data target */
> + rcar_pci_write_reg(pcie, lower_32_bits(res.start) | MSIFE, PCIEMSIALR);
> + rcar_pci_write_reg(pcie, upper_32_bits(res.start), PCIEMSIAUR);
>
> /* enable all MSI interrupts */
> rcar_pci_write_reg(pcie, 0x, PCIEMSIIER);
> @@ -735,7 +735,6 @@ static int rcar_pcie_enable_msi(struct rcar_pcie_host
> *host)
> }
>
> /* setup MSI data target */
> - msi->pages = __get_free_pages(GFP_KERNEL | GFP_DMA32, 0);
> rcar_pcie_hw_enable_msi(host);
>
> return 0;
> @@ -748,7 +747,6 @@ static int rcar_pcie_enable_msi(struct rcar_pcie_host
> *host)
> static void rcar_pcie_teardown_msi(struct rcar_pcie_host *host)
> {
> struct rcar_pcie *pcie = >pcie;
> - struct rcar_msi *msi = >msi;
>
> /* Disable all MSI interrupts */
> rcar_pci_write_reg(pcie, 0, PCIEMSIIER);
> @@ -756,8 +754,6 @@ static void rcar_pcie_teardown_msi(struct rcar_pcie_host
> *host)
> /* Disable address decoding of the MSI interrupt, MSIFE */
> rcar_pci_write_reg(pcie, 0, PCIEMSIALR);
>
> - free_pages(msi->pages, 0);
> -
> rcar_pcie_unmap_msi(host);
> }
>
> --
> 2.29.2
>
Re: [PATCH] irqchip/gic-v3: Fix IPRIORITYR can't perform byte operations in GIC-600
On Tue, Mar 30, 2021 at 12:05:46PM +0100, Lorenzo Pieralisi wrote:
> On Tue, Mar 30, 2021 at 11:33:13AM +0100, Marc Zyngier wrote:
> > [+Lorenzo, +Julien on an actual email address]
> >
> > On Tue, 30 Mar 2021 11:06:19 +0100,
> > Lecopzer Chen wrote:
> > >
> > > When pseudo-NMI enabled, register_nmi() set priority of specific IRQ
> > > by byte ops, and this doesn't work in GIC-600.
> > >
> > > We have asked ARM Support [1]:
> > > > Please refer to following description in
> > > > "2.1.2 Distributor ACE-Lite slave interface" of GIC-600 TRM for
> > > > the GIC600 ACE-lite slave interface supported sizes:
> > > > "The GIC-600 only accepts single beat accesses of the sizes for
> > > > each register that are shown in the Programmers model,
> > > > see Chapter 4 Programmer's model on page 4-102.
> > > > All other accesses are rejected and given either an
> > > > OKAY or SLVERR response that is based on the GICT_ERR0CTLR.UE bit.".
> > >
> > > Thus the register needs to be written by double word operation and
> > > the step will be: read 32bit, set byte and write it back.
> > >
> > > [1] https://services.arm.com/support/s/case/5003t1L4Pba
> >
> > You do realise that this link:
> >
> > - is unusable for most people as it is behind a registration interface
> > - discloses confidential information to other people
> >
> > I strongly suggest you stop posting such links.
> >
> > >
> > > Signed-off-by: Lecopzer Chen
> > > ---
> > > drivers/irqchip/irq-gic-v3.c | 13 -
> > > 1 file changed, 12 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> > > index eb0ee356a629..cfc5a6ad30dc 100644
> > > --- a/drivers/irqchip/irq-gic-v3.c
> > > +++ b/drivers/irqchip/irq-gic-v3.c
> > > @@ -440,10 +440,21 @@ static void gic_irq_set_prio(struct irq_data *d, u8
> > > prio)
> > > {
> > > void __iomem *base = gic_dist_base(d);
> > > u32 offset, index;
> > > + u32 val, prio_offset_mask, prio_offset_shift;
> > >
> > > offset = convert_offset_index(d, GICD_IPRIORITYR, );
> > >
> > > - writeb_relaxed(prio, base + offset + index);
> > > + /*
> > > + * GIC-600 memory mapping register doesn't support byte opteration,
> > > + * thus read 32-bits from register, set bytes and wtire back to it.
> > > + */
> > > + prio_offset_shift = (index & 0x3) * 8;
> > > + prio_offset_mask = GENMASK(prio_offset_shift + 7, prio_offset_shift);
> > > + index &= ~0x3;
> > > + val = readl_relaxed(base + offset + index);
> > > + val &= ~prio_offset_mask;
> > > + val |= prio << prio_offset_shift;
> > > + writel_relaxed(val, base + offset + index);
> > > }
> > >
> > > static u32 gic_get_ppi_index(struct irq_data *d)
> >
> > From the architecture spec:
> >
> >
> > 11.1.3 GIC memory-mapped register access
> >
> > In any system, access to the following registers must be supported:
> >
> > [...]
> > * Byte accesses to:
> > - GICD_IPRIORITYR.
> > - GICD_ITARGETSR.
> > - GICD_SPENDSGIR.
> > - GICD_CPENDSGIR.
> > - GICR_IPRIORITYR.
> >
> >
> > So if GIC600 doesn't follow this architectural requirement, this is a
> > HW erratum, and I want an actual description of the HW issue together
> > with an erratum number.
> >
> > Lorenzo, can you please investigate on your side?
>
> Sure - I will look into it and report back.
Checked - I don't think this patch is needed so it should be dropped and
a follow-up discussion can continue in the relevant/appropriate forum -
if there is anything left to discuss.
Thanks,
Lorenzo
Re: [PATCH] irqchip/gic-v3: Fix IPRIORITYR can't perform byte operations in GIC-600
On Tue, Mar 30, 2021 at 11:33:13AM +0100, Marc Zyngier wrote:
> [+Lorenzo, +Julien on an actual email address]
>
> On Tue, 30 Mar 2021 11:06:19 +0100,
> Lecopzer Chen wrote:
> >
> > When pseudo-NMI enabled, register_nmi() set priority of specific IRQ
> > by byte ops, and this doesn't work in GIC-600.
> >
> > We have asked ARM Support [1]:
> > > Please refer to following description in
> > > "2.1.2 Distributor ACE-Lite slave interface" of GIC-600 TRM for
> > > the GIC600 ACE-lite slave interface supported sizes:
> > > "The GIC-600 only accepts single beat accesses of the sizes for
> > > each register that are shown in the Programmers model,
> > > see Chapter 4 Programmer's model on page 4-102.
> > > All other accesses are rejected and given either an
> > > OKAY or SLVERR response that is based on the GICT_ERR0CTLR.UE bit.".
> >
> > Thus the register needs to be written by double word operation and
> > the step will be: read 32bit, set byte and write it back.
> >
> > [1] https://services.arm.com/support/s/case/5003t1L4Pba
>
> You do realise that this link:
>
> - is unusable for most people as it is behind a registration interface
> - discloses confidential information to other people
>
> I strongly suggest you stop posting such links.
>
> >
> > Signed-off-by: Lecopzer Chen
> > ---
> > drivers/irqchip/irq-gic-v3.c | 13 -
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> > index eb0ee356a629..cfc5a6ad30dc 100644
> > --- a/drivers/irqchip/irq-gic-v3.c
> > +++ b/drivers/irqchip/irq-gic-v3.c
> > @@ -440,10 +440,21 @@ static void gic_irq_set_prio(struct irq_data *d, u8
> > prio)
> > {
> > void __iomem *base = gic_dist_base(d);
> > u32 offset, index;
> > + u32 val, prio_offset_mask, prio_offset_shift;
> >
> > offset = convert_offset_index(d, GICD_IPRIORITYR, );
> >
> > - writeb_relaxed(prio, base + offset + index);
> > + /*
> > +* GIC-600 memory mapping register doesn't support byte opteration,
> > +* thus read 32-bits from register, set bytes and wtire back to it.
> > +*/
> > + prio_offset_shift = (index & 0x3) * 8;
> > + prio_offset_mask = GENMASK(prio_offset_shift + 7, prio_offset_shift);
> > + index &= ~0x3;
> > + val = readl_relaxed(base + offset + index);
> > + val &= ~prio_offset_mask;
> > + val |= prio << prio_offset_shift;
> > + writel_relaxed(val, base + offset + index);
> > }
> >
> > static u32 gic_get_ppi_index(struct irq_data *d)
>
> From the architecture spec:
>
>
> 11.1.3 GIC memory-mapped register access
>
> In any system, access to the following registers must be supported:
>
> [...]
> * Byte accesses to:
> - GICD_IPRIORITYR.
> - GICD_ITARGETSR.
> - GICD_SPENDSGIR.
> - GICD_CPENDSGIR.
> - GICR_IPRIORITYR.
>
>
> So if GIC600 doesn't follow this architectural requirement, this is a
> HW erratum, and I want an actual description of the HW issue together
> with an erratum number.
>
> Lorenzo, can you please investigate on your side?
Sure - I will look into it and report back.
Thanks,
Lorenzo
Re: [PATCH v5 2/2] PCI: brcmstb: Use reset/rearm instead of deassert/assert
On Mon, Mar 29, 2021 at 09:50:13AM -0700, Florian Fainelli wrote: > On 3/29/21 9:10 AM, Lorenzo Pieralisi wrote: > > On Fri, Mar 12, 2021 at 03:45:55PM -0500, Jim Quinlan wrote: > >> The Broadcom STB PCIe RC uses a reset control "rescal" for certain chips. > >> The "rescal" implements a "pulse reset" so using assert/deassert is wrong > >> for this device. Instead, we use reset/rearm. We need to use rearm so > >> that we can reset it after a suspend/resume cycle; w/o using "rearm", the > >> "rescal" device will only ever fire once. > >> > >> Of course for suspend/resume to work we also need to put the reset/rearm > >> calls in the suspend and resume routines. > > > > Actually - I am sorry but it looks like you will have to split the patch > > in two since this is two logical changes. > > I do not believe this can be easily split, since there is currently a > misused of the reset controller API and this patch fixes all call sites > at once. It would not really make sense to fix probe/remove and then > leave suspend/resume broken in the same manner. Right - I was reading the previous versions of the set, it makes sense to keep it in one logical change. Do you want me to take it or you prefer an ACK so that it can go via a different tree ? Thanks, Lorenzo
Re: [PATCH v5 2/2] PCI: brcmstb: Use reset/rearm instead of deassert/assert
On Fri, Mar 12, 2021 at 03:45:55PM -0500, Jim Quinlan wrote:
> The Broadcom STB PCIe RC uses a reset control "rescal" for certain chips.
> The "rescal" implements a "pulse reset" so using assert/deassert is wrong
> for this device. Instead, we use reset/rearm. We need to use rearm so
> that we can reset it after a suspend/resume cycle; w/o using "rearm", the
> "rescal" device will only ever fire once.
>
> Of course for suspend/resume to work we also need to put the reset/rearm
> calls in the suspend and resume routines.
Actually - I am sorry but it looks like you will have to split the patch
in two since this is two logical changes.
Thanks,
Lorenzo
> Fixes: 740d6c3708a9 ("PCI: brcmstb: Add control of rescal reset")
> Signed-off-by: Jim Quinlan
> Acked-by: Florian Fainelli
> ---
> drivers/pci/controller/pcie-brcmstb.c | 19 +--
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c
> b/drivers/pci/controller/pcie-brcmstb.c
> index e330e6811f0b..3b35d629035e 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -1148,6 +1148,7 @@ static int brcm_pcie_suspend(struct device *dev)
>
> brcm_pcie_turn_off(pcie);
> ret = brcm_phy_stop(pcie);
> + reset_control_rearm(pcie->rescal);
> clk_disable_unprepare(pcie->clk);
>
> return ret;
> @@ -1163,9 +1164,13 @@ static int brcm_pcie_resume(struct device *dev)
> base = pcie->base;
> clk_prepare_enable(pcie->clk);
>
> + ret = reset_control_reset(pcie->rescal);
> + if (ret)
> + goto err_disable_clk;
> +
> ret = brcm_phy_start(pcie);
> if (ret)
> - goto err;
> + goto err_reset;
>
> /* Take bridge out of reset so we can access the SERDES reg */
> pcie->bridge_sw_init_set(pcie, 0);
> @@ -1180,14 +1185,16 @@ static int brcm_pcie_resume(struct device *dev)
>
> ret = brcm_pcie_setup(pcie);
> if (ret)
> - goto err;
> + goto err_reset;
>
> if (pcie->msi)
> brcm_msi_set_regs(pcie->msi);
>
> return 0;
>
> -err:
> +err_reset:
> + reset_control_rearm(pcie->rescal);
> +err_disable_clk:
> clk_disable_unprepare(pcie->clk);
> return ret;
> }
> @@ -1197,7 +1204,7 @@ static void __brcm_pcie_remove(struct brcm_pcie *pcie)
> brcm_msi_remove(pcie);
> brcm_pcie_turn_off(pcie);
> brcm_phy_stop(pcie);
> - reset_control_assert(pcie->rescal);
> + reset_control_rearm(pcie->rescal);
> clk_disable_unprepare(pcie->clk);
> }
>
> @@ -1278,13 +1285,13 @@ static int brcm_pcie_probe(struct platform_device
> *pdev)
> return PTR_ERR(pcie->perst_reset);
> }
>
> - ret = reset_control_deassert(pcie->rescal);
> + ret = reset_control_reset(pcie->rescal);
> if (ret)
> dev_err(>dev, "failed to deassert 'rescal'\n");
>
> ret = brcm_phy_start(pcie);
> if (ret) {
> - reset_control_assert(pcie->rescal);
> + reset_control_rearm(pcie->rescal);
> clk_disable_unprepare(pcie->clk);
> return ret;
> }
> --
> 2.17.1
>
Re: [PATCH v5 2/2] PCI: brcmstb: Use reset/rearm instead of deassert/assert
On Fri, Mar 12, 2021 at 03:45:55PM -0500, Jim Quinlan wrote:
> The Broadcom STB PCIe RC uses a reset control "rescal" for certain chips.
> The "rescal" implements a "pulse reset" so using assert/deassert is wrong
> for this device. Instead, we use reset/rearm. We need to use rearm so
> that we can reset it after a suspend/resume cycle; w/o using "rearm", the
> "rescal" device will only ever fire once.
>
> Of course for suspend/resume to work we also need to put the reset/rearm
> calls in the suspend and resume routines.
>
> Fixes: 740d6c3708a9 ("PCI: brcmstb: Add control of rescal reset")
> Signed-off-by: Jim Quinlan
> Acked-by: Florian Fainelli
> ---
> drivers/pci/controller/pcie-brcmstb.c | 19 +--
> 1 file changed, 13 insertions(+), 6 deletions(-)
Should I take this patch in the PCI queue ?
Thanks,
Lorenzo
> diff --git a/drivers/pci/controller/pcie-brcmstb.c
> b/drivers/pci/controller/pcie-brcmstb.c
> index e330e6811f0b..3b35d629035e 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -1148,6 +1148,7 @@ static int brcm_pcie_suspend(struct device *dev)
>
> brcm_pcie_turn_off(pcie);
> ret = brcm_phy_stop(pcie);
> + reset_control_rearm(pcie->rescal);
> clk_disable_unprepare(pcie->clk);
>
> return ret;
> @@ -1163,9 +1164,13 @@ static int brcm_pcie_resume(struct device *dev)
> base = pcie->base;
> clk_prepare_enable(pcie->clk);
>
> + ret = reset_control_reset(pcie->rescal);
> + if (ret)
> + goto err_disable_clk;
> +
> ret = brcm_phy_start(pcie);
> if (ret)
> - goto err;
> + goto err_reset;
>
> /* Take bridge out of reset so we can access the SERDES reg */
> pcie->bridge_sw_init_set(pcie, 0);
> @@ -1180,14 +1185,16 @@ static int brcm_pcie_resume(struct device *dev)
>
> ret = brcm_pcie_setup(pcie);
> if (ret)
> - goto err;
> + goto err_reset;
>
> if (pcie->msi)
> brcm_msi_set_regs(pcie->msi);
>
> return 0;
>
> -err:
> +err_reset:
> + reset_control_rearm(pcie->rescal);
> +err_disable_clk:
> clk_disable_unprepare(pcie->clk);
> return ret;
> }
> @@ -1197,7 +1204,7 @@ static void __brcm_pcie_remove(struct brcm_pcie *pcie)
> brcm_msi_remove(pcie);
> brcm_pcie_turn_off(pcie);
> brcm_phy_stop(pcie);
> - reset_control_assert(pcie->rescal);
> + reset_control_rearm(pcie->rescal);
> clk_disable_unprepare(pcie->clk);
> }
>
> @@ -1278,13 +1285,13 @@ static int brcm_pcie_probe(struct platform_device
> *pdev)
> return PTR_ERR(pcie->perst_reset);
> }
>
> - ret = reset_control_deassert(pcie->rescal);
> + ret = reset_control_reset(pcie->rescal);
> if (ret)
> dev_err(>dev, "failed to deassert 'rescal'\n");
>
> ret = brcm_phy_start(pcie);
> if (ret) {
> - reset_control_assert(pcie->rescal);
> + reset_control_rearm(pcie->rescal);
> clk_disable_unprepare(pcie->clk);
> return ret;
> }
> --
> 2.17.1
>
Re: [PATCH] PCI: xgene: fix a mistake about cfg address
On Sun, 28 Mar 2021 22:41:18 +0800, Dejin Zheng wrote: > It has a wrong modification to the xgene driver by the commit > e2dcd20b1645a. it use devm_platform_ioremap_resource_byname() to > simplify codes and remove the res variable, But the following code > needs to use this res variable, So after this commit, the port->cfg_addr > will get a wrong address. Now, revert it. Applied to pci/xgene, thanks! [1/1] PCI: xgene: Fix cfg resource mapping https://git.kernel.org/lpieralisi/pci/c/f243b619b4 Thanks, Lorenzo
Re: [PATCH v5] PCI: endpoint: Fix NULL pointer dereference for ->get_features()
On Wed, 24 Mar 2021 15:46:09 +0530, Shradha Todi wrote: > get_features ops of pci_epc_ops may return NULL, causing NULL pointer > dereference in pci_epf_test_alloc_space function. Let us add a check for > pci_epc_feature pointer in pci_epf_test_bind before we access it to avoid > any such NULL pointer dereference and return -ENOTSUPP in case > pci_epc_feature is not found. > > When the patch is not applied and EPC features is not implemented in the > platform driver, we see the following dump due to kernel NULL pointer > dereference. > > [...] Applied to pci/endpoint, thanks! [1/1] PCI: endpoint: Fix NULL pointer dereference for ->get_features() https://git.kernel.org/lpieralisi/pci/c/6613bc2301 Thanks, Lorenzo
Re: [PATCH] arm64: PCI: Enable SMC conduit
On Tue, Jan 26, 2021 at 10:53:51PM +, Will Deacon wrote: > On Tue, Jan 26, 2021 at 11:08:31AM -0600, Vikram Sethi wrote: > > On 1/22/2021 1:48 PM, Will Deacon wrote: > > > On Fri, Jan 08, 2021 at 10:32:16AM +, Lorenzo Pieralisi wrote: > > >> On Thu, Jan 07, 2021 at 04:05:48PM -0500, Jon Masters wrote: > > >>> On 1/7/21 1:14 PM, Will Deacon wrote: > > >>>> On Mon, Jan 04, 2021 at 10:57:35PM -0600, Jeremy Linton wrote: > > >>>>> Given that most arm64 platform's PCI implementations needs quirks > > >>>>> to deal with problematic config accesses, this is a good place to > > >>>>> apply a firmware abstraction. The ARM PCI SMMCCC spec details a > > >>>>> standard SMC conduit designed to provide a simple PCI config > > >>>>> accessor. This specification enhances the existing ACPI/PCI > > >>>>> abstraction and expects power, config, etc functionality is handled > > >>>>> by the platform. It also is very explicit that the resulting config > > >>>>> space registers must behave as is specified by the pci specification. > > >>>>> > > >>>>> Lets hook the normal ACPI/PCI config path, and when we detect > > >>>>> missing MADT data, attempt to probe the SMC conduit. If the conduit > > >>>>> exists and responds for the requested segment number (provided by the > > >>>>> ACPI namespace) attach a custom pci_ecam_ops which redirects > > >>>>> all config read/write requests to the firmware. > > >>>>> > > >>>>> This patch is based on the Arm PCI Config space access document @ > > >>>>> https://developer.arm.com/documentation/den0115/latest > > >>>> Why does firmware need to be involved with this at all? Can't we just > > >>>> quirk Linux when these broken designs show up in production? We'll need > > >>>> to modify Linux _anyway_ when the firmware interface isn't implemented > > >>>> correctly... > > >>> I agree with Will on this. I think we want to find a way to address some > > >>> of the non-compliance concerns through quirks in Linux. However... > > >> I understand the concern and if you are asking me if this can be fixed > > >> in Linux it obviously can. The point is, at what cost for SW and > > >> maintenance - in Linux and other OSes, I think Jeremy summed it up > > >> pretty well: > > >> > > >> https://lore.kernel.org/linux-pci/61558f73-9ac8-69fe-34c1-2074dec5f...@arm.com > > >> > > >> The issue here is that what we are asked to support on ARM64 ACPI is a > > >> moving target and the target carries PCI with it. > > >> > > >> This potentially means that all drivers in: > > >> > > >> drivers/pci/controller > > >> > > >> may require an MCFG quirk and to implement it we may have to: > > >> > > >> - Define new ACPI bindings (that may need AML and that's already a > > >> showstopper for some OSes) > > >> - Require to manage clocks in the kernel (see link-up checks) > > >> - Handle PCI config space faults in the kernel > > >> > > >> Do we really want to do that ? I don't think so. Therefore we need > > >> to have a policy to define what constitutes a "reasonable" quirk and > > >> that's not objective I am afraid, however we slice it (there is no > > >> such a thing as eg 90% ECAM). > > > Without a doubt, I would much prefer to see these quirks and workarounds > > > in Linux than hidden behind a firmware interface. Every single time. > > > > In that case, can you please comment on/apply Tegra194 ECAM quirk that was > > rejected > > > > a year ago, and was the reason we worked with Samer/ARM to define this > > common > > > > mechanism? > > > > https://lkml.org/lkml/2020/1/3/395 > > > > The T194 ECAM is from widely used Root Port IP from a IP vendor. That is > > one reason so many > > > > *existing* SOCs have ECAM quirks. ARM is only now working with the Root > > port IP vendors > > > > to test ECAM, MSI etc, but the reality is there were deficiencies in > > industry IP that is widely > > > > used. If this common quirk is not the way to go, then please apply the T194 > > specific quirk which was > > > > NAK'd a year ago, or sugg
Re: [PATCH v2 12/15] PCI/MSI: Let PCI host bridges declare their reliance on MSI domains
On Tue, Mar 23, 2021 at 06:09:36PM +, Marc Zyngier wrote:
> Hi Robin,
>
> On Tue, 23 Mar 2021 11:45:02 +,
> Robin Murphy wrote:
> >
> > On 2021-03-22 18:46, Marc Zyngier wrote:
> > > The new 'no_msi' attribute solves the problem of advertising the lack
> > > of MSI capability for host bridges that know for sure that there will
> > > be no MSI for their end-points.
> > >
> > > However, there is a whole class of host bridges that cannot know
> > > whether MSIs will be provided or not, as they rely on other blocks
> > > to provide the MSI functionnality, using MSI domains. This is
> > > the case for example on systems that use the ARM GIC architecture.
> > >
> > > Introduce a new attribute ('msi_domain') indicating that implicit
> > > dependency, and use this property to set the NO_MSI flag when
> > > no MSI domain is found at probe time.
> > >
> > > Acked-by: Bjorn Helgaas
> > > Signed-off-by: Marc Zyngier
> > > ---
> > > drivers/pci/probe.c | 2 +-
> > > include/linux/pci.h | 1 +
> > > 2 files changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > > index 146bd85c037e..bac9f69a06a8 100644
> > > --- a/drivers/pci/probe.c
> > > +++ b/drivers/pci/probe.c
> > > @@ -925,7 +925,7 @@ static int pci_register_host_bridge(struct
> > > pci_host_bridge *bridge)
> > > device_enable_async_suspend(bus->bridge);
> > > pci_set_bus_of_node(bus);
> > > pci_set_bus_msi_domain(bus);
> > > - if (bridge->no_msi)
> > > + if (bridge->no_msi || (bridge->msi_domain && !bus->dev.msi_domain))
> > > bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
> > > if (!parent)
> > > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > > index 48605cca82ae..d322d00db432 100644
> > > --- a/include/linux/pci.h
> > > +++ b/include/linux/pci.h
> > > @@ -551,6 +551,7 @@ struct pci_host_bridge {
> > > unsigned intpreserve_config:1; /* Preserve FW resource
> > > setup */
> > > unsigned intsize_windows:1; /* Enable root bus
> > > sizing */
> > > unsigned intno_msi:1; /* Bridge has no MSI
> > > support */
> > > + unsigned intmsi_domain:1; /* Bridge wants MSI domain */
> >
> > Aren't these really the same thing? Either way we're saying the bridge
> > itself doesn't handle MSIs, it's just in one case we're effectively
> > encoding a platform-specific assumption that an external domain won't
> > be provided. I can't help wondering whether that distinction is really
> > necessary...
>
> There is a subtle difference: no_msi indicates that there is no way
> *any* MSI can be dealt with whatsoever (maybe because the RC doesn't
> forward the corresponding TLPs?). msi_domain says "no MSI unless...".
>
> We could implement the former with the latter, but I have the feeling
> that's not totally bullet proof. Happy to revisit this if you think it
> really matters.
IIUC msi_domain == 1 means: this host bridge needs an msi_domain to enable
MSIs, which in turn means that there are bridges that do _not_ require
an msi_domain to enable MSIs. I don't know how other arches handle the
msi_domain pointer but I am asking whether making:
if (bridge->no_msi || !bus->dev.msi_domain))
bus->bus_flags |= PCI_BUS_FLAGS_NO_MSI;
is a possibility (removing the need for the msi_domain flag).
At least this looks more like an arch property than a host bridge
specific property (eg patch [13] pci_host_common_probe() may be used on
arches other than ARM where it is not necessary true that it requires an
msi_domain to enable MSIs).
I agree that's complicated to untangle - just asking if there is way
to simplify it.
Thanks,
Lorenzo
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
Re: [PATCH] PCI: dwc/intel-gw: Fix enabling the legacy PCI interrupt lines
On Wed, Jan 06, 2021 at 02:55:40PM +0100, Martin Blumenstingl wrote:
> The legacy PCI interrupt lines need to be enabled using PCIE_APP_IRNEN
> bits 13 (INTA), 14 (INTB), 15 (INTC) and 16 (INTD). The old code however
> was taking (for example) "13" as raw value instead of taking BIT(13).
> Define the legacy PCI interrupt bits using the BIT() macro and then use
> these in PCIE_APP_IRN_INT.
>
> Fixes: ed22aaaede44 ("PCI: dwc: intel: PCIe RC controller driver")
> Signed-off-by: Martin Blumenstingl
> ---
> drivers/pci/controller/dwc/pcie-intel-gw.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-intel-gw.c
> b/drivers/pci/controller/dwc/pcie-intel-gw.c
> index 0cedd1f95f37..ae96bfbb6c83 100644
> --- a/drivers/pci/controller/dwc/pcie-intel-gw.c
> +++ b/drivers/pci/controller/dwc/pcie-intel-gw.c
> @@ -39,6 +39,10 @@
> #define PCIE_APP_IRN_PM_TO_ACK BIT(9)
> #define PCIE_APP_IRN_LINK_AUTO_BW_STAT BIT(11)
> #define PCIE_APP_IRN_BW_MGT BIT(12)
> +#define PCIE_APP_IRN_INTABIT(13)
> +#define PCIE_APP_IRN_INTBBIT(14)
> +#define PCIE_APP_IRN_INTCBIT(15)
> +#define PCIE_APP_IRN_INTDBIT(16)
> #define PCIE_APP_IRN_MSG_LTR BIT(18)
> #define PCIE_APP_IRN_SYS_ERR_RC BIT(29)
> #define PCIE_APP_INTX_OFST 12
> @@ -48,10 +52,8 @@
> PCIE_APP_IRN_RX_VDM_MSG | PCIE_APP_IRN_SYS_ERR_RC | \
> PCIE_APP_IRN_PM_TO_ACK | PCIE_APP_IRN_MSG_LTR | \
> PCIE_APP_IRN_BW_MGT | PCIE_APP_IRN_LINK_AUTO_BW_STAT | \
> - (PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTA) | \
> - (PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTB) | \
> - (PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTC) | \
> - (PCIE_APP_INTX_OFST + PCI_INTERRUPT_INTD))
> + PCIE_APP_IRN_INTA | PCIE_APP_IRN_INTB | \
> + PCIE_APP_IRN_INTC | PCIE_APP_IRN_INTD)
>
> #define BUS_IATU_OFFSET SZ_256M
> #define RESET_INTERVAL_MS100
This looks like a significant bug - which in turn raises the question
on how well this driver has been tested.
Dilip, can you review and ACK asap please ?
Thanks,
Lorenzo
Re: [PATCH 0/7] PCI: layerscape: Add power management support
On Mon, Sep 07, 2020 at 01:37:54PM +0800, Zhiqiang Hou wrote: > From: Hou Zhiqiang > > This patch series is to add PCIe power management support for NXP > Layerscape platfroms. > > Hou Zhiqiang (7): > PCI: dwc: Fix a bug of the case dw_pci->ops is NULL > PCI: layerscape: Change to use the DWC common link-up check function > dt-bindings: pci: layerscape-pci: Add a optional property big-endian > arm64: dts: layerscape: Add big-endian property for PCIe nodes > dt-bindings: pci: layerscape-pci: Update the description of SCFG > property > dts: arm64: ls1043a: Add SCFG phandle for PCIe nodes > PCI: layerscape: Add power management support > > .../bindings/pci/layerscape-pci.txt | 6 +- > .../arm64/boot/dts/freescale/fsl-ls1012a.dtsi | 1 + > .../arm64/boot/dts/freescale/fsl-ls1043a.dtsi | 6 + > .../arm64/boot/dts/freescale/fsl-ls1046a.dtsi | 3 + > drivers/pci/controller/dwc/pci-layerscape.c | 473 ++ > drivers/pci/controller/dwc/pcie-designware.c | 12 +- > drivers/pci/controller/dwc/pcie-designware.h | 1 + > 7 files changed, 388 insertions(+), 114 deletions(-) I don't know which patches are still applicable, I will mark this series as superseded since you will have to rebase it anyway - please let me know what's the plan. Thanks, Lorenzo
Re: [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for ->get_features()
On Tue, Jan 19, 2021 at 03:25:10PM +0530, Shradha Todi wrote: > > -Original Message- > > From: Leon Romanovsky > > Subject: Re: [PATCH v4] PCI: endpoint: Fix NULL pointer dereference for - > > >get_features() > > > > On Tue, Jan 12, 2021 at 07:32:25PM +0530, Shradha Todi wrote: > > > get_features ops of pci_epc_ops may return NULL, causing NULL pointer > > > dereference in pci_epf_test_bind function. Let us add a check for > > > pci_epc_feature pointer in pci_epf_test_bind before we access it to > > > avoid any such NULL pointer dereference and return -ENOTSUPP in case > > > pci_epc_feature is not found. > > > > > > When the patch is not applied and EPC features is not implemented in > > > the platform driver, we see the following dump due to kernel NULL > > > pointer dereference. > > > > > > [ 105.135936] Call trace: > > > [ 105.138363] pci_epf_test_bind+0xf4/0x388 [ 105.142354] > > > pci_epf_bind+0x3c/0x80 [ 105.145817] pci_epc_epf_link+0xa8/0xcc [ > > > 105.149632] configfs_symlink+0x1a4/0x48c [ 105.153616] > > > vfs_symlink+0x104/0x184 [ 105.157169] do_symlinkat+0x80/0xd4 [ > > > 105.160636] __arm64_sys_symlinkat+0x1c/0x24 [ 105.164885] > > > el0_svc_common.constprop.3+0xb8/0x170 > > > [ 105.169649] el0_svc_handler+0x70/0x88 [ 105.173377] > > > el0_svc+0x8/0x640 [ 105.176411] Code: d2800581 b9403ab9 f9404ebb > > > 8b394f60 (f9400400) [ 105.182478] ---[ end trace a438e3c5a24f9df0 > > > ]--- > > > > > > Description and call trace don't correlate with the proposed code change. > > > > The code in pci_epf_test_bind() doesn't dereference epc_features, at least > in > > direct manner. > > > > Thanks > > Thanks for the review. Yes, you're right. The dereference does not happen in > the pci_epf_test_bind() itself, but in pci_epf_test_alloc_space() being > called within. We will update the line "causing NULL pointer dereference in > pci_epf_test_bind function. " in the commit message to "causing NULL pointer > dereference in pci_epf_test_alloc_space function. " Would that be good > enough? Remove the timestamp information as well from the commit log, that's completely irrelevant information. I shall mark this as "changes requested", waiting for a new version (please keep the review tags). Lorenzo
Re: [PATCH v4 0/4] AM64: Add PCIe bindings and driver support
On Mon, 8 Mar 2021 12:05:46 +0530, Kishon Vijay Abraham I wrote: > AM64 uses the same PCIe controller as in J7200, however AM642 EVM > doesn't have a clock generator (unlike J7200 base board). Here > the clock from the SERDES has to be routed to the PCIE connector. > This series provides an option for the pci-j721e.c driver to > drive reference clock output to the connector. > > v1 of the patch series can be found @ [1] > v2 of the patch series can be found @ [2] > v3 of the patch series can be found @ [3] > > [...] Applied to pci/cadence, thanks! [1/4] dt-bindings: PCI: ti,j721e: Add binding to represent refclk to the connector https://git.kernel.org/lpieralisi/pci/c/f9875d1a36 [2/4] dt-bindings: PCI: ti,j721e: Add host mode dt-bindings for TI's AM64 SoC https://git.kernel.org/lpieralisi/pci/c/3201f355e9 [3/4] dt-bindings: PCI: ti,j721e: Add endpoint mode dt-bindings for TI's AM64 SoC https://git.kernel.org/lpieralisi/pci/c/6b7d5394c2 [4/4] PCI: j721e: Add support to provide refclk to PCIe connector https://git.kernel.org/lpieralisi/pci/c/49e0efdce7 Thanks, Lorenzo
Re: [PATCH] PCI: mobiveil: Improve PCIE_LAYERSCAPE_GEN4 dependencies
On Mon, 8 Feb 2021 15:23:01 +0100, Geert Uytterhoeven wrote: > - Drop the dependency on PCI, as this is implied by the dependency on > PCI_MSI_IRQ_DOMAIN, > - Drop the dependencies on OF and ARM64, as the driver compiles fine > without OF and/or on other architectures, > - The Freescale Layerscape PCIe Gen4 controller is present only on > Freescale Layerscape SoCs. Hence depend on ARCH_LAYERSCAPE, to > prevent asking the user about this driver when configuring a kernel > without Freescale Layerscape support, unless compile-testing. Applied to pci/misc, thanks! [1/1] PCI: mobiveil: Improve PCIE_LAYERSCAPE_GEN4 dependencies https://git.kernel.org/lpieralisi/pci/c/021a90fe60 Thanks, Lorenzo
Re: [PATCH] PCI:tegra:Correct typo for PCIe endpoint mode in Tegra194
On Thu, 31 Dec 2020 11:25:39 +0800, Wesley Sheng wrote: > In config PCIE_TEGRA194_EP the mode incorrectly referred to > host mode. Applied to pci/tegra, thanks! [1/1] PCI: tegra: Fix typo for PCIe endpoint mode in Tegra194 https://git.kernel.org/lpieralisi/pci/c/10739e2a5e Thanks, Lorenzo
Re: [PATCH] PCI: dwc: Move forward the iATU detection process
On Mon, Mar 22, 2021 at 06:03:57PM +, Lorenzo Pieralisi wrote:
> On Mon, 25 Jan 2021 12:48:03 +0800, Zhiqiang Hou wrote:
> > In the dw_pcie_ep_init(), it depends on the detected iATU region
> > numbers to allocate the in/outbound window management bit map.
> > It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number
> > of iATU windows").
> >
> > So this patch move the iATU region detection into a new function,
> > move forward the detection to the very beginning of functions
> > dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it
> > from the dw_pcie_setup(), since it's more like a software
> > perspective initialization step than hardware setup.
>
> Applied to pci/dwc, thanks!
>
> [1/1] PCI: dwc: Move forward the iATU detection process
> https://git.kernel.org/lpieralisi/pci/c/5ff8ca16f8
Bjorn will queue it for v5.12, so I have dropped it, ignore
this notification.
Thanks,
Lorenzo
Re: [PATCH] PCI: dwc: Move forward the iATU detection process
On Mon, 25 Jan 2021 12:48:03 +0800, Zhiqiang Hou wrote:
> In the dw_pcie_ep_init(), it depends on the detected iATU region
> numbers to allocate the in/outbound window management bit map.
> It fails after the commit 281f1f99cf3a ("PCI: dwc: Detect number
> of iATU windows").
>
> So this patch move the iATU region detection into a new function,
> move forward the detection to the very beginning of functions
> dw_pcie_host_init() and dw_pcie_ep_init(). And also remove it
> from the dw_pcie_setup(), since it's more like a software
> perspective initialization step than hardware setup.
Applied to pci/dwc, thanks!
[1/1] PCI: dwc: Move forward the iATU detection process
https://git.kernel.org/lpieralisi/pci/c/5ff8ca16f8
Thanks,
Lorenzo
Re: [PATCH] PCI: keystone: Let AM65 use the pci_ops defined in pcie-designware-host.c
On Wed, 17 Mar 2021 18:45:18 +0530, Kishon Vijay Abraham I wrote:
> Both TI's AM65x (K3) and TI's K2 PCIe driver are implemented in
> pci-keystone. However Only K2 PCIe driver should use it's own pci_ops
> for configuration space accesses. But commit 10a797c6e54a
> ("PCI: dwc: keystone: Use pci_ops for config space accessors") used
> custom pci_ops for both AM65x and K2. This breaks configuration space
> access for AM65x platform. Fix it here.
Applied to pci/dwc, thanks!
[1/1] PCI: keystone: Let AM65 use the pci_ops defined in pcie-designware-host.c
https://git.kernel.org/lpieralisi/pci/c/3d0b2a3a87
Thanks,
Lorenzo
Re: [PATCH -next] pci/controller/dwc: convert comma to semicolon
On Mon, Mar 22, 2021 at 01:40:16PM +, Roy Zang wrote: > Yes. It is maintained. To be maintained you should review its code please. > I will send out a patch. Krzysztof already posted one for you, you just need to ack it: https://patchwork.kernel.org/project/linux-pci/patch/20210311033745.1547044-1...@linux.com For the future email exchanges: don't top-post please. Thanks, Lorenzo > Thanks. > Roy > > -Original Message----- > From: Lorenzo Pieralisi > > On Sun, Mar 07, 2021 at 07:36:57PM +0100, Krzysztof Wilczyński wrote: > > Hi, > > > > [...] > > > I would request NXP maintainers to take this patch, rewrite it as > > > Bjorn requested and resend it as fast as possible, this is a very > > > relevant fix. > > [...] > > > > Looking at the state of the pci-layerscape-ep.c file in Linus' tree, > > this still hasn't been fixed, and it has been a while. > > > > NXP folks, are you intend to pick this up? Do let us know. > > Minghuan, Mingkai, Roy, > > either one of you reply and follow up this patch or I will have to update the > MAINTAINERS entry and take action accordingly, you are not maintaining this > driver and I won't maintain your code, sorry. > > Lorenzo > > > Krzysztof
Re: [PATCH] PCI: iproc: Fix return value of iproc_msi_irq_domain_alloc()
On Wed, 3 Mar 2021 15:22:02 +0100, Pali Rohár wrote: > IRQ domain alloc function should return zero on success. Non-zero value > indicates failure. Applied to pci/iproc, thanks! [1/1] PCI: iproc: Fix return value of iproc_msi_irq_domain_alloc() https://git.kernel.org/lpieralisi/pci/c/1e83130f01 Thanks, Lorenzo
Re: [PATCH] PCI: tegra: Constify static structs
On Sun, 7 Feb 2021 23:16:04 +0100, Rikard Falkeborn wrote: > The only usage of them is to assign their address to the 'ops' field in > the pcie_port and the dw_pcie_ep structs, both which are pointers to > const. Make them const to allow the compiler to put them in read-only > memory. Applied to pci/tegra, thanks! [1/1] PCI: tegra: Constify static structs https://git.kernel.org/lpieralisi/pci/c/d895ce7030 Thanks, Lorenzo
Re: [PATCH -next] pci/controller/dwc: convert comma to semicolon
On Sun, Mar 07, 2021 at 07:36:57PM +0100, Krzysztof Wilczyński wrote: > Hi, > > [...] > > I would request NXP maintainers to take this patch, rewrite it as > > Bjorn requested and resend it as fast as possible, this is a very > > relevant fix. > [...] > > Looking at the state of the pci-layerscape-ep.c file in Linus' tree, > this still hasn't been fixed, and it has been a while. > > NXP folks, are you intend to pick this up? Do let us know. Minghuan, Mingkai, Roy, either one of you reply and follow up this patch or I will have to update the MAINTAINERS entry and take action accordingly, you are not maintaining this driver and I won't maintain your code, sorry. Lorenzo > Krzysztof
Re: [PATCH 03/13] PCI: xilinx: Convert to MSI domains
On Thu, Feb 25, 2021 at 03:10:13PM +, Marc Zyngier wrote:
> In anticipation of the removal of the msi_controller structure, convert
> the ancient xilinx 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 xilinx-specific and handling
> the actual HW interrupt allocation.
>
> This allows us to fix some of the most appaling MSI programming, where
> the message programmed in the device is the virtual IRQ number instead
> of the allocated vector number. The allocator is also made safe with
> a mutex. This should allow support for MultiMSI, but I decided not to
> even try, since I cannot test it.
>
> Also take the opportunity to get rid of the cargo-culted memory allocation
> for the MSI capture address. *ANY* sufficiently aligned address should
> be good enough, so use the physical address of the xilinx_pcie_host
> structure instead.
I'd agree with Bjorn that the MSI doorbell change is better split into
a separate patch, I can do it myself at merge if you agree.
Thanks,
Lorenzo
>
> Signed-off-by: Marc Zyngier
> ---
> drivers/pci/controller/Kconfig | 2 +-
> drivers/pci/controller/pcie-xilinx.c | 238 +++
> 2 files changed, 96 insertions(+), 144 deletions(-)
>
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index ccbf034512d6..049c60016904 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -95,7 +95,7 @@ config PCI_HOST_GENERIC
> config PCIE_XILINX
> bool "Xilinx AXI PCIe host bridge support"
> depends on OF || COMPILE_TEST
> - select PCI_MSI_ARCH_FALLBACKS
> + depends on PCI_MSI_IRQ_DOMAIN
> help
> Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
> Host Bridge driver.
> diff --git a/drivers/pci/controller/pcie-xilinx.c
> b/drivers/pci/controller/pcie-xilinx.c
> index fa5baeb82653..ad9abf405167 100644
> --- a/drivers/pci/controller/pcie-xilinx.c
> +++ b/drivers/pci/controller/pcie-xilinx.c
> @@ -93,25 +93,23 @@
> /**
> * struct xilinx_pcie_port - PCIe port information
> * @reg_base: IO Mapped Register Base
> - * @irq: Interrupt number
> - * @msi_pages: MSI pages
> * @dev: Device pointer
> + * @msi_map: Bitmap of allocated MSIs
> + * @map_lock: Mutex protecting the MSI allocation
> * @msi_domain: MSI IRQ domain pointer
> * @leg_domain: Legacy IRQ domain pointer
> * @resources: Bus Resources
> */
> struct xilinx_pcie_port {
> void __iomem *reg_base;
> - u32 irq;
> - unsigned long msi_pages;
> struct device *dev;
> + unsigned long msi_map[BITS_TO_LONGS(XILINX_NUM_MSI_IRQS)];
> + struct mutex map_lock;
> struct irq_domain *msi_domain;
> struct irq_domain *leg_domain;
> struct list_head resources;
> };
>
> -static DECLARE_BITMAP(msi_irq_in_use, XILINX_NUM_MSI_IRQS);
> -
> static inline u32 pcie_read(struct xilinx_pcie_port *port, u32 reg)
> {
> return readl(port->reg_base + reg);
> @@ -196,151 +194,108 @@ static struct pci_ops xilinx_pcie_ops = {
>
> /* MSI functions */
>
> -/**
> - * xilinx_pcie_destroy_msi - Free MSI number
> - * @irq: IRQ to be freed
> - */
> -static void xilinx_pcie_destroy_msi(unsigned int irq)
> +static struct irq_chip xilinx_msi_top_chip = {
> + .name = "PCIe MSI",
> +};
> +
> +static int xilinx_msi_set_affinity(struct irq_data *d, const struct cpumask
> *mask, bool force)
> {
> - struct msi_desc *msi;
> - struct xilinx_pcie_port *port;
> - struct irq_data *d = irq_get_irq_data(irq);
> - irq_hw_number_t hwirq = irqd_to_hwirq(d);
> -
> - if (!test_bit(hwirq, msi_irq_in_use)) {
> - msi = irq_get_msi_desc(irq);
> - port = msi_desc_to_pci_sysdata(msi);
> - dev_err(port->dev, "Trying to free unused MSI#%d\n", irq);
> - } else {
> - clear_bit(hwirq, msi_irq_in_use);
> - }
> + return -EINVAL;
> }
>
> -/**
> - * xilinx_pcie_assign_msi - Allocate MSI number
> - *
> - * Return: A valid IRQ on success and error value on failure.
> - */
> -static int xilinx_pcie_assign_msi(void)
> +static void xilinx_compose_msi_msg(struct irq_data *data, struct msi_msg
> *msg)
> {
> - int pos;
> + struct xilinx_pcie_port *pcie = irq_data_get_irq_chip_data(data);
> + phys_addr_t pa = virt_to_phys(pcie);
>
> - pos = find_first_zero_bit(msi_irq_in_use, XILINX_NUM_MSI_IRQS);
> - if (pos < XILINX_NUM_MSI_IRQS)
> - set_bit(pos, msi_irq_in_use);
> - else
> - return -ENOSPC;
> -
> - return pos;
> + msg->address_lo = lower_32_bits(pa);
> + msg->address_hi = upper_32_bits(pa);
> + msg->data = data->hwirq;
> }
>
> -/**
> - * xilinx_msi_teardown_irq - Destroy the MSI
> - * @chip: MSI Chip descriptor
> - * @irq: MSI IRQ to destroy
> - */
> -static void xilinx_msi_teardown_irq(struct msi_controller *chip,
> -
Re: [PATCH 03/13] PCI: xilinx: Convert to MSI domains
On Thu, Feb 25, 2021 at 03:10:13PM +, Marc Zyngier wrote:
> In anticipation of the removal of the msi_controller structure, convert
> the ancient xilinx 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 xilinx-specific and handling
> the actual HW interrupt allocation.
>
> This allows us to fix some of the most appaling MSI programming, where
> the message programmed in the device is the virtual IRQ number instead
> of the allocated vector number. The allocator is also made safe with
> a mutex. This should allow support for MultiMSI, but I decided not to
> even try, since I cannot test it.
>
> Also take the opportunity to get rid of the cargo-culted memory allocation
> for the MSI capture address. *ANY* sufficiently aligned address should
> be good enough, so use the physical address of the xilinx_pcie_host
> structure instead.
>
> Signed-off-by: Marc Zyngier
> ---
> drivers/pci/controller/Kconfig | 2 +-
> drivers/pci/controller/pcie-xilinx.c | 238 +++
> 2 files changed, 96 insertions(+), 144 deletions(-)
Michal,
can you please test these changes or make sure someone does and report
back on the mailing list please ?
I would like to merge this series for v5.13.
Thanks,
Lorenzo
>
> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index ccbf034512d6..049c60016904 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -95,7 +95,7 @@ config PCI_HOST_GENERIC
> config PCIE_XILINX
> bool "Xilinx AXI PCIe host bridge support"
> depends on OF || COMPILE_TEST
> - select PCI_MSI_ARCH_FALLBACKS
> + depends on PCI_MSI_IRQ_DOMAIN
> help
> Say 'Y' here if you want kernel to support the Xilinx AXI PCIe
> Host Bridge driver.
> diff --git a/drivers/pci/controller/pcie-xilinx.c
> b/drivers/pci/controller/pcie-xilinx.c
> index fa5baeb82653..ad9abf405167 100644
> --- a/drivers/pci/controller/pcie-xilinx.c
> +++ b/drivers/pci/controller/pcie-xilinx.c
> @@ -93,25 +93,23 @@
> /**
> * struct xilinx_pcie_port - PCIe port information
> * @reg_base: IO Mapped Register Base
> - * @irq: Interrupt number
> - * @msi_pages: MSI pages
> * @dev: Device pointer
> + * @msi_map: Bitmap of allocated MSIs
> + * @map_lock: Mutex protecting the MSI allocation
> * @msi_domain: MSI IRQ domain pointer
> * @leg_domain: Legacy IRQ domain pointer
> * @resources: Bus Resources
> */
> struct xilinx_pcie_port {
> void __iomem *reg_base;
> - u32 irq;
> - unsigned long msi_pages;
> struct device *dev;
> + unsigned long msi_map[BITS_TO_LONGS(XILINX_NUM_MSI_IRQS)];
> + struct mutex map_lock;
> struct irq_domain *msi_domain;
> struct irq_domain *leg_domain;
> struct list_head resources;
> };
>
> -static DECLARE_BITMAP(msi_irq_in_use, XILINX_NUM_MSI_IRQS);
> -
> static inline u32 pcie_read(struct xilinx_pcie_port *port, u32 reg)
> {
> return readl(port->reg_base + reg);
> @@ -196,151 +194,108 @@ static struct pci_ops xilinx_pcie_ops = {
>
> /* MSI functions */
>
> -/**
> - * xilinx_pcie_destroy_msi - Free MSI number
> - * @irq: IRQ to be freed
> - */
> -static void xilinx_pcie_destroy_msi(unsigned int irq)
> +static struct irq_chip xilinx_msi_top_chip = {
> + .name = "PCIe MSI",
> +};
> +
> +static int xilinx_msi_set_affinity(struct irq_data *d, const struct cpumask
> *mask, bool force)
> {
> - struct msi_desc *msi;
> - struct xilinx_pcie_port *port;
> - struct irq_data *d = irq_get_irq_data(irq);
> - irq_hw_number_t hwirq = irqd_to_hwirq(d);
> -
> - if (!test_bit(hwirq, msi_irq_in_use)) {
> - msi = irq_get_msi_desc(irq);
> - port = msi_desc_to_pci_sysdata(msi);
> - dev_err(port->dev, "Trying to free unused MSI#%d\n", irq);
> - } else {
> - clear_bit(hwirq, msi_irq_in_use);
> - }
> + return -EINVAL;
> }
>
> -/**
> - * xilinx_pcie_assign_msi - Allocate MSI number
> - *
> - * Return: A valid IRQ on success and error value on failure.
> - */
> -static int xilinx_pcie_assign_msi(void)
> +static void xilinx_compose_msi_msg(struct irq_data *data, struct msi_msg
> *msg)
> {
> - int pos;
> + struct xilinx_pcie_port *pcie = irq_data_get_irq_chip_data(data);
> + phys_addr_t pa = virt_to_phys(pcie);
>
> - pos = find_first_zero_bit(msi_irq_in_use, XILINX_NUM_MSI_IRQS);
> - if (pos < XILINX_NUM_MSI_IRQS)
> - set_bit(pos, msi_irq_in_use);
> - else
> - return -ENOSPC;
> -
> - return pos;
> + msg->address_lo = lower_32_bits(pa);
> + msg->address_hi = upper_32_bits(pa);
> + msg->data = data->hwirq;
> }
>
> -/**
> - * xilinx_msi_teardown_irq - Destroy the MSI
> - * @chip: MSI Chip descriptor
> - * @irq: MSI IRQ to destroy
> - */
> -static void xilinx_msi_teardown_irq(struct
Re: [PATCH] Documentation: arm64/acpi : clarify arm64 support of IBFT
On Tue, Mar 16, 2021 at 12:50:41PM -0600, Tom Saeger wrote:
> In commit 94bccc340710 ("iscsi_ibft: make ISCSI_IBFT dependson ACPI instead
> of ISCSI_IBFT_FIND") Kconfig was disentangled to make ISCSI_IBFT selection
> not depend on x86.
>
> Update arm64 acpi documentation, changing IBFT support status from
> "Not Supported" to "Optional".
> Opportunistically re-flow paragraph for changed lines.
>
> Link:
> https://lore.kernel.org/lkml/1563475054-10680-1-git-send-email-thomas@oracle.com/
>
> Signed-off-by: Tom Saeger
> ---
> Documentation/arm64/acpi_object_usage.rst | 10 +-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
Acked-by: Lorenzo Pieralisi
> diff --git a/Documentation/arm64/acpi_object_usage.rst
> b/Documentation/arm64/acpi_object_usage.rst
> index 377e9d224db0..0609da73970b 100644
> --- a/Documentation/arm64/acpi_object_usage.rst
> +++ b/Documentation/arm64/acpi_object_usage.rst
> @@ -17,12 +17,12 @@ For ACPI on arm64, tables also fall into the following
> categories:
>
> - Recommended: BERT, EINJ, ERST, HEST, PCCT, SSDT
>
> - - Optional: BGRT, CPEP, CSRT, DBG2, DRTM, ECDT, FACS, FPDT, IORT,
> - MCHI, MPST, MSCT, NFIT, PMTT, RASF, SBST, SLIT, SPMI, SRAT, STAO,
> - TCPA, TPM2, UEFI, XENV
> + - Optional: BGRT, CPEP, CSRT, DBG2, DRTM, ECDT, FACS, FPDT, IBFT,
> + IORT, MCHI, MPST, MSCT, NFIT, PMTT, RASF, SBST, SLIT, SPMI, SRAT,
> + STAO, TCPA, TPM2, UEFI, XENV
>
> - - Not supported: BOOT, DBGP, DMAR, ETDT, HPET, IBFT, IVRS, LPIT,
> - MSDM, OEMx, PSDT, RSDT, SLIC, WAET, WDAT, WDRT, WPBT
> + - Not supported: BOOT, DBGP, DMAR, ETDT, HPET, IVRS, LPIT, MSDM,
> OEMx,
> + PSDT, RSDT, SLIC, WAET, WDAT, WDRT, WPBT
>
> ==
>
> Table Usage for ARMv8 Linux
> --
> 2.31.0
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH] Documentation: arm64/acpi : clarify arm64 support of IBFT
[+ Al, Ard]
On Thu, Mar 18, 2021 at 10:44:33AM +, Will Deacon wrote:
> [+Lorenzo]
>
> On Tue, Mar 16, 2021 at 12:50:41PM -0600, Tom Saeger wrote:
> > In commit 94bccc340710 ("iscsi_ibft: make ISCSI_IBFT dependson ACPI instead
> > of ISCSI_IBFT_FIND") Kconfig was disentangled to make ISCSI_IBFT selection
> > not depend on x86.
> >
> > Update arm64 acpi documentation, changing IBFT support status from
> > "Not Supported" to "Optional".
> > Opportunistically re-flow paragraph for changed lines.
> >
> > Link:
> > https://lore.kernel.org/lkml/1563475054-10680-1-git-send-email-thomas@oracle.com/
> >
> > Signed-off-by: Tom Saeger
> > ---
> > Documentation/arm64/acpi_object_usage.rst | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
>
> Lorenzo, please could you ack the change below if you're happy with it?
> If so, I can take it as a fix.
I don't see any issue with this patch, more so given that the IBFT
legacy discovery method was decoupled from the ACPI table method,
so it looks sound on ARM64.
However, I would like to get Al and Ard opinions on this to make sure
there is not something I am missing (in particular wrt the rationale
behind the "Not Supported" in the docs).
Lorenzo
> Thanks,
>
> Will
>
> > diff --git a/Documentation/arm64/acpi_object_usage.rst
> > b/Documentation/arm64/acpi_object_usage.rst
> > index 377e9d224db0..0609da73970b 100644
> > --- a/Documentation/arm64/acpi_object_usage.rst
> > +++ b/Documentation/arm64/acpi_object_usage.rst
> > @@ -17,12 +17,12 @@ For ACPI on arm64, tables also fall into the following
> > categories:
> >
> > - Recommended: BERT, EINJ, ERST, HEST, PCCT, SSDT
> >
> > - - Optional: BGRT, CPEP, CSRT, DBG2, DRTM, ECDT, FACS, FPDT, IORT,
> > - MCHI, MPST, MSCT, NFIT, PMTT, RASF, SBST, SLIT, SPMI, SRAT, STAO,
> > - TCPA, TPM2, UEFI, XENV
> > + - Optional: BGRT, CPEP, CSRT, DBG2, DRTM, ECDT, FACS, FPDT, IBFT,
> > + IORT, MCHI, MPST, MSCT, NFIT, PMTT, RASF, SBST, SLIT, SPMI, SRAT,
> > + STAO, TCPA, TPM2, UEFI, XENV
> >
> > - - Not supported: BOOT, DBGP, DMAR, ETDT, HPET, IBFT, IVRS, LPIT,
> > - MSDM, OEMx, PSDT, RSDT, SLIC, WAET, WDAT, WDRT, WPBT
> > + - Not supported: BOOT, DBGP, DMAR, ETDT, HPET, IVRS, LPIT, MSDM,
> > OEMx,
> > + PSDT, RSDT, SLIC, WAET, WDAT, WDRT, WPBT
> >
> > ==
> >
> > Table Usage for ARMv8 Linux
> > --
> > 2.31.0
> >
Re: [PATCH v3 1/1] irqchip/gic-v4.1: Disable vSGI upon (GIC CPUIF < v4.1) detection
On Wed, Mar 17, 2021 at 02:04:36PM +, Marc Zyngier wrote: > Hi Lorenzo, > > Wed, 17 Mar 2021 10:07:19 +, > Lorenzo Pieralisi wrote: > > > > GIC CPU interfaces versions predating GIC v4.1 were not built to > > accommodate vINTID within the vSGI range; as reported in the GIC > > specifications (8.2 "Changes to the CPU interface"), it is > > CONSTRAINED UNPREDICTABLE to deliver a vSGI to a PE with > > ID_AA64PFR0_EL1.GIC < b0011. > > > > Check the GIC CPUIF version by reading the SYS_ID_AA64_PFR0_EL1. > > > > Disable vSGIs if a CPUIF version < 4.1 is detected to prevent using > > vSGIs on systems where they may misbehave. > > > > Signed-off-by: Lorenzo Pieralisi > > Cc: Marc Zyngier > > Does it need to go in as a fix? or can it just be pushed into 5.13? > Given that there is no such HW in the wild just yet, I'm inclined to > do the latter... I agree with you; it is to make the driver/vgic robust against HW misconfigurations (because that's what they are and I am not aware of any _existing_ HW with such a misconfiguration - yet), 5.13 will perfectly do. Thanks a lot. Lorenzo
[PATCH v3 1/1] irqchip/gic-v4.1: Disable vSGI upon (GIC CPUIF < v4.1) detection
GIC CPU interfaces versions predating GIC v4.1 were not built to
accommodate vINTID within the vSGI range; as reported in the GIC
specifications (8.2 "Changes to the CPU interface"), it is
CONSTRAINED UNPREDICTABLE to deliver a vSGI to a PE with
ID_AA64PFR0_EL1.GIC < b0011.
Check the GIC CPUIF version by reading the SYS_ID_AA64_PFR0_EL1.
Disable vSGIs if a CPUIF version < 4.1 is detected to prevent using
vSGIs on systems where they may misbehave.
Signed-off-by: Lorenzo Pieralisi
Cc: Marc Zyngier
---
arch/arm64/kvm/vgic/vgic-mmio-v3.c | 4 ++--
drivers/irqchip/irq-gic-v4.c | 27 +--
include/linux/irqchip/arm-gic-v4.h | 2 ++
3 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index 15a6c98ee92f..2f1b156021a6 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -86,7 +86,7 @@ static unsigned long vgic_mmio_read_v3_misc(struct kvm_vcpu
*vcpu,
}
break;
case GICD_TYPER2:
- if (kvm_vgic_global_state.has_gicv4_1)
+ if (kvm_vgic_global_state.has_gicv4_1 && gic_cpuif_has_vsgi())
value = GICD_TYPER2_nASSGIcap;
break;
case GICD_IIDR:
@@ -119,7 +119,7 @@ static void vgic_mmio_write_v3_misc(struct kvm_vcpu *vcpu,
dist->enabled = val & GICD_CTLR_ENABLE_SS_G1;
/* Not a GICv4.1? No HW SGIs */
- if (!kvm_vgic_global_state.has_gicv4_1)
+ if (!kvm_vgic_global_state.has_gicv4_1 || !gic_cpuif_has_vsgi())
val &= ~GICD_CTLR_nASSGIreq;
/* Dist stays enabled? nASSGIreq is RO */
diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
index 5d1dc9915272..4ea71b28f9f5 100644
--- a/drivers/irqchip/irq-gic-v4.c
+++ b/drivers/irqchip/irq-gic-v4.c
@@ -87,17 +87,40 @@ static struct irq_domain *gic_domain;
static const struct irq_domain_ops *vpe_domain_ops;
static const struct irq_domain_ops *sgi_domain_ops;
+#ifdef CONFIG_ARM64
+#include
+
+bool gic_cpuif_has_vsgi(void)
+{
+ unsigned long fld, reg = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
+
+ fld = cpuid_feature_extract_unsigned_field(reg, ID_AA64PFR0_GIC_SHIFT);
+
+ return fld >= 0x3;
+}
+#else
+bool gic_cpuif_has_vsgi(void)
+{
+ return false;
+}
+#endif
+
static bool has_v4_1(void)
{
return !!sgi_domain_ops;
}
+static bool has_v4_1_sgi(void)
+{
+ return has_v4_1() && gic_cpuif_has_vsgi();
+}
+
static int its_alloc_vcpu_sgis(struct its_vpe *vpe, int idx)
{
char *name;
int sgi_base;
- if (!has_v4_1())
+ if (!has_v4_1_sgi())
return 0;
name = kasprintf(GFP_KERNEL, "GICv4-sgi-%d", task_pid_nr(current));
@@ -182,7 +205,7 @@ static void its_free_sgi_irqs(struct its_vm *vm)
{
int i;
- if (!has_v4_1())
+ if (!has_v4_1_sgi())
return;
for (i = 0; i < vm->nr_vpes; i++) {
diff --git a/include/linux/irqchip/arm-gic-v4.h
b/include/linux/irqchip/arm-gic-v4.h
index 943c3411ca10..2c63375bbd43 100644
--- a/include/linux/irqchip/arm-gic-v4.h
+++ b/include/linux/irqchip/arm-gic-v4.h
@@ -145,4 +145,6 @@ int its_init_v4(struct irq_domain *domain,
const struct irq_domain_ops *vpe_ops,
const struct irq_domain_ops *sgi_ops);
+bool gic_cpuif_has_vsgi(void);
+
#endif
--
2.29.1
[PATCH v3 0/1] GIC v4.1: Disable VSGI support for GIC CPUIF < v4.1
This patchset is v3 of a previous version [1]. v2 -> v3: - Coalesced all checks in one function (Marc's feedback) - Allow sgi_ops on cpuif mismatch (to keep v4.1 doorbell mechanism that works fine even if GIC CPUIF < v4.1) v1 -> v2: - Fixed vGIC behaviour according to v1 [1] review - Removed capability detection - rely on sanitised reg read - Added vsgi specific flag (for gic and kvm) [1] https://lore.kernel.org/linux-arm-kernel/20210302102744.12692-1-lorenzo.pieral...@arm.com -- Original cover letter -- GIC v4.1 introduced changes to the GIC CPU interface; systems that integrate CPUs that do not support GIC v4.1 features (as reported in the ID_AA64PFR0_EL1.GIC bitfield) and a GIC v4.1 controller must disable in software virtual SGIs support since the CPUIF and GIC controller version mismatch results in CONSTRAINED UNPREDICTABLE behaviour at architectural level. For systems with CPUs reporting ID_AA64PFR0_EL1.GIC == b0001 integrated in a system with a GIC v4.1 it _should_ still be safe to enable vLPIs (other than vSGI) since the protocol between the GIC redistributor and the GIC CPUIF was not changed from GIC v4.0 to GIC v4.1. Cc: Marc Zyngier Lorenzo Pieralisi (1): irqchip/gic-v4.1: Disable vSGI upon (GIC CPUIF < v4.1) detection arch/arm64/kvm/vgic/vgic-mmio-v3.c | 4 ++-- drivers/irqchip/irq-gic-v4.c | 27 +-- include/linux/irqchip/arm-gic-v4.h | 2 ++ 3 files changed, 29 insertions(+), 4 deletions(-) -- 2.29.1
Re: [PATCH v2 1/1] irqchip/gic-v4.1: Disable vSGI upon (GIC CPUIF < v4.1) detection
On Mon, Mar 08, 2021 at 07:22:57PM +, Marc Zyngier wrote:
> Hi Lorenzo,
>
> On Tue, 02 Mar 2021 10:27:44 +,
> Lorenzo Pieralisi wrote:
> >
> > GIC CPU interfaces versions predating GIC v4.1 were not built to
> > accommodate vINTID within the vSGI range; as reported in the GIC
> > specifications (8.2 "Changes to the CPU interface"), it is
> > CONSTRAINED UNPREDICTABLE to deliver a vSGI to a PE with
> > ID_AA64PFR0_EL1.GIC < b0011.
> >
> > Check the GIC CPUIF version by reading the SYS_ID_AA64_PFR0_EL1.
> >
> > Disable vSGIs if a CPUIF version < 4.1 is detected to prevent using
> > vSGIs on systems where they may misbehave.
> >
> > Signed-off-by: Lorenzo Pieralisi
> > Cc: Marc Zyngier
> > ---
> > arch/arm64/kvm/vgic/vgic-mmio-v3.c | 4 ++--
> > arch/arm64/kvm/vgic/vgic-v3.c | 3 ++-
> > drivers/irqchip/irq-gic-v3-its.c | 6 +-
> > drivers/irqchip/irq-gic-v3.c | 22 ++
> > include/kvm/arm_vgic.h | 1 +
> > include/linux/irqchip/arm-gic-common.h | 2 ++
> > include/linux/irqchip/arm-gic-v3.h | 1 +
> > 7 files changed, 35 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> > b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> > index 15a6c98ee92f..66548cd2a715 100644
> > --- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> > +++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
> > @@ -86,7 +86,7 @@ static unsigned long vgic_mmio_read_v3_misc(struct
> > kvm_vcpu *vcpu,
> > }
> > break;
> > case GICD_TYPER2:
> > - if (kvm_vgic_global_state.has_gicv4_1)
> > + if (kvm_vgic_global_state.has_gicv4_1_vsgi)
> > value = GICD_TYPER2_nASSGIcap;
> > break;
> > case GICD_IIDR:
> > @@ -119,7 +119,7 @@ static void vgic_mmio_write_v3_misc(struct kvm_vcpu
> > *vcpu,
> > dist->enabled = val & GICD_CTLR_ENABLE_SS_G1;
> >
> > /* Not a GICv4.1? No HW SGIs */
> > - if (!kvm_vgic_global_state.has_gicv4_1)
> > + if (!kvm_vgic_global_state.has_gicv4_1_vsgi)
> > val &= ~GICD_CTLR_nASSGIreq;
> >
> > /* Dist stays enabled? nASSGIreq is RO */
> > diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> > index 52915b342351..57b73100e8cc 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v3.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> > @@ -533,7 +533,7 @@ int vgic_v3_map_resources(struct kvm *kvm)
> > return ret;
> > }
> >
> > - if (kvm_vgic_global_state.has_gicv4_1)
> > + if (kvm_vgic_global_state.has_gicv4_1_vsgi)
> > vgic_v4_configure_vsgis(kvm);
> >
> > return 0;
> > @@ -589,6 +589,7 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
> > if (info->has_v4) {
> > kvm_vgic_global_state.has_gicv4 = gicv4_enable;
> > kvm_vgic_global_state.has_gicv4_1 = info->has_v4_1 &&
> > gicv4_enable;
> > + kvm_vgic_global_state.has_gicv4_1_vsgi = info->has_v4_1_vsgi &&
> > gicv4_enable;
> > kvm_info("GICv4%s support %sabled\n",
> > kvm_vgic_global_state.has_gicv4_1 ? ".1" : "",
> > gicv4_enable ? "en" : "dis");
> > diff --git a/drivers/irqchip/irq-gic-v3-its.c
> > b/drivers/irqchip/irq-gic-v3-its.c
> > index ed46e6057e33..ee2a2ca27d5c 100644
> > --- a/drivers/irqchip/irq-gic-v3-its.c
> > +++ b/drivers/irqchip/irq-gic-v3-its.c
> > @@ -5412,7 +5412,11 @@ int __init its_init(struct fwnode_handle *handle,
> > struct rdists *rdists,
> > if (has_v4 & rdists->has_vlpis) {
> > const struct irq_domain_ops *sgi_ops;
> >
> > - if (has_v4_1)
> > + /*
> > +* Enable vSGIs only if the ITS and the
> > +* GIC CPUIF support them.
> > +*/
> > + if (has_v4_1 && rdists->has_vsgi_cpuif)
> > sgi_ops = _sgi_domain_ops;
> > else
> > sgi_ops = NULL;
>
> This doesn't seem right. If you pass NULL for the SGI ops, you also
> lose the per-VPE doorbells and stick to the terrible GICv4.0 behaviour
> (see the use of has_v4_1() in irq-gic-v4.c). I don't think that is
> what you really want.
Yes, I was caught out again - we use the sgi_ops to detect v4.1 behavi
[PATCH v2 1/1] irqchip/gic-v4.1: Disable vSGI upon (GIC CPUIF < v4.1) detection
GIC CPU interfaces versions predating GIC v4.1 were not built to
accommodate vINTID within the vSGI range; as reported in the GIC
specifications (8.2 "Changes to the CPU interface"), it is
CONSTRAINED UNPREDICTABLE to deliver a vSGI to a PE with
ID_AA64PFR0_EL1.GIC < b0011.
Check the GIC CPUIF version by reading the SYS_ID_AA64_PFR0_EL1.
Disable vSGIs if a CPUIF version < 4.1 is detected to prevent using
vSGIs on systems where they may misbehave.
Signed-off-by: Lorenzo Pieralisi
Cc: Marc Zyngier
---
arch/arm64/kvm/vgic/vgic-mmio-v3.c | 4 ++--
arch/arm64/kvm/vgic/vgic-v3.c | 3 ++-
drivers/irqchip/irq-gic-v3-its.c | 6 +-
drivers/irqchip/irq-gic-v3.c | 22 ++
include/kvm/arm_vgic.h | 1 +
include/linux/irqchip/arm-gic-common.h | 2 ++
include/linux/irqchip/arm-gic-v3.h | 1 +
7 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index 15a6c98ee92f..66548cd2a715 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -86,7 +86,7 @@ static unsigned long vgic_mmio_read_v3_misc(struct kvm_vcpu
*vcpu,
}
break;
case GICD_TYPER2:
- if (kvm_vgic_global_state.has_gicv4_1)
+ if (kvm_vgic_global_state.has_gicv4_1_vsgi)
value = GICD_TYPER2_nASSGIcap;
break;
case GICD_IIDR:
@@ -119,7 +119,7 @@ static void vgic_mmio_write_v3_misc(struct kvm_vcpu *vcpu,
dist->enabled = val & GICD_CTLR_ENABLE_SS_G1;
/* Not a GICv4.1? No HW SGIs */
- if (!kvm_vgic_global_state.has_gicv4_1)
+ if (!kvm_vgic_global_state.has_gicv4_1_vsgi)
val &= ~GICD_CTLR_nASSGIreq;
/* Dist stays enabled? nASSGIreq is RO */
diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 52915b342351..57b73100e8cc 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -533,7 +533,7 @@ int vgic_v3_map_resources(struct kvm *kvm)
return ret;
}
- if (kvm_vgic_global_state.has_gicv4_1)
+ if (kvm_vgic_global_state.has_gicv4_1_vsgi)
vgic_v4_configure_vsgis(kvm);
return 0;
@@ -589,6 +589,7 @@ int vgic_v3_probe(const struct gic_kvm_info *info)
if (info->has_v4) {
kvm_vgic_global_state.has_gicv4 = gicv4_enable;
kvm_vgic_global_state.has_gicv4_1 = info->has_v4_1 &&
gicv4_enable;
+ kvm_vgic_global_state.has_gicv4_1_vsgi = info->has_v4_1_vsgi &&
gicv4_enable;
kvm_info("GICv4%s support %sabled\n",
kvm_vgic_global_state.has_gicv4_1 ? ".1" : "",
gicv4_enable ? "en" : "dis");
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index ed46e6057e33..ee2a2ca27d5c 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -5412,7 +5412,11 @@ int __init its_init(struct fwnode_handle *handle, struct
rdists *rdists,
if (has_v4 & rdists->has_vlpis) {
const struct irq_domain_ops *sgi_ops;
- if (has_v4_1)
+ /*
+* Enable vSGIs only if the ITS and the
+* GIC CPUIF support them.
+*/
+ if (has_v4_1 && rdists->has_vsgi_cpuif)
sgi_ops = _sgi_domain_ops;
else
sgi_ops = NULL;
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index eb0ee356a629..fd6cd9a5de34 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -31,6 +31,21 @@
#include "irq-gic-common.h"
+#ifdef CONFIG_ARM64
+#include
+
+static inline bool gic_cpuif_has_vsgi(void)
+{
+ unsigned long fld, reg = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
+
+ fld = cpuid_feature_extract_unsigned_field(reg, ID_AA64PFR0_GIC_SHIFT);
+
+ return fld >= 0x3;
+}
+#else
+static inline bool gic_cpuif_has_vsgi(void) { return false; }
+#endif
+
#define GICD_INT_NMI_PRI (GICD_INT_DEF_PRI & ~0x80)
#define FLAGS_WORKAROUND_GICR_WAKER_MSM8996(1ULL << 0)
@@ -1679,6 +1694,8 @@ static int __init gic_init_bases(void __iomem *dist_base,
gic_data.rdists.has_direct_lpi = true;
gic_data.rdists.has_vpend_valid_dirty = true;
+ gic_data.rdists.has_vsgi_cpuif = gic_cpuif_has_vsgi();
+
if (WARN_ON(!gic_data.domain) || WARN_ON(!gic_data.rdists.rdist)) {
err = -ENOMEM;
goto out_free;
@@ -1852,6 +1869,9 @@ static void __init gic_of_setup_kvm_info(struct
device_node *node)
gic_v3_kv
[PATCH v2 0/1] GIC v4.1: Disable VSGI support for GIC CPUIF < v4.1
This patchset is v2 of a previous version [1]. v1 -> v2: - Fixed vGIC behaviour according to v1 [1] review - Removed capability detection - rely on sanitised reg read - Added vsgi specific flag (for gic and kvm) [1] https://lore.kernel.org/linux-arm-kernel/2020162841.3151-1-lorenzo.pieral...@arm.com -- Original cover letter -- GIC v4.1 introduced changes to the GIC CPU interface; systems that integrate CPUs that do not support GIC v4.1 features (as reported in the ID_AA64PFR0_EL1.GIC bitfield) and a GIC v4.1 controller must disable in software virtual SGIs support since the CPUIF and GIC controller version mismatch results in CONSTRAINED UNPREDICTABLE behaviour at architectural level. For systems with CPUs reporting ID_AA64PFR0_EL1.GIC == b0001 integrated in a system with a GIC v4.1 it _should_ still be safe to enable vLPIs (other than vSGI) since the protocol between the GIC redistributor and the GIC CPUIF was not changed from GIC v4.0 to GIC v4.1. Lorenzo Pieralisi (1): irqchip/gic-v4.1: Disable vSGI upon (GIC CPUIF < v4.1) detection arch/arm64/kvm/vgic/vgic-mmio-v3.c | 4 ++-- arch/arm64/kvm/vgic/vgic-v3.c | 3 ++- drivers/irqchip/irq-gic-v3-its.c | 6 +- drivers/irqchip/irq-gic-v3.c | 22 ++ include/kvm/arm_vgic.h | 1 + include/linux/irqchip/arm-gic-common.h | 2 ++ include/linux/irqchip/arm-gic-v3.h | 1 + 7 files changed, 35 insertions(+), 4 deletions(-) -- 2.29.1
Re: [PATCH] arm64: PCI: Enable SMC conduit
On Thu, Feb 18, 2021 at 12:43:30PM -0500, Jon Masters wrote: > Hi Bjorn, all, > > On Thu, Jan 28, 2021 at 6:31 PM Bjorn Helgaas wrote: > > On Tue, Jan 26, 2021 at 10:46:04AM -0600, Jeremy Linton wrote: > > > > > Does that mean its open season for ECAM quirks, and we can expect > > them to start being merged now? > > "Open season" makes me cringe because it suggests we have a license to > use quirks indiscriminately forever, and I hope that's not the case. > > Lorenzo is closer to this issue than I am and has much better insight > into the mess this could turn into. From my point of view, it's > shocking how much of a hassle this is compared to x86. There just > aren't ECAM quirks, in-kernel clock management, or any of that crap. > I don't know how they do it on x86 and I don't have to care. Whatever > they need to do, they apparently do in AML. Eventually ARM64 has to > get there as well if vendors want distro support. > > I don't want to be in the position of enforcing a draconian "no more > quirks ever" policy. The intent -- to encourage/force vendors to > develop spec-compliant machines -- is good, but it seems like the > reward of having compliant machines "just work" vs the penalty of > having to write quirks and shepherd them upstream and into distros > will probably be more effective and not much slower. > > > The problem is that the third party IP vendors (still) make too much junk. For > years, there wasn't a compliance program (e.g. SystemReady with some of the > meat behind PCI-SIG compliance) and even when there was the third party IP > vendors building "root ports" (not even RCs) would make some junk with a > hacked > up Linux kernel booting on a model and demo that as "PCI". There wasn't the > kind of adult supervision that was required. It is (slowly) happening now, but > it's years and years late. It's just embarrassing to see the lack of ECAM that > works. In many cases, it's because the IP being used was baked years ago or > made for some "non server" (as if there is such a thing) use case, etc. But in > others, there was a chance to do it right, and it still happens. Some of us > have lost what hair we had over the years getting third party IP vendors to > wake up and start caring about this. > > So there's no excuse. None at all. However, this is where we are. And it /is/ > improving. But it's still too slow, and we have platforms still coming to > market that need to boot and run. Based on this, and the need to have > something > more flexible than just solving for ECAM deficiencies (which are really just a > symptom), I can see the allure of an SMC. I don't like it, but if that's where > folks want to go, and if we can find a way to constrain the enthusiasm for it, > then perhaps it is a path forward. But if we are to go down that path it needs > to come with a giant warning from the kernel that a system was booted at is > relying on that. Something that will cause an OS certification program to fail > without a waiver, or will cause customers to phone up for support wondering > why > the hw is broken. It *must* not be a silent thing. It needs to be "this > hardware is broken and non-standard, get the next version fixed". It is a stance I agree with in many respects, it should be shared (it was in HTML format - the lists unfortunately dropped the message) so I am replying to it to make it public. Thanks, Lorenzo
Re: [PATCH -next] NTB: Drop kfree for memory allocated with devm_kzalloc
On Wed, Feb 10, 2021 at 07:53:45AM +, Wei Yongjun wrote:
> It's not necessary to free memory allocated with devm_kzalloc
> and using kfree leads to a double free.
>
> Fixes: 363baf7d6051 ("NTB: Add support for EPF PCI-Express Non-Transparent
> Bridge")
Squashed it in the commit it is fixing and repushed pci/ntb out.
Thanks,
Lorenzo
> Reported-by: Hulk Robot
> Signed-off-by: Wei Yongjun
> ---
> drivers/ntb/hw/epf/ntb_hw_epf.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c
> index 2cccb7dff5dd..b019755e4e21 100644
> --- a/drivers/ntb/hw/epf/ntb_hw_epf.c
> +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c
> @@ -723,7 +723,6 @@ static void ntb_epf_pci_remove(struct pci_dev *pdev)
> ntb_unregister_device(>ntb);
> ntb_epf_cleanup_isr(ndev);
> ntb_epf_deinit_pci(ndev);
> - kfree(ndev);
> }
>
> static const struct ntb_epf_data j721e_data = {
>
Re: [PATCH v13 6/7] arm64: mte: Report async tag faults before suspend
On Fri, Feb 12, 2021 at 12:00:15PM +, Lorenzo Pieralisi wrote:
> On Thu, Feb 11, 2021 at 03:33:52PM +, Vincenzo Frascino wrote:
> > When MTE async mode is enabled TFSR_EL1 contains the accumulative
> > asynchronous tag check faults for EL1 and EL0.
> >
> > During the suspend/resume operations the firmware might perform some
> > operations that could change the state of the register resulting in
> > a spurious tag check fault report.
> >
> > Report asynchronous tag faults before suspend and clear the TFSR_EL1
> > register after resume to prevent this to happen.
> >
> > Cc: Catalin Marinas
> > Cc: Will Deacon
> > Cc: Lorenzo Pieralisi
> > Signed-off-by: Vincenzo Frascino
> > ---
> > arch/arm64/include/asm/mte.h | 4
> > arch/arm64/kernel/mte.c | 20
> > arch/arm64/kernel/suspend.c | 3 +++
> > 3 files changed, 27 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> > index 43169b978cd3..33e88a470357 100644
> > --- a/arch/arm64/include/asm/mte.h
> > +++ b/arch/arm64/include/asm/mte.h
> > @@ -41,6 +41,7 @@ void mte_sync_tags(pte_t *ptep, pte_t pte);
> > void mte_copy_page_tags(void *kto, const void *kfrom);
> > void flush_mte_state(void);
> > void mte_thread_switch(struct task_struct *next);
> > +void mte_suspend_enter(void);
> > void mte_suspend_exit(void);
> > long set_mte_ctrl(struct task_struct *task, unsigned long arg);
> > long get_mte_ctrl(struct task_struct *task);
> > @@ -66,6 +67,9 @@ static inline void flush_mte_state(void)
> > static inline void mte_thread_switch(struct task_struct *next)
> > {
> > }
> > +static inline void mte_suspend_enter(void)
> > +{
> > +}
> > static inline void mte_suspend_exit(void)
> > {
> > }
> > diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> > index f5aa5bea6dfe..de905102245a 100644
> > --- a/arch/arm64/kernel/mte.c
> > +++ b/arch/arm64/kernel/mte.c
> > @@ -258,12 +258,32 @@ void mte_thread_switch(struct task_struct *next)
> > mte_check_tfsr_el1();
> > }
> >
> > +void mte_suspend_enter(void)
> > +{
> > + if (!system_supports_mte())
> > + return;
> > +
> > + /*
> > +* The barriers are required to guarantee that the indirect writes
> > +* to TFSR_EL1 are synchronized before we report the state.
> > +*/
> > + dsb(nsh);
> > + isb();
> > +
> > + /* Report SYS_TFSR_EL1 before suspend entry */
> > + mte_check_tfsr_el1();
> > +}
> > +
> > void mte_suspend_exit(void)
> > {
> > if (!system_supports_mte())
> > return;
> >
> > update_gcr_el1_excl(gcr_kernel_excl);
> > +
> > + /* Clear SYS_TFSR_EL1 after suspend exit */
> > + write_sysreg_s(0, SYS_TFSR_EL1);
>
> AFAICS it is not needed, it is done already in __cpu_setup() (that is
> called by cpu_resume on return from cpu_suspend() from firmware).
>
> However, I have a question. We are relying on context switch to set
> sctlr_el1_tfc0 right ? If that's the case, till the thread resuming from
> low power switches context we are running with SCTLR_EL1_TCF0 not
> reflecting the actual value.
Forget this, we obviously restore sctlr_el1 on resume (cpu_do_resume()).
With the line above removed:
Reviewed-by: Lorenzo Pieralisi
> Just making sure that I understand it correctly, I need to check the
> resume from suspend-to-RAM path, it is something that came up with perf
> save/restore already in the past.
>
> Lorenzo
>
> > +
> > }
> >
> > long set_mte_ctrl(struct task_struct *task, unsigned long arg)
> > diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
> > index a67b37a7a47e..25a02926ad88 100644
> > --- a/arch/arm64/kernel/suspend.c
> > +++ b/arch/arm64/kernel/suspend.c
> > @@ -91,6 +91,9 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned
> > long))
> > unsigned long flags;
> > struct sleep_stack_data state;
> >
> > + /* Report any MTE async fault before going to suspend */
> > + mte_suspend_enter();
> > +
> > /*
> > * From this point debug exceptions are disabled to prevent
> > * updates to mdscr register (saved and restored along with
> > --
> > 2.30.0
> >
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v13 6/7] arm64: mte: Report async tag faults before suspend
On Thu, Feb 11, 2021 at 03:33:52PM +, Vincenzo Frascino wrote:
> When MTE async mode is enabled TFSR_EL1 contains the accumulative
> asynchronous tag check faults for EL1 and EL0.
>
> During the suspend/resume operations the firmware might perform some
> operations that could change the state of the register resulting in
> a spurious tag check fault report.
>
> Report asynchronous tag faults before suspend and clear the TFSR_EL1
> register after resume to prevent this to happen.
>
> Cc: Catalin Marinas
> Cc: Will Deacon
> Cc: Lorenzo Pieralisi
> Signed-off-by: Vincenzo Frascino
> ---
> arch/arm64/include/asm/mte.h | 4
> arch/arm64/kernel/mte.c | 20
> arch/arm64/kernel/suspend.c | 3 +++
> 3 files changed, 27 insertions(+)
>
> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> index 43169b978cd3..33e88a470357 100644
> --- a/arch/arm64/include/asm/mte.h
> +++ b/arch/arm64/include/asm/mte.h
> @@ -41,6 +41,7 @@ void mte_sync_tags(pte_t *ptep, pte_t pte);
> void mte_copy_page_tags(void *kto, const void *kfrom);
> void flush_mte_state(void);
> void mte_thread_switch(struct task_struct *next);
> +void mte_suspend_enter(void);
> void mte_suspend_exit(void);
> long set_mte_ctrl(struct task_struct *task, unsigned long arg);
> long get_mte_ctrl(struct task_struct *task);
> @@ -66,6 +67,9 @@ static inline void flush_mte_state(void)
> static inline void mte_thread_switch(struct task_struct *next)
> {
> }
> +static inline void mte_suspend_enter(void)
> +{
> +}
> static inline void mte_suspend_exit(void)
> {
> }
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index f5aa5bea6dfe..de905102245a 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -258,12 +258,32 @@ void mte_thread_switch(struct task_struct *next)
> mte_check_tfsr_el1();
> }
>
> +void mte_suspend_enter(void)
> +{
> + if (!system_supports_mte())
> + return;
> +
> + /*
> + * The barriers are required to guarantee that the indirect writes
> + * to TFSR_EL1 are synchronized before we report the state.
> + */
> + dsb(nsh);
> + isb();
> +
> + /* Report SYS_TFSR_EL1 before suspend entry */
> + mte_check_tfsr_el1();
> +}
> +
> void mte_suspend_exit(void)
> {
> if (!system_supports_mte())
> return;
>
> update_gcr_el1_excl(gcr_kernel_excl);
> +
> + /* Clear SYS_TFSR_EL1 after suspend exit */
> + write_sysreg_s(0, SYS_TFSR_EL1);
AFAICS it is not needed, it is done already in __cpu_setup() (that is
called by cpu_resume on return from cpu_suspend() from firmware).
However, I have a question. We are relying on context switch to set
sctlr_el1_tfc0 right ? If that's the case, till the thread resuming from
low power switches context we are running with SCTLR_EL1_TCF0 not
reflecting the actual value.
Just making sure that I understand it correctly, I need to check the
resume from suspend-to-RAM path, it is something that came up with perf
save/restore already in the past.
Lorenzo
> +
> }
>
> long set_mte_ctrl(struct task_struct *task, unsigned long arg)
> diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
> index a67b37a7a47e..25a02926ad88 100644
> --- a/arch/arm64/kernel/suspend.c
> +++ b/arch/arm64/kernel/suspend.c
> @@ -91,6 +91,9 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
> unsigned long flags;
> struct sleep_stack_data state;
>
> + /* Report any MTE async fault before going to suspend */
> + mte_suspend_enter();
> +
> /*
>* From this point debug exceptions are disabled to prevent
>* updates to mdscr register (saved and restored along with
> --
> 2.30.0
>
Re: [PATCH v8 0/2] PCI: cadence: Retrain Link to work around Gen2
On Tue, 9 Feb 2021 15:46:20 +0100, Nadeem Athani wrote: > Cadence controller will not initiate autonomous speed change if strapped > as Gen2. The Retrain Link bit is set as quirk to enable this speed change. > Adding a quirk flag for defective IP. In future IP revisions this will not > be applicable. > > Version history: > Changes in v8: > - Adding a new function cdns_pcie_host_start_link(). > Changes in v7: > - Changing the commit title of patch 1 in this series. > - Added a return value for function cdns_pcie_retrain(). > Changes in v6: > - Move the position of function cdns_pcie_host_wait_for_link to remove > compilation error. No changes in code. Separate patch for this. > Changes in v5: > - Remove the compatible string based setting of quirk flag. > - Removed additional Link Up Check > - Removed quirk from pcie-cadence-plat.c and added in pci-j721e.c > Changes in v4: > - Added a quirk flag based on a new compatible string. > - Change of api for link up: cdns_pcie_host_wait_for_link(). > Changes in v3: > - To set retrain link bit,checking device capability & link status. > - 32bit read in place of 8bit. > - Minor correction in patch comment. > - Change in variable & macro name. > Changes in v2: > - 16bit read in place of 8bit. > > [...] Applied to pci/cadence, squashed two commits together since it makes no sense to keep them separate. Also, please check: git log --oneline when writing patches to keep the changes uniform, I had to edit your commit. Thanks, Lorenzo
Re: [PATCH v12 6/7] arm64: mte: Save/Restore TFSR_EL1 during suspend
On Tue, Feb 09, 2021 at 11:55:33AM +, Catalin Marinas wrote: > On Mon, Feb 08, 2021 at 04:56:16PM +, Vincenzo Frascino wrote: > > When MTE async mode is enabled TFSR_EL1 contains the accumulative > > asynchronous tag check faults for EL1 and EL0. > > > > During the suspend/resume operations the firmware might perform some > > operations that could change the state of the register resulting in > > a spurious tag check fault report. > > > > Save/restore the state of the TFSR_EL1 register during the > > suspend/resume operations to prevent this to happen. > > Do we need a similar fix for TFSRE0_EL1? We get away with this if > suspend is only entered on the idle (kernel) thread but I recall we > could also enter suspend on behalf of a user process (I may be wrong > though). Yes, when we suspend the machine to RAM, we execute suspend on behalf on a userspace process (but that's only running on 1 cpu, the others are hotplugged out). IIUC (and that's an if) TFSRE0_EL1 is checked on kernel entry so I don't think there is a need to save/restore it (just reset it on suspend exit). TFSR_EL1, I don't see a point in saving/restoring it (it is a bit per-CPU AFAICS) either, IMO we should "check" it on suspend (if it is possible in that context) and reset it on resume. I don't think though you can "check" with IRQs disabled so I suspect that TFSR_EL1 has to be saved/restored (which means that there is a black out period where we run kernel code without being able to detect faults but there is no solution to that other than delaying saving the value to just before calling into PSCI). Likewise on resume from low power. Thanks, Lorenzo > If that's the case, it would make more sense to store the TFSR* regs in > the thread_struct alongside sctlr_tcf0. If we did that, we'd not need > the per-cpu mte_suspend_tfsr_el1 variable. > > -- > Catalin
Re: [PATCH v12 6/7] arm64: mte: Save/Restore TFSR_EL1 during suspend
On Mon, Feb 08, 2021 at 04:56:16PM +, Vincenzo Frascino wrote:
> When MTE async mode is enabled TFSR_EL1 contains the accumulative
> asynchronous tag check faults for EL1 and EL0.
>
> During the suspend/resume operations the firmware might perform some
> operations that could change the state of the register resulting in
> a spurious tag check fault report.
>
> Save/restore the state of the TFSR_EL1 register during the
> suspend/resume operations to prevent this to happen.
>
> Cc: Catalin Marinas
> Cc: Will Deacon
> Cc: Lorenzo Pieralisi
> Signed-off-by: Vincenzo Frascino
> ---
> arch/arm64/include/asm/mte.h | 4
> arch/arm64/kernel/mte.c | 22 ++
> arch/arm64/kernel/suspend.c | 3 +++
> 3 files changed, 29 insertions(+)
>
> diff --git a/arch/arm64/include/asm/mte.h b/arch/arm64/include/asm/mte.h
> index 237bb2f7309d..2d79bcaaeb30 100644
> --- a/arch/arm64/include/asm/mte.h
> +++ b/arch/arm64/include/asm/mte.h
> @@ -43,6 +43,7 @@ void mte_sync_tags(pte_t *ptep, pte_t pte);
> void mte_copy_page_tags(void *kto, const void *kfrom);
> void flush_mte_state(void);
> void mte_thread_switch(struct task_struct *next);
> +void mte_suspend_enter(void);
> void mte_suspend_exit(void);
> long set_mte_ctrl(struct task_struct *task, unsigned long arg);
> long get_mte_ctrl(struct task_struct *task);
> @@ -68,6 +69,9 @@ static inline void flush_mte_state(void)
> static inline void mte_thread_switch(struct task_struct *next)
> {
> }
> +static inline void mte_suspend_enter(void)
> +{
> +}
> static inline void mte_suspend_exit(void)
> {
> }
> diff --git a/arch/arm64/kernel/mte.c b/arch/arm64/kernel/mte.c
> index 3332aabda466..5c440967721b 100644
> --- a/arch/arm64/kernel/mte.c
> +++ b/arch/arm64/kernel/mte.c
> @@ -25,6 +25,7 @@
>
> u64 gcr_kernel_excl __ro_after_init;
>
> +static u64 mte_suspend_tfsr_el1;
IIUC you need this per-CPU (core loses context on suspend-to-RAM but also
CPUidle, S2R is single threaded but CPUidle runs on every core idle
thread).
Unless you sync/report it on enter/exit (please note: I am not familiar
with MTE so it is just a, perhaps silly, suggestion to avoid
saving/restoring it).
Lorenzo
> static bool report_fault_once = true;
>
> /* Whether the MTE asynchronous mode is enabled. */
> @@ -295,12 +296,33 @@ void mte_thread_switch(struct task_struct *next)
> mte_check_tfsr_el1();
> }
>
> +void mte_suspend_enter(void)
> +{
> + if (!system_supports_mte())
> + return;
> +
> + /*
> + * The barriers are required to guarantee that the indirect writes
> + * to TFSR_EL1 are synchronized before we save the state.
> + */
> + dsb(nsh);
> + isb();
> +
> + /* Save SYS_TFSR_EL1 before suspend entry */
> + mte_suspend_tfsr_el1 = read_sysreg_s(SYS_TFSR_EL1);
> +}
> +
> void mte_suspend_exit(void)
> {
> if (!system_supports_mte())
> return;
>
> update_gcr_el1_excl(gcr_kernel_excl);
> +
> + /* Resume SYS_TFSR_EL1 after suspend exit */
> + write_sysreg_s(mte_suspend_tfsr_el1, SYS_TFSR_EL1);
> +
> + mte_check_tfsr_el1();
> }
>
> long set_mte_ctrl(struct task_struct *task, unsigned long arg)
> diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c
> index a67b37a7a47e..16caa9b32dae 100644
> --- a/arch/arm64/kernel/suspend.c
> +++ b/arch/arm64/kernel/suspend.c
> @@ -91,6 +91,9 @@ int cpu_suspend(unsigned long arg, int (*fn)(unsigned long))
> unsigned long flags;
> struct sleep_stack_data state;
>
> + /* Report any MTE async fault before going to suspend. */
> + mte_suspend_enter();
> +
> /*
>* From this point debug exceptions are disabled to prevent
>* updates to mdscr register (saved and restored along with
> --
> 2.30.0
>
Re: [PATCH v7 2/2] PCI: cadence: Retrain Link to work around Gen2 training defect.
On Wed, Dec 30, 2020 at 01:05:15PM +0100, Nadeem Athani wrote:
> Cadence controller will not initiate autonomous speed change if strapped
> as Gen2. The Retrain Link bit is set as quirk to enable this speed change.
>
> Signed-off-by: Nadeem Athani
> ---
> drivers/pci/controller/cadence/pci-j721e.c | 3 ++
> drivers/pci/controller/cadence/pcie-cadence-host.c | 37
> +-
> drivers/pci/controller/cadence/pcie-cadence.h | 11 ++-
> 3 files changed, 49 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/cadence/pci-j721e.c
> b/drivers/pci/controller/cadence/pci-j721e.c
> index dac1ac8a7615..849f1e416ea5 100644
> --- a/drivers/pci/controller/cadence/pci-j721e.c
> +++ b/drivers/pci/controller/cadence/pci-j721e.c
> @@ -64,6 +64,7 @@ enum j721e_pcie_mode {
>
> struct j721e_pcie_data {
> enum j721e_pcie_modemode;
> + bool quirk_retrain_flag;
> };
>
> static inline u32 j721e_pcie_user_readl(struct j721e_pcie *pcie, u32 offset)
> @@ -280,6 +281,7 @@ static struct pci_ops cdns_ti_pcie_host_ops = {
>
> static const struct j721e_pcie_data j721e_pcie_rc_data = {
> .mode = PCI_MODE_RC,
> + .quirk_retrain_flag = true,
> };
>
> static const struct j721e_pcie_data j721e_pcie_ep_data = {
> @@ -388,6 +390,7 @@ static int j721e_pcie_probe(struct platform_device *pdev)
>
> bridge->ops = _ti_pcie_host_ops;
> rc = pci_host_bridge_priv(bridge);
> + rc->quirk_retrain_flag = data->quirk_retrain_flag;
>
> cdns_pcie = >pcie;
> cdns_pcie->dev = dev;
> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c
> b/drivers/pci/controller/cadence/pcie-cadence-host.c
> index 9f7aa718c8d4..f3496588862d 100644
> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c
> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
> @@ -94,6 +94,35 @@ static int cdns_pcie_host_wait_for_link(struct cdns_pcie
> *pcie)
> return -ETIMEDOUT;
> }
>
> +static int cdns_pcie_retrain(struct cdns_pcie *pcie)
> +{
> + u32 lnk_cap_sls, pcie_cap_off = CDNS_PCIE_RP_CAP_OFFSET;
> + u16 lnk_stat, lnk_ctl;
> + int ret = 0;
> +
> + /*
> + * Set retrain bit if current speed is 2.5 GB/s,
> + * but the PCIe root port support is > 2.5 GB/s.
> + */
> +
> + lnk_cap_sls = cdns_pcie_readl(pcie, (CDNS_PCIE_RP_BASE + pcie_cap_off +
> + PCI_EXP_LNKCAP));
> + if ((lnk_cap_sls & PCI_EXP_LNKCAP_SLS) <= PCI_EXP_LNKCAP_SLS_2_5GB)
> + return ret;
> +
> + lnk_stat = cdns_pcie_rp_readw(pcie, pcie_cap_off + PCI_EXP_LNKSTA);
> + if ((lnk_stat & PCI_EXP_LNKSTA_CLS) == PCI_EXP_LNKSTA_CLS_2_5GB) {
> + lnk_ctl = cdns_pcie_rp_readw(pcie,
> + pcie_cap_off + PCI_EXP_LNKCTL);
> + lnk_ctl |= PCI_EXP_LNKCTL_RL;
> + cdns_pcie_rp_writew(pcie, pcie_cap_off + PCI_EXP_LNKCTL,
> + lnk_ctl);
> +
> + ret = cdns_pcie_host_wait_for_link(pcie);
> + }
> + return ret;
> +}
> +
> static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc)
> {
> struct cdns_pcie *pcie = >pcie;
> @@ -457,8 +486,14 @@ int cdns_pcie_host_setup(struct cdns_pcie_rc *rc)
> }
>
> ret = cdns_pcie_host_wait_for_link(pcie);
> - if (ret)
> + if (ret) {
> dev_dbg(dev, "PCIe link never came up\n");
> + } else {
> + if (rc->quirk_retrain_flag) {
> + if (cdns_pcie_retrain(pcie))
> + dev_dbg(dev, "PCIe link never came up\n");
I'd move this whole if/else in a function cdns_pcie_host_start_link(),
IMO that's cleaner.
static int cdns_pcie_host_start_link(struct cdns_pcie_rc *rc)
{
struct cdns_pcie *pcie = >pcie;
int ret;
ret = cdns_pcie_host_wait_for_link(pcie);
/*
* PLS ADD A COMMENT HERE
*/
if (!ret && rc->quirk_retrain_flag)
ret = cdns_pcie_retrain(pcie);
return ret;
}
Re: [PATCH -next] PCI: endpoint: fix build error, EP NTB driver uses configfs
On Thu, Feb 04, 2021 at 07:15:39PM +0530, Kishon Vijay Abraham I wrote:
> Hi Lorenzo,
>
> On 04/02/21 3:28 pm, Lorenzo Pieralisi wrote:
> > On Tue, Feb 02, 2021 at 12:12:55PM -0800, Randy Dunlap wrote:
> >> The pci-epf-ntb driver uses configfs APIs, so it should depend on
> >> CONFIGFS_FS to prevent build errors.
> >>
> >> ld: drivers/pci/endpoint/functions/pci-epf-ntb.o: in function
> >> `epf_ntb_add_cfs':
> >> pci-epf-ntb.c:(.text+0x1b): undefined reference to
> >> `config_group_init_type_name'
> >>
> >> Fixes: 7dc64244f9e9 ("PCI: endpoint: Add EP function driver to provide NTB
> >> functionality")
> >>
> >> Signed-off-by: Randy Dunlap
> >> Cc: Kishon Vijay Abraham I
> >> Cc: Lorenzo Pieralisi
> >> Cc: linux-...@vger.kernel.org
> >> ---
> >> You may switch to 'select CONFIG_FS_FS' if you feel strongly about it.
> >
> > Kishon ?
>
> There seems to be some issue in the version that got merged. The v11
> patch series had this fixed [1] by using select CONFIGFS_FS. However
> whatever was merged seems to be v10 which didn't have select CONFIGFS_FS
> [2]. I had sent a private mail to you pointing the same (attached for
> reference, not sure if it was delivered).
I think that Bjorn has not pulled my pci/ntb branch yet (so the one
in -next is v10 indeed, my one should be v11, please check).
Thanks,
Lorenzo
> Thanks
> Kishon
>
> [1] ->
> https://lore.kernel.org/linux-doc/20210201195809.7342-14-kis...@ti.com/
>
> [2] ->
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/pci/endpoint/functions/Kconfig
> >
> > Thanks,
> > Lorenzo
> >
> >> drivers/pci/endpoint/functions/Kconfig |1 +
> >> 1 file changed, 1 insertion(+)
> >>
> >> --- linux-next-20210202.orig/drivers/pci/endpoint/functions/Kconfig
> >> +++ linux-next-20210202/drivers/pci/endpoint/functions/Kconfig
> >> @@ -16,6 +16,7 @@ config PCI_EPF_TEST
> >> config PCI_EPF_NTB
> >>tristate "PCI Endpoint NTB driver"
> >>depends on PCI_ENDPOINT
> >> + depends on CONFIGFS_FS
> >>help
> >> Select this configuration option to enable the NTB driver
> >> for PCI Endpoint. NTB driver implements NTB controller
> Date: Tue, 2 Feb 2021 21:57:37 +0530
> From: Kishon Vijay Abraham I
> To: Lorenzo Pieralisi
> Subject: Re: [PATCH v11 13/17] PCI: endpoint: Add EP function driver to
> provide NTB functionality
> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
> Thunderbird/68.10.0
>
> Hi Lorenzo,
>
> On 02/02/21 1:28 am, Kishon Vijay Abraham I wrote:
> > Add a new endpoint function driver to provide NTB functionality
> > using multiple PCIe endpoint instances.
> >
> > Signed-off-by: Kishon Vijay Abraham I
> > [a...@arndb.de: Select configfs dependency]
> > Signed-off-by: Arnd Bergmann
> > [yebi...@huawei.com: Fix unused but set variables]
> > Signed-off-by: Ye Bin
> > [geert+rene...@glider.be: Explain NTB in PCI_EPF_NTB help text]
> > Signed-off-by: Geert Uytterhoeven
> > ---
> > drivers/pci/endpoint/functions/Kconfig | 13 +
> > drivers/pci/endpoint/functions/Makefile |1 +
> > drivers/pci/endpoint/functions/pci-epf-ntb.c | 2128 ++
> > 3 files changed, 2142 insertions(+)
> > create mode 100644 drivers/pci/endpoint/functions/pci-epf-ntb.c
> >
> > diff --git a/drivers/pci/endpoint/functions/Kconfig
> > b/drivers/pci/endpoint/functions/Kconfig
> > index 8820d0f7ec77..5f1242ca2f4e 100644
> > --- a/drivers/pci/endpoint/functions/Kconfig
> > +++ b/drivers/pci/endpoint/functions/Kconfig
> > @@ -12,3 +12,16 @@ config PCI_EPF_TEST
> >for PCI Endpoint.
> >
> >If in doubt, say "N" to disable Endpoint test driver.
> > +
> > +config PCI_EPF_NTB
> > + tristate "PCI Endpoint NTB driver"
> > + depends on PCI_ENDPOINT
> > + select CONFIGFS_FS
>
> I'm seeing some difference between here and linux-next.
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/pci/endpoint/functions/Kconfig
>
> I see "select CONFIGFS_FS" missing in linux-next.
>
> Thank You,
> Kishon
>
> > + help
> > + Select this configuration option to enable the Non-Transparent
> > + Bridge (NTB) driver for PCI Endpoint. NTB driver implements NTB
> > + controller functionality using multiple PCIe endpoint instances.
> > +
Re: [PATCH v3] PCI: dwc: Add upper limit address for outbound iATU
On Tue, 2 Feb 2021 12:58:38 +0530, Shradha Todi wrote: > The size parameter is unsigned long type which can accept size > 4GB. In > that case, the upper limit address must be programmed. Add support to > program the upper limit address and set INCREASE_REGION_SIZE in case size > > 4GB. Applied to pci/dwc, thanks! [1/1] PCI: dwc: Add upper limit address for outbound iATU https://git.kernel.org/lpieralisi/pci/c/13662a07fd Thanks, Lorenzo
Re: [PATCH -next] PCI: endpoint: fix build error, EP NTB driver uses configfs
On Tue, Feb 02, 2021 at 12:12:55PM -0800, Randy Dunlap wrote:
> The pci-epf-ntb driver uses configfs APIs, so it should depend on
> CONFIGFS_FS to prevent build errors.
>
> ld: drivers/pci/endpoint/functions/pci-epf-ntb.o: in function
> `epf_ntb_add_cfs':
> pci-epf-ntb.c:(.text+0x1b): undefined reference to
> `config_group_init_type_name'
>
> Fixes: 7dc64244f9e9 ("PCI: endpoint: Add EP function driver to provide NTB
> functionality")
>
> Signed-off-by: Randy Dunlap
> Cc: Kishon Vijay Abraham I
> Cc: Lorenzo Pieralisi
> Cc: linux-...@vger.kernel.org
> ---
> You may switch to 'select CONFIG_FS_FS' if you feel strongly about it.
Kishon ?
Thanks,
Lorenzo
> drivers/pci/endpoint/functions/Kconfig |1 +
> 1 file changed, 1 insertion(+)
>
> --- linux-next-20210202.orig/drivers/pci/endpoint/functions/Kconfig
> +++ linux-next-20210202/drivers/pci/endpoint/functions/Kconfig
> @@ -16,6 +16,7 @@ config PCI_EPF_TEST
> config PCI_EPF_NTB
> tristate "PCI Endpoint NTB driver"
> depends on PCI_ENDPOINT
> + depends on CONFIGFS_FS
> help
> Select this configuration option to enable the NTB driver
> for PCI Endpoint. NTB driver implements NTB controller
Re: [PATCH v11 00/17] Implement NTB Controller using multiple PCI EP
On Tue, 2 Feb 2021 01:27:52 +0530, Kishon Vijay Abraham I wrote: > This series is about implementing SW defined Non-Transparent Bridge (NTB) > using multiple endpoint (EP) instances. This series has been tested using > 2 endpoint instances in J7 connected to J7 board on one end and DRA7 board > on the other end. However there is nothing platform specific for the NTB > functionality. > > This was presented in Linux Plumbers Conference. Link to presentation > and video can be found @ [1] > Created a video demo @ [9] > > [...] Applied to pci/ntb, thanks! [01/17] Documentation: PCI: Add specification for the *PCI NTB* function device https://git.kernel.org/lpieralisi/pci/c/051a6adf6e [02/17] PCI: endpoint: Make *_get_first_free_bar() take into account 64 bit BAR https://git.kernel.org/lpieralisi/pci/c/c0527dabcc [03/17] PCI: endpoint: Add helper API to get the 'next' unreserved BAR https://git.kernel.org/lpieralisi/pci/c/d91d6ddfd2 [04/17] PCI: endpoint: Make *_free_bar() to return error codes on failure https://git.kernel.org/lpieralisi/pci/c/b9bdfa3da3 [05/17] PCI: endpoint: Remove unused pci_epf_match_device() https://git.kernel.org/lpieralisi/pci/c/2872f07cb0 [06/17] PCI: endpoint: Add support to associate secondary EPC with EPF https://git.kernel.org/lpieralisi/pci/c/6d0b4a7f2c [07/17] PCI: endpoint: Add support in configfs to associate two EPCs with EPF https://git.kernel.org/lpieralisi/pci/c/c8e7d97270 [08/17] PCI: endpoint: Add pci_epc_ops to map MSI irq https://git.kernel.org/lpieralisi/pci/c/2bbb192338 [09/17] PCI: endpoint: Add pci_epf_ops for epf drivers to expose function specific attrs https://git.kernel.org/lpieralisi/pci/c/cea2edf604 [10/17] PCI: endpoint: Allow user to create sub-directory of 'EPF Device' directory https://git.kernel.org/lpieralisi/pci/c/1b0ef1c913 [11/17] PCI: cadence: Implement ->msi_map_irq() ops https://git.kernel.org/lpieralisi/pci/c/743a5d6309 [12/17] PCI: cadence: Configure LM_EP_FUNC_CFG based on epc->function_num_map https://git.kernel.org/lpieralisi/pci/c/54e9e441b0 [13/17] PCI: endpoint: Add EP function driver to provide NTB functionality https://git.kernel.org/lpieralisi/pci/c/e9d7f4603e [14/17] PCI: Add TI J721E device to pci ids https://git.kernel.org/lpieralisi/pci/c/7aac69682e [15/17] NTB: Add support for EPF PCI-Express Non-Transparent Bridge https://git.kernel.org/lpieralisi/pci/c/363baf7d60 [16/17] Documentation: PCI: Add configfs binding documentation for pci-ntb endpoint function https://git.kernel.org/lpieralisi/pci/c/0456a9cd0a [17/17] Documentation: PCI: Add userguide for PCI endpoint NTB function https://git.kernel.org/lpieralisi/pci/c/096ce75bf6 Thanks, Lorenzo
Re: [PATCH v2] PCI: dwc: Add upper limit address for outbound iATU
On Wed, Jan 06, 2021 at 04:20:10PM +0530, Shradha Todi wrote: > The size parameter is unsigned long type which can accept size > 4GB. In > that case, the upper limit address must be programmed. Add support to > program the upper limit address and set INCREASE_REGION_SIZE in case size > > 4GB. > > Signed-off-by: Shradha Todi > --- > v1: https://lkml.org/lkml/2020/12/20/187 > v2: >Addressed Rob's review comment and added PCI version check condition to >avoid writing to reserved registers. > > drivers/pci/controller/dwc/pcie-designware.c | 9 +++-- > drivers/pci/controller/dwc/pcie-designware.h | 1 + > 2 files changed, 8 insertions(+), 2 deletions(-) Does not apply to my pci/dwc branch, please rebase it on top of it and resend it while keeping review tags. Thanks, Lorenzo > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c > b/drivers/pci/controller/dwc/pcie-designware.c > index 74590c7..1d62ca9 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.c > +++ b/drivers/pci/controller/dwc/pcie-designware.c > @@ -290,12 +290,17 @@ static void __dw_pcie_prog_outbound_atu(struct dw_pcie > *pci, u8 func_no, > upper_32_bits(cpu_addr)); > dw_pcie_writel_dbi(pci, PCIE_ATU_LIMIT, > lower_32_bits(cpu_addr + size - 1)); > + if (pci->version >= 0x460A) > + dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_LIMIT, > +upper_32_bits(cpu_addr + size - 1)); > dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_TARGET, > lower_32_bits(pci_addr)); > dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_TARGET, > upper_32_bits(pci_addr)); > - dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type | > -PCIE_ATU_FUNC_NUM(func_no)); > + val = type | PCIE_ATU_FUNC_NUM(func_no); > + val = ((upper_32_bits(size - 1)) && (pci->version >= 0x460A)) ? > + val | PCIE_ATU_INCREASE_REGION_SIZE : val; > + dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, val); > dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE); > > /* > diff --git a/drivers/pci/controller/dwc/pcie-designware.h > b/drivers/pci/controller/dwc/pcie-designware.h > index 8b905a2..7da79eb 100644 > --- a/drivers/pci/controller/dwc/pcie-designware.h > +++ b/drivers/pci/controller/dwc/pcie-designware.h > @@ -102,6 +102,7 @@ > #define PCIE_ATU_DEV(x) FIELD_PREP(GENMASK(23, 19), x) > #define PCIE_ATU_FUNC(x) FIELD_PREP(GENMASK(18, 16), x) > #define PCIE_ATU_UPPER_TARGET0x91C > +#define PCIE_ATU_UPPER_LIMIT 0x924 > > #define PCIE_MISC_CONTROL_1_OFF 0x8BC > #define PCIE_DBI_RO_WR_ENBIT(0) > -- > 2.7.4 >
Re: [PATCH v2] PCI: dwc: Change size to u64 for EP outbound iATU
On Wed, 6 Jan 2021 16:15:00 +0530, Shradha Todi wrote: > Since outbound iATU permits size to be greater than 4GB for which the > support is also available, allow EP function to send u64 size instead of > truncating to u32. Applied to pci/dwc, thanks! [1/1] PCI: dwc: Change size to u64 for EP outbound iATU https://git.kernel.org/lpieralisi/pci/c/95a3472255 Thanks, Lorenzo
Re: [PATCH v10 00/17] Implement NTB Controller using multiple PCI EP
On Fri, Jan 29, 2021 at 06:12:56PM +0530, Kishon Vijay Abraham I wrote: > This series is about implementing SW defined Non-Transparent Bridge (NTB) > using multiple endpoint (EP) instances. This series has been tested using > 2 endpoint instances in J7 connected to J7 board on one end and DRA7 board > on the other end. However there is nothing platform specific for the NTB > functionality. > > This was presented in Linux Plumbers Conference. Link to presentation > and video can be found @ [1] > Created a video demo @ [9] > > RFC patch series can be found @ [2] > v1 patch series can be found @ [3] > v2 patch series can be found @ [4] > v3 patch series can be found @ [5] > v4 patch series can be found @ [6] > v5 patch series can be found @ [7] > v6 patch series can be found @ [8] > v7 patch series can be found @ [10] > v8 patch series can be found @ [11] > v9 patch series can be found @ [12] > > Changes from v9: > 1) Fix the typos pointed out Bjorn Helgaas > 2) Added the received Reviewed-by tags. > > Changes from v8: > 1) Do not use devm_request_irq/devm_free_irq as pci_free_irq_vectors() > has to be used after free_irq > 2) Drop "NTB: tool: Enable the NTB/PCIe link on the local or remote side > of bridge" as there is a debugfs entry to enable the link > > Changes from v7: > 1) Used values stored in ctrl_reg_bar, peer_spad_reg_bar and db_reg_bar >instead of hardcoded values in pci_iomap of ntb_hw_epf.c driver > > Changes from v6: > 1) Fixed issues when multiple NTB devices are creating using multiple >functions > 2) Fixed issue with writing scratchpad register > 3) Created a video demo @ [9] > > Changes from v5: > 1) Fixed a formatting issue in Kconfig pointed out by Randy > 2) Checked for Error or Null in pci_epc_add_epf() > > Changes from v4: > 1) Fixed error condition checks in pci_epc_add_epf() > > Changes from v3: > 1) Fixed Documentation edits suggested by Randy Dunlap > > Changes from v2: > 1) Add support for the user to create sub-directory of 'EPF Device' >directory (for endpoint function specific configuration using >configfs). > 2) Add documentation for NTB specific attributes in configfs > 3) Check for PCI_CLASS_MEMORY_RAM (PCIe class) before binding ntb_hw_epf >driver > 4) Other documentation fixes > > Changes from v1: > 1) As per Rob's comment, removed support for creating NTB function >device from DT > 2) Add support to create NTB EPF device using configfs (added support in >configfs to associate primary and secondary EPC with EPF. > > Changes from RFC: > 1) Converted the DT binding patches to YAML schema and merged the >DT binding patches together > 2) NTB documentation is converted to .rst > 3) One HOST can now interrupt the other HOST using MSI-X interrupts > 4) Added support for teardown of memory window and doorbell >configuration > 5) Add support to provide support 64-bit memory window size from >DT > > [1] -> https://linuxplumbersconf.org/event/4/contributions/395/ > [2] -> http://lore.kernel.org/r/20190926112933.8922-1-kis...@ti.com > [3] -> http://lore.kernel.org/r/20200514145927.17555-1-kis...@ti.com > [4] -> http://lore.kernel.org/r/20200611130525.22746-1-kis...@ti.com > [5] -> http://lore.kernel.org/r/20200904075052.8911-1-kis...@ti.com > [6] -> http://lore.kernel.org/r/20200915042110.3015-1-kis...@ti.com > [7] -> http://lore.kernel.org/r/20200918064227.1463-1-kis...@ti.com > [8] -> http://lore.kernel.org/r/20200924092519.17082-1-kis...@ti.com > [9] -> https://youtu.be/dLKKxrg5-rY > [10] -> http://lore.kernel.org/r/20200930153519.7282-1-kis...@ti.com > [11] -> http://lore.kernel.org/r/2020153559.19050-1-kis...@ti.com > [12] -> http://lore.kernel.org/r/20210104152909.22038-1-kis...@ti.com > > Kishon Vijay Abraham I (17): > Documentation: PCI: Add specification for the *PCI NTB* function > device > PCI: endpoint: Make *_get_first_free_bar() take into account 64 bit > BAR > PCI: endpoint: Add helper API to get the 'next' unreserved BAR > PCI: endpoint: Make *_free_bar() to return error codes on failure > PCI: endpoint: Remove unused pci_epf_match_device() > PCI: endpoint: Add support to associate secondary EPC with EPF > PCI: endpoint: Add support in configfs to associate two EPCs with EPF > PCI: endpoint: Add pci_epc_ops to map MSI irq > PCI: endpoint: Add pci_epf_ops for epf drivers to expose function > specific attrs > PCI: endpoint: Allow user to create sub-directory of 'EPF Device' > directory > PCI: cadence: Implement ->msi_map_irq() ops > PCI: cadence: Configure LM_EP_FUNC_CFG based on epc->function_num_map > PCI: endpoint: Add EP function driver to provide NTB functionality > PCI: Add TI J721E device to pci ids > NTB: Add support for EPF PCI-Express Non-Transparent Bridge > Documentation: PCI: Add configfs binding documentation for pci-ntb > endpoint function > Documentation: PCI: Add userguide for PCI endpoint NTB function > >
Re: [PATCH v9 01/17] Documentation: PCI: Add specification for the *PCI NTB* function device
On Fri, Jan 22, 2021 at 07:48:52PM +0530, Kishon Vijay Abraham I wrote: > Hi Bjorn, > > On 20/01/21 12:04 am, Bjorn Helgaas wrote: > > On Mon, Jan 04, 2021 at 08:58:53PM +0530, Kishon Vijay Abraham I wrote: > >> Add specification for the *PCI NTB* function device. The endpoint function > >> driver and the host PCI driver should be created based on this > >> specification. > >> > >> Signed-off-by: Kishon Vijay Abraham I > > > > A few typos below if there's opportunity for revisions. > > I'll fix them. Hi Kishon, if you have changes please send them along and I will re-merge the whole series. Thanks, Lorenzo > >> --- > >> Documentation/PCI/endpoint/index.rst | 1 + > >> .../PCI/endpoint/pci-ntb-function.rst | 351 ++ > >> 2 files changed, 352 insertions(+) > >> create mode 100644 Documentation/PCI/endpoint/pci-ntb-function.rst > >> > >> diff --git a/Documentation/PCI/endpoint/index.rst > >> b/Documentation/PCI/endpoint/index.rst > >> index 4ca7439fbfc9..ef6861128506 100644 > >> --- a/Documentation/PCI/endpoint/index.rst > >> +++ b/Documentation/PCI/endpoint/index.rst > >> @@ -11,5 +11,6 @@ PCI Endpoint Framework > >> pci-endpoint-cfs > >> pci-test-function > >> pci-test-howto > >> + pci-ntb-function > >> > >> function/binding/pci-test > >> diff --git a/Documentation/PCI/endpoint/pci-ntb-function.rst > >> b/Documentation/PCI/endpoint/pci-ntb-function.rst > >> new file mode 100644 > >> index ..a57908be4047 > >> --- /dev/null > >> +++ b/Documentation/PCI/endpoint/pci-ntb-function.rst > >> @@ -0,0 +1,351 @@ > >> +.. SPDX-License-Identifier: GPL-2.0 > >> + > >> += > >> +PCI NTB Function > >> += > >> + > >> +:Author: Kishon Vijay Abraham I > >> + > >> +PCI Non Transparent Bridges (NTB) allow two host systems to communicate > >> +with each other by exposing each host as a device to the other host. > >> +NTBs typically support the ability to generate interrupts on the remote > >> +machine, expose memory ranges as BARs and perform DMA. They also support > >> +scratchpads which are areas of memory within the NTB that are accessible > >> +from both machines. > >> + > >> +PCI NTB Function allows two different systems (or hosts) to communicate > >> +with each other by configurig the endpoint instances in such a way that > >> +transactions from one system is routed to the other system. > > > > s/is/are/ > > > >> +In the below diagram, PCI NTB function configures the SoC with multiple > >> +PCIe Endpoint (EP) instances in such a way that transaction from one EP > >> +controller is routed to the other EP controller. Once PCI NTB function > > > > s/transaction ... is/transactions ... are/ > > > >> +configures the SoC with multiple EP instances, HOST1 and HOST2 can > >> +communicate with each other using SoC as a bridge. > >> + > >> +.. code-block:: text > >> + > >> ++-+ +-+ > >> +| | | | > >> +|HOST1| |HOST2| > >> +| | | | > >> ++--^--+ +--^--+ > >> + | | > >> + | | > >> + +-|-|-+ > >> + | +--v--+ +--v--+ | > >> + | | | | | | > >> + | | EP | | EP | | > >> + | | CONTROLLER1 | | CONTROLLER2 | | > >> + | | <---> | | > >> + | | | | | | > >> + | | | | | | > >> + | | | SoC With Multiple EP Instances | | | > >> + | | | (Configured using NTB Function) | | | > >> + | +-+ +-+ | > >> + +-+ > >> + > >> +Constructs used for Implementing NTB > >> + > >> + > >> + 1) Config Region > >> + 2) Self Scratchpad Registers > >> + 3) Peer Scratchpad Registers > >> + 4) Doorbell Registers > >> + 5) Memory Window > >> + > >> + > >> +Config Region: > >> +-- > >> + > >> +Config Region is a construct that is specific to NTB implemented using NTB > >> +Endpoint Function Driver. The host and endpoint side NTB function driver > >> will > >> +exchange information with each other using this region. Config Region has > >> +Control/Status Registers for configuring the
Re: [PATCH v3 0/2] PCI: dwc: remove useless dw_pcie_ops
On Thu, 28 Jan 2021 14:42:13 +0800, Jisheng Zhang wrote: > Some designware based device driver especially host only driver may > work well with the default read_dbi/write_dbi/link_up implementation > in pcie-designware.c, thus remove the assumption to simplify those > drivers. > > Since v2: > - rebase to the latest pci/dwc > - add Acked-by tag > > [...] Applied to pci/dwc, thanks! [1/2] PCI: dwc: Don't assume the ops in dw_pcie always exists https://git.kernel.org/lpieralisi/pci/c/f2213e5f3b [2/2] PCI: dwc: al: Remove useless dw_pcie_ops https://git.kernel.org/lpieralisi/pci/c/05e11f20f5 Thanks, Lorenzo
Re: [PATCH v2] ACPI/IORT: Do not blindly trust DMA masks from firmware
On Thu, Jan 21, 2021 at 05:24:19PM -0800, Moritz Fischer wrote:
> Address issue observed on real world system with suboptimal IORT table
> where DMA masks of PCI devices would get set to 0 as result.
>
> iort_dma_setup() would query the root complex'/named component IORT
> entry for a DMA mask, and use that over the one the device has been
> configured with earlier.
>
> Ideally we want to use the minimum mask of what the IORT contains for
> the root complex and what the device was configured with.
>
> Fixes: 5ac65e8c8941 ("ACPI/IORT: Support address size limit for root
> complexes")
> Signed-off-by: Moritz Fischer
> ---
>
> Changes from v1:
> - Changed warning to FW_BUG
> - Warn for both Named Component or Root Complex
> - Replaced min_not_zero() with min()
>
> ---
> drivers/acpi/arm64/iort.c | 14 ++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
Acked-by: Lorenzo Pieralisi
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index d4eac6d7e9fb..2494138a6905 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1107,6 +1107,11 @@ static int nc_dma_get_range(struct device *dev, u64
> *size)
>
> ncomp = (struct acpi_iort_named_component *)node->node_data;
>
> + if (!ncomp->memory_address_limit) {
> + pr_warn(FW_BUG "Named component missing memory address
> limit\n");
> + return -EINVAL;
> + }
> +
> *size = ncomp->memory_address_limit >= 64 ? U64_MAX :
> 1ULL<memory_address_limit;
>
> @@ -1126,6 +1131,11 @@ static int rc_dma_get_range(struct device *dev, u64
> *size)
>
> rc = (struct acpi_iort_root_complex *)node->node_data;
>
> + if (!rc->memory_address_limit) {
> + pr_warn(FW_BUG "Root complex missing memory address limit\n");
> + return -EINVAL;
> + }
> +
> *size = rc->memory_address_limit >= 64 ? U64_MAX :
> 1ULL<memory_address_limit;
>
> @@ -1173,8 +1183,8 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr,
> u64 *dma_size)
> end = dmaaddr + size - 1;
> mask = DMA_BIT_MASK(ilog2(end) + 1);
> dev->bus_dma_limit = end;
> - dev->coherent_dma_mask = mask;
> - *dev->dma_mask = mask;
> + dev->coherent_dma_mask = min(dev->coherent_dma_mask, mask);
> + *dev->dma_mask = min(*dev->dma_mask, mask);
> }
>
> *dma_addr = dmaaddr;
> --
> 2.30.0
>
Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()
On Wed, Jan 20, 2021 at 11:52:46AM +0100, Michael Walle wrote:
> fw_devlink will defer the probe until all suppliers are ready. We can't
> use builtin_platform_driver_probe() because it doesn't retry after probe
> deferral. Convert it to builtin_platform_driver().
>
> Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default")
I will have to drop this Fixes: tag if you don't mind, it is not
in the mainline.
Lorenzo
> Signed-off-by: Michael Walle
> ---
> drivers/pci/controller/dwc/pci-layerscape.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-layerscape.c
> b/drivers/pci/controller/dwc/pci-layerscape.c
> index 44ad34cdc3bc..5b9c625df7b8 100644
> --- a/drivers/pci/controller/dwc/pci-layerscape.c
> +++ b/drivers/pci/controller/dwc/pci-layerscape.c
> @@ -232,7 +232,7 @@ static const struct of_device_id ls_pcie_of_match[] = {
> { },
> };
>
> -static int __init ls_pcie_probe(struct platform_device *pdev)
> +static int ls_pcie_probe(struct platform_device *pdev)
> {
> struct device *dev = >dev;
> struct dw_pcie *pci;
> @@ -271,10 +271,11 @@ static int __init ls_pcie_probe(struct platform_device
> *pdev)
> }
>
> static struct platform_driver ls_pcie_driver = {
> + .probe = ls_pcie_probe,
> .driver = {
> .name = "layerscape-pcie",
> .of_match_table = ls_pcie_of_match,
> .suppress_bind_attrs = true,
> },
> };
> -builtin_platform_driver_probe(ls_pcie_driver, ls_pcie_probe);
> +builtin_platform_driver(ls_pcie_driver);
> --
> 2.20.1
>
Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()
On Wed, 20 Jan 2021 11:52:46 +0100, Michael Walle wrote: > fw_devlink will defer the probe until all suppliers are ready. We can't > use builtin_platform_driver_probe() because it doesn't retry after probe > deferral. Convert it to builtin_platform_driver(). Applied to pci/dwc, thanks! [1/1] PCI: dwc: layerscape: Convert to builtin_platform_driver() https://git.kernel.org/lpieralisi/pci/c/538157be1e Thanks, Lorenzo
Re: [PATCH v4 0/4] arm64: rockchip: Fix PCIe ep-gpios requirement and Add Nanopi M4B
On Fri, 22 Jan 2021 00:23:17 +0800, Chen-Yu Tsai wrote: > This is v4 of my Nanopi M4B series. > > Changes since v3 include: > > - Directly return dev_err_probe() instead of having a separate return > statement > > [...] Applied to pci/rockchip (dropped dts patches 3-4), thanks! [1/2] PCI: rockchip: Make 'ep-gpios' DT property optional https://git.kernel.org/lpieralisi/pci/c/96f760cc00 [2/2] dt-bindings: arm: rockchip: Add FriendlyARM NanoPi M4B https://git.kernel.org/lpieralisi/pci/c/b205659626 Thanks, Lorenzo
Re: [PATCH] PCI: dwc: layerscape: convert to builtin_platform_driver()
On Wed, Jan 20, 2021 at 08:28:36PM +0100, Michael Walle wrote: > [RESEND, fat-fingered the buttons of my mail client and converted > all CCs to BCCs :(] > > Am 2021-01-20 20:02, schrieb Saravana Kannan: > > On Wed, Jan 20, 2021 at 6:24 AM Rob Herring wrote: > > > > > > On Wed, Jan 20, 2021 at 4:53 AM Michael Walle > > > wrote: > > > > > > > > fw_devlink will defer the probe until all suppliers are ready. We can't > > > > use builtin_platform_driver_probe() because it doesn't retry after probe > > > > deferral. Convert it to builtin_platform_driver(). > > > > > > If builtin_platform_driver_probe() doesn't work with fw_devlink, then > > > shouldn't it be fixed or removed? > > > > I was actually thinking about this too. The problem with fixing > > builtin_platform_driver_probe() to behave like > > builtin_platform_driver() is that these probe functions could be > > marked with __init. But there are also only 20 instances of > > builtin_platform_driver_probe() in the kernel: > > $ git grep ^builtin_platform_driver_probe | wc -l > > 20 > > > > So it might be easier to just fix them to not use > > builtin_platform_driver_probe(). > > > > Michael, > > > > Any chance you'd be willing to help me by converting all these to > > builtin_platform_driver() and delete builtin_platform_driver_probe()? > > If it just moving the probe function to the _driver struct and > remove the __init annotations. I could look into that. Can I drop this patch then ? Thanks, Lorenzo
Re: [PATCH] PCI: xilinx-cpm: Fix reference count leak on error path
On Wed, 20 Jan 2021 06:37:45 -0800, Pan Bian wrote: > Also drop the reference count of the node on error path. Applied to pci/xilinx, thanks! [1/1] PCI: xilinx-cpm: Fix reference count leak on error path https://git.kernel.org/lpieralisi/pci/c/ae191d2e51 Thanks, Lorenzo
Re: [PATCH dwc-next v2 0/2] PCI: dwc: remove useless dw_pcie_ops
On Fri, Nov 20, 2020 at 07:16:11PM +0800, Jisheng Zhang wrote: > Some designware based device driver especially host only driver may > work well with the default read_dbi/write_dbi/link_up implementation > in pcie-designware.c, thus remove the assumption to simplify those > drivers. > > Since v1: > - rebase to the latest dwc-next > > Jisheng Zhang (2): > PCI: dwc: Don't assume the ops in dw_pcie always exists > PCI: dwc: al: Remove useless dw_pcie_ops > > drivers/pci/controller/dwc/pcie-al.c | 4 > drivers/pci/controller/dwc/pcie-designware-ep.c | 8 +++- > drivers/pci/controller/dwc/pcie-designware-host.c | 2 +- > drivers/pci/controller/dwc/pcie-designware.c | 14 +++--- > 4 files changed, 11 insertions(+), 17 deletions(-) Would you mind rebasing it against my current pci/dwc branch please ? I shall apply it then. Thanks, Lorenzo
Re: [PATCH] pci: remove tango host controller driver
On Wed, 20 Jan 2021 16:07:29 +0100, Arnd Bergmann wrote: > The tango platform is getting removed, so the driver is no > longer needed. Applied to pci/tango, thanks! [1/1] PCI: Remove tango host controller driver https://git.kernel.org/lpieralisi/pci/c/de9427ca87 Thanks, Lorenzo
Re: [PATCH v4] PCI: brcmstb: Remove chained IRQ handler and data in one go
On Fri, 15 Jan 2021 22:15:32 +0100, Martin Kaiser wrote:
> Call irq_set_chained_handler_and_data() to clear the chained handler
> and the handler's data under irq_desc->lock.
>
> See also 2cf5a03cb29d ("PCI/keystone: Fix race in installing chained
> IRQ handler").
Applied to pci/misc, thanks!
[1/1] PCI: brcmstb: Remove chained IRQ handler and data in one go
https://git.kernel.org/lpieralisi/pci/c/5ce6697a44
Thanks,
Lorenzo
Re: [PATCH] PCI: Drop PCIE_RCAR config option
On Tue, 29 Dec 2020 17:08:48 +, Lad Prabhakar wrote: > All the defconfig files have replaced PCIE_RCAR config option with > PCIE_RCAR_HOST config option which built the same driver, so we can > now safely drop PCIE_RCAR config option. Applied to pci/misc, thanks! [1/1] PCI: Drop PCIE_RCAR config option https://git.kernel.org/lpieralisi/pci/c/ff591f7490 Thanks, Lorenzo
Re: [PATCH v9 00/17] Implement NTB Controller using multiple PCI EP
On Mon, 4 Jan 2021 20:58:52 +0530, Kishon Vijay Abraham I wrote: > This series is about implementing SW defined Non-Transparent Bridge (NTB) > using multiple endpoint (EP) instances. This series has been tested using > 2 endpoint instances in J7 connected to J7 board on one end and DRA7 board > on the other end. However there is nothing platform specific for the NTB > functionality. > > This was presented in Linux Plumbers Conference. Link to presentation > and video can be found @ [1] > Created a video demo @ [9] > > [...] Applied to pci/ntb to give it more visibility and testing, aiming for v5.12. [01/17] Documentation: PCI: Add specification for the *PCI NTB* function device https://git.kernel.org/lpieralisi/pci/c/75e6ac86ca [02/17] PCI: endpoint: Make *_get_first_free_bar() take into account 64 bit BAR https://git.kernel.org/lpieralisi/pci/c/b6c7a2a2b5 [03/17] PCI: endpoint: Add helper API to get the 'next' unreserved BAR https://git.kernel.org/lpieralisi/pci/c/43e293914d [04/17] PCI: endpoint: Make *_free_bar() to return error codes on failure https://git.kernel.org/lpieralisi/pci/c/293e2c258c [05/17] PCI: endpoint: Remove unused pci_epf_match_device() https://git.kernel.org/lpieralisi/pci/c/9a25bdab98 [06/17] PCI: endpoint: Add support to associate secondary EPC with EPF https://git.kernel.org/lpieralisi/pci/c/868fe90ea4 [07/17] PCI: endpoint: Add support in configfs to associate two EPCs with EPF https://git.kernel.org/lpieralisi/pci/c/632c92ec12 [08/17] PCI: endpoint: Add pci_epc_ops to map MSI irq https://git.kernel.org/lpieralisi/pci/c/310511a301 [09/17] PCI: endpoint: Add pci_epf_ops for epf drivers to expose function specific attrs https://git.kernel.org/lpieralisi/pci/c/34fb8ab2e3 [10/17] PCI: endpoint: Allow user to create sub-directory of 'EPF Device' directory https://git.kernel.org/lpieralisi/pci/c/3a5c112c7a [11/17] PCI: cadence: Implement ->msi_map_irq() ops https://git.kernel.org/lpieralisi/pci/c/d5c3d2ae7c [12/17] PCI: cadence: Configure LM_EP_FUNC_CFG based on epc->function_num_map https://git.kernel.org/lpieralisi/pci/c/d3f4973104 [13/17] PCI: endpoint: Add EP function driver to provide NTB functionality https://git.kernel.org/lpieralisi/pci/c/7dc64244f9 [14/17] PCI: Add TI J721E device to pci ids https://git.kernel.org/lpieralisi/pci/c/17d49876c3 [15/17] NTB: Add support for EPF PCI-Express Non-Transparent Bridge https://git.kernel.org/lpieralisi/pci/c/5d0db3f429 [16/17] Documentation: PCI: Add configfs binding documentation for pci-ntb endpoint function https://git.kernel.org/lpieralisi/pci/c/099f07051e [17/17] Documentation: PCI: Add userguide for PCI endpoint NTB function https://git.kernel.org/lpieralisi/pci/c/27f22f76c3 Thanks, Lorenzo
Re: [PATCH v4 1/3] PCI: altera-msi: Remove IRQ handler and data in one go
On Fri, 15 Jan 2021 22:24:33 +0100, Martin Kaiser wrote:
> Call irq_set_chained_handler_and_data() to clear the chained handler
> and the handler's data under irq_desc->lock.
>
> See also 2cf5a03cb29d ("PCI/keystone: Fix race in installing chained
> IRQ handler").
Applied to pci/misc, thanks!
[1/3] PCI: altera-msi: Remove IRQ handler and data in one go
https://git.kernel.org/lpieralisi/pci/c/3f0ea2360e
[2/3] PCI: dwc: Remove IRQ handler and data in one go
https://git.kernel.org/lpieralisi/pci/c/ad1cc6b75a
[3/3] PCI: xgene-msi: Fix race in installing chained irq handler
https://git.kernel.org/lpieralisi/pci/c/a93c00e5f9
Thanks,
Lorenzo
Re: [RESEND PATCH] PCI: qcom: use PHY_REFCLK_USE_PAD only for ipq8064
On Mon, 19 Oct 2020 18:55:55 +0200, Ansuel Smith wrote: > The use of PHY_REFCLK_USE_PAD introduced a regression for apq8064 > devices. It was tested that while apq doesn't require the padding, ipq > SoC must use it or the kernel hangs on boot. Applied to pci/dwc, thanks! [1/1] PCI: qcom: use PHY_REFCLK_USE_PAD only for ipq8064 https://git.kernel.org/lpieralisi/pci/c/cef11c377a Thanks, Lorenzo
Re: [PATCH 1/2] dt-bindings: pci: layerscape-pci: Add compatible strings for LX2160A rev2
On Mon, 26 Oct 2020 13:14:47 +0800, Zhiqiang Hou wrote: > Add PCIe Endpoint mode compatible string "fsl,lx2160ar2-pcie-ep" Applied to pci/dwc, thanks! [1/2] dt-bindings: pci: layerscape-pci: Add compatible strings for LX2160A rev2 https://git.kernel.org/lpieralisi/pci/c/514a39a653 [2/2] PCI: layerscape: Add EP mode support for LX2160A rev2 https://git.kernel.org/lpieralisi/pci/c/faff7b5ef5 Thanks, Lorenzo
Re: [RESEND PATCH] PCI: qcom: use PHY_REFCLK_USE_PAD only for ipq8064
On Mon, Oct 19, 2020 at 06:55:55PM +0200, Ansuel Smith wrote:
> The use of PHY_REFCLK_USE_PAD introduced a regression for apq8064
> devices. It was tested that while apq doesn't require the padding, ipq
> SoC must use it or the kernel hangs on boot.
>
> Fixes: de3c4bf6489 ("PCI: qcom: Add support for tx term offset for rev 2.1.0")
> Reported-by: Ilia Mirkin
> Signed-off-by: Ilia Mirkin
> Signed-off-by: Ansuel Smith
> Cc: sta...@vger.kernel.org # v4.19+
> ---
> drivers/pci/controller/dwc/pcie-qcom.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Need qcom maintainer's ack, thanks.
Lorenzo
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> index 3aac77a295ba..dad6e9ce66ba 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -387,7 +387,9 @@ static int qcom_pcie_init_2_1_0(struct qcom_pcie *pcie)
>
> /* enable external reference clock */
> val = readl(pcie->parf + PCIE20_PARF_PHY_REFCLK);
> - val &= ~PHY_REFCLK_USE_PAD;
> + /* USE_PAD is required only for ipq806x */
> + if (!of_device_is_compatible(node, "qcom,pcie-apq8064"))
> + val &= ~PHY_REFCLK_USE_PAD;
> val |= PHY_REFCLK_SSP_EN;
> writel(val, pcie->parf + PCIE20_PARF_PHY_REFCLK);
>
> --
> 2.27.0
>
Re: [PATCH] PCI: functions/pci-epf-test: fix missing destroy_workqueue() on error in pci_epf_test_init
On Wed, Oct 28, 2020 at 05:15:49PM +0800, Qinglang Miao wrote:
> Add the missing destroy_workqueue() before return from
> pci_epf_test_init() in the error handling case.
>
> Signed-off-by: Qinglang Miao
> ---
> drivers/pci/endpoint/functions/pci-epf-test.c | 1 +
> 1 file changed, 1 insertion(+)
Need Kishon's ack.
Lorenzo
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c
> b/drivers/pci/endpoint/functions/pci-epf-test.c
> index e4e51d884..6854f2525 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -918,6 +918,7 @@ static int __init pci_epf_test_init(void)
> ret = pci_epf_register_driver(_driver);
> if (ret) {
> pr_err("Failed to register pci epf test driver --> %d\n", ret);
> + destroy_workqueue(kpcitest_workqueue);
> return ret;
> }
>
> --
> 2.23.0
>
Re: [PATCH V3] PCI: dwc: Add support to configure for ECRC
On Wed, 30 Dec 2020 22:27:23 +0530, Vidya Sagar wrote: > DesignWare core has a TLP digest (TD) override bit in one of the control > registers of ATU. This bit also needs to be programmed for proper ECRC > functionality. This is currently identified as an issue with DesignWare > IP version 4.90a. Applied to pci/dwc, thanks! [1/1] PCI: dwc: Add support to configure for ECRC https://git.kernel.org/lpieralisi/pci/c/9b3a84d0f5 Thanks, Lorenzo
Re: [PATCH -next] pci/controller/dwc: convert comma to semicolon
On Wed, Jan 06, 2021 at 01:07:22PM -0600, Bjorn Helgaas wrote: > On Wed, Dec 16, 2020 at 09:19:44PM +0800, Zheng Yongjun wrote: > > Replace a comma between expression statements by a semicolon. > > Looks like a good fix, but read this about the changelog title: > > https://lore.kernel.org/r/20171026223701.ga25...@bhelgaas-glaptop.roam.corp.google.com I would request NXP maintainers to take this patch, rewrite it as Bjorn requested and resend it as fast as possible, this is a very relevant fix. Thanks, Lorenzo > > Signed-off-by: Zheng Yongjun > > --- > > drivers/pci/controller/dwc/pci-layerscape-ep.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c > > index 84206f265e54..917ba8d254fc 100644 > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c > > @@ -178,7 +178,7 @@ static int __init ls_pcie_ep_probe(struct > > platform_device *pdev) > > pci->dev = dev; > > pci->ops = pcie->drvdata->dw_pcie_ops; > > > > - ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4), > > + ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4); > > > > pcie->pci = pci; > > pcie->ls_epc = ls_epc; > > -- > > 2.22.0 > > > > > > ___ > > linux-arm-kernel mailing list > > linux-arm-ker...@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH] arm64: PCI: Enable SMC conduit
On Thu, Jan 07, 2021 at 04:05:48PM -0500, Jon Masters wrote: > Hi will, everyone, > > On 1/7/21 1:14 PM, Will Deacon wrote: > > > On Mon, Jan 04, 2021 at 10:57:35PM -0600, Jeremy Linton wrote: > > > Given that most arm64 platform's PCI implementations needs quirks > > > to deal with problematic config accesses, this is a good place to > > > apply a firmware abstraction. The ARM PCI SMMCCC spec details a > > > standard SMC conduit designed to provide a simple PCI config > > > accessor. This specification enhances the existing ACPI/PCI > > > abstraction and expects power, config, etc functionality is handled > > > by the platform. It also is very explicit that the resulting config > > > space registers must behave as is specified by the pci specification. > > > > > > Lets hook the normal ACPI/PCI config path, and when we detect > > > missing MADT data, attempt to probe the SMC conduit. If the conduit > > > exists and responds for the requested segment number (provided by the > > > ACPI namespace) attach a custom pci_ecam_ops which redirects > > > all config read/write requests to the firmware. > > > > > > This patch is based on the Arm PCI Config space access document @ > > > https://developer.arm.com/documentation/den0115/latest > > > > Why does firmware need to be involved with this at all? Can't we just > > quirk Linux when these broken designs show up in production? We'll need > > to modify Linux _anyway_ when the firmware interface isn't implemented > > correctly... > > I agree with Will on this. I think we want to find a way to address some > of the non-compliance concerns through quirks in Linux. However... I understand the concern and if you are asking me if this can be fixed in Linux it obviously can. The point is, at what cost for SW and maintenance - in Linux and other OSes, I think Jeremy summed it up pretty well: https://lore.kernel.org/linux-pci/61558f73-9ac8-69fe-34c1-2074dec5f...@arm.com The issue here is that what we are asked to support on ARM64 ACPI is a moving target and the target carries PCI with it. This potentially means that all drivers in: drivers/pci/controller may require an MCFG quirk and to implement it we may have to: - Define new ACPI bindings (that may need AML and that's already a showstopper for some OSes) - Require to manage clocks in the kernel (see link-up checks) - Handle PCI config space faults in the kernel Do we really want to do that ? I don't think so. Therefore we need to have a policy to define what constitutes a "reasonable" quirk and that's not objective I am afraid, however we slice it (there is no such a thing as eg 90% ECAM). The SMC is an olive branch and just to make sure it is crystal clear there won't be room for adding quirks if the implementation turns out to be broken, if a line in the sand is what we want here it is. The question is: is there a reason we must not implement SMC support given the above ? As I said and you could imagine, I am the first who has concerns over deviating from ECAM but if we do want ACPI support for platforms that are not ECAM compliant something has to be done (HW may take years to comply). > Several folks here (particularly Lorenzo) have diligently worked hard > over the past few years - and pushed their patience - to accommodate > hardware vendors with early "not quite compliant" systems. They've taken > lots of quirks that frankly shouldn't continue to be necessary were it > even remotely a priority in the vendor ecosystem to get a handle on > addressing PCIe compliance once and for all. But, again frankly, it > hasn't been enough of a priority to get this fixed. The third party IP > vendors *need* to address this, and their customers *need* to push back. > > We can't keep having a situation in which kinda-sorta compliant stuff > comes to market that would work out of the box but for whatever the quirk > is this time around. There have been multiple OS releases for the past > quite a few years on which this stuff could be tested prior to ever > taping out a chip, and so it ought not to be possible to come to market > now with an excuse that it wasn't tested. And yet here we still are. All > these years and still the message isn't quite being received properly. I > do know it takes time to make hardware, and some of it was designed years > before and is still trickling down into these threads. But I also think > there are cases where much more could have been done earlier. > > None of these vendors can possibly want this deep down. Their engineers > almost certainly realize that just having compliant ECAM would mean that > the hardware was infinitely more valuable being able to run out of the > box software that much more easily. And it's not just ECAM. Inevitably, > that is just the observable syndrome for worse issues, often with the ITS > and forcing quirked systems to have lousy legacy interrupts, etc. Alas, > this level of nuance is likely lost by the time it reaches upper > management, where
Re: [PATCH V2] PCI: dwc: Add support to configure for ECRC
On Fri, Dec 11, 2020 at 08:49:16AM -0600, Rob Herring wrote:
> On Fri, Dec 11, 2020 at 7:58 AM Vidya Sagar wrote:
> >
> > Hi Lorenzo,
> > Apologies to bug you, but wondering if you have any further comments on
> > this patch that I need to take care of?
>
> You can check the status of your patches in Patchwork:
>
> https://patchwork.kernel.org/project/linux-pci/patch/2020121145.7015-1-vid...@nvidia.com/
>
> If it's in 'New' state and delegated to Lorenzo or Bjorn, it's in
> their queue. You can shorten the queue by reviewing stuff in front of
> you. :)
Yes that's right. There are a couple of patches pending ahead, if this
one can be rebased against my pci/dwc branch and resent I can apply it.
Thanks,
Lorenzo
>
> Rob
>
> >
> > Thanks,
> > Vidya Sagar
> >
> > On 12/3/2020 5:40 PM, Vidya Sagar wrote:
> > >
> > >
> > > On 11/25/2020 2:32 AM, Bjorn Helgaas wrote:
> > >> External email: Use caution opening links or attachments
> > >>
> > >>
> > >> On Tue, Nov 24, 2020 at 03:50:01PM +0530, Vidya Sagar wrote:
> > >>> Hi Bjorn,
> > >>> Please let me know if this patch needs any further modifications
> > >>
> > >> I'm fine with it, but of course Lorenzo will take care of it.
> > > Thanks Bjorn.
> > >
> > > Hi Lorenzo,
> > > Please let me know if you have any comments for this patch.
> > >
> > > Thanks,
> > > Vidya Sagar
> > >
> > >>
> > >>> On 11/12/2020 10:32 PM, Vidya Sagar wrote:
> > External email: Use caution opening links or attachments
> >
> >
> > On 11/12/2020 3:59 AM, Bjorn Helgaas wrote:
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > On Wed, Nov 11, 2020 at 10:21:46PM +0530, Vidya Sagar wrote:
> > >>
> > >>
> > >> On 11/11/2020 9:57 PM, Jingoo Han wrote:
> > >>> External email: Use caution opening links or attachments
> > >>>
> > >>>
> > >>> On 11/11/20, 7:12 AM, Vidya Sagar wrote:
> >
> > DesignWare core has a TLP digest (TD) override bit in
> > one of the control
> > registers of ATU. This bit also needs to be programmed for
> > proper ECRC
> > functionality. This is currently identified as an issue
> > with DesignWare
> > IP version 4.90a.
> >
> > Signed-off-by: Vidya Sagar
> > Acked-by: Bjorn Helgaas
> > ---
> > V2:
> > * Addressed Bjorn's comments
> >
> > drivers/pci/controller/dwc/pcie-designware.c | 52
> > ++--
> > drivers/pci/controller/dwc/pcie-designware.h | 1 +
> > 2 files changed, 49 insertions(+), 4 deletions(-)
> >
> > diff --git
> > a/drivers/pci/controller/dwc/pcie-designware.c
> > b/drivers/pci/controller/dwc/pcie-designware.c
> > index c2dea8fc97c8..ec0d13ab6bad 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > @@ -225,6 +225,46 @@ static void
> > dw_pcie_writel_ob_unroll(struct dw_pcie *pci, u32 index,
> > u32 reg,
> > dw_pcie_writel_atu(pci, offset + reg, val);
> > }
> >
> > +static inline u32 dw_pcie_enable_ecrc(u32 val)
> > >>>
> > >>> What is the reason to use inline here?
> > >>
> > >> Actually, I wanted to move the programming part inside the
> > >> respective APIs
> > >> but then I wanted to give some details as well in comments so to
> > >> avoid
> > >> duplication, I came up with this function. But, I'm making it
> > >> inline for
> > >> better code optimization by compiler.
> > >
> > > I don't really care either way, but I'd be surprised if the compiler
> > > didn't inline this all by itself even without the explicit "inline".
> > I just checked it and you are right that compiler is indeed inlining it
> > without explicitly mentioning 'inline'.
> > I hope it is ok to leave it that way.
> >
> > >
> > +{
> > + /*
> > + * DesignWare core version 4.90A has this strange design
> > issue
> > + * where the 'TD' bit in the Control register-1 of
> > the ATU outbound
> > + * region acts like an override for the ECRC
> > setting i.e. the presence
> > + * of TLP Digest(ECRC) in the outgoing TLPs is
> > solely determined by
> > + * this bit. This is contrary to the PCIe spec
> > which says that the
> > + * enablement of the ECRC is solely determined by
> > the AER registers.
> > + *
> > + * Because of this, even when the ECRC is enabled through AER
> > + * registers, the transactions going through ATU
> > won't have TLP Digest
> > + * as there is no way the AER sub-system could
> >
Re: [RESEND PATCH 0/4] PCI: J7: J7200/J721E PCIe bindings
On Thu, 10 Dec 2020 18:19:13 +0530, Kishon Vijay Abraham I wrote: > Patch series adds PCIe binding for J7200 and and fixes > "ti,syscon-pcie-ctrl" applicable to both J721E and J7200. > > All the four patches here have got Acks from Rob Herring. > > Ack for "dt-bindings: pci: ti,j721e: Fix "ti,syscon-pcie-ctrl" to take > argument" > lore.kernel.org/r/CAL_JsqJQju8TUZA-wu=WA-5XH4H9s2ifO8Hf4TnT5epa=gg...@mail.gmail.com > > [...] Applied to pci/cadence, thanks! [1/4] dt-bindings: pci: ti,j721e: Fix "ti,syscon-pcie-ctrl" to take argument https://git.kernel.org/lpieralisi/pci/c/b6c81be912 [2/4] dt-bindings: PCI: Add host mode dt-bindings for TI's J7200 SoC https://git.kernel.org/lpieralisi/pci/c/3f1f870c01 [3/4] dt-bindings: PCI: Add EP mode dt-bindings for TI's J7200 SoC https://git.kernel.org/lpieralisi/pci/c/17c5b458a9 [4/4] PCI: j721e: Get offset within "syscon" from "ti,syscon-pcie-ctrl" phandle arg https://git.kernel.org/lpieralisi/pci/c/7aa256234c Thanks, Lorenzo
Re: [PATCH] PCI: dwc: Set 32-bit DMA mask for MSI target address allocation
On Tue, 17 Nov 2020 22:23:12 +0530, Vidya Sagar wrote: > Set DMA mask to 32-bit while allocating the MSI target address so that > the address is usable for both 32-bit and 64-bit MSI capable devices. > Throw a warning if it fails to set the mask to 32-bit to alert that > devices that are only 32-bit MSI capable may not work properly. Applied to pci/dwc, thanks! [1/1] PCI: dwc: Set 32-bit DMA mask for MSI target address allocation https://git.kernel.org/lpieralisi/pci/c/660c486590 Thanks, Lorenzo
Re: [PATCH v6 0/3] Add PCIe support for SM8250 SoC
On Tue, 8 Dec 2020 17:43:59 +0530, Manivannan Sadhasivam wrote: > This series adds PCIe support for Qualcomm SM8250 SoC with relevant PHYs. > There are 3 PCIe instances on this SoC each with different PHYs. The PCIe > controller and PHYs are mostly comaptible with the ones found on SDM845 > SoC, hence the old drivers are modified to add the support. > > This series has been tested on RB5 board with QCA6391 chipset connected > onboard. > > [...] Applied to pci/dwc, thanks! [1/3] dt-bindings: pci: qcom: Document PCIe bindings for SM8250 SoC https://git.kernel.org/lpieralisi/pci/c/458168247c [2/3] PCI: qcom: Add SM8250 SoC support https://git.kernel.org/lpieralisi/pci/c/e1dd639e37 [3/3] PCI: qcom: Add support for configuring BDF to SID mapping for SM8250 https://git.kernel.org/lpieralisi/pci/c/1c6072c743 Thanks, Lorenzo
Re: [PATCH v5 0/5] Add PCIe support for SM8250 SoC
On Tue, Dec 08, 2020 at 04:15:57PM +0530, Manivannan Sadhasivam wrote:
> Hi Lorenzo,
>
> On Tue, Dec 08, 2020 at 09:47:12AM +, Lorenzo Pieralisi wrote:
> > On Tue, Oct 27, 2020 at 10:30:28PM +0530, Manivannan Sadhasivam wrote:
> > > Hello,
> > >
> > > This series adds PCIe support for Qualcomm SM8250 SoC with relevant PHYs.
> > > There are 3 PCIe instances on this SoC each with different PHYs. The PCIe
> > > controller and PHYs are mostly comaptible with the ones found on SDM845
> > > SoC, hence the old drivers are modified to add the support.
> > >
> > > This series has been tested on RB5 board with QCA6391 chipset connected
> > > onboard.
> >
> > Hi,
> >
> > I would be merging this series, I understand patch {2) was already
> > taken by Vinod - should I take {1,3,4,5} via the pci tree ?
> >
>
> Vinod merged patches 1/5 and 2/5 as they belong to phy subsystem. You
> can take the rest of the patches via pci tree.
Would you mind rebasing them on top of my pci/dwc branch (with Bjorn's
tags) and resend them, I will apply them then.
Thanks,
Lorenzo
Re: [PATCH v5 0/5] Add PCIe support for SM8250 SoC
On Tue, Oct 27, 2020 at 10:30:28PM +0530, Manivannan Sadhasivam wrote:
> Hello,
>
> This series adds PCIe support for Qualcomm SM8250 SoC with relevant PHYs.
> There are 3 PCIe instances on this SoC each with different PHYs. The PCIe
> controller and PHYs are mostly comaptible with the ones found on SDM845
> SoC, hence the old drivers are modified to add the support.
>
> This series has been tested on RB5 board with QCA6391 chipset connected
> onboard.
Hi,
I would be merging this series, I understand patch {2) was already
taken by Vinod - should I take {1,3,4,5} via the pci tree ?
Thanks,
Lorenzo
> Thanks,
> Mani
>
> Changes in v5:
>
> * Added Review tags from Rob
> * Cleaned up the bdf to sid patch after discussing with Tony
>
> Changes in v4:
>
> * Fixed an issue with tx_tbl_sec in PHY driver
>
> Changes in v3:
>
> * Rebased on top of phy/next
> * Renamed ops_sm8250 to ops_1_9_0 to maintain uniformity
>
> Changes in v2:
>
> * Fixed the PHY and PCIe bindings
> * Introduced secondary table in PHY driver to abstract out the common configs.
> * Used a more generic way of configuring BDF to SID mapping
> * Dropped ATU change in favor of a patch spotted by Rob
>
> Manivannan Sadhasivam (5):
> dt-bindings: phy: qcom,qmp: Add SM8250 PCIe PHY bindings
> phy: qcom-qmp: Add SM8250 PCIe QMP PHYs
> dt-bindings: pci: qcom: Document PCIe bindings for SM8250 SoC
> PCI: qcom: Add SM8250 SoC support
> PCI: qcom: Add support for configuring BDF to SID mapping for SM8250
>
> .../devicetree/bindings/pci/qcom,pcie.txt | 6 +-
> .../devicetree/bindings/phy/qcom,qmp-phy.yaml | 6 +
> drivers/pci/controller/dwc/Kconfig| 1 +
> drivers/pci/controller/dwc/pcie-qcom.c| 92 ++
> drivers/phy/qualcomm/phy-qcom-qmp.c | 281 +-
> drivers/phy/qualcomm/phy-qcom-qmp.h | 18 ++
> 6 files changed, 398 insertions(+), 6 deletions(-)
>
> --
> 2.17.1
>
Re: [PATCH V5 0/5] Enhancements to Tegra194 PCIe driver
On Thu, 3 Dec 2020 19:04:46 +0530, Vidya Sagar wrote: > This series of patches do some enhancements and some bug fixes to the > Tegra194 PCIe platform driver like > - Fix Vendor-ID corruption > - Update DWC IP version > - Continue with uninitialization sequence even if parts fail > - Check return value of tegra_pcie_init_controller() > - Disable LTSSM during link's L2 entry > > [...] Applied to pci/dwc, thanks! [1/5] PCI: tegra: Fix ASPM-L1SS advertisement disable code https://git.kernel.org/lpieralisi/pci/c/6b6fafc1ab [2/5] PCI: tegra: Set DesignWare IP version https://git.kernel.org/lpieralisi/pci/c/01254b6d6b [3/5] PCI: tegra: Continue unconfig sequence even if parts fail https://git.kernel.org/lpieralisi/pci/c/b8f0d67149 [4/5] PCI: tegra: Check return value of tegra_pcie_init_controller() https://git.kernel.org/lpieralisi/pci/c/3d710af75b [5/5] PCI: tegra: Disable LTSSM during L2 entry https://git.kernel.org/lpieralisi/pci/c/cf68e3b7a6 Thanks, Lorenzo
Re: [PATCH v2] PCI: aardvark: Update comment about disabling link training
On Wed, 2 Dec 2020 19:46:59 +0100, Pali Rohár wrote: > It is not HW bug or workaround for some cards but it is requirement by PCI > Express spec. After fundamental reset is needed 100ms delay prior enabling > link training. So update comment in code to reflect this requirement. Applied to pci/aardvark, thanks! [1/1] PCI: aardvark: Update comment about disabling link training https://git.kernel.org/lpieralisi/pci/c/1d1cd163d0 Thanks, Lorenzo
Re: [PATCH V4 4/6] PCI: tegra: Continue unconfig sequence even if parts fail
On Tue, Dec 01, 2020 at 03:24:24PM +0100, Thierry Reding wrote: > On Mon, Nov 30, 2020 at 12:10:07PM +0000, Lorenzo Pieralisi wrote: > > On Mon, Nov 09, 2020 at 10:49:35PM +0530, Vidya Sagar wrote: > > > Currently the driver checks for error value of different APIs during the > > > uninitialization sequence. It just returns from there if there is any > > > error > > > observed for one of those calls. Comparatively it is better to continue > > > the > > > uninitialization sequence irrespective of whether some of them are > > > returning error. That way, it is more closer to complete uninitialization. > > > > Hi Vidya, Thierry, > > > > I can apply this series (dropping patches as suggested by Thierry), > > before though I wanted to ask you if this patch is really an > > improvement, it is hard to understand why skipping some error > > codes is OK for device correct operations to continue, maybe it > > is worth describing why some of those failures aren't really > > fatal. > > > > Please let me know, thanks. > > As explained in the commit message, the idea is to continue tearing > down even if things fail somewhere in the middle, because that ensures > that the hardware gets as close to an "uninitialized" state as possible. > If for example the first reset assert were to fail, then none of the > PHYs get disabled, the regulator stays on and the clocks stays on, all > of which can continue draining power after the controller has already > been torn down. Understood. By reading the code it looked weird that eg a reset failure was tolerable - I thought an error would be fatal (I don't know what are the consequences for instance on a subsequent resume) but it looks like it actually is not, that's the only point I raised. > So yes, I think this is an improvement. It's unclear to me what you're > asking for, though. Would you rather have a comment somewhere near the > tegra_pcie_unconfig_controller() function that states the same thing as > the commit message? It could be useful but it is up to you, I will merge the series as-is or I can add it myself, as you wish. Thanks, Lorenzo
Re: linux-next: Tree for Nov 30 (drivers/pci/controller/dwc/pcie-designware-host.c)
On Mon, Nov 30, 2020 at 08:44:55PM -0800, Randy Dunlap wrote: > On 11/30/20 12:36 AM, Stephen Rothwell wrote: > > Hi all, > > > > Changes since 20201127: > > > > on x86_64: > > WARNING: unmet direct dependencies detected for PCIE_DW_HOST > Depends on [n]: PCI [=y] && PCI_MSI_IRQ_DOMAIN [=n] > Selected by [y]: > - PCI_EXYNOS [=y] && PCI [=y] && (ARCH_EXYNOS || COMPILE_TEST [=y]) [...] > caused by: > commit f0a6743028f938cdd34e0c3249d3f0e6bfa04073 > Author: Jaehoon Chung > Date: Fri Nov 13 18:01:39 2020 +0100 > > PCI: dwc: exynos: Rework the driver to support Exynos5433 varian > > > which removed "depends on PCI_MSI_IRQ_DOMAIN from config PCI_EXYNOS. Fixed up and squashed in the original commit - we should probably rework the DWC driver dependencies on PCI_MSI_IRQ_DOMAIN to really fix it, for the time being this should do. Thanks, Lorenzo
