[PATCH 0/4] fpga/mfd/hwmon: Initial support for Silicom N5010 PAC

[PATCH 0/4] fpga/mfd/hwmon: Initial support for Silicom N5010 PAC

[Martin Hundebøll]

From: Martin Hundebøll <mhu@geanix.com>

This is an initial set of patches for the Silciom N5010 programmable
accelerated card adding support for reading out sensors.

I'm not really sure if these should be taken through each of the
affected trees separately, or just by fpga collectively?

Based on current master.

// Martin

Debarati Biswas (1):
  fpga: dfl: Move DFH header register macros to linux/dfl.h

Martin Hundebøll (3):
  fpga: dfl: pci: add device IDs for Silicom N501x PAC cards
  spi: spi-altera-dfl: support n5010 feature revision
  hwmon: intel-m10-bmc: add sensor support for Silicom N5010 card

 drivers/fpga/dfl-pci.c              |   5 ++
 drivers/fpga/dfl.h                  |  48 +-----------
 drivers/hwmon/intel-m10-bmc-hwmon.c | 116 ++++++++++++++++++++++++++++
 drivers/mfd/intel-m10-bmc.c         |  12 ++-
 drivers/spi/spi-altera-dfl.c        |  15 +++-
 include/linux/dfl.h                 |  52 +++++++++++++
 6 files changed, 198 insertions(+), 50 deletions(-)

-- 
2.31.0

[PATCH 1/4] fpga: dfl: pci: add device IDs for Silicom N501x PAC cards

[Martin Hundebøll]

This adds the approved PCI Express Device IDs for the
Silicom PAC N5010 and N5011 cards.

Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
---
 drivers/fpga/dfl-pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
index b44523ea8c91..4d68719e608f 100644
--- a/drivers/fpga/dfl-pci.c
+++ b/drivers/fpga/dfl-pci.c
@@ -74,6 +74,9 @@ static void cci_pci_free_irq(struct pci_dev *pcidev)
 #define PCIE_DEVICE_ID_PF_DSC_1_X		0x09C4
 #define PCIE_DEVICE_ID_INTEL_PAC_N3000		0x0B30
 #define PCIE_DEVICE_ID_INTEL_PAC_D5005		0x0B2B
+#define PCIE_DEVICE_ID_SILICOM_PAC_N5010	0x1000
+#define PCIE_DEVICE_ID_SILICOM_PAC_N5011	0x1001
+
 /* VF Device */
 #define PCIE_DEVICE_ID_VF_INT_5_X		0xBCBF
 #define PCIE_DEVICE_ID_VF_INT_6_X		0xBCC1
@@ -90,6 +93,8 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_PAC_N3000),},
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_PAC_D5005),},
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
+	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK, PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
+	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK, PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
 	{0,}
 };
 MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);
-- 
2.31.0

RE: [PATCH 1/4] fpga: dfl: pci: add device IDs for Silicom N501x PAC cards

[Wu, Hao]

> Subject: [PATCH 1/4] fpga: dfl: pci: add device IDs for Silicom N501x PAC cards
> 
> This adds the approved PCI Express Device IDs for the
> Silicom PAC N5010 and N5011 cards.

Maybe some short descriptions on these cards? And also full name of PAC
here may be helpful as well. Others look good to me.

> 
> Signed-off-by: Martin Hundebøll <mhu@silicom.dk>

With above change
Acked-by: Wu Hao <hao.wu@intel.com>

Thanks
Hao

> ---
>  drivers/fpga/dfl-pci.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/fpga/dfl-pci.c b/drivers/fpga/dfl-pci.c
> index b44523ea8c91..4d68719e608f 100644
> --- a/drivers/fpga/dfl-pci.c
> +++ b/drivers/fpga/dfl-pci.c
> @@ -74,6 +74,9 @@ static void cci_pci_free_irq(struct pci_dev *pcidev)
>  #define PCIE_DEVICE_ID_PF_DSC_1_X		0x09C4
>  #define PCIE_DEVICE_ID_INTEL_PAC_N3000		0x0B30
>  #define PCIE_DEVICE_ID_INTEL_PAC_D5005		0x0B2B
> +#define PCIE_DEVICE_ID_SILICOM_PAC_N5010	0x1000
> +#define PCIE_DEVICE_ID_SILICOM_PAC_N5011	0x1001
> +
>  /* VF Device */
>  #define PCIE_DEVICE_ID_VF_INT_5_X		0xBCBF
>  #define PCIE_DEVICE_ID_VF_INT_6_X		0xBCC1
> @@ -90,6 +93,8 @@ static struct pci_device_id cci_pcie_id_tbl[] = {
>  	{PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> PCIE_DEVICE_ID_INTEL_PAC_N3000),},
>  	{PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> PCIE_DEVICE_ID_INTEL_PAC_D5005),},
>  	{PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> PCIE_DEVICE_ID_INTEL_PAC_D5005_VF),},
> +	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
> PCIE_DEVICE_ID_SILICOM_PAC_N5010),},
> +	{PCI_DEVICE(PCI_VENDOR_ID_SILICOM_DENMARK,
> PCIE_DEVICE_ID_SILICOM_PAC_N5011),},
>  	{0,}
>  };
>  MODULE_DEVICE_TABLE(pci, cci_pcie_id_tbl);
> --
> 2.31.0

[PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Martin Hundebøll]

From: Debarati Biswas <debaratix.biswas@intel.com>

Device Feature List (DFL) drivers may be defined in subdirectories other
than drivers/fpga, and each DFL driver should have access to the Device
Feature Header (DFH) register, which contains revision and type
information. This change moves the macros specific to the DFH register
from drivers/fpga/dfl.h to include/linux/dfl.h.

Signed-off-by: Debarati Biswas <debaratix.biswas@intel.com>
Signed-off-by: Russ Weight <russell.h.weight@intel.com>
Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
---
 drivers/fpga/dfl.h  | 48 +----------------------------------------
 include/linux/dfl.h | 52 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 47 deletions(-)

diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
index 2b82c96ba56c..6ed0353e9a99 100644
--- a/drivers/fpga/dfl.h
+++ b/drivers/fpga/dfl.h
@@ -17,6 +17,7 @@
 #include <linux/bitfield.h>
 #include <linux/cdev.h>
 #include <linux/delay.h>
+#include <linux/dfl.h>
 #include <linux/eventfd.h>
 #include <linux/fs.h>
 #include <linux/interrupt.h>
@@ -53,32 +54,6 @@
 #define PORT_FEATURE_ID_UINT		0x12
 #define PORT_FEATURE_ID_STP		0x13
 
-/*
- * Device Feature Header Register Set
- *
- * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
- * For AFUs, they have DFH + GUID as common header registers.
- * For private features, they only have DFH register as common header.
- */
-#define DFH			0x0
-#define GUID_L			0x8
-#define GUID_H			0x10
-#define NEXT_AFU		0x18
-
-#define DFH_SIZE		0x8
-
-/* Device Feature Header Register Bitfield */
-#define DFH_ID			GENMASK_ULL(11, 0)	/* Feature ID */
-#define DFH_ID_FIU_FME		0
-#define DFH_ID_FIU_PORT		1
-#define DFH_REVISION		GENMASK_ULL(15, 12)	/* Feature revision */
-#define DFH_NEXT_HDR_OFST	GENMASK_ULL(39, 16)	/* Offset to next DFH */
-#define DFH_EOL			BIT_ULL(40)		/* End of list */
-#define DFH_TYPE		GENMASK_ULL(63, 60)	/* Feature type */
-#define DFH_TYPE_AFU		1
-#define DFH_TYPE_PRIVATE	3
-#define DFH_TYPE_FIU		4
-
 /* Next AFU Register Bitfield */
 #define NEXT_AFU_NEXT_DFH_OFST	GENMASK_ULL(23, 0)	/* Offset to next AFU */
 
@@ -403,27 +378,6 @@ struct device *dfl_fpga_pdata_to_parent(struct dfl_feature_platform_data *pdata)
 	return pdata->dev->dev.parent->parent;
 }
 
-static inline bool dfl_feature_is_fme(void __iomem *base)
-{
-	u64 v = readq(base + DFH);
-
-	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
-		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
-}
-
-static inline bool dfl_feature_is_port(void __iomem *base)
-{
-	u64 v = readq(base + DFH);
-
-	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
-		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
-}
-
-static inline u8 dfl_feature_revision(void __iomem *base)
-{
-	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
-}
-
 /**
  * struct dfl_fpga_enum_info - DFL FPGA enumeration information
  *
diff --git a/include/linux/dfl.h b/include/linux/dfl.h
index 6cc10982351a..1cd86b2e7cb1 100644
--- a/include/linux/dfl.h
+++ b/include/linux/dfl.h
@@ -8,7 +8,9 @@
 #ifndef __LINUX_DFL_H
 #define __LINUX_DFL_H
 
+#include <linux/bitfield.h>
 #include <linux/device.h>
+#include <linux/io.h>
 #include <linux/mod_devicetable.h>
 
 /**
@@ -83,4 +85,54 @@ void dfl_driver_unregister(struct dfl_driver *dfl_drv);
 	module_driver(__dfl_driver, dfl_driver_register, \
 		      dfl_driver_unregister)
 
+/*
+ * Device Feature Header Register Set
+ *
+ * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
+ * For AFUs, they have DFH + GUID as common header registers.
+ * For private features, they only have DFH register as common header.
+ */
+#define DFH                     0x0
+#define GUID_L                  0x8
+#define GUID_H                  0x10
+#define NEXT_AFU                0x18
+
+#define DFH_SIZE                0x8
+
+/* Device Feature Header Register Bitfield */
+#define DFH_ID                  GENMASK_ULL(11, 0)      /* Feature ID */
+#define DFH_ID_FIU_FME          0
+#define DFH_ID_FIU_PORT         1
+#define DFH_REVISION            GENMASK_ULL(15, 12)
+#define DFH_NEXT_HDR_OFST       GENMASK_ULL(39, 16)     /* Offset to next DFH */
+#define DFH_EOL                 BIT_ULL(40)             /* End of list */
+#define DFH_TYPE                GENMASK_ULL(63, 60)     /* Feature type */
+#define DFH_TYPE_AFU            1
+#define DFH_TYPE_PRIVATE        3
+#define DFH_TYPE_FIU            4
+
+/* Function to read from DFH and check if the Feature type is FME */
+static inline bool dfl_feature_is_fme(void __iomem *base)
+{
+	u64 v = readq(base + DFH);
+
+	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
+		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
+}
+
+/* Function to read from DFH and check if the Feature type is port*/
+static inline bool dfl_feature_is_port(void __iomem *base)
+{
+	u64 v = readq(base + DFH);
+
+	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
+		 (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
+}
+
+/* Function to read feature revision from DFH */
+static inline u8 dfl_feature_revision(void __iomem *base)
+{
+	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
+}
+
 #endif /* __LINUX_DFL_H */
-- 
2.31.0

RE: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Wu, Hao]

> Subject: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h
> 
> From: Debarati Biswas <debaratix.biswas@intel.com>
> 
> Device Feature List (DFL) drivers may be defined in subdirectories other
> than drivers/fpga, and each DFL driver should have access to the Device
> Feature Header (DFH) register, which contains revision and type
> information. This change moves the macros specific to the DFH register
> from drivers/fpga/dfl.h to include/linux/dfl.h.

