* [PATCH 0/2] net: stmmac: phy logging fixes
@ 2017-08-21 7:52 Romain Perier
2017-08-21 7:52 ` [PATCH 1/2] net: stmmac: Delete dead code for MDIO registration Romain Perier
2017-08-21 7:52 ` [PATCH 2/2] net: phy: Don't use drv when it is NULL in phy_attached_print Romain Perier
0 siblings, 2 replies; 6+ messages in thread
From: Romain Perier @ 2017-08-21 7:52 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Andrew Lunn, Florian Fainelli
Cc: netdev, linux-kernel, Romain Perier
This set of patches fixes issues related to logging. First it delete old
code that is no longer used in stmmac_mdio_register(). Then, it fixes a
crash that happens when phydev->drv is NULL and phy_attached_info() is
called prior phy_driver is binded to phydev.
Romain Perier (2):
net: stmmac: Delete dead code for MDIO registration
net: phy: Don't use drv when it is NULL in phy_attached_info
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 16 ----------------
drivers/net/phy/phy_device.c | 13 +++++++++++--
2 files changed, 11 insertions(+), 18 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] net: stmmac: Delete dead code for MDIO registration
2017-08-21 7:52 [PATCH 0/2] net: stmmac: phy logging fixes Romain Perier
@ 2017-08-21 7:52 ` Romain Perier
2017-08-21 7:52 ` [PATCH 2/2] net: phy: Don't use drv when it is NULL in phy_attached_print Romain Perier
1 sibling, 0 replies; 6+ messages in thread
From: Romain Perier @ 2017-08-21 7:52 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Andrew Lunn, Florian Fainelli
Cc: netdev, linux-kernel, Romain Perier
This code is no longer used, the logging function was changed by commit
fbca164776e4 ("net: stmmac: Use the right logging functi").
Fixes: commit fbca164776e4 ("net: stmmac: Use the right logging functi")
Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
index 72ec711fcba2..f5f37bfa1d58 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
@@ -248,9 +248,6 @@ int stmmac_mdio_register(struct net_device *ndev)
found = 0;
for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
struct phy_device *phydev = mdiobus_get_phy(new_bus, addr);
- int act = 0;
- char irq_num[4];
- char *irq_str;
if (!phydev)
continue;
@@ -273,19 +270,6 @@ int stmmac_mdio_register(struct net_device *ndev)
if (priv->plat->phy_addr == -1)
priv->plat->phy_addr = addr;
- act = (priv->plat->phy_addr == addr);
- switch (phydev->irq) {
- case PHY_POLL:
- irq_str = "POLL";
- break;
- case PHY_IGNORE_INTERRUPT:
- irq_str = "IGNORE";
- break;
- default:
- sprintf(irq_num, "%d", phydev->irq);
- irq_str = irq_num;
- break;
- }
phy_attached_info(phydev);
found = 1;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] net: phy: Don't use drv when it is NULL in phy_attached_print
2017-08-21 7:52 [PATCH 0/2] net: stmmac: phy logging fixes Romain Perier
2017-08-21 7:52 ` [PATCH 1/2] net: stmmac: Delete dead code for MDIO registration Romain Perier
@ 2017-08-21 7:52 ` Romain Perier
2017-08-21 9:45 ` Sergei Shtylyov
2017-08-21 13:16 ` Andrew Lunn
1 sibling, 2 replies; 6+ messages in thread
From: Romain Perier @ 2017-08-21 7:52 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue, Andrew Lunn, Florian Fainelli
Cc: netdev, linux-kernel, Romain Perier
Currently, if this logging function is used prior the phy driver is
binded to the phy device (that is usually done from .ndo_open),
'phydev->drv' might be NULL, resulting in a kernel crash. That is
typically the case in the stmmac driver, info about the phy is displayed
during the registration of the MDIO bus, and then genphy driver is binded
to this phydev when .ndo_open is called.
This commit fixes the issue by using the right genphy driver, when
phydev->drv is NULL.
Fixes: commit fbca164776e4 ("net: stmmac: Use the right logging functi")
Signed-off-by: Romain Perier <romain.perier@collabora.com>
---
drivers/net/phy/phy_device.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9493fb369682..b38926bc275f 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -877,15 +877,24 @@ EXPORT_SYMBOL(phy_attached_info);
#define ATTACHED_FMT "attached PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)"
void phy_attached_print(struct phy_device *phydev, const char *fmt, ...)
{
+ struct phy_driver *drv = phydev->drv;
+
+ if (!drv) {
+ if (phydev->is_c45)
+ drv = &genphy_10g_driver;
+ else
+ drv = &genphy_driver;
+ }
+
if (!fmt) {
dev_info(&phydev->mdio.dev, ATTACHED_FMT "\n",
- phydev->drv->name, phydev_name(phydev),
+ drv->name, phydev_name(phydev),
phydev->irq);
} else {
va_list ap;
dev_info(&phydev->mdio.dev, ATTACHED_FMT,
- phydev->drv->name, phydev_name(phydev),
+ drv->name, phydev_name(phydev),
phydev->irq);
va_start(ap, fmt);
--
2.11.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] net: phy: Don't use drv when it is NULL in phy_attached_print
2017-08-21 7:52 ` [PATCH 2/2] net: phy: Don't use drv when it is NULL in phy_attached_print Romain Perier
@ 2017-08-21 9:45 ` Sergei Shtylyov
2017-08-21 11:46 ` Romain Perier
2017-08-21 13:16 ` Andrew Lunn
1 sibling, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2017-08-21 9:45 UTC (permalink / raw)
To: Romain Perier, Giuseppe Cavallaro, Alexandre Torgue, Andrew Lunn,
Florian Fainelli
Cc: netdev, linux-kernel
Hello!
On 8/21/2017 10:52 AM, Romain Perier wrote:
> Currently, if this logging function is used prior the phy driver is
> binded to the phy device (that is usually done from .ndo_open),
s/binded/bound/.
> 'phydev->drv' might be NULL, resulting in a kernel crash. That is
> typically the case in the stmmac driver, info about the phy is displayed
> during the registration of the MDIO bus, and then genphy driver is binded
Likewise.
> to this phydev when .ndo_open is called.
>
> This commit fixes the issue by using the right genphy driver, when
> phydev->drv is NULL.
>
> Fixes: commit fbca164776e4 ("net: stmmac: Use the right logging functi")
"Commit" not needed here.
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] net: phy: Don't use drv when it is NULL in phy_attached_print
2017-08-21 9:45 ` Sergei Shtylyov
@ 2017-08-21 11:46 ` Romain Perier
0 siblings, 0 replies; 6+ messages in thread
From: Romain Perier @ 2017-08-21 11:46 UTC (permalink / raw)
To: Sergei Shtylyov, Giuseppe Cavallaro, Alexandre Torgue,
Andrew Lunn, Florian Fainelli
Cc: netdev, linux-kernel
Hello,
Le 21/08/2017 à 11:45, Sergei Shtylyov a écrit :
> Hello!
>
> On 8/21/2017 10:52 AM, Romain Perier wrote:
>
>> Currently, if this logging function is used prior the phy driver is
>> binded to the phy device (that is usually done from .ndo_open),
>
> s/binded/bound/.
>
>> 'phydev->drv' might be NULL, resulting in a kernel crash. That is
>> typically the case in the stmmac driver, info about the phy is displayed
>> during the registration of the MDIO bus, and then genphy driver is
>> binded
>
> Likewise.
>
>> to this phydev when .ndo_open is called.
>>
>> This commit fixes the issue by using the right genphy driver, when
>> phydev->drv is NULL.
>>
>> Fixes: commit fbca164776e4 ("net: stmmac: Use the right logging functi")
>
> "Commit" not needed here.
Fixed, thanks
Romain
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] net: phy: Don't use drv when it is NULL in phy_attached_print
2017-08-21 7:52 ` [PATCH 2/2] net: phy: Don't use drv when it is NULL in phy_attached_print Romain Perier
2017-08-21 9:45 ` Sergei Shtylyov
@ 2017-08-21 13:16 ` Andrew Lunn
1 sibling, 0 replies; 6+ messages in thread
From: Andrew Lunn @ 2017-08-21 13:16 UTC (permalink / raw)
To: Romain Perier
Cc: Giuseppe Cavallaro, Alexandre Torgue, Florian Fainelli, netdev,
linux-kernel
On Mon, Aug 21, 2017 at 09:52:35AM +0200, Romain Perier wrote:
> Currently, if this logging function is used prior the phy driver is
> binded to the phy device (that is usually done from .ndo_open),
> 'phydev->drv' might be NULL, resulting in a kernel crash. That is
> typically the case in the stmmac driver, info about the phy is displayed
> during the registration of the MDIO bus, and then genphy driver is binded
> to this phydev when .ndo_open is called.
>
> This commit fixes the issue by using the right genphy driver, when
> phydev->drv is NULL.
>
> Fixes: commit fbca164776e4 ("net: stmmac: Use the right logging functi")
> Signed-off-by: Romain Perier <romain.perier@collabora.com>
> ---
> drivers/net/phy/phy_device.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 9493fb369682..b38926bc275f 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -877,15 +877,24 @@ EXPORT_SYMBOL(phy_attached_info);
> #define ATTACHED_FMT "attached PHY driver [%s] (mii_bus:phy_addr=%s, irq=%d)"
> void phy_attached_print(struct phy_device *phydev, const char *fmt, ...)
> {
> + struct phy_driver *drv = phydev->drv;
> +
> + if (!drv) {
> + if (phydev->is_c45)
> + drv = &genphy_10g_driver;
> + else
> + drv = &genphy_driver;
> + }
Hi Romain
I don't like this. You end up with the same code twice. c45 does not
imply 10g, so i would not be surprised if sometime in the future this
changes. And then we have two places we need to make the same change.
I also wonder what happens if you load the PHY driver later, but
before it is bound. Will it then use the correct driver?
> +
> if (!fmt) {
> dev_info(&phydev->mdio.dev, ATTACHED_FMT "\n",
> - phydev->drv->name, phydev_name(phydev),
> + drv->name, phydev_name(phydev),
I would prefer (phydev->drv ? phydev->drv->name, "unknown")
Andrew
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-21 13:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-21 7:52 [PATCH 0/2] net: stmmac: phy logging fixes Romain Perier
2017-08-21 7:52 ` [PATCH 1/2] net: stmmac: Delete dead code for MDIO registration Romain Perier
2017-08-21 7:52 ` [PATCH 2/2] net: phy: Don't use drv when it is NULL in phy_attached_print Romain Perier
2017-08-21 9:45 ` Sergei Shtylyov
2017-08-21 11:46 ` Romain Perier
2017-08-21 13:16 ` 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.