LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/4] Fix whitespace issues in staging/kpc2000
@ 2019-05-24 11:07 Simon Sandström
  2019-05-24 11:07 ` [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c Simon Sandström
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Simon Sandström @ 2019-05-24 11:07 UTC (permalink / raw)
  To: gregkh; +Cc: jeremy, simon, devel, linux-kernel

Hi,

These patches fixes minor whitespace issues in staging/kpc2000/core.c as
reported by checkpatch.pl.

- Simon

Simon Sandström (4):
  staging: kpc2000: add spaces around operators in core.c
  staging: kpc2000: remove extra blank line in core.c
  staging: kpc2000: add missing spaces in core.c
  staging: kpc2000: remove extra spaces in core.c

 drivers/staging/kpc2000/kpc2000/core.c | 33 +++++++++++++-------------
 1 file changed, 16 insertions(+), 17 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c
  2019-05-24 11:07 [PATCH 0/4] Fix whitespace issues in staging/kpc2000 Simon Sandström
@ 2019-05-24 11:07 ` Simon Sandström
  2019-05-30 21:05   ` Greg KH
  2019-05-24 11:08 ` [PATCH 2/4] staging: kpc2000: remove extra blank line " Simon Sandström
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Simon Sandström @ 2019-05-24 11:07 UTC (permalink / raw)
  To: gregkh; +Cc: jeremy, simon, devel, linux-kernel

Fixes checkpatch.pl check "spaces preferred around that <op>".

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/kpc2000/kpc2000/core.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index 4110032d0cbb..b464973d12ad 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -276,18 +276,18 @@ static ssize_t kp2000_cdev_read(struct file *filp, char __user *buf,
 	if (WARN(NULL == buf, "kp2000_cdev_read: buf is a NULL pointer!\n"))
 		return -EINVAL;
 
-	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Card ID                 : 0x%08x\n", pcard->card_id);
-	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Build Version           : 0x%08x\n", pcard->build_version);
-	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Build Date              : 0x%08x\n", pcard->build_datestamp);
-	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Build Time              : 0x%08x\n", pcard->build_timestamp);
-	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Core Table Offset       : 0x%08x\n", pcard->core_table_offset);
-	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Core Table Length       : 0x%08x\n", pcard->core_table_length);
-	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Hardware Revision       : 0x%08x\n", pcard->hardware_revision);
-	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "SSID                    : 0x%016llx\n", pcard->ssid);
-	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "DDNA                    : 0x%016llx\n", pcard->ddna);
-	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "IRQ Mask                : 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_INTERRUPT_MASK));
-	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "IRQ Active              : 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_INTERRUPT_ACTIVE));
-	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "CPLD                    : 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_CPLD_CONFIG));
+	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Card ID                 : 0x%08x\n", pcard->card_id);
+	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Build Version           : 0x%08x\n", pcard->build_version);
+	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Build Date              : 0x%08x\n", pcard->build_datestamp);
+	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Build Time              : 0x%08x\n", pcard->build_timestamp);
+	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Core Table Offset       : 0x%08x\n", pcard->core_table_offset);
+	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Core Table Length       : 0x%08x\n", pcard->core_table_length);
+	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Hardware Revision       : 0x%08x\n", pcard->hardware_revision);
+	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "SSID                    : 0x%016llx\n", pcard->ssid);
+	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "DDNA                    : 0x%016llx\n", pcard->ddna);
+	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "IRQ Mask                : 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_INTERRUPT_MASK));
+	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "IRQ Active              : 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_INTERRUPT_ACTIVE));
+	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "CPLD                    : 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_CPLD_CONFIG));
 
 	if (*f_pos >= cnt)
 		return 0;
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/4] staging: kpc2000: remove extra blank line in core.c
  2019-05-24 11:07 [PATCH 0/4] Fix whitespace issues in staging/kpc2000 Simon Sandström
  2019-05-24 11:07 ` [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c Simon Sandström
@ 2019-05-24 11:08 ` Simon Sandström
  2019-05-24 11:08 ` [PATCH 3/4] staging: kpc2000: add missing spaces " Simon Sandström
  2019-05-24 11:08 ` [PATCH 4/4] staging: kpc2000: remove extra " Simon Sandström
  3 siblings, 0 replies; 11+ messages in thread
From: Simon Sandström @ 2019-05-24 11:08 UTC (permalink / raw)
  To: gregkh; +Cc: jeremy, simon, devel, linux-kernel

Fixes checkpatch.pl check "Please don't use multiple blank lines".

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/kpc2000/kpc2000/core.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index b464973d12ad..40f65f96986b 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -262,7 +262,6 @@ static int kp2000_cdev_close(struct inode *inode, struct file *filp)
 	return 0;
 }
 
-
 static ssize_t kp2000_cdev_read(struct file *filp, char __user *buf,
 				size_t count, loff_t *f_pos)
 {
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/4] staging: kpc2000: add missing spaces in core.c
  2019-05-24 11:07 [PATCH 0/4] Fix whitespace issues in staging/kpc2000 Simon Sandström
  2019-05-24 11:07 ` [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c Simon Sandström
  2019-05-24 11:08 ` [PATCH 2/4] staging: kpc2000: remove extra blank line " Simon Sandström
@ 2019-05-24 11:08 ` Simon Sandström
  2019-05-27  7:31   ` Dan Carpenter
  2019-05-24 11:08 ` [PATCH 4/4] staging: kpc2000: remove extra " Simon Sandström
  3 siblings, 1 reply; 11+ messages in thread
From: Simon Sandström @ 2019-05-24 11:08 UTC (permalink / raw)
  To: gregkh; +Cc: jeremy, simon, devel, linux-kernel

Fixes checkpatch.pl errors "space required before the open brace '{'"
and "(foo*)" should be "(foo *)".

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/kpc2000/kpc2000/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index 40f65f96986b..bb3b427a72b1 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -308,7 +308,7 @@ static long kp2000_cdev_ioctl(struct file *filp, unsigned int ioctl_num,
 
 	dev_dbg(&pcard->pdev->dev, "kp2000_cdev_ioctl(filp = [%p], ioctl_num = 0x%08x, ioctl_param = 0x%016lx) pcard = [%p]\n", filp, ioctl_num, ioctl_param, pcard);
 
-	switch (ioctl_num){
+	switch (ioctl_num) {
 	case KP2000_IOCTL_GET_CPLD_REG:             return readq(pcard->sysinfo_regs_base + REG_CPLD_CONFIG);
 	case KP2000_IOCTL_GET_PCIE_ERROR_REG:       return readq(pcard->sysinfo_regs_base + REG_PCIE_ERROR_COUNT);
 
@@ -326,7 +326,7 @@ static long kp2000_cdev_ioctl(struct file *filp, unsigned int ioctl_num,
 		temp.ddna = pcard->ddna;
 		temp.cpld_reg = readq(pcard->sysinfo_regs_base + REG_CPLD_CONFIG);
 
-		ret = copy_to_user((void*)ioctl_param, (void*)&temp, sizeof(temp));
+		ret = copy_to_user((void *)ioctl_param, (void *)&temp, sizeof(temp));
 		if (ret)
 			return -EFAULT;
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/4] staging: kpc2000: remove extra spaces in core.c
  2019-05-24 11:07 [PATCH 0/4] Fix whitespace issues in staging/kpc2000 Simon Sandström
                   ` (2 preceding siblings ...)
  2019-05-24 11:08 ` [PATCH 3/4] staging: kpc2000: add missing spaces " Simon Sandström
@ 2019-05-24 11:08 ` Simon Sandström
  3 siblings, 0 replies; 11+ messages in thread
From: Simon Sandström @ 2019-05-24 11:08 UTC (permalink / raw)
  To: gregkh; +Cc: jeremy, simon, devel, linux-kernel

Fixes checkpatch.pl error "foo __init  bar" should be "foo __init bar"
and "foo __exit  bar" should be "foo __exit bar".

Signed-off-by: Simon Sandström <simon@nikanor.nu>
---
 drivers/staging/kpc2000/kpc2000/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
index bb3b427a72b1..3b240eff62b7 100644
--- a/drivers/staging/kpc2000/kpc2000/core.c
+++ b/drivers/staging/kpc2000/kpc2000/core.c
@@ -655,7 +655,7 @@ static struct pci_driver kp2000_driver_inst = {
 	.remove =	kp2000_pcie_remove,
 };
 
-static int __init  kp2000_pcie_init(void)
+static int __init kp2000_pcie_init(void)
 {
 	kpc_uio_class = class_create(THIS_MODULE, "kpc_uio");
 	if (IS_ERR(kpc_uio_class))
@@ -666,7 +666,7 @@ static int __init  kp2000_pcie_init(void)
 }
 module_init(kp2000_pcie_init);
 
