Re: [PATCH v3 2/4] dt-bindings: regulator: add document bindings for mpq7920

From: Mark Brown <broonie@kernel.org>
To: saravanan sekar <sravanhome@gmail.com>
Cc: Maxime Ripard <mripard@kernel.org>,
	lgirdwood@gmail.com, robh+dt@kernel.org, mark.rutland@arm.com,
	shawnguo@kernel.org, heiko@sntech.de, sam@ravnborg.org,
	icenowy@aosc.io, laurent.pinchart@ideasonboard.com,
	gregkh@linuxfoundation.org, Jonathan.Cameron@huawei.com,
	davem@davemloft.net, mchehab+samsung@kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 2/4] dt-bindings: regulator: add document bindings for mpq7920
Date: Fri, 27 Dec 2019 00:20:11 +0000	[thread overview]
Message-ID: <20191227002011.GG27497@sirena.org.uk> (raw)
In-Reply-To: <ec7d8937-724e-946c-0569-da685223d21d@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2293 bytes --]

On Thu, Dec 26, 2019 at 11:23:36PM +0100, saravanan sekar wrote:
> On 23/12/19 11:50 am, Maxime Ripard wrote:
> > On Sun, Dec 22, 2019 at 09:45:05PM +0100, Saravanan Sekar wrote:

> > > +  mps,fixed-off-time:
> > > +     description:
> > > +        select power off sequence with fixed time output delay mentioned in
> > > +        time-slot reg for all the regulators.
> > > +     type: boolean

> > I'm not sure what this fixed-on-time and fixed-off-time property is

> the time slot register holds the time interval of Vout when enable the
> each regulator.
> fixed-on-time property is to specify each regulator shares same time
> interval mention in time slot register.
> variable on-time defines the factor or multiples of value in time slot
> register.

This really isn't very clear from the bindings document.  You
need an explanation like the above in there.

> > supposed to be doing. Why not just get rid of the time slot property,
> > and set the power on / power off time in fixed-on-time /
> > fixed-off-time property?

> 1. if fixed-on-time is needed with default time slot register settings,
> then time slot property is not needed
> 2. if variable time is needed with modified time slot, then both
> variable time factor & time slot property is needed,
> Hope above explanations answers the necessary of all the above property

Same here, though I'm still a bit unclear about what "needed with
modified time slot" means.

> > > +  regulators:
> > > +    type: object
> > > +    description:
> > > +      list of regulators provided by this controller, must be named
> > > +      after their hardware counterparts BUCK[1-4], one LDORTC, and LDO[2-5]
> > > +      The valid names for regulators are
> > > +      buck1, buck2, buck3, buck4, ldortc, ldo2, ldo3, ldo4, ldo5

> > For the third times now, the names should be validated using
> > propertyNames.

> Not sure did I understand your question correctly.
> all the node name under regulators node are parsed by regulator
> framework & validated against
> name in regulator descriptors.

That validates at kernel runtime but doesn't let bindings
validation at the time the DTS is built verify things, Maxime is
asking you to spell things out in the DT binding document so the
DT can be validated independently of the kernel.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]