On Fri, Jun 10, 2016 at 4:18 AM, Trent Piepho <tpiepho@xxxxxxxxxxxxxx> wrote: > On Fri, 2016-02-05 at 15:30 -0600, atull@xxxxxxxxxxxxxxxxxxxxx wrote: >> Supports Altera SOCFPGA bridges: >> * fpga2sdram >> * fpga2hps >> * hps2fpga >> * lwhps2fpga >> >> Allows enabling/disabling the bridges through the FPGA >> Bridge Framework API functions. > > I'm replying to v16 because it exists on gmane, while v17 appears not > to. lkml.org's forward feature appears to be broken so I can't reply to > that message (no way to get message-id). But v17 of this patch should > be the same. If a v18 was posted, I've not been able to find it. > >> diff --git a/drivers/fpga/altera-hps2fpga.c b/drivers/fpga/altera-hps2fpga.c >> new file mode 100644 >> index 0000000..c15df47 >> --- /dev/null >> +++ b/drivers/fpga/altera-hps2fpga.c >> @@ -0,0 +1,213 @@ >> +/* >> + * 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 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 { >> + 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 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; > > Doesn't seem like this belongs here. I realize the write-only register > is a problem. Maybe the syscon driver should be initializing this > value? > >> + >> + 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); > > This isn't going work if more than one bridge is used. Each bridge has > its own priv and thus priv->l3_remap_value. Each bridge's priv will > have just the bit for it's own remap set. The 2nd bridge to be enabled > will turn off the 1st bridge when it re-write the l3 register. I can confirm this is exactly what happens with tag "rel_socfpga-4.1.22-ltsi_16.06.02_pr" of socfpga-4.1.22-ltsi branch from altera-opensource/linux-socfpga which includes more or less the code in this patch. If you have 2 bridges (lw-hps2fpga and hps2fpga) you end up with only one of them being visible. Easily spot by logging l3_remap_value being passed to regmap_write()... -- 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