mbox series

[v2,00/11] intel-m10-bmc: Split BMC to core and SPI parts & add PMCI+N6000 support

Message ID 20221117120515.37807-1-ilpo.jarvinen@linux.intel.com (mailing list archive)
Headers show
Series intel-m10-bmc: Split BMC to core and SPI parts & add PMCI+N6000 support | expand

Message

Ilpo Järvinen Nov. 17, 2022, 12:05 p.m. UTC
Hi all,

Here are the patches for MAX 10 BMC core/SPI interface split and
addition of the PMCI interface. There are a few supporting rearrangement
patches prior to the actual split. This series also introduced regmap for
indirect register access generic to Intel FPGA IPs.

The current downside of the split is that there's not that much code
remaining in the core part when all the type specific definitions are
moved to the file with the relevant interface.

I'm not entirely sure if I did properly address Yilun's comment on
placement of the flash_ops related code. To me the original way which
keeps them in mfd/intel-m10-bmc-{spi,pmci}.c still seems to the best
way. If that is still not acceptable, I propose that I'll move just the
flash ops functions into intel-m10-bmc-sec-update-{spi,pmci}.c and
create sec related platform info struct to select the correct flash
ops. This would effectively be the "double split" approach I suggested
earlier (both mfd and sec have their own per interface splits, to me
the double split looks overkill but it would meet Yilun's goal of
keeping sec related code within the sec driver).

