On 12/20, Greg KH wrote:
> On Tue, Dec 19, 2017 at 12:02:53PM -0800, Jaegeuk Kim wrote:
> > From: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>
> >
> > This patch introduces attribute group to show existing sysfs entries.
> >
> > Cc: Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Jaegeuk Kim <jaegeuk@xxxxxxxxxx>
> > ---
> > drivers/scsi/ufs/ufshcd.c | 48 +++++++++++++++++++----------------------------
> > drivers/scsi/ufs/ufshcd.h | 2 --
> > 2 files changed, 19 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index 011c3369082c..12ff7daebb00 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -7605,7 +7605,7 @@ static inline ssize_t ufshcd_pm_lvl_store(struct device *dev,
> > return count;
> > }
> >
> > -static ssize_t ufshcd_rpm_lvl_show(struct device *dev,
> > +static ssize_t rpm_lvl_show(struct device *dev,
> > struct device_attribute *attr, char *buf)
> > {
> > struct ufs_hba *hba = dev_get_drvdata(dev);
> > @@ -7634,24 +7634,13 @@ static ssize_t ufshcd_rpm_lvl_show(struct device *dev,
> > return curr_len;
> > }
> >
> > -static ssize_t ufshcd_rpm_lvl_store(struct device *dev,
> > +static ssize_t rpm_lvl_store(struct device *dev,
> > struct device_attribute *attr, const char *buf, size_t count)
> > {
> > return ufshcd_pm_lvl_store(dev, attr, buf, count, true);
> > }
> >
> > -static void ufshcd_add_rpm_lvl_sysfs_nodes(struct ufs_hba *hba)
> > -{
> > - hba->rpm_lvl_attr.show = ufshcd_rpm_lvl_show;
> > - hba->rpm_lvl_attr.store = ufshcd_rpm_lvl_store;
> > - sysfs_attr_init(&hba->rpm_lvl_attr.attr);
> > - hba->rpm_lvl_attr.attr.name = "rpm_lvl";
> > - hba->rpm_lvl_attr.attr.mode = 0644;
> > - if (device_create_file(hba->dev, &hba->rpm_lvl_attr))
> > - dev_err(hba->dev, "Failed to create sysfs for rpm_lvl\n");
> > -}
> > -
> > -static ssize_t ufshcd_spm_lvl_show(struct device *dev,
> > +static ssize_t spm_lvl_show(struct device *dev,
> > struct device_attribute *attr, char *buf)
> > {
> > struct ufs_hba *hba = dev_get_drvdata(dev);
> > @@ -7680,33 +7669,34 @@ static ssize_t ufshcd_spm_lvl_show(struct device *dev,
> > return curr_len;
> > }
> >
> > -static ssize_t ufshcd_spm_lvl_store(struct device *dev,
> > +static ssize_t spm_lvl_store(struct device *dev,
> > struct device_attribute *attr, const char *buf, size_t count)
> > {
> > return ufshcd_pm_lvl_store(dev, attr, buf, count, false);
> > }
> >
> > -static void ufshcd_add_spm_lvl_sysfs_nodes(struct ufs_hba *hba)
> > -{
> > - hba->spm_lvl_attr.show = ufshcd_spm_lvl_show;
> > - hba->spm_lvl_attr.store = ufshcd_spm_lvl_store;
> > - sysfs_attr_init(&hba->spm_lvl_attr.attr);
> > - hba->spm_lvl_attr.attr.name = "spm_lvl";
> > - hba->spm_lvl_attr.attr.mode = 0644;
> > - if (device_create_file(hba->dev, &hba->spm_lvl_attr))
> > - dev_err(hba->dev, "Failed to create sysfs for spm_lvl\n");
> > -}
> > +static DEVICE_ATTR_RW(rpm_lvl);
> > +static DEVICE_ATTR_RW(spm_lvl);
> > +
> > +static struct attribute *ufshcd_attrs[] = {
> > + &dev_attr_rpm_lvl.attr,
> > + &dev_attr_spm_lvl.attr,
> > + NULL
> > +};
> > +
> > +static const struct attribute_group ufshcd_attr_group = {
> > + .attrs = ufshcd_attrs,
> > +};
>
> ATTRIBUTE_GROUPS()?
>
> > static inline void ufshcd_add_sysfs_nodes(struct ufs_hba *hba)
> > {
> > - ufshcd_add_rpm_lvl_sysfs_nodes(hba);
> > - ufshcd_add_spm_lvl_sysfs_nodes(hba);
> > + if (sysfs_create_group(&hba->dev->kobj, &ufshcd_attr_group))
> > + dev_err(hba->dev, "Failed to create sysfs group\n");
>
> That will turn this into sysfs_create_groups()
>
> But really, you should be able to do this all "at once" And really, it
> should be a "default attribute group" that the driver core adds to the
> device, but that's outside the scope of this patchset.
>
> So for now, this is just fine, the attribute groups work is for an
> add-on patch. Thanks for working to get this upstream, I'm tired of
> seeing all of the different variants of this driver floating around
> out-of-tree.
Agreed, it's really painful to sync it across many kernels. :(
Anyway, I've changed to use ATTRIBUTE_GROUPS() in v3 as well.
Thanks,
