Commit ac3a68d5 authored by Bartosz Golaszewski's avatar Bartosz Golaszewski Committed by David S. Miller
Browse files

net: phy: don't abuse devres in devm_mdiobus_register() 



We currently have two managed helpers for mdiobus - devm_mdiobus_alloc()
and devm_mdiobus_register(). The idea behind devres is that the release
callback releases whatever resource the devm function allocates. In the
mdiobus case however there's no devres associated with the device by
devm_mdiobus_register(). Instead the release callback for
devm_mdiobus_alloc(): _devm_mdiobus_free() unregisters the device if
it is marked as managed.

This all seems wrong. The managed structure shouldn't need to know or
care about whether it's managed or not - and this is the case now for
struct mii_bus. The devres wrapper should be opaque to the managed
resource.

This changeset makes devm_mdiobus_alloc() and devm_mdiobus_register()
conform to common devres standards: devm_mdiobus_alloc() allocates a
devres structure and registers a callback that will call mdiobus_free().
__devm_mdiobus_register() allocated another devres and registers a
callback that will unregister the bus.

Signed-off-by: default avatarBartosz Golaszewski <bgolaszewski@baylibre.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 6a9a5723

Documentation/driver-api/driver-model/devres.rst

+0 −1
Original line number Diff line number Diff line
@@ -342,7 +342,6 @@ LED 
MDIO

  devm_mdiobus_alloc()

  devm_mdiobus_alloc_size()

  devm_mdiobus_free()

  devm_mdiobus_register()



MEM

drivers/net/ethernet/realtek/r8169_main.c

+1 −1
Original line number Diff line number Diff line
@@ -5012,7 +5012,7 @@ static int r8169_mdio_register(struct rtl8169_private *tp) 
	new_bus->read = r8169_mdio_read_reg;

	new_bus->write = r8169_mdio_write_reg;



	ret = devm_mdiobus_register(new_bus);

	ret = devm_mdiobus_register(&pdev->dev, new_bus);

	if (ret)

		return ret;

drivers/net/phy/mdio_bus.c

+0 −73
Original line number Diff line number Diff line
@@ -165,79 +165,6 @@ struct mii_bus *mdiobus_alloc_size(size_t size) 
}

EXPORT_SYMBOL(mdiobus_alloc_size);



static void _devm_mdiobus_free(struct device *dev, void *res)

{

	struct mii_bus *bus = *(struct mii_bus **)res;



	if (bus->is_managed_registered && bus->state == MDIOBUS_REGISTERED)

		mdiobus_unregister(bus);



	mdiobus_free(bus);

}



static int devm_mdiobus_match(struct device *dev, void *res, void *data)

{

	struct mii_bus **r = res;



	if (WARN_ON(!r || !*r))

		return 0;



	return *r == data;

}



/**

 * devm_mdiobus_alloc_size - Resource-managed mdiobus_alloc_size()

 * @dev:		Device to allocate mii_bus for

 * @sizeof_priv:	Space to allocate for private structure.

 *

 * Managed mdiobus_alloc_size. mii_bus allocated with this function is

 * automatically freed on driver detach.

 *

 * If an mii_bus allocated with this function needs to be freed separately,

 * devm_mdiobus_free() must be used.

 *

 * RETURNS:

 * Pointer to allocated mii_bus on success, NULL on failure.

 */

struct mii_bus *devm_mdiobus_alloc_size(struct device *dev, int sizeof_priv)

{

	struct mii_bus **ptr, *bus;



	ptr = devres_alloc(_devm_mdiobus_free, sizeof(*ptr), GFP_KERNEL);

	if (!ptr)

		return NULL;



	/* use raw alloc_dr for kmalloc caller tracing */

	bus = mdiobus_alloc_size(sizeof_priv);

	if (bus) {

		*ptr = bus;

		devres_add(dev, ptr);

		bus->is_managed = 1;

	} else {

		devres_free(ptr);

	}



	return bus;

}

EXPORT_SYMBOL_GPL(devm_mdiobus_alloc_size);



/**

 * devm_mdiobus_free - Resource-managed mdiobus_free()

 * @dev:		Device this mii_bus belongs to

 * @bus:		the mii_bus associated with the device

 *

 * Free mii_bus allocated with devm_mdiobus_alloc_size().

 */

void devm_mdiobus_free(struct device *dev, struct mii_bus *bus)

{

	int rc;



	rc = devres_release(dev, _devm_mdiobus_free,

			    devm_mdiobus_match, bus);

	WARN_ON(rc);

}

EXPORT_SYMBOL_GPL(devm_mdiobus_free);



