Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[Steffen Trumtrar]

Hi!

Alan Tull writes:

> Supports Altera SOCFPGA bridges:
>  * fpga2sdram
>  * fpga2hps
>  * hps2fpga
>  * lwhps2fpga
>
> Allows enabling/disabling the bridges through the FPGA
> Bridge Framework API functions.
>
> The fpga2sdram driver only supports enabling and disabling
> of the ports that been configured early on.  This is due to
> a hardware limitation where the read, write, and command
> ports on the fpga2sdram bridge can only be reconfigured
> while there are no transactions to the sdram, i.e. when
> running out of OCRAM before the kernel boots.
>
> Device tree property 'init-val' configures the driver to
> enable or disable the bridge during probe.  If the property
> does not exist, the driver will leave the bridge in its
> current state.
>
> Signed-off-by: Alan Tull 
> Signed-off-by: Matthew Gerlach 
> Signed-off-by: Dinh Nguyen 

(...)

> +static inline int _alt_fpga2sdram_enable_set(struct alt_fpga2sdram_data 
> *priv,
> +  bool enable)
> +{
> + return regmap_update_bits(priv->sdrctl, ALT_SDR_CTL_FPGAPORTRST_OFST,
> +   priv->mask, enable ? priv->mask : 0);
> +}

(...)

> + /* Get f2s bridge configuration saved in handoff register */
> + regmap_read(sysmgr, SYSMGR_ISWGRP_HANDOFF3, >mask);
> +

Could you maybe add some documentation about this implicit information
shared between a bootloader and this driver?
I understand why you do this, but there must be a better way than
depending on something some bootloader wrote in some undocumented
register, no?
The documentation just says:

"These registers are used to store handoff infomation between the
preloader and the OS. These 8 registers can be used to store any
information. The contents of these registers have no impact on the
state of the HPS hardware"

If it is already agreed upon, that a bridge-enable property is okay,
why not add a port-enable property, too?

Regards,
Steffen Trumtrar

-- 
Pengutronix e.K.   | Steffen Trumtrar|
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- |

Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[Steffen Trumtrar]

Hi!

Alan Tull writes:

> Supports Altera SOCFPGA bridges:
>  * fpga2sdram
>  * fpga2hps
>  * hps2fpga
>  * lwhps2fpga
>
> Allows enabling/disabling the bridges through the FPGA
> Bridge Framework API functions.
>
> The fpga2sdram driver only supports enabling and disabling
> of the ports that been configured early on.  This is due to
> a hardware limitation where the read, write, and command
> ports on the fpga2sdram bridge can only be reconfigured
> while there are no transactions to the sdram, i.e. when
> running out of OCRAM before the kernel boots.
>
> Device tree property 'init-val' configures the driver to
> enable or disable the bridge during probe.  If the property
> does not exist, the driver will leave the bridge in its
> current state.
>
> Signed-off-by: Alan Tull 
> Signed-off-by: Matthew Gerlach 
> Signed-off-by: Dinh Nguyen 

(...)

> +static inline int _alt_fpga2sdram_enable_set(struct alt_fpga2sdram_data 
> *priv,
> +  bool enable)
> +{
> + return regmap_update_bits(priv->sdrctl, ALT_SDR_CTL_FPGAPORTRST_OFST,
> +   priv->mask, enable ? priv->mask : 0);
> +}

(...)

> + /* Get f2s bridge configuration saved in handoff register */
> + regmap_read(sysmgr, SYSMGR_ISWGRP_HANDOFF3, >mask);
> +

Could you maybe add some documentation about this implicit information
shared between a bootloader and this driver?
I understand why you do this, but there must be a better way than
depending on something some bootloader wrote in some undocumented
register, no?
The documentation just says:

"These registers are used to store handoff infomation between the
preloader and the OS. These 8 registers can be used to store any
information. The contents of these registers have no impact on the
state of the HPS hardware"

If it is already agreed upon, that a bridge-enable property is okay,
why not add a port-enable property, too?

Regards,
Steffen Trumtrar

-- 
Pengutronix e.K.   | Steffen Trumtrar|
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- |

Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[atull]

On Tue, 9 Aug 2016, Paul Gortmaker wrote:

> [Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support] On 08/08/2016 
> (Mon 13:44) Moritz Fischer wrote:
> 
> > Hi Alan,
> > 
> > On Mon, Aug 8, 2016 at 12:18 PM, atull <at...@opensource.altera.com> wrote:
> > 
> > >> Please don't use module.h in drivers controlled by a bool
> > >> Kconfig setting.
> > >>
> > >> THanks,
> > >> Paul.
> > >>
> > >
> > > Thanks for the feedback.  Can you provide an example of what you
> > > would consider to be proper usage in the kernel?
> > 
> > 
> > I think Paul is suggesting to use
> > 
> > static int __init alt_fpga_bridge_init(void)
> > {
> > platform_driver_register(_fpga_bridge_driver);
> > }
> > 
> > device_initcall(alt_fpga_bridge_init);
> > 
> > or better:
> > 
> > builtin_platform_driver(_fpga_bridge_driver);
> > 
> > Like for example in: drivers/cpuidle/cpuidle-mvebu-v7.c
> 
> Yes, pretty much that -- if you have a bool Kconfig, you should be using
> builtin registration functions, and have no need for module.h or
> anything MODULE_ or any module_init/module_exit calls.
> 
> An empty file containing nothing but #include  will
> cause cpp to emit about 750k of goop, so we really should only be using
> it for drivers that are genuinely modular; i.e. tristate Kconfig.
> 
> Thanks,
> Paul.
> --
> 
> > 
> > Cheers,
> > 
> > Moritz
> 

