[PATCH net-next] net: mdio: access c22 registers via debugfs

Provide a debugging interface to read and write MDIO registers directly
without the need for a device driver.

This is extremely useful when debugging switch hardware and phy hardware
issues. The interface provides proper locking for communication that
consists of a sequence of MDIO read/write commands.

The interface binds directly to the MDIO bus abstraction in order to
provide support for all devices whether there's a hardware driver for
them or not. Registers are written by writing address, offset, and
value in hex, separated by colon. Registeres are read by writing only
address and offset, then reading the value.

It can be easily tested using `socat`:

    # socat - /sys/kernel/debug/mdio/f802c000.ethernet-ffffffff/control

Example: Reading address 0x00 offset 0x00, value is 0x3000

    Input: 00:00
    Output: 3000

Example: Writing address 0x00 offset 0x00, value 0x2100

    Input: 00:00:2100

Signed-off-by: Pavel Šimerda <redacted>
---
 drivers/net/phy/Makefile       |   1 +
 drivers/net/phy/mdio-debugfs.c | 196 +++++++++++++++++++++++++++++++++
 drivers/net/phy/mdio-debugfs.h |   2 +
 drivers/net/phy/mdio_bus.c     |   5 +
 include/linux/phy.h            |   3 +
 5 files changed, 207 insertions(+)
 create mode 100644 drivers/net/phy/mdio-debugfs.c
 create mode 100644 drivers/net/phy/mdio-debugfs.h

diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index a13e402074cf..4999cb97844b 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile

@@ -17,6 +17,7 @@ libphy-y			+= $(mdio-bus-y)
 else
 obj-$(CONFIG_MDIO_DEVICE)	+= mdio-bus.o
 endif
+obj-$(CONFIG_MDIO_DEVICE)	+= mdio-debugfs.o
 obj-$(CONFIG_MDIO_DEVRES)	+= mdio_devres.o
 libphy-$(CONFIG_SWPHY)		+= swphy.o
 libphy-$(CONFIG_LED_TRIGGER_PHY)	+= phy_led_triggers.o

diff --git a/drivers/net/phy/mdio-debugfs.c b/drivers/net/phy/mdio-debugfs.c
new file mode 100644
index 000000000000..abed40052c20
--- /dev/null
+++ b/drivers/net/phy/mdio-debugfs.c

