* [PATCH net 0/2] net: dsa: bcm_sf2: Couple fixes
@ 2017-01-08 5:01 Florian Fainelli
2017-01-08 5:01 ` [PATCH net 1/2] net: dsa: bcm_sf2: Do not clobber b53_switch_ops Florian Fainelli
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Florian Fainelli @ 2017-01-08 5:01 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
Hi David,
Here are a couple of fixes for bcm_sf2, please queue these up for -stable
as well, thank you very much!
Florian Fainelli (2):
net: dsa: bcm_sf2: Do not clobber b53_switch_ops
net: dsa: bcm_sf2: Utilize nested MDIO read/write
drivers/net/dsa/bcm_sf2.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
--
2.9.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net 1/2] net: dsa: bcm_sf2: Do not clobber b53_switch_ops
2017-01-08 5:01 [PATCH net 0/2] net: dsa: bcm_sf2: Couple fixes Florian Fainelli
@ 2017-01-08 5:01 ` Florian Fainelli
2017-01-08 17:41 ` Andrew Lunn
2017-01-08 20:17 ` Andrew Lunn
2017-01-08 5:01 ` [PATCH net 2/2] net: dsa: bcm_sf2: Utilize nested MDIO read/write Florian Fainelli
2017-01-09 3:02 ` [PATCH net 0/2] net: dsa: bcm_sf2: Couple fixes David Miller
2 siblings, 2 replies; 9+ messages in thread
From: Florian Fainelli @ 2017-01-08 5:01 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
We make the bcm_sf2 driver override ds->ops which points to
b53_switch_ops since b53_switch_alloc() did the assignent. This is all
well and good until a second b53 switch comes in, and ends up using the
bcm_sf2 operations. Make a proper local copy, substitute the ds->ops
pointer and then override the operations.
Fixes: f458995b9ad8 ("net: dsa: bcm_sf2: Utilize core B53 driver when possible")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 9ec33b51a0ed..2f9f910c0e40 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -982,6 +982,7 @@ static int bcm_sf2_sw_probe(struct platform_device *pdev)
const char *reg_names[BCM_SF2_REGS_NUM] = BCM_SF2_REGS_NAME;
struct device_node *dn = pdev->dev.of_node;
struct b53_platform_data *pdata;
+ struct dsa_switch_ops *ops;
struct bcm_sf2_priv *priv;
struct b53_device *dev;
struct dsa_switch *ds;
@@ -995,6 +996,10 @@ static int bcm_sf2_sw_probe(struct platform_device *pdev)
if (!priv)
return -ENOMEM;
+ ops = devm_kzalloc(&pdev->dev, sizeof(*ops), GFP_KERNEL);
+ if (!ops)
+ return -ENOMEM;
+
dev = b53_switch_alloc(&pdev->dev, &bcm_sf2_io_ops, priv);
if (!dev)
return -ENOMEM;
@@ -1014,6 +1019,8 @@ static int bcm_sf2_sw_probe(struct platform_device *pdev)
ds = dev->ds;
/* Override the parts that are non-standard wrt. normal b53 devices */
+ memcpy(ops, ds->ops, sizeof(*ops));
+ ds->ops = ops;
ds->ops->get_tag_protocol = bcm_sf2_sw_get_tag_protocol;
ds->ops->setup = bcm_sf2_sw_setup;
ds->ops->get_phy_flags = bcm_sf2_sw_get_phy_flags;
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net 2/2] net: dsa: bcm_sf2: Utilize nested MDIO read/write
2017-01-08 5:01 [PATCH net 0/2] net: dsa: bcm_sf2: Couple fixes Florian Fainelli
2017-01-08 5:01 ` [PATCH net 1/2] net: dsa: bcm_sf2: Do not clobber b53_switch_ops Florian Fainelli
@ 2017-01-08 5:01 ` Florian Fainelli
2017-01-08 17:41 ` Andrew Lunn
2017-01-09 3:02 ` [PATCH net 0/2] net: dsa: bcm_sf2: Couple fixes David Miller
2 siblings, 1 reply; 9+ messages in thread
From: Florian Fainelli @ 2017-01-08 5:01 UTC (permalink / raw)
To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
We are implementing a MDIO bus which is behind another one, so use the
nested version of the accessors to get lockdep annotations correct.
Fixes: 461cd1b03e32 ("net: dsa: bcm_sf2: Register our slave MDIO bus")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/dsa/bcm_sf2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 2f9f910c0e40..2ce7ae97ac91 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -393,7 +393,7 @@ static int bcm_sf2_sw_mdio_read(struct mii_bus *bus, int addr, int regnum)
if (addr == BRCM_PSEUDO_PHY_ADDR && priv->indir_phy_mask & BIT(addr))
return bcm_sf2_sw_indir_rw(priv, 1, addr, regnum, 0);
else
- return mdiobus_read(priv->master_mii_bus, addr, regnum);
+ return mdiobus_read_nested(priv->master_mii_bus, addr, regnum);
}
static int bcm_sf2_sw_mdio_write(struct mii_bus *bus, int addr, int regnum,
@@ -407,7 +407,7 @@ static int bcm_sf2_sw_mdio_write(struct mii_bus *bus, int addr, int regnum,
if (addr == BRCM_PSEUDO_PHY_ADDR && priv->indir_phy_mask & BIT(addr))
bcm_sf2_sw_indir_rw(priv, 0, addr, regnum, val);
else
- mdiobus_write(priv->master_mii_bus, addr, regnum, val);
+ mdiobus_write_nested(priv->master_mii_bus, addr, regnum, val);
return 0;
}
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net 1/2] net: dsa: bcm_sf2: Do not clobber b53_switch_ops
2017-01-08 5:01 ` [PATCH net 1/2] net: dsa: bcm_sf2: Do not clobber b53_switch_ops Florian Fainelli
@ 2017-01-08 17:41 ` Andrew Lunn
2017-01-08 19:31 ` Florian Fainelli
2017-01-08 20:17 ` Andrew Lunn
1 sibling, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2017-01-08 17:41 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, davem, vivien.didelot
On Sat, Jan 07, 2017 at 09:01:56PM -0800, Florian Fainelli wrote:
> We make the bcm_sf2 driver override ds->ops which points to
> b53_switch_ops since b53_switch_alloc() did the assignent. This is all
> well and good until a second b53 switch comes in, and ends up using the
> bcm_sf2 operations. Make a proper local copy, substitute the ds->ops
> pointer and then override the operations.
>
> Fixes: f458995b9ad8 ("net: dsa: bcm_sf2: Utilize core B53 driver when possible")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Hi Florian
There is a general trend of making ops structures const. It closes off
kernel exploits. This coping and then modifying prevents us making
ds->ops a pointer to a const.
You are already using b53_common.c as a library. Could you go further
with the concept, and export the ops you need for SF2, and have SF2
define its own ops structure? We can then swap to const ops dsa wide.
Thanks
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 2/2] net: dsa: bcm_sf2: Utilize nested MDIO read/write
2017-01-08 5:01 ` [PATCH net 2/2] net: dsa: bcm_sf2: Utilize nested MDIO read/write Florian Fainelli
@ 2017-01-08 17:41 ` Andrew Lunn
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2017-01-08 17:41 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, davem, vivien.didelot
On Sat, Jan 07, 2017 at 09:01:57PM -0800, Florian Fainelli wrote:
> We are implementing a MDIO bus which is behind another one, so use the
> nested version of the accessors to get lockdep annotations correct.
>
> Fixes: 461cd1b03e32 ("net: dsa: bcm_sf2: Register our slave MDIO bus")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 1/2] net: dsa: bcm_sf2: Do not clobber b53_switch_ops
2017-01-08 17:41 ` Andrew Lunn
@ 2017-01-08 19:31 ` Florian Fainelli
2017-01-08 20:16 ` Andrew Lunn
0 siblings, 1 reply; 9+ messages in thread
From: Florian Fainelli @ 2017-01-08 19:31 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, davem, vivien.didelot
Le 01/08/17 à 09:41, Andrew Lunn a écrit :
> On Sat, Jan 07, 2017 at 09:01:56PM -0800, Florian Fainelli wrote:
>> We make the bcm_sf2 driver override ds->ops which points to
>> b53_switch_ops since b53_switch_alloc() did the assignent. This is all
>> well and good until a second b53 switch comes in, and ends up using the
>> bcm_sf2 operations. Make a proper local copy, substitute the ds->ops
>> pointer and then override the operations.
>>
>> Fixes: f458995b9ad8 ("net: dsa: bcm_sf2: Utilize core B53 driver when possible")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
> Hi Florian
Hi Andrew,
>
> There is a general trend of making ops structures const. It closes off
> kernel exploits. This coping and then modifying prevents us making
> ds->ops a pointer to a const.
Agreed, and this was my initial approach, but I also wanted a minimal
fix for David to pull into "net" while we can properly resolve this for
"net-next" see below.
>
> You are already using b53_common.c as a library. Could you go further
> with the concept, and export the ops you need for SF2, and have SF2
> define its own ops structure? We can then swap to const ops dsa wide.
Making the ops const was my initial approach but there are several
challenges to making it possible right now which I will address against
net-next:
- register/unregister_switch_driver actually do modify dsa_switch_ops
while updating the list pointer, so we need to encapsulate
dsa_switch_ops into a dsa_switch_driver plus a list member
- as you pointed out, b53 needs to export the operations to other
drivers that are going to make use of them
Thanks for your comments!
--
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 1/2] net: dsa: bcm_sf2: Do not clobber b53_switch_ops
2017-01-08 19:31 ` Florian Fainelli
@ 2017-01-08 20:16 ` Andrew Lunn
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2017-01-08 20:16 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, davem, vivien.didelot
> Agreed, and this was my initial approach, but I also wanted a minimal
> fix for David to pull into "net" while we can properly resolve this for
> "net-next" see below.
O.K, so in that case, this is fine.
> Making the ops const was my initial approach but there are several
> challenges to making it possible right now which I will address against
> net-next:
>
> - register/unregister_switch_driver actually do modify dsa_switch_ops
> while updating the list pointer, so we need to encapsulate
> dsa_switch_ops into a dsa_switch_driver plus a list member
O.K, this is dsa v1. I had v2 in mind. Yes, the list needs
abstracting.
Thanks
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 1/2] net: dsa: bcm_sf2: Do not clobber b53_switch_ops
2017-01-08 5:01 ` [PATCH net 1/2] net: dsa: bcm_sf2: Do not clobber b53_switch_ops Florian Fainelli
2017-01-08 17:41 ` Andrew Lunn
@ 2017-01-08 20:17 ` Andrew Lunn
1 sibling, 0 replies; 9+ messages in thread
From: Andrew Lunn @ 2017-01-08 20:17 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, davem, vivien.didelot
On Sat, Jan 07, 2017 at 09:01:56PM -0800, Florian Fainelli wrote:
> We make the bcm_sf2 driver override ds->ops which points to
> b53_switch_ops since b53_switch_alloc() did the assignent. This is all
> well and good until a second b53 switch comes in, and ends up using the
> bcm_sf2 operations. Make a proper local copy, substitute the ds->ops
> pointer and then override the operations.
>
> Fixes: f458995b9ad8 ("net: dsa: bcm_sf2: Utilize core B53 driver when possible")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net 0/2] net: dsa: bcm_sf2: Couple fixes
2017-01-08 5:01 [PATCH net 0/2] net: dsa: bcm_sf2: Couple fixes Florian Fainelli
2017-01-08 5:01 ` [PATCH net 1/2] net: dsa: bcm_sf2: Do not clobber b53_switch_ops Florian Fainelli
2017-01-08 5:01 ` [PATCH net 2/2] net: dsa: bcm_sf2: Utilize nested MDIO read/write Florian Fainelli
@ 2017-01-09 3:02 ` David Miller
2 siblings, 0 replies; 9+ messages in thread
From: David Miller @ 2017-01-09 3:02 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, vivien.didelot
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Sat, 7 Jan 2017 21:01:55 -0800
> Here are a couple of fixes for bcm_sf2, please queue these up for
> -stable as well, thank you very much!
Series applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-01-09 3:02 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-08 5:01 [PATCH net 0/2] net: dsa: bcm_sf2: Couple fixes Florian Fainelli
2017-01-08 5:01 ` [PATCH net 1/2] net: dsa: bcm_sf2: Do not clobber b53_switch_ops Florian Fainelli
2017-01-08 17:41 ` Andrew Lunn
2017-01-08 19:31 ` Florian Fainelli
2017-01-08 20:16 ` Andrew Lunn
2017-01-08 20:17 ` Andrew Lunn
2017-01-08 5:01 ` [PATCH net 2/2] net: dsa: bcm_sf2: Utilize nested MDIO read/write Florian Fainelli
2017-01-08 17:41 ` Andrew Lunn
2017-01-09 3:02 ` [PATCH net 0/2] net: dsa: bcm_sf2: Couple fixes David Miller
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.