| Message ID | 20210519001802.1863-2-jonathan@marek.ca (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v2,1/2] clk: qcom: add support for SM8350 DISPCC | expand |
Quoting Jonathan Marek (2021-05-18 17:18:02) > Add sm8350 DISPCC bindings, which are simply a symlink to the sm8250 > bindings. Update the documentation with the new compatible. > > Signed-off-by: Jonathan Marek <jonathan@marek.ca> > Reviewed-by: Rob Herring <robh@kernel.org> > --- > .../devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml | 6 ++++-- > include/dt-bindings/clock/qcom,dispcc-sm8350.h | 1 + > 2 files changed, 5 insertions(+), 2 deletions(-) > create mode 120000 include/dt-bindings/clock/qcom,dispcc-sm8350.h Why the symlink? Can we have the dt authors use the existing header file instead? > > diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml > index 0cdf53f41f84..8f414642445e 100644 > --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml > +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml > @@ -4,24 +4,26 @@ > $id: http://devicetree.org/schemas/clock/qcom,dispcc-sm8x50.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > -title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250 > +title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250/SM8350 Maybe just "Binding for SM8x50 SoCs" > > maintainers: > - Jonathan Marek <jonathan@marek.ca> > > description: | > Qualcomm display clock control module which supports the clocks, resets and > - power domains on SM8150 and SM8250. > + power domains on SM8150/SM8250/SM8350. same 8x50 comment. > > See also: > dt-bindings/clock/qcom,dispcc-sm8150.h > dt-bindings/clock/qcom,dispcc-sm8250.h > + dt-bindings/clock/qcom,dispcc-sm8350.h > > properties: > compatible: > enum: > - qcom,sm8150-dispcc > - qcom,sm8250-dispcc > + - qcom,sm8350-dispcc > > clocks: > items: > diff --git a/include/dt-bindings/clock/qcom,dispcc-sm8350.h b/include/dt-bindings/clock/qcom,dispcc-sm8350.h > new file mode 120000 > index 000000000000..0312b4544acb > --- /dev/null > +++ b/include/dt-bindings/clock/qcom,dispcc-sm8350.h > @@ -0,0 +1 @@ > +qcom,dispcc-sm8250.h > \ No newline at end of file
On 6/2/21 5:27 PM, Stephen Boyd wrote: > Quoting Jonathan Marek (2021-05-18 17:18:02) >> Add sm8350 DISPCC bindings, which are simply a symlink to the sm8250 >> bindings. Update the documentation with the new compatible. >> >> Signed-off-by: Jonathan Marek <jonathan@marek.ca> >> Reviewed-by: Rob Herring <robh@kernel.org> >> --- >> .../devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml | 6 ++++-- >> include/dt-bindings/clock/qcom,dispcc-sm8350.h | 1 + > >> 2 files changed, 5 insertions(+), 2 deletions(-) >> create mode 120000 include/dt-bindings/clock/qcom,dispcc-sm8350.h > > Why the symlink? Can we have the dt authors use the existing header file > instead? > It would be strange to include bindings with the name of a different SoC. I guess it is a matter a preference, is there any good reason to *not* do it like this? >> >> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml >> index 0cdf53f41f84..8f414642445e 100644 >> --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml >> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml >> @@ -4,24 +4,26 @@ >> $id: http://devicetree.org/schemas/clock/qcom,dispcc-sm8x50.yaml# >> $schema: http://devicetree.org/meta-schemas/core.yaml# >> >> -title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250 >> +title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250/SM8350 > > Maybe just "Binding for SM8x50 SoCs" > Its likely these bindings won't be compatible with future "SM8x50" SoCs, listing supported SoCs explicitly will avoid confusion in the future. >> >> maintainers: >> - Jonathan Marek <jonathan@marek.ca> >> >> description: | >> Qualcomm display clock control module which supports the clocks, resets and >> - power domains on SM8150 and SM8250. >> + power domains on SM8150/SM8250/SM8350. > > same 8x50 comment. > >> >> See also: >> dt-bindings/clock/qcom,dispcc-sm8150.h >> dt-bindings/clock/qcom,dispcc-sm8250.h >> + dt-bindings/clock/qcom,dispcc-sm8350.h >> >> properties: >> compatible: >> enum: >> - qcom,sm8150-dispcc >> - qcom,sm8250-dispcc >> + - qcom,sm8350-dispcc >> >> clocks: >> items: >> diff --git a/include/dt-bindings/clock/qcom,dispcc-sm8350.h b/include/dt-bindings/clock/qcom,dispcc-sm8350.h >> new file mode 120000 >> index 000000000000..0312b4544acb >> --- /dev/null >> +++ b/include/dt-bindings/clock/qcom,dispcc-sm8350.h >> @@ -0,0 +1 @@ >> +qcom,dispcc-sm8250.h >> \ No newline at end of file
On Wed 02 Jun 16:27 CDT 2021, Stephen Boyd wrote: > Quoting Jonathan Marek (2021-05-18 17:18:02) > > Add sm8350 DISPCC bindings, which are simply a symlink to the sm8250 > > bindings. Update the documentation with the new compatible. > > > > Signed-off-by: Jonathan Marek <jonathan@marek.ca> > > Reviewed-by: Rob Herring <robh@kernel.org> > > --- > > .../devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml | 6 ++++-- > > include/dt-bindings/clock/qcom,dispcc-sm8350.h | 1 + > > > 2 files changed, 5 insertions(+), 2 deletions(-) > > create mode 120000 include/dt-bindings/clock/qcom,dispcc-sm8350.h > > Why the symlink? Can we have the dt authors use the existing header file > instead? > > > > > diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml > > index 0cdf53f41f84..8f414642445e 100644 > > --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml > > +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml > > @@ -4,24 +4,26 @@ > > $id: http://devicetree.org/schemas/clock/qcom,dispcc-sm8x50.yaml# > > $schema: http://devicetree.org/meta-schemas/core.yaml# > > > > -title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250 > > +title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250/SM8350 > > Maybe just "Binding for SM8x50 SoCs" > That seems like a certain way to ensure that SM8450 etc will be different enough to not match this binding :) Regards, Bjorn
Quoting Jonathan Marek (2021-06-04 10:25:41) > On 6/2/21 5:27 PM, Stephen Boyd wrote: > > Quoting Jonathan Marek (2021-05-18 17:18:02) > >> Add sm8350 DISPCC bindings, which are simply a symlink to the sm8250 > >> bindings. Update the documentation with the new compatible. > >> > >> Signed-off-by: Jonathan Marek <jonathan@marek.ca> > >> Reviewed-by: Rob Herring <robh@kernel.org> > >> --- > >> .../devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml | 6 ++++-- > >> include/dt-bindings/clock/qcom,dispcc-sm8350.h | 1 + > > > >> 2 files changed, 5 insertions(+), 2 deletions(-) > >> create mode 120000 include/dt-bindings/clock/qcom,dispcc-sm8350.h > > > > Why the symlink? Can we have the dt authors use the existing header file > > instead? > > > > It would be strange to include bindings with the name of a different > SoC. I guess it is a matter a preference, is there any good reason to > *not* do it like this? $ find include/dt-bindings -type l include/dt-bindings/input/linux-event-codes.h include/dt-bindings/clock/qcom,dispcc-sm8150.h It seems to not be common at all. > > >> > >> diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml > >> index 0cdf53f41f84..8f414642445e 100644 > >> --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml > >> +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml > >> @@ -4,24 +4,26 @@ > >> $id: http://devicetree.org/schemas/clock/qcom,dispcc-sm8x50.yaml# > >> $schema: http://devicetree.org/meta-schemas/core.yaml# > >> > >> -title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250 > >> +title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250/SM8350 > > > > Maybe just "Binding for SM8x50 SoCs" > > > > Its likely these bindings won't be compatible with future "SM8x50" SoCs, > listing supported SoCs explicitly will avoid confusion in the future. The yaml file has sm8x50 in the name. What's the plan there?
diff --git a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml index 0cdf53f41f84..8f414642445e 100644 --- a/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml +++ b/Documentation/devicetree/bindings/clock/qcom,dispcc-sm8x50.yaml @@ -4,24 +4,26 @@ $id: http://devicetree.org/schemas/clock/qcom,dispcc-sm8x50.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250 +title: Qualcomm Display Clock & Reset Controller Binding for SM8150/SM8250/SM8350 maintainers: - Jonathan Marek <jonathan@marek.ca> description: | Qualcomm display clock control module which supports the clocks, resets and - power domains on SM8150 and SM8250. + power domains on SM8150/SM8250/SM8350. See also: dt-bindings/clock/qcom,dispcc-sm8150.h dt-bindings/clock/qcom,dispcc-sm8250.h + dt-bindings/clock/qcom,dispcc-sm8350.h properties: compatible: enum: - qcom,sm8150-dispcc - qcom,sm8250-dispcc + - qcom,sm8350-dispcc clocks: items: diff --git a/include/dt-bindings/clock/qcom,dispcc-sm8350.h b/include/dt-bindings/clock/qcom,dispcc-sm8350.h new file mode 120000 index 000000000000..0312b4544acb --- /dev/null +++ b/include/dt-bindings/clock/qcom,dispcc-sm8350.h @@ -0,0 +1 @@ +qcom,dispcc-sm8250.h \ No newline at end of file