10 comments:
File src/northbridge/intel/sandybridge/chip.h:
When I see bit fields that are not part of an ABI or hardware
interface, I always wonder if it doesn't increase code size
more than what is saved by packing them. What do we gain here?
Patch Set #7, Line 58: u8 spd_addresses[4];
These are not MRC specific, we could use the values for all
DIMM configs.
Patch Set #7, Line 68: u16 pcie_init : 2;
Why 2 bits?
Can we derive it from PEG device on/off?
Patch Set #7, Line 69: /* N mode functionality. Leave this setting at 0.
Doesn't sound like we'd ever need it in a devicetree.
Patch Set #7, Line 72: * 2 2N
I prefer enums over such comments. Makes the devicetrees more
readable without copying the comments all over. Doesn't even
need a separate declaration:
enum {
NMODE_AUTO,
NMODE_1N,
NMODE_2N,
} nmode;
Patch Set #7, Line 91: u16 dimm_channel1_disabled : 2;
Can we derive this from `spd_addresses`?
File src/northbridge/intel/sandybridge/raminit_mrc.c:
Patch Set #7, Line 294: const struct device *dev = pcidev_path_on_root(PCI_DEVFN(0, 0x19));
that would be `pcidev_on_root(0, 0x19)`
Patch Set #7, Line 307: = NULL
initialization is overwritten
Patch Set #7, Line 310: if (!dev)
check `!dev->chip_info` too?
Patch Set #7, Line 329: pei_data->max_ddr3_freq = 0;
Fall back to 800, with a warning? 0 doesn't seem valid.
To view, visit change 32069. To unsubscribe, or for help writing mail filters, visit settings.