[PATCH v7 3/6] reset: mchp: sparx5: Map cpu-syscon locally in case of LAN966x

Herve Codina posted 6 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v7 3/6] reset: mchp: sparx5: Map cpu-syscon locally in case of LAN966x
Posted by Herve Codina 1 month, 3 weeks ago
In the LAN966x PCI device use case, the syscon API cannot be used as
it does not support device removal [1]. A syscon device is a core
"system" device and not a device available in some addon boards and so,
it is not supposed to be removed. The syscon API follows this assumption
but this assumption is no longer valid in the LAN966x use case.

In order to avoid the use of the syscon API and so, support for removal,
use a local mapping of the syscon device.

Link: https://lore.kernel.org/all/20240923100741.11277439@bootlin.com/ [1]
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
---
 drivers/reset/reset-microchip-sparx5.c | 35 +++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/reset/reset-microchip-sparx5.c b/drivers/reset/reset-microchip-sparx5.c
index 636e85c388b0..48a62d5da78d 100644
--- a/drivers/reset/reset-microchip-sparx5.c
+++ b/drivers/reset/reset-microchip-sparx5.c
@@ -62,6 +62,28 @@ static const struct reset_control_ops sparx5_reset_ops = {
 	.reset = sparx5_reset_noop,
 };
 
