linux-fpga.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Xu Yilun <yilun.xu@intel.com>
To: Tom Rix <trix@redhat.com>
Cc: "Martin Hundebøll" <mhu@silicom.dk>, "Wu Hao" <hao.wu@intel.com>,
	"Moritz Fischer" <mdf@kernel.org>,
	"Jean Delvare" <jdelvare@suse.com>,
	"Guenter Roeck" <linux@roeck-us.net>,
	"Lee Jones" <lee.jones@linaro.org>,
	"Mark Brown" <broonie@kernel.org>,
	"Martin Hundebøll" <mhu@geanix.com>,
	linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-hwmon@vger.kernel.org, linux-spi@vger.kernel.org,
	"Debarati Biswas" <debaratix.biswas@intel.com>,
	"Russ Weight" <russell.h.weight@intel.com>
Subject: Re: [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h
Date: Wed, 23 Jun 2021 14:37:18 +0800	[thread overview]
Message-ID: <20210623063718.GA35761@yilunxu-OptiPlex-7050> (raw)
In-Reply-To: <d74f6e17-0fe2-bd18-de53-18181424db82@redhat.com>

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 */

  reply	other threads:[~2021-06-23  6:42 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-21  7:06 [PATCH 0/4] fpga/mfd/hwmon: Initial support for Silicom N5010 PAC Martin Hundebøll
2021-06-21  7:06 ` [PATCH 1/4] fpga: dfl: pci: add device IDs for Silicom N501x PAC cards Martin Hundebøll
2021-06-21  9:57   ` Wu, Hao
2021-06-21  7:06 ` [PATCH 2/4] fpga: dfl: Move DFH header register macros to linux/dfl.h Martin Hundebøll
2021-06-21 10:19   ` Wu, Hao
2021-06-22  5:22     ` Xu Yilun
2021-06-22  7:39       ` Wu, Hao
2021-06-23 11:56         ` Martin Hundebøll
2021-06-24  3:01           ` Xu Yilun
2021-06-24  4:45             ` Wu, Hao
2021-06-21 13:56   ` Tom Rix
2021-06-22  4:56     ` Xu Yilun
2021-06-22 12:31       ` Tom Rix
2021-06-23  6:37         ` Xu Yilun [this message]
2021-06-23 11:44     ` Martin Hundebøll
2021-06-23 13:38       ` Tom Rix
2021-06-21 19:33   ` kernel test robot
2021-06-21  7:06 ` [PATCH 3/4] spi: spi-altera-dfl: support n5010 feature revision Martin Hundebøll
2021-06-21  7:06 ` [PATCH 4/4] hwmon: intel-m10-bmc: add sensor support for Silicom N5010 card Martin Hundebøll
2021-06-21  8:55   ` Lee Jones
2021-06-21  8:38 ` [PATCH 0/4] fpga/mfd/hwmon: Initial support for Silicom N5010 PAC Xu Yilun
2021-06-25  7:11   ` Martin Hundebøll

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210623063718.GA35761@yilunxu-OptiPlex-7050 \
    --to=yilun.xu@intel.com \
    --cc=broonie@kernel.org \
    --cc=debaratix.biswas@intel.com \
    --cc=hao.wu@intel.com \
    --cc=jdelvare@suse.com \
    --cc=lee.jones@linaro.org \
    --cc=linux-fpga@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=mdf@kernel.org \
    --cc=mhu@geanix.com \
    --cc=mhu@silicom.dk \
    --cc=russell.h.weight@intel.com \
    --cc=trix@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).