Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: "David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <linux@armlinux.org.uk>,
	kernel@pengutronix.de, linux-kernel@vger.kernel.org,
	linux-usb@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH net-next v1 2/7] net: usb: asix: ax88772: add phylib support
Date: Sat, 5 Jun 2021 01:18:57 +0200	[thread overview]
Message-ID: <YLq04ap4kjuPRKVe@lunn.ch> (raw)
In-Reply-To: <20210604134244.2467-3-o.rempel@pengutronix.de>

On Fri, Jun 04, 2021 at 03:42:39PM +0200, Oleksij Rempel wrote:
> To be able to use ax88772 with external PHYs and use advantage of

s/use/take/

> +/* MDIO read and write wrappers for phylib */
> +int asix_mdio_bus_read(struct mii_bus *bus, int phy_id, int regnum)
> +{
> +	return asix_mdio_read(((struct usbnet *)bus->priv)->net, phy_id,
> +			      regnum);
> +}

Please avoid this cast. priv should be a void *, so you can do

       struct usbnet *priv = bus->priv;

       return asix_mdio_read(priv->net, phy_id, regnum);

> +static int ax88772_init_phy(struct usbnet *dev)
> +{
> +	struct asix_common_private *priv = dev->driver_priv;
> +	int ret;
> +
> +	priv->phy_addr = asix_get_phy_addr(dev);
> +	/* asix_read_phy_addr() is using ret < 2 as error value */
> +	if (priv->phy_addr < 2)
> +		return -ENODEV;

Really?

ax88172a.c does not check. ax88172_bind() does not
check. ax88772_bind() does not check. As far as i can see, nothing
really cares.

So please add another cleanup patch and make asix_read_phy_addr()
return -ENODEV.

Otherwise, this looks O.K.

  reply	other threads:[~2021-06-04 23:19 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-04 13:42 [PATCH net-next v1 0/7] port asix ax88772 to the PHYlib Oleksij Rempel
2021-06-04 13:42 ` [PATCH net-next v1 1/7] net: usb: asix: ax88772_bind: use devm_kzalloc() instead of kzalloc() Oleksij Rempel
2021-06-04 23:05   ` Andrew Lunn
2021-06-04 13:42 ` [PATCH net-next v1 2/7] net: usb: asix: ax88772: add phylib support Oleksij Rempel
2021-06-04 23:18   ` Andrew Lunn [this message]
2021-06-04 13:42 ` [PATCH net-next v1 3/7] net: usb/phy: asix: add support for ax88772A/C PHYs Oleksij Rempel
2021-06-04 23:23   ` Andrew Lunn
2021-06-04 13:42 ` [PATCH net-next v1 4/7] net: usb: asix: ax88772: add generic selftest support Oleksij Rempel
2021-06-04 23:24   ` Andrew Lunn
2021-06-04 13:42 ` [PATCH net-next v1 5/7] net: usb: asix: add error handling for asix_mdio_* functions Oleksij Rempel
2021-06-04 23:31   ` Andrew Lunn
2021-06-07  7:55     ` Oleksij Rempel
2021-06-04 13:42 ` [PATCH net-next v1 6/7] net: phy: do not print dump stack if device was removed Oleksij Rempel
2021-06-04 13:42 ` [PATCH net-next v1 7/7] usbnet: run unbind() before unregister_netdev() Oleksij Rempel
2021-06-04 23:41   ` Andrew Lunn
2021-06-07  8:04     ` Oleksij Rempel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YLq04ap4kjuPRKVe@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=hkallweit1@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --subject='Re: [PATCH net-next v1 2/7] net: usb: asix: ax88772: add phylib support' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).