Hi
The first two patches in this small series improve driver internals a bit
by using preferred APIs, the 3rd patch adds support for new DMAC versions,
present on AG5, APE6, H2.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
Switch shdma to using devm_* managed functions for allocation of memory,
requesting IRQs, mapping IO resources etc.
Signed-off-by: Guennadi Liakhovetski <[email protected]>
---
drivers/dma/sh/shdma-base.c | 11 +-----
drivers/dma/sh/shdma.c | 71 ++++++++-----------------------------------
drivers/dma/sh/sudmac.c | 1 -
include/linux/shdma-base.h | 1 -
4 files changed, 15 insertions(+), 69 deletions(-)
diff --git a/drivers/dma/sh/shdma-base.c b/drivers/dma/sh/shdma-base.c
index 28ca361..c5ea256 100644
--- a/drivers/dma/sh/shdma-base.c
+++ b/drivers/dma/sh/shdma-base.c
@@ -831,8 +831,8 @@ static irqreturn_t chan_irqt(int irq, void *dev)
int shdma_request_irq(struct shdma_chan *schan, int irq,
unsigned long flags, const char *name)
{
- int ret = request_threaded_irq(irq, chan_irq, chan_irqt,
- flags, name, schan);
+ int ret = devm_request_threaded_irq(schan->dev, irq, chan_irq,
+ chan_irqt, flags, name, schan);
schan->irq = ret < 0 ? ret : irq;
@@ -840,13 +840,6 @@ int shdma_request_irq(struct shdma_chan *schan, int irq,
}
EXPORT_SYMBOL(shdma_request_irq);
-void shdma_free_irq(struct shdma_chan *schan)
-{
- if (schan->irq >= 0)
- free_irq(schan->irq, schan);
-}
-EXPORT_SYMBOL(shdma_free_irq);
-
void shdma_chan_probe(struct shdma_dev *sdev,
struct shdma_chan *schan, int id)
{
diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
index c7950be..3083d62 100644
--- a/drivers/dma/sh/shdma.c
+++ b/drivers/dma/sh/shdma.c
@@ -507,7 +507,8 @@ static int sh_dmae_chan_probe(struct sh_dmae_device *shdev, int id,
struct shdma_chan *schan;
int err;
- sh_chan = kzalloc(sizeof(struct sh_dmae_chan), GFP_KERNEL);
+ sh_chan = devm_kzalloc(sdev->dma_dev.dev, sizeof(struct sh_dmae_chan),
+ GFP_KERNEL);
if (!sh_chan) {
dev_err(sdev->dma_dev.dev,
"No free memory for allocating dma channels!\n");
@@ -543,7 +544,6 @@ static int sh_dmae_chan_probe(struct sh_dmae_device *shdev, int id,
err_no_irq:
/* remove from dmaengine device node */
shdma_chan_remove(schan);
- kfree(sh_chan);
return err;
}
@@ -554,14 +554,9 @@ static void sh_dmae_chan_remove(struct sh_dmae_device *shdev)
int i;
shdma_for_each_chan(schan, &shdev->shdma_dev, i) {
- struct sh_dmae_chan *sh_chan = container_of(schan,
- struct sh_dmae_chan, shdma_chan);
BUG_ON(!schan);
- shdma_free_irq(&sh_chan->shdma_chan);
-
shdma_chan_remove(schan);
- kfree(sh_chan);
}
dma_dev->chancnt = 0;
}
@@ -698,33 +693,22 @@ static int sh_dmae_probe(struct platform_device *pdev)
if (!chan || !errirq_res)
return -ENODEV;
- if (!request_mem_region(chan->start, resource_size(chan), pdev->name)) {
- dev_err(&pdev->dev, "DMAC register region already claimed\n");
- return -EBUSY;
- }
-
- if (dmars && !request_mem_region(dmars->start, resource_size(dmars), pdev->name)) {
- dev_err(&pdev->dev, "DMAC DMARS region already claimed\n");
- err = -EBUSY;
- goto ermrdmars;
- }
-
- err = -ENOMEM;
- shdev = kzalloc(sizeof(struct sh_dmae_device), GFP_KERNEL);
+ shdev = devm_kzalloc(&pdev->dev, sizeof(struct sh_dmae_device),
+ GFP_KERNEL);
if (!shdev) {
dev_err(&pdev->dev, "Not enough memory\n");
- goto ealloc;
+ return -ENOMEM;
}
dma_dev = &shdev->shdma_dev.dma_dev;
- shdev->chan_reg = ioremap(chan->start, resource_size(chan));
- if (!shdev->chan_reg)
- goto emapchan;
+ shdev->chan_reg = devm_ioremap_resource(&pdev->dev, chan);
+ if (IS_ERR(shdev->chan_reg))
+ return PTR_ERR(shdev->chan_reg);
if (dmars) {
- shdev->dmars = ioremap(dmars->start, resource_size(dmars));
- if (!shdev->dmars)
- goto emapdmars;
+ shdev->dmars = devm_ioremap_resource(&pdev->dev, dmars);
+ if (IS_ERR(shdev->dmars))
+ return PTR_ERR(shdev->dmars);
}
if (!pdata->slave_only)
@@ -785,8 +769,8 @@ static int sh_dmae_probe(struct platform_device *pdev)
errirq = errirq_res->start;
- err = request_irq(errirq, sh_dmae_err, irqflags,
- "DMAC Address Error", shdev);
+ err = devm_request_irq(&pdev->dev, errirq, sh_dmae_err, irqflags,
+ "DMAC Address Error", shdev);
if (err) {
dev_err(&pdev->dev,
"DMA failed requesting irq #%d, error %d\n",
@@ -875,7 +859,6 @@ chan_probe_err:
sh_dmae_chan_remove(shdev);
#if defined(CONFIG_CPU_SH4) || defined(CONFIG_ARCH_SHMOBILE)
- free_irq(errirq, shdev);
eirq_err:
#endif
rst_err:
@@ -889,18 +872,7 @@ rst_err:
platform_set_drvdata(pdev, NULL);
shdma_cleanup(&shdev->shdma_dev);
eshdma:
- if (dmars)
- iounmap(shdev->dmars);
-emapdmars:
- iounmap(shdev->chan_reg);
synchronize_rcu();
-emapchan:
- kfree(shdev);
-ealloc:
- if (dmars)
- release_mem_region(dmars->start, resource_size(dmars));
-ermrdmars:
- release_mem_region(chan->start, resource_size(chan));
return err;
}
@@ -909,17 +881,12 @@ static int sh_dmae_remove(struct platform_device *pdev)
{
struct sh_dmae_device *shdev = platform_get_drvdata(pdev);
struct dma_device *dma_dev = &shdev->shdma_dev.dma_dev;
- struct resource *res;
- int errirq = platform_get_irq(pdev, 0);
dma_async_device_unregister(dma_dev);
/* Is a NOP if this controller isn't registered with of-dma */
of_dma_controller_free(pdev->dev.of_node);
- if (errirq > 0)
- free_irq(errirq, shdev);
-
spin_lock_irq(&sh_dmae_lock);
list_del_rcu(&shdev->node);
spin_unlock_irq(&sh_dmae_lock);
@@ -929,21 +896,9 @@ static int sh_dmae_remove(struct platform_device *pdev)
sh_dmae_chan_remove(shdev);
shdma_cleanup(&shdev->shdma_dev);
- if (shdev->dmars)
- iounmap(shdev->dmars);
- iounmap(shdev->chan_reg);
-
platform_set_drvdata(pdev, NULL);
synchronize_rcu();
- kfree(shdev);
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (res)
- release_mem_region(res->start, resource_size(res));
- res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
- if (res)
- release_mem_region(res->start, resource_size(res));
return 0;
}
diff --git a/drivers/dma/sh/sudmac.c b/drivers/dma/sh/sudmac.c
index e7c94bb..3477901 100644
--- a/drivers/dma/sh/sudmac.c
+++ b/drivers/dma/sh/sudmac.c
@@ -302,7 +302,6 @@ static void sudmac_chan_remove(struct sudmac_device *su_dev)
BUG_ON(!schan);
- shdma_free_irq(&sc->shdma_chan);
shdma_chan_remove(schan);
}
dma_dev->chancnt = 0;
diff --git a/include/linux/shdma-base.h b/include/linux/shdma-base.h
index 9d67bb1..1aa1f19 100644
--- a/include/linux/shdma-base.h
+++ b/include/linux/shdma-base.h
@@ -116,7 +116,6 @@ struct shdma_dev {
int shdma_request_irq(struct shdma_chan *, int,
unsigned long, const char *);
-void shdma_free_irq(struct shdma_chan *);
bool shdma_reset(struct shdma_dev *sdev);
void shdma_chan_probe(struct shdma_dev *sdev,
struct shdma_chan *schan, int id);
--
1.7.2.5
On newer r-car SoCs the CHCLR register only contains one bit per channel,
to which a 1 has to be written to reset the channel. Older SoC versions had
one CHCLR register per channel, to which a 0 must be written to reset the
channel and clear its buffers. This patch adds support for the newer
layout.
Signed-off-by: Guennadi Liakhovetski <[email protected]>
---
drivers/dma/sh/shdma.c | 6 ++++--
include/linux/sh_dma.h | 12 +++++++++++-
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
index 64eb7fb..883a4de 100644
--- a/drivers/dma/sh/shdma.c
+++ b/drivers/dma/sh/shdma.c
@@ -54,9 +54,11 @@ static LIST_HEAD(sh_dmae_devices);
static void channel_clear(struct sh_dmae_chan *sh_dc)
{
struct sh_dmae_device *shdev = to_sh_dev(sh_dc);
+ const struct sh_dmae_channel *chan_pdata = shdev->pdata->channel +
+ sh_dc->shdma_chan.id;
+ u32 val = chan_pdata->chclr_bit < 0 ? 0 : 1 << chan_pdata->chclr_bit;
- __raw_writel(0, shdev->chan_reg +
- shdev->pdata->channel[sh_dc->shdma_chan.id].chclr_offset);
+ __raw_writel(val, shdev->chan_reg + chan_pdata->chclr_offset);
}
static void sh_dmae_writel(struct sh_dmae_chan *sh_dc, u32 data, u32 reg)
diff --git a/include/linux/sh_dma.h b/include/linux/sh_dma.h
index 4e83f3e..9316a1d 100644
--- a/include/linux/sh_dma.h
+++ b/include/linux/sh_dma.h
@@ -33,11 +33,21 @@ struct sh_dmae_slave_config {
char mid_rid;
};
+/**
+ * struct sh_dmae_channel - DMAC channel platform data
+ * @offset: register offset within the main IOMEM resource
+ * @dmars: channel DMARS register offset
+ * @chclr_offset: channel CHCLR register offset
+ * @dmars_bit: channel DMARS field offset within the register
+ * @chclr_bit: > 0: bit position, to be set to reset the channel
+ * < 0: CHCLR has to be cleared to clear channel's buffers
+ */
struct sh_dmae_channel {
unsigned int offset;
unsigned int dmars;
- unsigned int dmars_bit;
unsigned int chclr_offset;
+ unsigned char dmars_bit;
+ signed char chclr_bit;
};
struct sh_dmae_pdata {
--
1.7.2.5
In the shdma driver __iomem pointers are used to point to hardware
registers. Using typed pointers like "u32 __iomem *" in this case is
inconvenient, because then offsets, added to such pointers, have to be
devided by sizeof(u32) or similar. Switch the driver to use void
pointers, which avoids this clumsiness.
Signed-off-by: Guennadi Liakhovetski <[email protected]>
---
drivers/dma/sh/shdma.c | 22 +++++++++++-----------
drivers/dma/sh/shdma.h | 6 +++---
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
index 3083d62..64eb7fb 100644
--- a/drivers/dma/sh/shdma.c
+++ b/drivers/dma/sh/shdma.c
@@ -56,22 +56,22 @@ static void channel_clear(struct sh_dmae_chan *sh_dc)
struct sh_dmae_device *shdev = to_sh_dev(sh_dc);
__raw_writel(0, shdev->chan_reg +
- shdev->pdata->channel[sh_dc->shdma_chan.id].chclr_offset / sizeof(u32));
+ shdev->pdata->channel[sh_dc->shdma_chan.id].chclr_offset);
}
static void sh_dmae_writel(struct sh_dmae_chan *sh_dc, u32 data, u32 reg)
{
- __raw_writel(data, sh_dc->base + reg / sizeof(u32));
+ __raw_writel(data, sh_dc->base + reg);
}
static u32 sh_dmae_readl(struct sh_dmae_chan *sh_dc, u32 reg)
{
- return __raw_readl(sh_dc->base + reg / sizeof(u32));
+ return __raw_readl(sh_dc->base + reg);
}
static u16 dmaor_read(struct sh_dmae_device *shdev)
{
- u32 __iomem *addr = shdev->chan_reg + DMAOR / sizeof(u32);
+ void __iomem *addr = shdev->chan_reg + DMAOR;
if (shdev->pdata->dmaor_is_32bit)
return __raw_readl(addr);
@@ -81,7 +81,7 @@ static u16 dmaor_read(struct sh_dmae_device *shdev)
static void dmaor_write(struct sh_dmae_device *shdev, u16 data)
{
- u32 __iomem *addr = shdev->chan_reg + DMAOR / sizeof(u32);
+ void __iomem *addr = shdev->chan_reg + DMAOR;
if (shdev->pdata->dmaor_is_32bit)
__raw_writel(data, addr);
@@ -93,14 +93,14 @@ static void chcr_write(struct sh_dmae_chan *sh_dc, u32 data)
{
struct sh_dmae_device *shdev = to_sh_dev(sh_dc);
- __raw_writel(data, sh_dc->base + shdev->chcr_offset / sizeof(u32));
+ __raw_writel(data, sh_dc->base + shdev->chcr_offset);
}
static u32 chcr_read(struct sh_dmae_chan *sh_dc)
{
struct sh_dmae_device *shdev = to_sh_dev(sh_dc);
- return __raw_readl(sh_dc->base + shdev->chcr_offset / sizeof(u32));
+ return __raw_readl(sh_dc->base + shdev->chcr_offset);
}
/*
@@ -244,7 +244,7 @@ static int dmae_set_dmars(struct sh_dmae_chan *sh_chan, u16 val)
struct sh_dmae_device *shdev = to_sh_dev(sh_chan);
struct sh_dmae_pdata *pdata = shdev->pdata;
const struct sh_dmae_channel *chan_pdata = &pdata->channel[sh_chan->shdma_chan.id];
- u16 __iomem *addr = shdev->dmars;
+ void __iomem *addr = shdev->dmars;
unsigned int shift = chan_pdata->dmars_bit;
if (dmae_is_busy(sh_chan))
@@ -255,8 +255,8 @@ static int dmae_set_dmars(struct sh_dmae_chan *sh_chan, u16 val)
/* in the case of a missing DMARS resource use first memory window */
if (!addr)
- addr = (u16 __iomem *)shdev->chan_reg;
- addr += chan_pdata->dmars / sizeof(u16);
+ addr = shdev->chan_reg;
+ addr += chan_pdata->dmars;
__raw_writew((__raw_readw(addr) & (0xff00 >> shift)) | (val << shift),
addr);
@@ -520,7 +520,7 @@ static int sh_dmae_chan_probe(struct sh_dmae_device *shdev, int id,
shdma_chan_probe(sdev, schan, id);
- sh_chan->base = shdev->chan_reg + chan_pdata->offset / sizeof(u32);
+ sh_chan->base = shdev->chan_reg + chan_pdata->offset;
/* set up channel irq */
if (pdev->id >= 0)
diff --git a/drivers/dma/sh/shdma.h b/drivers/dma/sh/shdma.h
index 9314e93..06aae6e 100644
--- a/drivers/dma/sh/shdma.h
+++ b/drivers/dma/sh/shdma.h
@@ -28,7 +28,7 @@ struct sh_dmae_chan {
struct shdma_chan shdma_chan;
const struct sh_dmae_slave_config *config; /* Slave DMA configuration */
int xmit_shift; /* log_2(bytes_per_xfer) */
- u32 __iomem *base;
+ void __iomem *base;
char dev_id[16]; /* unique name per DMAC of channel */
int pm_error;
};
@@ -38,8 +38,8 @@ struct sh_dmae_device {
struct sh_dmae_chan *chan[SH_DMAE_MAX_CHANNELS];
struct sh_dmae_pdata *pdata;
struct list_head node;
- u32 __iomem *chan_reg;
- u16 __iomem *dmars;
+ void __iomem *chan_reg;
+ void __iomem *dmars;
unsigned int chcr_offset;
u32 chcr_ie_bit;
};
--
1.7.2.5
On Tue, Jul 02, 2013 at 05:45:49PM +0200, Guennadi Liakhovetski wrote:
> Hi
>
> The first two patches in this small series improve driver internals a bit
> by using preferred APIs, the 3rd patch adds support for new DMAC versions,
> present on AG5, APE6, H2.
Applied, thanks
Although the patch 1 didnt apply for me :( I have manually remove a hunk, I dont
which tree yoy genertaed this agaisnt!
~Vinod
Hi Vinod,
Thanks for applying the patches!
On Sun, 25 Aug 2013, Vinod Koul wrote:
> On Tue, Jul 02, 2013 at 05:45:49PM +0200, Guennadi Liakhovetski wrote:
> > Hi
> >
> > The first two patches in this small series improve driver internals a bit
> > by using preferred APIs, the 3rd patch adds support for new DMAC versions,
> > present on AG5, APE6, H2.
> Applied, thanks
>
> Although the patch 1 didnt apply for me :( I have manually remove a hunk, I dont
> which tree yoy genertaed this agaisnt!
I'm not sure why it didn't apply for you. AFAICS, you mean this hunk:
diff --git a/drivers/dma/sh/shdma.c b/drivers/dma/sh/shdma.c
index c7950be..3083d62 100644
--- a/drivers/dma/sh/shdma.c
+++ b/drivers/dma/sh/shdma.c
[snip]
@@ -909,17 +881,12 @@ static int sh_dmae_remove(struct platform_device *pdev)
{
struct sh_dmae_device *shdev = platform_get_drvdata(pdev);
struct dma_device *dma_dev = &shdev->shdma_dev.dma_dev;
- struct resource *res;
- int errirq = platform_get_irq(pdev, 0);
dma_async_device_unregister(dma_dev);
/* Is a NOP if this controller isn't registered with of-dma */
of_dma_controller_free(pdev->dev.of_node);
- if (errirq > 0)
- free_irq(errirq, shdev);
-
spin_lock_irq(&sh_dmae_lock);
list_del_rcu(&shdev->node);
spin_unlock_irq(&sh_dmae_lock);
It didn't apply, because it is based on top of "DMA: shdma: also support
single top-level DMAC DT nodes" which has been dropped for now in favour
of keeping a multiplexer node compulsory for shdma for now, sorry. But
just dropping it isn't right too. With managed error IRQ allocation
calling free_irq() on clean up path is wrong. I'll post an incremental
patch asap.
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/