Re: [PATCH v1 net-next 2/3] net: dsa: felix: Configure Time-Aware Scheduler via taprio offload

From: Vladimir Oltean <olteanv@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>,
	Po Liu <po.liu@nxp.com>, Claudiu Manoil <claudiu.manoil@nxp.com>,
	Alexandru Marginean <alexandru.marginean@nxp.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Li Yang <leoyang.li@nxp.com>, Mingkai Hu <mingkai.hu@nxp.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Jiri Pirko <jiri@resnulli.us>, Ido Schimmel <idosch@idosch.org>,
	Jakub Kicinski <kuba@kernel.org>, netdev <netdev@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>,
	Horatiu Vultur <horatiu.vultur@microchip.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	"Allan W. Nielsen" <allan.nielsen@microchip.com>,
	Joergen Andreasen <joergen.andreasen@microchip.com>,
	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>,
	Vinicius Costa Gomes <vinicius.gomes@intel.com>,
	Nikolay Aleksandrov <nikolay@cumulusnetworks.com>,
	Roopa Prabhu <roopa@cumulusnetworks.com>,
	linux-devel@linux.nxdi.nxp.com
Subject: Re: [PATCH v1 net-next 2/3] net: dsa: felix: Configure Time-Aware Scheduler via taprio offload
Date: Tue, 12 May 2020 03:10:52 +0300	[thread overview]
Message-ID: <CA+h21ho3NStn5BSBA1FjfRy6H0QTepGPyLT8xgCSB5af6PaHqg@mail.gmail.com> (raw)
In-Reply-To: <20200511.165258.1371001598940636146.davem@davemloft.net>

On Tue, 12 May 2020 at 02:54, David Miller <davem@davemloft.net> wrote:
>
> From: Xiaoliang Yang <xiaoliang.yang_1@nxp.com>
> Date: Mon, 11 May 2020 13:43:31 +0800
>
> > @@ -710,7 +714,7 @@ static void felix_port_policer_del(struct dsa_switch *ds, int port)
> >       ocelot_port_policer_del(ocelot, port);
> >  }
> >
> > -static const struct dsa_switch_ops felix_switch_ops = {
> > +static struct dsa_switch_ops felix_switch_ops = {
> >       .get_tag_protocol       = felix_get_tag_protocol,
> >       .setup                  = felix_setup,
> >       .teardown               = felix_teardown,
>
> There has to be a better way to do this, removing const for operation
> structs is very undesirable.

Actually I think this was at my suggestion, but now I agree that it is
a bit undesirable.
struct felix_info is on its toes in case vsc7511 or any other switch
revision will be added to the felix driver. It is likely that those
other chips will have hardware support for a different set of qdiscs.
So we thought of populating the .port_setup_tc conditionally,
depending on chip version (while also having DSA return -EOPNOTSUPP
automatically when the function pointer is NULL). Otherwise an extra
function call would be needed. But I think that keeping felix flexible
for more hardware revisions is a concern to be had for another
time/somebody else.