| Message ID | 20200302133327.55929-1-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | platform/x86: Rework intel_scu_ipc and intel_pmc_ipc drivers | expand |
On Mon, 02 Mar 2020, Mika Westerberg wrote: > Hi all, > > Currently both intel_scu_ipc.c and intel_pmc_ipc.c implement the same SCU > IPC communications with minor differences. This duplication does not make > much sense so this series reworks the two drivers so that there is only a > single implementation of the SCU IPC. In addition to that the API will be > updated to take SCU instance pointer as an argument, and most of the > callers will be converted to this new API. The old API is left there but > the plan is to get rid the callers and then the old API as well (this is > something we are working with Andy Shevchenko). > > The intel_pmc_ipc.c is then moved under MFD which suits better for this > kind of a driver that pretty much sets up the SCU IPC and then creates a > bunch of platform devices for the things sitting behind the PMC. The driver > is renamed to intel_pmc_bxt.c which should follow the existing conventions > under drivers/mfd (and it is only meant for Intel Broxton derivatives). > > This is on top of platform-driver-x86.git/for-next branch because there is > already some cleanup work queued that re-organizes Kconfig and Makefile > entries. > > I have tested this on Intel Joule (Broxton-M) board. > > Changes from v6: > > * Added Reviewed-by tag from Andy > * Expanded PMC, IPC and IA acronyms > * Drop TCO_DEVICE_NAME, PUNIT_DEVICE_NAME and TELEMETRY_DEVICE_NAME > * Move struct intel_pmc_dev into include/linux/mfd/intel_pmc_bxt.h > * Add PMC_DEVICE_MAX to the enum and use it > * Add kernel-docs for simplecmd_store() and northpeak_store() > * Use if (ret) return ret; over the ternary operator > * Drop "This is index X" from comments > * Use acpi_has_watchdog() to determine whether iTCO_wdt is added or not. > * Rename intel_scu_ipc_pdata -> intel_scu_ipc_data to make it less > confusing wrt. platform data for platform drivers. Any reason why you've dropped all my tags?
On Mon, Mar 02, 2020 at 02:26:21PM +0000, Lee Jones wrote: > On Mon, 02 Mar 2020, Mika Westerberg wrote: > > > Hi all, > > > > Currently both intel_scu_ipc.c and intel_pmc_ipc.c implement the same SCU > > IPC communications with minor differences. This duplication does not make > > much sense so this series reworks the two drivers so that there is only a > > single implementation of the SCU IPC. In addition to that the API will be > > updated to take SCU instance pointer as an argument, and most of the > > callers will be converted to this new API. The old API is left there but > > the plan is to get rid the callers and then the old API as well (this is > > something we are working with Andy Shevchenko). > > > > The intel_pmc_ipc.c is then moved under MFD which suits better for this > > kind of a driver that pretty much sets up the SCU IPC and then creates a > > bunch of platform devices for the things sitting behind the PMC. The driver > > is renamed to intel_pmc_bxt.c which should follow the existing conventions > > under drivers/mfd (and it is only meant for Intel Broxton derivatives). > > > > This is on top of platform-driver-x86.git/for-next branch because there is > > already some cleanup work queued that re-organizes Kconfig and Makefile > > entries. > > > > I have tested this on Intel Joule (Broxton-M) board. > > > > Changes from v6: > > > > * Added Reviewed-by tag from Andy > > * Expanded PMC, IPC and IA acronyms > > * Drop TCO_DEVICE_NAME, PUNIT_DEVICE_NAME and TELEMETRY_DEVICE_NAME > > * Move struct intel_pmc_dev into include/linux/mfd/intel_pmc_bxt.h > > * Add PMC_DEVICE_MAX to the enum and use it > > * Add kernel-docs for simplecmd_store() and northpeak_store() > > * Use if (ret) return ret; over the ternary operator > > * Drop "This is index X" from comments > > * Use acpi_has_watchdog() to determine whether iTCO_wdt is added or not. > > * Rename intel_scu_ipc_pdata -> intel_scu_ipc_data to make it less > > confusing wrt. platform data for platform drivers. > > Any reason why you've dropped all my tags? You mean these? For my own reference: Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> I wasn't really sure what to do with them. They are not in the normal tag format I've seen so I thought you use them yourself somehow to manage your mailboxes. I can add them back if needed.
On Mon, 02 Mar 2020, Mika Westerberg wrote: > On Mon, Mar 02, 2020 at 02:26:21PM +0000, Lee Jones wrote: > > On Mon, 02 Mar 2020, Mika Westerberg wrote: > > > > > Hi all, > > > > > > Currently both intel_scu_ipc.c and intel_pmc_ipc.c implement the same SCU > > > IPC communications with minor differences. This duplication does not make > > > much sense so this series reworks the two drivers so that there is only a > > > single implementation of the SCU IPC. In addition to that the API will be > > > updated to take SCU instance pointer as an argument, and most of the > > > callers will be converted to this new API. The old API is left there but > > > the plan is to get rid the callers and then the old API as well (this is > > > something we are working with Andy Shevchenko). > > > > > > The intel_pmc_ipc.c is then moved under MFD which suits better for this > > > kind of a driver that pretty much sets up the SCU IPC and then creates a > > > bunch of platform devices for the things sitting behind the PMC. The driver > > > is renamed to intel_pmc_bxt.c which should follow the existing conventions > > > under drivers/mfd (and it is only meant for Intel Broxton derivatives). > > > > > > This is on top of platform-driver-x86.git/for-next branch because there is > > > already some cleanup work queued that re-organizes Kconfig and Makefile > > > entries. > > > > > > I have tested this on Intel Joule (Broxton-M) board. > > > > > > Changes from v6: > > > > > > * Added Reviewed-by tag from Andy > > > * Expanded PMC, IPC and IA acronyms > > > * Drop TCO_DEVICE_NAME, PUNIT_DEVICE_NAME and TELEMETRY_DEVICE_NAME > > > * Move struct intel_pmc_dev into include/linux/mfd/intel_pmc_bxt.h > > > * Add PMC_DEVICE_MAX to the enum and use it > > > * Add kernel-docs for simplecmd_store() and northpeak_store() > > > * Use if (ret) return ret; over the ternary operator > > > * Drop "This is index X" from comments > > > * Use acpi_has_watchdog() to determine whether iTCO_wdt is added or not. > > > * Rename intel_scu_ipc_pdata -> intel_scu_ipc_data to make it less > > > confusing wrt. platform data for platform drivers. > > > > Any reason why you've dropped all my tags? > > You mean these? > > For my own reference: > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > > I wasn't really sure what to do with them. They are not in the normal > tag format I've seen so I thought you use them yourself somehow to > manage your mailboxes. I can add them back if needed. Yes, please add them, so I can track them. It normally means that I plan to take the set through MFD and subsequently send an immutable pull-request out to the other Maintainers once all the other Acks have been provided. MFD handles these kinds of cross-subsystem patch-sets often.
On Mon, Mar 02, 2020 at 03:19:24PM +0000, Lee Jones wrote: > > I wasn't really sure what to do with them. They are not in the normal > > tag format I've seen so I thought you use them yourself somehow to > > manage your mailboxes. I can add them back if needed. > > Yes, please add them, so I can track them. > > It normally means that I plan to take the set through MFD and > subsequently send an immutable pull-request out to the other > Maintainers once all the other Acks have been provided. > > MFD handles these kinds of cross-subsystem patch-sets often. OK, thanks for the explanation. I'll re-send with your acks added then.
On Mon, Mar 02, 2020 at 03:19:24PM +0000, Lee Jones wrote: > On Mon, 02 Mar 2020, Mika Westerberg wrote: > > On Mon, Mar 02, 2020 at 02:26:21PM +0000, Lee Jones wrote: > > > On Mon, 02 Mar 2020, Mika Westerberg wrote: > > > > Currently both intel_scu_ipc.c and intel_pmc_ipc.c implement the same SCU > > > > IPC communications with minor differences. This duplication does not make > > > > much sense so this series reworks the two drivers so that there is only a > > > > single implementation of the SCU IPC. In addition to that the API will be > > > > updated to take SCU instance pointer as an argument, and most of the > > > > callers will be converted to this new API. The old API is left there but > > > > the plan is to get rid the callers and then the old API as well (this is > > > > something we are working with Andy Shevchenko). > > > > > > > > The intel_pmc_ipc.c is then moved under MFD which suits better for this > > > > kind of a driver that pretty much sets up the SCU IPC and then creates a > > > > bunch of platform devices for the things sitting behind the PMC. The driver > > > > is renamed to intel_pmc_bxt.c which should follow the existing conventions > > > > under drivers/mfd (and it is only meant for Intel Broxton derivatives). > > > > > > > > This is on top of platform-driver-x86.git/for-next branch because there is > > > > already some cleanup work queued that re-organizes Kconfig and Makefile > > > > entries. > > > > > > > > I have tested this on Intel Joule (Broxton-M) board. > > > > > > > > Changes from v6: > > > > > > > > * Added Reviewed-by tag from Andy > > > > * Expanded PMC, IPC and IA acronyms > > > > * Drop TCO_DEVICE_NAME, PUNIT_DEVICE_NAME and TELEMETRY_DEVICE_NAME > > > > * Move struct intel_pmc_dev into include/linux/mfd/intel_pmc_bxt.h > > > > * Add PMC_DEVICE_MAX to the enum and use it > > > > * Add kernel-docs for simplecmd_store() and northpeak_store() > > > > * Use if (ret) return ret; over the ternary operator > > > > * Drop "This is index X" from comments > > > > * Use acpi_has_watchdog() to determine whether iTCO_wdt is added or not. > > > > * Rename intel_scu_ipc_pdata -> intel_scu_ipc_data to make it less > > > > confusing wrt. platform data for platform drivers. > > > > > > Any reason why you've dropped all my tags? > > > > You mean these? > > > > For my own reference: > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > > > > I wasn't really sure what to do with them. They are not in the normal > > tag format I've seen so I thought you use them yourself somehow to > > manage your mailboxes. I can add them back if needed. > > Yes, please add them, so I can track them. > > It normally means that I plan to take the set through MFD and > subsequently send an immutable pull-request out to the other > Maintainers once all the other Acks have been provided. > > MFD handles these kinds of cross-subsystem patch-sets often. This series has dependencies to PDx86 (as mentioned in cover letter). What do you prefer then, me to: a) prepare ib from what I have, then you take it followed by me taking your ib, or b) take everything and prepare ib for you?
On Mon, 02 Mar 2020, Andy Shevchenko wrote: > On Mon, Mar 02, 2020 at 03:19:24PM +0000, Lee Jones wrote: > > On Mon, 02 Mar 2020, Mika Westerberg wrote: > > > On Mon, Mar 02, 2020 at 02:26:21PM +0000, Lee Jones wrote: > > > > On Mon, 02 Mar 2020, Mika Westerberg wrote: > > > > > > Currently both intel_scu_ipc.c and intel_pmc_ipc.c implement the same SCU > > > > > IPC communications with minor differences. This duplication does not make > > > > > much sense so this series reworks the two drivers so that there is only a > > > > > single implementation of the SCU IPC. In addition to that the API will be > > > > > updated to take SCU instance pointer as an argument, and most of the > > > > > callers will be converted to this new API. The old API is left there but > > > > > the plan is to get rid the callers and then the old API as well (this is > > > > > something we are working with Andy Shevchenko). > > > > > > > > > > The intel_pmc_ipc.c is then moved under MFD which suits better for this > > > > > kind of a driver that pretty much sets up the SCU IPC and then creates a > > > > > bunch of platform devices for the things sitting behind the PMC. The driver > > > > > is renamed to intel_pmc_bxt.c which should follow the existing conventions > > > > > under drivers/mfd (and it is only meant for Intel Broxton derivatives). > > > > > > > > > > This is on top of platform-driver-x86.git/for-next branch because there is > > > > > already some cleanup work queued that re-organizes Kconfig and Makefile > > > > > entries. > > > > > > > > > > I have tested this on Intel Joule (Broxton-M) board. > > > > > > > > > > Changes from v6: > > > > > > > > > > * Added Reviewed-by tag from Andy > > > > > * Expanded PMC, IPC and IA acronyms > > > > > * Drop TCO_DEVICE_NAME, PUNIT_DEVICE_NAME and TELEMETRY_DEVICE_NAME > > > > > * Move struct intel_pmc_dev into include/linux/mfd/intel_pmc_bxt.h > > > > > * Add PMC_DEVICE_MAX to the enum and use it > > > > > * Add kernel-docs for simplecmd_store() and northpeak_store() > > > > > * Use if (ret) return ret; over the ternary operator > > > > > * Drop "This is index X" from comments > > > > > * Use acpi_has_watchdog() to determine whether iTCO_wdt is added or not. > > > > > * Rename intel_scu_ipc_pdata -> intel_scu_ipc_data to make it less > > > > > confusing wrt. platform data for platform drivers. > > > > > > > > Any reason why you've dropped all my tags? > > > > > > You mean these? > > > > > > For my own reference: > > > Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org> > > > > > > I wasn't really sure what to do with them. They are not in the normal > > > tag format I've seen so I thought you use them yourself somehow to > > > manage your mailboxes. I can add them back if needed. > > > > Yes, please add them, so I can track them. > > > > It normally means that I plan to take the set through MFD and > > subsequently send an immutable pull-request out to the other > > Maintainers once all the other Acks have been provided. > > > > MFD handles these kinds of cross-subsystem patch-sets often. > > This series has dependencies to PDx86 (as mentioned in cover letter). > > What do you prefer then, me to: > a) prepare ib from what I have, then you take it followed by me taking your ib, or > b) take everything and prepare ib for you? Either would be fine by me. What kind of dependencies are they? Are they protected by Kconfig options? Another way of asking that would be to say, would this set throw build errors if I tried to apply and build it or would it just refuse to compile?
On Mon, Mar 02, 2020 at 03:57:16PM +0000, Lee Jones wrote: > > This series has dependencies to PDx86 (as mentioned in cover letter). > > > > What do you prefer then, me to: > > a) prepare ib from what I have, then you take it followed by me taking your ib, or > > b) take everything and prepare ib for you? > > Either would be fine by me. I would prefer these to go through pdx86 because of the dependency. > What kind of dependencies are they? Are they protected by Kconfig > options? Another way of asking that would be to say, would this set > throw build errors if I tried to apply and build it or would it just > refuse to compile? They do not apply cleanly because linux-platform-drivers-x86.git/for-next re-organizes Kconfig and Makefile entries. I think they should still build fine, though.
On Mon, Mar 02, 2020 at 06:07:46PM +0200, Mika Westerberg wrote: > On Mon, Mar 02, 2020 at 03:57:16PM +0000, Lee Jones wrote: > > > This series has dependencies to PDx86 (as mentioned in cover letter). > > > > > > What do you prefer then, me to: > > > a) prepare ib from what I have, then you take it followed by me taking your ib, or > > > b) take everything and prepare ib for you? > > > > Either would be fine by me. > > I would prefer these to go through pdx86 because of the dependency. > > > What kind of dependencies are they? Are they protected by Kconfig > > options? Another way of asking that would be to say, would this set > > throw build errors if I tried to apply and build it or would it just > > refuse to compile? > > They do not apply cleanly because linux-platform-drivers-x86.git/for-next > re-organizes Kconfig and Makefile entries. I think they should still > build fine, though. So, then it's third possible way, to build this on top of MFD next followed by importing an ib to PDx86 with conflict resolution.
Hi all, Currently both intel_scu_ipc.c and intel_pmc_ipc.c implement the same SCU IPC communications with minor differences. This duplication does not make much sense so this series reworks the two drivers so that there is only a single implementation of the SCU IPC. In addition to that the API will be updated to take SCU instance pointer as an argument, and most of the callers will be converted to this new API. The old API is left there but the plan is to get rid the callers and then the old API as well (this is something we are working with Andy Shevchenko). The intel_pmc_ipc.c is then moved under MFD which suits better for this kind of a driver that pretty much sets up the SCU IPC and then creates a bunch of platform devices for the things sitting behind the PMC. The driver is renamed to intel_pmc_bxt.c which should follow the existing conventions under drivers/mfd (and it is only meant for Intel Broxton derivatives). This is on top of platform-driver-x86.git/for-next branch because there is already some cleanup work queued that re-organizes Kconfig and Makefile entries. I have tested this on Intel Joule (Broxton-M) board. Changes from v6: * Added Reviewed-by tag from Andy * Expanded PMC, IPC and IA acronyms * Drop TCO_DEVICE_NAME, PUNIT_DEVICE_NAME and TELEMETRY_DEVICE_NAME * Move struct intel_pmc_dev into include/linux/mfd/intel_pmc_bxt.h * Add PMC_DEVICE_MAX to the enum and use it * Add kernel-docs for simplecmd_store() and northpeak_store() * Use if (ret) return ret; over the ternary operator * Drop "This is index X" from comments * Use acpi_has_watchdog() to determine whether iTCO_wdt is added or not. * Rename intel_scu_ipc_pdata -> intel_scu_ipc_data to make it less confusing wrt. platform data for platform drivers. Previous versions can be found: v6: https://www.spinics.net/lists/platform-driver-x86/msg20896.html v5: https://www.spinics.net/lists/platform-driver-x86/msg20841.html v4: https://www.spinics.net/lists/platform-driver-x86/msg20658.html v3: https://www.spinics.net/lists/platform-driver-x86/msg20583.html v2: https://www.spinics.net/lists/platform-driver-x86/msg20446.html v1: https://www.spinics.net/lists/platform-driver-x86/msg20359.html Mika Westerberg (19): platform/x86: intel_scu_ipc: Split out SCU IPC functionality from the SCU driver platform/x86: intel_scu_ipc: Log more information if SCU IPC command fails platform/x86: intel_scu_ipc: Move legacy SCU IPC API to a separate header platform/x86: intel_scu_ipc: Introduce new SCU IPC API platform/x86: intel_mid_powerbtn: Convert to use new SCU IPC API watchdog: intel-mid_wdt: Convert to use new SCU IPC API platform/x86: intel_scu_ipcutil: Convert to use new SCU IPC API platform/x86: intel_scu_ipc: Add managed function to register SCU IPC platform/x86: intel_pmc_ipc: Start using SCU IPC mfd: intel_soc_pmic: Add SCU IPC member to struct intel_soc_pmic mfd: intel_soc_pmic_bxtwc: Convert to use new SCU IPC API mfd: intel_soc_pmic_mrfld: Convert to use new SCU IPC API platform/x86: intel_telemetry: Convert to use new SCU IPC API platform/x86: intel_pmc_ipc: Drop intel_pmc_ipc_command() x86/platform/intel-mid: Add empty stubs for intel_scu_devices_[create|destroy]() platform/x86: intel_pmc_ipc: Move PCI IDs to intel_scu_pcidrv.c platform/x86: intel_telemetry: Add telemetry_get_pltdata() platform/x86: intel_pmc_ipc: Convert to MFD MAINTAINERS: Update entry for Intel Broxton PMC driver .../ABI/obsolete/sysfs-driver-intel_pmc_bxt | 22 + MAINTAINERS | 23 +- arch/x86/Kconfig | 2 +- arch/x86/include/asm/intel-mid.h | 9 +- arch/x86/include/asm/intel_pmc_ipc.h | 59 -- arch/x86/include/asm/intel_scu_ipc.h | 114 ++- arch/x86/include/asm/intel_scu_ipc_legacy.h | 91 ++ arch/x86/include/asm/intel_telemetry.h | 6 +- drivers/mfd/Kconfig | 20 +- drivers/mfd/Makefile | 1 + drivers/mfd/intel_pmc_bxt.c | 504 ++++++++++ drivers/mfd/intel_soc_pmic_bxtwc.c | 34 +- drivers/mfd/intel_soc_pmic_mrfld.c | 10 +- drivers/platform/x86/Kconfig | 46 +- drivers/platform/x86/Makefile | 2 +- drivers/platform/x86/intel_mid_powerbtn.c | 15 +- drivers/platform/x86/intel_pmc_ipc.c | 949 ------------------ drivers/platform/x86/intel_scu_ipc.c | 447 +++++++-- drivers/platform/x86/intel_scu_ipcutil.c | 43 +- drivers/platform/x86/intel_scu_pcidrv.c | 68 ++ drivers/platform/x86/intel_telemetry_core.c | 17 +- .../platform/x86/intel_telemetry_debugfs.c | 15 +- drivers/platform/x86/intel_telemetry_pltdrv.c | 97 +- drivers/usb/typec/tcpm/Kconfig | 2 +- drivers/watchdog/intel-mid_wdt.c | 53 +- include/linux/mfd/intel_pmc_bxt.h | 43 + include/linux/mfd/intel_soc_pmic.h | 15 + 27 files changed, 1402 insertions(+), 1305 deletions(-) create mode 100644 Documentation/ABI/obsolete/sysfs-driver-intel_pmc_bxt delete mode 100644 arch/x86/include/asm/intel_pmc_ipc.h create mode 100644 arch/x86/include/asm/intel_scu_ipc_legacy.h create mode 100644 drivers/mfd/intel_pmc_bxt.c delete mode 100644 drivers/platform/x86/intel_pmc_ipc.c create mode 100644 drivers/platform/x86/intel_scu_pcidrv.c create mode 100644 include/linux/mfd/intel_pmc_bxt.h