From: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
To: "robh@kernel.org" <robh@kernel.org>
Cc: "linux@roeck-us.net" <linux@roeck-us.net>,
"wim@linux-watchdog.org" <wim@linux-watchdog.org>,
"mazziesaccount@gmail.com" <mazziesaccount@gmail.com>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
linux-power <linux-power@fi.rohmeurope.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"lee.jones@linaro.org" <lee.jones@linaro.org>,
"linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>,
"broonie@kernel.org" <broonie@kernel.org>,
"lgirdwood@gmail.com" <lgirdwood@gmail.com>
Subject: Re: [PATCH v1 1/6] dt_bindings: mfd: Add ROHM BD9576MUF and BD9573MUF PMICs
Date: Thu, 24 Sep 2020 06:12:33 +0000 [thread overview]
Message-ID: <e5a5f3cb4844af421101d04bcff8534d7758c254.camel@fi.rohmeurope.com> (raw)
In-Reply-To: <CAL_JsqLj-JqnfH7eh=sR0=izK5NRBusXmwGiuDmX89cn3KA2+A@mail.gmail.com>
On Wed, 2020-09-23 at 08:27 -0600, Rob Herring wrote:
> On Sat, Sep 19, 2020 at 5:46 AM Vaittinen, Matti
> <Matti.Vaittinen@fi.rohmeurope.com> wrote:
> > Thanks Rob for taking a look at this!
> >
> > On Fri, 2020-09-18 at 11:28 -0600, Rob Herring wrote:
> > > On Thu, Sep 17, 2020 at 11:01:52AM +0300, Matti Vaittinen wrote:
> > > > Add bindings for ROHM BD9576MUF and BD9573MUF PMICs. These
> > > > PMICs are primarily intended to be used to power the R-Car
> > > > series
> > > > processors. They provide 6 power outputs, safety features and a
> > > > watchdog with two functional modes.
> > > >
> > > > Signed-off-by: Matti Vaittinen <
> > > > matti.vaittinen@fi.rohmeurope.com>
> > > > ---
> > > > .../bindings/mfd/rohm,bd9576-pmic.yaml | 129
> > > > ++++++++++++++++++
> > > > 1 file changed, 129 insertions(+)
> > > > create mode 100644
> > > > Documentation/devicetree/bindings/mfd/rohm,bd9576-pmic.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd9576-
> > > > pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd9576-
> > > > pmic.yaml
> > > > new file mode 100644
> > > > index 000000000000..f17d4d621585
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/mfd/rohm,bd9576-
> > > > pmic.yaml
> > > > @@ -0,0 +1,129 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/mfd/rohm,bd9576-pmic.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: ROHM BD9576MUF and BD9573MUF Power Management
> > > > Integrated
> > > > Circuit bindings
> > > > +
> > > > +maintainers:
> > > > + - Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
> > > > +
> > > > +description: |
> > > > + BD9576MUF and BD9573MUF are power management ICs primarily
> > > > intended for
> > > > + powering the R-Car series processors.
> > > > + The IC provides 6 power outputs with configurable sequencing
> > > > and
> > > > safety
> > > > + monitoring. A watchdog logic with slow ping/windowed modes
> > > > is
> > > > also included.
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + enum:
> > > > + - rohm,bd9576
> > > > + - rohm,bd9573
> > > > +
> > > > + reg:
> > > > + description:
> > > > + I2C slave address.
> > > > + maxItems: 1
> > > > +
> > > > + interrupts:
> > > > + maxItems: 1
> > > > +
> > > > + rohm,vout1-en-low:
> > > > + description:
> > > > + BD9576 and BD9573 VOUT1 regulator enable state can be
> > > > individually
> > > > + controlled by a GPIO. This is dictated by state of
> > > > vout1-en
> > > > pin during
> > > > + the PMIC startup. If vout1-en is LOW during PMIC startup
> > > > then the VOUT1
> > > > + enable sate is controlled via this pin. Set this
> > > > property if
> > > > vout1-en
> > > > + is wired to be down at PMIC start-up.
> > > > + type: boolean
> > > > +
> > > > + rohm,vout1-en-gpios:
> > > > + description:
> > > > + GPIO specifier to specify the GPIO connected to vout1-en
> > > > for
> > > > vout1 ON/OFF
> > > > + state control.
> > > > + maxItems: 1
> > > > +
> > > > + rohm,ddr-sel-low:
> > > > + description:
> > > > + The BD9576 and BD9573 output voltage for DDR can be
> > > > selected
> > > > by setting
> > > > + the ddr-sel pin low or high. Set this property if ddr-
> > > > sel is
> > > > grounded.
> > > > + type: boolean
> > > > +
> > > > + rohm,watchdog-enable-gpios:
> > > > + description: The GPIO line used to enable the watchdog.
> > > > + maxItems: 1
> > > > +
> > > > + rohm,watchdog-ping-gpios:
> > > > + description: The GPIO line used to ping the watchdog.
> > > > + maxItems: 1
> > > > +
> > > > + hw_margin_ms:
> > >
> > > Needs a vendor prefix.
> > >
> > > s/_/-/
> > >
> > > > + minimum: 4
> > > > + maximum: 4416
> > > > + description: Watchog timeout in milliseconds
> > >
> > > Maybe the words in the description should be in the property name
> > > as
> > > I don't see how 'h/w margin' relates to 'watchdog timeout'.
> >
> > The hw_margin_ms is an existing property. As I wrote to Guenter:
> > "hw_margin_ms" is an existing binding for specifying the maximum
> > TMO in
> > HW (if I understood it correctly). (It is used at least by the
> > generig
> > GPIO watchdog) I thought it's better to not invent a new vendor
> > specific binding when we have a generic one.
> >
> > https://elixir.bootlin.com/linux/v5.9-rc2/source/Documentation/devicetree/bindings/watchdog/gpio-wdt.txt
>
> That one is odd and I haven't found an actual user of it. It would
> make more sense as a collection of properties devices could use
> rather
> than a virtual device.
>
> I think I'd do something like 'watchdog-ping-time-msec' that can be
> either '<min> <max>' or '<max>'.
Your suggestion looks good to me. If we introduce such then it would
make sense to add handling for this in GPIO watchdog too.
What I do wonder is how "hw_margin_ms" is unused? I see it is a
required property for GPIO watchdog. The GPIO WDG probe seems to
actually error out if reading this property fails with any error. I
would assume the GPIO WDG is used somewhere? Hence I am a bit afraid of
touching it. Breaking existing setups would not be nice.
Guenter - how do you see this? Should we leave GPIO WDG as it is,
convert it to use this new binding Rob suggested - or support both the
old and new (at least for some time) in the driver - and possibly print
a warning when old is used?
Best Regards
Matti Vaittinen
next prev parent reply other threads:[~2020-09-24 6:12 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-17 8:01 [PATCH v1 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs Matti Vaittinen
2020-09-17 8:01 ` [PATCH v1 1/6] dt_bindings: mfd: Add " Matti Vaittinen
2020-09-18 17:28 ` Rob Herring
2020-09-19 11:46 ` Vaittinen, Matti
2020-09-23 14:27 ` Rob Herring
2020-09-24 6:12 ` Vaittinen, Matti [this message]
2020-09-24 9:06 ` Vaittinen, Matti
2020-09-17 8:02 ` [PATCH v1 2/6] dt_bindings: regulator: " Matti Vaittinen
2020-09-17 8:02 ` [PATCH v1 3/6] mfd: Support ROHM BD9576MUF and BD9573MUF Matti Vaittinen
2020-09-17 8:03 ` [PATCH v1 4/6] wdt: Support wdt on " Matti Vaittinen
2020-09-18 5:45 ` Guenter Roeck
2020-09-18 6:06 ` Vaittinen, Matti
2020-09-19 1:49 ` Guenter Roeck
2020-09-17 8:03 ` [PATCH v1 5/6] regulator: Support " Matti Vaittinen
2020-09-17 8:03 ` [PATCH v1 6/6] MAINTAINERS: Add ROHM BD9576MUF and BD9573MUF drivers Matti Vaittinen
2020-09-17 18:58 ` [PATCH v1 0/6] Support ROHM BD9576MUF and BD9573MUF PMICs Mark Brown
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=e5a5f3cb4844af421101d04bcff8534d7758c254.camel@fi.rohmeurope.com \
--to=matti.vaittinen@fi.rohmeurope.com \
--cc=broonie@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=lee.jones@linaro.org \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-power@fi.rohmeurope.com \
--cc=linux-watchdog@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=mazziesaccount@gmail.com \
--cc=robh@kernel.org \
--cc=wim@linux-watchdog.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).