Message ID | 20250407095119.588920-1-mitltlatltl@gmail.com |
---|---|
Headers | show |
Series | backlight: ktz8866: improve it and support slave | expand |
On 07/04/2025 11:51, Pengyu Luo wrote: > Kinetic ktz8866, found in many android devices, nowadays, some oem use > dual ktz8866 to support a larger panel and higher brightness, add the Just one space after 'and'. > binding for slave case. > > Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com> > --- > .../leds/backlight/kinetic,ktz8866.yaml | 29 +++++++++++++++---- > 1 file changed, 24 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/leds/backlight/kinetic,ktz8866.yaml b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktz8866.yaml > index c914e1276..825a6fbf1 100644 > --- a/Documentation/devicetree/bindings/leds/backlight/kinetic,ktz8866.yaml > +++ b/Documentation/devicetree/bindings/leds/backlight/kinetic,ktz8866.yaml > @@ -19,7 +19,9 @@ allOf: > > properties: > compatible: > - const: kinetic,ktz8866 > + enum: > + - kinetic,ktz8866 > + - kinetic,ktz8866-slave Does not look right. That's the same device. Same devices have same compatible. > > reg: > maxItems: 1 > @@ -58,9 +60,16 @@ properties: > required: > - compatible > - reg > - - vddpos-supply > - - vddneg-supply > - - enable-gpios > + > +if: > + properties: > + compatible: > + const: kinetic,ktz8866 > +then: > + required: > + - vddpos-supply > + - vddneg-supply > + - enable-gpios I don't understand why other device does not need power. > > unevaluatedProperties: false > > @@ -68,7 +77,7 @@ examples: > - | > #include <dt-bindings/gpio/gpio.h> > > - i2c { > + i2c0 { No, don't change. > #address-cells = <1>; > #size-cells = <0>; > > @@ -84,3 +93,13 @@ examples: > kinetic,enable-lcd-bias; > }; > }; > + > + i2c1 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + backlight@11 { > + compatible = "kinetic,ktz8866-slave"; > + reg = <0x11>; No real differences here, so drop the example. Anyway, new examples would start from - | (see other bindings). Best regards, Krzysztof
On Mon, Apr 07, 2025 at 05:51:18PM +0800, Pengyu Luo wrote: > I polled all registers when the module was loading, found that > current sinks have already been configured. Bootloader would set > when booting. So checking it before setting the all channels. Can you rephrase this so the problem and solution are more clearly expressed. Perhaps template Ingo suggests here would be good: https://www.spinics.net/lists/kernel/msg1633438.html > Signed-off-by: Pengyu Luo <mitltlatltl@gmail.com> > --- > drivers/video/backlight/ktz8866.c | 23 +++++++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) > > diff --git a/drivers/video/backlight/ktz8866.c b/drivers/video/backlight/ktz8866.c > index 017ad80dd..b67ca136d 100644 > --- a/drivers/video/backlight/ktz8866.c > +++ b/drivers/video/backlight/ktz8866.c > @@ -46,6 +46,8 @@ > #define LCD_BIAS_EN 0x9F > #define PWM_HYST 0x5 > > +#define CURRENT_SINKS_MASK GENMASK(5, 0) > + Call this BL_EN_CURRENT_SINKS_MASK and keep it next to the register it applies to. > struct ktz8866_slave { > struct i2c_client *client; > struct regmap *regmap; > @@ -112,11 +120,18 @@ static void ktz8866_init(struct ktz8866 *ktz) > { > unsigned int val = 0; > > - if (!of_property_read_u32(ktz->client->dev.of_node, "current-num-sinks", &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 specified. */ > - ktz8866_write(ktz, BL_EN, BIT(6) - 1); > + } else { > + /* > + * Enable all 6 current sinks if the number of current > + * sinks isn't specified and the bootloader didn't set > + * the value. > + */ > + ktz8866_read(ktz, BL_EN, &val); > + if (!(val && CURRENT_SINKS_MASK)) This is the wrong form of AND. > + ktz8866_write(ktz, BL_EN, CURRENT_SINKS_MASK); > + } Daniel.