diff mbox series

[1/2] dt-bindings: backlight: rt4831: Add the new property for ocp level selection

Message ID 1653534995-30794-2-git-send-email-u0084500@gmail.com (mailing list archive)
State New, archived
Headers show
Series Add the property to make ocp level selectable | expand

Commit Message

ChiYuan Huang May 26, 2022, 3:16 a.m. UTC
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(+)

Comments

Krzysztof Kozlowski May 26, 2022, 8:06 a.m. UTC | #1
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
ChiYuan Huang May 26, 2022, 8:13 a.m. UTC | #2
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
Krzysztof Kozlowski May 26, 2022, 10:32 a.m. UTC | #3
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
Rob Herring June 2, 2022, 1:56 p.m. UTC | #4
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
Krzysztof Kozlowski June 2, 2022, 1:58 p.m. UTC | #5
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
ChiYuan Huang June 2, 2022, 3:31 p.m. UTC | #6
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.
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.
>
> Best regards,
> Krzysztof
Krzysztof Kozlowski June 5, 2022, 4:11 p.m. UTC | #7
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
ChiYuan Huang June 6, 2022, 1:39 a.m. UTC | #8
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?
> Best regards,
> Krzysztof
Krzysztof Kozlowski June 6, 2022, 6:20 a.m. UTC | #9
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 mbox series

Patch

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)