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. > 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) > > 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 > > Regards, > > Hans > > > -- > You received this message because you are subscribed to the Google Groups > "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to linux-sunxi+unsubscribe@xxxxxxxxxxxxxxxx. > For more options, visit https://groups.google.com/d/optout. -- Julian Calaby Email: julian.calaby@xxxxxxxxx Profile: http://www.google.com/profiles/julian.calaby/ -- 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