Looks like it requires to access the revision info in the next patch, because
current dfl_device doesn't expose related information.

@Yilun, do you have any concern to expose those info via dfl_device?

Thanks
Hao

> 
> Signed-off-by: Debarati Biswas <debaratix.biswas@intel.com>
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
> ---
>  drivers/fpga/dfl.h  | 48 +----------------------------------------
>  include/linux/dfl.h | 52 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 53 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index 2b82c96ba56c..6ed0353e9a99 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -17,6 +17,7 @@
>  #include <linux/bitfield.h>
>  #include <linux/cdev.h>
>  #include <linux/delay.h>
> +#include <linux/dfl.h>
>  #include <linux/eventfd.h>
>  #include <linux/fs.h>
>  #include <linux/interrupt.h>
> @@ -53,32 +54,6 @@
>  #define PORT_FEATURE_ID_UINT		0x12
>  #define PORT_FEATURE_ID_STP		0x13
> 
> -/*
> - * Device Feature Header Register Set
> - *
> - * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
> - * For AFUs, they have DFH + GUID as common header registers.
> - * For private features, they only have DFH register as common header.
> - */
> -#define DFH			0x0
> -#define GUID_L			0x8
> -#define GUID_H			0x10
> -#define NEXT_AFU		0x18
> -
> -#define DFH_SIZE		0x8
> -
> -/* Device Feature Header Register Bitfield */
> -#define DFH_ID			GENMASK_ULL(11, 0)	/* Feature ID
> */
> -#define DFH_ID_FIU_FME		0
> -#define DFH_ID_FIU_PORT		1
> -#define DFH_REVISION		GENMASK_ULL(15, 12)	/* Feature revision */
> -#define DFH_NEXT_HDR_OFST	GENMASK_ULL(39, 16)	/* Offset to next DFH
> */
> -#define DFH_EOL			BIT_ULL(40)		/* End of list
> */
> -#define DFH_TYPE		GENMASK_ULL(63, 60)	/* Feature type */
> -#define DFH_TYPE_AFU		1
> -#define DFH_TYPE_PRIVATE	3
> -#define DFH_TYPE_FIU		4
> -
>  /* Next AFU Register Bitfield */
>  #define NEXT_AFU_NEXT_DFH_OFST	GENMASK_ULL(23, 0)	/* Offset to
> next AFU */
> 
> @@ -403,27 +378,6 @@ struct device *dfl_fpga_pdata_to_parent(struct
> dfl_feature_platform_data *pdata)
>  	return pdata->dev->dev.parent->parent;
>  }
> 
> -static inline bool dfl_feature_is_fme(void __iomem *base)
> -{
> -	u64 v = readq(base + DFH);
> -
> -	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> -		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
> -}
> -
> -static inline bool dfl_feature_is_port(void __iomem *base)
> -{
> -	u64 v = readq(base + DFH);
> -
> -	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> -		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
> -}
> -
> -static inline u8 dfl_feature_revision(void __iomem *base)
> -{
> -	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
> -}
> -
>  /**
>   * struct dfl_fpga_enum_info - DFL FPGA enumeration information
>   *
> diff --git a/include/linux/dfl.h b/include/linux/dfl.h
> index 6cc10982351a..1cd86b2e7cb1 100644
> --- a/include/linux/dfl.h
> +++ b/include/linux/dfl.h
> @@ -8,7 +8,9 @@
>  #ifndef __LINUX_DFL_H
>  #define __LINUX_DFL_H
> 
> +#include <linux/bitfield.h>
>  #include <linux/device.h>
> +#include <linux/io.h>
>  #include <linux/mod_devicetable.h>
> 
>  /**
> @@ -83,4 +85,54 @@ void dfl_driver_unregister(struct dfl_driver *dfl_drv);
>  	module_driver(__dfl_driver, dfl_driver_register, \
>  		      dfl_driver_unregister)
> 
> +/*
> + * Device Feature Header Register Set
> + *
> + * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
> + * For AFUs, they have DFH + GUID as common header registers.
> + * For private features, they only have DFH register as common header.
> + */
> +#define DFH                     0x0
> +#define GUID_L                  0x8
> +#define GUID_H                  0x10
> +#define NEXT_AFU                0x18
> +
> +#define DFH_SIZE                0x8
> +
> +/* Device Feature Header Register Bitfield */
> +#define DFH_ID                  GENMASK_ULL(11, 0)      /* Feature ID */
> +#define DFH_ID_FIU_FME          0
> +#define DFH_ID_FIU_PORT         1
> +#define DFH_REVISION            GENMASK_ULL(15, 12)
> +#define DFH_NEXT_HDR_OFST       GENMASK_ULL(39, 16)     /* Offset to next
> DFH */
> +#define DFH_EOL                 BIT_ULL(40)             /* End of list */
> +#define DFH_TYPE                GENMASK_ULL(63, 60)     /* Feature type */
> +#define DFH_TYPE_AFU            1
> +#define DFH_TYPE_PRIVATE        3
> +#define DFH_TYPE_FIU            4
> +
> +/* Function to read from DFH and check if the Feature type is FME */
> +static inline bool dfl_feature_is_fme(void __iomem *base)
> +{
> +	u64 v = readq(base + DFH);
> +
> +	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> +		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
> +}
> +
> +/* Function to read from DFH and check if the Feature type is port*/
> +static inline bool dfl_feature_is_port(void __iomem *base)
> +{
> +	u64 v = readq(base + DFH);
> +
> +	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> +		 (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
> +}
> +
> +/* Function to read feature revision from DFH */
> +static inline u8 dfl_feature_revision(void __iomem *base)
> +{
> +	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
> +}
> +
>  #endif /* __LINUX_DFL_H */
> --
> 2.31.0

Re: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Xu Yilun]

On Mon, Jun 21, 2021 at 06:19:15PM +0800, Wu, Hao wrote:
> > Subject: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h
> >
> > From: Debarati Biswas <debaratix.biswas@intel.com>
> >
> > Device Feature List (DFL) drivers may be defined in subdirectories other
> > than drivers/fpga, and each DFL driver should have access to the Device
> > Feature Header (DFH) register, which contains revision and type
> > information. This change moves the macros specific to the DFH register
> > from drivers/fpga/dfl.h to include/linux/dfl.h.
> 
> Looks like it requires to access the revision info in the next patch, because
> current dfl_device doesn't expose related information.
> 
> @Yilun, do you have any concern to expose those info via dfl_device?

Exposing these header register definitions are good to me. These registers
are in DFL device's MMIO region, so it is good to share these info with
all DFL drivers.

Thanks,
Yilun

