Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
From:   Flavio Suligoi <f.suligoi@asem.it>
To:     Daniel Thompson <daniel.thompson@linaro.org>
CC:     Lee Jones <lee@kernel.org>, Jingoo Han <jingoohan1@gmail.com>,
        Helge Deller <deller@gmx.de>, Pavel Machek <pavel@ucw.cz>,
        Rob Herring <robh+dt@kernel.org>,
        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>,
        Rob Herring <robh@kernel.org>
Subject: RE: [PATCH v3 1/2] dt-bindings: backlight: Add MPS MP3309C
Thread-Topic: [PATCH v3 1/2] dt-bindings: backlight: Add MPS MP3309C
Thread-Index: AQHZ76uO1Fb/EPNT/k67j1TrFzld37AtE7aAgAmVcBCAAT5MAIAAXgAggAGFVUA=
Date:   Wed, 4 Oct 2023 15:18:24 +0000
Message-ID: <DU2PR01MB8034F17A9C81FE198CCAC5B7F9CBA@DU2PR01MB8034.eurprd01.prod.exchangelabs.com>
References: <20230925122609.78849-1-f.suligoi@asem.it>
 <20230926130113.GB4356@aspen.lan>
 <DU2PR01MB8034F448F6E66BAC3823CDB1F9C4A@DU2PR01MB8034.eurprd01.prod.exchangelabs.com>
 <20231003102138.GB69443@aspen.lan>
 <DU2PR01MB80349344FB8CB1415BFA917DF9C4A@DU2PR01MB8034.eurprd01.prod.exchangelabs.com>
In-Reply-To: <DU2PR01MB80349344FB8CB1415BFA917DF9C4A@DU2PR01MB8034.eurprd01.prod.exchangelabs.com>
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?6UEWUcpwBy8EkfaPEZDemwfL1c/HBfplVlUTc3qG0OvjDRMuUAspoo/R9iDE?=
 =?us-ascii?Q?UTt+z9yRJXdm/ybTwyWZDmfJdHsQg/Bn94LaYQdsVFdKwKs1yw1vPnyO0sqW?=
 =?us-ascii?Q?JBk4l2SMEQSTNwQLxT6rKO8SJnpHp2ZbcHxRwz9USkjF5r47YBJhAxD8mgCd?=
 =?us-ascii?Q?ShSGxXq+hfNv35V3/R0OplzBcK6iHdHNyHWEb5OBEjmoFxn5kLubPmGhJDHN?=
 =?us-ascii?Q?kh4wMBFr33TefZf617GyztdPUjUgpqCnVxt3jwPCfHJ92keF2hjJmfZH6sRf?=
 =?us-ascii?Q?EUcOssyNeA7skollrXNR1nThE7t4sj1vHryHr0Cmy6ll2euhZWD+DHaLfYZF?=
 =?us-ascii?Q?IgFsRWE4VCS5kC3ve5vkz1/TeO30/QvzqrHOhhPWPS6YHtbZNN72yXhMGVMS?=
 =?us-ascii?Q?1ZH1QX8TwX6xyLRjrnXRoNCGWLdNh5tw8+j9+uJcWvoatRoPyp1niVFVf8fT?=
 =?us-ascii?Q?rkuZN34YuhBujYpHwXskJD2Y3fsQZ8Bj8IW4g52i8tcfiR9C8ahIIJx4Wgfw?=
 =?us-ascii?Q?ZM4cQIfPO3T/BfP2v0GoGcauW6I2rEZMCx984XXfwQL9Ubg4AW00UOMsrtIO?=
 =?us-ascii?Q?EtNuRTqfpGW0mpv4FcrWgSOLe9zl3NTXfg7+3ZPxAnGL5vQbBsskE98pxMbK?=
 =?us-ascii?Q?7+QSC3TArcog4O5I8SUOY/BPFy937gZfT4nmmle5WSc1V6Df+L1BEdVDlOgV?=
 =?us-ascii?Q?WMmcOXGiJP9p3B7TmoVuX6R08Nep2LB7DbltpBMwLDebz0BOaMGAt6pn7ID8?=
 =?us-ascii?Q?/i4d9AytWoFzu6RjKQUO5IVzLnhg++VPd52CTMEViJDfRpi3J/6eR4wGTSNS?=
 =?us-ascii?Q?J9oG7+BjBMM+7tug5njju8atMy5y3idwbim9sePvB7Ej1uzZNdmAEz9Jqcq1?=
 =?us-ascii?Q?t4X8wDSvVmusjzVEsdLcW7NWd5EzHa9CsX1L6fOiFNf81k4BEd3EkZs1dkBZ?=
 =?us-ascii?Q?vt+1wzA07pb5d3i+5l+zaw9U602VMLYu7jbr47dCZfCc+Y/DO33xi9Ezvdd6?=
 =?us-ascii?Q?CnaLmTUWyLbEccEklh1U1bf352b/2+QW7QwV36nZ61PT2NTecQJQIHIXFq9K?=
 =?us-ascii?Q?4heqB64ZxikNz+lkBVWTXuefPFj8yTjSUMO/5lrIpgqD8uVQZLaBezr0H8vW?=
 =?us-ascii?Q?KeBvCnYSP0zoISjeruQSMEjJoL0t0x6Lw8QC6nnfwHYWLZ09yKaeluMIIkoo?=
 =?us-ascii?Q?lmSknwfS7sdMeNkp8zxgLKlWW5+rQGELEIfX5Hfs/U/YxfIqhPbFxx2nqZou?=
 =?us-ascii?Q?PCQ76C0ev2v/dFiBZmMD5XqaXTFQ9l+Ss7Na/kJ6A9y0rNbN47us3/8fwubm?=
 =?us-ascii?Q?SBoFgvhY/5MSl39m+up1Y8hTa/zGgwxVJOCPqMsNOaKcAyzPqyAbdx9C0ROs?=
 =?us-ascii?Q?ioeR1oRU5Hlsti2nuLxVj2e9o7wwAS5jIR8A9PXIVsQTyX0A5OzJney/X3Fy?=
 =?us-ascii?Q?vW3cKbT6GPqI7CGwpVpxxdK1WmsVZ0hKQGqznmczLsbQ8QyrBjbu82MU2g0e?=
 =?us-ascii?Q?oV5FC113sUWzE1x1aMooT2Mryi2RVGqjIFAWN39Yp48qLYuOafsF5Zgf/TtV?=
 =?us-ascii?Q?yl4/N5RGoRk8YgY8zrs=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: DU2PR01MB8034.eurprd01.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f1d76810-552f-472c-4d02-08dbc4ed26f1
