Received-SPF: pass (google.com: domain of linux-kernel+bounces-199555-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Mon, 3 Jun 2024 14:21:27 -0400
From: Frank Li <Frank.li@nxp.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: Richard Zhu <hongxing.zhu@nxp.com>,
	Lucas Stach <l.stach@pengutronix.de>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <kw@linux.com>,
	Rob Herring <robh@kernel.org>, Bjorn Helgaas <bhelgaas@google.com>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>, linux-pci@vger.kernel.org,
	imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, bpf@vger.kernel.org,
	devicetree@vger.kernel.org, Will Deacon <will@kernel.org>,
	Robin Murphy <robin.murphy@arm.com>, Joerg Roedel <joro@8bytes.org>,
	Jason Gunthorpe <jgg@ziepe.ca>,
	Alyssa Rosenzweig <alyssa@rosenzweig.io>,
	Marc Zyngier <maz@kernel.org>
Subject: Re: [PATCH v5 08/12] PCI: imx6: Config look up table(LUT) to support
 MSI ITS and IOMMU for i.MX95
Message-ID: <Zl4Jp7YWDHyFSax4@lizhi-Precision-Tower-5810>
References: <Zln3WkiHC3AUPocL@lizhi-Precision-Tower-5810>
 <20240603171149.GA685507@bhelgaas>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240603171149.GA685507@bhelgaas>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?w3HmdY6mb5gI2qrctTIBUVTwYGYBQG/Kl0l0iAF2zG9tAB5nN5idYoZGLQy3?=
 =?us-ascii?Q?75P+ZMXST9WmzNQm6d9zq2BEELInQT2u/p5ndht37gm6PooudOteMM1jCUgL?=
 =?us-ascii?Q?JbOnA0Xib9lgfOgys9OUZTnrw1NlUPv56+OIrVo4MMxRoQcG0pc/YFJfHx+M?=
 =?us-ascii?Q?5+T7TE5caL4qEEAC2+wYm75HSQZlYqVT20sE+m4UYlfhCnOXqgBtSP4BL4FB?=
 =?us-ascii?Q?Kz5TkQKZlNvfiRpJkgEE6xb08rWnPkEPWtPQ8MOJxCNPv3wyelZctBBBi1x9?=
 =?us-ascii?Q?8gF2Nc+7mEt1zFbfmx4TNf61ILdsv2NCJUq0rQFHJiejCvrptAyc/zRQ2Sin?=
 =?us-ascii?Q?v18hU7nHSUvkiM9uNc+sxvh9StjU1I72DL+65ObkMPFmhKltk5CzyAcHGBdw?=
 =?us-ascii?Q?GapVDZY9C4K+eAYHB159F+aMUkCwQYjaH9B2Rly7lOkYo3/1qjVHquVRm7Vn?=
 =?us-ascii?Q?UkQdFTDwCpwcxmxorLyZv2RPob2BzBcP0erwAhj7nTz6S2GVKD966y77cxgi?=
 =?us-ascii?Q?CbxZOkKgItKeiYYwi6yaIidOasUYaXwncAcq0jt6xlj2a0tINy1EPDXdNsvo?=
 =?us-ascii?Q?3ZFKkRX2Zb44UWUG7hEkxkR/emQduz0VQSX4edCG7gnc1keOFVdOn5ss90/y?=
 =?us-ascii?Q?xP4zBulk5QzaNm3hWFAn4KvWUUa4F/7GpqvqmnSXYdxd7+EXBldlfVSD8h0h?=
 =?us-ascii?Q?mSSQkZAYEcSFSqiOzzbMa/S5Nszo/ex/UYrZMAIAzqXWvrvk0ZY9ioPsw8eE?=
 =?us-ascii?Q?2Hjq0LkOunOb3ydEXMRtWSkoxL8y2TgVtoIjSh21+jlS69idvtWBNlFJ5FBN?=
 =?us-ascii?Q?ENsjECuP39NOBuAE9+K2flBXkba8/Ktt5PwJALwSyoICKlFUZ9LG6MrXcWOy?=
 =?us-ascii?Q?HO8Fd/keax1FWk4uD6hQuOqAQ7LShD/BxynHO3AeUEOdMWhRzIYIaVHREZK9?=
 =?us-ascii?Q?shrS7pVpPyJG09yuK1BPwRUPGLBHeOnCG2MdQvY9962+BViAkwrycIfa6n/X?=
 =?us-ascii?Q?7nJoXEbkHjHAbqX7lK/aXipvZimHt6ruyO6/5sf5fszgoZYG+BZ5sYG51ft9?=
 =?us-ascii?Q?vZW0tLmAvwks/timUrASaOfuHStvxIvrpPHiftuDMJ/+hbw3il0/IKhSZ9f0?=
 =?us-ascii?Q?rgb/7qzccXpnfICX65W1Oa1AB4GFDwP09CzwDFkWtG+4vKcDpB1WAQsm3yTa?=
 =?us-ascii?Q?At+sJOqKUUrU64fxWexSuRXkvCO3zm8YILeTGgbBECuCFpddY9wDwp7RavTj?=
 =?us-ascii?Q?uHTr0qLP9h5/9LrGV2bSTWQJ/+IpI49VXPHgkShHbXjrfoN9K20+f/D4A1pg?=
 =?us-ascii?Q?SfyyOyCvBNafxkCtwCOf5yGHwttMkj8+YNxW7jOWy6dFyw5TpU59IIh6uI7a?=
 =?us-ascii?Q?y2AxrJpFpdhjeTZ+Mc1tmSJOPpLHT3G1F7+4mI/SLzY6vRjINr/1uTx+R6PF?=
 =?us-ascii?Q?6LKp7UbiTkE7/qpu5gvlaZOtN/4jv4Zm6B1UcegnbOHnnIg0d04h+RvK95SH?=
 =?us-ascii?Q?OyLwjJzYXicdc/B7g3TkDmL7buJRXmu6tOmCZEGY8J6qaChSd2LIGqNgPMoe?=
 =?us-ascii?Q?89aZs9M4ssBJ12ols22NeiqYpaTaHKweiyBck8Yv?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4d8ff925-b581-4d94-b270-08dc83fa02fa
X-MS-Exchange-CrossTenant-AuthSource: PAXPR04MB9642.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Jun 2024 18:21:39.8055
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: NitoS2m/LAgumKsmc0OGJrGWip0EfonmBY4fkmjadqv6L4oLH1VkBrqOOb56ueqjf0udhO0kXzUC6uh5dZMong==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: GV1PR04MB10704

On Mon, Jun 03, 2024 at 12:11:49PM -0500, Bjorn Helgaas wrote:
> On Fri, May 31, 2024 at 12:14:18PM -0400, Frank Li wrote:
> > On Thu, May 30, 2024 at 06:08:32PM -0500, Bjorn Helgaas wrote:
> > > On Tue, May 28, 2024 at 03:39:21PM -0400, Frank Li wrote:
> > > > For the i.MX95, configuration of a LUT is necessary to convert Bus Device
> > > > Function (BDF) to stream IDs, which are utilized by both IOMMU and ITS.
> > > > This involves examining the msi-map and smmu-map to ensure consistent
> > > > mapping of PCI BDF to the same stream IDs. Subsequently, LUT-related
> > > > registers are configured. In the absence of an msi-map, the built-in MSI
> > > > controller is utilized as a fallback.
> > > > 
> > > > Additionally, register a PCI bus notifier to trigger imx_pcie_add_device()
> > > > upon the appearance of a new PCI device and when the bus is an iMX6 PCI
> > > > controller. This function configures the correct LUT based on Device Tree
> > > > Settings (DTS).
> > > 
> > > This scheme is pretty similar to apple_pcie_bus_notifier().  If we
> > > have to do this, I wish it were *more* similar, i.e., copy the
> > > function names, bitmap tracking, code structure, etc.
> > 
> > Actually, I refer apple_pcie_bus_notifier(). I can't direct use apple's
> > implement because in imx95 have difference PCI host controller, another one
> > is PCI ECAM netc controller. At lease function name should be similar with
> > apple. 
> 
> I know it's different hardware, so obviously it can't be exactly the
> same.  These are the differences that looked possibly unnecessary:
> 
>   - registering from initcall instead of .probe():
> 
>       apple_pcie_probe                  # .probe() method
>         bus_register_notifier(&pci_bus_type, &apple_pcie_nb)
> 
>       imx_pcie_init                     # device_initcall()
>         bus_register_notifier(&pci_bus_type, &imx_pcie_nb)

Maybe apple only one PCIe controller with multi ports. One nb just register
once.

If there are multi PCIe controller instance, second bus_register_notifier()
report error.

Of course, if you like, I can use global reference counter to handle this.

> 
>   - naming BUS_NOTIFY_DEL_DEVICE function:
> 
>       apple_pcie_release_device()
>       imx_pcie_del_device()

Okay, I can change to release_device().

> 
>   - tracking entries in use via bitmap vs scanning hardware for
>     invalid entries:
> 
>       bitmap_find_free_region           # apple
> 
>       imx_pcie_config_lut               # imx
>         for (i = 0; i < IMX95_MAX_LUT; i++)
>           regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1, &data1)
>           if (data1 & IMX95_PE0_LUT_VLD)
>             continue

Okay, I can change to use bitmap.

> 
> When we fix a bug in one driver, it's easier to check whether other
> drivers also need the fix if they use the same structure and names.
> 
> Bjorn