On 17.06.2020 17:02, Ardelean, Alexandru wrote:

On Wed, 2020-06-17 at 13:25 +0000, Eugen.Hristev@microchip.com wrote:

quoted

On 31.05.2020 17:39, Jonathan Cameron wrote:

quoted

On Mon, 25 May 2020 13:53:41 +0300
Alexandru Ardelean [off-list ref] wrote:

quoted

We may want to get rid of the iio_priv_to_dev() helper. The reason is that
we will hide some of the members of the iio_dev structure (to prevent
drivers from accessing them directly), and that will also mean hiding the
implementation of the iio_priv_to_dev() helper inside the IIO core.

Hiding the implementation of iio_priv_to_dev() implies that some fast-
paths
may not be fast anymore, so a general idea is to try to get rid of the
iio_priv_to_dev() altogether.
The iio_priv() helper won't be affected by the rework, as the iio_dev
struct will keep a reference to the private information.

For this driver, not using iio_priv_to_dev(), means reworking some paths
to
pass the iio device and using iio_priv() to access the private
information,
and also keeping a reference to the iio device for some quirky paths.

One [quirky] path is the at91_adc_workq_handler() which requires the IIO
device & the state struct to push to buffers.
Since this requires the back-ref to the IIO device, the
at91_adc_touch_pos() also uses it. This simplifies the patch a bit. The
information required in this function is mostly for debugging purposes.
Replacing it with a reference to the IIO device would have been a slightly
bigger change, which may not be worth it (for just the debugging purpose
and given that we need the back-ref to the IIO device anyway).

That workq is indeed quirky.  This looks fine to me in general. I'll
want an appropriate ack from the at91 side of things if possible so
let's leave this on the list for a while longer.

Hi,

I am available to test this patch,
Can you tell me on which branch to apply it. On 5.8-rc1 it fails for me
(or maybe it needs rebasing ?)

Hmm, weird.
I rebased the patches on Jonathan's iio/testing.
It seemed to work.
https://github.com/commodo/linux/commits/iio-priv-to-dev

As for which branch to test/apply. Not sure.
Maybe Jonathan's iio/testing as base?
Looks like it's based on 5.8.

Tested-by: Eugen Hristev <redacted>

I have tested the major features of the driver (including the resistive 
touch) and it looks to be working fine.

I did not fully understand the quirkyness of the workq . Something you 
want to change to that ?

quoted

Eugen

quoted

Poke me if no action in a few weeks.

Thanks,

Jonathan

quoted

Signed-off-by: Alexandru Ardelean <redacted>
---
   drivers/iio/adc/at91-sama5d2_adc.c | 30 +++++++++++++++++-------------
   1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-

sama5d2_adc.c
index 9abbbdcc7420..7bce18444430 100644

--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c

@@ -402,6 +402,7 @@ struct at91_adc_state {
        wait_queue_head_t               wq_data_available;
        struct at91_adc_dma             dma_st;
        struct at91_adc_touch           touch_st;
+     struct iio_dev                  *indio_dev;
        u16                             buffer[AT91_BUFFER_MAX_HWORDS];
        /*
         * lock to prevent concurrent 'single conversion' requests through

@@ -642,13 +643,13 @@ static u16 at91_adc_touch_pos(struct at91_adc_state

*st, int reg)
        /* first half of register is the x or y, second half is the scale
*/
        val = at91_adc_readl(st, reg);
        if (!val)
-             dev_dbg(&iio_priv_to_dev(st)->dev, "pos is 0\n");
+             dev_dbg(&st->indio_dev->dev, "pos is 0\n");

        pos = val & AT91_SAMA5D2_XYZ_MASK;
        result = (pos << AT91_SAMA5D2_MAX_POS_BITS) - pos;
        scale = (val >> 16) & AT91_SAMA5D2_XYZ_MASK;
        if (scale == 0) {
-             dev_err(&iio_priv_to_dev(st)->dev, "scale is 0\n");
+             dev_err(&st->indio_dev->dev, "scale is 0\n");
                return 0;
        }
        result /= scale;

@@ -1204,9 +1205,9 @@ static unsigned at91_adc_startup_time(unsigned

startup_time_min,
        return i;
   }

-static void at91_adc_setup_samp_freq(struct at91_adc_state *st, unsigned
freq)
+static void at91_adc_setup_samp_freq(struct iio_dev *indio_dev, unsigned
freq)
   {
-     struct iio_dev *indio_dev = iio_priv_to_dev(st);
+     struct at91_adc_state *st = iio_priv(indio_dev);
        unsigned f_per, prescal, startup, mr;

        f_per = clk_get_rate(st->per_clk);

@@ -1275,9 +1276,9 @@ static void at91_adc_pen_detect_interrupt(struct

at91_adc_state *st)
        st->touch_st.touching = true;
   }

-static void at91_adc_no_pen_detect_interrupt(struct at91_adc_state *st)
+static void at91_adc_no_pen_detect_interrupt(struct iio_dev *indio_dev)
   {
-     struct iio_dev *indio_dev = iio_priv_to_dev(st);
+     struct at91_adc_state *st = iio_priv(indio_dev);

        at91_adc_writel(st, AT91_SAMA5D2_TRGR,
                        AT91_SAMA5D2_TRGR_TRGMOD_NO_TRIGGER);

@@ -1297,7 +1298,7 @@ static void at91_adc_workq_handler(struct

work_struct *workq)
                                        struct at91_adc_touch, workq);
        struct at91_adc_state *st = container_of(touch_st,
                                        struct at91_adc_state, touch_st);
-     struct iio_dev *indio_dev = iio_priv_to_dev(st);
+     struct iio_dev *indio_dev = st->indio_dev;

        iio_push_to_buffers(indio_dev, st->buffer);
   }

