[PATCH v2 0/4] Some improvement for Intel MAX 10 MFD drivers

[PATCH v2 0/4] Some improvement for Intel MAX 10 MFD drivers

[Xu Yilun]

This patchset is some improvements for intel-m10-bmc and its subdevs.

Main changes from v1:
- Add a patch (#2) to simplify the definition of the legacy version reg.
- Add a patch (#4), add entry in MAINTAINERS for intel-m10-bmc mfd driver
  and the subdev drivers.

Matthew Gerlach (1):
  mfd: intel-m10-bmc: Add access table configuration to the regmap

Xu Yilun (3):
  mfd: intel-m10-bmc: Fix the register access range
  mfd: intel-m10-bmc: Simplify the legacy version reg definition
  MAINTAINERS: Add entry for Intel MAX 10 mfd driver

 MAINTAINERS                       |  9 +++++++++
 drivers/mfd/intel-m10-bmc.c       | 25 ++++++++++++++++++-------
 include/linux/mfd/intel-m10-bmc.h |  7 +++++--
 3 files changed, 32 insertions(+), 9 deletions(-)

-- 
2.7.4

[PATCH v2 1/4] mfd: intel-m10-bmc: Fix the register access range

[Xu Yilun]

This patch fixes the max register address of MAX 10 BMC. The range
0x20000000 ~ 0x200000fc are for control registers of the QSPI flash
controller, which are not accessible to host.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
Reviewed-by: Tom Rix <trix@redhat.com>
---
v2: no change.
---
 include/linux/mfd/intel-m10-bmc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
index c8ef2f1..06da62c 100644
--- a/include/linux/mfd/intel-m10-bmc.h
+++ b/include/linux/mfd/intel-m10-bmc.h
@@ -11,7 +11,7 @@
 
 #define M10BMC_LEGACY_SYS_BASE		0x300400
 #define M10BMC_SYS_BASE			0x300800
-#define M10BMC_MEM_END			0x200000fc
+#define M10BMC_MEM_END			0x1fffffff
 
 /* Register offset of system registers */
 #define NIOS2_FW_VERSION		0x0
-- 
2.7.4

[PATCH v2 2/4] mfd: intel-m10-bmc: Simplify the legacy version reg definition

[Xu Yilun]

The version register is the only one in the legacy I/O space to be
accessed, so it is not necessary to define the legacy base & version
register offset. A direct definition of the legacy version register
address would be fine.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
---
 drivers/mfd/intel-m10-bmc.c       | 12 +++++-------
 include/linux/mfd/intel-m10-bmc.h |  2 +-
 2 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/intel-m10-bmc.c b/drivers/mfd/intel-m10-bmc.c
index b84579b..aad86f0 100644
--- a/drivers/mfd/intel-m10-bmc.c
+++ b/drivers/mfd/intel-m10-bmc.c
@@ -74,16 +74,14 @@ static int check_m10bmc_version(struct intel_m10bmc *ddata)
 
 	/*
 	 * This check is to filter out the very old legacy BMC versions,
-	 * M10BMC_LEGACY_SYS_BASE is the offset to this old block of mmio
-	 * registers. In the old BMC chips, the BMC version info is stored
-	 * in this old version register (M10BMC_LEGACY_SYS_BASE +
-	 * M10BMC_BUILD_VER), so its read out value would have not been
-	 * LEGACY_INVALID (0xffffffff). But in new BMC chips that the
+	 * 0x300400 is the offset to this old block of mmio registers. In the
+	 * old BMC chips, the BMC version info is stored in this old version
+	 * register (0x300400 + 0x68), so its read out value would have not
+	 * been LEGACY_INVALID (0xffffffff). But in new BMC chips that the
 	 * driver supports, the value of this register should be
 	 * LEGACY_INVALID.
 	 */
-	ret = m10bmc_raw_read(ddata,
-			      M10BMC_LEGACY_SYS_BASE + M10BMC_BUILD_VER, &v);
+	ret = m10bmc_raw_read(ddata, M10BMC_LEGACY_BUILD_VER, &v);
 	if (ret)
 		return -ENODEV;
 
diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
index 06da62c..99f44b1 100644
--- a/include/linux/mfd/intel-m10-bmc.h
+++ b/include/linux/mfd/intel-m10-bmc.h
@@ -9,7 +9,7 @@
 
 #include <linux/regmap.h>
 
-#define M10BMC_LEGACY_SYS_BASE		0x300400
+#define M10BMC_LEGACY_BUILD_VER		0x300468
 #define M10BMC_SYS_BASE			0x300800
 #define M10BMC_MEM_END			0x1fffffff
 
-- 
2.7.4

Re: [PATCH v2 2/4] mfd: intel-m10-bmc: Simplify the legacy version reg definition

[Tom Rix]

On 1/25/21 10:50 PM, Xu Yilun wrote:
> The version register is the only one in the legacy I/O space to be
> accessed, so it is not necessary to define the legacy base & version
> register offset. A direct definition of the legacy version register
> address would be fine.
>
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> ---
>  drivers/mfd/intel-m10-bmc.c       | 12 +++++-------
>  include/linux/mfd/intel-m10-bmc.h |  2 +-
>  2 files changed, 6 insertions(+), 8 deletions(-)

Thanks for adding M10BMC_LEGACY_BUILD_VER

Reviewed-by: Tom Rix <trix@redhat.com>

> diff --git a/drivers/mfd/intel-m10-bmc.c b/drivers/mfd/intel-m10-bmc.c
> index b84579b..aad86f0 100644
> --- a/drivers/mfd/intel-m10-bmc.c
> +++ b/drivers/mfd/intel-m10-bmc.c
> @@ -74,16 +74,14 @@ static int check_m10bmc_version(struct intel_m10bmc *ddata)
>  
>  	/*
>  	 * This check is to filter out the very old legacy BMC versions,
> -	 * M10BMC_LEGACY_SYS_BASE is the offset to this old block of mmio
> -	 * registers. In the old BMC chips, the BMC version info is stored
> -	 * in this old version register (M10BMC_LEGACY_SYS_BASE +
> -	 * M10BMC_BUILD_VER), so its read out value would have not been
> -	 * LEGACY_INVALID (0xffffffff). But in new BMC chips that the
> +	 * 0x300400 is the offset to this old block of mmio registers. In the
> +	 * old BMC chips, the BMC version info is stored in this old version
> +	 * register (0x300400 + 0x68), so its read out value would have not
> +	 * been LEGACY_INVALID (0xffffffff). But in new BMC chips that the
>  	 * driver supports, the value of this register should be
>  	 * LEGACY_INVALID.
>  	 */
> -	ret = m10bmc_raw_read(ddata,
> -			      M10BMC_LEGACY_SYS_BASE + M10BMC_BUILD_VER, &v);
> +	ret = m10bmc_raw_read(ddata, M10BMC_LEGACY_BUILD_VER, &v);
>  	if (ret)
>  		return -ENODEV;
>  
> diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
> index 06da62c..99f44b1 100644
> --- a/include/linux/mfd/intel-m10-bmc.h
> +++ b/include/linux/mfd/intel-m10-bmc.h
> @@ -9,7 +9,7 @@
>  
>  #include <linux/regmap.h>
>  
> -#define M10BMC_LEGACY_SYS_BASE		0x300400
> +#define M10BMC_LEGACY_BUILD_VER		0x300468
>  #define M10BMC_SYS_BASE			0x300800
>  #define M10BMC_MEM_END			0x1fffffff
>  

[PATCH v2 3/4] mfd: intel-m10-bmc: Add access table configuration to the regmap

[Xu Yilun]

From: Matthew Gerlach <matthew.gerlach@linux.intel.com>

This patch adds access tables to the MAX 10 BMC regmap. This prevents
the host from accessing the unwanted I/O space. It also filters out the
invalid outputs when reading the regmap debugfs interface.

Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
Signed-off-by: Xu Yilun <yilun.xu@intel.com>
---
v2: no change.
---
 drivers/mfd/intel-m10-bmc.c       | 13 +++++++++++++
 include/linux/mfd/intel-m10-bmc.h |  5 ++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/intel-m10-bmc.c b/drivers/mfd/intel-m10-bmc.c
index aad86f0..4240e99 100644
--- a/drivers/mfd/intel-m10-bmc.c
+++ b/drivers/mfd/intel-m10-bmc.c
@@ -23,10 +23,23 @@ static struct mfd_cell m10bmc_pacn3000_subdevs[] = {
 	{ .name = "n3000bmc-secure" },
 };
 
+static const struct regmap_range m10bmc_regmap_range[] = {
+	regmap_reg_range(M10BMC_LEGACY_BUILD_VER, M10BMC_LEGACY_BUILD_VER),
+	regmap_reg_range(M10BMC_SYS_BASE, M10BMC_SYS_END),
+	regmap_reg_range(M10BMC_FLASH_BASE, M10BMC_FLASH_END),
+};
+
+static const struct regmap_access_table m10bmc_access_table = {
+	.yes_ranges	= m10bmc_regmap_range,
+	.n_yes_ranges	= ARRAY_SIZE(m10bmc_regmap_range),
+};
+
 static struct regmap_config intel_m10bmc_regmap_config = {
 	.reg_bits = 32,
 	.val_bits = 32,
 	.reg_stride = 4,
+	.wr_table = &m10bmc_access_table,
+	.rd_table = &m10bmc_access_table,
 	.max_register = M10BMC_MEM_END,
 };
 
diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
index 99f44b1..dc2e858 100644
--- a/include/linux/mfd/intel-m10-bmc.h
+++ b/include/linux/mfd/intel-m10-bmc.h
@@ -11,7 +11,10 @@
 
 #define M10BMC_LEGACY_BUILD_VER		0x300468
 #define M10BMC_SYS_BASE			0x300800
-#define M10BMC_MEM_END			0x1fffffff
+#define M10BMC_SYS_END			0x300fff
+#define M10BMC_FLASH_BASE		0x10000000
+#define M10BMC_FLASH_END		0x1fffffff
+#define M10BMC_MEM_END			M10BMC_FLASH_END
 
 /* Register offset of system registers */
 #define NIOS2_FW_VERSION		0x0
-- 
2.7.4

Re: [PATCH v2 3/4] mfd: intel-m10-bmc: Add access table configuration to the regmap

[Tom Rix]

On 1/25/21 10:50 PM, Xu Yilun wrote:
> From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
>
> This patch adds access tables to the MAX 10 BMC regmap. This prevents
> the host from accessing the unwanted I/O space. It also filters out the
> invalid outputs when reading the regmap debugfs interface.
>
> Signed-off-by: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> ---
> v2: no change.

v2: Use M10BMC_LEGACY_BUILD_VER

Looks good.

Reviewed-by: Tom Rix <trix@redhat.com>


> ---
>  drivers/mfd/intel-m10-bmc.c       | 13 +++++++++++++
>  include/linux/mfd/intel-m10-bmc.h |  5 ++++-
>  2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/intel-m10-bmc.c b/drivers/mfd/intel-m10-bmc.c
> index aad86f0..4240e99 100644
> --- a/drivers/mfd/intel-m10-bmc.c
> +++ b/drivers/mfd/intel-m10-bmc.c
> @@ -23,10 +23,23 @@ static struct mfd_cell m10bmc_pacn3000_subdevs[] = {
>  	{ .name = "n3000bmc-secure" },
>  };
>  
> +static const struct regmap_range m10bmc_regmap_range[] = {
> +	regmap_reg_range(M10BMC_LEGACY_BUILD_VER, M10BMC_LEGACY_BUILD_VER),
> +	regmap_reg_range(M10BMC_SYS_BASE, M10BMC_SYS_END),
> +	regmap_reg_range(M10BMC_FLASH_BASE, M10BMC_FLASH_END),
> +};
> +
> +static const struct regmap_access_table m10bmc_access_table = {
> +	.yes_ranges	= m10bmc_regmap_range,
> +	.n_yes_ranges	= ARRAY_SIZE(m10bmc_regmap_range),
> +};
> +
>  static struct regmap_config intel_m10bmc_regmap_config = {
>  	.reg_bits = 32,
>  	.val_bits = 32,
>  	.reg_stride = 4,
> +	.wr_table = &m10bmc_access_table,
> +	.rd_table = &m10bmc_access_table,
>  	.max_register = M10BMC_MEM_END,
>  };
>  
> diff --git a/include/linux/mfd/intel-m10-bmc.h b/include/linux/mfd/intel-m10-bmc.h
> index 99f44b1..dc2e858 100644
> --- a/include/linux/mfd/intel-m10-bmc.h
> +++ b/include/linux/mfd/intel-m10-bmc.h
> @@ -11,7 +11,10 @@
>  
>  #define M10BMC_LEGACY_BUILD_VER		0x300468
>  #define M10BMC_SYS_BASE			0x300800
> -#define M10BMC_MEM_END			0x1fffffff
> +#define M10BMC_SYS_END			0x300fff
> +#define M10BMC_FLASH_BASE		0x10000000
> +#define M10BMC_FLASH_END		0x1fffffff
> +#define M10BMC_MEM_END			M10BMC_FLASH_END
>  
>  /* Register offset of system registers */
>  #define NIOS2_FW_VERSION		0x0

[PATCH v2 4/4] MAINTAINERS: Add entry for Intel MAX 10 mfd driver

[Xu Yilun]

This patch adds maintainer info for Intel MAX 10 mfd driver.

Signed-off-by: Xu Yilun <yilun.xu@intel.com>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5aa18cb..10985d3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9132,6 +9132,15 @@ F:	include/linux/mei_cl_bus.h
 F:	include/uapi/linux/mei.h
 F:	samples/mei/*
 
+INTEL MAX 10 BMC MFD DRIVER
+M:	Xu Yilun <yilun.xu@intel.com>
+S:	Maintained
+F:	Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
+F:	Documentation/hwmon/intel-m10-bmc-hwmon.rst
+F:	drivers/hwmon/intel-m10-bmc-hwmon.c
+F:	drivers/mfd/intel-m10-bmc.c
+F:	include/linux/mfd/intel-m10-bmc.h
+
 INTEL MENLOW THERMAL DRIVER
 M:	Sujith Thomas <sujith.thomas@intel.com>
 L:	platform-driver-x86@vger.kernel.org
-- 
2.7.4

Re: [PATCH v2 4/4] MAINTAINERS: Add entry for Intel MAX 10 mfd driver

[Tom Rix]

On 1/25/21 10:50 PM, Xu Yilun wrote:
> This patch adds maintainer info for Intel MAX 10 mfd driver.
>
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> ---
>  MAINTAINERS | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5aa18cb..10985d3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9132,6 +9132,15 @@ F:	include/linux/mei_cl_bus.h
>  F:	include/uapi/linux/mei.h
>  F:	samples/mei/*
>  

I am interested in reviewing these files like I do with

FPGA DFL DRIVERS

So can you add

R:    Tom Rix <trix@redhat.com>

if you have to rev this patchset ?

Else I will submit a followup.

Reviewed-by: Tom Rix <trix@redhat.com>

> +INTEL MAX 10 BMC MFD DRIVER
> +M:	Xu Yilun <yilun.xu@intel.com>
> +S:	Maintained
> +F:	Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
> +F:	Documentation/hwmon/intel-m10-bmc-hwmon.rst
> +F:	drivers/hwmon/intel-m10-bmc-hwmon.c
> +F:	drivers/mfd/intel-m10-bmc.c
> +F:	include/linux/mfd/intel-m10-bmc.h
> +
>  INTEL MENLOW THERMAL DRIVER
>  M:	Sujith Thomas <sujith.thomas@intel.com>
>  L:	platform-driver-x86@vger.kernel.org

Re: [PATCH v2 4/4] MAINTAINERS: Add entry for Intel MAX 10 mfd driver

[Xu Yilun]

On Tue, Jan 26, 2021 at 07:06:17AM -0800, Tom Rix wrote:
> 
> On 1/25/21 10:50 PM, Xu Yilun wrote:
> > This patch adds maintainer info for Intel MAX 10 mfd driver.
> >
> > Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> > ---
> >  MAINTAINERS | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 5aa18cb..10985d3 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9132,6 +9132,15 @@ F:	include/linux/mei_cl_bus.h
> >  F:	include/uapi/linux/mei.h
> >  F:	samples/mei/*
> >  
> 
> I am interested in reviewing these files like I do with
> 
> FPGA DFL DRIVERS
> 
> So can you add
> 
> R:    Tom Rix <trix@redhat.com>
> 
> if you have to rev this patchset ?

It's good to me. I'll add it if I need to send v3.

Thanks,
Yilun

> 
> Else I will submit a followup.
> 
> Reviewed-by: Tom Rix <trix@redhat.com>
> 
> > +INTEL MAX 10 BMC MFD DRIVER
> > +M:	Xu Yilun <yilun.xu@intel.com>
> > +S:	Maintained
> > +F:	Documentation/ABI/testing/sysfs-driver-intel-m10-bmc
> > +F:	Documentation/hwmon/intel-m10-bmc-hwmon.rst
> > +F:	drivers/hwmon/intel-m10-bmc-hwmon.c
> > +F:	drivers/mfd/intel-m10-bmc.c
> > +F:	include/linux/mfd/intel-m10-bmc.h
> > +
> >  INTEL MENLOW THERMAL DRIVER
> >  M:	Sujith Thomas <sujith.thomas@intel.com>
> >  L:	platform-driver-x86@vger.kernel.org