-static void __exit  kp2000_pcie_exit(void)
+static void __exit kp2000_pcie_exit(void)
 {
 	pci_unregister_driver(&kp2000_driver_inst);
 	class_destroy(kpc_uio_class);
-- 
2.20.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] staging: kpc2000: add missing spaces in core.c
  2019-05-24 11:08 ` [PATCH 3/4] staging: kpc2000: add missing spaces " Simon Sandström
@ 2019-05-27  7:31   ` Dan Carpenter
  2019-05-29 15:54     ` Simon Sandström
  0 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2019-05-27  7:31 UTC (permalink / raw)
  To: Simon Sandström; +Cc: gregkh, devel, linux-kernel

On Fri, May 24, 2019 at 01:08:01PM +0200, Simon Sandström wrote:
> Fixes checkpatch.pl errors "space required before the open brace '{'"
> and "(foo*)" should be "(foo *)".
> 
> Signed-off-by: Simon Sandström <simon@nikanor.nu>
> ---
>  drivers/staging/kpc2000/kpc2000/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
> index 40f65f96986b..bb3b427a72b1 100644
> --- a/drivers/staging/kpc2000/kpc2000/core.c
> +++ b/drivers/staging/kpc2000/kpc2000/core.c
> @@ -308,7 +308,7 @@ static long kp2000_cdev_ioctl(struct file *filp, unsigned int ioctl_num,
>  
>  	dev_dbg(&pcard->pdev->dev, "kp2000_cdev_ioctl(filp = [%p], ioctl_num = 0x%08x, ioctl_param = 0x%016lx) pcard = [%p]\n", filp, ioctl_num, ioctl_param, pcard);
>  
> -	switch (ioctl_num){
> +	switch (ioctl_num) {
>  	case KP2000_IOCTL_GET_CPLD_REG:             return readq(pcard->sysinfo_regs_base + REG_CPLD_CONFIG);
>  	case KP2000_IOCTL_GET_PCIE_ERROR_REG:       return readq(pcard->sysinfo_regs_base + REG_PCIE_ERROR_COUNT);
>  
> @@ -326,7 +326,7 @@ static long kp2000_cdev_ioctl(struct file *filp, unsigned int ioctl_num,
>  		temp.ddna = pcard->ddna;
>  		temp.cpld_reg = readq(pcard->sysinfo_regs_base + REG_CPLD_CONFIG);
>  
> -		ret = copy_to_user((void*)ioctl_param, (void*)&temp, sizeof(temp));
> +		ret = copy_to_user((void *)ioctl_param, (void *)&temp, sizeof(temp));
>  		if (ret)
>  			return -EFAULT;

This should really be written like so:

		if (copy_to_user((void __user *)ioctl_param, &temp,
				 sizeof(temp)))
			return -EFAULT;

temp is really the wrong name.  "temp" is for temperatures.  "tmp" means
temporary.  But also "tmp" is wrong here because it's not a temporary
variable.  It's better to call it "regs" here.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] staging: kpc2000: add missing spaces in core.c
  2019-05-27  7:31   ` Dan Carpenter
@ 2019-05-29 15:54     ` Simon Sandström
  2019-05-29 18:15       ` Dan Carpenter
  0 siblings, 1 reply; 11+ messages in thread
From: Simon Sandström @ 2019-05-29 15:54 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: gregkh, devel, linux-kernel

On Mon, May 27, 2019 at 10:31:59AM +0300, Dan Carpenter wrote:
> On Fri, May 24, 2019 at 01:08:01PM +0200, Simon Sandström wrote:
> > [..]
> > -		ret = copy_to_user((void*)ioctl_param, (void*)&temp, sizeof(temp));
> > +		ret = copy_to_user((void *)ioctl_param, (void *)&temp, sizeof(temp));
> >  		if (ret)
> >  			return -EFAULT;
> 
> This should really be written like so:
> 
> 		if (copy_to_user((void __user *)ioctl_param, &temp,
> 				 sizeof(temp)))
> 			return -EFAULT;
> 
> temp is really the wrong name.  "temp" is for temperatures.  "tmp" means
> temporary.  But also "tmp" is wrong here because it's not a temporary
> variable.  It's better to call it "regs" here.
> 
> regards,
> dan carpenter
> 

I agree, but I don't think it fits within this patch. I can send a
separate patch with this change.

Thanks

- Simon

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] staging: kpc2000: add missing spaces in core.c
  2019-05-29 15:54     ` Simon Sandström
@ 2019-05-29 18:15       ` Dan Carpenter
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2019-05-29 18:15 UTC (permalink / raw)
  To: Simon Sandström; +Cc: gregkh, devel, linux-kernel