X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Oct 2023 15:18:24.2081
 (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: b8Is2HQx+mpw1c858+TDKKPVpMQJ3pBKtFUZ+QCeOR9KL+QwJTygVBfLDXwY/hvbuEn5HbceRA3tk7ZCGX+9EQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DU2PR01MB8624
X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_BLOCKED,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,URIBL_BLOCKED autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
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 (howler.vger.email [0.0.0.0]); Wed, 04 Oct 2023 08:18:37 -0700 (PDT)

Hi Daniel,
...
> ...
> > > > > +required:
> > > > > +  - compatible
> > > > > +  - reg
> > > > > +  - max-brightness
> > > >
> > > > Why is this mandatory?
> > > >
> > > > There's no point in setting max-brightness when running in I2C
> > > > mode
> > > > (max- brightness should default to 31 in that case).
> > > >
> > > >
> > > > > +  - default-brightness
> > > >
> > > > Again. I'm not clear why this needs to be mandatory.
> > > >
> > > >
> > >
> > > Ok, you are right, I'll remove max-brightness and default-brightness
> > > from required properties list. I think to change these properties,
> > > for the pwm dimming, into a clearer:
> > >
> > > - brightness-levels (uint32)
> > > - default-brightness-levels (uint32).
> > >
> > > For example:
> > >
> > >   brightness-levels:
> > >     description:
> > >       Number of brightness levels. The actual brightness
> > >       level (PWM duty cycle) will be interpolated from 0 to this valu=
e.
> > >       0 means a  0% duty cycle (darkest/off), while the
> > > brightness-levels
> > represents
> > >       a 100% duty cycle (brightest).
> > >     $ref: /schemas/types.yaml#/definitions/uint32
> > >
> > >   default-brightness-level:
> > >     description:
> > >       The default brightness level (from 0 to brightness-levels)
> > >     $ref: /schemas/types.yaml#/definitions/uint32
> > >
> > > Example:
> > > brightness-levels =3D <10>;
> > > default-brightness-level =3D <6>;
> > >
> > > What do you think about this solution?
> >
> > If you want to introduce a brightness-levels property then I would
> > expect it to be defined with the same meaning as pwm-backlight (it's
> > not relevant to the bindings but ideally it would be implemented by
> > refactoring and reusing the code from pwm_bl.c).
>=20
> ok, I'll use the brightness-levels property as used in pwm-backlight
>=20
> >
> > Same with default-brightness-level although I'm not sure why one
> > wouldn't just use default-brightness for new bindings (doesn't
> > default-brightness-level simply do exactly the same thing as default-
> brightness).
>=20
> ok for default-brightness instead of default-brightness-level

Just a question: default-brightness-level is the index into the brightness-=
levels array.
But, if I use default-brightness instead of default-brightness-level, =20
should I consider default-brightness also as an index into brightness-level=
s array?
Or, in this case, have the default-brightness to be equal to one of the val=
ues inside the
brightness-levels array?

>=20
> >
> >
> > Daniel.
>=20
> Thanks an best regards,
> Flavio

Thanks,

Flavio