| Message ID | 20220802080146epcms2p24b86bfce3d3c09c79b91d861cb3b2cce@epcms2p2 |
|---|---|
| Headers | show |
| Series | scsi: ufs: wb: Add sysfs attribute and cleanup | expand |
Hi, On Tue, Aug 2, 2022 at 4:29 PM Jinyoung CHOI <j-young.choi@samsung.com> wrote: > > Messages are modified to fit the format of others. > > Reviewed-by: Avri Altman <avri.altman@wdc.com> > Reviewed-by: Bart Van Assche <bvanassche@acm.org> > Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com> > --- > drivers/ufs/core/ufs-sysfs.c | 2 +- > drivers/ufs/core/ufshcd.c | 23 +++++++++++------------ > 2 files changed, 12 insertions(+), 13 deletions(-) > > diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c > index 2c0b7f45de4b..117272cf7d61 100644 > --- a/drivers/ufs/core/ufs-sysfs.c > +++ b/drivers/ufs/core/ufs-sysfs.c > @@ -230,7 +230,7 @@ static ssize_t wb_on_store(struct device *dev, struct device_attribute *attr, > * If the platform supports UFSHCD_CAP_CLK_SCALING, turn WB > * on/off will be done while clock scaling up/down. > */ > - dev_warn(dev, "To control WB through wb_on is not allowed!\n"); > + dev_warn(dev, "It is not allowed to configure WB!\n"); > return -EOPNOTSUPP; > } > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > index 5099d161f115..dcd7f03db2a2 100644 > --- a/drivers/ufs/core/ufshcd.c > +++ b/drivers/ufs/core/ufshcd.c > @@ -5737,13 +5737,13 @@ int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable) > > ret = __ufshcd_wb_toggle(hba, enable, QUERY_FLAG_IDN_WB_EN); > if (ret) { > - dev_err(hba->dev, "%s Write Booster %s failed %d\n", > - __func__, enable ? "enable" : "disable", ret); > + dev_err(hba->dev, "%s: Write Booster %s failed %d\n", > + __func__, enable ? "enabling" : "disabling", ret); > return ret; > } > > hba->dev_info.wb_enabled = enable; > - dev_info(hba->dev, "%s Write Booster %s\n", > + dev_info(hba->dev, "%s: Write Booster %s\n", > __func__, enable ? "enabled" : "disabled"); You need to rebase this patch to follow the latest change as https://lore.kernel.org/all/20220709000027.3929970-1-bjorn.andersson@linaro.org/ > > return ret; > @@ -5757,11 +5757,11 @@ static void ufshcd_wb_toggle_buf_flush_during_h8(struct ufs_hba *hba, > ret = __ufshcd_wb_toggle(hba, enable, > QUERY_FLAG_IDN_WB_BUFF_FLUSH_DURING_HIBERN8); > if (ret) { > - dev_err(hba->dev, "%s: WB-Buf Flush during H8 %s failed: %d\n", > - __func__, enable ? "enable" : "disable", ret); > + dev_err(hba->dev, "%s: WB-Buf Flush during H8 %s failed %d\n", > + __func__, enable ? "enabling" : "disabling", ret); > return; > } > - dev_dbg(hba->dev, "%s WB-Buf Flush during H8 %s\n", > + dev_info(hba->dev, "%s: WB-Buf Flush during H8 %s\n", > __func__, enable ? "enabled" : "disabled"); > } > > @@ -5775,14 +5775,13 @@ int ufshcd_wb_toggle_buf_flush(struct ufs_hba *hba, bool enable) > > ret = __ufshcd_wb_toggle(hba, enable, QUERY_FLAG_IDN_WB_BUFF_FLUSH_EN); > if (ret) { > - dev_err(hba->dev, "%s WB-Buf Flush %s failed %d\n", __func__, > - enable ? "enable" : "disable", ret); > + dev_err(hba->dev, "%s: WB-Buf Flush %s failed %d\n", > + __func__, enable ? "enabling" : "disabling", ret); > return ret; > } > > hba->dev_info.wb_buf_flush_enabled = enable; > - > - dev_dbg(hba->dev, "%s WB-Buf Flush %s\n", > + dev_info(hba->dev, "%s: WB-Buf Flush %s\n", > __func__, enable ? "enabled" : "disabled"); > > return ret; > @@ -5800,7 +5799,7 @@ static bool ufshcd_wb_presrv_usrspc_keep_vcc_on(struct ufs_hba *hba, > QUERY_ATTR_IDN_CURR_WB_BUFF_SIZE, > index, 0, &cur_buf); > if (ret) { > - dev_err(hba->dev, "%s dCurWriteBoosterBufferSize read failed %d\n", > + dev_err(hba->dev, "%s: dCurWriteBoosterBufferSize read failed %d\n", > __func__, ret); > return false; > } > @@ -5885,7 +5884,7 @@ static bool ufshcd_wb_need_flush(struct ufs_hba *hba) > QUERY_ATTR_IDN_AVAIL_WB_BUFF_SIZE, > index, 0, &avail_buf); > if (ret) { > - dev_warn(hba->dev, "%s dAvailableWriteBoosterBufferSize read failed %d\n", > + dev_warn(hba->dev, "%s: dAvailableWriteBoosterBufferSize read failed %d\n", > __func__, ret); > return false; > } > -- > 2.25.1
Hi, Stanley, >Hi, > >On Tue, Aug 2, 2022 at 4:29 PM Jinyoung CHOI <j-young.choi@samsung.com> wrote: >> >> Messages are modified to fit the format of others. >> >> Reviewed-by: Avri Altman <avri.altman@wdc.com> >> Reviewed-by: Bart Van Assche <bvanassche@acm.org> >> Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com> >> --- >> drivers/ufs/core/ufs-sysfs.c | 2 +- >> drivers/ufs/core/ufshcd.c | 23 +++++++++++------------ >> 2 files changed, 12 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c >> index 2c0b7f45de4b..117272cf7d61 100644 >> --- a/drivers/ufs/core/ufs-sysfs.c >> +++ b/drivers/ufs/core/ufs-sysfs.c >> @@ -230,7 +230,7 @@ static ssize_t wb_on_store(struct device *dev, struct device_attribute *attr, >> * If the platform supports UFSHCD_CAP_CLK_SCALING, turn WB >> * on/off will be done while clock scaling up/down. >> */ >> - dev_warn(dev, "To control WB through wb_on is not allowed!\n"); >> + dev_warn(dev, "It is not allowed to configure WB!\n"); >> return -EOPNOTSUPP; >> } >> >> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c >> index 5099d161f115..dcd7f03db2a2 100644 >> --- a/drivers/ufs/core/ufshcd.c >> +++ b/drivers/ufs/core/ufshcd.c >> @@ -5737,13 +5737,13 @@ int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable) >> >> ret = __ufshcd_wb_toggle(hba, enable, QUERY_FLAG_IDN_WB_EN); >> if (ret) { >> - dev_err(hba->dev, "%s Write Booster %s failed %d\n", >> - __func__, enable ? "enable" : "disable", ret); >> + dev_err(hba->dev, "%s: Write Booster %s failed %d\n", >> + __func__, enable ? "enabling" : "disabling", ret); >> return ret; >> } >> >> hba->dev_info.wb_enabled = enable; >> - dev_info(hba->dev, "%s Write Booster %s\n", >> + dev_info(hba->dev, "%s: Write Booster %s\n", >> __func__, enable ? "enabled" : "disabled"); > >You need to rebase this patch to follow the latest change as >https://lore.kernel.org/all/20220709000027.3929970-1-bjorn.andersson@linaro.org/ I am currently working on the latest 5.20/scsi-staging. In this case, can I refer the commit of 5.19/scsi-fixes and add commit to 5.20/scsi-staging? Or can I reflect it in my change? I have no experience, so please guide. :) Thank you for checking. Jinyoung.
hi Jinyoung, On Wed, Aug 3, 2022 at 12:11 PM Jinyoung CHOI <j-young.choi@samsung.com> wrote: > > Hi, Stanley, > > >Hi, > > > >On Tue, Aug 2, 2022 at 4:29 PM Jinyoung CHOI <j-young.choi@samsung.com> wrote: > >> > >> Messages are modified to fit the format of others. > >> > >> Reviewed-by: Avri Altman <avri.altman@wdc.com> > >> Reviewed-by: Bart Van Assche <bvanassche@acm.org> > >> Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com> > >> --- > >> drivers/ufs/core/ufs-sysfs.c | 2 +- > >> drivers/ufs/core/ufshcd.c | 23 +++++++++++------------ > >> 2 files changed, 12 insertions(+), 13 deletions(-) > >> > >> diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c > >> index 2c0b7f45de4b..117272cf7d61 100644 > >> --- a/drivers/ufs/core/ufs-sysfs.c > >> +++ b/drivers/ufs/core/ufs-sysfs.c > >> @@ -230,7 +230,7 @@ static ssize_t wb_on_store(struct device *dev, struct device_attribute *attr, > >> * If the platform supports UFSHCD_CAP_CLK_SCALING, turn WB > >> * on/off will be done while clock scaling up/down. > >> */ > >> - dev_warn(dev, "To control WB through wb_on is not allowed!\n"); > >> + dev_warn(dev, "It is not allowed to configure WB!\n"); > >> return -EOPNOTSUPP; > >> } > >> > >> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c > >> index 5099d161f115..dcd7f03db2a2 100644 > >> --- a/drivers/ufs/core/ufshcd.c > >> +++ b/drivers/ufs/core/ufshcd.c > >> @@ -5737,13 +5737,13 @@ int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable) > >> > >> ret = __ufshcd_wb_toggle(hba, enable, QUERY_FLAG_IDN_WB_EN); > >> if (ret) { > >> - dev_err(hba->dev, "%s Write Booster %s failed %d\n", > >> - __func__, enable ? "enable" : "disable", ret); > >> + dev_err(hba->dev, "%s: Write Booster %s failed %d\n", > >> + __func__, enable ? "enabling" : "disabling", ret); > >> return ret; > >> } > >> > >> hba->dev_info.wb_enabled = enable; > >> - dev_info(hba->dev, "%s Write Booster %s\n", > >> + dev_info(hba->dev, "%s: Write Booster %s\n", > >> __func__, enable ? "enabled" : "disabled"); > > > >You need to rebase this patch to follow the latest change as > >https://lore.kernel.org/all/20220709000027.3929970-1-bjorn.andersson@linaro.org/ > > I am currently working on the latest 5.20/scsi-staging. > In this case, can I refer the commit of 5.19/scsi-fixes > and add commit to 5.20/scsi-staging? > Or can I reflect it in my change? > I have no experience, so please guide. :) > > Thank you for checking. > Jinyoung. > > I did not notice that the below patch is merged to 5.19/scsi-fixes, so I guess perhaps you could keep working on the latest 5.20/scsi-staging, and then Martin would help resolve the conflict. https://lore.kernel.org/all/20220709000027.3929970-1-bjorn.andersson@linaro.org/ Thanks, Stanley
>hi Jinyoung, > >On Wed, Aug 3, 2022 at 12:11 PM Jinyoung CHOI <j-young.choi@samsung.com> wrote: >> >> Hi, Stanley, >> >> >Hi, >> > >> >On Tue, Aug 2, 2022 at 4:29 PM Jinyoung CHOI <j-young.choi@samsung.com> wrote: >> >> >> >> Messages are modified to fit the format of others. >> >> >> >> Reviewed-by: Avri Altman <avri.altman@wdc.com> >> >> Reviewed-by: Bart Van Assche <bvanassche@acm.org> >> >> Signed-off-by: Jinyoung Choi <j-young.choi@samsung.com> >> >> --- >> >> drivers/ufs/core/ufs-sysfs.c | 2 +- >> >> drivers/ufs/core/ufshcd.c | 23 +++++++++++------------ >> >> 2 files changed, 12 insertions(+), 13 deletions(-) >> >> >> >> diff --git a/drivers/ufs/core/ufs-sysfs.c b/drivers/ufs/core/ufs-sysfs.c >> >> index 2c0b7f45de4b..117272cf7d61 100644 >> >> --- a/drivers/ufs/core/ufs-sysfs.c >> >> +++ b/drivers/ufs/core/ufs-sysfs.c >> >> @@ -230,7 +230,7 @@ static ssize_t wb_on_store(struct device *dev, struct device_attribute *attr, >> >> * If the platform supports UFSHCD_CAP_CLK_SCALING, turn WB >> >> * on/off will be done while clock scaling up/down. >> >> */ >> >> - dev_warn(dev, "To control WB through wb_on is not allowed!\n"); >> >> + dev_warn(dev, "It is not allowed to configure WB!\n"); >> >> return -EOPNOTSUPP; >> >> } >> >> >> >> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c >> >> index 5099d161f115..dcd7f03db2a2 100644 >> >> --- a/drivers/ufs/core/ufshcd.c >> >> +++ b/drivers/ufs/core/ufshcd.c >> >> @@ -5737,13 +5737,13 @@ int ufshcd_wb_toggle(struct ufs_hba *hba, bool enable) >> >> >> >> ret = __ufshcd_wb_toggle(hba, enable, QUERY_FLAG_IDN_WB_EN); >> >> if (ret) { >> >> - dev_err(hba->dev, "%s Write Booster %s failed %d\n", >> >> - __func__, enable ? "enable" : "disable", ret); >> >> + dev_err(hba->dev, "%s: Write Booster %s failed %d\n", >> >> + __func__, enable ? "enabling" : "disabling", ret); >> >> return ret; >> >> } >> >> >> >> hba->dev_info.wb_enabled = enable; >> >> - dev_info(hba->dev, "%s Write Booster %s\n", >> >> + dev_info(hba->dev, "%s: Write Booster %s\n", >> >> __func__, enable ? "enabled" : "disabled"); >> > >> >You need to rebase this patch to follow the latest change as >> >https://lore.kernel.org/all/20220709000027.3929970-1-bjorn.andersson@linaro.org/ >> >> I am currently working on the latest 5.20/scsi-staging. >> In this case, can I refer the commit of 5.19/scsi-fixes >> and add commit to 5.20/scsi-staging? >> Or can I reflect it in my change? >> I have no experience, so please guide. :) >> >> Thank you for checking. >> Jinyoung. >> >> > >I did not notice that the below patch is merged to 5.19/scsi-fixes, so >I guess perhaps you could keep working on the latest >5.20/scsi-staging, and then Martin would help resolve the conflict. >https://lore.kernel.org/all/20220709000027.3929970-1-bjorn.andersson@linaro.org/ > >Thanks, > >Stanley It has been applied to 5.19/scsi-fixes, so I will modify the debug level at my change for sync. Best Regards, Jinyoung.
On 8/2/22 01:07, Jinyoung CHOI wrote: > +What: /sys/bus/platform/drivers/ufshcd/*/wb_buf_flush_en > +What: /sys/bus/platform/devices/*.ufs/wb_buf_flush_en > +Date: July 2022 > +Contact: Jinyoung Choi <j-young.choi@samsung.com> > +Description: This entry shows the status of WriteBooster buffer flushing Can we rename this attribute into something that has a word order that is grammatically correct, e.g. enable_wb_buf_flush? > + and it can be used to allow or disallow the flushing. > + If the flushing is allowed, the device executes the flush > + operation when the command queue is empty. The attribute has "enabled" in its name while the above text uses the verb "allowed". Consider changing "allowed" into "enabled". Please also change "If the flushing" into "If flushing". Thanks, Bart.
>On 8/2/22 01:07, Jinyoung CHOI wrote: >> +What: /sys/bus/platform/drivers/ufshcd/*/wb_buf_flush_en >> +What: /sys/bus/platform/devices/*.ufs/wb_buf_flush_en >> +Date: July 2022 >> +Contact: Jinyoung Choi <j-young.choi@samsung.com> >> +Description: This entry shows the status of WriteBooster buffer flushing > >Can we rename this attribute into something that has a word order that >is grammatically correct, e.g. enable_wb_buf_flush? > OK, I will replace it. Instead, When the list is printed through "ls", it may be difficult to check because the prefix is different. >> + and it can be used to allow or disallow the flushing. >> + If the flushing is allowed, the device executes the flush >> + operation when the command queue is empty. > >The attribute has "enabled" in its name while the above text uses the >verb "allowed". Consider changing "allowed" into "enabled". Please also >change "If the flushing" into "If flushing". > >Thanks, > >Bart OK, I will apply next patch. Warm Regards, Jinyoung.