* [PATCH RFT v2 0/2] Teach phylib hard-resetting devices
@ 2016-04-28 22:09 Sergei Shtylyov
2016-04-28 22:15 ` [PATCH RFT v2 2/2] macb: kill PHY reset code Sergei Shtylyov
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Sergei Shtylyov @ 2016-04-28 22:09 UTC (permalink / raw)
To: grant.likely-QSEj5FYQhm4dnm+yROfE0A,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, devicetree-u79uwXL29TY76Z2rM5mHXA,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w, pawel.moll-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
galak-sgV2jX0FEOL9JmXXK+q4OQ
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Hello.
Here's the set of 2 patches against DaveM's 'net-next.git' repo. They add to
the 'phylib' support for resetting devices via GPIO and do some clean up after
doing that...
[1/2] phylib: add device reset GPIO support
[2/2] macb: kill PHY reset code
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH RFT v2 2/2] macb: kill PHY reset code
2016-04-28 22:09 [PATCH RFT v2 0/2] Teach phylib hard-resetting devices Sergei Shtylyov
@ 2016-04-28 22:15 ` Sergei Shtylyov
2016-04-29 9:36 ` Nicolas Ferre
2016-04-30 20:35 ` [PATCH] pxa168_eth: fix mdiobus_scan() error check Sergei Shtylyov
2016-05-07 19:53 ` [PATCH] pxa168_eth: mdiobus_scan() doesn't return NULL anymore Sergei Shtylyov
2 siblings, 1 reply; 10+ messages in thread
From: Sergei Shtylyov @ 2016-04-28 22:15 UTC (permalink / raw)
To: netdev, nicolas.ferre; +Cc: linux-kernel
With the 'phylib' now being aware of the "reset-gpios" PHY node property,
there should be no need to frob the PHY reset in this driver anymore...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
drivers/net/ethernet/cadence/macb.c | 17 -----------------
drivers/net/ethernet/cadence/macb.h | 1 -
2 files changed, 18 deletions(-)
Index: net-next/drivers/net/ethernet/cadence/macb.c
===================================================================
--- net-next.orig/drivers/net/ethernet/cadence/macb.c
+++ net-next/drivers/net/ethernet/cadence/macb.c
@@ -2884,7 +2884,6 @@ static int macb_probe(struct platform_de
= macb_clk_init;
int (*init)(struct platform_device *) = macb_init;
struct device_node *np = pdev->dev.of_node;
- struct device_node *phy_node;
const struct macb_config *macb_config = NULL;
struct clk *pclk, *hclk = NULL, *tx_clk = NULL;
unsigned int queue_mask, num_queues;
@@ -2977,18 +2976,6 @@ static int macb_probe(struct platform_de
else
macb_get_hwaddr(bp);
- /* Power up the PHY if there is a GPIO reset */
- phy_node = of_get_next_available_child(np, NULL);
- if (phy_node) {
- int gpio = of_get_named_gpio(phy_node, "reset-gpios", 0);
-
- if (gpio_is_valid(gpio)) {
- bp->reset_gpio = gpio_to_desc(gpio);
- gpiod_direction_output(bp->reset_gpio, 1);
- }
- }
- of_node_put(phy_node);
-
err = of_get_phy_mode(np);
if (err < 0) {
pdata = dev_get_platdata(&pdev->dev);
@@ -3054,10 +3041,6 @@ static int macb_remove(struct platform_d
mdiobus_unregister(bp->mii_bus);
mdiobus_free(bp->mii_bus);
- /* Shutdown the PHY if there is a GPIO reset */
- if (bp->reset_gpio)
- gpiod_set_value(bp->reset_gpio, 0);
-
unregister_netdev(dev);
clk_disable_unprepare(bp->tx_clk);
clk_disable_unprepare(bp->hclk);
Index: net-next/drivers/net/ethernet/cadence/macb.h
===================================================================
--- net-next.orig/drivers/net/ethernet/cadence/macb.h
+++ net-next/drivers/net/ethernet/cadence/macb.h
@@ -832,7 +832,6 @@ struct macb {
unsigned int dma_burst_length;
phy_interface_t phy_interface;
- struct gpio_desc *reset_gpio;
/* AT91RM9200 transmit */
struct sk_buff *skb; /* holds skb until xmit interrupt completes */
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH RFT v2 2/2] macb: kill PHY reset code
2016-04-28 22:15 ` [PATCH RFT v2 2/2] macb: kill PHY reset code Sergei Shtylyov
@ 2016-04-29 9:36 ` Nicolas Ferre
0 siblings, 0 replies; 10+ messages in thread
From: Nicolas Ferre @ 2016-04-29 9:36 UTC (permalink / raw)
To: Sergei Shtylyov, netdev; +Cc: linux-kernel
Le 29/04/2016 00:15, Sergei Shtylyov a écrit :
> With the 'phylib' now being aware of the "reset-gpios" PHY node property,
> there should be no need to frob the PHY reset in this driver anymore...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
So I queue my DT patch through arm-soc.
Thanks Sergei, bye.
> ---
> drivers/net/ethernet/cadence/macb.c | 17 -----------------
> drivers/net/ethernet/cadence/macb.h | 1 -
> 2 files changed, 18 deletions(-)
>
> Index: net-next/drivers/net/ethernet/cadence/macb.c
> ===================================================================
> --- net-next.orig/drivers/net/ethernet/cadence/macb.c
> +++ net-next/drivers/net/ethernet/cadence/macb.c
> @@ -2884,7 +2884,6 @@ static int macb_probe(struct platform_de
> = macb_clk_init;
> int (*init)(struct platform_device *) = macb_init;
> struct device_node *np = pdev->dev.of_node;
> - struct device_node *phy_node;
> const struct macb_config *macb_config = NULL;
> struct clk *pclk, *hclk = NULL, *tx_clk = NULL;
> unsigned int queue_mask, num_queues;
> @@ -2977,18 +2976,6 @@ static int macb_probe(struct platform_de
> else
> macb_get_hwaddr(bp);
>
> - /* Power up the PHY if there is a GPIO reset */
> - phy_node = of_get_next_available_child(np, NULL);
> - if (phy_node) {
> - int gpio = of_get_named_gpio(phy_node, "reset-gpios", 0);
> -
> - if (gpio_is_valid(gpio)) {
> - bp->reset_gpio = gpio_to_desc(gpio);
> - gpiod_direction_output(bp->reset_gpio, 1);
> - }
> - }
> - of_node_put(phy_node);
> -
> err = of_get_phy_mode(np);
> if (err < 0) {
> pdata = dev_get_platdata(&pdev->dev);
> @@ -3054,10 +3041,6 @@ static int macb_remove(struct platform_d
> mdiobus_unregister(bp->mii_bus);
> mdiobus_free(bp->mii_bus);
>
> - /* Shutdown the PHY if there is a GPIO reset */
> - if (bp->reset_gpio)
> - gpiod_set_value(bp->reset_gpio, 0);
> -
> unregister_netdev(dev);
> clk_disable_unprepare(bp->tx_clk);
> clk_disable_unprepare(bp->hclk);
> Index: net-next/drivers/net/ethernet/cadence/macb.h
> ===================================================================
> --- net-next.orig/drivers/net/ethernet/cadence/macb.h
> +++ net-next/drivers/net/ethernet/cadence/macb.h
> @@ -832,7 +832,6 @@ struct macb {
> unsigned int dma_burst_length;
>
> phy_interface_t phy_interface;
> - struct gpio_desc *reset_gpio;
>
> /* AT91RM9200 transmit */
> struct sk_buff *skb; /* holds skb until xmit interrupt completes */
>
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] pxa168_eth: fix mdiobus_scan() error check
2016-04-28 22:09 [PATCH RFT v2 0/2] Teach phylib hard-resetting devices Sergei Shtylyov
2016-04-28 22:15 ` [PATCH RFT v2 2/2] macb: kill PHY reset code Sergei Shtylyov
@ 2016-04-30 20:35 ` Sergei Shtylyov
2016-05-01 16:05 ` Florian Fainelli
2016-05-03 19:03 ` David Miller
2016-05-07 19:53 ` [PATCH] pxa168_eth: mdiobus_scan() doesn't return NULL anymore Sergei Shtylyov
2 siblings, 2 replies; 10+ messages in thread
From: Sergei Shtylyov @ 2016-04-30 20:35 UTC (permalink / raw)
To: netdev; +Cc: arnd
Since mdiobus_scan() returns either an error code or NULL on error, the
driver should check for both, not only for NULL, otherwise a crash is
imminent...
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
The patch is against DaveM's 'net.git' repo.
drivers/net/ethernet/marvell/pxa168_eth.c | 2 ++
1 file changed, 2 insertions(+)
Index: net/drivers/net/ethernet/marvell/pxa168_eth.c
===================================================================
--- net.orig/drivers/net/ethernet/marvell/pxa168_eth.c
+++ net/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -979,6 +979,8 @@ static int pxa168_init_phy(struct net_de
return 0;
pep->phy = mdiobus_scan(pep->smi_bus, pep->phy_addr);
+ if (IS_ERR(pep->phy))
+ return PTR_ERR(pep->phy);
if (!pep->phy)
return -ENODEV;
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] pxa168_eth: fix mdiobus_scan() error check
2016-04-30 20:35 ` [PATCH] pxa168_eth: fix mdiobus_scan() error check Sergei Shtylyov
@ 2016-05-01 16:05 ` Florian Fainelli
2016-05-01 20:25 ` Sergei Shtylyov
2016-05-03 19:03 ` David Miller
1 sibling, 1 reply; 10+ messages in thread
From: Florian Fainelli @ 2016-05-01 16:05 UTC (permalink / raw)
To: Sergei Shtylyov, netdev; +Cc: arnd
Le 30/04/2016 13:35, Sergei Shtylyov a écrit :
> Since mdiobus_scan() returns either an error code or NULL on error, the
> driver should check for both, not only for NULL, otherwise a crash is
> imminent...
>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> The patch is against DaveM's 'net.git' repo.
>
> drivers/net/ethernet/marvell/pxa168_eth.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> Index: net/drivers/net/ethernet/marvell/pxa168_eth.c
> ===================================================================
> --- net.orig/drivers/net/ethernet/marvell/pxa168_eth.c
> +++ net/drivers/net/ethernet/marvell/pxa168_eth.c
> @@ -979,6 +979,8 @@ static int pxa168_init_phy(struct net_de
> return 0;
>
> pep->phy = mdiobus_scan(pep->smi_bus, pep->phy_addr);
> + if (IS_ERR(pep->phy))
> + return PTR_ERR(pep->phy);
> if (!pep->phy)
> return -ENODEV;
Should not this check be removed too and converted to a
PTR_ERR(pep->phy) != -ENODEV?
--
Florian
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] pxa168_eth: fix mdiobus_scan() error check
2016-05-01 16:05 ` Florian Fainelli
@ 2016-05-01 20:25 ` Sergei Shtylyov
2016-05-01 20:30 ` Sergei Shtylyov
0 siblings, 1 reply; 10+ messages in thread
From: Sergei Shtylyov @ 2016-05-01 20:25 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: arnd
Hello.
On 05/01/2016 07:05 PM, Florian Fainelli wrote:
>> Since mdiobus_scan() returns either an error code or NULL on error, the
>> driver should check for both, not only for NULL, otherwise a crash is
>> imminent...
>>
>> Reported-by: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> The patch is against DaveM's 'net.git' repo.
>>
>> drivers/net/ethernet/marvell/pxa168_eth.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> Index: net/drivers/net/ethernet/marvell/pxa168_eth.c
>> ===================================================================
>> --- net.orig/drivers/net/ethernet/marvell/pxa168_eth.c
>> +++ net/drivers/net/ethernet/marvell/pxa168_eth.c
>> @@ -979,6 +979,8 @@ static int pxa168_init_phy(struct net_de
>> return 0;
>>
>> pep->phy = mdiobus_scan(pep->smi_bus, pep->phy_addr);
>> + if (IS_ERR(pep->phy))
>> + return PTR_ERR(pep->phy);
>> if (!pep->phy)
>> return -ENODEV;
>
> Should not this check be removed too and
That's my next move -- for now I'm fixing the existing bug in this driver
only.
> converted to a PTR_ERR(pep->phy) != -ENODEV?
I don't see what that would achieve, IMO it's enough to just drop this
*if* altogether.
MBR, Sergei
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] pxa168_eth: fix mdiobus_scan() error check
2016-05-01 20:25 ` Sergei Shtylyov
@ 2016-05-01 20:30 ` Sergei Shtylyov
0 siblings, 0 replies; 10+ messages in thread
From: Sergei Shtylyov @ 2016-05-01 20:30 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: arnd
On 05/01/2016 11:25 PM, Sergei Shtylyov wrote:
>>> Since mdiobus_scan() returns either an error code or NULL on error, the
>>> driver should check for both, not only for NULL, otherwise a crash is
>>> imminent...
>>>
>>> Reported-by: Arnd Bergmann <arnd@arndb.de>
>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>
>>> ---
>>> The patch is against DaveM's 'net.git' repo.
>>>
>>> drivers/net/ethernet/marvell/pxa168_eth.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> Index: net/drivers/net/ethernet/marvell/pxa168_eth.c
>>> ===================================================================
>>> --- net.orig/drivers/net/ethernet/marvell/pxa168_eth.c
>>> +++ net/drivers/net/ethernet/marvell/pxa168_eth.c
>>> @@ -979,6 +979,8 @@ static int pxa168_init_phy(struct net_de
>>> return 0;
>>>
>>> pep->phy = mdiobus_scan(pep->smi_bus, pep->phy_addr);
>>> + if (IS_ERR(pep->phy))
>>> + return PTR_ERR(pep->phy);
>>> if (!pep->phy)
>>> return -ENODEV;
>>
>> Should not this check be removed too and
>
> That's my next move -- for now I'm fixing the existing bug in this driver
> only.
In fact, it can't be removed yet as mdiobus_scan() may return NULL on
other error path. There's certainly a space for improvements yet...
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] pxa168_eth: fix mdiobus_scan() error check
2016-04-30 20:35 ` [PATCH] pxa168_eth: fix mdiobus_scan() error check Sergei Shtylyov
2016-05-01 16:05 ` Florian Fainelli
@ 2016-05-03 19:03 ` David Miller
1 sibling, 0 replies; 10+ messages in thread
From: David Miller @ 2016-05-03 19:03 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: netdev, arnd
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Sat, 30 Apr 2016 23:35:11 +0300
> Since mdiobus_scan() returns either an error code or NULL on error, the
> driver should check for both, not only for NULL, otherwise a crash is
> imminent...
>
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Applied.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] pxa168_eth: mdiobus_scan() doesn't return NULL anymore
2016-04-28 22:09 [PATCH RFT v2 0/2] Teach phylib hard-resetting devices Sergei Shtylyov
2016-04-28 22:15 ` [PATCH RFT v2 2/2] macb: kill PHY reset code Sergei Shtylyov
2016-04-30 20:35 ` [PATCH] pxa168_eth: fix mdiobus_scan() error check Sergei Shtylyov
@ 2016-05-07 19:53 ` Sergei Shtylyov
2016-05-09 4:14 ` David Miller
2 siblings, 1 reply; 10+ messages in thread
From: Sergei Shtylyov @ 2016-05-07 19:53 UTC (permalink / raw)
To: netdev
Now that mdiobus_scan() doesn't return NULL on failure anymore, this driver
no longer needs to check for it...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
The patch is against DaveM's 'net-next.git' repo.
drivers/net/ethernet/marvell/pxa168_eth.c | 2 --
1 file changed, 2 deletions(-)
Index: net-next/drivers/net/ethernet/marvell/pxa168_eth.c
===================================================================
--- net-next.orig/drivers/net/ethernet/marvell/pxa168_eth.c
+++ net-next/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -981,8 +981,6 @@ static int pxa168_init_phy(struct net_de
pep->phy = mdiobus_scan(pep->smi_bus, pep->phy_addr);
if (IS_ERR(pep->phy))
return PTR_ERR(pep->phy);
- if (!pep->phy)
- return -ENODEV;
err = phy_connect_direct(dev, pep->phy, pxa168_eth_adjust_link,
pep->phy_intf);
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-05-09 4:14 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-28 22:09 [PATCH RFT v2 0/2] Teach phylib hard-resetting devices Sergei Shtylyov
2016-04-28 22:15 ` [PATCH RFT v2 2/2] macb: kill PHY reset code Sergei Shtylyov
2016-04-29 9:36 ` Nicolas Ferre
2016-04-30 20:35 ` [PATCH] pxa168_eth: fix mdiobus_scan() error check Sergei Shtylyov
2016-05-01 16:05 ` Florian Fainelli
2016-05-01 20:25 ` Sergei Shtylyov
2016-05-01 20:30 ` Sergei Shtylyov
2016-05-03 19:03 ` David Miller
2016-05-07 19:53 ` [PATCH] pxa168_eth: mdiobus_scan() doesn't return NULL anymore Sergei Shtylyov
2016-05-09 4:14 ` David Miller
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).