[PATCH] Fix coding style in drivers/staging/crystalhd/crystalhd_lnx.c

[PATCH] Fix coding style in drivers/staging/crystalhd/crystalhd_lnx.c

[Pascal COMBES]

From: Pascal COMBES <pascom@orange.fr>

Fix alignement issues and two or three other coding style problems in 
drivers/staging/crystalhd/crystalhd_lnx.c.

NB:	-I did this for task 10 of Eudyptula challenge <http://eudyptula-challenge.org/>
	-I did not address camel case related problem because it would have make a big patch.

Signed-off-by: Pascal COMBES <pascom@orange.fr>
---
diff --git a/drivers/staging/crystalhd/crystalhd_lnx.c b/drivers/staging/crystalhd/crystalhd_lnx.c
index 20be957..9f6a9d4 100644
--- a/drivers/staging/crystalhd/crystalhd_lnx.c
+++ b/drivers/staging/crystalhd/crystalhd_lnx.c
@@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
 
 static irqreturn_t chd_dec_isr(int irq, void *arg)
 {
-	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
+	struct crystalhd_adp *adp = (struct crystalhd_adp *)arg;
 	int rc = 0;
 	if (adp)
 		rc = crystalhd_cmd_interrupt(&adp->cmds);
@@ -112,7 +112,7 @@ static void chd_dec_free_iodata(struct crystalhd_adp *adp,
 }
 
 static inline int crystalhd_user_data(void __user *ud, void *dr,
-			 int size, int set)
+				      int size, int set)
 {
 	int rc;
 
@@ -135,7 +135,9 @@ static inline int crystalhd_user_data(void __user *ud, void *dr,
 }
 
 static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
-	 struct crystalhd_ioctl_data *io, uint32_t m_sz, unsigned long ua)
+			       struct crystalhd_ioctl_data *io,
+			       uint32_t m_sz,
+			       unsigned long ua)
 {
 	unsigned long ua_off;
 	int rc = 0;
@@ -154,7 +156,7 @@ static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
 	io->add_cdata_sz = m_sz;
 	ua_off = ua + sizeof(io->udata);
 	rc = crystalhd_user_data((void __user *)ua_off, io->add_cdata,
-			io->add_cdata_sz, 0);
+				 io->add_cdata_sz, 0);
 	if (rc) {
 		BCMLOG_ERR("failed to pull add_cdata sz:%x ua_off:%x\n",
 			   io->add_cdata_sz, (unsigned int)ua_off);
@@ -167,7 +169,8 @@ static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
 }
 
 static int chd_dec_release_cdata(struct crystalhd_adp *adp,
-			 struct crystalhd_ioctl_data *io, unsigned long ua)
+				 struct crystalhd_ioctl_data *io,
+				 unsigned long ua)
 {
 	unsigned long ua_off;
 	int rc;
@@ -180,7 +183,7 @@ static int chd_dec_release_cdata(struct crystalhd_adp *adp,
 	if (io->cmd != BCM_IOC_FW_DOWNLOAD) {
 		ua_off = ua + sizeof(io->udata);
 		rc = crystalhd_user_data((void __user *)ua_off, io->add_cdata,
-					io->add_cdata_sz, 1);
+					 io->add_cdata_sz, 1);
 		if (rc) {
 			BCMLOG_ERR(
 				"failed to push add_cdata sz:%x ua_off:%x\n",
@@ -210,7 +213,7 @@ static int chd_dec_proc_user_data(struct crystalhd_adp *adp,
 	}
 
 	rc = crystalhd_user_data((void __user *)ua, &io->udata,
-			sizeof(io->udata), set);
+				 sizeof(io->udata), set);
 	if (rc) {
 		BCMLOG_ERR("failed to %s iodata\n", (set ? "set" : "get"));
 		return rc;
@@ -382,7 +385,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 	}
 
 	dev = device_create(crystalhd_class, NULL,
-			 MKDEV(adp->chd_dec_major, 0), NULL, "crystalhd");
+			    MKDEV(adp->chd_dec_major, 0), NULL, "crystalhd");
 	if (IS_ERR(dev)) {
 		rc = PTR_ERR(dev);
 		BCMLOG_ERR("failed to create device\n");
@@ -397,8 +400,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 
 	/* Allocate general purpose ioctl pool. */
 	for (i = 0; i < CHD_IODATA_POOL_SZ; i++) {
-		temp = kzalloc(sizeof(struct crystalhd_ioctl_data),
-					 GFP_KERNEL);
+		temp = kzalloc(sizeof(*temp), GFP_KERNEL);
 		if (!temp) {
 			BCMLOG_ERR("ioctl data pool kzalloc failed\n");
 			rc = -ENOMEM;
@@ -549,11 +551,11 @@ static int chd_dec_pci_probe(struct pci_dev *pdev,
 	enum BC_STATUS sts = BC_STS_SUCCESS;
 
 	BCMLOG(BCMLOG_DBG,
-		"PCI_INFO: Vendor:0x%04x Device:0x%04x s_vendor:0x%04x s_device: 0x%04x\n",
-		pdev->vendor, pdev->device, pdev->subsystem_vendor,
-		pdev->subsystem_device);
+	       "PCI_INFO: Vendor:0x%04x Device:0x%04x s_vendor:0x%04x s_device: 0x%04x\n",
+	       pdev->vendor, pdev->device, pdev->subsystem_vendor,
+	       pdev->subsystem_device);
 
-	pinfo = kzalloc(sizeof(struct crystalhd_adp), GFP_KERNEL);
+	pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL);
 	if (!pinfo) {
 		BCMLOG_ERR("Failed to allocate memory\n");
 		return -ENOMEM;

Re: [PATCH] Fix coding style in drivers/staging/crystalhd/crystalhd_lnx.c

[Greg Kroah-Hartman]

On Sun, Apr 13, 2014 at 05:35:46PM +0200, Pascal COMBES wrote:
> From: Pascal COMBES <pascom@orange.fr>
> 
> Fix alignement issues and two or three other coding style problems in 
> drivers/staging/crystalhd/crystalhd_lnx.c.

Patches need to do one thing, as you are addressing a number of
different things all in one patch, this needs to be broken up into
smaller pieces, and sent as a series of patches.

> 
> NB:	-I did this for task 10 of Eudyptula challenge <http://eudyptula-challenge.org/>
> 	-I did not address camel case related problem because it would have make a big patch.

This doesn't belong here, in the patch changelog body, please remove it.
If you want to include comments like this, they need to be below the
"---" line where git will remove them when the patch is applied.

> 
> Signed-off-by: Pascal COMBES <pascom@orange.fr>
> ---
> diff --git a/drivers/staging/crystalhd/crystalhd_lnx.c b/drivers/staging/crystalhd/crystalhd_lnx.c
> index 20be957..9f6a9d4 100644
> --- a/drivers/staging/crystalhd/crystalhd_lnx.c
> +++ b/drivers/staging/crystalhd/crystalhd_lnx.c
> @@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
>  
>  static irqreturn_t chd_dec_isr(int irq, void *arg)
>  {
> -	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
> +	struct crystalhd_adp *adp = (struct crystalhd_adp *)arg;
>  	int rc = 0;
>  	if (adp)
>  		rc = crystalhd_cmd_interrupt(&adp->cmds);
> @@ -112,7 +112,7 @@ static void chd_dec_free_iodata(struct crystalhd_adp *adp,
>  }
>  
>  static inline int crystalhd_user_data(void __user *ud, void *dr,
> -			 int size, int set)
> +				      int size, int set)
>  {
>  	int rc;
>  
> @@ -135,7 +135,9 @@ static inline int crystalhd_user_data(void __user *ud, void *dr,
>  }
>  
>  static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
> -	 struct crystalhd_ioctl_data *io, uint32_t m_sz, unsigned long ua)
> +			       struct crystalhd_ioctl_data *io,
> +			       uint32_t m_sz,
> +			       unsigned long ua)

Why can't these two lines be on the same line?

thanks,

greg k-h

[PATCH v2 1/3] Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c

[Pascal COMBES]

From: Pascal COMBES <pascom@orange.fr>

Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c.

Signed-off-by: Pascal COMBES <pascom@orange.fr>
---
As asked I split the patch in three (because I'm addressing 3 types of
problems). I also took into account your last comment: What I did seemmed 
better to me, but as I'm not the reference I discarded it.
	Regards,
Pascal COMBES.


diff --git a/drivers/staging/crystalhd/crystalhd_lnx.c b/drivers/staging/crystalhd/crystalhd_lnx.c
index 20be957..fd7f08a 100644
--- a/drivers/staging/crystalhd/crystalhd_lnx.c
+++ b/drivers/staging/crystalhd/crystalhd_lnx.c
@@ -112,7 +112,7 @@ static void chd_dec_free_iodata(struct crystalhd_adp *adp,
 }
 
 static inline int crystalhd_user_data(void __user *ud, void *dr,
-			 int size, int set)
+				      int size, int set)
 {
 	int rc;
 
@@ -135,7 +135,8 @@ static inline int crystalhd_user_data(void __user *ud, void *dr,
 }
 
 static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
-	 struct crystalhd_ioctl_data *io, uint32_t m_sz, unsigned long ua)
+			       struct crystalhd_ioctl_data *io, uint32_t m_sz,
+			       unsigned long ua)
 {
 	unsigned long ua_off;
 	int rc = 0;
@@ -154,7 +155,7 @@ static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
 	io->add_cdata_sz = m_sz;
 	ua_off = ua + sizeof(io->udata);
 	rc = crystalhd_user_data((void __user *)ua_off, io->add_cdata,
-			io->add_cdata_sz, 0);
+				 io->add_cdata_sz, 0);
 	if (rc) {
 		BCMLOG_ERR("failed to pull add_cdata sz:%x ua_off:%x\n",
 			   io->add_cdata_sz, (unsigned int)ua_off);
@@ -167,7 +168,8 @@ static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
 }
 
 static int chd_dec_release_cdata(struct crystalhd_adp *adp,
-			 struct crystalhd_ioctl_data *io, unsigned long ua)
+				 struct crystalhd_ioctl_data *io,
+				 unsigned long ua)
 {
 	unsigned long ua_off;
 	int rc;
@@ -180,7 +182,7 @@ static int chd_dec_release_cdata(struct crystalhd_adp *adp,
 	if (io->cmd != BCM_IOC_FW_DOWNLOAD) {
 		ua_off = ua + sizeof(io->udata);
 		rc = crystalhd_user_data((void __user *)ua_off, io->add_cdata,
-					io->add_cdata_sz, 1);
+					 io->add_cdata_sz, 1);
 		if (rc) {
 			BCMLOG_ERR(
 				"failed to push add_cdata sz:%x ua_off:%x\n",
@@ -210,7 +212,7 @@ static int chd_dec_proc_user_data(struct crystalhd_adp *adp,
 	}
 
 	rc = crystalhd_user_data((void __user *)ua, &io->udata,
-			sizeof(io->udata), set);
+				 sizeof(io->udata), set);
 	if (rc) {
 		BCMLOG_ERR("failed to %s iodata\n", (set ? "set" : "get"));
 		return rc;
@@ -382,7 +384,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 	}
 
 	dev = device_create(crystalhd_class, NULL,
-			 MKDEV(adp->chd_dec_major, 0), NULL, "crystalhd");
+			    MKDEV(adp->chd_dec_major, 0), NULL, "crystalhd");
 	if (IS_ERR(dev)) {
 		rc = PTR_ERR(dev);
 		BCMLOG_ERR("failed to create device\n");
@@ -398,7 +400,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 	/* Allocate general purpose ioctl pool. */
 	for (i = 0; i < CHD_IODATA_POOL_SZ; i++) {
 		temp = kzalloc(sizeof(struct crystalhd_ioctl_data),
-					 GFP_KERNEL);
+			       GFP_KERNEL);
 		if (!temp) {
 			BCMLOG_ERR("ioctl data pool kzalloc failed\n");
 			rc = -ENOMEM;
@@ -549,9 +551,9 @@ static int chd_dec_pci_probe(struct pci_dev *pdev,
 	enum BC_STATUS sts = BC_STS_SUCCESS;
 
 	BCMLOG(BCMLOG_DBG,
-		"PCI_INFO: Vendor:0x%04x Device:0x%04x s_vendor:0x%04x s_device: 0x%04x\n",
-		pdev->vendor, pdev->device, pdev->subsystem_vendor,
-		pdev->subsystem_device);
+	       "PCI_INFO: Vendor:0x%04x Device:0x%04x s_vendor:0x%04x s_device: 0x%04x\n",
+	       pdev->vendor, pdev->device, pdev->subsystem_vendor,
+	       pdev->subsystem_device);
 
 	pinfo = kzalloc(sizeof(struct crystalhd_adp), GFP_KERNEL);
 	if (!pinfo) {

[PATCH v2 2/3] Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c

[Pascal COMBES]

From: Pascal COMBES <pascom@orange.fr>

Fix coding style problem in drivers/staging/crystalhd/crystalhd_lnx.c:
No space needed before a cast.

Signed-off-by: Pascal COMBES <pascom@orange.fr>
---
diff --git a/drivers/staging/crystalhd/crystalhd_lnx.c b/drivers/staging/crystalhd/crystalhd_lnx.c
index fd7f08a..15e8f02 100644
--- a/drivers/staging/crystalhd/crystalhd_lnx.c
+++ b/drivers/staging/crystalhd/crystalhd_lnx.c
@@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
 
 static irqreturn_t chd_dec_isr(int irq, void *arg)
 {
-	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
+	struct crystalhd_adp *adp = (struct crystalhd_adp *)arg;
 	int rc = 0;
 	if (adp)
 		rc = crystalhd_cmd_interrupt(&adp->cmds);

[PATCH v2 3/3] Fix coding style problem (sizeof with type) in drivers/staging/crystalhd/crystalhd_lnx.c

[Pascal COMBES]

From: Pascal COMBES <pascom@orange.fr>

Replace sizeof(type) by sizeof(variable) in
drivers/staging/crystalhd/crystalhd_lnx.c.

Signed-off-by: Pascal COMBES <pascom@orange.fr>
---
diff --git a/drivers/staging/crystalhd/crystalhd_lnx.c
b/drivers/staging/crystalhd/crystalhd_lnx.c
index 15e8f02..7b14b28 100644
--- a/drivers/staging/crystalhd/crystalhd_lnx.c
+++ b/drivers/staging/crystalhd/crystalhd_lnx.c
@@ -399,8 +399,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)

 	/* Allocate general purpose ioctl pool. */
 	for (i = 0; i < CHD_IODATA_POOL_SZ; i++) {
-		temp = kzalloc(sizeof(struct crystalhd_ioctl_data),
-			       GFP_KERNEL);
+		temp = kzalloc(sizeof(*temp), GFP_KERNEL);
 		if (!temp) {
 			BCMLOG_ERR("ioctl data pool kzalloc failed\n");
 			rc = -ENOMEM;
@@ -555,7 +554,7 @@ static int chd_dec_pci_probe(struct pci_dev *pdev,
 	       pdev->vendor, pdev->device, pdev->subsystem_vendor,
 	       pdev->subsystem_device);

-	pinfo = kzalloc(sizeof(struct crystalhd_adp), GFP_KERNEL);
+	pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL);
 	if (!pinfo) {
 		BCMLOG_ERR("Failed to allocate memory\n");
 		return -ENOMEM;

Re: [PATCH v2 1/3] Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c

[Dan Carpenter]

The subeject should be:

[PATCH v2 1/3] Staging: crystalhd: Fix alignement in crystalhd_lnx.c

Please resend it.  Also you should use shorter subject lines.

"Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c"

This is too long.  Say:

"Remove a stray space character in crystalhd_lnx.c"

On Sun, Apr 13, 2014 at 09:09:18PM +0200, Pascal COMBES wrote:
> From: Pascal COMBES <pascom@orange.fr>

Don't include this for your own patches, only when you are sending
someone else's.  We get the From information from the email header.

(This is harmless but it's bad style).

regards,
dan carpenter

Re: [PATCH v2 1/3] Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c

[Paul Bolle]

On Mon, 2014-04-14 at 00:02 +0300, Dan Carpenter wrote:
> The subeject should be:
> 
> [PATCH v2 1/3] Staging: crystalhd: Fix alignement in crystalhd_lnx.c
> 
> Please resend it.  Also you should use shorter subject lines.
> 
> "Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c"
> 
> This is too long.  Say:
> 
> "Remove a stray space character in crystalhd_lnx.c"

The commit explanation also was a copy of the subject line. That
suggests that the explanation can be dropped. Many trivial fixes can
(and should) be submitted without a commit explanation.


Paul Bolle

Re: [PATCH v2 2/3] Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c

[Paul Bolle]

On Sun, 2014-04-13 at 21:13 +0200, Pascal COMBES wrote:
> From: Pascal COMBES <pascom@orange.fr>
> 
> Fix coding style problem in drivers/staging/crystalhd/crystalhd_lnx.c:
> No space needed before a cast.
> 
> Signed-off-by: Pascal COMBES <pascom@orange.fr>
> ---
> diff --git a/drivers/staging/crystalhd/crystalhd_lnx.c b/drivers/staging/crystalhd/crystalhd_lnx.c
> index fd7f08a..15e8f02 100644
> --- a/drivers/staging/crystalhd/crystalhd_lnx.c
> +++ b/drivers/staging/crystalhd/crystalhd_lnx.c
> @@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
>  
>  static irqreturn_t chd_dec_isr(int irq, void *arg)
>  {
> -	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
> +	struct crystalhd_adp *adp = (struct crystalhd_adp *)arg;

Shouldn't this cast just be dropped instead?

>  	int rc = 0;
>  	if (adp)
>  		rc = crystalhd_cmd_interrupt(&adp->cmds);
> 


Paul Bolle

Re: [PATCH v2 1/3] Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c

[Dan Carpenter]

On Sun, Apr 13, 2014 at 11:26:15PM +0200, Paul Bolle wrote:
> On Mon, 2014-04-14 at 00:02 +0300, Dan Carpenter wrote:
> > The subeject should be:
> > 
> > [PATCH v2 1/3] Staging: crystalhd: Fix alignement in crystalhd_lnx.c
> > 
> > Please resend it.  Also you should use shorter subject lines.
> > 
> > "Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c"
> > 
> > This is too long.  Say:
> > 
> > "Remove a stray space character in crystalhd_lnx.c"
> 
> The commit explanation also was a copy of the subject line.

Huh?  No it wasn't.

> That
> suggests that the explanation can be dropped.

The long description was fine.  It included a copy of the error message
and everything.

> Many trivial fixes can
> (and should) be submitted without a commit explanation.
> 

Don't encourage people to leave out the commit message.  They can figure
out on their own when that is appropriate.  In my experience people only
err on the other side, by not commenting enough.  I've only rarely been
annoyed by a long description which was too long.

regards,
dan carpenter

Re: [PATCH v2 1/3] Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c

[Paul Bolle]

On Mon, 2014-04-14 at 01:07 +0300, Dan Carpenter wrote:
> The long description was fine.  It included a copy of the error message
> and everything.

This is what I saw in the body of the message above the "---" line:
    From: Pascal COMBES <pascom@orange.fr>

    Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c.

    Signed-off-by: Pascal COMBES <pascom@orange.fr>

Are you and I looking at different patches?


Paul Bolle

Re: [PATCH v2 1/3] Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c

[Dan Carpenter]

On Mon, Apr 14, 2014 at 12:13:03AM +0200, Paul Bolle wrote:
> On Mon, 2014-04-14 at 01:07 +0300, Dan Carpenter wrote:
> > The long description was fine.  It included a copy of the error message
> > and everything.
> 
> This is what I saw in the body of the message above the "---" line:
>     From: Pascal COMBES <pascom@orange.fr>
> 
>     Fix alignement problems in drivers/staging/crystalhd/crystalhd_lnx.c.
> 
>     Signed-off-by: Pascal COMBES <pascom@orange.fr>
> 
> Are you and I looking at different patches?


Yeah.  I was looking at patch 2 where the space was removed.

regards,
dan carpenter

Re: [PATCH v2 2/3] Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c

[Pascal COMBES]

Le 13/04/2014 23:36, Paul Bolle a écrit :
> On Sun, 2014-04-13 at 21:13 +0200, Pascal COMBES wrote:
>> From: Pascal COMBES <pascom@orange.fr>
>>
>> Fix coding style problem in drivers/staging/crystalhd/crystalhd_lnx.c:
>> No space needed before a cast.
>>
>> Signed-off-by: Pascal COMBES <pascom@orange.fr>
>> ---
>> diff --git a/drivers/staging/crystalhd/crystalhd_lnx.c b/drivers/staging/crystalhd/crystalhd_lnx.c
>> index fd7f08a..15e8f02 100644
>> --- a/drivers/staging/crystalhd/crystalhd_lnx.c
>> +++ b/drivers/staging/crystalhd/crystalhd_lnx.c
>> @@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
>>  
>>  static irqreturn_t chd_dec_isr(int irq, void *arg)
>>  {
>> -	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
>> +	struct crystalhd_adp *adp = (struct crystalhd_adp *)arg;
> 
> Shouldn't this cast just be dropped instead?

I think as you and was wondering why it was there but I didn't dare
removing it. I considered it was not really coding style.
But after second thought, I will remove it when I resend tomrorrow.
	Regards,
Pascal COMBES.
> 
>>  	int rc = 0;
>>  	if (adp)
>>  		rc = crystalhd_cmd_interrupt(&adp->cmds);
>>
> 
> 
> Paul Bolle
> 
> 

Re: [PATCH v2 2/3] Fix coding style problem (cast with space) in drivers/staging/crystalhd/crystalhd_lnx.c

[Jingoo Han]

On Tuesday, April 15, 2014 1:38 AM, Pascal COMBES wrote:
> 
> Le 13/04/2014 23:36, Paul Bolle a écrit :
> > On Sun, 2014-04-13 at 21:13 +0200, Pascal COMBES wrote:
> >> From: Pascal COMBES <pascom@orange.fr>
> >>
> >> Fix coding style problem in drivers/staging/crystalhd/crystalhd_lnx.c:
> >> No space needed before a cast.
> >>
> >> Signed-off-by: Pascal COMBES <pascom@orange.fr>
> >> ---
> >> diff --git a/drivers/staging/crystalhd/crystalhd_lnx.c b/drivers/staging/crystalhd/crystalhd_lnx.c
> >> index fd7f08a..15e8f02 100644
> >> --- a/drivers/staging/crystalhd/crystalhd_lnx.c
> >> +++ b/drivers/staging/crystalhd/crystalhd_lnx.c
> >> @@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
> >>
> >>  static irqreturn_t chd_dec_isr(int irq, void *arg)
> >>  {
> >> -	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
> >> +	struct crystalhd_adp *adp = (struct crystalhd_adp *)arg;
> >
> > Shouldn't this cast just be dropped instead?
> 
> I think as you and was wondering why it was there but I didn't dare
> removing it. I considered it was not really coding style.
> But after second thought, I will remove it when I resend tomrorrow.

Please, remove unnecessary void cast as below.

 -	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
 +	struct crystalhd_adp *adp = arg;

This is because the conversion from void pointer to any other
pointer type is guaranteed by the C programming language.
So, void cast is unnecessary.

Best regards,
Jingoo Han

> 	Regards,
> Pascal COMBES.
> >
> >>  	int rc = 0;
> >>  	if (adp)
> >>  		rc = crystalhd_cmd_interrupt(&adp->cmds);
> >>
> >
> >
> > Paul Bolle
> >
> >

[PATCH v3 1/3] Staging: crystalhd: Fix alignement in crystalhd_lnx.c

[Pascal COMBES]

Signed-off-by: Pascal COMBES <pascom@orange.fr>
---
Description in subject.

diff --git a/drivers/staging/crystalhd/crystalhd_lnx.c b/drivers/staging/crystalhd/crystalhd_lnx.c
index 20be957..fd7f08a 100644
--- a/drivers/staging/crystalhd/crystalhd_lnx.c
+++ b/drivers/staging/crystalhd/crystalhd_lnx.c
@@ -112,7 +112,7 @@ static void chd_dec_free_iodata(struct crystalhd_adp *adp,
 }
 
 static inline int crystalhd_user_data(void __user *ud, void *dr,
-			 int size, int set)
+				      int size, int set)
 {
 	int rc;
 
@@ -135,7 +135,8 @@ static inline int crystalhd_user_data(void __user *ud, void *dr,
 }
 
 static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
-	 struct crystalhd_ioctl_data *io, uint32_t m_sz, unsigned long ua)
+			       struct crystalhd_ioctl_data *io, uint32_t m_sz,
+			       unsigned long ua)
 {
 	unsigned long ua_off;
 	int rc = 0;
@@ -154,7 +155,7 @@ static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
 	io->add_cdata_sz = m_sz;
 	ua_off = ua + sizeof(io->udata);
 	rc = crystalhd_user_data((void __user *)ua_off, io->add_cdata,
-			io->add_cdata_sz, 0);
+				 io->add_cdata_sz, 0);
 	if (rc) {
 		BCMLOG_ERR("failed to pull add_cdata sz:%x ua_off:%x\n",
 			   io->add_cdata_sz, (unsigned int)ua_off);
@@ -167,7 +168,8 @@ static int chd_dec_fetch_cdata(struct crystalhd_adp *adp,
 }
 
 static int chd_dec_release_cdata(struct crystalhd_adp *adp,
-			 struct crystalhd_ioctl_data *io, unsigned long ua)
+				 struct crystalhd_ioctl_data *io,
+				 unsigned long ua)
 {
 	unsigned long ua_off;
 	int rc;
@@ -180,7 +182,7 @@ static int chd_dec_release_cdata(struct crystalhd_adp *adp,
 	if (io->cmd != BCM_IOC_FW_DOWNLOAD) {
 		ua_off = ua + sizeof(io->udata);
 		rc = crystalhd_user_data((void __user *)ua_off, io->add_cdata,
-					io->add_cdata_sz, 1);
+					 io->add_cdata_sz, 1);
 		if (rc) {
 			BCMLOG_ERR(
 				"failed to push add_cdata sz:%x ua_off:%x\n",
@@ -210,7 +212,7 @@ static int chd_dec_proc_user_data(struct crystalhd_adp *adp,
 	}
 
 	rc = crystalhd_user_data((void __user *)ua, &io->udata,
-			sizeof(io->udata), set);
+				 sizeof(io->udata), set);
 	if (rc) {
 		BCMLOG_ERR("failed to %s iodata\n", (set ? "set" : "get"));
 		return rc;
@@ -382,7 +384,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 	}
 
 	dev = device_create(crystalhd_class, NULL,
-			 MKDEV(adp->chd_dec_major, 0), NULL, "crystalhd");
+			    MKDEV(adp->chd_dec_major, 0), NULL, "crystalhd");
 	if (IS_ERR(dev)) {
 		rc = PTR_ERR(dev);
 		BCMLOG_ERR("failed to create device\n");
@@ -398,7 +400,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 	/* Allocate general purpose ioctl pool. */
 	for (i = 0; i < CHD_IODATA_POOL_SZ; i++) {
 		temp = kzalloc(sizeof(struct crystalhd_ioctl_data),
-					 GFP_KERNEL);
+			       GFP_KERNEL);
 		if (!temp) {
 			BCMLOG_ERR("ioctl data pool kzalloc failed\n");
 			rc = -ENOMEM;
@@ -549,9 +551,9 @@ static int chd_dec_pci_probe(struct pci_dev *pdev,
 	enum BC_STATUS sts = BC_STS_SUCCESS;
 
 	BCMLOG(BCMLOG_DBG,
-		"PCI_INFO: Vendor:0x%04x Device:0x%04x s_vendor:0x%04x s_device: 0x%04x\n",
-		pdev->vendor, pdev->device, pdev->subsystem_vendor,
-		pdev->subsystem_device);
+	       "PCI_INFO: Vendor:0x%04x Device:0x%04x s_vendor:0x%04x s_device: 0x%04x\n",
+	       pdev->vendor, pdev->device, pdev->subsystem_vendor,
+	       pdev->subsystem_device);
 
 	pinfo = kzalloc(sizeof(struct crystalhd_adp), GFP_KERNEL);
 	if (!pinfo) {

[PATCH v3 2/3] Staging: crystalhd: Removed cast in crystalhd_lnx.c

[Pascal COMBES]

Removed useless (because automatic) cast from void * in crystalhd_lnx.c

Signed-off-by: Pascal COMBES <pascom@orange.fr>
---
diff --git a/drivers/staging/crystalhd/crystalhd_lnx.c b/drivers/staging/crystalhd/crystalhd_lnx.c
index fd7f08a..0e432bc 100644
--- a/drivers/staging/crystalhd/crystalhd_lnx.c
+++ b/drivers/staging/crystalhd/crystalhd_lnx.c
@@ -28,7 +28,7 @@ static struct crystalhd_adp *g_adp_info;
 
 static irqreturn_t chd_dec_isr(int irq, void *arg)
 {
-	struct crystalhd_adp *adp = (struct crystalhd_adp *) arg;
+	struct crystalhd_adp *adp = arg;
 	int rc = 0;
 	if (adp)
 		rc = crystalhd_cmd_interrupt(&adp->cmds);

[PATCH v3 3/3] Staging: crystalhd: Improve kzalloc calls in crystalhd_lnx.c

[Pascal COMBES]

Replace sizeof(type) by sizeof(variable) in crystalhd_lnx.c.

Signed-off-by: Pascal COMBES <pascom@orange.fr>
---
diff --git a/drivers/staging/crystalhd/crystalhd_lnx.c b/drivers/staging/crystalhd/crystalhd_lnx.c
index 0e432bc..e6fb331 100644
--- a/drivers/staging/crystalhd/crystalhd_lnx.c
+++ b/drivers/staging/crystalhd/crystalhd_lnx.c
@@ -399,8 +399,7 @@ static int chd_dec_init_chdev(struct crystalhd_adp *adp)
 
 	/* Allocate general purpose ioctl pool. */
 	for (i = 0; i < CHD_IODATA_POOL_SZ; i++) {
-		temp = kzalloc(sizeof(struct crystalhd_ioctl_data),
-			       GFP_KERNEL);
+		temp = kzalloc(sizeof(*temp), GFP_KERNEL);
 		if (!temp) {
 			BCMLOG_ERR("ioctl data pool kzalloc failed\n");
 			rc = -ENOMEM;
@@ -555,7 +554,7 @@ static int chd_dec_pci_probe(struct pci_dev *pdev,
 	       pdev->vendor, pdev->device, pdev->subsystem_vendor,
 	       pdev->subsystem_device);
 
-	pinfo = kzalloc(sizeof(struct crystalhd_adp), GFP_KERNEL);
+	pinfo = kzalloc(sizeof(*pinfo), GFP_KERNEL);
 	if (!pinfo) {
 		BCMLOG_ERR("Failed to allocate memory\n");
 		return -ENOMEM;

Re: [PATCH v3 1/3] Staging: crystalhd: Fix alignement in crystalhd_lnx.c

[Dan Carpenter]

All three look great now.

regards,
dan carpenter