@@ -0,0 +1,196 @@
+#include <linux/module.h>
+#include <linux/debugfs.h>
+#include <linux/phy.h>
+#include <linux/poll.h>
+
+/*
+ * TODO: May need locking implementation to avoid being susceptible to file
+ * descriptor sharing concurrency issues
+ */
+struct mdio_debug {
+	wait_queue_head_t queue;
+	int value;
+};
+
+static int mdio_debug_open(struct inode *inode, struct file *file)
+{
+	struct mii_bus *bus = file->f_inode->i_private;
+	struct mdio_debug *data;
+	int err;
+
+	data = kzalloc(sizeof *data, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	err = mutex_lock_interruptible(&bus->mdio_lock);
+	if (err) {
+		kfree(data);
+		return err;
+	}
+	dev_dbg(&bus->dev, "MDIO locked for user space program.\n");
+
+	init_waitqueue_head(&data->queue);
+	data->value = -1;
+	file->private_data = data;
+
+	return 0;
+}
+
+static int mdio_debug_release(struct inode *inode, struct file *file)
+{
+	struct mii_bus *bus = file->f_inode->i_private;
+	struct mdio_debug *data = file->private_data;
+
+	file->private_data = NULL;
+
+	mutex_unlock(&bus->mdio_lock);
+	dev_dbg(&bus->dev, "MDIO unlocked.\n");
+
+	kfree(data);
+
+	return 0;
+}
+
+static ssize_t mdio_debug_write(struct file *file, const char __user *buffer, size_t size, loff_t *off)
+{
+	struct mii_bus *bus = file->f_inode->i_private;
+	struct mdio_debug *data = file->private_data;
+	char str[64] = {};
+	char *s = str;
+	char *token;
+	int addr;
+	int offset;
+	int value;
+	int ret;
+
+	if (data->value != -1)
+		return -EWOULDBLOCK;
+
+	if (size > sizeof str - 1)
+		return -EINVAL;
+
+	ret = copy_from_user(str, buffer, size);
+	if (ret)
+		return -EFAULT;
+
+	if (str[size-1] == '\n')
+		str[size-1] = '\0';
+
+	token = strsep(&s, ":");
+	if (!token)
+		return -EINVAL;
+	ret = kstrtoint(token, 16, &addr);
+	if (ret)
+		return ret;
+
+	token = strsep(&s, ":");
+	if (!token)
+		return -EINVAL;
+	ret = kstrtoint(token, 16, &offset);
+	if (ret)
+		return ret;
+
+	token = strsep(&s, ":");
+
+	if (token) {
+		ret = kstrtoint(token, 16, &value);
+		if (ret)
+			return ret;
+
+		ret = __mdiobus_write(bus, addr, offset, value);
+		if (ret)
+			return ret;
+
+		dev_dbg(&bus->dev, "write: addr=0x%.2x offset=0x%.2x value=%.4x\n",
+			addr, offset, value);
+	} else {
+		value = __mdiobus_read(bus, addr, offset);
+		if (value < 0)
+			return value;
+
+		dev_dbg(&bus->dev, "read: addr=0x%.2x offset=0x%.2x value=%.4x\n",
+			addr, offset, value);
+
+		data->value = value;
+		wake_up_all(&data->queue);
+	}
+
+	return size;
+}
+
+static ssize_t mdio_debug_read(struct file *file, char __user *buffer, size_t size, loff_t *off)
+{
+	struct mdio_debug *data = file->private_data;
+	char str[6];
+	int ret;
+	ssize_t rsize;
+
+	if (data->value == -1)
+		return -EWOULDBLOCK;
+
+	rsize = snprintf(str, sizeof str, "%04x\n", data->value);
+	if (rsize > size)
+		return -EINVAL;
+
+	ret = copy_to_user(buffer, str, rsize);
+	if (ret)
+		return -EFAULT;
+
+	data->value = -1;
+	wake_up_all(&data->queue);
+
+	return rsize;
+}
+
+static unsigned int mdio_debug_poll(struct file *file, poll_table *wait)
+{
+	struct mdio_debug *data = file->private_data;
+
+	poll_wait(file, &data->queue, wait);
+
+	return data->value == -1 ? POLLOUT : POLLIN;
+}
+
+struct file_operations mdio_debug_fops = {
+	.owner = THIS_MODULE,
+	.open = mdio_debug_open,
+	.release = mdio_debug_release,
+	.write = mdio_debug_write,
+	.read = mdio_debug_read,
+	.poll = mdio_debug_poll,
+};
+
+/*
+ * TODO: This implementation doesn't support module load/unload and has no
+ * error checking.
+ */
+
+static struct dentry *mdio_debugfs_dentry;
+
+void mdio_debugfs_add(struct mii_bus *bus)
+{
+	bus->debugfs_dentry = debugfs_create_dir(dev_name(&bus->dev), mdio_debugfs_dentry);
+	debugfs_create_file("control", 0600, bus->debugfs_dentry, bus, &mdio_debug_fops);
+}
+EXPORT_SYMBOL_GPL(mdio_debugfs_add);
+
+void mdio_debugfs_remove(struct mii_bus *bus)
+{
+	debugfs_remove(bus->debugfs_dentry);
+	bus->debugfs_dentry = NULL;
+}
+EXPORT_SYMBOL_GPL(mdio_debugfs_remove);
+
+int __init mdio_debugfs_init(void)
+{
+	mdio_debugfs_dentry = debugfs_create_dir("mdio", NULL);
+
+	return PTR_ERR_OR_ZERO(mdio_debugfs_dentry);
+}
+module_init(mdio_debugfs_init);
+
+void __exit mdio_debugfs_exit(void)
+{
+	debugfs_remove(mdio_debugfs_dentry);
+}
+module_exit(mdio_debugfs_exit);

diff --git a/drivers/net/phy/mdio-debugfs.h b/drivers/net/phy/mdio-debugfs.h
new file mode 100644
index 000000000000..fe98dcdfcacf
--- /dev/null
+++ b/drivers/net/phy/mdio-debugfs.h

@@ -0,0 +1,2 @@
+void mdio_debugfs_add(struct mii_bus *bus);
+void mdio_debugfs_remove(struct mii_bus *bus);

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 040509b81f02..969cb8e1ebee 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)
 {

@@ -581,6 +582,8 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
 
 	mdiobus_setup_mdiodev_from_board_info(bus, mdiobus_create_device);
 
+	mdio_debugfs_add(bus);
+
 	bus->state = MDIOBUS_REGISTERED;
 	pr_info("%s: probed\n", bus->name);
 	return 0;

@@ -612,6 +615,8 @@ void mdiobus_unregister(struct mii_bus *bus)
 	BUG_ON(bus->state != MDIOBUS_REGISTERED);
 	bus->state = MDIOBUS_UNREGISTERED;
 
+	mdio_debugfs_remove(bus);
+
 	for (i = 0; i < PHY_MAX_ADDR; i++) {
 		mdiodev = bus->mdio_map[i];
 		if (!mdiodev)

diff --git a/include/linux/phy.h b/include/linux/phy.h
index 24fcc6456a9e..f4cc93c11006 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h

@@ -371,6 +371,9 @@ struct mii_bus {
 
 	/** @shared: shared state across different PHYs */
 	struct phy_package_shared *shared[PHY_MAX_ADDR];
+
+	/* debugfs file for MDIO access */
+	struct dentry *debugfs_dentry;
 };
 #define to_mii_bus(d) container_of(d, struct mii_bus, dev)
 

-- 
2.29.2

Hi "Pavel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Pavel-imerda/net-mdio-access-c22-registers-via-debugfs/20210117-053409
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 9ab7e76aefc97a9aa664accb59d6e8dc5e52514a
config: powerpc-adder875_defconfig (attached as .config)
compiler: powerpc-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
        # https://github.com/0day-ci/linux/commit/22d74f4896850840331d36d6867f7bc5ce728bbd
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Pavel-imerda/net-mdio-access-c22-registers-via-debugfs/20210117-053409
        git checkout 22d74f4896850840331d36d6867f7bc5ce728bbd
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All warnings (new ones prefixed by >>):

quoted

drivers/net/phy/mdio-debugfs.c:170:6: warning: no previous prototype for 'mdio_debugfs_add' [-Wmissing-prototypes]

     170 | void mdio_debugfs_add(struct mii_bus *bus)
         |      ^~~~~~~~~~~~~~~~

quoted

drivers/net/phy/mdio-debugfs.c:177:6: warning: no previous prototype for 'mdio_debugfs_remove' [-Wmissing-prototypes]

     177 | void mdio_debugfs_remove(struct mii_bus *bus)
         |      ^~~~~~~~~~~~~~~~~~~

quoted

drivers/net/phy/mdio-debugfs.c:184:12: warning: no previous prototype for 'mdio_debugfs_init' [-Wmissing-prototypes]

     184 | int __init mdio_debugfs_init(void)
         |            ^~~~~~~~~~~~~~~~~

quoted

drivers/net/phy/mdio-debugfs.c:192:13: warning: no previous prototype for 'mdio_debugfs_exit' [-Wmissing-prototypes]

     192 | void __exit mdio_debugfs_exit(void)
         |             ^~~~~~~~~~~~~~~~~


vim +/mdio_debugfs_add +170 drivers/net/phy/mdio-debugfs.c

   169	
 > 170	void mdio_debugfs_add(struct mii_bus *bus)
   171	{
   172		bus->debugfs_dentry = debugfs_create_dir(dev_name(&bus->dev), mdio_debugfs_dentry);
   173		debugfs_create_file("control", 0600, bus->debugfs_dentry, bus, &mdio_debug_fops);
   174	}
   175	EXPORT_SYMBOL_GPL(mdio_debugfs_add);
   176	
 > 177	void mdio_debugfs_remove(struct mii_bus *bus)
   178	{
   179		debugfs_remove(bus->debugfs_dentry);
   180		bus->debugfs_dentry = NULL;
   181	}
   182	EXPORT_SYMBOL_GPL(mdio_debugfs_remove);
   183	
 > 184	int __init mdio_debugfs_init(void)
   185	{
   186		mdio_debugfs_dentry = debugfs_create_dir("mdio", NULL);
   187	
   188		return PTR_ERR_OR_ZERO(mdio_debugfs_dentry);
   189	}
   190	module_init(mdio_debugfs_init);
   191	
 > 192	void __exit mdio_debugfs_exit(void)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Hi "Pavel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Pavel-imerda/net-mdio-access-c22-registers-via-debugfs/20210117-053409
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 9ab7e76aefc97a9aa664accb59d6e8dc5e52514a
config: i386-randconfig-s002-20210117 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-208-g46a52ca4-dirty
        # https://github.com/0day-ci/linux/commit/22d74f4896850840331d36d6867f7bc5ce728bbd
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Pavel-imerda/net-mdio-access-c22-registers-via-debugfs/20210117-053409
        git checkout 22d74f4896850840331d36d6867f7bc5ce728bbd
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>


"sparse warnings: (new ones prefixed by >>)"

quoted

drivers/net/phy/mdio-debugfs.c:160:17: sparse: sparse: incorrect type in initializer (different base types) @@     expected restricted __poll_t ( *poll )( ... ) @@     got unsigned int ( * )( ... ) @@

   drivers/net/phy/mdio-debugfs.c:160:17: sparse:     expected restricted __poll_t ( *poll )( ... )
   drivers/net/phy/mdio-debugfs.c:160:17: sparse:     got unsigned int ( * )( ... )

quoted

drivers/net/phy/mdio-debugfs.c:154:24: sparse: sparse: symbol 'mdio_debug_fops' was not declared. Should it be static?
drivers/net/phy/mdio-debugfs.c:184:12: sparse: sparse: symbol 'mdio_debugfs_init' was not declared. Should it be static?
drivers/net/phy/mdio-debugfs.c:192:13: sparse: sparse: symbol 'mdio_debugfs_exit' was not declared. Should it be static?

Please review and possibly fold the followup patch.

vim +160 drivers/net/phy/mdio-debugfs.c

   153	
 > 154	struct file_operations mdio_debug_fops = {
   155		.owner = THIS_MODULE,
   156		.open = mdio_debug_open,
   157		.release = mdio_debug_release,
   158		.write = mdio_debug_write,
   159		.read = mdio_debug_read,
 > 160		.poll = mdio_debug_poll,
   161	};
   162	
   163	/*
   164	 * TODO: This implementation doesn't support module load/unload and has no
   165	 * error checking.
   166	 */
   167	
   168	static struct dentry *mdio_debugfs_dentry;
   169	
   170	void mdio_debugfs_add(struct mii_bus *bus)
   171	{
   172		bus->debugfs_dentry = debugfs_create_dir(dev_name(&bus->dev), mdio_debugfs_dentry);
   173		debugfs_create_file("control", 0600, bus->debugfs_dentry, bus, &mdio_debug_fops);
   174	}
   175	EXPORT_SYMBOL_GPL(mdio_debugfs_add);
   176	
   177	void mdio_debugfs_remove(struct mii_bus *bus)
   178	{
   179		debugfs_remove(bus->debugfs_dentry);
   180		bus->debugfs_dentry = NULL;
   181	}
   182	EXPORT_SYMBOL_GPL(mdio_debugfs_remove);
   183	
 > 184	int __init mdio_debugfs_init(void)
   185	{
   186		mdio_debugfs_dentry = debugfs_create_dir("mdio", NULL);
   187	
   188		return PTR_ERR_OR_ZERO(mdio_debugfs_dentry);
   189	}
   190	module_init(mdio_debugfs_init);
   191	
 > 192	void __exit mdio_debugfs_exit(void)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Reported-by: kernel test robot <redacted>
Signed-off-by: kernel test robot <redacted>
---
 mdio-debugfs.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/mdio-debugfs.c b/drivers/net/phy/mdio-debugfs.c
index abed40052c20a1d..f1c9a3c604714c2 100644
--- a/drivers/net/phy/mdio-debugfs.c
+++ b/drivers/net/phy/mdio-debugfs.c

@@ -151,7 +151,7 @@ static unsigned int mdio_debug_poll(struct file *file, poll_table *wait)
 	return data->value == -1 ? POLLOUT : POLLIN;
 }
 
-struct file_operations mdio_debug_fops = {
+static struct file_operations mdio_debug_fops = {
 	.owner = THIS_MODULE,
 	.open = mdio_debug_open,
 	.release = mdio_debug_release,

@@ -181,7 +181,7 @@ void mdio_debugfs_remove(struct mii_bus *bus)
 }
 EXPORT_SYMBOL_GPL(mdio_debugfs_remove);
 
-int __init mdio_debugfs_init(void)
+static int __init mdio_debugfs_init(void)
 {
 	mdio_debugfs_dentry = debugfs_create_dir("mdio", NULL);
 

@@ -189,7 +189,7 @@ int __init mdio_debugfs_init(void)
 }
 module_init(mdio_debugfs_init);
 
-void __exit mdio_debugfs_exit(void)
+static void __exit mdio_debugfs_exit(void)
 {
 	debugfs_remove(mdio_debugfs_dentry);
 }

Hi "Pavel,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Pavel-imerda/net-mdio-access-c22-registers-via-debugfs/20210117-053409
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 9ab7e76aefc97a9aa664accb59d6e8dc5e52514a
config: s390-randconfig-r015-20210117 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 2082b10d100e8dbaffc2ba8f497db5d2ab61beb2)
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
        # install s390 cross compiling tool for clang build
        # apt-get install binutils-s390x-linux-gnu
        # https://github.com/0day-ci/linux/commit/22d74f4896850840331d36d6867f7bc5ce728bbd
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Pavel-imerda/net-mdio-access-c22-registers-via-debugfs/20210117-053409
        git checkout 22d74f4896850840331d36d6867f7bc5ce728bbd
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All warnings (new ones prefixed by >>):

                              ^
   include/uapi/linux/swab.h:20:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0x0000ff00UL) <<  8) |            \
                     ^
   In file included from drivers/net/phy/mdio-debugfs.c:3:
   In file included from include/linux/phy.h:16:
   In file included from include/linux/ethtool.h:18:
   In file included from include/uapi/linux/ethtool.h:19:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:80:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
           ___constant_swab32(x) :                 \
                              ^
   include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0x00ff0000UL) >>  8) |            \
                     ^
   In file included from drivers/net/phy/mdio-debugfs.c:3:
   In file included from include/linux/phy.h:16:
   In file included from include/linux/ethtool.h:18:
   In file included from include/uapi/linux/ethtool.h:19:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:80:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
           ___constant_swab32(x) :                 \
                              ^
   include/uapi/linux/swab.h:22:12: note: expanded from macro '___constant_swab32'
           (((__u32)(x) & (__u32)0xff000000UL) >> 24)))
                     ^
   In file included from drivers/net/phy/mdio-debugfs.c:3:
   In file included from include/linux/phy.h:16:
   In file included from include/linux/ethtool.h:18:
   In file included from include/uapi/linux/ethtool.h:19:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:80:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
                                                             ^
   include/uapi/linux/swab.h:120:12: note: expanded from macro '__swab32'
           __fswab32(x))
                     ^
   In file included from drivers/net/phy/mdio-debugfs.c:3:
   In file included from include/linux/phy.h:16:
   In file included from include/linux/ethtool.h:18:
   In file included from include/uapi/linux/ethtool.h:19:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:31:
   In file included from include/linux/dma-mapping.h:10:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/s390/include/asm/io.h:80:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsb(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsw(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           readsl(PCI_IOBASE + addr, buffer, count);
                  ~~~~~~~~~~ ^
   include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesb(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesw(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^
   include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           writesl(PCI_IOBASE + addr, buffer, count);
                   ~~~~~~~~~~ ^

quoted

drivers/net/phy/mdio-debugfs.c:170:6: warning: no previous prototype for function 'mdio_debugfs_add' [-Wmissing-prototypes]

   void mdio_debugfs_add(struct mii_bus *bus)
        ^
   drivers/net/phy/mdio-debugfs.c:170:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void mdio_debugfs_add(struct mii_bus *bus)
   ^
   static 

quoted

drivers/net/phy/mdio-debugfs.c:177:6: warning: no previous prototype for function 'mdio_debugfs_remove' [-Wmissing-prototypes]

   void mdio_debugfs_remove(struct mii_bus *bus)
        ^
   drivers/net/phy/mdio-debugfs.c:177:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void mdio_debugfs_remove(struct mii_bus *bus)
   ^
   static 

quoted

drivers/net/phy/mdio-debugfs.c:184:12: warning: no previous prototype for function 'mdio_debugfs_init' [-Wmissing-prototypes]

   int __init mdio_debugfs_init(void)
              ^
   drivers/net/phy/mdio-debugfs.c:184:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int __init mdio_debugfs_init(void)
   ^
   static 

quoted

drivers/net/phy/mdio-debugfs.c:192:13: warning: no previous prototype for function 'mdio_debugfs_exit' [-Wmissing-prototypes]

   void __exit mdio_debugfs_exit(void)
               ^
   drivers/net/phy/mdio-debugfs.c:192:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void __exit mdio_debugfs_exit(void)
   ^
   static 
   24 warnings generated.


vim +/mdio_debugfs_add +170 drivers/net/phy/mdio-debugfs.c

   169	
 > 170	void mdio_debugfs_add(struct mii_bus *bus)
   171	{
   172		bus->debugfs_dentry = debugfs_create_dir(dev_name(&bus->dev), mdio_debugfs_dentry);
   173		debugfs_create_file("control", 0600, bus->debugfs_dentry, bus, &mdio_debug_fops);
   174	}
   175	EXPORT_SYMBOL_GPL(mdio_debugfs_add);
   176	
 > 177	void mdio_debugfs_remove(struct mii_bus *bus)
   178	{
   179		debugfs_remove(bus->debugfs_dentry);
   180		bus->debugfs_dentry = NULL;
   181	}
   182	EXPORT_SYMBOL_GPL(mdio_debugfs_remove);
   183	
 > 184	int __init mdio_debugfs_init(void)
   185	{
   186		mdio_debugfs_dentry = debugfs_create_dir("mdio", NULL);
   187	
   188		return PTR_ERR_OR_ZERO(mdio_debugfs_dentry);
   189	}
   190	module_init(mdio_debugfs_init);
   191	
 > 192	void __exit mdio_debugfs_exit(void)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

On Sat, Jan 16, 2021 at 22:19, Pavel Šimerda [off-list ref] wrote:

Provide a debugging interface to read and write MDIO registers directly
without the need for a device driver.

This is extremely useful when debugging switch hardware and phy hardware
issues. The interface provides proper locking for communication that
consists of a sequence of MDIO read/write commands.

The interface binds directly to the MDIO bus abstraction in order to
provide support for all devices whether there's a hardware driver for
them or not. Registers are written by writing address, offset, and
value in hex, separated by colon. Registeres are read by writing only
address and offset, then reading the value.

It can be easily tested using `socat`:

    # socat - /sys/kernel/debug/mdio/f802c000.ethernet-ffffffff/control

Example: Reading address 0x00 offset 0x00, value is 0x3000

    Input: 00:00
    Output: 3000

Example: Writing address 0x00 offset 0x00, value 0x2100

    Input: 00:00:2100

Signed-off-by: Pavel Šimerda <redacted>

Hi Pavel,

I also tried my luck at adding an MDIO debug interface to the kernel a
while back:

https://lore.kernel.org/netdev/C42DZQLTPHM5.2THDSRK84BI3T@wkz-x280

The conclusion was that, while nice to have, it makes it too easy for
shady vendors to write out-of-tree drivers.

You might want to have a look at https://github.com/wkz/mdio-tools. It
solves the same issue that your debugfs interface does, and also some
other nice things like clause 45 addressing and atomic read/mask/write
operations.

On 1/19/21 12:27 AM, Tobias Waldekranz wrote:

On Sat, Jan 16, 2021 at 22:19, Pavel Šimerda [off-list ref] wrote:

quoted

Provide a debugging interface to read and write MDIO registers directly
without the need for a device driver.

This is extremely useful when debugging switch hardware and phy hardware
issues. The interface provides proper locking for communication that
consists of a sequence of MDIO read/write commands.

The interface binds directly to the MDIO bus abstraction in order to
provide support for all devices whether there's a hardware driver for
them or not. Registers are written by writing address, offset, and
value in hex, separated by colon. Registeres are read by writing only
address and offset, then reading the value.

It can be easily tested using `socat`:

     # socat - /sys/kernel/debug/mdio/f802c000.ethernet-ffffffff/control

Example: Reading address 0x00 offset 0x00, value is 0x3000

     Input: 00:00
     Output: 3000

Example: Writing address 0x00 offset 0x00, value 0x2100

     Input: 00:00:2100

Signed-off-by: Pavel Šimerda <redacted>

Hi Pavel,

I also tried my luck at adding an MDIO debug interface to the kernel a
while back:

https://lore.kernel.org/netdev/C42DZQLTPHM5.2THDSRK84BI3T@wkz-x280

Hey Tobias,

nice to meet you!

The conclusion was that, while nice to have, it makes it too easy for
shady vendors to write out-of-tree drivers.

That was exactly what I was afraid of.

You might want to have a look at https://github.com/wkz/mdio-tools. It
solves the same issue that your debugfs interface does, and also some
other nice things like clause 45 addressing and atomic read/mask/write
operations.

Thank you very much!

Cheers,

Pavel

Hi Tobias,

given the reasons stated in the mailing list, I'd like to discuss the situation off-list. I would be more than happy to join your effort and provide an OpenWRT package. I understand the motivation to reject that, and I do use it partially also for the “bad purpose” and therefore I'd like to solve it as consistently as possible.

I'm working with mv88e6xxx where c45 can be coded in c22 anyway, so I didn't care to implement it in the MDIO driver. I'd like to share with you the user space script I'm using to access both mv88e6xxx and direct PHY registers.

I see you're working with mv88e6xxx as well. Can you access all of the inderect registers up to multichip+indirect+paged/c45 registers even without the mv88e6xxx driver loaded, or not? I'm using this feature to bootstrap the switch and get it onto the network when the driver doesn't work yet.

I've seen a few new patches submitted to next-next regarding mv88e6393 and the lag support. I'm also going to explore MSTP and more. I published some of my changes that might not be accepted upstream any soon, or like this one, at all:

https://github.com/switchwrt

Regards,

Pavel

My mdio acces script attached.

Regards,

Pavel


On 1/19/21 2:08 AM, Pavel Šimerda wrote:

Hi Tobias,

given the reasons stated in the mailing list, I'd like to discuss the situation off-list. I would be more than happy to join your effort and provide an OpenWRT package. I understand the motivation to reject that, and I do use it partially also for the “bad purpose” and therefore I'd like to solve it as consistently as possible.

I'm working with mv88e6xxx where c45 can be coded in c22 anyway, so I didn't care to implement it in the MDIO driver. I'd like to share with you the user space script I'm using to access both mv88e6xxx and direct PHY registers.

I see you're working with mv88e6xxx as well. Can you access all of the inderect registers up to multichip+indirect+paged/c45 registers even without the mv88e6xxx driver loaded, or not? I'm using this feature to bootstrap the switch and get it onto the network when the driver doesn't work yet.

I've seen a few new patches submitted to next-next regarding mv88e6393 and the lag support. I'm also going to explore MSTP and more. I published some of my changes that might not be accepted upstream any soon, or like this one, at all:

https://github.com/switchwrt

Regards,

Pavel