All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iio: adc: rockchip_saradc: Add support iio buffers
@ 2020-03-01 11:23 Heiko Stuebner
  2020-03-02  2:11   ` xxm
  0 siblings, 1 reply; 8+ messages in thread
From: Heiko Stuebner @ 2020-03-01 11:23 UTC (permalink / raw)
  To: jic23
  Cc: knaack.h, lars, pmeerw, heiko, linux-iio, linux-kernel,
	linux-rockchip, xxm, kever.yang, Heiko Stuebner

From: Simon Xue <xxm@rock-chips.com>

Add the ability to also support access via (triggered) buffers
next to the existing direct mode.

Device in question is the Odroid Go Advance that connects a joystick
to two of the saradc channels for X and Y axis and the new (and still
pending) adc joystick driver of course wants to use triggered buffers
from the iio subsystem.

Signed-off-by: Simon Xue <xxm@rock-chips.com>
[some simplifications and added commit description]
Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
---
 drivers/iio/adc/Kconfig           |   2 +
 drivers/iio/adc/rockchip_saradc.c | 137 ++++++++++++++++++++++--------
 2 files changed, 102 insertions(+), 37 deletions(-)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 82e33082958c..55d2499ff757 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -787,6 +787,8 @@ config ROCKCHIP_SARADC
 	tristate "Rockchip SARADC driver"
 	depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
 	depends on RESET_CONTROLLER
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
 	help
 	  Say yes here to build support for the SARADC found in SoCs from
 	  Rockchip.
diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
index 582ba047c4a6..402b2210a682 100644
--- a/drivers/iio/adc/rockchip_saradc.c
+++ b/drivers/iio/adc/rockchip_saradc.c
@@ -15,7 +15,11 @@
 #include <linux/delay.h>
 #include <linux/reset.h>
 #include <linux/regulator/consumer.h>
+#include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
 
 #define SARADC_DATA			0x00
 
@@ -34,7 +38,6 @@
 #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;
@@ -49,8 +52,37 @@ struct rockchip_saradc {
 	struct reset_control	*reset;
 	const struct rockchip_saradc_data *data;
 	u16			last_val;
+	const struct iio_chan_spec *last_chan;
 };
 
+static void rockchip_saradc_power_down(struct rockchip_saradc *info)
+{
+	/* Clear irq & power down adc */
+	writel_relaxed(0, info->regs + SARADC_CTRL);
+}
+
+static int rockchip_saradc_conversion(struct rockchip_saradc *info,
+				   struct iio_chan_spec const *chan)
+{
+	reinit_completion(&info->completion);
+
+	/* 8 clock periods as delay between power up and start cmd */
+	writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
+
+	info->last_chan = chan;
+
+	/* Select the channel to be used and trigger conversion */
+	writel(SARADC_CTRL_POWER_CTRL
+			| (chan->channel & SARADC_CTRL_CHN_MASK)
+			| SARADC_CTRL_IRQ_ENABLE,
+		   info->regs + SARADC_CTRL);
+
+	if (!wait_for_completion_timeout(&info->completion, SARADC_TIMEOUT))
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
 static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
 				    struct iio_chan_spec const *chan,
 				    int *val, int *val2, long mask)
@@ -62,24 +94,12 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		mutex_lock(&indio_dev->mlock);
 
-		reinit_completion(&info->completion);
-
-		/* 8 clock periods as delay between power up and start cmd */
-		writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
-
-		/* Select the channel to be used and trigger conversion */
-		writel(SARADC_CTRL_POWER_CTRL
-				| (chan->channel & SARADC_CTRL_CHN_MASK)
-				| SARADC_CTRL_IRQ_ENABLE,
-		       info->regs + SARADC_CTRL);
-
-		if (!wait_for_completion_timeout(&info->completion,
-						 SARADC_TIMEOUT)) {
-			writel_relaxed(0, info->regs + SARADC_CTRL);
+		ret = rockchip_saradc_conversion(info, chan);
+		if (ret) {
+			rockchip_saradc_power_down(info);
 			mutex_unlock(&indio_dev->mlock);
-			return -ETIMEDOUT;
+			return ret;
 		}
-
 		*val = info->last_val;
 		mutex_unlock(&indio_dev->mlock);
 		return IIO_VAL_INT;
@@ -91,7 +111,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
 		}
 
 		*val = ret / 1000;
-		*val2 = info->data->num_bits;
+		*val2 = chan->scan_type.realbits;
 		return IIO_VAL_FRACTIONAL_LOG2;
 	default:
 		return -EINVAL;
@@ -104,10 +124,9 @@ 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 &= GENMASK(info->data->num_bits - 1, 0);
+	info->last_val &= GENMASK(info->last_chan->scan_type.realbits - 1, 0);
 
-	/* Clear irq & power down adc */
-	writel_relaxed(0, info->regs + SARADC_CTRL);
+	rockchip_saradc_power_down(info);
 
 	complete(&info->completion);
 
@@ -118,51 +137,55 @@ static const struct iio_info rockchip_saradc_iio_info = {
 	.read_raw = rockchip_saradc_read_raw,
 };
 
-#define ADC_CHANNEL(_index, _id) {				\
+#define ADC_CHANNEL(_index, _id, _res) {			\
 	.type = IIO_VOLTAGE,					\
 	.indexed = 1,						\
 	.channel = _index,					\
 	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
 	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
 	.datasheet_name = _id,					\
+	.scan_index = _index,					\
+	.scan_type = {						\
+		.sign = 'u',					\
+		.realbits = _res,				\
+		.storagebits = 16,				\
+		.endianness = IIO_LE,				\
+	},							\
 }
 
 static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
-	ADC_CHANNEL(0, "adc0"),
-	ADC_CHANNEL(1, "adc1"),
-	ADC_CHANNEL(2, "adc2"),
+	ADC_CHANNEL(0, "adc0", 10),
+	ADC_CHANNEL(1, "adc1", 10),
+	ADC_CHANNEL(2, "adc2", 10),
 };
 
 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"),
+	ADC_CHANNEL(0, "adc0", 12),
+	ADC_CHANNEL(1, "adc1", 12),
 };
 
 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 iio_chan_spec rockchip_rk3399_saradc_iio_channels[] = {
-	ADC_CHANNEL(0, "adc0"),
-	ADC_CHANNEL(1, "adc1"),
-	ADC_CHANNEL(2, "adc2"),
-	ADC_CHANNEL(3, "adc3"),
-	ADC_CHANNEL(4, "adc4"),
-	ADC_CHANNEL(5, "adc5"),
+	ADC_CHANNEL(0, "adc0", 10),
+	ADC_CHANNEL(1, "adc1", 10),
+	ADC_CHANNEL(2, "adc2", 10),
+	ADC_CHANNEL(3, "adc3", 10),
+	ADC_CHANNEL(4, "adc4", 10),
+	ADC_CHANNEL(5, "adc5", 10),
 };
 
 static const struct rockchip_saradc_data rk3399_saradc_data = {
-	.num_bits = 10,
 	.channels = rockchip_rk3399_saradc_iio_channels,
 	.num_channels = ARRAY_SIZE(rockchip_rk3399_saradc_iio_channels),
 	.clk_rate = 1000000,
@@ -193,6 +216,39 @@ static void rockchip_saradc_reset_controller(struct reset_control *reset)
 	reset_control_deassert(reset);
 }
 
+static irqreturn_t rockchip_saradc_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *i_dev = pf->indio_dev;
+	struct rockchip_saradc *info = iio_priv(i_dev);
+	u16 data[20];
+	int ret;
+	int i, j = 0;
+
+	mutex_lock(&i_dev->mlock);
+
+	for_each_set_bit(i, i_dev->active_scan_mask, i_dev->masklength) {
+		const struct iio_chan_spec *chan = &i_dev->channels[i];
+
+		ret = rockchip_saradc_conversion(info, chan);
+		if (ret) {
+			rockchip_saradc_power_down(info);
+			goto out;
+		}
+
+		data[j] = info->last_val;
+		j++;
+	}
+
+	iio_push_to_buffers_with_timestamp(i_dev, data, iio_get_time_ns(i_dev));
+out:
+	mutex_unlock(&i_dev->mlock);
+
+	iio_trigger_notify_done(i_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
 static int rockchip_saradc_probe(struct platform_device *pdev)
 {
 	struct rockchip_saradc *info = NULL;
@@ -315,12 +371,19 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
 	indio_dev->channels = info->data->channels;
 	indio_dev->num_channels = info->data->num_channels;
 
-	ret = iio_device_register(indio_dev);
+	ret = iio_triggered_buffer_setup(indio_dev, NULL,
+					 rockchip_saradc_trigger_handler, NULL);
 	if (ret)
 		goto err_clk;
 
+	ret = iio_device_register(indio_dev);
+	if (ret)
+		goto err_buffer_cleanup;
+
 	return 0;
 
+err_buffer_cleanup:
+	iio_triggered_buffer_cleanup(indio_dev);
 err_clk:
 	clk_disable_unprepare(info->clk);
 err_pclk:
-- 
2.24.1


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

* Re: [PATCH] iio: adc: rockchip_saradc: Add support iio buffers【请注意,邮件由linux-rockchip-bounces+xxm=rock-chips.com@lists.infradead.org代发】
@ 2020-03-02  2:11   ` xxm
  0 siblings, 0 replies; 8+ messages in thread
From: xxm @ 2020-03-02  2:11 UTC (permalink / raw)
  To: Heiko Stuebner, jic23
  Cc: lars, linux-iio, Heiko Stuebner, linux-kernel, kever.yang,
	linux-rockchip, pmeerw, knaack.h

Hi, Heiko

