All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Add reset deassertion for Aspeed MDIO
@ 2022-03-21  7:01 ` Dylan Hung
  0 siblings, 0 replies; 16+ messages in thread
From: Dylan Hung @ 2022-03-21  7:01 UTC (permalink / raw)
  To: robh+dt, joel, andrew, andrew, hkallweit1, linux, davem, kuba,
	pabeni, p.zabel, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev
  Cc: BMC-SW

Add missing reset deassertion for Aspeed MDIO. There are 4 MDIOs
embedded in Aspeed AST2600 and share one reset control bit SCU50[3].

Dylan Hung (2):
  net: mdio: add reset deassertion for Aspeed MDIO
  ARM: dts: aspeed: add reset properties into MDIO nodes

 arch/arm/boot/dts/aspeed-g6.dtsi | 4 ++++
 drivers/net/mdio/mdio-aspeed.c   | 8 ++++++++
 2 files changed, 12 insertions(+)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 0/2] Add reset deassertion for Aspeed MDIO
@ 2022-03-21  7:01 ` Dylan Hung
  0 siblings, 0 replies; 16+ messages in thread
From: Dylan Hung @ 2022-03-21  7:01 UTC (permalink / raw)
  To: robh+dt, joel, andrew, andrew, hkallweit1, linux, davem, kuba,
	pabeni, p.zabel, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev
  Cc: BMC-SW

Add missing reset deassertion for Aspeed MDIO. There are 4 MDIOs
embedded in Aspeed AST2600 and share one reset control bit SCU50[3].

Dylan Hung (2):
  net: mdio: add reset deassertion for Aspeed MDIO
  ARM: dts: aspeed: add reset properties into MDIO nodes

 arch/arm/boot/dts/aspeed-g6.dtsi | 4 ++++
 drivers/net/mdio/mdio-aspeed.c   | 8 ++++++++
 2 files changed, 12 insertions(+)

-- 
2.25.1


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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/2] net: mdio: add reset deassertion for Aspeed MDIO
  2022-03-21  7:01 ` Dylan Hung
@ 2022-03-21  7:01   ` Dylan Hung
  -1 siblings, 0 replies; 16+ messages in thread
From: Dylan Hung @ 2022-03-21  7:01 UTC (permalink / raw)
  To: robh+dt, joel, andrew, andrew, hkallweit1, linux, davem, kuba,
	pabeni, p.zabel, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev
  Cc: BMC-SW, stable

Add reset deassertion for Aspeed MDIO.  There are 4 MDIO controllers
embedded in Aspeed AST2600 SOC and share one reset control register
SCU50[3]. So devm_reset_control_get_shared is used in this change.

Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
Cc: stable@vger.kernel.org
---
 drivers/net/mdio/mdio-aspeed.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/mdio/mdio-aspeed.c b/drivers/net/mdio/mdio-aspeed.c
index e2273588c75b..8ac262a12d13 100644
--- a/drivers/net/mdio/mdio-aspeed.c
+++ b/drivers/net/mdio/mdio-aspeed.c
@@ -3,6 +3,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/delay.h>
+#include <linux/reset.h>
 #include <linux/iopoll.h>
 #include <linux/mdio.h>
 #include <linux/module.h>
@@ -37,6 +38,7 @@
 
 struct aspeed_mdio {
 	void __iomem *base;
+	struct reset_control *reset;
 };
 
 static int aspeed_mdio_read(struct mii_bus *bus, int addr, int regnum)
@@ -120,6 +122,12 @@ static int aspeed_mdio_probe(struct platform_device *pdev)
 	if (IS_ERR(ctx->base))
 		return PTR_ERR(ctx->base);
 
