From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752037AbbLUVO4 (ORCPT <rfc822;w@1wt.eu>);
	Mon, 21 Dec 2015 16:14:56 -0500
Received: from mail.linuxfoundation.org ([140.211.169.12]:60279 "EHLO
	mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751652AbbLUVOy (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 21 Dec 2015 16:14:54 -0500
Date: Mon, 21 Dec 2015 13:14:38 -0800
From: Greg KH <gregkh@linuxfoundation.org>
To: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: linux-kernel@vger.kernel.org, devel@linuxdriverproject.org, olaf@aepfle.de,
        apw@canonical.com, vkuznets@redhat.com, jasowang@redhat.com
Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Add vendor and device atttributes
Message-ID: <20151221211438.GB8015@kroah.com>
References: <1450396289-17309-1-git-send-email-kys@microsoft.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1450396289-17309-1-git-send-email-kys@microsoft.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Dec 17, 2015 at 03:51:29PM -0800, K. Y. Srinivasan wrote:
> Add vendor and device attributes to VMBUS devices. These will be used
> by Hyper-V tools as well user-level RDMA libraries that will use the
> vendor/device tuple to discover the RDMA device.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
>  Documentation/ABI/stable/sysfs-bus-vmbus |   14 ++++++++++++++
>  drivers/hv/vmbus_drv.c                   |   21 +++++++++++++++++++++
>  include/linux/hyperv.h                   |    2 ++
>  3 files changed, 37 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus b/Documentation/ABI/stable/sysfs-bus-vmbus
> index 636e938..5d0125f 100644
> --- a/Documentation/ABI/stable/sysfs-bus-vmbus
> +++ b/Documentation/ABI/stable/sysfs-bus-vmbus
> @@ -27,3 +27,17 @@ Description:	The mapping of which primary/sub channels are bound to which
>  		Virtual Processors.
>  		Format: <channel's child_relid:the bound cpu's number>
>  Users:		tools/hv/lsvmbus
> +
> +What:		/sys/bus/vmbus/devices/vmbus_*/device

Shouldn't that be 'device_id' as 'device' is a symlink in the sysfs
tree?

> +Date:		Dec. 2015
> +KernelVersion:	4.5
> +Contact:	K. Y. Srinivasan <kys@microsoft.com>
> +Description:	The 16 bit device ID of the device
> +Users:		tools/hv/lsvmbus and user level RDMA libraries
> +
> +What:		/sys/bus/vmbus/devices/vmbus_*/vendor

'vendor_id'?


> +Date:		Dec. 2015
> +KernelVersion:	4.5
> +Contact:	K. Y. Srinivasan <kys@microsoft.com>
> +Description:	The 16 bit vendor ID of the device
> +Users:		tools/hv/lsvmbus and user level RDMA libraries
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 328e4c3..3668a95 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -477,6 +477,24 @@ static ssize_t channel_vp_mapping_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(channel_vp_mapping);
>  
> +static ssize_t vendor_show(struct device *dev,
> +			   struct device_attribute *dev_attr,
> +			   char *buf)
> +{
> +	struct hv_device *hv_dev = device_to_hv_device(dev);
> +	return sprintf(buf, "0x%x\n", hv_dev->vendor_id);
> +}
> +static DEVICE_ATTR_RO(vendor);
> +
> +static ssize_t device_show(struct device *dev,
> +			   struct device_attribute *dev_attr,
> +			   char *buf)
> +{
> +	struct hv_device *hv_dev = device_to_hv_device(dev);
> +	return sprintf(buf, "0x%x\n", hv_dev->device_id);
> +}
> +static DEVICE_ATTR_RO(device);
> +
>  /* Set up per device attributes in /sys/bus/vmbus/devices/<bus device> */
>  static struct attribute *vmbus_attrs[] = {
>  	&dev_attr_id.attr,
> @@ -502,6 +520,8 @@ static struct attribute *vmbus_attrs[] = {
>  	&dev_attr_in_read_bytes_avail.attr,
>  	&dev_attr_in_write_bytes_avail.attr,
>  	&dev_attr_channel_vp_mapping.attr,
> +	&dev_attr_vendor.attr,
> +	&dev_attr_device.attr,
>  	NULL,
>  };
>  ATTRIBUTE_GROUPS(vmbus);
> @@ -957,6 +977,7 @@ struct hv_device *vmbus_device_create(const uuid_le *type,
>  	memcpy(&child_device_obj->dev_type, type, sizeof(uuid_le));
>  	memcpy(&child_device_obj->dev_instance, instance,
>  	       sizeof(uuid_le));
> +	child_device_obj->vendor_id = 0x1414; /* MSFT vendor ID */

So this is always the same value?

And device_id is never set?  What does it default to?  Why include it in
this patch if it's never used?

thanks,

greg k-h