| Message ID | 1380188023-3936-4-git-send-email-hongbo.zhang@freescale.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Scott Wood |
| Headers | show |
Hi Vinod, I have gotten ACK from Mark for both the 1/3 and 2/3 patches. Thanks. On 09/26/2013 05:33 PM, hongbo.zhang@freescale.com wrote: > From: Hongbo Zhang <hongbo.zhang@freescale.com> > > This patch adds support to 8-channel DMA engine, thus the driver works for both > the new 8-channel and the legacy 4-channel DMA engines. > > Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com> > --- > drivers/dma/Kconfig | 9 +++++---- > drivers/dma/fsldma.c | 9 ++++++--- > drivers/dma/fsldma.h | 2 +- > 3 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig > index 6825957..3979c65 100644 > --- a/drivers/dma/Kconfig > +++ b/drivers/dma/Kconfig > @@ -89,14 +89,15 @@ config AT_HDMAC > Support the Atmel AHB DMA controller. > > config FSL_DMA > - tristate "Freescale Elo and Elo Plus DMA support" > + tristate "Freescale Elo series DMA support" > depends on FSL_SOC > select DMA_ENGINE > select ASYNC_TX_ENABLE_CHANNEL_SWITCH > ---help--- > - Enable support for the Freescale Elo and Elo Plus DMA controllers. > - The Elo is the DMA controller on some 82xx and 83xx parts, and the > - Elo Plus is the DMA controller on 85xx and 86xx parts. > + Enable support for the Freescale Elo series DMA controllers. > + The Elo is the DMA controller on some mpc82xx and mpc83xx parts, the > + EloPlus is on mpc85xx and mpc86xx and Pxxx parts, and the Elo3 is on > + some Txxx and Bxxx parts. > > config MPC512X_DMA > tristate "Freescale MPC512x built-in DMA engine support" > diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c > index 49e8fbd..16a9a48 100644 > --- a/drivers/dma/fsldma.c > +++ b/drivers/dma/fsldma.c > @@ -1261,7 +1261,9 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev, > WARN_ON(fdev->feature != chan->feature); > > chan->dev = fdev->dev; > - chan->id = ((res.start - 0x100) & 0xfff) >> 7; > + chan->id = (res.start & 0xfff) < 0x300 ? > + ((res.start - 0x100) & 0xfff) >> 7 : > + ((res.start - 0x200) & 0xfff) >> 7; > if (chan->id >= FSL_DMA_MAX_CHANS_PER_DEVICE) { > dev_err(fdev->dev, "too many channels for device\n"); > err = -EINVAL; > @@ -1434,6 +1436,7 @@ static int fsldma_of_remove(struct platform_device *op) > } > > static const struct of_device_id fsldma_of_ids[] = { > + { .compatible = "fsl,elo3-dma", }, > { .compatible = "fsl,eloplus-dma", }, > { .compatible = "fsl,elo-dma", }, > {} > @@ -1455,7 +1458,7 @@ static struct platform_driver fsldma_of_driver = { > > static __init int fsldma_init(void) > { > - pr_info("Freescale Elo / Elo Plus DMA driver\n"); > + pr_info("Freescale Elo series DMA driver\n"); > return platform_driver_register(&fsldma_of_driver); > } > > @@ -1467,5 +1470,5 @@ static void __exit fsldma_exit(void) > subsys_initcall(fsldma_init); > module_exit(fsldma_exit); > > -MODULE_DESCRIPTION("Freescale Elo / Elo Plus DMA driver"); > +MODULE_DESCRIPTION("Freescale Elo series DMA driver"); > MODULE_LICENSE("GPL"); > diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h > index f5c3879..1ffc244 100644 > --- a/drivers/dma/fsldma.h > +++ b/drivers/dma/fsldma.h > @@ -112,7 +112,7 @@ struct fsldma_chan_regs { > }; > > struct fsldma_chan; > -#define FSL_DMA_MAX_CHANS_PER_DEVICE 4 > +#define FSL_DMA_MAX_CHANS_PER_DEVICE 8 > > struct fsldma_device { > void __iomem *regs; /* DGSR register base */
Hi Vinod Koul and Dan Williams, Ping? On 10/17/2013 01:56 PM, Hongbo Zhang wrote: > Hi Vinod, > I have gotten ACK from Mark for both the 1/3 and 2/3 patches. > Thanks. > > > On 09/26/2013 05:33 PM, hongbo.zhang@freescale.com wrote: >> From: Hongbo Zhang <hongbo.zhang@freescale.com> >> >> This patch adds support to 8-channel DMA engine, thus the driver >> works for both >> the new 8-channel and the legacy 4-channel DMA engines. >> >> Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com> >> --- >> drivers/dma/Kconfig | 9 +++++---- >> drivers/dma/fsldma.c | 9 ++++++--- >> drivers/dma/fsldma.h | 2 +- >> 3 files changed, 12 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig >> index 6825957..3979c65 100644 >> --- a/drivers/dma/Kconfig >> +++ b/drivers/dma/Kconfig >> @@ -89,14 +89,15 @@ config AT_HDMAC >> Support the Atmel AHB DMA controller. >> config FSL_DMA >> - tristate "Freescale Elo and Elo Plus DMA support" >> + tristate "Freescale Elo series DMA support" >> depends on FSL_SOC >> select DMA_ENGINE >> select ASYNC_TX_ENABLE_CHANNEL_SWITCH >> ---help--- >> - Enable support for the Freescale Elo and Elo Plus DMA >> controllers. >> - The Elo is the DMA controller on some 82xx and 83xx parts, and >> the >> - Elo Plus is the DMA controller on 85xx and 86xx parts. >> + Enable support for the Freescale Elo series DMA controllers. >> + The Elo is the DMA controller on some mpc82xx and mpc83xx >> parts, the >> + EloPlus is on mpc85xx and mpc86xx and Pxxx parts, and the Elo3 >> is on >> + some Txxx and Bxxx parts. >> config MPC512X_DMA >> tristate "Freescale MPC512x built-in DMA engine support" >> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c >> index 49e8fbd..16a9a48 100644 >> --- a/drivers/dma/fsldma.c >> +++ b/drivers/dma/fsldma.c >> @@ -1261,7 +1261,9 @@ static int fsl_dma_chan_probe(struct >> fsldma_device *fdev, >> WARN_ON(fdev->feature != chan->feature); >> chan->dev = fdev->dev; >> - chan->id = ((res.start - 0x100) & 0xfff) >> 7; >> + chan->id = (res.start & 0xfff) < 0x300 ? >> + ((res.start - 0x100) & 0xfff) >> 7 : >> + ((res.start - 0x200) & 0xfff) >> 7; >> if (chan->id >= FSL_DMA_MAX_CHANS_PER_DEVICE) { >> dev_err(fdev->dev, "too many channels for device\n"); >> err = -EINVAL; >> @@ -1434,6 +1436,7 @@ static int fsldma_of_remove(struct >> platform_device *op) >> } >> static const struct of_device_id fsldma_of_ids[] = { >> + { .compatible = "fsl,elo3-dma", }, >> { .compatible = "fsl,eloplus-dma", }, >> { .compatible = "fsl,elo-dma", }, >> {} >> @@ -1455,7 +1458,7 @@ static struct platform_driver fsldma_of_driver = { >> static __init int fsldma_init(void) >> { >> - pr_info("Freescale Elo / Elo Plus DMA driver\n"); >> + pr_info("Freescale Elo series DMA driver\n"); >> return platform_driver_register(&fsldma_of_driver); >> } >> @@ -1467,5 +1470,5 @@ static void __exit fsldma_exit(void) >> subsys_initcall(fsldma_init); >> module_exit(fsldma_exit); >> -MODULE_DESCRIPTION("Freescale Elo / Elo Plus DMA driver"); >> +MODULE_DESCRIPTION("Freescale Elo series DMA driver"); >> MODULE_LICENSE("GPL"); >> diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h >> index f5c3879..1ffc244 100644 >> --- a/drivers/dma/fsldma.h >> +++ b/drivers/dma/fsldma.h >> @@ -112,7 +112,7 @@ struct fsldma_chan_regs { >> }; >> struct fsldma_chan; >> -#define FSL_DMA_MAX_CHANS_PER_DEVICE 4 >> +#define FSL_DMA_MAX_CHANS_PER_DEVICE 8 >> struct fsldma_device { >> void __iomem *regs; /* DGSR register base */ > >
On Mon, Nov 4, 2013 at 6:31 PM, Hongbo Zhang <hongbo.zhang@freescale.com> wrote: > Hi Vinod Koul and Dan Williams, > Ping? > Not much to review from the dmaengine side, just one question below. It would be helpful if you can send these to the new dmaengine patchwork at dmaengine@vger.kernel.org with the Acks you have already collected. > > > On 10/17/2013 01:56 PM, Hongbo Zhang wrote: >> >> Hi Vinod, >> I have gotten ACK from Mark for both the 1/3 and 2/3 patches. >> Thanks. >> >> >> On 09/26/2013 05:33 PM, hongbo.zhang@freescale.com wrote: >>> >>> From: Hongbo Zhang <hongbo.zhang@freescale.com> >>> >>> This patch adds support to 8-channel DMA engine, thus the driver works >>> for both >>> the new 8-channel and the legacy 4-channel DMA engines. >>> >>> Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com> >>> --- >>> drivers/dma/Kconfig | 9 +++++---- >>> drivers/dma/fsldma.c | 9 ++++++--- >>> drivers/dma/fsldma.h | 2 +- >>> 3 files changed, 12 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig >>> index 6825957..3979c65 100644 >>> --- a/drivers/dma/Kconfig >>> +++ b/drivers/dma/Kconfig >>> @@ -89,14 +89,15 @@ config AT_HDMAC >>> Support the Atmel AHB DMA controller. >>> config FSL_DMA >>> - tristate "Freescale Elo and Elo Plus DMA support" >>> + tristate "Freescale Elo series DMA support" >>> depends on FSL_SOC >>> select DMA_ENGINE >>> select ASYNC_TX_ENABLE_CHANNEL_SWITCH >>> ---help--- >>> - Enable support for the Freescale Elo and Elo Plus DMA controllers. >>> - The Elo is the DMA controller on some 82xx and 83xx parts, and the >>> - Elo Plus is the DMA controller on 85xx and 86xx parts. >>> + Enable support for the Freescale Elo series DMA controllers. >>> + The Elo is the DMA controller on some mpc82xx and mpc83xx parts, >>> the >>> + EloPlus is on mpc85xx and mpc86xx and Pxxx parts, and the Elo3 is >>> on >>> + some Txxx and Bxxx parts. >>> config MPC512X_DMA >>> tristate "Freescale MPC512x built-in DMA engine support" >>> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c >>> index 49e8fbd..16a9a48 100644 >>> --- a/drivers/dma/fsldma.c >>> +++ b/drivers/dma/fsldma.c >>> @@ -1261,7 +1261,9 @@ static int fsl_dma_chan_probe(struct fsldma_device >>> *fdev, >>> WARN_ON(fdev->feature != chan->feature); >>> chan->dev = fdev->dev; >>> - chan->id = ((res.start - 0x100) & 0xfff) >> 7; >>> + chan->id = (res.start & 0xfff) < 0x300 ? >>> + ((res.start - 0x100) & 0xfff) >> 7 : >>> + ((res.start - 0x200) & 0xfff) >> 7; >>> if (chan->id >= FSL_DMA_MAX_CHANS_PER_DEVICE) { Isn't it a bit fragile to have this based on the resource address? Can't device tree tell you the channel id directly by an index into the "dma0: dma@100300" node? -- Dan
On 11/08/2013 10:45 AM, Dan Williams wrote: > On Mon, Nov 4, 2013 at 6:31 PM, Hongbo Zhang <hongbo.zhang@freescale.com> wrote: >> Hi Vinod Koul and Dan Williams, >> Ping? >> > Not much to review from the dmaengine side, just one question below. > It would be helpful if you can send these to the new dmaengine > patchwork at dmaengine@vger.kernel.org with the Acks you have already > collected. > Sorry didn't notice this new mailing list. I will resend these patches to it again. >> >> On 10/17/2013 01:56 PM, Hongbo Zhang wrote: >>> Hi Vinod, >>> I have gotten ACK from Mark for both the 1/3 and 2/3 patches. >>> Thanks. >>> >>> >>> On 09/26/2013 05:33 PM, hongbo.zhang@freescale.com wrote: >>>> From: Hongbo Zhang <hongbo.zhang@freescale.com> >>>> >>>> This patch adds support to 8-channel DMA engine, thus the driver works >>>> for both >>>> the new 8-channel and the legacy 4-channel DMA engines. >>>> >>>> Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com> >>>> --- >>>> drivers/dma/Kconfig | 9 +++++---- >>>> drivers/dma/fsldma.c | 9 ++++++--- >>>> drivers/dma/fsldma.h | 2 +- >>>> 3 files changed, 12 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig >>>> index 6825957..3979c65 100644 >>>> --- a/drivers/dma/Kconfig >>>> +++ b/drivers/dma/Kconfig >>>> @@ -89,14 +89,15 @@ config AT_HDMAC >>>> Support the Atmel AHB DMA controller. >>>> config FSL_DMA >>>> - tristate "Freescale Elo and Elo Plus DMA support" >>>> + tristate "Freescale Elo series DMA support" >>>> depends on FSL_SOC >>>> select DMA_ENGINE >>>> select ASYNC_TX_ENABLE_CHANNEL_SWITCH >>>> ---help--- >>>> - Enable support for the Freescale Elo and Elo Plus DMA controllers. >>>> - The Elo is the DMA controller on some 82xx and 83xx parts, and the >>>> - Elo Plus is the DMA controller on 85xx and 86xx parts. >>>> + Enable support for the Freescale Elo series DMA controllers. >>>> + The Elo is the DMA controller on some mpc82xx and mpc83xx parts, >>>> the >>>> + EloPlus is on mpc85xx and mpc86xx and Pxxx parts, and the Elo3 is >>>> on >>>> + some Txxx and Bxxx parts. >>>> config MPC512X_DMA >>>> tristate "Freescale MPC512x built-in DMA engine support" >>>> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c >>>> index 49e8fbd..16a9a48 100644 >>>> --- a/drivers/dma/fsldma.c >>>> +++ b/drivers/dma/fsldma.c >>>> @@ -1261,7 +1261,9 @@ static int fsl_dma_chan_probe(struct fsldma_device >>>> *fdev, >>>> WARN_ON(fdev->feature != chan->feature); >>>> chan->dev = fdev->dev; >>>> - chan->id = ((res.start - 0x100) & 0xfff) >> 7; >>>> + chan->id = (res.start & 0xfff) < 0x300 ? >>>> + ((res.start - 0x100) & 0xfff) >> 7 : >>>> + ((res.start - 0x200) & 0xfff) >> 7; >>>> if (chan->id >= FSL_DMA_MAX_CHANS_PER_DEVICE) { > Isn't it a bit fragile to have this based on the resource address? > Can't device tree tell you the channel id directly by an index into > the "dma0: dma@100300" node? Yes, both this way and putting a "cell-index" into device tree work. This won't be fragile, because the resource address should always be defined correctly, otherwise even if we can tell a channel id by "cell-index" but with wrong resource address, nothing will work. This piece of code only doesn't seem as neat as using "cell-index", but we prefer the style that let the device tree describes as true as what hardware really has. This doesn't mean "cell-index" isn't acceptable, if it is necessary and unavoidable, we can send another patch to add it, but currently there is no need and we don't have to do this. > -- > Dan >
On Mon, Nov 11, 2013 at 1:12 AM, Hongbo Zhang <hongbo.zhang@freescale.com> wrote: >>>>> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c >>>>> index 49e8fbd..16a9a48 100644 >>>>> --- a/drivers/dma/fsldma.c >>>>> +++ b/drivers/dma/fsldma.c >>>>> @@ -1261,7 +1261,9 @@ static int fsl_dma_chan_probe(struct >>>>> fsldma_device >>>>> *fdev, >>>>> WARN_ON(fdev->feature != chan->feature); >>>>> chan->dev = fdev->dev; >>>>> - chan->id = ((res.start - 0x100) & 0xfff) >> 7; >>>>> + chan->id = (res.start & 0xfff) < 0x300 ? >>>>> + ((res.start - 0x100) & 0xfff) >> 7 : >>>>> + ((res.start - 0x200) & 0xfff) >> 7; >>>>> if (chan->id >= FSL_DMA_MAX_CHANS_PER_DEVICE) { >> >> Isn't it a bit fragile to have this based on the resource address? >> Can't device tree tell you the channel id directly by an index into >> the "dma0: dma@100300" node? > > > Yes, both this way and putting a "cell-index" into device tree work. > This won't be fragile, because the resource address should always be defined > correctly, otherwise even if we can tell a channel id by "cell-index" but > with wrong resource address, nothing will work. > This piece of code only doesn't seem as neat as using "cell-index", but we > prefer the style that let the device tree describes as true as what hardware > really has. This doesn't mean "cell-index" isn't acceptable, if it is > necessary and unavoidable, we can send another patch to add it, but > currently there is no need and we don't have to do this. > I'm pointing it out because we just had a bug fix to another driver motivated by the fact that resource addresses may move from one implementation to another, whereas the cell index provided by device tree is static. Just a note, no need to fix it now. -- Dan
On 11/12/2013 08:09 AM, Dan Williams wrote: > On Mon, Nov 11, 2013 at 1:12 AM, Hongbo Zhang > <hongbo.zhang@freescale.com> wrote: >>>>>> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c >>>>>> index 49e8fbd..16a9a48 100644 >>>>>> --- a/drivers/dma/fsldma.c >>>>>> +++ b/drivers/dma/fsldma.c >>>>>> @@ -1261,7 +1261,9 @@ static int fsl_dma_chan_probe(struct >>>>>> fsldma_device >>>>>> *fdev, >>>>>> WARN_ON(fdev->feature != chan->feature); >>>>>> chan->dev = fdev->dev; >>>>>> - chan->id = ((res.start - 0x100) & 0xfff) >> 7; >>>>>> + chan->id = (res.start & 0xfff) < 0x300 ? >>>>>> + ((res.start - 0x100) & 0xfff) >> 7 : >>>>>> + ((res.start - 0x200) & 0xfff) >> 7; >>>>>> if (chan->id >= FSL_DMA_MAX_CHANS_PER_DEVICE) { >>> Isn't it a bit fragile to have this based on the resource address? >>> Can't device tree tell you the channel id directly by an index into >>> the "dma0: dma@100300" node? >> >> Yes, both this way and putting a "cell-index" into device tree work. >> This won't be fragile, because the resource address should always be defined >> correctly, otherwise even if we can tell a channel id by "cell-index" but >> with wrong resource address, nothing will work. >> This piece of code only doesn't seem as neat as using "cell-index", but we >> prefer the style that let the device tree describes as true as what hardware >> really has. This doesn't mean "cell-index" isn't acceptable, if it is >> necessary and unavoidable, we can send another patch to add it, but >> currently there is no need and we don't have to do this. >> > I'm pointing it out because we just had a bug fix to another driver > motivated by the fact that resource addresses may move from one > implementation to another, whereas the cell index provided by device > tree is static. Just a note, no need to fix it now. Get it, and will remember it, thank you Dan. > -- > Dan >
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig index 6825957..3979c65 100644 --- a/drivers/dma/Kconfig +++ b/drivers/dma/Kconfig @@ -89,14 +89,15 @@ config AT_HDMAC Support the Atmel AHB DMA controller. config FSL_DMA - tristate "Freescale Elo and Elo Plus DMA support" + tristate "Freescale Elo series DMA support" depends on FSL_SOC select DMA_ENGINE select ASYNC_TX_ENABLE_CHANNEL_SWITCH ---help--- - Enable support for the Freescale Elo and Elo Plus DMA controllers. - The Elo is the DMA controller on some 82xx and 83xx parts, and the - Elo Plus is the DMA controller on 85xx and 86xx parts. + Enable support for the Freescale Elo series DMA controllers. + The Elo is the DMA controller on some mpc82xx and mpc83xx parts, the + EloPlus is on mpc85xx and mpc86xx and Pxxx parts, and the Elo3 is on + some Txxx and Bxxx parts. config MPC512X_DMA tristate "Freescale MPC512x built-in DMA engine support" diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c index 49e8fbd..16a9a48 100644 --- a/drivers/dma/fsldma.c +++ b/drivers/dma/fsldma.c @@ -1261,7 +1261,9 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev, WARN_ON(fdev->feature != chan->feature); chan->dev = fdev->dev; - chan->id = ((res.start - 0x100) & 0xfff) >> 7; + chan->id = (res.start & 0xfff) < 0x300 ? + ((res.start - 0x100) & 0xfff) >> 7 : + ((res.start - 0x200) & 0xfff) >> 7; if (chan->id >= FSL_DMA_MAX_CHANS_PER_DEVICE) { dev_err(fdev->dev, "too many channels for device\n"); err = -EINVAL; @@ -1434,6 +1436,7 @@ static int fsldma_of_remove(struct platform_device *op) } static const struct of_device_id fsldma_of_ids[] = { + { .compatible = "fsl,elo3-dma", }, { .compatible = "fsl,eloplus-dma", }, { .compatible = "fsl,elo-dma", }, {} @@ -1455,7 +1458,7 @@ static struct platform_driver fsldma_of_driver = { static __init int fsldma_init(void) { - pr_info("Freescale Elo / Elo Plus DMA driver\n"); + pr_info("Freescale Elo series DMA driver\n"); return platform_driver_register(&fsldma_of_driver); } @@ -1467,5 +1470,5 @@ static void __exit fsldma_exit(void) subsys_initcall(fsldma_init); module_exit(fsldma_exit); -MODULE_DESCRIPTION("Freescale Elo / Elo Plus DMA driver"); +MODULE_DESCRIPTION("Freescale Elo series DMA driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h index f5c3879..1ffc244 100644 --- a/drivers/dma/fsldma.h +++ b/drivers/dma/fsldma.h @@ -112,7 +112,7 @@ struct fsldma_chan_regs { }; struct fsldma_chan; -#define FSL_DMA_MAX_CHANS_PER_DEVICE 4 +#define FSL_DMA_MAX_CHANS_PER_DEVICE 8 struct fsldma_device { void __iomem *regs; /* DGSR register base */