+	ctx->reset = devm_reset_control_get_shared(&pdev->dev, NULL);
+	if (IS_ERR(ctx->reset))
+		return PTR_ERR(ctx->reset);
+
+	reset_control_deassert(ctx->reset);
+
 	bus->name = DRV_NAME;
 	snprintf(bus->id, MII_BUS_ID_SIZE, "%s%d", pdev->name, pdev->id);
 	bus->parent = &pdev->dev;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 1/2] net: mdio: add reset deassertion for Aspeed MDIO
@ 2022-03-21  7:01   ` Dylan Hung
  0 siblings, 0 replies; 16+ messages in thread
From: Dylan Hung @ 2022-03-21  7:01 UTC (permalink / raw)
  To: robh+dt, joel, andrew, andrew, hkallweit1, linux, davem, kuba,
	pabeni, p.zabel, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev
  Cc: BMC-SW, stable

Add reset deassertion for Aspeed MDIO.  There are 4 MDIO controllers
embedded in Aspeed AST2600 SOC and share one reset control register
SCU50[3]. So devm_reset_control_get_shared is used in this change.

Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
Cc: stable@vger.kernel.org
---
 drivers/net/mdio/mdio-aspeed.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/mdio/mdio-aspeed.c b/drivers/net/mdio/mdio-aspeed.c
index e2273588c75b..8ac262a12d13 100644
--- a/drivers/net/mdio/mdio-aspeed.c
+++ b/drivers/net/mdio/mdio-aspeed.c
@@ -3,6 +3,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/delay.h>
+#include <linux/reset.h>
 #include <linux/iopoll.h>
 #include <linux/mdio.h>
 #include <linux/module.h>
@@ -37,6 +38,7 @@
 
 struct aspeed_mdio {
 	void __iomem *base;
+	struct reset_control *reset;
 };
 
 static int aspeed_mdio_read(struct mii_bus *bus, int addr, int regnum)
@@ -120,6 +122,12 @@ static int aspeed_mdio_probe(struct platform_device *pdev)
 	if (IS_ERR(ctx->base))
 		return PTR_ERR(ctx->base);
 
+	ctx->reset = devm_reset_control_get_shared(&pdev->dev, NULL);
+	if (IS_ERR(ctx->reset))
+		return PTR_ERR(ctx->reset);
+
+	reset_control_deassert(ctx->reset);
+
 	bus->name = DRV_NAME;
 	snprintf(bus->id, MII_BUS_ID_SIZE, "%s%d", pdev->name, pdev->id);
 	bus->parent = &pdev->dev;
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] ARM: dts: aspeed: add reset properties into MDIO nodes
  2022-03-21  7:01 ` Dylan Hung
@ 2022-03-21  7:01   ` Dylan Hung
  -1 siblings, 0 replies; 16+ messages in thread
From: Dylan Hung @ 2022-03-21  7:01 UTC (permalink / raw)
  To: robh+dt, joel, andrew, andrew, hkallweit1, linux, davem, kuba,
	pabeni, p.zabel, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev
  Cc: BMC-SW, stable

Add reset control properties into MDIO nodes.  The 4 MDIO controllers in
AST2600 SOC share one reset control bit SCU50[3].

Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
Cc: stable@vger.kernel.org
---
 arch/arm/boot/dts/aspeed-g6.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
index c32e87fad4dc..ab20ea8d829d 100644
--- a/arch/arm/boot/dts/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6.dtsi
@@ -181,6 +181,7 @@ mdio0: mdio@1e650000 {
 			status = "disabled";
 			pinctrl-names = "default";
 			pinctrl-0 = <&pinctrl_mdio1_default>;
+			resets = <&syscon ASPEED_RESET_MII>;
 		};
 
 		mdio1: mdio@1e650008 {
@@ -191,6 +192,7 @@ mdio1: mdio@1e650008 {
 			status = "disabled";
 			pinctrl-names = "default";
 			pinctrl-0 = <&pinctrl_mdio2_default>;
+			resets = <&syscon ASPEED_RESET_MII>;
 		};
 
 		mdio2: mdio@1e650010 {
@@ -201,6 +203,7 @@ mdio2: mdio@1e650010 {
 			status = "disabled";
 			pinctrl-names = "default";
 			pinctrl-0 = <&pinctrl_mdio3_default>;
+			resets = <&syscon ASPEED_RESET_MII>;
 		};
 
 		mdio3: mdio@1e650018 {
@@ -211,6 +214,7 @@ mdio3: mdio@1e650018 {
 			status = "disabled";
 			pinctrl-names = "default";
 			pinctrl-0 = <&pinctrl_mdio4_default>;
+			resets = <&syscon ASPEED_RESET_MII>;
 		};
 
 		mac0: ftgmac@1e660000 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] ARM: dts: aspeed: add reset properties into MDIO nodes
@ 2022-03-21  7:01   ` Dylan Hung
  0 siblings, 0 replies; 16+ messages in thread
