Bug in warning message from MTRR rework in uvesafb

Bug in warning message from MTRR rework in uvesafb

[Torsten Kaiser]

Commit 63e28a7a5ffce59b645ca9cbcc01e1e8be56bd75, "uvesafb: Clean up
MTRR code" contains the following change:

@@ -1930,6 +1891,9 @@ static int uvesafb_setup(char *options)
         }
     }

+    if (mtrr != 3 && mtrr != 1)
+        pr_warn("uvesafb: mtrr should be set to 0 or 3; %d is
unsupported", mtrr);
+
     return 0;
 }
 #endif /* !MODULE */

Shouldn't this be && mtrr != 0?


HTH, Torsten

Re: Bug in warning message from MTRR rework in uvesafb

[Andy Lutomirski]

On Wed, Jul 10, 2013 at 10:07 AM, Torsten Kaiser
<just.for.lkml@googlemail.com> wrote:
> Commit 63e28a7a5ffce59b645ca9cbcc01e1e8be56bd75, "uvesafb: Clean up
> MTRR code" contains the following change:
>
> @@ -1930,6 +1891,9 @@ static int uvesafb_setup(char *options)
>          }
>      }
>
> +    if (mtrr != 3 && mtrr != 1)
> +        pr_warn("uvesafb: mtrr should be set to 0 or 3; %d is
> unsupported", mtrr);
> +
>      return 0;
>  }
>  #endif /* !MODULE */
>
> Shouldn't this be && mtrr != 0?

Indeed, and Sylvain Hitier (cc'd) sent a patch (off-list) that must
have gotten lost somewhere.

--Andy

Re: Bug in warning message from MTRR rework in uvesafb

[Dave Airlie]

On Thu, Jul 11, 2013 at 3:36 AM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Wed, Jul 10, 2013 at 10:07 AM, Torsten Kaiser
> <just.for.lkml@googlemail.com> wrote:
>> Commit 63e28a7a5ffce59b645ca9cbcc01e1e8be56bd75, "uvesafb: Clean up
>> MTRR code" contains the following change:
>>
>> @@ -1930,6 +1891,9 @@ static int uvesafb_setup(char *options)
>>          }
>>      }
>>
>> +    if (mtrr != 3 && mtrr != 1)
>> +        pr_warn("uvesafb: mtrr should be set to 0 or 3; %d is
>> unsupported", mtrr);
>> +
>>      return 0;
>>  }
>>  #endif /* !MODULE */
>>
>> Shouldn't this be && mtrr != 0?
>
> Indeed, and Sylvain Hitier (cc'd) sent a patch (off-list) that must
> have gotten lost somewhere.
>


Yeah I should get an email reply thing for off-list stuff, as its even
more likely I'll lose it.

Dave.

[PATCH] uvesafb: Really allow mtrr being 0, as documented and warn()ed

[Sylvain 'ythier' Hitier]

Hello,

When: 2013-07-10_3@10-36-22 -0700
Who:  Andy Lutomirski
What:
> On Wed, Jul 10, 2013 at 10:07 AM, Torsten Kaiser
> <just.for.lkml@googlemail.com> wrote:
> > Commit 63e28a7a5ffce59b645ca9cbcc01e1e8be56bd75, "uvesafb: Clean up
> > MTRR code" contains the following change:
> >
> > @@ -1930,6 +1891,9 @@ static int uvesafb_setup(char *options)
> >          }
> >      }
> >
> > +    if (mtrr != 3 && mtrr != 1)
> > +        pr_warn("uvesafb: mtrr should be set to 0 or 3; %d is
> > unsupported", mtrr);
> > +
> >      return 0;
> >  }
> >  #endif /* !MODULE */
> >
> > Shouldn't this be && mtrr != 0?
> 
> Indeed, and Sylvain Hitier (cc'd) sent a patch (off-list) that must
> have gotten lost somewhere.

RIP patch born on 2013-06-08_6@23-02-26 +0200 ...

May your soul be happy, because here is a clone of you who will hopefully
fulfill your quest:



Fixup for commit "uvesafb: Clean up MTRR code"
    (63e28a7a5ffce59b645ca9cbcc01e1e8be56bd75)

Signed-off-by: Sylvain "ythier" Hitier <sylvain.hitier@gmail.com>
Acked-by: Andy Lutomirski <luto@amacapital.net>
Also-spotted-by: Torsten Kaiser <just.for.lkml@googlemail.com>
---
 drivers/video/uvesafb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c
index 296279b..2121fc3 100644
--- a/drivers/video/uvesafb.c
+++ b/drivers/video/uvesafb.c
@@ -1891,7 +1891,7 @@ static int uvesafb_setup(char *options)
                }
        }
 
-       if (mtrr != 3 && mtrr != 1)
+       if (mtrr != 3 && mtrr != 0)
                pr_warn("uvesafb: mtrr should be set to 0 or 3; %d is unsupported", mtrr);
 
        return 0;
--



Regards,
Sylvain "ythier" Hitier

-- 
Business is about being busy, not being rich...
Lived 777 days in a Debian package => http://en.wikipedia.org/wiki/Apt,_Vaucluse
There's THE room for ideals in this mechanical place!

Re: Bug in warning message from MTRR rework in uvesafb

[Daniel Vetter]

On Wed, Jul 10, 2013 at 7:07 PM, Torsten Kaiser
<just.for.lkml@googlemail.com> wrote:
> Commit 63e28a7a5ffce59b645ca9cbcc01e1e8be56bd75, "uvesafb: Clean up
> MTRR code" contains the following change:
>
> @@ -1930,6 +1891,9 @@ static int uvesafb_setup(char *options)
>          }
>      }
>
> +    if (mtrr != 3 && mtrr != 1)
> +        pr_warn("uvesafb: mtrr should be set to 0 or 3; %d is
> unsupported", mtrr);
> +
>      return 0;
>  }
>  #endif /* !MODULE */
>
> Shouldn't this be && mtrr != 0?

Yes, you're right, I've failed to spot that while reviewing. Can you
please submit a proper kernel patch, see
Documentation/SubmittingPatches?
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch