From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Lunn Subject: Re: [PATCH 4/4] cpsw: add switchdev support Date: Thu, 24 May 2018 15:12:29 +0200 Message-ID: <20180524131229.GC24557@lunn.ch> References: <1527144984-31236-1-git-send-email-ilias.apalodimas@linaro.org> <1527144984-31236-5-git-send-email-ilias.apalodimas@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, grygorii.strashko@ti.com, ivan.khoronzhuk@linaro.org, nsekhar@ti.com, jiri@resnulli.us, ivecera@redhat.com, francois.ozog@linaro.org, yogeshs@ti.com, spatton@ti.com To: Ilias Apalodimas Return-path: Received: from vps0.lunn.ch ([185.16.172.187]:43544 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935598AbeEXNMb (ORCPT ); Thu, 24 May 2018 09:12:31 -0400 Content-Disposition: inline In-Reply-To: <1527144984-31236-5-git-send-email-ilias.apalodimas@linaro.org> Sender: netdev-owner@vger.kernel.org List-ID: > @@ -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