| Message ID | 20200714142213.21365-1-marek.behun@nic.cz |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next,v1,1/2] net: mdiobus: add support to access PHY registers via debugfs | expand |
On Tue, Jul 14, 2020 at 04:22:12PM +0200, Marek Behún wrote: > This adds config option CONFIG_MDIO_BUS_DEBUGFS which, when enabled, > adds support to communicate with the devices connected to the MDIO > via debugfs. > > For every MDIO bus this creates directory > /sys/kernel/debug/mdio_bus/MDIO_BUS_NAME > with files "addr", "reg" and "val". > User can write device address to the "addr" file and register number to > the "reg" file, and then can read the value of the register from the > "val" file, or can write new value by writing to the "val" file. > > This is useful when debugging various PHYs or switches. Hi Marek Please work with Tobias Waldekranz <tobias@waldekranz.com>. I'm not particularly keen on allowing write access to such registers, but it seems like there is demand. But we don't want two ways to do this. > static int mdiobus_register_gpiod(struct mdio_device *mdiodev) > { > @@ -576,6 +577,12 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner) > } > } > > + err = mdiobus_register_debugfs(bus); > + if (err) { > + dev_err(&bus->dev, "mii_bus %s couldn't create debugfs entries\n", bus->id); > + goto error; > + } FYI: You should never error out for debugfs. You should not even check the return values from debugfs calls. Andrew
OMG, this won't even compile, I lost the newly added files mdio_debugfs.c/h when rebasing!!! AAAAAARRGH Please ignore these patches
Hi "Marek,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Marek-Beh-n/net-mdiobus-add-support-to-access-PHY-registers-via-debugfs/20200714-222419
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 07dd1b7e68e4b83a1004b14dffd7e142c0bc79bd
config: ia64-randconfig-r003-20200714 (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
In file included from arch/ia64/include/asm/pgtable.h:154,
from include/linux/pgtable.h:6,
from include/linux/mm.h:32,
from include/linux/bvec.h:13,
from include/linux/skbuff.h:17,
from include/linux/if_ether.h:19,
from include/linux/etherdevice.h:20,
from drivers/net/phy/mdio_bus.c:14:
arch/ia64/include/asm/mmu_context.h: In function 'reload_context':
arch/ia64/include/asm/mmu_context.h:137:41: warning: variable 'old_rr4' set but not used [-Wunused-but-set-variable]
137 | unsigned long rr0, rr1, rr2, rr3, rr4, old_rr4;
| ^~~~~~~
drivers/net/phy/mdio_bus.c: At top level:
>> drivers/net/phy/mdio_bus.c:42:10: fatal error: mdio_debugfs.h: No such file or directory
42 | #include "mdio_debugfs.h"
| ^~~~~~~~~~~~~~~~
compilation terminated.
vim +42 drivers/net/phy/mdio_bus.c
40
41 #include "mdio-boardinfo.h"
> 42 #include "mdio_debugfs.h"
43
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index dd20c2c27c2f..aca4b52225b1 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -24,6 +24,14 @@ if MDIO_BUS config MDIO_DEVRES tristate +config MDIO_BUS_DEBUGFS + bool "MDIO bus debugfs support" + depends on DEBUG_FS + help + This adds support to communicate via the MDIO bus via files in + debugfs. Note that using this on a PHY device that is being handled by + a driver can break the state of the PHY. + config MDIO_ASPEED tristate "ASPEED MDIO bus controller" depends on ARCH_ASPEED || COMPILE_TEST diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index d84bab489a53..4500050faf64 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -5,6 +5,8 @@ libphy-y := phy.o phy-c45.o phy-core.o phy_device.o \ linkmode.o mdio-bus-y += mdio_bus.o mdio_device.o +obj-$(CONFIG_MDIO_BUS_DEBUGFS) += mdio_debugfs.o + ifdef CONFIG_MDIO_DEVICE obj-y += mdio-boardinfo.o endif diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c index 46b33701ad4b..b31fa70dbd95 100644 --- a/drivers/net/phy/mdio_bus.c +++ b/drivers/net/phy/mdio_bus.c @@ -39,6 +39,7 @@ #include <trace/events/mdio.h> #include "mdio-boardinfo.h" +#include "mdio_debugfs.h" static int mdiobus_register_gpiod(struct mdio_device *mdiodev) { @@ -576,6 +577,12 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner) } } + err = mdiobus_register_debugfs(bus); + if (err) { + dev_err(&bus->dev, "mii_bus %s couldn't create debugfs entries\n", bus->id); + goto error; + } + mdiobus_setup_mdiodev_from_board_info(bus, mdiobus_create_device); bus->state = MDIOBUS_REGISTERED; @@ -609,6 +616,8 @@ void mdiobus_unregister(struct mii_bus *bus) BUG_ON(bus->state != MDIOBUS_REGISTERED); bus->state = MDIOBUS_UNREGISTERED; + mdiobus_unregister_debugfs(bus); + for (i = 0; i < PHY_MAX_ADDR; i++) { mdiodev = bus->mdio_map[i]; if (!mdiodev) @@ -1005,12 +1014,23 @@ int __init mdio_bus_init(void) int ret; ret = class_register(&mdio_bus_class); - if (!ret) { - ret = bus_register(&mdio_bus_type); - if (ret) - class_unregister(&mdio_bus_class); - } + if (ret) + return ret; + ret = bus_register(&mdio_bus_type); + if (ret) + goto err_class; + + ret = mdiobus_debugfs_init(); + if (ret) + goto err_bus; + + return 0; + +err_bus: + bus_unregister(&mdio_bus_type); +err_class: + class_unregister(&mdio_bus_class); return ret; } EXPORT_SYMBOL_GPL(mdio_bus_init); @@ -1018,6 +1038,7 @@ EXPORT_SYMBOL_GPL(mdio_bus_init); #if IS_ENABLED(CONFIG_PHYLIB) void mdio_bus_exit(void) { + mdiobus_debugfs_exit(); class_unregister(&mdio_bus_class); bus_unregister(&mdio_bus_type); } diff --git a/include/linux/phy.h b/include/linux/phy.h index 0403eb799913..e281099fb526 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -309,6 +309,11 @@ struct mii_bus { /* shared state across different PHYs */ struct phy_package_shared *shared[PHY_MAX_ADDR]; + +#if IS_ENABLED(CONFIG_MDIO_BUS_DEBUGFS) + /* address and regnum for debugfs */ + u32 debug_addr, debug_reg; +#endif }; #define to_mii_bus(d) container_of(d, struct mii_bus, dev)
This adds config option CONFIG_MDIO_BUS_DEBUGFS which, when enabled, adds support to communicate with the devices connected to the MDIO via debugfs. For every MDIO bus this creates directory /sys/kernel/debug/mdio_bus/MDIO_BUS_NAME with files "addr", "reg" and "val". User can write device address to the "addr" file and register number to the "reg" file, and then can read the value of the register from the "val" file, or can write new value by writing to the "val" file. This is useful when debugging various PHYs or switches. Signed-off-by: Marek Behún <marek.behun@nic.cz> --- drivers/net/phy/Kconfig | 8 ++++++++ drivers/net/phy/Makefile | 2 ++ drivers/net/phy/mdio_bus.c | 31 ++++++++++++++++++++++++++----- include/linux/phy.h | 5 +++++ 4 files changed, 41 insertions(+), 5 deletions(-)