| Message ID | 20210115013431.27667-2-nava.manne@xilinx.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] dt-bindings: fpga: Add compatible value for Xilinx DFX AXI shutdown manger | expand |
Hi Tom, Thanks for the review. Please find my response inline. > -----Original Message----- > From: Tom Rix <trix@redhat.com> > Sent: Friday, January 15, 2021 11:56 PM > To: Nava kishore Manne <navam@xilinx.com>; mdf@kernel.org; > robh+dt@kernel.org; Michal Simek <michals@xilinx.com>; linux- > fpga@vger.kernel.org; devicetree@vger.kernel.org; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org > Cc: git <git@xilinx.com>; chinnikishore369@gmail.com > Subject: Re: [PATCH 2/2] fpga: Add support for Xilinx DFX AXI Shutdown > manager > > > On 1/14/21 5:34 PM, Nava kishore Manne wrote: > > This patch adds support for Xilinx Dynamic Function eXchange(DFX) AXI > > shutdown manager IP. It can be used to safely handling the AXI traffic > > on a Reconfigurable Partition when it is undergoing dynamic > > reconfiguration and there by preventing system deadlock that may occur > > if AXI transactions are interrupted during reconfiguration. > > > > PR-Decoupler and AXI shutdown manager are completely different IPs. > > But both the IP registers are compatible and also both belong to the > > same sub-system (fpga-bridge).So using same driver for both IP's. > > > > Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com> > > --- > > drivers/fpga/xilinx-pr-decoupler.c | 35 > > ++++++++++++++++++++++++++---- > > It looks like the copyright is wrong, please review spelling of Xilix > > * Copyright (c) 2017, Xilix Inc > Will fix in v2. > > > 1 file changed, 31 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/fpga/xilinx-pr-decoupler.c > > b/drivers/fpga/xilinx-pr-decoupler.c > > index 7d69af230567..c95f3d065ccb 100644 > > --- a/drivers/fpga/xilinx-pr-decoupler.c > > +++ b/drivers/fpga/xilinx-pr-decoupler.c > > @@ -19,10 +19,15 @@ > > #define CTRL_OFFSET 0 > > > > struct xlnx_pr_decoupler_data { > > + const struct xlnx_config_data *ipconfig; > > void __iomem *io_base; > > struct clk *clk; > > }; > > > > +struct xlnx_config_data { > > + char *name; > > +}; > > Move xlnx_config_data above xlnx_pr_decouple_data. > Will fix in v2. > could you 'const' char *name ? > Will fix in v2. > > + > > static inline void xlnx_pr_decoupler_write(struct xlnx_pr_decoupler_data > *d, > > u32 offset, u32 val) > > { > > @@ -76,15 +81,28 @@ static const struct fpga_bridge_ops > xlnx_pr_decoupler_br_ops = { > > .enable_show = xlnx_pr_decoupler_enable_show, }; > > > > +static const struct xlnx_config_data decoupler_config = { > > + .name = "Xilinx PR Decoupler", > > +}; > > + > > +static const struct xlnx_config_data shutdown_config = { > > + .name = "Xilinx DFX AXI shutdown mgr", > > To be consistent with decoupler name, > > shutdown mgr -> Shutdown Manager > Will fix in v2. > > +}; > > + > > static const struct of_device_id xlnx_pr_decoupler_of_match[] = { > > - { .compatible = "xlnx,pr-decoupler-1.00", }, > > - { .compatible = "xlnx,pr-decoupler", }, > > + { .compatible = "xlnx,pr-decoupler-1.00", .data = &decoupler_config > }, > > + { .compatible = "xlnx,pr-decoupler", .data = &decoupler_config }, > > + { .compatible = "xlnx,dfx-axi-shutdown-manager-1.00", > > + .data = &shutdown_config }, > > + { .compatible = "xlnx,dfx-axi-shutdown-manager", > > + .data = &shutdown_config }, > > {}, > > }; > > MODULE_DEVICE_TABLE(of, xlnx_pr_decoupler_of_match); > > > > static int xlnx_pr_decoupler_probe(struct platform_device *pdev) { > > + struct device_node *np = pdev->dev.of_node; > > struct xlnx_pr_decoupler_data *priv; > > struct fpga_bridge *br; > > int err; > > @@ -94,6 +112,14 @@ static int xlnx_pr_decoupler_probe(struct > platform_device *pdev) > > if (!priv) > > return -ENOMEM; > > > > + if (np) { > > + const struct of_device_id *match; > > + > > + match = of_match_node(xlnx_pr_decoupler_of_match, np); > > + if (match && match->data) > > + priv->ipconfig = match->data; > > + } > > + > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > priv->io_base = devm_ioremap_resource(&pdev->dev, res); > > if (IS_ERR(priv->io_base)) > > @@ -114,7 +140,7 @@ static int xlnx_pr_decoupler_probe(struct > > platform_device *pdev) > > > > clk_disable(priv->clk); > > > > - br = devm_fpga_bridge_create(&pdev->dev, "Xilinx PR Decoupler", > > + br = devm_fpga_bridge_create(&pdev->dev, priv->ipconfig->name, > > &xlnx_pr_decoupler_br_ops, priv); > > if (!br) { > > err = -ENOMEM; > > @@ -125,7 +151,8 @@ static int xlnx_pr_decoupler_probe(struct > > platform_device *pdev) > > > > err = fpga_bridge_register(br); > > if (err) { > > - dev_err(&pdev->dev, "unable to register Xilinx PR > Decoupler"); > > + dev_err(&pdev->dev, "unable to register %s", > > + priv->ipconfig->name); > > goto err_clk; > > } > > Look at XILINX_PR_DECOUPLER entry in Kconfig, maybe add something like > > help > > Say Y to enable drivers for the ... Decoupler or DFX AIX Shutdown Manager > Will in v2. Regards, Navakishore.
diff --git a/drivers/fpga/xilinx-pr-decoupler.c b/drivers/fpga/xilinx-pr-decoupler.c index 7d69af230567..c95f3d065ccb 100644 --- a/drivers/fpga/xilinx-pr-decoupler.c +++ b/drivers/fpga/xilinx-pr-decoupler.c @@ -19,10 +19,15 @@ #define CTRL_OFFSET 0 struct xlnx_pr_decoupler_data { + const struct xlnx_config_data *ipconfig; void __iomem *io_base; struct clk *clk; }; +struct xlnx_config_data { + char *name; +}; + static inline void xlnx_pr_decoupler_write(struct xlnx_pr_decoupler_data *d, u32 offset, u32 val) { @@ -76,15 +81,28 @@ static const struct fpga_bridge_ops xlnx_pr_decoupler_br_ops = { .enable_show = xlnx_pr_decoupler_enable_show, }; +static const struct xlnx_config_data decoupler_config = { + .name = "Xilinx PR Decoupler", +}; + +static const struct xlnx_config_data shutdown_config = { + .name = "Xilinx DFX AXI shutdown mgr", +}; + static const struct of_device_id xlnx_pr_decoupler_of_match[] = { - { .compatible = "xlnx,pr-decoupler-1.00", }, - { .compatible = "xlnx,pr-decoupler", }, + { .compatible = "xlnx,pr-decoupler-1.00", .data = &decoupler_config }, + { .compatible = "xlnx,pr-decoupler", .data = &decoupler_config }, + { .compatible = "xlnx,dfx-axi-shutdown-manager-1.00", + .data = &shutdown_config }, + { .compatible = "xlnx,dfx-axi-shutdown-manager", + .data = &shutdown_config }, {}, }; MODULE_DEVICE_TABLE(of, xlnx_pr_decoupler_of_match); static int xlnx_pr_decoupler_probe(struct platform_device *pdev) { + struct device_node *np = pdev->dev.of_node; struct xlnx_pr_decoupler_data *priv; struct fpga_bridge *br; int err; @@ -94,6 +112,14 @@ static int xlnx_pr_decoupler_probe(struct platform_device *pdev) if (!priv) return -ENOMEM; + if (np) { + const struct of_device_id *match; + + match = of_match_node(xlnx_pr_decoupler_of_match, np); + if (match && match->data) + priv->ipconfig = match->data; + } + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); priv->io_base = devm_ioremap_resource(&pdev->dev, res); if (IS_ERR(priv->io_base)) @@ -114,7 +140,7 @@ static int xlnx_pr_decoupler_probe(struct platform_device *pdev) clk_disable(priv->clk); - br = devm_fpga_bridge_create(&pdev->dev, "Xilinx PR Decoupler", + br = devm_fpga_bridge_create(&pdev->dev, priv->ipconfig->name, &xlnx_pr_decoupler_br_ops, priv); if (!br) { err = -ENOMEM; @@ -125,7 +151,8 @@ static int xlnx_pr_decoupler_probe(struct platform_device *pdev) err = fpga_bridge_register(br); if (err) { - dev_err(&pdev->dev, "unable to register Xilinx PR Decoupler"); + dev_err(&pdev->dev, "unable to register %s", + priv->ipconfig->name); goto err_clk; }
This patch adds support for Xilinx Dynamic Function eXchange(DFX) AXI shutdown manager IP. It can be used to safely handling the AXI traffic on a Reconfigurable Partition when it is undergoing dynamic reconfiguration and there by preventing system deadlock that may occur if AXI transactions are interrupted during reconfiguration. PR-Decoupler and AXI shutdown manager are completely different IPs. But both the IP registers are compatible and also both belong to the same sub-system (fpga-bridge).So using same driver for both IP's. Signed-off-by: Nava kishore Manne <nava.manne@xilinx.com> --- drivers/fpga/xilinx-pr-decoupler.c | 35 ++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-)