[PATCH 0/9] iio: adc: at91_adc: cleanup DT bindings

[PATCH 0/9] iio: adc: at91_adc: cleanup DT bindings

[Alexandre Belloni]

Hello,

This series cleans up the at91_adc devicetree bindings. This mainly
moves back the resolution options and names and the triggers description
back in the driver.

There are also other cleanups, like removing platform data support, this
was pending for a while.

Alexandre Belloni (8):
  iio: adc: at91_adc: remove platform data
  iio: adc: at91_adc: rework resolution selection
  iio: adc: at91_adc: rework trigger definition
  iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe
  iio: adc: at91_adc: remove forward declaration
  iio: adc: at91_adc: use devm_input_allocate_device
  ARM: dts: at91: sama5d3: use proper ADC compatible
  ARM: dts: at91: remove deprecated ADC properties

Jonathan Cameron (1):
  dt-bindings:iio:adc:atmel,sama9260-adc: conversion to yaml from
    at91_adc.txt

 .../devicetree/bindings/iio/adc/at91_adc.txt  |  83 ----
 .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 121 ++++++
 arch/arm/boot/dts/at91sam9260.dtsi            |  25 --
 arch/arm/boot/dts/at91sam9g45.dtsi            |  27 --
 arch/arm/boot/dts/at91sam9rl.dtsi             |  25 --
 arch/arm/boot/dts/at91sam9x5.dtsi             |  28 --
 arch/arm/boot/dts/sama5d3.dtsi                |  26 +-
 arch/arm/boot/dts/sama5d4.dtsi                |  22 -
 drivers/iio/adc/at91_adc.c                    | 377 +++++++-----------
 include/linux/platform_data/at91_adc.h        |  49 ---
 10 files changed, 259 insertions(+), 524 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/adc/at91_adc.txt
 create mode 100644 Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
 delete mode 100644 include/linux/platform_data/at91_adc.h

-- 
2.28.0

[PATCH 0/9] iio: adc: at91_adc: cleanup DT bindings

[Alexandre Belloni]

Hello,

This series cleans up the at91_adc devicetree bindings. This mainly
moves back the resolution options and names and the triggers description
back in the driver.

There are also other cleanups, like removing platform data support, this
was pending for a while.

Alexandre Belloni (8):
  iio: adc: at91_adc: remove platform data
  iio: adc: at91_adc: rework resolution selection
  iio: adc: at91_adc: rework trigger definition
  iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe
  iio: adc: at91_adc: remove forward declaration
  iio: adc: at91_adc: use devm_input_allocate_device
  ARM: dts: at91: sama5d3: use proper ADC compatible
  ARM: dts: at91: remove deprecated ADC properties

Jonathan Cameron (1):
  dt-bindings:iio:adc:atmel,sama9260-adc: conversion to yaml from
    at91_adc.txt

 .../devicetree/bindings/iio/adc/at91_adc.txt  |  83 ----
 .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 121 ++++++
 arch/arm/boot/dts/at91sam9260.dtsi            |  25 --
 arch/arm/boot/dts/at91sam9g45.dtsi            |  27 --
 arch/arm/boot/dts/at91sam9rl.dtsi             |  25 --
 arch/arm/boot/dts/at91sam9x5.dtsi             |  28 --
 arch/arm/boot/dts/sama5d3.dtsi                |  26 +-
 arch/arm/boot/dts/sama5d4.dtsi                |  22 -
 drivers/iio/adc/at91_adc.c                    | 377 +++++++-----------
 include/linux/platform_data/at91_adc.h        |  49 ---
 10 files changed, 259 insertions(+), 524 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/adc/at91_adc.txt
 create mode 100644 Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
 delete mode 100644 include/linux/platform_data/at91_adc.h

-- 
2.28.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/9] iio: adc: at91_adc: remove platform data

[Alexandre Belloni]

The at91 platforms have been DT only for a while, remove platform data.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/iio/adc/at91_adc.c             | 80 +++++++-------------------
 include/linux/platform_data/at91_adc.h | 49 ----------------
 2 files changed, 22 insertions(+), 107 deletions(-)
 delete mode 100644 include/linux/platform_data/at91_adc.h

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 9b2c548fae95..62bd35af8b13 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -22,8 +22,6 @@
 #include <linux/slab.h>
 #include <linux/wait.h>
 
-#include <linux/platform_data/at91_adc.h>
-
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/trigger.h>
@@ -153,6 +151,25 @@
 #define TOUCH_SHTIM                    0xa
 #define TOUCH_SCTIM_US		10		/* 10us for the Touchscreen Switches Closure Time */
 
