On Thu, May 24, 2018 at 06:39:04PM +0200, Andrew Lunn wrote: > On Thu, May 24, 2018 at 04:32:34PM +0300, Ilias Apalodimas wrote: > > On Thu, May 24, 2018 at 03:12:29PM +0200, Andrew Lunn wrote: > > > Device tree is supposed to describe the hardware. Using that hardware > > > in different ways is not something you should describe in DT. > > > > > The new switchdev mode is applied with a .config option in the kernel. What > > you > > see is pre-existing code, so i am not sure if i should change it in this > > patchset. > > If you break the code up into a library and two drivers, it becomes a > moot point. Agree
> > But what i don't like here is that the device tree says to do dual > mac. But you ignore that and do sometime else. I would prefer that if > DT says dual mac, and switchdev is compiled in, the probe fails with > EINVAL. Rather than ignore something, make it clear it is invalid. The switch has 3 modes of operation as is. 1. switch mode, to enable that you don't need to add anything on the DTS and linux registers a single netdev interface. 2. dual mac mode, this is when you need to add dual_emac; on the DTS. 3. switchdev mode which is controlled by a .config option, since as you pointed out DTS was not made for controlling config options. I agree that this is far from beautiful. If the driver remains as in though, i'd prefer either keeping what's there or making "switchdev" a DTS option, following the pre-existing erroneous usage rather than making the device unusable. If we end up returning some error and refuse to initialize, users that remote upgrade their equipment, without taking a good look at changelog, will loose access to their devices with no means of remotely fixing that. Regards Ilias
