Sathya Prakash M R has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31280 )
Change subject: mb/google/hatch: Enable Audio support ......................................................................
Patch Set 5:
(6 comments)
https://review.coreboot.org/#/c/31280/5//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31280/5//COMMIT_MSG@10 PS5, Line 10: 3.3V
Is this really 3.3V? Its pulled up to 1.8V in schematics.
will confirm. thanks for bringing this up
https://review.coreboot.org/#/c/31280/5/src/mainboard/google/hatch/Kconfig File src/mainboard/google/hatch/Kconfig:
https://review.coreboot.org/#/c/31280/5/src/mainboard/google/hatch/Kconfig@6 PS5, Line 6: DRIVERS_GENERIC_MAX98357A
Can you please place this alphabetically before DRIVERS_I2C_GENERIC?
Sure.
https://review.coreboot.org/#/c/31280/5/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/devicetree.cb:
https://review.coreboot.org/#/c/31280/5/src/mainboard/google/hatch/variants/... PS5, Line 247: register "probed" = "1"
Probed is not required here.
Yes. i've confirmed that. will update this along with other comments
https://review.coreboot.org/#/c/31280/5/src/mainboard/google/hatch/variants/... File src/mainboard/google/hatch/variants/baseboard/gpio.c:
https://review.coreboot.org/#/c/31280/5/src/mainboard/google/hatch/variants/... PS5, Line 72: /* PCH_I2C_AUDIO_SDA */ : PAD_CFG_NF(GPP_H8, NONE, DEEP, NF1), : /* PCH_I2C_AUDIO_SCL */ : PAD_CFG_NF(GPP_H9, NONE, DEEP, NF1),
Can you please order these and the pins below as per GPP_* groups?
Yes. its pending from previous patch set too. Will fix with other comments.
https://review.coreboot.org/#/c/31280/5/src/mainboard/google/hatch/variants/... PS5, Line 163: * AUDIO IRQ
(HP_INT#)
Sure. will correct.
https://review.coreboot.org/#/c/31280/5/src/mainboard/google/hatch/variants/... PS5, Line 168: SPEAKER SD MODE ENABLE
Please use net name as comment so that its easier to compare with schematics.
Will correct it as suggested.