[LinuxBIOS] [patch 4/4] AMD Geode GX/LX: CS5536 Southbridge
Marc Jones
marc.jones at amd.com
Wed May 2 21:44:24 CEST 2007
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
More information about the coreboot
mailing list