[PATCH 0/3] riscv: sophgo: add thermal sensor support for cv180x/sg200x SoCs

[PATCH 0/3] riscv: sophgo: add thermal sensor support for cv180x/sg200x SoCs

[Haylen Chu]

This series implements driver for Sophgo cv180x/sg200x on-chip thermal
sensor and adds common thermal zones for these SoCs.

Haylen Chu (3):
  dt-bindings: thermal: sophgo,cv180x-thermal: Add Sophgo CV180x thermal
  riscv: dts: sophgo: cv18xx: Add sensor device and thermal zone
  thermal: cv180x: Add cv180x thermal driver support

 .../thermal/sophgo,cv180x-thermal.yaml        |  46 ++++
 arch/riscv/boot/dts/sophgo/cv18xx.dtsi        |  36 +++
 drivers/thermal/Kconfig                       |   6 +
 drivers/thermal/Makefile                      |   1 +
 drivers/thermal/cv180x_thermal.c              | 210 ++++++++++++++++++
 5 files changed, 299 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml
 create mode 100644 drivers/thermal/cv180x_thermal.c

-- 
2.45.1

[PATCH 1/3] dt-bindings: thermal: sophgo,cv180x-thermal: Add Sophgo CV180x thermal

[Haylen Chu]

Add devicetree binding documentation for thermal sensors integrated in
Sophgo CV180X SoCs.

Signed-off-by: Haylen Chu <heylenay@outlook.com>
---
 .../thermal/sophgo,cv180x-thermal.yaml        | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml

diff --git a/Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml b/Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml
new file mode 100644
index 000000000000..0364ae6c1055
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/sophgo,cv180x-thermal.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo CV180x on-SoC Thermal Sensor
+
+maintainers:
+  - Haylen Chu <heylenay@outlook.com>
+
+description: Binding for Sophgo CV180x on-SoC thermal sensor
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - sophgo,cv180x-thermal
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  '#thermal-sensor-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+        #include <dt-bindings/clock/sophgo,cv1800.h>
+        soc_temp: temperature_sensor@30e0000 {
+                compatible = "sophgo,cv180x-thermal";
+                reg = <0x30e0000 0x100>;
+                clocks = <&clk CLK_TEMPSEN>;
+                clock-names = "clk_tempsen";
+                #thermal-sensor-cells = <0>;
+        };
+...
-- 
2.45.1

Re: [PATCH 1/3] dt-bindings: thermal: sophgo,cv180x-thermal: Add Sophgo CV180x thermal

[Rob Herring (Arm)]

On Thu, 30 May 2024 13:48:25 +0000, Haylen Chu wrote:
> Add devicetree binding documentation for thermal sensors integrated in
> Sophgo CV180X SoCs.
> 
> Signed-off-by: Haylen Chu <heylenay@outlook.com>
> ---
>  .../thermal/sophgo,cv180x-thermal.yaml        | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.example.dtb: temperature_sensor@30e0000: 'clock-names' does not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/thermal/sophgo,cv180x-thermal.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/SEYPR01MB4221BD44992A23E2B0061023D7F32@SEYPR01MB4221.apcprd01.prod.exchangelabs.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/3] dt-bindings: thermal: sophgo,cv180x-thermal: Add Sophgo CV180x thermal

[Krzysztof Kozlowski]

On 30/05/2024 15:48, Haylen Chu wrote:
> Add devicetree binding documentation for thermal sensors integrated in
> Sophgo CV180X SoCs.
> 
> Signed-off-by: Haylen Chu <heylenay@outlook.com>
> ---
>  .../thermal/sophgo,cv180x-thermal.yaml        | 46 +++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml
> 
> diff --git a/Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml b/Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml
> new file mode 100644
> index 000000000000..0364ae6c1055
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml
> @@ -0,0 +1,46 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/thermal/sophgo,cv180x-thermal.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sophgo CV180x on-SoC Thermal Sensor
> +
> +maintainers:
> +  - Haylen Chu <heylenay@outlook.com>
> +
> +description: Binding for Sophgo CV180x on-SoC thermal sensor
> +
> +properties:
> +  compatible:
> +    items:

Drop items

> +      - enum:
> +          - sophgo,cv180x-thermal
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  '#thermal-sensor-cells':
> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +        #include <dt-bindings/clock/sophgo,cv1800.h>

