Re: [PATCH v3 2/2] net: phy: add marvell usb to mdio controller

[Greg KH <gregkh@linuxfoundation.org>]

On Sat, Mar 21, 2020 at 09:24:43PM +0100, Tobias Waldekranz wrote:
> An MDIO controller present on development boards for Marvell switches
> from the Link Street (88E6xxx) family.
> 
> Using this module, you can use the following setup as a development
> platform for switchdev and DSA related work.
> 
>    .-------.      .-----------------.
>    |      USB----USB                |
>    |  SoC  |      |  88E6390X-DB  ETH1-10
>    |      ETH----ETH0               |
>    '-------'      '-----------------'
> 
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
> 
> Hi linux-usb,
> 
> This is my first ever USB driver, therefore I would really appreciate
> it if someone could have a look at it from a USB perspective before it
> is (hopefully) pulled into net-next.

From a USB point of view, it looks sane, only one question:

> +static int mvusb_mdio_probe(struct usb_interface *interface,
> +			    const struct usb_device_id *id)
> +{
> +	struct device *dev = &interface->dev;
> +	struct mvusb_mdio *mvusb;
> +	struct mii_bus *mdio;
> +
> +	mdio = devm_mdiobus_alloc_size(dev, sizeof(*mvusb));

You allocate a bigger buffer here than the original pointer thinks it is
pointing to?

> +	if (!mdio)
> +		return -ENOMEM;
> +
> +	mvusb = mdio->priv;

And then you set this pointer here?

If that's the way this is supposed to work, that's fine, just feels like
the math is wrong somewhere...

thanks,

greg k-h