diff mbox series

[1/2] eal: expose device states in rte device

Message ID 1541583691-145432-2-git-send-email-jia.guo@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers
Series expose device states for hot-unplug |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Guo, Jia Nov. 7, 2018, 9:41 a.m. UTC 
  Since the hotplug API and device event API have make public, so in order
to let applications or driver deal with device directly, it would be
helpful if the states of device could be exposed, especially for hotplug
process.

This patch will add some devices states in rte device structure
to recode the device's current status, such as “RTE_DEV_UNUSED”,
“RTE_DEV_ATTACHED” and “RTE_DEV_REMOVED”.

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
 lib/librte_eal/common/include/rte_dev.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Stephen Hemminger Nov. 7, 2018, 7:32 p.m. UTC | #1 
On Wed,  7 Nov 2018 17:41:30 +0800
Jeff Guo <jia.guo@intel.com> wrote:

> Since the hotplug API and device event API have make public, so in order
> to let applications or driver deal with device directly, it would be
> helpful if the states of device could be exposed, especially for hotplug
> process.
> 
> This patch will add some devices states in rte device structure
> to recode the device's current status, such as “RTE_DEV_UNUSED”,
> “RTE_DEV_ATTACHED” and “RTE_DEV_REMOVED”.
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>

Looks good.

I did spot a couple of minor nits you might want to address if
resending this.


> ---
>  lib/librte_eal/common/include/rte_dev.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> index cd6c187..1bab0dd 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -151,6 +151,18 @@ struct rte_driver {
>  #define RTE_DEV_NAME_MAX_LEN 64
>  
>  /**
> + * Possible states of an rte devcie.
> + */

s/devcie/device/

> +enum rte_dev_state {
> +	/** Device is unused before being hotplug add. */
> +	RTE_DEV_UNUSED = 0,
> +	/** Device is attached when allocated in probing. */
> +	RTE_DEV_ATTACHED,
> +	/** Device is in removed state when plug-out is detected. */
> +	RTE_DEV_REMOVED,
> +};
> +
> +/**
>   * A structure describing a generic device.
>   */
>  struct rte_device {
> @@ -160,6 +172,7 @@ struct rte_device {
>  	const struct rte_bus *bus;    /**< Bus handle assigned on scan */
>  	int numa_node;                /**< NUMA node connection */
>  	struct rte_devargs *devargs;  /**< Arguments for latest probing */
> +	enum rte_dev_state state; /**< Flag indicating the device state */

Why not align comment with other fields here?

>  };
>  
>  /**
Guo, Jia Nov. 9, 2018, 7:35 a.m. UTC | #2 
On 11/8/2018 3:32 AM, Stephen Hemminger wrote:
> On Wed,  7 Nov 2018 17:41:30 +0800
> Jeff Guo <jia.guo@intel.com> wrote:
>
>> Since the hotplug API and device event API have make public, so in order
>> to let applications or driver deal with device directly, it would be
>> helpful if the states of device could be exposed, especially for hotplug
>> process.
>>
>> This patch will add some devices states in rte device structure
>> to recode the device's current status, such as “RTE_DEV_UNUSED”,
>> “RTE_DEV_ATTACHED” and “RTE_DEV_REMOVED”.
>>
>> Signed-off-by: Jeff Guo <jia.guo@intel.com>
> Looks good.
>
> I did spot a couple of minor nits you might want to address if
> resending this.
>
>
>> ---
>>   lib/librte_eal/common/include/rte_dev.h | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
>> index cd6c187..1bab0dd 100644
>> --- a/lib/librte_eal/common/include/rte_dev.h
>> +++ b/lib/librte_eal/common/include/rte_dev.h
>> @@ -151,6 +151,18 @@ struct rte_driver {
>>   #define RTE_DEV_NAME_MAX_LEN 64
>>   
>>   /**
>> + * Possible states of an rte devcie.
>> + */
> s/devcie/device/


got it.


>> +enum rte_dev_state {
>> +	/** Device is unused before being hotplug add. */
>> +	RTE_DEV_UNUSED = 0,
>> +	/** Device is attached when allocated in probing. */
>> +	RTE_DEV_ATTACHED,
>> +	/** Device is in removed state when plug-out is detected. */
>> +	RTE_DEV_REMOVED,
>> +};
>> +
>> +/**
>>    * A structure describing a generic device.
>>    */
>>   struct rte_device {
>> @@ -160,6 +172,7 @@ struct rte_device {
>>   	const struct rte_bus *bus;    /**< Bus handle assigned on scan */
>>   	int numa_node;                /**< NUMA node connection */
>>   	struct rte_devargs *devargs;  /**< Arguments for latest probing */
>> +	enum rte_dev_state state; /**< Flag indicating the device state */
> Why not align comment with other fields here?


ok, maybe i could split the line if considerate with the line number limit.


>>   };
>>   
>>   /**
diff mbox series

Patch

diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index cd6c187..1bab0dd 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -151,6 +151,18 @@  struct rte_driver {
 #define RTE_DEV_NAME_MAX_LEN 64
 
 /**
+ * Possible states of an rte devcie.
+ */
+enum rte_dev_state {
+	/** Device is unused before being hotplug add. */
+	RTE_DEV_UNUSED = 0,
+	/** Device is attached when allocated in probing. */
+	RTE_DEV_ATTACHED,
+	/** Device is in removed state when plug-out is detected. */
+	RTE_DEV_REMOVED,
+};
+
+/**
  * A structure describing a generic device.
  */
 struct rte_device {
@@ -160,6 +172,7 @@  struct rte_device {
 	const struct rte_bus *bus;    /**< Bus handle assigned on scan */
 	int numa_node;                /**< NUMA node connection */
 	struct rte_devargs *devargs;  /**< Arguments for latest probing */
+	enum rte_dev_state state; /**< Flag indicating the device state */
 };
 
 /**