* [PATCH] pch_dma: Fix suspend issue
@ 2011-10-04 7:27 Tomoya MORINAGA
2011-10-07 5:00 ` Vinod Koul
0 siblings, 1 reply; 6+ messages in thread
From: Tomoya MORINAGA @ 2011-10-04 7:27 UTC (permalink / raw)
To: Dan Williams, Vinod Koul, linux-kernel
Cc: qi.wang, yong.y.wang, joel.clark, kok.howg.ewe, Tomoya MORINAGA
Currently, executing suspend/hibernation,
memory access violation occurs.
This patch fixes the issue
- Modify array size (MAX_CHAN_NR)
Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.lapis-semi.com>
---
drivers/dma/pch_dma.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/dma/pch_dma.c b/drivers/dma/pch_dma.c
index 1ac8d4b..42bcc80 100644
--- a/drivers/dma/pch_dma.c
+++ b/drivers/dma/pch_dma.c
@@ -60,7 +60,7 @@
#define DMA_DESC_FOLLOW_WITHOUT_IRQ 0x2
#define DMA_DESC_FOLLOW_WITH_IRQ 0x3
-#define MAX_CHAN_NR 8
+#define MAX_CHAN_NR 12
#define DMA_MASK_CTL0_MODE 0x33333333
#define DMA_MASK_CTL2_MODE 0x00003333
--
1.7.4.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pch_dma: Fix suspend issue
2011-10-04 7:27 [PATCH] pch_dma: Fix suspend issue Tomoya MORINAGA
@ 2011-10-07 5:00 ` Vinod Koul
2011-10-07 5:54 ` Tomoya MORINAGA
0 siblings, 1 reply; 6+ messages in thread
From: Vinod Koul @ 2011-10-07 5:00 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: Dan Williams, linux-kernel, qi.wang, yong.y.wang, joel.clark,
kok.howg.ewe
On Tue, 2011-10-04 at 16:27 +0900, Tomoya MORINAGA wrote:
> Currently, executing suspend/hibernation,
> memory access violation occurs.
>
> This patch fixes the issue
> - Modify array size (MAX_CHAN_NR)
>
> Signed-off-by: Tomoya MORINAGA <tomoya-linux@dsn.lapis-semi.com>
> ---
> drivers/dma/pch_dma.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/dma/pch_dma.c b/drivers/dma/pch_dma.c
> index 1ac8d4b..42bcc80 100644
> --- a/drivers/dma/pch_dma.c
> +++ b/drivers/dma/pch_dma.c
> @@ -60,7 +60,7 @@
> #define DMA_DESC_FOLLOW_WITHOUT_IRQ 0x2
> #define DMA_DESC_FOLLOW_WITH_IRQ 0x3
>
> -#define MAX_CHAN_NR 8
> +#define MAX_CHAN_NR 12
>
> #define DMA_MASK_CTL0_MODE 0x33333333
> #define DMA_MASK_CTL2_MODE 0x00003333
How would changing the max number of descriptors fix the memory
violation?
--
~Vinod
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pch_dma: Fix suspend issue
2011-10-07 5:00 ` Vinod Koul
@ 2011-10-07 5:54 ` Tomoya MORINAGA
2011-10-07 6:14 ` Vinod Koul
0 siblings, 1 reply; 6+ messages in thread
From: Tomoya MORINAGA @ 2011-10-07 5:54 UTC (permalink / raw)
To: Vinod Koul
Cc: Dan Williams, linux-kernel, qi.wang, yong.y.wang, joel.clark,
kok.howg.ewe
(2011/10/07 14:00), Vinod Koul wrote:
> How would changing the max number of descriptors fix the memory
> violation?
>
In probe of pch_dma, you can see the following code.
for (i = 0; i < nr_channels; i++) {
snip...
pd_chan->membase = ®s->desc[i];
snip...
}
Max value of nr_channels is 12 defined at pci_table.
So, this caused memory access violation.
--
tomoya
ROHM Co., Ltd.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] pch_dma: Fix suspend issue
2011-10-07 5:54 ` Tomoya MORINAGA
@ 2011-10-07 6:14 ` Vinod Koul
2011-10-07 10:33 ` Tomoya MORINAGA
0 siblings, 1 reply; 6+ messages in thread
From: Vinod Koul @ 2011-10-07 6:14 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: Dan Williams, linux-kernel, qi.wang, yong.y.wang, joel.clark,
kok.howg.ewe
On Fri, 2011-10-07 at 14:54 +0900, Tomoya MORINAGA wrote:
> (2011/10/07 14:00), Vinod Koul wrote:
> > How would changing the max number of descriptors fix the memory
> > violation?
> >
>
> In probe of pch_dma, you can see the following code.
>
> for (i = 0; i < nr_channels; i++) {
>
> snip...
>
> pd_chan->membase = ®s->desc[i];
>
> snip...
>
> }
>
> Max value of nr_channels is 12 defined at pci_table.
> So, this caused memory access violation.
This is all is due to not saving nr_channels in probe and using that
instead.
Looking deeper, struct pch_dma defines array of length MAX_CHAN_NR for
channels. Then why do you allocate memory in probe as
pd = kzalloc(sizeof(struct pch_dma)+
sizeof(struct pch_dma_chan) * nr_channels, GFP_KERNEL);
what is the point in allocating additional memory for each channel?
Given this, why should there be predefined channel array in pcm_dma?
It would be great if we could fix this by dynamically allocating memory
for channels based on nr_channels and not wasting due to a static array.
--
~Vinod
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] pch_dma: Fix suspend issue
2011-10-07 6:14 ` Vinod Koul
@ 2011-10-07 10:33 ` Tomoya MORINAGA
2011-10-07 14:18 ` Vinod Koul
0 siblings, 1 reply; 6+ messages in thread
From: Tomoya MORINAGA @ 2011-10-07 10:33 UTC (permalink / raw)
To: Vinod Koul
Cc: Dan Williams, linux-kernel, qi.wang, yong.y.wang, joel.clark,
kok.howg.ewe
(2011/10/07 15:14), Vinod Koul wrote:
> This is all is due to not saving nr_channels in probe and using that
> instead.
>
> Looking deeper, struct pch_dma defines array of length MAX_CHAN_NR for
> channels. Then why do you allocate memory in probe as
> pd = kzalloc(sizeof(struct pch_dma)+
> sizeof(struct pch_dma_chan) * nr_channels, GFP_KERNEL);
> what is the point in allocating additional memory for each channel?
>
> Given this, why should there be predefined channel array in pcm_dma?
>
> It would be great if we could fix this by dynamically allocating memory
> for channels based on nr_channels and not wasting due to a static array.
You are right.
Current pch_dma driver looks like wasting memory.
We must review pch_dma driver again.
Thanks,
--
tomoya
ROHM Co., Ltd.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pch_dma: Fix suspend issue
2011-10-07 10:33 ` Tomoya MORINAGA
@ 2011-10-07 14:18 ` Vinod Koul
0 siblings, 0 replies; 6+ messages in thread
From: Vinod Koul @ 2011-10-07 14:18 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: Dan Williams, linux-kernel, qi.wang, yong.y.wang, joel.clark,
kok.howg.ewe
On Fri, 2011-10-07 at 19:33 +0900, Tomoya MORINAGA wrote:
> (2011/10/07 15:14), Vinod Koul wrote:
> > This is all is due to not saving nr_channels in probe and using that
> > instead.
> >
> > Looking deeper, struct pch_dma defines array of length MAX_CHAN_NR for
> > channels. Then why do you allocate memory in probe as
> > pd = kzalloc(sizeof(struct pch_dma)+
> > sizeof(struct pch_dma_chan) * nr_channels, GFP_KERNEL);
> > what is the point in allocating additional memory for each channel?
> >
> > Given this, why should there be predefined channel array in pcm_dma?
> >
> > It would be great if we could fix this by dynamically allocating memory
> > for channels based on nr_channels and not wasting due to a static array.
>
> You are right.
> Current pch_dma driver looks like wasting memory.
>
> We must review pch_dma driver again.
Okay, can you update the changelog to indicate how it is fixing the
suspend issue, original change log doesn't help
--
~Vinod
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-10-07 14:25 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-04 7:27 [PATCH] pch_dma: Fix suspend issue Tomoya MORINAGA
2011-10-07 5:00 ` Vinod Koul
2011-10-07 5:54 ` Tomoya MORINAGA
2011-10-07 6:14 ` Vinod Koul
2011-10-07 10:33 ` Tomoya MORINAGA
2011-10-07 14:18 ` Vinod Koul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®