Lane Brooks wrote: > Here it is again with the patch actually attached. > > Lane Brooks wrote: >> Here is the patch again with an updated comment regarding SMP and a >> signed-off-by line. >> >> Signed-off-by: Lane Brooks <[EMAIL PROTECTED]> >> >> ron minnich wrote: >>> On 10/2/07, Lane Brooks <[EMAIL PROTECTED]> wrote: >>>> Attached is a patch that enables AMD Geode CS5536 chipset support. I >>>> have tested it successfully on a MSM800 board from digital logic. It >>>> does, however, have a few issues that I would like some feedback on. >>>> >>>> In my discussions with Marc Jones, Geode systems write protect the BIOS >>>> via RCONFs (cache settings similar to MTRRs). Unlocking requires >>>> changing MSR 0x1808 top byte to 0x22. Reading and writing to the msr, >>>> however, requires instrucitons rdmsr/wrmsr, which are ring0 privileged >>>> instructions so only the kernel can do the read/write. So my patch >>>> uses >>>> the msr kernel module to access these instructions from user space >>>> using >>>> the /dev/cpu/0/msr device. >>>> >>>> My questions are: >>>> - I do not think this is portable beyond linux. Is that an issue? >>> it's not, but it is not really an issue at present. >>>> - My code assumes the msr kernel module is already loaded. Is there a >>>> way to force a kernel module to load from the C code? My code does die >>>> gracefully with a message reminding them to load the kernel module if >>>> things fail. >>> it is possible I think to have udev help with this, but I am not sure. >>> Graceful failure is certainly a good start. >>> >>>> - It seems like there should be a way to revert the msr back after >>>> flashing is completed to put the bios back in write protect mode. Is >>>> there a cleanup mechanism available? Something like disable_flash... >>> I thought that was in the structure of flashrom. Now that I look, it >>> seems like we lost it! >>> I propose this at the end of flashrom: >>> board_flash_disable(lb_vendor, lb_part); >>> chipset_flash_disable(chipset); >>> >>> but we'll need to change some things to make this all work. We need a >>> penable struct * to use for the disable; no point in searching each >>> time we touch a chip. or not? >>> >>> one comment on your patch: you use /dev/cpu/0/msr. This is great for a >>> single-cpu machine, and will always be fine on geode. Lots of times, >>> people use one piece of flashrom to write another. Imagine some future >>> hacker, in a year or two, who has copied your code for some SMP >>> system, not understanding why sometimes flashrom fails (i.e. they set >>> CPU0 msr but end up running on cpu1). We had this very problem when we >>> were getting graphics going (and, earlier, SMP going). I suggest a >>> comment as to why the /dev/cpu/0/msr is always safe on geode, but >>> future hackers beware on SMP. Or something like that. >>> >>> That's up to you, however :-) >>> >>> If you had a signed-off-by line, I would ack and commit for you :-) >>> >>> ron >>> >>> ron >>
Did anyone ack and commit this last version? Marc -- Marc Jones Senior Firmware Engineer (970) 226-9684 Office mailto:[EMAIL PROTECTED] http://www.amd.com/embeddedprocessors -- linuxbios mailing list [email protected] http://www.linuxbios.org/mailman/listinfo/linuxbios
