[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <DM2PR0401MB0975383CF11F3932BB5DA77E9AFF0@DM2PR0401MB0975.namprd04.prod.outlook.com>
Date: Sun, 4 Feb 2018 13:09:09 +0000
From: Stanislav Nijnikov <Stanislav.Nijnikov@....com>
To: Greg KH <gregkh@...uxfoundation.org>
CC: "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"jaegeuk@...nel.org" <jaegeuk@...nel.org>,
Alex Lemberg <Alex.Lemberg@....com>
Subject: RE: [PATCH v4 01/10] ufs: sysfs: attribute group for existing sysfs
entries.
> -----Original Message-----
> From: Greg KH [mailto:gregkh@...uxfoundation.org]
> Sent: Sunday, February 4, 2018 2:34 PM
> To: Stanislav Nijnikov <Stanislav.Nijnikov@....com>
> Cc: linux-scsi@...r.kernel.org; linux-kernel@...r.kernel.org;
> jaegeuk@...nel.org; Alex Lemberg <Alex.Lemberg@....com>
> Subject: Re: [PATCH v4 01/10] ufs: sysfs: attribute group for existing sysfs
> entries.
>
> On Sun, Feb 04, 2018 at 12:29:06PM +0000, Stanislav Nijnikov wrote:
> > > > + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> > > > + "\nAll available Runtime PM levels info:\n");
> > > > + for (lvl = UFS_PM_LVL_0; lvl < UFS_PM_LVL_MAX; lvl++)
> > > > + curr_len += snprintf((buf + curr_len), (PAGE_SIZE - curr_len),
> > > > + "\tRuntime PM level [%d] => dev_state
> > > [%s] link_state [%s]\n",
> > > > + lvl,
> > > > + ufschd_ufs_dev_pwr_mode_to_string(
> > > > + ufs_pm_lvl_states[lvl].dev_state),
> > > > + ufschd_uic_link_state_to_string(
> > > > + ufs_pm_lvl_states[lvl].link_state));
> > > > +
> > >
> > > sysfs if "one value per file", not "random text that someone has to
> > > parse per file" please.
> > >
> > > Huge hint, if you ever care about checking the size of the sysfs
> > > buffer you are writing into, you are doing something really really wrong.
> > >
> > Hi Greg
> > It's the existing code, added by:
> > commit 09690d5a6ae1b7e4cb5ac429c311b99d09352c12
> > Author: subhashj@...eaurora.org <subhashj@...eaurora.org>
> > Date: Thu Dec 22 18:41:00 2016 -0800
> >
> > scsi: ufs: provide sysfs attribute to select the PM level
> >
> > This patch provides the sysfs attribute to choose the power management
> > level for UFS runtime and system suspend.
> >
> > Reviewed-by: Sujit Reddy Thumma <sthumma@...eaurora.org>
> > Signed-off-by: Subhash Jadavani <subhashj@...eaurora.org>
> > Signed-off-by: Martin K. Petersen <martin.petersen@...cle.com>
> >
> > I just moved it to an another file and changed the sysfs entries
> > creation by Jaegeuk Kim' request. At the moment the entry shows the PM
> > level, the device state, the link state and all possible PM levels. Do you
> want me to change it?
>
> Ah, you are just moving this code around. Ok, that's fine for this patch, but
> please fix it up as part of this patch series because this isn't an acceptable
> sysfs file at all. If it were documented that would be a lot more obvious as to
> just how wrong it was :(
>
> And, as it wasn't documented, you can change it as it's obvious no one used it
> :)
>
> thanks,
>
> greg k-h
Can I fix these entries not in this patchset? As long as I know they are used and I
would prefer that change of the existing sysfs entries behavior be related to a
separate patch.
Regards.
Stanislav.
Powered by blists - more mailing lists