Use 4 spaces for example indentation.

> +        soc_temp: temperature_sensor@30e0000 {

No underscores, but hyphens. Drop label.

> +                compatible = "sophgo,cv180x-thermal";
> +                reg = <0x30e0000 0x100>;
> +                clocks = <&clk CLK_TEMPSEN>;
> +                clock-names = "clk_tempsen";

You did not bother to test it, right?


Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: thermal: sophgo,cv180x-thermal: Add Sophgo CV180x thermal

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1472 bytes --]

On Fri, May 31, 2024 at 10:54:24AM +0200, Krzysztof Kozlowski wrote:
> On 30/05/2024 15:48, Haylen Chu wrote:
> > Add devicetree binding documentation for thermal sensors integrated in
> > Sophgo CV180X SoCs.
> > 
> > Signed-off-by: Haylen Chu <heylenay@outlook.com>
> > ---
> >  .../thermal/sophgo,cv180x-thermal.yaml        | 46 +++++++++++++++++++
> >  1 file changed, 46 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml b/Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml
> > new file mode 100644
> > index 000000000000..0364ae6c1055
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/thermal/sophgo,cv180x-thermal.yaml
> > @@ -0,0 +1,46 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/thermal/sophgo,cv180x-thermal.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Sophgo CV180x on-SoC Thermal Sensor
> > +
> > +maintainers:
> > +  - Haylen Chu <heylenay@outlook.com>
> > +
> > +description: Binding for Sophgo CV180x on-SoC thermal sensor
> > +
> > +properties:
> > +  compatible:
> > +    items:
> 
> Drop items
> 
> > +      - enum:
> > +          - sophgo,cv180x-thermal

And a soc-specific compatible too please.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 2/3] riscv: dts: sophgo: cv18xx: Add sensor device and thermal zone

[Haylen Chu]

Add common sensor device and thermal zones for Sophgo CV18xx SoCs.

Signed-off-by: Haylen Chu <heylenay@outlook.com>
---
 arch/riscv/boot/dts/sophgo/cv18xx.dtsi | 36 ++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
index 891932ae470f..dfb4bb6eb319 100644
--- a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
+++ b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
@@ -310,5 +310,41 @@ clint: timer@74000000 {
 			reg = <0x74000000 0x10000>;
 			interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
 		};
+
+		soc_temp: thermal-sensor@30e0000 {
+			compatible = "sophgo,cv180x-thermal";
+			reg = <0x30e0000 0x100>;
+			clocks = <&clk CLK_TEMPSEN>;
+			clock-names = "clk_tempsen";
+			#thermal-sensor-cells = <0>;
+		};
+	};
+
+	thermal-zones {
+		soc-thermal-0 {
+			polling-delay-passive	= <1000>;
+			polling-delay		= <1000>;
+			thermal-sensors		= <&soc_temp>;
+
+			trips {
+				soc_passive: soc-passive {
+					temperature	= <75000>;
+					hysteresis	= <5000>;
+					type		= "passive";
+				};
+
+				soc_hot: soc-hot {
+					temperature	= <85000>;
+					hysteresis	= <5000>;
+					type		= "hot";
+				};
+
+				soc_critical: soc-critical {
+					temperature	= <100000>;
+					hysteresis	= <0>;
+					type		= "critical";
+				};
+			};
+		};
 	};
 };
-- 
2.45.1

Re: [PATCH 2/3] riscv: dts: sophgo: cv18xx: Add sensor device and thermal zone

[Inochi Amaoto]

On Thu, May 30, 2024 at 01:48:26PM GMT, Haylen Chu wrote:
> Add common sensor device and thermal zones for Sophgo CV18xx SoCs.
> 
> Signed-off-by: Haylen Chu <heylenay@outlook.com>
> ---
>  arch/riscv/boot/dts/sophgo/cv18xx.dtsi | 36 ++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
> index 891932ae470f..dfb4bb6eb319 100644
> --- a/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
> +++ b/arch/riscv/boot/dts/sophgo/cv18xx.dtsi
> @@ -310,5 +310,41 @@ clint: timer@74000000 {
>  			reg = <0x74000000 0x10000>;
>  			interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
>  		};
> +
> +		soc_temp: thermal-sensor@30e0000 {
> +			compatible = "sophgo,cv180x-thermal";
> +			reg = <0x30e0000 0x100>;
> +			clocks = <&clk CLK_TEMPSEN>;
> +			clock-names = "clk_tempsen";
> +			#thermal-sensor-cells = <0>;
> +		};
> +	};
> +

