> @@ -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

Reply via email to