Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Received-SPF: Pass (protection.outlook.com: domain of analog.com designates
 137.71.25.55 as permitted sender) receiver=protection.outlook.com;
 client-ip=137.71.25.55; helo=nwd2mta1.analog.com;
From:   Beniamin Bia <beniamin.bia@analog.com>
To:     <jic23@kernel.org>
CC:     <lars@metafoo.de>, <Michael.Hennerich@analog.com>,
        <knaack.h@gmx.de>, <pmeerw@pmeerw.net>,
        <gregkh@linuxfoundation.org>, <linux-iio@vger.kernel.org>,
        <devel@driverdev.osuosl.org>, <linux-kernel@vger.kernel.org>,
        <biabeniamin@outlook.com>, Beniamin Bia <beniamin.bia@analog.com>
Subject: [PATCH v3 1/3] staging: iio: frequency: ad9834: Move frequency to standard iio types
Date:   Mon, 25 Feb 2019 21:17:30 +0200
Message-ID: <20190225191731.5822-1-beniamin.bia@analog.com>
MIME-Version: 1.0
Content-Type: text/plain
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Feb 2019 17:17:55.0648
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 117eabb4-cf88-4725-2b3b-08d69b452e52
X-MS-Exchange-CrossTenant-Id: eaa689b4-8f87-40e0-9c6f-7228de4d754a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=eaa689b4-8f87-40e0-9c6f-7228de4d754a;Ip=[137.71.25.55];Helo=[nwd2mta1.analog.com]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR03MB3754
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Frequency attribute is added with a standard type from iio framework
instead of custom attribute. This is a small step towards removing any
unnecessary custom attribute. Ad9834 will diverge from ad9833 in the
future, that is why we have two identical arrays for ad9834 and 9833.