From: Dylan Hung @ 2022-03-21  7:01 UTC (permalink / raw)
  To: robh+dt, joel, andrew, andrew, hkallweit1, linux, davem, kuba,
	pabeni, p.zabel, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev
  Cc: BMC-SW, stable

Add reset control properties into MDIO nodes.  The 4 MDIO controllers in
AST2600 SOC share one reset control bit SCU50[3].

Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
Cc: stable@vger.kernel.org
---
 arch/arm/boot/dts/aspeed-g6.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi
index c32e87fad4dc..ab20ea8d829d 100644
--- a/arch/arm/boot/dts/aspeed-g6.dtsi
+++ b/arch/arm/boot/dts/aspeed-g6.dtsi
@@ -181,6 +181,7 @@ mdio0: mdio@1e650000 {
 			status = "disabled";
 			pinctrl-names = "default";
 			pinctrl-0 = <&pinctrl_mdio1_default>;
+			resets = <&syscon ASPEED_RESET_MII>;
 		};
 
 		mdio1: mdio@1e650008 {
@@ -191,6 +192,7 @@ mdio1: mdio@1e650008 {
 			status = "disabled";
 			pinctrl-names = "default";
 			pinctrl-0 = <&pinctrl_mdio2_default>;
+			resets = <&syscon ASPEED_RESET_MII>;
 		};
 
 		mdio2: mdio@1e650010 {
@@ -201,6 +203,7 @@ mdio2: mdio@1e650010 {
 			status = "disabled";
 			pinctrl-names = "default";
 			pinctrl-0 = <&pinctrl_mdio3_default>;
+			resets = <&syscon ASPEED_RESET_MII>;
 		};
 
 		mdio3: mdio@1e650018 {
@@ -211,6 +214,7 @@ mdio3: mdio@1e650018 {
 			status = "disabled";
 			pinctrl-names = "default";
 			pinctrl-0 = <&pinctrl_mdio4_default>;
+			resets = <&syscon ASPEED_RESET_MII>;
 		};
 
 		mac0: ftgmac@1e660000 {
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] net: mdio: add reset deassertion for Aspeed MDIO
  2022-03-21  7:01   ` Dylan Hung
@ 2022-03-21  8:14     ` Philipp Zabel
  -1 siblings, 0 replies; 16+ messages in thread
From: Philipp Zabel @ 2022-03-21  8:14 UTC (permalink / raw)
  To: Dylan Hung, robh+dt, joel, andrew, andrew, hkallweit1, linux,
	davem, kuba, pabeni, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev
  Cc: BMC-SW, stable

Hi Dylan,

On Mo, 2022-03-21 at 15:01 +0800, Dylan Hung wrote:
> Add reset deassertion for Aspeed MDIO.  There are 4 MDIO controllers
> embedded in Aspeed AST2600 SOC and share one reset control register
> SCU50[3]. So devm_reset_control_get_shared is used in this change.
> 
> Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/net/mdio/mdio-aspeed.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/mdio/mdio-aspeed.c b/drivers/net/mdio/mdio-
> aspeed.c
> index e2273588c75b..8ac262a12d13 100644
> --- a/drivers/net/mdio/mdio-aspeed.c
> +++ b/drivers/net/mdio/mdio-aspeed.c
> @@ -3,6 +3,7 @@
>  
>  #include <linux/bitfield.h>
>  #include <linux/delay.h>
> +#include <linux/reset.h>
>  #include <linux/iopoll.h>
>  #include <linux/mdio.h>
>  #include <linux/module.h>
> @@ -37,6 +38,7 @@
>  
>  struct aspeed_mdio {
>         void __iomem *base;
> +       struct reset_control *reset;
>  };
>  
>  static int aspeed_mdio_read(struct mii_bus *bus, int addr, int
> regnum)
> @@ -120,6 +122,12 @@ static int aspeed_mdio_probe(struct
> platform_device *pdev)
>         if (IS_ERR(ctx->base))
>                 return PTR_ERR(ctx->base);
>  
> +       ctx->reset = devm_reset_control_get_shared(&pdev->dev, NULL);

The device tree bindings should have a required reset control property
documented before this is added.

> +       if (IS_ERR(ctx->reset))
> +               return PTR_ERR(ctx->reset);
> +
> +       reset_control_deassert(ctx->reset);
> +

This is missing a corresponding reset_control_assert() in
aspeed_mdio_remove() and in the error path of of_mdiobus_register().
That would allow to assert the reset line again after all MDIO
controllers are unbound.

regards
Philipp

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] net: mdio: add reset deassertion for Aspeed MDIO
@ 2022-03-21  8:14     ` Philipp Zabel
  0 siblings, 0 replies; 16+ messages in thread
From: Philipp Zabel @ 2022-03-21  8:14 UTC (permalink / raw)
  To: Dylan Hung, robh+dt, joel, andrew, andrew, hkallweit1, linux,
	davem, kuba, pabeni, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev
  Cc: BMC-SW, stable

Hi Dylan,

On Mo, 2022-03-21 at 15:01 +0800, Dylan Hung wrote:
> Add reset deassertion for Aspeed MDIO.  There are 4 MDIO controllers
> embedded in Aspeed AST2600 SOC and share one reset control register
> SCU50[3]. So devm_reset_control_get_shared is used in this change.
> 
> Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/net/mdio/mdio-aspeed.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/net/mdio/mdio-aspeed.c b/drivers/net/mdio/mdio-
> aspeed.c
> index e2273588c75b..8ac262a12d13 100644
> --- a/drivers/net/mdio/mdio-aspeed.c
> +++ b/drivers/net/mdio/mdio-aspeed.c
> @@ -3,6 +3,7 @@
>  
>  #include <linux/bitfield.h>
>  #include <linux/delay.h>
> +#include <linux/reset.h>
>  #include <linux/iopoll.h>
>  #include <linux/mdio.h>
>  #include <linux/module.h>
> @@ -37,6 +38,7 @@
>  
>  struct aspeed_mdio {
>         void __iomem *base;
> +       struct reset_control *reset;
>  };
>  
>  static int aspeed_mdio_read(struct mii_bus *bus, int addr, int
> regnum)
> @@ -120,6 +122,12 @@ static int aspeed_mdio_probe(struct
> platform_device *pdev)
>         if (IS_ERR(ctx->base))
>                 return PTR_ERR(ctx->base);
>  
> +       ctx->reset = devm_reset_control_get_shared(&pdev->dev, NULL);

The device tree bindings should have a required reset control property
documented before this is added.

> +       if (IS_ERR(ctx->reset))
> +               return PTR_ERR(ctx->reset);
> +
> +       reset_control_deassert(ctx->reset);
> +

This is missing a corresponding reset_control_assert() in
aspeed_mdio_remove() and in the error path of of_mdiobus_register().
That would allow to assert the reset line again after all MDIO
controllers are unbound.

regards
Philipp

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 1/2] net: mdio: add reset deassertion for Aspeed MDIO
  2022-03-21  8:14     ` Philipp Zabel
@ 2022-03-21  8:20       ` Dylan Hung
  -1 siblings, 0 replies; 16+ messages in thread
From: Dylan Hung @ 2022-03-21  8:20 UTC (permalink / raw)
  To: Philipp Zabel, robh+dt, joel, andrew, andrew, hkallweit1, linux,
	davem, kuba, pabeni, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev
  Cc: BMC-SW, stable

> -----Original Message-----
> From: Philipp Zabel [mailto:p.zabel@pengutronix.de]
> Sent: 2022年3月21日 4:15 PM
> To: Dylan Hung <dylan_hung@aspeedtech.com>; robh+dt@kernel.org;
> joel@jms.id.au; andrew@aj.id.au; andrew@lunn.ch; hkallweit1@gmail.com;
> linux@armlinux.org.uk; davem@davemloft.net; kuba@kernel.org;
> pabeni@redhat.com; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org;
> linux-kernel@vger.kernel.org; netdev@vger.kernel.org
> Cc: BMC-SW <BMC-SW@aspeedtech.com>; stable@vger.kernel.org
> Subject: Re: [PATCH 1/2] net: mdio: add reset deassertion for Aspeed MDIO
> 
> Hi Dylan,
> 
> On Mo, 2022-03-21 at 15:01 +0800, Dylan Hung wrote:
> > Add reset deassertion for Aspeed MDIO.  There are 4 MDIO controllers
> > embedded in Aspeed AST2600 SOC and share one reset control register
> > SCU50[3]. So devm_reset_control_get_shared is used in this change.
> >
> > Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
> > Cc: stable@vger.kernel.org
> > ---
> >  drivers/net/mdio/mdio-aspeed.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/net/mdio/mdio-aspeed.c b/drivers/net/mdio/mdio-
> > aspeed.c index e2273588c75b..8ac262a12d13 100644
> > --- a/drivers/net/mdio/mdio-aspeed.c
> > +++ b/drivers/net/mdio/mdio-aspeed.c
> > @@ -3,6 +3,7 @@
> >
> >  #include <linux/bitfield.h>
> >  #include <linux/delay.h>
> > +#include <linux/reset.h>
> >  #include <linux/iopoll.h>
> >  #include <linux/mdio.h>
> >  #include <linux/module.h>
> > @@ -37,6 +38,7 @@
> >
> >  struct aspeed_mdio {
> >         void __iomem *base;
> > +       struct reset_control *reset;
> >  };
> >
> >  static int aspeed_mdio_read(struct mii_bus *bus, int addr, int
> > regnum)
> > @@ -120,6 +122,12 @@ static int aspeed_mdio_probe(struct
> > platform_device *pdev)
> >         if (IS_ERR(ctx->base))
> >                 return PTR_ERR(ctx->base);
> >
> > +       ctx->reset = devm_reset_control_get_shared(&pdev->dev, NULL);
> 
> The device tree bindings should have a required reset control property
> documented before this is added.
> 

OK, I will add a commit for the document in V2.

> > +       if (IS_ERR(ctx->reset))
> > +               return PTR_ERR(ctx->reset);
> > +
> > +       reset_control_deassert(ctx->reset);
> > +
> 
> This is missing a corresponding reset_control_assert() in
> aspeed_mdio_remove() and in the error path of of_mdiobus_register().
> That would allow to assert the reset line again after all MDIO controllers are
> unbound.

Thank you for your comment. I will fix it in V2.

> 
> regards
> Philipp

--
Dylan

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 1/2] net: mdio: add reset deassertion for Aspeed MDIO
@ 2022-03-21  8:20       ` Dylan Hung
  0 siblings, 0 replies; 16+ messages in thread
From: Dylan Hung @ 2022-03-21  8:20 UTC (permalink / raw)
  To: Philipp Zabel, robh+dt, joel, andrew, andrew, hkallweit1, linux,
	davem, kuba, pabeni, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev
  Cc: BMC-SW, stable

> -----Original Message-----
> From: Philipp Zabel [mailto:p.zabel@pengutronix.de]
> Sent: 2022年3月21日 4:15 PM
> To: Dylan Hung <dylan_hung@aspeedtech.com>; robh+dt@kernel.org;
> joel@jms.id.au; andrew@aj.id.au; andrew@lunn.ch; hkallweit1@gmail.com;
> linux@armlinux.org.uk; davem@davemloft.net; kuba@kernel.org;
> pabeni@redhat.com; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-aspeed@lists.ozlabs.org;
> linux-kernel@vger.kernel.org; netdev@vger.kernel.org
> Cc: BMC-SW <BMC-SW@aspeedtech.com>; stable@vger.kernel.org
> Subject: Re: [PATCH 1/2] net: mdio: add reset deassertion for Aspeed MDIO
> 
> Hi Dylan,
> 
> On Mo, 2022-03-21 at 15:01 +0800, Dylan Hung wrote:
> > Add reset deassertion for Aspeed MDIO.  There are 4 MDIO controllers
> > embedded in Aspeed AST2600 SOC and share one reset control register
> > SCU50[3]. So devm_reset_control_get_shared is used in this change.
> >
> > Signed-off-by: Dylan Hung <dylan_hung@aspeedtech.com>
> > Cc: stable@vger.kernel.org
> > ---
> >  drivers/net/mdio/mdio-aspeed.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/net/mdio/mdio-aspeed.c b/drivers/net/mdio/mdio-
> > aspeed.c index e2273588c75b..8ac262a12d13 100644
> > --- a/drivers/net/mdio/mdio-aspeed.c
> > +++ b/drivers/net/mdio/mdio-aspeed.c
> > @@ -3,6 +3,7 @@
> >
> >  #include <linux/bitfield.h>
> >  #include <linux/delay.h>
> > +#include <linux/reset.h>
> >  #include <linux/iopoll.h>
> >  #include <linux/mdio.h>
> >  #include <linux/module.h>
> > @@ -37,6 +38,7 @@
> >
> >  struct aspeed_mdio {
> >         void __iomem *base;
> > +       struct reset_control *reset;
> >  };
> >
> >  static int aspeed_mdio_read(struct mii_bus *bus, int addr, int
> > regnum)
> > @@ -120,6 +122,12 @@ static int aspeed_mdio_probe(struct
> > platform_device *pdev)
> >         if (IS_ERR(ctx->base))
> >                 return PTR_ERR(ctx->base);
> >
> > +       ctx->reset = devm_reset_control_get_shared(&pdev->dev, NULL);
> 
> The device tree bindings should have a required reset control property
> documented before this is added.
> 

OK, I will add a commit for the document in V2.

> > +       if (IS_ERR(ctx->reset))
> > +               return PTR_ERR(ctx->reset);
> > +
> > +       reset_control_deassert(ctx->reset);
> > +
> 
> This is missing a corresponding reset_control_assert() in
> aspeed_mdio_remove() and in the error path of of_mdiobus_register().
> That would allow to assert the reset line again after all MDIO controllers are
> unbound.

Thank you for your comment. I will fix it in V2.

> 
> regards
> Philipp

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] Add reset deassertion for Aspeed MDIO
  2022-03-21  7:01 ` Dylan Hung
@ 2022-03-21 12:10   ` Andrew Lunn
  -1 siblings, 0 replies; 16+ messages in thread
From: Andrew Lunn @ 2022-03-21 12:10 UTC (permalink / raw)
  To: Dylan Hung
  Cc: robh+dt, joel, andrew, hkallweit1, linux, davem, kuba, pabeni,
	p.zabel, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev, BMC-SW

On Mon, Mar 21, 2022 at 03:01:29PM +0800, Dylan Hung wrote:
> Add missing reset deassertion for Aspeed MDIO. There are 4 MDIOs
> embedded in Aspeed AST2600 and share one reset control bit SCU50[3].

Is the reset limited to the MDIO bus masters, or are PHYs one the bus
potentially also reset?

Who asserts the reset in the first place? Don't you want the first
MDIO bus to probe to assert and then deassert the reset in order that
all the hardware is reset?

    Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] Add reset deassertion for Aspeed MDIO
