Change in coreboot[master]: mb/asus/f2a85-m_pro: Clean up super-i/o GPIO settings

[Nico Huber (Code Review)]

Hello Paul Menzel,

I'd like you to do a code review. Please visit

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

to review the following change.

Change subject: mb/asus/f2a85-m_pro: Clean up super-i/o GPIO settings
......................................................................

mb/asus/f2a85-m_pro: Clean up super-i/o GPIO settings

Drop useless writes to read-only registers and don't re-write
default 0x00 values. In detail:

* Don't write read-only status registers.
* Don't try to write input bits in data registers
  (iow. mask data values: `data &= ~io`).
* Don't write data registers if all GPIOs are set as
  inputs (`io == 0xff`).
* Don't write default 0x00 for inversion and multiplex
  registers.

Note: Both GPIO0 and WDT1 values look spurious. Maybe they
were dumped with the virtual devices disabled?

Change-Id: I7d948d6b697285e61e4352b7354b924dbf511e9a
Signed-off-by: Nico Huber 
---
M src/mainboard/asus/f2a85-m/devicetree_f2a85-m_pro.cb
1 file changed, 2 insertions(+), 36 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/46020/1

diff --git a/src/mainboard/asus/f2a85-m/devicetree_f2a85-m_pro.cb b/src/mainboard/asus/f2a85-m/devicetree_f2a85-m_pro.cb
index aa2d52e..654716b 100644
--- a/src/mainboard/asus/f2a85-m/devicetree_f2a85-m_pro.cb
+++ b/src/mainboard/asus/f2a85-m/devicetree_f2a85-m_pro.cb
@@ -46,36 +46,18 @@
    					device pnp 2e.6 off end # CIR
    					device pnp 2e.7 on	# GPIO6, GPIO7, GPIO8
    						irq 0xf4 = 0xff # GPIO6 i/o
-							irq 0xf5 = 0xff # GPIO6 data
-							irq 0xf6 = 0x00 # GPIO6 inversion
-							irq 0xf7 = 0x00 # GPIO6 status
-							irq 0xf8 = 0x00 # GPIO6 multiplex

irq 0xe0 = 0x7f # GPIO7 i/o
-							irq 0xe1 = 0x10 # GPIO7 data
-							irq 0xe2 = 0x00 # GPIO7 inversion
-							irq 0xe3 = 0x00 # GPIO7 status
-							irq 0xec = 0x00 # GPIO7 multiplex
-
-							irq 0xe4 = 0xff # GPIO8 i/o
-							irq 0xe5 = 0xff # GPIO8 data
-							irq 0xe6 = 0xff # GPIO8 inversion
-							irq 0xe7 = 0xff # GPIO8 status
-							irq 0xed = 0xff # GPIO8 multiplex
+							irq 0xe1 = 0x00 # GPIO7 data
    					end
    					device pnp 2e.008 off	# WDT1
    					end
    					device pnp 2e.108 on	# GPIO0, GPIO1
    						irq 0xe0 = 0xff # GPIO0 i/o
-							irq 0xe1 = 0xff # GPIO0 data
    						irq 0xe2 = 0xff # GPIO0 inversion
-							irq 0xe3 = 0xff # GPIO0 status
    						irq 0xe4 = 0xff # GPIO0 multiplex

irq 0xf0 = 0xff # GPIO1 i/o
-							irq 0xf1 = 0x28 # GPIO1 data
-							irq 0xf2 = 0x00 # GPIO1 inversion
-							irq 0xf3 = 0x00 # GPIO1 status
    						irq 0xf4 = 0x08 # GPIO1 multiplex

irq 0xf5 = 0xff # WDT1 control mode
@@ -88,32 +70,16 @@
    					end
    					device pnp 2e.209 on	# GPIO2
    						irq 0xe0 = 0xff # GPIO2 i/o
-							irq 0xe1 = 0x90 # GPIO2 data
-							irq 0xe2 = 0x00 # GPIO2 inversion
-							irq 0xe3 = 0x00 # GPIO2 status
-							irq 0xe9 = 0x00 # GPIO2 multiplex
    					end
    					device pnp 2e.309 on	# GPIO3
    						irq 0xe4 = 0x7f # GPIO3 i/o
-							irq 0xe5 = 0x76 # GPIO3 data
-							irq 0xe6 = 0x00 # GPIO3 inversion
-							irq 0xe7 = 0x00 # GPIO3 status
-							irq 0xea = 0x00 # GPIO3 multiplex
-							irq 0xfe = 0x00 # GPIO3/4 debounce
+							irq 0xe5 = 0x00 # GPIO3 data
    					end
    					device pnp 2e.409 on	# GPIO4
    						irq 0xf0 = 0xff # GPIO4 i/o
-							irq 0xf1 = 0x7b # GPIO4 data
-							irq 0xf2 = 0x00 # GPIO4 inversion
-							irq 0xe8 = 0x00 # GPIO4 status
-							irq 0xee = 0x00 # GPIO4 multiplex
    					end
    					device pnp 2e.509 on	# GPIO5
    						irq 0xf4 = 0xff # GPIO5 i/o
-							irq 0xf5 = 0xef # GPIO5 data
-							irq 0xf6 = 0x00 # GPIO5 inversion
-							irq 0xf7 = 0x00 # GPIO5 status
-							irq 0xeb = 0x00 # GPIO5 multiplex
    					end
    					device pnp 2e.609 on	# GPIO6
    					end

[Angel Pons (Code Review)]

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

Change subject: mb/asus/f2a85-m_pro: Clean up super-i/o GPIO settings
......................................................................

Patch Set 2: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/46020/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/46020/2//COMMIT_MSG@7 
PS2, Line 7: super-i/o
nit: super I/O

Change in coreboot[master]: mb/asus/f2a85-m_pro: Clean up super-I/O GPIO settings

[Nico Huber (Code Review)]

Hello build bot (Jenkins), Paul Menzel, Angel Pons,

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

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

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

Change subject: mb/asus/f2a85-m_pro: Clean up super-I/O GPIO settings
......................................................................

mb/asus/f2a85-m_pro: Clean up super-I/O GPIO settings

Drop useless writes to read-only registers and don't re-write
default 0x00 values. In detail:

* Don't write read-only status registers.
* Don't try to write input bits in data registers
  (iow. mask data values: `data &= ~io`).
* Don't write data registers if all GPIOs are set as
  inputs (`io == 0xff`).
* Don't write default 0x00 for inversion and multiplex
  registers.

Note: Both GPIO0 and WDT1 values look spurious. Maybe they
were dumped with the virtual devices disabled?

Change-Id: I7d948d6b697285e61e4352b7354b924dbf511e9a
Signed-off-by: Nico Huber 
---
M src/mainboard/asus/f2a85-m/devicetree_f2a85-m_pro.cb
1 file changed, 2 insertions(+), 36 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/46020/3

Change in coreboot[master]: mb/asus/f2a85-m_pro: Clean up super-I/O GPIO settings

[Nico Huber (Code Review)]

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

Change subject: mb/asus/f2a85-m_pro: Clean up super-I/O GPIO settings
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/46020/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/46020/2//COMMIT_MSG@7 
PS2, Line 7: super-i/o
> nit: super I/O

Ack

Change in coreboot[master]: mb/asus/f2a85-m_pro: Clean up super-I/O GPIO settings

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46020 )

Change subject: mb/asus/f2a85-m_pro: Clean up super-I/O GPIO settings
......................................................................

mb/asus/f2a85-m_pro: Clean up super-I/O GPIO settings

Drop useless writes to read-only registers and don't re-write
default 0x00 values. In detail:

* Don't write read-only status registers.
* Don't try to write input bits in data registers
  (iow. mask data values: `data &= ~io`).
* Don't write data registers if all GPIOs are set as
  inputs (`io == 0xff`).
* Don't write default 0x00 for inversion and multiplex
  registers.

Note: Both GPIO0 and WDT1 values look spurious. Maybe they
were dumped with the virtual devices disabled?

Change-Id: I7d948d6b697285e61e4352b7354b924dbf511e9a
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46020
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M src/mainboard/asus/f2a85-m/devicetree_f2a85-m_pro.cb
1 file changed, 2 insertions(+), 36 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved

diff --git a/src/mainboard/asus/f2a85-m/devicetree_f2a85-m_pro.cb b/src/mainboard/asus/f2a85-m/devicetree_f2a85-m_pro.cb
index aa2d52e..654716b 100644
--- a/src/mainboard/asus/f2a85-m/devicetree_f2a85-m_pro.cb
+++ b/src/mainboard/asus/f2a85-m/devicetree_f2a85-m_pro.cb
@@ -46,36 +46,18 @@
    					device pnp 2e.6 off end # CIR
    					device pnp 2e.7 on	# GPIO6, GPIO7, GPIO8
    						irq 0xf4 = 0xff # GPIO6 i/o
-							irq 0xf5 = 0xff # GPIO6 data
-							irq 0xf6 = 0x00 # GPIO6 inversion
-							irq 0xf7 = 0x00 # GPIO6 status
-							irq 0xf8 = 0x00 # GPIO6 multiplex

irq 0xe0 = 0x7f # GPIO7 i/o
-							irq 0xe1 = 0x10 # GPIO7 data
-							irq 0xe2 = 0x00 # GPIO7 inversion
-							irq 0xe3 = 0x00 # GPIO7 status
-							irq 0xec = 0x00 # GPIO7 multiplex
-
-							irq 0xe4 = 0xff # GPIO8 i/o
-							irq 0xe5 = 0xff # GPIO8 data
-							irq 0xe6 = 0xff # GPIO8 inversion
-							irq 0xe7 = 0xff # GPIO8 status
-							irq 0xed = 0xff # GPIO8 multiplex
+							irq 0xe1 = 0x00 # GPIO7 data
    					end
    					device pnp 2e.008 off	# WDT1
    					end
    					device pnp 2e.108 on	# GPIO0, GPIO1
    						irq 0xe0 = 0xff # GPIO0 i/o
-							irq 0xe1 = 0xff # GPIO0 data
    						irq 0xe2 = 0xff # GPIO0 inversion
-							irq 0xe3 = 0xff # GPIO0 status
    						irq 0xe4 = 0xff # GPIO0 multiplex

irq 0xf0 = 0xff # GPIO1 i/o
-							irq 0xf1 = 0x28 # GPIO1 data
-							irq 0xf2 = 0x00 # GPIO1 inversion
-							irq 0xf3 = 0x00 # GPIO1 status
    						irq 0xf4 = 0x08 # GPIO1 multiplex

irq 0xf5 = 0xff # WDT1 control mode
@@ -88,32 +70,16 @@
    					end
    					device pnp 2e.209 on	# GPIO2
    						irq 0xe0 = 0xff # GPIO2 i/o
-							irq 0xe1 = 0x90 # GPIO2 data
-							irq 0xe2 = 0x00 # GPIO2 inversion
-							irq 0xe3 = 0x00 # GPIO2 status
-							irq 0xe9 = 0x00 # GPIO2 multiplex
    					end
    					device pnp 2e.309 on	# GPIO3
    						irq 0xe4 = 0x7f # GPIO3 i/o
-							irq 0xe5 = 0x76 # GPIO3 data
-							irq 0xe6 = 0x00 # GPIO3 inversion
-							irq 0xe7 = 0x00 # GPIO3 status
-							irq 0xea = 0x00 # GPIO3 multiplex
-							irq 0xfe = 0x00 # GPIO3/4 debounce
+							irq 0xe5 = 0x00 # GPIO3 data
    					end
    					device pnp 2e.409 on	# GPIO4
    						irq 0xf0 = 0xff # GPIO4 i/o
-							irq 0xf1 = 0x7b # GPIO4 data
-							irq 0xf2 = 0x00 # GPIO4 inversion
-							irq 0xe8 = 0x00 # GPIO4 status
-							irq 0xee = 0x00 # GPIO4 multiplex
    					end
    					device pnp 2e.509 on	# GPIO5
    						irq 0xf4 = 0xff # GPIO5 i/o
-							irq 0xf5 = 0xef # GPIO5 data
-							irq 0xf6 = 0x00 # GPIO5 inversion
-							irq 0xf7 = 0x00 # GPIO5 status
-							irq 0xeb = 0x00 # GPIO5 multiplex
    					end
    					device pnp 2e.609 on	# GPIO6
    					end