From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED, USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C5586C282D7 for ; Wed, 30 Jan 2019 09:24:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A069B20882 for ; Wed, 30 Jan 2019 09:24:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730155AbfA3JYz (ORCPT ); Wed, 30 Jan 2019 04:24:55 -0500 Received: from atrey.karlin.mff.cuni.cz ([195.113.26.193]:46614 "EHLO atrey.karlin.mff.cuni.cz" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730091AbfA3JYz (ORCPT ); Wed, 30 Jan 2019 04:24:55 -0500 Received: by atrey.karlin.mff.cuni.cz (Postfix, from userid 512) id A9BDD801E8; Wed, 30 Jan 2019 10:24:45 +0100 (CET) Date: Wed, 30 Jan 2019 10:24:51 +0100 From: Pavel Machek To: Andrew Lunn Cc: netdev , Vivien Didelot , Florian Fainelli Subject: Re: [PATCH RFC RFT net-next 03/10] net: dsa: mv88e6060: Replace REG_WRITE macro Message-ID: <20190130092451.GA22071@amd> References: <20190130003758.23852-1-andrew@lunn.ch> <20190130003758.23852-4-andrew@lunn.ch> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="cWoXeonUoKmBZSoM" Content-Disposition: inline In-Reply-To: <20190130003758.23852-4-andrew@lunn.ch> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org --cWoXeonUoKmBZSoM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed 2019-01-30 01:37:51, Andrew Lunn wrote: > The REG_WRITE macro contains a return statement, making it not very > safe. Remove it by inlining the code. Not bad, but maybe there should be dev_err() or something in case of reg_write() returns an error? Because no errors are expected in this case... AFAICT. Pavel > Signed-off-by: Andrew Lunn > --- > drivers/net/dsa/mv88e6060.c | 73 +++++++++++++++++++++---------------- > 1 file changed, 41 insertions(+), 32 deletions(-) >=20 > diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c > index 631358bf3d6b..da88c56e092c 100644 > --- a/drivers/net/dsa/mv88e6060.c > +++ b/drivers/net/dsa/mv88e6060.c > @@ -39,15 +39,6 @@ static int reg_write(struct mv88e6060_priv *priv, int = addr, int reg, u16 val) > return mdiobus_write_nested(priv->bus, priv->sw_addr + addr, reg, val); > } > =20 > -#define REG_WRITE(addr, reg, val) \ > - ({ \ > - int __ret; \ > - \ > - __ret =3D reg_write(priv, addr, reg, val); \ > - if (__ret < 0) \ > - return __ret; \ > - }) > - > static const char *mv88e6060_get_name(struct mii_bus *bus, int sw_addr) > { > int ret; > @@ -102,17 +93,21 @@ static int mv88e6060_switch_reset(struct mv88e6060_p= riv *priv) > /* Set all ports to the disabled state. */ > for (i =3D 0; i < MV88E6060_PORTS; i++) { > ret =3D REG_READ(REG_PORT(i), PORT_CONTROL); > - REG_WRITE(REG_PORT(i), PORT_CONTROL, > - ret & ~PORT_CONTROL_STATE_MASK); > + ret =3D reg_write(priv, REG_PORT(i), PORT_CONTROL, > + ret & ~PORT_CONTROL_STATE_MASK); > + if (ret) > + return ret; > } > =20 > /* Wait for transmit queues to drain. */ > usleep_range(2000, 4000); > =20 > /* Reset the switch. */ > - REG_WRITE(REG_GLOBAL, GLOBAL_ATU_CONTROL, > - GLOBAL_ATU_CONTROL_SWRESET | > - GLOBAL_ATU_CONTROL_LEARNDIS); > + ret =3D reg_write(priv, REG_GLOBAL, GLOBAL_ATU_CONTROL, > + GLOBAL_ATU_CONTROL_SWRESET | > + GLOBAL_ATU_CONTROL_LEARNDIS); > + if (ret) > + return ret; > =20 > /* Wait up to one second for reset to complete. */ > timeout =3D jiffies + 1 * HZ; > @@ -131,59 +126,67 @@ static int mv88e6060_switch_reset(struct mv88e6060_= priv *priv) > =20 > static int mv88e6060_setup_global(struct mv88e6060_priv *priv) > { > + int ret; > + > /* Disable discarding of frames with excessive collisions, > * set the maximum frame size to 1536 bytes, and mask all > * interrupt sources. > */ > - REG_WRITE(REG_GLOBAL, GLOBAL_CONTROL, GLOBAL_CONTROL_MAX_FRAME_1536); > + ret =3D reg_write(priv, REG_GLOBAL, GLOBAL_CONTROL, > + GLOBAL_CONTROL_MAX_FRAME_1536); > + if (ret) > + return ret; > =20 > /* Disable automatic address learning. > */ > - REG_WRITE(REG_GLOBAL, GLOBAL_ATU_CONTROL, > - GLOBAL_ATU_CONTROL_LEARNDIS); > - > - return 0; > + return reg_write(priv, REG_GLOBAL, GLOBAL_ATU_CONTROL, > + GLOBAL_ATU_CONTROL_LEARNDIS); > } > =20 > static int mv88e6060_setup_port(struct mv88e6060_priv *priv, int p) > { > int addr =3D REG_PORT(p); > + int ret; > =20 > /* Do not force flow control, disable Ingress and Egress > * Header tagging, disable VLAN tunneling, and set the port > * state to Forwarding. Additionally, if this is the CPU > * port, enable Ingress and Egress Trailer tagging mode. > */ > - REG_WRITE(addr, PORT_CONTROL, > - dsa_is_cpu_port(priv->ds, p) ? > + ret =3D reg_write(priv, addr, PORT_CONTROL, > + dsa_is_cpu_port(priv->ds, p) ? > PORT_CONTROL_TRAILER | > PORT_CONTROL_INGRESS_MODE | > PORT_CONTROL_STATE_FORWARDING : > PORT_CONTROL_STATE_FORWARDING); > + if (ret) > + return ret; > =20 > /* Port based VLAN map: give each port its own address > * database, allow the CPU port to talk to each of the 'real' > * ports, and allow each of the 'real' ports to only talk to > * the CPU port. > */ > - REG_WRITE(addr, PORT_VLAN_MAP, > - ((p & 0xf) << PORT_VLAN_MAP_DBNUM_SHIFT) | > - (dsa_is_cpu_port(priv->ds, p) ? dsa_user_ports(priv->ds) : > - BIT(dsa_to_port(priv->ds, p)->cpu_dp->index))); > + ret =3D reg_write(priv, addr, PORT_VLAN_MAP, > + ((p & 0xf) << PORT_VLAN_MAP_DBNUM_SHIFT) | > + (dsa_is_cpu_port(priv->ds, p) ? > + dsa_user_ports(priv->ds) : > + BIT(dsa_to_port(priv->ds, p)->cpu_dp->index))); > + if (ret) > + return ret; > =20 > /* Port Association Vector: when learning source addresses > * of packets, add the address to the address database using > * a port bitmap that has only the bit for this port set and > * the other bits clear. > */ > - REG_WRITE(addr, PORT_ASSOC_VECTOR, BIT(p)); > - > - return 0; > + return reg_write(priv, addr, PORT_ASSOC_VECTOR, BIT(p)); > } > =20 > static int mv88e6060_setup_addr(struct mv88e6060_priv *priv) > { > u8 addr[ETH_ALEN]; > + int ret; > u16 val; > =20 > eth_random_addr(addr); > @@ -195,11 +198,17 @@ static int mv88e6060_setup_addr(struct mv88e6060_pr= iv *priv) > */ > val &=3D 0xfeff; > =20 > - REG_WRITE(REG_GLOBAL, GLOBAL_MAC_01, val); > - REG_WRITE(REG_GLOBAL, GLOBAL_MAC_23, (addr[2] << 8) | addr[3]); > - REG_WRITE(REG_GLOBAL, GLOBAL_MAC_45, (addr[4] << 8) | addr[5]); > + ret =3D reg_write(priv, REG_GLOBAL, GLOBAL_MAC_01, val); > + if (ret) > + return ret; > + > + ret =3D reg_write(priv, REG_GLOBAL, GLOBAL_MAC_23, > + (addr[2] << 8) | addr[3]); > + if (ret) > + return ret; > =20 > - return 0; > + return reg_write(priv, REG_GLOBAL, GLOBAL_MAC_45, > + (addr[4] << 8) | addr[5]); > } > =20 > static int mv88e6060_setup(struct dsa_switch *ds) --=20 (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blo= g.html --cWoXeonUoKmBZSoM Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iEYEARECAAYFAlxRbWMACgkQMOfwapXb+vLU7ACfT+DzwoJ1acBsUiKXuhtFtC2Y pmMAn1JmkD7xyCG1hzu5qveVwV0wK9/h =I46e -----END PGP SIGNATURE----- --cWoXeonUoKmBZSoM--