v6:
fix the format error of mediatek-syscon-reset.yaml
v5:
1. revert ti-syscon-reset.txt, and add a new mediatek reset binding.
2. split the patch [v4, 3/4] with the change to force write and the
change to integrate assert and deassert together.
3. separate the dts patch from this patch sets
v4:
fix typos on v3 commit message.
v3:
1. revert v2 changes.
2. add 'reset-duration-us' property to declare a minimum delay,
which needs to be waited between assert and deassert.
3. add 'mediatek,infra-reset' to compatible.
v2 changes:
https://patchwork.kernel.org/patch/11697371/
1. add 'assert-deassert-together' property to introduce a new reset handler,
which allows device to do serialized assert and deassert operations in a single
step by 'reset' method.
2. add 'update-force' property to introduce force-update method, which forces
the write operation in case the read already happens to return the correct value.
3. add 'generic-reset' to compatible
v1 changes:
https://patchwork.kernel.org/patch/11690523/
https://patchwork.kernel.org/patch/11690527/
Crystal Guo (3):
dt-binding: reset-controller: mediatek: add YAML schemas
reset-controller: ti: introduce a new reset handler
reset-controller: ti: force the write operation when assert or
deassert
.../bindings/reset/mediatek-syscon-reset.yaml | 51 +++++++++++++++++++
drivers/reset/reset-ti-syscon.c | 44 ++++++++++++++--
2 files changed, 92 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/reset/mediatek-syscon-reset.yaml
Force the write operation in case the read already happens
to return the correct value.
Signed-off-by: Crystal Guo <[email protected]>
---
drivers/reset/reset-ti-syscon.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c
index 5d1f8306cd4f..c34394f1e9e2 100644
--- a/drivers/reset/reset-ti-syscon.c
+++ b/drivers/reset/reset-ti-syscon.c
@@ -97,7 +97,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev,
mask = BIT(control->assert_bit);
value = (control->flags & ASSERT_SET) ? mask : 0x0;
- return regmap_update_bits(data->regmap, control->assert_offset, mask, value);
+ return regmap_write_bits(data->regmap, control->assert_offset, mask, value);
}
/**
@@ -128,7 +128,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev,
mask = BIT(control->deassert_bit);
value = (control->flags & DEASSERT_SET) ? mask : 0x0;
- return regmap_update_bits(data->regmap, control->deassert_offset, mask, value);
+ return regmap_write_bits(data->regmap, control->deassert_offset, mask, value);
}
/**
--
2.18.0
On Wed, Sep 30, 2020 at 10:21:59AM +0800, Crystal Guo wrote:
> Force the write operation in case the read already happens
> to return the correct value.
>
> Signed-off-by: Crystal Guo <[email protected]>
> ---
> drivers/reset/reset-ti-syscon.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c
> index 5d1f8306cd4f..c34394f1e9e2 100644
> --- a/drivers/reset/reset-ti-syscon.c
> +++ b/drivers/reset/reset-ti-syscon.c
> @@ -97,7 +97,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev,
> mask = BIT(control->assert_bit);
> value = (control->flags & ASSERT_SET) ? mask : 0x0;
>
> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value);
> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value);
> }
I don't think there are no reset controllers with cached regmap,
thus I don't think this is needed.
Are there any specific reasons behind this, what I've missed here?
We need to be sure that all other devices using this driver
should have no side effects on write.
I can think of a weird controller doing unwanted things internally
on every write disregarding the current state. (or is this overly
paranoid?)
>
> /**
> @@ -128,7 +128,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev,
> mask = BIT(control->deassert_bit);
> value = (control->flags & DEASSERT_SET) ? mask : 0x0;
>
> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value);
> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value);
> }
>
> /**
On Mon, 2020-11-30 at 19:13 +0800, Ikjoon Jang wrote:
> On Wed, Sep 30, 2020 at 10:21:59AM +0800, Crystal Guo wrote:
> > Force the write operation in case the read already happens
> > to return the correct value.
> >
> > Signed-off-by: Crystal Guo <[email protected]>
> > ---
> > drivers/reset/reset-ti-syscon.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c
> > index 5d1f8306cd4f..c34394f1e9e2 100644
> > --- a/drivers/reset/reset-ti-syscon.c
> > +++ b/drivers/reset/reset-ti-syscon.c
> > @@ -97,7 +97,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev,
> > mask = BIT(control->assert_bit);
> > value = (control->flags & ASSERT_SET) ? mask : 0x0;
> >
> > - return regmap_update_bits(data->regmap, control->assert_offset, mask, value);
> > + return regmap_write_bits(data->regmap, control->assert_offset, mask, value);
> > }
>
> I don't think there are no reset controllers with cached regmap,
> thus I don't think this is needed.
> Are there any specific reasons behind this, what I've missed here?
>
> We need to be sure that all other devices using this driver
> should have no side effects on write.
>
> I can think of a weird controller doing unwanted things internally
> on every write disregarding the current state. (or is this overly
> paranoid?)
>
The specific reason is that, the clear bit may keep the same value with
the set bit, but the clear operation can be only be completed by writing
1 to the clear bit, not just with the current fake state "1".
> >
> > /**
> > @@ -128,7 +128,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev,
> > mask = BIT(control->deassert_bit);
> > value = (control->flags & DEASSERT_SET) ? mask : 0x0;
> >
> > - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value);
> > + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value);
> > }
> >
> > /**
On Wed, Dec 2, 2020 at 7:07 PM Crystal Guo <[email protected]> wrote:
>
> On Mon, 2020-11-30 at 19:13 +0800, Ikjoon Jang wrote:
> > On Wed, Sep 30, 2020 at 10:21:59AM +0800, Crystal Guo wrote:
> > > Force the write operation in case the read already happens
> > > to return the correct value.
> > >
> > > Signed-off-by: Crystal Guo <[email protected]>
> > > ---
> > > drivers/reset/reset-ti-syscon.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c
> > > index 5d1f8306cd4f..c34394f1e9e2 100644
> > > --- a/drivers/reset/reset-ti-syscon.c
> > > +++ b/drivers/reset/reset-ti-syscon.c
> > > @@ -97,7 +97,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev,
> > > mask = BIT(control->assert_bit);
> > > value = (control->flags & ASSERT_SET) ? mask : 0x0;
> > >
> > > - return regmap_update_bits(data->regmap, control->assert_offset, mask, value);
> > > + return regmap_write_bits(data->regmap, control->assert_offset, mask, value);
> > > }
> >
> > I don't think there are no reset controllers with cached regmap,
> > thus I don't think this is needed.
> > Are there any specific reasons behind this, what I've missed here?
> >
> > We need to be sure that all other devices using this driver
> > should have no side effects on write.
> >
> > I can think of a weird controller doing unwanted things internally
> > on every write disregarding the current state. (or is this overly
> > paranoid?)
> >
> The specific reason is that, the clear bit may keep the same value with
> the set bit, but the clear operation can be only be completed by writing
> 1 to the clear bit, not just with the current fake state "1".
>
sorry. I didn't think of that case,
then I think this patch must be applied. 8-)
I've thought that the bit automatically flipped to the current reset state
after the internal operation is done.
> > >
> > > /**
> > > @@ -128,7 +128,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev,
> > > mask = BIT(control->deassert_bit);
> > > value = (control->flags & DEASSERT_SET) ? mask : 0x0;
> > >
> > > - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value);
> > > + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value);
> > > }
> > >
> > > /**
>
On Wed, 2020-09-30 at 10:21 +0800, Crystal Guo wrote:
> Force the write operation in case the read already happens
> to return the correct value.
>
> Signed-off-by: Crystal Guo <[email protected]>
> ---
> drivers/reset/reset-ti-syscon.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c
> index 5d1f8306cd4f..c34394f1e9e2 100644
> --- a/drivers/reset/reset-ti-syscon.c
> +++ b/drivers/reset/reset-ti-syscon.c
> @@ -97,7 +97,7 @@ static int ti_syscon_reset_assert(struct reset_controller_dev *rcdev,
> mask = BIT(control->assert_bit);
> value = (control->flags & ASSERT_SET) ? mask : 0x0;
>
> - return regmap_update_bits(data->regmap, control->assert_offset, mask, value);
> + return regmap_write_bits(data->regmap, control->assert_offset, mask, value);
> }
>
> /**
> @@ -128,7 +128,7 @@ static int ti_syscon_reset_deassert(struct reset_controller_dev *rcdev,
> mask = BIT(control->deassert_bit);
> value = (control->flags & DEASSERT_SET) ? mask : 0x0;
>
> - return regmap_update_bits(data->regmap, control->deassert_offset, mask, value);
> + return regmap_write_bits(data->regmap, control->deassert_offset, mask, value);
> }
>
> /**
> --
> 2.18.0
Thank you. Since Suman tested v4, this should be safe.
Applied to reset/next.
regards
Philipp