| Message ID | e7e2c84206f2cc8c0cb36cd734226f73f3331198.1582533919.git-series.maxime@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vc4: Support BCM2711 Display Pipeline | expand |
Hi Maxime, On Mon, 2020-02-24 at 10:06 +0100, Maxime Ripard wrote: > The reset-simple code lacks a reset callback that is still pretty easy to > implement. The only real thing to consider is the delay needed for a device > to be reset, so let's expose that as part of the reset-simple driver data. > > Cc: Philipp Zabel <p.zabel@pengutronix.de> > Signed-off-by: Maxime Ripard <maxime@cerno.tech> This shoulod be done in such a way that simple reset drivers which do not set the reset delay continue to return -ENOTSUPP from reset_control_reset(). > --- > drivers/reset/reset-simple.c | 21 +++++++++++++++++++++ > include/linux/reset/reset-simple.h | 4 ++++ > 2 files changed, 25 insertions(+) > > diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c > index c854aa351640..7a8c56512ae9 100644 > --- a/drivers/reset/reset-simple.c > +++ b/drivers/reset/reset-simple.c > @@ -11,6 +11,7 @@ > * Maxime Ripard <maxime.ripard@free-electrons.com> > */ > > +#include <linux/delay.h> > #include <linux/device.h> > #include <linux/err.h> > #include <linux/io.h> > @@ -63,6 +64,25 @@ static int reset_simple_deassert(struct reset_controller_dev *rcdev, > return reset_simple_update(rcdev, id, false); > } > > +static int reset_simple_reset(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + struct reset_simple_data *data = to_reset_simple_data(rcdev); > + int ret; You could just return -ENOTSUPP here if data->reset_ms == 0. > + ret = reset_simple_assert(rcdev, id); > + if (ret) > + return ret; > + > + mdelay(data->reset_ms); Have you considered specifying the delay in microseconds instead? That would allow to use usleep_range() for shorter delays. > + ret = reset_simple_deassert(rcdev, id); > + if (ret) > + return ret; > + > + return 0; > +} > + > static int reset_simple_status(struct reset_controller_dev *rcdev, > unsigned long id) > { > @@ -80,6 +100,7 @@ static int reset_simple_status(struct reset_controller_dev *rcdev, > const struct reset_control_ops reset_simple_ops = { > .assert = reset_simple_assert, > .deassert = reset_simple_deassert, > + .reset = reset_simple_reset, > .status = reset_simple_status, > }; > EXPORT_SYMBOL_GPL(reset_simple_ops); > diff --git a/include/linux/reset/reset-simple.h b/include/linux/reset/reset-simple.h > index 08ccb25a55e6..a5887f6cbe50 100644 > --- a/include/linux/reset/reset-simple.h > +++ b/include/linux/reset/reset-simple.h > @@ -27,6 +27,9 @@ > * @status_active_low: if true, bits read back as cleared while the reset is > * asserted. Otherwise, bits read back as set while the > * reset is asserted. > + * @reset_ms: Minimum delay in milliseconds needed that needs to be > + * waited for between an assert and a deassert to reset the > + * device. If multiple consumers with different delay requirements are connected to this reset controllers, this must the largest minimum delay. Could you add mention for this in the comment? regards Philipp
diff --git a/drivers/reset/reset-simple.c b/drivers/reset/reset-simple.c index c854aa351640..7a8c56512ae9 100644 --- a/drivers/reset/reset-simple.c +++ b/drivers/reset/reset-simple.c @@ -11,6 +11,7 @@ * Maxime Ripard <maxime.ripard@free-electrons.com> */ +#include <linux/delay.h> #include <linux/device.h> #include <linux/err.h> #include <linux/io.h> @@ -63,6 +64,25 @@ static int reset_simple_deassert(struct reset_controller_dev *rcdev, return reset_simple_update(rcdev, id, false); } +static int reset_simple_reset(struct reset_controller_dev *rcdev, + unsigned long id) +{ + struct reset_simple_data *data = to_reset_simple_data(rcdev); + int ret; + + ret = reset_simple_assert(rcdev, id); + if (ret) + return ret; + + mdelay(data->reset_ms); + + ret = reset_simple_deassert(rcdev, id); + if (ret) + return ret; + + return 0; +} + static int reset_simple_status(struct reset_controller_dev *rcdev, unsigned long id) { @@ -80,6 +100,7 @@ static int reset_simple_status(struct reset_controller_dev *rcdev, const struct reset_control_ops reset_simple_ops = { .assert = reset_simple_assert, .deassert = reset_simple_deassert, + .reset = reset_simple_reset, .status = reset_simple_status, }; EXPORT_SYMBOL_GPL(reset_simple_ops); diff --git a/include/linux/reset/reset-simple.h b/include/linux/reset/reset-simple.h index 08ccb25a55e6..a5887f6cbe50 100644 --- a/include/linux/reset/reset-simple.h +++ b/include/linux/reset/reset-simple.h @@ -27,6 +27,9 @@ * @status_active_low: if true, bits read back as cleared while the reset is * asserted. Otherwise, bits read back as set while the * reset is asserted. + * @reset_ms: Minimum delay in milliseconds needed that needs to be + * waited for between an assert and a deassert to reset the + * device. */ struct reset_simple_data { spinlock_t lock; @@ -34,6 +37,7 @@ struct reset_simple_data { struct reset_controller_dev rcdev; bool active_low; bool status_active_low; + unsigned int reset_ms; }; extern const struct reset_control_ops reset_simple_ops;
The reset-simple code lacks a reset callback that is still pretty easy to implement. The only real thing to consider is the delay needed for a device to be reset, so let's expose that as part of the reset-simple driver data. Cc: Philipp Zabel <p.zabel@pengutronix.de> Signed-off-by: Maxime Ripard <maxime@cerno.tech> --- drivers/reset/reset-simple.c | 21 +++++++++++++++++++++ include/linux/reset/reset-simple.h | 4 ++++ 2 files changed, 25 insertions(+)