* Re: [PATCH] staging: rts5208: Replace delay function.
2023-10-18 1:03 [PATCH] staging: rts5208: Replace delay function kenechukwu maduechesi
@ 2023-10-18 3:38 ` Andi Shyti
2023-10-18 7:03 ` Julia Lawall
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Andi Shyti @ 2023-10-18 3:38 UTC (permalink / raw)
To: kenechukwu maduechesi
Cc: outreachy, shreeya.patel23498, Greg Kroah-Hartman, linux-staging,
linux-kernel
Hi Kenechukwu,
On Tue, Oct 17, 2023 at 06:03:05PM -0700, kenechukwu maduechesi wrote:
> Replace udelay() with usleep_range() for more precise delay handling.
The reason is not the precise handling, quite the opposite.
> Reported by checkpatch:
>
> CHECK: usleep_range is preferred over udelay
Can you tell why?[*]
> Signed-off-by: kenechukwu maduechesi <maduechesik@gmail.com>
> ---
> drivers/staging/rts5208/sd.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/rts5208/sd.c b/drivers/staging/rts5208/sd.c
> index 74c4f476b3a4..059f99b0a727 100644
> --- a/drivers/staging/rts5208/sd.c
> +++ b/drivers/staging/rts5208/sd.c
> @@ -865,7 +865,7 @@ static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir)
> PHASE_CHANGE);
> if (retval)
> return retval;
> - udelay(50);
> + usleep_range(50);
What is the range you will be sleeping, now?
Andi
[*] Documentation/timers/timers-howto.rst
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] staging: rts5208: Replace delay function.
2023-10-18 1:03 [PATCH] staging: rts5208: Replace delay function kenechukwu maduechesi
2023-10-18 3:38 ` Andi Shyti
@ 2023-10-18 7:03 ` Julia Lawall
2023-10-18 7:32 ` Karolina Stolarek
2023-10-18 10:38 ` Andi Shyti
2023-10-18 8:04 ` Karolina Stolarek
` (2 subsequent siblings)
4 siblings, 2 replies; 14+ messages in thread
From: Julia Lawall @ 2023-10-18 7:03 UTC (permalink / raw)
To: kenechukwu maduechesi
Cc: outreachy, shreeya.patel23498, Greg Kroah-Hartman, linux-staging,
linux-kernel
On Tue, 17 Oct 2023, kenechukwu maduechesi wrote:
> Replace udelay() with usleep_range() for more precise delay handling.
>
> Reported by checkpatch:
>
> CHECK: usleep_range is preferred over udelay
This message is typically not a good candidate for outreachy patches,
because you need access to the device to be sure that any change is
correct.
julia
>
> Signed-off-by: kenechukwu maduechesi <maduechesik@gmail.com>
> ---
> drivers/staging/rts5208/sd.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/rts5208/sd.c b/drivers/staging/rts5208/sd.c
> index 74c4f476b3a4..059f99b0a727 100644
> --- a/drivers/staging/rts5208/sd.c
> +++ b/drivers/staging/rts5208/sd.c
> @@ -865,7 +865,7 @@ static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir)
> PHASE_CHANGE);
> if (retval)
> return retval;
> - udelay(50);
> + usleep_range(50);
> retval = rtsx_write_register(chip, SD_VP_CTL, 0xFF,
> PHASE_CHANGE |
> PHASE_NOT_RESET |
> @@ -877,14 +877,14 @@ static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir)
> CHANGE_CLK, CHANGE_CLK);
> if (retval)
> return retval;
> - udelay(50);
> + usleep_range(50);
> retval = rtsx_write_register(chip, SD_VP_CTL, 0xFF,
> PHASE_NOT_RESET |
> sample_point);
> if (retval)
> return retval;
> }
> - udelay(100);
> + usleep_range(100);
>
> rtsx_init_cmd(chip);
> rtsx_add_cmd(chip, WRITE_REG_CMD, SD_DCMPS_CTL, DCMPS_CHANGE,
> @@ -918,7 +918,7 @@ static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir)
> return retval;
> }
>
> - udelay(50);
> + usleep_range(50);
> }
>
> retval = rtsx_write_register(chip, SD_CFG1, SD_ASYNC_FIFO_NOT_RST, 0);
> @@ -1416,7 +1416,7 @@ static int sd_wait_data_idle(struct rtsx_chip *chip)
> retval = STATUS_SUCCESS;
> break;
> }
> - udelay(100);
> + usleep_range(100);
> }
> dev_dbg(rtsx_dev(chip), "SD_DATA_STATE: 0x%02x\n", val);
>
> --
> 2.25.1
>
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] staging: rts5208: Replace delay function.
2023-10-18 7:03 ` Julia Lawall
@ 2023-10-18 7:32 ` Karolina Stolarek
2023-10-18 7:45 ` Greg Kroah-Hartman
2023-10-18 10:42 ` Dan Carpenter
2023-10-18 10:38 ` Andi Shyti
1 sibling, 2 replies; 14+ messages in thread
From: Karolina Stolarek @ 2023-10-18 7:32 UTC (permalink / raw)
To: Julia Lawall
Cc: kenechukwu maduechesi, outreachy, shreeya.patel23498,
Greg Kroah-Hartman, linux-staging, linux-kernel
On 18.10.2023 09:03, Julia Lawall wrote:
>
>
> On Tue, 17 Oct 2023, kenechukwu maduechesi wrote:
>
>> Replace udelay() with usleep_range() for more precise delay handling.
>>
>> Reported by checkpatch:
>>
>> CHECK: usleep_range is preferred over udelay
>
> This message is typically not a good candidate for outreachy patches,
> because you need access to the device to be sure that any change is
> correct.
Could we add a paragraph on how to pick good checkpatch.pl error to fix
to the Outreachyfirstpatch docs? This could go to "Find a driver to
clean up" section, for example.
All the best,
Karolina
>
> julia
>
>>
>> Signed-off-by: kenechukwu maduechesi <maduechesik@gmail.com>
>> ---
>> drivers/staging/rts5208/sd.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/staging/rts5208/sd.c b/drivers/staging/rts5208/sd.c
>> index 74c4f476b3a4..059f99b0a727 100644
>> --- a/drivers/staging/rts5208/sd.c
>> +++ b/drivers/staging/rts5208/sd.c
>> @@ -865,7 +865,7 @@ static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir)
>> PHASE_CHANGE);
>> if (retval)
>> return retval;
>> - udelay(50);
>> + usleep_range(50);
>> retval = rtsx_write_register(chip, SD_VP_CTL, 0xFF,
>> PHASE_CHANGE |
>> PHASE_NOT_RESET |
>> @@ -877,14 +877,14 @@ static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir)
>> CHANGE_CLK, CHANGE_CLK);
>> if (retval)
>> return retval;
>> - udelay(50);
>> + usleep_range(50);
>> retval = rtsx_write_register(chip, SD_VP_CTL, 0xFF,
>> PHASE_NOT_RESET |
>> sample_point);
>> if (retval)
>> return retval;
>> }
>> - udelay(100);
>> + usleep_range(100);
>>
>> rtsx_init_cmd(chip);
>> rtsx_add_cmd(chip, WRITE_REG_CMD, SD_DCMPS_CTL, DCMPS_CHANGE,
>> @@ -918,7 +918,7 @@ static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir)
>> return retval;
>> }
>>
>> - udelay(50);
>> + usleep_range(50);
>> }
>>
>> retval = rtsx_write_register(chip, SD_CFG1, SD_ASYNC_FIFO_NOT_RST, 0);
>> @@ -1416,7 +1416,7 @@ static int sd_wait_data_idle(struct rtsx_chip *chip)
>> retval = STATUS_SUCCESS;
>> break;
>> }
>> - udelay(100);
>> + usleep_range(100);
>> }
>> dev_dbg(rtsx_dev(chip), "SD_DATA_STATE: 0x%02x\n", val);
>>
>> --
>> 2.25.1
>>
>>
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] staging: rts5208: Replace delay function.
2023-10-18 7:32 ` Karolina Stolarek
@ 2023-10-18 7:45 ` Greg Kroah-Hartman
2023-10-18 7:52 ` Karolina Stolarek
2023-10-18 10:42 ` Dan Carpenter
1 sibling, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2023-10-18 7:45 UTC (permalink / raw)
To: Karolina Stolarek
Cc: Julia Lawall, kenechukwu maduechesi, outreachy,
shreeya.patel23498, linux-staging, linux-kernel
On Wed, Oct 18, 2023 at 09:32:46AM +0200, Karolina Stolarek wrote:
> On 18.10.2023 09:03, Julia Lawall wrote:
> >
> >
> > On Tue, 17 Oct 2023, kenechukwu maduechesi wrote:
> >
> > > Replace udelay() with usleep_range() for more precise delay handling.
> > >
> > > Reported by checkpatch:
> > >
> > > CHECK: usleep_range is preferred over udelay
> >
> > This message is typically not a good candidate for outreachy patches,
> > because you need access to the device to be sure that any change is
> > correct.
>
> Could we add a paragraph on how to pick good checkpatch.pl error to fix to
> the Outreachyfirstpatch docs? This could go to "Find a driver to clean up"
> section, for example.
The ability to find a "good" error changes over time, so this might be
hard to do.
good luck!
greg k-h
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] staging: rts5208: Replace delay function.
2023-10-18 7:45 ` Greg Kroah-Hartman
@ 2023-10-18 7:52 ` Karolina Stolarek
2023-10-18 10:28 ` Julia Lawall
0 siblings, 1 reply; 14+ messages in thread
From: Karolina Stolarek @ 2023-10-18 7:52 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Julia Lawall, kenechukwu maduechesi, outreachy,
shreeya.patel23498, linux-staging, linux-kernel
On 18.10.2023 09:45, Greg Kroah-Hartman wrote:
> On Wed, Oct 18, 2023 at 09:32:46AM +0200, Karolina Stolarek wrote:
>> On 18.10.2023 09:03, Julia Lawall wrote:
>>>
>>>
>>> On Tue, 17 Oct 2023, kenechukwu maduechesi wrote:
>>>
>>>> Replace udelay() with usleep_range() for more precise delay handling.
>>>>
>>>> Reported by checkpatch:
>>>>
>>>> CHECK: usleep_range is preferred over udelay
>>>
>>> This message is typically not a good candidate for outreachy patches,
>>> because you need access to the device to be sure that any change is
>>> correct.
>>
>> Could we add a paragraph on how to pick good checkpatch.pl error to fix to
>> the Outreachyfirstpatch docs? This could go to "Find a driver to clean up"
>> section, for example.
>
> The ability to find a "good" error changes over time, so this might be
> hard to do.
I agree, but we can all agree that experimenting with udelay during
Outreachy is not a good idea, and people should know about it
All the best,
Karolina
>
> good luck!
>
> greg k-h
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] staging: rts5208: Replace delay function.
2023-10-18 7:52 ` Karolina Stolarek
@ 2023-10-18 10:28 ` Julia Lawall
2023-10-18 10:40 ` Karolina Stolarek
0 siblings, 1 reply; 14+ messages in thread
From: Julia Lawall @ 2023-10-18 10:28 UTC (permalink / raw)
To: Karolina Stolarek
Cc: Greg Kroah-Hartman, kenechukwu maduechesi, outreachy,
shreeya.patel23498, linux-staging, linux-kernel
On Wed, 18 Oct 2023, Karolina Stolarek wrote:
> On 18.10.2023 09:45, Greg Kroah-Hartman wrote:
> > On Wed, Oct 18, 2023 at 09:32:46AM +0200, Karolina Stolarek wrote:
> > > On 18.10.2023 09:03, Julia Lawall wrote:
> > > >
> > > >
> > > > On Tue, 17 Oct 2023, kenechukwu maduechesi wrote:
> > > >
> > > > > Replace udelay() with usleep_range() for more precise delay handling.
> > > > >
> > > > > Reported by checkpatch:
> > > > >
> > > > > CHECK: usleep_range is preferred over udelay
> > > >
> > > > This message is typically not a good candidate for outreachy patches,
> > > > because you need access to the device to be sure that any change is
> > > > correct.
> > >
> > > Could we add a paragraph on how to pick good checkpatch.pl error to fix to
> > > the Outreachyfirstpatch docs? This could go to "Find a driver to clean up"
> > > section, for example.
> >
> > The ability to find a "good" error changes over time, so this might be
> > hard to do.
>
> I agree, but we can all agree that experimenting with udelay during Outreachy
> is not a good idea, and people should know about it
In general, I think that it is better in the contribution period to do the
wrong thing and then learn about why it is wrong, but this case comes up
over and over, and it is always not the right thing to do, so I added an
appropriate explanation. Thanks for the suggestion.
julia
>
> All the best,
> Karolina
>
> >
> > good luck!
> >
> > greg k-h
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] staging: rts5208: Replace delay function.
2023-10-18 10:28 ` Julia Lawall
@ 2023-10-18 10:40 ` Karolina Stolarek
2023-10-18 22:17 ` Alison Schofield
0 siblings, 1 reply; 14+ messages in thread
From: Karolina Stolarek @ 2023-10-18 10:40 UTC (permalink / raw)
To: Julia Lawall
Cc: Greg Kroah-Hartman, kenechukwu maduechesi, outreachy,
shreeya.patel23498, linux-staging, linux-kernel
On 18.10.2023 12:28, Julia Lawall wrote:
>
>
> On Wed, 18 Oct 2023, Karolina Stolarek wrote:
>
>> On 18.10.2023 09:45, Greg Kroah-Hartman wrote:
>>> On Wed, Oct 18, 2023 at 09:32:46AM +0200, Karolina Stolarek wrote:
>>>> On 18.10.2023 09:03, Julia Lawall wrote:
>>>>>
>>>>>
>>>>> On Tue, 17 Oct 2023, kenechukwu maduechesi wrote:
>>>>>
>>>>>> Replace udelay() with usleep_range() for more precise delay handling.
>>>>>>
>>>>>> Reported by checkpatch:
>>>>>>
>>>>>> CHECK: usleep_range is preferred over udelay
>>>>>
>>>>> This message is typically not a good candidate for outreachy patches,
>>>>> because you need access to the device to be sure that any change is
>>>>> correct.
>>>>
>>>> Could we add a paragraph on how to pick good checkpatch.pl error to fix to
>>>> the Outreachyfirstpatch docs? This could go to "Find a driver to clean up"
>>>> section, for example.
>>>
>>> The ability to find a "good" error changes over time, so this might be
>>> hard to do.
>>
>> I agree, but we can all agree that experimenting with udelay during Outreachy
>> is not a good idea, and people should know about it
>
> In general, I think that it is better in the contribution period to do the
> wrong thing and then learn about why it is wrong, but this case comes up
> over and over, and it is always not the right thing to do, so I added an
> appropriate explanation. Thanks for the suggestion.
Absolutely. Thanks for the docs update. Still, one thing -- is empty
section after "Some drivers that are on their way out of the kernel
are:" intentional?
All the best,
Karolina
>
> julia
>
>>
>> All the best,
>> Karolina
>>
>>>
>>> good luck!
>>>
>>> greg k-h
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] staging: rts5208: Replace delay function.
2023-10-18 10:40 ` Karolina Stolarek
@ 2023-10-18 22:17 ` Alison Schofield
0 siblings, 0 replies; 14+ messages in thread
From: Alison Schofield @ 2023-10-18 22:17 UTC (permalink / raw)
To: Karolina Stolarek
Cc: Julia Lawall, Greg Kroah-Hartman, kenechukwu maduechesi,
outreachy, shreeya.patel23498, linux-staging, linux-kernel
On Wed, Oct 18, 2023 at 12:40:33PM +0200, Karolina Stolarek wrote:
> On 18.10.2023 12:28, Julia Lawall wrote:
> >
> >
> > On Wed, 18 Oct 2023, Karolina Stolarek wrote:
> >
> > > On 18.10.2023 09:45, Greg Kroah-Hartman wrote:
> > > > On Wed, Oct 18, 2023 at 09:32:46AM +0200, Karolina Stolarek wrote:
> > > > > On 18.10.2023 09:03, Julia Lawall wrote:
> > > > > >
> > > > > >
> > > > > > On Tue, 17 Oct 2023, kenechukwu maduechesi wrote:
> > > > > >
> > > > > > > Replace udelay() with usleep_range() for more precise delay handling.
> > > > > > >
> > > > > > > Reported by checkpatch:
> > > > > > >
> > > > > > > CHECK: usleep_range is preferred over udelay
> > > > > >
> > > > > > This message is typically not a good candidate for outreachy patches,
> > > > > > because you need access to the device to be sure that any change is
> > > > > > correct.
> > > > >
> > > > > Could we add a paragraph on how to pick good checkpatch.pl error to fix to
> > > > > the Outreachyfirstpatch docs? This could go to "Find a driver to clean up"
> > > > > section, for example.
> > > >
> > > > The ability to find a "good" error changes over time, so this might be
> > > > hard to do.
> > >
> > > I agree, but we can all agree that experimenting with udelay during Outreachy
> > > is not a good idea, and people should know about it
> >
> > In general, I think that it is better in the contribution period to do the
> > wrong thing and then learn about why it is wrong, but this case comes up
> > over and over, and it is always not the right thing to do, so I added an
> > appropriate explanation. Thanks for the suggestion.
>
> Absolutely. Thanks for the docs update. Still, one thing -- is empty section
> after "Some drivers that are on their way out of the kernel are:"
> intentional?
>
> All the best,
> Karolina
Perhaps an applicant can expand on that "Find a driver to clean up..."
section with a overview of how to use the Outreachy Mailing list,
or any Mailing list to search for patches that cleaned up up the
checkpatch error/warning/check that they are examining.
Here's the query for the udelay one:
https://lore.kernel.org/outreachy/?q=%22CHECK%3A+usleep_range+is+preferred+over+udelay%22
When submitters include the actual checkpatch string in their commit log
it very easy to find and reference.
For the first patch tutorial, I'll suggest something as simple as:
1) goto https://lore.kernel.org/outreachy/
2) enter the checkpatch string in the 'search' box
3) boom - go see what others have done with this checkpatch error
Or, if someone wants to get fancy they can add screenshots :)
Note, that we really like to keep the language similar on these
cleanups. Find a few references, and do as they did.
>
> >
> > julia
> >
> > >
> > > All the best,
> > > Karolina
> > >
> > > >
> > > > good luck!
> > > >
> > > > greg k-h
> > >
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] staging: rts5208: Replace delay function.
2023-10-18 7:32 ` Karolina Stolarek
2023-10-18 7:45 ` Greg Kroah-Hartman
@ 2023-10-18 10:42 ` Dan Carpenter
1 sibling, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2023-10-18 10:42 UTC (permalink / raw)
To: Karolina Stolarek
Cc: Julia Lawall, kenechukwu maduechesi, outreachy,
shreeya.patel23498, Greg Kroah-Hartman, linux-staging,
linux-kernel
On Wed, Oct 18, 2023 at 09:32:46AM +0200, Karolina Stolarek wrote:
> On 18.10.2023 09:03, Julia Lawall wrote:
> >
> >
> > On Tue, 17 Oct 2023, kenechukwu maduechesi wrote:
> >
> > > Replace udelay() with usleep_range() for more precise delay handling.
> > >
> > > Reported by checkpatch:
> > >
> > > CHECK: usleep_range is preferred over udelay
> >
> > This message is typically not a good candidate for outreachy patches,
> > because you need access to the device to be sure that any change is
> > correct.
>
> Could we add a paragraph on how to pick good checkpatch.pl error to fix to
> the Outreachyfirstpatch docs? This could go to "Find a driver to clean up"
> section, for example.
I think you should put a note about usleep_range() and the extra
parentheses one.
Also say something about looking up stuff on lore.
https://lore.kernel.org/all/?q=sd_change_phase%20usleep_range
In this case someone tried to update this before. The patch wasn't
merged but it wasn't clearly explained to the developer why the patch
wasn't merged. Ah well.
Generally fresh warnings are better than old warnings because we fix the
simple stuff where checkpatch is obviously correct.
The other thing is that people really need to look at the wider context.
Look at the surrounding code. Look at when the checkpatch warning was
introduced and try to put yourself in the developer's head to figure out
what they were thinking. Are there other opportunities for cleanups
nearby.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] staging: rts5208: Replace delay function.
2023-10-18 7:03 ` Julia Lawall
2023-10-18 7:32 ` Karolina Stolarek
@ 2023-10-18 10:38 ` Andi Shyti
1 sibling, 0 replies; 14+ messages in thread
From: Andi Shyti @ 2023-10-18 10:38 UTC (permalink / raw)
To: Julia Lawall
Cc: kenechukwu maduechesi, outreachy, shreeya.patel23498,
Greg Kroah-Hartman, linux-staging, linux-kernel
Hi Julia,
> > Replace udelay() with usleep_range() for more precise delay handling.
> >
> > Reported by checkpatch:
> >
> > CHECK: usleep_range is preferred over udelay
>
> This message is typically not a good candidate for outreachy patches,
> because you need access to the device to be sure that any change is
> correct.
I actually don't really mind this patch... I would exchange these
udelay() with almost anything, they look to me placed a bit
random anyway (without going too much in detail).
But in general, for this project, I think you are right and it's
a good idea not to blindly change delay and sleeping functions
without really knowing what you are doing.
Andi
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] staging: rts5208: Replace delay function.
2023-10-18 1:03 [PATCH] staging: rts5208: Replace delay function kenechukwu maduechesi
2023-10-18 3:38 ` Andi Shyti
2023-10-18 7:03 ` Julia Lawall
@ 2023-10-18 8:04 ` Karolina Stolarek
2023-10-23 23:09 ` kernel test robot
2023-11-05 20:58 ` kernel test robot
4 siblings, 0 replies; 14+ messages in thread
From: Karolina Stolarek @ 2023-10-18 8:04 UTC (permalink / raw)
To: kenechukwu maduechesi
Cc: outreachy, shreeya.patel23498, Greg Kroah-Hartman, linux-staging,
linux-kernel
On 18.10.2023 03:03, kenechukwu maduechesi wrote:
> Replace udelay() with usleep_range() for more precise delay handling.
>
> Reported by checkpatch:
>
> CHECK: usleep_range is preferred over udelay
Have you tried to compile your patch? This is something you should do
before sending it to the mailing list.
All the best,
Karolina
>
> Signed-off-by: kenechukwu maduechesi <maduechesik@gmail.com>
> ---
> drivers/staging/rts5208/sd.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/rts5208/sd.c b/drivers/staging/rts5208/sd.c
> index 74c4f476b3a4..059f99b0a727 100644
> --- a/drivers/staging/rts5208/sd.c
> +++ b/drivers/staging/rts5208/sd.c
> @@ -865,7 +865,7 @@ static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir)
> PHASE_CHANGE);
> if (retval)
> return retval;
> - udelay(50);
> + usleep_range(50);
> retval = rtsx_write_register(chip, SD_VP_CTL, 0xFF,
> PHASE_CHANGE |
> PHASE_NOT_RESET |
> @@ -877,14 +877,14 @@ static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir)
> CHANGE_CLK, CHANGE_CLK);
> if (retval)
> return retval;
> - udelay(50);
> + usleep_range(50);
> retval = rtsx_write_register(chip, SD_VP_CTL, 0xFF,
> PHASE_NOT_RESET |
> sample_point);
> if (retval)
> return retval;
> }
> - udelay(100);
> + usleep_range(100);
>
> rtsx_init_cmd(chip);
> rtsx_add_cmd(chip, WRITE_REG_CMD, SD_DCMPS_CTL, DCMPS_CHANGE,
> @@ -918,7 +918,7 @@ static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir)
> return retval;
> }
>
> - udelay(50);
> + usleep_range(50);
> }
>
> retval = rtsx_write_register(chip, SD_CFG1, SD_ASYNC_FIFO_NOT_RST, 0);
> @@ -1416,7 +1416,7 @@ static int sd_wait_data_idle(struct rtsx_chip *chip)
> retval = STATUS_SUCCESS;
> break;
> }
> - udelay(100);
> + usleep_range(100);
> }
> dev_dbg(rtsx_dev(chip), "SD_DATA_STATE: 0x%02x\n", val);
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] staging: rts5208: Replace delay function.
2023-10-18 1:03 [PATCH] staging: rts5208: Replace delay function kenechukwu maduechesi
` (2 preceding siblings ...)
2023-10-18 8:04 ` Karolina Stolarek
@ 2023-10-23 23:09 ` kernel test robot
2023-11-05 20:58 ` kernel test robot
4 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2023-10-23 23:09 UTC (permalink / raw)
To: kenechukwu maduechesi, outreachy, shreeya.patel23498
Cc: oe-kbuild-all, Greg Kroah-Hartman, linux-staging, linux-kernel
Hi kenechukwu,
kernel test robot noticed the following build errors:
[auto build test ERROR on staging/staging-testing]
url: https://github.com/intel-lab-lkp/linux/commits/kenechukwu-maduechesi/staging-rts5208-Replace-delay-function/20231018-090457
base: staging/staging-testing
patch link: https://lore.kernel.org/r/20231018004300.GA3189%40ubuntu
patch subject: [PATCH] staging: rts5208: Replace delay function.
config: microblaze-allyesconfig (https://download.01.org/0day-ci/archive/20231024/202310240604.OZnfTcMT-lkp@intel.com/config)
compiler: microblaze-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231024/202310240604.OZnfTcMT-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310240604.OZnfTcMT-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/staging/rts5208/sd.c: In function 'sd_change_phase':
>> drivers/staging/rts5208/sd.c:868:25: error: too few arguments to function 'usleep_range'
868 | usleep_range(50);
| ^~~~~~~~~~~~
In file included from drivers/staging/rts5208/rtsx.h:17,
from drivers/staging/rts5208/sd.c:16:
include/linux/delay.h:66:20: note: declared here
66 | static inline void usleep_range(unsigned long min, unsigned long max)
| ^~~~~~~~~~~~
drivers/staging/rts5208/sd.c:880:25: error: too few arguments to function 'usleep_range'
880 | usleep_range(50);
| ^~~~~~~~~~~~
include/linux/delay.h:66:20: note: declared here
66 | static inline void usleep_range(unsigned long min, unsigned long max)
| ^~~~~~~~~~~~
drivers/staging/rts5208/sd.c:887:17: error: too few arguments to function 'usleep_range'
887 | usleep_range(100);
| ^~~~~~~~~~~~
include/linux/delay.h:66:20: note: declared here
66 | static inline void usleep_range(unsigned long min, unsigned long max)
| ^~~~~~~~~~~~
drivers/staging/rts5208/sd.c:921:17: error: too few arguments to function 'usleep_range'
921 | usleep_range(50);
| ^~~~~~~~~~~~
include/linux/delay.h:66:20: note: declared here
66 | static inline void usleep_range(unsigned long min, unsigned long max)
| ^~~~~~~~~~~~
drivers/staging/rts5208/sd.c: In function 'sd_wait_data_idle':
drivers/staging/rts5208/sd.c:1419:17: error: too few arguments to function 'usleep_range'
1419 | usleep_range(100);
| ^~~~~~~~~~~~
include/linux/delay.h:66:20: note: declared here
66 | static inline void usleep_range(unsigned long min, unsigned long max)
| ^~~~~~~~~~~~
vim +/usleep_range +868 drivers/staging/rts5208/sd.c
814
815 static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir)
816 {
817 struct sd_info *sd_card = &chip->sd_card;
818 u16 SD_VP_CTL, SD_DCMPS_CTL;
819 u8 val;
820 int retval;
821 bool ddr_rx = false;
822
823 dev_dbg(rtsx_dev(chip), "%s (sample_point = %d, tune_dir = %d)\n",
824 __func__, sample_point, tune_dir);
825
826 if (tune_dir == TUNE_RX) {
827 SD_VP_CTL = SD_VPRX_CTL;
828 SD_DCMPS_CTL = SD_DCMPS_RX_CTL;
829 if (CHK_SD_DDR50(sd_card))
830 ddr_rx = true;
831 } else {
832 SD_VP_CTL = SD_VPTX_CTL;
833 SD_DCMPS_CTL = SD_DCMPS_TX_CTL;
834 }
835
836 if (chip->asic_code) {
837 retval = rtsx_write_register(chip, CLK_CTL, CHANGE_CLK,
838 CHANGE_CLK);
839 if (retval)
840 return retval;
841 retval = rtsx_write_register(chip, SD_VP_CTL, 0x1F,
842 sample_point);
843 if (retval)
844 return retval;
845 retval = rtsx_write_register(chip, SD_VPCLK0_CTL,
846 PHASE_NOT_RESET, 0);
847 if (retval)
848 return retval;
849 retval = rtsx_write_register(chip, SD_VPCLK0_CTL,
850 PHASE_NOT_RESET, PHASE_NOT_RESET);
851 if (retval)
852 return retval;
853 retval = rtsx_write_register(chip, CLK_CTL, CHANGE_CLK, 0);
854 if (retval)
855 return retval;
856 } else {
857 rtsx_read_register(chip, SD_VP_CTL, &val);
858 dev_dbg(rtsx_dev(chip), "SD_VP_CTL: 0x%x\n", val);
859 rtsx_read_register(chip, SD_DCMPS_CTL, &val);
860 dev_dbg(rtsx_dev(chip), "SD_DCMPS_CTL: 0x%x\n", val);
861
862 if (ddr_rx) {
863 retval = rtsx_write_register(chip, SD_VP_CTL,
864 PHASE_CHANGE,
865 PHASE_CHANGE);
866 if (retval)
867 return retval;
> 868 usleep_range(50);
869 retval = rtsx_write_register(chip, SD_VP_CTL, 0xFF,
870 PHASE_CHANGE |
871 PHASE_NOT_RESET |
872 sample_point);
873 if (retval)
874 return retval;
875 } else {
876 retval = rtsx_write_register(chip, CLK_CTL,
877 CHANGE_CLK, CHANGE_CLK);
878 if (retval)
879 return retval;
880 usleep_range(50);
881 retval = rtsx_write_register(chip, SD_VP_CTL, 0xFF,
882 PHASE_NOT_RESET |
883 sample_point);
884 if (retval)
885 return retval;
886 }
887 usleep_range(100);
888
889 rtsx_init_cmd(chip);
890 rtsx_add_cmd(chip, WRITE_REG_CMD, SD_DCMPS_CTL, DCMPS_CHANGE,
891 DCMPS_CHANGE);
892 rtsx_add_cmd(chip, CHECK_REG_CMD, SD_DCMPS_CTL,
893 DCMPS_CHANGE_DONE, DCMPS_CHANGE_DONE);
894 retval = rtsx_send_cmd(chip, SD_CARD, 100);
895 if (retval != STATUS_SUCCESS)
896 goto fail;
897
898 val = *rtsx_get_cmd_data(chip);
899 if (val & DCMPS_ERROR)
900 goto fail;
901
902 if ((val & DCMPS_CURRENT_PHASE) != sample_point)
903 goto fail;
904
905 retval = rtsx_write_register(chip, SD_DCMPS_CTL,
906 DCMPS_CHANGE, 0);
907 if (retval)
908 return retval;
909 if (ddr_rx) {
910 retval = rtsx_write_register(chip, SD_VP_CTL,
911 PHASE_CHANGE, 0);
912 if (retval)
913 return retval;
914 } else {
915 retval = rtsx_write_register(chip, CLK_CTL,
916 CHANGE_CLK, 0);
917 if (retval)
918 return retval;
919 }
920
921 usleep_range(50);
922 }
923
924 retval = rtsx_write_register(chip, SD_CFG1, SD_ASYNC_FIFO_NOT_RST, 0);
925 if (retval)
926 return retval;
927
928 return STATUS_SUCCESS;
929
930 fail:
931 rtsx_read_register(chip, SD_VP_CTL, &val);
932 dev_dbg(rtsx_dev(chip), "SD_VP_CTL: 0x%x\n", val);
933 rtsx_read_register(chip, SD_DCMPS_CTL, &val);
934 dev_dbg(rtsx_dev(chip), "SD_DCMPS_CTL: 0x%x\n", val);
935
936 rtsx_write_register(chip, SD_DCMPS_CTL, DCMPS_CHANGE, 0);
937 rtsx_write_register(chip, SD_VP_CTL, PHASE_CHANGE, 0);
938 mdelay(10);
939 sd_reset_dcm(chip, tune_dir);
940 return STATUS_FAIL;
941 }
942
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH] staging: rts5208: Replace delay function.
2023-10-18 1:03 [PATCH] staging: rts5208: Replace delay function kenechukwu maduechesi
` (3 preceding siblings ...)
2023-10-23 23:09 ` kernel test robot
@ 2023-11-05 20:58 ` kernel test robot
4 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2023-11-05 20:58 UTC (permalink / raw)
To: kenechukwu maduechesi, outreachy, shreeya.patel23498
Cc: llvm, oe-kbuild-all, Greg Kroah-Hartman, linux-staging,
linux-kernel
Hi kenechukwu,
kernel test robot noticed the following build errors:
[auto build test ERROR on staging/staging-testing]
url: https://github.com/intel-lab-lkp/linux/commits/kenechukwu-maduechesi/staging-rts5208-Replace-delay-function/20231018-090457
base: staging/staging-testing
patch link: https://lore.kernel.org/r/20231018004300.GA3189%40ubuntu
patch subject: [PATCH] staging: rts5208: Replace delay function.
config: i386-allmodconfig (https://download.01.org/0day-ci/archive/20231106/202311060455.MQsyG6kq-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231106/202311060455.MQsyG6kq-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311060455.MQsyG6kq-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/staging/rts5208/sd.c:868:19: error: too few arguments to function call, expected 2, have 1
usleep_range(50);
~~~~~~~~~~~~ ^
include/linux/delay.h:66:20: note: 'usleep_range' declared here
static inline void usleep_range(unsigned long min, unsigned long max)
^
drivers/staging/rts5208/sd.c:880:19: error: too few arguments to function call, expected 2, have 1
usleep_range(50);
~~~~~~~~~~~~ ^
include/linux/delay.h:66:20: note: 'usleep_range' declared here
static inline void usleep_range(unsigned long min, unsigned long max)
^
drivers/staging/rts5208/sd.c:887:19: error: too few arguments to function call, expected 2, have 1
usleep_range(100);
~~~~~~~~~~~~ ^
include/linux/delay.h:66:20: note: 'usleep_range' declared here
static inline void usleep_range(unsigned long min, unsigned long max)
^
drivers/staging/rts5208/sd.c:921:18: error: too few arguments to function call, expected 2, have 1
usleep_range(50);
~~~~~~~~~~~~ ^
include/linux/delay.h:66:20: note: 'usleep_range' declared here
static inline void usleep_range(unsigned long min, unsigned long max)
^
drivers/staging/rts5208/sd.c:1419:19: error: too few arguments to function call, expected 2, have 1
usleep_range(100);
~~~~~~~~~~~~ ^
include/linux/delay.h:66:20: note: 'usleep_range' declared here
static inline void usleep_range(unsigned long min, unsigned long max)
^
5 errors generated.
vim +868 drivers/staging/rts5208/sd.c
814
815 static int sd_change_phase(struct rtsx_chip *chip, u8 sample_point, u8 tune_dir)
816 {
817 struct sd_info *sd_card = &chip->sd_card;
818 u16 SD_VP_CTL, SD_DCMPS_CTL;
819 u8 val;
820 int retval;
821 bool ddr_rx = false;
822
823 dev_dbg(rtsx_dev(chip), "%s (sample_point = %d, tune_dir = %d)\n",
824 __func__, sample_point, tune_dir);
825
826 if (tune_dir == TUNE_RX) {
827 SD_VP_CTL = SD_VPRX_CTL;
828 SD_DCMPS_CTL = SD_DCMPS_RX_CTL;
829 if (CHK_SD_DDR50(sd_card))
830 ddr_rx = true;
831 } else {
832 SD_VP_CTL = SD_VPTX_CTL;
833 SD_DCMPS_CTL = SD_DCMPS_TX_CTL;
834 }
835
836 if (chip->asic_code) {
837 retval = rtsx_write_register(chip, CLK_CTL, CHANGE_CLK,
838 CHANGE_CLK);
839 if (retval)
840 return retval;
841 retval = rtsx_write_register(chip, SD_VP_CTL, 0x1F,
842 sample_point);
843 if (retval)
844 return retval;
845 retval = rtsx_write_register(chip, SD_VPCLK0_CTL,
846 PHASE_NOT_RESET, 0);
847 if (retval)
848 return retval;
849 retval = rtsx_write_register(chip, SD_VPCLK0_CTL,
850 PHASE_NOT_RESET, PHASE_NOT_RESET);
851 if (retval)
852 return retval;
853 retval = rtsx_write_register(chip, CLK_CTL, CHANGE_CLK, 0);
854 if (retval)
855 return retval;
856 } else {
857 rtsx_read_register(chip, SD_VP_CTL, &val);
858 dev_dbg(rtsx_dev(chip), "SD_VP_CTL: 0x%x\n", val);
859 rtsx_read_register(chip, SD_DCMPS_CTL, &val);
860 dev_dbg(rtsx_dev(chip), "SD_DCMPS_CTL: 0x%x\n", val);
861
862 if (ddr_rx) {
863 retval = rtsx_write_register(chip, SD_VP_CTL,
864 PHASE_CHANGE,
865 PHASE_CHANGE);
866 if (retval)
867 return retval;
> 868 usleep_range(50);
869 retval = rtsx_write_register(chip, SD_VP_CTL, 0xFF,
870 PHASE_CHANGE |
871 PHASE_NOT_RESET |
872 sample_point);
873 if (retval)
874 return retval;
875 } else {
876 retval = rtsx_write_register(chip, CLK_CTL,
877 CHANGE_CLK, CHANGE_CLK);
878 if (retval)
879 return retval;
880 usleep_range(50);
881 retval = rtsx_write_register(chip, SD_VP_CTL, 0xFF,
882 PHASE_NOT_RESET |
883 sample_point);
884 if (retval)
885 return retval;
886 }
887 usleep_range(100);
888
889 rtsx_init_cmd(chip);
890 rtsx_add_cmd(chip, WRITE_REG_CMD, SD_DCMPS_CTL, DCMPS_CHANGE,
891 DCMPS_CHANGE);
892 rtsx_add_cmd(chip, CHECK_REG_CMD, SD_DCMPS_CTL,
893 DCMPS_CHANGE_DONE, DCMPS_CHANGE_DONE);
894 retval = rtsx_send_cmd(chip, SD_CARD, 100);
895 if (retval != STATUS_SUCCESS)
896 goto fail;
897
898 val = *rtsx_get_cmd_data(chip);
899 if (val & DCMPS_ERROR)
900 goto fail;
901
902 if ((val & DCMPS_CURRENT_PHASE) != sample_point)
903 goto fail;
904
905 retval = rtsx_write_register(chip, SD_DCMPS_CTL,
906 DCMPS_CHANGE, 0);
907 if (retval)
908 return retval;
909 if (ddr_rx) {
910 retval = rtsx_write_register(chip, SD_VP_CTL,
911 PHASE_CHANGE, 0);
912 if (retval)
913 return retval;
914 } else {
915 retval = rtsx_write_register(chip, CLK_CTL,
916 CHANGE_CLK, 0);
917 if (retval)
918 return retval;
919 }
920
921 usleep_range(50);
922 }
923
924 retval = rtsx_write_register(chip, SD_CFG1, SD_ASYNC_FIFO_NOT_RST, 0);
925 if (retval)
926 return retval;
927
928 return STATUS_SUCCESS;
929
930 fail:
931 rtsx_read_register(chip, SD_VP_CTL, &val);
932 dev_dbg(rtsx_dev(chip), "SD_VP_CTL: 0x%x\n", val);
933 rtsx_read_register(chip, SD_DCMPS_CTL, &val);
934 dev_dbg(rtsx_dev(chip), "SD_DCMPS_CTL: 0x%x\n", val);
935
936 rtsx_write_register(chip, SD_DCMPS_CTL, DCMPS_CHANGE, 0);
937 rtsx_write_register(chip, SD_VP_CTL, PHASE_CHANGE, 0);
938 mdelay(10);
939 sd_reset_dcm(chip, tune_dir);
940 return STATUS_FAIL;
941 }
942
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread