Hi,
On 18-11-15 12:59, Philipp Zabel wrote:
Hi Hans,
Am Mittwoch, den 18.11.2015, 11:38 +0100 schrieb Hans de Goede:
Hi,
On 18-11-15 10:46, Philipp Zabel wrote:
Hi Hans,
Am Montag, den 16.11.2015, 18:13 +0100 schrieb Hans de Goede:
On 16-11-15 18:01, Philipp Zabel wrote:
If there are two devices sharing the same reset line that is initially
held asserted, do the two drivers somehow have to synchronize before
releasing the reset together?
Not to my knowledge, I suggest that we simply treat this same as
regulators / clocks where the first one to enable it actually gets
it enabled (de-asserted in case of a reset), and the last one
to disable (assert) it (so dropping the usage counter back to 0) leads
to it being asserted again.
This seems to work well enough for clocks / regulators and phys, and
at least for my use-case it should work fine for the shared reset too
(I will test to make sure of course).
So from my pov a simple counter should suffice, does that work for you ?
I don't quite understand what counting will help with, then. The first
driver to call reset_control_deassert will deassert the reset, whether
you count or not.
But if the two drivers have deasserted an initially asserted reset, a
reset_control_assert for one of them will silently fail.
Correct, which is what we want, although I would not call it silently
fail I would call it a nop as it is intended.
I fear the deassertion count maps well to the case where resets are used
just like clocks (when inactive modules are kept in reset), but I'm not
sure this is useful in the case of resets that are kept deasserted most
of the time, only to be asserted for a short pulse. Maybe we have to
differentiate between the two cases?
Ack. I think that the "just like clocks" case is the more common one,
We have to accommodate both.
and it seems to me that the short-pulse case should use reset_control_reset.
reset_control_reset can only be used if the reset controller knows the
necessary reset pulse duration itself.
Maybe we need to provide a default implementation of reset_control_reset which
does the pulse when no implementation is provided by the driver ?
I have thought about adding a version that takes a delay parameter, but
then you'd end up with udelay and msleep variants.
Although that brings the question with it what to do with the deassert_count in
that case, as some drivers may also use that for the initial deassert... I guess
we could document to not do that if you want to assure that no other drivers
muck with the reset-line ...
Hmm, this is getting messy pretty quickly. New proposal:
1) Add a concept of shared resets, adding: reset_control_get_shared and
devm_reset_control_get_shared functions, which set a shared bool
in struct reset_control
Yes, that is a good idea. And disallow reset_control_get (non-shared) on
an already in-use control.
2) Add int refcnt to struct reset_controller_dev, which gets
incremented/decremented on reset_control_get/reset_control_put
do a BUG_ON on refcnt == 1 in the get functions when the non-shared
variant gets called (this is optional but probably a good extra
check)
Maybe add a new structure between reset_controller_dev and
reset_control, or keep the reset_control structures themselves on a list
and hand them out again for reset_control_get_shared and do the
reference counting there?
Otherwise the reset_controller_dev would have to allocate a (possibly
huge, sparse) array of reference counters.
I see the problem here. I will see what I can come up with. Not sure when
exactly I will have time to work on this, may be a bit before you see
an actual patch for this from me.
3) Do the whole deassert_count thingy only when the shared bool is true
4) Make reset_control_reset fail (BUG_ON) if the shared bool is true
Sounds good, though I'd prefer WARN_ON over BUG_ON.
Ack, I always mix the 2 up I meant WARN_ON.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
