On 8/20/19 2:11 AM, Maxime Ripard wrote: > Hi, > > On Mon, Aug 19, 2019 at 10:23:03PM -0500, Samuel Holland wrote: >> On sun8i, sun9i, and sun50i SoCs, system suspend/resume support requires >> firmware running on the AR100 coprocessor (the "SCP"). Such firmware can >> provide additional features, such as thermal monitoring and poweron/off >> support for boards without a PMIC. >> >> Since the AR100 may be running critical firmware, even if Linux does not >> know about it or directly interact with it (all requests may go through >> an intermediary interface such as PSCI), Linux must not turn off its >> clock. This paragraph here is the key. The firmware won't necessarily have a device tree node, and in the current design it will not, since Linux never communicates with it directly. All communication goes through ATF via PSCI. >> At this time, such power management firmware only exists for the A64 and >> H5 SoCs. However, it makes sense to take care of all CCU drivers now >> for consistency, and to ease the transition in the future once firmware >> is ported to the other SoCs. >> >> Leaving the clock running is safe even if no firmware is present, since >> the AR100 stays in reset by default. In most cases, the AR100 clock is >> kept enabled by Linux anyway, since it is the parent of all APB0 bus >> peripherals. This change only prevents Linux from turning off the AR100 >> clock in the rare case that no peripherals are in use. >> >> Signed-off-by: Samuel Holland <samuel@xxxxxxxxxxxx> > > So I'm not really sure where you want to go with this. > > That clock is only useful where you're having a firmware running on > the AR100, and that firmware would have a device tree node of its own, > where we could list the clocks needed for the firmware to keep > running, if it ever runs. If the driver has not been compiled in / > loaded, then we don't care either. See above. I don't expect that the firmware would have a device tree node, because the firmware doesn't need any Linux drivers. > But more fundamentally, if we're going to use SCPI, then those clocks > will not be handled by that driver anyway, but by the firmware, right? In the future, we might use SCPI clocks/sensors/regulators/etc. from Linux, but that's not the plan at the moment. Given that it's already been two years since I started this project, I'm trying to limit its scope so I can get at least some part merged. The first step is to integrate a firmware that provides suspend/resume functionality only. That firmware does implement SCPI, and if the top-level Linux SCPI driver worked with multiple mailbox channels, it could query the firmware's version and fetures. But all of the SCPI commands used for suspend/resume must go through ATF via PSCI. > So I'm not really sure that we should do it statically this way, and > that we should do it at all. Do you have a better way to model "firmware uses this clock behind the scenes, so Linux please don't touch it"? It's unfortunate that we have Linux and firmware fighting over the R_CCU, but since we didn't have firmware (e.g. SCPI clocks) in the beginning, it's where we are today. The AR100 clock doesn't actually have a gate, and it generally has dependencies like R_INTC in use. So as I mentioned in the commit message, the clock will normally be on anyway. The goal was to model the fact that there are users of this clock that Linux doesn't/can't know about. > Maxime Thanks, Samuel