On Wed, Jan 18, 2023 at 12:08:01PM +0000, Daniel Thompson wrote: > On Tue, Jan 17, 2023 at 11:44:08PM +0800, Jianhua Lu wrote: > > Add support for Kinetic KTZ8866 backlight, which is used in > > Xiaomi tablet, Mi Pad 5 series. This driver lightly based on > > downstream implementation [1]. > > [1] https://github.com/MiCode/Xiaomi_Kernel_OpenSource/blob/elish-r-oss/drivers/video/backlight/ktz8866.c > > > > Signed-off-by: Jianhua Lu <lujianhua000@xxxxxxxxx> > > --- > > Changes in v2: > > - Add missing staitc modifier to ktz8866_write function. > > > > Changes in v3: > > - Add 2022 to Copyright line. > > - Sort headers. > > - Remove meaningless comment. > > - Use definitions instead of hardcoding. > > - Add missing maintainer info. > > > > Changes in v4: > > - Change 2022 to 2023. > > - Remove useless macro and enum. > > - Describe settings by devicetree. > > - Move header file to C file. > > Changes in v5: > > - Change "2023" to "2022, 2023" in Copyright line. > > - Set scale property for backlight. > > > > MAINTAINERS | 6 + > > drivers/video/backlight/Kconfig | 8 ++ > > drivers/video/backlight/Makefile | 1 + > > drivers/video/backlight/ktz8866.c | 201 ++++++++++++++++++++++++++++++ > > 4 files changed, 216 insertions(+) > > create mode 100644 drivers/video/backlight/ktz8866.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 42fc47c6edfd..2084e74e1b58 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -11674,6 +11674,12 @@ M: John Hawley <warthog9@xxxxxxxxxxxxxx> > > S: Maintained > > F: tools/testing/ktest > > > > +KTZ8866 BACKLIGHT DRIVER > > +M: Jianhua Lu <lujianhua000@xxxxxxxxx> > > +S: Maintained > > +F: Documentation/devicetree/bindings/leds/backlight/kinetic,ktz8866.yaml > > +F: drivers/video/backlight/ktz8866.c > > + > > L3MDEV > > M: David Ahern <dsahern@xxxxxxxxxx> > > L: netdev@xxxxxxxxxxxxxxx > > diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig > > index 936ba1e4d35e..2845fd7e33ad 100644 > > --- a/drivers/video/backlight/Kconfig > > +++ b/drivers/video/backlight/Kconfig > > @@ -190,6 +190,14 @@ config BACKLIGHT_KTD253 > > which is a 1-wire GPIO-controlled backlight found in some mobile > > phones. > > > > +config BACKLIGHT_KTZ8866 > > + tristate "Backlight Driver for Kinetic KTZ8866" > > + depends on I2C > > + select REGMAP_I2C > > + help > > + Say Y to enabled the backlight driver for the Kinetic KTZ8866 > > + found in Xiaomi Mi Pad 5 series. > > s/enabled/enable/ (and sorry for spotting this one so late) Get it. > > > > +++ b/drivers/video/backlight/ktz8866.c > > @@ -0,0 +1,201 @@ > > [...] > > +static void ktz8866_init(struct ktz8866 *ktz) > > +{ > > + unsigned int val; > > + > > + if(of_property_read_u32(ktz->client->dev.of_node, "current-num-sinks", &val)) > > + ktz8866_write(ktz, BL_EN, BIT(val) - 1); > > + else > > + /* Enable all 6 current sinks if the number of current sinks isn't specifed. */ > > + ktz8866_write(ktz, BL_EN, BIT(6) - 1); > > + > > + if(of_property_read_u32(ktz->client->dev.of_node, "current-ramping-time-us", &val)) { > > + if(val <= 128) { > > + ktz8866_write(ktz, BL_CFG2, BIT(7) | (ilog2(val) << 3) | PWM_HYST); > > + } else { > > + ktz8866_write(ktz, BL_CFG2, BIT(7) | ((5 + val / 64) << 3) | PWM_HYST); > > + } > > This code is interpreting current-ramping-time-us as milliseconds rather > than microseconds! > > I know I used microseconds in the example I proposed in the feedback for > v4 DT bindings but "something like" means I am merely providing an > example (mostly I was intending to show that the units should be > included both in the property name *and* description). > > It is up to you whether you fix the mismatch by changing the DT bindings > document to current-ramping-time-ms or change this code to accept > values in microseconds. This is my mistake, sorry. > > > > + } > > + > > + if(of_property_read_u32(ktz->client->dev.of_node, "led-ramping-time-us", &val)) { > > + unsigned int ramp_off_time = ilog2(val) + 1; > > + unsigned int ramp_on_time = ramp_off_time << 4; > > + ktz8866_write(ktz, BL_DIMMING, ramp_on_time | ramp_off_time); > > + } > > Similarly, this code has not adopted the units specified in the > bindings documentation. > > In this case 0 would map to 512us so if you decided to use milliseconds > you will need to add comment in the description saying that 0 will map > to 512us because the hardware cannot ramp faster than this! Get it, thank you very much. > > > Daniel.