@@ -1318,7 +1319,7 @@ static irqreturn_t at91_adc_interrupt(int irq, void

*private)
                at91_adc_pen_detect_interrupt(st);
        } else if ((status & AT91_SAMA5D2_IER_NOPEN)) {
                /* nopen detected IRQ */
-             at91_adc_no_pen_detect_interrupt(st);
+             at91_adc_no_pen_detect_interrupt(indio);
        } else if ((status & AT91_SAMA5D2_ISR_PENS) &&
                   ((status & rdy_mask) == rdy_mask)) {
                /* periodic trigger IRQ - during pen sense */

@@ -1486,7 +1487,7 @@ static int at91_adc_write_raw(struct iio_dev

*indio_dev,
                    val > st->soc_info.max_sample_rate)
                        return -EINVAL;

-             at91_adc_setup_samp_freq(st, val);
+             at91_adc_setup_samp_freq(indio_dev, val);
                return 0;
        default:
                return -EINVAL;

@@ -1624,8 +1625,10 @@ static int at91_adc_update_scan_mode(struct iio_dev

*indio_dev,
        return 0;
   }

-static void at91_adc_hw_init(struct at91_adc_state *st)
+static void at91_adc_hw_init(struct iio_dev *indio_dev)
   {
+     struct at91_adc_state *st = iio_priv(indio_dev);
+
        at91_adc_writel(st, AT91_SAMA5D2_CR, AT91_SAMA5D2_CR_SWRST);
        at91_adc_writel(st, AT91_SAMA5D2_IDR, 0xffffffff);
        /*

@@ -1635,7 +1638,7 @@ static void at91_adc_hw_init(struct at91_adc_state

*st)
        at91_adc_writel(st, AT91_SAMA5D2_MR,
                        AT91_SAMA5D2_MR_TRANSFER(2) |
AT91_SAMA5D2_MR_ANACH);

-     at91_adc_setup_samp_freq(st, st->soc_info.min_sample_rate);
+     at91_adc_setup_samp_freq(indio_dev, st->soc_info.min_sample_rate);

        /* configure extended mode register */
        at91_adc_config_emr(st);

@@ -1718,6 +1721,7 @@ static int at91_adc_probe(struct platform_device

*pdev)
        indio_dev->num_channels = ARRAY_SIZE(at91_adc_channels);

        st = iio_priv(indio_dev);
+     st->indio_dev = indio_dev;

        bitmap_set(&st->touch_st.channels_bitmask,
                   AT91_SAMA5D2_TOUCH_X_CHAN_IDX, 1);

@@ -1829,7 +1833,7 @@ static int at91_adc_probe(struct platform_device

*pdev)
                goto vref_disable;
        }

-     at91_adc_hw_init(st);
+     at91_adc_hw_init(indio_dev);

        ret = clk_prepare_enable(st->per_clk);
        if (ret)

@@ -1945,7 +1949,7 @@ static __maybe_unused int at91_adc_resume(struct

device *dev)
        if (ret)
                goto vref_disable_resume;

-     at91_adc_hw_init(st);
+     at91_adc_hw_init(indio_dev);

        /* reconfiguring trigger hardware state */
        if (!iio_buffer_enabled(indio_dev))

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel