diff mbox series

[v2,08/16] PCI: epf-test: Simplify IRQ test commands execution

Message ID 20230308090313.1653-9-damien.lemoal@opensource.wdc.com (mailing list archive)
State Superseded
Headers show
Series PCI endpoint fixes and improvements | expand

Commit Message

Damien Le Moal March 8, 2023, 9:03 a.m. UTC 
For the commands COMMAND_RAISE_LEGACY_IRQ, COMMAND_RAISE_MSI_IRQ and
COMMAND_RAISE_MSIX_IRQ, the function pci_epf_test_cmd_handler()
sets the STATUS_IRQ_RAISED status flag and calls the epc function
pci_epc_raise_irq() directly. However, this is also exactly what the
pci_epf_test_raise_irq() function does. Avoid duplicating these
operations by directly using pci_epf_test_raise_irq() for the IRQ test
commands. It is OK to do so as the host side endpoint test driver always
set the correct irq type for the IRQ test commands.

At the same time, the irq number check done for the
COMMAND_RAISE_MSI_IRQ and COMMAND_RAISE_MSIX_IRQ commands can also be
moved to pci_epf_test_raise_irq() to also check the IRQ number requested
by the host for other test commands.

Overall, this significantly simplifies the pci_epf_test_cmd_handler()
function.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/pci/endpoint/functions/pci-epf-test.c | 43 ++++++++-----------
 1 file changed, 17 insertions(+), 26 deletions(-)

Comments

Manivannan Sadhasivam March 15, 2023, 3:37 p.m. UTC | #1 
On Wed, Mar 08, 2023 at 06:03:05PM +0900, Damien Le Moal wrote:
> For the commands COMMAND_RAISE_LEGACY_IRQ, COMMAND_RAISE_MSI_IRQ and
> COMMAND_RAISE_MSIX_IRQ, the function pci_epf_test_cmd_handler()
> sets the STATUS_IRQ_RAISED status flag and calls the epc function
> pci_epc_raise_irq() directly. However, this is also exactly what the
> pci_epf_test_raise_irq() function does. Avoid duplicating these
> operations by directly using pci_epf_test_raise_irq() for the IRQ test
> commands. It is OK to do so as the host side endpoint test driver always
> set the correct irq type for the IRQ test commands.
> 
> At the same time, the irq number check done for the
> COMMAND_RAISE_MSI_IRQ and COMMAND_RAISE_MSIX_IRQ commands can also be
> moved to pci_epf_test_raise_irq() to also check the IRQ number requested
> by the host for other test commands.
> 
> Overall, this significantly simplifies the pci_epf_test_cmd_handler()
> function.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

Thanks,
Mani

> ---
>  drivers/pci/endpoint/functions/pci-epf-test.c | 43 ++++++++-----------
>  1 file changed, 17 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 6f4ef5251452..43d623682850 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -615,6 +615,7 @@ static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test,
>  	struct pci_epf *epf = epf_test->epf;
>  	struct device *dev = &epf->dev;
>  	struct pci_epc *epc = epf->epc;
> +	int count;
>  
>  	reg->status |= STATUS_IRQ_RAISED;
>  
> @@ -624,10 +625,22 @@ static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test,
>  				  PCI_EPC_IRQ_LEGACY, 0);
>  		break;
>  	case IRQ_TYPE_MSI:
> +		count = pci_epc_get_msi(epc, epf->func_no, epf->vfunc_no);
> +		if (reg->irq_number > count || count <= 0) {
> +			dev_err(dev, "Invalid MSI IRQ number %d / %d\n",
> +				reg->irq_number, count);
> +			return;
> +		}
>  		pci_epc_raise_irq(epc, epf->func_no, epf->vfunc_no,
>  				  PCI_EPC_IRQ_MSI, reg->irq_number);
>  		break;
>  	case IRQ_TYPE_MSIX:
> +		count = pci_epc_get_msix(epc, epf->func_no, epf->vfunc_no);
> +		if (reg->irq_number > count || count <= 0) {
> +			dev_err(dev, "Invalid MSIX IRQ number %d / %d\n",
> +				reg->irq_number, count);
> +			return;
> +		}
>  		pci_epc_raise_irq(epc, epf->func_no, epf->vfunc_no,
>  				  PCI_EPC_IRQ_MSIX, reg->irq_number);
>  		break;
> @@ -640,13 +653,11 @@ static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test,
>  static void pci_epf_test_cmd_handler(struct work_struct *work)
>  {
>  	int ret;
> -	int count;
>  	u32 command;
>  	struct pci_epf_test *epf_test = container_of(work, struct pci_epf_test,
>  						     cmd_handler.work);
>  	struct pci_epf *epf = epf_test->epf;
>  	struct device *dev = &epf->dev;
> -	struct pci_epc *epc = epf->epc;
>  	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
>  	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
>  
> @@ -662,10 +673,10 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
>  		goto reset_handler;
>  	}
>  
> -	if (command & COMMAND_RAISE_LEGACY_IRQ) {
> -		reg->status = STATUS_IRQ_RAISED;
> -		pci_epc_raise_irq(epc, epf->func_no, epf->vfunc_no,
> -				  PCI_EPC_IRQ_LEGACY, 0);
> +	if ((command & COMMAND_RAISE_LEGACY_IRQ) ||
> +	    (command & COMMAND_RAISE_MSI_IRQ) ||
> +	    (command & COMMAND_RAISE_MSIX_IRQ)) {
> +		pci_epf_test_raise_irq(epf_test, reg);
>  		goto reset_handler;
>  	}
>  
> @@ -699,26 +710,6 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
>  		goto reset_handler;
>  	}
>  
> -	if (command & COMMAND_RAISE_MSI_IRQ) {
> -		count = pci_epc_get_msi(epc, epf->func_no, epf->vfunc_no);
> -		if (reg->irq_number > count || count <= 0)
> -			goto reset_handler;
> -		reg->status = STATUS_IRQ_RAISED;
> -		pci_epc_raise_irq(epc, epf->func_no, epf->vfunc_no,
> -				  PCI_EPC_IRQ_MSI, reg->irq_number);
> -		goto reset_handler;
> -	}
> -
> -	if (command & COMMAND_RAISE_MSIX_IRQ) {
> -		count = pci_epc_get_msix(epc, epf->func_no, epf->vfunc_no);
> -		if (reg->irq_number > count || count <= 0)
> -			goto reset_handler;
> -		reg->status = STATUS_IRQ_RAISED;
> -		pci_epc_raise_irq(epc, epf->func_no, epf->vfunc_no,
> -				  PCI_EPC_IRQ_MSIX, reg->irq_number);
> -		goto reset_handler;
> -	}
> -
>  reset_handler:
>  	queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>  			   msecs_to_jiffies(1));
> -- 
> 2.39.2
>
diff mbox series

Patch

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 6f4ef5251452..43d623682850 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -615,6 +615,7 @@  static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test,
 	struct pci_epf *epf = epf_test->epf;
 	struct device *dev = &epf->dev;
 	struct pci_epc *epc = epf->epc;
+	int count;
 
 	reg->status |= STATUS_IRQ_RAISED;
 
@@ -624,10 +625,22 @@  static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test,
 				  PCI_EPC_IRQ_LEGACY, 0);
 		break;
 	case IRQ_TYPE_MSI:
+		count = pci_epc_get_msi(epc, epf->func_no, epf->vfunc_no);
+		if (reg->irq_number > count || count <= 0) {
+			dev_err(dev, "Invalid MSI IRQ number %d / %d\n",
+				reg->irq_number, count);
+			return;
+		}
 		pci_epc_raise_irq(epc, epf->func_no, epf->vfunc_no,
 				  PCI_EPC_IRQ_MSI, reg->irq_number);
 		break;
 	case IRQ_TYPE_MSIX:
+		count = pci_epc_get_msix(epc, epf->func_no, epf->vfunc_no);
+		if (reg->irq_number > count || count <= 0) {
+			dev_err(dev, "Invalid MSIX IRQ number %d / %d\n",
+				reg->irq_number, count);
+			return;
+		}
 		pci_epc_raise_irq(epc, epf->func_no, epf->vfunc_no,
 				  PCI_EPC_IRQ_MSIX, reg->irq_number);
 		break;
@@ -640,13 +653,11 @@  static void pci_epf_test_raise_irq(struct pci_epf_test *epf_test,
 static void pci_epf_test_cmd_handler(struct work_struct *work)
 {
 	int ret;
-	int count;
 	u32 command;
 	struct pci_epf_test *epf_test = container_of(work, struct pci_epf_test,
 						     cmd_handler.work);
 	struct pci_epf *epf = epf_test->epf;
 	struct device *dev = &epf->dev;
-	struct pci_epc *epc = epf->epc;
 	enum pci_barno test_reg_bar = epf_test->test_reg_bar;
 	struct pci_epf_test_reg *reg = epf_test->reg[test_reg_bar];
 
@@ -662,10 +673,10 @@  static void pci_epf_test_cmd_handler(struct work_struct *work)
 		goto reset_handler;
 	}
 
-	if (command & COMMAND_RAISE_LEGACY_IRQ) {
-		reg->status = STATUS_IRQ_RAISED;
-		pci_epc_raise_irq(epc, epf->func_no, epf->vfunc_no,
-				  PCI_EPC_IRQ_LEGACY, 0);
+	if ((command & COMMAND_RAISE_LEGACY_IRQ) ||
+	    (command & COMMAND_RAISE_MSI_IRQ) ||
+	    (command & COMMAND_RAISE_MSIX_IRQ)) {
+		pci_epf_test_raise_irq(epf_test, reg);
 		goto reset_handler;
 	}
 
@@ -699,26 +710,6 @@  static void pci_epf_test_cmd_handler(struct work_struct *work)
 		goto reset_handler;
 	}
 
-	if (command & COMMAND_RAISE_MSI_IRQ) {
-		count = pci_epc_get_msi(epc, epf->func_no, epf->vfunc_no);
-		if (reg->irq_number > count || count <= 0)
-			goto reset_handler;
-		reg->status = STATUS_IRQ_RAISED;
-		pci_epc_raise_irq(epc, epf->func_no, epf->vfunc_no,
-				  PCI_EPC_IRQ_MSI, reg->irq_number);
-		goto reset_handler;
-	}
-
-	if (command & COMMAND_RAISE_MSIX_IRQ) {
-		count = pci_epc_get_msix(epc, epf->func_no, epf->vfunc_no);
-		if (reg->irq_number > count || count <= 0)
-			goto reset_handler;
-		reg->status = STATUS_IRQ_RAISED;
-		pci_epc_raise_irq(epc, epf->func_no, epf->vfunc_no,
-				  PCI_EPC_IRQ_MSIX, reg->irq_number);
-		goto reset_handler;
-	}
-
 reset_handler:
 	queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
 			   msecs_to_jiffies(1));