Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34337 )
Change subject: soc/intel/common: add advanced macros for gpio config ......................................................................
Patch Set 2:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34337/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34337/2//COMMIT_MSG@19 PS2, Line 19: trig
What is trig set to for a) GPO and b) NF. […]
Thanks for the review. No, sometimes I see: 0x0410: 0x00003c1a00000602 GPD2 LAN_WAKE# it is defined as PAD_CFG_ADV_NF(GPD2, NATIVE, PWROK, RX_DIS, LEVEL, NF1), /* LAN_WAKE# */
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... File src/soc/intel/common/block/gpio/Kconfig:
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... PS2, Line 49: SOC_INTEL_COMMON_BLOCK_GPIO_ADVANCED_MACROS
Why do we need a special Kconfig to guard the macro definitions? If a board doesn't use it, there wo […]
Ok. I will fix this in a new patch.
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/gpio_defs.h:
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... PS2, Line 421: trig
Is the trig always set to disable for NF by inteltool?
In only one case 0h = Level is used: 0x0410: 0x00003c1a00000602 GPD2 LAN_WAKE# or PAD_CFG_ADV_NF(GPD2, NATIVE, PWROK, RX_DIS, LEVEL, NF1), /* LAN_WAKE# */
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... PS2, Line 427: trig
The trigger matters only for the internal Rx pad state. […]
I don’t sure about trig for a GPO. In the documentation, I mean that "This field does not affect the received pad state (to GPIORXState or native functions)...", but from the inteltool dump for asrock-h110m-dvs:
0x0728: 0x0000104d84000201 GPP_G16 GPIO or PAD_CFG_ADV_GPO(GPP_G16, 1, 20K_PD, PLTRST, OFF), /* GPIO */
At the same time, trig is always set to 0 (level) in the standard macro PAD_CFG_GPO. I would prefer to use standard macros. May be I ignore inteltool information about trig and set trig to 0 (level) for all GPOs? and may be ignore trig for NF?
The board also works well in the case when I use standard macros for this.
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... PS2, Line 427: pull
Why is pull required on pad which is configured as GPO and being actively driven?
I repeated the inteltool dump:
0x0728: 0x0000104d84000201 GPP_G16 GPIO or PAD_CFG_ADV_GPO(GPP_G16, 1, 20K_PD, PLTRST, OFF), /* GPIO */