Re: Atari mach64 support (fwd)

Random code review (see below).

On Sep 22, 2005, at 7:15 AM, Geert Uytterhoeven wrote:

Should "if (pll_ref_dev) {" be "if (pll_ref_div) {" to keep the logic  
the same as the old code, or was this a bug fix?

Dave



-------------------------------------------------------
This SF.Net email is sponsored by:
Power Architecture Resource Center: Free content, downloads, discussions,
and more. http://solutions.newsforge.com/ibmarch.tmpl

Just code merger. First you had the M64_HAS test then test for 
pll_ref_div. I combined the two test into one test.
Its not needed tho so I broke it into 2 again. I think it is time to send 
this to Andrew.

Random code review (see below).

On Sep 22, 2005, at 7:15 AM, Geert Uytterhoeven wrote:

quoted

---------- Forwarded message ----------
Date: Wed, 21 Sep 2005 22:07:01 +0100 (BST)
From: James Simmons <redacted>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Linux Fbdev development list <linux-fbdev- 
devel@lists.sourceforge.net>
Subject: Atari mach64 support


It's back. Please give this a try. It does compile but I can't test  
it.

diff -urN -X /home/jsimmons/dontdiff linus-2.6/drivers/video/aty/ 
atyfb_base.c fbdev-2.6/drivers/video/aty/atyfb_base.c

--- linus-2.6/drivers/video/aty/atyfb_base.c    2005-09-13  

14:01:40.000000000 -0700

+++ fbdev-2.6/drivers/video/aty/atyfb_base.c    2005-09-21  

13:54:02.000000000 -0700
[...]

@@ -2276,18 +2268,20 @@
             par->pll_limits.mclk = 63;
     }

-    if (M64_HAS(GTB_DSP)
-        && (pll_ref_div = aty_ld_pll_ct(PLL_REF_DIV, par))) {
-        int diff1, diff2;
-        diff1 = 510 * 14 / pll_ref_div - par->pll_limits.pll_max;
-        diff2 = 510 * 29 / pll_ref_div - par->pll_limits.pll_max;
-        if (diff1 < 0)
-            diff1 = -diff1;
-        if (diff2 < 0)
-            diff2 = -diff2;
-        if (diff2 < diff1) {
-            par->ref_clk_per = 1000000000000ULL / 29498928;
-            xtal = "29.498928";
+    if (M64_HAS(GTB_DSP)) {
+        u8 pll_ref_div = aty_ld_pll_ct(PLL_REF_DIV, par);
+        if (pll_ref_dev) {
+            int diff1, diff2;
+            diff1 = 510 * 14 / pll_ref_div - par->pll_limits.pll_max;
+            diff2 = 510 * 29 / pll_ref_div - par->pll_limits.pll_max;
+            if (diff1 < 0)
+                diff1 = -diff1;
+            if (diff2 < 0)
+                diff2 = -diff2;
+            if (diff2 < diff1) {
+                par->ref_clk_per = 1000000000000ULL / 29498928;
+                xtal = "29.498928";
+            }
         }
     }
 #endif /* CONFIG_FB_ATY_CT */

Should "if (pll_ref_dev) {" be "if (pll_ref_div) {" to keep the logic  
the same as the old code, or was this a bug fix?

-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc.
Get Certified Today * Register for a JBoss Training Course
Free Certification Exam for All Training Attendees Through End of 2005
Visit http://www.jboss.com/services/certification for more information

On Mon, Oct 24, 2005 at 09:42:42PM +0100, James Simmons wrote:

Just code merger. First you had the M64_HAS test then test for 
pll_ref_div. I combined the two test into one test.
Its not needed tho so I broke it into 2 again. I think it is time to send 
this to Andrew.

He probably meant that you've mistyped pll_ref_div as pll_ref_dev.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/


-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc.
Get Certified Today * Register for a JBoss Training Course
Free Certification Exam for All Training Attendees Through End of 2005
Visit http://www.jboss.com/services/certification for more information

Yeap. Your right. I will send a patch with the correct spelling.


On Tue, 25 Oct 2005, Ville [iso-8859-1] Syrjälä wrote:

On Mon, Oct 24, 2005 at 09:42:42PM +0100, James Simmons wrote:

quoted

Just code merger. First you had the M64_HAS test then test for 
pll_ref_div. I combined the two test into one test.
Its not needed tho so I broke it into 2 again. I think it is time to send 
this to Andrew.

He probably meant that you've mistyped pll_ref_div as pll_ref_dev.

-- 
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/


-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc.
Get Certified Today * Register for a JBoss Training Course
Free Certification Exam for All Training Attendees Through End of 2005
Visit http://www.jboss.com/services/certification for more information
_______________________________________________
Linux-fbdev-devel mailing list
Linux-fbdev-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-fbdev-devel

-------------------------------------------------------
This SF.Net email is sponsored by the JBoss Inc.
Get Certified Today * Register for a JBoss Training Course
Free Certification Exam for All Training Attendees Through End of 2005
Visit http://www.jboss.com/services/certification for more information