Re: [PATCH v2 08/11] staging: pi433: Remove enum data_mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 06, 2017 at 11:11:27AM +0200, Marcus Wolf wrote:
> 
> Since the rule for kernel development seems to be, not to care about future,
> most probably you patch is fine, anyway.
> 

Yeah.  Deleting code if there is no user is required to move this code
out of staging...

I've worked in staging for a long time and I've seen patches from
thousands of people.  Simon really seems to understand kernel style and
that's less common than one might think.  It's a valuable skill.  If you
would just leave him to work then this driver would get fixed...

You're making it very difficult because you're complaining about the
work which *needs* to be done.  It's discouraging for people sending
patches.  This is very frustrating...  :(

On the naming, I think having a function which is named "enable" but
actually disables a feature is a non-starter.  What about we do either
one of these:
    pi433_enable_<feature>(spi);
    pi433_disable_<feature>(spi);
Or:
    pi433_set_<feature>(spi, SETTING);

It's still a standard and we'll just decide on a case by case basis what
to use.  This is a tiny driver and it's really easy to grep for the
feature name to find the functions you want.  Plus all the config is
done in one location so it's really not that hard.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux