netdev - Re: [PATCH V3 00/18] net: ks8851: Unify KS8851 SPI and MLL drivers

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 for Android: free password hash cracker in your pocket

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

Message-ID: <f9449a3b-7536-0bde-4ee6-b254fd90923f@denx.de>
Date:   Mon, 6 Apr 2020 13:20:22 +0200
From:   Marek Vasut <marex@...x.de>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
        Petr Stetiar <ynezz@...e.cz>,
        YueHaibing <yuehaibing@...wei.com>
Subject: Re: [PATCH V3 00/18] net: ks8851: Unify KS8851 SPI and MLL drivers

On 4/6/20 5:16 AM, Lukas Wunner wrote:
> On Sat, Mar 28, 2020 at 01:31:30AM +0100, Marek Vasut wrote:
>> The KS8851SNL/SNLI and KS8851-16MLL/MLLI/MLLU are very much the same pieces
>> of silicon, except the former has an SPI interface, while the later has a
>> parallel bus interface. Thus far, Linux has two separate drivers for each
>> and they are diverging considerably.
>>
>> This series unifies them into a single driver with small SPI and parallel
>> bus specific parts. The approach here is to first separate out the SPI
>> specific parts into a separate file, then add parallel bus accessors in
>> another separate file and then finally remove the old parallel bus driver.
>> The reason for replacing the old parallel bus driver is because the SPI
>> bus driver is much higher quality.
> 
> Sorry for the delay Marek.
> 
> ks8851.ko (SPI variant) no longer compiles with this series.
> The attached 0001 patch fixes it.
> 
> Both drivers can only be compiled as modules with this series:
> If only one of them is built-in, there's a linker error because of
> the module_param_named() for msg_enable.
> If both are built-in, the symbol collisions you've mentioned occur.
> 
> It seems Kbuild can't support building a .o file with a different name
> than the corresponding .c file because of the implicit rules used
> everywhere.  However, ks8851_common.c can be renamed to be a header
> file (a library of sorts) which is included by the two .c files.
> I've renamed ks8851_spi.c back to the original ks8851.c and
> ks8851_par.c back to ks8851_mll.c. The result is the attached 0002 patch.
> Compiles without any errors regardless if one or both drivers are
> built-in or modules.
> 
> I'll be back at the office this week and will conduct performance
> measurements with this version.

This looks like a hack, I'm more inclined to go back to using callbacks
for the various functions, since I don't see any performance problems
there. We're still talking about 25 MHz SPI bus here and the SPI
subsystem is full of such indirection anyway, so I'm not even sure what
you're hoping to gain here ; it seems to me like a premature
optimization which only causes trouble.