On Tue, Nov 3, 2015 at 10:28 AM, atull <atull@xxxxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, 30 Oct 2015, Rob Herring wrote:
>
>> On Thu, Oct 29, 2015 at 11:02 AM, atull <atull@xxxxxxxxxxxxxxxxxxxxx> wrote:
>> > On Wed, 28 Oct 2015, Rob Herring wrote:
>> >
>> >> On Tue, Oct 27, 2015 at 5:09 PM, <atull@xxxxxxxxxxxxxxxxxxxxx> wrote:
>> >> > From: Alan Tull <atull@xxxxxxxxxxxxxxxxxxxxx>
>> >> >
>> >> > New bindings document for simple fpga bus.
>> >> >
>> >> > Signed-off-by: Alan Tull <atull@xxxxxxxxxxxxxxxxxxxxx>
>> >> > ---
>> >> > v9: initial version added to this patchset
>> >> > v10: s/fpga/FPGA/g
>> >> > replace DT overlay example with slightly more complicated example
>> >> > move to staging/simple-fpga-bus
>> >> > v11: No change in this patch for v11 of the patch set
>> >> > v12: Moved out of staging.
>> >> > Changed to use FPGA bridges framework instead of resets
>> >> > for bridges.
>> >> > ---
>> >> > .../devicetree/bindings/fpga/simple-fpga-bus.txt | 81 ++++++++++++++++++++
>> >> > 1 file changed, 81 insertions(+)
>> >> > create mode 100644 Documentation/devicetree/bindings/fpga/simple-fpga-bus.txt
>> >> >
>> >> > diff --git a/Documentation/devicetree/bindings/fpga/simple-fpga-bus.txt b/Documentation/devicetree/bindings/fpga/simple-fpga-bus.txt
>> >> > new file mode 100644
>> >> > index 0000000..2e742f7
>> >> > --- /dev/null
>> >> > +++ b/Documentation/devicetree/bindings/fpga/simple-fpga-bus.txt
>> >> > @@ -0,0 +1,81 @@
>> >> > +Simple FPGA Bus
>> >> > +===============
>> >> > +
>> >> > +A Simple FPGA Bus is a bus that handles configuring an FPGA and its bridges
>> >> > +before populating the devices below its node. All this happens when a device
>> >> > +tree overlay is added to the live tree. This document describes that device
>> >> > +tree overlay.
>> >> > +
>> >> > +Required properties:
>> >> > +- compatible : should contain "simple-fpga-bus"
>> >> > +- #address-cells, #size-cells, ranges: must be present to handle address space
>> >> > + mapping for children.
>> >> > +
>> >> > +Optional properties:
>> >> > +- fpga-mgr : should contain a phandle to a FPGA manager.
>> >> > +- fpga-firmware : should contain the name of a FPGA image file located on the
>> >> > + firmware search path.
>> >>
>> >> Putting firmware filename in DT has come up in other cases recently[1]
>> >> and we concluded it should not be in the DT. Maybe the conclusion
>> >> would be different here, and if so we should have a common property
>> >> here.
>> >
>> > Interesting discussion.
>> >
>> > One FPGA image will almost always have multiple hardware devices in it.
>> > The device blocks will be reused in various combinations in different
>> > FPGA images. So hardwiring the image name in some specific driver won't
>> > be possible. Also many of the devices that could appear on a FPGA also
>> > could appear in normal hardware. Same driver for both cases, just
>> > different address.
>> >
>> > I won't have control of the name of the firmware file. It will be something
>> > that makes sense to a FPGA hardware guy so translating the node name to an
>> > image name would be brittle.
>> >
>> > Renaming this as a generic property "firmware-name" or "firmware" would be
>> > an improvement on what I proposed.
>> >
>> > If it is acceptible to have this in the DT, it means FPGA hardware development
>> > workflow does not require a kernel rebuild. The DT can collect together the
>> > image name, the bridges to the FPGA (or to that area of the FPGA), and a list
>> > of the devices/drivers in that image.
>>
>> You can deal with this purely in userspace.
>
> We have. It's ugly. That means we have to also deal with bridges from
> userspace since they have to be disable during FPGA programming. And the
> drivers have to be modules that we modprobe after FPGA programming.
I didn't mean everything. From the FPGA mgr perspective, it just calls
request_firmware. From there, whether the kernel does it or a
userspace script does it should be transparent to the rest of the
flow. I fully agree the flow should be controlled from within the
kernel.
>> The firmware userspace
>> helper can load any file you like. If the name is frequently changing,
>> then I agree it should not be in the kernel. But neither should it be
>> in the base DT. However, this would be in the overlay DT? In that use,
>> I think having the firmware name in DT is fine. The other option is
>> just put the firmware itself into the DT overlay (u-boot FIT images
>> are actually DTs with binary blobs). Either way please just create and
>> use a generic binding here.
>
> Planning to use overlays. I'll use "firmware-name."
Okay, but in v13 you didn't...
>
>>
>> >> > +- partial-reconfig : boolean property should be defined if partial
>> >> > + reconfiguration of the FPGA is to be done, otherwise full reconfiguration
>> >> > + is done.
>> >> > +- fpga-bridges : should contain a list of bridges that the bus will disable
>> >> > + before programming the FPGA and then enable after the FPGA has been
>> >> > +
>> >> > +Example:
>> >> > +
>> >> > +/dts-v1/;
>> >> > +/plugin/;
>> >> > +/ {
>> >> > + fragment@0 {
>> >> > + target-path="/soc";
>> >> > + __overlay__ {
>> >> > + #address-cells = <1>;
>> >> > + #size-cells = <1>;
>> >> > +
>> >> > + bridge@0xff200000 {
>> >> > + compatible = "simple-fpga-bus";
>> >> > + reg = <0xc0000000 0x20000000>,
>> >> > + <0xff200000 0x00200000>;
>> >>
>> >> You have registers for the bus, so therefore it is not simple. I think
>> >> the bus or bridge needs a specific compatible
>> >>
>> >
>> > The reg here is cruft from device tree generation. I don't use it and will
>> > clean it out. After I've done that, does that become simple again?
>> >
>> > What I need in a bus is:
>> > - Handles 'ranges'
>> > - Controls enabling/disabling bridges and programs the FPGA
>>
>> Where are these controls?
>
> The Simple FPGA Bus calls FPGA Bridge Framework functions to
> enable/disable the bridges.
Can you describe this in terms of the h/w? I'd expect the s/w to be
the other way around. The bridge f/w instantiates the bus.
What I'm failing to understand is how the bridges and buses you are
defining relate to each other. I understand that a bridge controls the
bus behind it, but that relationship is not evident in this series.
>
>>
>> > - Populates the child devices (and there will probably be many)
>>
>> simple-bus handles at least the 1st and 3rd item. I suppose you don't
>> want the bus to probe before the bridge driver. If you want the bridge
>> driver to control that, you don't actually need a bus name.
>> of_platform_populate() will create all child devices. It is only if
>> you want to create the grandchildren that you need a bus match on the
>> child nodes.
>>
>>
>> > This raises another issue: each area of the FPGA is likely to have multiple
>> > bridges. That's why I had a list of phandles to bridges rather than
>> > different bus for each type of bridge. Is that acceptible-ish?
>>
>> Hard to say. A bridge tends to mean a parent bus to child bus
>> translator and the DT hierarchy generally follows the bus hierarchy,
>> so really it is better if DT follows the physical bus structure. Of
>> course, we really only do that with outbound bus and not the inbound
>> side.
>>
>> I also worry that all this looks like it may be somewhat Altera specific.
>
> I don't think it is Altera specific. If you want to reprogram an
> FPGA with devices that need drivers, you will likely need to:
> 1. Disable some bridges to prevent spurious stuff on the bus
> 2. Load the FPGA
> 3. Enable the bridges
> 4. Probe some drivers
>
> In the case where the whole FPGA gets rewritten, it is possible to
> combine the bridge control with the FPGA manager (then it *is* manufacturer
> specific). In the case where part of the FPGA gets rewritten (partial
> reconfiguration), it is likely that some sort of bridge will exist on
> the FPGA to protect the bus. And there will be that kind of bridge for
> each different area of the FPGA that needs to get rewritten.
>
> The device tree overlay seems a very natural way of keeping all that
> information together in one place. Otherwise we have to invent
> something in userspace.
I'm not saying it doesn't belong in DT. I just want to hear that this
all makes sense from someone else that is !Altera.
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