On Wed, May 29, 2019 at 05:54:19PM +0200, Simon Sandström wrote:
> On Mon, May 27, 2019 at 10:31:59AM +0300, Dan Carpenter wrote:
> > On Fri, May 24, 2019 at 01:08:01PM +0200, Simon Sandström wrote:
> > > [..]
> > > -		ret = copy_to_user((void*)ioctl_param, (void*)&temp, sizeof(temp));
> > > +		ret = copy_to_user((void *)ioctl_param, (void *)&temp, sizeof(temp));
> > >  		if (ret)
> > >  			return -EFAULT;
> > 
> > This should really be written like so:
> > 
> > 		if (copy_to_user((void __user *)ioctl_param, &temp,
> > 				 sizeof(temp)))
> > 			return -EFAULT;
> > 
> > temp is really the wrong name.  "temp" is for temperatures.  "tmp" means
> > temporary.  But also "tmp" is wrong here because it's not a temporary
> > variable.  It's better to call it "regs" here.
> > 
> > regards,
> > dan carpenter
> > 
> 
> I agree, but I don't think it fits within this patch. I can send a
> separate patch with this change.

You could send the other chunk as a separate patch, but I don't think it
makes sense to apply this chunk when really it just needs to be
re-written.

I normally don't complain too much about mechanical no-thought patches,
but in this case the function is very sub-par and should be re-written.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c
  2019-05-24 11:07 ` [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c Simon Sandström
@ 2019-05-30 21:05   ` Greg KH
  2019-05-30 22:57     ` Matt Sickler
  0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2019-05-30 21:05 UTC (permalink / raw)
  To: Simon Sandström; +Cc: devel, linux-kernel

On Fri, May 24, 2019 at 01:07:59PM +0200, Simon Sandström wrote:
> Fixes checkpatch.pl check "spaces preferred around that <op>".
> 
> Signed-off-by: Simon Sandström <simon@nikanor.nu>
> ---
>  drivers/staging/kpc2000/kpc2000/core.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/kpc2000/kpc2000/core.c b/drivers/staging/kpc2000/kpc2000/core.c
> index 4110032d0cbb..b464973d12ad 100644
> --- a/drivers/staging/kpc2000/kpc2000/core.c
> +++ b/drivers/staging/kpc2000/kpc2000/core.c
> @@ -276,18 +276,18 @@ static ssize_t kp2000_cdev_read(struct file *filp, char __user *buf,
>  	if (WARN(NULL == buf, "kp2000_cdev_read: buf is a NULL pointer!\n"))
>  		return -EINVAL;
>  
> -	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Card ID                 : 0x%08x\n", pcard->card_id);
> -	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Build Version           : 0x%08x\n", pcard->build_version);
> -	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Build Date              : 0x%08x\n", pcard->build_datestamp);
> -	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Build Time              : 0x%08x\n", pcard->build_timestamp);
> -	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Core Table Offset       : 0x%08x\n", pcard->core_table_offset);
> -	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Core Table Length       : 0x%08x\n", pcard->core_table_length);
> -	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "Hardware Revision       : 0x%08x\n", pcard->hardware_revision);
> -	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "SSID                    : 0x%016llx\n", pcard->ssid);
> -	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "DDNA                    : 0x%016llx\n", pcard->ddna);
> -	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "IRQ Mask                : 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_INTERRUPT_MASK));
> -	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "IRQ Active              : 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_INTERRUPT_ACTIVE));
> -	cnt += scnprintf(buff+cnt, BUFF_CNT-cnt, "CPLD                    : 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_CPLD_CONFIG));
> +	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Card ID                 : 0x%08x\n", pcard->card_id);
> +	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Build Version           : 0x%08x\n", pcard->build_version);
> +	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Build Date              : 0x%08x\n", pcard->build_datestamp);
> +	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Build Time              : 0x%08x\n", pcard->build_timestamp);
> +	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Core Table Offset       : 0x%08x\n", pcard->core_table_offset);
> +	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Core Table Length       : 0x%08x\n", pcard->core_table_length);
> +	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "Hardware Revision       : 0x%08x\n", pcard->hardware_revision);
> +	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "SSID                    : 0x%016llx\n", pcard->ssid);
> +	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "DDNA                    : 0x%016llx\n", pcard->ddna);
> +	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "IRQ Mask                : 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_INTERRUPT_MASK));
> +	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "IRQ Active              : 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_INTERRUPT_ACTIVE));
> +	cnt += scnprintf(buff + cnt, BUFF_CNT - cnt, "CPLD                    : 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_CPLD_CONFIG));
>  
>  	if (*f_pos >= cnt)
>  		return 0;

This whole function just needs to be deleted, it's a horrible hack.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c
  2019-05-30 21:05   ` Greg KH
