All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] iio: adc: vf610: respect ADC clocking limitations
@ 2015-03-24 12:47 ` Stefan Agner
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Agner @ 2015-03-24 12:47 UTC (permalink / raw)
  To: jic23, shawn.guo, kernel
  Cc: knaack.h, lars, pmeerw, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, B38611, maitysanchayan, devicetree,
	linux-iio, linux-arm-kernel, linux-kernel, Stefan Agner

Respect ADC clocking limitations which lead to bogous reading on
500MHz clocked Vybrid SoC's. Additionally, also implement a
sysfs-property to configure the conversion mode available in this
ADC peripherial.

The clock limitations are specified using the device tree, hence
I seek an Ack from the device tree maintainers here...

Changes since v3:
- Move device tree bindings to driver changes

Changes since v2:
- Add sysfs ABI documentation
- Fix commit message spelling errors

Changes since v1:
- Use ext_info for conversion mode


Stefan Agner (3):
  iio: adc: vf610: use ADC clock within specification
  iio: adc: vf610: implement configurable conversion modes
  ARM: dts: add property for maximum ADC clock frequencies

 Documentation/ABI/testing/sysfs-bus-iio-vf610      |   7 +
 .../devicetree/bindings/iio/adc/vf610-adc.txt      |   9 +
 arch/arm/boot/dts/vfxxx.dtsi                       |   4 +
 drivers/iio/adc/vf610_adc.c                        | 225 +++++++++++++++------
 4 files changed, 179 insertions(+), 66 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-vf610

-- 
2.3.3


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

* [PATCH v4 0/3] iio: adc: vf610: respect ADC clocking limitations
@ 2015-03-24 12:47 ` Stefan Agner
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Agner @ 2015-03-24 12:47 UTC (permalink / raw)
  To: jic23-DgEjT+Ai2ygdnm+yROfE0A, shawn.guo-QSEj5FYQhm4dnm+yROfE0A,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ
  Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
	pmeerw-jW+XmwGofnusTnJN9+BGXg, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, B38611-KZfg59tc24xl57MIdRCFDg,
	maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stefan Agner

Respect ADC clocking limitations which lead to bogous reading on
500MHz clocked Vybrid SoC's. Additionally, also implement a
sysfs-property to configure the conversion mode available in this
ADC peripherial.

The clock limitations are specified using the device tree, hence
I seek an Ack from the device tree maintainers here...

Changes since v3:
- Move device tree bindings to driver changes

Changes since v2:
- Add sysfs ABI documentation
- Fix commit message spelling errors

Changes since v1:
- Use ext_info for conversion mode


Stefan Agner (3):
  iio: adc: vf610: use ADC clock within specification
  iio: adc: vf610: implement configurable conversion modes
  ARM: dts: add property for maximum ADC clock frequencies

 Documentation/ABI/testing/sysfs-bus-iio-vf610      |   7 +
 .../devicetree/bindings/iio/adc/vf610-adc.txt      |   9 +
 arch/arm/boot/dts/vfxxx.dtsi                       |   4 +
 drivers/iio/adc/vf610_adc.c                        | 225 +++++++++++++++------
 4 files changed, 179 insertions(+), 66 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-vf610

-- 
2.3.3

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

* [PATCH v4 0/3] iio: adc: vf610: respect ADC clocking limitations
@ 2015-03-24 12:47 ` Stefan Agner
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Agner @ 2015-03-24 12:47 UTC (permalink / raw)
  To: linux-arm-kernel

Respect ADC clocking limitations which lead to bogous reading on
500MHz clocked Vybrid SoC's. Additionally, also implement a
sysfs-property to configure the conversion mode available in this
ADC peripherial.

The clock limitations are specified using the device tree, hence
I seek an Ack from the device tree maintainers here...

Changes since v3:
- Move device tree bindings to driver changes

Changes since v2:
- Add sysfs ABI documentation
- Fix commit message spelling errors

Changes since v1:
- Use ext_info for conversion mode


Stefan Agner (3):
  iio: adc: vf610: use ADC clock within specification
  iio: adc: vf610: implement configurable conversion modes
  ARM: dts: add property for maximum ADC clock frequencies

 Documentation/ABI/testing/sysfs-bus-iio-vf610      |   7 +
 .../devicetree/bindings/iio/adc/vf610-adc.txt      |   9 +
 arch/arm/boot/dts/vfxxx.dtsi                       |   4 +
 drivers/iio/adc/vf610_adc.c                        | 225 +++++++++++++++------
 4 files changed, 179 insertions(+), 66 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-vf610

-- 
2.3.3

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

* [PATCH v4 1/3] iio: adc: vf610: use ADC clock within specification
  2015-03-24 12:47 ` Stefan Agner
@ 2015-03-24 12:47   ` Stefan Agner
  -1 siblings, 0 replies; 18+ messages in thread
From: Stefan Agner @ 2015-03-24 12:47 UTC (permalink / raw)
  To: jic23, shawn.guo, kernel
  Cc: knaack.h, lars, pmeerw, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, B38611, maitysanchayan, devicetree,
	linux-iio, linux-arm-kernel, linux-kernel, Stefan Agner

Depending on conversion mode used, the ADC clock (ADCK) needs
to be below a maximum frequency. According to Vybrid's data
sheet this is 20MHz for the low power conversion mode.

The ADC clock is depending on input clock, which is the bus
clock by default. Vybrid SoC are typically clocked at at 400MHz
or 500MHz, which leads to 66MHz or 83MHz bus clock respectively.
Hence, a divider of 8 is required to stay below the specified
maximum clock of 20MHz.

Due to the different bus clock speeds, the resulting sampling
frequency is not static. Hence use the ADC clock and calculate
the actual available sampling frequency dynamically.

This fixes bogous values observed on some 500MHz clocked Vybrid
SoC. The resulting value usually showed Bit 9 being stuck at 1,
or 0, which lead to a value of +/-512.

Acked-by: Fugang Duan <B38611@freescale.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/iio/adc/vf610_adc.c | 91 ++++++++++++++++++++++++++++++---------------
 1 file changed, 61 insertions(+), 30 deletions(-)

diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
index 8ec353c..e63b8e7 100644
--- a/drivers/iio/adc/vf610_adc.c
+++ b/drivers/iio/adc/vf610_adc.c
@@ -141,9 +141,13 @@ struct vf610_adc {
 	struct regulator *vref;
 	struct vf610_adc_feature adc_feature;
 
+	u32 sample_freq_avail[5];
+
 	struct completion completion;
 };
 
+static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
+
 #define VF610_ADC_CHAN(_idx, _chan_type) {			\
 	.type = (_chan_type),					\
 	.indexed = 1,						\
@@ -180,35 +184,47 @@ static const struct iio_chan_spec vf610_adc_iio_channels[] = {
 	/* sentinel */
 };
 
-/*
- * ADC sample frequency, unit is ADCK cycles.
- * ADC clk source is ipg clock, which is the same as bus clock.
- *
- * ADC conversion time = SFCAdder + AverageNum x (BCT + LSTAdder)
- * SFCAdder: fixed to 6 ADCK cycles
- * AverageNum: 1, 4, 8, 16, 32 samples for hardware average.
- * BCT (Base Conversion Time): fixed to 25 ADCK cycles for 12 bit mode
- * LSTAdder(Long Sample Time): fixed to 3 ADCK cycles
- *
- * By default, enable 12 bit resolution mode, clock source
- * set to ipg clock, So get below frequency group:
- */
-static const u32 vf610_sample_freq_avail[5] =
-{1941176, 559332, 286957, 145374, 73171};
+static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
+{
+	unsigned long adck_rate, ipg_rate = clk_get_rate(info->clk);
+	int i;
+
+	/*
+	 * Calculate ADC sample frequencies
+	 * Sample time unit is ADCK cycles. ADCK clk source is ipg clock,
+	 * which is the same as bus clock.
+	 *
+	 * ADC conversion time = SFCAdder + AverageNum x (BCT + LSTAdder)
+	 * SFCAdder: fixed to 6 ADCK cycles
+	 * AverageNum: 1, 4, 8, 16, 32 samples for hardware average.
+	 * BCT (Base Conversion Time): fixed to 25 ADCK cycles for 12 bit mode
+	 * LSTAdder(Long Sample Time): fixed to 3 ADCK cycles
+	 */
+	adck_rate = ipg_rate / info->adc_feature.clk_div;
+	for (i = 0; i < ARRAY_SIZE(vf610_hw_avgs); i++)
+		info->sample_freq_avail[i] =
+			adck_rate / (6 + vf610_hw_avgs[i] * (25 + 3));
+}
 
 static inline void vf610_adc_cfg_init(struct vf610_adc *info)
 {
+	struct vf610_adc_feature *adc_feature = &info->adc_feature;
+
 	/* set default Configuration for ADC controller */
-	info->adc_feature.clk_sel = VF610_ADCIOC_BUSCLK_SET;
-	info->adc_feature.vol_ref = VF610_ADCIOC_VR_VREF_SET;
+	adc_feature->clk_sel = VF610_ADCIOC_BUSCLK_SET;
+	adc_feature->vol_ref = VF610_ADCIOC_VR_VREF_SET;
+
+	adc_feature->calibration = true;
+	adc_feature->ovwren = true;
+
+	adc_feature->res_mode = 12;
+	adc_feature->sample_rate = 1;
+	adc_feature->lpm = true;
 
-	info->adc_feature.calibration = true;
-	info->adc_feature.ovwren = true;
+	/* Use a save ADCK which is below 20MHz on all devices */
+	adc_feature->clk_div = 8;
 
-	info->adc_feature.clk_div = 1;
-	info->adc_feature.res_mode = 12;
-	info->adc_feature.sample_rate = 1;
-	info->adc_feature.lpm = true;
+	vf610_adc_calculate_rates(info);
 }
 
 static void vf610_adc_cfg_post_set(struct vf610_adc *info)
@@ -290,12 +306,10 @@ static void vf610_adc_cfg_set(struct vf610_adc *info)
 
 	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
 
-	/* low power configuration */
 	cfg_data &= ~VF610_ADC_ADLPC_EN;
 	if (adc_feature->lpm)
 		cfg_data |= VF610_ADC_ADLPC_EN;
 
-	/* disable high speed */
 	cfg_data &= ~VF610_ADC_ADHSC_EN;
 
 	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
@@ -435,10 +449,27 @@ static irqreturn_t vf610_adc_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("1941176, 559332, 286957, 145374, 73171");
+static ssize_t vf610_show_samp_freq_avail(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct vf610_adc *info = iio_priv(dev_to_iio_dev(dev));
+	size_t len = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(info->sample_freq_avail); i++)
+		len += scnprintf(buf + len, PAGE_SIZE - len,
+			"%u ", info->sample_freq_avail[i]);
+
+	/* replace trailing space by newline */
+	buf[len - 1] = '\n';
+
+	return len;
+}
+
+static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(vf610_show_samp_freq_avail);
 
 static struct attribute *vf610_attributes[] = {
-	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
+	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
 	NULL
 };
 
@@ -502,7 +533,7 @@ static int vf610_read_raw(struct iio_dev *indio_dev,
 		return IIO_VAL_FRACTIONAL_LOG2;
 
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		*val = vf610_sample_freq_avail[info->adc_feature.sample_rate];
+		*val = info->sample_freq_avail[info->adc_feature.sample_rate];
 		*val2 = 0;
 		return IIO_VAL_INT;
 
@@ -525,9 +556,9 @@ static int vf610_write_raw(struct iio_dev *indio_dev,
 	switch (mask) {
 		case IIO_CHAN_INFO_SAMP_FREQ:
 			for (i = 0;
-				i < ARRAY_SIZE(vf610_sample_freq_avail);
+				i < ARRAY_SIZE(info->sample_freq_avail);
 				i++)
-				if (val == vf610_sample_freq_avail[i]) {
+				if (val == info->sample_freq_avail[i]) {
 					info->adc_feature.sample_rate = i;
 					vf610_adc_sample_set(info);
 					return 0;
-- 
2.3.3


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

* [PATCH v4 1/3] iio: adc: vf610: use ADC clock within specification
@ 2015-03-24 12:47   ` Stefan Agner
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Agner @ 2015-03-24 12:47 UTC (permalink / raw)
  To: linux-arm-kernel

Depending on conversion mode used, the ADC clock (ADCK) needs
to be below a maximum frequency. According to Vybrid's data
sheet this is 20MHz for the low power conversion mode.

The ADC clock is depending on input clock, which is the bus
clock by default. Vybrid SoC are typically clocked at at 400MHz
or 500MHz, which leads to 66MHz or 83MHz bus clock respectively.
Hence, a divider of 8 is required to stay below the specified
maximum clock of 20MHz.

Due to the different bus clock speeds, the resulting sampling
frequency is not static. Hence use the ADC clock and calculate
the actual available sampling frequency dynamically.

This fixes bogous values observed on some 500MHz clocked Vybrid
SoC. The resulting value usually showed Bit 9 being stuck at 1,
or 0, which lead to a value of +/-512.

Acked-by: Fugang Duan <B38611@freescale.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/iio/adc/vf610_adc.c | 91 ++++++++++++++++++++++++++++++---------------
 1 file changed, 61 insertions(+), 30 deletions(-)

diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
index 8ec353c..e63b8e7 100644
--- a/drivers/iio/adc/vf610_adc.c
+++ b/drivers/iio/adc/vf610_adc.c
@@ -141,9 +141,13 @@ struct vf610_adc {
 	struct regulator *vref;
 	struct vf610_adc_feature adc_feature;
 
+	u32 sample_freq_avail[5];
+
 	struct completion completion;
 };
 
+static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
+
 #define VF610_ADC_CHAN(_idx, _chan_type) {			\
 	.type = (_chan_type),					\
 	.indexed = 1,						\
@@ -180,35 +184,47 @@ static const struct iio_chan_spec vf610_adc_iio_channels[] = {
 	/* sentinel */
 };
 
-/*
- * ADC sample frequency, unit is ADCK cycles.
- * ADC clk source is ipg clock, which is the same as bus clock.
- *
- * ADC conversion time = SFCAdder + AverageNum x (BCT + LSTAdder)
- * SFCAdder: fixed to 6 ADCK cycles
- * AverageNum: 1, 4, 8, 16, 32 samples for hardware average.
- * BCT (Base Conversion Time): fixed to 25 ADCK cycles for 12 bit mode
- * LSTAdder(Long Sample Time): fixed to 3 ADCK cycles
- *
- * By default, enable 12 bit resolution mode, clock source
- * set to ipg clock, So get below frequency group:
- */
-static const u32 vf610_sample_freq_avail[5] =
-{1941176, 559332, 286957, 145374, 73171};
+static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
+{
+	unsigned long adck_rate, ipg_rate = clk_get_rate(info->clk);
+	int i;
+
+	/*
+	 * Calculate ADC sample frequencies
+	 * Sample time unit is ADCK cycles. ADCK clk source is ipg clock,
+	 * which is the same as bus clock.
+	 *
+	 * ADC conversion time = SFCAdder + AverageNum x (BCT + LSTAdder)
+	 * SFCAdder: fixed to 6 ADCK cycles
+	 * AverageNum: 1, 4, 8, 16, 32 samples for hardware average.
+	 * BCT (Base Conversion Time): fixed to 25 ADCK cycles for 12 bit mode
+	 * LSTAdder(Long Sample Time): fixed to 3 ADCK cycles
+	 */
+	adck_rate = ipg_rate / info->adc_feature.clk_div;
+	for (i = 0; i < ARRAY_SIZE(vf610_hw_avgs); i++)
+		info->sample_freq_avail[i] =
+			adck_rate / (6 + vf610_hw_avgs[i] * (25 + 3));
+}
 
 static inline void vf610_adc_cfg_init(struct vf610_adc *info)
 {
+	struct vf610_adc_feature *adc_feature = &info->adc_feature;
+
 	/* set default Configuration for ADC controller */
-	info->adc_feature.clk_sel = VF610_ADCIOC_BUSCLK_SET;
-	info->adc_feature.vol_ref = VF610_ADCIOC_VR_VREF_SET;
+	adc_feature->clk_sel = VF610_ADCIOC_BUSCLK_SET;
+	adc_feature->vol_ref = VF610_ADCIOC_VR_VREF_SET;
+
+	adc_feature->calibration = true;
+	adc_feature->ovwren = true;
+
+	adc_feature->res_mode = 12;
+	adc_feature->sample_rate = 1;
+	adc_feature->lpm = true;
 
-	info->adc_feature.calibration = true;
-	info->adc_feature.ovwren = true;
+	/* Use a save ADCK which is below 20MHz on all devices */
+	adc_feature->clk_div = 8;
 
-	info->adc_feature.clk_div = 1;
-	info->adc_feature.res_mode = 12;
-	info->adc_feature.sample_rate = 1;
-	info->adc_feature.lpm = true;
+	vf610_adc_calculate_rates(info);
 }
 
 static void vf610_adc_cfg_post_set(struct vf610_adc *info)
