Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37822 )
Change subject: mainboard/google/puff: Don't enable MAX98357A in dt ......................................................................
Patch Set 7:
(1 comment)
Patch Set 7:
(1 comment)
Patch Set 7:
(1 comment)
CB:37909 undoes what is done in this CL. Can we just drop this?
This fixes an immediate issue while the follow up is the proposed 'proper fix' which is more invasive. As I explained to Dan already, here we fix the issue while in the follow up we propose the more long term maintainable strategy on detailing with the current assumptions in baseboard.
Understood. But, both changes are going to effectively land at the same time and since you already have a solution in CB:37909 which is right long term fix, having this separate short-lived change seems unnecessary.
https://review.coreboot.org/c/coreboot/+/37822/7/src/mainboard/google/hatch/... File src/mainboard/google/hatch/Kconfig:
https://review.coreboot.org/c/coreboot/+/37822/7/src/mainboard/google/hatch/... PS7, Line 5: if !BOARD_GOOGLE_PUFF
That's incorrect, and if you see the history of Dan asking me to change the commit message from before may give insight.
I was referring to your entire patch series. I agree that the devicetree chip node needs to be removed from baseboard/devicetree.cb. But once that is done, there is no need to get rid of this driver selection from this Kconfig file. For puff variant, this driver would go unused and hence won't cause any side-effects for Puff.
To summarize, I think the changes that are really required are: 1. Get rid of max98537a device node from baseboard/devicetree.cb 2. Add max98537a device node to each variant overridetree.cb except Puff
Rest can still remain the same.