From be1cc96ddf82bb0c0a159751f73239d6d3e9594a Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Sun, 4 May 2025 19:29:15 +0200 Subject: [PATCH 01/16] net: phy: realtek: use __set_bit() in rtl8211f_led_hw_control_get() rtl8211f_led_hw_control_get() does not need atomic bit operations, replace set_bit() by __set_bit(). Signed-off-by: Michael Klein Reviewed-by: Andrew Lunn Link: https://patch.msgid.link/20250504172916.243185-6-michael@fossekall.de Signed-off-by: Paolo Abeni --- drivers/net/phy/realtek/realtek_main.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c index e01b13a9b5c3..e26098a2ff27 100644 --- a/drivers/net/phy/realtek/realtek_main.c +++ b/drivers/net/phy/realtek/realtek_main.c @@ -659,17 +659,17 @@ static int rtl8211f_led_hw_control_get(struct phy_device *phydev, u8 index, val &= RTL8211F_LEDCR_MASK; if (val & RTL8211F_LEDCR_LINK_10) - set_bit(TRIGGER_NETDEV_LINK_10, rules); + __set_bit(TRIGGER_NETDEV_LINK_10, rules); if (val & RTL8211F_LEDCR_LINK_100) - set_bit(TRIGGER_NETDEV_LINK_100, rules); + __set_bit(TRIGGER_NETDEV_LINK_100, rules); if (val & RTL8211F_LEDCR_LINK_1000) - set_bit(TRIGGER_NETDEV_LINK_1000, rules); + __set_bit(TRIGGER_NETDEV_LINK_1000, rules); if (val & RTL8211F_LEDCR_ACT_TXRX) { - set_bit(TRIGGER_NETDEV_RX, rules); - set_bit(TRIGGER_NETDEV_TX, rules); + __set_bit(TRIGGER_NETDEV_RX, rules); + __set_bit(TRIGGER_NETDEV_TX, rules); } return 0; -- 2.51.0 From 708686132ba02659267c0cebcc414348ece389a5 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Sun, 4 May 2025 19:29:16 +0200 Subject: [PATCH 02/16] net: phy: realtek: Add support for PHY LEDs on RTL8211E Like the RTL8211F, the RTL8211E PHY supports up to three LEDs. Add netdev trigger support for them, too. Signed-off-by: Michael Klein Reviewed-by: Andrew Lunn Link: https://patch.msgid.link/20250504172916.243185-7-michael@fossekall.de Signed-off-by: Paolo Abeni --- drivers/net/phy/realtek/realtek_main.c | 125 +++++++++++++++++++++++-- 1 file changed, 119 insertions(+), 6 deletions(-) diff --git a/drivers/net/phy/realtek/realtek_main.c b/drivers/net/phy/realtek/realtek_main.c index e26098a2ff27..301fbe141b9b 100644 --- a/drivers/net/phy/realtek/realtek_main.c +++ b/drivers/net/phy/realtek/realtek_main.c @@ -40,6 +40,20 @@ #define RTL821x_PAGE_SELECT 0x1f #define RTL821x_SET_EXT_PAGE 0x07 +/* RTL8211E extension page 44/0x2c */ +#define RTL8211E_LEDCR_EXT_PAGE 0x2c +#define RTL8211E_LEDCR1 0x1a +#define RTL8211E_LEDCR1_ACT_TXRX BIT(4) +#define RTL8211E_LEDCR1_MASK BIT(4) +#define RTL8211E_LEDCR1_SHIFT 1 + +#define RTL8211E_LEDCR2 0x1c +#define RTL8211E_LEDCR2_LINK_1000 BIT(2) +#define RTL8211E_LEDCR2_LINK_100 BIT(1) +#define RTL8211E_LEDCR2_LINK_10 BIT(0) +#define RTL8211E_LEDCR2_MASK GENMASK(2, 0) +#define RTL8211E_LEDCR2_SHIFT 4 + /* RTL8211E extension page 164/0xa4 */ #define RTL8211E_RGMII_EXT_PAGE 0xa4 #define RTL8211E_RGMII_DELAY 0x1c @@ -145,7 +159,8 @@ #define RTL_8221B_VN_CG 0x001cc84a #define RTL_8251B 0x001cc862 -#define RTL8211F_LED_COUNT 3 +/* RTL8211E and RTL8211F support up to three LEDs */ +#define RTL8211x_LED_COUNT 3 MODULE_DESCRIPTION("Realtek PHY driver"); MODULE_AUTHOR("Johnson Leung"); @@ -169,6 +184,21 @@ static int rtl821x_write_page(struct phy_device *phydev, int page) return __phy_write(phydev, RTL821x_PAGE_SELECT, page); } +static int rtl821x_read_ext_page(struct phy_device *phydev, u16 ext_page, + u32 regnum) +{ + int oldpage, ret = 0; + + oldpage = phy_select_page(phydev, RTL821x_SET_EXT_PAGE); + if (oldpage >= 0) { + ret = __phy_write(phydev, RTL821x_EXT_PAGE_SELECT, ext_page); + if (ret == 0) + ret = __phy_read(phydev, regnum); + } + + return phy_restore_page(phydev, oldpage, ret); +} + static int rtl821x_modify_ext_page(struct phy_device *phydev, u16 ext_page, u32 regnum, u16 mask, u16 set) { @@ -608,7 +638,7 @@ static int rtl821x_resume(struct phy_device *phydev) return 0; } -static int rtl8211f_led_hw_is_supported(struct phy_device *phydev, u8 index, +static int rtl8211x_led_hw_is_supported(struct phy_device *phydev, u8 index, unsigned long rules) { const unsigned long mask = BIT(TRIGGER_NETDEV_LINK_10) | @@ -627,9 +657,11 @@ static int rtl8211f_led_hw_is_supported(struct phy_device *phydev, u8 index, * rates and Active indication always at all three 10+100+1000 * link rates. * This code currently uses mode B only. + * + * RTL8211E PHY LED has one mode, which works like RTL8211F mode B. */ - if (index >= RTL8211F_LED_COUNT) + if (index >= RTL8211x_LED_COUNT) return -EINVAL; /* Filter out any other unsupported triggers. */ @@ -648,7 +680,7 @@ static int rtl8211f_led_hw_control_get(struct phy_device *phydev, u8 index, { int val; - if (index >= RTL8211F_LED_COUNT) + if (index >= RTL8211x_LED_COUNT) return -EINVAL; val = phy_read_paged(phydev, 0xd04, RTL8211F_LEDCR); @@ -681,7 +713,7 @@ static int rtl8211f_led_hw_control_set(struct phy_device *phydev, u8 index, const u16 mask = RTL8211F_LEDCR_MASK << (RTL8211F_LEDCR_SHIFT * index); u16 reg = 0; - if (index >= RTL8211F_LED_COUNT) + if (index >= RTL8211x_LED_COUNT) return -EINVAL; if (test_bit(TRIGGER_NETDEV_LINK_10, &rules)) @@ -704,6 +736,84 @@ static int rtl8211f_led_hw_control_set(struct phy_device *phydev, u8 index, return phy_modify_paged(phydev, 0xd04, RTL8211F_LEDCR, mask, reg); } +static int rtl8211e_led_hw_control_get(struct phy_device *phydev, u8 index, + unsigned long *rules) +{ + int ret; + u16 cr1, cr2; + + if (index >= RTL8211x_LED_COUNT) + return -EINVAL; + + ret = rtl821x_read_ext_page(phydev, RTL8211E_LEDCR_EXT_PAGE, + RTL8211E_LEDCR1); + if (ret < 0) + return ret; + + cr1 = ret >> RTL8211E_LEDCR1_SHIFT * index; + if (cr1 & RTL8211E_LEDCR1_ACT_TXRX) { + __set_bit(TRIGGER_NETDEV_RX, rules); + __set_bit(TRIGGER_NETDEV_TX, rules); + } + + ret = rtl821x_read_ext_page(phydev, RTL8211E_LEDCR_EXT_PAGE, + RTL8211E_LEDCR2); + if (ret < 0) + return ret; + + cr2 = ret >> RTL8211E_LEDCR2_SHIFT * index; + if (cr2 & RTL8211E_LEDCR2_LINK_10) + __set_bit(TRIGGER_NETDEV_LINK_10, rules); + + if (cr2 & RTL8211E_LEDCR2_LINK_100) + __set_bit(TRIGGER_NETDEV_LINK_100, rules); + + if (cr2 & RTL8211E_LEDCR2_LINK_1000) + __set_bit(TRIGGER_NETDEV_LINK_1000, rules); + + return ret; +} + +static int rtl8211e_led_hw_control_set(struct phy_device *phydev, u8 index, + unsigned long rules) +{ + const u16 cr1mask = + RTL8211E_LEDCR1_MASK << (RTL8211E_LEDCR1_SHIFT * index); + const u16 cr2mask = + RTL8211E_LEDCR2_MASK << (RTL8211E_LEDCR2_SHIFT * index); + u16 cr1 = 0, cr2 = 0; + int ret; + + if (index >= RTL8211x_LED_COUNT) + return -EINVAL; + + if (test_bit(TRIGGER_NETDEV_RX, &rules) || + test_bit(TRIGGER_NETDEV_TX, &rules)) { + cr1 |= RTL8211E_LEDCR1_ACT_TXRX; + } + + cr1 <<= RTL8211E_LEDCR1_SHIFT * index; + ret = rtl821x_modify_ext_page(phydev, RTL8211E_LEDCR_EXT_PAGE, + RTL8211E_LEDCR1, cr1mask, cr1); + if (ret < 0) + return ret; + + if (test_bit(TRIGGER_NETDEV_LINK_10, &rules)) + cr2 |= RTL8211E_LEDCR2_LINK_10; + + if (test_bit(TRIGGER_NETDEV_LINK_100, &rules)) + cr2 |= RTL8211E_LEDCR2_LINK_100; + + if (test_bit(TRIGGER_NETDEV_LINK_1000, &rules)) + cr2 |= RTL8211E_LEDCR2_LINK_1000; + + cr2 <<= RTL8211E_LEDCR2_SHIFT * index; + ret = rtl821x_modify_ext_page(phydev, RTL8211E_LEDCR_EXT_PAGE, + RTL8211E_LEDCR2, cr2mask, cr2); + + return ret; +} + static int rtl8211e_config_init(struct phy_device *phydev) { u16 val; @@ -1479,6 +1589,9 @@ static struct phy_driver realtek_drvs[] = { .resume = genphy_resume, .read_page = rtl821x_read_page, .write_page = rtl821x_write_page, + .led_hw_is_supported = rtl8211x_led_hw_is_supported, + .led_hw_control_get = rtl8211e_led_hw_control_get, + .led_hw_control_set = rtl8211e_led_hw_control_set, }, { PHY_ID_MATCH_EXACT(0x001cc916), .name = "RTL8211F Gigabit Ethernet", @@ -1494,7 +1607,7 @@ static struct phy_driver realtek_drvs[] = { .read_page = rtl821x_read_page, .write_page = rtl821x_write_page, .flags = PHY_ALWAYS_CALL_SUSPEND, - .led_hw_is_supported = rtl8211f_led_hw_is_supported, + .led_hw_is_supported = rtl8211x_led_hw_is_supported, .led_hw_control_get = rtl8211f_led_hw_control_get, .led_hw_control_set = rtl8211f_led_hw_control_set, }, { -- 2.51.0 From df6a69bc8f31fc34ac1f5408a82e60a3f31d905e Mon Sep 17 00:00:00 2001 From: David Wei Date: Fri, 2 May 2025 21:30:07 -0700 Subject: [PATCH 03/16] io_uring/zcrx: selftests: fix setting ntuple rule into rss Fix ethtool syntax for setting ntuple rule into rss. It should be `context' instead of `action'. Signed-off-by: David Wei Link: https://patch.msgid.link/20250503043007.857215-1-dw@davidwei.uk Signed-off-by: Jakub Kicinski --- tools/testing/selftests/drivers/net/hw/iou-zcrx.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py index aef43f82edd5..9c03fd777f3d 100755 --- a/tools/testing/selftests/drivers/net/hw/iou-zcrx.py +++ b/tools/testing/selftests/drivers/net/hw/iou-zcrx.py @@ -19,8 +19,8 @@ def _get_combined_channels(cfg): return int(values[1]) -def _create_rss_ctx(cfg, chans): - output = ethtool(f"-X {cfg.ifname} context new start {chans - 1} equal 1", host=cfg.remote).stdout +def _create_rss_ctx(cfg, chan): + output = ethtool(f"-X {cfg.ifname} context new start {chan} equal 1", host=cfg.remote).stdout values = re.search(r'New RSS context is (\d+)', output).group(1) ctx_id = int(values) return (ctx_id, defer(ethtool, f"-X {cfg.ifname} delete context {ctx_id}", host=cfg.remote)) @@ -32,8 +32,8 @@ def _set_flow_rule(cfg, port, chan): return int(values) -def _set_flow_rule_rss(cfg, port, chan): - output = ethtool(f"-N {cfg.ifname} flow-type tcp6 dst-port {port} action {chan}", host=cfg.remote).stdout +def _set_flow_rule_rss(cfg, port, ctx_id): + output = ethtool(f"-N {cfg.ifname} flow-type tcp6 dst-port {port} context {ctx_id}", host=cfg.remote).stdout values = re.search(r'ID (\d+)', output).group(1) return int(values) @@ -121,7 +121,7 @@ def test_zcrx_rss(cfg) -> None: ethtool(f"-X {cfg.ifname} equal {combined_chans - 1}", host=cfg.remote) defer(ethtool, f"-X {cfg.ifname} default", host=cfg.remote) - (ctx_id, delete_ctx) = _create_rss_ctx(cfg, combined_chans) + (ctx_id, delete_ctx) = _create_rss_ctx(cfg, combined_chans - 1) flow_rule_id = _set_flow_rule_rss(cfg, port, ctx_id) defer(ethtool, f"-N {cfg.ifname} delete {flow_rule_id}", host=cfg.remote) -- 2.51.0 From 37006af675e8ced690ffb2285d0f15a98323a7de Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Mon, 5 May 2025 13:45:10 +0200 Subject: [PATCH 04/16] tools: ynl-gen: allow noncontiguous enums in case the enum has holes, instead of hard stop, generate a validation callback to check valid enum values. Signed-off-by: Jiri Pirko Link: https://patch.msgid.link/20250505114513.53370-2-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- tools/net/ynl/pyynl/ynl_gen_c.py | 58 ++++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 6 deletions(-) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index 09b87c9a6908..acba03bbe67d 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -389,11 +389,14 @@ class TypeScalar(Type): if 'enum' in self.attr: enum = self.family.consts[self.attr['enum']] low, high = enum.value_range() - if 'min' not in self.checks: - if low != 0 or self.type[0] == 's': - self.checks['min'] = low - if 'max' not in self.checks: - self.checks['max'] = high + if low == None and high == None: + self.checks['sparse'] = True + else: + if 'min' not in self.checks: + if low != 0 or self.type[0] == 's': + self.checks['min'] = low + if 'max' not in self.checks: + self.checks['max'] = high if 'min' in self.checks and 'max' in self.checks: if self.get_limit('min') > self.get_limit('max'): @@ -425,6 +428,8 @@ class TypeScalar(Type): return f"NLA_POLICY_MIN({policy}, {self.get_limit_str('min')})" elif 'max' in self.checks: return f"NLA_POLICY_MAX({policy}, {self.get_limit_str('max')})" + elif 'sparse' in self.checks: + return f"NLA_POLICY_VALIDATE_FN({policy}, &{c_lower(self.enum_name)}_validate)" return super()._attr_policy(policy) def _attr_typol(self): @@ -930,7 +935,7 @@ class EnumSet(SpecEnumSet): high = max([x.value for x in self.entries.values()]) if high - low + 1 != len(self.entries): - raise Exception("Can't get value range for a noncontiguous enum") + return None, None return low, high @@ -2426,6 +2431,46 @@ def print_kernel_policy_ranges(family, cw): cw.nl() +def print_kernel_policy_sparse_enum_validates(family, cw): + first = True + for _, attr_set in family.attr_sets.items(): + if attr_set.subset_of: + continue + + for _, attr in attr_set.items(): + if not attr.request: + continue + if not attr.enum_name: + continue + if 'sparse' not in attr.checks: + continue + + if first: + cw.p('/* Sparse enums validation callbacks */') + first = False + + sign = '' if attr.type[0] == 'u' else '_signed' + suffix = 'ULL' if attr.type[0] == 'u' else 'LL' + cw.write_func_prot('static int', f'{c_lower(attr.enum_name)}_validate', + ['const struct nlattr *attr', 'struct netlink_ext_ack *extack']) + cw.block_start() + cw.block_start(line=f'switch (nla_get_{attr["type"]}(attr))') + enum = family.consts[attr['enum']] + first_entry = True + for entry in enum.entries.values(): + if first_entry: + first_entry = False + else: + cw.p('fallthrough;') + cw.p(f'case {entry.c_name}:') + cw.p('return 0;') + cw.block_end() + cw.p('NL_SET_ERR_MSG_ATTR(extack, attr, "invalid enum value");') + cw.p('return -EINVAL;') + cw.block_end() + cw.nl() + + def print_kernel_op_table_fwd(family, cw, terminate): exported = not kernel_can_gen_family_struct(family) @@ -3097,6 +3142,7 @@ def main(): print_kernel_family_struct_hdr(parsed, cw) else: print_kernel_policy_ranges(parsed, cw) + print_kernel_policy_sparse_enum_validates(parsed, cw) for _, struct in sorted(parsed.pure_nested_structs.items()): if struct.request: -- 2.51.0 From 429ac6211494c12b668dac59811ea8a96db6d757 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Mon, 5 May 2025 13:45:11 +0200 Subject: [PATCH 05/16] devlink: define enum for attr types of dynamic attributes Devlink param and health reporter fmsg use attributes with dynamic type which is determined according to a different type. Currently used values are NLA_*. The problem is, they are not part of UAPI. They may change which would cause a break. To make this future safe, introduce a enum that shadows NLA_* values in it and is part of UAPI. Also, this allows to possibly carry types that are unrelated to NLA_* values. Signed-off-by: Saeed Mahameed Signed-off-by: Jiri Pirko Link: https://patch.msgid.link/20250505114513.53370-3-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- Documentation/netlink/specs/devlink.yaml | 24 +++++++++++++++++++ include/uapi/linux/devlink.h | 15 ++++++++++++ net/devlink/health.c | 17 ++++++++++++-- net/devlink/netlink_gen.c | 29 ++++++++++++++++++++++- net/devlink/param.c | 30 ++++++++++++------------ 5 files changed, 97 insertions(+), 18 deletions(-) diff --git a/Documentation/netlink/specs/devlink.yaml b/Documentation/netlink/specs/devlink.yaml index bd9726269b4f..05fee1b7fe19 100644 --- a/Documentation/netlink/specs/devlink.yaml +++ b/Documentation/netlink/specs/devlink.yaml @@ -202,6 +202,28 @@ definitions: name: exception - name: control + - + type: enum + name: var-attr-type + entries: + - + name: u8 + value: 1 + - + name: u16 + - + name: u32 + - + name: u64 + - + name: string + - + name: flag + - + name: nul_string + value: 10 + - + name: binary attribute-sets: - @@ -498,6 +520,7 @@ attribute-sets: - name: param-type type: u8 + enum: var-attr-type # TODO: fill in the attributes in between @@ -592,6 +615,7 @@ attribute-sets: - name: fmsg-obj-value-type type: u8 + enum: var-attr-type # TODO: fill in the attributes in between diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h index 9401aa343673..a5ee0f13740a 100644 --- a/include/uapi/linux/devlink.h +++ b/include/uapi/linux/devlink.h @@ -385,6 +385,21 @@ enum devlink_linecard_state { DEVLINK_LINECARD_STATE_MAX = __DEVLINK_LINECARD_STATE_MAX - 1 }; +/* Variable attribute type. */ +enum devlink_var_attr_type { + /* Following values relate to the internal NLA_* values */ + DEVLINK_VAR_ATTR_TYPE_U8 = 1, + DEVLINK_VAR_ATTR_TYPE_U16, + DEVLINK_VAR_ATTR_TYPE_U32, + DEVLINK_VAR_ATTR_TYPE_U64, + DEVLINK_VAR_ATTR_TYPE_STRING, + DEVLINK_VAR_ATTR_TYPE_FLAG, + DEVLINK_VAR_ATTR_TYPE_NUL_STRING = 10, + DEVLINK_VAR_ATTR_TYPE_BINARY, + __DEVLINK_VAR_ATTR_TYPE_CUSTOM_BASE = 0x80, + /* Any possible custom types, unrelated to NLA_* values go below */ +}; + enum devlink_attr { /* don't change the order or add anything between, this is ABI! */ DEVLINK_ATTR_UNSPEC, diff --git a/net/devlink/health.c b/net/devlink/health.c index 57db6799722a..95a7a62d85a2 100644 --- a/net/devlink/health.c +++ b/net/devlink/health.c @@ -930,18 +930,31 @@ EXPORT_SYMBOL_GPL(devlink_fmsg_binary_pair_put); static int devlink_fmsg_item_fill_type(struct devlink_fmsg_item *msg, struct sk_buff *skb) { + enum devlink_var_attr_type var_attr_type; + switch (msg->nla_type) { case NLA_FLAG: + var_attr_type = DEVLINK_VAR_ATTR_TYPE_FLAG; + break; case NLA_U8: + var_attr_type = DEVLINK_VAR_ATTR_TYPE_U8; + break; case NLA_U32: + var_attr_type = DEVLINK_VAR_ATTR_TYPE_U32; + break; case NLA_U64: + var_attr_type = DEVLINK_VAR_ATTR_TYPE_U64; + break; case NLA_NUL_STRING: + var_attr_type = DEVLINK_VAR_ATTR_TYPE_NUL_STRING; + break; case NLA_BINARY: - return nla_put_u8(skb, DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE, - msg->nla_type); + var_attr_type = DEVLINK_VAR_ATTR_TYPE_BINARY; + break; default: return -EINVAL; } + return nla_put_u8(skb, DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE, var_attr_type); } static int diff --git a/net/devlink/netlink_gen.c b/net/devlink/netlink_gen.c index f9786d51f68f..e340d955cf3b 100644 --- a/net/devlink/netlink_gen.c +++ b/net/devlink/netlink_gen.c @@ -10,6 +10,33 @@ #include +/* Sparse enums validation callbacks */ +static int +devlink_attr_param_type_validate(const struct nlattr *attr, + struct netlink_ext_ack *extack) +{ + switch (nla_get_u8(attr)) { + case DEVLINK_VAR_ATTR_TYPE_U8: + fallthrough; + case DEVLINK_VAR_ATTR_TYPE_U16: + fallthrough; + case DEVLINK_VAR_ATTR_TYPE_U32: + fallthrough; + case DEVLINK_VAR_ATTR_TYPE_U64: + fallthrough; + case DEVLINK_VAR_ATTR_TYPE_STRING: + fallthrough; + case DEVLINK_VAR_ATTR_TYPE_FLAG: + fallthrough; + case DEVLINK_VAR_ATTR_TYPE_NUL_STRING: + fallthrough; + case DEVLINK_VAR_ATTR_TYPE_BINARY: + return 0; + } + NL_SET_ERR_MSG_ATTR(extack, attr, "invalid enum value"); + return -EINVAL; +} + /* Common nested types */ const struct nla_policy devlink_dl_port_function_nl_policy[DEVLINK_PORT_FN_ATTR_CAPS + 1] = { [DEVLINK_PORT_FUNCTION_ATTR_HW_ADDR] = { .type = NLA_BINARY, }, @@ -273,7 +300,7 @@ static const struct nla_policy devlink_param_set_nl_policy[DEVLINK_ATTR_PARAM_VA [DEVLINK_ATTR_BUS_NAME] = { .type = NLA_NUL_STRING, }, [DEVLINK_ATTR_DEV_NAME] = { .type = NLA_NUL_STRING, }, [DEVLINK_ATTR_PARAM_NAME] = { .type = NLA_NUL_STRING, }, - [DEVLINK_ATTR_PARAM_TYPE] = { .type = NLA_U8, }, + [DEVLINK_ATTR_PARAM_TYPE] = NLA_POLICY_VALIDATE_FN(NLA_U8, &devlink_attr_param_type_validate), [DEVLINK_ATTR_PARAM_VALUE_CMODE] = NLA_POLICY_MAX(NLA_U8, 2), }; diff --git a/net/devlink/param.c b/net/devlink/param.c index dcf0d1ccebba..d0fb7c88bdb8 100644 --- a/net/devlink/param.c +++ b/net/devlink/param.c @@ -167,19 +167,19 @@ static int devlink_param_set(struct devlink *devlink, } static int -devlink_param_type_to_nla_type(enum devlink_param_type param_type) +devlink_param_type_to_var_attr_type(enum devlink_param_type param_type) { switch (param_type) { case DEVLINK_PARAM_TYPE_U8: - return NLA_U8; + return DEVLINK_VAR_ATTR_TYPE_U8; case DEVLINK_PARAM_TYPE_U16: - return NLA_U16; + return DEVLINK_VAR_ATTR_TYPE_U16; case DEVLINK_PARAM_TYPE_U32: - return NLA_U32; + return DEVLINK_VAR_ATTR_TYPE_U32; case DEVLINK_PARAM_TYPE_STRING: - return NLA_STRING; + return DEVLINK_VAR_ATTR_TYPE_STRING; case DEVLINK_PARAM_TYPE_BOOL: - return NLA_FLAG; + return DEVLINK_VAR_ATTR_TYPE_FLAG; default: return -EINVAL; } @@ -247,7 +247,7 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink, struct devlink_param_gset_ctx ctx; struct nlattr *param_values_list; struct nlattr *param_attr; - int nla_type; + int var_attr_type; void *hdr; int err; int i; @@ -294,10 +294,10 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink, if (param->generic && nla_put_flag(msg, DEVLINK_ATTR_PARAM_GENERIC)) goto param_nest_cancel; - nla_type = devlink_param_type_to_nla_type(param->type); - if (nla_type < 0) + var_attr_type = devlink_param_type_to_var_attr_type(param->type); + if (var_attr_type < 0) goto param_nest_cancel; - if (nla_put_u8(msg, DEVLINK_ATTR_PARAM_TYPE, nla_type)) + if (nla_put_u8(msg, DEVLINK_ATTR_PARAM_TYPE, var_attr_type)) goto param_nest_cancel; param_values_list = nla_nest_start_noflag(msg, @@ -420,19 +420,19 @@ devlink_param_type_get_from_info(struct genl_info *info, return -EINVAL; switch (nla_get_u8(info->attrs[DEVLINK_ATTR_PARAM_TYPE])) { - case NLA_U8: + case DEVLINK_VAR_ATTR_TYPE_U8: *param_type = DEVLINK_PARAM_TYPE_U8; break; - case NLA_U16: + case DEVLINK_VAR_ATTR_TYPE_U16: *param_type = DEVLINK_PARAM_TYPE_U16; break; - case NLA_U32: + case DEVLINK_VAR_ATTR_TYPE_U32: *param_type = DEVLINK_PARAM_TYPE_U32; break; - case NLA_STRING: + case DEVLINK_VAR_ATTR_TYPE_STRING: *param_type = DEVLINK_PARAM_TYPE_STRING; break; - case NLA_FLAG: + case DEVLINK_VAR_ATTR_TYPE_FLAG: *param_type = DEVLINK_PARAM_TYPE_BOOL; break; default: -- 2.51.0 From f9e78932eac650cf1385244482b85e65ccaa87cf Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Mon, 5 May 2025 13:45:12 +0200 Subject: [PATCH 06/16] devlink: avoid param type value translations Assign DEVLINK_PARAM_TYPE_* enum values to DEVLINK_VAR_ATTR_TYPE_* to ensure the same values are used internally and in UAPI. Benefit from that by removing the value translations. Signed-off-by: Jiri Pirko Link: https://patch.msgid.link/20250505114513.53370-4-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- include/net/devlink.h | 10 +++++----- net/devlink/param.c | 46 ++----------------------------------------- 2 files changed, 7 insertions(+), 49 deletions(-) diff --git a/include/net/devlink.h b/include/net/devlink.h index b8783126c1ed..0091f23a40f7 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -420,11 +420,11 @@ typedef u64 devlink_resource_occ_get_t(void *priv); #define __DEVLINK_PARAM_MAX_STRING_VALUE 32 enum devlink_param_type { - DEVLINK_PARAM_TYPE_U8, - DEVLINK_PARAM_TYPE_U16, - DEVLINK_PARAM_TYPE_U32, - DEVLINK_PARAM_TYPE_STRING, - DEVLINK_PARAM_TYPE_BOOL, + DEVLINK_PARAM_TYPE_U8 = DEVLINK_VAR_ATTR_TYPE_U8, + DEVLINK_PARAM_TYPE_U16 = DEVLINK_VAR_ATTR_TYPE_U16, + DEVLINK_PARAM_TYPE_U32 = DEVLINK_VAR_ATTR_TYPE_U32, + DEVLINK_PARAM_TYPE_STRING = DEVLINK_VAR_ATTR_TYPE_STRING, + DEVLINK_PARAM_TYPE_BOOL = DEVLINK_VAR_ATTR_TYPE_FLAG, }; union devlink_param_value { diff --git a/net/devlink/param.c b/net/devlink/param.c index d0fb7c88bdb8..b29abf8d3ed4 100644 --- a/net/devlink/param.c +++ b/net/devlink/param.c @@ -166,25 +166,6 @@ static int devlink_param_set(struct devlink *devlink, return param->set(devlink, param->id, ctx, extack); } -static int -devlink_param_type_to_var_attr_type(enum devlink_param_type param_type) -{ - switch (param_type) { - case DEVLINK_PARAM_TYPE_U8: - return DEVLINK_VAR_ATTR_TYPE_U8; - case DEVLINK_PARAM_TYPE_U16: - return DEVLINK_VAR_ATTR_TYPE_U16; - case DEVLINK_PARAM_TYPE_U32: - return DEVLINK_VAR_ATTR_TYPE_U32; - case DEVLINK_PARAM_TYPE_STRING: - return DEVLINK_VAR_ATTR_TYPE_STRING; - case DEVLINK_PARAM_TYPE_BOOL: - return DEVLINK_VAR_ATTR_TYPE_FLAG; - default: - return -EINVAL; - } -} - static int devlink_nl_param_value_fill_one(struct sk_buff *msg, enum devlink_param_type type, @@ -247,7 +228,6 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink, struct devlink_param_gset_ctx ctx; struct nlattr *param_values_list; struct nlattr *param_attr; - int var_attr_type; void *hdr; int err; int i; @@ -293,11 +273,7 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink, goto param_nest_cancel; if (param->generic && nla_put_flag(msg, DEVLINK_ATTR_PARAM_GENERIC)) goto param_nest_cancel; - - var_attr_type = devlink_param_type_to_var_attr_type(param->type); - if (var_attr_type < 0) - goto param_nest_cancel; - if (nla_put_u8(msg, DEVLINK_ATTR_PARAM_TYPE, var_attr_type)) + if (nla_put_u8(msg, DEVLINK_ATTR_PARAM_TYPE, param->type)) goto param_nest_cancel; param_values_list = nla_nest_start_noflag(msg, @@ -419,25 +395,7 @@ devlink_param_type_get_from_info(struct genl_info *info, if (GENL_REQ_ATTR_CHECK(info, DEVLINK_ATTR_PARAM_TYPE)) return -EINVAL; - switch (nla_get_u8(info->attrs[DEVLINK_ATTR_PARAM_TYPE])) { - case DEVLINK_VAR_ATTR_TYPE_U8: - *param_type = DEVLINK_PARAM_TYPE_U8; - break; - case DEVLINK_VAR_ATTR_TYPE_U16: - *param_type = DEVLINK_PARAM_TYPE_U16; - break; - case DEVLINK_VAR_ATTR_TYPE_U32: - *param_type = DEVLINK_PARAM_TYPE_U32; - break; - case DEVLINK_VAR_ATTR_TYPE_STRING: - *param_type = DEVLINK_PARAM_TYPE_STRING; - break; - case DEVLINK_VAR_ATTR_TYPE_FLAG: - *param_type = DEVLINK_PARAM_TYPE_BOOL; - break; - default: - return -EINVAL; - } + *param_type = nla_get_u8(info->attrs[DEVLINK_ATTR_PARAM_TYPE]); return 0; } -- 2.51.0 From 88debb521f15d73728d55294f458992f5f69a7c4 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Mon, 5 May 2025 13:45:13 +0200 Subject: [PATCH 07/16] devlink: use DEVLINK_VAR_ATTR_TYPE_* instead of NLA_* in fmsg Use newly introduced DEVLINK_VAR_ATTR_TYPE_* enum values instead of internal NLA_* in fmsg health reporter code. Signed-off-by: Jiri Pirko Link: https://patch.msgid.link/20250505114513.53370-5-jiri@resnulli.us Signed-off-by: Jakub Kicinski --- net/devlink/health.c | 65 ++++++++++++++------------------------------ 1 file changed, 21 insertions(+), 44 deletions(-) diff --git a/net/devlink/health.c b/net/devlink/health.c index 95a7a62d85a2..b3ce8ecbb7fb 100644 --- a/net/devlink/health.c +++ b/net/devlink/health.c @@ -735,7 +735,7 @@ static void devlink_fmsg_put_name(struct devlink_fmsg *fmsg, const char *name) return; } - item->nla_type = NLA_NUL_STRING; + item->nla_type = DEVLINK_VAR_ATTR_TYPE_NUL_STRING; item->len = strlen(name) + 1; item->attrtype = DEVLINK_ATTR_FMSG_OBJ_NAME; memcpy(&item->value, name, item->len); @@ -822,32 +822,37 @@ static void devlink_fmsg_put_value(struct devlink_fmsg *fmsg, static void devlink_fmsg_bool_put(struct devlink_fmsg *fmsg, bool value) { devlink_fmsg_err_if_binary(fmsg); - devlink_fmsg_put_value(fmsg, &value, sizeof(value), NLA_FLAG); + devlink_fmsg_put_value(fmsg, &value, sizeof(value), + DEVLINK_VAR_ATTR_TYPE_FLAG); } static void devlink_fmsg_u8_put(struct devlink_fmsg *fmsg, u8 value) { devlink_fmsg_err_if_binary(fmsg); - devlink_fmsg_put_value(fmsg, &value, sizeof(value), NLA_U8); + devlink_fmsg_put_value(fmsg, &value, sizeof(value), + DEVLINK_VAR_ATTR_TYPE_U8); } void devlink_fmsg_u32_put(struct devlink_fmsg *fmsg, u32 value) { devlink_fmsg_err_if_binary(fmsg); - devlink_fmsg_put_value(fmsg, &value, sizeof(value), NLA_U32); + devlink_fmsg_put_value(fmsg, &value, sizeof(value), + DEVLINK_VAR_ATTR_TYPE_U32); } EXPORT_SYMBOL_GPL(devlink_fmsg_u32_put); static void devlink_fmsg_u64_put(struct devlink_fmsg *fmsg, u64 value) { devlink_fmsg_err_if_binary(fmsg); - devlink_fmsg_put_value(fmsg, &value, sizeof(value), NLA_U64); + devlink_fmsg_put_value(fmsg, &value, sizeof(value), + DEVLINK_VAR_ATTR_TYPE_U64); } void devlink_fmsg_string_put(struct devlink_fmsg *fmsg, const char *value) { devlink_fmsg_err_if_binary(fmsg); - devlink_fmsg_put_value(fmsg, value, strlen(value) + 1, NLA_NUL_STRING); + devlink_fmsg_put_value(fmsg, value, strlen(value) + 1, + DEVLINK_VAR_ATTR_TYPE_NUL_STRING); } EXPORT_SYMBOL_GPL(devlink_fmsg_string_put); @@ -857,7 +862,8 @@ void devlink_fmsg_binary_put(struct devlink_fmsg *fmsg, const void *value, if (!fmsg->err && !fmsg->putting_binary) fmsg->err = -EINVAL; - devlink_fmsg_put_value(fmsg, value, value_len, NLA_BINARY); + devlink_fmsg_put_value(fmsg, value, value_len, + DEVLINK_VAR_ATTR_TYPE_BINARY); } EXPORT_SYMBOL_GPL(devlink_fmsg_binary_put); @@ -927,36 +933,6 @@ void devlink_fmsg_binary_pair_put(struct devlink_fmsg *fmsg, const char *name, } EXPORT_SYMBOL_GPL(devlink_fmsg_binary_pair_put); -static int -devlink_fmsg_item_fill_type(struct devlink_fmsg_item *msg, struct sk_buff *skb) -{ - enum devlink_var_attr_type var_attr_type; - - switch (msg->nla_type) { - case NLA_FLAG: - var_attr_type = DEVLINK_VAR_ATTR_TYPE_FLAG; - break; - case NLA_U8: - var_attr_type = DEVLINK_VAR_ATTR_TYPE_U8; - break; - case NLA_U32: - var_attr_type = DEVLINK_VAR_ATTR_TYPE_U32; - break; - case NLA_U64: - var_attr_type = DEVLINK_VAR_ATTR_TYPE_U64; - break; - case NLA_NUL_STRING: - var_attr_type = DEVLINK_VAR_ATTR_TYPE_NUL_STRING; - break; - case NLA_BINARY: - var_attr_type = DEVLINK_VAR_ATTR_TYPE_BINARY; - break; - default: - return -EINVAL; - } - return nla_put_u8(skb, DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE, var_attr_type); -} - static int devlink_fmsg_item_fill_data(struct devlink_fmsg_item *msg, struct sk_buff *skb) { @@ -964,20 +940,20 @@ devlink_fmsg_item_fill_data(struct devlink_fmsg_item *msg, struct sk_buff *skb) u8 tmp; switch (msg->nla_type) { - case NLA_FLAG: + case DEVLINK_VAR_ATTR_TYPE_FLAG: /* Always provide flag data, regardless of its value */ tmp = *(bool *)msg->value; return nla_put_u8(skb, attrtype, tmp); - case NLA_U8: + case DEVLINK_VAR_ATTR_TYPE_U8: return nla_put_u8(skb, attrtype, *(u8 *)msg->value); - case NLA_U32: + case DEVLINK_VAR_ATTR_TYPE_U32: return nla_put_u32(skb, attrtype, *(u32 *)msg->value); - case NLA_U64: + case DEVLINK_VAR_ATTR_TYPE_U64: return devlink_nl_put_u64(skb, attrtype, *(u64 *)msg->value); - case NLA_NUL_STRING: + case DEVLINK_VAR_ATTR_TYPE_NUL_STRING: return nla_put_string(skb, attrtype, (char *)&msg->value); - case NLA_BINARY: + case DEVLINK_VAR_ATTR_TYPE_BINARY: return nla_put(skb, attrtype, msg->len, (void *)&msg->value); default: return -EINVAL; @@ -1011,7 +987,8 @@ devlink_fmsg_prepare_skb(struct devlink_fmsg *fmsg, struct sk_buff *skb, err = nla_put_flag(skb, item->attrtype); break; case DEVLINK_ATTR_FMSG_OBJ_VALUE_DATA: - err = devlink_fmsg_item_fill_type(item, skb); + err = nla_put_u8(skb, DEVLINK_ATTR_FMSG_OBJ_VALUE_TYPE, + item->nla_type); if (err) break; err = devlink_fmsg_item_fill_data(item, skb); -- 2.51.0 From 232aa459aa40a6f3c7d27063b3e9c1d57786eaff Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Mon, 5 May 2025 10:43:35 +0200 Subject: [PATCH 08/16] net: usb: lan78xx: Improve error handling in PHY initialization Ensure that return values from `lan78xx_write_reg()`, `lan78xx_read_reg()`, and `phy_find_first()` are properly checked and propagated. Use `ERR_PTR(ret)` for error reporting in `lan7801_phy_init()` and replace `-EIO` with `-ENODEV` where appropriate to provide more accurate error codes. Signed-off-by: Oleksij Rempel Reviewed-by: Thangaraj Samynathan Signed-off-by: David S. Miller --- drivers/net/usb/lan78xx.c | 47 ++++++++++++++++++++++++++------------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index e4f1663b6204..19db18cf0504 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -2510,14 +2510,13 @@ static void lan78xx_remove_irq_domain(struct lan78xx_net *dev) static struct phy_device *lan7801_phy_init(struct lan78xx_net *dev) { - u32 buf; - int ret; struct fixed_phy_status fphy_status = { .link = 1, .speed = SPEED_1000, .duplex = DUPLEX_FULL, }; struct phy_device *phydev; + int ret; phydev = phy_find_first(dev->mdiobus); if (!phydev) { @@ -2525,30 +2524,40 @@ static struct phy_device *lan7801_phy_init(struct lan78xx_net *dev) phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL); if (IS_ERR(phydev)) { netdev_err(dev->net, "No PHY/fixed_PHY found\n"); - return NULL; + return ERR_PTR(-ENODEV); } netdev_dbg(dev->net, "Registered FIXED PHY\n"); dev->interface = PHY_INTERFACE_MODE_RGMII; ret = lan78xx_write_reg(dev, MAC_RGMII_ID, MAC_RGMII_ID_TXC_DELAY_EN_); + if (ret < 0) + return ERR_PTR(ret); + ret = lan78xx_write_reg(dev, RGMII_TX_BYP_DLL, 0x3D00); - ret = lan78xx_read_reg(dev, HW_CFG, &buf); - buf |= HW_CFG_CLK125_EN_; - buf |= HW_CFG_REFCLK25_EN_; - ret = lan78xx_write_reg(dev, HW_CFG, buf); + if (ret < 0) + return ERR_PTR(ret); + + ret = lan78xx_update_reg(dev, HW_CFG, HW_CFG_CLK125_EN_ | + HW_CFG_REFCLK25_EN_, + HW_CFG_CLK125_EN_ | HW_CFG_REFCLK25_EN_); + if (ret < 0) + return ERR_PTR(ret); } else { if (!phydev->drv) { netdev_err(dev->net, "no PHY driver found\n"); - return NULL; + return ERR_PTR(-EINVAL); } dev->interface = PHY_INTERFACE_MODE_RGMII_ID; /* The PHY driver is responsible to configure proper RGMII * interface delays. Disable RGMII delays on MAC side. */ - lan78xx_write_reg(dev, MAC_RGMII_ID, 0); + ret = lan78xx_write_reg(dev, MAC_RGMII_ID, 0); + if (ret < 0) + return ERR_PTR(ret); phydev->is_internal = false; } + return phydev; } @@ -2562,9 +2571,10 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) switch (dev->chipid) { case ID_REV_CHIP_ID_7801_: phydev = lan7801_phy_init(dev); - if (!phydev) { - netdev_err(dev->net, "lan7801: PHY Init Failed"); - return -EIO; + if (IS_ERR(phydev)) { + netdev_err(dev->net, "lan7801: failed to init PHY: %pe\n", + phydev); + return PTR_ERR(phydev); } break; @@ -2573,7 +2583,7 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) phydev = phy_find_first(dev->mdiobus); if (!phydev) { netdev_err(dev->net, "no PHY found\n"); - return -EIO; + return -ENODEV; } phydev->is_internal = true; dev->interface = PHY_INTERFACE_MODE_GMII; @@ -2581,7 +2591,7 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) default: netdev_err(dev->net, "Unknown CHIP ID found\n"); - return -EIO; + return -ENODEV; } /* if phyirq is not set, use polling mode in phylib */ @@ -2633,7 +2643,10 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) sizeof(u32)); if (len >= 0) { /* Ensure the appropriate LEDs are enabled */ - lan78xx_read_reg(dev, HW_CFG, ®); + ret = lan78xx_read_reg(dev, HW_CFG, ®); + if (ret < 0) + return ret; + reg &= ~(HW_CFG_LED0_EN_ | HW_CFG_LED1_EN_ | HW_CFG_LED2_EN_ | @@ -2642,7 +2655,9 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) (len > 1) * HW_CFG_LED1_EN_ | (len > 2) * HW_CFG_LED2_EN_ | (len > 3) * HW_CFG_LED3_EN_; - lan78xx_write_reg(dev, HW_CFG, reg); + ret = lan78xx_write_reg(dev, HW_CFG, reg); + if (ret < 0) + return ret; } } -- 2.51.0 From 3da0ae52705d609f7d080a8158cd46db222b29de Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Mon, 5 May 2025 10:43:36 +0200 Subject: [PATCH 09/16] net: usb: lan78xx: remove explicit check for missing PHY driver RGMII timing correctness relies on the PHY providing internal delays. This is typically ensured via PHY driver, strap pins, or PCB layout. Explicitly checking for a PHY driver here is unnecessary and non-standard. This logic applies to all MACs, not just LAN78xx, and should be left to phylib, phylink, or platform configuration. Drop the check and rely on standard subsystem behavior. Signed-off-by: Oleksij Rempel Reviewed-by: Thangaraj Samynathan Signed-off-by: David S. Miller --- drivers/net/usb/lan78xx.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index 19db18cf0504..9c0658227bde 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -2543,10 +2543,6 @@ static struct phy_device *lan7801_phy_init(struct lan78xx_net *dev) if (ret < 0) return ERR_PTR(ret); } else { - if (!phydev->drv) { - netdev_err(dev->net, "no PHY driver found\n"); - return ERR_PTR(-EINVAL); - } dev->interface = PHY_INTERFACE_MODE_RGMII_ID; /* The PHY driver is responsible to configure proper RGMII * interface delays. Disable RGMII delays on MAC side. -- 2.51.0 From d39f339d2603736ff90b0b32d9aea3b954307fd5 Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Mon, 5 May 2025 10:43:37 +0200 Subject: [PATCH 10/16] net: usb: lan78xx: refactor PHY init to separate detection and MAC configuration Split out PHY detection into lan78xx_get_phy() and MAC-side setup into lan78xx_mac_prepare_for_phy(), making the main lan78xx_phy_init() cleaner and easier to follow. This improves separation of concerns and prepares the code for a future transition to phylink. Fixed PHY registration and interface selection are now handled in lan78xx_get_phy(), while MAC-side delay configuration is done in lan78xx_mac_prepare_for_phy(). The fixed PHY fallback is preserved for setups like EVB-KSZ9897-1, where LAN7801 connects directly to a KSZ switch without a standard PHY or device tree support. No functional changes intended. Signed-off-by: Oleksij Rempel Reviewed-by: Thangaraj Samynathan Signed-off-by: David S. Miller --- drivers/net/usb/lan78xx.c | 174 ++++++++++++++++++++++++++++---------- 1 file changed, 128 insertions(+), 46 deletions(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index 9c0658227bde..7f1ecc415d53 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -2508,53 +2508,145 @@ static void lan78xx_remove_irq_domain(struct lan78xx_net *dev) dev->domain_data.irqdomain = NULL; } -static struct phy_device *lan7801_phy_init(struct lan78xx_net *dev) +/** + * lan78xx_register_fixed_phy() - Register a fallback fixed PHY + * @dev: LAN78xx device + * + * Registers a fixed PHY with 1 Gbps full duplex. This is used in special cases + * like EVB-KSZ9897-1, where LAN7801 acts as a USB-to-Ethernet interface to a + * switch without a visible PHY. + * + * Return: pointer to the registered fixed PHY, or ERR_PTR() on error. + */ +static struct phy_device *lan78xx_register_fixed_phy(struct lan78xx_net *dev) { struct fixed_phy_status fphy_status = { .link = 1, .speed = SPEED_1000, .duplex = DUPLEX_FULL, }; + + netdev_info(dev->net, + "No PHY found on LAN7801 – registering fixed PHY (e.g. EVB-KSZ9897-1)\n"); + + return fixed_phy_register(PHY_POLL, &fphy_status, NULL); +} + +/** + * lan78xx_get_phy() - Probe or register PHY device and set interface mode + * @dev: LAN78xx device structure + * + * This function attempts to find a PHY on the MDIO bus. If no PHY is found + * and the chip is LAN7801, it registers a fixed PHY as fallback. It also + * sets dev->interface based on chip ID and detected PHY type. + * + * Return: a valid PHY device pointer, or ERR_PTR() on failure. + */ +static struct phy_device *lan78xx_get_phy(struct lan78xx_net *dev) +{ struct phy_device *phydev; - int ret; + /* Attempt to locate a PHY on the MDIO bus */ phydev = phy_find_first(dev->mdiobus); - if (!phydev) { - netdev_dbg(dev->net, "PHY Not Found!! Registering Fixed PHY\n"); - phydev = fixed_phy_register(PHY_POLL, &fphy_status, NULL); - if (IS_ERR(phydev)) { - netdev_err(dev->net, "No PHY/fixed_PHY found\n"); - return ERR_PTR(-ENODEV); + + switch (dev->chipid) { + case ID_REV_CHIP_ID_7801_: + if (phydev) { + /* External RGMII PHY detected */ + dev->interface = PHY_INTERFACE_MODE_RGMII_ID; + phydev->is_internal = false; + + if (!phydev->drv) + netdev_warn(dev->net, + "PHY driver not found – assuming RGMII delays are on PCB or strapped for the PHY\n"); + + return phydev; } - netdev_dbg(dev->net, "Registered FIXED PHY\n"); + dev->interface = PHY_INTERFACE_MODE_RGMII; + /* No PHY found – fallback to fixed PHY (e.g. KSZ switch board) */ + return lan78xx_register_fixed_phy(dev); + + case ID_REV_CHIP_ID_7800_: + case ID_REV_CHIP_ID_7850_: + if (!phydev) + return ERR_PTR(-ENODEV); + + /* These use internal GMII-connected PHY */ + dev->interface = PHY_INTERFACE_MODE_GMII; + phydev->is_internal = true; + return phydev; + + default: + netdev_err(dev->net, "Unknown CHIP ID: 0x%08x\n", dev->chipid); + return ERR_PTR(-ENODEV); + } +} + +/** + * lan78xx_mac_prepare_for_phy() - Preconfigure MAC-side interface settings + * @dev: LAN78xx device + * + * Configure MAC-side registers according to dev->interface, which should be + * set by lan78xx_get_phy(). + * + * - For PHY_INTERFACE_MODE_RGMII: + * Enable MAC-side TXC delay. This mode seems to be used in a special setup + * without a real PHY, likely on EVB-KSZ9897-1. In that design, LAN7801 is + * connected to the KSZ9897 switch, and the link timing is expected to be + * hardwired (e.g. via strapping or board layout). No devicetree support is + * assumed here. + * + * - For PHY_INTERFACE_MODE_RGMII_ID: + * Disable MAC-side delay and rely on the PHY driver to provide delay. + * + * - For GMII, no MAC-specific config is needed. + * + * Return: 0 on success or a negative error code. + */ +static int lan78xx_mac_prepare_for_phy(struct lan78xx_net *dev) +{ + int ret; + + switch (dev->interface) { + case PHY_INTERFACE_MODE_RGMII: + /* Enable MAC-side TX clock delay */ ret = lan78xx_write_reg(dev, MAC_RGMII_ID, MAC_RGMII_ID_TXC_DELAY_EN_); if (ret < 0) - return ERR_PTR(ret); + return ret; ret = lan78xx_write_reg(dev, RGMII_TX_BYP_DLL, 0x3D00); if (ret < 0) - return ERR_PTR(ret); + return ret; - ret = lan78xx_update_reg(dev, HW_CFG, HW_CFG_CLK125_EN_ | - HW_CFG_REFCLK25_EN_, + ret = lan78xx_update_reg(dev, HW_CFG, + HW_CFG_CLK125_EN_ | HW_CFG_REFCLK25_EN_, HW_CFG_CLK125_EN_ | HW_CFG_REFCLK25_EN_); if (ret < 0) - return ERR_PTR(ret); - } else { - dev->interface = PHY_INTERFACE_MODE_RGMII_ID; - /* The PHY driver is responsible to configure proper RGMII - * interface delays. Disable RGMII delays on MAC side. - */ + return ret; + + break; + + case PHY_INTERFACE_MODE_RGMII_ID: + /* Disable MAC-side TXC delay, PHY provides it */ ret = lan78xx_write_reg(dev, MAC_RGMII_ID, 0); if (ret < 0) - return ERR_PTR(ret); + return ret; - phydev->is_internal = false; + break; + + case PHY_INTERFACE_MODE_GMII: + /* No MAC-specific configuration required */ + break; + + default: + netdev_warn(dev->net, "Unsupported interface mode: %d\n", + dev->interface); + break; } - return phydev; + return 0; } static int lan78xx_phy_init(struct lan78xx_net *dev) @@ -2564,31 +2656,13 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) u32 mii_adv; struct phy_device *phydev; - switch (dev->chipid) { - case ID_REV_CHIP_ID_7801_: - phydev = lan7801_phy_init(dev); - if (IS_ERR(phydev)) { - netdev_err(dev->net, "lan7801: failed to init PHY: %pe\n", - phydev); - return PTR_ERR(phydev); - } - break; - - case ID_REV_CHIP_ID_7800_: - case ID_REV_CHIP_ID_7850_: - phydev = phy_find_first(dev->mdiobus); - if (!phydev) { - netdev_err(dev->net, "no PHY found\n"); - return -ENODEV; - } - phydev->is_internal = true; - dev->interface = PHY_INTERFACE_MODE_GMII; - break; + phydev = lan78xx_get_phy(dev); + if (IS_ERR(phydev)) + return PTR_ERR(phydev); - default: - netdev_err(dev->net, "Unknown CHIP ID found\n"); - return -ENODEV; - } + ret = lan78xx_mac_prepare_for_phy(dev); + if (ret < 0) + goto free_phy; /* if phyirq is not set, use polling mode in phylib */ if (dev->domain_data.phyirq > 0) @@ -2662,6 +2736,14 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) dev->fc_autoneg = phydev->autoneg; return 0; + +free_phy: + if (phy_is_pseudo_fixed_link(phydev)) { + fixed_phy_unregister(phydev); + phy_device_free(phydev); + } + + return ret; } static int lan78xx_set_rx_max_frame_length(struct lan78xx_net *dev, int size) -- 2.51.0 From 8ba1f33c55d213ee0da1b972b481407cfddeda4a Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Mon, 5 May 2025 10:43:38 +0200 Subject: [PATCH 11/16] net: usb: lan78xx: move LED DT configuration to helper Extract the LED enable logic based on the "microchip,led-modes" property into a new helper function lan78xx_configure_leds_from_dt(). This simplifies lan78xx_phy_init() and improves modularity. No functional changes intended. Signed-off-by: Oleksij Rempel Reviewed-by: Thangaraj Samynathan Signed-off-by: David S. Miller --- drivers/net/usb/lan78xx.c | 72 +++++++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 26 deletions(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index 7f1ecc415d53..07530eef82cb 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -2649,6 +2649,49 @@ static int lan78xx_mac_prepare_for_phy(struct lan78xx_net *dev) return 0; } +/** + * lan78xx_configure_leds_from_dt() - Configure LED enables based on DT + * @dev: LAN78xx device + * @phydev: PHY device (must be valid) + * + * Reads "microchip,led-modes" property from the PHY's DT node and enables + * the corresponding number of LEDs by writing to HW_CFG. + * + * This helper preserves the original logic, enabling up to 4 LEDs. + * If the property is not present, this function does nothing. + * + * Return: 0 on success or a negative error code. + */ +static int lan78xx_configure_leds_from_dt(struct lan78xx_net *dev, + struct phy_device *phydev) +{ + struct device_node *np = phydev->mdio.dev.of_node; + u32 reg; + int len, ret; + + if (!np) + return 0; + + len = of_property_count_elems_of_size(np, "microchip,led-modes", + sizeof(u32)); + if (len < 0) + return 0; + + ret = lan78xx_read_reg(dev, HW_CFG, ®); + if (ret < 0) + return ret; + + reg &= ~(HW_CFG_LED0_EN_ | HW_CFG_LED1_EN_ | + HW_CFG_LED2_EN_ | HW_CFG_LED3_EN_); + + reg |= (len > 0) * HW_CFG_LED0_EN_ | + (len > 1) * HW_CFG_LED1_EN_ | + (len > 2) * HW_CFG_LED2_EN_ | + (len > 3) * HW_CFG_LED3_EN_; + + return lan78xx_write_reg(dev, HW_CFG, reg); +} + static int lan78xx_phy_init(struct lan78xx_net *dev) { __ETHTOOL_DECLARE_LINK_MODE_MASK(fc) = { 0, }; @@ -2704,32 +2747,9 @@ static int lan78xx_phy_init(struct lan78xx_net *dev) phy_support_eee(phydev); - if (phydev->mdio.dev.of_node) { - u32 reg; - int len; - - len = of_property_count_elems_of_size(phydev->mdio.dev.of_node, - "microchip,led-modes", - sizeof(u32)); - if (len >= 0) { - /* Ensure the appropriate LEDs are enabled */ - ret = lan78xx_read_reg(dev, HW_CFG, ®); - if (ret < 0) - return ret; - - reg &= ~(HW_CFG_LED0_EN_ | - HW_CFG_LED1_EN_ | - HW_CFG_LED2_EN_ | - HW_CFG_LED3_EN_); - reg |= (len > 0) * HW_CFG_LED0_EN_ | - (len > 1) * HW_CFG_LED1_EN_ | - (len > 2) * HW_CFG_LED2_EN_ | - (len > 3) * HW_CFG_LED3_EN_; - ret = lan78xx_write_reg(dev, HW_CFG, reg); - if (ret < 0) - return ret; - } - } + ret = lan78xx_configure_leds_from_dt(dev, phydev); + if (ret) + goto free_phy; genphy_config_aneg(phydev); -- 2.51.0 From f485849a381f829234143e044663f35330d0b71a Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Mon, 5 May 2025 10:43:39 +0200 Subject: [PATCH 12/16] net: usb: lan78xx: Extract PHY interrupt acknowledgment to helper Move the PHY interrupt acknowledgment logic from lan78xx_link_reset() to a new helper function lan78xx_phy_int_ack(). This simplifies the code and prepares for reusing the acknowledgment logic independently from the full link reset process, such as when using phylink. No functional change intended. Signed-off-by: Oleksij Rempel Reviewed-by: Thangaraj Samynathan Signed-off-by: David S. Miller --- drivers/net/usb/lan78xx.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index 07530eef82cb..de2b429e906e 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -1636,6 +1636,20 @@ exit_unlock: return ret; } +/** + * lan78xx_phy_int_ack - Acknowledge PHY interrupt + * @dev: pointer to the LAN78xx device structure + * + * This function acknowledges the PHY interrupt by setting the + * INT_STS_PHY_INT_ bit in the interrupt status register (INT_STS). + * + * Return: 0 on success or a negative error code on failure. + */ +static int lan78xx_phy_int_ack(struct lan78xx_net *dev) +{ + return lan78xx_write_reg(dev, INT_STS, INT_STS_PHY_INT_); +} + static int lan78xx_link_reset(struct lan78xx_net *dev) { struct phy_device *phydev = dev->net->phydev; @@ -1644,7 +1658,7 @@ static int lan78xx_link_reset(struct lan78xx_net *dev) u32 buf; /* clear LAN78xx interrupt status */ - ret = lan78xx_write_reg(dev, INT_STS, INT_STS_PHY_INT_); + ret = lan78xx_phy_int_ack(dev); if (unlikely(ret < 0)) return ret; -- 2.51.0 From d746e0740b2805abbd7b56f1dd34a6b9a0f5728f Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Mon, 5 May 2025 10:43:40 +0200 Subject: [PATCH 13/16] net: usb: lan78xx: Refactor USB link power configuration into helper Move the USB link power configuration logic from lan78xx_link_reset() to a new helper function lan78xx_configure_usb(). This simplifies the main link reset path and isolates USB-specific logic. The new function handles U1/U2 enablement based on Ethernet link speed, but only for SuperSpeed-capable devices (LAN7800 and LAN7801). LAN7850, a High-Speed-only device, is explicitly excluded. A warning is logged if SuperSpeed is reported unexpectedly for LAN7850. Add a forward declaration for lan78xx_configure_usb() as preparation for the upcoming phylink conversion, where it will also be used from the mac_link_up() callback. Open questions remain: - Why is the 1000 Mbps configuration split into two steps (U2 disable, then U1 enable), unlike the single-step config used for 10/100 Mbps? - U1/U2 behavior appears to depend on proper EEPROM configuration. There are known devices in the field without EEPROM. Should the driver enforce safe defaults in such cases? Due to lack of USB subsystem expertise, no changes were made to this logic beyond structural refactoring. Signed-off-by: Oleksij Rempel Signed-off-by: David S. Miller --- drivers/net/usb/lan78xx.c | 90 +++++++++++++++++++++++++-------------- 1 file changed, 59 insertions(+), 31 deletions(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index de2b429e906e..bff53324c70a 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -1650,12 +1650,13 @@ static int lan78xx_phy_int_ack(struct lan78xx_net *dev) return lan78xx_write_reg(dev, INT_STS, INT_STS_PHY_INT_); } +static int lan78xx_configure_usb(struct lan78xx_net *dev, int speed); + static int lan78xx_link_reset(struct lan78xx_net *dev) { struct phy_device *phydev = dev->net->phydev; struct ethtool_link_ksettings ecmd; int ladv, radv, ret, link; - u32 buf; /* clear LAN78xx interrupt status */ ret = lan78xx_phy_int_ack(dev); @@ -1681,36 +1682,9 @@ static int lan78xx_link_reset(struct lan78xx_net *dev) phy_ethtool_ksettings_get(phydev, &ecmd); - if (dev->udev->speed == USB_SPEED_SUPER) { - if (ecmd.base.speed == 1000) { - /* disable U2 */ - ret = lan78xx_read_reg(dev, USB_CFG1, &buf); - if (ret < 0) - return ret; - buf &= ~USB_CFG1_DEV_U2_INIT_EN_; - ret = lan78xx_write_reg(dev, USB_CFG1, buf); - if (ret < 0) - return ret; - /* enable U1 */ - ret = lan78xx_read_reg(dev, USB_CFG1, &buf); - if (ret < 0) - return ret; - buf |= USB_CFG1_DEV_U1_INIT_EN_; - ret = lan78xx_write_reg(dev, USB_CFG1, buf); - if (ret < 0) - return ret; - } else { - /* enable U1 & U2 */ - ret = lan78xx_read_reg(dev, USB_CFG1, &buf); - if (ret < 0) - return ret; - buf |= USB_CFG1_DEV_U2_INIT_EN_; - buf |= USB_CFG1_DEV_U1_INIT_EN_; - ret = lan78xx_write_reg(dev, USB_CFG1, buf); - if (ret < 0) - return ret; - } - } + ret = lan78xx_configure_usb(dev, ecmd.base.speed); + if (ret < 0) + return ret; ladv = phy_read(phydev, MII_ADVERTISE); if (ladv < 0) @@ -2522,6 +2496,60 @@ static void lan78xx_remove_irq_domain(struct lan78xx_net *dev) dev->domain_data.irqdomain = NULL; } +/** + * lan78xx_configure_usb - Configure USB link power settings + * @dev: pointer to the LAN78xx device structure + * @speed: negotiated Ethernet link speed (in Mbps) + * + * This function configures U1/U2 link power management for SuperSpeed + * USB devices based on the current Ethernet link speed. It uses the + * USB_CFG1 register to enable or disable U1 and U2 low-power states. + * + * Note: Only LAN7800 and LAN7801 support SuperSpeed (USB 3.x). + * LAN7850 is a High-Speed-only (USB 2.0) device and is skipped. + * + * Return: 0 on success or a negative error code on failure. + */ +static int lan78xx_configure_usb(struct lan78xx_net *dev, int speed) +{ + u32 mask, val; + int ret; + + /* Only configure USB settings for SuperSpeed devices */ + if (dev->udev->speed != USB_SPEED_SUPER) + return 0; + + /* LAN7850 does not support USB 3.x */ + if (dev->chipid == ID_REV_CHIP_ID_7850_) { + netdev_warn_once(dev->net, "Unexpected SuperSpeed for LAN7850 (USB 2.0 only)\n"); + return 0; + } + + switch (speed) { + case SPEED_1000: + /* Disable U2, enable U1 */ + ret = lan78xx_update_reg(dev, USB_CFG1, + USB_CFG1_DEV_U2_INIT_EN_, 0); + if (ret < 0) + return ret; + + return lan78xx_update_reg(dev, USB_CFG1, + USB_CFG1_DEV_U1_INIT_EN_, + USB_CFG1_DEV_U1_INIT_EN_); + + case SPEED_100: + case SPEED_10: + /* Enable both U1 and U2 */ + mask = USB_CFG1_DEV_U1_INIT_EN_ | USB_CFG1_DEV_U2_INIT_EN_; + val = mask; + return lan78xx_update_reg(dev, USB_CFG1, mask, val); + + default: + netdev_warn(dev->net, "Unsupported link speed: %d\n", speed); + return -EINVAL; + } +} + /** * lan78xx_register_fixed_phy() - Register a fallback fixed PHY * @dev: LAN78xx device -- 2.51.0 From ef6a29e86785fe56e4af814509617183d7d8779c Mon Sep 17 00:00:00 2001 From: Oleksij Rempel Date: Mon, 5 May 2025 10:43:41 +0200 Subject: [PATCH 14/16] net: usb: lan78xx: Extract flow control configuration to helper Move flow control register configuration from lan78xx_update_flowcontrol() into a new helper function lan78xx_configure_flowcontrol(). This separates hardware-specific programming from policy logic and simplifies the upcoming phylink integration. The values used in this initial version of lan78xx_configure_flowcontrol() are taken over as-is from the original implementation to avoid functional changes. While they may not be optimal for all USB and link speed combinations, they are known to work reliably. Optimization of pause time and thresholds based on runtime conditions can be done in a separate follow-up patch. The forward declaration of lan78xx_configure_flowcontrol() will also be removed later during the phylink conversion. Signed-off-by: Oleksij Rempel Signed-off-by: David S. Miller --- drivers/net/usb/lan78xx.c | 105 +++++++++++++++++++++++++++++++------- 1 file changed, 87 insertions(+), 18 deletions(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index bff53324c70a..58e3589e3b89 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -1554,10 +1554,12 @@ static void lan78xx_set_multicast(struct net_device *netdev) schedule_work(&pdata->set_multicast); } +static int lan78xx_configure_flowcontrol(struct lan78xx_net *dev, + bool tx_pause, bool rx_pause); + static int lan78xx_update_flowcontrol(struct lan78xx_net *dev, u8 duplex, u16 lcladv, u16 rmtadv) { - u32 flow = 0, fct_flow = 0; u8 cap; if (dev->fc_autoneg) @@ -1565,27 +1567,13 @@ static int lan78xx_update_flowcontrol(struct lan78xx_net *dev, u8 duplex, else cap = dev->fc_request_control; - if (cap & FLOW_CTRL_TX) - flow |= (FLOW_CR_TX_FCEN_ | 0xFFFF); - - if (cap & FLOW_CTRL_RX) - flow |= FLOW_CR_RX_FCEN_; - - if (dev->udev->speed == USB_SPEED_SUPER) - fct_flow = FLOW_CTRL_THRESHOLD(FLOW_ON_SS, FLOW_OFF_SS); - else if (dev->udev->speed == USB_SPEED_HIGH) - fct_flow = FLOW_CTRL_THRESHOLD(FLOW_ON_HS, FLOW_OFF_HS); - netif_dbg(dev, link, dev->net, "rx pause %s, tx pause %s", (cap & FLOW_CTRL_RX ? "enabled" : "disabled"), (cap & FLOW_CTRL_TX ? "enabled" : "disabled")); - lan78xx_write_reg(dev, FCT_FLOW, fct_flow); - - /* threshold value should be set before enabling flow */ - lan78xx_write_reg(dev, FLOW, flow); - - return 0; + return lan78xx_configure_flowcontrol(dev, + cap & FLOW_CTRL_TX, + cap & FLOW_CTRL_RX); } static void lan78xx_rx_urb_submit_all(struct lan78xx_net *dev); @@ -2550,6 +2538,87 @@ static int lan78xx_configure_usb(struct lan78xx_net *dev, int speed) } } +/** + * lan78xx_configure_flowcontrol - Set MAC and FIFO flow control configuration + * @dev: pointer to the LAN78xx device structure + * @tx_pause: enable transmission of pause frames + * @rx_pause: enable reception of pause frames + * + * This function configures the LAN78xx flow control settings by writing + * to the FLOW and FCT_FLOW registers. The pause time is set to the + * maximum allowed value (65535 quanta). FIFO thresholds are selected + * based on USB speed. + * + * The Pause Time field is measured in units of 512-bit times (quanta): + * - At 1 Gbps: 1 quanta = 512 ns → max ~33.6 ms pause + * - At 100 Mbps: 1 quanta = 5.12 µs → max ~335 ms pause + * - At 10 Mbps: 1 quanta = 51.2 µs → max ~3.3 s pause + * + * Flow control thresholds (FCT_FLOW) are used to trigger pause/resume: + * - RXUSED is the number of bytes used in the RX FIFO + * - Flow is turned ON when RXUSED ≥ FLOW_ON threshold + * - Flow is turned OFF when RXUSED ≤ FLOW_OFF threshold + * - Both thresholds are encoded in units of 512 bytes (rounded up) + * + * Thresholds differ by USB speed because available USB bandwidth + * affects how fast packets can be drained from the RX FIFO: + * - USB 3.x (SuperSpeed): + * FLOW_ON = 9216 bytes → 18 units + * FLOW_OFF = 4096 bytes → 8 units + * - USB 2.0 (High-Speed): + * FLOW_ON = 8704 bytes → 17 units + * FLOW_OFF = 1024 bytes → 2 units + * + * Note: The FCT_FLOW register must be configured before enabling TX pause + * (i.e., before setting FLOW_CR_TX_FCEN_), as required by the hardware. + * + * Return: 0 on success or a negative error code on failure. + */ +static int lan78xx_configure_flowcontrol(struct lan78xx_net *dev, + bool tx_pause, bool rx_pause) +{ + /* Use maximum pause time: 65535 quanta (512-bit times) */ + const u32 pause_time_quanta = 65535; + u32 fct_flow = 0; + u32 flow = 0; + int ret; + + /* Prepare MAC flow control bits */ + if (tx_pause) + flow |= FLOW_CR_TX_FCEN_ | pause_time_quanta; + + if (rx_pause) + flow |= FLOW_CR_RX_FCEN_; + + /* Select RX FIFO thresholds based on USB speed + * + * FCT_FLOW layout: + * bits [6:0] FLOW_ON threshold (RXUSED ≥ ON → assert pause) + * bits [14:8] FLOW_OFF threshold (RXUSED ≤ OFF → deassert pause) + * thresholds are expressed in units of 512 bytes + */ + switch (dev->udev->speed) { + case USB_SPEED_SUPER: + fct_flow = FLOW_CTRL_THRESHOLD(FLOW_ON_SS, FLOW_OFF_SS); + break; + case USB_SPEED_HIGH: + fct_flow = FLOW_CTRL_THRESHOLD(FLOW_ON_HS, FLOW_OFF_HS); + break; + default: + netdev_warn(dev->net, "Unsupported USB speed: %d\n", + dev->udev->speed); + return -EINVAL; + } + + /* Step 1: Write FIFO thresholds before enabling pause frames */ + ret = lan78xx_write_reg(dev, FCT_FLOW, fct_flow); + if (ret < 0) + return ret; + + /* Step 2: Enable MAC pause functionality */ + return lan78xx_write_reg(dev, FLOW, flow); +} + /** * lan78xx_register_fixed_phy() - Register a fallback fixed PHY * @dev: LAN78xx device -- 2.51.0 From a512be0ecb147e25ec0492335953ae8ad8e28fcb Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 5 May 2025 09:52:05 -0700 Subject: [PATCH 15/16] tools: ynl-gen: rename basic presence from 'bit' to 'present' Internal change to the code gen. Rename how we indicate a type has a single bit presence from using a 'bit' string to 'present'. This is a noop in terms of generated code but will make next breaking change easier. Reviewed-by: David Wei Link: https://patch.msgid.link/20250505165208.248049-2-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/net/ynl/pyynl/ynl_gen_c.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index acba03bbe67d..19d8a46895f6 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -142,13 +142,13 @@ class Type(SpecAttr): return self.is_recursive() and not ri.op def presence_type(self): - return 'bit' + return 'present' def presence_member(self, space, type_filter): if self.presence_type() != type_filter: return - if self.presence_type() == 'bit': + if self.presence_type() == 'present': pfx = '__' if space == 'user' else '' return f"{pfx}u32 {self.c_name}:1;" @@ -217,7 +217,7 @@ class Type(SpecAttr): cw.p(f'[{self.enum_name}] = {"{"} .name = "{self.name}", {typol}{"}"},') def _attr_put_line(self, ri, var, line): - if self.presence_type() == 'bit': + if self.presence_type() == 'present': ri.cw.p(f"if ({var}->_present.{self.c_name})") elif self.presence_type() == 'len': ri.cw.p(f"if ({var}->_present.{self.c_name}_len)") @@ -250,7 +250,7 @@ class Type(SpecAttr): if not self.is_multi_val(): ri.cw.p("if (ynl_attr_validate(yarg, attr))") ri.cw.p("return YNL_PARSE_CB_ERROR;") - if self.presence_type() == 'bit': + if self.presence_type() == 'present': ri.cw.p(f"{var}->_present.{self.c_name} = 1;") if init_lines: @@ -281,7 +281,7 @@ class Type(SpecAttr): presence = f"{var}->{'.'.join(ref[:i] + [''])}_present.{ref[i]}" # Every layer below last is a nest, so we know it uses bit presence # last layer is "self" and may be a complex type - if i == len(ref) - 1 and self.presence_type() != 'bit': + if i == len(ref) - 1 and self.presence_type() != 'present': continue code.append(presence + ' = 1;') ref_path = '.'.join(ref[:-1]) @@ -2188,7 +2188,7 @@ def _print_type(ri, direction, struct): meta_started = False for _, attr in struct.member_list(): - for type_filter in ['len', 'bit']: + for type_filter in ['len', 'present']: line = attr.presence_member(ri.ku_space, type_filter) if line: if not meta_started: -- 2.51.0 From b8ae9f70aaf134dc57f227fd1730b64ce89e109d Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 5 May 2025 09:52:06 -0700 Subject: [PATCH 16/16] tools: ynl-gen: split presence metadata Each YNL struct contains the data and a sub-struct indicating which fields are valid. Something like: struct family_op_req { struct { u32 a:1; u32 b:1; u32 bin_len; } _present; u32 a; u64 b; const unsigned char *bin; }; Note that the bin object 'bin' has a length stored, and that length has a _len suffix added to the field name. This breaks if there is a explicit field called bin_len, which is the case for some TC actions. Move the length fields out of the _present struct, create a new struct called _len: struct family_op_req { struct { u32 a:1; u32 b:1; } _present; struct { u32 bin; } _len; u32 a; u64 b; const unsigned char *bin; }; This should prevent name collisions and help with the packing of the struct. Unfortunately this is a breaking change, but hopefully the migration isn't too painful. Link: https://patch.msgid.link/20250505165208.248049-3-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/net/ynl/pyynl/ynl_gen_c.py | 46 ++++++++++++++++---------------- tools/net/ynl/samples/devlink.c | 2 +- tools/net/ynl/samples/rt-addr.c | 4 +-- tools/net/ynl/samples/rt-route.c | 4 +-- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/tools/net/ynl/pyynl/ynl_gen_c.py b/tools/net/ynl/pyynl/ynl_gen_c.py index 19d8a46895f6..e761a20cc53f 100755 --- a/tools/net/ynl/pyynl/ynl_gen_c.py +++ b/tools/net/ynl/pyynl/ynl_gen_c.py @@ -154,7 +154,7 @@ class Type(SpecAttr): if self.presence_type() == 'len': pfx = '__' if space == 'user' else '' - return f"{pfx}u32 {self.c_name}_len;" + return f"{pfx}u32 {self.c_name};" def _complex_member_type(self, ri): return None @@ -217,10 +217,9 @@ class Type(SpecAttr): cw.p(f'[{self.enum_name}] = {"{"} .name = "{self.name}", {typol}{"}"},') def _attr_put_line(self, ri, var, line): - if self.presence_type() == 'present': - ri.cw.p(f"if ({var}->_present.{self.c_name})") - elif self.presence_type() == 'len': - ri.cw.p(f"if ({var}->_present.{self.c_name}_len)") + presence = self.presence_type() + if presence in {'present', 'len'}: + ri.cw.p(f"if ({var}->_{presence}.{self.c_name})") ri.cw.p(f"{line};") def _attr_put_simple(self, ri, var, put_type): @@ -282,6 +281,7 @@ class Type(SpecAttr): # Every layer below last is a nest, so we know it uses bit presence # last layer is "self" and may be a complex type if i == len(ref) - 1 and self.presence_type() != 'present': + presence = f"{var}->{'.'.join(ref[:i] + [''])}_{self.presence_type()}.{ref[i]}" continue code.append(presence + ' = 1;') ref_path = '.'.join(ref[:-1]) @@ -501,7 +501,7 @@ class TypeString(Type): self._attr_put_simple(ri, var, 'str') def _attr_get(self, ri, var): - len_mem = var + '->_present.' + self.c_name + '_len' + len_mem = var + '->_len.' + self.c_name return [f"{len_mem} = len;", f"{var}->{self.c_name} = malloc(len + 1);", f"memcpy({var}->{self.c_name}, ynl_attr_get_str(attr), len);", @@ -510,10 +510,10 @@ class TypeString(Type): ['unsigned int len;'] def _setter_lines(self, ri, member, presence): - return [f"{presence}_len = strlen({self.c_name});", - f"{member} = malloc({presence}_len + 1);", - f'memcpy({member}, {self.c_name}, {presence}_len);', - f'{member}[{presence}_len] = 0;'] + return [f"{presence} = strlen({self.c_name});", + f"{member} = malloc({presence} + 1);", + f'memcpy({member}, {self.c_name}, {presence});', + f'{member}[{presence}] = 0;'] class TypeBinary(Type): @@ -552,10 +552,10 @@ class TypeBinary(Type): def attr_put(self, ri, var): self._attr_put_line(ri, var, f"ynl_attr_put(nlh, {self.enum_name}, " + - f"{var}->{self.c_name}, {var}->_present.{self.c_name}_len)") + f"{var}->{self.c_name}, {var}->_len.{self.c_name})") def _attr_get(self, ri, var): - len_mem = var + '->_present.' + self.c_name + '_len' + len_mem = var + '->_len.' + self.c_name return [f"{len_mem} = len;", f"{var}->{self.c_name} = malloc(len);", f"memcpy({var}->{self.c_name}, ynl_attr_data(attr), len);"], \ @@ -563,9 +563,9 @@ class TypeBinary(Type): ['unsigned int len;'] def _setter_lines(self, ri, member, presence): - return [f"{presence}_len = len;", - f"{member} = malloc({presence}_len);", - f'memcpy({member}, {self.c_name}, {presence}_len);'] + return [f"{presence} = len;", + f"{member} = malloc({presence});", + f'memcpy({member}, {self.c_name}, {presence});'] class TypeBitfield32(Type): @@ -721,7 +721,7 @@ class TypeMultiAttr(Type): def _setter_lines(self, ri, member, presence): # For multi-attr we have a count, not presence, hack up the presence - presence = presence[:-(len('_present.') + len(self.c_name))] + "n_" + self.c_name + presence = presence[:-(len('_count.') + len(self.c_name))] + "n_" + self.c_name return [f"{member} = {self.c_name};", f"{presence} = n_{self.c_name};"] @@ -784,7 +784,7 @@ class TypeArrayNest(Type): def _setter_lines(self, ri, member, presence): # For multi-attr we have a count, not presence, hack up the presence - presence = presence[:-(len('_present.') + len(self.c_name))] + "n_" + self.c_name + presence = presence[:-(len('_count.') + len(self.c_name))] + "n_" + self.c_name return [f"{member} = {self.c_name};", f"{presence} = n_{self.c_name};"] @@ -2186,18 +2186,18 @@ def _print_type(ri, direction, struct): ri.cw.p(ri.fixed_hdr + ' _hdr;') ri.cw.nl() - meta_started = False - for _, attr in struct.member_list(): - for type_filter in ['len', 'present']: + for type_filter in ['present', 'len']: + meta_started = False + for _, attr in struct.member_list(): line = attr.presence_member(ri.ku_space, type_filter) if line: if not meta_started: ri.cw.block_start(line=f"struct") meta_started = True ri.cw.p(line) - if meta_started: - ri.cw.block_end(line='_present;') - ri.cw.nl() + if meta_started: + ri.cw.block_end(line=f'_{type_filter};') + ri.cw.nl() for arg in struct.inherited: ri.cw.p(f"__u32 {arg};") diff --git a/tools/net/ynl/samples/devlink.c b/tools/net/ynl/samples/devlink.c index d2611d7ebab4..3d32a6335044 100644 --- a/tools/net/ynl/samples/devlink.c +++ b/tools/net/ynl/samples/devlink.c @@ -34,7 +34,7 @@ int main(int argc, char **argv) if (!info_rsp) goto err_free_devs; - if (info_rsp->_present.info_driver_name_len) + if (info_rsp->_len.info_driver_name) printf(" driver: %s\n", info_rsp->info_driver_name); if (info_rsp->n_info_version_running) printf(" running fw:\n"); diff --git a/tools/net/ynl/samples/rt-addr.c b/tools/net/ynl/samples/rt-addr.c index 0f4851b4ec57..2edde5c36b18 100644 --- a/tools/net/ynl/samples/rt-addr.c +++ b/tools/net/ynl/samples/rt-addr.c @@ -20,7 +20,7 @@ static void rt_addr_print(struct rt_addr_getaddr_rsp *a) if (name) printf("%16s: ", name); - switch (a->_present.address_len) { + switch (a->_len.address) { case 4: addr = inet_ntop(AF_INET, a->address, addr_str, sizeof(addr_str)); @@ -36,7 +36,7 @@ static void rt_addr_print(struct rt_addr_getaddr_rsp *a) if (addr) printf("%s", addr); else - printf("[%d]", a->_present.address_len); + printf("[%d]", a->_len.address); printf("\n"); } diff --git a/tools/net/ynl/samples/rt-route.c b/tools/net/ynl/samples/rt-route.c index 9d9c868f8873..7427104a96df 100644 --- a/tools/net/ynl/samples/rt-route.c +++ b/tools/net/ynl/samples/rt-route.c @@ -26,13 +26,13 @@ static void rt_route_print(struct rt_route_getroute_rsp *r) printf("oif: %-16s ", name); } - if (r->_present.dst_len) { + if (r->_len.dst) { route = inet_ntop(r->_hdr.rtm_family, r->dst, route_str, sizeof(route_str)); printf("dst: %s/%d", route, r->_hdr.rtm_dst_len); } - if (r->_present.gateway_len) { + if (r->_len.gateway) { route = inet_ntop(r->_hdr.rtm_family, r->gateway, route_str, sizeof(route_str)); printf("gateway: %s ", route); -- 2.51.0