Skip to contentSkip to conversation

staging: mt7621-dma: Add braces around else branches

PATCH Superseded Kimberly Brown 5 participants 9 messages Oct 23, 2018 – Oct 24, 2018

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 staging: mt7621-dma: Add braces around else branches

Add braces around else branches in conditional statements that include branches with multiple statements. This style complies with the Linux kernel coding style and improves consistency and readability. Issues found by checkpatch.

---
 drivers/staging/mt7621-dma/mtk-hsdma.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/staging/mt7621-dma/mtk-hsdma.c +4 -2
index df6ebf41bdea..35556f024aa1 100644
@@ -418,8 +418,9 @@ static void mtk_hsdma_chan_done(struct mtk_hsdam_engine *hsdma,
 			vchan_cookie_complete(&desc->vdesc);
 			chan_issued = gdma_next_desc(chan);
 		}
-	} else
+	} else {
 		dev_dbg(hsdma->ddev.dev, "no desc to complete\n");
+	}
 
 	if (chan_issued)
 		set_bit(chan->id, &hsdma->chan_issued);
@@ -456,8 +457,9 @@ static void mtk_hsdma_issue_pending(struct dma_chan *c)
 		if (gdma_next_desc(chan)) {
 			set_bit(chan->id, &hsdma->chan_issued);
 			tasklet_schedule(&hsdma->task);
-		} else
+		} else {
 			dev_dbg(hsdma->ddev.dev, "no desc to issue\n");
+		}
 	}
 	spin_unlock_bh(&chan->vchan.lock);
 }
Signature · 1 line
2.17.1
maintainer Reviewed-by on v1 1/1
#1 lore raw
Quoted 9 lines · Kimberly Brown

Add braces around else branches in conditional statements that include branches with multiple statements. This style complies with the Linux kernel coding style and improves consistency and readability. Issues found by checkpatch.

---
 drivers/staging/mt7621-dma/mtk-hsdma.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Looks good to me, thanks!

Quoted 28 lines
drivers/staging/mt7621-dma/mtk-hsdma.c +4 -2
index df6ebf41bdea..35556f024aa1 100644
@@ -418,8 +418,9 @@ static void mtk_hsdma_chan_done(struct mtk_hsdam_engine *hsdma,
 			vchan_cookie_complete(&desc->vdesc);
 			chan_issued = gdma_next_desc(chan);
 		}
-	} else
+	} else {
 		dev_dbg(hsdma->ddev.dev, "no desc to complete\n");
+	}
 
 	if (chan_issued)
 		set_bit(chan->id, &hsdma->chan_issued);
@@ -456,8 +457,9 @@ static void mtk_hsdma_issue_pending(struct dma_chan *c)
 		if (gdma_next_desc(chan)) {
 			set_bit(chan->id, &hsdma->chan_issued);
 			tasklet_schedule(&hsdma->task);
-		} else
+		} else {
 			dev_dbg(hsdma->ddev.dev, "no desc to issue\n");
+		}
 	}
 	spin_unlock_bh(&chan->vchan.lock);
 }
maintainer
#2 lore raw
Quoted 8 lines · Matthias Brugger
Quoted 5 lines · Kimberly Brown

Add braces around else branches in conditional statements that include branches with multiple statements. This style complies with the Linux kernel coding style and improves consistency and readability. Issues found by checkpatch.

Quoted 8 lines
Quoted 4 lines
---
 drivers/staging/mt7621-dma/mtk-hsdma.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Looks good to me, thanks!

Hm, just one thing that I see right now. Can you please use ./scripts/get_maintainer.pl
to get the whole list of persons and mailinglists that you should your patch to. Otherwise people might not be able to follow your patches or the relevant persons will not notice.

Regards,
Matthias

Quoted 28 lines
Quoted 28 lines
drivers/staging/mt7621-dma/mtk-hsdma.c +4 -2
index df6ebf41bdea..35556f024aa1 100644
@@ -418,8 +418,9 @@ static void mtk_hsdma_chan_done(struct mtk_hsdam_engine *hsdma,
 			vchan_cookie_complete(&desc->vdesc);
 			chan_issued = gdma_next_desc(chan);
 		}
