[PATCH 0/3] platform/x86: simatic-ipc: add another model BX-56A/BX-59A

[PATCH 0/3] platform/x86: simatic-ipc: add another model BX-56A/BX-59A

[xingtong_wu]

From: "xingtong.wu" <xingtong.wu@siemens.com>

These are patches adding features for another Simatic IPC model
BX-56A/BX-59A, features for led and battery monitor are involved.

Based on:
 [PATCH v2 0/3] platform/x86: move simatic drivers into subdir
 [PATCH v2 0/2] leds: simatic-ipc-leds-gpio: add new model BX-21A
 [PATCH 0/2] platform/x86/siemens: fixes for CMOS battery hwmon

xingtong.wu (3):
  platform/x86: simatic-ipc: add new models BX-56A/BX-59A
  leds: simatic-ipc-leds-gpio: add support for module BX-59A
  batt: simatic-ipc-batt: add support for module BX-59A

 .../leds/simple/simatic-ipc-leds-gpio-core.c  |  1 +
 .../simple/simatic-ipc-leds-gpio-f7188x.c     | 42 ++++++++++++++++---
 drivers/platform/x86/siemens/Kconfig          |  1 +
 .../x86/siemens/simatic-ipc-batt-f7188x.c     | 37 +++++++++++-----
 .../platform/x86/siemens/simatic-ipc-batt.c   |  3 +-
 drivers/platform/x86/siemens/simatic-ipc.c    | 12 +++++-
 .../platform_data/x86/simatic-ipc-base.h      |  1 +
 include/linux/platform_data/x86/simatic-ipc.h |  2 +
 8 files changed, 80 insertions(+), 19 deletions(-)

-- 
2.25.1

[PATCH 1/3] platform/x86: simatic-ipc: add new models BX-56A/BX-59A

[xingtong_wu]

From: "xingtong.wu" <xingtong.wu@siemens.com>

This adds support for the Siemens Simatic IPC models BX-56A/BX-59A,
led/watchdog/battery on these models are same, actual drivers for
models will be sent in separate patches.

Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
---
 drivers/platform/x86/siemens/simatic-ipc.c         | 12 ++++++++++--
 include/linux/platform_data/x86/simatic-ipc-base.h |  1 +
 include/linux/platform_data/x86/simatic-ipc.h      |  2 ++
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/siemens/simatic-ipc.c b/drivers/platform/x86/siemens/simatic-ipc.c
index 13c857316c7f..02c540cf4070 100644
--- a/drivers/platform/x86/siemens/simatic-ipc.c
+++ b/drivers/platform/x86/siemens/simatic-ipc.c
@@ -78,6 +78,12 @@ static struct {
 	{SIMATIC_IPC_IPCBX_21A,
 		SIMATIC_IPC_DEVICE_BX_21A, SIMATIC_IPC_DEVICE_NONE, SIMATIC_IPC_DEVICE_BX_21A,
 		{ "emc1403", NULL }},
+	{SIMATIC_IPC_IPCBX_56A,
+		SIMATIC_IPC_DEVICE_BX_59A, SIMATIC_IPC_DEVICE_NONE, SIMATIC_IPC_DEVICE_BX_59A,
+		{ "emc1403", "w83627hf_wdt" }},
+	{SIMATIC_IPC_IPCBX_59A,
+		SIMATIC_IPC_DEVICE_BX_59A, SIMATIC_IPC_DEVICE_NONE, SIMATIC_IPC_DEVICE_BX_59A,
+		{ "emc1403", "w83627hf_wdt" }},
 };
 
 static int register_platform_devices(u32 station_id)
@@ -103,7 +109,9 @@ static int register_platform_devices(u32 station_id)
 			pdevname = KBUILD_MODNAME "_batt_apollolake";
 		if (battmode == SIMATIC_IPC_DEVICE_BX_21A)
 			pdevname = KBUILD_MODNAME "_batt_elkhartlake";
-		if (battmode == SIMATIC_IPC_DEVICE_227G || battmode == SIMATIC_IPC_DEVICE_BX_39A)
+		if (battmode == SIMATIC_IPC_DEVICE_227G ||
+		    battmode == SIMATIC_IPC_DEVICE_BX_39A ||
+		    battmode == SIMATIC_IPC_DEVICE_BX_59A)
 			pdevname = KBUILD_MODNAME "_batt_f7188x";
 		platform_data.devmode = battmode;
 		ipc_batt_platform_device =
@@ -121,7 +129,7 @@ static int register_platform_devices(u32 station_id)
 		pdevname = KBUILD_MODNAME "_leds";
 		if (ledmode == SIMATIC_IPC_DEVICE_127E)
 			pdevname = KBUILD_MODNAME "_leds_gpio_apollolake";
-		if (ledmode == SIMATIC_IPC_DEVICE_227G)
+		if (ledmode == SIMATIC_IPC_DEVICE_227G || SIMATIC_IPC_DEVICE_BX_59A)
 			pdevname = KBUILD_MODNAME "_leds_gpio_f7188x";
 		if (ledmode == SIMATIC_IPC_DEVICE_BX_21A)
 			pdevname = KBUILD_MODNAME "_leds_gpio_elkhartlake";
diff --git a/include/linux/platform_data/x86/simatic-ipc-base.h b/include/linux/platform_data/x86/simatic-ipc-base.h
index 4ca21065c862..2d7f7120ba6b 100644
--- a/include/linux/platform_data/x86/simatic-ipc-base.h
+++ b/include/linux/platform_data/x86/simatic-ipc-base.h
@@ -22,6 +22,7 @@
 #define SIMATIC_IPC_DEVICE_227G 5
 #define SIMATIC_IPC_DEVICE_BX_21A 6
 #define SIMATIC_IPC_DEVICE_BX_39A 7
+#define SIMATIC_IPC_DEVICE_BX_59A 8
 
 struct simatic_ipc_platform {
 	u8	devmode;
diff --git a/include/linux/platform_data/x86/simatic-ipc.h b/include/linux/platform_data/x86/simatic-ipc.h
index f2eafa43a605..8d8b3b919674 100644
--- a/include/linux/platform_data/x86/simatic-ipc.h
+++ b/include/linux/platform_data/x86/simatic-ipc.h
@@ -36,6 +36,8 @@ enum simatic_ipc_station_ids {
 	SIMATIC_IPC_IPCBX_39A = 0x00001001,
 	SIMATIC_IPC_IPCPX_39A = 0x00001002,
 	SIMATIC_IPC_IPCBX_21A = 0x00001101,
+	SIMATIC_IPC_IPCBX_56A = 0x00001201,
+	SIMATIC_IPC_IPCBX_59A = 0x00001202,
 };
 
 static inline u32 simatic_ipc_get_station_id(u8 *data, int max_len)
-- 
2.25.1

[PATCH 1/2] platform/x86/siemens: simatic-ipc: fix logic error in BX-59A

[xingtong_wu]

From: "xingtong.wu" <xingtong.wu@siemens.com>

There is a bug in if statement that lead to logical error
and have influence to other IPC, it get correct now.

Fixes: b8af77951941 ("platform/x86/siemens: simatic-ipc: add new models BX-56A/BX-59A")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202307312322.Aa8upHWK-lkp@intel.com/
Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
---
 drivers/platform/x86/siemens/simatic-ipc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/siemens/simatic-ipc.c b/drivers/platform/x86/siemens/simatic-ipc.c
index 02c540cf4070..8ca6e277fa03 100644
--- a/drivers/platform/x86/siemens/simatic-ipc.c
+++ b/drivers/platform/x86/siemens/simatic-ipc.c
@@ -129,7 +129,7 @@ static int register_platform_devices(u32 station_id)
 		pdevname = KBUILD_MODNAME "_leds";
 		if (ledmode == SIMATIC_IPC_DEVICE_127E)
 			pdevname = KBUILD_MODNAME "_leds_gpio_apollolake";
-		if (ledmode == SIMATIC_IPC_DEVICE_227G || SIMATIC_IPC_DEVICE_BX_59A)
+		if (ledmode == SIMATIC_IPC_DEVICE_227G || ledmode == SIMATIC_IPC_DEVICE_BX_59A)
 			pdevname = KBUILD_MODNAME "_leds_gpio_f7188x";
 		if (ledmode == SIMATIC_IPC_DEVICE_BX_21A)
 			pdevname = KBUILD_MODNAME "_leds_gpio_elkhartlake";
-- 
2.25.1

Re: [PATCH 1/2] platform/x86/siemens: simatic-ipc: fix logic error in BX-59A

[Hans de Goede]

Hi xingtong,

Thank you for the quick fixes for the issues lkp spotted.

I'll merge these into the pdx86-simatic-ipc branch next time
I do a round of merges (within about a week).

Regards,

Hans





On 8/1/23 11:04, xingtong_wu@163.com wrote:
> From: "xingtong.wu" <xingtong.wu@siemens.com>
> 
> There is a bug in if statement that lead to logical error
> and have influence to other IPC, it get correct now.
> 
> Fixes: b8af77951941 ("platform/x86/siemens: simatic-ipc: add new models BX-56A/BX-59A")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202307312322.Aa8upHWK-lkp@intel.com/
> Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
> ---
>  drivers/platform/x86/siemens/simatic-ipc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/siemens/simatic-ipc.c b/drivers/platform/x86/siemens/simatic-ipc.c
> index 02c540cf4070..8ca6e277fa03 100644
> --- a/drivers/platform/x86/siemens/simatic-ipc.c
> +++ b/drivers/platform/x86/siemens/simatic-ipc.c
> @@ -129,7 +129,7 @@ static int register_platform_devices(u32 station_id)
>  		pdevname = KBUILD_MODNAME "_leds";
>  		if (ledmode == SIMATIC_IPC_DEVICE_127E)
>  			pdevname = KBUILD_MODNAME "_leds_gpio_apollolake";
> -		if (ledmode == SIMATIC_IPC_DEVICE_227G || SIMATIC_IPC_DEVICE_BX_59A)
> +		if (ledmode == SIMATIC_IPC_DEVICE_227G || ledmode == SIMATIC_IPC_DEVICE_BX_59A)
>  			pdevname = KBUILD_MODNAME "_leds_gpio_f7188x";
>  		if (ledmode == SIMATIC_IPC_DEVICE_BX_21A)
>  			pdevname = KBUILD_MODNAME "_leds_gpio_elkhartlake";

Re: [PATCH 1/2] platform/x86/siemens: simatic-ipc: fix logic error in BX-59A

[Ilpo Järvinen]

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

On Tue, 1 Aug 2023, xingtong_wu@163.com wrote:

> From: "xingtong.wu" <xingtong.wu@siemens.com>
> 
> There is a bug in if statement that lead to logical error

Variable is missing from the ledmode if statement that leads to a logic 
error.

> and have influence to other IPC

I don't understand what you're trying to say with this. Not that it feels 
very necessary anyway.

>, it get correct now.

I'd say (in own sentence):

"Add the missing variable to the if condition."

> Fixes: b8af77951941 ("platform/x86/siemens: simatic-ipc: add new models BX-56A/BX-59A")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202307312322.Aa8upHWK-lkp@intel.com/
> Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
> ---
>  drivers/platform/x86/siemens/simatic-ipc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/siemens/simatic-ipc.c b/drivers/platform/x86/siemens/simatic-ipc.c
> index 02c540cf4070..8ca6e277fa03 100644
> --- a/drivers/platform/x86/siemens/simatic-ipc.c
> +++ b/drivers/platform/x86/siemens/simatic-ipc.c
> @@ -129,7 +129,7 @@ static int register_platform_devices(u32 station_id)
>  		pdevname = KBUILD_MODNAME "_leds";
>  		if (ledmode == SIMATIC_IPC_DEVICE_127E)
>  			pdevname = KBUILD_MODNAME "_leds_gpio_apollolake";
> -		if (ledmode == SIMATIC_IPC_DEVICE_227G || SIMATIC_IPC_DEVICE_BX_59A)
> +		if (ledmode == SIMATIC_IPC_DEVICE_227G || ledmode == SIMATIC_IPC_DEVICE_BX_59A)
>  			pdevname = KBUILD_MODNAME "_leds_gpio_f7188x";
>  		if (ledmode == SIMATIC_IPC_DEVICE_BX_21A)
>  			pdevname = KBUILD_MODNAME "_leds_gpio_elkhartlake";
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

[PATCH v2 0/2] fix logic errors in BX-59A

[xingtong_wu]

From: "xingtong.wu" <xingtong.wu@siemens.com>

change since v1:
 - Improve the changelog to make things clear

These are rather simple patches fixing logic errors in Siemens
IPC drivers.

xingtong.wu (2):
  platform/x86/siemens: simatic-ipc: fix logic error in BX-59A
  platform/x86/siemens: simatic-ipc-batt: fix logic error for BX-59A

 drivers/platform/x86/siemens/simatic-ipc-batt.c | 3 ++-
 drivers/platform/x86/siemens/simatic-ipc.c      | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] platform/x86/siemens: simatic-ipc: fix logic error in BX-59A

[xingtong_wu]

From: "xingtong.wu" <xingtong.wu@siemens.com>

The variable "ledmode" is missing from if statement that leads to
a logical error. Add the missing variable to the if condition.

Fixes: b8af77951941 ("platform/x86/siemens: simatic-ipc: add new models BX-56A/BX-59A")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202307312322.Aa8upHWK-lkp@intel.com/

Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
---
 drivers/platform/x86/siemens/simatic-ipc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/siemens/simatic-ipc.c b/drivers/platform/x86/siemens/simatic-ipc.c
index 02c540cf4070..8ca6e277fa03 100644
--- a/drivers/platform/x86/siemens/simatic-ipc.c
+++ b/drivers/platform/x86/siemens/simatic-ipc.c
@@ -129,7 +129,7 @@ static int register_platform_devices(u32 station_id)
 		pdevname = KBUILD_MODNAME "_leds";
 		if (ledmode == SIMATIC_IPC_DEVICE_127E)
 			pdevname = KBUILD_MODNAME "_leds_gpio_apollolake";
-		if (ledmode == SIMATIC_IPC_DEVICE_227G || SIMATIC_IPC_DEVICE_BX_59A)
+		if (ledmode == SIMATIC_IPC_DEVICE_227G || ledmode == SIMATIC_IPC_DEVICE_BX_59A)
 			pdevname = KBUILD_MODNAME "_leds_gpio_f7188x";
 		if (ledmode == SIMATIC_IPC_DEVICE_BX_21A)
 			pdevname = KBUILD_MODNAME "_leds_gpio_elkhartlake";
-- 
2.25.1

Re: [PATCH v2 1/2] platform/x86/siemens: simatic-ipc: fix logic error in BX-59A

[Andy Shevchenko]

On Sat, Aug 01, 2043 at 01:30:26AM +0800, xingtong_wu@163.com wrote:
> From: "xingtong.wu" <xingtong.wu@siemens.com>
> 
> The variable "ledmode" is missing from if statement that leads to
> a logical error. Add the missing variable to the if condition.
> 
> Fixes: b8af77951941 ("platform/x86/siemens: simatic-ipc: add new models BX-56A/BX-59A")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202307312322.Aa8upHWK-lkp@intel.com/

> 

Blank lines are not allowed in the tag block.

> Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 0/2] fix logical errors for BX-59A

[xingtong_wu]

From: "xingtong.wu" <xingtong.wu@siemens.com>

changes since v2:
 - Collect tag "Reviewed-by" in changelog
 - Delete blank line between tags block "Closes" and "Signed-off-by"

changes since v1:
 - Improve the changelog to make things clear

These are rather simple patches fixing logical errors in Siemens
IPC drivers.

xingtong.wu (2):
  platform/x86/siemens: simatic-ipc: fix logical error for BX-59A
  platform/x86/siemens: simatic-ipc-batt: fix logical error for BX-59A

 drivers/platform/x86/siemens/simatic-ipc-batt.c | 3 ++-
 drivers/platform/x86/siemens/simatic-ipc.c      | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

-- 
2.25.1

Re: [PATCH v3 0/2] fix logical errors for BX-59A

[Hans de Goede]

Hi,

On 8/2/43 19:35, xingtong_wu@163.com wrote:
> From: "xingtong.wu" <xingtong.wu@siemens.com>
> 
> changes since v2:
>  - Collect tag "Reviewed-by" in changelog
>  - Delete blank line between tags block "Closes" and "Signed-off-by"
> 
> changes since v1:
>  - Improve the changelog to make things clear
> 
> These are rather simple patches fixing logical errors in Siemens
> IPC drivers.
> 
> xingtong.wu (2):
>   platform/x86/siemens: simatic-ipc: fix logical error for BX-59A
>   platform/x86/siemens: simatic-ipc-batt: fix logical error for BX-59A
> 
>  drivers/platform/x86/siemens/simatic-ipc-batt.c | 3 ++-
>  drivers/platform/x86/siemens/simatic-ipc.c      | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)


Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans

[PATCH v3 1/2] platform/x86/siemens: simatic-ipc: fix logical error for BX-59A

[xingtong_wu]

From: "xingtong.wu" <xingtong.wu@siemens.com>

The variable "ledmode" is missing from if statement that leads to
a logical error. Add the missing variable to the if condition.

Fixes: b8af77951941 ("platform/x86/siemens: simatic-ipc: add new models BX-56A/BX-59A")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202307312322.Aa8upHWK-lkp@intel.com/
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
---
 drivers/platform/x86/siemens/simatic-ipc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/siemens/simatic-ipc.c b/drivers/platform/x86/siemens/simatic-ipc.c
index 02c540cf4070..8ca6e277fa03 100644
--- a/drivers/platform/x86/siemens/simatic-ipc.c
+++ b/drivers/platform/x86/siemens/simatic-ipc.c
@@ -129,7 +129,7 @@ static int register_platform_devices(u32 station_id)
 		pdevname = KBUILD_MODNAME "_leds";
 		if (ledmode == SIMATIC_IPC_DEVICE_127E)
 			pdevname = KBUILD_MODNAME "_leds_gpio_apollolake";
-		if (ledmode == SIMATIC_IPC_DEVICE_227G || SIMATIC_IPC_DEVICE_BX_59A)
+		if (ledmode == SIMATIC_IPC_DEVICE_227G || ledmode == SIMATIC_IPC_DEVICE_BX_59A)
 			pdevname = KBUILD_MODNAME "_leds_gpio_f7188x";
 		if (ledmode == SIMATIC_IPC_DEVICE_BX_21A)
 			pdevname = KBUILD_MODNAME "_leds_gpio_elkhartlake";
-- 
2.25.1

[PATCH 2/3] leds: simatic-ipc-leds-gpio: add support for module BX-59A

[xingtong_wu]

From: "xingtong.wu" <xingtong.wu@siemens.com>

This is used for the Siemens Simatic IPC BX-59A, which has its LEDs
connected to GPIOs provided by the Nuvoton NCT6126D

Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
---
 .../leds/simple/simatic-ipc-leds-gpio-core.c  |  1 +
 .../simple/simatic-ipc-leds-gpio-f7188x.c     | 42 ++++++++++++++++---
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio-core.c b/drivers/leds/simple/simatic-ipc-leds-gpio-core.c
index c552ea73ed9d..10dca208d8cc 100644
--- a/drivers/leds/simple/simatic-ipc-leds-gpio-core.c
+++ b/drivers/leds/simple/simatic-ipc-leds-gpio-core.c
@@ -58,6 +58,7 @@ int simatic_ipc_leds_gpio_probe(struct platform_device *pdev,
 	case SIMATIC_IPC_DEVICE_127E:
 	case SIMATIC_IPC_DEVICE_227G:
 	case SIMATIC_IPC_DEVICE_BX_21A:
+	case SIMATIC_IPC_DEVICE_BX_59A:
 		break;
 	default:
 		return -ENODEV;
diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c b/drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c
index 583a6b6c7c22..a5b544b20857 100644
--- a/drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c
+++ b/drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c
@@ -17,7 +17,10 @@
 
 #include "simatic-ipc-leds-gpio.h"
 
-static struct gpiod_lookup_table simatic_ipc_led_gpio_table = {
+static struct gpiod_lookup_table *led_lookup_table;
+static struct gpiod_lookup_table *led_lookup_table_extra;
+
+static struct gpiod_lookup_table simatic_ipc_led_gpio_table_227g = {
 	.dev_id = "leds-gpio",
 	.table = {
 		GPIO_LOOKUP_IDX("gpio-f7188x-2", 0, NULL, 0, GPIO_ACTIVE_LOW),
@@ -30,7 +33,7 @@ static struct gpiod_lookup_table simatic_ipc_led_gpio_table = {
 	},
 };
 
-static struct gpiod_lookup_table simatic_ipc_led_gpio_table_extra = {
+static struct gpiod_lookup_table simatic_ipc_led_gpio_table_extra_227g = {
 	.dev_id = NULL, /* Filled during initialization */
 	.table = {
 		GPIO_LOOKUP_IDX("gpio-f7188x-3", 6, NULL, 6, GPIO_ACTIVE_HIGH),
@@ -39,16 +42,43 @@ static struct gpiod_lookup_table simatic_ipc_led_gpio_table_extra = {
 	},
 };
 
+static struct gpiod_lookup_table simatic_ipc_led_gpio_table_bx_59a = {
+	.dev_id = "leds-gpio",
+	.table = {
+		GPIO_LOOKUP_IDX("gpio-f7188x-2", 0, NULL, 0, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("gpio-f7188x-2", 3, NULL, 1, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("gpio-f7188x-5", 3, NULL, 2, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("gpio-f7188x-5", 2, NULL, 3, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("gpio-f7188x-7", 7, NULL, 4, GPIO_ACTIVE_LOW),
+		GPIO_LOOKUP_IDX("gpio-f7188x-7", 4, NULL, 5, GPIO_ACTIVE_LOW),
+		{} /* Terminating entry */
+	}
+};
+
 static int simatic_ipc_leds_gpio_f7188x_probe(struct platform_device *pdev)
 {
-	return simatic_ipc_leds_gpio_probe(pdev, &simatic_ipc_led_gpio_table,
-					   &simatic_ipc_led_gpio_table_extra);
+	const struct simatic_ipc_platform *plat = pdev->dev.platform_data;
+
+	switch (plat->devmode) {
+	case SIMATIC_IPC_DEVICE_227G:
+		led_lookup_table = &simatic_ipc_led_gpio_table_227g;
+		led_lookup_table_extra = &simatic_ipc_led_gpio_table_extra_227g;
+		break;
+	case SIMATIC_IPC_DEVICE_BX_59A:
+		led_lookup_table = &simatic_ipc_led_gpio_table_bx_59a;
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	return simatic_ipc_leds_gpio_probe(pdev, led_lookup_table,
+					   led_lookup_table_extra);
 }
 
 static int simatic_ipc_leds_gpio_f7188x_remove(struct platform_device *pdev)
 {
-	return simatic_ipc_leds_gpio_remove(pdev, &simatic_ipc_led_gpio_table,
-					    &simatic_ipc_led_gpio_table_extra);
+	return simatic_ipc_leds_gpio_remove(pdev, led_lookup_table,
+					    led_lookup_table_extra);
 }
 
 static struct platform_driver simatic_ipc_led_gpio_driver = {
-- 
2.25.1

Re:[PATCH 2/3] leds: simatic-ipc-leds-gpio: add support for module BX-59A

[xingtong.wu]

At 2023-07-31 15:18:33, xingtong_wu@163.com wrote:
>From: "xingtong.wu" <xingtong.wu@siemens.com>
>
>This is used for the Siemens Simatic IPC BX-59A, which has its LEDs
>connected to GPIOs provided by the Nuvoton NCT6126D
>
>Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
>---
> .../leds/simple/simatic-ipc-leds-gpio-core.c  |  1 +
> .../simple/simatic-ipc-leds-gpio-f7188x.c     | 42 ++++++++++++++++---
> 2 files changed, 37 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio-core.c b/drivers/leds/simple/simatic-ipc-leds-gpio-core.c
>index c552ea73ed9d..10dca208d8cc 100644
>--- a/drivers/leds/simple/simatic-ipc-leds-gpio-core.c
>+++ b/drivers/leds/simple/simatic-ipc-leds-gpio-core.c
>@@ -58,6 +58,7 @@ int simatic_ipc_leds_gpio_probe(struct platform_device *pdev,
> 	case SIMATIC_IPC_DEVICE_127E:
> 	case SIMATIC_IPC_DEVICE_227G:
> 	case SIMATIC_IPC_DEVICE_BX_21A:
>+	case SIMATIC_IPC_DEVICE_BX_59A:
> 		break;
> 	default:
> 		return -ENODEV;
>diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c b/drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c
>index 583a6b6c7c22..a5b544b20857 100644
>--- a/drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c
>+++ b/drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c
>@@ -17,7 +17,10 @@
> 
> #include "simatic-ipc-leds-gpio.h"
> 
>-static struct gpiod_lookup_table simatic_ipc_led_gpio_table = {
>+static struct gpiod_lookup_table *led_lookup_table;
>+static struct gpiod_lookup_table *led_lookup_table_extra;
>+
>+static struct gpiod_lookup_table simatic_ipc_led_gpio_table_227g = {
> 	.dev_id = "leds-gpio",
> 	.table = {
> 		GPIO_LOOKUP_IDX("gpio-f7188x-2", 0, NULL, 0, GPIO_ACTIVE_LOW),
>@@ -30,7 +33,7 @@ static struct gpiod_lookup_table simatic_ipc_led_gpio_table = {
> 	},
> };
> 
>-static struct gpiod_lookup_table simatic_ipc_led_gpio_table_extra = {
>+static struct gpiod_lookup_table simatic_ipc_led_gpio_table_extra_227g = {
> 	.dev_id = NULL, /* Filled during initialization */
> 	.table = {
> 		GPIO_LOOKUP_IDX("gpio-f7188x-3", 6, NULL, 6, GPIO_ACTIVE_HIGH),
>@@ -39,16 +42,43 @@ static struct gpiod_lookup_table simatic_ipc_led_gpio_table_extra = {
> 	},
> };
> 
>+static struct gpiod_lookup_table simatic_ipc_led_gpio_table_bx_59a = {
>+	.dev_id = "leds-gpio",
>+	.table = {
>+		GPIO_LOOKUP_IDX("gpio-f7188x-2", 0, NULL, 0, GPIO_ACTIVE_LOW),
>+		GPIO_LOOKUP_IDX("gpio-f7188x-2", 3, NULL, 1, GPIO_ACTIVE_LOW),
>+		GPIO_LOOKUP_IDX("gpio-f7188x-5", 3, NULL, 2, GPIO_ACTIVE_LOW),
>+		GPIO_LOOKUP_IDX("gpio-f7188x-5", 2, NULL, 3, GPIO_ACTIVE_LOW),
>+		GPIO_LOOKUP_IDX("gpio-f7188x-7", 7, NULL, 4, GPIO_ACTIVE_LOW),
>+		GPIO_LOOKUP_IDX("gpio-f7188x-7", 4, NULL, 5, GPIO_ACTIVE_LOW),
>+		{} /* Terminating entry */
>+	}
>+};
>+
> static int simatic_ipc_leds_gpio_f7188x_probe(struct platform_device *pdev)
> {
>-	return simatic_ipc_leds_gpio_probe(pdev, &simatic_ipc_led_gpio_table,
>-					   &simatic_ipc_led_gpio_table_extra);
>+	const struct simatic_ipc_platform *plat = pdev->dev.platform_data;
>+
>+	switch (plat->devmode) {
>+	case SIMATIC_IPC_DEVICE_227G:
>+		led_lookup_table = &simatic_ipc_led_gpio_table_227g;
>+		led_lookup_table_extra = &simatic_ipc_led_gpio_table_extra_227g;
>+		break;
>+	case SIMATIC_IPC_DEVICE_BX_59A:
>+		led_lookup_table = &simatic_ipc_led_gpio_table_bx_59a;
>+		break;
>+	default:
>+		return -ENODEV;
>+	}
>+
>+	return simatic_ipc_leds_gpio_probe(pdev, led_lookup_table,
>+					   led_lookup_table_extra);
> }
> 
> static int simatic_ipc_leds_gpio_f7188x_remove(struct platform_device *pdev)
> {
>-	return simatic_ipc_leds_gpio_remove(pdev, &simatic_ipc_led_gpio_table,
>-					    &simatic_ipc_led_gpio_table_extra);
>+	return simatic_ipc_leds_gpio_remove(pdev, led_lookup_table,
>+					    led_lookup_table_extra);
> }
> 
> static struct platform_driver simatic_ipc_led_gpio_driver = {
>-- 
>2.25.1

Hi

After engaging in a thorough discussion, it appears that we have
reached a consensus to merge this patch. Considering the considerable
duration that has passed, I am sending this email as a gentle reminder,
in case there is a possibility that it may have slipped your mind.

XingTong

Re: [PATCH 2/3] leds: simatic-ipc-leds-gpio: add support for module BX-59A

[Lee Jones]

On Fri, 02 Feb 2024, xingtong.wu wrote:

> At 2023-07-31 15:18:33, xingtong_wu@163.com wrote:
> >From: "xingtong.wu" <xingtong.wu@siemens.com>
> >
> >This is used for the Siemens Simatic IPC BX-59A, which has its LEDs
> >connected to GPIOs provided by the Nuvoton NCT6126D
> >
> >Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
> >---
> > .../leds/simple/simatic-ipc-leds-gpio-core.c  |  1 +
> > .../simple/simatic-ipc-leds-gpio-f7188x.c     | 42 ++++++++++++++++---
> > 2 files changed, 37 insertions(+), 6 deletions(-)
> >
> >diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio-core.c b/drivers/leds/simple/simatic-ipc-leds-gpio-core.c
> >index c552ea73ed9d..10dca208d8cc 100644
> >--- a/drivers/leds/simple/simatic-ipc-leds-gpio-core.c
> >+++ b/drivers/leds/simple/simatic-ipc-leds-gpio-core.c
> >@@ -58,6 +58,7 @@ int simatic_ipc_leds_gpio_probe(struct platform_device *pdev,
> > 	case SIMATIC_IPC_DEVICE_127E:
> > 	case SIMATIC_IPC_DEVICE_227G:
> > 	case SIMATIC_IPC_DEVICE_BX_21A:
> >+	case SIMATIC_IPC_DEVICE_BX_59A:
> > 		break;
> > 	default:
> > 		return -ENODEV;
> >diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c b/drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c
> >index 583a6b6c7c22..a5b544b20857 100644
> >--- a/drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c
> >+++ b/drivers/leds/simple/simatic-ipc-leds-gpio-f7188x.c
> >@@ -17,7 +17,10 @@
> > 
> > #include "simatic-ipc-leds-gpio.h"
> > 
> >-static struct gpiod_lookup_table simatic_ipc_led_gpio_table = {
> >+static struct gpiod_lookup_table *led_lookup_table;
> >+static struct gpiod_lookup_table *led_lookup_table_extra;
> >+
> >+static struct gpiod_lookup_table simatic_ipc_led_gpio_table_227g = {
> > 	.dev_id = "leds-gpio",
> > 	.table = {
> > 		GPIO_LOOKUP_IDX("gpio-f7188x-2", 0, NULL, 0, GPIO_ACTIVE_LOW),
> >@@ -30,7 +33,7 @@ static struct gpiod_lookup_table simatic_ipc_led_gpio_table = {
> > 	},
> > };
> > 
> >-static struct gpiod_lookup_table simatic_ipc_led_gpio_table_extra = {
> >+static struct gpiod_lookup_table simatic_ipc_led_gpio_table_extra_227g = {
> > 	.dev_id = NULL, /* Filled during initialization */
> > 	.table = {
> > 		GPIO_LOOKUP_IDX("gpio-f7188x-3", 6, NULL, 6, GPIO_ACTIVE_HIGH),
> >@@ -39,16 +42,43 @@ static struct gpiod_lookup_table simatic_ipc_led_gpio_table_extra = {
> > 	},
> > };
> > 
> >+static struct gpiod_lookup_table simatic_ipc_led_gpio_table_bx_59a = {
> >+	.dev_id = "leds-gpio",
> >+	.table = {
> >+		GPIO_LOOKUP_IDX("gpio-f7188x-2", 0, NULL, 0, GPIO_ACTIVE_LOW),
> >+		GPIO_LOOKUP_IDX("gpio-f7188x-2", 3, NULL, 1, GPIO_ACTIVE_LOW),
> >+		GPIO_LOOKUP_IDX("gpio-f7188x-5", 3, NULL, 2, GPIO_ACTIVE_LOW),
> >+		GPIO_LOOKUP_IDX("gpio-f7188x-5", 2, NULL, 3, GPIO_ACTIVE_LOW),
> >+		GPIO_LOOKUP_IDX("gpio-f7188x-7", 7, NULL, 4, GPIO_ACTIVE_LOW),
> >+		GPIO_LOOKUP_IDX("gpio-f7188x-7", 4, NULL, 5, GPIO_ACTIVE_LOW),
> >+		{} /* Terminating entry */
> >+	}
> >+};
> >+
> > static int simatic_ipc_leds_gpio_f7188x_probe(struct platform_device *pdev)
> > {
> >-	return simatic_ipc_leds_gpio_probe(pdev, &simatic_ipc_led_gpio_table,
> >-					   &simatic_ipc_led_gpio_table_extra);
> >+	const struct simatic_ipc_platform *plat = pdev->dev.platform_data;
> >+
> >+	switch (plat->devmode) {
> >+	case SIMATIC_IPC_DEVICE_227G:
> >+		led_lookup_table = &simatic_ipc_led_gpio_table_227g;
> >+		led_lookup_table_extra = &simatic_ipc_led_gpio_table_extra_227g;
> >+		break;
> >+	case SIMATIC_IPC_DEVICE_BX_59A:
> >+		led_lookup_table = &simatic_ipc_led_gpio_table_bx_59a;
> >+		break;
> >+	default:
> >+		return -ENODEV;
> >+	}
> >+
> >+	return simatic_ipc_leds_gpio_probe(pdev, led_lookup_table,
> >+					   led_lookup_table_extra);
> > }
> > 
> > static int simatic_ipc_leds_gpio_f7188x_remove(struct platform_device *pdev)
> > {
> >-	return simatic_ipc_leds_gpio_remove(pdev, &simatic_ipc_led_gpio_table,
> >-					    &simatic_ipc_led_gpio_table_extra);
> >+	return simatic_ipc_leds_gpio_remove(pdev, led_lookup_table,
> >+					    led_lookup_table_extra);
> > }
> > 
> > static struct platform_driver simatic_ipc_led_gpio_driver = {
> >-- 
> >2.25.1
> 
> Hi
> 
> After engaging in a thorough discussion, it appears that we have
> reached a consensus to merge this patch.

I have no idea what you're referring to here.

> Considering the considerable
> duration that has passed, I am sending this email as a gentle reminder,
> in case there is a possibility that it may have slipped your mind.

Please resubmit all unmerged patches as a [RESEND].

-- 
Lee Jones [李琼斯]

[PATCH 3/3] batt: simatic-ipc-batt: add support for module BX-59A

[xingtong_wu]

From: "xingtong.wu" <xingtong.wu@siemens.com>

This is used for the Siemens Simatic IPC BX-59A, which
can monitor the voltage of the CMOS battery with two bits
that indicate low or empty state

Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
---
 drivers/platform/x86/siemens/Kconfig          |  1 +
 .../x86/siemens/simatic-ipc-batt-f7188x.c     | 37 ++++++++++++++-----
 .../platform/x86/siemens/simatic-ipc-batt.c   |  3 +-
 3 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/x86/siemens/Kconfig b/drivers/platform/x86/siemens/Kconfig
index 9c227a19432f..2ee36e33b03d 100644
--- a/drivers/platform/x86/siemens/Kconfig
+++ b/drivers/platform/x86/siemens/Kconfig
@@ -53,6 +53,7 @@ config SIEMENS_SIMATIC_IPC_BATT_ELKHARTLAKE
 config SIEMENS_SIMATIC_IPC_BATT_F7188X
 	tristate "CMOS Battery monitoring for Simatic IPCs based on Nuvoton GPIO"
 	depends on GPIO_F7188X
+	depends on PINCTRL_ALDERLAKE
 	depends on SIEMENS_SIMATIC_IPC_BATT
 	default SIEMENS_SIMATIC_IPC_BATT
 	help
diff --git a/drivers/platform/x86/siemens/simatic-ipc-batt-f7188x.c b/drivers/platform/x86/siemens/simatic-ipc-batt-f7188x.c
index ed330f6a8ea8..a66107e0fe1e 100644
--- a/drivers/platform/x86/siemens/simatic-ipc-batt-f7188x.c
+++ b/drivers/platform/x86/siemens/simatic-ipc-batt-f7188x.c
@@ -17,6 +17,8 @@
 
 #include "simatic-ipc-batt.h"
 
+static struct gpiod_lookup_table *batt_lookup_table;
+
 static struct gpiod_lookup_table simatic_ipc_batt_gpio_table_227g = {
 	.table = {
 		GPIO_LOOKUP_IDX("gpio-f7188x-7", 6, NULL, 0, GPIO_ACTIVE_HIGH),
@@ -34,24 +36,39 @@ static struct gpiod_lookup_table simatic_ipc_batt_gpio_table_bx_39a = {
 	},
 };
 
+static struct gpiod_lookup_table simatic_ipc_batt_gpio_table_bx_59a = {
+	.table = {
+		GPIO_LOOKUP_IDX("gpio-f7188x-7", 6, NULL, 0, GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP_IDX("gpio-f7188x-7", 5, NULL, 1, GPIO_ACTIVE_HIGH),
+		GPIO_LOOKUP_IDX("INTC1056:00", 438, NULL, 2, GPIO_ACTIVE_HIGH),
+		{} /* Terminating entry */
+	}
+};
+
 static int simatic_ipc_batt_f7188x_remove(struct platform_device *pdev)
 {
-	const struct simatic_ipc_platform *plat = pdev->dev.platform_data;
-
-	if (plat->devmode == SIMATIC_IPC_DEVICE_227G)
-		return simatic_ipc_batt_remove(pdev, &simatic_ipc_batt_gpio_table_227g);
-
-	return simatic_ipc_batt_remove(pdev, &simatic_ipc_batt_gpio_table_bx_39a);
+	return simatic_ipc_batt_remove(pdev, batt_lookup_table);
 }
 
 static int simatic_ipc_batt_f7188x_probe(struct platform_device *pdev)
 {
 	const struct simatic_ipc_platform *plat = pdev->dev.platform_data;
 
-	if (plat->devmode == SIMATIC_IPC_DEVICE_227G)
-		return simatic_ipc_batt_probe(pdev, &simatic_ipc_batt_gpio_table_227g);
+	switch (plat->devmode) {
+	case SIMATIC_IPC_DEVICE_227G:
+		batt_lookup_table = &simatic_ipc_batt_gpio_table_227g;
+		break;
+	case SIMATIC_IPC_DEVICE_BX_39A:
+		batt_lookup_table = &simatic_ipc_batt_gpio_table_bx_39a;
+		break;
+	case SIMATIC_IPC_DEVICE_BX_59A:
+		batt_lookup_table = &simatic_ipc_batt_gpio_table_bx_59a;
+		break;
+	default:
+		return -ENODEV;
+	}
 
-	return simatic_ipc_batt_probe(pdev, &simatic_ipc_batt_gpio_table_bx_39a);
+	return simatic_ipc_batt_probe(pdev, batt_lookup_table);
 }
 
 static struct platform_driver simatic_ipc_batt_driver = {
@@ -66,5 +83,5 @@ module_platform_driver(simatic_ipc_batt_driver);
 
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:" KBUILD_MODNAME);
-MODULE_SOFTDEP("pre: simatic-ipc-batt gpio_f7188x platform:elkhartlake-pinctrl");
+MODULE_SOFTDEP("pre: simatic-ipc-batt gpio_f7188x platform:elkhartlake-pinctrl platform:alderlake-pinctrl");
 MODULE_AUTHOR("Henning Schild <henning.schild@siemens.com>");
diff --git a/drivers/platform/x86/siemens/simatic-ipc-batt.c b/drivers/platform/x86/siemens/simatic-ipc-batt.c
index e34417ca9e13..d66b9969234b 100644
--- a/drivers/platform/x86/siemens/simatic-ipc-batt.c
+++ b/drivers/platform/x86/siemens/simatic-ipc-batt.c
@@ -169,6 +169,7 @@ int simatic_ipc_batt_probe(struct platform_device *pdev, struct gpiod_lookup_tab
 	case SIMATIC_IPC_DEVICE_227G:
 	case SIMATIC_IPC_DEVICE_BX_39A:
 	case SIMATIC_IPC_DEVICE_BX_21A:
+	case SIMATIC_IPC_DEVICE_BX_59A:
 		table->dev_id = dev_name(dev);
 		gpiod_add_lookup_table(table);
 		break;
@@ -193,7 +194,7 @@ int simatic_ipc_batt_probe(struct platform_device *pdev, struct gpiod_lookup_tab
 
 	if (table->table[2].key) {
 		flags = GPIOD_OUT_HIGH;
-		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A)
+		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
 			flags = GPIOD_OUT_LOW;
 		priv.gpios[2] = devm_gpiod_get_index(dev, "CMOSBattery meter", 2, flags);
 		if (IS_ERR(priv.gpios[2])) {
-- 
2.25.1

[PATCH 2/2] platform/x86/siemens: simatic-ipc-batt: fix logic error for BX-59A

[xingtong_wu]

From: "xingtong.wu" <xingtong.wu@siemens.com>

There is a bug in if statement that lead to logical error
and have influence to other IPC, it get correct now.

Fixes: c56beff20375 ("platform/x86/siemens: simatic-ipc-batt: add support for module BX-59A")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202308010001.BGYCSQrl-lkp@intel.com/
Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
---
 drivers/platform/x86/siemens/simatic-ipc-batt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/siemens/simatic-ipc-batt.c b/drivers/platform/x86/siemens/simatic-ipc-batt.c
index d66b9969234b..e6c12c52843c 100644
--- a/drivers/platform/x86/siemens/simatic-ipc-batt.c
+++ b/drivers/platform/x86/siemens/simatic-ipc-batt.c
@@ -194,7 +194,8 @@ int simatic_ipc_batt_probe(struct platform_device *pdev, struct gpiod_lookup_tab
 
 	if (table->table[2].key) {
 		flags = GPIOD_OUT_HIGH;
-		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
+		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A ||
+		    priv.devmode == SIMATIC_IPC_DEVICE_BX_59A)
 			flags = GPIOD_OUT_LOW;
 		priv.gpios[2] = devm_gpiod_get_index(dev, "CMOSBattery meter", 2, flags);
 		if (IS_ERR(priv.gpios[2])) {
-- 
2.25.1

Re: [PATCH 2/2] platform/x86/siemens: simatic-ipc-batt: fix logic error for BX-59A

[Ilpo Järvinen]

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

On Tue, 1 Aug 2023, xingtong_wu@163.com wrote:

> From: "xingtong.wu" <xingtong.wu@siemens.com>
> 
> There is a bug in if statement that lead to logical error
> and have influence to other IPC, it get correct now.
> 
> Fixes: c56beff20375 ("platform/x86/siemens: simatic-ipc-batt: add support for module BX-59A")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202308010001.BGYCSQrl-lkp@intel.com/
> Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
> ---
>  drivers/platform/x86/siemens/simatic-ipc-batt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/siemens/simatic-ipc-batt.c b/drivers/platform/x86/siemens/simatic-ipc-batt.c
> index d66b9969234b..e6c12c52843c 100644
> --- a/drivers/platform/x86/siemens/simatic-ipc-batt.c
> +++ b/drivers/platform/x86/siemens/simatic-ipc-batt.c
> @@ -194,7 +194,8 @@ int simatic_ipc_batt_probe(struct platform_device *pdev, struct gpiod_lookup_tab
>  
>  	if (table->table[2].key) {
>  		flags = GPIOD_OUT_HIGH;
> -		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
> +		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A ||
> +		    priv.devmode == SIMATIC_IPC_DEVICE_BX_59A)
>  			flags = GPIOD_OUT_LOW;
>  		priv.gpios[2] = devm_gpiod_get_index(dev, "CMOSBattery meter", 2, flags);
>  		if (IS_ERR(priv.gpios[2])) {
> 

The code change is fine but please try to improve the changelog (has 
similar problems as the other one).

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

[PATCH v2 2/2] platform/x86/siemens: simatic-ipc-batt: fix logic error for BX-59A

[xingtong_wu]

From: "xingtong.wu" <xingtong.wu@siemens.com>

The variable "priv.devmode" is missing from if statement that leads
to a logical error. Add the missing variable to the if condition.

Fixes: c56beff20375 ("platform/x86/siemens: simatic-ipc-batt: add support for module BX-59A")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202308010001.BGYCSQrl-lkp@intel.com/

Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
---
 drivers/platform/x86/siemens/simatic-ipc-batt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/siemens/simatic-ipc-batt.c b/drivers/platform/x86/siemens/simatic-ipc-batt.c
index d66b9969234b..e6c12c52843c 100644
--- a/drivers/platform/x86/siemens/simatic-ipc-batt.c
+++ b/drivers/platform/x86/siemens/simatic-ipc-batt.c
@@ -194,7 +194,8 @@ int simatic_ipc_batt_probe(struct platform_device *pdev, struct gpiod_lookup_tab
 
 	if (table->table[2].key) {
 		flags = GPIOD_OUT_HIGH;
-		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
+		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A ||
+		    priv.devmode == SIMATIC_IPC_DEVICE_BX_59A)
 			flags = GPIOD_OUT_LOW;
 		priv.gpios[2] = devm_gpiod_get_index(dev, "CMOSBattery meter", 2, flags);
 		if (IS_ERR(priv.gpios[2])) {
-- 
2.25.1

Re: [PATCH v2 2/2] platform/x86/siemens: simatic-ipc-batt: fix logic error for BX-59A

[Ilpo Järvinen]

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

On Sat, 1 Aug 2043, xingtong_wu@163.com wrote:

> From: "xingtong.wu" <xingtong.wu@siemens.com>
> 
> The variable "priv.devmode" is missing from if statement that leads
> to a logical error. Add the missing variable to the if condition.
>
> Fixes: c56beff20375 ("platform/x86/siemens: simatic-ipc-batt: add support for module BX-59A")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202308010001.BGYCSQrl-lkp@intel.com/
> 
> Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>

The tags block should not have extra newlines like there's now one in 
between Closes and Signed-off-by.

For patches 1-2:

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

(In case you end up submitting a new version, you should collect the tags 
other people have given you, except e.g., if you did major modifications 
to the patch.)

-- 
 i.

> ---
>  drivers/platform/x86/siemens/simatic-ipc-batt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/siemens/simatic-ipc-batt.c b/drivers/platform/x86/siemens/simatic-ipc-batt.c
> index d66b9969234b..e6c12c52843c 100644
> --- a/drivers/platform/x86/siemens/simatic-ipc-batt.c
> +++ b/drivers/platform/x86/siemens/simatic-ipc-batt.c
> @@ -194,7 +194,8 @@ int simatic_ipc_batt_probe(struct platform_device *pdev, struct gpiod_lookup_tab
>  
>  	if (table->table[2].key) {
>  		flags = GPIOD_OUT_HIGH;
> -		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
> +		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A ||
> +		    priv.devmode == SIMATIC_IPC_DEVICE_BX_59A)
>  			flags = GPIOD_OUT_LOW;
>  		priv.gpios[2] = devm_gpiod_get_index(dev, "CMOSBattery meter", 2, flags);
>  		if (IS_ERR(priv.gpios[2])) {
> 

[PATCH v3 2/2] platform/x86/siemens: simatic-ipc-batt: fix logical error for BX-59A

[xingtong_wu]

From: "xingtong.wu" <xingtong.wu@siemens.com>

The variable "priv.devmode" is missing from if statement that leads
to a logical error. Add the missing variable to the if condition.

Fixes: c56beff20375 ("platform/x86/siemens: simatic-ipc-batt: add support for module BX-59A")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202308010001.BGYCSQrl-lkp@intel.com/
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
---
 drivers/platform/x86/siemens/simatic-ipc-batt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/siemens/simatic-ipc-batt.c b/drivers/platform/x86/siemens/simatic-ipc-batt.c
index d66b9969234b..e6c12c52843c 100644
--- a/drivers/platform/x86/siemens/simatic-ipc-batt.c
+++ b/drivers/platform/x86/siemens/simatic-ipc-batt.c
@@ -194,7 +194,8 @@ int simatic_ipc_batt_probe(struct platform_device *pdev, struct gpiod_lookup_tab
 
 	if (table->table[2].key) {
 		flags = GPIOD_OUT_HIGH;
-		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
+		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A ||
+		    priv.devmode == SIMATIC_IPC_DEVICE_BX_59A)
 			flags = GPIOD_OUT_LOW;
 		priv.gpios[2] = devm_gpiod_get_index(dev, "CMOSBattery meter", 2, flags);
 		if (IS_ERR(priv.gpios[2])) {
-- 
2.25.1

Re: [PATCH v2 2/2] platform/x86/siemens: simatic-ipc-batt: fix logic error for BX-59A

[Ashok Raj]

On Sat, Aug 01, 2043 at 01:34:17AM +0800, xingtong_wu@163.com wrote:
^^^^^^^^^^^^^^^^^^^^

Nice way to keep your mail on top of the sort :-).. You need to fixup your
system date.

> From: "xingtong.wu" <xingtong.wu@siemens.com>
> 
> The variable "priv.devmode" is missing from if statement that leads
> to a logical error. Add the missing variable to the if condition.
> 
> Fixes: c56beff20375 ("platform/x86/siemens: simatic-ipc-batt: add support for module BX-59A")
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202308010001.BGYCSQrl-lkp@intel.com/
> 
> Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
> ---
>  drivers/platform/x86/siemens/simatic-ipc-batt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/siemens/simatic-ipc-batt.c b/drivers/platform/x86/siemens/simatic-ipc-batt.c
> index d66b9969234b..e6c12c52843c 100644
> --- a/drivers/platform/x86/siemens/simatic-ipc-batt.c
> +++ b/drivers/platform/x86/siemens/simatic-ipc-batt.c
> @@ -194,7 +194,8 @@ int simatic_ipc_batt_probe(struct platform_device *pdev, struct gpiod_lookup_tab
>  
>  	if (table->table[2].key) {
>  		flags = GPIOD_OUT_HIGH;
> -		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
> +		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A ||
> +		    priv.devmode == SIMATIC_IPC_DEVICE_BX_59A)
>  			flags = GPIOD_OUT_LOW;
>  		priv.gpios[2] = devm_gpiod_get_index(dev, "CMOSBattery meter", 2, flags);
>  		if (IS_ERR(priv.gpios[2])) {
> -- 
> 2.25.1
> 

Re: [PATCH v2 2/2] platform/x86/siemens: simatic-ipc-batt: fix logic error for BX-59A

[xingtong.wu]

On 2023/8/16 23:45, Ashok Raj wrote:
> On Sat, Aug 01, 2043 at 01:34:17AM +0800, xingtong_wu@163.com wrote:
> ^^^^^^^^^^^^^^^^^^^^
> 
> Nice way to keep your mail on top of the sort :-).. You need to fixup your
> system date.

I'm sorry that there are time problem on my local machine which lead to a
wrong time in email, but I just sent an email to remind hdegoede@redhat.com
individually, the time problem was solved by him on his branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/commit/?h=review-hans&id=b01c1e022f7f0c327ecc7544dc44d5f80a2d2bd9
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/commit/?h=review-hans&id=7abf253afa5c72c0c7eb21f67da1d443f036737a

> 
>> From: "xingtong.wu" <xingtong.wu@siemens.com>
>>
>> The variable "priv.devmode" is missing from if statement that leads
>> to a logical error. Add the missing variable to the if condition.
>>
>> Fixes: c56beff20375 ("platform/x86/siemens: simatic-ipc-batt: add support for module BX-59A")
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202308010001.BGYCSQrl-lkp@intel.com/
>>
>> Signed-off-by: xingtong.wu <xingtong.wu@siemens.com>
>> ---
>>  drivers/platform/x86/siemens/simatic-ipc-batt.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/platform/x86/siemens/simatic-ipc-batt.c b/drivers/platform/x86/siemens/simatic-ipc-batt.c
>> index d66b9969234b..e6c12c52843c 100644
>> --- a/drivers/platform/x86/siemens/simatic-ipc-batt.c
>> +++ b/drivers/platform/x86/siemens/simatic-ipc-batt.c
>> @@ -194,7 +194,8 @@ int simatic_ipc_batt_probe(struct platform_device *pdev, struct gpiod_lookup_tab
>>  
>>  	if (table->table[2].key) {
>>  		flags = GPIOD_OUT_HIGH;
>> -		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A || SIMATIC_IPC_DEVICE_BX_59A)
>> +		if (priv.devmode == SIMATIC_IPC_DEVICE_BX_21A ||
>> +		    priv.devmode == SIMATIC_IPC_DEVICE_BX_59A)
>>  			flags = GPIOD_OUT_LOW;
>>  		priv.gpios[2] = devm_gpiod_get_index(dev, "CMOSBattery meter", 2, flags);
>>  		if (IS_ERR(priv.gpios[2])) {
>> -- 
>> 2.25.1
>>

Re: [PATCH 0/3] platform/x86: simatic-ipc: add another model BX-56A/BX-59A

[Hans de Goede]

Hi,

On 7/31/23 09:14, xingtong_wu@163.com wrote:
> From: "xingtong.wu" <xingtong.wu@siemens.com>
> 
> These are patches adding features for another Simatic IPC model
> BX-56A/BX-59A, features for led and battery monitor are involved.
> 
> Based on:
>  [PATCH v2 0/3] platform/x86: move simatic drivers into subdir
>  [PATCH v2 0/2] leds: simatic-ipc-leds-gpio: add new model BX-21A
>  [PATCH 0/2] platform/x86/siemens: fixes for CMOS battery hwmon
> 
> xingtong.wu (3):
>   platform/x86: simatic-ipc: add new models BX-56A/BX-59A
>   leds: simatic-ipc-leds-gpio: add support for module BX-59A
>   batt: simatic-ipc-batt: add support for module BX-59A

Thank you. I have merged patches 1/3 and 3/3 into
the platform-drivers-x86-simatic-ipc branch now:

https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=platform-drivers-x86-simatic-ipc

Lee, I'll send you a pull-req for an immutable tag so that you
can merge 2/3 into lee/leds.git .

Once I've run some tests on this branch the patches there will be
merged into the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans






> 
>  .../leds/simple/simatic-ipc-leds-gpio-core.c  |  1 +
>  .../simple/simatic-ipc-leds-gpio-f7188x.c     | 42 ++++++++++++++++---
>  drivers/platform/x86/siemens/Kconfig          |  1 +
>  .../x86/siemens/simatic-ipc-batt-f7188x.c     | 37 +++++++++++-----
>  .../platform/x86/siemens/simatic-ipc-batt.c   |  3 +-
>  drivers/platform/x86/siemens/simatic-ipc.c    | 12 +++++-
>  .../platform_data/x86/simatic-ipc-base.h      |  1 +
>  include/linux/platform_data/x86/simatic-ipc.h |  2 +
>  8 files changed, 80 insertions(+), 19 deletions(-)
>