@ 2019-05-30 22:57     ` Matt Sickler
  2019-05-31  0:11       ` Greg KH
  0 siblings, 1 reply; 11+ messages in thread
From: Matt Sickler @ 2019-05-30 22:57 UTC (permalink / raw)
  To: Greg KH, Simon Sandström; +Cc: devel, linux-kernel

>From: devel <driverdev-devel-bounces@linuxdriverproject.org> On Behalf Of
>Greg KH
>On Fri, May 24, 2019 at 01:07:59PM +0200, Simon Sandström wrote:
>> --- a/drivers/staging/kpc2000/kpc2000/core.c
>> +++ b/drivers/staging/kpc2000/kpc2000/core.c
>> @@ -276,18 +276,18 @@ static ssize_t kp2000_cdev_read(struct file *filp,
>
>This whole function just needs to be deleted, it's a horrible hack.

From the outside, I would definitely agree with you.  On the inside though, we
rely on this function to quickly identify what kind and version is running on
a given piece of hardware.  Since that same information is provided by an ioctl,
I could be convinced to remove this API and write a userspace application that
uses the ioctl to get the information and pretty prints it.
I'd be more inclined to agree with the deletion if it means the whole char dev
interface can be removed from the kpc2000 driver.  That won't be straightforward
as the ioctl is exposed through this interface.  We could remove the ioctl, but
we'd need to ensure that all the same information is exposed via sysfs.  Our
userspace side is all funneled through a single class, so changing it to use
sysfs wouldn't be too difficult.  I'd support that change if someone wants to make it.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c
  2019-05-30 22:57     ` Matt Sickler
@ 2019-05-31  0:11       ` Greg KH
  0 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2019-05-31  0:11 UTC (permalink / raw)
  To: Matt Sickler; +Cc: Simon Sandström, devel, linux-kernel

On Thu, May 30, 2019 at 10:57:09PM +0000, Matt Sickler wrote:
> >From: devel <driverdev-devel-bounces@linuxdriverproject.org> On Behalf Of
> >Greg KH
> >On Fri, May 24, 2019 at 01:07:59PM +0200, Simon Sandström wrote:
> >> --- a/drivers/staging/kpc2000/kpc2000/core.c
> >> +++ b/drivers/staging/kpc2000/kpc2000/core.c
> >> @@ -276,18 +276,18 @@ static ssize_t kp2000_cdev_read(struct file *filp,
> >
> >This whole function just needs to be deleted, it's a horrible hack.
> 
> >From the outside, I would definitely agree with you.  On the inside though, we
> rely on this function to quickly identify what kind and version is running on
> a given piece of hardware.  Since that same information is provided by an ioctl,
> I could be convinced to remove this API and write a userspace application that
> uses the ioctl to get the information and pretty prints it.

The ioctl needs to die as well, just use the sysfs entries instead, as
you already have them :)

> I'd be more inclined to agree with the deletion if it means the whole char dev
> interface can be removed from the kpc2000 driver.  That won't be straightforward
> as the ioctl is exposed through this interface.  We could remove the ioctl, but
> we'd need to ensure that all the same information is exposed via sysfs.

I think you are there already, what is missing?

> Our userspace side is all funneled through a single class, so changing
> it to use sysfs wouldn't be too difficult.  I'd support that change if
> someone wants to make it.

I will be glad to do that, it's always nice to delete code :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-05-31  0:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-24 11:07 [PATCH 0/4] Fix whitespace issues in staging/kpc2000 Simon Sandström
2019-05-24 11:07 ` [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c Simon Sandström
2019-05-30 21:05   ` Greg KH
2019-05-30 22:57     ` Matt Sickler
2019-05-31  0:11       ` Greg KH
2019-05-24 11:08 ` [PATCH 2/4] staging: kpc2000: remove extra blank line " Simon Sandström
2019-05-24 11:08 ` [PATCH 3/4] staging: kpc2000: add missing spaces " Simon Sandström
2019-05-27  7:31   ` Dan Carpenter
2019-05-29 15:54     ` Simon Sandström
2019-05-29 18:15       ` Dan Carpenter
2019-05-24 11:08 ` [PATCH 4/4] staging: kpc2000: remove extra " Simon Sandström

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).