Prudhvi Yarlagadda has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32546 )
Change subject: qcom: Add i2c driver ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/#/c/32546/5/src/soc/qualcomm/qcs405/blsp.c File src/soc/qualcomm/qcs405/blsp.c:
https://review.coreboot.org/#/c/32546/5/src/soc/qualcomm/qcs405/blsp.c@45 PS5, Line 45: clock_configure_i2c(19200000);
I'd rather remove that value altogether, given how this function can only configure that one block.
I understand this wrt the fix use case of one i2c instance. Are you confirming that there won't be any further change wrt usecase ? In that case we can remove this block and let us know where that change will happen ? Clock driver ?
https://review.coreboot.org/#/c/32546/5/src/soc/qualcomm/qcs405/blsp.c@56 PS5, Line 56: 1
That seems to make more sense to me, yes. […]
Ok we will add the check for id at the beginning of the API. "id" is needed for porting of this driver to future chips where number of i2c use cases can be more than one.