All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-08-30 13:07 ` Heiko Stübner
  0 siblings, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2014-08-30 13:07 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-arm-kernel, linux-kernel, linux-iio, linux-rockchip, devicetree

Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
for temperature measurements. This so called tsadc does not contain any
active parts like temperature interrupts and only supports polling the
current temperature. The returned voltage can then be converted by a
suitable thermal driver to and actual temperature and used for thermal
handling.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
As the commit messages states, the one used on the rk3066 is just a saradc,
while the new tsadc in the rk3288 contains active components for thermal
handling.

A working example using the pending iio-thermal driver can found on
https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f254176619fae36f2aad0

 .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
 drivers/iio/adc/rockchip_saradc.c                  | 62 +++++++++++++++++-----
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
index 5d3ec1d..a9a5fe1 100644
--- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
@@ -1,7 +1,7 @@
 Rockchip Successive Approximation Register (SAR) A/D Converter bindings
 
 Required properties:
-- compatible: Should be "rockchip,saradc"
+- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
 - reg: physical base address of the controller and length of memory mapped
        region.
 - interrupts: The interrupt number to the cpu. The interrupt specifier format
diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
index e074a0b..7effada 100644
--- a/drivers/iio/adc/rockchip_saradc.c
+++ b/drivers/iio/adc/rockchip_saradc.c
@@ -18,13 +18,13 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/regulator/consumer.h>
 #include <linux/iio/iio.h>
 
 #define SARADC_DATA			0x00
-#define SARADC_DATA_MASK		0x3ff
 
 #define SARADC_STAS			0x04
 #define SARADC_STAS_BUSY		BIT(0)
@@ -38,15 +38,22 @@
 #define SARADC_DLY_PU_SOC		0x0c
 #define SARADC_DLY_PU_SOC_MASK		0x3f
 
-#define SARADC_BITS			10
 #define SARADC_TIMEOUT			msecs_to_jiffies(100)
 
+struct rockchip_saradc_data {
+	int				num_bits;
+	const struct iio_chan_spec	*channels;
+	int				num_channels;
+	unsigned long			clk_rate;
+};
+
 struct rockchip_saradc {
 	void __iomem		*regs;
 	struct clk		*pclk;
 	struct clk		*clk;
 	struct completion	completion;
 	struct regulator	*vref;
+	const struct rockchip_saradc_data *data;
 	u16			last_val;
 };
 
@@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
 		}
 
 		*val = ret / 1000;
-		*val2 = SARADC_BITS;
+		*val2 = info->data->num_bits;
 		return IIO_VAL_FRACTIONAL_LOG2;
 	default:
 		return -EINVAL;
@@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void *dev_id)
 
 	/* Read value */
 	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
-	info->last_val &= SARADC_DATA_MASK;
+	info->last_val &= BIT(info->data->num_bits) - 1;
 
 	/* Clear irq & power down adc */
 	writel_relaxed(0, info->regs + SARADC_CTRL);
@@ -133,12 +140,44 @@ static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
 	ADC_CHANNEL(2, "adc2"),
 };
 
+static const struct rockchip_saradc_data saradc_data = {
+	.num_bits = 10,
+	.channels = rockchip_saradc_iio_channels,
+	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
+	.clk_rate = 1000000,
+};
+
+static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] = {
+	ADC_CHANNEL(0, "adc0"),
+	ADC_CHANNEL(1, "adc1"),
+};
+
+static const struct rockchip_saradc_data rk3066_tsadc_data = {
+	.num_bits = 12,
+	.channels = rockchip_rk3066_tsadc_iio_channels,
+	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
+	.clk_rate = 50000,
+};
+
+static const struct of_device_id rockchip_saradc_match[] = {
+	{
+		.compatible = "rockchip,saradc",
+		.data = &saradc_data,
+	}, {
+		.compatible = "rockchip,rk3066-tsadc",
+		.data = &rk3066_tsadc_data,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
+
 static int rockchip_saradc_probe(struct platform_device *pdev)
 {
 	struct rockchip_saradc *info = NULL;
 	struct device_node *np = pdev->dev.of_node;
 	struct iio_dev *indio_dev = NULL;
 	struct resource	*mem;
+	const struct of_device_id *match;
 	int ret;
 	int irq;
 
@@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
 	}
 	info = iio_priv(indio_dev);
 
+	match = of_match_device(rockchip_saradc_match, &pdev->dev);
+	info->data = match->data;
+
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	info->regs = devm_ioremap_resource(&pdev->dev, mem);
 	if (IS_ERR(info->regs))
@@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
 	 * Use a default of 1MHz for the converter clock.
 	 * This may become user-configurable in the future.
 	 */
-	ret = clk_set_rate(info->clk, 1000000);
+	ret = clk_set_rate(info->clk, info->data->clk_rate);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
 		return ret;
@@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
 	indio_dev->info = &rockchip_saradc_iio_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	indio_dev->channels = rockchip_saradc_iio_channels;
-	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
+	indio_dev->channels = info->data->channels;
+	indio_dev->num_channels = info->data->num_channels;
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
@@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
 			 rockchip_saradc_suspend, rockchip_saradc_resume);
 
-static const struct of_device_id rockchip_saradc_match[] = {
-	{ .compatible = "rockchip,saradc" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
-
 static struct platform_driver rockchip_saradc_driver = {
 	.probe		= rockchip_saradc_probe,
 	.remove		= rockchip_saradc_remove,
-- 
2.0.1



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

* [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-08-30 13:07 ` Heiko Stübner
  0 siblings, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2014-08-30 13:07 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
for temperature measurements. This so called tsadc does not contain any
active parts like temperature interrupts and only supports polling the
current temperature. The returned voltage can then be converted by a
suitable thermal driver to and actual temperature and used for thermal
handling.

Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
---
This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
As the commit messages states, the one used on the rk3066 is just a saradc,
while the new tsadc in the rk3288 contains active components for thermal
handling.

A working example using the pending iio-thermal driver can found on
https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f254176619fae36f2aad0

 .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
 drivers/iio/adc/rockchip_saradc.c                  | 62 +++++++++++++++++-----
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
index 5d3ec1d..a9a5fe1 100644
--- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
@@ -1,7 +1,7 @@
 Rockchip Successive Approximation Register (SAR) A/D Converter bindings
 
 Required properties:
-- compatible: Should be "rockchip,saradc"
+- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
 - reg: physical base address of the controller and length of memory mapped
        region.
 - interrupts: The interrupt number to the cpu. The interrupt specifier format
diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
index e074a0b..7effada 100644
--- a/drivers/iio/adc/rockchip_saradc.c
+++ b/drivers/iio/adc/rockchip_saradc.c
@@ -18,13 +18,13 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/regulator/consumer.h>
 #include <linux/iio/iio.h>
 
 #define SARADC_DATA			0x00
-#define SARADC_DATA_MASK		0x3ff
 
 #define SARADC_STAS			0x04
 #define SARADC_STAS_BUSY		BIT(0)
@@ -38,15 +38,22 @@
 #define SARADC_DLY_PU_SOC		0x0c
 #define SARADC_DLY_PU_SOC_MASK		0x3f
 
-#define SARADC_BITS			10
 #define SARADC_TIMEOUT			msecs_to_jiffies(100)
 
+struct rockchip_saradc_data {
+	int				num_bits;
+	const struct iio_chan_spec	*channels;
+	int				num_channels;
+	unsigned long			clk_rate;
+};
+
 struct rockchip_saradc {
 	void __iomem		*regs;
 	struct clk		*pclk;
 	struct clk		*clk;
 	struct completion	completion;
 	struct regulator	*vref;
+	const struct rockchip_saradc_data *data;
 	u16			last_val;
 };
 
@@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
 		}
 
 		*val = ret / 1000;
-		*val2 = SARADC_BITS;
+		*val2 = info->data->num_bits;
 		return IIO_VAL_FRACTIONAL_LOG2;
 	default:
 		return -EINVAL;
@@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void *dev_id)
 
 	/* Read value */
 	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
-	info->last_val &= SARADC_DATA_MASK;
+	info->last_val &= BIT(info->data->num_bits) - 1;
 
 	/* Clear irq & power down adc */
 	writel_relaxed(0, info->regs + SARADC_CTRL);
@@ -133,12 +140,44 @@ static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
 	ADC_CHANNEL(2, "adc2"),
 };
 