@ 2022-03-21 12:10   ` Andrew Lunn
  0 siblings, 0 replies; 16+ messages in thread
From: Andrew Lunn @ 2022-03-21 12:10 UTC (permalink / raw)
  To: Dylan Hung
  Cc: robh+dt, joel, andrew, hkallweit1, linux, davem, kuba, pabeni,
	p.zabel, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev, BMC-SW

On Mon, Mar 21, 2022 at 03:01:29PM +0800, Dylan Hung wrote:
> Add missing reset deassertion for Aspeed MDIO. There are 4 MDIOs
> embedded in Aspeed AST2600 and share one reset control bit SCU50[3].

Is the reset limited to the MDIO bus masters, or are PHYs one the bus
potentially also reset?

Who asserts the reset in the first place? Don't you want the first
MDIO bus to probe to assert and then deassert the reset in order that
all the hardware is reset?

    Andrew

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 0/2] Add reset deassertion for Aspeed MDIO
  2022-03-21 12:10   ` Andrew Lunn
@ 2022-03-22  9:10     ` Dylan Hung
  -1 siblings, 0 replies; 16+ messages in thread
From: Dylan Hung @ 2022-03-22  9:10 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: robh+dt, joel, andrew, hkallweit1, linux, davem, kuba, pabeni,
	p.zabel, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev, BMC-SW

Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: 2022年3月21日 8:11 PM
> To: Dylan Hung <dylan_hung@aspeedtech.com>
> Cc: robh+dt@kernel.org; joel@jms.id.au; andrew@aj.id.au;
> hkallweit1@gmail.com; linux@armlinux.org.uk; davem@davemloft.net;
> kuba@kernel.org; pabeni@redhat.com; p.zabel@pengutronix.de;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; BMC-SW <BMC-SW@aspeedtech.com>
> Subject: Re: [PATCH 0/2] Add reset deassertion for Aspeed MDIO
> 
> On Mon, Mar 21, 2022 at 03:01:29PM +0800, Dylan Hung wrote:
> > Add missing reset deassertion for Aspeed MDIO. There are 4 MDIOs
> > embedded in Aspeed AST2600 and share one reset control bit SCU50[3].
> 
> Is the reset limited to the MDIO bus masters, or are PHYs one the bus
> potentially also reset?

It is limited to the MDIO bus masters.

> 
> Who asserts the reset in the first place? 

The hardware asserts the reset by default.

> Don't you want the first MDIO bus to
> probe to assert and then deassert the reset in order that all the hardware is
> reset?

Do I still need to add a reset assertion/deassertion if the hardware asserts the reset by default?

> 
>     Andrew

--
Dylan


^ permalink raw reply	[flat|nested] 16+ messages in thread

* RE: [PATCH 0/2] Add reset deassertion for Aspeed MDIO
@ 2022-03-22  9:10     ` Dylan Hung
  0 siblings, 0 replies; 16+ messages in thread
From: Dylan Hung @ 2022-03-22  9:10 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: robh+dt, joel, andrew, hkallweit1, linux, davem, kuba, pabeni,
	p.zabel, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev, BMC-SW

Hi Andrew,

> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: 2022年3月21日 8:11 PM
> To: Dylan Hung <dylan_hung@aspeedtech.com>
> Cc: robh+dt@kernel.org; joel@jms.id.au; andrew@aj.id.au;
> hkallweit1@gmail.com; linux@armlinux.org.uk; davem@davemloft.net;
> kuba@kernel.org; pabeni@redhat.com; p.zabel@pengutronix.de;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; BMC-SW <BMC-SW@aspeedtech.com>
> Subject: Re: [PATCH 0/2] Add reset deassertion for Aspeed MDIO
> 
> On Mon, Mar 21, 2022 at 03:01:29PM +0800, Dylan Hung wrote:
> > Add missing reset deassertion for Aspeed MDIO. There are 4 MDIOs
> > embedded in Aspeed AST2600 and share one reset control bit SCU50[3].
> 
> Is the reset limited to the MDIO bus masters, or are PHYs one the bus
> potentially also reset?

It is limited to the MDIO bus masters.

> 
> Who asserts the reset in the first place? 

The hardware asserts the reset by default.

