Re: [PATCH v3] mmc: Convert from tasklet to BH workqueue

quoted

quoted

The only generic interface to execute asynchronously in the BH context is
tasklet; however, it's marked deprecated and has some design flaws. To
replace tasklets, BH workqueue support was recently added. A BH workqueue
behaves similarly to regular workqueues except that the queued work items
are executed in the BH context.

This patch converts drivers/mmc/* from tasklet to BH workqueue.

Based on the work done by Tejun Heo [off-list ref]

Has this been fully build-tested?

===
drivers/mmc/host/renesas_sdhi_internal_dmac.c: In function ‘renesas_sdhi_internal_dmac_complete_work_fn’:
./include/linux/container_of.h:20:54: error: ‘struct tmio_mmc_host’ has no member named ‘dma_complete’
===

 Yes, it does break. My bad, my local compile testing failed to catch this.

quoted

In deed, 'dma_complete' is only in 'struct renesas_sdhi_dma'. From
there, we can get to the parent 'struct renesas_sdhi' using
container_of. But then, I don't see a way to go to 'struct
tmio_mmc_host' from there. The other way around is possible because
there is the pointer 'struct tmio_mmc_data *pdata' in the TMIO struct
pointing to the data contained in 'struct renesas_sdhi'. 'host_to_priv()'
does the math. But I don't see a path the other way around.

 I have been looking at this code since the issue was reported. Yes it
is a bit tricky and so far, the only way I found was to introduce a new pointer.
But, I am not very familiar with the code and have asked Ulf for pointers.

If introducing the pointer is the only way forward and is an
acceptable solution,
I can send out a draft.

Thanks,
Allen

quoted

So, it doesn't look like the workqueue interface can provide a
generic pointer like tasklets could do? This means we have to add a
pointer from 'struct renesas_sdhi' to 'struct tmio_mmc_host'?

How about the following?

diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c

b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index 5fd276492f80..9a63b78837e2 100644

--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c

@@ -484,9 +484,14 @@ static bool

renesas_sdhi_internal_dmac_complete(struct tmio_mmc_host *host)
        return true;
 }

-static void renesas_sdhi_internal_dmac_complete_work_fn(struct work_struct *t)
+static void renesas_sdhi_internal_dmac_complete_work_fn(struct
work_struct *work)
 {
-       struct tmio_mmc_host *host = from_work(host, t, dma_complete);
+       struct renesas_sdhi_dma *dma_priv = container_of(work,
+                                                        struct
renesas_sdhi_dma,
+                                                        dma_complete);
+       struct renesas_sdhi *sdhi = container_of(dma_priv,
+                                                struct renesas_sdhi, dma_priv);
+       struct tmio_mmc_host *host = sdhi->mmc_data->host;

        spin_lock_irq(&host->lock);
        if (!renesas_sdhi_internal_dmac_complete(host))

diff --git a/include/linux/mfd/tmio.h b/include/linux/mfd/tmio.h
index eace8ea6cda0..cf2cca0e96b5 100644
--- a/include/linux/mfd/tmio.h
+++ b/include/linux/mfd/tmio.h

@@ -100,6 +100,7 @@ struct tmio_mmc_data {
        dma_addr_t                      dma_rx_offset;
        unsigned int                    max_blk_count;
        unsigned short                  max_segs;
+       struct tmio_mmc_host            *host;          /* back

pointer to the parent struct */
        void (*set_pwr)(struct platform_device *host, int state);
        void (*set_clk_div)(struct platform_device *host, int state);
 };

Thanks.