Some custom IP-block connected to ARC AXS10x board need assert and
deassert functions to control reset signal of selected peripherals.
This patch improve AXS10x reset driver by adding assert and deassert
callbacks.
Signed-off-by: Vitor Soares <[email protected]>
Cc: Eugeniy Paltsev <[email protected]>
Cc: Alexey Brodkin <[email protected]>
Cc: Joao Pinto <[email protected]>
Cc: Jose Abreu <[email protected]>
Cc: Luis Oliveira <[email protected]>
Cc: Gustavo Pimentel <[email protected]>
Cc: Nelson Costa <[email protected]>
Cc: Pedro Sousa <[email protected]>
Cc: Philipp Zabel <[email protected]>
---
drivers/reset/reset-axs10x.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/reset/reset-axs10x.c b/drivers/reset/reset-axs10x.c
index a854ef41..12dac8b 100644
--- a/drivers/reset/reset-axs10x.c
+++ b/drivers/reset/reset-axs10x.c
@@ -37,8 +37,36 @@ static int axs10x_reset_reset(struct reset_controller_dev *rcdev,
return 0;
}
+static int axs10x_reset_assert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct axs10x_rst *rst = to_axs10x_rst(rcdev);
+ unsigned long flags;
+
+ spin_lock_irqsave(&rst->lock, flags);
+ writel(readl(rst->regs_rst) & ~BIT(id), rst->regs_rst);
+ spin_unlock_irqrestore(&rst->lock, flags);
+
+ return 0;
+}
+
+static int axs10x_reset_deassert(struct reset_controller_dev *rcdev,
+ unsigned long id)
+{
+ struct axs10x_rst *rst = to_axs10x_rst(rcdev);
+ unsigned long flags;
+
+ spin_lock_irqsave(&rst->lock, flags);
+ writel(readl(rst->regs_rst) | BIT(id), rst->regs_rst);
+ spin_unlock_irqrestore(&rst->lock, flags);
+
+ return 0;
+}
+
static const struct reset_control_ops axs10x_reset_ops = {
- .reset = axs10x_reset_reset,
+ .reset = axs10x_reset_reset,
+ .assert = axs10x_reset_assert,
+ .deassert = axs10x_reset_deassert,
};
static int axs10x_reset_probe(struct platform_device *pdev)
--
2.7.4
From: Vitor Soares <[email protected]>
Date: Mon, Apr 08, 2019 at 11:31:23
> static const struct reset_control_ops axs10x_reset_ops = {
> - .reset = axs10x_reset_reset,
Eugeniy, Alexey,
I think the implementation of this callback is wrong: you should
readl_poll_timeout_atomic() until the reset bit is not cleared.
Thanks,
Jose Miguel Abreu
Hi Vitor,
On Mon, 2019-04-08 at 12:31 +0200, Vitor Soares wrote:
> Some custom IP-block connected to ARC AXS10x board need assert and
> deassert functions to control reset signal of selected peripherals.
>
> This patch improve AXS10x reset driver by adding assert and deassert
> callbacks.
In the AXS10x reset driver only 'reset' callback is intentionally implemented.
AXS10x is FPGA based boards and with our default firmware AXS10x reset register is implemented as self-deasserted.
Do you have somehow modified AXS10x firmware where reset register is not self-deasserted?
In that case "simple-reset" driver will be suitable for you, I guess.
Otherwise this implementation is incorrect - there should be no 'assert' for reset controller with self-deasserted logic.
>
> Signed-off-by: Vitor Soares <[email protected]>
>
> Cc: Eugeniy Paltsev <[email protected]>
> Cc: Alexey Brodkin <[email protected]>
> Cc: Joao Pinto <[email protected]>
> Cc: Jose Abreu <[email protected]>
> Cc: Luis Oliveira <[email protected]>
> Cc: Gustavo Pimentel <[email protected]>
> Cc: Nelson Costa <[email protected]>
> Cc: Pedro Sousa <[email protected]>
> Cc: Philipp Zabel <[email protected]>
> ---
> drivers/reset/reset-axs10x.c | 30 +++++++++++++++++++++++++++++-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/reset/reset-axs10x.c b/drivers/reset/reset-axs10x.c
> index a854ef41..12dac8b 100644
> --- a/drivers/reset/reset-axs10x.c
> +++ b/drivers/reset/reset-axs10x.c
> @@ -37,8 +37,36 @@ static int axs10x_reset_reset(struct reset_controller_dev *rcdev,
> return 0;
> }
>
> +static int axs10x_reset_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct axs10x_rst *rst = to_axs10x_rst(rcdev);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&rst->lock, flags);
> + writel(readl(rst->regs_rst) & ~BIT(id), rst->regs_rst);
> + spin_unlock_irqrestore(&rst->lock, flags);
> +
> + return 0;
> +}
> +
> +static int axs10x_reset_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct axs10x_rst *rst = to_axs10x_rst(rcdev);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&rst->lock, flags);
> + writel(readl(rst->regs_rst) | BIT(id), rst->regs_rst);
> + spin_unlock_irqrestore(&rst->lock, flags);
> +
> + return 0;
> +}
> +
> static const struct reset_control_ops axs10x_reset_ops = {
> - .reset = axs10x_reset_reset,
> + .reset = axs10x_reset_reset,
> + .assert = axs10x_reset_assert,
> + .deassert = axs10x_reset_deassert,
> };
>
> static int axs10x_reset_probe(struct platform_device *pdev)
--
Eugeniy Paltsev
Hi Eugeniy,
From: Eugeniy Paltsev <[email protected]>
Date: Mon, Apr 08, 2019 at 12:40:00
> Hi Vitor,
>
> On Mon, 2019-04-08 at 12:31 +0200, Vitor Soares wrote:
> > Some custom IP-block connected to ARC AXS10x board need assert and
> > deassert functions to control reset signal of selected peripherals.
> >
> > This patch improve AXS10x reset driver by adding assert and deassert
> > callbacks.
>
>
> In the AXS10x reset driver only 'reset' callback is intentionally
> > implemented.
> AXS10x is FPGA based boards and with our default firmware AXS10x reset
> > register is implemented as self-deasserted.
>
I have another reset block connect through AXI.
> Do you have somehow modified AXS10x firmware where reset register is not
> > self-deasserted?
> In that case "simple-reset" driver will be suitable for you, I guess.
I will try it.
> Otherwise this implementation is incorrect - there should be no 'assert' for
> > reset controller with self-deasserted logic.
So the assert and reset callback are mutually exclusive?
>
>
> >
> > Signed-off-by: Vitor Soares <[email protected]>
> >
> > Cc: Eugeniy Paltsev <[email protected]>
> > Cc: Alexey Brodkin <[email protected]>
> > Cc: Joao Pinto <[email protected]>
> > Cc: Jose Abreu <[email protected]>
> > Cc: Luis Oliveira <[email protected]>
> > Cc: Gustavo Pimentel <[email protected]>
> > Cc: Nelson Costa <[email protected]>
> > Cc: Pedro Sousa <[email protected]>
> > Cc: Philipp Zabel <[email protected]>
> > ---
> > drivers/reset/reset-axs10x.c | 30 +++++++++++++++++++++++++++++-
> > 1 file changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/reset/reset-axs10x.c b/drivers/reset/reset-axs10x.c
> > index a854ef41..12dac8b 100644
> > --- a/drivers/reset/reset-axs10x.c
> > +++ b/drivers/reset/reset-axs10x.c
> > @@ -37,8 +37,36 @@ static int axs10x_reset_reset(struct
> > reset_controller_dev *rcdev,
> > return 0;
> > }
> >
> > +static int axs10x_reset_assert(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct axs10x_rst *rst = to_axs10x_rst(rcdev);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&rst->lock, flags);
> > + writel(readl(rst->regs_rst) & ~BIT(id), rst->regs_rst);
> > + spin_unlock_irqrestore(&rst->lock, flags);
> > +
> > + return 0;
> > +}
> > +
> > +static int axs10x_reset_deassert(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + struct axs10x_rst *rst = to_axs10x_rst(rcdev);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&rst->lock, flags);
> > + writel(readl(rst->regs_rst) | BIT(id), rst->regs_rst);
> > + spin_unlock_irqrestore(&rst->lock, flags);
> > +
> > + return 0;
> > +}
> > +
> > static const struct reset_control_ops axs10x_reset_ops = {
> > - .reset = axs10x_reset_reset,
> > + .reset = axs10x_reset_reset,
> > + .assert = axs10x_reset_assert,
> > + .deassert = axs10x_reset_deassert,
> > };
> >
> > static int axs10x_reset_probe(struct platform_device *pdev)
> --
> Eugeniy Paltsev
>
Best regards,
Vitor Soares
Hi Vitor,
On Mon, 2019-04-08 at 13:25 +0000, Vitor Soares wrote:
> Hi Eugeniy,
>
> From: Eugeniy Paltsev <[email protected]>
> Date: Mon, Apr 08, 2019 at 12:40:00
>
> > Hi Vitor,
> >
> > On Mon, 2019-04-08 at 12:31 +0200, Vitor Soares wrote:
> > > Some custom IP-block connected to ARC AXS10x board need assert and
> > > deassert functions to control reset signal of selected peripherals.
> > >
> > > This patch improve AXS10x reset driver by adding assert and deassert
> > > callbacks.
> >
> >
> > In the AXS10x reset driver only 'reset' callback is intentionally
> > > implemented.
> >
> > AXS10x is FPGA based boards and with our default firmware AXS10x reset
> > > register is implemented as self-deasserted.
>
> I have another reset block connect through AXI.
>
> > Do you have somehow modified AXS10x firmware where reset register is not
> > > self-deasserted?
> >
> > In that case "simple-reset" driver will be suitable for you, I guess.
>
> I will try it.
>
> > Otherwise this implementation is incorrect - there should be no 'assert' for
> > > reset controller with self-deasserted logic.
>
> So the assert and reset callback are mutually exclusive?
Not actually.
Adding 'assert' callback is incorrect for exclusive reset controls in
case of self-deasserted reset controller.
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.
[snap]
>
> Best regards,
> Vitor Soares
>
--
Eugeniy Paltsev
Hi Jose,
On Mon, 2019-04-08 at 11:23 +0000, Jose Abreu wrote:
> From: Vitor Soares <[email protected]>
> Date: Mon, Apr 08, 2019 at 11:31:23
>
> > static const struct reset_control_ops axs10x_reset_ops = {
> > - .reset = axs10x_reset_reset,
>
> Eugeniy, Alexey,
>
> I think the implementation of this callback is wrong: you should
> readl_poll_timeout_atomic() until the reset bit is not cleared.
I don't remember if this register is read/write or write-only. I'll check it.
Thanks for report.
>
> Thanks,
> Jose Miguel Abreu
--
Eugeniy Paltsev
Hi Eugeniy,
From: Eugeniy Paltsev <[email protected]>
Date: Fri, Apr 12, 2019 at 14:22:33
> Hi Vitor,
>
> On Mon, 2019-04-08 at 13:25 +0000, Vitor Soares wrote:
> > Hi Eugeniy,
> >
> > From: Eugeniy Paltsev <[email protected]>
> > Date: Mon, Apr 08, 2019 at 12:40:00
> >
> > > Hi Vitor,
> > >
> > > On Mon, 2019-04-08 at 12:31 +0200, Vitor Soares wrote:
> > > > Some custom IP-block connected to ARC AXS10x board need assert and
> > > > deassert functions to control reset signal of selected peripherals.
> > > >
> > > > This patch improve AXS10x reset driver by adding assert and deassert
> > > > callbacks.
> > >
> > >
> > > In the AXS10x reset driver only 'reset' callback is intentionally
> > > > implemented.
> > >
> > > AXS10x is FPGA based boards and with our default firmware AXS10x reset
> > > > register is implemented as self-deasserted.
> >
> > I have another reset block connect through AXI.
> >
> > > Do you have somehow modified AXS10x firmware where reset register is not
> > > > self-deasserted?
> > >
> > > In that case "simple-reset" driver will be suitable for you, I guess.
> >
> > I will try it.
> >
> > > Otherwise this implementation is incorrect - there should be no 'assert' for
> > > > reset controller with self-deasserted logic.
> >
> > So the assert and reset callback are mutually exclusive?
>
> Not actually.
>
> Adding 'assert' callback is incorrect for exclusive reset controls in
> case of self-deasserted reset controller.
> 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.
>
Sorry I'm not understanding. Can you please explain?
What I see in reset_control_assert() when not shared is return -ENOTSUPP.
>
> [snap]
> >
> > Best regards,
> > Vitor Soares
> >
> --
> Eugeniy Paltsev
Thanks for your feedback.
Best regards,
Vitor Soares