| Message ID | 20220508185313.2222956-7-colin.foster@in-advantage.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | add support for VSC7512 control over SPI | expand |
On Sun, May 8, 2022 at 8:53 PM Colin Foster <colin.foster@in-advantage.com> wrote: > > There are a few Ocelot chips that can contain SGPIO logic, but can be > controlled externally. Specifically the VSC7511, 7512, 7513, and 7514. In > the externally controlled configurations these registers are not > memory-mapped. > > Add support for these non-memory-mapped configurations. ... > - regs = devm_platform_ioremap_resource(pdev, 0); > - if (IS_ERR(regs)) > - return PTR_ERR(regs); > + regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL); > + if (IS_ERR(regs)) { > + /* > + * Fall back to using IORESOURCE_REG, which is possible in an > + * MFD configuration > + */ > + res = platform_get_resource(pdev, IORESOURCE_REG, 0); > + if (!res) { > + dev_err(dev, "Failed to get resource\n"); > + return -ENODEV; > + } > + > + priv->regs = ocelot_init_regmap_from_resource(dev, res); > + } else { > + priv->regs = devm_regmap_init_mmio(dev, regs, ®map_config); > + } > > - priv->regs = devm_regmap_init_mmio(dev, regs, ®map_config); > if (IS_ERR(priv->regs)) > return PTR_ERR(priv->regs); This looks like repetition of something you have done in a few previous patches. Can you avoid code duplication by introducing a corresponding helper function?
Hi Andy, On Mon, May 09, 2022 at 10:44:42AM +0200, Andy Shevchenko wrote: > On Sun, May 8, 2022 at 8:53 PM Colin Foster > <colin.foster@in-advantage.com> wrote: > > > > There are a few Ocelot chips that can contain SGPIO logic, but can be > > controlled externally. Specifically the VSC7511, 7512, 7513, and 7514. In > > the externally controlled configurations these registers are not > > memory-mapped. > > > > Add support for these non-memory-mapped configurations. > > ... > > > - regs = devm_platform_ioremap_resource(pdev, 0); > > - if (IS_ERR(regs)) > > - return PTR_ERR(regs); > > + regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL); > > + if (IS_ERR(regs)) { > > + /* > > + * Fall back to using IORESOURCE_REG, which is possible in an > > + * MFD configuration > > + */ > > + res = platform_get_resource(pdev, IORESOURCE_REG, 0); > > + if (!res) { > > + dev_err(dev, "Failed to get resource\n"); > > + return -ENODEV; > > + } > > + > > + priv->regs = ocelot_init_regmap_from_resource(dev, res); > > + } else { > > + priv->regs = devm_regmap_init_mmio(dev, regs, ®map_config); > > + } > > > > - priv->regs = devm_regmap_init_mmio(dev, regs, ®map_config); > > if (IS_ERR(priv->regs)) > > return PTR_ERR(priv->regs); > > This looks like repetition of something you have done in a few > previous patches. Can you avoid code duplication by introducing a > corresponding helper function? That's a good idea. Thanks for the feedback! > > -- > With Best Regards, > Andy Shevchenko
diff --git a/drivers/pinctrl/pinctrl-microchip-sgpio.c b/drivers/pinctrl/pinctrl-microchip-sgpio.c index 8953175c7e3e..88ab961cc5b9 100644 --- a/drivers/pinctrl/pinctrl-microchip-sgpio.c +++ b/drivers/pinctrl/pinctrl-microchip-sgpio.c @@ -20,6 +20,7 @@ #include <linux/regmap.h> #include <linux/reset.h> #include <linux/spinlock.h> +#include <soc/mscc/ocelot.h> #include "core.h" #include "pinconf.h" @@ -899,6 +900,7 @@ static int microchip_sgpio_probe(struct platform_device *pdev) struct fwnode_handle *fwnode; struct reset_control *reset; struct sgpio_priv *priv; + struct resource *res; struct clk *clk; u32 __iomem *regs; u32 val; @@ -933,11 +935,23 @@ static int microchip_sgpio_probe(struct platform_device *pdev) return -EINVAL; } - regs = devm_platform_ioremap_resource(pdev, 0); - if (IS_ERR(regs)) - return PTR_ERR(regs); + regs = devm_platform_get_and_ioremap_resource(pdev, 0, NULL); + if (IS_ERR(regs)) { + /* + * Fall back to using IORESOURCE_REG, which is possible in an + * MFD configuration + */ + res = platform_get_resource(pdev, IORESOURCE_REG, 0); + if (!res) { + dev_err(dev, "Failed to get resource\n"); + return -ENODEV; + } + + priv->regs = ocelot_init_regmap_from_resource(dev, res); + } else { + priv->regs = devm_regmap_init_mmio(dev, regs, ®map_config); + } - priv->regs = devm_regmap_init_mmio(dev, regs, ®map_config); if (IS_ERR(priv->regs)) return PTR_ERR(priv->regs);
There are a few Ocelot chips that can contain SGPIO logic, but can be controlled externally. Specifically the VSC7511, 7512, 7513, and 7514. In the externally controlled configurations these registers are not memory-mapped. Add support for these non-memory-mapped configurations. Signed-off-by: Colin Foster <colin.foster@in-advantage.com> --- drivers/pinctrl/pinctrl-microchip-sgpio.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)