| Message ID | cover.1531384019.git.amit.kucheria@linaro.org |
|---|---|
| Headers | show |
| Series | thermal: tsens: Refactoring for TSENSv2 IP | expand |
Hi, On Thu, Jul 12, 2018 at 1:39 AM, Amit Kucheria <amit.kucheria@linaro.org> wrote: > We also split up the regmap address space into two, for the TM and SROT > registers. This was required to deal with different address offsets for the > TM and SROT registers across different SoC families. > > 8996 has two TSENS IP blocks, initialise the second one too. > > Since tsens-common.c/init_common() currently only registers one address > space, the order is important (TM before SROT). This is OK since the code > doesn't really use the SROT functionality yet. > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Tested-by: Matthias Kaehlcke <mka@chromium.org> > --- > arch/arm64/boot/dts/qcom/msm8996.dtsi | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) Reviewed-by: Douglas Anderson <dianders@chromium.org> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Thu, Jul 12, 2018 at 1:39 AM, Amit Kucheria <amit.kucheria@linaro.org> wrote: > SDM845 has two tsens blocks, one with 13 sensors and the other with 8 > sensors. It uses version 2 of the TSENS IP, so use the fallback property to > allow more common code. > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Tested-by: Matthias Kaehlcke <mka@chromium.org> > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) Reviewed-by: Douglas Anderson <dianders@chromium.org> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 12, 2018 at 02:09:04PM +0530, Amit Kucheria wrote: > We also split up the regmap address space into two, for the TM and SROT > registers. This was required to deal with different address offsets for the > TM and SROT registers across different SoC families. > > 8996 has two TSENS IP blocks, initialise the second one too. > > Since tsens-common.c/init_common() currently only registers one address > space, the order is important (TM before SROT). This is OK since the code > doesn't really use the SROT functionality yet. > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Tested-by: Matthias Kaehlcke <mka@chromium.org> > --- > arch/arm64/boot/dts/qcom/msm8996.dtsi | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi > index 8c7f9ca..688e752 100644 > --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi > @@ -459,9 +459,19 @@ > status = "disabled"; > }; > > - tsens0: thermal-sensor@4a8000 { > + tsens0: thermal-sensor@4a9000 { ~~~~~~ I suppose the address of the TM block is used here instead of the SROT address (which is lower) since SROT functionality is currently not used. Would/should this change if/when the driver uses SROT? > compatible = "qcom,msm8996-tsens"; > - reg = <0x4a8000 0x2000>; > + reg = <0x4a9000 0x1000>, /* TM */ > + <0x4a8000 0x1000>; /* SROT */ > + #qcom,sensors = <13>; > + #thermal-sensor-cells = <1>; > + }; > + > + tsens1: thermal-sensor@4ad000 { > + compatible = "qcom,msm8996-tsens"; > + reg = <0x4ad000 0x1000>, /* TM */ > + <0x4ac000 0x1000>; /* SROT */ > + #qcom,sensors = <8>; > #thermal-sensor-cells = <1>; > }; -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Tue, Jul 17, 2018 at 4:42 PM, Matthias Kaehlcke <mka@chromium.org> wrote: > On Thu, Jul 12, 2018 at 02:09:04PM +0530, Amit Kucheria wrote: >> We also split up the regmap address space into two, for the TM and SROT >> registers. This was required to deal with different address offsets for the >> TM and SROT registers across different SoC families. >> >> 8996 has two TSENS IP blocks, initialise the second one too. >> >> Since tsens-common.c/init_common() currently only registers one address >> space, the order is important (TM before SROT). This is OK since the code >> doesn't really use the SROT functionality yet. >> >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> >> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> >> Tested-by: Matthias Kaehlcke <mka@chromium.org> >> --- >> arch/arm64/boot/dts/qcom/msm8996.dtsi | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> >> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi >> index 8c7f9ca..688e752 100644 >> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi >> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi >> @@ -459,9 +459,19 @@ >> status = "disabled"; >> }; >> >> - tsens0: thermal-sensor@4a8000 { >> + tsens0: thermal-sensor@4a9000 { > ~~~~~~ > > I suppose the address of the TM block is used here instead of the SROT > address (which is lower) since SROT functionality is currently not > used. Would/should this change if/when the driver uses SROT? For device tree you're always supposed to use the address of the first "reg" listed as the unit address in the node name. It doesn't matter if it's bigger or smaller as long as it's the first one listed. The bindings indicate that the TM block should be listed as the first register. This won't change even if you start using SROT. -Doug -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 17, 2018 at 04:55:10PM -0700, Doug Anderson wrote: > Hi, > > On Tue, Jul 17, 2018 at 4:42 PM, Matthias Kaehlcke <mka@chromium.org> wrote: > > On Thu, Jul 12, 2018 at 02:09:04PM +0530, Amit Kucheria wrote: > >> We also split up the regmap address space into two, for the TM and SROT > >> registers. This was required to deal with different address offsets for the > >> TM and SROT registers across different SoC families. > >> > >> 8996 has two TSENS IP blocks, initialise the second one too. > >> > >> Since tsens-common.c/init_common() currently only registers one address > >> space, the order is important (TM before SROT). This is OK since the code > >> doesn't really use the SROT functionality yet. > >> > >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > >> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > >> Tested-by: Matthias Kaehlcke <mka@chromium.org> > >> --- > >> arch/arm64/boot/dts/qcom/msm8996.dtsi | 14 ++++++++++++-- > >> 1 file changed, 12 insertions(+), 2 deletions(-) > >> > >> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi > >> index 8c7f9ca..688e752 100644 > >> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi > >> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi > >> @@ -459,9 +459,19 @@ > >> status = "disabled"; > >> }; > >> > >> - tsens0: thermal-sensor@4a8000 { > >> + tsens0: thermal-sensor@4a9000 { > > ~~~~~~ > > > > I suppose the address of the TM block is used here instead of the SROT > > address (which is lower) since SROT functionality is currently not > > used. Would/should this change if/when the driver uses SROT? > > For device tree you're always supposed to use the address of the first > "reg" listed as the unit address in the node name. It doesn't matter > if it's bigger or smaller as long as it's the first one listed. > > The bindings indicate that the TM block should be listed as the first > register. This won't change even if you start using SROT. Thanks for the clarification, there is always something more to learn! Reviewed-by: Matthias Kaehlcke <mka@chromium.org> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 12, 2018 at 02:09:08PM +0530, Amit Kucheria wrote: > SDM845 has two tsens blocks, one with 13 sensors and the other with 8 > sensors. It uses version 2 of the TSENS IP, so use the fallback property to > allow more common code. > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > Tested-by: Matthias Kaehlcke <mka@chromium.org> > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index cdaabeb..01ff146 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -221,6 +221,22 @@ > #interrupt-cells = <2>; > }; > > + tsens0: thermal-sensor@c263000 { > + compatible = "qcom,sdm845-tsens", "qcom,tsens-v2"; > + reg = <0xc263000 0x1ff>, /* TM */ > + <0xc222000 0x1ff>; /* SROT */ > + #qcom,sensors = <13>; > + #thermal-sensor-cells = <1>; > + }; > + > + tsens1: thermal-sensor@c265000 { > + compatible = "qcom,sdm845-tsens", "qcom,tsens-v2"; > + reg = <0xc265000 0x1ff>, /* TM */ > + <0xc223000 0x1ff>; /* SROT */ > + #qcom,sensors = <8>; > + #thermal-sensor-cells = <1>; > + }; > + > spmi_bus: spmi@c440000 { > compatible = "qcom,spmi-pmic-arb"; > reg = <0xc440000 0x1100>, Reviewed-by: Matthias Kaehlcke <mka@chromium.org> -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
This series is a mixed bag: - Some code moves to allow code sharing between different SoCs with v2 of the TSENS IP, - a generic qcom,tsens-v2 property as a fallback compatible for all v2.x.y platforms, - new platform support (sdm845) - a cleanup patch and - a DT change to have a common way to deal with the SROT and TM registers despite slightly different features across the IP family and different register offsets. Changes since v6: - Fix comments and patch descriptions as per Doug's review - Rename tsens to thermal-sensor in DT - Add various review tags Changes since v5: - Actually fix unit addressses for the two tsens blocks as per Stephen's comment. Changes since v4: - Revert back to a single fallback bindind qcom,tsens-v2 as per Rob's suggestion. - Rework how old (unsplit SROT and TM address space) DTs are handled by needing a 0x1000 offset but still sharing common code in tsens-v2.c - Remove the patch to added TRDY checks while we investigate Matthias' reports - Fix unit addressses for the two tsens blocks as per Stephen's comment. Changes since v3: - Introduce qcom,tsens-v2.4.0 property and make qcom,tsens-v2 a fallback, compatible property. - Rename ops_v2 to ops_generic_v2 Changes since v2: - Based on review, moved tsens-8996.c to tsens-v2.c and changed corresponding function names, struct names to allow for generic tsensv2 platforms - All v2 platforms will now only need to use the qcom,tsen-v2 property - Added a DT patch to initialize tsens driver on sdm845, now that 4.18-rc1 will contain an sdm845.dtsi Changes since v1: - Move get_temp() from tsens-8996 to tsens-common and rename - Change 8996 DT entry to allow init_common() to work across sdm845 and 8996 due to different offsets Amit Kucheria (7): thermal: tsens: Get rid of unused fields in structure thermal: tsens: Add support to split up register address space into two arm64: dts: msm8996: thermal: Initialise via DT and add second controller thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse dt: thermal: tsens: Document the fallback DT property for v2 of TSENS IP thermal: tsens: Add generic support for TSENS v2 IP arm64: dts: sdm845: Add tsens nodes .../devicetree/bindings/thermal/qcom-tsens.txt | 31 +++++++++++++++++---- arch/arm64/boot/dts/qcom/msm8996.dtsi | 14 ++++++++-- arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 +++++++++++ drivers/thermal/qcom/Makefile | 2 +- drivers/thermal/qcom/tsens-common.c | 12 ++++++++ drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} | 32 ++++++++++------------ drivers/thermal/qcom/tsens.c | 3 ++ drivers/thermal/qcom/tsens.h | 8 ++++-- 8 files changed, 88 insertions(+), 30 deletions(-) rename drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} (64%)