[RFC,v1,0/2] support various values per device

Message

Kiwoong Kim June 20, 2020, 7:31 a.m. UTC

Respective UFS devices have their own characteristics and
many of them could be a form of numbers, such as timeout
and a number of retires. This introduces the way to set
those things per specific device vendor or specific device.

I wrote this like the style of ufs_fixups stuffs.

Kiwoong Kim (2):
  ufs: support various values per device
  ufs: change the way to complete fDeviceInit

 drivers/scsi/ufs/ufs_quirks.h | 20 +++++++++++++
 drivers/scsi/ufs/ufshcd.c     | 70 +++++++++++++++++++++++++++++++++++++------
 drivers/scsi/ufs/ufshcd.h     |  1 +
 3 files changed, 82 insertions(+), 9 deletions(-)

Comments

Avri Altman June 21, 2020, 8:22 a.m. UTC | #1

> 
> Respective UFS devices have their own characteristics and
> many of them could be a form of numbers, such as timeout
> and a number of retires. This introduces the way to set
> those things per specific device vendor or specific device.
> 
> I wrote this like the style of ufs_fixups stuffs.
Looks like your preparing the ground of making quirks a common practice...
Anyway, we already have that option per platform/vendor - 
what is that you are missing?

And as far as fDeviceInit  - the spec says:
"The host polls the fDeviceInit flag to check the completion of the process."
Your changes are align with that definition, so I am not sure that even a new quirk is needed?

Thanks,
Avri

Kiwoong Kim June 23, 2020, 2:14 a.m. UTC | #2

> > Respective UFS devices have their own characteristics and many of them
> > could be a form of numbers, such as timeout and a number of retires.
> > This introduces the way to set those things per specific device vendor
> > or specific device.
> >
> > I wrote this like the style of ufs_fixups stuffs.
> Looks like your preparing the ground of making quirks a common practice...
> Anyway, we already have that option per platform/vendor - what is that you
> are missing?
> 
> And as far as fDeviceInit  - the spec says:
> "The host polls the fDeviceInit flag to check the completion of the
> process."
> Your changes are align with that definition, so I am not sure that even a
> new quirk is needed?
> 
> Thanks,
> Avri

I see what you're saying and thank you for your opinion.

But I don’t think what the spec mentions fits in operating system all the time.
In fact, current driver doesn't work exactly the same with the description,
since current driver somehow terminates the waiting after a series of retrying.
Besides, I'm not sure if waiting something infinitely is great for Linux kernel,
Many are suffering from things like that point in the mobile industry
because that makes harder to figure out what's happening. Someone could tell
that kernel log shows it but in many development environments, it's difficult to 
check and occurring errors for timed-out is beneficial, I think.

That's why errors for not completing something have been introduced in some domains,
possibly even some architectures which don't define that.