linux-kernel - Re: [PATCH 04/10] PCI: exynos: Add platform device private data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <fty3sp3brzpcs6jeqg6yxiwgpbyqogxqt5mo7owiw4vkl7vj2j@b33zfdizuy4d>
Date: Fri, 13 Jun 2025 14:34:54 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Shradha Todi <shradha.t@...sung.com>
Cc: 'Krzysztof Kozlowski' <krzk@...nel.org>, linux-pci@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-samsung-soc@...r.kernel.or, linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org, 
	manivannan.sadhasivam@...aro.org, lpieralisi@...nel.org, kw@...ux.com, robh@...nel.org, 
	bhelgaas@...gle.com, jingoohan1@...il.com, krzk+dt@...nel.org, conor+dt@...nel.org, 
	alim.akhtar@...sung.com, vkoul@...nel.org, kishon@...nel.org, arnd@...db.de, 
	m.szyprowski@...sung.com, jh80.chung@...sung.com, 
	'Pankaj Dubey' <pankaj.dubey@...sung.com>
Subject: Re: [PATCH 04/10] PCI: exynos: Add platform device private data

On Tue, May 27, 2025 at 04:13:58PM +0530, Shradha Todi wrote:
> 
> 
> > -----Original Message-----
> > From: Krzysztof Kozlowski <krzk@...nel.org>
> > Sent: 21 May 2025 15:15
> > To: Shradha Todi <shradha.t@...sung.com>
> > Cc: linux-pci@...r.kernel.org; devicetree@...r.kernel.org; linux-arm-kernel@...ts.infradead.org; linux-samsung-soc@...r.kernel.or;
> > linux-kernel@...r.kernel.org; linux-phy@...ts.infradead.org; manivannan.sadhasivam@...aro.org; lpieralisi@...nel.org;
> > kw@...ux.com; robh@...nel.org; bhelgaas@...gle.com; jingoohan1@...il.com; krzk+dt@...nel.org; conor+dt@...nel.org;
> > alim.akhtar@...sung.com; vkoul@...nel.org; kishon@...nel.org; arnd@...db.de; m.szyprowski@...sung.com;
> > jh80.chung@...sung.com; Pankaj Dubey <pankaj.dubey@...sung.com>
> > Subject: Re: [PATCH 04/10] PCI: exynos: Add platform device private data
> > 
> > On Mon, May 19, 2025 at 01:01:46AM GMT, Shradha Todi wrote:
> > > -static const struct dw_pcie_ops dw_pcie_ops = {
> > > +static const struct dw_pcie_ops exynos_dw_pcie_ops = {
> > >  	.read_dbi = exynos_pcie_read_dbi,
> > >  	.write_dbi = exynos_pcie_write_dbi,
> > >  	.link_up = exynos_pcie_link_up,
> > > @@ -279,6 +286,7 @@ static int exynos_pcie_probe(struct
> > > platform_device *pdev)  {
> > >  	struct device *dev = &pdev->dev;
> > >  	struct exynos_pcie *ep;
> > > +	const struct samsung_pcie_pdata *pdata;
> > >  	struct device_node *np = dev->of_node;
> > >  	int ret;
> > >
> > > @@ -286,8 +294,11 @@ static int exynos_pcie_probe(struct platform_device *pdev)
> > >  	if (!ep)
> > >  		return -ENOMEM;
> > >
> > > +	pdata = of_device_get_match_data(dev);
> > > +
> > > +	ep->pdata = pdata;
> > >  	ep->pci.dev = dev;
> > > -	ep->pci.ops = &dw_pcie_ops;
> > > +	ep->pci.ops = pdata->dwc_ops;
> > >
> > >  	ep->phy = devm_of_phy_get(dev, np, NULL);
> > >  	if (IS_ERR(ep->phy))
> > > @@ -363,9 +374,9 @@ static int exynos_pcie_resume_noirq(struct device *dev)
> > >  		return ret;
> > >
> > >  	/* exynos_pcie_host_init controls ep->phy */
> > > -	exynos_pcie_host_init(pp);
> > > +	ep->pdata->host_ops->init(pp);
> > >  	dw_pcie_setup_rc(pp);
> > > -	exynos_pcie_start_link(pci);
> > > +	ep->pdata->dwc_ops->start_link(pci);
> > 
> > One more layer of indirection.
> > 
> > Read:
> > https://lore.kernel.org/all/CAL_JsqJgaeOcnUzw+rUF2yO4hQYCdZYssjxHzrDvvHGJimrASA@mail.gmail.com/
> > 
> 
> I went through this thread and the solution to avoid redirection seems to be:
> 1. Make the common parts into a library that each driver can call
> 2. When there is barely anything in common, make a separate driver
> 
> From my understanding of these 2 drivers, there is hardly anything that can go into common library
> 1. host_init, dbi_read, dbi_write these ops have completely different flow
> 2. link_up, start_link have similar flow but different register offsets
> 3. write_dbi2 and stop_link is not implemented for exynos but needed for FSD
> 4. Resources are different - FSD does not have regulator, Exynos5433 does not have syscon, FSD has msi IRQ vs exynos5433 has legacy
> 5. Exynos is host only whereas FSD is dual mode controller.
> 
> I don’t see any other way except redirection, or using lots of if(variant == FSD) which is also discouraged.

Please wrap your replies to 80 columns.

> 

IMO it is OK to have the callbacks for cases like this. This is also similar to
Qcom driver where each SoC requires custom register updates and going by the
common library or a new driver wouldn't be feasible.

> And about making it a different driver altogether, I'm completely okay to do so. In fact we had previously tried to post it as a
> different driver which was rejected.
> 

No, please do not create a new driver, that is another maintainer burden.

- Mani

-- 
மணிவண்ணன் சதாசிவம்