HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/12430 )
Change subject: northbridge/intel/pineview: Add remaining boilerplate code for northbridge ......................................................................
Patch Set 18:
(8 comments)
https://review.coreboot.org/#/c/12430/18/src/northbridge/intel/pineview/earl... File src/northbridge/intel/pineview/early_init.c:
https://review.coreboot.org/#/c/12430/18/src/northbridge/intel/pineview/earl... PS18, Line 66: reg32 = MCHBAR32(0x30); : MCHBAR32(0x30) = 0x21800; Do we need to read this register before writing on it ?
https://review.coreboot.org/#/c/12430/18/src/northbridge/intel/pineview/earl... PS18, Line 73: reg16 not used? do we need to read it?
https://review.coreboot.org/#/c/12430/18/src/northbridge/intel/pineview/earl... PS18, Line 75: reg16 not used. maybe we need to read this ?
https://review.coreboot.org/#/c/12430/18/src/northbridge/intel/pineview/earl... PS18, Line 77: reg16 not used
https://review.coreboot.org/#/c/12430/18/src/northbridge/intel/pineview/earl... PS18, Line 80: reg8 not used?
https://review.coreboot.org/#/c/12430/18/src/northbridge/intel/pineview/earl... PS18, Line 81: reg16 = MCHBAR16(0xc8c); : MCHBAR16(0xc8c) = reg16 | 0x0200; why don't we use MCHBAR16(0xc8c) |= 0x0200 ?
https://review.coreboot.org/#/c/12430/18/src/northbridge/intel/pineview/earl... PS18, Line 83: reg8 = MCHBAR8(0xc8c); : MCHBAR8(0xc8c) = reg8; : MCHBAR8(0xc8c) = 0x12; why please ?
https://review.coreboot.org/#/c/12430/18/src/northbridge/intel/pineview/earl... PS18, Line 98: eg32 = MCHBAR32(0x40); : MCHBAR32(0x40) = 0x0; : reg32 = MCHBAR32(0x40); : MCHBAR32(0x40) = 0x8; do we need to read before write some thing ?