Re: [PATCH v2 3/7] staging:iio:ad2s90: Add max frequency check at probe

2018-11-19 Thread Ardelean, Alexandru
On Sun, 2018-11-18 at 02:25 -0200, Matheus Tavares wrote:
> From: Alexandru Ardelean 
> 
> This patch adds a max frequency check at the beginning of ad2s90_probe
> function so that when it is set to a value above 0.83Mhz, dev_err is
> called with an appropriate message and -EINVAL is returned.
> 
> The defined limit is 0.83Mhz instead of 2Mhz, which is the chip's max
> frequency as specified in the datasheet, because, as also specified in
> the datasheet, a 600ns delay is expected between the application of a
> logic LO to CS and the application of SCLK. Since the delay is not
> implemented in the spi code, to satisfy it, SCLK's period should be at
> most 2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which
> gives roughly 83Hz.
> 
> Signed-off-by: Alexandru Ardelean 

I think you can use "Suggested-by:" instead.
But this is also fine.

> Signed-off-by: Matheus Tavares 
> ---
> Changes in v2:
>  - Correctly credit Alexandru as the patch's author
> 
>  drivers/staging/iio/resolver/ad2s90.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s90.c
> b/drivers/staging/iio/resolver/ad2s90.c
> index 913d6fad2d4d..fe90f2056bff 100644
> --- a/drivers/staging/iio/resolver/ad2s90.c
> +++ b/drivers/staging/iio/resolver/ad2s90.c
> @@ -19,6 +19,12 @@
>  #include 
>  #include 
>  
> +/*
> + * Although chip's max frequency is 2Mhz, it needs 600ns between CS and
> the
> + * first falling edge of SCLK, so frequency should be at most 1 / (2 *
> 6e-7)
> + */
> +#define AD2S90_MAX_SPI_FREQ_HZ  83
> +
>  struct ad2s90_state {
>   struct mutex lock;
>   struct spi_device *sdev;
> @@ -78,6 +84,12 @@ static int ad2s90_probe(struct spi_device *spi)
>   struct iio_dev *indio_dev;
>   struct ad2s90_state *st;
>  
> + if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) {
> + dev_err(>dev, "SPI CLK, %d Hz exceeds %d Hz\n",
> + spi->max_speed_hz, AD2S90_MAX_SPI_FREQ_HZ);
> + return -EINVAL;
> + }
> +
>   indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
>   if (!indio_dev)
>   return -ENOMEM;


Re: [PATCH v2 3/7] staging:iio:ad2s90: Add max frequency check at probe

2018-11-19 Thread Ardelean, Alexandru
On Sun, 2018-11-18 at 02:25 -0200, Matheus Tavares wrote:
> From: Alexandru Ardelean 
> 
> This patch adds a max frequency check at the beginning of ad2s90_probe
> function so that when it is set to a value above 0.83Mhz, dev_err is
> called with an appropriate message and -EINVAL is returned.
> 
> The defined limit is 0.83Mhz instead of 2Mhz, which is the chip's max
> frequency as specified in the datasheet, because, as also specified in
> the datasheet, a 600ns delay is expected between the application of a
> logic LO to CS and the application of SCLK. Since the delay is not
> implemented in the spi code, to satisfy it, SCLK's period should be at
> most 2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which
> gives roughly 83Hz.
> 
> Signed-off-by: Alexandru Ardelean 

I think you can use "Suggested-by:" instead.
But this is also fine.

> Signed-off-by: Matheus Tavares 
> ---
> Changes in v2:
>  - Correctly credit Alexandru as the patch's author
> 
>  drivers/staging/iio/resolver/ad2s90.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/staging/iio/resolver/ad2s90.c
> b/drivers/staging/iio/resolver/ad2s90.c
> index 913d6fad2d4d..fe90f2056bff 100644
> --- a/drivers/staging/iio/resolver/ad2s90.c
> +++ b/drivers/staging/iio/resolver/ad2s90.c
> @@ -19,6 +19,12 @@
>  #include 
>  #include 
>  
> +/*
> + * Although chip's max frequency is 2Mhz, it needs 600ns between CS and
> the
> + * first falling edge of SCLK, so frequency should be at most 1 / (2 *
> 6e-7)
> + */
> +#define AD2S90_MAX_SPI_FREQ_HZ  83
> +
>  struct ad2s90_state {
>   struct mutex lock;
>   struct spi_device *sdev;
> @@ -78,6 +84,12 @@ static int ad2s90_probe(struct spi_device *spi)
>   struct iio_dev *indio_dev;
>   struct ad2s90_state *st;
>  
> + if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) {
> + dev_err(>dev, "SPI CLK, %d Hz exceeds %d Hz\n",
> + spi->max_speed_hz, AD2S90_MAX_SPI_FREQ_HZ);
> + return -EINVAL;
> + }
> +
>   indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
>   if (!indio_dev)
>   return -ENOMEM;