The patch set is based on top of commit dfd10332596e ("fpga:
m10bmc-sec: Fix kconfig dependencies") to avoid triggering a Kconfig
conflict.

v2:
- Drop type from mfd side, the platform info takes care of differentation
- Explain introducing ->info to struct m10bmc in commit message (belongs logically there)
- Intro PMCI better
- Improve naming
        - Rename M10BMC_PMCI_FLASH_CTRL to M10BMC_PMCI_FLASH_MUX_CTRL
        - Use m10bmc_pmci/M10BMC_PMCI prefix consistently
        - Use M10BMC_SPI prefix for SPI related defines
        - Newly added stuff gets m10bmc_spi prefix
- Removed dev from struct m10bmc_pmci_device
- Rename "n_offset" variable to "offset" in PMCI flash ops
- Always issue idle command in regmap indirect to clear RD/WR/ACK bits
- Handle stride misaligned sizes in flash read/write ops

Ilpo Järvinen (11):
  mfd: intel-m10-bmc: Create m10bmc_platform_info for type specific info
  mfd: intel-m10-bmc: Rename the local variables
  mfd: intel-m10-bmc: Split into core and spi specific parts
  mfd: intel-m10-bmc: Support multiple CSR register layouts
  fpga: intel-m10-bmc: Add flash ops for sec update
  mfd: intel-m10-bmc: Downscope SPI defines & prefix with M10BMC_SPI
  regmap: indirect: Add indirect regmap support
  intel-m10-bmc: Add regmap_indirect_cfg for Intel FPGA IPs
  mfd: intel-m10-bmc: Add PMCI driver
  fpga: m10bmc-sec: Add support for N6000
  mfd: intel-m10-bmc: Change MODULE_LICENSE() to GPL

 .../ABI/testing/sysfs-driver-intel-m10-bmc    |   8 +-
 MAINTAINERS                                   |   2 +-
 drivers/base/regmap/Kconfig                   |   3 +
 drivers/base/regmap/Makefile                  |   1 +
 drivers/base/regmap/regmap-indirect.c         | 128 +++++++
 drivers/fpga/Kconfig                          |   2 +-
 drivers/fpga/intel-m10-bmc-sec-update.c       | 149 ++++----
 drivers/hwmon/Kconfig                         |   2 +-
 drivers/mfd/Kconfig                           |  34 +-
 drivers/mfd/Makefile                          |   6 +-
 drivers/mfd/intel-m10-bmc-core.c              | 133 +++++++
 drivers/mfd/intel-m10-bmc-pmci.c              | 361 ++++++++++++++++++
 drivers/mfd/intel-m10-bmc-spi.c               | 270 +++++++++++++
 drivers/mfd/intel-m10-bmc.c                   | 238 ------------
 include/linux/mfd/intel-m10-bmc.h             | 122 +++---
 include/linux/regmap.h                        |  55 +++
 16 files changed, 1131 insertions(+), 383 deletions(-)
 create mode 100644 drivers/base/regmap/regmap-indirect.c
 create mode 100644 drivers/mfd/intel-m10-bmc-core.c
 create mode 100644 drivers/mfd/intel-m10-bmc-pmci.c
 create mode 100644 drivers/mfd/intel-m10-bmc-spi.c
 delete mode 100644 drivers/mfd/intel-m10-bmc.c

Comments

Xu Yilun Nov. 22, 2022, 2:43 a.m. UTC | #1
On 2022-11-17 at 14:05:04 +0200, Ilpo Järvinen wrote:
> Hi all,
> 
> Here are the patches for MAX 10 BMC core/SPI interface split and
> addition of the PMCI interface. There are a few supporting rearrangement
> patches prior to the actual split. This series also introduced regmap for
> indirect register access generic to Intel FPGA IPs.
> 
> The current downside of the split is that there's not that much code
> remaining in the core part when all the type specific definitions are
> moved to the file with the relevant interface.
> 
> I'm not entirely sure if I did properly address Yilun's comment on
> placement of the flash_ops related code. To me the original way which
> keeps them in mfd/intel-m10-bmc-{spi,pmci}.c still seems to the best
> way. If that is still not acceptable, I propose that I'll move just the
> flash ops functions into intel-m10-bmc-sec-update-{spi,pmci}.c and

I don't think the split of intel-m10-bmc-sec-update-{spi, pmci}.c is
needed. The mfd subdev is a platform device actually, it doesn't have to
know the bus type.

> create sec related platform info struct to select the correct flash
> ops. This would effectively be the "double split" approach I suggested
> earlier (both mfd and sec have their own per interface splits, to me
> the double split looks overkill but it would meet Yilun's goal of
> keeping sec related code within the sec driver).

Yes, this is still my preference.

My idea is, the secure update driver could be told by mfd core whether
there is a bypass channel for flash bulk read/write. If yes, use it. If
no, just direct access to flash staging areas as it previously does.

This is like:

struct intel_m10bmc {
        struct device *dev;
        struct regmap *regmap;
	...
+       const struct intel_m10bmc_flash_bulk_ops *flash_bulk_ops;
}

In intel-m10-bmc-pmci.c,

  +static const struct intel_m10bmc_flash_bulk_ops m10bmc_pmci_flash_bulk_ops = {
  +       .read = m10bmc_pmci_flash_bulk_read,
  +       .write = m10bmc_pmci_flash_bulk_write,
  };

In intel-m10-bmc-spi.c, no need to have flash_bulk_ops. 

In intel-m10-bmc-sec-update.c,

  Check if flash_bulk_ops is available, if yes,

    set_flash_host_mux(aquire)   /* I assume flash mux is dedicated for sec-update dev */
    sec->m10bmc->flash_bulk_ops->write()
    set_flash_host_mux(release)

  if no:
    follow previous direct access.

Thanks,
Yilun

> 
> The patch set is based on top of commit dfd10332596e ("fpga:
> m10bmc-sec: Fix kconfig dependencies") to avoid triggering a Kconfig
> conflict.
> 
> v2:
> - Drop type from mfd side, the platform info takes care of differentation
> - Explain introducing ->info to struct m10bmc in commit message (belongs logically there)
> - Intro PMCI better
> - Improve naming
>         - Rename M10BMC_PMCI_FLASH_CTRL to M10BMC_PMCI_FLASH_MUX_CTRL
>         - Use m10bmc_pmci/M10BMC_PMCI prefix consistently
>         - Use M10BMC_SPI prefix for SPI related defines
>         - Newly added stuff gets m10bmc_spi prefix
> - Removed dev from struct m10bmc_pmci_device
> - Rename "n_offset" variable to "offset" in PMCI flash ops
> - Always issue idle command in regmap indirect to clear RD/WR/ACK bits
> - Handle stride misaligned sizes in flash read/write ops
> 
> Ilpo Järvinen (11):
>   mfd: intel-m10-bmc: Create m10bmc_platform_info for type specific info
>   mfd: intel-m10-bmc: Rename the local variables
>   mfd: intel-m10-bmc: Split into core and spi specific parts
>   mfd: intel-m10-bmc: Support multiple CSR register layouts
>   fpga: intel-m10-bmc: Add flash ops for sec update
>   mfd: intel-m10-bmc: Downscope SPI defines & prefix with M10BMC_SPI
>   regmap: indirect: Add indirect regmap support
>   intel-m10-bmc: Add regmap_indirect_cfg for Intel FPGA IPs
>   mfd: intel-m10-bmc: Add PMCI driver
>   fpga: m10bmc-sec: Add support for N6000
>   mfd: intel-m10-bmc: Change MODULE_LICENSE() to GPL
> 
>  .../ABI/testing/sysfs-driver-intel-m10-bmc    |   8 +-
>  MAINTAINERS                                   |   2 +-
>  drivers/base/regmap/Kconfig                   |   3 +
>  drivers/base/regmap/Makefile                  |   1 +
>  drivers/base/regmap/regmap-indirect.c         | 128 +++++++
>  drivers/fpga/Kconfig                          |   2 +-
>  drivers/fpga/intel-m10-bmc-sec-update.c       | 149 ++++----
>  drivers/hwmon/Kconfig                         |   2 +-
>  drivers/mfd/Kconfig                           |  34 +-
>  drivers/mfd/Makefile                          |   6 +-
>  drivers/mfd/intel-m10-bmc-core.c              | 133 +++++++
>  drivers/mfd/intel-m10-bmc-pmci.c              | 361 ++++++++++++++++++
>  drivers/mfd/intel-m10-bmc-spi.c               | 270 +++++++++++++
>  drivers/mfd/intel-m10-bmc.c                   | 238 ------------
>  include/linux/mfd/intel-m10-bmc.h             | 122 +++---
>  include/linux/regmap.h                        |  55 +++
>  16 files changed, 1131 insertions(+), 383 deletions(-)
>  create mode 100644 drivers/base/regmap/regmap-indirect.c
>  create mode 100644 drivers/mfd/intel-m10-bmc-core.c
>  create mode 100644 drivers/mfd/intel-m10-bmc-pmci.c
>  create mode 100644 drivers/mfd/intel-m10-bmc-spi.c
>  delete mode 100644 drivers/mfd/intel-m10-bmc.c
> 
> -- 
> 2.30.2
>