Re: [RFC v2 net-next 5/8] net: dsa: felix: support psfp filter on vsc9959

[Vladimir Oltean]

On Wed, Aug 18, 2021 at 02:19:19PM +0800, Xiaoliang Yang wrote:
> +struct felix_psfp_list {
> +	struct list_head stream_list;
> +	struct list_head sfi_list;
> +	struct list_head sgi_list;
> +};
> +

Hmm, is there any reason why this data structure is not part of struct ocelot?
Three empty list_head items should not consume that much memory. To
reiterate, now we're trying to minimize the stuff that sits in DSA vs
what is in the ocelot library itself.

Microchip people, please shout if you have other hardware with this TSN
implementation that can be supported by the ocelot driver.

>  /* Platform-specific information */
>  struct felix_info {
>  	const struct resource		*target_io_res;
> @@ -36,6 +42,8 @@ struct felix_info {
>  	 */
>  	bool				quirk_no_xtr_irq;
>  
> +	struct felix_psfp_list		*psfp;
> +
>  	int	(*mdio_bus_alloc)(struct ocelot *ocelot);
>  	void	(*mdio_bus_free)(struct ocelot *ocelot);
>  	void	(*phylink_validate)(struct ocelot *ocelot, int port,
> diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c
> index f966a253d1c7..4bb3c4023b85 100644
> --- a/drivers/net/dsa/ocelot/felix_vsc9959.c
> +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c
> +struct felix_stream_filter_counters {
> +	u32 match;
> +	u32 not_pass_gate;
> +	u32 not_pass_sdu;
> +	u32 red;
> +};
> +
> +static struct felix_psfp_list vsc9959_psfp;

You cannot just do that, instantiate a singleton structure in a driver
that can potentially probe on more than one switch in a system. It is
just not proper driver design. Just put the lists

> +static bool vsc9959_stream_table_lookup(struct list_head *stream_list,
> +					struct felix_stream *stream)
> +{
> +	struct felix_stream *tmp;
> +
> +	list_for_each_entry(tmp, stream_list, list)
> +		if (ether_addr_equal(tmp->dmac, stream->dmac) &&
> +		    tmp->vid == stream->vid)
> +			return 1;
> +
> +	return 0;

A function that returns bool should return true or false.

> +}
> +
> +static struct felix_stream *
> +vsc9959_stream_table_get(struct list_head *stream_list, unsigned long id)
> +{
> +	struct felix_stream *tmp;
> +
> +	list_for_each_entry(tmp, stream_list, list)
> +		if (tmp->id == id)
> +			return tmp;
> +
> +	return NULL;
> +}

I mostly don't have a problem with the rest of the patch. When you send
v3 you can just drop the RFC tag.