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:

quoted

On Thu, May 24, 2018 at 03:12:29PM +0200, Andrew Lunn wrote:

quoted

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