| Message ID | 1414108267-22058-3-git-send-email-atull@opensource.altera.com |
|---|---|
| State | Needs Review / ACK, archived |
| Headers | show |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 1 errors, 0 warnings, 0 lines checked |
| robh/patch-applied | success |
Hi! On Thu, Oct 23, 2014 at 06:51:06PM -0500, atull@opensource.altera.com wrote: > From: Alan Tull <atull@opensource.altera.com> (...) > diff --git a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > new file mode 100644 > index 0000000..bc24a2e > --- /dev/null > +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > @@ -0,0 +1,53 @@ > +Altera FPGA/HPS Bridge Driver > + > +This driver manages a bridge between a FPGA and a host processor system (HPS). > +User space can enable or disable the bridge by writing a "1" or a "0", > +respectively, to its enable file under bridge's entry in > +/sys/class/fpga-bridge. Typically, one disables the bridges before > +reprogramming the FPGA. Once the FPGA is reprogrammed, the bridges are > +reenabled. > + NAK. This is all linux specific and doesn't belong here. > +Required properties: > + > + - compatible : should contain one of: > + "altr,socfpga-hps2fpga-bridge" > + "altr,socfpga-lwhps2fpga-bridge" > + "altr,socfpga-fpga2hps-bridge" > + > + - clocks : clocks used by this module > + > + - altr,l3-syscon : phandle of the l3 interconnect module > + L3 shouldn't be a syscon. Have you tried dumping the regmap in the debugfs if L3 is a syscon? Doesn't work. > +Optional properties: > + - label : name that you want this bridge to show up as under > + /sys/class/fpga-bridge. Default is br<device#> if this is > + not specified. > + Why? Linux-specific. > + - init-val : 0 if driver should disable bridge at startup > + 1 if driver should enable bridge at startup > + driver leaves bridge in current state if property not > + specified. > + Configuration in the DT? Really? > +Example: > + hps_fpgabridge0: fpgabridge@0 { > + compatible = "altr,socfpga-hps2fpga-bridge"; > + label = "hps2fpga"; > + altr,l3-syscon = <&l3regs>; > + clocks = <&l4_main_clk>; > + init-val = <1>; > + }; > + > + hps_fpgabridge1: fpgabridge@1 { > + compatible = "altr,socfpga-lwhps2fpga-bridge"; > + label = "lwhps2fpga"; > + altr,l3-syscon = <&l3regs>; > + clocks = <&l4_main_clk>; > + init-val = <0>; > + }; > + > + hps_fpgabridge2: fpgabridge@2 { > + compatible = "altr,socfpga-fpga2hps-bridge"; > + label = "fpga2hps"; > + altr,l3-syscon = <&l3regs>; > + clocks = <&l4_main_clk>; > + }; The bridges are the buses into the FPGA. This has to be accomodated. The bridges have two specified memory ranges: one the address space of the bus, the second the register space for configuration. This binding does NOT correctly describe the hardware. Sorry. Regards, Steffen
Hi Stefan, Allan, Sorry, haven’t had a chance to review all this yet; will do so in the weekend. Just wanted to pop in and comment on a few things. > On Oct 24, 2014, at 10:00 AM, Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: > > Hi! > > On Thu, Oct 23, 2014 at 06:51:06PM -0500, atull@opensource.altera.com wrote: >> From: Alan Tull <atull@opensource.altera.com> > > (...) > >> diff --git a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt >> new file mode 100644 >> index 0000000..bc24a2e >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt >> @@ -0,0 +1,53 @@ >> +Altera FPGA/HPS Bridge Driver >> + >> +This driver manages a bridge between a FPGA and a host processor system (HPS). >> +User space can enable or disable the bridge by writing a "1" or a "0", >> +respectively, to its enable file under bridge's entry in >> +/sys/class/fpga-bridge. Typically, one disables the bridges before >> +reprogramming the FPGA. Once the FPGA is reprogrammed, the bridges are >> +reenabled. >> + > > NAK. > > This is all linux specific and doesn't belong here. > We know. The DT spec hasn’t been updated for a while, and this is going to be part of what we want to do with the restarting of the ePAPR spec process. So absolutes like “DT is a hardware description only" might be too strong statements, that do not work in practice anymore. There’s no such thing as pure hardware devices lately - there is always a configuration component; some of it OS specific, some of it not. We have to be engaged in the process and figure out how to update the spec for what is now the state of the art in the industry. Regards — Pantelis -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu 2014-10-23 18:51:06, atull@opensource.altera.com wrote: > From: Alan Tull <atull@opensource.altera.com> > > Add DTS binding documentation for the Altera FPGA bridges. > > Signed-off-by: Alan Tull <atull@opensource.altera.com> Acked-by: Pavel Machek <pavel@ucw.cz>
Hi Pantelis! On Fri, Oct 24, 2014 at 12:20:53PM +0300, Pantelis Antoniou wrote: > Hi Stefan, Allan, > > Sorry, haven’t had a chance to review all this yet; will do so in the weekend. > Just wanted to pop in and comment on a few things. > > > On Oct 24, 2014, at 10:00 AM, Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: > > > > Hi! > > > > On Thu, Oct 23, 2014 at 06:51:06PM -0500, atull@opensource.altera.com wrote: > >> From: Alan Tull <atull@opensource.altera.com> > > > > (...) > > > >> diff --git a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > >> new file mode 100644 > >> index 0000000..bc24a2e > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > >> @@ -0,0 +1,53 @@ > >> +Altera FPGA/HPS Bridge Driver > >> + > >> +This driver manages a bridge between a FPGA and a host processor system (HPS). > >> +User space can enable or disable the bridge by writing a "1" or a "0", > >> +respectively, to its enable file under bridge's entry in > >> +/sys/class/fpga-bridge. Typically, one disables the bridges before > >> +reprogramming the FPGA. Once the FPGA is reprogrammed, the bridges are > >> +reenabled. > >> + > > > > NAK. > > > > This is all linux specific and doesn't belong here. > > > > We know. The DT spec hasn’t been updated for a while, and this is going to be > part of what we want to do with the restarting of the ePAPR spec process. > As we don't have a new spec yet, I go with the current state of the art. And I don't see how "file under ... /sys/class/fpga-bridge" fits the current spec. > So absolutes like “DT is a hardware description only" might be too strong statements, that > do not work in practice anymore. > > There’s no such thing as pure hardware devices lately - there is always a configuration > component; some of it OS specific, some of it not. > If it really is necessary, I'm not against it, don't get me wrong. But in the grand scheme I wouldn't say that this fits my experience. There are some devices that need configuration and often when it is done in the DT, it is just a shortcut or not thought through. And can be also introspected dynamically. With some discussion on the list these bindings often change. Regarding OS specifics: already there, e.g. console setup in the chosen node. And other things I saw are ethernet aliases just for u-boot. Not a problem of the spec, just a problem of u-boot and unneccessary "configuration". Just to fix a bad bootloader. barebox can handle this without any such stuff. Maybe we should integrate the DT "overlays" barebox uses into the in-kernel DTs as well...but does it really help/interest someone who decides to use u-boot where barebox stores its environment? I guess not. > We have to be engaged in the process and figure out how to update the spec for what is > now the state of the art in the industry. > Again, not against that if it is necessary. For example your overlay stuff might be a good update to the spec. Would be better if it is official instead of a "hack". I want that for SoCFPGA. But having looked at one or two vendor kernels+DTs, the state of the art in the industry is: using DT wrong. (And doing HW wrong, which makes some updates to the ePAPR necessary ;-)) Regards, Steffen
Hi Alan, > On Oct 24, 2014, at 02:51 , atull@opensource.altera.com wrote: > > From: Alan Tull <atull@opensource.altera.com> > > Add DTS binding documentation for the Altera FPGA bridges. > > Signed-off-by: Alan Tull <atull@opensource.altera.com> > --- > .../bindings/fpga/altera-fpga2sdram-bridge.txt | 57 ++++++++++++++++++++ > .../bindings/fpga/altera-hps2fpga-bridge.txt | 53 ++++++++++++++++++ > 2 files changed, 110 insertions(+) > create mode 100644 Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt > create mode 100644 Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > > diff --git a/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt > new file mode 100644 > index 0000000..cc8f522 > --- /dev/null > +++ b/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt > @@ -0,0 +1,57 @@ > +Altera FPGA To SDRAM Bridge Driver > + > +This driver manages a bridge between an FPGA and the SDRAM used by an host > +processor system (HPS). The bridge contains a number read ports, write ports, > +and command ports. Reconfiguring these ports requires that no SDRAM > +transactions occur during reconfiguration. In other words, the code > +reconfiguring the ports cannot be run out of SDRAM nor can the FPGA access the > +SDRAM during the reconfiguration. This driver does not support reconfiguring > +the ports. Typcially, the ports are configured by code running out of onchip > +ram before Linux is started. > + > +This driver supports enabling and disabling of the configured ports all at > +once, which allows for safe reprogramming of the FPGA from user space, provided > +the new FPGA image uses the same port configuration. User space can enable or > +disable the bridge by writing a "1" or a "0", respectively, to its enable file > +under bridge's entry in /sys/class/fpga-bridge. Typically, one disables the > +bridges before reprogramming the FPGA. Once the FPGA is reprogrammed, the > +bridges are reenabled. > + > +Required properties: > + > + - compatible : "altr,socfpga-fpga2sdram-bridge" > + > + - read-ports-mask : Bits 0 to 3 corresponds read ports 0 to 3. A bit set to 1 > + indicates the corresponding read port should be enabled. > + > + - write-ports-mask : Bits 0 to 3 corresponds write ports 0 to 3. A bit set > + to 1 indicates the corresponding write port should be > + enabled. > + > + - cmd-ports-mask : Bits 0 to 5 correspond to command ports 0 to 5. A bit set > + to 1 indicates the corresponding command port should be > + enabled. > + > + - altr,sdr-syscon : phandle of the sdr module > + > +Optional properties: > + > + - label : name that you want this bridge to show up as under > + /sys/class/fpga-bridge. Default is br<device#> if this is > + not specified. > + > + - init-val : 0 if driver should disable bridge at startup > + 1 if driver should enable bridge at startup > + driver leaves bridge in current state if property not > + specified. Isn’t init-val a boolean property? It’s not named very well. Along with the label, is kinda hard to defend as configuration in DT. We need to start thinking about configuration, so let me throw that in and fan the Monday flames to a nice red-hot color. / { chosen { linux { fpga-bridge { node = <&FPGA_BRIDGE>; label = “foo”; enable-bridge-on-startup; } }; }; }; We will need accessors for drivers that iterate over the chosen/linux node children and pick up the properties meant for the given driver node. We also need to define behaviour in case those configuration properties are absent. Let the flames begin… > + > +Example: > + fpga2sdram_br: fpgabridge@3 { > + compatible = "altr,socfpga-fpga2sdram-bridge"; > + label = "fpga2sdram"; > + altr,sdr-syscon = <&sdr>; > + read-ports-mask = <3>; > + write-ports-mask = <3>; > + cmd-ports-mask = <0xd>; > + init-val = <0>; > + }; > diff --git a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > new file mode 100644 > index 0000000..bc24a2e > --- /dev/null > +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > @@ -0,0 +1,53 @@ > +Altera FPGA/HPS Bridge Driver > + > +This driver manages a bridge between a FPGA and a host processor system (HPS). > +User space can enable or disable the bridge by writing a "1" or a "0", > +respectively, to its enable file under bridge's entry in > +/sys/class/fpga-bridge. Typically, one disables the bridges before > +reprogramming the FPGA. Once the FPGA is reprogrammed, the bridges are > +reenabled. > + > +Required properties: > + > + - compatible : should contain one of: > + "altr,socfpga-hps2fpga-bridge" > + "altr,socfpga-lwhps2fpga-bridge" > + "altr,socfpga-fpga2hps-bridge" > + > + - clocks : clocks used by this module > + > + - altr,l3-syscon : phandle of the l3 interconnect module > + > +Optional properties: > + - label : name that you want this bridge to show up as under > + /sys/class/fpga-bridge. Default is br<device#> if this is > + not specified. > + > + - init-val : 0 if driver should disable bridge at startup > + 1 if driver should enable bridge at startup > + driver leaves bridge in current state if property not > + specified. > + > +Example: > + hps_fpgabridge0: fpgabridge@0 { > + compatible = "altr,socfpga-hps2fpga-bridge"; > + label = "hps2fpga"; > + altr,l3-syscon = <&l3regs>; > + clocks = <&l4_main_clk>; > + init-val = <1>; > + }; > + > + hps_fpgabridge1: fpgabridge@1 { > + compatible = "altr,socfpga-lwhps2fpga-bridge"; > + label = "lwhps2fpga"; > + altr,l3-syscon = <&l3regs>; > + clocks = <&l4_main_clk>; > + init-val = <0>; > + }; > + > + hps_fpgabridge2: fpgabridge@2 { > + compatible = "altr,socfpga-fpga2hps-bridge"; > + label = "fpga2hps"; > + altr,l3-syscon = <&l3regs>; > + clocks = <&l4_main_clk>; > + }; > -- > 1.7.9.5 > Regards — Pantelis -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Stefen, > On Oct 25, 2014, at 17:42 , Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: > > Hi Pantelis! > > On Fri, Oct 24, 2014 at 12:20:53PM +0300, Pantelis Antoniou wrote: >> Hi Stefan, Allan, >> >> Sorry, haven’t had a chance to review all this yet; will do so in the weekend. >> Just wanted to pop in and comment on a few things. >> >>> On Oct 24, 2014, at 10:00 AM, Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: >>> >>> Hi! >>> >>> On Thu, Oct 23, 2014 at 06:51:06PM -0500, atull@opensource.altera.com wrote: >>>> From: Alan Tull <atull@opensource.altera.com> >>> >>> (...) >>> >>>> diff --git a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt >>>> new file mode 100644 >>>> index 0000000..bc24a2e >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt >>>> @@ -0,0 +1,53 @@ >>>> +Altera FPGA/HPS Bridge Driver >>>> + >>>> +This driver manages a bridge between a FPGA and a host processor system (HPS). >>>> +User space can enable or disable the bridge by writing a "1" or a "0", >>>> +respectively, to its enable file under bridge's entry in >>>> +/sys/class/fpga-bridge. Typically, one disables the bridges before >>>> +reprogramming the FPGA. Once the FPGA is reprogrammed, the bridges are >>>> +reenabled. >>>> + >>> >>> NAK. >>> >>> This is all linux specific and doesn't belong here. >>> >> >> We know. The DT spec hasn’t been updated for a while, and this is going to be >> part of what we want to do with the restarting of the ePAPR spec process. >> > > As we don't have a new spec yet, I go with the current state of the art. > And I don't see how "file under ... /sys/class/fpga-bridge" fits the > current spec. > >> So absolutes like “DT is a hardware description only" might be too strong statements, that >> do not work in practice anymore. >> >> There’s no such thing as pure hardware devices lately - there is always a configuration >> component; some of it OS specific, some of it not. >> > > If it really is necessary, I'm not against it, don't get me wrong. > But in the grand scheme I wouldn't say that this fits my experience. > There are some devices that need configuration and often when it is > done in the DT, it is just a shortcut or not thought through. > And can be also introspected dynamically. With some discussion on the > list these bindings often change. > Already answered and given a possible way this might work; admittedly this specific example is not a good fit for what I was trying to say, but whatever. Let’s get the ball rolling. > Regarding OS specifics: already there, e.g. console setup in the chosen node. > And other things I saw are ethernet aliases just for u-boot. Not a problem > of the spec, just a problem of u-boot and unneccessary "configuration". > Just to fix a bad bootloader. barebox can handle this without any > such stuff. Maybe we should integrate the DT "overlays" barebox uses into > the in-kernel DTs as well...but does it really help/interest someone who > decides to use u-boot where barebox stores its environment? I guess not. > Although I’m not against having DT overlays in the bootloader, I only see them as a method that helps a platform developer express things easier. I am completely against having the kernel tied to any particular bootloader. We’ve all been through the hell where a kernel only boots if the bootloader has setup the platform “correctly”. This “correctly” has a very loose definition and 99/100 times is extremely badly documented or not at all. IMHO the bootloader should (as part of the normal boot sequence) only setup the absolutely bare minimum and then pass control to the kernel as soon as possible. The full featured bootloader environment should only be presented when the user requests so. >> We have to be engaged in the process and figure out how to update the spec for what is >> now the state of the art in the industry. >> > > Again, not against that if it is necessary. For example your overlay stuff might > be a good update to the spec. Would be better if it is official instead of a "hack". > I want that for SoCFPGA. > > But having looked at one or two vendor kernels+DTs, the state of the art in the > industry is: using DT wrong. (And doing HW wrong, which makes some updates to the > ePAPR necessary ;-)) > Vendor H/W, vendor DT and a crack pipe. Or as I call it Monday. > Regards, > Steffen Regards — Pantelis -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 27, 2014 at 01:48:02PM +0200, Pantelis Antoniou wrote: > > On Oct 24, 2014, at 02:51 , atull@opensource.altera.com wrote: > > + - init-val : 0 if driver should disable bridge at startup > > + 1 if driver should enable bridge at startup > > + driver leaves bridge in current state if property not > > + specified. > Isn’t init-val a boolean property? It’s not named very well. It's not boolean, it's tristate - turn on, turn off or don't touch. > Along with the label, is kinda hard to defend as configuration in DT. Yeah... presumably this decision would fall out of the users?
Hi Mark, > On Oct 27, 2014, at 17:01 , Mark Brown <broonie@kernel.org> wrote: > > On Mon, Oct 27, 2014 at 01:48:02PM +0200, Pantelis Antoniou wrote: >>> On Oct 24, 2014, at 02:51 , atull@opensource.altera.com wrote: > >>> + - init-val : 0 if driver should disable bridge at startup >>> + 1 if driver should enable bridge at startup >>> + driver leaves bridge in current state if property not >>> + specified. > >> Isn’t init-val a boolean property? It’s not named very well. > > It's not boolean, it's tristate - turn on, turn off or don't touch. > I see. Even then ‘init-val’ is cryptic. I’d prefer two booleans, enable-at-startup; disable-at-startup. >> Along with the label, is kinda hard to defend as configuration in DT. > > Yeah... presumably this decision would fall out of the users? Well, it’s the user that should make the decision, but the driver should pick it up. This works but it’s not very nice. Regards — Pantelis -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi! On Mon, Oct 27, 2014 at 01:54:20PM +0200, Pantelis Antoniou wrote: > Hi Stefen, > > > On Oct 25, 2014, at 17:42 , Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: > > > > Hi Pantelis! > > > > On Fri, Oct 24, 2014 at 12:20:53PM +0300, Pantelis Antoniou wrote: > >> Hi Stefan, Allan, > >> > >> Sorry, haven’t had a chance to review all this yet; will do so in the weekend. > >> Just wanted to pop in and comment on a few things. > >> > >>> On Oct 24, 2014, at 10:00 AM, Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: > >>> > >>> Hi! > >>> > >>> On Thu, Oct 23, 2014 at 06:51:06PM -0500, atull@opensource.altera.com wrote: > >>>> From: Alan Tull <atull@opensource.altera.com> > >>> > >>> (...) > >>> > >>>> diff --git a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > >>>> new file mode 100644 > >>>> index 0000000..bc24a2e > >>>> --- /dev/null > >>>> +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > >>>> @@ -0,0 +1,53 @@ > >>>> +Altera FPGA/HPS Bridge Driver > >>>> + > >>>> +This driver manages a bridge between a FPGA and a host processor system (HPS). > >>>> +User space can enable or disable the bridge by writing a "1" or a "0", > >>>> +respectively, to its enable file under bridge's entry in > >>>> +/sys/class/fpga-bridge. Typically, one disables the bridges before > >>>> +reprogramming the FPGA. Once the FPGA is reprogrammed, the bridges are > >>>> +reenabled. > >>>> + > >>> > >>> NAK. > >>> > >>> This is all linux specific and doesn't belong here. > >>> > >> > >> We know. The DT spec hasn’t been updated for a while, and this is going to be > >> part of what we want to do with the restarting of the ePAPR spec process. > >> > > > > As we don't have a new spec yet, I go with the current state of the art. > > And I don't see how "file under ... /sys/class/fpga-bridge" fits the > > current spec. > > > >> So absolutes like “DT is a hardware description only" might be too strong statements, that > >> do not work in practice anymore. > >> > >> There’s no such thing as pure hardware devices lately - there is always a configuration > >> component; some of it OS specific, some of it not. > >> > > > > If it really is necessary, I'm not against it, don't get me wrong. > > But in the grand scheme I wouldn't say that this fits my experience. > > There are some devices that need configuration and often when it is > > done in the DT, it is just a shortcut or not thought through. > > And can be also introspected dynamically. With some discussion on the > > list these bindings often change. > > > > Already answered and given a possible way this might work; admittedly this > specific example is not a good fit for what I was trying to say, but whatever. > Let’s get the ball rolling. > I would be okay with chosen-node. Not for this driver; but in general. > > Regarding OS specifics: already there, e.g. console setup in the chosen node. > > And other things I saw are ethernet aliases just for u-boot. Not a problem > > of the spec, just a problem of u-boot and unneccessary "configuration". > > Just to fix a bad bootloader. barebox can handle this without any > > such stuff. Maybe we should integrate the DT "overlays" barebox uses into > > the in-kernel DTs as well...but does it really help/interest someone who > > decides to use u-boot where barebox stores its environment? I guess not. > > > > Although I’m not against having DT overlays in the bootloader, I only see them > as a method that helps a platform developer express things easier. I am completely > against having the kernel tied to any particular bootloader. > > We’ve all been through the hell where a kernel only boots if the bootloader has > setup the platform “correctly”. This “correctly” has a very loose definition and > 99/100 times is extremely badly documented or not at all. > > IMHO the bootloader should (as part of the normal boot sequence) only setup the > absolutely bare minimum and then pass control to the kernel as soon as possible. > > The full featured bootloader environment should only be presented when the user > requests so. > Totally agree. The kernel shouldn't be tied to any bootloader if at all possible (the SoCFPGA is an especially bad example here again, as the pinmuxing can only happen in the bootloader). But should the DT include stuff than, that is only interesting for linux? > >> We have to be engaged in the process and figure out how to update the spec for what is > >> now the state of the art in the industry. > >> > > > > Again, not against that if it is necessary. For example your overlay stuff might > > be a good update to the spec. Would be better if it is official instead of a "hack". > > I want that for SoCFPGA. > > > > But having looked at one or two vendor kernels+DTs, the state of the art in the > > industry is: using DT wrong. (And doing HW wrong, which makes some updates to the > > ePAPR necessary ;-)) > > > > Vendor H/W, vendor DT and a crack pipe. Or as I call it Monday. > ;-) Regards, Steffen
On Mon, Oct 27, 2014 at 05:05:29PM +0200, Pantelis Antoniou wrote: > Hi Mark, > > > On Oct 27, 2014, at 17:01 , Mark Brown <broonie@kernel.org> wrote: > > > > On Mon, Oct 27, 2014 at 01:48:02PM +0200, Pantelis Antoniou wrote: > >>> On Oct 24, 2014, at 02:51 , atull@opensource.altera.com wrote: > > > >>> + - init-val : 0 if driver should disable bridge at startup > >>> + 1 if driver should enable bridge at startup > >>> + driver leaves bridge in current state if property not > >>> + specified. > > > >> Isn’t init-val a boolean property? It’s not named very well. > > > > It's not boolean, it's tristate - turn on, turn off or don't touch. > > > > I see. Even then ‘init-val’ is cryptic. I’d prefer two booleans, > enable-at-startup; disable-at-startup. > > >> Along with the label, is kinda hard to defend as configuration in DT. > > > > Yeah... presumably this decision would fall out of the users? > > Well, it’s the user that should make the decision, but the driver should > pick it up. This works but it’s not very nice. > Hm, convince me why this AXI bus is so special, that I even need an "init-val" property? Other buses don't have that. Why don't I add a property "init-val" to my SPI buses, so I can enable it in the DT and still have it in reset, just because.... The bridges on the SoCFPGA are buses, from the HPS to the FPGA. If I have written firmware to the FPGA and I have subnodes on that bus, I have to get it out of reset and probe everything. Normal procedure, no ?! Regards, Steffen
Hi Stefan, > On Oct 27, 2014, at 17:32 , Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: > > On Mon, Oct 27, 2014 at 05:05:29PM +0200, Pantelis Antoniou wrote: >> Hi Mark, >> >>> On Oct 27, 2014, at 17:01 , Mark Brown <broonie@kernel.org> wrote: >>> >>> On Mon, Oct 27, 2014 at 01:48:02PM +0200, Pantelis Antoniou wrote: >>>>> On Oct 24, 2014, at 02:51 , atull@opensource.altera.com wrote: >>> >>>>> + - init-val : 0 if driver should disable bridge at startup >>>>> + 1 if driver should enable bridge at startup >>>>> + driver leaves bridge in current state if property not >>>>> + specified. >>> >>>> Isn’t init-val a boolean property? It’s not named very well. >>> >>> It's not boolean, it's tristate - turn on, turn off or don't touch. >>> >> >> I see. Even then ‘init-val’ is cryptic. I’d prefer two booleans, >> enable-at-startup; disable-at-startup. >> >>>> Along with the label, is kinda hard to defend as configuration in DT. >>> >>> Yeah... presumably this decision would fall out of the users? >> >> Well, it’s the user that should make the decision, but the driver should >> pick it up. This works but it’s not very nice. >> > > Hm, convince me why this AXI bus is so special, that I even need an > "init-val" property? Other buses don't have that. > Why don't I add a property "init-val" to my SPI buses, so I can enable > it in the DT and still have it in reset, just because.... > > The bridges on the SoCFPGA are buses, from the HPS to the FPGA. If I have > written firmware to the FPGA and I have subnodes on that bus, I have to > get it out of reset and probe everything. Normal procedure, no ?! > Well, it’s not my speciality, but my understanding is that FPGAs take (considerable) time to be programmed. If someone has already configured the ‘bus’ it is considered a win to not reload the bitstream. I.e. if you boot with the bootloader having loaded the bitstream already, you don’t want to do it again. I’m afraid there’s no such analogue with standard hardware busses like SPI, where the bus setup time is instantaneous. > > Regards, > Steffen > Regards — Pantelis > -- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Steffen, > On Oct 27, 2014, at 17:23 , Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: > > Hi! > > On Mon, Oct 27, 2014 at 01:54:20PM +0200, Pantelis Antoniou wrote: >> Hi Stefen, >> >>> On Oct 25, 2014, at 17:42 , Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: >>> >>> Hi Pantelis! >>> >>> On Fri, Oct 24, 2014 at 12:20:53PM +0300, Pantelis Antoniou wrote: >>>> Hi Stefan, Allan, >>>> >>>> Sorry, haven’t had a chance to review all this yet; will do so in the weekend. >>>> Just wanted to pop in and comment on a few things. >>>> >>>>> On Oct 24, 2014, at 10:00 AM, Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: >>>>> >>>>> Hi! >>>>> >>>>> On Thu, Oct 23, 2014 at 06:51:06PM -0500, atull@opensource.altera.com wrote: >>>>>> From: Alan Tull <atull@opensource.altera.com> >>>>> >>>>> (...) >>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt >>>>>> new file mode 100644 >>>>>> index 0000000..bc24a2e >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt >>>>>> @@ -0,0 +1,53 @@ >>>>>> +Altera FPGA/HPS Bridge Driver >>>>>> + >>>>>> +This driver manages a bridge between a FPGA and a host processor system (HPS). >>>>>> +User space can enable or disable the bridge by writing a "1" or a "0", >>>>>> +respectively, to its enable file under bridge's entry in >>>>>> +/sys/class/fpga-bridge. Typically, one disables the bridges before >>>>>> +reprogramming the FPGA. Once the FPGA is reprogrammed, the bridges are >>>>>> +reenabled. >>>>>> + >>>>> >>>>> NAK. >>>>> >>>>> This is all linux specific and doesn't belong here. >>>>> >>>> >>>> We know. The DT spec hasn’t been updated for a while, and this is going to be >>>> part of what we want to do with the restarting of the ePAPR spec process. >>>> >>> >>> As we don't have a new spec yet, I go with the current state of the art. >>> And I don't see how "file under ... /sys/class/fpga-bridge" fits the >>> current spec. >>> >>>> So absolutes like “DT is a hardware description only" might be too strong statements, that >>>> do not work in practice anymore. >>>> >>>> There’s no such thing as pure hardware devices lately - there is always a configuration >>>> component; some of it OS specific, some of it not. >>>> >>> >>> If it really is necessary, I'm not against it, don't get me wrong. >>> But in the grand scheme I wouldn't say that this fits my experience. >>> There are some devices that need configuration and often when it is >>> done in the DT, it is just a shortcut or not thought through. >>> And can be also introspected dynamically. With some discussion on the >>> list these bindings often change. >>> >> >> Already answered and given a possible way this might work; admittedly this >> specific example is not a good fit for what I was trying to say, but whatever. >> Let’s get the ball rolling. >> > > I would be okay with chosen-node. Not for this driver; but in general. > Let’s figure out how to do it then… >>> Regarding OS specifics: already there, e.g. console setup in the chosen node. >>> And other things I saw are ethernet aliases just for u-boot. Not a problem >>> of the spec, just a problem of u-boot and unneccessary "configuration". >>> Just to fix a bad bootloader. barebox can handle this without any >>> such stuff. Maybe we should integrate the DT "overlays" barebox uses into >>> the in-kernel DTs as well...but does it really help/interest someone who >>> decides to use u-boot where barebox stores its environment? I guess not. >>> >> >> Although I’m not against having DT overlays in the bootloader, I only see them >> as a method that helps a platform developer express things easier. I am completely >> against having the kernel tied to any particular bootloader. >> >> We’ve all been through the hell where a kernel only boots if the bootloader has >> setup the platform “correctly”. This “correctly” has a very loose definition and >> 99/100 times is extremely badly documented or not at all. >> >> IMHO the bootloader should (as part of the normal boot sequence) only setup the >> absolutely bare minimum and then pass control to the kernel as soon as possible. >> >> The full featured bootloader environment should only be presented when the user >> requests so. >> > > Totally agree. The kernel shouldn't be tied to any bootloader if at all possible > (the SoCFPGA is an especially bad example here again, as the pinmuxing can only > happen in the bootloader). I’m not familiar with SoCFPGA. Why pinmuxing is only possible in the bootloader? Can’t the bootloader configure the minimum pinmux config and then have Linux setup the rest? If we’re feeling particularly nasty, we might just require bootloaders to clean the hardware state before passing control to Linux (besides memory controller setup) and see what’s broken. > But should the DT include stuff than, that is only interesting for linux? > Like it or not it’s Linux that’s in the forefront of many hardware designs. There are configuration settings in DT that are part of the way hardware is presented to Linux and user-space, and have been for quite some time. We’ve been pretending they don’t exist, but they are there… We have a chance to try to get it right, so why not do so? >>>> We have to be engaged in the process and figure out how to update the spec for what is >>>> now the state of the art in the industry. >>>> >>> >>> Again, not against that if it is necessary. For example your overlay stuff might >>> be a good update to the spec. Would be better if it is official instead of a "hack". >>> I want that for SoCFPGA. >>> >>> But having looked at one or two vendor kernels+DTs, the state of the art in the >>> industry is: using DT wrong. (And doing HW wrong, which makes some updates to the >>> ePAPR necessary ;-)) >>> >> >> Vendor H/W, vendor DT and a crack pipe. Or as I call it Monday. >> > > ;-) > > Regards, > Steffen > Regards — Pantelis > -- > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 27, 2014 at 05:45:03PM +0200, Pantelis Antoniou wrote: Please fix your mail client to word wrap at less than 80 columns. > Well, it’s not my speciality, but my understanding is that FPGAs take (considerable) > time to be programmed. If someone has already configured the ‘bus’ it is considered > a win to not reload the bitstream. I.e. if you boot with the bootloader having loaded > the bitstream already, you don’t want to do it again. That's not what your property is saying, though - if it were just about handover between the bootloader and the kernel that'd be one thing but it's also got this additional possibility to instruct the kernel that the device must be either disabled or explicitly programmed. Having a property that just says "this FPGA is programmed" covers the handover case but this property does more than that.
Hi Mark, > On Oct 27, 2014, at 19:17 , Mark Brown <broonie@kernel.org> wrote: > > On Mon, Oct 27, 2014 at 05:45:03PM +0200, Pantelis Antoniou wrote: > > Please fix your mail client to word wrap at less than 80 columns. > >> Well, it’s not my speciality, but my understanding is that FPGAs take (considerable) >> time to be programmed. If someone has already configured the ‘bus’ it is considered >> a win to not reload the bitstream. I.e. if you boot with the bootloader having loaded >> the bitstream already, you don’t want to do it again. > > That's not what your property is saying, though - if it were just about > handover between the bootloader and the kernel that'd be one thing but > it's also got this additional possibility to instruct the kernel that > the device must be either disabled or explicitly programmed. Having a > property that just says "this FPGA is programmed" covers the handover > case but this property does more than that. It’s not my property. This is not my driver. The property is badly-named as it is IMHO. Why don’t we let Alan chime in? Regards — Pantelis -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 27, 2014 at 05:52:02PM +0200, Pantelis Antoniou wrote: > Hi Steffen, > > > On Oct 27, 2014, at 17:23 , Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: > > > > Hi! > > > > On Mon, Oct 27, 2014 at 01:54:20PM +0200, Pantelis Antoniou wrote: > >> Hi Stefen, > >> > >>> On Oct 25, 2014, at 17:42 , Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: > >>> > >>> Hi Pantelis! > >>> > >>> On Fri, Oct 24, 2014 at 12:20:53PM +0300, Pantelis Antoniou wrote: > >>>> Hi Stefan, Allan, > >>>> > >>>> Sorry, haven’t had a chance to review all this yet; will do so in the weekend. > >>>> Just wanted to pop in and comment on a few things. > >>>> > >>>>> On Oct 24, 2014, at 10:00 AM, Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: > >>>>> > >>>>> Hi! > >>>>> > >>>>> On Thu, Oct 23, 2014 at 06:51:06PM -0500, atull@opensource.altera.com wrote: > >>>>>> From: Alan Tull <atull@opensource.altera.com> > >>>>> > >>>>> (...) > >>>>> > >>>>>> diff --git a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > >>>>>> new file mode 100644 > >>>>>> index 0000000..bc24a2e > >>>>>> --- /dev/null > >>>>>> +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > >>>>>> @@ -0,0 +1,53 @@ > >>>>>> +Altera FPGA/HPS Bridge Driver > >>>>>> + > >>>>>> +This driver manages a bridge between a FPGA and a host processor system (HPS). > >>>>>> +User space can enable or disable the bridge by writing a "1" or a "0", > >>>>>> +respectively, to its enable file under bridge's entry in > >>>>>> +/sys/class/fpga-bridge. Typically, one disables the bridges before > >>>>>> +reprogramming the FPGA. Once the FPGA is reprogrammed, the bridges are > >>>>>> +reenabled. > >>>>>> + > >>>>> > >>>>> NAK. > >>>>> > >>>>> This is all linux specific and doesn't belong here. > >>>>> > >>>> > >>>> We know. The DT spec hasn’t been updated for a while, and this is going to be > >>>> part of what we want to do with the restarting of the ePAPR spec process. > >>>> > >>> > >>> As we don't have a new spec yet, I go with the current state of the art. > >>> And I don't see how "file under ... /sys/class/fpga-bridge" fits the > >>> current spec. > >>> > >>>> So absolutes like “DT is a hardware description only" might be too strong statements, that > >>>> do not work in practice anymore. > >>>> > >>>> There’s no such thing as pure hardware devices lately - there is always a configuration > >>>> component; some of it OS specific, some of it not. > >>>> > >>> > >>> If it really is necessary, I'm not against it, don't get me wrong. > >>> But in the grand scheme I wouldn't say that this fits my experience. > >>> There are some devices that need configuration and often when it is > >>> done in the DT, it is just a shortcut or not thought through. > >>> And can be also introspected dynamically. With some discussion on the > >>> list these bindings often change. > >>> > >> > >> Already answered and given a possible way this might work; admittedly this > >> specific example is not a good fit for what I was trying to say, but whatever. > >> Let’s get the ball rolling. > >> > > > > I would be okay with chosen-node. Not for this driver; but in general. > > > > Let’s figure out how to do it then… > > >>> Regarding OS specifics: already there, e.g. console setup in the chosen node. > >>> And other things I saw are ethernet aliases just for u-boot. Not a problem > >>> of the spec, just a problem of u-boot and unneccessary "configuration". > >>> Just to fix a bad bootloader. barebox can handle this without any > >>> such stuff. Maybe we should integrate the DT "overlays" barebox uses into > >>> the in-kernel DTs as well...but does it really help/interest someone who > >>> decides to use u-boot where barebox stores its environment? I guess not. > >>> > >> > >> Although I’m not against having DT overlays in the bootloader, I only see them > >> as a method that helps a platform developer express things easier. I am completely > >> against having the kernel tied to any particular bootloader. > >> > >> We’ve all been through the hell where a kernel only boots if the bootloader has > >> setup the platform “correctly”. This “correctly” has a very loose definition and > >> 99/100 times is extremely badly documented or not at all. > >> > >> IMHO the bootloader should (as part of the normal boot sequence) only setup the > >> absolutely bare minimum and then pass control to the kernel as soon as possible. > >> > >> The full featured bootloader environment should only be presented when the user > >> requests so. > >> > > > > Totally agree. The kernel shouldn't be tied to any bootloader if at all possible > > (the SoCFPGA is an especially bad example here again, as the pinmuxing can only > > happen in the bootloader). > > I’m not familiar with SoCFPGA. Why pinmuxing is only possible in the bootloader? > Can’t the bootloader configure the minimum pinmux config and then have Linux setup > the rest? > > If we’re feeling particularly nasty, we might just require bootloaders to clean the > hardware state before passing control to Linux (besides memory controller setup) and > see what’s broken. > Okay, offtopic, but: a) datasheet explicitely states that dynamic pinmuxing is not supported. b) last time I checked, no documentation at all. You get some magic values spit out by Altera's tools. c) you don't have registers. Pinmuxing is done via JTAG scan chain; which you feed the magic values. Apart from that, it seems to be the most sane idea, to at least write your driver in a way, that doesn't make any assumptions on HW state. But I think this is already common practice in mainline. > > But should the DT include stuff than, that is only interesting for linux? > > > > Like it or not it’s Linux that’s in the forefront of many hardware designs. > I like it; pays my rent ;-) But I also like freedom to choose... > There are configuration settings in DT that are part of the way hardware is presented to > Linux and user-space, and have been for quite some time. We’ve been pretending they don’t > exist, but they are there… > As far as I know, a lot of that are older bindings, where more things just got applied, because it wasn't clear where DT will go. Regards, Steffen
On Mon, Oct 27, 2014 at 05:45:03PM +0200, Pantelis Antoniou wrote: > Hi Stefan, > > > On Oct 27, 2014, at 17:32 , Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: > > > > On Mon, Oct 27, 2014 at 05:05:29PM +0200, Pantelis Antoniou wrote: > >> Hi Mark, > >> > >>> On Oct 27, 2014, at 17:01 , Mark Brown <broonie@kernel.org> wrote: > >>> > >>> On Mon, Oct 27, 2014 at 01:48:02PM +0200, Pantelis Antoniou wrote: > >>>>> On Oct 24, 2014, at 02:51 , atull@opensource.altera.com wrote: > >>> > >>>>> + - init-val : 0 if driver should disable bridge at startup > >>>>> + 1 if driver should enable bridge at startup > >>>>> + driver leaves bridge in current state if property not > >>>>> + specified. > >>> > >>>> Isn’t init-val a boolean property? It’s not named very well. > >>> > >>> It's not boolean, it's tristate - turn on, turn off or don't touch. > >>> > >> > >> I see. Even then ‘init-val’ is cryptic. I’d prefer two booleans, > >> enable-at-startup; disable-at-startup. > >> > >>>> Along with the label, is kinda hard to defend as configuration in DT. > >>> > >>> Yeah... presumably this decision would fall out of the users? > >> > >> Well, it’s the user that should make the decision, but the driver should > >> pick it up. This works but it’s not very nice. > >> > > > > Hm, convince me why this AXI bus is so special, that I even need an > > "init-val" property? Other buses don't have that. > > Why don't I add a property "init-val" to my SPI buses, so I can enable > > it in the DT and still have it in reset, just because.... > > > > The bridges on the SoCFPGA are buses, from the HPS to the FPGA. If I have > > written firmware to the FPGA and I have subnodes on that bus, I have to > > get it out of reset and probe everything. Normal procedure, no ?! > > > > Well, it’s not my speciality, but my understanding is that FPGAs take (considerable) > time to be programmed. If someone has already configured the ‘bus’ it is considered > a win to not reload the bitstream. I.e. if you boot with the bootloader having loaded > the bitstream already, you don’t want to do it again. > > I’m afraid there’s no such analogue with standard hardware busses like SPI, where > the bus setup time is instantaneous. Ah, okay. I see why you got confused. The bridges are not in any way responsible for loading the FPGA nor will resetting them reset the bitstream. The FPGA Manager, a different IP core, is responsible for that. And AFAIK it has a status bit, that tells you if the FPGA is programmed or not. Loading the bitstream is in the order of milliseconds. So from the bridges point of view, when you probe it, you can ask the FPGA manager if is done, otherwise -EPROBE_DEFER and than go on and probe the subdevices. If you are bored, take a look at http://www.altera.com/literature/hb/cyclone-v/cv_54004.pdf page 7-2. There you can see the hardware setup. Regards, Steffen
Hi Steffen, > On Oct 27, 2014, at 20:00 , Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: > > On Mon, Oct 27, 2014 at 05:45:03PM +0200, Pantelis Antoniou wrote: >> Hi Stefan, >> >>> On Oct 27, 2014, at 17:32 , Steffen Trumtrar <s.trumtrar@pengutronix.de> wrote: >>> >>> On Mon, Oct 27, 2014 at 05:05:29PM +0200, Pantelis Antoniou wrote: >>>> Hi Mark, >>>> >>>>> On Oct 27, 2014, at 17:01 , Mark Brown <broonie@kernel.org> wrote: >>>>> >>>>> On Mon, Oct 27, 2014 at 01:48:02PM +0200, Pantelis Antoniou wrote: >>>>>>> On Oct 24, 2014, at 02:51 , atull@opensource.altera.com wrote: >>>>> >>>>>>> + - init-val : 0 if driver should disable bridge at startup >>>>>>> + 1 if driver should enable bridge at startup >>>>>>> + driver leaves bridge in current state if property not >>>>>>> + specified. >>>>> >>>>>> Isn’t init-val a boolean property? It’s not named very well. >>>>> >>>>> It's not boolean, it's tristate - turn on, turn off or don't touch. >>>>> >>>> >>>> I see. Even then ‘init-val’ is cryptic. I’d prefer two booleans, >>>> enable-at-startup; disable-at-startup. >>>> >>>>>> Along with the label, is kinda hard to defend as configuration in DT. >>>>> >>>>> Yeah... presumably this decision would fall out of the users? >>>> >>>> Well, it’s the user that should make the decision, but the driver should >>>> pick it up. This works but it’s not very nice. >>>> >>> >>> Hm, convince me why this AXI bus is so special, that I even need an >>> "init-val" property? Other buses don't have that. >>> Why don't I add a property "init-val" to my SPI buses, so I can enable >>> it in the DT and still have it in reset, just because.... >>> >>> The bridges on the SoCFPGA are buses, from the HPS to the FPGA. If I have >>> written firmware to the FPGA and I have subnodes on that bus, I have to >>> get it out of reset and probe everything. Normal procedure, no ?! >>> >> >> Well, it’s not my speciality, but my understanding is that FPGAs take (considerable) >> time to be programmed. If someone has already configured the ‘bus’ it is considered >> a win to not reload the bitstream. I.e. if you boot with the bootloader having loaded >> the bitstream already, you don’t want to do it again. >> >> I’m afraid there’s no such analogue with standard hardware busses like SPI, where >> the bus setup time is instantaneous. > > Ah, okay. I see why you got confused. > The bridges are not in any way responsible for loading the FPGA nor will > resetting them reset the bitstream. > > The FPGA Manager, a different IP core, is responsible for that. And AFAIK > it has a status bit, that tells you if the FPGA is programmed or not. > Loading the bitstream is in the order of milliseconds. > > So from the bridges point of view, when you probe it, you can ask the > FPGA manager if is done, otherwise -EPROBE_DEFER and than go on and > probe the subdevices. > > Ohh, I see. Sorry for the confusion. > If you are bored, take a look at > http://www.altera.com/literature/hb/cyclone-v/cv_54004.pdf > page 7-2. There you can see the hardware setup. > Thanks. /me throws it at the pile of things to read… > 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 | Regards — Pantelis -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 24 Oct 2014, Steffen Trumtrar wrote: > Hi! > Hi, I see that my documentation sucks and needs cleanup. I'll try to answer some of the flames and get a more coherent version out soon. > On Thu, Oct 23, 2014 at 06:51:06PM -0500, atull@opensource.altera.com wrote: > > From: Alan Tull <atull@opensource.altera.com> > > (...) > > > diff --git a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > > new file mode 100644 > > index 0000000..bc24a2e > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > > @@ -0,0 +1,53 @@ > > +Altera FPGA/HPS Bridge Driver > > + > > +This driver manages a bridge between a FPGA and a host processor system (HPS). > > +User space can enable or disable the bridge by writing a "1" or a "0", > > +respectively, to its enable file under bridge's entry in > > +/sys/class/fpga-bridge. Typically, one disables the bridges before > > +reprogramming the FPGA. Once the FPGA is reprogrammed, the bridges are > > +reenabled. > > + > > NAK. > > This is all linux specific and doesn't belong here. Right. This stuff shouldn't be in this document. While I was squashing patches and cleaning up for posting on the mailing list, I added a sysfs document and forgot to clean up my DT bindings documents. > > > +Required properties: > > + > > + - compatible : should contain one of: > > + "altr,socfpga-hps2fpga-bridge" > > + "altr,socfpga-lwhps2fpga-bridge" > > + "altr,socfpga-fpga2hps-bridge" > > + > > + - clocks : clocks used by this module > > + > > + - altr,l3-syscon : phandle of the l3 interconnect module > > + > > L3 shouldn't be a syscon. L3 is actually a good candidate for syscon. Lots of registers, each one affects a different hardware block. > Have you tried dumping the regmap in the debugfs if L3 > is a syscon? Doesn't work. Is that a bug in regmap or is that because the register in L3 that I am actully interested in here is write-only (ugh)? > > > +Optional properties: > > + - label : name that you want this bridge to show up as under > > + /sys/class/fpga-bridge. Default is br<device#> if this is > > + not specified. > > + > > Why? Linux-specific. That was a convience for the user. I can take that out and won't miss it. > > > + - init-val : 0 if driver should disable bridge at startup > > + 1 if driver should enable bridge at startup > > + driver leaves bridge in current state if property not > > + specified. > > + > > Configuration in the DT? Really? > > > +Example: > > + hps_fpgabridge0: fpgabridge@0 { > > + compatible = "altr,socfpga-hps2fpga-bridge"; > > + label = "hps2fpga"; > > + altr,l3-syscon = <&l3regs>; > > + clocks = <&l4_main_clk>; > > + init-val = <1>; > > + }; > > + > > + hps_fpgabridge1: fpgabridge@1 { > > + compatible = "altr,socfpga-lwhps2fpga-bridge"; > > + label = "lwhps2fpga"; > > + altr,l3-syscon = <&l3regs>; > > + clocks = <&l4_main_clk>; > > + init-val = <0>; > > + }; > > + > > + hps_fpgabridge2: fpgabridge@2 { > > + compatible = "altr,socfpga-fpga2hps-bridge"; > > + label = "fpga2hps"; > > + altr,l3-syscon = <&l3regs>; > > + clocks = <&l4_main_clk>; > > + }; > > The bridges are the buses into the FPGA. This has to be accomodated. > The bridges have two specified memory ranges: one the address space > of the bus, the second the register space for configuration. > > This binding does NOT correctly describe the hardware. Sorry. > OK that was outdated. More cleanup needed. How about this type of binding? Here's an actual use case for something that has multiple pieces of soft IP in the FPGA (sysid, gpio). I eliminated a few. *snippet of DT* sopc@0 { device_type = "soc"; ranges; #address-cells = <0x1>; #size-cells = <0x1>; compatible = "ALTR,avalon", "simple-bus"; bus-frequency = <0x0>; bridge@0xc0000000 { compatible = "altr,bridge-14.0", "simple-bus"; reg = <0xc0000000 0x20000000 0xff200000 0x200000>; reg-names = "axi_h2f", "axi_h2f_lw"; clocks = <0x2 0x2 0x2>; clock-names = "h2f_axi_clock", "h2f_lw_axi_clock", "f2h_sdram0_clock"; #address-cells = <0x2>; #size-cells = <0x1>; ranges = <0x0 0x0 0xc0000000 0x10000 0x1 0x10000 0xff210000 0x8 0x1 0x10040 0xff210040 0x20 0x1 0x10080 0xff210080 0x10 0x1 0x100c0 0xff2100c0 0x10 0x1 0x20000 0xff220000 0x8>; sysid@0x100010000 { compatible = "altr,sysid-14.0", "altr,sysid-1.0"; reg = <0x1 0x10000 0x8>; clocks = <0x2>; id = <0xacd51402>; timestamp = <0x540a048e>; }; gpio@0x100010040 { compatible = "altr,pio-14.0", "altr,pio-1.0"; reg = <0x1 0x10040 0x20>; clocks = <0x2>; altr,gpio-bank-width = <0x4>; resetvalue = <0x0>; #gpio-cells = <0x2>; gpio-controller; linux,phandle = <0x2a>; }; /* other hardware that exists on FPGA here */ }; }; Alan > 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 linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 28 Oct 2014, atull wrote: > On Fri, 24 Oct 2014, Steffen Trumtrar wrote: > > > > > + - init-val : 0 if driver should disable bridge at startup > > > + 1 if driver should enable bridge at startup > > > + driver leaves bridge in current state if property not > > > + specified. > > > + > > > > Configuration in the DT? Really? The FPGA can be programmed by several different methods (eeprom, jtag, preloader, Linux). The guiding principle is: whoever programmed the FPGA should release the bridge. At one point, we thought 'init-val' would be needed, but I think it can be taken out and we won't miss it. Alan -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi! On Tue, Oct 28, 2014 at 04:19:03PM -0500, atull wrote: > On Fri, 24 Oct 2014, Steffen Trumtrar wrote: > > > Hi! > > > > Hi, > > I see that my documentation sucks and needs cleanup. I'll try to > answer some of the flames and get a more coherent version out soon. > > > On Thu, Oct 23, 2014 at 06:51:06PM -0500, atull@opensource.altera.com wrote: > > > From: Alan Tull <atull@opensource.altera.com> > > > > (...) > > > > > diff --git a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > > > new file mode 100644 > > > index 0000000..bc24a2e > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > > > @@ -0,0 +1,53 @@ > > > +Altera FPGA/HPS Bridge Driver > > > + > > > +This driver manages a bridge between a FPGA and a host processor system (HPS). > > > +User space can enable or disable the bridge by writing a "1" or a "0", > > > +respectively, to its enable file under bridge's entry in > > > +/sys/class/fpga-bridge. Typically, one disables the bridges before > > > +reprogramming the FPGA. Once the FPGA is reprogrammed, the bridges are > > > +reenabled. > > > + > > > > NAK. > > > > This is all linux specific and doesn't belong here. > > Right. This stuff shouldn't be in this document. While I was squashing > patches and cleaning up for posting on the mailing list, I added a sysfs > document and forgot to clean up my DT bindings documents. > > > > > > +Required properties: > > > + > > > + - compatible : should contain one of: > > > + "altr,socfpga-hps2fpga-bridge" > > > + "altr,socfpga-lwhps2fpga-bridge" > > > + "altr,socfpga-fpga2hps-bridge" > > > + > > > + - clocks : clocks used by this module > > > + > > > + - altr,l3-syscon : phandle of the l3 interconnect module > > > + > > > > L3 shouldn't be a syscon. > > L3 is actually a good candidate for syscon. Lots of registers, each one > affects a different hardware block. > > > Have you tried dumping the regmap in the debugfs if L3 > > is a syscon? Doesn't work. > > Is that a bug in regmap or is that because the register in L3 that > I am actully interested in here is write-only (ugh)? > That is not a bug in regmap. It is because you only have a register at 0x4008, than one at 0x4104, that one at 0x5008, etc.... (values from memory, so might be wrong) Of course regmap tries to read from the whole register range if specified as syscon. Which it can't. So, that shouldn't be a problem though, as I already cooked up a driver for the L3 with all the ranges specified. The only thing I need to figure out before I will post it, is how to nicely handle the WO remap register. I think regmap might be able to handle this itself, but am not sure yet. > > > > > +Optional properties: > > > + - label : name that you want this bridge to show up as under > > > + /sys/class/fpga-bridge. Default is br<device#> if this is > > > + not specified. > > > + > > > > Why? Linux-specific. > > That was a convience for the user. I can take that out and won't miss it. > > > > > > + - init-val : 0 if driver should disable bridge at startup > > > + 1 if driver should enable bridge at startup > > > + driver leaves bridge in current state if property not > > > + specified. > > > + > > > > Configuration in the DT? Really? > > > > > +Example: > > > + hps_fpgabridge0: fpgabridge@0 { > > > + compatible = "altr,socfpga-hps2fpga-bridge"; > > > + label = "hps2fpga"; > > > + altr,l3-syscon = <&l3regs>; > > > + clocks = <&l4_main_clk>; > > > + init-val = <1>; > > > + }; > > > + > > > + hps_fpgabridge1: fpgabridge@1 { > > > + compatible = "altr,socfpga-lwhps2fpga-bridge"; > > > + label = "lwhps2fpga"; > > > + altr,l3-syscon = <&l3regs>; > > > + clocks = <&l4_main_clk>; > > > + init-val = <0>; > > > + }; > > > + > > > + hps_fpgabridge2: fpgabridge@2 { > > > + compatible = "altr,socfpga-fpga2hps-bridge"; > > > + label = "fpga2hps"; > > > + altr,l3-syscon = <&l3regs>; > > > + clocks = <&l4_main_clk>; > > > + }; > > > > The bridges are the buses into the FPGA. This has to be accomodated. > > The bridges have two specified memory ranges: one the address space > > of the bus, the second the register space for configuration. > > > > This binding does NOT correctly describe the hardware. Sorry. > > > > OK that was outdated. More cleanup needed. How about this type of > binding? Here's an actual use case for something that has multiple > pieces of soft IP in the FPGA (sysid, gpio). I eliminated a few. > > *snippet of DT* > sopc@0 { > device_type = "soc"; > ranges; > #address-cells = <0x1>; > #size-cells = <0x1>; > compatible = "ALTR,avalon", "simple-bus"; > bus-frequency = <0x0>; > > bridge@0xc0000000 { > compatible = "altr,bridge-14.0", "simple-bus"; > reg = <0xc0000000 0x20000000 0xff200000 0x200000>; > reg-names = "axi_h2f", "axi_h2f_lw"; > clocks = <0x2 0x2 0x2>; > clock-names = "h2f_axi_clock", "h2f_lw_axi_clock", > "f2h_sdram0_clock"; > #address-cells = <0x2>; > #size-cells = <0x1>; > ranges = <0x0 0x0 0xc0000000 0x10000 > 0x1 0x10000 0xff210000 0x8 > 0x1 0x10040 0xff210040 0x20 > 0x1 0x10080 0xff210080 0x10 > 0x1 0x100c0 0xff2100c0 0x10 > 0x1 0x20000 0xff220000 0x8>; > > sysid@0x100010000 { > compatible = "altr,sysid-14.0", "altr,sysid-1.0"; > reg = <0x1 0x10000 0x8>; > clocks = <0x2>; > id = <0xacd51402>; > timestamp = <0x540a048e>; > }; > > gpio@0x100010040 { > compatible = "altr,pio-14.0", "altr,pio-1.0"; > reg = <0x1 0x10040 0x20>; > clocks = <0x2>; > altr,gpio-bank-width = <0x4>; > resetvalue = <0x0>; > #gpio-cells = <0x2>; > gpio-controller; > linux,phandle = <0x2a>; > }; > > /* other hardware that exists on FPGA here */ > > }; > }; > Yes, something like this seems appropriate. Again, I also cooked up a driver for this. I need to figure out why the GPV register writing sometimes doesn't work as expected and will try to marry it to your FPGA manager framework before I consider sending it to the list. My proposed binding at the moment looks like: Example: hps2fpga: axibridge@ff500000 { #address-cells = <1>; #size-cells = <1>; compatible = "altr,hps2fpga-axi-bridge"; reg = <0xff500000 0x100000>, <0xc0000000 0x3c000000>; clocks = <&l4_mp_clk>, <&l3_main_clk>; clock-names = "gpv_clk", "data_clk"; resets = <&rst HPS2FPGA_RESET>; reset-names = "hps2fpga"; altr,gpv-master = <&lwhps2fpga>; altr,l3-gpv = <&l3regs>; bus-width = <64>; status = "disabled"; ranges; }; Board file example: &hps2fpga { bus-width = <32>; status = "okay"; axi-ip: axi-ip@c0000000 { compatible = "axi-ip"; reg = <0xc0000000 0x10000>; clocks = <&h2f_usr2_clk>; status = "okay"; }; }; So we seem to be almost on the same page. I first had the simple-bus in there, too. But this will lead to all sorts of problems regarding driver probing/removing on that bus. Regards, Steffen
Hi! > > > +Optional properties: > > > + - label : name that you want this bridge to show up as under > > > + /sys/class/fpga-bridge. Default is br<device#> if this is > > > + not specified. > > > + > > > > Why? Linux-specific. > > That was a convience for the user. I can take that out and won't > miss it. Actually make it - label : optional user-readable name for this bridge ...and it is no longer Linux-specific, and you can keep it if it is otherwise useful... Pavel
On Wed, Oct 29, 2014 at 08:57:01AM +0100, Steffen Trumtrar wrote: > So, that shouldn't be a problem though, as I already cooked up a driver for > the L3 with all the ranges specified. The only thing I need to figure out > before I will post it, is how to nicely handle the WO remap register. > I think regmap might be able to handle this itself, but am not sure yet. It's supposed to be able to, not sure if anyone really tests that or not though so it's possible bugs crept in. Most write only registers can physically be read so actually come out as volatile rather than write only even if functionally there is no reason to read.
On Wed, Oct 29, 2014 at 10:16:32AM +0000, Mark Brown wrote: > On Wed, Oct 29, 2014 at 08:57:01AM +0100, Steffen Trumtrar wrote: > > > So, that shouldn't be a problem though, as I already cooked up a driver for > > the L3 with all the ranges specified. The only thing I need to figure out > > before I will post it, is how to nicely handle the WO remap register. > > I think regmap might be able to handle this itself, but am not sure yet. > > It's supposed to be able to, not sure if anyone really tests that or not > though so it's possible bugs crept in. Most write only registers can > physically be read so actually come out as volatile rather than write > only even if functionally there is no reason to read. Ah, I think you might be actually right there. IIRC it IS possible to read from that register, but you only get 0s. So, I shot myself in the knee with specifying that register as write-only in my driver. Thanks, Steffen
On Wed, 29 Oct 2014, Steffen Trumtrar wrote: > On Wed, Oct 29, 2014 at 10:16:32AM +0000, Mark Brown wrote: > > On Wed, Oct 29, 2014 at 08:57:01AM +0100, Steffen Trumtrar wrote: > > > > > So, that shouldn't be a problem though, as I already cooked up a driver for > > > the L3 with all the ranges specified. The only thing I need to figure out > > > before I will post it, is how to nicely handle the WO remap register. > > > I think regmap might be able to handle this itself, but am not sure yet. > > > > It's supposed to be able to, not sure if anyone really tests that or not > > though so it's possible bugs crept in. Most write only registers can > > physically be read so actually come out as volatile rather than write > > only even if functionally there is no reason to read. > > Ah, I think you might be actually right there. IIRC it IS possible to read > from that register, but you only get 0s. Yes, you can read and it will be all 0s. So read-modify-write is not something you would want to do for instance. Alan > So, I shot myself in the knee with specifying that register as write-only > in my driver. > > Thanks, > 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 linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 29 Oct 2014, Pavel Machek wrote: > Hi! > > > > > +Optional properties: > > > > + - label : name that you want this bridge to show up as under > > > > + /sys/class/fpga-bridge. Default is br<device#> if this is > > > > + not specified. > > > > + > > > > > > Why? Linux-specific. > > > > That was a convience for the user. I can take that out and won't > > miss it. > > Actually make it > > - label : optional user-readable name for this bridge > > ...and it is no longer Linux-specific, and you can keep it if it is > otherwise useful... > Pavel Great! Thanks for the review. In the future I'll keep my sysfs documentation separate from my DT bindings! Alan > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 29 Oct 2014, Steffen Trumtrar wrote: > Hi! > > On Tue, Oct 28, 2014 at 04:19:03PM -0500, atull wrote: > > On Fri, 24 Oct 2014, Steffen Trumtrar wrote: > > > > > Hi! > > > > > > > Hi, > > > > I see that my documentation sucks and needs cleanup. I'll try to > > answer some of the flames and get a more coherent version out soon. > > > > > On Thu, Oct 23, 2014 at 06:51:06PM -0500, atull@opensource.altera.com wrote: > > > > From: Alan Tull <atull@opensource.altera.com> > > > > > > (...) > > > > > > > diff --git a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > > > > new file mode 100644 > > > > index 0000000..bc24a2e > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > > > > @@ -0,0 +1,53 @@ > > > > +Altera FPGA/HPS Bridge Driver > > > > + > > > > +This driver manages a bridge between a FPGA and a host processor system (HPS). > > > > +User space can enable or disable the bridge by writing a "1" or a "0", > > > > +respectively, to its enable file under bridge's entry in > > > > +/sys/class/fpga-bridge. Typically, one disables the bridges before > > > > +reprogramming the FPGA. Once the FPGA is reprogrammed, the bridges are > > > > +reenabled. > > > > + > > > > > > NAK. > > > > > > This is all linux specific and doesn't belong here. > > > > Right. This stuff shouldn't be in this document. While I was squashing > > patches and cleaning up for posting on the mailing list, I added a sysfs > > document and forgot to clean up my DT bindings documents. > > > > > > > > > +Required properties: > > > > + > > > > + - compatible : should contain one of: > > > > + "altr,socfpga-hps2fpga-bridge" > > > > + "altr,socfpga-lwhps2fpga-bridge" > > > > + "altr,socfpga-fpga2hps-bridge" > > > > + > > > > + - clocks : clocks used by this module > > > > + > > > > + - altr,l3-syscon : phandle of the l3 interconnect module > > > > + > > > > > > L3 shouldn't be a syscon. > > > > L3 is actually a good candidate for syscon. Lots of registers, each one > > affects a different hardware block. > > > > > Have you tried dumping the regmap in the debugfs if L3 > > > is a syscon? Doesn't work. > > > > Is that a bug in regmap or is that because the register in L3 that > > I am actully interested in here is write-only (ugh)? > > > > That is not a bug in regmap. It is because you only have a register at 0x4008, > than one at 0x4104, that one at 0x5008, etc.... (values from memory, so might > be wrong) > > Of course regmap tries to read from the whole register range if specified as > syscon. Which it can't. > > So, that shouldn't be a problem though, as I already cooked up a driver for > the L3 with all the ranges specified. The only thing I need to figure out > before I will post it, is how to nicely handle the WO remap register. > I think regmap might be able to handle this itself, but am not sure yet. > > > > > > > > +Optional properties: > > > > + - label : name that you want this bridge to show up as under > > > > + /sys/class/fpga-bridge. Default is br<device#> if this is > > > > + not specified. > > > > + > > > > > > Why? Linux-specific. > > > > That was a convience for the user. I can take that out and won't miss it. > > > > > > > > > + - init-val : 0 if driver should disable bridge at startup > > > > + 1 if driver should enable bridge at startup > > > > + driver leaves bridge in current state if property not > > > > + specified. > > > > + > > > > > > Configuration in the DT? Really? > > > > > > > +Example: > > > > + hps_fpgabridge0: fpgabridge@0 { > > > > + compatible = "altr,socfpga-hps2fpga-bridge"; > > > > + label = "hps2fpga"; > > > > + altr,l3-syscon = <&l3regs>; > > > > + clocks = <&l4_main_clk>; > > > > + init-val = <1>; > > > > + }; > > > > + > > > > + hps_fpgabridge1: fpgabridge@1 { > > > > + compatible = "altr,socfpga-lwhps2fpga-bridge"; > > > > + label = "lwhps2fpga"; > > > > + altr,l3-syscon = <&l3regs>; > > > > + clocks = <&l4_main_clk>; > > > > + init-val = <0>; > > > > + }; > > > > + > > > > + hps_fpgabridge2: fpgabridge@2 { > > > > + compatible = "altr,socfpga-fpga2hps-bridge"; > > > > + label = "fpga2hps"; > > > > + altr,l3-syscon = <&l3regs>; > > > > + clocks = <&l4_main_clk>; > > > > + }; > > > > > > The bridges are the buses into the FPGA. This has to be accomodated. > > > The bridges have two specified memory ranges: one the address space > > > of the bus, the second the register space for configuration. > > > > > > This binding does NOT correctly describe the hardware. Sorry. > > > > > > > OK that was outdated. More cleanup needed. How about this type of > > binding? Here's an actual use case for something that has multiple > > pieces of soft IP in the FPGA (sysid, gpio). I eliminated a few. > > > > *snippet of DT* > > sopc@0 { > > device_type = "soc"; > > ranges; > > #address-cells = <0x1>; > > #size-cells = <0x1>; > > compatible = "ALTR,avalon", "simple-bus"; > > bus-frequency = <0x0>; > > > > bridge@0xc0000000 { > > compatible = "altr,bridge-14.0", "simple-bus"; > > reg = <0xc0000000 0x20000000 0xff200000 0x200000>; > > reg-names = "axi_h2f", "axi_h2f_lw"; > > clocks = <0x2 0x2 0x2>; > > clock-names = "h2f_axi_clock", "h2f_lw_axi_clock", > > "f2h_sdram0_clock"; > > #address-cells = <0x2>; > > #size-cells = <0x1>; > > ranges = <0x0 0x0 0xc0000000 0x10000 > > 0x1 0x10000 0xff210000 0x8 > > 0x1 0x10040 0xff210040 0x20 > > 0x1 0x10080 0xff210080 0x10 > > 0x1 0x100c0 0xff2100c0 0x10 > > 0x1 0x20000 0xff220000 0x8>; > > > > sysid@0x100010000 { > > compatible = "altr,sysid-14.0", "altr,sysid-1.0"; > > reg = <0x1 0x10000 0x8>; > > clocks = <0x2>; > > id = <0xacd51402>; > > timestamp = <0x540a048e>; > > }; > > > > gpio@0x100010040 { > > compatible = "altr,pio-14.0", "altr,pio-1.0"; > > reg = <0x1 0x10040 0x20>; > > clocks = <0x2>; > > altr,gpio-bank-width = <0x4>; > > resetvalue = <0x0>; > > #gpio-cells = <0x2>; > > gpio-controller; > > linux,phandle = <0x2a>; > > }; > > > > /* other hardware that exists on FPGA here */ > > > > }; > > }; > > > > Yes, something like this seems appropriate. Again, I also cooked up a driver > for this. I need to figure out why the GPV register writing sometimes doesn't > work as expected and will try to marry it to your FPGA manager framework > before I consider sending it to the list. Hi Steffen, I look forward to seeing it. > > My proposed binding at the moment looks like: > > Example: > > hps2fpga: axibridge@ff500000 { > #address-cells = <1>; > #size-cells = <1>; > compatible = "altr,hps2fpga-axi-bridge"; > reg = <0xff500000 0x100000>, > <0xc0000000 0x3c000000>; > clocks = <&l4_mp_clk>, <&l3_main_clk>; > clock-names = "gpv_clk", "data_clk"; > resets = <&rst HPS2FPGA_RESET>; > reset-names = "hps2fpga"; > altr,gpv-master = <&lwhps2fpga>; > altr,l3-gpv = <&l3regs>; > bus-width = <64>; > status = "disabled"; > ranges; > }; > > Board file example: > > &hps2fpga { > bus-width = <32>; > status = "okay"; > > axi-ip: axi-ip@c0000000 { > compatible = "axi-ip"; > reg = <0xc0000000 0x10000>; > clocks = <&h2f_usr2_clk>; > status = "okay"; > }; > }; > > So we seem to be almost on the same page. I first had the simple-bus in > there, too. But this will lead to all sorts of problems regarding driver > probing/removing on that bus. > I think the problems we have had was that the bus had to be arch_initcall so that it would be there before the drivers were. Are there other problems you are thinking of? It would be good if we could get around that problem as otherwise it will be a problem with the l3-nic not being early enough. Alan > 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 linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt new file mode 100644 index 0000000..cc8f522 --- /dev/null +++ b/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt @@ -0,0 +1,57 @@ +Altera FPGA To SDRAM Bridge Driver + +This driver manages a bridge between an FPGA and the SDRAM used by an host +processor system (HPS). The bridge contains a number read ports, write ports, +and command ports. Reconfiguring these ports requires that no SDRAM +transactions occur during reconfiguration. In other words, the code +reconfiguring the ports cannot be run out of SDRAM nor can the FPGA access the +SDRAM during the reconfiguration. This driver does not support reconfiguring +the ports. Typcially, the ports are configured by code running out of onchip +ram before Linux is started. + +This driver supports enabling and disabling of the configured ports all at +once, which allows for safe reprogramming of the FPGA from user space, provided +the new FPGA image uses the same port configuration. User space can enable or +disable the bridge by writing a "1" or a "0", respectively, to its enable file +under bridge's entry in /sys/class/fpga-bridge. Typically, one disables the +bridges before reprogramming the FPGA. Once the FPGA is reprogrammed, the +bridges are reenabled. + +Required properties: + + - compatible : "altr,socfpga-fpga2sdram-bridge" + + - read-ports-mask : Bits 0 to 3 corresponds read ports 0 to 3. A bit set to 1 + indicates the corresponding read port should be enabled. + + - write-ports-mask : Bits 0 to 3 corresponds write ports 0 to 3. A bit set + to 1 indicates the corresponding write port should be + enabled. + + - cmd-ports-mask : Bits 0 to 5 correspond to command ports 0 to 5. A bit set + to 1 indicates the corresponding command port should be + enabled. + + - altr,sdr-syscon : phandle of the sdr module + +Optional properties: + + - label : name that you want this bridge to show up as under + /sys/class/fpga-bridge. Default is br<device#> if this is + not specified. + + - init-val : 0 if driver should disable bridge at startup + 1 if driver should enable bridge at startup + driver leaves bridge in current state if property not + specified. + +Example: + fpga2sdram_br: fpgabridge@3 { + compatible = "altr,socfpga-fpga2sdram-bridge"; + label = "fpga2sdram"; + altr,sdr-syscon = <&sdr>; + read-ports-mask = <3>; + write-ports-mask = <3>; + cmd-ports-mask = <0xd>; + init-val = <0>; + }; diff --git a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt new file mode 100644 index 0000000..bc24a2e --- /dev/null +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt @@ -0,0 +1,53 @@ +Altera FPGA/HPS Bridge Driver + +This driver manages a bridge between a FPGA and a host processor system (HPS). +User space can enable or disable the bridge by writing a "1" or a "0", +respectively, to its enable file under bridge's entry in +/sys/class/fpga-bridge. Typically, one disables the bridges before +reprogramming the FPGA. Once the FPGA is reprogrammed, the bridges are +reenabled. + +Required properties: + + - compatible : should contain one of: + "altr,socfpga-hps2fpga-bridge" + "altr,socfpga-lwhps2fpga-bridge" + "altr,socfpga-fpga2hps-bridge" + + - clocks : clocks used by this module + + - altr,l3-syscon : phandle of the l3 interconnect module + +Optional properties: + - label : name that you want this bridge to show up as under + /sys/class/fpga-bridge. Default is br<device#> if this is + not specified. + + - init-val : 0 if driver should disable bridge at startup + 1 if driver should enable bridge at startup + driver leaves bridge in current state if property not + specified. + +Example: + hps_fpgabridge0: fpgabridge@0 { + compatible = "altr,socfpga-hps2fpga-bridge"; + label = "hps2fpga"; + altr,l3-syscon = <&l3regs>; + clocks = <&l4_main_clk>; + init-val = <1>; + }; + + hps_fpgabridge1: fpgabridge@1 { + compatible = "altr,socfpga-lwhps2fpga-bridge"; + label = "lwhps2fpga"; + altr,l3-syscon = <&l3regs>; + clocks = <&l4_main_clk>; + init-val = <0>; + }; + + hps_fpgabridge2: fpgabridge@2 { + compatible = "altr,socfpga-fpga2hps-bridge"; + label = "fpga2hps"; + altr,l3-syscon = <&l3regs>; + clocks = <&l4_main_clk>; + };