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

[Stephen Brennan]

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

[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 
---
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

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

[Gilad Ben-Yossef]

On Mon, Oct 23, 2017 at 6:00 PM, Stephen Brennan  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-ker...@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

[Gilad Ben-Yossef]

Hi Tobin,

On Tue, Oct 24, 2017 at 6:02 AM, Tobin C. Harding  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

[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.

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-ker...@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

[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 
---
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

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  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]

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

[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