Thanks for the feedback and explanations!

I've retested my stuff with it all built as modules (mgr, bridged,
and fpga-region) and it all works that way as well as built in.
So I'll fix up the Kconfig as tristates for everybody.  Also
I'll add some dependencies as FPGA_REGION should be dependent
on FPGA_BRIDGE.

Alan

Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[atull]

On Tue, 9 Aug 2016, Paul Gortmaker wrote:

> [Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support] On 08/08/2016 
> (Mon 13:44) Moritz Fischer wrote:
> 
> > Hi Alan,
> > 
> > On Mon, Aug 8, 2016 at 12:18 PM, atull  wrote:
> > 
> > >> Please don't use module.h in drivers controlled by a bool
> > >> Kconfig setting.
> > >>
> > >> THanks,
> > >> Paul.
> > >>
> > >
> > > Thanks for the feedback.  Can you provide an example of what you
> > > would consider to be proper usage in the kernel?
> > 
> > 
> > I think Paul is suggesting to use
> > 
> > static int __init alt_fpga_bridge_init(void)
> > {
> > platform_driver_register(_fpga_bridge_driver);
> > }
> > 
> > device_initcall(alt_fpga_bridge_init);
> > 
> > or better:
> > 
> > builtin_platform_driver(_fpga_bridge_driver);
> > 
> > Like for example in: drivers/cpuidle/cpuidle-mvebu-v7.c
> 
> Yes, pretty much that -- if you have a bool Kconfig, you should be using
> builtin registration functions, and have no need for module.h or
> anything MODULE_ or any module_init/module_exit calls.
> 
> An empty file containing nothing but #include  will
> cause cpp to emit about 750k of goop, so we really should only be using
> it for drivers that are genuinely modular; i.e. tristate Kconfig.
> 
> Thanks,
> Paul.
> --
> 
> > 
> > Cheers,
> > 
> > Moritz
> 

Thanks for the feedback and explanations!

I've retested my stuff with it all built as modules (mgr, bridged,
and fpga-region) and it all works that way as well as built in.
So I'll fix up the Kconfig as tristates for everybody.  Also
I'll add some dependencies as FPGA_REGION should be dependent
on FPGA_BRIDGE.

Alan

Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[Paul Gortmaker]

[Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support] On 08/08/2016 
(Mon 13:44) Moritz Fischer wrote:

> Hi Alan,
> 
> On Mon, Aug 8, 2016 at 12:18 PM, atull <at...@opensource.altera.com> wrote:
> 
> >> Please don't use module.h in drivers controlled by a bool
> >> Kconfig setting.
> >>
> >> THanks,
> >> Paul.
> >>
> >
> > Thanks for the feedback.  Can you provide an example of what you
> > would consider to be proper usage in the kernel?
> 
> 
> I think Paul is suggesting to use
> 
> static int __init alt_fpga_bridge_init(void)
> {
> platform_driver_register(_fpga_bridge_driver);
> }
> 
> device_initcall(alt_fpga_bridge_init);
> 
> or better:
> 
> builtin_platform_driver(_fpga_bridge_driver);
> 
> Like for example in: drivers/cpuidle/cpuidle-mvebu-v7.c

Yes, pretty much that -- if you have a bool Kconfig, you should be using
builtin registration functions, and have no need for module.h or
anything MODULE_ or any module_init/module_exit calls.

An empty file containing nothing but #include  will
cause cpp to emit about 750k of goop, so we really should only be using
it for drivers that are genuinely modular; i.e. tristate Kconfig.

Thanks,
Paul.
--

> 
> Cheers,
> 
> Moritz

Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[Paul Gortmaker]

[Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support] On 08/08/2016 
(Mon 13:44) Moritz Fischer wrote:

> Hi Alan,
> 
> On Mon, Aug 8, 2016 at 12:18 PM, atull  wrote:
> 
> >> Please don't use module.h in drivers controlled by a bool
> >> Kconfig setting.
> >>
> >> THanks,
> >> Paul.
> >>
> >
> > Thanks for the feedback.  Can you provide an example of what you
> > would consider to be proper usage in the kernel?
> 
> 
> I think Paul is suggesting to use
> 
> static int __init alt_fpga_bridge_init(void)
> {
> platform_driver_register(_fpga_bridge_driver);
> }
> 
> device_initcall(alt_fpga_bridge_init);
> 
> or better:
> 
> builtin_platform_driver(_fpga_bridge_driver);
> 
> Like for example in: drivers/cpuidle/cpuidle-mvebu-v7.c

Yes, pretty much that -- if you have a bool Kconfig, you should be using
builtin registration functions, and have no need for module.h or
anything MODULE_ or any module_init/module_exit calls.

An empty file containing nothing but #include  will
cause cpp to emit about 750k of goop, so we really should only be using
it for drivers that are genuinely modular; i.e. tristate Kconfig.

Thanks,
Paul.
--

> 
> Cheers,
> 
> Moritz

Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[Moritz Fischer]

Hi Alan,

On Mon, Aug 8, 2016 at 12:18 PM, atull  wrote:

>> Please don't use module.h in drivers controlled by a bool
>> Kconfig setting.
>>
>> THanks,
>> Paul.
>>
>
> Thanks for the feedback.  Can you provide an example of what you
> would consider to be proper usage in the kernel?


I think Paul is suggesting to use

static int __init alt_fpga_bridge_init(void)
{
platform_driver_register(_fpga_bridge_driver);
}

device_initcall(alt_fpga_bridge_init);

or better:

builtin_platform_driver(_fpga_bridge_driver);

Like for example in: drivers/cpuidle/cpuidle-mvebu-v7.c

Cheers,

Moritz

Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[Moritz Fischer]

Hi Alan,

On Mon, Aug 8, 2016 at 12:18 PM, atull  wrote:

>> Please don't use module.h in drivers controlled by a bool
>> Kconfig setting.
>>
>> THanks,
>> Paul.
>>
>
> Thanks for the feedback.  Can you provide an example of what you
> would consider to be proper usage in the kernel?


I think Paul is suggesting to use

static int __init alt_fpga_bridge_init(void)
{
platform_driver_register(_fpga_bridge_driver);
}

device_initcall(alt_fpga_bridge_init);

or better:

builtin_platform_driver(_fpga_bridge_driver);

Like for example in: drivers/cpuidle/cpuidle-mvebu-v7.c

Cheers,

Moritz

Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[atull]

On Thu, 14 Jul 2016, Paul Gortmaker wrote:

> On Tue, Jul 12, 2016 at 3:36 PM, Alan Tull  
> wrote:
> > Supports Altera SOCFPGA bridges:
> >  * fpga2sdram
> >  * fpga2hps
> >  * hps2fpga
> >  * lwhps2fpga
> >
> > Allows enabling/disabling the bridges through the FPGA
> > Bridge Framework API functions.
> >
> > The fpga2sdram driver only supports enabling and disabling
> > of the ports that been configured early on.  This is due to
> > a hardware limitation where the read, write, and command
> > ports on the fpga2sdram bridge can only be reconfigured
> > while there are no transactions to the sdram, i.e. when
> > running out of OCRAM before the kernel boots.
> >
> > Device tree property 'init-val' configures the driver to
> > enable or disable the bridge during probe.  If the property
> > does not exist, the driver will leave the bridge in its
> > current state.
> >
> > Signed-off-by: Alan Tull 
> > Signed-off-by: Matthew Gerlach 
> > Signed-off-by: Dinh Nguyen 
> > ---
> > v2:  Use resets instead of directly writing reset registers
> > v12: Bump version to align with simple-fpga-bus version
> >  Get rid of the sysfs interface
> >  fpga2sdram: get configuration stored in handoff register
> > v13: Remove unneeded WARN_ON
> >  Change property from init-val to bridge-enable
> >  Checkpatch cleanup
> >  Fix email address
> > v14: use module_platform_driver
> >  remove unused struct field and some #defines
> >  don't really need exclamation points on error msgs
> >  *const* struct fpga_bridge_ops
> > v15: No change in this patch for v15 of this patch set
> > v16: No change in this patch for v16 of this patch set
> > v17: No change to this patch for v17 of this patch set
> > v18: Eliminate need to specify reset names since only one reset
> > ---
> >  drivers/fpga/Kconfig |   7 ++
> >  drivers/fpga/Makefile|   1 +
> >  drivers/fpga/altera-fpga2sdram.c | 174 
> >  drivers/fpga/altera-hps2fpga.c   | 213 
> > +++
> >  4 files changed, 395 insertions(+)
> >  create mode 100644 drivers/fpga/altera-fpga2sdram.c
> >  create mode 100644 drivers/fpga/altera-hps2fpga.c
> >
> > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > index ec81e21..b346166 100644
> > --- a/drivers/fpga/Kconfig
> > +++ b/drivers/fpga/Kconfig
> > @@ -39,6 +39,13 @@ config FPGA_BRIDGE
> >   Say Y here if you want to support bridges connected between host
> >  processors and FPGAs or between FPGAs.
> >
> > +config SOCFPGA_FPGA_BRIDGE
> > +   bool "Altera SoCFPGA FPGA Bridges"
> > +   depends on ARCH_SOCFPGA && FPGA_BRIDGE
> > +   help
> > + Say Y to enable drivers for FPGA bridges for Altera SOCFPGA
> > + devices.
> > +
> >  endif # FPGA
> >
> >  endmenu
> > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > index 8d746c3..e658436 100644
> > --- a/drivers/fpga/Makefile
> > +++ b/drivers/fpga/Makefile
> > @@ -11,6 +11,7 @@ obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)  += zynq-fpga.o
> >
> >  # FPGA Bridge Drivers
> >  obj-$(CONFIG_FPGA_BRIDGE)  += fpga-bridge.o
> > +obj-$(CONFIG_SOCFPGA_FPGA_BRIDGE)  += altera-hps2fpga.o 
> > altera-fpga2sdram.o
> >
> >  # High Level Interfaces
> >  obj-$(CONFIG_FPGA_REGION)  += fpga-region.o
> > diff --git a/drivers/fpga/altera-fpga2sdram.c 
> > b/drivers/fpga/altera-fpga2sdram.c
> > new file mode 100644
> > index 000..91f4a40
> > --- /dev/null
> > +++ b/drivers/fpga/altera-fpga2sdram.c
> > @@ -0,0 +1,174 @@
> > +/*
> > + * FPGA to SDRAM Bridge Driver for Altera SoCFPGA Devices
> > + *
> > + *  Copyright (C) 2013-2015 Altera Corporation, All Rights Reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along 
> > with
> > + * this program.  If not, see .
> > + */
> > +
> > +/*
> > + * This driver manages a bridge between an FPGA and the SDRAM used by the 
> > ARM
> > + * host processor system (HPS).
> > + *
> > + * The bridge contains 4 read ports, 4 write ports, and 6 command ports.
> > + * Reconfiguring these ports requires that no SDRAM transactions occur 
> > during
> > + * reconfiguration.  The code reconfiguring the ports cannot run out of 
> > SDRAM
> > + * nor can the FPGA access the SDRAM during reconfiguration.  This driver 
> > 

Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[atull]

On Thu, 14 Jul 2016, Paul Gortmaker wrote:

> On Tue, Jul 12, 2016 at 3:36 PM, Alan Tull  
> wrote:
> > Supports Altera SOCFPGA bridges:
> >  * fpga2sdram
> >  * fpga2hps
> >  * hps2fpga
> >  * lwhps2fpga
> >
> > Allows enabling/disabling the bridges through the FPGA
> > Bridge Framework API functions.
> >
> > The fpga2sdram driver only supports enabling and disabling
> > of the ports that been configured early on.  This is due to
> > a hardware limitation where the read, write, and command
> > ports on the fpga2sdram bridge can only be reconfigured
> > while there are no transactions to the sdram, i.e. when
> > running out of OCRAM before the kernel boots.
> >
> > Device tree property 'init-val' configures the driver to
> > enable or disable the bridge during probe.  If the property
> > does not exist, the driver will leave the bridge in its
> > current state.
> >
> > Signed-off-by: Alan Tull 
> > Signed-off-by: Matthew Gerlach 
> > Signed-off-by: Dinh Nguyen 
> > ---
> > v2:  Use resets instead of directly writing reset registers
> > v12: Bump version to align with simple-fpga-bus version
> >  Get rid of the sysfs interface
> >  fpga2sdram: get configuration stored in handoff register
> > v13: Remove unneeded WARN_ON
> >  Change property from init-val to bridge-enable
> >  Checkpatch cleanup
> >  Fix email address
> > v14: use module_platform_driver
> >  remove unused struct field and some #defines
> >  don't really need exclamation points on error msgs
> >  *const* struct fpga_bridge_ops
> > v15: No change in this patch for v15 of this patch set
> > v16: No change in this patch for v16 of this patch set
> > v17: No change to this patch for v17 of this patch set
> > v18: Eliminate need to specify reset names since only one reset
> > ---
> >  drivers/fpga/Kconfig |   7 ++
> >  drivers/fpga/Makefile|   1 +
> >  drivers/fpga/altera-fpga2sdram.c | 174 
> >  drivers/fpga/altera-hps2fpga.c   | 213 
> > +++
> >  4 files changed, 395 insertions(+)
> >  create mode 100644 drivers/fpga/altera-fpga2sdram.c
> >  create mode 100644 drivers/fpga/altera-hps2fpga.c
> >
> > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > index ec81e21..b346166 100644
> > --- a/drivers/fpga/Kconfig
> > +++ b/drivers/fpga/Kconfig
> > @@ -39,6 +39,13 @@ config FPGA_BRIDGE
> >   Say Y here if you want to support bridges connected between host
> >  processors and FPGAs or between FPGAs.
> >
> > +config SOCFPGA_FPGA_BRIDGE
> > +   bool "Altera SoCFPGA FPGA Bridges"
> > +   depends on ARCH_SOCFPGA && FPGA_BRIDGE
> > +   help
> > + Say Y to enable drivers for FPGA bridges for Altera SOCFPGA
> > + devices.
> > +
> >  endif # FPGA
> >
> >  endmenu
> > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> > index 8d746c3..e658436 100644
> > --- a/drivers/fpga/Makefile
> > +++ b/drivers/fpga/Makefile
> > @@ -11,6 +11,7 @@ obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)  += zynq-fpga.o
> >
> >  # FPGA Bridge Drivers
> >  obj-$(CONFIG_FPGA_BRIDGE)  += fpga-bridge.o
> > +obj-$(CONFIG_SOCFPGA_FPGA_BRIDGE)  += altera-hps2fpga.o 
> > altera-fpga2sdram.o
> >
> >  # High Level Interfaces
> >  obj-$(CONFIG_FPGA_REGION)  += fpga-region.o
> > diff --git a/drivers/fpga/altera-fpga2sdram.c 
> > b/drivers/fpga/altera-fpga2sdram.c
> > new file mode 100644
> > index 000..91f4a40
> > --- /dev/null
> > +++ b/drivers/fpga/altera-fpga2sdram.c
> > @@ -0,0 +1,174 @@
> > +/*
> > + * FPGA to SDRAM Bridge Driver for Altera SoCFPGA Devices
> > + *
> > + *  Copyright (C) 2013-2015 Altera Corporation, All Rights Reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along 
> > with
> > + * this program.  If not, see .
> > + */
> > +
> > +/*
> > + * This driver manages a bridge between an FPGA and the SDRAM used by the 
> > ARM
> > + * host processor system (HPS).
> > + *
> > + * The bridge contains 4 read ports, 4 write ports, and 6 command ports.
> > + * Reconfiguring these ports requires that no SDRAM transactions occur 
> > during
> > + * reconfiguration.  The code reconfiguring the ports cannot run out of 
> > SDRAM
> > + * nor can the FPGA access the SDRAM during reconfiguration.  This driver 
> > does
> > + * not support reconfiguring the ports.  The ports are configured by code
> > + * running out of on 

Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[Paul Gortmaker]

On Tue, Jul 12, 2016 at 3:36 PM, Alan Tull  wrote:
> Supports Altera SOCFPGA bridges:
>  * fpga2sdram
>  * fpga2hps
>  * hps2fpga
>  * lwhps2fpga
>
> Allows enabling/disabling the bridges through the FPGA
> Bridge Framework API functions.
>
> The fpga2sdram driver only supports enabling and disabling
> of the ports that been configured early on.  This is due to
> a hardware limitation where the read, write, and command
> ports on the fpga2sdram bridge can only be reconfigured
> while there are no transactions to the sdram, i.e. when
> running out of OCRAM before the kernel boots.
>
> Device tree property 'init-val' configures the driver to
> enable or disable the bridge during probe.  If the property
> does not exist, the driver will leave the bridge in its
> current state.
>
> Signed-off-by: Alan Tull 
> Signed-off-by: Matthew Gerlach 
> Signed-off-by: Dinh Nguyen 
> ---
> v2:  Use resets instead of directly writing reset registers
> v12: Bump version to align with simple-fpga-bus version
>  Get rid of the sysfs interface
>  fpga2sdram: get configuration stored in handoff register
> v13: Remove unneeded WARN_ON
>  Change property from init-val to bridge-enable
>  Checkpatch cleanup
>  Fix email address
> v14: use module_platform_driver
>  remove unused struct field and some #defines
>  don't really need exclamation points on error msgs
>  *const* struct fpga_bridge_ops
> v15: No change in this patch for v15 of this patch set
> v16: No change in this patch for v16 of this patch set
> v17: No change to this patch for v17 of this patch set
> v18: Eliminate need to specify reset names since only one reset
> ---
>  drivers/fpga/Kconfig |   7 ++
>  drivers/fpga/Makefile|   1 +
>  drivers/fpga/altera-fpga2sdram.c | 174 
>  drivers/fpga/altera-hps2fpga.c   | 213 
> +++
>  4 files changed, 395 insertions(+)
>  create mode 100644 drivers/fpga/altera-fpga2sdram.c
>  create mode 100644 drivers/fpga/altera-hps2fpga.c
>
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index ec81e21..b346166 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -39,6 +39,13 @@ config FPGA_BRIDGE
>   Say Y here if you want to support bridges connected between host
>  processors and FPGAs or between FPGAs.
>
> +config SOCFPGA_FPGA_BRIDGE
> +   bool "Altera SoCFPGA FPGA Bridges"
> +   depends on ARCH_SOCFPGA && FPGA_BRIDGE
> +   help
> + Say Y to enable drivers for FPGA bridges for Altera SOCFPGA
> + devices.
> +
>  endif # FPGA
>
>  endmenu
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 8d746c3..e658436 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)  += zynq-fpga.o
>
>  # FPGA Bridge Drivers
>  obj-$(CONFIG_FPGA_BRIDGE)  += fpga-bridge.o
> +obj-$(CONFIG_SOCFPGA_FPGA_BRIDGE)  += altera-hps2fpga.o 
> altera-fpga2sdram.o
>
>  # High Level Interfaces
>  obj-$(CONFIG_FPGA_REGION)  += fpga-region.o
> diff --git a/drivers/fpga/altera-fpga2sdram.c 
> b/drivers/fpga/altera-fpga2sdram.c
> new file mode 100644
> index 000..91f4a40
> --- /dev/null
> +++ b/drivers/fpga/altera-fpga2sdram.c
> @@ -0,0 +1,174 @@
> +/*
> + * FPGA to SDRAM Bridge Driver for Altera SoCFPGA Devices
> + *
> + *  Copyright (C) 2013-2015 Altera Corporation, All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see .
> + */
> +
> +/*
> + * This driver manages a bridge between an FPGA and the SDRAM used by the ARM
> + * host processor system (HPS).
> + *
> + * The bridge contains 4 read ports, 4 write ports, and 6 command ports.
> + * Reconfiguring these ports requires that no SDRAM transactions occur during
> + * reconfiguration.  The code reconfiguring the ports cannot run out of SDRAM
> + * nor can the FPGA access the SDRAM during reconfiguration.  This driver 
> does
> + * not support reconfiguring the ports.  The ports are configured by code
> + * running out of on chip ram before Linux is started and the configuration
> + * is passed in a handoff register in the system manager.
> + *
> + * This driver supports enabling and disabling of the configured ports, 

Re: [PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[Paul Gortmaker]

On Tue, Jul 12, 2016 at 3:36 PM, Alan Tull  wrote:
> Supports Altera SOCFPGA bridges:
>  * fpga2sdram
>  * fpga2hps
>  * hps2fpga
>  * lwhps2fpga
>
> Allows enabling/disabling the bridges through the FPGA
> Bridge Framework API functions.
>
> The fpga2sdram driver only supports enabling and disabling
> of the ports that been configured early on.  This is due to
> a hardware limitation where the read, write, and command
> ports on the fpga2sdram bridge can only be reconfigured
> while there are no transactions to the sdram, i.e. when
> running out of OCRAM before the kernel boots.
>
> Device tree property 'init-val' configures the driver to
> enable or disable the bridge during probe.  If the property
> does not exist, the driver will leave the bridge in its
> current state.
>
> Signed-off-by: Alan Tull 
> Signed-off-by: Matthew Gerlach 
> Signed-off-by: Dinh Nguyen 
> ---
> v2:  Use resets instead of directly writing reset registers
> v12: Bump version to align with simple-fpga-bus version
>  Get rid of the sysfs interface
>  fpga2sdram: get configuration stored in handoff register
> v13: Remove unneeded WARN_ON
>  Change property from init-val to bridge-enable
>  Checkpatch cleanup
>  Fix email address
> v14: use module_platform_driver
>  remove unused struct field and some #defines
>  don't really need exclamation points on error msgs
>  *const* struct fpga_bridge_ops
> v15: No change in this patch for v15 of this patch set
> v16: No change in this patch for v16 of this patch set
> v17: No change to this patch for v17 of this patch set
> v18: Eliminate need to specify reset names since only one reset
> ---
>  drivers/fpga/Kconfig |   7 ++
>  drivers/fpga/Makefile|   1 +
>  drivers/fpga/altera-fpga2sdram.c | 174 
>  drivers/fpga/altera-hps2fpga.c   | 213 
> +++
>  4 files changed, 395 insertions(+)
>  create mode 100644 drivers/fpga/altera-fpga2sdram.c
>  create mode 100644 drivers/fpga/altera-hps2fpga.c
>
> diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> index ec81e21..b346166 100644
> --- a/drivers/fpga/Kconfig
> +++ b/drivers/fpga/Kconfig
> @@ -39,6 +39,13 @@ config FPGA_BRIDGE
>   Say Y here if you want to support bridges connected between host
>  processors and FPGAs or between FPGAs.
>
> +config SOCFPGA_FPGA_BRIDGE
> +   bool "Altera SoCFPGA FPGA Bridges"
> +   depends on ARCH_SOCFPGA && FPGA_BRIDGE
> +   help
> + Say Y to enable drivers for FPGA bridges for Altera SOCFPGA
> + devices.
> +
>  endif # FPGA
>
>  endmenu
> diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
> index 8d746c3..e658436 100644
> --- a/drivers/fpga/Makefile
> +++ b/drivers/fpga/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)  += zynq-fpga.o
>
>  # FPGA Bridge Drivers
>  obj-$(CONFIG_FPGA_BRIDGE)  += fpga-bridge.o
> +obj-$(CONFIG_SOCFPGA_FPGA_BRIDGE)  += altera-hps2fpga.o 
> altera-fpga2sdram.o
>
>  # High Level Interfaces
>  obj-$(CONFIG_FPGA_REGION)  += fpga-region.o
> diff --git a/drivers/fpga/altera-fpga2sdram.c 
> b/drivers/fpga/altera-fpga2sdram.c
> new file mode 100644
> index 000..91f4a40
> --- /dev/null
> +++ b/drivers/fpga/altera-fpga2sdram.c
> @@ -0,0 +1,174 @@
> +/*
> + * FPGA to SDRAM Bridge Driver for Altera SoCFPGA Devices
> + *
> + *  Copyright (C) 2013-2015 Altera Corporation, All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along 
> with
> + * this program.  If not, see .
> + */
> +
> +/*
> + * This driver manages a bridge between an FPGA and the SDRAM used by the ARM
> + * host processor system (HPS).
> + *
> + * The bridge contains 4 read ports, 4 write ports, and 6 command ports.
> + * Reconfiguring these ports requires that no SDRAM transactions occur during
> + * reconfiguration.  The code reconfiguring the ports cannot run out of SDRAM
> + * nor can the FPGA access the SDRAM during reconfiguration.  This driver 
> does
> + * not support reconfiguring the ports.  The ports are configured by code
> + * running out of on chip ram before Linux is started and the configuration
> + * is passed in a handoff register in the system manager.
> + *
> + * This driver supports enabling and disabling of the configured ports, which
> + * allows for safe reprogramming of the FPGA, assuming that the new FPGA 
> image
> + * uses the same 

[PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[Alan Tull]

Supports Altera SOCFPGA bridges:
 * fpga2sdram
 * fpga2hps
 * hps2fpga
 * lwhps2fpga

Allows enabling/disabling the bridges through the FPGA
Bridge Framework API functions.

The fpga2sdram driver only supports enabling and disabling
of the ports that been configured early on.  This is due to
a hardware limitation where the read, write, and command
ports on the fpga2sdram bridge can only be reconfigured
while there are no transactions to the sdram, i.e. when
running out of OCRAM before the kernel boots.

Device tree property 'init-val' configures the driver to
enable or disable the bridge during probe.  If the property
does not exist, the driver will leave the bridge in its
current state.

Signed-off-by: Alan Tull 
Signed-off-by: Matthew Gerlach 
Signed-off-by: Dinh Nguyen 
---
v2:  Use resets instead of directly writing reset registers
v12: Bump version to align with simple-fpga-bus version
 Get rid of the sysfs interface
 fpga2sdram: get configuration stored in handoff register
v13: Remove unneeded WARN_ON
 Change property from init-val to bridge-enable
 Checkpatch cleanup
 Fix email address
v14: use module_platform_driver
 remove unused struct field and some #defines
 don't really need exclamation points on error msgs
 *const* struct fpga_bridge_ops
v15: No change in this patch for v15 of this patch set
v16: No change in this patch for v16 of this patch set
v17: No change to this patch for v17 of this patch set
v18: Eliminate need to specify reset names since only one reset
---
 drivers/fpga/Kconfig |   7 ++
 drivers/fpga/Makefile|   1 +
 drivers/fpga/altera-fpga2sdram.c | 174 
 drivers/fpga/altera-hps2fpga.c   | 213 +++
 4 files changed, 395 insertions(+)
 create mode 100644 drivers/fpga/altera-fpga2sdram.c
 create mode 100644 drivers/fpga/altera-hps2fpga.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index ec81e21..b346166 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -39,6 +39,13 @@ config FPGA_BRIDGE
  Say Y here if you want to support bridges connected between host
 processors and FPGAs or between FPGAs.
 
+config SOCFPGA_FPGA_BRIDGE
+   bool "Altera SoCFPGA FPGA Bridges"
+   depends on ARCH_SOCFPGA && FPGA_BRIDGE
+   help
+ Say Y to enable drivers for FPGA bridges for Altera SOCFPGA
+ devices.
+
 endif # FPGA
 
 endmenu
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 8d746c3..e658436 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)  += zynq-fpga.o
 
 # FPGA Bridge Drivers
 obj-$(CONFIG_FPGA_BRIDGE)  += fpga-bridge.o
+obj-$(CONFIG_SOCFPGA_FPGA_BRIDGE)  += altera-hps2fpga.o altera-fpga2sdram.o
 
 # High Level Interfaces
 obj-$(CONFIG_FPGA_REGION)  += fpga-region.o
diff --git a/drivers/fpga/altera-fpga2sdram.c b/drivers/fpga/altera-fpga2sdram.c
new file mode 100644
index 000..91f4a40
--- /dev/null
+++ b/drivers/fpga/altera-fpga2sdram.c
@@ -0,0 +1,174 @@
+/*
+ * FPGA to SDRAM Bridge Driver for Altera SoCFPGA Devices
+ *
+ *  Copyright (C) 2013-2015 Altera Corporation, All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+/*
+ * This driver manages a bridge between an FPGA and the SDRAM used by the ARM
+ * host processor system (HPS).
+ *
+ * The bridge contains 4 read ports, 4 write ports, and 6 command ports.
+ * Reconfiguring these ports requires that no SDRAM transactions occur during
+ * reconfiguration.  The code reconfiguring the ports cannot run out of SDRAM
+ * nor can the FPGA access the SDRAM during reconfiguration.  This driver does
+ * not support reconfiguring the ports.  The ports are configured by code
+ * running out of on chip ram before Linux is started and the configuration
+ * is passed in a handoff register in the system manager.
+ *
+ * This driver supports enabling and disabling of the configured ports, which
+ * allows for safe reprogramming of the FPGA, assuming that the new FPGA image
+ * uses the same port configuration.  Bridges must be disabled before
+ * reprogramming the FPGA and re-enabled after the FPGA has been programmed.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define 

[PATCH v18 6/6] ARM: socfpga: fpga bridge driver support

[Alan Tull]

Supports Altera SOCFPGA bridges:
 * fpga2sdram
 * fpga2hps
 * hps2fpga
 * lwhps2fpga

Allows enabling/disabling the bridges through the FPGA
Bridge Framework API functions.

The fpga2sdram driver only supports enabling and disabling
of the ports that been configured early on.  This is due to
a hardware limitation where the read, write, and command
ports on the fpga2sdram bridge can only be reconfigured
while there are no transactions to the sdram, i.e. when
running out of OCRAM before the kernel boots.

Device tree property 'init-val' configures the driver to
enable or disable the bridge during probe.  If the property
does not exist, the driver will leave the bridge in its
current state.

Signed-off-by: Alan Tull 
Signed-off-by: Matthew Gerlach 
Signed-off-by: Dinh Nguyen 
---
v2:  Use resets instead of directly writing reset registers
v12: Bump version to align with simple-fpga-bus version
 Get rid of the sysfs interface
 fpga2sdram: get configuration stored in handoff register
v13: Remove unneeded WARN_ON
 Change property from init-val to bridge-enable
 Checkpatch cleanup
 Fix email address
v14: use module_platform_driver
 remove unused struct field and some #defines
 don't really need exclamation points on error msgs
 *const* struct fpga_bridge_ops
v15: No change in this patch for v15 of this patch set
v16: No change in this patch for v16 of this patch set
v17: No change to this patch for v17 of this patch set
v18: Eliminate need to specify reset names since only one reset
---
 drivers/fpga/Kconfig |   7 ++
 drivers/fpga/Makefile|   1 +
 drivers/fpga/altera-fpga2sdram.c | 174 
 drivers/fpga/altera-hps2fpga.c   | 213 +++
 4 files changed, 395 insertions(+)
 create mode 100644 drivers/fpga/altera-fpga2sdram.c
 create mode 100644 drivers/fpga/altera-hps2fpga.c

diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index ec81e21..b346166 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -39,6 +39,13 @@ config FPGA_BRIDGE
  Say Y here if you want to support bridges connected between host
 processors and FPGAs or between FPGAs.
 
+config SOCFPGA_FPGA_BRIDGE
+   bool "Altera SoCFPGA FPGA Bridges"
+   depends on ARCH_SOCFPGA && FPGA_BRIDGE
+   help
+ Say Y to enable drivers for FPGA bridges for Altera SOCFPGA
+ devices.
+
 endif # FPGA
 
 endmenu
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 8d746c3..e658436 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA)  += zynq-fpga.o
 
 # FPGA Bridge Drivers
 obj-$(CONFIG_FPGA_BRIDGE)  += fpga-bridge.o
+obj-$(CONFIG_SOCFPGA_FPGA_BRIDGE)  += altera-hps2fpga.o altera-fpga2sdram.o
 
 # High Level Interfaces
 obj-$(CONFIG_FPGA_REGION)  += fpga-region.o
diff --git a/drivers/fpga/altera-fpga2sdram.c b/drivers/fpga/altera-fpga2sdram.c
new file mode 100644
index 000..91f4a40
--- /dev/null
+++ b/drivers/fpga/altera-fpga2sdram.c
@@ -0,0 +1,174 @@
+/*
+ * FPGA to SDRAM Bridge Driver for Altera SoCFPGA Devices
+ *
+ *  Copyright (C) 2013-2015 Altera Corporation, All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+/*
+ * This driver manages a bridge between an FPGA and the SDRAM used by the ARM
+ * host processor system (HPS).
+ *
+ * The bridge contains 4 read ports, 4 write ports, and 6 command ports.
+ * Reconfiguring these ports requires that no SDRAM transactions occur during
+ * reconfiguration.  The code reconfiguring the ports cannot run out of SDRAM
+ * nor can the FPGA access the SDRAM during reconfiguration.  This driver does
+ * not support reconfiguring the ports.  The ports are configured by code
+ * running out of on chip ram before Linux is started and the configuration
+ * is passed in a handoff register in the system manager.
+ *
+ * This driver supports enabling and disabling of the configured ports, which
+ * allows for safe reprogramming of the FPGA, assuming that the new FPGA image
+ * uses the same port configuration.  Bridges must be disabled before
+ * reprogramming the FPGA and re-enabled after the FPGA has been programmed.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define ALT_SDR_CTL_FPGAPORTRST_OFST   0x80
+#define