From: Heikki Krogerus <heikki.krogerus@linux•intel.com>
To: "Russell King (Oracle)" <rmk+kernel@armlinux•org.uk>
Cc: Andrew Lunn <andrew@lunn•ch>,
Heiner Kallweit <hkallweit1@gmail•com>,
Andy Shevchenko <andriy.shevchenko@linux•intel.com>,
Daniel Scally <djrscally@gmail•com>,
"David S. Miller" <davem@davemloft•net>,
Eric Dumazet <edumazet@google•com>,
Florian Fainelli <f.fainelli@gmail•com>,
Greg Kroah-Hartman <gregkh@linuxfoundation•org>,
Jakub Kicinski <kuba@kernel•org>,
linux-acpi@vger•kernel.org, netdev@vger•kernel.org,
Paolo Abeni <pabeni@redhat•com>,
"Rafael J. Wysocki" <rafael@kernel•org>,
Sakari Ailus <sakari.ailus@linux•intel.com>,
Vladimir Oltean <olteanv@gmail•com>
Subject: Re: [PATCH RFC net-next 6/7] net: dsa: mv88e6xxx: provide software node for default settings
Date: Fri, 24 Mar 2023 16:49:32 +0200 [thread overview]
Message-ID: <ZB24fDEqwx53Rthm@kuha.fi.intel.com> (raw)
In-Reply-To: <E1pex8f-00Dvo9-KT@rmk-PC.armlinux.org.uk>
Hi Russell,
On Wed, Mar 22, 2023 at 12:00:21PM +0000, Russell King (Oracle) wrote:
> +static struct fwnode_handle *mv88e6xxx_create_fixed_swnode(struct fwnode_handle *parent,
> + int speed,
> + int duplex)
> +{
> + struct property_entry fixed_link_props[3] = { };
> +
> + fixed_link_props[0] = PROPERTY_ENTRY_U32("speed", speed);
> + if (duplex == DUPLEX_FULL)
> + fixed_link_props[1] = PROPERTY_ENTRY_BOOL("full-duplex");
> +
> + return fwnode_create_named_software_node(fixed_link_props, parent,
> + "fixed-link");
> +}
> +
> +static struct fwnode_handle *mv88e6xxx_create_port_swnode(phy_interface_t mode,
> + int speed,
> + int duplex)
> +{
> + struct property_entry port_props[2] = {};
> + struct fwnode_handle *fixed_link_fwnode;
> + struct fwnode_handle *new_port_fwnode;
> +
> + 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;
> +
> + fixed_link_fwnode = mv88e6xxx_create_fixed_swnode(new_port_fwnode,
> + speed, duplex);
> + if (IS_ERR(fixed_link_fwnode)) {
> + fwnode_remove_software_node(new_port_fwnode);
> + return fixed_link_fwnode;
> + }
> +
> + return new_port_fwnode;
> +}
That new fwnode_create_named_software_node() function looks like a
conflict waiting to happen - if a driver adds a node to the root level
(does not have to be root level), all the tests will pass because
there is only a single device, but when a user later tries the driver
with two devices, it fails, because the node already exist. But you
don't need that function at all.
Here's an example how you can add the nodes with the already existing
APIs. To keep this example simple, I'm expecting that you have members
for the software nodes to the struct mv88e6xxx_chip:
struct mv88e6xxx_chip {
...
/* swnodes */
struct software_node port_swnode;
struct software_node fixed_link_swnode;
};
Of course, you don't have to add those members if you don't want to.
Then you just need to allocate the nodes separately, but that should
not be a problem. In any case, something like this:
static struct fwnode_handle *mv88e6xxx_create_port_swnode(struct mv88e6xxx_chip *chip,
phy_interface_t mode,
int speed,
int duplex)
{
struct property_entry fixed_link_props[3] = { };
struct property_entry port_props[2] = { };
int ret;
/*
* First register the port node.
*/
port_props[0] = PROPERTY_ENTRY_STRING("phy-mode", phy_modes(mode));
chip->port_swnode.properties = property_entries_dup(port_props);
if (IS_ERR(chip->port_swnode.properties))
return ERR_CAST(chip->port_swnode.properties);
ret = software_node_register(&chip->port_swnode);
if (ret) {
kfree(chip->port_swnode.properties);
return ERR_PTR(ret);
}
/*
* Then the second node, child of the port node.
*/
fixed_link_props[0] = PROPERTY_ENTRY_U32("speed", speed);
if (duplex == DUPLEX_FULL)
fixed_link_props[1] = PROPERTY_ENTRY_BOOL("full-duplex");
chip->fixed_link_swnode.name = "fixed-link";
chip->fixed_link_swnode.parent = &chip->port_swnode;
chip->fixed_link_swnode.properties = property_entries_dup(fixed_link_props);
if (IS_ERR(chip->port_swnode.properties)) {
software_node_unregister(&chip->port_swnode);
kfree(chip->port_swnode.properties);
return ERR_CAST(chip->fixed_link_swnode.properties);
}
ret = software_node_register(&chip->fixed_link_swnode);
if (ret) {
software_node_unregister(&chip->port_swnode);
kfree(chip->port_swnode.properties);
kfree(chip->fixed_link_swnode.properties);
return ERR_PTR(ret);
}
/*
* Finally, return the port fwnode.
*/
return software_node_fwnode(&chip->port_swnode);
}
thanks,
--
heikki
next prev parent reply other threads:[~2023-03-24 14:50 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-22 11:59 [PATCH RFC net-next 0/7] Another attempt at moving mv88e6xxx forward Russell King (Oracle)
2023-03-22 11:59 ` [PATCH RFC net-next 1/7] software node: allow named software node to be created Russell King
2023-03-23 13:59 ` Andy Shevchenko
2023-03-23 14:29 ` Russell King (Oracle)
2023-03-23 14:39 ` Andy Shevchenko
2023-03-22 12:00 ` [PATCH RFC net-next 2/7] net: phylink: provide phylink_find_max_speed() Russell King (Oracle)
2023-03-22 18:44 ` Andrew Lunn
2023-03-22 12:00 ` [PATCH RFC net-next 3/7] net: dsa: use fwnode_get_phy_mode() to get phy interface mode Russell King (Oracle)
2023-03-22 18:42 ` Andrew Lunn
2023-03-23 14:03 ` Andy Shevchenko
2023-03-23 14:31 ` Russell King (Oracle)
2023-03-23 14:38 ` Andy Shevchenko
2023-03-23 14:49 ` Russell King (Oracle)
2023-03-23 15:00 ` Andy Shevchenko
2023-03-23 15:23 ` Russell King (Oracle)
2023-03-23 15:33 ` Andy Shevchenko
2023-03-23 16:29 ` Russell King (Oracle)
2023-03-23 16:18 ` Russell King (Oracle)
2023-03-23 16:34 ` Andy Shevchenko
2023-03-23 16:39 ` Andy Shevchenko
2023-03-23 17:06 ` Russell King (Oracle)
2023-03-23 17:28 ` Andy Shevchenko
2023-03-23 17:53 ` Russell King (Oracle)
2023-03-23 18:04 ` Andy Shevchenko
2023-03-23 20:46 ` Russell King (Oracle)
2023-03-22 12:00 ` [PATCH RFC net-next 4/7] net: dsa: add ability for switch driver to provide a swnode Russell King (Oracle)
2023-03-22 12:00 ` [PATCH RFC net-next 5/7] net: dsa: avoid DT validation for drivers which provide default config Russell King (Oracle)
2023-03-22 18:51 ` Andrew Lunn
2023-03-22 20:09 ` Russell King (Oracle)
2023-03-22 20:14 ` Andrew Lunn
2023-03-22 20:20 ` Russell King (Oracle)
2023-03-22 12:00 ` [PATCH RFC net-next 6/7] net: dsa: mv88e6xxx: provide software node for default settings Russell King (Oracle)
2023-03-22 18:57 ` Andrew Lunn
2023-03-22 20:13 ` Russell King (Oracle)
2023-03-22 20:17 ` Andrew Lunn
2023-03-22 20:22 ` Russell King (Oracle)
2023-03-22 21:40 ` Andrew Lunn
2023-03-23 8:41 ` Russell King (Oracle)
2023-03-23 18:17 ` Andrew Lunn
2023-03-23 18:25 ` Russell King (Oracle)
2023-03-23 18:34 ` Andrew Lunn
2023-03-24 14:49 ` Heikki Krogerus [this message]
2023-03-24 17:04 ` Russell King (Oracle)
2023-03-27 10:28 ` Heikki Krogerus
2023-03-27 10:55 ` Russell King (Oracle)
2023-03-27 14:13 ` Heikki Krogerus
2023-03-27 14:32 ` Russell King (Oracle)
2023-03-27 15:45 ` Russell King (Oracle)
2023-03-28 12:09 ` Heikki Krogerus
2023-03-28 13:23 ` Russell King (Oracle)
2023-03-29 14:07 ` Heikki Krogerus
2023-03-29 14:33 ` Russell King (Oracle)
2023-03-30 13:54 ` Heikki Krogerus
2023-04-03 13:02 ` Russell King (Oracle)
2023-04-05 17:51 ` Greg Kroah-Hartman
2023-03-22 12:00 ` [PATCH RFC net-next 7/7] net: dsa: mv88e6xxx: remove handling for DSA and CPU ports Russell King (Oracle)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZB24fDEqwx53Rthm@kuha.fi.intel.com \
--to=heikki.krogerus@linux$(echo .)intel.com \
--cc=andrew@lunn$(echo .)ch \
--cc=andriy.shevchenko@linux$(echo .)intel.com \
--cc=davem@davemloft$(echo .)net \
--cc=djrscally@gmail$(echo .)com \
--cc=edumazet@google$(echo .)com \
--cc=f.fainelli@gmail$(echo .)com \
--cc=gregkh@linuxfoundation$(echo .)org \
--cc=hkallweit1@gmail$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=linux-acpi@vger$(echo .)kernel.org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=olteanv@gmail$(echo .)com \
--cc=pabeni@redhat$(echo .)com \
--cc=rafael@kernel$(echo .)org \
--cc=rmk+kernel@armlinux$(echo .)org.uk \
--cc=sakari.ailus@linux$(echo .)intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox