| Message ID | 20220406002648.393486-1-dmitry.baryshkov@linaro.org |
|---|---|
| Headers | show |
| Series | arm: qcom: qcom-apq8064: add separate device node for tsens | expand |
Quoting Dmitry Baryshkov (2022-04-06 12:57:30) > On Wed, 6 Apr 2022 at 18:40, Stephen Boyd <sboyd@kernel.org> wrote: > > > > Quoting Dmitry Baryshkov (2022-04-05 17:26:44) > > > Currently gcc-msm8960 driver manually creates tsens device. Instantiate > > > the device using DT node instead. This follow the IPQ8064 device tree > > > schema. > > > > Why can't the schema be changed? > > But these commits change the schema. They make apq8064 follow more > logical scheme of ipq8064. > Sounds like ipq8064 and apq8064 follow different schemas. Is there any benefit to harmonizing the two vs. just leaving it as it is in the dts and making the schema match whatever the dts has?
On Tue, 12 Apr 2022 at 21:43, Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Dmitry Baryshkov (2022-04-06 12:57:30) > > On Wed, 6 Apr 2022 at 18:40, Stephen Boyd <sboyd@kernel.org> wrote: > > > > > > Quoting Dmitry Baryshkov (2022-04-05 17:26:44) > > > > Currently gcc-msm8960 driver manually creates tsens device. Instantiate > > > > the device using DT node instead. This follow the IPQ8064 device tree > > > > schema. > > > > > > Why can't the schema be changed? > > > > But these commits change the schema. They make apq8064 follow more > > logical scheme of ipq8064. > > > > Sounds like ipq8064 and apq8064 follow different schemas. Is there any > benefit to harmonizing the two vs. just leaving it as it is in the dts > and making the schema match whatever the dts has? I'd prefer to harmonize them. It makes no sense to have two different approaches for the single IP block (shared between ipq and apq/msm). And having a separate device tree node for the tsens removes a dependency from gcc on the nvmem/qfprom. Note, upstream qcom-msm8960.dtsi doesn't describe tsens at all, so we don't have to worry about it.
On Tue 12 Apr 12:20 PDT 2022, Dmitry Baryshkov wrote: > On Tue, 12 Apr 2022 at 21:43, Stephen Boyd <sboyd@kernel.org> wrote: > > > > Quoting Dmitry Baryshkov (2022-04-06 12:57:30) > > > On Wed, 6 Apr 2022 at 18:40, Stephen Boyd <sboyd@kernel.org> wrote: > > > > > > > > Quoting Dmitry Baryshkov (2022-04-05 17:26:44) > > > > > Currently gcc-msm8960 driver manually creates tsens device. Instantiate > > > > > the device using DT node instead. This follow the IPQ8064 device tree > > > > > schema. > > > > > > > > Why can't the schema be changed? > > > > > > But these commits change the schema. They make apq8064 follow more > > > logical scheme of ipq8064. > > > > > > > Sounds like ipq8064 and apq8064 follow different schemas. Is there any > > benefit to harmonizing the two vs. just leaving it as it is in the dts > > and making the schema match whatever the dts has? > > I'd prefer to harmonize them. It makes no sense to have two different > approaches for the single IP block (shared between ipq and apq/msm). > And having a separate device tree node for the tsens removes a > dependency from gcc on the nvmem/qfprom. > Note, upstream qcom-msm8960.dtsi doesn't describe tsens at all, so we > don't have to worry about it. > The apq8064 design was chosen in order to make the dts represent the GCC being a single hardware block, and the fact that this is a clock and a thermal driver in Linux is an implementation decision. Seems like we forgot about this decision when we introduce the ipq8064... I'm not against harmonizing the two, but I don't see any changes to Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml and the clock patch describes what happens, but not why (i.e. if it's to harmonize the implementations the commit message should say so). Regards, Bjorn
On 12/04/2022 23:08, Bjorn Andersson wrote: > On Tue 12 Apr 12:20 PDT 2022, Dmitry Baryshkov wrote: > >> On Tue, 12 Apr 2022 at 21:43, Stephen Boyd <sboyd@kernel.org> wrote: >>> >>> Quoting Dmitry Baryshkov (2022-04-06 12:57:30) >>>> On Wed, 6 Apr 2022 at 18:40, Stephen Boyd <sboyd@kernel.org> wrote: >>>>> >>>>> Quoting Dmitry Baryshkov (2022-04-05 17:26:44) >>>>>> Currently gcc-msm8960 driver manually creates tsens device. Instantiate >>>>>> the device using DT node instead. This follow the IPQ8064 device tree >>>>>> schema. >>>>> >>>>> Why can't the schema be changed? >>>> >>>> But these commits change the schema. They make apq8064 follow more >>>> logical scheme of ipq8064. >>>> >>> >>> Sounds like ipq8064 and apq8064 follow different schemas. Is there any >>> benefit to harmonizing the two vs. just leaving it as it is in the dts >>> and making the schema match whatever the dts has? >> >> I'd prefer to harmonize them. It makes no sense to have two different >> approaches for the single IP block (shared between ipq and apq/msm). >> And having a separate device tree node for the tsens removes a >> dependency from gcc on the nvmem/qfprom. >> Note, upstream qcom-msm8960.dtsi doesn't describe tsens at all, so we >> don't have to worry about it. >> > > The apq8064 design was chosen in order to make the dts represent the GCC > being a single hardware block, and the fact that this is a clock and a > thermal driver in Linux is an implementation decision. > > Seems like we forgot about this decision when we introduce the > ipq8064... > > > I'm not against harmonizing the two, but I don't see any changes to > Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml and the > clock patch describes what happens, but not why (i.e. if it's to > harmonize the implementations the commit message should say so). Nice catch. I forgot about the gcc-apq8064 schema. Will fix in the next iteration.
Currently gcc-msm8960 driver manually creates tsens device. Instantiate the device using DT node instead. This follow the IPQ8064 device tree schema. Compatibility with the previous devices trees is kept intact. Changes since v1: - populate child devices in gcc-msm8960 - add syscon to the gcc device tree node Dmitry Baryshkov (4): dt-bindings: thermal: qcom-tsens.yaml: add msm8960 compat string thermal/drivers/tsens: add compat string for the qcom,msm8960 clk: qcom: gcc-msm8960: create tsens device if there are no child nodes arm: dts: qcom-apq8064: create tsens device node .../bindings/thermal/qcom-tsens.yaml | 4 ++- arch/arm/boot/dts/qcom-apq8064.dtsi | 25 +++++++++++++------ drivers/clk/qcom/gcc-msm8960.c | 6 ++++- drivers/thermal/qcom/tsens.c | 3 +++ 4 files changed, 28 insertions(+), 10 deletions(-)