Attention is currently required from: Máté Kukri (mkukri), Michael Niewöhner, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55250 )
Change subject: [WIP] src/superio/smsc: Add support for the SCH5555 ......................................................................
Patch Set 2:
(1 comment)
File src/superio/smsc/sch5555/sch5555.h:
https://review.coreboot.org/c/coreboot/+/55250/comment/4ae6b135_03b7dc77 PS1, Line 51: #define EMI_IOBASE 0xa00 : #define RUNTIME_IOBASE 0xa40
Well it could be, I don't think it matters to the mainboard code what address these are mapped to as […]
My main concern is that mainboard code needs to enable LPC forwarding on these addresses. I guess we don't need to make them configurable for now (it would boil down to creating some Kconfig options), but I'd look into providing a way for a mainboard to know these values.
For example, I would #define these values in `chip.h` along with their size, and use the macros to program the `genX_dec` values in the devicetree:
superio/smsc/sch5555/chip.h:
#define EMI_IOBASE 0xa00 #define EMI_IOSIZE 0x40 /* TODO: placeholder, confirm */ #define RUNTIME_IOBASE 0xa40 #define RUNTIME_IOSIZE 0x40 /* TODO: placeholder, confirm */
mainboard/foo/bar/devicetree.cb:
register "gen1_dec" = "(EMI_IOSIZE - 1) << 16 | EMI_IOBASE | 1" register "gen2_dec" = "(RUNTIME_IOSIZE - 1) << 16 | RUNTIME_IOBASE | 1"
If both I/O windows are contiguous, a single `genX_dec` register to cover both ranges would suffice.
The most straightforward way to determine the size of these I/O windows would be to derive them from vendor-programmed GENx_DEC values.
However, MMIO/PMIO windows almost always (I've yet to find an exception to this rule) need to be aligned to their size, and the BARs (base address registers) consequently have the lower address bits hardwired to 0. This allows software to check which bits "move", i.e. can be written to:
1. Write zeroes to the BAR. 2. Read back the BAR value. 3. Write ones to the BAR. 4. Read back the BAR value. 5. XOR the values from steps 2 and 4 to obtain a mask of the bits that move.
We have `pci_moving_configX` (X = 8, 16, 32) functions which do this with PCI registers. You could try to do this with the SCH5555 registers to double-check the values derived from GENx_DEC registers.