Re: [linux-sunxi] Re: [PATCH 2/2] ehci-platform: Add support for controllers with multiple reset lines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 18-11-15 13:32, Julian Calaby wrote:
Hi,

On Wed, Nov 18, 2015 at 9:38 PM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
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, and
it seems to me that the short-pulse case should use reset_control_reset.

Maybe we need to provide a default implementation of reset_control_reset
which
does the pulse when no implementation is provided by the driver ?

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

Assuming board designers are sufficiently ... stupid, won't most
drivers eventually need to use the _shared variants? Wouldn't it be
(horrifically more complicated and) better to just build this into the
generic code and switch on the shared reset line handling if multiple
devices take references to the same reset line? (Can we run through
the device tree and mark them in advance? What happens with overlays?)

That said, I suspect that to deal sensibly with shared reset lines
we're also going to need some way to init devices in lockstep with
others, i.e. we can't deassert device A until it's clocks are running,
however the reset line is shared with device B who also can't be
deasserted until it's clocks are running. This seems like it'll get
rather complicated quickly, to say the least.

It almost seems that, unless some other board has a similarly broken
design, that it might almost be easier to just make variants of
generic-ehci and generic-ohci to handle this one particular case.

Having a specific version of the ehci / ohci driver is not going to
help, unless a lot of magic is added to these drivers to coordinate
between themselves. Magic which is not strictly necessary, as a simple
usage-counter in the reset-core suffices and is much more KISS.

I believe that the solution is to add a usage-counting variant
of reset_control_[de]assert and switch to that in the case where the
only needed coordination is to not assert the reset when one of the
2 drivers is still active.

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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux