linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fec: convert to gpio descriptor
@ 2023-01-26 13:52 Arnd Bergmann
  2023-01-26 18:02 ` Andrew Lunn
  0 siblings, 1 reply; 3+ messages in thread
From: Arnd Bergmann @ 2023-01-26 13:52 UTC (permalink / raw)
  To: Wei Fang
  Cc: linux-gpio, Arnd Bergmann, Shenwei Wang, Clark Wang,
	NXP Linux Team, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Andrew Lunn, netdev, linux-kernel

From: Arnd Bergmann <arnd@arndb.de>

The driver can be trivially converted, as it only triggers the gpio
pin briefly to do a reset, and it already only supports DT.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/freescale/fec_main.c | 25 ++++++++++-------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 5ff45b1a74a5..dee2890fd702 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -56,7 +56,7 @@
 #include <linux/fec.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
-#include <linux/of_gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
 #include <linux/regulator/consumer.h>
@@ -4035,7 +4035,8 @@ static int fec_enet_init(struct net_device *ndev)
 #ifdef CONFIG_OF
 static int fec_reset_phy(struct platform_device *pdev)
 {
-	int err, phy_reset;
+	int err;
+	struct gpio_desc *phy_reset;
 	bool active_high = false;
 	int msec = 1, phy_post_delay = 0;
 	struct device_node *np = pdev->dev.of_node;
@@ -4048,12 +4049,6 @@ static int fec_reset_phy(struct platform_device *pdev)
 	if (!err && msec > 1000)
 		msec = 1;
 
-	phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
-	if (phy_reset == -EPROBE_DEFER)
-		return phy_reset;
-	else if (!gpio_is_valid(phy_reset))
-		return 0;
-
 	err = of_property_read_u32(np, "phy-reset-post-delay", &phy_post_delay);
 	/* valid reset duration should be less than 1s */
 	if (!err && phy_post_delay > 1000)
@@ -4061,11 +4056,13 @@ static int fec_reset_phy(struct platform_device *pdev)
 
 	active_high = of_property_read_bool(np, "phy-reset-active-high");
 
-	err = devm_gpio_request_one(&pdev->dev, phy_reset,
-			active_high ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
-			"phy-reset");
-	if (err) {
-		dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err);
+	phy_reset = devm_gpiod_get(&pdev->dev, "phy-reset",
+			active_high ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
+	if (IS_ERR(phy_reset)) {
+		err = PTR_ERR(phy_reset);
+		if (err != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
+				"failed to get phy-reset-gpios: %d\n", err);
 		return err;
 	}
 
@@ -4074,7 +4071,7 @@ static int fec_reset_phy(struct platform_device *pdev)
 	else
 		usleep_range(msec * 1000, msec * 1000 + 1000);
 
-	gpio_set_value_cansleep(phy_reset, !active_high);
+	gpiod_set_value_cansleep(phy_reset, !active_high);
 
 	if (!phy_post_delay)
 		return 0;
-- 
2.39.0


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

* Re: [PATCH] fec: convert to gpio descriptor
  2023-01-26 13:52 [PATCH] fec: convert to gpio descriptor Arnd Bergmann
@ 2023-01-26 18:02 ` Andrew Lunn
  2023-01-26 20:32   ` Arnd Bergmann
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Lunn @ 2023-01-26 18:02 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Wei Fang, linux-gpio, Arnd Bergmann, Shenwei Wang, Clark Wang,
	NXP Linux Team, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel

On Thu, Jan 26, 2023 at 02:52:58PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The driver can be trivially converted, as it only triggers the gpio
> pin briefly to do a reset, and it already only supports DT.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/ethernet/freescale/fec_main.c | 25 ++++++++++-------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index 5ff45b1a74a5..dee2890fd702 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -56,7 +56,7 @@
>  #include <linux/fec.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> -#include <linux/of_gpio.h>
> +#include <linux/gpio/consumer.h>
>  #include <linux/of_mdio.h>
>  #include <linux/of_net.h>
>  #include <linux/regulator/consumer.h>
> @@ -4035,7 +4035,8 @@ static int fec_enet_init(struct net_device *ndev)
>  #ifdef CONFIG_OF
>  static int fec_reset_phy(struct platform_device *pdev)
>  {
> -	int err, phy_reset;
> +	int err;
> +	struct gpio_desc *phy_reset;
>  	bool active_high = false;
>  	int msec = 1, phy_post_delay = 0;
>  	struct device_node *np = pdev->dev.of_node;

Hi Arnd

netdev drivers are supposed to use 'reverse Christmas tree'. It looks
like this function is actually using 'Christmas tree' :-) Please could
you keep with the current coding style.

> @@ -4048,12 +4049,6 @@ static int fec_reset_phy(struct platform_device *pdev)
>  	if (!err && msec > 1000)
>  		msec = 1;
>  
> -	phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
> -	if (phy_reset == -EPROBE_DEFER)
> -		return phy_reset;
> -	else if (!gpio_is_valid(phy_reset))
> -		return 0;
> -
>  	err = of_property_read_u32(np, "phy-reset-post-delay", &phy_post_delay);
>  	/* valid reset duration should be less than 1s */
>  	if (!err && phy_post_delay > 1000)
> @@ -4061,11 +4056,13 @@ static int fec_reset_phy(struct platform_device *pdev)
>  
>  	active_high = of_property_read_bool(np, "phy-reset-active-high");
>  
> -	err = devm_gpio_request_one(&pdev->dev, phy_reset,
> -			active_high ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
> -			"phy-reset");
> -	if (err) {
> -		dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err);
> +	phy_reset = devm_gpiod_get(&pdev->dev, "phy-reset",
> +			active_high ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
> +	if (IS_ERR(phy_reset)) {
> +		err = PTR_ERR(phy_reset);
> +		if (err != -EPROBE_DEFER)
> +			dev_err(&pdev->dev,
> +				"failed to get phy-reset-gpios: %d\n", err);

dev_err_probe() looks usable here.

		Andrew

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

* Re: [PATCH] fec: convert to gpio descriptor
  2023-01-26 18:02 ` Andrew Lunn
@ 2023-01-26 20:32   ` Arnd Bergmann
  0 siblings, 0 replies; 3+ messages in thread
From: Arnd Bergmann @ 2023-01-26 20:32 UTC (permalink / raw)
  To: Andrew Lunn, Arnd Bergmann
  Cc: Wei Fang, open list:GPIO SUBSYSTEM, Shenwei Wang, Clark Wang,
	NXP Linux Team, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Netdev, linux-kernel

On Thu, Jan 26, 2023, at 19:02, Andrew Lunn wrote:
> On Thu, Jan 26, 2023 at 02:52:58PM +0100, Arnd Bergmann wrote:
>>  static int fec_reset_phy(struct platform_device *pdev)
>>  {
>> -	int err, phy_reset;
>> +	int err;
>> +	struct gpio_desc *phy_reset;
>>  	bool active_high = false;
>>  	int msec = 1, phy_post_delay = 0;
>>  	struct device_node *np = pdev->dev.of_node;
>
> Hi Arnd
>
> netdev drivers are supposed to use 'reverse Christmas tree'. It looks
> like this function is actually using 'Christmas tree' :-) Please could
> you keep with the current coding style.

My feeling is that the style in this file is just random,
but having 'err' as the last one fits with the usual style
and with what some of the other functions do, so I'll do that.

>> +			active_high ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
>> +	if (IS_ERR(phy_reset)) {
>> +		err = PTR_ERR(phy_reset);
>> +		if (err != -EPROBE_DEFER)
>> +			dev_err(&pdev->dev,
>> +				"failed to get phy-reset-gpios: %d\n", err);
>
> dev_err_probe() looks usable here.

Ah nice, I've never been able to use that one so far.

Will send a v2 with both suggestions.

    ARnd

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

end of thread, other threads:[~2023-01-26 20:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-26 13:52 [PATCH] fec: convert to gpio descriptor Arnd Bergmann
2023-01-26 18:02 ` Andrew Lunn
2023-01-26 20:32   ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).