| Message ID | 1653534995-30794-2-git-send-email-u0084500@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add the property to make ocp level selectable | expand |
On 26/05/2022 05:16, cy_huang wrote: > From: ChiYuan Huang <cy_huang@richtek.com> > > Add the new property for ocp level selection. > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > --- > .../bindings/leds/backlight/richtek,rt4831-backlight.yaml | 8 ++++++++ > include/dt-bindings/leds/rt4831-backlight.h | 5 +++++ > 2 files changed, 13 insertions(+) > > diff --git a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > index e0ac686..c1c59de 100644 > --- a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > +++ b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > @@ -47,6 +47,14 @@ properties: > minimum: 0 > maximum: 3 > > + richtek,bled-ocp-sel: Skip "sel" as it is a shortcut of selection. Name instead: "richtek,backlight-ocp" > + description: | > + Backlight OCP level selection, currently support 0.9A/1.2A/1.5A/1.8A Could you explain here what is OCP (unfold the acronym)? Best regards, Krzysztof
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 於 2022年5月26日 週四 下午4:06寫道： > > On 26/05/2022 05:16, cy_huang wrote: > > From: ChiYuan Huang <cy_huang@richtek.com> > > > > Add the new property for ocp level selection. > > > > Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > > --- > > .../bindings/leds/backlight/richtek,rt4831-backlight.yaml | 8 ++++++++ > > include/dt-bindings/leds/rt4831-backlight.h | 5 +++++ > > 2 files changed, 13 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > > index e0ac686..c1c59de 100644 > > --- a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > > +++ b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > > @@ -47,6 +47,14 @@ properties: > > minimum: 0 > > maximum: 3 > > > > + richtek,bled-ocp-sel: > > Skip "sel" as it is a shortcut of selection. Name instead: > "richtek,backlight-ocp" > OK, if so, do I need to rename all properties from 'bled' to 'backlight' ? If only this property is naming as 'backlight'. it may conflict with the others like as "richtek,bled-ovp-sel". > > > + description: | > > + Backlight OCP level selection, currently support 0.9A/1.2A/1.5A/1.8A > > Could you explain here what is OCP (unfold the acronym)? Yes. And the full name is 'over current protection'. > > > Best regards, > Krzysztof
On 26/05/2022 10:13, ChiYuan Huang wrote: > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 於 2022年5月26日 週四 下午4:06寫道： >> >> On 26/05/2022 05:16, cy_huang wrote: >>> From: ChiYuan Huang <cy_huang@richtek.com> >>> >>> Add the new property for ocp level selection. >>> >>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> >>> --- >>> .../bindings/leds/backlight/richtek,rt4831-backlight.yaml | 8 ++++++++ >>> include/dt-bindings/leds/rt4831-backlight.h | 5 +++++ >>> 2 files changed, 13 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>> index e0ac686..c1c59de 100644 >>> --- a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>> +++ b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>> @@ -47,6 +47,14 @@ properties: >>> minimum: 0 >>> maximum: 3 >>> >>> + richtek,bled-ocp-sel: >> >> Skip "sel" as it is a shortcut of selection. Name instead: >> "richtek,backlight-ocp" >> > OK, if so, do I need to rename all properties from 'bled' to 'backlight' ? > If only this property is naming as 'backlight'. it may conflict with > the others like as "richtek,bled-ovp-sel". Ah, no, no need. >> >>> + description: | >>> + Backlight OCP level selection, currently support 0.9A/1.2A/1.5A/1.8A >> >> Could you explain here what is OCP (unfold the acronym)? > Yes. And the full name is 'over current protection'. Thanks and this leads to second thing - you encode register value instead of logical value. This must be a logical value in mA, so "richtek,bled-ocp-microamp". I see you already did some register-style for voltage. It's wrong but it was done, so let it be. But let's don't make that a pattern... >> >> >> Best regards, >> Krzysztof Best regards, Krzysztof
On Thu, May 26, 2022 at 12:32:12PM +0200, Krzysztof Kozlowski wrote: > On 26/05/2022 10:13, ChiYuan Huang wrote: > > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 於 2022年5月26日 週四 下午4:06寫道： > >> > >> On 26/05/2022 05:16, cy_huang wrote: > >>> From: ChiYuan Huang <cy_huang@richtek.com> > >>> > >>> Add the new property for ocp level selection. > >>> > >>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > >>> --- > >>> .../bindings/leds/backlight/richtek,rt4831-backlight.yaml | 8 ++++++++ > >>> include/dt-bindings/leds/rt4831-backlight.h | 5 +++++ > >>> 2 files changed, 13 insertions(+) > >>> > >>> diff --git a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > >>> index e0ac686..c1c59de 100644 > >>> --- a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > >>> +++ b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > >>> @@ -47,6 +47,14 @@ properties: > >>> minimum: 0 > >>> maximum: 3 > >>> > >>> + richtek,bled-ocp-sel: > >> > >> Skip "sel" as it is a shortcut of selection. Name instead: > >> "richtek,backlight-ocp" > >> > > OK, if so, do I need to rename all properties from 'bled' to 'backlight' ? > > If only this property is naming as 'backlight'. it may conflict with > > the others like as "richtek,bled-ovp-sel". > > Ah, no, no need. > > >> > >>> + description: | > >>> + Backlight OCP level selection, currently support 0.9A/1.2A/1.5A/1.8A > >> > >> Could you explain here what is OCP (unfold the acronym)? > > Yes. And the full name is 'over current protection'. > > Thanks and this leads to second thing - you encode register value > instead of logical value. This must be a logical value in mA, so > "richtek,bled-ocp-microamp". We already have common properties for setting current of LEDs. We should use that here I think. Rob
On 02/06/2022 15:56, Rob Herring wrote: > On Thu, May 26, 2022 at 12:32:12PM +0200, Krzysztof Kozlowski wrote: >> On 26/05/2022 10:13, ChiYuan Huang wrote: >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 於 2022年5月26日 週四 下午4:06寫道： >>>> >>>> On 26/05/2022 05:16, cy_huang wrote: >>>>> From: ChiYuan Huang <cy_huang@richtek.com> >>>>> >>>>> Add the new property for ocp level selection. >>>>> >>>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> >>>>> --- >>>>> .../bindings/leds/backlight/richtek,rt4831-backlight.yaml | 8 ++++++++ >>>>> include/dt-bindings/leds/rt4831-backlight.h | 5 +++++ >>>>> 2 files changed, 13 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>>>> index e0ac686..c1c59de 100644 >>>>> --- a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>>>> +++ b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>>>> @@ -47,6 +47,14 @@ properties: >>>>> minimum: 0 >>>>> maximum: 3 >>>>> >>>>> + richtek,bled-ocp-sel: >>>> >>>> Skip "sel" as it is a shortcut of selection. Name instead: >>>> "richtek,backlight-ocp" >>>> >>> OK, if so, do I need to rename all properties from 'bled' to 'backlight' ? >>> If only this property is naming as 'backlight'. it may conflict with >>> the others like as "richtek,bled-ovp-sel". >> >> Ah, no, no need. >> >>>> >>>>> + description: | >>>>> + Backlight OCP level selection, currently support 0.9A/1.2A/1.5A/1.8A >>>> >>>> Could you explain here what is OCP (unfold the acronym)? >>> Yes. And the full name is 'over current protection'. >> >> Thanks and this leads to second thing - you encode register value >> instead of logical value. This must be a logical value in mA, so >> "richtek,bled-ocp-microamp". > > We already have common properties for setting current of LEDs. We should > use that here I think. It might not be exactly the same. We have "led-max-microamp" which is the maximum allowed current. I guess over-current protection level is slightly higher (e.g. led-max-microamp + 1). IOW, led-max-microamp is something which still can be set and used by system/hardware. OCP should not. Best regards, Krzysztof
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 於 2022年6月2日 週四 下午9:58寫道： > > On 02/06/2022 15:56, Rob Herring wrote: > > On Thu, May 26, 2022 at 12:32:12PM +0200, Krzysztof Kozlowski wrote: > >> On 26/05/2022 10:13, ChiYuan Huang wrote: > >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 於 2022年5月26日 週四 下午4:06寫道： > >>>> > >>>> On 26/05/2022 05:16, cy_huang wrote: > >>>>> From: ChiYuan Huang <cy_huang@richtek.com> > >>>>> > >>>>> Add the new property for ocp level selection. > >>>>> > >>>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> > >>>>> --- > >>>>> .../bindings/leds/backlight/richtek,rt4831-backlight.yaml | 8 ++++++++ > >>>>> include/dt-bindings/leds/rt4831-backlight.h | 5 +++++ > >>>>> 2 files changed, 13 insertions(+) > >>>>> > >>>>> diff --git a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > >>>>> index e0ac686..c1c59de 100644 > >>>>> --- a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > >>>>> +++ b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml > >>>>> @@ -47,6 +47,14 @@ properties: > >>>>> minimum: 0 > >>>>> maximum: 3 > >>>>> > >>>>> + richtek,bled-ocp-sel: > >>>> > >>>> Skip "sel" as it is a shortcut of selection. Name instead: > >>>> "richtek,backlight-ocp" > >>>> > >>> OK, if so, do I need to rename all properties from 'bled' to 'backlight' ? > >>> If only this property is naming as 'backlight'. it may conflict with > >>> the others like as "richtek,bled-ovp-sel". > >> > >> Ah, no, no need. > >> > >>>> > >>>>> + description: | > >>>>> + Backlight OCP level selection, currently support 0.9A/1.2A/1.5A/1.8A > >>>> > >>>> Could you explain here what is OCP (unfold the acronym)? > >>> Yes. And the full name is 'over current protection'. > >> > >> Thanks and this leads to second thing - you encode register value > >> instead of logical value. This must be a logical value in mA, so > >> "richtek,bled-ocp-microamp". > > > > We already have common properties for setting current of LEDs. We should > > use that here I think. > > It might not be exactly the same. We have "led-max-microamp" which is > the maximum allowed current. I guess over-current protection level is > slightly higher (e.g. led-max-microamp + 1). IOW, led-max-microamp is > something which still can be set and used by system/hardware. OCP should > not. > Yap, you're right.
On 02/06/2022 17:31, ChiYuan Huang wrote: > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 於 2022年6月2日 週四 下午9:58寫道： >> >> On 02/06/2022 15:56, Rob Herring wrote: >>> On Thu, May 26, 2022 at 12:32:12PM +0200, Krzysztof Kozlowski wrote: >>>> On 26/05/2022 10:13, ChiYuan Huang wrote: >>>>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 於 2022年5月26日 週四 下午4:06寫道： >>>>>> >>>>>> On 26/05/2022 05:16, cy_huang wrote: >>>>>>> From: ChiYuan Huang <cy_huang@richtek.com> >>>>>>> >>>>>>> Add the new property for ocp level selection. >>>>>>> >>>>>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> >>>>>>> --- >>>>>>> .../bindings/leds/backlight/richtek,rt4831-backlight.yaml | 8 ++++++++ >>>>>>> include/dt-bindings/leds/rt4831-backlight.h | 5 +++++ >>>>>>> 2 files changed, 13 insertions(+) >>>>>>> >>>>>>> diff --git a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>>>>>> index e0ac686..c1c59de 100644 >>>>>>> --- a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>>>>>> +++ b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>>>>>> @@ -47,6 +47,14 @@ properties: >>>>>>> minimum: 0 >>>>>>> maximum: 3 >>>>>>> >>>>>>> + richtek,bled-ocp-sel: >>>>>> >>>>>> Skip "sel" as it is a shortcut of selection. Name instead: >>>>>> "richtek,backlight-ocp" >>>>>> >>>>> OK, if so, do I need to rename all properties from 'bled' to 'backlight' ? >>>>> If only this property is naming as 'backlight'. it may conflict with >>>>> the others like as "richtek,bled-ovp-sel". >>>> >>>> Ah, no, no need. >>>> >>>>>> >>>>>>> + description: | >>>>>>> + Backlight OCP level selection, currently support 0.9A/1.2A/1.5A/1.8A >>>>>> >>>>>> Could you explain here what is OCP (unfold the acronym)? >>>>> Yes. And the full name is 'over current protection'. >>>> >>>> Thanks and this leads to second thing - you encode register value >>>> instead of logical value. This must be a logical value in mA, so >>>> "richtek,bled-ocp-microamp". >>> >>> We already have common properties for setting current of LEDs. We should >>> use that here I think. >> >> It might not be exactly the same. We have "led-max-microamp" which is >> the maximum allowed current. I guess over-current protection level is >> slightly higher (e.g. led-max-microamp + 1). IOW, led-max-microamp is >> something which still can be set and used by system/hardware. OCP should >> not. >> > Yap, you're right. So I am right or Rob? > From the modern backlight IC design, it uses the boost converter architecture. > This OCP level is to limit the inductor current when the internal MOS > switch turn on. > Details can refer to the below wiki link > https://en.wikipedia.org/wiki/Boost_converter > > And based on it, OVP is used to limit the inductor output voltage. > Each channel maximum current is based on the IC affordable limit. > It is more like as what you said 'led-max-microamp'. > > So boost voltage level may depend on the LED VF. > The different series of LED may cause different boost voltage. > > RT4831's OVP/OCP is not just the protection, more like as the limit. This suggests Rob is right, so let's use led-max-microamp property? Best regards, Krzysztof
On 06/06/2022 03:39, ChiYuan Huang wrote: > Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 於 2022年6月6日 週一 上午12:11寫道： >> >> On 02/06/2022 17:31, ChiYuan Huang wrote: >>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 於 2022年6月2日 週四 下午9:58寫道： >>>> >>>> On 02/06/2022 15:56, Rob Herring wrote: >>>>> On Thu, May 26, 2022 at 12:32:12PM +0200, Krzysztof Kozlowski wrote: >>>>>> On 26/05/2022 10:13, ChiYuan Huang wrote: >>>>>>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 於 2022年5月26日 週四 下午4:06寫道： >>>>>>>> >>>>>>>> On 26/05/2022 05:16, cy_huang wrote: >>>>>>>>> From: ChiYuan Huang <cy_huang@richtek.com> >>>>>>>>> >>>>>>>>> Add the new property for ocp level selection. >>>>>>>>> >>>>>>>>> Signed-off-by: ChiYuan Huang <cy_huang@richtek.com> >>>>>>>>> --- >>>>>>>>> .../bindings/leds/backlight/richtek,rt4831-backlight.yaml | 8 ++++++++ >>>>>>>>> include/dt-bindings/leds/rt4831-backlight.h | 5 +++++ >>>>>>>>> 2 files changed, 13 insertions(+) >>>>>>>>> >>>>>>>>> diff --git a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>>>>>>>> index e0ac686..c1c59de 100644 >>>>>>>>> --- a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>>>>>>>> +++ b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml >>>>>>>>> @@ -47,6 +47,14 @@ properties: >>>>>>>>> minimum: 0 >>>>>>>>> maximum: 3 >>>>>>>>> >>>>>>>>> + richtek,bled-ocp-sel: >>>>>>>> >>>>>>>> Skip "sel" as it is a shortcut of selection. Name instead: >>>>>>>> "richtek,backlight-ocp" >>>>>>>> >>>>>>> OK, if so, do I need to rename all properties from 'bled' to 'backlight' ? >>>>>>> If only this property is naming as 'backlight'. it may conflict with >>>>>>> the others like as "richtek,bled-ovp-sel". >>>>>> >>>>>> Ah, no, no need. >>>>>> >>>>>>>> >>>>>>>>> + description: | >>>>>>>>> + Backlight OCP level selection, currently support 0.9A/1.2A/1.5A/1.8A >>>>>>>> >>>>>>>> Could you explain here what is OCP (unfold the acronym)? >>>>>>> Yes. And the full name is 'over current protection'. >>>>>> >>>>>> Thanks and this leads to second thing - you encode register value >>>>>> instead of logical value. This must be a logical value in mA, so >>>>>> "richtek,bled-ocp-microamp". >>>>> >>>>> We already have common properties for setting current of LEDs. We should >>>>> use that here I think. >>>> >>>> It might not be exactly the same. We have "led-max-microamp" which is >>>> the maximum allowed current. I guess over-current protection level is >>>> slightly higher (e.g. led-max-microamp + 1). IOW, led-max-microamp is >>>> something which still can be set and used by system/hardware. OCP should >>>> not. >>>> >>> Yap, you're right. >> >> So I am right or Rob? >> > As I know, both are incorrect. >>> From the modern backlight IC design, it uses the boost converter architecture. >>> This OCP level is to limit the inductor current when the internal MOS >>> switch turn on. >>> Details can refer to the below wiki link >>> https://en.wikipedia.org/wiki/Boost_converter >>> >>> And based on it, OVP is used to limit the inductor output voltage. >>> Each channel maximum current is based on the IC affordable limit. >>> It is more like as what you said 'led-max-microamp'. >>> >>> So boost voltage level may depend on the LED VF. >>> The different series of LED may cause different boost voltage. >>> >>> RT4831's OVP/OCP is not just the protection, more like as the limit. >> >> This suggests Rob is right, so let's use led-max-microamp property? >> > No, the meaning is different. 'led-max-microamp' always means the > channel output current. > It already can be adjusted by backlight brightness node. > > For example > low voltage side (3.3~4.4V) to generate the boost voltage to 16~17V, > even 20V for BLED Vout. > This OCP is to limit the input current of low voltage side. > > After the explanation, do you still think it's the same thing? This sounds differently so I propose to use this dedicated property with the changes I asked for. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml index e0ac686..c1c59de 100644 --- a/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/richtek,rt4831-backlight.yaml @@ -47,6 +47,14 @@ properties: minimum: 0 maximum: 3 + richtek,bled-ocp-sel: + description: | + Backlight OCP level selection, currently support 0.9A/1.2A/1.5A/1.8A + $ref: /schemas/types.yaml#/definitions/uint8 + default: 1 + minimum: 0 + maximum: 3 + richtek,channel-use: description: | Backlight LED channel to be used. diff --git a/include/dt-bindings/leds/rt4831-backlight.h b/include/dt-bindings/leds/rt4831-backlight.h index 125c635..e8b8609 100644 --- a/include/dt-bindings/leds/rt4831-backlight.h +++ b/include/dt-bindings/leds/rt4831-backlight.h @@ -14,6 +14,11 @@ #define RT4831_BLOVPLVL_25V 2 #define RT4831_BLOVPLVL_29V 3 +#define RT4831_BLOCPLVL_0P9A 0 +#define RT4831_BLOCPLVL_1P2A 1 +#define RT4831_BLOCPLVL_1P5A 2 +#define RT4831_BLOCPLVL_1P8A 3 + #define RT4831_BLED_CH1EN (1 << 0) #define RT4831_BLED_CH2EN (1 << 1) #define RT4831_BLED_CH3EN (1 << 2)