Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32356 )
Change subject: drivers/usb/ucsi: Add driver to generate UCSI memory region ......................................................................
Patch Set 1:
Patch Set 1:
(1 comment)
I'm struggling to see why you made the cut between generated and static ASL code where you made it. It all depends on the CBMEM address, right? Wouldn't it be enough to generate a named object, e.g.
Name (CBMU, 0x...)and use that in static code like
OperationRegion (UCSM, SystemMemory, CBMU, UCSI_SIZE)and something similar for _CRS, etc.
Yes that could work too.
What I really wanted was to put almost everything in the SSDT and just have the EC code provide the _DSM method, but DSDT ASL can't scope back into the SSDT so I couldn't declare the device here. Since I couldn't do that I decided to try and provide a framework and split the memory side into the a driver that could be reused if another EC implements UCSI in the future.
But it was just one of many possible paths. Making it into a library function that just allocates the cbmem region and adds a basic Name() for it is another, I will look at it and see how it turns out.