On Fri, Sep 15, 2023 at 09:31:44PM +0000, Niklas Cassel wrote: > I'm not saying that these controllers shouldn't work with Linux. > However, these controller used to work with CC.CRIME == 0, so perhaps > we should continue to use them that way? Perhaps I missed something, but I didn't get any indication that these controllers were reporting CRIMS capability. They're just reporting CRWMS as far as I know, so CRIME doesn't apply. I only included that case in the patch for completeness. > So having both fields defined to zero, or rather, to have both fields > defined to a value smaller than CAP.TO, regardless of CC.CRIME value, > is quite bad. > > So perhaps it is better to keep CC.CRIME == 0 for such controllers. > > > > If we have a way to sanity check for spec non-compliance, I would prefer > > doing that generically rather than quirk specific devices. > > It's not going to be beautiful, but one way could be to: > -check CAP.CRMS.CRIMS, if it is set to 1: > -write CC.CRIME == 1, > -re-read CAP register, since it can change depending on CC.CRIME (urgh) > -check if CRTO.CRIMT is less than CAP.TO, if so: > -write CC.CRIME == 0 (disable the feature since it is obviously broken) > -re-read CAP register, since it can change depending on CC.CRIME (urgh) There is a corner case that I am somewhat purposefully ignoring: CAP.TO is worst case for both CC.EN 0->1 and 1->0, whereas both CRTO values are only for the 0->1 transition. It's entirely possible some implementation needs a longer 1->0 transition, so CAP.TO *could* validly be greater than the either CRTO value. I just don't see that happening in practice, and even if we do encounter such a device, waiting a little longer on init for a broken controller doesn't make any situation worse.
