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

Uwe Hermann uwe at hermann-uwe.de
Wed May 2 02:35:29 CEST 2007


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?


> 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.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20070502/e0020029/attachment.sig>


More information about the coreboot mailing list