mbox series

[v2,0/2] Two changes related with UFS clock scaling

Message ID 1607520942-22254-1-git-send-email-cang@codeaurora.org
Headers show
Series Two changes related with UFS clock scaling | expand

Message

Can Guo Dec. 9, 2020, 1:35 p.m. UTC
This series is made based on 5.11/scsi-fixes branch.
The 1st change allows contexts to prevent clock scaling from being invoked through sysfs nodes like clkscale_enable.
The 2nd change is just a minor code cleanup.

Change since v1:
- Updated the 2nd change

Can Guo (2):
  scsi: ufs: Protect some contexts from unexpected clock scaling
  scsi: ufs: Clean up ufshcd_exit_clk_scaling/gating()

 drivers/scsi/ufs/ufshcd.c | 116 ++++++++++++++++++++++++++--------------------
 drivers/scsi/ufs/ufshcd.h |   6 +++
 2 files changed, 71 insertions(+), 51 deletions(-)

Comments

Bean Huo Dec. 10, 2020, 6:03 p.m. UTC | #1
On Wed, 2020-12-09 at 05:35 -0800, Can Guo wrote:
> ufshcd_hba_exit() is always called after ufshcd_exit_clk_scaling()

> and

> ufshcd_exit_clk_gating(), so move ufshcd_exit_clk_scaling/gating() to

> ufshcd_hba_exit().

> 

> Signed-off-by: Can Guo <cang@codeaurora.org>

> 

> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c

> index 12266bd..41a12d6 100644

> --- a/drivers/scsi/ufs/ufshcd.c

> +++ b/drivers/scsi/ufs/ufshcd.c

> @@ -1846,11 +1846,14 @@ static void ufshcd_init_clk_scaling(struct

> ufs_hba *hba)

>         snprintf(wq_name, sizeof(wq_name), "ufs_clkscaling_%d",

>                  hba->host->host_no);

>         hba->clk_scaling.workq =

> create_singlethread_workqueue(wq_name);

> +

> +       hba->clk_scaling.is_initialized = true;

>  }

>  

>  static void ufshcd_exit_clk_scaling(struct ufs_hba *hba)

>  {

> -       if (!ufshcd_is_clkscaling_supported(hba))

> +       if (!ufshcd_is_clkscaling_supported(hba) ||

> +           !hba->clk_scaling.is_initialized)

>                 return;

>  

>         if (hba->devfreq)

> @@ -1894,12 +1897,16 @@ static void ufshcd_init_clk_gating(struct

> ufs_hba *hba)

>         hba->clk_gating.enable_attr.attr.mode = 0644;

>         if (device_create_file(hba->dev, &hba-

> >clk_gating.enable_attr))

>                 dev_err(hba->dev, "Failed to create sysfs for

> clkgate_enable\n");

> +

> +       hba->clk_gating.is_initialized = true;

>  }


you don't need these two is_initialized at all. they are only be false
when scaling/gating is not supported??

Bean
Can Guo Dec. 11, 2020, 1:11 a.m. UTC | #2
On 2020-12-11 02:03, Bean Huo wrote:
> On Wed, 2020-12-09 at 05:35 -0800, Can Guo wrote:

>> ufshcd_hba_exit() is always called after ufshcd_exit_clk_scaling()

>> and

>> ufshcd_exit_clk_gating(), so move ufshcd_exit_clk_scaling/gating() to

>> ufshcd_hba_exit().

>> 

>> Signed-off-by: Can Guo <cang@codeaurora.org>

>> 

>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c

>> index 12266bd..41a12d6 100644

>> --- a/drivers/scsi/ufs/ufshcd.c

>> +++ b/drivers/scsi/ufs/ufshcd.c

>> @@ -1846,11 +1846,14 @@ static void ufshcd_init_clk_scaling(struct

>> ufs_hba *hba)

>>         snprintf(wq_name, sizeof(wq_name), "ufs_clkscaling_%d",

>>                  hba->host->host_no);

>>         hba->clk_scaling.workq =

>> create_singlethread_workqueue(wq_name);

>> +

>> +       hba->clk_scaling.is_initialized = true;

>>  }

>> 

>>  static void ufshcd_exit_clk_scaling(struct ufs_hba *hba)

>>  {

>> -       if (!ufshcd_is_clkscaling_supported(hba))

>> +       if (!ufshcd_is_clkscaling_supported(hba) ||

>> +           !hba->clk_scaling.is_initialized)

>>                 return;

>> 

>>         if (hba->devfreq)

>> @@ -1894,12 +1897,16 @@ static void ufshcd_init_clk_gating(struct

>> ufs_hba *hba)

>>         hba->clk_gating.enable_attr.attr.mode = 0644;

>>         if (device_create_file(hba->dev, &hba-

>> >clk_gating.enable_attr))

>>                 dev_err(hba->dev, "Failed to create sysfs for

>> clkgate_enable\n");

>> +

>> +       hba->clk_gating.is_initialized = true;

>>  }

> 

> you don't need these two is_initialized at all. they are only be false

> when scaling/gating is not supported??

> 

> Bean


In the case of scaling/gating are supported, the flags are used in
ufshcd_exit_clk_scaling/gating() - when ufshcd_hba_exit() calls
ufshcd_exit_clk_scaling/gating(), the two funcs need to make sure
they really have something to remove, otherwise NULL pointer issues.

Can Guo.