Re: [PATCH] nvme: add support for TP4084 - Time-to-Ready Enhancements

[Niklas Cassel <Niklas.Cassel@wdc.com>]

On Fri, May 20, 2022 at 08:13:04AM -0600, Keith Busch wrote:
> On Fri, May 20, 2022 at 11:51:49AM +0000, Niklas Cassel wrote:
> > From the CC.CRIME description:
> > Changing the value of this field may cause a change in the time reported in the
> > CAP.TO field. Refer to the definition of CAP.TO for more details.
> > 
> > The definition for CAP.TO:
> > If the Controller Ready Independent of Media Enable (CC.CRIME) bit is set to ‘1’
> > and the worst-case time for CSTS.RDY to change state is due to enabling the
> > controller after CC.EN transitions from ‘0’ to ‘1’, then this field shall be set
> > to:
> > a) the value in Controller Ready Independent of Media Timeout (CRTO.CRIMT); or
> > b) FFh if CRTO.CRIMT is greater than FFh.
> > 
> > This phrasing is quite confusing IMO.
> 
> Yes, that is rather confusing. I see you started a thread on the nvme workgroup
> reflector. I hope they can clear this up.

The current patch that is in Jens's tree is be fine, since we cache
the CAP register at start-up (after a reset), and never re-read it.

However, while reading through the spec, in NVMe 2.0b,
3.5.3 Controller Ready Modes During Initialization
it says:

[...] In this situation, the host should set the controller ready mode by
writing to the CC.CRIME bit before the controller is enabled [...].



Right now we do not write CC.CRIME bit _before_ the controller is enabled.
We set CC.CRIME and CC.ENABLE at the same time, which strictly speaking
is not according to spec.

Should we perhaps consider splitting the write up into two,
the first write sets everything except the enable bit,
and the second write sets everything + the enable bit?


Kind regards,
Niklas