Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.38 as permitted sender) client-ip=23.128.96.38;
From:   Flavio Suligoi <f.suligoi@asem.it>
To:     Rob Herring <robh@kernel.org>
CC:     Lee Jones <lee@kernel.org>,
        Daniel Thompson <daniel.thompson@linaro.org>,
        Jingoo Han <jingoohan1@gmail.com>,
        Helge Deller <deller@gmx.de>, Pavel Machek <pavel@ucw.cz>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "linux-leds@vger.kernel.org" <linux-leds@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-fbdev@vger.kernel.org" <linux-fbdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v4 1/2] dt-bindings: backlight: Add MPS MP3309C
Thread-Topic: [PATCH v4 1/2] dt-bindings: backlight: Add MPS MP3309C
Thread-Index: AQHZ+3Oa23Umjg05nUq8U4rLHzRJ47BDMv0AgAFHCjA=
Date:   Wed, 11 Oct 2023 11:52:14 +0000
Message-ID: <AM8PR01MB8045ADD01F342CCCAB6FA4A1F9CCA@AM8PR01MB8045.eurprd01.prod.exchangelabs.com>
References: <20231010121621.3009154-1-f.suligoi@asem.it>
 <20231010161512.GA944015-robh@kernel.org>
In-Reply-To: <20231010161512.GA944015-robh@kernel.org>
Accept-Language: it-IT, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?rXU0wCSznBjMSrQi0XILzyWvk0jg4wTAy2kSvAti6Dc26mLFYJI/aCfBQPx/?=
 =?us-ascii?Q?b2DXXQtABAXUIa/GMzRyNj9W8Q/zYgy1nrkNFOSP278chOKBxF/tHEgS0LwC?=
 =?us-ascii?Q?Y5bc5Cj+/D0L5WT4YQF2+4RgwSWZRyTq1JcoGoIAVGeAY7H0Ymj2tz3zTkLh?=
 =?us-ascii?Q?NOb0lBDhLRw9/Wmn6Dz5GncG9jBR6ADvr6uOhDkLRssU2CYaoJmqlHEMdVne?=
 =?us-ascii?Q?o0R0SvNp6pE/BLYdWhvIOfPto0B6gZ5zMeGS31EiJrFTHQ+TJUwzag+9Ogwd?=
 =?us-ascii?Q?NQb6jmlzY99TPYkp0fAGUtyO96ius3KtFnOUK2E8NRUqF/gH7P3Zub7sWVYj?=
 =?us-ascii?Q?1dAs84dII9sLG3+CDhY8qKKgYrB/qq6ZSEKZ5NOamJxjlb+nzyBLNfOu1E00?=
 =?us-ascii?Q?3psEMDBXho0ASrZ72bAoSico+2HUPZsNYAcKtksaghOZTubltK81OqmSCENS?=
 =?us-ascii?Q?Y+8VI3GJUo5w6Agjzx8PJ9oVCtwUUaZHBMt0cGoswGGCqbqOT8QpbWW11DYP?=
 =?us-ascii?Q?V9sVdl1tPuUB2apRQnYoNuLHqKovk5bIc5ZspVgjRyVnWTWXc8+Ocp1TRAJ7?=
 =?us-ascii?Q?3xqRMYwluMpTMMfZvSxssKr+Cg0qQLGwKvuszCXzk9mRB75LEKq0oYBGXLft?=
 =?us-ascii?Q?x/9guyf+LPGjNqNqosDyKK/WrQmsR0ou4Tw0aTAKLu2s9lqmWecs96G2Wj35?=
 =?us-ascii?Q?GTnHb2hOUfVeLk+CIGK46k2aPtybMPKDeOh6MPRqSYrBHK0j6TURu0WufKAv?=
 =?us-ascii?Q?zUdSdjzegmEe3wlJvRZENWOD85rICr14t6U5AFzCFhvEqY+RqwOwbIfvolU2?=
 =?us-ascii?Q?OT4f41sIR/Lljf0x+a9UKAUberaeQVEfYNDkVwdg/syr8RXiV+eh4l9dDrF8?=
 =?us-ascii?Q?rvk8jNMMau4j18fkDWm4tBfX2gPp3tCEHA+EevixuFXeGsHcTZ4vCgzj4v0c?=
 =?us-ascii?Q?Hi4Kqiq8MGWm3/KT84mmOoycgu8nykE1YNNAqofQyGGZfT+yCx8mvObbFF2/?=
 =?us-ascii?Q?84lAj37vDfof8oLJk8ToL5NfiQH9UxNHv6rYmj7jZJ9nxnPjVDXptDKwDhnj?=
 =?us-ascii?Q?M3iQFwCSnervD+JgvPC6JhcWQ2JaR9yiVGvlWY/gbyR1DxTrqwXIg+UBJdZL?=
 =?us-ascii?Q?7Gk5f9VyM5kM/9n/xi9ML9uev9THStCboDEAYtbFZyGbMNZD8XyTkRkf5SA4?=
 =?us-ascii?Q?1ARlylRW9u6LIlYsaZgBBKdV1GPLUDeJanFjOiMBcKJOIsOod2bb747V9/Pk?=
 =?us-ascii?Q?07z0Ew3yH/Qz3F8m0EMwO8Qb7DAgXoXQh8m9j6BmbjSIx9/0iD8kf8LoBwLr?=
 =?us-ascii?Q?jCQ9SwO23Vs+3XlYwavyaenIipTeoNb/oxyiBhcjygrGo7COvQqLd2JzAKNx?=
 =?us-ascii?Q?nK8olfpbijLAOCXEnzURMdZKvNdNEVAy0cwGuI2PaVrJxr8IFnvgOzyalG+S?=
 =?us-ascii?Q?kk/a6Qz0t1cRXqlGlpg6pYsBV7PNFFz2gdGlNGrc5f/Y+Uk3qIQThHMYG4j6?=
 =?us-ascii?Q?5MU6ZUFSdAp4MmW6XXnb9vaYJ0Fi3uaVcgaEYVlwXqnBGzs5WMGRbueLyzDw?=
 =?us-ascii?Q?UFQlofJTnl3AHX/KhIQ=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: asem.it
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: AM8PR01MB8045.eurprd01.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 07a56bdb-e1b3-4946-316f-08dbca5082ce
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Oct 2023 11:52:14.3477
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: d0a766c6-7992-4344-a4a2-a467a7bb1ed2
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: DVRVL4xQGGuNR9effRnvXyySCYU71xz4nCs7sZk/+5HcKGw1B67pEsB6Er7JysbG55/62M/q5TymCX2b4iti6w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS1PR01MB9180
X-Spam-Status: No, score=2.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	RCVD_IN_SBL_CSS,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on fry.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (fry.vger.email [0.0.0.0]); Wed, 11 Oct 2023 04:53:00 -0700 (PDT)
X-Spam-Level: **

Hi Rob,

...=20
> >  .../bindings/leds/backlight/mps,mp3309c.yaml  | 82
> > +++++++++++++++++++
> >  1 file changed, 82 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> > b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yaml
> > new file mode 100644
> > index 000000000000..e2f9ae2b3fb4
> > --- /dev/null
> > +++
> b/Documentation/devicetree/bindings/leds/backlight/mps,mp3309c.yam
> > +++ l
> > @@ -0,0 +1,82 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/leds/backlight/mps,mp3309c.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MPS MP3309C backlight
> > +
> > +maintainers:
> > +  - Flavio Suligoi <f.suligoi@asem.it>
> > +
> > +description: |
> > +  The Monolithic Power (MPS) MP3309C is a WLED step-up converter,
> > +featuring a
> > +  programmable switching frequency to optimize efficiency.
> > +  It supports two different dimming modes:
> > +
> > +  - analog mode, via I2C commands, as default mode (32 dimming
> > + levels)
> > +  - PWM controlled mode (optional)
> > +
> > +  The datasheet is available at:
> > +  https://www.monolithicpower.com/en/mp3309c.html
> > +
> > +properties:
> > +  compatible:
> > +    const: mps,mp3309c
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  pwms:
> > +    description: if present, the backlight is controlled in PWM mode.
> > +    maxItems: 1
> > +
> > +  enable-gpios:
> > +    description: GPIO used to enable the backlight in "analog-i2c" dim=
ming
> mode.
> > +    maxItems: 1
> > +
> > +  brightness-levels:
> > +    description:
> > +      Array of distinct brightness levels, in PWM dimming mode.
> > +      Typically these are in the range from 0 to 255, but any range st=
arting
> > +      at 0 will do.
> > +      The 0 value means a 0% duty cycle (darkest/off), while the last =
value in
> > +      the array represents a 100% duty cycle (brightest).
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
>=20
> This already has a type defined. Please add it to backlight/common.yaml a=
nd
> remove from led-backlight.yaml and pwm-backlight.yaml.

Ok, I'll add common.yaml and I'll prepare two other separate patches for th=
ese.

>=20
> You say 0-255 here, but your example is 0-10. One of those seems wrong.
> Anyways, don't define constraints in prose, use a schema:
>=20
> items:
>   maximum: 10 (or 255?)

Ok, I'll add maximum, and I'll change the example, using 0..255

>=20
> > +
> > +  default-brightness:
> > +    description:
> > +      The default brightness (index into the levels array).
> > +    $ref: /schemas/types.yaml#/definitions/uint32
>=20
> Already has a type. You need to reference backlight/common.yaml.

Ok

>=20
> > +
> > +  mps,overvoltage-protection-microvolt:
> > +    description: Overvoltage protection (13.5V, 24V or 35.5V).
> > +    enum: [ 13500000, 24000000, 35500000 ]
> > +    default: 35500000
> > +
> > +  mps,no-sync-mode:
> > +    description: disable synchronous rectification mode
> > +    type: boolean
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +        #address-cells =3D <1>;
> > +        #size-cells =3D <0>;
> > +
> > +        /* Backlight with PWM control */
> > +        backlight_pwm: backlight@17 {
> > +            compatible =3D "mps,mp3309c";
> > +            reg =3D <0x17>;
> > +            pwms =3D <&pwm1 0 3333333 0>; /* 300 Hz --> (1/f) * 1*10^9=
 */
> > +            brightness-levels =3D <0 1 2 3 4 5 6 7 8 9 10>;
> > +            default-brightness =3D <8>;
> > +            mps,overvoltage-protection-microvolt =3D <24000000>;
> > +        };
> > +    };
> > --
> > 2.34.1
> >

Thanks,
Flavio