Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1612441

Re: [PATCH net-next v3 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch

From Andrew Lunn <andrew@lunn.ch>
Newsgroups linux.kernel
Subject Re: [PATCH net-next v3 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch
Date 2017-03-30 01:20 +0200
Message-ID <tqv2G-5Ex-21@gated-at.bofh.it> (permalink)
References <tqif7-4VF-11@gated-at.bofh.it> <tqif8-4VF-25@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


> +static struct mt7530_priv *lpriv;

Hi Sean

Why do you need this global variable? What if somebody has two
switches connected?

> +static void mt7530_port_disable(struct dsa_switch *ds, int port,
> +				struct phy_device *phy);
> +static int mt7530_cpu_port_enable(struct mt7530_priv *priv,
> +				  int port);

It is better to move the functions around than have forward
declarations.

> +static u32
> +_mt7530_read(u32 reg)
> +{
> +	struct mt7530_priv	*priv = lpriv;
> +	struct mii_bus		*bus = priv->bus;
> +	u32 val;
> +
> +	mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
> +
> +	val = mt7530_mii_read(priv, reg);
> +
> +	mutex_unlock(&bus->mdio_lock);
> +
> +	return val;
> +}
> +
> +static u32
> +mt7530_read(struct mt7530_priv *priv, u32 reg)
> +{
> +	return _mt7530_read(reg);
> +}

So here you would pass priv to _mt7530_read().

> +static int
> +mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp)
> +{
> +	u32 val;
> +	int ret;
> +
> +	/* Set the command operating upon the MAC address entries */
> +	val = ATC_BUSY | ATC_MAT(0) | cmd;
> +	mt7530_write(priv, MT7530_ATC, val);
> +
> +	ret = readx_poll_timeout(_mt7530_read, MT7530_ATC, val,
> +				 !(val & ATC_BUSY), 20, 20000);

This is where your need for lpriv comes, you can only pass a single
argument. But you can work around it. readx_poll_timeout is a
#define. So there is no type check for _mt7530_read, or the arg passed
to it. So pass a struct containing the address and priv.

I think you can then kill of your global lpriv.

  Andrew

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[PATCH net-next v3 0/5] net-next: dsa: add Mediatek MT7530 support <sean.wang@mediatek.com> - 2017-03-29 11:40 +0200
  [PATCH net-next v3 1/5] dt-bindings: net: dsa: add Mediatek MT7530 binding <sean.wang@mediatek.com> - 2017-03-29 11:40 +0200
  [PATCH net-next v3 3/5] net-next: ethernet: mediatek: add CDM able to recognize the tag for DSA <sean.wang@mediatek.com> - 2017-03-29 11:40 +0200
  [PATCH net-next v3 4/5] net-next: ethernet: mediatek: add device_node of GMAC pointing into the netdev instance <sean.wang@mediatek.com> - 2017-03-29 11:40 +0200
  [PATCH net-next v3 2/5] net-next: dsa: add Mediatek tag RX/TX handler <sean.wang@mediatek.com> - 2017-03-29 11:50 +0200
  Re: [PATCH net-next v3 5/5] net-next: dsa: add dsa support for  Mediatek MT7530 switch Andrew Lunn <andrew@lunn.ch> - 2017-03-30 01:20 +0200

csiph-web