[PATCH v3 1/1] x86: Support APU5 in PCEngines platform driver

[PATCH v3 1/1] x86: Support APU5 in PCEngines platform driver

[Philip Prindeville]

From: Philip Prindeville <philipp@redfish-solutions.com>

PCEngines make a number of SBC. APU5 has 5 mpcie slots + MSATA.
It also has support for 3x LTE modems with 6x SIM slots (pairs with a
SIM switch device). Each mpcie slot for modems has a reset GPIO

To ensure that the naming is sane between APU2-6 the GPIOS are
renamed to be modem1-reset, modem2-reset, etc. This is significant
because the slots that can be reset change between APU2 and APU3/4

GPIO for simswap is moved to the end of the list as it could be dropped
for APU2 boards (but causes no harm to leave it in, hardware could be
added to a future rev of the board).

Structure of the GPIOs for APU5 is extremely similar to APU2-4, but
many lines are moved around and there are simply more
modems/resets/sim-swap lines to breakout.

Also added APU6, which is essentially APU4 with a different ethernet
interface and SFP cage on eth0.

Revision history:

v1: originally titled, "apu6: add apu6 variation to apu2 driver family"
this dealt only with detecting the APUv6, which is otherwise identical
to the v4 excepting the SFP cage on eth0.

v2: at Ed's request, merged with his previous pull-request titled
"x86: Support APU5 in PCEngines platform driver", and some cleanup
to that changeset (including dropping the table "apu5_driver_data"
which did not have a defined type "struct apu_driver_data"), but got
mistitled when the Subject of that commit got accidentally dropped.

v3: retitled to match Ed's previous pull-request.

Cc: platform-driver-x86@vger.kernel.org
Cc: linux-x86_64@vger.kernel.org
Reviewed-by: Andreas Eberlein <foodeas@aeberlein.de>
Reviewed-by: Paul Spooren <paul@spooren.de>
Signed-off-by: Ed Wildgoose <lists@wildgooses.com>
Sighed-off-by: Philip Prindeville <philipp@redfish-solutions.com>
---
 drivers/leds/leds-apu.c                |   2 +-
 drivers/platform/x86/Kconfig           |   4 +-
 drivers/platform/x86/pcengines-apuv2.c | 118 ++++++++++++++++++++++---
 3 files changed, 107 insertions(+), 17 deletions(-)

diff --git a/drivers/leds/leds-apu.c b/drivers/leds/leds-apu.c
index c409b80c236d7182c79944225dd4b9e5d32f0a9f..fb3332863c36241e4b7f75f89213171ec0c0dd04 100644
--- a/drivers/leds/leds-apu.c
+++ b/drivers/leds/leds-apu.c
@@ -183,7 +183,7 @@ static int __init apu_led_init(void)
 
 	if (!(dmi_match(DMI_SYS_VENDOR, "PC Engines") &&
 	      (dmi_match(DMI_PRODUCT_NAME, "APU") || dmi_match(DMI_PRODUCT_NAME, "apu1")))) {
-		pr_err("No PC Engines APUv1 board detected. For APUv2,3 support, enable CONFIG_PCENGINES_APU2\n");
+		pr_err("No PC Engines APUv1 board detected. For APUv2,3,4,5,6 support, enable CONFIG_PCENGINES_APU2\n");
 		return -ENODEV;
 	}
 
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 5692385e2d26fed92785488ce1c6e88449c54c78..f930d5dd3efee11fbad4ad9508eb288bb4181bab 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -698,7 +698,7 @@ config XO1_RFKILL
 	  laptop.
 
 config PCENGINES_APU2
-	tristate "PC Engines APUv2/3 front button and LEDs driver"
+	tristate "PC Engines APUv2/3/4/5/6 front button and LEDs driver"
 	depends on INPUT && INPUT_KEYBOARD && GPIOLIB
 	depends on LEDS_CLASS
 	select GPIO_AMD_FCH
@@ -706,7 +706,7 @@ config PCENGINES_APU2
 	select LEDS_GPIO
 	help
 	  This driver provides support for the front button and LEDs on
-	  PC Engines APUv2/APUv3 board.
+	  PC Engines APUv2/APUv3/APUv4/APUv5/APUv6 board.
 
 	  To compile this driver as a module, choose M here: the module
 	  will be called pcengines-apuv2.
diff --git a/drivers/platform/x86/pcengines-apuv2.c b/drivers/platform/x86/pcengines-apuv2.c
index d063d91db9bcbe5ceb2ac641d3105df37651ac4d..bd58d5294f0146088d435f1c9df83eb218b67b0c 100644
--- a/drivers/platform/x86/pcengines-apuv2.c
+++ b/drivers/platform/x86/pcengines-apuv2.c
@@ -1,10 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0+
 
 /*
- * PC-Engines APUv2/APUv3 board platform driver
+ * PC-Engines APUv2-6 board platform driver
  * for GPIO buttons and LEDs
  *
  * Copyright (C) 2018 metux IT consult
+ * Copyright (C) 2022 Ed Wildgoose <lists@wildgooses.com>
+ * Copyright (C) 2022 Philip Prindeville <philipp@redfish-solutions.com>
  * Author: Enrico Weigelt <info@metux.net>
  */
 
@@ -22,38 +24,70 @@
 #include <linux/platform_data/gpio/gpio-amd-fch.h>
 
 /*
- * NOTE: this driver only supports APUv2/3 - not APUv1, as this one
+ * NOTE: this driver only supports APUv2-6 - not APUv1, as this one
  * has completely different register layouts.
  */
 
+/*
+ * There are a number of APU variants, with differing features
+ * APU2 has SIM slots 1/2 mapping to mPCIe sockets 1/2
+ * APU3/4 moved SIM slot 1 to mPCIe socket 3, ie logically reversed
+ * However, most APU3/4 have a SIM switch which we default on to reverse
+ * the order and keep physical SIM order matching physical modem order
+ * APU6 is approximately the same as APU4 with different ethernet layout
+ *
+ * APU5 has 3x SIM sockets, all with a SIM switch
+ * several GPIOs are shuffled (see schematic), including MODESW
+ */
+
 /* Register mappings */
 #define APU2_GPIO_REG_LED1		AMD_FCH_GPIO_REG_GPIO57
 #define APU2_GPIO_REG_LED2		AMD_FCH_GPIO_REG_GPIO58
 #define APU2_GPIO_REG_LED3		AMD_FCH_GPIO_REG_GPIO59_DEVSLP1
 #define APU2_GPIO_REG_MODESW		AMD_FCH_GPIO_REG_GPIO32_GE1
 #define APU2_GPIO_REG_SIMSWAP		AMD_FCH_GPIO_REG_GPIO33_GE2
-#define APU2_GPIO_REG_MPCIE2		AMD_FCH_GPIO_REG_GPIO55_DEVSLP0
-#define APU2_GPIO_REG_MPCIE3		AMD_FCH_GPIO_REG_GPIO51
+#define APU2_GPIO_REG_RESETM1		AMD_FCH_GPIO_REG_GPIO51
+#define APU2_GPIO_REG_RESETM2		AMD_FCH_GPIO_REG_GPIO55_DEVSLP0
+
+#define APU5_GPIO_REG_MODESW		AMT_FCH_GPIO_REG_GEVT22
+#define APU5_GPIO_REG_SIMSWAP1		AMD_FCH_GPIO_REG_GPIO68
+#define APU5_GPIO_REG_SIMSWAP2		AMD_FCH_GPIO_REG_GPIO32_GE1
+#define APU5_GPIO_REG_SIMSWAP3		AMD_FCH_GPIO_REG_GPIO33_GE2
+#define APU5_GPIO_REG_RESETM1		AMD_FCH_GPIO_REG_GPIO51
+#define APU5_GPIO_REG_RESETM2		AMD_FCH_GPIO_REG_GPIO55_DEVSLP0
+#define APU5_GPIO_REG_RESETM3		AMD_FCH_GPIO_REG_GPIO64
 
 /* Order in which the GPIO lines are defined in the register list */
 #define APU2_GPIO_LINE_LED1		0
 #define APU2_GPIO_LINE_LED2		1
 #define APU2_GPIO_LINE_LED3		2
 #define APU2_GPIO_LINE_MODESW		3
-#define APU2_GPIO_LINE_SIMSWAP		4
-#define APU2_GPIO_LINE_MPCIE2		5
-#define APU2_GPIO_LINE_MPCIE3		6
+#define APU2_GPIO_LINE_RESETM1		4
+#define APU2_GPIO_LINE_RESETM2		5
+#define APU2_GPIO_LINE_SIMSWAP		6
+
+#define APU5_GPIO_LINE_LED1		0
+#define APU5_GPIO_LINE_LED2		1
+#define APU5_GPIO_LINE_LED3		2
+#define APU5_GPIO_LINE_MODESW		3
+#define APU5_GPIO_LINE_RESETM1		4
+#define APU5_GPIO_LINE_RESETM2		5
+#define APU5_GPIO_LINE_RESETM3		6
+#define APU5_GPIO_LINE_SIMSWAP1		7
+#define APU5_GPIO_LINE_SIMSWAP2		8
+#define APU5_GPIO_LINE_SIMSWAP3		9
 
-/* GPIO device */
+
+/* GPIO device - APU2/3/4/6 */
 
 static int apu2_gpio_regs[] = {
 	[APU2_GPIO_LINE_LED1]		= APU2_GPIO_REG_LED1,
 	[APU2_GPIO_LINE_LED2]		= APU2_GPIO_REG_LED2,
 	[APU2_GPIO_LINE_LED3]		= APU2_GPIO_REG_LED3,
 	[APU2_GPIO_LINE_MODESW]		= APU2_GPIO_REG_MODESW,
+	[APU2_GPIO_LINE_RESETM1]	= APU2_GPIO_REG_RESETM1,
+	[APU2_GPIO_LINE_RESETM2]	= APU2_GPIO_REG_RESETM2,
 	[APU2_GPIO_LINE_SIMSWAP]	= APU2_GPIO_REG_SIMSWAP,
-	[APU2_GPIO_LINE_MPCIE2]		= APU2_GPIO_REG_MPCIE2,
-	[APU2_GPIO_LINE_MPCIE3]		= APU2_GPIO_REG_MPCIE3,
 };
 
 static const char * const apu2_gpio_names[] = {
@@ -61,9 +95,9 @@ static const char * const apu2_gpio_names[] = {
 	[APU2_GPIO_LINE_LED2]		= "front-led2",
 	[APU2_GPIO_LINE_LED3]		= "front-led3",
 	[APU2_GPIO_LINE_MODESW]		= "front-button",
+	[APU2_GPIO_LINE_RESETM1]	= "modem1-reset",
+	[APU2_GPIO_LINE_RESETM2]	= "modem2-reset",
 	[APU2_GPIO_LINE_SIMSWAP]	= "simswap",
-	[APU2_GPIO_LINE_MPCIE2]		= "mpcie2_reset",
-	[APU2_GPIO_LINE_MPCIE3]		= "mpcie3_reset",
 };
 
 static const struct amd_fch_gpio_pdata board_apu2 = {
@@ -72,6 +106,40 @@ static const struct amd_fch_gpio_pdata board_apu2 = {
 	.gpio_names	= apu2_gpio_names,
 };
 
+/* GPIO device - APU5 */
+
+static int apu5_gpio_regs[] = {
+	[APU5_GPIO_LINE_LED1]		= APU2_GPIO_REG_LED1,
+	[APU5_GPIO_LINE_LED2]		= APU2_GPIO_REG_LED2,
+	[APU5_GPIO_LINE_LED3]		= APU2_GPIO_REG_LED3,
+	[APU5_GPIO_LINE_MODESW]		= APU5_GPIO_REG_MODESW,
+	[APU5_GPIO_LINE_RESETM1]	= APU5_GPIO_REG_RESETM1,
+	[APU5_GPIO_LINE_RESETM2]	= APU5_GPIO_REG_RESETM2,
+	[APU5_GPIO_LINE_RESETM3]	= APU5_GPIO_REG_RESETM3,
+	[APU5_GPIO_LINE_SIMSWAP1]	= APU5_GPIO_REG_SIMSWAP1,
+	[APU5_GPIO_LINE_SIMSWAP2]	= APU5_GPIO_REG_SIMSWAP2,
+	[APU5_GPIO_LINE_SIMSWAP3]	= APU5_GPIO_REG_SIMSWAP3,
+};
+
+static const char * const apu5_gpio_names[] = {
+	[APU5_GPIO_LINE_LED1]		= "front-led1",
+	[APU5_GPIO_LINE_LED2]		= "front-led2",
+	[APU5_GPIO_LINE_LED3]		= "front-led3",
+	[APU5_GPIO_LINE_MODESW]		= "front-button",
+	[APU5_GPIO_LINE_RESETM1]	= "modem1-reset",
+	[APU5_GPIO_LINE_RESETM2]	= "modem2-reset",
+	[APU5_GPIO_LINE_RESETM3]	= "modem3-reset",
+	[APU5_GPIO_LINE_SIMSWAP1]	= "simswap1",
+	[APU5_GPIO_LINE_SIMSWAP2]	= "simswap2",
+	[APU5_GPIO_LINE_SIMSWAP3]	= "simswap3",
+};
+
+static const struct amd_fch_gpio_pdata board_apu5 = {
+	.gpio_num	= ARRAY_SIZE(apu5_gpio_regs),
+	.gpio_reg	= apu5_gpio_regs,
+	.gpio_names	= apu5_gpio_names,
+};
+
 /* GPIO LEDs device */
 
 static const struct gpio_led apu2_leds[] = {
@@ -215,6 +283,24 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
 		},
 		.driver_data = (void *)&board_apu2,
 	},
+	/* APU5 w/ mainline BIOS */
+	{
+		.ident		= "apu5",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "apu5")
+		},
+		.driver_data	= (void *)&board_apu5,
+	},
+	/* APU6 w/ mainline BIOS */
+	{
+		.ident		= "apu6",
+		.matches	= {
+			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
+			DMI_MATCH(DMI_BOARD_NAME, "apu6")
+		},
+		.driver_data	= (void *)&board_apu2,
+	},
 	{}
 };
 
@@ -249,7 +335,7 @@ static int __init apu_board_init(void)
 
 	id = dmi_first_match(apu_gpio_dmi_table);
 	if (!id) {
-		pr_err("failed to detect APU board via DMI\n");
+		pr_err("No APU board detected via DMI\n");
 		return -ENODEV;
 	}
 
@@ -288,8 +374,12 @@ module_init(apu_board_init);
 module_exit(apu_board_exit);
 
 MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
-MODULE_DESCRIPTION("PC Engines APUv2/APUv3 board GPIO/LEDs/keys driver");
+MODULE_DESCRIPTION("PC Engines APUv2-6 board GPIO/LEDs/keys driver");
 MODULE_LICENSE("GPL");
 MODULE_DEVICE_TABLE(dmi, apu_gpio_dmi_table);
 MODULE_ALIAS("platform:pcengines-apuv2");
+MODULE_ALIAS("platform:pcengines-apuv3");
+MODULE_ALIAS("platform:pcengines-apuv4");
+MODULE_ALIAS("platform:pcengines-apuv5");
+MODULE_ALIAS("platform:pcengines-apuv6");
 MODULE_SOFTDEP("pre: platform:" AMD_FCH_GPIO_DRIVER_NAME " platform:leds-gpio platform:gpio_keys_polled");
-- 
2.34.1

Re: [PATCH v3 1/1] x86: Support APU5 in PCEngines platform driver

[Hans de Goede]

Hi,

On 1/2/23 07:54, Philip Prindeville wrote:
> From: Philip Prindeville <philipp@redfish-solutions.com>
> 
> PCEngines make a number of SBC. APU5 has 5 mpcie slots + MSATA.
> It also has support for 3x LTE modems with 6x SIM slots (pairs with a
> SIM switch device). Each mpcie slot for modems has a reset GPIO
> 
> To ensure that the naming is sane between APU2-6 the GPIOS are
> renamed to be modem1-reset, modem2-reset, etc. This is significant
> because the slots that can be reset change between APU2 and APU3/4
> 
> GPIO for simswap is moved to the end of the list as it could be dropped
> for APU2 boards (but causes no harm to leave it in, hardware could be
> added to a future rev of the board).
> 
> Structure of the GPIOs for APU5 is extremely similar to APU2-4, but
> many lines are moved around and there are simply more
> modems/resets/sim-swap lines to breakout.
> 
> Also added APU6, which is essentially APU4 with a different ethernet
> interface and SFP cage on eth0.
> 
> Revision history:
> 
> v1: originally titled, "apu6: add apu6 variation to apu2 driver family"
> this dealt only with detecting the APUv6, which is otherwise identical
> to the v4 excepting the SFP cage on eth0.
> 
> v2: at Ed's request, merged with his previous pull-request titled
> "x86: Support APU5 in PCEngines platform driver", and some cleanup
> to that changeset (including dropping the table "apu5_driver_data"
> which did not have a defined type "struct apu_driver_data"), but got
> mistitled when the Subject of that commit got accidentally dropped.
> 
> v3: retitled to match Ed's previous pull-request.
> 
> Cc: platform-driver-x86@vger.kernel.org
> Cc: linux-x86_64@vger.kernel.org
> Reviewed-by: Andreas Eberlein <foodeas@aeberlein.de>
> Reviewed-by: Paul Spooren <paul@spooren.de>
> Signed-off-by: Ed Wildgoose <lists@wildgooses.com>
> Sighed-off-by: Philip Prindeville <philipp@redfish-solutions.com>

Hmm, squashing the 2 patches for APU5 and APU6 support together
really is not helpful. this really should be
a series with 2 separate patches.

Also I would really like to have Enrico's input about this,
IIRC last time this was looked at there were some none trivial
compatibility issues...

Regards,

Hans


> ---
>  drivers/leds/leds-apu.c                |   2 +-
>  drivers/platform/x86/Kconfig           |   4 +-
>  drivers/platform/x86/pcengines-apuv2.c | 118 ++++++++++++++++++++++---
>  3 files changed, 107 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/leds/leds-apu.c b/drivers/leds/leds-apu.c
> index c409b80c236d7182c79944225dd4b9e5d32f0a9f..fb3332863c36241e4b7f75f89213171ec0c0dd04 100644
> --- a/drivers/leds/leds-apu.c
> +++ b/drivers/leds/leds-apu.c
> @@ -183,7 +183,7 @@ static int __init apu_led_init(void)
>  
>  	if (!(dmi_match(DMI_SYS_VENDOR, "PC Engines") &&
>  	      (dmi_match(DMI_PRODUCT_NAME, "APU") || dmi_match(DMI_PRODUCT_NAME, "apu1")))) {
> -		pr_err("No PC Engines APUv1 board detected. For APUv2,3 support, enable CONFIG_PCENGINES_APU2\n");
> +		pr_err("No PC Engines APUv1 board detected. For APUv2,3,4,5,6 support, enable CONFIG_PCENGINES_APU2\n");
>  		return -ENODEV;
>  	}
>  
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 5692385e2d26fed92785488ce1c6e88449c54c78..f930d5dd3efee11fbad4ad9508eb288bb4181bab 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -698,7 +698,7 @@ config XO1_RFKILL
>  	  laptop.
>  
>  config PCENGINES_APU2
> -	tristate "PC Engines APUv2/3 front button and LEDs driver"
> +	tristate "PC Engines APUv2/3/4/5/6 front button and LEDs driver"
>  	depends on INPUT && INPUT_KEYBOARD && GPIOLIB
>  	depends on LEDS_CLASS
>  	select GPIO_AMD_FCH
> @@ -706,7 +706,7 @@ config PCENGINES_APU2
>  	select LEDS_GPIO
>  	help
>  	  This driver provides support for the front button and LEDs on
> -	  PC Engines APUv2/APUv3 board.
> +	  PC Engines APUv2/APUv3/APUv4/APUv5/APUv6 board.
>  
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called pcengines-apuv2.
> diff --git a/drivers/platform/x86/pcengines-apuv2.c b/drivers/platform/x86/pcengines-apuv2.c
> index d063d91db9bcbe5ceb2ac641d3105df37651ac4d..bd58d5294f0146088d435f1c9df83eb218b67b0c 100644
> --- a/drivers/platform/x86/pcengines-apuv2.c
> +++ b/drivers/platform/x86/pcengines-apuv2.c
> @@ -1,10 +1,12 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  
>  /*
> - * PC-Engines APUv2/APUv3 board platform driver
> + * PC-Engines APUv2-6 board platform driver
>   * for GPIO buttons and LEDs
>   *
>   * Copyright (C) 2018 metux IT consult
> + * Copyright (C) 2022 Ed Wildgoose <lists@wildgooses.com>
> + * Copyright (C) 2022 Philip Prindeville <philipp@redfish-solutions.com>
>   * Author: Enrico Weigelt <info@metux.net>
>   */
>  
> @@ -22,38 +24,70 @@
>  #include <linux/platform_data/gpio/gpio-amd-fch.h>
>  
>  /*
> - * NOTE: this driver only supports APUv2/3 - not APUv1, as this one
> + * NOTE: this driver only supports APUv2-6 - not APUv1, as this one
>   * has completely different register layouts.
>   */
>  
> +/*
> + * There are a number of APU variants, with differing features
> + * APU2 has SIM slots 1/2 mapping to mPCIe sockets 1/2
> + * APU3/4 moved SIM slot 1 to mPCIe socket 3, ie logically reversed
> + * However, most APU3/4 have a SIM switch which we default on to reverse
> + * the order and keep physical SIM order matching physical modem order
> + * APU6 is approximately the same as APU4 with different ethernet layout
> + *
> + * APU5 has 3x SIM sockets, all with a SIM switch
> + * several GPIOs are shuffled (see schematic), including MODESW
> + */
> +
>  /* Register mappings */
>  #define APU2_GPIO_REG_LED1		AMD_FCH_GPIO_REG_GPIO57
>  #define APU2_GPIO_REG_LED2		AMD_FCH_GPIO_REG_GPIO58
>  #define APU2_GPIO_REG_LED3		AMD_FCH_GPIO_REG_GPIO59_DEVSLP1
>  #define APU2_GPIO_REG_MODESW		AMD_FCH_GPIO_REG_GPIO32_GE1
>  #define APU2_GPIO_REG_SIMSWAP		AMD_FCH_GPIO_REG_GPIO33_GE2
> -#define APU2_GPIO_REG_MPCIE2		AMD_FCH_GPIO_REG_GPIO55_DEVSLP0
> -#define APU2_GPIO_REG_MPCIE3		AMD_FCH_GPIO_REG_GPIO51
> +#define APU2_GPIO_REG_RESETM1		AMD_FCH_GPIO_REG_GPIO51
> +#define APU2_GPIO_REG_RESETM2		AMD_FCH_GPIO_REG_GPIO55_DEVSLP0
> +
> +#define APU5_GPIO_REG_MODESW		AMT_FCH_GPIO_REG_GEVT22
> +#define APU5_GPIO_REG_SIMSWAP1		AMD_FCH_GPIO_REG_GPIO68
> +#define APU5_GPIO_REG_SIMSWAP2		AMD_FCH_GPIO_REG_GPIO32_GE1
> +#define APU5_GPIO_REG_SIMSWAP3		AMD_FCH_GPIO_REG_GPIO33_GE2
> +#define APU5_GPIO_REG_RESETM1		AMD_FCH_GPIO_REG_GPIO51
> +#define APU5_GPIO_REG_RESETM2		AMD_FCH_GPIO_REG_GPIO55_DEVSLP0
> +#define APU5_GPIO_REG_RESETM3		AMD_FCH_GPIO_REG_GPIO64
>  
>  /* Order in which the GPIO lines are defined in the register list */
>  #define APU2_GPIO_LINE_LED1		0
>  #define APU2_GPIO_LINE_LED2		1
>  #define APU2_GPIO_LINE_LED3		2
>  #define APU2_GPIO_LINE_MODESW		3
> -#define APU2_GPIO_LINE_SIMSWAP		4
> -#define APU2_GPIO_LINE_MPCIE2		5
> -#define APU2_GPIO_LINE_MPCIE3		6
> +#define APU2_GPIO_LINE_RESETM1		4
> +#define APU2_GPIO_LINE_RESETM2		5
> +#define APU2_GPIO_LINE_SIMSWAP		6
> +
> +#define APU5_GPIO_LINE_LED1		0
> +#define APU5_GPIO_LINE_LED2		1
> +#define APU5_GPIO_LINE_LED3		2
> +#define APU5_GPIO_LINE_MODESW		3
> +#define APU5_GPIO_LINE_RESETM1		4
> +#define APU5_GPIO_LINE_RESETM2		5
> +#define APU5_GPIO_LINE_RESETM3		6
> +#define APU5_GPIO_LINE_SIMSWAP1		7
> +#define APU5_GPIO_LINE_SIMSWAP2		8
> +#define APU5_GPIO_LINE_SIMSWAP3		9
>  
> -/* GPIO device */
> +
> +/* GPIO device - APU2/3/4/6 */
>  
>  static int apu2_gpio_regs[] = {
>  	[APU2_GPIO_LINE_LED1]		= APU2_GPIO_REG_LED1,
>  	[APU2_GPIO_LINE_LED2]		= APU2_GPIO_REG_LED2,
>  	[APU2_GPIO_LINE_LED3]		= APU2_GPIO_REG_LED3,
>  	[APU2_GPIO_LINE_MODESW]		= APU2_GPIO_REG_MODESW,
> +	[APU2_GPIO_LINE_RESETM1]	= APU2_GPIO_REG_RESETM1,
> +	[APU2_GPIO_LINE_RESETM2]	= APU2_GPIO_REG_RESETM2,
>  	[APU2_GPIO_LINE_SIMSWAP]	= APU2_GPIO_REG_SIMSWAP,
> -	[APU2_GPIO_LINE_MPCIE2]		= APU2_GPIO_REG_MPCIE2,
> -	[APU2_GPIO_LINE_MPCIE3]		= APU2_GPIO_REG_MPCIE3,
>  };
>  
>  static const char * const apu2_gpio_names[] = {
> @@ -61,9 +95,9 @@ static const char * const apu2_gpio_names[] = {
>  	[APU2_GPIO_LINE_LED2]		= "front-led2",
>  	[APU2_GPIO_LINE_LED3]		= "front-led3",
>  	[APU2_GPIO_LINE_MODESW]		= "front-button",
> +	[APU2_GPIO_LINE_RESETM1]	= "modem1-reset",
> +	[APU2_GPIO_LINE_RESETM2]	= "modem2-reset",
>  	[APU2_GPIO_LINE_SIMSWAP]	= "simswap",
> -	[APU2_GPIO_LINE_MPCIE2]		= "mpcie2_reset",
> -	[APU2_GPIO_LINE_MPCIE3]		= "mpcie3_reset",
>  };
>  
>  static const struct amd_fch_gpio_pdata board_apu2 = {
> @@ -72,6 +106,40 @@ static const struct amd_fch_gpio_pdata board_apu2 = {
>  	.gpio_names	= apu2_gpio_names,
>  };
>  
> +/* GPIO device - APU5 */
> +
> +static int apu5_gpio_regs[] = {
> +	[APU5_GPIO_LINE_LED1]		= APU2_GPIO_REG_LED1,
> +	[APU5_GPIO_LINE_LED2]		= APU2_GPIO_REG_LED2,
> +	[APU5_GPIO_LINE_LED3]		= APU2_GPIO_REG_LED3,
> +	[APU5_GPIO_LINE_MODESW]		= APU5_GPIO_REG_MODESW,
> +	[APU5_GPIO_LINE_RESETM1]	= APU5_GPIO_REG_RESETM1,
> +	[APU5_GPIO_LINE_RESETM2]	= APU5_GPIO_REG_RESETM2,
> +	[APU5_GPIO_LINE_RESETM3]	= APU5_GPIO_REG_RESETM3,
> +	[APU5_GPIO_LINE_SIMSWAP1]	= APU5_GPIO_REG_SIMSWAP1,
> +	[APU5_GPIO_LINE_SIMSWAP2]	= APU5_GPIO_REG_SIMSWAP2,
> +	[APU5_GPIO_LINE_SIMSWAP3]	= APU5_GPIO_REG_SIMSWAP3,
> +};
> +
> +static const char * const apu5_gpio_names[] = {
> +	[APU5_GPIO_LINE_LED1]		= "front-led1",
> +	[APU5_GPIO_LINE_LED2]		= "front-led2",
> +	[APU5_GPIO_LINE_LED3]		= "front-led3",
> +	[APU5_GPIO_LINE_MODESW]		= "front-button",
> +	[APU5_GPIO_LINE_RESETM1]	= "modem1-reset",
> +	[APU5_GPIO_LINE_RESETM2]	= "modem2-reset",
> +	[APU5_GPIO_LINE_RESETM3]	= "modem3-reset",
> +	[APU5_GPIO_LINE_SIMSWAP1]	= "simswap1",
> +	[APU5_GPIO_LINE_SIMSWAP2]	= "simswap2",
> +	[APU5_GPIO_LINE_SIMSWAP3]	= "simswap3",
> +};
> +
> +static const struct amd_fch_gpio_pdata board_apu5 = {
> +	.gpio_num	= ARRAY_SIZE(apu5_gpio_regs),
> +	.gpio_reg	= apu5_gpio_regs,
> +	.gpio_names	= apu5_gpio_names,
> +};
> +
>  /* GPIO LEDs device */
>  
>  static const struct gpio_led apu2_leds[] = {
> @@ -215,6 +283,24 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
>  		},
>  		.driver_data = (void *)&board_apu2,
>  	},
> +	/* APU5 w/ mainline BIOS */
> +	{
> +		.ident		= "apu5",
> +		.matches	= {
> +			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
> +			DMI_MATCH(DMI_BOARD_NAME, "apu5")
> +		},
> +		.driver_data	= (void *)&board_apu5,
> +	},
> +	/* APU6 w/ mainline BIOS */
> +	{
> +		.ident		= "apu6",
> +		.matches	= {
> +			DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
> +			DMI_MATCH(DMI_BOARD_NAME, "apu6")
> +		},
> +		.driver_data	= (void *)&board_apu2,
> +	},
>  	{}
>  };
>  
> @@ -249,7 +335,7 @@ static int __init apu_board_init(void)
>  
>  	id = dmi_first_match(apu_gpio_dmi_table);
>  	if (!id) {
> -		pr_err("failed to detect APU board via DMI\n");
> +		pr_err("No APU board detected via DMI\n");
>  		return -ENODEV;
>  	}
>  
> @@ -288,8 +374,12 @@ module_init(apu_board_init);
>  module_exit(apu_board_exit);
>  
>  MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
> -MODULE_DESCRIPTION("PC Engines APUv2/APUv3 board GPIO/LEDs/keys driver");
> +MODULE_DESCRIPTION("PC Engines APUv2-6 board GPIO/LEDs/keys driver");
>  MODULE_LICENSE("GPL");
>  MODULE_DEVICE_TABLE(dmi, apu_gpio_dmi_table);
>  MODULE_ALIAS("platform:pcengines-apuv2");
> +MODULE_ALIAS("platform:pcengines-apuv3");
> +MODULE_ALIAS("platform:pcengines-apuv4");
> +MODULE_ALIAS("platform:pcengines-apuv5");
> +MODULE_ALIAS("platform:pcengines-apuv6");
>  MODULE_SOFTDEP("pre: platform:" AMD_FCH_GPIO_DRIVER_NAME " platform:leds-gpio platform:gpio_keys_polled");

Re: [PATCH v3 1/1] x86: Support APU5 in PCEngines platform driver

[Philip Prindeville]

> On Jan 12, 2023, at 11:57 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> 
> Hi,
> 
> On 1/2/23 07:54, Philip Prindeville wrote:
>> From: Philip Prindeville <philipp@redfish-solutions.com>
>> 
>> PCEngines make a number of SBC. APU5 has 5 mpcie slots + MSATA.
>> It also has support for 3x LTE modems with 6x SIM slots (pairs with a
>> SIM switch device). Each mpcie slot for modems has a reset GPIO
>> 
>> To ensure that the naming is sane between APU2-6 the GPIOS are
>> renamed to be modem1-reset, modem2-reset, etc. This is significant
>> because the slots that can be reset change between APU2 and APU3/4
>> 
>> GPIO for simswap is moved to the end of the list as it could be dropped
>> for APU2 boards (but causes no harm to leave it in, hardware could be
>> added to a future rev of the board).
>> 
>> Structure of the GPIOs for APU5 is extremely similar to APU2-4, but
>> many lines are moved around and there are simply more
>> modems/resets/sim-swap lines to breakout.
>> 
>> Also added APU6, which is essentially APU4 with a different ethernet
>> interface and SFP cage on eth0.
>> 
>> Revision history:
>> 
>> v1: originally titled, "apu6: add apu6 variation to apu2 driver family"
>> this dealt only with detecting the APUv6, which is otherwise identical
>> to the v4 excepting the SFP cage on eth0.
>> 
>> v2: at Ed's request, merged with his previous pull-request titled
>> "x86: Support APU5 in PCEngines platform driver", and some cleanup
>> to that changeset (including dropping the table "apu5_driver_data"
>> which did not have a defined type "struct apu_driver_data"), but got
>> mistitled when the Subject of that commit got accidentally dropped.
>> 
>> v3: retitled to match Ed's previous pull-request.
>> 
>> Cc: platform-driver-x86@vger.kernel.org
>> Cc: linux-x86_64@vger.kernel.org
>> Reviewed-by: Andreas Eberlein <foodeas@aeberlein.de>
>> Reviewed-by: Paul Spooren <paul@spooren.de>
>> Signed-off-by: Ed Wildgoose <lists@wildgooses.com>
>> Sighed-off-by: Philip Prindeville <philipp@redfish-solutions.com>
> 
> Hmm, squashing the 2 patches for APU5 and APU6 support together
> really is not helpful. this really should be
> a series with 2 separate patches.
> 
> Also I would really like to have Enrico's input about this,
> IIRC last time this was looked at there were some none trivial
> compatibility issues...
> 
> Regards,
> 
> Hans


