linux-kernel - Re: [PATCH v2 08/11] staging: pi433: Remove enum data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171206100312.zt3grfx2zlsz52kz@mwanda>
Date:   Wed, 6 Dec 2017 15:47:31 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Marcus Wolf <marcus.wolf@...rthome-wolf.de>
Cc:     Simon Sandström <simon@...anor.nu>,
        gregkh@...uxfoundation.org, devel@...verdev.osuosl.org,
        linux@...f-Entwicklungen.de, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 08/11] staging: pi433: Remove enum data_mode

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