On 10/4/21 9:52 AM, Mark Brown wrote: > On Mon, Oct 04, 2021 at 09:36:37AM -0700, Florian Fainelli wrote: >> On 10/4/21 9:31 AM, Mark Brown wrote: > >>> an issue, someone could press a button or whatever. Frankly for SPI the >>> quiescing part doesn't seem like logic that should be implemented in >>> drivers, it's a subsystem level thing since there's nothing driver >>> specific about it. > >> Surely the SPI subsystem can help avoid queuing new transfers towards >> the SPI controller while the controller can shut down the resources that >> only it knows about. > > Yes, that's what I was saying. > >>> In the case of this specific driver I'm still not clear that the best >>> thing isn't just to delete the shutdown callback and let any ongoing >>> transfers complete, though I guess there'd be issues in kexec cases with >>> long enough tansfers. > >> No please don't, I should have arguably justified the reasons why >> better, but the main reason is that one of the platforms on which this >> driver is used has received extensive power management analysis and >> changes, and shutting down every bit of hardware, including something as >> small as a SPI controller, and its clock (and its PLL) helped meet >> stringent power targets. > > OK, so it's similar to a lot of the other embedded cases where it's for > a power down that doesn't cut as much power as would be desirable - > that's reasonable. Like you say you didn't mention it at all in the > changelog. Ideally the hardware would just cut all power to the SoC in > shutdown but then IIRC those boards don't have a PMIC so... Yes, that's is what we do on other types of SoCs, this particular one however only has a single power domain and so software must come to the rescue to shut down as much as it can. Newer boards do have a PMIC that can help us with that, but not with everything, still. > >> TBH, I still wonder why we have .shutdown() and we simply don't use >> .remove() which would reduce the amount of work that people have to do >> validate that the hardware is put in a low power state and would also >> reduce the amount of burden on the various subsystems. > > Yeah, it does seem a bit odd - I'd figured it was for speed reasons. > -- Florian