My read of Enrico's comments were that using ACPI information to map
the GPIO lines would break backward compatibility.  This part of the
effort was dropped.

Are there other concerns?

-Philip


> 
> 
>> ---
>> drivers/leds/leds-apu.c                |   2 +-
>> drivers/platform/x86/Kconfig           |   4 +-
>> drivers/platform/x86/pcengines-apuv2.c | 118 ++++++++++++++++++++++---
>> 3 files changed, 107 insertions(+), 17 deletions(-)
>> 
>> diff --git a/drivers/leds/leds-apu.c b/drivers/leds/leds-apu.c
>> index c409b80c236d7182c79944225dd4b9e5d32f0a9f..fb3332863c36241e4b7f75f89213171ec0c0dd04 100644
>> --- a/drivers/leds/leds-apu.c
>> +++ b/drivers/leds/leds-apu.c
>> @@ -183,7 +183,7 @@ static int __init apu_led_init(void)
>> 
>> if (!(dmi_match(DMI_SYS_VENDOR, "PC Engines") &&
>>       (dmi_match(DMI_PRODUCT_NAME, "APU") || dmi_match(DMI_PRODUCT_NAME, "apu1")))) {
>> - pr_err("No PC Engines APUv1 board detected. For APUv2,3 support, enable CONFIG_PCENGINES_APU2\n");
>> + pr_err("No PC Engines APUv1 board detected. For APUv2,3,4,5,6 support, enable CONFIG_PCENGINES_APU2\n");
>> return -ENODEV;
>> }
>> 
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 5692385e2d26fed92785488ce1c6e88449c54c78..f930d5dd3efee11fbad4ad9508eb288bb4181bab 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -698,7 +698,7 @@ config XO1_RFKILL
>>   laptop.
>> 
>> config PCENGINES_APU2
>> - tristate "PC Engines APUv2/3 front button and LEDs driver"
>> + tristate "PC Engines APUv2/3/4/5/6 front button and LEDs driver"
>> depends on INPUT && INPUT_KEYBOARD && GPIOLIB
>> depends on LEDS_CLASS
>> select GPIO_AMD_FCH
>> @@ -706,7 +706,7 @@ config PCENGINES_APU2
>> select LEDS_GPIO
>> help
>>   This driver provides support for the front button and LEDs on
>> -   PC Engines APUv2/APUv3 board.
>> +   PC Engines APUv2/APUv3/APUv4/APUv5/APUv6 board.
>> 
>>   To compile this driver as a module, choose M here: the module
>>   will be called pcengines-apuv2.
>> diff --git a/drivers/platform/x86/pcengines-apuv2.c b/drivers/platform/x86/pcengines-apuv2.c
>> index d063d91db9bcbe5ceb2ac641d3105df37651ac4d..bd58d5294f0146088d435f1c9df83eb218b67b0c 100644
>> --- a/drivers/platform/x86/pcengines-apuv2.c
>> +++ b/drivers/platform/x86/pcengines-apuv2.c
>> @@ -1,10 +1,12 @@
>> // SPDX-License-Identifier: GPL-2.0+
>> 
>> /*
>> - * PC-Engines APUv2/APUv3 board platform driver
>> + * PC-Engines APUv2-6 board platform driver
>>  * for GPIO buttons and LEDs
>>  *
>>  * Copyright (C) 2018 metux IT consult
>> + * Copyright (C) 2022 Ed Wildgoose <lists@wildgooses.com>
>> + * Copyright (C) 2022 Philip Prindeville <philipp@redfish-solutions.com>
>>  * Author: Enrico Weigelt <info@metux.net>
>>  */
>> 
>> @@ -22,38 +24,70 @@
>> #include <linux/platform_data/gpio/gpio-amd-fch.h>
>> 
>> /*
>> - * NOTE: this driver only supports APUv2/3 - not APUv1, as this one
>> + * NOTE: this driver only supports APUv2-6 - not APUv1, as this one
>>  * has completely different register layouts.
>>  */
>> 
>> +/*
>> + * There are a number of APU variants, with differing features
>> + * APU2 has SIM slots 1/2 mapping to mPCIe sockets 1/2
>> + * APU3/4 moved SIM slot 1 to mPCIe socket 3, ie logically reversed
>> + * However, most APU3/4 have a SIM switch which we default on to reverse
>> + * the order and keep physical SIM order matching physical modem order
>> + * APU6 is approximately the same as APU4 with different ethernet layout
>> + *
>> + * APU5 has 3x SIM sockets, all with a SIM switch
>> + * several GPIOs are shuffled (see schematic), including MODESW
>> + */
>> +
>> /* Register mappings */
>> #define APU2_GPIO_REG_LED1 AMD_FCH_GPIO_REG_GPIO57
>> #define APU2_GPIO_REG_LED2 AMD_FCH_GPIO_REG_GPIO58
>> #define APU2_GPIO_REG_LED3 AMD_FCH_GPIO_REG_GPIO59_DEVSLP1
>> #define APU2_GPIO_REG_MODESW AMD_FCH_GPIO_REG_GPIO32_GE1
>> #define APU2_GPIO_REG_SIMSWAP AMD_FCH_GPIO_REG_GPIO33_GE2
>> -#define APU2_GPIO_REG_MPCIE2 AMD_FCH_GPIO_REG_GPIO55_DEVSLP0
>> -#define APU2_GPIO_REG_MPCIE3 AMD_FCH_GPIO_REG_GPIO51
>> +#define APU2_GPIO_REG_RESETM1 AMD_FCH_GPIO_REG_GPIO51
>> +#define APU2_GPIO_REG_RESETM2 AMD_FCH_GPIO_REG_GPIO55_DEVSLP0
>> +
>> +#define APU5_GPIO_REG_MODESW AMT_FCH_GPIO_REG_GEVT22
>> +#define APU5_GPIO_REG_SIMSWAP1 AMD_FCH_GPIO_REG_GPIO68
>> +#define APU5_GPIO_REG_SIMSWAP2 AMD_FCH_GPIO_REG_GPIO32_GE1
>> +#define APU5_GPIO_REG_SIMSWAP3 AMD_FCH_GPIO_REG_GPIO33_GE2
>> +#define APU5_GPIO_REG_RESETM1 AMD_FCH_GPIO_REG_GPIO51
>> +#define APU5_GPIO_REG_RESETM2 AMD_FCH_GPIO_REG_GPIO55_DEVSLP0
>> +#define APU5_GPIO_REG_RESETM3 AMD_FCH_GPIO_REG_GPIO64
>> 
>> /* Order in which the GPIO lines are defined in the register list */
>> #define APU2_GPIO_LINE_LED1 0
>> #define APU2_GPIO_LINE_LED2 1
>> #define APU2_GPIO_LINE_LED3 2
>> #define APU2_GPIO_LINE_MODESW 3
>> -#define APU2_GPIO_LINE_SIMSWAP 4
>> -#define APU2_GPIO_LINE_MPCIE2 5
>> -#define APU2_GPIO_LINE_MPCIE3 6
>> +#define APU2_GPIO_LINE_RESETM1 4
>> +#define APU2_GPIO_LINE_RESETM2 5
>> +#define APU2_GPIO_LINE_SIMSWAP 6
>> +
>> +#define APU5_GPIO_LINE_LED1 0
>> +#define APU5_GPIO_LINE_LED2 1
>> +#define APU5_GPIO_LINE_LED3 2
>> +#define APU5_GPIO_LINE_MODESW 3
>> +#define APU5_GPIO_LINE_RESETM1 4
>> +#define APU5_GPIO_LINE_RESETM2 5
>> +#define APU5_GPIO_LINE_RESETM3 6
>> +#define APU5_GPIO_LINE_SIMSWAP1 7
>> +#define APU5_GPIO_LINE_SIMSWAP2 8
>> +#define APU5_GPIO_LINE_SIMSWAP3 9
>> 
>> -/* GPIO device */
>> +
>> +/* GPIO device - APU2/3/4/6 */
>> 
>> static int apu2_gpio_regs[] = {
>> [APU2_GPIO_LINE_LED1] = APU2_GPIO_REG_LED1,
>> [APU2_GPIO_LINE_LED2] = APU2_GPIO_REG_LED2,
>> [APU2_GPIO_LINE_LED3] = APU2_GPIO_REG_LED3,
>> [APU2_GPIO_LINE_MODESW] = APU2_GPIO_REG_MODESW,
>> + [APU2_GPIO_LINE_RESETM1] = APU2_GPIO_REG_RESETM1,
>> + [APU2_GPIO_LINE_RESETM2] = APU2_GPIO_REG_RESETM2,
>> [APU2_GPIO_LINE_SIMSWAP] = APU2_GPIO_REG_SIMSWAP,
>> - [APU2_GPIO_LINE_MPCIE2] = APU2_GPIO_REG_MPCIE2,
>> - [APU2_GPIO_LINE_MPCIE3] = APU2_GPIO_REG_MPCIE3,
>> };
>> 
>> static const char * const apu2_gpio_names[] = {
>> @@ -61,9 +95,9 @@ static const char * const apu2_gpio_names[] = {
>> [APU2_GPIO_LINE_LED2] = "front-led2",
>> [APU2_GPIO_LINE_LED3] = "front-led3",
>> [APU2_GPIO_LINE_MODESW] = "front-button",
>> + [APU2_GPIO_LINE_RESETM1] = "modem1-reset",
>> + [APU2_GPIO_LINE_RESETM2] = "modem2-reset",
>> [APU2_GPIO_LINE_SIMSWAP] = "simswap",
>> - [APU2_GPIO_LINE_MPCIE2] = "mpcie2_reset",
>> - [APU2_GPIO_LINE_MPCIE3] = "mpcie3_reset",
>> };
>> 
>> static const struct amd_fch_gpio_pdata board_apu2 = {
>> @@ -72,6 +106,40 @@ static const struct amd_fch_gpio_pdata board_apu2 = {
>> .gpio_names = apu2_gpio_names,
>> };
>> 
>> +/* GPIO device - APU5 */
>> +
>> +static int apu5_gpio_regs[] = {
>> + [APU5_GPIO_LINE_LED1] = APU2_GPIO_REG_LED1,
>> + [APU5_GPIO_LINE_LED2] = APU2_GPIO_REG_LED2,
>> + [APU5_GPIO_LINE_LED3] = APU2_GPIO_REG_LED3,
>> + [APU5_GPIO_LINE_MODESW] = APU5_GPIO_REG_MODESW,
>> + [APU5_GPIO_LINE_RESETM1] = APU5_GPIO_REG_RESETM1,
>> + [APU5_GPIO_LINE_RESETM2] = APU5_GPIO_REG_RESETM2,
>> + [APU5_GPIO_LINE_RESETM3] = APU5_GPIO_REG_RESETM3,
>> + [APU5_GPIO_LINE_SIMSWAP1] = APU5_GPIO_REG_SIMSWAP1,
>> + [APU5_GPIO_LINE_SIMSWAP2] = APU5_GPIO_REG_SIMSWAP2,
>> + [APU5_GPIO_LINE_SIMSWAP3] = APU5_GPIO_REG_SIMSWAP3,
>> +};
>> +
>> +static const char * const apu5_gpio_names[] = {
>> + [APU5_GPIO_LINE_LED1] = "front-led1",
>> + [APU5_GPIO_LINE_LED2] = "front-led2",
>> + [APU5_GPIO_LINE_LED3] = "front-led3",
>> + [APU5_GPIO_LINE_MODESW] = "front-button",
>> + [APU5_GPIO_LINE_RESETM1] = "modem1-reset",
>> + [APU5_GPIO_LINE_RESETM2] = "modem2-reset",
>> + [APU5_GPIO_LINE_RESETM3] = "modem3-reset",
>> + [APU5_GPIO_LINE_SIMSWAP1] = "simswap1",
>> + [APU5_GPIO_LINE_SIMSWAP2] = "simswap2",
>> + [APU5_GPIO_LINE_SIMSWAP3] = "simswap3",
>> +};
>> +
>> +static const struct amd_fch_gpio_pdata board_apu5 = {
>> + .gpio_num = ARRAY_SIZE(apu5_gpio_regs),
>> + .gpio_reg = apu5_gpio_regs,
>> + .gpio_names = apu5_gpio_names,
>> +};
>> +
>> /* GPIO LEDs device */
>> 
>> static const struct gpio_led apu2_leds[] = {
>> @@ -215,6 +283,24 @@ static const struct dmi_system_id apu_gpio_dmi_table[] __initconst = {
>> },
>> .driver_data = (void *)&board_apu2,
>> },
>> + /* APU5 w/ mainline BIOS */
>> + {
>> + .ident = "apu5",
>> + .matches = {
>> + DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
>> + DMI_MATCH(DMI_BOARD_NAME, "apu5")
>> + },
>> + .driver_data = (void *)&board_apu5,
>> + },
>> + /* APU6 w/ mainline BIOS */
>> + {
>> + .ident = "apu6",
>> + .matches = {
>> + DMI_MATCH(DMI_SYS_VENDOR, "PC Engines"),
>> + DMI_MATCH(DMI_BOARD_NAME, "apu6")
>> + },
>> + .driver_data = (void *)&board_apu2,
>> + },
>> {}
>> };
>> 
>> @@ -249,7 +335,7 @@ static int __init apu_board_init(void)
>> 
>> id = dmi_first_match(apu_gpio_dmi_table);
>> if (!id) {
>> - pr_err("failed to detect APU board via DMI\n");
>> + pr_err("No APU board detected via DMI\n");
>> return -ENODEV;
>> }
>> 
>> @@ -288,8 +374,12 @@ module_init(apu_board_init);
>> module_exit(apu_board_exit);
>> 
>> MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>");
>> -MODULE_DESCRIPTION("PC Engines APUv2/APUv3 board GPIO/LEDs/keys driver");
>> +MODULE_DESCRIPTION("PC Engines APUv2-6 board GPIO/LEDs/keys driver");
>> MODULE_LICENSE("GPL");
>> MODULE_DEVICE_TABLE(dmi, apu_gpio_dmi_table);
>> MODULE_ALIAS("platform:pcengines-apuv2");
>> +MODULE_ALIAS("platform:pcengines-apuv3");
>> +MODULE_ALIAS("platform:pcengines-apuv4");
>> +MODULE_ALIAS("platform:pcengines-apuv5");
>> +MODULE_ALIAS("platform:pcengines-apuv6");
>> MODULE_SOFTDEP("pre: platform:" AMD_FCH_GPIO_DRIVER_NAME " platform:leds-gpio platform:gpio_keys_polled");

Re: [PATCH v3 1/1] x86: Support APU5 in PCEngines platform driver

[Philip Prindeville]

[-- Attachment #1: Type: text/plain, Size: 239 bytes --]

Hi,

I wanted to get the documentation straight from the proverbial horse's mouth, before I added any confusion of my own to the conversation.  I reached out to Pascal and he was good enough to share this document with me.

-Philip


[-- Attachment #2: apugpio.xls --]
[-- Type: application/vnd.ms-excel, Size: 28160 bytes --]

[-- Attachment #3: Type: text/plain, Size: 1793 bytes --]




> On Feb 17, 2023, at 5:20 AM, Enrico Weigelt, metux IT consult <lkml@metux.net> wrote:
> 
> On 14.01.23 00:04, Philip Prindeville wrote:
> 
> Hello friends,
> 
> sorry for being so late, busy with totally different things ...
> 
>> My read of Enrico's comments were that using ACPI information to map
>> the GPIO lines would break backward compatibility.  This part of the
>> effort was dropped.
> Yes, the big problem is inconsistent support in different firmware versions in the field. Older version generally don't have any acpi
> entries at all, later added it (but inconsitent and incomplete) and was
> dropped again later (haven't checked whether they reintroduced it
> again).
> 
> Obviously, we can't expect users in the field to upgrade firmware and
> kernel in lockstep. So, we can only rely on this data for those boards
> where we can be sure that all shipped firmware versions have proper
> support (that really does it right). The problem also goes a bit deeper:
> just adding the GPIOs isn't really enough, they need proper (and
> consistent) names as well as mapping to the correct drivers (eg. LEDs).
> 
> Oh, BTW, don't arbitrarily change gpio line names (at least for the
> already mainline-supported boards) - they're are used in the field.
> (well, I'm not actually satisfied with direct gpio access or things
> like modem reset lines, but haven't seen an actually fitting subsys
> for those).
> 
> 
> --mtx
> 
> -- 
> ---
> Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
> werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
> GPG/PGP-Schlüssel zu.
> ---
> Enrico Weigelt, metux IT consult
> Free software and Linux embedded engineering
> info@metux.net -- +49-151-27565287

Re: [PATCH v3 1/1] x86: Support APU5 in PCEngines platform driver

[Ed W]

Excellent! I was just trying to pull together something similar based on the individual schematic
documents!

I think this supports the proposal on the table already, that we should prefer naming to be "modem
orientated", rather than "pcie slot" orientated.

For example, on APU2, the PE3/4_RST lines are wired to PCIe slots 1 & 2 (which are the two with USB)
But on APU4, the PE3_RST reset, USB and SIM lines move from slot 1 to slot 3.

So on APU4, you don't get the control over wireless disable (and reset is hazy) on the mpcie slot 1
for the wifi card. But in all cases the use of the reset/enable lines follow the modem slots, not
the wifi slots

So I maintain my proposal that it's far better to name the GPIOs relative to the USB & modem slots,
since this is how they are being used on ALL schematics. Especially on APU5 (which is the oddball,
having 3x modems + 6x SIMs), this is very much the case

(Also, Enrico, you should beware that your current use might not be working as you expect, because I
don't see that you have control over the wifi card enable on APU4 at all?)

If we are now in agreement, perhaps we can proceed? I think if we make progress here, then I might
also send in a patch to wire up all the other GPIOs.

Thanks all

Ed W


On 27/02/2023 00:22, Philip Prindeville wrote:
> Hi,
>
> I wanted to get the documentation straight from the proverbial horse's mouth, before I added any confusion of my own to the conversation.  I reached out to Pascal and he was good enough to share this document with me.
>
> -Philip
>
>
>
>
>> On Feb 17, 2023, at 5:20 AM, Enrico Weigelt, metux IT consult <lkml@metux.net> wrote:
>>
>> On 14.01.23 00:04, Philip Prindeville wrote:
>>
>> Hello friends,
>>
>> sorry for being so late, busy with totally different things ...
>>
>>> My read of Enrico's comments were that using ACPI information to map
>>> the GPIO lines would break backward compatibility.  This part of the
>>> effort was dropped.
>> Yes, the big problem is inconsistent support in different firmware versions in the field. Older version generally don't have any acpi
>> entries at all, later added it (but inconsitent and incomplete) and was
>> dropped again later (haven't checked whether they reintroduced it
>> again).
>>
>> Obviously, we can't expect users in the field to upgrade firmware and
>> kernel in lockstep. So, we can only rely on this data for those boards
>> where we can be sure that all shipped firmware versions have proper
>> support (that really does it right). The problem also goes a bit deeper:
>> just adding the GPIOs isn't really enough, they need proper (and
>> consistent) names as well as mapping to the correct drivers (eg. LEDs).
>>
>> Oh, BTW, don't arbitrarily change gpio line names (at least for the
>> already mainline-supported boards) - they're are used in the field.
>> (well, I'm not actually satisfied with direct gpio access or things
>> like modem reset lines, but haven't seen an actually fitting subsys
>> for those).
>>
>>
>> --mtx
>>
>> -- 
>> ---
>> Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert
>> werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren
>> GPG/PGP-Schlüssel zu.
>> ---
>> Enrico Weigelt, metux IT consult
>> Free software and Linux embedded engineering
>> info@metux.net -- +49-151-27565287