[PATCH 1/2] dt-bindings: fpga: Add compatible value for Xilinx DFX AXI shutdown manger

Subsystems: fpga manager framework, open firmware and flattened device tree bindings, the rest

STALE2019d

10 messages, 4 authors, 2021-01-27 · open the first message on its own page

[PATCH 1/2] dt-bindings: fpga: Add compatible value for Xilinx DFX AXI shutdown manger

From: Nava kishore Manne <hidden>
Date: 2021-01-15 01:41:58

This patch Adds compatible value for Xilinx Dynamic Function eXchnage(DFX)
AXI Shutdown manager IP.

Signed-off-by: Nava kishore Manne <redacted>
---
 .../bindings/fpga/xilinx-pr-decoupler.txt     | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
index 4284d293fa61..42fca058a5f4 100644
--- a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
+++ b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt

@@ -7,13 +7,26 @@ changes from passing through the bridge.  The controller can also
 couple / enable the bridges which allows traffic to pass through the
 bridge normally.
 
+Xilinx LogiCORE Dynamic Function eXchange(DFX) AXI shutdown manager
+Softcore
+
+The Xilinx LogiCORE Dynamic Function eXchange(DFX) AXI shutdown manager
+manages one or more shutdown managers / fpga bridges.
+The controller can shutdown/passthrough the bridges which prevents
+AXI traffic from passing through the bridge. The controller safely
+handles AXI4MM and AXI4-Lite interfaces on a Reconfigurable Partition
+when it is undergoing dynamic reconfiguration (DFX), preventing system
+deadlock that can occur if AXI transactions are interrupted by DFX
+
 The Driver supports only MMIO handling. A PR region can have multiple
-PR Decouplers which can be handled independently or chained via decouple/
-decouple_status signals.
+PR Decouplers/AXI shutdown manager which can be handled independently
+or chained via decouple/ decouple_status signals.
 
 Required properties:
 - compatible		: Should contain "xlnx,pr-decoupler-1.00" followed by
-                          "xlnx,pr-decoupler"
+                          "xlnx,pr-decoupler" or
+			  "xlnx,dfx-axi-shutdown-manager-1.00" followed by
+			  "xlnx,dfx-axi-shutdown-manager"
 - regs			: base address and size for decoupler module
 - clocks		: input clock to IP
 - clock-names		: should contain "aclk"

-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] fpga: Add support for Xilinx DFX AXI Shutdown manager

From: Nava kishore Manne <hidden>
Date: 2021-01-15 01:41:58

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 <redacted>
---
 drivers/fpga/xilinx-pr-decoupler.c | 35 ++++++++++++++++++++++++++----
 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;
+};
+
 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;
 	}

-- 
2.18.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] dt-bindings: fpga: Add compatible value for Xilinx DFX AXI shutdown manger

From: Tom Rix <trix@redhat.com>
Date: 2021-01-15 18:09:23

On 1/14/21 5:34 PM, Nava kishore Manne wrote:

This patch Adds compatible value for Xilinx Dynamic Function eXchnage(DFX)
AXI Shutdown manager IP.

A multi patch set should have a cover letter.

Use git format-patch --cover-letter

quoted hunk

Signed-off-by: Nava kishore Manne <redacted>
---
 .../bindings/fpga/xilinx-pr-decoupler.txt     | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
index 4284d293fa61..42fca058a5f4 100644
--- a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
+++ b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt

@@ -7,13 +7,26 @@ changes from passing through the bridge.  The controller can also
 couple / enable the bridges which allows traffic to pass through the
 bridge normally.

Below is some wordsmithing to make the compatibility explicit and remove unneeded words

 
+Xilinx LogiCORE Dynamic Function eXchange(DFX) AXI shutdown manager
+Softcore

is compatible with the Xilinx LogiCORE pr-decoupler.

+
+The Xilinx LogiCORE Dynamic Function eXchange(DFX) AXI shutdown manager
+manages one or more shutdown managers / fpga bridges.
+The controller can shutdown/passthrough the bridges which prevents

Delete 4 lines above.

The DFX AXI shutdown manager prevents

+AXI traffic from passing through the bridge. The controller safely
+handles AXI4MM and AXI4-Lite interfaces on a Reconfigurable Partition
+when it is undergoing dynamic reconfiguration (DFX), preventing system

I don't know about the use of '(DFX)' here, above it is Dynamic Function eXchange'

Here it is dynamic reconfiguration, maybe just delete the '(DFX)'

+deadlock that can occur if AXI transactions are interrupted by DFX
+
 The Driver supports only MMIO handling. A PR region can have multiple
-PR Decouplers which can be handled independently or chained via decouple/
-decouple_status signals.
+PR Decouplers/AXI shutdown manager which can be handled independently
+or chained via decouple/ decouple_status signals.

Being compatible, i don't think it is necessary to add to / change this section.

 
 Required properties:
 - compatible		: Should contain "xlnx,pr-decoupler-1.00" followed by
-                          "xlnx,pr-decoupler"
+                          "xlnx,pr-decoupler" or
+			  "xlnx,dfx-axi-shutdown-manager-1.00" followed by
+			  "xlnx,dfx-axi-shutdown-manager"
 - regs			: base address and size for decoupler module
 - clocks		: input clock to IP
 - clock-names		: should contain "aclk"

Adding an example similar to the existing would be helpful.

Tom


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] fpga: Add support for Xilinx DFX AXI Shutdown manager

From: Tom Rix <trix@redhat.com>
Date: 2021-01-15 18:28:09

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 <redacted>
---
 drivers/fpga/xilinx-pr-decoupler.c | 35 ++++++++++++++++++++++++++----

It looks like the copyright is wrong, please review spelling of Xilix

 * Copyright (c) 2017, Xilix Inc

quoted hunk

 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.

could you 'const' char *name ?

quoted hunk

+
 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

quoted hunk

+};
+
 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

Tom

 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] fpga: Add support for Xilinx DFX AXI Shutdown manager

From: Moritz Fischer <mdf@kernel.org>
Date: 2021-01-16 02:59:28

Hi,

On Fri, Jan 15, 2021 at 07:04:31AM +0530, 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.

I'm a bit confused, the whole goal here is to give the thing a
different name?

quoted hunk

Signed-off-by: Nava kishore Manne <redacted>
---
 drivers/fpga/xilinx-pr-decoupler.c | 35 ++++++++++++++++++++++++++----
 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;
+};
+
 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",
+};

If it's just the strings, why not store them as is?

quoted hunk

+
 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;
 	}

-- 
2.18.0

Thanks,
Moritz

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 1/2] dt-bindings: fpga: Add compatible value for Xilinx DFX AXI shutdown manger

From: Nava kishore Manne <hidden>
Date: 2021-01-19 06:27:19

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:37 PM
To: Nava kishore Manne <redacted>; mdf@kernel.org;
robh+dt@kernel.org; Michal Simek [off-list ref]; linux-
fpga@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
kernel@lists.infradead.org; linux-kernel@vger.kernel.org
Cc: git <redacted>; chinnikishore369@gmail.com
Subject: Re: [PATCH 1/2] dt-bindings: fpga: Add compatible value for Xilinx
DFX AXI shutdown manger


On 1/14/21 5:34 PM, Nava kishore Manne wrote:

quoted

This patch Adds compatible value for Xilinx Dynamic Function
eXchnage(DFX) AXI Shutdown manager IP.

A multi patch set should have a cover letter.

Use git format-patch --cover-letter

Will  fix in v2.

quoted

Signed-off-by: Nava kishore Manne <redacted>
---
 .../bindings/fpga/xilinx-pr-decoupler.txt     | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git
a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
index 4284d293fa61..42fca058a5f4 100644

--- a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt
+++ b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt

@@ -7,13 +7,26 @@ changes from passing through the bridge.  The

controller can also  couple / enable the bridges which allows traffic
to pass through the  bridge normally.

Below is some wordsmithing to make the compatibility explicit and remove
unneeded words

Will fix in v2

quoted

+Xilinx LogiCORE Dynamic Function eXchange(DFX) AXI shutdown manager
+Softcore

is compatible with the Xilinx LogiCORE pr-decoupler.

Yes, Both IP's are compatible with respect to the IP registers spec.

quoted

+
+The Xilinx LogiCORE Dynamic Function eXchange(DFX) AXI shutdown
+manager manages one or more shutdown managers / fpga bridges.
+The controller can shutdown/passthrough the bridges which prevents

Delete 4 lines above.

Will fix in v2.

The DFX AXI shutdown manager prevents

Will fix in v2.

quoted

+AXI traffic from passing through the bridge. The controller safely
+handles AXI4MM and AXI4-Lite interfaces on a Reconfigurable Partition
+when it is undergoing dynamic reconfiguration (DFX), preventing
+system

I don't know about the use of '(DFX)' here, above it is Dynamic Function
eXchange'

Here it is dynamic reconfiguration, maybe just delete the '(DFX)'


Will fix in v2.

quoted

+deadlock that can occur if AXI transactions are interrupted by DFX
+
 The Driver supports only MMIO handling. A PR region can have multiple
