* 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(-)
diff --git a/Documentation/ABI/testing/sysfs-block-rssd b/Documentation/ABI/testing/sysfs-block-rssd
index d535757..679ce35 100644
--- a/Documentation/ABI/testing/sysfs-block-rssd
+++ b/Documentation/ABI/testing/sysfs-block-rssd
@@ -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
diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
index d0fa357..18daa04 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -2562,40 +2562,58 @@ static ssize_t mtip_hw_show_registers(struct device *dev,
int size = 0;
int n;
- 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]));
- size += sprintf(&buf[size], "Command Issue:\n");
+ size += sprintf(&buf[size], "]\n");
+ size += sprintf(&buf[size], "Command Issue : [ 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->cmd_issue[n]));
- size += sprintf(&buf[size], "Allocated:\n");
+ size += sprintf(&buf[size], "]\n");
+ size += sprintf(&buf[size], "Completed : [ 0x");
- for (n = 0; n < dd->slot_groups; n++) {
+ for (n = dd->slot_groups-1; n >= 0; n--)
+ size += sprintf(&buf[size], "%08X ",
+ readl(dd->port->completed[n]));
+
+ size += sprintf(&buf[size], "]\n");
+ size += sprintf(&buf[size], "PORT IRQ STAT : [ 0x%08X ]\n",
+ readl(dd->port->mmio + PORT_IRQ_STAT));
+ size += sprintf(&buf[size], "HOST IRQ STAT : [ 0x%08X ]\n",
+ readl(dd->mmio + HOST_IRQ_STAT));
+ size += sprintf(&buf[size], "\n");
+
+ size += sprintf(&buf[size], "Local\n-----\n");
+ size += sprintf(&buf[size], "Allocated : [ 0x");
+
+ for (n = dd->slot_groups-1; n >= 0; n--) {
if (sizeof(long) > sizeof(u32))
group_allocated =
dd->port->allocated[n/2] >> (32*(n&1));
else
group_allocated = dd->port->allocated[n];
- size += sprintf(&buf[size], "0x%08x\n",
- group_allocated);
+ size += sprintf(&buf[size], "%08X ", group_allocated);
}
+ size += sprintf(&buf[size], "]\n");
- size += sprintf(&buf[size], "Completed:\n");
+ size += sprintf(&buf[size], "Commands in Q: [ 0x");
- for (n = 0; n < dd->slot_groups; n++)
- size += sprintf(&buf[size], "0x%08x\n",
- readl(dd->port->completed[n]));
-
- size += sprintf(&buf[size], "PORT IRQ STAT : 0x%08x\n",
- readl(dd->port->mmio + PORT_IRQ_STAT));
- size += sprintf(&buf[size], "HOST IRQ STAT : 0x%08x\n",
- readl(dd->mmio + HOST_IRQ_STAT));
+ for (n = dd->slot_groups-1; n >= 0; n--) {
+ if (sizeof(long) > sizeof(u32))
+ group_allocated =
+ dd->port->cmds_to_issue[n/2] >> (32*(n&1));
+ else
+ group_allocated = dd->port->cmds_to_issue[n];
+ size += sprintf(&buf[size], "%08X ", group_allocated);
+ }
+ size += sprintf(&buf[size], "]\n");
return size;
}
@@ -2617,8 +2635,24 @@ static ssize_t mtip_hw_show_status(struct device *dev,
return size;
}
+static ssize_t mtip_hw_show_flags(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct driver_data *dd = dev_to_disk(dev)->private_data;
+ int size = 0;
+
+ size += sprintf(&buf[size], "Flag in port struct : [ %08lX ]\n",
+ dd->port->flags);
+ size += sprintf(&buf[size], "Flag in dd struct : [ %08lX ]\n",
+ dd->dd_flag);
+
+ return size;
+}
+
static DEVICE_ATTR(registers, S_IRUGO, mtip_hw_show_registers, NULL);
static DEVICE_ATTR(status, S_IRUGO, mtip_hw_show_status, NULL);
+static DEVICE_ATTR(flags, S_IRUGO, mtip_hw_show_flags, NULL);
/*
* Create the sysfs related attributes.
@@ -2641,6 +2675,9 @@ static int mtip_hw_sysfs_init(struct driver_data *dd, struct kobject *kobj)
if (sysfs_create_file(kobj, &dev_attr_status.attr))
dev_warn(&dd->pdev->dev,
"Error creating 'status' sysfs entry\n");
+ if (sysfs_create_file(kobj, &dev_attr_flags.attr))
+ dev_warn(&dd->pdev->dev,
+ "Error creating 'flags' sysfs entry\n");
return 0;
}
@@ -2661,6 +2698,7 @@ static int mtip_hw_sysfs_exit(struct driver_data *dd, struct kobject *kobj)
sysfs_remove_file(kobj, &dev_attr_registers.attr);
sysfs_remove_file(kobj, &dev_attr_status.attr);
+ sysfs_remove_file(kobj, &dev_attr_flags.attr);
return 0;
}
--
1.7.1
On Tue, May 29, 2012 at 06:44:54PM -0700, Asai Thambi S P wrote:
>
> * 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(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-block-rssd b/Documentation/ABI/testing/sysfs-block-rssd
> index d535757..679ce35 100644
> --- a/Documentation/ABI/testing/sysfs-block-rssd
> +++ b/Documentation/ABI/testing/sysfs-block-rssd
> @@ -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
> diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
> index d0fa357..18daa04 100644
> --- a/drivers/block/mtip32xx/mtip32xx.c
> +++ b/drivers/block/mtip32xx/mtip32xx.c
> @@ -2562,40 +2562,58 @@ static ssize_t mtip_hw_show_registers(struct device *dev,
> int size = 0;
> int n;
>
> - 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
On 05/31/2012 08:08 AM, Greg KH wrote:
> On Tue, May 29, 2012 at 06:44:54PM -0700, Asai Thambi S P wrote:
>>
>> * 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(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-block-rssd b/Documentation/ABI/testing/sysfs-block-rssd
>> index d535757..679ce35 100644
>> --- a/Documentation/ABI/testing/sysfs-block-rssd
>> +++ b/Documentation/ABI/testing/sysfs-block-rssd
>> @@ -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
>> diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
>> index d0fa357..18daa04 100644
>> --- a/drivers/block/mtip32xx/mtip32xx.c
>> +++ b/drivers/block/mtip32xx/mtip32xx.c
>> @@ -2562,40 +2562,58 @@ static ssize_t mtip_hw_show_registers(struct device *dev,
>> int size = 0;
>> int n;
>>
>> - 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.
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.
--
Jens Axboe
On 5/31/2012 1:24 AM, Jens Axboe wrote:
> On 05/31/2012 08:08 AM, Greg KH wrote:
>> On Tue, May 29, 2012 at 06:44:54PM -0700, Asai Thambi S P wrote:
>>>
>>> * 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(-)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-block-rssd b/Documentation/ABI/testing/sysfs-block-rssd
>>> index d535757..679ce35 100644
>>> --- a/Documentation/ABI/testing/sysfs-block-rssd
>>> +++ b/Documentation/ABI/testing/sysfs-block-rssd
>>> @@ -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
>>> diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c
>>> index d0fa357..18daa04 100644
>>> --- a/drivers/block/mtip32xx/mtip32xx.c
>>> +++ b/drivers/block/mtip32xx/mtip32xx.c
>>> @@ -2562,40 +2562,58 @@ static ssize_t mtip_hw_show_registers(struct device *dev,
>>> int size = 0;
>>> int n;
>>>
>>> - 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.
>
> 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