lkml.org 
[lkml]   [2015]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] net: dsa: mv88e6xxx: add write access to debugfs regs file
Hi David,

On Jul 11, 2015, at 2:01 AM, David davem@davemloft.net wrote:

> From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> Date: Thu, 9 Jul 2015 17:13:29 -0400
>
>> 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>
>
> I don't know about this.
>
> This starts to smell like a back door for proprietary userspace SDKs to
> program the switch hardware.
>
> Yes, they can do it via other mechanisms, but we don't have to make it
> any eaiser for them either.

I agree with you and I wouldn't want that neither.

> If you want to poke registers, hack the module just like any other
> person with appropriate privileges can do.

I'm not sure what you mean. Keeping some custom patches in our local tree?

> Frankly, all of this debugfs crap in the DSA drivers smells like poo.
> I don't like it _AT_ _ALL_, and I shouldn't have allowed any of it
> into the tree in the first place.
>
> I might just remove it all myself, it bothers me so much.
>
> Fetching information should be done by well typed, generic, interfaces
> that apply to any similar device or object. All of this debugfs stuff
> smells of hacks and special case crap that's only usable for one
> device type and that makes it the single most terrible interface to
> give to users.

In the meantime, this is really useful for development. i.e. ensuring a good
switchdev/DSA interaction without being able to read and write directly the
hardware VLAN table, is a bit a PITA. A dynamic debugfs looked appropriate.

On the other hand, the mv88e6xxx driver gets cluttered with all this code. I'd
gladly move all this code in a mv88e6xxx-debugfs.c file, and conditionally
compile it with:

mv88e6xxx_drv-$(CONFIG_DEBUG_FS) += mv88e6xxx-debugfs.o

similar to what the i2400m driver does.

Would that be appreciated?

Thanks,
-v


\
 
 \ /
  Last update: 2015-07-11 21:01    [W:0.044 / U:2.404 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site