> Don't you want the first MDIO bus to
> probe to assert and then deassert the reset in order that all the hardware is
> reset?

Do I still need to add a reset assertion/deassertion if the hardware asserts the reset by default?

> 
>     Andrew

--
Dylan

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] Add reset deassertion for Aspeed MDIO
  2022-03-22  9:10     ` Dylan Hung
@ 2022-03-22 12:17       ` Andrew Lunn
  -1 siblings, 0 replies; 16+ messages in thread
From: Andrew Lunn @ 2022-03-22 12:17 UTC (permalink / raw)
  To: Dylan Hung
  Cc: robh+dt, joel, andrew, hkallweit1, linux, davem, kuba, pabeni,
	p.zabel, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev, BMC-SW

> Do I still need to add a reset assertion/deassertion if the hardware
> asserts the reset by default?

One use case would be kexec when one kernel is used to boot another
kernel. But since this is shared by multiple busses, it is probably
not worth the complexity unless that is something you expect you
customers to do, e.g. during kernel development work.

	Andrew

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/2] Add reset deassertion for Aspeed MDIO
@ 2022-03-22 12:17       ` Andrew Lunn
  0 siblings, 0 replies; 16+ messages in thread
From: Andrew Lunn @ 2022-03-22 12:17 UTC (permalink / raw)
  To: Dylan Hung
  Cc: robh+dt, joel, andrew, hkallweit1, linux, davem, kuba, pabeni,
	p.zabel, devicetree, linux-arm-kernel, linux-aspeed,
	linux-kernel, netdev, BMC-SW

> Do I still need to add a reset assertion/deassertion if the hardware
> asserts the reset by default?

One use case would be kexec when one kernel is used to boot another
kernel. But since this is shared by multiple busses, it is probably
not worth the complexity unless that is something you expect you
customers to do, e.g. during kernel development work.

	Andrew

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-03-22 15:47 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-21  7:01 [PATCH 0/2] Add reset deassertion for Aspeed MDIO Dylan Hung
2022-03-21  7:01 ` Dylan Hung
2022-03-21  7:01 ` [PATCH 1/2] net: mdio: add " Dylan Hung
2022-03-21  7:01   ` Dylan Hung
2022-03-21  8:14   ` Philipp Zabel
2022-03-21  8:14     ` Philipp Zabel
2022-03-21  8:20     ` Dylan Hung
2022-03-21  8:20       ` Dylan Hung
2022-03-21  7:01 ` [PATCH 2/2] ARM: dts: aspeed: add reset properties into MDIO nodes Dylan Hung
2022-03-21  7:01   ` Dylan Hung
2022-03-21 12:10 ` [PATCH 0/2] Add reset deassertion for Aspeed MDIO Andrew Lunn
2022-03-21 12:10   ` Andrew Lunn
2022-03-22  9:10   ` Dylan Hung
2022-03-22  9:10     ` Dylan Hung
2022-03-22 12:17     ` Andrew Lunn
2022-03-22 12:17       ` Andrew Lunn

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.