[LinuxBIOS] [patch 4/4] AMD Geode GX/LX: CS5536 Southbridge

[Marc Jones]

Uwe Hermann wrote:
> On Tue, May 01, 2007 at 04:48:31PM -0600, Marc Jones wrote:
>> Index: LinuxBIOSv2/src/southbridge/amd/cs5536/Config.lb
>> ===================================================================
>> --- LinuxBIOSv2.orig/src/southbridge/amd/cs5536/Config.lb	2007-04-30 15:14:24.000000000 -0600
>> +++ LinuxBIOSv2/src/southbridge/amd/cs5536/Config.lb	2007-04-30 15:14:24.000000000 -0600
>> @@ -1,4 +1,22 @@
>> +#/*
>> +#* This file is part of the LinuxBIOS project.
>> +#*
>> +#*
>> +#* This program is free software; you can redistribute it and/or modify
>> +#* it under the terms of the GNU General Public License version 2 as
>> +#* published by the Free Software Foundation.
>> +#*
>> +#* This program is distributed in the hope that it will be useful,
>> +#* but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +#* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +#* GNU General Public License for more details.
>> +#*
>> +#* You should have received a copy of the GNU General Public License
>> +#* along with this program; if not, write to the Free Software
>> +#* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
>> +#*/
> 
> Missing copyright holder.
> 
> 
>> +/* ***************************************************************************/
>> +/* **/
>> +/* *	ChipsetInit */
>> +/*			Called from northbridge init (Pre-VSA). */
>> +/* **/
>> +/* ***************************************************************************/
> 
> More cosmetics -- please use Doxygen-style comments if possible. We're
> trying to consistently use Doxygen comments in all of LinuxBIOS (at
> least in the next version, LinuxBIOSv3).
> 
> 
>>  struct chip_operations southbridge_amd_cs5536_ops = {
>> -	CHIP_NAME("AMD Geode CS5536 Southbridge")
>> -	/* This only called when this device is listed in the 
>> +	CHIP_NAME("AMD cs5536")
> 
> Why the name change?
> 
Oversite on my part. I missed that in the merge  to code that changed 
from when I started. - fixing.


> 
>> Index: LinuxBIOSv2/src/southbridge/amd/cs5536/cs5536_smbus2.h
>> ===================================================================
>> --- LinuxBIOSv2.orig/src/southbridge/amd/cs5536/cs5536_smbus2.h	2007-04-30 15:14:24.000000000 -0600
>> +++ LinuxBIOSv2/src/southbridge/amd/cs5536/cs5536_smbus2.h	2007-04-30 15:14:24.000000000 -0600
> [...]
>>  static void smbus_delay(void)
>>  {
>> -	outb(0x80, 0x80);
>> +	inb(0x80);
>>  }
> 
> Why? Please document this in the code (what, how, why). Did you perform
> measurements? Why did the outb() not work?
> 
> 
> Uwe.
> 

-- 
Marc Jones
Senior Software Engineer
(970) 226-9684 Office
mailto:Marc.Jones at amd.com
http://www.amd.com/embeddedprocessors