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

On Mon, 2021-06-07 at 09:34 +0200, Steen Hegelund wrote:

Hi Philipp,

Thanks for your comments.

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

quoted

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?

It returns the same (shared) reset control. But the following
reset_control_reset() [1] is a no-op if the control has already
triggered once.

If multiple drivers need to coordinate to trigger the reset more than
once (for example once after suspend/resume), both have to use
reset_control_rearm() [2].

[1] https://www.kernel.org/doc/html/latest/driver-api/reset.html#c.reset_control_reset
[2] https://www.kernel.org/doc/html/latest/driver-api/reset.html#c.reset_control_rearm

regards
Philipp

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