-	} else
+	} else {
 		dev_dbg(hsdma->ddev.dev, "no desc to complete\n");
+	}
 
 	if (chan_issued)
 		set_bit(chan->id, &hsdma->chan_issued);
@@ -456,8 +457,9 @@ static void mtk_hsdma_issue_pending(struct dma_chan *c)
 		if (gdma_next_desc(chan)) {
 			set_bit(chan->id, &hsdma->chan_issued);
 			tasklet_schedule(&hsdma->task);
-		} else
+		} else {
 			dev_dbg(hsdma->ddev.dev, "no desc to issue\n");
+		}
 	}
 	spin_unlock_bh(&chan->vchan.lock);
 }
#3 lore raw

PATCH v2 1/1 staging: mt7621-dma: Add braces around else branches

In reply to Kimberly Brown

Add braces around else branches in conditional statements that include branches with multiple statements. This style complies with the Linux kernel coding style and improves consistency and readability. Issues found by checkpatch.

---
Changes in v2:
- Added people and mailing lists from get_maintainer.pl to the CC list
- Added Reviewed-by line

 drivers/staging/mt7621-dma/mtk-hsdma.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/staging/mt7621-dma/mtk-hsdma.c +4 -2
index df6ebf41bdea..35556f024aa1 100644
@@ -418,8 +418,9 @@ static void mtk_hsdma_chan_done(struct mtk_hsdam_engine *hsdma,
 			vchan_cookie_complete(&desc->vdesc);
 			chan_issued = gdma_next_desc(chan);
 		}
-	} else
+	} else {
 		dev_dbg(hsdma->ddev.dev, "no desc to complete\n");
+	}
 
 	if (chan_issued)
 		set_bit(chan->id, &hsdma->chan_issued);
@@ -456,8 +457,9 @@ static void mtk_hsdma_issue_pending(struct dma_chan *c)
 		if (gdma_next_desc(chan)) {
 			set_bit(chan->id, &hsdma->chan_issued);
 			tasklet_schedule(&hsdma->task);
-		} else
+		} else {
 			dev_dbg(hsdma->ddev.dev, "no desc to issue\n");
+		}
 	}
 	spin_unlock_bh(&chan->vchan.lock);
 }
Signature · 1 line
2.17.1
maintainer
#4 lore raw

PATCH v2 1/1 staging: mt7621-dma: Add braces around else branches

Hi Kiberly,

Thanks for adding all the emails in CC.
I would encourage you for your next patch to distinguish between CC and TO. You should send your patch TO important maintainers in the get_maintainers.pl list (as default, to all of them). If there is someone you really want to look into the patch, then add him/her in TO as well.

Put the rest (people and mailing lists) in CC. Why? Some people filter their mails so that they can concentrate on the mails they got send directly and look on mails they are in CC with lower priority (maybe not at all, because there are too much?). So it is important to have the maintainers in the TO list and not in CC.

Keep up the good work :)
Matthias

Quoted 42 lines · Kimberly Brown

Add braces around else branches in conditional statements that include branches with multiple statements. This style complies with the Linux kernel coding style and improves consistency and readability. Issues found by checkpatch.

---
Changes in v2:
- Added people and mailing lists from get_maintainer.pl to the CC list
- Added Reviewed-by line

 drivers/staging/mt7621-dma/mtk-hsdma.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/staging/mt7621-dma/mtk-hsdma.c +4 -2
index df6ebf41bdea..35556f024aa1 100644
@@ -418,8 +418,9 @@ static void mtk_hsdma_chan_done(struct mtk_hsdam_engine *hsdma,
 			vchan_cookie_complete(&desc->vdesc);
 			chan_issued = gdma_next_desc(chan);
 		}
-	} else
+	} else {
 		dev_dbg(hsdma->ddev.dev, "no desc to complete\n");
+	}
 
 	if (chan_issued)
 		set_bit(chan->id, &hsdma->chan_issued);
@@ -456,8 +457,9 @@ static void mtk_hsdma_issue_pending(struct dma_chan *c)
 		if (gdma_next_desc(chan)) {
 			set_bit(chan->id, &hsdma->chan_issued);
 			tasklet_schedule(&hsdma->task);
-		} else
+		} else {
 			dev_dbg(hsdma->ddev.dev, "no desc to issue\n");
+		}
 	}
 	spin_unlock_bh(&chan->vchan.lock);
 }
#5 lore raw

PATCH v2 1/1 staging: mt7621-dma: Add braces around else branches

Quoted 8 lines · Matthias Brugger

Hi Kiberly,

Thanks for adding all the emails in CC.
I would encourage you for your next patch to distinguish between CC and TO. You should send your patch TO important maintainers in the get_maintainers.pl list (as default, to all of them). If there is someone you really want to look into the patch, then add him/her in TO as well.

Quoted 4 lines

Put the rest (people and mailing lists) in CC. Why? Some people filter their mails so that they can concentrate on the mails they got send directly and look on mails they are in CC with lower priority (maybe not at all, because there are too much?). So it is important to have the maintainers in the TO list and not in CC.

+1

I'm glad that there's someone else in the Linux community that agrees with me on this point, and is willing to speak out about it.

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
#6 lore raw
Quoted 14 lines · Russell King - ARM Linux
Quoted 8 lines · Matthias Brugger

Hi Kiberly,

Thanks for adding all the emails in CC.
I would encourage you for your next patch to distinguish between CC and TO. You should send your patch TO important maintainers in the get_maintainers.pl list (as default, to all of them). If there is someone you really want to look into the patch, then add him/her in TO as well.

Quoted 4 lines

Put the rest (people and mailing lists) in CC. Why? Some people filter their mails so that they can concentrate on the mails they got send directly and look on mails they are in CC with lower priority (maybe not at all, because there are too much?). So it is important to have the maintainers in the TO list and not in CC.

Quoted 4 lines

+1

I'm glad that there's someone else in the Linux community that agrees with me on this point, and is willing to speak out about it.

If it's an important point, perhaps it should be mentioned in submitting-patches.rst? There is a mention of the Cc tag, but no indication of who to put in CC.

julia

#7 lore raw
Quoted 18 lines · Julia Lawall
Quoted 14 lines · Russell King - ARM Linux
Quoted 12 lines · Matthias Brugger

Hi Kiberly,

Thanks for adding all the emails in CC.
I would encourage you for your next patch to distinguish between CC and TO. You should send your patch TO important maintainers in the get_maintainers.pl list (as default, to all of them). If there is someone you really want to look into the patch, then add him/her in TO as well.

Put the rest (people and mailing lists) in CC. Why? Some people filter their mails so that they can concentrate on the mails they got send directly and look on mails they are in CC with lower priority (maybe not at all, because there are too much?). So it is important to have the maintainers in the TO list and not in CC.

Quoted 8 lines
Quoted 4 lines

+1

I'm glad that there's someone else in the Linux community that agrees with me on this point, and is willing to speak out about it.

If it's an important point, perhaps it should be mentioned in submitting-patches.rst? There is a mention of the Cc tag, but no indication of who to put in CC.

submitting-patches.rst talks about the Cc tag in the commit, not the To or Cc in the email client.

In any case, there's a lot of personal issues here: most kernel developers don't care whether they're in the To or Cc header of an email, but there are some who do use it as Matthias says - which is actually the long-standing definition of these headers.

Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
#8 lore raw

PATCH v2 1/1 staging: mt7621-dma: Add braces around else branches

In reply to Matthias Brugger
Quoted 8 lines · Matthias Brugger

Hi Kiberly,

Thanks for adding all the emails in CC.
I would encourage you for your next patch to distinguish between CC and TO. You should send your patch TO important maintainers in the get_maintainers.pl list (as default, to all of them). If there is someone you really want to look into the patch, then add him/her in TO as well.

I normally just add the first person from get_maintainer.pl to the To:. It's basically random I think. The rest I put in the CC. This is a staging patch so no one is very picky.

There isn't a Fixes tag for this but if there were I add the person who wrote the original patch to the To header. Hopefully, they review and Ack my patch. I feel like that's very important.

regards,
dan carpenter