Skip to content

Commit

Permalink
net: dsa: qca8k: assign ds->user_mii_bus only for the non-OF case
Browse files Browse the repository at this point in the history
To simplify reasoning about why the DSA framework provides the
ds->user_mii_bus functionality, drivers should only use it if they
need to. The qca8k driver appears to also use it simply as storage
for a pointer, which is not a good enough reason to make the core
much more difficult to follow.

ds->user_mii_bus is useful for only 2 cases:

1. The driver probes on platform_data (no OF)
2. The driver probes on OF, but there is no OF node for the MDIO bus.

It is unclear if case (1) is supported with qca8k. It might not be:
the driver might crash when of_device_get_match_data() returns NULL
and then it dereferences priv->info without NULL checking.

Anyway, let us limit the ds->user_mii_bus usage only to the above cases,
and not assign it when an OF node is present.

The bus->phy_mask assignment follows along with the movement, because
__of_mdiobus_register() overwrites this bus field anyway. The value set
by the driver only matters for the non-OF code path.

Signed-off-by: Vladimir Oltean <[email protected]>
Reviewed-by: Alvin Šipraga <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
Reviewed-by: Luiz Angelo Daros de Luca <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
vladimiroltean authored and davem330 committed Jan 5, 2024
1 parent e66bf63 commit 525366b
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 4 deletions.
5 changes: 3 additions & 2 deletions drivers/net/dsa/qca/qca8k-8xxx.c
Original file line number Diff line number Diff line change
Expand Up @@ -961,12 +961,11 @@ qca8k_mdio_register(struct qca8k_priv *priv)
goto out_put_node;
}

priv->internal_mdio_bus = bus;
bus->priv = (void *)priv;
snprintf(bus->id, MII_BUS_ID_SIZE, "qca8k-%d.%d",
ds->dst->index, ds->index);
bus->parent = ds->dev;
bus->phy_mask = ~ds->phys_mii_mask;
ds->user_mii_bus = bus;

/* Check if the devicetree declare the port:phy mapping */
if (mdio) {
Expand All @@ -980,6 +979,8 @@ qca8k_mdio_register(struct qca8k_priv *priv)
/* If a mapping can't be found the legacy mapping is used,
* using the qca8k_port_to_phy function
*/
ds->user_mii_bus = bus;
bus->phy_mask = ~ds->phys_mii_mask;
bus->name = "qca8k-legacy user mii";
bus->read = qca8k_legacy_mdio_read;
bus->write = qca8k_legacy_mdio_write;
Expand Down
4 changes: 2 additions & 2 deletions drivers/net/dsa/qca/qca8k-leds.c
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,6 @@ qca8k_parse_port_leds(struct qca8k_priv *priv, struct fwnode_handle *port, int p
{
struct fwnode_handle *led = NULL, *leds = NULL;
struct led_init_data init_data = { };
struct dsa_switch *ds = priv->ds;
enum led_default_state state;
struct qca8k_led *port_led;
int led_num, led_index;
Expand Down Expand Up @@ -429,7 +428,8 @@ qca8k_parse_port_leds(struct qca8k_priv *priv, struct fwnode_handle *port, int p
init_data.default_label = ":port";
init_data.fwnode = led;
init_data.devname_mandatory = true;
init_data.devicename = kasprintf(GFP_KERNEL, "%s:0%d", ds->user_mii_bus->id,
init_data.devicename = kasprintf(GFP_KERNEL, "%s:0%d",
priv->internal_mdio_bus->id,
port_num);
if (!init_data.devicename)
return -ENOMEM;
Expand Down
1 change: 1 addition & 0 deletions drivers/net/dsa/qca/qca8k.h
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,7 @@ struct qca8k_priv {
struct qca8k_ports_config ports_config;
struct regmap *regmap;
struct mii_bus *bus;
struct mii_bus *internal_mdio_bus;
struct dsa_switch *ds;
struct mutex reg_mutex;
struct device *dev;
Expand Down

0 comments on commit 525366b

Please sign in to comment.