Re: [PATCH] staging: ced1401: fix coding style in ced_ioc.c (resend)
On Wed, Jan 15, 2014 at 10:37:13PM +0100, Pol Eyschen wrote:
> Fixed checkpatch.pl issues and removed redundant comment in ced_ioc.cs
>
> Signed-off-by: Pol Eyschen
> ---
> drivers/staging/ced1401/ced_ioc.c | 483
> +
> 1 file changed, 271 insertions(+), 212 deletions(-)
>
> diff --git a/drivers/staging/ced1401/ced_ioc.c
> b/drivers/staging/ced1401/ced_ioc.c
> index bf532b1..94a361d 100644
> --- a/drivers/staging/ced1401/ced_ioc.c
> +++ b/drivers/staging/ced1401/ced_ioc.c
> @@ -40,8 +40,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
> {
> dev_dbg(>interface->dev, "%s currentState=%d", __func__,
> pdx->sCurrentState);
> - if (pdx->sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
> trouble */
> - return;
> + if (pdx->sCurrentState == U14ERR_TIME)
> + return; /* Do nothing if hardware in trouble */
That's really not a good cleanup, is it?
And this patch no longer applies to my tree, care to fix it up and
resend?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: ced1401: fix coding style in ced_ioc.c (resend)
On Wed, Jan 15, 2014 at 10:37:13PM +0100, Pol Eyschen wrote:
Fixed checkpatch.pl issues and removed redundant comment in ced_ioc.cs
Signed-off-by: Pol Eyschen poleysc...@gmail.com
---
drivers/staging/ced1401/ced_ioc.c | 483
+
1 file changed, 271 insertions(+), 212 deletions(-)
diff --git a/drivers/staging/ced1401/ced_ioc.c
b/drivers/staging/ced1401/ced_ioc.c
index bf532b1..94a361d 100644
--- a/drivers/staging/ced1401/ced_ioc.c
+++ b/drivers/staging/ced1401/ced_ioc.c
@@ -40,8 +40,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(pdx-interface-dev, %s currentState=%d, __func__,
pdx-sCurrentState);
- if (pdx-sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
- return;
+ if (pdx-sCurrentState == U14ERR_TIME)
+ return; /* Do nothing if hardware in trouble */
That's really not a good cleanup, is it?
And this patch no longer applies to my tree, care to fix it up and
resend?
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] staging: ced1401: fix coding style in ced_ioc.c (resend)
Fixed checkpatch.pl issues and removed redundant comment in ced_ioc.cs
Signed-off-by: Pol Eyschen
---
drivers/staging/ced1401/ced_ioc.c | 483 +
1 file changed, 271 insertions(+), 212 deletions(-)
diff --git a/drivers/staging/ced1401/ced_ioc.c
b/drivers/staging/ced1401/ced_ioc.c
index bf532b1..94a361d 100644
--- a/drivers/staging/ced1401/ced_ioc.c
+++ b/drivers/staging/ced1401/ced_ioc.c
@@ -40,8 +40,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(>interface->dev, "%s currentState=%d", __func__,
pdx->sCurrentState);
- if (pdx->sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
- return;
+ if (pdx->sCurrentState == U14ERR_TIME)
+ return; /* Do nothing if hardware in trouble */
/* Kill off any pending I/O */
/* CharSend_Cancel(pdx); */
spin_lock_irq(>charOutLock);
@@ -61,8 +61,8 @@ static void FlushInBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(>interface->dev, "%s currentState=%d", __func__,
pdx->sCurrentState);
- if (pdx->sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
- return;
+ if (pdx->sCurrentState == U14ERR_TIME)
+ return; /* Do nothing if hardware in trouble */
/* Kill off any pending I/O */
/* CharRead_Cancel(pDevObject); */
spin_lock_irq(>charInLock);
@@ -82,7 +82,7 @@ static int PutChars(DEVICE_EXTENSION *pdx, const char *pCh,
unsigned int uCount)
{
int iReturn;
- spin_lock_irq(>charOutLock); /* get the output spin lock */
+ spin_lock_irq(>charOutLock); /* get the output spin lock */
if ((OUTBUF_SZ - pdx->dwNumOutput) >= uCount) {
unsigned int u;
for (u = 0; u < uCount; u++) {
@@ -92,9 +92,10 @@ static int PutChars(DEVICE_EXTENSION *pdx, const char *pCh,
}
pdx->dwNumOutput += uCount;
spin_unlock_irq(>charOutLock);
- iReturn = SendChars(pdx); /* ...give a chance to
transmit data */
+ /* give a chance to transmit data */
+ iReturn = SendChars(pdx);
} else {
- iReturn = U14ERR_NOOUT; /* no room at the out (ha-ha) */
+ iReturn = U14ERR_NOOUT; /* no room at the out (ha-ha) */
spin_unlock_irq(>charOutLock);
}
return iReturn;
@@ -108,16 +109,17 @@ static int PutChars(DEVICE_EXTENSION *pdx, const char
*pCh,
int SendString(DEVICE_EXTENSION *pdx, const char __user *pData,
unsigned int n)
{
- int iReturn = U14ERR_NOERROR; /* assume all will be well */
- char buffer[OUTBUF_SZ + 1]; /* space in our address space for
characters */
- if (n > OUTBUF_SZ) /* check space in local buffer... */
- return U14ERR_NOOUT;/* ...too many characters */
+ int iReturn = U14ERR_NOERROR;
+ /* space in our address space for characters */
+ char buffer[OUTBUF_SZ + 1];
+ if (n > OUTBUF_SZ)
+ return U14ERR_NOOUT;
if (copy_from_user(buffer, pData, n))
return -EFAULT;
buffer[n] = 0; /* terminate for debug purposes */
mutex_lock(>io_mutex); /* Protect disconnect from new i/o */
- if (n > 0) {/* do nothing if nowt to do! */
+ if (n > 0) {
dev_dbg(>interface->dev, "%s n=%d>%s<", __func__, n,
buffer);
iReturn = PutChars(pdx, buffer, n);
@@ -183,7 +185,7 @@ int Get1401State(DEVICE_EXTENSION *pdx, __u32 *state, __u32
*error)
if (nGot != sizeof(pdx->statBuf)) {
dev_err(>interface->dev,
"Get1401State() FAILED, return code %d", nGot);
- pdx->sCurrentState = U14ERR_TIME; /* Indicate that
things are very wrong indeed */
+ pdx->sCurrentState = U14ERR_TIME;
*state = 0; /* Force status values to a known state */
*error = 0;
} else {
@@ -192,16 +194,20 @@ int Get1401State(DEVICE_EXTENSION *pdx, __u32 *state,
__u32 *error)
"Get1401State() Success, state: 0x%x, 0x%x",
pdx->statBuf[0], pdx->statBuf[1]);
- *state = pdx->statBuf[0]; /* Return the state values to
the calling code */
+ /* Return the state values to the calling code */
+ *state = pdx->statBuf[0];
*error = pdx->statBuf[1];
- nDevice = pdx->udev->descriptor.bcdDevice >> 8; /* 1401 type
code value */
- switch (nDevice) { /* so we can clean up current state */
+ /* 1401 type code value */
+ nDevice = pdx->udev->descriptor.bcdDevice >> 8;
+ /* so we can clean up current state */
+
[PATCH] staging: ced1401: fix coding style in ced_ioc.c (resend)
Fixed checkpatch.pl issues and removed redundant comment in ced_ioc.cs
Signed-off-by: Pol Eyschen poleysc...@gmail.com
---
drivers/staging/ced1401/ced_ioc.c | 483 +
1 file changed, 271 insertions(+), 212 deletions(-)
diff --git a/drivers/staging/ced1401/ced_ioc.c
b/drivers/staging/ced1401/ced_ioc.c
index bf532b1..94a361d 100644
--- a/drivers/staging/ced1401/ced_ioc.c
+++ b/drivers/staging/ced1401/ced_ioc.c
@@ -40,8 +40,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(pdx-interface-dev, %s currentState=%d, __func__,
pdx-sCurrentState);
- if (pdx-sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
- return;
+ if (pdx-sCurrentState == U14ERR_TIME)
+ return; /* Do nothing if hardware in trouble */
/* Kill off any pending I/O */
/* CharSend_Cancel(pdx); */
spin_lock_irq(pdx-charOutLock);
@@ -61,8 +61,8 @@ static void FlushInBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(pdx-interface-dev, %s currentState=%d, __func__,
pdx-sCurrentState);
- if (pdx-sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
- return;
+ if (pdx-sCurrentState == U14ERR_TIME)
+ return; /* Do nothing if hardware in trouble */
/* Kill off any pending I/O */
/* CharRead_Cancel(pDevObject); */
spin_lock_irq(pdx-charInLock);
@@ -82,7 +82,7 @@ static int PutChars(DEVICE_EXTENSION *pdx, const char *pCh,
unsigned int uCount)
{
int iReturn;
- spin_lock_irq(pdx-charOutLock); /* get the output spin lock */
+ spin_lock_irq(pdx-charOutLock); /* get the output spin lock */
if ((OUTBUF_SZ - pdx-dwNumOutput) = uCount) {
unsigned int u;
for (u = 0; u uCount; u++) {
@@ -92,9 +92,10 @@ static int PutChars(DEVICE_EXTENSION *pdx, const char *pCh,
}
pdx-dwNumOutput += uCount;
spin_unlock_irq(pdx-charOutLock);
- iReturn = SendChars(pdx); /* ...give a chance to
transmit data */
+ /* give a chance to transmit data */
+ iReturn = SendChars(pdx);
} else {
- iReturn = U14ERR_NOOUT; /* no room at the out (ha-ha) */
+ iReturn = U14ERR_NOOUT; /* no room at the out (ha-ha) */
spin_unlock_irq(pdx-charOutLock);
}
return iReturn;
@@ -108,16 +109,17 @@ static int PutChars(DEVICE_EXTENSION *pdx, const char
*pCh,
int SendString(DEVICE_EXTENSION *pdx, const char __user *pData,
unsigned int n)
{
- int iReturn = U14ERR_NOERROR; /* assume all will be well */
- char buffer[OUTBUF_SZ + 1]; /* space in our address space for
characters */
- if (n OUTBUF_SZ) /* check space in local buffer... */
- return U14ERR_NOOUT;/* ...too many characters */
+ int iReturn = U14ERR_NOERROR;
+ /* space in our address space for characters */
+ char buffer[OUTBUF_SZ + 1];
+ if (n OUTBUF_SZ)
+ return U14ERR_NOOUT;
if (copy_from_user(buffer, pData, n))
return -EFAULT;
buffer[n] = 0; /* terminate for debug purposes */
mutex_lock(pdx-io_mutex); /* Protect disconnect from new i/o */
- if (n 0) {/* do nothing if nowt to do! */
+ if (n 0) {
dev_dbg(pdx-interface-dev, %s n=%d%s, __func__, n,
buffer);
iReturn = PutChars(pdx, buffer, n);
@@ -183,7 +185,7 @@ int Get1401State(DEVICE_EXTENSION *pdx, __u32 *state, __u32
*error)
if (nGot != sizeof(pdx-statBuf)) {
dev_err(pdx-interface-dev,
Get1401State() FAILED, return code %d, nGot);
- pdx-sCurrentState = U14ERR_TIME; /* Indicate that
things are very wrong indeed */
+ pdx-sCurrentState = U14ERR_TIME;
*state = 0; /* Force status values to a known state */
*error = 0;
} else {
@@ -192,16 +194,20 @@ int Get1401State(DEVICE_EXTENSION *pdx, __u32 *state,
__u32 *error)
Get1401State() Success, state: 0x%x, 0x%x,
pdx-statBuf[0], pdx-statBuf[1]);
- *state = pdx-statBuf[0]; /* Return the state values to
the calling code */
+ /* Return the state values to the calling code */
+ *state = pdx-statBuf[0];
*error = pdx-statBuf[1];
- nDevice = pdx-udev-descriptor.bcdDevice 8; /* 1401 type
code value */
- switch (nDevice) { /* so we can clean up current state */
+ /* 1401 type code value */
+ nDevice = pdx-udev-descriptor.bcdDevice 8;
+ /* so we can clean up current state */
+
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On Mon, 2014-01-13 at 15:56 -0800, Greg KH wrote:
> On Mon, Jan 13, 2014 at 08:41:40PM +0100, Pol Eyschen wrote:
[]
> > diff --git a/drivers/staging/ced1401/ced_ioc.c
> > b/drivers/staging/ced1401/ced_ioc.c
[]
> > @@ -41,7 +41,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
> > {
> > dev_dbg(>interface->dev, "%s currentState=%d", __func__,
> > pdx->sCurrentState);
> > - if (pdx->sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
> > trouble */
> > + if (pdx->sCurrentState == U14ERR_TIME)
> > + /* Do nothing if hardware in trouble */
> > return;
>
> Please wrap stuff like this in { } to not confuse people.
Yes please, or maybe put the comment on the return line like:
if (pdx->sCurrentState == U14ERR_TIME)
return; /* Do nothing if hardware in trouble */
or above the test like:
/* Do nothing if hardware in trouble */
if (pdx->sCurrentState == U14ERR_TIME)
return;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On Mon, Jan 13, 2014 at 08:41:40PM +0100, Pol Eyschen wrote:
> From: Pol Eyschen
>
> All comments fixed to match the kernel coding style.
>
> Signed-off-by: Pol Eyschen
> ---
> drivers/staging/ced1401/ced_ioc.c | 382
> -
> 1 file changed, 249 insertions(+), 133 deletions(-)
>
> diff --git a/drivers/staging/ced1401/ced_ioc.c
> b/drivers/staging/ced1401/ced_ioc.c
> index 62efd74..e5172cb 100644
> --- a/drivers/staging/ced1401/ced_ioc.c
> +++ b/drivers/staging/ced1401/ced_ioc.c
> @@ -41,7 +41,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
> {
> dev_dbg(>interface->dev, "%s currentState=%d", __func__,
> pdx->sCurrentState);
> - if (pdx->sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
> trouble */
> + if (pdx->sCurrentState == U14ERR_TIME)
> + /* Do nothing if hardware in trouble */
> return;
Please wrap stuff like this in { } to not confuse people.
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On Mon, Jan 13, 2014 at 11:31:29PM +0100, Pol Eyschen wrote: > > On 13.01.2014, at 22:44, Dan Carpenter wrote: > > > On Mon, Jan 13, 2014 at 09:35:53PM +, Mark Einon wrote: > >> On Mon, Jan 13, 2014 at 08:41:40PM +0100, Pol Eyschen wrote: > >>> From: Pol Eyschen > >>> > >>> All comments fixed to match the kernel coding style. > >>> > >>> Signed-off-by: Pol Eyschen > >>> --- > >> > >> This patch doesn't apply to my staging-next branch. Are you making your > >> changes to the staging-next branch of > >> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git? > >> > > I cloned > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git and > made my changes to that branch. That is Linus's tree, not "linux-next" which we all develop against. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On Mon, Jan 13, 2014 at 11:31:29PM +0100, Pol Eyschen wrote: > > > I took the original comments that were there, and just formatted them > to be in order with the coding style, I didn't add or remove > anything. This is my first patch so I didn't want to mess too much > with what was there before, even if the comments sometimes seemed > superfluous to me. No problem, everyone's first patch is rejected. Just fix it up in line with the review comments and resend. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On 13.01.2014, at 22:44, Dan Carpenter wrote:
> On Mon, Jan 13, 2014 at 09:35:53PM +, Mark Einon wrote:
>> On Mon, Jan 13, 2014 at 08:41:40PM +0100, Pol Eyschen wrote:
>>> From: Pol Eyschen
>>>
>>> All comments fixed to match the kernel coding style.
>>>
>>> Signed-off-by: Pol Eyschen
>>> ---
>>
>> This patch doesn't apply to my staging-next branch. Are you making your
>> changes to the staging-next branch of
>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git?
>>
I cloned git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git and
made my changes to that branch.
>>> drivers/staging/ced1401/ced_ioc.c | 382
>>> -
>>> 1 file changed, 249 insertions(+), 133 deletions(-)
>>>
>>> diff --git a/drivers/staging/ced1401/ced_ioc.c
>>> b/drivers/staging/ced1401/ced_ioc.c
>>> index 62efd74..e5172cb 100644
>>> --- a/drivers/staging/ced1401/ced_ioc.c
>>> +++ b/drivers/staging/ced1401/ced_ioc.c
>>> @@ -41,7 +41,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
>>> {
>>> dev_dbg(>interface->dev, "%s currentState=%d", __func__,
>>> pdx->sCurrentState);
>>> - if (pdx->sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
>>> trouble */
>>> + if (pdx->sCurrentState == U14ERR_TIME)
>>> + /* Do nothing if hardware in trouble */
>>> return;
>>
>> Putting a comment after a single line 'if' and before the statement is
>> not the norm and can be confusing. Please don't do it.
>>
> The guideline is that multi-line indents should get {} braces for
> readability even though they aren't needed for syntax.
>
> if (pdx->sCurrentState == U14ERR_TIME) {
> /* Do nothing if hardware in trouble */
> return;
> }
>
I took the original comments that were there, and just formatted them to be in
order with the coding style, I didn't add or remove anything. This is my first
patch so I didn't want to mess too much with what was there before, even if the
comments sometimes seemed superfluous to me.
Pol
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On Mon, Jan 13, 2014 at 09:35:53PM +, Mark Einon wrote:
> On Mon, Jan 13, 2014 at 08:41:40PM +0100, Pol Eyschen wrote:
> > From: Pol Eyschen
> >
> > All comments fixed to match the kernel coding style.
> >
> > Signed-off-by: Pol Eyschen
> > ---
>
> This patch doesn't apply to my staging-next branch. Are you making your
> changes to the staging-next branch of
> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git?
>
> > drivers/staging/ced1401/ced_ioc.c | 382
> > -
> > 1 file changed, 249 insertions(+), 133 deletions(-)
> >
> > diff --git a/drivers/staging/ced1401/ced_ioc.c
> > b/drivers/staging/ced1401/ced_ioc.c
> > index 62efd74..e5172cb 100644
> > --- a/drivers/staging/ced1401/ced_ioc.c
> > +++ b/drivers/staging/ced1401/ced_ioc.c
> > @@ -41,7 +41,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
> > {
> > dev_dbg(>interface->dev, "%s currentState=%d", __func__,
> > pdx->sCurrentState);
> > - if (pdx->sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
> > trouble */
> > + if (pdx->sCurrentState == U14ERR_TIME)
> > + /* Do nothing if hardware in trouble */
> > return;
>
> Putting a comment after a single line 'if' and before the statement is
> not the norm and can be confusing. Please don't do it.
>
The guideline is that multi-line indents should get {} braces for
readability even though they aren't needed for syntax.
if (pdx->sCurrentState == U14ERR_TIME) {
/* Do nothing if hardware in trouble */
return;
}
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On Mon, Jan 13, 2014 at 08:41:40PM +0100, Pol Eyschen wrote:
> From: Pol Eyschen
>
> All comments fixed to match the kernel coding style.
>
> Signed-off-by: Pol Eyschen
> ---
This patch doesn't apply to my staging-next branch. Are you making your
changes to the staging-next branch of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git?
> drivers/staging/ced1401/ced_ioc.c | 382
> -
> 1 file changed, 249 insertions(+), 133 deletions(-)
>
> diff --git a/drivers/staging/ced1401/ced_ioc.c
> b/drivers/staging/ced1401/ced_ioc.c
> index 62efd74..e5172cb 100644
> --- a/drivers/staging/ced1401/ced_ioc.c
> +++ b/drivers/staging/ced1401/ced_ioc.c
> @@ -41,7 +41,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
> {
> dev_dbg(>interface->dev, "%s currentState=%d", __func__,
> pdx->sCurrentState);
> - if (pdx->sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
> trouble */
> + if (pdx->sCurrentState == U14ERR_TIME)
> + /* Do nothing if hardware in trouble */
> return;
Putting a comment after a single line 'if' and before the statement is
not the norm and can be confusing. Please don't do it.
Also, quite a few of the comments you're moving around here look to be
unecessary, and can probably be deleted.
Mark
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Staging: ced1401: fix coding style in ced_ioc.c
From: Pol Eyschen
All comments fixed to match the kernel coding style.
Signed-off-by: Pol Eyschen
---
drivers/staging/ced1401/ced_ioc.c | 382 -
1 file changed, 249 insertions(+), 133 deletions(-)
diff --git a/drivers/staging/ced1401/ced_ioc.c
b/drivers/staging/ced1401/ced_ioc.c
index 62efd74..e5172cb 100644
--- a/drivers/staging/ced1401/ced_ioc.c
+++ b/drivers/staging/ced1401/ced_ioc.c
@@ -41,7 +41,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(>interface->dev, "%s currentState=%d", __func__,
pdx->sCurrentState);
- if (pdx->sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
+ if (pdx->sCurrentState == U14ERR_TIME)
+ /* Do nothing if hardware in trouble */
return;
/* Kill off any pending I/O */
/* CharSend_Cancel(pdx); */
@@ -62,7 +63,8 @@ static void FlushInBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(>interface->dev, "%s currentState=%d", __func__,
pdx->sCurrentState);
- if (pdx->sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
+ if (pdx->sCurrentState == U14ERR_TIME)
+ /* Do nothing if hardware in trouble */
return;
/* Kill off any pending I/O */
/* CharRead_Cancel(pDevObject); */
@@ -93,7 +95,8 @@ static int PutChars(DEVICE_EXTENSION *pdx, const char *pCh,
}
pdx->dwNumOutput += uCount;
spin_unlock_irq(>charOutLock);
- iReturn = SendChars(pdx); /* ...give a chance to
transmit data */
+ /* ...give a chance to transmit data */
+ iReturn = SendChars(pdx);
} else {
iReturn = U14ERR_NOOUT; /* no room at the out (ha-ha) */
spin_unlock_irq(>charOutLock);
@@ -110,7 +113,8 @@ int SendString(DEVICE_EXTENSION *pdx, const char __user
*pData,
unsigned int n)
{
int iReturn = U14ERR_NOERROR; /* assume all will be well */
- char buffer[OUTBUF_SZ + 1]; /* space in our address space for
characters */
+ /* space in our address space for characters */
+ char buffer[OUTBUF_SZ + 1];
if (n > OUTBUF_SZ) /* check space in local buffer... */
return U14ERR_NOOUT;/* ...too many characters */
if (copy_from_user(buffer, pData, n))
@@ -184,7 +188,8 @@ int Get1401State(DEVICE_EXTENSION *pdx, __u32 *state, __u32
*error)
if (nGot != sizeof(pdx->statBuf)) {
dev_err(>interface->dev,
"Get1401State() FAILED, return code %d", nGot);
- pdx->sCurrentState = U14ERR_TIME; /* Indicate that
things are very wrong indeed */
+ /* Indicate that things are very wrong indeed */
+ pdx->sCurrentState = U14ERR_TIME;
*state = 0; /* Force status values to a known state */
*error = 0;
} else {
@@ -193,16 +198,19 @@ int Get1401State(DEVICE_EXTENSION *pdx, __u32 *state,
__u32 *error)
"Get1401State() Success, state: 0x%x, 0x%x",
pdx->statBuf[0], pdx->statBuf[1]);
- *state = pdx->statBuf[0]; /* Return the state values to
the calling code */
+ /* Return the state values to the calling code */
+ *state = pdx->statBuf[0];
*error = pdx->statBuf[1];
- nDevice = pdx->udev->descriptor.bcdDevice >> 8; /* 1401 type
code value */
+ /* 1401 type code value */
+ nDevice = pdx->udev->descriptor.bcdDevice >> 8;
switch (nDevice) { /* so we can clean up current state */
case 0:
pdx->sCurrentState = U14ERR_U1401;
break;
- default:/* allow lots of device codes for future 1401s
*/
+ default:
+ /* allow lots of device codes for future 1401s */
if ((nDevice >= 1) && (nDevice <= 23))
pdx->sCurrentState = (short)(nDevice + 6);
else
@@ -227,23 +235,33 @@ int ReadWrite_Cancel(DEVICE_EXTENSION *pdx)
int ntStatus = STATUS_SUCCESS;
bool bResult = false;
unsigned int i;
- /* We can fill this in when we know how we will implement the staged
transfer stuff */
+ /*
+* We can fill this in when we know how
+* we will implement the staged transfer stuff
+*/
spin_lock_irq(>stagedLock);
- if (pdx->bStagedUrbPending) { /* anything to be cancelled? May need
more... */
+ if (pdx->bStagedUrbPending) {
+ /* anything to be cancelled? May need more... */
dev_info(>interface - dev,
"ReadWrite_Cancel about to cancel Urb");
/* Clear
[PATCH] Staging: ced1401: fix coding style in ced_ioc.c
From: Pol Eyschen poleysc...@hotmail.com
All comments fixed to match the kernel coding style.
Signed-off-by: Pol Eyschen poleysc...@gmail.com
---
drivers/staging/ced1401/ced_ioc.c | 382 -
1 file changed, 249 insertions(+), 133 deletions(-)
diff --git a/drivers/staging/ced1401/ced_ioc.c
b/drivers/staging/ced1401/ced_ioc.c
index 62efd74..e5172cb 100644
--- a/drivers/staging/ced1401/ced_ioc.c
+++ b/drivers/staging/ced1401/ced_ioc.c
@@ -41,7 +41,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(pdx-interface-dev, %s currentState=%d, __func__,
pdx-sCurrentState);
- if (pdx-sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
+ if (pdx-sCurrentState == U14ERR_TIME)
+ /* Do nothing if hardware in trouble */
return;
/* Kill off any pending I/O */
/* CharSend_Cancel(pdx); */
@@ -62,7 +63,8 @@ static void FlushInBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(pdx-interface-dev, %s currentState=%d, __func__,
pdx-sCurrentState);
- if (pdx-sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
+ if (pdx-sCurrentState == U14ERR_TIME)
+ /* Do nothing if hardware in trouble */
return;
/* Kill off any pending I/O */
/* CharRead_Cancel(pDevObject); */
@@ -93,7 +95,8 @@ static int PutChars(DEVICE_EXTENSION *pdx, const char *pCh,
}
pdx-dwNumOutput += uCount;
spin_unlock_irq(pdx-charOutLock);
- iReturn = SendChars(pdx); /* ...give a chance to
transmit data */
+ /* ...give a chance to transmit data */
+ iReturn = SendChars(pdx);
} else {
iReturn = U14ERR_NOOUT; /* no room at the out (ha-ha) */
spin_unlock_irq(pdx-charOutLock);
@@ -110,7 +113,8 @@ int SendString(DEVICE_EXTENSION *pdx, const char __user
*pData,
unsigned int n)
{
int iReturn = U14ERR_NOERROR; /* assume all will be well */
- char buffer[OUTBUF_SZ + 1]; /* space in our address space for
characters */
+ /* space in our address space for characters */
+ char buffer[OUTBUF_SZ + 1];
if (n OUTBUF_SZ) /* check space in local buffer... */
return U14ERR_NOOUT;/* ...too many characters */
if (copy_from_user(buffer, pData, n))
@@ -184,7 +188,8 @@ int Get1401State(DEVICE_EXTENSION *pdx, __u32 *state, __u32
*error)
if (nGot != sizeof(pdx-statBuf)) {
dev_err(pdx-interface-dev,
Get1401State() FAILED, return code %d, nGot);
- pdx-sCurrentState = U14ERR_TIME; /* Indicate that
things are very wrong indeed */
+ /* Indicate that things are very wrong indeed */
+ pdx-sCurrentState = U14ERR_TIME;
*state = 0; /* Force status values to a known state */
*error = 0;
} else {
@@ -193,16 +198,19 @@ int Get1401State(DEVICE_EXTENSION *pdx, __u32 *state,
__u32 *error)
Get1401State() Success, state: 0x%x, 0x%x,
pdx-statBuf[0], pdx-statBuf[1]);
- *state = pdx-statBuf[0]; /* Return the state values to
the calling code */
+ /* Return the state values to the calling code */
+ *state = pdx-statBuf[0];
*error = pdx-statBuf[1];
- nDevice = pdx-udev-descriptor.bcdDevice 8; /* 1401 type
code value */
+ /* 1401 type code value */
+ nDevice = pdx-udev-descriptor.bcdDevice 8;
switch (nDevice) { /* so we can clean up current state */
case 0:
pdx-sCurrentState = U14ERR_U1401;
break;
- default:/* allow lots of device codes for future 1401s
*/
+ default:
+ /* allow lots of device codes for future 1401s */
if ((nDevice = 1) (nDevice = 23))
pdx-sCurrentState = (short)(nDevice + 6);
else
@@ -227,23 +235,33 @@ int ReadWrite_Cancel(DEVICE_EXTENSION *pdx)
int ntStatus = STATUS_SUCCESS;
bool bResult = false;
unsigned int i;
- /* We can fill this in when we know how we will implement the staged
transfer stuff */
+ /*
+* We can fill this in when we know how
+* we will implement the staged transfer stuff
+*/
spin_lock_irq(pdx-stagedLock);
- if (pdx-bStagedUrbPending) { /* anything to be cancelled? May need
more... */
+ if (pdx-bStagedUrbPending) {
+ /* anything to be cancelled? May need more... */
dev_info(pdx-interface - dev,
ReadWrite_Cancel about to cancel Urb);
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On Mon, Jan 13, 2014 at 08:41:40PM +0100, Pol Eyschen wrote:
From: Pol Eyschen poleysc...@hotmail.com
All comments fixed to match the kernel coding style.
Signed-off-by: Pol Eyschen poleysc...@gmail.com
---
This patch doesn't apply to my staging-next branch. Are you making your
changes to the staging-next branch of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git?
drivers/staging/ced1401/ced_ioc.c | 382
-
1 file changed, 249 insertions(+), 133 deletions(-)
diff --git a/drivers/staging/ced1401/ced_ioc.c
b/drivers/staging/ced1401/ced_ioc.c
index 62efd74..e5172cb 100644
--- a/drivers/staging/ced1401/ced_ioc.c
+++ b/drivers/staging/ced1401/ced_ioc.c
@@ -41,7 +41,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(pdx-interface-dev, %s currentState=%d, __func__,
pdx-sCurrentState);
- if (pdx-sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
+ if (pdx-sCurrentState == U14ERR_TIME)
+ /* Do nothing if hardware in trouble */
return;
Putting a comment after a single line 'if' and before the statement is
not the norm and can be confusing. Please don't do it.
Also, quite a few of the comments you're moving around here look to be
unecessary, and can probably be deleted.
Mark
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On Mon, Jan 13, 2014 at 09:35:53PM +, Mark Einon wrote:
On Mon, Jan 13, 2014 at 08:41:40PM +0100, Pol Eyschen wrote:
From: Pol Eyschen poleysc...@hotmail.com
All comments fixed to match the kernel coding style.
Signed-off-by: Pol Eyschen poleysc...@gmail.com
---
This patch doesn't apply to my staging-next branch. Are you making your
changes to the staging-next branch of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git?
drivers/staging/ced1401/ced_ioc.c | 382
-
1 file changed, 249 insertions(+), 133 deletions(-)
diff --git a/drivers/staging/ced1401/ced_ioc.c
b/drivers/staging/ced1401/ced_ioc.c
index 62efd74..e5172cb 100644
--- a/drivers/staging/ced1401/ced_ioc.c
+++ b/drivers/staging/ced1401/ced_ioc.c
@@ -41,7 +41,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(pdx-interface-dev, %s currentState=%d, __func__,
pdx-sCurrentState);
- if (pdx-sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
+ if (pdx-sCurrentState == U14ERR_TIME)
+ /* Do nothing if hardware in trouble */
return;
Putting a comment after a single line 'if' and before the statement is
not the norm and can be confusing. Please don't do it.
The guideline is that multi-line indents should get {} braces for
readability even though they aren't needed for syntax.
if (pdx-sCurrentState == U14ERR_TIME) {
/* Do nothing if hardware in trouble */
return;
}
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On 13.01.2014, at 22:44, Dan Carpenter wrote:
On Mon, Jan 13, 2014 at 09:35:53PM +, Mark Einon wrote:
On Mon, Jan 13, 2014 at 08:41:40PM +0100, Pol Eyschen wrote:
From: Pol Eyschen poleysc...@hotmail.com
All comments fixed to match the kernel coding style.
Signed-off-by: Pol Eyschen poleysc...@gmail.com
---
This patch doesn't apply to my staging-next branch. Are you making your
changes to the staging-next branch of
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git?
I cloned git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git and
made my changes to that branch.
drivers/staging/ced1401/ced_ioc.c | 382
-
1 file changed, 249 insertions(+), 133 deletions(-)
diff --git a/drivers/staging/ced1401/ced_ioc.c
b/drivers/staging/ced1401/ced_ioc.c
index 62efd74..e5172cb 100644
--- a/drivers/staging/ced1401/ced_ioc.c
+++ b/drivers/staging/ced1401/ced_ioc.c
@@ -41,7 +41,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(pdx-interface-dev, %s currentState=%d, __func__,
pdx-sCurrentState);
- if (pdx-sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
+ if (pdx-sCurrentState == U14ERR_TIME)
+ /* Do nothing if hardware in trouble */
return;
Putting a comment after a single line 'if' and before the statement is
not the norm and can be confusing. Please don't do it.
The guideline is that multi-line indents should get {} braces for
readability even though they aren't needed for syntax.
if (pdx-sCurrentState == U14ERR_TIME) {
/* Do nothing if hardware in trouble */
return;
}
I took the original comments that were there, and just formatted them to be in
order with the coding style, I didn't add or remove anything. This is my first
patch so I didn't want to mess too much with what was there before, even if the
comments sometimes seemed superfluous to me.
Pol
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On Mon, Jan 13, 2014 at 11:31:29PM +0100, Pol Eyschen wrote: I took the original comments that were there, and just formatted them to be in order with the coding style, I didn't add or remove anything. This is my first patch so I didn't want to mess too much with what was there before, even if the comments sometimes seemed superfluous to me. No problem, everyone's first patch is rejected. Just fix it up in line with the review comments and resend. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On Mon, Jan 13, 2014 at 11:31:29PM +0100, Pol Eyschen wrote: On 13.01.2014, at 22:44, Dan Carpenter wrote: On Mon, Jan 13, 2014 at 09:35:53PM +, Mark Einon wrote: On Mon, Jan 13, 2014 at 08:41:40PM +0100, Pol Eyschen wrote: From: Pol Eyschen poleysc...@hotmail.com All comments fixed to match the kernel coding style. Signed-off-by: Pol Eyschen poleysc...@gmail.com --- This patch doesn't apply to my staging-next branch. Are you making your changes to the staging-next branch of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git? I cloned git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git and made my changes to that branch. That is Linus's tree, not linux-next which we all develop against. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On Mon, Jan 13, 2014 at 08:41:40PM +0100, Pol Eyschen wrote:
From: Pol Eyschen poleysc...@hotmail.com
All comments fixed to match the kernel coding style.
Signed-off-by: Pol Eyschen poleysc...@gmail.com
---
drivers/staging/ced1401/ced_ioc.c | 382
-
1 file changed, 249 insertions(+), 133 deletions(-)
diff --git a/drivers/staging/ced1401/ced_ioc.c
b/drivers/staging/ced1401/ced_ioc.c
index 62efd74..e5172cb 100644
--- a/drivers/staging/ced1401/ced_ioc.c
+++ b/drivers/staging/ced1401/ced_ioc.c
@@ -41,7 +41,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(pdx-interface-dev, %s currentState=%d, __func__,
pdx-sCurrentState);
- if (pdx-sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
+ if (pdx-sCurrentState == U14ERR_TIME)
+ /* Do nothing if hardware in trouble */
return;
Please wrap stuff like this in { } to not confuse people.
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: ced1401: fix coding style in ced_ioc.c
On Mon, 2014-01-13 at 15:56 -0800, Greg KH wrote:
On Mon, Jan 13, 2014 at 08:41:40PM +0100, Pol Eyschen wrote:
[]
diff --git a/drivers/staging/ced1401/ced_ioc.c
b/drivers/staging/ced1401/ced_ioc.c
[]
@@ -41,7 +41,8 @@ static void FlushOutBuff(DEVICE_EXTENSION *pdx)
{
dev_dbg(pdx-interface-dev, %s currentState=%d, __func__,
pdx-sCurrentState);
- if (pdx-sCurrentState == U14ERR_TIME) /* Do nothing if hardware in
trouble */
+ if (pdx-sCurrentState == U14ERR_TIME)
+ /* Do nothing if hardware in trouble */
return;
Please wrap stuff like this in { } to not confuse people.
Yes please, or maybe put the comment on the return line like:
if (pdx-sCurrentState == U14ERR_TIME)
return; /* Do nothing if hardware in trouble */
or above the test like:
/* Do nothing if hardware in trouble */
if (pdx-sCurrentState == U14ERR_TIME)
return;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
