Attention is currently required from: Furquan Shaikh, Neill Corlett. Joe Tessler has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50101 )
Change subject: hatch: Modify genesis PcieClkSrc settings ......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/50101/comment/a829c4b3_e879169c PS2, Line 9: to work around issue with Longsys SSD
My understanding of hybrid storage devices is that there are two devices seen on two separate PCIe RPs, but they share the same CLKSRC. Thus, while configuring these RPs in devicetree, you need to set both as enabled, but configure CLKSRC for only one of those.
That matches my expectations as well, hence my surprise by this patch from Quanta.
After chatting again with them, I confirmed this new SSD module is a raw PCIe drive (not SATA). Apparently the original settings only supported SATA drives.
I don't think that is accurate. SATA does not really require the CLKSRC/CLKREQ programming.
TBH I didn't even know raw PCIe SSDs were a thing 😕. I'll work in Quanta once I get my hands on this SSD module; this is all new to me.
I don't think there is clear understanding of the problem or how the change is really helping. I would recommend ensuring the design is understood and then translating that into appropriate device tree configs.
100% agree. I've decided to leave this review open until we better understand the situation. Quanta agreed to simply not use this SSD module for now.
-
Joe Tessler (Code Review)