> +	thermal-zones {
> +		soc-thermal-0 {
> +			polling-delay-passive	= <1000>;
> +			polling-delay		= <1000>;
> +			thermal-sensors		= <&soc_temp>;
> +
> +			trips {
> +				soc_passive: soc-passive {
> +					temperature	= <75000>;
> +					hysteresis	= <5000>;
> +					type		= "passive";
> +				};
> +
> +				soc_hot: soc-hot {
> +					temperature	= <85000>;
> +					hysteresis	= <5000>;
> +					type		= "hot";
> +				};
> +
> +				soc_critical: soc-critical {
> +					temperature	= <100000>;
> +					hysteresis	= <0>;
> +					type		= "critical";
> +				};
> +			};
> +		};
>  	};

Move this to the cpu specific file. Different cpu should have different
thermal-zones.

>  };
> -- 
> 2.45.1
> 

[PATCH 3/3] thermal: cv180x: Add cv180x thermal driver support

[Haylen Chu]

Add support for cv180x SoCs integrated thermal sensors.

Signed-off-by: Haylen Chu <heylenay@outlook.com>
---
 drivers/thermal/Kconfig          |   6 +
 drivers/thermal/Makefile         |   1 +
 drivers/thermal/cv180x_thermal.c | 210 +++++++++++++++++++++++++++++++
 3 files changed, 217 insertions(+)
 create mode 100644 drivers/thermal/cv180x_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 204ed89a3ec9..f53c973a361d 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -514,4 +514,10 @@ config LOONGSON2_THERMAL
 	  is higher than the high temperature threshold or lower than the low
 	  temperature threshold, the interrupt will occur.
 
+config CV180X_THERMAL
+	tristate "Temperature sensor driver for Sophgo CV180X"
+	help
+	  If you say yes here you get support for thermal sensor integrated in
+	  Sophgo CV180X SoCs.
+
 endif
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 5cdf7d68687f..5b59bde8a579 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -65,3 +65,4 @@ obj-$(CONFIG_AMLOGIC_THERMAL)     += amlogic_thermal.o
 obj-$(CONFIG_SPRD_THERMAL)	+= sprd_thermal.o
 obj-$(CONFIG_KHADAS_MCU_FAN_THERMAL)	+= khadas_mcu_fan.o
 obj-$(CONFIG_LOONGSON2_THERMAL)	+= loongson2_thermal.o
+obj-$(CONFIG_CV180X_THERMAL)	+= cv180x_thermal.o
diff --git a/drivers/thermal/cv180x_thermal.c b/drivers/thermal/cv180x_thermal.c
new file mode 100644
index 000000000000..618e031b4515
--- /dev/null
+++ b/drivers/thermal/cv180x_thermal.c
@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2021 CVITEK Inc.
+ * Copyright (C) 2024 Haylen Chu <heylenay@outlook.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/io.h>
+#include <linux/thermal.h>
+
+#define TEMPSEN_VERSION					0x0
+#define TEMPSEN_CTRL					0x4
+#define TEMPSEN_CTRL_EN					BIT(0)
+#define TEMPSEN_CTRL_SEL_MASK				GENMASK(7, 4)
+#define TEMPSEN_CTRL_SEL_OFFSET				4
+#define TEMPSEN_STATUS					0x8
+#define TEMPSEN_SET					0xc
+#define TEMPSEN_SET_CHOPSEL_MASK			GENMASK(5, 4)
+#define TEMPSEN_SET_CHOPSEL_OFFSET			4
+#define TEMPSEN_SET_CHOPSEL_128T			0
+#define TEMPSEN_SET_CHOPSEL_256T			1
+#define TEMPSEN_SET_CHOPSEL_512T			2
+#define TEMPSEN_SET_CHOPSEL_1024T			3
+#define TEMPSEN_SET_ACCSEL_MASK				GENMASK(7, 6)
+#define TEMPSEN_SET_ACCSEL_OFFSET			6
+#define TEMPSEN_SET_ACCSEL_512T				0
+#define TEMPSEN_SET_ACCSEL_1024T			1
+#define TEMPSEN_SET_ACCSEL_2048T			2
+#define TEMPSEN_SET_ACCSEL_4096T			3
+#define TEMPSEN_SET_CYC_CLKDIV_MASK			GENMASK(15, 8)
+#define TEMPSEN_SET_CYC_CLKDIV_OFFSET			8
+#define TEMPSEN_INTR_EN					0x10
+#define TEMPSEN_INTR_CLR				0x14
+#define TEMPSEN_INTR_STA				0x18
+#define TEMPSEN_INTR_RAW				0x1c
+#define TEMPSEN_RESULT(n)				(0x20 + (n) * 4)
+#define TEMPSEN_RESULT_RESULT_MASK			GENMASK(12, 0)
+#define TEMPSEN_RESULT_MAX_RESULT_MASK			GENMASK(28, 16)
+#define TEMPSEN_RESULT_CLR_MAX_RESULT			BIT(31)
+#define TEMPSEN_AUTO_PERIOD				0x64
+#define TEMPSEN_AUTO_PERIOD_AUTO_CYCLE_MASK		GENMASK(23, 0)
+#define TEMPSEN_AUTO_PERIOD_AUTO_CYCLE_OFFSET		0
+
+struct cv180x_thermal_zone {
+	struct device *dev;
+	void __iomem *base;
+	struct clk *clk_tempsen;
+};
+
+static void cv180x_thermal_init(struct cv180x_thermal_zone *ctz)
+{
+	void __iomem *base = ctz->base;
+	u32 regval;
+
+	writel(readl(base + TEMPSEN_INTR_RAW), base + TEMPSEN_INTR_CLR);
+	writel(TEMPSEN_RESULT_CLR_MAX_RESULT, base + TEMPSEN_RESULT(0));
+
+	regval = readl(base + TEMPSEN_SET);
+	regval &= ~TEMPSEN_SET_CHOPSEL_MASK;
+	regval &= ~TEMPSEN_SET_ACCSEL_MASK;
+	regval &= ~TEMPSEN_SET_CYC_CLKDIV_MASK;
+	regval |= TEMPSEN_SET_CHOPSEL_1024T << TEMPSEN_SET_CHOPSEL_OFFSET;
+	regval |= TEMPSEN_SET_ACCSEL_2048T  << TEMPSEN_SET_ACCSEL_OFFSET;
+	regval |= 0x31 << TEMPSEN_SET_CYC_CLKDIV_OFFSET;
+	writel(regval, base + TEMPSEN_SET);
+
+	regval = readl(base + TEMPSEN_AUTO_PERIOD);
+	regval &= ~TEMPSEN_AUTO_PERIOD_AUTO_CYCLE_MASK;
+	regval |= 0x100000 << TEMPSEN_AUTO_PERIOD_AUTO_CYCLE_OFFSET;
+	writel(regval, base + TEMPSEN_AUTO_PERIOD);
+
+	regval = readl(base + TEMPSEN_CTRL);
+	regval &= ~TEMPSEN_CTRL_SEL_MASK;
+	regval |= 1 << TEMPSEN_CTRL_SEL_OFFSET;
+	regval |= TEMPSEN_CTRL_EN;
+	writel(regval, base + TEMPSEN_CTRL);
+}
+
+static void cv180x_thermal_deinit(struct cv180x_thermal_zone *ct)
+{
+	void __iomem *base = ct->base;
+	u32 regval;
+
+	regval = readl(base + TEMPSEN_CTRL);
+	regval &= ~(TEMPSEN_CTRL_SEL_MASK | TEMPSEN_CTRL_EN);
+	writel(regval, base + TEMPSEN_CTRL);
+
+	writel(readl(base + TEMPSEN_INTR_RAW), base + TEMPSEN_INTR_CLR);
+}
+
+static int calc_temp(uint32_t result)
+{
+	return ((result * 1000) * 716 / 2048 - 273000);
+}
+
+static int cv180x_get_temp(struct thermal_zone_device *tdev, int *temperature)
+{
+	struct cv180x_thermal_zone *ctz = thermal_zone_device_priv(tdev);
+	void __iomem *base = ctz->base;
+	u32 result;
+
+	result = readl(base + TEMPSEN_RESULT(0)) & TEMPSEN_RESULT_RESULT_MASK;
+	*temperature = calc_temp(result);
+
+	return 0;
+}
+
+static const struct thermal_zone_device_ops cv180x_thermal_ops = {
+	.get_temp = cv180x_get_temp,
+};
+
+static const struct of_device_id cv180x_thermal_of_match[] = {
+	{
+		.compatible = "sophgo,cv180x-thermal",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, cv180x_thermal_of_match);
+
+static int cv180x_thermal_probe(struct platform_device *pdev)
+{
+	struct cv180x_thermal_zone *ctz;
+	struct thermal_zone_device *tz;
+	struct resource *res;
+
+	ctz = devm_kzalloc(&pdev->dev, sizeof(*ctz), GFP_KERNEL);
+	if (!ctz)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ctz->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(ctz->base))
+		return dev_err_probe(&pdev->dev, PTR_ERR(ctz->base),
+				     "failed to map tempsen registers\n");
+
+	ctz->clk_tempsen = devm_clk_get(&pdev->dev, "clk_tempsen");
+	if (IS_ERR(ctz->clk_tempsen))
+		return dev_err_probe(&pdev->dev, PTR_ERR(ctz->clk_tempsen),
+				     "failed to get clk_tempsen\n");
+
+	clk_prepare_enable(ctz->clk_tempsen);
+
+	ctz->dev = &pdev->dev;
+
+	cv180x_thermal_init(ctz);
+
+	platform_set_drvdata(pdev, ctz);
+
+	tz = devm_thermal_of_zone_register(&pdev->dev, 0, ctz,
+					   &cv180x_thermal_ops);
+	if (IS_ERR(tz))
+		return dev_err_probe(&pdev->dev, PTR_ERR(tz),
+				     "failed to register thermal zone\n");
+
+	return 0;
+}
+
+static int cv180x_thermal_remove(struct platform_device *pdev)
+{
+	struct cv180x_thermal_zone *ctz = platform_get_drvdata(pdev);
+
+	cv180x_thermal_deinit(ctz);
+	clk_disable_unprepare(ctz->clk_tempsen);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int cv180x_thermal_suspend(struct device *dev)
+{
+	struct cv180x_thermal_zone *ctz = dev_get_drvdata(dev);
+
+	cv180x_thermal_deinit(ctz);
+	clk_disable_unprepare(ctz->clk_tempsen);
+
+	return 0;
+}
+
+static int cv180x_thermal_resume(struct device *dev)
+{
+	struct cv180x_thermal_zone *ctz = dev_get_drvdata(dev);
+
+	clk_prepare_enable(ctz->clk_tempsen);
+	cv180x_thermal_init(ctz);
+
+	return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(cv180x_thermal_pm_ops,
+			 cv180x_thermal_suspend, cv180x_thermal_resume);
+
+static struct platform_driver cv180x_thermal_driver = {
+	.probe = cv180x_thermal_probe,
+	.remove = cv180x_thermal_remove,
+	.driver = {
+		.name = "cv180x-thermal",
+		.pm = &cv180x_thermal_pm_ops,
+		.of_match_table = cv180x_thermal_of_match,
+	},
+};
+
+module_platform_driver(cv180x_thermal_driver);
+
+MODULE_AUTHOR("Haylen Chu <heylenay@outlook.com>");
+MODULE_LICENSE("GPL");
-- 
2.45.1

Re: [PATCH 3/3] thermal: cv180x: Add cv180x thermal driver support

[Inochi Amaoto]

On Thu, May 30, 2024 at 01:48:27PM GMT, Haylen Chu wrote:
> Add support for cv180x SoCs integrated thermal sensors.
> 
> Signed-off-by: Haylen Chu <heylenay@outlook.com>
> ---
>  drivers/thermal/Kconfig          |   6 +
>  drivers/thermal/Makefile         |   1 +
>  drivers/thermal/cv180x_thermal.c | 210 +++++++++++++++++++++++++++++++
>  3 files changed, 217 insertions(+)
>  create mode 100644 drivers/thermal/cv180x_thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 204ed89a3ec9..f53c973a361d 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -514,4 +514,10 @@ config LOONGSON2_THERMAL
>  	  is higher than the high temperature threshold or lower than the low
>  	  temperature threshold, the interrupt will occur.
>  
> +config CV180X_THERMAL
> +	tristate "Temperature sensor driver for Sophgo CV180X"
> +	help
> +	  If you say yes here you get support for thermal sensor integrated in
> +	  Sophgo CV180X SoCs.
> +
>  endif
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 5cdf7d68687f..5b59bde8a579 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -65,3 +65,4 @@ obj-$(CONFIG_AMLOGIC_THERMAL)     += amlogic_thermal.o
>  obj-$(CONFIG_SPRD_THERMAL)	+= sprd_thermal.o
>  obj-$(CONFIG_KHADAS_MCU_FAN_THERMAL)	+= khadas_mcu_fan.o
>  obj-$(CONFIG_LOONGSON2_THERMAL)	+= loongson2_thermal.o
> +obj-$(CONFIG_CV180X_THERMAL)	+= cv180x_thermal.o
> diff --git a/drivers/thermal/cv180x_thermal.c b/drivers/thermal/cv180x_thermal.c
> new file mode 100644
> index 000000000000..618e031b4515
> --- /dev/null
> +++ b/drivers/thermal/cv180x_thermal.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2021 CVITEK Inc.

There is no CVITEK anyway, use SOPHGO instead. 

> + * Copyright (C) 2024 Haylen Chu <heylenay@outlook.com>
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/io.h>
> +#include <linux/thermal.h>
> +

> +#define TEMPSEN_VERSION					0x0
> +#define TEMPSEN_CTRL					0x4
> +#define TEMPSEN_CTRL_EN					BIT(0)
> +#define TEMPSEN_CTRL_SEL_MASK				GENMASK(7, 4)
> +#define TEMPSEN_CTRL_SEL_OFFSET				4
> +#define TEMPSEN_STATUS					0x8
> +#define TEMPSEN_SET					0xc
> +#define TEMPSEN_SET_CHOPSEL_MASK			GENMASK(5, 4)
> +#define TEMPSEN_SET_CHOPSEL_OFFSET			4
> +#define TEMPSEN_SET_CHOPSEL_128T			0
> +#define TEMPSEN_SET_CHOPSEL_256T			1
> +#define TEMPSEN_SET_CHOPSEL_512T			2
> +#define TEMPSEN_SET_CHOPSEL_1024T			3
> +#define TEMPSEN_SET_ACCSEL_MASK				GENMASK(7, 6)
> +#define TEMPSEN_SET_ACCSEL_OFFSET			6
> +#define TEMPSEN_SET_ACCSEL_512T				0
> +#define TEMPSEN_SET_ACCSEL_1024T			1
> +#define TEMPSEN_SET_ACCSEL_2048T			2
> +#define TEMPSEN_SET_ACCSEL_4096T			3
> +#define TEMPSEN_SET_CYC_CLKDIV_MASK			GENMASK(15, 8)
> +#define TEMPSEN_SET_CYC_CLKDIV_OFFSET			8
> +#define TEMPSEN_INTR_EN					0x10
> +#define TEMPSEN_INTR_CLR				0x14
> +#define TEMPSEN_INTR_STA				0x18
> +#define TEMPSEN_INTR_RAW				0x1c
> +#define TEMPSEN_RESULT(n)				(0x20 + (n) * 4)
> +#define TEMPSEN_RESULT_RESULT_MASK			GENMASK(12, 0)
> +#define TEMPSEN_RESULT_MAX_RESULT_MASK			GENMASK(28, 16)
> +#define TEMPSEN_RESULT_CLR_MAX_RESULT			BIT(31)
> +#define TEMPSEN_AUTO_PERIOD				0x64
> +#define TEMPSEN_AUTO_PERIOD_AUTO_CYCLE_MASK		GENMASK(23, 0)
> +#define TEMPSEN_AUTO_PERIOD_AUTO_CYCLE_OFFSET		0

I suggest adding extra indent to the register value macro. It is too hard to
identify which one is register offset, which one it register value.

> +
> +struct cv180x_thermal_zone {
> +	struct device *dev;
> +	void __iomem *base;
> +	struct clk *clk_tempsen;
> +};
> +

> +static void cv180x_thermal_init(struct cv180x_thermal_zone *ctz)
> +{
> +	void __iomem *base = ctz->base;
> +	u32 regval;
> +
> +	writel(readl(base + TEMPSEN_INTR_RAW), base + TEMPSEN_INTR_CLR);
> +	writel(TEMPSEN_RESULT_CLR_MAX_RESULT, base + TEMPSEN_RESULT(0));
> +
> +	regval = readl(base + TEMPSEN_SET);
> +	regval &= ~TEMPSEN_SET_CHOPSEL_MASK;
> +	regval &= ~TEMPSEN_SET_ACCSEL_MASK;
> +	regval &= ~TEMPSEN_SET_CYC_CLKDIV_MASK;
> +	regval |= TEMPSEN_SET_CHOPSEL_1024T << TEMPSEN_SET_CHOPSEL_OFFSET;
> +	regval |= TEMPSEN_SET_ACCSEL_2048T  << TEMPSEN_SET_ACCSEL_OFFSET;
> +	regval |= 0x31 << TEMPSEN_SET_CYC_CLKDIV_OFFSET;
> +	writel(regval, base + TEMPSEN_SET);
> +
> +	regval = readl(base + TEMPSEN_AUTO_PERIOD);
> +	regval &= ~TEMPSEN_AUTO_PERIOD_AUTO_CYCLE_MASK;
> +	regval |= 0x100000 << TEMPSEN_AUTO_PERIOD_AUTO_CYCLE_OFFSET;
> +	writel(regval, base + TEMPSEN_AUTO_PERIOD);
> +
> +	regval = readl(base + TEMPSEN_CTRL);
> +	regval &= ~TEMPSEN_CTRL_SEL_MASK;
> +	regval |= 1 << TEMPSEN_CTRL_SEL_OFFSET;
> +	regval |= TEMPSEN_CTRL_EN;
> +	writel(regval, base + TEMPSEN_CTRL);
> +}

The sensors of CV1800 support various periods, I think you should add
support for all of them and let user select them. The configuration
you use now can be left as the default.

> +
> +static void cv180x_thermal_deinit(struct cv180x_thermal_zone *ct)
> +{
> +	void __iomem *base = ct->base;
> +	u32 regval;
> +
> +	regval = readl(base + TEMPSEN_CTRL);
> +	regval &= ~(TEMPSEN_CTRL_SEL_MASK | TEMPSEN_CTRL_EN);
> +	writel(regval, base + TEMPSEN_CTRL);
> +
> +	writel(readl(base + TEMPSEN_INTR_RAW), base + TEMPSEN_INTR_CLR);
> +}
> +

> +static int calc_temp(uint32_t result)

Add "cv180x" prefix.

> +{
> +	return ((result * 1000) * 716 / 2048 - 273000);
> +}

Why these magic number, I have not see any info in the document. 

> +
> +static int cv180x_get_temp(struct thermal_zone_device *tdev, int *temperature)
> +{
> +	struct cv180x_thermal_zone *ctz = thermal_zone_device_priv(tdev);
> +	void __iomem *base = ctz->base;
> +	u32 result;
> +
> +	result = readl(base + TEMPSEN_RESULT(0)) & TEMPSEN_RESULT_RESULT_MASK;
> +	*temperature = calc_temp(result);
> +
> +	return 0;
> +}
> +
> +static const struct thermal_zone_device_ops cv180x_thermal_ops = {
> +	.get_temp = cv180x_get_temp,
> +};
> +
> +static const struct of_device_id cv180x_thermal_of_match[] = {
> +	{
> +		.compatible = "sophgo,cv180x-thermal",
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, cv180x_thermal_of_match);
> +
> +static int cv180x_thermal_probe(struct platform_device *pdev)
> +{
> +	struct cv180x_thermal_zone *ctz;
> +	struct thermal_zone_device *tz;
> +	struct resource *res;
> +
> +	ctz = devm_kzalloc(&pdev->dev, sizeof(*ctz), GFP_KERNEL);
> +	if (!ctz)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	ctz->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(ctz->base))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(ctz->base),
> +				     "failed to map tempsen registers\n");
> +
> +	ctz->clk_tempsen = devm_clk_get(&pdev->dev, "clk_tempsen");
> +	if (IS_ERR(ctz->clk_tempsen))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(ctz->clk_tempsen),
> +				     "failed to get clk_tempsen\n");
> +
> +	clk_prepare_enable(ctz->clk_tempsen);
> +
> +	ctz->dev = &pdev->dev;
> +
> +	cv180x_thermal_init(ctz);
> +
> +	platform_set_drvdata(pdev, ctz);

Set this after register thermal zone.

> +
> +	tz = devm_thermal_of_zone_register(&pdev->dev, 0, ctz,
> +					   &cv180x_thermal_ops);
> +	if (IS_ERR(tz))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(tz),
> +				     "failed to register thermal zone\n");
> +
> +	return 0;
> +}
> +
> +static int cv180x_thermal_remove(struct platform_device *pdev)
> +{
> +	struct cv180x_thermal_zone *ctz = platform_get_drvdata(pdev);
> +
> +	cv180x_thermal_deinit(ctz);
> +	clk_disable_unprepare(ctz->clk_tempsen);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int cv180x_thermal_suspend(struct device *dev)
> +{
> +	struct cv180x_thermal_zone *ctz = dev_get_drvdata(dev);
> +
> +	cv180x_thermal_deinit(ctz);
> +	clk_disable_unprepare(ctz->clk_tempsen);
> +
> +	return 0;
> +}
> +
> +static int cv180x_thermal_resume(struct device *dev)
> +{
> +	struct cv180x_thermal_zone *ctz = dev_get_drvdata(dev);
> +
> +	clk_prepare_enable(ctz->clk_tempsen);
> +	cv180x_thermal_init(ctz);
> +
> +	return 0;
> +}
> +#endif /* CONFIG_PM_SLEEP */

Use __maybe_unused may be more clear with SIMPLE_DEV_PM_OPS.

> +
> +static SIMPLE_DEV_PM_OPS(cv180x_thermal_pm_ops,
> +			 cv180x_thermal_suspend, cv180x_thermal_resume);
> +
> +static struct platform_driver cv180x_thermal_driver = {
> +	.probe = cv180x_thermal_probe,
> +	.remove = cv180x_thermal_remove,
> +	.driver = {
> +		.name = "cv180x-thermal",
> +		.pm = &cv180x_thermal_pm_ops,
> +		.of_match_table = cv180x_thermal_of_match,
> +	},
> +};
> +
> +module_platform_driver(cv180x_thermal_driver);
> +
> +MODULE_AUTHOR("Haylen Chu <heylenay@outlook.com>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.45.1
> 

Re: [PATCH 3/3] thermal: cv180x: Add cv180x thermal driver support

[Haylen Chu]

Sorry, I forgot add cc and to in the last email. This is a resend.

On Fri, May 31, 2024 at 07:45:37AM +0800, Inochi Amaoto wrote:
> The sensors of CV1800 support various periods, I think you should add
> support for all of them and let user select them. The configuration
> you use now can be left as the default.

I will make sample period configurable in next revision.

> > +{
> > +   return ((result * 1000) * 716 / 2048 - 273000);
> > +}
>
> Why these magic number, I have not see any info in the document.

Actually, there is no document of calculating real temperature from raw
register value. The equation above is extracted from code provided by
Sophgo.

I have figured out meaning of part of the equation and could add some
comments to document it.

---
Haylen Chu

Re: [PATCH 3/3] thermal: cv180x: Add cv180x thermal driver support

[Inochi Amaoto]

On Fri, May 31, 2024 at 01:37:01PM GMT, Haylen Chu wrote:
> Sorry, I forgot add cc and to in the last email. This is a resend.
> 
> On Fri, May 31, 2024 at 07:45:37AM +0800, Inochi Amaoto wrote:
> > The sensors of CV1800 support various periods, I think you should add
> > support for all of them and let user select them. The configuration
> > you use now can be left as the default.
> 
> I will make sample period configurable in next revision.
> 
> > > +{
> > > +   return ((result * 1000) * 716 / 2048 - 273000);
> > > +}
> >
> > Why these magic number, I have not see any info in the document.
> 
> Actually, there is no document of calculating real temperature from raw
> register value. The equation above is extracted from code provided by
> Sophgo.
> 
> I have figured out meaning of part of the equation and could add some
> comments to document it.
> 

I think you misunderstood. I did not only ask for the document, but also 
a formula that respects the configuration. It is pretty weird to use all
fix magic number to calculate the temp value since the sensors itself
supports various configuration, right?