* [PATCH] net: dsa: qca8k: fix kernel panic with legacy mdio mapping
@ 2021-09-11 15:07 Ansuel Smith
2021-09-11 15:31 ` Florian Fainelli
0 siblings, 1 reply; 5+ messages in thread
From: Ansuel Smith @ 2021-09-11 15:07 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, Vladimir Oltean,
David S. Miller, Jakub Kicinski, Ansuel Smith, netdev,
linux-kernel
When the mdio legacy mapping is used the mii_bus priv registred by DSA
refer to the dsa switch struct instead of the qca8k_priv struct and
cause a kernel panic. Create dedicated function when the internal
dedicated mdio driver is used to proprely handle the 2 different
implementation.
Fixes: 759bafb8a322 ("net: dsa: qca8k: add support for internal phy and internal mdio")
Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
drivers/net/dsa/qca8k.c | 30 ++++++++++++++++++++++--------
1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index 1f63f50f73f1..a701323daf72 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -643,10 +643,8 @@ qca8k_mdio_busy_wait(struct mii_bus *bus, u32 reg, u32 mask)
}
static int
-qca8k_mdio_write(struct mii_bus *salve_bus, int phy, int regnum, u16 data)
+qca8k_mdio_write(struct mii_bus *bus, int phy, int regnum, u16 data)
{
- struct qca8k_priv *priv = salve_bus->priv;
- struct mii_bus *bus = priv->bus;
u16 r1, r2, page;
u32 val;
int ret;
@@ -682,10 +680,8 @@ qca8k_mdio_write(struct mii_bus *salve_bus, int phy, int regnum, u16 data)
}
static int
-qca8k_mdio_read(struct mii_bus *salve_bus, int phy, int regnum)
+qca8k_mdio_read(struct mii_bus *bus, int phy, int regnum)
{
- struct qca8k_priv *priv = salve_bus->priv;
- struct mii_bus *bus = priv->bus;
u16 r1, r2, page;
u32 val;
int ret;
@@ -726,6 +722,24 @@ qca8k_mdio_read(struct mii_bus *salve_bus, int phy, int regnum)
return ret;
}
+static int
+qca8k_internal_mdio_write(struct mii_bus *salve_bus, int phy, int regnum, u16 data)
+{
+ struct qca8k_priv *priv = salve_bus->priv;
+ struct mii_bus *bus = priv->bus;
+
+ return qca8k_mdio_write(bus, phy, regnum, data);
+}
+
+static int
+qca8k_internal_mdio_read(struct mii_bus *salve_bus, int phy, int regnum)
+{
+ struct qca8k_priv *priv = salve_bus->priv;
+ struct mii_bus *bus = priv->bus;
+
+ return qca8k_mdio_read(bus, phy, regnum);
+}
+
static int
qca8k_phy_write(struct dsa_switch *ds, int port, int regnum, u16 data)
{
@@ -775,8 +789,8 @@ qca8k_mdio_register(struct qca8k_priv *priv, struct device_node *mdio)
bus->priv = (void *)priv;
bus->name = "qca8k slave mii";
- bus->read = qca8k_mdio_read;
- bus->write = qca8k_mdio_write;
+ bus->read = qca8k_internal_mdio_read;
+ bus->write = qca8k_internal_mdio_write;
snprintf(bus->id, MII_BUS_ID_SIZE, "qca8k-%d",
ds->index);
--
2.32.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] net: dsa: qca8k: fix kernel panic with legacy mdio mapping
2021-09-11 15:07 [PATCH] net: dsa: qca8k: fix kernel panic with legacy mdio mapping Ansuel Smith
@ 2021-09-11 15:31 ` Florian Fainelli
2021-09-11 15:36 ` Ansuel Smith
0 siblings, 1 reply; 5+ messages in thread
From: Florian Fainelli @ 2021-09-11 15:31 UTC (permalink / raw)
To: Ansuel Smith, Andrew Lunn, Vivien Didelot, Vladimir Oltean,
David S. Miller, Jakub Kicinski, netdev, linux-kernel
On 9/11/2021 08:07, Ansuel Smith wrote:
> When the mdio legacy mapping is used the mii_bus priv registred by DSA
typo: registered
> refer to the dsa switch struct instead of the qca8k_priv struct and
> cause a kernel panic.
typo: causes
> Create dedicated function when the internal
> dedicated mdio driver is used to proprely handle the 2 different
> implementation.
typo: properly
>
> Fixes: 759bafb8a322 ("net: dsa: qca8k: add support for internal phy and internal mdio")
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
> drivers/net/dsa/qca8k.c | 30 ++++++++++++++++++++++--------
> 1 file changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> index 1f63f50f73f1..a701323daf72 100644
> --- a/drivers/net/dsa/qca8k.c
> +++ b/drivers/net/dsa/qca8k.c
> @@ -643,10 +643,8 @@ qca8k_mdio_busy_wait(struct mii_bus *bus, u32 reg, u32 mask)
> }
>
> static int
> -qca8k_mdio_write(struct mii_bus *salve_bus, int phy, int regnum, u16 data)
> +qca8k_mdio_write(struct mii_bus *bus, int phy, int regnum, u16 data)
> {
> - struct qca8k_priv *priv = salve_bus->priv;
> - struct mii_bus *bus = priv->bus;
> u16 r1, r2, page;
> u32 val;
> int ret;
> @@ -682,10 +680,8 @@ qca8k_mdio_write(struct mii_bus *salve_bus, int phy, int regnum, u16 data)
> }
>
> static int
> -qca8k_mdio_read(struct mii_bus *salve_bus, int phy, int regnum)
> +qca8k_mdio_read(struct mii_bus *bus, int phy, int regnum)
> {
> - struct qca8k_priv *priv = salve_bus->priv;
> - struct mii_bus *bus = priv->bus;
> u16 r1, r2, page;
> u32 val;
> int ret;
> @@ -726,6 +722,24 @@ qca8k_mdio_read(struct mii_bus *salve_bus, int phy, int regnum)
> return ret;
> }
>
> +static int
> +qca8k_internal_mdio_write(struct mii_bus *salve_bus, int phy, int regnum, u16 data)
> +{
> + struct qca8k_priv *priv = salve_bus->priv;
You are only moving code here but while at it, mind fixing that typo?
> + struct mii_bus *bus = priv->bus;
> +
> + return qca8k_mdio_write(bus, phy, regnum, data);
> +}
> +
> +static int
> +qca8k_internal_mdio_read(struct mii_bus *salve_bus, int phy, int regnum)
> +{
> + struct qca8k_priv *priv = salve_bus->priv;
> + struct mii_bus *bus = priv->bus;
> +
> + return qca8k_mdio_read(bus, phy, regnum);
> +}
> +
> static int
> qca8k_phy_write(struct dsa_switch *ds, int port, int regnum, u16 data)
> {
> @@ -775,8 +789,8 @@ qca8k_mdio_register(struct qca8k_priv *priv, struct device_node *mdio)
>
> bus->priv = (void *)priv;
> bus->name = "qca8k slave mii";
> - bus->read = qca8k_mdio_read;
> - bus->write = qca8k_mdio_write;
> + bus->read = qca8k_internal_mdio_read;
> + bus->write = qca8k_internal_mdio_write;
> snprintf(bus->id, MII_BUS_ID_SIZE, "qca8k-%d",
> ds->index);
>
>
--
Florian
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] net: dsa: qca8k: fix kernel panic with legacy mdio mapping
2021-09-11 15:31 ` Florian Fainelli
@ 2021-09-11 15:36 ` Ansuel Smith
2021-09-11 15:40 ` Vladimir Oltean
0 siblings, 1 reply; 5+ messages in thread
From: Ansuel Smith @ 2021-09-11 15:36 UTC (permalink / raw)
To: Florian Fainelli
Cc: Andrew Lunn, Vivien Didelot, Vladimir Oltean, David S. Miller,
Jakub Kicinski, netdev, linux-kernel
On Sat, Sep 11, 2021 at 08:31:51AM -0700, Florian Fainelli wrote:
>
>
> On 9/11/2021 08:07, Ansuel Smith wrote:
> > When the mdio legacy mapping is used the mii_bus priv registred by DSA
>
> typo: registered
>
> > refer to the dsa switch struct instead of the qca8k_priv struct and
> > cause a kernel panic.
>
> typo: causes
>
> > Create dedicated function when the internal
> > dedicated mdio driver is used to proprely handle the 2 different
> > implementation.
>
> typo: properly
>
> >
> > Fixes: 759bafb8a322 ("net: dsa: qca8k: add support for internal phy and internal mdio")
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> > drivers/net/dsa/qca8k.c | 30 ++++++++++++++++++++++--------
> > 1 file changed, 22 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
> > index 1f63f50f73f1..a701323daf72 100644
> > --- a/drivers/net/dsa/qca8k.c
> > +++ b/drivers/net/dsa/qca8k.c
> > @@ -643,10 +643,8 @@ qca8k_mdio_busy_wait(struct mii_bus *bus, u32 reg, u32 mask)
> > }
> > static int
> > -qca8k_mdio_write(struct mii_bus *salve_bus, int phy, int regnum, u16 data)
> > +qca8k_mdio_write(struct mii_bus *bus, int phy, int regnum, u16 data)
> > {
> > - struct qca8k_priv *priv = salve_bus->priv;
> > - struct mii_bus *bus = priv->bus;
> > u16 r1, r2, page;
> > u32 val;
> > int ret;
> > @@ -682,10 +680,8 @@ qca8k_mdio_write(struct mii_bus *salve_bus, int phy, int regnum, u16 data)
> > }
> > static int
> > -qca8k_mdio_read(struct mii_bus *salve_bus, int phy, int regnum)
> > +qca8k_mdio_read(struct mii_bus *bus, int phy, int regnum)
> > {
> > - struct qca8k_priv *priv = salve_bus->priv;
> > - struct mii_bus *bus = priv->bus;
> > u16 r1, r2, page;
> > u32 val;
> > int ret;
> > @@ -726,6 +722,24 @@ qca8k_mdio_read(struct mii_bus *salve_bus, int phy, int regnum)
> > return ret;
> > }
> > +static int
> > +qca8k_internal_mdio_write(struct mii_bus *salve_bus, int phy, int regnum, u16 data)
> > +{
> > + struct qca8k_priv *priv = salve_bus->priv;
>
> You are only moving code here but while at it, mind fixing that typo?
>
I think I didn't understand what you mean here.
Sure I will fix the typo and sorry about it.
Aside from that anything wrong with the 2 new function or there is a
better fix that I can't think of.
> > + struct mii_bus *bus = priv->bus;
> > +
> > + return qca8k_mdio_write(bus, phy, regnum, data);
> > +}
> > +
> > +static int
> > +qca8k_internal_mdio_read(struct mii_bus *salve_bus, int phy, int regnum)
> > +{
> > + struct qca8k_priv *priv = salve_bus->priv;
> > + struct mii_bus *bus = priv->bus;
> > +
> > + return qca8k_mdio_read(bus, phy, regnum);
> > +}
> > +
> > static int
> > qca8k_phy_write(struct dsa_switch *ds, int port, int regnum, u16 data)
> > {
> > @@ -775,8 +789,8 @@ qca8k_mdio_register(struct qca8k_priv *priv, struct device_node *mdio)
> > bus->priv = (void *)priv;
> > bus->name = "qca8k slave mii";
> > - bus->read = qca8k_mdio_read;
> > - bus->write = qca8k_mdio_write;
> > + bus->read = qca8k_internal_mdio_read;
> > + bus->write = qca8k_internal_mdio_write;
> > snprintf(bus->id, MII_BUS_ID_SIZE, "qca8k-%d",
> > ds->index);
> >
>
> --
> Florian
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] net: dsa: qca8k: fix kernel panic with legacy mdio mapping
2021-09-11 15:36 ` Ansuel Smith
@ 2021-09-11 15:40 ` Vladimir Oltean
2021-09-11 15:44 ` Ansuel Smith
0 siblings, 1 reply; 5+ messages in thread
From: Vladimir Oltean @ 2021-09-11 15:40 UTC (permalink / raw)
To: Ansuel Smith
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
Jakub Kicinski, netdev, linux-kernel
On Sat, Sep 11, 2021 at 05:36:40PM +0200, Ansuel Smith wrote:
> > > +static int
> > > +qca8k_internal_mdio_write(struct mii_bus *salve_bus, int phy, int regnum, u16 data)
> > > +{
> > > + struct qca8k_priv *priv = salve_bus->priv;
> >
> > You are only moving code here but while at it, mind fixing that typo?
> >
>
> I think I didn't understand what you mean here.
> Sure I will fix the typo and sorry about it.
> Aside from that anything wrong with the 2 new function or there is a
> better fix that I can't think of.
"salve" is "hello" in Italian, and even though that is a greeting,
surely that is not what was meant, but rather "slave". So even though
that is less positive, it is at least a technical term with a clear
meaning, so I think Florian's request was to replace "salve" with
"slave" while you are moving the code anyway.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] net: dsa: qca8k: fix kernel panic with legacy mdio mapping
2021-09-11 15:40 ` Vladimir Oltean
@ 2021-09-11 15:44 ` Ansuel Smith
0 siblings, 0 replies; 5+ messages in thread
From: Ansuel Smith @ 2021-09-11 15:44 UTC (permalink / raw)
To: Vladimir Oltean
Cc: Florian Fainelli, Andrew Lunn, Vivien Didelot, David S. Miller,
Jakub Kicinski, netdev, linux-kernel
On Sat, Sep 11, 2021 at 06:40:55PM +0300, Vladimir Oltean wrote:
> On Sat, Sep 11, 2021 at 05:36:40PM +0200, Ansuel Smith wrote:
> > > > +static int
> > > > +qca8k_internal_mdio_write(struct mii_bus *salve_bus, int phy, int regnum, u16 data)
> > > > +{
> > > > + struct qca8k_priv *priv = salve_bus->priv;
> > >
> > > You are only moving code here but while at it, mind fixing that typo?
> > >
> >
> > I think I didn't understand what you mean here.
> > Sure I will fix the typo and sorry about it.
> > Aside from that anything wrong with the 2 new function or there is a
> > better fix that I can't think of.
>
> "salve" is "hello" in Italian, and even though that is a greeting,
> surely that is not what was meant, but rather "slave". So even though
> that is less positive, it is at least a technical term with a clear
> meaning, so I think Florian's request was to replace "salve" with
> "slave" while you are moving the code anyway.
Ok this is embarrassing... Typo by vscode autocorrection... Fixing that
and sending v2. Again sorry.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-11 15:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-11 15:07 [PATCH] net: dsa: qca8k: fix kernel panic with legacy mdio mapping Ansuel Smith
2021-09-11 15:31 ` Florian Fainelli
2021-09-11 15:36 ` Ansuel Smith
2021-09-11 15:40 ` Vladimir Oltean
2021-09-11 15:44 ` Ansuel Smith
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).