All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Marijn Suijten <marijn.suijten@somainline.org>,
	Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org,
	devicetree@vger.kernel.org, robdclark@gmail.com,
	quic_abhinavk@quicinc.com, sean@poorly.run, airlied@gmail.com,
	daniel@ffwll.ch, robh+dt@kernel.org, dianders@chromium.org,
	david@ixit.cz, krzysztof.kozlowski+dt@linaro.org,
	swboyd@chromium.org, konrad.dybcio@somainline.org,
	agross@kernel.org, andersson@kernel.org,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Subject: Re: [PATCH v9 2/2] dt-bindings: msm: dsi-controller-main: Document clocks on a per compatible basis
Date: Thu, 22 Jun 2023 01:18:13 +0300	[thread overview]
Message-ID: <c05a9a02-0a33-6160-9072-717efe30031a@linaro.org> (raw)
In-Reply-To: <qew6nd3jqnasb3mivvdxcwugfrvxdeafilaxk35v7uihagk2qi@oxe3oqdgfwpe>

On 22/06/2023 00:45, Marijn Suijten wrote:
> Hi!
> 
> On 2023-01-18 17:16:21, Bryan O'Donoghue wrote:
>> Each compatible has a different set of clocks which are associated with it.
>> Add in the list of clocks for each compatible.
> 
> So if each set of compatibles have their own unique set of clocks, is
> there a reason to have so many duplicate then: blocks?  I ran into this
> while preparing for submitting SM6125 DPU and having no clue where to
> add it.
> 
>> Acked-by: Rob Herring <robh@kernel.org>
>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>   .../display/msm/dsi-controller-main.yaml      | 218 ++++++++++++++++--
>>   1 file changed, 201 insertions(+), 17 deletions(-)
>>

[skipped most of the comments]

> 
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sc7180-dsi-ctrl
>> +              - qcom,sc7280-dsi-ctrl
>> +              - qcom,sm8250-dsi-ctrl
>> +              - qcom,sm8150-dsi-ctrl
>> +              - qcom,sm8250-dsi-ctrl
>> +              - qcom,sm8350-dsi-ctrl
>> +              - qcom,sm8450-dsi-ctrl
>> +              - qcom,sm8550-dsi-ctrl
>> +    then:
>> +      properties:
>> +        clocks:
>> +          maxItems: 6
>> +        clock-names:
>> +          items:
>> +            - const: byte
>> +            - const: byte_intf
>> +            - const: pixel
>> +            - const: core
>> +            - const: iface
>> +            - const: bus
> 
> ... and here ...
> 
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sdm660-dsi-ctrl
>> +    then:
>> +      properties:
>> +        clocks:
>> +          maxItems: 9
>> +        clock-names:
>> +          items:
>> +            - const: mdp_core
>> +            - const: byte
>> +            - const: byte_intf
>> +            - const: mnoc
>> +            - const: iface
>> +            - const: bus
>> +            - const: core_mmss
>> +            - const: pixel
>> +            - const: core
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sdm845-dsi-ctrl
>> +    then:
>> +      properties:
>> +        clocks:
>> +          maxItems: 6
>> +        clock-names:
>> +          items:
>> +            - const: byte
>> +            - const: byte_intf
>> +            - const: pixel
>> +            - const: core
>> +            - const: iface
>> +            - const: bus
> 
> and here, we have *three* identical lists of clocks.  Should they (have
> been) combined?
> 
> I can send a patch fixing these all if desired!

Probably it would be logical to split follow DPU and MDSS schema and 
split this file into per-SoC compatibles and a generic file. Then it 
would be easier to review different SoC parts.

Regarding reordering of clocks. I think we have 5 different 
configurations in dsi_cfg.c, but we definitely can optimize the schema.

> 
> - Marijn
> 
>> +
>>   additionalProperties: false
>>   
>>   examples:
>> -- 
>> 2.38.1
>>

-- 
With best wishes
Dmitry


WARNING: multiple messages have this Message-ID (diff)
From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: Marijn Suijten <marijn.suijten@somainline.org>,
	Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org,
	krzysztof.kozlowski+dt@linaro.org, devicetree@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, andersson@kernel.org,
	konrad.dybcio@somainline.org, quic_abhinavk@quicinc.com,
	david@ixit.cz, dianders@chromium.org,
	Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	robh+dt@kernel.org, agross@kernel.org, swboyd@chromium.org,
	sean@poorly.run, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v9 2/2] dt-bindings: msm: dsi-controller-main: Document clocks on a per compatible basis
Date: Thu, 22 Jun 2023 01:18:13 +0300	[thread overview]
Message-ID: <c05a9a02-0a33-6160-9072-717efe30031a@linaro.org> (raw)
In-Reply-To: <qew6nd3jqnasb3mivvdxcwugfrvxdeafilaxk35v7uihagk2qi@oxe3oqdgfwpe>

On 22/06/2023 00:45, Marijn Suijten wrote:
> Hi!
> 
> On 2023-01-18 17:16:21, Bryan O'Donoghue wrote:
>> Each compatible has a different set of clocks which are associated with it.
>> Add in the list of clocks for each compatible.
> 
> So if each set of compatibles have their own unique set of clocks, is
> there a reason to have so many duplicate then: blocks?  I ran into this
> while preparing for submitting SM6125 DPU and having no clue where to
> add it.
> 
>> Acked-by: Rob Herring <robh@kernel.org>
>> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>   .../display/msm/dsi-controller-main.yaml      | 218 ++++++++++++++++--
>>   1 file changed, 201 insertions(+), 17 deletions(-)
>>

[skipped most of the comments]

> 
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sc7180-dsi-ctrl
>> +              - qcom,sc7280-dsi-ctrl
>> +              - qcom,sm8250-dsi-ctrl
>> +              - qcom,sm8150-dsi-ctrl
>> +              - qcom,sm8250-dsi-ctrl
>> +              - qcom,sm8350-dsi-ctrl
>> +              - qcom,sm8450-dsi-ctrl
>> +              - qcom,sm8550-dsi-ctrl
>> +    then:
>> +      properties:
>> +        clocks:
>> +          maxItems: 6
>> +        clock-names:
>> +          items:
>> +            - const: byte
>> +            - const: byte_intf
>> +            - const: pixel
>> +            - const: core
>> +            - const: iface
>> +            - const: bus
> 
> ... and here ...
> 
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sdm660-dsi-ctrl
>> +    then:
>> +      properties:
>> +        clocks:
>> +          maxItems: 9
>> +        clock-names:
>> +          items:
>> +            - const: mdp_core
>> +            - const: byte
>> +            - const: byte_intf
>> +            - const: mnoc
>> +            - const: iface
>> +            - const: bus
>> +            - const: core_mmss
>> +            - const: pixel
>> +            - const: core
>> +
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - qcom,sdm845-dsi-ctrl
>> +    then:
>> +      properties:
>> +        clocks:
>> +          maxItems: 6
>> +        clock-names:
>> +          items:
>> +            - const: byte
>> +            - const: byte_intf
>> +            - const: pixel
>> +            - const: core
>> +            - const: iface
>> +            - const: bus
> 
> and here, we have *three* identical lists of clocks.  Should they (have
> been) combined?
> 
> I can send a patch fixing these all if desired!

Probably it would be logical to split follow DPU and MDSS schema and 
split this file into per-SoC compatibles and a generic file. Then it 
would be easier to review different SoC parts.

Regarding reordering of clocks. I think we have 5 different 
configurations in dsi_cfg.c, but we definitely can optimize the schema.

> 
> - Marijn
> 
>> +
>>   additionalProperties: false
>>   
>>   examples:
>> -- 
>> 2.38.1
>>

-- 
With best wishes
Dmitry


  reply	other threads:[~2023-06-21 22:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-18 17:16 [PATCH v9 0/2] mdss-dsi-ctrl binding and dts fixes Bryan O'Donoghue
2023-01-18 17:16 ` Bryan O'Donoghue
2023-01-18 17:16 ` [PATCH v9 1/2] dt-bindings: msm: dsi-controller-main: Add compatible strings for every current SoC Bryan O'Donoghue
2023-01-18 17:16   ` Bryan O'Donoghue
2023-01-18 17:16 ` [PATCH v9 2/2] dt-bindings: msm: dsi-controller-main: Document clocks on a per compatible basis Bryan O'Donoghue
2023-01-18 17:16   ` Bryan O'Donoghue
2023-06-21 21:45   ` Marijn Suijten
2023-06-21 21:45     ` Marijn Suijten
2023-06-21 22:18     ` Dmitry Baryshkov [this message]
2023-06-21 22:18       ` Dmitry Baryshkov
2023-01-26 19:00 ` [PATCH v9 0/2] mdss-dsi-ctrl binding and dts fixes Dmitry Baryshkov
2023-01-26 19:00   ` Dmitry Baryshkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c05a9a02-0a33-6160-9072-717efe30031a@linaro.org \
    --to=dmitry.baryshkov@linaro.org \
    --cc=agross@kernel.org \
    --cc=airlied@gmail.com \
    --cc=andersson@kernel.org \
    --cc=bryan.odonoghue@linaro.org \
    --cc=daniel@ffwll.ch \
    --cc=david@ixit.cz \
    --cc=devicetree@vger.kernel.org \
    --cc=dianders@chromium.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=konrad.dybcio@somainline.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marijn.suijten@somainline.org \
    --cc=quic_abhinavk@quicinc.com \
    --cc=robdclark@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=robh@kernel.org \
    --cc=sean@poorly.run \
    --cc=swboyd@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.