> 
> Thanks
> Hao
> 
> >
> > Signed-off-by: Debarati Biswas <debaratix.biswas@intel.com>
> > Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> > Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
> > ---
> >  drivers/fpga/dfl.h  | 48 +----------------------------------------
> >  include/linux/dfl.h | 52 +++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 53 insertions(+), 47 deletions(-)
> >
> > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> > index 2b82c96ba56c..6ed0353e9a99 100644
> > --- a/drivers/fpga/dfl.h
> > +++ b/drivers/fpga/dfl.h
> > @@ -17,6 +17,7 @@
> >  #include <linux/bitfield.h>
> >  #include <linux/cdev.h>
> >  #include <linux/delay.h>
> > +#include <linux/dfl.h>
> >  #include <linux/eventfd.h>
> >  #include <linux/fs.h>
> >  #include <linux/interrupt.h>
> > @@ -53,32 +54,6 @@
> >  #define PORT_FEATURE_ID_UINT         0x12
> >  #define PORT_FEATURE_ID_STP          0x13
> >
> > -/*
> > - * Device Feature Header Register Set
> > - *
> > - * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
> > - * For AFUs, they have DFH + GUID as common header registers.
> > - * For private features, they only have DFH register as common header.
> > - */
> > -#define DFH                  0x0
> > -#define GUID_L                       0x8
> > -#define GUID_H                       0x10
> > -#define NEXT_AFU             0x18
> > -
> > -#define DFH_SIZE             0x8
> > -
> > -/* Device Feature Header Register Bitfield */
> > -#define DFH_ID                       GENMASK_ULL(11, 0)      /* Feature ID
> > */
> > -#define DFH_ID_FIU_FME               0
> > -#define DFH_ID_FIU_PORT              1
> > -#define DFH_REVISION         GENMASK_ULL(15, 12)     /* Feature revision */
> > -#define DFH_NEXT_HDR_OFST    GENMASK_ULL(39, 16)     /* Offset to next DFH
> > */
> > -#define DFH_EOL                      BIT_ULL(40)             /* End of list
> > */
> > -#define DFH_TYPE             GENMASK_ULL(63, 60)     /* Feature type */
> > -#define DFH_TYPE_AFU         1
> > -#define DFH_TYPE_PRIVATE     3
> > -#define DFH_TYPE_FIU         4
> > -
> >  /* Next AFU Register Bitfield */
> >  #define NEXT_AFU_NEXT_DFH_OFST       GENMASK_ULL(23, 0)      /* Offset to
> > next AFU */
> >
> > @@ -403,27 +378,6 @@ struct device *dfl_fpga_pdata_to_parent(struct
> > dfl_feature_platform_data *pdata)
> >       return pdata->dev->dev.parent->parent;
> >  }
> >
> > -static inline bool dfl_feature_is_fme(void __iomem *base)
> > -{
> > -     u64 v = readq(base + DFH);
> > -
> > -     return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > -             (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
> > -}
> > -
> > -static inline bool dfl_feature_is_port(void __iomem *base)
> > -{
> > -     u64 v = readq(base + DFH);
> > -
> > -     return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > -             (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
> > -}
> > -
> > -static inline u8 dfl_feature_revision(void __iomem *base)
> > -{
> > -     return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
> > -}
> > -
> >  /**
> >   * struct dfl_fpga_enum_info - DFL FPGA enumeration information
> >   *
> > diff --git a/include/linux/dfl.h b/include/linux/dfl.h
> > index 6cc10982351a..1cd86b2e7cb1 100644
> > --- a/include/linux/dfl.h
> > +++ b/include/linux/dfl.h
> > @@ -8,7 +8,9 @@
> >  #ifndef __LINUX_DFL_H
> >  #define __LINUX_DFL_H
> >
> > +#include <linux/bitfield.h>
> >  #include <linux/device.h>
> > +#include <linux/io.h>
> >  #include <linux/mod_devicetable.h>

The <linux/io-64-nonatomic-lo-hi.h> is needed here, or readq/writeq
definitions may be missing on some platform, as the kernel test robot
says.

Thanks,
Yilun

> >
> >  /**
> > @@ -83,4 +85,54 @@ void dfl_driver_unregister(struct dfl_driver *dfl_drv);
> >       module_driver(__dfl_driver, dfl_driver_register, \
> >                     dfl_driver_unregister)
> >
> > +/*
> > + * Device Feature Header Register Set
> > + *
> > + * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
> > + * For AFUs, they have DFH + GUID as common header registers.
> > + * For private features, they only have DFH register as common header.
> > + */
> > +#define DFH                     0x0
> > +#define GUID_L                  0x8
> > +#define GUID_H                  0x10
> > +#define NEXT_AFU                0x18
> > +
> > +#define DFH_SIZE                0x8
> > +
> > +/* Device Feature Header Register Bitfield */
> > +#define DFH_ID                  GENMASK_ULL(11, 0)      /* Feature ID */
> > +#define DFH_ID_FIU_FME          0
> > +#define DFH_ID_FIU_PORT         1
> > +#define DFH_REVISION            GENMASK_ULL(15, 12)
> > +#define DFH_NEXT_HDR_OFST       GENMASK_ULL(39, 16)     /* Offset to next
> > DFH */
> > +#define DFH_EOL                 BIT_ULL(40)             /* End of list */
> > +#define DFH_TYPE                GENMASK_ULL(63, 60)     /* Feature type */
> > +#define DFH_TYPE_AFU            1
> > +#define DFH_TYPE_PRIVATE        3
> > +#define DFH_TYPE_FIU            4
> > +
> > +/* Function to read from DFH and check if the Feature type is FME */
> > +static inline bool dfl_feature_is_fme(void __iomem *base)
> > +{
> > +     u64 v = readq(base + DFH);
> > +
> > +     return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > +             (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
> > +}
> > +
> > +/* Function to read from DFH and check if the Feature type is port*/
> > +static inline bool dfl_feature_is_port(void __iomem *base)
> > +{
> > +     u64 v = readq(base + DFH);
> > +
> > +     return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > +              (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
> > +}
> > +
> > +/* Function to read feature revision from DFH */
> > +static inline u8 dfl_feature_revision(void __iomem *base)
> > +{
> > +     return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
> > +}
> > +
> >  #endif /* __LINUX_DFL_H */
> > --
> > 2.31.0
> 

RE: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Wu, Hao]

> On Mon, Jun 21, 2021 at 06:19:15PM +0800, Wu, Hao wrote:
> > > Subject: [PATCH 2/4] fpga: dfl: Move DFH header register macros to
> linux/dfl.h
> > >
> > > From: Debarati Biswas <debaratix.biswas@intel.com>
> > >
> > > Device Feature List (DFL) drivers may be defined in subdirectories other
> > > than drivers/fpga, and each DFL driver should have access to the Device
> > > Feature Header (DFH) register, which contains revision and type
> > > information. This change moves the macros specific to the DFH register
> > > from drivers/fpga/dfl.h to include/linux/dfl.h.
> >
> > Looks like it requires to access the revision info in the next patch, because
> > current dfl_device doesn't expose related information.
> >
> > @Yilun, do you have any concern to expose those info via dfl_device?
> 
> Exposing these header register definitions are good to me. These registers
> are in DFL device's MMIO region, so it is good to share these info with
> all DFL drivers.

I mean expose revision via dfl_device, as dfl core already reads the DFL
header, it sounds duplicate read in each dfl device driver. And if we
consider this as a common need from dfl device driver, then the code
can be moved to a common place as well.

I hope from dfl device driver side, it doesn't need to know details of
how DFH register is defined, only simple way from dfl device data
structure or some simple helper function, then dfl device driver could
know all common information from DFH.

How do you think?

Thanks
Hao

> 
> Thanks,
> Yilun
> 
> >
> > Thanks
> > Hao
> >
> > >
> > > Signed-off-by: Debarati Biswas <debaratix.biswas@intel.com>
> > > Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> > > Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
> > > ---
> > >  drivers/fpga/dfl.h  | 48 +----------------------------------------
> > >  include/linux/dfl.h | 52
> +++++++++++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 53 insertions(+), 47 deletions(-)
> > >
> > > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> > > index 2b82c96ba56c..6ed0353e9a99 100644
> > > --- a/drivers/fpga/dfl.h
> > > +++ b/drivers/fpga/dfl.h
> > > @@ -17,6 +17,7 @@
> > >  #include <linux/bitfield.h>
> > >  #include <linux/cdev.h>
> > >  #include <linux/delay.h>
> > > +#include <linux/dfl.h>
> > >  #include <linux/eventfd.h>
> > >  #include <linux/fs.h>
> > >  #include <linux/interrupt.h>
> > > @@ -53,32 +54,6 @@
> > >  #define PORT_FEATURE_ID_UINT         0x12
> > >  #define PORT_FEATURE_ID_STP          0x13
> > >
> > > -/*
> > > - * Device Feature Header Register Set
> > > - *
> > > - * For FIUs, they all have DFH + GUID + NEXT_AFU as common header
> registers.
> > > - * For AFUs, they have DFH + GUID as common header registers.
> > > - * For private features, they only have DFH register as common header.
> > > - */
> > > -#define DFH                  0x0
> > > -#define GUID_L                       0x8
> > > -#define GUID_H                       0x10
> > > -#define NEXT_AFU             0x18
> > > -
> > > -#define DFH_SIZE             0x8
> > > -
> > > -/* Device Feature Header Register Bitfield */
> > > -#define DFH_ID                       GENMASK_ULL(11, 0)      /* Feature ID
> > > */
> > > -#define DFH_ID_FIU_FME               0
> > > -#define DFH_ID_FIU_PORT              1
> > > -#define DFH_REVISION         GENMASK_ULL(15, 12)     /* Feature revision */
> > > -#define DFH_NEXT_HDR_OFST    GENMASK_ULL(39, 16)     /* Offset to next
> DFH
> > > */
> > > -#define DFH_EOL                      BIT_ULL(40)             /* End of list
> > > */
> > > -#define DFH_TYPE             GENMASK_ULL(63, 60)     /* Feature type */
> > > -#define DFH_TYPE_AFU         1
> > > -#define DFH_TYPE_PRIVATE     3
> > > -#define DFH_TYPE_FIU         4
> > > -
> > >  /* Next AFU Register Bitfield */
> > >  #define NEXT_AFU_NEXT_DFH_OFST       GENMASK_ULL(23, 0)      /* Offset
> to
> > > next AFU */
> > >
> > > @@ -403,27 +378,6 @@ struct device *dfl_fpga_pdata_to_parent(struct
> > > dfl_feature_platform_data *pdata)
> > >       return pdata->dev->dev.parent->parent;
> > >  }
> > >
> > > -static inline bool dfl_feature_is_fme(void __iomem *base)
> > > -{
> > > -     u64 v = readq(base + DFH);
> > > -
> > > -     return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > > -             (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
> > > -}
> > > -
> > > -static inline bool dfl_feature_is_port(void __iomem *base)
> > > -{
> > > -     u64 v = readq(base + DFH);
> > > -
> > > -     return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > > -             (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
> > > -}
> > > -
> > > -static inline u8 dfl_feature_revision(void __iomem *base)
> > > -{
> > > -     return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
> > > -}
> > > -
> > >  /**
> > >   * struct dfl_fpga_enum_info - DFL FPGA enumeration information
> > >   *
> > > diff --git a/include/linux/dfl.h b/include/linux/dfl.h
> > > index 6cc10982351a..1cd86b2e7cb1 100644
> > > --- a/include/linux/dfl.h
> > > +++ b/include/linux/dfl.h
> > > @@ -8,7 +8,9 @@
> > >  #ifndef __LINUX_DFL_H
> > >  #define __LINUX_DFL_H
> > >
> > > +#include <linux/bitfield.h>
> > >  #include <linux/device.h>
> > > +#include <linux/io.h>
> > >  #include <linux/mod_devicetable.h>
> 
> The <linux/io-64-nonatomic-lo-hi.h> is needed here, or readq/writeq
> definitions may be missing on some platform, as the kernel test robot
> says.
> 
> Thanks,
> Yilun
> 
> > >
> > >  /**
> > > @@ -83,4 +85,54 @@ void dfl_driver_unregister(struct dfl_driver *dfl_drv);
> > >       module_driver(__dfl_driver, dfl_driver_register, \
> > >                     dfl_driver_unregister)
> > >
> > > +/*
> > > + * Device Feature Header Register Set
> > > + *
> > > + * For FIUs, they all have DFH + GUID + NEXT_AFU as common header
> registers.
> > > + * For AFUs, they have DFH + GUID as common header registers.
> > > + * For private features, they only have DFH register as common header.
> > > + */
> > > +#define DFH                     0x0
> > > +#define GUID_L                  0x8
> > > +#define GUID_H                  0x10
> > > +#define NEXT_AFU                0x18
> > > +
> > > +#define DFH_SIZE                0x8
> > > +
> > > +/* Device Feature Header Register Bitfield */
> > > +#define DFH_ID                  GENMASK_ULL(11, 0)      /* Feature ID */
> > > +#define DFH_ID_FIU_FME          0
> > > +#define DFH_ID_FIU_PORT         1
> > > +#define DFH_REVISION            GENMASK_ULL(15, 12)
> > > +#define DFH_NEXT_HDR_OFST       GENMASK_ULL(39, 16)     /* Offset to
> next
> > > DFH */
> > > +#define DFH_EOL                 BIT_ULL(40)             /* End of list */
> > > +#define DFH_TYPE                GENMASK_ULL(63, 60)     /* Feature type */
> > > +#define DFH_TYPE_AFU            1
> > > +#define DFH_TYPE_PRIVATE        3
> > > +#define DFH_TYPE_FIU            4
> > > +
> > > +/* Function to read from DFH and check if the Feature type is FME */
> > > +static inline bool dfl_feature_is_fme(void __iomem *base)
> > > +{
> > > +     u64 v = readq(base + DFH);
> > > +
> > > +     return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > > +             (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
> > > +}
> > > +
> > > +/* Function to read from DFH and check if the Feature type is port*/
> > > +static inline bool dfl_feature_is_port(void __iomem *base)
> > > +{
> > > +     u64 v = readq(base + DFH);
> > > +
> > > +     return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > > +              (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
> > > +}
> > > +
> > > +/* Function to read feature revision from DFH */
> > > +static inline u8 dfl_feature_revision(void __iomem *base)
> > > +{
> > > +     return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
> > > +}
> > > +
> > >  #endif /* __LINUX_DFL_H */
> > > --
> > > 2.31.0
> >

Re: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Martin Hundebøll]

On 22/06/2021 09.39, Wu, Hao wrote:
>> On Mon, Jun 21, 2021 at 06:19:15PM +0800, Wu, Hao wrote:
>>>> Subject: [PATCH 2/4] fpga: dfl: Move DFH header register macros to
>> linux/dfl.h
>>>>
>>>> From: Debarati Biswas <debaratix.biswas@intel.com>
>>>>
>>>> Device Feature List (DFL) drivers may be defined in subdirectories other
>>>> than drivers/fpga, and each DFL driver should have access to the Device
>>>> Feature Header (DFH) register, which contains revision and type
>>>> information. This change moves the macros specific to the DFH register
>>>> from drivers/fpga/dfl.h to include/linux/dfl.h.
>>>
>>> Looks like it requires to access the revision info in the next patch, because
>>> current dfl_device doesn't expose related information.
>>>
>>> @Yilun, do you have any concern to expose those info via dfl_device?
>>
>> Exposing these header register definitions are good to me. These registers
>> are in DFL device's MMIO region, so it is good to share these info with
>> all DFL drivers.
> 
> I mean expose revision via dfl_device, as dfl core already reads the DFL
> header, it sounds duplicate read in each dfl device driver. And if we
> consider this as a common need from dfl device driver, then the code
> can be moved to a common place as well.
> 
> I hope from dfl device driver side, it doesn't need to know details of
> how DFH register is defined, only simple way from dfl device data
> structure or some simple helper function, then dfl device driver could
> know all common information from DFH.
> 
> How do you think?

struct dfl_device {} already has "u16 type" and "u16 feature_id", so it would make sense to add "u8 feature_rev" as well?

// Martin

Re: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Xu Yilun]

On Wed, Jun 23, 2021 at 01:56:59PM +0200, Martin Hundebøll wrote:
> 
> 
> On 22/06/2021 09.39, Wu, Hao wrote:
> > > On Mon, Jun 21, 2021 at 06:19:15PM +0800, Wu, Hao wrote:
> > > > > Subject: [PATCH 2/4] fpga: dfl: Move DFH header register macros to
> > > linux/dfl.h
> > > > > 
> > > > > From: Debarati Biswas <debaratix.biswas@intel.com>
> > > > > 
> > > > > Device Feature List (DFL) drivers may be defined in subdirectories other
> > > > > than drivers/fpga, and each DFL driver should have access to the Device
> > > > > Feature Header (DFH) register, which contains revision and type
> > > > > information. This change moves the macros specific to the DFH register
> > > > > from drivers/fpga/dfl.h to include/linux/dfl.h.
> > > > 
> > > > Looks like it requires to access the revision info in the next patch, because
> > > > current dfl_device doesn't expose related information.
> > > > 
> > > > @Yilun, do you have any concern to expose those info via dfl_device?
> > > 
> > > Exposing these header register definitions are good to me. These registers
> > > are in DFL device's MMIO region, so it is good to share these info with
> > > all DFL drivers.
> > 
> > I mean expose revision via dfl_device, as dfl core already reads the DFL
> > header, it sounds duplicate read in each dfl device driver. And if we
> > consider this as a common need from dfl device driver, then the code
> > can be moved to a common place as well.
> > 
> > I hope from dfl device driver side, it doesn't need to know details of
> > how DFH register is defined, only simple way from dfl device data
> > structure or some simple helper function, then dfl device driver could
> > know all common information from DFH.
> > 
> > How do you think?

It's good idea.

> 
> struct dfl_device {} already has "u16 type" and "u16 feature_id", so it would make sense to add "u8 feature_rev" as well?

I think we may name it "u8 revision".

Thanks,
Yilun

> 
> // Martin

RE: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Wu, Hao]

> On Wed, Jun 23, 2021 at 01:56:59PM +0200, Martin Hundebøll wrote:
> >
> >
> > On 22/06/2021 09.39, Wu, Hao wrote:
> > > > On Mon, Jun 21, 2021 at 06:19:15PM +0800, Wu, Hao wrote:
> > > > > > Subject: [PATCH 2/4] fpga: dfl: Move DFH header register macros to
> > > > linux/dfl.h
> > > > > >
> > > > > > From: Debarati Biswas <debaratix.biswas@intel.com>
> > > > > >
> > > > > > Device Feature List (DFL) drivers may be defined in subdirectories other
> > > > > > than drivers/fpga, and each DFL driver should have access to the Device
> > > > > > Feature Header (DFH) register, which contains revision and type
> > > > > > information. This change moves the macros specific to the DFH register
> > > > > > from drivers/fpga/dfl.h to include/linux/dfl.h.
> > > > >
> > > > > Looks like it requires to access the revision info in the next patch,
> because
> > > > > current dfl_device doesn't expose related information.
> > > > >
> > > > > @Yilun, do you have any concern to expose those info via dfl_device?
> > > >
> > > > Exposing these header register definitions are good to me. These registers
> > > > are in DFL device's MMIO region, so it is good to share these info with
> > > > all DFL drivers.
> > >
> > > I mean expose revision via dfl_device, as dfl core already reads the DFL
> > > header, it sounds duplicate read in each dfl device driver. And if we
> > > consider this as a common need from dfl device driver, then the code
> > > can be moved to a common place as well.
> > >
> > > I hope from dfl device driver side, it doesn't need to know details of
> > > how DFH register is defined, only simple way from dfl device data
> > > structure or some simple helper function, then dfl device driver could
> > > know all common information from DFH.
> > >
> > > How do you think?
> 
> It's good idea.
> 
> >
> > struct dfl_device {} already has "u16 type" and "u16 feature_id", so it would
> make sense to add "u8 feature_rev" as well?
> 
> I think we may name it "u8 revision".

Sounds good.

Then we don't have to expose detailed registers information to such header file.
dfl_device data structure + helper function should be enough. : )

Thanks
Hao

> 
> Thanks,
> Yilun
> 
> >
> > // Martin

Re: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Tom Rix]

On 6/21/21 12:06 AM, Martin Hundebøll wrote:
> From: Debarati Biswas <debaratix.biswas@intel.com>
>
> Device Feature List (DFL) drivers may be defined in subdirectories other
> than drivers/fpga, and each DFL driver should have access to the Device
> Feature Header (DFH) register, which contains revision and type
> information. This change moves the macros specific to the DFH register
> from drivers/fpga/dfl.h to include/linux/dfl.h.
>
> Signed-off-by: Debarati Biswas <debaratix.biswas@intel.com>
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
> ---
>   drivers/fpga/dfl.h  | 48 +----------------------------------------
>   include/linux/dfl.h | 52 +++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 53 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> index 2b82c96ba56c..6ed0353e9a99 100644
> --- a/drivers/fpga/dfl.h
> +++ b/drivers/fpga/dfl.h
> @@ -17,6 +17,7 @@
>   #include <linux/bitfield.h>

bitfield.h was added to linux/dfl.h

Likely both aren't needed, try removing this one.

Tom

>   #include <linux/cdev.h>
>   #include <linux/delay.h>
> +#include <linux/dfl.h>
>   #include <linux/eventfd.h>
>   #include <linux/fs.h>
>   #include <linux/interrupt.h>
> @@ -53,32 +54,6 @@
>   #define PORT_FEATURE_ID_UINT		0x12
>   #define PORT_FEATURE_ID_STP		0x13
>   
> -/*
> - * Device Feature Header Register Set
> - *
> - * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
> - * For AFUs, they have DFH + GUID as common header registers.
> - * For private features, they only have DFH register as common header.
> - */
> -#define DFH			0x0
> -#define GUID_L			0x8
> -#define GUID_H			0x10
> -#define NEXT_AFU		0x18
> -
> -#define DFH_SIZE		0x8
> -
> -/* Device Feature Header Register Bitfield */
> -#define DFH_ID			GENMASK_ULL(11, 0)	/* Feature ID */
> -#define DFH_ID_FIU_FME		0
> -#define DFH_ID_FIU_PORT		1
> -#define DFH_REVISION		GENMASK_ULL(15, 12)	/* Feature revision */
> -#define DFH_NEXT_HDR_OFST	GENMASK_ULL(39, 16)	/* Offset to next DFH */
> -#define DFH_EOL			BIT_ULL(40)		/* End of list */
> -#define DFH_TYPE		GENMASK_ULL(63, 60)	/* Feature type */
> -#define DFH_TYPE_AFU		1
> -#define DFH_TYPE_PRIVATE	3
> -#define DFH_TYPE_FIU		4
> -
>   /* Next AFU Register Bitfield */
>   #define NEXT_AFU_NEXT_DFH_OFST	GENMASK_ULL(23, 0)	/* Offset to next AFU */
>   
> @@ -403,27 +378,6 @@ struct device *dfl_fpga_pdata_to_parent(struct dfl_feature_platform_data *pdata)
>   	return pdata->dev->dev.parent->parent;
>   }
>   
> -static inline bool dfl_feature_is_fme(void __iomem *base)
> -{
> -	u64 v = readq(base + DFH);
> -
> -	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> -		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
> -}
> -
> -static inline bool dfl_feature_is_port(void __iomem *base)
> -{
> -	u64 v = readq(base + DFH);
> -
> -	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> -		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
> -}
> -
> -static inline u8 dfl_feature_revision(void __iomem *base)
> -{
> -	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
> -}
> -
>   /**
>    * struct dfl_fpga_enum_info - DFL FPGA enumeration information
>    *
> diff --git a/include/linux/dfl.h b/include/linux/dfl.h
> index 6cc10982351a..1cd86b2e7cb1 100644
> --- a/include/linux/dfl.h
> +++ b/include/linux/dfl.h
> @@ -8,7 +8,9 @@
>   #ifndef __LINUX_DFL_H
>   #define __LINUX_DFL_H
>   
> +#include <linux/bitfield.h>
>   #include <linux/device.h>
> +#include <linux/io.h>
>   #include <linux/mod_devicetable.h>
>   
>   /**
> @@ -83,4 +85,54 @@ void dfl_driver_unregister(struct dfl_driver *dfl_drv);
>   	module_driver(__dfl_driver, dfl_driver_register, \
>   		      dfl_driver_unregister)
>   
> +/*
> + * Device Feature Header Register Set
> + *
> + * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
> + * For AFUs, they have DFH + GUID as common header registers.
> + * For private features, they only have DFH register as common header.
> + */
> +#define DFH                     0x0
> +#define GUID_L                  0x8
> +#define GUID_H                  0x10
> +#define NEXT_AFU                0x18
> +
> +#define DFH_SIZE                0x8
> +
> +/* Device Feature Header Register Bitfield */
> +#define DFH_ID                  GENMASK_ULL(11, 0)      /* Feature ID */
> +#define DFH_ID_FIU_FME          0
> +#define DFH_ID_FIU_PORT         1
> +#define DFH_REVISION            GENMASK_ULL(15, 12)
> +#define DFH_NEXT_HDR_OFST       GENMASK_ULL(39, 16)     /* Offset to next DFH */
> +#define DFH_EOL                 BIT_ULL(40)             /* End of list */
> +#define DFH_TYPE                GENMASK_ULL(63, 60)     /* Feature type */
> +#define DFH_TYPE_AFU            1
> +#define DFH_TYPE_PRIVATE        3
> +#define DFH_TYPE_FIU            4
> +
> +/* Function to read from DFH and check if the Feature type is FME */
> +static inline bool dfl_feature_is_fme(void __iomem *base)
> +{
> +	u64 v = readq(base + DFH);
> +
> +	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> +		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
> +}
> +
> +/* Function to read from DFH and check if the Feature type is port*/
> +static inline bool dfl_feature_is_port(void __iomem *base)
> +{
> +	u64 v = readq(base + DFH);
> +
> +	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> +		 (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
> +}
> +
> +/* Function to read feature revision from DFH */
> +static inline u8 dfl_feature_revision(void __iomem *base)
> +{
> +	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
> +}
> +
>   #endif /* __LINUX_DFL_H */

Re: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Xu Yilun]

