LinuxLists.cc - [PATCH 1/2] dma: omap-dma: Fix line over 80 characters

2015-05-20 19:11:33

Subject: [PATCH 1/2] dma: omap-dma: Fix line over 80 characters

This patch fixes line over 80 characters warninings while
running checkpatch.pl

Signed-off-by: Jagan Teki <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: Vinod Koul <[email protected]>
---
drivers/dma/omap-dma.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
index 167dbaf..3806524 100644
--- a/drivers/dma/omap-dma.c
+++ b/drivers/dma/omap-dma.c
@@ -168,7 +168,8 @@ static inline struct omap_chan *to_omap_dma_chan(struct dma_chan *c)
return container_of(c, struct omap_chan, vc.chan);
}

-static inline struct omap_desc *to_omap_dma_desc(struct dma_async_tx_descriptor *t)
+static inline struct omap_desc *to_omap_dma_desc(
+ struct dma_async_tx_descriptor *t)
{
return container_of(t, struct omap_desc, vd.tx);
}
@@ -219,7 +220,8 @@ static unsigned omap_dma_read(unsigned type, void __iomem *addr)
return val;
}

-static void omap_dma_glbl_write(struct omap_dmadev *od, unsigned reg, unsigned val)
+static void omap_dma_glbl_write(struct omap_dmadev *od,
+ unsigned reg, unsigned val)
{
const struct omap_dma_reg *r = od->reg_map + reg;

@@ -334,7 +336,7 @@ static void omap_dma_stop(struct omap_chan *c)
if (val & (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
dev_err(c->vc.chan.device->dev,
"DMA drain did not complete on lch %d\n",
- c->dma_ch);
+ c->dma_ch);

omap_dma_glbl_write(od, OCP_SYSCONFIG, sysconfig);
} else {
@@ -544,7 +546,8 @@ static int omap_dma_alloc_chan_resources(struct dma_chan *chan)
val = BIT(c->dma_ch);
omap_dma_glbl_write(od, IRQSTATUS_L1, val);
od->irq_enable_mask |= val;
- omap_dma_glbl_write(od, IRQENABLE_L1, od->irq_enable_mask);
+ omap_dma_glbl_write(od, IRQENABLE_L1,
+ od->irq_enable_mask);

val = omap_dma_glbl_read(od, IRQENABLE_L0);
val &= ~BIT(c->dma_ch);
@@ -707,7 +710,8 @@ static enum dma_status omap_dma_tx_status(struct dma_chan *chan,
spin_lock_irqsave(&c->vc.lock, flags);
vd = vchan_find_desc(&c->vc, cookie);
if (vd) {
- txstate->residue = omap_dma_desc_size(to_omap_dma_desc(&vd->tx));
+ txstate->residue = omap_dma_desc_size(
+ to_omap_dma_desc(&vd->tx));
} else if (c->desc && c->desc->vd.tx.cookie == cookie) {
struct omap_desc *d = c->desc;
dma_addr_t pos;
@@ -741,6 +745,7 @@ static void omap_dma_issue_pending(struct dma_chan *chan)
*/
if (!c->cyclic) {
struct omap_dmadev *d = to_omap_dma_dev(chan->device);
+
spin_lock(&d->lock);
if (list_empty(&c->node))
list_add_tail(&c->node, &d->pending);
@@ -948,7 +953,8 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_cyclic(
return vchan_tx_prep(&c->vc, &d->vd, flags);
}

-static int omap_dma_slave_config(struct dma_chan *chan, struct dma_slave_config *cfg)
+static int omap_dma_slave_config(struct dma_chan *chan,
+ struct dma_slave_config *cfg)
{
struct omap_chan *c = to_omap_dma_chan(chan);

--
1.9.1

2015-05-20 19:11:38

by Jagan Teki

[permalink] [raw]

Subject: [PATCH 2/2] dma: amba-pl08x: Fix line over 80 characters

2015-05-20 20:24:26

by Joe Perches

[permalink] [raw]

Subject: Re: [PATCH 1/2] dma: omap-dma: Fix line over 80 characters

On Thu, 2015-05-21 at 00:41 +0530, Jagan Teki wrote:
> This patch fixes line over 80 characters warninings while
> running checkpatch.pl

These changes aren't necessary.

If you're really going to do this, please change the style
of the functions you are using.

> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
[]
> @@ -168,7 +168,8 @@ static inline struct omap_chan *to_omap_dma_chan(struct dma_chan *c)
> return container_of(c, struct omap_nochan, vc.chan);
> }
>
> -static inline struct omap_desc *to_omap_dma_desc(struct dma_async_tx_descriptor *t)
> +static inline struct omap_desc *to_omap_dma_desc(
> + struct dma_async_tx_descriptor *t)

This isn't particularly human readable.

If the return type is long, it's sometimes better to
use a separate line for the return type, then put the
function on another line like:

static inline struct omap_desc *
to_omap_dma_desc(struct dma_async_tx_descriptor *t)

> @@ -707,7 +710,8 @@ static enum dma_status omap_dma_tx_status(struct dma_chan *chan,
> spin_lock_irqsave(&c->vc.lock, flags);
> vd = vchan_find_desc(&c->vc, cookie);
> if (vd) {
> - txstate->residue = omap_dma_desc_size(to_omap_dma_desc(&vd->tx));
> + txstate->residue = omap_dma_desc_size(
> + to_omap_dma_desc(&vd->tx));

This is unpleasant to read.
Just let some lines be > 80 columns.

2015-05-21 15:58:34

by Vinod Koul

[permalink] [raw]

Subject: Re: [PATCH 1/2] dma: omap-dma: Fix line over 80 characters

On Thu, May 21, 2015 at 12:41:09AM +0530, Jagan Teki wrote:
> This patch fixes line over 80 characters warninings while
> running checkpatch.pl

You should know 80 char is a warn not an error, and you should read the
Codingstyle documentation before you embark on these changes.
The guiding principle here is readablity and greapblity, anything which
breaks these is strict NO.
>
> Signed-off-by: Jagan Teki <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Vinod Koul <[email protected]>
> ---
> drivers/dma/omap-dma.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
> index 167dbaf..3806524 100644
> --- a/drivers/dma/omap-dma.c
> +++ b/drivers/dma/omap-dma.c
> @@ -168,7 +168,8 @@ static inline struct omap_chan *to_omap_dma_chan(struct dma_chan *c)
> return container_of(c, struct omap_chan, vc.chan);
> }
>
> -static inline struct omap_desc *to_omap_dma_desc(struct dma_async_tx_descriptor *t)
> +static inline struct omap_desc *to_omap_dma_desc(
> + struct dma_async_tx_descriptor *t)
This makes it harder to read, if you see drivers the will do it:

static inline struct omap_desc
*to_omap_dma_desc(struct dma_async_tx_descriptor *t)

> {
> return container_of(t, struct omap_desc, vd.tx);
> }
> @@ -219,7 +220,8 @@ static unsigned omap_dma_read(unsigned type, void __iomem *addr)
> return val;
> }
>
> -static void omap_dma_glbl_write(struct omap_dmadev *od, unsigned reg, unsigned val)
> +static void omap_dma_glbl_write(struct omap_dmadev *od,
> + unsigned reg, unsigned val)
> {
> const struct omap_dma_reg *r = od->reg_map + reg;
>
> @@ -334,7 +336,7 @@ static void omap_dma_stop(struct omap_chan *c)
> if (val & (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
> dev_err(c->vc.chan.device->dev,
> "DMA drain did not complete on lch %d\n",
> - c->dma_ch);
> + c->dma_ch);
whats changes here? title says 80 char!

>
> omap_dma_glbl_write(od, OCP_SYSCONFIG, sysconfig);
> } else {
> @@ -544,7 +546,8 @@ static int omap_dma_alloc_chan_resources(struct dma_chan *chan)
> val = BIT(c->dma_ch);
> omap_dma_glbl_write(od, IRQSTATUS_L1, val);
> od->irq_enable_mask |= val;
> - omap_dma_glbl_write(od, IRQENABLE_L1, od->irq_enable_mask);
> + omap_dma_glbl_write(od, IRQENABLE_L1,
> + od->irq_enable_mask);
>
> val = omap_dma_glbl_read(od, IRQENABLE_L0);
> val &= ~BIT(c->dma_ch);
> @@ -707,7 +710,8 @@ static enum dma_status omap_dma_tx_status(struct dma_chan *chan,
> spin_lock_irqsave(&c->vc.lock, flags);
> vd = vchan_find_desc(&c->vc, cookie);
> if (vd) {
> - txstate->residue = omap_dma_desc_size(to_omap_dma_desc(&vd->tx));
> + txstate->residue = omap_dma_desc_size(
> + to_omap_dma_desc(&vd->tx));
> } else if (c->desc && c->desc->vd.tx.cookie == cookie) {
> struct omap_desc *d = c->desc;
> dma_addr_t pos;
> @@ -741,6 +745,7 @@ static void omap_dma_issue_pending(struct dma_chan *chan)
> */
> if (!c->cyclic) {
> struct omap_dmadev *d = to_omap_dma_dev(chan->device);
> +
this is not per title

--
~Vinod
> spin_lock(&d->lock);
> if (list_empty(&c->node))
> list_add_tail(&c->node, &d->pending);
> @@ -948,7 +953,8 @@ static struct dma_async_tx_descriptor *omap_dma_prep_dma_cyclic(
> return vchan_tx_prep(&c->vc, &d->vd, flags);
> }
>
> -static int omap_dma_slave_config(struct dma_chan *chan, struct dma_slave_config *cfg)
> +static int omap_dma_slave_config(struct dma_chan *chan,
> + struct dma_slave_config *cfg)
> {
> struct omap_chan *c = to_omap_dma_chan(chan);
>
> --
> 1.9.1
>

--

2015-05-21 15:58:49

by Vinod Koul

[permalink] [raw]

Subject: Re: [PATCH 2/2] dma: amba-pl08x: Fix line over 80 characters

On Thu, May 21, 2015 at 12:41:10AM +0530, Jagan Teki wrote:
> This patch fixes line over 80 characters warninings while
> running checkpatch.pl
>
> Signed-off-by: Jagan Teki <[email protected]>
> Cc: Dan Williams <[email protected]>
> Cc: Vinod Koul <[email protected]>
> ---
> drivers/dma/amba-pl08x.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
> index 49d396e..a5d9cc9 100644
> --- a/drivers/dma/amba-pl08x.c
> +++ b/drivers/dma/amba-pl08x.c
> @@ -646,7 +646,9 @@ static void pl08x_phy_alloc_and_start(struct pl08x_dma_chan *plchan)
>
> ch = pl08x_get_phy_channel(pl08x, plchan);
> if (!ch) {
> - dev_dbg(&pl08x->adev->dev, "no physical channel available for xfer on %s\n", plchan->name);
> + dev_dbg(&pl08x->adev->dev,
> + "no physical channel available for xfer on %s\n",
> + plchan->name);
This looks bad to read including the below ones so I am going to decline it

--
~Vinod
> plchan->state = PL08X_CHAN_WAITING;
> return;
> }
> @@ -698,7 +700,8 @@ static void pl08x_phy_free(struct pl08x_dma_chan *plchan)
> }
>
> if (!next) {
> - list_for_each_entry(p, &pl08x->slave.channels, vc.chan.device_node)
> + list_for_each_entry(p, &pl08x->slave.channels,
> + vc.chan.device_node)

> if (p->state == PL08X_CHAN_WAITING) {
> next = p;
> break;
> @@ -1988,7 +1991,8 @@ static int pl08x_debugfs_show(struct seq_file *s, void *data)
> seq_printf(s, "\nPL08x virtual memcpy channels:\n");
> seq_printf(s, "CHANNEL:\tSTATE:\n");
> seq_printf(s, "--------\t------\n");
> - list_for_each_entry(chan, &pl08x->memcpy.channels, vc.chan.device_node) {
> + list_for_each_entry(chan, &pl08x->memcpy.channels,
> + vc.chan.device_node) {
> seq_printf(s, "%s\t\t%s\n", chan->name,
> pl08x_state_str(chan->state));
> }
> @@ -2172,7 +2176,8 @@ static int pl08x_probe(struct amba_device *adev, const struct amba_id *id)
> u32 val;
>
> val = readl(ch->reg_config);
> - if (val & (PL080N_CONFIG_ITPROT | PL080N_CONFIG_SECPROT)) {
> + if (val & (PL080N_CONFIG_ITPROT |
> + PL080N_CONFIG_SECPROT)) {
> dev_info(&adev->dev, "physical channel %d reserved for secure access only\n", i);
> ch->locked = true;
> }
> --
> 1.9.1
>

--

2015-05-21 16:40:10

by Joe Perches

[permalink] [raw]

Subject: Re: [PATCH 1/2] dma: omap-dma: Fix line over 80 characters

On Thu, 2015-05-21 at 21:29 +0530, Vinod Koul wrote:
> On Thu, May 21, 2015 at 12:41:09AM +0530, Jagan Teki wrote:
> > This patch fixes line over 80 characters warninings while
> > running checkpatch.pl
>
> You should know 80 char is a warn not an error, and you should read the
> Codingstyle documentation before you embark on these changes.
> The guiding principle here is readablity and greapblity, anything which
> breaks these is strict NO.

Well, those abilities are at a minimum certainly goalposts
and should be preferred over most everything else.

> > diff --git a/drivers/dma/omap-dma.c b/drivers/dma/omap-dma.c
[]
> > @@ -168,7 +168,8 @@ static inline struct omap_chan *to_omap_dma_chan(struct dma_chan *c)
> > return container_of(c, struct omap_chan, vc.chan);
> > }
> >
> > -static inline struct omap_desc *to_omap_dma_desc(struct dma_async_tx_descriptor *t)
> > +static inline struct omap_desc *to_omap_dma_desc(
> > + struct dma_async_tx_descriptor *t)
> This makes it harder to read, if you see drivers the will do it:
>
> static inline struct omap_desc
> *to_omap_dma_desc(struct dma_async_tx_descriptor *t)

That's a relatively unusual style.

The more common style is to keep the pointer on the same line
as the return type like:

static inline struct omap_desc *
to_omap_dma_desc(struct dma_async_tx_descriptor *t)

It's roughly 50:1 for the * on the 1st line.

> > @@ -334,7 +336,7 @@ static void omap_dma_stop(struct omap_chan *c)
> > if (val & (CCR_RD_ACTIVE | CCR_WR_ACTIVE))
> > dev_err(c->vc.chan.device->dev,
> > "DMA drain did not complete on lch %d\n",
> > - c->dma_ch);
> > + c->dma_ch);
> whats changes here? title says 80 char!

spacing, there's an 8 char space where there could be a tab.