Re: [PATCH net-next 01/16] net: dsa: use dsa_to_port helper everywhere

[Vivien Didelot]

Hi Andrew,

On Mon, 21 Oct 2019 14:31:49 +0200, Andrew Lunn  wrote:
> On Sat, Oct 19, 2019 at 11:19:26PM -0400, Vivien Didelot wrote:
> > Do not let the drivers access the ds->ports static array directly
> > while there is a dsa_to_port helper for this purpose.
> > 
> > At the same time, un-const this helper since the SJA1105 driver
> > assigns the priv member of the returned dsa_port structure.
> 
> Hi Vivien
> 
> Is priv the only member we expect drivers to change? Is the rest
> private to the core/RO? Rather then remove the const, i wonder if it
> would be better to add a helper to set priv?
> 
> Otherwise:
> 
> Reviewed-by: Andrew Lunn 

I had the same thought but actually since the SJA1105 driver is the only
user, I was thinking about maybe getting rid of it, I don't really see the
point of having some "dp->priv = priv, priv->dp = dp" kind of code...

In the meantime I kept the eventual helper or priv removal for a future series.


Thank you,
Vivien

Re: [PATCH net-next 01/16] net: dsa: use dsa_to_port helper everywhere

[Andrew Lunn]

On Sat, Oct 19, 2019 at 11:19:26PM -0400, Vivien Didelot wrote:
> Do not let the drivers access the ds->ports static array directly
> while there is a dsa_to_port helper for this purpose.
> 
> At the same time, un-const this helper since the SJA1105 driver
> assigns the priv member of the returned dsa_port structure.

Hi Vivien

Is priv the only member we expect drivers to change? Is the rest
private to the core/RO? Rather then remove the const, i wonder if it
would be better to add a helper to set priv?

Otherwise:

Reviewed-by: Andrew Lunn 

Andrew

Re: [PATCH net-next 01/16] net: dsa: use dsa_to_port helper everywhere

[Florian Fainelli]

On 10/19/2019 8:19 PM, Vivien Didelot wrote:
> Do not let the drivers access the ds->ports static array directly
> while there is a dsa_to_port helper for this purpose.
> 
> At the same time, un-const this helper since the SJA1105 driver
> assigns the priv member of the returned dsa_port structure.
> 
> Signed-off-by: Vivien Didelot 

Reviewed-by: Florian Fainelli 
-- 
Florian

[PATCH net-next 01/16] net: dsa: use dsa_to_port helper everywhere

[Vivien Didelot]

Do not let the drivers access the ds->ports static array directly
while there is a dsa_to_port helper for this purpose.

At the same time, un-const this helper since the SJA1105 driver
assigns the priv member of the returned dsa_port structure.

Signed-off-by: Vivien Didelot 
---
 drivers/net/dsa/b53/b53_common.c   |  6 +++---
 drivers/net/dsa/bcm_sf2.c  |  8 
 drivers/net/dsa/bcm_sf2_cfp.c  |  6 +++---
 drivers/net/dsa/mt7530.c   | 12 ++--
 drivers/net/dsa/mv88e6xxx/chip.c   | 10 +-
 drivers/net/dsa/qca8k.c|  2 +-
 drivers/net/dsa/sja1105/sja1105_main.c | 18 +-
 include/net/dsa.h  |  2 +-
 net/dsa/dsa.c  |  8 +---
 net/dsa/dsa2.c |  4 ++--
 net/dsa/switch.c   |  4 ++--
 net/dsa/tag_8021q.c|  6 +++---
 12 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 526ba2ab66f1..9ba91f1370ac 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -524,7 +524,7 @@ int b53_enable_port(struct dsa_switch *ds, int port, struct 
phy_device *phy)
if (!dsa_is_user_port(ds, port))
return 0;
 
-   cpu_port = ds->ports[port].cpu_dp->index;
+   cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
 
if (dev->ops->irq_enable)
ret = dev->ops->irq_enable(dev, port);
@@ -1629,7 +1629,7 @@ EXPORT_SYMBOL(b53_fdb_dump);
 int b53_br_join(struct dsa_switch *ds, int port, struct net_device *br)
 {
struct b53_device *dev = ds->priv;
-   s8 cpu_port = ds->ports[port].cpu_dp->index;
+   s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
u16 pvlan, reg;
unsigned int i;
 
@@ -1675,7 +1675,7 @@ void b53_br_leave(struct dsa_switch *ds, int port, struct 
net_device *br)
 {
struct b53_device *dev = ds->priv;
struct b53_vlan *vl = >vlans[0];
-   s8 cpu_port = ds->ports[port].cpu_dp->index;
+   s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
unsigned int i;
u16 pvlan, reg, pvid;
 
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 26509fa37a50..c068a3b7207b 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -662,7 +662,7 @@ static void bcm_sf2_sw_fixed_state(struct dsa_switch *ds, 
int port,
 * state machine and make it go in PHY_FORCING state instead.
 */
if (!status->link)
-   netif_carrier_off(ds->ports[port].slave);
+   netif_carrier_off(dsa_to_port(ds, port)->slave);
status->duplex = DUPLEX_FULL;
} else {
status->link = true;
@@ -728,7 +728,7 @@ static int bcm_sf2_sw_resume(struct dsa_switch *ds)
 static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int port,
   struct ethtool_wolinfo *wol)
 {
-   struct net_device *p = ds->ports[port].cpu_dp->master;
+   struct net_device *p = dsa_to_port(ds, port)->cpu_dp->master;
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
struct ethtool_wolinfo pwol = { };
 
@@ -752,9 +752,9 @@ static void bcm_sf2_sw_get_wol(struct dsa_switch *ds, int 
port,
 static int bcm_sf2_sw_set_wol(struct dsa_switch *ds, int port,
  struct ethtool_wolinfo *wol)
 {
-   struct net_device *p = ds->ports[port].cpu_dp->master;
+   struct net_device *p = dsa_to_port(ds, port)->cpu_dp->master;
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
-   s8 cpu_port = ds->ports[port].cpu_dp->index;
+   s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
struct ethtool_wolinfo pwol =  { };
 
if (p->ethtool_ops->get_wol)
diff --git a/drivers/net/dsa/bcm_sf2_cfp.c b/drivers/net/dsa/bcm_sf2_cfp.c
index d264776a95a3..f3f0c3f07391 100644
--- a/drivers/net/dsa/bcm_sf2_cfp.c
+++ b/drivers/net/dsa/bcm_sf2_cfp.c
@@ -821,7 +821,7 @@ static int bcm_sf2_cfp_rule_insert(struct dsa_switch *ds, 
int port,
   struct ethtool_rx_flow_spec *fs)
 {
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
-   s8 cpu_port = ds->ports[port].cpu_dp->index;
+   s8 cpu_port = dsa_to_port(ds, port)->cpu_dp->index;
__u64 ring_cookie = fs->ring_cookie;
unsigned int queue_num, port_num;
int ret;
@@ -1049,7 +1049,7 @@ static int bcm_sf2_cfp_rule_get_all(struct bcm_sf2_priv 
*priv,
 int bcm_sf2_get_rxnfc(struct dsa_switch *ds, int port,
  struct ethtool_rxnfc *nfc, u32 *rule_locs)
 {
-   struct net_device *p = ds->ports[port].cpu_dp->master;
+   struct net_device *p = dsa_to_port(ds, port)->cpu_dp->master;
struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
int ret = 0;
 
@@ -1092,7 +1092,7 @@ int bcm_sf2_get_rxnfc(struct dsa_switch *ds,