On Fri, Jun 16, 2017 at 09:09:31AM +0200, Geert Uytterhoeven wrote: > Hi Simon, > > On Fri, Jun 16, 2017 at 9:04 AM, Simon Horman <horms@xxxxxxxxxxxx> wrote: > > On Fri, Jun 16, 2017 at 08:25:12AM +0200, Geert Uytterhoeven wrote: > >> 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? > > > > Thanks, I see that is still a problem. > > > > As there is only one other driver and its closerly related > > perhaps it could just implement the callback in this patch? > > Yes, that sounds good to me. > Else you need 3 patches: > 1. Introduced .complete field, > 2. Update driver to provide .complete handledr, > 3. Update generic code. Yes, and 3 patch seems a bit excessive in this case. Sorry for letting this bug creep in to renesas-drivers. Do you want a patch against your tree? I will plan to post v2 of this patchset soon in any case. -- 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