Message-ID: <ZzYp1G+cVlzPvBXb@lizhi-Precision-Tower-5810>
Date: Thu, 14 Nov 2024 11:48:20 -0500
From: Frank Li <Frank.li@....com>
To: Bjorn Helgaas <bhelgaas@...gle.com>, Richard Zhu <hongxing.zhu@....com>,
	Lucas Stach <l.stach@...gutronix.de>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	Rob Herring <robh@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>
Cc: linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, imx@...ts.linux.dev,
	alyssa@...enzweig.io, bpf@...r.kernel.org, broonie@...nel.org,
	jgg@...pe.ca, joro@...tes.org, lgirdwood@...il.com, maz@...nel.org,
	p.zabel@...gutronix.de, robin.murphy@....com, will@...nel.org
Subject: Re: [PATCH v5 0/2] PCI: add enabe(disable)_device() hook for bridge

On Wed, Nov 13, 2024 at 10:55:41AM -0500, Frank Li wrote:
> On Mon, Nov 04, 2024 at 02:22:58PM -0500, Frank Li wrote:
>
> Any comments for this patches?
>
> Bjorn and give ack at v4 and Marc Zyngier give test/review tag at v4. I
> just drop these because change to use helper function and funtionality is
> the same.
>
> After this patch merge, I think apply's bus notification can convert to
> this way.

Bjorn:
	Can I keep your ack tag in next version? you give ack tag at v4,
but I change to helper function at v5. I plan send v6 soon to fix mani's
comment about patch2.

Frank

>
> Frank
>
> > Some system's IOMMU stream(master) ID bits(such as 6bits) less than
> > pci_device_id (16bit). It needs add hardware configuration to enable
> > pci_device_id to stream ID convert.
> >
> > https://lore.kernel.org/imx/20240622173849.GA1432357@bhelgaas/
> > This ways use pcie bus notifier (like apple pci controller), when new PCIe
> > device added, bus notifier will call register specific callback to handle
> > look up table (LUT) configuration.
> >
> > https://lore.kernel.org/imx/20240429150842.GC1709920-robh@kernel.org/
> > which parse dt's 'msi-map' and 'iommu-map' property to static config LUT
> > table (qcom use this way). This way is rejected by DT maintainer Rob.
> >
> > Above ways can resolve LUT take or stream id out of usage the problem. If
> > there are not enough stream id resource, not error return, EP hardware
> > still issue DMA to do transfer, which may transfer to wrong possition.
> >
> > Add enable(disable)_device() hook for bridge can return error when not
> > enough resource, and PCI device can't enabled.
> >
> > Basicallly this version can match Bjorn's requirement:
> > 1: simple, because it is rare that there are no LUT resource.
> > 2: EP driver probe failure when no LUT, but lspci can see such device.
> >
> > [    2.164415] nvme nvme0: pci function 0000:01:00.0
> > [    2.169142] pci 0000:00:00.0: Error enabling bridge (-1), continuing
> > [    2.175654] nvme 0000:01:00.0: probe with driver nvme failed with error -12
> >
> > > lspci
> > 0000:00:00.0 PCI bridge: Philips Semiconductors Device 0000
> > 0000:01:00.0 Non-Volatile memory controller: Micron Technology Inc 2100AI NVMe SSD [Nitro] (rev 03)
> >
> > To: Bjorn Helgaas <bhelgaas@...gle.com>
> > To: Richard Zhu <hongxing.zhu@....com>
> > To: Lucas Stach <l.stach@...gutronix.de>
> > To: Lorenzo Pieralisi <lpieralisi@...nel.org>
> > To: Krzysztof Wilczyński <kw@...ux.com>
> > To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> > To: Rob Herring <robh@...nel.org>
> > To: Shawn Guo <shawnguo@...nel.org>
> > To: Sascha Hauer <s.hauer@...gutronix.de>
> > To: Pengutronix Kernel Team <kernel@...gutronix.de>
> > To: Fabio Estevam <festevam@...il.com>
> > Cc: linux-pci@...r.kernel.org
> > Cc: linux-kernel@...r.kernel.org
> > Cc: linux-arm-kernel@...ts.infradead.org
> > Cc: imx@...ts.linux.dev
> > Cc: Frank.li@....com \
> > Cc: alyssa@...enzweig.io \
> > Cc: bpf@...r.kernel.org \
> > Cc: broonie@...nel.org \
> > Cc: jgg@...pe.ca \
> > Cc: joro@...tes.org \
> > Cc: l.stach@...gutronix.de \
> > Cc: lgirdwood@...il.com \
> > Cc: maz@...nel.org \
> > Cc: p.zabel@...gutronix.de \
> > Cc: robin.murphy@....com \
> > Cc: will@...nel.org \
> > Cc: Robin Murphy <robin.murphy@....com>
> > Cc: Marc Zyngier <maz@...nel.org>
> >
> > Signed-off-by: Frank Li <Frank.Li@....com>
> > ---
> > Changes in v5:
> > - Add help function of pci_bridge_enable(disable)_device
> > - Because big change, removed Bjorn's review tags and have not
> > added
> > Marc Zyngier't review and test tags
> > - Fix pci-imx6.c according to Mani's feedback
> > - Link to v4: https://lore.kernel.org/r/20241101-imx95_lut-v4-0-0fdf9a2fe754@nxp.com
> >
> > Changes in v4:
> > - Add Bjorn Helgaas review tag for patch1
> > - check 'target' value for patch2
> > - detail see each patches
> > - Link to v3: https://lore.kernel.org/r/20241024-imx95_lut-v3-0-7509c9bbab86@nxp.com
> >
> > Changes in v3:
> > - disable_device when error happen
> > - use target for of_map_id
> > - Check if rid already in lut table when enable deviced
> > - Link to v2: https://lore.kernel.org/r/20240930-imx95_lut-v2-0-3b6467ba539a@nxp.com
> >
> > Changes in v2:
> > - see each patch
> > - Link to v1: https://lore.kernel.org/r/20240926-imx95_lut-v1-0-d0c62087dbab@nxp.com
> >
> > ---
> > Frank Li (2):
> >       PCI: Add enable_device() and disable_device() callbacks for bridges
> >       PCI: imx6: Add IOMMU and ITS MSI support for i.MX95
> >
> >  drivers/pci/controller/dwc/pci-imx6.c | 176 +++++++++++++++++++++++++++++++++-
> >  drivers/pci/pci.c                     |  36 ++++++-
> >  include/linux/pci.h                   |   2 +
> >  3 files changed, 212 insertions(+), 2 deletions(-)
> > ---
> > base-commit: 06fb071a1aefbe4c6cc8fd41aacd0b9422361721
> > change-id: 20240926-imx95_lut-1c68222e0944
> >
> > Best regards,
> > ---
> > Frank Li <Frank.Li@....com>
> >

Powered by blists - more mailing lists