[PATCH] staging: ccree: Fix lines longer than 80 characters

[PATCH] staging: ccree: Fix lines longer than 80 characters

[Stephen Brennan]

---
I'm new to kernel development and hoping to start with some simple changes to
get familiar with the process. Please let me know if there's anything I can do
to improve this very trivial patch!

 drivers/staging/ccree/ssi_pm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ccree/ssi_pm.c b/drivers/staging/ccree/ssi_pm.c
index 11bbdbeec22e..98ba9e918d2a 100644
--- a/drivers/staging/ccree/ssi_pm.c
+++ b/drivers/staging/ccree/ssi_pm.c
@@ -41,7 +41,9 @@ int ssi_power_mgr_runtime_suspend(struct device *dev)
 	int rc;
 
 	dev_dbg(dev, "set HOST_POWER_DOWN_EN\n");
-	WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), POWER_DOWN_ENABLE);
+	WRITE_REGISTER(
+		drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN),
+		POWER_DOWN_ENABLE);
 	rc = ssi_request_mgr_runtime_suspend_queue(drvdata);
 	if (rc != 0) {
 		dev_err(dev, "ssi_request_mgr_runtime_suspend_queue (%x)\n",
@@ -60,7 +62,9 @@ int ssi_power_mgr_runtime_resume(struct device *dev)
 		(struct ssi_drvdata *)dev_get_drvdata(dev);
 
 	dev_dbg(dev, "unset HOST_POWER_DOWN_EN\n");
-	WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), POWER_DOWN_DISABLE);
+	WRITE_REGISTER(
+		drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN),
+		POWER_DOWN_DISABLE);
 
 	rc = cc_clk_on(drvdata);
 	if (rc) {
-- 
2.14.2

Re: [PATCH] staging: ccree: Fix lines longer than 80 characters

[Stephen Brennan]

Hello,

Just bumping this patch. I know it's only a very trivial change that shuts
up a checkpatch warning. Please let me know if I can do anything to help.

Thanks,
Stephen

Re: [PATCH] staging: ccree: Fix lines longer than 80 characters

[Gilad Ben-Yossef]

Hello Stephen,

Thank you for your patch! sorry for not responding to your first post.
I seem to have missed that
email completely.

The CryptoCell driver is currently in the staging process for
inclusion in the Linux kernel.
As such, patches such as these are discussed in the staging mailing
list and should include
also the staging tree maintainer.

TIP: if you run the scripts/get_maintainers.pl script on your patch it
will tell you exactly which
list and which people your patch needs to be addressed, so you don't
have to guess.

Also, you are missing a patch description. While it's rather trivial
in this case, it still needs to be written.

TIP: Run the scripts/checkpath.pl on your patch before sending it and
it will let you know what you are missing, if any :-)

Please keep sending patches :-)

Thanks,
Gilad


On Fri, Oct 20, 2017 at 10:57 PM, Stephen Brennan <stephen@brennan.io> wrote:
> Hello,
>
> Just bumping this patch. I know it's only a very trivial change that shuts
> up a checkpatch warning. Please let me know if I can do anything to help.
>
> Thanks,
> Stephen
>



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

Re: [PATCH] staging: ccree: Fix lines longer than 80 characters

[Stephen Brennan]

Hi Gilad,

Thanks for the quick reply, I really appreciate your taking time to help a
newbie get started. I've made the appropriate changes and re-submitted.

> TIP: if you run the scripts/get_maintainers.pl script on your patch it
> will tell you exactly which
> list and which people your patch needs to be addressed, so you don't
> have to guess.

When I ran this tool, it listed out quite a few mailing lists, including
linux-kernel@vger.kernel.org. Is it correct to simply address your patch to
the whole list output by the script? I omitted linux-kernel on my
resubmission, simply to avoid contributing to the heavy volume of that
list, given how trivial this patch is.

Thanks again!
Stephen

Re: [PATCH] staging: ccree: Fix lines longer than 80 characters

[Gilad Ben-Yossef]

On Mon, Oct 23, 2017 at 6:00 PM, Stephen Brennan <stephen@brennan.io> wrote:
> Hi Gilad,
>
> Thanks for the quick reply, I really appreciate your taking time to help a
> newbie get started. I've made the appropriate changes and re-submitted.

It is completely my pleasure. Thanks,
>
>> TIP: if you run the scripts/get_maintainers.pl script on your patch it
>> will tell you exactly which
>> list and which people your patch needs to be addressed, so you don't
>> have to guess.
>
> When I ran this tool, it listed out quite a few mailing lists, including
> linux-kernel@vger.kernel.org. Is it correct to simply address your patch to
> the whole list output by the script? I omitted linux-kernel on my
> resubmission, simply to avoid contributing to the heavy volume of that
> list, given how trivial this patch is.
>

Strange as it may sound, it is the protocol. If you think you cringe
when sending
a trivial patch there wait till you send the 20th revision of a patch
that get_maintainer
says needs to be cross posted to half a dozen mailing lists... :-)

Posting to lkml is more than just informative. There are actually
automated tool that
will pick your patch and run it through a bunch of static analysers
tools and try to compile
it and sometime boot on a dozen different archs.

Besides, lkml is a fire host as it is... :-)

Gilad


> Thanks again!
> Stephen
>



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

Re: [PATCH] staging: ccree: Fix lines longer than 80 characters

[Stephen Brennan]

Apologies for the noise, this was the wrong patch. Please ignore this.

Re: [PATCH] staging: ccree: Fix lines longer than 80 characters

[Stephen Brennan]

Apologies for the noise, this was the wrong patch. Please ignore this.

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: ccree: Fix lines longer than 80 characters

[Stephen Brennan]

Simply break down some long lines and tab-indent them.

Signed-off-by: Stephen Brennan <stephen@brennan.io>
---
I'm learning the patch submission process, and this is my first patch. I know
it's trivial but I'm just trying to get my feet wet. Thanks in advance for
taking the time to review this!

 drivers/staging/ccree/ssi_pm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ccree/ssi_pm.c b/drivers/staging/ccree/ssi_pm.c
index 11bbdbeec22e..98ba9e918d2a 100644
--- a/drivers/staging/ccree/ssi_pm.c
+++ b/drivers/staging/ccree/ssi_pm.c
@@ -41,7 +41,9 @@ int ssi_power_mgr_runtime_suspend(struct device *dev)
 	int rc;
 
 	dev_dbg(dev, "set HOST_POWER_DOWN_EN\n");
