Re: [PATCH 2/2] scsi: ufs: core: Fix the code for entering hibernation

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Fri, Aug 23, 2024 at 07:48:50PM -0700, Bart Van Assche wrote:
> On 8/23/24 7:29 PM, Manivannan Sadhasivam wrote:
> > What if other vendors start adding the workaround in the core driver citing GKI
> > requirement (provided it also removes some code as you justified)? Will it be
> > acceptable? NO.
> 
> It's not up to you to define new rules for upstream kernel development.

I'm not framing new rules, but just pointing out the common practice.

> Anyone is allowed to publish patches that rework kernel code, whether
> or not the purpose of such a patch is to work around a SoC bug.
> 

Yes, at the same time if that code deviates from the norm, then anyone can
complain. We are all working towards making the code better.

> Additionally, it has already happened that one of your colleagues
> submitted a workaround for a SoC bug to the UFS core driver.
> From the description of commit 0f52fcb99ea2 ("scsi: ufs: Try to save
> power mode change and UIC cmd completion timeout"): "This is to deal
> with the scenario in which completion has been raised but the one
> waiting for the completion cannot be awaken in time due to kernel
> scheduling problem." That description makes zero sense to me. My
> conclusion from commit 0f52fcb99ea2 is that it is a workaround for a
> bug in a UFS host controller, namely that a particular UFS host
> controller not always generates a UIC completion interrupt when it
> should.
> 

0f52fcb99ea2 was submitted in 2020 before I started contributing to UFS driver
seriously. But the description of that commit never mentioned any issue with the
controller. It vaguely mentions 'kernel scheduling problem' which I don't know
how to interpret. If I were looking into the code at that time, I would've
definitely asked for clarity during the review phase.

But there is no need to take it as an example. I can only assert the fact that
working around the controller defect in core code when we already have quirks
for the same purpose defeats the purpose of quirks. And it will encourage other
people to start changing the core code in the future thus bypassing the quirks.

But I'm not a maintainer of this part of the code. So I cannot definitely stop
you from getting this patch merged. I'll leave it up to Martin to decide.

- Mani

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