+static const struct rockchip_saradc_data saradc_data = {
+	.num_bits = 10,
+	.channels = rockchip_saradc_iio_channels,
+	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
+	.clk_rate = 1000000,
+};
+
+static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] = {
+	ADC_CHANNEL(0, "adc0"),
+	ADC_CHANNEL(1, "adc1"),
+};
+
+static const struct rockchip_saradc_data rk3066_tsadc_data = {
+	.num_bits = 12,
+	.channels = rockchip_rk3066_tsadc_iio_channels,
+	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
+	.clk_rate = 50000,
+};
+
+static const struct of_device_id rockchip_saradc_match[] = {
+	{
+		.compatible = "rockchip,saradc",
+		.data = &saradc_data,
+	}, {
+		.compatible = "rockchip,rk3066-tsadc",
+		.data = &rk3066_tsadc_data,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
+
 static int rockchip_saradc_probe(struct platform_device *pdev)
 {
 	struct rockchip_saradc *info = NULL;
 	struct device_node *np = pdev->dev.of_node;
 	struct iio_dev *indio_dev = NULL;
 	struct resource	*mem;
+	const struct of_device_id *match;
 	int ret;
 	int irq;
 
@@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
 	}
 	info = iio_priv(indio_dev);
 
+	match = of_match_device(rockchip_saradc_match, &pdev->dev);
+	info->data = match->data;
+
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	info->regs = devm_ioremap_resource(&pdev->dev, mem);
 	if (IS_ERR(info->regs))
@@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
 	 * Use a default of 1MHz for the converter clock.
 	 * This may become user-configurable in the future.
 	 */
-	ret = clk_set_rate(info->clk, 1000000);
+	ret = clk_set_rate(info->clk, info->data->clk_rate);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
 		return ret;
@@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
 	indio_dev->info = &rockchip_saradc_iio_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	indio_dev->channels = rockchip_saradc_iio_channels;
-	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
+	indio_dev->channels = info->data->channels;
+	indio_dev->num_channels = info->data->num_channels;
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
@@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
 			 rockchip_saradc_suspend, rockchip_saradc_resume);
 
-static const struct of_device_id rockchip_saradc_match[] = {
-	{ .compatible = "rockchip,saradc" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
-
 static struct platform_driver rockchip_saradc_driver = {
 	.probe		= rockchip_saradc_probe,
 	.remove		= rockchip_saradc_remove,
-- 
2.0.1

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

* [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-08-30 13:07 ` Heiko Stübner
  0 siblings, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2014-08-30 13:07 UTC (permalink / raw)
  To: linux-arm-kernel

Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
for temperature measurements. This so called tsadc does not contain any
active parts like temperature interrupts and only supports polling the
current temperature. The returned voltage can then be converted by a
suitable thermal driver to and actual temperature and used for thermal
handling.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
As the commit messages states, the one used on the rk3066 is just a saradc,
while the new tsadc in the rk3288 contains active components for thermal
handling.

A working example using the pending iio-thermal driver can found on
https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f254176619fae36f2aad0

 .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
 drivers/iio/adc/rockchip_saradc.c                  | 62 +++++++++++++++++-----
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
index 5d3ec1d..a9a5fe1 100644
--- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
@@ -1,7 +1,7 @@
 Rockchip Successive Approximation Register (SAR) A/D Converter bindings
 
 Required properties:
-- compatible: Should be "rockchip,saradc"
+- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
 - reg: physical base address of the controller and length of memory mapped
        region.
 - interrupts: The interrupt number to the cpu. The interrupt specifier format
diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
index e074a0b..7effada 100644
--- a/drivers/iio/adc/rockchip_saradc.c
+++ b/drivers/iio/adc/rockchip_saradc.c
@@ -18,13 +18,13 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/regulator/consumer.h>
 #include <linux/iio/iio.h>
 
 #define SARADC_DATA			0x00
-#define SARADC_DATA_MASK		0x3ff
 
 #define SARADC_STAS			0x04
 #define SARADC_STAS_BUSY		BIT(0)
@@ -38,15 +38,22 @@
 #define SARADC_DLY_PU_SOC		0x0c
 #define SARADC_DLY_PU_SOC_MASK		0x3f
 
-#define SARADC_BITS			10
 #define SARADC_TIMEOUT			msecs_to_jiffies(100)
 
+struct rockchip_saradc_data {
+	int				num_bits;
+	const struct iio_chan_spec	*channels;
+	int				num_channels;
+	unsigned long			clk_rate;
+};
+
 struct rockchip_saradc {
 	void __iomem		*regs;
 	struct clk		*pclk;
 	struct clk		*clk;
 	struct completion	completion;
 	struct regulator	*vref;
+	const struct rockchip_saradc_data *data;
 	u16			last_val;
 };
 
@@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
 		}
 
 		*val = ret / 1000;
-		*val2 = SARADC_BITS;
+		*val2 = info->data->num_bits;
 		return IIO_VAL_FRACTIONAL_LOG2;
 	default:
 		return -EINVAL;
@@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void *dev_id)
 
 	/* Read value */
 	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
-	info->last_val &= SARADC_DATA_MASK;
+	info->last_val &= BIT(info->data->num_bits) - 1;
 
 	/* Clear irq & power down adc */
 	writel_relaxed(0, info->regs + SARADC_CTRL);
@@ -133,12 +140,44 @@ static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
 	ADC_CHANNEL(2, "adc2"),
 };
 
+static const struct rockchip_saradc_data saradc_data = {
+	.num_bits = 10,
+	.channels = rockchip_saradc_iio_channels,
+	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
+	.clk_rate = 1000000,
+};
+
+static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] = {
+	ADC_CHANNEL(0, "adc0"),
+	ADC_CHANNEL(1, "adc1"),
+};
+
+static const struct rockchip_saradc_data rk3066_tsadc_data = {
+	.num_bits = 12,
+	.channels = rockchip_rk3066_tsadc_iio_channels,
+	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
+	.clk_rate = 50000,
+};
+
+static const struct of_device_id rockchip_saradc_match[] = {
+	{
+		.compatible = "rockchip,saradc",
+		.data = &saradc_data,
+	}, {
+		.compatible = "rockchip,rk3066-tsadc",
+		.data = &rk3066_tsadc_data,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
+
 static int rockchip_saradc_probe(struct platform_device *pdev)
 {
 	struct rockchip_saradc *info = NULL;
 	struct device_node *np = pdev->dev.of_node;
 	struct iio_dev *indio_dev = NULL;
 	struct resource	*mem;
+	const struct of_device_id *match;
 	int ret;
 	int irq;
 
@@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
 	}
 	info = iio_priv(indio_dev);
 
+	match = of_match_device(rockchip_saradc_match, &pdev->dev);
+	info->data = match->data;
+
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	info->regs = devm_ioremap_resource(&pdev->dev, mem);
 	if (IS_ERR(info->regs))
@@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
 	 * Use a default of 1MHz for the converter clock.
 	 * This may become user-configurable in the future.
 	 */
-	ret = clk_set_rate(info->clk, 1000000);
+	ret = clk_set_rate(info->clk, info->data->clk_rate);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
 		return ret;
@@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
 	indio_dev->info = &rockchip_saradc_iio_info;
 	indio_dev->modes = INDIO_DIRECT_MODE;
 
-	indio_dev->channels = rockchip_saradc_iio_channels;
-	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
+	indio_dev->channels = info->data->channels;
+	indio_dev->num_channels = info->data->num_channels;
 
 	ret = iio_device_register(indio_dev);
 	if (ret)
@@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
 static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
 			 rockchip_saradc_suspend, rockchip_saradc_resume);
 
-static const struct of_device_id rockchip_saradc_match[] = {
-	{ .compatible = "rockchip,saradc" },
-	{},
-};
-MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
-
 static struct platform_driver rockchip_saradc_driver = {
 	.probe		= rockchip_saradc_probe,
 	.remove		= rockchip_saradc_remove,
-- 
2.0.1

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

* Re: [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
  2014-08-30 13:07 ` Heiko Stübner
  (?)
@ 2014-08-30 20:15   ` Jonathan Cameron
  -1 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2014-08-30 20:15 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: linux-arm-kernel, linux-kernel, linux-iio, linux-rockchip, devicetree

On 30/08/14 14:07, Heiko Stübner wrote:
> Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
> for temperature measurements. This so called tsadc does not contain any
> active parts like temperature interrupts and only supports polling the
> current temperature. The returned voltage can then be converted by a
> suitable thermal driver to and actual temperature and used for thermal
> handling.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Looks, good to me, but will let this sit for a few days for other comments on
it.  Long shot, but are the docs for the vairous rockchip parts publically
available somewhere?
> ---
> This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
> As the commit messages states, the one used on the rk3066 is just a saradc,
> while the new tsadc in the rk3288 contains active components for thermal
> handling.
>
> A working example using the pending iio-thermal driver can found on
> https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f254176619fae36f2aad0
>
>  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
>  drivers/iio/adc/rockchip_saradc.c                  | 62 +++++++++++++++++-----
>  2 files changed, 50 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> index 5d3ec1d..a9a5fe1 100644
> --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> @@ -1,7 +1,7 @@
>  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
>
>  Required properties:
> -- compatible: Should be "rockchip,saradc"
> +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
>  - reg: physical base address of the controller and length of memory mapped
>         region.
>  - interrupts: The interrupt number to the cpu. The interrupt specifier format
> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> index e074a0b..7effada 100644
> --- a/drivers/iio/adc/rockchip_saradc.c
> +++ b/drivers/iio/adc/rockchip_saradc.c
> @@ -18,13 +18,13 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/iio/iio.h>
>
>  #define SARADC_DATA			0x00
> -#define SARADC_DATA_MASK		0x3ff
>
>  #define SARADC_STAS			0x04
>  #define SARADC_STAS_BUSY		BIT(0)
> @@ -38,15 +38,22 @@
>  #define SARADC_DLY_PU_SOC		0x0c
>  #define SARADC_DLY_PU_SOC_MASK		0x3f
>
> -#define SARADC_BITS			10
>  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
>
> +struct rockchip_saradc_data {
> +	int				num_bits;
> +	const struct iio_chan_spec	*channels;
> +	int				num_channels;
> +	unsigned long			clk_rate;
> +};
> +
>  struct rockchip_saradc {
>  	void __iomem		*regs;
>  	struct clk		*pclk;
>  	struct clk		*clk;
>  	struct completion	completion;
>  	struct regulator	*vref;
> +	const struct rockchip_saradc_data *data;
>  	u16			last_val;
>  };
>
> @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>  		}
>
>  		*val = ret / 1000;
> -		*val2 = SARADC_BITS;
> +		*val2 = info->data->num_bits;
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  	default:
>  		return -EINVAL;
> @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void *dev_id)
>
>  	/* Read value */
>  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
> -	info->last_val &= SARADC_DATA_MASK;
> +	info->last_val &= BIT(info->data->num_bits) - 1;
>
>  	/* Clear irq & power down adc */
>  	writel_relaxed(0, info->regs + SARADC_CTRL);
> @@ -133,12 +140,44 @@ static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
>  	ADC_CHANNEL(2, "adc2"),
>  };
>
> +static const struct rockchip_saradc_data saradc_data = {
> +	.num_bits = 10,
> +	.channels = rockchip_saradc_iio_channels,
> +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
> +	.clk_rate = 1000000,
> +};
> +
> +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] = {
> +	ADC_CHANNEL(0, "adc0"),
> +	ADC_CHANNEL(1, "adc1"),
> +};
> +
> +static const struct rockchip_saradc_data rk3066_tsadc_data = {
> +	.num_bits = 12,
> +	.channels = rockchip_rk3066_tsadc_iio_channels,
> +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
> +	.clk_rate = 50000,
> +};
> +
> +static const struct of_device_id rockchip_saradc_match[] = {
> +	{
> +		.compatible = "rockchip,saradc",
> +		.data = &saradc_data,
> +	}, {
> +		.compatible = "rockchip,rk3066-tsadc",
> +		.data = &rk3066_tsadc_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> +
>  static int rockchip_saradc_probe(struct platform_device *pdev)
>  {
>  	struct rockchip_saradc *info = NULL;
>  	struct device_node *np = pdev->dev.of_node;
>  	struct iio_dev *indio_dev = NULL;
>  	struct resource	*mem;
> +	const struct of_device_id *match;
>  	int ret;
>  	int irq;
>
> @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	}
>  	info = iio_priv(indio_dev);
>
> +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
> +	info->data = match->data;
> +
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
>  	if (IS_ERR(info->regs))
> @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	 * Use a default of 1MHz for the converter clock.
>  	 * This may become user-configurable in the future.
>  	 */
> -	ret = clk_set_rate(info->clk, 1000000);
> +	ret = clk_set_rate(info->clk, info->data->clk_rate);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
>  		return ret;
> @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	indio_dev->info = &rockchip_saradc_iio_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>
> -	indio_dev->channels = rockchip_saradc_iio_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
> +	indio_dev->channels = info->data->channels;
> +	indio_dev->num_channels = info->data->num_channels;
>
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
>  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
>  			 rockchip_saradc_suspend, rockchip_saradc_resume);
>
> -static const struct of_device_id rockchip_saradc_match[] = {
> -	{ .compatible = "rockchip,saradc" },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> -
>  static struct platform_driver rockchip_saradc_driver = {
>  	.probe		= rockchip_saradc_probe,
>  	.remove		= rockchip_saradc_remove,
>

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

* Re: [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-08-30 20:15   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2014-08-30 20:15 UTC (permalink / raw)
  To: Heiko Stübner
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 30/08/14 14:07, Heiko Stübner wrote:
> Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
> for temperature measurements. This so called tsadc does not contain any
> active parts like temperature interrupts and only supports polling the
> current temperature. The returned voltage can then be converted by a
> suitable thermal driver to and actual temperature and used for thermal
> handling.
>
> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
Looks, good to me, but will let this sit for a few days for other comments on
it.  Long shot, but are the docs for the vairous rockchip parts publically
available somewhere?
> ---
> This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
> As the commit messages states, the one used on the rk3066 is just a saradc,
> while the new tsadc in the rk3288 contains active components for thermal
> handling.
>
> A working example using the pending iio-thermal driver can found on
> https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f254176619fae36f2aad0
>
>  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
>  drivers/iio/adc/rockchip_saradc.c                  | 62 +++++++++++++++++-----
>  2 files changed, 50 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> index 5d3ec1d..a9a5fe1 100644
> --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> @@ -1,7 +1,7 @@
>  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
>
>  Required properties:
> -- compatible: Should be "rockchip,saradc"
> +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
>  - reg: physical base address of the controller and length of memory mapped
>         region.
>  - interrupts: The interrupt number to the cpu. The interrupt specifier format
> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> index e074a0b..7effada 100644
> --- a/drivers/iio/adc/rockchip_saradc.c
> +++ b/drivers/iio/adc/rockchip_saradc.c
> @@ -18,13 +18,13 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/iio/iio.h>
>
>  #define SARADC_DATA			0x00
> -#define SARADC_DATA_MASK		0x3ff
>
>  #define SARADC_STAS			0x04
>  #define SARADC_STAS_BUSY		BIT(0)
> @@ -38,15 +38,22 @@
>  #define SARADC_DLY_PU_SOC		0x0c
>  #define SARADC_DLY_PU_SOC_MASK		0x3f
>
> -#define SARADC_BITS			10
>  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
>
> +struct rockchip_saradc_data {
> +	int				num_bits;
> +	const struct iio_chan_spec	*channels;
> +	int				num_channels;
> +	unsigned long			clk_rate;
> +};
> +
>  struct rockchip_saradc {
>  	void __iomem		*regs;
>  	struct clk		*pclk;
>  	struct clk		*clk;
>  	struct completion	completion;
>  	struct regulator	*vref;
> +	const struct rockchip_saradc_data *data;
>  	u16			last_val;
>  };
>
> @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>  		}
>
>  		*val = ret / 1000;
> -		*val2 = SARADC_BITS;
> +		*val2 = info->data->num_bits;
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  	default:
>  		return -EINVAL;
> @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void *dev_id)
>
>  	/* Read value */
>  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
> -	info->last_val &= SARADC_DATA_MASK;
> +	info->last_val &= BIT(info->data->num_bits) - 1;
>
>  	/* Clear irq & power down adc */
>  	writel_relaxed(0, info->regs + SARADC_CTRL);
> @@ -133,12 +140,44 @@ static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
>  	ADC_CHANNEL(2, "adc2"),
>  };
>
> +static const struct rockchip_saradc_data saradc_data = {
> +	.num_bits = 10,
> +	.channels = rockchip_saradc_iio_channels,
> +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
> +	.clk_rate = 1000000,
> +};
> +
> +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] = {
> +	ADC_CHANNEL(0, "adc0"),
> +	ADC_CHANNEL(1, "adc1"),
> +};
> +
> +static const struct rockchip_saradc_data rk3066_tsadc_data = {
> +	.num_bits = 12,
> +	.channels = rockchip_rk3066_tsadc_iio_channels,
> +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
> +	.clk_rate = 50000,
> +};
> +
> +static const struct of_device_id rockchip_saradc_match[] = {
> +	{
> +		.compatible = "rockchip,saradc",
> +		.data = &saradc_data,
> +	}, {
> +		.compatible = "rockchip,rk3066-tsadc",
> +		.data = &rk3066_tsadc_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> +
>  static int rockchip_saradc_probe(struct platform_device *pdev)
>  {
>  	struct rockchip_saradc *info = NULL;
>  	struct device_node *np = pdev->dev.of_node;
>  	struct iio_dev *indio_dev = NULL;
>  	struct resource	*mem;
> +	const struct of_device_id *match;
>  	int ret;
>  	int irq;
>
> @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	}
>  	info = iio_priv(indio_dev);
>
> +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
> +	info->data = match->data;
> +
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
>  	if (IS_ERR(info->regs))
> @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	 * Use a default of 1MHz for the converter clock.
>  	 * This may become user-configurable in the future.
>  	 */
> -	ret = clk_set_rate(info->clk, 1000000);
> +	ret = clk_set_rate(info->clk, info->data->clk_rate);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
>  		return ret;
> @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	indio_dev->info = &rockchip_saradc_iio_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>
> -	indio_dev->channels = rockchip_saradc_iio_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
> +	indio_dev->channels = info->data->channels;
> +	indio_dev->num_channels = info->data->num_channels;
>
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
>  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
>  			 rockchip_saradc_suspend, rockchip_saradc_resume);
>
> -static const struct of_device_id rockchip_saradc_match[] = {
> -	{ .compatible = "rockchip,saradc" },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> -
>  static struct platform_driver rockchip_saradc_driver = {
>  	.probe		= rockchip_saradc_probe,
>  	.remove		= rockchip_saradc_remove,
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-08-30 20:15   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2014-08-30 20:15 UTC (permalink / raw)
  To: linux-arm-kernel

On 30/08/14 14:07, Heiko St?bner wrote:
> Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
> for temperature measurements. This so called tsadc does not contain any
> active parts like temperature interrupts and only supports polling the
> current temperature. The returned voltage can then be converted by a
> suitable thermal driver to and actual temperature and used for thermal
> handling.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
Looks, good to me, but will let this sit for a few days for other comments on
it.  Long shot, but are the docs for the vairous rockchip parts publically
available somewhere?
> ---
> This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
> As the commit messages states, the one used on the rk3066 is just a saradc,
> while the new tsadc in the rk3288 contains active components for thermal
> handling.
>
> A working example using the pending iio-thermal driver can found on
> https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f254176619fae36f2aad0
>
>  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
>  drivers/iio/adc/rockchip_saradc.c                  | 62 +++++++++++++++++-----
>  2 files changed, 50 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> index 5d3ec1d..a9a5fe1 100644
> --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> @@ -1,7 +1,7 @@
>  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
>
>  Required properties:
> -- compatible: Should be "rockchip,saradc"
> +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
>  - reg: physical base address of the controller and length of memory mapped
>         region.
>  - interrupts: The interrupt number to the cpu. The interrupt specifier format
> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> index e074a0b..7effada 100644
> --- a/drivers/iio/adc/rockchip_saradc.c
> +++ b/drivers/iio/adc/rockchip_saradc.c
> @@ -18,13 +18,13 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/iio/iio.h>
>
>  #define SARADC_DATA			0x00
> -#define SARADC_DATA_MASK		0x3ff
>
>  #define SARADC_STAS			0x04
>  #define SARADC_STAS_BUSY		BIT(0)
> @@ -38,15 +38,22 @@
>  #define SARADC_DLY_PU_SOC		0x0c
>  #define SARADC_DLY_PU_SOC_MASK		0x3f
>
> -#define SARADC_BITS			10
>  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
>
> +struct rockchip_saradc_data {
> +	int				num_bits;
> +	const struct iio_chan_spec	*channels;
> +	int				num_channels;
> +	unsigned long			clk_rate;
> +};
> +
>  struct rockchip_saradc {
>  	void __iomem		*regs;
>  	struct clk		*pclk;
>  	struct clk		*clk;
>  	struct completion	completion;
>  	struct regulator	*vref;
> +	const struct rockchip_saradc_data *data;
>  	u16			last_val;
>  };
>
> @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>  		}
>
>  		*val = ret / 1000;
> -		*val2 = SARADC_BITS;
> +		*val2 = info->data->num_bits;
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  	default:
>  		return -EINVAL;
> @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void *dev_id)
>
>  	/* Read value */
>  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
> -	info->last_val &= SARADC_DATA_MASK;
> +	info->last_val &= BIT(info->data->num_bits) - 1;
>
>  	/* Clear irq & power down adc */
>  	writel_relaxed(0, info->regs + SARADC_CTRL);
> @@ -133,12 +140,44 @@ static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
>  	ADC_CHANNEL(2, "adc2"),
>  };
>
> +static const struct rockchip_saradc_data saradc_data = {
> +	.num_bits = 10,
> +	.channels = rockchip_saradc_iio_channels,
> +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
> +	.clk_rate = 1000000,
> +};
> +
> +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] = {
> +	ADC_CHANNEL(0, "adc0"),
> +	ADC_CHANNEL(1, "adc1"),
> +};
> +
> +static const struct rockchip_saradc_data rk3066_tsadc_data = {
> +	.num_bits = 12,
> +	.channels = rockchip_rk3066_tsadc_iio_channels,
> +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
> +	.clk_rate = 50000,
> +};
> +
> +static const struct of_device_id rockchip_saradc_match[] = {
> +	{
> +		.compatible = "rockchip,saradc",
> +		.data = &saradc_data,
> +	}, {
> +		.compatible = "rockchip,rk3066-tsadc",
> +		.data = &rk3066_tsadc_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> +
>  static int rockchip_saradc_probe(struct platform_device *pdev)
>  {
>  	struct rockchip_saradc *info = NULL;
>  	struct device_node *np = pdev->dev.of_node;
>  	struct iio_dev *indio_dev = NULL;
>  	struct resource	*mem;
> +	const struct of_device_id *match;
>  	int ret;
>  	int irq;
>
> @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	}
>  	info = iio_priv(indio_dev);
>
> +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
> +	info->data = match->data;
> +
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
>  	if (IS_ERR(info->regs))
> @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	 * Use a default of 1MHz for the converter clock.
>  	 * This may become user-configurable in the future.
>  	 */
> -	ret = clk_set_rate(info->clk, 1000000);
> +	ret = clk_set_rate(info->clk, info->data->clk_rate);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
>  		return ret;
> @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	indio_dev->info = &rockchip_saradc_iio_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>
> -	indio_dev->channels = rockchip_saradc_iio_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
> +	indio_dev->channels = info->data->channels;
> +	indio_dev->num_channels = info->data->num_channels;
>
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
>  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
>  			 rockchip_saradc_suspend, rockchip_saradc_resume);
>
> -static const struct of_device_id rockchip_saradc_match[] = {
> -	{ .compatible = "rockchip,saradc" },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> -
>  static struct platform_driver rockchip_saradc_driver = {
>  	.probe		= rockchip_saradc_probe,
>  	.remove		= rockchip_saradc_remove,
>

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

* Re: [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-08-30 20:38     ` Heiko Stübner
  0 siblings, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2014-08-30 20:38 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-arm-kernel, linux-kernel, linux-iio, linux-rockchip, devicetree

Am Samstag, 30. August 2014, 21:15:53 schrieb Jonathan Cameron:
> On 30/08/14 14:07, Heiko Stübner wrote:
> > Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
> > for temperature measurements. This so called tsadc does not contain any
> > active parts like temperature interrupts and only supports polling the
> > current temperature. The returned voltage can then be converted by a
> > suitable thermal driver to and actual temperature and used for thermal
> > handling.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> 
> Looks, good to me, but will let this sit for a few days for other comments
> on it. 

ok, cool


> Long shot, but are the docs for the vairous rockchip parts
> publically available somewhere?

Not publically, at this point ... maybe somewhere in the future [hope :-) ]


Heiko

> 
> > ---
> > This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
> > As the commit messages states, the one used on the rk3066 is just a
> > saradc,
> > while the new tsadc in the rk3288 contains active components for thermal
> > handling.
> > 
> > A working example using the pending iio-thermal driver can found on
> > https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f2541766
> > 19fae36f2aad0> 
> >  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
> >  drivers/iio/adc/rockchip_saradc.c                  | 62
> >  +++++++++++++++++----- 2 files changed, 50 insertions(+), 14
> >  deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt index
> > 5d3ec1d..a9a5fe1 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > @@ -1,7 +1,7 @@
> > 
> >  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
> > 
> >  Required properties:
> > -- compatible: Should be "rockchip,saradc"
> > +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
> > 
> >  - reg: physical base address of the controller and length of memory
> >  mapped
> >  
> >         region.
> >  
> >  - interrupts: The interrupt number to the cpu. The interrupt specifier
> >  format> 
> > diff --git a/drivers/iio/adc/rockchip_saradc.c
> > b/drivers/iio/adc/rockchip_saradc.c index e074a0b..7effada 100644
> > --- a/drivers/iio/adc/rockchip_saradc.c
> > +++ b/drivers/iio/adc/rockchip_saradc.c
> > @@ -18,13 +18,13 @@
> > 
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> >  #include <linux/of.h>
> > 
> > +#include <linux/of_device.h>
> > 
> >  #include <linux/clk.h>
> >  #include <linux/completion.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/iio/iio.h>
> >  
> >  #define SARADC_DATA			0x00
> > 
> > -#define SARADC_DATA_MASK		0x3ff
> > 
> >  #define SARADC_STAS			0x04
> >  #define SARADC_STAS_BUSY		BIT(0)
> > 
> > @@ -38,15 +38,22 @@
> > 
> >  #define SARADC_DLY_PU_SOC		0x0c
> >  #define SARADC_DLY_PU_SOC_MASK		0x3f
> > 
> > -#define SARADC_BITS			10
> > 
> >  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
> > 
> > +struct rockchip_saradc_data {
> > +	int				num_bits;
> > +	const struct iio_chan_spec	*channels;
> > +	int				num_channels;
> > +	unsigned long			clk_rate;
> > +};
> > +
> > 
> >  struct rockchip_saradc {
> >  
> >  	void __iomem		*regs;
> >  	struct clk		*pclk;
> >  	struct clk		*clk;
> >  	struct completion	completion;
> >  	struct regulator	*vref;
> > 
> > +	const struct rockchip_saradc_data *data;
> > 
> >  	u16			last_val;
> >  
> >  };
> > 
> > @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev
> > *indio_dev,> 
> >  		}
> >  		
> >  		*val = ret / 1000;
> > 
> > -		*val2 = SARADC_BITS;
> > +		*val2 = info->data->num_bits;
> > 
> >  		return IIO_VAL_FRACTIONAL_LOG2;
> >  	
> >  	default:
> >  		return -EINVAL;
> > 
> > @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void
> > *dev_id)> 
> >  	/* Read value */
> >  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
> > 
> > -	info->last_val &= SARADC_DATA_MASK;
> > +	info->last_val &= BIT(info->data->num_bits) - 1;
> > 
> >  	/* Clear irq & power down adc */
> >  	writel_relaxed(0, info->regs + SARADC_CTRL);
> > 
> > @@ -133,12 +140,44 @@ static const struct iio_chan_spec
> > rockchip_saradc_iio_channels[] = {> 
> >  	ADC_CHANNEL(2, "adc2"),
> >  
> >  };
> > 
> > +static const struct rockchip_saradc_data saradc_data = {
> > +	.num_bits = 10,
> > +	.channels = rockchip_saradc_iio_channels,
> > +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
> > +	.clk_rate = 1000000,
> > +};
> > +
> > +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] =
> > {
> > +	ADC_CHANNEL(0, "adc0"),
> > +	ADC_CHANNEL(1, "adc1"),
> > +};
> > +
> > +static const struct rockchip_saradc_data rk3066_tsadc_data = {
> > +	.num_bits = 12,
> > +	.channels = rockchip_rk3066_tsadc_iio_channels,
> > +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
> > +	.clk_rate = 50000,
> > +};
> > +
> > +static const struct of_device_id rockchip_saradc_match[] = {
> > +	{
> > +		.compatible = "rockchip,saradc",
> > +		.data = &saradc_data,
> > +	}, {
> > +		.compatible = "rockchip,rk3066-tsadc",
> > +		.data = &rk3066_tsadc_data,
> > +	},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> > +
> > 
> >  static int rockchip_saradc_probe(struct platform_device *pdev)
> >  {
> >  
> >  	struct rockchip_saradc *info = NULL;
> >  	struct device_node *np = pdev->dev.of_node;
> >  	struct iio_dev *indio_dev = NULL;
> >  	struct resource	*mem;
> > 
> > +	const struct of_device_id *match;
> > 
> >  	int ret;
> >  	int irq;
> > 
> > @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	}
> >  	info = iio_priv(indio_dev);
> > 
> > +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
> > +	info->data = match->data;
> > +
> > 
> >  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
> >  	if (IS_ERR(info->regs))
> > 
> > @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	 * Use a default of 1MHz for the converter clock.
> >  	 * This may become user-configurable in the future.
> >  	 */
> > 
> > -	ret = clk_set_rate(info->clk, 1000000);
> > +	ret = clk_set_rate(info->clk, info->data->clk_rate);
> > 
> >  	if (ret < 0) {
> >  	
> >  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
> >  		return ret;
> > 
> > @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	indio_dev->info = &rockchip_saradc_iio_info;
> >  	indio_dev->modes = INDIO_DIRECT_MODE;
> > 
> > -	indio_dev->channels = rockchip_saradc_iio_channels;
> > -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
> > +	indio_dev->channels = info->data->channels;
> > +	indio_dev->num_channels = info->data->num_channels;
> > 
> >  	ret = iio_device_register(indio_dev);
> >  	if (ret)
> > 
> > @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
> > 
> >  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
> >  
> >  			 rockchip_saradc_suspend, rockchip_saradc_resume);
> > 
> > -static const struct of_device_id rockchip_saradc_match[] = {
> > -	{ .compatible = "rockchip,saradc" },
> > -	{},
> > -};
> > -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> > -
> > 
> >  static struct platform_driver rockchip_saradc_driver = {
> >  
> >  	.probe		= rockchip_saradc_probe,
> >  	.remove		= rockchip_saradc_remove,


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

* Re: [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-08-30 20:38     ` Heiko Stübner
  0 siblings, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2014-08-30 20:38 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Am Samstag, 30. August 2014, 21:15:53 schrieb Jonathan Cameron:
> On 30/08/14 14:07, Heiko Stübner wrote:
> > Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
> > for temperature measurements. This so called tsadc does not contain any
> > active parts like temperature interrupts and only supports polling the
> > current temperature. The returned voltage can then be converted by a
> > suitable thermal driver to and actual temperature and used for thermal
> > handling.
> > 
> > Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> 
> Looks, good to me, but will let this sit for a few days for other comments
> on it. 

ok, cool


> Long shot, but are the docs for the vairous rockchip parts
> publically available somewhere?

Not publically, at this point ... maybe somewhere in the future [hope :-) ]


Heiko

> 
> > ---
> > This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
> > As the commit messages states, the one used on the rk3066 is just a
> > saradc,
> > while the new tsadc in the rk3288 contains active components for thermal
> > handling.
> > 
> > A working example using the pending iio-thermal driver can found on
> > https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f2541766
> > 19fae36f2aad0> 
> >  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
> >  drivers/iio/adc/rockchip_saradc.c                  | 62
> >  +++++++++++++++++----- 2 files changed, 50 insertions(+), 14
> >  deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt index
> > 5d3ec1d..a9a5fe1 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > @@ -1,7 +1,7 @@
> > 
> >  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
> > 
> >  Required properties:
> > -- compatible: Should be "rockchip,saradc"
> > +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
> > 
> >  - reg: physical base address of the controller and length of memory
> >  mapped
> >  
> >         region.
> >  
> >  - interrupts: The interrupt number to the cpu. The interrupt specifier
> >  format> 
> > diff --git a/drivers/iio/adc/rockchip_saradc.c
> > b/drivers/iio/adc/rockchip_saradc.c index e074a0b..7effada 100644
> > --- a/drivers/iio/adc/rockchip_saradc.c
> > +++ b/drivers/iio/adc/rockchip_saradc.c
> > @@ -18,13 +18,13 @@
> > 
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> >  #include <linux/of.h>
> > 
> > +#include <linux/of_device.h>
> > 
> >  #include <linux/clk.h>
> >  #include <linux/completion.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/iio/iio.h>
> >  
> >  #define SARADC_DATA			0x00
> > 
> > -#define SARADC_DATA_MASK		0x3ff
> > 
> >  #define SARADC_STAS			0x04
> >  #define SARADC_STAS_BUSY		BIT(0)
> > 
> > @@ -38,15 +38,22 @@
> > 
> >  #define SARADC_DLY_PU_SOC		0x0c
> >  #define SARADC_DLY_PU_SOC_MASK		0x3f
> > 
> > -#define SARADC_BITS			10
> > 
> >  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
> > 
> > +struct rockchip_saradc_data {
> > +	int				num_bits;
> > +	const struct iio_chan_spec	*channels;
> > +	int				num_channels;
> > +	unsigned long			clk_rate;
> > +};
> > +
> > 
> >  struct rockchip_saradc {
> >  
> >  	void __iomem		*regs;
> >  	struct clk		*pclk;
> >  	struct clk		*clk;
> >  	struct completion	completion;
> >  	struct regulator	*vref;
> > 
> > +	const struct rockchip_saradc_data *data;
> > 
> >  	u16			last_val;
> >  
> >  };
> > 
> > @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev
> > *indio_dev,> 
> >  		}
> >  		
> >  		*val = ret / 1000;
> > 
> > -		*val2 = SARADC_BITS;
> > +		*val2 = info->data->num_bits;
> > 
> >  		return IIO_VAL_FRACTIONAL_LOG2;
> >  	
> >  	default:
> >  		return -EINVAL;
> > 
> > @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void
> > *dev_id)> 
> >  	/* Read value */
> >  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
> > 
> > -	info->last_val &= SARADC_DATA_MASK;
> > +	info->last_val &= BIT(info->data->num_bits) - 1;
> > 
> >  	/* Clear irq & power down adc */
> >  	writel_relaxed(0, info->regs + SARADC_CTRL);
> > 
> > @@ -133,12 +140,44 @@ static const struct iio_chan_spec
> > rockchip_saradc_iio_channels[] = {> 
> >  	ADC_CHANNEL(2, "adc2"),
> >  
> >  };
> > 
> > +static const struct rockchip_saradc_data saradc_data = {
> > +	.num_bits = 10,
> > +	.channels = rockchip_saradc_iio_channels,
> > +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
> > +	.clk_rate = 1000000,
> > +};
> > +
> > +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] =
> > {
> > +	ADC_CHANNEL(0, "adc0"),
> > +	ADC_CHANNEL(1, "adc1"),
> > +};
> > +
> > +static const struct rockchip_saradc_data rk3066_tsadc_data = {
> > +	.num_bits = 12,
> > +	.channels = rockchip_rk3066_tsadc_iio_channels,
> > +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
> > +	.clk_rate = 50000,
> > +};
> > +
> > +static const struct of_device_id rockchip_saradc_match[] = {
> > +	{
> > +		.compatible = "rockchip,saradc",
> > +		.data = &saradc_data,
> > +	}, {
> > +		.compatible = "rockchip,rk3066-tsadc",
> > +		.data = &rk3066_tsadc_data,
> > +	},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> > +
> > 
> >  static int rockchip_saradc_probe(struct platform_device *pdev)
> >  {
> >  
> >  	struct rockchip_saradc *info = NULL;
> >  	struct device_node *np = pdev->dev.of_node;
> >  	struct iio_dev *indio_dev = NULL;
> >  	struct resource	*mem;
> > 
> > +	const struct of_device_id *match;
> > 
> >  	int ret;
> >  	int irq;
> > 
> > @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	}
> >  	info = iio_priv(indio_dev);
> > 
> > +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
> > +	info->data = match->data;
> > +
> > 
> >  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
> >  	if (IS_ERR(info->regs))
> > 
> > @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	 * Use a default of 1MHz for the converter clock.
> >  	 * This may become user-configurable in the future.
> >  	 */
> > 
> > -	ret = clk_set_rate(info->clk, 1000000);
> > +	ret = clk_set_rate(info->clk, info->data->clk_rate);
> > 
> >  	if (ret < 0) {
> >  	
> >  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
> >  		return ret;
> > 
> > @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	indio_dev->info = &rockchip_saradc_iio_info;
> >  	indio_dev->modes = INDIO_DIRECT_MODE;
> > 
> > -	indio_dev->channels = rockchip_saradc_iio_channels;
> > -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
> > +	indio_dev->channels = info->data->channels;
> > +	indio_dev->num_channels = info->data->num_channels;
> > 
> >  	ret = iio_device_register(indio_dev);
> >  	if (ret)
> > 
> > @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
> > 
> >  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
> >  
> >  			 rockchip_saradc_suspend, rockchip_saradc_resume);
> > 
> > -static const struct of_device_id rockchip_saradc_match[] = {
> > -	{ .compatible = "rockchip,saradc" },
> > -	{},
> > -};
> > -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> > -
> > 
> >  static struct platform_driver rockchip_saradc_driver = {
> >  
> >  	.probe		= rockchip_saradc_probe,
> >  	.remove		= rockchip_saradc_remove,

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

* [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-08-30 20:38     ` Heiko Stübner
  0 siblings, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2014-08-30 20:38 UTC (permalink / raw)
  To: linux-arm-kernel

Am Samstag, 30. August 2014, 21:15:53 schrieb Jonathan Cameron:
> On 30/08/14 14:07, Heiko St?bner wrote:
> > Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
> > for temperature measurements. This so called tsadc does not contain any
> > active parts like temperature interrupts and only supports polling the
> > current temperature. The returned voltage can then be converted by a
> > suitable thermal driver to and actual temperature and used for thermal
> > handling.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> 
> Looks, good to me, but will let this sit for a few days for other comments
> on it. 

ok, cool


> Long shot, but are the docs for the vairous rockchip parts
> publically available somewhere?

Not publically, at this point ... maybe somewhere in the future [hope :-) ]


Heiko

> 
> > ---
> > This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
> > As the commit messages states, the one used on the rk3066 is just a
> > saradc,
> > while the new tsadc in the rk3288 contains active components for thermal
> > handling.
> > 
> > A working example using the pending iio-thermal driver can found on
> > https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f2541766
> > 19fae36f2aad0> 
> >  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
> >  drivers/iio/adc/rockchip_saradc.c                  | 62
> >  +++++++++++++++++----- 2 files changed, 50 insertions(+), 14
> >  deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt index
> > 5d3ec1d..a9a5fe1 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > @@ -1,7 +1,7 @@
> > 
> >  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
> > 
> >  Required properties:
> > -- compatible: Should be "rockchip,saradc"
> > +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
> > 
> >  - reg: physical base address of the controller and length of memory
> >  mapped
> >  
> >         region.
> >  
> >  - interrupts: The interrupt number to the cpu. The interrupt specifier
> >  format> 
> > diff --git a/drivers/iio/adc/rockchip_saradc.c
> > b/drivers/iio/adc/rockchip_saradc.c index e074a0b..7effada 100644
> > --- a/drivers/iio/adc/rockchip_saradc.c
> > +++ b/drivers/iio/adc/rockchip_saradc.c
> > @@ -18,13 +18,13 @@
> > 
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> >  #include <linux/of.h>
> > 
> > +#include <linux/of_device.h>
> > 
> >  #include <linux/clk.h>
> >  #include <linux/completion.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/iio/iio.h>
> >  
> >  #define SARADC_DATA			0x00
> > 
> > -#define SARADC_DATA_MASK		0x3ff
> > 
> >  #define SARADC_STAS			0x04
> >  #define SARADC_STAS_BUSY		BIT(0)
> > 
> > @@ -38,15 +38,22 @@
> > 
> >  #define SARADC_DLY_PU_SOC		0x0c
> >  #define SARADC_DLY_PU_SOC_MASK		0x3f
> > 
> > -#define SARADC_BITS			10
> > 
> >  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
> > 
> > +struct rockchip_saradc_data {
> > +	int				num_bits;
> > +	const struct iio_chan_spec	*channels;
> > +	int				num_channels;
> > +	unsigned long			clk_rate;
> > +};
> > +
> > 
> >  struct rockchip_saradc {
> >  
> >  	void __iomem		*regs;
> >  	struct clk		*pclk;
> >  	struct clk		*clk;
> >  	struct completion	completion;
> >  	struct regulator	*vref;
> > 
> > +	const struct rockchip_saradc_data *data;
> > 
> >  	u16			last_val;
> >  
> >  };
> > 
> > @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev
> > *indio_dev,> 
> >  		}
> >  		
> >  		*val = ret / 1000;
> > 
> > -		*val2 = SARADC_BITS;
> > +		*val2 = info->data->num_bits;
> > 
> >  		return IIO_VAL_FRACTIONAL_LOG2;
> >  	
> >  	default:
> >  		return -EINVAL;
> > 
> > @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void
> > *dev_id)> 
> >  	/* Read value */
> >  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
> > 
> > -	info->last_val &= SARADC_DATA_MASK;
> > +	info->last_val &= BIT(info->data->num_bits) - 1;
> > 
> >  	/* Clear irq & power down adc */
> >  	writel_relaxed(0, info->regs + SARADC_CTRL);
> > 
> > @@ -133,12 +140,44 @@ static const struct iio_chan_spec
> > rockchip_saradc_iio_channels[] = {> 
> >  	ADC_CHANNEL(2, "adc2"),
> >  
> >  };
> > 
> > +static const struct rockchip_saradc_data saradc_data = {
> > +	.num_bits = 10,
> > +	.channels = rockchip_saradc_iio_channels,
> > +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
> > +	.clk_rate = 1000000,
> > +};
> > +
> > +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] =
> > {
> > +	ADC_CHANNEL(0, "adc0"),
> > +	ADC_CHANNEL(1, "adc1"),
> > +};
> > +
> > +static const struct rockchip_saradc_data rk3066_tsadc_data = {
> > +	.num_bits = 12,
> > +	.channels = rockchip_rk3066_tsadc_iio_channels,
> > +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
> > +	.clk_rate = 50000,
> > +};
> > +
> > +static const struct of_device_id rockchip_saradc_match[] = {
> > +	{
> > +		.compatible = "rockchip,saradc",
> > +		.data = &saradc_data,
> > +	}, {
> > +		.compatible = "rockchip,rk3066-tsadc",
> > +		.data = &rk3066_tsadc_data,
> > +	},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> > +
> > 
> >  static int rockchip_saradc_probe(struct platform_device *pdev)
> >  {
> >  
> >  	struct rockchip_saradc *info = NULL;
> >  	struct device_node *np = pdev->dev.of_node;
> >  	struct iio_dev *indio_dev = NULL;
> >  	struct resource	*mem;
> > 
> > +	const struct of_device_id *match;
> > 
> >  	int ret;
> >  	int irq;
> > 
> > @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	}
> >  	info = iio_priv(indio_dev);
> > 
> > +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
> > +	info->data = match->data;
> > +
> > 
> >  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
> >  	if (IS_ERR(info->regs))
> > 
> > @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	 * Use a default of 1MHz for the converter clock.
> >  	 * This may become user-configurable in the future.
> >  	 */
> > 
> > -	ret = clk_set_rate(info->clk, 1000000);
> > +	ret = clk_set_rate(info->clk, info->data->clk_rate);
> > 
> >  	if (ret < 0) {
> >  	
> >  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
> >  		return ret;
> > 
> > @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	indio_dev->info = &rockchip_saradc_iio_info;
> >  	indio_dev->modes = INDIO_DIRECT_MODE;
> > 
> > -	indio_dev->channels = rockchip_saradc_iio_channels;
> > -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
> > +	indio_dev->channels = info->data->channels;
> > +	indio_dev->num_channels = info->data->num_channels;
> > 
> >  	ret = iio_device_register(indio_dev);
> >  	if (ret)
> > 
> > @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
> > 
> >  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
> >  
> >  			 rockchip_saradc_suspend, rockchip_saradc_resume);
> > 
> > -static const struct of_device_id rockchip_saradc_match[] = {
> > -	{ .compatible = "rockchip,saradc" },
> > -	{},
> > -};
> > -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> > -
> > 
> >  static struct platform_driver rockchip_saradc_driver = {
> >  
> >  	.probe		= rockchip_saradc_probe,
> >  	.remove		= rockchip_saradc_remove,

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

* Re: [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
  2014-08-30 13:07 ` Heiko Stübner
  (?)
@ 2014-09-02 22:20   ` Hartmut Knaack
  -1 siblings, 0 replies; 18+ messages in thread
From: Hartmut Knaack @ 2014-09-02 22:20 UTC (permalink / raw)
  To: Heiko Stübner, Jonathan Cameron
  Cc: linux-arm-kernel, linux-kernel, linux-iio, linux-rockchip, devicetree

Heiko Stübner schrieb:
> Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
> for temperature measurements. This so called tsadc does not contain any
> active parts like temperature interrupts and only supports polling the
> current temperature. The returned voltage can then be converted by a
> suitable thermal driver to and actual temperature and used for thermal
> handling.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
Looks mostly good to me, just some minor style issues inline.
> This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
> As the commit messages states, the one used on the rk3066 is just a saradc,
> while the new tsadc in the rk3288 contains active components for thermal
> handling.
>
> A working example using the pending iio-thermal driver can found on
> https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f254176619fae36f2aad0
>
>  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
>  drivers/iio/adc/rockchip_saradc.c                  | 62 +++++++++++++++++-----
>  2 files changed, 50 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> index 5d3ec1d..a9a5fe1 100644
> --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> @@ -1,7 +1,7 @@
>  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
>  
>  Required properties:
> -- compatible: Should be "rockchip,saradc"
> +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
>  - reg: physical base address of the controller and length of memory mapped
>         region.
>  - interrupts: The interrupt number to the cpu. The interrupt specifier format
> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> index e074a0b..7effada 100644
> --- a/drivers/iio/adc/rockchip_saradc.c
> +++ b/drivers/iio/adc/rockchip_saradc.c
> @@ -18,13 +18,13 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/iio/iio.h>
>  
>  #define SARADC_DATA			0x00
> -#define SARADC_DATA_MASK		0x3ff
>  
>  #define SARADC_STAS			0x04
>  #define SARADC_STAS_BUSY		BIT(0)
> @@ -38,15 +38,22 @@
>  #define SARADC_DLY_PU_SOC		0x0c
>  #define SARADC_DLY_PU_SOC_MASK		0x3f
>  
> -#define SARADC_BITS			10
>  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
>  
> +struct rockchip_saradc_data {
> +	int				num_bits;
> +	const struct iio_chan_spec	*channels;
> +	int				num_channels;
> +	unsigned long			clk_rate;
> +};
> +
>  struct rockchip_saradc {
>  	void __iomem		*regs;
>  	struct clk		*pclk;
>  	struct clk		*clk;
>  	struct completion	completion;
>  	struct regulator	*vref;
> +	const struct rockchip_saradc_data *data;
>  	u16			last_val;
>  };
The alignment of the whole block should be adjusted to fit with the new element.
>  
> @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>  		}
>  
>  		*val = ret / 1000;
> -		*val2 = SARADC_BITS;
> +		*val2 = info->data->num_bits;
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  	default:
>  		return -EINVAL;
> @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void *dev_id)
>  
>  	/* Read value */
>  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
> -	info->last_val &= SARADC_DATA_MASK;
> +	info->last_val &= BIT(info->data->num_bits) - 1;
I think using GENMASK would reflect the intention better, that you want to mask out your data.
>  
>  	/* Clear irq & power down adc */
>  	writel_relaxed(0, info->regs + SARADC_CTRL);
> @@ -133,12 +140,44 @@ static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
>  	ADC_CHANNEL(2, "adc2"),
>  };
>  
> +static const struct rockchip_saradc_data saradc_data = {
> +	.num_bits = 10,
> +	.channels = rockchip_saradc_iio_channels,
> +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
> +	.clk_rate = 1000000,
> +};
> +
> +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] = {
> +	ADC_CHANNEL(0, "adc0"),
> +	ADC_CHANNEL(1, "adc1"),
> +};
> +
> +static const struct rockchip_saradc_data rk3066_tsadc_data = {
> +	.num_bits = 12,
> +	.channels = rockchip_rk3066_tsadc_iio_channels,
> +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
> +	.clk_rate = 50000,
> +};
> +
> +static const struct of_device_id rockchip_saradc_match[] = {
> +	{
> +		.compatible = "rockchip,saradc",
> +		.data = &saradc_data,
> +	}, {
> +		.compatible = "rockchip,rk3066-tsadc",
> +		.data = &rk3066_tsadc_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> +
>  static int rockchip_saradc_probe(struct platform_device *pdev)
>  {
>  	struct rockchip_saradc *info = NULL;
>  	struct device_node *np = pdev->dev.of_node;
>  	struct iio_dev *indio_dev = NULL;
>  	struct resource	*mem;
> +	const struct of_device_id *match;
>  	int ret;
>  	int irq;
>  
> @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	}
>  	info = iio_priv(indio_dev);
>  
> +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
> +	info->data = match->data;
> +
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
>  	if (IS_ERR(info->regs))
> @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	 * Use a default of 1MHz for the converter clock.
>  	 * This may become user-configurable in the future.
>  	 */
> -	ret = clk_set_rate(info->clk, 1000000);
> +	ret = clk_set_rate(info->clk, info->data->clk_rate);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
>  		return ret;
> @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	indio_dev->info = &rockchip_saradc_iio_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	indio_dev->channels = rockchip_saradc_iio_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
> +	indio_dev->channels = info->data->channels;
> +	indio_dev->num_channels = info->data->num_channels;
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
>  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
>  			 rockchip_saradc_suspend, rockchip_saradc_resume);
>  
> -static const struct of_device_id rockchip_saradc_match[] = {
> -	{ .compatible = "rockchip,saradc" },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> -
>  static struct platform_driver rockchip_saradc_driver = {
>  	.probe		= rockchip_saradc_probe,
>  	.remove		= rockchip_saradc_remove,


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

* Re: [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-09-02 22:20   ` Hartmut Knaack
  0 siblings, 0 replies; 18+ messages in thread
From: Hartmut Knaack @ 2014-09-02 22:20 UTC (permalink / raw)
  To: Heiko Stübner, Jonathan Cameron
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Heiko Stübner schrieb:
> Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
> for temperature measurements. This so called tsadc does not contain any
> active parts like temperature interrupts and only supports polling the
> current temperature. The returned voltage can then be converted by a
> suitable thermal driver to and actual temperature and used for thermal
> handling.
>
> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> ---
Looks mostly good to me, just some minor style issues inline.
> This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
> As the commit messages states, the one used on the rk3066 is just a saradc,
> while the new tsadc in the rk3288 contains active components for thermal
> handling.
>
> A working example using the pending iio-thermal driver can found on
> https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f254176619fae36f2aad0
>
>  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
>  drivers/iio/adc/rockchip_saradc.c                  | 62 +++++++++++++++++-----
>  2 files changed, 50 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> index 5d3ec1d..a9a5fe1 100644
> --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> @@ -1,7 +1,7 @@
>  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
>  
>  Required properties:
> -- compatible: Should be "rockchip,saradc"
> +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
>  - reg: physical base address of the controller and length of memory mapped
>         region.
>  - interrupts: The interrupt number to the cpu. The interrupt specifier format
> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> index e074a0b..7effada 100644
> --- a/drivers/iio/adc/rockchip_saradc.c
> +++ b/drivers/iio/adc/rockchip_saradc.c
> @@ -18,13 +18,13 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/iio/iio.h>
>  
>  #define SARADC_DATA			0x00
> -#define SARADC_DATA_MASK		0x3ff
>  
>  #define SARADC_STAS			0x04
>  #define SARADC_STAS_BUSY		BIT(0)
> @@ -38,15 +38,22 @@
>  #define SARADC_DLY_PU_SOC		0x0c
>  #define SARADC_DLY_PU_SOC_MASK		0x3f
>  
> -#define SARADC_BITS			10
>  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
>  
> +struct rockchip_saradc_data {
> +	int				num_bits;
> +	const struct iio_chan_spec	*channels;
> +	int				num_channels;
> +	unsigned long			clk_rate;
> +};
> +
>  struct rockchip_saradc {
>  	void __iomem		*regs;
>  	struct clk		*pclk;
>  	struct clk		*clk;
>  	struct completion	completion;
>  	struct regulator	*vref;
> +	const struct rockchip_saradc_data *data;
>  	u16			last_val;
>  };
The alignment of the whole block should be adjusted to fit with the new element.
>  
> @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>  		}
>  
>  		*val = ret / 1000;
> -		*val2 = SARADC_BITS;
> +		*val2 = info->data->num_bits;
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  	default:
>  		return -EINVAL;
> @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void *dev_id)
>  
>  	/* Read value */
>  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
> -	info->last_val &= SARADC_DATA_MASK;
> +	info->last_val &= BIT(info->data->num_bits) - 1;
I think using GENMASK would reflect the intention better, that you want to mask out your data.
>  
>  	/* Clear irq & power down adc */
>  	writel_relaxed(0, info->regs + SARADC_CTRL);
> @@ -133,12 +140,44 @@ static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
>  	ADC_CHANNEL(2, "adc2"),
>  };
>  
> +static const struct rockchip_saradc_data saradc_data = {
> +	.num_bits = 10,
> +	.channels = rockchip_saradc_iio_channels,
> +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
> +	.clk_rate = 1000000,
> +};
> +
> +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] = {
> +	ADC_CHANNEL(0, "adc0"),
> +	ADC_CHANNEL(1, "adc1"),
> +};
> +
> +static const struct rockchip_saradc_data rk3066_tsadc_data = {
> +	.num_bits = 12,
> +	.channels = rockchip_rk3066_tsadc_iio_channels,
> +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
> +	.clk_rate = 50000,
> +};
> +
> +static const struct of_device_id rockchip_saradc_match[] = {
> +	{
> +		.compatible = "rockchip,saradc",
> +		.data = &saradc_data,
> +	}, {
> +		.compatible = "rockchip,rk3066-tsadc",
> +		.data = &rk3066_tsadc_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> +
>  static int rockchip_saradc_probe(struct platform_device *pdev)
>  {
>  	struct rockchip_saradc *info = NULL;
>  	struct device_node *np = pdev->dev.of_node;
>  	struct iio_dev *indio_dev = NULL;
>  	struct resource	*mem;
> +	const struct of_device_id *match;
>  	int ret;
>  	int irq;
>  
> @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	}
>  	info = iio_priv(indio_dev);
>  
> +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
> +	info->data = match->data;
> +
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
>  	if (IS_ERR(info->regs))
> @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	 * Use a default of 1MHz for the converter clock.
>  	 * This may become user-configurable in the future.
>  	 */
> -	ret = clk_set_rate(info->clk, 1000000);
> +	ret = clk_set_rate(info->clk, info->data->clk_rate);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
>  		return ret;
> @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	indio_dev->info = &rockchip_saradc_iio_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	indio_dev->channels = rockchip_saradc_iio_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
> +	indio_dev->channels = info->data->channels;
> +	indio_dev->num_channels = info->data->num_channels;
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
>  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
>  			 rockchip_saradc_suspend, rockchip_saradc_resume);
>  
> -static const struct of_device_id rockchip_saradc_match[] = {
> -	{ .compatible = "rockchip,saradc" },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> -
>  static struct platform_driver rockchip_saradc_driver = {
>  	.probe		= rockchip_saradc_probe,
>  	.remove		= rockchip_saradc_remove,

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

* [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-09-02 22:20   ` Hartmut Knaack
  0 siblings, 0 replies; 18+ messages in thread
From: Hartmut Knaack @ 2014-09-02 22:20 UTC (permalink / raw)
  To: linux-arm-kernel

Heiko St?bner schrieb:
> Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
> for temperature measurements. This so called tsadc does not contain any
> active parts like temperature interrupts and only supports polling the
> current temperature. The returned voltage can then be converted by a
> suitable thermal driver to and actual temperature and used for thermal
> handling.
>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
Looks mostly good to me, just some minor style issues inline.
> This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
> As the commit messages states, the one used on the rk3066 is just a saradc,
> while the new tsadc in the rk3288 contains active components for thermal
> handling.
>
> A working example using the pending iio-thermal driver can found on
> https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f254176619fae36f2aad0
>
>  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
>  drivers/iio/adc/rockchip_saradc.c                  | 62 +++++++++++++++++-----
>  2 files changed, 50 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> index 5d3ec1d..a9a5fe1 100644
> --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> @@ -1,7 +1,7 @@
>  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
>  
>  Required properties:
> -- compatible: Should be "rockchip,saradc"
> +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
>  - reg: physical base address of the controller and length of memory mapped
>         region.
>  - interrupts: The interrupt number to the cpu. The interrupt specifier format
> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> index e074a0b..7effada 100644
> --- a/drivers/iio/adc/rockchip_saradc.c
> +++ b/drivers/iio/adc/rockchip_saradc.c
> @@ -18,13 +18,13 @@
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/iio/iio.h>
>  
>  #define SARADC_DATA			0x00
> -#define SARADC_DATA_MASK		0x3ff
>  
>  #define SARADC_STAS			0x04
>  #define SARADC_STAS_BUSY		BIT(0)
> @@ -38,15 +38,22 @@
>  #define SARADC_DLY_PU_SOC		0x0c
>  #define SARADC_DLY_PU_SOC_MASK		0x3f
>  
> -#define SARADC_BITS			10
>  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
>  
> +struct rockchip_saradc_data {
> +	int				num_bits;
> +	const struct iio_chan_spec	*channels;
> +	int				num_channels;
> +	unsigned long			clk_rate;
> +};
> +
>  struct rockchip_saradc {
>  	void __iomem		*regs;
>  	struct clk		*pclk;
>  	struct clk		*clk;
>  	struct completion	completion;
>  	struct regulator	*vref;
> +	const struct rockchip_saradc_data *data;
>  	u16			last_val;
>  };
The alignment of the whole block should be adjusted to fit with the new element.
>  
> @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>  		}
>  
>  		*val = ret / 1000;
> -		*val2 = SARADC_BITS;
> +		*val2 = info->data->num_bits;
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  	default:
>  		return -EINVAL;
> @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void *dev_id)
>  
>  	/* Read value */
>  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
> -	info->last_val &= SARADC_DATA_MASK;
> +	info->last_val &= BIT(info->data->num_bits) - 1;
I think using GENMASK would reflect the intention better, that you want to mask out your data.
>  
>  	/* Clear irq & power down adc */
>  	writel_relaxed(0, info->regs + SARADC_CTRL);
> @@ -133,12 +140,44 @@ static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
>  	ADC_CHANNEL(2, "adc2"),
>  };
>  
> +static const struct rockchip_saradc_data saradc_data = {
> +	.num_bits = 10,
> +	.channels = rockchip_saradc_iio_channels,
> +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
> +	.clk_rate = 1000000,
> +};
> +
> +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] = {
> +	ADC_CHANNEL(0, "adc0"),
> +	ADC_CHANNEL(1, "adc1"),
> +};
> +
> +static const struct rockchip_saradc_data rk3066_tsadc_data = {
> +	.num_bits = 12,
> +	.channels = rockchip_rk3066_tsadc_iio_channels,
> +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
> +	.clk_rate = 50000,
> +};
> +
> +static const struct of_device_id rockchip_saradc_match[] = {
> +	{
> +		.compatible = "rockchip,saradc",
> +		.data = &saradc_data,
> +	}, {
> +		.compatible = "rockchip,rk3066-tsadc",
> +		.data = &rk3066_tsadc_data,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> +
>  static int rockchip_saradc_probe(struct platform_device *pdev)
>  {
>  	struct rockchip_saradc *info = NULL;
>  	struct device_node *np = pdev->dev.of_node;
>  	struct iio_dev *indio_dev = NULL;
>  	struct resource	*mem;
> +	const struct of_device_id *match;
>  	int ret;
>  	int irq;
>  
> @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	}
>  	info = iio_priv(indio_dev);
>  
> +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
> +	info->data = match->data;
> +
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
>  	if (IS_ERR(info->regs))
> @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	 * Use a default of 1MHz for the converter clock.
>  	 * This may become user-configurable in the future.
>  	 */
> -	ret = clk_set_rate(info->clk, 1000000);
> +	ret = clk_set_rate(info->clk, info->data->clk_rate);
>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
>  		return ret;
> @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>  	indio_dev->info = &rockchip_saradc_iio_info;
>  	indio_dev->modes = INDIO_DIRECT_MODE;
>  
> -	indio_dev->channels = rockchip_saradc_iio_channels;
> -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
> +	indio_dev->channels = info->data->channels;
> +	indio_dev->num_channels = info->data->num_channels;
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret)
> @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
>  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
>  			 rockchip_saradc_suspend, rockchip_saradc_resume);
>  
> -static const struct of_device_id rockchip_saradc_match[] = {
> -	{ .compatible = "rockchip,saradc" },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> -
>  static struct platform_driver rockchip_saradc_driver = {
>  	.probe		= rockchip_saradc_probe,
>  	.remove		= rockchip_saradc_remove,

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

* Re: [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
  2014-09-02 22:20   ` Hartmut Knaack
  (?)
@ 2014-09-03 10:13     ` Heiko Stübner
  -1 siblings, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2014-09-03 10:13 UTC (permalink / raw)
  To: Hartmut Knaack, Jonathan Cameron
  Cc: linux-arm-kernel, linux-kernel, linux-iio, linux-rockchip, devicetree

Am Mittwoch, 3. September 2014, 00:20:58 schrieb Hartmut Knaack:
> Heiko Stübner schrieb:
> > Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
> > for temperature measurements. This so called tsadc does not contain any
> > active parts like temperature interrupts and only supports polling the
> > current temperature. The returned voltage can then be converted by a
> > suitable thermal driver to and actual temperature and used for thermal
> > handling.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> 
> Looks mostly good to me, just some minor style issues inline.
> 
> > This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
> > As the commit messages states, the one used on the rk3066 is just a
> > saradc,
> > while the new tsadc in the rk3288 contains active components for thermal
> > handling.
> > 
> > A working example using the pending iio-thermal driver can found on
> > https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f2541766
> > 19fae36f2aad0> 
> >  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
> >  drivers/iio/adc/rockchip_saradc.c                  | 62
> >  +++++++++++++++++----- 2 files changed, 50 insertions(+), 14
> >  deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt index
> > 5d3ec1d..a9a5fe1 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > @@ -1,7 +1,7 @@
> > 
> >  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
> > 
> >  Required properties:
> > -- compatible: Should be "rockchip,saradc"
> > +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
> > 
> >  - reg: physical base address of the controller and length of memory
> >  mapped
> >  
> >         region.
> >  
> >  - interrupts: The interrupt number to the cpu. The interrupt specifier
> >  format> 
> > diff --git a/drivers/iio/adc/rockchip_saradc.c
> > b/drivers/iio/adc/rockchip_saradc.c index e074a0b..7effada 100644
> > --- a/drivers/iio/adc/rockchip_saradc.c
> > +++ b/drivers/iio/adc/rockchip_saradc.c
> > @@ -18,13 +18,13 @@
> > 
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> >  #include <linux/of.h>
> > 
> > +#include <linux/of_device.h>
> > 
> >  #include <linux/clk.h>
> >  #include <linux/completion.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/iio/iio.h>
> >  
> >  #define SARADC_DATA			0x00
> > 
> > -#define SARADC_DATA_MASK		0x3ff
> > 
> >  #define SARADC_STAS			0x04
> >  #define SARADC_STAS_BUSY		BIT(0)
> > 
> > @@ -38,15 +38,22 @@
> > 
> >  #define SARADC_DLY_PU_SOC		0x0c
> >  #define SARADC_DLY_PU_SOC_MASK		0x3f
> > 
> > -#define SARADC_BITS			10
> > 
> >  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
> > 
> > +struct rockchip_saradc_data {
> > +	int				num_bits;
> > +	const struct iio_chan_spec	*channels;
> > +	int				num_channels;
> > +	unsigned long			clk_rate;
> > +};
> > +
> > 
> >  struct rockchip_saradc {
> >  
> >  	void __iomem		*regs;
> >  	struct clk		*pclk;
> >  	struct clk		*clk;
> >  	struct completion	completion;
> >  	struct regulator	*vref;
> > 
> > +	const struct rockchip_saradc_data *data;
> > 
> >  	u16			last_val;
> >  
> >  };
> 
> The alignment of the whole block should be adjusted to fit with the new
> element.

Hmm, I'm not sure if this should be in this patch, as changing the indentation 
is more or less an unrelated change which makes recognizing the actual change 
of adding the data field harder to see. 

It could be a second patch on top to adapt the formating, but this also sounds 
a bit like overkill.

In any case I guess I'll wait for Jonathan to decide how he likes this to be 
approached.


> > @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev
> > *indio_dev,> 
> >  		}
> >  		
> >  		*val = ret / 1000;
> > 
> > -		*val2 = SARADC_BITS;
> > +		*val2 = info->data->num_bits;
> > 
> >  		return IIO_VAL_FRACTIONAL_LOG2;
> >  	
> >  	default:
> >  		return -EINVAL;
> > 
> > @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void
> > *dev_id)> 
> >  	/* Read value */
> >  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
> > 
> > -	info->last_val &= SARADC_DATA_MASK;
> > +	info->last_val &= BIT(info->data->num_bits) - 1;
> 
> I think using GENMASK would reflect the intention better, that you want to
> mask out your data.

I didn't know GENMASK before, so thanks for the pointer .. this looks quite 
cool :-)


