| Message ID | 20240208062836.19767-2-quic_tdas@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add updates for clock controllers to support QCM6490 | expand |
On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote: > > When remoteproc is used to boot the LPASS the ADSP PLL should not be > configured from the high level OS. Why? > Thus add support for property to > avoid configuring the LPASS PLL. > > Signed-off-by: Taniya Das <quic_tdas@quicinc.com> > --- > .../devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml > index deee5423d66e..358eb4a1cffd 100644 > --- a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml > +++ b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml > @@ -49,6 +49,11 @@ properties: > peripheral loader. > type: boolean > > + qcom,adsp-skip-pll: > + description: > + Indicates if the LPASS PLL configuration is to be skipped. > + type: boolean This property describes OS behaviour rather than the hardware. Such things are generally not acceptable. > + > required: > - compatible > - reg > -- > 2.17.1 > >
On 08/02/2024 07:28, Taniya Das wrote: > When remoteproc is used to boot the LPASS the ADSP PLL should not be > configured from the high level OS. Thus add support for property to > avoid configuring the LPASS PLL. > > Signed-off-by: Taniya Das <quic_tdas@quicinc.com> You nicely bypassed all my filters. Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. Anyway, I don't understand point of this commit. Aren't you now duplicating qcom,adsp-pil-mode? Best regards, Krzysztof
Thanks for your review Krzysztof. On 2/8/2024 1:45 PM, Krzysztof Kozlowski wrote: > On 08/02/2024 07:28, Taniya Das wrote: >> When remoteproc is used to boot the LPASS the ADSP PLL should not be >> configured from the high level OS. Thus add support for property to >> avoid configuring the LPASS PLL. >> >> Signed-off-by: Taniya Das <quic_tdas@quicinc.com> > > You nicely bypassed all my filters. > > Please use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > your patch is touching. > My bad, I will update the commit subject. > Anyway, I don't understand point of this commit. Aren't you now > duplicating qcom,adsp-pil-mode? No, the expectation with pil-mode was still certain level of configuration from HLOS LPASS clock drivers. But on the QCM6490 boards these clocks need to be completely reserved except the resets. > > Best regards, > Krzysztof >
Thanks for your review, I will take care of the comments in my next patch series. On 2/8/2024 12:29 PM, Dmitry Baryshkov wrote: > On Thu, 8 Feb 2024 at 08:29, Taniya Das <quic_tdas@quicinc.com> wrote: >> >> When remoteproc is used to boot the LPASS the ADSP PLL should not be >> configured from the high level OS. > > Why? > >> Thus add support for property to >> avoid configuring the LPASS PLL. >> >> Signed-off-by: Taniya Das <quic_tdas@quicinc.com> >> --- >> .../devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml >> index deee5423d66e..358eb4a1cffd 100644 >> --- a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml >> +++ b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml >> @@ -49,6 +49,11 @@ properties: >> peripheral loader. >> type: boolean >> >> + qcom,adsp-skip-pll: >> + description: >> + Indicates if the LPASS PLL configuration is to be skipped. >> + type: boolean > > This property describes OS behaviour rather than the hardware. Such > things are generally not acceptable. > >> + >> required: >> - compatible >> - reg >> -- >> 2.17.1 >> >> > >
On 06/03/2024 07:53, Taniya Das wrote: > Thanks for your review Krzysztof. > > On 2/8/2024 1:45 PM, Krzysztof Kozlowski wrote: >> On 08/02/2024 07:28, Taniya Das wrote: >>> When remoteproc is used to boot the LPASS the ADSP PLL should not be >>> configured from the high level OS. Thus add support for property to >>> avoid configuring the LPASS PLL. >>> >>> Signed-off-by: Taniya Das <quic_tdas@quicinc.com> >> >> You nicely bypassed all my filters. >> >> Please use subject prefixes matching the subsystem. You can get them for >> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory >> your patch is touching. >> > > My bad, I will update the commit subject. > >> Anyway, I don't understand point of this commit. Aren't you now >> duplicating qcom,adsp-pil-mode? > > No, the expectation with pil-mode was still certain level of > configuration from HLOS LPASS clock drivers. But on the QCM6490 boards > these clocks need to be completely reserved except the resets. Sounds like qcom,adsp-pil-mode or qcom,controlled-remotely or one of others. Stop inventing 10 properties describing similar case, one for each binding. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml index deee5423d66e..358eb4a1cffd 100644 --- a/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml +++ b/Documentation/devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml @@ -49,6 +49,11 @@ properties: peripheral loader. type: boolean + qcom,adsp-skip-pll: + description: + Indicates if the LPASS PLL configuration is to be skipped. + type: boolean + required: - compatible - reg
When remoteproc is used to boot the LPASS the ADSP PLL should not be configured from the high level OS. Thus add support for property to avoid configuring the LPASS PLL. Signed-off-by: Taniya Das <quic_tdas@quicinc.com> --- .../devicetree/bindings/clock/qcom,sc7280-lpasscorecc.yaml | 5 +++++ 1 file changed, 5 insertions(+) -- 2.17.1