[patch 2/2] pata_amd: fix sparse warning

[patch 2/2] pata_amd: fix sparse warning

[akpm]

From: Harvey Harrison <harvey.harrison@gmail.com>

Current code is essentially choosing between dividing by 1 or
dividing by two, make the conditions a little more obvious.

As a bonus, removes a sparse error:
drivers/ata/pata_amd.c:59:11: warning: symbol '__x' shadows an earlier one
drivers/ata/pata_amd.c:59:11: originally declared here

Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/ata/pata_amd.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff -puN drivers/ata/pata_amd.c~pata_amd-fix-sparse-warning drivers/ata/pata_amd.c
--- a/drivers/ata/pata_amd.c~pata_amd-fix-sparse-warning
+++ a/drivers/ata/pata_amd.c
@@ -56,7 +56,9 @@ static void timing_setup(struct ata_port
 	u8 t;
 
 	T = 1000000000 / amd_clock;
-	UT = T / min_t(int, max_t(int, clock, 1), 2);
+	UT = T;
+	if (clock >= 2)
+		UT = T / 2;
 
 	if (ata_timing_compute(adev, speed, &at, T, UT) < 0) {
 		dev_printk(KERN_ERR, &pdev->dev, "unknown mode %d.\n", speed);
_

Re: [patch 2/2] pata_amd: fix sparse warning

[Jeff Garzik]

akpm@linux-foundation.org wrote:
> From: Harvey Harrison <harvey.harrison@gmail.com>
> 
> Current code is essentially choosing between dividing by 1 or
> dividing by two, make the conditions a little more obvious.
> 
> As a bonus, removes a sparse error:
> drivers/ata/pata_amd.c:59:11: warning: symbol '__x' shadows an earlier one
> drivers/ata/pata_amd.c:59:11: originally declared here
> 
> Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  drivers/ata/pata_amd.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff -puN drivers/ata/pata_amd.c~pata_amd-fix-sparse-warning drivers/ata/pata_amd.c
> --- a/drivers/ata/pata_amd.c~pata_amd-fix-sparse-warning
> +++ a/drivers/ata/pata_amd.c
> @@ -56,7 +56,9 @@ static void timing_setup(struct ata_port
>  	u8 t;
>  
>  	T = 1000000000 / amd_clock;
> -	UT = T / min_t(int, max_t(int, clock, 1), 2);
> +	UT = T;
> +	if (clock >= 2)
> +		UT = T / 2;

Why assign UT twice, potentially?

Re: [patch 2/2] pata_amd: fix sparse warning

[Grant Grundler]

On Sat, Mar 29, 2008 at 9:17 AM, Jeff Garzik <jeff@garzik.org> wrote:
...
>  > +     UT = T;
>  > +     if (clock >= 2)
>  > +             UT = T / 2;
>
>  Why assign UT twice, potentially?

This construct used in many places in the kernel because it
makes the code more readable.

"UT" is a regular "int" type and assigning it twice is ok.

e.g. from tg3_writephy():
...
        ret = -EBUSY;
        if (loops != 0)
                ret = 0;

hth,
grant

Re: [patch 2/2] pata_amd: fix sparse warning

[Jeff Garzik]

akpm@linux-foundation.org wrote:
> From: Harvey Harrison <harvey.harrison@gmail.com>
> 
> Current code is essentially choosing between dividing by 1 or
> dividing by two, make the conditions a little more obvious.
> 
> As a bonus, removes a sparse error:
> drivers/ata/pata_amd.c:59:11: warning: symbol '__x' shadows an earlier one
> drivers/ata/pata_amd.c:59:11: originally declared here
> 
> Signed-off-by: Harvey Harrison <harvey.harrison@gmail.com>
> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
>  drivers/ata/pata_amd.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff -puN drivers/ata/pata_amd.c~pata_amd-fix-sparse-warning drivers/ata/pata_amd.c
> --- a/drivers/ata/pata_amd.c~pata_amd-fix-sparse-warning
> +++ a/drivers/ata/pata_amd.c
> @@ -56,7 +56,9 @@ static void timing_setup(struct ata_port
>  	u8 t;
>  
>  	T = 1000000000 / amd_clock;
> -	UT = T / min_t(int, max_t(int, clock, 1), 2);
> +	UT = T;
> +	if (clock >= 2)
> +		UT = T / 2;
>  
>  	if (ata_timing_compute(adev, speed, &at, T, UT) < 0) {

applied