Hi Maxime, Am Mittwoch, den 16.12.2015, 11:29 +0100 schrieb Maxime Ripard: > On Mon, Dec 14, 2015 at 10:50:55AM +0100, Philipp Zabel wrote: > > Am Montag, den 14.12.2015, 10:36 +0100 schrieb Maxime Ripard: > > > Hi, > > > > > > On Fri, Dec 11, 2015 at 04:41:58PM +0100, Hans de Goede wrote: > > > > diff --git a/include/linux/reset.h b/include/linux/reset.h > > > > index c4c097d..1cca8ce 100644 > > > > --- a/include/linux/reset.h > > > > +++ b/include/linux/reset.h > > > > @@ -11,6 +11,8 @@ int reset_control_reset(struct reset_control *rstc); > > > > int reset_control_assert(struct reset_control *rstc); > > > > int reset_control_deassert(struct reset_control *rstc); > > > > int reset_control_status(struct reset_control *rstc); > > > > +int reset_control_assert_shared(struct reset_control *rstc); > > > > +int reset_control_deassert_shared(struct reset_control *rstc); > > > > > > Shouldn't that be handled in reset_control_get directly? I think I see your point now. Maybe we should add a flags parameter to reset_control_get and/or wrap it in two versions, reset_control_get_exclusive and reset_control_get_shared (or just add the _shared variant). Then reset_control_get(_exclusive) could return -EBUSY if a reset line is already in use. > > This is about different expectations of the caller. > > A driver calling reset_control_assert expects the reset line to be > > asserted after the call. > > Is that behaviour documented explicitly somewhere? /** * reset_control_assert - asserts the reset line * @rstc: reset controller */ Also, that expected behavior matches the function name, which I like. So I still welcome adding new API calls for the shared/refcounting variant. > > A driver calling reset_control_assert_shared > > just signals that it doesn't care about the state of the reset line > > anymore. > > We could just as well call the two new functions > > reset_control_deassert_get and reset_control_deassert_put. > > What happens if you mix them? What happens if you have several drivers > ignoring this API? The core should give useful error messages and disallow non-shared reset calls on shared lines. > The current default API totally allows to have several drivers getting > the same reset line, and happily poking that reset line without any > way for the others to A) know they're not the single users B) let them > know their device has been reset. That's why I'd like the WARN_ON and error return in reset_control_* when the reset_line reference count is > 1. > And not being able to tell at the consumer level if and when our > device is going to be reset behind our back is a big issue. Because > then, we simply have to expect it can be reset at any point in time, > good luck writing a driver with that expectation. Yes, that is unacceptable. > The reset framework should make sure that the shared case is an > exception, and not the default case (and make sure that it cannot > happen in the default case). Just like for any other framework with > similar resources constraints. regards Philipp -- 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
