> > + if (IS_ERR(hwmon)) { > > + dev_warn(dev, "Failed to instantiate hwmon device\n"); > > + kfree(data); > > + return PTR_ERR(hwmon); > > If the error is ignored by the caller, it doesn't make sense to return it. Done. > > > + } > > + > > + hba->hwmon_device = hwmon; > > + > > + return 0; > > +} > > + > > +void ufs_hwmon_remove(struct ufs_hba *hba) { > > + if (hba->hwmon_device) { > > + struct ufs_hwmon_data *data = > > + dev_get_drvdata(hba->hwmon_device); > > + > > + hwmon_device_unregister(hba->hwmon_device); > > + hba->hwmon_device = NULL; > > + kfree(data); > > + } > > +} > > That function is never called and thus pointless (suggesting that there may > be some structural problem in the code). Yayks... Forgot to call it from ufshcd_remve() Thanks. > > > > ufshcd_wb_probe(hba, desc_buf); > > > > + ufshcd_temp_notif_probe(hba, desc_buf); > > + > > Is that the appropriate place to make this call ? Yes I think so. This path is on link startup when the host reads the device configuration. > > +#ifdef CONFIG_SCSI_UFS_HWMON > > +int ufs_hwmon_probe(struct ufs_hba *hba, u8 mask); void > > +ufs_hwmon_remove(struct ufs_hba *hba); #else static inline int > > +ufs_hwmon_probe(struct ufs_hba *hba, u8 mask) { return 0; } static > > +inline void nvme_hwmon_remove(struct ufs_hba *hba) {} > > ufs_hwmon_remove Done. Thanks.