| Message ID | 20220922040102.1554459-9-colin.foster@in-advantage.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | add support for the the vsc7512 internal copper phys | expand |
On Wed, 21 Sep 2022 21:00:56 -0700 Colin Foster wrote: > During development, it was believed that a wrapper for ocelot_regmap_init() > would be sufficient for the felix driver to work in non-mmio scenarios. > This was merged in during commit 242bd0c10bbd ("net: dsa: ocelot: felix: > add interface for custom regmaps") > > As the external ocelot DSA driver grew closer to an acceptable state, it > was realized that most of the parameters that were passed in from struct > resource *res were useless and ignored. This is due to the fact that the > external ocelot DSA driver utilizes dev_get_regmap(dev, resource->name). > > Instead of simply ignoring those parameters, refactor the API to only > require the name as an argument. MMIO scenarios this will reconstruct the > struct resource before calling ocelot_regmap_init(ocelot, resource). MFD > scenarios need only call dev_get_regmap(dev, name). > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> drivers/net/dsa/ocelot/felix.c:1328:14: warning: variable 'match' is used uninitialized whenever 'for' loop exits because its condition is false [-Wsometimes-uninitialized] for (i = 0; i < TARGET_MAX; i++) { ^~~~~~~~~~~~~~ drivers/net/dsa/ocelot/felix.c:1338:7: note: uninitialized use occurs here if (!match) { ^~~~~ drivers/net/dsa/ocelot/felix.c:1328:14: note: remove the condition if it is always true for (i = 0; i < TARGET_MAX; i++) { ^~~~~~~~~~~~~~ drivers/net/dsa/ocelot/felix.c:1324:30: note: initialize the variable 'match' to silence this warning const struct resource *match; ^ = NULL
On Thu, 22 Sep 2022 19:39:06 -0700 Jakub Kicinski wrote: > On Wed, 21 Sep 2022 21:00:56 -0700 Colin Foster wrote: > > During development, it was believed that a wrapper for ocelot_regmap_init() > > would be sufficient for the felix driver to work in non-mmio scenarios. > > This was merged in during commit 242bd0c10bbd ("net: dsa: ocelot: felix: > > add interface for custom regmaps") > > > > As the external ocelot DSA driver grew closer to an acceptable state, it > > was realized that most of the parameters that were passed in from struct > > resource *res were useless and ignored. This is due to the fact that the > > external ocelot DSA driver utilizes dev_get_regmap(dev, resource->name). > > > > Instead of simply ignoring those parameters, refactor the API to only > > require the name as an argument. MMIO scenarios this will reconstruct the > > struct resource before calling ocelot_regmap_init(ocelot, resource). MFD > > scenarios need only call dev_get_regmap(dev, name). Ah, and the modpost: ERROR: modpost: drivers/net/dsa/ocelot/mscc_seville: 'felix_init_regmap' exported twice. Previous export was in drivers/net/dsa/ocelot/mscc_felix.ko
On Thu, Sep 22, 2022 at 07:40:09PM -0700, Jakub Kicinski wrote: > On Thu, 22 Sep 2022 19:39:06 -0700 Jakub Kicinski wrote: > > On Wed, 21 Sep 2022 21:00:56 -0700 Colin Foster wrote: > > > During development, it was believed that a wrapper for ocelot_regmap_init() > > > would be sufficient for the felix driver to work in non-mmio scenarios. > > > This was merged in during commit 242bd0c10bbd ("net: dsa: ocelot: felix: > > > add interface for custom regmaps") > > > > > > As the external ocelot DSA driver grew closer to an acceptable state, it > > > was realized that most of the parameters that were passed in from struct > > > resource *res were useless and ignored. This is due to the fact that the > > > external ocelot DSA driver utilizes dev_get_regmap(dev, resource->name). > > > > > > Instead of simply ignoring those parameters, refactor the API to only > > > require the name as an argument. MMIO scenarios this will reconstruct the > > > struct resource before calling ocelot_regmap_init(ocelot, resource). MFD > > > scenarios need only call dev_get_regmap(dev, name). > > Ah, and the modpost: > > ERROR: modpost: drivers/net/dsa/ocelot/mscc_seville: 'felix_init_regmap' exported twice. Previous export was in drivers/net/dsa/ocelot/mscc_felix.ko Yep. Since felix.c gets compiled into both drivers I don't need to export the symbol. And there's the NULL assignment. That one I'm surprised doesn't throw a warning... Looking forward to more feedback before I send out v3 next week.
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c index a8196cdedcc5..d600a14872e3 100644 --- a/drivers/net/dsa/ocelot/felix.c +++ b/drivers/net/dsa/ocelot/felix.c @@ -1318,11 +1318,49 @@ static int felix_parse_dt(struct felix *felix, phy_interface_t *port_phy_modes) return err; } +struct regmap *felix_init_regmap(struct ocelot *ocelot, const char *name) +{ + struct felix *felix = ocelot_to_felix(ocelot); + const struct resource *match; + struct resource res; + int i; + + for (i = 0; i < TARGET_MAX; i++) { + if (!felix->info->target_io_res[i].name) + continue; + + if (!strcmp(name, felix->info->target_io_res[i].name)) { + match = &felix->info->target_io_res[i]; + break; + } + } + + if (!match) { + for (i = 0; i < ocelot->num_phys_ports; i++) { + if (!strcmp(name, felix->info->port_io_res[i].name)) { + match = &felix->info->port_io_res[i]; + break; + } + } + } + + if (!match) + return ERR_PTR(-EINVAL); + + memcpy(&res, match, sizeof(res)); + res.flags = IORESOURCE_MEM; + res.start += felix->switch_base; + res.end += felix->switch_base; + + return ocelot_regmap_init(ocelot, &res); +} +EXPORT_SYMBOL(felix_init_regmap); + static int felix_init_structs(struct felix *felix, int num_phys_ports) { struct ocelot *ocelot = &felix->ocelot; phy_interface_t *port_phy_modes; - struct resource res; + const char *name; int port, i, err; ocelot->num_phys_ports = num_phys_ports; @@ -1358,15 +1396,12 @@ static int felix_init_structs(struct felix *felix, int num_phys_ports) for (i = 0; i < TARGET_MAX; i++) { struct regmap *target; - if (!felix->info->target_io_res[i].name) - continue; + name = felix->info->target_io_res[i].name; - memcpy(&res, &felix->info->target_io_res[i], sizeof(res)); - res.flags = IORESOURCE_MEM; - res.start += felix->switch_base; - res.end += felix->switch_base; + if (!name) + continue; - target = felix->info->init_regmap(ocelot, &res); + target = felix->info->init_regmap(ocelot, name); if (IS_ERR(target)) { dev_err(ocelot->dev, "Failed to map device memory space\n"); @@ -1398,12 +1433,8 @@ static int felix_init_structs(struct felix *felix, int num_phys_ports) return -ENOMEM; } - memcpy(&res, &felix->info->port_io_res[port], sizeof(res)); - res.flags = IORESOURCE_MEM; - res.start += felix->switch_base; - res.end += felix->switch_base; - - target = felix->info->init_regmap(ocelot, &res); + name = felix->info->port_io_res[port].name; + target = felix->info->init_regmap(ocelot, name); if (IS_ERR(target)) { dev_err(ocelot->dev, "Failed to map memory space for port %d\n", diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h index f94a445c2542..e623806eb8ee 100644 --- a/drivers/net/dsa/ocelot/felix.h +++ b/drivers/net/dsa/ocelot/felix.h @@ -57,8 +57,7 @@ struct felix_info { void (*tas_guard_bands_update)(struct ocelot *ocelot, int port); void (*port_sched_speed_set)(struct ocelot *ocelot, int port, u32 speed); - struct regmap *(*init_regmap)(struct ocelot *ocelot, - struct resource *res); + struct regmap *(*init_regmap)(struct ocelot *ocelot, const char *name); }; /* Methods for initializing the hardware resources specific to a tagging @@ -97,5 +96,6 @@ struct felix { struct net_device *felix_port_to_netdev(struct ocelot *ocelot, int port); int felix_netdev_to_port(struct net_device *dev); +struct regmap *felix_init_regmap(struct ocelot *ocelot, const char *name); #endif diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c index 4adb109c2e77..87a38a57613c 100644 --- a/drivers/net/dsa/ocelot/felix_vsc9959.c +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c @@ -2615,7 +2615,7 @@ static const struct felix_info felix_info_vsc9959 = { .port_setup_tc = vsc9959_port_setup_tc, .port_sched_speed_set = vsc9959_sched_speed_set, .tas_guard_bands_update = vsc9959_tas_guard_bands_update, - .init_regmap = ocelot_regmap_init, + .init_regmap = felix_init_regmap, }; static irqreturn_t felix_irq_handler(int irq, void *data) diff --git a/drivers/net/dsa/ocelot/seville_vsc9953.c b/drivers/net/dsa/ocelot/seville_vsc9953.c index ba71e5fa5921..a66bd35ce4ee 100644 --- a/drivers/net/dsa/ocelot/seville_vsc9953.c +++ b/drivers/net/dsa/ocelot/seville_vsc9953.c @@ -1079,7 +1079,7 @@ static const struct felix_info seville_info_vsc9953 = { .mdio_bus_free = vsc9953_mdio_bus_free, .phylink_validate = vsc9953_phylink_validate, .port_modes = vsc9953_port_modes, - .init_regmap = ocelot_regmap_init, + .init_regmap = felix_init_regmap, }; static int seville_probe(struct platform_device *pdev)
During development, it was believed that a wrapper for ocelot_regmap_init() would be sufficient for the felix driver to work in non-mmio scenarios. This was merged in during commit 242bd0c10bbd ("net: dsa: ocelot: felix: add interface for custom regmaps") As the external ocelot DSA driver grew closer to an acceptable state, it was realized that most of the parameters that were passed in from struct resource *res were useless and ignored. This is due to the fact that the external ocelot DSA driver utilizes dev_get_regmap(dev, resource->name). Instead of simply ignoring those parameters, refactor the API to only require the name as an argument. MMIO scenarios this will reconstruct the struct resource before calling ocelot_regmap_init(ocelot, resource). MFD scenarios need only call dev_get_regmap(dev, name). Signed-off-by: Colin Foster <colin.foster@in-advantage.com> --- v2 * New patch --- drivers/net/dsa/ocelot/felix.c | 59 ++++++++++++++++++------ drivers/net/dsa/ocelot/felix.h | 4 +- drivers/net/dsa/ocelot/felix_vsc9959.c | 2 +- drivers/net/dsa/ocelot/seville_vsc9953.c | 2 +- 4 files changed, 49 insertions(+), 18 deletions(-)