LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [v5 00/15] Add support for ast2600 ADC
@ 2021-08-31  7:14 Billy Tsai
  2021-08-31  7:14 ` [v5 01/15] iio: adc: aspeed: set driver data when adc probe Billy Tsai
                   ` (14 more replies)
  0 siblings, 15 replies; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

This patch serials make aspeed_adc.c can support ast2600 and backward
compatible.

Change since v4:
dt-bindings:
  - Add clocks maxItems.
  - Rename the property to meet the property-units.yaml.
  - Add the description for the difference between adc0 and adc1.
aspeed_adc.c:
  - Use new property name to get internal reference voltage: units from mv to
  uv.
  - Fix -Wnonnull warning caused by snprintf parameters.
  - Add suffix mv to the vref parameters.
  - Use ARRAY_SIZE instead of 32.
  - Add a reset action for ADC power down and Use devm_iio_device_register.
  - Fix typo error.
  - Separate the offset interface of ch7 when battery sensing enable

Change since v3:
dt-bindings:
  - Fix properties:aspeed,int_vref_mv type error.

Change since v2:
dt-bindings:
  - Create a new dt-bindings for ast2600 adc
aspeed_adc.c:
  - Splits the patch for more details
  - Remove version enum and use the flags in model data to distinguish
  hardware feature
  - Support trimming data get and set.
  - Use devm_add_action_or_reset to simplify probe error handling.

Changes since v1:
dt-bindings:
  - Fix the aspeed,adc.yaml check error.
  - Add battery-sensing property.
aspeed_adc.c:
  - Change the init flow:
    Clock and reference voltage setting should be completed before adc
    engine enable.
  - Change the default sampling rate to meet most user case.
  - Add patch #8 to suppoert battery sensing mode.

Billy Tsai (15):
  iio: adc: aspeed: set driver data when adc probe.
  dt-bindings: iio: adc: Add ast2600-adc bindings
  iio: adc: aspeed: completes the bitfield declare.
  iio: adc: aspeed: Keep model data to driver data.
  iio: adc: aspeed: Refactory model data structure
  iio: adc: aspeed: Add vref config function
  iio: adc: aspeed: Set num_channels with model data
  iio: adc: aspeed: Use model_data to set clk scaler.
  iio: adc: aspeed: Use devm_add_action_or_reset.
  iio: adc: aspeed: Support ast2600 adc.
  iio: adc: aspeed: Fix the calculate error of clock.
  iio: adc: aspeed: Add func to set sampling rate.
  iio: adc: aspeed: Add compensation phase.
  iio: adc: aspeed: Support battery sensing.
  iio: adc: aspeed: Get and set trimming data.

 .../bindings/iio/adc/aspeed,ast2600-adc.yaml  | 100 +++
 drivers/iio/adc/aspeed_adc.c                  | 617 +++++++++++++++---
 2 files changed, 619 insertions(+), 98 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/aspeed,ast2600-adc.yaml

-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 01/15] iio: adc: aspeed: set driver data when adc probe.
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-09-05 14:25   ` Jonathan Cameron
  2021-08-31  7:14 ` [v5 02/15] dt-bindings: iio: adc: Add ast2600-adc bindings Billy Tsai
                   ` (13 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

Fix the issue when adc remove will get the null driver data.

Fixed: commit 573803234e72 ("iio: Aspeed ADC")
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index 19efaa41bc34..34ec0c28b2df 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -183,6 +183,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 
 	data = iio_priv(indio_dev);
 	data->dev = &pdev->dev;
+	platform_set_drvdata(pdev, indio_dev);
 
 	data->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(data->base))
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 02/15] dt-bindings: iio: adc: Add ast2600-adc bindings
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
  2021-08-31  7:14 ` [v5 01/15] iio: adc: aspeed: set driver data when adc probe Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-09-01  1:38   ` Rob Herring
  2021-08-31  7:14 ` [v5 03/15] iio: adc: aspeed: completes the bitfield declare Billy Tsai
                   ` (12 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

Add device tree bindings document for the aspeed ast2600 adc device
driver.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 .../bindings/iio/adc/aspeed,ast2600-adc.yaml  | 100 ++++++++++++++++++
 1 file changed, 100 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/aspeed,ast2600-adc.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/aspeed,ast2600-adc.yaml b/Documentation/devicetree/bindings/iio/adc/aspeed,ast2600-adc.yaml
new file mode 100644
index 000000000000..b283c8ca2bbf
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/aspeed,ast2600-adc.yaml
@@ -0,0 +1,100 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/aspeed,ast2600-adc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADC that forms part of an ASPEED server management processor.
+
+maintainers:
+  - Billy Tsai <billy_tsai@aspeedtech.com>
+
+description: |
+  • 10-bits resolution for 16 voltage channels.
+  • The device split into two individual engine and each contains 8 voltage
+  channels.
+  • Channel scanning can be non-continuous.
+  • Programmable ADC clock frequency.
+  • Programmable upper and lower threshold for each channels.
+  • Interrupt when larger or less than threshold for each channels.
+  • Support hysteresis for each channels.
+  • Built-in a compensating method.
+  • Built-in a register to trim internal reference voltage.
+  • Internal or External reference voltage.
+  • Support 2 Internal reference voltage 1.2v or 2.5v.
+  • Integrate dividing circuit for battery sensing.
+
+properties:
+  compatible:
+    enum:
+      - aspeed,ast2600-adc0
+      - aspeed,ast2600-adc1
+    description:
+      Their trimming data, which is used to calibrate internal reference volage,
+      locates in different address of OTP.
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description:
+      Input clock used to derive the sample clock. Expected to be the
+      SoC's APB clock.
+
+  resets:
+    maxItems: 1
+
+  "#io-channel-cells":
+    const: 1
+
+  vref-supply:
+    description:
+      The external regulator supply ADC reference voltage.
+
+  aspeed,int-vref-microvolt:
+    enum: [1200000, 2500000]
+    description:
+      ADC internal reference voltage in microvolts.
+
+  aspeed,battery-sensing:
+    type: boolean
+    description:
+      Inform the driver that last channel will be used to sensor battery.
+
+  aspeed,trim-data-valid:
+    type: boolean
+    description: |
+      The ADC reference voltage can be calibrated to obtain the trimming
+      data which will be stored in otp. This property informs the driver that
+      the data store in the otp is valid.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - resets
+  - "#io-channel-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/ast2600-clock.h>
+    adc0: adc@1e6e9000 {
+        compatible = "aspeed,ast2600-adc0";
+        reg = <0x1e6e9000 0x100>;
+        clocks = <&syscon ASPEED_CLK_APB2>;
+        resets = <&syscon ASPEED_RESET_ADC>;
+        #io-channel-cells = <1>;
+        aspeed,int-vref-microvolt = <2500000>;
+    };
+    adc1: adc@1e6e9100 {
+        compatible = "aspeed,ast2600-adc1";
+        reg = <0x1e6e9100 0x100>;
+        clocks = <&syscon ASPEED_CLK_APB2>;
+        resets = <&syscon ASPEED_RESET_ADC>;
+        #io-channel-cells = <1>;
+        aspeed,int-vref-microvolt = <2500000>;
+    };
+...
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 03/15] iio: adc: aspeed: completes the bitfield declare.
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
  2021-08-31  7:14 ` [v5 01/15] iio: adc: aspeed: set driver data when adc probe Billy Tsai
  2021-08-31  7:14 ` [v5 02/15] dt-bindings: iio: adc: Add ast2600-adc bindings Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-09-05 14:30   ` Jonathan Cameron
  2021-08-31  7:14 ` [v5 04/15] iio: adc: aspeed: Keep model data to driver data Billy Tsai
                   ` (11 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

This patch completes the declare of ADC register bitfields and uses the
same prefix ASPEED_ADC_* for these bitfields. In addition, tidy up space
alignment of the codes.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 64 ++++++++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index 34ec0c28b2df..f055fe7b2c40 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -3,6 +3,7 @@
  * Aspeed AST2400/2500 ADC
  *
  * Copyright (C) 2017 Google, Inc.
+ * Copyright (C) 2021 Aspeed Technology Inc.
  */
 
 #include <linux/clk.h>
@@ -16,6 +17,7 @@
 #include <linux/reset.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
+#include <linux/bitfield.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/driver.h>
@@ -28,15 +30,39 @@
 #define ASPEED_REG_INTERRUPT_CONTROL	0x04
 #define ASPEED_REG_VGA_DETECT_CONTROL	0x08
 #define ASPEED_REG_CLOCK_CONTROL	0x0C
-#define ASPEED_REG_MAX			0xC0
-
-#define ASPEED_OPERATION_MODE_POWER_DOWN	(0x0 << 1)
-#define ASPEED_OPERATION_MODE_STANDBY		(0x1 << 1)
-#define ASPEED_OPERATION_MODE_NORMAL		(0x7 << 1)
-
-#define ASPEED_ENGINE_ENABLE		BIT(0)
-
-#define ASPEED_ADC_CTRL_INIT_RDY	BIT(8)
+#define ASPEED_REG_COMPENSATION_TRIM	0xC4
+/*
+ * The register offset between 0xC8~0xCC can be read and won't affect the
+ * hardware logic in each version of ADC.
+ */
+#define ASPEED_REG_MAX			0xD0
+
+#define ASPEED_ADC_ENGINE_ENABLE		BIT(0)
+#define ASPEED_ADC_OP_MODE			GENMASK(3, 1)
+#define ASPEED_ADC_OP_MODE_PWR_DOWN		0
+#define ASPEED_ADC_OP_MODE_STANDBY		1
+#define ASPEED_ADC_OP_MODE_NORMAL		7
+#define ASPEED_ADC_CTRL_COMPENSATION		BIT(4)
+#define ASPEED_ADC_AUTO_COMPENSATION		BIT(5)
+/*
+ * Bit 6 determines not only the reference voltage range but also the dividing
+ * circuit for battery sensing.
+ */
+#define ASPEED_ADC_REF_VOLTAGE			GENMASK(7, 6)
+#define ASPEED_ADC_REF_VOLTAGE_2500mV		0
+#define ASPEED_ADC_REF_VOLTAGE_1200mV		1
+#define ASPEED_ADC_REF_VOLTAGE_EXT_HIGH		2
+#define ASPEED_ADC_REF_VOLTAGE_EXT_LOW		3
+#define ASPEED_ADC_BAT_SENSING_DIV		BIT(6)
+#define ASPEED_ADC_BAT_SENSING_DIV_2_3		0
+#define ASPEED_ADC_BAT_SENSING_DIV_1_3		1
+#define ASPEED_ADC_CTRL_INIT_RDY		BIT(8)
+#define ASPEED_ADC_CH7_MODE			BIT(12)
+#define ASPEED_ADC_CH7_NORMAL			0
+#define ASPEED_ADC_CH7_BAT			1
+#define ASPEED_ADC_BAT_SENSING_ENABLE		BIT(13)
+#define ASPEED_ADC_CTRL_CHANNEL			GENMASK(31, 16)
+#define ASPEED_ADC_CTRL_CHANNEL_ENABLE(ch)	FIELD_PREP(ASPEED_ADC_CTRL_CHANNEL, BIT(ch))
 
 #define ASPEED_ADC_INIT_POLLING_TIME	500
 #define ASPEED_ADC_INIT_TIMEOUT		500000
@@ -227,7 +253,9 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 
 	if (model_data->wait_init_sequence) {
 		/* Enable engine in normal mode. */
-		writel(ASPEED_OPERATION_MODE_NORMAL | ASPEED_ENGINE_ENABLE,
+		writel(FIELD_PREP(ASPEED_ADC_OP_MODE,
+				  ASPEED_ADC_OP_MODE_NORMAL) |
+			       ASPEED_ADC_ENGINE_ENABLE,
 		       data->base + ASPEED_REG_ENGINE_CONTROL);
 
 		/* Wait for initial sequence complete. */
@@ -246,10 +274,12 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	if (ret)
 		goto clk_enable_error;
 
-	adc_engine_control_reg_val = GENMASK(31, 16) |
-		ASPEED_OPERATION_MODE_NORMAL | ASPEED_ENGINE_ENABLE;
+	adc_engine_control_reg_val =
+		ASPEED_ADC_CTRL_CHANNEL |
+		FIELD_PREP(ASPEED_ADC_OP_MODE, ASPEED_ADC_OP_MODE_NORMAL) |
+		ASPEED_ADC_ENGINE_ENABLE;
 	writel(adc_engine_control_reg_val,
-		data->base + ASPEED_REG_ENGINE_CONTROL);
+	       data->base + ASPEED_REG_ENGINE_CONTROL);
 
 	model_data = of_device_get_match_data(&pdev->dev);
 	indio_dev->name = model_data->model_name;
@@ -265,8 +295,8 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	return 0;
 
 iio_register_error:
-	writel(ASPEED_OPERATION_MODE_POWER_DOWN,
-		data->base + ASPEED_REG_ENGINE_CONTROL);
+	writel(FIELD_PREP(ASPEED_ADC_OP_MODE, ASPEED_ADC_OP_MODE_PWR_DOWN),
+	       data->base + ASPEED_REG_ENGINE_CONTROL);
 	clk_disable_unprepare(data->clk_scaler->clk);
 clk_enable_error:
 poll_timeout_error:
@@ -284,8 +314,8 @@ static int aspeed_adc_remove(struct platform_device *pdev)
 	struct aspeed_adc_data *data = iio_priv(indio_dev);
 
 	iio_device_unregister(indio_dev);
-	writel(ASPEED_OPERATION_MODE_POWER_DOWN,
-		data->base + ASPEED_REG_ENGINE_CONTROL);
+	writel(FIELD_PREP(ASPEED_ADC_OP_MODE, ASPEED_ADC_OP_MODE_PWR_DOWN),
+	       data->base + ASPEED_REG_ENGINE_CONTROL);
 	clk_disable_unprepare(data->clk_scaler->clk);
 	reset_control_assert(data->rst);
 	clk_hw_unregister_divider(data->clk_scaler);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 04/15] iio: adc: aspeed: Keep model data to driver data.
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
                   ` (2 preceding siblings ...)
  2021-08-31  7:14 ` [v5 03/15] iio: adc: aspeed: completes the bitfield declare Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-09-05 14:33   ` Jonathan Cameron
  2021-08-31  7:14 ` [v5 05/15] iio: adc: aspeed: Refactory model data structure Billy Tsai
                   ` (10 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

Keep the model data pointer to driver data for reducing the usage of
of_device_get_match_data().

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index f055fe7b2c40..76ae1c3f584b 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -77,6 +77,7 @@ struct aspeed_adc_model_data {
 
 struct aspeed_adc_data {
 	struct device		*dev;
+	const struct aspeed_adc_model_data *model_data;
 	void __iomem		*base;
 	spinlock_t		clk_lock;
 	struct clk_hw		*clk_prescaler;
@@ -118,8 +119,6 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
 			       int *val, int *val2, long mask)
 {
 	struct aspeed_adc_data *data = iio_priv(indio_dev);
-	const struct aspeed_adc_model_data *model_data =
-			of_device_get_match_data(data->dev);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
@@ -127,7 +126,7 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
 		return IIO_VAL_INT;
 
 	case IIO_CHAN_INFO_SCALE:
-		*val = model_data->vref_voltage;
+		*val = data->model_data->vref_voltage;
 		*val2 = ASPEED_RESOLUTION_BITS;
 		return IIO_VAL_FRACTIONAL_LOG2;
 
@@ -146,13 +145,11 @@ static int aspeed_adc_write_raw(struct iio_dev *indio_dev,
 				int val, int val2, long mask)
 {
 	struct aspeed_adc_data *data = iio_priv(indio_dev);
-	const struct aspeed_adc_model_data *model_data =
-			of_device_get_match_data(data->dev);
 
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		if (val < model_data->min_sampling_rate ||
-			val > model_data->max_sampling_rate)
+		if (val < data->model_data->min_sampling_rate ||
+			val > data->model_data->max_sampling_rate)
 			return -EINVAL;
 
 		clk_set_rate(data->clk_scaler->clk,
@@ -198,7 +195,6 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 {
 	struct iio_dev *indio_dev;
 	struct aspeed_adc_data *data;
-	const struct aspeed_adc_model_data *model_data;
 	const char *clk_parent_name;
 	int ret;
 	u32 adc_engine_control_reg_val;
@@ -209,6 +205,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 
 	data = iio_priv(indio_dev);
 	data->dev = &pdev->dev;
+	data->model_data = of_device_get_match_data(&pdev->dev);
 	platform_set_drvdata(pdev, indio_dev);
 
 	data->base = devm_platform_ioremap_resource(pdev, 0);
@@ -249,9 +246,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	}
 	reset_control_deassert(data->rst);
 
-	model_data = of_device_get_match_data(&pdev->dev);
-
-	if (model_data->wait_init_sequence) {
+	if (data->model_data->wait_init_sequence) {
 		/* Enable engine in normal mode. */
 		writel(FIELD_PREP(ASPEED_ADC_OP_MODE,
 				  ASPEED_ADC_OP_MODE_NORMAL) |
@@ -281,8 +276,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	writel(adc_engine_control_reg_val,
 	       data->base + ASPEED_REG_ENGINE_CONTROL);
 
-	model_data = of_device_get_match_data(&pdev->dev);
-	indio_dev->name = model_data->model_name;
+	indio_dev->name = data->model_data->model_name;
 	indio_dev->info = &aspeed_adc_iio_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = aspeed_adc_iio_channels;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 05/15] iio: adc: aspeed: Refactory model data structure
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
                   ` (3 preceding siblings ...)
  2021-08-31  7:14 ` [v5 04/15] iio: adc: aspeed: Keep model data to driver data Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-09-05 14:37   ` Jonathan Cameron
  2021-08-31  7:14 ` [v5 06/15] iio: adc: aspeed: Add vref config function Billy Tsai
                   ` (9 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

This patch refactors the model data structure to distinguish the
function form different versions of aspeed ADC.
- Rename the vref_voltage to vref_fixed_mv and add vref_mv driver data
When driver probe will check vref_fixed_mv value and store it to vref_mv
which isn't const value.
- Add num_channels
Make num_channles of iio device can be changed by different model_data
- Add need_prescaler flag and scaler_bit_width
The need_prescaler flag is used to tell the driver the clock divider needs
another Prescaler and the scaler_bit_width to set the clock divider
bitfield width.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index 76ae1c3f584b..6ce2f676c54a 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -71,8 +71,11 @@ struct aspeed_adc_model_data {
 	const char *model_name;
 	unsigned int min_sampling_rate;	// Hz
 	unsigned int max_sampling_rate;	// Hz
-	unsigned int vref_voltage;	// mV
+	unsigned int vref_fixed_mv;
 	bool wait_init_sequence;
+	bool need_prescaler;
+	u8 scaler_bit_width;
+	unsigned int num_channels;
 };
 
 struct aspeed_adc_data {
@@ -83,6 +86,7 @@ struct aspeed_adc_data {
 	struct clk_hw		*clk_prescaler;
 	struct clk_hw		*clk_scaler;
 	struct reset_control	*rst;
+	int			vref_mv;
 };
 
 #define ASPEED_CHAN(_idx, _data_reg_addr) {			\
@@ -126,7 +130,7 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
 		return IIO_VAL_INT;
 
 	case IIO_CHAN_INFO_SCALE:
-		*val = data->model_data->vref_voltage;
+		*val = data->model_data->vref_fixed_mv;
 		*val2 = ASPEED_RESOLUTION_BITS;
 		return IIO_VAL_FRACTIONAL_LOG2;
 
@@ -320,17 +324,23 @@ static int aspeed_adc_remove(struct platform_device *pdev)
 
 static const struct aspeed_adc_model_data ast2400_model_data = {
 	.model_name = "ast2400-adc",
-	.vref_voltage = 2500, // mV
+	.vref_fixed_mv = 2500,
 	.min_sampling_rate = 10000,
 	.max_sampling_rate = 500000,
+	.need_prescaler = true,
+	.scaler_bit_width = 10,
+	.num_channels = 16,
 };
 
 static const struct aspeed_adc_model_data ast2500_model_data = {
 	.model_name = "ast2500-adc",
-	.vref_voltage = 1800, // mV
+	.vref_fixed_mv = 1800,
 	.min_sampling_rate = 1,
 	.max_sampling_rate = 1000000,
 	.wait_init_sequence = true,
+	.need_prescaler = true,
+	.scaler_bit_width = 10,
+	.num_channels = 16,
 };
 
 static const struct of_device_id aspeed_adc_matches[] = {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 06/15] iio: adc: aspeed: Add vref config function
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
                   ` (4 preceding siblings ...)
  2021-08-31  7:14 ` [v5 05/15] iio: adc: aspeed: Refactory model data structure Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-08-31  7:14 ` [v5 07/15] iio: adc: aspeed: Set num_channels with model data Billy Tsai
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

Add the function to check the vref_fixed_mv and set the value to driver
data.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index 6ce2f676c54a..01df24f6d3f1 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -130,7 +130,7 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
 		return IIO_VAL_INT;
 
 	case IIO_CHAN_INFO_SCALE:
-		*val = data->model_data->vref_fixed_mv;
+		*val = data->vref_mv;
 		*val2 = ASPEED_RESOLUTION_BITS;
 		return IIO_VAL_FRACTIONAL_LOG2;
 
@@ -195,6 +195,17 @@ static const struct iio_info aspeed_adc_iio_info = {
 	.debugfs_reg_access = aspeed_adc_reg_access,
 };
 
+static int aspeed_adc_vref_config(struct iio_dev *indio_dev)
+{
+	struct aspeed_adc_data *data = iio_priv(indio_dev);
+
+	if (data->model_data->vref_fixed_mv) {
+		data->vref_mv = data->model_data->vref_fixed_mv;
+		return 0;
+	}
+	return 0;
+}
+
 static int aspeed_adc_probe(struct platform_device *pdev)
 {
 	struct iio_dev *indio_dev;
@@ -250,6 +261,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	}
 	reset_control_deassert(data->rst);
 
+	ret = aspeed_adc_vref_config(indio_dev);
+	if (ret)
+		goto vref_config_error;
+
 	if (data->model_data->wait_init_sequence) {
 		/* Enable engine in normal mode. */
 		writel(FIELD_PREP(ASPEED_ADC_OP_MODE,
@@ -298,6 +313,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	clk_disable_unprepare(data->clk_scaler->clk);
 clk_enable_error:
 poll_timeout_error:
+vref_config_error:
 	reset_control_assert(data->rst);
 reset_error:
 	clk_hw_unregister_divider(data->clk_scaler);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 07/15] iio: adc: aspeed: Set num_channels with model data
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
                   ` (5 preceding siblings ...)
  2021-08-31  7:14 ` [v5 06/15] iio: adc: aspeed: Add vref config function Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-08-31  7:14 ` [v5 08/15] iio: adc: aspeed: Use model_data to set clk scaler Billy Tsai
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

Use the model_data member num_channels to set the num_channels of iio
device.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index 01df24f6d3f1..aec335956310 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -299,7 +299,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	indio_dev->info = &aspeed_adc_iio_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 	indio_dev->channels = aspeed_adc_iio_channels;
-	indio_dev->num_channels = ARRAY_SIZE(aspeed_adc_iio_channels);
+	indio_dev->num_channels = data->model_data->num_channels;
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 08/15] iio: adc: aspeed: Use model_data to set clk scaler.
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
                   ` (6 preceding siblings ...)
  2021-08-31  7:14 ` [v5 07/15] iio: adc: aspeed: Set num_channels with model data Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-08-31  7:14 ` [v5 09/15] iio: adc: aspeed: Use devm_add_action_or_reset Billy Tsai
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW, kernel test robot

This patch uses need_prescaler and scaler_bit_width to set the ADC clock
scaler.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 43 ++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index aec335956310..e53d1541ad1d 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -210,9 +210,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 {
 	struct iio_dev *indio_dev;
 	struct aspeed_adc_data *data;
-	const char *clk_parent_name;
 	int ret;
 	u32 adc_engine_control_reg_val;
+	unsigned long scaler_flags = 0;
+	char clk_name[32], clk_parent_name[32];
 
 	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*data));
 	if (!indio_dev)
@@ -229,24 +230,32 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 
 	/* Register ADC clock prescaler with source specified by device tree. */
 	spin_lock_init(&data->clk_lock);
-	clk_parent_name = of_clk_get_parent_name(pdev->dev.of_node, 0);
-
-	data->clk_prescaler = clk_hw_register_divider(
-				&pdev->dev, "prescaler", clk_parent_name, 0,
-				data->base + ASPEED_REG_CLOCK_CONTROL,
-				17, 15, 0, &data->clk_lock);
-	if (IS_ERR(data->clk_prescaler))
-		return PTR_ERR(data->clk_prescaler);
-
+	snprintf(clk_parent_name, ARRAY_SIZE(clk_parent_name), "%s",
+		 of_clk_get_parent_name(pdev->dev.of_node, 0));
+
+	if (data->model_data->need_prescaler) {
+		snprintf(clk_name, ARRAY_SIZE(clk_name), "%s-prescaler",
+			 data->model_data->model_name);
+		data->clk_prescaler = clk_hw_register_divider(
+			&pdev->dev, clk_name, clk_parent_name, 0,
+			data->base + ASPEED_REG_CLOCK_CONTROL, 17, 15, 0,
+			&data->clk_lock);
+		if (IS_ERR(data->clk_prescaler))
+			return PTR_ERR(data->clk_prescaler);
+		snprintf(clk_parent_name, ARRAY_SIZE(clk_parent_name),
+			 clk_name);
+		scaler_flags = CLK_SET_RATE_PARENT;
+	}
 	/*
 	 * Register ADC clock scaler downstream from the prescaler. Allow rate
 	 * setting to adjust the prescaler as well.
 	 */
+	snprintf(clk_name, ARRAY_SIZE(clk_name), "%s-scaler",
+		 data->model_data->model_name);
 	data->clk_scaler = clk_hw_register_divider(
-				&pdev->dev, "scaler", "prescaler",
-				CLK_SET_RATE_PARENT,
-				data->base + ASPEED_REG_CLOCK_CONTROL,
-				0, 10, 0, &data->clk_lock);
+		&pdev->dev, clk_name, clk_parent_name, scaler_flags,
+		data->base + ASPEED_REG_CLOCK_CONTROL, 0,
+		data->model_data->scaler_bit_width, 0, &data->clk_lock);
 	if (IS_ERR(data->clk_scaler)) {
 		ret = PTR_ERR(data->clk_scaler);
 		goto scaler_error;
@@ -318,7 +327,8 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 reset_error:
 	clk_hw_unregister_divider(data->clk_scaler);
 scaler_error:
-	clk_hw_unregister_divider(data->clk_prescaler);
+	if (data->model_data->need_prescaler)
+		clk_hw_unregister_divider(data->clk_prescaler);
 	return ret;
 }
 
@@ -333,7 +343,8 @@ static int aspeed_adc_remove(struct platform_device *pdev)
 	clk_disable_unprepare(data->clk_scaler->clk);
 	reset_control_assert(data->rst);
 	clk_hw_unregister_divider(data->clk_scaler);
-	clk_hw_unregister_divider(data->clk_prescaler);
+	if (data->model_data->need_prescaler)
+		clk_hw_unregister_divider(data->clk_prescaler);
 
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 09/15] iio: adc: aspeed: Use devm_add_action_or_reset.
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
                   ` (7 preceding siblings ...)
  2021-08-31  7:14 ` [v5 08/15] iio: adc: aspeed: Use model_data to set clk scaler Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-08-31  7:51   ` Philipp Zabel
  2021-08-31  7:14 ` [v5 10/15] iio: adc: aspeed: Support ast2600 adc Billy Tsai
                   ` (5 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

This patch use devm_add_action_or_reset to handle the error in probe
phase.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 112 +++++++++++++++++++++--------------
 1 file changed, 66 insertions(+), 46 deletions(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index e53d1541ad1d..af00d9752c1e 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -195,6 +195,35 @@ static const struct iio_info aspeed_adc_iio_info = {
 	.debugfs_reg_access = aspeed_adc_reg_access,
 };
 
+static void aspeed_adc_unregister_divider(void *data)
+{
+	struct clk_hw *clk = data;
+
+	clk_hw_unregister_divider(clk);
+}
+
+static void aspeed_adc_reset_assert(void *data)
+{
+	struct reset_control *rst = data;
+
+	reset_control_assert(rst);
+}
+
+static void aspeed_adc_clk_disable_unprepare(void *data)
+{
+	struct clk *clk = data;
+
+	clk_disable_unprepare(clk);
+}
+
+static void aspeed_adc_power_down(void *data)
+{
+	struct aspeed_adc_data *priv_data = data;
+
+	writel(FIELD_PREP(ASPEED_ADC_OP_MODE, ASPEED_ADC_OP_MODE_PWR_DOWN),
+	       priv_data->base + ASPEED_REG_ENGINE_CONTROL);
+}
+
 static int aspeed_adc_vref_config(struct iio_dev *indio_dev)
 {
 	struct aspeed_adc_data *data = iio_priv(indio_dev);
@@ -242,6 +271,11 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 			&data->clk_lock);
 		if (IS_ERR(data->clk_prescaler))
 			return PTR_ERR(data->clk_prescaler);
+		ret = devm_add_action_or_reset(data->dev,
+					       aspeed_adc_unregister_divider,
+					       data->clk_prescaler);
+		if (ret)
+			return ret;
 		snprintf(clk_parent_name, ARRAY_SIZE(clk_parent_name),
 			 clk_name);
 		scaler_flags = CLK_SET_RATE_PARENT;
@@ -256,23 +290,30 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 		&pdev->dev, clk_name, clk_parent_name, scaler_flags,
 		data->base + ASPEED_REG_CLOCK_CONTROL, 0,
 		data->model_data->scaler_bit_width, 0, &data->clk_lock);
-	if (IS_ERR(data->clk_scaler)) {
-		ret = PTR_ERR(data->clk_scaler);
-		goto scaler_error;
-	}
+	if (IS_ERR(data->clk_scaler))
+		return PTR_ERR(data->clk_scaler);
+
+	ret = devm_add_action_or_reset(data->dev, aspeed_adc_unregister_divider,
+				       data->clk_scaler);
+	if (ret)
+		return ret;
 
 	data->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
 	if (IS_ERR(data->rst)) {
 		dev_err(&pdev->dev,
 			"invalid or missing reset controller device tree entry");
-		ret = PTR_ERR(data->rst);
-		goto reset_error;
+		return PTR_ERR(data->rst);
 	}
 	reset_control_deassert(data->rst);
 
+	ret = devm_add_action_or_reset(data->dev, aspeed_adc_reset_assert,
+				       data->rst);
+	if (ret)
+		return ret;
+
 	ret = aspeed_adc_vref_config(indio_dev);
 	if (ret)
-		goto vref_config_error;
+		return ret;
 
 	if (data->model_data->wait_init_sequence) {
 		/* Enable engine in normal mode. */
@@ -281,6 +322,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 			       ASPEED_ADC_ENGINE_ENABLE,
 		       data->base + ASPEED_REG_ENGINE_CONTROL);
 
+		ret = devm_add_action_or_reset(data->dev, aspeed_adc_power_down,
+					       data);
+		if (ret)
+			return ret;
 		/* Wait for initial sequence complete. */
 		ret = readl_poll_timeout(data->base + ASPEED_REG_ENGINE_CONTROL,
 					 adc_engine_control_reg_val,
@@ -289,13 +334,19 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 					 ASPEED_ADC_INIT_POLLING_TIME,
 					 ASPEED_ADC_INIT_TIMEOUT);
 		if (ret)
-			goto poll_timeout_error;
+			return ret;
 	}
 
 	/* Start all channels in normal mode. */
 	ret = clk_prepare_enable(data->clk_scaler->clk);
 	if (ret)
-		goto clk_enable_error;
+		return ret;
+
+	ret = devm_add_action_or_reset(data->dev,
+				       aspeed_adc_clk_disable_unprepare,
+				       data->clk_scaler->clk);
+	if (ret)
+		return ret;
 
 	adc_engine_control_reg_val =
 		ASPEED_ADC_CTRL_CHANNEL |
@@ -303,6 +354,11 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 		ASPEED_ADC_ENGINE_ENABLE;
 	writel(adc_engine_control_reg_val,
 	       data->base + ASPEED_REG_ENGINE_CONTROL);
+	ret = devm_add_action_or_reset(data->dev,
+				       aspeed_adc_power_down,
+				       data);
+	if (ret)
+		return ret;
 
 	indio_dev->name = data->model_data->model_name;
 	indio_dev->info = &aspeed_adc_iio_info;
@@ -310,45 +366,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	indio_dev->channels = aspeed_adc_iio_channels;
 	indio_dev->num_channels = data->model_data->num_channels;
 
-	ret = iio_device_register(indio_dev);
-	if (ret)
-		goto iio_register_error;
-
-	return 0;
-
-iio_register_error:
-	writel(FIELD_PREP(ASPEED_ADC_OP_MODE, ASPEED_ADC_OP_MODE_PWR_DOWN),
-	       data->base + ASPEED_REG_ENGINE_CONTROL);
-	clk_disable_unprepare(data->clk_scaler->clk);
-clk_enable_error:
-poll_timeout_error:
-vref_config_error:
-	reset_control_assert(data->rst);
-reset_error:
-	clk_hw_unregister_divider(data->clk_scaler);
-scaler_error:
-	if (data->model_data->need_prescaler)
-		clk_hw_unregister_divider(data->clk_prescaler);
+	ret = devm_iio_device_register(data->dev, indio_dev);
 	return ret;
 }
 
-static int aspeed_adc_remove(struct platform_device *pdev)
-{
-	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
-	struct aspeed_adc_data *data = iio_priv(indio_dev);
-
-	iio_device_unregister(indio_dev);
-	writel(FIELD_PREP(ASPEED_ADC_OP_MODE, ASPEED_ADC_OP_MODE_PWR_DOWN),
-	       data->base + ASPEED_REG_ENGINE_CONTROL);
-	clk_disable_unprepare(data->clk_scaler->clk);
-	reset_control_assert(data->rst);
-	clk_hw_unregister_divider(data->clk_scaler);
-	if (data->model_data->need_prescaler)
-		clk_hw_unregister_divider(data->clk_prescaler);
-
-	return 0;
-}
-
 static const struct aspeed_adc_model_data ast2400_model_data = {
 	.model_name = "ast2400-adc",
 	.vref_fixed_mv = 2500,
@@ -379,7 +400,6 @@ MODULE_DEVICE_TABLE(of, aspeed_adc_matches);
 
 static struct platform_driver aspeed_adc_driver = {
 	.probe = aspeed_adc_probe,
-	.remove = aspeed_adc_remove,
 	.driver = {
 		.name = KBUILD_MODNAME,
 		.of_match_table = aspeed_adc_matches,
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 10/15] iio: adc: aspeed: Support ast2600 adc.
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
                   ` (8 preceding siblings ...)
  2021-08-31  7:14 ` [v5 09/15] iio: adc: aspeed: Use devm_add_action_or_reset Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-08-31  7:14 ` [v5 11/15] iio: adc: aspeed: Fix the calculate error of clock Billy Tsai
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

Make driver to support ast2600 adc device.
- Use shared reset controller
- Complete the vref configure function
- Add the model data for ast2600 adc

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 105 +++++++++++++++++++++++++++++++++--
 1 file changed, 99 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index af00d9752c1e..40b7ba58c1dc 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Aspeed AST2400/2500 ADC
+ * Aspeed AST2400/2500/2600 ADC
  *
  * Copyright (C) 2017 Google, Inc.
  * Copyright (C) 2021 Aspeed Technology Inc.
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/of_platform.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 #include <linux/spinlock.h>
 #include <linux/types.h>
@@ -81,6 +82,7 @@ struct aspeed_adc_model_data {
 struct aspeed_adc_data {
 	struct device		*dev;
 	const struct aspeed_adc_model_data *model_data;
+	struct regulator	*regulator;
 	void __iomem		*base;
 	spinlock_t		clk_lock;
 	struct clk_hw		*clk_prescaler;
@@ -224,14 +226,79 @@ static void aspeed_adc_power_down(void *data)
 	       priv_data->base + ASPEED_REG_ENGINE_CONTROL);
 }
 
+static void aspeed_adc_reg_disable(void *data)
+{
+	struct regulator *reg = data;
+
+	regulator_disable(reg);
+}
+
 static int aspeed_adc_vref_config(struct iio_dev *indio_dev)
 {
 	struct aspeed_adc_data *data = iio_priv(indio_dev);
+	int ret;
+	u32 adc_engine_control_reg_val;
 
 	if (data->model_data->vref_fixed_mv) {
 		data->vref_mv = data->model_data->vref_fixed_mv;
 		return 0;
 	}
+	adc_engine_control_reg_val =
+		readl(data->base + ASPEED_REG_ENGINE_CONTROL);
+	data->regulator = devm_regulator_get_optional(data->dev, "vref");
+	if (!IS_ERR(data->regulator)) {
+		ret = regulator_enable(data->regulator);
+		if (ret)
+			return ret;
+		ret = devm_add_action_or_reset(
+			data->dev, aspeed_adc_reg_disable, data->regulator);
+		if (ret)
+			return ret;
+		data->vref_mv = regulator_get_voltage(data->regulator);
+		/* Conversion from uV to mV */
+		data->vref_mv /= 1000;
+		if ((data->vref_mv >= 1550) && (data->vref_mv <= 2700))
+			writel(adc_engine_control_reg_val |
+				FIELD_PREP(
+					ASPEED_ADC_REF_VOLTAGE,
+					ASPEED_ADC_REF_VOLTAGE_EXT_HIGH),
+			data->base + ASPEED_REG_ENGINE_CONTROL);
+		else if ((data->vref_mv >= 900) && (data->vref_mv <= 1650))
+			writel(adc_engine_control_reg_val |
+				FIELD_PREP(
+					ASPEED_ADC_REF_VOLTAGE,
+					ASPEED_ADC_REF_VOLTAGE_EXT_LOW),
+			data->base + ASPEED_REG_ENGINE_CONTROL);
+		else {
+			dev_err(data->dev, "Regulator voltage %d not support",
+				data->vref_mv);
+			return -EOPNOTSUPP;
+		}
+	} else {
+		if (PTR_ERR(data->regulator) != -ENODEV)
+			return PTR_ERR(data->regulator);
+		data->vref_mv = 2500000;
+		of_property_read_u32(data->dev->of_node,
+				     "aspeed,int-vref-microvolt",
+				     &data->vref_mv);
+		/* Conversion from uV to mV */
+		data->vref_mv /= 1000;
+		if (data->vref_mv == 2500)
+			writel(adc_engine_control_reg_val |
+				FIELD_PREP(ASPEED_ADC_REF_VOLTAGE,
+						ASPEED_ADC_REF_VOLTAGE_2500mV),
+			data->base + ASPEED_REG_ENGINE_CONTROL);
+		else if (data->vref_mv == 1200)
+			writel(adc_engine_control_reg_val |
+				FIELD_PREP(ASPEED_ADC_REF_VOLTAGE,
+						ASPEED_ADC_REF_VOLTAGE_1200mV),
+			data->base + ASPEED_REG_ENGINE_CONTROL);
+		else {
+			dev_err(data->dev, "Voltage %d not support", data->vref_mv);
+			return -EOPNOTSUPP;
+		}
+	}
+
 	return 0;
 }
 
@@ -298,7 +365,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	data->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	data->rst = devm_reset_control_get_shared(&pdev->dev, NULL);
 	if (IS_ERR(data->rst)) {
 		dev_err(&pdev->dev,
 			"invalid or missing reset controller device tree entry");
@@ -316,10 +383,14 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 		return ret;
 
 	if (data->model_data->wait_init_sequence) {
+		adc_engine_control_reg_val =
+			readl(data->base + ASPEED_REG_ENGINE_CONTROL);
+		adc_engine_control_reg_val |=
+			FIELD_PREP(ASPEED_ADC_OP_MODE,
+				   ASPEED_ADC_OP_MODE_NORMAL) |
+			ASPEED_ADC_ENGINE_ENABLE;
 		/* Enable engine in normal mode. */
-		writel(FIELD_PREP(ASPEED_ADC_OP_MODE,
-				  ASPEED_ADC_OP_MODE_NORMAL) |
-			       ASPEED_ADC_ENGINE_ENABLE,
+		writel(adc_engine_control_reg_val,
 		       data->base + ASPEED_REG_ENGINE_CONTROL);
 
 		ret = devm_add_action_or_reset(data->dev, aspeed_adc_power_down,
@@ -349,6 +420,8 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 		return ret;
 
 	adc_engine_control_reg_val =
+		readl(data->base + ASPEED_REG_ENGINE_CONTROL);
+	adc_engine_control_reg_val |=
 		ASPEED_ADC_CTRL_CHANNEL |
 		FIELD_PREP(ASPEED_ADC_OP_MODE, ASPEED_ADC_OP_MODE_NORMAL) |
 		ASPEED_ADC_ENGINE_ENABLE;
@@ -391,9 +464,29 @@ static const struct aspeed_adc_model_data ast2500_model_data = {
 	.num_channels = 16,
 };
 
+static const struct aspeed_adc_model_data ast2600_adc0_model_data = {
+	.model_name = "ast2600-adc0",
+	.min_sampling_rate = 10000,
+	.max_sampling_rate = 500000,
+	.wait_init_sequence = true,
+	.scaler_bit_width = 16,
+	.num_channels = 8,
+};
+
+static const struct aspeed_adc_model_data ast2600_adc1_model_data = {
+	.model_name = "ast2600-adc1",
+	.min_sampling_rate = 10000,
+	.max_sampling_rate = 500000,
+	.wait_init_sequence = true,
+	.scaler_bit_width = 16,
+	.num_channels = 8,
+};
+
 static const struct of_device_id aspeed_adc_matches[] = {
 	{ .compatible = "aspeed,ast2400-adc", .data = &ast2400_model_data },
 	{ .compatible = "aspeed,ast2500-adc", .data = &ast2500_model_data },
+	{ .compatible = "aspeed,ast2600-adc0", .data = &ast2600_adc0_model_data },
+	{ .compatible = "aspeed,ast2600-adc1", .data = &ast2600_adc1_model_data },
 	{},
 };
 MODULE_DEVICE_TABLE(of, aspeed_adc_matches);
@@ -409,5 +502,5 @@ static struct platform_driver aspeed_adc_driver = {
 module_platform_driver(aspeed_adc_driver);
 
 MODULE_AUTHOR("Rick Altherr <raltherr@google.com>");
-MODULE_DESCRIPTION("Aspeed AST2400/2500 ADC Driver");
+MODULE_DESCRIPTION("Aspeed AST2400/2500/2600 ADC Driver");
 MODULE_LICENSE("GPL");
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 11/15] iio: adc: aspeed: Fix the calculate error of clock.
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
                   ` (9 preceding siblings ...)
  2021-08-31  7:14 ` [v5 10/15] iio: adc: aspeed: Support ast2600 adc Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-09-05 14:47   ` Jonathan Cameron
  2021-08-31  7:14 ` [v5 12/15] iio: adc: aspeed: Add func to set sampling rate Billy Tsai
                   ` (3 subsequent siblings)
  14 siblings, 1 reply; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

The ADC clock formula is
ast2400/2500:
ADC clock period = PCLK * 2 * (ADC0C[31:17] + 1) * (ADC0C[9:0] + 1)
ast2600:
ADC clock period = PCLK * 2 * (ADC0C[15:0] + 1)
They all have one fixed divided 2 and the legacy driver didn't handle it.
This patch register the fixed factory clock device as the parent of ADC
clock scaler to fix this issue.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index 40b7ba58c1dc..349377b9fac0 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -4,6 +4,12 @@
  *
  * Copyright (C) 2017 Google, Inc.
  * Copyright (C) 2021 Aspeed Technology Inc.
+ *
+ * ADC clock formula:
+ * Ast2400/Ast2500:
+ * clock period = period of PCLK * 2 * (ADC0C[31:17] + 1) * (ADC0C[9:0] + 1)
+ * Ast2600:
+ * clock period = period of PCLK * 2 * (ADC0C[15:0] + 1)
  */
 
 #include <linux/clk.h>
@@ -85,6 +91,7 @@ struct aspeed_adc_data {
 	struct regulator	*regulator;
 	void __iomem		*base;
 	spinlock_t		clk_lock;
+	struct clk_hw		*fixed_div_clk;
 	struct clk_hw		*clk_prescaler;
 	struct clk_hw		*clk_scaler;
 	struct reset_control	*rst;
@@ -204,6 +211,13 @@ static void aspeed_adc_unregister_divider(void *data)
 	clk_hw_unregister_divider(clk);
 }
 
+static void aspeed_adc_unregister_fixed_divider(void *data)
+{
+	struct clk_hw *clk = data;
+
+	clk_hw_unregister_fixed_factor(clk);
+}
+
 static void aspeed_adc_reset_assert(void *data)
 {
 	struct reset_control *rst = data;
@@ -328,6 +342,19 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	spin_lock_init(&data->clk_lock);
 	snprintf(clk_parent_name, ARRAY_SIZE(clk_parent_name), "%s",
 		 of_clk_get_parent_name(pdev->dev.of_node, 0));
+	snprintf(clk_name, ARRAY_SIZE(clk_name), "%s-fixed-div",
+		 data->model_data->model_name);
+	data->fixed_div_clk = clk_hw_register_fixed_factor(
+		&pdev->dev, clk_name, clk_parent_name, 0, 1, 2);
+	if (IS_ERR(data->fixed_div_clk))
+		return PTR_ERR(data->fixed_div_clk);
+
+	ret = devm_add_action_or_reset(data->dev,
+				       aspeed_adc_unregister_fixed_divider,
+				       data->clk_prescaler);
+	if (ret)
+		return ret;
+	snprintf(clk_parent_name, ARRAY_SIZE(clk_parent_name), clk_name);
 
 	if (data->model_data->need_prescaler) {
 		snprintf(clk_name, ARRAY_SIZE(clk_name), "%s-prescaler",
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 12/15] iio: adc: aspeed: Add func to set sampling rate.
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
                   ` (10 preceding siblings ...)
  2021-08-31  7:14 ` [v5 11/15] iio: adc: aspeed: Fix the calculate error of clock Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-08-31  7:14 ` [v5 13/15] iio: adc: aspeed: Add compensation phase Billy Tsai
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

Add the function to set the sampling rate and keep the sampling period
for a driver used to wait the fresh value.
In addition, since the ADC clock is required when initializing the ADC
device, move clk_prepare_enable ahead of the initialization phase.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 59 ++++++++++++++++++++++++------------
 1 file changed, 40 insertions(+), 19 deletions(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index 349377b9fac0..1333d7a88427 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -73,6 +73,12 @@
 
 #define ASPEED_ADC_INIT_POLLING_TIME	500
 #define ASPEED_ADC_INIT_TIMEOUT		500000
+/*
+ * When the sampling rate is too high, the ADC may not have enough charging
+ * time, resulting in a low voltage value. Thus, the default uses a slow
+ * sampling rate for most use cases.
+ */
+#define ASPEED_ADC_DEF_SAMPLING_RATE	65000
 
 struct aspeed_adc_model_data {
 	const char *model_name;
@@ -96,6 +102,7 @@ struct aspeed_adc_data {
 	struct clk_hw		*clk_scaler;
 	struct reset_control	*rst;
 	int			vref_mv;
+	u32			sample_period_ns;
 };
 
 #define ASPEED_CHAN(_idx, _data_reg_addr) {			\
@@ -127,6 +134,24 @@ static const struct iio_chan_spec aspeed_adc_iio_channels[] = {
 	ASPEED_CHAN(15, 0x2E),
 };
 
+static int aspeed_adc_set_sampling_rate(struct iio_dev *indio_dev, u32 rate)
+{
+	struct aspeed_adc_data *data = iio_priv(indio_dev);
+
+	if (rate < data->model_data->min_sampling_rate ||
+	    rate > data->model_data->max_sampling_rate)
+		return -EINVAL;
+	/* Each sampling needs 12 clocks to convert.*/
+	clk_set_rate(data->clk_scaler->clk, rate * ASPEED_CLOCKS_PER_SAMPLE);
+	rate = clk_get_rate(data->clk_scaler->clk);
+	data->sample_period_ns = DIV_ROUND_UP_ULL(
+		(u64)NSEC_PER_SEC * ASPEED_CLOCKS_PER_SAMPLE, rate);
+	dev_dbg(data->dev, "Adc clock = %d sample period = %d ns", rate,
+		data->sample_period_ns);
+
+	return 0;
+}
+
 static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
 			       struct iio_chan_spec const *chan,
 			       int *val, int *val2, long mask)
@@ -157,17 +182,9 @@ static int aspeed_adc_write_raw(struct iio_dev *indio_dev,
 				struct iio_chan_spec const *chan,
 				int val, int val2, long mask)
 {
-	struct aspeed_adc_data *data = iio_priv(indio_dev);
-
 	switch (mask) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		if (val < data->model_data->min_sampling_rate ||
-			val > data->model_data->max_sampling_rate)
-			return -EINVAL;
-
-		clk_set_rate(data->clk_scaler->clk,
-				val * ASPEED_CLOCKS_PER_SAMPLE);
-		return 0;
+		return aspeed_adc_set_sampling_rate(indio_dev, val);
 
 	case IIO_CHAN_INFO_SCALE:
 	case IIO_CHAN_INFO_RAW:
@@ -409,6 +426,20 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	ret = clk_prepare_enable(data->clk_scaler->clk);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(data->dev,
+				       aspeed_adc_clk_disable_unprepare,
+				       data->clk_scaler->clk);
+	if (ret)
+		return ret;
+
+	ret = aspeed_adc_set_sampling_rate(indio_dev, ASPEED_ADC_DEF_SAMPLING_RATE);
+	if (ret)
+		return ret;
+
 	if (data->model_data->wait_init_sequence) {
 		adc_engine_control_reg_val =
 			readl(data->base + ASPEED_REG_ENGINE_CONTROL);
@@ -436,16 +467,6 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	}
 
 	/* Start all channels in normal mode. */
-	ret = clk_prepare_enable(data->clk_scaler->clk);
-	if (ret)
-		return ret;
-
-	ret = devm_add_action_or_reset(data->dev,
-				       aspeed_adc_clk_disable_unprepare,
-				       data->clk_scaler->clk);
-	if (ret)
-		return ret;
-
 	adc_engine_control_reg_val =
 		readl(data->base + ASPEED_REG_ENGINE_CONTROL);
 	adc_engine_control_reg_val |=
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 13/15] iio: adc: aspeed: Add compensation phase.
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
                   ` (11 preceding siblings ...)
  2021-08-31  7:14 ` [v5 12/15] iio: adc: aspeed: Add func to set sampling rate Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-08-31  7:14 ` [v5 14/15] iio: adc: aspeed: Support battery sensing Billy Tsai
  2021-08-31  7:14 ` [v5 15/15] iio: adc: aspeed: Get and set trimming data Billy Tsai
  14 siblings, 0 replies; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

This patch adds a compensation phase to improve the accuracy of ADC
measurement. This is the built-in function through input half of the
reference voltage to get the ADC offset.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 54 +++++++++++++++++++++++++++++++++++-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index 1333d7a88427..121c96de03bd 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -103,6 +103,7 @@ struct aspeed_adc_data {
 	struct reset_control	*rst;
 	int			vref_mv;
 	u32			sample_period_ns;
+	int			cv;
 };
 
 #define ASPEED_CHAN(_idx, _data_reg_addr) {			\
@@ -112,7 +113,8 @@ struct aspeed_adc_data {
 	.address = (_data_reg_addr),				\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
-				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+				BIT(IIO_CHAN_INFO_SAMP_FREQ) |	\
+				BIT(IIO_CHAN_INFO_OFFSET),	\
 }
 
 static const struct iio_chan_spec aspeed_adc_iio_channels[] = {
@@ -134,6 +136,51 @@ static const struct iio_chan_spec aspeed_adc_iio_channels[] = {
 	ASPEED_CHAN(15, 0x2E),
 };
 
+static int aspeed_adc_compensation(struct iio_dev *indio_dev)
+{
+	struct aspeed_adc_data *data = iio_priv(indio_dev);
+	u32 index, adc_raw = 0;
+	u32 adc_engine_control_reg_val;
+
+	adc_engine_control_reg_val =
+		readl(data->base + ASPEED_REG_ENGINE_CONTROL);
+	adc_engine_control_reg_val &= ~ASPEED_ADC_OP_MODE;
+	adc_engine_control_reg_val |=
+		(FIELD_PREP(ASPEED_ADC_OP_MODE, ASPEED_ADC_OP_MODE_NORMAL) |
+		 ASPEED_ADC_ENGINE_ENABLE);
+	/*
+	 * Enable compensating sensing:
+	 * After that, the input voltage of ADC will force to half of the reference
+	 * voltage. So the expected reading raw data will become half of the max
+	 * value. We can get compensating value = 0x200 - ADC read raw value.
+	 * It is recommended to average at least 10 samples to get a final CV.
+	 */
+	writel(adc_engine_control_reg_val | ASPEED_ADC_CTRL_COMPENSATION |
+		       ASPEED_ADC_CTRL_CHANNEL_ENABLE(0),
+	       data->base + ASPEED_REG_ENGINE_CONTROL);
+	/*
+	 * After enable compensating sensing mode need to wait some time for ADC stable
+	 * Experiment result is 1ms.
+	 */
+	mdelay(1);
+
+	for (index = 0; index < 16; index++) {
+		/*
+		 * Waiting for the sampling period ensures that the value acquired
+		 * is fresh each time.
+		 */
+		ndelay(data->sample_period_ns);
+		adc_raw += readw(data->base + aspeed_adc_iio_channels[0].address);
+	}
+	adc_raw >>= 4;
+	data->cv = BIT(ASPEED_RESOLUTION_BITS - 1) - adc_raw;
+	writel(adc_engine_control_reg_val,
+	       data->base + ASPEED_REG_ENGINE_CONTROL);
+	dev_dbg(data->dev, "Compensating value = %d\n", data->cv);
+
+	return 0;
+}
+
 static int aspeed_adc_set_sampling_rate(struct iio_dev *indio_dev, u32 rate)
 {
 	struct aspeed_adc_data *data = iio_priv(indio_dev);
@@ -163,6 +210,10 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
 		*val = readw(data->base + chan->address);
 		return IIO_VAL_INT;
 
+	case IIO_CHAN_INFO_OFFSET:
+		*val = data->cv;
+		return IIO_VAL_INT;
+
 	case IIO_CHAN_INFO_SCALE:
 		*val = data->vref_mv;
 		*val2 = ASPEED_RESOLUTION_BITS;
@@ -466,6 +517,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	aspeed_adc_compensation(indio_dev);
 	/* Start all channels in normal mode. */
 	adc_engine_control_reg_val =
 		readl(data->base + ASPEED_REG_ENGINE_CONTROL);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 14/15] iio: adc: aspeed: Support battery sensing.
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
                   ` (12 preceding siblings ...)
  2021-08-31  7:14 ` [v5 13/15] iio: adc: aspeed: Add compensation phase Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  2021-08-31  7:14 ` [v5 15/15] iio: adc: aspeed: Get and set trimming data Billy Tsai
  14 siblings, 0 replies; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

In ast2600, ADC integrate dividing circuit at last input channel for
battery sensing. This patch use the dts property "battery-sensing" to
enable this feature makes the last channel of each adc can tolerance
higher voltage than reference voltage. The offset interface of ch7 will
be separated when enabling the battery sensing mode.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 81 ++++++++++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index 121c96de03bd..b594a723db2e 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -87,10 +87,16 @@ struct aspeed_adc_model_data {
 	unsigned int vref_fixed_mv;
 	bool wait_init_sequence;
 	bool need_prescaler;
+	bool bat_sense_sup;
 	u8 scaler_bit_width;
 	unsigned int num_channels;
 };
 
+struct adc_gain {
+	u8 mult;
+	u8 div;
+};
+
 struct aspeed_adc_data {
 	struct device		*dev;
 	const struct aspeed_adc_model_data *model_data;
@@ -104,6 +110,8 @@ struct aspeed_adc_data {
 	int			vref_mv;
 	u32			sample_period_ns;
 	int			cv;
+	bool			battery_sensing;
+	struct adc_gain		battery_mode_gain;
 };
 
 #define ASPEED_CHAN(_idx, _data_reg_addr) {			\
@@ -136,6 +144,27 @@ static const struct iio_chan_spec aspeed_adc_iio_channels[] = {
 	ASPEED_CHAN(15, 0x2E),
 };
 
+#define ASPEED_BAT_CHAN(_idx, _data_reg_addr) {					\
+		.type = IIO_VOLTAGE,						\
+		.indexed = 1,							\
+		.channel = (_idx),						\
+		.address = (_data_reg_addr),					\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
+				      BIT(IIO_CHAN_INFO_OFFSET),		\
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |		\
+					    BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+}
+static const struct iio_chan_spec aspeed_adc_iio_bat_channels[] = {
+	ASPEED_CHAN(0, 0x10),
+	ASPEED_CHAN(1, 0x12),
+	ASPEED_CHAN(2, 0x14),
+	ASPEED_CHAN(3, 0x16),
+	ASPEED_CHAN(4, 0x18),
+	ASPEED_CHAN(5, 0x1A),
+	ASPEED_CHAN(6, 0x1C),
+	ASPEED_BAT_CHAN(7, 0x1E),
+};
+
 static int aspeed_adc_compensation(struct iio_dev *indio_dev)
 {
 	struct aspeed_adc_data *data = iio_priv(indio_dev);
@@ -204,14 +233,39 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
 			       int *val, int *val2, long mask)
 {
 	struct aspeed_adc_data *data = iio_priv(indio_dev);
+	u32 adc_engine_control_reg_val;
 
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
-		*val = readw(data->base + chan->address);
+		if (data->battery_sensing && chan->channel == 7) {
+			adc_engine_control_reg_val =
+				readl(data->base + ASPEED_REG_ENGINE_CONTROL);
+			writel(adc_engine_control_reg_val |
+				       FIELD_PREP(ASPEED_ADC_CH7_MODE,
+						  ASPEED_ADC_CH7_BAT) |
+				       ASPEED_ADC_BAT_SENSING_ENABLE,
+			       data->base + ASPEED_REG_ENGINE_CONTROL);
+			/*
+			 * After enable battery sensing mode need to wait some time for adc stable
+			 * Experiment result is 1ms.
+			 */
+			mdelay(1);
+			*val = readw(data->base + chan->address);
+			*val = (*val * data->battery_mode_gain.mult) /
+			       data->battery_mode_gain.div;
+			/* Restore control register value */
+			writel(adc_engine_control_reg_val,
+			       data->base + ASPEED_REG_ENGINE_CONTROL);
+		} else
+			*val = readw(data->base + chan->address);
 		return IIO_VAL_INT;
 
 	case IIO_CHAN_INFO_OFFSET:
-		*val = data->cv;
+		if (data->battery_sensing && chan->channel == 7)
+			*val = (data->cv * data->battery_mode_gain.mult) /
+			       data->battery_mode_gain.div;
+		else
+			*val = data->cv;
 		return IIO_VAL_INT;
 
 	case IIO_CHAN_INFO_SCALE:
@@ -491,6 +545,23 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	if (of_find_property(data->dev->of_node, "aspeed,battery-sensing",
+			     NULL)) {
+		if (data->model_data->bat_sense_sup) {
+			data->battery_sensing = 1;
+			if (readl(data->base + ASPEED_REG_ENGINE_CONTROL) &
+			ASPEED_ADC_BAT_SENSING_DIV) {
+				data->battery_mode_gain.mult = 3;
+				data->battery_mode_gain.div = 1;
+			} else {
+				data->battery_mode_gain.mult = 3;
+				data->battery_mode_gain.div = 2;
+			}
+		} else
+			dev_warn(&pdev->dev,
+				"Failed to enable battey-sensing mode\n");
+	}
+
 	if (data->model_data->wait_init_sequence) {
 		adc_engine_control_reg_val =
 			readl(data->base + ASPEED_REG_ENGINE_CONTROL);
@@ -536,7 +607,9 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	indio_dev->name = data->model_data->model_name;
 	indio_dev->info = &aspeed_adc_iio_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
-	indio_dev->channels = aspeed_adc_iio_channels;
+	indio_dev->channels = data->battery_sensing ?
+					    aspeed_adc_iio_bat_channels :
+					    aspeed_adc_iio_channels;
 	indio_dev->num_channels = data->model_data->num_channels;
 
 	ret = devm_iio_device_register(data->dev, indio_dev);
@@ -569,6 +642,7 @@ static const struct aspeed_adc_model_data ast2600_adc0_model_data = {
 	.min_sampling_rate = 10000,
 	.max_sampling_rate = 500000,
 	.wait_init_sequence = true,
+	.bat_sense_sup = true,
 	.scaler_bit_width = 16,
 	.num_channels = 8,
 };
@@ -578,6 +652,7 @@ static const struct aspeed_adc_model_data ast2600_adc1_model_data = {
 	.min_sampling_rate = 10000,
 	.max_sampling_rate = 500000,
 	.wait_init_sequence = true,
+	.bat_sense_sup = true,
 	.scaler_bit_width = 16,
 	.num_channels = 8,
 };
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* [v5 15/15] iio: adc: aspeed: Get and set trimming data.
  2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
                   ` (13 preceding siblings ...)
  2021-08-31  7:14 ` [v5 14/15] iio: adc: aspeed: Support battery sensing Billy Tsai
@ 2021-08-31  7:14 ` Billy Tsai
  14 siblings, 0 replies; 27+ messages in thread
From: Billy Tsai @ 2021-08-31  7:14 UTC (permalink / raw)
  To: jic23, lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood,
	broonie, linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel
  Cc: BMC-SW

The ADC controller has a trimming register for fine-tune the reference
voltage. The trimming value comes from the OTP register which will be
written during chip production. This patch will read this OTP value and
configure it to the ADC register when the ADC controller probes and using
dts property "aspeed,trim-data-valid" to determine whether to execute this
flow.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 drivers/iio/adc/aspeed_adc.c | 71 ++++++++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index b594a723db2e..fddd7508099f 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -25,6 +25,8 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/bitfield.h>
+#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/driver.h>
@@ -80,6 +82,11 @@
  */
 #define ASPEED_ADC_DEF_SAMPLING_RATE	65000
 
+struct aspeed_adc_trim_locate {
+	const unsigned int offset;
+	const unsigned int field;
+};
+
 struct aspeed_adc_model_data {
 	const char *model_name;
 	unsigned int min_sampling_rate;	// Hz
@@ -90,6 +97,7 @@ struct aspeed_adc_model_data {
 	bool bat_sense_sup;
 	u8 scaler_bit_width;
 	unsigned int num_channels;
+	const struct aspeed_adc_trim_locate *trim_locate;
 };
 
 struct adc_gain {
@@ -165,6 +173,44 @@ static const struct iio_chan_spec aspeed_adc_iio_bat_channels[] = {
 	ASPEED_BAT_CHAN(7, 0x1E),
 };
 
+static int aspeed_adc_set_trim_data(struct iio_dev *indio_dev)
+{
+	struct device_node *syscon;
+	struct regmap *scu;
+	u32 scu_otp, trimming_val;
+	struct aspeed_adc_data *data = iio_priv(indio_dev);
+
+	syscon = of_find_node_by_name(NULL, "syscon");
+	if (syscon == NULL) {
+		dev_warn(data->dev, "Couldn't find syscon node\n");
+		return -EOPNOTSUPP;
+	}
+	scu = syscon_node_to_regmap(syscon);
+	if (IS_ERR(scu)) {
+		dev_warn(data->dev, "Failed to get syscon regmap\n");
+		return -EOPNOTSUPP;
+	}
+	if (data->model_data->trim_locate) {
+		if (regmap_read(scu, data->model_data->trim_locate->offset,
+				&scu_otp)) {
+			dev_warn(data->dev,
+				 "Failed to get adc trimming data\n");
+			trimming_val = 0x8;
+		} else {
+			trimming_val =
+				((scu_otp) &
+				 (data->model_data->trim_locate->field)) >>
+				__ffs(data->model_data->trim_locate->field);
+		}
+		dev_dbg(data->dev,
+			"trimming val = %d, offset = %08x, fields = %08x\n",
+			trimming_val, data->model_data->trim_locate->offset,
+			data->model_data->trim_locate->field);
+		writel(trimming_val, data->base + ASPEED_REG_COMPENSATION_TRIM);
+	}
+	return 0;
+}
+
 static int aspeed_adc_compensation(struct iio_dev *indio_dev)
 {
 	struct aspeed_adc_data *data = iio_priv(indio_dev);
@@ -545,6 +591,13 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	if (of_find_property(data->dev->of_node, "aspeed,trim-data-valid",
+			     NULL)) {
+		ret = aspeed_adc_set_trim_data(indio_dev);
+		if (ret)
+			return ret;
+	}
+
 	if (of_find_property(data->dev->of_node, "aspeed,battery-sensing",
 			     NULL)) {
 		if (data->model_data->bat_sense_sup) {
@@ -616,6 +669,21 @@ static int aspeed_adc_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static const struct aspeed_adc_trim_locate ast2500_adc_trim = {
+	.offset = 0x154,
+	.field = GENMASK(31, 28),
+};
+
+static const struct aspeed_adc_trim_locate ast2600_adc0_trim = {
+	.offset = 0x5d0,
+	.field = GENMASK(3, 0),
+};
+
+static const struct aspeed_adc_trim_locate ast2600_adc1_trim = {
+	.offset = 0x5d0,
+	.field = GENMASK(7, 4),
+};
+
 static const struct aspeed_adc_model_data ast2400_model_data = {
 	.model_name = "ast2400-adc",
 	.vref_fixed_mv = 2500,
@@ -635,6 +703,7 @@ static const struct aspeed_adc_model_data ast2500_model_data = {
 	.need_prescaler = true,
 	.scaler_bit_width = 10,
 	.num_channels = 16,
+	.trim_locate = &ast2500_adc_trim,
 };
 
 static const struct aspeed_adc_model_data ast2600_adc0_model_data = {
@@ -645,6 +714,7 @@ static const struct aspeed_adc_model_data ast2600_adc0_model_data = {
 	.bat_sense_sup = true,
 	.scaler_bit_width = 16,
 	.num_channels = 8,
+	.trim_locate = &ast2600_adc0_trim,
 };
 
 static const struct aspeed_adc_model_data ast2600_adc1_model_data = {
@@ -655,6 +725,7 @@ static const struct aspeed_adc_model_data ast2600_adc1_model_data = {
 	.bat_sense_sup = true,
 	.scaler_bit_width = 16,
 	.num_channels = 8,
+	.trim_locate = &ast2600_adc1_trim,
 };
 
 static const struct of_device_id aspeed_adc_matches[] = {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [v5 09/15] iio: adc: aspeed: Use devm_add_action_or_reset.
  2021-08-31  7:14 ` [v5 09/15] iio: adc: aspeed: Use devm_add_action_or_reset Billy Tsai
@ 2021-08-31  7:51   ` Philipp Zabel
  0 siblings, 0 replies; 27+ messages in thread
From: Philipp Zabel @ 2021-08-31  7:51 UTC (permalink / raw)
  To: Billy Tsai, jic23, lars, pmeerw, robh+dt, joel, andrew,
	lgirdwood, broonie, linux-iio, devicetree, linux-arm-kernel,
	linux-aspeed, linux-kernel
  Cc: BMC-SW

Hi Billy,

On Tue, 2021-08-31 at 15:14 +0800, Billy Tsai wrote:
> This patch use devm_add_action_or_reset to handle the error in probe
> phase.
> 
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
>  drivers/iio/adc/aspeed_adc.c | 112 +++++++++++++++++++++--------------
>  1 file changed, 66 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
> index e53d1541ad1d..af00d9752c1e 100644
> --- a/drivers/iio/adc/aspeed_adc.c
> +++ b/drivers/iio/adc/aspeed_adc.c
[...]
> @@ -242,6 +271,11 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  			&data->clk_lock);
>  		if (IS_ERR(data->clk_prescaler))
>  			return PTR_ERR(data->clk_prescaler);
> +		ret = devm_add_action_or_reset(data->dev,
> +					       aspeed_adc_unregister_divider,
> +					       data->clk_prescaler);
> +		if (ret)
> +			return ret;

I think here you could switch to devm_clk_hw_register_divider() instead.


>  		snprintf(clk_parent_name, ARRAY_SIZE(clk_parent_name),
>  			 clk_name);
>  		scaler_flags = CLK_SET_RATE_PARENT;
> @@ -256,23 +290,30 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  		&pdev->dev, clk_name, clk_parent_name, scaler_flags,
>  		data->base + ASPEED_REG_CLOCK_CONTROL, 0,
>  		data->model_data->scaler_bit_width, 0, &data->clk_lock);
> -	if (IS_ERR(data->clk_scaler)) {
> -		ret = PTR_ERR(data->clk_scaler);
> -		goto scaler_error;
> -	}
> +	if (IS_ERR(data->clk_scaler))
> +		return PTR_ERR(data->clk_scaler);
> +
> +	ret = devm_add_action_or_reset(data->dev, aspeed_adc_unregister_divider,
> +				       data->clk_scaler);
> +	if (ret)
> +		return ret;

Same as above.

[...]
> @@ -281,6 +322,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  			       ASPEED_ADC_ENGINE_ENABLE,
>  		       data->base + ASPEED_REG_ENGINE_CONTROL);
>  
> +		ret = devm_add_action_or_reset(data->dev, aspeed_adc_power_down,
> +					       data);
> +		if (ret)
> +			return ret;
>  		/* Wait for initial sequence complete. */
>  		ret = readl_poll_timeout(data->base + ASPEED_REG_ENGINE_CONTROL,
>  					 adc_engine_control_reg_val,
[...]
> @@ -303,6 +354,11 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  		ASPEED_ADC_ENGINE_ENABLE;
>  	writel(adc_engine_control_reg_val,
>  	       data->base + ASPEED_REG_ENGINE_CONTROL);
> +	ret = devm_add_action_or_reset(data->dev,
> +				       aspeed_adc_power_down,
> +				       data);
> +	if (ret)
> +		return ret;

Should this be if (!model_dta->wait_init_sequence) ? Otherwise it looks
like you've already registered the same aspeed_adc_power_down action
above.

regards
Philipp

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [v5 02/15] dt-bindings: iio: adc: Add ast2600-adc bindings
  2021-08-31  7:14 ` [v5 02/15] dt-bindings: iio: adc: Add ast2600-adc bindings Billy Tsai
@ 2021-09-01  1:38   ` Rob Herring
  2021-09-05 14:27     ` Jonathan Cameron
  0 siblings, 1 reply; 27+ messages in thread
From: Rob Herring @ 2021-09-01  1:38 UTC (permalink / raw)
  To: Billy Tsai
  Cc: robh+dt, linux-aspeed, p.zabel, pmeerw, lars, linux-iio, broonie,
	linux-kernel, linux-arm-kernel, jic23, andrew, joel, devicetree,
	BMC-SW, lgirdwood

On Tue, 31 Aug 2021 15:14:45 +0800, Billy Tsai wrote:
> Add device tree bindings document for the aspeed ast2600 adc device
> driver.
> 
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
>  .../bindings/iio/adc/aspeed,ast2600-adc.yaml  | 100 ++++++++++++++++++
>  1 file changed, 100 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/aspeed,ast2600-adc.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [v5 01/15] iio: adc: aspeed: set driver data when adc probe.
  2021-08-31  7:14 ` [v5 01/15] iio: adc: aspeed: set driver data when adc probe Billy Tsai
@ 2021-09-05 14:25   ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2021-09-05 14:25 UTC (permalink / raw)
  To: Billy Tsai
  Cc: lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood, broonie,
	linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

On Tue, 31 Aug 2021 15:14:44 +0800
Billy Tsai <billy_tsai@aspeedtech.com> wrote:

> Fix the issue when adc remove will get the null driver data.
> 
> Fixed: commit 573803234e72 ("iio: Aspeed ADC")
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
Thanks Billy

Applied to the fixes-togreg branch of iio.git and marked for stable.

Jonathan

> ---
>  drivers/iio/adc/aspeed_adc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
> index 19efaa41bc34..34ec0c28b2df 100644
> --- a/drivers/iio/adc/aspeed_adc.c
> +++ b/drivers/iio/adc/aspeed_adc.c
> @@ -183,6 +183,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  
>  	data = iio_priv(indio_dev);
>  	data->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, indio_dev);
>  
>  	data->base = devm_platform_ioremap_resource(pdev, 0);
>  	if (IS_ERR(data->base))


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [v5 02/15] dt-bindings: iio: adc: Add ast2600-adc bindings
  2021-09-01  1:38   ` Rob Herring
@ 2021-09-05 14:27     ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2021-09-05 14:27 UTC (permalink / raw)
  To: Rob Herring
  Cc: Billy Tsai, robh+dt, linux-aspeed, p.zabel, pmeerw, lars,
	linux-iio, broonie, linux-kernel, linux-arm-kernel, andrew, joel,
	devicetree, BMC-SW, lgirdwood

On Tue, 31 Aug 2021 20:38:36 -0500
Rob Herring <robh@kernel.org> wrote:

> On Tue, 31 Aug 2021 15:14:45 +0800, Billy Tsai wrote:
> > Add device tree bindings document for the aspeed ast2600 adc device
> > driver.
> > 
> > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> > ---
> >  .../bindings/iio/adc/aspeed,ast2600-adc.yaml  | 100 ++++++++++++++++++
> >  1 file changed, 100 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/aspeed,ast2600-adc.yaml
> >   
> 
> Reviewed-by: Rob Herring <robh@kernel.org>

I'm going to push on with this series until I hit anything that needs to wait
for the fix to be available.  If that happens I'll have to wait until that's
in Linus' tree before taking the rest.

Applied to the togreg branch of iio.git and pushed out as testing for 0-day
to work it's magic.

Thanks,

Jonathan



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [v5 03/15] iio: adc: aspeed: completes the bitfield declare.
  2021-08-31  7:14 ` [v5 03/15] iio: adc: aspeed: completes the bitfield declare Billy Tsai
@ 2021-09-05 14:30   ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2021-09-05 14:30 UTC (permalink / raw)
  To: Billy Tsai
  Cc: lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood, broonie,
	linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

On Tue, 31 Aug 2021 15:14:46 +0800
Billy Tsai <billy_tsai@aspeedtech.com> wrote:

> This patch completes the declare of ADC register bitfields and uses the
> same prefix ASPEED_ADC_* for these bitfields. In addition, tidy up space
> alignment of the codes.
> 
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
LGTM

Applied to the togreg branch of iio.git.

Note this won't be going out as non rebasing for a while yet (given mid merge window)
so if anyone else has time to look at this that would be much appreciated!

Jonathan

> ---
>  drivers/iio/adc/aspeed_adc.c | 64 ++++++++++++++++++++++++++----------
>  1 file changed, 47 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
> index 34ec0c28b2df..f055fe7b2c40 100644
> --- a/drivers/iio/adc/aspeed_adc.c
> +++ b/drivers/iio/adc/aspeed_adc.c
> @@ -3,6 +3,7 @@
>   * Aspeed AST2400/2500 ADC
>   *
>   * Copyright (C) 2017 Google, Inc.
> + * Copyright (C) 2021 Aspeed Technology Inc.
>   */
>  
>  #include <linux/clk.h>
> @@ -16,6 +17,7 @@
>  #include <linux/reset.h>
>  #include <linux/spinlock.h>
>  #include <linux/types.h>
> +#include <linux/bitfield.h>
>  
>  #include <linux/iio/iio.h>
>  #include <linux/iio/driver.h>
> @@ -28,15 +30,39 @@
>  #define ASPEED_REG_INTERRUPT_CONTROL	0x04
>  #define ASPEED_REG_VGA_DETECT_CONTROL	0x08
>  #define ASPEED_REG_CLOCK_CONTROL	0x0C
> -#define ASPEED_REG_MAX			0xC0
> -
> -#define ASPEED_OPERATION_MODE_POWER_DOWN	(0x0 << 1)
> -#define ASPEED_OPERATION_MODE_STANDBY		(0x1 << 1)
> -#define ASPEED_OPERATION_MODE_NORMAL		(0x7 << 1)
> -
> -#define ASPEED_ENGINE_ENABLE		BIT(0)
> -
> -#define ASPEED_ADC_CTRL_INIT_RDY	BIT(8)
> +#define ASPEED_REG_COMPENSATION_TRIM	0xC4
> +/*
> + * The register offset between 0xC8~0xCC can be read and won't affect the
> + * hardware logic in each version of ADC.
> + */
> +#define ASPEED_REG_MAX			0xD0
> +
> +#define ASPEED_ADC_ENGINE_ENABLE		BIT(0)
> +#define ASPEED_ADC_OP_MODE			GENMASK(3, 1)
> +#define ASPEED_ADC_OP_MODE_PWR_DOWN		0
> +#define ASPEED_ADC_OP_MODE_STANDBY		1
> +#define ASPEED_ADC_OP_MODE_NORMAL		7
> +#define ASPEED_ADC_CTRL_COMPENSATION		BIT(4)
> +#define ASPEED_ADC_AUTO_COMPENSATION		BIT(5)
> +/*
> + * Bit 6 determines not only the reference voltage range but also the dividing
> + * circuit for battery sensing.
> + */
> +#define ASPEED_ADC_REF_VOLTAGE			GENMASK(7, 6)
> +#define ASPEED_ADC_REF_VOLTAGE_2500mV		0
> +#define ASPEED_ADC_REF_VOLTAGE_1200mV		1
> +#define ASPEED_ADC_REF_VOLTAGE_EXT_HIGH		2
> +#define ASPEED_ADC_REF_VOLTAGE_EXT_LOW		3
> +#define ASPEED_ADC_BAT_SENSING_DIV		BIT(6)
> +#define ASPEED_ADC_BAT_SENSING_DIV_2_3		0
> +#define ASPEED_ADC_BAT_SENSING_DIV_1_3		1
> +#define ASPEED_ADC_CTRL_INIT_RDY		BIT(8)
> +#define ASPEED_ADC_CH7_MODE			BIT(12)
> +#define ASPEED_ADC_CH7_NORMAL			0
> +#define ASPEED_ADC_CH7_BAT			1
> +#define ASPEED_ADC_BAT_SENSING_ENABLE		BIT(13)
> +#define ASPEED_ADC_CTRL_CHANNEL			GENMASK(31, 16)
> +#define ASPEED_ADC_CTRL_CHANNEL_ENABLE(ch)	FIELD_PREP(ASPEED_ADC_CTRL_CHANNEL, BIT(ch))
>  
>  #define ASPEED_ADC_INIT_POLLING_TIME	500
>  #define ASPEED_ADC_INIT_TIMEOUT		500000
> @@ -227,7 +253,9 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  
>  	if (model_data->wait_init_sequence) {
>  		/* Enable engine in normal mode. */
> -		writel(ASPEED_OPERATION_MODE_NORMAL | ASPEED_ENGINE_ENABLE,
> +		writel(FIELD_PREP(ASPEED_ADC_OP_MODE,
> +				  ASPEED_ADC_OP_MODE_NORMAL) |
> +			       ASPEED_ADC_ENGINE_ENABLE,
>  		       data->base + ASPEED_REG_ENGINE_CONTROL);
>  
>  		/* Wait for initial sequence complete. */
> @@ -246,10 +274,12 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto clk_enable_error;
>  
> -	adc_engine_control_reg_val = GENMASK(31, 16) |
> -		ASPEED_OPERATION_MODE_NORMAL | ASPEED_ENGINE_ENABLE;
> +	adc_engine_control_reg_val =
> +		ASPEED_ADC_CTRL_CHANNEL |
> +		FIELD_PREP(ASPEED_ADC_OP_MODE, ASPEED_ADC_OP_MODE_NORMAL) |
> +		ASPEED_ADC_ENGINE_ENABLE;
>  	writel(adc_engine_control_reg_val,
> -		data->base + ASPEED_REG_ENGINE_CONTROL);
> +	       data->base + ASPEED_REG_ENGINE_CONTROL);
>  
>  	model_data = of_device_get_match_data(&pdev->dev);
>  	indio_dev->name = model_data->model_name;
> @@ -265,8 +295,8 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  	return 0;
>  
>  iio_register_error:
> -	writel(ASPEED_OPERATION_MODE_POWER_DOWN,
> -		data->base + ASPEED_REG_ENGINE_CONTROL);
> +	writel(FIELD_PREP(ASPEED_ADC_OP_MODE, ASPEED_ADC_OP_MODE_PWR_DOWN),
> +	       data->base + ASPEED_REG_ENGINE_CONTROL);
>  	clk_disable_unprepare(data->clk_scaler->clk);
>  clk_enable_error:
>  poll_timeout_error:
> @@ -284,8 +314,8 @@ static int aspeed_adc_remove(struct platform_device *pdev)
>  	struct aspeed_adc_data *data = iio_priv(indio_dev);
>  
>  	iio_device_unregister(indio_dev);
> -	writel(ASPEED_OPERATION_MODE_POWER_DOWN,
> -		data->base + ASPEED_REG_ENGINE_CONTROL);
> +	writel(FIELD_PREP(ASPEED_ADC_OP_MODE, ASPEED_ADC_OP_MODE_PWR_DOWN),
> +	       data->base + ASPEED_REG_ENGINE_CONTROL);
>  	clk_disable_unprepare(data->clk_scaler->clk);
>  	reset_control_assert(data->rst);
>  	clk_hw_unregister_divider(data->clk_scaler);


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [v5 04/15] iio: adc: aspeed: Keep model data to driver data.
  2021-08-31  7:14 ` [v5 04/15] iio: adc: aspeed: Keep model data to driver data Billy Tsai
@ 2021-09-05 14:33   ` Jonathan Cameron
  2021-09-05 14:50     ` Jonathan Cameron
  0 siblings, 1 reply; 27+ messages in thread
From: Jonathan Cameron @ 2021-09-05 14:33 UTC (permalink / raw)
  To: Billy Tsai
  Cc: lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood, broonie,
	linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

On Tue, 31 Aug 2021 15:14:47 +0800
Billy Tsai <billy_tsai@aspeedtech.com> wrote:

> Keep the model data pointer to driver data for reducing the usage of
> of_device_get_match_data().
> 
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
This one starts to be impacted by the fix (as its in the context).
Rather than making a mess of things for linux-next etc I'll hold
off on these until that fix is upstream in a few weeks.

If I seem to have lost it (it's been known to happen :( ) then
feel free to poke me!

Thanks,

Jonathan
> ---
>  drivers/iio/adc/aspeed_adc.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
> index f055fe7b2c40..76ae1c3f584b 100644
> --- a/drivers/iio/adc/aspeed_adc.c
> +++ b/drivers/iio/adc/aspeed_adc.c
> @@ -77,6 +77,7 @@ struct aspeed_adc_model_data {
>  
>  struct aspeed_adc_data {
>  	struct device		*dev;
> +	const struct aspeed_adc_model_data *model_data;
>  	void __iomem		*base;
>  	spinlock_t		clk_lock;
>  	struct clk_hw		*clk_prescaler;
> @@ -118,8 +119,6 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
>  			       int *val, int *val2, long mask)
>  {
>  	struct aspeed_adc_data *data = iio_priv(indio_dev);
> -	const struct aspeed_adc_model_data *model_data =
> -			of_device_get_match_data(data->dev);
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_RAW:
> @@ -127,7 +126,7 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
>  		return IIO_VAL_INT;
>  
>  	case IIO_CHAN_INFO_SCALE:
> -		*val = model_data->vref_voltage;
> +		*val = data->model_data->vref_voltage;
>  		*val2 = ASPEED_RESOLUTION_BITS;
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  
> @@ -146,13 +145,11 @@ static int aspeed_adc_write_raw(struct iio_dev *indio_dev,
>  				int val, int val2, long mask)
>  {
>  	struct aspeed_adc_data *data = iio_priv(indio_dev);
> -	const struct aspeed_adc_model_data *model_data =
> -			of_device_get_match_data(data->dev);
>  
>  	switch (mask) {
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> -		if (val < model_data->min_sampling_rate ||
> -			val > model_data->max_sampling_rate)
> +		if (val < data->model_data->min_sampling_rate ||
> +			val > data->model_data->max_sampling_rate)
>  			return -EINVAL;
>  
>  		clk_set_rate(data->clk_scaler->clk,
> @@ -198,7 +195,6 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  {
>  	struct iio_dev *indio_dev;
>  	struct aspeed_adc_data *data;
> -	const struct aspeed_adc_model_data *model_data;
>  	const char *clk_parent_name;
>  	int ret;
>  	u32 adc_engine_control_reg_val;
> @@ -209,6 +205,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  
>  	data = iio_priv(indio_dev);
>  	data->dev = &pdev->dev;
> +	data->model_data = of_device_get_match_data(&pdev->dev);
>  	platform_set_drvdata(pdev, indio_dev);
>  
>  	data->base = devm_platform_ioremap_resource(pdev, 0);
> @@ -249,9 +246,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  	}
>  	reset_control_deassert(data->rst);
>  
> -	model_data = of_device_get_match_data(&pdev->dev);
> -
> -	if (model_data->wait_init_sequence) {
> +	if (data->model_data->wait_init_sequence) {
>  		/* Enable engine in normal mode. */
>  		writel(FIELD_PREP(ASPEED_ADC_OP_MODE,
>  				  ASPEED_ADC_OP_MODE_NORMAL) |
> @@ -281,8 +276,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  	writel(adc_engine_control_reg_val,
>  	       data->base + ASPEED_REG_ENGINE_CONTROL);
>  
> -	model_data = of_device_get_match_data(&pdev->dev);
> -	indio_dev->name = model_data->model_name;
> +	indio_dev->name = data->model_data->model_name;
>  	indio_dev->info = &aspeed_adc_iio_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  	indio_dev->channels = aspeed_adc_iio_channels;


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [v5 05/15] iio: adc: aspeed: Refactory model data structure
  2021-08-31  7:14 ` [v5 05/15] iio: adc: aspeed: Refactory model data structure Billy Tsai
@ 2021-09-05 14:37   ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2021-09-05 14:37 UTC (permalink / raw)
  To: Billy Tsai
  Cc: lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood, broonie,
	linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

On Tue, 31 Aug 2021 15:14:48 +0800
Billy Tsai <billy_tsai@aspeedtech.com> wrote:

Title.  Refactor (refactory isn't a word as far as I know though it perhaps should
be ;)

If you are rerolling the latter part of this series, merge patch 7 down into this one.
If not, it's fine as it stands.  That one is trivial and a direct result of this one.

Jonathan


> This patch refactors the model data structure to distinguish the
> function form different versions of aspeed ADC.
> - Rename the vref_voltage to vref_fixed_mv and add vref_mv driver data
> When driver probe will check vref_fixed_mv value and store it to vref_mv
> which isn't const value.
> - Add num_channels
> Make num_channles of iio device can be changed by different model_data
> - Add need_prescaler flag and scaler_bit_width
> The need_prescaler flag is used to tell the driver the clock divider needs
> another Prescaler and the scaler_bit_width to set the clock divider
> bitfield width.
> 
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
>  drivers/iio/adc/aspeed_adc.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
> index 76ae1c3f584b..6ce2f676c54a 100644
> --- a/drivers/iio/adc/aspeed_adc.c
> +++ b/drivers/iio/adc/aspeed_adc.c
> @@ -71,8 +71,11 @@ struct aspeed_adc_model_data {
>  	const char *model_name;
>  	unsigned int min_sampling_rate;	// Hz
>  	unsigned int max_sampling_rate;	// Hz
> -	unsigned int vref_voltage;	// mV
> +	unsigned int vref_fixed_mv;
>  	bool wait_init_sequence;
> +	bool need_prescaler;
> +	u8 scaler_bit_width;
> +	unsigned int num_channels;
>  };
>  
>  struct aspeed_adc_data {
> @@ -83,6 +86,7 @@ struct aspeed_adc_data {
>  	struct clk_hw		*clk_prescaler;
>  	struct clk_hw		*clk_scaler;
>  	struct reset_control	*rst;
> +	int			vref_mv;
>  };
>  
>  #define ASPEED_CHAN(_idx, _data_reg_addr) {			\
> @@ -126,7 +130,7 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
>  		return IIO_VAL_INT;
>  
>  	case IIO_CHAN_INFO_SCALE:
> -		*val = data->model_data->vref_voltage;
> +		*val = data->model_data->vref_fixed_mv;
>  		*val2 = ASPEED_RESOLUTION_BITS;
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  
> @@ -320,17 +324,23 @@ static int aspeed_adc_remove(struct platform_device *pdev)
>  
>  static const struct aspeed_adc_model_data ast2400_model_data = {
>  	.model_name = "ast2400-adc",
> -	.vref_voltage = 2500, // mV
> +	.vref_fixed_mv = 2500,
>  	.min_sampling_rate = 10000,
>  	.max_sampling_rate = 500000,
> +	.need_prescaler = true,
> +	.scaler_bit_width = 10,
> +	.num_channels = 16,
>  };
>  
>  static const struct aspeed_adc_model_data ast2500_model_data = {
>  	.model_name = "ast2500-adc",
> -	.vref_voltage = 1800, // mV
> +	.vref_fixed_mv = 1800,
>  	.min_sampling_rate = 1,
>  	.max_sampling_rate = 1000000,
>  	.wait_init_sequence = true,
> +	.need_prescaler = true,
> +	.scaler_bit_width = 10,
> +	.num_channels = 16,
>  };
>  
>  static const struct of_device_id aspeed_adc_matches[] = {


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [v5 11/15] iio: adc: aspeed: Fix the calculate error of clock.
  2021-08-31  7:14 ` [v5 11/15] iio: adc: aspeed: Fix the calculate error of clock Billy Tsai
@ 2021-09-05 14:47   ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2021-09-05 14:47 UTC (permalink / raw)
  To: Billy Tsai
  Cc: lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood, broonie,
	linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

On Tue, 31 Aug 2021 15:14:54 +0800
Billy Tsai <billy_tsai@aspeedtech.com> wrote:

> The ADC clock formula is
> ast2400/2500:
> ADC clock period = PCLK * 2 * (ADC0C[31:17] + 1) * (ADC0C[9:0] + 1)
> ast2600:
> ADC clock period = PCLK * 2 * (ADC0C[15:0] + 1)
> They all have one fixed divided 2 and the legacy driver didn't handle it.
> This patch register the fixed factory clock device as the parent of ADC
> clock scaler to fix this issue.
> 
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
>  drivers/iio/adc/aspeed_adc.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
> index 40b7ba58c1dc..349377b9fac0 100644
> --- a/drivers/iio/adc/aspeed_adc.c
> +++ b/drivers/iio/adc/aspeed_adc.c
> @@ -4,6 +4,12 @@
>   *
>   * Copyright (C) 2017 Google, Inc.
>   * Copyright (C) 2021 Aspeed Technology Inc.
> + *
> + * ADC clock formula:
> + * Ast2400/Ast2500:
> + * clock period = period of PCLK * 2 * (ADC0C[31:17] + 1) * (ADC0C[9:0] + 1)
> + * Ast2600:
> + * clock period = period of PCLK * 2 * (ADC0C[15:0] + 1)
>   */
>  
>  #include <linux/clk.h>
> @@ -85,6 +91,7 @@ struct aspeed_adc_data {
>  	struct regulator	*regulator;
>  	void __iomem		*base;
>  	spinlock_t		clk_lock;
> +	struct clk_hw		*fixed_div_clk;
>  	struct clk_hw		*clk_prescaler;
>  	struct clk_hw		*clk_scaler;
>  	struct reset_control	*rst;
> @@ -204,6 +211,13 @@ static void aspeed_adc_unregister_divider(void *data)
>  	clk_hw_unregister_divider(clk);
>  }
>  
> +static void aspeed_adc_unregister_fixed_divider(void *data)
> +{
> +	struct clk_hw *clk = data;
> +
> +	clk_hw_unregister_fixed_factor(clk);
> +}
> +
>  static void aspeed_adc_reset_assert(void *data)
>  {
>  	struct reset_control *rst = data;
> @@ -328,6 +342,19 @@ static int aspeed_adc_probe(struct platform_device *pdev)
>  	spin_lock_init(&data->clk_lock);
>  	snprintf(clk_parent_name, ARRAY_SIZE(clk_parent_name), "%s",
>  		 of_clk_get_parent_name(pdev->dev.of_node, 0));
> +	snprintf(clk_name, ARRAY_SIZE(clk_name), "%s-fixed-div",
> +		 data->model_data->model_name);
> +	data->fixed_div_clk = clk_hw_register_fixed_factor(
> +		&pdev->dev, clk_name, clk_parent_name, 0, 1, 2);

Obvious follow on from Philipp's review - there is a devm_ version of
this as well which you can use rather than rolling a custom version.
As a side note, I'm fairly sure you could refactor all those devm_clk_hw
functions to use devm_add_action_or_reset() internally and simplify that
code nicely.

A recent series did the same for all the similar functions in IIO.



> +	if (IS_ERR(data->fixed_div_clk))
> +		return PTR_ERR(data->fixed_div_clk);
> +
> +	ret = devm_add_action_or_reset(data->dev,
> +				       aspeed_adc_unregister_fixed_divider,
> +				       data->clk_prescaler);
> +	if (ret)
> +		return ret;
> +	snprintf(clk_parent_name, ARRAY_SIZE(clk_parent_name), clk_name);
>  
>  	if (data->model_data->need_prescaler) {
>  		snprintf(clk_name, ARRAY_SIZE(clk_name), "%s-prescaler",


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [v5 04/15] iio: adc: aspeed: Keep model data to driver data.
  2021-09-05 14:33   ` Jonathan Cameron
@ 2021-09-05 14:50     ` Jonathan Cameron
  2021-09-16  3:52       ` Joel Stanley
  0 siblings, 1 reply; 27+ messages in thread
From: Jonathan Cameron @ 2021-09-05 14:50 UTC (permalink / raw)
  To: Billy Tsai
  Cc: lars, pmeerw, robh+dt, joel, andrew, p.zabel, lgirdwood, broonie,
	linux-iio, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, BMC-SW

On Sun, 5 Sep 2021 15:33:39 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Tue, 31 Aug 2021 15:14:47 +0800
> Billy Tsai <billy_tsai@aspeedtech.com> wrote:
> 
> > Keep the model data pointer to driver data for reducing the usage of
> > of_device_get_match_data().
> > 
> > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>  
> This one starts to be impacted by the fix (as its in the context).
> Rather than making a mess of things for linux-next etc I'll hold
> off on these until that fix is upstream in a few weeks.
> 
> If I seem to have lost it (it's been known to happen :( ) then
> feel free to poke me!

Having taken another look at the rest of the series (and Philipp's review)
please do a v6 starting from this patch.

Thanks,
Jonathan

> 
> Thanks,
> 
> Jonathan
> > ---
> >  drivers/iio/adc/aspeed_adc.c | 20 +++++++-------------
> >  1 file changed, 7 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
> > index f055fe7b2c40..76ae1c3f584b 100644
> > --- a/drivers/iio/adc/aspeed_adc.c
> > +++ b/drivers/iio/adc/aspeed_adc.c
> > @@ -77,6 +77,7 @@ struct aspeed_adc_model_data {
> >  
> >  struct aspeed_adc_data {
> >  	struct device		*dev;
> > +	const struct aspeed_adc_model_data *model_data;
> >  	void __iomem		*base;
> >  	spinlock_t		clk_lock;
> >  	struct clk_hw		*clk_prescaler;
> > @@ -118,8 +119,6 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
> >  			       int *val, int *val2, long mask)
> >  {
> >  	struct aspeed_adc_data *data = iio_priv(indio_dev);
> > -	const struct aspeed_adc_model_data *model_data =
> > -			of_device_get_match_data(data->dev);
> >  
> >  	switch (mask) {
> >  	case IIO_CHAN_INFO_RAW:
> > @@ -127,7 +126,7 @@ static int aspeed_adc_read_raw(struct iio_dev *indio_dev,
> >  		return IIO_VAL_INT;
> >  
> >  	case IIO_CHAN_INFO_SCALE:
> > -		*val = model_data->vref_voltage;
> > +		*val = data->model_data->vref_voltage;
> >  		*val2 = ASPEED_RESOLUTION_BITS;
> >  		return IIO_VAL_FRACTIONAL_LOG2;
> >  
> > @@ -146,13 +145,11 @@ static int aspeed_adc_write_raw(struct iio_dev *indio_dev,
> >  				int val, int val2, long mask)
> >  {
> >  	struct aspeed_adc_data *data = iio_priv(indio_dev);
> > -	const struct aspeed_adc_model_data *model_data =
> > -			of_device_get_match_data(data->dev);
> >  
> >  	switch (mask) {
> >  	case IIO_CHAN_INFO_SAMP_FREQ:
> > -		if (val < model_data->min_sampling_rate ||
> > -			val > model_data->max_sampling_rate)
> > +		if (val < data->model_data->min_sampling_rate ||
> > +			val > data->model_data->max_sampling_rate)
> >  			return -EINVAL;
> >  
> >  		clk_set_rate(data->clk_scaler->clk,
> > @@ -198,7 +195,6 @@ static int aspeed_adc_probe(struct platform_device *pdev)
> >  {
> >  	struct iio_dev *indio_dev;
> >  	struct aspeed_adc_data *data;
> > -	const struct aspeed_adc_model_data *model_data;
> >  	const char *clk_parent_name;
> >  	int ret;
> >  	u32 adc_engine_control_reg_val;
> > @@ -209,6 +205,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
> >  
> >  	data = iio_priv(indio_dev);
> >  	data->dev = &pdev->dev;
> > +	data->model_data = of_device_get_match_data(&pdev->dev);
> >  	platform_set_drvdata(pdev, indio_dev);
> >  
> >  	data->base = devm_platform_ioremap_resource(pdev, 0);
> > @@ -249,9 +246,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
> >  	}
> >  	reset_control_deassert(data->rst);
> >  
> > -	model_data = of_device_get_match_data(&pdev->dev);
> > -
> > -	if (model_data->wait_init_sequence) {
> > +	if (data->model_data->wait_init_sequence) {
> >  		/* Enable engine in normal mode. */
> >  		writel(FIELD_PREP(ASPEED_ADC_OP_MODE,
> >  				  ASPEED_ADC_OP_MODE_NORMAL) |
> > @@ -281,8 +276,7 @@ static int aspeed_adc_probe(struct platform_device *pdev)
> >  	writel(adc_engine_control_reg_val,
> >  	       data->base + ASPEED_REG_ENGINE_CONTROL);
> >  
> > -	model_data = of_device_get_match_data(&pdev->dev);
> > -	indio_dev->name = model_data->model_name;
> > +	indio_dev->name = data->model_data->model_name;
> >  	indio_dev->info = &aspeed_adc_iio_info;
> >  	indio_dev->modes = INDIO_DIRECT_MODE;
> >  	indio_dev->channels = aspeed_adc_iio_channels;  
> 


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [v5 04/15] iio: adc: aspeed: Keep model data to driver data.
  2021-09-05 14:50     ` Jonathan Cameron
@ 2021-09-16  3:52       ` Joel Stanley
  2021-09-18 17:55         ` Jonathan Cameron
  0 siblings, 1 reply; 27+ messages in thread
From: Joel Stanley @ 2021-09-16  3:52 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Billy Tsai, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Andrew Jeffery, Philipp Zabel, lgirdwood,
	Mark Brown, linux-iio, devicetree, Linux ARM, linux-aspeed,
	Linux Kernel Mailing List, BMC-SW

On Sun, 5 Sept 2021 at 14:47, Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sun, 5 Sep 2021 15:33:39 +0100
> Jonathan Cameron <jic23@kernel.org> wrote:
>
> > On Tue, 31 Aug 2021 15:14:47 +0800
> > Billy Tsai <billy_tsai@aspeedtech.com> wrote:
> >
> > > Keep the model data pointer to driver data for reducing the usage of
> > > of_device_get_match_data().
> > >
> > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> > This one starts to be impacted by the fix (as its in the context).
> > Rather than making a mess of things for linux-next etc I'll hold
> > off on these until that fix is upstream in a few weeks.
> >
> > If I seem to have lost it (it's been known to happen :( ) then
> > feel free to poke me!
>
> Having taken another look at the rest of the series (and Philipp's review)
> please do a v6 starting from this patch.

I'd recommend against the practice of half applying a series. I have
just spent a good chunk of time looking at v6, and wondering why it
won't apply to any tags in Linus tree nor to next.

(It was made worse by the branch you applied them to not being part of
linux-next.)

Cheers,

Joel

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [v5 04/15] iio: adc: aspeed: Keep model data to driver data.
  2021-09-16  3:52       ` Joel Stanley
@ 2021-09-18 17:55         ` Jonathan Cameron
  0 siblings, 0 replies; 27+ messages in thread
From: Jonathan Cameron @ 2021-09-18 17:55 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Billy Tsai, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Rob Herring, Andrew Jeffery, Philipp Zabel, lgirdwood,
	Mark Brown, linux-iio, devicetree, Linux ARM, linux-aspeed,
	Linux Kernel Mailing List, BMC-SW

On Thu, 16 Sep 2021 03:52:24 +0000
Joel Stanley <joel@jms.id.au> wrote:

> On Sun, 5 Sept 2021 at 14:47, Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Sun, 5 Sep 2021 15:33:39 +0100
> > Jonathan Cameron <jic23@kernel.org> wrote:
> >  
> > > On Tue, 31 Aug 2021 15:14:47 +0800
> > > Billy Tsai <billy_tsai@aspeedtech.com> wrote:
> > >  
> > > > Keep the model data pointer to driver data for reducing the usage of
> > > > of_device_get_match_data().
> > > >
> > > > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>  
> > > This one starts to be impacted by the fix (as its in the context).
> > > Rather than making a mess of things for linux-next etc I'll hold
> > > off on these until that fix is upstream in a few weeks.
> > >
> > > If I seem to have lost it (it's been known to happen :( ) then
> > > feel free to poke me!  
> >
> > Having taken another look at the rest of the series (and Philipp's review)
> > please do a v6 starting from this patch.  
> 
> I'd recommend against the practice of half applying a series. I have
> just spent a good chunk of time looking at v6, and wondering why it
> won't apply to any tags in Linus tree nor to next.

Hi Joel,

In this particular case it may been unwise, but in general it allows me to
handle a higher volume of patches than would otherwise be possible.
There are of course other approaches, but this one works well for me.
I did express to what tree and branch these were being applied
+ exposed for build tests.

> 
> (It was made worse by the branch you applied them to not being part of
> linux-next.)

It will be shortly. That was just unfortunate timing because of the end of the
merge window and a some issues that 0-day found in other patches in the tree
that needed to be fixed up before making a mess in next.

Jonathan

> 
> Cheers,
> 
> Joel


^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2021-09-18 17:51 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-31  7:14 [v5 00/15] Add support for ast2600 ADC Billy Tsai
2021-08-31  7:14 ` [v5 01/15] iio: adc: aspeed: set driver data when adc probe Billy Tsai
2021-09-05 14:25   ` Jonathan Cameron
2021-08-31  7:14 ` [v5 02/15] dt-bindings: iio: adc: Add ast2600-adc bindings Billy Tsai
2021-09-01  1:38   ` Rob Herring
2021-09-05 14:27     ` Jonathan Cameron
2021-08-31  7:14 ` [v5 03/15] iio: adc: aspeed: completes the bitfield declare Billy Tsai
2021-09-05 14:30   ` Jonathan Cameron
2021-08-31  7:14 ` [v5 04/15] iio: adc: aspeed: Keep model data to driver data Billy Tsai
2021-09-05 14:33   ` Jonathan Cameron
2021-09-05 14:50     ` Jonathan Cameron
2021-09-16  3:52       ` Joel Stanley
2021-09-18 17:55         ` Jonathan Cameron
2021-08-31  7:14 ` [v5 05/15] iio: adc: aspeed: Refactory model data structure Billy Tsai
2021-09-05 14:37   ` Jonathan Cameron
2021-08-31  7:14 ` [v5 06/15] iio: adc: aspeed: Add vref config function Billy Tsai
2021-08-31  7:14 ` [v5 07/15] iio: adc: aspeed: Set num_channels with model data Billy Tsai
2021-08-31  7:14 ` [v5 08/15] iio: adc: aspeed: Use model_data to set clk scaler Billy Tsai
2021-08-31  7:14 ` [v5 09/15] iio: adc: aspeed: Use devm_add_action_or_reset Billy Tsai
2021-08-31  7:51   ` Philipp Zabel
2021-08-31  7:14 ` [v5 10/15] iio: adc: aspeed: Support ast2600 adc Billy Tsai
2021-08-31  7:14 ` [v5 11/15] iio: adc: aspeed: Fix the calculate error of clock Billy Tsai
2021-09-05 14:47   ` Jonathan Cameron
2021-08-31  7:14 ` [v5 12/15] iio: adc: aspeed: Add func to set sampling rate Billy Tsai
2021-08-31  7:14 ` [v5 13/15] iio: adc: aspeed: Add compensation phase Billy Tsai
2021-08-31  7:14 ` [v5 14/15] iio: adc: aspeed: Support battery sensing Billy Tsai
2021-08-31  7:14 ` [v5 15/15] iio: adc: aspeed: Get and set trimming data Billy Tsai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).