Hi Steen,
On Fri, 2021-06-04 at 10:55 +0200, Steen Hegelund wrote:
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 hunk
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
[...]
+static int mchp_sparx5_probe(struct platform_device *pdev)
+{
[...]
+
+ 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.
+ 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.
regards
Philipp
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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
[...]
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
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
[...]
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