Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
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+3Oa23Umjg05nUq8U4rLHzRJ47BDMv0AgAF+mwA=
Date:   Wed, 11 Oct 2023 15:13:20 +0000
Message-ID: <AM8PR01MB8045F549C8A65A853E214D37F9CCA@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?iRiHXW67oFyMDo82nFwJfcKxp1Ua4gJCB4QM4iYMqQxb6+pZPZFUWmcvOwFB?=
 =?us-ascii?Q?FnBM8o77Ay9OjRSI1wftsS7EF0KsdU3AJTqdBLk8QTCX5+2eSGtM3ksHC9r2?=
 =?us-ascii?Q?SjCnkApjE6J0r4LvuaJ1CGjStWSHHOSA59HSjlUDgdRU7hjhFXm9fLM3q9JW?=
 =?us-ascii?Q?Xa5gU7PueeM/vES5Wx7XGKbX126azVOYZpfo7PW4BHCjj2UqRsWNuIMwa/0y?=
 =?us-ascii?Q?0I41k5kSiDYfG2XmHBIAKFo6xkerYQ2g3oXdNlZdaFp3YxfZwC3wUfJ/y+TF?=
 =?us-ascii?Q?3kyXRh+Ee9wzIUid9MvbMRVnWB2OL7PKmBi22xCH4nL6V1rTeTU0YJgxTk3c?=
 =?us-ascii?Q?01wP/FkbpbJlXAB8j1lp/85v305elPlerA9cRx8GKhXOZaSEMZ5Vuy5WEVUk?=
 =?us-ascii?Q?1yBvfrPqP9MrPhz2v30I6oHmvrDTmkkugESsHPRQRtjOwPysFjCi6HR2ESB5?=
 =?us-ascii?Q?6wrUy2VKDmxFbunJHgVAxl550HCs+9USF0/ZyJhxWPBNpDyQm/s6fy4mDYbM?=
 =?us-ascii?Q?mYh2VdjU/89AuLZ2CcVA7FDMMrdnonheclTdwjNkLvuUoAv99gU5g/3WDiwL?=
 =?us-ascii?Q?mk8T4MayvAw5o+Sj4nniQH5G/h6JSOpVrzf5DnN8asXnrkowRE0cZIaZom2Y?=
 =?us-ascii?Q?G5xYJKQWKLN3qY1fV+/CZRVWVSuu5A28RcL+e9mfsopintxEQqbgUJIzAtbA?=
 =?us-ascii?Q?LaHT/VoZcA3bjbOIr024mvW8/LKuMQM4XXDqYpABYev81uiUS86U+GxxTIbn?=
 =?us-ascii?Q?MIGpEFY+b009Wr4esnRf4HWhnmn630VOj0hy5Is54Q8RTIoI/U5wXBmYQRIx?=
 =?us-ascii?Q?+ivQKIitbSsY8mynLtdfG1gBQ/wjy8kWqZnq5sygOhYzn6hBJLsHptnCoV3v?=
 =?us-ascii?Q?E24lVMkD6NXIDiDE8tTdCdmLdQCvYDahAnwoTuCLdd026wql7FZbJQW+Mc3/?=
 =?us-ascii?Q?EG6TAJU0+piyT010IQaVN6FEPojWFSBG9J5l5iAYQiT9/Ufe4uP3C0j0DBmj?=
 =?us-ascii?Q?TgVNq2vZOI7S5Ad6mVbnqSlYo7deIF1xfWBnk5Z0xdtVQEFeAPjX7jhYOFsY?=
 =?us-ascii?Q?5pH82WKkd7cStl6xHaoaI8tcIkvCH0j7y089ZvNYwUKDdA/B5gB1zhS0yHPj?=
 =?us-ascii?Q?BV8ay0cMNny//5qiKJUVtILKKOkBaMC1J+Fm/UXlc5PcqURc10C1Sw4SDhqw?=
 =?us-ascii?Q?b/L77iarxCu/WTu0Wwf9pZ0DX7T6tOcP8aar4/vVCRAgt2VF1zt3i1tdqhXG?=
 =?us-ascii?Q?DKjTrmEy62RLFM/wEw2NvypvrKWRbT2B/cCa8gSYwZ1tisohdCF1MUun5Vx3?=
 =?us-ascii?Q?meNC4h3+d+HgSQwJ/DtuhniU+V8iPKCWBh11OpacITBwMddYj3P5NBS8zikL?=
 =?us-ascii?Q?TkguwhPF2meGs/L18IVxKxLZGoPai+LAc2I25Km+L9hiwzVUF3Rs7Kl6SCel?=
 =?us-ascii?Q?7WWYr+e0X4Oj/dWRJQiWWH1pj9ONbiIXDfZEQ6v4AWodXd6aBqWx5xtI0OdD?=
 =?us-ascii?Q?xZnMBIJY3q1nb2PNKzpkhPjDgHkToDRjKwVo+sYsDoXYL80eCx5e1cUrm33q?=
 =?us-ascii?Q?dJm4Pcpw7Ywx6tzstxo=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: ec544215-7d60-402c-fc8a-08dbca6c9acb
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Oct 2023 15:13:20.5279
 (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: dZMpIb9arJe8PT9YSRVUuv4+CGg2KpEXs3f/4DdfSV7+O0KB38ejxdD3aTfzG0uVx9eajuqkECuyK/cmYz0izQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR01MB7303
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 pete.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 (pete.vger.email [0.0.0.0]); Wed, 11 Oct 2023 08:13:35 -0700 (PDT)
X-Spam-Level: **

Hi Rob,

just a question about led-backlight.yaml and pwm-backlight.yaml
common properties.

...

> > +
> > +  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.

As well as the brightness-levels property, also the default-brightness-leve=
l is in common
between led-backlight.yaml and pwm-backlight.yaml.
What do you think about removing it from both led-backlight.yaml and pwm-ba=
cklight.yaml, and
moving it into backlight/common.yaml?

>=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?)
>=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.
>=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
> >

Regards,
Flavio