-PR Decouplers which can be handled independently or chained via
decouple/ -decouple_status signals.
+PR Decouplers/AXI shutdown manager which can be handled

independently

quoted

+or chained via decouple/ decouple_status signals.

Being compatible, i don't think it is necessary to add to / change this section.

quoted

 Required properties:
 - compatible		: Should contain "xlnx,pr-decoupler-1.00" followed

by

quoted

-                          "xlnx,pr-decoupler"
+                          "xlnx,pr-decoupler" or
+			  "xlnx,dfx-axi-shutdown-manager-1.00" followed by
+			  "xlnx,dfx-axi-shutdown-manager"
 - regs			: base address and size for decoupler module
 - clocks		: input clock to IP
 - clock-names		: should contain "aclk"

Adding an example similar to the existing would be helpful.

Will fix in v2.

Regards,
Navakishore.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 2/2] fpga: Add support for Xilinx DFX AXI Shutdown manager

From: Nava kishore Manne <hidden>
Date: 2021-01-19 06:28:59

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 <redacted>; mdf@kernel.org;
robh+dt@kernel.org; Michal Simek [off-list ref]; linux-
fpga@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
kernel@lists.infradead.org; linux-kernel@vger.kernel.org
Cc: git <redacted>; 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:

quoted

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 <redacted>
---
 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.

quoted

 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.

quoted

+
 static inline void xlnx_pr_decoupler_write(struct xlnx_pr_decoupler_data

*d,

quoted

 					   u32 offset, u32 val)
 {

@@ -76,15 +81,28 @@ static const struct fpga_bridge_ops

xlnx_pr_decoupler_br_ops = {

quoted

 	.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.

quoted

+};
+
 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

},

quoted

+	{ .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)

quoted

 	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");

quoted

+		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.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 2/2] fpga: Add support for Xilinx DFX AXI Shutdown manager

From: Nava kishore Manne <hidden>
Date: 2021-01-19 06:36:52

Hi Moritz,

	Thanks for the review.
Please find my response inline.

-----Original Message-----
From: Moritz Fischer <mdf@kernel.org>
Sent: Saturday, January 16, 2021 8:28 AM
To: Nava kishore Manne <redacted>
Cc: mdf@kernel.org; trix@redhat.com; robh+dt@kernel.org; Michal Simek
[off-list ref]; linux-fpga@vger.kernel.org;
devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
kernel@vger.kernel.org; git [off-list ref]; chinnikishore369@gmail.com
Subject: Re: [PATCH 2/2] fpga: Add support for Xilinx DFX AXI Shutdown
manager

Hi,

On Fri, Jan 15, 2021 at 07:04:31AM +0530, Nava kishore Manne wrote:

quoted

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.

I'm a bit confused, the whole goal here is to give the thing a different name?


Both the PR Decoupler and AXI Shutdown IP manager IP's are follows same register spec.
Most of the code is common so we thought of reusing  same driver for AXI shutdown manager as well.

quoted

Signed-off-by: Nava kishore Manne <redacted>
---
 drivers/fpga/xilinx-pr-decoupler.c | 35
++++++++++++++++++++++++++----
 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;
+};
+
 static inline void xlnx_pr_decoupler_write(struct xlnx_pr_decoupler_data

*d,

quoted

 					   u32 offset, u32 val)
 {

@@ -76,15 +81,28 @@ static const struct fpga_bridge_ops

xlnx_pr_decoupler_br_ops = {

quoted

 	.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", };

If it's just the strings, why not store them as is?

In order to differentiate the IP's at probe time we are using this name filed.

quoted

+
 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

},

quoted

+	{ .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)

quoted

 	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");

quoted

+		dev_err(&pdev->dev, "unable to register %s",
+			priv->ipconfig->name);
 		goto err_clk;
 	}

--
2.18.0

Regards,
Navakishore.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] fpga: Add support for Xilinx DFX AXI Shutdown manager

From: Moritz Fischer <mdf@kernel.org>
Date: 2021-01-22 05:16:15

On Tue, Jan 19, 2021 at 06:34:54AM +0000, Nava kishore Manne wrote:

Hi Moritz,

	Thanks for the review.
Please find my response inline.

quoted

-----Original Message-----
From: Moritz Fischer <mdf@kernel.org>
Sent: Saturday, January 16, 2021 8:28 AM
To: Nava kishore Manne <redacted>
Cc: mdf@kernel.org; trix@redhat.com; robh+dt@kernel.org; Michal Simek
[off-list ref]; linux-fpga@vger.kernel.org;
devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
kernel@vger.kernel.org; git [off-list ref]; chinnikishore369@gmail.com
Subject: Re: [PATCH 2/2] fpga: Add support for Xilinx DFX AXI Shutdown
manager

Hi,

On Fri, Jan 15, 2021 at 07:04:31AM +0530, Nava kishore Manne wrote:

quoted

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.

I'm a bit confused, the whole goal here is to give the thing a different name?


Both the PR Decoupler and AXI Shutdown IP manager IP's are follows same register spec.
Most of the code is common so we thought of reusing  same driver for AXI shutdown manager as well.

What are the differences, though other than it shows a different name?

quoted

Signed-off-by: Nava kishore Manne <redacted>
---
 drivers/fpga/xilinx-pr-decoupler.c | 35
++++++++++++++++++++++++++----
 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;
+};
+
 static inline void xlnx_pr_decoupler_write(struct xlnx_pr_decoupler_data

*d,

quoted

 					   u32 offset, u32 val)
 {

@@ -76,15 +81,28 @@ static const struct fpga_bridge_ops

xlnx_pr_decoupler_br_ops = {

quoted

 	.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", };

If it's just the strings, why not store them as is?

In order to differentiate the IP's at probe time we are using this name filed.

quoted

+
 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

},

quoted

+	{ .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)

quoted

 	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");

quoted

+		dev_err(&pdev->dev, "unable to register %s",
+			priv->ipconfig->name);
 		goto err_clk;
 	}

--
2.18.0

Regards,
Navakishore.

- Moritz

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH 2/2] fpga: Add support for Xilinx DFX AXI Shutdown manager

From: Nava kishore Manne <hidden>
Date: 2021-01-27 08:49:48

Hi Moritz,

	Please find my response inline.

-----Original Message-----
From: Moritz Fischer <mdf@kernel.org>
Sent: Friday, January 22, 2021 10:45 AM
To: Nava kishore Manne <redacted>
Cc: Moritz Fischer <mdf@kernel.org>; trix@redhat.com;
robh+dt@kernel.org; Michal Simek [off-list ref]; linux-
fpga@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
kernel@lists.infradead.org; linux-kernel@vger.kernel.org; git
[off-list ref]; chinnikishore369@gmail.com
Subject: Re: [PATCH 2/2] fpga: Add support for Xilinx DFX AXI Shutdown
manager

On Tue, Jan 19, 2021 at 06:34:54AM +0000, Nava kishore Manne wrote:

quoted

Hi Moritz,

	Thanks for the review.
Please find my response inline.

quoted

-----Original Message-----
From: Moritz Fischer <mdf@kernel.org>
Sent: Saturday, January 16, 2021 8:28 AM
To: Nava kishore Manne <redacted>
Cc: mdf@kernel.org; trix@redhat.com; robh+dt@kernel.org; Michal
Simek [off-list ref]; linux-fpga@vger.kernel.org;
devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
linux- kernel@vger.kernel.org; git [off-list ref];
chinnikishore369@gmail.com
Subject: Re: [PATCH 2/2] fpga: Add support for Xilinx DFX AXI
Shutdown manager

Hi,

On Fri, Jan 15, 2021 at 07:04:31AM +0530, Nava kishore Manne wrote:

quoted

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.

quoted

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.

I'm a bit confused, the whole goal here is to give the thing a different

name?

quoted


Both the PR Decoupler and AXI Shutdown IP manager IP's are follows same

register spec.

quoted

Most of the code is common so we thought of reusing  same driver for AXI

shutdown manager as well.

What are the differences, though other than it shows a different name?

The PR-Decoupler And Axi Shut down manager are different IP's 

PR-Decoupler IP can be used to make the interface between a Reconfigurable Partition
and the static logic safe while Partial Reconfiguration is occurring. When active,
user-selected signals crossing between the Reconfigurable Partition and the static logic
are driven to user configurable values. When inactive, signals are passed unaltered.

Coming to the Axi Shutdown manager IP it's safely handles AXI4MM and AXI4-Lite interfaces
on a Reconfigurable Partition when it is undergoing dynamic reconfiguration (DFX),
preventing system deadlock that can occur if AXI transactions are interrupted by DFX.

For more details please refer the below links:
https://www.xilinx.com/support/documentation/ip_documentation/pr_decoupler/v1_0/pg227-pr-decoupler.pdf 
https://www.xilinx.com/support/documentation/ip_documentation/dfx_axi_shutdown_manager/v1_0/pg377-dfx-axi-shutdown-manager.pdf  

Both the IP functionality's are different but the IP's registers spec is same and also they fall under
the same bridge drivers category and most of the code is common except the naming conventions.
So we thought of using the same driver for both the IP's with different naming conventions.

Regards,
Navakishore.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help