在 2020/3/1 19:23, Heiko Stuebner 写道:
> From: Simon Xue <xxm@rock-chips.com>
> 
> Add the ability to also support access via (triggered) buffers
> next to the existing direct mode.
> 
> Device in question is the Odroid Go Advance that connects a joystick
> to two of the saradc channels for X and Y axis and the new (and still
> pending) adc joystick driver of course wants to use triggered buffers
> from the iio subsystem.
> 
> Signed-off-by: Simon Xue <xxm@rock-chips.com>
> [some simplifications and added commit description]
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
>   drivers/iio/adc/Kconfig           |   2 +
>   drivers/iio/adc/rockchip_saradc.c | 137 ++++++++++++++++++++++--------
>   2 files changed, 102 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 82e33082958c..55d2499ff757 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -787,6 +787,8 @@ config ROCKCHIP_SARADC
>   	tristate "Rockchip SARADC driver"
>   	depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
>   	depends on RESET_CONTROLLER
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
>   	help
>   	  Say yes here to build support for the SARADC found in SoCs from
>   	  Rockchip.
> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> index 582ba047c4a6..402b2210a682 100644
> --- a/drivers/iio/adc/rockchip_saradc.c
> +++ b/drivers/iio/adc/rockchip_saradc.c
> @@ -15,7 +15,11 @@
>   #include <linux/delay.h>
>   #include <linux/reset.h>
>   #include <linux/regulator/consumer.h>
> +#include <linux/iio/buffer.h>
>   #include <linux/iio/iio.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
>   
>   #define SARADC_DATA			0x00
>   
> @@ -34,7 +38,6 @@
>   #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;
> @@ -49,8 +52,37 @@ struct rockchip_saradc {
>   	struct reset_control	*reset;
>   	const struct rockchip_saradc_data *data;
>   	u16			last_val;
> +	const struct iio_chan_spec *last_chan;
>   };
>   
> +static void rockchip_saradc_power_down(struct rockchip_saradc *info)
> +{
> +	/* Clear irq & power down adc */
> +	writel_relaxed(0, info->regs + SARADC_CTRL);
> +}
> +
> +static int rockchip_saradc_conversion(struct rockchip_saradc *info,
> +				   struct iio_chan_spec const *chan)
> +{
> +	reinit_completion(&info->completion);
> +
> +	/* 8 clock periods as delay between power up and start cmd */
> +	writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
> +
> +	info->last_chan = chan;
> +
> +	/* Select the channel to be used and trigger conversion */
> +	writel(SARADC_CTRL_POWER_CTRL
> +			| (chan->channel & SARADC_CTRL_CHN_MASK)
> +			| SARADC_CTRL_IRQ_ENABLE,
> +		   info->regs + SARADC_CTRL);
> +
> +	if (!wait_for_completion_timeout(&info->completion, SARADC_TIMEOUT))
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
>   static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>   				    struct iio_chan_spec const *chan,
>   				    int *val, int *val2, long mask)
> @@ -62,24 +94,12 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>   	case IIO_CHAN_INFO_RAW:
>   		mutex_lock(&indio_dev->mlock);
>   
> -		reinit_completion(&info->completion);
> -
> -		/* 8 clock periods as delay between power up and start cmd */
> -		writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
> -
> -		/* Select the channel to be used and trigger conversion */
> -		writel(SARADC_CTRL_POWER_CTRL
> -				| (chan->channel & SARADC_CTRL_CHN_MASK)
> -				| SARADC_CTRL_IRQ_ENABLE,
> -		       info->regs + SARADC_CTRL);
> -
> -		if (!wait_for_completion_timeout(&info->completion,
> -						 SARADC_TIMEOUT)) {
> -			writel_relaxed(0, info->regs + SARADC_CTRL);
> +		ret = rockchip_saradc_conversion(info, chan);
> +		if (ret) {
> +			rockchip_saradc_power_down(info);
>   			mutex_unlock(&indio_dev->mlock);
> -			return -ETIMEDOUT;
> +			return ret;
>   		}
> -
>   		*val = info->last_val;
>   		mutex_unlock(&indio_dev->mlock);
>   		return IIO_VAL_INT;
> @@ -91,7 +111,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>   		}
>   
>   		*val = ret / 1000;
> -		*val2 = info->data->num_bits;
> +		*val2 = chan->scan_type.realbits;
>   		return IIO_VAL_FRACTIONAL_LOG2;
>   	default:
>   		return -EINVAL;
> @@ -104,10 +124,9 @@ 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 &= GENMASK(info->data->num_bits - 1, 0);
> +	info->last_val &= GENMASK(info->last_chan->scan_type.realbits - 1, 0);
>   
> -	/* Clear irq & power down adc */
> -	writel_relaxed(0, info->regs + SARADC_CTRL);
> +	rockchip_saradc_power_down(info);
>   
>   	complete(&info->completion);
>   
> @@ -118,51 +137,55 @@ static const struct iio_info rockchip_saradc_iio_info = {
>   	.read_raw = rockchip_saradc_read_raw,
>   };
>   
> -#define ADC_CHANNEL(_index, _id) {				\
> +#define ADC_CHANNEL(_index, _id, _res) {			\
>   	.type = IIO_VOLTAGE,					\
>   	.indexed = 1,						\
>   	.channel = _index,					\
>   	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
>   	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
>   	.datasheet_name = _id,					\
> +	.scan_index = _index,					\
> +	.scan_type = {						\
> +		.sign = 'u',					\
> +		.realbits = _res,				\
> +		.storagebits = 16,				\
> +		.endianness = IIO_LE,				\
> +	},							\
>   }
>   
>   static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
> -	ADC_CHANNEL(0, "adc0"),
> -	ADC_CHANNEL(1, "adc1"),
> -	ADC_CHANNEL(2, "adc2"),
> +	ADC_CHANNEL(0, "adc0", 10),
> +	ADC_CHANNEL(1, "adc1", 10),
> +	ADC_CHANNEL(2, "adc2", 10),
>   };
>   
>   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"),
> +	ADC_CHANNEL(0, "adc0", 12),
> +	ADC_CHANNEL(1, "adc1", 12),
>   };
>   
>   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 iio_chan_spec rockchip_rk3399_saradc_iio_channels[] = {
> -	ADC_CHANNEL(0, "adc0"),
> -	ADC_CHANNEL(1, "adc1"),
> -	ADC_CHANNEL(2, "adc2"),
> -	ADC_CHANNEL(3, "adc3"),
> -	ADC_CHANNEL(4, "adc4"),
> -	ADC_CHANNEL(5, "adc5"),
> +	ADC_CHANNEL(0, "adc0", 10),
> +	ADC_CHANNEL(1, "adc1", 10),
> +	ADC_CHANNEL(2, "adc2", 10),
> +	ADC_CHANNEL(3, "adc3", 10),
> +	ADC_CHANNEL(4, "adc4", 10),
> +	ADC_CHANNEL(5, "adc5", 10),
>   };
>   
>   static const struct rockchip_saradc_data rk3399_saradc_data = {
> -	.num_bits = 10,
>   	.channels = rockchip_rk3399_saradc_iio_channels,
>   	.num_channels = ARRAY_SIZE(rockchip_rk3399_saradc_iio_channels),
>   	.clk_rate = 1000000,
> @@ -193,6 +216,39 @@ static void rockchip_saradc_reset_controller(struct reset_control *reset)
>   	reset_control_deassert(reset);
>   }
>   
> +static irqreturn_t rockchip_saradc_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *i_dev = pf->indio_dev;
> +	struct rockchip_saradc *info = iio_priv(i_dev);
> +	u16 data[20];
How about this:
#define MAX_CHANNEL_NUM 16
u16 data[MAX_CHANNEL_NUM];
> +	int ret;
> +	int i, j = 0;
> +
> +	mutex_lock(&i_dev->mlock);
> +
> +	for_each_set_bit(i, i_dev->active_scan_mask, i_dev->masklength) {
> +		const struct iio_chan_spec *chan = &i_dev->channels[i];
> +
> +		ret = rockchip_saradc_conversion(info, chan);
> +		if (ret) {
> +			rockchip_saradc_power_down(info);
> +			goto out;
> +		}
> +
> +		data[j] = info->last_val;
> +		j++;
> +	}
> +
> +	iio_push_to_buffers_with_timestamp(i_dev, data, iio_get_time_ns(i_dev));
> +out:
> +	mutex_unlock(&i_dev->mlock);
> +
> +	iio_trigger_notify_done(i_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
>   static int rockchip_saradc_probe(struct platform_device *pdev)
>   {
>   	struct rockchip_saradc *info = NULL;
> @@ -315,12 +371,19 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>   	indio_dev->channels = info->data->channels;
>   	indio_dev->num_channels = info->data->num_channels;
>   
> -	ret = iio_device_register(indio_dev);
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +					 rockchip_saradc_trigger_handler, NULL);
devm_iio_triggered_buffer_setup seems better
>   	if (ret)
>   		goto err_clk;
>   
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		goto err_buffer_cleanup;
> +
>   	return 0;
>   
> +err_buffer_cleanup:
> +	iio_triggered_buffer_cleanup(indio_dev);
>   err_clk:
>   	clk_disable_unprepare(info->clk);
>   err_pclk:
> 
xxm@rock-chips.com



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

* Re: [PATCH] iio: adc: rockchip_saradc: Add support iio buffers【请注意,邮件由linux-rockchip-bounces+xxm=rock-chips.com@lists.infradead.org代发】
@ 2020-03-02  2:11   ` xxm
  0 siblings, 0 replies; 8+ messages in thread
From: xxm @ 2020-03-02  2:11 UTC (permalink / raw)
  To: Heiko Stuebner, jic23-DgEjT+Ai2ygdnm+yROfE0A
  Cc: lars-Qo5EllUWu/uELgA04lAiVw, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	Heiko Stuebner, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	kever.yang-TNX95d0MmH7DzftRWevZcw,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	pmeerw-jW+XmwGofnusTnJN9+BGXg, knaack.h-Mmb7MZpHnFY

Hi, Heiko

在 2020/3/1 19:23, Heiko Stuebner 写道:
> From: Simon Xue <xxm@rock-chips.com>
> 
> Add the ability to also support access via (triggered) buffers
> next to the existing direct mode.
> 
> Device in question is the Odroid Go Advance that connects a joystick
> to two of the saradc channels for X and Y axis and the new (and still
> pending) adc joystick driver of course wants to use triggered buffers
> from the iio subsystem.
> 
> Signed-off-by: Simon Xue <xxm@rock-chips.com>
> [some simplifications and added commit description]
> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> ---
>   drivers/iio/adc/Kconfig           |   2 +
>   drivers/iio/adc/rockchip_saradc.c | 137 ++++++++++++++++++++++--------
>   2 files changed, 102 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 82e33082958c..55d2499ff757 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -787,6 +787,8 @@ config ROCKCHIP_SARADC
>   	tristate "Rockchip SARADC driver"
>   	depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
>   	depends on RESET_CONTROLLER
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
>   	help
>   	  Say yes here to build support for the SARADC found in SoCs from
>   	  Rockchip.
> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> index 582ba047c4a6..402b2210a682 100644
> --- a/drivers/iio/adc/rockchip_saradc.c
> +++ b/drivers/iio/adc/rockchip_saradc.c
> @@ -15,7 +15,11 @@
>   #include <linux/delay.h>
>   #include <linux/reset.h>
>   #include <linux/regulator/consumer.h>
> +#include <linux/iio/buffer.h>
>   #include <linux/iio/iio.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/triggered_buffer.h>
>   
>   #define SARADC_DATA			0x00
>   
> @@ -34,7 +38,6 @@
>   #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;
> @@ -49,8 +52,37 @@ struct rockchip_saradc {
>   	struct reset_control	*reset;
>   	const struct rockchip_saradc_data *data;
>   	u16			last_val;
> +	const struct iio_chan_spec *last_chan;
>   };
>   
> +static void rockchip_saradc_power_down(struct rockchip_saradc *info)
> +{
> +	/* Clear irq & power down adc */
> +	writel_relaxed(0, info->regs + SARADC_CTRL);
> +}
> +
> +static int rockchip_saradc_conversion(struct rockchip_saradc *info,
> +				   struct iio_chan_spec const *chan)
> +{
> +	reinit_completion(&info->completion);
> +
> +	/* 8 clock periods as delay between power up and start cmd */
> +	writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
> +
> +	info->last_chan = chan;
> +
> +	/* Select the channel to be used and trigger conversion */
> +	writel(SARADC_CTRL_POWER_CTRL
> +			| (chan->channel & SARADC_CTRL_CHN_MASK)
> +			| SARADC_CTRL_IRQ_ENABLE,
> +		   info->regs + SARADC_CTRL);
> +
> +	if (!wait_for_completion_timeout(&info->completion, SARADC_TIMEOUT))
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}
> +
>   static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>   				    struct iio_chan_spec const *chan,
>   				    int *val, int *val2, long mask)
> @@ -62,24 +94,12 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>   	case IIO_CHAN_INFO_RAW:
>   		mutex_lock(&indio_dev->mlock);
>   
> -		reinit_completion(&info->completion);
> -
> -		/* 8 clock periods as delay between power up and start cmd */
> -		writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
> -
> -		/* Select the channel to be used and trigger conversion */
> -		writel(SARADC_CTRL_POWER_CTRL
> -				| (chan->channel & SARADC_CTRL_CHN_MASK)
> -				| SARADC_CTRL_IRQ_ENABLE,
> -		       info->regs + SARADC_CTRL);
> -
> -		if (!wait_for_completion_timeout(&info->completion,
> -						 SARADC_TIMEOUT)) {
> -			writel_relaxed(0, info->regs + SARADC_CTRL);
> +		ret = rockchip_saradc_conversion(info, chan);
> +		if (ret) {
> +			rockchip_saradc_power_down(info);
>   			mutex_unlock(&indio_dev->mlock);
> -			return -ETIMEDOUT;
> +			return ret;
>   		}
> -
>   		*val = info->last_val;
>   		mutex_unlock(&indio_dev->mlock);
>   		return IIO_VAL_INT;
> @@ -91,7 +111,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>   		}
>   
>   		*val = ret / 1000;
> -		*val2 = info->data->num_bits;
> +		*val2 = chan->scan_type.realbits;
>   		return IIO_VAL_FRACTIONAL_LOG2;
>   	default:
>   		return -EINVAL;
> @@ -104,10 +124,9 @@ 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 &= GENMASK(info->data->num_bits - 1, 0);
> +	info->last_val &= GENMASK(info->last_chan->scan_type.realbits - 1, 0);
>   
> -	/* Clear irq & power down adc */
> -	writel_relaxed(0, info->regs + SARADC_CTRL);
> +	rockchip_saradc_power_down(info);
>   
>   	complete(&info->completion);
>   
> @@ -118,51 +137,55 @@ static const struct iio_info rockchip_saradc_iio_info = {
>   	.read_raw = rockchip_saradc_read_raw,
>   };
>   
> -#define ADC_CHANNEL(_index, _id) {				\
> +#define ADC_CHANNEL(_index, _id, _res) {			\
>   	.type = IIO_VOLTAGE,					\
>   	.indexed = 1,						\
>   	.channel = _index,					\
>   	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
>   	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
>   	.datasheet_name = _id,					\
> +	.scan_index = _index,					\
> +	.scan_type = {						\
> +		.sign = 'u',					\
> +		.realbits = _res,				\
> +		.storagebits = 16,				\
> +		.endianness = IIO_LE,				\
> +	},							\
>   }
>   
>   static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
> -	ADC_CHANNEL(0, "adc0"),
> -	ADC_CHANNEL(1, "adc1"),
> -	ADC_CHANNEL(2, "adc2"),
> +	ADC_CHANNEL(0, "adc0", 10),
> +	ADC_CHANNEL(1, "adc1", 10),
> +	ADC_CHANNEL(2, "adc2", 10),
>   };
>   
>   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"),
> +	ADC_CHANNEL(0, "adc0", 12),
> +	ADC_CHANNEL(1, "adc1", 12),
>   };
>   
>   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 iio_chan_spec rockchip_rk3399_saradc_iio_channels[] = {
> -	ADC_CHANNEL(0, "adc0"),
> -	ADC_CHANNEL(1, "adc1"),
> -	ADC_CHANNEL(2, "adc2"),
> -	ADC_CHANNEL(3, "adc3"),
> -	ADC_CHANNEL(4, "adc4"),
> -	ADC_CHANNEL(5, "adc5"),
> +	ADC_CHANNEL(0, "adc0", 10),
> +	ADC_CHANNEL(1, "adc1", 10),
> +	ADC_CHANNEL(2, "adc2", 10),
> +	ADC_CHANNEL(3, "adc3", 10),
> +	ADC_CHANNEL(4, "adc4", 10),
> +	ADC_CHANNEL(5, "adc5", 10),
>   };
>   
>   static const struct rockchip_saradc_data rk3399_saradc_data = {
> -	.num_bits = 10,
>   	.channels = rockchip_rk3399_saradc_iio_channels,
>   	.num_channels = ARRAY_SIZE(rockchip_rk3399_saradc_iio_channels),
>   	.clk_rate = 1000000,
> @@ -193,6 +216,39 @@ static void rockchip_saradc_reset_controller(struct reset_control *reset)
>   	reset_control_deassert(reset);
>   }
>   
> +static irqreturn_t rockchip_saradc_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *i_dev = pf->indio_dev;
> +	struct rockchip_saradc *info = iio_priv(i_dev);
> +	u16 data[20];
How about this:
#define MAX_CHANNEL_NUM 16
u16 data[MAX_CHANNEL_NUM];
> +	int ret;
> +	int i, j = 0;
> +
> +	mutex_lock(&i_dev->mlock);
> +
> +	for_each_set_bit(i, i_dev->active_scan_mask, i_dev->masklength) {
> +		const struct iio_chan_spec *chan = &i_dev->channels[i];
> +
> +		ret = rockchip_saradc_conversion(info, chan);
> +		if (ret) {
> +			rockchip_saradc_power_down(info);
> +			goto out;
> +		}
> +
> +		data[j] = info->last_val;
> +		j++;
> +	}
> +
> +	iio_push_to_buffers_with_timestamp(i_dev, data, iio_get_time_ns(i_dev));
> +out:
> +	mutex_unlock(&i_dev->mlock);
> +
> +	iio_trigger_notify_done(i_dev->trig);
> +
> +	return IRQ_HANDLED;
> +}
> +
>   static int rockchip_saradc_probe(struct platform_device *pdev)
>   {
>   	struct rockchip_saradc *info = NULL;
> @@ -315,12 +371,19 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>   	indio_dev->channels = info->data->channels;
>   	indio_dev->num_channels = info->data->num_channels;
>   
> -	ret = iio_device_register(indio_dev);
> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> +					 rockchip_saradc_trigger_handler, NULL);
devm_iio_triggered_buffer_setup seems better
>   	if (ret)
>   		goto err_clk;
>   
> +	ret = iio_device_register(indio_dev);
> +	if (ret)
> +		goto err_buffer_cleanup;
> +
>   	return 0;
>   
> +err_buffer_cleanup:
> +	iio_triggered_buffer_cleanup(indio_dev);
>   err_clk:
>   	clk_disable_unprepare(info->clk);
>   err_pclk:
> 
xxm@rock-chips.com



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [PATCH] iio: adc: rockchip_saradc: Add support iio buffers【请注意,邮件由linux-rockchip-bounces+xxm=rock-chips.com@lists.infradead.org代发】
@ 2020-03-03 20:32     ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2020-03-03 20:32 UTC (permalink / raw)
  To: xxm
  Cc: Heiko Stuebner, lars, linux-iio, Heiko Stuebner, linux-kernel,
	kever.yang, linux-rockchip, pmeerw, knaack.h

On Mon, 2 Mar 2020 10:11:02 +0800
xxm <xxm@rock-chips.com> wrote:

> Hi, Heiko
> 
> 在 2020/3/1 19:23, Heiko Stuebner 写道:
> > From: Simon Xue <xxm@rock-chips.com>
> > 
> > Add the ability to also support access via (triggered) buffers
> > next to the existing direct mode.
> > 
> > Device in question is the Odroid Go Advance that connects a joystick
> > to two of the saradc channels for X and Y axis and the new (and still
> > pending) adc joystick driver of course wants to use triggered buffers
> > from the iio subsystem.
> > 
> > Signed-off-by: Simon Xue <xxm@rock-chips.com>
> > [some simplifications and added commit description]
> > Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> > ---
> >   drivers/iio/adc/Kconfig           |   2 +
> >   drivers/iio/adc/rockchip_saradc.c | 137 ++++++++++++++++++++++--------
> >   2 files changed, 102 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 82e33082958c..55d2499ff757 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -787,6 +787,8 @@ config ROCKCHIP_SARADC
> >   	tristate "Rockchip SARADC driver"
> >   	depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
> >   	depends on RESET_CONTROLLER
> > +	select IIO_BUFFER
> > +	select IIO_TRIGGERED_BUFFER
> >   	help
> >   	  Say yes here to build support for the SARADC found in SoCs from
> >   	  Rockchip.
> > diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> > index 582ba047c4a6..402b2210a682 100644
> > --- a/drivers/iio/adc/rockchip_saradc.c
> > +++ b/drivers/iio/adc/rockchip_saradc.c
> > @@ -15,7 +15,11 @@
> >   #include <linux/delay.h>
> >   #include <linux/reset.h>
> >   #include <linux/regulator/consumer.h>
> > +#include <linux/iio/buffer.h>
> >   #include <linux/iio/iio.h>
> > +#include <linux/iio/trigger.h>
> > +#include <linux/iio/trigger_consumer.h>
> > +#include <linux/iio/triggered_buffer.h>
> >   
> >   #define SARADC_DATA			0x00
> >   
> > @@ -34,7 +38,6 @@
> >   #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;
> > @@ -49,8 +52,37 @@ struct rockchip_saradc {
> >   	struct reset_control	*reset;
> >   	const struct rockchip_saradc_data *data;
> >   	u16			last_val;
> > +	const struct iio_chan_spec *last_chan;
> >   };
> >   
> > +static void rockchip_saradc_power_down(struct rockchip_saradc *info)
> > +{
> > +	/* Clear irq & power down adc */
> > +	writel_relaxed(0, info->regs + SARADC_CTRL);
> > +}
> > +
> > +static int rockchip_saradc_conversion(struct rockchip_saradc *info,
> > +				   struct iio_chan_spec const *chan)
> > +{
> > +	reinit_completion(&info->completion);
> > +
> > +	/* 8 clock periods as delay between power up and start cmd */
> > +	writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
> > +
> > +	info->last_chan = chan;
> > +
> > +	/* Select the channel to be used and trigger conversion */
> > +	writel(SARADC_CTRL_POWER_CTRL
> > +			| (chan->channel & SARADC_CTRL_CHN_MASK)
> > +			| SARADC_CTRL_IRQ_ENABLE,
> > +		   info->regs + SARADC_CTRL);
> > +
> > +	if (!wait_for_completion_timeout(&info->completion, SARADC_TIMEOUT))
> > +		return -ETIMEDOUT;
> > +
> > +	return 0;
> > +}
> > +
> >   static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
> >   				    struct iio_chan_spec const *chan,
> >   				    int *val, int *val2, long mask)
> > @@ -62,24 +94,12 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
> >   	case IIO_CHAN_INFO_RAW:
> >   		mutex_lock(&indio_dev->mlock);
> >   
> > -		reinit_completion(&info->completion);
> > -
> > -		/* 8 clock periods as delay between power up and start cmd */
> > -		writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
> > -
> > -		/* Select the channel to be used and trigger conversion */
> > -		writel(SARADC_CTRL_POWER_CTRL
> > -				| (chan->channel & SARADC_CTRL_CHN_MASK)
> > -				| SARADC_CTRL_IRQ_ENABLE,
> > -		       info->regs + SARADC_CTRL);
> > -
> > -		if (!wait_for_completion_timeout(&info->completion,
> > -						 SARADC_TIMEOUT)) {
> > -			writel_relaxed(0, info->regs + SARADC_CTRL);
> > +		ret = rockchip_saradc_conversion(info, chan);
> > +		if (ret) {
> > +			rockchip_saradc_power_down(info);
> >   			mutex_unlock(&indio_dev->mlock);
> > -			return -ETIMEDOUT;
> > +			return ret;
> >   		}
> > -
> >   		*val = info->last_val;
> >   		mutex_unlock(&indio_dev->mlock);
> >   		return IIO_VAL_INT;
> > @@ -91,7 +111,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
> >   		}
> >   
> >   		*val = ret / 1000;
> > -		*val2 = info->data->num_bits;
> > +		*val2 = chan->scan_type.realbits;
> >   		return IIO_VAL_FRACTIONAL_LOG2;
> >   	default:
> >   		return -EINVAL;
> > @@ -104,10 +124,9 @@ 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 &= GENMASK(info->data->num_bits - 1, 0);
> > +	info->last_val &= GENMASK(info->last_chan->scan_type.realbits - 1, 0);
> >   
> > -	/* Clear irq & power down adc */
> > -	writel_relaxed(0, info->regs + SARADC_CTRL);
> > +	rockchip_saradc_power_down(info);
> >   
> >   	complete(&info->completion);
> >   
> > @@ -118,51 +137,55 @@ static const struct iio_info rockchip_saradc_iio_info = {
> >   	.read_raw = rockchip_saradc_read_raw,
> >   };
> >   
> > -#define ADC_CHANNEL(_index, _id) {				\
> > +#define ADC_CHANNEL(_index, _id, _res) {			\
> >   	.type = IIO_VOLTAGE,					\
> >   	.indexed = 1,						\
> >   	.channel = _index,					\
> >   	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> >   	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> >   	.datasheet_name = _id,					\
> > +	.scan_index = _index,					\
> > +	.scan_type = {						\
> > +		.sign = 'u',					\
> > +		.realbits = _res,				\
> > +		.storagebits = 16,				\
> > +		.endianness = IIO_LE,				\
> > +	},							\
> >   }
> >   
> >   static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
> > -	ADC_CHANNEL(0, "adc0"),
> > -	ADC_CHANNEL(1, "adc1"),
> > -	ADC_CHANNEL(2, "adc2"),
> > +	ADC_CHANNEL(0, "adc0", 10),
> > +	ADC_CHANNEL(1, "adc1", 10),
> > +	ADC_CHANNEL(2, "adc2", 10),
> >   };
> >   
> >   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"),
> > +	ADC_CHANNEL(0, "adc0", 12),
> > +	ADC_CHANNEL(1, "adc1", 12),
> >   };
> >   
> >   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 iio_chan_spec rockchip_rk3399_saradc_iio_channels[] = {
> > -	ADC_CHANNEL(0, "adc0"),
> > -	ADC_CHANNEL(1, "adc1"),
> > -	ADC_CHANNEL(2, "adc2"),
> > -	ADC_CHANNEL(3, "adc3"),
> > -	ADC_CHANNEL(4, "adc4"),
> > -	ADC_CHANNEL(5, "adc5"),
> > +	ADC_CHANNEL(0, "adc0", 10),
> > +	ADC_CHANNEL(1, "adc1", 10),
> > +	ADC_CHANNEL(2, "adc2", 10),
> > +	ADC_CHANNEL(3, "adc3", 10),
> > +	ADC_CHANNEL(4, "adc4", 10),
> > +	ADC_CHANNEL(5, "adc5", 10),
> >   };
> >   
> >   static const struct rockchip_saradc_data rk3399_saradc_data = {
> > -	.num_bits = 10,
> >   	.channels = rockchip_rk3399_saradc_iio_channels,
> >   	.num_channels = ARRAY_SIZE(rockchip_rk3399_saradc_iio_channels),
> >   	.clk_rate = 1000000,
> > @@ -193,6 +216,39 @@ static void rockchip_saradc_reset_controller(struct reset_control *reset)
> >   	reset_control_deassert(reset);
> >   }
> >   
> > +static irqreturn_t rockchip_saradc_trigger_handler(int irq, void *p)
> > +{
> > +	struct iio_poll_func *pf = p;
> > +	struct iio_dev *i_dev = pf->indio_dev;
> > +	struct rockchip_saradc *info = iio_priv(i_dev);
> > +	u16 data[20];  
> How about this:
> #define MAX_CHANNEL_NUM 16

Unfortunately this is a bit more complex than it seems. 
The buffer needs to be big enough for all the channels
+ a 8 byte aligned space to put the timestamp in.

You can construct that in a fashion suitable to use in a
macro but it's a bit more fiddly than simply being the
maximum number of channels.

> u16 data[MAX_CHANNEL_NUM];
> > +	int ret;
> > +	int i, j = 0;
> > +
> > +	mutex_lock(&i_dev->mlock);
> > +
> > +	for_each_set_bit(i, i_dev->active_scan_mask, i_dev->masklength) {
> > +		const struct iio_chan_spec *chan = &i_dev->channels[i];
> > +
> > +		ret = rockchip_saradc_conversion(info, chan);
> > +		if (ret) {
> > +			rockchip_saradc_power_down(info);
> > +			goto out;
> > +		}
> > +
> > +		data[j] = info->last_val;
> > +		j++;
> > +	}
> > +
> > +	iio_push_to_buffers_with_timestamp(i_dev, data, iio_get_time_ns(i_dev));
> > +out:
> > +	mutex_unlock(&i_dev->mlock);
> > +
> > +	iio_trigger_notify_done(i_dev->trig);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> >   static int rockchip_saradc_probe(struct platform_device *pdev)
> >   {
> >   	struct rockchip_saradc *info = NULL;
> > @@ -315,12 +371,19 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
> >   	indio_dev->channels = info->data->channels;
> >   	indio_dev->num_channels = info->data->num_channels;
> >   
> > -	ret = iio_device_register(indio_dev);
> > +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> > +					 rockchip_saradc_trigger_handler, NULL);  
> devm_iio_triggered_buffer_setup seems better
> >   	if (ret)
> >   		goto err_clk;
> >   
> > +	ret = iio_device_register(indio_dev);
> > +	if (ret)
> > +		goto err_buffer_cleanup;
> > +
> >   	return 0;
> >   
> > +err_buffer_cleanup:
> > +	iio_triggered_buffer_cleanup(indio_dev);
> >   err_clk:
> >   	clk_disable_unprepare(info->clk);
> >   err_pclk:
> >   
> xxm@rock-chips.com
> 
> 


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

* Re: [PATCH] iio: adc: rockchip_saradc: Add support iio buffers【请注意,邮件由linux-rockchip-bounces+xxm=rock-chips.com@lists.infradead.org代发】
@ 2020-03-03 20:32     ` Jonathan Cameron
  0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2020-03-03 20:32 UTC (permalink / raw)
  To: xxm
  Cc: Heiko Stuebner, lars-Qo5EllUWu/uELgA04lAiVw,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Heiko Stuebner,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	kever.yang-TNX95d0MmH7DzftRWevZcw,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	pmeerw-jW+XmwGofnusTnJN9+BGXg, knaack.h-Mmb7MZpHnFY

On Mon, 2 Mar 2020 10:11:02 +0800
xxm <xxm-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:

> Hi, Heiko
> 
> 在 2020/3/1 19:23, Heiko Stuebner 写道:
> > From: Simon Xue <xxm-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> > 
> > Add the ability to also support access via (triggered) buffers
> > next to the existing direct mode.
> > 
> > Device in question is the Odroid Go Advance that connects a joystick
> > to two of the saradc channels for X and Y axis and the new (and still
> > pending) adc joystick driver of course wants to use triggered buffers
> > from the iio subsystem.
> > 
> > Signed-off-by: Simon Xue <xxm-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> > [some simplifications and added commit description]
> > Signed-off-by: Heiko Stuebner <heiko.stuebner-SN7IsUiht6C/RdPyistoZJqQE7yCjDx5@public.gmane.org>
> > ---
> >   drivers/iio/adc/Kconfig           |   2 +
> >   drivers/iio/adc/rockchip_saradc.c | 137 ++++++++++++++++++++++--------
> >   2 files changed, 102 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> > index 82e33082958c..55d2499ff757 100644
> > --- a/drivers/iio/adc/Kconfig
> > +++ b/drivers/iio/adc/Kconfig
> > @@ -787,6 +787,8 @@ config ROCKCHIP_SARADC
> >   	tristate "Rockchip SARADC driver"
> >   	depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
> >   	depends on RESET_CONTROLLER
> > +	select IIO_BUFFER
> > +	select IIO_TRIGGERED_BUFFER
> >   	help
> >   	  Say yes here to build support for the SARADC found in SoCs from
> >   	  Rockchip.
> > diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> > index 582ba047c4a6..402b2210a682 100644
> > --- a/drivers/iio/adc/rockchip_saradc.c
> > +++ b/drivers/iio/adc/rockchip_saradc.c
> > @@ -15,7 +15,11 @@
> >   #include <linux/delay.h>
> >   #include <linux/reset.h>
> >   #include <linux/regulator/consumer.h>
> > +#include <linux/iio/buffer.h>
> >   #include <linux/iio/iio.h>
> > +#include <linux/iio/trigger.h>
> > +#include <linux/iio/trigger_consumer.h>
> > +#include <linux/iio/triggered_buffer.h>
> >   
> >   #define SARADC_DATA			0x00
> >   
> > @@ -34,7 +38,6 @@
> >   #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;
> > @@ -49,8 +52,37 @@ struct rockchip_saradc {
> >   	struct reset_control	*reset;
> >   	const struct rockchip_saradc_data *data;
> >   	u16			last_val;
> > +	const struct iio_chan_spec *last_chan;
> >   };
> >   
> > +static void rockchip_saradc_power_down(struct rockchip_saradc *info)
> > +{
> > +	/* Clear irq & power down adc */
> > +	writel_relaxed(0, info->regs + SARADC_CTRL);
> > +}
> > +
> > +static int rockchip_saradc_conversion(struct rockchip_saradc *info,
> > +				   struct iio_chan_spec const *chan)
> > +{
> > +	reinit_completion(&info->completion);
> > +
> > +	/* 8 clock periods as delay between power up and start cmd */
> > +	writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
> > +
> > +	info->last_chan = chan;
> > +
> > +	/* Select the channel to be used and trigger conversion */
> > +	writel(SARADC_CTRL_POWER_CTRL
> > +			| (chan->channel & SARADC_CTRL_CHN_MASK)
> > +			| SARADC_CTRL_IRQ_ENABLE,
> > +		   info->regs + SARADC_CTRL);
> > +
> > +	if (!wait_for_completion_timeout(&info->completion, SARADC_TIMEOUT))
> > +		return -ETIMEDOUT;
> > +
> > +	return 0;
> > +}
> > +
> >   static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
> >   				    struct iio_chan_spec const *chan,
> >   				    int *val, int *val2, long mask)
> > @@ -62,24 +94,12 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
> >   	case IIO_CHAN_INFO_RAW:
> >   		mutex_lock(&indio_dev->mlock);
> >   
> > -		reinit_completion(&info->completion);
> > -
> > -		/* 8 clock periods as delay between power up and start cmd */
> > -		writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
> > -
> > -		/* Select the channel to be used and trigger conversion */
> > -		writel(SARADC_CTRL_POWER_CTRL
> > -				| (chan->channel & SARADC_CTRL_CHN_MASK)
> > -				| SARADC_CTRL_IRQ_ENABLE,
> > -		       info->regs + SARADC_CTRL);
> > -
> > -		if (!wait_for_completion_timeout(&info->completion,
> > -						 SARADC_TIMEOUT)) {
> > -			writel_relaxed(0, info->regs + SARADC_CTRL);
> > +		ret = rockchip_saradc_conversion(info, chan);
> > +		if (ret) {
> > +			rockchip_saradc_power_down(info);
> >   			mutex_unlock(&indio_dev->mlock);
> > -			return -ETIMEDOUT;
> > +			return ret;
> >   		}
> > -
> >   		*val = info->last_val;
> >   		mutex_unlock(&indio_dev->mlock);
> >   		return IIO_VAL_INT;
> > @@ -91,7 +111,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
> >   		}
> >   
> >   		*val = ret / 1000;
> > -		*val2 = info->data->num_bits;
> > +		*val2 = chan->scan_type.realbits;
> >   		return IIO_VAL_FRACTIONAL_LOG2;
> >   	default:
> >   		return -EINVAL;
> > @@ -104,10 +124,9 @@ 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 &= GENMASK(info->data->num_bits - 1, 0);
> > +	info->last_val &= GENMASK(info->last_chan->scan_type.realbits - 1, 0);
> >   
> > -	/* Clear irq & power down adc */
> > -	writel_relaxed(0, info->regs + SARADC_CTRL);
> > +	rockchip_saradc_power_down(info);
> >   
> >   	complete(&info->completion);
> >   
> > @@ -118,51 +137,55 @@ static const struct iio_info rockchip_saradc_iio_info = {
> >   	.read_raw = rockchip_saradc_read_raw,
> >   };
> >   
> > -#define ADC_CHANNEL(_index, _id) {				\
> > +#define ADC_CHANNEL(_index, _id, _res) {			\
> >   	.type = IIO_VOLTAGE,					\
> >   	.indexed = 1,						\
> >   	.channel = _index,					\
> >   	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> >   	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> >   	.datasheet_name = _id,					\
> > +	.scan_index = _index,					\
> > +	.scan_type = {						\
> > +		.sign = 'u',					\
> > +		.realbits = _res,				\
> > +		.storagebits = 16,				\
> > +		.endianness = IIO_LE,				\
> > +	},							\
> >   }
> >   
> >   static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
> > -	ADC_CHANNEL(0, "adc0"),
> > -	ADC_CHANNEL(1, "adc1"),
> > -	ADC_CHANNEL(2, "adc2"),
> > +	ADC_CHANNEL(0, "adc0", 10),
> > +	ADC_CHANNEL(1, "adc1", 10),
> > +	ADC_CHANNEL(2, "adc2", 10),
> >   };
> >   
> >   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"),
> > +	ADC_CHANNEL(0, "adc0", 12),
> > +	ADC_CHANNEL(1, "adc1", 12),
> >   };
> >   
> >   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 iio_chan_spec rockchip_rk3399_saradc_iio_channels[] = {
> > -	ADC_CHANNEL(0, "adc0"),
> > -	ADC_CHANNEL(1, "adc1"),
> > -	ADC_CHANNEL(2, "adc2"),
> > -	ADC_CHANNEL(3, "adc3"),
> > -	ADC_CHANNEL(4, "adc4"),
> > -	ADC_CHANNEL(5, "adc5"),
> > +	ADC_CHANNEL(0, "adc0", 10),
> > +	ADC_CHANNEL(1, "adc1", 10),
> > +	ADC_CHANNEL(2, "adc2", 10),
> > +	ADC_CHANNEL(3, "adc3", 10),
> > +	ADC_CHANNEL(4, "adc4", 10),
> > +	ADC_CHANNEL(5, "adc5", 10),
> >   };
> >   
> >   static const struct rockchip_saradc_data rk3399_saradc_data = {
> > -	.num_bits = 10,
> >   	.channels = rockchip_rk3399_saradc_iio_channels,
> >   	.num_channels = ARRAY_SIZE(rockchip_rk3399_saradc_iio_channels),
> >   	.clk_rate = 1000000,
> > @@ -193,6 +216,39 @@ static void rockchip_saradc_reset_controller(struct reset_control *reset)
> >   	reset_control_deassert(reset);
> >   }
> >   
> > +static irqreturn_t rockchip_saradc_trigger_handler(int irq, void *p)
> > +{
> > +	struct iio_poll_func *pf = p;
> > +	struct iio_dev *i_dev = pf->indio_dev;
> > +	struct rockchip_saradc *info = iio_priv(i_dev);
> > +	u16 data[20];  
> How about this:
> #define MAX_CHANNEL_NUM 16

Unfortunately this is a bit more complex than it seems. 
The buffer needs to be big enough for all the channels
+ a 8 byte aligned space to put the timestamp in.

You can construct that in a fashion suitable to use in a
macro but it's a bit more fiddly than simply being the
maximum number of channels.

> u16 data[MAX_CHANNEL_NUM];
> > +	int ret;
> > +	int i, j = 0;
> > +
> > +	mutex_lock(&i_dev->mlock);
> > +
> > +	for_each_set_bit(i, i_dev->active_scan_mask, i_dev->masklength) {
> > +		const struct iio_chan_spec *chan = &i_dev->channels[i];
> > +
> > +		ret = rockchip_saradc_conversion(info, chan);
> > +		if (ret) {
> > +			rockchip_saradc_power_down(info);
> > +			goto out;
> > +		}
> > +
> > +		data[j] = info->last_val;
> > +		j++;
> > +	}
> > +
> > +	iio_push_to_buffers_with_timestamp(i_dev, data, iio_get_time_ns(i_dev));
> > +out:
> > +	mutex_unlock(&i_dev->mlock);
> > +
> > +	iio_trigger_notify_done(i_dev->trig);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> >   static int rockchip_saradc_probe(struct platform_device *pdev)
> >   {
> >   	struct rockchip_saradc *info = NULL;
> > @@ -315,12 +371,19 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
> >   	indio_dev->channels = info->data->channels;
> >   	indio_dev->num_channels = info->data->num_channels;
> >   
> > -	ret = iio_device_register(indio_dev);
> > +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> > +					 rockchip_saradc_trigger_handler, NULL);  
> devm_iio_triggered_buffer_setup seems better
> >   	if (ret)
> >   		goto err_clk;
> >   
> > +	ret = iio_device_register(indio_dev);
> > +	if (ret)
> > +		goto err_buffer_cleanup;
> > +
> >   	return 0;
> >   
> > +err_buffer_cleanup:
> > +	iio_triggered_buffer_cleanup(indio_dev);
> >   err_clk:
> >   	clk_disable_unprepare(info->clk);
> >   err_pclk:
> >   
> xxm-TNX95d0MmH7DzftRWevZcw@public.gmane.org
> 
> 

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

* Re: [PATCH] iio: adc: rockchip_saradc: Add support iio buffers【请注意,邮件由linux-rockchip-bounces+xxm=rock-chips.com@lists.infradead.org代发】
  2020-03-03 20:32     ` Jonathan Cameron
@ 2020-03-04  1:39       ` xxm
  -1 siblings, 0 replies; 8+ messages in thread
From: xxm @ 2020-03-04  1:39 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Heiko Stuebner, lars, linux-iio, Heiko Stuebner, linux-kernel,
	kever.yang, linux-rockchip, pmeerw, knaack.h

Hi,

在 2020/3/4 4:32, Jonathan Cameron 写道:
> On Mon, 2 Mar 2020 10:11:02 +0800
> xxm <xxm@rock-chips.com> wrote:
> 
>> Hi, Heiko
>>
>> 在 2020/3/1 19:23, Heiko Stuebner 写道:
>>> From: Simon Xue <xxm@rock-chips.com>
>>>
>>> Add the ability to also support access via (triggered) buffers
>>> next to the existing direct mode.
>>>
>>> Device in question is the Odroid Go Advance that connects a joystick
>>> to two of the saradc channels for X and Y axis and the new (and still
>>> pending) adc joystick driver of course wants to use triggered buffers
>>> from the iio subsystem.
>>>
>>> Signed-off-by: Simon Xue <xxm@rock-chips.com>
>>> [some simplifications and added commit description]
>>> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>>> ---
>>>    drivers/iio/adc/Kconfig           |   2 +
>>>    drivers/iio/adc/rockchip_saradc.c | 137 ++++++++++++++++++++++--------
>>>    2 files changed, 102 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 82e33082958c..55d2499ff757 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -787,6 +787,8 @@ config ROCKCHIP_SARADC
>>>    	tristate "Rockchip SARADC driver"
>>>    	depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
>>>    	depends on RESET_CONTROLLER
>>> +	select IIO_BUFFER
>>> +	select IIO_TRIGGERED_BUFFER
>>>    	help
>>>    	  Say yes here to build support for the SARADC found in SoCs from
>>>    	  Rockchip.
>>> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
>>> index 582ba047c4a6..402b2210a682 100644
>>> --- a/drivers/iio/adc/rockchip_saradc.c
>>> +++ b/drivers/iio/adc/rockchip_saradc.c
>>> @@ -15,7 +15,11 @@
>>>    #include <linux/delay.h>
>>>    #include <linux/reset.h>
>>>    #include <linux/regulator/consumer.h>
>>> +#include <linux/iio/buffer.h>
>>>    #include <linux/iio/iio.h>
>>> +#include <linux/iio/trigger.h>
>>> +#include <linux/iio/trigger_consumer.h>
>>> +#include <linux/iio/triggered_buffer.h>
>>>    
>>>    #define SARADC_DATA			0x00
>>>    
>>> @@ -34,7 +38,6 @@
>>>    #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;
>>> @@ -49,8 +52,37 @@ struct rockchip_saradc {
>>>    	struct reset_control	*reset;
>>>    	const struct rockchip_saradc_data *data;
>>>    	u16			last_val;
>>> +	const struct iio_chan_spec *last_chan;
>>>    };
>>>    
>>> +static void rockchip_saradc_power_down(struct rockchip_saradc *info)
>>> +{
>>> +	/* Clear irq & power down adc */
>>> +	writel_relaxed(0, info->regs + SARADC_CTRL);
>>> +}
>>> +
>>> +static int rockchip_saradc_conversion(struct rockchip_saradc *info,
>>> +				   struct iio_chan_spec const *chan)
>>> +{
>>> +	reinit_completion(&info->completion);
>>> +
>>> +	/* 8 clock periods as delay between power up and start cmd */
>>> +	writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
>>> +
>>> +	info->last_chan = chan;
>>> +
>>> +	/* Select the channel to be used and trigger conversion */
>>> +	writel(SARADC_CTRL_POWER_CTRL
>>> +			| (chan->channel & SARADC_CTRL_CHN_MASK)
>>> +			| SARADC_CTRL_IRQ_ENABLE,
>>> +		   info->regs + SARADC_CTRL);
>>> +
>>> +	if (!wait_for_completion_timeout(&info->completion, SARADC_TIMEOUT))
>>> +		return -ETIMEDOUT;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>    static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>>>    				    struct iio_chan_spec const *chan,
>>>    				    int *val, int *val2, long mask)
>>> @@ -62,24 +94,12 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>>>    	case IIO_CHAN_INFO_RAW:
>>>    		mutex_lock(&indio_dev->mlock);
>>>    
>>> -		reinit_completion(&info->completion);
>>> -
>>> -		/* 8 clock periods as delay between power up and start cmd */
>>> -		writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
>>> -
>>> -		/* Select the channel to be used and trigger conversion */
>>> -		writel(SARADC_CTRL_POWER_CTRL
>>> -				| (chan->channel & SARADC_CTRL_CHN_MASK)
>>> -				| SARADC_CTRL_IRQ_ENABLE,
>>> -		       info->regs + SARADC_CTRL);
>>> -
>>> -		if (!wait_for_completion_timeout(&info->completion,
>>> -						 SARADC_TIMEOUT)) {
>>> -			writel_relaxed(0, info->regs + SARADC_CTRL);
>>> +		ret = rockchip_saradc_conversion(info, chan);
>>> +		if (ret) {
>>> +			rockchip_saradc_power_down(info);
>>>    			mutex_unlock(&indio_dev->mlock);
>>> -			return -ETIMEDOUT;
>>> +			return ret;
>>>    		}
>>> -
>>>    		*val = info->last_val;
>>>    		mutex_unlock(&indio_dev->mlock);
>>>    		return IIO_VAL_INT;
>>> @@ -91,7 +111,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>>>    		}
>>>    
>>>    		*val = ret / 1000;
>>> -		*val2 = info->data->num_bits;
>>> +		*val2 = chan->scan_type.realbits;
>>>    		return IIO_VAL_FRACTIONAL_LOG2;
>>>    	default:
>>>    		return -EINVAL;
>>> @@ -104,10 +124,9 @@ 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 &= GENMASK(info->data->num_bits - 1, 0);
>>> +	info->last_val &= GENMASK(info->last_chan->scan_type.realbits - 1, 0);
>>>    
>>> -	/* Clear irq & power down adc */
>>> -	writel_relaxed(0, info->regs + SARADC_CTRL);
>>> +	rockchip_saradc_power_down(info);
>>>    
>>>    	complete(&info->completion);
>>>    
>>> @@ -118,51 +137,55 @@ static const struct iio_info rockchip_saradc_iio_info = {
>>>    	.read_raw = rockchip_saradc_read_raw,
>>>    };
>>>    
>>> -#define ADC_CHANNEL(_index, _id) {				\
>>> +#define ADC_CHANNEL(_index, _id, _res) {			\
>>>    	.type = IIO_VOLTAGE,					\
>>>    	.indexed = 1,						\
>>>    	.channel = _index,					\
>>>    	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
>>>    	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
>>>    	.datasheet_name = _id,					\
>>> +	.scan_index = _index,					\
>>> +	.scan_type = {						\
>>> +		.sign = 'u',					\
>>> +		.realbits = _res,				\
>>> +		.storagebits = 16,				\
>>> +		.endianness = IIO_LE,				\
>>> +	},							\
>>>    }
>>>    
>>>    static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
>>> -	ADC_CHANNEL(0, "adc0"),
>>> -	ADC_CHANNEL(1, "adc1"),
>>> -	ADC_CHANNEL(2, "adc2"),
>>> +	ADC_CHANNEL(0, "adc0", 10),
>>> +	ADC_CHANNEL(1, "adc1", 10),
>>> +	ADC_CHANNEL(2, "adc2", 10),
>>>    };
>>>    
>>>    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"),
>>> +	ADC_CHANNEL(0, "adc0", 12),
>>> +	ADC_CHANNEL(1, "adc1", 12),
>>>    };
>>>    
>>>    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 iio_chan_spec rockchip_rk3399_saradc_iio_channels[] = {
>>> -	ADC_CHANNEL(0, "adc0"),
>>> -	ADC_CHANNEL(1, "adc1"),
>>> -	ADC_CHANNEL(2, "adc2"),
>>> -	ADC_CHANNEL(3, "adc3"),
>>> -	ADC_CHANNEL(4, "adc4"),
>>> -	ADC_CHANNEL(5, "adc5"),
>>> +	ADC_CHANNEL(0, "adc0", 10),
>>> +	ADC_CHANNEL(1, "adc1", 10),
>>> +	ADC_CHANNEL(2, "adc2", 10),
>>> +	ADC_CHANNEL(3, "adc3", 10),
>>> +	ADC_CHANNEL(4, "adc4", 10),
>>> +	ADC_CHANNEL(5, "adc5", 10),
>>>    };
>>>    
>>>    static const struct rockchip_saradc_data rk3399_saradc_data = {
>>> -	.num_bits = 10,
>>>    	.channels = rockchip_rk3399_saradc_iio_channels,
>>>    	.num_channels = ARRAY_SIZE(rockchip_rk3399_saradc_iio_channels),
>>>    	.clk_rate = 1000000,
>>> @@ -193,6 +216,39 @@ static void rockchip_saradc_reset_controller(struct reset_control *reset)
>>>    	reset_control_deassert(reset);
>>>    }
>>>    
>>> +static irqreturn_t rockchip_saradc_trigger_handler(int irq, void *p)
>>> +{
>>> +	struct iio_poll_func *pf = p;
>>> +	struct iio_dev *i_dev = pf->indio_dev;
>>> +	struct rockchip_saradc *info = iio_priv(i_dev);
>>> +	u16 data[20];
>> How about this:
>> #define MAX_CHANNEL_NUM 16
> 
> Unfortunately this is a bit more complex than it seems.
> The buffer needs to be big enough for all the channels
> + a 8 byte aligned space to put the timestamp in.
> 
> You can construct that in a fashion suitable to use in a
> macro but it's a bit more fiddly than simply being the
> maximum number of channels.
> 
Make use of iio_dev->scan_bytes to alloc a buffer in 
iio_info->update_scan_mode callback for storing the
"data + timestamp" is another way
>> u16 data[MAX_CHANNEL_NUM];
>>> +	int ret;
>>> +	int i, j = 0;
>>> +
>>> +	mutex_lock(&i_dev->mlock);
>>> +
>>> +	for_each_set_bit(i, i_dev->active_scan_mask, i_dev->masklength) {
>>> +		const struct iio_chan_spec *chan = &i_dev->channels[i];
>>> +
>>> +		ret = rockchip_saradc_conversion(info, chan);
>>> +		if (ret) {
>>> +			rockchip_saradc_power_down(info);
>>> +			goto out;
>>> +		}
>>> +
>>> +		data[j] = info->last_val;
>>> +		j++;
>>> +	}
>>> +
>>> +	iio_push_to_buffers_with_timestamp(i_dev, data, iio_get_time_ns(i_dev));
>>> +out:
>>> +	mutex_unlock(&i_dev->mlock);
>>> +
>>> +	iio_trigger_notify_done(i_dev->trig);
>>> +
>>> +	return IRQ_HANDLED;
>>> +}
>>> +
>>>    static int rockchip_saradc_probe(struct platform_device *pdev)
>>>    {
>>>    	struct rockchip_saradc *info = NULL;
>>> @@ -315,12 +371,19 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>>>    	indio_dev->channels = info->data->channels;
>>>    	indio_dev->num_channels = info->data->num_channels;
>>>    
>>> -	ret = iio_device_register(indio_dev);
>>> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>>> +					 rockchip_saradc_trigger_handler, NULL);
>> devm_iio_triggered_buffer_setup seems better
>>>    	if (ret)
>>>    		goto err_clk;
>>>    
>>> +	ret = iio_device_register(indio_dev);
>>> +	if (ret)
>>> +		goto err_buffer_cleanup;
>>> +
>>>    	return 0;
>>>    
>>> +err_buffer_cleanup:
>>> +	iio_triggered_buffer_cleanup(indio_dev);
>>>    err_clk:
>>>    	clk_disable_unprepare(info->clk);
>>>    err_pclk:
>>>    
>> xxm@rock-chips.com
>>
>>
> 
> 
> 
> 



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

* Re: [PATCH] iio: adc: rockchip_saradc: Add support iio buffers【请注意,邮件由linux-rockchip-bounces+xxm=rock-chips.com@lists.infradead.org代发】
@ 2020-03-04  1:39       ` xxm
  0 siblings, 0 replies; 8+ messages in thread
From: xxm @ 2020-03-04  1:39 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: lars-Qo5EllUWu/uELgA04lAiVw, Heiko Stuebner,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Heiko Stuebner,
	kever.yang-TNX95d0MmH7DzftRWevZcw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	pmeerw-jW+XmwGofnusTnJN9+BGXg, knaack.h-Mmb7MZpHnFY

Hi,

在 2020/3/4 4:32, Jonathan Cameron 写道:
> On Mon, 2 Mar 2020 10:11:02 +0800
> xxm <xxm@rock-chips.com> wrote:
> 
>> Hi, Heiko
>>
>> 在 2020/3/1 19:23, Heiko Stuebner 写道:
>>> From: Simon Xue <xxm@rock-chips.com>
>>>
>>> Add the ability to also support access via (triggered) buffers
>>> next to the existing direct mode.
>>>
>>> Device in question is the Odroid Go Advance that connects a joystick
>>> to two of the saradc channels for X and Y axis and the new (and still
>>> pending) adc joystick driver of course wants to use triggered buffers
>>> from the iio subsystem.
>>>
>>> Signed-off-by: Simon Xue <xxm@rock-chips.com>
>>> [some simplifications and added commit description]
>>> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
>>> ---
>>>    drivers/iio/adc/Kconfig           |   2 +
>>>    drivers/iio/adc/rockchip_saradc.c | 137 ++++++++++++++++++++++--------
>>>    2 files changed, 102 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>>> index 82e33082958c..55d2499ff757 100644
>>> --- a/drivers/iio/adc/Kconfig
>>> +++ b/drivers/iio/adc/Kconfig
>>> @@ -787,6 +787,8 @@ config ROCKCHIP_SARADC
>>>    	tristate "Rockchip SARADC driver"
>>>    	depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
>>>    	depends on RESET_CONTROLLER
>>> +	select IIO_BUFFER
>>> +	select IIO_TRIGGERED_BUFFER
>>>    	help
>>>    	  Say yes here to build support for the SARADC found in SoCs from
>>>    	  Rockchip.
>>> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
>>> index 582ba047c4a6..402b2210a682 100644
>>> --- a/drivers/iio/adc/rockchip_saradc.c
>>> +++ b/drivers/iio/adc/rockchip_saradc.c
>>> @@ -15,7 +15,11 @@
>>>    #include <linux/delay.h>
>>>    #include <linux/reset.h>
>>>    #include <linux/regulator/consumer.h>
>>> +#include <linux/iio/buffer.h>
>>>    #include <linux/iio/iio.h>
>>> +#include <linux/iio/trigger.h>
>>> +#include <linux/iio/trigger_consumer.h>
>>> +#include <linux/iio/triggered_buffer.h>
>>>    
>>>    #define SARADC_DATA			0x00
>>>    
>>> @@ -34,7 +38,6 @@
>>>    #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;
>>> @@ -49,8 +52,37 @@ struct rockchip_saradc {
>>>    	struct reset_control	*reset;
>>>    	const struct rockchip_saradc_data *data;
>>>    	u16			last_val;
>>> +	const struct iio_chan_spec *last_chan;
>>>    };
>>>    
>>> +static void rockchip_saradc_power_down(struct rockchip_saradc *info)
>>> +{
>>> +	/* Clear irq & power down adc */
>>> +	writel_relaxed(0, info->regs + SARADC_CTRL);
>>> +}
>>> +
>>> +static int rockchip_saradc_conversion(struct rockchip_saradc *info,
>>> +				   struct iio_chan_spec const *chan)
>>> +{
>>> +	reinit_completion(&info->completion);
>>> +
>>> +	/* 8 clock periods as delay between power up and start cmd */
>>> +	writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
>>> +
>>> +	info->last_chan = chan;
>>> +
>>> +	/* Select the channel to be used and trigger conversion */
>>> +	writel(SARADC_CTRL_POWER_CTRL
>>> +			| (chan->channel & SARADC_CTRL_CHN_MASK)
>>> +			| SARADC_CTRL_IRQ_ENABLE,
>>> +		   info->regs + SARADC_CTRL);
>>> +
>>> +	if (!wait_for_completion_timeout(&info->completion, SARADC_TIMEOUT))
>>> +		return -ETIMEDOUT;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>    static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>>>    				    struct iio_chan_spec const *chan,
>>>    				    int *val, int *val2, long mask)
>>> @@ -62,24 +94,12 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>>>    	case IIO_CHAN_INFO_RAW:
>>>    		mutex_lock(&indio_dev->mlock);
>>>    
>>> -		reinit_completion(&info->completion);
>>> -
>>> -		/* 8 clock periods as delay between power up and start cmd */
>>> -		writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
>>> -
>>> -		/* Select the channel to be used and trigger conversion */
>>> -		writel(SARADC_CTRL_POWER_CTRL
>>> -				| (chan->channel & SARADC_CTRL_CHN_MASK)
>>> -				| SARADC_CTRL_IRQ_ENABLE,
>>> -		       info->regs + SARADC_CTRL);
>>> -
>>> -		if (!wait_for_completion_timeout(&info->completion,
>>> -						 SARADC_TIMEOUT)) {
>>> -			writel_relaxed(0, info->regs + SARADC_CTRL);
>>> +		ret = rockchip_saradc_conversion(info, chan);
>>> +		if (ret) {
>>> +			rockchip_saradc_power_down(info);
>>>    			mutex_unlock(&indio_dev->mlock);
>>> -			return -ETIMEDOUT;
>>> +			return ret;
>>>    		}
>>> -
>>>    		*val = info->last_val;
>>>    		mutex_unlock(&indio_dev->mlock);
>>>    		return IIO_VAL_INT;
>>> @@ -91,7 +111,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
>>>    		}
>>>    
>>>    		*val = ret / 1000;
>>> -		*val2 = info->data->num_bits;
>>> +		*val2 = chan->scan_type.realbits;
>>>    		return IIO_VAL_FRACTIONAL_LOG2;
>>>    	default:
>>>    		return -EINVAL;
>>> @@ -104,10 +124,9 @@ 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 &= GENMASK(info->data->num_bits - 1, 0);
>>> +	info->last_val &= GENMASK(info->last_chan->scan_type.realbits - 1, 0);
>>>    
>>> -	/* Clear irq & power down adc */
>>> -	writel_relaxed(0, info->regs + SARADC_CTRL);
>>> +	rockchip_saradc_power_down(info);
>>>    
>>>    	complete(&info->completion);
>>>    
>>> @@ -118,51 +137,55 @@ static const struct iio_info rockchip_saradc_iio_info = {
>>>    	.read_raw = rockchip_saradc_read_raw,
>>>    };
>>>    
>>> -#define ADC_CHANNEL(_index, _id) {				\
>>> +#define ADC_CHANNEL(_index, _id, _res) {			\
>>>    	.type = IIO_VOLTAGE,					\
>>>    	.indexed = 1,						\
>>>    	.channel = _index,					\
>>>    	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
>>>    	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
>>>    	.datasheet_name = _id,					\
>>> +	.scan_index = _index,					\
>>> +	.scan_type = {						\
>>> +		.sign = 'u',					\
>>> +		.realbits = _res,				\
>>> +		.storagebits = 16,				\
>>> +		.endianness = IIO_LE,				\
>>> +	},							\
>>>    }
>>>    
>>>    static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
>>> -	ADC_CHANNEL(0, "adc0"),
>>> -	ADC_CHANNEL(1, "adc1"),
>>> -	ADC_CHANNEL(2, "adc2"),
>>> +	ADC_CHANNEL(0, "adc0", 10),
>>> +	ADC_CHANNEL(1, "adc1", 10),
>>> +	ADC_CHANNEL(2, "adc2", 10),
>>>    };
>>>    
>>>    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"),
>>> +	ADC_CHANNEL(0, "adc0", 12),
>>> +	ADC_CHANNEL(1, "adc1", 12),
>>>    };
>>>    
>>>    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 iio_chan_spec rockchip_rk3399_saradc_iio_channels[] = {
>>> -	ADC_CHANNEL(0, "adc0"),
>>> -	ADC_CHANNEL(1, "adc1"),
>>> -	ADC_CHANNEL(2, "adc2"),
>>> -	ADC_CHANNEL(3, "adc3"),
>>> -	ADC_CHANNEL(4, "adc4"),
>>> -	ADC_CHANNEL(5, "adc5"),
>>> +	ADC_CHANNEL(0, "adc0", 10),
>>> +	ADC_CHANNEL(1, "adc1", 10),
>>> +	ADC_CHANNEL(2, "adc2", 10),
>>> +	ADC_CHANNEL(3, "adc3", 10),
>>> +	ADC_CHANNEL(4, "adc4", 10),
>>> +	ADC_CHANNEL(5, "adc5", 10),
>>>    };
>>>    
>>>    static const struct rockchip_saradc_data rk3399_saradc_data = {
>>> -	.num_bits = 10,
>>>    	.channels = rockchip_rk3399_saradc_iio_channels,
>>>    	.num_channels = ARRAY_SIZE(rockchip_rk3399_saradc_iio_channels),
>>>    	.clk_rate = 1000000,
>>> @@ -193,6 +216,39 @@ static void rockchip_saradc_reset_controller(struct reset_control *reset)
>>>    	reset_control_deassert(reset);
>>>    }
>>>    
>>> +static irqreturn_t rockchip_saradc_trigger_handler(int irq, void *p)
>>> +{
>>> +	struct iio_poll_func *pf = p;
>>> +	struct iio_dev *i_dev = pf->indio_dev;
>>> +	struct rockchip_saradc *info = iio_priv(i_dev);
>>> +	u16 data[20];
>> How about this:
>> #define MAX_CHANNEL_NUM 16
> 
> Unfortunately this is a bit more complex than it seems.
> The buffer needs to be big enough for all the channels
> + a 8 byte aligned space to put the timestamp in.
> 
> You can construct that in a fashion suitable to use in a
> macro but it's a bit more fiddly than simply being the
> maximum number of channels.
> 
Make use of iio_dev->scan_bytes to alloc a buffer in 
iio_info->update_scan_mode callback for storing the
"data + timestamp" is another way
>> u16 data[MAX_CHANNEL_NUM];
>>> +	int ret;
>>> +	int i, j = 0;
>>> +
>>> +	mutex_lock(&i_dev->mlock);
>>> +
>>> +	for_each_set_bit(i, i_dev->active_scan_mask, i_dev->masklength) {
>>> +		const struct iio_chan_spec *chan = &i_dev->channels[i];
>>> +
>>> +		ret = rockchip_saradc_conversion(info, chan);
>>> +		if (ret) {
>>> +			rockchip_saradc_power_down(info);
>>> +			goto out;
>>> +		}
>>> +
>>> +		data[j] = info->last_val;
>>> +		j++;
>>> +	}
>>> +
>>> +	iio_push_to_buffers_with_timestamp(i_dev, data, iio_get_time_ns(i_dev));
>>> +out:
>>> +	mutex_unlock(&i_dev->mlock);
>>> +
>>> +	iio_trigger_notify_done(i_dev->trig);
>>> +
>>> +	return IRQ_HANDLED;
>>> +}
>>> +
>>>    static int rockchip_saradc_probe(struct platform_device *pdev)
>>>    {
>>>    	struct rockchip_saradc *info = NULL;
>>> @@ -315,12 +371,19 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
>>>    	indio_dev->channels = info->data->channels;
>>>    	indio_dev->num_channels = info->data->num_channels;
>>>    
>>> -	ret = iio_device_register(indio_dev);
>>> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>>> +					 rockchip_saradc_trigger_handler, NULL);
>> devm_iio_triggered_buffer_setup seems better
>>>    	if (ret)
>>>    		goto err_clk;
>>>    
>>> +	ret = iio_device_register(indio_dev);
>>> +	if (ret)
>>> +		goto err_buffer_cleanup;
>>> +
>>>    	return 0;
>>>    
>>> +err_buffer_cleanup:
>>> +	iio_triggered_buffer_cleanup(indio_dev);
>>>    err_clk:
>>>    	clk_disable_unprepare(info->clk);
>>>    err_pclk:
>>>    
>> xxm@rock-chips.com
>>
>>
> 
> 
> 
> 



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

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

* Re: [PATCH] iio: adc: rockchip_saradc: Add support iio buffers【请注意,邮件由linux-rockchip-bounces+xxm=rock-chips.com@lists.infradead.org代发】
  2020-03-04  1:39       ` xxm
  (?)
@ 2020-03-07 11:42       ` Jonathan Cameron
  -1 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2020-03-07 11:42 UTC (permalink / raw)
  To: xxm
  Cc: Heiko Stuebner, lars, linux-iio, Heiko Stuebner, linux-kernel,
	kever.yang, linux-rockchip, pmeerw, knaack.h

On Wed, 4 Mar 2020 09:39:10 +0800
xxm <xxm@rock-chips.com> wrote:

> Hi,
> 
> 在 2020/3/4 4:32, Jonathan Cameron 写道:
> > On Mon, 2 Mar 2020 10:11:02 +0800
> > xxm <xxm@rock-chips.com> wrote:
> >   
> >> Hi, Heiko
> >>
> >> 在 2020/3/1 19:23, Heiko Stuebner 写道:  
> >>> From: Simon Xue <xxm@rock-chips.com>
> >>>
> >>> Add the ability to also support access via (triggered) buffers
> >>> next to the existing direct mode.
> >>>
> >>> Device in question is the Odroid Go Advance that connects a joystick
> >>> to two of the saradc channels for X and Y axis and the new (and still
> >>> pending) adc joystick driver of course wants to use triggered buffers
> >>> from the iio subsystem.
> >>>
> >>> Signed-off-by: Simon Xue <xxm@rock-chips.com>
> >>> [some simplifications and added commit description]
> >>> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com>
> >>> ---
> >>>    drivers/iio/adc/Kconfig           |   2 +
> >>>    drivers/iio/adc/rockchip_saradc.c | 137 ++++++++++++++++++++++--------
> >>>    2 files changed, 102 insertions(+), 37 deletions(-)
> >>>
> >>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> >>> index 82e33082958c..55d2499ff757 100644
> >>> --- a/drivers/iio/adc/Kconfig
> >>> +++ b/drivers/iio/adc/Kconfig
> >>> @@ -787,6 +787,8 @@ config ROCKCHIP_SARADC
> >>>    	tristate "Rockchip SARADC driver"
> >>>    	depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
> >>>    	depends on RESET_CONTROLLER
> >>> +	select IIO_BUFFER
> >>> +	select IIO_TRIGGERED_BUFFER
> >>>    	help
> >>>    	  Say yes here to build support for the SARADC found in SoCs from
> >>>    	  Rockchip.
> >>> diff --git a/drivers/iio/adc/rockchip_saradc.c b/drivers/iio/adc/rockchip_saradc.c
> >>> index 582ba047c4a6..402b2210a682 100644
> >>> --- a/drivers/iio/adc/rockchip_saradc.c
> >>> +++ b/drivers/iio/adc/rockchip_saradc.c
> >>> @@ -15,7 +15,11 @@
> >>>    #include <linux/delay.h>
> >>>    #include <linux/reset.h>
> >>>    #include <linux/regulator/consumer.h>
> >>> +#include <linux/iio/buffer.h>
> >>>    #include <linux/iio/iio.h>
> >>> +#include <linux/iio/trigger.h>
> >>> +#include <linux/iio/trigger_consumer.h>
> >>> +#include <linux/iio/triggered_buffer.h>
> >>>    
> >>>    #define SARADC_DATA			0x00
> >>>    
> >>> @@ -34,7 +38,6 @@
> >>>    #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;
> >>> @@ -49,8 +52,37 @@ struct rockchip_saradc {
> >>>    	struct reset_control	*reset;
> >>>    	const struct rockchip_saradc_data *data;
> >>>    	u16			last_val;
> >>> +	const struct iio_chan_spec *last_chan;
> >>>    };
> >>>    
> >>> +static void rockchip_saradc_power_down(struct rockchip_saradc *info)
> >>> +{
> >>> +	/* Clear irq & power down adc */
> >>> +	writel_relaxed(0, info->regs + SARADC_CTRL);
> >>> +}
> >>> +
> >>> +static int rockchip_saradc_conversion(struct rockchip_saradc *info,
> >>> +				   struct iio_chan_spec const *chan)
> >>> +{
> >>> +	reinit_completion(&info->completion);
> >>> +
> >>> +	/* 8 clock periods as delay between power up and start cmd */
> >>> +	writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
> >>> +
> >>> +	info->last_chan = chan;
> >>> +
> >>> +	/* Select the channel to be used and trigger conversion */
> >>> +	writel(SARADC_CTRL_POWER_CTRL
> >>> +			| (chan->channel & SARADC_CTRL_CHN_MASK)
> >>> +			| SARADC_CTRL_IRQ_ENABLE,
> >>> +		   info->regs + SARADC_CTRL);
> >>> +
> >>> +	if (!wait_for_completion_timeout(&info->completion, SARADC_TIMEOUT))
> >>> +		return -ETIMEDOUT;
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>>    static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
> >>>    				    struct iio_chan_spec const *chan,
> >>>    				    int *val, int *val2, long mask)
> >>> @@ -62,24 +94,12 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
> >>>    	case IIO_CHAN_INFO_RAW:
> >>>    		mutex_lock(&indio_dev->mlock);
> >>>    
> >>> -		reinit_completion(&info->completion);
> >>> -
> >>> -		/* 8 clock periods as delay between power up and start cmd */
> >>> -		writel_relaxed(8, info->regs + SARADC_DLY_PU_SOC);
> >>> -
> >>> -		/* Select the channel to be used and trigger conversion */
> >>> -		writel(SARADC_CTRL_POWER_CTRL
> >>> -				| (chan->channel & SARADC_CTRL_CHN_MASK)
> >>> -				| SARADC_CTRL_IRQ_ENABLE,
> >>> -		       info->regs + SARADC_CTRL);
> >>> -
> >>> -		if (!wait_for_completion_timeout(&info->completion,
> >>> -						 SARADC_TIMEOUT)) {
> >>> -			writel_relaxed(0, info->regs + SARADC_CTRL);
> >>> +		ret = rockchip_saradc_conversion(info, chan);
> >>> +		if (ret) {
> >>> +			rockchip_saradc_power_down(info);
> >>>    			mutex_unlock(&indio_dev->mlock);
> >>> -			return -ETIMEDOUT;
> >>> +			return ret;
> >>>    		}
> >>> -
> >>>    		*val = info->last_val;
> >>>    		mutex_unlock(&indio_dev->mlock);
> >>>    		return IIO_VAL_INT;
> >>> @@ -91,7 +111,7 @@ static int rockchip_saradc_read_raw(struct iio_dev *indio_dev,
> >>>    		}
> >>>    
> >>>    		*val = ret / 1000;
> >>> -		*val2 = info->data->num_bits;
> >>> +		*val2 = chan->scan_type.realbits;
> >>>    		return IIO_VAL_FRACTIONAL_LOG2;
> >>>    	default:
> >>>    		return -EINVAL;
> >>> @@ -104,10 +124,9 @@ 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 &= GENMASK(info->data->num_bits - 1, 0);
> >>> +	info->last_val &= GENMASK(info->last_chan->scan_type.realbits - 1, 0);
> >>>    
> >>> -	/* Clear irq & power down adc */
> >>> -	writel_relaxed(0, info->regs + SARADC_CTRL);
> >>> +	rockchip_saradc_power_down(info);
> >>>    
> >>>    	complete(&info->completion);
> >>>    
> >>> @@ -118,51 +137,55 @@ static const struct iio_info rockchip_saradc_iio_info = {
> >>>    	.read_raw = rockchip_saradc_read_raw,
> >>>    };
> >>>    
> >>> -#define ADC_CHANNEL(_index, _id) {				\
> >>> +#define ADC_CHANNEL(_index, _id, _res) {			\
> >>>    	.type = IIO_VOLTAGE,					\
> >>>    	.indexed = 1,						\
> >>>    	.channel = _index,					\
> >>>    	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> >>>    	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),	\
> >>>    	.datasheet_name = _id,					\
> >>> +	.scan_index = _index,					\
> >>> +	.scan_type = {						\
> >>> +		.sign = 'u',					\
> >>> +		.realbits = _res,				\
> >>> +		.storagebits = 16,				\
> >>> +		.endianness = IIO_LE,				\
> >>> +	},							\
> >>>    }
> >>>    
> >>>    static const struct iio_chan_spec rockchip_saradc_iio_channels[] = {
> >>> -	ADC_CHANNEL(0, "adc0"),
> >>> -	ADC_CHANNEL(1, "adc1"),
> >>> -	ADC_CHANNEL(2, "adc2"),
> >>> +	ADC_CHANNEL(0, "adc0", 10),
> >>> +	ADC_CHANNEL(1, "adc1", 10),
> >>> +	ADC_CHANNEL(2, "adc2", 10),
> >>>    };
> >>>    
> >>>    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"),
> >>> +	ADC_CHANNEL(0, "adc0", 12),
> >>> +	ADC_CHANNEL(1, "adc1", 12),
> >>>    };
> >>>    
> >>>    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 iio_chan_spec rockchip_rk3399_saradc_iio_channels[] = {
> >>> -	ADC_CHANNEL(0, "adc0"),
> >>> -	ADC_CHANNEL(1, "adc1"),
> >>> -	ADC_CHANNEL(2, "adc2"),
> >>> -	ADC_CHANNEL(3, "adc3"),
> >>> -	ADC_CHANNEL(4, "adc4"),
> >>> -	ADC_CHANNEL(5, "adc5"),
> >>> +	ADC_CHANNEL(0, "adc0", 10),
> >>> +	ADC_CHANNEL(1, "adc1", 10),
> >>> +	ADC_CHANNEL(2, "adc2", 10),
> >>> +	ADC_CHANNEL(3, "adc3", 10),
> >>> +	ADC_CHANNEL(4, "adc4", 10),
> >>> +	ADC_CHANNEL(5, "adc5", 10),
> >>>    };
> >>>    
> >>>    static const struct rockchip_saradc_data rk3399_saradc_data = {
> >>> -	.num_bits = 10,
> >>>    	.channels = rockchip_rk3399_saradc_iio_channels,
> >>>    	.num_channels = ARRAY_SIZE(rockchip_rk3399_saradc_iio_channels),
> >>>    	.clk_rate = 1000000,
> >>> @@ -193,6 +216,39 @@ static void rockchip_saradc_reset_controller(struct reset_control *reset)
> >>>    	reset_control_deassert(reset);
> >>>    }
> >>>    
> >>> +static irqreturn_t rockchip_saradc_trigger_handler(int irq, void *p)
> >>> +{
> >>> +	struct iio_poll_func *pf = p;
> >>> +	struct iio_dev *i_dev = pf->indio_dev;
> >>> +	struct rockchip_saradc *info = iio_priv(i_dev);
> >>> +	u16 data[20];  
> >> How about this:
> >> #define MAX_CHANNEL_NUM 16  
> > 
> > Unfortunately this is a bit more complex than it seems.
> > The buffer needs to be big enough for all the channels
> > + a 8 byte aligned space to put the timestamp in.
> > 
> > You can construct that in a fashion suitable to use in a
> > macro but it's a bit more fiddly than simply being the
> > maximum number of channels.
> >   
> Make use of iio_dev->scan_bytes to alloc a buffer in 
> iio_info->update_scan_mode callback for storing the
> "data + timestamp" is another way

Please don't do that. scan_bytes is marked as [INTERN] in
the docs in iio.h.   The reason for this is it is an internal
implementation detail.  Drivers should not be touching it.
We never provided any sort of utility function because it's normally
really easy for a driver to establish an upper bound for itself.

Jonathan


> >> u16 data[MAX_CHANNEL_NUM];  
> >>> +	int ret;
> >>> +	int i, j = 0;
> >>> +
> >>> +	mutex_lock(&i_dev->mlock);
> >>> +
> >>> +	for_each_set_bit(i, i_dev->active_scan_mask, i_dev->masklength) {
> >>> +		const struct iio_chan_spec *chan = &i_dev->channels[i];
> >>> +
> >>> +		ret = rockchip_saradc_conversion(info, chan);
> >>> +		if (ret) {
> >>> +			rockchip_saradc_power_down(info);
> >>> +			goto out;
> >>> +		}
> >>> +
> >>> +		data[j] = info->last_val;
> >>> +		j++;
> >>> +	}
> >>> +
> >>> +	iio_push_to_buffers_with_timestamp(i_dev, data, iio_get_time_ns(i_dev));
> >>> +out:
> >>> +	mutex_unlock(&i_dev->mlock);
> >>> +
> >>> +	iio_trigger_notify_done(i_dev->trig);
> >>> +
> >>> +	return IRQ_HANDLED;
> >>> +}
> >>> +
> >>>    static int rockchip_saradc_probe(struct platform_device *pdev)
> >>>    {
> >>>    	struct rockchip_saradc *info = NULL;
> >>> @@ -315,12 +371,19 @@ static int rockchip_saradc_probe(struct platform_device *pdev)
> >>>    	indio_dev->channels = info->data->channels;
> >>>    	indio_dev->num_channels = info->data->num_channels;
> >>>    
> >>> -	ret = iio_device_register(indio_dev);
> >>> +	ret = iio_triggered_buffer_setup(indio_dev, NULL,
> >>> +					 rockchip_saradc_trigger_handler, NULL);  
> >> devm_iio_triggered_buffer_setup seems better  
> >>>    	if (ret)
> >>>    		goto err_clk;
> >>>    
> >>> +	ret = iio_device_register(indio_dev);
> >>> +	if (ret)
> >>> +		goto err_buffer_cleanup;
> >>> +
> >>>    	return 0;
> >>>    
> >>> +err_buffer_cleanup:
> >>> +	iio_triggered_buffer_cleanup(indio_dev);
> >>>    err_clk:
> >>>    	clk_disable_unprepare(info->clk);
> >>>    err_pclk:
> >>>      
> >> xxm@rock-chips.com
> >>
> >>  
> > 
> > 
> > 
> >   
> 
> 


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

end of thread, other threads:[~2020-03-07 11:42 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-01 11:23 [PATCH] iio: adc: rockchip_saradc: Add support iio buffers Heiko Stuebner
2020-03-02  2:11 ` [PATCH] iio: adc: rockchip_saradc: Add support iio buffers【请注意,邮件由linux-rockchip-bounces+xxm=rock-chips.com@lists.infradead.org代发】 xxm
2020-03-02  2:11   ` xxm
2020-03-03 20:32   ` Jonathan Cameron
2020-03-03 20:32     ` Jonathan Cameron
2020-03-04  1:39     ` xxm
2020-03-04  1:39       ` xxm
2020-03-07 11:42       ` 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.