[PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Guennadi Liakhovetski]

On platforms with no support for the shdma dmaengine driver build is
currently failing with

drivers/built-in.o: In function `sh_mobile_sdhi_probe':
drivers/mmc/host/sh_mobile_sdhi.c:170: undefined reference to`shdma_chan_filter'

Fix the breakage by defining shdma_chan_filter to NULL in such
configurations.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---

This is for "next." Compile-tested only. I'll test it on hardware next 
week, but I don't think it shall break anything.

 include/linux/sh_dma.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/include/linux/sh_dma.h b/include/linux/sh_dma.h
index b64d6be..1fd8a20 100644
--- a/include/linux/sh_dma.h
+++ b/include/linux/sh_dma.h
@@ -99,6 +99,10 @@ struct sh_dmae_pdata {
 #define CHCR_TE	0x00000002
 #define CHCR_IE	0x00000004
 
+#if IS_ENABLED(CONFIG_SH_DMAE)
 bool shdma_chan_filter(struct dma_chan *chan, void *arg);
+#else
+#define shdma_chan_filter NULL
+#endif
 
 #endif
-- 
1.7.2.5

Re: [PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Dan Murphy]

On 05/30/2013 11:02 AM, Guennadi Liakhovetski wrote:
> On platforms with no support for the shdma dmaengine driver build is
> currently failing with
>
> drivers/built-in.o: In function `sh_mobile_sdhi_probe':
> drivers/mmc/host/sh_mobile_sdhi.c:170: undefined reference to`shdma_chan_filter'
>
> Fix the breakage by defining shdma_chan_filter to NULL in such
> configurations.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> ---
>
> This is for "next." Compile-tested only. I'll test it on hardware next 
> week, but I don't think it shall break anything.
>
>  include/linux/sh_dma.h |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/sh_dma.h b/include/linux/sh_dma.h
> index b64d6be..1fd8a20 100644
> --- a/include/linux/sh_dma.h
> +++ b/include/linux/sh_dma.h
> @@ -99,6 +99,10 @@ struct sh_dmae_pdata {
>  #define CHCR_TE	0x00000002
>  #define CHCR_IE	0x00000004
>  
> +#if IS_ENABLED(CONFIG_SH_DMAE)
>  bool shdma_chan_filter(struct dma_chan *chan, void *arg);
> +#else
> +#define shdma_chan_filter NULL
Would this not be better as a
#else
static inline bool shdma_chan_filter(struct dma_chan *chan, void *arg)
{
    return false;
}
#endif

Otherwise runtime will call a NULL pointer

> +#endif
>  
>  #endif


-- 
------------------
Dan Murphy

Re: [PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Simon Horman]

On Thu, May 30, 2013 at 11:23:13AM -0500, Dan Murphy wrote:
> On 05/30/2013 11:02 AM, Guennadi Liakhovetski wrote:
> > On platforms with no support for the shdma dmaengine driver build is
> > currently failing with
> >
> > drivers/built-in.o: In function `sh_mobile_sdhi_probe':
> > drivers/mmc/host/sh_mobile_sdhi.c:170: undefined reference to`shdma_chan_filter'
> >
> > Fix the breakage by defining shdma_chan_filter to NULL in such
> > configurations.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > ---
> >
> > This is for "next." Compile-tested only. I'll test it on hardware next 
> > week, but I don't think it shall break anything.
> >
> >  include/linux/sh_dma.h |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/sh_dma.h b/include/linux/sh_dma.h
> > index b64d6be..1fd8a20 100644
> > --- a/include/linux/sh_dma.h
> > +++ b/include/linux/sh_dma.h
> > @@ -99,6 +99,10 @@ struct sh_dmae_pdata {
> >  #define CHCR_TE	0x00000002
> >  #define CHCR_IE	0x00000004
> >  
> > +#if IS_ENABLED(CONFIG_SH_DMAE)
> >  bool shdma_chan_filter(struct dma_chan *chan, void *arg);
> > +#else
> > +#define shdma_chan_filter NULL
> Would this not be better as a
> #else
> static inline bool shdma_chan_filter(struct dma_chan *chan, void *arg)
> {
>     return false;
> }
> #endif
> 
> Otherwise runtime will call a NULL pointer
> 
> > +#endif
> >  
> >  #endif

Hi Guennadi,

thanks for this. I will wait for a refresh (as we discussed earlier today).
Can I confirm that this is a fix for v3.10? If so, could ou note
that when you post your revised patch?

Thanks

Re: [PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Kevin Hilman]

On Thu, May 30, 2013 at 7:44 PM, Simon Horman <horms@verge.net.au> wrote:

[...]

> thanks for this. I will wait for a refresh (as we discussed earlier today).
> Can I confirm that this is a fix for v3.10? If so, could ou note
> that when you post your revised patch?

Any progress on this patch?

The SH-mobile defconfigs are still all failing in linux-next.

Kevin

Re: [PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Guennadi Liakhovetski]

Hi Vinod

On Wed, 19 Jun 2013, Kevin Hilman wrote:

> On Thu, May 30, 2013 at 7:44 PM, Simon Horman <horms@verge.net.au> wrote:
> 
> [...]
> 
> > thanks for this. I will wait for a refresh (as we discussed earlier today).
> > Can I confirm that this is a fix for v3.10? If so, could ou note
> > that when you post your revised patch?
> 
> Any progress on this patch?
> 
> The SH-mobile defconfigs are still all failing in linux-next.

In

https://patchwork.kernel.org/patch/2640061/

I proposed a simple immediate fix for this problem. Arnd at the same time 
developed an alternative solution:

https://patchwork.kernel.org/patch/2644121/
https://patchwork.kernel.org/patch/2644111/

Which, I think, is a much larger change and isn't suitable as a bug-fix 
this late in the release cycle. Instead those patches address other issues 
with the shdma drivers, which might need to be carefully studied and 
solutions have to be found, which, however, would require more work and 
need a longer time. What is your opinion on this? Would you agree to apply 
v2 of this patch, as referenced above or would you prefer a different 
solution?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Vinod Koul]

On Wed, Jun 19, 2013 at 09:32:18PM +0200, Guennadi Liakhovetski wrote:
> Hi Vinod
> 
> On Wed, 19 Jun 2013, Kevin Hilman wrote:
> 
> > On Thu, May 30, 2013 at 7:44 PM, Simon Horman <horms@verge.net.au> wrote:
> > 
> > [...]
> > 
> > > thanks for this. I will wait for a refresh (as we discussed earlier today).
> > > Can I confirm that this is a fix for v3.10? If so, could ou note
> > > that when you post your revised patch?
> > 
> > Any progress on this patch?
> > 
> > The SH-mobile defconfigs are still all failing in linux-next.
> 
> In
> 
> https://patchwork.kernel.org/patch/2640061/
And you havent CC maintainers on this patch, so I dont have it!
> 
> I proposed a simple immediate fix for this problem. Arnd at the same time 
> developed an alternative solution:
> 
> https://patchwork.kernel.org/patch/2644121/
> https://patchwork.kernel.org/patch/2644111/
Reading these patches I agree with Arnd that client drivers should not depend on
dma slave drivers. Existing issue need to be fixed

Arnd, Have you merged these changes?

--
~Vinod
> 
> Which, I think, is a much larger change and isn't suitable as a bug-fix 
> this late in the release cycle. Instead those patches address other issues 
> with the shdma drivers, which might need to be carefully studied and 
> solutions have to be found, which, however, would require more work and 
> need a longer time. What is your opinion on this? Would you agree to apply 
> v2 of this patch, as referenced above or would you prefer a different 
> solution?
> 
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/

-- 

Re: [PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Olof Johansson]

On Thu, Jul 4, 2013 at 9:53 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Wed, Jun 19, 2013 at 09:32:18PM +0200, Guennadi Liakhovetski wrote:
>> Hi Vinod
>>
>> On Wed, 19 Jun 2013, Kevin Hilman wrote:
>>
>> > On Thu, May 30, 2013 at 7:44 PM, Simon Horman <horms@verge.net.au> wrote:
>> >
>> > [...]
>> >
>> > > thanks for this. I will wait for a refresh (as we discussed earlier today).
>> > > Can I confirm that this is a fix for v3.10? If so, could ou note
>> > > that when you post your revised patch?
>> >
>> > Any progress on this patch?
>> >
>> > The SH-mobile defconfigs are still all failing in linux-next.
>>
>> In
>>
>> https://patchwork.kernel.org/patch/2640061/
> And you havent CC maintainers on this patch, so I dont have it!
>>
>> I proposed a simple immediate fix for this problem. Arnd at the same time
>> developed an alternative solution:
>>
>> https://patchwork.kernel.org/patch/2644121/
>> https://patchwork.kernel.org/patch/2644111/
> Reading these patches I agree with Arnd that client drivers should not depend on
> dma slave drivers. Existing issue need to be fixed
>
> Arnd, Have you merged these changes?

Looks like we now have breakage in linux-next for this again (new
breakage due to the header file move being applied by you on Friday,
Vinod? Are you planning on sending the code in for 3.11? If not, you
shouldn't apply patches right now).

bockw_defconfig, kota2_defconfig, marzen_defconfig and others are
currently broken.

-Olof

Re: [PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Vinod Koul]

On Mon, Jul 08, 2013 at 12:52:16AM -0700, Olof Johansson wrote:
> On Thu, Jul 4, 2013 at 9:53 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> > On Wed, Jun 19, 2013 at 09:32:18PM +0200, Guennadi Liakhovetski wrote:
> >> Hi Vinod
> >>
> >> On Wed, 19 Jun 2013, Kevin Hilman wrote:
> >>
> >> > On Thu, May 30, 2013 at 7:44 PM, Simon Horman <horms@verge.net.au> wrote:
> >> >
> >> > [...]
> >> >
> >> > > thanks for this. I will wait for a refresh (as we discussed earlier today).
> >> > > Can I confirm that this is a fix for v3.10? If so, could ou note
> >> > > that when you post your revised patch?
> >> >
> >> > Any progress on this patch?
> >> >
> >> > The SH-mobile defconfigs are still all failing in linux-next.
> >>
> >> In
> >>
> >> https://patchwork.kernel.org/patch/2640061/
> > And you havent CC maintainers on this patch, so I dont have it!
> >>
> >> I proposed a simple immediate fix for this problem. Arnd at the same time
> >> developed an alternative solution:
> >>
> >> https://patchwork.kernel.org/patch/2644121/
> >> https://patchwork.kernel.org/patch/2644111/
> > Reading these patches I agree with Arnd that client drivers should not depend on
> > dma slave drivers. Existing issue need to be fixed
> >
> > Arnd, Have you merged these changes?
> 
> Looks like we now have breakage in linux-next for this again (new
> breakage due to the header file move being applied by you on Friday,
> Vinod? Are you planning on sending the code in for 3.11? If not, you
> shouldn't apply patches right now).
Well they were already in my tree since 18th June. (tree was rebased to fixup for
pull on friday) and now these are in Linus's tree.

Can we have a fix for breakage for now sent to linus for now and then address the
proper way to do this?

~Vinod

Re: [PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Simon Horman]

On Mon, Jul 08, 2013 at 12:51:14PM +0530, Vinod Koul wrote:
> On Mon, Jul 08, 2013 at 12:52:16AM -0700, Olof Johansson wrote:
> > On Thu, Jul 4, 2013 at 9:53 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> > > On Wed, Jun 19, 2013 at 09:32:18PM +0200, Guennadi Liakhovetski wrote:
> > >> Hi Vinod
> > >>
> > >> On Wed, 19 Jun 2013, Kevin Hilman wrote:
> > >>
> > >> > On Thu, May 30, 2013 at 7:44 PM, Simon Horman <horms@verge.net.au> wrote:
> > >> >
> > >> > [...]
> > >> >
> > >> > > thanks for this. I will wait for a refresh (as we discussed earlier today).
> > >> > > Can I confirm that this is a fix for v3.10? If so, could ou note
> > >> > > that when you post your revised patch?
> > >> >
> > >> > Any progress on this patch?
> > >> >
> > >> > The SH-mobile defconfigs are still all failing in linux-next.
> > >>
> > >> In
> > >>
> > >> https://patchwork.kernel.org/patch/2640061/
> > > And you havent CC maintainers on this patch, so I dont have it!
> > >>
> > >> I proposed a simple immediate fix for this problem. Arnd at the same time
> > >> developed an alternative solution:
> > >>
> > >> https://patchwork.kernel.org/patch/2644121/
> > >> https://patchwork.kernel.org/patch/2644111/
> > > Reading these patches I agree with Arnd that client drivers should not depend on
> > > dma slave drivers. Existing issue need to be fixed
> > >
> > > Arnd, Have you merged these changes?
> > 
> > Looks like we now have breakage in linux-next for this again (new
> > breakage due to the header file move being applied by you on Friday,
> > Vinod? Are you planning on sending the code in for 3.11? If not, you
> > shouldn't apply patches right now).
> Well they were already in my tree since 18th June. (tree was rebased to fixup for
> pull on friday) and now these are in Linus's tree.
> 
> Can we have a fix for breakage for now sent to linus for now and then address the
> proper way to do this?

That is my preferred approach.

I will repost the patch at the link
https://patchwork.kernel.org/patch/2640061/ above, rebased for the header
file rename.

Vinod, can you take things from there?

Re: [PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Guennadi Liakhovetski]

On Fri, 5 Jul 2013, Vinod Koul wrote:

> On Wed, Jun 19, 2013 at 09:32:18PM +0200, Guennadi Liakhovetski wrote:
> > Hi Vinod
> > 
> > On Wed, 19 Jun 2013, Kevin Hilman wrote:
> > 
> > > On Thu, May 30, 2013 at 7:44 PM, Simon Horman <horms@verge.net.au> wrote:
> > > 
> > > [...]
> > > 
> > > > thanks for this. I will wait for a refresh (as we discussed earlier today).
> > > > Can I confirm that this is a fix for v3.10? If so, could ou note
> > > > that when you post your revised patch?
> > > 
> > > Any progress on this patch?
> > > 
> > > The SH-mobile defconfigs are still all failing in linux-next.
> > 
> > In
> > 
> > https://patchwork.kernel.org/patch/2640061/
> And you havent CC maintainers on this patch, so I dont have it!

Ouch, sorry, no idea how I managed not to CC you, my apologies!

> > I proposed a simple immediate fix for this problem. Arnd at the same time 
> > developed an alternative solution:
> > 
> > https://patchwork.kernel.org/patch/2644121/
> > https://patchwork.kernel.org/patch/2644111/
> Reading these patches I agree with Arnd that client drivers should not depend on
> dma slave drivers. Existing issue need to be fixed

I'm not arguing against that. My point is the following: also Arnd's 
patches have to deal with the fact, that the filter function has to be in 
files, which will be compiled with SHDMA enabled or disabled in .config. 
He solves this problem by adding #ifdef's to each .c file. Whereas I think 
it's better to add just one such #ifdef to the header. The actual issue of 
DMA client driver dependency on a specific dmaengine controller driver is 
a different (though related) problem and should be addressed separately.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Simon Horman]

From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>

On platforms with no support for the shdma dmaengine driver build is
currently failing with

drivers/built-in.o: In function `sh_mobile_sdhi_probe':
drivers/mmc/host/sh_mobile_sdhi.c:170: undefined reference to`shdma_chan_filter'

Fix the breakage by defining shdma_chan_filter to NULL in such
configurations.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
[horms+renesas@verge.net.au: Apply change to shdma-base.h instead of sh_dma.h]
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
 include/linux/shdma-base.h | 4 ++++
 1 file changed, 4 insertions(+)

Hi Vinod,

please consider this fix from Guennadi for v3.11 which I have rebased on top of
next-20130709. It fixes a build problem on a number of shmobile defconfigs
including bockw.

diff --git a/include/linux/shdma-base.h b/include/linux/shdma-base.h
index 382cf71..5b1c984 100644
--- a/include/linux/shdma-base.h
+++ b/include/linux/shdma-base.h
@@ -124,6 +124,10 @@ void shdma_chan_remove(struct shdma_chan *schan);
 int shdma_init(struct device *dev, struct shdma_dev *sdev,
 		    int chan_num);
 void shdma_cleanup(struct shdma_dev *sdev);
+#if IS_ENABLED(CONFIG_SH_DMAE_BASE)
 bool shdma_chan_filter(struct dma_chan *chan, void *arg);
+#else
+#define shdma_chan_filter NULL
+#endif
 
 #endif
-- 
1.8.2.1

Re: [PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Simon Horman]

On Wed, Jul 10, 2013 at 11:09:12AM +0900, Simon Horman wrote:
> From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> 
> On platforms with no support for the shdma dmaengine driver build is
> currently failing with
> 
> drivers/built-in.o: In function `sh_mobile_sdhi_probe':
> drivers/mmc/host/sh_mobile_sdhi.c:170: undefined reference to`shdma_chan_filter'
> 
> Fix the breakage by defining shdma_chan_filter to NULL in such
> configurations.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> [horms+renesas@verge.net.au: Apply change to shdma-base.h instead of sh_dma.h]
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
>  include/linux/shdma-base.h | 4 ++++
>  1 file changed, 4 insertions(+)

> 
> Hi Vinod,
> 
> please consider this fix from Guennadi for v3.11 which I have rebased on top of
> next-20130709. It fixes a build problem on a number of shmobile defconfigs
> including bockw.

Ping.

Would it be appropriate for me to take this change through my tree?
It seems to resolve a regression in v3.11-rc1.

> diff --git a/include/linux/shdma-base.h b/include/linux/shdma-base.h
> index 382cf71..5b1c984 100644
> --- a/include/linux/shdma-base.h
> +++ b/include/linux/shdma-base.h
> @@ -124,6 +124,10 @@ void shdma_chan_remove(struct shdma_chan *schan);
>  int shdma_init(struct device *dev, struct shdma_dev *sdev,
>  		    int chan_num);
>  void shdma_cleanup(struct shdma_dev *sdev);
> +#if IS_ENABLED(CONFIG_SH_DMAE_BASE)
>  bool shdma_chan_filter(struct dma_chan *chan, void *arg);
> +#else
> +#define shdma_chan_filter NULL
> +#endif
>  
>  #endif
> -- 
> 1.8.2.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Olof Johansson]

On Tue, Jul 16, 2013 at 10:20:41AM +0900, Simon Horman wrote:
> On Wed, Jul 10, 2013 at 11:09:12AM +0900, Simon Horman wrote:
> > From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > 
> > On platforms with no support for the shdma dmaengine driver build is
> > currently failing with
> > 
> > drivers/built-in.o: In function `sh_mobile_sdhi_probe':
> > drivers/mmc/host/sh_mobile_sdhi.c:170: undefined reference to`shdma_chan_filter'
> > 
> > Fix the breakage by defining shdma_chan_filter to NULL in such
> > configurations.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > [horms+renesas@verge.net.au: Apply change to shdma-base.h instead of sh_dma.h]
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > ---
> >  include/linux/shdma-base.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> 
> > 
> > Hi Vinod,
> > 
> > please consider this fix from Guennadi for v3.11 which I have rebased on top of
> > next-20130709. It fixes a build problem on a number of shmobile defconfigs
> > including bockw.
> 
> Ping.
> 
> Would it be appropriate for me to take this change through my tree?
> It seems to resolve a regression in v3.11-rc1.

I'll take it through arm-soc since there's no response from Vinod. I've
applied it in the fixes branch we have.


-Olof

Re: [PATCH] dmaengine: shdma: fix a build failure on platforms with no DMA support

[Simon Horman]

On Mon, Jul 22, 2013 at 07:39:07PM -0700, Olof Johansson wrote:
> On Tue, Jul 16, 2013 at 10:20:41AM +0900, Simon Horman wrote:
> > On Wed, Jul 10, 2013 at 11:09:12AM +0900, Simon Horman wrote:
> > > From: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > 
> > > On platforms with no support for the shdma dmaengine driver build is
> > > currently failing with
> > > 
> > > drivers/built-in.o: In function `sh_mobile_sdhi_probe':
> > > drivers/mmc/host/sh_mobile_sdhi.c:170: undefined reference to`shdma_chan_filter'
> > > 
> > > Fix the breakage by defining shdma_chan_filter to NULL in such
> > > configurations.
> > > 
> > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> > > [horms+renesas@verge.net.au: Apply change to shdma-base.h instead of sh_dma.h]
> > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> > > ---
> > >  include/linux/shdma-base.h | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > 
> > > 
> > > Hi Vinod,
> > > 
> > > please consider this fix from Guennadi for v3.11 which I have rebased on top of
> > > next-20130709. It fixes a build problem on a number of shmobile defconfigs
> > > including bockw.
> > 
> > Ping.
> > 
> > Would it be appropriate for me to take this change through my tree?
> > It seems to resolve a regression in v3.11-rc1.
> 
> I'll take it through arm-soc since there's no response from Vinod. I've
> applied it in the fixes branch we have.

Thanks!

Very much appreciated.

[PATCH resend] shdma: fixup sh_dmae_get_partial() calculation error

[Kuninori Morimoto]

sh_desc->hw.tcr is controlling real data size,
and, register TCR is controlling data transfer count
which was xmit_shifted value of hw.tcr.
Current sh_dmae_get_partial() is calculating in different unit.
This patch fixes it.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
>> Guennadi

Could you please check this patch, and give it your Acked-by ?

 drivers/dma/sh/shdma.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
index b70709b..d670b8b 100644
--- a/drivers/dma/sh/shdma.c
+++ b/drivers/dma/sh/shdma.c
@@ -388,8 +388,8 @@ static size_t sh_dmae_get_partial(struct shdma_chan *schan,
 						    shdma_chan);
 	struct sh_dmae_desc *sh_desc = container_of(sdesc,
 					struct sh_dmae_desc, shdma_desc);
-	return (sh_desc->hw.tcr - sh_dmae_readl(sh_chan, TCR)) <<
-		sh_chan->xmit_shift;
+	return sh_desc->hw.tcr -
+		(sh_dmae_readl(sh_chan, TCR) << sh_chan->xmit_shift);
 }
 
 /* Called from error IRQ or NMI */
-- 
1.7.9.5

Re: [PATCH resend] shdma: fixup sh_dmae_get_partial() calculation error

[Guennadi Liakhovetski]

Hi Morimoto-san

On Tue, 23 Jul 2013, Kuninori Morimoto wrote:

> sh_desc->hw.tcr is controlling real data size,
> and, register TCR is controlling data transfer count
> which was xmit_shifted value of hw.tcr.
> Current sh_dmae_get_partial() is calculating in different unit.
> This patch fixes it.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Looks right to me

Acked-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Thanks
Guennadi

> ---
> >> Guennadi
> 
> Could you please check this patch, and give it your Acked-by ?
> 
>  drivers/dma/sh/shdma.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
> index b70709b..d670b8b 100644
> --- a/drivers/dma/sh/shdma.c
> +++ b/drivers/dma/sh/shdma.c
> @@ -388,8 +388,8 @@ static size_t sh_dmae_get_partial(struct shdma_chan *schan,
>  						    shdma_chan);
>  	struct sh_dmae_desc *sh_desc = container_of(sdesc,
>  					struct sh_dmae_desc, shdma_desc);
> -	return (sh_desc->hw.tcr - sh_dmae_readl(sh_chan, TCR)) <<
> -		sh_chan->xmit_shift;
> +	return sh_desc->hw.tcr -
> +		(sh_dmae_readl(sh_chan, TCR) << sh_chan->xmit_shift);
>  }
>  
>  /* Called from error IRQ or NMI */
> -- 
> 1.7.9.5
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH resend] shdma: fixup sh_dmae_get_partial() calculation error

[Simon Horman]

On Wed, Jul 24, 2013 at 10:43:44AM +0200, Guennadi Liakhovetski wrote:
> Hi Morimoto-san
> 
> On Tue, 23 Jul 2013, Kuninori Morimoto wrote:
> 
> > sh_desc->hw.tcr is controlling real data size,
> > and, register TCR is controlling data transfer count
> > which was xmit_shifted value of hw.tcr.
> > Current sh_dmae_get_partial() is calculating in different unit.
> > This patch fixes it.
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Looks right to me
> 
> Acked-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Thanks, I will queue this up for v3.11 in the fixes-for-v3.11 branch.

Re: [PATCH resend] shdma: fixup sh_dmae_get_partial() calculation error

[Simon Horman]

[ Cc Vinod ]

On Tue, Jul 23, 2013 at 11:12:41PM -0700, Kuninori Morimoto wrote:
> sh_desc->hw.tcr is controlling real data size,
> and, register TCR is controlling data transfer count
> which was xmit_shifted value of hw.tcr.
> Current sh_dmae_get_partial() is calculating in different unit.
> This patch fixes it.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Can I confirm that this was a regression introduced by
4f46f8ac80416b0e8fd3aba6a0d842205fb29140                   
("dmaengine: shdma: restore partial transfer calculation") ?

> ---
> >> Guennadi
> 
> Could you please check this patch, and give it your Acked-by ?
> 
>  drivers/dma/sh/shdma.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
> index b70709b..d670b8b 100644
> --- a/drivers/dma/sh/shdma.c
> +++ b/drivers/dma/sh/shdma.c
> @@ -388,8 +388,8 @@ static size_t sh_dmae_get_partial(struct shdma_chan *schan,
>  						    shdma_chan);
>  	struct sh_dmae_desc *sh_desc = container_of(sdesc,
>  					struct sh_dmae_desc, shdma_desc);
> -	return (sh_desc->hw.tcr - sh_dmae_readl(sh_chan, TCR)) <<
> -		sh_chan->xmit_shift;
> +	return sh_desc->hw.tcr -
> +		(sh_dmae_readl(sh_chan, TCR) << sh_chan->xmit_shift);
>  }
>  
>  /* Called from error IRQ or NMI */
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH resend] shdma: fixup sh_dmae_get_partial() calculation error

[Guennadi Liakhovetski]

Hi Simon

On Tue, 30 Jul 2013, Simon Horman wrote:

> [ Cc Vinod ]
> 
> On Tue, Jul 23, 2013 at 11:12:41PM -0700, Kuninori Morimoto wrote:
> > sh_desc->hw.tcr is controlling real data size,
> > and, register TCR is controlling data transfer count
> > which was xmit_shifted value of hw.tcr.
> > Current sh_dmae_get_partial() is calculating in different unit.
> > This patch fixes it.
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Can I confirm that this was a regression introduced by
> 4f46f8ac80416b0e8fd3aba6a0d842205fb29140                   
> ("dmaengine: shdma: restore partial transfer calculation") ?

