On Fri, Mar 2, 2018 at 1:24 AM, Shubhrajyoti Datta
<shubhrajyoti.datta@xxxxxxxxx> wrote:
> Hi Moritz,
>
> <snip>
>>> >
>>> > Normally, as Moritz is saying, the reset would be handled by the
>>> > driver for the fabric-based hardware.
>>> I didnt understand you mean the platform-fpga.c ?
>>
>> I don't understand this sentence ;-) I'll try to re-explain:
>>
>> Say you have an AXI DMA engine in there that needs a reset to be toggled
>> after programming the FPGA then you are in either one of these cases:
>>
>> a) You're doing a full reprogram of the entire fabric, at which point
>> you can reset everything by asserting them in the driver like in
>> drivers/fpga/zynq-fpga.c
> That would work however I was thinking the reset technically should be in
> the region and not the fpga manager as it is more related to the region than
> the manager.
OK now I understand that this is supporting a reset that is resetting
all the hardware in a region of fabric. The region could contain
multiple devices. This change isn't meant to support toggling a
single fpga-based device's reset (that would be handled in the
device's driver).
> Ofcourse manager could proxy for the region.
Mapping the reset to the region is the more general case. It will
support resets that either apply to a single PR region or resetting a
whole fabric of a FPGA that is doing full reconfig, i
>
> how about [1]
[1] makes sense to me now. That is a better solution than making this
be a bridge.
>
>>
>> b) You're doing a partial reconfiguration in which case the regions that
>> are being reconfigured contain some peripherals you want to selectively
>> reset. If you need a software reset, the driver for this peripheral can
>> request a reset through the normal reset API.
>
> So if the ip was written in full bitstream case the fpga manager does
> the request.
> In PRR case the driver would do it ?
>
> I would have preferred to keep that(full or PRR case) agnostic to the driver.
>>
>> Am I missing somehting here? Why do you need the bridge to do the reset?
>>
>> - Moritz
> [1]
> diff --git a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> index 6db8aed..955a863 100644
> --- a/Documentation/devicetree/bindings/fpga/fpga-region.txt
> +++ b/Documentation/devicetree/bindings/fpga/fpga-region.txt
> @@ -196,6 +196,7 @@ Optional properties:
> - config-complete-timeout-us : The maximum time in microseconds time for the
> FPGA to go to operating mode after the region has been programmed.
> - child nodes : devices in the FPGA after programming.
> +- resets : Phandle and reset specifier for this region
>
> In the example below, when an overlay is applied targeting fpga-region0,
> fpga_mgr is used to program the FPGA. Two bridges are controlled during
> diff --git a/drivers/fpga/fpga-region.c b/drivers/fpga/fpga-region.c
> index 58789b9..8c87a6b 100644
> --- a/drivers/fpga/fpga-region.c
> +++ b/drivers/fpga/fpga-region.c
> @@ -25,6 +25,7 @@
> #include <linux/of_platform.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
> +#include <linux/reset.h>
>
> /**
> * struct fpga_region - FPGA Region structure
> @@ -235,6 +236,8 @@ static int fpga_region_program_fpga(struct
> fpga_region *region,
> {
> struct fpga_manager *mgr;
> int ret;
> + struct device *dev = ®ion->dev;
> + struct reset_control *rstc;
>
> region = fpga_region_get(region);
> if (IS_ERR(region)) {
> @@ -273,6 +276,15 @@ static int fpga_region_program_fpga(struct
> fpga_region *region,
> goto err_put_br;
> }
>
> + rstc = of_reset_control_array_get(dev->of_node, false, true);
> + if (IS_ERR(rstc)) {
> + goto err_put_br;
> + }
> +
> + reset_control_reset(rstc);
> + reset_control_put(rstc);
> +
This looks good to me. This allows there to be a reset that's not
dedicated to a specific device, but really is resetting a whole PR
region. Or resetting the whole FPGA, it handles both cases. And we
get that pretty pain-free, just add the reset to the fpga region in
the device tree, if this device tree is being used, for example.
Alan
> fpga_bridges_put(®ion->bridge_list);
> fpga_mgr_put(mgr);
> fpga_region_put(region);
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
