| Message ID | 20211218214954.109755-3-colin.foster@in-advantage.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | add support for VSC75XX control over SPI | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/apply | fail | Patch does not apply to net-next |
On Sat, 18 Dec 2021, Colin Foster wrote: > Child devices need to get a regmap from a resource struct, specifically > from the MFD parent. The MFD parent has the interface to the hardware > layer, which could be I2C, SPI, PCIe, etc. > > This is somewhat a hack... ideally child devices would interface with the > struct device* directly, by way of a function like > devm_get_regmap_from_resource which would be akin to > devm_get_and_ioremap_resource. A less ideal option would be to interface > directly with MFD to get a regmap from the parent. > > This solution is even less ideal than both of the two suggestions, so is > intentionally left in a separate commit after the initial MFD addition. > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > --- > drivers/mfd/ocelot-core.c | 9 +++++++++ > include/soc/mscc/ocelot.h | 12 ++++++++++++ > 2 files changed, 21 insertions(+) > > diff --git a/drivers/mfd/ocelot-core.c b/drivers/mfd/ocelot-core.c > index a65619a8190b..09132ea52760 100644 > --- a/drivers/mfd/ocelot-core.c > +++ b/drivers/mfd/ocelot-core.c > @@ -94,6 +94,15 @@ static struct regmap *ocelot_mfd_regmap_init(struct ocelot_mfd_core *core, > return regmap; > } > > +struct regmap *ocelot_mfd_get_regmap_from_resource(struct device *dev, > + const struct resource *res) > +{ > + struct ocelot_mfd_core *core = dev_get_drvdata(dev); > + > + return ocelot_mfd_regmap_init(core, res); > +} > +EXPORT_SYMBOL(ocelot_mfd_get_regmap_from_resource); This is almost certainly not the right way to do whatever it is you're trying to do! Please don't try to upstream "somewhat a hack"s into the Mainline kernel.
Hello Lee, On 29/12/2021 15:23:59+0000, Lee Jones wrote: > On Sat, 18 Dec 2021, Colin Foster wrote: > > > Child devices need to get a regmap from a resource struct, specifically > > from the MFD parent. The MFD parent has the interface to the hardware > > layer, which could be I2C, SPI, PCIe, etc. > > > > This is somewhat a hack... ideally child devices would interface with the > > struct device* directly, by way of a function like > > devm_get_regmap_from_resource which would be akin to > > devm_get_and_ioremap_resource. A less ideal option would be to interface > > directly with MFD to get a regmap from the parent. > > > > This solution is even less ideal than both of the two suggestions, so is > > intentionally left in a separate commit after the initial MFD addition. > > > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > > --- > > drivers/mfd/ocelot-core.c | 9 +++++++++ > > include/soc/mscc/ocelot.h | 12 ++++++++++++ > > 2 files changed, 21 insertions(+) > > > > diff --git a/drivers/mfd/ocelot-core.c b/drivers/mfd/ocelot-core.c > > index a65619a8190b..09132ea52760 100644 > > --- a/drivers/mfd/ocelot-core.c > > +++ b/drivers/mfd/ocelot-core.c > > @@ -94,6 +94,15 @@ static struct regmap *ocelot_mfd_regmap_init(struct ocelot_mfd_core *core, > > return regmap; > > } > > > > +struct regmap *ocelot_mfd_get_regmap_from_resource(struct device *dev, > > + const struct resource *res) > > +{ > > + struct ocelot_mfd_core *core = dev_get_drvdata(dev); > > + > > + return ocelot_mfd_regmap_init(core, res); > > +} > > +EXPORT_SYMBOL(ocelot_mfd_get_regmap_from_resource); > > This is almost certainly not the right way to do whatever it is you're > trying to do! > > Please don't try to upstream "somewhat a hack"s into the Mainline > kernel. > Please elaborate on the correct way to do that. What we have here is a SoC (vsc7514) that has MMIO devices. This SoC has a MIPS CPU and everything is fine when using it. However, the CPU can be disabled and the SoC connected to another CPU using SPI or PCIe. What Colin is doing here is using this SoC over SPI. Don't tell me this is not an MFD because this is exactly what this is, a single chip with a collection of devices that are also available separately. The various drivers for the VSC7514 have been written using regmap exactly for this use case. The missing piece is probing the devices over SPI instead of MMIO. Notice that all of that gets worse when using PCIe on architectures that don't have device tree support and Clément will submit multiple series trying to fix that.
> > This is almost certainly not the right way to do whatever it is you're > > trying to do! > > > > Please don't try to upstream "somewhat a hack"s into the Mainline > > kernel. > > > > Please elaborate on the correct way to do that. What we have here is a > SoC (vsc7514) that has MMIO devices. This SoC has a MIPS CPU and > everything is fine when using it. However, the CPU can be disabled and > the SoC connected to another CPU using SPI or PCIe. What Colin is doing > here is using this SoC over SPI. Don't tell me this is not an MFD When did anyone say that? > because this is exactly what this is, a single chip with a collection of > devices that are also available separately. > > The various drivers for the VSC7514 have been written using regmap > exactly for this use case. The missing piece is probing the devices over > SPI instead of MMIO. > > Notice that all of that gets worse when using PCIe on architectures that > don't have device tree support and Clément will submit multiple series > trying to fix that. Okay, it sounds like I'm missing some information, let's see if we can get to the bottom of this so that I can provide some informed guidance. > On 29/12/2021 15:23:59+0000, Lee Jones wrote: > > On Sat, 18 Dec 2021, Colin Foster wrote: > > > > > Child devices need to get a regmap from a resource struct, > > > specifically from the MFD parent. Child devices usually fetch the Regmap from a pointer the parent provides. Usually via either 'driver_data' or 'platform_data'. However, we can't do that here because ... > > > The MFD parent has the interface to the hardware > > > layer, which could be I2C, SPI, PCIe, etc. > > > > > > This is somewhat a hack... ideally child devices would interface with the > > > struct device* directly, by way of a function like > > > devm_get_regmap_from_resource which would be akin to > > > devm_get_and_ioremap_resource. However, we can't do that here because ... > > > A less ideal option would be to interface > > > directly with MFD to get a regmap from the parent. > > > > > > This solution is even less ideal than both of the two suggestions, so is > > > intentionally left in a separate commit after the initial MFD addition. > > > > > > Signed-off-by: Colin Foster <colin.foster@in-advantage.com> > > > --- > > > drivers/mfd/ocelot-core.c | 9 +++++++++ > > > include/soc/mscc/ocelot.h | 12 ++++++++++++ > > > 2 files changed, 21 insertions(+) > > > > > > diff --git a/drivers/mfd/ocelot-core.c b/drivers/mfd/ocelot-core.c > > > index a65619a8190b..09132ea52760 100644 > > > --- a/drivers/mfd/ocelot-core.c > > > +++ b/drivers/mfd/ocelot-core.c > > > @@ -94,6 +94,15 @@ static struct regmap *ocelot_mfd_regmap_init(struct ocelot_mfd_core *core, > > > return regmap; > > > } > > > > > > +struct regmap *ocelot_mfd_get_regmap_from_resource(struct device *dev, > > > + const struct resource *res) > > > +{ > > > + struct ocelot_mfd_core *core = dev_get_drvdata(dev); > > > + > > > + return ocelot_mfd_regmap_init(core, res); > > > +} > > > +EXPORT_SYMBOL(ocelot_mfd_get_regmap_from_resource); > > >
diff --git a/drivers/mfd/ocelot-core.c b/drivers/mfd/ocelot-core.c index a65619a8190b..09132ea52760 100644 --- a/drivers/mfd/ocelot-core.c +++ b/drivers/mfd/ocelot-core.c @@ -94,6 +94,15 @@ static struct regmap *ocelot_mfd_regmap_init(struct ocelot_mfd_core *core, return regmap; } +struct regmap *ocelot_mfd_get_regmap_from_resource(struct device *dev, + const struct resource *res) +{ + struct ocelot_mfd_core *core = dev_get_drvdata(dev); + + return ocelot_mfd_regmap_init(core, res); +} +EXPORT_SYMBOL(ocelot_mfd_get_regmap_from_resource); + int ocelot_mfd_init(struct ocelot_mfd_config *config) { struct device *dev = config->dev; diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h index 3e9454b00562..a641c9cc6f3f 100644 --- a/include/soc/mscc/ocelot.h +++ b/include/soc/mscc/ocelot.h @@ -968,4 +968,16 @@ ocelot_mrp_del_ring_role(struct ocelot *ocelot, int port, } #endif +#if IS_ENABLED(CONFIG_MFD_OCELOT_CORE) +struct regmap *ocelot_mfd_get_regmap_from_resource(struct device *dev, + const struct resource *res); +#else +static inline regmap * +ocelot_mfd_get_regmap_from_resource(struct device *dev, + const struct resource *res) +{ + return NULL; +} +#endif + #endif
Child devices need to get a regmap from a resource struct, specifically from the MFD parent. The MFD parent has the interface to the hardware layer, which could be I2C, SPI, PCIe, etc. This is somewhat a hack... ideally child devices would interface with the struct device* directly, by way of a function like devm_get_regmap_from_resource which would be akin to devm_get_and_ioremap_resource. A less ideal option would be to interface directly with MFD to get a regmap from the parent. This solution is even less ideal than both of the two suggestions, so is intentionally left in a separate commit after the initial MFD addition. Signed-off-by: Colin Foster <colin.foster@in-advantage.com> --- drivers/mfd/ocelot-core.c | 9 +++++++++ include/soc/mscc/ocelot.h | 12 ++++++++++++ 2 files changed, 21 insertions(+)