diff mbox

[03/10] fpga: allow to compile-test Altera FPGA bridge drivers

Message ID 20170605190741.10508-4-atull@kernel.org (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Alan Tull June 5, 2017, 7:07 p.m. UTC
From: Tobias Klauser <tklauser@distanz.ch>

Add COMPILE_TEST to the Kconfig entry for the Altera SoCFPGA FPGA
Bridge. The Altera FPGA Freeze Bridge can also be used on Altera PEIe
FPGAs, so the driver shouldn't depend on ARCH_SOCFPGA in the first place.
This allows test-compiling the drivers on other architectures to catch
compiler errors/warnings, e.g. due to API/header changes earlier on.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Signed-off-by: Alan Tull <atull@kernel.org>
---
 drivers/fpga/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

kernel test robot June 6, 2017, 1:45 p.m. UTC | #1
Hi Tobias,

[auto build test WARNING on linus/master]
[also build test WARNING on v4.12-rc4 next-20170606]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Alan-Tull/patches-for-fpga/20170606-072535
config: m32r-allyesconfig (attached as .config)
compiler: m32r-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=m32r 

All warnings (new ones prefixed by >>):

   In file included from include/linux/printk.h:329:0,
                    from include/linux/kernel.h:13,
                    from include/linux/delay.h:21,
                    from drivers/fpga/altera-freeze-bridge.c:18:
   drivers/fpga/altera-freeze-bridge.c: In function 'altera_freeze_br_do_freeze':
>> drivers/fpga/altera-freeze-bridge.c:108:15: warning: format '%d' expects argument of type 'int', but argument 6 has type 'long unsigned int' [-Wformat=]
     dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
                  ^
   include/linux/dynamic_debug.h:134:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
>> drivers/fpga/altera-freeze-bridge.c:108:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
     ^~~~~~~
   drivers/fpga/altera-freeze-bridge.c: In function 'altera_freeze_br_do_unfreeze':
   drivers/fpga/altera-freeze-bridge.c:145:15: warning: format '%d' expects argument of type 'int', but argument 6 has type 'long unsigned int' [-Wformat=]
     dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
                  ^
   include/linux/dynamic_debug.h:134:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/fpga/altera-freeze-bridge.c:145:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
     ^~~~~~~
   drivers/fpga/altera-freeze-bridge.c:163:15: warning: format '%d' expects argument of type 'int', but argument 6 has type 'long unsigned int' [-Wformat=]
     dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
                  ^
   include/linux/dynamic_debug.h:134:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/fpga/altera-freeze-bridge.c:163:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
     ^~~~~~~

vim +108 drivers/fpga/altera-freeze-bridge.c

ca24a648 Alan Tull       2016-11-01   12   * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
ca24a648 Alan Tull       2016-11-01   13   * more details.
ca24a648 Alan Tull       2016-11-01   14   *
ca24a648 Alan Tull       2016-11-01   15   * You should have received a copy of the GNU General Public License along with
ca24a648 Alan Tull       2016-11-01   16   * this program.  If not, see <http://www.gnu.org/licenses/>.
ca24a648 Alan Tull       2016-11-01   17   */
ca24a648 Alan Tull       2016-11-01  @18  #include <linux/delay.h>
ca24a648 Alan Tull       2016-11-01   19  #include <linux/io.h>
ca24a648 Alan Tull       2016-11-01   20  #include <linux/kernel.h>
ca24a648 Alan Tull       2016-11-01   21  #include <linux/of_device.h>
ca24a648 Alan Tull       2016-11-01   22  #include <linux/module.h>
ca24a648 Alan Tull       2016-11-01   23  #include <linux/fpga/fpga-bridge.h>
ca24a648 Alan Tull       2016-11-01   24  
ca24a648 Alan Tull       2016-11-01   25  #define FREEZE_CSR_STATUS_OFFSET		0
ca24a648 Alan Tull       2016-11-01   26  #define FREEZE_CSR_CTRL_OFFSET			4
ca24a648 Alan Tull       2016-11-01   27  #define FREEZE_CSR_ILLEGAL_REQ_OFFSET		8
ca24a648 Alan Tull       2016-11-01   28  #define FREEZE_CSR_REG_VERSION			12
ca24a648 Alan Tull       2016-11-01   29  
ca24a648 Alan Tull       2016-11-01   30  #define FREEZE_CSR_SUPPORTED_VERSION		2
dd17cc7b Matthew Gerlach 2017-04-24   31  #define FREEZE_CSR_OFFICIAL_VERSION		0xad000003
ca24a648 Alan Tull       2016-11-01   32  
ca24a648 Alan Tull       2016-11-01   33  #define FREEZE_CSR_STATUS_FREEZE_REQ_DONE	BIT(0)
ca24a648 Alan Tull       2016-11-01   34  #define FREEZE_CSR_STATUS_UNFREEZE_REQ_DONE	BIT(1)
ca24a648 Alan Tull       2016-11-01   35  
ca24a648 Alan Tull       2016-11-01   36  #define FREEZE_CSR_CTRL_FREEZE_REQ		BIT(0)
ca24a648 Alan Tull       2016-11-01   37  #define FREEZE_CSR_CTRL_RESET_REQ		BIT(1)
ca24a648 Alan Tull       2016-11-01   38  #define FREEZE_CSR_CTRL_UNFREEZE_REQ		BIT(2)
ca24a648 Alan Tull       2016-11-01   39  
ca24a648 Alan Tull       2016-11-01   40  #define FREEZE_BRIDGE_NAME			"freeze"
ca24a648 Alan Tull       2016-11-01   41  
ca24a648 Alan Tull       2016-11-01   42  struct altera_freeze_br_data {
ca24a648 Alan Tull       2016-11-01   43  	struct device *dev;
ca24a648 Alan Tull       2016-11-01   44  	void __iomem *base_addr;
ca24a648 Alan Tull       2016-11-01   45  	bool enable;
ca24a648 Alan Tull       2016-11-01   46  };
ca24a648 Alan Tull       2016-11-01   47  
ca24a648 Alan Tull       2016-11-01   48  /*
ca24a648 Alan Tull       2016-11-01   49   * Poll status until status bit is set or we have a timeout.
ca24a648 Alan Tull       2016-11-01   50   */
ca24a648 Alan Tull       2016-11-01   51  static int altera_freeze_br_req_ack(struct altera_freeze_br_data *priv,
ca24a648 Alan Tull       2016-11-01   52  				    u32 timeout, u32 req_ack)
ca24a648 Alan Tull       2016-11-01   53  {
ca24a648 Alan Tull       2016-11-01   54  	struct device *dev = priv->dev;
ca24a648 Alan Tull       2016-11-01   55  	void __iomem *csr_illegal_req_addr = priv->base_addr +
ca24a648 Alan Tull       2016-11-01   56  					     FREEZE_CSR_ILLEGAL_REQ_OFFSET;
ca24a648 Alan Tull       2016-11-01   57  	u32 status, illegal, ctrl;
ca24a648 Alan Tull       2016-11-01   58  	int ret = -ETIMEDOUT;
ca24a648 Alan Tull       2016-11-01   59  
ca24a648 Alan Tull       2016-11-01   60  	do {
ca24a648 Alan Tull       2016-11-01   61  		illegal = readl(csr_illegal_req_addr);
ca24a648 Alan Tull       2016-11-01   62  		if (illegal) {
ca24a648 Alan Tull       2016-11-01   63  			dev_err(dev, "illegal request detected 0x%x", illegal);
ca24a648 Alan Tull       2016-11-01   64  
ca24a648 Alan Tull       2016-11-01   65  			writel(1, csr_illegal_req_addr);
ca24a648 Alan Tull       2016-11-01   66  
ca24a648 Alan Tull       2016-11-01   67  			illegal = readl(csr_illegal_req_addr);
ca24a648 Alan Tull       2016-11-01   68  			if (illegal)
ca24a648 Alan Tull       2016-11-01   69  				dev_err(dev, "illegal request not cleared 0x%x",
ca24a648 Alan Tull       2016-11-01   70  					illegal);
ca24a648 Alan Tull       2016-11-01   71  
ca24a648 Alan Tull       2016-11-01   72  			ret = -EINVAL;
ca24a648 Alan Tull       2016-11-01   73  			break;
ca24a648 Alan Tull       2016-11-01   74  		}
ca24a648 Alan Tull       2016-11-01   75  
ca24a648 Alan Tull       2016-11-01   76  		status = readl(priv->base_addr + FREEZE_CSR_STATUS_OFFSET);
ca24a648 Alan Tull       2016-11-01   77  		dev_dbg(dev, "%s %x %x\n", __func__, status, req_ack);
ca24a648 Alan Tull       2016-11-01   78  		status &= req_ack;
ca24a648 Alan Tull       2016-11-01   79  		if (status) {
ca24a648 Alan Tull       2016-11-01   80  			ctrl = readl(priv->base_addr + FREEZE_CSR_CTRL_OFFSET);
ca24a648 Alan Tull       2016-11-01   81  			dev_dbg(dev, "%s request %x acknowledged %x %x\n",
ca24a648 Alan Tull       2016-11-01   82  				__func__, req_ack, status, ctrl);
ca24a648 Alan Tull       2016-11-01   83  			ret = 0;
ca24a648 Alan Tull       2016-11-01   84  			break;
ca24a648 Alan Tull       2016-11-01   85  		}
ca24a648 Alan Tull       2016-11-01   86  
ca24a648 Alan Tull       2016-11-01   87  		udelay(1);
ca24a648 Alan Tull       2016-11-01   88  	} while (timeout--);
ca24a648 Alan Tull       2016-11-01   89  
ca24a648 Alan Tull       2016-11-01   90  	if (ret == -ETIMEDOUT)
ca24a648 Alan Tull       2016-11-01   91  		dev_err(dev, "%s timeout waiting for 0x%x\n",
ca24a648 Alan Tull       2016-11-01   92  			__func__, req_ack);
ca24a648 Alan Tull       2016-11-01   93  
ca24a648 Alan Tull       2016-11-01   94  	return ret;
ca24a648 Alan Tull       2016-11-01   95  }
ca24a648 Alan Tull       2016-11-01   96  
ca24a648 Alan Tull       2016-11-01   97  static int altera_freeze_br_do_freeze(struct altera_freeze_br_data *priv,
ca24a648 Alan Tull       2016-11-01   98  				      u32 timeout)
ca24a648 Alan Tull       2016-11-01   99  {
ca24a648 Alan Tull       2016-11-01  100  	struct device *dev = priv->dev;
ca24a648 Alan Tull       2016-11-01  101  	void __iomem *csr_ctrl_addr = priv->base_addr +
ca24a648 Alan Tull       2016-11-01  102  				      FREEZE_CSR_CTRL_OFFSET;
ca24a648 Alan Tull       2016-11-01  103  	u32 status;
ca24a648 Alan Tull       2016-11-01  104  	int ret;
ca24a648 Alan Tull       2016-11-01  105  
ca24a648 Alan Tull       2016-11-01  106  	status = readl(priv->base_addr + FREEZE_CSR_STATUS_OFFSET);
ca24a648 Alan Tull       2016-11-01  107  
ca24a648 Alan Tull       2016-11-01 @108  	dev_dbg(dev, "%s %d %d\n", __func__, status, readl(csr_ctrl_addr));
ca24a648 Alan Tull       2016-11-01  109  
ca24a648 Alan Tull       2016-11-01  110  	if (status & FREEZE_CSR_STATUS_FREEZE_REQ_DONE) {
ca24a648 Alan Tull       2016-11-01  111  		dev_dbg(dev, "%s bridge already disabled %d\n",

:::::: The code at line 108 was first introduced by commit
:::::: ca24a648f535a02b4163ca4f4d2e51869f155a3a fpga: add altera freeze bridge support

:::::: TO: Alan Tull <atull@opensource.altera.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Greg KH June 9, 2017, 9:49 a.m. UTC | #2
On Mon, Jun 05, 2017 at 02:07:34PM -0500, Alan Tull wrote:
> From: Tobias Klauser <tklauser@distanz.ch>
> 
> Add COMPILE_TEST to the Kconfig entry for the Altera SoCFPGA FPGA
> Bridge. The Altera FPGA Freeze Bridge can also be used on Altera PEIe
> FPGAs, so the driver shouldn't depend on ARCH_SOCFPGA in the first place.
> This allows test-compiling the drivers on other architectures to catch
> compiler errors/warnings, e.g. due to API/header changes earlier on.
> 
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> Signed-off-by: Alan Tull <atull@kernel.org>
> ---
>  drivers/fpga/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

I'll drop this patch from the series, as kbuild reports errors with it
:(

--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tobias Klauser June 9, 2017, 1:44 p.m. UTC | #3
On 2017-06-09 at 11:49:02 +0200, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> On Mon, Jun 05, 2017 at 02:07:34PM -0500, Alan Tull wrote:
> > From: Tobias Klauser <tklauser@distanz.ch>
> > 
> > Add COMPILE_TEST to the Kconfig entry for the Altera SoCFPGA FPGA
> > Bridge. The Altera FPGA Freeze Bridge can also be used on Altera PEIe
> > FPGAs, so the driver shouldn't depend on ARCH_SOCFPGA in the first place.
> > This allows test-compiling the drivers on other architectures to catch
> > compiler errors/warnings, e.g. due to API/header changes earlier on.
> > 
> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > Signed-off-by: Alan Tull <atull@kernel.org>
> > ---
> >  drivers/fpga/Kconfig | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> I'll drop this patch from the series, as kbuild reports errors with it
> :(

These are warnings from the m32r cross compiler, not errors. They are
due to the way readl() is defined on m32r (returning unsigned long
instead of u32, as all other architectures do, see [1]). On all other
architectures the patch doesn't cause any issues. There are also other
cases which are already in mainline where this issue appers, so I'd say
this patch rather uncovers the symptom of a problem rather than causing
it ;)

[1] https://marc.info/?l=linux-kernel&m=149252925326414
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alan Tull June 10, 2017, 3:55 a.m. UTC | #4
On Fri, Jun 9, 2017 at 8:44 AM, Tobias Klauser <tklauser@distanz.ch> wrote:
> On 2017-06-09 at 11:49:02 +0200, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>> On Mon, Jun 05, 2017 at 02:07:34PM -0500, Alan Tull wrote:
>> > From: Tobias Klauser <tklauser@distanz.ch>
>> >
>> > Add COMPILE_TEST to the Kconfig entry for the Altera SoCFPGA FPGA
>> > Bridge. The Altera FPGA Freeze Bridge can also be used on Altera PEIe
>> > FPGAs, so the driver shouldn't depend on ARCH_SOCFPGA in the first place.
>> > This allows test-compiling the drivers on other architectures to catch
>> > compiler errors/warnings, e.g. due to API/header changes earlier on.
>> >
>> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
>> > Signed-off-by: Alan Tull <atull@kernel.org>
>> > ---
>> >  drivers/fpga/Kconfig | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> I'll drop this patch from the series, as kbuild reports errors with it
>> :(
>
> These are warnings from the m32r cross compiler, not errors. They are
> due to the way readl() is defined on m32r (returning unsigned long
> instead of u32, as all other architectures do, see [1]). On all other
> architectures the patch doesn't cause any issues. There are also other
> cases which are already in mainline where this issue appers, so I'd say
> this patch rather uncovers the symptom of a problem rather than causing
> it ;)
>
> [1] https://marc.info/?l=linux-kernel&m=149252925326414

Yes the issue is the way m32r defines readl().  Tobias has pointed out
[2] other kernel patches cause the same warning.  There is a trivial
suggested workaround in [1] above (declare a local u32 to take the
readl() value).  Doing that would be effort to mask a known issue that
lies in the m32r code though.

Alan

[2] https://marc.info/?l=linux-kernel&m=149260369012554&w=2
--
To unsubscribe from this list: send the line "unsubscribe linux-fpga" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 10361902c830..d6493655de1b 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -67,14 +67,14 @@  config FPGA_BRIDGE
 
 config SOCFPGA_FPGA_BRIDGE
 	tristate "Altera SoCFPGA FPGA Bridges"
-	depends on ARCH_SOCFPGA && FPGA_BRIDGE
+	depends on (ARCH_SOCFPGA || COMPILE_TEST) && FPGA_BRIDGE
 	help
 	  Say Y to enable drivers for FPGA bridges for Altera SOCFPGA
 	  devices.
 
 config ALTERA_FREEZE_BRIDGE
 	tristate "Altera FPGA Freeze Bridge"
-	depends on ARCH_SOCFPGA && FPGA_BRIDGE
+	depends on FPGA_BRIDGE
 	help
 	  Say Y to enable drivers for Altera FPGA Freeze bridges.  A
 	  freeze bridge is a bridge that exists in the FPGA fabric to