linux-kernel - Re: [PATCH v4] ufs: core: print UFSHCD capabilities in controller's sysfs node

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]

Date:   Tue, 2 Aug 2022 07:29:18 +1000
From:   Daniil Lunev <dlunev@...omium.org>
To:     Bart Van Assche <bvanassche@....org>
Cc:     Adrian Hunter <adrian.hunter@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alim Akhtar <alim.akhtar@...sung.com>,
        Avri Altman <avri.altman@....com>,
        Bean Huo <beanhuo@...ron.com>, Can Guo <cang@...eaurora.org>,
        Daejun Park <daejun7.park@...sung.com>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        Sohaib Mohamed <sohaib.amhmd@...il.com>,
        Stanley Chu <stanley.chu@...iatek.com>,
        linux-kernel@...r.kernel.org, linux-scsi@...r.kernel.org
Subject: Re: [PATCH v4] ufs: core: print UFSHCD capabilities in controller's
 sysfs node

HI Bart
> The above sentence is not complete. Did you perhaps want to write "are
> supported by the host controller" instead of "status"?
Will fix it in the next version.

> I don't think the above documentation is correct. My understanding is
> that the UFSHCD_CAP_CLK_SCALING flag indicates whether or not the host
> controller supports clock scaling. It does not indicate whether or not
> clock scaling is enabled.
Ah, right, there is a control for it in sysfs, it is just hidden in
ufshcd.c I was under
a wrong impression that it is like the writebooster capability that
has quite a bit of
conditions for staying present. WIll fix it in the next version.

> Please change "can not / can be enabled" into "is not supported by the
> host controller / is supported by the host controller".
That would be incorrect. The "caps" variable semantics is a bit weird
in the sense
that it is used at times to convey "active" capabilities, not just
supported one. For
example, for the writebooster capability to be present in caps, first
controller driver
should indicate it is ready to support it, then the part that is
attached to the host
controller has to indicate support in the device descriptor, then WB has to be
configured and its lifetime should not be exhausted. If any of those parameters
are not satisfied, the capability will be removed from the set despite generally
being supported. I am not sure how to properly word it, but just
saying "controller
supports it" would becounter-factual (especially since the controller
doesn't really
knows anything about writebooster per-ce, it is part's functionality).
What would
be suggested wording in that case?

--Daniil