netdev - Re: [PATCH 3/7] net: phy: spi_ks8995: add register initialization

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: <56B81BC1.6090904@gmail.com>
Date:	Sun, 7 Feb 2016 20:38:25 -0800
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Helmut Buchsbaum <helmut.buchsbaum@...il.com>,
	"David S. Miller" <davem@...emloft.net>
Cc:	netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH 3/7] net: phy: spi_ks8995: add register initialization

On 07/02/2016 14:39, Helmut Buchsbaum wrote:
> Since several use cases need to setup at least some basic control
> registers add the ability to configure an array containing such
> register initialization values within the platform data of the switch.
> Furthermore expose this capabilty to the devicetree.
> 
> Platform data now contains a pointer to an array and the array length
> where each member contains the register to be initialized, the
> initialization value and a register mask, since in many use cases there
> is only the need to init some bits of a register, e.g. disabling unused
> ports.
> 
> The devicetree notation add the property 'settings' to the SPI node of the
> ks8985 driver, which is a list of triple values (register, value, mask),
> e.g.:
> 
> settings = <0x4D 0x08 0x08
>             0x5D 0x08 0x08>;

You encode way too much in the Device Tree that should be knowledge to
the driver on how to configure the switch. This is very tempting,
because you do not dictate any use case and let people define it based
on their Device Tree source, but at the same time, this is very error
prone and does not provide what a proper device driver needs to be doing
by defining a standard and predictable behavior.

Right now this driver is a PHY driver, but it should be moved to a DSA
driver eventually such that each port is exposed as a network interface,
and you have hooks to power on/off ports based on whether a
corresponding network interface is up/down.
--
Florian