Hi Simon,
On Fri, Jun 16, 2017 at 8:06 AM, Simon Horman <horms@xxxxxxxxxxxx> wrote:
> On Fri, Jun 09, 2017 at 09:35:20AM +0200, Geert Uytterhoeven wrote:
>> On Thu, Jun 8, 2017 at 3:09 PM, Simon Horman <horms+renesas@xxxxxxxxxxxx> wrote:
>> > Add complete to DMA ops to allow DMAC implementation dependent
>> > handling of DMA completion.
>> >
>> > Signed-off-by: Simon Horman <horms+renesas@xxxxxxxxxxxx>
>> > ---
>> > drivers/mmc/host/tmio_mmc.h | 1 +
>> > drivers/mmc/host/tmio_mmc_core.c | 10 ++++++++--
>> > 2 files changed, 9 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
>> > index 9c94b6eb9b49..82725f9de359 100644
>> > --- a/drivers/mmc/host/tmio_mmc.h
>> > +++ b/drivers/mmc/host/tmio_mmc.h
>> > @@ -121,6 +121,7 @@ struct tmio_mmc_dma_ops {
>> > struct tmio_mmc_data *pdata);
>> > void (*release)(struct tmio_mmc_host *host);
>> > void (*abort)(struct tmio_mmc_host *host);
>> > + void (*complete)(struct tmio_mmc_host *host);
>> > };
>> >
>> > struct tmio_mmc_host {
>> > diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
>> > index 0e76e099dc7f..26fe9bb64849 100644
>> > --- a/drivers/mmc/host/tmio_mmc_core.c
>> > +++ b/drivers/mmc/host/tmio_mmc_core.c
>> > @@ -86,6 +86,12 @@ static inline void tmio_mmc_abort_dma(struct tmio_mmc_host *host)
>> > host->dma_ops->abort(host);
>> > }
>> >
>> > +static inline void tmio_mmc_complete_dma(struct tmio_mmc_host *host)
>> > +{
>> > + if (host->dma_ops)
>> > + host->dma_ops->complete(host);
>>
>> I believe this will cause a crash and thus break bisection, as no driver
>> fills in the complete pointer yet.
>
> Indeed. And it seems to cause a panic on non-Gen3 users of the driver.
>
> I think the fix should be:
>
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index 26fe9bb64849..9ec2e933b6f0 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -88,7 +88,7 @@ static inline void tmio_mmc_abort_dma(struct tmio_mmc_host *host)
>
> static inline void tmio_mmc_complete_dma(struct tmio_mmc_host *host)
> {
> - if (host->dma_ops)
> + if (host->dma_ops && host->dma_ops->complete)
> host->dma_ops->complete(host);
> }
Is it? Now nothing will be completed?
The old code did:
complete(&host->dma_dataend);
so perhaps
else
complete(&host->dma_dataend);
with the "else" branch to be deleted when all drivers that need it provide
their own complete callback?
>> > +}
>> > +
>> > void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i)
>> > {
>> > host->sdcard_irq_mask &= ~(i & TMIO_MASK_IRQ);
>> > @@ -604,11 +610,11 @@ static void tmio_mmc_data_irq(struct tmio_mmc_host *host, unsigned int stat)
>> >
>> > if (done) {
>> > tmio_mmc_disable_mmc_irqs(host, TMIO_STAT_DATAEND);
>> > - complete(&host->dma_dataend);
>> > + tmio_mmc_complete_dma(host);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
