Change in coreboot[master]: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table

[Peichao Li (Code Review)]

Peichao Li has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table

BUG=b:140008849, b:140573677
TEST=verify eMMC SKU and SSD SKU will bring up normally.

Signed-off-by: Peichao Wang 
Change-Id: I0c0adf569cc92e8b44ab72379420f2b190fa31f5
---
M src/mainboard/google/hatch/variants/akemi/gpio.c
1 file changed, 2 insertions(+), 73 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/35344/1

diff --git a/src/mainboard/google/hatch/variants/akemi/gpio.c b/src/mainboard/google/hatch/variants/akemi/gpio.c
index 27b49e5..b873754 100644
--- a/src/mainboard/google/hatch/variants/akemi/gpio.c
+++ b/src/mainboard/google/hatch/variants/akemi/gpio.c
@@ -90,77 +90,6 @@
    PAD_CFG_GPI(GPP_H22, NONE, PLTRST),
 };

-static const struct pad_config emmc_sku_gpio_table[] = {
-	/* A0  : NC */
-	PAD_NC(GPP_A0, NONE),
-	/* A6  : NC */
-	PAD_NC(GPP_A6, NONE),
-	/* A8  : NC */
-	PAD_NC(GPP_A8, NONE),
-	/* A10 : NC */
-	PAD_NC(GPP_A10, NONE),
-	/* A11 : NC */
-	PAD_NC(GPP_A11, NONE),
-	/* A12 : NC */
-	PAD_NC(GPP_A12, NONE),
-	/* A18 : NC */
-	PAD_NC(GPP_A18, NONE),
-	/* A19 : NC */
-	PAD_NC(GPP_A19, NONE),
-	/* A22 : NC */
-	PAD_NC(GPP_A22, NONE),
-	/* A23 : NC */
-	PAD_NC(GPP_A23, NONE),
-	/* B20 : NC */
-	PAD_NC(GPP_B20, NONE),
-	/* B21 : NC */
-	PAD_NC(GPP_B21, NONE),
-	/* B22 : NC */
-	PAD_NC(GPP_B22, NONE),
-	/* C11 : NC */
-	PAD_NC(GPP_C11, NONE),
-	/* C15 : NC */
-	PAD_NC(GPP_C15, NONE),
-	/* F1  : NC */
-	PAD_NC(GPP_F1, NONE),
-	/* F3  : MEM_STRAP_3 */
-	PAD_CFG_GPI(GPP_F3, NONE, PLTRST),
-	/* F10 : MEM_STRAP_2 */
-	PAD_CFG_GPI(GPP_F10, NONE, PLTRST),
-	/* F11 : EMMC_CMD ==> EMMC_CMD */
-	PAD_CFG_NF(GPP_F11, NONE, DEEP, NF1),
-	/* F12 : EMMC_DATA0 ==> EMMC_DAT0 */
-	PAD_CFG_NF(GPP_F12, NONE, DEEP, NF1),
-	/* F13 : EMMC_DATA1 ==> EMMC_DAT1 */
-	PAD_CFG_NF(GPP_F13, NONE, DEEP, NF1),
-	/* F14 : EMMC_DATA2 ==> EMMC_DAT2 */
-	PAD_CFG_NF(GPP_F14, NONE, DEEP, NF1),
-	/* F15 : EMMC_DATA3 ==> EMMC_DAT3 */
-	PAD_CFG_NF(GPP_F15, NONE, DEEP, NF1),
-	/* F16 : EMMC_DATA4 ==> EMMC_DAT4 */
-	PAD_CFG_NF(GPP_F16, NONE, DEEP, NF1),
-	/* F17 : EMMC_DATA5 ==> EMMC_DAT5 */
-	PAD_CFG_NF(GPP_F17, NONE, DEEP, NF1),
-	/* F18 : EMMC_DATA6 ==> EMMC_DAT6 */
-	PAD_CFG_NF(GPP_F18, NONE, DEEP, NF1),
-	/* F19 : EMMC_DATA7 ==> EMMC_DAT7 */
-	PAD_CFG_NF(GPP_F19, NONE, DEEP, NF1),
-	/* F20 : EMMC_RCLK ==> EMMC_RCLK */
-	PAD_CFG_NF(GPP_F20, NONE, DEEP, NF1),
-	/* F21 : EMMC_CLK ==> EMMC_CLK */
-	PAD_CFG_NF(GPP_F21, NONE, DEEP, NF1),
-	/* F22 : EMMC_RESET# ==> EMMC_RST_L */
-	PAD_CFG_NF(GPP_F22, NONE, DEEP, NF1),
-	/* H6  : NC */
-	PAD_NC(GPP_H6, NONE),
-	/* H7  : NC */
-	PAD_NC(GPP_H7, NONE),
-	/* H19 : MEM_STRAP_0 */
-	PAD_CFG_GPI(GPP_H19, NONE, PLTRST),
-	/* H22 : MEM_STRAP_1 */
-	PAD_CFG_GPI(GPP_H22, NONE, PLTRST),
-};
-
 static const struct pad_config gpio_table[] = {
    /* A0  : NC */
    PAD_NC(GPP_A0, NONE),
@@ -242,8 +171,8 @@
    }
    /* For eMMC SKU */
    if (sku_id == 1) {
-		*num = ARRAY_SIZE(emmc_sku_gpio_table);
-		return emmc_sku_gpio_table;
+		*num = ARRAY_SIZE(gpio_table);
+		return gpio_table;
    }
    *num = ARRAY_SIZE(gpio_table);
    return gpio_table;

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
File src/mainboard/google/hatch/variants/akemi/gpio.c:

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
PS1, Line 173: if (sku_id == 1) {
You don't need this check anymore, right?

[Peichao Li (Code Review)]

Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
File src/mainboard/google/hatch/variants/akemi/gpio.c:

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
PS1, Line 173: if (sku_id == 1) {
> You don't need this check anymore, right?

At this moment, confirm with our PM, just have eMMC and SSD SKU, so not other section need to modify. Thanks a lot!

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
File src/mainboard/google/hatch/variants/akemi/gpio.c:

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
PS1, Line 173: if (sku_id == 1) {
> At this moment, confirm with our PM, just have eMMC and SSD SKU, so not other section need to modify […]

So, you removing this check?

[Peichao Li (Code Review)]

Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
File src/mainboard/google/hatch/variants/akemi/gpio.c:

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
PS1, Line 173: if (sku_id == 1) {
> So, you removing this check?

I need check SKU ID is 1 or 2.

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
File src/mainboard/google/hatch/variants/akemi/gpio.c:

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
PS1, Line 173: if (sku_id == 1) {
> I need check SKU ID is 1 or 2.

But why? 
If SKU ID is 2, then use ssd_sku_gpio_table, else use gpio_table. That is what currently happens with your change too. Isn't it?

[Peichao Li (Code Review)]

Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
File src/mainboard/google/hatch/variants/akemi/gpio.c:

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
PS1, Line 173: if (sku_id == 1) {
> But why?  […]

Yes, change cause is emmc_sku_gpio_table and gpio_table no different. So merge these two tables to one table. Thanks a lot!

[Marco Chen (Code Review)]

Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
File src/mainboard/google/hatch/variants/akemi/gpio.c:

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
PS1, Line 173: if (sku_id == 1) {
> Yes, change cause is emmc_sku_gpio_table and gpio_table no different. […]

Hi Peichao, Furquan reminded you that your line 174, 175 are the same with 177,178, right? As a result, you can just do this

if (sku_id == 2) {
    	*num = ARRAY_SIZE(ssd_sku_gpio_table);
    	return ssd_sku_gpio_table;
    }

*num = ARRAY_SIZE(gpio_table);
    return gpio_table;

[Peichao Li (Code Review)]

Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

Patch Set 1:
> Patch Set 1:
>
> (1 comment)

Thanks for you remind, it's great.

[Peichao Li (Code Review)]

Peichao Li has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
File src/mainboard/google/hatch/variants/akemi/gpio.c:

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
PS1, Line 173: if (sku_id == 1) {
> Hi Peichao, Furquan reminded you that your line 174, 175 are the same with 177,178, right? As a resu […]

Yes, it is same.

[Peichao Li (Code Review)]

Hello Marco Chen, build bot (Jenkins), Furquan Shaikh,

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

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

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

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table

BUG=b:140008849, b:140573677
TEST=verify eMMC SKU and SSD SKU will bring up normally.

Signed-off-by: Peichao Wang 
Change-Id: I0c0adf569cc92e8b44ab72379420f2b190fa31f5
---
M src/mainboard/google/hatch/variants/akemi/gpio.c
1 file changed, 0 insertions(+), 76 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/35344/2

[Marco Chen (Code Review)]

Marco Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

Patch Set 2: Code-Review+1

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

Patch Set 2: Code-Review+2

[Justin TerAvest (Code Review)]

Justin TerAvest has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

Patch Set 2: Code-Review+2

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
File src/mainboard/google/hatch/variants/akemi/gpio.c:

https://review.coreboot.org/c/coreboot/+/35344/1/src/mainboard/google/hatch/... 
PS1, Line 173: if (sku_id == 1) {
> Yes, it is same.

Done

[Furquan Shaikh (Code Review)]

Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35344 )

Change subject: mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table
......................................................................

mb/google/hatch: Merge emmc_sku_gpio_table and gpio_table to one table

BUG=b:140008849, b:140573677
TEST=verify eMMC SKU and SSD SKU will bring up normally.

Signed-off-by: Peichao Wang 
Change-Id: I0c0adf569cc92e8b44ab72379420f2b190fa31f5
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35344
Tested-by: build bot (Jenkins) 
Reviewed-by: Marco Chen 
Reviewed-by: Furquan Shaikh 
Reviewed-by: Justin TerAvest 
---
M src/mainboard/google/hatch/variants/akemi/gpio.c
1 file changed, 0 insertions(+), 76 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved
  Justin TerAvest: Looks good to me, approved
  Marco Chen: Looks good to me, but someone else must approve

diff --git a/src/mainboard/google/hatch/variants/akemi/gpio.c b/src/mainboard/google/hatch/variants/akemi/gpio.c
index 27b49e5..1ba9d35 100644
--- a/src/mainboard/google/hatch/variants/akemi/gpio.c
+++ b/src/mainboard/google/hatch/variants/akemi/gpio.c
@@ -90,77 +90,6 @@
    PAD_CFG_GPI(GPP_H22, NONE, PLTRST),
 };

-static const struct pad_config emmc_sku_gpio_table[] = {
-	/* A0  : NC */
-	PAD_NC(GPP_A0, NONE),
-	/* A6  : NC */
-	PAD_NC(GPP_A6, NONE),
-	/* A8  : NC */
-	PAD_NC(GPP_A8, NONE),
-	/* A10 : NC */
-	PAD_NC(GPP_A10, NONE),
-	/* A11 : NC */
-	PAD_NC(GPP_A11, NONE),
-	/* A12 : NC */
-	PAD_NC(GPP_A12, NONE),
-	/* A18 : NC */
-	PAD_NC(GPP_A18, NONE),
-	/* A19 : NC */
-	PAD_NC(GPP_A19, NONE),
-	/* A22 : NC */
-	PAD_NC(GPP_A22, NONE),
-	/* A23 : NC */
-	PAD_NC(GPP_A23, NONE),
-	/* B20 : NC */
-	PAD_NC(GPP_B20, NONE),
-	/* B21 : NC */
-	PAD_NC(GPP_B21, NONE),
-	/* B22 : NC */
-	PAD_NC(GPP_B22, NONE),
-	/* C11 : NC */
-	PAD_NC(GPP_C11, NONE),
-	/* C15 : NC */
-	PAD_NC(GPP_C15, NONE),
-	/* F1  : NC */
-	PAD_NC(GPP_F1, NONE),
-	/* F3  : MEM_STRAP_3 */
-	PAD_CFG_GPI(GPP_F3, NONE, PLTRST),
-	/* F10 : MEM_STRAP_2 */
-	PAD_CFG_GPI(GPP_F10, NONE, PLTRST),
-	/* F11 : EMMC_CMD ==> EMMC_CMD */
-	PAD_CFG_NF(GPP_F11, NONE, DEEP, NF1),
-	/* F12 : EMMC_DATA0 ==> EMMC_DAT0 */
-	PAD_CFG_NF(GPP_F12, NONE, DEEP, NF1),
-	/* F13 : EMMC_DATA1 ==> EMMC_DAT1 */
-	PAD_CFG_NF(GPP_F13, NONE, DEEP, NF1),
-	/* F14 : EMMC_DATA2 ==> EMMC_DAT2 */
-	PAD_CFG_NF(GPP_F14, NONE, DEEP, NF1),
-	/* F15 : EMMC_DATA3 ==> EMMC_DAT3 */
-	PAD_CFG_NF(GPP_F15, NONE, DEEP, NF1),
-	/* F16 : EMMC_DATA4 ==> EMMC_DAT4 */
-	PAD_CFG_NF(GPP_F16, NONE, DEEP, NF1),
-	/* F17 : EMMC_DATA5 ==> EMMC_DAT5 */
-	PAD_CFG_NF(GPP_F17, NONE, DEEP, NF1),
-	/* F18 : EMMC_DATA6 ==> EMMC_DAT6 */
-	PAD_CFG_NF(GPP_F18, NONE, DEEP, NF1),
-	/* F19 : EMMC_DATA7 ==> EMMC_DAT7 */
-	PAD_CFG_NF(GPP_F19, NONE, DEEP, NF1),
-	/* F20 : EMMC_RCLK ==> EMMC_RCLK */
-	PAD_CFG_NF(GPP_F20, NONE, DEEP, NF1),
-	/* F21 : EMMC_CLK ==> EMMC_CLK */
-	PAD_CFG_NF(GPP_F21, NONE, DEEP, NF1),
-	/* F22 : EMMC_RESET# ==> EMMC_RST_L */
-	PAD_CFG_NF(GPP_F22, NONE, DEEP, NF1),
-	/* H6  : NC */
-	PAD_NC(GPP_H6, NONE),
-	/* H7  : NC */
-	PAD_NC(GPP_H7, NONE),
-	/* H19 : MEM_STRAP_0 */
-	PAD_CFG_GPI(GPP_H19, NONE, PLTRST),
-	/* H22 : MEM_STRAP_1 */
-	PAD_CFG_GPI(GPP_H22, NONE, PLTRST),
-};
-
 static const struct pad_config gpio_table[] = {
    /* A0  : NC */
    PAD_NC(GPP_A0, NONE),
@@ -240,11 +169,6 @@
    	*num = ARRAY_SIZE(ssd_sku_gpio_table);
    	return ssd_sku_gpio_table;
    }
-	/* For eMMC SKU */
-	if (sku_id == 1) {
-		*num = ARRAY_SIZE(emmc_sku_gpio_table);
-		return emmc_sku_gpio_table;
-	}
    *num = ARRAY_SIZE(gpio_table);
    return gpio_table;
 }