No, don't think so. That patch only restores what was accidentally deleted 
from the driver before. So, you cannot say, that before that patch the 
calculation was correct. And even the way it restores it was already wrong 
before. The error goes back to the original implementation:

commit c014906a870ce70e009def0c9d170ccabeb0be63
Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Date:   Thu Feb 18 16:30:02 2010 +0000

    dmaengine: shdma: extend .device_terminate_all() to record partial transfer

Thanks
Guennadi

> 
> > ---
> > >> Guennadi
> > 
> > Could you please check this patch, and give it your Acked-by ?
> > 
> >  drivers/dma/sh/shdma.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
> > index b70709b..d670b8b 100644
> > --- a/drivers/dma/sh/shdma.c
> > +++ b/drivers/dma/sh/shdma.c
> > @@ -388,8 +388,8 @@ static size_t sh_dmae_get_partial(struct shdma_chan *schan,
> >  						    shdma_chan);
> >  	struct sh_dmae_desc *sh_desc = container_of(sdesc,
> >  					struct sh_dmae_desc, shdma_desc);
> > -	return (sh_desc->hw.tcr - sh_dmae_readl(sh_chan, TCR)) <<
> > -		sh_chan->xmit_shift;
> > +	return sh_desc->hw.tcr -
> > +		(sh_dmae_readl(sh_chan, TCR) << sh_chan->xmit_shift);
> >  }
> >  
> >  /* Called from error IRQ or NMI */
> > -- 
> > 1.7.9.5
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH resend] shdma: fixup sh_dmae_get_partial() calculation error