Signed-off-by: Beniamin Bia <beniamin.bia@analog.com>
---
Changed in v3:
        -based on Jonathan suggestion, i replaced default option with
        Ad9834 DeviceId
        -added a local variable in frequency to simplify the code
        -added ABI documentation

 .../testing/sysfs-bus-iio-frequency-ad9834    | 129 ++++++++++++++++++
 drivers/staging/iio/frequency/ad9834.c        | 104 +++++++++++---
 2 files changed, 216 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9834

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9834 b/Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9834
new file mode 100644
index 000000000000..b912b49473a3
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-frequency-ad9834
@@ -0,0 +1,129 @@
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_frequency
+KernelVersion:	3.5.0
+Date:		April 2012
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Represents the value from frequency register 0 of device. The
+		value is between 0 and clock frequency / 2.
+		Reading returns the value of frequency written in register 0.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage1_frequency
+KernelVersion:	3.5.0
+Date:		April 2012
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Represents the value from frequency register 1 of device. The
+		value is between 0 and clock frequency / 2.
+		Reading returns the value of frequency written in register 1.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_phase0
+KernelVersion:	3.5.0
+Date:		April 2012
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Represents the value from phase register 0 of device. The value
+		is between 0 and 4096 rad.
+		Reading returns the value of phase written in register 0.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_phase1
+KernelVersion:	3.5.0
+Date:		April 2012
+Date:		February 2019
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Represents the value from phase register 1 of device.
+		The value is between 0 and 4096 rad
+		Reading returns the value of phase written in register 1.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_out0_wavetype_available
+KernelVersion:	3.5.0
+Date:		April 2012
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Reading returns the possible waveform of output:
+		sine - for a sinewave
+		triangle - for a triangle signal
+		square - squarewave, this is only available on ad9833/7
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_out0_wavetype
+KernelVersion:	3.5.0
+Date:		April 2012
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Represents the output waveform of channel:
+		sine - for a sinewave
+		triangle - for a triangle signal
+		square - squarewave, this is only available on ad9833/7
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_out1_wavetype_available
+KernelVersion:	3.5.0
+Date:		April 2012
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Reading returns the possible waveform type for accumulator
+		output:
+		square - for a squarewave output
+		nothing - when orbiten is activated
+		This is only available on ad9834
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_out1_wavetype
+KernelVersion:	3.5.0
+Date:		April 2012
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Represents the accumulator output waveform:
+		square or nothing when orbiten is activated
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_pincontrol_en
+KernelVersion:	3.5.0
+Date:		April 2012
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Represents the PIN_SW bit and determines if the device is
+		control by spi or by gpio pins.
+		Reading returns the selected method.
+		Is only available for ad9834.
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_frequencysymbol
+KernelVersion:	3.5.0
+Date:		April 2012
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Represents which frequency register is selected. These devices
+		have two registers for frequency and phase but only one
+		output. The user can select which one controls the output.
+		0 represents frequency 0 which is mapped to
+		out_altvoltage0_frequency
+		1 represents frequency 1 which is mapped to
+		out_altvoltage1_frequency
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_phasesymbol
+KernelVersion:	3.5.0
+Date:		April 2012
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Represents which phase register is selected. These devices
+		have two registers for frequency and phase but only one
+		output. The user can select which one controls the output.
+		0 represents phase 0 which is mapped to
+		out_altvoltage0_phase0
+		1 represents phase 1 which is mapped to
+		out_altvoltage0_phase1
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_out0_enable
+KernelVersion:	3.5.0
+Date:		April 2012
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Enable or disable the output from channel.
+		0 represents disabled
+		1 represents enabled
+
+What:		/sys/bus/iio/devices/iio:deviceX/out_altvoltage0_out1_enable
+KernelVersion:	3.5.0
+Date:		April 2012
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Enable or disable the output from accumulator channel.
+		0 represents disabled
+		1 represents enabled
diff --git a/drivers/staging/iio/frequency/ad9834.c b/drivers/staging/iio/frequency/ad9834.c
index f036f75d1f22..8465dac656dd 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -81,6 +81,8 @@ struct ad9834_state {
 	struct spi_message		freq_msg;
 	struct mutex                    lock;   /* protect sensor state */
 
+	unsigned long			frequency[2];
+
 	/*
 	 * DMA (thus cache coherency maintenance) requires the
 	 * transfer buffers to live in their own cache lines.
@@ -89,6 +91,11 @@ struct ad9834_state {
 	__be16				freq_data[2];
 };
 
+enum ad9834_ch_addr {
+	AD9834_CHANNEL_ADDRESS0,
+	AD9834_CHANNEL_ADDRESS1,
+};
+
 /**
  * ad9834_supported_device_ids:
  */
@@ -100,6 +107,24 @@ enum ad9834_supported_device_ids {
 	ID_AD9838,
 };
 
+#define AD9833_CHANNEL(chan) {						\
+		.type = IIO_ALTVOLTAGE,					\
+		.indexed = 1,						\
+		.output = 1,						\
+		.channel = (chan),					\
+		.info_mask_separate = BIT(IIO_CHAN_INFO_FREQUENCY)	\
+}
+
+static const struct iio_chan_spec ad9833_channels[] = {
+	AD9833_CHANNEL(0),
+	AD9833_CHANNEL(1),
+};
+
+static const struct iio_chan_spec ad9834_channels[] = {
+	AD9833_CHANNEL(0),
+	AD9833_CHANNEL(1),
+};
+
 static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
 {
 	unsigned long long freqreg = (u64)fout * (u64)BIT(AD9834_FREQ_BITS);
@@ -109,10 +134,13 @@ static unsigned int ad9834_calc_freqreg(unsigned long mclk, unsigned long fout)
 }
 
 static int ad9834_write_frequency(struct ad9834_state *st,
-				  unsigned long addr, unsigned long fout)
+				  enum ad9834_ch_addr addr,
+				  unsigned long fout)
 {
+	unsigned long frequency_register;
 	unsigned long clk_freq;
 	unsigned long regval;
+	int ret;
 
 	clk_freq = clk_get_rate(st->mclk);
 
@@ -121,13 +149,24 @@ static int ad9834_write_frequency(struct ad9834_state *st,
 
 	regval = ad9834_calc_freqreg(clk_freq, fout);
 
-	st->freq_data[0] = cpu_to_be16(addr | (regval &
+	if (addr == AD9834_CHANNEL_ADDRESS0)
+		frequency_register = AD9834_REG_FREQ0;
+	else
+		frequency_register = AD9834_REG_FREQ1;
+
+	st->freq_data[0] = cpu_to_be16(frequency_register | (regval &
 				       RES_MASK(AD9834_FREQ_BITS / 2)));
-	st->freq_data[1] = cpu_to_be16(addr | ((regval >>
+	st->freq_data[1] = cpu_to_be16(frequency_register | ((regval >>
 				       (AD9834_FREQ_BITS / 2)) &
 				       RES_MASK(AD9834_FREQ_BITS / 2)));
 
-	return spi_sync(st->spi, &st->freq_msg);
+	ret = spi_sync(st->spi, &st->freq_msg);
+	if (ret)
+		return ret;
+
+	st->frequency[(int)addr] = fout;
+
+	return 0;
 }
 
 static int ad9834_write_phase(struct ad9834_state *st,
@@ -140,6 +179,39 @@ static int ad9834_write_phase(struct ad9834_state *st,
 	return spi_sync(st->spi, &st->msg);
 }
 
+static int ad9834_read_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int *val, int *val2, long mask)
+{
+	struct ad9834_state *st = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_FREQUENCY:
+		*val = st->frequency[chan->channel];
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static int ad9834_write_raw(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan,
+			    int val, int val2, long mask)
+{
+	struct ad9834_state *st = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_FREQUENCY:
+		return ad9834_write_frequency(st,
+					      (enum ad9834_ch_addr)chan->channel,
+					      val);
+	default:
+		return  -EINVAL;
+	}
+
+	return 0;
+}
+
 static ssize_t ad9834_write(struct device *dev,
 			    struct device_attribute *attr,
 			    const char *buf,
@@ -157,10 +229,6 @@ static ssize_t ad9834_write(struct device *dev,
 
 	mutex_lock(&st->lock);
 	switch ((u32)this_attr->address) {
-	case AD9834_REG_FREQ0:
-	case AD9834_REG_FREQ1:
-		ret = ad9834_write_frequency(st, this_attr->address, val);
-		break;
 	case AD9834_REG_PHASE0:
 	case AD9834_REG_PHASE1:
 		ret = ad9834_write_phase(st, this_attr->address, val);
@@ -323,8 +391,6 @@ static IIO_DEVICE_ATTR(out_altvoltage0_out1_wavetype_available, 0444,
  * see dds.h for further information
  */
 
-static IIO_DEV_ATTR_FREQ(0, 0, 0200, NULL, ad9834_write, AD9834_REG_FREQ0);
-static IIO_DEV_ATTR_FREQ(0, 1, 0200, NULL, ad9834_write, AD9834_REG_FREQ1);
 static IIO_DEV_ATTR_FREQSYMBOL(0, 0200, NULL, ad9834_write, AD9834_FSEL);
 static IIO_CONST_ATTR_FREQ_SCALE(0, "1"); /* 1Hz */
 
@@ -342,8 +408,6 @@ static IIO_DEV_ATTR_OUT_WAVETYPE(0, 0, ad9834_store_wavetype, 0);
 static IIO_DEV_ATTR_OUT_WAVETYPE(0, 1, ad9834_store_wavetype, 1);
 
 static struct attribute *ad9834_attributes[] = {
-	&iio_dev_attr_out_altvoltage0_frequency0.dev_attr.attr,
-	&iio_dev_attr_out_altvoltage0_frequency1.dev_attr.attr,
 	&iio_const_attr_out_altvoltage0_frequency_scale.dev_attr.attr,
 	&iio_dev_attr_out_altvoltage0_phase0.dev_attr.attr,
 	&iio_dev_attr_out_altvoltage0_phase1.dev_attr.attr,
@@ -361,8 +425,6 @@ static struct attribute *ad9834_attributes[] = {
 };
 
 static struct attribute *ad9833_attributes[] = {
-	&iio_dev_attr_out_altvoltage0_frequency0.dev_attr.attr,
-	&iio_dev_attr_out_altvoltage0_frequency1.dev_attr.attr,
 	&iio_const_attr_out_altvoltage0_frequency_scale.dev_attr.attr,
 	&iio_dev_attr_out_altvoltage0_phase0.dev_attr.attr,
 	&iio_dev_attr_out_altvoltage0_phase1.dev_attr.attr,
@@ -384,11 +446,15 @@ static const struct attribute_group ad9833_attribute_group = {
 };
 
 static const struct iio_info ad9834_info = {
+	.write_raw = &ad9834_write_raw,
+	.read_raw = &ad9834_read_raw,
 	.attrs = &ad9834_attribute_group,
 	.driver_module = THIS_MODULE,
 };
 
 static const struct iio_info ad9833_info = {
+	.write_raw = &ad9834_write_raw,
+	.read_raw = &ad9834_read_raw,
 	.attrs = &ad9833_attribute_group,
 	.driver_module = THIS_MODULE,
 };
@@ -435,9 +501,13 @@ static int ad9834_probe(struct spi_device *spi)
 	switch (st->devid) {
 	case ID_AD9833:
 	case ID_AD9837:
+		indio_dev->channels = ad9833_channels;
+		indio_dev->num_channels = ARRAY_SIZE(ad9833_channels);
 		indio_dev->info = &ad9833_info;
 		break;
-	default:
+	case ID_AD9834:
+		indio_dev->channels = ad9834_channels;
+		indio_dev->num_channels = ARRAY_SIZE(ad9834_channels);
 		indio_dev->info = &ad9834_info;
 		break;
 	}
@@ -474,11 +544,11 @@ static int ad9834_probe(struct spi_device *spi)
 		goto error_clock_unprepare;
 	}
 
-	ret = ad9834_write_frequency(st, AD9834_REG_FREQ0, 1000000);
+	ret = ad9834_write_frequency(st, AD9834_CHANNEL_ADDRESS0, 1000000);
 	if (ret)
 		goto error_clock_unprepare;
 
-	ret = ad9834_write_frequency(st, AD9834_REG_FREQ1, 5000000);
+	ret = ad9834_write_frequency(st, AD9834_CHANNEL_ADDRESS1, 5000000);
 	if (ret)
 		goto error_clock_unprepare;
 
-- 
2.17.1