| Message ID | cover.1530533998.git.amit.kucheria@linaro.org |
|---|---|
| Headers | show |
| Series | thermal: tsens: Refactoring for TSENSv2 IP | expand |
On Mon, Jul 02, 2018 at 06:14:08PM +0530, Amit Kucheria wrote: Empty description, cannot apply. Please describe your patch. Thanks > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon 02 Jul 05:44 PDT 2018, Amit Kucheria wrote: > SDM845 has two tsens blocks, one with 13 sensors and the other with 8 > sensors. > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > --- > 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..edf8505 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: tsens@c222000 { > + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2"; > + reg = <0xc263000 0x1ff>, /* TM */ > + <0xc222000 0x1ff>; /* SROT */ > + #qcom,sensors = <13>; > + #thermal-sensor-cells = <1>; > + }; > + > + tsens1: tsens@c223000 { > + compatible = "qcom,tsens-v2.4.0", "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>, > -- > 2.7.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon 02 Jul 05:44 PDT 2018, Amit Kucheria wrote: > SDM845 uses v2.4.0 of the TSENS IP block but the get_temp() function > appears to be identical across v2.x.y in code seen so far. We use the > generic get_temp() function. > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> Regards, Bjorn > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > --- > Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 2 ++ > drivers/thermal/qcom/tsens-v2.c | 6 +++++- > drivers/thermal/qcom/tsens.c | 6 ++++++ > drivers/thermal/qcom/tsens.h | 5 ++++- > 4 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt > index 06195e8..075182e 100644 > --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt > +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt > @@ -5,6 +5,8 @@ Required properties: > - "qcom,msm8916-tsens" : For 8916 Family of SoCs > - "qcom,msm8974-tsens" : For 8974 Family of SoCs > - "qcom,msm8996-tsens" : For 8996 Family of SoCs > + - "qcom,tsens-v2.4.0" : For SDM845 Family of SoCs > + - "qcom,tsens-v2" : Generic fallback binding for any Soc using 2.x.y version of the tsens IP > > - reg: Address range of the thermal registers > - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description. > diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c > index 2eca7ff..1871945 100644 > --- a/drivers/thermal/qcom/tsens-v2.c > +++ b/drivers/thermal/qcom/tsens-v2.c > @@ -69,8 +69,12 @@ static const struct tsens_ops ops_generic_v2 = { > .get_temp = get_temp_tsens_v2, > }; > > +const struct tsens_data data_tsens_v2 = { > + .ops = &ops_generic_v2, > +}; > + > +/* Kept around for backward compatibility with old msm8996.dtsi */ > const struct tsens_data data_8996 = { > .num_sensors = 13, > .ops = &ops_generic_v2, > }; > - > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c > index 3440166c..992ed17 100644 > --- a/drivers/thermal/qcom/tsens.c > +++ b/drivers/thermal/qcom/tsens.c > @@ -72,6 +72,12 @@ static const struct of_device_id tsens_table[] = { > }, { > .compatible = "qcom,msm8996-tsens", > .data = &data_8996, > + }, { > + .compatible = "qcom,tsens-v2.4.0", > + .data = &data_tsens_v2, > + }, { > + .compatible = "qcom,tsens-v2", > + .data = &data_tsens_v2, > }, > {} > }; > diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h > index dc56e1e..69212cb 100644 > --- a/drivers/thermal/qcom/tsens.h > +++ b/drivers/thermal/qcom/tsens.h > @@ -87,6 +87,9 @@ void compute_intercept_slope(struct tsens_device *, u32 *, u32 *, u32); > int init_common(struct tsens_device *); > int get_temp_common(struct tsens_device *, int, int *); > > -extern const struct tsens_data data_8916, data_8974, data_8960, data_8996; > +/* TSENS v1 targets */ > +extern const struct tsens_data data_8916, data_8974, data_8960; > +/* TSENS v2 targets */ > +extern const struct tsens_data data_8996, data_tsens_v2; > > #endif /* __QCOM_TSENS_H__ */ > -- > 2.7.4 > -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 02, 2018 at 06:14:07PM +0530, Amit Kucheria wrote: > SDM845 uses v2.4.0 of the TSENS IP block but the get_temp() function > appears to be identical across v2.x.y in code seen so far. We use the > generic get_temp() function. > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > --- > Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 2 ++ > drivers/thermal/qcom/tsens-v2.c | 6 +++++- > drivers/thermal/qcom/tsens.c | 6 ++++++ > drivers/thermal/qcom/tsens.h | 5 ++++- > 4 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt > index 06195e8..075182e 100644 > --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt > +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt > @@ -5,6 +5,8 @@ Required properties: > - "qcom,msm8916-tsens" : For 8916 Family of SoCs > - "qcom,msm8974-tsens" : For 8974 Family of SoCs > - "qcom,msm8996-tsens" : For 8996 Family of SoCs > + - "qcom,tsens-v2.4.0" : For SDM845 Family of SoCs > + - "qcom,tsens-v2" : Generic fallback binding for any Soc using 2.x.y version of the tsens IP You need to show what are valid combinations of compatibles. Does v2 apply to 8996? One valid combination per line. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoting Amit Kucheria (2018-07-02 05:44:09) > SDM845 has two tsens blocks, one with 13 sensors and the other with 8 > sensors. > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.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..edf8505 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: tsens@c222000 { These unit addresses should match the first reg property, not the second one. > + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2"; > + reg = <0xc263000 0x1ff>, /* TM */ > + <0xc222000 0x1ff>; /* SROT */ > + #qcom,sensors = <13>; > + #thermal-sensor-cells = <1>; > + }; > + > + tsens1: tsens@c223000 { > + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2"; > + reg = <0xc265000 0x1ff>, /* TM */ > + <0xc223000 0x1ff>; /* SROT */ > + #qcom,sensors = <8>; -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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 5, 2018 at 11:13 PM Amit Kucheria <amit.kucheria@linaro.org> wrote: > > On Fri, Jul 6, 2018 at 2:07 AM Rob Herring <robh@kernel.org> wrote: > > > > On Wed, Jul 04, 2018 at 10:56:26PM +0530, Amit Kucheria wrote: > > > On Tue, Jul 3, 2018 at 9:56 PM, Rob Herring <robh@kernel.org> wrote: > > > > On Mon, Jul 02, 2018 at 06:14:07PM +0530, Amit Kucheria wrote: > > > >> SDM845 uses v2.4.0 of the TSENS IP block but the get_temp() function > > > >> appears to be identical across v2.x.y in code seen so far. We use the > > > >> generic get_temp() function. > > > >> > > > >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org> > > > >> --- > > > >> Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 2 ++ > > > >> drivers/thermal/qcom/tsens-v2.c | 6 +++++- > > > >> drivers/thermal/qcom/tsens.c | 6 ++++++ > > > >> drivers/thermal/qcom/tsens.h | 5 ++++- > > > >> 4 files changed, 17 insertions(+), 2 deletions(-) > > > >> > > > >> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt > > > >> index 06195e8..075182e 100644 > > > >> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt > > > >> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt > > > >> @@ -5,6 +5,8 @@ Required properties: > > > >> - "qcom,msm8916-tsens" : For 8916 Family of SoCs > > > >> - "qcom,msm8974-tsens" : For 8974 Family of SoCs > > > >> - "qcom,msm8996-tsens" : For 8996 Family of SoCs > > > >> + - "qcom,tsens-v2.4.0" : For SDM845 Family of SoCs > > > >> + - "qcom,tsens-v2" : Generic fallback binding for any Soc using 2.x.y version of the tsens IP > > > > > > > > You need to show what are valid combinations of compatibles. Does v2 > > > > apply to 8996? One valid combination per line. > > > > > > I've restructured qcom-tsens.txt to look like this: > > > > > > -----%<------- > > > > > > * QCOM SoC Temperature Sensor (TSENS) > > > > > > Required properties: > > > - compatible: must be one of the following: > > > - "qcom,msm8916-tsens" (MSM8916) > > > - "qcom,msm8974-tsens" (MSM8974) > > > - "qcom,msm8996-tsens" (MSM8996) > > > - "qcom,tsens-<ip_version>", "qcom,tsens-v2" (TSENS IP version and a > > > generic v2 property as fallback except for MSM8996) > > > > > > Examples with ip_version are: > > > - "qcom,tsens-v2.4.0", "qcom,tsens-v2" (SDM845) > > > - "qcom,tsens-v2.2.1", "qcom,tsens-v2" (MSM8998) > > > > > > -----%<------- > > > > > > 8996 would end up being something like this if needed, though we're > > > stuck with "qcom,msm8996-tsens": > > > "qcom,msm8996-tsens", "qcom,tsens-v2.1.0", "qcom,tsens-v2" (MSM8996) > > > > 3 versions here for 3 SoCs. I'm not getting that convinced version > > numbers really are better. I would assume that other QCom IP blocks > > Yeah, it is a bit unfortunate that the 3-4 SoCs we're focusing on > getting supported upstream have different versions of the TSENS IP. > The other goal of this work was to make the upstream driver > feature-complete so we can make a case to switch to it in the > downstream trees, even on platforms that aren't being active > upstreamed. They'll still need to keep around those SoC-specific > one-liner patches in the downstream trees. Why? They can just use the "v2" fallback (which was sufficient for your original version). That's not my recommendation, but what do I care for downstream. Only upstream needs the specific strings to appease the annoying DT maintainers. Plus, if it's not upstream, it doesn't exist. :) > > have versions too, but pretty much *every* *other* binding uses SoC names. > > Why is this one special? > > I'm not an expert on all QC IPs, but this one _seems_ to be reused a > lot more than others. > > > The other problem with versions is the mapping > > of versions to SoCs most likely can't be validated outside of QCom > > unless there's a version register. > > There is in fact a HW version register that I was hoping to add > support for later. Yes, you should rely on that as much as possible. But I have seen h/w designers forget to update revision registers or there can be integration differences even if the IP version is the same. > > So, sorry to go in circles, but can you go back to qcom,<soc>-tsens. You > > can keep qcom,tsens-v2 as a fallback. > > OK. > > > Yes, it's annoying to have to update bindings for new SoCs. But it's > > trivial one line patches. Look at Renesas bindings. Maybe adding new > > ones will be scriptable once we move to json-schema binding docs. > > I did look at the Renesas RCar bindings when restructuring the > documentation. They seem to have settled upon a 3-level fallback: > "soc-specific", "generic-no TZ", "generic-TZ". But > drivers/thermal/rcar_thermal.c seems to be compatible with only one > soc-specific property (thermal-r8a77995) and all other SoCs seem to be > just relying the fallbacks. > > Anyways, I'll respin. > > Thanks. > > Regards, > Amit -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jul 9, 2018 at 9:31 PM, Matthias Kaehlcke <mka@chromium.org> wrote: > On Mon, Jul 09, 2018 at 04:56:51PM +0530, Amit Kucheria wrote: >> On Wed, Jul 4, 2018 at 3:18 AM, Matthias Kaehlcke <mka@chromium.org> wrote: >> > Hi, >> > >> > On Mon, Jul 02, 2018 at 06:14:09PM +0530, Amit Kucheria wrote: >> >> SDM845 has two tsens blocks, one with 13 sensors and the other with 8 >> >> sensors. >> >> >> >> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.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..edf8505 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: tsens@c222000 { >> >> + compatible = "qcom,tsens-v2.4.0", "qcom,tsens-v2"; >> >> + reg = <0xc263000 0x1ff>, /* TM */ >> >> + <0xc222000 0x1ff>; /* SROT */ >> >> + #qcom,sensors = <13>; >> >> + #thermal-sensor-cells = <1>; >> >> + }; >> >> + >> >> + tsens1: tsens@c223000 { >> >> + compatible = "qcom,tsens-v2.4.0", "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>, >> > >> > I tested this series on an SDM845 board. Temperatures can be read from >> > the tsens0 sensors, however for tsens1 sensors I get: >> > >> > cat temp >> > cat: temp: No data available >> > >> > TRDY_READY_BIT set when reading TRDY_OFFSET in get_temp_tsens_v2(). >> > Unfortunately I currently have no documentation of the registers or the >> > thermal sensors in general and couldn't investigate further. >> > >> > Any idea what could be the problem? >> >> Strangely enough, I'm seeing this now after rebuilding my integration >> branch to test some other bits. >> >> I've sent another version with this patch removed while I investigate >> this. Could you check if you're able to read all 8 sensors with v5? > > With v6 all sensors can be read, thanks! Great! Would it be OK to add your Tested-by tag? -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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 various v2.x.y versions of the TSENS IP, - new qcom,tsens-v2.4.0 DT property for SDM845 and 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. Rob mentioned offline that we should expose the full version string of the TSENS IP (x.y.z) and have a fallback compatible. I hope patch 4 does what you were looking for. Regards, Amit Amit Kucheria (6): thermal: tsens: Get rid of unused fields in structure dt: qcom: 8996: thermal: Move to DT initialisation thermal: tsens: Rename tsens-8996 to tsens-v2 for reuse thermal: tsens: Add support for SDM845 thermal: tsens: Check if we have valid data before reading arm64: dts: sdm845: Add tsens nodes .../devicetree/bindings/thermal/qcom-tsens.txt | 2 ++ arch/arm64/boot/dts/qcom/msm8996.dtsi | 12 ++++++- arch/arm64/boot/dts/qcom/sdm845.dtsi | 16 +++++++++ drivers/thermal/qcom/Makefile | 2 +- drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} | 39 ++++++++++++---------- drivers/thermal/qcom/tsens.c | 6 ++++ drivers/thermal/qcom/tsens.h | 7 ++-- 7 files changed, 62 insertions(+), 22 deletions(-) rename drivers/thermal/qcom/{tsens-8996.c => tsens-v2.c} (66%) -- 2.7.4 -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html