[PATCH v2 3/7] staging:iio:ad2s90: Add max frequency check at probe

2018-11-17 Thread Matheus Tavares
From: Alexandru Ardelean 

This patch adds a max frequency check at the beginning of ad2s90_probe
function so that when it is set to a value above 0.83Mhz, dev_err is
called with an appropriate message and -EINVAL is returned.

The defined limit is 0.83Mhz instead of 2Mhz, which is the chip's max
frequency as specified in the datasheet, because, as also specified in
the datasheet, a 600ns delay is expected between the application of a
logic LO to CS and the application of SCLK. Since the delay is not
implemented in the spi code, to satisfy it, SCLK's period should be at
most 2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which
gives roughly 83Hz.

Signed-off-by: Alexandru Ardelean 
Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - Correctly credit Alexandru as the patch's author

 drivers/staging/iio/resolver/ad2s90.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s90.c 
b/drivers/staging/iio/resolver/ad2s90.c
index 913d6fad2d4d..fe90f2056bff 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -19,6 +19,12 @@
 #include 
 #include 
 
+/*
+ * Although chip's max frequency is 2Mhz, it needs 600ns between CS and the
+ * first falling edge of SCLK, so frequency should be at most 1 / (2 * 6e-7)
+ */
+#define AD2S90_MAX_SPI_FREQ_HZ  83
+
 struct ad2s90_state {
struct mutex lock;
struct spi_device *sdev;
@@ -78,6 +84,12 @@ static int ad2s90_probe(struct spi_device *spi)
struct iio_dev *indio_dev;
struct ad2s90_state *st;
 
+   if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) {
+   dev_err(>dev, "SPI CLK, %d Hz exceeds %d Hz\n",
+   spi->max_speed_hz, AD2S90_MAX_SPI_FREQ_HZ);
+   return -EINVAL;
+   }
+
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
-- 
2.18.0



[PATCH v2 3/7] staging:iio:ad2s90: Add max frequency check at probe

2018-11-17 Thread Matheus Tavares
From: Alexandru Ardelean 

This patch adds a max frequency check at the beginning of ad2s90_probe
function so that when it is set to a value above 0.83Mhz, dev_err is
called with an appropriate message and -EINVAL is returned.

The defined limit is 0.83Mhz instead of 2Mhz, which is the chip's max
frequency as specified in the datasheet, because, as also specified in
the datasheet, a 600ns delay is expected between the application of a
logic LO to CS and the application of SCLK. Since the delay is not
implemented in the spi code, to satisfy it, SCLK's period should be at
most 2 * 600ns, so the max frequency should be 1 / (2 * 6e-7), which
gives roughly 83Hz.

Signed-off-by: Alexandru Ardelean 
Signed-off-by: Matheus Tavares 
---
Changes in v2:
 - Correctly credit Alexandru as the patch's author

 drivers/staging/iio/resolver/ad2s90.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/staging/iio/resolver/ad2s90.c 
b/drivers/staging/iio/resolver/ad2s90.c
index 913d6fad2d4d..fe90f2056bff 100644
--- a/drivers/staging/iio/resolver/ad2s90.c
+++ b/drivers/staging/iio/resolver/ad2s90.c
@@ -19,6 +19,12 @@
 #include 
 #include 
 
+/*
+ * Although chip's max frequency is 2Mhz, it needs 600ns between CS and the
+ * first falling edge of SCLK, so frequency should be at most 1 / (2 * 6e-7)
+ */
+#define AD2S90_MAX_SPI_FREQ_HZ  83
+
 struct ad2s90_state {
struct mutex lock;
struct spi_device *sdev;
@@ -78,6 +84,12 @@ static int ad2s90_probe(struct spi_device *spi)
struct iio_dev *indio_dev;
struct ad2s90_state *st;
 
+   if (spi->max_speed_hz > AD2S90_MAX_SPI_FREQ_HZ) {
+   dev_err(>dev, "SPI CLK, %d Hz exceeds %d Hz\n",
+   spi->max_speed_hz, AD2S90_MAX_SPI_FREQ_HZ);
+   return -EINVAL;
+   }
+
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
-- 
2.18.0