+static const struct regmap_config mchp_lan966x_syscon_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+static struct regmap *mchp_lan966x_syscon_to_regmap(struct device *dev,
+						    struct device_node *syscon_np)
+{
+	struct regmap_config regmap_config = mchp_lan966x_syscon_regmap_config;
+	resource_size_t size;
+	void __iomem *base;
+
+	base = devm_of_iomap(dev, syscon_np, 0, &size);
+	if (IS_ERR(base))
+		return ERR_CAST(base);
+
+	regmap_config.max_register = size - 4;
+
+	return devm_regmap_init_mmio(dev, base, &regmap_config);
+}
+
 static int mchp_sparx5_map_syscon(struct platform_device *pdev, char *name,
 				  struct regmap **target)
 {
@@ -72,7 +94,18 @@ static int mchp_sparx5_map_syscon(struct platform_device *pdev, char *name,
 	syscon_np = of_parse_phandle(pdev->dev.of_node, name, 0);
 	if (!syscon_np)
 		return -ENODEV;
-	regmap = syscon_node_to_regmap(syscon_np);
+
+	/*
+	 * The syscon API doesn't support syscon device removal.
+	 * When used in LAN966x PCI device, the cpu-syscon device needs to be
+	 * removed when the PCI device is removed.
+	 * In case of LAN966x, map the syscon device locally to support the
+	 * device removal.
+	 */
+	if (of_device_is_compatible(pdev->dev.of_node, "microchip,lan966x-switch-reset"))
+		regmap = mchp_lan966x_syscon_to_regmap(&pdev->dev, syscon_np);
+	else
+		regmap = syscon_node_to_regmap(syscon_np);
 	of_node_put(syscon_np);
 	if (IS_ERR(regmap)) {
 		err = PTR_ERR(regmap);
-- 
2.46.1
Re: [PATCH v7 3/6] reset: mchp: sparx5: Map cpu-syscon locally in case of LAN966x
Posted by Philipp Zabel 1 month, 2 weeks ago
On Do, 2024-10-03 at 10:16 +0200, Herve Codina wrote:
> In the LAN966x PCI device use case, the syscon API cannot be used as
> it does not support device removal [1]. A syscon device is a core
> "system" device and not a device available in some addon boards and so,
> it is not supposed to be removed. The syscon API follows this assumption
> but this assumption is no longer valid in the LAN966x use case.
> 
> In order to avoid the use of the syscon API and so, support for removal,
> use a local mapping of the syscon device.
> 
> Link: https://lore.kernel.org/all/20240923100741.11277439@bootlin.com/ [1]
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp
Re: [PATCH v7 3/6] reset: mchp: sparx5: Map cpu-syscon locally in case of LAN966x
Posted by Steen Hegelund 1 month, 2 weeks ago
Hi Herve,

On Thu, 2024-10-03 at 10:16 +0200, Herve Codina wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> In the LAN966x PCI device use case, the syscon API cannot be used as
> it does not support device removal [1]. A syscon device is a core
> "system" device and not a device available in some addon boards and
> so,
> it is not supposed to be removed. The syscon API follows this
> assumption
> but this assumption is no longer valid in the LAN966x use case.
> 
> In order to avoid the use of the syscon API and so, support for
> removal,
> use a local mapping of the syscon device.
> 
> Link:
> https://lore.kernel.org/all/20240923100741.11277439@bootlin.com/ [1]
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  drivers/reset/reset-microchip-sparx5.c | 35
> +++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/reset/reset-microchip-sparx5.c
> b/drivers/reset/reset-microchip-sparx5.c
> index 636e85c388b0..48a62d5da78d 100644
> --- a/drivers/reset/reset-microchip-sparx5.c
> +++ b/drivers/reset/reset-microchip-sparx5.c
> @@ -62,6 +62,28 @@ static const struct reset_control_ops
> sparx5_reset_ops = {
>         .reset = sparx5_reset_noop,
>  };
> 
> +static const struct regmap_config mchp_lan966x_syscon_regmap_config
> = {
> +       .reg_bits = 32,
> +       .val_bits = 32,
> +       .reg_stride = 4,
> +};
> +
> +static struct regmap *mchp_lan966x_syscon_to_regmap(struct device
> *dev,
> +                                                   struct
> device_node *syscon_np)
> +{
> +       struct regmap_config regmap_config =
> mchp_lan966x_syscon_regmap_config;
> +       resource_size_t size;
> +       void __iomem *base;
> +
> +       base = devm_of_iomap(dev, syscon_np, 0, &size);
> +       if (IS_ERR(base))
> +               return ERR_CAST(base);
> +
> +       regmap_config.max_register = size - 4;
> +
> +       return devm_regmap_init_mmio(dev, base, &regmap_config);
> +}
> +
>  static int mchp_sparx5_map_syscon(struct platform_device *pdev, char
> *name,
>                                   struct regmap **target)
>  {
> @@ -72,7 +94,18 @@ static int mchp_sparx5_map_syscon(struct
> platform_device *pdev, char *name,
>         syscon_np = of_parse_phandle(pdev->dev.of_node, name, 0);
>         if (!syscon_np)
>                 return -ENODEV;
> -       regmap = syscon_node_to_regmap(syscon_np);
> +
> +       /*
> +        * The syscon API doesn't support syscon device removal.
> +        * When used in LAN966x PCI device, the cpu-syscon device
> needs to be
> +        * removed when the PCI device is removed.
> +        * In case of LAN966x, map the syscon device locally to
> support the
> +        * device removal.
> +        */
> +       if (of_device_is_compatible(pdev->dev.of_node,
> "microchip,lan966x-switch-reset"))
> +               regmap = mchp_lan966x_syscon_to_regmap(&pdev->dev,
> syscon_np);
> +       else
> +               regmap = syscon_node_to_regmap(syscon_np);
>         of_node_put(syscon_np);
>         if (IS_ERR(regmap)) {
>                 err = PTR_ERR(regmap);
> --
> 2.46.1
> 

This looks good to me.

Reviewed-by: Steen Hegelund <Steen.Hegelund@microchip.com>

BR
Steen
Re: [PATCH v7 3/6] reset: mchp: sparx5: Map cpu-syscon locally in case of LAN966x
Posted by Geert Uytterhoeven 1 month, 2 weeks ago
Hi Steve,

On Wed, Oct 9, 2024 at 9:30 AM Steen Hegelund
<steen.hegelund@microchip.com> wrote:
> On Thu, 2024-10-03 at 10:16 +0200, Herve Codina wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe

Hmm, the email I received directly from Hervé did not have the part
you are quoting, so it looks like you are subject to a MiTM-attack ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH v7 3/6] reset: mchp: sparx5: Map cpu-syscon locally in case of LAN966x
Posted by Conor Dooley 1 month, 2 weeks ago
On Wed, Oct 09, 2024 at 12:20:32PM +0200, Geert Uytterhoeven wrote:
> Hi Steve,
> 
> On Wed, Oct 9, 2024 at 9:30 AM Steen Hegelund
> <steen.hegelund@microchip.com> wrote:
> > On Thu, 2024-10-03 at 10:16 +0200, Herve Codina wrote:
> > > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > > know the content is safe
> 
> Hmm, the email I received directly from Hervé did not have the part
> you are quoting, so it looks like you are subject to a MiTM-attack ;-)

Yeah, unfortunately we are subjected to that. This one at least just
adds some text, there's another "MiTM attacker" we have that sometimes
crops up and has a side effect of tab-to-space conversion. The joys of
corporate IT.