-	WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), POWER_DOWN_ENABLE);
+	WRITE_REGISTER(
+		drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN),
+		POWER_DOWN_ENABLE);
 	rc = ssi_request_mgr_runtime_suspend_queue(drvdata);
 	if (rc != 0) {
 		dev_err(dev, "ssi_request_mgr_runtime_suspend_queue (%x)\n",
@@ -60,7 +62,9 @@ int ssi_power_mgr_runtime_resume(struct device *dev)
 		(struct ssi_drvdata *)dev_get_drvdata(dev);
 
 	dev_dbg(dev, "unset HOST_POWER_DOWN_EN\n");
-	WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), POWER_DOWN_DISABLE);
+	WRITE_REGISTER(
+		drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN),
+		POWER_DOWN_DISABLE);
 
 	rc = cc_clk_on(drvdata);
 	if (rc) {
-- 
2.14.2

[PATCH] staging: ccree: Fix lines longer than 80 characters

[Stephen Brennan]

Simply break down some long lines and tab-indent them.

Signed-off-by: Stephen Brennan <stephen@brennan.io>
---
I'm learning the patch submission process, and this is my first patch. I know
it's trivial but I'm just trying to get my feet wet. Thanks in advance for
taking the time to review this!

 drivers/staging/ccree/ssi_pm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ccree/ssi_pm.c b/drivers/staging/ccree/ssi_pm.c
index 11bbdbeec22e..98ba9e918d2a 100644
--- a/drivers/staging/ccree/ssi_pm.c
+++ b/drivers/staging/ccree/ssi_pm.c
@@ -41,7 +41,9 @@ int ssi_power_mgr_runtime_suspend(struct device *dev)
 	int rc;
 
 	dev_dbg(dev, "set HOST_POWER_DOWN_EN\n");
-	WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), POWER_DOWN_ENABLE);
+	WRITE_REGISTER(
+		drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN),
+		POWER_DOWN_ENABLE);
 	rc = ssi_request_mgr_runtime_suspend_queue(drvdata);
 	if (rc != 0) {
 		dev_err(dev, "ssi_request_mgr_runtime_suspend_queue (%x)\n",
@@ -60,7 +62,9 @@ int ssi_power_mgr_runtime_resume(struct device *dev)
 		(struct ssi_drvdata *)dev_get_drvdata(dev);
 
 	dev_dbg(dev, "unset HOST_POWER_DOWN_EN\n");
-	WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), POWER_DOWN_DISABLE);
+	WRITE_REGISTER(
+		drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN),
+		POWER_DOWN_DISABLE);
 
 	rc = cc_clk_on(drvdata);
 	if (rc) {
-- 
2.14.2


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ccree: Fix lines longer than 80 characters

[Tobin C. Harding]

On Tue, Oct 24, 2017 at 11:52:58PM -0700, Stephen Brennan wrote:
> Hi Gilad & Tobin,
> 
> > > Perhaps, if Stephen is willing, re-write the code to be more readable
> > > by, for example, using a temp
> > > variable for the register address, and in doing so both making the
> > > code more readable as well as
> > > treating the symptom?
> 
> I'm definitely willing; however, I don't know the hardware, so I'm already
> off to a bad start with questions like "which integer type is appropriate
> for the register address?"
> 
> > Oh, refactoring, that's a whole 'nother story. I'm a huge fan. And I agree with you that that is the
> > correct way to deal with 'line over 80' warnings. For me, it helped to get a few _trivial_
> > checkpatch fixes in before I moved onto refactoring. 'line over 80' warnings are great to fix if
> > viewed as an opportunity to refactor but not so great if viewed as a trivial white space fix to get
> > started with.
> 
> I think this makes the most sense right now. A pure trivial change is what
> I'd like, to help get some experience with the development process and
> working with the community. I'll look for something else (hopefully within
> the same staging driver, since you've all been super welcoming) and submit
> a patch for it. Hopefully I'll circle back for some slightly less trivial
> refactors after that!

When you want to link a few patches together into a series I wrote a
blog post a while back, check it out if you want some further guidance

http://tobin.cc/blog/kernel-dev-2/

Stick at it. There ain't nothin' to it but to do it

Good luck,
Tobin
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ccree: Fix lines longer than 80 characters

[Stephen Brennan]

Hi Gilad & Tobin,

> > Perhaps, if Stephen is willing, re-write the code to be more readable
> > by, for example, using a temp
> > variable for the register address, and in doing so both making the
> > code more readable as well as
> > treating the symptom?

I'm definitely willing; however, I don't know the hardware, so I'm already
off to a bad start with questions like "which integer type is appropriate
for the register address?"

> Oh, refactoring, that's a whole 'nother story. I'm a huge fan. And I agree with you that that is the
> correct way to deal with 'line over 80' warnings. For me, it helped to get a few _trivial_
> checkpatch fixes in before I moved onto refactoring. 'line over 80' warnings are great to fix if
> viewed as an opportunity to refactor but not so great if viewed as a trivial white space fix to get
> started with.

I think this makes the most sense right now. A pure trivial change is what
I'd like, to help get some experience with the development process and
working with the community. I'll look for something else (hopefully within
the same staging driver, since you've all been super welcoming) and submit
a patch for it. Hopefully I'll circle back for some slightly less trivial
refactors after that!

Thanks for your feedback!
Stephen

Re: [PATCH] staging: ccree: Fix lines longer than 80 characters

[Greg Kroah-Hartman]

On Tue, Oct 24, 2017 at 08:44:31PM +1100, Tobin C. Harding wrote:
> Removing from CC list:
> 
> - devel@driverdev.osuosl.org (this is the old address, it forwards to
>   driverdev-devel@linuxdriverproject.org now I believe). 

Both work equally well, and are interchangable for odd historical
reasons.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ccree: Fix lines longer than 80 characters

[Tobin C. Harding]

Removing from CC list:

- devel@driverdev.osuosl.org (this is the old address, it forwards to
  driverdev-devel@linuxdriverproject.org now I believe). 

- Linux Crypto Mailing List <linux-crypto@vger.kernel.org>, should we be spamming these guys with
  checkpatch fixing discussions? Please do correct me if this is not the correct etiquette, I am
  still learning also.

On Tue, Oct 24, 2017 at 11:49:41AM +0300, Gilad Ben-Yossef wrote:
> Hi Tobin,
> 
> On Tue, Oct 24, 2017 at 6:02 AM, Tobin C. Harding <me@tobin.cc> wrote:
> > On Mon, Oct 23, 2017 at 07:53:18AM -0700, Stephen Brennan wrote:
> >> Simply break down some long lines and tab-indent them.
> >
> > Hi Stephen,
> >
> > Welcome to the Linux kernel. Great that you have put in a patch, you are, however, unlikely to see
> > success fixing 'line over 80' warnings. There are a bunch of arguments for and against the line over
> > 80 limit, a web search will surely show them to you. The TL;DR is that it these changes do not
> > _really_ improve the readability of the code, they are just changes to quiet the static analysis
> > tool.
> 
> I completely agree with you that the end target is more readable code
> and that lines over 80 char is
> only a symptom but I do think in this case there is something useful to do.
> 
> Perhaps, if Stephen is willing, re-write the code to be more readable

Oh, refactoring, that's a whole 'nother story. I'm a huge fan. And I agree with you that that is the
correct way to deal with 'line over 80' warnings. For me, it helped to get a few _trivial_
checkpatch fixes in before I moved onto refactoring. 'line over 80' warnings are great to fix if
viewed as an opportunity to refactor but not so great if viewed as a trivial white space fix to get
started with.

thanks,
Tobin.

Re: [PATCH] staging: ccree: Fix lines longer than 80 characters

[Gilad Ben-Yossef]

Hi Tobin,

On Tue, Oct 24, 2017 at 6:02 AM, Tobin C. Harding <me@tobin.cc> wrote:
> On Mon, Oct 23, 2017 at 07:53:18AM -0700, Stephen Brennan wrote:
>> Simply break down some long lines and tab-indent them.
>
> Hi Stephen,
>
> Welcome to the Linux kernel. Great that you have put in a patch, you are, however, unlikely to see
> success fixing 'line over 80' warnings. There are a bunch of arguments for and against the line over
> 80 limit, a web search will surely show them to you. The TL;DR is that it these changes do not
> _really_ improve the readability of the code, they are just changes to quiet the static analysis
> tool.

I completely agree with you that the end target is more readable code
and that lines over 80 char is
only a symptom but I do think in this case there is something useful to do.

Perhaps, if Stephen is willing, re-write the code to be more readable
by, for example, using a temp
variable for the register address, and in doing so both making the
code more readable as well as
treating the symptom?

Thanks,
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

Re: [PATCH] staging: ccree: Fix lines longer than 80 characters

[Gilad Ben-Yossef]

Hi Tobin,

On Tue, Oct 24, 2017 at 6:02 AM, Tobin C. Harding <me@tobin.cc> wrote:
> On Mon, Oct 23, 2017 at 07:53:18AM -0700, Stephen Brennan wrote:
>> Simply break down some long lines and tab-indent them.
>
> Hi Stephen,
>
> Welcome to the Linux kernel. Great that you have put in a patch, you are, however, unlikely to see
> success fixing 'line over 80' warnings. There are a bunch of arguments for and against the line over
> 80 limit, a web search will surely show them to you. The TL;DR is that it these changes do not
> _really_ improve the readability of the code, they are just changes to quiet the static analysis
> tool.

I completely agree with you that the end target is more readable code
and that lines over 80 char is
only a symptom but I do think in this case there is something useful to do.

Perhaps, if Stephen is willing, re-write the code to be more readable
by, for example, using a temp
variable for the register address, and in doing so both making the
code more readable as well as
treating the symptom?

Thanks,
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: ccree: Fix lines longer than 80 characters

[Tobin C. Harding]

On Mon, Oct 23, 2017 at 07:53:18AM -0700, Stephen Brennan wrote:
> Simply break down some long lines and tab-indent them.

Hi Stephen,

Welcome to the Linux kernel. Great that you have put in a patch, you are, however, unlikely to see
success fixing 'line over 80' warnings. There are a bunch of arguments for and against the line over
80 limit, a web search will surely show them to you. The TL;DR is that it these changes do not
_really_ improve the readability of the code, they are just changes to quiet the static analysis
tool.

There are a bunch of other white space fixes that are more beneficial, perhaps you could wet your
feet with these (incorrect placement of parenthesis for example).

Good luck,
Tobin.

[PATCH] staging: ccree: Fix lines longer than 80 characters

[Stephen Brennan]

Simply break down some long lines and tab-indent them.

Signed-off-by: Stephen Brennan <stephen@brennan.io>
---
I'm learning the patch submission process, and this is my first patch. I know
it's trivial but I'm just trying to get my feet wet. Thanks in advance for
taking the time to review this!

 drivers/staging/ccree/ssi_pm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ccree/ssi_pm.c b/drivers/staging/ccree/ssi_pm.c
index 11bbdbeec22e..98ba9e918d2a 100644
--- a/drivers/staging/ccree/ssi_pm.c
+++ b/drivers/staging/ccree/ssi_pm.c
@@ -41,7 +41,9 @@ int ssi_power_mgr_runtime_suspend(struct device *dev)
 	int rc;
 
 	dev_dbg(dev, "set HOST_POWER_DOWN_EN\n");
-	WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), POWER_DOWN_ENABLE);
+	WRITE_REGISTER(
+		drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN),
+		POWER_DOWN_ENABLE);
 	rc = ssi_request_mgr_runtime_suspend_queue(drvdata);
 	if (rc != 0) {
 		dev_err(dev, "ssi_request_mgr_runtime_suspend_queue (%x)\n",
@@ -60,7 +62,9 @@ int ssi_power_mgr_runtime_resume(struct device *dev)
 		(struct ssi_drvdata *)dev_get_drvdata(dev);
 
 	dev_dbg(dev, "unset HOST_POWER_DOWN_EN\n");
-	WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), POWER_DOWN_DISABLE);
+	WRITE_REGISTER(
+		drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN),
+		POWER_DOWN_DISABLE);
 
 	rc = cc_clk_on(drvdata);
 	if (rc) {
-- 
2.14.2

[PATCH] staging: ccree: Fix lines longer than 80 characters

[Stephen Brennan]

Simply break down some long lines and tab-indent them.

Signed-off-by: Stephen Brennan <stephen@brennan.io>
---
I'm learning the patch submission process, and this is my first patch. I know
it's trivial but I'm just trying to get my feet wet. Thanks in advance for
taking the time to review this!

 drivers/staging/ccree/ssi_pm.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ccree/ssi_pm.c b/drivers/staging/ccree/ssi_pm.c
index 11bbdbeec22e..98ba9e918d2a 100644
--- a/drivers/staging/ccree/ssi_pm.c
+++ b/drivers/staging/ccree/ssi_pm.c
@@ -41,7 +41,9 @@ int ssi_power_mgr_runtime_suspend(struct device *dev)
 	int rc;
 
 	dev_dbg(dev, "set HOST_POWER_DOWN_EN\n");
-	WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), POWER_DOWN_ENABLE);
+	WRITE_REGISTER(
+		drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN),
+		POWER_DOWN_ENABLE);
 	rc = ssi_request_mgr_runtime_suspend_queue(drvdata);
 	if (rc != 0) {
 		dev_err(dev, "ssi_request_mgr_runtime_suspend_queue (%x)\n",
@@ -60,7 +62,9 @@ int ssi_power_mgr_runtime_resume(struct device *dev)
 		(struct ssi_drvdata *)dev_get_drvdata(dev);
 
 	dev_dbg(dev, "unset HOST_POWER_DOWN_EN\n");
-	WRITE_REGISTER(drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN), POWER_DOWN_DISABLE);
+	WRITE_REGISTER(
+		drvdata->cc_base + CC_REG_OFFSET(HOST_RGF, HOST_POWER_DOWN_EN),
+		POWER_DOWN_DISABLE);
 
 	rc = cc_clk_on(drvdata);
 	if (rc) {
-- 
2.14.2


_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel