lkml.org 
[lkml]   [2013]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] mmp_pdma: Style neatening
    On Sun, Nov 17, 2013 at 12:12:56PM -0800, Joe Perches wrote:
    > Neaten code used as a template for other drivers.
    > Make the code more consistent with kernel styles.
    >
    > o Convert #defines with (1<<foo) to BIT(foo)
    > o Alignment wrapping
    > o Logic inversions to put return at end of functions
    > o Convert devm_kzalloc with multiply to devm_kcalloc
    > o typo of Peripheral fix
    >
    > Signed-off-by: Joe Perches <joe@perches.com>
    > ---
    > > At least, the code is directly taken from mmp_pdma.c ;-)
    >
    > Well, maybe the template code should be updated if there
    > are going to be more of these.
    >
    > Uncompiled/untested.
    Compile tested and applied.

    BUT you should not have hijacked the thread and sent this patch on a different
    chain!

    --
    ~Vinod
    >
    > drivers/dma/mmp_pdma.c | 204 +++++++++++++++++++++++++------------------------
    > 1 file changed, 105 insertions(+), 99 deletions(-)
    >
    > diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
    > index dcb1e05..c2658f6 100644
    > --- a/drivers/dma/mmp_pdma.c
    > +++ b/drivers/dma/mmp_pdma.c
    > @@ -5,6 +5,7 @@
    > * it under the terms of the GNU General Public License version 2 as
    > * published by the Free Software Foundation.
    > */
    > +
    > #include <linux/err.h>
    > #include <linux/module.h>
    > #include <linux/init.h>
    > @@ -32,38 +33,37 @@
    > #define DTADR 0x0208
    > #define DCMD 0x020c
    >
    > -#define DCSR_RUN (1 << 31) /* Run Bit (read / write) */
    > -#define DCSR_NODESC (1 << 30) /* No-Descriptor Fetch (read / write) */
    > -#define DCSR_STOPIRQEN (1 << 29) /* Stop Interrupt Enable (read / write) */
    > -#define DCSR_REQPEND (1 << 8) /* Request Pending (read-only) */
    > -#define DCSR_STOPSTATE (1 << 3) /* Stop State (read-only) */
    > -#define DCSR_ENDINTR (1 << 2) /* End Interrupt (read / write) */
    > -#define DCSR_STARTINTR (1 << 1) /* Start Interrupt (read / write) */
    > -#define DCSR_BUSERR (1 << 0) /* Bus Error Interrupt (read / write) */
    > -
    > -#define DCSR_EORIRQEN (1 << 28) /* End of Receive Interrupt Enable (R/W) */
    > -#define DCSR_EORJMPEN (1 << 27) /* Jump to next descriptor on EOR */
    > -#define DCSR_EORSTOPEN (1 << 26) /* STOP on an EOR */
    > -#define DCSR_SETCMPST (1 << 25) /* Set Descriptor Compare Status */
    > -#define DCSR_CLRCMPST (1 << 24) /* Clear Descriptor Compare Status */
    > -#define DCSR_CMPST (1 << 10) /* The Descriptor Compare Status */
    > -#define DCSR_EORINTR (1 << 9) /* The end of Receive */
    > -
    > -#define DRCMR(n) ((((n) < 64) ? 0x0100 : 0x1100) + \
    > - (((n) & 0x3f) << 2))
    > -#define DRCMR_MAPVLD (1 << 7) /* Map Valid (read / write) */
    > -#define DRCMR_CHLNUM 0x1f /* mask for Channel Number (read / write) */
    > +#define DCSR_RUN BIT(31) /* Run Bit (read / write) */
    > +#define DCSR_NODESC BIT(30) /* No-Descriptor Fetch (read / write) */
    > +#define DCSR_STOPIRQEN BIT(29) /* Stop Interrupt Enable (read / write) */
    > +#define DCSR_REQPEND BIT(8) /* Request Pending (read-only) */
    > +#define DCSR_STOPSTATE BIT(3) /* Stop State (read-only) */
    > +#define DCSR_ENDINTR BIT(2) /* End Interrupt (read / write) */
    > +#define DCSR_STARTINTR BIT(1) /* Start Interrupt (read / write) */
    > +#define DCSR_BUSERR BIT(0) /* Bus Error Interrupt (read / write) */
    > +
    > +#define DCSR_EORIRQEN BIT(28) /* End of Receive Interrupt Enable (R/W) */
    > +#define DCSR_EORJMPEN BIT(27) /* Jump to next descriptor on EOR */
    > +#define DCSR_EORSTOPEN BIT(26) /* STOP on an EOR */
    > +#define DCSR_SETCMPST BIT(25) /* Set Descriptor Compare Status */
    > +#define DCSR_CLRCMPST BIT(24) /* Clear Descriptor Compare Status */
    > +#define DCSR_CMPST BIT(10) /* The Descriptor Compare Status */
    > +#define DCSR_EORINTR BIT(9) /* The end of Receive */
    > +
    > +#define DRCMR(n) ((((n) < 64) ? 0x0100 : 0x1100) + (((n) & 0x3f) << 2))
    > +#define DRCMR_MAPVLD BIT(7) /* Map Valid (read / write) */
    > +#define DRCMR_CHLNUM 0x1f /* mask for Channel Number (read / write) */
    >
    > #define DDADR_DESCADDR 0xfffffff0 /* Address of next descriptor (mask) */
    > -#define DDADR_STOP (1 << 0) /* Stop (read / write) */
    > -
    > -#define DCMD_INCSRCADDR (1 << 31) /* Source Address Increment Setting. */
    > -#define DCMD_INCTRGADDR (1 << 30) /* Target Address Increment Setting. */
    > -#define DCMD_FLOWSRC (1 << 29) /* Flow Control by the source. */
    > -#define DCMD_FLOWTRG (1 << 28) /* Flow Control by the target. */
    > -#define DCMD_STARTIRQEN (1 << 22) /* Start Interrupt Enable */
    > -#define DCMD_ENDIRQEN (1 << 21) /* End Interrupt Enable */
    > -#define DCMD_ENDIAN (1 << 18) /* Device Endian-ness. */
    > +#define DDADR_STOP BIT(0) /* Stop (read / write) */
    > +
    > +#define DCMD_INCSRCADDR BIT(31) /* Source Address Increment Setting. */
    > +#define DCMD_INCTRGADDR BIT(30) /* Target Address Increment Setting. */
    > +#define DCMD_FLOWSRC BIT(29) /* Flow Control by the source. */
    > +#define DCMD_FLOWTRG BIT(28) /* Flow Control by the target. */
    > +#define DCMD_STARTIRQEN BIT(22) /* Start Interrupt Enable */
    > +#define DCMD_ENDIRQEN BIT(21) /* End Interrupt Enable */
    > +#define DCMD_ENDIAN BIT(18) /* Device Endian-ness. */
    > #define DCMD_BURST8 (1 << 16) /* 8 byte burst */
    > #define DCMD_BURST16 (2 << 16) /* 16 byte burst */
    > #define DCMD_BURST32 (3 << 16) /* 32 byte burst */
    > @@ -132,10 +132,14 @@ struct mmp_pdma_device {
    > spinlock_t phy_lock; /* protect alloc/free phy channels */
    > };
    >
    > -#define tx_to_mmp_pdma_desc(tx) container_of(tx, struct mmp_pdma_desc_sw, async_tx)
    > -#define to_mmp_pdma_desc(lh) container_of(lh, struct mmp_pdma_desc_sw, node)
    > -#define to_mmp_pdma_chan(dchan) container_of(dchan, struct mmp_pdma_chan, chan)
    > -#define to_mmp_pdma_dev(dmadev) container_of(dmadev, struct mmp_pdma_device, device)
    > +#define tx_to_mmp_pdma_desc(tx) \
    > + container_of(tx, struct mmp_pdma_desc_sw, async_tx)
    > +#define to_mmp_pdma_desc(lh) \
    > + container_of(lh, struct mmp_pdma_desc_sw, node)
    > +#define to_mmp_pdma_chan(dchan) \
    > + container_of(dchan, struct mmp_pdma_chan, chan)
    > +#define to_mmp_pdma_dev(dmadev) \
    > + container_of(dmadev, struct mmp_pdma_device, device)
    >
    > static void set_desc(struct mmp_pdma_phy *phy, dma_addr_t addr)
    > {
    > @@ -162,19 +166,18 @@ static void enable_chan(struct mmp_pdma_phy *phy)
    > writel(dalgn, phy->base + DALGN);
    >
    > reg = (phy->idx << 2) + DCSR;
    > - writel(readl(phy->base + reg) | DCSR_RUN,
    > - phy->base + reg);
    > + writel(readl(phy->base + reg) | DCSR_RUN, phy->base + reg);
    > }
    >
    > static void disable_chan(struct mmp_pdma_phy *phy)
    > {
    > u32 reg;
    >
    > - if (phy) {
    > - reg = (phy->idx << 2) + DCSR;
    > - writel(readl(phy->base + reg) & ~DCSR_RUN,
    > - phy->base + reg);
    > - }
    > + if (!phy)
    > + return;
    > +
    > + reg = (phy->idx << 2) + DCSR;
    > + writel(readl(phy->base + reg) & ~DCSR_RUN, phy->base + reg);
    > }
    >
    > static int clear_chan_irq(struct mmp_pdma_phy *phy)
    > @@ -183,26 +186,27 @@ static int clear_chan_irq(struct mmp_pdma_phy *phy)
    > u32 dint = readl(phy->base + DINT);
    > u32 reg = (phy->idx << 2) + DCSR;
    >
    > - if (dint & BIT(phy->idx)) {
    > - /* clear irq */
    > - dcsr = readl(phy->base + reg);
    > - writel(dcsr, phy->base + reg);
    > - if ((dcsr & DCSR_BUSERR) && (phy->vchan))
    > - dev_warn(phy->vchan->dev, "DCSR_BUSERR\n");
    > - return 0;
    > - }
    > - return -EAGAIN;
    > + if (!(dint & BIT(phy->idx)))
    > + return -EAGAIN;
    > +
    > + /* clear irq */
    > + dcsr = readl(phy->base + reg);
    > + writel(dcsr, phy->base + reg);
    > + if ((dcsr & DCSR_BUSERR) && (phy->vchan))
    > + dev_warn(phy->vchan->dev, "DCSR_BUSERR\n");
    > +
    > + return 0;
    > }
    >
    > static irqreturn_t mmp_pdma_chan_handler(int irq, void *dev_id)
    > {
    > struct mmp_pdma_phy *phy = dev_id;
    >
    > - if (clear_chan_irq(phy) == 0) {
    > - tasklet_schedule(&phy->vchan->tasklet);
    > - return IRQ_HANDLED;
    > - } else
    > + if (clear_chan_irq(phy) != 0)
    > return IRQ_NONE;
    > +
    > + tasklet_schedule(&phy->vchan->tasklet);
    > + return IRQ_HANDLED;
    > }
    >
    > static irqreturn_t mmp_pdma_int_handler(int irq, void *dev_id)
    > @@ -224,8 +228,8 @@ static irqreturn_t mmp_pdma_int_handler(int irq, void *dev_id)
    >
    > if (irq_num)
    > return IRQ_HANDLED;
    > - else
    > - return IRQ_NONE;
    > +
    > + return IRQ_NONE;
    > }
    >
    > /* lookup free phy channel as descending priority */
    > @@ -245,9 +249,9 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan)
    > */
    >
    > spin_lock_irqsave(&pdev->phy_lock, flags);
    > - for (prio = 0; prio <= (((pdev->dma_channels - 1) & 0xf) >> 2); prio++) {
    > + for (prio = 0; prio <= ((pdev->dma_channels - 1) & 0xf) >> 2; prio++) {
    > for (i = 0; i < pdev->dma_channels; i++) {
    > - if (prio != ((i & 0xf) >> 2))
    > + if (prio != (i & 0xf) >> 2)
    > continue;
    > phy = &pdev->phy[i];
    > if (!phy->vchan) {
    > @@ -389,14 +393,16 @@ static int mmp_pdma_alloc_chan_resources(struct dma_chan *dchan)
    > if (chan->desc_pool)
    > return 1;
    >
    > - chan->desc_pool =
    > - dma_pool_create(dev_name(&dchan->dev->device), chan->dev,
    > - sizeof(struct mmp_pdma_desc_sw),
    > - __alignof__(struct mmp_pdma_desc_sw), 0);
    > + chan->desc_pool = dma_pool_create(dev_name(&dchan->dev->device),
    > + chan->dev,
    > + sizeof(struct mmp_pdma_desc_sw),
    > + __alignof__(struct mmp_pdma_desc_sw),
    > + 0);
    > if (!chan->desc_pool) {
    > dev_err(chan->dev, "unable to allocate descriptor pool\n");
    > return -ENOMEM;
    > }
    > +
    > mmp_pdma_free_phy(chan);
    > chan->idle = true;
    > chan->dev_addr = 0;
    > @@ -404,7 +410,7 @@ static int mmp_pdma_alloc_chan_resources(struct dma_chan *dchan)
    > }
    >
    > static void mmp_pdma_free_desc_list(struct mmp_pdma_chan *chan,
    > - struct list_head *list)
    > + struct list_head *list)
    > {
    > struct mmp_pdma_desc_sw *desc, *_desc;
    >
    > @@ -434,8 +440,8 @@ static void mmp_pdma_free_chan_resources(struct dma_chan *dchan)
    >
    > static struct dma_async_tx_descriptor *
    > mmp_pdma_prep_memcpy(struct dma_chan *dchan,
    > - dma_addr_t dma_dst, dma_addr_t dma_src,
    > - size_t len, unsigned long flags)
    > + dma_addr_t dma_dst, dma_addr_t dma_src,
    > + size_t len, unsigned long flags)
    > {
    > struct mmp_pdma_chan *chan;
    > struct mmp_pdma_desc_sw *first = NULL, *prev = NULL, *new;
    > @@ -515,8 +521,8 @@ fail:
    >
    > static struct dma_async_tx_descriptor *
    > mmp_pdma_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
    > - unsigned int sg_len, enum dma_transfer_direction dir,
    > - unsigned long flags, void *context)
    > + unsigned int sg_len, enum dma_transfer_direction dir,
    > + unsigned long flags, void *context)
    > {
    > struct mmp_pdma_chan *chan = to_mmp_pdma_chan(dchan);
    > struct mmp_pdma_desc_sw *first = NULL, *prev = NULL, *new = NULL;
    > @@ -591,10 +597,11 @@ fail:
    > return NULL;
    > }
    >
    > -static struct dma_async_tx_descriptor *mmp_pdma_prep_dma_cyclic(
    > - struct dma_chan *dchan, dma_addr_t buf_addr, size_t len,
    > - size_t period_len, enum dma_transfer_direction direction,
    > - unsigned long flags, void *context)
    > +static struct dma_async_tx_descriptor *
    > +mmp_pdma_prep_dma_cyclic(struct dma_chan *dchan,
    > + dma_addr_t buf_addr, size_t len, size_t period_len,
    > + enum dma_transfer_direction direction,
    > + unsigned long flags, void *context)
    > {
    > struct mmp_pdma_chan *chan;
    > struct mmp_pdma_desc_sw *first = NULL, *prev = NULL, *new;
    > @@ -636,8 +643,8 @@ static struct dma_async_tx_descriptor *mmp_pdma_prep_dma_cyclic(
    > goto fail;
    > }
    >
    > - new->desc.dcmd = chan->dcmd | DCMD_ENDIRQEN |
    > - (DCMD_LENGTH & period_len);
    > + new->desc.dcmd = (chan->dcmd | DCMD_ENDIRQEN |
    > + (DCMD_LENGTH & period_len));
    > new->desc.dsadr = dma_src;
    > new->desc.dtadr = dma_dst;
    >
    > @@ -677,12 +684,11 @@ fail:
    > }
    >
    > static int mmp_pdma_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd,
    > - unsigned long arg)
    > + unsigned long arg)
    > {
    > struct mmp_pdma_chan *chan = to_mmp_pdma_chan(dchan);
    > struct dma_slave_config *cfg = (void *)arg;
    > unsigned long flags;
    > - int ret = 0;
    > u32 maxburst = 0, addr = 0;
    > enum dma_slave_buswidth width = DMA_SLAVE_BUSWIDTH_UNDEFINED;
    >
    > @@ -739,11 +745,12 @@ static int mmp_pdma_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd,
    > return -ENOSYS;
    > }
    >
    > - return ret;
    > + return 0;
    > }
    >
    > static enum dma_status mmp_pdma_tx_status(struct dma_chan *dchan,
    > - dma_cookie_t cookie, struct dma_tx_state *txstate)
    > + dma_cookie_t cookie,
    > + struct dma_tx_state *txstate)
    > {
    > return dma_cookie_status(dchan, cookie, txstate);
    > }
    > @@ -845,15 +852,14 @@ static int mmp_pdma_remove(struct platform_device *op)
    > return 0;
    > }
    >
    > -static int mmp_pdma_chan_init(struct mmp_pdma_device *pdev,
    > - int idx, int irq)
    > +static int mmp_pdma_chan_init(struct mmp_pdma_device *pdev, int idx, int irq)
    > {
    > struct mmp_pdma_phy *phy = &pdev->phy[idx];
    > struct mmp_pdma_chan *chan;
    > int ret;
    >
    > - chan = devm_kzalloc(pdev->dev,
    > - sizeof(struct mmp_pdma_chan), GFP_KERNEL);
    > + chan = devm_kzalloc(pdev->dev, sizeof(struct mmp_pdma_chan),
    > + GFP_KERNEL);
    > if (chan == NULL)
    > return -ENOMEM;
    >
    > @@ -861,8 +867,8 @@ static int mmp_pdma_chan_init(struct mmp_pdma_device *pdev,
    > phy->base = pdev->base;
    >
    > if (irq) {
    > - ret = devm_request_irq(pdev->dev, irq,
    > - mmp_pdma_chan_handler, 0, "pdma", phy);
    > + ret = devm_request_irq(pdev->dev, irq, mmp_pdma_chan_handler, 0,
    > + "pdma", phy);
    > if (ret) {
    > dev_err(pdev->dev, "channel request irq fail!\n");
    > return ret;
    > @@ -877,8 +883,7 @@ static int mmp_pdma_chan_init(struct mmp_pdma_device *pdev,
    > INIT_LIST_HEAD(&chan->chain_running);
    >
    > /* register virt channel to dma engine */
    > - list_add_tail(&chan->chan.device_node,
    > - &pdev->device.channels);
    > + list_add_tail(&chan->chan.device_node, &pdev->device.channels);
    >
    > return 0;
    > }
    > @@ -913,13 +918,12 @@ retry:
    > * the lookup and the reservation */
    > chan = dma_get_slave_channel(candidate);
    >
    > - if (chan) {
    > - struct mmp_pdma_chan *c = to_mmp_pdma_chan(chan);
    > - c->drcmr = dma_spec->args[0];
    > - return chan;
    > - }
    > + if (!chan)
    > + goto retry;
    >
    > - goto retry;
    > + to_mmp_pdma_chan(chan)->drcmr = dma_spec->args[0];
    > +
    > + return chan;
    > }
    >
    > static int mmp_pdma_probe(struct platform_device *op)
    > @@ -934,6 +938,7 @@ static int mmp_pdma_probe(struct platform_device *op)
    > pdev = devm_kzalloc(&op->dev, sizeof(*pdev), GFP_KERNEL);
    > if (!pdev)
    > return -ENOMEM;
    > +
    > pdev->dev = &op->dev;
    >
    > spin_lock_init(&pdev->phy_lock);
    > @@ -945,8 +950,8 @@ static int mmp_pdma_probe(struct platform_device *op)
    >
    > of_id = of_match_device(mmp_pdma_dt_ids, pdev->dev);
    > if (of_id)
    > - of_property_read_u32(pdev->dev->of_node,
    > - "#dma-channels", &dma_channels);
    > + of_property_read_u32(pdev->dev->of_node, "#dma-channels",
    > + &dma_channels);
    > else if (pdata && pdata->dma_channels)
    > dma_channels = pdata->dma_channels;
    > else
    > @@ -958,8 +963,9 @@ static int mmp_pdma_probe(struct platform_device *op)
    > irq_num++;
    > }
    >
    > - pdev->phy = devm_kzalloc(pdev->dev,
    > - dma_channels * sizeof(struct mmp_pdma_chan), GFP_KERNEL);
    > + pdev->phy = devm_kcalloc(pdev->dev,
    > + dma_channels, sizeof(struct mmp_pdma_chan),
    > + GFP_KERNEL);
    > if (pdev->phy == NULL)
    > return -ENOMEM;
    >
    > @@ -968,8 +974,8 @@ static int mmp_pdma_probe(struct platform_device *op)
    > if (irq_num != dma_channels) {
    > /* all chan share one irq, demux inside */
    > irq = platform_get_irq(op, 0);
    > - ret = devm_request_irq(pdev->dev, irq,
    > - mmp_pdma_int_handler, 0, "pdma", pdev);
    > + ret = devm_request_irq(pdev->dev, irq, mmp_pdma_int_handler, 0,
    > + "pdma", pdev);
    > if (ret)
    > return ret;
    > }
    > @@ -1044,7 +1050,7 @@ bool mmp_pdma_filter_fn(struct dma_chan *chan, void *param)
    > if (chan->device->dev->driver != &mmp_pdma_driver.driver)
    > return false;
    >
    > - c->drcmr = *(unsigned int *) param;
    > + c->drcmr = *(unsigned int *)param;
    >
    > return true;
    > }
    > @@ -1052,6 +1058,6 @@ EXPORT_SYMBOL_GPL(mmp_pdma_filter_fn);
    >
    > module_platform_driver(mmp_pdma_driver);
    >
    > -MODULE_DESCRIPTION("MARVELL MMP Periphera DMA Driver");
    > +MODULE_DESCRIPTION("MARVELL MMP Peripheral DMA Driver");
    > MODULE_AUTHOR("Marvell International Ltd.");
    > MODULE_LICENSE("GPL v2");
    >
    >
    >

    --


    \
     
     \ /
      Last update: 2013-11-28 11:41    [W:6.751 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site