On Mon, Jun 21, 2021 at 06:56:28AM -0700, Tom Rix wrote:
> 
> On 6/21/21 12:06 AM, Martin Hundebøll wrote:
> > From: Debarati Biswas <debaratix.biswas@intel.com>
> > 
> > Device Feature List (DFL) drivers may be defined in subdirectories other
> > than drivers/fpga, and each DFL driver should have access to the Device
> > Feature Header (DFH) register, which contains revision and type
> > information. This change moves the macros specific to the DFH register
> > from drivers/fpga/dfl.h to include/linux/dfl.h.
> > 
> > Signed-off-by: Debarati Biswas <debaratix.biswas@intel.com>
> > Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> > Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
> > ---
> >   drivers/fpga/dfl.h  | 48 +----------------------------------------
> >   include/linux/dfl.h | 52 +++++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 53 insertions(+), 47 deletions(-)
> > 
> > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> > index 2b82c96ba56c..6ed0353e9a99 100644
> > --- a/drivers/fpga/dfl.h
> > +++ b/drivers/fpga/dfl.h
> > @@ -17,6 +17,7 @@
> >   #include <linux/bitfield.h>
> 
> bitfield.h was added to linux/dfl.h
> 
> Likely both aren't needed, try removing this one.

The DFL register definitions are in dfl.h, and Source files which include
dfl.h are likely to use bitfield ops for DFL register access, so could we
keep it here?

Thanks,
Yilun

> 
> Tom
> 
> >   #include <linux/cdev.h>
> >   #include <linux/delay.h>
> > +#include <linux/dfl.h>
> >   #include <linux/eventfd.h>
> >   #include <linux/fs.h>
> >   #include <linux/interrupt.h>
> > @@ -53,32 +54,6 @@
> >   #define PORT_FEATURE_ID_UINT		0x12
> >   #define PORT_FEATURE_ID_STP		0x13
> > -/*
> > - * Device Feature Header Register Set
> > - *
> > - * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
> > - * For AFUs, they have DFH + GUID as common header registers.
> > - * For private features, they only have DFH register as common header.
> > - */
> > -#define DFH			0x0
> > -#define GUID_L			0x8
> > -#define GUID_H			0x10
> > -#define NEXT_AFU		0x18
> > -
> > -#define DFH_SIZE		0x8
> > -
> > -/* Device Feature Header Register Bitfield */
> > -#define DFH_ID			GENMASK_ULL(11, 0)	/* Feature ID */
> > -#define DFH_ID_FIU_FME		0
> > -#define DFH_ID_FIU_PORT		1
> > -#define DFH_REVISION		GENMASK_ULL(15, 12)	/* Feature revision */
> > -#define DFH_NEXT_HDR_OFST	GENMASK_ULL(39, 16)	/* Offset to next DFH */
> > -#define DFH_EOL			BIT_ULL(40)		/* End of list */
> > -#define DFH_TYPE		GENMASK_ULL(63, 60)	/* Feature type */
> > -#define DFH_TYPE_AFU		1
> > -#define DFH_TYPE_PRIVATE	3
> > -#define DFH_TYPE_FIU		4
> > -
> >   /* Next AFU Register Bitfield */
> >   #define NEXT_AFU_NEXT_DFH_OFST	GENMASK_ULL(23, 0)	/* Offset to next AFU */
> > @@ -403,27 +378,6 @@ struct device *dfl_fpga_pdata_to_parent(struct dfl_feature_platform_data *pdata)
> >   	return pdata->dev->dev.parent->parent;
> >   }
> > -static inline bool dfl_feature_is_fme(void __iomem *base)
> > -{
> > -	u64 v = readq(base + DFH);
> > -
> > -	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > -		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
> > -}
> > -
> > -static inline bool dfl_feature_is_port(void __iomem *base)
> > -{
> > -	u64 v = readq(base + DFH);
> > -
> > -	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > -		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
> > -}
> > -
> > -static inline u8 dfl_feature_revision(void __iomem *base)
> > -{
> > -	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
> > -}
> > -
> >   /**
> >    * struct dfl_fpga_enum_info - DFL FPGA enumeration information
> >    *
> > diff --git a/include/linux/dfl.h b/include/linux/dfl.h
> > index 6cc10982351a..1cd86b2e7cb1 100644
> > --- a/include/linux/dfl.h
> > +++ b/include/linux/dfl.h
> > @@ -8,7 +8,9 @@
> >   #ifndef __LINUX_DFL_H
> >   #define __LINUX_DFL_H
> > +#include <linux/bitfield.h>
> >   #include <linux/device.h>
> > +#include <linux/io.h>
> >   #include <linux/mod_devicetable.h>
> >   /**
> > @@ -83,4 +85,54 @@ void dfl_driver_unregister(struct dfl_driver *dfl_drv);
> >   	module_driver(__dfl_driver, dfl_driver_register, \
> >   		      dfl_driver_unregister)
> > +/*
> > + * Device Feature Header Register Set
> > + *
> > + * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
> > + * For AFUs, they have DFH + GUID as common header registers.
> > + * For private features, they only have DFH register as common header.
> > + */
> > +#define DFH                     0x0
> > +#define GUID_L                  0x8
> > +#define GUID_H                  0x10
> > +#define NEXT_AFU                0x18
> > +
> > +#define DFH_SIZE                0x8
> > +
> > +/* Device Feature Header Register Bitfield */
> > +#define DFH_ID                  GENMASK_ULL(11, 0)      /* Feature ID */
> > +#define DFH_ID_FIU_FME          0
> > +#define DFH_ID_FIU_PORT         1
> > +#define DFH_REVISION            GENMASK_ULL(15, 12)
> > +#define DFH_NEXT_HDR_OFST       GENMASK_ULL(39, 16)     /* Offset to next DFH */
> > +#define DFH_EOL                 BIT_ULL(40)             /* End of list */
> > +#define DFH_TYPE                GENMASK_ULL(63, 60)     /* Feature type */
> > +#define DFH_TYPE_AFU            1
> > +#define DFH_TYPE_PRIVATE        3
> > +#define DFH_TYPE_FIU            4
> > +
> > +/* Function to read from DFH and check if the Feature type is FME */
> > +static inline bool dfl_feature_is_fme(void __iomem *base)
> > +{
> > +	u64 v = readq(base + DFH);
> > +
> > +	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > +		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
> > +}
> > +
> > +/* Function to read from DFH and check if the Feature type is port*/
> > +static inline bool dfl_feature_is_port(void __iomem *base)
> > +{
> > +	u64 v = readq(base + DFH);
> > +
> > +	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > +		 (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
> > +}
> > +
> > +/* Function to read feature revision from DFH */
> > +static inline u8 dfl_feature_revision(void __iomem *base)
> > +{
> > +	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
> > +}
> > +
> >   #endif /* __LINUX_DFL_H */

Re: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Tom Rix]

On 6/21/21 9:56 PM, Xu Yilun wrote:
> On Mon, Jun 21, 2021 at 06:56:28AM -0700, Tom Rix wrote:
>> On 6/21/21 12:06 AM, Martin Hundebøll wrote:
>>> From: Debarati Biswas <debaratix.biswas@intel.com>
>>>
>>> Device Feature List (DFL) drivers may be defined in subdirectories other
>>> than drivers/fpga, and each DFL driver should have access to the Device
>>> Feature Header (DFH) register, which contains revision and type
>>> information. This change moves the macros specific to the DFH register
>>> from drivers/fpga/dfl.h to include/linux/dfl.h.
>>>
>>> Signed-off-by: Debarati Biswas <debaratix.biswas@intel.com>
>>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
>>> Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
>>> ---
>>>    drivers/fpga/dfl.h  | 48 +----------------------------------------
>>>    include/linux/dfl.h | 52 +++++++++++++++++++++++++++++++++++++++++++++
>>>    2 files changed, 53 insertions(+), 47 deletions(-)
>>>
>>> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
>>> index 2b82c96ba56c..6ed0353e9a99 100644
>>> --- a/drivers/fpga/dfl.h
>>> +++ b/drivers/fpga/dfl.h
>>> @@ -17,6 +17,7 @@
>>>    #include <linux/bitfield.h>
>> bitfield.h was added to linux/dfl.h
>>
>> Likely both aren't needed, try removing this one.
> The DFL register definitions are in dfl.h, and Source files which include
> dfl.h are likely to use bitfield ops for DFL register access, so could we
> keep it here?

Not sure which dfl.h you mean.

It looks like you are copying header includes in both files and 
including the one file in the other.

So it is likely you are including headers you don't need.

Tom

>
> Thanks,
> Yilun
>
>> Tom
>>
>>>    #include <linux/cdev.h>
>>>    #include <linux/delay.h>
>>> +#include <linux/dfl.h>
>>>    #include <linux/eventfd.h>
>>>    #include <linux/fs.h>
>>>    #include <linux/interrupt.h>
>>> @@ -53,32 +54,6 @@
>>>    #define PORT_FEATURE_ID_UINT		0x12
>>>    #define PORT_FEATURE_ID_STP		0x13
>>> -/*
>>> - * Device Feature Header Register Set
>>> - *
>>> - * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
>>> - * For AFUs, they have DFH + GUID as common header registers.
>>> - * For private features, they only have DFH register as common header.
>>> - */
>>> -#define DFH			0x0
>>> -#define GUID_L			0x8
>>> -#define GUID_H			0x10
>>> -#define NEXT_AFU		0x18
>>> -
>>> -#define DFH_SIZE		0x8
>>> -
>>> -/* Device Feature Header Register Bitfield */
>>> -#define DFH_ID			GENMASK_ULL(11, 0)	/* Feature ID */
>>> -#define DFH_ID_FIU_FME		0
>>> -#define DFH_ID_FIU_PORT		1
>>> -#define DFH_REVISION		GENMASK_ULL(15, 12)	/* Feature revision */
>>> -#define DFH_NEXT_HDR_OFST	GENMASK_ULL(39, 16)	/* Offset to next DFH */
>>> -#define DFH_EOL			BIT_ULL(40)		/* End of list */
>>> -#define DFH_TYPE		GENMASK_ULL(63, 60)	/* Feature type */
>>> -#define DFH_TYPE_AFU		1
>>> -#define DFH_TYPE_PRIVATE	3
>>> -#define DFH_TYPE_FIU		4
>>> -
>>>    /* Next AFU Register Bitfield */
>>>    #define NEXT_AFU_NEXT_DFH_OFST	GENMASK_ULL(23, 0)	/* Offset to next AFU */
>>> @@ -403,27 +378,6 @@ struct device *dfl_fpga_pdata_to_parent(struct dfl_feature_platform_data *pdata)
>>>    	return pdata->dev->dev.parent->parent;
>>>    }
>>> -static inline bool dfl_feature_is_fme(void __iomem *base)
>>> -{
>>> -	u64 v = readq(base + DFH);
>>> -
>>> -	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
>>> -		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
>>> -}
>>> -
>>> -static inline bool dfl_feature_is_port(void __iomem *base)
>>> -{
>>> -	u64 v = readq(base + DFH);
>>> -
>>> -	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
>>> -		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
>>> -}
>>> -
>>> -static inline u8 dfl_feature_revision(void __iomem *base)
>>> -{
>>> -	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
>>> -}
>>> -
>>>    /**
>>>     * struct dfl_fpga_enum_info - DFL FPGA enumeration information
>>>     *
>>> diff --git a/include/linux/dfl.h b/include/linux/dfl.h
>>> index 6cc10982351a..1cd86b2e7cb1 100644
>>> --- a/include/linux/dfl.h
>>> +++ b/include/linux/dfl.h
>>> @@ -8,7 +8,9 @@
>>>    #ifndef __LINUX_DFL_H
>>>    #define __LINUX_DFL_H
>>> +#include <linux/bitfield.h>
>>>    #include <linux/device.h>
>>> +#include <linux/io.h>
>>>    #include <linux/mod_devicetable.h>
>>>    /**
>>> @@ -83,4 +85,54 @@ void dfl_driver_unregister(struct dfl_driver *dfl_drv);
>>>    	module_driver(__dfl_driver, dfl_driver_register, \
>>>    		      dfl_driver_unregister)
>>> +/*
>>> + * Device Feature Header Register Set
>>> + *
>>> + * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
>>> + * For AFUs, they have DFH + GUID as common header registers.
>>> + * For private features, they only have DFH register as common header.
>>> + */
>>> +#define DFH                     0x0
>>> +#define GUID_L                  0x8
>>> +#define GUID_H                  0x10
>>> +#define NEXT_AFU                0x18
>>> +
>>> +#define DFH_SIZE                0x8
>>> +
>>> +/* Device Feature Header Register Bitfield */
>>> +#define DFH_ID                  GENMASK_ULL(11, 0)      /* Feature ID */
>>> +#define DFH_ID_FIU_FME          0
>>> +#define DFH_ID_FIU_PORT         1
>>> +#define DFH_REVISION            GENMASK_ULL(15, 12)
>>> +#define DFH_NEXT_HDR_OFST       GENMASK_ULL(39, 16)     /* Offset to next DFH */
>>> +#define DFH_EOL                 BIT_ULL(40)             /* End of list */
>>> +#define DFH_TYPE                GENMASK_ULL(63, 60)     /* Feature type */
>>> +#define DFH_TYPE_AFU            1
>>> +#define DFH_TYPE_PRIVATE        3
>>> +#define DFH_TYPE_FIU            4
>>> +
>>> +/* Function to read from DFH and check if the Feature type is FME */
>>> +static inline bool dfl_feature_is_fme(void __iomem *base)
>>> +{
>>> +	u64 v = readq(base + DFH);
>>> +
>>> +	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
>>> +		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
>>> +}
>>> +
>>> +/* Function to read from DFH and check if the Feature type is port*/
>>> +static inline bool dfl_feature_is_port(void __iomem *base)
>>> +{
>>> +	u64 v = readq(base + DFH);
>>> +
>>> +	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
>>> +		 (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
>>> +}
>>> +
>>> +/* Function to read feature revision from DFH */
>>> +static inline u8 dfl_feature_revision(void __iomem *base)
>>> +{
>>> +	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
>>> +}
>>> +
>>>    #endif /* __LINUX_DFL_H */

Re: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Xu Yilun]

On Tue, Jun 22, 2021 at 05:31:57AM -0700, Tom Rix wrote:
> 
> On 6/21/21 9:56 PM, Xu Yilun wrote:
> > On Mon, Jun 21, 2021 at 06:56:28AM -0700, Tom Rix wrote:
> > > On 6/21/21 12:06 AM, Martin Hundebøll wrote:
> > > > From: Debarati Biswas <debaratix.biswas@intel.com>
> > > > 
> > > > Device Feature List (DFL) drivers may be defined in subdirectories other
> > > > than drivers/fpga, and each DFL driver should have access to the Device
> > > > Feature Header (DFH) register, which contains revision and type
> > > > information. This change moves the macros specific to the DFH register
> > > > from drivers/fpga/dfl.h to include/linux/dfl.h.
> > > > 
> > > > Signed-off-by: Debarati Biswas <debaratix.biswas@intel.com>
> > > > Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> > > > Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
> > > > ---
> > > >    drivers/fpga/dfl.h  | 48 +----------------------------------------
> > > >    include/linux/dfl.h | 52 +++++++++++++++++++++++++++++++++++++++++++++
> > > >    2 files changed, 53 insertions(+), 47 deletions(-)
> > > > 
> > > > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
> > > > index 2b82c96ba56c..6ed0353e9a99 100644
> > > > --- a/drivers/fpga/dfl.h
> > > > +++ b/drivers/fpga/dfl.h
> > > > @@ -17,6 +17,7 @@
> > > >    #include <linux/bitfield.h>
> > > bitfield.h was added to linux/dfl.h
> > > 
> > > Likely both aren't needed, try removing this one.
> > The DFL register definitions are in dfl.h, and Source files which include
> > dfl.h are likely to use bitfield ops for DFL register access, so could we
> > keep it here?
> 
> Not sure which dfl.h you mean.
> 
> It looks like you are copying header includes in both files and including
> the one file in the other.
> 
> So it is likely you are including headers you don't need.

Maybe I misunderstand, I thought you suggest remove the headers in both
drivers/fpga/dfl.h and linux/dfl.h.

Move the bitfield.h from drivers/fpga/dfl.h to linux/dfl.h is good to
me.

Thanks,
Yilun

> 
> Tom
> 
> > 
> > Thanks,
> > Yilun
> > 
> > > Tom
> > > 
> > > >    #include <linux/cdev.h>
> > > >    #include <linux/delay.h>
> > > > +#include <linux/dfl.h>
> > > >    #include <linux/eventfd.h>
> > > >    #include <linux/fs.h>
> > > >    #include <linux/interrupt.h>
> > > > @@ -53,32 +54,6 @@
> > > >    #define PORT_FEATURE_ID_UINT		0x12
> > > >    #define PORT_FEATURE_ID_STP		0x13
> > > > -/*
> > > > - * Device Feature Header Register Set
> > > > - *
> > > > - * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
> > > > - * For AFUs, they have DFH + GUID as common header registers.
> > > > - * For private features, they only have DFH register as common header.
> > > > - */
> > > > -#define DFH			0x0
> > > > -#define GUID_L			0x8
> > > > -#define GUID_H			0x10
> > > > -#define NEXT_AFU		0x18
> > > > -
> > > > -#define DFH_SIZE		0x8
> > > > -
> > > > -/* Device Feature Header Register Bitfield */
> > > > -#define DFH_ID			GENMASK_ULL(11, 0)	/* Feature ID */
> > > > -#define DFH_ID_FIU_FME		0
> > > > -#define DFH_ID_FIU_PORT		1
> > > > -#define DFH_REVISION		GENMASK_ULL(15, 12)	/* Feature revision */
> > > > -#define DFH_NEXT_HDR_OFST	GENMASK_ULL(39, 16)	/* Offset to next DFH */
> > > > -#define DFH_EOL			BIT_ULL(40)		/* End of list */
> > > > -#define DFH_TYPE		GENMASK_ULL(63, 60)	/* Feature type */
> > > > -#define DFH_TYPE_AFU		1
> > > > -#define DFH_TYPE_PRIVATE	3
> > > > -#define DFH_TYPE_FIU		4
> > > > -
> > > >    /* Next AFU Register Bitfield */
> > > >    #define NEXT_AFU_NEXT_DFH_OFST	GENMASK_ULL(23, 0)	/* Offset to next AFU */
> > > > @@ -403,27 +378,6 @@ struct device *dfl_fpga_pdata_to_parent(struct dfl_feature_platform_data *pdata)
> > > >    	return pdata->dev->dev.parent->parent;
> > > >    }
> > > > -static inline bool dfl_feature_is_fme(void __iomem *base)
> > > > -{
> > > > -	u64 v = readq(base + DFH);
> > > > -
> > > > -	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > > > -		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
> > > > -}
> > > > -
> > > > -static inline bool dfl_feature_is_port(void __iomem *base)
> > > > -{
> > > > -	u64 v = readq(base + DFH);
> > > > -
> > > > -	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > > > -		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
> > > > -}
> > > > -
> > > > -static inline u8 dfl_feature_revision(void __iomem *base)
> > > > -{
> > > > -	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
> > > > -}
> > > > -
> > > >    /**
> > > >     * struct dfl_fpga_enum_info - DFL FPGA enumeration information
> > > >     *
> > > > diff --git a/include/linux/dfl.h b/include/linux/dfl.h
> > > > index 6cc10982351a..1cd86b2e7cb1 100644
> > > > --- a/include/linux/dfl.h
> > > > +++ b/include/linux/dfl.h
> > > > @@ -8,7 +8,9 @@
> > > >    #ifndef __LINUX_DFL_H
> > > >    #define __LINUX_DFL_H
> > > > +#include <linux/bitfield.h>
> > > >    #include <linux/device.h>
> > > > +#include <linux/io.h>
> > > >    #include <linux/mod_devicetable.h>
> > > >    /**
> > > > @@ -83,4 +85,54 @@ void dfl_driver_unregister(struct dfl_driver *dfl_drv);
> > > >    	module_driver(__dfl_driver, dfl_driver_register, \
> > > >    		      dfl_driver_unregister)
> > > > +/*
> > > > + * Device Feature Header Register Set
> > > > + *
> > > > + * For FIUs, they all have DFH + GUID + NEXT_AFU as common header registers.
> > > > + * For AFUs, they have DFH + GUID as common header registers.
> > > > + * For private features, they only have DFH register as common header.
> > > > + */
> > > > +#define DFH                     0x0
> > > > +#define GUID_L                  0x8
> > > > +#define GUID_H                  0x10
> > > > +#define NEXT_AFU                0x18
> > > > +
> > > > +#define DFH_SIZE                0x8
> > > > +
> > > > +/* Device Feature Header Register Bitfield */
> > > > +#define DFH_ID                  GENMASK_ULL(11, 0)      /* Feature ID */
> > > > +#define DFH_ID_FIU_FME          0
> > > > +#define DFH_ID_FIU_PORT         1
> > > > +#define DFH_REVISION            GENMASK_ULL(15, 12)
> > > > +#define DFH_NEXT_HDR_OFST       GENMASK_ULL(39, 16)     /* Offset to next DFH */
> > > > +#define DFH_EOL                 BIT_ULL(40)             /* End of list */
> > > > +#define DFH_TYPE                GENMASK_ULL(63, 60)     /* Feature type */
> > > > +#define DFH_TYPE_AFU            1
> > > > +#define DFH_TYPE_PRIVATE        3
> > > > +#define DFH_TYPE_FIU            4
> > > > +
> > > > +/* Function to read from DFH and check if the Feature type is FME */
> > > > +static inline bool dfl_feature_is_fme(void __iomem *base)
> > > > +{
> > > > +	u64 v = readq(base + DFH);
> > > > +
> > > > +	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > > > +		(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
> > > > +}
> > > > +
> > > > +/* Function to read from DFH and check if the Feature type is port*/
> > > > +static inline bool dfl_feature_is_port(void __iomem *base)
> > > > +{
> > > > +	u64 v = readq(base + DFH);
> > > > +
> > > > +	return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
> > > > +		 (FIELD_GET(DFH_ID, v) == DFH_ID_FIU_PORT);
> > > > +}
> > > > +
> > > > +/* Function to read feature revision from DFH */
> > > > +static inline u8 dfl_feature_revision(void __iomem *base)
> > > > +{
> > > > +	return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
> > > > +}
> > > > +
> > > >    #endif /* __LINUX_DFL_H */

Re: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Martin Hundebøll]

