Attention is currently required from: Kangheui Won, Tim Wawrzynczak, Rizwan Qureshi, Meera Ravindranath. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/62856 )
Change subject: soc/intel/alderlake: Add support for UFS controller ......................................................................
Patch Set 1:
(1 comment)
File src/soc/intel/alderlake/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/62856/comment/281bc1e6_a2a08627 PS1, Line 619: s_cfg->UfsEnable[1] = 0;
As I commented on CB:62662 I don't understand whyt this is array and why we're using index 1 here.
Comments on FspsUpd.h seems wrong not helping anything.
https://review.coreboot.org/c/coreboot/+/62914/1..2/src/soc/intel/alderlake/...
It's an adjustment made in FSP to compliment many platform IMO.
I don't think we need to disable index 0 and enable 1 (unless you know the exact assumption inside FSP). (I guess you have carried away with FSP default UPD value assignment which enforces index 0 to set disable. is it ?) Also in EDS PMC offset 0x1E44 bit 8-7 marked as reserved hence, unable to understand what is the exact check that FSP does.
-
Subrata Banik (Code Review)