* [PATCH 1/2] net: fec: Do a sanity check on the gpio number
@ 2013-02-16 22:53 Fabio Estevam
2013-02-16 22:53 ` [PATCH 2/2] net: fec: Improve logging Fabio Estevam
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Fabio Estevam @ 2013-02-16 22:53 UTC (permalink / raw)
To: davem; +Cc: s.hauer, shawn.guo, marex, netdev, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
Since commit 372e722ea4d (gpiolib: use descriptors internally) the following
warning is seen on a mx28evk board:
[ 5.116291] ------------[ cut here ]------------
[ 5.121306] WARNING: at drivers/gpio/gpiolib.c:125 gpio_to_desc+0x30/0x44()
[ 5.128491] invalid GPIO -2
[ 5.131563] Modules linked in:
[ 5.134846] [<c0014e20>] (unwind_backtrace+0x0/0xf4) from [<c001d428>] (warn_slowpath_common+0x4c/0x68)
[ 5.144682] [<c001d428>] (warn_slowpath_common+0x4c/0x68) from [<c001d4d8>] (warn_slowpath_fmt+0x30/0x40)
[ 5.154693] [<c001d4d8>] (warn_slowpath_fmt+0x30/0x40) from [<c0283434>] (gpio_to_desc+0x30/0x44)
[ 5.164002] [<c0283434>] (gpio_to_desc+0x30/0x44) from [<c0285470>] (gpio_request_one+0x10/0xe8)
[ 5.173294] [<c0285470>] (gpio_request_one+0x10/0xe8) from [<c0282f50>] (devm_gpio_request_one+0x40/0x74)
[ 5.183332] [<c0282f50>] (devm_gpio_request_one+0x40/0x74) from [<c0319be0>] (fec_probe+0x2d0/0x99c)
[ 5.192923] [<c0319be0>] (fec_probe+0x2d0/0x99c) from [<c02c8114>] (platform_drv_probe+0x14/0x18)
[ 5.202228] [<c02c8114>] (platform_drv_probe+0x14/0x18) from [<c02c6e00>] (driver_probe_device+0x90/0x224)
[ 5.212332] [<c02c6e00>] (driver_probe_device+0x90/0x224) from [<c02c7028>] (__driver_attach+0x94/0x98)
[ 5.222162] [<c02c7028>] (__driver_attach+0x94/0x98) from [<c02c5750>] (bus_for_each_dev+0x78/0x98)
[ 5.231642] [<c02c5750>] (bus_for_each_dev+0x78/0x98) from [<c02c5fd0>] (bus_add_driver+0x1a4/0x240)
[ 5.241207] [<c02c5fd0>] (bus_add_driver+0x1a4/0x240) from [<c02c7608>] (driver_register+0x78/0x140)
[ 5.250768] [<c02c7608>] (driver_register+0x78/0x140) from [<c00087a4>] (do_one_initcall+0x30/0x17c)
[ 5.260347] [<c00087a4>] (do_one_initcall+0x30/0x17c) from [<c05fa29c>] (kernel_init_freeable+0xe8/0x1b0)
[ 5.270381] [<c05fa29c>] (kernel_init_freeable+0xe8/0x1b0) from [<c044dfd4>] (kernel_init+0x8/0xe4)
[ 5.279886] [<c044dfd4>] (kernel_init+0x8/0xe4) from [<c000f248>] (ret_from_fork+0x14/0x2c)
[ 5.288740] ---[ end trace c15c72a22979d58d ]---
mx28evk has two ethernet controllers. The GPIO that performs the
ethernet reset on both ports is the same GPIO, so on the board dts file, only in
one ethernet instance is passed the GPIO reset property.
Validate the gpio number prior to requesting it in order to avoid such warning.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 0fe68c4..5864a67 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -1689,6 +1689,9 @@ static void fec_reset_phy(struct platform_device *pdev)
msec = 1;
phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
+ if (!gpio_is_valid(phy_reset))
+ return;
+
err = devm_gpio_request_one(&pdev->dev, phy_reset,
GPIOF_OUT_INIT_LOW, "phy-reset");
if (err) {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/2] net: fec: Improve logging
2013-02-16 22:53 [PATCH 1/2] net: fec: Do a sanity check on the gpio number Fabio Estevam
@ 2013-02-16 22:53 ` Fabio Estevam
2013-02-17 10:33 ` Marek Vasut
2013-02-17 11:55 ` Shawn Guo
2013-02-17 5:32 ` [PATCH 1/2] net: fec: Do a sanity check on the gpio number Shawn Guo
2013-02-17 10:32 ` Marek Vasut
2 siblings, 2 replies; 8+ messages in thread
From: Fabio Estevam @ 2013-02-16 22:53 UTC (permalink / raw)
To: davem; +Cc: s.hauer, shawn.guo, marex, netdev, Fabio Estevam
From: Fabio Estevam <fabio.estevam@freescale.com>
In case the request of the GPIO reset fails it is interesting to log
such error even if DEBUG is not enabled, so promote the message to dev_err.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/net/ethernet/freescale/fec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 5864a67..29d82cf 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -1695,7 +1695,7 @@ static void fec_reset_phy(struct platform_device *pdev)
err = devm_gpio_request_one(&pdev->dev, phy_reset,
GPIOF_OUT_INIT_LOW, "phy-reset");
if (err) {
- pr_debug("FEC: failed to get gpio phy-reset: %d\n", err);
+ dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err);
return;
}
msleep(msec);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/2] net: fec: Improve logging
2013-02-16 22:53 ` [PATCH 2/2] net: fec: Improve logging Fabio Estevam
@ 2013-02-17 10:33 ` Marek Vasut
2013-02-17 11:55 ` Shawn Guo
1 sibling, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2013-02-17 10:33 UTC (permalink / raw)
To: Fabio Estevam; +Cc: davem, s.hauer, shawn.guo, netdev, Fabio Estevam
Dear Fabio Estevam,
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> In case the request of the GPIO reset fails it is interesting to log
> such error even if DEBUG is not enabled, so promote the message to dev_err.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Acked-by: Marek Vasut <marex@denx.de>
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] net: fec: Improve logging
2013-02-16 22:53 ` [PATCH 2/2] net: fec: Improve logging Fabio Estevam
2013-02-17 10:33 ` Marek Vasut
@ 2013-02-17 11:55 ` Shawn Guo
2013-02-17 13:41 ` Fabio Estevam
1 sibling, 1 reply; 8+ messages in thread
From: Shawn Guo @ 2013-02-17 11:55 UTC (permalink / raw)
To: Fabio Estevam; +Cc: davem, s.hauer, marex, netdev, Fabio Estevam
On Sat, Feb 16, 2013 at 08:53:27PM -0200, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> In case the request of the GPIO reset fails it is interesting to log
> such error even if DEBUG is not enabled, so promote the message to dev_err.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> drivers/net/ethernet/freescale/fec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
> index 5864a67..29d82cf 100644
> --- a/drivers/net/ethernet/freescale/fec.c
> +++ b/drivers/net/ethernet/freescale/fec.c
> @@ -1695,7 +1695,7 @@ static void fec_reset_phy(struct platform_device *pdev)
> err = devm_gpio_request_one(&pdev->dev, phy_reset,
> GPIOF_OUT_INIT_LOW, "phy-reset");
> if (err) {
> - pr_debug("FEC: failed to get gpio phy-reset: %d\n", err);
> + dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err);
Shouldn't dev_dbg be more like a equivalent of pr_debug? The reason
why it's taken as a debug message rather than an error is that some
board design may not have reset line for phy.
Shawn
> return;
> }
> msleep(msec);
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/2] net: fec: Improve logging
2013-02-17 11:55 ` Shawn Guo
@ 2013-02-17 13:41 ` Fabio Estevam
2013-02-17 15:08 ` Shawn Guo
0 siblings, 1 reply; 8+ messages in thread
From: Fabio Estevam @ 2013-02-17 13:41 UTC (permalink / raw)
To: Shawn Guo; +Cc: davem, s.hauer, marex, netdev, Fabio Estevam
On Sun, Feb 17, 2013 at 8:55 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>> --- a/drivers/net/ethernet/freescale/fec.c
>> +++ b/drivers/net/ethernet/freescale/fec.c
>> @@ -1695,7 +1695,7 @@ static void fec_reset_phy(struct platform_device *pdev)
>> err = devm_gpio_request_one(&pdev->dev, phy_reset,
>> GPIOF_OUT_INIT_LOW, "phy-reset");
>> if (err) {
>> - pr_debug("FEC: failed to get gpio phy-reset: %d\n", err);
>> + dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err);
>
> Shouldn't dev_dbg be more like a equivalent of pr_debug? The reason
> why it's taken as a debug message rather than an error is that some
> board design may not have reset line for phy.
If a board does not have a reset line for the PHY then it will not try
to request the GPIO as per patch 1/2 of this series.
I sent a v2 with dev_dbg, but I think that the first version with
dev_err is more useful.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/2] net: fec: Improve logging
2013-02-17 13:41 ` Fabio Estevam
@ 2013-02-17 15:08 ` Shawn Guo
0 siblings, 0 replies; 8+ messages in thread
From: Shawn Guo @ 2013-02-17 15:08 UTC (permalink / raw)
To: Fabio Estevam; +Cc: davem, s.hauer, marex, netdev, Fabio Estevam
On Sun, Feb 17, 2013 at 10:41:11AM -0300, Fabio Estevam wrote:
> On Sun, Feb 17, 2013 at 8:55 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
>
> >> --- a/drivers/net/ethernet/freescale/fec.c
> >> +++ b/drivers/net/ethernet/freescale/fec.c
> >> @@ -1695,7 +1695,7 @@ static void fec_reset_phy(struct platform_device *pdev)
> >> err = devm_gpio_request_one(&pdev->dev, phy_reset,
> >> GPIOF_OUT_INIT_LOW, "phy-reset");
> >> if (err) {
> >> - pr_debug("FEC: failed to get gpio phy-reset: %d\n", err);
> >> + dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err);
> >
> > Shouldn't dev_dbg be more like a equivalent of pr_debug? The reason
> > why it's taken as a debug message rather than an error is that some
> > board design may not have reset line for phy.
>
> If a board does not have a reset line for the PHY then it will not try
> to request the GPIO as per patch 1/2 of this series.
Argh, yes, you're right. The pr_debug makes no sense any more with the
first patch in place, and it should become a error message. But they
should be one patch rather than a series to make the most sense, IMO.
Shawn
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] net: fec: Do a sanity check on the gpio number
2013-02-16 22:53 [PATCH 1/2] net: fec: Do a sanity check on the gpio number Fabio Estevam
2013-02-16 22:53 ` [PATCH 2/2] net: fec: Improve logging Fabio Estevam
@ 2013-02-17 5:32 ` Shawn Guo
2013-02-17 10:32 ` Marek Vasut
2 siblings, 0 replies; 8+ messages in thread
From: Shawn Guo @ 2013-02-17 5:32 UTC (permalink / raw)
To: Fabio Estevam; +Cc: davem, s.hauer, marex, netdev, Fabio Estevam
On Sat, Feb 16, 2013 at 08:53:26PM -0200, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Since commit 372e722ea4d (gpiolib: use descriptors internally) the following
> warning is seen on a mx28evk board:
>
> [ 5.116291] ------------[ cut here ]------------
> [ 5.121306] WARNING: at drivers/gpio/gpiolib.c:125 gpio_to_desc+0x30/0x44()
> [ 5.128491] invalid GPIO -2
> [ 5.131563] Modules linked in:
> [ 5.134846] [<c0014e20>] (unwind_backtrace+0x0/0xf4) from [<c001d428>] (warn_slowpath_common+0x4c/0x68)
> [ 5.144682] [<c001d428>] (warn_slowpath_common+0x4c/0x68) from [<c001d4d8>] (warn_slowpath_fmt+0x30/0x40)
> [ 5.154693] [<c001d4d8>] (warn_slowpath_fmt+0x30/0x40) from [<c0283434>] (gpio_to_desc+0x30/0x44)
> [ 5.164002] [<c0283434>] (gpio_to_desc+0x30/0x44) from [<c0285470>] (gpio_request_one+0x10/0xe8)
> [ 5.173294] [<c0285470>] (gpio_request_one+0x10/0xe8) from [<c0282f50>] (devm_gpio_request_one+0x40/0x74)
> [ 5.183332] [<c0282f50>] (devm_gpio_request_one+0x40/0x74) from [<c0319be0>] (fec_probe+0x2d0/0x99c)
> [ 5.192923] [<c0319be0>] (fec_probe+0x2d0/0x99c) from [<c02c8114>] (platform_drv_probe+0x14/0x18)
> [ 5.202228] [<c02c8114>] (platform_drv_probe+0x14/0x18) from [<c02c6e00>] (driver_probe_device+0x90/0x224)
> [ 5.212332] [<c02c6e00>] (driver_probe_device+0x90/0x224) from [<c02c7028>] (__driver_attach+0x94/0x98)
> [ 5.222162] [<c02c7028>] (__driver_attach+0x94/0x98) from [<c02c5750>] (bus_for_each_dev+0x78/0x98)
> [ 5.231642] [<c02c5750>] (bus_for_each_dev+0x78/0x98) from [<c02c5fd0>] (bus_add_driver+0x1a4/0x240)
> [ 5.241207] [<c02c5fd0>] (bus_add_driver+0x1a4/0x240) from [<c02c7608>] (driver_register+0x78/0x140)
> [ 5.250768] [<c02c7608>] (driver_register+0x78/0x140) from [<c00087a4>] (do_one_initcall+0x30/0x17c)
> [ 5.260347] [<c00087a4>] (do_one_initcall+0x30/0x17c) from [<c05fa29c>] (kernel_init_freeable+0xe8/0x1b0)
> [ 5.270381] [<c05fa29c>] (kernel_init_freeable+0xe8/0x1b0) from [<c044dfd4>] (kernel_init+0x8/0xe4)
> [ 5.279886] [<c044dfd4>] (kernel_init+0x8/0xe4) from [<c000f248>] (ret_from_fork+0x14/0x2c)
> [ 5.288740] ---[ end trace c15c72a22979d58d ]---
>
> mx28evk has two ethernet controllers. The GPIO that performs the
> ethernet reset on both ports is the same GPIO, so on the board dts file, only in
> one ethernet instance is passed the GPIO reset property.
>
> Validate the gpio number prior to requesting it in order to avoid such warning.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Both
Acked-by: Shawn Guo <shawn.guo@linaro.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] net: fec: Do a sanity check on the gpio number
2013-02-16 22:53 [PATCH 1/2] net: fec: Do a sanity check on the gpio number Fabio Estevam
2013-02-16 22:53 ` [PATCH 2/2] net: fec: Improve logging Fabio Estevam
2013-02-17 5:32 ` [PATCH 1/2] net: fec: Do a sanity check on the gpio number Shawn Guo
@ 2013-02-17 10:32 ` Marek Vasut
2 siblings, 0 replies; 8+ messages in thread
From: Marek Vasut @ 2013-02-17 10:32 UTC (permalink / raw)
To: Fabio Estevam; +Cc: davem, s.hauer, shawn.guo, netdev, Fabio Estevam
Dear Fabio Estevam,
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Since commit 372e722ea4d (gpiolib: use descriptors internally) the
> following warning is seen on a mx28evk board:
[...]
I wonder if some dev_dbg() won't be a good idea here. Otherwise:
Acked-by: Marek Vasut <marex@denx.de>
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-02-17 15:08 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-16 22:53 [PATCH 1/2] net: fec: Do a sanity check on the gpio number Fabio Estevam
2013-02-16 22:53 ` [PATCH 2/2] net: fec: Improve logging Fabio Estevam
2013-02-17 10:33 ` Marek Vasut
2013-02-17 11:55 ` Shawn Guo
2013-02-17 13:41 ` Fabio Estevam
2013-02-17 15:08 ` Shawn Guo
2013-02-17 5:32 ` [PATCH 1/2] net: fec: Do a sanity check on the gpio number Shawn Guo
2013-02-17 10:32 ` Marek Vasut
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).