/**

 * mdiobus_release - mii_bus device release callback

 * @d: the target struct device that contains the mii_bus

drivers/net/phy/mdio_devres.c

+77 −6
Original line number Diff line number Diff line 
// SPDX-License-Identifier: GPL-2.0-or-later



#include <linux/device.h>

#include <linux/phy.h>

#include <linux/stddef.h>



struct mdiobus_devres {

	struct mii_bus *mii;

};



static void devm_mdiobus_free(struct device *dev, void *this)

{

	struct mdiobus_devres *dr = this;



	mdiobus_free(dr->mii);

}



/**

 * devm_mdiobus_alloc_size - Resource-managed mdiobus_alloc_size()

 * @dev:		Device to allocate mii_bus for

 * @sizeof_priv:	Space to allocate for private structure

 *

 * Managed mdiobus_alloc_size. mii_bus allocated with this function is

 * automatically freed on driver detach.

 *

 * RETURNS:

 * Pointer to allocated mii_bus on success, NULL on out-of-memory error.

 */

struct mii_bus *devm_mdiobus_alloc_size(struct device *dev, int sizeof_priv)

{

	struct mdiobus_devres *dr;



	dr = devres_alloc(devm_mdiobus_free, sizeof(*dr), GFP_KERNEL);

	if (!dr)

		return NULL;



	dr->mii = mdiobus_alloc_size(sizeof_priv);

	if (!dr->mii) {

		devres_free(dr);

		return NULL;

	}



	devres_add(dev, dr);

	return dr->mii;

}

EXPORT_SYMBOL(devm_mdiobus_alloc_size);



static void devm_mdiobus_unregister(struct device *dev, void *this)

{

	struct mdiobus_devres *dr = this;



	mdiobus_unregister(dr->mii);

}



static int mdiobus_devres_match(struct device *dev,

				void *this, void *match_data)

{

	struct mdiobus_devres *res = this;

	struct mii_bus *mii = match_data;



	return mii == res->mii;

}



/**

 * __devm_mdiobus_register - Resource-managed variant of mdiobus_register()

 * @dev:	Device to register mii_bus for

 * @bus:	MII bus structure to register

 * @owner:	Owning module

 *

 * Returns 0 on success, negative error number on failure.

 */

int __devm_mdiobus_register(struct mii_bus *bus, struct module *owner)

int __devm_mdiobus_register(struct device *dev, struct mii_bus *bus,

			    struct module *owner)

{

	struct mdiobus_devres *dr;

	int ret;



	if (!bus->is_managed)

		return -EPERM;

	if (WARN_ON(!devres_find(dev, devm_mdiobus_free,

				 mdiobus_devres_match, bus)))

		return -EINVAL;



	ret = __mdiobus_register(bus, owner);

	if (!ret)

		bus->is_managed_registered = 1;

	dr = devres_alloc(devm_mdiobus_unregister, sizeof(*dr), GFP_KERNEL);

	if (!dr)

		return -ENOMEM;



	ret = __mdiobus_register(bus, owner);

	if (ret) {

		devres_free(dr);

		return ret;

	}



	dr->mii = bus;

	devres_add(dev, dr);

	return 0;

}

EXPORT_SYMBOL(__devm_mdiobus_register);

include/linux/phy.h

+4 −6
Original line number Diff line number Diff line
@@ -261,9 +261,6 @@ struct mii_bus { 
	int (*reset)(struct mii_bus *bus);

	struct mdio_bus_stats stats[PHY_MAX_ADDR];



	unsigned int is_managed:1;	/* is device-managed */

	unsigned int is_managed_registered:1;



	/*

	 * A lock to ensure that only one thing can read/write

	 * the MDIO bus at a time
@@ -322,9 +319,11 @@ static inline struct mii_bus *mdiobus_alloc(void) 
}



int __mdiobus_register(struct mii_bus *bus, struct module *owner);

int __devm_mdiobus_register(struct mii_bus *bus, struct module *owner);

int __devm_mdiobus_register(struct device *dev, struct mii_bus *bus,

			    struct module *owner);

#define mdiobus_register(bus) __mdiobus_register(bus, THIS_MODULE)

#define devm_mdiobus_register(bus) __devm_mdiobus_register(bus, THIS_MODULE)

#define devm_mdiobus_register(dev, bus) \

		__devm_mdiobus_register(dev, bus, THIS_MODULE)



void mdiobus_unregister(struct mii_bus *bus);

void mdiobus_free(struct mii_bus *bus);
@@ -335,7 +334,6 @@ static inline struct mii_bus *devm_mdiobus_alloc(struct device *dev) 
}



struct mii_bus *mdio_find_bus(const char *mdio_name);

void devm_mdiobus_free(struct device *dev, struct mii_bus *bus);

struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr);



#define PHY_INTERRUPT_DISABLED	false

CRA Git | Maintained and supported by SUSTech CRA and CCSE