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

From: Greg KH
Date: Thu May 30 2019 - 20:15:37 EST


On Thu, May 30, 2019 at 10:57:09PM +0000, Matt Sickler wrote:
> >From: devel <driverdev-devel-bounces@xxxxxxxxxxxxxxxxxxxxxx> 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