On Wed, 28 Oct 2015, Steffen Trumtrar wrote: > On Tue, Oct 27, 2015 at 05:09:15PM -0500, atull@xxxxxxxxxxxxxxxxxxxxx wrote: > > From: Alan Tull <atull@xxxxxxxxxxxxxxxxxxxxx> > > > > Supports Altera SOCFPGA bridges: > > * fpga2sdram > > * fpga2hps > > * hps2fpga > > * lwhps2fpga > > > > Allows enabling/disabling the bridges through the FPGA > > Bridge Framework API functions. > > > > The fpga2sdram driver only supports enabling and disabling > > of the ports that been configured early on. This is due to > > a hardware limitation where the read, write, and command > > ports on the fpga2sdram bridge can only be reconfigured > > while there are no transactions to the sdram, i.e. when > > running out of OCRAM before the kernel boots. > > > > Device tree property 'init-val' configures the driver to > > enable or disable the bridge during probe. If the property > > does not exist, the driver will leave the bridge in its > > current state. > > > > Signed-off-by: Alan Tull <dinguyen@xxxxxxxxxxxxxxxxxxxxx> > > Signed-off-by: Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx> > > Signed-off-by: Matthew Gerlach <mgerlach@xxxxxxxxxx> > > --- > > v2: Use resets instead of directly writing reset registers > > v12: Bump version to align with simple-fpga-bus version > > Get rid of the sysfs interface > > fpga2sdram: get configuration stored in handoff register > > --- > > drivers/fpga/Kconfig | 7 ++ > > drivers/fpga/Makefile | 1 + > > drivers/fpga/altera-fpga2sdram.c | 185 ++++++++++++++++++++++++++++++ > > drivers/fpga/altera-hps2fpga.c | 234 ++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 427 insertions(+) > > create mode 100644 drivers/fpga/altera-fpga2sdram.c > > create mode 100644 drivers/fpga/altera-hps2fpga.c > > > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig > > index 143072b..004b83a 100644 > > --- a/drivers/fpga/Kconfig > > +++ b/drivers/fpga/Kconfig > > @@ -39,6 +39,13 @@ config FPGA_BRIDGE > > Say Y here if you want to support bridges connected between host > > processors and FPGAs or between FPGAs. > > > > +config SOCFPGA_FPGA_BRIDGE > > + bool "Altera SoCFPGA FPGA Bridges" > > + depends on ARCH_SOCFPGA && FPGA_BRIDGE > > + help > > + Say Y to enable drivers for FPGA bridges for Altera SOCFPGA > > + devices. > > + > > endif # FPGA > > > > endmenu > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile > > index 9302662..cc01db3 100644 > > --- a/drivers/fpga/Makefile > > +++ b/drivers/fpga/Makefile > > @@ -11,6 +11,7 @@ obj-$(CONFIG_FPGA_MGR_SOCFPGA_A10) += socfpga-a10.o > > > > # FPGA Bridge Drivers > > obj-$(CONFIG_FPGA_BRIDGE) += fpga-bridge.o > > +obj-$(CONFIG_SOCFPGA_FPGA_BRIDGE) += altera-hps2fpga.o altera-fpga2sdram.o > > > > # High Level Interfaces > > obj-$(CONFIG_SIMPLE_FPGA_BUS) += simple-fpga-bus.o > > diff --git a/drivers/fpga/altera-fpga2sdram.c b/drivers/fpga/altera-fpga2sdram.c > > new file mode 100644 > > index 0000000..ee56903 > > --- /dev/null > > +++ b/drivers/fpga/altera-fpga2sdram.c > > @@ -0,0 +1,185 @@ > > +/* > > + * FPGA to SDRAM Bridge Driver for Altera SoCFPGA Devices > > + * > > + * Copyright (C) 2013-2015 Altera Corporation, All Rights Reserved. > > + * > > + * This program is free software; you can redistribute it and/or modify it > > + * under the terms and conditions of the GNU General Public License, > > + * version 2, as published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope it will be useful, but WITHOUT > > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > > + * more details. > > + * > > + * You should have received a copy of the GNU General Public License along with > > + * this program. If not, see <http://www.gnu.org/licenses/>. > > + */ > > + > > +/* > > + * This driver manages a bridge between an FPGA and the SDRAM used by the ARM > > + * host processor system (HPS). > > + * > > + * The bridge contains 4 read ports, 4 write ports, and 6 command ports. > > + * Reconfiguring these ports requires that no SDRAM transactions occur during > > + * reconfiguration. The code reconfiguring the ports cannot run out of SDRAM > > + * nor can the FPGA access the SDRAM during reconfiguration. This driver does > > + * not support reconfiguring the ports. The ports are configured by code > > + * running out of on chip ram before Linux is started and the configuration > > + * is passed in a handoff register in the system manager. > > + * > > + * This driver supports enabling and disabling of the configured ports, which > > + * allows for safe reprogramming of the FPGA, assuming that the new FPGA image > > + * uses the same port configuration. Bridges must be disabled before > > + * reprogramming the FPGA and re-enabled after the FPGA has been programmed. > > + */ > > + > > +#include <linux/fpga/fpga-bridge.h> > > +#include <linux/kernel.h> > > +#include <linux/mfd/syscon.h> > > +#include <linux/module.h> > > +#include <linux/of_platform.h> > > +#include <linux/regmap.h> > > + > > +#define ALT_SDR_CTL_FPGAPORTRST_OFST 0x80 > > +#define ALT_SDR_CTL_FPGAPORTRST_PORTRSTN_MSK 0x00003fff > > +#define ALT_SDR_CTL_FPGAPORTRST_RD_SHIFT 0 > > +#define ALT_SDR_CTL_FPGAPORTRST_WR_SHIFT 4 > > +#define ALT_SDR_CTL_FPGAPORTRST_CTRL_SHIFT 8 > > + > > +#define SYSMGR_ISWGRP_HANDOFF3 (0x8C) > > +#define ISWGRP_HANDOFF_FPGA2SDR SYSMGR_ISWGRP_HANDOFF3 > > + > > +#define F2S_BRIDGE_NAME "fpga2sdram" > > + > > +struct alt_fpga2sdram_data { > > + struct device *dev; > > + struct regmap *sdrctl; > > + int mask; > > +}; > > + > > +static int alt_fpga2sdram_enable_show(struct fpga_bridge *bridge) > > +{ > > + struct alt_fpga2sdram_data *priv = bridge->priv; > > + int value; > > + > > + regmap_read(priv->sdrctl, ALT_SDR_CTL_FPGAPORTRST_OFST, &value); > > + > > + return (value & priv->mask) == priv->mask; > > +} > > + > > +static inline int _alt_fpga2sdram_enable_set(struct alt_fpga2sdram_data *priv, > > + bool enable) > > +{ > > + return regmap_update_bits(priv->sdrctl, ALT_SDR_CTL_FPGAPORTRST_OFST, > > + priv->mask, enable ? priv->mask : 0); > > +} > > + > > +static int alt_fpga2sdram_enable_set(struct fpga_bridge *bridge, bool enable) > > +{ > > + return _alt_fpga2sdram_enable_set(bridge->priv, enable); > > +} > > + > > +struct prop_map { > > + char *prop_name; > > + uint32_t *prop_value; > > + uint32_t prop_max; > > +}; > > + > > +struct fpga_bridge_ops altera_fpga2sdram_br_ops = { > > + .enable_set = alt_fpga2sdram_enable_set, > > + .enable_show = alt_fpga2sdram_enable_show, > > +}; > > + > > +static const struct of_device_id altera_fpga_of_match[] = { > > + { .compatible = "altr,socfpga-fpga2sdram-bridge" }, > > + {}, > > +}; > > + > > +static int alt_fpga_bridge_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct alt_fpga2sdram_data *priv; > > + uint32_t init_val; > > + struct regmap *sysmgr; > > + int ret = 0; > > + > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + priv->dev = dev; > > + > > + priv->sdrctl = syscon_regmap_lookup_by_compatible("altr,sdr-ctl"); > > + if (IS_ERR(priv->sdrctl)) { > > + dev_err(dev, "regmap for altr,sdr-ctl lookup failed.\n"); > > + return PTR_ERR(priv->sdrctl); > > + } > > + > > + sysmgr = syscon_regmap_lookup_by_compatible("altr,sys-mgr"); > > + if (IS_ERR(priv->sdrctl)) { > > + dev_err(dev, "regmap for altr,sys-mgr lookup failed.\n"); > > + return PTR_ERR(sysmgr); > > + } > > + > > + /* Get f2s bridge configuration saved in handoff register */ > > + regmap_read(sysmgr, SYSMGR_ISWGRP_HANDOFF3, &priv->mask); > > + > > + ret = fpga_bridge_register(dev, F2S_BRIDGE_NAME, > > + &altera_fpga2sdram_br_ops, priv); > > + if (ret) > > + return ret; > > + > > + dev_info(dev, "driver initialized with handoff %08x\n", priv->mask); > > Hm, what does this do? All I can get from the documentation is, that this > is a general purpose register and has nothing to do with this particular bridge. > It's described at the top of the file. This bridge has to be configured before the kernel boots, and this is a way of passing that configuration. > > + > > + if (!of_property_read_u32(dev->of_node, "init-val", &init_val)) { > > + if (init_val > 1) { > > + dev_warn(dev, "invalid init-val %u > 1\n", init_val); > > + } else { > > + dev_info(dev, "%s bridge\n", > > + (init_val ? "enabling" : "disabling")); > > + ret = _alt_fpga2sdram_enable_set(priv, init_val); > > + if (ret) { > > + fpga_bridge_unregister(&pdev->dev); > > + return ret; > > + } > > + } > > + } > > + > > + return ret; > > +} > > + > > +static int alt_fpga_bridge_remove(struct platform_device *pdev) > > +{ > > + fpga_bridge_unregister(&pdev->dev); > > + > > + return 0; > > +} > > + > > +MODULE_DEVICE_TABLE(of, altera_fpga_of_match); > > + > > +static struct platform_driver altera_fpga_driver = { > > + .remove = alt_fpga_bridge_remove, > > + .driver = { > > + .name = "altera_fpga2sdram_bridge", > > + .of_match_table = of_match_ptr(altera_fpga_of_match), > > + }, > > +}; > > + > > +static int __init alt_fpga_bridge_init(void) > > +{ > > + return platform_driver_probe(&altera_fpga_driver, > > + alt_fpga_bridge_probe); > > +} > > + > > +static void __exit alt_fpga_bridge_exit(void) > > +{ > > + platform_driver_unregister(&altera_fpga_driver); > > +} > > + > > +module_init(alt_fpga_bridge_init); > > +module_exit(alt_fpga_bridge_exit); > > + > > +MODULE_DESCRIPTION("Altera SoCFPGA FPGA to SDRAM Bridge"); > > +MODULE_AUTHOR("Alan Tull <atull@xxxxxxxxxxxxxxxxxxxxx>"); > > +MODULE_LICENSE("GPL v2"); > > diff --git a/drivers/fpga/altera-hps2fpga.c b/drivers/fpga/altera-hps2fpga.c > > new file mode 100644 > > index 0000000..3ec6394 > > --- /dev/null > > +++ b/drivers/fpga/altera-hps2fpga.c > > @@ -0,0 +1,234 @@ > > +/* > > + * FPGA to/from HPS Bridge Driver for Altera SoCFPGA Devices > > + * > > + * Copyright (C) 2013-2015 Altera Corporation, All Rights Reserved. > > + * > > + * This program is free software; you can redistribute it and/or modify it > > + * under the terms and conditions of the GNU General Public License, > > + * version 2, as published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope it will be useful, but WITHOUT > > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > > + * more details. > > + * > > + * You should have received a copy of the GNU General Public License along with > > + * this program. If not, see <http://www.gnu.org/licenses/>. > > + */ > > + > > +/* > > + * This driver manages bridges on a Altera SOCFPGA between the ARM host > > + * processor system (HPS) and the embedded FPGA. > > + * > > + * This driver supports enabling and disabling of the configured ports, which > > + * allows for safe reprogramming of the FPGA, assuming that the new FPGA image > > + * uses the same port configuration. Bridges must be disabled before > > + * reprogramming the FPGA and re-enabled after the FPGA has been programmed. > > + */ > > + > > +#include <linux/clk.h> > > +#include <linux/fpga/fpga-bridge.h> > > +#include <linux/kernel.h> > > +#include <linux/mfd/syscon.h> > > +#include <linux/module.h> > > +#include <linux/of_platform.h> > > +#include <linux/regmap.h> > > +#include <linux/reset.h> > > + > > +#define SOCFPGA_RSTMGR_BRGMODRST 0x1c > > +#define ALT_RSTMGR_BRGMODRST_H2F_MSK 0x00000001 > > +#define ALT_RSTMGR_BRGMODRST_LWH2F_MSK 0x00000002 > > +#define ALT_RSTMGR_BRGMODRST_F2H_MSK 0x00000004 > > + > > +#define ALT_L3_REMAP_OFST 0x0 > > +#define ALT_L3_REMAP_MPUZERO_MSK 0x00000001 > > +#define ALT_L3_REMAP_H2F_MSK 0x00000008 > > +#define ALT_L3_REMAP_LWH2F_MSK 0x00000010 > > + > > +#define HPS2FPGA_BRIDGE_NAME "hps2fpga" > > +#define LWHPS2FPGA_BRIDGE_NAME "lwhps2fpga" > > +#define FPGA2HPS_BRIDGE_NAME "fpga2hps" > > + > > +struct altera_hps2fpga_data { > This name and the alt_hps2fpga_xxx functions are confusing, as you have > one bridge of this name but use it for all the bridges. I think it's OK. > > > + const char *name; > > + struct reset_control *bridge_reset; > > + struct regmap *l3reg; > > + /* The L3 REMAP register is write only, so keep a cached value. */ > > + unsigned int l3_remap_value; > > + unsigned int reset_mask; > > + unsigned int remap_mask; > > + struct clk *clk; > > +}; > > + > > +static int alt_hps2fpga_enable_show(struct fpga_bridge *bridge) > > +{ > > + struct altera_hps2fpga_data *priv = bridge->priv; > > + > > + return reset_control_status(priv->bridge_reset); > > +} > > + > > +static int _alt_hps2fpga_enable_set(struct altera_hps2fpga_data *priv, > > + bool enable) > > +{ > > + int ret; > > + > > + /* bring bridge out of reset */ > > + if (enable) > > + ret = reset_control_deassert(priv->bridge_reset); > > + else > > + ret = reset_control_assert(priv->bridge_reset); > > + if (ret) > > + return ret; > > + > > + /* Allow bridge to be visible to L3 masters or not */ > > + if (priv->remap_mask) { > > + priv->l3_remap_value |= ALT_L3_REMAP_MPUZERO_MSK; > > Why do you remap the On-chip RAM here. This doesn't belong in this > driver. We have to when we enable or disable. > > > + > > + if (enable) > > + priv->l3_remap_value |= priv->remap_mask; > > + else > > + priv->l3_remap_value &= ~priv->remap_mask; > > + > > + ret = regmap_write(priv->l3reg, ALT_L3_REMAP_OFST, > > + priv->l3_remap_value); > > regmap_update_bits? Otherwise you will overwrite the other bridges > settings. This works. This is a write-only register. > > > + } > > + > > + return ret; > > +} > > + > > +static int alt_hps2fpga_enable_set(struct fpga_bridge *bridge, bool enable) > > +{ > > + return _alt_hps2fpga_enable_set(bridge->priv, enable); > > +} > > + > > +struct fpga_bridge_ops altera_hps2fpga_br_ops = { > > + .enable_set = alt_hps2fpga_enable_set, > > + .enable_show = alt_hps2fpga_enable_show, > > +}; > > + > > +static struct altera_hps2fpga_data hps2fpga_data = { > > + .name = HPS2FPGA_BRIDGE_NAME, > > + .reset_mask = ALT_RSTMGR_BRGMODRST_H2F_MSK, > > + .remap_mask = ALT_L3_REMAP_H2F_MSK, > > +}; > > + > > +static struct altera_hps2fpga_data lwhps2fpga_data = { > > + .name = LWHPS2FPGA_BRIDGE_NAME, > > + .reset_mask = ALT_RSTMGR_BRGMODRST_LWH2F_MSK, > > + .remap_mask = ALT_L3_REMAP_LWH2F_MSK, > > +}; > > + > > +static struct altera_hps2fpga_data fpga2hps_data = { > > + .name = FPGA2HPS_BRIDGE_NAME, > > + .reset_mask = ALT_RSTMGR_BRGMODRST_F2H_MSK, > > +}; > > + > > +static const struct of_device_id altera_fpga_of_match[] = { > > + { .compatible = "altr,socfpga-hps2fpga-bridge", > > + .data = &hps2fpga_data }, > > + { .compatible = "altr,socfpga-lwhps2fpga-bridge", > > + .data = &lwhps2fpga_data }, > > + { .compatible = "altr,socfpga-fpga2hps-bridge", > > + .data = &fpga2hps_data }, > > + {}, > > +}; > > + > > +static int alt_fpga_bridge_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct altera_hps2fpga_data *priv; > > + const struct of_device_id *of_id; > > + uint32_t init_val; > > + int ret; > > + > > + of_id = of_match_device(altera_fpga_of_match, dev); > > + priv = (struct altera_hps2fpga_data *)of_id->data; > > + WARN_ON(!priv); > > Why don't you just return here if you have a NULL pointer? Actually we don't need that WARN_ON. I can take it out. > > > + > > + priv->bridge_reset = devm_reset_control_get(dev, priv->name); > > + if (IS_ERR(priv->bridge_reset)) { > > + dev_err(dev, "Could not get %s reset control!\n", priv->name); > > + return PTR_ERR(priv->bridge_reset); > > + } > > + > > + priv->l3reg = syscon_regmap_lookup_by_compatible("altr,l3regs"); > > + if (IS_ERR(priv->l3reg)) { > > + dev_err(dev, "regmap for altr,l3regs lookup failed.\n"); > > + return PTR_ERR(priv->l3reg); > > + } > > + > > + priv->clk = of_clk_get(dev->of_node, 0); > > + if (IS_ERR(priv->clk)) { > > + dev_err(dev, "no clock specified\n"); > > + return PTR_ERR(priv->clk); > > + } > > + > > + ret = clk_prepare_enable(priv->clk); > > + if (ret) { > > + dev_err(dev, "could not enable clock\n"); > > + return -EBUSY; > > + } > > + > > + ret = fpga_bridge_register(dev, priv->name, &altera_hps2fpga_br_ops, > > + priv); > > + if (ret) > > + return ret; > > + > > + if (!of_property_read_u32(dev->of_node, "init-val", &init_val)) { > > + if (init_val > 1) { > > + dev_warn(dev, "invalid init-val %u > 1\n", init_val); > > + } else { > > + dev_info(dev, "%s bridge\n", > > + (init_val ? "enabling" : "disabling")); > > + > > + ret = _alt_hps2fpga_enable_set(priv, init_val); > > + if (ret) { > > + fpga_bridge_unregister(&pdev->dev); > > + return ret; > > + } > > + } > > + } > > + > > + return ret; > > +} > > + > > +static int alt_fpga_bridge_remove(struct platform_device *pdev) > > +{ > > + struct fpga_bridge *bridge = platform_get_drvdata(pdev); > > + struct altera_hps2fpga_data *priv = bridge->priv; > > + > > + fpga_bridge_unregister(&pdev->dev); > > + > > + clk_disable_unprepare(priv->clk); > > + clk_put(priv->clk); > > + > > + return 0; > > +} > > + > > +MODULE_DEVICE_TABLE(of, altera_fpga_of_match); > > + > > +static struct platform_driver altera_fpga_driver = { > > + .remove = alt_fpga_bridge_remove, > > + .driver = { > > + .name = "altera_hps2fpga_bridge", > > + .of_match_table = of_match_ptr(altera_fpga_of_match), > > + }, > > +}; > > + > > +static int __init alt_fpga_bridge_init(void) > > +{ > > + return platform_driver_probe(&altera_fpga_driver, > > + alt_fpga_bridge_probe); > > +} > > + > > +static void __exit alt_fpga_bridge_exit(void) > > +{ > > + platform_driver_unregister(&altera_fpga_driver); > > +} > > + > > +module_init(alt_fpga_bridge_init); > > +module_exit(alt_fpga_bridge_exit); > > + > > +MODULE_DESCRIPTION("Altera SoCFPGA HPS to FPGA Bridge"); > > +MODULE_AUTHOR("Alan Tull <atull@xxxxxxxxxxxxxxxxxxxxx>"); > > +MODULE_LICENSE("GPL v2"); > > Regards, > Steffen > > -- > Pengutronix e.K. | | > Industrial Linux Solutions | http://www.pengutronix.de/ | > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html