Re: [PATCH v2 2/6] phy: qcom: Introduce PCIe UNIPHY 28LP driver

From: Krzysztof Kozlowski <krzk@kernel.org>
To: Varadarajan Narayanan <quic_varada@quicinc.com>
Cc: lpieralisi@kernel.org, kw@linux.com,
	manivannan.sadhasivam@linaro.org, robh@kernel.org,
	bhelgaas@google.com, krzk+dt@kernel.org, conor+dt@kernel.org,
	vkoul@kernel.org, kishon@kernel.org, andersson@kernel.org,
	konradybcio@kernel.org, p.zabel@pengutronix.de,
	quic_nsekar@quicinc.com, dmitry.baryshkov@linaro.org,
	linux-arm-msm@vger.kernel.org, linux-pci@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-phy@lists.infradead.org
Subject: Re: [PATCH v2 2/6] phy: qcom: Introduce PCIe UNIPHY 28LP driver
Date: Fri, 6 Dec 2024 09:46:58 +0100	[thread overview]
Message-ID: <800e3635-f496-4c2f-8037-d565404a3691@kernel.org> (raw)
In-Reply-To: <gawyxbgql7pru43e6rz4luhefaayrwiw3fplzj3k5cahzkppzr@uhuemcq5djw7>

On 05/12/2024 10:39, Krzysztof Kozlowski wrote:
> On Wed, Dec 04, 2024 at 05:03:25PM +0530, Varadarajan Narayanan wrote:
>> +	return 0;
>> +}
>> +
>> +static struct platform_driver qcom_uniphy_pcie_driver = {
>> +	.probe		= qcom_uniphy_pcie_probe,
>> +	.driver		= {
>> +		.name	= "qcom-uniphy-pcie",
>> +		.owner	= THIS_MODULE,
> 
> Srsly, upstreaming 10 year old code? No one figured out to fix 10 year
> old code before sending it upstream or entirely drop it and use new code
> as template?
> 
> NAK

I should express clearer what is the problem. You sent code which looks
like 10-or-more years old driver. This means that you have there all the
issues we fixed over last 10 years. It is really meaningless for the
reviewers to point out all the things we already fixed. It is much
better if you start from new driver from scratch, thus not replicating
10-year old bugs or deprecated styles.

I suggest dropping this driver entirely and starting from scratch from
the newest accepted driver.

Whatever you choose, be 100% sure that standard tools are happy, see
below instruction:

Please run standard kernel tools for static analysis, like coccinelle,
smatch and sparse, and fix reported warnings. Also please check for
warnings when building with W=1. Most of these commands (checks or W=1
build) can build specific targets, like some directory, to narrow the
scope to only your code. The code here looks like it needs a fix. Feel
free to get in touch if the warning is not clear.

Best regards,
Krzysztof

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy