From: Philipp Zabel <p.zabel@pengutronix.de>
To: Matthias Brugger <matthias.bgg@gmail.com>,
Crystal Guo <crystal.guo@mediatek.com>,
robh+dt@kernel.org
Cc: srv_heupstream@mediatek.com, linux-mediatek@lists.infradead.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
seiya.wang@mediatek.com
Subject: Re: [PATCH 1/2] reset-controller: ti: adjust the reset assert and deassert interface
Date: Wed, 29 Jul 2020 10:02:16 +0200 [thread overview]
Message-ID: <d259a74ca9e425f9b39ebbf47b0decb6be0beed5.camel@pengutronix.de> (raw)
In-Reply-To: <ba0d1e29-3ba3-5379-d03e-1ccec21c2ffa@gmail.com>
Hi Crystal, Matthias,
On Wed, 2020-07-29 at 09:48 +0200, Matthias Brugger wrote:
>
> On 29/07/2020 09:39, Crystal Guo wrote:
> > Add ti_syscon_reset() to integrate assert and deassert together,
> > and change return value of the reset assert and deassert interface
> > from regmap_update_bits to regmap_write_bits.
> >
> > when clear bit is already 1, regmap_update_bits can not write 1 to it again.
> > Some IC has the feature that, when set bit is 1, the clear bit change
> > to 1 together. It will truly clear bit to 0 by write 1 to the clear bit
> >
> > Signed-off-by: Crystal Guo <crystal.guo@mediatek.com>
> > ---
> > drivers/reset/reset-ti-syscon.c | 13 +++++++++++--
> > 1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/reset/reset-ti-syscon.c b/drivers/reset/reset-ti-syscon.c
> > index a2635c2..5a8ec8f 100644
> > --- a/drivers/reset/reset-ti-syscon.c
> > +++ b/drivers/reset/reset-ti-syscon.c
> > @@ -89,7 +89,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);
>
> Nack, this will break the driver for the other devices.
I don't think this will break the driver for existing hardware.
regmap_write_bits() is the same as regmap_update_bits(), it just forces
the write in case the read already happens to return the correct value.
Of course it would be good to check that this actually works.
> The kernel has to work not just for your SoC but for all devices of all
> architectures. You can't just hack something up, that will work on your specific
> SoC.
>
> Regards,
> Matthias
>
> > }
> >
> > /**
> > @@ -120,7 +120,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);
> > }
> >
> > /**
> > @@ -158,10 +158,19 @@ static int ti_syscon_reset_status(struct reset_controller_dev *rcdev,
> > !(control->flags & STATUS_SET);
> > }
> >
> > +static int ti_syscon_reset(struct reset_controller_dev *rcdev,
> > + unsigned long id)
> > +{
> > + ti_syscon_reset_assert(rcdev, id);
> > +
> > + return ti_syscon_reset_deassert(rcdev, id);
> > +}
> > +
I'm unsure about this one, though. This is an incompatible change. At
the very least this would have to be optional depending on compatible.
regards
Philipp
next prev parent reply other threads:[~2020-07-29 8:02 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-29 7:39 [PATCH 0/2] adjust the reset assert and deassert interface Crystal Guo
2020-07-29 7:39 ` [PATCH 1/2] reset-controller: ti: " Crystal Guo
2020-07-29 7:48 ` Matthias Brugger
2020-07-29 8:02 ` Philipp Zabel [this message]
2020-07-30 6:27 ` Crystal Guo
2020-07-29 7:39 ` [PATCH 2/2] arm64: dts: mt8192: add infracfg_rst node Crystal Guo
2020-07-29 7:45 ` Matthias Brugger
2020-07-29 8:18 ` Crystal Guo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d259a74ca9e425f9b39ebbf47b0decb6be0beed5.camel@pengutronix.de \
--to=p.zabel@pengutronix.de \
--cc=crystal.guo@mediatek.com \
--cc=devicetree@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=matthias.bgg@gmail.com \
--cc=robh+dt@kernel.org \
--cc=seiya.wang@mediatek.com \
--cc=srv_heupstream@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).