drivers/edac/Kconfig | 1 + drivers/edac/pnd2_edac.c | 62 ++--- drivers/i2c/busses/Kconfig | 1 + drivers/i2c/busses/i2c-i801.c | 39 +--- drivers/mfd/Kconfig | 1 + drivers/mfd/lpc_ich.c | 136 +++++++++-- drivers/pinctrl/intel/pinctrl-intel.c | 14 +- drivers/platform/x86/intel/Kconfig | 12 + drivers/platform/x86/intel/Makefile | 1 + drivers/platform/x86/intel/p2sb.c | 305 +++++++++++++++++++++++++ include/linux/platform_data/x86/p2sb.h | 27 +++ 11 files changed, 500 insertions(+), 99 deletions(-) create mode 100644 drivers/platform/x86/intel/p2sb.c create mode 100644 include/linux/platform_data/x86/p2sb.h
There are a few users and at least one more is coming (*) that would like
to utilize P2SB mechanism of hiding and unhiding a device from the PCI
configuration space.
Here is the series to consolidate p2sb handling code for existing users
and provide a generic way for new comer(s).
It also includes a patch to enable GPIO controllers on Apollo Lake
when it's used with ABL bootloader w/o ACPI support.
The patch that bring the helper ("platform/x86/intel: Add Primary
to Sideband (P2SB) bridge support") has a commit message that
sheds a light on what the P2SB is and why this is needed.
The changes made in v2 do not change the main idea and the functionality
in a big scale. What we need is probably one more (RE-)test done by Henning.
I hope to have it merged to v5.18-rc1 that Siemens can develop their changes
based on this series.
I have tested this on Apollo Lake platform (I'm able to see SPI NOR and
since we have an ACPI device for GPIO I do not see any attempts to recreate
one).
*) One in this series, and one is a due after merge in the Simatic IPC drivers
The series may be routed either via MFD (and I guess Lee would prefer that)
or via PDx86, whichever seems better for you, folks. As of today patches
are ACKed by the respective maintainers, but I2C one and one of the MFD.
Wolfram, can you ACK the patch against i2c-i801 driver, if you have no
objections?
Changes in v4:
- added tag to the entire series (Hans)
- added tag to pin control patch (Mika)
- dropped PCI core changes (PCI core doesn't want modifications to be made)
- as a consequence of the above merged necessary bits into p2sb.c
- added a check that p2sb is really hidden (Hans)
- added EDAC patches (reviewed by maintainer internally)
Changes in v3:
- resent with cover letter
Changes in v2:
- added parentheses around bus in macros (Joe)
- added tag (Jean)
- fixed indentation and wrapping in the header (Christoph)
- moved out of PCI realm to PDx86 as the best common denominator (Bjorn)
- added a verbose commit message to explain P2SB thingy (Bjorn)
- converted first parameter from pci_dev to pci_bus
- made first two parameters (bus and devfn) optional (Henning, Lee)
- added Intel pin control patch to the series (Henning, Mika)
- fixed English style in the commit message of one of MFD patch (Lee)
- added tags to my MFD LPC ICH patches (Lee)
- used consistently (c) (Lee)
- made indexing for MFD cell and resource arrays (Lee)
- fixed the resource size in i801 (Jean)
Andy Shevchenko (6):
pinctrl: intel: Check against matching data instead of ACPI companion
mfd: lpc_ich: Factor out lpc_ich_enable_spi_write()
mfd: lpc_ich: Switch to generic p2sb_bar()
i2c: i801: convert to use common P2SB accessor
EDAC, pnd2: Use proper I/O accessors and address space annotation
EDAC, pnd2: convert to use common P2SB accessor
Jonathan Yong (1):
platform/x86/intel: Add Primary to Sideband (P2SB) bridge support
Tan Jui Nee (1):
mfd: lpc_ich: Add support for pinctrl in non-ACPI system
drivers/edac/Kconfig | 1 +
drivers/edac/pnd2_edac.c | 62 ++---
drivers/i2c/busses/Kconfig | 1 +
drivers/i2c/busses/i2c-i801.c | 39 +---
drivers/mfd/Kconfig | 1 +
drivers/mfd/lpc_ich.c | 136 +++++++++--
drivers/pinctrl/intel/pinctrl-intel.c | 14 +-
drivers/platform/x86/intel/Kconfig | 12 +
drivers/platform/x86/intel/Makefile | 1 +
drivers/platform/x86/intel/p2sb.c | 305 +++++++++++++++++++++++++
include/linux/platform_data/x86/p2sb.h | 27 +++
11 files changed, 500 insertions(+), 99 deletions(-)
create mode 100644 drivers/platform/x86/intel/p2sb.c
create mode 100644 include/linux/platform_data/x86/p2sb.h
--
2.34.1
This switches the simatic-ipc modules to using the p2sb interface introduced by Andy with "platform/x86: introduce p2sb_bar() helper". It also switches to one apollo lake device to using gpio leds. I am kind of hoping Andy will take this on top and propose it in his series. Henning Schild (2): simatic-ipc: convert to use common P2SB accessor leds: simatic-ipc-leds-gpio: add GPIO version of Siemens driver drivers/leds/simple/Kconfig | 11 ++ drivers/leds/simple/Makefile | 3 +- drivers/leds/simple/simatic-ipc-leds-gpio.c | 108 ++++++++++++++++++ drivers/leds/simple/simatic-ipc-leds.c | 77 +------------ drivers/platform/x86/simatic-ipc.c | 43 +------ drivers/watchdog/Kconfig | 1 + drivers/watchdog/simatic-ipc-wdt.c | 15 +-- .../platform_data/x86/simatic-ipc-base.h | 2 - 8 files changed, 139 insertions(+), 121 deletions(-) create mode 100644 drivers/leds/simple/simatic-ipc-leds-gpio.c -- 2.34.1
On Tue, Mar 08, 2022 at 08:35:20PM +0100, Henning Schild wrote: > This switches the simatic-ipc modules to using the p2sb interface > introduced by Andy with "platform/x86: introduce p2sb_bar() helper". > > It also switches to one apollo lake device to using gpio leds. > > I am kind of hoping Andy will take this on top and propose it in his > series. First of all, they are not applicable to my current version [1] of the series (it maybe something changed in the Simatic drivers upstream, because I have got conflicts there. For the record, I'm using Linux Next as a base. Second question is could it be possible to split first patch into three, or it has to be in one? [1]: https://gitlab.com/andy-shev/next/-/tree/topic/p2sb-next It would be nice if you can perform another round of testing. > Henning Schild (2): > simatic-ipc: convert to use common P2SB accessor > leds: simatic-ipc-leds-gpio: add GPIO version of Siemens driver > > drivers/leds/simple/Kconfig | 11 ++ > drivers/leds/simple/Makefile | 3 +- > drivers/leds/simple/simatic-ipc-leds-gpio.c | 108 ++++++++++++++++++ > drivers/leds/simple/simatic-ipc-leds.c | 77 +------------ > drivers/platform/x86/simatic-ipc.c | 43 +------ > drivers/watchdog/Kconfig | 1 + > drivers/watchdog/simatic-ipc-wdt.c | 15 +-- > .../platform_data/x86/simatic-ipc-base.h | 2 - > 8 files changed, 139 insertions(+), 121 deletions(-) > create mode 100644 drivers/leds/simple/simatic-ipc-leds-gpio.c > > -- > 2.34.1 > -- With Best Regards, Andy Shevchenko
Am Wed, 4 May 2022 15:51:01 +0300 schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>: > On Tue, Mar 08, 2022 at 08:35:20PM +0100, Henning Schild wrote: > > This switches the simatic-ipc modules to using the p2sb interface > > introduced by Andy with "platform/x86: introduce p2sb_bar() helper". > > > > It also switches to one apollo lake device to using gpio leds. > > > > I am kind of hoping Andy will take this on top and propose it in his > > series. > > First of all, they are not applicable to my current version [1] of > the series (it maybe something changed in the Simatic drivers > upstream, because I have got conflicts there. For the record, I'm > using Linux Next as a base. That is possible, some sparse findings have been fixed lately. > Second question is could it be possible to split first patch into > three, or it has to be in one? I assume one for leds one for wdt and finally drop stuff from platform, and i will go with that assumption for a next round based on your tree directly. Can you explain why that will be useful? While it is kind of a separation of concerns and subsystems ... it also kind of all belongs together and needs to be merged in a rather strict order. regards, Henning > [1]: https://gitlab.com/andy-shev/next/-/tree/topic/p2sb-next > It would be nice if you can perform another round of testing. > > > Henning Schild (2): > > simatic-ipc: convert to use common P2SB accessor > > leds: simatic-ipc-leds-gpio: add GPIO version of Siemens driver > > > > drivers/leds/simple/Kconfig | 11 ++ > > drivers/leds/simple/Makefile | 3 +- > > drivers/leds/simple/simatic-ipc-leds-gpio.c | 108 > > ++++++++++++++++++ drivers/leds/simple/simatic-ipc-leds.c | > > 77 +------------ drivers/platform/x86/simatic-ipc.c | > > 43 +------ drivers/watchdog/Kconfig | 1 + > > drivers/watchdog/simatic-ipc-wdt.c | 15 +-- > > .../platform_data/x86/simatic-ipc-base.h | 2 - > > 8 files changed, 139 insertions(+), 121 deletions(-) > > create mode 100644 drivers/leds/simple/simatic-ipc-leds-gpio.c > > > > -- > > 2.34.1 > > >
Am Wed, 4 May 2022 17:19:51 +0200
schrieb Henning Schild <henning.schild@siemens.com>:
> Am Wed, 4 May 2022 15:51:01 +0300
> schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
>
> > On Tue, Mar 08, 2022 at 08:35:20PM +0100, Henning Schild wrote:
> > > This switches the simatic-ipc modules to using the p2sb interface
> > > introduced by Andy with "platform/x86: introduce p2sb_bar()
> > > helper".
> > >
> > > It also switches to one apollo lake device to using gpio leds.
> > >
> > > I am kind of hoping Andy will take this on top and propose it in
> > > his series.
> >
> > First of all, they are not applicable to my current version [1] of
> > the series (it maybe something changed in the Simatic drivers
> > upstream, because I have got conflicts there. For the record, I'm
> > using Linux Next as a base.
>
> That is possible, some sparse findings have been fixed lately.
>
> > Second question is could it be possible to split first patch into
> > three, or it has to be in one?
>
> I assume one for leds one for wdt and finally drop stuff from
> platform, and i will go with that assumption for a next round based
> on your tree directly.
> Can you explain why that will be useful? While it is kind of a
> separation of concerns and subsystems ... it also kind of all belongs
> together and needs to be merged in a rather strict order.
>
> regards,
> Henning
>
> > [1]: https://gitlab.com/andy-shev/next/-/tree/topic/p2sb-next
> > It would be nice if you can perform another round of testing.
Just got around to testing this with my patches on top. My stuff will
need some more work before i can send again.
Is this a rebasing branch?
With efc7d77ea372 ("EDAC, pnd2: convert to use common P2SB accessor")
I am seeing problems while booting ... things do work but still error
messages which probably should not be there.
[ 2.215715] gpio gpiochip0: (apollolake-pinctrl.0): not an immutable chip, please consider fixing it!
[ 2.217506] broxton-pinctrl apollolake-pinctrl.1: Failed to attach ACPI GPIO chip
[ 2.217542] gpio gpiochip1: (apollolake-pinctrl.1): not an immutable chip, please consider fixing it!
[ 2.217771] i801_smbus 0000:00:1f.1: Failed to enable SMBus PCI device (-22)
[ 2.217788] i801_smbus: probe of 0000:00:1f.1 failed with error -22
[ 2.221460] broxton-pinctrl apollolake-pinctrl.2: Failed to attach ACPI GPIO chip
[ 2.221482] gpio gpiochip2: (apollolake-pinctrl.2): not an immutable chip, please consider fixing it!
[ 2.222010] broxton-pinctrl apollolake-pinctrl.3: Failed to attach ACPI GPIO chip
[ 2.222023] gpio gpiochip3: (apollolake-pinctrl.3): not an immutable
chip, please consider fixing it!
regards,
Henning
> > > Henning Schild (2):
> > > simatic-ipc: convert to use common P2SB accessor
> > > leds: simatic-ipc-leds-gpio: add GPIO version of Siemens driver
> > >
> > > drivers/leds/simple/Kconfig | 11 ++
> > > drivers/leds/simple/Makefile | 3 +-
> > > drivers/leds/simple/simatic-ipc-leds-gpio.c | 108
> > > ++++++++++++++++++ drivers/leds/simple/simatic-ipc-leds.c |
> > > 77 +------------ drivers/platform/x86/simatic-ipc.c |
> > > 43 +------ drivers/watchdog/Kconfig | 1 +
> > > drivers/watchdog/simatic-ipc-wdt.c | 15 +--
> > > .../platform_data/x86/simatic-ipc-base.h | 2 -
> > > 8 files changed, 139 insertions(+), 121 deletions(-)
> > > create mode 100644 drivers/leds/simple/simatic-ipc-leds-gpio.c
> > >
> > > --
> > > 2.34.1
> > >
> >
>
On Tue, May 10, 2022 at 05:30:53PM +0200, Henning Schild wrote:
> Am Wed, 4 May 2022 17:19:51 +0200
> schrieb Henning Schild <henning.schild@siemens.com>:
> > Am Wed, 4 May 2022 15:51:01 +0300
> > schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> > > On Tue, Mar 08, 2022 at 08:35:20PM +0100, Henning Schild wrote:
...
> > > [1]: https://gitlab.com/andy-shev/next/-/tree/topic/p2sb-next
> > > It would be nice if you can perform another round of testing.
>
> Just got around to testing this with my patches on top. My stuff will
> need some more work before i can send again.
>
> Is this a rebasing branch?
> With efc7d77ea372 ("EDAC, pnd2: convert to use common P2SB accessor")
It's rebased over and over. I just pushed the same version I have sent as v5.
> I am seeing problems while booting ... things do work but still error
> messages which probably should not be there.
It's okay. This is not related to my stuff, it's a new series from Marc which
enables that (harmless) warning.
> [ 2.217506] broxton-pinctrl apollolake-pinctrl.1: Failed to attach ACPI GPIO chip
> [ 2.217542] gpio gpiochip1: (apollolake-pinctrl.1): not an immutable chip, please consider fixing it!
> [ 2.217771] i801_smbus 0000:00:1f.1: Failed to enable SMBus PCI device (-22)
> [ 2.217788] i801_smbus: probe of 0000:00:1f.1 failed with error -22
> [ 2.221460] broxton-pinctrl apollolake-pinctrl.2: Failed to attach ACPI GPIO chip
> [ 2.221482] gpio gpiochip2: (apollolake-pinctrl.2): not an immutable chip, please consider fixing it!
> [ 2.222010] broxton-pinctrl apollolake-pinctrl.3: Failed to attach ACPI GPIO chip
> [ 2.222023] gpio gpiochip3: (apollolake-pinctrl.3): not an immutable
> chip, please consider fixing it!
--
With Best Regards,
Andy Shevchenko
On Wed, May 04, 2022 at 05:19:51PM +0200, Henning Schild wrote: > Am Wed, 4 May 2022 15:51:01 +0300 > schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>: > > > On Tue, Mar 08, 2022 at 08:35:20PM +0100, Henning Schild wrote: > > > This switches the simatic-ipc modules to using the p2sb interface > > > introduced by Andy with "platform/x86: introduce p2sb_bar() helper". > > > > > > It also switches to one apollo lake device to using gpio leds. > > > > > > I am kind of hoping Andy will take this on top and propose it in his > > > series. > > > > First of all, they are not applicable to my current version [1] of > > the series (it maybe something changed in the Simatic drivers > > upstream, because I have got conflicts there. For the record, I'm > > using Linux Next as a base. > > That is possible, some sparse findings have been fixed lately. > > > Second question is could it be possible to split first patch into > > three, or it has to be in one? > > I assume one for leds one for wdt and finally drop stuff from platform, > and i will go with that assumption for a next round based on your tree > directly. > Can you explain why that will be useful? While it is kind of a > separation of concerns and subsystems ... it also kind of all belongs > together and needs to be merged in a rather strict order. > That is not really correct. It should be possible to split the patches and only remove simatic_ipc_get_membase0() after the other patches have been applied. On a side note, neither subject nor description of patch 1/2 mention that the patch touches both LED and watchdog code, which is at the very least bad style. Guenter > regards, > Henning > > > [1]: https://gitlab.com/andy-shev/next/-/tree/topic/p2sb-next > > It would be nice if you can perform another round of testing. > > > > > Henning Schild (2): > > > simatic-ipc: convert to use common P2SB accessor > > > leds: simatic-ipc-leds-gpio: add GPIO version of Siemens driver > > > > > > drivers/leds/simple/Kconfig | 11 ++ > > > drivers/leds/simple/Makefile | 3 +- > > > drivers/leds/simple/simatic-ipc-leds-gpio.c | 108 > > > ++++++++++++++++++ drivers/leds/simple/simatic-ipc-leds.c | > > > 77 +------------ drivers/platform/x86/simatic-ipc.c | > > > 43 +------ drivers/watchdog/Kconfig | 1 + > > > drivers/watchdog/simatic-ipc-wdt.c | 15 +-- > > > .../platform_data/x86/simatic-ipc-base.h | 2 - > > > 8 files changed, 139 insertions(+), 121 deletions(-) > > > create mode 100644 drivers/leds/simple/simatic-ipc-leds-gpio.c > > > > > > -- > > > 2.34.1 > > > > > >
On Wed, May 4, 2022 at 6:16 PM Henning Schild <henning.schild@siemens.com> wrote: > Am Wed, 4 May 2022 15:51:01 +0300 > schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>: > > On Tue, Mar 08, 2022 at 08:35:20PM +0100, Henning Schild wrote: ... > > Second question is could it be possible to split first patch into > > three, or it has to be in one? > > I assume one for leds one for wdt and finally drop stuff from platform, Yes. > and i will go with that assumption for a next round based on your tree > directly. > Can you explain why that will be useful? While it is kind of a > separation of concerns and subsystems ... it also kind of all belongs > together and needs to be merged in a rather strict order. The main case here is that it's easy to review during upstreaming and in case of somebody looking into the history. It keeps each of the changes logically isolated. I.o.w. it adds flexibility, for example changing ordering of the WDT and LED patches in the series in this case. I admit that for _this_ series my arguments are not strong, but I'm speaking out of general approach. The pattern 1) add new api 2) switch driver #1 to it ... 2+n) switch driver #n to it 3+n) drop old API is how we do in the Linux kernel, even if the changes are coupled together from a functional / compile perspective. -- With Best Regards, Andy Shevchenko
Since we have a common P2SB accessor in tree we may use it instead of
open coded variants.
Replace custom code by p2sb_bar() call.
Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
drivers/leds/simple/Kconfig | 1 +
drivers/leds/simple/simatic-ipc-leds.c | 14 +++----
drivers/platform/x86/simatic-ipc.c | 38 -------------------
drivers/watchdog/Kconfig | 1 +
drivers/watchdog/simatic-ipc-wdt.c | 15 ++++----
.../platform_data/x86/simatic-ipc-base.h | 2 -
6 files changed, 17 insertions(+), 54 deletions(-)
diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig
index 9f6a68336659..9293e6b36c75 100644
--- a/drivers/leds/simple/Kconfig
+++ b/drivers/leds/simple/Kconfig
@@ -3,6 +3,7 @@ config LEDS_SIEMENS_SIMATIC_IPC
tristate "LED driver for Siemens Simatic IPCs"
depends on LEDS_CLASS
depends on SIEMENS_SIMATIC_IPC
+ select P2SB if X86
help
This option enables support for the LEDs of several Industrial PCs
from Siemens.
diff --git a/drivers/leds/simple/simatic-ipc-leds.c b/drivers/leds/simple/simatic-ipc-leds.c
index ff2c96e73241..215ef5b74236 100644
--- a/drivers/leds/simple/simatic-ipc-leds.c
+++ b/drivers/leds/simple/simatic-ipc-leds.c
@@ -15,6 +15,7 @@
#include <linux/leds.h>
#include <linux/module.h>
#include <linux/pci.h>
+#include <linux/platform_data/x86/p2sb.h>
#include <linux/platform_data/x86/simatic-ipc-base.h>
#include <linux/platform_device.h>
#include <linux/sizes.h>
@@ -38,8 +39,8 @@ static struct simatic_ipc_led simatic_ipc_leds_io[] = {
{ }
};
-/* the actual start will be discovered with PCI, 0 is a placeholder */
-struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, SZ_4K, KBUILD_MODNAME);
+/* the actual start will be discovered with p2sb, 0 is a placeholder */
+struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, 0, KBUILD_MODNAME);
static void *simatic_ipc_led_memory;
@@ -143,14 +144,13 @@ static int simatic_ipc_leds_probe(struct platform_device *pdev)
ipcled = simatic_ipc_leds_mem;
type = IORESOURCE_MEM;
- /* get GPIO base from PCI */
- res->start = simatic_ipc_get_membase0(PCI_DEVFN(13, 0));
- if (res->start == 0)
- return -ENODEV;
+ err = p2sb_bar(NULL, 0, res);
+ if (err)
+ return err;
/* do the final address calculation */
res->start = res->start + (0xC5 << 16);
- res->end += res->start;
+ res->end = res->start + SZ_4K - 1;
simatic_ipc_led_memory = devm_ioremap_resource(dev, res);
if (IS_ERR(simatic_ipc_led_memory))
diff --git a/drivers/platform/x86/simatic-ipc.c b/drivers/platform/x86/simatic-ipc.c
index b599cda5ba3c..26c35e1660cb 100644
--- a/drivers/platform/x86/simatic-ipc.c
+++ b/drivers/platform/x86/simatic-ipc.c
@@ -101,44 +101,6 @@ static int register_platform_devices(u32 station_id)
return 0;
}
-/* FIXME: this should eventually be done with generic P2SB discovery code
- * the individual drivers for watchdogs and LEDs access memory that implements
- * GPIO, but pinctrl will not come up because of missing ACPI entries
- *
- * While there is no conflict a cleaner solution would be to somehow bring up
- * pinctrl even with these ACPI entries missing, and base the drivers on pinctrl.
- * After which the following function could be dropped, together with the code
- * poking the memory.
- */
-/*
- * Get membase address from PCI, used in leds and wdt module. Here we read
- * the bar0. The final address calculation is done in the appropriate modules
- */
-u32 simatic_ipc_get_membase0(unsigned int p2sb)
-{
- struct pci_bus *bus;
- u32 bar0 = 0;
- /*
- * The GPIO memory is in bar0 of the hidden P2SB device.
- * Unhide the device to have a quick look at it, before we hide it
- * again.
- * Also grab the pci rescan lock so that device does not get discovered
- * and remapped while it is visible.
- * This code is inspired by drivers/mfd/lpc_ich.c
- */
- bus = pci_find_bus(0, 0);
- pci_lock_rescan_remove();
- pci_bus_write_config_byte(bus, p2sb, 0xE1, 0x0);
- pci_bus_read_config_dword(bus, p2sb, PCI_BASE_ADDRESS_0, &bar0);
-
- bar0 &= ~0xf;
- pci_bus_write_config_byte(bus, p2sb, 0xE1, 0x1);
- pci_unlock_rescan_remove();
-
- return bar0;
-}
-EXPORT_SYMBOL(simatic_ipc_get_membase0);
-
static int __init simatic_ipc_init_module(void)
{
const struct dmi_system_id *match;
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index c8fa79da23b3..ce44a942fc68 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1628,6 +1628,7 @@ config SIEMENS_SIMATIC_IPC_WDT
tristate "Siemens Simatic IPC Watchdog"
depends on SIEMENS_SIMATIC_IPC
select WATCHDOG_CORE
+ select P2SB if X86
help
This driver adds support for several watchdogs found in Industrial
PCs from Siemens.
diff --git a/drivers/watchdog/simatic-ipc-wdt.c b/drivers/watchdog/simatic-ipc-wdt.c
index 8bac793c63fb..6599695dc672 100644
--- a/drivers/watchdog/simatic-ipc-wdt.c
+++ b/drivers/watchdog/simatic-ipc-wdt.c
@@ -16,6 +16,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
+#include <linux/platform_data/x86/p2sb.h>
#include <linux/platform_data/x86/simatic-ipc-base.h>
#include <linux/platform_device.h>
#include <linux/sizes.h>
@@ -54,9 +55,9 @@ static struct resource io_resource_trigger =
DEFINE_RES_IO_NAMED(WD_TRIGGER_IOADR, SZ_1,
KBUILD_MODNAME " WD_TRIGGER_IOADR");
-/* the actual start will be discovered with pci, 0 is a placeholder */
+/* the actual start will be discovered with p2sb, 0 is a placeholder */
static struct resource mem_resource =
- DEFINE_RES_MEM_NAMED(0, SZ_4, "WD_RESET_BASE_ADR");
+ DEFINE_RES_MEM_NAMED(0, 0, "WD_RESET_BASE_ADR");
static u32 wd_timeout_table[] = {2, 4, 6, 8, 16, 32, 48, 64 };
static void __iomem *wd_reset_base_addr;
@@ -150,6 +151,7 @@ static int simatic_ipc_wdt_probe(struct platform_device *pdev)
struct simatic_ipc_platform *plat = pdev->dev.platform_data;
struct device *dev = &pdev->dev;
struct resource *res;
+ int ret;
switch (plat->devmode) {
case SIMATIC_IPC_DEVICE_227E:
@@ -190,15 +192,14 @@ static int simatic_ipc_wdt_probe(struct platform_device *pdev)
if (plat->devmode == SIMATIC_IPC_DEVICE_427E) {
res = &mem_resource;
- /* get GPIO base from PCI */
- res->start = simatic_ipc_get_membase0(PCI_DEVFN(0x1f, 1));
- if (res->start == 0)
- return -ENODEV;
+ ret = p2sb_bar(NULL, 0, res);
+ if (ret)
+ return ret;
/* do the final address calculation */
res->start = res->start + (GPIO_COMMUNITY0_PORT_ID << 16) +
PAD_CFG_DW0_GPP_A_23;
- res->end += res->start;
+ res->end = res->start + SZ_4 - 1;
wd_reset_base_addr = devm_ioremap_resource(dev, res);
if (IS_ERR(wd_reset_base_addr))
diff --git a/include/linux/platform_data/x86/simatic-ipc-base.h b/include/linux/platform_data/x86/simatic-ipc-base.h
index 62d2bc774067..39fefd48cf4d 100644
--- a/include/linux/platform_data/x86/simatic-ipc-base.h
+++ b/include/linux/platform_data/x86/simatic-ipc-base.h
@@ -24,6 +24,4 @@ struct simatic_ipc_platform {
u8 devmode;
};
-u32 simatic_ipc_get_membase0(unsigned int p2sb);
-
#endif /* __PLATFORM_DATA_X86_SIMATIC_IPC_BASE_H */
--
2.34.1
Am Tue, 8 Mar 2022 20:35:21 +0100
schrieb Henning Schild <henning.schild@siemens.com>:
> Since we have a common P2SB accessor in tree we may use it instead of
> open coded variants.
>
> Replace custom code by p2sb_bar() call.
>
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
> drivers/leds/simple/Kconfig | 1 +
> drivers/leds/simple/simatic-ipc-leds.c | 14 +++----
> drivers/platform/x86/simatic-ipc.c | 38
> ------------------- drivers/watchdog/Kconfig |
> 1 + drivers/watchdog/simatic-ipc-wdt.c | 15 ++++----
> .../platform_data/x86/simatic-ipc-base.h | 2 -
> 6 files changed, 17 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig
> index 9f6a68336659..9293e6b36c75 100644
> --- a/drivers/leds/simple/Kconfig
> +++ b/drivers/leds/simple/Kconfig
> @@ -3,6 +3,7 @@ config LEDS_SIEMENS_SIMATIC_IPC
> tristate "LED driver for Siemens Simatic IPCs"
> depends on LEDS_CLASS
> depends on SIEMENS_SIMATIC_IPC
> + select P2SB if X86
> help
> This option enables support for the LEDs of several
> Industrial PCs from Siemens.
> diff --git a/drivers/leds/simple/simatic-ipc-leds.c
> b/drivers/leds/simple/simatic-ipc-leds.c index
> ff2c96e73241..215ef5b74236 100644 ---
> a/drivers/leds/simple/simatic-ipc-leds.c +++
> b/drivers/leds/simple/simatic-ipc-leds.c @@ -15,6 +15,7 @@
> #include <linux/leds.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> +#include <linux/platform_data/x86/p2sb.h>
> #include <linux/platform_data/x86/simatic-ipc-base.h>
> #include <linux/platform_device.h>
> #include <linux/sizes.h>
> @@ -38,8 +39,8 @@ static struct simatic_ipc_led simatic_ipc_leds_io[]
> = { { }
> };
>
> -/* the actual start will be discovered with PCI, 0 is a placeholder
> */ -struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0,
> SZ_4K, KBUILD_MODNAME); +/* the actual start will be discovered with
> p2sb, 0 is a placeholder */ +struct resource simatic_ipc_led_mem_res
> = DEFINE_RES_MEM_NAMED(0, 0, KBUILD_MODNAME);
> static void *simatic_ipc_led_memory;
>
> @@ -143,14 +144,13 @@ static int simatic_ipc_leds_probe(struct
> platform_device *pdev) ipcled = simatic_ipc_leds_mem;
> type = IORESOURCE_MEM;
>
> - /* get GPIO base from PCI */
> - res->start = simatic_ipc_get_membase0(PCI_DEVFN(13,
> 0));
> - if (res->start == 0)
> - return -ENODEV;
> + err = p2sb_bar(NULL, 0, res);
> + if (err)
> + return err;
>
> /* do the final address calculation */
> res->start = res->start + (0xC5 << 16);
> - res->end += res->start;
> + res->end = res->start + SZ_4K - 1;
>
> simatic_ipc_led_memory = devm_ioremap_resource(dev,
After "mfd: lpc_ich: Add support for pinctrl in non-ACPI system" this
will fail because the region will be claimed by pinctrl, did not check
if it would work with !CONFIG_LPC_SCH.
But not a big deal and was expected to happen. I do have a version that
will devm_ioremap (no region) but that seems too hacky to put here.
After all the next patch will remove all that and switch to GPIO.
If we have to propose things in two series i propose to make
CONFIG_LPC_SCH and CONFIG_LEDS_SIEMENS_SIMATIC_IPC conflicting for
intermediate steps.
regards,
Henning
> res); if (IS_ERR(simatic_ipc_led_memory))
> diff --git a/drivers/platform/x86/simatic-ipc.c
> b/drivers/platform/x86/simatic-ipc.c index b599cda5ba3c..26c35e1660cb
> 100644 --- a/drivers/platform/x86/simatic-ipc.c
> +++ b/drivers/platform/x86/simatic-ipc.c
> @@ -101,44 +101,6 @@ static int register_platform_devices(u32
> station_id) return 0;
> }
>
> -/* FIXME: this should eventually be done with generic P2SB discovery
> code
> - * the individual drivers for watchdogs and LEDs access memory that
> implements
> - * GPIO, but pinctrl will not come up because of missing ACPI entries
> - *
> - * While there is no conflict a cleaner solution would be to somehow
> bring up
> - * pinctrl even with these ACPI entries missing, and base the
> drivers on pinctrl.
> - * After which the following function could be dropped, together
> with the code
> - * poking the memory.
> - */
> -/*
> - * Get membase address from PCI, used in leds and wdt module. Here
> we read
> - * the bar0. The final address calculation is done in the
> appropriate modules
> - */
> -u32 simatic_ipc_get_membase0(unsigned int p2sb)
> -{
> - struct pci_bus *bus;
> - u32 bar0 = 0;
> - /*
> - * The GPIO memory is in bar0 of the hidden P2SB device.
> - * Unhide the device to have a quick look at it, before we
> hide it
> - * again.
> - * Also grab the pci rescan lock so that device does not get
> discovered
> - * and remapped while it is visible.
> - * This code is inspired by drivers/mfd/lpc_ich.c
> - */
> - bus = pci_find_bus(0, 0);
> - pci_lock_rescan_remove();
> - pci_bus_write_config_byte(bus, p2sb, 0xE1, 0x0);
> - pci_bus_read_config_dword(bus, p2sb, PCI_BASE_ADDRESS_0,
> &bar0); -
> - bar0 &= ~0xf;
> - pci_bus_write_config_byte(bus, p2sb, 0xE1, 0x1);
> - pci_unlock_rescan_remove();
> -
> - return bar0;
> -}
> -EXPORT_SYMBOL(simatic_ipc_get_membase0);
> -
> static int __init simatic_ipc_init_module(void)
> {
> const struct dmi_system_id *match;
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index c8fa79da23b3..ce44a942fc68 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1628,6 +1628,7 @@ config SIEMENS_SIMATIC_IPC_WDT
> tristate "Siemens Simatic IPC Watchdog"
> depends on SIEMENS_SIMATIC_IPC
> select WATCHDOG_CORE
> + select P2SB if X86
> help
> This driver adds support for several watchdogs found in
> Industrial PCs from Siemens.
> diff --git a/drivers/watchdog/simatic-ipc-wdt.c
> b/drivers/watchdog/simatic-ipc-wdt.c index 8bac793c63fb..6599695dc672
> 100644 --- a/drivers/watchdog/simatic-ipc-wdt.c
> +++ b/drivers/watchdog/simatic-ipc-wdt.c
> @@ -16,6 +16,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> +#include <linux/platform_data/x86/p2sb.h>
> #include <linux/platform_data/x86/simatic-ipc-base.h>
> #include <linux/platform_device.h>
> #include <linux/sizes.h>
> @@ -54,9 +55,9 @@ static struct resource io_resource_trigger =
> DEFINE_RES_IO_NAMED(WD_TRIGGER_IOADR, SZ_1,
> KBUILD_MODNAME " WD_TRIGGER_IOADR");
>
> -/* the actual start will be discovered with pci, 0 is a placeholder
> */ +/* the actual start will be discovered with p2sb, 0 is a
> placeholder */ static struct resource mem_resource =
> - DEFINE_RES_MEM_NAMED(0, SZ_4, "WD_RESET_BASE_ADR");
> + DEFINE_RES_MEM_NAMED(0, 0, "WD_RESET_BASE_ADR");
>
> static u32 wd_timeout_table[] = {2, 4, 6, 8, 16, 32, 48, 64 };
> static void __iomem *wd_reset_base_addr;
> @@ -150,6 +151,7 @@ static int simatic_ipc_wdt_probe(struct
> platform_device *pdev) struct simatic_ipc_platform *plat =
> pdev->dev.platform_data; struct device *dev = &pdev->dev;
> struct resource *res;
> + int ret;
>
> switch (plat->devmode) {
> case SIMATIC_IPC_DEVICE_227E:
> @@ -190,15 +192,14 @@ static int simatic_ipc_wdt_probe(struct
> platform_device *pdev) if (plat->devmode == SIMATIC_IPC_DEVICE_427E) {
> res = &mem_resource;
>
> - /* get GPIO base from PCI */
> - res->start =
> simatic_ipc_get_membase0(PCI_DEVFN(0x1f, 1));
> - if (res->start == 0)
> - return -ENODEV;
> + ret = p2sb_bar(NULL, 0, res);
> + if (ret)
> + return ret;
>
> /* do the final address calculation */
> res->start = res->start + (GPIO_COMMUNITY0_PORT_ID
> << 16) + PAD_CFG_DW0_GPP_A_23;
> - res->end += res->start;
> + res->end = res->start + SZ_4 - 1;
>
> wd_reset_base_addr = devm_ioremap_resource(dev, res);
> if (IS_ERR(wd_reset_base_addr))
> diff --git a/include/linux/platform_data/x86/simatic-ipc-base.h
> b/include/linux/platform_data/x86/simatic-ipc-base.h index
> 62d2bc774067..39fefd48cf4d 100644 ---
> a/include/linux/platform_data/x86/simatic-ipc-base.h +++
> b/include/linux/platform_data/x86/simatic-ipc-base.h @@ -24,6 +24,4
> @@ struct simatic_ipc_platform { u8 devmode;
> };
>
> -u32 simatic_ipc_get_membase0(unsigned int p2sb);
> -
> #endif /* __PLATFORM_DATA_X86_SIMATIC_IPC_BASE_H */
On Apollo Lake the pinctrl drivers will now come up without ACPI. Use
that instead of open coding it.
Create a new driver for that which can later be filled with more GPIO
based models, and which has different dependencies.
Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
drivers/leds/simple/Kconfig | 12 ++-
drivers/leds/simple/Makefile | 3 +-
drivers/leds/simple/simatic-ipc-leds-gpio.c | 108 ++++++++++++++++++++
drivers/leds/simple/simatic-ipc-leds.c | 75 +-------------
drivers/platform/x86/simatic-ipc.c | 5 +-
5 files changed, 129 insertions(+), 74 deletions(-)
create mode 100644 drivers/leds/simple/simatic-ipc-leds-gpio.c
diff --git a/drivers/leds/simple/Kconfig b/drivers/leds/simple/Kconfig
index 9293e6b36c75..9d2487908743 100644
--- a/drivers/leds/simple/Kconfig
+++ b/drivers/leds/simple/Kconfig
@@ -3,10 +3,20 @@ config LEDS_SIEMENS_SIMATIC_IPC
tristate "LED driver for Siemens Simatic IPCs"
depends on LEDS_CLASS
depends on SIEMENS_SIMATIC_IPC
- select P2SB if X86
help
This option enables support for the LEDs of several Industrial PCs
from Siemens.
To compile this driver as a module, choose M here: the module
will be called simatic-ipc-leds.
+
+config LEDS_SIEMENS_SIMATIC_IPC_GPIO
+ tristate "LED driver for Siemens Simatic IPCs, GPIO based"
+ depends on SIEMENS_SIMATIC_IPC
+ depends on LEDS_GPIO
+ help
+ This option enables support for the LEDs of several Industrial PCs
+ from Siemens.
+
+ To compile this driver as a module, choose M here: the module
+ will be called simatic-ipc-leds-gpio.
diff --git a/drivers/leds/simple/Makefile b/drivers/leds/simple/Makefile
index 8481f1e9e360..e1df74fb5915 100644
--- a/drivers/leds/simple/Makefile
+++ b/drivers/leds/simple/Makefile
@@ -1,2 +1,3 @@
# SPDX-License-Identifier: GPL-2.0
-obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC) += simatic-ipc-leds.o
+obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC) += simatic-ipc-leds.o
+obj-$(CONFIG_LEDS_SIEMENS_SIMATIC_IPC_GPIO) += simatic-ipc-leds-gpio.o
diff --git a/drivers/leds/simple/simatic-ipc-leds-gpio.c b/drivers/leds/simple/simatic-ipc-leds-gpio.c
new file mode 100644
index 000000000000..552b65a72e04
--- /dev/null
+++ b/drivers/leds/simple/simatic-ipc-leds-gpio.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Siemens SIMATIC IPC driver for GPIO based LEDs
+ *
+ * Copyright (c) Siemens AG, 2022
+ *
+ * Authors:
+ * Henning Schild <henning.schild@siemens.com>
+ */
+
+#include <linux/gpio/machine.h>
+#include <linux/gpio/consumer.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+static struct gpiod_lookup_table simatic_ipc_led_gpio_table = {
+ .dev_id = "leds-gpio",
+ .table = {
+ GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 51, NULL, 0, GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 52, NULL, 1, GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 53, NULL, 2, GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 57, NULL, 3, GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 58, NULL, 4, GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 60, NULL, 5, GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 56, NULL, 6, GPIO_ACTIVE_LOW),
+ GPIO_LOOKUP_IDX("apollolake-pinctrl.0", 59, NULL, 7, GPIO_ACTIVE_HIGH),
+ }
+};
+
+static const struct gpio_led simatic_ipc_gpio_leds[] = {
+ { .name = "green:" LED_FUNCTION_STATUS "-3" },
+ { .name = "red:" LED_FUNCTION_STATUS "-1" },
+ { .name = "green:" LED_FUNCTION_STATUS "-1" },
+ { .name = "red:" LED_FUNCTION_STATUS "-2" },
+ { .name = "green:" LED_FUNCTION_STATUS "-2" },
+ { .name = "red:" LED_FUNCTION_STATUS "-3" },
+};
+
+static const struct gpio_led_platform_data simatic_ipc_gpio_leds_pdata = {
+ .num_leds = ARRAY_SIZE(simatic_ipc_gpio_leds),
+ .leds = simatic_ipc_gpio_leds,
+};
+
+static struct platform_device *simatic_leds_pdev;
+
+static int simatic_ipc_leds_gpio_remove(struct platform_device *pdev)
+{
+ gpiod_remove_lookup_table(&simatic_ipc_led_gpio_table);
+ platform_device_unregister(simatic_leds_pdev);
+
+ return 0;
+}
+
+static int simatic_ipc_leds_gpio_probe(struct platform_device *pdev)
+{
+ struct gpio_desc *gpiod;
+ int err;
+
+ gpiod_add_lookup_table(&simatic_ipc_led_gpio_table);
+ simatic_leds_pdev = platform_device_register_resndata(NULL,
+ "leds-gpio", PLATFORM_DEVID_NONE, NULL, 0,
+ &simatic_ipc_gpio_leds_pdata,
+ sizeof(simatic_ipc_gpio_leds_pdata));
+ if (IS_ERR(simatic_leds_pdev)) {
+ err = PTR_ERR(simatic_leds_pdev);
+ goto out;
+ }
+
+ /* PM_BIOS_BOOT_N */
+ gpiod = gpiod_get_index(&simatic_leds_pdev->dev, NULL, 6, 0);
+ if (IS_ERR(gpiod)) {
+ err = PTR_ERR(gpiod);
+ goto out;
+ }
+ gpiod_set_value(gpiod, 0);
+ gpiod_put(gpiod);
+
+ /* PM_WDT_OUT */
+ gpiod = gpiod_get_index(&simatic_leds_pdev->dev, NULL, 7, 0);
+ if (IS_ERR(gpiod)) {
+ err = PTR_ERR(gpiod);
+ goto out;
+ }
+ gpiod_set_value(gpiod, 0);
+ gpiod_put(gpiod);
+
+ return 0;
+out:
+ simatic_ipc_leds_gpio_remove(pdev);
+
+ return err;
+}
+
+static struct platform_driver simatic_ipc_led_gpio_driver = {
+ .probe = simatic_ipc_leds_gpio_probe,
+ .remove = simatic_ipc_leds_gpio_remove,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ }
+};
+
+module_platform_driver(simatic_ipc_led_gpio_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" KBUILD_MODNAME);
+MODULE_SOFTDEP("pre: platform:leds-gpio");
+MODULE_AUTHOR("Henning Schild <henning.schild@siemens.com>");
diff --git a/drivers/leds/simple/simatic-ipc-leds.c b/drivers/leds/simple/simatic-ipc-leds.c
index 215ef5b74236..0f0dee053aa1 100644
--- a/drivers/leds/simple/simatic-ipc-leds.c
+++ b/drivers/leds/simple/simatic-ipc-leds.c
@@ -24,7 +24,7 @@
#define SIMATIC_IPC_LED_PORT_BASE 0x404E
struct simatic_ipc_led {
- unsigned int value; /* mask for io and offset for mem */
+ unsigned int value; /* mask for io */
char *name;
struct led_classdev cdev;
};
@@ -39,21 +39,6 @@ static struct simatic_ipc_led simatic_ipc_leds_io[] = {
{ }
};
-/* the actual start will be discovered with p2sb, 0 is a placeholder */
-struct resource simatic_ipc_led_mem_res = DEFINE_RES_MEM_NAMED(0, 0, KBUILD_MODNAME);
-
-static void *simatic_ipc_led_memory;
-
-static struct simatic_ipc_led simatic_ipc_leds_mem[] = {
- {0x500 + 0x1A0, "red:" LED_FUNCTION_STATUS "-1"},
- {0x500 + 0x1A8, "green:" LED_FUNCTION_STATUS "-1"},
- {0x500 + 0x1C8, "red:" LED_FUNCTION_STATUS "-2"},
- {0x500 + 0x1D0, "green:" LED_FUNCTION_STATUS "-2"},
- {0x500 + 0x1E0, "red:" LED_FUNCTION_STATUS "-3"},
- {0x500 + 0x198, "green:" LED_FUNCTION_STATUS "-3"},
- { }
-};
-
static struct resource simatic_ipc_led_io_res =
DEFINE_RES_IO_NAMED(SIMATIC_IPC_LED_PORT_BASE, SZ_2, KBUILD_MODNAME);
@@ -89,27 +74,6 @@ static enum led_brightness simatic_ipc_led_get_io(struct led_classdev *led_cd)
return inw(SIMATIC_IPC_LED_PORT_BASE) & led->value ? LED_OFF : led_cd->max_brightness;
}
-static void simatic_ipc_led_set_mem(struct led_classdev *led_cd,
- enum led_brightness brightness)
-{
- struct simatic_ipc_led *led = cdev_to_led(led_cd);
-
- u32 *p;
-
- p = simatic_ipc_led_memory + led->value;
- *p = (*p & ~1) | (brightness == LED_OFF);
-}
-
-static enum led_brightness simatic_ipc_led_get_mem(struct led_classdev *led_cd)
-{
- struct simatic_ipc_led *led = cdev_to_led(led_cd);
-
- u32 *p;
-
- p = simatic_ipc_led_memory + led->value;
- return (*p & 1) ? LED_OFF : led_cd->max_brightness;
-}
-
static int simatic_ipc_leds_probe(struct platform_device *pdev)
{
const struct simatic_ipc_platform *plat = pdev->dev.platform_data;
@@ -117,8 +81,7 @@ static int simatic_ipc_leds_probe(struct platform_device *pdev)
struct simatic_ipc_led *ipcled;
struct led_classdev *cdev;
struct resource *res;
- int err, type;
- u32 *p;
+ int err;
switch (plat->devmode) {
case SIMATIC_IPC_DEVICE_227D:
@@ -133,35 +96,10 @@ static int simatic_ipc_leds_probe(struct platform_device *pdev)
}
ipcled = simatic_ipc_leds_io;
}
- type = IORESOURCE_IO;
if (!devm_request_region(dev, res->start, resource_size(res), KBUILD_MODNAME)) {
dev_err(dev, "Unable to register IO resource at %pR\n", res);
return -EBUSY;
}
- break;
- case SIMATIC_IPC_DEVICE_127E:
- res = &simatic_ipc_led_mem_res;
- ipcled = simatic_ipc_leds_mem;
- type = IORESOURCE_MEM;
-
- err = p2sb_bar(NULL, 0, res);
- if (err)
- return err;
-
- /* do the final address calculation */
- res->start = res->start + (0xC5 << 16);
- res->end = res->start + SZ_4K - 1;
-
- simatic_ipc_led_memory = devm_ioremap_resource(dev, res);
- if (IS_ERR(simatic_ipc_led_memory))
- return PTR_ERR(simatic_ipc_led_memory);
-
- /* initialize power/watchdog LED */
- p = simatic_ipc_led_memory + 0x500 + 0x1D8; /* PM_WDT_OUT */
- *p = (*p & ~1);
- p = simatic_ipc_led_memory + 0x500 + 0x1C0; /* PM_BIOS_BOOT_N */
- *p = (*p | 1);
-
break;
default:
return -ENODEV;
@@ -169,13 +107,8 @@ static int simatic_ipc_leds_probe(struct platform_device *pdev)
while (ipcled->value) {
cdev = &ipcled->cdev;
- if (type == IORESOURCE_MEM) {
- cdev->brightness_set = simatic_ipc_led_set_mem;
- cdev->brightness_get = simatic_ipc_led_get_mem;
- } else {
- cdev->brightness_set = simatic_ipc_led_set_io;
- cdev->brightness_get = simatic_ipc_led_get_io;
- }
+ cdev->brightness_set = simatic_ipc_led_set_io;
+ cdev->brightness_get = simatic_ipc_led_get_io;
cdev->max_brightness = LED_ON;
cdev->name = ipcled->name;
diff --git a/drivers/platform/x86/simatic-ipc.c b/drivers/platform/x86/simatic-ipc.c
index 26c35e1660cb..ca3647b751d5 100644
--- a/drivers/platform/x86/simatic-ipc.c
+++ b/drivers/platform/x86/simatic-ipc.c
@@ -51,6 +51,7 @@ static int register_platform_devices(u32 station_id)
{
u8 ledmode = SIMATIC_IPC_DEVICE_NONE;
u8 wdtmode = SIMATIC_IPC_DEVICE_NONE;
+ char *pdevname = KBUILD_MODNAME "_leds";
int i;
platform_data.devmode = SIMATIC_IPC_DEVICE_NONE;
@@ -64,10 +65,12 @@ static int register_platform_devices(u32 station_id)
}
if (ledmode != SIMATIC_IPC_DEVICE_NONE) {
+ if (ledmode == SIMATIC_IPC_DEVICE_127E)
+ pdevname = KBUILD_MODNAME "_leds_gpio";
platform_data.devmode = ledmode;
ipc_led_platform_device =
platform_device_register_data(NULL,
- KBUILD_MODNAME "_leds", PLATFORM_DEVID_NONE,
+ pdevname, PLATFORM_DEVID_NONE,
&platform_data,
sizeof(struct simatic_ipc_platform));
if (IS_ERR(ipc_led_platform_device))
--
2.34.1
Am Mon, 31 Jan 2022 17:13:38 +0200
schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> There are a few users and at least one more is coming (*) that would
> like to utilize P2SB mechanism of hiding and unhiding a device from
> the PCI configuration space.
>
> Here is the series to consolidate p2sb handling code for existing
> users and provide a generic way for new comer(s).
>
> It also includes a patch to enable GPIO controllers on Apollo Lake
> when it's used with ABL bootloader w/o ACPI support.
>
> The patch that bring the helper ("platform/x86/intel: Add Primary
> to Sideband (P2SB) bridge support") has a commit message that
> sheds a light on what the P2SB is and why this is needed.
>
> The changes made in v2 do not change the main idea and the
> functionality in a big scale. What we need is probably one more
> (RE-)test done by Henning.
Just actually read all this the first time, sorry for that! Will take
care of testing on a few of those Simatic IPCs and write patches to
switch to P2SB ... possibly pinctrl where that might pop up without
ACPI.
I guess p5 will make pinctrl show up on what i would call a 127E ...
apollo lake.
> I hope to have it merged to v5.18-rc1 that
> Siemens can develop their changes based on this series.
Will do.
Sorry for the delay after having passed you i now seem to slow you down.
Henning
> I have tested this on Apollo Lake platform (I'm able to see SPI NOR
> and since we have an ACPI device for GPIO I do not see any attempts
> to recreate one).
>
> *) One in this series, and one is a due after merge in the Simatic
> IPC drivers
>
> The series may be routed either via MFD (and I guess Lee would prefer
> that) or via PDx86, whichever seems better for you, folks. As of
> today patches are ACKed by the respective maintainers, but I2C one
> and one of the MFD.
>
> Wolfram, can you ACK the patch against i2c-i801 driver, if you have no
> objections?
>
> Changes in v4:
> - added tag to the entire series (Hans)
> - added tag to pin control patch (Mika)
> - dropped PCI core changes (PCI core doesn't want modifications to be
> made)
> - as a consequence of the above merged necessary bits into p2sb.c
> - added a check that p2sb is really hidden (Hans)
> - added EDAC patches (reviewed by maintainer internally)
>
> Changes in v3:
> - resent with cover letter
>
> Changes in v2:
> - added parentheses around bus in macros (Joe)
> - added tag (Jean)
> - fixed indentation and wrapping in the header (Christoph)
> - moved out of PCI realm to PDx86 as the best common denominator
> (Bjorn)
> - added a verbose commit message to explain P2SB thingy (Bjorn)
> - converted first parameter from pci_dev to pci_bus
> - made first two parameters (bus and devfn) optional (Henning, Lee)
> - added Intel pin control patch to the series (Henning, Mika)
> - fixed English style in the commit message of one of MFD patch (Lee)
> - added tags to my MFD LPC ICH patches (Lee)
> - used consistently (c) (Lee)
> - made indexing for MFD cell and resource arrays (Lee)
> - fixed the resource size in i801 (Jean)
>
> Andy Shevchenko (6):
> pinctrl: intel: Check against matching data instead of ACPI
> companion mfd: lpc_ich: Factor out lpc_ich_enable_spi_write()
> mfd: lpc_ich: Switch to generic p2sb_bar()
> i2c: i801: convert to use common P2SB accessor
> EDAC, pnd2: Use proper I/O accessors and address space annotation
> EDAC, pnd2: convert to use common P2SB accessor
>
> Jonathan Yong (1):
> platform/x86/intel: Add Primary to Sideband (P2SB) bridge support
>
> Tan Jui Nee (1):
> mfd: lpc_ich: Add support for pinctrl in non-ACPI system
>
> drivers/edac/Kconfig | 1 +
> drivers/edac/pnd2_edac.c | 62 ++---
> drivers/i2c/busses/Kconfig | 1 +
> drivers/i2c/busses/i2c-i801.c | 39 +---
> drivers/mfd/Kconfig | 1 +
> drivers/mfd/lpc_ich.c | 136 +++++++++--
> drivers/pinctrl/intel/pinctrl-intel.c | 14 +-
> drivers/platform/x86/intel/Kconfig | 12 +
> drivers/platform/x86/intel/Makefile | 1 +
> drivers/platform/x86/intel/p2sb.c | 305
> +++++++++++++++++++++++++ include/linux/platform_data/x86/p2sb.h |
> 27 +++ 11 files changed, 500 insertions(+), 99 deletions(-)
> create mode 100644 drivers/platform/x86/intel/p2sb.c
> create mode 100644 include/linux/platform_data/x86/p2sb.h
>
Am Mon, 31 Jan 2022 17:13:38 +0200
schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> There are a few users and at least one more is coming (*) that would
> like to utilize P2SB mechanism of hiding and unhiding a device from
> the PCI configuration space.
>
> Here is the series to consolidate p2sb handling code for existing
> users and provide a generic way for new comer(s).
>
> It also includes a patch to enable GPIO controllers on Apollo Lake
> when it's used with ABL bootloader w/o ACPI support.
>
> The patch that bring the helper ("platform/x86/intel: Add Primary
> to Sideband (P2SB) bridge support") has a commit message that
> sheds a light on what the P2SB is and why this is needed.
>
> The changes made in v2 do not change the main idea and the
> functionality in a big scale. What we need is probably one more
> (RE-)test done by Henning. I hope to have it merged to v5.18-rc1 that
> Siemens can develop their changes based on this series.
I did test this series and it works as expected. Only problem is that
the leds driver will not work together with the pinctrl. Because two
"in tree drivers" will try to reserve the same memory region when both
are enabled. Who wins is a matter of probing order ...
If you can take my changes into your series we will not have a problem.
Otherwise we might need to create sort of a conflict which my series
would revert when switching apl lake to gpio.
I would not know the process, let us see what the reviews bring and how
to continue here.
Thanks so much for taking care, especially the pinctrl coming up
without ACPI really improves the simatic leds on the apl lake.
In fact i will have to double check if i really need the p2sb for the
427E wdt ... but until i have an answer, p2sb works just fine.
regards,
Henning
> I have tested this on Apollo Lake platform (I'm able to see SPI NOR
> and since we have an ACPI device for GPIO I do not see any attempts
> to recreate one).
>
> *) One in this series, and one is a due after merge in the Simatic
> IPC drivers
>
> The series may be routed either via MFD (and I guess Lee would prefer
> that) or via PDx86, whichever seems better for you, folks. As of
> today patches are ACKed by the respective maintainers, but I2C one
> and one of the MFD.
>
> Wolfram, can you ACK the patch against i2c-i801 driver, if you have no
> objections?
>
> Changes in v4:
> - added tag to the entire series (Hans)
> - added tag to pin control patch (Mika)
> - dropped PCI core changes (PCI core doesn't want modifications to be
> made)
> - as a consequence of the above merged necessary bits into p2sb.c
> - added a check that p2sb is really hidden (Hans)
> - added EDAC patches (reviewed by maintainer internally)
>
> Changes in v3:
> - resent with cover letter
>
> Changes in v2:
> - added parentheses around bus in macros (Joe)
> - added tag (Jean)
> - fixed indentation and wrapping in the header (Christoph)
> - moved out of PCI realm to PDx86 as the best common denominator
> (Bjorn)
> - added a verbose commit message to explain P2SB thingy (Bjorn)
> - converted first parameter from pci_dev to pci_bus
> - made first two parameters (bus and devfn) optional (Henning, Lee)
> - added Intel pin control patch to the series (Henning, Mika)
> - fixed English style in the commit message of one of MFD patch (Lee)
> - added tags to my MFD LPC ICH patches (Lee)
> - used consistently (c) (Lee)
> - made indexing for MFD cell and resource arrays (Lee)
> - fixed the resource size in i801 (Jean)
>
> Andy Shevchenko (6):
> pinctrl: intel: Check against matching data instead of ACPI
> companion mfd: lpc_ich: Factor out lpc_ich_enable_spi_write()
> mfd: lpc_ich: Switch to generic p2sb_bar()
> i2c: i801: convert to use common P2SB accessor
> EDAC, pnd2: Use proper I/O accessors and address space annotation
> EDAC, pnd2: convert to use common P2SB accessor
>
> Jonathan Yong (1):
> platform/x86/intel: Add Primary to Sideband (P2SB) bridge support
>
> Tan Jui Nee (1):
> mfd: lpc_ich: Add support for pinctrl in non-ACPI system
>
> drivers/edac/Kconfig | 1 +
> drivers/edac/pnd2_edac.c | 62 ++---
> drivers/i2c/busses/Kconfig | 1 +
> drivers/i2c/busses/i2c-i801.c | 39 +---
> drivers/mfd/Kconfig | 1 +
> drivers/mfd/lpc_ich.c | 136 +++++++++--
> drivers/pinctrl/intel/pinctrl-intel.c | 14 +-
> drivers/platform/x86/intel/Kconfig | 12 +
> drivers/platform/x86/intel/Makefile | 1 +
> drivers/platform/x86/intel/p2sb.c | 305
> +++++++++++++++++++++++++ include/linux/platform_data/x86/p2sb.h |
> 27 +++ 11 files changed, 500 insertions(+), 99 deletions(-)
> create mode 100644 drivers/platform/x86/intel/p2sb.c
> create mode 100644 include/linux/platform_data/x86/p2sb.h
>
On Tue, Mar 08, 2022 at 08:50:16PM +0100, Henning Schild wrote:
> Am Mon, 31 Jan 2022 17:13:38 +0200
> schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
>
> > There are a few users and at least one more is coming (*) that would
> > like to utilize P2SB mechanism of hiding and unhiding a device from
> > the PCI configuration space.
> >
> > Here is the series to consolidate p2sb handling code for existing
> > users and provide a generic way for new comer(s).
> >
> > It also includes a patch to enable GPIO controllers on Apollo Lake
> > when it's used with ABL bootloader w/o ACPI support.
> >
> > The patch that bring the helper ("platform/x86/intel: Add Primary
> > to Sideband (P2SB) bridge support") has a commit message that
> > sheds a light on what the P2SB is and why this is needed.
> >
> > The changes made in v2 do not change the main idea and the
> > functionality in a big scale. What we need is probably one more
> > (RE-)test done by Henning. I hope to have it merged to v5.18-rc1 that
> > Siemens can develop their changes based on this series.
>
> I did test this series and it works as expected. Only problem is that
> the leds driver will not work together with the pinctrl. Because two
> "in tree drivers" will try to reserve the same memory region when both
> are enabled. Who wins is a matter of probing order ...
Can we have your formal Tested-by tag?
> If you can take my changes into your series we will not have a problem.
Yes, that's the plan, but your patches needs a bit of work I believe.
> Otherwise we might need to create sort of a conflict which my series
> would revert when switching apl lake to gpio.
>
> I would not know the process, let us see what the reviews bring and how
> to continue here.
I'm about to comment on the patches.
> Thanks so much for taking care, especially the pinctrl coming up
> without ACPI really improves the simatic leds on the apl lake.
You are welcome!
> In fact i will have to double check if i really need the p2sb for the
> 427E wdt ... but until i have an answer, p2sb works just fine.
Thanks!
> > I have tested this on Apollo Lake platform (I'm able to see SPI NOR
> > and since we have an ACPI device for GPIO I do not see any attempts
> > to recreate one).
> >
> > *) One in this series, and one is a due after merge in the Simatic
> > IPC drivers
> >
> > The series may be routed either via MFD (and I guess Lee would prefer
> > that) or via PDx86, whichever seems better for you, folks. As of
> > today patches are ACKed by the respective maintainers, but I2C one
> > and one of the MFD.
> >
> > Wolfram, can you ACK the patch against i2c-i801 driver, if you have no
> > objections?
> >
> > Changes in v4:
> > - added tag to the entire series (Hans)
> > - added tag to pin control patch (Mika)
> > - dropped PCI core changes (PCI core doesn't want modifications to be
> > made)
> > - as a consequence of the above merged necessary bits into p2sb.c
> > - added a check that p2sb is really hidden (Hans)
> > - added EDAC patches (reviewed by maintainer internally)
> >
> > Changes in v3:
> > - resent with cover letter
> >
> > Changes in v2:
> > - added parentheses around bus in macros (Joe)
> > - added tag (Jean)
> > - fixed indentation and wrapping in the header (Christoph)
> > - moved out of PCI realm to PDx86 as the best common denominator
> > (Bjorn)
> > - added a verbose commit message to explain P2SB thingy (Bjorn)
> > - converted first parameter from pci_dev to pci_bus
> > - made first two parameters (bus and devfn) optional (Henning, Lee)
> > - added Intel pin control patch to the series (Henning, Mika)
> > - fixed English style in the commit message of one of MFD patch (Lee)
> > - added tags to my MFD LPC ICH patches (Lee)
> > - used consistently (c) (Lee)
> > - made indexing for MFD cell and resource arrays (Lee)
> > - fixed the resource size in i801 (Jean)
> >
> > Andy Shevchenko (6):
> > pinctrl: intel: Check against matching data instead of ACPI
> > companion mfd: lpc_ich: Factor out lpc_ich_enable_spi_write()
> > mfd: lpc_ich: Switch to generic p2sb_bar()
> > i2c: i801: convert to use common P2SB accessor
> > EDAC, pnd2: Use proper I/O accessors and address space annotation
> > EDAC, pnd2: convert to use common P2SB accessor
> >
> > Jonathan Yong (1):
> > platform/x86/intel: Add Primary to Sideband (P2SB) bridge support
> >
> > Tan Jui Nee (1):
> > mfd: lpc_ich: Add support for pinctrl in non-ACPI system
> >
> > drivers/edac/Kconfig | 1 +
> > drivers/edac/pnd2_edac.c | 62 ++---
> > drivers/i2c/busses/Kconfig | 1 +
> > drivers/i2c/busses/i2c-i801.c | 39 +---
> > drivers/mfd/Kconfig | 1 +
> > drivers/mfd/lpc_ich.c | 136 +++++++++--
> > drivers/pinctrl/intel/pinctrl-intel.c | 14 +-
> > drivers/platform/x86/intel/Kconfig | 12 +
> > drivers/platform/x86/intel/Makefile | 1 +
> > drivers/platform/x86/intel/p2sb.c | 305
> > +++++++++++++++++++++++++ include/linux/platform_data/x86/p2sb.h |
> > 27 +++ 11 files changed, 500 insertions(+), 99 deletions(-)
> > create mode 100644 drivers/platform/x86/intel/p2sb.c
> > create mode 100644 include/linux/platform_data/x86/p2sb.h
> >
>
--
With Best Regards,
Andy Shevchenko
Am Wed, 4 May 2022 15:42:29 +0300
schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> On Tue, Mar 08, 2022 at 08:50:16PM +0100, Henning Schild wrote:
> > Am Mon, 31 Jan 2022 17:13:38 +0200
> > schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> >
> > > There are a few users and at least one more is coming (*) that
> > > would like to utilize P2SB mechanism of hiding and unhiding a
> > > device from the PCI configuration space.
> > >
> > > Here is the series to consolidate p2sb handling code for existing
> > > users and provide a generic way for new comer(s).
> > >
> > > It also includes a patch to enable GPIO controllers on Apollo Lake
> > > when it's used with ABL bootloader w/o ACPI support.
> > >
> > > The patch that bring the helper ("platform/x86/intel: Add Primary
> > > to Sideband (P2SB) bridge support") has a commit message that
> > > sheds a light on what the P2SB is and why this is needed.
> > >
> > > The changes made in v2 do not change the main idea and the
> > > functionality in a big scale. What we need is probably one more
> > > (RE-)test done by Henning. I hope to have it merged to v5.18-rc1
> > > that Siemens can develop their changes based on this series.
> >
> > I did test this series and it works as expected. Only problem is
> > that the leds driver will not work together with the pinctrl.
> > Because two "in tree drivers" will try to reserve the same memory
> > region when both are enabled. Who wins is a matter of probing order
> > ...
>
> Can we have your formal Tested-by tag?
Sure. I will just put it here for you to take.
Tested-by: Henning Schild <henning.schild@siemens.com>
Let me know if you need it in another shape or form.
Henning
>
> > If you can take my changes into your series we will not have a
> > problem.
>
> Yes, that's the plan, but your patches needs a bit of work I believe.
>
> > Otherwise we might need to create sort of a conflict which my series
> > would revert when switching apl lake to gpio.
> >
> > I would not know the process, let us see what the reviews bring and
> > how to continue here.
>
> I'm about to comment on the patches.
>
> > Thanks so much for taking care, especially the pinctrl coming up
> > without ACPI really improves the simatic leds on the apl lake.
>
> You are welcome!
>
> > In fact i will have to double check if i really need the p2sb for
> > the 427E wdt ... but until i have an answer, p2sb works just fine.
>
> Thanks!
>
> > > I have tested this on Apollo Lake platform (I'm able to see SPI
> > > NOR and since we have an ACPI device for GPIO I do not see any
> > > attempts to recreate one).
> > >
> > > *) One in this series, and one is a due after merge in the Simatic
> > > IPC drivers
> > >
> > > The series may be routed either via MFD (and I guess Lee would
> > > prefer that) or via PDx86, whichever seems better for you, folks.
> > > As of today patches are ACKed by the respective maintainers, but
> > > I2C one and one of the MFD.
> > >
> > > Wolfram, can you ACK the patch against i2c-i801 driver, if you
> > > have no objections?
> > >
> > > Changes in v4:
> > > - added tag to the entire series (Hans)
> > > - added tag to pin control patch (Mika)
> > > - dropped PCI core changes (PCI core doesn't want modifications
> > > to be made)
> > > - as a consequence of the above merged necessary bits into p2sb.c
> > > - added a check that p2sb is really hidden (Hans)
> > > - added EDAC patches (reviewed by maintainer internally)
> > >
> > > Changes in v3:
> > > - resent with cover letter
> > >
> > > Changes in v2:
> > > - added parentheses around bus in macros (Joe)
> > > - added tag (Jean)
> > > - fixed indentation and wrapping in the header (Christoph)
> > > - moved out of PCI realm to PDx86 as the best common denominator
> > > (Bjorn)
> > > - added a verbose commit message to explain P2SB thingy (Bjorn)
> > > - converted first parameter from pci_dev to pci_bus
> > > - made first two parameters (bus and devfn) optional (Henning,
> > > Lee)
> > > - added Intel pin control patch to the series (Henning, Mika)
> > > - fixed English style in the commit message of one of MFD patch
> > > (Lee)
> > > - added tags to my MFD LPC ICH patches (Lee)
> > > - used consistently (c) (Lee)
> > > - made indexing for MFD cell and resource arrays (Lee)
> > > - fixed the resource size in i801 (Jean)
> > >
> > > Andy Shevchenko (6):
> > > pinctrl: intel: Check against matching data instead of ACPI
> > > companion mfd: lpc_ich: Factor out lpc_ich_enable_spi_write()
> > > mfd: lpc_ich: Switch to generic p2sb_bar()
> > > i2c: i801: convert to use common P2SB accessor
> > > EDAC, pnd2: Use proper I/O accessors and address space
> > > annotation EDAC, pnd2: convert to use common P2SB accessor
> > >
> > > Jonathan Yong (1):
> > > platform/x86/intel: Add Primary to Sideband (P2SB) bridge
> > > support
> > >
> > > Tan Jui Nee (1):
> > > mfd: lpc_ich: Add support for pinctrl in non-ACPI system
> > >
> > > drivers/edac/Kconfig | 1 +
> > > drivers/edac/pnd2_edac.c | 62 ++---
> > > drivers/i2c/busses/Kconfig | 1 +
> > > drivers/i2c/busses/i2c-i801.c | 39 +---
> > > drivers/mfd/Kconfig | 1 +
> > > drivers/mfd/lpc_ich.c | 136 +++++++++--
> > > drivers/pinctrl/intel/pinctrl-intel.c | 14 +-
> > > drivers/platform/x86/intel/Kconfig | 12 +
> > > drivers/platform/x86/intel/Makefile | 1 +
> > > drivers/platform/x86/intel/p2sb.c | 305
> > > +++++++++++++++++++++++++ include/linux/platform_data/x86/p2sb.h |
> > > 27 +++ 11 files changed, 500 insertions(+), 99 deletions(-)
> > > create mode 100644 drivers/platform/x86/intel/p2sb.c
> > > create mode 100644 include/linux/platform_data/x86/p2sb.h
> > >
> >
>
On Wed, May 4, 2022 at 5:10 PM Henning Schild <henning.schild@siemens.com> wrote: > Am Wed, 4 May 2022 15:42:29 +0300 > schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>: > > On Tue, Mar 08, 2022 at 08:50:16PM +0100, Henning Schild wrote: ... > > Can we have your formal Tested-by tag? > > Sure. I will just put it here for you to take. > > Tested-by: Henning Schild <henning.schild@siemens.com> Thank you! > Let me know if you need it in another shape or form. Form is okay, just would be nice if you can retest what I have in the branch as an updated version. -- With Best Regards, Andy Shevchenko
On Wed, May 04, 2022 at 05:55:26PM +0200, Andy Shevchenko wrote: > On Wed, May 4, 2022 at 5:10 PM Henning Schild > <henning.schild@siemens.com> wrote: > > Am Wed, 4 May 2022 15:42:29 +0300 > > schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>: > ... > > Let me know if you need it in another shape or form. > > Form is okay, just would be nice if you can retest what I have in the > branch as an updated version. Is there any news? I would like to send a new version sooner than later. We may also apply the patches from this series and when you will be ready apply yours. -- With Best Regards, Andy Shevchenko
© 2016 - 2026 Red Hat, Inc.