Maxim Polyakov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34337 )
Change subject: soc/intel/common: add advanced macros for gpio config ......................................................................
soc/intel/common: add advanced macros for gpio config
In the case there is no the circuit diagram for motherboard, the PCH/SoC GPIOs config is based on information from the inteltool dump. However, available macros from gpio_defs.h can't define the pad configuration from this dump. This patch resolve this problem by adding advanced macros:
PAD_CFG_ADV_NF(pad, pull, rst, bufdis, trig, func), PAD_CFG_ADV_GPO(pad, val, pull, trig, rst)
Unlike PAD_CFG_NF and PAD_CFG_GPO, these new macros allow to set: trig - RX Level/Edge Configuration, RXEVCFG field in DW0 reg[1] bufdis - GPIORXDIS/GPIOTXDIS fields in DW0 to disable the input/ output buffer[1] of pad (PAD_CFG_ADV_NF only)
These changes were tested on Asrock H110M-DVS motherboard[2]. It also resolves the problem of automatically creating pads configuration[3]
[1] page 1429,Intel (R) 100 Series and Intel (R) C230 Series PCH Family Platform Controller Hub (PCH), Datasheet, Vol 2 of 2, February 2019, Document Number: 332691-003EN https://www.intel.com/content/dam/www/public/us/en/documents/ datasheets/100-series-chipset-datasheet-vol-2.pdf [2] https://review.coreboot.org/c/coreboot/+/33565 [3] https://github.com/maxpoliak/pch-pads-parser/issues/1
Change-Id: If9fe50ff9a680633db6228564345200c0e1ee3ea Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/common/block/gpio/Kconfig M src/soc/intel/common/block/include/intelblocks/gpio_defs.h 2 files changed, 30 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34337/1
diff --git a/src/soc/intel/common/block/gpio/Kconfig b/src/soc/intel/common/block/gpio/Kconfig index bdbc323..ff9781b 100644 --- a/src/soc/intel/common/block/gpio/Kconfig +++ b/src/soc/intel/common/block/gpio/Kconfig @@ -43,3 +43,11 @@ depends on SOC_INTEL_COMMON_BLOCK_GPIO bool default n + +# Indicate if SoC supports advanced macros of GPIOs. Unlike the standard ones, +# these macros have an increased number of parameters for configuring the pad. +config SOC_INTEL_COMMON_BLOCK_GPIO_ADVANCED_MACROS + depends on SOC_INTEL_COMMON_BLOCK_GPIO + bool + default n + diff --git a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h index e1ddd4b..8b3daf6 100644 --- a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h +++ b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h @@ -409,4 +409,26 @@
#endif /* CONFIG_SOC_INTEL_COMMON_BLOCK_GPIO_DUAL_ROUTE_SUPPORT */
+#if CONFIG(SOC_INTEL_COMMON_BLOCK_GPIO_ADVANCED_MACROS) + +/* Disable the input or output buffer of the pad */ +#define PAD_CFG0_BUF_RX_DIS PAD_CFG0_RX_DISABLE +#define PAD_CFG0_BUF_TX_DIS PAD_CFG0_TX_DISABLE +#define PAD_CFG0_BUF_BOTH_DIS PAD_CFG0_TX_DISABLE | PAD_CFG0_RX_DISABLE +#define PAD_CFG0_BUF_EN 0 + +/* Advanced function configuration */ +#define PAD_CFG_ADV_NF(pad, pull, rst, bufdis, trig, func) \ + _PAD_CFG_STRUCT(pad, PAD_RESET(rst) | PAD_CFG0_TRIG_##trig | \ + PAD_CFG0_BUF_##bufdis | PAD_FUNC(func), \ + PAD_PULL(pull) | PAD_IOSSTATE(TxLASTRxE)) + +/* Advanced settings for general purpose output */ +#define PAD_CFG_ADV_GPO(pad, val, pull, rst, trig) \ + _PAD_CFG_STRUCT(pad, \ + PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_CFG0_TRIG_##trig | \ + PAD_CFG0_RX_DISABLE | !!val, \ + PAD_PULL(pull) | PAD_IOSSTATE(TxLASTRxE)) +#endif /* CONFIG_SOC_INTEL_COMMON_BLOCK_GPIO_ADVANCED_MACROS */ + #endif /* _SOC_BLOCK_GPIO_DEFS_H_ */
build bot (Jenkins) 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 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34337/1/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/gpio_defs.h:
https://review.coreboot.org/c/coreboot/+/34337/1/src/soc/intel/common/block/... PS1, Line 417: #define PAD_CFG0_BUF_BOTH_DIS PAD_CFG0_TX_DISABLE | PAD_CFG0_RX_DISABLE Macros with complex values should be enclosed in parentheses
Hello Patrick Rudolph, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34337
to look at the new patch set (#2).
Change subject: soc/intel/common: add advanced macros for gpio config ......................................................................
soc/intel/common: add advanced macros for gpio config
In the case there is no the circuit diagram for motherboard, the PCH/SoC GPIOs config is based on information from the inteltool dump. However, available macros from gpio_defs.h can't define the pad configuration from this dump. This patch resolve this problem by adding advanced macros:
PAD_CFG_ADV_NF(pad, pull, rst, bufdis, trig, func), PAD_CFG_ADV_GPO(pad, val, pull, trig, rst)
Unlike PAD_CFG_NF and PAD_CFG_GPO, these new macros allow to set: trig - RX Level/Edge Configuration, RXEVCFG field in DW0 reg[1] bufdis - GPIORXDIS/GPIOTXDIS fields in DW0 to disable the input/ output buffer[1] of pad (PAD_CFG_ADV_NF only)
These changes were tested on Asrock H110M-DVS motherboard[2]. It also resolves the problem of automatically creating pads configuration[3]
[1] page 1429,Intel (R) 100 Series and Intel (R) C230 Series PCH Family Platform Controller Hub (PCH), Datasheet, Vol 2 of 2, February 2019, Document Number: 332691-003EN https://www.intel.com/content/dam/www/public/us/en/documents/ datasheets/100-series-chipset-datasheet-vol-2.pdf [2] https://review.coreboot.org/c/coreboot/+/33565 [3] https://github.com/maxpoliak/pch-pads-parser/issues/1
Change-Id: If9fe50ff9a680633db6228564345200c0e1ee3ea Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com --- M src/soc/intel/common/block/gpio/Kconfig M src/soc/intel/common/block/include/intelblocks/gpio_defs.h 2 files changed, 29 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/34337/2
Furquan Shaikh 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.
I am wondering if it is always just set to disable?
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 would be no harm.
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?
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?
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. Is this required to be configured by mainboard? Or can this be just set to DISABLE always?
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 */
Furquan Shaikh 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:
(6 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
Thanks for the review. […]
Ah okay. That makes sense for NF. What about GPO? Is a trig type set for GPO as well anytime?
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 136: You can move your macros up here:
#define PAD_CFG0_BUF_TX_DISABLE PAD_CFG0_TX_DISABLE #define PAD_CFG0_BUF_RX_DISABLE PAD_CFG0_RX_DISABLE #define PAD_CFG0_BUF_TX_RX_DISABLE (PAD_CFG0_TX_DISABLE | PAD_CFG0_RX_DISABLE) #define PAD_CFG0_BUF_NO_DISABLE (0)
#define PAD_BUF(value) PAD_CFG0_BUF_##value
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... PS2, Line 182: And then add your NF macro here:
#define PAD_CFG_NF_BUF_TRIG(pad, pull, rst, bufdis, trig, func)
I know its kind of a bigger name, but just trying to make its intent clear.
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... PS2, Line 421: trig
In only one case 0h = Level is used: […]
I think it makes sense to have a macro for this case.
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... PS2, Line 427: trig
I don’t sure about trig for a GPO. […]
In my opinion, we should just update PAD_CFG_GPO to set trig to DISABLE. Since Rx Trig should not matter for a GPO.
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... PS2, Line 427: pull
I repeated the inteltool dump: […]
I honestly think that this is incorrect. If the pad is being actively driven why is an internal 20K_PD required?
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34337 )
Change subject: soc/intel/common: add PAD_CFG_NF_BUF_TRIG macro ......................................................................
Patch Set 4:
(1 comment)
This change is ready for review.
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
Ah okay. That makes sense for NF. […]
Yes, trig type set to 2h = Drive '0' for all GPO.
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34337 )
Change subject: soc/intel/common: add PAD_CFG_NF_BUF_TRIG macro ......................................................................
Patch Set 4:
(3 comments)
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 136:
You can move your macros up here: […]
Thanks, done
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... PS2, Line 182:
And then add your NF macro here: […]
Done
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... PS2, Line 427: pull
I honestly think that this is incorrect. […]
I think you are right, but at the same time for some boards I see
PAD_CFG_TERM_GPO(GPP_B14, 1, 20K_PD, DEEP),
https://github.com/coreboot/coreboot/blob/master/src/mainboard/intel/kblrvp/...
This is a bit confusing
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34337 )
Change subject: soc/intel/common: add PAD_CFG_NF_BUF_TRIG macro ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
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 427: pull
I think you are right, but at the same time for some boards I see […]
That is definitely weird. Not sure what the use that really achieves.
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34337 )
Change subject: soc/intel/common: add PAD_CFG_NF_BUF_TRIG macro ......................................................................
Patch Set 4:
Patch Set 3:
I personally find the GPIO macros very cryptic, and would prefer something more like sandybridge and ivybridge's gpio.c if possible.
Did you mean to use separate structures instead of macros? As it is done in https://github.com/coreboot/coreboot/blob/master/src/mainboard/asrock/b75pro...
Honestly, macros look better to me ) But that's my personal opinion
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34337 )
Change subject: soc/intel/common: add PAD_CFG_NF_BUF_TRIG macro ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 3:
I personally find the GPIO macros very cryptic, and would prefer something more like sandybridge and ivybridge's gpio.c if possible.
Did you mean to use separate structures instead of macros? As it is done in https://github.com/coreboot/coreboot/blob/master/src/mainboard/asrock/b75pro...
Honestly, macros look better to me ) But that's my personal opinion
One of the advantages of using structures like the one you linked to is that you have to assign values to named members. If you forget one of the elements in an array, the rest of the values would get shifted downwards. And that can be potentially fatal to the GPIO pins: if a pin used as input gets configured as an output, short-circuits can happen!
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34337 )
Change subject: soc/intel/common: add PAD_CFG_NF_BUF_TRIG macro ......................................................................
Patch Set 4:
(5 comments)
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
Ok. I will fix this in a new patch.
Ack
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
I think it makes sense to have a macro for this case.
Ack
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... PS2, Line 427: trig
In my opinion, we should just update PAD_CFG_GPO to set trig to DISABLE. […]
Done in https://review.coreboot.org/c/coreboot/+/34406
https://review.coreboot.org/c/coreboot/+/34337/2/src/soc/intel/common/block/... PS2, Line 427: pull
That is definitely weird. Not sure what the use that really achieves.
Ok, I'll take this into account in the pads config for my motherboard Thanks
https://review.coreboot.org/c/coreboot/+/34337/1/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/gpio_defs.h:
https://review.coreboot.org/c/coreboot/+/34337/1/src/soc/intel/common/block/... PS1, Line 417: #define PAD_CFG0_BUF_BOTH_DIS PAD_CFG0_TX_DISABLE | PAD_CFG0_RX_DISABLE
Macros with complex values should be enclosed in parentheses
Ack
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34337 )
Change subject: soc/intel/common: add PAD_CFG_NF_BUF_TRIG macro ......................................................................
Patch Set 4:
(1 comment)
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
Yes, trig type set to 2h = Drive '0' for all GPO.
Ack
Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34337 )
Change subject: soc/intel/common: add PAD_CFG_NF_BUF_TRIG macro ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 3:
I personally find the GPIO macros very cryptic, and would prefer something more like sandybridge and ivybridge's gpio.c if possible.
Did you mean to use separate structures instead of macros? As it is done in https://github.com/coreboot/coreboot/blob/master/src/mainboard/asrock/b75pro...
Honestly, macros look better to me ) But that's my personal opinion
One of the advantages of using structures like the one you linked to is that you have to assign values to named members. If you forget one of the elements in an array, the rest of the values would get shifted downwards.
https://github.com/coreboot/coreboot/blob/master/src/soc/intel/common/block/... In the case of macros, I get the gpio structure from the pad value.
If I have not configured a pad, then it will be defined as NC. GPIO logic is disconnected from the pad output. For example coffeelake_rvp: https://github.com/coreboot/coreboot/blob/master/src/mainboard/intel/coffeel... Please correct me if I'm mistaken.
And that can be potentially fatal to the GPIO pins: if a pin used as input gets configured as an output, short-circuits can happen!
But, if I miss the .gpio16 = GPIO_DIR_INPUT element, it will be configured as an output. Any mistake in both cases will be fatal.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34337 )
Change subject: soc/intel/common: add PAD_CFG_NF_BUF_TRIG macro ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 4:
Patch Set 3:
I personally find the GPIO macros very cryptic, and would prefer something more like sandybridge and ivybridge's gpio.c if possible.
Did you mean to use separate structures instead of macros? As it is done in https://github.com/coreboot/coreboot/blob/master/src/mainboard/asrock/b75pro...
Honestly, macros look better to me ) But that's my personal opinion
One of the advantages of using structures like the one you linked to is that you have to assign values to named members. If you forget one of the elements in an array, the rest of the values would get shifted downwards.
https://github.com/coreboot/coreboot/blob/master/src/soc/intel/common/block/... In the case of macros, I get the gpio structure from the pad value.
If I have not configured a pad, then it will be defined as NC. GPIO logic is disconnected from the pad output. For example coffeelake_rvp: https://github.com/coreboot/coreboot/blob/master/src/mainboard/intel/coffeel... Please correct me if I'm mistaken.
And that can be potentially fatal to the GPIO pins: if a pin used as input gets configured as an output, short-circuits can happen!
But, if I miss the .gpio16 = GPIO_DIR_INPUT element, it will be configured as an output. Any mistake in both cases will be fatal.
I was thinking of Baytrail, which is using a different kind of macros. I know the Baytrail macros are position-dependent, but am not sure about the ones you linked (I have no board which uses that code). My observations on Baytrail may not apply to these macros, but it would be nice to check.
At least on Baytrail, GPIO muxes default to function 0, which may or may not be a GPIO. However, GPIOs seem to default to inputs. In addition, GPIO_NC on Baytrail is defined as GPIO_OUT_HIGH :) It makes sense if you consider unconnected pins set as inputs will float, though. Floating inputs can act as antennas and activate randomly, which is not desired.
If you miss '.gpio16 = GPIO_DIR_INPUT', only GPIO16 will be affected. However, with the Baytrail GPIO array, GPIO16 would end up with the settings for GPIO17, GPIO17 would end up with the settings for GPIO18... That could cause a lot of havoc.
In any case, this is not a blocker for this change.
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34337 )
Change subject: soc/intel/common: add PAD_CFG_NF_BUF_TRIG macro ......................................................................
soc/intel/common: add PAD_CFG_NF_BUF_TRIG macro
In the case there is no the circuit diagram for motherboard, the PCH/SoC GPIOs config is based on information from the inteltool dump. However, available macros from gpio_defs.h can't define the pad configuration from this dump:
0x0440: 0x0000002084000500 GPP_A8 CLKRUN# 0x0448: 0x0000102184000600 GPP_A9 CLKOUT_LPC0 0x0450: 0x0000102284000600 GPP_A10 CLKOUT_LPC1
To convert these raw DW0/DW1 register values to macros, the following parameters must be set:
func - pad function, pull - termination, rst - pad reset config, trig - rx level/edge configuration, bufdis - rx/tx (in/output) buffer disable.
The patch resolves the above problem by adding a new macro for the native function configuration:
PAD_CFG_NF_BUF_TRIG(pad, pull, rst, func, bufdis, trig)
These changes were tested on Asrock H110M-DVS motherboard [2]. It also resolves the problem of automatically creating pads configuration [3,4]
[1] page 1429,Intel (R) 100 Series and Intel (R) C230 Series PCH Family Platform Controller Hub (PCH), Datasheet, Vol 2 of 2, February 2019, Document Number: 332691-003EN https://www.intel.com/content/dam/www/public/us/en/documents/ datasheets/100-series-chipset-datasheet-vol-2.pdf [2] https://review.coreboot.org/c/coreboot/+/33565 [3] https://github.com/maxpoliak/pch-pads-parser/issues/1 [4] https://github.com/maxpoliak/pch-pads-parser/commit/215d303
Change-Id: If9fe50ff9a680633db6228564345200c0e1ee3ea Signed-off-by: Maxim Polyakov max.senia.poliak@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34337 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/common/block/include/intelblocks/gpio_defs.h 1 file changed, 18 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h index e1ddd4b..0a3e117 100644 --- a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h +++ b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h @@ -134,6 +134,15 @@ #define PAD_RESET(value) PAD_CFG0_LOGICAL_RESET_##value #define PAD_PULL(value) PAD_CFG1_PULL_##value
+/* Disable the input/output buffer of the pad */ +#define PAD_CFG0_BUF_NO_DISABLE (0) +#define PAD_CFG0_BUF_TX_DISABLE PAD_CFG0_TX_DISABLE +#define PAD_CFG0_BUF_RX_DISABLE PAD_CFG0_RX_DISABLE +#define PAD_CFG0_BUF_TX_RX_DISABLE \ + (PAD_CFG0_TX_DISABLE | PAD_CFG0_RX_DISABLE) + +#define PAD_BUF(value) PAD_CFG0_BUF_##value + #if CONFIG(SOC_INTEL_COMMON_BLOCK_GPIO_IOSTANDBY) #define PAD_IOSSTATE(value) PAD_CFG1_IOSSTATE_##value #define PAD_IOSTERM(value) PAD_CFG1_IOSTERM_##value @@ -180,6 +189,15 @@ _PAD_CFG_STRUCT(pad, PAD_RESET(rst) | PAD_FUNC(func), PAD_PULL(pull) | \ PAD_IOSSTATE(TxLASTRxE))
+/* + * Set native function with RX Level/Edge configuration and disable + * input/output buffer if necessary + */ +#define PAD_CFG_NF_BUF_TRIG(pad, pull, rst, func, bufdis, trig) \ + _PAD_CFG_STRUCT(pad, PAD_RESET(rst) | PAD_CFG0_TRIG_##trig | \ + PAD_BUF(bufdis) | PAD_FUNC(func), \ + PAD_PULL(pull) | PAD_IOSSTATE(TxLASTRxE)) + #if CONFIG(SOC_INTEL_COMMON_BLOCK_GPIO_PADCFG_PADTOL) /* Native 1.8V tolerant pad, only applies to some pads like I2C/I2S Not applicable to all SOCs. Refer EDS
-
Angel Pons (Code Review) -
build bot (Jenkins) (Code Review)
-
Furquan Shaikh (Code Review)
-
Martin Roth (Code Review)
-
Maxim Polyakov (Code Review)