Re: [RFC v2, net-next, PATCH 4/4] net/cpsw_switchdev: add switchdev mode of operation on cpsw driver

[Grygorii Strashko <grygorii.strashko@ti.com>]

On 06/18/2018 03:19 PM, Ilias Apalodimas wrote:
> On Mon, Jun 18, 2018 at 06:16:27PM +0200, Andrew Lunn wrote:
>>> @@ -2711,6 +2789,10 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
>>>   	if (of_property_read_bool(node, "dual_emac"))
>>>   		data->switch_mode = CPSW_DUAL_EMAC;
>>>   
>>> +	/* switchdev overrides DTS */
>>> +	if (IS_ENABLED(CONFIG_TI_CPSW_SWITCHDEV))
>>> +		data->switch_mode = CPSW_SWITCHDEV;
>>> +
>>
>> I know you discussed this a bit with Jiri, but i still think if
>> 'dual_mac" is found, you should do dual mac. The DT clearly requests
>> dual mac, and doing anything else is going to cause confusion.
>>
>> The device tree binding is ambiguous what should happen when dual-mac
>> is missing. So i would only enable swithdev mode in that case.
> At the moment if no 'dual_emac;' is found on DTS the driver operates in "switch
> mode". It only registers 1 ethernet interface with no ability (unless you patch
> the kernel) to configure the switch. If we use DTS instead of a .config option
> we should add parsing for something like 'switchdev;' in the DTS.
> Jiri proposed using devlink, which makes sense, but i am not sure it's
> applicable on this patchset. This will change the driver completely and will
> totally break backwards compatibility.
> 
> Ideally i'd prefer something like:
> 1. Add a DTS option and continue the current behavior. I agree with you that
> this will cause less confusion (in fact i prefer it for the current state of the
> driver compared to the .config).
> or
> 2. Keep the .config option which is better suited over DTS but might cause some
> confusion.
>>
>> But ideally, it should be a new driver with a new binding.
> TI is better suited to comment on this. The work proposed here is mostly to
> accomodate future TSN related configuration for switches. This patchset has
> been tested against Ivan's patchset for CBS and is working as expected
> configuration wise.
> (http://lkml.iu.edu/hypermail/linux/kernel/1806.1/05302.html)

We'd try the new driver in the future

-- 
regards,
-grygorii