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

["ira.weiny" <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

On Wed, Jun 15, 2016 at 09:40:08AM +0300, Leon Romanovsky wrote:
> On Wed, Jun 15, 2016 at 02:21:56AM -0400, ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org wrote:
> > From: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

[snip]

> > 
> > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > index 5c155fa91eec..760ef603a468 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
> > +		str[0] = '\0';
> 
> Interesting point, what is better for the caller to get as a version?
> You chose empty string, but it can be all Fs or all 0s.

The default here is that a device does not provide a firmware string.  In that
case the assumption is that there is no firmware.  Hence the empty string.  I
don't think that reporting 0's or F's are appropriate if a device does not have
firmware.

In the cases you speak of I would expect the device to supply the firmware
function and report all 0s or all Fs for whatever version that may represent.
(I'm assuming that would be some sort of debug or unreleased firmware...)

I could just as well done something like.

snprintf(str, str_len, "<none>");

Which may be more informative?  I just don't want to get too wrapped up in the
policy here.  I think this is best left to the driver/hw vendor.

Ira

> 
> > +}
> > +EXPORT_SYMBOL(ib_get_device_fw_str);


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