LinuxLists.cc - [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c

2019-05-24 11:09:47

Subject: [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c

Fixes checkpatch.pl check "spaces preferred around
Signed-off-by: Simon Sandström <[email protected]>
/> b/drivers/staging/kpc2000/kpc2000/core.c
file *filp, char __user *buf,
is a NULL pointer!\n"))
: 0x%08x\n", pcard->card_id);
: 0x%08x\n", pcard->build_version);
: 0x%08x\n", pcard->build_datestamp);
: 0x%08x\n", pcard->build_timestamp);
: 0x%08x\n", pcard->core_table_offset);
: 0x%08x\n", pcard->core_table_length);
: 0x%08x\n", pcard->hardware_revision);
: 0x%016llx\n", pcard->ssid);
: 0x%016llx\n", pcard->ddna);
: 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_INTERRUPT_MASK));
: 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_INTERRUPT_ACTIVE));
: 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_CPLD_CONFIG));
: 0x%08x\n", pcard->card_id);
: 0x%08x\n", pcard->build_version);
: 0x%08x\n", pcard->build_datestamp);
: 0x%08x\n", pcard->build_timestamp);
: 0x%08x\n", pcard->core_table_offset);
: 0x%08x\n", pcard->core_table_length);
: 0x%08x\n", pcard->hardware_revision);
: 0x%016llx\n", pcard->ssid);
: 0x%016llx\n", pcard->ddna);
: 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_INTERRUPT_MASK));
: 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_INTERRUPT_ACTIVE));
: 0x%016llx\n", readq(pcard->sysinfo_regs_base + REG_CPLD_CONFIG));

2019-05-30 21:07:54

by Greg Kroah-Hartman

[permalink] [raw]

Subject: Re: [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c

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 <[email protected]>
> ---
> 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

2019-05-30 22:59:09

by Matt Sickler

[permalink] [raw]

Subject: RE: [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c

>From: devel <[email protected]> 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.

2019-05-31 00:13:35

by Greg Kroah-Hartman

[permalink] [raw]

Subject: Re: [PATCH 1/4] staging: kpc2000: add spaces around operators in core.c

On Thu, May 30, 2019 at 10:57:09PM +0000, Matt Sickler wrote:
> >From: devel <[email protected]> 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