@@ -290,12 +306,10 @@ static void vf610_adc_cfg_set(struct vf610_adc *info)
 
 	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
 
-	/* low power configuration */
 	cfg_data &= ~VF610_ADC_ADLPC_EN;
 	if (adc_feature->lpm)
 		cfg_data |= VF610_ADC_ADLPC_EN;
 
-	/* disable high speed */
 	cfg_data &= ~VF610_ADC_ADHSC_EN;
 
 	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
@@ -435,10 +449,27 @@ static irqreturn_t vf610_adc_isr(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("1941176, 559332, 286957, 145374, 73171");
+static ssize_t vf610_show_samp_freq_avail(struct device *dev,
+				struct device_attribute *attr, char *buf)
+{
+	struct vf610_adc *info = iio_priv(dev_to_iio_dev(dev));
+	size_t len = 0;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(info->sample_freq_avail); i++)
+		len += scnprintf(buf + len, PAGE_SIZE - len,
+			"%u ", info->sample_freq_avail[i]);
+
+	/* replace trailing space by newline */
+	buf[len - 1] = '\n';
+
+	return len;
+}
+
+static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(vf610_show_samp_freq_avail);
 
 static struct attribute *vf610_attributes[] = {
-	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
+	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
 	NULL
 };
 
@@ -502,7 +533,7 @@ static int vf610_read_raw(struct iio_dev *indio_dev,
 		return IIO_VAL_FRACTIONAL_LOG2;
 
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		*val = vf610_sample_freq_avail[info->adc_feature.sample_rate];
+		*val = info->sample_freq_avail[info->adc_feature.sample_rate];
 		*val2 = 0;
 		return IIO_VAL_INT;
 
@@ -525,9 +556,9 @@ static int vf610_write_raw(struct iio_dev *indio_dev,
 	switch (mask) {
 		case IIO_CHAN_INFO_SAMP_FREQ:
 			for (i = 0;
-				i < ARRAY_SIZE(vf610_sample_freq_avail);
+				i < ARRAY_SIZE(info->sample_freq_avail);
 				i++)
-				if (val == vf610_sample_freq_avail[i]) {
+				if (val == info->sample_freq_avail[i]) {
 					info->adc_feature.sample_rate = i;
 					vf610_adc_sample_set(info);
 					return 0;
-- 
2.3.3

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

* [PATCH v4 2/3] iio: adc: vf610: implement configurable conversion modes
  2015-03-24 12:47 ` Stefan Agner
@ 2015-03-24 12:47   ` Stefan Agner
  -1 siblings, 0 replies; 18+ messages in thread
From: Stefan Agner @ 2015-03-24 12:47 UTC (permalink / raw)
  To: jic23, shawn.guo, kernel
  Cc: knaack.h, lars, pmeerw, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, B38611, maitysanchayan, devicetree,
	linux-iio, linux-arm-kernel, linux-kernel, Stefan Agner

Support configurable conversion mode through sysfs. So far, the
mode used was low-power, which is enabled by default now. Beside
that, the modes normal and high-speed are selectable as well.

Use the new device tree property which specifies the maximum ADC
conversion clock frequencies. Depending on the mode used, the
available resulting conversion frequency are calculated
dynamically.

Acked-by: Fugang Duan <B38611@freescale.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 Documentation/ABI/testing/sysfs-bus-iio-vf610      |   7 +
 .../devicetree/bindings/iio/adc/vf610-adc.txt      |   9 ++
 drivers/iio/adc/vf610_adc.c                        | 146 +++++++++++++++------
 3 files changed, 120 insertions(+), 42 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-vf610

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-vf610 b/Documentation/ABI/testing/sysfs-bus-iio-vf610
new file mode 100644
index 0000000..85fd0ecd5
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-vf610
@@ -0,0 +1,7 @@
+What:		/sys/bus/iio/devices/iio:deviceX/conversion_mode
+KernelVersion:	4.1
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Specifies the hardware conversion mode used. The three
+		available modes are "normal", "high-speed" and "low-power",
+		whereas the last is the default mode.
diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
index 1a4a43d..3eb40e2 100644
--- a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
@@ -11,6 +11,13 @@ Required properties:
 - clock-names: Must contain "adc", matching entry in the clocks property.
 - vref-supply: The regulator supply ADC reference voltage.
 
+Recommended properties:
+- fsl,adck-max-frequency: Maximum frequencies according to datasheets operating
+  requirements. Three values are required, depending on conversion mode:
+  - Frequency in normal mode (ADLPC=0, ADHSC=0)
+  - Frequency in high-speed mode (ADLPC=0, ADHSC=1)
+  - Frequency in low-power mode (ADLPC=1, ADHSC=0)
+
 Example:
 adc0: adc@4003b000 {
 	compatible = "fsl,vf610-adc";
@@ -18,5 +25,7 @@ adc0: adc@4003b000 {
 	interrupts = <0 53 0x04>;
 	clocks = <&clks VF610_CLK_ADC0>;
 	clock-names = "adc";
+	fsl,adck-max-frequency = <30000000>, <40000000>,
+				<20000000>;
 	vref-supply = <&reg_vcc_3v3_mcu>;
 };
diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
index e63b8e7..b5f94ab8 100644
--- a/drivers/iio/adc/vf610_adc.c
+++ b/drivers/iio/adc/vf610_adc.c
@@ -118,15 +118,21 @@ enum average_sel {
 	VF610_ADC_SAMPLE_32,
 };
 
+enum conversion_mode_sel {
+	VF610_ADC_CONV_NORMAL,
+	VF610_ADC_CONV_HIGH_SPEED,
+	VF610_ADC_CONV_LOW_POWER,
+};
+
 struct vf610_adc_feature {
 	enum clk_sel	clk_sel;
 	enum vol_ref	vol_ref;
+	enum conversion_mode_sel conv_mode;
 
 	int	clk_div;
 	int     sample_rate;
 	int	res_mode;
 
-	bool	lpm;
 	bool	calibration;
 	bool	ovwren;
 };
@@ -139,6 +145,8 @@ struct vf610_adc {
 	u32 vref_uv;
 	u32 value;
 	struct regulator *vref;
+
+	u32 max_adck_rate[3];
 	struct vf610_adc_feature adc_feature;
 
 	u32 sample_freq_avail[5];
@@ -148,46 +156,22 @@ struct vf610_adc {
 
 static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
 
-#define VF610_ADC_CHAN(_idx, _chan_type) {			\
-	.type = (_chan_type),					\
-	.indexed = 1,						\
-	.channel = (_idx),					\
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
-	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
-				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
-}
-
-#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
-	.type = (_chan_type),	\
-	.channel = (_idx),		\
-	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
-}
-
-static const struct iio_chan_spec vf610_adc_iio_channels[] = {
-	VF610_ADC_CHAN(0, IIO_VOLTAGE),
-	VF610_ADC_CHAN(1, IIO_VOLTAGE),
-	VF610_ADC_CHAN(2, IIO_VOLTAGE),
-	VF610_ADC_CHAN(3, IIO_VOLTAGE),
-	VF610_ADC_CHAN(4, IIO_VOLTAGE),
-	VF610_ADC_CHAN(5, IIO_VOLTAGE),
-	VF610_ADC_CHAN(6, IIO_VOLTAGE),
-	VF610_ADC_CHAN(7, IIO_VOLTAGE),
-	VF610_ADC_CHAN(8, IIO_VOLTAGE),
-	VF610_ADC_CHAN(9, IIO_VOLTAGE),
-	VF610_ADC_CHAN(10, IIO_VOLTAGE),
-	VF610_ADC_CHAN(11, IIO_VOLTAGE),
-	VF610_ADC_CHAN(12, IIO_VOLTAGE),
-	VF610_ADC_CHAN(13, IIO_VOLTAGE),
-	VF610_ADC_CHAN(14, IIO_VOLTAGE),
-	VF610_ADC_CHAN(15, IIO_VOLTAGE),
-	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
-	/* sentinel */
-};
-
 static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
 {
+	struct vf610_adc_feature *adc_feature = &info->adc_feature;
 	unsigned long adck_rate, ipg_rate = clk_get_rate(info->clk);
-	int i;
+	int divisor, i;
+
+	adck_rate = info->max_adck_rate[adc_feature->conv_mode];
+
+	if (adck_rate) {
+		/* calculate clk divider which is within specification */
+		divisor = ipg_rate / adck_rate;
+		adc_feature->clk_div = 1 << fls(divisor + 1);
+	} else {
+		/* fall-back value using a safe divisor */
+		adc_feature->clk_div = 8;
+	}
 
 	/*
 	 * Calculate ADC sample frequencies
@@ -219,10 +203,8 @@ static inline void vf610_adc_cfg_init(struct vf610_adc *info)
 
 	adc_feature->res_mode = 12;
 	adc_feature->sample_rate = 1;
-	adc_feature->lpm = true;
 
-	/* Use a save ADCK which is below 20MHz on all devices */
-	adc_feature->clk_div = 8;
+	adc_feature->conv_mode = VF610_ADC_CONV_LOW_POWER;
 
 	vf610_adc_calculate_rates(info);
 }
@@ -307,10 +289,12 @@ static void vf610_adc_cfg_set(struct vf610_adc *info)
 	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
 
 	cfg_data &= ~VF610_ADC_ADLPC_EN;
-	if (adc_feature->lpm)
+	if (adc_feature->conv_mode == VF610_ADC_CONV_LOW_POWER)
 		cfg_data |= VF610_ADC_ADLPC_EN;
 
 	cfg_data &= ~VF610_ADC_ADHSC_EN;
+	if (adc_feature->conv_mode == VF610_ADC_CONV_HIGH_SPEED)
+		cfg_data |= VF610_ADC_ADHSC_EN;
 
 	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
 }
@@ -412,6 +396,81 @@ static void vf610_adc_hw_init(struct vf610_adc *info)
 	vf610_adc_cfg_set(info);
 }
 
+static int vf610_set_conversion_mode(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     unsigned int mode)
+{
+	struct vf610_adc *info = iio_priv(indio_dev);
+
+	mutex_lock(&indio_dev->mlock);
+	info->adc_feature.conv_mode = mode;
+	vf610_adc_calculate_rates(info);
+	vf610_adc_hw_init(info);
+	mutex_unlock(&indio_dev->mlock);
+
+	return 0;
+}
+
+static int vf610_get_conversion_mode(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan)
+{
+	struct vf610_adc *info = iio_priv(indio_dev);
+
+	return info->adc_feature.conv_mode;
+}
+
+static const char * const vf610_conv_modes[] = { "normal", "high-speed",
+						 "low-power" };
+
+static const struct iio_enum vf610_conversion_mode = {
+	.items = vf610_conv_modes,
+	.num_items = ARRAY_SIZE(vf610_conv_modes),
+	.get = vf610_get_conversion_mode,
+	.set = vf610_set_conversion_mode,
+};
+
+static const struct iio_chan_spec_ext_info vf610_ext_info[] = {
+	IIO_ENUM("conversion_mode", IIO_SHARED_BY_DIR, &vf610_conversion_mode),
+	{},
+};
+
+#define VF610_ADC_CHAN(_idx, _chan_type) {			\
+	.type = (_chan_type),					\
+	.indexed = 1,						\
+	.channel = (_idx),					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
+				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.ext_info = vf610_ext_info,				\
+}
+
+#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
+	.type = (_chan_type),	\
+	.channel = (_idx),		\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
+}
+
+static const struct iio_chan_spec vf610_adc_iio_channels[] = {
+	VF610_ADC_CHAN(0, IIO_VOLTAGE),
+	VF610_ADC_CHAN(1, IIO_VOLTAGE),
+	VF610_ADC_CHAN(2, IIO_VOLTAGE),
+	VF610_ADC_CHAN(3, IIO_VOLTAGE),
+	VF610_ADC_CHAN(4, IIO_VOLTAGE),
+	VF610_ADC_CHAN(5, IIO_VOLTAGE),
+	VF610_ADC_CHAN(6, IIO_VOLTAGE),
+	VF610_ADC_CHAN(7, IIO_VOLTAGE),
+	VF610_ADC_CHAN(8, IIO_VOLTAGE),
+	VF610_ADC_CHAN(9, IIO_VOLTAGE),
+	VF610_ADC_CHAN(10, IIO_VOLTAGE),
+	VF610_ADC_CHAN(11, IIO_VOLTAGE),
+	VF610_ADC_CHAN(12, IIO_VOLTAGE),
+	VF610_ADC_CHAN(13, IIO_VOLTAGE),
+	VF610_ADC_CHAN(14, IIO_VOLTAGE),
+	VF610_ADC_CHAN(15, IIO_VOLTAGE),
+	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
+	/* sentinel */
+};
+
 static int vf610_adc_read_data(struct vf610_adc *info)
 {
 	int result;
@@ -654,6 +713,9 @@ static int vf610_adc_probe(struct platform_device *pdev)
 
 	info->vref_uv = regulator_get_voltage(info->vref);
 
+	of_property_read_u32_array(pdev->dev.of_node, "fsl,adck-max-frequency",
+			info->max_adck_rate, 3);
+
 	platform_set_drvdata(pdev, indio_dev);
 
 	init_completion(&info->completion);
-- 
2.3.3


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

* [PATCH v4 2/3] iio: adc: vf610: implement configurable conversion modes
@ 2015-03-24 12:47   ` Stefan Agner
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Agner @ 2015-03-24 12:47 UTC (permalink / raw)
  To: linux-arm-kernel

Support configurable conversion mode through sysfs. So far, the
mode used was low-power, which is enabled by default now. Beside
that, the modes normal and high-speed are selectable as well.

Use the new device tree property which specifies the maximum ADC
conversion clock frequencies. Depending on the mode used, the
available resulting conversion frequency are calculated
dynamically.

Acked-by: Fugang Duan <B38611@freescale.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 Documentation/ABI/testing/sysfs-bus-iio-vf610      |   7 +
 .../devicetree/bindings/iio/adc/vf610-adc.txt      |   9 ++
 drivers/iio/adc/vf610_adc.c                        | 146 +++++++++++++++------
 3 files changed, 120 insertions(+), 42 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-vf610

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-vf610 b/Documentation/ABI/testing/sysfs-bus-iio-vf610
new file mode 100644
index 0000000..85fd0ecd5
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-vf610
@@ -0,0 +1,7 @@
+What:		/sys/bus/iio/devices/iio:deviceX/conversion_mode
+KernelVersion:	4.1
+Contact:	linux-iio at vger.kernel.org
+Description:
+		Specifies the hardware conversion mode used. The three
+		available modes are "normal", "high-speed" and "low-power",
+		whereas the last is the default mode.
diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
index 1a4a43d..3eb40e2 100644
--- a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
@@ -11,6 +11,13 @@ Required properties:
 - clock-names: Must contain "adc", matching entry in the clocks property.
 - vref-supply: The regulator supply ADC reference voltage.
 
+Recommended properties:
+- fsl,adck-max-frequency: Maximum frequencies according to datasheets operating
+  requirements. Three values are required, depending on conversion mode:
+  - Frequency in normal mode (ADLPC=0, ADHSC=0)
+  - Frequency in high-speed mode (ADLPC=0, ADHSC=1)
+  - Frequency in low-power mode (ADLPC=1, ADHSC=0)
+
 Example:
 adc0: adc at 4003b000 {
 	compatible = "fsl,vf610-adc";
@@ -18,5 +25,7 @@ adc0: adc at 4003b000 {
 	interrupts = <0 53 0x04>;
 	clocks = <&clks VF610_CLK_ADC0>;
 	clock-names = "adc";
+	fsl,adck-max-frequency = <30000000>, <40000000>,
+				<20000000>;
 	vref-supply = <&reg_vcc_3v3_mcu>;
 };
diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
index e63b8e7..b5f94ab8 100644
--- a/drivers/iio/adc/vf610_adc.c
+++ b/drivers/iio/adc/vf610_adc.c
@@ -118,15 +118,21 @@ enum average_sel {
 	VF610_ADC_SAMPLE_32,
 };
 
+enum conversion_mode_sel {
+	VF610_ADC_CONV_NORMAL,
+	VF610_ADC_CONV_HIGH_SPEED,
+	VF610_ADC_CONV_LOW_POWER,
+};
+
 struct vf610_adc_feature {
 	enum clk_sel	clk_sel;
 	enum vol_ref	vol_ref;
+	enum conversion_mode_sel conv_mode;
 
 	int	clk_div;
 	int     sample_rate;
 	int	res_mode;
 
-	bool	lpm;
 	bool	calibration;
 	bool	ovwren;
 };
@@ -139,6 +145,8 @@ struct vf610_adc {
 	u32 vref_uv;
 	u32 value;
 	struct regulator *vref;
+
+	u32 max_adck_rate[3];
 	struct vf610_adc_feature adc_feature;
 
 	u32 sample_freq_avail[5];
@@ -148,46 +156,22 @@ struct vf610_adc {
 
 static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
 
-#define VF610_ADC_CHAN(_idx, _chan_type) {			\
-	.type = (_chan_type),					\
-	.indexed = 1,						\
-	.channel = (_idx),					\
-	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
-	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
-				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
-}
-
-#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
-	.type = (_chan_type),	\
-	.channel = (_idx),		\
-	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
-}
-
-static const struct iio_chan_spec vf610_adc_iio_channels[] = {
-	VF610_ADC_CHAN(0, IIO_VOLTAGE),
-	VF610_ADC_CHAN(1, IIO_VOLTAGE),
-	VF610_ADC_CHAN(2, IIO_VOLTAGE),
-	VF610_ADC_CHAN(3, IIO_VOLTAGE),
-	VF610_ADC_CHAN(4, IIO_VOLTAGE),
-	VF610_ADC_CHAN(5, IIO_VOLTAGE),
-	VF610_ADC_CHAN(6, IIO_VOLTAGE),
-	VF610_ADC_CHAN(7, IIO_VOLTAGE),
-	VF610_ADC_CHAN(8, IIO_VOLTAGE),
-	VF610_ADC_CHAN(9, IIO_VOLTAGE),
-	VF610_ADC_CHAN(10, IIO_VOLTAGE),
-	VF610_ADC_CHAN(11, IIO_VOLTAGE),
-	VF610_ADC_CHAN(12, IIO_VOLTAGE),
-	VF610_ADC_CHAN(13, IIO_VOLTAGE),
-	VF610_ADC_CHAN(14, IIO_VOLTAGE),
-	VF610_ADC_CHAN(15, IIO_VOLTAGE),
-	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
-	/* sentinel */
-};
-
 static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
 {
+	struct vf610_adc_feature *adc_feature = &info->adc_feature;
 	unsigned long adck_rate, ipg_rate = clk_get_rate(info->clk);
-	int i;
+	int divisor, i;
+
+	adck_rate = info->max_adck_rate[adc_feature->conv_mode];
+
+	if (adck_rate) {
+		/* calculate clk divider which is within specification */
+		divisor = ipg_rate / adck_rate;
+		adc_feature->clk_div = 1 << fls(divisor + 1);
+	} else {
+		/* fall-back value using a safe divisor */
+		adc_feature->clk_div = 8;
+	}
 
 	/*
 	 * Calculate ADC sample frequencies
@@ -219,10 +203,8 @@ static inline void vf610_adc_cfg_init(struct vf610_adc *info)
 
 	adc_feature->res_mode = 12;
 	adc_feature->sample_rate = 1;
-	adc_feature->lpm = true;
 
-	/* Use a save ADCK which is below 20MHz on all devices */
-	adc_feature->clk_div = 8;
+	adc_feature->conv_mode = VF610_ADC_CONV_LOW_POWER;
 
 	vf610_adc_calculate_rates(info);
 }
@@ -307,10 +289,12 @@ static void vf610_adc_cfg_set(struct vf610_adc *info)
 	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
 
 	cfg_data &= ~VF610_ADC_ADLPC_EN;
-	if (adc_feature->lpm)
+	if (adc_feature->conv_mode == VF610_ADC_CONV_LOW_POWER)
 		cfg_data |= VF610_ADC_ADLPC_EN;
 
 	cfg_data &= ~VF610_ADC_ADHSC_EN;
+	if (adc_feature->conv_mode == VF610_ADC_CONV_HIGH_SPEED)
+		cfg_data |= VF610_ADC_ADHSC_EN;
 
 	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
 }
@@ -412,6 +396,81 @@ static void vf610_adc_hw_init(struct vf610_adc *info)
 	vf610_adc_cfg_set(info);
 }
 
+static int vf610_set_conversion_mode(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     unsigned int mode)
+{
+	struct vf610_adc *info = iio_priv(indio_dev);
+
+	mutex_lock(&indio_dev->mlock);
+	info->adc_feature.conv_mode = mode;
+	vf610_adc_calculate_rates(info);
+	vf610_adc_hw_init(info);
+	mutex_unlock(&indio_dev->mlock);
+
+	return 0;
+}
+
+static int vf610_get_conversion_mode(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan)
+{
+	struct vf610_adc *info = iio_priv(indio_dev);
+
+	return info->adc_feature.conv_mode;
+}
+
+static const char * const vf610_conv_modes[] = { "normal", "high-speed",
+						 "low-power" };
+
+static const struct iio_enum vf610_conversion_mode = {
+	.items = vf610_conv_modes,
+	.num_items = ARRAY_SIZE(vf610_conv_modes),
+	.get = vf610_get_conversion_mode,
+	.set = vf610_set_conversion_mode,
+};
+
+static const struct iio_chan_spec_ext_info vf610_ext_info[] = {
+	IIO_ENUM("conversion_mode", IIO_SHARED_BY_DIR, &vf610_conversion_mode),
+	{},
+};
+
+#define VF610_ADC_CHAN(_idx, _chan_type) {			\
+	.type = (_chan_type),					\
+	.indexed = 1,						\
+	.channel = (_idx),					\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
+				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.ext_info = vf610_ext_info,				\
+}
+
+#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
+	.type = (_chan_type),	\
+	.channel = (_idx),		\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
+}
+
+static const struct iio_chan_spec vf610_adc_iio_channels[] = {
+	VF610_ADC_CHAN(0, IIO_VOLTAGE),
+	VF610_ADC_CHAN(1, IIO_VOLTAGE),
+	VF610_ADC_CHAN(2, IIO_VOLTAGE),
+	VF610_ADC_CHAN(3, IIO_VOLTAGE),
+	VF610_ADC_CHAN(4, IIO_VOLTAGE),
+	VF610_ADC_CHAN(5, IIO_VOLTAGE),
+	VF610_ADC_CHAN(6, IIO_VOLTAGE),
+	VF610_ADC_CHAN(7, IIO_VOLTAGE),
+	VF610_ADC_CHAN(8, IIO_VOLTAGE),
+	VF610_ADC_CHAN(9, IIO_VOLTAGE),
+	VF610_ADC_CHAN(10, IIO_VOLTAGE),
+	VF610_ADC_CHAN(11, IIO_VOLTAGE),
+	VF610_ADC_CHAN(12, IIO_VOLTAGE),
+	VF610_ADC_CHAN(13, IIO_VOLTAGE),
+	VF610_ADC_CHAN(14, IIO_VOLTAGE),
+	VF610_ADC_CHAN(15, IIO_VOLTAGE),
+	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
+	/* sentinel */
+};
+
 static int vf610_adc_read_data(struct vf610_adc *info)
 {
 	int result;
@@ -654,6 +713,9 @@ static int vf610_adc_probe(struct platform_device *pdev)
 
 	info->vref_uv = regulator_get_voltage(info->vref);
 
+	of_property_read_u32_array(pdev->dev.of_node, "fsl,adck-max-frequency",
+			info->max_adck_rate, 3);
+
 	platform_set_drvdata(pdev, indio_dev);
 
 	init_completion(&info->completion);
-- 
2.3.3

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

* [PATCH v4 3/3] ARM: dts: add property for maximum ADC clock frequencies
@ 2015-03-24 12:47   ` Stefan Agner
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Agner @ 2015-03-24 12:47 UTC (permalink / raw)
  To: jic23, shawn.guo, kernel
  Cc: knaack.h, lars, pmeerw, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, B38611, maitysanchayan, devicetree,
	linux-iio, linux-arm-kernel, linux-kernel, Stefan Agner

The ADC clock frequency is limited depending on modes used. Add
device tree property which allow to set the mode used and the
maximum frequency ratings for the instance. These allows to
set the ADC clock to a frequency which is within specification
according to the actual mode used.

Acked-by: Fugang Duan <B38611@freescale.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/vfxxx.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index a29c7ce..c6609bd 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -189,6 +189,8 @@
 				clocks = <&clks VF610_CLK_ADC0>;
 				clock-names = "adc";
 				status = "disabled";
+				fsl,adck-max-frequency = <30000000>, <40000000>,
+							<20000000>;
 			};
 
 			wdoga5: wdog@4003e000 {
@@ -387,6 +389,8 @@
 				clocks = <&clks VF610_CLK_ADC1>;
 				clock-names = "adc";
 				status = "disabled";
+				fsl,adck-max-frequency = <30000000>, <40000000>,
+							<20000000>;
 			};
 
 			esdhc1: esdhc@400b2000 {
-- 
2.3.3


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

* [PATCH v4 3/3] ARM: dts: add property for maximum ADC clock frequencies
@ 2015-03-24 12:47   ` Stefan Agner
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Agner @ 2015-03-24 12:47 UTC (permalink / raw)
  To: jic23-DgEjT+Ai2ygdnm+yROfE0A, shawn.guo-QSEj5FYQhm4dnm+yROfE0A,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ
  Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
	pmeerw-jW+XmwGofnusTnJN9+BGXg, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, B38611-KZfg59tc24xl57MIdRCFDg,
	maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stefan Agner

The ADC clock frequency is limited depending on modes used. Add
device tree property which allow to set the mode used and the
maximum frequency ratings for the instance. These allows to
set the ADC clock to a frequency which is within specification
according to the actual mode used.

Acked-by: Fugang Duan <B38611-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Signed-off-by: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>
---
 arch/arm/boot/dts/vfxxx.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index a29c7ce..c6609bd 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -189,6 +189,8 @@
 				clocks = <&clks VF610_CLK_ADC0>;
 				clock-names = "adc";
 				status = "disabled";
+				fsl,adck-max-frequency = <30000000>, <40000000>,
+							<20000000>;
 			};
 
 			wdoga5: wdog@4003e000 {
@@ -387,6 +389,8 @@
 				clocks = <&clks VF610_CLK_ADC1>;
 				clock-names = "adc";
 				status = "disabled";
+				fsl,adck-max-frequency = <30000000>, <40000000>,
+							<20000000>;
 			};
 
 			esdhc1: esdhc@400b2000 {
-- 
2.3.3

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

* [PATCH v4 3/3] ARM: dts: add property for maximum ADC clock frequencies
@ 2015-03-24 12:47   ` Stefan Agner
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Agner @ 2015-03-24 12:47 UTC (permalink / raw)
  To: linux-arm-kernel

The ADC clock frequency is limited depending on modes used. Add
device tree property which allow to set the mode used and the
maximum frequency ratings for the instance. These allows to
set the ADC clock to a frequency which is within specification
according to the actual mode used.

Acked-by: Fugang Duan <B38611@freescale.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/vfxxx.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index a29c7ce..c6609bd 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -189,6 +189,8 @@
 				clocks = <&clks VF610_CLK_ADC0>;
 				clock-names = "adc";
 				status = "disabled";
+				fsl,adck-max-frequency = <30000000>, <40000000>,
+							<20000000>;
 			};
 
 			wdoga5: wdog at 4003e000 {
@@ -387,6 +389,8 @@
 				clocks = <&clks VF610_CLK_ADC1>;
 				clock-names = "adc";
 				status = "disabled";
+				fsl,adck-max-frequency = <30000000>, <40000000>,
+							<20000000>;
 			};
 
 			esdhc1: esdhc at 400b2000 {
-- 
2.3.3

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

* Re: [PATCH v4 1/3] iio: adc: vf610: use ADC clock within specification
  2015-03-24 12:47   ` Stefan Agner
@ 2015-03-28 12:03     ` Jonathan Cameron
  -1 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2015-03-28 12:03 UTC (permalink / raw)
  To: Stefan Agner, shawn.guo, kernel
  Cc: knaack.h, lars, pmeerw, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, B38611, maitysanchayan, devicetree,
	linux-iio, linux-arm-kernel, linux-kernel

On 24/03/15 12:47, Stefan Agner wrote:
> Depending on conversion mode used, the ADC clock (ADCK) needs
> to be below a maximum frequency. According to Vybrid's data
> sheet this is 20MHz for the low power conversion mode.
> 
> The ADC clock is depending on input clock, which is the bus
> clock by default. Vybrid SoC are typically clocked at at 400MHz
> or 500MHz, which leads to 66MHz or 83MHz bus clock respectively.
> Hence, a divider of 8 is required to stay below the specified
> maximum clock of 20MHz.
> 
> Due to the different bus clock speeds, the resulting sampling
> frequency is not static. Hence use the ADC clock and calculate
> the actual available sampling frequency dynamically.
> 
> This fixes bogous values observed on some 500MHz clocked Vybrid
> SoC. The resulting value usually showed Bit 9 being stuck at 1,
> or 0, which lead to a value of +/-512.
> 
> Acked-by: Fugang Duan <B38611@freescale.com>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
I'd perhaps have stuck the word 'fix' in the description ;)

Anyhow, applied to the fixes-togreg branch of iio.git and marked for stable.

I'll leave the other two for nwo to allow for device tree review and this one to
propagate through to my upstream...

Jonathan
> ---
>  drivers/iio/adc/vf610_adc.c | 91 ++++++++++++++++++++++++++++++---------------
>  1 file changed, 61 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
> index 8ec353c..e63b8e7 100644
> --- a/drivers/iio/adc/vf610_adc.c
> +++ b/drivers/iio/adc/vf610_adc.c
> @@ -141,9 +141,13 @@ struct vf610_adc {
>  	struct regulator *vref;
>  	struct vf610_adc_feature adc_feature;
>  
> +	u32 sample_freq_avail[5];
> +
>  	struct completion completion;
>  };
>  
> +static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
> +
>  #define VF610_ADC_CHAN(_idx, _chan_type) {			\
>  	.type = (_chan_type),					\
>  	.indexed = 1,						\
> @@ -180,35 +184,47 @@ static const struct iio_chan_spec vf610_adc_iio_channels[] = {
>  	/* sentinel */
>  };
>  
> -/*
> - * ADC sample frequency, unit is ADCK cycles.
> - * ADC clk source is ipg clock, which is the same as bus clock.
> - *
> - * ADC conversion time = SFCAdder + AverageNum x (BCT + LSTAdder)
> - * SFCAdder: fixed to 6 ADCK cycles
> - * AverageNum: 1, 4, 8, 16, 32 samples for hardware average.
> - * BCT (Base Conversion Time): fixed to 25 ADCK cycles for 12 bit mode
> - * LSTAdder(Long Sample Time): fixed to 3 ADCK cycles
> - *
> - * By default, enable 12 bit resolution mode, clock source
> - * set to ipg clock, So get below frequency group:
> - */
> -static const u32 vf610_sample_freq_avail[5] =
> -{1941176, 559332, 286957, 145374, 73171};
> +static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
> +{
> +	unsigned long adck_rate, ipg_rate = clk_get_rate(info->clk);
> +	int i;
> +
> +	/*
> +	 * Calculate ADC sample frequencies
> +	 * Sample time unit is ADCK cycles. ADCK clk source is ipg clock,
> +	 * which is the same as bus clock.
> +	 *
> +	 * ADC conversion time = SFCAdder + AverageNum x (BCT + LSTAdder)
> +	 * SFCAdder: fixed to 6 ADCK cycles
> +	 * AverageNum: 1, 4, 8, 16, 32 samples for hardware average.
> +	 * BCT (Base Conversion Time): fixed to 25 ADCK cycles for 12 bit mode
> +	 * LSTAdder(Long Sample Time): fixed to 3 ADCK cycles
> +	 */
> +	adck_rate = ipg_rate / info->adc_feature.clk_div;
> +	for (i = 0; i < ARRAY_SIZE(vf610_hw_avgs); i++)
> +		info->sample_freq_avail[i] =
> +			adck_rate / (6 + vf610_hw_avgs[i] * (25 + 3));
> +}
>  
>  static inline void vf610_adc_cfg_init(struct vf610_adc *info)
>  {
> +	struct vf610_adc_feature *adc_feature = &info->adc_feature;
> +
>  	/* set default Configuration for ADC controller */
> -	info->adc_feature.clk_sel = VF610_ADCIOC_BUSCLK_SET;
> -	info->adc_feature.vol_ref = VF610_ADCIOC_VR_VREF_SET;
> +	adc_feature->clk_sel = VF610_ADCIOC_BUSCLK_SET;
> +	adc_feature->vol_ref = VF610_ADCIOC_VR_VREF_SET;
> +
> +	adc_feature->calibration = true;
> +	adc_feature->ovwren = true;
> +
> +	adc_feature->res_mode = 12;
> +	adc_feature->sample_rate = 1;
> +	adc_feature->lpm = true;
>  
> -	info->adc_feature.calibration = true;
> -	info->adc_feature.ovwren = true;
> +	/* Use a save ADCK which is below 20MHz on all devices */
> +	adc_feature->clk_div = 8;
>  
> -	info->adc_feature.clk_div = 1;
> -	info->adc_feature.res_mode = 12;
> -	info->adc_feature.sample_rate = 1;
> -	info->adc_feature.lpm = true;
> +	vf610_adc_calculate_rates(info);
>  }
>  
>  static void vf610_adc_cfg_post_set(struct vf610_adc *info)
> @@ -290,12 +306,10 @@ static void vf610_adc_cfg_set(struct vf610_adc *info)
>  
>  	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
>  
> -	/* low power configuration */
>  	cfg_data &= ~VF610_ADC_ADLPC_EN;
>  	if (adc_feature->lpm)
>  		cfg_data |= VF610_ADC_ADLPC_EN;
>  
> -	/* disable high speed */
>  	cfg_data &= ~VF610_ADC_ADHSC_EN;
>  
>  	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
> @@ -435,10 +449,27 @@ static irqreturn_t vf610_adc_isr(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> -static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("1941176, 559332, 286957, 145374, 73171");
> +static ssize_t vf610_show_samp_freq_avail(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct vf610_adc *info = iio_priv(dev_to_iio_dev(dev));
> +	size_t len = 0;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(info->sample_freq_avail); i++)
> +		len += scnprintf(buf + len, PAGE_SIZE - len,
> +			"%u ", info->sample_freq_avail[i]);
> +
> +	/* replace trailing space by newline */
> +	buf[len - 1] = '\n';
> +
> +	return len;
> +}
> +
> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(vf610_show_samp_freq_avail);
>  
>  static struct attribute *vf610_attributes[] = {
> -	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> +	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
>  	NULL
>  };
>  
> @@ -502,7 +533,7 @@ static int vf610_read_raw(struct iio_dev *indio_dev,
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> -		*val = vf610_sample_freq_avail[info->adc_feature.sample_rate];
> +		*val = info->sample_freq_avail[info->adc_feature.sample_rate];
>  		*val2 = 0;
>  		return IIO_VAL_INT;
>  
> @@ -525,9 +556,9 @@ static int vf610_write_raw(struct iio_dev *indio_dev,
>  	switch (mask) {
>  		case IIO_CHAN_INFO_SAMP_FREQ:
>  			for (i = 0;
> -				i < ARRAY_SIZE(vf610_sample_freq_avail);
> +				i < ARRAY_SIZE(info->sample_freq_avail);
>  				i++)
> -				if (val == vf610_sample_freq_avail[i]) {
> +				if (val == info->sample_freq_avail[i]) {
>  					info->adc_feature.sample_rate = i;
>  					vf610_adc_sample_set(info);
>  					return 0;
> 


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

* [PATCH v4 1/3] iio: adc: vf610: use ADC clock within specification
@ 2015-03-28 12:03     ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2015-03-28 12:03 UTC (permalink / raw)
  To: linux-arm-kernel

On 24/03/15 12:47, Stefan Agner wrote:
> Depending on conversion mode used, the ADC clock (ADCK) needs
> to be below a maximum frequency. According to Vybrid's data
> sheet this is 20MHz for the low power conversion mode.
> 
> The ADC clock is depending on input clock, which is the bus
> clock by default. Vybrid SoC are typically clocked at at 400MHz
> or 500MHz, which leads to 66MHz or 83MHz bus clock respectively.
> Hence, a divider of 8 is required to stay below the specified
> maximum clock of 20MHz.
> 
> Due to the different bus clock speeds, the resulting sampling
> frequency is not static. Hence use the ADC clock and calculate
> the actual available sampling frequency dynamically.
> 
> This fixes bogous values observed on some 500MHz clocked Vybrid
> SoC. The resulting value usually showed Bit 9 being stuck at 1,
> or 0, which lead to a value of +/-512.
> 
> Acked-by: Fugang Duan <B38611@freescale.com>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
I'd perhaps have stuck the word 'fix' in the description ;)

Anyhow, applied to the fixes-togreg branch of iio.git and marked for stable.

I'll leave the other two for nwo to allow for device tree review and this one to
propagate through to my upstream...

Jonathan
> ---
>  drivers/iio/adc/vf610_adc.c | 91 ++++++++++++++++++++++++++++++---------------
>  1 file changed, 61 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
> index 8ec353c..e63b8e7 100644
> --- a/drivers/iio/adc/vf610_adc.c
> +++ b/drivers/iio/adc/vf610_adc.c
> @@ -141,9 +141,13 @@ struct vf610_adc {
>  	struct regulator *vref;
>  	struct vf610_adc_feature adc_feature;
>  
> +	u32 sample_freq_avail[5];
> +
>  	struct completion completion;
>  };
>  
> +static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
> +
>  #define VF610_ADC_CHAN(_idx, _chan_type) {			\
>  	.type = (_chan_type),					\
>  	.indexed = 1,						\
> @@ -180,35 +184,47 @@ static const struct iio_chan_spec vf610_adc_iio_channels[] = {
>  	/* sentinel */
>  };
>  
> -/*
> - * ADC sample frequency, unit is ADCK cycles.
> - * ADC clk source is ipg clock, which is the same as bus clock.
> - *
> - * ADC conversion time = SFCAdder + AverageNum x (BCT + LSTAdder)
> - * SFCAdder: fixed to 6 ADCK cycles
> - * AverageNum: 1, 4, 8, 16, 32 samples for hardware average.
> - * BCT (Base Conversion Time): fixed to 25 ADCK cycles for 12 bit mode
> - * LSTAdder(Long Sample Time): fixed to 3 ADCK cycles
> - *
> - * By default, enable 12 bit resolution mode, clock source
> - * set to ipg clock, So get below frequency group:
> - */
> -static const u32 vf610_sample_freq_avail[5] =
> -{1941176, 559332, 286957, 145374, 73171};
> +static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
> +{
> +	unsigned long adck_rate, ipg_rate = clk_get_rate(info->clk);
> +	int i;
> +
> +	/*
> +	 * Calculate ADC sample frequencies
> +	 * Sample time unit is ADCK cycles. ADCK clk source is ipg clock,
> +	 * which is the same as bus clock.
> +	 *
> +	 * ADC conversion time = SFCAdder + AverageNum x (BCT + LSTAdder)
> +	 * SFCAdder: fixed to 6 ADCK cycles
> +	 * AverageNum: 1, 4, 8, 16, 32 samples for hardware average.
> +	 * BCT (Base Conversion Time): fixed to 25 ADCK cycles for 12 bit mode
> +	 * LSTAdder(Long Sample Time): fixed to 3 ADCK cycles
> +	 */
> +	adck_rate = ipg_rate / info->adc_feature.clk_div;
> +	for (i = 0; i < ARRAY_SIZE(vf610_hw_avgs); i++)
> +		info->sample_freq_avail[i] =
> +			adck_rate / (6 + vf610_hw_avgs[i] * (25 + 3));
> +}
>  
>  static inline void vf610_adc_cfg_init(struct vf610_adc *info)
>  {
> +	struct vf610_adc_feature *adc_feature = &info->adc_feature;
> +
>  	/* set default Configuration for ADC controller */
> -	info->adc_feature.clk_sel = VF610_ADCIOC_BUSCLK_SET;
> -	info->adc_feature.vol_ref = VF610_ADCIOC_VR_VREF_SET;
> +	adc_feature->clk_sel = VF610_ADCIOC_BUSCLK_SET;
> +	adc_feature->vol_ref = VF610_ADCIOC_VR_VREF_SET;
> +
> +	adc_feature->calibration = true;
> +	adc_feature->ovwren = true;
> +
> +	adc_feature->res_mode = 12;
> +	adc_feature->sample_rate = 1;
> +	adc_feature->lpm = true;
>  
> -	info->adc_feature.calibration = true;
> -	info->adc_feature.ovwren = true;
> +	/* Use a save ADCK which is below 20MHz on all devices */
> +	adc_feature->clk_div = 8;
>  
> -	info->adc_feature.clk_div = 1;
> -	info->adc_feature.res_mode = 12;
> -	info->adc_feature.sample_rate = 1;
> -	info->adc_feature.lpm = true;
> +	vf610_adc_calculate_rates(info);
>  }
>  
>  static void vf610_adc_cfg_post_set(struct vf610_adc *info)
> @@ -290,12 +306,10 @@ static void vf610_adc_cfg_set(struct vf610_adc *info)
>  
>  	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
>  
> -	/* low power configuration */
>  	cfg_data &= ~VF610_ADC_ADLPC_EN;
>  	if (adc_feature->lpm)
>  		cfg_data |= VF610_ADC_ADLPC_EN;
>  
> -	/* disable high speed */
>  	cfg_data &= ~VF610_ADC_ADHSC_EN;
>  
>  	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
> @@ -435,10 +449,27 @@ static irqreturn_t vf610_adc_isr(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> -static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("1941176, 559332, 286957, 145374, 73171");
> +static ssize_t vf610_show_samp_freq_avail(struct device *dev,
> +				struct device_attribute *attr, char *buf)
> +{
> +	struct vf610_adc *info = iio_priv(dev_to_iio_dev(dev));
> +	size_t len = 0;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(info->sample_freq_avail); i++)
> +		len += scnprintf(buf + len, PAGE_SIZE - len,
> +			"%u ", info->sample_freq_avail[i]);
> +
> +	/* replace trailing space by newline */
> +	buf[len - 1] = '\n';
> +
> +	return len;
> +}
> +
> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(vf610_show_samp_freq_avail);
>  
>  static struct attribute *vf610_attributes[] = {
> -	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> +	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
>  	NULL
>  };
>  
> @@ -502,7 +533,7 @@ static int vf610_read_raw(struct iio_dev *indio_dev,
>  		return IIO_VAL_FRACTIONAL_LOG2;
>  
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> -		*val = vf610_sample_freq_avail[info->adc_feature.sample_rate];
> +		*val = info->sample_freq_avail[info->adc_feature.sample_rate];
>  		*val2 = 0;
>  		return IIO_VAL_INT;
>  
> @@ -525,9 +556,9 @@ static int vf610_write_raw(struct iio_dev *indio_dev,
>  	switch (mask) {
>  		case IIO_CHAN_INFO_SAMP_FREQ:
>  			for (i = 0;
> -				i < ARRAY_SIZE(vf610_sample_freq_avail);
> +				i < ARRAY_SIZE(info->sample_freq_avail);
>  				i++)
> -				if (val == vf610_sample_freq_avail[i]) {
> +				if (val == info->sample_freq_avail[i]) {
>  					info->adc_feature.sample_rate = i;
>  					vf610_adc_sample_set(info);
>  					return 0;
> 

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

* Re: [PATCH v4 2/3] iio: adc: vf610: implement configurable conversion modes
@ 2015-03-28 12:07     ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2015-03-28 12:07 UTC (permalink / raw)
  To: Stefan Agner, shawn.guo, kernel
  Cc: knaack.h, lars, pmeerw, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, B38611, maitysanchayan, devicetree,
	linux-iio, linux-arm-kernel, linux-kernel

On 24/03/15 12:47, Stefan Agner wrote:
> Support configurable conversion mode through sysfs. So far, the
> mode used was low-power, which is enabled by default now. Beside
> that, the modes normal and high-speed are selectable as well.
> 
> Use the new device tree property which specifies the maximum ADC
> conversion clock frequencies. Depending on the mode used, the
> available resulting conversion frequency are calculated
> dynamically.
> 
One trivial bit inline, otherwise looks good to me.
I wish there was a better way of 'hinting' to drivers what power mode
is required, but in the absense of this I guess explicit control is
all we can do.
> Acked-by: Fugang Duan <B38611@freescale.com>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio-vf610      |   7 +
>  .../devicetree/bindings/iio/adc/vf610-adc.txt      |   9 ++
>  drivers/iio/adc/vf610_adc.c                        | 146 +++++++++++++++------
>  3 files changed, 120 insertions(+), 42 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-vf610
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-vf610 b/Documentation/ABI/testing/sysfs-bus-iio-vf610
> new file mode 100644
> index 0000000..85fd0ecd5
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-vf610
> @@ -0,0 +1,7 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/conversion_mode
> +KernelVersion:	4.1
> +Contact:	linux-iio@vger.kernel.org
> +Description:
> +		Specifies the hardware conversion mode used. The three
> +		available modes are "normal", "high-speed" and "low-power",
> +		whereas the last is the default mode.
Fussy English of the day.  where rather than whereas (whereas is only
used if you say something like. A is normally the case, whereas here B is the case)
I like the default that isn't 'normal' :)

> diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> index 1a4a43d..3eb40e2 100644
> --- a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> @@ -11,6 +11,13 @@ Required properties:
>  - clock-names: Must contain "adc", matching entry in the clocks property.
>  - vref-supply: The regulator supply ADC reference voltage.
>  
> +Recommended properties:
> +- fsl,adck-max-frequency: Maximum frequencies according to datasheets operating
> +  requirements. Three values are required, depending on conversion mode:
> +  - Frequency in normal mode (ADLPC=0, ADHSC=0)
> +  - Frequency in high-speed mode (ADLPC=0, ADHSC=1)
> +  - Frequency in low-power mode (ADLPC=1, ADHSC=0)
> +
>  Example:
>  adc0: adc@4003b000 {
>  	compatible = "fsl,vf610-adc";
> @@ -18,5 +25,7 @@ adc0: adc@4003b000 {
>  	interrupts = <0 53 0x04>;
>  	clocks = <&clks VF610_CLK_ADC0>;
>  	clock-names = "adc";
> +	fsl,adck-max-frequency = <30000000>, <40000000>,
> +				<20000000>;
>  	vref-supply = <&reg_vcc_3v3_mcu>;
>  };
> diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
> index e63b8e7..b5f94ab8 100644
> --- a/drivers/iio/adc/vf610_adc.c
> +++ b/drivers/iio/adc/vf610_adc.c
> @@ -118,15 +118,21 @@ enum average_sel {
>  	VF610_ADC_SAMPLE_32,
>  };
>  
> +enum conversion_mode_sel {
> +	VF610_ADC_CONV_NORMAL,
> +	VF610_ADC_CONV_HIGH_SPEED,
> +	VF610_ADC_CONV_LOW_POWER,
> +};
> +
>  struct vf610_adc_feature {
>  	enum clk_sel	clk_sel;
>  	enum vol_ref	vol_ref;
> +	enum conversion_mode_sel conv_mode;
>  
>  	int	clk_div;
>  	int     sample_rate;
>  	int	res_mode;
>  
> -	bool	lpm;
>  	bool	calibration;
>  	bool	ovwren;
>  };
> @@ -139,6 +145,8 @@ struct vf610_adc {
>  	u32 vref_uv;
>  	u32 value;
>  	struct regulator *vref;
> +
> +	u32 max_adck_rate[3];
>  	struct vf610_adc_feature adc_feature;
>  
>  	u32 sample_freq_avail[5];
> @@ -148,46 +156,22 @@ struct vf610_adc {
>  
>  static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
>  
> -#define VF610_ADC_CHAN(_idx, _chan_type) {			\
> -	.type = (_chan_type),					\
> -	.indexed = 1,						\
> -	.channel = (_idx),					\
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> -				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> -}
> -
> -#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
> -	.type = (_chan_type),	\
> -	.channel = (_idx),		\
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
> -}
> -
> -static const struct iio_chan_spec vf610_adc_iio_channels[] = {
> -	VF610_ADC_CHAN(0, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(1, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(2, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(3, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(4, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(5, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(6, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(7, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(8, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(9, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(10, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(11, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(12, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(13, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(14, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(15, IIO_VOLTAGE),
> -	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
> -	/* sentinel */
> -};
> -
>  static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
>  {
> +	struct vf610_adc_feature *adc_feature = &info->adc_feature;
>  	unsigned long adck_rate, ipg_rate = clk_get_rate(info->clk);
> -	int i;
> +	int divisor, i;
> +
> +	adck_rate = info->max_adck_rate[adc_feature->conv_mode];
> +
> +	if (adck_rate) {
> +		/* calculate clk divider which is within specification */
> +		divisor = ipg_rate / adck_rate;
> +		adc_feature->clk_div = 1 << fls(divisor + 1);
> +	} else {
> +		/* fall-back value using a safe divisor */
> +		adc_feature->clk_div = 8;
> +	}
>  
>  	/*
>  	 * Calculate ADC sample frequencies
> @@ -219,10 +203,8 @@ static inline void vf610_adc_cfg_init(struct vf610_adc *info)
>  
>  	adc_feature->res_mode = 12;
>  	adc_feature->sample_rate = 1;
> -	adc_feature->lpm = true;
>  
> -	/* Use a save ADCK which is below 20MHz on all devices */
> -	adc_feature->clk_div = 8;
> +	adc_feature->conv_mode = VF610_ADC_CONV_LOW_POWER;
>  
>  	vf610_adc_calculate_rates(info);
>  }
> @@ -307,10 +289,12 @@ static void vf610_adc_cfg_set(struct vf610_adc *info)
>  	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
>  
>  	cfg_data &= ~VF610_ADC_ADLPC_EN;
> -	if (adc_feature->lpm)
> +	if (adc_feature->conv_mode == VF610_ADC_CONV_LOW_POWER)
>  		cfg_data |= VF610_ADC_ADLPC_EN;
>  
>  	cfg_data &= ~VF610_ADC_ADHSC_EN;
> +	if (adc_feature->conv_mode == VF610_ADC_CONV_HIGH_SPEED)
> +		cfg_data |= VF610_ADC_ADHSC_EN;
>  
>  	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
>  }
> @@ -412,6 +396,81 @@ static void vf610_adc_hw_init(struct vf610_adc *info)
>  	vf610_adc_cfg_set(info);
>  }
>  
> +static int vf610_set_conversion_mode(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan,
> +				     unsigned int mode)
> +{
> +	struct vf610_adc *info = iio_priv(indio_dev);
> +
> +	mutex_lock(&indio_dev->mlock);
> +	info->adc_feature.conv_mode = mode;
> +	vf610_adc_calculate_rates(info);
> +	vf610_adc_hw_init(info);
> +	mutex_unlock(&indio_dev->mlock);
> +
> +	return 0;
> +}
> +
> +static int vf610_get_conversion_mode(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan)
> +{
> +	struct vf610_adc *info = iio_priv(indio_dev);
> +
> +	return info->adc_feature.conv_mode;
> +}
> +
> +static const char * const vf610_conv_modes[] = { "normal", "high-speed",
> +						 "low-power" };
> +
> +static const struct iio_enum vf610_conversion_mode = {
> +	.items = vf610_conv_modes,
> +	.num_items = ARRAY_SIZE(vf610_conv_modes),
> +	.get = vf610_get_conversion_mode,
> +	.set = vf610_set_conversion_mode,
> +};
> +
> +static const struct iio_chan_spec_ext_info vf610_ext_info[] = {
> +	IIO_ENUM("conversion_mode", IIO_SHARED_BY_DIR, &vf610_conversion_mode),
> +	{},
> +};
> +
> +#define VF610_ADC_CHAN(_idx, _chan_type) {			\
> +	.type = (_chan_type),					\
> +	.indexed = 1,						\
> +	.channel = (_idx),					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +	.ext_info = vf610_ext_info,				\
> +}
> +
> +#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
> +	.type = (_chan_type),	\
> +	.channel = (_idx),		\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
> +}
> +
> +static const struct iio_chan_spec vf610_adc_iio_channels[] = {
> +	VF610_ADC_CHAN(0, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(1, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(2, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(3, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(4, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(5, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(6, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(7, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(8, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(9, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(10, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(11, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(12, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(13, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(14, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(15, IIO_VOLTAGE),
> +	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
> +	/* sentinel */
> +};
> +
>  static int vf610_adc_read_data(struct vf610_adc *info)
>  {
>  	int result;
> @@ -654,6 +713,9 @@ static int vf610_adc_probe(struct platform_device *pdev)
>  
>  	info->vref_uv = regulator_get_voltage(info->vref);
>  
> +	of_property_read_u32_array(pdev->dev.of_node, "fsl,adck-max-frequency",
> +			info->max_adck_rate, 3);
> +
>  	platform_set_drvdata(pdev, indio_dev);
>  
>  	init_completion(&info->completion);
> 


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

* Re: [PATCH v4 2/3] iio: adc: vf610: implement configurable conversion modes
@ 2015-03-28 12:07     ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2015-03-28 12:07 UTC (permalink / raw)
  To: Stefan Agner, shawn.guo-QSEj5FYQhm4dnm+yROfE0A,
	kernel-bIcnvbaLZ9MEGnE8C9+IrQ
  Cc: knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
	pmeerw-jW+XmwGofnusTnJN9+BGXg, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, B38611-KZfg59tc24xl57MIdRCFDg,
	maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 24/03/15 12:47, Stefan Agner wrote:
> Support configurable conversion mode through sysfs. So far, the
> mode used was low-power, which is enabled by default now. Beside
> that, the modes normal and high-speed are selectable as well.
> 
> Use the new device tree property which specifies the maximum ADC
> conversion clock frequencies. Depending on the mode used, the
> available resulting conversion frequency are calculated
> dynamically.
> 
One trivial bit inline, otherwise looks good to me.
I wish there was a better way of 'hinting' to drivers what power mode
is required, but in the absense of this I guess explicit control is
all we can do.
> Acked-by: Fugang Duan <B38611-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> Signed-off-by: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio-vf610      |   7 +
>  .../devicetree/bindings/iio/adc/vf610-adc.txt      |   9 ++
>  drivers/iio/adc/vf610_adc.c                        | 146 +++++++++++++++------
>  3 files changed, 120 insertions(+), 42 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-vf610
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-vf610 b/Documentation/ABI/testing/sysfs-bus-iio-vf610
> new file mode 100644
> index 0000000..85fd0ecd5
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-vf610
> @@ -0,0 +1,7 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/conversion_mode
> +KernelVersion:	4.1
> +Contact:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description:
> +		Specifies the hardware conversion mode used. The three
> +		available modes are "normal", "high-speed" and "low-power",
> +		whereas the last is the default mode.
Fussy English of the day.  where rather than whereas (whereas is only
used if you say something like. A is normally the case, whereas here B is the case)
I like the default that isn't 'normal' :)

> diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> index 1a4a43d..3eb40e2 100644
> --- a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> @@ -11,6 +11,13 @@ Required properties:
>  - clock-names: Must contain "adc", matching entry in the clocks property.
>  - vref-supply: The regulator supply ADC reference voltage.
>  
> +Recommended properties:
> +- fsl,adck-max-frequency: Maximum frequencies according to datasheets operating
> +  requirements. Three values are required, depending on conversion mode:
> +  - Frequency in normal mode (ADLPC=0, ADHSC=0)
> +  - Frequency in high-speed mode (ADLPC=0, ADHSC=1)
> +  - Frequency in low-power mode (ADLPC=1, ADHSC=0)
> +
>  Example:
>  adc0: adc@4003b000 {
>  	compatible = "fsl,vf610-adc";
> @@ -18,5 +25,7 @@ adc0: adc@4003b000 {
>  	interrupts = <0 53 0x04>;
>  	clocks = <&clks VF610_CLK_ADC0>;
>  	clock-names = "adc";
> +	fsl,adck-max-frequency = <30000000>, <40000000>,
> +				<20000000>;
>  	vref-supply = <&reg_vcc_3v3_mcu>;
>  };
> diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
> index e63b8e7..b5f94ab8 100644
> --- a/drivers/iio/adc/vf610_adc.c
> +++ b/drivers/iio/adc/vf610_adc.c
> @@ -118,15 +118,21 @@ enum average_sel {
>  	VF610_ADC_SAMPLE_32,
>  };
>  
> +enum conversion_mode_sel {
> +	VF610_ADC_CONV_NORMAL,
> +	VF610_ADC_CONV_HIGH_SPEED,
> +	VF610_ADC_CONV_LOW_POWER,
> +};
> +
>  struct vf610_adc_feature {
>  	enum clk_sel	clk_sel;
>  	enum vol_ref	vol_ref;
> +	enum conversion_mode_sel conv_mode;
>  
>  	int	clk_div;
>  	int     sample_rate;
>  	int	res_mode;
>  
> -	bool	lpm;
>  	bool	calibration;
>  	bool	ovwren;
>  };
> @@ -139,6 +145,8 @@ struct vf610_adc {
>  	u32 vref_uv;
>  	u32 value;
>  	struct regulator *vref;
> +
> +	u32 max_adck_rate[3];
>  	struct vf610_adc_feature adc_feature;
>  
>  	u32 sample_freq_avail[5];
> @@ -148,46 +156,22 @@ struct vf610_adc {
>  
>  static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
>  
> -#define VF610_ADC_CHAN(_idx, _chan_type) {			\
> -	.type = (_chan_type),					\
> -	.indexed = 1,						\
> -	.channel = (_idx),					\
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> -				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> -}
> -
> -#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
> -	.type = (_chan_type),	\
> -	.channel = (_idx),		\
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
> -}
> -
> -static const struct iio_chan_spec vf610_adc_iio_channels[] = {
> -	VF610_ADC_CHAN(0, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(1, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(2, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(3, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(4, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(5, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(6, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(7, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(8, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(9, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(10, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(11, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(12, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(13, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(14, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(15, IIO_VOLTAGE),
> -	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
> -	/* sentinel */
> -};
> -
>  static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
>  {
> +	struct vf610_adc_feature *adc_feature = &info->adc_feature;
>  	unsigned long adck_rate, ipg_rate = clk_get_rate(info->clk);
> -	int i;
> +	int divisor, i;
> +
> +	adck_rate = info->max_adck_rate[adc_feature->conv_mode];
> +
> +	if (adck_rate) {
> +		/* calculate clk divider which is within specification */
> +		divisor = ipg_rate / adck_rate;
> +		adc_feature->clk_div = 1 << fls(divisor + 1);
> +	} else {
> +		/* fall-back value using a safe divisor */
> +		adc_feature->clk_div = 8;
> +	}
>  
>  	/*
>  	 * Calculate ADC sample frequencies
> @@ -219,10 +203,8 @@ static inline void vf610_adc_cfg_init(struct vf610_adc *info)
>  
>  	adc_feature->res_mode = 12;
>  	adc_feature->sample_rate = 1;
> -	adc_feature->lpm = true;
>  
> -	/* Use a save ADCK which is below 20MHz on all devices */
> -	adc_feature->clk_div = 8;
> +	adc_feature->conv_mode = VF610_ADC_CONV_LOW_POWER;
>  
>  	vf610_adc_calculate_rates(info);
>  }
> @@ -307,10 +289,12 @@ static void vf610_adc_cfg_set(struct vf610_adc *info)
>  	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
>  
>  	cfg_data &= ~VF610_ADC_ADLPC_EN;
> -	if (adc_feature->lpm)
> +	if (adc_feature->conv_mode == VF610_ADC_CONV_LOW_POWER)
>  		cfg_data |= VF610_ADC_ADLPC_EN;
>  
>  	cfg_data &= ~VF610_ADC_ADHSC_EN;
> +	if (adc_feature->conv_mode == VF610_ADC_CONV_HIGH_SPEED)
> +		cfg_data |= VF610_ADC_ADHSC_EN;
>  
>  	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
>  }
> @@ -412,6 +396,81 @@ static void vf610_adc_hw_init(struct vf610_adc *info)
>  	vf610_adc_cfg_set(info);
>  }
>  
> +static int vf610_set_conversion_mode(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan,
> +				     unsigned int mode)
> +{
> +	struct vf610_adc *info = iio_priv(indio_dev);
> +
> +	mutex_lock(&indio_dev->mlock);
> +	info->adc_feature.conv_mode = mode;
> +	vf610_adc_calculate_rates(info);
> +	vf610_adc_hw_init(info);
> +	mutex_unlock(&indio_dev->mlock);
> +
> +	return 0;
> +}
> +
> +static int vf610_get_conversion_mode(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan)
> +{
> +	struct vf610_adc *info = iio_priv(indio_dev);
> +
> +	return info->adc_feature.conv_mode;
> +}
> +
> +static const char * const vf610_conv_modes[] = { "normal", "high-speed",
> +						 "low-power" };
> +
> +static const struct iio_enum vf610_conversion_mode = {
> +	.items = vf610_conv_modes,
> +	.num_items = ARRAY_SIZE(vf610_conv_modes),
> +	.get = vf610_get_conversion_mode,
> +	.set = vf610_set_conversion_mode,
> +};
> +
> +static const struct iio_chan_spec_ext_info vf610_ext_info[] = {
> +	IIO_ENUM("conversion_mode", IIO_SHARED_BY_DIR, &vf610_conversion_mode),
> +	{},
> +};
> +
> +#define VF610_ADC_CHAN(_idx, _chan_type) {			\
> +	.type = (_chan_type),					\
> +	.indexed = 1,						\
> +	.channel = (_idx),					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +	.ext_info = vf610_ext_info,				\
> +}
> +
> +#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
> +	.type = (_chan_type),	\
> +	.channel = (_idx),		\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
> +}
> +
> +static const struct iio_chan_spec vf610_adc_iio_channels[] = {
> +	VF610_ADC_CHAN(0, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(1, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(2, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(3, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(4, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(5, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(6, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(7, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(8, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(9, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(10, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(11, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(12, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(13, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(14, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(15, IIO_VOLTAGE),
> +	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
> +	/* sentinel */
> +};
> +
>  static int vf610_adc_read_data(struct vf610_adc *info)
>  {
>  	int result;
> @@ -654,6 +713,9 @@ static int vf610_adc_probe(struct platform_device *pdev)
>  
>  	info->vref_uv = regulator_get_voltage(info->vref);
>  
> +	of_property_read_u32_array(pdev->dev.of_node, "fsl,adck-max-frequency",
> +			info->max_adck_rate, 3);
> +
>  	platform_set_drvdata(pdev, indio_dev);
>  
>  	init_completion(&info->completion);
> 

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

* [PATCH v4 2/3] iio: adc: vf610: implement configurable conversion modes
@ 2015-03-28 12:07     ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2015-03-28 12:07 UTC (permalink / raw)
  To: linux-arm-kernel

On 24/03/15 12:47, Stefan Agner wrote:
> Support configurable conversion mode through sysfs. So far, the
> mode used was low-power, which is enabled by default now. Beside
> that, the modes normal and high-speed are selectable as well.
> 
> Use the new device tree property which specifies the maximum ADC
> conversion clock frequencies. Depending on the mode used, the
> available resulting conversion frequency are calculated
> dynamically.
> 
One trivial bit inline, otherwise looks good to me.
I wish there was a better way of 'hinting' to drivers what power mode
is required, but in the absense of this I guess explicit control is
all we can do.
> Acked-by: Fugang Duan <B38611@freescale.com>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  Documentation/ABI/testing/sysfs-bus-iio-vf610      |   7 +
>  .../devicetree/bindings/iio/adc/vf610-adc.txt      |   9 ++
>  drivers/iio/adc/vf610_adc.c                        | 146 +++++++++++++++------
>  3 files changed, 120 insertions(+), 42 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-vf610
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-vf610 b/Documentation/ABI/testing/sysfs-bus-iio-vf610
> new file mode 100644
> index 0000000..85fd0ecd5
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-vf610
> @@ -0,0 +1,7 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/conversion_mode
> +KernelVersion:	4.1
> +Contact:	linux-iio at vger.kernel.org
> +Description:
> +		Specifies the hardware conversion mode used. The three
> +		available modes are "normal", "high-speed" and "low-power",
> +		whereas the last is the default mode.
Fussy English of the day.  where rather than whereas (whereas is only
used if you say something like. A is normally the case, whereas here B is the case)
I like the default that isn't 'normal' :)

> diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> index 1a4a43d..3eb40e2 100644
> --- a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> +++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
> @@ -11,6 +11,13 @@ Required properties:
>  - clock-names: Must contain "adc", matching entry in the clocks property.
>  - vref-supply: The regulator supply ADC reference voltage.
>  
> +Recommended properties:
> +- fsl,adck-max-frequency: Maximum frequencies according to datasheets operating
> +  requirements. Three values are required, depending on conversion mode:
> +  - Frequency in normal mode (ADLPC=0, ADHSC=0)
> +  - Frequency in high-speed mode (ADLPC=0, ADHSC=1)
> +  - Frequency in low-power mode (ADLPC=1, ADHSC=0)
> +
>  Example:
>  adc0: adc at 4003b000 {
>  	compatible = "fsl,vf610-adc";
> @@ -18,5 +25,7 @@ adc0: adc at 4003b000 {
>  	interrupts = <0 53 0x04>;
>  	clocks = <&clks VF610_CLK_ADC0>;
>  	clock-names = "adc";
> +	fsl,adck-max-frequency = <30000000>, <40000000>,
> +				<20000000>;
>  	vref-supply = <&reg_vcc_3v3_mcu>;
>  };
> diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
> index e63b8e7..b5f94ab8 100644
> --- a/drivers/iio/adc/vf610_adc.c
> +++ b/drivers/iio/adc/vf610_adc.c
> @@ -118,15 +118,21 @@ enum average_sel {
>  	VF610_ADC_SAMPLE_32,
>  };
>  
> +enum conversion_mode_sel {
> +	VF610_ADC_CONV_NORMAL,
> +	VF610_ADC_CONV_HIGH_SPEED,
> +	VF610_ADC_CONV_LOW_POWER,
> +};
> +
>  struct vf610_adc_feature {
>  	enum clk_sel	clk_sel;
>  	enum vol_ref	vol_ref;
> +	enum conversion_mode_sel conv_mode;
>  
>  	int	clk_div;
>  	int     sample_rate;
>  	int	res_mode;
>  
> -	bool	lpm;
>  	bool	calibration;
>  	bool	ovwren;
>  };
> @@ -139,6 +145,8 @@ struct vf610_adc {
>  	u32 vref_uv;
>  	u32 value;
>  	struct regulator *vref;
> +
> +	u32 max_adck_rate[3];
>  	struct vf610_adc_feature adc_feature;
>  
>  	u32 sample_freq_avail[5];
> @@ -148,46 +156,22 @@ struct vf610_adc {
>  
>  static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
>  
> -#define VF610_ADC_CHAN(_idx, _chan_type) {			\
> -	.type = (_chan_type),					\
> -	.indexed = 1,						\
> -	.channel = (_idx),					\
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> -				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> -}
> -
> -#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
> -	.type = (_chan_type),	\
> -	.channel = (_idx),		\
> -	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
> -}
> -
> -static const struct iio_chan_spec vf610_adc_iio_channels[] = {
> -	VF610_ADC_CHAN(0, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(1, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(2, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(3, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(4, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(5, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(6, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(7, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(8, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(9, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(10, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(11, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(12, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(13, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(14, IIO_VOLTAGE),
> -	VF610_ADC_CHAN(15, IIO_VOLTAGE),
> -	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
> -	/* sentinel */
> -};
> -
>  static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
>  {
> +	struct vf610_adc_feature *adc_feature = &info->adc_feature;
>  	unsigned long adck_rate, ipg_rate = clk_get_rate(info->clk);
> -	int i;
> +	int divisor, i;
> +
> +	adck_rate = info->max_adck_rate[adc_feature->conv_mode];
> +
> +	if (adck_rate) {
> +		/* calculate clk divider which is within specification */
> +		divisor = ipg_rate / adck_rate;
> +		adc_feature->clk_div = 1 << fls(divisor + 1);
> +	} else {
> +		/* fall-back value using a safe divisor */
> +		adc_feature->clk_div = 8;
> +	}
>  
>  	/*
>  	 * Calculate ADC sample frequencies
> @@ -219,10 +203,8 @@ static inline void vf610_adc_cfg_init(struct vf610_adc *info)
>  
>  	adc_feature->res_mode = 12;
>  	adc_feature->sample_rate = 1;
> -	adc_feature->lpm = true;
>  
> -	/* Use a save ADCK which is below 20MHz on all devices */
> -	adc_feature->clk_div = 8;
> +	adc_feature->conv_mode = VF610_ADC_CONV_LOW_POWER;
>  
>  	vf610_adc_calculate_rates(info);
>  }
> @@ -307,10 +289,12 @@ static void vf610_adc_cfg_set(struct vf610_adc *info)
>  	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
>  
>  	cfg_data &= ~VF610_ADC_ADLPC_EN;
> -	if (adc_feature->lpm)
> +	if (adc_feature->conv_mode == VF610_ADC_CONV_LOW_POWER)
>  		cfg_data |= VF610_ADC_ADLPC_EN;
>  
>  	cfg_data &= ~VF610_ADC_ADHSC_EN;
> +	if (adc_feature->conv_mode == VF610_ADC_CONV_HIGH_SPEED)
> +		cfg_data |= VF610_ADC_ADHSC_EN;
>  
>  	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
>  }
> @@ -412,6 +396,81 @@ static void vf610_adc_hw_init(struct vf610_adc *info)
>  	vf610_adc_cfg_set(info);
>  }
>  
> +static int vf610_set_conversion_mode(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan,
> +				     unsigned int mode)
> +{
> +	struct vf610_adc *info = iio_priv(indio_dev);
> +
> +	mutex_lock(&indio_dev->mlock);
> +	info->adc_feature.conv_mode = mode;
> +	vf610_adc_calculate_rates(info);
> +	vf610_adc_hw_init(info);
> +	mutex_unlock(&indio_dev->mlock);
> +
> +	return 0;
> +}
> +
> +static int vf610_get_conversion_mode(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan)
> +{
> +	struct vf610_adc *info = iio_priv(indio_dev);
> +
> +	return info->adc_feature.conv_mode;
> +}
> +
> +static const char * const vf610_conv_modes[] = { "normal", "high-speed",
> +						 "low-power" };
> +
> +static const struct iio_enum vf610_conversion_mode = {
> +	.items = vf610_conv_modes,
> +	.num_items = ARRAY_SIZE(vf610_conv_modes),
> +	.get = vf610_get_conversion_mode,
> +	.set = vf610_set_conversion_mode,
> +};
> +
> +static const struct iio_chan_spec_ext_info vf610_ext_info[] = {
> +	IIO_ENUM("conversion_mode", IIO_SHARED_BY_DIR, &vf610_conversion_mode),
> +	{},
> +};
> +
> +#define VF610_ADC_CHAN(_idx, _chan_type) {			\
> +	.type = (_chan_type),					\
> +	.indexed = 1,						\
> +	.channel = (_idx),					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +	.ext_info = vf610_ext_info,				\
> +}
> +
> +#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
> +	.type = (_chan_type),	\
> +	.channel = (_idx),		\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
> +}
> +
> +static const struct iio_chan_spec vf610_adc_iio_channels[] = {
> +	VF610_ADC_CHAN(0, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(1, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(2, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(3, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(4, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(5, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(6, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(7, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(8, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(9, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(10, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(11, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(12, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(13, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(14, IIO_VOLTAGE),
> +	VF610_ADC_CHAN(15, IIO_VOLTAGE),
> +	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
> +	/* sentinel */
> +};
> +
>  static int vf610_adc_read_data(struct vf610_adc *info)
>  {
>  	int result;
> @@ -654,6 +713,9 @@ static int vf610_adc_probe(struct platform_device *pdev)
>  
>  	info->vref_uv = regulator_get_voltage(info->vref);
>  
> +	of_property_read_u32_array(pdev->dev.of_node, "fsl,adck-max-frequency",
> +			info->max_adck_rate, 3);
> +
>  	platform_set_drvdata(pdev, indio_dev);
>  
>  	init_completion(&info->completion);
> 

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

* Re: [PATCH v4 2/3] iio: adc: vf610: implement configurable conversion modes
@ 2015-03-28 12:49       ` Stefan Agner
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Agner @ 2015-03-28 12:49 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: shawn.guo, kernel, knaack.h, lars, pmeerw, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, B38611, maitysanchayan,
	devicetree, linux-iio, linux-arm-kernel, linux-kernel

On 2015-03-28 13:07, Jonathan Cameron wrote:
> On 24/03/15 12:47, Stefan Agner wrote:
>> Support configurable conversion mode through sysfs. So far, the
>> mode used was low-power, which is enabled by default now. Beside
>> that, the modes normal and high-speed are selectable as well.
>>
>> Use the new device tree property which specifies the maximum ADC
>> conversion clock frequencies. Depending on the mode used, the
>> available resulting conversion frequency are calculated
>> dynamically.
>>
> One trivial bit inline, otherwise looks good to me.
> I wish there was a better way of 'hinting' to drivers what power mode
> is required, but in the absense of this I guess explicit control is
> all we can do.

An implicit implementation would be possible by just calculating all
frequencies for the three power modes. The driver would then select the
right power mode to get the individual frequencies. However, due to the
nature of sampling timing of the different modes, the frequencies of the
individual modes would overlap over a large range. For the user it would
not be possible to tell which frequency leads to which mode... Hence I
like this implementation more...

>> Acked-by: Fugang Duan <B38611@freescale.com>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  Documentation/ABI/testing/sysfs-bus-iio-vf610      |   7 +
>>  .../devicetree/bindings/iio/adc/vf610-adc.txt      |   9 ++
>>  drivers/iio/adc/vf610_adc.c                        | 146 +++++++++++++++------
>>  3 files changed, 120 insertions(+), 42 deletions(-)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-vf610
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-vf610 b/Documentation/ABI/testing/sysfs-bus-iio-vf610
>> new file mode 100644
>> index 0000000..85fd0ecd5
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-vf610
>> @@ -0,0 +1,7 @@
>> +What:		/sys/bus/iio/devices/iio:deviceX/conversion_mode
>> +KernelVersion:	4.1
>> +Contact:	linux-iio@vger.kernel.org
>> +Description:
>> +		Specifies the hardware conversion mode used. The three
>> +		available modes are "normal", "high-speed" and "low-power",
>> +		whereas the last is the default mode.
> Fussy English of the day.  where rather than whereas (whereas is only
> used if you say something like. A is normally the case, whereas here B
> is the case)

Ok, got it, thx for pointing that out.

> I like the default that isn't 'normal' :)

Hehe, like that too. The driver historically used the low power mode by
default, and it is probably fine for most application, hence I just kept
that.

What didn't came out by your comment above, do you expect me to do
another revision or did you applied the patch?

--
Stefan

> 
>> diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
>> index 1a4a43d..3eb40e2 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
>> +++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
>> @@ -11,6 +11,13 @@ Required properties:
>>  - clock-names: Must contain "adc", matching entry in the clocks property.
>>  - vref-supply: The regulator supply ADC reference voltage.
>>
>> +Recommended properties:
>> +- fsl,adck-max-frequency: Maximum frequencies according to datasheets operating
>> +  requirements. Three values are required, depending on conversion mode:
>> +  - Frequency in normal mode (ADLPC=0, ADHSC=0)
>> +  - Frequency in high-speed mode (ADLPC=0, ADHSC=1)
>> +  - Frequency in low-power mode (ADLPC=1, ADHSC=0)
>> +
>>  Example:
>>  adc0: adc@4003b000 {
>>  	compatible = "fsl,vf610-adc";
>> @@ -18,5 +25,7 @@ adc0: adc@4003b000 {
>>  	interrupts = <0 53 0x04>;
>>  	clocks = <&clks VF610_CLK_ADC0>;
>>  	clock-names = "adc";
>> +	fsl,adck-max-frequency = <30000000>, <40000000>,
>> +				<20000000>;
>>  	vref-supply = <&reg_vcc_3v3_mcu>;
>>  };
>> diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
>> index e63b8e7..b5f94ab8 100644
>> --- a/drivers/iio/adc/vf610_adc.c
>> +++ b/drivers/iio/adc/vf610_adc.c
>> @@ -118,15 +118,21 @@ enum average_sel {
>>  	VF610_ADC_SAMPLE_32,
>>  };
>>
>> +enum conversion_mode_sel {
>> +	VF610_ADC_CONV_NORMAL,
>> +	VF610_ADC_CONV_HIGH_SPEED,
>> +	VF610_ADC_CONV_LOW_POWER,
>> +};
>> +
>>  struct vf610_adc_feature {
>>  	enum clk_sel	clk_sel;
>>  	enum vol_ref	vol_ref;
>> +	enum conversion_mode_sel conv_mode;
>>
>>  	int	clk_div;
>>  	int     sample_rate;
>>  	int	res_mode;
>>
>> -	bool	lpm;
>>  	bool	calibration;
>>  	bool	ovwren;
>>  };
>> @@ -139,6 +145,8 @@ struct vf610_adc {
>>  	u32 vref_uv;
>>  	u32 value;
>>  	struct regulator *vref;
>> +
>> +	u32 max_adck_rate[3];
>>  	struct vf610_adc_feature adc_feature;
>>
>>  	u32 sample_freq_avail[5];
>> @@ -148,46 +156,22 @@ struct vf610_adc {
>>
>>  static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
>>
>> -#define VF610_ADC_CHAN(_idx, _chan_type) {			\
>> -	.type = (_chan_type),					\
>> -	.indexed = 1,						\
>> -	.channel = (_idx),					\
>> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
>> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
>> -				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
>> -}
>> -
>> -#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
>> -	.type = (_chan_type),	\
>> -	.channel = (_idx),		\
>> -	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
>> -}
>> -
>> -static const struct iio_chan_spec vf610_adc_iio_channels[] = {
>> -	VF610_ADC_CHAN(0, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(1, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(2, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(3, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(4, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(5, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(6, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(7, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(8, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(9, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(10, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(11, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(12, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(13, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(14, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(15, IIO_VOLTAGE),
>> -	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
>> -	/* sentinel */
>> -};
>> -
>>  static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
>>  {
>> +	struct vf610_adc_feature *adc_feature = &info->adc_feature;
>>  	unsigned long adck_rate, ipg_rate = clk_get_rate(info->clk);
>> -	int i;
>> +	int divisor, i;
>> +
>> +	adck_rate = info->max_adck_rate[adc_feature->conv_mode];
>> +
>> +	if (adck_rate) {
>> +		/* calculate clk divider which is within specification */
>> +		divisor = ipg_rate / adck_rate;
>> +		adc_feature->clk_div = 1 << fls(divisor + 1);
>> +	} else {
>> +		/* fall-back value using a safe divisor */
>> +		adc_feature->clk_div = 8;
>> +	}
>>
>>  	/*
>>  	 * Calculate ADC sample frequencies
>> @@ -219,10 +203,8 @@ static inline void vf610_adc_cfg_init(struct vf610_adc *info)
>>
>>  	adc_feature->res_mode = 12;
>>  	adc_feature->sample_rate = 1;
>> -	adc_feature->lpm = true;
>>
>> -	/* Use a save ADCK which is below 20MHz on all devices */
>> -	adc_feature->clk_div = 8;
>> +	adc_feature->conv_mode = VF610_ADC_CONV_LOW_POWER;
>>
>>  	vf610_adc_calculate_rates(info);
>>  }
>> @@ -307,10 +289,12 @@ static void vf610_adc_cfg_set(struct vf610_adc *info)
>>  	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
>>
>>  	cfg_data &= ~VF610_ADC_ADLPC_EN;
>> -	if (adc_feature->lpm)
>> +	if (adc_feature->conv_mode == VF610_ADC_CONV_LOW_POWER)
>>  		cfg_data |= VF610_ADC_ADLPC_EN;
>>
>>  	cfg_data &= ~VF610_ADC_ADHSC_EN;
>> +	if (adc_feature->conv_mode == VF610_ADC_CONV_HIGH_SPEED)
>> +		cfg_data |= VF610_ADC_ADHSC_EN;
>>
>>  	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
>>  }
>> @@ -412,6 +396,81 @@ static void vf610_adc_hw_init(struct vf610_adc *info)
>>  	vf610_adc_cfg_set(info);
>>  }
>>
>> +static int vf610_set_conversion_mode(struct iio_dev *indio_dev,
>> +				     const struct iio_chan_spec *chan,
>> +				     unsigned int mode)
>> +{
>> +	struct vf610_adc *info = iio_priv(indio_dev);
>> +
>> +	mutex_lock(&indio_dev->mlock);
>> +	info->adc_feature.conv_mode = mode;
>> +	vf610_adc_calculate_rates(info);
>> +	vf610_adc_hw_init(info);
>> +	mutex_unlock(&indio_dev->mlock);
>> +
>> +	return 0;
>> +}
>> +
>> +static int vf610_get_conversion_mode(struct iio_dev *indio_dev,
>> +				     const struct iio_chan_spec *chan)
>> +{
>> +	struct vf610_adc *info = iio_priv(indio_dev);
>> +
>> +	return info->adc_feature.conv_mode;
>> +}
>> +
>> +static const char * const vf610_conv_modes[] = { "normal", "high-speed",
>> +						 "low-power" };
>> +
>> +static const struct iio_enum vf610_conversion_mode = {
>> +	.items = vf610_conv_modes,
>> +	.num_items = ARRAY_SIZE(vf610_conv_modes),
>> +	.get = vf610_get_conversion_mode,
>> +	.set = vf610_set_conversion_mode,
>> +};
>> +
>> +static const struct iio_chan_spec_ext_info vf610_ext_info[] = {
>> +	IIO_ENUM("conversion_mode", IIO_SHARED_BY_DIR, &vf610_conversion_mode),
>> +	{},
>> +};
>> +
>> +#define VF610_ADC_CHAN(_idx, _chan_type) {			\
>> +	.type = (_chan_type),					\
>> +	.indexed = 1,						\
>> +	.channel = (_idx),					\
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
>> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
>> +				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
>> +	.ext_info = vf610_ext_info,				\
>> +}
>> +
>> +#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
>> +	.type = (_chan_type),	\
>> +	.channel = (_idx),		\
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
>> +}
>> +
>> +static const struct iio_chan_spec vf610_adc_iio_channels[] = {
>> +	VF610_ADC_CHAN(0, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(1, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(2, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(3, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(4, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(5, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(6, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(7, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(8, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(9, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(10, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(11, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(12, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(13, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(14, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(15, IIO_VOLTAGE),
>> +	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
>> +	/* sentinel */
>> +};
>> +
>>  static int vf610_adc_read_data(struct vf610_adc *info)
>>  {
>>  	int result;
>> @@ -654,6 +713,9 @@ static int vf610_adc_probe(struct platform_device *pdev)
>>
>>  	info->vref_uv = regulator_get_voltage(info->vref);
>>
>> +	of_property_read_u32_array(pdev->dev.of_node, "fsl,adck-max-frequency",
>> +			info->max_adck_rate, 3);
>> +
>>  	platform_set_drvdata(pdev, indio_dev);
>>
>>  	init_completion(&info->completion);
>>


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

* Re: [PATCH v4 2/3] iio: adc: vf610: implement configurable conversion modes
@ 2015-03-28 12:49       ` Stefan Agner
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Agner @ 2015-03-28 12:49 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: shawn.guo-QSEj5FYQhm4dnm+yROfE0A, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
	knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw,
	pmeerw-jW+XmwGofnusTnJN9+BGXg, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	pawel.moll-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
	ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, B38611-KZfg59tc24xl57MIdRCFDg,
	maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA

On 2015-03-28 13:07, Jonathan Cameron wrote:
> On 24/03/15 12:47, Stefan Agner wrote:
>> Support configurable conversion mode through sysfs. So far, the
>> mode used was low-power, which is enabled by default now. Beside
>> that, the modes normal and high-speed are selectable as well.
>>
>> Use the new device tree property which specifies the maximum ADC
>> conversion clock frequencies. Depending on the mode used, the
>> available resulting conversion frequency are calculated
>> dynamically.
>>
> One trivial bit inline, otherwise looks good to me.
> I wish there was a better way of 'hinting' to drivers what power mode
> is required, but in the absense of this I guess explicit control is
> all we can do.

An implicit implementation would be possible by just calculating all
frequencies for the three power modes. The driver would then select the
right power mode to get the individual frequencies. However, due to the
nature of sampling timing of the different modes, the frequencies of the
individual modes would overlap over a large range. For the user it would
not be possible to tell which frequency leads to which mode... Hence I
like this implementation more...

>> Acked-by: Fugang Duan <B38611-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>> Signed-off-by: Stefan Agner <stefan-XLVq0VzYD2Y@public.gmane.org>
>> ---
>>  Documentation/ABI/testing/sysfs-bus-iio-vf610      |   7 +
>>  .../devicetree/bindings/iio/adc/vf610-adc.txt      |   9 ++
>>  drivers/iio/adc/vf610_adc.c                        | 146 +++++++++++++++------
>>  3 files changed, 120 insertions(+), 42 deletions(-)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-vf610
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-vf610 b/Documentation/ABI/testing/sysfs-bus-iio-vf610
>> new file mode 100644
>> index 0000000..85fd0ecd5
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-vf610
>> @@ -0,0 +1,7 @@
>> +What:		/sys/bus/iio/devices/iio:deviceX/conversion_mode
>> +KernelVersion:	4.1
>> +Contact:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> +Description:
>> +		Specifies the hardware conversion mode used. The three
>> +		available modes are "normal", "high-speed" and "low-power",
>> +		whereas the last is the default mode.
> Fussy English of the day.  where rather than whereas (whereas is only
> used if you say something like. A is normally the case, whereas here B
> is the case)

Ok, got it, thx for pointing that out.

> I like the default that isn't 'normal' :)

Hehe, like that too. The driver historically used the low power mode by
default, and it is probably fine for most application, hence I just kept
that.

What didn't came out by your comment above, do you expect me to do
another revision or did you applied the patch?

--
Stefan

> 
>> diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
>> index 1a4a43d..3eb40e2 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
>> +++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
>> @@ -11,6 +11,13 @@ Required properties:
>>  - clock-names: Must contain "adc", matching entry in the clocks property.
>>  - vref-supply: The regulator supply ADC reference voltage.
>>
>> +Recommended properties:
>> +- fsl,adck-max-frequency: Maximum frequencies according to datasheets operating
>> +  requirements. Three values are required, depending on conversion mode:
>> +  - Frequency in normal mode (ADLPC=0, ADHSC=0)
>> +  - Frequency in high-speed mode (ADLPC=0, ADHSC=1)
>> +  - Frequency in low-power mode (ADLPC=1, ADHSC=0)
>> +
>>  Example:
>>  adc0: adc@4003b000 {
>>  	compatible = "fsl,vf610-adc";
>> @@ -18,5 +25,7 @@ adc0: adc@4003b000 {
>>  	interrupts = <0 53 0x04>;
>>  	clocks = <&clks VF610_CLK_ADC0>;
>>  	clock-names = "adc";
>> +	fsl,adck-max-frequency = <30000000>, <40000000>,
>> +				<20000000>;
>>  	vref-supply = <&reg_vcc_3v3_mcu>;
>>  };
>> diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
>> index e63b8e7..b5f94ab8 100644
>> --- a/drivers/iio/adc/vf610_adc.c
>> +++ b/drivers/iio/adc/vf610_adc.c
>> @@ -118,15 +118,21 @@ enum average_sel {
>>  	VF610_ADC_SAMPLE_32,
>>  };
>>
>> +enum conversion_mode_sel {
>> +	VF610_ADC_CONV_NORMAL,
>> +	VF610_ADC_CONV_HIGH_SPEED,
>> +	VF610_ADC_CONV_LOW_POWER,
>> +};
>> +
>>  struct vf610_adc_feature {
>>  	enum clk_sel	clk_sel;
>>  	enum vol_ref	vol_ref;
>> +	enum conversion_mode_sel conv_mode;
>>
>>  	int	clk_div;
>>  	int     sample_rate;
>>  	int	res_mode;
>>
>> -	bool	lpm;
>>  	bool	calibration;
>>  	bool	ovwren;
>>  };
>> @@ -139,6 +145,8 @@ struct vf610_adc {
>>  	u32 vref_uv;
>>  	u32 value;
>>  	struct regulator *vref;
>> +
>> +	u32 max_adck_rate[3];
>>  	struct vf610_adc_feature adc_feature;
>>
>>  	u32 sample_freq_avail[5];
>> @@ -148,46 +156,22 @@ struct vf610_adc {
>>
>>  static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
>>
>> -#define VF610_ADC_CHAN(_idx, _chan_type) {			\
>> -	.type = (_chan_type),					\
>> -	.indexed = 1,						\
>> -	.channel = (_idx),					\
>> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
>> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
>> -				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
>> -}
>> -
>> -#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
>> -	.type = (_chan_type),	\
>> -	.channel = (_idx),		\
>> -	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
>> -}
>> -
>> -static const struct iio_chan_spec vf610_adc_iio_channels[] = {
>> -	VF610_ADC_CHAN(0, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(1, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(2, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(3, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(4, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(5, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(6, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(7, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(8, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(9, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(10, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(11, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(12, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(13, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(14, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(15, IIO_VOLTAGE),
>> -	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
>> -	/* sentinel */
>> -};
>> -
>>  static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
>>  {
>> +	struct vf610_adc_feature *adc_feature = &info->adc_feature;
>>  	unsigned long adck_rate, ipg_rate = clk_get_rate(info->clk);
>> -	int i;
>> +	int divisor, i;
>> +
>> +	adck_rate = info->max_adck_rate[adc_feature->conv_mode];
>> +
>> +	if (adck_rate) {
>> +		/* calculate clk divider which is within specification */
>> +		divisor = ipg_rate / adck_rate;
>> +		adc_feature->clk_div = 1 << fls(divisor + 1);
>> +	} else {
>> +		/* fall-back value using a safe divisor */
>> +		adc_feature->clk_div = 8;
>> +	}
>>
>>  	/*
>>  	 * Calculate ADC sample frequencies
>> @@ -219,10 +203,8 @@ static inline void vf610_adc_cfg_init(struct vf610_adc *info)
>>
>>  	adc_feature->res_mode = 12;
>>  	adc_feature->sample_rate = 1;
>> -	adc_feature->lpm = true;
>>
>> -	/* Use a save ADCK which is below 20MHz on all devices */
>> -	adc_feature->clk_div = 8;
>> +	adc_feature->conv_mode = VF610_ADC_CONV_LOW_POWER;
>>
>>  	vf610_adc_calculate_rates(info);
>>  }
>> @@ -307,10 +289,12 @@ static void vf610_adc_cfg_set(struct vf610_adc *info)
>>  	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
>>
>>  	cfg_data &= ~VF610_ADC_ADLPC_EN;
>> -	if (adc_feature->lpm)
>> +	if (adc_feature->conv_mode == VF610_ADC_CONV_LOW_POWER)
>>  		cfg_data |= VF610_ADC_ADLPC_EN;
>>
>>  	cfg_data &= ~VF610_ADC_ADHSC_EN;
>> +	if (adc_feature->conv_mode == VF610_ADC_CONV_HIGH_SPEED)
>> +		cfg_data |= VF610_ADC_ADHSC_EN;
>>
>>  	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
>>  }
>> @@ -412,6 +396,81 @@ static void vf610_adc_hw_init(struct vf610_adc *info)
>>  	vf610_adc_cfg_set(info);
>>  }
>>
>> +static int vf610_set_conversion_mode(struct iio_dev *indio_dev,
>> +				     const struct iio_chan_spec *chan,
>> +				     unsigned int mode)
>> +{
>> +	struct vf610_adc *info = iio_priv(indio_dev);
>> +
>> +	mutex_lock(&indio_dev->mlock);
>> +	info->adc_feature.conv_mode = mode;
>> +	vf610_adc_calculate_rates(info);
>> +	vf610_adc_hw_init(info);
>> +	mutex_unlock(&indio_dev->mlock);
>> +
>> +	return 0;
>> +}
>> +
>> +static int vf610_get_conversion_mode(struct iio_dev *indio_dev,
>> +				     const struct iio_chan_spec *chan)
>> +{
>> +	struct vf610_adc *info = iio_priv(indio_dev);
>> +
>> +	return info->adc_feature.conv_mode;
>> +}
>> +
>> +static const char * const vf610_conv_modes[] = { "normal", "high-speed",
>> +						 "low-power" };
>> +
>> +static const struct iio_enum vf610_conversion_mode = {
>> +	.items = vf610_conv_modes,
>> +	.num_items = ARRAY_SIZE(vf610_conv_modes),
>> +	.get = vf610_get_conversion_mode,
>> +	.set = vf610_set_conversion_mode,
>> +};
>> +
>> +static const struct iio_chan_spec_ext_info vf610_ext_info[] = {
>> +	IIO_ENUM("conversion_mode", IIO_SHARED_BY_DIR, &vf610_conversion_mode),
>> +	{},
>> +};
>> +
>> +#define VF610_ADC_CHAN(_idx, _chan_type) {			\
>> +	.type = (_chan_type),					\
>> +	.indexed = 1,						\
>> +	.channel = (_idx),					\
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
>> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
>> +				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
>> +	.ext_info = vf610_ext_info,				\
>> +}
>> +
>> +#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
>> +	.type = (_chan_type),	\
>> +	.channel = (_idx),		\
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
>> +}
>> +
>> +static const struct iio_chan_spec vf610_adc_iio_channels[] = {
>> +	VF610_ADC_CHAN(0, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(1, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(2, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(3, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(4, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(5, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(6, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(7, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(8, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(9, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(10, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(11, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(12, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(13, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(14, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(15, IIO_VOLTAGE),
>> +	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
>> +	/* sentinel */
>> +};
>> +
>>  static int vf610_adc_read_data(struct vf610_adc *info)
>>  {
>>  	int result;
>> @@ -654,6 +713,9 @@ static int vf610_adc_probe(struct platform_device *pdev)
>>
>>  	info->vref_uv = regulator_get_voltage(info->vref);
>>
>> +	of_property_read_u32_array(pdev->dev.of_node, "fsl,adck-max-frequency",
>> +			info->max_adck_rate, 3);
>> +
>>  	platform_set_drvdata(pdev, indio_dev);
>>
>>  	init_completion(&info->completion);
>>

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

* [PATCH v4 2/3] iio: adc: vf610: implement configurable conversion modes
@ 2015-03-28 12:49       ` Stefan Agner
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Agner @ 2015-03-28 12:49 UTC (permalink / raw)
  To: linux-arm-kernel

On 2015-03-28 13:07, Jonathan Cameron wrote:
> On 24/03/15 12:47, Stefan Agner wrote:
>> Support configurable conversion mode through sysfs. So far, the
>> mode used was low-power, which is enabled by default now. Beside
>> that, the modes normal and high-speed are selectable as well.
>>
>> Use the new device tree property which specifies the maximum ADC
>> conversion clock frequencies. Depending on the mode used, the
>> available resulting conversion frequency are calculated
>> dynamically.
>>
> One trivial bit inline, otherwise looks good to me.
> I wish there was a better way of 'hinting' to drivers what power mode
> is required, but in the absense of this I guess explicit control is
> all we can do.

An implicit implementation would be possible by just calculating all
frequencies for the three power modes. The driver would then select the
right power mode to get the individual frequencies. However, due to the
nature of sampling timing of the different modes, the frequencies of the
individual modes would overlap over a large range. For the user it would
not be possible to tell which frequency leads to which mode... Hence I
like this implementation more...

>> Acked-by: Fugang Duan <B38611@freescale.com>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  Documentation/ABI/testing/sysfs-bus-iio-vf610      |   7 +
>>  .../devicetree/bindings/iio/adc/vf610-adc.txt      |   9 ++
>>  drivers/iio/adc/vf610_adc.c                        | 146 +++++++++++++++------
>>  3 files changed, 120 insertions(+), 42 deletions(-)
>>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-vf610
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-vf610 b/Documentation/ABI/testing/sysfs-bus-iio-vf610
>> new file mode 100644
>> index 0000000..85fd0ecd5
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio-vf610
>> @@ -0,0 +1,7 @@
>> +What:		/sys/bus/iio/devices/iio:deviceX/conversion_mode
>> +KernelVersion:	4.1
>> +Contact:	linux-iio at vger.kernel.org
>> +Description:
>> +		Specifies the hardware conversion mode used. The three
>> +		available modes are "normal", "high-speed" and "low-power",
>> +		whereas the last is the default mode.
> Fussy English of the day.  where rather than whereas (whereas is only
> used if you say something like. A is normally the case, whereas here B
> is the case)

Ok, got it, thx for pointing that out.

> I like the default that isn't 'normal' :)

Hehe, like that too. The driver historically used the low power mode by
default, and it is probably fine for most application, hence I just kept
that.

What didn't came out by your comment above, do you expect me to do
another revision or did you applied the patch?

--
Stefan

> 
>> diff --git a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
>> index 1a4a43d..3eb40e2 100644
>> --- a/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
>> +++ b/Documentation/devicetree/bindings/iio/adc/vf610-adc.txt
>> @@ -11,6 +11,13 @@ Required properties:
>>  - clock-names: Must contain "adc", matching entry in the clocks property.
>>  - vref-supply: The regulator supply ADC reference voltage.
>>
>> +Recommended properties:
>> +- fsl,adck-max-frequency: Maximum frequencies according to datasheets operating
>> +  requirements. Three values are required, depending on conversion mode:
>> +  - Frequency in normal mode (ADLPC=0, ADHSC=0)
>> +  - Frequency in high-speed mode (ADLPC=0, ADHSC=1)
>> +  - Frequency in low-power mode (ADLPC=1, ADHSC=0)
>> +
>>  Example:
>>  adc0: adc at 4003b000 {
>>  	compatible = "fsl,vf610-adc";
>> @@ -18,5 +25,7 @@ adc0: adc at 4003b000 {
>>  	interrupts = <0 53 0x04>;
>>  	clocks = <&clks VF610_CLK_ADC0>;
>>  	clock-names = "adc";
>> +	fsl,adck-max-frequency = <30000000>, <40000000>,
>> +				<20000000>;
>>  	vref-supply = <&reg_vcc_3v3_mcu>;
>>  };
>> diff --git a/drivers/iio/adc/vf610_adc.c b/drivers/iio/adc/vf610_adc.c
>> index e63b8e7..b5f94ab8 100644
>> --- a/drivers/iio/adc/vf610_adc.c
>> +++ b/drivers/iio/adc/vf610_adc.c
>> @@ -118,15 +118,21 @@ enum average_sel {
>>  	VF610_ADC_SAMPLE_32,
>>  };
>>
>> +enum conversion_mode_sel {
>> +	VF610_ADC_CONV_NORMAL,
>> +	VF610_ADC_CONV_HIGH_SPEED,
>> +	VF610_ADC_CONV_LOW_POWER,
>> +};
>> +
>>  struct vf610_adc_feature {
>>  	enum clk_sel	clk_sel;
>>  	enum vol_ref	vol_ref;
>> +	enum conversion_mode_sel conv_mode;
>>
>>  	int	clk_div;
>>  	int     sample_rate;
>>  	int	res_mode;
>>
>> -	bool	lpm;
>>  	bool	calibration;
>>  	bool	ovwren;
>>  };
>> @@ -139,6 +145,8 @@ struct vf610_adc {
>>  	u32 vref_uv;
>>  	u32 value;
>>  	struct regulator *vref;
>> +
>> +	u32 max_adck_rate[3];
>>  	struct vf610_adc_feature adc_feature;
>>
>>  	u32 sample_freq_avail[5];
>> @@ -148,46 +156,22 @@ struct vf610_adc {
>>
>>  static const u32 vf610_hw_avgs[] = { 1, 4, 8, 16, 32 };
>>
>> -#define VF610_ADC_CHAN(_idx, _chan_type) {			\
>> -	.type = (_chan_type),					\
>> -	.indexed = 1,						\
>> -	.channel = (_idx),					\
>> -	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
>> -	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
>> -				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
>> -}
>> -
>> -#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
>> -	.type = (_chan_type),	\
>> -	.channel = (_idx),		\
>> -	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
>> -}
>> -
>> -static const struct iio_chan_spec vf610_adc_iio_channels[] = {
>> -	VF610_ADC_CHAN(0, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(1, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(2, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(3, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(4, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(5, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(6, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(7, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(8, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(9, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(10, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(11, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(12, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(13, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(14, IIO_VOLTAGE),
>> -	VF610_ADC_CHAN(15, IIO_VOLTAGE),
>> -	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
>> -	/* sentinel */
>> -};
>> -
>>  static inline void vf610_adc_calculate_rates(struct vf610_adc *info)
>>  {
>> +	struct vf610_adc_feature *adc_feature = &info->adc_feature;
>>  	unsigned long adck_rate, ipg_rate = clk_get_rate(info->clk);
>> -	int i;
>> +	int divisor, i;
>> +
>> +	adck_rate = info->max_adck_rate[adc_feature->conv_mode];
>> +
>> +	if (adck_rate) {
>> +		/* calculate clk divider which is within specification */
>> +		divisor = ipg_rate / adck_rate;
>> +		adc_feature->clk_div = 1 << fls(divisor + 1);
>> +	} else {
>> +		/* fall-back value using a safe divisor */
>> +		adc_feature->clk_div = 8;
>> +	}
>>
>>  	/*
>>  	 * Calculate ADC sample frequencies
>> @@ -219,10 +203,8 @@ static inline void vf610_adc_cfg_init(struct vf610_adc *info)
>>
>>  	adc_feature->res_mode = 12;
>>  	adc_feature->sample_rate = 1;
>> -	adc_feature->lpm = true;
>>
>> -	/* Use a save ADCK which is below 20MHz on all devices */
>> -	adc_feature->clk_div = 8;
>> +	adc_feature->conv_mode = VF610_ADC_CONV_LOW_POWER;
>>
>>  	vf610_adc_calculate_rates(info);
>>  }
>> @@ -307,10 +289,12 @@ static void vf610_adc_cfg_set(struct vf610_adc *info)
>>  	cfg_data = readl(info->regs + VF610_REG_ADC_CFG);
>>
>>  	cfg_data &= ~VF610_ADC_ADLPC_EN;
>> -	if (adc_feature->lpm)
>> +	if (adc_feature->conv_mode == VF610_ADC_CONV_LOW_POWER)
>>  		cfg_data |= VF610_ADC_ADLPC_EN;
>>
>>  	cfg_data &= ~VF610_ADC_ADHSC_EN;
>> +	if (adc_feature->conv_mode == VF610_ADC_CONV_HIGH_SPEED)
>> +		cfg_data |= VF610_ADC_ADHSC_EN;
>>
>>  	writel(cfg_data, info->regs + VF610_REG_ADC_CFG);
>>  }
>> @@ -412,6 +396,81 @@ static void vf610_adc_hw_init(struct vf610_adc *info)
>>  	vf610_adc_cfg_set(info);
>>  }
>>
>> +static int vf610_set_conversion_mode(struct iio_dev *indio_dev,
>> +				     const struct iio_chan_spec *chan,
>> +				     unsigned int mode)
>> +{
>> +	struct vf610_adc *info = iio_priv(indio_dev);
>> +
>> +	mutex_lock(&indio_dev->mlock);
>> +	info->adc_feature.conv_mode = mode;
>> +	vf610_adc_calculate_rates(info);
>> +	vf610_adc_hw_init(info);
>> +	mutex_unlock(&indio_dev->mlock);
>> +
>> +	return 0;
>> +}
>> +
>> +static int vf610_get_conversion_mode(struct iio_dev *indio_dev,
>> +				     const struct iio_chan_spec *chan)
>> +{
>> +	struct vf610_adc *info = iio_priv(indio_dev);
>> +
>> +	return info->adc_feature.conv_mode;
>> +}
>> +
>> +static const char * const vf610_conv_modes[] = { "normal", "high-speed",
>> +						 "low-power" };
>> +
>> +static const struct iio_enum vf610_conversion_mode = {
>> +	.items = vf610_conv_modes,
>> +	.num_items = ARRAY_SIZE(vf610_conv_modes),
>> +	.get = vf610_get_conversion_mode,
>> +	.set = vf610_set_conversion_mode,
>> +};
>> +
>> +static const struct iio_chan_spec_ext_info vf610_ext_info[] = {
>> +	IIO_ENUM("conversion_mode", IIO_SHARED_BY_DIR, &vf610_conversion_mode),
>> +	{},
>> +};
>> +
>> +#define VF610_ADC_CHAN(_idx, _chan_type) {			\
>> +	.type = (_chan_type),					\
>> +	.indexed = 1,						\
>> +	.channel = (_idx),					\
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
>> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
>> +				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
>> +	.ext_info = vf610_ext_info,				\
>> +}
>> +
>> +#define VF610_ADC_TEMPERATURE_CHAN(_idx, _chan_type) {	\
>> +	.type = (_chan_type),	\
>> +	.channel = (_idx),		\
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),	\
>> +}
>> +
>> +static const struct iio_chan_spec vf610_adc_iio_channels[] = {
>> +	VF610_ADC_CHAN(0, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(1, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(2, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(3, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(4, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(5, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(6, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(7, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(8, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(9, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(10, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(11, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(12, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(13, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(14, IIO_VOLTAGE),
>> +	VF610_ADC_CHAN(15, IIO_VOLTAGE),
>> +	VF610_ADC_TEMPERATURE_CHAN(26, IIO_TEMP),
>> +	/* sentinel */
>> +};
>> +
>>  static int vf610_adc_read_data(struct vf610_adc *info)
>>  {
>>  	int result;
>> @@ -654,6 +713,9 @@ static int vf610_adc_probe(struct platform_device *pdev)
>>
>>  	info->vref_uv = regulator_get_voltage(info->vref);
>>
>> +	of_property_read_u32_array(pdev->dev.of_node, "fsl,adck-max-frequency",
>> +			info->max_adck_rate, 3);
>> +
>>  	platform_set_drvdata(pdev, indio_dev);
>>
>>  	init_completion(&info->completion);
>>

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

end of thread, other threads:[~2015-03-28 12:50 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-24 12:47 [PATCH v4 0/3] iio: adc: vf610: respect ADC clocking limitations Stefan Agner
2015-03-24 12:47 ` Stefan Agner
2015-03-24 12:47 ` Stefan Agner
2015-03-24 12:47 ` [PATCH v4 1/3] iio: adc: vf610: use ADC clock within specification Stefan Agner
2015-03-24 12:47   ` Stefan Agner
2015-03-28 12:03   ` Jonathan Cameron
2015-03-28 12:03     ` Jonathan Cameron
2015-03-24 12:47 ` [PATCH v4 2/3] iio: adc: vf610: implement configurable conversion modes Stefan Agner
2015-03-24 12:47   ` Stefan Agner
2015-03-28 12:07   ` Jonathan Cameron
2015-03-28 12:07     ` Jonathan Cameron
2015-03-28 12:07     ` Jonathan Cameron
2015-03-28 12:49     ` Stefan Agner
2015-03-28 12:49       ` Stefan Agner
2015-03-28 12:49       ` Stefan Agner
2015-03-24 12:47 ` [PATCH v4 3/3] ARM: dts: add property for maximum ADC clock frequencies Stefan Agner
2015-03-24 12:47   ` Stefan Agner
2015-03-24 12:47   ` Stefan Agner

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.