| Message ID | 20211119224313.2803941-4-colin.foster@in-advantage.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | prepare ocelot for external interface control | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/cc_maintainers | success | CCed 11 of 11 maintainers |
| netdev/build_clang | success | Errors and warnings before: 0 this patch: 0 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 38 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Fri, Nov 19, 2021 at 02:43:10PM -0800, Colin Foster wrote: > Add an interface so that non-mmio regmaps can be used > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > --- What is your plan with treating the vsc7514 spi chip as a multi function device, of which the DSA driver would probe only on the Ethernet switch portion of it? Would this patch still be needed in its current form?
On Sun, Nov 21, 2021 at 05:19:02PM +0000, Vladimir Oltean wrote: > On Fri, Nov 19, 2021 at 02:43:10PM -0800, Colin Foster wrote: > > Add an interface so that non-mmio regmaps can be used > > > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > > --- > > What is your plan with treating the vsc7514 spi chip as a multi function > device, of which the DSA driver would probe only on the Ethernet switch > portion of it? Would this patch still be needed in its current form? I don't have this fully mapped out, so I'm not positive. I think it would be needed though. Felix and Ocelot need regmaps and will need to get them from somewhere. The VSC7512 switch driver will need to provide the regmap directly (current form) or indirectly (by requesting it from the MFD parent). I'll be looking more into MFD devices as well. The madera driver seems like one I'd use to model the VSC751X MFD after - just from a brief look around the drivers/mfd directory.
On Mon, Nov 22, 2021 at 08:45:56AM -0800, Colin Foster wrote: > On Sun, Nov 21, 2021 at 05:19:02PM +0000, Vladimir Oltean wrote: > > On Fri, Nov 19, 2021 at 02:43:10PM -0800, Colin Foster wrote: > > > Add an interface so that non-mmio regmaps can be used > > > > > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > > > --- > > > > What is your plan with treating the vsc7514 spi chip as a multi function > > device, of which the DSA driver would probe only on the Ethernet switch > > portion of it? Would this patch still be needed in its current form? > > I don't have this fully mapped out, so I'm not positive. I think it > would be needed though. Felix and Ocelot need regmaps and will need to > get them from somewhere. The VSC7512 switch driver will need to provide > the regmap directly (current form) or indirectly (by requesting it from > the MFD parent). > > I'll be looking more into MFD devices as well. The madera driver seems > like one I'd use to model the VSC751X MFD after - just from a brief look > around the drivers/mfd directory. As you can infer from my RFC today - I've looked more into what the MFD implementation will be. I believe that will have no effect on this patch. Felix needs "ANA", so if it gets that regmap from the resource (felix / seville), by "devm_regmap_init" (current implementation) or "dev_get_regmap(dev->parent, res->name);" should make no difference from the Felix driver standpoint. That said, I'm fine holding this one off until that's proven out. I'd like to get feedback on my general RFC before skinking a couple days into that restructure.
On Fri, Dec 03, 2021 at 04:11:40PM -0800, Colin Foster wrote: > On Mon, Nov 22, 2021 at 08:45:56AM -0800, Colin Foster wrote: > > On Sun, Nov 21, 2021 at 05:19:02PM +0000, Vladimir Oltean wrote: > > > On Fri, Nov 19, 2021 at 02:43:10PM -0800, Colin Foster wrote: > > > > Add an interface so that non-mmio regmaps can be used > > > > > > > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > > > > --- > > > > > > What is your plan with treating the vsc7514 spi chip as a multi function > > > device, of which the DSA driver would probe only on the Ethernet switch > > > portion of it? Would this patch still be needed in its current form? > > > > I don't have this fully mapped out, so I'm not positive. I think it > > would be needed though. Felix and Ocelot need regmaps and will need to > > get them from somewhere. The VSC7512 switch driver will need to provide > > the regmap directly (current form) or indirectly (by requesting it from > > the MFD parent). > > > > I'll be looking more into MFD devices as well. The madera driver seems > > like one I'd use to model the VSC751X MFD after - just from a brief look > > around the drivers/mfd directory. > > As you can infer from my RFC today - I've looked more into what the MFD > implementation will be. I believe that will have no effect on this > patch. Felix needs "ANA", so if it gets that regmap from the resource Oh, Felix needs much more than just the analyzer target. But I get the overall point, having the DSA driver get the regmaps from the MFD parent should be fine. > (felix / seville), by "devm_regmap_init" (current implementation) or > "dev_get_regmap(dev->parent, res->name);" should make no difference from > the Felix driver standpoint. > > That said, I'm fine holding this one off until that's proven out. I'd > like to get feedback on my general RFC before skinking a couple days > into that restructure. Some feedback given: https://lore.kernel.org/all/20211204022037.dkipkk42qet4u7go@skbuf/T/ TL;DR: my current opinion is that MFD is the way to go. Andrew, Florian, Vivien, please chime in and share your opinion too, because this has potential implications on the design of future DSA switch drivers too, once a model like Colin's gets established.
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c index 7b42d219545c..2a90a703162d 100644 --- a/drivers/net/dsa/ocelot/felix.c +++ b/drivers/net/dsa/ocelot/felix.c @@ -1020,7 +1020,7 @@ static int felix_init_structs(struct felix *felix, int num_phys_ports) res.start += felix->switch_base; res.end += felix->switch_base; - target = ocelot_regmap_init(ocelot, &res); + target = felix->info->init_regmap(ocelot, &res); if (IS_ERR(target)) { dev_err(ocelot->dev, "Failed to map device memory space\n"); @@ -1057,7 +1057,7 @@ static int felix_init_structs(struct felix *felix, int num_phys_ports) res.start += felix->switch_base; res.end += felix->switch_base; - target = ocelot_regmap_init(ocelot, &res); + target = felix->info->init_regmap(ocelot, &res); 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 183dbf832db9..515bddc012c0 100644 --- a/drivers/net/dsa/ocelot/felix.h +++ b/drivers/net/dsa/ocelot/felix.h @@ -50,6 +50,8 @@ struct felix_info { enum tc_setup_type type, void *type_data); void (*port_sched_speed_set)(struct ocelot *ocelot, int port, u32 speed); + struct regmap *(*init_regmap)(struct ocelot *ocelot, + struct resource *res); }; extern const struct dsa_switch_ops felix_switch_ops; diff --git a/drivers/net/dsa/ocelot/felix_vsc9959.c b/drivers/net/dsa/ocelot/felix_vsc9959.c index 9a144fd8c2e3..4ddec3325f61 100644 --- a/drivers/net/dsa/ocelot/felix_vsc9959.c +++ b/drivers/net/dsa/ocelot/felix_vsc9959.c @@ -2165,6 +2165,7 @@ static const struct felix_info felix_info_vsc9959 = { .prevalidate_phy_mode = vsc9959_prevalidate_phy_mode, .port_setup_tc = vsc9959_port_setup_tc, .port_sched_speed_set = vsc9959_sched_speed_set, + .init_regmap = ocelot_regmap_init, }; 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 899b98193b4a..ce30464371e2 100644 --- a/drivers/net/dsa/ocelot/seville_vsc9953.c +++ b/drivers/net/dsa/ocelot/seville_vsc9953.c @@ -1187,6 +1187,7 @@ static const struct felix_info seville_info_vsc9953 = { .mdio_bus_free = vsc9953_mdio_bus_free, .phylink_validate = vsc9953_phylink_validate, .prevalidate_phy_mode = vsc9953_prevalidate_phy_mode, + .init_regmap = ocelot_regmap_init, }; static int seville_probe(struct platform_device *pdev)
Add an interface so that non-mmio regmaps can be used Signed-off-by: Colin Foster <colin.foster@in-advantage.com> --- drivers/net/dsa/ocelot/felix.c | 4 ++-- drivers/net/dsa/ocelot/felix.h | 2 ++ drivers/net/dsa/ocelot/felix_vsc9959.c | 1 + drivers/net/dsa/ocelot/seville_vsc9953.c | 1 + 4 files changed, 6 insertions(+), 2 deletions(-)