netdev - Re: [PATCH 4/4] cpsw: add switchdev support

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: <20180524131229.GC24557@lunn.ch>
Date:   Thu, 24 May 2018 15:12:29 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Ilias Apalodimas <ilias.apalodimas@...aro.org>
Cc:     netdev@...r.kernel.org, grygorii.strashko@...com,
        ivan.khoronzhuk@...aro.org, nsekhar@...com, jiri@...nulli.us,
        ivecera@...hat.com, francois.ozog@...aro.org, yogeshs@...com,
        spatton@...com
Subject: Re: [PATCH 4/4] cpsw: add switchdev support

> @@ -2626,7 +2750,11 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
>  	data->mac_control = prop;
>  
>  	if (of_property_read_bool(node, "dual_emac"))
> -		data->dual_emac = 1;
> +		data->switch_mode = CPSW_DUAL_EMAC;
> +
> +	/* switchdev overrides DTS */
> +	if (IS_ENABLED(CONFIG_TI_CPSW_SWITCHDEV))
> +		data->switch_mode = CPSW_SWITCHDEV;

Device tree is supposed to describe the hardware. Using that hardware
in different ways is not something you should describe in DT.

There are also a lot of IS_ENABLED() here, which i don't like. It is a
lot better than #ifdef, but we should try to do better. It would be
good to split this cleanly into three parts. A generic library, which
does not care about DUAL_MAC or SWITCHDEV. A driver which implements
legacy DUAL MAC etc. And a driver which implements SWITCHDEV. We can
then give this new switchdev driver a different compatible. It i still
encoding in device tree how to use the hardware, but it is more
implicit, rather than explicit.

	  Andrew