* [PATCH] ARC: HSDK: improve reset driver @ 2018-08-27 14:38 Eugeniy Paltsev 2018-09-11 15:25 ` Eugeniy Paltsev 2018-09-14 10:38 ` Philipp Zabel 0 siblings, 2 replies; 4+ messages in thread From: Eugeniy Paltsev @ 2018-08-27 14:38 UTC (permalink / raw) To: Philipp Zabel Cc: linux-snps-arc, linux-kernel, Alexey Brodkin, Eugeniy Paltsev As for today HSDK reset driver implements only .reset() callback. In case of driver which implements one of standard reset controller usage pattern (call *_deassert() in probe(), call *_assert() in remove()) that leads to inoperability of this reset driver. Improve HSDK reset driver by calling .reset() callback inside of .assert()/.deassert() callbacks to avoid each reset controller user adaptation for work with both reset methods (reset() and .assert()/.deassert() pair) Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> --- drivers/reset/reset-hsdk.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/reset/reset-hsdk.c b/drivers/reset/reset-hsdk.c index 8bce391c6943..1fd91df91343 100644 --- a/drivers/reset/reset-hsdk.c +++ b/drivers/reset/reset-hsdk.c @@ -86,6 +86,8 @@ static int hsdk_reset_reset(struct reset_controller_dev *rcdev, static const struct reset_control_ops hsdk_reset_ops = { .reset = hsdk_reset_reset, + .assert = hsdk_reset_reset, + .deassert = hsdk_reset_reset, }; static int hsdk_reset_probe(struct platform_device *pdev) -- 2.14.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ARC: HSDK: improve reset driver 2018-08-27 14:38 [PATCH] ARC: HSDK: improve reset driver Eugeniy Paltsev @ 2018-09-11 15:25 ` Eugeniy Paltsev 2018-09-14 10:38 ` Philipp Zabel 1 sibling, 0 replies; 4+ messages in thread From: Eugeniy Paltsev @ 2018-09-11 15:25 UTC (permalink / raw) To: p.zabel; +Cc: linux-kernel, Alexey Brodkin, linux-snps-arc Hi Philipp, Maybe you have any comments or remarks about this patch? And if you don't could you please apply it. Thanks! On Mon, 2018-08-27 at 17:38 +0300, Eugeniy Paltsev wrote: > As for today HSDK reset driver implements only > .reset() callback. > > In case of driver which implements one of standard > reset controller usage pattern > (call *_deassert() in probe(), call *_assert() in remove()) > that leads to inoperability of this reset driver. > > Improve HSDK reset driver by calling .reset() callback inside of > .assert()/.deassert() callbacks to avoid each reset controller > user adaptation for work with both reset methods > (reset() and .assert()/.deassert() pair) > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > --- > drivers/reset/reset-hsdk.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/reset/reset-hsdk.c b/drivers/reset/reset-hsdk.c > index 8bce391c6943..1fd91df91343 100644 > --- a/drivers/reset/reset-hsdk.c > +++ b/drivers/reset/reset-hsdk.c > @@ -86,6 +86,8 @@ static int hsdk_reset_reset(struct reset_controller_dev *rcdev, > > static const struct reset_control_ops hsdk_reset_ops = { > .reset = hsdk_reset_reset, > + .assert = hsdk_reset_reset, > + .deassert = hsdk_reset_reset, > }; > > static int hsdk_reset_probe(struct platform_device *pdev) -- Eugeniy Paltsev ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ARC: HSDK: improve reset driver 2018-08-27 14:38 [PATCH] ARC: HSDK: improve reset driver Eugeniy Paltsev 2018-09-11 15:25 ` Eugeniy Paltsev @ 2018-09-14 10:38 ` Philipp Zabel 2018-09-24 11:08 ` Eugeniy Paltsev 1 sibling, 1 reply; 4+ messages in thread From: Philipp Zabel @ 2018-09-14 10:38 UTC (permalink / raw) To: Eugeniy Paltsev; +Cc: linux-snps-arc, linux-kernel, Alexey Brodkin Hi Eugeniy, On Mon, 2018-08-27 at 17:38 +0300, Eugeniy Paltsev wrote: > As for today HSDK reset driver implements only > .reset() callback. > > In case of driver which implements one of standard > reset controller usage pattern > (call *_deassert() in probe(), call *_assert() in remove()) > that leads to inoperability of this reset driver. > > Improve HSDK reset driver by calling .reset() callback inside of > .assert()/.deassert() callbacks to avoid each reset controller > user adaptation for work with both reset methods > (reset() and .assert()/.deassert() pair) > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > --- > drivers/reset/reset-hsdk.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/reset/reset-hsdk.c b/drivers/reset/reset-hsdk.c > index 8bce391c6943..1fd91df91343 100644 > --- a/drivers/reset/reset-hsdk.c > +++ b/drivers/reset/reset-hsdk.c > @@ -86,6 +86,8 @@ static int hsdk_reset_reset(struct reset_controller_dev *rcdev, > > static const struct reset_control_ops hsdk_reset_ops = { > .reset = hsdk_reset_reset, > + .assert = hsdk_reset_reset, This is incorrect for exclusive reset controls. It will cause reset_control_assert() to return success for exclusive reset controls, even though the .assert op failed to leave the reset line asserted after the function returns. While calling hsdk_reset_reset from .assert for shared reset controls would be fine, I don't see how this is necessary of useful. If a consumer driver requires the reset to be asserted upon remove(), it must not request a shared reset control anyway, because with shared reset controls other drivers may keep the reset line deasserted indefinitely. > + .deassert = hsdk_reset_reset, This should be fine. I wonder from time to time whether this should be implemented in the core, in reset_control_deassert(). regards Philipp ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ARC: HSDK: improve reset driver 2018-09-14 10:38 ` Philipp Zabel @ 2018-09-24 11:08 ` Eugeniy Paltsev 0 siblings, 0 replies; 4+ messages in thread From: Eugeniy Paltsev @ 2018-09-24 11:08 UTC (permalink / raw) To: p.zabel, Eugeniy.Paltsev; +Cc: linux-kernel, Alexey.Brodkin, linux-snps-arc Hi Philip, On Fri, 2018-09-14 at 12:38 +0200, Philipp Zabel wrote: > Hi Eugeniy, > > On Mon, 2018-08-27 at 17:38 +0300, Eugeniy Paltsev wrote: > > As for today HSDK reset driver implements only > > .reset() callback. > > > > In case of driver which implements one of standard > > reset controller usage pattern > > (call *_deassert() in probe(), call *_assert() in remove()) > > that leads to inoperability of this reset driver. > > > > Improve HSDK reset driver by calling .reset() callback inside of > > .assert()/.deassert() callbacks to avoid each reset controller > > user adaptation for work with both reset methods > > (reset() and .assert()/.deassert() pair) > > > > Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com> > > --- > > drivers/reset/reset-hsdk.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/reset/reset-hsdk.c b/drivers/reset/reset-hsdk.c > > index 8bce391c6943..1fd91df91343 100644 > > --- a/drivers/reset/reset-hsdk.c > > +++ b/drivers/reset/reset-hsdk.c > > @@ -86,6 +86,8 @@ static int hsdk_reset_reset(struct reset_controller_dev *rcdev, > > > > static const struct reset_control_ops hsdk_reset_ops = { > > .reset = hsdk_reset_reset, > > + = hsdk_reset_reset, > > This is incorrect for exclusive reset controls. > It will cause reset_control_assert() to return success for exclusive > reset controls, even though the .assert op failed to leave the reset > line asserted after the function returns. > > While calling hsdk_reset_reset from .assert for shared reset controls > would be fine, I don't see how this is necessary of useful. > If a consumer driver requires the reset to be asserted upon remove(), it > must not request a shared reset control anyway, because with shared > reset controls other drivers may keep the reset line deasserted > indefinitely. Ok, I agree that doing reset from .assert isn't necessary/useful here. The reason I added hsdk_reset_reset into .assert is to prevent -ENOTSUPP returning by reset_control_assert(). Lots of drivers implement following pattern to reset HW: ------------------------->8------------------------------ reset_control_assert(resets); usleep_range(10, 1000); reset_control_deassert(resets); ------------------------->8------------------------------ And some of them check reset_control_assert() and reset_control_deassert() return status. So these driver will fail if I implement only .reset and .deassert callback in my reset driver. I can implement something like that: ------------------------->8------------------------------ static int hsdk_dummy_assert(struct reset_controller_dev *rd, unsigned long id) { return 0; } static const struct reset_control_ops hsdk_reset_ops = { .reset = hsdk_reset_reset, .assert = hsdk_dummy_assert, .deassert = hsdk_reset_reset, }; ------------------------->8------------------------------ > > + .deassert = hsdk_reset_reset, > > This should be fine. > I wonder from time to time whether this should be > implemented in the core, in reset_control_deassert(). Sounds OK for me. At least I don't see any issues it may cause. > regards > Philipp -- Eugeniy Paltsev ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-09-24 11:08 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-08-27 14:38 [PATCH] ARC: HSDK: improve reset driver Eugeniy Paltsev 2018-09-11 15:25 ` Eugeniy Paltsev 2018-09-14 10:38 ` Philipp Zabel 2018-09-24 11:08 ` Eugeniy Paltsev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).