Messages in this thread | | | Date | Tue, 21 Nov 2023 13:11:00 +0200 | Subject | Re: [PATCH v6 net-next 2/7] net: ethernet: am65-cpsw: cleanup TAPRIO handling | From | Roger Quadros <> |
| |
On 21/11/2023 11:23, Roger Quadros wrote: > > > On 21/11/2023 00:56, Vladimir Oltean wrote: >> On Mon, Nov 20, 2023 at 04:01:42PM +0200, Roger Quadros wrote: >>> -static int am65_cpsw_configure_taprio(struct net_device *ndev, >>> - struct am65_cpsw_est *est_new) >>> +static void am65_cpsw_cp_taprio(struct tc_taprio_qopt_offload *from, >>> + struct tc_taprio_qopt_offload *to) >>> +{ >>> + int i; >>> + >>> + *to = *from; >>> + for (i = 0; i < from->num_entries; i++) >>> + to->entries[i] = from->entries[i]; >>> +} >> >> I think I mentioned this before: have you looked at taprio_offload_get() >> and taprio_offload_put()? > > I'm sorry that I missed this. I'll take a look.
Now I recollect. You mentioned this in a different series review https://lore.kernel.org/all/20231011102536.r65xyzmh5kap2cf2@skbuf/
Since this patch is more trivial cleanups I will do the taprio_offload_get/free() change to a separate patch. > >> >>> + >>> +static int am65_cpsw_taprio_replace(struct net_device *ndev, >>> + struct tc_taprio_qopt_offload *taprio) >>> { >>> struct am65_cpsw_common *common = am65_ndev_to_common(ndev); >>> + struct netlink_ext_ack *extack = taprio->mqprio.extack; >>> + struct am65_cpsw_port *port = am65_ndev_to_port(ndev); >>> struct am65_cpts *cpts = common->cpts; >>> int ret = 0, tact = TACT_PROG; >>> + struct am65_cpsw_est *est_new; >>> >>> - am65_cpsw_est_update_state(ndev); >>> + if (!netif_running(ndev)) { >>> + NL_SET_ERR_MSG_MOD(extack, "interface is down, link speed unknown\n"); >> >> The extack message doesn't need a \n. > > OK. > >> >>> + return -ENETDOWN; >>> + } >>> >>> - if (est_new->taprio.cmd == TAPRIO_CMD_DESTROY) { >>> - am65_cpsw_stop_est(ndev); >>> - return ret; >>> + if (common->pf_p0_rx_ptype_rrobin) { >>> + NL_SET_ERR_MSG_MOD(extack, >>> + "p0-rx-ptype-rrobin flag conflicts with taprio qdisc\n"); >> >> Also here. >> >>> + return -EINVAL; >>> + } >>> + >>> + if (port->qos.link_speed == SPEED_UNKNOWN) >>> + return -ENOLINK; >>> + >>> + if (taprio->cycle_time_extension) { >>> + NL_SET_ERR_MSG_MOD(extack, >>> + "cycle time extension not supported"); >> >> Here it's ok. >> >>> + return -EOPNOTSUPP; >>> } > > Thanks for the detailed review! >
-- cheers, -roger
| |