On 21/06/2021 15.56, Tom Rix wrote:
> 
> On 6/21/21 12:06 AM, Martin Hundebøll wrote:
>> From: Debarati Biswas <debaratix.biswas@intel.com>
>>
>> Device Feature List (DFL) drivers may be defined in subdirectories other
>> than drivers/fpga, and each DFL driver should have access to the Device
>> Feature Header (DFH) register, which contains revision and type
>> information. This change moves the macros specific to the DFH register
>> from drivers/fpga/dfl.h to include/linux/dfl.h.
>>
>> Signed-off-by: Debarati Biswas <debaratix.biswas@intel.com>
>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
>> Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
>> ---
>>   drivers/fpga/dfl.h  | 48 +----------------------------------------
>>   include/linux/dfl.h | 52 +++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 53 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
>> index 2b82c96ba56c..6ed0353e9a99 100644
>> --- a/drivers/fpga/dfl.h
>> +++ b/drivers/fpga/dfl.h
>> @@ -17,6 +17,7 @@
>>   #include <linux/bitfield.h>
> 
> bitfield.h was added to linux/dfl.h
> 
> Likely both aren't needed, try removing this one.

After this patch both headers use GENMASK_ULL() and BIT_ULL(), which are both defined in linux/bits.h, so I would expect both to include that instead. A lot of the users of dfl.h uses FIELD_GET(), which is defined in linux/bitfield.h, so I would expect that to be included there instead.

I can prepare a patch to fix that up, if you like?

// Martin

Re: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[Tom Rix]

On 6/23/21 4:44 AM, Martin Hundebøll wrote:
> On 21/06/2021 15.56, Tom Rix wrote:
>>
>> On 6/21/21 12:06 AM, Martin Hundebøll wrote:
>>> From: Debarati Biswas <debaratix.biswas@intel.com>
>>>
>>> Device Feature List (DFL) drivers may be defined in subdirectories 
>>> other
>>> than drivers/fpga, and each DFL driver should have access to the Device
>>> Feature Header (DFH) register, which contains revision and type
>>> information. This change moves the macros specific to the DFH register
>>> from drivers/fpga/dfl.h to include/linux/dfl.h.
>>>
>>> Signed-off-by: Debarati Biswas <debaratix.biswas@intel.com>
>>> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
>>> Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
>>> ---
>>>   drivers/fpga/dfl.h  | 48 +----------------------------------------
>>>   include/linux/dfl.h | 52 
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 53 insertions(+), 47 deletions(-)
>>>
>>> diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h
>>> index 2b82c96ba56c..6ed0353e9a99 100644
>>> --- a/drivers/fpga/dfl.h
>>> +++ b/drivers/fpga/dfl.h
>>> @@ -17,6 +17,7 @@
>>>   #include <linux/bitfield.h>
>>
>> bitfield.h was added to linux/dfl.h
>>
>> Likely both aren't needed, try removing this one.
>
> After this patch both headers use GENMASK_ULL() and BIT_ULL(), which 
> are both defined in linux/bits.h, so I would expect both to include 
> that instead. A lot of the users of dfl.h uses FIELD_GET(), which is 
> defined in linux/bitfield.h, so I would expect that to be included 
> there instead.
>
> I can prepare a patch to fix that up, if you like?

A rule of thumb, if your commit says 'A and B' , likely it should only be A.

Remove the the linux/bitfield.h from drivers/fpga/dfl.h

Leave the linux/bitfield.h as-is in include/linux/dfl.h

If you feel include/linux/dfl.h can be optimized by the reduction of 
linux/bitfield.h to linux/bits.h, do that as a follow on patchset.

Tom

>
> // Martin
>

Re: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2977 bytes --]

Hi "Martin,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on hwmon/hwmon-next]
[also build test ERROR on spi/for-next lee-mfd/for-mfd-next linus/master v5.13-rc7 next-20210621]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Martin-Hundeb-ll/fpga-mfd-hwmon-Initial-support-for-Silicom-N5010-PAC/20210621-150923
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: mips-randconfig-r005-20210621 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project e1adf90826a57b674eee79b071fb46c1f5683cd0)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # https://github.com/0day-ci/linux/commit/3cd517ceee3e0bb24e44dd409f3ba2652c2ed297
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Martin-Hundeb-ll/fpga-mfd-hwmon-Initial-support-for-Silicom-N5010-PAC/20210621-150923
        git checkout 3cd517ceee3e0bb24e44dd409f3ba2652c2ed297
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/fpga/dfl.c:13:
>> include/linux/dfl.h:117:10: error: implicit declaration of function 'readq' [-Werror,-Wimplicit-function-declaration]
           u64 v = readq(base + DFH);
                   ^
   include/linux/dfl.h:126:10: error: implicit declaration of function 'readq' [-Werror,-Wimplicit-function-declaration]
           u64 v = readq(base + DFH);
                   ^
   include/linux/dfl.h:135:37: error: implicit declaration of function 'readq' [-Werror,-Wimplicit-function-declaration]
           return (u8)FIELD_GET(DFH_REVISION, readq(base + DFH));
                                              ^
   include/linux/dfl.h:135:37: error: implicit declaration of function 'readq' [-Werror,-Wimplicit-function-declaration]
   4 errors generated.


vim +/readq +117 include/linux/dfl.h

   113	
   114	/* Function to read from DFH and check if the Feature type is FME */
   115	static inline bool dfl_feature_is_fme(void __iomem *base)
   116	{
 > 117		u64 v = readq(base + DFH);
   118	
   119		return (FIELD_GET(DFH_TYPE, v) == DFH_TYPE_FIU) &&
   120			(FIELD_GET(DFH_ID, v) == DFH_ID_FIU_FME);
   121	}
   122	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23249 bytes --]

[PATCH 3/4] spi: spi-altera-dfl: support n5010 feature revision

[Martin Hundebøll]

The Max10 BMC on the Silicom n5010 PAC is slightly different than the
existing BMC's, so use a dedicated feature revision detect it.

Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
---
 drivers/spi/spi-altera-dfl.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-altera-dfl.c b/drivers/spi/spi-altera-dfl.c
index 3e32e4fe5895..d4908db39d87 100644
--- a/drivers/spi/spi-altera-dfl.c
+++ b/drivers/spi/spi-altera-dfl.c
@@ -111,6 +111,13 @@ static struct spi_board_info m10_bmc_info = {
 	.chip_select = 0,
 };
 
+static struct spi_board_info m10_n5010_bmc_info = {
+	.modalias = "m10-n5010",
+	.max_speed_hz = 12500000,
+	.bus_num = 0,
+	.chip_select = 0,
+};
+
 static void config_spi_master(void __iomem *base, struct spi_master *master)
 {
 	u64 v;
@@ -130,6 +137,7 @@ static void config_spi_master(void __iomem *base, struct spi_master *master)
 
 static int dfl_spi_altera_probe(struct dfl_device *dfl_dev)
 {
+	struct spi_board_info *board_info = &m10_bmc_info;
 	struct device *dev = &dfl_dev->dev;
 	struct spi_master *master;
 	struct altera_spi *hw;
@@ -172,9 +180,12 @@ static int dfl_spi_altera_probe(struct dfl_device *dfl_dev)
 		goto exit;
 	}
 
-	if (!spi_new_device(master,  &m10_bmc_info)) {
+	if (dfl_feature_revision(base) == FME_FEATURE_REV_MAX10_SPI_N5010)
+		board_info = &m10_n5010_bmc_info;
+
+	if (!spi_new_device(master, board_info)) {
 		dev_err(dev, "%s failed to create SPI device: %s\n",
-			__func__, m10_bmc_info.modalias);
+			__func__, board_info->modalias);
 	}
 
 	return 0;
-- 
2.31.0

[PATCH 4/4] hwmon: intel-m10-bmc: add sensor support for Silicom N5010 card

[Martin Hundebøll]

The Silicom N5010 PAC is similar to Intel N3000 and D5005. Enable
monitoring of its sensors like it is done for the two Intel cards.

Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
---
 drivers/hwmon/intel-m10-bmc-hwmon.c | 116 ++++++++++++++++++++++++++++
 drivers/mfd/intel-m10-bmc.c         |  12 ++-
 2 files changed, 127 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/intel-m10-bmc-hwmon.c b/drivers/hwmon/intel-m10-bmc-hwmon.c
index bd7ed2ed3a1e..7a08e4c44a4b 100644
--- a/drivers/hwmon/intel-m10-bmc-hwmon.c
+++ b/drivers/hwmon/intel-m10-bmc-hwmon.c
@@ -228,6 +228,118 @@ static const struct m10bmc_hwmon_board_data d5005bmc_hwmon_bdata = {
 	.hinfo = d5005bmc_hinfo,
 };
 
+static const struct m10bmc_sdata n5010bmc_temp_tbl[] = {
+	{ 0x100, 0x0, 0x104, 0x0, 0x0, 1000, "Board Local Temperature" },
+	{ 0x108, 0x0, 0x10c, 0x0, 0x0, 1000, "FPGA 1 Temperature" },
+	{ 0x110, 0x0, 0x114, 0x0, 0x0, 1000, "FPGA 2 Temperature" },
+	{ 0x118, 0x0, 0x0, 0x0, 0x0, 1000, "Card Top Temperature" },
+	{ 0x11c, 0x0, 0x0, 0x0, 0x0, 1000, "Card Bottom Temperature" },
+	{ 0x128, 0x0, 0x0, 0x0, 0x0, 1000, "FPGA 1.2V Temperature" },
+	{ 0x134, 0x0, 0x0, 0x0, 0x0, 1000, "FPGA 5V Temperature" },
+	{ 0x140, 0x0, 0x0, 0x0, 0x0, 1000, "FPGA 0.9V Temperature" },
+	{ 0x14c, 0x0, 0x0, 0x0, 0x0, 1000, "FPGA 0.85V Temperature" },
+	{ 0x158, 0x0, 0x0, 0x0, 0x0, 1000, "AUX 12V Temperature" },
+	{ 0x164, 0x0, 0x0, 0x0, 0x0, 1000, "Backplane 12V Temperature" },
+	{ 0x1a8, 0x0, 0x0, 0x0, 0x0, 1000, "QSFP28-1 Temperature" },
+	{ 0x1ac, 0x0, 0x0, 0x0, 0x0, 1000, "QSFP28-2 Temperature" },
+	{ 0x1b0, 0x0, 0x0, 0x0, 0x0, 1000, "QSFP28-3 Temperature" },
+	{ 0x1b4, 0x0, 0x0, 0x0, 0x0, 1000, "QSFP28-4 Temperature" },
+	{ 0x1b8, 0x0, 0x0, 0x0, 0x0, 1000, "CVL1 Internal Temperature" },
+	{ 0x1bc, 0x0, 0x0, 0x0, 0x0, 1000, "CVL2 Internal Temperature" },
+};
+
+static const struct m10bmc_sdata n5010bmc_in_tbl[] = {
+	{ 0x120, 0x0, 0x0, 0x0, 0x0, 1, "FPGA 1.2V Voltage" },
+	{ 0x12c, 0x0, 0x0, 0x0, 0x0, 1, "FPGA 5V Voltage" },
+	{ 0x138, 0x0, 0x0, 0x0, 0x0, 1, "FPGA 0.9V Voltage" },
+	{ 0x144, 0x0, 0x0, 0x0, 0x0, 1, "FPGA 0.85V Voltage" },
+	{ 0x150, 0x0, 0x0, 0x0, 0x0, 1, "AUX 12V Voltage" },
+	{ 0x15c, 0x0, 0x0, 0x0, 0x0, 1, "Backplane 12V Voltage" },
+	{ 0x16c, 0x0, 0x0, 0x0, 0x0, 1, "DDR4 1.2V Voltage" },
+	{ 0x17c, 0x0, 0x0, 0x0, 0x0, 1, "FPGA 1.8V Voltage" },
+	{ 0x184, 0x0, 0x0, 0x0, 0x0, 1, "QDR 1.3V Voltage" },
+	{ 0x18c, 0x0, 0x0, 0x0, 0x0, 1, "CVL1 0.8V Voltage" },
+	{ 0x194, 0x0, 0x0, 0x0, 0x0, 1, "CVL1 1.05V Voltage" },
+	{ 0x19c, 0x0, 0x0, 0x0, 0x0, 1, "CVL2 1.05V Voltage" },
+	{ 0x1a4, 0x0, 0x0, 0x0, 0x0, 1, "CVL2 0.8V Voltage" },
+};
+
+static const struct m10bmc_sdata n5010bmc_curr_tbl[] = {
+	{ 0x124, 0x0, 0x0, 0x0, 0x0, 1, "FPGA 1.2V Current" },
+	{ 0x130, 0x0, 0x0, 0x0, 0x0, 1, "FPGA 5V Current" },
+	{ 0x13c, 0x0, 0x0, 0x0, 0x0, 1, "FPGA 0.9V Current" },
+	{ 0x148, 0x0, 0x0, 0x0, 0x0, 1, "FPGA 0.85V Current" },
+	{ 0x154, 0x0, 0x0, 0x0, 0x0, 1, "AUX 12V Current" },
+	{ 0x160, 0x0, 0x0, 0x0, 0x0, 1, "Backplane 12V Current" },
+	{ 0x168, 0x0, 0x0, 0x0, 0x0, 1, "DDR4 1.2V Current" },
+	{ 0x178, 0x0, 0x0, 0x0, 0x0, 1, "FPGA 1.8V Current" },
+	{ 0x180, 0x0, 0x0, 0x0, 0x0, 1, "QDR 1.3V Current" },
+	{ 0x188, 0x0, 0x0, 0x0, 0x0, 1, "CVL1 0.8V Current" },
+	{ 0x190, 0x0, 0x0, 0x0, 0x0, 1, "CVL1 1.05V Current" },
+	{ 0x198, 0x0, 0x0, 0x0, 0x0, 1, "CVL2 1.05V Current" },
+	{ 0x1a0, 0x0, 0x0, 0x0, 0x0, 1, "CVL2 0.8V Current" },
+};
+
+static const struct hwmon_channel_info *n5010bmc_hinfo[] = {
+	HWMON_CHANNEL_INFO(temp,
+			   HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_CRIT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL,
+			   HWMON_T_INPUT | HWMON_T_LABEL),
+	HWMON_CHANNEL_INFO(in,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL,
+			   HWMON_I_INPUT | HWMON_I_LABEL),
+	HWMON_CHANNEL_INFO(curr,
+			   HWMON_C_INPUT | HWMON_C_LABEL,
+			   HWMON_C_INPUT | HWMON_C_LABEL,
+			   HWMON_C_INPUT | HWMON_C_LABEL,
+			   HWMON_C_INPUT | HWMON_C_LABEL,
+			   HWMON_C_INPUT | HWMON_C_LABEL,
+			   HWMON_C_INPUT | HWMON_C_LABEL,
+			   HWMON_C_INPUT | HWMON_C_LABEL,
+			   HWMON_C_INPUT | HWMON_C_LABEL,
+			   HWMON_C_INPUT | HWMON_C_LABEL,
+			   HWMON_C_INPUT | HWMON_C_LABEL,
+			   HWMON_C_INPUT | HWMON_C_LABEL,
+			   HWMON_C_INPUT | HWMON_C_LABEL,
+			   HWMON_C_INPUT | HWMON_C_LABEL),
+	NULL
+};
+
+static const struct m10bmc_hwmon_board_data n5010bmc_hwmon_bdata = {
+	.tables = {
+		[hwmon_temp] = n5010bmc_temp_tbl,
+		[hwmon_in] = n5010bmc_in_tbl,
+		[hwmon_curr] = n5010bmc_curr_tbl,
+	},
+
+	.hinfo = n5010bmc_hinfo,
+};
+
 static umode_t
 m10bmc_hwmon_is_visible(const void *data, enum hwmon_sensor_types type,
 			u32 attr, int channel)
@@ -438,6 +550,10 @@ static const struct platform_device_id intel_m10bmc_hwmon_ids[] = {
 		.name = "d5005bmc-hwmon",
 		.driver_data = (unsigned long)&d5005bmc_hwmon_bdata,
 	},
+	{
+		.name = "n5010bmc-hwmon",
+		.driver_data = (unsigned long)&n5010bmc_hwmon_bdata,
+	},
 	{ }
 };
 
