On Mon, Sep 21, 2020 at 09:38:12PM +0530, Manivannan Sadhasivam wrote:
> From: Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxx>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
> ---
> 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 000000000000..ecfe7662f8d0
> --- /dev/null
> +++ b/Documentation/ABI/stable/sysfs-bus-mhi
> @@ -0,0 +1,21 @@
> +What: /sys/bus/mhi/devices/.../serialnumber
> +Date: Sept 2020
> +KernelVersion: 5.10
> +Contact: Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx>
> +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.
> +
> +What: /sys/bus/mhi/devices/.../oem_pk_hash
> +Date: Sept 2020
> +KernelVersion: 5.10
> +Contact: Bhaumik Bhatt <bbhatt@xxxxxxxxxxxxxx>
> +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 deaafb617361..11e7be9b9163 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11323,6 +11323,7 @@ M: Hemant Kumar <hemantk@xxxxxxxxxxxxxx>
> L: linux-arm-msm@xxxxxxxxxxxxxxx
> 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 61e5885a331a..1b4161eaf0d8 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);
You should never have to call a sysfs_* function from a driver or bus
code, that implies something is wrong :)
Just set the sysfs attributes to be the default groups for your
bus/device/whatever-you-have-here and then the driver core will properly
manage the creation and removal of these files, in a race-free manner.
As it is, I think this is racy and will cause userspace confusion, but I
haven't looked for sure, as you should use the api that guarantees it is
safe...
thanks,
greg k-h
