[PATCH 00/10] spi: finalize 'delay_usecs' removal/transition
A while back I started the introduction of the 'spi_delay' data type: https://lore.kernel.org/linux-spi/20190926105147.7839-1-alexandru.ardel...@analog.com/ Users of the 'delay_usecs' were removed from drivers. Now it's time to remove the 'delay_usecs' from the SPI subsystem and use only the 'delay' field. This changeset adapts all SPI drivers to do without 'delay_usecs'. Additionally, for greybus we need to adapt it to use the 'delay' in nano-seconds and convert it to micro-seconds. Alexandru Ardelean (10): spi: spi-axi-spi-engine: remove usage of delay_usecs spi: bcm63xx-spi: don't check 'delay_usecs' field spi: spi-bcm-qspi: replace 'delay_usecs' with 'delay.value' check spi: spi-sh: replace 'delay_usecs' with 'delay.value' in pr_debug spi: spi-tegra20-flash: don't check 'delay_usecs' field for spi transfer staging: greybus: spilib: use 'spi_delay_to_ns' for getting xfer delay spi: spi-falcon: remove check for 'delay_usecs' spi: fsl-espi: remove usage of 'delay_usecs' field spi: core: remove 'delay_usecs' field from spi_transfer spi: docs: update info about 'delay_usecs' Documentation/spi/spi-summary.rst | 7 +-- drivers/spi/spi-axi-spi-engine.c | 12 drivers/spi/spi-bcm-qspi.c| 2 +- drivers/spi/spi-bcm63xx.c | 2 +- drivers/spi/spi-falcon.c | 2 +- drivers/spi/spi-fsl-espi.c| 17 + drivers/spi/spi-sh.c | 4 ++-- drivers/spi/spi-tegra20-sflash.c | 3 +-- drivers/spi/spi.c | 1 - drivers/staging/greybus/spilib.c | 5 - include/linux/spi/spi.h | 12 11 files changed, 24 insertions(+), 43 deletions(-) -- 2.29.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/10] spi: spi-axi-spi-engine: remove usage of delay_usecs
The 'delay_usecs' field was handled for backwards compatibility in case
there were some users that still configured SPI delay transfers with
this field.
They should all be removed by now.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-axi-spi-engine.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index af86e6d6e16b..80c3e38f5c1b 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -170,14 +170,10 @@ static void spi_engine_gen_sleep(struct
spi_engine_program *p, bool dry,
unsigned int t;
int delay;
- if (xfer->delay_usecs) {
- delay = xfer->delay_usecs;
- } else {
- delay = spi_delay_to_ns(&xfer->delay, xfer);
- if (delay < 0)
- return;
- delay /= 1000;
- }
+ delay = spi_delay_to_ns(&xfer->delay, xfer);
+ if (delay < 0)
+ return;
+ delay /= 1000;
if (delay == 0)
return;
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/10] spi: bcm63xx-spi: don't check 'delay_usecs' field
The 'delay_usecs' field was handled for backwards compatibility in case
there were some users that still configured SPI delay transfers with
this field.
They should all be removed by now.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-bcm63xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c
index d08bb7600150..80fa0ef8909c 100644
--- a/drivers/spi/spi-bcm63xx.c
+++ b/drivers/spi/spi-bcm63xx.c
@@ -369,7 +369,7 @@ static int bcm63xx_spi_transfer_one(struct spi_master
*master,
}
/* CS will be deasserted directly after transfer */
- if (t->delay_usecs || t->delay.value) {
+ if (t->delay.value) {
dev_err(&spi->dev, "unable to keep CS asserted after
transfer\n");
status = -EINVAL;
goto exit;
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 03/10] spi: spi-bcm-qspi: replace 'delay_usecs' with 'delay.value' check
The 'delay_usecs' field is going away. The replacement for it is the
'delay' field. So, we should check for 'delay.value' being non-zero.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-bcm-qspi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c
index 707fe3a5d8ef..a78e56f566dd 100644
--- a/drivers/spi/spi-bcm-qspi.c
+++ b/drivers/spi/spi-bcm-qspi.c
@@ -671,7 +671,7 @@ static int update_qspi_trans_byte_count(struct bcm_qspi
*qspi,
if (qt->byte >= qt->trans->len) {
/* we're at the end of the spi_transfer */
/* in TX mode, need to pause for a delay or CS change */
- if (qt->trans->delay_usecs &&
+ if (qt->trans->delay.value &&
(flags & TRANS_STATUS_BREAK_DELAY))
ret |= TRANS_STATUS_BREAK_DELAY;
if (qt->trans->cs_change &&
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 04/10] spi: spi-sh: replace 'delay_usecs' with 'delay.value' in pr_debug
The 'delay_usecs' field is going away. The replacement for it is the
'delay' field. So, we should print the 'delay.value' value instead.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-sh.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-sh.c b/drivers/spi/spi-sh.c
index 15123a8f41e1..45f304935332 100644
--- a/drivers/spi/spi-sh.c
+++ b/drivers/spi/spi-sh.c
@@ -290,8 +290,8 @@ static void spi_sh_work(struct work_struct *work)
list_for_each_entry(t, &mesg->transfers, transfer_list) {
pr_debug("tx_buf = %p, rx_buf = %p\n",
t->tx_buf, t->rx_buf);
- pr_debug("len = %d, delay_usecs = %d\n",
- t->len, t->delay_usecs);
+ pr_debug("len = %d, delay.value = %d\n",
+ t->len, t->delay.value);
if (t->tx_buf) {
ret = spi_sh_send(ss, mesg, t);
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/10] spi: spi-tegra20-flash: don't check 'delay_usecs' field for spi transfer
The 'delay_usecs' field was handled for backwards compatibility in case
there were some users that still configured SPI delay transfers with
this field.
They should all be removed by now. So we can remove the 'delay_usecs'
handling in this driver.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-tegra20-sflash.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/spi/spi-tegra20-sflash.c b/drivers/spi/spi-tegra20-sflash.c
index cfb7de737937..2888d8a8dc6d 100644
--- a/drivers/spi/spi-tegra20-sflash.c
+++ b/drivers/spi/spi-tegra20-sflash.c
@@ -341,8 +341,7 @@ static int tegra_sflash_transfer_one_message(struct
spi_master *master,
goto exit;
}
msg->actual_length += xfer->len;
- if (xfer->cs_change &&
- (xfer->delay_usecs || xfer->delay.value)) {
+ if (xfer->cs_change && xfer->delay.value) {
tegra_sflash_writel(tsd, tsd->def_command_reg,
SPI_COMMAND);
spi_transfer_delay_exec(xfer);
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 06/10] staging: greybus: spilib: use 'spi_delay_to_ns' for getting xfer delay
The intent is the removal of the 'delay_usecs' field from the
spi_transfer struct, as there is a 'delay' field that does the same
thing.
The spi_delay_to_ns() can be used to get the transfer delay. It works by
using the 'delay_usecs' field first (if it is non-zero), and finally
uses the 'delay' field.
Since the 'delay_usecs' field is going away, this change makes use of the
spi_delay_to_ns() function. This also means dividing the return value of
the function by 1000, to convert it to microseconds.
To prevent any potential faults when converting to microseconds and since
the result of spi_delay_to_ns() is int, the delay is being computed in 32
bits and then clamped between 0 & U16_MAX.
Signed-off-by: Alexandru Ardelean
---
drivers/staging/greybus/spilib.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c
index 672d540d3365..30655153df6a 100644
--- a/drivers/staging/greybus/spilib.c
+++ b/drivers/staging/greybus/spilib.c
@@ -245,6 +245,7 @@ static struct gb_operation *gb_spi_operation_create(struct
gb_spilib *spi,
/* Fill in the transfers array */
xfer = spi->first_xfer;
while (msg->state != GB_SPI_STATE_OP_DONE) {
+ int xfer_delay;
if (xfer == spi->last_xfer)
xfer_len = spi->last_xfer_size;
else
@@ -259,7 +260,9 @@ static struct gb_operation *gb_spi_operation_create(struct
gb_spilib *spi,
gb_xfer->speed_hz = cpu_to_le32(xfer->speed_hz);
gb_xfer->len = cpu_to_le32(xfer_len);
- gb_xfer->delay_usecs = cpu_to_le16(xfer->delay_usecs);
+ xfer_delay = spi_delay_to_ns(&xfer->delay, xfer) / 1000;
+ xfer_delay = clamp_t(u16, xfer_delay, 0, U16_MAX);
+ gb_xfer->delay_usecs = cpu_to_le16(xfer_delay);
gb_xfer->cs_change = xfer->cs_change;
gb_xfer->bits_per_word = xfer->bits_per_word;
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 07/10] spi: spi-falcon: remove check for 'delay_usecs'
The 'delay_usecs' field is being removed from the spi_transfer struct. This change removes it from the SPI Falcon driver. Signed-off-by: Alexandru Ardelean --- drivers/spi/spi-falcon.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/spi-falcon.c b/drivers/spi/spi-falcon.c index d3336a63f462..a7d4dffac66b 100644 --- a/drivers/spi/spi-falcon.c +++ b/drivers/spi/spi-falcon.c @@ -377,7 +377,7 @@ static int falcon_sflash_xfer_one(struct spi_master *master, m->actual_length += t->len; - WARN_ON(t->delay_usecs || t->delay.value || t->cs_change); + WARN_ON(t->delay.value || t->cs_change); spi_flags = 0; } -- 2.29.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 08/10] spi: fsl-espi: remove usage of 'delay_usecs' field
The 'delay_usecs' field is being removed from the spi_transfer struct.
This change removes it from the SPI FSL ESPI driver.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi-fsl-espi.c | 17 +
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index cf2b947c600e..f7066bef7b06 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -435,8 +435,7 @@ static int fsl_espi_trans(struct spi_message *m, struct
spi_transfer *trans)
static int fsl_espi_do_one_msg(struct spi_master *master,
struct spi_message *m)
{
- unsigned int delay_usecs = 0, rx_nbits = 0;
- unsigned int delay_nsecs = 0, delay_nsecs1 = 0;
+ unsigned int rx_nbits = 0, delay_nsecs = 0;
struct spi_transfer *t, trans = {};
int ret;
@@ -445,16 +444,10 @@ static int fsl_espi_do_one_msg(struct spi_master *master,
goto out;
list_for_each_entry(t, &m->transfers, transfer_list) {
- if (t->delay_usecs) {
- if (t->delay_usecs > delay_usecs) {
- delay_usecs = t->delay_usecs;
- delay_nsecs = delay_usecs * 1000;
- }
- } else {
- delay_nsecs1 = spi_delay_to_ns(&t->delay, t);
- if (delay_nsecs1 > delay_nsecs)
- delay_nsecs = delay_nsecs1;
- }
+ unsigned int delay = spi_delay_to_ns(&t->delay, t);
+
+ if (delay > delay_nsecs)
+ delay_nsecs = delay;
if (t->rx_nbits > rx_nbits)
rx_nbits = t->rx_nbits;
}
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 09/10] spi: core: remove 'delay_usecs' field from spi_transfer
The 'delay' field in the spi_transfer struct is meant to replace the
'delay_usecs' field. However some cleanup was required to remove the
uses of 'delay_usecs'. Now that it's been cleaned up, we can remove it
from the kernel tree.
Signed-off-by: Alexandru Ardelean
---
drivers/spi/spi.c | 1 -
include/linux/spi/spi.h | 12
2 files changed, 13 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b08efe88ccd6..481427780162 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3178,7 +3178,6 @@ struct spi_replaced_transfers *spi_replace_transfers(
/* clear cs_change and delay for all but the last */
if (i) {
xfer->cs_change = false;
- xfer->delay_usecs = 0;
xfer->delay.value = 0;
}
}
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 592897fa4f03..ea1784a43267 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -832,9 +832,6 @@ extern void spi_res_release(struct spi_controller *ctlr,
* @delay: delay to be introduced after this transfer before
* (optionally) changing the chipselect status, then starting
* the next transfer or completing this @spi_message.
- * @delay_usecs: microseconds to delay after this transfer before
- * (optionally) changing the chipselect status, then starting
- * the next transfer or completing this @spi_message.
* @word_delay: inter word delay to be introduced after each word size
* (set by bits_per_word) transmission.
* @effective_speed_hz: the effective SCK-speed that was used to
@@ -946,7 +943,6 @@ struct spi_transfer {
#defineSPI_NBITS_DUAL 0x02 /* 2bits transfer */
#defineSPI_NBITS_QUAD 0x04 /* 4bits transfer */
u8 bits_per_word;
- u16 delay_usecs;
struct spi_delaydelay;
struct spi_delaycs_change_delay;
struct spi_delayword_delay;
@@ -1060,14 +1056,6 @@ spi_transfer_del(struct spi_transfer *t)
static inline int
spi_transfer_delay_exec(struct spi_transfer *t)
{
- struct spi_delay d;
-
- if (t->delay_usecs) {
- d.value = t->delay_usecs;
- d.unit = SPI_DELAY_UNIT_USECS;
- return spi_delay_exec(&d, NULL);
- }
-
return spi_delay_exec(&t->delay, t);
}
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 10/10] spi: docs: update info about 'delay_usecs'
The 'delay_usecs' field is no longer present on the spi_transfer struct. This change updates the doc to mention the usage of the (relatively) new 'delay' field. Signed-off-by: Alexandru Ardelean --- Documentation/spi/spi-summary.rst | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Documentation/spi/spi-summary.rst b/Documentation/spi/spi-summary.rst index f1daffe10d78..d4239025461d 100644 --- a/Documentation/spi/spi-summary.rst +++ b/Documentation/spi/spi-summary.rst @@ -411,8 +411,11 @@ any more such messages. duplex (one pointer is NULL) transfers; + optionally defining short delays after transfers ... using -the spi_transfer.delay_usecs setting (this delay can be the -only protocol effect, if the buffer length is zero); +the spi_transfer.delay.value setting (this delay can be the +only protocol effect, if the buffer length is zero) ... +when specifying this delay the default spi_transfer.delay.unit +is microseconds, however this can be adjusted to clock cycles +or nanoseconds if needed; + whether the chipselect becomes inactive after a transfer and any delay ... by using the spi_transfer.cs_change flag; -- 2.29.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/10] spi: spi-axi-spi-engine: remove usage of delay_usecs
On Mon, 8 Mar 2021 at 18:42, Lars-Peter Clausen wrote:
>
> On 3/8/21 3:54 PM, Alexandru Ardelean wrote:
> > The 'delay_usecs' field was handled for backwards compatibility in case
> > there were some users that still configured SPI delay transfers with
> > this field.
> >
> > They should all be removed by now.
> >
> > Signed-off-by: Alexandru Ardelean
> > ---
> > drivers/spi/spi-axi-spi-engine.c | 12
> > 1 file changed, 4 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/spi/spi-axi-spi-engine.c
> > b/drivers/spi/spi-axi-spi-engine.c
> > index af86e6d6e16b..80c3e38f5c1b 100644
> > --- a/drivers/spi/spi-axi-spi-engine.c
> > +++ b/drivers/spi/spi-axi-spi-engine.c
> > @@ -170,14 +170,10 @@ static void spi_engine_gen_sleep(struct
> > spi_engine_program *p, bool dry,
> > unsigned int t;
> > int delay;
> >
> > - if (xfer->delay_usecs) {
> > - delay = xfer->delay_usecs;
> > - } else {
> > - delay = spi_delay_to_ns(&xfer->delay, xfer);
> > - if (delay < 0)
> > - return;
> > - delay /= 1000;
> > - }
> > + delay = spi_delay_to_ns(&xfer->delay, xfer);
> > + if (delay < 0)
> > + return;
>
> Bit of a nit, but this could be `delay <= 0` and then drop the check for
> `delay == 0` below.
hmm, that's a bit debatable, because the `delay == 0` check comes
after `delay /= 1000` ;
to do what you're suggesting, it would probably need a
DIV_ROUND_UP(delay, 1000) to make sure that even sub-microsecond
delays don't become zero;
if you're acking this suggestion i'll implement it;
i'll wait a few more days to see if there are any other acks or
complaints on the set before sending a V2;
btw: this new spi_delay struct supports delays in microseconds,
nanoseconds and clock cycles;
at some point it may be interesting to use a
`spi_delay_to_clk_cycles()` for this driver and other similar;
>
> > + delay /= 1000;
> >
> > if (delay == 0)
> > return;
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: ad9834: convert to device-managed functions in probe
This change converts the driver to use device-managed functions in the
probe function. For the clock and regulator disable, some
devm_add_action_or_reset() calls are required, and then
devm_iio_device_register() function can be used register the IIO device.
The final aim here would be for IIO to export only the device-managed
functions of it's API. That's a long way to go and this a small step in
that direction.
Signed-off-by: Alexandru Ardelean
---
drivers/staging/iio/frequency/ad9834.c | 64 +-
1 file changed, 31 insertions(+), 33 deletions(-)
diff --git a/drivers/staging/iio/frequency/ad9834.c
b/drivers/staging/iio/frequency/ad9834.c
index 262c3590e64e..b063cfd0e0e1 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -390,6 +390,20 @@ static const struct iio_info ad9833_info = {
.attrs = &ad9833_attribute_group,
};
+static void ad9834_disable_reg(void *data)
+{
+ struct regulator *reg = data;
+
+ regulator_disable(reg);
+}
+
+static void ad9834_disable_clk(void *data)
+{
+ struct clk *clk = data;
+
+ clk_disable_unprepare(clk);
+}
+
static int ad9834_probe(struct spi_device *spi)
{
struct ad9834_state *st;
@@ -407,26 +421,33 @@ static int ad9834_probe(struct spi_device *spi)
return ret;
}
+ ret = devm_add_action_or_reset(&spi->dev, ad9834_disable_reg, reg);
+ if (ret)
+ return ret;
+
indio_dev = devm_iio_device_alloc(&spi->dev, sizeof(*st));
if (!indio_dev) {
ret = -ENOMEM;
- goto error_disable_reg;
+ return ret;
}
- spi_set_drvdata(spi, indio_dev);
st = iio_priv(indio_dev);
mutex_init(&st->lock);
st->mclk = devm_clk_get(&spi->dev, NULL);
if (IS_ERR(st->mclk)) {
ret = PTR_ERR(st->mclk);
- goto error_disable_reg;
+ return ret;
}
ret = clk_prepare_enable(st->mclk);
if (ret) {
dev_err(&spi->dev, "Failed to enable master clock\n");
- goto error_disable_reg;
+ return ret;
}
+ ret = devm_add_action_or_reset(&spi->dev, ad9834_disable_clk, st->mclk);
+ if (ret)
+ return ret;
+
st->spi = spi;
st->devid = spi_get_device_id(spi)->driver_data;
st->reg = reg;
@@ -470,48 +491,26 @@ static int ad9834_probe(struct spi_device *spi)
ret = spi_sync(st->spi, &st->msg);
if (ret) {
dev_err(&spi->dev, "device init failed\n");
- goto error_clock_unprepare;
+ return ret;
}
ret = ad9834_write_frequency(st, AD9834_REG_FREQ0, 100);
if (ret)
- goto error_clock_unprepare;
+ return ret;
ret = ad9834_write_frequency(st, AD9834_REG_FREQ1, 500);
if (ret)
- goto error_clock_unprepare;
+ return ret;
ret = ad9834_write_phase(st, AD9834_REG_PHASE0, 512);
if (ret)
- goto error_clock_unprepare;
+ return ret;
ret = ad9834_write_phase(st, AD9834_REG_PHASE1, 1024);
if (ret)
- goto error_clock_unprepare;
-
- ret = iio_device_register(indio_dev);
- if (ret)
- goto error_clock_unprepare;
-
- return 0;
-error_clock_unprepare:
- clk_disable_unprepare(st->mclk);
-error_disable_reg:
- regulator_disable(reg);
-
- return ret;
-}
-
-static int ad9834_remove(struct spi_device *spi)
-{
- struct iio_dev *indio_dev = spi_get_drvdata(spi);
- struct ad9834_state *st = iio_priv(indio_dev);
-
- iio_device_unregister(indio_dev);
- clk_disable_unprepare(st->mclk);
- regulator_disable(st->reg);
+ return ret;
- return 0;
+ return devm_iio_device_register(&spi->dev, indio_dev);
}
static const struct spi_device_id ad9834_id[] = {
@@ -539,7 +538,6 @@ static struct spi_driver ad9834_driver = {
.of_match_table = ad9834_of_match
},
.probe = ad9834_probe,
- .remove = ad9834_remove,
.id_table = ad9834_id,
};
module_spi_driver(ad9834_driver);
--
2.29.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: iio: adc: ad7192: Add clock for external clock reference
On Fri, Jan 25, 2019 at 12:41 AM Stephen Boyd wrote: > > Quoting Jonathan Cameron (2018-12-16 02:07:41) > > Rob, Clk experts, questions for you below. > > > > Jonathan > > > > > > On Thu, 13 Dec 2018 17:39:22 -0800 > > Stephen Boyd wrote: > > > > > Quoting Jonathan Cameron (2018-12-08 07:29:54) > > > > On Thu, 6 Dec 2018 11:10:51 +0200 > > > > Mircea Caprioru wrote: > > > > > > > > > This patch adds a clock to the state structure of ad7192 for getting > > > > > the > > > > > external clock frequency. This modifications is in accordance with > > > > > clock > > > > > framework dt bindings documentation. > > > > > > > > > > Signed-off-by: Mircea Caprioru > > > > > > > > +cc Rob and the clk list for advise on how to do the binding for this > > > > one. > > > > > > > > It is basically 2 pins, you can put a clock in on one of them or connect > > > > a crystal across them. The driver has to set a register to say which is > > > > the case. > > > > > > > > Current proposal is two optional clocks (fall back to internal > > > > oscillator) > > > > but that doesn't seem to be commonly done, so I'm wondering if there > > > > is a 'standard' way to handle this sort of thing. > > > > > > > > > > I'm not sure I fully understand, but I think perhaps > > > assigned-clock-parents would work? Or does that not work because either > > > way some parent is assigned, either the crystal or the optional clk that > > > isn't a crystal? > > > > > My concern is they aren't really separate clock inputs. They are just > > different > > ways of providing the same fundamental clock. So I think we may want to > > just > > provide a single clock and have another dt binding to say what it is. > > > > So lots of ways we could do it, but I'm not sure what the right one to > > go with is! > > > > Sorry for getting back to this so late. Is the datasheet public for this > device? If so, any link to it? > Hey, Link is http://analog.com/ad7192 and that should resolve to the proper page, where the datasheet is available. [ General info: most [if not all] datasheets from Analog Devices can be found by concatenating "http://analog.com/ + "" ] But more directly, the link to the PDF is: https://www.analog.com/media/en/technical-documentation/data-sheets/AD7192.pdf Page 10 provides some description of the pins, page 20 the mode register for the clock, and page 32 a general description of the clock. If you search for MCLK1 or MCLK2 you should navigate pretty quick through the doc. The clock circuitry [the 2 pins] is common for all chips this driver supports [AD7190/2/3/5]. Thanks Alex > If it's two pins, and sometimes one pin is connected and other times two > pins are connected but a register needs to be set if the pins are > connected in one configuration or the other I would say your plan for a > DT property indicating how the pins are configured sounds good. Usually > the hardware can figure these things out itself so I find this sort of > odd, but if this is how it is then there's not much that we can do. > > It sounds like there aren't two different clk inputs to the device. > Given that information specifying two optional clks is incorrect because > there is only one 'slot' is the external clk source. > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro wrote: > > Remove the checkpatch.pl check: > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? Hey, A bit curios about this one. Are you using this chip/driver ? Thing is: the part is nearing EOL, and it could be an idea to be marked for removal (since it's still in staging). But if there are users for this driver, it could be left around for a while. Thanks Alex > > Signed-off-by: Rodrigo Ribeiro > Signed-off-by: Rafael Tsuha > --- > This macro is not used anywhere. Should we just correct the > spelling or remove the macro? > > drivers/staging/iio/cdc/ad7152.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/iio/cdc/ad7152.c > b/drivers/staging/iio/cdc/ad7152.c > index 25f51db..c9da6d4 100644 > --- a/drivers/staging/iio/cdc/ad7152.c > +++ b/drivers/staging/iio/cdc/ad7152.c > @@ -35,7 +35,7 @@ > #define AD7152_REG_CH2_GAIN_HIGH 12 > #define AD7152_REG_CH2_SETUP 14 > #define AD7152_REG_CFG 15 > -#define AD7152_REG_RESEVERD16 > +#define AD7152_REG_RESERVED16 > #define AD7152_REG_CAPDAC_POS 17 > #define AD7152_REG_CAPDAC_NEG 18 > #define AD7152_REG_CFG226 > -- > 2.7.4 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Sat, Jan 26, 2019 at 8:09 PM Jonathan Cameron wrote: > > On Fri, 25 Jan 2019 10:19:54 +0200 > Alexandru Ardelean wrote: > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > wrote: > > > > > > Remove the checkpatch.pl check: > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > Hey, > > > > A bit curios about this one. > > Are you using this chip/driver ? > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > marked for removal (since it's still in staging). > > But if there are users for this driver, it could be left around for a while. > > While it might be going away soon, I'm also not that bothered about > the small amount of churn that a tidy up patch like this causes, > and it might not go away ;) > > However it is rather odd to have a 'reserved' entry for a register. > can't see that providing any useful information. Normally I'm > happy to have complete register sets as a form of documentation > if the author wants to do it that way. This however seems silly. > > Alex, we haven't really gone with marking things as 'going away' > before. I'd suggest we take a guess and remove it if you and the > team an analog don't think it's in use. Happy to get a patch for > that if you want to send one. Of course, Rodrigo could do that > patch to get started if that works for everyone? :) > We'll also start a discussion about this particular driver internally. And maybe a procedure for removing drivers that become obsolete [or come close to it]. We also don't/didn't have one for removing "going away" drivers; I just remembered that we took a look over this one and decided not to invest time into it as it's close to being obsolete. Thanks Alex > Jonathan > > > > Thanks > > Alex > > > > > > > > Signed-off-by: Rodrigo Ribeiro > > > Signed-off-by: Rafael Tsuha > > > --- > > > This macro is not used anywhere. Should we just correct the > > > spelling or remove the macro? > > > > > > drivers/staging/iio/cdc/ad7152.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/iio/cdc/ad7152.c > > > b/drivers/staging/iio/cdc/ad7152.c > > > index 25f51db..c9da6d4 100644 > > > --- a/drivers/staging/iio/cdc/ad7152.c > > > +++ b/drivers/staging/iio/cdc/ad7152.c > > > @@ -35,7 +35,7 @@ > > > #define AD7152_REG_CH2_GAIN_HIGH 12 > > > #define AD7152_REG_CH2_SETUP 14 > > > #define AD7152_REG_CFG 15 > > > -#define AD7152_REG_RESEVERD16 > > > +#define AD7152_REG_RESERVED16 > > > #define AD7152_REG_CAPDAC_POS 17 > > > #define AD7152_REG_CAPDAC_NEG 18 > > > #define AD7152_REG_CFG226 > > > -- > > > 2.7.4 > > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:iio:ad7152: Rename misspelled RESEVERD -> RESERVED
On Sat, Jan 26, 2019 at 8:13 PM Jonathan Cameron wrote: > > On Fri, 25 Jan 2019 22:14:32 -0200 > Rodrigo Ribeiro wrote: > > > Em sex, 25 de jan de 2019 às 21:46, Rodrigo Ribeiro > > escreveu: > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > escreveu: > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > > > wrote: > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > Hey, > > > > > > Hi, > > > > > > Thanks for answering. > > > > > > > A bit curios about this one. > > > > Are you using this chip/driver ? > > > > > > No, I am just a student at USP (University of São Paulo) starting in Linux > > > Kernel and a new member of the USP Linux Kernel group that has contributed > > > for a few months. > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > > > marked for removal (since it's still in staging). > > > > But if there are users for this driver, it could be left around for a > > > > while. > > > > > > This is my first patch in Linux Kernel, but if the driver will be > > > removed, I > > > can send a patch for another driver. Is there any driver that I can > > > fix a style warning? > > > > Maybe, one checkstyle patch is enough, right? Which drivers can I truly > > contribute to? > > How about the ad7150? That one is still listed as production. > What do you think Alex, you probably have better visibility on the > status of these parts and their drivers than I do! > > (note I haven't even opened that one for a few years so no idea > what state the driver is in!) > ad7150 is a good alternative. At a first glance over the driver it looks like it could do with some polish/conversions to newer IIO constructs (like IIO triggers, maybe some newer event handling mechanisms?). I'll sync with Stefan [Popa] to see about this stuff at a later point in time. I'd also add here the adxl345 driver. This is mostly informational for anyone who'd find this interesting. There are 2 drivers for this chip, one in IIO [drivers/iio/accel/adxl345] and another one in "drivers/misc/adxl34x.c" as part of the input sub-system. What would be interesting here is to finalize the IIO driver [ I think some features are lacking behind the input driver], and make the input driver a consumer of the IIO driver. From my side, both these variants are fine to take on. The ad7150 is a good idea as a starter project, and the adxl one is more of a long-term medium-level project. Thanks Alex > Jonathan > > > > > > Thanks > > > > > > > > > Em sex, 25 de jan de 2019 às 06:20, Alexandru Ardelean > > > escreveu: > > > > > > > > On Thu, Jan 24, 2019 at 9:35 PM Rodrigo Ribeiro > > > > wrote: > > > > > > > > > > Remove the checkpatch.pl check: > > > > > > > > > > CHECK: 'RESEVERD' may be misspelled - perhaps 'RESERVED'? > > > > > > > > Hey, > > > > > > > > A bit curios about this one. > > > > Are you using this chip/driver ? > > > > > > > > Thing is: the part is nearing EOL, and it could be an idea to be > > > > marked for removal (since it's still in staging). > > > > But if there are users for this driver, it could be left around for a > > > > while. > > > > > > > > Thanks > > > > Alex > > > > > > > > > > > > > > Signed-off-by: Rodrigo Ribeiro > > > > > Signed-off-by: Rafael Tsuha > > > > > --- > > > > > This macro is not used anywhere. Should we just correct the > > > > > spelling or remove the macro? > > > > > > > > > > drivers/staging/iio/cdc/ad7152.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/staging/iio/cdc/ad7152.c > > > > > b/drivers/staging/iio/cdc/ad7152.c > > > > > index 25f51db..c9da6d4 100644 > > > > > --- a/drivers/staging/iio/cdc/ad7152.c > > > > > +++ b/drivers/staging/iio/cdc/ad7152.c > > > > > @@ -35,7 +35,7 @@ > > > > > #define AD7152_REG_CH2_GAIN_HIGH 12 > > > > > #define AD7152_REG_CH2_SETUP 14 > > > > > #define AD7152_REG_CFG 15 > > > > > -#define AD7152_REG_RESEVERD16 > > > > > +#define AD7152_REG_RESERVED16 > > > > > #define AD7152_REG_CAPDAC_POS 17 > > > > > #define AD7152_REG_CAPDAC_NEG 18 > > > > > #define AD7152_REG_CFG226 > > > > > -- > > > > > 2.7.4 > > > > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 0/9] staging: iio: ad7780: move out of staging
On Sun, Mar 3, 2019 at 3:52 PM Renato Lui Geh wrote: > > Hi Alexandru, > > Thanks for the review. Some questions inline. > > Thanks, > Renato > > On 03/01, Ardelean, Alexandru wrote: > >On Thu, 2019-02-28 at 11:23 -0300, Renato Lui Geh wrote: > >> > > > >The patch-series is a bit big. > >I guess that the intent is to move this out-of-staging, but various patches > >are holding this in it's place. > >For patch series above a certain size, you could get many re-spins > >[V2,3,4... so on]. > > > >You could send some of the changes as individual patches, or group them in > >series of 1,2 or 3 patches. That way, you "parallelize" patch sending, and > >when you get reviews on each patch, you can re-spin them individually. > >You'll find over time that certain patches get accepted on V1, others on V2 > >and some on V7 [ hopefully, there isn't any frustration at that point ]. > > On these subseries, should versioning follow this patchset (v5) or should > they start anew (v1), ignoring this series version? I guess, in this case it's fine to leave it as is [in this series]. The series has been reviewed now. But [for me typically], I delay doing a review if a patch-series is longer than 4-5 patches. And I think some reviewers may do the same. So, if I want more people to review/look at my code, I try to make things as easy to review, as possible. And one way, is to definitely keep things decoupled. If one patch can be independent of another [for the same driver/code], I send them as separate patches. This [of course], is a preference. Some reviewers don't mind longer series [than 4-5 patches]. > > > >Well, this is a technique I use to distribute some of my upstream-patch- > >work, so that I can switch easier between internal-work & upstreaming-work. > > > >Coming back to this patch-series. > >My general input, is that the patches are fine over-all; some are just > >cosmetics/noise/a-different-way-of-doing-things-for-this-driver, and those > >usually can be left to preference [of the maintainer usually]. > > > >I do suggest to not hurry when re-spinning patches, and not change too much > >the number of patches in a new series. That can complicate things > >sometimes. But, if doing small patch-series or individual patches, you > >won't have this problem too much. > > > >Thanks > >Alex > > > >> > >> This series of patches contains the following: > >> - Adds user input for the 'gain' and 'filter' GPIO pins for the ad778x > >>family chips; > >> - Filter reading for the ad778x; > >> - Sets pattern macro values and mask for PATTERN status bits; > >> - Adds ID values for the ad7170, ad7171, ad7780 and ad7781 for ID > >>status bits checking; > >> - Moves regulator initialization to after GPIO init to maintain > >>consistency between probe and remove; > >> - Copyright edits, adding SPDX identifier and new copyright holder; > >> - Moves the ad7780 driver out of staging to the mainline; > >> - Adds device tree binding for the ad7780 driver. > >> > >> Renato Lui Geh (9): > >> staging: iio: ad7780: add gain & filter gpio support > >> staging: iio: ad7780: add filter reading to ad778x > >> staging: iio: ad7780: set pattern values and masks directly > >> staging:iio:ad7780: add chip ID values and mask > >> staging: iio: ad7780: move regulator to after GPIO init > >> staging: iio: ad7780: add SPDX identifier > >> staging: iio: ad7780: add new copyright holder > >> staging: iio: ad7780: moving ad7780 out of staging > >> staging: iio: ad7780: add device tree binding > >> > >> Changelog: > >> *v3 > >> - SPDX and regulator init as patches > >> - Renamed filter to odr and ad778x_filter to ad778x_odr_avail > >> - Removed unnecessary regulator disabling > >> - Removed unnecessary AD_SD_CHANNEL macro > >> - Changed unsigned int to unsigned long long to avoid overflow > >> *v4 > >> - Split gain & filter patch into two, with the new commit adding only > >>filter reading > >> - Changed pattern values to direct values, and added pattern mask > >> - Added ID values and mask > >> - Added new copyright holder > >> - Added device tree binding to the ad7780 driver > >> > >> .../bindings/iio/adc/adi,ad7780.txt | 48 +++ > >> drivers/iio/adc/Kconfig | 12 + > >> drivers/iio/adc/Makefile | 1 + > >> drivers/iio/adc/ad7780.c | 365 ++ > >> drivers/staging/iio/adc/Kconfig | 13 - > >> drivers/staging/iio/adc/Makefile | 1 - > >> drivers/staging/iio/adc/ad7780.c | 277 - > >> 7 files changed, 426 insertions(+), 291 deletions(-) > >> create mode 100644 > >> Documentation/devicetree/bindings/iio/adc/adi,ad7780.txt > >> create mode 100644 drivers/iio/adc/ad7780.c > >> delete mode 100644 drivers/staging/iio/adc/ad7780.c > >> > >> -- > >> 2.21.0 > >> > > > >-- > >You received this message because you are subscribed to the Google Groups > >"Kernel USP" gr
Re: [PATCH] staging: iio: adc: ad7192: Add spaces around minus operator
On Sun, Mar 10, 2019 at 11:23 PM Karen Palacio wrote: > > Add spaces around minus operator to fix readibility. > > Signed-off-by: Karen Palacio > --- > drivers/staging/iio/adc/ad7192.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/iio/adc/ad7192.c > b/drivers/staging/iio/adc/ad7192.c > index acdbc07..7c632cf 100644 > --- a/drivers/staging/iio/adc/ad7192.c > +++ b/drivers/staging/iio/adc/ad7192.c > @@ -355,7 +355,7 @@ ad7192_show_scale_available(struct device *dev, > } > > static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available, > -in_voltage-voltage_scale_available, > +in_voltage - voltage_scale_available, This isn't broken, but I do agree it should be addressed. I think it's the second time I see a similar patch trying to fix this. So, obviously the code is a bit misleading. > 0444, ad7192_show_scale_available, NULL, 0); > > static IIO_DEVICE_ATTR(in_voltage_scale_available, 0444, > -- > 2.7.4 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Help on testing ad5933 driver
On Thu, Mar 21, 2019 at 9:39 PM Marcelo Schmitt wrote: > > Hello, would anyone mind helping me test ad5933 driver on actual > hardware? I went through this > (https://oslongjourney.github.io/linux-kernel/experiment-one-iio-dummy/) > tutorial so I was able to load iio_simple_dummy driver, create and > inspect some dummy devices. Now, as Jonathan has asked me, I would like > to test ad5933 driver on an EVAL-AD5933 board which was donated to FLUSP > (https://flusp.ime.usp.br/). > > So far I've been hesitating to plug this device on my Debian distro > since this > (https://www.analog.com/media/en/technical-documentation/user-guides/UG-364.pdf) > user guide for Windows says not to connect it before driver > installation. Is there something that could harm the board if plugged > on a computer without a proper driver? > You should take into account that a lot of eval boards have their eval SW written for Windows. This is something of a legacy-thing, because most corporations have been running their computers (for work & dev & offices) on Windows. So, you shouldn't take things ad-literam (to the letter) when reading stuff for Windows and when writing code for Linux. > I also didn't understand the hardware configuration showed on this > (https://wiki.analog.com/resources/tools-software/linux-drivers/iio-impedance-analyzer/ad5933) > page. Hmm, that doc was written a while ago. The newer eval board doesn't look like the one in the wiki. Also, since the eval SW was targeted for Windows, getting it to work for Linux (and IIO) implies some hacking/reverse-engineering. The reason for this (reverse-engineering) is because [traditionally] eval boards are meant to highlight characteristics of the chip, so if using Windows, this should be simple. Unfortunately, the docs aren't helping in this case. So, in this case, I would get some volt-meter & oscilloscope to help. It looks to me that U2 is the AD5933 on the eval-board. Worst case, you can solder directly to the pins and link them to a Raspberry PI [on the I2C], power, ground, etc. But, you can take a look at the T1 to T8 (if I didn't miss anything) and connect to them, and see what each of them is for. Hopefully, one of those test-points is for I2C, in which case you can attach wires to them and connect them to a host. I did not find a good doc for them yet. But anyway, I would ask some HW guy to help here (because I'm a SW guy mostly),and get help on figuring out the eval board > > Any advice will be greatly appreciated. > Thanks in advance, > > Marcelo ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] staging: iio: ad9832: add devicetree documentation
On Mon, Apr 1, 2019 at 5:38 PM Marcelo Schmitt
wrote:
>
> Add a devicetree documentation for the ad9832 direct digital
> synthesizer, waveform generator.
>
> Signed-off-by: Marcelo Schmitt
> ---
> .../bindings/iio/frequency/ad9832.txt | 26 +++
> 1 file changed, 26 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/frequency/ad9832.txt
>
> diff --git a/Documentation/devicetree/bindings/iio/frequency/ad9832.txt
> b/Documentation/devicetree/bindings/iio/frequency/ad9832.txt
> new file mode 100644
> index ..6a35fdff5a48
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/frequency/ad9832.txt
> @@ -0,0 +1,26 @@
> +Analog Devices AD9832 Direct Digital Synthesizer, Waveform Generator
> +
> +Data sheet:
> +https://www.analog.com/media/en/technical-documentation/data-sheets/AD9832.pdf
> +
> +Required properties:
> + - compatible : Must be "adi,ad9832"
> + - reg : SPI chip select number for the device
> + - spi-max-frequency = Max SPI frequency to use (< 2500)
> + - clocks : The clock reference for the DDS output
> + - clock-names : Must be "mclk"
It's always a good idea to reference other base dt docs.
For SPI you could:
```
For more information on SPI properties, please consult
Documentation/devicetree/bindings/spi/spi-bus.txt
```
For clock:
```
For more information on clock bindings properties, please consult
Documentation/devicetree/bindings/clock/clock-bindings.txt
```
For regulator:
```
For more information on regulator bindings properties, please consult
Documentation/devicetree/bindings/regulator/regulator.txt
```
> +
> +Optional properties:
> + - avdd-supply: Definition of the regulator used as analog supply
> + - dvdd-supply : Definition of the regulator used as digital supply
> +
> +Example:
> + adi9832-dds@0 {
> + compatible = "adi,ad9832";
> + reg = <0>;
> + spi-max-frequency = <2500>;
> + clocks = <&ad9832_mclk>;
> + clock-names = "mclk";
> + avdd-suppy = <&avdd>;
> + dvdd-suppy = <&dvdd>;
> + };
> --
> 2.20.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Work on iio: stating: frequency: ad9832
On Mon, Apr 1, 2019 at 7:13 PM Jonathan Cameron wrote: > > On Mon, 1 Apr 2019 11:25:29 -0300 > Marcelo Schmitt wrote: > > > Hello, > > > > I was looking for some work on staging: iio: ad9832 and made some > > observations while reading the driver. > > > > Apparently it had no devicetree documentation so I tried to elaborate > > one. > > It uses a platform_data variable to load external clock > > frequency (I tried to make it use linux's clock framework). > Good. > > > Some device attributes don't seem to be standardized on > > Documentation/ABI/testing/sysfs-bus-iio and there's no specific ABI > > for ad9832 nearby nor at staging/iio/Documentation. So maybe those > > missing ABI could be documented. > Beware. It's an old driver, so it may be that we actually want to change > it's ABI rather than documenting what is there (I have haven't looked!) > This one can actually be coupled a bit with the AD9834 driver. There's been some work on trying to move that one out of staging as well. You can take a look at the patches sent for that driver. They should be find-able on patchwork https://patchwork.kernel.org/project/linux-iio/list/?series=&submitter=&state=*&q=ad9834&archive=both&delegate= There are ideas worth borrowing from there. The issue with the AD9834 [if i recall correctly] is that it doesn't quite fit the classical IIO channel model. Meaning, you can only activate the output of one channel at one moment in time, and not both. > > The device has to set some internal registers to operate correctly, > > AD9832_FREQXHM and AD9832_PHASEXH, would it be feasible to set iio > > chanels for this? > > What are they? If they correspond to output channels in some sensible > way then maybe... > > > I couldn't understand why checkpatch.pl gave errors on IIO_DEV_ATTR_* > > macros. To me they seem to have no problem. > > Also it has that platform_data to be moved to include/linux/iio. Is > > there any special reason for it not being there already? Which are > > the criterions a platform_data need to satisfy to be put there? > A driver moving out of staging shouldn't have platform data. It needs > to be converted over to more modern mechanisms. We don't have a problem > supporting platform data for devices that have old school device files > already in tree, but that shouldn't be the case for a driver in staging. > > Hence we can clean it up and move forward with just DT bindings. > > > > I'm sending a patchset with some things I've already done. > Cool. I'll look at those later in the week if no one beats me to them. > > > > > Is there something else that could be done in this device driver? > > Please, tell if I've forgotten something. > > I'll take a look, but it may be a little while before I do. > Hopefully someone else gets there first! > > Jonathan > > > > > Any advice is welcome. > > Thanks, > > > > Marcelo > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: cdc: ad7746: Replace bitshift by BIT
On Wed, Apr 3, 2019 at 11:46 PM Lucas Oshiro wrote: > > Replace bitshifts on lines 54, 56 and 78 of ad7746.c. > Hey, This is only partially done. If doing conversions to BIT(x) macro, I would say to do them for all cases. Thanks Alex > Signed-off-by: Lucas Oshiro > --- > drivers/staging/iio/cdc/ad7746.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/staging/iio/cdc/ad7746.c > b/drivers/staging/iio/cdc/ad7746.c > index 0eb28fea876e..ea48b14cee72 100644 > --- a/drivers/staging/iio/cdc/ad7746.c > +++ b/drivers/staging/iio/cdc/ad7746.c > @@ -51,9 +51,9 @@ > #define AD7746_CAPSETUP_CACHOP BIT(0) > > /* Voltage/Temperature Setup Register Bit Designations (AD7746_REG_VT_SETUP) > */ > -#define AD7746_VTSETUP_VTEN(1 << 7) > +#define AD7746_VTSETUP_VTENBIT(7) > #define AD7746_VTSETUP_VTMD_INT_TEMP (0 << 5) > -#define AD7746_VTSETUP_VTMD_EXT_TEMP (1 << 5) > +#define AD7746_VTSETUP_VTMD_EXT_TEMP BIT(5) > #define AD7746_VTSETUP_VTMD_VDD_MON(2 << 5) > #define AD7746_VTSETUP_VTMD_EXT_VIN(3 << 5) > #define AD7746_VTSETUP_EXTREF BIT(4) > @@ -75,7 +75,7 @@ > #define AD7746_CONF_VTFS_MASK GENMASK(7, 6) > #define AD7746_CONF_CAPFS_MASK GENMASK(5, 3) > #define AD7746_CONF_MODE_IDLE (0 << 0) > -#define AD7746_CONF_MODE_CONT_CONV (1 << 0) > +#define AD7746_CONF_MODE_CONT_CONV BIT(0) > #define AD7746_CONF_MODE_SINGLE_CONV (2 << 0) > #define AD7746_CONF_MODE_PWRDN (3 << 0) > #define AD7746_CONF_MODE_OFFS_CAL (5 << 0) > -- > 2.21.0 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: fbtft: fbtft-core: Fix last line displayed on fbcon
From: Michael Hennerich
For the special case when fbtft_mkdirty() is called with with -1 for the y
coordinate, the height is truncated by 1.
This isn't required, and causes the last line to not update.
Signed-off-by: Michael Hennerich
Signed-off-by: Alexandru Ardelean
---
drivers/staging/fbtft/fbtft-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/fbtft/fbtft-core.c
b/drivers/staging/fbtft/fbtft-core.c
index cf5700a2ea66..90eec45d11fc 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -317,7 +317,7 @@ static void fbtft_mkdirty(struct fb_info *info, int y, int
height)
/* special case, needed ? */
if (y == -1) {
y = 0;
- height = info->var.yres - 1;
+ height = info->var.yres;
}
/* Mark display lines/area as dirty */
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: make efuse_power_switch() function static
The `rtl8188eu` driver is built as a kmod in order to avoid symbol
conflicts (at link-time) with other Realtek drivers.
Internally, we use this driver as builtin [vs kmod], and we've identified
the `efuse_power_switch()` symbol to be conflicting at link-time with the
one from the `rtlwifi` driver.
An alternative solution would have been to rename the function, but it
doesn't look like it's being used outside of this driver, so just make it
static.
Signed-off-by: Alexandru Ardelean
---
drivers/staging/rtl8188eu/core/rtw_efuse.c| 2 +-
drivers/staging/rtl8188eu/include/rtw_efuse.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_efuse.c
b/drivers/staging/rtl8188eu/core/rtw_efuse.c
index 02c476f45b33..47eaff9ffd58 100644
--- a/drivers/staging/rtl8188eu/core/rtw_efuse.c
+++ b/drivers/staging/rtl8188eu/core/rtw_efuse.c
@@ -25,7 +25,7 @@ enum{
* When we want to enable write operation, we should change to pwr on state.
* When we stop write, we should switch to 500k mode and disable LDO 2.5V.
*/
-void efuse_power_switch(struct adapter *pAdapter, u8 write, u8 pwrstate)
+static void efuse_power_switch(struct adapter *pAdapter, u8 write, u8 pwrstate)
{
u8 tempval;
u16 tmpv16;
diff --git a/drivers/staging/rtl8188eu/include/rtw_efuse.h
b/drivers/staging/rtl8188eu/include/rtw_efuse.h
index 3ec53761e9fd..7a9c8ff0daa9 100644
--- a/drivers/staging/rtl8188eu/include/rtw_efuse.h
+++ b/drivers/staging/rtl8188eu/include/rtw_efuse.h
@@ -82,7 +82,6 @@ u8 efuse_OneByteWrite(struct adapter *adapter, u16 addr, u8
data);
void efuse_ReadEFuse(struct adapter *Adapter, u8 efuseType, u16 _offset,
u16 _size_byte, u8 *pbuf);
-void efuse_power_switch(struct adapter *adapt, u8 write, u8 pwrstate);
int Efuse_PgPacketRead(struct adapter *adapt, u8 offset, u8 *data);
bool Efuse_PgPacketWrite(struct adapter *adapter, u8 offset, u8 word, u8
*data);
void efuse_WordEnableDataRead(u8 word_en, u8 *sourdata, u8 *targetdata);
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: iio: ad7150: organize registers definition
On Sat, May 4, 2019 at 1:25 AM Melissa Wen wrote:
>
> Use the suffix REG to make the register addresses clear
> and indentation to highlight field names.
>
I'm inclined to say that this change is a bit too much noise versus added value.
While the REG suffix does make sense (generally), since it hasn't been
added from the beginning, it doesn't make much sense to add it now.
It is sufficiently clear (as-is) that these macros refer to registers.
They should be easy to match with the datasheet as well.
> Signed-off-by: Melissa Wen
> ---
> drivers/staging/iio/cdc/ad7150.c | 75
> 1 file changed, 37 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/staging/iio/cdc/ad7150.c
> b/drivers/staging/iio/cdc/ad7150.c
> index dd7fcab8e19e..24601ba7db88 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -15,35 +15,34 @@
> #include
> #include
> #include
> -/*
> - * AD7150 registers definition
> - */
>
> -#define AD7150_STATUS 0
> -#define AD7150_STATUS_OUT1 BIT(3)
> -#define AD7150_STATUS_OUT2 BIT(5)
> -#define AD7150_CH1_DATA_HIGH 1
> -#define AD7150_CH2_DATA_HIGH 3
> -#define AD7150_CH1_AVG_HIGH5
> -#define AD7150_CH2_AVG_HIGH7
> -#define AD7150_CH1_SENSITIVITY 9
> -#define AD7150_CH1_THR_HOLD_H 9
> -#define AD7150_CH1_TIMEOUT 10
> -#define AD7150_CH1_SETUP 11
> -#define AD7150_CH2_SENSITIVITY 12
> -#define AD7150_CH2_THR_HOLD_H 12
> -#define AD7150_CH2_TIMEOUT 13
> -#define AD7150_CH2_SETUP 14
> -#define AD7150_CFG 15
> -#define AD7150_CFG_FIX BIT(7)
> -#define AD7150_PD_TIMER16
> -#define AD7150_CH1_CAPDAC 17
> -#define AD7150_CH2_CAPDAC 18
> -#define AD7150_SN3 19
> -#define AD7150_SN2 20
> -#define AD7150_SN1 21
> -#define AD7150_SN0 22
> -#define AD7150_ID 23
> +/* AD7150 registers */
> +
> +#define AD7150_STATUS_REG 0x00
> +#define AD7150_STATUS_OUT1 BIT(3)
> +#define AD7150_STATUS_OUT2 BIT(5)
> +#define AD7150_CH1_DATA_HIGH_REG 0x01
> +#define AD7150_CH2_DATA_HIGH_REG 0x03
> +#define AD7150_CH1_AVG_HIGH_REG0x05
> +#define AD7150_CH2_AVG_HIGH_REG0x07
> +#define AD7150_CH1_SENSITIVITY_REG 0x09
> +#define AD7150_CH1_THR_HOLD_H_REG 0x09
> +#define AD7150_CH2_SENSITIVITY_REG 0x0C
> +#define AD7150_CH1_TIMEOUT_REG 0x0A
> +#define AD7150_CH1_SETUP_REG 0x0B
> +#define AD7150_CH2_THR_HOLD_H_REG 0x0C
> +#define AD7150_CH2_TIMEOUT_REG 0x0D
> +#define AD7150_CH2_SETUP_REG 0x0E
> +#define AD7150_CFG_REG 0x0F
> +#define AD7150_CFG_FIX BIT(7)
> +#define AD7150_PD_TIMER_REG0x10
> +#define AD7150_CH1_CAPDAC_REG 0x11
> +#define AD7150_CH2_CAPDAC_REG 0x12
> +#define AD7150_SN3_REG 0x13
> +#define AD7150_SN2_REG 0x14
> +#define AD7150_SN1_REG 0x15
> +#define AD7150_SN0_REG 0x16
> +#define AD7150_ID_REG 0x17
>
> /**
> * struct ad7150_chip_info - instance specific chip data
> @@ -85,12 +84,12 @@ struct ad7150_chip_info {
> */
>
> static const u8 ad7150_addresses[][6] = {
> - { AD7150_CH1_DATA_HIGH, AD7150_CH1_AVG_HIGH,
> - AD7150_CH1_SETUP, AD7150_CH1_THR_HOLD_H,
> - AD7150_CH1_SENSITIVITY, AD7150_CH1_TIMEOUT },
> - { AD7150_CH2_DATA_HIGH, AD7150_CH2_AVG_HIGH,
> - AD7150_CH2_SETUP, AD7150_CH2_THR_HOLD_H,
> - AD7150_CH2_SENSITIVITY, AD7150_CH2_TIMEOUT },
> + { AD7150_CH1_DATA_HIGH_REG, AD7150_CH1_AVG_HIGH_REG,
> + AD7150_CH1_SETUP_REG, AD7150_CH1_THR_HOLD_H_REG,
> + AD7150_CH1_SENSITIVITY_REG, AD7150_CH1_TIMEOUT_REG },
> + { AD7150_CH2_DATA_HIGH_REG, AD7150_CH2_AVG_HIGH_REG,
> + AD7150_CH2_SETUP_REG, AD7150_CH2_THR_HOLD_H_REG,
> + AD7150_CH2_SENSITIVITY_REG, AD7150_CH2_TIMEOUT_REG },
> };
>
> static int ad7150_read_raw(struct iio_dev *indio_dev,
> @@ -133,7 +132,7 @@ static int ad7150_read_event_config(struct iio_dev
> *indio_dev,
> bool adaptive;
> struct ad7150_chip_info *chip = iio_priv(indio_dev);
>
> - ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
> + ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG_REG);
> if (ret < 0)
> return ret;
>
> @@ -229,7 +228,7 @@ static int ad7150_write_event_config(struct iio_dev
> *indio_dev,
> if (event_code == chip->current_event)
> return 0;
> mutex_lock(&chip->state_lock
Re: [PATCH 3/4] staging: iio: ad7150: simplify i2c SMBus return treatment
On Sat, May 4, 2019 at 1:26 AM Melissa Wen wrote: > > Since i2c_smbus_write_byte_data returns no-positive value, this commit > making the treatment of its return value less verbose. > > Signed-off-by: Melissa Wen > --- > drivers/staging/iio/cdc/ad7150.c | 10 +++--- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/drivers/staging/iio/cdc/ad7150.c > b/drivers/staging/iio/cdc/ad7150.c > index 4ba46fb6ac02..3a4572a9e5ec 100644 > --- a/drivers/staging/iio/cdc/ad7150.c > +++ b/drivers/staging/iio/cdc/ad7150.c > @@ -201,16 +201,12 @@ static int ad7150_write_event_params(struct iio_dev > *indio_dev, > ret = i2c_smbus_write_byte_data(chip->client, > ad7150_addresses[chan][4], > sens); > - if (ret < 0) > + if (ret) For i2c_smbus_write_byte_data(), checking "ret < 0" or non-zero, is the same. Changing this doesn't have any added value. > return ret; > - > - ret = i2c_smbus_write_byte_data(chip->client, > + else > + return i2c_smbus_write_byte_data(chip->client, > ad7150_addresses[chan][5], > timeout); The introduction of the "else" branch is a bit noisy. The code was a bit neater (and readable) before the else branch, and functionally identical. Well, when I say neater before, you have to understand, that I (and I assume that some other people who write drivers) have a slight fixation for this pattern: example1: ret = fn1(); if (ret < 0) // could also be just "if (ret)" return ret; ret = fn2(); if (ret < 0) // could also be just "if (ret)" return ret; example1a: +ret = fn3(); +if (ret < 0) // could also be just "if (ret)" +return ret; Various higher-level programming languages, will discourage this pattern in favor of neater patterns. I personally, have a few arguments in favor of this pattern: 1) it is closer to how the machine code ; so, closer to how a low-level instruction looks like 2) if (ever) this needs to be patched, the patch could be neat (see example1a) ; the examle assumes that it's been added via a patch at a later point in time 3) it keeps indentation level to a minimum ; this also aligns with kernel-coding guidelines (https://www.kernel.org/doc/html/v4.10/process/coding-style.html ) (indentation seems a bit OCD-like when someone points it out at a review, but it has it's value over time) > - if (ret < 0) > - return ret; > - > - return 0; > } > > static int ad7150_write_event_config(struct iio_dev *indio_dev, > -- > 2.20.1 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: iio: ad7150: use FIELD_GET and GENMASK
On Sat, May 4, 2019 at 1:25 AM Melissa Wen wrote:
>
> Use the bitfield macro FIELD_GET, and GENMASK to do the shift and mask in
> one go. This makes the code more readable than explicit masking followed
> by a shift.
>
This looks neat.
I'd have to remember to ack it from my work email.
One minor comment inline, which would be the object of a new patch.
> Signed-off-by: Melissa Wen
> ---
> drivers/staging/iio/cdc/ad7150.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/cdc/ad7150.c
> b/drivers/staging/iio/cdc/ad7150.c
> index 24601ba7db88..4ba46fb6ac02 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -5,6 +5,7 @@
> * Copyright 2010-2011 Analog Devices Inc.
> */
>
> +#include
> #include
> #include
> #include
> @@ -44,6 +45,9 @@
> #define AD7150_SN0_REG 0x16
> #define AD7150_ID_REG 0x17
>
> +/* AD7150 masks */
> +#define AD7150_THRESHTYPE_MSK GENMASK(6, 5)
> +
> /**
> * struct ad7150_chip_info - instance specific chip data
> * @client: i2c client for this device
> @@ -136,7 +140,7 @@ static int ad7150_read_event_config(struct iio_dev
> *indio_dev,
> if (ret < 0)
> return ret;
>
> - threshtype = (ret >> 5) & 0x03;
> + threshtype = FIELD_GET(AD7150_THRESHTYPE_MSK, ret);
> adaptive = !!(ret & 0x80);
Why not also do something similar for the `adaptive` value ?
>
> switch (type) {
> --
> 2.20.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/4] staging: iio: ad7150: improve driver readability
On Sat, May 4, 2019 at 1:24 AM Melissa Wen wrote: > > This patchset solves readability issues in AD7150 code, such as clarify > register and mask definition, fashion improvement of mask uses, reduce > tedious operation and useless comments. > Hey, Two patches seem a bit noisy/un-needed. The other 2 are fine from me. This driver does need some work to move it out of staging. I am not sure what would be a big blocker for it, other than maybe it needs a device-tree binding doc (in YAML format). Maybe Jonathan remembers. Some other low-hanging-fruit ideas would be: 1) remove the code for platform_data ; that one seems forgotten from some other time; the interrupts should be coming from device-tree, from the i2c bindings 2) you could do a AD7150_EVENT_SPEC() macro (similar to AD7150_TIMEOUT() macro) and use it in the ad7150_events[] list; that would reduce a few lines 3) similar to 2), you could do a AD7150_CHANNEL(x) macro ; 4) in ad7150_event_handler() the checks could be wrapped into a macro, or maybe some function ; i am referring to "(int_status & AD7150_STATUS_OUT1) && (chip->old_state & AD7150_STATUS_OUT1)" checks ; those seem to be repeated 5) add of_match_table to the driver I (now) suspect that the reason this driver is still in staging is this comment: /* Timeouts not currently handled by core */ I wonder if things changed since then ? If not, it would be interesting to implement it in core. Thanks Alex > Melissa Wen (4): > staging: iio: ad7150: organize registers definition > staging: iio: ad7150: use FIELD_GET and GENMASK > staging: iio: ad7150: simplify i2c SMBus return treatment > staging: iio: ad7150: clean up of comments > > drivers/staging/iio/cdc/ad7150.c | 102 ++- > 1 file changed, 47 insertions(+), 55 deletions(-) > > -- > 2.20.1 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/4] staging: iio: ad7150: improve driver readability
On Sat, May 4, 2019 at 2:12 PM Alexandru Ardelean wrote: > > On Sat, May 4, 2019 at 1:24 AM Melissa Wen wrote: > > > > This patchset solves readability issues in AD7150 code, such as clarify > > register and mask definition, fashion improvement of mask uses, reduce > > tedious operation and useless comments. > > > > Hey, > > Two patches seem a bit noisy/un-needed. > The other 2 are fine from me. > > This driver does need some work to move it out of staging. > I am not sure what would be a big blocker for it, other than maybe it > needs a device-tree binding doc (in YAML format). > Maybe Jonathan remembers. > > Some other low-hanging-fruit ideas would be: > 1) remove the code for platform_data ; that one seems forgotten from > some other time; the interrupts should be coming from device-tree, > from the i2c bindings > 2) you could do a AD7150_EVENT_SPEC() macro (similar to > AD7150_TIMEOUT() macro) and use it in the ad7150_events[] list; that > would reduce a few lines > 3) similar to 2), you could do a AD7150_CHANNEL(x) macro ; > 4) in ad7150_event_handler() the checks could be wrapped into a macro, > or maybe some function ; i am referring to "(int_status & > AD7150_STATUS_OUT1) && (chip->old_state & AD7150_STATUS_OUT1)" checks > ; those seem to be repeated > 5) add of_match_table to the driver > > I (now) suspect that the reason this driver is still in staging is this > comment: > /* Timeouts not currently handled by core */ > > I wonder if things changed since then ? > If not, it would be interesting to implement it in core. > I forgot to mention the wiki page for the driver: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-cdc/ad7150 it may help with a few things > Thanks > Alex > > > > Melissa Wen (4): > > staging: iio: ad7150: organize registers definition > > staging: iio: ad7150: use FIELD_GET and GENMASK > > staging: iio: ad7150: simplify i2c SMBus return treatment > > staging: iio: ad7150: clean up of comments > > > > drivers/staging/iio/cdc/ad7150.c | 102 ++- > > 1 file changed, 47 insertions(+), 55 deletions(-) > > > > -- > > 2.20.1 > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 04/16] powerpc/xmon: use new match_string() helper/macro
The change is purely cosmetic at this point in time, but it does highlight
the change done in lib/string.c for match_string().
Particularly for this change, if a regname is removed (replaced with NULL)
in the list, the match_string() helper will continue until the end of the
array and ignore the NULL.
This would technically allow for "reserved" regs, though here it's not the
case.
Signed-off-by: Alexandru Ardelean
---
arch/powerpc/xmon/xmon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index efca104ac0cb..b84a7fc1112b 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3231,7 +3231,7 @@ scanhex(unsigned long *vp)
regname[i] = c;
}
regname[i] = 0;
- i = __match_string(regnames, N_PTREGS, regname);
+ i = match_string(regnames, regname);
if (i < 0) {
printf("invalid register name '%%%s'\n", regname);
return 0;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/16] treewide: rename match_string() -> __match_string()
This change does a rename of match_string() -> __match_string().
There are a few parts to the intention here (with this change):
1. Align with sysfs_match_string()/__sysfs_match_string()
2. This helps to group users of `match_string()` into simple users:
a. those that use ARRAY_SIZE(_a) to specify the number of elements
b. those that use -1 to pass a NULL terminated array of strings
c. special users, which (after eliminating 1 & 2) are not that many
3. The final intent is to fix match_string()/__match_string() which is
slightly broken, in the sense that passing -1 or a positive value does
not make any difference: the iteration will stop at the first NULL
element.
Signed-off-by: Alexandru Ardelean
---
arch/powerpc/xmon/xmon.c | 2 +-
arch/x86/kernel/cpu/mtrr/if.c| 2 +-
drivers/ata/pata_hpt366.c| 2 +-
drivers/ata/pata_hpt37x.c| 2 +-
drivers/base/devcon.c| 2 +-
drivers/base/property.c | 2 +-
drivers/clk/bcm/clk-bcm2835.c| 6 +++---
drivers/clk/clk.c| 4 ++--
drivers/clk/rockchip/clk.c | 4 ++--
drivers/cpufreq/intel_pstate.c | 2 +-
drivers/gpio/gpiolib-of.c| 2 +-
drivers/gpu/drm/drm_edid_load.c | 2 +-
drivers/gpu/drm/drm_panel_orientation_quirks.c | 2 +-
drivers/gpu/drm/i915/intel_pipe_crc.c| 2 +-
drivers/ide/hpt366.c | 2 +-
drivers/mfd/omap-usb-host.c | 2 +-
drivers/mmc/host/sdhci-xenon-phy.c | 2 +-
drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 2 +-
drivers/pci/pcie/aer.c | 2 +-
drivers/phy/tegra/xusb.c | 2 +-
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 4 ++--
drivers/pinctrl/pinmux.c | 2 +-
drivers/power/supply/ab8500_btemp.c | 2 +-
drivers/power/supply/ab8500_charger.c| 2 +-
drivers/power/supply/ab8500_fg.c | 2 +-
drivers/power/supply/abx500_chargalg.c | 2 +-
drivers/power/supply/charger-manager.c | 4 ++--
drivers/staging/gdm724x/gdm_tty.c| 4 ++--
drivers/usb/common/common.c | 4 ++--
drivers/usb/typec/class.c| 10 +-
drivers/usb/typec/tps6598x.c | 2 +-
drivers/vfio/vfio.c | 6 +++---
drivers/video/fbdev/pxafb.c | 2 +-
fs/ubifs/auth.c | 4 ++--
include/linux/string.h | 2 +-
kernel/cgroup/rdma.c | 2 +-
kernel/sched/debug.c | 2 +-
kernel/trace/trace.c | 2 +-
lib/string.c | 8
mm/mempolicy.c | 2 +-
mm/vmpressure.c | 4 ++--
security/apparmor/lsm.c | 4 ++--
security/integrity/ima/ima_main.c| 2 +-
sound/firewire/oxfw/oxfw.c | 2 +-
sound/pci/oxygen/oxygen_mixer.c | 2 +-
sound/soc/codecs/max98088.c | 2 +-
sound/soc/codecs/max98095.c | 2 +-
sound/soc/soc-dapm.c | 2 +-
48 files changed, 68 insertions(+), 68 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a0f44f992360..efca104ac0cb 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3231,7 +3231,7 @@ scanhex(unsigned long *vp)
regname[i] = c;
}
regname[i] = 0;
- i = match_string(regnames, N_PTREGS, regname);
+ i = __match_string(regnames, N_PTREGS, regname);
if (i < 0) {
printf("invalid register name '%%%s'\n", regname);
return 0;
diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
index 4d36dcc1cf87..4ec7a5f7b94c 100644
--- a/arch/x86/kernel/cpu/mtrr/if.c
+++ b/arch/x86/kernel/cpu/mtrr/if.c
@@ -142,7 +142,7 @@ mtrr_write(struct file *file, const char __user *buf,
size_t len, loff_t * ppos)
return -EINVAL;
ptr = skip_spaces(ptr + 5);
- i = match_string(mtrr_strings, MTRR_NUM_TYPES, ptr);
+ i = __match_string(mtrr_strings, MTRR_NUM_TYPES, ptr);
if (i < 0)
return i;
diff --git a/drivers/ata/pata_hpt366.c b/drivers/ata/pata_hpt366.c
index a219a503c229..4ba5fc9d20be 100644
--- a/drivers/ata/pata_hpt366.c
+++ b/drivers/ata/pata_hpt366.c
@@ -180,7 +180,7 @@ static int hpt_dma_blacklisted(const struc
[PATCH 07/16] device connection: use new match_string() helper/macro
The `device_connection` struct is defined as:
struct device_connection {
struct fwnode_handle*fwnode;
const char *endpoint[2];
const char *id;
struct list_headlist;
};
The `endpoint` member is a static array of strings (on the struct), so
using the match_string() (which does an ARRAY_SIZE((con->endpoint)) should
be fine.
The recent change to match_string() (to ignore NULL entries up to the size
of the array) shouldn't affect this.
Signed-off-by: Alexandru Ardelean
---
drivers/base/devcon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/devcon.c b/drivers/base/devcon.c
index 7bc1c619b721..4a2338665585 100644
--- a/drivers/base/devcon.c
+++ b/drivers/base/devcon.c
@@ -70,7 +70,7 @@ void *device_connection_find_match(struct device *dev, const
char *con_id,
mutex_lock(&devcon_lock);
list_for_each_entry(con, &devcon_list, list) {
- ep = __match_string(con->endpoint, 2, devname);
+ ep = match_string(con->endpoint, devname);
if (ep < 0)
continue;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/16] ALSA: oxygen: use new match_string() helper/macro
The change is purely cosmetic at this point in time, but it does highlight
the change done in lib/string.c for match_string().
Particularly for this change, a control mode can be removed/added at a
different index/enum-value, and the match_string() helper will continue
until the end of the array and ignore the NULL.
Signed-off-by: Alexandru Ardelean
---
sound/pci/oxygen/oxygen_mixer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/pci/oxygen/oxygen_mixer.c b/sound/pci/oxygen/oxygen_mixer.c
index 13c2fb75fd71..961fd1cbc712 100644
--- a/sound/pci/oxygen/oxygen_mixer.c
+++ b/sound/pci/oxygen/oxygen_mixer.c
@@ -1086,7 +1086,7 @@ static int add_controls(struct oxygen *chip,
err = snd_ctl_add(chip->card, ctl);
if (err < 0)
return err;
- j = __match_string(known_ctl_names, CONTROL_COUNT,
ctl->id.name);
+ j = match_string(known_ctl_names, ctl->id.name);
if (j >= 0) {
chip->controls[j] = ctl;
ctl->private_free = oxygen_any_ctl_free;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/16] treewide: fix match_string() helper when array size
The intent of this patch series is to make a case for fixing the match_string() string helper. The doc-string of the `__sysfs_match_string()` helper mentions that `n` (the size of the given array) should be: * @n: number of strings in the array or -1 for NULL terminated arrays However, this is not the case. The helper stops on the first NULL in the array, regardless of whether -1 is provided or not. There are some advantages to allowing this behavior (NULL elements within in the array). One example, is to allow reserved registers as NULL in an array. One example in the series is patch: x86/mtrr: use new match_string() helper + add gaps == minor fix which uses a "?" string for values that are reserved/don't care. Since the change is a bit big, the change was coupled with renaming match_string() -> __match_string(). The new match_string() helper (resulted here) does an ARRAY_SIZE() over the array, which is useful when the array is static. Also, this way of doing things is a way to go through all the users of this helpers and check that nothing goes wrong, and notify them about the change to match_string(). It's a way of grouping changes in a manage-able way. The first patch is important, the others can be dropped. Signed-off-by: Alexandru Ardelean Alexandru Ardelean (16): lib: fix match_string() helper when array size is positive treewide: rename match_string() -> __match_string() lib,treewide: add new match_string() helper/macro powerpc/xmon: use new match_string() helper/macro ALSA: oxygen: use new match_string() helper/macro x86/mtrr: use new match_string() helper + add gaps == minor fix device connection: use new match_string() helper/macro cpufreq/intel_pstate: remove NULL entry + use match_string() mmc: sdhci-xenon: use new match_string() helper/macro pinctrl: armada-37xx: use new match_string() helper/macro mm/vmpressure.c: use new match_string() helper/macro rdmacg: use new match_string() helper/macro drm/edid: use new match_string() helper/macro staging: gdm724x: use new match_string() helper/macro video: fbdev: pxafb: use new match_string() helper/macro sched: debug: use new match_string() helper/macro arch/powerpc/xmon/xmon.c | 2 +- arch/x86/kernel/cpu/mtrr/if.c| 10 ++ drivers/ata/pata_hpt366.c| 2 +- drivers/ata/pata_hpt37x.c| 2 +- drivers/base/devcon.c| 2 +- drivers/base/property.c | 2 +- drivers/clk/bcm/clk-bcm2835.c| 4 +--- drivers/clk/clk.c| 4 ++-- drivers/clk/rockchip/clk.c | 4 ++-- drivers/cpufreq/intel_pstate.c | 9 - drivers/gpio/gpiolib-of.c| 2 +- drivers/gpu/drm/drm_edid_load.c | 2 +- drivers/gpu/drm/drm_panel_orientation_quirks.c | 2 +- drivers/gpu/drm/i915/intel_pipe_crc.c| 2 +- drivers/ide/hpt366.c | 2 +- drivers/mfd/omap-usb-host.c | 2 +- drivers/mmc/host/sdhci-xenon-phy.c | 12 ++-- drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 2 +- drivers/pci/pcie/aer.c | 2 +- drivers/phy/tegra/xusb.c | 2 +- drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 4 ++-- drivers/pinctrl/pinmux.c | 2 +- drivers/power/supply/ab8500_btemp.c | 2 +- drivers/power/supply/ab8500_charger.c| 2 +- drivers/power/supply/ab8500_fg.c | 2 +- drivers/power/supply/abx500_chargalg.c | 2 +- drivers/power/supply/charger-manager.c | 4 ++-- drivers/staging/gdm724x/gdm_tty.c| 3 +-- drivers/usb/common/common.c | 4 ++-- drivers/usb/typec/class.c| 8 +++- drivers/usb/typec/tps6598x.c | 2 +- drivers/vfio/vfio.c | 4 +--- drivers/video/fbdev/pxafb.c | 4 ++-- fs/ubifs/auth.c | 4 ++-- include/linux/string.h | 11 ++- kernel/cgroup/rdma.c | 2 +- kernel/sched/debug.c | 2 +- kernel/trace/trace.c | 2 +- lib/string.c | 13 - mm/mempolicy.c | 2 +- mm/vmpressure.c | 4 ++-- security/apparmor/lsm.c | 4 ++-- security/integrity/ima/ima_main.c| 2 +- sound/firewire/oxfw/oxfw.c | 2 +- sound/pci/oxygen/oxygen_mixer.c | 2 +- sound/soc/codecs/max98088.c | 2 +- sound/soc/codecs/max98095.c
[PATCH 03/16] lib,treewide: add new match_string() helper/macro
This change re-introduces `match_string()` as a macro that uses
ARRAY_SIZE() to compute the size of the array.
The macro is added in all the places that do
`match_string(_a, ARRAY_SIZE(_a), s)`, since the change is pretty
straightforward.
Signed-off-by: Alexandru Ardelean
---
drivers/clk/bcm/clk-bcm2835.c| 4 +---
drivers/gpio/gpiolib-of.c| 2 +-
drivers/gpu/drm/i915/intel_pipe_crc.c| 2 +-
drivers/mfd/omap-usb-host.c | 2 +-
drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 2 +-
drivers/pci/pcie/aer.c | 2 +-
drivers/usb/common/common.c | 4 ++--
drivers/usb/typec/class.c| 8 +++-
drivers/usb/typec/tps6598x.c | 2 +-
drivers/vfio/vfio.c | 4 +---
include/linux/string.h | 9 +
sound/firewire/oxfw/oxfw.c | 2 +-
sound/soc/codecs/max98088.c | 2 +-
sound/soc/codecs/max98095.c | 2 +-
14 files changed, 25 insertions(+), 22 deletions(-)
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index a775f6a1f717..1ab388590ead 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -1390,9 +1390,7 @@ static struct clk_hw *bcm2835_register_clock(struct
bcm2835_cprman *cprman,
for (i = 0; i < data->num_mux_parents; i++) {
parents[i] = data->parents[i];
- ret = __match_string(cprman_parent_names,
-ARRAY_SIZE(cprman_parent_names),
-parents[i]);
+ ret = match_string(cprman_parent_names, parents[i]);
if (ret >= 0)
parents[i] = cprman->real_parent_names[ret];
}
diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index 27d6f04ab58e..71e886869d78 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -279,7 +279,7 @@ static struct gpio_desc *of_find_regulator_gpio(struct
device *dev, const char *
if (!con_id)
return ERR_PTR(-ENOENT);
- i = __match_string(whitelist, ARRAY_SIZE(whitelist), con_id);
+ i = match_string(whitelist, con_id);
if (i < 0)
return ERR_PTR(-ENOENT);
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c
b/drivers/gpu/drm/i915/intel_pipe_crc.c
index 286fad1f0e08..6fc4f3d3d1f6 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -449,7 +449,7 @@ display_crc_ctl_parse_source(const char *buf, enum
intel_pipe_crc_source *s)
return 0;
}
- i = __match_string(pipe_crc_sources, ARRAY_SIZE(pipe_crc_sources), buf);
+ i = match_string(pipe_crc_sources, buf);
if (i < 0)
return i;
diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 9aaacb5bdb26..53dff34c0afc 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -509,7 +509,7 @@ static int usbhs_omap_get_dt_pdata(struct device *dev,
continue;
/* get 'enum usbhs_omap_port_mode' from port mode string */
- ret = __match_string(port_modes, ARRAY_SIZE(port_modes), mode);
+ ret = match_string(port_modes, mode);
if (ret < 0) {
dev_warn(dev, "Invalid port%d-mode \"%s\" in device
tree\n",
i, mode);
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
index 59ce3ff35553..778b4dfd8b75 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c
@@ -667,7 +667,7 @@ iwl_dbgfs_bt_force_ant_write(struct iwl_mvm *mvm, char *buf,
};
int ret, bt_force_ant_mode;
- ret = __match_string(modes_str, ARRAY_SIZE(modes_str), buf);
+ ret = match_string(modes_str, buf);
if (ret < 0)
return ret;
diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
index 41a0773a1cbc..2278caba109c 100644
--- a/drivers/pci/pcie/aer.c
+++ b/drivers/pci/pcie/aer.c
@@ -203,7 +203,7 @@ void pcie_ecrc_get_policy(char *str)
{
int i;
- i = __match_string(ecrc_policy_str, ARRAY_SIZE(ecrc_policy_str), str);
+ i = match_string(ecrc_policy_str, str);
if (i < 0)
return;
diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index bca0c404c6ca..5a651d311d38 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c
@@ -68,7 +68,7 @@ enum usb_device_speed usb_get_maximum_speed(struct device
*dev)
if (ret < 0)
return USB_SPEED_UNKNOWN;
- ret = __match_string(spe
[PATCH 06/16] x86/mtrr: use new match_string() helper + add gaps == minor fix
This change is a bit more than cosmetic.
It replaces 2 values in mtrr_strings with NULL. Previously, they were
defined as "?", which is not great because you could technically pass "?",
and you would get value 2.
It's not sure whether that was intended (likely it wasn't), but this fixes
that.
Signed-off-by: Alexandru Ardelean
---
arch/x86/kernel/cpu/mtrr/if.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
index 4ec7a5f7b94c..e67820a044cc 100644
--- a/arch/x86/kernel/cpu/mtrr/if.c
+++ b/arch/x86/kernel/cpu/mtrr/if.c
@@ -20,8 +20,8 @@ static const char *const mtrr_strings[MTRR_NUM_TYPES] =
{
"uncachable", /* 0 */
"write-combining", /* 1 */
- "?",/* 2 */
- "?",/* 3 */
+ NULL, /* 2 */
+ NULL, /* 3 */
"write-through",/* 4 */
"write-protect",/* 5 */
"write-back", /* 6 */
@@ -29,7 +29,9 @@ static const char *const mtrr_strings[MTRR_NUM_TYPES] =
const char *mtrr_attrib_to_str(int x)
{
- return (x <= 6) ? mtrr_strings[x] : "?";
+ if ((x >= ARRAY_SIZE(mtrr_strings)) || (mtrr_strings[x] == NULL))
+ return "?";
+ return mtrr_strings[x];
}
#ifdef CONFIG_PROC_FS
@@ -142,7 +144,7 @@ mtrr_write(struct file *file, const char __user *buf,
size_t len, loff_t * ppos)
return -EINVAL;
ptr = skip_spaces(ptr + 5);
- i = __match_string(mtrr_strings, MTRR_NUM_TYPES, ptr);
+ i = match_string(mtrr_strings, ptr);
if (i < 0)
return i;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 09/16] mmc: sdhci-xenon: use new match_string() helper/macro
The change is also cosmetic, but it also does a tighter coupling between
the enums & the string values. This way, the ARRAY_SIZE(phy_types) that is
implicitly done in the match_string() macro is also a bit safer.
Signed-off-by: Alexandru Ardelean
---
drivers/mmc/host/sdhci-xenon-phy.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/sdhci-xenon-phy.c
b/drivers/mmc/host/sdhci-xenon-phy.c
index 59b7a6cac995..2a9206867fe1 100644
--- a/drivers/mmc/host/sdhci-xenon-phy.c
+++ b/drivers/mmc/host/sdhci-xenon-phy.c
@@ -135,17 +135,17 @@ struct xenon_emmc_phy_regs {
u32 logic_timing_val;
};
-static const char * const phy_types[] = {
- "emmc 5.0 phy",
- "emmc 5.1 phy"
-};
-
enum xenon_phy_type_enum {
EMMC_5_0_PHY,
EMMC_5_1_PHY,
NR_PHY_TYPES
};
+static const char * const phy_types[NR_PHY_TYPES] = {
+ [EMMC_5_0_PHY] = "emmc 5.0 phy",
+ [EMMC_5_1_PHY] = "emmc 5.1 phy"
+};
+
enum soc_pad_ctrl_type {
SOC_PAD_SD,
SOC_PAD_FIXED_1_8V,
@@ -821,7 +821,7 @@ static int xenon_add_phy(struct device_node *np, struct
sdhci_host *host,
struct xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
int ret;
- priv->phy_type = __match_string(phy_types, NR_PHY_TYPES, phy_name);
+ priv->phy_type = match_string(phy_types, phy_name);
if (priv->phy_type < 0) {
dev_err(mmc_dev(host->mmc),
"Unable to determine PHY name %s. Use default eMMC 5.1
PHY\n",
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/16] lib: fix match_string() helper on -1 array size
The documentation the `_match_string()` helper mentions that `n`
should be:
* @n: number of strings in the array or -1 for NULL terminated arrays
The behavior of the function is different, in the sense that it exits on
the first NULL element in the array, regardless of whether `n` is -1 or a
positive number.
This patch changes the behavior, to exit the loop when a NULL element is
found and n == -1. Essentially, this aligns the behavior with the
doc-string.
There are currently many users of `match_string()`, and so, in order to go
through them, the next patches in the series will focus on doing some
cosmetic changes, which are aimed at grouping the users of
`match_string()`.
Signed-off-by: Alexandru Ardelean
---
lib/string.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/string.c b/lib/string.c
index 3ab861c1a857..76edb7bf76cb 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -648,8 +648,11 @@ int match_string(const char * const *array, size_t n,
const char *string)
for (index = 0; index < n; index++) {
item = array[index];
- if (!item)
+ if (!item) {
+ if (n != (size_t)-1)
+ continue;
break;
+ }
if (!strcmp(item, string))
return index;
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 08/16] cpufreq/intel_pstate: remove NULL entry + use match_string()
The change is mostly cosmetic.
The `energy_perf_strings` array is static, so match_string() can be used
(which will implicitly do a ARRAY_SIZE(energy_perf_strings)).
The only small benefit here, is the reduction of the array size by 1
element.
Signed-off-by: Alexandru Ardelean
---
drivers/cpufreq/intel_pstate.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 6ed1e705bc05..ab9a0b34b900 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -593,8 +593,7 @@ static const char * const energy_perf_strings[] = {
"performance",
"balance_performance",
"balance_power",
- "power",
- NULL
+ "power"
};
static const unsigned int epp_values[] = {
HWP_EPP_PERFORMANCE,
@@ -680,8 +679,8 @@ static ssize_t
show_energy_performance_available_preferences(
int i = 0;
int ret = 0;
- while (energy_perf_strings[i] != NULL)
- ret += sprintf(&buf[ret], "%s ", energy_perf_strings[i++]);
+ for (; i < ARRAY_SIZE(energy_perf_strings); i++)
+ ret += sprintf(&buf[ret], "%s ", energy_perf_strings[i]);
ret += sprintf(&buf[ret], "\n");
@@ -701,7 +700,7 @@ static ssize_t store_energy_performance_preference(
if (ret != 1)
return -EINVAL;
- ret = __match_string(energy_perf_strings, -1, str_preference);
+ ret = match_string(energy_perf_strings, str_preference);
if (ret < 0)
return ret;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/16] lib: fix match_string() helper when array size is positive
The documentation the `_match_string()` helper mentions that `n`
(size of the given array) should be:
* @n: number of strings in the array or -1 for NULL terminated arrays
The behavior of the function is different, in the sense that it exits on
the first NULL element in the array, regardless of whether `n` is -1 or a
positive number.
This patch changes the behavior, to exit the loop when a NULL element is
found and n == -1. Essentially, this aligns the behavior with the
doc-string.
There are currently many users of `match_string()`, and so, in order to go
through them, the next patches in the series will focus on doing some
cosmetic changes, which are aimed at grouping the users of
`match_string()`.
Signed-off-by: Alexandru Ardelean
---
lib/string.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/string.c b/lib/string.c
index 3ab861c1a857..76edb7bf76cb 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -648,8 +648,11 @@ int match_string(const char * const *array, size_t n,
const char *string)
for (index = 0; index < n; index++) {
item = array[index];
- if (!item)
+ if (!item) {
+ if (n != (size_t)-1)
+ continue;
break;
+ }
if (!strcmp(item, string))
return index;
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 11/16] mm/vmpressure.c: use new match_string() helper/macro
__match_string() is called on 2 static array of strings in this file. For
this reason, the conversion to the new match_string() macro/helper, was
done in this separate commit.
Using the new match_string() helper is mostly a cosmetic change (at this
point in time). The sizes of the arrays will be computed automatically,
which would only help if they ever get expanded.
Signed-off-by: Alexandru Ardelean
---
mm/vmpressure.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/vmpressure.c b/mm/vmpressure.c
index d43f33139568..b8149924f078 100644
--- a/mm/vmpressure.c
+++ b/mm/vmpressure.c
@@ -378,7 +378,7 @@ int vmpressure_register_event(struct mem_cgroup *memcg,
/* Find required level */
token = strsep(&spec, ",");
- level = __match_string(vmpressure_str_levels, VMPRESSURE_NUM_LEVELS,
token);
+ level = match_string(vmpressure_str_levels, token);
if (level < 0) {
ret = level;
goto out;
@@ -387,7 +387,7 @@ int vmpressure_register_event(struct mem_cgroup *memcg,
/* Find optional mode */
token = strsep(&spec, ",");
if (token) {
- mode = __match_string(vmpressure_str_modes,
VMPRESSURE_NUM_MODES, token);
+ mode = match_string(vmpressure_str_modes, token);
if (mode < 0) {
ret = mode;
goto out;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 10/16] pinctrl: armada-37xx: use new match_string() helper/macro
The change is mostly cosmetic.
The `armada_37xx_pin_group` struct is defined as.
struct armada_37xx_pin_group {
const char *name;
unsigned intstart_pin;
unsigned intnpins;
u32 reg_mask;
u32 val[NB_FUNCS];
unsigned intextra_pin;
unsigned intextra_npins;
const char *funcs[NB_FUNCS];
unsigned int*pins;
};
The `funcs` field is a static array of strings, so using the
new `match_string()` helper (which does an implicit ARRAY_SIZE(gp->funcs))
should be fine.
Signed-off-by: Alexandru Ardelean
---
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 07a5bcaa0067..68b0db5ef5e9 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -348,7 +348,7 @@ static int armada_37xx_pmx_set_by_name(struct pinctrl_dev
*pctldev,
dev_dbg(info->dev, "enable function %s group %s\n",
name, grp->name);
- func = __match_string(grp->funcs, NB_FUNCS, name);
+ func = match_string(grp->funcs, name);
if (func < 0)
return -ENOTSUPP;
@@ -938,7 +938,7 @@ static int armada_37xx_fill_func(struct armada_37xx_pinctrl
*info)
struct armada_37xx_pin_group *gp = &info->groups[g];
int f;
- f = __match_string(gp->funcs, NB_FUNCS, name);
+ f = match_string(gp->funcs, name);
if (f < 0)
continue;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 12/16] rdmacg: use new match_string() helper/macro
The `rdmacg_resource_names` array is a static array of strings. Using match_string() (which computes the array size via ARRAY_SIZE()) is possible. The change is mostly cosmetic. No functionality change. Signed-off-by: Alexandru Ardelean --- kernel/cgroup/rdma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/cgroup/rdma.c b/kernel/cgroup/rdma.c index 65d4df148603..71c3d305bd1f 100644 --- a/kernel/cgroup/rdma.c +++ b/kernel/cgroup/rdma.c @@ -367,7 +367,7 @@ static int parse_resource(char *c, int *intval) if (!name || !value) return -EINVAL; - i = __match_string(rdmacg_resource_names, RDMACG_RESOURCE_MAX, name); + i = match_string(rdmacg_resource_names, name); if (i < 0) return i; -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 15/16] video: fbdev: pxafb: use new match_string() helper/macro
The `lcd_types` array is a static array of strings.
Using match_string() (which computes the array size via ARRAY_SIZE())
is possible.
This reduces the array by 1 element, since the NULL (at the end of the
array) is no longer needed.
Signed-off-by: Alexandru Ardelean
---
drivers/video/fbdev/pxafb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/pxafb.c b/drivers/video/fbdev/pxafb.c
index 0025781e6e1e..e657a04f5b1d 100644
--- a/drivers/video/fbdev/pxafb.c
+++ b/drivers/video/fbdev/pxafb.c
@@ -2114,7 +2114,7 @@ static void pxafb_check_options(struct device *dev,
struct pxafb_mach_info *inf)
#if defined(CONFIG_OF)
static const char * const lcd_types[] = {
"unknown", "mono-stn", "mono-dstn", "color-stn", "color-dstn",
- "color-tft", "smart-panel", NULL
+ "color-tft", "smart-panel"
};
static int of_get_pxafb_display(struct device *dev, struct device_node *disp,
@@ -2129,7 +2129,7 @@ static int of_get_pxafb_display(struct device *dev,
struct device_node *disp,
if (ret)
s = "color-tft";
- i = __match_string(lcd_types, -1, s);
+ i = match_string(lcd_types, s);
if (i < 0) {
dev_err(dev, "lcd-type %s is unknown\n", s);
return i;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 14/16] staging: gdm724x: use new match_string() helper/macro
The `DRIVER_STRING` array is a static array of strings. Using match_string() (which computes the array size via ARRAY_SIZE()) is possible. The change is mostly cosmetic. No functionality change. Signed-off-by: Alexandru Ardelean --- drivers/staging/gdm724x/gdm_tty.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/staging/gdm724x/gdm_tty.c b/drivers/staging/gdm724x/gdm_tty.c index 6e147a324652..81dd6795599f 100644 --- a/drivers/staging/gdm724x/gdm_tty.c +++ b/drivers/staging/gdm724x/gdm_tty.c @@ -56,8 +56,7 @@ static int gdm_tty_install(struct tty_driver *driver, struct tty_struct *tty) struct gdm *gdm = NULL; int ret; - ret = __match_string(DRIVER_STRING, TTY_MAX_COUNT, -tty->driver->driver_name); + ret = match_string(DRIVER_STRING, tty->driver->driver_name); if (ret < 0) return -ENODEV; -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 16/16] sched: debug: use new match_string() helper/macro
The `sched_feat_names` array is a static array of strings. Using match_string() (which computes the array size via ARRAY_SIZE()) is possible. The change is mostly cosmetic. No functionality change. Signed-off-by: Alexandru Ardelean --- kernel/sched/debug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c index b0efc5fe641e..6d5f370bdfde 100644 --- a/kernel/sched/debug.c +++ b/kernel/sched/debug.c @@ -111,7 +111,7 @@ static int sched_feat_set(char *cmp) cmp += 3; } - i = __match_string(sched_feat_names, __SCHED_FEAT_NR, cmp); + i = match_string(sched_feat_names, cmp); if (i < 0) return i; -- 2.17.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 13/16] drm/edid: use new match_string() helper/macro
The `generic_edid_name` is a static array of strings.
Using match_string() (which computes the array size via ARRAY_SIZE())
is possible.
The change is mostly cosmetic.
No functionality change.
Signed-off-by: Alexandru Ardelean
---
drivers/gpu/drm/drm_edid_load.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_edid_load.c b/drivers/gpu/drm/drm_edid_load.c
index 1450051972ea..66e1e325ff37 100644
--- a/drivers/gpu/drm/drm_edid_load.c
+++ b/drivers/gpu/drm/drm_edid_load.c
@@ -186,7 +186,7 @@ static void *edid_load(struct drm_connector *connector,
const char *name,
int i, valid_extensions = 0;
bool print_bad_edid = !connector->bad_edid_counter || (drm_debug &
DRM_UT_KMS);
- builtin = __match_string(generic_edid_name, GENERIC_EDIDS, name);
+ builtin = match_string(generic_edid_name, name);
if (builtin >= 0) {
fwdata = generic_edid[builtin];
fwsize = sizeof(generic_edid[builtin]);
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: ad9832: Add device tree support
On Sun, May 19, 2019 at 8:17 PM Jonathan Cameron wrote:
>
> On Sat, 18 May 2019 17:48:25 -0300
> João Seckler wrote:
>
> > Add a of_device_id struct variable and subsequent call to
> > MODULE_DEVICE_TABLE macro to support device tree.
> >
> > Signed-off-by: João Seckler
> > Signed-off-by: Anderson Reis
> > Co-developed-by: Anderson Reis
> > Signed-off-by: Andre Tadeu de Carvalho
> > Co-developed-by: Andre Tadeu de Carvalho
> Hi All,
>
> Missing the setting of the relevant entry in the spi_driver structure.
> Otherwise looks fine,
>
> Thanks,
>
> Jonathan
> > ---
> > drivers/staging/iio/frequency/ad9832.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/staging/iio/frequency/ad9832.c
> > b/drivers/staging/iio/frequency/ad9832.c
> > index 74308a2e72db..51e97c74c6b2 100644
> > --- a/drivers/staging/iio/frequency/ad9832.c
> > +++ b/drivers/staging/iio/frequency/ad9832.c
> > @@ -451,6 +451,13 @@ static int ad9832_remove(struct spi_device *spi)
> > return 0;
> > }
> >
> > +static const struct of_device_id ad9832_of_match[] = {
> > + { .compatible = "adi,ad9832", },
> > + { .compatible = "adi,ad9835", },
> > + { /* sentinel */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, ad9832_of_match);
> > +
Yep.
To clarify what Jonathan said (see line below with plus + ) :
static struct spi_driver ad9832_driver = {
.driver = {
.name = "ad9832",
+ .of_match_table = ad9832_of_match,
},
.probe = ad9832_probe,
.remove = ad9832_remove,
.id_table = ad9832_id,
};
> > static const struct spi_device_id ad9832_id[] = {
> > {"ad9832", 0},
> > {"ad9835", 0},
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RESEND PATCH] staging: iio: adt7316: create of_device_id array
On Sun, May 19, 2019 at 8:54 PM Jonathan Cameron wrote:
>
> On Sat, 18 May 2019 19:43:33 -0300
> Bárbara Fernandes wrote:
>
> > Create structure of type of_device_id in order to register all devices
> > the driver is able to manage.
> >
> > Signed-off-by: Bárbara Fernandes
> > Signed-off-by: Wilson Sales
> > Co-developed-by: Wilson Sales
> Looks good to me.
>
> Applied to the togreg branch of iio.git and pushed out as testing
> for the autobuilders to play with it.
>
> Thanks,
Also,
Acked-by: Alexandru Ardelean
CC-ing my work-email
There are some issues with it and mailing lists; I'll hopefully sort
them out in the next weeks.
>
> Jonathan
>
> > ---
> > drivers/staging/iio/addac/adt7316-spi.c | 13 +
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/staging/iio/addac/adt7316-spi.c
> > b/drivers/staging/iio/addac/adt7316-spi.c
> > index 8294b9c1e3c2..9968775f1d23 100644
> > --- a/drivers/staging/iio/addac/adt7316-spi.c
> > +++ b/drivers/staging/iio/addac/adt7316-spi.c
> > @@ -127,9 +127,22 @@ static const struct spi_device_id adt7316_spi_id[] = {
> >
> > MODULE_DEVICE_TABLE(spi, adt7316_spi_id);
> >
> > +static const struct of_device_id adt7316_of_spi_match[] = {
> > + { .compatible = "adi,adt7316" },
> > + { .compatible = "adi,adt7317" },
> > + { .compatible = "adi,adt7318" },
> > + { .compatible = "adi,adt7516" },
> > + { .compatible = "adi,adt7517" },
> > + { .compatible = "adi,adt7519" },
> > + { }
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, adt7316_of_spi_match);
> > +
> > static struct spi_driver adt7316_driver = {
> > .driver = {
> > .name = "adt7316",
> > + .of_match_table = adt7316_of_spi_match,
> > .pm = ADT7316_PM_OPS,
> > },
> > .probe = adt7316_spi_probe,
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:iio:ad7150: fix threshold mode config bit
On Sun, May 19, 2019 at 8:38 PM Jonathan Cameron wrote:
>
> On Sat, 18 May 2019 22:04:56 -0300
> Melissa Wen wrote:
>
> > According to the AD7150 configuration register description, bit 7 assumes
> > value 1 when the threshold mode is fixed and 0 when it is adaptive,
> > however, the operation that identifies this mode was considering the
> > opposite values.
> >
> > This patch renames the boolean variable to describe it correctly and
> > properly replaces it in the places where it is used.
> >
> > Fixes: 531efd6aa0991 ("staging:iio:adc:ad7150: chan_spec conv + i2c_smbus
> > commands + drop unused poweroff timeout control.")
> > Signed-off-by: Melissa Wen
>
> Looks good to me. Applied to the fixes-togreg branch of iio.git pushed out as
> as testing-fixes for the autobuilders to see if they can find anything
> we have missed.
>
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/staging/iio/cdc/ad7150.c | 19 +++
> > 1 file changed, 11 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/iio/cdc/ad7150.c
> > b/drivers/staging/iio/cdc/ad7150.c
> > index dd7fcab8e19e..e075244c602b 100644
> > --- a/drivers/staging/iio/cdc/ad7150.c
> > +++ b/drivers/staging/iio/cdc/ad7150.c
> > @@ -5,6 +5,7 @@
> > * Copyright 2010-2011 Analog Devices Inc.
> > */
> >
> > +#include
> > #include
> > #include
> > #include
> > @@ -130,7 +131,7 @@ static int ad7150_read_event_config(struct iio_dev
> > *indio_dev,
> > {
> > int ret;
> > u8 threshtype;
> > - bool adaptive;
> > + bool thrfixed;
> > struct ad7150_chip_info *chip = iio_priv(indio_dev);
> >
> > ret = i2c_smbus_read_byte_data(chip->client, AD7150_CFG);
> > @@ -138,21 +139,23 @@ static int ad7150_read_event_config(struct iio_dev
> > *indio_dev,
> > return ret;
> >
> > threshtype = (ret >> 5) & 0x03;
> > - adaptive = !!(ret & 0x80);
> > +
> > + /*check if threshold mode is fixed or adaptive*/
> > + thrfixed = FIELD_GET(AD7150_CFG_FIX, ret);
nitpick: i would have kept the original variable name as "adaptive",
mostly for consistency.
"adaptive" is used in other places as well;
as i recall, the fix is just oneliner in this case:
- adaptive = !!(ret & 0x80);
+ adaptive = !(ret & 0x80);
> >
> > switch (type) {
> > case IIO_EV_TYPE_MAG_ADAPTIVE:
> > if (dir == IIO_EV_DIR_RISING)
> > - return adaptive && (threshtype == 0x1);
> > - return adaptive && (threshtype == 0x0);
> > + return !thrfixed && (threshtype == 0x1);
> > + return !thrfixed && (threshtype == 0x0);
> > case IIO_EV_TYPE_THRESH_ADAPTIVE:
> > if (dir == IIO_EV_DIR_RISING)
> > - return adaptive && (threshtype == 0x3);
> > - return adaptive && (threshtype == 0x2);
> > + return !thrfixed && (threshtype == 0x3);
> > + return !thrfixed && (threshtype == 0x2);
> > case IIO_EV_TYPE_THRESH:
> > if (dir == IIO_EV_DIR_RISING)
> > - return !adaptive && (threshtype == 0x1);
> > - return !adaptive && (threshtype == 0x0);
> > + return thrfixed && (threshtype == 0x1);
> > + return thrfixed && (threshtype == 0x0);
> > default:
> > break;
> > }
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: iio: ad2s1210: Add devicetree yaml doc
On Sun, May 19, 2019 at 8:18 PM Jonathan Cameron wrote:
>
> On Sat, 18 May 2019 19:15:58 -0300
> Tallys Martins wrote:
>
CC-ing my work-email
There are some issues with it and mailing lists; I'll hopefully sort
them out in the next weeks.
> > The driver currently has no devicetree documentation. This commit adds a
> > devicetree folder and documentation for it. Documentation must be moved
> > as well when the driver gets out of staging.
> >
> > Signed-off-by: Tallys Martins
> > Signed-off-by: Souza Guilherme
> > Co-developed-by: Souza Guilherme
>
> Hi,
>
> There is no need for a direct relationship between a binding and a driver
> at all. As such, we normally don't take binding documents in staging.
>
> Just put it directly in it's final destination. The driver can catch
> up with it later!
>
> I'm still not that comfortable with yaml (haven't gotten around
> to doing any conversions myself yet) so I'll be looking for additional
> review on these from others.
Personal note: I also don't like YAML, but that may be me feeling old.
I'm not that old to prefer XML, but old enough to prefer JSON.
I am still trying to accommodate my taste/feeling for the format, even
though I've been using it sporadically for ~5 years now in various
other elements.
Travis-CI may be the biggest user of it.
Looks like Ruby users were the biggest pushers/initiators of YAML, and
Python people seem to have jumped in shortly after.
Oh well: ¯\_(ツ)_/¯
>
> A few comments inline.
>
> > ---
> > .../Documentation/devicetree/ad2s1210.yaml| 41 +++
> > 1 file changed, 41 insertions(+)
> > create mode 100644
> > drivers/staging/iio/Documentation/devicetree/ad2s1210.yaml
Maybe also remove the old txt binding if adding this new one.
No need to keep them both.
> >
> > diff --git a/drivers/staging/iio/Documentation/devicetree/ad2s1210.yaml
> > b/drivers/staging/iio/Documentation/devicetree/ad2s1210.yaml
> > new file mode 100644
> > index ..733aa07b4626
> > --- /dev/null
> > +++ b/drivers/staging/iio/Documentation/devicetree/ad2s1210.yaml
> > @@ -0,0 +1,41 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/iio/ad2s1210.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: |
> > + Analog Devices Inc. AD2S1210 10-Bit to 16-Bit R/D Converters
> > +
> > +maintainers:
> > + - Graff Yang
> I would check that one with the Analog devices team.
I guess, add (here):
Michael Hennerich
>
> > +
> > +description: |
> > + Analog Devices AD2S1210 Resolver to Digital SPI driver
> > +
> > + https://www.analog.com/en/products/ad2s1210.html
> > +
> > +properties:
> > + compatible:
> > +enum:
> > + - adi,ad2s1210
These 2 are required here, since this chip operates in SPI mode 3.
spi-cpha: true
spi-cpol: true
Also, from the driver, some GPIOs should also be documented:
static const struct ad2s1210_gpio gpios[] = {
[AD2S1210_SAMPLE] = { .name = "adi,sample", .flags = GPIOD_OUT_LOW },
[AD2S1210_A0] = { .name = "adi,a0", .flags = GPIOD_OUT_LOW },
[AD2S1210_A1] = { .name = "adi,a1", .flags = GPIOD_OUT_LOW },
[AD2S1210_RES0] = { .name = "adi,res0", .flags = GPIOD_OUT_LOW },
[AD2S1210_RES1] = { .name = "adi,res1", .flags = GPIOD_OUT_LOW },
};
So,
adi,sample-gpios:
adi,a0-gpios:
adi,a1-gpios:
adi,res0-gpios:
adi,res10-gpios:
These also look like they are required.
> > +
> > + reg:
> > +maxItems: 1
> > +
> > + clock-frequency:
> > +minimum: 2000
> > +maximum: 2
> > +default: 8192
> This doesn't look like a modern clock binding.
> If we are going to end up changing this, then we should probably delay
> having a binding doc until after that change is made.
>
> We often only do binding docs for drivers in staging just before moving
> them out so as to avoid this sort of issue.
These clock properties are not needed here.
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > +
> > +examples:
> > + - |
> > + resolver@0 {
> > +compatible = "adi,ad2s1210";
> > +reg = <0>;
> An example is probably more useful if it includes all the optional properties
> as well.
This should also be included in an spi block:
So:
spi0 {
resolver@0 {
compatible = "adi,ad2s1210";
reg = <0>;
spi-cpha;
spi-cpol;
// and then the adi,***-gpios I think here
};
};
I don't think much more-else is needed for this driver.
> > + };
> > +...
>
> Thanks,
>
> Jonathan
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RESEND PATCH] staging: iio: ad7192: create of_device_id array
On Sun, May 19, 2019 at 8:53 PM Jonathan Cameron wrote:
>
> On Sat, 18 May 2019 19:44:35 -0300
> Bárbara Fernandes wrote:
>
I don't have anything else on top of what Jonathan added.
Acked-by: Alexandru Ardelean
CC-ing my work-email
There are some issues with it and mailing lists; I'll hopefully sort
them out in the next weeks.
> > Create list of compatible device ids to be matched with those stated in
> > the device tree.
> >
> > Signed-off-by: Bárbara Fernandes
> > Signed-off-by: Wilson Sales
> > Co-developed by: Wilson Sales
> Hi Bárbara, Wilson,
>
> One minor issue inline about code ordering.
> Actual content is fine.
>
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/staging/iio/adc/ad7192.c | 12
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/staging/iio/adc/ad7192.c
> > b/drivers/staging/iio/adc/ad7192.c
> > index 3d74da9d37e7..cc886f944dbf 100644
> > --- a/drivers/staging/iio/adc/ad7192.c
> > +++ b/drivers/staging/iio/adc/ad7192.c
> > @@ -810,11 +810,23 @@ static const struct spi_device_id ad7192_id[] = {
> > {"ad7195", ID_AD7195},
> > {}
> > };
> > +
> > +static const struct of_device_id ad7192_of_spi_match[] = {
> > + { .compatible = "adi,ad7190" },
> > + { .compatible = "adi,ad7192" },
> > + { .compatible = "adi,ad7193" },
> > + { .compatible = "adi,ad7195" },
> > + {}
> > +};
> > +
> > +MODULE_DEVICE_TABLE(of, ad7192_of_spi_match);
> > +
> Please keep the declaration of the table alongside the relevant
> MODULE_DEVICE_TABLE.
>
> In short, better to have your additions after this next line.
> > MODULE_DEVICE_TABLE(spi, ad7192_id);
> >
> > static struct spi_driver ad7192_driver = {
> > .driver = {
> > .name = "ad7192",
> > + .of_match_table = ad7192_of_spi_match,
> > },
> > .probe = ad7192_probe,
> > .remove = ad7192_remove,
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: iio: adis16240: add of_match_table entry
On Fri, May 24, 2019 at 6:30 AM Rodrigo Ribeiro wrote:
>
> This patch adds of_match_table entry in device driver in order to
> enable spi fallback probing.
>
> Signed-off-by: Rodrigo Ribeiro
> Reviewed-by: Marcelo Schmitt
> ---
> drivers/staging/iio/accel/adis16240.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/iio/accel/adis16240.c
> b/drivers/staging/iio/accel/adis16240.c
> index 8c6d23604eca..b80c8529784b 100644
> --- a/drivers/staging/iio/accel/adis16240.c
> +++ b/drivers/staging/iio/accel/adis16240.c
> @@ -444,6 +444,7 @@ MODULE_DEVICE_TABLE(of, adis16240_of_match);
> static struct spi_driver adis16240_driver = {
> .driver = {
> .name = "adis16240",
> + .of_match_table = adis16240_of_match,
This patch is missing the actual table.
> },
> .probe = adis16240_probe,
> .remove = adis16240_remove,
> --
> 2.20.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3][V2] treewide: rename match_string() -> __match_string()
This change does a rename of match_string() -> __match_string().
There are a few parts to the intention here (with this change):
1. Align with sysfs_match_string()/__sysfs_match_string()
2. This helps to group users of `match_string()`:
a. those that use ARRAY_SIZE(_a) to specify the number of elements
b. those that use -1 to pass a NULL terminated array of strings
c. special users, which (after eliminating 1 & 2) are not that many
This change is done treewide. Updates to the new match_string() helper will
be done on a per-subsystem basis, as the cadence of each subsystem differs.
Signed-off-by: Alexandru Ardelean
---
arch/powerpc/xmon/xmon.c | 2 +-
arch/x86/kernel/cpu/mtrr/if.c| 2 +-
drivers/ata/pata_hpt366.c| 2 +-
drivers/ata/pata_hpt37x.c| 2 +-
drivers/base/devcon.c| 2 +-
drivers/base/property.c | 2 +-
drivers/clk/bcm/clk-bcm2835.c| 6 +++---
drivers/clk/rockchip/clk.c | 4 ++--
drivers/cpufreq/intel_pstate.c | 2 +-
drivers/gpio/gpiolib-of.c| 2 +-
drivers/gpu/drm/drm_edid_load.c | 2 +-
drivers/gpu/drm/drm_panel_orientation_quirks.c | 2 +-
drivers/gpu/drm/i915/intel_pipe_crc.c| 2 +-
drivers/ide/hpt366.c | 2 +-
drivers/mfd/omap-usb-host.c | 2 +-
drivers/mmc/host/sdhci-xenon-phy.c | 2 +-
drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 2 +-
drivers/pci/pcie/aer.c | 2 +-
drivers/phy/tegra/xusb.c | 4 ++--
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 4 ++--
drivers/pinctrl/pinmux.c | 2 +-
drivers/power/supply/ab8500_btemp.c | 2 +-
drivers/power/supply/ab8500_charger.c| 2 +-
drivers/power/supply/ab8500_fg.c | 2 +-
drivers/power/supply/abx500_chargalg.c | 2 +-
drivers/power/supply/charger-manager.c | 4 ++--
drivers/staging/gdm724x/gdm_tty.c| 4 ++--
drivers/usb/common/common.c | 4 ++--
drivers/usb/typec/class.c| 10 +-
drivers/usb/typec/tps6598x.c | 2 +-
drivers/vfio/vfio.c | 6 +++---
drivers/video/fbdev/pxafb.c | 2 +-
fs/ubifs/auth.c | 4 ++--
include/linux/string.h | 2 +-
kernel/cgroup/rdma.c | 2 +-
kernel/sched/debug.c | 2 +-
kernel/trace/trace.c | 2 +-
lib/string.c | 8
mm/mempolicy.c | 2 +-
mm/vmpressure.c | 4 ++--
security/apparmor/lsm.c | 4 ++--
security/integrity/ima/ima_main.c| 2 +-
sound/firewire/oxfw/oxfw.c | 2 +-
sound/pci/oxygen/oxygen_mixer.c | 2 +-
sound/soc/codecs/max98088.c | 2 +-
sound/soc/codecs/max98095.c | 2 +-
sound/soc/soc-dapm.c | 2 +-
47 files changed, 67 insertions(+), 67 deletions(-)
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 1b0149b2bb6c..8039759a9e82 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3264,7 +3264,7 @@ scanhex(unsigned long *vp)
regname[i] = c;
}
regname[i] = 0;
- i = match_string(regnames, N_PTREGS, regname);
+ i = __match_string(regnames, N_PTREGS, regname);
if (i < 0) {
printf("invalid register name '%%%s'\n", regname);
return 0;
diff --git a/arch/x86/kernel/cpu/mtrr/if.c b/arch/x86/kernel/cpu/mtrr/if.c
index 4d36dcc1cf87..4ec7a5f7b94c 100644
--- a/arch/x86/kernel/cpu/mtrr/if.c
+++ b/arch/x86/kernel/cpu/mtrr/if.c
@@ -142,7 +142,7 @@ mtrr_write(struct file *file, const char __user *buf,
size_t len, loff_t * ppos)
return -EINVAL;
ptr = skip_spaces(ptr + 5);
- i = match_string(mtrr_strings, MTRR_NUM_TYPES, ptr);
+ i = __match_string(mtrr_strings, MTRR_NUM_TYPES, ptr);
if (i < 0)
return i;
diff --git a/drivers/ata/pata_hpt366.c b/drivers/ata/pata_hpt366.c
index 2574d6fbb1ad..a23ec26cc95f 100644
--- a/drivers/ata/pata_hpt366.c
+++ b/drivers/ata/pata_hpt366.c
@@ -181,7 +181,7 @@ static int hpt_dma_blacklisted(const struct ata_device
*dev, char *modestr,
ata_id_c_string(dev->id, model_num, ATA_ID_PROD, sizeof(model_num));
- i = match_string(list, -1, model_num);
[PATCH 3/3][V2] lib: re-introduce new match_string() helper/macro
This change re-introduces `match_string()` as a macro that uses ARRAY_SIZE() to compute the size of the array. After this change, work can start on migrating subsystems to use this new helper. Since the original helper is pretty used, migrating to this new one will take a while, and will be reviewed by each subsystem. Signed-off-by: Alexandru Ardelean --- include/linux/string.h | 9 + 1 file changed, 9 insertions(+) diff --git a/include/linux/string.h b/include/linux/string.h index 7149fcdf62df..34491b075449 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -198,6 +198,15 @@ static inline int strtobool(const char *s, bool *res) int __match_string(const char * const *array, size_t n, const char *string); int __sysfs_match_string(const char * const *array, size_t n, const char *s); +/** + * match_string - matches given string in an array + * @_a: array of strings + * @_s: string to match with + * + * Helper for __match_string(). Calculates the size of @a automatically. + */ +#define match_string(_a, _s) __match_string(_a, ARRAY_SIZE(_a), _s) + /** * sysfs_match_string - matches given string in an array * @_a: array of strings -- 2.20.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3][V2] lib: fix match_string() helper on -1 array size
The documentation the `_match_string()` helper mentions that `n`
should be:
* @n: number of strings in the array or -1 for NULL terminated arrays
The behavior of the function is different, in the sense that it exits on
the first NULL element in the array, regardless of whether `n` is -1 or a
positive number.
This patch changes the behavior, to exit the loop when a NULL element is
found and n == -1. Essentially, this aligns the behavior with the
doc-string.
There are currently many users of `match_string()`, and so, in order to go
through them, the next patches in the series will focus on doing some
cosmetic changes, which are aimed at grouping the users of
`match_string()`.
Signed-off-by: Alexandru Ardelean
---
Changelog v1 -> v2:
* split the initial series into just 3 patches that fix the
`match_string()` helper and start introducing a new version of this
helper, which computes array-size of static arrays
lib/string.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/string.c b/lib/string.c
index 6016eb3ac73d..e2cf5acc83bd 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -681,8 +681,11 @@ int match_string(const char * const *array, size_t n,
const char *string)
for (index = 0; index < n; index++) {
item = array[index];
- if (!item)
+ if (!item) {
+ if (n != (size_t)-1)
+ continue;
break;
+ }
if (!strcmp(item, string))
return index;
}
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] iio: adis16480: Make use of __adis_initial_startup
From: Nuno Sá
All actions done in `adis16480_initial_setup()` are now done in
`__adis_initial_startup()` so, there's no need for code duplication.
Furthermore, the call to `adis16480_initial_setup()` is done before any
device configuration since the device will be reset if not already (via
rst pin). This is actually fixing a potential bug since `adis_reset()` was
being called after configuring the device which is obviously a problem.
Signed-off-by: Nuno Sá
Signed-off-by: Alexandru Ardelean
---
drivers/iio/imu/adis16480.c | 55 -
1 file changed, 11 insertions(+), 44 deletions(-)
diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
index e1de25f18e2e..36973662a31d 100644
--- a/drivers/iio/imu/adis16480.c
+++ b/drivers/iio/imu/adis16480.c
@@ -1014,40 +1014,6 @@ static int adis16480_enable_irq(struct adis *adis, bool
enable)
return __adis_write_reg_16(adis, ADIS16480_REG_FNCTIO_CTRL, val);
}
-static int adis16480_initial_setup(struct iio_dev *indio_dev)
-{
- struct adis16480 *st = iio_priv(indio_dev);
- uint16_t prod_id;
- unsigned int device_id;
- int ret;
-
- adis_reset(&st->adis);
- msleep(70);
-
- ret = adis_write_reg_16(&st->adis, ADIS16480_REG_GLOB_CMD, BIT(1));
- if (ret)
- return ret;
- msleep(30);
-
- ret = adis_check_status(&st->adis);
- if (ret)
- return ret;
-
- ret = adis_read_reg_16(&st->adis, ADIS16480_REG_PROD_ID, &prod_id);
- if (ret)
- return ret;
-
- ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
- if (ret != 1)
- return -EINVAL;
-
- if (prod_id != device_id)
- dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do
not match.",
- device_id, prod_id);
-
- return 0;
-}
-
#define ADIS16480_DIAG_STAT_XGYRO_FAIL 0
#define ADIS16480_DIAG_STAT_YGYRO_FAIL 1
#define ADIS16480_DIAG_STAT_ZGYRO_FAIL 2
@@ -1075,6 +1041,7 @@ static const char * const adis16480_status_error_msgs[] =
{
static const struct adis_data adis16480_data = {
.diag_stat_reg = ADIS16480_REG_DIAG_STS,
.glob_cmd_reg = ADIS16480_REG_GLOB_CMD,
+ .prod_id_reg = ADIS16480_REG_PROD_ID,
.has_paging = true,
.read_delay = 5,
@@ -1296,18 +1263,22 @@ static int adis16480_probe(struct spi_device *spi)
if (ret)
return ret;
- ret = adis16480_config_irq_pin(spi->dev.of_node, st);
+ ret = __adis_initial_startup(&st->adis);
if (ret)
return ret;
+ ret = adis16480_config_irq_pin(spi->dev.of_node, st);
+ if (ret)
+ goto error_stop_device;
+
ret = adis16480_get_ext_clocks(st);
if (ret)
- return ret;
+ goto error_stop_device;
if (!IS_ERR_OR_NULL(st->ext_clk)) {
ret = adis16480_ext_clk_config(st, spi->dev.of_node, true);
if (ret)
- return ret;
+ goto error_stop_device;
st->clk_freq = clk_get_rate(st->ext_clk);
st->clk_freq *= 1000; /* micro */
@@ -1319,24 +1290,20 @@ static int adis16480_probe(struct spi_device *spi)
if (ret)
goto error_clk_disable_unprepare;
- ret = adis16480_initial_setup(indio_dev);
- if (ret)
- goto error_cleanup_buffer;
-
ret = iio_device_register(indio_dev);
if (ret)
- goto error_stop_device;
+ goto error_cleanup_buffer;
adis16480_debugfs_init(indio_dev);
return 0;
-error_stop_device:
- adis16480_stop_device(indio_dev);
error_cleanup_buffer:
adis_cleanup_buffer_and_trigger(&st->adis, indio_dev);
error_clk_disable_unprepare:
clk_disable_unprepare(st->ext_clk);
+error_stop_device:
+ adis16480_stop_device(indio_dev);
return ret;
}
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] iio: imu: adis: Add self_test_reg variable
From: Nuno Sá
This patch adds a dedicated self_test_reg variable. This is also a step
to let new drivers make use of `adis_initial_startup()`. Some devices
use MSG_CTRL reg to request a self_test command while others use the
GLOB_CMD register.
Signed-off-by: Nuno Sá
Signed-off-by: Alexandru Ardelean
---
drivers/iio/accel/adis16201.c | 1 +
drivers/iio/accel/adis16209.c | 1 +
drivers/iio/gyro/adis16136.c | 1 +
drivers/iio/gyro/adis16260.c | 1 +
drivers/iio/imu/adis.c| 6 +++---
drivers/iio/imu/adis16400.c | 1 +
drivers/iio/imu/adis16460.c | 2 ++
drivers/iio/imu/adis16480.c | 3 +++
drivers/staging/iio/accel/adis16203.c | 1 +
drivers/staging/iio/accel/adis16240.c | 1 +
include/linux/iio/imu/adis.h | 2 ++
11 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/accel/adis16201.c b/drivers/iio/accel/adis16201.c
index 0f0f27a8184e..4154e7396bbe 100644
--- a/drivers/iio/accel/adis16201.c
+++ b/drivers/iio/accel/adis16201.c
@@ -246,6 +246,7 @@ static const struct adis_data adis16201_data = {
.diag_stat_reg = ADIS16201_DIAG_STAT_REG,
.self_test_mask = ADIS16201_MSC_CTRL_SELF_TEST_EN,
+ .self_test_reg = ADIS16201_MSC_CTRL_REG,
.self_test_no_autoclear = true,
.timeouts = &adis16201_timeouts,
diff --git a/drivers/iio/accel/adis16209.c b/drivers/iio/accel/adis16209.c
index c6dbd2424e10..31d45e7c5485 100644
--- a/drivers/iio/accel/adis16209.c
+++ b/drivers/iio/accel/adis16209.c
@@ -256,6 +256,7 @@ static const struct adis_data adis16209_data = {
.diag_stat_reg = ADIS16209_STAT_REG,
.self_test_mask = ADIS16209_MSC_CTRL_SELF_TEST_EN,
+ .self_test_reg = ADIS16209_MSC_CTRL_REG,
.self_test_no_autoclear = true,
.timeouts = &adis16209_timeouts,
diff --git a/drivers/iio/gyro/adis16136.c b/drivers/iio/gyro/adis16136.c
index d5e03a406d4a..e8375d4a408f 100644
--- a/drivers/iio/gyro/adis16136.c
+++ b/drivers/iio/gyro/adis16136.c
@@ -472,6 +472,7 @@ static const struct adis_data adis16136_data = {
.msc_ctrl_reg = ADIS16136_REG_MSC_CTRL,
.self_test_mask = ADIS16136_MSC_CTRL_SELF_TEST,
+ .self_test_reg = ADIS16136_REG_MSC_CTRL,
.read_delay = 10,
.write_delay = 10,
diff --git a/drivers/iio/gyro/adis16260.c b/drivers/iio/gyro/adis16260.c
index be09b3e5910c..9823573e811a 100644
--- a/drivers/iio/gyro/adis16260.c
+++ b/drivers/iio/gyro/adis16260.c
@@ -346,6 +346,7 @@ static const struct adis_data adis16260_data = {
.diag_stat_reg = ADIS16260_DIAG_STAT,
.self_test_mask = ADIS16260_MSC_CTRL_MEM_TEST,
+ .self_test_reg = ADIS16260_MSC_CTRL,
.timeouts = &adis16260_timeouts,
.status_error_msgs = adis1620_status_error_msgs,
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index 022bb54fb748..d02b1911b0f2 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -346,8 +346,8 @@ static int adis_self_test(struct adis *adis)
int ret;
const struct adis_timeout *timeouts = adis->data->timeouts;
- ret = __adis_write_reg_16(adis, adis->data->msc_ctrl_reg,
- adis->data->self_test_mask);
+ ret = __adis_write_reg_16(adis, adis->data->self_test_reg,
+ adis->data->self_test_mask);
if (ret) {
dev_err(&adis->spi->dev, "Failed to initiate self test: %d\n",
ret);
@@ -359,7 +359,7 @@ static int adis_self_test(struct adis *adis)
ret = __adis_check_status(adis);
if (adis->data->self_test_no_autoclear)
- __adis_write_reg_16(adis, adis->data->msc_ctrl_reg, 0x00);
+ __adis_write_reg_16(adis, adis->data->self_test_reg, 0x00);
return ret;
}
diff --git a/drivers/iio/imu/adis16400.c b/drivers/iio/imu/adis16400.c
index cfb1c19eb930..a59cd7e0c7ed 100644
--- a/drivers/iio/imu/adis16400.c
+++ b/drivers/iio/imu/adis16400.c
@@ -1126,6 +1126,7 @@ static const struct adis_data adis16400_data = {
.write_delay = 50,
.self_test_mask = ADIS16400_MSC_CTRL_MEM_TEST,
+ .self_test_reg = ADIS16400_MSC_CTRL,
.status_error_msgs = adis16400_status_error_msgs,
.status_error_mask = BIT(ADIS16400_DIAG_STAT_ZACCL_FAIL) |
diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c
index 9539cfe4a259..42fa473c6d81 100644
--- a/drivers/iio/imu/adis16460.c
+++ b/drivers/iio/imu/adis16460.c
@@ -392,6 +392,8 @@ static const struct adis_timeout adis16460_timeouts = {
static const struct adis_data adis16460_data = {
.diag_stat_reg = ADIS16460_REG_DIAG_STAT,
.glob_cmd_reg = ADIS16460_REG_GLOB_CMD,
+ .self_test_mask = BIT(2),
+ .self_test_reg = ADIS16460_REG_GLOB_CMD,
.has_paging = false,
.read_delay = 5,
.write_delay =
[PATCH 4/4] iio: adis16460: Make use of __adis_initial_startup
From: Nuno Sá
All of the actions done in `adis16460_initial_setup()` are now done in
`__adis_initial_startup()` so, there's no need for code duplication.
Signed-off-by: Nuno Sá
Signed-off-by: Alexandru Ardelean
---
drivers/iio/imu/adis16460.c | 37 ++---
1 file changed, 2 insertions(+), 35 deletions(-)
diff --git a/drivers/iio/imu/adis16460.c b/drivers/iio/imu/adis16460.c
index 42fa473c6d81..6f94e81c41eb 100644
--- a/drivers/iio/imu/adis16460.c
+++ b/drivers/iio/imu/adis16460.c
@@ -333,40 +333,6 @@ static int adis16460_enable_irq(struct adis *adis, bool
enable)
return 0;
}
-static int adis16460_initial_setup(struct iio_dev *indio_dev)
-{
- struct adis16460 *st = iio_priv(indio_dev);
- uint16_t prod_id;
- unsigned int device_id;
- int ret;
-
- adis_reset(&st->adis);
- msleep(222);
-
- ret = adis_write_reg_16(&st->adis, ADIS16460_REG_GLOB_CMD, BIT(1));
- if (ret)
- return ret;
- msleep(75);
-
- ret = adis_check_status(&st->adis);
- if (ret)
- return ret;
-
- ret = adis_read_reg_16(&st->adis, ADIS16460_REG_PROD_ID, &prod_id);
- if (ret)
- return ret;
-
- ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
- if (ret != 1)
- return -EINVAL;
-
- if (prod_id != device_id)
- dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do
not match.",
- device_id, prod_id);
-
- return 0;
-}
-
#define ADIS16460_DIAG_STAT_IN_CLK_OOS 7
#define ADIS16460_DIAG_STAT_FLASH_MEM 6
#define ADIS16460_DIAG_STAT_SELF_TEST 5
@@ -392,6 +358,7 @@ static const struct adis_timeout adis16460_timeouts = {
static const struct adis_data adis16460_data = {
.diag_stat_reg = ADIS16460_REG_DIAG_STAT,
.glob_cmd_reg = ADIS16460_REG_GLOB_CMD,
+ .prod_id_reg = ADIS16460_REG_PROD_ID,
.self_test_mask = BIT(2),
.self_test_reg = ADIS16460_REG_GLOB_CMD,
.has_paging = false,
@@ -441,7 +408,7 @@ static int adis16460_probe(struct spi_device *spi)
adis16460_enable_irq(&st->adis, 0);
- ret = adis16460_initial_setup(indio_dev);
+ ret = __adis_initial_startup(&st->adis);
if (ret)
goto error_cleanup_buffer;
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] iio: imu: adis: Refactor adis_initial_startup
From: Nuno Sá
All the ADIS devices perform, at the beginning, a self test to make sure
the device is in a sane state. Furthermore, some drivers also do a call
to `adis_reset()` before the test which is also a good practice. This
patch unifies all those operation so that, there's no need for code
duplication. Furthermore, the rst pin is also checked to make sure the
device is not in HW reset. On top of this, some drivers also read the
device product id and compare it with the device being probed to make
sure the correct device is being handled. This can also be passed to the
library by introducing a variable holding the PROD_ID register of the
device.
Signed-off-by: Nuno Sá
Signed-off-by: Alexandru Ardelean
---
drivers/iio/imu/Kconfig | 1 +
drivers/iio/imu/adis.c | 63 ++--
include/linux/iio/imu/adis.h | 15 -
3 files changed, 61 insertions(+), 18 deletions(-)
diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
index 60bb1029e759..63036cf473c7 100644
--- a/drivers/iio/imu/Kconfig
+++ b/drivers/iio/imu/Kconfig
@@ -85,6 +85,7 @@ endmenu
config IIO_ADIS_LIB
tristate
+ depends on GPIOLIB
help
A set of IO helper functions for the Analog Devices ADIS* device
family.
diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
index d02b1911b0f2..1eca5271380e 100644
--- a/drivers/iio/imu/adis.c
+++ b/drivers/iio/imu/adis.c
@@ -7,6 +7,7 @@
*/
#include
+#include
#include
#include
#include
@@ -365,36 +366,64 @@ static int adis_self_test(struct adis *adis)
}
/**
- * adis_inital_startup() - Performs device self-test
+ * __adis_initial_startup() - Device initial setup
* @adis: The adis device
*
+ * This functions makes sure the device is not in reset, via rst pin.
+ * Furthermore it performs a SW reset (only in the case we are not coming from
+ * reset already) and a self test. It also compares the product id with the
+ * device id if the prod_id_reg variable is set.
+ *
* Returns 0 if the device is operational, a negative error code otherwise.
*
* This function should be called early on in the device initialization
sequence
* to ensure that the device is in a sane and known state and that it is
usable.
*/
-int adis_initial_startup(struct adis *adis)
+int __adis_initial_startup(struct adis *adis)
{
int ret;
-
- mutex_lock(&adis->state_lock);
+ struct gpio_desc *gpio;
+ const struct adis_timeout *timeouts = adis->data->timeouts;
+ const char *iio_name = spi_get_device_id(adis->spi)->name;
+ u16 prod_id, dev_id;
+
+ /* check if the device has rst pin low */
+ gpio = devm_gpiod_get_optional(&adis->spi->dev, "reset", GPIOD_ASIS);
+ if (IS_ERR(gpio)) {
+ return PTR_ERR(gpio);
+ } else if (gpio && gpiod_get_value_cansleep(gpio)) {
+ /* bring device out of reset */
+ gpiod_set_value_cansleep(gpio, 0);
+ msleep(timeouts->reset_ms);
+ } else {
+ ret = __adis_reset(adis);
+ if (ret)
+ return ret;
+ }
ret = adis_self_test(adis);
- if (ret) {
- dev_err(&adis->spi->dev, "Self-test failed, trying reset.\n");
- __adis_reset(adis);
- ret = adis_self_test(adis);
- if (ret) {
- dev_err(&adis->spi->dev, "Second self-test failed,
giving up.\n");
- goto out_unlock;
- }
- }
+ if (ret)
+ return ret;
-out_unlock:
- mutex_unlock(&adis->state_lock);
- return ret;
+ if (!adis->data->prod_id_reg)
+ return 0;
+
+ ret = adis_read_reg_16(adis, adis->data->prod_id_reg, &prod_id);
+ if (ret)
+ return ret;
+
+ ret = sscanf(iio_name, "adis%hu\n", &dev_id);
+ if (ret != 1)
+ return -EINVAL;
+
+ if (prod_id != dev_id)
+ dev_warn(&adis->spi->dev,
+"Device ID(%u) and product ID(%u) do not match.",
+dev_id, prod_id);
+
+ return 0;
}
-EXPORT_SYMBOL_GPL(adis_initial_startup);
+EXPORT_SYMBOL_GPL(__adis_initial_startup);
/**
* adis_single_conversion() - Performs a single sample conversion
diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
index d21a013d1122..c43e7922ab32 100644
--- a/include/linux/iio/imu/adis.h
+++ b/include/linux/iio/imu/adis.h
@@ -41,6 +41,7 @@ struct adis_timeout {
* @glob_cmd_reg: Register address of the GLOB_CMD register
* @msc_ctrl_reg: Register address of the MSC_CTRL register
* @diag_stat_reg: Register address of the DIAG_STAT register
+ * @prod_id_reg: Register address of the PROD_ID register
* @self_test_reg:
[PATCH 1/3] iio: kfifo: add iio_device_attach_kfifo_buffer() helper
This change adds the iio_device_attach_kfifo_buffer() helper/short-hand,
which groups the simple routine of allocating a kfifo buffers via
devm_iio_kfifo_allocate() and calling iio_device_attach_buffer().
The mode_flags parameter is required. The setup_ops parameter is optional.
This function will be a bit more useful when needing to define multiple
buffers per IIO device.
One requirement [that is more a recommendation] for this helper, is to call
it after 'indio_dev' has been populated.
Also, one consequence related to using this helper is that the resource
management of the buffer will be tied to 'indio_dev->dev'. Previously it
was open-coded, and each driver does it slightly differently. Most of them
tied it to the parent device, some of them to 'indio_dev->dev'.
This shouldn't be a problem, and may be a good idea when adding more
buffers per-device.
Signed-off-by: Alexandru Ardelean
---
drivers/iio/buffer/kfifo_buf.c | 37 ++
include/linux/iio/kfifo_buf.h | 4
2 files changed, 41 insertions(+)
diff --git a/drivers/iio/buffer/kfifo_buf.c b/drivers/iio/buffer/kfifo_buf.c
index 3150f8ab984b..05b7c5fc6f1d 100644
--- a/drivers/iio/buffer/kfifo_buf.c
+++ b/drivers/iio/buffer/kfifo_buf.c
@@ -228,4 +228,41 @@ void devm_iio_kfifo_free(struct device *dev, struct
iio_buffer *r)
}
EXPORT_SYMBOL(devm_iio_kfifo_free);
+/**
+ * iio_device_attach_kfifo_buffer - Allocate a kfifo buffer & attach it to an
IIO device
+ * @indio_dev: The device the buffer should be attached to
+ * @mode_flags: The mode flags for this buffer (INDIO_BUFFER_SOFTWARE and/or
+ * INDIO_BUFFER_TRIGGERED).
+ * @setup_ops: The setup_ops required to configure the HW part of the buffer
(optional)
+ *
+ * This function allocates a kfifo buffer via devm_iio_kfifo_allocate() and
+ * attaches it to the IIO device via iio_device_attach_buffer().
+ * This is meant to be a bit of a short-hand/helper function as many driver
+ * seem to do this.
+ */
+int iio_device_attach_kfifo_buffer(struct iio_dev *indio_dev,
+ int mode_flags,
+ const struct iio_buffer_setup_ops *setup_ops)
+{
+ struct iio_buffer *buffer;
+
+ if (mode_flags)
+ mode_flags &= kfifo_access_funcs.modes;
+
+ if (!mode_flags)
+ return -EINVAL;
+
+ buffer = devm_iio_kfifo_allocate(&indio_dev->dev);
+ if (!buffer)
+ return -ENOMEM;
+
+ iio_device_attach_buffer(indio_dev, buffer);
+
+ indio_dev->modes |= mode_flags;
+ indio_dev->setup_ops = setup_ops;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(iio_device_attach_kfifo_buffer);
+
MODULE_LICENSE("GPL");
diff --git a/include/linux/iio/kfifo_buf.h b/include/linux/iio/kfifo_buf.h
index 764659e01b68..2363a931be14 100644
--- a/include/linux/iio/kfifo_buf.h
+++ b/include/linux/iio/kfifo_buf.h
@@ -11,4 +11,8 @@ void iio_kfifo_free(struct iio_buffer *r);
struct iio_buffer *devm_iio_kfifo_allocate(struct device *dev);
void devm_iio_kfifo_free(struct device *dev, struct iio_buffer *r);
+int iio_device_attach_kfifo_buffer(struct iio_dev *indio_dev,
+ int mode_flags,
+ const struct iio_buffer_setup_ops
*setup_ops);
+
#endif
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: iio: ad5933: use iio_device_attach_kfifo_buffer() helper
This driver calls iio_kfifo_allocate() vs devm_iio_kfifo_allocate(). But
the conversion is still simpler here, and cleans-up/reduces some error
paths.
Signed-off-by: Alexandru Ardelean
---
.../staging/iio/impedance-analyzer/ad5933.c | 28 ---
1 file changed, 5 insertions(+), 23 deletions(-)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index af0bcf95ee8a..7bde93c6dd74 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -602,22 +602,6 @@ static const struct iio_buffer_setup_ops
ad5933_ring_setup_ops = {
.postdisable = ad5933_ring_postdisable,
};
-static int ad5933_register_ring_funcs_and_init(struct iio_dev *indio_dev)
-{
- struct iio_buffer *buffer;
-
- buffer = iio_kfifo_allocate();
- if (!buffer)
- return -ENOMEM;
-
- iio_device_attach_buffer(indio_dev, buffer);
-
- /* Ring buffer functions - here trigger setup related */
- indio_dev->setup_ops = &ad5933_ring_setup_ops;
-
- return 0;
-}
-
static void ad5933_work(struct work_struct *work)
{
struct ad5933_state *st = container_of(work,
@@ -738,26 +722,25 @@ static int ad5933_probe(struct i2c_client *client,
indio_dev->dev.parent = &client->dev;
indio_dev->info = &ad5933_info;
indio_dev->name = id->name;
- indio_dev->modes = (INDIO_BUFFER_SOFTWARE | INDIO_DIRECT_MODE);
+ indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->channels = ad5933_channels;
indio_dev->num_channels = ARRAY_SIZE(ad5933_channels);
- ret = ad5933_register_ring_funcs_and_init(indio_dev);
+ ret = iio_device_attach_kfifo_buffer(indio_dev, INDIO_BUFFER_SOFTWARE,
+&ad5933_ring_setup_ops);
if (ret)
goto error_disable_mclk;
ret = ad5933_setup(st);
if (ret)
- goto error_unreg_ring;
+ goto error_disable_mclk;
ret = iio_device_register(indio_dev);
if (ret)
- goto error_unreg_ring;
+ goto error_disable_mclk;
return 0;
-error_unreg_ring:
- iio_kfifo_free(indio_dev->buffer);
error_disable_mclk:
clk_disable_unprepare(st->mclk);
error_disable_reg:
@@ -772,7 +755,6 @@ static int ad5933_remove(struct i2c_client *client)
struct ad5933_state *st = iio_priv(indio_dev);
iio_device_unregister(indio_dev);
- iio_kfifo_free(indio_dev->buffer);
regulator_disable(st->reg);
clk_disable_unprepare(st->mclk);
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] iio: make use of iio_device_attach_kfifo_buffer() where straightforward
All drivers that already call devm_iio_kfifo_allocate() &
iio_device_attach_buffer() are simple to convert to
iio_device_attach_kfifo_buffer() in a single go/patch/.
This change does that.
For drivers max30100 & max30102 this helper is called after indio_dev has
been populated. This doesn't make any difference [at this point in time].
Signed-off-by: Alexandru Ardelean
---
drivers/iio/accel/sca3000.c| 18 ++
drivers/iio/accel/ssp_accel_sensor.c | 13 -
drivers/iio/adc/ina2xx-adc.c | 13 +
drivers/iio/gyro/ssp_gyro_sensor.c | 13 -
drivers/iio/health/max30100.c | 15 ++-
drivers/iio/health/max30102.c | 15 ++-
drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 17 +++--
drivers/iio/light/acpi-als.c | 11 +--
drivers/iio/light/apds9960.c | 15 ++-
9 files changed, 45 insertions(+), 85 deletions(-)
diff --git a/drivers/iio/accel/sca3000.c b/drivers/iio/accel/sca3000.c
index 66d768d971e1..a0db76082ba6 100644
--- a/drivers/iio/accel/sca3000.c
+++ b/drivers/iio/accel/sca3000.c
@@ -1272,20 +1272,6 @@ static int sca3000_write_event_config(struct iio_dev
*indio_dev,
return ret;
}
-static int sca3000_configure_ring(struct iio_dev *indio_dev)
-{
- struct iio_buffer *buffer;
-
- buffer = devm_iio_kfifo_allocate(&indio_dev->dev);
- if (!buffer)
- return -ENOMEM;
-
- iio_device_attach_buffer(indio_dev, buffer);
- indio_dev->modes |= INDIO_BUFFER_SOFTWARE;
-
- return 0;
-}
-
static inline
int __sca3000_hw_ring_state_set(struct iio_dev *indio_dev, bool state)
{
@@ -1480,7 +1466,8 @@ static int sca3000_probe(struct spi_device *spi)
}
indio_dev->modes = INDIO_DIRECT_MODE;
- ret = sca3000_configure_ring(indio_dev);
+ ret = iio_device_attach_kfifo_buffer(indio_dev, INDIO_BUFFER_SOFTWARE,
+&sca3000_ring_setup_ops);
if (ret)
return ret;
@@ -1494,7 +1481,6 @@ static int sca3000_probe(struct spi_device *spi)
if (ret)
return ret;
}
- indio_dev->setup_ops = &sca3000_ring_setup_ops;
ret = sca3000_clean_setup(st);
if (ret)
goto error_free_irq;
diff --git a/drivers/iio/accel/ssp_accel_sensor.c
b/drivers/iio/accel/ssp_accel_sensor.c
index c32647abce20..1d9971db949d 100644
--- a/drivers/iio/accel/ssp_accel_sensor.c
+++ b/drivers/iio/accel/ssp_accel_sensor.c
@@ -96,7 +96,6 @@ static int ssp_accel_probe(struct platform_device *pdev)
int ret;
struct iio_dev *indio_dev;
struct ssp_sensor_data *spd;
- struct iio_buffer *buffer;
indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*spd));
if (!indio_dev)
@@ -111,18 +110,14 @@ static int ssp_accel_probe(struct platform_device *pdev)
indio_dev->dev.parent = &pdev->dev;
indio_dev->dev.of_node = pdev->dev.of_node;
indio_dev->info = &ssp_accel_iio_info;
- indio_dev->modes = INDIO_BUFFER_SOFTWARE;
indio_dev->channels = ssp_acc_channels;
indio_dev->num_channels = ARRAY_SIZE(ssp_acc_channels);
indio_dev->available_scan_masks = ssp_accel_scan_mask;
- buffer = devm_iio_kfifo_allocate(&pdev->dev);
- if (!buffer)
- return -ENOMEM;
-
- iio_device_attach_buffer(indio_dev, buffer);
-
- indio_dev->setup_ops = &ssp_accel_buffer_ops;
+ ret = iio_device_attach_kfifo_buffer(indio_dev, INDIO_BUFFER_SOFTWARE,
+&ssp_accel_buffer_ops);
+ if (ret)
+ return ret;
platform_set_drvdata(pdev, indio_dev);
diff --git a/drivers/iio/adc/ina2xx-adc.c b/drivers/iio/adc/ina2xx-adc.c
index bdd7cba6f6b0..2070809e1acc 100644
--- a/drivers/iio/adc/ina2xx-adc.c
+++ b/drivers/iio/adc/ina2xx-adc.c
@@ -950,7 +950,6 @@ static int ina2xx_probe(struct i2c_client *client,
{
struct ina2xx_chip_info *chip;
struct iio_dev *indio_dev;
- struct iio_buffer *buffer;
unsigned int val;
enum ina2xx_ids type;
int ret;
@@ -1014,7 +1013,7 @@ static int ina2xx_probe(struct i2c_client *client,
return ret;
}
- indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_SOFTWARE;
+ indio_dev->modes = INDIO_DIRECT_MODE;
indio_dev->dev.parent = &client->dev;
indio_dev->dev.of_node = client->dev.of_node;
if (id->driver_data == ina226) {
@@ -1027,13 +1026,11 @@ static int ina2xx_probe(struct i2c_client *client,
indio_dev->info = &ina219_info;
}
indio_dev->name = id->name;
- indio_dev-&
[PATCH] staging: iio: ad5933: rework probe to use devm_ function variants
This change cleans up the driver's probe function to use only devm_
function variants. This also gets rid of the remove function and moves the
clock & regulator de-initializations to the 'ad5933_cleanup()' callback.
Signed-off-by: Alexandru Ardelean
---
.../staging/iio/impedance-analyzer/ad5933.c | 59 ---
1 file changed, 23 insertions(+), 36 deletions(-)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index af0bcf95ee8a..06a6dcd7883b 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -602,11 +602,12 @@ static const struct iio_buffer_setup_ops
ad5933_ring_setup_ops = {
.postdisable = ad5933_ring_postdisable,
};
-static int ad5933_register_ring_funcs_and_init(struct iio_dev *indio_dev)
+static int ad5933_register_ring_funcs_and_init(struct device *dev,
+ struct iio_dev *indio_dev)
{
struct iio_buffer *buffer;
- buffer = iio_kfifo_allocate();
+ buffer = devm_iio_kfifo_allocate(dev);
if (!buffer)
return -ENOMEM;
@@ -676,6 +677,14 @@ static void ad5933_work(struct work_struct *work)
}
}
+static void ad5933_cleanup(void *data)
+{
+ struct ad5933_state *st = data;
+
+ clk_disable_unprepare(st->mclk);
+ regulator_disable(st->reg);
+}
+
static int ad5933_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -703,23 +712,28 @@ static int ad5933_probe(struct i2c_client *client,
dev_err(&client->dev, "Failed to enable specified VDD
supply\n");
return ret;
}
+
+ ret = devm_add_action_or_reset(&client->dev, ad5933_cleanup, st);
+ if (ret)
+ return ret;
+
ret = regulator_get_voltage(st->reg);
if (ret < 0)
- goto error_disable_reg;
+ return ret;
st->vref_mv = ret / 1000;
st->mclk = devm_clk_get(&client->dev, "mclk");
if (IS_ERR(st->mclk) && PTR_ERR(st->mclk) != -ENOENT) {
ret = PTR_ERR(st->mclk);
- goto error_disable_reg;
+ return ret;
}
if (!IS_ERR(st->mclk)) {
ret = clk_prepare_enable(st->mclk);
if (ret < 0)
- goto error_disable_reg;
+ return ret;
ext_clk_hz = clk_get_rate(st->mclk);
}
@@ -742,41 +756,15 @@ static int ad5933_probe(struct i2c_client *client,
indio_dev->channels = ad5933_channels;
indio_dev->num_channels = ARRAY_SIZE(ad5933_channels);
- ret = ad5933_register_ring_funcs_and_init(indio_dev);
+ ret = ad5933_register_ring_funcs_and_init(&client->dev, indio_dev);
if (ret)
- goto error_disable_mclk;
+ return ret;
ret = ad5933_setup(st);
if (ret)
- goto error_unreg_ring;
-
- ret = iio_device_register(indio_dev);
- if (ret)
- goto error_unreg_ring;
-
- return 0;
-
-error_unreg_ring:
- iio_kfifo_free(indio_dev->buffer);
-error_disable_mclk:
- clk_disable_unprepare(st->mclk);
-error_disable_reg:
- regulator_disable(st->reg);
-
- return ret;
-}
-
-static int ad5933_remove(struct i2c_client *client)
-{
- struct iio_dev *indio_dev = i2c_get_clientdata(client);
- struct ad5933_state *st = iio_priv(indio_dev);
-
- iio_device_unregister(indio_dev);
- iio_kfifo_free(indio_dev->buffer);
- regulator_disable(st->reg);
- clk_disable_unprepare(st->mclk);
+ return ret;
- return 0;
+ return devm_iio_device_register(&client->dev, indio_dev);
}
static const struct i2c_device_id ad5933_id[] = {
@@ -801,7 +789,6 @@ static struct i2c_driver ad5933_driver = {
.of_match_table = ad5933_of_match,
},
.probe = ad5933_probe,
- .remove = ad5933_remove,
.id_table = ad5933_id,
};
module_i2c_driver(ad5933_driver);
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: ad2s1210: Fix SPI reading
From: Dragos Bogdan
If the serial interface is used, the 8-bit address should be latched using
the rising edge of the WR/FSYNC signal.
This basically means that a CS change is required between the first byte
sent, and the second one.
This change splits the single-transfer transfer of 2 bytes into 2 transfers
with a single byte, and CS change in-between.
Signed-off-by: Dragos Bogdan
Signed-off-by: Alexandru Ardelean
---
drivers/staging/iio/resolver/ad2s1210.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/iio/resolver/ad2s1210.c
b/drivers/staging/iio/resolver/ad2s1210.c
index 4b25a3a314ed..ed404355ea4c 100644
--- a/drivers/staging/iio/resolver/ad2s1210.c
+++ b/drivers/staging/iio/resolver/ad2s1210.c
@@ -130,17 +130,24 @@ static int ad2s1210_config_write(struct ad2s1210_state
*st, u8 data)
static int ad2s1210_config_read(struct ad2s1210_state *st,
unsigned char address)
{
- struct spi_transfer xfer = {
- .len = 2,
- .rx_buf = st->rx,
- .tx_buf = st->tx,
+ struct spi_transfer xfers[] = {
+ {
+ .len = 1,
+ .rx_buf = &st->rx[0],
+ .tx_buf = &st->tx[0],
+ .cs_change = 1,
+ }, {
+ .len = 1,
+ .rx_buf = &st->rx[1],
+ .tx_buf = &st->tx[1],
+ },
};
int ret = 0;
ad2s1210_set_mode(MOD_CONFIG, st);
st->tx[0] = address | AD2S1210_MSB_IS_HIGH;
st->tx[1] = AD2S1210_REG_FAULT;
- ret = spi_sync_transfer(st->sdev, &xfer, 1);
+ ret = spi_sync_transfer(st->sdev, xfers, 2);
if (ret < 0)
return ret;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/5] iio: core: pass parent device as parameter during allocation
The change passes the parent device to the iio_device_alloc() call. This
also updates the devm_iio_device_alloc() call to consider the device object
as the parent device by default.
Having it passed like this, should ensure that any IIO device object
already has a device object as parent, allowing for neater control, like
passing the 'indio_dev' object for other stuff [like buffers/triggers/etc],
and potentially creating iiom_xxx(indio_dev) functions.
With this patch, only the 'drivers/platform/x86/toshiba_acpi.c' needs an
update to pass the parent object as a parameter.
In the next patch all devm_iio_device_alloc() calls will be handled.
Signed-off-by: Alexandru Ardelean
---
drivers/iio/dummy/iio_simple_dummy.c | 14 --
drivers/iio/industrialio-core.c | 11 ++-
drivers/platform/x86/toshiba_acpi.c | 3 +--
drivers/staging/iio/Documentation/device.txt | 4 +---
include/linux/iio/iio.h | 4 ++--
5 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/iio/dummy/iio_simple_dummy.c
b/drivers/iio/dummy/iio_simple_dummy.c
index 6cb02299a215..b35ae7c039f7 100644
--- a/drivers/iio/dummy/iio_simple_dummy.c
+++ b/drivers/iio/dummy/iio_simple_dummy.c
@@ -566,6 +566,13 @@ static struct iio_sw_device *iio_dummy_probe(const char
*name)
struct iio_dev *indio_dev;
struct iio_dummy_state *st;
struct iio_sw_device *swd;
+ struct device *parent = NULL;
+
+ /*
+* With hardware: Set the parent device.
+* parent = &spi->dev;
+* parent = &client->dev;
+*/
swd = kzalloc(sizeof(*swd), GFP_KERNEL);
if (!swd) {
@@ -580,7 +587,7 @@ static struct iio_sw_device *iio_dummy_probe(const char
*name)
* It also has a region (accessed by iio_priv()
* for chip specific state information.
*/
- indio_dev = iio_device_alloc(sizeof(*st));
+ indio_dev = iio_device_alloc(parent, sizeof(*st));
if (!indio_dev) {
ret = -ENOMEM;
goto error_ret;
@@ -590,11 +597,6 @@ static struct iio_sw_device *iio_dummy_probe(const char
*name)
mutex_init(&st->lock);
iio_dummy_init_device(indio_dev);
- /*
-* With hardware: Set the parent device.
-* indio_dev->dev.parent = &spi->dev;
-* indio_dev->dev.parent = &client->dev;
-*/
/*
* Make the iio_dev struct available to remove function.
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 1527f01a44f1..75661661aaba 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -1493,7 +1493,7 @@ struct device_type iio_device_type = {
* iio_device_alloc() - allocate an iio_dev from a driver
* @sizeof_priv: Space to allocate for private structure.
**/
-struct iio_dev *iio_device_alloc(int sizeof_priv)
+struct iio_dev *iio_device_alloc(struct device *parent, int sizeof_priv)
{
struct iio_dev *dev;
size_t alloc_size;
@@ -1510,6 +1510,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
if (!dev)
return NULL;
+ dev->dev.parent = parent;
dev->dev.groups = dev->groups;
dev->dev.type = &iio_device_type;
dev->dev.bus = &iio_bus_type;
@@ -1551,7 +1552,7 @@ static void devm_iio_device_release(struct device *dev,
void *res)
/**
* devm_iio_device_alloc - Resource-managed iio_device_alloc()
- * @dev: Device to allocate iio_dev for
+ * @parent:Device to allocate iio_dev for, and parent for this IIO
device
* @sizeof_priv: Space to allocate for private structure.
*
* Managed iio_device_alloc. iio_dev allocated with this function is
@@ -1560,7 +1561,7 @@ static void devm_iio_device_release(struct device *dev,
void *res)
* RETURNS:
* Pointer to allocated iio_dev on success, NULL on failure.
*/
-struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv)
+struct iio_dev *devm_iio_device_alloc(struct device *parent, int sizeof_priv)
{
struct iio_dev **ptr, *iio_dev;
@@ -1569,10 +1570,10 @@ struct iio_dev *devm_iio_device_alloc(struct device
*dev, int sizeof_priv)
if (!ptr)
return NULL;
- iio_dev = iio_device_alloc(sizeof_priv);
+ iio_dev = iio_device_alloc(parent, sizeof_priv);
if (iio_dev) {
*ptr = iio_dev;
- devres_add(dev, ptr);
+ devres_add(parent, ptr);
} else {
devres_free(ptr);
}
diff --git a/drivers/platform/x86/toshiba_acpi.c
b/drivers/platform/x86/toshiba_acpi.c
index 808944546739..4a4d09c352dd 100644
--- a/drivers/platform/x86/toshiba_acpi.c
+++ b/drivers/platform/x86/toshiba_acpi.c
@@ -3128,7 +3128,7 @@ static int toshiba_acpi_add(struct acpi_device *acpi_dev)
[PATCH 3/5] iio: remove left-over comments about parent assignment
These were obtained by doing a 'git diff | grep \/\*', in the previous diff to find comments. These needed a bit more manual review, as the semantic patch isn't great for catching these. The result is: /* Initialize Counter device and driver data */ /* Initialize IIO device */ /* Establish that the iio_dev is a child of the spi device */ /* Estabilish that the iio_dev is a child of the spi device */ /* Initiate the Industrial I/O device */ /* Establish that the iio_dev is a child of the device */ - /* establish that the iio_dev is a child of the i2c device */ - /* establish that the iio_dev is a child of the i2c device */ /* This is only used for removal purposes */ /* setup the industrialio driver allocated elements */ /* variant specific configuration */ /* Setup for userspace synchronous on demand sampling. */ st->readback_delay_us += 5; /* Add tWAIT */ - /* Establish that the iio_dev is a child of the i2c device */ /* Establish that the iio_dev is a child of the i2c device */ Out of which, 4 are really left-over comments about parent assignment. 3 of them are removed by the semantic patch, as the comment removed (by spatch) would be for an empty line. Signed-off-by: Alexandru Ardelean --- drivers/iio/adc/ad7476.c | 1 - drivers/iio/adc/ad7887.c | 1 - drivers/iio/dac/ad5446.c | 1 - drivers/staging/iio/cdc/ad7746.c | 1 - 4 files changed, 4 deletions(-) diff --git a/drivers/iio/adc/ad7476.c b/drivers/iio/adc/ad7476.c index e2a69dd6a47e..6286e230f55b 100644 --- a/drivers/iio/adc/ad7476.c +++ b/drivers/iio/adc/ad7476.c @@ -300,7 +300,6 @@ static int ad7476_probe(struct spi_device *spi) st->spi = spi; - /* Establish that the iio_dev is a child of the spi device */ indio_dev->dev.of_node = spi->dev.of_node; indio_dev->name = spi_get_device_id(spi)->name; indio_dev->modes = INDIO_DIRECT_MODE; diff --git a/drivers/iio/adc/ad7887.c b/drivers/iio/adc/ad7887.c index ca4c98401ebc..0f93f5c8965d 100644 --- a/drivers/iio/adc/ad7887.c +++ b/drivers/iio/adc/ad7887.c @@ -264,7 +264,6 @@ static int ad7887_probe(struct spi_device *spi) spi_set_drvdata(spi, indio_dev); st->spi = spi; - /* Estabilish that the iio_dev is a child of the spi device */ indio_dev->dev.of_node = spi->dev.of_node; indio_dev->name = spi_get_device_id(spi)->name; indio_dev->info = &ad7887_info; diff --git a/drivers/iio/dac/ad5446.c b/drivers/iio/dac/ad5446.c index e01ba90dc106..5931bd630c4e 100644 --- a/drivers/iio/dac/ad5446.c +++ b/drivers/iio/dac/ad5446.c @@ -250,7 +250,6 @@ static int ad5446_probe(struct device *dev, const char *name, st->reg = reg; st->dev = dev; - /* Establish that the iio_dev is a child of the device */ indio_dev->name = name; indio_dev->info = &ad5446_info; indio_dev->modes = INDIO_DIRECT_MODE; diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c index bd9803c7c5b6..dfd71e99e872 100644 --- a/drivers/staging/iio/cdc/ad7746.c +++ b/drivers/staging/iio/cdc/ad7746.c @@ -693,7 +693,6 @@ static int ad7746_probe(struct i2c_client *client, chip->client = client; chip->capdac_set = -1; - /* Establish that the iio_dev is a child of the i2c device */ indio_dev->name = id->name; indio_dev->info = &ad7746_info; indio_dev->channels = ad7746_channels; -- 2.25.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/5] iio: light: lm3533-als: remove explicit parent assignment
This assignment is the more peculiar of the bunch as it assigns the parent of the platform-device's device (i.e. pdev->dev.parent) as the IIO device's parent. It's unclear whether this is intentional or not. Hence it is in it's own patch. Signed-off-by: Alexandru Ardelean --- drivers/iio/light/lm3533-als.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/iio/light/lm3533-als.c b/drivers/iio/light/lm3533-als.c index bc196c212881..0f380ec8d30c 100644 --- a/drivers/iio/light/lm3533-als.c +++ b/drivers/iio/light/lm3533-als.c @@ -852,7 +852,6 @@ static int lm3533_als_probe(struct platform_device *pdev) indio_dev->channels = lm3533_als_channels; indio_dev->num_channels = ARRAY_SIZE(lm3533_als_channels); indio_dev->name = dev_name(&pdev->dev); - indio_dev->dev.parent = pdev->dev.parent; indio_dev->modes = INDIO_DIRECT_MODE; als = iio_priv(indio_dev); -- 2.25.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/5] iio: remove left-over parent assignments
These were found by doing some shell magic: for file in $(git grep -w devm_iio_device_alloc | cut -d: -f1 | sort | uniq) ; do if grep 'parent =' $file | grep -v trig | grep -vq devm_; then echo "$file -> $(grep "parent =" $file)" fi done --- The output is bearable [after the semantic patch is applied]. There is a mix of trigger assignments with some iio device parent assignments that are removed via this patch. Signed-off-by: Alexandru Ardelean --- drivers/iio/accel/kxcjk-1013.c| 1 - drivers/iio/accel/mma8452.c | 1 - drivers/iio/accel/mma9553.c | 1 - drivers/iio/adc/ad7192.c | 1 - drivers/iio/adc/hx711.c | 1 - drivers/iio/adc/max1363.c | 2 -- drivers/iio/adc/mcp3911.c | 1 - drivers/iio/adc/qcom-spmi-iadc.c | 1 - drivers/iio/amplifiers/ad8366.c | 1 - drivers/iio/chemical/vz89x.c | 1 - drivers/iio/dac/ad5770r.c | 1 - drivers/iio/health/afe4403.c | 1 - drivers/iio/health/afe4404.c | 1 - drivers/iio/humidity/dht11.c | 1 - drivers/iio/humidity/hts221_core.c| 1 - drivers/iio/imu/inv_mpu6050/inv_mpu_core.c| 1 - drivers/iio/light/cm3605.c| 1 - drivers/iio/light/ltr501.c| 1 - drivers/iio/magnetometer/ak8975.c | 1 - drivers/iio/orientation/hid-sensor-rotation.c | 1 - drivers/iio/potentiostat/lmp91000.c | 1 - drivers/iio/proximity/ping.c | 1 - drivers/iio/proximity/pulsedlight-lidar-lite-v2.c | 1 - drivers/iio/proximity/srf04.c | 1 - drivers/iio/proximity/srf08.c | 1 - drivers/iio/temperature/tsys01.c | 1 - drivers/staging/iio/addac/adt7316.c | 1 - 27 files changed, 28 deletions(-) diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c index c9924a65c32a..6b93521c0e17 100644 --- a/drivers/iio/accel/kxcjk-1013.c +++ b/drivers/iio/accel/kxcjk-1013.c @@ -1311,7 +1311,6 @@ static int kxcjk1013_probe(struct i2c_client *client, mutex_init(&data->mutex); - indio_dev->dev.parent = &client->dev; indio_dev->channels = kxcjk1013_channels; indio_dev->num_channels = ARRAY_SIZE(kxcjk1013_channels); indio_dev->available_scan_masks = kxcjk1013_scan_masks; diff --git a/drivers/iio/accel/mma8452.c b/drivers/iio/accel/mma8452.c index 00e100fc845a..ef3df402fc3c 100644 --- a/drivers/iio/accel/mma8452.c +++ b/drivers/iio/accel/mma8452.c @@ -1592,7 +1592,6 @@ static int mma8452_probe(struct i2c_client *client, i2c_set_clientdata(client, indio_dev); indio_dev->info = &mma8452_info; indio_dev->name = id->name; - indio_dev->dev.parent = &client->dev; indio_dev->modes = INDIO_DIRECT_MODE; indio_dev->channels = data->chip_info->channels; indio_dev->num_channels = data->chip_info->num_channels; diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c index 312070dcf035..c15908faa381 100644 --- a/drivers/iio/accel/mma9553.c +++ b/drivers/iio/accel/mma9553.c @@ -1103,7 +1103,6 @@ static int mma9553_probe(struct i2c_client *client, if (ret < 0) return ret; - indio_dev->dev.parent = &client->dev; indio_dev->channels = mma9553_channels; indio_dev->num_channels = ARRAY_SIZE(mma9553_channels); indio_dev->name = name; diff --git a/drivers/iio/adc/ad7192.c b/drivers/iio/adc/ad7192.c index 08ba1a8f05eb..a0837d7e9176 100644 --- a/drivers/iio/adc/ad7192.c +++ b/drivers/iio/adc/ad7192.c @@ -970,7 +970,6 @@ static int ad7192_probe(struct spi_device *spi) spi_set_drvdata(spi, indio_dev); st->chip_info = of_device_get_match_data(&spi->dev); - indio_dev->dev.parent = &spi->dev; indio_dev->name = st->chip_info->name; indio_dev->modes = INDIO_DIRECT_MODE; diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c index c8686558429b..6a173531d355 100644 --- a/drivers/iio/adc/hx711.c +++ b/drivers/iio/adc/hx711.c @@ -551,7 +551,6 @@ static int hx711_probe(struct platform_device *pdev) platform_set_drvdata(pdev, indio_dev); indio_dev->name = "hx711"; - indio_dev->dev.parent = &pdev->dev; indio_dev->info = &hx711_iio_info; indio_dev->modes = INDIO_DIRECT_MODE; indio_dev->channels = hx711_chan_spec; diff --git a/drivers/iio/adc/max1363.c b/drivers/iio/adc/max1363.c index 9d92017c79b2..cc1ba7bfc8e6 100644
