diff mbox series

[2/4] dt-bindings: display: msm: dp-controller: document clock parents better

Message ID 20241202-dp_mst_bindings-v1-2-9a9a43b0624a@quicinc.com (mailing list archive)
State New, archived
Headers show
Series dt-bindings: msm/dp: add support for pixel clock to driver another stream | expand

Commit Message

Abhinav Kumar Dec. 3, 2024, 3:31 a.m. UTC
Document the assigned-clock-parents better for the DP controller node
to indicate its functionality better.

Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
---
 Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Krzysztof Kozlowski Dec. 3, 2024, 8:01 a.m. UTC | #1
On 03/12/2024 04:31, Abhinav Kumar wrote:
> Document the assigned-clock-parents better for the DP controller node
> to indicate its functionality better.


You change the clocks entirely, not "document". I would say that's an
ABI break if it really is a Linux requirement. You could avoid any
problems by just dropping the property from binding.

> 
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
>  Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> index 35ae2630c2b3..9fe2bf0484d8 100644
> --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> @@ -72,8 +72,8 @@ properties:
>  
>    assigned-clock-parents:
>      items:
> -      - description: phy 0 parent
> -      - description: phy 1 parent
> +      - description: Link clock PLL output provided by PHY block
> +      - description: Stream 0 pixel clock PLL output provided by PHY block


Best regards,
Krzysztof
Dmitry Baryshkov Dec. 3, 2024, 1:41 p.m. UTC | #2
On Tue, Dec 03, 2024 at 09:01:31AM +0100, Krzysztof Kozlowski wrote:
> On 03/12/2024 04:31, Abhinav Kumar wrote:
> > Document the assigned-clock-parents better for the DP controller node
> > to indicate its functionality better.
> 
> 
> You change the clocks entirely, not "document". I would say that's an
> ABI break if it really is a Linux requirement. You could avoid any
> problems by just dropping the property from binding.

But if you take a look at the existing usage, the proposed change
matches the behaviour. So, I'd say, it's really a change that makes
documentation follow the actual hardware.

> 
> > 
> > Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> > ---
> >  Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > index 35ae2630c2b3..9fe2bf0484d8 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > @@ -72,8 +72,8 @@ properties:
> >  
> >    assigned-clock-parents:
> >      items:
> > -      - description: phy 0 parent
> > -      - description: phy 1 parent
> > +      - description: Link clock PLL output provided by PHY block
> > +      - description: Stream 0 pixel clock PLL output provided by PHY block
> 
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Dec. 4, 2024, 8:02 a.m. UTC | #3
On Tue, Dec 03, 2024 at 03:41:48PM +0200, Dmitry Baryshkov wrote:
> On Tue, Dec 03, 2024 at 09:01:31AM +0100, Krzysztof Kozlowski wrote:
> > On 03/12/2024 04:31, Abhinav Kumar wrote:
> > > Document the assigned-clock-parents better for the DP controller node
> > > to indicate its functionality better.
> > 
> > 
> > You change the clocks entirely, not "document". I would say that's an
> > ABI break if it really is a Linux requirement. You could avoid any
> > problems by just dropping the property from binding.
> 
> But if you take a look at the existing usage, the proposed change
> matches the behaviour. So, I'd say, it's really a change that makes
> documentation follow the actual hardware.

First, this should be in the commit msg, instead of "document better to
indicate functionality better".

Second, what is the point of documenting it in the first place if you
can change it and changing has no impact? So maybe just drop?

Best regards,
Krzysztof
Dmitry Baryshkov Dec. 4, 2024, 10:09 a.m. UTC | #4
On Wed, Dec 04, 2024 at 09:02:18AM +0100, Krzysztof Kozlowski wrote:
> On Tue, Dec 03, 2024 at 03:41:48PM +0200, Dmitry Baryshkov wrote:
> > On Tue, Dec 03, 2024 at 09:01:31AM +0100, Krzysztof Kozlowski wrote:
> > > On 03/12/2024 04:31, Abhinav Kumar wrote:
> > > > Document the assigned-clock-parents better for the DP controller node
> > > > to indicate its functionality better.
> > > 
> > > 
> > > You change the clocks entirely, not "document". I would say that's an
> > > ABI break if it really is a Linux requirement. You could avoid any
> > > problems by just dropping the property from binding.
> > 
> > But if you take a look at the existing usage, the proposed change
> > matches the behaviour. So, I'd say, it's really a change that makes
> > documentation follow the actual hardware.
> 
> First, this should be in the commit msg, instead of "document better to
> indicate functionality better".
> 
> Second, what is the point of documenting it in the first place if you
> can change it and changing has no impact? So maybe just drop?

