| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Fri, 15 Jul 2022 23:11:18 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>,
Heiner Kallweit <hkallweit1@...il.com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Alvin __ipraga <alsi@...g-olufsen.dk>,
Claudiu Manoil <claudiu.manoil@....com>,
Daniel Scally <djrscally@...il.com>,
"David S. Miller" <davem@...emloft.net>,
DENG Qingfang <dqfext@...il.com>,
Eric Dumazet <edumazet@...gle.com>,
Florian Fainelli <f.fainelli@...il.com>,
George McCollister <george.mccollister@...il.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Hauke Mehrtens <hauke@...ke-m.de>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Jakub Kicinski <kuba@...nel.org>,
Kurt Kanzenbach <kurt@...utronix.de>,
Landen Chao <Landen.Chao@...iatek.com>,
Linus Walleij <linus.walleij@...aro.org>,
linux-acpi@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org,
Matthias Brugger <matthias.bgg@...il.com>,
netdev@...r.kernel.org, Paolo Abeni <pabeni@...hat.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Sean Wang <sean.wang@...iatek.com>,
UNGLinuxDriver@...rochip.com,
Vivien Didelot <vivien.didelot@...il.com>,
Vladimir Oltean <olteanv@...il.com>,
Woojung Huh <woojung.huh@...rochip.com>,
Marek Behún <kabel@...nel.org>
Subject: Re: [PATCH net-next 5/6] net: dsa: use swnode fixed-link if using
default params
On Fri, Jul 15, 2022 at 05:01:48PM +0100, Russell King (Oracle) wrote:
> Create and use a swnode fixed-link specification for phylink if no
> parameters are given in DT for a fixed-link. This allows phylink to
> be used for "default" cases for DSA and CPU ports. Enable the use
> of phylink in all cases for DSA and CPU ports.
> Co-developed by Vladimir Oltean and myself.
Why not to use
Co-developed-by: Vladimir Oltean <vladimir.oltean@....com>
?
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> Reviewed-by: Marek Behún <kabel@...nel.org>
> Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
...
> +static struct {
> + unsigned long mask;
> + int speed;
> + int duplex;
> +} phylink_caps_params[] = {
> + { MAC_400000FD, SPEED_400000, DUPLEX_FULL },
> + { MAC_200000FD, SPEED_200000, DUPLEX_FULL },
> + { MAC_100000FD, SPEED_100000, DUPLEX_FULL },
> + { MAC_56000FD, SPEED_56000, DUPLEX_FULL },
> + { MAC_50000FD, SPEED_50000, DUPLEX_FULL },
> + { MAC_40000FD, SPEED_40000, DUPLEX_FULL },
> + { MAC_25000FD, SPEED_25000, DUPLEX_FULL },
> + { MAC_20000FD, SPEED_20000, DUPLEX_FULL },
> + { MAC_10000FD, SPEED_10000, DUPLEX_FULL },
> + { MAC_5000FD, SPEED_5000, DUPLEX_FULL },
> + { MAC_2500FD, SPEED_2500, DUPLEX_FULL },
> + { MAC_1000FD, SPEED_1000, DUPLEX_FULL },
> + { MAC_100FD, SPEED_100, DUPLEX_FULL },
> + { MAC_10FD, SPEED_10, DUPLEX_FULL },
> + { MAC_1000HD, SPEED_1000, DUPLEX_HALF },
> + { MAC_100HD, SPEED_100, DUPLEX_HALF },
> + { MAC_10HD, SPEED_10, DUPLEX_HALF },
> +};
> +
> +static int dsa_port_find_max_speed(unsigned long caps, int *speed, int *duplex)
> +{
> + int i;
> +
> + *speed = SPEED_UNKNOWN;
> + *duplex = DUPLEX_UNKNOWN;
> +
> + for (i = 0; i < ARRAY_SIZE(phylink_caps_params); i++) {
> + if (caps & phylink_caps_params[i].mask) {
> + *speed = phylink_caps_params[i].speed;
> + *duplex = phylink_caps_params[i].duplex;
> + break;
With the below check it's way too protective programming.
return 0;
> + }
> + }
> +
> + return *speed == SPEED_UNKNOWN ? -EINVAL : 0;
return -EINVAL;
> +}
...
> +static struct fwnode_handle *dsa_port_get_fwnode(struct dsa_port *dp,
> + phy_interface_t mode)
> +{
> + struct property_entry fixed_link_props[3] = { };
> + struct property_entry port_props[3] = {};
A bit of consistency in the assignments?
Also it seems you are using up to 2 for the first one and only 1 in the second
one. IIUC it requires a terminator entry, so it means 3 and 2. Do we really
need 3 in the second case?
> + struct fwnode_handle *fixed_link_fwnode;
> + struct fwnode_handle *new_port_fwnode;
> + struct device_node *dn = dp->dn;
> + struct device_node *phy_node;
> + int err, speed, duplex;
> + unsigned long caps;
> +
> + phy_node = of_parse_phandle(dn, "phy-handle", 0);
fwnode in the name, why not to use fwnode APIs?
fwnode_find_reference();
> + of_node_put(phy_node);
> + if (phy_node || of_phy_is_fixed_link(dn))
> + /* Nothing broken, nothing to fix.
> + * TODO: As discussed with Russell, maybe phylink could provide
> + * a more comprehensive helper to determine what constitutes a
> + * valid fwnode binding than this guerilla kludge.
> + */
> + return of_fwnode_handle(dn);
> +
> + if (mode == PHY_INTERFACE_MODE_NA)
> + dsa_port_find_max_caps(dp, &mode, &caps);
> + else
> + caps = dp->pl_config.mac_capabilities &
> + phylink_interface_to_caps(mode);
> +
> + err = dsa_port_find_max_speed(caps, &speed, &duplex);
> + if (err)
> + return ERR_PTR(err);
> +
> + fixed_link_props[0] = PROPERTY_ENTRY_U32("speed", speed);
> + if (duplex == DUPLEX_FULL)
> + fixed_link_props[1] = PROPERTY_ENTRY_BOOL("full-duplex");
> +
> + port_props[0] = PROPERTY_ENTRY_STRING("phy-mode", phy_modes(mode));
> +
> + new_port_fwnode = fwnode_create_software_node(port_props, NULL);
> + if (IS_ERR(new_port_fwnode))
> + return new_port_fwnode;
> +
> + /* Node needs to be named so that phylink's call to
> + * fwnode_get_named_child_node() finds it.
> + */
> + fixed_link_fwnode = fwnode_create_named_software_node(fixed_link_props,
> + new_port_fwnode,
> + "fixed-link");
> + if (IS_ERR(fixed_link_fwnode)) {
> + fwnode_remove_software_node(new_port_fwnode);
> + return fixed_link_fwnode;
> + }
> +
> + return new_port_fwnode;
> +}
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists