netdev - Re: [PATCH v1 net-next] net: dsa: microchip: add KSZ9477 I2C 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:   Wed, 9 Jan 2019 17:15:26 +0100
From:   Marek Vasut <marex@...x.de>
To:     Pavel Machek <pavel@...x.de>, Tristram.Ha@...rochip.com
Cc:     jiri@...nulli.us, andrew@...n.ch, sergio.paracuellos@...il.com,
        f.fainelli@...il.com, dan.carpenter@...cle.com,
        vivien.didelot@...oirfairelinux.com, UNGLinuxDriver@...rochip.com,
        netdev@...r.kernel.org
Subject: Re: [PATCH v1 net-next] net: dsa: microchip: add KSZ9477 I2C driver

On 1/9/19 5:05 PM, Pavel Machek wrote:
> On Wed 2018-12-19 22:50:16, Tristram.Ha@...rochip.com wrote:
>>>>>>> This header file makes no sense. Please move the functions into .c
>>>>>>
>>>>>> No, that would make code bigger & slower.
>>>>>>
>>>>>> It makes sense to me. But I'd add "inline" keyword to make the goal
>>>>>> explicit.
>>>>>
>>>>> 1) It makes no sense to have header files for things like this. The
>>>>>    functions are only used within the single .c file.
>>>>>
>>>>> 2) You cannot inline them, as they are used as ops.
>>>>
>>>> Ok, sorry for the noise.
>>>
>>> If you were to use regmap, this whole boilerplate would go away ...
>>
>> Sometimes I am confused about this review process.
>>
>> The new code is same as previous code submitted.  The old code was accepted,
>> but then there are objections to the new code.
> 
> Sometimes you just need to reject review comment...
> 
> It took extremely long to merge this driver; that does not help,
> either.

FYI I submitted the regmap conversion series a few weeks ago, however I
couldn't test it on this specific switch. It'd be real nice to get
feedback on it and then build the subsequent patches on top of it, since
regmap hides the accessor differences and even makes the code cleaner.

-- 
Best regards,
Marek Vasut