On Mon, 26 Dec 2022, Ilpo Järvinen wrote:
> Move SPI based board definitions to per interface file from the global
> header. This makes it harder to use them accidently in the
> generic/interface agnostic code. Prefix the defines with M10BMC_N3000
> to make it more obvious these are related to some board type. All
> current non-N3000 board types have the same layout so they'll be
> reused.
>
> Some bitfield defs are also moved to intel-m10-bmc-core which seems
> more appropriate for them.
>
> Reviewed-by: Russ Weight <russell.h.weight@xxxxxxxxx>
> Reviewed-by: Xu Yilun <yilun.xu@xxxxxxxxx>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> ---
> drivers/mfd/intel-m10-bmc-core.c | 11 ++++
> drivers/mfd/intel-m10-bmc-spi.c | 89 ++++++++++++++++++++++---------
> include/linux/mfd/intel-m10-bmc.h | 46 ----------------
> 3 files changed, 74 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/mfd/intel-m10-bmc-core.c b/drivers/mfd/intel-m10-bmc-core.c
> index 51b78b868235..50a4ec758bdb 100644
> --- a/drivers/mfd/intel-m10-bmc-core.c
> +++ b/drivers/mfd/intel-m10-bmc-core.c
> @@ -12,6 +12,17 @@
> #include <linux/mfd/intel-m10-bmc.h>
> #include <linux/module.h>
>
> +/* Register fields of system registers */
> +#define M10BMC_MAC_BYTE4 GENMASK(7, 0)
> +#define M10BMC_MAC_BYTE3 GENMASK(15, 8)
> +#define M10BMC_MAC_BYTE2 GENMASK(23, 16)
> +#define M10BMC_MAC_BYTE1 GENMASK(31, 24)
> +#define M10BMC_MAC_BYTE6 GENMASK(7, 0)
> +#define M10BMC_MAC_BYTE5 GENMASK(15, 8)
> +#define M10BMC_MAC_COUNT GENMASK(23, 16)
> +#define M10BMC_VER_MAJOR_MSK GENMASK(23, 16)
> +#define M10BMC_VER_PCB_INFO_MSK GENMASK(31, 24)
> +
> static ssize_t bmc_version_show(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> diff --git a/drivers/mfd/intel-m10-bmc-spi.c b/drivers/mfd/intel-m10-bmc-spi.c
> index e8986154e965..04c83f9c6492 100644
> --- a/drivers/mfd/intel-m10-bmc-spi.c
> +++ b/drivers/mfd/intel-m10-bmc-spi.c
> @@ -13,10 +13,47 @@
> #include <linux/regmap.h>
> #include <linux/spi/spi.h>
>
> +#define M10BMC_N3000_LEGACY_BUILD_VER 0x300468
> +#define M10BMC_N3000_SYS_BASE 0x300800
> +#define M10BMC_N3000_SYS_END 0x300fff
> +#define M10BMC_N3000_FLASH_BASE 0x10000000
> +#define M10BMC_N3000_FLASH_END 0x1fffffff
> +#define M10BMC_N3000_MEM_END M10BMC_N3000_FLASH_END
> +
> +/* Register offset of system registers */
> +#define NIOS2_FW_VERSION 0x0
> +#define M10BMC_N3000_MAC_LOW 0x10
> +#define M10BMC_N3000_MAC_HIGH 0x14
> +#define M10BMC_N3000_TEST_REG 0x3c
> +#define M10BMC_N3000_BUILD_VER 0x68
> +#define M10BMC_N3000_VER_LEGACY_INVALID 0xffffffff
> +
> +/* Secure update doorbell register, in system register region */
> +#define M10BMC_N3000_DOORBELL 0x400
> +
> +/* Authorization Result register, in system register region */
> +#define M10BMC_N3000_AUTH_RESULT 0x404
> +
> +/* Addresses for security related data in FLASH */
> +#define M10BMC_N3000_BMC_REH_ADDR 0x17ffc004
> +#define M10BMC_N3000_BMC_PROG_ADDR 0x17ffc000
> +#define M10BMC_N3000_BMC_PROG_MAGIC 0x5746
> +
> +#define M10BMC_N3000_SR_REH_ADDR 0x17ffd004
> +#define M10BMC_N3000_SR_PROG_ADDR 0x17ffd000
> +#define M10BMC_N3000_SR_PROG_MAGIC 0x5253
> +
> +#define M10BMC_N3000_PR_REH_ADDR 0x17ffe004
> +#define M10BMC_N3000_PR_PROG_ADDR 0x17ffe000
> +#define M10BMC_N3000_PR_PROG_MAGIC 0x5250
My preference would definitely be to keep these blocks of defines tucked
away inside a header file somewhere.
Premise of the change looks fine, however.
--
Lee Jones [李琼斯]
