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

Dan Carpenter dan.carpenter at oracle.com
Wed Dec 6 12:47:31 UTC 2017


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



More information about the devel mailing list