On 1/9/24 04:41, Krzysztof Kozlowski wrote: > On 05/01/2024 15:31, Philipp Zabel wrote: >>>>> Sorry, then I don't get what you refer to. The driver calls deassert >>>>> when it is safe for it to do it, so the driver *knows*. Now, you claim >>>>> that driver does not know that... core also does not know, so no one knows. >>>> >>>> Yes! That is the problem with this design. Someone has to coordinate the >>>> reset, and it can't be the driver. But the core also doesn't have enough >>>> information. So no one can do it. >>> >>> The point is that the driver coordinates. >> >> Currently the reset controller API supports two types of shared resets. >> I hope distinguishing the two types and illustrating them helps the >> discussion: >> >> 1) For devices that just require the reset to be deasserted while they >> are active, and don't care otherwise, there is the clk-like behavior >> described in [1]. >> >> requested reset signal via reset_control_deassert/assert(): >> device A: ⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺\⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽/⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺ >> device B: ⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺\⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽/⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺ >> >> actual reset signal to both devices: >> ⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺\⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽/⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺⎺ >> >> In this scenario, there should be no delays in the reset controller >> driver. reset_control_deassert() may return as soon as the physical >> reset signal is deasserted [2]. Any post-deassert delays required by >> the devices are handled in the device drivers, and they can be >> different for each device. The devices have to be able to cope with a >> (much) longer post-deassert delay than expected (e.g. device B in this >> case). It is assumed that the reset signal is initially asserted. >> >> The reset-gpio patchset supports this. > > Yep! :) > >> >> 2) The second type is for devices that require a single reset pulse for >> initialization, at any time before they become active. This is >> described in [3]. >> >> requested reset signal via reset_control_reset/rearm(): >> device A: ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽/⎺⎺\⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽ >> device B: ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽/⎺⎺\⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽ >> >> actual reset signal to both devices: >> ⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽/⎺⎺\⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽⎽ >> >> Here the reset controller needs to know the delay between assertion and >> deassertion - either baked into the hardware or as a delay call in the >> .reset callback. >> >> This is not supported by the reset-gpio patchset. It could be > > Yep, as well. > >> implemented via a delay property in the device tree that would have to >> be the same for all devices sharing the reset line, and by adding the > > Or through dedicated node to which reset-gpio binds, just like in Sean's > code some years ago. Nothing stops achieving that, except of course > convincing Rob. The point is that although my design does not solve it, > it also does not prevent it in the future. Given this and > If the reset deassert (or assert, depending what's the default state) is > triggered in the probe, then it will happen with the probe of the first > device. If the delays of that reset are not suitable for the second - > not yet probed - then what do you propose? I have the answer: do not use > the simple, generic solution. The simple and generic solutions work for > simple and generic cases. I think a separate pseudo-device is necessary a generic solution. So I guess I will revive my patchset. >> .reset callback to the reset controller driver. The only issue is that >> the initial state of the reset line should be deasserted, and at >> reset_control_get() time, when the reset-gpio controller is >> instantiated, it is not yet known which type the driver will use. >> >> Sharing a reset line between devices of different type is not >> supported. Unfortunately, this will only fail at >> reset_control_deassert() / reset_control_reset() time when the second >> device tries to use the reset control in a different way than the >> first. >> >> [1] https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fdocs.kernel.org%2fdriver%2dapi%2freset.html%23assertion%2dand%2ddeassertion&umid=0ba4c26a-9b7a-4e4b-8dba-ac7f2f194fcd&auth=d807158c60b7d2502abde8a2fc01f40662980862-4bd780a2a258eadb798324d4af563d691f01efb6 >> [2] https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fdocs.kernel.org%2fdriver%2dapi%2freset.html%23c.reset%5fcontrol%5fdeassert&umid=0ba4c26a-9b7a-4e4b-8dba-ac7f2f194fcd&auth=d807158c60b7d2502abde8a2fc01f40662980862-d6349120c92e1887c5765842e10b274192584bde >> [3] https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fdocs.kernel.org%2fdriver%2dapi%2freset.html%23triggering&umid=0ba4c26a-9b7a-4e4b-8dba-ac7f2f194fcd&auth=d807158c60b7d2502abde8a2fc01f40662980862-973380c68f114aad02c47e69b5ceb92a23759963 >> >>>> For example, say we want to share a reset GPIO between two devices. Each >>>> device has the following constraints: >>>> >>>> device post-assert delay post-deassert delay >>>> ====== ================= =================== >>>> A 500us 1ms >>>> B 1ms 300us >>> >>> And now imagine that these values are incompatible between them, so >>> using 1ms on device A is wrong - too long. >>> >>> This is just not doable. You invented some imaginary case to prove that >>> hardware is broken. >>> >>> Now, if we are back to realistic cases - use just the longest reset time. >> >> Right. This all only works if no device has an upper bound to the >> allowed delays on the shared reset line. > > If device had an upper bound, it would be quite a conflicting design, > tricky to implement. I don't think we should target such case with > generic solution. This is why I had explicit properties for the various durations. That way the system integrator can go through the reset requirements and specify something which satisfies all devices. --Sean
