| Message ID | 20200921160815.28071-17-manivannan.sadhasivam@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | MHI changes for v5.10 | expand |
On Mon, Sep 21, 2020 at 09:38:12PM +0530, Manivannan Sadhasivam wrote: > From: Bhaumik Bhatt <bbhatt@codeaurora.org> > > 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> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.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 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@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. > + > +What: /sys/bus/mhi/devices/.../oem_pk_hash > +Date: Sept 2020 > +KernelVersion: 5.10 > +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 deaafb617361..11e7be9b9163 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -11323,6 +11323,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 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
On Sun, Sep 27, 2020 at 12:26:59PM +0200, Greg KH wrote: > On Mon, Sep 21, 2020 at 09:38:12PM +0530, Manivannan Sadhasivam wrote: > > From: Bhaumik Bhatt <bbhatt@codeaurora.org> > > > > 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> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.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 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@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. > > + > > +What: /sys/bus/mhi/devices/.../oem_pk_hash > > +Date: Sept 2020 > > +KernelVersion: 5.10 > > +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 deaafb617361..11e7be9b9163 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -11323,6 +11323,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 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 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 :) > Hmm. I think the inspiration is drawn from an old bus driver :( Will submit a patch later to fix it! > 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... > Okay. Will use the dev_groups callback to manage the sysfs attributes. Thanks, Mani > thanks, > > greg k-h
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@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. + +What: /sys/bus/mhi/devices/.../oem_pk_hash +Date: Sept 2020 +KernelVersion: 5.10 +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 deaafb617361..11e7be9b9163 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -11323,6 +11323,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 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); +} + +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);