| Message ID | 20230122-msm8974-bacon-features-v1-5-4049f565c24c@z3ntu.xyz (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add support for some features on OnePlus One | expand |
On 22.01.2023 17:48, Luca Weiss wrote: > Add the node describing the sn3193 that's used to provide notification > LED. > > Unfortunately the driver currently supports neither multicolor API nor > using the properties function & color, so we use label instead. > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > .../arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts | 28 ++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > index ffb486ceb6a6..a672c45d7070 100644 > --- a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > +++ b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > @@ -114,6 +114,34 @@ led@0 { > default-brightness = <80>; > }; > }; > + > + led-controller@68 { > + compatible = "si-en,sn3193"; > + reg = <0x68>; > + > + shutdown-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + led@1 { > + reg = <1>; > + label = "red:status"; > + led-max-microamp = <17500>; > + }; > + > + led@2 { > + reg = <2>; > + label = "green:status"; > + led-max-microamp = <17500>; > + }; > + > + led@3 { > + reg = <3>; > + label = "blue:status"; > + led-max-microamp = <17500>; > + }; > + }; > }; > > &blsp1_i2c6 { >
On 22/01/2023 17:48, Luca Weiss wrote: > Add the node describing the sn3193 that's used to provide notification > LED. > > Unfortunately the driver currently supports neither multicolor API nor > using the properties function & color, so we use label instead. > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > --- > .../arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts | 28 ++++++++++++++++++++++ > 1 file changed, 28 insertions(+) > > diff --git a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > index ffb486ceb6a6..a672c45d7070 100644 > --- a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > +++ b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > @@ -114,6 +114,34 @@ led@0 { > default-brightness = <80>; > }; > }; > + > + led-controller@68 { > + compatible = "si-en,sn3193"; > + reg = <0x68>; > + > + shutdown-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + led@1 { > + reg = <1>; > + label = "red:status"; These should be colors and functions. Best regards, Krzysztof
On Montag, 23. Jänner 2023 18:18:16 CET Krzysztof Kozlowski wrote: > On 22/01/2023 17:48, Luca Weiss wrote: > > Add the node describing the sn3193 that's used to provide notification > > LED. > > > > Unfortunately the driver currently supports neither multicolor API nor > > using the properties function & color, so we use label instead. > > > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > > --- > > > > .../arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts | 28 > > ++++++++++++++++++++++ 1 file changed, 28 insertions(+) > > > > diff --git a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > > b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts index > > ffb486ceb6a6..a672c45d7070 100644 > > --- a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > > +++ b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > > @@ -114,6 +114,34 @@ led@0 { > > > > default-brightness = <80>; > > > > }; > > > > }; > > > > + > > + led-controller@68 { > > + compatible = "si-en,sn3193"; > > + reg = <0x68>; > > + > > + shutdown-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + led@1 { > > + reg = <1>; > > + label = "red:status"; > > These should be colors and functions. Hi Krzysztof, please check the commit message for this. I tried using it but it didn't work, the driver in general might need an overhaul at some point... > Unfortunately the driver currently supports neither multicolor API nor > using the properties function & color, so we use label instead. Or maybe I messed up but pretty sure this was the case when I wrote the dts. Regards Luca > > > Best regards, > Krzysztof
On 25/01/2023 21:51, Luca Weiss wrote: > On Montag, 23. Jänner 2023 18:18:16 CET Krzysztof Kozlowski wrote: >> On 22/01/2023 17:48, Luca Weiss wrote: >>> Add the node describing the sn3193 that's used to provide notification >>> LED. >>> >>> Unfortunately the driver currently supports neither multicolor API nor >>> using the properties function & color, so we use label instead. >>> >>> Signed-off-by: Luca Weiss <luca@z3ntu.xyz> >>> --- >>> >>> .../arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts | 28 >>> ++++++++++++++++++++++ 1 file changed, 28 insertions(+) >>> >>> diff --git a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts >>> b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts index >>> ffb486ceb6a6..a672c45d7070 100644 >>> --- a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts >>> +++ b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts >>> @@ -114,6 +114,34 @@ led@0 { >>> >>> default-brightness = <80>; >>> >>> }; >>> >>> }; >>> >>> + >>> + led-controller@68 { >>> + compatible = "si-en,sn3193"; >>> + reg = <0x68>; >>> + >>> + shutdown-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>; >>> + >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + led@1 { >>> + reg = <1>; >>> + label = "red:status"; >> >> These should be colors and functions. > > Hi Krzysztof, > > please check the commit message for this. I tried using it but it didn't work, > the driver in general might need an overhaul at some point... > >> Unfortunately the driver currently supports neither multicolor API nor >> using the properties function & color, so we use label instead. > > Or maybe I messed up but pretty sure this was the case when I wrote the dts. I don't understand how driver is related here - it's not the driver's property, but LED core. If the driver "supports" label, then it supports color and status. Best regards, Krzysztof
On Donnerstag, 26. Jänner 2023 10:41:24 CET Krzysztof Kozlowski wrote: > On 25/01/2023 21:51, Luca Weiss wrote: > > On Montag, 23. Jänner 2023 18:18:16 CET Krzysztof Kozlowski wrote: > >> On 22/01/2023 17:48, Luca Weiss wrote: > >>> Add the node describing the sn3193 that's used to provide notification > >>> LED. > >>> > >>> Unfortunately the driver currently supports neither multicolor API nor > >>> using the properties function & color, so we use label instead. > >>> > >>> Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > >>> --- > >>> > >>> .../arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts | 28 > >>> ++++++++++++++++++++++ 1 file changed, 28 insertions(+) > >>> > >>> diff --git a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > >>> b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts index > >>> ffb486ceb6a6..a672c45d7070 100644 > >>> --- a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > >>> +++ b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > >>> @@ -114,6 +114,34 @@ led@0 { > >>> > >>> default-brightness = <80>; > >>> > >>> }; > >>> > >>> }; > >>> > >>> + > >>> + led-controller@68 { > >>> + compatible = "si-en,sn3193"; > >>> + reg = <0x68>; > >>> + > >>> + shutdown-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>; > >>> + > >>> + #address-cells = <1>; > >>> + #size-cells = <0>; > >>> + > >>> + led@1 { > >>> + reg = <1>; > >>> + label = "red:status"; > >> > >> These should be colors and functions. > > > > Hi Krzysztof, > > > > please check the commit message for this. I tried using it but it didn't > > work, the driver in general might need an overhaul at some point... > > > >> Unfortunately the driver currently supports neither multicolor API nor > >> using the properties function & color, so we use label instead. > > > > Or maybe I messed up but pretty sure this was the case when I wrote the > > dts. > I don't understand how driver is related here - it's not the driver's > property, but LED core. If the driver "supports" label, then it supports > color and status. The leds-is31fl319x.c driver uses devm_led_classdev_register but the function where 'function' and 'color' is parsed is led_parse_fwnode_props which is called from led_compose_name which is called from led_classdev_register_ext (or with the devm_ prefix), and not called from the register method without _ext suffix. So in other words, the led driver would need changes to use the new registration API to be able to use 'function' and 'color' instead of 'label'. Regards Luca > > Best regards, > Krzysztof
On 26/01/2023 21:41, Luca Weiss wrote: > On Donnerstag, 26. Jänner 2023 10:41:24 CET Krzysztof Kozlowski wrote: >> On 25/01/2023 21:51, Luca Weiss wrote: >>> On Montag, 23. Jänner 2023 18:18:16 CET Krzysztof Kozlowski wrote: >>>> On 22/01/2023 17:48, Luca Weiss wrote: >>>>> Add the node describing the sn3193 that's used to provide notification >>>>> LED. >>>>> >>>>> Unfortunately the driver currently supports neither multicolor API nor >>>>> using the properties function & color, so we use label instead. >>>>> >>>>> Signed-off-by: Luca Weiss <luca@z3ntu.xyz> >>>>> --- >>>>> >>>>> .../arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts | 28 >>>>> ++++++++++++++++++++++ 1 file changed, 28 insertions(+) >>>>> >>>>> diff --git a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts >>>>> b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts index >>>>> ffb486ceb6a6..a672c45d7070 100644 >>>>> --- a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts >>>>> +++ b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts >>>>> @@ -114,6 +114,34 @@ led@0 { >>>>> >>>>> default-brightness = <80>; >>>>> >>>>> }; >>>>> >>>>> }; >>>>> >>>>> + >>>>> + led-controller@68 { >>>>> + compatible = "si-en,sn3193"; >>>>> + reg = <0x68>; >>>>> + >>>>> + shutdown-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>; >>>>> + >>>>> + #address-cells = <1>; >>>>> + #size-cells = <0>; >>>>> + >>>>> + led@1 { >>>>> + reg = <1>; >>>>> + label = "red:status"; >>>> >>>> These should be colors and functions. >>> >>> Hi Krzysztof, >>> >>> please check the commit message for this. I tried using it but it didn't >>> work, the driver in general might need an overhaul at some point... >>> >>>> Unfortunately the driver currently supports neither multicolor API nor >>>> using the properties function & color, so we use label instead. >>> >>> Or maybe I messed up but pretty sure this was the case when I wrote the >>> dts. >> I don't understand how driver is related here - it's not the driver's >> property, but LED core. If the driver "supports" label, then it supports >> color and status. > > The leds-is31fl319x.c driver uses devm_led_classdev_register but the function > where 'function' and 'color' is parsed is led_parse_fwnode_props which is > called from led_compose_name which is called from led_classdev_register_ext > (or with the devm_ prefix), and not called from the register method without > _ext suffix. No, devm_led_classdev_register calls devm_led_classdev_register_ext which calls led_classdev_register_ext which calls led_compose_name (if initdata is set). The point is that we lack initdata, so indeed color and function cannot be used. > > So in other words, the led driver would need changes to use the new > registration API to be able to use 'function' and 'color' instead of 'label'. > > Regards > Luca > >> >> Best regards, >> Krzysztof > > > > Best regards, Krzysztof
Hi! > > Unfortunately the driver currently supports neither multicolor API nor > > using the properties function & color, so we use label instead. > > > > Signed-off-by: Luca Weiss <luca@z3ntu.xyz> > > --- > > .../arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts | 28 ++++++++++++++++++++++ > > 1 file changed, 28 insertions(+) > > > > diff --git a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > > index ffb486ceb6a6..a672c45d7070 100644 > > --- a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > > +++ b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts > > @@ -114,6 +114,34 @@ led@0 { > > default-brightness = <80>; > > }; > > }; > > + > > + led-controller@68 { > > + compatible = "si-en,sn3193"; > > + reg = <0x68>; > > + > > + shutdown-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + led@1 { > > + reg = <1>; > > + label = "red:status"; > > These should be colors and functions. Yep. Plus this is one LED, not three, so it should use the multicolor class. And then we should have an entry in Documentation/leds/well-known-leds.txt so that we have same ABI on all the phones. Best regards, Pavel
diff --git a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts index ffb486ceb6a6..a672c45d7070 100644 --- a/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts +++ b/arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts @@ -114,6 +114,34 @@ led@0 { default-brightness = <80>; }; }; + + led-controller@68 { + compatible = "si-en,sn3193"; + reg = <0x68>; + + shutdown-gpios = <&tlmm 45 GPIO_ACTIVE_HIGH>; + + #address-cells = <1>; + #size-cells = <0>; + + led@1 { + reg = <1>; + label = "red:status"; + led-max-microamp = <17500>; + }; + + led@2 { + reg = <2>; + label = "green:status"; + led-max-microamp = <17500>; + }; + + led@3 { + reg = <3>; + label = "blue:status"; + led-max-microamp = <17500>; + }; + }; }; &blsp1_i2c6 {
Add the node describing the sn3193 that's used to provide notification LED. Unfortunately the driver currently supports neither multicolor API nor using the properties function & color, so we use label instead. Signed-off-by: Luca Weiss <luca@z3ntu.xyz> --- .../arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dts | 28 ++++++++++++++++++++++ 1 file changed, 28 insertions(+)