linux-kernel - Re: [lm-sensors] [RFC PATCH] hwmon: (max6650) Convert to be a platform driver

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]

Date:	Thu, 13 Feb 2014 16:53:24 +0000
From:	Laszlo Papp <lpapp@....org>
To:	Guenter Roeck <linux@...ck-us.net>
Cc:	Lee Jones <lee.jones@...aro.org>, Jean Delvare <jdelvare@...e.de>,
	LKML <linux-kernel@...r.kernel.org>, lm-sensors@...sensors.org
Subject: Re: [lm-sensors] [RFC PATCH] hwmon: (max6650) Convert to be a
 platform driver

On Thu, Feb 13, 2014 at 4:16 PM, Guenter Roeck <linux@...ck-us.net> wrote:
> On 02/13/2014 04:27 AM, Laszlo Papp wrote:
>>
>> On Thu, Feb 13, 2014 at 11:33 AM, Lee Jones <lee.jones@...aro.org> wrote:
>>>>>>>>
>>>>>>>> -static int max6650_probe(struct i2c_client *client,
>>>>>>>> -                    const struct i2c_device_id *id);
>>>>>>>> -static int max6650_init_client(struct i2c_client *client);
>>>>>>>> -static int max6650_remove(struct i2c_client *client);
>>>>>>>> +static int max6650_probe(struct platform_device *pdev);
>>>>>>>> +static int max6650_init_client(struct platform_device *pdev);
>>>>>>>> +static int max6650_remove(struct platform_device *pdev);
>>>>>>>>   static struct max6650_data *max6650_update_device(struct device
>>>>>>>> *dev);
>>>>>>>
>>>>>>>
>>>>>>> It would be good to remove these forward declarations in the future.
>>>>>>>
>>>>>>> If no one volunteers I'll happily do it.
>>>>>>
>>>>>>
>>>>>> Guenter just did:
>>>>>>
>>>>>>
>>>>>> http://lists.lm-sensors.org/pipermail/lm-sensors/2014-February/041224.html
>>>>>>
>>>>>> Any change to the max6650 driver should go on top of his patch series
>>>>>> to avoid conflicts:
>>>>>>
>>>>>>
>>>>>> http://lists.lm-sensors.org/pipermail/lm-sensors/2014-February/041223.html
>>>>>
>>>>>
>>>> As far as I can see, that patch set was not even tested, so how can it
>>>> go in? I was told that any patch should be _runtime_ tested, too.
>>>> Fwiw, I do not have time to test those personally, he would need to
>>>> find someone else if that requirement really holds true.
>>>>
>>>> I would not really like to fix bugs appearing in that code to get my
>>>> features in.
>>>>
>>>> Also, since my change has been around for 2-3 months now, I would
>>>> really prefer not to be forced to rewrite it again from scratch.
>>>> Surely, you can wait with those, more or less, cosmetic non-runtime
>>>> tested changes?
>>>>
>>>> This would impose me a lot of additional work again, and I personally
>>>> do not see the benefit of it. In my book at least, feature is over
>>>> internal polishing.
>>>
>>>
>>> Right, I've had enough. I'm removing your patch from the MFD tree.
>>>
>>> I've asked too many people to give you a second chance and asked you
>>> privately to behave yourself and treat others with respect. So far I
>>> haven't seen an ounce of self control or depomacy from you.
>>>
>>> This is how it's going to work from now on:
>>>
>>>   - You submit a patch
>>>   - It gets reviewed                            <----\
>>>   - You fix up the review comments as requested -----/
>>>   - Non-compliance or arguments with the _experts_ results in:
>>>      `$INTEREST > /dev/null || \
>>>        grep "From: Laszio Papp" ~/.mail | xargs rm -rf`
>>
>>
>> http://comments.gmane.org/gmane.linux.kernel/1645251
>>
>> Step 2 did not happen. I did not get any review for my change. I
>> literally submitted that within a couple of hours after the request.
>>
>
> If you had tested your patch on real or simulated hardware,
> you might have noticed a crash whenever you accessed any
> of the attributes. So you did not test your patch.
>
> Instead of trying to educate you how the conversion to the
> new API works, I decided to help you out a bit and do
> the conversion myself.

I am afraid that with this attitude and approach towards potential new
contributors, you will put more and more work on you making it more
difficult to get things done at large. "Educating" new people is a win
in the long run, and this is a general practice out there before
claiming that I am crazy (e.g. someone even told me in the kernel
circles that is "rude").

I am surprised that mentoring new people is considered bad idea in
here. Since there is no guarantee you would not do it next time if I
tried to put effort into contributing, I feel more comfortable to skip
this project for now, and work where I know this cannot happen.
Currently, I do not feel inclusive in this project due to this
approach.

I do not even claim that you would be this to me intentionally, but it
has happened either way.

> I did some cleanup before, since
> that made the actual feature patch easier for me to implement,
> and I did some more cleanup afterwards just because I like
> cleaning up code.

IMO, it is not worth making the git history noisy with needless
re-arranging like forward headers, but that is just my opinion. Please
do not say, it is rude and not respectful. I definitely respect your
opinion, I just do not agree with it, but that is fine, you are a
maintainer so you get to decide, I guess. That being said, perhaps I
missed something why such re-arranging outweighs the git history
disadvantage.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/