Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Mon, 10 Aug 2020 12:11:04 -0700
From:   bbhatt@codeaurora.org
To:     Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Cc:     linux-arm-msm@vger.kernel.org, hemantk@codeaurora.org,
        jhugo@codeaurora.org, linux-kernel@vger.kernel.org,
        linux-arm-msm-owner@vger.kernel.org
Subject: Re: [PATCH v6 11/11] bus: mhi: core: Introduce sysfs entries for MHI
In-Reply-To: <20200807052242.GD3230@Mani-XPS-13-9360>
References: <1595901740-27379-1-git-send-email-bbhatt@codeaurora.org>
 <1595901740-27379-12-git-send-email-bbhatt@codeaurora.org>
 <20200807052242.GD3230@Mani-XPS-13-9360>
Message-ID: <dbe47f5d3b9aaef9815d2659df3a5161@codeaurora.org>
User-Agent: Roundcube Webmail/1.3.9
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 2020-08-06 22:22, Manivannan Sadhasivam wrote:
> On Mon, Jul 27, 2020 at 07:02:20PM -0700, Bhaumik Bhatt wrote:
>> Introduce sysfs entries to enable userspace clients the ability to 
>> read
>> the serial number and the OEM PK Hash values obtained from BHI. OEMs
>> need to read these device-specific hardware information values through
>> userspace for factory testing purposes and cannot be exposed via 
>> degbufs
>> as it may remain disabled for performance reasons. Also, update the
>> documentation for ABI to include these entries.
>> 
>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>> ---
>>  Documentation/ABI/stable/sysfs-bus-mhi | 21 ++++++++++++++
>>  MAINTAINERS                            |  1 +
>>  drivers/bus/mhi/core/init.c            | 53 
>> ++++++++++++++++++++++++++++++++++
>>  3 files changed, 75 insertions(+)
>>  create mode 100644 Documentation/ABI/stable/sysfs-bus-mhi
>> 
>> diff --git a/Documentation/ABI/stable/sysfs-bus-mhi 
>> b/Documentation/ABI/stable/sysfs-bus-mhi
>> new file mode 100644
>> index 0000000..1d5d0d6
>> --- /dev/null
>> +++ b/Documentation/ABI/stable/sysfs-bus-mhi
>> @@ -0,0 +1,21 @@
>> +What:		/sys/bus/mhi/devices/.../serialnumber
>> +Date:		Jul 2020
>> +KernelVersion:	5.8
>> +Contact:	Bhaumik Bhatt <bbhatt@codeaurora.org>
>> +Description:	The file holds the serial number of the client device 
>> obtained
>> +		using a BHI (Boot Host Interface) register read after at least
>> +		one attempt to power up the device has been done. If read
>> +		without having the device power on at least once, the file will
>> +		read all 0's.
>> +Users:		Any userspace application or clients interested in device 
>> info.
> 
> I think you're not using tabs here and that's why it is showing 
> mangled. Please
> use tabs as like other files.
> 
> Thanks,
> Mani
> 
Hi Mani,

I am using tabs actually. I, in fact, copied another file 
(sysfs-bus-vmbus) and only modified the
required entries and did a diff to confirm.

I doubt there is more I can do.

Please let me know if the next patch is acceptable soon.

>> +
>> +What:		/sys/bus/mhi/devices/.../oem_pk_hash
>> +Date:		Jul 2020
>> +KernelVersion:	5.8
>> +Contact:	Bhaumik Bhatt <bbhatt@codeaurora.org>
>> +Description:	The file holds the OEM PK Hash value of the endpoint 
>> device
>> +		obtained using a BHI (Boot Host Interface) register read after
>> +		at least one attempt to power up the device has been done. If
>> +		read without having the device power on at least once, the file
>> +		will read all 0's.
>> +Users:		Any userspace application or clients interested in device 
>> info.
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index e64e5db..5e49316 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11018,6 +11018,7 @@ M:	Hemant Kumar <hemantk@codeaurora.org>
>>  L:	linux-arm-msm@vger.kernel.org
>>  S:	Maintained
>>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mani/mhi.git
>> +F:	Documentation/ABI/stable/sysfs-bus-mhi
>>  F:	Documentation/mhi/
>>  F:	drivers/bus/mhi/
>>  F:	include/linux/mhi.h
>> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
>> index 972dbf0..c086ef2 100644
>> --- a/drivers/bus/mhi/core/init.c
>> +++ b/drivers/bus/mhi/core/init.c
>> @@ -76,6 +76,56 @@ const char *to_mhi_pm_state_str(enum mhi_pm_state 
>> state)
>>  	return mhi_pm_state_str[index];
>>  }
>> 
>> +static ssize_t serial_number_show(struct device *dev,
>> +				  struct device_attribute *attr,
>> +				  char *buf)
>> +{
>> +	struct mhi_device *mhi_dev = to_mhi_device(dev);
>> +	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
>> +
>> +	return snprintf(buf, PAGE_SIZE, "Serial Number: %u\n",
>> +			mhi_cntrl->serial_number);
>> +}
>> +static DEVICE_ATTR_RO(serial_number);
>> +
>> +static ssize_t oem_pk_hash_show(struct device *dev,
>> +				struct device_attribute *attr,
>> +				char *buf)
>> +{
>> +	struct mhi_device *mhi_dev = to_mhi_device(dev);
>> +	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
>> +	int i, cnt = 0;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(mhi_cntrl->oem_pk_hash); i++)
>> +		cnt += snprintf(buf + cnt, PAGE_SIZE - cnt,
>> +				"OEMPKHASH[%d]: 0x%x\n", i,
>> +				mhi_cntrl->oem_pk_hash[i]);
>> +
>> +	return cnt;
>> +}
>> +static DEVICE_ATTR_RO(oem_pk_hash);
>> +
>> +static struct attribute *mhi_sysfs_attrs[] = {
>> +	&dev_attr_serial_number.attr,
>> +	&dev_attr_oem_pk_hash.attr,
>> +	NULL,
>> +};
>> +
>> +static const struct attribute_group mhi_sysfs_group = {
>> +	.attrs = mhi_sysfs_attrs,
>> +};
>> +
>> +static int mhi_create_sysfs(struct mhi_controller *mhi_cntrl)
>> +{
>> +	return sysfs_create_group(&mhi_cntrl->mhi_dev->dev.kobj,
>> +				  &mhi_sysfs_group);
>> +}
>> +
>> +static void mhi_destroy_sysfs(struct mhi_controller *mhi_cntrl)
>> +{
>> +	sysfs_remove_group(&mhi_cntrl->mhi_dev->dev.kobj, &mhi_sysfs_group);
>> +}
>> +
>>  /* MHI protocol requires the transfer ring to be aligned with ring 
>> length */
>>  static int mhi_alloc_aligned_ring(struct mhi_controller *mhi_cntrl,
>>  				  struct mhi_ring *ring,
>> @@ -917,6 +967,8 @@ int mhi_register_controller(struct mhi_controller 
>> *mhi_cntrl,
>>  	mhi_cntrl->mhi_dev = mhi_dev;
>> 
>>  	mhi_create_debugfs(mhi_cntrl);
>> +	if (mhi_create_sysfs(mhi_cntrl))
>> +		dev_err(mhi_cntrl->cntrl_dev, "Failed to create sysfs entries\n");
>> 
>>  	return 0;
>> 
>> @@ -940,6 +992,7 @@ void mhi_unregister_controller(struct 
>> mhi_controller *mhi_cntrl)
>>  	struct mhi_chan *mhi_chan = mhi_cntrl->mhi_chan;
>>  	unsigned int i;
>> 
>> +	mhi_destroy_sysfs(mhi_cntrl);
>>  	mhi_destroy_debugfs(mhi_cntrl);
>> 
>>  	kfree(mhi_cntrl->mhi_cmd);
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>> a Linux Foundation Collaborative Project
>>