> >  	/* Clear irq & power down adc */
> >  	writel_relaxed(0, info->regs + SARADC_CTRL);
> > 
> > @@ -133,12 +140,44 @@ static const struct iio_chan_spec
> > rockchip_saradc_iio_channels[] = {> 
> >  	ADC_CHANNEL(2, "adc2"),
> >  
> >  };
> > 
> > +static const struct rockchip_saradc_data saradc_data = {
> > +	.num_bits = 10,
> > +	.channels = rockchip_saradc_iio_channels,
> > +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
> > +	.clk_rate = 1000000,
> > +};
> > +
> > +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] =
> > {
> > +	ADC_CHANNEL(0, "adc0"),
> > +	ADC_CHANNEL(1, "adc1"),
> > +};
> > +
> > +static const struct rockchip_saradc_data rk3066_tsadc_data = {
> > +	.num_bits = 12,
> > +	.channels = rockchip_rk3066_tsadc_iio_channels,
> > +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
> > +	.clk_rate = 50000,
> > +};
> > +
> > +static const struct of_device_id rockchip_saradc_match[] = {
> > +	{
> > +		.compatible = "rockchip,saradc",
> > +		.data = &saradc_data,
> > +	}, {
> > +		.compatible = "rockchip,rk3066-tsadc",
> > +		.data = &rk3066_tsadc_data,
> > +	},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> > +
> > 
> >  static int rockchip_saradc_probe(struct platform_device *pdev)
> >  {
> >  
> >  	struct rockchip_saradc *info = NULL;
> >  	struct device_node *np = pdev->dev.of_node;
> >  	struct iio_dev *indio_dev = NULL;
> >  	struct resource	*mem;
> > 
> > +	const struct of_device_id *match;
> > 
> >  	int ret;
> >  	int irq;
> > 
> > @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	}
> >  	info = iio_priv(indio_dev);
> > 
> > +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
> > +	info->data = match->data;
> > +
> > 
> >  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
> >  	if (IS_ERR(info->regs))
> > 
> > @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	 * Use a default of 1MHz for the converter clock.
> >  	 * This may become user-configurable in the future.
> >  	 */
> > 
> > -	ret = clk_set_rate(info->clk, 1000000);
> > +	ret = clk_set_rate(info->clk, info->data->clk_rate);
> > 
> >  	if (ret < 0) {
> >  	
> >  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
> >  		return ret;
> > 
> > @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	indio_dev->info = &rockchip_saradc_iio_info;
> >  	indio_dev->modes = INDIO_DIRECT_MODE;
> > 
> > -	indio_dev->channels = rockchip_saradc_iio_channels;
> > -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
> > +	indio_dev->channels = info->data->channels;
> > +	indio_dev->num_channels = info->data->num_channels;
> > 
> >  	ret = iio_device_register(indio_dev);
> >  	if (ret)
> > 
> > @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
> > 
> >  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
> >  
> >  			 rockchip_saradc_suspend, rockchip_saradc_resume);
> > 
> > -static const struct of_device_id rockchip_saradc_match[] = {
> > -	{ .compatible = "rockchip,saradc" },
> > -	{},
> > -};
> > -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> > -
> > 
> >  static struct platform_driver rockchip_saradc_driver = {
> >  
> >  	.probe		= rockchip_saradc_probe,
> >  	.remove		= rockchip_saradc_remove,

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

* Re: [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-09-03 10:13     ` Heiko Stübner
  0 siblings, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2014-09-03 10:13 UTC (permalink / raw)
  To: Hartmut Knaack, Jonathan Cameron
  Cc: linux-iio, linux-rockchip, linux-kernel, linux-arm-kernel, devicetree

Am Mittwoch, 3. September 2014, 00:20:58 schrieb Hartmut Knaack:
> Heiko Stübner schrieb:
> > Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
> > for temperature measurements. This so called tsadc does not contain any
> > active parts like temperature interrupts and only supports polling the
> > current temperature. The returned voltage can then be converted by a
> > suitable thermal driver to and actual temperature and used for thermal
> > handling.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> 
> Looks mostly good to me, just some minor style issues inline.
> 
> > This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
> > As the commit messages states, the one used on the rk3066 is just a
> > saradc,
> > while the new tsadc in the rk3288 contains active components for thermal
> > handling.
> > 
> > A working example using the pending iio-thermal driver can found on
> > https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f2541766
> > 19fae36f2aad0> 
> >  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
> >  drivers/iio/adc/rockchip_saradc.c                  | 62
> >  +++++++++++++++++----- 2 files changed, 50 insertions(+), 14
> >  deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt index
> > 5d3ec1d..a9a5fe1 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > @@ -1,7 +1,7 @@
> > 
> >  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
> > 
> >  Required properties:
> > -- compatible: Should be "rockchip,saradc"
> > +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
> > 
> >  - reg: physical base address of the controller and length of memory
> >  mapped
> >  
> >         region.
> >  
> >  - interrupts: The interrupt number to the cpu. The interrupt specifier
> >  format> 
> > diff --git a/drivers/iio/adc/rockchip_saradc.c
> > b/drivers/iio/adc/rockchip_saradc.c index e074a0b..7effada 100644
> > --- a/drivers/iio/adc/rockchip_saradc.c
> > +++ b/drivers/iio/adc/rockchip_saradc.c
> > @@ -18,13 +18,13 @@
> > 
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> >  #include <linux/of.h>
> > 
> > +#include <linux/of_device.h>
> > 
> >  #include <linux/clk.h>
> >  #include <linux/completion.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/iio/iio.h>
> >  
> >  #define SARADC_DATA			0x00
> > 
> > -#define SARADC_DATA_MASK		0x3ff
> > 
> >  #define SARADC_STAS			0x04
> >  #define SARADC_STAS_BUSY		BIT(0)
> > 
> > @@ -38,15 +38,22 @@
> > 
> >  #define SARADC_DLY_PU_SOC		0x0c
> >  #define SARADC_DLY_PU_SOC_MASK		0x3f
> > 
> > -#define SARADC_BITS			10
> > 
> >  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
> > 
> > +struct rockchip_saradc_data {
> > +	int				num_bits;
> > +	const struct iio_chan_spec	*channels;
> > +	int				num_channels;
> > +	unsigned long			clk_rate;
> > +};
> > +
> > 
> >  struct rockchip_saradc {
> >  
> >  	void __iomem		*regs;
> >  	struct clk		*pclk;
> >  	struct clk		*clk;
> >  	struct completion	completion;
> >  	struct regulator	*vref;
> > 
> > +	const struct rockchip_saradc_data *data;
> > 
> >  	u16			last_val;
> >  
> >  };
> 
> The alignment of the whole block should be adjusted to fit with the new
> element.

Hmm, I'm not sure if this should be in this patch, as changing the indentation 
is more or less an unrelated change which makes recognizing the actual change 
of adding the data field harder to see. 

It could be a second patch on top to adapt the formating, but this also sounds 
a bit like overkill.

In any case I guess I'll wait for Jonathan to decide how he likes this to be 
approached.


> > @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev
> > *indio_dev,> 
> >  		}
> >  		
> >  		*val = ret / 1000;
> > 
> > -		*val2 = SARADC_BITS;
> > +		*val2 = info->data->num_bits;
> > 
> >  		return IIO_VAL_FRACTIONAL_LOG2;
> >  	
> >  	default:
> >  		return -EINVAL;
> > 
> > @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void
> > *dev_id)> 
> >  	/* Read value */
> >  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
> > 
> > -	info->last_val &= SARADC_DATA_MASK;
> > +	info->last_val &= BIT(info->data->num_bits) - 1;
> 
> I think using GENMASK would reflect the intention better, that you want to
> mask out your data.

I didn't know GENMASK before, so thanks for the pointer .. this looks quite 
cool :-)


> >  	/* Clear irq & power down adc */
> >  	writel_relaxed(0, info->regs + SARADC_CTRL);
> > 
> > @@ -133,12 +140,44 @@ static const struct iio_chan_spec
> > rockchip_saradc_iio_channels[] = {> 
> >  	ADC_CHANNEL(2, "adc2"),
> >  
> >  };
> > 
> > +static const struct rockchip_saradc_data saradc_data = {
> > +	.num_bits = 10,
> > +	.channels = rockchip_saradc_iio_channels,
> > +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
> > +	.clk_rate = 1000000,
> > +};
> > +
> > +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] =
> > {
> > +	ADC_CHANNEL(0, "adc0"),
> > +	ADC_CHANNEL(1, "adc1"),
> > +};
> > +
> > +static const struct rockchip_saradc_data rk3066_tsadc_data = {
> > +	.num_bits = 12,
> > +	.channels = rockchip_rk3066_tsadc_iio_channels,
> > +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
> > +	.clk_rate = 50000,
> > +};
> > +
> > +static const struct of_device_id rockchip_saradc_match[] = {
> > +	{
> > +		.compatible = "rockchip,saradc",
> > +		.data = &saradc_data,
> > +	}, {
> > +		.compatible = "rockchip,rk3066-tsadc",
> > +		.data = &rk3066_tsadc_data,
> > +	},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> > +
> > 
> >  static int rockchip_saradc_probe(struct platform_device *pdev)
> >  {
> >  
> >  	struct rockchip_saradc *info = NULL;
> >  	struct device_node *np = pdev->dev.of_node;
> >  	struct iio_dev *indio_dev = NULL;
> >  	struct resource	*mem;
> > 
> > +	const struct of_device_id *match;
> > 
> >  	int ret;
> >  	int irq;
> > 
> > @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	}
> >  	info = iio_priv(indio_dev);
> > 
> > +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
> > +	info->data = match->data;
> > +
> > 
> >  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
> >  	if (IS_ERR(info->regs))
> > 
> > @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	 * Use a default of 1MHz for the converter clock.
> >  	 * This may become user-configurable in the future.
> >  	 */
> > 
> > -	ret = clk_set_rate(info->clk, 1000000);
> > +	ret = clk_set_rate(info->clk, info->data->clk_rate);
> > 
> >  	if (ret < 0) {
> >  	
> >  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
> >  		return ret;
> > 
> > @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	indio_dev->info = &rockchip_saradc_iio_info;
> >  	indio_dev->modes = INDIO_DIRECT_MODE;
> > 
> > -	indio_dev->channels = rockchip_saradc_iio_channels;
> > -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
> > +	indio_dev->channels = info->data->channels;
> > +	indio_dev->num_channels = info->data->num_channels;
> > 
> >  	ret = iio_device_register(indio_dev);
> >  	if (ret)
> > 
> > @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
> > 
> >  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
> >  
> >  			 rockchip_saradc_suspend, rockchip_saradc_resume);
> > 
> > -static const struct of_device_id rockchip_saradc_match[] = {
> > -	{ .compatible = "rockchip,saradc" },
> > -	{},
> > -};
> > -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> > -
> > 
> >  static struct platform_driver rockchip_saradc_driver = {
> >  
> >  	.probe		= rockchip_saradc_probe,
> >  	.remove		= rockchip_saradc_remove,

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

* [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-09-03 10:13     ` Heiko Stübner
  0 siblings, 0 replies; 18+ messages in thread
From: Heiko Stübner @ 2014-09-03 10:13 UTC (permalink / raw)
  To: linux-arm-kernel

Am Mittwoch, 3. September 2014, 00:20:58 schrieb Hartmut Knaack:
> Heiko St?bner schrieb:
> > Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
> > for temperature measurements. This so called tsadc does not contain any
> > active parts like temperature interrupts and only supports polling the
> > current temperature. The returned voltage can then be converted by a
> > suitable thermal driver to and actual temperature and used for thermal
> > handling.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> 
> Looks mostly good to me, just some minor style issues inline.
> 
> > This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
> > As the commit messages states, the one used on the rk3066 is just a
> > saradc,
> > while the new tsadc in the rk3288 contains active components for thermal
> > handling.
> > 
> > A working example using the pending iio-thermal driver can found on
> > https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f2541766
> > 19fae36f2aad0> 
> >  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
> >  drivers/iio/adc/rockchip_saradc.c                  | 62
> >  +++++++++++++++++----- 2 files changed, 50 insertions(+), 14
> >  deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt index
> > 5d3ec1d..a9a5fe1 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
> > @@ -1,7 +1,7 @@
> > 
> >  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
> > 
> >  Required properties:
> > -- compatible: Should be "rockchip,saradc"
> > +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
> > 
> >  - reg: physical base address of the controller and length of memory
> >  mapped
> >  
> >         region.
> >  
> >  - interrupts: The interrupt number to the cpu. The interrupt specifier
> >  format> 
> > diff --git a/drivers/iio/adc/rockchip_saradc.c
> > b/drivers/iio/adc/rockchip_saradc.c index e074a0b..7effada 100644
> > --- a/drivers/iio/adc/rockchip_saradc.c
> > +++ b/drivers/iio/adc/rockchip_saradc.c
> > @@ -18,13 +18,13 @@
> > 
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> >  #include <linux/of.h>
> > 
> > +#include <linux/of_device.h>
> > 
> >  #include <linux/clk.h>
> >  #include <linux/completion.h>
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/iio/iio.h>
> >  
> >  #define SARADC_DATA			0x00
> > 
> > -#define SARADC_DATA_MASK		0x3ff
> > 
> >  #define SARADC_STAS			0x04
> >  #define SARADC_STAS_BUSY		BIT(0)
> > 
> > @@ -38,15 +38,22 @@
> > 
> >  #define SARADC_DLY_PU_SOC		0x0c
> >  #define SARADC_DLY_PU_SOC_MASK		0x3f
> > 
> > -#define SARADC_BITS			10
> > 
> >  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
> > 
> > +struct rockchip_saradc_data {
> > +	int				num_bits;
> > +	const struct iio_chan_spec	*channels;
> > +	int				num_channels;
> > +	unsigned long			clk_rate;
> > +};
> > +
> > 
> >  struct rockchip_saradc {
> >  
> >  	void __iomem		*regs;
> >  	struct clk		*pclk;
> >  	struct clk		*clk;
> >  	struct completion	completion;
> >  	struct regulator	*vref;
> > 
> > +	const struct rockchip_saradc_data *data;
> > 
> >  	u16			last_val;
> >  
> >  };
> 
> The alignment of the whole block should be adjusted to fit with the new
> element.

Hmm, I'm not sure if this should be in this patch, as changing the indentation 
is more or less an unrelated change which makes recognizing the actual change 
of adding the data field harder to see. 

It could be a second patch on top to adapt the formating, but this also sounds 
a bit like overkill.

In any case I guess I'll wait for Jonathan to decide how he likes this to be 
approached.


> > @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev
> > *indio_dev,> 
> >  		}
> >  		
> >  		*val = ret / 1000;
> > 
> > -		*val2 = SARADC_BITS;
> > +		*val2 = info->data->num_bits;
> > 
> >  		return IIO_VAL_FRACTIONAL_LOG2;
> >  	
> >  	default:
> >  		return -EINVAL;
> > 
> > @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void
> > *dev_id)> 
> >  	/* Read value */
> >  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
> > 
> > -	info->last_val &= SARADC_DATA_MASK;
> > +	info->last_val &= BIT(info->data->num_bits) - 1;
> 
> I think using GENMASK would reflect the intention better, that you want to
> mask out your data.

I didn't know GENMASK before, so thanks for the pointer .. this looks quite 
cool :-)


> >  	/* Clear irq & power down adc */
> >  	writel_relaxed(0, info->regs + SARADC_CTRL);
> > 
> > @@ -133,12 +140,44 @@ static const struct iio_chan_spec
> > rockchip_saradc_iio_channels[] = {> 
> >  	ADC_CHANNEL(2, "adc2"),
> >  
> >  };
> > 
> > +static const struct rockchip_saradc_data saradc_data = {
> > +	.num_bits = 10,
> > +	.channels = rockchip_saradc_iio_channels,
> > +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
> > +	.clk_rate = 1000000,
> > +};
> > +
> > +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] =
> > {
> > +	ADC_CHANNEL(0, "adc0"),
> > +	ADC_CHANNEL(1, "adc1"),
> > +};
> > +
> > +static const struct rockchip_saradc_data rk3066_tsadc_data = {
> > +	.num_bits = 12,
> > +	.channels = rockchip_rk3066_tsadc_iio_channels,
> > +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
> > +	.clk_rate = 50000,
> > +};
> > +
> > +static const struct of_device_id rockchip_saradc_match[] = {
> > +	{
> > +		.compatible = "rockchip,saradc",
> > +		.data = &saradc_data,
> > +	}, {
> > +		.compatible = "rockchip,rk3066-tsadc",
> > +		.data = &rk3066_tsadc_data,
> > +	},
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> > +
> > 
> >  static int rockchip_saradc_probe(struct platform_device *pdev)
> >  {
> >  
> >  	struct rockchip_saradc *info = NULL;
> >  	struct device_node *np = pdev->dev.of_node;
> >  	struct iio_dev *indio_dev = NULL;
> >  	struct resource	*mem;
> > 
> > +	const struct of_device_id *match;
> > 
> >  	int ret;
> >  	int irq;
> > 
> > @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	}
> >  	info = iio_priv(indio_dev);
> > 
> > +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
> > +	info->data = match->data;
> > +
> > 
> >  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
> >  	if (IS_ERR(info->regs))
> > 
> > @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	 * Use a default of 1MHz for the converter clock.
> >  	 * This may become user-configurable in the future.
> >  	 */
> > 
> > -	ret = clk_set_rate(info->clk, 1000000);
> > +	ret = clk_set_rate(info->clk, info->data->clk_rate);
> > 
> >  	if (ret < 0) {
> >  	
> >  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
> >  		return ret;
> > 
> > @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct
> > platform_device *pdev)> 
> >  	indio_dev->info = &rockchip_saradc_iio_info;
> >  	indio_dev->modes = INDIO_DIRECT_MODE;
> > 
> > -	indio_dev->channels = rockchip_saradc_iio_channels;
> > -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
> > +	indio_dev->channels = info->data->channels;
> > +	indio_dev->num_channels = info->data->num_channels;
> > 
> >  	ret = iio_device_register(indio_dev);
> >  	if (ret)
> > 
> > @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
> > 
> >  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
> >  
> >  			 rockchip_saradc_suspend, rockchip_saradc_resume);
> > 
> > -static const struct of_device_id rockchip_saradc_match[] = {
> > -	{ .compatible = "rockchip,saradc" },
> > -	{},
> > -};
> > -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
> > -
> > 
> >  static struct platform_driver rockchip_saradc_driver = {
> >  
> >  	.probe		= rockchip_saradc_probe,
> >  	.remove		= rockchip_saradc_remove,

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

* Re: [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
  2014-09-03 10:13     ` Heiko Stübner
  (?)
@ 2014-09-10 19:05       ` Jonathan Cameron
  -1 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2014-09-10 19:05 UTC (permalink / raw)
  To: Heiko Stübner, Hartmut Knaack
  Cc: linux-arm-kernel, linux-kernel, linux-iio, linux-rockchip, devicetree

On 03/09/14 11:13, Heiko Stübner wrote:
> Am Mittwoch, 3. September 2014, 00:20:58 schrieb Hartmut Knaack:
>> Heiko Stübner schrieb:
>>> Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
>>> for temperature measurements. This so called tsadc does not contain any
>>> active parts like temperature interrupts and only supports polling the
>>> current temperature. The returned voltage can then be converted by a
>>> suitable thermal driver to and actual temperature and used for thermal
>>> handling.
>>>
>>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>> ---
>>
>> Looks mostly good to me, just some minor style issues inline.
>>
>>> This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
>>> As the commit messages states, the one used on the rk3066 is just a
>>> saradc,
>>> while the new tsadc in the rk3288 contains active components for thermal
>>> handling.
>>>
>>> A working example using the pending iio-thermal driver can found on
>>> https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f2541766
>>> 19fae36f2aad0> 
>>>  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
>>>  drivers/iio/adc/rockchip_saradc.c                  | 62
>>>  +++++++++++++++++----- 2 files changed, 50 insertions(+), 14
>>>  deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
>>> b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt index
>>> 5d3ec1d..a9a5fe1 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
>>> +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
>>> @@ -1,7 +1,7 @@
>>>
>>>  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
>>>
>>>  Required properties:
>>> -- compatible: Should be "rockchip,saradc"
>>> +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
>>>
>>>  - reg: physical base address of the controller and length of memory
>>>  mapped
>>>  
>>>         region.
>>>  
>>>  - interrupts: The interrupt number to the cpu. The interrupt specifier
>>>  format> 
>>> diff --git a/drivers/iio/adc/rockchip_saradc.c
>>> b/drivers/iio/adc/rockchip_saradc.c index e074a0b..7effada 100644
>>> --- a/drivers/iio/adc/rockchip_saradc.c
>>> +++ b/drivers/iio/adc/rockchip_saradc.c
>>> @@ -18,13 +18,13 @@
>>>
>>>  #include <linux/interrupt.h>
>>>  #include <linux/io.h>
>>>  #include <linux/of.h>
>>>
>>> +#include <linux/of_device.h>
>>>
>>>  #include <linux/clk.h>
>>>  #include <linux/completion.h>
>>>  #include <linux/regulator/consumer.h>
>>>  #include <linux/iio/iio.h>
>>>  
>>>  #define SARADC_DATA			0x00
>>>
>>> -#define SARADC_DATA_MASK		0x3ff
>>>
>>>  #define SARADC_STAS			0x04
>>>  #define SARADC_STAS_BUSY		BIT(0)
>>>
>>> @@ -38,15 +38,22 @@
>>>
>>>  #define SARADC_DLY_PU_SOC		0x0c
>>>  #define SARADC_DLY_PU_SOC_MASK		0x3f
>>>
>>> -#define SARADC_BITS			10
>>>
>>>  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
>>>
>>> +struct rockchip_saradc_data {
>>> +	int				num_bits;
>>> +	const struct iio_chan_spec	*channels;
>>> +	int				num_channels;
>>> +	unsigned long			clk_rate;
>>> +};
>>> +
>>>
>>>  struct rockchip_saradc {
>>>  
>>>  	void __iomem		*regs;
>>>  	struct clk		*pclk;
>>>  	struct clk		*clk;
>>>  	struct completion	completion;
>>>  	struct regulator	*vref;
>>>
>>> +	const struct rockchip_saradc_data *data;
>>>
>>>  	u16			last_val;
>>>  
>>>  };
>>
>> The alignment of the whole block should be adjusted to fit with the new
>> element.
> 
> Hmm, I'm not sure if this should be in this patch, as changing the indentation 
> is more or less an unrelated change which makes recognizing the actual change 
> of adding the data field harder to see. 
> 
> It could be a second patch on top to adapt the formating, but this also sounds 
> a bit like overkill.
> 
> In any case I guess I'll wait for Jonathan to decide how he likes this to be 
> approached.
> 
Personally I hate this sort of careful block alignment for structures precisely because
it generates churn.  I just don't hate it enough to moan about it in reviews :)

Definitely doesn't want to be in this patch, but feel free to submit another
either tidying up or dropping it in favour of not bothering to align it.


> 
>>> @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev
>>> *indio_dev,> 
>>>  		}
>>>  		
>>>  		*val = ret / 1000;
>>>
>>> -		*val2 = SARADC_BITS;
>>> +		*val2 = info->data->num_bits;
>>>
>>>  		return IIO_VAL_FRACTIONAL_LOG2;
>>>  	
>>>  	default:
>>>  		return -EINVAL;
>>>
>>> @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void
>>> *dev_id)> 
>>>  	/* Read value */
>>>  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
>>>
>>> -	info->last_val &= SARADC_DATA_MASK;
>>> +	info->last_val &= BIT(info->data->num_bits) - 1;
>>
>> I think using GENMASK would reflect the intention better, that you want to
>> mask out your data.
> 
> I didn't know GENMASK before, so thanks for the pointer .. this looks quite 
> cool :-)
> 
> 
>>>  	/* Clear irq & power down adc */
>>>  	writel_relaxed(0, info->regs + SARADC_CTRL);
>>>
>>> @@ -133,12 +140,44 @@ static const struct iio_chan_spec
>>> rockchip_saradc_iio_channels[] = {> 
>>>  	ADC_CHANNEL(2, "adc2"),
>>>  
>>>  };
>>>
>>> +static const struct rockchip_saradc_data saradc_data = {
>>> +	.num_bits = 10,
>>> +	.channels = rockchip_saradc_iio_channels,
>>> +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
>>> +	.clk_rate = 1000000,
>>> +};
>>> +
>>> +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] =
>>> {
>>> +	ADC_CHANNEL(0, "adc0"),
>>> +	ADC_CHANNEL(1, "adc1"),
>>> +};
>>> +
>>> +static const struct rockchip_saradc_data rk3066_tsadc_data = {
>>> +	.num_bits = 12,
>>> +	.channels = rockchip_rk3066_tsadc_iio_channels,
>>> +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
>>> +	.clk_rate = 50000,
>>> +};
>>> +
>>> +static const struct of_device_id rockchip_saradc_match[] = {
>>> +	{
>>> +		.compatible = "rockchip,saradc",
>>> +		.data = &saradc_data,
>>> +	}, {
>>> +		.compatible = "rockchip,rk3066-tsadc",
>>> +		.data = &rk3066_tsadc_data,
>>> +	},
>>> +	{},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
>>> +
>>>
>>>  static int rockchip_saradc_probe(struct platform_device *pdev)
>>>  {
>>>  
>>>  	struct rockchip_saradc *info = NULL;
>>>  	struct device_node *np = pdev->dev.of_node;
>>>  	struct iio_dev *indio_dev = NULL;
>>>  	struct resource	*mem;
>>>
>>> +	const struct of_device_id *match;
>>>
>>>  	int ret;
>>>  	int irq;
>>>
>>> @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct
>>> platform_device *pdev)> 
>>>  	}
>>>  	info = iio_priv(indio_dev);
>>>
>>> +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
>>> +	info->data = match->data;
>>> +
>>>
>>>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
>>>  	if (IS_ERR(info->regs))
>>>
>>> @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct
>>> platform_device *pdev)> 
>>>  	 * Use a default of 1MHz for the converter clock.
>>>  	 * This may become user-configurable in the future.
>>>  	 */
>>>
>>> -	ret = clk_set_rate(info->clk, 1000000);
>>> +	ret = clk_set_rate(info->clk, info->data->clk_rate);
>>>
>>>  	if (ret < 0) {
>>>  	
>>>  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
>>>  		return ret;
>>>
>>> @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct
>>> platform_device *pdev)> 
>>>  	indio_dev->info = &rockchip_saradc_iio_info;
>>>  	indio_dev->modes = INDIO_DIRECT_MODE;
>>>
>>> -	indio_dev->channels = rockchip_saradc_iio_channels;
>>> -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
>>> +	indio_dev->channels = info->data->channels;
>>> +	indio_dev->num_channels = info->data->num_channels;
>>>
>>>  	ret = iio_device_register(indio_dev);
>>>  	if (ret)
>>>
>>> @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
>>>
>>>  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
>>>  
>>>  			 rockchip_saradc_suspend, rockchip_saradc_resume);
>>>
>>> -static const struct of_device_id rockchip_saradc_match[] = {
>>> -	{ .compatible = "rockchip,saradc" },
>>> -	{},
>>> -};
>>> -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
>>> -
>>>
>>>  static struct platform_driver rockchip_saradc_driver = {
>>>  
>>>  	.probe		= rockchip_saradc_probe,
>>>  	.remove		= rockchip_saradc_remove,
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-09-10 19:05       ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2014-09-10 19:05 UTC (permalink / raw)
  To: Heiko Stübner, Hartmut Knaack
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 03/09/14 11:13, Heiko Stübner wrote:
> Am Mittwoch, 3. September 2014, 00:20:58 schrieb Hartmut Knaack:
>> Heiko Stübner schrieb:
>>> Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
>>> for temperature measurements. This so called tsadc does not contain any
>>> active parts like temperature interrupts and only supports polling the
>>> current temperature. The returned voltage can then be converted by a
>>> suitable thermal driver to and actual temperature and used for thermal
>>> handling.
>>>
>>> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
>>> ---
>>
>> Looks mostly good to me, just some minor style issues inline.
>>
>>> This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
>>> As the commit messages states, the one used on the rk3066 is just a
>>> saradc,
>>> while the new tsadc in the rk3288 contains active components for thermal
>>> handling.
>>>
>>> A working example using the pending iio-thermal driver can found on
>>> https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f2541766
>>> 19fae36f2aad0> 
>>>  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
>>>  drivers/iio/adc/rockchip_saradc.c                  | 62
>>>  +++++++++++++++++----- 2 files changed, 50 insertions(+), 14
>>>  deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
>>> b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt index
>>> 5d3ec1d..a9a5fe1 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
>>> +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
>>> @@ -1,7 +1,7 @@
>>>
>>>  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
>>>
>>>  Required properties:
>>> -- compatible: Should be "rockchip,saradc"
>>> +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
>>>
>>>  - reg: physical base address of the controller and length of memory
>>>  mapped
>>>  
>>>         region.
>>>  
>>>  - interrupts: The interrupt number to the cpu. The interrupt specifier
>>>  format> 
>>> diff --git a/drivers/iio/adc/rockchip_saradc.c
>>> b/drivers/iio/adc/rockchip_saradc.c index e074a0b..7effada 100644
>>> --- a/drivers/iio/adc/rockchip_saradc.c
>>> +++ b/drivers/iio/adc/rockchip_saradc.c
>>> @@ -18,13 +18,13 @@
>>>
>>>  #include <linux/interrupt.h>
>>>  #include <linux/io.h>
>>>  #include <linux/of.h>
>>>
>>> +#include <linux/of_device.h>
>>>
>>>  #include <linux/clk.h>
>>>  #include <linux/completion.h>
>>>  #include <linux/regulator/consumer.h>
>>>  #include <linux/iio/iio.h>
>>>  
>>>  #define SARADC_DATA			0x00
>>>
>>> -#define SARADC_DATA_MASK		0x3ff
>>>
>>>  #define SARADC_STAS			0x04
>>>  #define SARADC_STAS_BUSY		BIT(0)
>>>
>>> @@ -38,15 +38,22 @@
>>>
>>>  #define SARADC_DLY_PU_SOC		0x0c
>>>  #define SARADC_DLY_PU_SOC_MASK		0x3f
>>>
>>> -#define SARADC_BITS			10
>>>
>>>  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
>>>
>>> +struct rockchip_saradc_data {
>>> +	int				num_bits;
>>> +	const struct iio_chan_spec	*channels;
>>> +	int				num_channels;
>>> +	unsigned long			clk_rate;
>>> +};
>>> +
>>>
>>>  struct rockchip_saradc {
>>>  
>>>  	void __iomem		*regs;
>>>  	struct clk		*pclk;
>>>  	struct clk		*clk;
>>>  	struct completion	completion;
>>>  	struct regulator	*vref;
>>>
>>> +	const struct rockchip_saradc_data *data;
>>>
>>>  	u16			last_val;
>>>  
>>>  };
>>
>> The alignment of the whole block should be adjusted to fit with the new
>> element.
> 
> Hmm, I'm not sure if this should be in this patch, as changing the indentation 
> is more or less an unrelated change which makes recognizing the actual change 
> of adding the data field harder to see. 
> 
> It could be a second patch on top to adapt the formating, but this also sounds 
> a bit like overkill.
> 
> In any case I guess I'll wait for Jonathan to decide how he likes this to be 
> approached.
> 
Personally I hate this sort of careful block alignment for structures precisely because
it generates churn.  I just don't hate it enough to moan about it in reviews :)

Definitely doesn't want to be in this patch, but feel free to submit another
either tidying up or dropping it in favour of not bothering to align it.


> 
>>> @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev
>>> *indio_dev,> 
>>>  		}
>>>  		
>>>  		*val = ret / 1000;
>>>
>>> -		*val2 = SARADC_BITS;
>>> +		*val2 = info->data->num_bits;
>>>
>>>  		return IIO_VAL_FRACTIONAL_LOG2;
>>>  	
>>>  	default:
>>>  		return -EINVAL;
>>>
>>> @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void
>>> *dev_id)> 
>>>  	/* Read value */
>>>  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
>>>
>>> -	info->last_val &= SARADC_DATA_MASK;
>>> +	info->last_val &= BIT(info->data->num_bits) - 1;
>>
>> I think using GENMASK would reflect the intention better, that you want to
>> mask out your data.
> 
> I didn't know GENMASK before, so thanks for the pointer .. this looks quite 
> cool :-)
> 
> 
>>>  	/* Clear irq & power down adc */
>>>  	writel_relaxed(0, info->regs + SARADC_CTRL);
>>>
>>> @@ -133,12 +140,44 @@ static const struct iio_chan_spec
>>> rockchip_saradc_iio_channels[] = {> 
>>>  	ADC_CHANNEL(2, "adc2"),
>>>  
>>>  };
>>>
>>> +static const struct rockchip_saradc_data saradc_data = {
>>> +	.num_bits = 10,
>>> +	.channels = rockchip_saradc_iio_channels,
>>> +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
>>> +	.clk_rate = 1000000,
>>> +};
>>> +
>>> +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] =
>>> {
>>> +	ADC_CHANNEL(0, "adc0"),
>>> +	ADC_CHANNEL(1, "adc1"),
>>> +};
>>> +
>>> +static const struct rockchip_saradc_data rk3066_tsadc_data = {
>>> +	.num_bits = 12,
>>> +	.channels = rockchip_rk3066_tsadc_iio_channels,
>>> +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
>>> +	.clk_rate = 50000,
>>> +};
>>> +
>>> +static const struct of_device_id rockchip_saradc_match[] = {
>>> +	{
>>> +		.compatible = "rockchip,saradc",
>>> +		.data = &saradc_data,
>>> +	}, {
>>> +		.compatible = "rockchip,rk3066-tsadc",
>>> +		.data = &rk3066_tsadc_data,
>>> +	},
>>> +	{},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
>>> +
>>>
>>>  static int rockchip_saradc_probe(struct platform_device *pdev)
>>>  {
>>>  
>>>  	struct rockchip_saradc *info = NULL;
>>>  	struct device_node *np = pdev->dev.of_node;
>>>  	struct iio_dev *indio_dev = NULL;
>>>  	struct resource	*mem;
>>>
>>> +	const struct of_device_id *match;
>>>
>>>  	int ret;
>>>  	int irq;
>>>
>>> @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct
>>> platform_device *pdev)> 
>>>  	}
>>>  	info = iio_priv(indio_dev);
>>>
>>> +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
>>> +	info->data = match->data;
>>> +
>>>
>>>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
>>>  	if (IS_ERR(info->regs))
>>>
>>> @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct
>>> platform_device *pdev)> 
>>>  	 * Use a default of 1MHz for the converter clock.
>>>  	 * This may become user-configurable in the future.
>>>  	 */
>>>
>>> -	ret = clk_set_rate(info->clk, 1000000);
>>> +	ret = clk_set_rate(info->clk, info->data->clk_rate);
>>>
>>>  	if (ret < 0) {
>>>  	
>>>  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
>>>  		return ret;
>>>
>>> @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct
>>> platform_device *pdev)> 
>>>  	indio_dev->info = &rockchip_saradc_iio_info;
>>>  	indio_dev->modes = INDIO_DIRECT_MODE;
>>>
>>> -	indio_dev->channels = rockchip_saradc_iio_channels;
>>> -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
>>> +	indio_dev->channels = info->data->channels;
>>> +	indio_dev->num_channels = info->data->num_channels;
>>>
>>>  	ret = iio_device_register(indio_dev);
>>>  	if (ret)
>>>
>>> @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
>>>
>>>  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
>>>  
>>>  			 rockchip_saradc_suspend, rockchip_saradc_resume);
>>>
>>> -static const struct of_device_id rockchip_saradc_match[] = {
>>> -	{ .compatible = "rockchip,saradc" },
>>> -	{},
>>> -};
>>> -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
>>> -
>>>
>>>  static struct platform_driver rockchip_saradc_driver = {
>>>  
>>>  	.probe		= rockchip_saradc_probe,
>>>  	.remove		= rockchip_saradc_remove,
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant
@ 2014-09-10 19:05       ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2014-09-10 19:05 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/09/14 11:13, Heiko St?bner wrote:
> Am Mittwoch, 3. September 2014, 00:20:58 schrieb Hartmut Knaack:
>> Heiko St?bner schrieb:
>>> Older Rockchip SoCs, at least the rk3066, used a slightly modified saradc
>>> for temperature measurements. This so called tsadc does not contain any
>>> active parts like temperature interrupts and only supports polling the
>>> current temperature. The returned voltage can then be converted by a
>>> suitable thermal driver to and actual temperature and used for thermal
>>> handling.
>>>
>>> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
>>> ---
>>
>> Looks mostly good to me, just some minor style issues inline.
>>
>>> This is a different IP than the rk3288-tsadc Ceasar provided a driver for.
>>> As the commit messages states, the one used on the rk3066 is just a
>>> saradc,
>>> while the new tsadc in the rk3288 contains active components for thermal
>>> handling.
>>>
>>> A working example using the pending iio-thermal driver can found on
>>> https://github.com/mmind/linux-rockchip/commit/c07e02f069d9677d489f2541766
>>> 19fae36f2aad0> 
>>>  .../bindings/iio/adc/rockchip-saradc.txt           |  2 +-
>>>  drivers/iio/adc/rockchip_saradc.c                  | 62
>>>  +++++++++++++++++----- 2 files changed, 50 insertions(+), 14
>>>  deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
>>> b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt index
>>> 5d3ec1d..a9a5fe1 100644
>>> --- a/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
>>> +++ b/Documentation/devicetree/bindings/iio/adc/rockchip-saradc.txt
>>> @@ -1,7 +1,7 @@
>>>
>>>  Rockchip Successive Approximation Register (SAR) A/D Converter bindings
>>>
>>>  Required properties:
>>> -- compatible: Should be "rockchip,saradc"
>>> +- compatible: Should be "rockchip,saradc" or "rockchip,rk3066-tsadc"
>>>
>>>  - reg: physical base address of the controller and length of memory
>>>  mapped
>>>  
>>>         region.
>>>  
>>>  - interrupts: The interrupt number to the cpu. The interrupt specifier
>>>  format> 
>>> diff --git a/drivers/iio/adc/rockchip_saradc.c
>>> b/drivers/iio/adc/rockchip_saradc.c index e074a0b..7effada 100644
>>> --- a/drivers/iio/adc/rockchip_saradc.c
>>> +++ b/drivers/iio/adc/rockchip_saradc.c
>>> @@ -18,13 +18,13 @@
>>>
>>>  #include <linux/interrupt.h>
>>>  #include <linux/io.h>
>>>  #include <linux/of.h>
>>>
>>> +#include <linux/of_device.h>
>>>
>>>  #include <linux/clk.h>
>>>  #include <linux/completion.h>
>>>  #include <linux/regulator/consumer.h>
>>>  #include <linux/iio/iio.h>
>>>  
>>>  #define SARADC_DATA			0x00
>>>
>>> -#define SARADC_DATA_MASK		0x3ff
>>>
>>>  #define SARADC_STAS			0x04
>>>  #define SARADC_STAS_BUSY		BIT(0)
>>>
>>> @@ -38,15 +38,22 @@
>>>
>>>  #define SARADC_DLY_PU_SOC		0x0c
>>>  #define SARADC_DLY_PU_SOC_MASK		0x3f
>>>
>>> -#define SARADC_BITS			10
>>>
>>>  #define SARADC_TIMEOUT			msecs_to_jiffies(100)
>>>
>>> +struct rockchip_saradc_data {
>>> +	int				num_bits;
>>> +	const struct iio_chan_spec	*channels;
>>> +	int				num_channels;
>>> +	unsigned long			clk_rate;
>>> +};
>>> +
>>>
>>>  struct rockchip_saradc {
>>>  
>>>  	void __iomem		*regs;
>>>  	struct clk		*pclk;
>>>  	struct clk		*clk;
>>>  	struct completion	completion;
>>>  	struct regulator	*vref;
>>>
>>> +	const struct rockchip_saradc_data *data;
>>>
>>>  	u16			last_val;
>>>  
>>>  };
>>
>> The alignment of the whole block should be adjusted to fit with the new
>> element.
> 
> Hmm, I'm not sure if this should be in this patch, as changing the indentation 
> is more or less an unrelated change which makes recognizing the actual change 
> of adding the data field harder to see. 
> 
> It could be a second patch on top to adapt the formating, but this also sounds 
> a bit like overkill.
> 
> In any case I guess I'll wait for Jonathan to decide how he likes this to be 
> approached.
> 
Personally I hate this sort of careful block alignment for structures precisely because
it generates churn.  I just don't hate it enough to moan about it in reviews :)

Definitely doesn't want to be in this patch, but feel free to submit another
either tidying up or dropping it in favour of not bothering to align it.


> 
>>> @@ -90,7 +97,7 @@ static int rockchip_saradc_read_raw(struct iio_dev
>>> *indio_dev,> 
>>>  		}
>>>  		
>>>  		*val = ret / 1000;
>>>
>>> -		*val2 = SARADC_BITS;
>>> +		*val2 = info->data->num_bits;
>>>
>>>  		return IIO_VAL_FRACTIONAL_LOG2;
>>>  	
>>>  	default:
>>>  		return -EINVAL;
>>>
>>> @@ -103,7 +110,7 @@ static irqreturn_t rockchip_saradc_isr(int irq, void
>>> *dev_id)> 
>>>  	/* Read value */
>>>  	info->last_val = readl_relaxed(info->regs + SARADC_DATA);
>>>
>>> -	info->last_val &= SARADC_DATA_MASK;
>>> +	info->last_val &= BIT(info->data->num_bits) - 1;
>>
>> I think using GENMASK would reflect the intention better, that you want to
>> mask out your data.
> 
> I didn't know GENMASK before, so thanks for the pointer .. this looks quite 
> cool :-)
> 
> 
>>>  	/* Clear irq & power down adc */
>>>  	writel_relaxed(0, info->regs + SARADC_CTRL);
>>>
>>> @@ -133,12 +140,44 @@ static const struct iio_chan_spec
>>> rockchip_saradc_iio_channels[] = {> 
>>>  	ADC_CHANNEL(2, "adc2"),
>>>  
>>>  };
>>>
>>> +static const struct rockchip_saradc_data saradc_data = {
>>> +	.num_bits = 10,
>>> +	.channels = rockchip_saradc_iio_channels,
>>> +	.num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels),
>>> +	.clk_rate = 1000000,
>>> +};
>>> +
>>> +static const struct iio_chan_spec rockchip_rk3066_tsadc_iio_channels[] =
>>> {
>>> +	ADC_CHANNEL(0, "adc0"),
>>> +	ADC_CHANNEL(1, "adc1"),
>>> +};
>>> +
>>> +static const struct rockchip_saradc_data rk3066_tsadc_data = {
>>> +	.num_bits = 12,
>>> +	.channels = rockchip_rk3066_tsadc_iio_channels,
>>> +	.num_channels = ARRAY_SIZE(rockchip_rk3066_tsadc_iio_channels),
>>> +	.clk_rate = 50000,
>>> +};
>>> +
>>> +static const struct of_device_id rockchip_saradc_match[] = {
>>> +	{
>>> +		.compatible = "rockchip,saradc",
>>> +		.data = &saradc_data,
>>> +	}, {
>>> +		.compatible = "rockchip,rk3066-tsadc",
>>> +		.data = &rk3066_tsadc_data,
>>> +	},
>>> +	{},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
>>> +
>>>
>>>  static int rockchip_saradc_probe(struct platform_device *pdev)
>>>  {
>>>  
>>>  	struct rockchip_saradc *info = NULL;
>>>  	struct device_node *np = pdev->dev.of_node;
>>>  	struct iio_dev *indio_dev = NULL;
>>>  	struct resource	*mem;
>>>
>>> +	const struct of_device_id *match;
>>>
>>>  	int ret;
>>>  	int irq;
>>>
>>> @@ -152,6 +191,9 @@ static int rockchip_saradc_probe(struct
>>> platform_device *pdev)> 
>>>  	}
>>>  	info = iio_priv(indio_dev);
>>>
>>> +	match = of_match_device(rockchip_saradc_match, &pdev->dev);
>>> +	info->data = match->data;
>>> +
>>>
>>>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>>  	info->regs = devm_ioremap_resource(&pdev->dev, mem);
>>>  	if (IS_ERR(info->regs))
>>>
>>> @@ -195,7 +237,7 @@ static int rockchip_saradc_probe(struct
>>> platform_device *pdev)> 
>>>  	 * Use a default of 1MHz for the converter clock.
>>>  	 * This may become user-configurable in the future.
>>>  	 */
>>>
>>> -	ret = clk_set_rate(info->clk, 1000000);
>>> +	ret = clk_set_rate(info->clk, info->data->clk_rate);
>>>
>>>  	if (ret < 0) {
>>>  	
>>>  		dev_err(&pdev->dev, "failed to set adc clk rate, %d\n", ret);
>>>  		return ret;
>>>
>>> @@ -227,8 +269,8 @@ static int rockchip_saradc_probe(struct
>>> platform_device *pdev)> 
>>>  	indio_dev->info = &rockchip_saradc_iio_info;
>>>  	indio_dev->modes = INDIO_DIRECT_MODE;
>>>
>>> -	indio_dev->channels = rockchip_saradc_iio_channels;
>>> -	indio_dev->num_channels = ARRAY_SIZE(rockchip_saradc_iio_channels);
>>> +	indio_dev->channels = info->data->channels;
>>> +	indio_dev->num_channels = info->data->num_channels;
>>>
>>>  	ret = iio_device_register(indio_dev);
>>>  	if (ret)
>>>
>>> @@ -296,12 +338,6 @@ static int rockchip_saradc_resume(struct device *dev)
>>>
>>>  static SIMPLE_DEV_PM_OPS(rockchip_saradc_pm_ops,
>>>  
>>>  			 rockchip_saradc_suspend, rockchip_saradc_resume);
>>>
>>> -static const struct of_device_id rockchip_saradc_match[] = {
>>> -	{ .compatible = "rockchip,saradc" },
>>> -	{},
>>> -};
>>> -MODULE_DEVICE_TABLE(of, rockchip_saradc_match);
>>> -
>>>
>>>  static struct platform_driver rockchip_saradc_driver = {
>>>  
>>>  	.probe		= rockchip_saradc_probe,
>>>  	.remove		= rockchip_saradc_remove,
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

end of thread, other threads:[~2014-09-10 19:06 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-30 13:07 [PATCH] iio: adc: rockchip_saradc: add support for rk3066-tsadc variant Heiko Stübner
2014-08-30 13:07 ` Heiko Stübner
2014-08-30 13:07 ` Heiko Stübner
2014-08-30 20:15 ` Jonathan Cameron
2014-08-30 20:15   ` Jonathan Cameron
2014-08-30 20:15   ` Jonathan Cameron
2014-08-30 20:38   ` Heiko Stübner
2014-08-30 20:38     ` Heiko Stübner
2014-08-30 20:38     ` Heiko Stübner
2014-09-02 22:20 ` Hartmut Knaack
2014-09-02 22:20   ` Hartmut Knaack
2014-09-02 22:20   ` Hartmut Knaack
2014-09-03 10:13   ` Heiko Stübner
2014-09-03 10:13     ` Heiko Stübner
2014-09-03 10:13     ` Heiko Stübner
2014-09-10 19:05     ` Jonathan Cameron
2014-09-10 19:05       ` Jonathan Cameron
2014-09-10 19:05       ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.