Hello Baruch,
On Tue, 19 Dec 2017 08:09:18 +0200
Baruch Siach <baruch@xxxxxxxxxx> wrote:
> Hi Miquèl,
>
> On Tue, Dec 19, 2017 at 01:43:20AM +0100, Miquel RAYNAL wrote:
> > On Mon, 18 Dec 2017 22:33:24 +0200
> > Baruch Siach <baruch@xxxxxxxxxx> wrote:
> > > On Mon, Dec 18, 2017 at 03:36:32PM +0100, Miquel Raynal wrote:
> > > > From: Baruch Siach <baruch@xxxxxxxxxx>
> > > >
> > > > Add compatible strings for AP806 and CP110 that are part of the
> > > > Armada 8k/7k line of SoCs.
> > > >
> > > > Add a note on the differences in the size of the control area in
> > > > different bindings. This is an existing difference between the
> > > > Armada 375 binding and the other boards already supported. The
> > > > new AP806 and CP110 bindings are similar to the existing Armada
> > > > 375 in this regard.
> > > >
> > > > Signed-off-by: Baruch Siach <baruch@xxxxxxxxxx>
> > > > [<miquel.raynal@xxxxxxxxxxxxxxxxxx>: reword, additional details]
> > > > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxxxxxxxxx>
> > > > ---
> > > > .../devicetree/bindings/thermal/armada-thermal.txt | 24
> > > > +++++++++++++++++----- 1 file changed, 19 insertions(+), 5
> > > > deletions(-)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> > > > b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> > > > index 24aacf8948c5..9b7b2c03cc6f 100644 ---
> > > > a/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> > > > +++
> > > > b/Documentation/devicetree/bindings/thermal/armada-thermal.txt
> > > > @@ -7,17 +7,31 @@ Required properties:
> > > > marvell,armada375-thermal marvell,armada380-thermal
> > > > marvell,armadaxp-thermal
> > > > + marvell,armada-ap806-thermal
> > > > + marvell,armada-cp110-thermal
> > > >
> > > > - reg: Device's register space.
> > > > Two entries are expected, see the examples
> > > > below.
> > > > - The first one is required for the sensor
> > > > register;
> > > > - the second one is required for the control
> > > > register
> > > > - to be used for sensor initialization (a.k.a.
> > > > calibration).
> > > > + The first one points to the status register
> > > > (4B).
> > > > + The second one points to the control registers
> > > > (8B).
> > > > + Note: with legacy bindings, the second entry
> > > > pointed
> > > > + only to the so called "control MSB" ("control
> > > > 1"), was
> > > > + 4B wide and did not let the possibility to
> > > > reach the
> > > > + "control LSB" ("control 0") register. This is
> > > > only
> > > > + allowed for compatibility reasons in Armada
> > > > + 370/375/38x/XP DT nodes.
> > >
> > > "allowed" is not the right term, IMO. Legacy compatibles MUST
> > > point to the MSB control register to preserve compatibility with
> > > existing DTs.
> > >
> > > The original patch had a list of legacy and non-legacy
> > > compatibles. I think we need to keep them.
> >
> > Maybe I should reword this paragraph because we both agree on the
> > meaning:
> >
> > "
> > Note: Legacy bindings are only supported with Armada 370/375/38x/XP
> > compatibles. The second memory resource entry only points to
> > "control MSB/control 1", is 4 bytes wide and is preventing any
> > access to "control LSB/control 0".
> > "
> >
> > Does this sounds better to you?
>
> I think we need to explicitly list the affected compatible strings.
> Something like:
I thought listing the SoCs directly would have been acceptable
but I am really not against listing them explicitly :)
>
> For backwards compatibility reasons, the compatibles
> marvell,armada370-thermal, marvell,armada380-thermal, and
> marvell,armadaxp-thermal must point to "control MSB/control 1",
> with size of 4. All other compatibles must point to "control
> LSB/control 0" with size of 8.
>
> But I think you are right that we can use the size of the control
> registers to tell whether e.g. marvell,armada380-thermal is of the
> old binding of the new one. So maybe the "allow" language is more
> correct. But let's make it explicit to avoid any doubt. How about:
>
> The compatibles marvell,armada370-thermal,
> marvell,armada380-thermal, and marvell,armadaxp-thermal must point to
> "control MSB/control 1", with size of 4 (deprecated binding), or
> point to "control LSB/control 0" with size of 8 (current binding).
> All other compatibles must point to "control LSB/control 0" with size
> of 8.
This version looks good to me!
Thank you,
Miquèl
>
> baruch
>
--
Miquel Raynal, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