[Simon Horman]

On Tue, Jul 30, 2013 at 11:22:31AM +0200, Guennadi Liakhovetski wrote:
> Hi Simon
> 
> On Tue, 30 Jul 2013, Simon Horman wrote:
> 
> > [ Cc Vinod ]
> > 
> > On Tue, Jul 23, 2013 at 11:12:41PM -0700, Kuninori Morimoto wrote:
> > > sh_desc->hw.tcr is controlling real data size,
> > > and, register TCR is controlling data transfer count
> > > which was xmit_shifted value of hw.tcr.
> > > Current sh_dmae_get_partial() is calculating in different unit.
> > > This patch fixes it.
> > > 
> > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > Can I confirm that this was a regression introduced by
> > 4f46f8ac80416b0e8fd3aba6a0d842205fb29140                   
> > ("dmaengine: shdma: restore partial transfer calculation") ?
> 
> No, don't think so. That patch only restores what was accidentally deleted 
> from the driver before. So, you cannot say, that before that patch the 
> calculation was correct. And even the way it restores it was already wrong 
> before. The error goes back to the original implementation:
> 
> commit c014906a870ce70e009def0c9d170ccabeb0be63
> Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Date:   Thu Feb 18 16:30:02 2010 +0000
> 
>     dmaengine: shdma: extend .device_terminate_all() to record partial transfer

Thanks, I will add the following to the changelog:

This bug has been present since c014906a870ce70e009def0c9d170ccabeb0be63
("dmaengine: shdma: extend .device_terminate_all() to record partial
transfer"), which was added in 2.6.34-rc1.