From: Philipp Zabel <p.zabel@pengutronix.de>
To: Claudiu.Beznea@microchip.com, robh+dt@kernel.org,
Nicolas.Ferre@microchip.com, alexandre.belloni@bootlin.com,
sre@kernel.org
Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org
Subject: Re: [PATCH 6/8] power: reset: at91-reset: add reset_controller_dev support
Date: Tue, 05 Apr 2022 17:15:06 +0200 [thread overview]
Message-ID: <f90d23f8c781d75bd0d171e1ab4e99c1d217d1ff.camel@pengutronix.de> (raw)
In-Reply-To: <9683a951-160a-b4e4-9494-c2e6ee51582e@microchip.com>
Hi Claudio,
On Di, 2022-04-05 at 14:47 +0000, Claudiu.Beznea@microchip.com wrote:
> Hi, Philipp,
>
> On 05.04.2022 14:47, Philipp Zabel wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> >
> > Hi Claudiu,
> >
> > On Di, 2022-04-05 at 14:27 +0300, Claudiu Beznea wrote:
> > > SAMA7G5 reset controller has 5 extra lines that goes to different
> > > devices
> > > (3 lines to USB PHYs, 1 line to DDR controller, one line DDR PHY
> > > controller). These reset lines could be requested by different
> > > controller
> > > drivers (e.g. USB PHY driver) and these controllers' drivers
> > > could
> > > assert/deassert these lines when necessary. Thus add support for
> > > reset_controller_dev which brings this functionality.
> > >
> > > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> > > ---
> > > drivers/power/reset/at91-reset.c | 92
> > > ++++++++++++++++++++++++++++++--
> > > 1 file changed, 88 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/power/reset/at91-reset.c
> > > b/drivers/power/reset/at91-reset.c
> > > index 0d721e27f545..b04df54c15d2 100644
> > > --- a/drivers/power/reset/at91-reset.c
> > > +++ b/drivers/power/reset/at91-reset.c
> > > @@ -17,6 +17,7 @@
> > > #include <linux/of_address.h>
> > > #include <linux/platform_device.h>
> > > #include <linux/reboot.h>
> > > +#include <linux/reset-controller.h>
> > >
> > > #include <soc/at91/at91sam9_ddrsdr.h>
> > > #include <soc/at91/at91sam9_sdramc.h>
> > > @@ -53,12 +54,16 @@ enum reset_type {
> > > struct at91_reset {
> > > void __iomem *rstc_base;
> > > void __iomem *ramc_base[2];
> > > + void __iomem *dev_base;
> > > + struct reset_controller_dev rcdev;
> > > struct clk *sclk;
> > > struct notifier_block nb;
> > > u32 args;
> > > u32 ramc_lpr;
> > > };
> > >
> > > +#define to_at91_reset(r) container_of(r, struct
> > > at91_reset, rcdev)
> > > +
> > > struct at91_reset_data {
> > > u32 reset_args;
> > > u32 n_device_reset;
> > > @@ -191,6 +196,79 @@ static const struct of_device_id
> > > at91_reset_of_match[] = {
> > > };
> > > MODULE_DEVICE_TABLE(of, at91_reset_of_match);
> > >
> > > +static int at91_reset_update(struct reset_controller_dev *rcdev,
> > > + unsigned long id, bool assert)
> > > +{
> > > + struct at91_reset *reset = to_at91_reset(rcdev);
> > > + u32 val;
> > > +
> > > + val = readl_relaxed(reset->dev_base);
> > > + if (assert)
> > > + val |= BIT(id);
> > > + else
> > > + val &= ~BIT(id);
> > > + writel_relaxed(val, reset->dev_base);
> >
> > This read-modify-update should be protected by a spinlock.
> >
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int at91_reset_assert(struct reset_controller_dev *rcdev,
> > > + unsigned long id)
> > > +{
> > > + return at91_reset_update(rcdev, id, true);
> > > +}
> > > +
> > > +static int at91_reset_deassert(struct reset_controller_dev
> > > *rcdev,
> > > + unsigned long id)
> > > +{
> > > + return at91_reset_update(rcdev, id, false);
> > > +}
> > > +
> > > +static int at91_reset_dev_status(struct reset_controller_dev
> > > *rcdev,
> > > + unsigned long id)
> > > +{
> > > + struct at91_reset *reset = to_at91_reset(rcdev);
> > > + u32 val;
> > > +
> > > + val = readl_relaxed(reset->dev_base);
> > > +
> > > + return !!(val & BIT(id));
> > > +}
> > > +
> > > +static const struct reset_control_ops at91_reset_ops = {
> > > + .assert = at91_reset_assert,
> > > + .deassert = at91_reset_deassert,
> > > + .status = at91_reset_dev_status,
> > > +};
> > > +
> > > +static int at91_reset_of_xlate(struct reset_controller_dev
> > > *rcdev,
> > > + const struct of_phandle_args
> > > *reset_spec)
> > > +{
> > > + return reset_spec->args[0];
> > > +}
> >
> > For 1:1 mappings there is no need for a custom of_xlate handler.
> > Just
> > leave of_xlate and of_reset_n_cells empty.
>
> I've double checked that. This would work if reset ids are continuous
> from
> zero to rcdev.nr_resets. This the of_reset_simple_xlate:
>
> static int of_reset_simple_xlate(struct reset_controller_dev *rcdev,
> const struct of_phandle_args
> *reset_spec)
> {
> if (reset_spec->args[0] >= rcdev->nr_resets)
> return -EINVAL;
> return reset_spec->args[0];
> }
>
> But in this driver's case we have 3 ids: 4, 5, 6. That is the reason
> I had this simple xlate function.
I see. In that case I'd say keep the custom of_xlate but let it return
-EINVAL if the args[0] value is not 4, 5, or 6.
Or you could set nr_resets to 7, but unless there are more resets at
the lower bits, that wouldn't necessarily be better.
regards
Philipp
next prev parent reply other threads:[~2022-04-06 0:26 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-05 11:27 [PATCH 0/8] power: reset: at91-reset: add support for sama7g5 Claudiu Beznea
2022-04-05 11:27 ` [PATCH 1/8] ARM: dts: at91: use generic name for reset controller Claudiu Beznea
2022-04-05 11:27 ` [PATCH 2/8] dt-bindings: reset: convert Atmel/Microchip reset controller to YAML Claudiu Beznea
2022-04-06 18:33 ` Rob Herring
2022-04-05 11:27 ` [PATCH 3/8] dt-bindings: reset: atmel,at91sam9260-reset: add sama7g5 bindings Claudiu Beznea
2022-04-06 18:34 ` Rob Herring
2022-04-05 11:27 ` [PATCH 4/8] dt-bindings: reset: add sama7g5 definitions Claudiu Beznea
2022-04-05 15:09 ` Philipp Zabel
2022-04-05 15:39 ` Claudiu.Beznea
2022-04-06 18:35 ` Rob Herring
2022-04-05 11:27 ` [PATCH 5/8] power: reset: at91-reset: add at91_reset_data Claudiu Beznea
2022-04-05 11:27 ` [PATCH 6/8] power: reset: at91-reset: add reset_controller_dev support Claudiu Beznea
2022-04-05 11:47 ` Philipp Zabel
2022-04-05 13:19 ` Claudiu.Beznea
2022-04-05 14:47 ` Claudiu.Beznea
2022-04-05 15:15 ` Philipp Zabel [this message]
2022-04-05 15:42 ` Claudiu.Beznea
2022-04-05 11:27 ` [PATCH 7/8] power: reset: at91-reset: add support for SAMA7G5 Claudiu Beznea
2022-04-05 11:27 ` [PATCH 8/8] ARM: dts: at91: sama7g5: add reset-controller node Claudiu Beznea
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=f90d23f8c781d75bd0d171e1ab4e99c1d217d1ff.camel@pengutronix.de \
--to=p.zabel@pengutronix.de \
--cc=Claudiu.Beznea@microchip.com \
--cc=Nicolas.Ferre@microchip.com \
--cc=alexandre.belloni@bootlin.com \
--cc=devicetree@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=robh+dt@kernel.org \
--cc=sre@kernel.org \
/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).