On Mon, 25 Jan 2016, Rob Herring wrote:
> On Fri, Jan 22, 2016 at 6:07 PM, Moritz Fischer
> <moritz.fischer@xxxxxxxxx> wrote:
> > On Fri, Jan 22, 2016 at 5:37 PM, atull <atull@xxxxxxxxxxxxxxxxxxxxx> wrote:
> >> On Fri, 22 Jan 2016, Moritz Fischer wrote:
> >>
> >>> Alan,
> >>>
> >>> On Wed, Jan 20, 2016 at 8:24 PM, <atull@xxxxxxxxxxxxxxxxxxxxx> wrote:
> >>>
> >>> > +static int fpga_area_probe(struct platform_device *pdev)
> >>> > +{
> >>> > + struct device *dev = &pdev->dev;
> >>> > + struct device_node *np = dev->of_node;
> >>> > + struct fpga_area *area;
> >>> > + int ret;
> >>> > +
> >>> > + area = devm_kzalloc(dev, sizeof(*area), GFP_KERNEL);
> >>> > + if (!area)
> >>> > + return -ENOMEM;
> >>> > +
> >>> > + INIT_LIST_HEAD(&area->bridge_list);
> >>> > +
> >>> > + ret = fpga_bridge_register(dev, "FPGA Area", NULL, area);
> >>> > + if (ret)
> >>> > + return ret;
> >>> > + area->br = dev_get_drvdata(dev);
> >>> > +
> >>> > + if (of_property_read_string(np, "firmware-name",
> >>> > + &area->firmware_name)) {
> >>> > + of_platform_populate(np, of_default_bus_match_table, NULL, dev);
> >>> > + return 0;
> >>> > + }
> >>>
> >>> This is the use case where the bootloader loaded the fpga, and you
> >>> just want to populate
> >>> the devices in the fabric, right?
> >>
> >> Hi Moritz,
> >>
> >> Yes
>
> That is very strange logic. It should be fine to just call
> of_platform_populate unconditionally. If there are no children of np,
> then it will be a nop.
That's what it is doing. It's coded this way to reduce indentation. If there
is no FPGA image to load, call of_platform_populate() and return. Otherwise do
a bunch of steps to load the FPGA image and handle the bridges, then call
of_platform_populate() and return. If there's no children, no problem.
>
> >>> > + if (of_property_read_bool(np, "partial-reconfig"))
> >>> > + area->flags |= FPGA_MGR_PARTIAL_RECONFIG;
> >>> > +
> >>> > + ret = fpga_area_get_bus(area);
> >>> > + if (ret) {
> >>> > + dev_dbg(dev, "Should be child of a FPGA Bus");
> >>> > + goto err_unreg;
> >>> > + }
> >>>
> >>> Looking at socfpga.dtsi, would that mean that the fpgamgr0 node would
> >>> need to become a subnode of fpgabus@0 at the same place?
> >>>
> >>> i.e. /soc/fpgamgr@ff706000 -> /soc/fpgabus@0/fpgamgr@ff706000
> >>>
> >>> and the ranges property would be used to translate to the fpga memory
> >>> mapped space?
> >>>
> >>> I know we're going back and forth on this. I think Rob brought up a
> >>> similar question:
> >>> "Does the bus really go thru the fpgamgr and then the bridge as this
> >>> implies? Or fpgamgr is a sideband controller?"
> >>>
> >>> To which I think the answer is 'sideband' controller, yet with the new
> >>> bindings it looks like
> >>> the bus goes through the fpgamgr.
> >>
> >> Yeah, let's get this right. First, let's be clear on the reason for FPGA Bus to
> >> exist. There may be >1 FPGA in a system. I want the FPGA Bus bring together
> >> the bridges and manager that are associated with a certain FPGA. This allows
> >> the system designer to specify which FPGA is getting programmed with which
> >> image/hardware. So at minimum, we need some way of associating a FPGA Bus with
> >> a FPGA Manager.
> >
> > I see your argument for the FPGA bus. I agree that we need to
> > distinguish different FPGAs,
> > and we need a way to associate an area with a manager (and potentially bridges).
> >
> >> As far as the target path is concerned, in the case of no bridges, we could have
> >> the overlay target the FPGA Bus instead of the FPGA Manager. That may be more
> >> logical. This would just be a documentation change; I think fpga-area.c will
> >> work OK if you specify the FPGA bus as your target (the manager still has to
> >> be a child of the bus so the bus knows what manager to use).
> >
> > Could the bus not just use a phandle to the manager? Or the area a
> > phandle to the bus?
>
> That may be better as it would avoid the virtual fpga-bus which is a
> bit questionable. I'm okay with allowing it, but will happily take any
> examples where it doesn't work. However, I'm not sure this case is
> one.
I have no problem with specifying FPGA manager with a phandle, seems
like it will be better in some cases.
I'm proposing FPGA Bus to specify all the things (manager and bridges) that are
needed to do a reprogramming cycle on a specific FPGA. The FPGA Bus may seem
less necessary in Moritz' case where there are no FPGA Bridges in the DT.
I'll discuss this more on the other thread.
>
> > Like that one could have potentially disjunct groups. Say I have a SPI
> > device that is in an FPGA area.
> > With our current system, I'd have a FPGA Manager that needs to be a
> > child of the bus as child of the SPI controller
> > with the memory addresses being addresses on the SOC's memory bus:
> >
> > spi_ctrl@deadbeef {
> > fpga_bus@0 {
> > fpgamgr@f8007000 {
> > mgr regs etc...
> > ... and now the SPI slaves ...
> > slave@42 {
> > };
> > };
> > };
> > };
>
> I think Alan's assumption is there's always a memory mapped control
> interface and the DT hierarchy follows that. While I see you could
> have other interfaces like SPI, that would not be the control
> interface related to FPGA programming AIUI. You could also have GPIOs,
> I2C, or any other bus/interface we describe in DT in theory.
>
> How to deal with additional connections like this is similar to
> overlays for daughterboards (e.g. capes, hats, shields) where the need
> for mapping slave devices on a connector to host controllers/buses
> which can vary. In that case, you don't want to have to create a
> different overlay for every host board just to change the parent
> devices of each child. If there is a SPI connection to FPGA (or area),
> then we need a way to map the host SPI bus to the FPGA. In this
> example, the overlay should only say "slave@42" is connected to the
> spi bus of the FPGA area and it would not have direct reference to
> "spi_ctrl@deadbeef." There's some ideas around how to support these
> usecases probably involving a connector node with mappings of
> connector i/o's to host i/o's (paging Pantelis, where's your
> proposal?). I think the same would work here, so I would suggest we
> not try to address it immediately other than decide the requirements
> here are similar enough to what is needed for connectors.
>
> Rob
>
--
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
