Handle offloading commands using switch-case in
am65_cpsw_setup_taprio().
Move checks to am65_cpsw_taprio_replace().
Use NL_SET_ERR_MSG_MOD for error messages.
Change error message from "Failed to set cycle time extension"
to "cycle time extension not supported"
Signed-off-by: Roger Quadros <rogerq@kernel.org>
---
drivers/net/ethernet/ti/am65-cpsw-qos.c | 149 +++++++++++-------------
1 file changed, 70 insertions(+), 79 deletions(-)
Changelog:
v6: initial commit
diff --git a/drivers/net/ethernet/ti/am65-cpsw-qos.c b/drivers/net/ethernet/ti/am65-cpsw-qos.c
index 4bc611cc4aad..3539fb31541b 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-qos.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-qos.c
@@ -428,7 +428,7 @@ static void am65_cpsw_stop_est(struct net_device *ndev)
am65_cpsw_timer_stop(ndev);
}
-static void am65_cpsw_purge_est(struct net_device *ndev)
+static void am65_cpsw_taprio_destroy(struct net_device *ndev)
{
struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
@@ -441,29 +441,66 @@ static void am65_cpsw_purge_est(struct net_device *ndev)
port->qos.est_admin = NULL;
}
-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];
+}
+
+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");
+ 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");
+ 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");
+ return -EOPNOTSUPP;
}
+ est_new = devm_kzalloc(&ndev->dev,
+ struct_size(est_new, taprio.entries, taprio->num_entries),
+ GFP_KERNEL);
+ if (!est_new)
+ return -ENOMEM;
+
+ am65_cpsw_cp_taprio(taprio, &est_new->taprio);
+
+ am65_cpsw_est_update_state(ndev);
+
ret = am65_cpsw_est_check_scheds(ndev, est_new);
if (ret < 0)
- return ret;
+ goto fail;
tact = am65_cpsw_timer_act(ndev, est_new);
if (tact == TACT_NEED_STOP) {
- dev_err(&ndev->dev,
- "Can't toggle estf timer, stop taprio first");
- return -EINVAL;
+ NL_SET_ERR_MSG_MOD(extack,
+ "Can't toggle estf timer, stop taprio first");
+ ret = -EINVAL;
+ goto fail;
}
if (tact == TACT_PROG)
@@ -476,62 +513,24 @@ static int am65_cpsw_configure_taprio(struct net_device *ndev,
am65_cpsw_est_set_sched_list(ndev, est_new);
am65_cpsw_port_est_assign_buf_num(ndev, est_new->buf);
- am65_cpsw_est_set(ndev, est_new->taprio.cmd == TAPRIO_CMD_REPLACE);
+ am65_cpsw_est_set(ndev, 1);
if (tact == TACT_PROG) {
ret = am65_cpsw_timer_set(ndev, est_new);
if (ret) {
- dev_err(&ndev->dev, "Failed to set cycle time");
- return ret;
+ NL_SET_ERR_MSG_MOD(extack,
+ "Failed to set cycle time");
+ goto fail;
}
}
- return ret;
-}
-
-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];
-}
-
-static int am65_cpsw_set_taprio(struct net_device *ndev, void *type_data)
-{
- struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
- struct tc_taprio_qopt_offload *taprio = type_data;
- struct am65_cpsw_est *est_new;
- int ret = 0;
-
- if (taprio->cycle_time_extension) {
- dev_err(&ndev->dev, "Failed to set cycle time extension");
- return -EOPNOTSUPP;
- }
-
- est_new = devm_kzalloc(&ndev->dev,
- struct_size(est_new, taprio.entries, taprio->num_entries),
- GFP_KERNEL);
- if (!est_new)
- return -ENOMEM;
-
- am65_cpsw_cp_taprio(taprio, &est_new->taprio);
- ret = am65_cpsw_configure_taprio(ndev, est_new);
- if (!ret) {
- if (taprio->cmd == TAPRIO_CMD_REPLACE) {
- devm_kfree(&ndev->dev, port->qos.est_admin);
+ devm_kfree(&ndev->dev, port->qos.est_admin);
+ port->qos.est_admin = est_new;
- port->qos.est_admin = est_new;
- } else {
- devm_kfree(&ndev->dev, est_new);
- am65_cpsw_purge_est(ndev);
- }
- } else {
- devm_kfree(&ndev->dev, est_new);
- }
+ return 0;
+fail:
+ devm_kfree(&ndev->dev, est_new);
return ret;
}
@@ -558,34 +557,26 @@ static void am65_cpsw_est_link_up(struct net_device *ndev, int link_speed)
return;
purge_est:
- am65_cpsw_purge_est(ndev);
+ am65_cpsw_taprio_destroy(ndev);
}
static int am65_cpsw_setup_taprio(struct net_device *ndev, void *type_data)
{
- struct am65_cpsw_port *port = am65_ndev_to_port(ndev);
struct tc_taprio_qopt_offload *taprio = type_data;
- struct am65_cpsw_common *common = port->common;
-
- if (taprio->cmd != TAPRIO_CMD_REPLACE &&
- taprio->cmd != TAPRIO_CMD_DESTROY)
- return -EOPNOTSUPP;
-
- if (!netif_running(ndev)) {
- dev_err(&ndev->dev, "interface is down, link speed unknown\n");
- return -ENETDOWN;
- }
-
- if (common->pf_p0_rx_ptype_rrobin) {
- dev_err(&ndev->dev,
- "p0-rx-ptype-rrobin flag conflicts with taprio qdisc\n");
- return -EINVAL;
+ int err = 0;
+
+ switch (taprio->cmd) {
+ case TAPRIO_CMD_REPLACE:
+ err = am65_cpsw_taprio_replace(ndev, taprio);
+ break;
+ case TAPRIO_CMD_DESTROY:
+ am65_cpsw_taprio_destroy(ndev);
+ break;
+ default:
+ err = -EOPNOTSUPP;
}
- if (port->qos.link_speed == SPEED_UNKNOWN)
- return -ENOLINK;
-
- return am65_cpsw_set_taprio(ndev, type_data);
+ return err;
}
static int am65_cpsw_tc_query_caps(struct net_device *ndev, void *type_data)
--
2.34.1On 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()?
> +
> +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.
> + 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;
> }
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.
>
>> +
>> +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
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
© 2016 - 2025 Red Hat, Inc.