+enum atmel_adc_ts_type {
+	ATMEL_ADC_TOUCHSCREEN_NONE = 0,
+	ATMEL_ADC_TOUCHSCREEN_4WIRE = 4,
+	ATMEL_ADC_TOUCHSCREEN_5WIRE = 5,
+};
+
+/**
+ * struct at91_adc_trigger - description of triggers
+ * @name:		name of the trigger advertised to the user
+ * @value:		value to set in the ADC's trigger setup register
+ *			to enable the trigger
+ * @is_external:	Does the trigger rely on an external pin?
+ */
+struct at91_adc_trigger {
+	const char	*name;
+	u8		value;
+	bool		is_external;
+};
+
 /**
  * struct at91_adc_reg_desc - Various informations relative to registers
  * @channel_base:	Base offset for the channel data registers
@@ -875,9 +892,6 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
 	int i = 0, ret;
 	u32 prop;
 
-	if (!node)
-		return -EINVAL;
-
 	st->caps = (struct at91_adc_caps *)
 		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
 
@@ -960,30 +974,6 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
 	return ret;
 }
 
-static int at91_adc_probe_pdata(struct at91_adc_state *st,
-				struct platform_device *pdev)
-{
-	struct at91_adc_data *pdata = pdev->dev.platform_data;
-
-	if (!pdata)
-		return -EINVAL;
-
-	st->caps = (struct at91_adc_caps *)
-			platform_get_device_id(pdev)->driver_data;
-
-	st->use_external = pdata->use_external_triggers;
-	st->vref_mv = pdata->vref;
-	st->channels_mask = pdata->channels_used;
-	st->num_channels = st->caps->num_channels;
-	st->startup_time = pdata->startup_time;
-	st->trigger_number = pdata->trigger_number;
-	st->trigger_list = pdata->trigger_list;
-	st->registers = &st->caps->registers;
-	st->touchscreen_type = pdata->touchscreen_type;
-
-	return 0;
-}
-
 static const struct iio_info at91_adc_info = {
 	.read_raw = &at91_adc_read_raw,
 };
@@ -1160,15 +1150,9 @@ static int at91_adc_probe(struct platform_device *pdev)
 
 	st = iio_priv(idev);
 
-	if (pdev->dev.of_node)
-		ret = at91_adc_probe_dt(idev, pdev);
-	else
-		ret = at91_adc_probe_pdata(st, pdev);
-
-	if (ret) {
-		dev_err(&pdev->dev, "No platform data available.\n");
-		return -EINVAL;
-	}
+	ret = at91_adc_probe_dt(idev, pdev);
+	if (ret)
+		return ret;
 
 	platform_set_drvdata(pdev, idev);
 
@@ -1444,29 +1428,9 @@ static const struct of_device_id at91_adc_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, at91_adc_dt_ids);
 
-static const struct platform_device_id at91_adc_ids[] = {
-	{
-		.name = "at91sam9260-adc",
-		.driver_data = (unsigned long)&at91sam9260_caps,
-	}, {
-		.name = "at91sam9rl-adc",
-		.driver_data = (unsigned long)&at91sam9rl_caps,
-	}, {
-		.name = "at91sam9g45-adc",
-		.driver_data = (unsigned long)&at91sam9g45_caps,
-	}, {
-		.name = "at91sam9x5-adc",
-		.driver_data = (unsigned long)&at91sam9x5_caps,
-	}, {
-		/* terminator */
-	}
-};
-MODULE_DEVICE_TABLE(platform, at91_adc_ids);
-
 static struct platform_driver at91_adc_driver = {
 	.probe = at91_adc_probe,
 	.remove = at91_adc_remove,
-	.id_table = at91_adc_ids,
 	.driver = {
 		   .name = DRIVER_NAME,
 		   .of_match_table = of_match_ptr(at91_adc_dt_ids),
diff --git a/include/linux/platform_data/at91_adc.h b/include/linux/platform_data/at91_adc.h
deleted file mode 100644
index f20eaeb827ce..000000000000
--- a/include/linux/platform_data/at91_adc.h
+++ /dev/null
@@ -1,49 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * Copyright (C) 2011 Free Electrons
- */
-
-#ifndef _AT91_ADC_H_
-#define _AT91_ADC_H_
-
-enum atmel_adc_ts_type {
-	ATMEL_ADC_TOUCHSCREEN_NONE = 0,
-	ATMEL_ADC_TOUCHSCREEN_4WIRE = 4,
-	ATMEL_ADC_TOUCHSCREEN_5WIRE = 5,
-};
-
-/**
- * struct at91_adc_trigger - description of triggers
- * @name:		name of the trigger advertised to the user
- * @value:		value to set in the ADC's trigger setup register
-			to enable the trigger
- * @is_external:	Does the trigger rely on an external pin?
- */
-struct at91_adc_trigger {
-	const char	*name;
-	u8		value;
-	bool		is_external;
-};
-
-/**
- * struct at91_adc_data - platform data for ADC driver
- * @channels_used:		channels in use on the board as a bitmask
- * @startup_time:		startup time of the ADC in microseconds
- * @trigger_list:		Triggers available in the ADC
- * @trigger_number:		Number of triggers available in the ADC
- * @use_external_triggers:	does the board has external triggers availables
- * @vref:			Reference voltage for the ADC in millivolts
- * @touchscreen_type:		If a touchscreen is connected, its type (4 or 5 wires)
- */
-struct at91_adc_data {
-	unsigned long			channels_used;
-	u8				startup_time;
-	struct at91_adc_trigger		*trigger_list;
-	u8				trigger_number;
-	bool				use_external_triggers;
-	u16				vref;
-	enum atmel_adc_ts_type		touchscreen_type;
-};
-
-extern void __init at91_add_device_adc(struct at91_adc_data *data);
-#endif
-- 
2.28.0

[PATCH 1/9] iio: adc: at91_adc: remove platform data

[Alexandre Belloni]

The at91 platforms have been DT only for a while, remove platform data.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/iio/adc/at91_adc.c             | 80 +++++++-------------------
 include/linux/platform_data/at91_adc.h | 49 ----------------
 2 files changed, 22 insertions(+), 107 deletions(-)
 delete mode 100644 include/linux/platform_data/at91_adc.h

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 9b2c548fae95..62bd35af8b13 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -22,8 +22,6 @@
 #include <linux/slab.h>
 #include <linux/wait.h>
 
-#include <linux/platform_data/at91_adc.h>
-
 #include <linux/iio/iio.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/trigger.h>
@@ -153,6 +151,25 @@
 #define TOUCH_SHTIM                    0xa
 #define TOUCH_SCTIM_US		10		/* 10us for the Touchscreen Switches Closure Time */
 
+enum atmel_adc_ts_type {
+	ATMEL_ADC_TOUCHSCREEN_NONE = 0,
+	ATMEL_ADC_TOUCHSCREEN_4WIRE = 4,
+	ATMEL_ADC_TOUCHSCREEN_5WIRE = 5,
+};
+
+/**
+ * struct at91_adc_trigger - description of triggers
+ * @name:		name of the trigger advertised to the user
+ * @value:		value to set in the ADC's trigger setup register
+ *			to enable the trigger
+ * @is_external:	Does the trigger rely on an external pin?
+ */
+struct at91_adc_trigger {
+	const char	*name;
+	u8		value;
+	bool		is_external;
+};
+
 /**
  * struct at91_adc_reg_desc - Various informations relative to registers
  * @channel_base:	Base offset for the channel data registers
@@ -875,9 +892,6 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
 	int i = 0, ret;
 	u32 prop;
 
-	if (!node)
-		return -EINVAL;
-
 	st->caps = (struct at91_adc_caps *)
 		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
 
@@ -960,30 +974,6 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
 	return ret;
 }
 
-static int at91_adc_probe_pdata(struct at91_adc_state *st,
-				struct platform_device *pdev)
-{
-	struct at91_adc_data *pdata = pdev->dev.platform_data;
-
-	if (!pdata)
-		return -EINVAL;
-
-	st->caps = (struct at91_adc_caps *)
-			platform_get_device_id(pdev)->driver_data;
-
-	st->use_external = pdata->use_external_triggers;
-	st->vref_mv = pdata->vref;
-	st->channels_mask = pdata->channels_used;
-	st->num_channels = st->caps->num_channels;
-	st->startup_time = pdata->startup_time;
-	st->trigger_number = pdata->trigger_number;
-	st->trigger_list = pdata->trigger_list;
-	st->registers = &st->caps->registers;
-	st->touchscreen_type = pdata->touchscreen_type;
-
-	return 0;
-}
-
 static const struct iio_info at91_adc_info = {
 	.read_raw = &at91_adc_read_raw,
 };
@@ -1160,15 +1150,9 @@ static int at91_adc_probe(struct platform_device *pdev)
 
 	st = iio_priv(idev);
 
-	if (pdev->dev.of_node)
-		ret = at91_adc_probe_dt(idev, pdev);
-	else
-		ret = at91_adc_probe_pdata(st, pdev);
-
-	if (ret) {
-		dev_err(&pdev->dev, "No platform data available.\n");
-		return -EINVAL;
-	}
+	ret = at91_adc_probe_dt(idev, pdev);
+	if (ret)
+		return ret;
 
 	platform_set_drvdata(pdev, idev);
 
@@ -1444,29 +1428,9 @@ static const struct of_device_id at91_adc_dt_ids[] = {
 };
 MODULE_DEVICE_TABLE(of, at91_adc_dt_ids);
 
-static const struct platform_device_id at91_adc_ids[] = {
-	{
-		.name = "at91sam9260-adc",
-		.driver_data = (unsigned long)&at91sam9260_caps,
-	}, {
-		.name = "at91sam9rl-adc",
-		.driver_data = (unsigned long)&at91sam9rl_caps,
-	}, {
-		.name = "at91sam9g45-adc",
-		.driver_data = (unsigned long)&at91sam9g45_caps,
-	}, {
-		.name = "at91sam9x5-adc",
-		.driver_data = (unsigned long)&at91sam9x5_caps,
-	}, {
-		/* terminator */
-	}
-};
-MODULE_DEVICE_TABLE(platform, at91_adc_ids);
-
 static struct platform_driver at91_adc_driver = {
 	.probe = at91_adc_probe,
 	.remove = at91_adc_remove,
-	.id_table = at91_adc_ids,
 	.driver = {
 		   .name = DRIVER_NAME,
 		   .of_match_table = of_match_ptr(at91_adc_dt_ids),
diff --git a/include/linux/platform_data/at91_adc.h b/include/linux/platform_data/at91_adc.h
deleted file mode 100644
index f20eaeb827ce..000000000000
--- a/include/linux/platform_data/at91_adc.h
+++ /dev/null
@@ -1,49 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * Copyright (C) 2011 Free Electrons
- */
-
-#ifndef _AT91_ADC_H_
-#define _AT91_ADC_H_
-
-enum atmel_adc_ts_type {
-	ATMEL_ADC_TOUCHSCREEN_NONE = 0,
-	ATMEL_ADC_TOUCHSCREEN_4WIRE = 4,
-	ATMEL_ADC_TOUCHSCREEN_5WIRE = 5,
-};
-
-/**
- * struct at91_adc_trigger - description of triggers
- * @name:		name of the trigger advertised to the user
- * @value:		value to set in the ADC's trigger setup register
-			to enable the trigger
- * @is_external:	Does the trigger rely on an external pin?
- */
-struct at91_adc_trigger {
-	const char	*name;
-	u8		value;
-	bool		is_external;
-};
-
-/**
- * struct at91_adc_data - platform data for ADC driver
- * @channels_used:		channels in use on the board as a bitmask
- * @startup_time:		startup time of the ADC in microseconds
- * @trigger_list:		Triggers available in the ADC
- * @trigger_number:		Number of triggers available in the ADC
- * @use_external_triggers:	does the board has external triggers availables
- * @vref:			Reference voltage for the ADC in millivolts
- * @touchscreen_type:		If a touchscreen is connected, its type (4 or 5 wires)
- */
-struct at91_adc_data {
-	unsigned long			channels_used;
-	u8				startup_time;
-	struct at91_adc_trigger		*trigger_list;
-	u8				trigger_number;
-	bool				use_external_triggers;
-	u16				vref;
-	enum atmel_adc_ts_type		touchscreen_type;
-};
-
-extern void __init at91_add_device_adc(struct at91_adc_data *data);
-#endif
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/9] iio: adc: at91_adc: rework resolution selection

[Alexandre Belloni]

Move the possible resolution values back to the driver. This removes the
atmel,adc-res and atmel,adc-res-names properties, leaving only
atmel,adc-use-res. As atmel,adc-res-names had to contain "lowres" and
"highres", those where already the only allowed values for
atmel,adc-use-res.

Also introduce a new compatible string for the sama5d3 as this is the only
one with a different resolution. Also it doesn't event have the LOWRES
bit.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 .../devicetree/bindings/iio/adc/at91_adc.txt  | 13 +--
 drivers/iio/adc/at91_adc.c                    | 97 ++++++++-----------
 2 files changed, 44 insertions(+), 66 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt b/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
index f65b04fb7962..da393ac5c05f 100644
--- a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
@@ -2,7 +2,7 @@
 
 Required properties:
   - compatible: Should be "atmel,<chip>-adc"
-    <chip> can be "at91sam9260", "at91sam9g45" or "at91sam9x5"
+    <chip> can be "at91sam9260", "at91sam9g45", "at91sam9x5" or "sama5d3"
   - reg: Should contain ADC registers location and length
   - interrupts: Should contain the IRQ line for the ADC
   - clock-names: tuple listing input clock names.
@@ -13,17 +13,12 @@ Required properties:
   - atmel,adc-startup-time: Startup Time of the ADC in microseconds as
     defined in the datasheet
   - atmel,adc-vref: Reference voltage in millivolts for the conversions
-  - atmel,adc-res: List of resolutions in bits supported by the ADC. List size
-		   must be two at least.
-  - atmel,adc-res-names: Contains one identifier string for each resolution
-			 in atmel,adc-res property. "lowres" and "highres"
-			 identifiers are required.
 
 Optional properties:
   - atmel,adc-use-external-triggers: Boolean to enable the external triggers
-  - atmel,adc-use-res: String corresponding to an identifier from
-		       atmel,adc-res-names property. If not specified, the highest
-		       resolution will be used.
+  - atmel,adc-use-res: String selecting the resolution, can be "lowres" or
+		       "highres". If not specified, the highest resolution will
+		       be used.
   - atmel,adc-sleep-mode: Boolean to enable sleep mode when no conversion
   - atmel,adc-sample-hold-time: Sample and Hold Time in microseconds
   - atmel,adc-ts-wires: Number of touchscreen wires. Should be 4 or 5. If this
diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 62bd35af8b13..0d67c812ef3d 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -204,6 +204,9 @@ struct at91_adc_caps {
 	u32 (*calc_startup_ticks)(u32 startup_time, u32 adc_clk_khz);
 
 	u8	num_channels;
+
+	u8	low_res_bits;
+	u8	high_res_bits;
 	struct at91_adc_reg_desc registers;
 };
 
@@ -229,7 +232,6 @@ struct at91_adc_state {
 	bool			use_external;
 	u32			vref_mv;
 	u32			res;		/* resolution used for convertions */
-	bool			low_res;	/* the resolution corresponds to the lowest one */
 	wait_queue_head_t	wq_data_avail;
 	struct at91_adc_caps	*caps;
 
@@ -754,58 +756,6 @@ static int at91_adc_read_raw(struct iio_dev *idev,
 	return -EINVAL;
 }
 
-static int at91_adc_of_get_resolution(struct iio_dev *idev,
-				      struct platform_device *pdev)
-{
-	struct at91_adc_state *st = iio_priv(idev);
-	struct device_node *np = pdev->dev.of_node;
-	int count, i, ret = 0;
-	char *res_name, *s;
-	u32 *resolutions;
-
-	count = of_property_count_strings(np, "atmel,adc-res-names");
-	if (count < 2) {
-		dev_err(&idev->dev, "You must specified at least two resolution names for "
-				    "adc-res-names property in the DT\n");
-		return count;
-	}
-
-	resolutions = kmalloc_array(count, sizeof(*resolutions), GFP_KERNEL);
-	if (!resolutions)
-		return -ENOMEM;
-
-	if (of_property_read_u32_array(np, "atmel,adc-res", resolutions, count)) {
-		dev_err(&idev->dev, "Missing adc-res property in the DT.\n");
-		ret = -ENODEV;
-		goto ret;
-	}
-
-	if (of_property_read_string(np, "atmel,adc-use-res", (const char **)&res_name))
-		res_name = "highres";
-
-	for (i = 0; i < count; i++) {
-		if (of_property_read_string_index(np, "atmel,adc-res-names", i, (const char **)&s))
-			continue;
-
-		if (strcmp(res_name, s))
-			continue;
-
-		st->res = resolutions[i];
-		if (!strcmp(res_name, "lowres"))
-			st->low_res = true;
-		else
-			st->low_res = false;
-
-		dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
-		goto ret;
-	}
-
-	dev_err(&idev->dev, "There is no resolution for %s\n", res_name);
-
-ret:
-	kfree(resolutions);
-	return ret;
-}
 
 static u32 calc_startup_ticks_9260(u32 startup_time, u32 adc_clk_khz)
 {
@@ -891,6 +841,7 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
 	struct device_node *trig_node;
 	int i = 0, ret;
 	u32 prop;
+	char *s;
 
 	st->caps = (struct at91_adc_caps *)
 		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
@@ -924,9 +875,13 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
 	}
 	st->vref_mv = prop;
 
-	ret = at91_adc_of_get_resolution(idev, pdev);
-	if (ret)
-		goto error_ret;
+	st->res = st->caps->high_res_bits;
+	if (st->caps->low_res_bits &&
+	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
+	    && !strcmp(s, "lowres"))
+		st->res = st->caps->low_res_bits;
+
+	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
 
 	st->registers = &st->caps->registers;
 	st->num_channels = st->caps->num_channels;
@@ -1248,7 +1203,7 @@ static int at91_adc_probe(struct platform_device *pdev)
 
 	reg = AT91_ADC_PRESCAL_(prsc) & st->registers->mr_prescal_mask;
 	reg |= AT91_ADC_STARTUP_(ticks) & st->registers->mr_startup_mask;
-	if (st->low_res)
+	if (st->res == st->caps->low_res_bits)
 		reg |= AT91_ADC_LOWRES;
 	if (st->sleep_mode)
 		reg |= AT91_ADC_SLEEP;
@@ -1363,6 +1318,8 @@ static SIMPLE_DEV_PM_OPS(at91_adc_pm_ops, at91_adc_suspend, at91_adc_resume);
 static struct at91_adc_caps at91sam9260_caps = {
 	.calc_startup_ticks = calc_startup_ticks_9260,
 	.num_channels = 4,
+	.low_res_bits = 8,
+	.high_res_bits = 10,
 	.registers = {
 		.channel_base = AT91_ADC_CHR(0),
 		.drdy_mask = AT91_ADC_DRDY,
@@ -1377,6 +1334,8 @@ static struct at91_adc_caps at91sam9rl_caps = {
 	.has_ts = true,
 	.calc_startup_ticks = calc_startup_ticks_9260,	/* same as 9260 */
 	.num_channels = 6,
+	.low_res_bits = 8,
+	.high_res_bits = 10,
 	.registers = {
 		.channel_base = AT91_ADC_CHR(0),
 		.drdy_mask = AT91_ADC_DRDY,
@@ -1391,6 +1350,8 @@ static struct at91_adc_caps at91sam9g45_caps = {
 	.has_ts = true,
 	.calc_startup_ticks = calc_startup_ticks_9260,	/* same as 9260 */
 	.num_channels = 8,
+	.low_res_bits = 8,
+	.high_res_bits = 10,
 	.registers = {
 		.channel_base = AT91_ADC_CHR(0),
 		.drdy_mask = AT91_ADC_DRDY,
@@ -1408,6 +1369,8 @@ static struct at91_adc_caps at91sam9x5_caps = {
 	.ts_pen_detect_sensitivity = 2,
 	.calc_startup_ticks = calc_startup_ticks_9x5,
 	.num_channels = 12,
+	.low_res_bits = 8,
+	.high_res_bits = 10,
 	.registers = {
 		.channel_base = AT91_ADC_CDR0_9X5,
 		.drdy_mask = AT91_ADC_SR_DRDY_9X5,
@@ -1419,11 +1382,31 @@ static struct at91_adc_caps at91sam9x5_caps = {
 	},
 };
 
+static struct at91_adc_caps sama5d3_caps = {
+	.has_ts = true,
+	.has_tsmr = true,
+	.ts_filter_average = 3,
+	.ts_pen_detect_sensitivity = 2,
+	.calc_startup_ticks = calc_startup_ticks_9x5,
+	.num_channels = 12,
+	.low_res_bits = 0,
+	.high_res_bits = 12,
+	.registers = {
+		.channel_base = AT91_ADC_CDR0_9X5,
+		.drdy_mask = AT91_ADC_SR_DRDY_9X5,
+		.status_register = AT91_ADC_SR_9X5,
+		.trigger_register = AT91_ADC_TRGR_9X5,
+		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
+		.mr_startup_mask = AT91_ADC_STARTUP_9X5,
+	},
+};
+
 static const struct of_device_id at91_adc_dt_ids[] = {
 	{ .compatible = "atmel,at91sam9260-adc", .data = &at91sam9260_caps },
 	{ .compatible = "atmel,at91sam9rl-adc", .data = &at91sam9rl_caps },
 	{ .compatible = "atmel,at91sam9g45-adc", .data = &at91sam9g45_caps },
 	{ .compatible = "atmel,at91sam9x5-adc", .data = &at91sam9x5_caps },
+	{ .compatible = "atmel,sama5d3-adc", .data = &sama5d3_caps },
 	{},
 };
 MODULE_DEVICE_TABLE(of, at91_adc_dt_ids);
-- 
2.28.0

[PATCH 2/9] iio: adc: at91_adc: rework resolution selection

[Alexandre Belloni]

Move the possible resolution values back to the driver. This removes the
atmel,adc-res and atmel,adc-res-names properties, leaving only
atmel,adc-use-res. As atmel,adc-res-names had to contain "lowres" and
"highres", those where already the only allowed values for
atmel,adc-use-res.

Also introduce a new compatible string for the sama5d3 as this is the only
one with a different resolution. Also it doesn't event have the LOWRES
bit.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 .../devicetree/bindings/iio/adc/at91_adc.txt  | 13 +--
 drivers/iio/adc/at91_adc.c                    | 97 ++++++++-----------
 2 files changed, 44 insertions(+), 66 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt b/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
index f65b04fb7962..da393ac5c05f 100644
--- a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
@@ -2,7 +2,7 @@
 
 Required properties:
   - compatible: Should be "atmel,<chip>-adc"
-    <chip> can be "at91sam9260", "at91sam9g45" or "at91sam9x5"
+    <chip> can be "at91sam9260", "at91sam9g45", "at91sam9x5" or "sama5d3"
   - reg: Should contain ADC registers location and length
   - interrupts: Should contain the IRQ line for the ADC
   - clock-names: tuple listing input clock names.
@@ -13,17 +13,12 @@ Required properties:
   - atmel,adc-startup-time: Startup Time of the ADC in microseconds as
     defined in the datasheet
   - atmel,adc-vref: Reference voltage in millivolts for the conversions
-  - atmel,adc-res: List of resolutions in bits supported by the ADC. List size
-		   must be two at least.
-  - atmel,adc-res-names: Contains one identifier string for each resolution
-			 in atmel,adc-res property. "lowres" and "highres"
-			 identifiers are required.
 
 Optional properties:
   - atmel,adc-use-external-triggers: Boolean to enable the external triggers
-  - atmel,adc-use-res: String corresponding to an identifier from
-		       atmel,adc-res-names property. If not specified, the highest
-		       resolution will be used.
+  - atmel,adc-use-res: String selecting the resolution, can be "lowres" or
+		       "highres". If not specified, the highest resolution will
+		       be used.
   - atmel,adc-sleep-mode: Boolean to enable sleep mode when no conversion
   - atmel,adc-sample-hold-time: Sample and Hold Time in microseconds
   - atmel,adc-ts-wires: Number of touchscreen wires. Should be 4 or 5. If this
diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 62bd35af8b13..0d67c812ef3d 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -204,6 +204,9 @@ struct at91_adc_caps {
 	u32 (*calc_startup_ticks)(u32 startup_time, u32 adc_clk_khz);
 
 	u8	num_channels;
+
+	u8	low_res_bits;
+	u8	high_res_bits;
 	struct at91_adc_reg_desc registers;
 };
 
@@ -229,7 +232,6 @@ struct at91_adc_state {
 	bool			use_external;
 	u32			vref_mv;
 	u32			res;		/* resolution used for convertions */
-	bool			low_res;	/* the resolution corresponds to the lowest one */
 	wait_queue_head_t	wq_data_avail;
 	struct at91_adc_caps	*caps;
 
@@ -754,58 +756,6 @@ static int at91_adc_read_raw(struct iio_dev *idev,
 	return -EINVAL;
 }
 
-static int at91_adc_of_get_resolution(struct iio_dev *idev,
-				      struct platform_device *pdev)
-{
-	struct at91_adc_state *st = iio_priv(idev);
-	struct device_node *np = pdev->dev.of_node;
-	int count, i, ret = 0;
-	char *res_name, *s;
-	u32 *resolutions;
-
-	count = of_property_count_strings(np, "atmel,adc-res-names");
-	if (count < 2) {
-		dev_err(&idev->dev, "You must specified at least two resolution names for "
-				    "adc-res-names property in the DT\n");
-		return count;
-	}
-
-	resolutions = kmalloc_array(count, sizeof(*resolutions), GFP_KERNEL);
-	if (!resolutions)
-		return -ENOMEM;
-
-	if (of_property_read_u32_array(np, "atmel,adc-res", resolutions, count)) {
-		dev_err(&idev->dev, "Missing adc-res property in the DT.\n");
-		ret = -ENODEV;
-		goto ret;
-	}
-
-	if (of_property_read_string(np, "atmel,adc-use-res", (const char **)&res_name))
-		res_name = "highres";
-
-	for (i = 0; i < count; i++) {
-		if (of_property_read_string_index(np, "atmel,adc-res-names", i, (const char **)&s))
-			continue;
-
-		if (strcmp(res_name, s))
-			continue;
-
-		st->res = resolutions[i];
-		if (!strcmp(res_name, "lowres"))
-			st->low_res = true;
-		else
-			st->low_res = false;
-
-		dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
-		goto ret;
-	}
-
-	dev_err(&idev->dev, "There is no resolution for %s\n", res_name);
-
-ret:
-	kfree(resolutions);
-	return ret;
-}
 
 static u32 calc_startup_ticks_9260(u32 startup_time, u32 adc_clk_khz)
 {
@@ -891,6 +841,7 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
 	struct device_node *trig_node;
 	int i = 0, ret;
 	u32 prop;
+	char *s;
 
 	st->caps = (struct at91_adc_caps *)
 		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
@@ -924,9 +875,13 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
 	}
 	st->vref_mv = prop;
 
-	ret = at91_adc_of_get_resolution(idev, pdev);
-	if (ret)
-		goto error_ret;
+	st->res = st->caps->high_res_bits;
+	if (st->caps->low_res_bits &&
+	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
+	    && !strcmp(s, "lowres"))
+		st->res = st->caps->low_res_bits;
+
+	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
 
 	st->registers = &st->caps->registers;
 	st->num_channels = st->caps->num_channels;
@@ -1248,7 +1203,7 @@ static int at91_adc_probe(struct platform_device *pdev)
 
 	reg = AT91_ADC_PRESCAL_(prsc) & st->registers->mr_prescal_mask;
 	reg |= AT91_ADC_STARTUP_(ticks) & st->registers->mr_startup_mask;
-	if (st->low_res)
+	if (st->res == st->caps->low_res_bits)
 		reg |= AT91_ADC_LOWRES;
 	if (st->sleep_mode)
 		reg |= AT91_ADC_SLEEP;
@@ -1363,6 +1318,8 @@ static SIMPLE_DEV_PM_OPS(at91_adc_pm_ops, at91_adc_suspend, at91_adc_resume);
 static struct at91_adc_caps at91sam9260_caps = {
 	.calc_startup_ticks = calc_startup_ticks_9260,
 	.num_channels = 4,
+	.low_res_bits = 8,
+	.high_res_bits = 10,
 	.registers = {
 		.channel_base = AT91_ADC_CHR(0),
 		.drdy_mask = AT91_ADC_DRDY,
@@ -1377,6 +1334,8 @@ static struct at91_adc_caps at91sam9rl_caps = {
 	.has_ts = true,
 	.calc_startup_ticks = calc_startup_ticks_9260,	/* same as 9260 */
 	.num_channels = 6,
+	.low_res_bits = 8,
+	.high_res_bits = 10,
 	.registers = {
 		.channel_base = AT91_ADC_CHR(0),
 		.drdy_mask = AT91_ADC_DRDY,
@@ -1391,6 +1350,8 @@ static struct at91_adc_caps at91sam9g45_caps = {
 	.has_ts = true,
 	.calc_startup_ticks = calc_startup_ticks_9260,	/* same as 9260 */
 	.num_channels = 8,
+	.low_res_bits = 8,
+	.high_res_bits = 10,
 	.registers = {
 		.channel_base = AT91_ADC_CHR(0),
 		.drdy_mask = AT91_ADC_DRDY,
@@ -1408,6 +1369,8 @@ static struct at91_adc_caps at91sam9x5_caps = {
 	.ts_pen_detect_sensitivity = 2,
 	.calc_startup_ticks = calc_startup_ticks_9x5,
 	.num_channels = 12,
+	.low_res_bits = 8,
+	.high_res_bits = 10,
 	.registers = {
 		.channel_base = AT91_ADC_CDR0_9X5,
 		.drdy_mask = AT91_ADC_SR_DRDY_9X5,
@@ -1419,11 +1382,31 @@ static struct at91_adc_caps at91sam9x5_caps = {
 	},
 };
 
+static struct at91_adc_caps sama5d3_caps = {
+	.has_ts = true,
+	.has_tsmr = true,
+	.ts_filter_average = 3,
+	.ts_pen_detect_sensitivity = 2,
+	.calc_startup_ticks = calc_startup_ticks_9x5,
+	.num_channels = 12,
+	.low_res_bits = 0,
+	.high_res_bits = 12,
+	.registers = {
+		.channel_base = AT91_ADC_CDR0_9X5,
+		.drdy_mask = AT91_ADC_SR_DRDY_9X5,
+		.status_register = AT91_ADC_SR_9X5,
+		.trigger_register = AT91_ADC_TRGR_9X5,
+		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
+		.mr_startup_mask = AT91_ADC_STARTUP_9X5,
+	},
+};
+
 static const struct of_device_id at91_adc_dt_ids[] = {
 	{ .compatible = "atmel,at91sam9260-adc", .data = &at91sam9260_caps },
 	{ .compatible = "atmel,at91sam9rl-adc", .data = &at91sam9rl_caps },
 	{ .compatible = "atmel,at91sam9g45-adc", .data = &at91sam9g45_caps },
 	{ .compatible = "atmel,at91sam9x5-adc", .data = &at91sam9x5_caps },
+	{ .compatible = "atmel,sama5d3-adc", .data = &sama5d3_caps },
 	{},
 };
 MODULE_DEVICE_TABLE(of, at91_adc_dt_ids);
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/9] iio: adc: at91_adc: rework resolution selection

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:43 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> Move the possible resolution values back to the driver. This removes the
> atmel,adc-res and atmel,adc-res-names properties, leaving only
> atmel,adc-use-res. As atmel,adc-res-names had to contain "lowres" and
> "highres", those where already the only allowed values for
> atmel,adc-use-res.
> 
> Also introduce a new compatible string for the sama5d3 as this is the only
> one with a different resolution. Also it doesn't event have the LOWRES

even?

> bit.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

Looks good otherwise.  

> ---
>  .../devicetree/bindings/iio/adc/at91_adc.txt  | 13 +--
>  drivers/iio/adc/at91_adc.c                    | 97 ++++++++-----------
>  2 files changed, 44 insertions(+), 66 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt b/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
> index f65b04fb7962..da393ac5c05f 100644
> --- a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
> @@ -2,7 +2,7 @@
>  
>  Required properties:
>    - compatible: Should be "atmel,<chip>-adc"
> -    <chip> can be "at91sam9260", "at91sam9g45" or "at91sam9x5"
> +    <chip> can be "at91sam9260", "at91sam9g45", "at91sam9x5" or "sama5d3"
>    - reg: Should contain ADC registers location and length
>    - interrupts: Should contain the IRQ line for the ADC
>    - clock-names: tuple listing input clock names.
> @@ -13,17 +13,12 @@ Required properties:
>    - atmel,adc-startup-time: Startup Time of the ADC in microseconds as
>      defined in the datasheet
>    - atmel,adc-vref: Reference voltage in millivolts for the conversions
> -  - atmel,adc-res: List of resolutions in bits supported by the ADC. List size
> -		   must be two at least.
> -  - atmel,adc-res-names: Contains one identifier string for each resolution
> -			 in atmel,adc-res property. "lowres" and "highres"
> -			 identifiers are required.
>  
>  Optional properties:
>    - atmel,adc-use-external-triggers: Boolean to enable the external triggers
> -  - atmel,adc-use-res: String corresponding to an identifier from
> -		       atmel,adc-res-names property. If not specified, the highest
> -		       resolution will be used.
> +  - atmel,adc-use-res: String selecting the resolution, can be "lowres" or
> +		       "highres". If not specified, the highest resolution will
> +		       be used.
>    - atmel,adc-sleep-mode: Boolean to enable sleep mode when no conversion
>    - atmel,adc-sample-hold-time: Sample and Hold Time in microseconds
>    - atmel,adc-ts-wires: Number of touchscreen wires. Should be 4 or 5. If this
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 62bd35af8b13..0d67c812ef3d 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -204,6 +204,9 @@ struct at91_adc_caps {
>  	u32 (*calc_startup_ticks)(u32 startup_time, u32 adc_clk_khz);
>  
>  	u8	num_channels;
> +
> +	u8	low_res_bits;
> +	u8	high_res_bits;
>  	struct at91_adc_reg_desc registers;
>  };
>  
> @@ -229,7 +232,6 @@ struct at91_adc_state {
>  	bool			use_external;
>  	u32			vref_mv;
>  	u32			res;		/* resolution used for convertions */
> -	bool			low_res;	/* the resolution corresponds to the lowest one */
>  	wait_queue_head_t	wq_data_avail;
>  	struct at91_adc_caps	*caps;
>  
> @@ -754,58 +756,6 @@ static int at91_adc_read_raw(struct iio_dev *idev,
>  	return -EINVAL;
>  }
>  
> -static int at91_adc_of_get_resolution(struct iio_dev *idev,
> -				      struct platform_device *pdev)
> -{
> -	struct at91_adc_state *st = iio_priv(idev);
> -	struct device_node *np = pdev->dev.of_node;
> -	int count, i, ret = 0;
> -	char *res_name, *s;
> -	u32 *resolutions;
> -
> -	count = of_property_count_strings(np, "atmel,adc-res-names");
> -	if (count < 2) {
> -		dev_err(&idev->dev, "You must specified at least two resolution names for "
> -				    "adc-res-names property in the DT\n");
> -		return count;
> -	}
> -
> -	resolutions = kmalloc_array(count, sizeof(*resolutions), GFP_KERNEL);
> -	if (!resolutions)
> -		return -ENOMEM;
> -
> -	if (of_property_read_u32_array(np, "atmel,adc-res", resolutions, count)) {
> -		dev_err(&idev->dev, "Missing adc-res property in the DT.\n");
> -		ret = -ENODEV;
> -		goto ret;
> -	}
> -
> -	if (of_property_read_string(np, "atmel,adc-use-res", (const char **)&res_name))
> -		res_name = "highres";
> -
> -	for (i = 0; i < count; i++) {
> -		if (of_property_read_string_index(np, "atmel,adc-res-names", i, (const char **)&s))
> -			continue;
> -
> -		if (strcmp(res_name, s))
> -			continue;
> -
> -		st->res = resolutions[i];
> -		if (!strcmp(res_name, "lowres"))
> -			st->low_res = true;
> -		else
> -			st->low_res = false;
> -
> -		dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
> -		goto ret;
> -	}
> -
> -	dev_err(&idev->dev, "There is no resolution for %s\n", res_name);
> -
> -ret:
> -	kfree(resolutions);
> -	return ret;
> -}
>  
>  static u32 calc_startup_ticks_9260(u32 startup_time, u32 adc_clk_khz)
>  {
> @@ -891,6 +841,7 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
>  	struct device_node *trig_node;
>  	int i = 0, ret;
>  	u32 prop;
> +	char *s;
>  
>  	st->caps = (struct at91_adc_caps *)
>  		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
> @@ -924,9 +875,13 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
>  	}
>  	st->vref_mv = prop;
>  
> -	ret = at91_adc_of_get_resolution(idev, pdev);
> -	if (ret)
> -		goto error_ret;
> +	st->res = st->caps->high_res_bits;
> +	if (st->caps->low_res_bits &&
> +	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
> +	    && !strcmp(s, "lowres"))
> +		st->res = st->caps->low_res_bits;
> +
> +	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
>  
>  	st->registers = &st->caps->registers;
>  	st->num_channels = st->caps->num_channels;
> @@ -1248,7 +1203,7 @@ static int at91_adc_probe(struct platform_device *pdev)
>  
>  	reg = AT91_ADC_PRESCAL_(prsc) & st->registers->mr_prescal_mask;
>  	reg |= AT91_ADC_STARTUP_(ticks) & st->registers->mr_startup_mask;
> -	if (st->low_res)
> +	if (st->res == st->caps->low_res_bits)
>  		reg |= AT91_ADC_LOWRES;
>  	if (st->sleep_mode)
>  		reg |= AT91_ADC_SLEEP;
> @@ -1363,6 +1318,8 @@ static SIMPLE_DEV_PM_OPS(at91_adc_pm_ops, at91_adc_suspend, at91_adc_resume);
>  static struct at91_adc_caps at91sam9260_caps = {
>  	.calc_startup_ticks = calc_startup_ticks_9260,
>  	.num_channels = 4,
> +	.low_res_bits = 8,
> +	.high_res_bits = 10,
>  	.registers = {
>  		.channel_base = AT91_ADC_CHR(0),
>  		.drdy_mask = AT91_ADC_DRDY,
> @@ -1377,6 +1334,8 @@ static struct at91_adc_caps at91sam9rl_caps = {
>  	.has_ts = true,
>  	.calc_startup_ticks = calc_startup_ticks_9260,	/* same as 9260 */
>  	.num_channels = 6,
> +	.low_res_bits = 8,
> +	.high_res_bits = 10,
>  	.registers = {
>  		.channel_base = AT91_ADC_CHR(0),
>  		.drdy_mask = AT91_ADC_DRDY,
> @@ -1391,6 +1350,8 @@ static struct at91_adc_caps at91sam9g45_caps = {
>  	.has_ts = true,
>  	.calc_startup_ticks = calc_startup_ticks_9260,	/* same as 9260 */
>  	.num_channels = 8,
> +	.low_res_bits = 8,
> +	.high_res_bits = 10,
>  	.registers = {
>  		.channel_base = AT91_ADC_CHR(0),
>  		.drdy_mask = AT91_ADC_DRDY,
> @@ -1408,6 +1369,8 @@ static struct at91_adc_caps at91sam9x5_caps = {
>  	.ts_pen_detect_sensitivity = 2,
>  	.calc_startup_ticks = calc_startup_ticks_9x5,
>  	.num_channels = 12,
> +	.low_res_bits = 8,
> +	.high_res_bits = 10,
>  	.registers = {
>  		.channel_base = AT91_ADC_CDR0_9X5,
>  		.drdy_mask = AT91_ADC_SR_DRDY_9X5,
> @@ -1419,11 +1382,31 @@ static struct at91_adc_caps at91sam9x5_caps = {
>  	},
>  };
>  
> +static struct at91_adc_caps sama5d3_caps = {
> +	.has_ts = true,
> +	.has_tsmr = true,
> +	.ts_filter_average = 3,
> +	.ts_pen_detect_sensitivity = 2,
> +	.calc_startup_ticks = calc_startup_ticks_9x5,
> +	.num_channels = 12,
> +	.low_res_bits = 0,
> +	.high_res_bits = 12,
> +	.registers = {
> +		.channel_base = AT91_ADC_CDR0_9X5,
> +		.drdy_mask = AT91_ADC_SR_DRDY_9X5,
> +		.status_register = AT91_ADC_SR_9X5,
> +		.trigger_register = AT91_ADC_TRGR_9X5,
> +		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
> +		.mr_startup_mask = AT91_ADC_STARTUP_9X5,
> +	},
> +};
> +
>  static const struct of_device_id at91_adc_dt_ids[] = {
>  	{ .compatible = "atmel,at91sam9260-adc", .data = &at91sam9260_caps },
>  	{ .compatible = "atmel,at91sam9rl-adc", .data = &at91sam9rl_caps },
>  	{ .compatible = "atmel,at91sam9g45-adc", .data = &at91sam9g45_caps },
>  	{ .compatible = "atmel,at91sam9x5-adc", .data = &at91sam9x5_caps },
> +	{ .compatible = "atmel,sama5d3-adc", .data = &sama5d3_caps },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, at91_adc_dt_ids);

Re: [PATCH 2/9] iio: adc: at91_adc: rework resolution selection

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:43 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> Move the possible resolution values back to the driver. This removes the
> atmel,adc-res and atmel,adc-res-names properties, leaving only
> atmel,adc-use-res. As atmel,adc-res-names had to contain "lowres" and
> "highres", those where already the only allowed values for
> atmel,adc-use-res.
> 
> Also introduce a new compatible string for the sama5d3 as this is the only
> one with a different resolution. Also it doesn't event have the LOWRES

even?

> bit.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

Looks good otherwise.  

> ---
>  .../devicetree/bindings/iio/adc/at91_adc.txt  | 13 +--
>  drivers/iio/adc/at91_adc.c                    | 97 ++++++++-----------
>  2 files changed, 44 insertions(+), 66 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt b/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
> index f65b04fb7962..da393ac5c05f 100644
> --- a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
> @@ -2,7 +2,7 @@
>  
>  Required properties:
>    - compatible: Should be "atmel,<chip>-adc"
> -    <chip> can be "at91sam9260", "at91sam9g45" or "at91sam9x5"
> +    <chip> can be "at91sam9260", "at91sam9g45", "at91sam9x5" or "sama5d3"
>    - reg: Should contain ADC registers location and length
>    - interrupts: Should contain the IRQ line for the ADC
>    - clock-names: tuple listing input clock names.
> @@ -13,17 +13,12 @@ Required properties:
>    - atmel,adc-startup-time: Startup Time of the ADC in microseconds as
>      defined in the datasheet
>    - atmel,adc-vref: Reference voltage in millivolts for the conversions
> -  - atmel,adc-res: List of resolutions in bits supported by the ADC. List size
> -		   must be two at least.
> -  - atmel,adc-res-names: Contains one identifier string for each resolution
> -			 in atmel,adc-res property. "lowres" and "highres"
> -			 identifiers are required.
>  
>  Optional properties:
>    - atmel,adc-use-external-triggers: Boolean to enable the external triggers
> -  - atmel,adc-use-res: String corresponding to an identifier from
> -		       atmel,adc-res-names property. If not specified, the highest
> -		       resolution will be used.
> +  - atmel,adc-use-res: String selecting the resolution, can be "lowres" or
> +		       "highres". If not specified, the highest resolution will
> +		       be used.
>    - atmel,adc-sleep-mode: Boolean to enable sleep mode when no conversion
>    - atmel,adc-sample-hold-time: Sample and Hold Time in microseconds
>    - atmel,adc-ts-wires: Number of touchscreen wires. Should be 4 or 5. If this
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 62bd35af8b13..0d67c812ef3d 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -204,6 +204,9 @@ struct at91_adc_caps {
>  	u32 (*calc_startup_ticks)(u32 startup_time, u32 adc_clk_khz);
>  
>  	u8	num_channels;
> +
> +	u8	low_res_bits;
> +	u8	high_res_bits;
>  	struct at91_adc_reg_desc registers;
>  };
>  
> @@ -229,7 +232,6 @@ struct at91_adc_state {
>  	bool			use_external;
>  	u32			vref_mv;
>  	u32			res;		/* resolution used for convertions */
> -	bool			low_res;	/* the resolution corresponds to the lowest one */
>  	wait_queue_head_t	wq_data_avail;
>  	struct at91_adc_caps	*caps;
>  
> @@ -754,58 +756,6 @@ static int at91_adc_read_raw(struct iio_dev *idev,
>  	return -EINVAL;
>  }
>  
> -static int at91_adc_of_get_resolution(struct iio_dev *idev,
> -				      struct platform_device *pdev)
> -{
> -	struct at91_adc_state *st = iio_priv(idev);
> -	struct device_node *np = pdev->dev.of_node;
> -	int count, i, ret = 0;
> -	char *res_name, *s;
> -	u32 *resolutions;
> -
> -	count = of_property_count_strings(np, "atmel,adc-res-names");
> -	if (count < 2) {
> -		dev_err(&idev->dev, "You must specified at least two resolution names for "
> -				    "adc-res-names property in the DT\n");
> -		return count;
> -	}
> -
> -	resolutions = kmalloc_array(count, sizeof(*resolutions), GFP_KERNEL);
> -	if (!resolutions)
> -		return -ENOMEM;
> -
> -	if (of_property_read_u32_array(np, "atmel,adc-res", resolutions, count)) {
> -		dev_err(&idev->dev, "Missing adc-res property in the DT.\n");
> -		ret = -ENODEV;
> -		goto ret;
> -	}
> -
> -	if (of_property_read_string(np, "atmel,adc-use-res", (const char **)&res_name))
> -		res_name = "highres";
> -
> -	for (i = 0; i < count; i++) {
> -		if (of_property_read_string_index(np, "atmel,adc-res-names", i, (const char **)&s))
> -			continue;
> -
> -		if (strcmp(res_name, s))
> -			continue;
> -
> -		st->res = resolutions[i];
> -		if (!strcmp(res_name, "lowres"))
> -			st->low_res = true;
> -		else
> -			st->low_res = false;
> -
> -		dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
> -		goto ret;
> -	}
> -
> -	dev_err(&idev->dev, "There is no resolution for %s\n", res_name);
> -
> -ret:
> -	kfree(resolutions);
> -	return ret;
> -}
>  
>  static u32 calc_startup_ticks_9260(u32 startup_time, u32 adc_clk_khz)
>  {
> @@ -891,6 +841,7 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
>  	struct device_node *trig_node;
>  	int i = 0, ret;
>  	u32 prop;
> +	char *s;
>  
>  	st->caps = (struct at91_adc_caps *)
>  		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
> @@ -924,9 +875,13 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
>  	}
>  	st->vref_mv = prop;
>  
> -	ret = at91_adc_of_get_resolution(idev, pdev);
> -	if (ret)
> -		goto error_ret;
> +	st->res = st->caps->high_res_bits;
> +	if (st->caps->low_res_bits &&
> +	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
> +	    && !strcmp(s, "lowres"))
> +		st->res = st->caps->low_res_bits;
> +
> +	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
>  
>  	st->registers = &st->caps->registers;
>  	st->num_channels = st->caps->num_channels;
> @@ -1248,7 +1203,7 @@ static int at91_adc_probe(struct platform_device *pdev)
>  
>  	reg = AT91_ADC_PRESCAL_(prsc) & st->registers->mr_prescal_mask;
>  	reg |= AT91_ADC_STARTUP_(ticks) & st->registers->mr_startup_mask;
> -	if (st->low_res)
> +	if (st->res == st->caps->low_res_bits)
>  		reg |= AT91_ADC_LOWRES;
>  	if (st->sleep_mode)
>  		reg |= AT91_ADC_SLEEP;
> @@ -1363,6 +1318,8 @@ static SIMPLE_DEV_PM_OPS(at91_adc_pm_ops, at91_adc_suspend, at91_adc_resume);
>  static struct at91_adc_caps at91sam9260_caps = {
>  	.calc_startup_ticks = calc_startup_ticks_9260,
>  	.num_channels = 4,
> +	.low_res_bits = 8,
> +	.high_res_bits = 10,
>  	.registers = {
>  		.channel_base = AT91_ADC_CHR(0),
>  		.drdy_mask = AT91_ADC_DRDY,
> @@ -1377,6 +1334,8 @@ static struct at91_adc_caps at91sam9rl_caps = {
>  	.has_ts = true,
>  	.calc_startup_ticks = calc_startup_ticks_9260,	/* same as 9260 */
>  	.num_channels = 6,
> +	.low_res_bits = 8,
> +	.high_res_bits = 10,
>  	.registers = {
>  		.channel_base = AT91_ADC_CHR(0),
>  		.drdy_mask = AT91_ADC_DRDY,
> @@ -1391,6 +1350,8 @@ static struct at91_adc_caps at91sam9g45_caps = {
>  	.has_ts = true,
>  	.calc_startup_ticks = calc_startup_ticks_9260,	/* same as 9260 */
>  	.num_channels = 8,
> +	.low_res_bits = 8,
> +	.high_res_bits = 10,
>  	.registers = {
>  		.channel_base = AT91_ADC_CHR(0),
>  		.drdy_mask = AT91_ADC_DRDY,
> @@ -1408,6 +1369,8 @@ static struct at91_adc_caps at91sam9x5_caps = {
>  	.ts_pen_detect_sensitivity = 2,
>  	.calc_startup_ticks = calc_startup_ticks_9x5,
>  	.num_channels = 12,
> +	.low_res_bits = 8,
> +	.high_res_bits = 10,
>  	.registers = {
>  		.channel_base = AT91_ADC_CDR0_9X5,
>  		.drdy_mask = AT91_ADC_SR_DRDY_9X5,
> @@ -1419,11 +1382,31 @@ static struct at91_adc_caps at91sam9x5_caps = {
>  	},
>  };
>  
> +static struct at91_adc_caps sama5d3_caps = {
> +	.has_ts = true,
> +	.has_tsmr = true,
> +	.ts_filter_average = 3,
> +	.ts_pen_detect_sensitivity = 2,
> +	.calc_startup_ticks = calc_startup_ticks_9x5,
> +	.num_channels = 12,
> +	.low_res_bits = 0,
> +	.high_res_bits = 12,
> +	.registers = {
> +		.channel_base = AT91_ADC_CDR0_9X5,
> +		.drdy_mask = AT91_ADC_SR_DRDY_9X5,
> +		.status_register = AT91_ADC_SR_9X5,
> +		.trigger_register = AT91_ADC_TRGR_9X5,
> +		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
> +		.mr_startup_mask = AT91_ADC_STARTUP_9X5,
> +	},
> +};
> +
>  static const struct of_device_id at91_adc_dt_ids[] = {
>  	{ .compatible = "atmel,at91sam9260-adc", .data = &at91sam9260_caps },
>  	{ .compatible = "atmel,at91sam9rl-adc", .data = &at91sam9rl_caps },
>  	{ .compatible = "atmel,at91sam9g45-adc", .data = &at91sam9g45_caps },
>  	{ .compatible = "atmel,at91sam9x5-adc", .data = &at91sam9x5_caps },
> +	{ .compatible = "atmel,sama5d3-adc", .data = &sama5d3_caps },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, at91_adc_dt_ids);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/9] dt-bindings:iio:adc:atmel,sama9260-adc: conversion to yaml from at91_adc.txt

[Alexandre Belloni]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

There are a few things we would do differently in an ADC binding if we
were starting from scratch but we are stuck with what we have (which
made sense back when this was written!)

We may be able to tighten up some elements of this binding in the future
by careful checking of what values properties can actually take.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Ludovic Desroches <ludovic.desroches@microchip.com>
[Alexandre Belloni: add sama5d3, remove atmel,adc-res and atmel,adc-res-names]
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 .../devicetree/bindings/iio/adc/at91_adc.txt  |  78 --------
 .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 167 ++++++++++++++++++
 2 files changed, 167 insertions(+), 78 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/adc/at91_adc.txt
 create mode 100644 Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt b/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
deleted file mode 100644
index da393ac5c05f..000000000000
--- a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
+++ /dev/null
@@ -1,78 +0,0 @@
-* AT91's Analog to Digital Converter (ADC)
-
-Required properties:
-  - compatible: Should be "atmel,<chip>-adc"
-    <chip> can be "at91sam9260", "at91sam9g45", "at91sam9x5" or "sama5d3"
-  - reg: Should contain ADC registers location and length
-  - interrupts: Should contain the IRQ line for the ADC
-  - clock-names: tuple listing input clock names.
-	Required elements: "adc_clk", "adc_op_clk".
-  - clocks: phandles to input clocks.
-  - atmel,adc-channels-used: Bitmask of the channels muxed and enabled for this
-    device
-  - atmel,adc-startup-time: Startup Time of the ADC in microseconds as
-    defined in the datasheet
-  - atmel,adc-vref: Reference voltage in millivolts for the conversions
-
-Optional properties:
-  - atmel,adc-use-external-triggers: Boolean to enable the external triggers
-  - atmel,adc-use-res: String selecting the resolution, can be "lowres" or
-		       "highres". If not specified, the highest resolution will
-		       be used.
-  - atmel,adc-sleep-mode: Boolean to enable sleep mode when no conversion
-  - atmel,adc-sample-hold-time: Sample and Hold Time in microseconds
-  - atmel,adc-ts-wires: Number of touchscreen wires. Should be 4 or 5. If this
-                        value is set, then the adc driver will enable touchscreen
-                        support.
-    NOTE: when adc touchscreen is enabled, the adc hardware trigger will be
-          disabled. Since touchscreen will occupy the trigger register.
-  - atmel,adc-ts-pressure-threshold: a pressure threshold for touchscreen. It
-                                     makes touch detection more precise.
-
-Optional trigger Nodes:
-  - Required properties:
-    * trigger-name: Name of the trigger exposed to the user
-    * trigger-value: Value to put in the Trigger register
-      to activate this trigger
-  - Optional properties:
-    * trigger-external: Is the trigger an external trigger?
-
-Examples:
-adc0: adc@fffb0000 {
-	#address-cells = <1>;
-	#size-cells = <0>;
-	compatible = "atmel,at91sam9260-adc";
-	reg = <0xfffb0000 0x100>;
-	interrupts = <20 IRQ_TYPE_LEVEL_HIGH 0>;
-	clocks = <&adc_clk>, <&adc_op_clk>;
-	clock-names = "adc_clk", "adc_op_clk";
-	atmel,adc-channels-used = <0xff>;
-	atmel,adc-startup-time = <40>;
-	atmel,adc-use-external-triggers;
-	atmel,adc-vref = <3300>;
-	atmel,adc-res = <8 10>;
-	atmel,adc-res-names = "lowres", "highres";
-	atmel,adc-use-res = "lowres";
-
-	trigger0 {
-		trigger-name = "external-rising";
-		trigger-value = <0x1>;
-		trigger-external;
-	};
-	trigger1 {
-		trigger-name = "external-falling";
-		trigger-value = <0x2>;
-		trigger-external;
-	};
-
-	trigger2 {
-		trigger-name = "external-any";
-		trigger-value = <0x3>;
-		trigger-external;
-	};
-
-	trigger3 {
-		trigger-name = "continuous";
-		trigger-value = <0x6>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
new file mode 100644
index 000000000000..9b0ff59e75de
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
@@ -0,0 +1,167 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/atmel,sama9260-adc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AT91 sama9260 and similar Analog to Digital Converter (ADC)
+
+maintainers:
+  - Alexandre Belloni <alexandre.belloni@bootlin.com>
+
+properties:
+  compatible:
+    enum:
+      - atmel,at91sam9260-adc
+      - atmel,at91sam9rl-adc
+      - atmel,at91sam9g45-adc
+      - atmel,at91sam9x5-adc
+      - atmel,at91sama5d3-adc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 2
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: adc_clk
+      - const: adc_op_clk
+
+  atmel,adc-channels-used:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Bitmask of the channels muxed and enabled for this device
+
+  atmel,adc-startup-time:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Startup Time of the ADC in microseconds as defined in the datasheet
+
+  atmel,adc-vref:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Reference voltage in millivolts for the conversions
+
+  atmel,adc-use-external-triggers:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: Enable the external triggers
+
+  atmel,adc-use-res:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      String corresponding to an identifier from atmel,adc-res-names property.
+      If not specified, the highest resolution will be used.
+    enum:
+      - "lowres"
+      - "highres"
+
+  atmel,adc-sleep-mode:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: Enable sleep mode when no conversion
+
+  atmel,adc-sample-hold-time:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Sample and Hold Time in microseconds
+
+  atmel,adc-ts-wires:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Number of touchscreen wires. Must be set to enable touchscreen.
+      NOTE: when adc touchscreen is enabled, the adc hardware trigger will be
+      disabled. Since touchscreen will occupy the trigger register.
+    enum:
+      - 4
+      - 5
+
+  atmel,adc-ts-pressure-threshold:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Pressure threshold for touchscreen.
+
+  "#io-channel-cells":
+    const: 1
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - atmel,adc-channels-used
+  - atmel,adc-startup-time
+  - atmel,adc-vref
+
+patternProperties:
+  "^(trigger)[0-9]$":
+    type: object
+    description: Child node to describe a trigger exposed to the user.
+    properties:
+      trigger-name:
+        $ref: /schemas/types.yaml#/definitions/string
+        description: Identifying name.
+
+      trigger-value:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Value to put in the Trigger register to activate this trigger
+
+      trigger-external:
+        $ref: /schemas/types.yaml#/definitions/flag
+        description: This trigger is provided from an external pin.
+
+    additionalProperties: false
+    required:
+      - trigger-name
+      - trigger-value
+
+examples:
+  - |
+    #include <dt-bindings/dma/at91.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    soc {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        adc@fffb0000 {
+            compatible = "atmel,at91sam9260-adc";
+            reg = <0xfffb0000 0x100>;
+            interrupts = <20 IRQ_TYPE_LEVEL_HIGH 0>;
+            clocks = <&adc_clk>, <&adc_op_clk>;
+            clock-names = "adc_clk", "adc_op_clk";
+            atmel,adc-channels-used = <0xff>;
+            atmel,adc-startup-time = <40>;
+            atmel,adc-use-external-triggers;
+            atmel,adc-vref = <3300>;
+            atmel,adc-use-res = "lowres";
+
+            trigger0 {
+                trigger-name = "external-rising";
+                trigger-value = <0x1>;
+                trigger-external;
+            };
+
+            trigger1 {
+                trigger-name = "external-falling";
+                trigger-value = <0x2>;
+                trigger-external;
+            };
+
+            trigger2 {
+                trigger-name = "external-any";
+                trigger-value = <0x3>;
+                trigger-external;
+            };
+
+            trigger3 {
+                trigger-name = "continuous";
+                trigger-value = <0x6>;
+            };
+        };
+    };
+...
-- 
2.28.0

[PATCH 3/9] dt-bindings:iio:adc:atmel, sama9260-adc: conversion to yaml from at91_adc.txt

[Alexandre Belloni]

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

There are a few things we would do differently in an ADC binding if we
were starting from scratch but we are stuck with what we have (which
made sense back when this was written!)

We may be able to tighten up some elements of this binding in the future
by careful checking of what values properties can actually take.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Ludovic Desroches <ludovic.desroches@microchip.com>
[Alexandre Belloni: add sama5d3, remove atmel,adc-res and atmel,adc-res-names]
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 .../devicetree/bindings/iio/adc/at91_adc.txt  |  78 --------
 .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 167 ++++++++++++++++++
 2 files changed, 167 insertions(+), 78 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/adc/at91_adc.txt
 create mode 100644 Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt b/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
deleted file mode 100644
index da393ac5c05f..000000000000
--- a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
+++ /dev/null
@@ -1,78 +0,0 @@
-* AT91's Analog to Digital Converter (ADC)
-
-Required properties:
-  - compatible: Should be "atmel,<chip>-adc"
-    <chip> can be "at91sam9260", "at91sam9g45", "at91sam9x5" or "sama5d3"
-  - reg: Should contain ADC registers location and length
-  - interrupts: Should contain the IRQ line for the ADC
-  - clock-names: tuple listing input clock names.
-	Required elements: "adc_clk", "adc_op_clk".
-  - clocks: phandles to input clocks.
-  - atmel,adc-channels-used: Bitmask of the channels muxed and enabled for this
-    device
-  - atmel,adc-startup-time: Startup Time of the ADC in microseconds as
-    defined in the datasheet
-  - atmel,adc-vref: Reference voltage in millivolts for the conversions
-
-Optional properties:
-  - atmel,adc-use-external-triggers: Boolean to enable the external triggers
-  - atmel,adc-use-res: String selecting the resolution, can be "lowres" or
-		       "highres". If not specified, the highest resolution will
-		       be used.
-  - atmel,adc-sleep-mode: Boolean to enable sleep mode when no conversion
-  - atmel,adc-sample-hold-time: Sample and Hold Time in microseconds
-  - atmel,adc-ts-wires: Number of touchscreen wires. Should be 4 or 5. If this
-                        value is set, then the adc driver will enable touchscreen
-                        support.
-    NOTE: when adc touchscreen is enabled, the adc hardware trigger will be
-          disabled. Since touchscreen will occupy the trigger register.
-  - atmel,adc-ts-pressure-threshold: a pressure threshold for touchscreen. It
-                                     makes touch detection more precise.
-
-Optional trigger Nodes:
-  - Required properties:
-    * trigger-name: Name of the trigger exposed to the user
-    * trigger-value: Value to put in the Trigger register
-      to activate this trigger
-  - Optional properties:
-    * trigger-external: Is the trigger an external trigger?
-
-Examples:
-adc0: adc@fffb0000 {
-	#address-cells = <1>;
-	#size-cells = <0>;
-	compatible = "atmel,at91sam9260-adc";
-	reg = <0xfffb0000 0x100>;
-	interrupts = <20 IRQ_TYPE_LEVEL_HIGH 0>;
-	clocks = <&adc_clk>, <&adc_op_clk>;
-	clock-names = "adc_clk", "adc_op_clk";
-	atmel,adc-channels-used = <0xff>;
-	atmel,adc-startup-time = <40>;
-	atmel,adc-use-external-triggers;
-	atmel,adc-vref = <3300>;
-	atmel,adc-res = <8 10>;
-	atmel,adc-res-names = "lowres", "highres";
-	atmel,adc-use-res = "lowres";
-
-	trigger0 {
-		trigger-name = "external-rising";
-		trigger-value = <0x1>;
-		trigger-external;
-	};
-	trigger1 {
-		trigger-name = "external-falling";
-		trigger-value = <0x2>;
-		trigger-external;
-	};
-
-	trigger2 {
-		trigger-name = "external-any";
-		trigger-value = <0x3>;
-		trigger-external;
-	};
-
-	trigger3 {
-		trigger-name = "continuous";
-		trigger-value = <0x6>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
new file mode 100644
index 000000000000..9b0ff59e75de
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
@@ -0,0 +1,167 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/atmel,sama9260-adc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AT91 sama9260 and similar Analog to Digital Converter (ADC)
+
+maintainers:
+  - Alexandre Belloni <alexandre.belloni@bootlin.com>
+
+properties:
+  compatible:
+    enum:
+      - atmel,at91sam9260-adc
+      - atmel,at91sam9rl-adc
+      - atmel,at91sam9g45-adc
+      - atmel,at91sam9x5-adc
+      - atmel,at91sama5d3-adc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    minItems: 2
+    maxItems: 2
+
+  clock-names:
+    items:
+      - const: adc_clk
+      - const: adc_op_clk
+
+  atmel,adc-channels-used:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Bitmask of the channels muxed and enabled for this device
+
+  atmel,adc-startup-time:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Startup Time of the ADC in microseconds as defined in the datasheet
+
+  atmel,adc-vref:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Reference voltage in millivolts for the conversions
+
+  atmel,adc-use-external-triggers:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: Enable the external triggers
+
+  atmel,adc-use-res:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      String corresponding to an identifier from atmel,adc-res-names property.
+      If not specified, the highest resolution will be used.
+    enum:
+      - "lowres"
+      - "highres"
+
+  atmel,adc-sleep-mode:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description: Enable sleep mode when no conversion
+
+  atmel,adc-sample-hold-time:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Sample and Hold Time in microseconds
+
+  atmel,adc-ts-wires:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Number of touchscreen wires. Must be set to enable touchscreen.
+      NOTE: when adc touchscreen is enabled, the adc hardware trigger will be
+      disabled. Since touchscreen will occupy the trigger register.
+    enum:
+      - 4
+      - 5
+
+  atmel,adc-ts-pressure-threshold:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Pressure threshold for touchscreen.
+
+  "#io-channel-cells":
+    const: 1
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - atmel,adc-channels-used
+  - atmel,adc-startup-time
+  - atmel,adc-vref
+
+patternProperties:
+  "^(trigger)[0-9]$":
+    type: object
+    description: Child node to describe a trigger exposed to the user.
+    properties:
+      trigger-name:
+        $ref: /schemas/types.yaml#/definitions/string
+        description: Identifying name.
+
+      trigger-value:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          Value to put in the Trigger register to activate this trigger
+
+      trigger-external:
+        $ref: /schemas/types.yaml#/definitions/flag
+        description: This trigger is provided from an external pin.
+
+    additionalProperties: false
+    required:
+      - trigger-name
+      - trigger-value
+
+examples:
+  - |
+    #include <dt-bindings/dma/at91.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    soc {
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        adc@fffb0000 {
+            compatible = "atmel,at91sam9260-adc";
+            reg = <0xfffb0000 0x100>;
+            interrupts = <20 IRQ_TYPE_LEVEL_HIGH 0>;
+            clocks = <&adc_clk>, <&adc_op_clk>;
+            clock-names = "adc_clk", "adc_op_clk";
+            atmel,adc-channels-used = <0xff>;
+            atmel,adc-startup-time = <40>;
+            atmel,adc-use-external-triggers;
+            atmel,adc-vref = <3300>;
+            atmel,adc-use-res = "lowres";
+
+            trigger0 {
+                trigger-name = "external-rising";
+                trigger-value = <0x1>;
+                trigger-external;
+            };
+
+            trigger1 {
+                trigger-name = "external-falling";
+                trigger-value = <0x2>;
+                trigger-external;
+            };
+
+            trigger2 {
+                trigger-name = "external-any";
+                trigger-value = <0x3>;
+                trigger-external;
+            };
+
+            trigger3 {
+                trigger-name = "continuous";
+                trigger-value = <0x6>;
+            };
+        };
+    };
+...
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/9] dt-bindings:iio:adc:atmel,sama9260-adc: conversion to yaml from at91_adc.txt

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:44 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> There are a few things we would do differently in an ADC binding if we
> were starting from scratch but we are stuck with what we have (which
> made sense back when this was written!)
> 
> We may be able to tighten up some elements of this binding in the future
> by careful checking of what values properties can actually take.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
> Cc: Ludovic Desroches <ludovic.desroches@microchip.com>
> [Alexandre Belloni: add sama5d3, remove atmel,adc-res and atmel,adc-res-names]
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

Hi Alexandre,

Rob gave a tag for this in the series I originally posted it in.
It was 'hidden' in a reply to the cover letter which you weren't cc'd on
due to it being a huge patch set with too many people to cc! 
https://lore.kernel.org/linux-iio/20201103163800.GA1791071@bogus/

Please add it to a v2, or if we don't do one of those I can add it whilst
applying.

Thanks,

Jonathan


> ---
>  .../devicetree/bindings/iio/adc/at91_adc.txt  |  78 --------
>  .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 167 ++++++++++++++++++
>  2 files changed, 167 insertions(+), 78 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/iio/adc/at91_adc.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt b/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
> deleted file mode 100644
> index da393ac5c05f..000000000000
> --- a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
> +++ /dev/null
> @@ -1,78 +0,0 @@
> -* AT91's Analog to Digital Converter (ADC)
> -
> -Required properties:
> -  - compatible: Should be "atmel,<chip>-adc"
> -    <chip> can be "at91sam9260", "at91sam9g45", "at91sam9x5" or "sama5d3"
> -  - reg: Should contain ADC registers location and length
> -  - interrupts: Should contain the IRQ line for the ADC
> -  - clock-names: tuple listing input clock names.
> -	Required elements: "adc_clk", "adc_op_clk".
> -  - clocks: phandles to input clocks.
> -  - atmel,adc-channels-used: Bitmask of the channels muxed and enabled for this
> -    device
> -  - atmel,adc-startup-time: Startup Time of the ADC in microseconds as
> -    defined in the datasheet
> -  - atmel,adc-vref: Reference voltage in millivolts for the conversions
> -
> -Optional properties:
> -  - atmel,adc-use-external-triggers: Boolean to enable the external triggers
> -  - atmel,adc-use-res: String selecting the resolution, can be "lowres" or
> -		       "highres". If not specified, the highest resolution will
> -		       be used.
> -  - atmel,adc-sleep-mode: Boolean to enable sleep mode when no conversion
> -  - atmel,adc-sample-hold-time: Sample and Hold Time in microseconds
> -  - atmel,adc-ts-wires: Number of touchscreen wires. Should be 4 or 5. If this
> -                        value is set, then the adc driver will enable touchscreen
> -                        support.
> -    NOTE: when adc touchscreen is enabled, the adc hardware trigger will be
> -          disabled. Since touchscreen will occupy the trigger register.
> -  - atmel,adc-ts-pressure-threshold: a pressure threshold for touchscreen. It
> -                                     makes touch detection more precise.
> -
> -Optional trigger Nodes:
> -  - Required properties:
> -    * trigger-name: Name of the trigger exposed to the user
> -    * trigger-value: Value to put in the Trigger register
> -      to activate this trigger
> -  - Optional properties:
> -    * trigger-external: Is the trigger an external trigger?
> -
> -Examples:
> -adc0: adc@fffb0000 {
> -	#address-cells = <1>;
> -	#size-cells = <0>;
> -	compatible = "atmel,at91sam9260-adc";
> -	reg = <0xfffb0000 0x100>;
> -	interrupts = <20 IRQ_TYPE_LEVEL_HIGH 0>;
> -	clocks = <&adc_clk>, <&adc_op_clk>;
> -	clock-names = "adc_clk", "adc_op_clk";
> -	atmel,adc-channels-used = <0xff>;
> -	atmel,adc-startup-time = <40>;
> -	atmel,adc-use-external-triggers;
> -	atmel,adc-vref = <3300>;
> -	atmel,adc-res = <8 10>;
> -	atmel,adc-res-names = "lowres", "highres";
> -	atmel,adc-use-res = "lowres";
> -
> -	trigger0 {
> -		trigger-name = "external-rising";
> -		trigger-value = <0x1>;
> -		trigger-external;
> -	};
> -	trigger1 {
> -		trigger-name = "external-falling";
> -		trigger-value = <0x2>;
> -		trigger-external;
> -	};
> -
> -	trigger2 {
> -		trigger-name = "external-any";
> -		trigger-value = <0x3>;
> -		trigger-external;
> -	};
> -
> -	trigger3 {
> -		trigger-name = "continuous";
> -		trigger-value = <0x6>;
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> new file mode 100644
> index 000000000000..9b0ff59e75de
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> @@ -0,0 +1,167 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/atmel,sama9260-adc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: AT91 sama9260 and similar Analog to Digital Converter (ADC)
> +
> +maintainers:
> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - atmel,at91sam9260-adc
> +      - atmel,at91sam9rl-adc
> +      - atmel,at91sam9g45-adc
> +      - atmel,at91sam9x5-adc
> +      - atmel,at91sama5d3-adc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 2
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: adc_clk
> +      - const: adc_op_clk
> +
> +  atmel,adc-channels-used:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Bitmask of the channels muxed and enabled for this device
> +
> +  atmel,adc-startup-time:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Startup Time of the ADC in microseconds as defined in the datasheet
> +
> +  atmel,adc-vref:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Reference voltage in millivolts for the conversions
> +
> +  atmel,adc-use-external-triggers:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: Enable the external triggers
> +
> +  atmel,adc-use-res:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description:
> +      String corresponding to an identifier from atmel,adc-res-names property.
> +      If not specified, the highest resolution will be used.
> +    enum:
> +      - "lowres"
> +      - "highres"
> +
> +  atmel,adc-sleep-mode:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: Enable sleep mode when no conversion
> +
> +  atmel,adc-sample-hold-time:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Sample and Hold Time in microseconds
> +
> +  atmel,adc-ts-wires:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Number of touchscreen wires. Must be set to enable touchscreen.
> +      NOTE: when adc touchscreen is enabled, the adc hardware trigger will be
> +      disabled. Since touchscreen will occupy the trigger register.
> +    enum:
> +      - 4
> +      - 5
> +
> +  atmel,adc-ts-pressure-threshold:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Pressure threshold for touchscreen.
> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - atmel,adc-channels-used
> +  - atmel,adc-startup-time
> +  - atmel,adc-vref
> +
> +patternProperties:
> +  "^(trigger)[0-9]$":
> +    type: object
> +    description: Child node to describe a trigger exposed to the user.
> +    properties:
> +      trigger-name:
> +        $ref: /schemas/types.yaml#/definitions/string
> +        description: Identifying name.
> +
> +      trigger-value:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Value to put in the Trigger register to activate this trigger
> +
> +      trigger-external:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: This trigger is provided from an external pin.
> +
> +    additionalProperties: false
> +    required:
> +      - trigger-name
> +      - trigger-value
> +
> +examples:
> +  - |
> +    #include <dt-bindings/dma/at91.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    soc {
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        adc@fffb0000 {
> +            compatible = "atmel,at91sam9260-adc";
> +            reg = <0xfffb0000 0x100>;
> +            interrupts = <20 IRQ_TYPE_LEVEL_HIGH 0>;
> +            clocks = <&adc_clk>, <&adc_op_clk>;
> +            clock-names = "adc_clk", "adc_op_clk";
> +            atmel,adc-channels-used = <0xff>;
> +            atmel,adc-startup-time = <40>;
> +            atmel,adc-use-external-triggers;
> +            atmel,adc-vref = <3300>;
> +            atmel,adc-use-res = "lowres";
> +
> +            trigger0 {
> +                trigger-name = "external-rising";
> +                trigger-value = <0x1>;
> +                trigger-external;
> +            };
> +
> +            trigger1 {
> +                trigger-name = "external-falling";
> +                trigger-value = <0x2>;
> +                trigger-external;
> +            };
> +
> +            trigger2 {
> +                trigger-name = "external-any";
> +                trigger-value = <0x3>;
> +                trigger-external;
> +            };
> +
> +            trigger3 {
> +                trigger-name = "continuous";
> +                trigger-value = <0x6>;
> +            };
> +        };
> +    };
> +...

Re: [PATCH 3/9] dt-bindings:iio:adc:atmel,sama9260-adc: conversion to yaml from at91_adc.txt

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:44 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> There are a few things we would do differently in an ADC binding if we
> were starting from scratch but we are stuck with what we have (which
> made sense back when this was written!)
> 
> We may be able to tighten up some elements of this binding in the future
> by careful checking of what values properties can actually take.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
> Cc: Ludovic Desroches <ludovic.desroches@microchip.com>
> [Alexandre Belloni: add sama5d3, remove atmel,adc-res and atmel,adc-res-names]
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

Hi Alexandre,

Rob gave a tag for this in the series I originally posted it in.
It was 'hidden' in a reply to the cover letter which you weren't cc'd on
due to it being a huge patch set with too many people to cc! 
https://lore.kernel.org/linux-iio/20201103163800.GA1791071@bogus/

Please add it to a v2, or if we don't do one of those I can add it whilst
applying.

Thanks,

Jonathan


> ---
>  .../devicetree/bindings/iio/adc/at91_adc.txt  |  78 --------
>  .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 167 ++++++++++++++++++
>  2 files changed, 167 insertions(+), 78 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/iio/adc/at91_adc.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt b/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
> deleted file mode 100644
> index da393ac5c05f..000000000000
> --- a/Documentation/devicetree/bindings/iio/adc/at91_adc.txt
> +++ /dev/null
> @@ -1,78 +0,0 @@
> -* AT91's Analog to Digital Converter (ADC)
> -
> -Required properties:
> -  - compatible: Should be "atmel,<chip>-adc"
> -    <chip> can be "at91sam9260", "at91sam9g45", "at91sam9x5" or "sama5d3"
> -  - reg: Should contain ADC registers location and length
> -  - interrupts: Should contain the IRQ line for the ADC
> -  - clock-names: tuple listing input clock names.
> -	Required elements: "adc_clk", "adc_op_clk".
> -  - clocks: phandles to input clocks.
> -  - atmel,adc-channels-used: Bitmask of the channels muxed and enabled for this
> -    device
> -  - atmel,adc-startup-time: Startup Time of the ADC in microseconds as
> -    defined in the datasheet
> -  - atmel,adc-vref: Reference voltage in millivolts for the conversions
> -
> -Optional properties:
> -  - atmel,adc-use-external-triggers: Boolean to enable the external triggers
> -  - atmel,adc-use-res: String selecting the resolution, can be "lowres" or
> -		       "highres". If not specified, the highest resolution will
> -		       be used.
> -  - atmel,adc-sleep-mode: Boolean to enable sleep mode when no conversion
> -  - atmel,adc-sample-hold-time: Sample and Hold Time in microseconds
> -  - atmel,adc-ts-wires: Number of touchscreen wires. Should be 4 or 5. If this
> -                        value is set, then the adc driver will enable touchscreen
> -                        support.
> -    NOTE: when adc touchscreen is enabled, the adc hardware trigger will be
> -          disabled. Since touchscreen will occupy the trigger register.
> -  - atmel,adc-ts-pressure-threshold: a pressure threshold for touchscreen. It
> -                                     makes touch detection more precise.
> -
> -Optional trigger Nodes:
> -  - Required properties:
> -    * trigger-name: Name of the trigger exposed to the user
> -    * trigger-value: Value to put in the Trigger register
> -      to activate this trigger
> -  - Optional properties:
> -    * trigger-external: Is the trigger an external trigger?
> -
> -Examples:
> -adc0: adc@fffb0000 {
> -	#address-cells = <1>;
> -	#size-cells = <0>;
> -	compatible = "atmel,at91sam9260-adc";
> -	reg = <0xfffb0000 0x100>;
> -	interrupts = <20 IRQ_TYPE_LEVEL_HIGH 0>;
> -	clocks = <&adc_clk>, <&adc_op_clk>;
> -	clock-names = "adc_clk", "adc_op_clk";
> -	atmel,adc-channels-used = <0xff>;
> -	atmel,adc-startup-time = <40>;
> -	atmel,adc-use-external-triggers;
> -	atmel,adc-vref = <3300>;
> -	atmel,adc-res = <8 10>;
> -	atmel,adc-res-names = "lowres", "highres";
> -	atmel,adc-use-res = "lowres";
> -
> -	trigger0 {
> -		trigger-name = "external-rising";
> -		trigger-value = <0x1>;
> -		trigger-external;
> -	};
> -	trigger1 {
> -		trigger-name = "external-falling";
> -		trigger-value = <0x2>;
> -		trigger-external;
> -	};
> -
> -	trigger2 {
> -		trigger-name = "external-any";
> -		trigger-value = <0x3>;
> -		trigger-external;
> -	};
> -
> -	trigger3 {
> -		trigger-name = "continuous";
> -		trigger-value = <0x6>;
> -	};
> -};
> diff --git a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> new file mode 100644
> index 000000000000..9b0ff59e75de
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> @@ -0,0 +1,167 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/atmel,sama9260-adc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: AT91 sama9260 and similar Analog to Digital Converter (ADC)
> +
> +maintainers:
> +  - Alexandre Belloni <alexandre.belloni@bootlin.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - atmel,at91sam9260-adc
> +      - atmel,at91sam9rl-adc
> +      - atmel,at91sam9g45-adc
> +      - atmel,at91sam9x5-adc
> +      - atmel,at91sama5d3-adc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    minItems: 2
> +    maxItems: 2
> +
> +  clock-names:
> +    items:
> +      - const: adc_clk
> +      - const: adc_op_clk
> +
> +  atmel,adc-channels-used:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Bitmask of the channels muxed and enabled for this device
> +
> +  atmel,adc-startup-time:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Startup Time of the ADC in microseconds as defined in the datasheet
> +
> +  atmel,adc-vref:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Reference voltage in millivolts for the conversions
> +
> +  atmel,adc-use-external-triggers:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: Enable the external triggers
> +
> +  atmel,adc-use-res:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description:
> +      String corresponding to an identifier from atmel,adc-res-names property.
> +      If not specified, the highest resolution will be used.
> +    enum:
> +      - "lowres"
> +      - "highres"
> +
> +  atmel,adc-sleep-mode:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: Enable sleep mode when no conversion
> +
> +  atmel,adc-sample-hold-time:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Sample and Hold Time in microseconds
> +
> +  atmel,adc-ts-wires:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Number of touchscreen wires. Must be set to enable touchscreen.
> +      NOTE: when adc touchscreen is enabled, the adc hardware trigger will be
> +      disabled. Since touchscreen will occupy the trigger register.
> +    enum:
> +      - 4
> +      - 5
> +
> +  atmel,adc-ts-pressure-threshold:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Pressure threshold for touchscreen.
> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - atmel,adc-channels-used
> +  - atmel,adc-startup-time
> +  - atmel,adc-vref
> +
> +patternProperties:
> +  "^(trigger)[0-9]$":
> +    type: object
> +    description: Child node to describe a trigger exposed to the user.
> +    properties:
> +      trigger-name:
> +        $ref: /schemas/types.yaml#/definitions/string
> +        description: Identifying name.
> +
> +      trigger-value:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          Value to put in the Trigger register to activate this trigger
> +
> +      trigger-external:
> +        $ref: /schemas/types.yaml#/definitions/flag
> +        description: This trigger is provided from an external pin.
> +
> +    additionalProperties: false
> +    required:
> +      - trigger-name
> +      - trigger-value
> +
> +examples:
> +  - |
> +    #include <dt-bindings/dma/at91.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    soc {
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        adc@fffb0000 {
> +            compatible = "atmel,at91sam9260-adc";
> +            reg = <0xfffb0000 0x100>;
> +            interrupts = <20 IRQ_TYPE_LEVEL_HIGH 0>;
> +            clocks = <&adc_clk>, <&adc_op_clk>;
> +            clock-names = "adc_clk", "adc_op_clk";
> +            atmel,adc-channels-used = <0xff>;
> +            atmel,adc-startup-time = <40>;
> +            atmel,adc-use-external-triggers;
> +            atmel,adc-vref = <3300>;
> +            atmel,adc-use-res = "lowres";
> +
> +            trigger0 {
> +                trigger-name = "external-rising";
> +                trigger-value = <0x1>;
> +                trigger-external;
> +            };
> +
> +            trigger1 {
> +                trigger-name = "external-falling";
> +                trigger-value = <0x2>;
> +                trigger-external;
> +            };
> +
> +            trigger2 {
> +                trigger-name = "external-any";
> +                trigger-value = <0x3>;
> +                trigger-external;
> +            };
> +
> +            trigger3 {
> +                trigger-name = "continuous";
> +                trigger-value = <0x6>;
> +            };
> +        };
> +    };
> +...


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/9] iio: adc: at91_adc: rework trigger definition

[Alexandre Belloni]

Move the available trigger definition back in the driver to stop cluttering
the device tree. There is no functional change except that it actually
fixes the available triggers for at91sam9rl as it inherited the list from
at91sam9260 but actually has the triggers from at91sam9x5.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 46 -----------
 drivers/iio/adc/at91_adc.c                    | 80 +++++++++----------
 2 files changed, 36 insertions(+), 90 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
index 9b0ff59e75de..e6a1f915b542 100644
--- a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
@@ -97,29 +97,6 @@ required:
   - atmel,adc-startup-time
   - atmel,adc-vref
 
-patternProperties:
-  "^(trigger)[0-9]$":
-    type: object
-    description: Child node to describe a trigger exposed to the user.
-    properties:
-      trigger-name:
-        $ref: /schemas/types.yaml#/definitions/string
-        description: Identifying name.
-
-      trigger-value:
-        $ref: /schemas/types.yaml#/definitions/uint32
-        description:
-          Value to put in the Trigger register to activate this trigger
-
-      trigger-external:
-        $ref: /schemas/types.yaml#/definitions/flag
-        description: This trigger is provided from an external pin.
-
-    additionalProperties: false
-    required:
-      - trigger-name
-      - trigger-value
-
 examples:
   - |
     #include <dt-bindings/dma/at91.h>
@@ -139,29 +116,6 @@ examples:
             atmel,adc-use-external-triggers;
             atmel,adc-vref = <3300>;
             atmel,adc-use-res = "lowres";
-
-            trigger0 {
-                trigger-name = "external-rising";
-                trigger-value = <0x1>;
-                trigger-external;
-            };
-
-            trigger1 {
-                trigger-name = "external-falling";
-                trigger-value = <0x2>;
-                trigger-external;
-            };
-
-            trigger2 {
-                trigger-name = "external-any";
-                trigger-value = <0x3>;
-                trigger-external;
-            };
-
-            trigger3 {
-                trigger-name = "continuous";
-                trigger-value = <0x6>;
-            };
         };
     };
 ...
diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 0d67c812ef3d..83539422b704 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -207,6 +207,8 @@ struct at91_adc_caps {
 
 	u8	low_res_bits;
 	u8	high_res_bits;
+	u32	trigger_number;
+	const struct at91_adc_trigger *triggers;
 	struct at91_adc_reg_desc registers;
 };
 
@@ -227,8 +229,6 @@ struct at91_adc_state {
 	u8			sample_hold_time;
 	bool			sleep_mode;
 	struct iio_trigger	**trig;
-	struct at91_adc_trigger	*trigger_list;
-	u32			trigger_number;
 	bool			use_external;
 	u32			vref_mv;
 	u32			res;		/* resolution used for convertions */
@@ -537,13 +537,13 @@ static int at91_adc_channel_init(struct iio_dev *idev)
 }
 
 static int at91_adc_get_trigger_value_by_name(struct iio_dev *idev,
-					     struct at91_adc_trigger *triggers,
+					     const struct at91_adc_trigger *triggers,
 					     const char *trigger_name)
 {
 	struct at91_adc_state *st = iio_priv(idev);
 	int i;
 
-	for (i = 0; i < st->trigger_number; i++) {
+	for (i = 0; i < st->caps->trigger_number; i++) {
 		char *name = kasprintf(GFP_KERNEL,
 				"%s-dev%d-%s",
 				idev->name,
@@ -575,7 +575,7 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
 	u8 bit;
 
 	value = at91_adc_get_trigger_value_by_name(idev,
-						   st->trigger_list,
+						   st->caps->triggers,
 						   idev->trig->name);
 	if (value < 0)
 		return value;
@@ -620,7 +620,7 @@ static const struct iio_trigger_ops at91_adc_trigger_ops = {
 };
 
 static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *idev,
-						     struct at91_adc_trigger *trigger)
+						     const struct at91_adc_trigger *trigger)
 {
 	struct iio_trigger *trig;
 	int ret;
@@ -647,7 +647,7 @@ static int at91_adc_trigger_init(struct iio_dev *idev)
 	int i, ret;
 
 	st->trig = devm_kcalloc(&idev->dev,
-				st->trigger_number, sizeof(*st->trig),
+				st->caps->trigger_number, sizeof(*st->trig),
 				GFP_KERNEL);
 
 	if (st->trig == NULL) {
@@ -655,12 +655,12 @@ static int at91_adc_trigger_init(struct iio_dev *idev)
 		goto error_ret;
 	}
 
-	for (i = 0; i < st->trigger_number; i++) {
-		if (st->trigger_list[i].is_external && !(st->use_external))
+	for (i = 0; i < st->caps->trigger_number; i++) {
+		if (st->caps->triggers[i].is_external && !(st->use_external))
 			continue;
 
 		st->trig[i] = at91_adc_allocate_trigger(idev,
-							st->trigger_list + i);
+							st->caps->triggers + i);
 		if (st->trig[i] == NULL) {
 			dev_err(&idev->dev,
 				"Could not allocate trigger %d\n", i);
@@ -685,7 +685,7 @@ static void at91_adc_trigger_remove(struct iio_dev *idev)
 	struct at91_adc_state *st = iio_priv(idev);
 	int i;
 
-	for (i = 0; i < st->trigger_number; i++) {
+	for (i = 0; i < st->caps->trigger_number; i++) {
 		iio_trigger_unregister(st->trig[i]);
 		iio_trigger_free(st->trig[i]);
 	}
@@ -838,8 +838,7 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
 {
 	struct at91_adc_state *st = iio_priv(idev);
 	struct device_node *node = pdev->dev.of_node;
-	struct device_node *trig_node;
-	int i = 0, ret;
+	int ret;
 	u32 prop;
 	char *s;
 
@@ -885,37 +884,6 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
 
 	st->registers = &st->caps->registers;
 	st->num_channels = st->caps->num_channels;
-	st->trigger_number = of_get_child_count(node);
-	st->trigger_list = devm_kcalloc(&idev->dev,
-					st->trigger_number,
-					sizeof(struct at91_adc_trigger),
-					GFP_KERNEL);
-	if (!st->trigger_list) {
-		dev_err(&idev->dev, "Could not allocate trigger list memory.\n");
-		ret = -ENOMEM;
-		goto error_ret;
-	}
-
-	for_each_child_of_node(node, trig_node) {
-		struct at91_adc_trigger *trig = st->trigger_list + i;
-		const char *name;
-
-		if (of_property_read_string(trig_node, "trigger-name", &name)) {
-			dev_err(&idev->dev, "Missing trigger-name property in the DT.\n");
-			ret = -EINVAL;
-			goto error_ret;
-		}
-		trig->name = name;
-
-		if (of_property_read_u32(trig_node, "trigger-value", &prop)) {
-			dev_err(&idev->dev, "Missing trigger-value property in the DT.\n");
-			ret = -EINVAL;
-			goto error_ret;
-		}
-		trig->value = prop;
-		trig->is_external = of_property_read_bool(trig_node, "trigger-external");
-		i++;
-	}
 
 	/* Check if touchscreen is supported. */
 	if (st->caps->has_ts)
@@ -1315,6 +1283,13 @@ static int at91_adc_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(at91_adc_pm_ops, at91_adc_suspend, at91_adc_resume);
 
+static const struct at91_adc_trigger at91sam9260_triggers[] = {
+	{ .name = "timer-counter-0", .value = 0x1 },
+	{ .name = "timer-counter-1", .value = 0x3 },
+	{ .name = "timer-counter-2", .value = 0x5 },
+	{ .name = "external", .value = 0xd, .is_external = true },
+};
+
 static struct at91_adc_caps at91sam9260_caps = {
 	.calc_startup_ticks = calc_startup_ticks_9260,
 	.num_channels = 4,
@@ -1328,6 +1303,15 @@ static struct at91_adc_caps at91sam9260_caps = {
 		.mr_prescal_mask = AT91_ADC_PRESCAL_9260,
 		.mr_startup_mask = AT91_ADC_STARTUP_9260,
 	},
+	.triggers = at91sam9260_triggers,
+	.trigger_number = ARRAY_SIZE(at91sam9260_triggers),
+};
+
+static const struct at91_adc_trigger at91sam9x5_triggers[] = {
+	{ .name = "external-rising", .value = 0x1, .is_external = true },
+	{ .name = "external-falling", .value = 0x2, .is_external = true },
+	{ .name = "external-any", .value = 0x3, .is_external = true },
+	{ .name = "continuous", .value = 0x6 },
 };
 
 static struct at91_adc_caps at91sam9rl_caps = {
@@ -1344,6 +1328,8 @@ static struct at91_adc_caps at91sam9rl_caps = {
 		.mr_prescal_mask = AT91_ADC_PRESCAL_9260,
 		.mr_startup_mask = AT91_ADC_STARTUP_9G45,
 	},
+	.triggers = at91sam9x5_triggers,
+	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
 };
 
 static struct at91_adc_caps at91sam9g45_caps = {
@@ -1360,6 +1346,8 @@ static struct at91_adc_caps at91sam9g45_caps = {
 		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
 		.mr_startup_mask = AT91_ADC_STARTUP_9G45,
 	},
+	.triggers = at91sam9x5_triggers,
+	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
 };
 
 static struct at91_adc_caps at91sam9x5_caps = {
@@ -1380,6 +1368,8 @@ static struct at91_adc_caps at91sam9x5_caps = {
 		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
 		.mr_startup_mask = AT91_ADC_STARTUP_9X5,
 	},
+	.triggers = at91sam9x5_triggers,
+	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
 };
 
 static struct at91_adc_caps sama5d3_caps = {
@@ -1399,6 +1389,8 @@ static struct at91_adc_caps sama5d3_caps = {
 		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
 		.mr_startup_mask = AT91_ADC_STARTUP_9X5,
 	},
+	.triggers = at91sam9x5_triggers,
+	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
 };
 
 static const struct of_device_id at91_adc_dt_ids[] = {
-- 
2.28.0

[PATCH 4/9] iio: adc: at91_adc: rework trigger definition

[Alexandre Belloni]

Move the available trigger definition back in the driver to stop cluttering
the device tree. There is no functional change except that it actually
fixes the available triggers for at91sam9rl as it inherited the list from
at91sam9260 but actually has the triggers from at91sam9x5.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 46 -----------
 drivers/iio/adc/at91_adc.c                    | 80 +++++++++----------
 2 files changed, 36 insertions(+), 90 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
index 9b0ff59e75de..e6a1f915b542 100644
--- a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
@@ -97,29 +97,6 @@ required:
   - atmel,adc-startup-time
   - atmel,adc-vref
 
-patternProperties:
-  "^(trigger)[0-9]$":
-    type: object
-    description: Child node to describe a trigger exposed to the user.
-    properties:
-      trigger-name:
-        $ref: /schemas/types.yaml#/definitions/string
-        description: Identifying name.
-
-      trigger-value:
-        $ref: /schemas/types.yaml#/definitions/uint32
-        description:
-          Value to put in the Trigger register to activate this trigger
-
-      trigger-external:
-        $ref: /schemas/types.yaml#/definitions/flag
-        description: This trigger is provided from an external pin.
-
-    additionalProperties: false
-    required:
-      - trigger-name
-      - trigger-value
-
 examples:
   - |
     #include <dt-bindings/dma/at91.h>
@@ -139,29 +116,6 @@ examples:
             atmel,adc-use-external-triggers;
             atmel,adc-vref = <3300>;
             atmel,adc-use-res = "lowres";
-
-            trigger0 {
-                trigger-name = "external-rising";
-                trigger-value = <0x1>;
-                trigger-external;
-            };
-
-            trigger1 {
-                trigger-name = "external-falling";
-                trigger-value = <0x2>;
-                trigger-external;
-            };
-
-            trigger2 {
-                trigger-name = "external-any";
-                trigger-value = <0x3>;
-                trigger-external;
-            };
-
-            trigger3 {
-                trigger-name = "continuous";
-                trigger-value = <0x6>;
-            };
         };
     };
 ...
diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 0d67c812ef3d..83539422b704 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -207,6 +207,8 @@ struct at91_adc_caps {
 
 	u8	low_res_bits;
 	u8	high_res_bits;
+	u32	trigger_number;
+	const struct at91_adc_trigger *triggers;
 	struct at91_adc_reg_desc registers;
 };
 
@@ -227,8 +229,6 @@ struct at91_adc_state {
 	u8			sample_hold_time;
 	bool			sleep_mode;
 	struct iio_trigger	**trig;
-	struct at91_adc_trigger	*trigger_list;
-	u32			trigger_number;
 	bool			use_external;
 	u32			vref_mv;
 	u32			res;		/* resolution used for convertions */
@@ -537,13 +537,13 @@ static int at91_adc_channel_init(struct iio_dev *idev)
 }
 
 static int at91_adc_get_trigger_value_by_name(struct iio_dev *idev,
-					     struct at91_adc_trigger *triggers,
+					     const struct at91_adc_trigger *triggers,
 					     const char *trigger_name)
 {
 	struct at91_adc_state *st = iio_priv(idev);
 	int i;
 
-	for (i = 0; i < st->trigger_number; i++) {
+	for (i = 0; i < st->caps->trigger_number; i++) {
 		char *name = kasprintf(GFP_KERNEL,
 				"%s-dev%d-%s",
 				idev->name,
@@ -575,7 +575,7 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
 	u8 bit;
 
 	value = at91_adc_get_trigger_value_by_name(idev,
-						   st->trigger_list,
+						   st->caps->triggers,
 						   idev->trig->name);
 	if (value < 0)
 		return value;
@@ -620,7 +620,7 @@ static const struct iio_trigger_ops at91_adc_trigger_ops = {
 };
 
 static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *idev,
-						     struct at91_adc_trigger *trigger)
+						     const struct at91_adc_trigger *trigger)
 {
 	struct iio_trigger *trig;
 	int ret;
@@ -647,7 +647,7 @@ static int at91_adc_trigger_init(struct iio_dev *idev)
 	int i, ret;
 
 	st->trig = devm_kcalloc(&idev->dev,
-				st->trigger_number, sizeof(*st->trig),
+				st->caps->trigger_number, sizeof(*st->trig),
 				GFP_KERNEL);
 
 	if (st->trig == NULL) {
@@ -655,12 +655,12 @@ static int at91_adc_trigger_init(struct iio_dev *idev)
 		goto error_ret;
 	}
 
-	for (i = 0; i < st->trigger_number; i++) {
-		if (st->trigger_list[i].is_external && !(st->use_external))
+	for (i = 0; i < st->caps->trigger_number; i++) {
+		if (st->caps->triggers[i].is_external && !(st->use_external))
 			continue;
 
 		st->trig[i] = at91_adc_allocate_trigger(idev,
-							st->trigger_list + i);
+							st->caps->triggers + i);
 		if (st->trig[i] == NULL) {
 			dev_err(&idev->dev,
 				"Could not allocate trigger %d\n", i);
@@ -685,7 +685,7 @@ static void at91_adc_trigger_remove(struct iio_dev *idev)
 	struct at91_adc_state *st = iio_priv(idev);
 	int i;
 
-	for (i = 0; i < st->trigger_number; i++) {
+	for (i = 0; i < st->caps->trigger_number; i++) {
 		iio_trigger_unregister(st->trig[i]);
 		iio_trigger_free(st->trig[i]);
 	}
@@ -838,8 +838,7 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
 {
 	struct at91_adc_state *st = iio_priv(idev);
 	struct device_node *node = pdev->dev.of_node;
-	struct device_node *trig_node;
-	int i = 0, ret;
+	int ret;
 	u32 prop;
 	char *s;
 
@@ -885,37 +884,6 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
 
 	st->registers = &st->caps->registers;
 	st->num_channels = st->caps->num_channels;
-	st->trigger_number = of_get_child_count(node);
-	st->trigger_list = devm_kcalloc(&idev->dev,
-					st->trigger_number,
-					sizeof(struct at91_adc_trigger),
-					GFP_KERNEL);
-	if (!st->trigger_list) {
-		dev_err(&idev->dev, "Could not allocate trigger list memory.\n");
-		ret = -ENOMEM;
-		goto error_ret;
-	}
-
-	for_each_child_of_node(node, trig_node) {
-		struct at91_adc_trigger *trig = st->trigger_list + i;
-		const char *name;
-
-		if (of_property_read_string(trig_node, "trigger-name", &name)) {
-			dev_err(&idev->dev, "Missing trigger-name property in the DT.\n");
-			ret = -EINVAL;
-			goto error_ret;
-		}
-		trig->name = name;
-
-		if (of_property_read_u32(trig_node, "trigger-value", &prop)) {
-			dev_err(&idev->dev, "Missing trigger-value property in the DT.\n");
-			ret = -EINVAL;
-			goto error_ret;
-		}
-		trig->value = prop;
-		trig->is_external = of_property_read_bool(trig_node, "trigger-external");
-		i++;
-	}
 
 	/* Check if touchscreen is supported. */
 	if (st->caps->has_ts)
@@ -1315,6 +1283,13 @@ static int at91_adc_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(at91_adc_pm_ops, at91_adc_suspend, at91_adc_resume);
 
+static const struct at91_adc_trigger at91sam9260_triggers[] = {
+	{ .name = "timer-counter-0", .value = 0x1 },
+	{ .name = "timer-counter-1", .value = 0x3 },
+	{ .name = "timer-counter-2", .value = 0x5 },
+	{ .name = "external", .value = 0xd, .is_external = true },
+};
+
 static struct at91_adc_caps at91sam9260_caps = {
 	.calc_startup_ticks = calc_startup_ticks_9260,
 	.num_channels = 4,
@@ -1328,6 +1303,15 @@ static struct at91_adc_caps at91sam9260_caps = {
 		.mr_prescal_mask = AT91_ADC_PRESCAL_9260,
 		.mr_startup_mask = AT91_ADC_STARTUP_9260,
 	},
+	.triggers = at91sam9260_triggers,
+	.trigger_number = ARRAY_SIZE(at91sam9260_triggers),
+};
+
+static const struct at91_adc_trigger at91sam9x5_triggers[] = {
+	{ .name = "external-rising", .value = 0x1, .is_external = true },
+	{ .name = "external-falling", .value = 0x2, .is_external = true },
+	{ .name = "external-any", .value = 0x3, .is_external = true },
+	{ .name = "continuous", .value = 0x6 },
 };
 
 static struct at91_adc_caps at91sam9rl_caps = {
@@ -1344,6 +1328,8 @@ static struct at91_adc_caps at91sam9rl_caps = {
 		.mr_prescal_mask = AT91_ADC_PRESCAL_9260,
 		.mr_startup_mask = AT91_ADC_STARTUP_9G45,
 	},
+	.triggers = at91sam9x5_triggers,
+	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
 };
 
 static struct at91_adc_caps at91sam9g45_caps = {
@@ -1360,6 +1346,8 @@ static struct at91_adc_caps at91sam9g45_caps = {
 		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
 		.mr_startup_mask = AT91_ADC_STARTUP_9G45,
 	},
+	.triggers = at91sam9x5_triggers,
+	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
 };
 
 static struct at91_adc_caps at91sam9x5_caps = {
@@ -1380,6 +1368,8 @@ static struct at91_adc_caps at91sam9x5_caps = {
 		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
 		.mr_startup_mask = AT91_ADC_STARTUP_9X5,
 	},
+	.triggers = at91sam9x5_triggers,
+	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
 };
 
 static struct at91_adc_caps sama5d3_caps = {
@@ -1399,6 +1389,8 @@ static struct at91_adc_caps sama5d3_caps = {
 		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
 		.mr_startup_mask = AT91_ADC_STARTUP_9X5,
 	},
+	.triggers = at91sam9x5_triggers,
+	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
 };
 
 static const struct of_device_id at91_adc_dt_ids[] = {
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/9] iio: adc: at91_adc: rework trigger definition

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:45 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> Move the available trigger definition back in the driver to stop cluttering
> the device tree. There is no functional change except that it actually
> fixes the available triggers for at91sam9rl as it inherited the list from
> at91sam9260 but actually has the triggers from at91sam9x5.

Is that a fix we might want to backport?  If not we should probably put a clear
statement in here to avoid it getting picked up by the bot.

I'd argue it's to invasive a change to backport.

Otherwise, sensible looking change.

> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
>  .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 46 -----------
>  drivers/iio/adc/at91_adc.c                    | 80 +++++++++----------
>  2 files changed, 36 insertions(+), 90 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> index 9b0ff59e75de..e6a1f915b542 100644
> --- a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> @@ -97,29 +97,6 @@ required:
>    - atmel,adc-startup-time
>    - atmel,adc-vref
>  
> -patternProperties:
> -  "^(trigger)[0-9]$":
> -    type: object
> -    description: Child node to describe a trigger exposed to the user.
> -    properties:
> -      trigger-name:
> -        $ref: /schemas/types.yaml#/definitions/string
> -        description: Identifying name.
> -
> -      trigger-value:
> -        $ref: /schemas/types.yaml#/definitions/uint32
> -        description:
> -          Value to put in the Trigger register to activate this trigger
> -
> -      trigger-external:
> -        $ref: /schemas/types.yaml#/definitions/flag
> -        description: This trigger is provided from an external pin.
> -
> -    additionalProperties: false
> -    required:
> -      - trigger-name
> -      - trigger-value
> -
>  examples:
>    - |
>      #include <dt-bindings/dma/at91.h>
> @@ -139,29 +116,6 @@ examples:
>              atmel,adc-use-external-triggers;
>              atmel,adc-vref = <3300>;
>              atmel,adc-use-res = "lowres";
> -
> -            trigger0 {
> -                trigger-name = "external-rising";
> -                trigger-value = <0x1>;
> -                trigger-external;
> -            };
> -
> -            trigger1 {
> -                trigger-name = "external-falling";
> -                trigger-value = <0x2>;
> -                trigger-external;
> -            };
> -
> -            trigger2 {
> -                trigger-name = "external-any";
> -                trigger-value = <0x3>;
> -                trigger-external;
> -            };
> -
> -            trigger3 {
> -                trigger-name = "continuous";
> -                trigger-value = <0x6>;
> -            };
>          };
>      };
>  ...
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 0d67c812ef3d..83539422b704 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -207,6 +207,8 @@ struct at91_adc_caps {
>  
>  	u8	low_res_bits;
>  	u8	high_res_bits;
> +	u32	trigger_number;
> +	const struct at91_adc_trigger *triggers;
>  	struct at91_adc_reg_desc registers;
>  };
>  
> @@ -227,8 +229,6 @@ struct at91_adc_state {
>  	u8			sample_hold_time;
>  	bool			sleep_mode;
>  	struct iio_trigger	**trig;
> -	struct at91_adc_trigger	*trigger_list;
> -	u32			trigger_number;
>  	bool			use_external;
>  	u32			vref_mv;
>  	u32			res;		/* resolution used for convertions */
> @@ -537,13 +537,13 @@ static int at91_adc_channel_init(struct iio_dev *idev)
>  }
>  
>  static int at91_adc_get_trigger_value_by_name(struct iio_dev *idev,
> -					     struct at91_adc_trigger *triggers,
> +					     const struct at91_adc_trigger *triggers,
>  					     const char *trigger_name)
>  {
>  	struct at91_adc_state *st = iio_priv(idev);
>  	int i;
>  
> -	for (i = 0; i < st->trigger_number; i++) {
> +	for (i = 0; i < st->caps->trigger_number; i++) {
>  		char *name = kasprintf(GFP_KERNEL,
>  				"%s-dev%d-%s",
>  				idev->name,
> @@ -575,7 +575,7 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
>  	u8 bit;
>  
>  	value = at91_adc_get_trigger_value_by_name(idev,
> -						   st->trigger_list,
> +						   st->caps->triggers,
>  						   idev->trig->name);
>  	if (value < 0)
>  		return value;
> @@ -620,7 +620,7 @@ static const struct iio_trigger_ops at91_adc_trigger_ops = {
>  };
>  
>  static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *idev,
> -						     struct at91_adc_trigger *trigger)
> +						     const struct at91_adc_trigger *trigger)
>  {
>  	struct iio_trigger *trig;
>  	int ret;
> @@ -647,7 +647,7 @@ static int at91_adc_trigger_init(struct iio_dev *idev)
>  	int i, ret;
>  
>  	st->trig = devm_kcalloc(&idev->dev,
> -				st->trigger_number, sizeof(*st->trig),
> +				st->caps->trigger_number, sizeof(*st->trig),
>  				GFP_KERNEL);
>  
>  	if (st->trig == NULL) {
> @@ -655,12 +655,12 @@ static int at91_adc_trigger_init(struct iio_dev *idev)
>  		goto error_ret;
>  	}
>  
> -	for (i = 0; i < st->trigger_number; i++) {
> -		if (st->trigger_list[i].is_external && !(st->use_external))
> +	for (i = 0; i < st->caps->trigger_number; i++) {
> +		if (st->caps->triggers[i].is_external && !(st->use_external))
>  			continue;
>  
>  		st->trig[i] = at91_adc_allocate_trigger(idev,
> -							st->trigger_list + i);
> +							st->caps->triggers + i);
>  		if (st->trig[i] == NULL) {
>  			dev_err(&idev->dev,
>  				"Could not allocate trigger %d\n", i);
> @@ -685,7 +685,7 @@ static void at91_adc_trigger_remove(struct iio_dev *idev)
>  	struct at91_adc_state *st = iio_priv(idev);
>  	int i;
>  
> -	for (i = 0; i < st->trigger_number; i++) {
> +	for (i = 0; i < st->caps->trigger_number; i++) {
>  		iio_trigger_unregister(st->trig[i]);
>  		iio_trigger_free(st->trig[i]);
>  	}
> @@ -838,8 +838,7 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
>  {
>  	struct at91_adc_state *st = iio_priv(idev);
>  	struct device_node *node = pdev->dev.of_node;
> -	struct device_node *trig_node;
> -	int i = 0, ret;
> +	int ret;
>  	u32 prop;
>  	char *s;
>  
> @@ -885,37 +884,6 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
>  
>  	st->registers = &st->caps->registers;
>  	st->num_channels = st->caps->num_channels;
> -	st->trigger_number = of_get_child_count(node);
> -	st->trigger_list = devm_kcalloc(&idev->dev,
> -					st->trigger_number,
> -					sizeof(struct at91_adc_trigger),
> -					GFP_KERNEL);
> -	if (!st->trigger_list) {
> -		dev_err(&idev->dev, "Could not allocate trigger list memory.\n");
> -		ret = -ENOMEM;
> -		goto error_ret;
> -	}
> -
> -	for_each_child_of_node(node, trig_node) {
> -		struct at91_adc_trigger *trig = st->trigger_list + i;
> -		const char *name;
> -
> -		if (of_property_read_string(trig_node, "trigger-name", &name)) {
> -			dev_err(&idev->dev, "Missing trigger-name property in the DT.\n");
> -			ret = -EINVAL;
> -			goto error_ret;
> -		}
> -		trig->name = name;
> -
> -		if (of_property_read_u32(trig_node, "trigger-value", &prop)) {
> -			dev_err(&idev->dev, "Missing trigger-value property in the DT.\n");
> -			ret = -EINVAL;
> -			goto error_ret;
> -		}
> -		trig->value = prop;
> -		trig->is_external = of_property_read_bool(trig_node, "trigger-external");
> -		i++;
> -	}
>  
>  	/* Check if touchscreen is supported. */
>  	if (st->caps->has_ts)
> @@ -1315,6 +1283,13 @@ static int at91_adc_resume(struct device *dev)
>  
>  static SIMPLE_DEV_PM_OPS(at91_adc_pm_ops, at91_adc_suspend, at91_adc_resume);
>  
> +static const struct at91_adc_trigger at91sam9260_triggers[] = {
> +	{ .name = "timer-counter-0", .value = 0x1 },
> +	{ .name = "timer-counter-1", .value = 0x3 },
> +	{ .name = "timer-counter-2", .value = 0x5 },
> +	{ .name = "external", .value = 0xd, .is_external = true },
> +};
> +
>  static struct at91_adc_caps at91sam9260_caps = {
>  	.calc_startup_ticks = calc_startup_ticks_9260,
>  	.num_channels = 4,
> @@ -1328,6 +1303,15 @@ static struct at91_adc_caps at91sam9260_caps = {
>  		.mr_prescal_mask = AT91_ADC_PRESCAL_9260,
>  		.mr_startup_mask = AT91_ADC_STARTUP_9260,
>  	},
> +	.triggers = at91sam9260_triggers,
> +	.trigger_number = ARRAY_SIZE(at91sam9260_triggers),
> +};
> +
> +static const struct at91_adc_trigger at91sam9x5_triggers[] = {
> +	{ .name = "external-rising", .value = 0x1, .is_external = true },
> +	{ .name = "external-falling", .value = 0x2, .is_external = true },
> +	{ .name = "external-any", .value = 0x3, .is_external = true },
> +	{ .name = "continuous", .value = 0x6 },
>  };
>  
>  static struct at91_adc_caps at91sam9rl_caps = {
> @@ -1344,6 +1328,8 @@ static struct at91_adc_caps at91sam9rl_caps = {
>  		.mr_prescal_mask = AT91_ADC_PRESCAL_9260,
>  		.mr_startup_mask = AT91_ADC_STARTUP_9G45,
>  	},
> +	.triggers = at91sam9x5_triggers,
> +	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
>  };
>  
>  static struct at91_adc_caps at91sam9g45_caps = {
> @@ -1360,6 +1346,8 @@ static struct at91_adc_caps at91sam9g45_caps = {
>  		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
>  		.mr_startup_mask = AT91_ADC_STARTUP_9G45,
>  	},
> +	.triggers = at91sam9x5_triggers,
> +	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
>  };
>  
>  static struct at91_adc_caps at91sam9x5_caps = {
> @@ -1380,6 +1368,8 @@ static struct at91_adc_caps at91sam9x5_caps = {
>  		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
>  		.mr_startup_mask = AT91_ADC_STARTUP_9X5,
>  	},
> +	.triggers = at91sam9x5_triggers,
> +	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
>  };
>  
>  static struct at91_adc_caps sama5d3_caps = {
> @@ -1399,6 +1389,8 @@ static struct at91_adc_caps sama5d3_caps = {
>  		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
>  		.mr_startup_mask = AT91_ADC_STARTUP_9X5,
>  	},
> +	.triggers = at91sam9x5_triggers,
> +	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
>  };
>  
>  static const struct of_device_id at91_adc_dt_ids[] = {

Re: [PATCH 4/9] iio: adc: at91_adc: rework trigger definition

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:45 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> Move the available trigger definition back in the driver to stop cluttering
> the device tree. There is no functional change except that it actually
> fixes the available triggers for at91sam9rl as it inherited the list from
> at91sam9260 but actually has the triggers from at91sam9x5.

Is that a fix we might want to backport?  If not we should probably put a clear
statement in here to avoid it getting picked up by the bot.

I'd argue it's to invasive a change to backport.

Otherwise, sensible looking change.

> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> ---
>  .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 46 -----------
>  drivers/iio/adc/at91_adc.c                    | 80 +++++++++----------
>  2 files changed, 36 insertions(+), 90 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> index 9b0ff59e75de..e6a1f915b542 100644
> --- a/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
> @@ -97,29 +97,6 @@ required:
>    - atmel,adc-startup-time
>    - atmel,adc-vref
>  
> -patternProperties:
> -  "^(trigger)[0-9]$":
> -    type: object
> -    description: Child node to describe a trigger exposed to the user.
> -    properties:
> -      trigger-name:
> -        $ref: /schemas/types.yaml#/definitions/string
> -        description: Identifying name.
> -
> -      trigger-value:
> -        $ref: /schemas/types.yaml#/definitions/uint32
> -        description:
> -          Value to put in the Trigger register to activate this trigger
> -
> -      trigger-external:
> -        $ref: /schemas/types.yaml#/definitions/flag
> -        description: This trigger is provided from an external pin.
> -
> -    additionalProperties: false
> -    required:
> -      - trigger-name
> -      - trigger-value
> -
>  examples:
>    - |
>      #include <dt-bindings/dma/at91.h>
> @@ -139,29 +116,6 @@ examples:
>              atmel,adc-use-external-triggers;
>              atmel,adc-vref = <3300>;
>              atmel,adc-use-res = "lowres";
> -
> -            trigger0 {
> -                trigger-name = "external-rising";
> -                trigger-value = <0x1>;
> -                trigger-external;
> -            };
> -
> -            trigger1 {
> -                trigger-name = "external-falling";
> -                trigger-value = <0x2>;
> -                trigger-external;
> -            };
> -
> -            trigger2 {
> -                trigger-name = "external-any";
> -                trigger-value = <0x3>;
> -                trigger-external;
> -            };
> -
> -            trigger3 {
> -                trigger-name = "continuous";
> -                trigger-value = <0x6>;
> -            };
>          };
>      };
>  ...
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 0d67c812ef3d..83539422b704 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -207,6 +207,8 @@ struct at91_adc_caps {
>  
>  	u8	low_res_bits;
>  	u8	high_res_bits;
> +	u32	trigger_number;
> +	const struct at91_adc_trigger *triggers;
>  	struct at91_adc_reg_desc registers;
>  };
>  
> @@ -227,8 +229,6 @@ struct at91_adc_state {
>  	u8			sample_hold_time;
>  	bool			sleep_mode;
>  	struct iio_trigger	**trig;
> -	struct at91_adc_trigger	*trigger_list;
> -	u32			trigger_number;
>  	bool			use_external;
>  	u32			vref_mv;
>  	u32			res;		/* resolution used for convertions */
> @@ -537,13 +537,13 @@ static int at91_adc_channel_init(struct iio_dev *idev)
>  }
>  
>  static int at91_adc_get_trigger_value_by_name(struct iio_dev *idev,
> -					     struct at91_adc_trigger *triggers,
> +					     const struct at91_adc_trigger *triggers,
>  					     const char *trigger_name)
>  {
>  	struct at91_adc_state *st = iio_priv(idev);
>  	int i;
>  
> -	for (i = 0; i < st->trigger_number; i++) {
> +	for (i = 0; i < st->caps->trigger_number; i++) {
>  		char *name = kasprintf(GFP_KERNEL,
>  				"%s-dev%d-%s",
>  				idev->name,
> @@ -575,7 +575,7 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
>  	u8 bit;
>  
>  	value = at91_adc_get_trigger_value_by_name(idev,
> -						   st->trigger_list,
> +						   st->caps->triggers,
>  						   idev->trig->name);
>  	if (value < 0)
>  		return value;
> @@ -620,7 +620,7 @@ static const struct iio_trigger_ops at91_adc_trigger_ops = {
>  };
>  
>  static struct iio_trigger *at91_adc_allocate_trigger(struct iio_dev *idev,
> -						     struct at91_adc_trigger *trigger)
> +						     const struct at91_adc_trigger *trigger)
>  {
>  	struct iio_trigger *trig;
>  	int ret;
> @@ -647,7 +647,7 @@ static int at91_adc_trigger_init(struct iio_dev *idev)
>  	int i, ret;
>  
>  	st->trig = devm_kcalloc(&idev->dev,
> -				st->trigger_number, sizeof(*st->trig),
> +				st->caps->trigger_number, sizeof(*st->trig),
>  				GFP_KERNEL);
>  
>  	if (st->trig == NULL) {
> @@ -655,12 +655,12 @@ static int at91_adc_trigger_init(struct iio_dev *idev)
>  		goto error_ret;
>  	}
>  
> -	for (i = 0; i < st->trigger_number; i++) {
> -		if (st->trigger_list[i].is_external && !(st->use_external))
> +	for (i = 0; i < st->caps->trigger_number; i++) {
> +		if (st->caps->triggers[i].is_external && !(st->use_external))
>  			continue;
>  
>  		st->trig[i] = at91_adc_allocate_trigger(idev,
> -							st->trigger_list + i);
> +							st->caps->triggers + i);
>  		if (st->trig[i] == NULL) {
>  			dev_err(&idev->dev,
>  				"Could not allocate trigger %d\n", i);
> @@ -685,7 +685,7 @@ static void at91_adc_trigger_remove(struct iio_dev *idev)
>  	struct at91_adc_state *st = iio_priv(idev);
>  	int i;
>  
> -	for (i = 0; i < st->trigger_number; i++) {
> +	for (i = 0; i < st->caps->trigger_number; i++) {
>  		iio_trigger_unregister(st->trig[i]);
>  		iio_trigger_free(st->trig[i]);
>  	}
> @@ -838,8 +838,7 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
>  {
>  	struct at91_adc_state *st = iio_priv(idev);
>  	struct device_node *node = pdev->dev.of_node;
> -	struct device_node *trig_node;
> -	int i = 0, ret;
> +	int ret;
>  	u32 prop;
>  	char *s;
>  
> @@ -885,37 +884,6 @@ static int at91_adc_probe_dt(struct iio_dev *idev,
>  
>  	st->registers = &st->caps->registers;
>  	st->num_channels = st->caps->num_channels;
> -	st->trigger_number = of_get_child_count(node);
> -	st->trigger_list = devm_kcalloc(&idev->dev,
> -					st->trigger_number,
> -					sizeof(struct at91_adc_trigger),
> -					GFP_KERNEL);
> -	if (!st->trigger_list) {
> -		dev_err(&idev->dev, "Could not allocate trigger list memory.\n");
> -		ret = -ENOMEM;
> -		goto error_ret;
> -	}
> -
> -	for_each_child_of_node(node, trig_node) {
> -		struct at91_adc_trigger *trig = st->trigger_list + i;
> -		const char *name;
> -
> -		if (of_property_read_string(trig_node, "trigger-name", &name)) {
> -			dev_err(&idev->dev, "Missing trigger-name property in the DT.\n");
> -			ret = -EINVAL;
> -			goto error_ret;
> -		}
> -		trig->name = name;
> -
> -		if (of_property_read_u32(trig_node, "trigger-value", &prop)) {
> -			dev_err(&idev->dev, "Missing trigger-value property in the DT.\n");
> -			ret = -EINVAL;
> -			goto error_ret;
> -		}
> -		trig->value = prop;
> -		trig->is_external = of_property_read_bool(trig_node, "trigger-external");
> -		i++;
> -	}
>  
>  	/* Check if touchscreen is supported. */
>  	if (st->caps->has_ts)
> @@ -1315,6 +1283,13 @@ static int at91_adc_resume(struct device *dev)
>  
>  static SIMPLE_DEV_PM_OPS(at91_adc_pm_ops, at91_adc_suspend, at91_adc_resume);
>  
> +static const struct at91_adc_trigger at91sam9260_triggers[] = {
> +	{ .name = "timer-counter-0", .value = 0x1 },
> +	{ .name = "timer-counter-1", .value = 0x3 },
> +	{ .name = "timer-counter-2", .value = 0x5 },
> +	{ .name = "external", .value = 0xd, .is_external = true },
> +};
> +
>  static struct at91_adc_caps at91sam9260_caps = {
>  	.calc_startup_ticks = calc_startup_ticks_9260,
>  	.num_channels = 4,
> @@ -1328,6 +1303,15 @@ static struct at91_adc_caps at91sam9260_caps = {
>  		.mr_prescal_mask = AT91_ADC_PRESCAL_9260,
>  		.mr_startup_mask = AT91_ADC_STARTUP_9260,
>  	},
> +	.triggers = at91sam9260_triggers,
> +	.trigger_number = ARRAY_SIZE(at91sam9260_triggers),
> +};
> +
> +static const struct at91_adc_trigger at91sam9x5_triggers[] = {
> +	{ .name = "external-rising", .value = 0x1, .is_external = true },
> +	{ .name = "external-falling", .value = 0x2, .is_external = true },
> +	{ .name = "external-any", .value = 0x3, .is_external = true },
> +	{ .name = "continuous", .value = 0x6 },
>  };
>  
>  static struct at91_adc_caps at91sam9rl_caps = {
> @@ -1344,6 +1328,8 @@ static struct at91_adc_caps at91sam9rl_caps = {
>  		.mr_prescal_mask = AT91_ADC_PRESCAL_9260,
>  		.mr_startup_mask = AT91_ADC_STARTUP_9G45,
>  	},
> +	.triggers = at91sam9x5_triggers,
> +	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
>  };
>  
>  static struct at91_adc_caps at91sam9g45_caps = {
> @@ -1360,6 +1346,8 @@ static struct at91_adc_caps at91sam9g45_caps = {
>  		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
>  		.mr_startup_mask = AT91_ADC_STARTUP_9G45,
>  	},
> +	.triggers = at91sam9x5_triggers,
> +	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
>  };
>  
>  static struct at91_adc_caps at91sam9x5_caps = {
> @@ -1380,6 +1368,8 @@ static struct at91_adc_caps at91sam9x5_caps = {
>  		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
>  		.mr_startup_mask = AT91_ADC_STARTUP_9X5,
>  	},
> +	.triggers = at91sam9x5_triggers,
> +	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
>  };
>  
>  static struct at91_adc_caps sama5d3_caps = {
> @@ -1399,6 +1389,8 @@ static struct at91_adc_caps sama5d3_caps = {
>  		.mr_prescal_mask = AT91_ADC_PRESCAL_9G45,
>  		.mr_startup_mask = AT91_ADC_STARTUP_9X5,
>  	},
> +	.triggers = at91sam9x5_triggers,
> +	.trigger_number = ARRAY_SIZE(at91sam9x5_triggers),
>  };
>  
>  static const struct of_device_id at91_adc_dt_ids[] = {


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/9] iio: adc: at91_adc: rework trigger definition

[Alexandre Belloni]

On 14/11/2020 17:02:30+0000, Jonathan Cameron wrote:
> On Fri, 13 Nov 2020 22:26:45 +0100
> Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> 
> > Move the available trigger definition back in the driver to stop cluttering
> > the device tree. There is no functional change except that it actually
> > fixes the available triggers for at91sam9rl as it inherited the list from
> > at91sam9260 but actually has the triggers from at91sam9x5.
> 
> Is that a fix we might want to backport?  If not we should probably put a clear
> statement in here to avoid it getting picked up by the bot.
> 
> I'd argue it's to invasive a change to backport.
> 

Nobody ever complained about it so I don't think it is necessary to
backport. Anyway, the proper backportable fix would be to simply change
the device tree, that avoids the driver change which I also think is too
invasive. I'll include the DT change in v2.

Re: [PATCH 4/9] iio: adc: at91_adc: rework trigger definition

[Alexandre Belloni]

On 14/11/2020 17:02:30+0000, Jonathan Cameron wrote:
> On Fri, 13 Nov 2020 22:26:45 +0100
> Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> 
> > Move the available trigger definition back in the driver to stop cluttering
> > the device tree. There is no functional change except that it actually
> > fixes the available triggers for at91sam9rl as it inherited the list from
> > at91sam9260 but actually has the triggers from at91sam9x5.
> 
> Is that a fix we might want to backport?  If not we should probably put a clear
> statement in here to avoid it getting picked up by the bot.
> 
> I'd argue it's to invasive a change to backport.
> 

Nobody ever complained about it so I don't think it is necessary to
backport. Anyway, the proper backportable fix would be to simply change
the device tree, that avoids the driver change which I also think is too
invasive. I'll include the DT change in v2.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/9] iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe

[Alexandre Belloni]

at91_adc_probe_dt is now small enough to be merged back in at91_adc_probe.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/iio/adc/at91_adc.c | 118 +++++++++++++++----------------------
 1 file changed, 49 insertions(+), 69 deletions(-)

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 83539422b704..9f05eb722f5e 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -833,70 +833,6 @@ static int at91_adc_probe_dt_ts(struct device_node *node,
 	}
 }
 
-static int at91_adc_probe_dt(struct iio_dev *idev,
-			     struct platform_device *pdev)
-{
-	struct at91_adc_state *st = iio_priv(idev);
-	struct device_node *node = pdev->dev.of_node;
-	int ret;
-	u32 prop;
-	char *s;
-
-	st->caps = (struct at91_adc_caps *)
-		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
-
-	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
-
-	if (of_property_read_u32(node, "atmel,adc-channels-used", &prop)) {
-		dev_err(&idev->dev, "Missing adc-channels-used property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
-	}
-	st->channels_mask = prop;
-
-	st->sleep_mode = of_property_read_bool(node, "atmel,adc-sleep-mode");
-
-	if (of_property_read_u32(node, "atmel,adc-startup-time", &prop)) {
-		dev_err(&idev->dev, "Missing adc-startup-time property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
-	}
-	st->startup_time = prop;
-
-	prop = 0;
-	of_property_read_u32(node, "atmel,adc-sample-hold-time", &prop);
-	st->sample_hold_time = prop;
-
-	if (of_property_read_u32(node, "atmel,adc-vref", &prop)) {
-		dev_err(&idev->dev, "Missing adc-vref property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
-	}
-	st->vref_mv = prop;
-
-	st->res = st->caps->high_res_bits;
-	if (st->caps->low_res_bits &&
-	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
-	    && !strcmp(s, "lowres"))
-		st->res = st->caps->low_res_bits;
-
-	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
-
-	st->registers = &st->caps->registers;
-	st->num_channels = st->caps->num_channels;
-
-	/* Check if touchscreen is supported. */
-	if (st->caps->has_ts)
-		return at91_adc_probe_dt_ts(node, st, &idev->dev);
-	else
-		dev_info(&idev->dev, "not support touchscreen in the adc compatible string.\n");
-
-	return 0;
-
-error_ret:
-	return ret;
-}
-
 static const struct iio_info at91_adc_info = {
 	.read_raw = &at91_adc_read_raw,
 };
@@ -1062,10 +998,12 @@ static void at91_ts_unregister(struct at91_adc_state *st)
 static int at91_adc_probe(struct platform_device *pdev)
 {
 	unsigned int prsc, mstrclk, ticks, adc_clk, adc_clk_khz, shtim;
+	struct device_node *node = pdev->dev.of_node;
 	int ret;
 	struct iio_dev *idev;
 	struct at91_adc_state *st;
-	u32 reg;
+	u32 reg, prop;
+	char *s;
 
 	idev = devm_iio_device_alloc(&pdev->dev, sizeof(struct at91_adc_state));
 	if (!idev)
@@ -1073,9 +1011,52 @@ static int at91_adc_probe(struct platform_device *pdev)
 
 	st = iio_priv(idev);
 
-	ret = at91_adc_probe_dt(idev, pdev);
-	if (ret)
-		return ret;
+	st->caps = (struct at91_adc_caps *)
+		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
+
+	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
+
+	if (of_property_read_u32(node, "atmel,adc-channels-used", &prop)) {
+		dev_err(&idev->dev, "Missing adc-channels-used property in the DT.\n");
+		return -EINVAL;
+	}
+	st->channels_mask = prop;
+
+	st->sleep_mode = of_property_read_bool(node, "atmel,adc-sleep-mode");
+
+	if (of_property_read_u32(node, "atmel,adc-startup-time", &prop)) {
+		dev_err(&idev->dev, "Missing adc-startup-time property in the DT.\n");
+		return -EINVAL;
+	}
+	st->startup_time = prop;
+
+	prop = 0;
+	of_property_read_u32(node, "atmel,adc-sample-hold-time", &prop);
+	st->sample_hold_time = prop;
+
+	if (of_property_read_u32(node, "atmel,adc-vref", &prop)) {
+		dev_err(&idev->dev, "Missing adc-vref property in the DT.\n");
+		return -EINVAL;
+	}
+	st->vref_mv = prop;
+
+	st->res = st->caps->high_res_bits;
+	if (st->caps->low_res_bits &&
+	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
+	    && !strcmp(s, "lowres"))
+		st->res = st->caps->low_res_bits;
+
+	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
+
+	st->registers = &st->caps->registers;
+	st->num_channels = st->caps->num_channels;
+
+	/* Check if touchscreen is supported. */
+	if (st->caps->has_ts) {
+		ret = at91_adc_probe_dt_ts(node, st, &idev->dev);
+		if (ret)
+			return ret;
+	}
 
 	platform_set_drvdata(pdev, idev);
 
@@ -1091,7 +1072,6 @@ static int at91_adc_probe(struct platform_device *pdev)
 	if (IS_ERR(st->reg_base))
 		return PTR_ERR(st->reg_base);
 
-
 	/*
 	 * Disable all IRQs before setting up the handler
 	 */
-- 
2.28.0

[PATCH 5/9] iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe

[Alexandre Belloni]

at91_adc_probe_dt is now small enough to be merged back in at91_adc_probe.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/iio/adc/at91_adc.c | 118 +++++++++++++++----------------------
 1 file changed, 49 insertions(+), 69 deletions(-)

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 83539422b704..9f05eb722f5e 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -833,70 +833,6 @@ static int at91_adc_probe_dt_ts(struct device_node *node,
 	}
 }
 
-static int at91_adc_probe_dt(struct iio_dev *idev,
-			     struct platform_device *pdev)
-{
-	struct at91_adc_state *st = iio_priv(idev);
-	struct device_node *node = pdev->dev.of_node;
-	int ret;
-	u32 prop;
-	char *s;
-
-	st->caps = (struct at91_adc_caps *)
-		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
-
-	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
-
-	if (of_property_read_u32(node, "atmel,adc-channels-used", &prop)) {
-		dev_err(&idev->dev, "Missing adc-channels-used property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
-	}
-	st->channels_mask = prop;
-
-	st->sleep_mode = of_property_read_bool(node, "atmel,adc-sleep-mode");
-
-	if (of_property_read_u32(node, "atmel,adc-startup-time", &prop)) {
-		dev_err(&idev->dev, "Missing adc-startup-time property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
-	}
-	st->startup_time = prop;
-
-	prop = 0;
-	of_property_read_u32(node, "atmel,adc-sample-hold-time", &prop);
-	st->sample_hold_time = prop;
-
-	if (of_property_read_u32(node, "atmel,adc-vref", &prop)) {
-		dev_err(&idev->dev, "Missing adc-vref property in the DT.\n");
-		ret = -EINVAL;
-		goto error_ret;
-	}
-	st->vref_mv = prop;
-
-	st->res = st->caps->high_res_bits;
-	if (st->caps->low_res_bits &&
-	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
-	    && !strcmp(s, "lowres"))
-		st->res = st->caps->low_res_bits;
-
-	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
-
-	st->registers = &st->caps->registers;
-	st->num_channels = st->caps->num_channels;
-
-	/* Check if touchscreen is supported. */
-	if (st->caps->has_ts)
-		return at91_adc_probe_dt_ts(node, st, &idev->dev);
-	else
-		dev_info(&idev->dev, "not support touchscreen in the adc compatible string.\n");
-
-	return 0;
-
-error_ret:
-	return ret;
-}
-
 static const struct iio_info at91_adc_info = {
 	.read_raw = &at91_adc_read_raw,
 };
@@ -1062,10 +998,12 @@ static void at91_ts_unregister(struct at91_adc_state *st)
 static int at91_adc_probe(struct platform_device *pdev)
 {
 	unsigned int prsc, mstrclk, ticks, adc_clk, adc_clk_khz, shtim;
+	struct device_node *node = pdev->dev.of_node;
 	int ret;
 	struct iio_dev *idev;
 	struct at91_adc_state *st;
-	u32 reg;
+	u32 reg, prop;
+	char *s;
 
 	idev = devm_iio_device_alloc(&pdev->dev, sizeof(struct at91_adc_state));
 	if (!idev)
@@ -1073,9 +1011,52 @@ static int at91_adc_probe(struct platform_device *pdev)
 
 	st = iio_priv(idev);
 
-	ret = at91_adc_probe_dt(idev, pdev);
-	if (ret)
-		return ret;
+	st->caps = (struct at91_adc_caps *)
+		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
+
+	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
+
+	if (of_property_read_u32(node, "atmel,adc-channels-used", &prop)) {
+		dev_err(&idev->dev, "Missing adc-channels-used property in the DT.\n");
+		return -EINVAL;
+	}
+	st->channels_mask = prop;
+
+	st->sleep_mode = of_property_read_bool(node, "atmel,adc-sleep-mode");
+
+	if (of_property_read_u32(node, "atmel,adc-startup-time", &prop)) {
+		dev_err(&idev->dev, "Missing adc-startup-time property in the DT.\n");
+		return -EINVAL;
+	}
+	st->startup_time = prop;
+
+	prop = 0;
+	of_property_read_u32(node, "atmel,adc-sample-hold-time", &prop);
+	st->sample_hold_time = prop;
+
+	if (of_property_read_u32(node, "atmel,adc-vref", &prop)) {
+		dev_err(&idev->dev, "Missing adc-vref property in the DT.\n");
+		return -EINVAL;
+	}
+	st->vref_mv = prop;
+
+	st->res = st->caps->high_res_bits;
+	if (st->caps->low_res_bits &&
+	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
+	    && !strcmp(s, "lowres"))
+		st->res = st->caps->low_res_bits;
+
+	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
+
+	st->registers = &st->caps->registers;
+	st->num_channels = st->caps->num_channels;
+
+	/* Check if touchscreen is supported. */
+	if (st->caps->has_ts) {
+		ret = at91_adc_probe_dt_ts(node, st, &idev->dev);
+		if (ret)
+			return ret;
+	}
 
 	platform_set_drvdata(pdev, idev);
 
@@ -1091,7 +1072,6 @@ static int at91_adc_probe(struct platform_device *pdev)
 	if (IS_ERR(st->reg_base))
 		return PTR_ERR(st->reg_base);
 
-
 	/*
 	 * Disable all IRQs before setting up the handler
 	 */
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/9] iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:46 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> at91_adc_probe_dt is now small enough to be merged back in at91_adc_probe.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Minor largely unrelated suggestion inline as we are touching this code.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/at91_adc.c | 118 +++++++++++++++----------------------
>  1 file changed, 49 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 83539422b704..9f05eb722f5e 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -833,70 +833,6 @@ static int at91_adc_probe_dt_ts(struct device_node *node,
>  	}
>  }
>  
> -static int at91_adc_probe_dt(struct iio_dev *idev,
> -			     struct platform_device *pdev)
> -{
> -	struct at91_adc_state *st = iio_priv(idev);
> -	struct device_node *node = pdev->dev.of_node;
> -	int ret;
> -	u32 prop;
> -	char *s;
> -
> -	st->caps = (struct at91_adc_caps *)
> -		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
> -
> -	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
> -
> -	if (of_property_read_u32(node, "atmel,adc-channels-used", &prop)) {
> -		dev_err(&idev->dev, "Missing adc-channels-used property in the DT.\n");
> -		ret = -EINVAL;
> -		goto error_ret;
> -	}
> -	st->channels_mask = prop;
> -
> -	st->sleep_mode = of_property_read_bool(node, "atmel,adc-sleep-mode");
> -
> -	if (of_property_read_u32(node, "atmel,adc-startup-time", &prop)) {
> -		dev_err(&idev->dev, "Missing adc-startup-time property in the DT.\n");
> -		ret = -EINVAL;
> -		goto error_ret;
> -	}
> -	st->startup_time = prop;
> -
> -	prop = 0;
> -	of_property_read_u32(node, "atmel,adc-sample-hold-time", &prop);
> -	st->sample_hold_time = prop;
> -
> -	if (of_property_read_u32(node, "atmel,adc-vref", &prop)) {
> -		dev_err(&idev->dev, "Missing adc-vref property in the DT.\n");
> -		ret = -EINVAL;
> -		goto error_ret;
> -	}
> -	st->vref_mv = prop;
> -
> -	st->res = st->caps->high_res_bits;
> -	if (st->caps->low_res_bits &&
> -	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
> -	    && !strcmp(s, "lowres"))
> -		st->res = st->caps->low_res_bits;
> -
> -	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
> -
> -	st->registers = &st->caps->registers;
> -	st->num_channels = st->caps->num_channels;
> -
> -	/* Check if touchscreen is supported. */
> -	if (st->caps->has_ts)
> -		return at91_adc_probe_dt_ts(node, st, &idev->dev);
> -	else
> -		dev_info(&idev->dev, "not support touchscreen in the adc compatible string.\n");
> -
> -	return 0;
> -
> -error_ret:
> -	return ret;
> -}
> -
>  static const struct iio_info at91_adc_info = {
>  	.read_raw = &at91_adc_read_raw,
>  };
> @@ -1062,10 +998,12 @@ static void at91_ts_unregister(struct at91_adc_state *st)
>  static int at91_adc_probe(struct platform_device *pdev)
>  {
>  	unsigned int prsc, mstrclk, ticks, adc_clk, adc_clk_khz, shtim;
> +	struct device_node *node = pdev->dev.of_node;
>  	int ret;
>  	struct iio_dev *idev;
>  	struct at91_adc_state *st;
> -	u32 reg;
> +	u32 reg, prop;
> +	char *s;
>  
>  	idev = devm_iio_device_alloc(&pdev->dev, sizeof(struct at91_adc_state));
>  	if (!idev)
> @@ -1073,9 +1011,52 @@ static int at91_adc_probe(struct platform_device *pdev)
>  
>  	st = iio_priv(idev);
>  
> -	ret = at91_adc_probe_dt(idev, pdev);
> -	if (ret)
> -		return ret;
> +	st->caps = (struct at91_adc_caps *)
> +		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;

of_device_get match_data  - obviously an unrelated change but trivial enough
I'd just slip it in this patch (unless you have it a later one!)

That returns a void * so no need for the cast.

> +
> +	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
> +
> +	if (of_property_read_u32(node, "atmel,adc-channels-used", &prop)) {
> +		dev_err(&idev->dev, "Missing adc-channels-used property in the DT.\n");
> +		return -EINVAL;
> +	}
> +	st->channels_mask = prop;
> +
> +	st->sleep_mode = of_property_read_bool(node, "atmel,adc-sleep-mode");
> +
> +	if (of_property_read_u32(node, "atmel,adc-startup-time", &prop)) {
> +		dev_err(&idev->dev, "Missing adc-startup-time property in the DT.\n");
> +		return -EINVAL;
> +	}
> +	st->startup_time = prop;
> +
> +	prop = 0;
> +	of_property_read_u32(node, "atmel,adc-sample-hold-time", &prop);
> +	st->sample_hold_time = prop;
> +
> +	if (of_property_read_u32(node, "atmel,adc-vref", &prop)) {
> +		dev_err(&idev->dev, "Missing adc-vref property in the DT.\n");
> +		return -EINVAL;
> +	}
> +	st->vref_mv = prop;
> +
> +	st->res = st->caps->high_res_bits;
> +	if (st->caps->low_res_bits &&
> +	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
> +	    && !strcmp(s, "lowres"))
> +		st->res = st->caps->low_res_bits;
> +
> +	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
> +
> +	st->registers = &st->caps->registers;
> +	st->num_channels = st->caps->num_channels;
> +
> +	/* Check if touchscreen is supported. */
> +	if (st->caps->has_ts) {
> +		ret = at91_adc_probe_dt_ts(node, st, &idev->dev);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	platform_set_drvdata(pdev, idev);
>  
> @@ -1091,7 +1072,6 @@ static int at91_adc_probe(struct platform_device *pdev)
>  	if (IS_ERR(st->reg_base))
>  		return PTR_ERR(st->reg_base);
>  
> -
Stray change that shouldn't be in this patch ideally but not that important.

>  	/*
>  	 * Disable all IRQs before setting up the handler
>  	 */

Re: [PATCH 5/9] iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:46 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> at91_adc_probe_dt is now small enough to be merged back in at91_adc_probe.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Minor largely unrelated suggestion inline as we are touching this code.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/at91_adc.c | 118 +++++++++++++++----------------------
>  1 file changed, 49 insertions(+), 69 deletions(-)
> 
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 83539422b704..9f05eb722f5e 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -833,70 +833,6 @@ static int at91_adc_probe_dt_ts(struct device_node *node,
>  	}
>  }
>  
> -static int at91_adc_probe_dt(struct iio_dev *idev,
> -			     struct platform_device *pdev)
> -{
> -	struct at91_adc_state *st = iio_priv(idev);
> -	struct device_node *node = pdev->dev.of_node;
> -	int ret;
> -	u32 prop;
> -	char *s;
> -
> -	st->caps = (struct at91_adc_caps *)
> -		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
> -
> -	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
> -
> -	if (of_property_read_u32(node, "atmel,adc-channels-used", &prop)) {
> -		dev_err(&idev->dev, "Missing adc-channels-used property in the DT.\n");
> -		ret = -EINVAL;
> -		goto error_ret;
> -	}
> -	st->channels_mask = prop;
> -
> -	st->sleep_mode = of_property_read_bool(node, "atmel,adc-sleep-mode");
> -
> -	if (of_property_read_u32(node, "atmel,adc-startup-time", &prop)) {
> -		dev_err(&idev->dev, "Missing adc-startup-time property in the DT.\n");
> -		ret = -EINVAL;
> -		goto error_ret;
> -	}
> -	st->startup_time = prop;
> -
> -	prop = 0;
> -	of_property_read_u32(node, "atmel,adc-sample-hold-time", &prop);
> -	st->sample_hold_time = prop;
> -
> -	if (of_property_read_u32(node, "atmel,adc-vref", &prop)) {
> -		dev_err(&idev->dev, "Missing adc-vref property in the DT.\n");
> -		ret = -EINVAL;
> -		goto error_ret;
> -	}
> -	st->vref_mv = prop;
> -
> -	st->res = st->caps->high_res_bits;
> -	if (st->caps->low_res_bits &&
> -	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
> -	    && !strcmp(s, "lowres"))
> -		st->res = st->caps->low_res_bits;
> -
> -	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
> -
> -	st->registers = &st->caps->registers;
> -	st->num_channels = st->caps->num_channels;
> -
> -	/* Check if touchscreen is supported. */
> -	if (st->caps->has_ts)
> -		return at91_adc_probe_dt_ts(node, st, &idev->dev);
> -	else
> -		dev_info(&idev->dev, "not support touchscreen in the adc compatible string.\n");
> -
> -	return 0;
> -
> -error_ret:
> -	return ret;
> -}
> -
>  static const struct iio_info at91_adc_info = {
>  	.read_raw = &at91_adc_read_raw,
>  };
> @@ -1062,10 +998,12 @@ static void at91_ts_unregister(struct at91_adc_state *st)
>  static int at91_adc_probe(struct platform_device *pdev)
>  {
>  	unsigned int prsc, mstrclk, ticks, adc_clk, adc_clk_khz, shtim;
> +	struct device_node *node = pdev->dev.of_node;
>  	int ret;
>  	struct iio_dev *idev;
>  	struct at91_adc_state *st;
> -	u32 reg;
> +	u32 reg, prop;
> +	char *s;
>  
>  	idev = devm_iio_device_alloc(&pdev->dev, sizeof(struct at91_adc_state));
>  	if (!idev)
> @@ -1073,9 +1011,52 @@ static int at91_adc_probe(struct platform_device *pdev)
>  
>  	st = iio_priv(idev);
>  
> -	ret = at91_adc_probe_dt(idev, pdev);
> -	if (ret)
> -		return ret;
> +	st->caps = (struct at91_adc_caps *)
> +		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;

of_device_get match_data  - obviously an unrelated change but trivial enough
I'd just slip it in this patch (unless you have it a later one!)

That returns a void * so no need for the cast.

> +
> +	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
> +
> +	if (of_property_read_u32(node, "atmel,adc-channels-used", &prop)) {
> +		dev_err(&idev->dev, "Missing adc-channels-used property in the DT.\n");
> +		return -EINVAL;
> +	}
> +	st->channels_mask = prop;
> +
> +	st->sleep_mode = of_property_read_bool(node, "atmel,adc-sleep-mode");
> +
> +	if (of_property_read_u32(node, "atmel,adc-startup-time", &prop)) {
> +		dev_err(&idev->dev, "Missing adc-startup-time property in the DT.\n");
> +		return -EINVAL;
> +	}
> +	st->startup_time = prop;
> +
> +	prop = 0;
> +	of_property_read_u32(node, "atmel,adc-sample-hold-time", &prop);
> +	st->sample_hold_time = prop;
> +
> +	if (of_property_read_u32(node, "atmel,adc-vref", &prop)) {
> +		dev_err(&idev->dev, "Missing adc-vref property in the DT.\n");
> +		return -EINVAL;
> +	}
> +	st->vref_mv = prop;
> +
> +	st->res = st->caps->high_res_bits;
> +	if (st->caps->low_res_bits &&
> +	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
> +	    && !strcmp(s, "lowres"))
> +		st->res = st->caps->low_res_bits;
> +
> +	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
> +
> +	st->registers = &st->caps->registers;
> +	st->num_channels = st->caps->num_channels;
> +
> +	/* Check if touchscreen is supported. */
> +	if (st->caps->has_ts) {
> +		ret = at91_adc_probe_dt_ts(node, st, &idev->dev);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	platform_set_drvdata(pdev, idev);
>  
> @@ -1091,7 +1072,6 @@ static int at91_adc_probe(struct platform_device *pdev)
>  	if (IS_ERR(st->reg_base))
>  		return PTR_ERR(st->reg_base);
>  
> -
Stray change that shouldn't be in this patch ideally but not that important.

>  	/*
>  	 * Disable all IRQs before setting up the handler
>  	 */


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/9] iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe

[Jonathan Cameron]

On Sat, 14 Nov 2020 17:08:04 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 13 Nov 2020 22:26:46 +0100
> Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> 
> > at91_adc_probe_dt is now small enough to be merged back in at91_adc_probe.
> > 
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>  
> Minor largely unrelated suggestion inline as we are touching this code.
> 
Ignore that, you do it in the next patch which is better anyway :)

Jonathan

> Thanks,
> 
> Jonathan
> 
> > ---
> >  drivers/iio/adc/at91_adc.c | 118 +++++++++++++++----------------------
> >  1 file changed, 49 insertions(+), 69 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> > index 83539422b704..9f05eb722f5e 100644
> > --- a/drivers/iio/adc/at91_adc.c
> > +++ b/drivers/iio/adc/at91_adc.c
> > @@ -833,70 +833,6 @@ static int at91_adc_probe_dt_ts(struct device_node *node,
> >  	}
> >  }
> >  
> > -static int at91_adc_probe_dt(struct iio_dev *idev,
> > -			     struct platform_device *pdev)
> > -{
> > -	struct at91_adc_state *st = iio_priv(idev);
> > -	struct device_node *node = pdev->dev.of_node;
> > -	int ret;
> > -	u32 prop;
> > -	char *s;
> > -
> > -	st->caps = (struct at91_adc_caps *)
> > -		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
> > -
> > -	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
> > -
> > -	if (of_property_read_u32(node, "atmel,adc-channels-used", &prop)) {
> > -		dev_err(&idev->dev, "Missing adc-channels-used property in the DT.\n");
> > -		ret = -EINVAL;
> > -		goto error_ret;
> > -	}
> > -	st->channels_mask = prop;
> > -
> > -	st->sleep_mode = of_property_read_bool(node, "atmel,adc-sleep-mode");
> > -
> > -	if (of_property_read_u32(node, "atmel,adc-startup-time", &prop)) {
> > -		dev_err(&idev->dev, "Missing adc-startup-time property in the DT.\n");
> > -		ret = -EINVAL;
> > -		goto error_ret;
> > -	}
> > -	st->startup_time = prop;
> > -
> > -	prop = 0;
> > -	of_property_read_u32(node, "atmel,adc-sample-hold-time", &prop);
> > -	st->sample_hold_time = prop;
> > -
> > -	if (of_property_read_u32(node, "atmel,adc-vref", &prop)) {
> > -		dev_err(&idev->dev, "Missing adc-vref property in the DT.\n");
> > -		ret = -EINVAL;
> > -		goto error_ret;
> > -	}
> > -	st->vref_mv = prop;
> > -
> > -	st->res = st->caps->high_res_bits;
> > -	if (st->caps->low_res_bits &&
> > -	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
> > -	    && !strcmp(s, "lowres"))
> > -		st->res = st->caps->low_res_bits;
> > -
> > -	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
> > -
> > -	st->registers = &st->caps->registers;
> > -	st->num_channels = st->caps->num_channels;
> > -
> > -	/* Check if touchscreen is supported. */
> > -	if (st->caps->has_ts)
> > -		return at91_adc_probe_dt_ts(node, st, &idev->dev);
> > -	else
> > -		dev_info(&idev->dev, "not support touchscreen in the adc compatible string.\n");
> > -
> > -	return 0;
> > -
> > -error_ret:
> > -	return ret;
> > -}
> > -
> >  static const struct iio_info at91_adc_info = {
> >  	.read_raw = &at91_adc_read_raw,
> >  };
> > @@ -1062,10 +998,12 @@ static void at91_ts_unregister(struct at91_adc_state *st)
> >  static int at91_adc_probe(struct platform_device *pdev)
> >  {
> >  	unsigned int prsc, mstrclk, ticks, adc_clk, adc_clk_khz, shtim;
> > +	struct device_node *node = pdev->dev.of_node;
> >  	int ret;
> >  	struct iio_dev *idev;
> >  	struct at91_adc_state *st;
> > -	u32 reg;
> > +	u32 reg, prop;
> > +	char *s;
> >  
> >  	idev = devm_iio_device_alloc(&pdev->dev, sizeof(struct at91_adc_state));
> >  	if (!idev)
> > @@ -1073,9 +1011,52 @@ static int at91_adc_probe(struct platform_device *pdev)
> >  
> >  	st = iio_priv(idev);
> >  
> > -	ret = at91_adc_probe_dt(idev, pdev);
> > -	if (ret)
> > -		return ret;
> > +	st->caps = (struct at91_adc_caps *)
> > +		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;  
> 
> of_device_get match_data  - obviously an unrelated change but trivial enough
> I'd just slip it in this patch (unless you have it a later one!)
> 
> That returns a void * so no need for the cast.
> 
> > +
> > +	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
> > +
> > +	if (of_property_read_u32(node, "atmel,adc-channels-used", &prop)) {
> > +		dev_err(&idev->dev, "Missing adc-channels-used property in the DT.\n");
> > +		return -EINVAL;
> > +	}
> > +	st->channels_mask = prop;
> > +
> > +	st->sleep_mode = of_property_read_bool(node, "atmel,adc-sleep-mode");
> > +
> > +	if (of_property_read_u32(node, "atmel,adc-startup-time", &prop)) {
> > +		dev_err(&idev->dev, "Missing adc-startup-time property in the DT.\n");
> > +		return -EINVAL;
> > +	}
> > +	st->startup_time = prop;
> > +
> > +	prop = 0;
> > +	of_property_read_u32(node, "atmel,adc-sample-hold-time", &prop);
> > +	st->sample_hold_time = prop;
> > +
> > +	if (of_property_read_u32(node, "atmel,adc-vref", &prop)) {
> > +		dev_err(&idev->dev, "Missing adc-vref property in the DT.\n");
> > +		return -EINVAL;
> > +	}
> > +	st->vref_mv = prop;
> > +
> > +	st->res = st->caps->high_res_bits;
> > +	if (st->caps->low_res_bits &&
> > +	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
> > +	    && !strcmp(s, "lowres"))
> > +		st->res = st->caps->low_res_bits;
> > +
> > +	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
> > +
> > +	st->registers = &st->caps->registers;
> > +	st->num_channels = st->caps->num_channels;
> > +
> > +	/* Check if touchscreen is supported. */
> > +	if (st->caps->has_ts) {
> > +		ret = at91_adc_probe_dt_ts(node, st, &idev->dev);
> > +		if (ret)
> > +			return ret;
> > +	}
> >  
> >  	platform_set_drvdata(pdev, idev);
> >  
> > @@ -1091,7 +1072,6 @@ static int at91_adc_probe(struct platform_device *pdev)
> >  	if (IS_ERR(st->reg_base))
> >  		return PTR_ERR(st->reg_base);
> >  
> > -  
> Stray change that shouldn't be in this patch ideally but not that important.
> 
> >  	/*
> >  	 * Disable all IRQs before setting up the handler
> >  	 */  
> 

Re: [PATCH 5/9] iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe

[Jonathan Cameron]

On Sat, 14 Nov 2020 17:08:04 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 13 Nov 2020 22:26:46 +0100
> Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> 
> > at91_adc_probe_dt is now small enough to be merged back in at91_adc_probe.
> > 
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>  
> Minor largely unrelated suggestion inline as we are touching this code.
> 
Ignore that, you do it in the next patch which is better anyway :)

Jonathan

> Thanks,
> 
> Jonathan
> 
> > ---
> >  drivers/iio/adc/at91_adc.c | 118 +++++++++++++++----------------------
> >  1 file changed, 49 insertions(+), 69 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> > index 83539422b704..9f05eb722f5e 100644
> > --- a/drivers/iio/adc/at91_adc.c
> > +++ b/drivers/iio/adc/at91_adc.c
> > @@ -833,70 +833,6 @@ static int at91_adc_probe_dt_ts(struct device_node *node,
> >  	}
> >  }
> >  
> > -static int at91_adc_probe_dt(struct iio_dev *idev,
> > -			     struct platform_device *pdev)
> > -{
> > -	struct at91_adc_state *st = iio_priv(idev);
> > -	struct device_node *node = pdev->dev.of_node;
> > -	int ret;
> > -	u32 prop;
> > -	char *s;
> > -
> > -	st->caps = (struct at91_adc_caps *)
> > -		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
> > -
> > -	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
> > -
> > -	if (of_property_read_u32(node, "atmel,adc-channels-used", &prop)) {
> > -		dev_err(&idev->dev, "Missing adc-channels-used property in the DT.\n");
> > -		ret = -EINVAL;
> > -		goto error_ret;
> > -	}
> > -	st->channels_mask = prop;
> > -
> > -	st->sleep_mode = of_property_read_bool(node, "atmel,adc-sleep-mode");
> > -
> > -	if (of_property_read_u32(node, "atmel,adc-startup-time", &prop)) {
> > -		dev_err(&idev->dev, "Missing adc-startup-time property in the DT.\n");
> > -		ret = -EINVAL;
> > -		goto error_ret;
> > -	}
> > -	st->startup_time = prop;
> > -
> > -	prop = 0;
> > -	of_property_read_u32(node, "atmel,adc-sample-hold-time", &prop);
> > -	st->sample_hold_time = prop;
> > -
> > -	if (of_property_read_u32(node, "atmel,adc-vref", &prop)) {
> > -		dev_err(&idev->dev, "Missing adc-vref property in the DT.\n");
> > -		ret = -EINVAL;
> > -		goto error_ret;
> > -	}
> > -	st->vref_mv = prop;
> > -
> > -	st->res = st->caps->high_res_bits;
> > -	if (st->caps->low_res_bits &&
> > -	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
> > -	    && !strcmp(s, "lowres"))
> > -		st->res = st->caps->low_res_bits;
> > -
> > -	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
> > -
> > -	st->registers = &st->caps->registers;
> > -	st->num_channels = st->caps->num_channels;
> > -
> > -	/* Check if touchscreen is supported. */
> > -	if (st->caps->has_ts)
> > -		return at91_adc_probe_dt_ts(node, st, &idev->dev);
> > -	else
> > -		dev_info(&idev->dev, "not support touchscreen in the adc compatible string.\n");
> > -
> > -	return 0;
> > -
> > -error_ret:
> > -	return ret;
> > -}
> > -
> >  static const struct iio_info at91_adc_info = {
> >  	.read_raw = &at91_adc_read_raw,
> >  };
> > @@ -1062,10 +998,12 @@ static void at91_ts_unregister(struct at91_adc_state *st)
> >  static int at91_adc_probe(struct platform_device *pdev)
> >  {
> >  	unsigned int prsc, mstrclk, ticks, adc_clk, adc_clk_khz, shtim;
> > +	struct device_node *node = pdev->dev.of_node;
> >  	int ret;
> >  	struct iio_dev *idev;
> >  	struct at91_adc_state *st;
> > -	u32 reg;
> > +	u32 reg, prop;
> > +	char *s;
> >  
> >  	idev = devm_iio_device_alloc(&pdev->dev, sizeof(struct at91_adc_state));
> >  	if (!idev)
> > @@ -1073,9 +1011,52 @@ static int at91_adc_probe(struct platform_device *pdev)
> >  
> >  	st = iio_priv(idev);
> >  
> > -	ret = at91_adc_probe_dt(idev, pdev);
> > -	if (ret)
> > -		return ret;
> > +	st->caps = (struct at91_adc_caps *)
> > +		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;  
> 
> of_device_get match_data  - obviously an unrelated change but trivial enough
> I'd just slip it in this patch (unless you have it a later one!)
> 
> That returns a void * so no need for the cast.
> 
> > +
> > +	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
> > +
> > +	if (of_property_read_u32(node, "atmel,adc-channels-used", &prop)) {
> > +		dev_err(&idev->dev, "Missing adc-channels-used property in the DT.\n");
> > +		return -EINVAL;
> > +	}
> > +	st->channels_mask = prop;
> > +
> > +	st->sleep_mode = of_property_read_bool(node, "atmel,adc-sleep-mode");
> > +
> > +	if (of_property_read_u32(node, "atmel,adc-startup-time", &prop)) {
> > +		dev_err(&idev->dev, "Missing adc-startup-time property in the DT.\n");
> > +		return -EINVAL;
> > +	}
> > +	st->startup_time = prop;
> > +
> > +	prop = 0;
> > +	of_property_read_u32(node, "atmel,adc-sample-hold-time", &prop);
> > +	st->sample_hold_time = prop;
> > +
> > +	if (of_property_read_u32(node, "atmel,adc-vref", &prop)) {
> > +		dev_err(&idev->dev, "Missing adc-vref property in the DT.\n");
> > +		return -EINVAL;
> > +	}
> > +	st->vref_mv = prop;
> > +
> > +	st->res = st->caps->high_res_bits;
> > +	if (st->caps->low_res_bits &&
> > +	    !of_property_read_string(node, "atmel,adc-use-res", (const char **)&s)
> > +	    && !strcmp(s, "lowres"))
> > +		st->res = st->caps->low_res_bits;
> > +
> > +	dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
> > +
> > +	st->registers = &st->caps->registers;
> > +	st->num_channels = st->caps->num_channels;
> > +
> > +	/* Check if touchscreen is supported. */
> > +	if (st->caps->has_ts) {
> > +		ret = at91_adc_probe_dt_ts(node, st, &idev->dev);
> > +		if (ret)
> > +			return ret;
> > +	}
> >  
> >  	platform_set_drvdata(pdev, idev);
> >  
> > @@ -1091,7 +1072,6 @@ static int at91_adc_probe(struct platform_device *pdev)
> >  	if (IS_ERR(st->reg_base))
> >  		return PTR_ERR(st->reg_base);
> >  
> > -  
> Stray change that shouldn't be in this patch ideally but not that important.
> 
> >  	/*
> >  	 * Disable all IRQs before setting up the handler
> >  	 */  
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/9] iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe

[Alexandre Belloni]

On 14/11/2020 17:08:04+0000, Jonathan Cameron wrote:
> > -	ret = at91_adc_probe_dt(idev, pdev);
> > -	if (ret)
> > -		return ret;
> > +	st->caps = (struct at91_adc_caps *)
> > +		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
> 
> of_device_get match_data  - obviously an unrelated change but trivial enough
> I'd just slip it in this patch (unless you have it a later one!)
> 
> That returns a void * so no need for the cast.
> 

I guess I will change the next patch to use of_device_get_match_data as
Andy doesn't like it when the fo_ and device_ APIs are mixed.

> > +	st->registers = &st->caps->registers;
> > +	st->num_channels = st->caps->num_channels;
> > +
> > +	/* Check if touchscreen is supported. */
> > +	if (st->caps->has_ts) {
> > +		ret = at91_adc_probe_dt_ts(node, st, &idev->dev);
> > +		if (ret)
> > +			return ret;
> > +	}
> >  
> >  	platform_set_drvdata(pdev, idev);
> >  
> > @@ -1091,7 +1072,6 @@ static int at91_adc_probe(struct platform_device *pdev)
> >  	if (IS_ERR(st->reg_base))
> >  		return PTR_ERR(st->reg_base);
> >  
> > -
> Stray change that shouldn't be in this patch ideally but not that important.
> 

I know I sneaked this one in but I didn't want to have that as a separate
patch ;). I'll drop it.

Re: [PATCH 5/9] iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe

[Alexandre Belloni]

On 14/11/2020 17:08:04+0000, Jonathan Cameron wrote:
> > -	ret = at91_adc_probe_dt(idev, pdev);
> > -	if (ret)
> > -		return ret;
> > +	st->caps = (struct at91_adc_caps *)
> > +		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
> 
> of_device_get match_data  - obviously an unrelated change but trivial enough
> I'd just slip it in this patch (unless you have it a later one!)
> 
> That returns a void * so no need for the cast.
> 

I guess I will change the next patch to use of_device_get_match_data as
Andy doesn't like it when the fo_ and device_ APIs are mixed.

> > +	st->registers = &st->caps->registers;
> > +	st->num_channels = st->caps->num_channels;
> > +
> > +	/* Check if touchscreen is supported. */
> > +	if (st->caps->has_ts) {
> > +		ret = at91_adc_probe_dt_ts(node, st, &idev->dev);
> > +		if (ret)
> > +			return ret;
> > +	}
> >  
> >  	platform_set_drvdata(pdev, idev);
> >  
> > @@ -1091,7 +1072,6 @@ static int at91_adc_probe(struct platform_device *pdev)
> >  	if (IS_ERR(st->reg_base))
> >  		return PTR_ERR(st->reg_base);
> >  
> > -
> Stray change that shouldn't be in this patch ideally but not that important.
> 

I know I sneaked this one in but I didn't want to have that as a separate
patch ;). I'll drop it.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/9] iio: adc: at91_adc: remove forward declaration

[Alexandre Belloni]

Remove the forward declaration of at91_adc_dt_ids by using
device_get_match_data. Also add const were possible since it is not
discarded by the cast anymore.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/iio/adc/at91_adc.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 9f05eb722f5e..76aeebce6f4d 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -224,7 +224,6 @@ struct at91_adc_state {
 	struct mutex		lock;
 	u8			num_channels;
 	void __iomem		*reg_base;
-	struct at91_adc_reg_desc *registers;
 	u32			startup_time;
 	u8			sample_hold_time;
 	bool			sleep_mode;
@@ -233,7 +232,8 @@ struct at91_adc_state {
 	u32			vref_mv;
 	u32			res;		/* resolution used for convertions */
 	wait_queue_head_t	wq_data_avail;
-	struct at91_adc_caps	*caps;
+	const struct at91_adc_caps *caps;
+	const struct at91_adc_reg_desc *registers;
 
 	/*
 	 * Following ADC channels are shared by touchscreen:
@@ -569,7 +569,7 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
 {
 	struct iio_dev *idev = iio_trigger_get_drvdata(trig);
 	struct at91_adc_state *st = iio_priv(idev);
-	struct at91_adc_reg_desc *reg = st->registers;
+	const struct at91_adc_reg_desc *reg = st->registers;
 	u32 status = at91_adc_readl(st, reg->trigger_register);
 	int value;
 	u8 bit;
@@ -796,8 +796,6 @@ static u32 calc_startup_ticks_9x5(u32 startup_time, u32 adc_clk_khz)
 	return ticks;
 }
 
-static const struct of_device_id at91_adc_dt_ids[];
-
 static int at91_adc_probe_dt_ts(struct device_node *node,
 	struct at91_adc_state *st, struct device *dev)
 {
@@ -1011,8 +1009,7 @@ static int at91_adc_probe(struct platform_device *pdev)
 
 	st = iio_priv(idev);
 
-	st->caps = (struct at91_adc_caps *)
-		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
+	st->caps = device_get_match_data(&pdev->dev);
 
 	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
 
-- 
2.28.0

[PATCH 6/9] iio: adc: at91_adc: remove forward declaration

[Alexandre Belloni]

Remove the forward declaration of at91_adc_dt_ids by using
device_get_match_data. Also add const were possible since it is not
discarded by the cast anymore.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/iio/adc/at91_adc.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 9f05eb722f5e..76aeebce6f4d 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -224,7 +224,6 @@ struct at91_adc_state {
 	struct mutex		lock;
 	u8			num_channels;
 	void __iomem		*reg_base;
-	struct at91_adc_reg_desc *registers;
 	u32			startup_time;
 	u8			sample_hold_time;
 	bool			sleep_mode;
@@ -233,7 +232,8 @@ struct at91_adc_state {
 	u32			vref_mv;
 	u32			res;		/* resolution used for convertions */
 	wait_queue_head_t	wq_data_avail;
-	struct at91_adc_caps	*caps;
+	const struct at91_adc_caps *caps;
+	const struct at91_adc_reg_desc *registers;
 
 	/*
 	 * Following ADC channels are shared by touchscreen:
@@ -569,7 +569,7 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
 {
 	struct iio_dev *idev = iio_trigger_get_drvdata(trig);
 	struct at91_adc_state *st = iio_priv(idev);
-	struct at91_adc_reg_desc *reg = st->registers;
+	const struct at91_adc_reg_desc *reg = st->registers;
 	u32 status = at91_adc_readl(st, reg->trigger_register);
 	int value;
 	u8 bit;
@@ -796,8 +796,6 @@ static u32 calc_startup_ticks_9x5(u32 startup_time, u32 adc_clk_khz)
 	return ticks;
 }
 
-static const struct of_device_id at91_adc_dt_ids[];
-
 static int at91_adc_probe_dt_ts(struct device_node *node,
 	struct at91_adc_state *st, struct device *dev)
 {
@@ -1011,8 +1009,7 @@ static int at91_adc_probe(struct platform_device *pdev)
 
 	st = iio_priv(idev);
 
-	st->caps = (struct at91_adc_caps *)
-		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
+	st->caps = device_get_match_data(&pdev->dev);
 
 	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
 
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/9] iio: adc: at91_adc: remove forward declaration

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:47 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> Remove the forward declaration of at91_adc_dt_ids by using
> device_get_match_data. Also add const were possible since it is not
> discarded by the cast anymore.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

Ignore comment on previous patch ;)

Jonathan

> ---
>  drivers/iio/adc/at91_adc.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 9f05eb722f5e..76aeebce6f4d 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -224,7 +224,6 @@ struct at91_adc_state {
>  	struct mutex		lock;
>  	u8			num_channels;
>  	void __iomem		*reg_base;
> -	struct at91_adc_reg_desc *registers;
>  	u32			startup_time;
>  	u8			sample_hold_time;
>  	bool			sleep_mode;
> @@ -233,7 +232,8 @@ struct at91_adc_state {
>  	u32			vref_mv;
>  	u32			res;		/* resolution used for convertions */
>  	wait_queue_head_t	wq_data_avail;
> -	struct at91_adc_caps	*caps;
> +	const struct at91_adc_caps *caps;
> +	const struct at91_adc_reg_desc *registers;
>  
>  	/*
>  	 * Following ADC channels are shared by touchscreen:
> @@ -569,7 +569,7 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
>  {
>  	struct iio_dev *idev = iio_trigger_get_drvdata(trig);
>  	struct at91_adc_state *st = iio_priv(idev);
> -	struct at91_adc_reg_desc *reg = st->registers;
> +	const struct at91_adc_reg_desc *reg = st->registers;
>  	u32 status = at91_adc_readl(st, reg->trigger_register);
>  	int value;
>  	u8 bit;
> @@ -796,8 +796,6 @@ static u32 calc_startup_ticks_9x5(u32 startup_time, u32 adc_clk_khz)
>  	return ticks;
>  }
>  
> -static const struct of_device_id at91_adc_dt_ids[];
> -
>  static int at91_adc_probe_dt_ts(struct device_node *node,
>  	struct at91_adc_state *st, struct device *dev)
>  {
> @@ -1011,8 +1009,7 @@ static int at91_adc_probe(struct platform_device *pdev)
>  
>  	st = iio_priv(idev);
>  
> -	st->caps = (struct at91_adc_caps *)
> -		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
> +	st->caps = device_get_match_data(&pdev->dev);
>  
>  	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
>  

Re: [PATCH 6/9] iio: adc: at91_adc: remove forward declaration

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:47 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> Remove the forward declaration of at91_adc_dt_ids by using
> device_get_match_data. Also add const were possible since it is not
> discarded by the cast anymore.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

Ignore comment on previous patch ;)

Jonathan

> ---
>  drivers/iio/adc/at91_adc.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 9f05eb722f5e..76aeebce6f4d 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -224,7 +224,6 @@ struct at91_adc_state {
>  	struct mutex		lock;
>  	u8			num_channels;
>  	void __iomem		*reg_base;
> -	struct at91_adc_reg_desc *registers;
>  	u32			startup_time;
>  	u8			sample_hold_time;
>  	bool			sleep_mode;
> @@ -233,7 +232,8 @@ struct at91_adc_state {
>  	u32			vref_mv;
>  	u32			res;		/* resolution used for convertions */
>  	wait_queue_head_t	wq_data_avail;
> -	struct at91_adc_caps	*caps;
> +	const struct at91_adc_caps *caps;
> +	const struct at91_adc_reg_desc *registers;
>  
>  	/*
>  	 * Following ADC channels are shared by touchscreen:
> @@ -569,7 +569,7 @@ static int at91_adc_configure_trigger(struct iio_trigger *trig, bool state)
>  {
>  	struct iio_dev *idev = iio_trigger_get_drvdata(trig);
>  	struct at91_adc_state *st = iio_priv(idev);
> -	struct at91_adc_reg_desc *reg = st->registers;
> +	const struct at91_adc_reg_desc *reg = st->registers;
>  	u32 status = at91_adc_readl(st, reg->trigger_register);
>  	int value;
>  	u8 bit;
> @@ -796,8 +796,6 @@ static u32 calc_startup_ticks_9x5(u32 startup_time, u32 adc_clk_khz)
>  	return ticks;
>  }
>  
> -static const struct of_device_id at91_adc_dt_ids[];
> -
>  static int at91_adc_probe_dt_ts(struct device_node *node,
>  	struct at91_adc_state *st, struct device *dev)
>  {
> @@ -1011,8 +1009,7 @@ static int at91_adc_probe(struct platform_device *pdev)
>  
>  	st = iio_priv(idev);
>  
> -	st->caps = (struct at91_adc_caps *)
> -		of_match_device(at91_adc_dt_ids, &pdev->dev)->data;
> +	st->caps = device_get_match_data(&pdev->dev);
>  
>  	st->use_external = of_property_read_bool(node, "atmel,adc-use-external-triggers");
>  


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 7/9] iio: adc: at91_adc: use devm_input_allocate_device

[Alexandre Belloni]

Use devm_input_allocate_device to allocate the input device to simplify the
error and remove paths.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/iio/adc/at91_adc.c | 25 +++----------------------
 1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 76aeebce6f4d..d09ceb315c5a 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -937,9 +937,8 @@ static int at91_ts_register(struct iio_dev *idev,
 {
 	struct at91_adc_state *st = iio_priv(idev);
 	struct input_dev *input;
-	int ret;
 
-	input = input_allocate_device();
+	input = devm_input_allocate_device(&pdev->dev);
 	if (!input) {
 		dev_err(&idev->dev, "Failed to allocate TS device!\n");
 		return -ENOMEM;
@@ -964,8 +963,7 @@ static int at91_ts_register(struct iio_dev *idev,
 		if (st->touchscreen_type != ATMEL_ADC_TOUCHSCREEN_4WIRE) {
 			dev_err(&pdev->dev,
 				"This touchscreen controller only support 4 wires\n");
-			ret = -EINVAL;
-			goto err;
+			return -EINVAL;
 		}
 
 		input_set_abs_params(input, ABS_X, 0, (1 << MAX_RLPOS_BITS) - 1,
@@ -977,20 +975,7 @@ static int at91_ts_register(struct iio_dev *idev,
 	st->ts_input = input;
 	input_set_drvdata(input, st);
 
-	ret = input_register_device(input);
-	if (ret)
-		goto err;
-
-	return ret;
-
-err:
-	input_free_device(st->ts_input);
-	return ret;
-}
-
-static void at91_ts_unregister(struct at91_adc_state *st)
-{
-	input_unregister_device(st->ts_input);
+	return input_register_device(input);
 }
 
 static int at91_adc_probe(struct platform_device *pdev)
@@ -1203,8 +1188,6 @@ static int at91_adc_probe(struct platform_device *pdev)
 	if (!st->touchscreen_type) {
 		at91_adc_trigger_remove(idev);
 		at91_adc_buffer_remove(idev);
-	} else {
-		at91_ts_unregister(st);
 	}
 error_disable_adc_clk:
 	clk_disable_unprepare(st->adc_clk);
@@ -1224,8 +1207,6 @@ static int at91_adc_remove(struct platform_device *pdev)
 	if (!st->touchscreen_type) {
 		at91_adc_trigger_remove(idev);
 		at91_adc_buffer_remove(idev);
-	} else {
-		at91_ts_unregister(st);
 	}
 	clk_disable_unprepare(st->adc_clk);
 	clk_disable_unprepare(st->clk);
-- 
2.28.0

[PATCH 7/9] iio: adc: at91_adc: use devm_input_allocate_device

[Alexandre Belloni]

Use devm_input_allocate_device to allocate the input device to simplify the
error and remove paths.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 drivers/iio/adc/at91_adc.c | 25 +++----------------------
 1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
index 76aeebce6f4d..d09ceb315c5a 100644
--- a/drivers/iio/adc/at91_adc.c
+++ b/drivers/iio/adc/at91_adc.c
@@ -937,9 +937,8 @@ static int at91_ts_register(struct iio_dev *idev,
 {
 	struct at91_adc_state *st = iio_priv(idev);
 	struct input_dev *input;
-	int ret;
 
-	input = input_allocate_device();
+	input = devm_input_allocate_device(&pdev->dev);
 	if (!input) {
 		dev_err(&idev->dev, "Failed to allocate TS device!\n");
 		return -ENOMEM;
@@ -964,8 +963,7 @@ static int at91_ts_register(struct iio_dev *idev,
 		if (st->touchscreen_type != ATMEL_ADC_TOUCHSCREEN_4WIRE) {
 			dev_err(&pdev->dev,
 				"This touchscreen controller only support 4 wires\n");
-			ret = -EINVAL;
-			goto err;
+			return -EINVAL;
 		}
 
 		input_set_abs_params(input, ABS_X, 0, (1 << MAX_RLPOS_BITS) - 1,
@@ -977,20 +975,7 @@ static int at91_ts_register(struct iio_dev *idev,
 	st->ts_input = input;
 	input_set_drvdata(input, st);
 
-	ret = input_register_device(input);
-	if (ret)
-		goto err;
-
-	return ret;
-
-err:
-	input_free_device(st->ts_input);
-	return ret;
-}
-
-static void at91_ts_unregister(struct at91_adc_state *st)
-{
-	input_unregister_device(st->ts_input);
+	return input_register_device(input);
 }
 
 static int at91_adc_probe(struct platform_device *pdev)
@@ -1203,8 +1188,6 @@ static int at91_adc_probe(struct platform_device *pdev)
 	if (!st->touchscreen_type) {
 		at91_adc_trigger_remove(idev);
 		at91_adc_buffer_remove(idev);
-	} else {
-		at91_ts_unregister(st);
 	}
 error_disable_adc_clk:
 	clk_disable_unprepare(st->adc_clk);
@@ -1224,8 +1207,6 @@ static int at91_adc_remove(struct platform_device *pdev)
 	if (!st->touchscreen_type) {
 		at91_adc_trigger_remove(idev);
 		at91_adc_buffer_remove(idev);
-	} else {
-		at91_ts_unregister(st);
 	}
 	clk_disable_unprepare(st->adc_clk);
 	clk_disable_unprepare(st->clk);
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 7/9] iio: adc: at91_adc: use devm_input_allocate_device

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:48 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> Use devm_input_allocate_device to allocate the input device to simplify the
> error and remove paths.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

This one I'm less keen on.  Whilst it's obviously not a problem in
this particular case I'd ideally like to keep the remove order
as the exact reverse of probe - that makes it easy to review changes
quickly.

Now, you could easily enough make this fine by using devm for the
other items that happen before this (dev_add_action_or_reset needed
in a few cases).

Jonathan

> ---
>  drivers/iio/adc/at91_adc.c | 25 +++----------------------
>  1 file changed, 3 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 76aeebce6f4d..d09ceb315c5a 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -937,9 +937,8 @@ static int at91_ts_register(struct iio_dev *idev,
>  {
>  	struct at91_adc_state *st = iio_priv(idev);
>  	struct input_dev *input;
> -	int ret;
>  
> -	input = input_allocate_device();
> +	input = devm_input_allocate_device(&pdev->dev);
>  	if (!input) {
>  		dev_err(&idev->dev, "Failed to allocate TS device!\n");
>  		return -ENOMEM;
> @@ -964,8 +963,7 @@ static int at91_ts_register(struct iio_dev *idev,
>  		if (st->touchscreen_type != ATMEL_ADC_TOUCHSCREEN_4WIRE) {
>  			dev_err(&pdev->dev,
>  				"This touchscreen controller only support 4 wires\n");
> -			ret = -EINVAL;
> -			goto err;
> +			return -EINVAL;
>  		}
>  
>  		input_set_abs_params(input, ABS_X, 0, (1 << MAX_RLPOS_BITS) - 1,
> @@ -977,20 +975,7 @@ static int at91_ts_register(struct iio_dev *idev,
>  	st->ts_input = input;
>  	input_set_drvdata(input, st);
>  
> -	ret = input_register_device(input);
> -	if (ret)
> -		goto err;
> -
> -	return ret;
> -
> -err:
> -	input_free_device(st->ts_input);
> -	return ret;
> -}
> -
> -static void at91_ts_unregister(struct at91_adc_state *st)
> -{
> -	input_unregister_device(st->ts_input);
> +	return input_register_device(input);
>  }
>  
>  static int at91_adc_probe(struct platform_device *pdev)
> @@ -1203,8 +1188,6 @@ static int at91_adc_probe(struct platform_device *pdev)
>  	if (!st->touchscreen_type) {
>  		at91_adc_trigger_remove(idev);
>  		at91_adc_buffer_remove(idev);
> -	} else {
> -		at91_ts_unregister(st);
>  	}
>  error_disable_adc_clk:
>  	clk_disable_unprepare(st->adc_clk);
> @@ -1224,8 +1207,6 @@ static int at91_adc_remove(struct platform_device *pdev)
>  	if (!st->touchscreen_type) {
>  		at91_adc_trigger_remove(idev);
>  		at91_adc_buffer_remove(idev);
> -	} else {
> -		at91_ts_unregister(st);
>  	}
>  	clk_disable_unprepare(st->adc_clk);
>  	clk_disable_unprepare(st->clk);

Re: [PATCH 7/9] iio: adc: at91_adc: use devm_input_allocate_device

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:48 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> Use devm_input_allocate_device to allocate the input device to simplify the
> error and remove paths.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

This one I'm less keen on.  Whilst it's obviously not a problem in
this particular case I'd ideally like to keep the remove order
as the exact reverse of probe - that makes it easy to review changes
quickly.

Now, you could easily enough make this fine by using devm for the
other items that happen before this (dev_add_action_or_reset needed
in a few cases).

Jonathan

> ---
>  drivers/iio/adc/at91_adc.c | 25 +++----------------------
>  1 file changed, 3 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/iio/adc/at91_adc.c b/drivers/iio/adc/at91_adc.c
> index 76aeebce6f4d..d09ceb315c5a 100644
> --- a/drivers/iio/adc/at91_adc.c
> +++ b/drivers/iio/adc/at91_adc.c
> @@ -937,9 +937,8 @@ static int at91_ts_register(struct iio_dev *idev,
>  {
>  	struct at91_adc_state *st = iio_priv(idev);
>  	struct input_dev *input;
> -	int ret;
>  
> -	input = input_allocate_device();
> +	input = devm_input_allocate_device(&pdev->dev);
>  	if (!input) {
>  		dev_err(&idev->dev, "Failed to allocate TS device!\n");
>  		return -ENOMEM;
> @@ -964,8 +963,7 @@ static int at91_ts_register(struct iio_dev *idev,
>  		if (st->touchscreen_type != ATMEL_ADC_TOUCHSCREEN_4WIRE) {
>  			dev_err(&pdev->dev,
>  				"This touchscreen controller only support 4 wires\n");
> -			ret = -EINVAL;
> -			goto err;
> +			return -EINVAL;
>  		}
>  
>  		input_set_abs_params(input, ABS_X, 0, (1 << MAX_RLPOS_BITS) - 1,
> @@ -977,20 +975,7 @@ static int at91_ts_register(struct iio_dev *idev,
>  	st->ts_input = input;
>  	input_set_drvdata(input, st);
>  
> -	ret = input_register_device(input);
> -	if (ret)
> -		goto err;
> -
> -	return ret;
> -
> -err:
> -	input_free_device(st->ts_input);
> -	return ret;
> -}
> -
> -static void at91_ts_unregister(struct at91_adc_state *st)
> -{
> -	input_unregister_device(st->ts_input);
> +	return input_register_device(input);
>  }
>  
>  static int at91_adc_probe(struct platform_device *pdev)
> @@ -1203,8 +1188,6 @@ static int at91_adc_probe(struct platform_device *pdev)
>  	if (!st->touchscreen_type) {
>  		at91_adc_trigger_remove(idev);
>  		at91_adc_buffer_remove(idev);
> -	} else {
> -		at91_ts_unregister(st);
>  	}
>  error_disable_adc_clk:
>  	clk_disable_unprepare(st->adc_clk);
> @@ -1224,8 +1207,6 @@ static int at91_adc_remove(struct platform_device *pdev)
>  	if (!st->touchscreen_type) {
>  		at91_adc_trigger_remove(idev);
>  		at91_adc_buffer_remove(idev);
> -	} else {
> -		at91_ts_unregister(st);
>  	}
>  	clk_disable_unprepare(st->adc_clk);
>  	clk_disable_unprepare(st->clk);


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 7/9] iio: adc: at91_adc: use devm_input_allocate_device

[Alexandre Belloni]

On 14/11/2020 17:13:40+0000, Jonathan Cameron wrote:
> On Fri, 13 Nov 2020 22:26:48 +0100
> Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> 
> > Use devm_input_allocate_device to allocate the input device to simplify the
> > error and remove paths.
> > 
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> 
> This one I'm less keen on.  Whilst it's obviously not a problem in
> this particular case I'd ideally like to keep the remove order
> as the exact reverse of probe - that makes it easy to review changes
> quickly.
> 
> Now, you could easily enough make this fine by using devm for the
> other items that happen before this (dev_add_action_or_reset needed
> in a few cases).
> 

Right, I'll drop it for now.

Re: [PATCH 7/9] iio: adc: at91_adc: use devm_input_allocate_device

[Alexandre Belloni]

On 14/11/2020 17:13:40+0000, Jonathan Cameron wrote:
> On Fri, 13 Nov 2020 22:26:48 +0100
> Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> 
> > Use devm_input_allocate_device to allocate the input device to simplify the
> > error and remove paths.
> > 
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> 
> This one I'm less keen on.  Whilst it's obviously not a problem in
> this particular case I'd ideally like to keep the remove order
> as the exact reverse of probe - that makes it easy to review changes
> quickly.
> 
> Now, you could easily enough make this fine by using devm for the
> other items that happen before this (dev_add_action_or_reset needed
> in a few cases).
> 

Right, I'll drop it for now.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 8/9] ARM: dts: at91: sama5d3: use proper ADC compatible

[Alexandre Belloni]

The ADC is different from the at91sam9x5 ADC. Not only it doesn't have the
same resolution but it even have only one and the LOWRES bit doesn't exist.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 arch/arm/boot/dts/sama5d3.dtsi | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index 86137f8d2b45..e0807b723533 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -307,7 +307,7 @@ ssc1: ssc@f800c000 {
 			adc0: adc@f8018000 {
 				#address-cells = <1>;
 				#size-cells = <0>;
-				compatible = "atmel,at91sam9x5-adc";
+				compatible = "atmel,sama5d3-adc";
 				reg = <0xf8018000 0x100>;
 				interrupts = <29 IRQ_TYPE_LEVEL_HIGH 5>;
 				pinctrl-names = "default";
@@ -333,9 +333,7 @@ &pinctrl_adc0_ad11
 				atmel,adc-startup-time = <40>;
 				atmel,adc-use-external-triggers;
 				atmel,adc-vref = <3000>;
-				atmel,adc-res = <10 12>;
 				atmel,adc-sample-hold-time = <11>;
-				atmel,adc-res-names = "lowres", "highres";
 				status = "disabled";
 
 				trigger0 {
-- 
2.28.0

[PATCH 8/9] ARM: dts: at91: sama5d3: use proper ADC compatible

[Alexandre Belloni]

The ADC is different from the at91sam9x5 ADC. Not only it doesn't have the
same resolution but it even have only one and the LOWRES bit doesn't exist.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 arch/arm/boot/dts/sama5d3.dtsi | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index 86137f8d2b45..e0807b723533 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -307,7 +307,7 @@ ssc1: ssc@f800c000 {
 			adc0: adc@f8018000 {
 				#address-cells = <1>;
 				#size-cells = <0>;
-				compatible = "atmel,at91sam9x5-adc";
+				compatible = "atmel,sama5d3-adc";
 				reg = <0xf8018000 0x100>;
 				interrupts = <29 IRQ_TYPE_LEVEL_HIGH 5>;
 				pinctrl-names = "default";
@@ -333,9 +333,7 @@ &pinctrl_adc0_ad11
 				atmel,adc-startup-time = <40>;
 				atmel,adc-use-external-triggers;
 				atmel,adc-vref = <3000>;
-				atmel,adc-res = <10 12>;
 				atmel,adc-sample-hold-time = <11>;
-				atmel,adc-res-names = "lowres", "highres";
 				status = "disabled";
 
 				trigger0 {
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 9/9] ARM: dts: at91: remove deprecated ADC properties

[Alexandre Belloni]

atmel,adc-res, atmel,adc-res-names and the trigger nodes are not parsed by
the driver anymore and the information is now defined in the driver data.

Also remove the leftover #address-cells and #size-cells that were used when
the trigger nodes had a unit-address.

Finally, the default is already to use the highest resoution. Remove
atmel,adc-use-res from the SoC dtsi.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 arch/arm/boot/dts/at91sam9260.dtsi | 25 -------------------------
 arch/arm/boot/dts/at91sam9g45.dtsi | 27 ---------------------------
 arch/arm/boot/dts/at91sam9rl.dtsi  | 25 -------------------------
 arch/arm/boot/dts/at91sam9x5.dtsi  | 28 ----------------------------
 arch/arm/boot/dts/sama5d3.dtsi     | 22 ----------------------
 arch/arm/boot/dts/sama5d4.dtsi     | 22 ----------------------
 6 files changed, 149 deletions(-)

diff --git a/arch/arm/boot/dts/at91sam9260.dtsi b/arch/arm/boot/dts/at91sam9260.dtsi
index 82c5d7fd9811..019f1c3d4d30 100644
--- a/arch/arm/boot/dts/at91sam9260.dtsi
+++ b/arch/arm/boot/dts/at91sam9260.dtsi
@@ -697,8 +697,6 @@ spi1: spi@fffcc000 {
 			};
 
 			adc0: adc@fffe0000 {
-				#address-cells = <1>;
-				#size-cells = <0>;
 				compatible = "atmel,at91sam9260-adc";
 				reg = <0xfffe0000 0x100>;
 				interrupts = <5 IRQ_TYPE_LEVEL_HIGH 0>;
@@ -708,29 +706,6 @@ adc0: adc@fffe0000 {
 				atmel,adc-channels-used = <0xf>;
 				atmel,adc-vref = <3300>;
 				atmel,adc-startup-time = <15>;
-				atmel,adc-res = <8 10>;
-				atmel,adc-res-names = "lowres", "highres";
-				atmel,adc-use-res = "highres";
-
-				trigger0 {
-					trigger-name = "timer-counter-0";
-					trigger-value = <0x1>;
-				};
-				trigger1 {
-					trigger-name = "timer-counter-1";
-					trigger-value = <0x3>;
-				};
-
-				trigger2 {
-					trigger-name = "timer-counter-2";
-					trigger-value = <0x5>;
-				};
-
-				trigger3 {
-					trigger-name = "external";
-					trigger-value = <0xd>;
-					trigger-external;
-				};
 			};
 
 			rtc@fffffd20 {
diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index 19fc748a87c5..2ab730fd6472 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -812,8 +812,6 @@ ac97: sound@fffac000 {
 			};
 
 			adc0: adc@fffb0000 {
-				#address-cells = <1>;
-				#size-cells = <0>;
 				compatible = "atmel,at91sam9g45-adc";
 				reg = <0xfffb0000 0x100>;
 				interrupts = <20 IRQ_TYPE_LEVEL_HIGH 0>;
@@ -822,31 +820,6 @@ adc0: adc@fffb0000 {
 				atmel,adc-channels-used = <0xff>;
 				atmel,adc-vref = <3300>;
 				atmel,adc-startup-time = <40>;
-				atmel,adc-res = <8 10>;
-				atmel,adc-res-names = "lowres", "highres";
-				atmel,adc-use-res = "highres";
-
-				trigger0 {
-					trigger-name = "external-rising";
-					trigger-value = <0x1>;
-					trigger-external;
-				};
-				trigger1 {
-					trigger-name = "external-falling";
-					trigger-value = <0x2>;
-					trigger-external;
-				};
-
-				trigger2 {
-					trigger-name = "external-any";
-					trigger-value = <0x3>;
-					trigger-external;
-				};
-
-				trigger3 {
-					trigger-name = "continuous";
-					trigger-value = <0x6>;
-				};
 			};
 
 			isi@fffb4000 {
diff --git a/arch/arm/boot/dts/at91sam9rl.dtsi b/arch/arm/boot/dts/at91sam9rl.dtsi
index 5653e70c84b4..730d1182c73e 100644
--- a/arch/arm/boot/dts/at91sam9rl.dtsi
+++ b/arch/arm/boot/dts/at91sam9rl.dtsi
@@ -266,8 +266,6 @@ spi0: spi@fffcc000 {
 			};
 
 			adc0: adc@fffd0000 {
-				#address-cells = <1>;
-				#size-cells = <0>;
 				compatible = "atmel,at91sam9rl-adc";
 				reg = <0xfffd0000 0x100>;
 				interrupts = <20 IRQ_TYPE_LEVEL_HIGH 0>;
@@ -277,29 +275,6 @@ adc0: adc@fffd0000 {
 				atmel,adc-channels-used = <0x3f>;
 				atmel,adc-vref = <3300>;
 				atmel,adc-startup-time = <40>;
-				atmel,adc-res = <8 10>;
-				atmel,adc-res-names = "lowres", "highres";
-				atmel,adc-use-res = "highres";
-
-				trigger0 {
-					trigger-name = "timer-counter-0";
-					trigger-value = <0x1>;
-				};
-				trigger1 {
-					trigger-name = "timer-counter-1";
-					trigger-value = <0x3>;
-				};
-
-				trigger2 {
-					trigger-name = "timer-counter-2";
-					trigger-value = <0x5>;
-				};
-
-				trigger3 {
-					trigger-name = "external";
-					trigger-value = <0x13>;
-					trigger-external;
-				};
 			};
 
 			usb0: gadget@fffd4000 {
diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
index 4cdb05079cc7..395e883644cd 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -795,8 +795,6 @@ uart1: serial@f8044000 {
 			};
 
 			adc0: adc@f804c000 {
-				#address-cells = <1>;
-				#size-cells = <0>;
 				compatible = "atmel,at91sam9x5-adc";
 				reg = <0xf804c000 0x100>;
 				interrupts = <19 IRQ_TYPE_LEVEL_HIGH 0>;
@@ -808,32 +806,6 @@ adc0: adc@f804c000 {
 				atmel,adc-vref = <3300>;
 				atmel,adc-startup-time = <40>;
 				atmel,adc-sample-hold-time = <11>;
-				atmel,adc-res = <8 10>;
-				atmel,adc-res-names = "lowres", "highres";
-				atmel,adc-use-res = "highres";
-
-				trigger0 {
-					trigger-name = "external-rising";
-					trigger-value = <0x1>;
-					trigger-external;
-				};
-
-				trigger1 {
-					trigger-name = "external-falling";
-					trigger-value = <0x2>;
-					trigger-external;
-				};
-
-				trigger2 {
-					trigger-name = "external-any";
-					trigger-value = <0x3>;
-					trigger-external;
-				};
-
-				trigger3 {
-					trigger-name = "continuous";
-					trigger-value = <0x6>;
-				};
 			};
 
 			spi0: spi@f0000000 {
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index e0807b723533..7c979652f330 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -305,8 +305,6 @@ ssc1: ssc@f800c000 {
 			};
 
 			adc0: adc@f8018000 {
-				#address-cells = <1>;
-				#size-cells = <0>;
 				compatible = "atmel,sama5d3-adc";
 				reg = <0xf8018000 0x100>;
 				interrupts = <29 IRQ_TYPE_LEVEL_HIGH 5>;
@@ -335,26 +333,6 @@ &pinctrl_adc0_ad11
 				atmel,adc-vref = <3000>;
 				atmel,adc-sample-hold-time = <11>;
 				status = "disabled";
-
-				trigger0 {
-					trigger-name = "external-rising";
-					trigger-value = <0x1>;
-					trigger-external;
-				};
-				trigger1 {
-					trigger-name = "external-falling";
-					trigger-value = <0x2>;
-					trigger-external;
-				};
-				trigger2 {
-					trigger-name = "external-any";
-					trigger-value = <0x3>;
-					trigger-external;
-				};
-				trigger3 {
-					trigger-name = "continuous";
-					trigger-value = <0x6>;
-				};
 			};
 
 			i2c2: i2c@f801c000 {
diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index 04f24cf752d3..05c55875835d 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -661,31 +661,9 @@ adc0: adc@fc034000 {
 				atmel,adc-startup-time = <40>;
 				atmel,adc-use-external-triggers;
 				atmel,adc-vref = <3000>;
-				atmel,adc-res = <8 10>;
 				atmel,adc-sample-hold-time = <11>;
-				atmel,adc-res-names = "lowres", "highres";
 				atmel,adc-ts-pressure-threshold = <10000>;
 				status = "disabled";
-
-				trigger0 {
-					trigger-name = "external-rising";
-					trigger-value = <0x1>;
-					trigger-external;
-				};
-				trigger1 {
-					trigger-name = "external-falling";
-					trigger-value = <0x2>;
-					trigger-external;
-				};
-				trigger2 {
-					trigger-name = "external-any";
-					trigger-value = <0x3>;
-					trigger-external;
-				};
-				trigger3 {
-					trigger-name = "continuous";
-					trigger-value = <0x6>;
-				};
 			};
 
 			aes@fc044000 {
-- 
2.28.0

[PATCH 9/9] ARM: dts: at91: remove deprecated ADC properties

[Alexandre Belloni]

atmel,adc-res, atmel,adc-res-names and the trigger nodes are not parsed by
the driver anymore and the information is now defined in the driver data.

Also remove the leftover #address-cells and #size-cells that were used when
the trigger nodes had a unit-address.

Finally, the default is already to use the highest resoution. Remove
atmel,adc-use-res from the SoC dtsi.

Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
 arch/arm/boot/dts/at91sam9260.dtsi | 25 -------------------------
 arch/arm/boot/dts/at91sam9g45.dtsi | 27 ---------------------------
 arch/arm/boot/dts/at91sam9rl.dtsi  | 25 -------------------------
 arch/arm/boot/dts/at91sam9x5.dtsi  | 28 ----------------------------
 arch/arm/boot/dts/sama5d3.dtsi     | 22 ----------------------
 arch/arm/boot/dts/sama5d4.dtsi     | 22 ----------------------
 6 files changed, 149 deletions(-)

diff --git a/arch/arm/boot/dts/at91sam9260.dtsi b/arch/arm/boot/dts/at91sam9260.dtsi
index 82c5d7fd9811..019f1c3d4d30 100644
--- a/arch/arm/boot/dts/at91sam9260.dtsi
+++ b/arch/arm/boot/dts/at91sam9260.dtsi
@@ -697,8 +697,6 @@ spi1: spi@fffcc000 {
 			};
 
 			adc0: adc@fffe0000 {
-				#address-cells = <1>;
-				#size-cells = <0>;
 				compatible = "atmel,at91sam9260-adc";
 				reg = <0xfffe0000 0x100>;
 				interrupts = <5 IRQ_TYPE_LEVEL_HIGH 0>;
@@ -708,29 +706,6 @@ adc0: adc@fffe0000 {
 				atmel,adc-channels-used = <0xf>;
 				atmel,adc-vref = <3300>;
 				atmel,adc-startup-time = <15>;
-				atmel,adc-res = <8 10>;
-				atmel,adc-res-names = "lowres", "highres";
-				atmel,adc-use-res = "highres";
-
-				trigger0 {
-					trigger-name = "timer-counter-0";
-					trigger-value = <0x1>;
-				};
-				trigger1 {
-					trigger-name = "timer-counter-1";
-					trigger-value = <0x3>;
-				};
-
-				trigger2 {
-					trigger-name = "timer-counter-2";
-					trigger-value = <0x5>;
-				};
-
-				trigger3 {
-					trigger-name = "external";
-					trigger-value = <0xd>;
-					trigger-external;
-				};
 			};
 
 			rtc@fffffd20 {
diff --git a/arch/arm/boot/dts/at91sam9g45.dtsi b/arch/arm/boot/dts/at91sam9g45.dtsi
index 19fc748a87c5..2ab730fd6472 100644
--- a/arch/arm/boot/dts/at91sam9g45.dtsi
+++ b/arch/arm/boot/dts/at91sam9g45.dtsi
@@ -812,8 +812,6 @@ ac97: sound@fffac000 {
 			};
 
 			adc0: adc@fffb0000 {
-				#address-cells = <1>;
-				#size-cells = <0>;
 				compatible = "atmel,at91sam9g45-adc";
 				reg = <0xfffb0000 0x100>;
 				interrupts = <20 IRQ_TYPE_LEVEL_HIGH 0>;
@@ -822,31 +820,6 @@ adc0: adc@fffb0000 {
 				atmel,adc-channels-used = <0xff>;
 				atmel,adc-vref = <3300>;
 				atmel,adc-startup-time = <40>;
-				atmel,adc-res = <8 10>;
-				atmel,adc-res-names = "lowres", "highres";
-				atmel,adc-use-res = "highres";
-
-				trigger0 {
-					trigger-name = "external-rising";
-					trigger-value = <0x1>;
-					trigger-external;
-				};
-				trigger1 {
-					trigger-name = "external-falling";
-					trigger-value = <0x2>;
-					trigger-external;
-				};
-
-				trigger2 {
-					trigger-name = "external-any";
-					trigger-value = <0x3>;
-					trigger-external;
-				};
-
-				trigger3 {
-					trigger-name = "continuous";
-					trigger-value = <0x6>;
-				};
 			};
 
 			isi@fffb4000 {
diff --git a/arch/arm/boot/dts/at91sam9rl.dtsi b/arch/arm/boot/dts/at91sam9rl.dtsi
index 5653e70c84b4..730d1182c73e 100644
--- a/arch/arm/boot/dts/at91sam9rl.dtsi
+++ b/arch/arm/boot/dts/at91sam9rl.dtsi
@@ -266,8 +266,6 @@ spi0: spi@fffcc000 {
 			};
 
 			adc0: adc@fffd0000 {
-				#address-cells = <1>;
-				#size-cells = <0>;
 				compatible = "atmel,at91sam9rl-adc";
 				reg = <0xfffd0000 0x100>;
 				interrupts = <20 IRQ_TYPE_LEVEL_HIGH 0>;
@@ -277,29 +275,6 @@ adc0: adc@fffd0000 {
 				atmel,adc-channels-used = <0x3f>;
 				atmel,adc-vref = <3300>;
 				atmel,adc-startup-time = <40>;
-				atmel,adc-res = <8 10>;
-				atmel,adc-res-names = "lowres", "highres";
-				atmel,adc-use-res = "highres";
-
-				trigger0 {
-					trigger-name = "timer-counter-0";
-					trigger-value = <0x1>;
-				};
-				trigger1 {
-					trigger-name = "timer-counter-1";
-					trigger-value = <0x3>;
-				};
-
-				trigger2 {
-					trigger-name = "timer-counter-2";
-					trigger-value = <0x5>;
-				};
-
-				trigger3 {
-					trigger-name = "external";
-					trigger-value = <0x13>;
-					trigger-external;
-				};
 			};
 
 			usb0: gadget@fffd4000 {
diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
index 4cdb05079cc7..395e883644cd 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -795,8 +795,6 @@ uart1: serial@f8044000 {
 			};
 
 			adc0: adc@f804c000 {
-				#address-cells = <1>;
-				#size-cells = <0>;
 				compatible = "atmel,at91sam9x5-adc";
 				reg = <0xf804c000 0x100>;
 				interrupts = <19 IRQ_TYPE_LEVEL_HIGH 0>;
@@ -808,32 +806,6 @@ adc0: adc@f804c000 {
 				atmel,adc-vref = <3300>;
 				atmel,adc-startup-time = <40>;
 				atmel,adc-sample-hold-time = <11>;
-				atmel,adc-res = <8 10>;
-				atmel,adc-res-names = "lowres", "highres";
-				atmel,adc-use-res = "highres";
-
-				trigger0 {
-					trigger-name = "external-rising";
-					trigger-value = <0x1>;
-					trigger-external;
-				};
-
-				trigger1 {
-					trigger-name = "external-falling";
-					trigger-value = <0x2>;
-					trigger-external;
-				};
-
-				trigger2 {
-					trigger-name = "external-any";
-					trigger-value = <0x3>;
-					trigger-external;
-				};
-
-				trigger3 {
-					trigger-name = "continuous";
-					trigger-value = <0x6>;
-				};
 			};
 
 			spi0: spi@f0000000 {
diff --git a/arch/arm/boot/dts/sama5d3.dtsi b/arch/arm/boot/dts/sama5d3.dtsi
index e0807b723533..7c979652f330 100644
--- a/arch/arm/boot/dts/sama5d3.dtsi
+++ b/arch/arm/boot/dts/sama5d3.dtsi
@@ -305,8 +305,6 @@ ssc1: ssc@f800c000 {
 			};
 
 			adc0: adc@f8018000 {
-				#address-cells = <1>;
-				#size-cells = <0>;
 				compatible = "atmel,sama5d3-adc";
 				reg = <0xf8018000 0x100>;
 				interrupts = <29 IRQ_TYPE_LEVEL_HIGH 5>;
@@ -335,26 +333,6 @@ &pinctrl_adc0_ad11
 				atmel,adc-vref = <3000>;
 				atmel,adc-sample-hold-time = <11>;
 				status = "disabled";
-
-				trigger0 {
-					trigger-name = "external-rising";
-					trigger-value = <0x1>;
-					trigger-external;
-				};
-				trigger1 {
-					trigger-name = "external-falling";
-					trigger-value = <0x2>;
-					trigger-external;
-				};
-				trigger2 {
-					trigger-name = "external-any";
-					trigger-value = <0x3>;
-					trigger-external;
-				};
-				trigger3 {
-					trigger-name = "continuous";
-					trigger-value = <0x6>;
-				};
 			};
 
 			i2c2: i2c@f801c000 {
diff --git a/arch/arm/boot/dts/sama5d4.dtsi b/arch/arm/boot/dts/sama5d4.dtsi
index 04f24cf752d3..05c55875835d 100644
--- a/arch/arm/boot/dts/sama5d4.dtsi
+++ b/arch/arm/boot/dts/sama5d4.dtsi
@@ -661,31 +661,9 @@ adc0: adc@fc034000 {
 				atmel,adc-startup-time = <40>;
 				atmel,adc-use-external-triggers;
 				atmel,adc-vref = <3000>;
-				atmel,adc-res = <8 10>;
 				atmel,adc-sample-hold-time = <11>;
-				atmel,adc-res-names = "lowres", "highres";
 				atmel,adc-ts-pressure-threshold = <10000>;
 				status = "disabled";
-
-				trigger0 {
-					trigger-name = "external-rising";
-					trigger-value = <0x1>;
-					trigger-external;
-				};
-				trigger1 {
-					trigger-name = "external-falling";
-					trigger-value = <0x2>;
-					trigger-external;
-				};
-				trigger2 {
-					trigger-name = "external-any";
-					trigger-value = <0x3>;
-					trigger-external;
-				};
-				trigger3 {
-					trigger-name = "continuous";
-					trigger-value = <0x6>;
-				};
 			};
 
 			aes@fc044000 {
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/9] iio: adc: at91_adc: cleanup DT bindings

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:41 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> Hello,
> 
> This series cleans up the at91_adc devicetree bindings. This mainly
> moves back the resolution options and names and the triggers description
> back in the driver.
> 
> There are also other cleanups, like removing platform data support, this
> was pending for a while.

I'm fine with everything up to patch 7.  Given it touches DT I'll leave
a bit of time for Rob to take a quick look (can't see him objecting to the
removal of sections of the binding though!)

If you don't mind me dropping 7 I can pick 1-6 up in a week or two.
Thanks,

Jonathan

> 
> Alexandre Belloni (8):
>   iio: adc: at91_adc: remove platform data
>   iio: adc: at91_adc: rework resolution selection
>   iio: adc: at91_adc: rework trigger definition
>   iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe
>   iio: adc: at91_adc: remove forward declaration
>   iio: adc: at91_adc: use devm_input_allocate_device
>   ARM: dts: at91: sama5d3: use proper ADC compatible
>   ARM: dts: at91: remove deprecated ADC properties
> 
> Jonathan Cameron (1):
>   dt-bindings:iio:adc:atmel,sama9260-adc: conversion to yaml from
>     at91_adc.txt
> 
>  .../devicetree/bindings/iio/adc/at91_adc.txt  |  83 ----
>  .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 121 ++++++
>  arch/arm/boot/dts/at91sam9260.dtsi            |  25 --
>  arch/arm/boot/dts/at91sam9g45.dtsi            |  27 --
>  arch/arm/boot/dts/at91sam9rl.dtsi             |  25 --
>  arch/arm/boot/dts/at91sam9x5.dtsi             |  28 --
>  arch/arm/boot/dts/sama5d3.dtsi                |  26 +-
>  arch/arm/boot/dts/sama5d4.dtsi                |  22 -
>  drivers/iio/adc/at91_adc.c                    | 377 +++++++-----------
>  include/linux/platform_data/at91_adc.h        |  49 ---
>  10 files changed, 259 insertions(+), 524 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/iio/adc/at91_adc.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
>  delete mode 100644 include/linux/platform_data/at91_adc.h
> 

Re: [PATCH 0/9] iio: adc: at91_adc: cleanup DT bindings

[Jonathan Cameron]

On Fri, 13 Nov 2020 22:26:41 +0100
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> Hello,
> 
> This series cleans up the at91_adc devicetree bindings. This mainly
> moves back the resolution options and names and the triggers description
> back in the driver.
> 
> There are also other cleanups, like removing platform data support, this
> was pending for a while.

I'm fine with everything up to patch 7.  Given it touches DT I'll leave
a bit of time for Rob to take a quick look (can't see him objecting to the
removal of sections of the binding though!)

If you don't mind me dropping 7 I can pick 1-6 up in a week or two.
Thanks,

Jonathan

> 
> Alexandre Belloni (8):
>   iio: adc: at91_adc: remove platform data
>   iio: adc: at91_adc: rework resolution selection
>   iio: adc: at91_adc: rework trigger definition
>   iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe
>   iio: adc: at91_adc: remove forward declaration
>   iio: adc: at91_adc: use devm_input_allocate_device
>   ARM: dts: at91: sama5d3: use proper ADC compatible
>   ARM: dts: at91: remove deprecated ADC properties
> 
> Jonathan Cameron (1):
>   dt-bindings:iio:adc:atmel,sama9260-adc: conversion to yaml from
>     at91_adc.txt
> 
>  .../devicetree/bindings/iio/adc/at91_adc.txt  |  83 ----
>  .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 121 ++++++
>  arch/arm/boot/dts/at91sam9260.dtsi            |  25 --
>  arch/arm/boot/dts/at91sam9g45.dtsi            |  27 --
>  arch/arm/boot/dts/at91sam9rl.dtsi             |  25 --
>  arch/arm/boot/dts/at91sam9x5.dtsi             |  28 --
>  arch/arm/boot/dts/sama5d3.dtsi                |  26 +-
>  arch/arm/boot/dts/sama5d4.dtsi                |  22 -
>  drivers/iio/adc/at91_adc.c                    | 377 +++++++-----------
>  include/linux/platform_data/at91_adc.h        |  49 ---
>  10 files changed, 259 insertions(+), 524 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/iio/adc/at91_adc.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
>  delete mode 100644 include/linux/platform_data/at91_adc.h
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/9] iio: adc: at91_adc: cleanup DT bindings

[Ludovic Desroches]

On Fri, Nov 13, 2020 at 10:26:41PM +0100, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hello,
> 
> This series cleans up the at91_adc devicetree bindings. This mainly
> moves back the resolution options and names and the triggers description
> back in the driver.
> 
> There are also other cleanups, like removing platform data support, this
> was pending for a while.
>

Nice cleanup of this old and obscur binding.

Reviewed-by: Ludovic Desroches <ludovic.desroches@microchip.com>

Thanks.

> Alexandre Belloni (8):
>   iio: adc: at91_adc: remove platform data
>   iio: adc: at91_adc: rework resolution selection
>   iio: adc: at91_adc: rework trigger definition
>   iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe
>   iio: adc: at91_adc: remove forward declaration
>   iio: adc: at91_adc: use devm_input_allocate_device
>   ARM: dts: at91: sama5d3: use proper ADC compatible
>   ARM: dts: at91: remove deprecated ADC properties
> 
> Jonathan Cameron (1):
>   dt-bindings:iio:adc:atmel,sama9260-adc: conversion to yaml from
>     at91_adc.txt
> 
>  .../devicetree/bindings/iio/adc/at91_adc.txt  |  83 ----
>  .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 121 ++++++
>  arch/arm/boot/dts/at91sam9260.dtsi            |  25 --
>  arch/arm/boot/dts/at91sam9g45.dtsi            |  27 --
>  arch/arm/boot/dts/at91sam9rl.dtsi             |  25 --
>  arch/arm/boot/dts/at91sam9x5.dtsi             |  28 --
>  arch/arm/boot/dts/sama5d3.dtsi                |  26 +-
>  arch/arm/boot/dts/sama5d4.dtsi                |  22 -
>  drivers/iio/adc/at91_adc.c                    | 377 +++++++-----------
>  include/linux/platform_data/at91_adc.h        |  49 ---
>  10 files changed, 259 insertions(+), 524 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/iio/adc/at91_adc.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
>  delete mode 100644 include/linux/platform_data/at91_adc.h
> 
> --
> 2.28.0

Re: [PATCH 0/9] iio: adc: at91_adc: cleanup DT bindings

[Ludovic Desroches]

On Fri, Nov 13, 2020 at 10:26:41PM +0100, Alexandre Belloni wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hello,
> 
> This series cleans up the at91_adc devicetree bindings. This mainly
> moves back the resolution options and names and the triggers description
> back in the driver.
> 
> There are also other cleanups, like removing platform data support, this
> was pending for a while.
>

Nice cleanup of this old and obscur binding.

Reviewed-by: Ludovic Desroches <ludovic.desroches@microchip.com>

Thanks.

> Alexandre Belloni (8):
>   iio: adc: at91_adc: remove platform data
>   iio: adc: at91_adc: rework resolution selection
>   iio: adc: at91_adc: rework trigger definition
>   iio: adc: at91_adc: merge at91_adc_probe_dt back in at91_adc_probe
>   iio: adc: at91_adc: remove forward declaration
>   iio: adc: at91_adc: use devm_input_allocate_device
>   ARM: dts: at91: sama5d3: use proper ADC compatible
>   ARM: dts: at91: remove deprecated ADC properties
> 
> Jonathan Cameron (1):
>   dt-bindings:iio:adc:atmel,sama9260-adc: conversion to yaml from
>     at91_adc.txt
> 
>  .../devicetree/bindings/iio/adc/at91_adc.txt  |  83 ----
>  .../bindings/iio/adc/atmel,sama9260-adc.yaml  | 121 ++++++
>  arch/arm/boot/dts/at91sam9260.dtsi            |  25 --
>  arch/arm/boot/dts/at91sam9g45.dtsi            |  27 --
>  arch/arm/boot/dts/at91sam9rl.dtsi             |  25 --
>  arch/arm/boot/dts/at91sam9x5.dtsi             |  28 --
>  arch/arm/boot/dts/sama5d3.dtsi                |  26 +-
>  arch/arm/boot/dts/sama5d4.dtsi                |  22 -
>  drivers/iio/adc/at91_adc.c                    | 377 +++++++-----------
>  include/linux/platform_data/at91_adc.h        |  49 ---
>  10 files changed, 259 insertions(+), 524 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/iio/adc/at91_adc.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/atmel,sama9260-adc.yaml
>  delete mode 100644 include/linux/platform_data/at91_adc.h
> 
> --
> 2.28.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel