Received-SPF: pass (google.com: domain of linux-kernel+bounces-146863-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
From: Peng Fan <peng.fan@nxp.com>
To: Rob Herring <robh@kernel.org>
CC: "Peng Fan (OSS)" <peng.fan@oss.nxp.com>, Jonathan Corbet <corbet@lwn.net>,
	Shawn Guo <shawnguo@kernel.org>, Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>, Fabio Estevam
	<festevam@gmail.com>, Krzysztof Kozlowski <krzk+dt@kernel.org>, Conor Dooley
	<conor+dt@kernel.org>, Sudeep Holla <sudeep.holla@arm.com>, Cristian Marussi
	<cristian.marussi@arm.com>, "linux-doc@vger.kernel.org"
	<linux-doc@vger.kernel.org>, "linux-kernel@vger.kernel.org"
	<linux-kernel@vger.kernel.org>, "imx@lists.linux.dev" <imx@lists.linux.dev>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>, "devicetree@vger.kernel.org"
	<devicetree@vger.kernel.org>
Subject: RE: [PATCH v3 2/6] dt-bindings: firmware: add i.MX95 SCMI Extension
 protocol
Thread-Topic: [PATCH v3 2/6] dt-bindings: firmware: add i.MX95 SCMI Extension
 protocol
Thread-Index: AQHajMWwPzKaKnlwaUWTuAs/m6AnGLFkopsAgAADorCABj4qgIAAAx8Q
Date: Tue, 16 Apr 2024 13:19:21 +0000
Message-ID:
 <DU0PR04MB9417D5A577A5E91AC98B69C588082@DU0PR04MB9417.eurprd04.prod.outlook.com>
References: <20240412-imx95-bbm-misc-v2-v3-0-4380a4070980@nxp.com>
 <20240412-imx95-bbm-misc-v2-v3-2-4380a4070980@nxp.com>
 <20240412133400.GA2282201-robh@kernel.org>
 <PA4PR04MB941680A7FCBCDF926417F6A388042@PA4PR04MB9416.eurprd04.prod.outlook.com>
 <20240416130703.GA1992329-robh@kernel.org>
In-Reply-To: <20240416130703.GA1992329-robh@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?us-ascii?Q?XIyl3yizMrlQt6H1YAk+Zvp/QjSqO7QyFSjFtO26JaDBbIuB6/5TIWIMsUVH?=
 =?us-ascii?Q?as94ZwjGYPZEoRlvyt+5iZ3RAMCgbpWBlS0/zfSIFSm5u4L9TtaDKMVu0pUz?=
 =?us-ascii?Q?iFnqSeOaaJyM+PbGddf7k94Z+EO/QDMp2y79TfnG9rCuaIyhuHNhYsdUXu6V?=
 =?us-ascii?Q?k8ovCKePdfG3sWKBCNzQXiIIUPwOCcWu0JuWtGxDr9cuKOZtpowRuL/WVFSH?=
 =?us-ascii?Q?05KzvL+deU1ZBnQlw1nlAycexg78stLnnVBEcGEOlLZTkCTbKSgdUhWGKTte?=
 =?us-ascii?Q?GLXA/OhTwtgZyOCKzKhqO9TIPAw7M5Q6nrwCAqiOY1DdrXjtmVzvkrlyN+9A?=
 =?us-ascii?Q?79IddHpyqJP6tWghudBvewXYtuL8D7vEEqdZebr24cE/MEdCtUuPbQ8PEqi6?=
 =?us-ascii?Q?yX99hfiQVjA/EiAJF1GqXVriacAGt0vqMZHaRPg+aYQOTL/EKBovWflJ2h6J?=
 =?us-ascii?Q?/+gEg9NuoBBkHC2FHEZh2e+KzGJNk6f1T9RJ8UrHuc3mMO7cReuYGhSq/GDS?=
 =?us-ascii?Q?4g0R4G/bbT4d4UBjZpzeUyh+cawcmWBlJuAWyjAKuGf7ssKYx0sMyZP/8ITs?=
 =?us-ascii?Q?PJxDW/ZJiabmh5Xt4SLEl0i/RYXWwETcsP2VNEloMJ1qYBBSaMR9t12j7M+4?=
 =?us-ascii?Q?xJBIeBEumVD+Xd3yBYcKIP3MarbtYk4qPYdAlDHI61dwbbCVM6dHuV9Gti8M?=
 =?us-ascii?Q?IkQrc5xCXQvPaqgjYpZzhy+jvHnMz60+rbTPzwprvsLp/QSt1NrQPzEFfT2K?=
 =?us-ascii?Q?VCGqScqfzmQJGlPccqSh5qHPdTc4qqpzE1d/KLETLAT1n+hpWqt7NkL/dydU?=
 =?us-ascii?Q?qvUoX0Es4WpJKa1cO4pDdyOtgQD/pQOU1JzamE5fvbKenQh+ED11adxj/5ih?=
 =?us-ascii?Q?m1U3FNUKtNggFejaXQyJY0vw7+SXj/1SN2Tm9gSYda77nuNJuKuqcdCq216P?=
 =?us-ascii?Q?1uKXq9BjHce1dr3HQLmc2AMPTcPHI8s/dr4r5wy8hqPevqVE40/7IrKCy+3e?=
 =?us-ascii?Q?/Gp4wESZOd9ILz/K9aAPL+8HcJrCNwNa22dMKmisjtmO9gHJ2nb89nd8EjoI?=
 =?us-ascii?Q?iVEIHfA2R0LOPrsBCKXHlUAprvXqQ+Y8KmsBW51S2IOPr1Sl45hpq5AyLbyy?=
 =?us-ascii?Q?dv6xPG3UGckTNxw3H+7BdUFahbulve41zMGEzfUB8XQk3oAHqpHrXK7jSncV?=
 =?us-ascii?Q?a8gQelsams4zRLH+P3whnFMj1e5Jj4XB/0kL7Z+x+lxFeZ83dkpNJRPQMFWj?=
 =?us-ascii?Q?umUSTRq9e1piGJh/xRT0KedF4cWTACeYS/PB/l0/xLfLmoKHvf3jj54Uzfc8?=
 =?us-ascii?Q?9Uzke/KnEa1qTSFuq9oqalf6v6FuSz4hrlxUApb3VXagPsVET5+Q9QO3mtWh?=
 =?us-ascii?Q?ntv5hQ5HLs0+d7jYaXMJr1aYwMDaZcmoty5eNd03ENB7ppeudsKmVYNH2qW3?=
 =?us-ascii?Q?b6iPFsJ2N+VWPWcJOdl5PKvQiE/4v8+cXvOz50E2hGwo28a7xEuERV3Y++8R?=
 =?us-ascii?Q?+J/Oq/nx/zaH4A12WujmKui9k+ki4S+ntgvmEC/Z+uTzQTV/0LJlFD59xKVF?=
 =?us-ascii?Q?LFlGhcCs2AlPN1VQI2g=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DU0PR04MB9417.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: cb67333e-655c-406d-3160-08dc5e17d439
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Apr 2024 13:19:21.7453
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: JgHprwJcEoyjBmelzh/PPE+dBTmSmIQ2g1LMFy3mlVtdF+ipPoM3CwIdPBfWeHzKYwP4MzDkUI6Szpz0oqHavA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PA4PR04MB9438

> Subject: Re: [PATCH v3 2/6] dt-bindings: firmware: add i.MX95 SCMI
> Extension protocol
>=20
> On Fri, Apr 12, 2024 at 01:50:37PM +0000, Peng Fan wrote:
> > Hi Rob,
> >
> > > Subject: Re: [PATCH v3 2/6] dt-bindings: firmware: add i.MX95 SCMI
> > > Extension protocol
> > >
> > > On Fri, Apr 12, 2024 at 06:47:08PM +0800, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > Add i.MX SCMI Extension protocols bindings for:
> > > >  - Battery Backed Module(BBM) Protocol
> > > >    This contains persistent storage (GPR), an RTC, and the ON/OFF b=
utton.
> > > >    The protocol can also provide access to similar functions implem=
ented
> via
> > > >    external board components.
> > > >  - MISC Protocol.
> > > >    This includes controls that are misc settings/actions that must =
be
> exposed
> > > >    from the SM to agents. They are device specific and are usually =
define
> to
> > > >    access bit fields in various mix block control modules,
> > > > IOMUX_GPR, and
> > > other
> > > >    GPR/CSR owned by the SM.
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > >  .../devicetree/bindings/firmware/arm,scmi.yaml     | 21 ++++++++++=
+++
> > > >  .../bindings/firmware/nxp,imx95-scmi.yaml          | 36
> > > ++++++++++++++++++++++
> > > >  2 files changed, 57 insertions(+)
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > index 93fb7d05f849..fa2cc910c485 100644
> > > > --- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > +++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
> > > > @@ -247,6 +247,27 @@ properties:
> > > >        reg:
> > > >          const: 0x18
> > > >
> > > > +  protocol@81:
> > > > +    $ref: '#/$defs/protocol-node'
> > > > +    unevaluatedProperties: false
> > > > +
> > > > +    properties:
> > > > +      reg:
> > > > +        const: 0x81
> > > > +
> > > > +  protocol@84:
> > > > +    type: object
> > > > +    anyOf:
> > > > +      - allOf:
> > > > +          - $ref: /schemas/firmware/nxp,imx95-scmi.yaml
> > > > +          - $ref: '#/$defs/protocol-node'
> > >
> > > If you put the ref under the protocol node, then it's 1 schema file
> > > per protocol per vendor. Also, we then have to list every possible
> > > protocol node here, and every one listed here will be valid for every
> vendor.
> > > What we discussed is putting the list of vendor protocol schemas at
> > > the top- level here and then the vendor schemas can list out all the
> protocol nodes.
> > >
> > > Also, move "$ref: '#/$defs/protocol-node'" to nxp,imx95-scmi.yaml.
> >
> > In arm,scmi.yaml top level, add below:
> > +anyOf:
> > +  - $ref: /schemas/firmware/nxp,imx95-scmi.yaml
> >
> > And also add a protocol node:
> >   protocol@84:
> >     $ref: '#/$defs/protocol-node'
> >
> >     properties:
> >       reg:
> >         const: 0x84
> > But here I not add unevaludatedProperties =3D false; otherwise the
> > vendor yaml new properties will not work.
>=20
> Just drop 'protocol@84' entirely here and change the top-level
> additionalProperties to unevaluatedProperties.

Thanks, will fix in v4.
>=20
> >
> > In nxp,imx95-scmi.yaml:
> > properties:
> >   protocol@84:
> >     $ref: '/schemas/firmware/arm,scmi.yaml#/$defs/protocol-node'
> >     unevaluatedProperties: false
> >
> >     properties:
> >       reg:
> >         const: 0x84
> >
> >       nxp,wakeup-sources:
> >         description:
> >           Each entry consists of 2 integers, represents the source and =
electric
> signal edge
> >         items:
> >           items:
> >             - description: the wakeup source
> >             - description: the wakeup electric signal edge
>=20
> Constraints on the items values?

will add in v4.

>=20
> >         minItems: 1
> >         maxItems: 32
> >         $ref: /schemas/types.yaml#/definitions/uint32-matrix
> >
> > additionalProperties: true
> >
> > Are the upper looks good to you?
>=20
> Yes, other than the one comment.

Thanks for help reviewing the patch.

Thanks,
Peng.
>=20
> Rob