[PATCH 1/2] net: dsa: mv88e6xxx: add write access to debugfs regs file

[PATCH 1/2] net: dsa: mv88e6xxx: add write access to debugfs regs file

[Vivien Didelot]

Allow write access to the regs file in the debugfs interface, with the
following parameters:

    echo <name> <reg> <value> > regs

Where "name" is the register name (as shown in the header row), "reg" is
the register address (as shown in the first column) and "value" is the
16-bit value. e.g.:

    echo GLOBAL 1a 5550 > regs

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 8c130c0..04e6eb6 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1648,6 +1648,35 @@ static int mv88e6xxx_regs_show(struct seq_file *s, void *p)
 	return 0;
 }
 
+static ssize_t mv88e6xxx_regs_write(struct file *file, const char __user *buf,
+				    size_t count, loff_t *ppos)
+{
+	struct seq_file *s = file->private_data;
+	struct dsa_switch *ds = s->private;
+	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
+	char name[count];
+	unsigned int port, reg, val;
+	int ret;
+
+	ret = sscanf(buf, "%s %x %x", name, &reg, &val);
+	if (ret != 3)
+		return -EINVAL;
+
+	if (reg > 0x1f || val > 0xffff)
+		return -ERANGE;
+
+	if (strcasecmp(name, "GLOBAL") == 0)
+		ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, reg, val);
+	else if (strcasecmp(name, "GLOBAL2") == 0)
+		ret = mv88e6xxx_reg_write(ds, REG_GLOBAL2, reg, val);
+	else if (kstrtouint(name, 10, &port) == 0 && port < ps->num_ports)
+		ret = mv88e6xxx_reg_write(ds, REG_PORT(port), reg, val);
+	else
+		return -EINVAL;
+
+	return ret < 0 ? ret : count;
+}
+
 static int mv88e6xxx_regs_open(struct inode *inode, struct file *file)
 {
 	return single_open(file, mv88e6xxx_regs_show, inode->i_private);
@@ -1656,6 +1685,7 @@ static int mv88e6xxx_regs_open(struct inode *inode, struct file *file)
 static const struct file_operations mv88e6xxx_regs_fops = {
 	.open   = mv88e6xxx_regs_open,
 	.read   = seq_read,
+	.write	= mv88e6xxx_regs_write,
 	.llseek = no_llseek,
 	.release = single_release,
 	.owner  = THIS_MODULE,
@@ -1895,7 +1925,7 @@ int mv88e6xxx_setup_common(struct dsa_switch *ds)
 	ps->dbgfs = debugfs_create_dir(name, NULL);
 	kfree(name);
 
-	debugfs_create_file("regs", S_IRUGO, ps->dbgfs, ds,
+	debugfs_create_file("regs", S_IRUGO | S_IWUSR, ps->dbgfs, ds,
 			    &mv88e6xxx_regs_fops);
 
 	debugfs_create_file("atu", S_IRUGO, ps->dbgfs, ds,
-- 
2.4.5

[PATCH 2/2] net: dsa: mv88e6xxx: add SERDES registers to debugfs

[Vivien Didelot]

Add read and write access to a SERDES column in the regs debugfs file
for the Fiber/SERDES registers.

For instance, imagining that the Fiber Control register value is 0x1940,
you can clear the POWER_DOWN bit (11) with:

    echo SERDES 0 1140 > /sys/kernel/debug/dsa0/regs

Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/net/dsa/mv88e6xxx.c | 11 ++++++++---
 drivers/net/dsa/mv88e6xxx.h |  4 ++++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index 04e6eb6..f1e9ab4 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -1628,16 +1628,18 @@ static int mv88e6xxx_regs_show(struct seq_file *s, void *p)
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
 	int reg, port;
 
-	seq_puts(s, "    GLOBAL GLOBAL2 ");
+	seq_puts(s, "    GLOBAL GLOBAL2 SERDES ");
 	for (port = 0 ; port < ps->num_ports; port++)
 		seq_printf(s, " %2d  ", port);
 	seq_puts(s, "\n");
 
 	for (reg = 0; reg < 32; reg++) {
 		seq_printf(s, "%2x: ", reg);
-		seq_printf(s, " %4x    %4x  ",
+		seq_printf(s, " %4x    %4x    %4x  ",
 			   mv88e6xxx_reg_read(ds, REG_GLOBAL, reg),
-			   mv88e6xxx_reg_read(ds, REG_GLOBAL2, reg));
+			   mv88e6xxx_reg_read(ds, REG_GLOBAL2, reg),
+			   mv88e6xxx_phy_page_read(ds, REG_FIBER_SERDES,
+						   PAGE_FIBER_SERDES, reg));
 
 		for (port = 0 ; port < ps->num_ports; port++)
 			seq_printf(s, "%4x ",
@@ -1669,6 +1671,9 @@ static ssize_t mv88e6xxx_regs_write(struct file *file, const char __user *buf,
 		ret = mv88e6xxx_reg_write(ds, REG_GLOBAL, reg, val);
 	else if (strcasecmp(name, "GLOBAL2") == 0)
 		ret = mv88e6xxx_reg_write(ds, REG_GLOBAL2, reg, val);
+	else if (strcasecmp(name, "SERDES") == 0)
+		ret = mv88e6xxx_phy_page_write(ds, REG_FIBER_SERDES,
+					       PAGE_FIBER_SERDES, reg, val);
 	else if (kstrtouint(name, 10, &port) == 0 && port < ps->num_ports)
 		ret = mv88e6xxx_reg_write(ds, REG_PORT(port), reg, val);
 	else
diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h
index 1bc5a3e..87573c3 100644
--- a/drivers/net/dsa/mv88e6xxx.h
+++ b/drivers/net/dsa/mv88e6xxx.h
@@ -310,6 +310,10 @@
 #define GLOBAL2_QOS_WEIGHT	0x1c
 #define GLOBAL2_MISC		0x1d
 
+/* Fiber/SERDES Registers are located at SMI address F, page 1 */
+#define REG_FIBER_SERDES	0x0f
+#define PAGE_FIBER_SERDES	0x01
+
 struct mv88e6xxx_priv_state {
 	/* When using multi-chip addressing, this mutex protects
 	 * access to the indirect access registers.  (In single-chip
-- 
2.4.5

Re: [PATCH 1/2] net: dsa: mv88e6xxx: add write access to debugfs regs file

[Andrew Lunn]

On Wed, Jul 08, 2015 at 04:36:18PM -0400, Vivien Didelot wrote:
> Allow write access to the regs file in the debugfs interface, with the
> following parameters:
> 
>     echo <name> <reg> <value> > regs
> 
> Where "name" is the register name (as shown in the header row), "reg" is
> the register address (as shown in the first column) and "value" is the
> 16-bit value. e.g.:
> 
>     echo GLOBAL 1a 5550 > regs
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>  drivers/net/dsa/mv88e6xxx.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
> index 8c130c0..04e6eb6 100644
> --- a/drivers/net/dsa/mv88e6xxx.c
> +++ b/drivers/net/dsa/mv88e6xxx.c
> @@ -1648,6 +1648,35 @@ static int mv88e6xxx_regs_show(struct seq_file *s, void *p)
>  	return 0;
>  }
>  
> +static ssize_t mv88e6xxx_regs_write(struct file *file, const char __user *buf,
> +				    size_t count, loff_t *ppos)
> +{
> +	struct seq_file *s = file->private_data;
> +	struct dsa_switch *ds = s->private;
> +	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
> +	char name[count];

Is this safe? Is count somehow limited? If i was to echo 8K to the
file would i not exceed the kernel stack space?

     Andrew

Re: [PATCH 2/2] net: dsa: mv88e6xxx: add SERDES registers to debugfs

[Andrew Lunn]

On Wed, Jul 08, 2015 at 04:36:19PM -0400, Vivien Didelot wrote:
> Add read and write access to a SERDES column in the regs debugfs file
> for the Fiber/SERDES registers.


Humm. I don't really like this. These SERDES registers are in a
complete different address space to the port, globalX registers. I
would suggest adding a new file, 'phy', which allows access to all the
phy registers, and SERDES at 0x0f.

This also makes it simpler to deal with devices which don't have
indirect access to the PHYs via Global 2 offset 0x18 and 0x19. For
such chips, the debugfs file should not be created.

Thanks
	Andrew

Re: [PATCH 1/2] net: dsa: mv88e6xxx: add write access to debugfs regs file

[Vivien Didelot]

Hi Andrew,

----- On Jul 8, 2015, at 4:53 PM, Andrew Lunn andrew@lunn.ch wrote:

> On Wed, Jul 08, 2015 at 04:36:18PM -0400, Vivien Didelot wrote:
>> Allow write access to the regs file in the debugfs interface, with the
>> following parameters:
>> 
>>     echo <name> <reg> <value> > regs
>> 
>> Where "name" is the register name (as shown in the header row), "reg" is
>> the register address (as shown in the first column) and "value" is the
>> 16-bit value. e.g.:
>> 
>>     echo GLOBAL 1a 5550 > regs
>> 
>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>> ---
>>  drivers/net/dsa/mv88e6xxx.c | 32 +++++++++++++++++++++++++++++++-
>>  1 file changed, 31 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
>> index 8c130c0..04e6eb6 100644
>> --- a/drivers/net/dsa/mv88e6xxx.c
>> +++ b/drivers/net/dsa/mv88e6xxx.c
>> @@ -1648,6 +1648,35 @@ static int mv88e6xxx_regs_show(struct seq_file *s, void
>> *p)
>>  	return 0;
>>  }
>>  
>> +static ssize_t mv88e6xxx_regs_write(struct file *file, const char __user *buf,
>> +				    size_t count, loff_t *ppos)
>> +{
>> +	struct seq_file *s = file->private_data;
>> +	struct dsa_switch *ds = s->private;
>> +	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
>> +	char name[count];
> 
> Is this safe? Is count somehow limited? If i was to echo 8K to the
> file would i not exceed the kernel stack space?
> 
>      Andrew

I thought it was. But maybe 32 is a better value here. I'll resend these
two patches with char name[32] instead, tomorrow.

Thanks for your time,
-v