mtip32xx: Changes to sysfs entries
PATCH 10/11 mtip32xx: Changes to sysfs entries
* Formatted the output of 'registers' entry
* Added "Commands in Q' to output of 'registers' entry
* Added a new entry 'flags'
--- Documentation/ABI/testing/sysfs-block-rssd | 12 ++++- drivers/block/mtip32xx/mtip32xx.c | 76 +++++++++++++++++++++------- 2 files changed, 67 insertions(+), 21 deletions(-)
Documentation/ABI/testing/sysfs-block-rssd +10 -2
@@ -6,13 +6,21 @@ Description: This is a read-only file. Dumps below driver information and hardware registers. - S ACTive - Command Issue - - Allocated - Completed - PORT IRQ STAT - HOST IRQ STAT + - Allocated + - Commands in Q What: /sys/block/rssd*/status Date: April 2012 KernelVersion: 3.4 Contact: Asai Thambi S P <[email protected]> -Description: This is a read-only file. Indicates the status of the device. +Description: This is a read-only file. Indicates the status of the device. + +What: /sys/block/rssd*/flags +Date: May 2012 +KernelVersion: 3.5 +Contact: Asai Thambi S P <[email protected]> +Description: This is a read-only file. Dumps the flags in port and driver + data structure
drivers/block/mtip32xx/mtip32xx.c +57 -19
117 lines not on this page · raw message
Signature · 1 line
1.7.1
Quoted 46 lines · Asai Thambi S P
* Formatted the output of 'registers' entry
* Added "Commands in Q' to output of 'registers' entry
* Added a new entry 'flags'Signed-off-by Asai Thambi S P <[email protected]>--- Documentation/ABI/testing/sysfs-block-rssd | 12 ++++- drivers/block/mtip32xx/mtip32xx.c | 76 +++++++++++++++++++++------- 2 files changed, 67 insertions(+), 21 deletions(-)Documentation/ABI/testing/sysfs-block-rssd +10
-2@@ -6,13 +6,21 @@ Description: This is a read-only file. Dumps below driver information and hardware registers. - S ACTive - Command Issue - - Allocated - Completed - PORT IRQ STAT - HOST IRQ STAT + - Allocated + - Commands in Q What: /sys/block/rssd*/status Date: April 2012 KernelVersion: 3.4 Contact: Asai Thambi S P <[email protected]> -Description: This is a read-only file. Indicates the status of the device. +Description: This is a read-only file. Indicates the status of the device. + +What: /sys/block/rssd*/flags +Date: May 2012 +KernelVersion: 3.5 +Contact: Asai Thambi S P <[email protected]> +Description: This is a read-only file. Dumps the flags in port and driver + data structuredrivers/block/mtip32xx/mtip32xx.c +0
-0@@ -2562,40 +2562,58 @@ static ssize_t mtip_hw_show_registers(struct device *dev,int size =0 ;int n;
drivers/block/mtip32xx/mtip32xx.c static ssize_t mtip_hw_show_registers(struct device *dev, · line 2570
Quoted 10 lines
drivers/block/mtip32xx/mtip32xx.c +4
-3- size +=sprintf (&buf[size],"S ACTive:\n" ); + size +=sprintf (&buf[size],"Hardware\n--------\n" ); + size +=sprintf (&buf[size],"S ACTive : [ 0x" ); -for (n =0 ; n < dd->slot_groups; n++) - size +=sprintf (&buf[size],"0x%08x\n" , +for (n = dd->slot_groups-1 ; n >=0 ; n--) + size +=sprintf (&buf[size],"%08X " ,readl (dd->port->s_active[n]));
<snip>
WHAT???
No, this needs to be a debugfs file, sysfs is "one value per file", stuff like this is not allowed at all.
Please remove these sysfs files entirely, they are not allowed. If you feel you really need them, put them in debugfs.
As-is, this patch is not acceptable, and a follow-on patch needs to be created to remove these sysfs files now.
thanks,
greg k-h
Jens Axboe
maintainerQuoted 62 lines · Greg KH
Quoted 46 lines · Asai Thambi S P
* Formatted the output of 'registers' entry
* Added "Commands in Q' to output of 'registers' entry
* Added a new entry 'flags'Signed-off-by Asai Thambi S P <[email protected]>--- Documentation/ABI/testing/sysfs-block-rssd | 12 ++++- drivers/block/mtip32xx/mtip32xx.c | 76 +++++++++++++++++++++------- 2 files changed, 67 insertions(+), 21 deletions(-)Documentation/ABI/testing/sysfs-block-rssd +10
-2@@ -6,13 +6,21 @@ Description: This is a read-only file. Dumps below driver information and hardware registers. - S ACTive - Command Issue - - Allocated - Completed - PORT IRQ STAT - HOST IRQ STAT + - Allocated + - Commands in Q What: /sys/block/rssd*/status Date: April 2012 KernelVersion: 3.4 Contact: Asai Thambi S P <[email protected]> -Description: This is a read-only file. Indicates the status of the device. +Description: This is a read-only file. Indicates the status of the device. + +What: /sys/block/rssd*/flags +Date: May 2012 +KernelVersion: 3.5 +Contact: Asai Thambi S P <[email protected]> +Description: This is a read-only file. Dumps the flags in port and driver + data structuredrivers/block/mtip32xx/mtip32xx.c +0
-0@@ -2562,40 +2562,58 @@ static ssize_t mtip_hw_show_registers(struct device *dev,int size =0 ;int n;drivers/block/mtip32xx/mtip32xx.c static ssize_t mtip_hw_show_registers(struct device *dev, · line 2570
Quoted 10 lines
drivers/block/mtip32xx/mtip32xx.c +4
-3- size +=sprintf (&buf[size],"S ACTive:\n" ); + size +=sprintf (&buf[size],"Hardware\n--------\n" ); + size +=sprintf (&buf[size],"S ACTive : [ 0x" ); -for (n =0 ; n < dd->slot_groups; n++) - size +=sprintf (&buf[size],"0x%08x\n" , +for (n = dd->slot_groups-1 ; n >=0 ; n--) + size +=sprintf (&buf[size],"%08X " ,readl (dd->port->s_active[n]));<snip>
WHAT???
Quoted 8 lines
No, this needs to be a debugfs file, sysfs is "one value per file", stuff like this is not allowed at all.
Please remove these sysfs files entirely, they are not allowed. If you feel you really need them, put them in debugfs.
As-is, this patch is not acceptable, and a follow-on patch needs to be created to remove these sysfs files now.
True, it really should be moved to debugfs. Not only because of the multiple value thing (not going to render my personal opinion on that), but because it is indeed debug functionality. It does not constitute a real API of any sort.
The file is already there, the patch is only changing the contents. It wasn't a single value thing before either. So Asai, I'd suggest you guys send a followup patch moving it to debugfs.
Signature · 1 line
Jens Axboe
Quoted 72 lines · Jens Axboe
Quoted 62 lines · Greg KH
Quoted 46 lines · Asai Thambi S P
* Formatted the output of 'registers' entry
* Added "Commands in Q' to output of 'registers' entry
* Added a new entry 'flags'Signed-off-by Asai Thambi S P <[email protected]>--- Documentation/ABI/testing/sysfs-block-rssd | 12 ++++- drivers/block/mtip32xx/mtip32xx.c | 76 +++++++++++++++++++++------- 2 files changed, 67 insertions(+), 21 deletions(-)Documentation/ABI/testing/sysfs-block-rssd +10
-2@@ -6,13 +6,21 @@ Description: This is a read-only file. Dumps below driver information and hardware registers. - S ACTive - Command Issue - - Allocated - Completed - PORT IRQ STAT - HOST IRQ STAT + - Allocated + - Commands in Q What: /sys/block/rssd*/status Date: April 2012 KernelVersion: 3.4 Contact: Asai Thambi S P <[email protected]> -Description: This is a read-only file. Indicates the status of the device. +Description: This is a read-only file. Indicates the status of the device. + +What: /sys/block/rssd*/flags +Date: May 2012 +KernelVersion: 3.5 +Contact: Asai Thambi S P <[email protected]> +Description: This is a read-only file. Dumps the flags in port and driver + data structuredrivers/block/mtip32xx/mtip32xx.c +0
-0@@ -2562,40 +2562,58 @@ static ssize_t mtip_hw_show_registers(struct device *dev,int size =0 ;int n;drivers/block/mtip32xx/mtip32xx.c static ssize_t mtip_hw_show_registers(struct device *dev, · line 2570
Quoted 10 lines
drivers/block/mtip32xx/mtip32xx.c +4
-3- size +=sprintf (&buf[size],"S ACTive:\n" ); + size +=sprintf (&buf[size],"Hardware\n--------\n" ); + size +=sprintf (&buf[size],"S ACTive : [ 0x" ); -for (n =0 ; n < dd->slot_groups; n++) - size +=sprintf (&buf[size],"0x%08x\n" , +for (n = dd->slot_groups-1 ; n >=0 ; n--) + size +=sprintf (&buf[size],"%08X " ,readl (dd->port->s_active[n]));<snip>
WHAT???
Quoted 8 lines
No, this needs to be a debugfs file, sysfs is "one value per file", stuff like this is not allowed at all.
Please remove these sysfs files entirely, they are not allowed. If you feel you really need them, put them in debugfs.
As-is, this patch is not acceptable, and a follow-on patch needs to be created to remove these sysfs files now.
Quoted 9 lines
True, it really should be moved to debugfs. Not only because of the multiple value thing (not going to render my personal opinion on that), but because it is indeed debug functionality. It does not constitute a real API of any sort.
The file is already there, the patch is only changing the contents. It wasn't a single value thing before either. So Asai, I'd suggest you guys send a followup patch moving it to debugfs.
I will send out a patch to fix this.
--
Regards,
Asai Thambi