Re: [PATCH net-next v3 02/10] net: sparx5: add the basic sparx5 driver

Hi Philipp,

Thanks for your comments.

On Fri, 2021-06-04 at 11:28 +0200, Philipp Zabel wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

Hi Steen,

On Fri, 2021-06-04 at 10:55 +0200, Steen Hegelund wrote:

quoted

This adds the Sparx5 basic SwitchDev driver framework with IO range
mapping, switch device detection and core clock configuration.

Support for ports, phylink, netdev, mactable etc. are in the following
patches.

Signed-off-by: Steen Hegelund <steen.hegelund@microchip.com>
Signed-off-by: Bjarni Jonasson <redacted>
Signed-off-by: Lars Povlsen <redacted>
---
 drivers/net/ethernet/microchip/Kconfig        |    2 +
 drivers/net/ethernet/microchip/Makefile       |    2 +
 drivers/net/ethernet/microchip/sparx5/Kconfig |    9 +
 .../net/ethernet/microchip/sparx5/Makefile    |    8 +
 .../ethernet/microchip/sparx5/sparx5_main.c   |  746 +++
 .../ethernet/microchip/sparx5/sparx5_main.h   |  273 +
 .../microchip/sparx5/sparx5_main_regs.h       | 4642 +++++++++++++++++
 7 files changed, 5682 insertions(+)
 create mode 100644 drivers/net/ethernet/microchip/sparx5/Kconfig
 create mode 100644 drivers/net/ethernet/microchip/sparx5/Makefile
 create mode 100644 drivers/net/ethernet/microchip/sparx5/sparx5_main.c
 create mode 100644 drivers/net/ethernet/microchip/sparx5/sparx5_main.h
 create mode 100644 drivers/net/ethernet/microchip/sparx5/sparx5_main_regs.h

[...]

quoted

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c

b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
new file mode 100644
index 000000000000..73beb85bc52d

--- /dev/null
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c

@@ -0,0 +1,746 @@

[...]

quoted

+static int mchp_sparx5_probe(struct platform_device *pdev)
+{

[...]

quoted

+
+     sparx5->reset = devm_reset_control_get_shared(&pdev->dev, "switch");
+     if (IS_ERR(sparx5->reset)) {

Could you use devm_reset_control_get_optional_shared() instead of
ignoring this error? That would just return NULL if there's no "switch"
reset specified in the device tree.

Yes.  That sounds like a good idea.  I assume that the devm_reset_control_get_optional_shared()
would also return null if another driver has already performed the reset?

quoted

+             dev_warn(sparx5->dev, "Could not obtain reset control: %ld\n",
+                      PTR_ERR(sparx5->reset));
+             sparx5->reset = NULL;
+     } else {
+             reset_control_reset(sparx5->reset);
+     }

If this is the only place the reset is used, I'd remove it from struct
sparx5 and use a local variable instead.

Yes.  I will do that.

regards
Philipp

Best 

-- 
BR
Steen

-=-=-=-=-=-=-=-=-=-=-=-=-=-=
steen.hegelund@microchip.com



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel