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] [thread-next>] [day] [month] [year] [list]

Message-ID: <0e2301dbcef4$42969af0$c7c3d0d0$@samsung.com>
Date: Tue, 27 May 2025 16:13:58 +0530
From: "Shradha Todi" <shradha.t@...sung.com>
To: "'Krzysztof Kozlowski'" <krzk@...nel.org>
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



> -----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.

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.

If you still feel there is a way to separate out the common parts into a library, please guide me.

> Best regards,
> Krzysztof