So, do you suggest setting both of the property descriptions to true? Or
dropping them completely and using unevaluatedProperties instead of
additionalProperties?
Krzysztof Kozlowski Dec. 5, 2024, 7:33 a.m. UTC | #5
On 04/12/2024 11:09, Dmitry Baryshkov wrote:
> On Wed, Dec 04, 2024 at 09:02:18AM +0100, Krzysztof Kozlowski wrote:
>> On Tue, Dec 03, 2024 at 03:41:48PM +0200, Dmitry Baryshkov wrote:
>>> On Tue, Dec 03, 2024 at 09:01:31AM +0100, Krzysztof Kozlowski wrote:
>>>> On 03/12/2024 04:31, Abhinav Kumar wrote:
>>>>> Document the assigned-clock-parents better for the DP controller node
>>>>> to indicate its functionality better.
>>>>
>>>>
>>>> You change the clocks entirely, not "document". I would say that's an
>>>> ABI break if it really is a Linux requirement. You could avoid any
>>>> problems by just dropping the property from binding.
>>>
>>> But if you take a look at the existing usage, the proposed change
>>> matches the behaviour. So, I'd say, it's really a change that makes
>>> documentation follow the actual hardware.
>>
>> First, this should be in the commit msg, instead of "document better to
>> indicate functionality better".
>>
>> Second, what is the point of documenting it in the first place if you
>> can change it and changing has no impact? So maybe just drop?
> 
> So, do you suggest setting both of the property descriptions to true? Or
> dropping them completely and using unevaluatedProperties instead of
> additionalProperties?
> 

Dropping them entirely, without any changes of additionalProperties.
Unless this property was added due to limitation of dtschema?


Best regards,
Krzysztof
Dmitry Baryshkov Dec. 5, 2024, 11:32 a.m. UTC | #6
On Thu, 5 Dec 2024 at 09:33, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 04/12/2024 11:09, Dmitry Baryshkov wrote:
> > On Wed, Dec 04, 2024 at 09:02:18AM +0100, Krzysztof Kozlowski wrote:
> >> On Tue, Dec 03, 2024 at 03:41:48PM +0200, Dmitry Baryshkov wrote:
> >>> On Tue, Dec 03, 2024 at 09:01:31AM +0100, Krzysztof Kozlowski wrote:
> >>>> On 03/12/2024 04:31, Abhinav Kumar wrote:
> >>>>> Document the assigned-clock-parents better for the DP controller node
> >>>>> to indicate its functionality better.
> >>>>
> >>>>
> >>>> You change the clocks entirely, not "document". I would say that's an
> >>>> ABI break if it really is a Linux requirement. You could avoid any
> >>>> problems by just dropping the property from binding.
> >>>
> >>> But if you take a look at the existing usage, the proposed change
> >>> matches the behaviour. So, I'd say, it's really a change that makes
> >>> documentation follow the actual hardware.
> >>
> >> First, this should be in the commit msg, instead of "document better to
> >> indicate functionality better".
> >>
> >> Second, what is the point of documenting it in the first place if you
> >> can change it and changing has no impact? So maybe just drop?
> >
> > So, do you suggest setting both of the property descriptions to true? Or
> > dropping them completely and using unevaluatedProperties instead of
> > additionalProperties?
> >
>
> Dropping them entirely, without any changes of additionalProperties.
> Unless this property was added due to limitation of dtschema?

I  don't remember at this point. I think it's worth trying to drop them.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
index 35ae2630c2b3..9fe2bf0484d8 100644
--- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
+++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
@@ -72,8 +72,8 @@  properties:
 
   assigned-clock-parents:
     items:
-      - description: phy 0 parent
-      - description: phy 1 parent
+      - description: Link clock PLL output provided by PHY block
+      - description: Stream 0 pixel clock PLL output provided by PHY block
 
   phys:
     maxItems: 1