diff mbox

[01/13] IB/core: Add get FW version string to the core

Message ID 1465509248-11324-2-git-send-email-ira.weiny@intel.com (mailing list archive)
State Superseded
Headers show

Commit Message

Ira Weiny June 9, 2016, 9:53 p.m. UTC 
From: Ira Weiny <ira.weiny@intel.com>

Allow for a common core function to get firmware version strings
from the individual devices.

In later patches this format can then then be used to pass a
properly formated version string through the IPoIB layer.

The problem with the current code in the IPoIB layer is that it is
specific to certain hardware types.

Furthermore, this gives us a common function through which the core
can provide a common sysfs entry.  Eventually we may want to
remove the sysfs export but this provides for user space backwards
compatibility.

Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Ira Weiny <ira.weiny@intel.com>
---
 drivers/infiniband/core/device.c | 9 +++++++++
 include/rdma/ib_verbs.h          | 3 +++
 2 files changed, 12 insertions(+)

Comments

Jason Gunthorpe June 9, 2016, 10:29 p.m. UTC | #1 
On Thu, Jun 09, 2016 at 05:53:56PM -0400, ira.weiny@intel.com wrote:
> +void ib_get_device_fw_str(struct ib_device *dev, char *str, size_t str_len)
> +{
> +	if (dev->get_dev_fw_str)
> +		dev->get_dev_fw_str(dev, str, str_len);
> +	else
> +		snprintf(str, str_len, "%s", "");

That is the strangest way to write str[0] = 0;

>  void ib_dealloc_device(struct ib_device *device);
>  
> +void ib_get_device_fw_str(struct ib_device *device, char *, size_t);

It is not common in the kernel to drop the parameter names from the
prototype, please don't add more.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ira Weiny June 14, 2016, 2:45 a.m. UTC | #2 
On Thu, Jun 09, 2016 at 04:29:19PM -0600, Jason Gunthorpe wrote:
> On Thu, Jun 09, 2016 at 05:53:56PM -0400, ira.weiny@intel.com wrote:
> > +void ib_get_device_fw_str(struct ib_device *dev, char *str, size_t str_len)
> > +{
> > +	if (dev->get_dev_fw_str)
> > +		dev->get_dev_fw_str(dev, str, str_len);
> > +	else
> > +		snprintf(str, str_len, "%s", "");
> 
> That is the strangest way to write str[0] = 0;

I find it to be very explicit without any performance requirements.

How about?

str[0] = '\0';

> 
> >  void ib_dealloc_device(struct ib_device *device);
> >  
> > +void ib_get_device_fw_str(struct ib_device *device, char *, size_t);
> 
> It is not common in the kernel to drop the parameter names from the
> prototype, please don't add more.

Good catch sorry about that.

Fixed.

Ira

> 
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 5516fb070344..3ec3c8306e14 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -311,6 +311,15 @@  static int read_port_immutable(struct ib_device *device)
 	return 0;
 }
 
+void ib_get_device_fw_str(struct ib_device *dev, char *str, size_t str_len)
+{
+	if (dev->get_dev_fw_str)
+		dev->get_dev_fw_str(dev, str, str_len);
+	else
+		snprintf(str, str_len, "%s", "");
+}
+EXPORT_SYMBOL(ib_get_device_fw_str);
+
 /**
  * ib_register_device - Register an IB device with IB core
  * @device:Device to register
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 432bed510369..e013668d2483 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -1956,6 +1956,7 @@  struct ib_device {
 	 * in fast paths.
 	 */
 	int (*get_port_immutable)(struct ib_device *, u8, struct ib_port_immutable *);
+	void (*get_dev_fw_str)(struct ib_device *, char *str, size_t str_len);
 };
 
 struct ib_client {
@@ -1991,6 +1992,8 @@  struct ib_client {
 struct ib_device *ib_alloc_device(size_t size);
 void ib_dealloc_device(struct ib_device *device);
 
+void ib_get_device_fw_str(struct ib_device *device, char *, size_t);
+
 int ib_register_device(struct ib_device *device,
 		       int (*port_callback)(struct ib_device *,
 					    u8, struct kobject *));