diff mbox series

dmaengine: idxd: add command status to idxd sysfs attribute

Message ID 159856348828.3418.7745506843326238999.stgit@djiang5-desk3.ch.intel.com (mailing list archive)
State Changes Requested
Headers show
Series dmaengine: idxd: add command status to idxd sysfs attribute | expand

Commit Message

Dave Jiang Aug. 27, 2020, 9:24 p.m. UTC 
Export admin command status to sysfs attribute in order to allow user to
retrieve configuration error. Allows user tooling to retrieve the command
error and provide more user friendly error messages.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/dma/idxd/device.c |    6 +++++-
 drivers/dma/idxd/idxd.h   |    1 +
 drivers/dma/idxd/sysfs.c  |   10 ++++++++++
 3 files changed, 16 insertions(+), 1 deletion(-)

Comments

Vinod Koul Aug. 28, 2020, 10:42 a.m. UTC | #1 
On 27-08-20, 14:24, Dave Jiang wrote:
> Export admin command status to sysfs attribute in order to allow user to
> retrieve configuration error. Allows user tooling to retrieve the command
> error and provide more user friendly error messages.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/dma/idxd/device.c |    6 +++++-
>  drivers/dma/idxd/idxd.h   |    1 +
>  drivers/dma/idxd/sysfs.c  |   10 ++++++++++
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
> index 00dab1465ca3..bdca1bc55cfa 100644
> --- a/drivers/dma/idxd/device.c
> +++ b/drivers/dma/idxd/device.c
> @@ -368,6 +368,7 @@ static void idxd_cmd_exec(struct idxd_device *idxd, int cmd_code, u32 operand,
>  	dev_dbg(&idxd->pdev->dev, "%s: sending cmd: %#x op: %#x\n",
>  		__func__, cmd_code, operand);
>  
> +	idxd->cmd_status = 0;
>  	__set_bit(IDXD_FLAG_CMD_RUNNING, &idxd->flags);
>  	idxd->cmd_done = &done;
>  	iowrite32(cmd.bits, idxd->reg_base + IDXD_CMD_OFFSET);
> @@ -379,8 +380,11 @@ static void idxd_cmd_exec(struct idxd_device *idxd, int cmd_code, u32 operand,
>  	spin_unlock_irqrestore(&idxd->dev_lock, flags);
>  	wait_for_completion(&done);
>  	spin_lock_irqsave(&idxd->dev_lock, flags);
> -	if (status)
> +	if (status) {
>  		*status = ioread32(idxd->reg_base + IDXD_CMDSTS_OFFSET);
> +		idxd->cmd_status = *status & 0xff;

define the magic 0xff and use GENMASK to define that!

> +	}
> +
>  	__clear_bit(IDXD_FLAG_CMD_RUNNING, &idxd->flags);
>  	/* Wake up other pending commands */
>  	wake_up(&idxd->cmd_waitq);
> diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
> index e8bec6eb9f7e..c64df197e724 100644
> --- a/drivers/dma/idxd/idxd.h
> +++ b/drivers/dma/idxd/idxd.h
> @@ -156,6 +156,7 @@ struct idxd_device {
>  	unsigned long flags;
>  	int id;
>  	int major;
> +	u8 cmd_status;
>  
>  	struct pci_dev *pdev;
>  	void __iomem *reg_base;
> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
> index c5f19802cb9e..1db8d021f5ae 100644
> --- a/drivers/dma/idxd/sysfs.c
> +++ b/drivers/dma/idxd/sysfs.c
> @@ -1395,6 +1395,15 @@ static ssize_t cdev_major_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(cdev_major);
>  
> +static ssize_t cmd_status_show(struct device *dev,
> +			       struct device_attribute *attr, char *buf)
> +{
> +	struct idxd_device *idxd = container_of(dev, struct idxd_device, conf_dev);
> +
> +	return sprintf(buf, "%#x\n", idxd->cmd_status);
> +}
> +static DEVICE_ATTR_RO(cmd_status);

Update ABI docs for this?

> +
>  static struct attribute *idxd_device_attributes[] = {
>  	&dev_attr_version.attr,
>  	&dev_attr_max_groups.attr,
> @@ -1413,6 +1422,7 @@ static struct attribute *idxd_device_attributes[] = {
>  	&dev_attr_max_tokens.attr,
>  	&dev_attr_token_limit.attr,
>  	&dev_attr_cdev_major.attr,
> +	&dev_attr_cmd_status.attr,
>  	NULL,
>  };
>
Dave Jiang Aug. 28, 2020, 2:31 p.m. UTC | #2 
On 8/28/2020 3:42 AM, Vinod Koul wrote:
> On 27-08-20, 14:24, Dave Jiang wrote:
>> Export admin command status to sysfs attribute in order to allow user to
>> retrieve configuration error. Allows user tooling to retrieve the command
>> error and provide more user friendly error messages.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>   drivers/dma/idxd/device.c |    6 +++++-
>>   drivers/dma/idxd/idxd.h   |    1 +
>>   drivers/dma/idxd/sysfs.c  |   10 ++++++++++
>>   3 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
>> index 00dab1465ca3..bdca1bc55cfa 100644
>> --- a/drivers/dma/idxd/device.c
>> +++ b/drivers/dma/idxd/device.c
>> @@ -368,6 +368,7 @@ static void idxd_cmd_exec(struct idxd_device *idxd, int cmd_code, u32 operand,
>>   	dev_dbg(&idxd->pdev->dev, "%s: sending cmd: %#x op: %#x\n",
>>   		__func__, cmd_code, operand);
>>   
>> +	idxd->cmd_status = 0;
>>   	__set_bit(IDXD_FLAG_CMD_RUNNING, &idxd->flags);
>>   	idxd->cmd_done = &done;
>>   	iowrite32(cmd.bits, idxd->reg_base + IDXD_CMD_OFFSET);
>> @@ -379,8 +380,11 @@ static void idxd_cmd_exec(struct idxd_device *idxd, int cmd_code, u32 operand,
>>   	spin_unlock_irqrestore(&idxd->dev_lock, flags);
>>   	wait_for_completion(&done);
>>   	spin_lock_irqsave(&idxd->dev_lock, flags);
>> -	if (status)
>> +	if (status) {
>>   		*status = ioread32(idxd->reg_base + IDXD_CMDSTS_OFFSET);
>> +		idxd->cmd_status = *status & 0xff;
> 
> define the magic 0xff and use GENMASK to define that!

Ok will do.

> 
>> +	}
>> +
>>   	__clear_bit(IDXD_FLAG_CMD_RUNNING, &idxd->flags);
>>   	/* Wake up other pending commands */
>>   	wake_up(&idxd->cmd_waitq);
>> diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
>> index e8bec6eb9f7e..c64df197e724 100644
>> --- a/drivers/dma/idxd/idxd.h
>> +++ b/drivers/dma/idxd/idxd.h
>> @@ -156,6 +156,7 @@ struct idxd_device {
>>   	unsigned long flags;
>>   	int id;
>>   	int major;
>> +	u8 cmd_status;
>>   
>>   	struct pci_dev *pdev;
>>   	void __iomem *reg_base;
>> diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
>> index c5f19802cb9e..1db8d021f5ae 100644
>> --- a/drivers/dma/idxd/sysfs.c
>> +++ b/drivers/dma/idxd/sysfs.c
>> @@ -1395,6 +1395,15 @@ static ssize_t cdev_major_show(struct device *dev,
>>   }
>>   static DEVICE_ATTR_RO(cdev_major);
>>   
>> +static ssize_t cmd_status_show(struct device *dev,
>> +			       struct device_attribute *attr, char *buf)
>> +{
>> +	struct idxd_device *idxd = container_of(dev, struct idxd_device, conf_dev);
>> +
>> +	return sprintf(buf, "%#x\n", idxd->cmd_status);
>> +}
>> +static DEVICE_ATTR_RO(cmd_status);
> 
> Update ABI docs for this?
> 

Yes

>> +
>>   static struct attribute *idxd_device_attributes[] = {
>>   	&dev_attr_version.attr,
>>   	&dev_attr_max_groups.attr,
>> @@ -1413,6 +1422,7 @@ static struct attribute *idxd_device_attributes[] = {
>>   	&dev_attr_max_tokens.attr,
>>   	&dev_attr_token_limit.attr,
>>   	&dev_attr_cdev_major.attr,
>> +	&dev_attr_cmd_status.attr,
>>   	NULL,
>>   };
>>   
>
diff mbox series

Patch

diff --git a/drivers/dma/idxd/device.c b/drivers/dma/idxd/device.c
index 00dab1465ca3..bdca1bc55cfa 100644
--- a/drivers/dma/idxd/device.c
+++ b/drivers/dma/idxd/device.c
@@ -368,6 +368,7 @@  static void idxd_cmd_exec(struct idxd_device *idxd, int cmd_code, u32 operand,
 	dev_dbg(&idxd->pdev->dev, "%s: sending cmd: %#x op: %#x\n",
 		__func__, cmd_code, operand);
 
+	idxd->cmd_status = 0;
 	__set_bit(IDXD_FLAG_CMD_RUNNING, &idxd->flags);
 	idxd->cmd_done = &done;
 	iowrite32(cmd.bits, idxd->reg_base + IDXD_CMD_OFFSET);
@@ -379,8 +380,11 @@  static void idxd_cmd_exec(struct idxd_device *idxd, int cmd_code, u32 operand,
 	spin_unlock_irqrestore(&idxd->dev_lock, flags);
 	wait_for_completion(&done);
 	spin_lock_irqsave(&idxd->dev_lock, flags);
-	if (status)
+	if (status) {
 		*status = ioread32(idxd->reg_base + IDXD_CMDSTS_OFFSET);
+		idxd->cmd_status = *status & 0xff;
+	}
+
 	__clear_bit(IDXD_FLAG_CMD_RUNNING, &idxd->flags);
 	/* Wake up other pending commands */
 	wake_up(&idxd->cmd_waitq);
diff --git a/drivers/dma/idxd/idxd.h b/drivers/dma/idxd/idxd.h
index e8bec6eb9f7e..c64df197e724 100644
--- a/drivers/dma/idxd/idxd.h
+++ b/drivers/dma/idxd/idxd.h
@@ -156,6 +156,7 @@  struct idxd_device {
 	unsigned long flags;
 	int id;
 	int major;
+	u8 cmd_status;
 
 	struct pci_dev *pdev;
 	void __iomem *reg_base;
diff --git a/drivers/dma/idxd/sysfs.c b/drivers/dma/idxd/sysfs.c
index c5f19802cb9e..1db8d021f5ae 100644
--- a/drivers/dma/idxd/sysfs.c
+++ b/drivers/dma/idxd/sysfs.c
@@ -1395,6 +1395,15 @@  static ssize_t cdev_major_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(cdev_major);
 
+static ssize_t cmd_status_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	struct idxd_device *idxd = container_of(dev, struct idxd_device, conf_dev);
+
+	return sprintf(buf, "%#x\n", idxd->cmd_status);
+}
+static DEVICE_ATTR_RO(cmd_status);
+
 static struct attribute *idxd_device_attributes[] = {
 	&dev_attr_version.attr,
 	&dev_attr_max_groups.attr,
@@ -1413,6 +1422,7 @@  static struct attribute *idxd_device_attributes[] = {
 	&dev_attr_max_tokens.attr,
 	&dev_attr_token_limit.attr,
 	&dev_attr_cdev_major.attr,
+	&dev_attr_cmd_status.attr,
 	NULL,
 };