netdev - Re: [PATCH net-next 1/8] dsa: mv88e6xxx: Prepare for turning this into a library module

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: <874mb454b3.fsf@ketchup.mtl.sfl>
Date:	Thu, 14 Apr 2016 16:22:08 -0400
From:	Vivien Didelot <vivien.didelot@...oirfairelinux.com>
To:	Andrew Lunn <andrew@...n.ch>, David Miller <davem@...emloft.net>
Cc:	Florian Fainelli <f.fainelli@...il.com>,
	netdev <netdev@...r.kernel.org>, Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH net-next 1/8] dsa: mv88e6xxx: Prepare for turning this into a library module

Hi Andrew,

Andrew Lunn <andrew@...n.ch> writes:

> Export all the functions so that we can later turn the module into a
> library module.
>
> Signed-off-by: Andrew Lunn <andrew@...n.ch>

Sorry but I don't like this. We don't want one module per 88E6xxx switch
model. We need one driver supporting them all, like any other driver.

Multiple modules will continue to confuse us with duplicated code. For
instance, every specific mv88e6*_setup_global functions program the
switch's DSA device number with something like:

    REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL_2, ds->index & 0x1f);

Looking at every drivers/net/dsa/mv88e6*.c file, there are only few
differences in their dsa_switch_driver structures:

The .setup function is always specific, but easily factorizable in a
mv88e6xxx_setup function. The .probe function can be merged once we have
a single driver. mv88e6131 has different phy_{read,write} functions
which can be abstracted in mv88e6xxx_phy_{read,write}. Only mv88e6352
has support for the EEPROM, which is simple to abstract too.

I'm working on a few patches right away to factorize this and lighten up
that part from your current refactoring of DSA.

Here's an example of duplicated code fixed for the 6131 PHY access code:

    http://ix.io/wJm

Thanks,
Vivien