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
>
