[v6,1/2] bus/pci: harmonize and document rte_pci_read_config return value
diff mbox series

Message ID 20180828101240.12597-1-bluca@debian.org
State Accepted, archived
Delegated to: Thomas Monjalon
Headers show
Series
  • [v6,1/2] bus/pci: harmonize and document rte_pci_read_config return value
Related show

Checks

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

Commit Message

Luca Boccassi Aug. 28, 2018, 10:12 a.m. UTC
On Linux, rte_pci_read_config on success returns the number of read
bytes, but on BSD it returns 0.
Document the return values, and have BSD behave as Linux does.

At least one case (bnx2x PMD) treats 0 as an error, so the change
makes sense also for that.

Signed-off-by: Luca Boccassi <bluca@debian.org>
---
 drivers/bus/pci/bsd/pci.c     | 4 +++-
 drivers/bus/pci/rte_bus_pci.h | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Luca Boccassi Oct. 17, 2018, 9:58 a.m. UTC | #1
On Tue, 2018-08-28 at 11:12 +0100, Luca Boccassi wrote:
> On Linux, rte_pci_read_config on success returns the number of read
> bytes, but on BSD it returns 0.
> Document the return values, and have BSD behave as Linux does.
> 
> At least one case (bnx2x PMD) treats 0 as an error, so the change
> makes sense also for that.
> 
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---
>  drivers/bus/pci/bsd/pci.c     | 4 +++-
>  drivers/bus/pci/rte_bus_pci.h | 2 ++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c
> index 655b34b7e4..175d83cf1b 100644
> --- a/drivers/bus/pci/bsd/pci.c
> +++ b/drivers/bus/pci/bsd/pci.c
> @@ -439,6 +439,8 @@ int rte_pci_read_config(const struct
> rte_pci_device *dev,
>  {
>  	int fd = -1;
>  	int size;
> +	/* Copy Linux implementation's behaviour */
> +	const int return_len = len;
>  	struct pci_io pi = {
>  		.pi_sel = {
>  			.pc_domain = dev->addr.domain,
> @@ -469,7 +471,7 @@ int rte_pci_read_config(const struct
> rte_pci_device *dev,
>  	}
>  	close(fd);
>  
> -	return 0;
> +	return return_len;
>  
>   error:
>  	if (fd >= 0)
> diff --git a/drivers/bus/pci/rte_bus_pci.h
> b/drivers/bus/pci/rte_bus_pci.h
> index 0d1955ffe0..df8f64798d 100644
> --- a/drivers/bus/pci/rte_bus_pci.h
> +++ b/drivers/bus/pci/rte_bus_pci.h
> @@ -219,6 +219,8 @@ void rte_pci_unregister(struct rte_pci_driver
> *driver);
>   *   The length of the data buffer.
>   * @param offset
>   *   The offset into PCI config space
> + * @return
> + *  Number of bytes read on success, negative on error.
>   */
>  int rte_pci_read_config(const struct rte_pci_device *device,
>  		void *buf, size_t len, off_t offset);

Hi Bruce,

Any chance you could please review the small BSD patch above when you
have a sec? Thanks!
Bruce Richardson Oct. 17, 2018, 10:57 a.m. UTC | #2
On Wed, Oct 17, 2018 at 10:58:58AM +0100, Luca Boccassi wrote:
> On Tue, 2018-08-28 at 11:12 +0100, Luca Boccassi wrote:
> > On Linux, rte_pci_read_config on success returns the number of read
> > bytes, but on BSD it returns 0.
> > Document the return values, and have BSD behave as Linux does.
> > 
> > At least one case (bnx2x PMD) treats 0 as an error, so the change
> > makes sense also for that.
> > 
> > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > ---

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Patch
diff mbox series

diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c
index 655b34b7e4..175d83cf1b 100644
--- a/drivers/bus/pci/bsd/pci.c
+++ b/drivers/bus/pci/bsd/pci.c
@@ -439,6 +439,8 @@  int rte_pci_read_config(const struct rte_pci_device *dev,
 {
 	int fd = -1;
 	int size;
+	/* Copy Linux implementation's behaviour */
+	const int return_len = len;
 	struct pci_io pi = {
 		.pi_sel = {
 			.pc_domain = dev->addr.domain,
@@ -469,7 +471,7 @@  int rte_pci_read_config(const struct rte_pci_device *dev,
 	}
 	close(fd);
 
-	return 0;
+	return return_len;
 
  error:
 	if (fd >= 0)
diff --git a/drivers/bus/pci/rte_bus_pci.h b/drivers/bus/pci/rte_bus_pci.h
index 0d1955ffe0..df8f64798d 100644
--- a/drivers/bus/pci/rte_bus_pci.h
+++ b/drivers/bus/pci/rte_bus_pci.h
@@ -219,6 +219,8 @@  void rte_pci_unregister(struct rte_pci_driver *driver);
  *   The length of the data buffer.
  * @param offset
  *   The offset into PCI config space
+ * @return
+ *  Number of bytes read on success, negative on error.
  */
 int rte_pci_read_config(const struct rte_pci_device *device,
 		void *buf, size_t len, off_t offset);