Change in coreboot[master]: soc/intel/common/gpio_defs: Add macros for bidirectional pad

[Maxim Polyakov (Code Review)]

Maxim Polyakov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42914 )


Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................

soc/intel/common/gpio_defs: Add macros for bidirectional pad

Adds new macros to configure the pad in bidirectional mode when both
(Tx/Rx) buffers are enabled in the configuration register DW0.

Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Signed-off-by: Maxim Polyakov <[e-mail]>
---
M src/soc/intel/common/block/include/intelblocks/gpio_defs.h
1 file changed, 14 insertions(+), 0 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42914/1

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 0668131..9bfc5dd 100644
--- a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h
+++ b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h
@@ -301,6 +301,20 @@
 #define PAD_CFG_GPI_INT(pad, pull, rst, trig)	\
 		PAD_CFG_GPI_TRIG_OWN(pad, pull, rst, trig, DRIVER)
 
+/* Bidirectional GPIO port when both RX and TX buffer is enabled */
+#define PAD_CFG_GPIO_BIDIRECT_IOS(pad, val, pull, rst, trig, iosstate, iosterm, own) \
+	_PAD_CFG_STRUCT(pad,						\
+		PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_TRIG(trig) |	\
+		PAD_BUF(NO_DISABLE) | val,				\
+		PAD_PULL(pull) | PAD_CFG_OWN_GPIO(own) |		\
+		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm))
+
+#define PAD_CFG_GPIO_BIDIRECT(pad, val, pull, rst, trig, own)		\
+	_PAD_CFG_STRUCT(pad,						\
+		PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_TRIG(trig) |	\
+		PAD_BUF(NO_DISABLE) | val,				\
+		PAD_PULL(pull) | PAD_CFG_OWN_GPIO(own))
+
 /*
  * No Connect configuration for unused pad.
  * Both TX and RX are disabled. RX disabling is done to avoid unnecessary

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 1
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-MessageType: newchange

[Maxim Polyakov (Code Review)]

Hello build bot (Jenkins), Patrick Rudolph, 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/coreboot/+/42914

to look at the new patch set (#3).

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................

soc/intel/common/gpio_defs: Add macros for bidirectional pad

Adds new macros to configure the pad in bidirectional mode when both
(Tx/Rx) buffers are enabled in the configuration register DW0.

Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Signed-off-by: Maxim Polyakov <[e-mail]>
---
M src/soc/intel/common/block/include/intelblocks/gpio_defs.h
1 file changed, 14 insertions(+), 0 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42914/3
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 3
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-MessageType: newpatchset

[Maxim Polyakov (Code Review)]

Hello Lance Zhao, build bot (Jenkins), Nico Huber, Angel Pons, Patrick Rudolph, 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/coreboot/+/42914

to look at the new patch set (#10).

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................

soc/intel/common/gpio_defs: Add macros for bidirectional pad

Adds new macros to configure the pad in bidirectional mode when both
(Tx/Rx) buffers are enabled in the configuration register DW0.

This macro is used in the pad configuration for some boards:
[1] CB:43456 - mb/intel/cedarisland_crb;
[2] CB:39133 - mb/kontron/mal10;
[3] CB:43410 - mb/51nb/x210;
[4] CB:43411 - mb/razer/blade_stealth_kbl.

Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Signed-off-by: Maxim Polyakov <[e-mail]>
---
M src/soc/intel/common/block/include/intelblocks/gpio_defs.h
1 file changed, 14 insertions(+), 0 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42914/10
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 10
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Michael Niewöhner
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: newpatchset

[Maxim Polyakov (Code Review)]

Hello Lance Zhao, build bot (Jenkins), Nico Huber, Angel Pons, Patrick Rudolph, 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/coreboot/+/42914

to look at the new patch set (#11).

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................

soc/intel/common/gpio_defs: Add macros for bidirectional pad

Adds new macros to configure the pad in bidirectional mode when both
(Tx/Rx) buffers are enabled in the configuration register DW0. This
corresponds to FSP's < GpioDirInOut > parameter for port direction
(for example see config for SOUTH_GROUP0_DFX_SPARE2 pad in
src/mainboard/intel/harcuvar/gpio.h).

This macro is used in the pad configuration for some boards:
 CB:43456 - mb/intel/cedarisland_crb;
 CB:39133 - mb/kontron/mal10;
 CB:43410 - mb/51nb/x210;
 CB:43411 - mb/razer/blade_stealth_kbl.

Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Signed-off-by: Maxim Polyakov <[e-mail]>
---
M src/soc/intel/common/block/include/intelblocks/gpio_defs.h
1 file changed, 14 insertions(+), 0 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42914/11
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 11
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Michael Niewöhner
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: newpatchset

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42914 )

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................


Patch Set 11: Code-Review+1


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 11
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Sat, 25 Jul 2020 00:13:46 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Maxim Polyakov (Code Review)]

Hello Lance Zhao, build bot (Jenkins), Nico Huber, Angel Pons, Michael Niewöhner, Patrick Rudolph, 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/coreboot/+/42914

to look at the new patch set (#13).

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................

soc/intel/common/gpio_defs: Add macros for bidirectional pad

Adds new macros to configure the pad in bidirectional mode when both
(Tx/Rx) buffers are enabled in the configuration register DW0. This
corresponds to FSP's < GpioDirInOut > parameter for port direction
(for example see config for SOUTH_GROUP0_DFX_SPARE2 pad in
src/mainboard/intel/harcuvar/gpio.h).

This macro is used in the pad configuration for some boards:
 CB:43456 - mb/intel/cedarisland_crb;
 CB:39133 - mb/kontron/mal10;
 CB:43410 - mb/51nb/x210;
 CB:43411 - mb/razer/blade_stealth_kbl.

Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Signed-off-by: Maxim Polyakov <[e-mail]>
---
M src/soc/intel/common/block/include/intelblocks/gpio_defs.h
1 file changed, 14 insertions(+), 0 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42914/13
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 13
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: newpatchset

[Maxim Polyakov (Code Review)]

Hello Lance Zhao, build bot (Jenkins), Nico Huber, Angel Pons, Michael Niewöhner, Patrick Rudolph, 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/coreboot/+/42914

to look at the new patch set (#14).

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................

soc/intel/common/gpio_defs: Add macros for bidirectional pad

Adds new macros to configure the pad in bidirectional mode when both
(Tx/Rx) buffers are enabled in the configuration register DW0. This
corresponds to FSP's < GpioDirInOut > parameter for port direction
(for example see config for SOUTH_GROUP0_DFX_SPARE2 pad in
src/mainboard/intel/harcuvar/gpio.h).

This macro is used in the pad configuration for some boards:
 CB:43456 - mb/intel/cedarisland_crb;
 CB:39133 - mb/kontron/mal10;
 CB:43410 - mb/51nb/x210;
 CB:43411 - mb/razer/blade_stealth_kbl.

Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Signed-off-by: Maxim Polyakov <[e-mail]>
---
M src/soc/intel/common/block/include/intelblocks/gpio_defs.h
1 file changed, 14 insertions(+), 0 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42914/14
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 14
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: newpatchset

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42914 )

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................


Patch Set 14:

What is this useful for? All the mentioned boards have probably
copy-pasta entries that nobody reasoned about. Adding a macro
might make it look like the entries are intentional and yet
something more to mantain... I don't think we should add any
entry that doesn't make sense in the context of the coreboot
ports.


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 14
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Sat, 25 Jul 2020 11:36:43 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42914 )

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................


Patch Set 14:
> Patch Set 14:
>
> What is this useful for? All the mentioned boards have probably
> copy-pasta entries that nobody reasoned about. Adding a macro
> might make it look like the entries are intentional and yet
> something more to mantain... I don't think we should add any
> entry that doesn't make sense in the context of the coreboot
> ports.

Agreed.


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 14
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Sat, 25 Jul 2020 11:38:45 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42914 )

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................


Patch Set 14: -Code-Review


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 14
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Sun, 26 Jul 2020 13:33:47 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42914 )

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................


Patch Set 14:
> Patch Set 14:
> > Patch Set 14:
> >
> > What is this useful for? All the mentioned boards have probably
> > copy-pasta entries that nobody reasoned about. Adding a macro
> > might make it look like the entries are intentional and yet
> > something more to mantain... I don't think we should add any
> > entry that doesn't make sense in the context of the coreboot
> > ports.
>
> Agreed.

ack


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 14
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Sun, 26 Jul 2020 13:33:57 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Maxim Polyakov (Code Review)]

Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42914 )

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................


Patch Set 15:

This patch was published for the following reasons:

1. Reply to the review:
https://review.coreboot.org/c/coreboot/+/39133/20/src/mainboard/kontron/mal1...

2. I have been working on converting raw values to macros in GPIO config and
this config for some boards contains pads in which both the Rx and Tx buffers
are enabled.

Please can you help me with this macro?
I'm not sure about this port configuration and I didn’t find any information
about it, except for a few pads from the gpio.h files (see the commit message).
Do you know anything about this? Is this configuration correct for a pad in the
Intel GPIO controller? If not, what should I use instead? GPI or GPO?

Thanks.

Alternative way:

1) add this macro to the local gpio.h for each of the boards;
2) always use bitfield macros and don't convert this to PAD_CFG_*() macro:

_PAD_CFG_STRUCT(GPP_I4,
		PAD_FUNC(GPIO) | PAD_RESET(RSMRST) |
		PAD_CFG0_TRIG_OFF | PAD_CFG0_RX_POL_NONE | PAD_BUF(NO_DISABLE),
		PAD_CFG_OWN_GPIO(ACPI) | PAD_PULL(NONE)),


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 15
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Andrey Petrov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Felix Held <[e-mail]>
Gerrit-Reviewer: Furquan Shaikh <[e-mail]>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Michael Niewöhner
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Georgi <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: Subrata Banik <[e-mail]>
Gerrit-Reviewer: Werner Zeh <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Benjamin Doron <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Fri, 14 Aug 2020 18:30:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42914 )

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................


Patch Set 15:
> Patch Set 15:
>
> This patch was published for the following reasons:
>
> 1. Reply to the review:
> https://review.coreboot.org/c/coreboot/+/39133/20/src/mainboard/kontron/mal1...
>
> 2. I have been working on converting raw values to macros in GPIO config and
> this config for some boards contains pads in which both the Rx and Tx buffers
> are enabled.
>
> Please can you help me with this macro?
> I'm not sure about this port configuration and I didn’t find any information
> about it, except for a few pads from the gpio.h files (see the commit message).
> Do you know anything about this? Is this configuration correct for a pad in the
> Intel GPIO controller? If not, what should I use instead? GPI or GPO?
>
> Thanks.
>
> Alternative way:
>
> 1) add this macro to the local gpio.h for each of the boards;
> 2) always use bitfield macros and don't convert this to PAD_CFG_*() macro:
>
> _PAD_CFG_STRUCT(GPP_I4,
>       PAD_FUNC(GPIO) | PAD_RESET(RSMRST) |
>       PAD_CFG0_TRIG_OFF | PAD_CFG0_RX_POL_NONE | PAD_BUF(NO_DISABLE),
>       PAD_CFG_OWN_GPIO(ACPI) | PAD_PULL(NONE)),

Do you know what the bidir pad is used for on Kontron mal10? If not, Nico or Felix maybe?


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 15
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer:  Felix Singer <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Andrey Petrov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Felix Held <[e-mail]>
Gerrit-Reviewer: Furquan Shaikh <[e-mail]>
Gerrit-Reviewer: Lance Zhao
Gerrit-Reviewer: Michael Niewöhner <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Georgi <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: Subrata Banik <[e-mail]>
Gerrit-Reviewer: Werner Zeh <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Benjamin Doron <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Tue, 17 Nov 2020 14:01:26 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[S] Change in coreboot[main]: soc/intel/common/gpio_defs: Add macros for bidirectional pad

[Matt DeVillier (Code Review)]

Attention is currently required from: Maxim.

Matt DeVillier has posted comments on this change by Maxim. ( https://review.coreboot.org/c/coreboot/+/42914?usp=email )

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................


Patch Set 15:

(1 comment)

Patchset:

PS15: 
looks like this needs a manual rebase @[e-mail]



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 15
Gerrit-Owner: Maxim <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Andrey Petrov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Felix Held <[e-mail]>
Gerrit-Reviewer: Felix Singer <[e-mail]>
Gerrit-Reviewer: Furquan Shaikh <[e-mail]>
Gerrit-Reviewer: Lance Zhao <[e-mail]>
Gerrit-Reviewer: Michael Niewöhner <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: Subrata Banik <[e-mail]>
Gerrit-Reviewer: Werner Zeh <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Matt DeVillier <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Attention: Maxim <[e-mail]>
Gerrit-Comment-Date: Wed, 11 Sep 2024 17:43:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

[S] Change in coreboot[main]: soc/intel/common/gpio_defs: Add macros for bidirectional pad

[Maxim (Code Review)]

Attention is currently required from: Maxim.

Hello Aaron Durbin, Andrey Petrov, Angel Pons, Felix Held, Felix Singer, Furquan Shaikh, Lance Zhao, Michael Niewöhner, Nico Huber, Patrick Rudolph, Subrata Banik, Werner Zeh, build bot (Jenkins), 

I'd like you to reexamine a change. Please visit

    https://review.coreboot.org/c/coreboot/+/42914?usp=email

to look at the new patch set (#16).


Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................

soc/intel/common/gpio_defs: Add macros for bidirectional pad

Adds new macros to configure the pad in bidirectional mode when both
(Tx/Rx) buffers are enabled in the configuration register DW0. This
corresponds to FSP's < GpioDirInOut > parameter for port direction
(for example see config for SOUTH_GROUP0_DFX_SPARE2 pad in
src/mainboard/intel/harcuvar/gpio.h).

This macro is used in the pad configuration for some boards:
 CB:43456 - mb/intel/cedarisland_crb;
 CB:39133 - mb/kontron/mal10;
 CB:43410 - mb/51nb/x210;
 CB:43411 - mb/razer/blade_stealth_kbl.

Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Signed-off-by: Maxim Polyakov <[e-mail]>
---
M src/soc/intel/common/block/include/intelblocks/gpio_defs.h
1 file changed, 14 insertions(+), 0 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/42914/16
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 16
Gerrit-Owner: Maxim <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Andrey Petrov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Felix Held <[e-mail]>
Gerrit-Reviewer: Felix Singer <[e-mail]>
Gerrit-Reviewer: Furquan Shaikh <[e-mail]>
Gerrit-Reviewer: Lance Zhao <[e-mail]>
Gerrit-Reviewer: Michael Niewöhner <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: Subrata Banik <[e-mail]>
Gerrit-Reviewer: Werner Zeh <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Matt DeVillier <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Attention: Maxim <[e-mail]>

[S] Change in coreboot[main]: soc/intel/common/gpio_defs: Add macros for bidirectional pad

[Nico Huber (Code Review)]

Attention is currently required from: Maxim.

Nico Huber has posted comments on this change by Maxim. ( https://review.coreboot.org/c/coreboot/+/42914?usp=email )

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................


Patch Set 16:

(1 comment)

Patchset:

PS16: 
I still believe we shouldn't use inexplicable vendor values as a
role model for coreboot. If I don't miss anything, with these
settings one could read back the exact value written. That's all
isn't it? If we don't have any software that does that (why would
it?), why make our GPIO configs less transparent?



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 16
Gerrit-Owner: Maxim <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Andrey Petrov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Felix Held <[e-mail]>
Gerrit-Reviewer: Felix Singer <[e-mail]>
Gerrit-Reviewer: Furquan Shaikh <[e-mail]>
Gerrit-Reviewer: Lance Zhao <[e-mail]>
Gerrit-Reviewer: Michael Niewöhner <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: Subrata Banik <[e-mail]>
Gerrit-Reviewer: Werner Zeh <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Matt DeVillier <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Attention: Maxim <[e-mail]>
Gerrit-Comment-Date: Thu, 12 Sep 2024 09:11:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

[S] Change in coreboot[main]: soc/intel/common/gpio_defs: Add macros for bidirectional pad

[Maxim (Code Review)]

Attention is currently required from: Matt DeVillier, Nico Huber.

Maxim has posted comments on this change by Maxim. ( https://review.coreboot.org/c/coreboot/+/42914?usp=email )

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................


Patch Set 16:

(2 comments)

Patchset:

PS15:
> looks like this needs a manual rebase @max.senia.poliak@gmail. […]
> Done

Patchset:

PS16:
> I still believe we shouldn't use inexplicable vendor values as a […]
> In my understanding, this is an alternative to < GpioDirInOut > config from fsp or reference bios. But I don't insist on it and the decision is yours =)

[S] Change in coreboot[main]: soc/intel/common/gpio_defs: Add macros for bidirectional pad

[Angel Pons (Code Review)]

Attention is currently required from: Matt DeVillier, Maxim, Nico Huber.

Angel Pons has posted comments on this change by Maxim. ( https://review.coreboot.org/c/coreboot/+/42914?usp=email )

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................


Patch Set 16: Code-Review+1

(1 comment)

Patchset:

PS16:
> In my understanding, this is an alternative to < GpioDirInOut > config from fsp or reference bios. […]
> Now I'm curious. Is there any known use-case anywhere for this `GpioDirInOut` setting? Ideally something with schematics.

[S] Change in coreboot[main]: soc/intel/common/gpio_defs: Add macros for bidirectional pad

[Maxim (Code Review)]

Attention is currently required from: Angel Pons, Matt DeVillier, Nico Huber.

Maxim has posted comments on this change by Maxim. ( https://review.coreboot.org/c/coreboot/+/42914?usp=email )

Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................


Patch Set 16:

(1 comment)

Patchset:

PS16:
> Now I'm curious. […]
> It seems to me that this is used for testing: the circuit is configured to allow us to read the value from the GPIO output buffer and check its value (for example, it does not match the set value if there is a short circuit). You'd better ask Intel about this. It depends on the internal circuit.

[S] Change in coreboot[main]: soc/intel/common/gpio_defs: Add macros for bidirectional pad

[Matt DeVillier (Code Review)]

Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42914?usp=email )

 ( 

16 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
 )Change subject: soc/intel/common/gpio_defs: Add macros for bidirectional pad
......................................................................

soc/intel/common/gpio_defs: Add macros for bidirectional pad

Adds new macros to configure the pad in bidirectional mode when both
(Tx/Rx) buffers are enabled in the configuration register DW0. This
corresponds to FSP's < GpioDirInOut > parameter for port direction
(for example see config for SOUTH_GROUP0_DFX_SPARE2 pad in
src/mainboard/intel/harcuvar/gpio.h).

This macro is used in the pad configuration for some boards:
 CB:43456 - mb/intel/cedarisland_crb;
 CB:39133 - mb/kontron/mal10;
 CB:43410 - mb/51nb/x210;
 CB:43411 - mb/razer/blade_stealth_kbl.

Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Signed-off-by: Maxim Polyakov <[e-mail]>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42914
Reviewed-by: coreboot org <[e-mail]>
Reviewed-by: Angel Pons <[e-mail]>
Tested-by: build bot (Jenkins) <[e-mail]>
---
M src/soc/intel/common/block/include/intelblocks/gpio_defs.h
1 file changed, 14 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, but someone else must approve
  coreboot org: 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 a51ebf3..f0ff08c 100644
--- a/src/soc/intel/common/block/include/intelblocks/gpio_defs.h
+++ b/src/soc/intel/common/block/include/intelblocks/gpio_defs.h
@@ -395,6 +395,20 @@
 #define PAD_CFG_GPI_INT_LOCK_SWAPPED(pad, pull, trig, lock_action)	\
 		PAD_CFG_GPI_TRIG_OWN_LOCK_SWAPPED(pad, pull, PWROK, trig, DRIVER, lock_action)
 
+/* Bidirectional GPIO port when both RX and TX buffer is enabled */
+#define PAD_CFG_GPIO_BIDIRECT_IOS(pad, val, pull, rst, trig, iosstate, iosterm, own) \
+	_PAD_CFG_STRUCT(pad,						\
+		PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_TRIG(trig) |	\
+		PAD_BUF(NO_DISABLE) | val,				\
+		PAD_PULL(pull) | PAD_CFG_OWN_GPIO(own) |		\
+		PAD_IOSSTATE(iosstate) | PAD_IOSTERM(iosterm))
+
+#define PAD_CFG_GPIO_BIDIRECT(pad, val, pull, rst, trig, own)		\
+	_PAD_CFG_STRUCT(pad,						\
+		PAD_FUNC(GPIO) | PAD_RESET(rst) | PAD_TRIG(trig) |	\
+		PAD_BUF(NO_DISABLE) | val,				\
+		PAD_PULL(pull) | PAD_CFG_OWN_GPIO(own))
+
 /*
  * No Connect configuration for unconnected or unused pad.
  * Both TX and RX are disabled. RX disabling is done to avoid unnecessary

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/42914?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: coreboot
Gerrit-Branch: main
Gerrit-Change-Id: I7b65f4da7616f2eefcd33a728d4d3ae5a79b014e
Gerrit-Change-Number: 42914
Gerrit-PatchSet: 20
Gerrit-Owner: Maxim Polyakov <[e-mail]>
Gerrit-Reviewer: Aaron Durbin <[e-mail]>
Gerrit-Reviewer: Andrey Petrov <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Felix Held <[e-mail]>
Gerrit-Reviewer: Felix Singer <[e-mail]>
Gerrit-Reviewer: Furquan Shaikh <[e-mail]>
Gerrit-Reviewer: Intel coreboot Reviewers <[e-mail]>
Gerrit-Reviewer: Lance Zhao <[e-mail]>
Gerrit-Reviewer: Matt DeVillier <[e-mail]>
Gerrit-Reviewer: Michael Niewöhner <[e-mail]>
Gerrit-Reviewer: Nico Huber <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: Subrata Banik <[e-mail]>
Gerrit-Reviewer: Werner Zeh <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Reviewer: coreboot org <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>