diff --git a/drivers/mfd/intel-m10-bmc.c b/drivers/mfd/intel-m10-bmc.c
index 1a9bfb7f48cd..8db3bcf5fccc 100644
--- a/drivers/mfd/intel-m10-bmc.c
+++ b/drivers/mfd/intel-m10-bmc.c
@@ -15,7 +15,8 @@
 
 enum m10bmc_type {
 	M10_N3000,
-	M10_D5005
+	M10_D5005,
+	M10_N5010,
 };
 
 static struct mfd_cell m10bmc_d5005_subdevs[] = {
@@ -28,6 +29,10 @@ static struct mfd_cell m10bmc_pacn3000_subdevs[] = {
 	{ .name = "n3000bmc-secure" },
 };
 
+static struct mfd_cell m10bmc_n5010_subdevs[] = {
+	{ .name = "n5010bmc-hwmon" },
+};
+
 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),
@@ -192,6 +197,10 @@ static int intel_m10_bmc_spi_probe(struct spi_device *spi)
 		cells = m10bmc_d5005_subdevs;
 		n_cell = ARRAY_SIZE(m10bmc_d5005_subdevs);
 		break;
+	case M10_N5010:
+		cells = m10bmc_n5010_subdevs;
+		n_cell = ARRAY_SIZE(m10bmc_n5010_subdevs);
+		break;
 	default:
 		return -ENODEV;
 	}
@@ -207,6 +216,7 @@ static int intel_m10_bmc_spi_probe(struct spi_device *spi)
 static const struct spi_device_id m10bmc_spi_id[] = {
 	{ "m10-n3000", M10_N3000 },
 	{ "m10-d5005", M10_D5005 },
+	{ "m10-n5010", M10_N5010 },
 	{ }
 };
 MODULE_DEVICE_TABLE(spi, m10bmc_spi_id);
-- 
2.31.0

Re: [PATCH 4/4] hwmon: intel-m10-bmc: add sensor support for Silicom N5010 card

[Lee Jones]

On Mon, 21 Jun 2021, Martin Hundebøll wrote:

> The Silicom N5010 PAC is similar to Intel N3000 and D5005. Enable
> monitoring of its sensors like it is done for the two Intel cards.
> 
> Signed-off-by: Martin Hundebøll <mhu@silicom.dk>
> ---
>  drivers/hwmon/intel-m10-bmc-hwmon.c | 116 ++++++++++++++++++++++++++++
>  drivers/mfd/intel-m10-bmc.c         |  12 ++-

Please split out the MFD part into a different patch.

>  2 files changed, 127 insertions(+), 1 deletion(-)

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 0/4] fpga/mfd/hwmon: Initial support for Silicom N5010 PAC

[Xu Yilun]

On Mon, Jun 21, 2021 at 09:06:17AM +0200, Martin Hundebøll wrote:
> From: Martin Hundebøll <mhu@geanix.com>
> 
> This is an initial set of patches for the Silciom N5010 programmable
> accelerated card adding support for reading out sensors.

Seems the card is a variant of d5005, just changes the layout of the
sensors in BMC. It may not worth a dedicated PCI DID, and pass
down the info all the way from
  pcie -> dfl -> spi-altera -> m10bmc -> m10bmc-hwmon

Is it possible we just have some version check in m10bmc?

Thank,
Yilun

> 
> I'm not really sure if these should be taken through each of the
> affected trees separately, or just by fpga collectively?
> 
> Based on current master.
> 
> // Martin
> 
> Debarati Biswas (1):
>   fpga: dfl: Move DFH header register macros to linux/dfl.h
> 
> Martin Hundebøll (3):
>   fpga: dfl: pci: add device IDs for Silicom N501x PAC cards
>   spi: spi-altera-dfl: support n5010 feature revision
>   hwmon: intel-m10-bmc: add sensor support for Silicom N5010 card
> 
>  drivers/fpga/dfl-pci.c              |   5 ++
>  drivers/fpga/dfl.h                  |  48 +-----------
>  drivers/hwmon/intel-m10-bmc-hwmon.c | 116 ++++++++++++++++++++++++++++
>  drivers/mfd/intel-m10-bmc.c         |  12 ++-
>  drivers/spi/spi-altera-dfl.c        |  15 +++-
>  include/linux/dfl.h                 |  52 +++++++++++++
>  6 files changed, 198 insertions(+), 50 deletions(-)
> 
> -- 
> 2.31.0

Re: [PATCH 0/4] fpga/mfd/hwmon: Initial support for Silicom N5010 PAC

[Martin Hundebøll]

Hi Yilun,

On 21/06/2021 10.38, Xu Yilun wrote:
> On Mon, Jun 21, 2021 at 09:06:17AM +0200, Martin Hundebøll wrote:
>> From: Martin Hundebøll <mhu@geanix.com>
>>
>> This is an initial set of patches for the Silciom N5010 programmable
>> accelerated card adding support for reading out sensors.
> 
> Seems the card is a variant of d5005, just changes the layout of the
> sensors in BMC. It may not worth a dedicated PCI DID, and pass
> down the info all the way from
>    pcie -> dfl -> spi-altera -> m10bmc -> m10bmc-hwmon
> 
> Is it possible we just have some version check in m10bmc?

I think not.

The n501x is a Silicom card, and d5005 is from Intel. The Max10 based BMC
is quite similar, which is why we use a feature revision to differentiate,
but the main FPGA register layout is a different story.

// Martin

>>
>> I'm not really sure if these should be taken through each of the
>> affected trees separately, or just by fpga collectively?
>>
>> Based on current master.
>>
>> // Martin
>>
>> Debarati Biswas (1):
>>    fpga: dfl: Move DFH header register macros to linux/dfl.h
>>
>> Martin Hundebøll (3):
>>    fpga: dfl: pci: add device IDs for Silicom N501x PAC cards
>>    spi: spi-altera-dfl: support n5010 feature revision
>>    hwmon: intel-m10-bmc: add sensor support for Silicom N5010 card
>>
>>   drivers/fpga/dfl-pci.c              |   5 ++
>>   drivers/fpga/dfl.h                  |  48 +-----------
>>   drivers/hwmon/intel-m10-bmc-hwmon.c | 116 ++++++++++++++++++++++++++++
>>   drivers/mfd/intel-m10-bmc.c         |  12 ++-
>>   drivers/spi/spi-altera-dfl.c        |  15 +++-
>>   include/linux/dfl.h                 |  52 +++++++++++++
>>   6 files changed, 198 insertions(+), 50 deletions(-)
>>
>> -- 
>> 2.31.0