Re: [PATCH net-next v2 2/2] net: dsa: mv88e6xxx: add support for credit based shaper

[Andrew Lunn <andrew@lunn.ch>]

> +static int mv88e6xxx_setup_tc_cbs(struct dsa_switch *ds, int port,
> +				  struct tc_cbs_qopt_offload *cbs)
> +{
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +	const struct mv88e6xxx_ops *ops = chip->info->ops;
> +	const struct mv88e6xxx_avb_ops *avb_ops;
> +	const struct mv88e6xxx_qav_info *qav;

Reverse Christmas tree. Longest first please.

> +	int rate_reg;
> +	int hilimit_reg;
> +	u8 queue_bit;
> +	u16 rate = 0;
> +	u16 hi_limit;
> +	int err;
> +
> +	avb_ops = ops->avb_ops;
> +	qav = chip->info->qav;

You might be able to move this into the variable declarations? But not
always, depending on reverse Christmas.

> +	if (!qav || !avb_ops || !avb_ops->port_qav_write ||
> +	    !ops->port_set_scheduling_mode)
> +		return -EOPNOTSUPP;
> +
> +	if (!dsa_is_user_port(ds, port))
> +		return -EOPNOTSUPP;
> +
> +	if (cbs->queue < 0 || cbs->queue >= chip->info->num_tx_queues)
> +		return -EINVAL;
> +
> +	if (!(qav->queue_mask & BIT(cbs->queue)))
> +		return -EOPNOTSUPP;
> +
> +	queue_bit = BIT(cbs->queue);
> +	rate_reg = cbs->queue * 2;
> +	hilimit_reg = rate_reg + 1;
> +
> +	if (cbs->enable) {
> +		if (cbs->idleslope <= 0 ||
> +		    cbs->idleslope > qav->max_rate ||
> +		    cbs->sendslope >= 0 || cbs->hicredit <= 0 ||
> +		    cbs->hicredit > qav->hi_limit_mask)
> +			return -ERANGE;
> +
> +		rate = DIV_ROUND_UP(cbs->idleslope, qav->rate_unit);

In the commit message, you mentioned 20Mbps, for a 1G link. Does the
link speed actually matter? I don't see it here.

	Andrew