linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 net-next 0/3] net: hns3: Refactors "reset" handling code in HCLGE layer of HNS3 driver
@ 2017-12-04  1:29 Salil Mehta
  2017-12-04  1:29 ` [PATCH V2 net-next 1/3] net: hns3: Refactor of the reset interrupt handling logic Salil Mehta
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Salil Mehta @ 2017-12-04  1:29 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil.lnk, netdev,
	linux-kernel, linux-rdma, linuxarm

This patch refactors the code of the reset feature in HCLGE layer
of HNS3 PF driver. Prime motivation to do this change is: 
1. To reduce the time for which common miscellaneous Vector 0
   interrupt is disabled because of the reset. Simplification
   of the common miscellaneous interrupt handler routine(for
   Vector 0) used to handle reset and other sources of Vector
   0 interrupt.
2. Separate the task for handling the reset
3. Simplification of reset request submission and pending reset
   logic.


To achieve above below few things have been done:
1. Interrupt is disabled while common miscellaneous interrupt
   handler is entered and re-enabled before it is exit. This
   reduces the interrupt handling latency as compared to older
   interrupt handling scheme where interrupt was being disabled
   in interrupt handler context and re-enabled in task context
   some time later. Made Miscellaneous interrupt handler more
   generic to handle all sources including reset interrupt source.
2. New reset service task has been introduced to service the 
   reset handling.
3. Introduces new reset service task for honoring software reset
   requests like from network stack related to timeout and serving
   the pending reset request(to reset the driver and associated
   clients).


Change Log:
Patch V2: Addressed comment by Andrew Lunn
   Link: https://lkml.org/lkml/2017/12/1/366
Patch V1: Initial Submit

Salil Mehta (3):
  net: hns3: Refactor of the reset interrupt handling logic
  net: hns3: Add reset service task for handling reset requests
  net: hns3: Refactors the requested reset & pending reset handling code

 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c    | 222 ++++++++++++++-------
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h    |  12 +-
 2 files changed, 166 insertions(+), 68 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH V2 net-next 1/3] net: hns3: Refactor of the reset interrupt handling logic
  2017-12-04  1:29 [PATCH V2 net-next 0/3] net: hns3: Refactors "reset" handling code in HCLGE layer of HNS3 driver Salil Mehta
@ 2017-12-04  1:29 ` Salil Mehta
  2017-12-04  1:29 ` [PATCH V2 net-next 2/3] net: hns3: Add reset service task for handling reset requests Salil Mehta
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Salil Mehta @ 2017-12-04  1:29 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil.lnk, netdev,
	linux-kernel, linux-rdma, linuxarm

The reset interrupt event shares common miscellaneous interrupt
Vector 0. In the existing reset interrupt handling we disable
the Vector 0 interrupt in misc interrupt handler and re-enable
them later in context to common service task.

This also means other event sources like mailbox would also be
deferred or if the interrupt event was due to mailbox(which shall
be supported for VF soon), it could delay the reset handling.

This patch reorganizes the reset interrupt handling logic and
makes it more fair to other events.

Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
Signed-off-by: lipeng <lipeng321@huawei.com>
---
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c    | 103 ++++++++++++++-------
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h    |   7 ++
 2 files changed, 78 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 59ed806a52c3..0c0543e84957 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -2362,6 +2362,46 @@ static void hclge_service_complete(struct hclge_dev *hdev)
 	clear_bit(HCLGE_STATE_SERVICE_SCHED, &hdev->state);
 }
 
+static u32 hclge_check_event_cause(struct hclge_dev *hdev, u32 *clearval)
+{
+	u32 rst_src_reg;
+
+	/* fetch the events from their corresponding regs */
+	rst_src_reg = hclge_read_dev(&hdev->hw, HCLGE_MISC_RESET_STS_REG);
+
+	/* check for vector0 reset event sources */
+	if (BIT(HCLGE_VECTOR0_GLOBALRESET_INT_B) & rst_src_reg) {
+		set_bit(HNAE3_GLOBAL_RESET, &hdev->reset_pending);
+		*clearval = BIT(HCLGE_VECTOR0_GLOBALRESET_INT_B);
+		return HCLGE_VECTOR0_EVENT_RST;
+	}
+
+	if (BIT(HCLGE_VECTOR0_CORERESET_INT_B) & rst_src_reg) {
+		set_bit(HNAE3_CORE_RESET, &hdev->reset_pending);
+		*clearval = BIT(HCLGE_VECTOR0_CORERESET_INT_B);
+		return HCLGE_VECTOR0_EVENT_RST;
+	}
+
+	if (BIT(HCLGE_VECTOR0_IMPRESET_INT_B) & rst_src_reg) {
+		set_bit(HNAE3_IMP_RESET, &hdev->reset_pending);
+		*clearval = BIT(HCLGE_VECTOR0_IMPRESET_INT_B);
+		return HCLGE_VECTOR0_EVENT_RST;
+	}
+
+	/* mailbox event sharing vector 0 interrupt would be placed here */
+
+	return HCLGE_VECTOR0_EVENT_OTHER;
+}
+
+static void hclge_clear_event_cause(struct hclge_dev *hdev, u32 event_type,
+				    u32 regclr)
+{
+	if (event_type == HCLGE_VECTOR0_EVENT_RST)
+		hclge_write_dev(&hdev->hw, HCLGE_MISC_RESET_STS_REG, regclr);
+
+	/* mailbox event sharing vector 0 interrupt would be placed here */
+}
+
 static void hclge_enable_vector(struct hclge_misc_vector *vector, bool enable)
 {
 	writel(enable ? 1 : 0, vector->addr);
@@ -2370,10 +2410,28 @@ static void hclge_enable_vector(struct hclge_misc_vector *vector, bool enable)
 static irqreturn_t hclge_misc_irq_handle(int irq, void *data)
 {
 	struct hclge_dev *hdev = data;
+	u32 event_cause;
+	u32 clearval;
 
 	hclge_enable_vector(&hdev->misc_vector, false);
-	if (!test_and_set_bit(HCLGE_STATE_SERVICE_SCHED, &hdev->state))
-		schedule_work(&hdev->service_task);
+	event_cause = hclge_check_event_cause(hdev, &clearval);
+
+	/* vector 0 interrupt is shared with reset and mailbox source events.
+	 * For now, we are not handling mailbox events.
+	 */
+	switch (event_cause) {
+	case HCLGE_VECTOR0_EVENT_RST:
+		/* reset task to be scheduled here */
+		break;
+	default:
+		dev_dbg(&hdev->pdev->dev,
+			"received unknown or unhandled event of vector0\n");
+		break;
+	}
+
+	/* we should clear the source of interrupt */
+	hclge_clear_event_cause(hdev, event_cause, clearval);
+	hclge_enable_vector(&hdev->misc_vector, true);
 
 	return IRQ_HANDLED;
 }
@@ -2404,9 +2462,9 @@ static int hclge_misc_irq_init(struct hclge_dev *hdev)
 
 	hclge_get_misc_vector(hdev);
 
-	ret = devm_request_irq(&hdev->pdev->dev,
-			       hdev->misc_vector.vector_irq,
-			       hclge_misc_irq_handle, 0, "hclge_misc", hdev);
+	/* this would be explicitly freed in the end */
+	ret = request_irq(hdev->misc_vector.vector_irq, hclge_misc_irq_handle,
+			  0, "hclge_misc", hdev);
 	if (ret) {
 		hclge_free_vector(hdev, 0);
 		dev_err(&hdev->pdev->dev, "request misc irq(%d) fail\n",
@@ -2416,6 +2474,12 @@ static int hclge_misc_irq_init(struct hclge_dev *hdev)
 	return ret;
 }
 
+static void hclge_misc_irq_uninit(struct hclge_dev *hdev)
+{
+	free_irq(hdev->misc_vector.vector_irq, hdev);
+	hclge_free_vector(hdev, 0);
+}
+
 static int hclge_notify_client(struct hclge_dev *hdev,
 			       enum hnae3_reset_notify_type type)
 {
@@ -2471,12 +2535,6 @@ static int hclge_reset_wait(struct hclge_dev *hdev)
 		cnt++;
 	}
 
-	/* must clear reset status register to
-	 * prevent driver detect reset interrupt again
-	 */
-	reg = hclge_read_dev(&hdev->hw, HCLGE_MISC_RESET_STS_REG);
-	hclge_write_dev(&hdev->hw, HCLGE_MISC_RESET_STS_REG, reg);
-
 	if (cnt >= HCLGE_RESET_WAIT_CNT) {
 		dev_warn(&hdev->pdev->dev,
 			 "Wait for reset timeout: %d\n", hdev->reset_type);
@@ -2534,22 +2592,6 @@ static void hclge_do_reset(struct hclge_dev *hdev, enum hnae3_reset_type type)
 	}
 }
 
-static enum hnae3_reset_type hclge_detected_reset_event(struct hclge_dev *hdev)
-{
-	enum hnae3_reset_type rst_level = HNAE3_NONE_RESET;
-	u32 rst_reg_val;
-
-	rst_reg_val = hclge_read_dev(&hdev->hw, HCLGE_MISC_RESET_STS_REG);
-	if (BIT(HCLGE_VECTOR0_GLOBALRESET_INT_B) & rst_reg_val)
-		rst_level = HNAE3_GLOBAL_RESET;
-	else if (BIT(HCLGE_VECTOR0_CORERESET_INT_B) & rst_reg_val)
-		rst_level = HNAE3_CORE_RESET;
-	else if (BIT(HCLGE_VECTOR0_IMPRESET_INT_B) & rst_reg_val)
-		rst_level = HNAE3_IMP_RESET;
-
-	return rst_level;
-}
-
 static void hclge_reset_event(struct hnae3_handle *handle,
 			      enum hnae3_reset_type reset)
 {
@@ -2584,9 +2626,6 @@ static void hclge_reset_subtask(struct hclge_dev *hdev)
 
 	do_reset = hdev->reset_type != HNAE3_NONE_RESET;
 
-	/* Reset is detected by interrupt */
-	if (hdev->reset_type == HNAE3_NONE_RESET)
-		hdev->reset_type = hclge_detected_reset_event(hdev);
 
 	if (hdev->reset_type == HNAE3_NONE_RESET)
 		return;
@@ -2622,7 +2661,6 @@ static void hclge_reset_subtask(struct hclge_dev *hdev)
 static void hclge_misc_irq_service_task(struct hclge_dev *hdev)
 {
 	hclge_reset_subtask(hdev);
-	hclge_enable_vector(&hdev->misc_vector, true);
 }
 
 static void hclge_service_task(struct work_struct *work)
@@ -4661,6 +4699,7 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
 	hdev->pdev = pdev;
 	hdev->ae_dev = ae_dev;
 	hdev->reset_type = HNAE3_NONE_RESET;
+	hdev->reset_pending = 0;
 	ae_dev->priv = hdev;
 
 	ret = hclge_pci_init(hdev);
@@ -4895,8 +4934,8 @@ static void hclge_uninit_ae_dev(struct hnae3_ae_dev *ae_dev)
 
 	/* Disable MISC vector(vector0) */
 	hclge_enable_vector(&hdev->misc_vector, false);
-	hclge_free_vector(hdev, 0);
 	hclge_destroy_cmd_queue(&hdev->hw);
+	hclge_misc_irq_uninit(hdev);
 	hclge_pci_uninit(hdev);
 	ae_dev->priv = NULL;
 }
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
index 7027814ea5d7..d4429496f4b5 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
@@ -105,6 +105,12 @@ enum HCLGE_DEV_STATE {
 	HCLGE_STATE_MAX
 };
 
+enum hclge_evt_cause {
+	HCLGE_VECTOR0_EVENT_RST,
+	HCLGE_VECTOR0_EVENT_MBX,
+	HCLGE_VECTOR0_EVENT_OTHER,
+};
+
 #define HCLGE_MPF_ENBALE 1
 struct hclge_caps {
 	u16 num_tqp;
@@ -420,6 +426,7 @@ struct hclge_dev {
 	unsigned long state;
 
 	enum hnae3_reset_type reset_type;
+	unsigned long reset_pending;	/* client rst is pending to be served */
 	u32 fw_version;
 	u16 num_vmdq_vport;		/* Num vmdq vport this PF has set up */
 	u16 num_tqps;			/* Num task queue pairs of this PF */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH V2 net-next 2/3] net: hns3: Add reset service task for handling reset requests
  2017-12-04  1:29 [PATCH V2 net-next 0/3] net: hns3: Refactors "reset" handling code in HCLGE layer of HNS3 driver Salil Mehta
  2017-12-04  1:29 ` [PATCH V2 net-next 1/3] net: hns3: Refactor of the reset interrupt handling logic Salil Mehta
@ 2017-12-04  1:29 ` Salil Mehta
  2017-12-04  1:29 ` [PATCH V2 net-next 3/3] net: hns3: Refactors the requested reset & pending reset handling code Salil Mehta
  2017-12-05 16:48 ` [PATCH V2 net-next 0/3] net: hns3: Refactors "reset" handling code in HCLGE layer of HNS3 driver David Miller
  3 siblings, 0 replies; 6+ messages in thread
From: Salil Mehta @ 2017-12-04  1:29 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil.lnk, netdev,
	linux-kernel, linux-rdma, linuxarm

Existing common service task was being used to service the reset
requests. This patch tries to make the handling of reset cleaner
by separating task to handle the reset requests. This might in
turn help in adapting similar handling approach for other
interrupt events like mailbox, sharing vector 0 interrupt.

Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
Signed-off-by: lipeng <lipeng321@huawei.com>
---
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c    | 41 ++++++++++++++++------
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h    |  4 +++
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 0c0543e84957..345868470201 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -2226,6 +2226,12 @@ static int hclge_mac_init(struct hclge_dev *hdev)
 	return hclge_cfg_func_mta_filter(hdev, 0, hdev->accept_mta_mc);
 }
 
+static void hclge_reset_task_schedule(struct hclge_dev *hdev)
+{
+	if (!test_and_set_bit(HCLGE_STATE_RST_SERVICE_SCHED, &hdev->state))
+		schedule_work(&hdev->rst_service_task);
+}
+
 static void hclge_task_schedule(struct hclge_dev *hdev)
 {
 	if (!test_bit(HCLGE_STATE_DOWN, &hdev->state) &&
@@ -2421,7 +2427,7 @@ static irqreturn_t hclge_misc_irq_handle(int irq, void *data)
 	 */
 	switch (event_cause) {
 	case HCLGE_VECTOR0_EVENT_RST:
-		/* reset task to be scheduled here */
+		hclge_reset_task_schedule(hdev);
 		break;
 	default:
 		dev_dbg(&hdev->pdev->dev,
@@ -2584,6 +2590,9 @@ static void hclge_do_reset(struct hclge_dev *hdev, enum hnae3_reset_type type)
 	case HNAE3_FUNC_RESET:
 		dev_info(&pdev->dev, "PF Reset requested\n");
 		hclge_func_reset_cmd(hdev, 0);
+		/* schedule again to check later */
+		set_bit(HNAE3_FUNC_RESET, &hdev->reset_pending);
+		hclge_reset_task_schedule(hdev);
 		break;
 	default:
 		dev_warn(&pdev->dev,
@@ -2605,14 +2614,9 @@ static void hclge_reset_event(struct hnae3_handle *handle,
 	case HNAE3_FUNC_RESET:
 	case HNAE3_CORE_RESET:
 	case HNAE3_GLOBAL_RESET:
-		if (test_bit(HCLGE_STATE_RESET_INT, &hdev->state)) {
-			dev_err(&hdev->pdev->dev, "Already in reset state");
-			return;
-		}
-		hdev->reset_type = reset;
-		set_bit(HCLGE_STATE_RESET_INT, &hdev->state);
-		set_bit(HCLGE_STATE_SERVICE_SCHED, &hdev->state);
-		schedule_work(&hdev->service_task);
+		/* request reset & schedule reset task */
+		set_bit(reset, &hdev->reset_request);
+		hclge_reset_task_schedule(hdev);
 		break;
 	default:
 		dev_warn(&hdev->pdev->dev, "Unsupported reset event:%d", reset);
@@ -2658,9 +2662,19 @@ static void hclge_reset_subtask(struct hclge_dev *hdev)
 	hdev->reset_type = HNAE3_NONE_RESET;
 }
 
-static void hclge_misc_irq_service_task(struct hclge_dev *hdev)
+static void hclge_reset_service_task(struct work_struct *work)
 {
+	struct hclge_dev *hdev =
+		container_of(work, struct hclge_dev, rst_service_task);
+
+	if (test_and_set_bit(HCLGE_STATE_RST_HANDLING, &hdev->state))
+		return;
+
+	clear_bit(HCLGE_STATE_RST_SERVICE_SCHED, &hdev->state);
+
 	hclge_reset_subtask(hdev);
+
+	clear_bit(HCLGE_STATE_RST_HANDLING, &hdev->state);
 }
 
 static void hclge_service_task(struct work_struct *work)
@@ -2668,7 +2682,6 @@ static void hclge_service_task(struct work_struct *work)
 	struct hclge_dev *hdev =
 		container_of(work, struct hclge_dev, service_task);
 
-	hclge_misc_irq_service_task(hdev);
 	hclge_update_speed_duplex(hdev);
 	hclge_update_link_status(hdev);
 	hclge_update_stats_for_all(hdev);
@@ -4699,6 +4712,7 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
 	hdev->pdev = pdev;
 	hdev->ae_dev = ae_dev;
 	hdev->reset_type = HNAE3_NONE_RESET;
+	hdev->reset_request = 0;
 	hdev->reset_pending = 0;
 	ae_dev->priv = hdev;
 
@@ -4811,12 +4825,15 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
 
 	timer_setup(&hdev->service_timer, hclge_service_timer, 0);
 	INIT_WORK(&hdev->service_task, hclge_service_task);
+	INIT_WORK(&hdev->rst_service_task, hclge_reset_service_task);
 
 	/* Enable MISC vector(vector0) */
 	hclge_enable_vector(&hdev->misc_vector, true);
 
 	set_bit(HCLGE_STATE_SERVICE_INITED, &hdev->state);
 	set_bit(HCLGE_STATE_DOWN, &hdev->state);
+	clear_bit(HCLGE_STATE_RST_SERVICE_SCHED, &hdev->state);
+	clear_bit(HCLGE_STATE_RST_HANDLING, &hdev->state);
 
 	pr_info("%s driver initialization finished.\n", HCLGE_DRIVER_NAME);
 	return 0;
@@ -4928,6 +4945,8 @@ static void hclge_uninit_ae_dev(struct hnae3_ae_dev *ae_dev)
 		del_timer_sync(&hdev->service_timer);
 	if (hdev->service_task.func)
 		cancel_work_sync(&hdev->service_task);
+	if (hdev->rst_service_task.func)
+		cancel_work_sync(&hdev->rst_service_task);
 
 	if (mac->phydev)
 		mdiobus_unregister(mac->mdio_bus);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
index d4429496f4b5..fd04fd23b76a 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
@@ -99,6 +99,8 @@ enum HCLGE_DEV_STATE {
 	HCLGE_STATE_REMOVING,
 	HCLGE_STATE_SERVICE_INITED,
 	HCLGE_STATE_SERVICE_SCHED,
+	HCLGE_STATE_RST_SERVICE_SCHED,
+	HCLGE_STATE_RST_HANDLING,
 	HCLGE_STATE_MBX_HANDLING,
 	HCLGE_STATE_MBX_IRQ,
 	HCLGE_STATE_RESET_INT,
@@ -426,6 +428,7 @@ struct hclge_dev {
 	unsigned long state;
 
 	enum hnae3_reset_type reset_type;
+	unsigned long reset_request;	/* reset has been requested */
 	unsigned long reset_pending;	/* client rst is pending to be served */
 	u32 fw_version;
 	u16 num_vmdq_vport;		/* Num vmdq vport this PF has set up */
@@ -476,6 +479,7 @@ struct hclge_dev {
 	unsigned long service_timer_previous;
 	struct timer_list service_timer;
 	struct work_struct service_task;
+	struct work_struct rst_service_task;
 
 	bool cur_promisc;
 	int num_alloc_vfs;	/* Actual number of VFs allocated */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH V2 net-next 3/3] net: hns3: Refactors the requested reset & pending reset handling code
  2017-12-04  1:29 [PATCH V2 net-next 0/3] net: hns3: Refactors "reset" handling code in HCLGE layer of HNS3 driver Salil Mehta
  2017-12-04  1:29 ` [PATCH V2 net-next 1/3] net: hns3: Refactor of the reset interrupt handling logic Salil Mehta
  2017-12-04  1:29 ` [PATCH V2 net-next 2/3] net: hns3: Add reset service task for handling reset requests Salil Mehta
@ 2017-12-04  1:29 ` Salil Mehta
  2017-12-05 16:48 ` [PATCH V2 net-next 0/3] net: hns3: Refactors "reset" handling code in HCLGE layer of HNS3 driver David Miller
  3 siblings, 0 replies; 6+ messages in thread
From: Salil Mehta @ 2017-12-04  1:29 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil.lnk, netdev,
	linux-kernel, linux-rdma, linuxarm

In exisiting code, the way to detect if driver/client reset should
be executed or if hardware should be be soft resetted was overly
complex.

Existing code use to read the interrupt status register from task
context to figure out if the interrupt source event was reset and
then use clear the interrupt source for reset while waiting for the
hardware to finish the reset. This behaviour again was confusing
and overly complex in terms of the flow.

This patch simplifies the handling of the requested reset and the
pending reset(i.e. reset which have already been asserted by the
software and hardware has acknowledged back to driver that it is
processing the hardware reset through interrupt)

Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
Signed-off-by: lipeng <lipeng321@huawei.com>
---
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c    | 100 +++++++++++++--------
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.h    |   1 -
 2 files changed, 65 insertions(+), 36 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 345868470201..d07c700c7ff8 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -17,7 +17,7 @@
 #include <linux/netdevice.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
-
+#include <net/rtnetlink.h>
 #include "hclge_cmd.h"
 #include "hclge_dcb.h"
 #include "hclge_main.h"
@@ -2569,12 +2569,12 @@ static int hclge_func_reset_cmd(struct hclge_dev *hdev, int func_id)
 	return ret;
 }
 
-static void hclge_do_reset(struct hclge_dev *hdev, enum hnae3_reset_type type)
+static void hclge_do_reset(struct hclge_dev *hdev)
 {
 	struct pci_dev *pdev = hdev->pdev;
 	u32 val;
 
-	switch (type) {
+	switch (hdev->reset_type) {
 	case HNAE3_GLOBAL_RESET:
 		val = hclge_read_dev(&hdev->hw, HCLGE_GLOBAL_RESET_REG);
 		hnae_set_bit(val, HCLGE_GLOBAL_RESET_BIT, 1);
@@ -2596,11 +2596,56 @@ static void hclge_do_reset(struct hclge_dev *hdev, enum hnae3_reset_type type)
 		break;
 	default:
 		dev_warn(&pdev->dev,
-			 "Unsupported reset type: %d\n", type);
+			 "Unsupported reset type: %d\n", hdev->reset_type);
 		break;
 	}
 }
 
+static enum hnae3_reset_type hclge_get_reset_level(struct hclge_dev *hdev,
+						   unsigned long *addr)
+{
+	enum hnae3_reset_type rst_level = HNAE3_NONE_RESET;
+
+	/* return the highest priority reset level amongst all */
+	if (test_bit(HNAE3_GLOBAL_RESET, addr))
+		rst_level = HNAE3_GLOBAL_RESET;
+	else if (test_bit(HNAE3_CORE_RESET, addr))
+		rst_level = HNAE3_CORE_RESET;
+	else if (test_bit(HNAE3_IMP_RESET, addr))
+		rst_level = HNAE3_IMP_RESET;
+	else if (test_bit(HNAE3_FUNC_RESET, addr))
+		rst_level = HNAE3_FUNC_RESET;
+
+	/* now, clear all other resets */
+	clear_bit(HNAE3_GLOBAL_RESET, addr);
+	clear_bit(HNAE3_CORE_RESET, addr);
+	clear_bit(HNAE3_IMP_RESET, addr);
+	clear_bit(HNAE3_FUNC_RESET, addr);
+
+	return rst_level;
+}
+
+static void hclge_reset(struct hclge_dev *hdev)
+{
+	/* perform reset of the stack & ae device for a client */
+
+	hclge_notify_client(hdev, HNAE3_DOWN_CLIENT);
+
+	if (!hclge_reset_wait(hdev)) {
+		rtnl_lock();
+		hclge_notify_client(hdev, HNAE3_UNINIT_CLIENT);
+		hclge_reset_ae_dev(hdev->ae_dev);
+		hclge_notify_client(hdev, HNAE3_INIT_CLIENT);
+		rtnl_unlock();
+	} else {
+		/* schedule again to check pending resets later */
+		set_bit(hdev->reset_type, &hdev->reset_pending);
+		hclge_reset_task_schedule(hdev);
+	}
+
+	hclge_notify_client(hdev, HNAE3_UP_CLIENT);
+}
+
 static void hclge_reset_event(struct hnae3_handle *handle,
 			      enum hnae3_reset_type reset)
 {
@@ -2626,39 +2671,24 @@ static void hclge_reset_event(struct hnae3_handle *handle,
 
 static void hclge_reset_subtask(struct hclge_dev *hdev)
 {
-	bool do_reset;
-
-	do_reset = hdev->reset_type != HNAE3_NONE_RESET;
-
-
-	if (hdev->reset_type == HNAE3_NONE_RESET)
-		return;
-
-	switch (hdev->reset_type) {
-	case HNAE3_FUNC_RESET:
-	case HNAE3_CORE_RESET:
-	case HNAE3_GLOBAL_RESET:
-	case HNAE3_IMP_RESET:
-		hclge_notify_client(hdev, HNAE3_DOWN_CLIENT);
+	/* check if there is any ongoing reset in the hardware. This status can
+	 * be checked from reset_pending. If there is then, we need to wait for
+	 * hardware to complete reset.
+	 *    a. If we are able to figure out in reasonable time that hardware
+	 *       has fully resetted then, we can proceed with driver, client
+	 *       reset.
+	 *    b. else, we can come back later to check this status so re-sched
+	 *       now.
+	 */
+	hdev->reset_type = hclge_get_reset_level(hdev, &hdev->reset_pending);
+	if (hdev->reset_type != HNAE3_NONE_RESET)
+		hclge_reset(hdev);
 
-		if (do_reset)
-			hclge_do_reset(hdev, hdev->reset_type);
-		else
-			set_bit(HCLGE_STATE_RESET_INT, &hdev->state);
+	/* check if we got any *new* reset requests to be honored */
+	hdev->reset_type = hclge_get_reset_level(hdev, &hdev->reset_request);
+	if (hdev->reset_type != HNAE3_NONE_RESET)
+		hclge_do_reset(hdev);
 
-		if (!hclge_reset_wait(hdev)) {
-			hclge_notify_client(hdev, HNAE3_UNINIT_CLIENT);
-			hclge_reset_ae_dev(hdev->ae_dev);
-			hclge_notify_client(hdev, HNAE3_INIT_CLIENT);
-			clear_bit(HCLGE_STATE_RESET_INT, &hdev->state);
-		}
-		hclge_notify_client(hdev, HNAE3_UP_CLIENT);
-		break;
-	default:
-		dev_err(&hdev->pdev->dev, "Unsupported reset type:%d\n",
-			hdev->reset_type);
-		break;
-	}
 	hdev->reset_type = HNAE3_NONE_RESET;
 }
 
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
index fd04fd23b76a..aacec438b933 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
@@ -103,7 +103,6 @@ enum HCLGE_DEV_STATE {
 	HCLGE_STATE_RST_HANDLING,
 	HCLGE_STATE_MBX_HANDLING,
 	HCLGE_STATE_MBX_IRQ,
-	HCLGE_STATE_RESET_INT,
 	HCLGE_STATE_MAX
 };
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V2 net-next 0/3] net: hns3: Refactors "reset" handling code in HCLGE layer of HNS3 driver
  2017-12-04  1:29 [PATCH V2 net-next 0/3] net: hns3: Refactors "reset" handling code in HCLGE layer of HNS3 driver Salil Mehta
                   ` (2 preceding siblings ...)
  2017-12-04  1:29 ` [PATCH V2 net-next 3/3] net: hns3: Refactors the requested reset & pending reset handling code Salil Mehta
@ 2017-12-05 16:48 ` David Miller
  2017-12-05 16:59   ` Salil Mehta
  3 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2017-12-05 16:48 UTC (permalink / raw)
  To: salil.mehta
  Cc: yisen.zhuang, lipeng321, mehta.salil.lnk, netdev, linux-kernel,
	linux-rdma, linuxarm

From: Salil Mehta <salil.mehta@huawei.com>
Date: Mon, 4 Dec 2017 01:29:52 +0000

> This patch refactors the code of the reset feature in HCLGE layer
> of HNS3 PF driver. Prime motivation to do this change is: 
> 1. To reduce the time for which common miscellaneous Vector 0
>    interrupt is disabled because of the reset. Simplification
>    of the common miscellaneous interrupt handler routine(for
>    Vector 0) used to handle reset and other sources of Vector
>    0 interrupt.
> 2. Separate the task for handling the reset
> 3. Simplification of reset request submission and pending reset
>    logic.
> 
> 
> To achieve above below few things have been done:
> 1. Interrupt is disabled while common miscellaneous interrupt
>    handler is entered and re-enabled before it is exit. This
>    reduces the interrupt handling latency as compared to older
>    interrupt handling scheme where interrupt was being disabled
>    in interrupt handler context and re-enabled in task context
>    some time later. Made Miscellaneous interrupt handler more
>    generic to handle all sources including reset interrupt source.
> 2. New reset service task has been introduced to service the 
>    reset handling.
> 3. Introduces new reset service task for honoring software reset
>    requests like from network stack related to timeout and serving
>    the pending reset request(to reset the driver and associated
>    clients).
> 
> 
> Change Log:
> Patch V2: Addressed comment by Andrew Lunn
>    Link: https://lkml.org/lkml/2017/12/1/366
> Patch V1: Initial Submit

Series applied, thank you.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH V2 net-next 0/3] net: hns3: Refactors "reset" handling code in HCLGE layer of HNS3 driver
  2017-12-05 16:48 ` [PATCH V2 net-next 0/3] net: hns3: Refactors "reset" handling code in HCLGE layer of HNS3 driver David Miller
@ 2017-12-05 16:59   ` Salil Mehta
  0 siblings, 0 replies; 6+ messages in thread
From: Salil Mehta @ 2017-12-05 16:59 UTC (permalink / raw)
  To: David Miller
  Cc: Zhuangyuzeng (Yisen), lipeng (Y),
	mehta.salil.lnk, netdev, linux-kernel, linux-rdma, Linuxarm

> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Tuesday, December 05, 2017 4:49 PM
> To: Salil Mehta <salil.mehta@huawei.com>
> Cc: Zhuangyuzeng (Yisen) <yisen.zhuang@huawei.com>; lipeng (Y)
> <lipeng321@huawei.com>; mehta.salil.lnk@gmail.com;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> rdma@vger.kernel.org; Linuxarm <linuxarm@huawei.com>
> Subject: Re: [PATCH V2 net-next 0/3] net: hns3: Refactors "reset"
> handling code in HCLGE layer of HNS3 driver
> 
> From: Salil Mehta <salil.mehta@huawei.com>
> Date: Mon, 4 Dec 2017 01:29:52 +0000
> 
> > This patch refactors the code of the reset feature in HCLGE layer
> > of HNS3 PF driver. Prime motivation to do this change is:
> > 1. To reduce the time for which common miscellaneous Vector 0
> >    interrupt is disabled because of the reset. Simplification
> >    of the common miscellaneous interrupt handler routine(for
> >    Vector 0) used to handle reset and other sources of Vector
> >    0 interrupt.
> > 2. Separate the task for handling the reset
> > 3. Simplification of reset request submission and pending reset
> >    logic.
> >
> >
> > To achieve above below few things have been done:
> > 1. Interrupt is disabled while common miscellaneous interrupt
> >    handler is entered and re-enabled before it is exit. This
> >    reduces the interrupt handling latency as compared to older
> >    interrupt handling scheme where interrupt was being disabled
> >    in interrupt handler context and re-enabled in task context
> >    some time later. Made Miscellaneous interrupt handler more
> >    generic to handle all sources including reset interrupt source.
> > 2. New reset service task has been introduced to service the
> >    reset handling.
> > 3. Introduces new reset service task for honoring software reset
> >    requests like from network stack related to timeout and serving
> >    the pending reset request(to reset the driver and associated
> >    clients).
> >
> >
> > Change Log:
> > Patch V2: Addressed comment by Andrew Lunn
> >    Link: https://lkml.org/lkml/2017/12/1/366
> > Patch V1: Initial Submit
> 
> Series applied, thank you.

Thanks Dave.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-12-05 16:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-04  1:29 [PATCH V2 net-next 0/3] net: hns3: Refactors "reset" handling code in HCLGE layer of HNS3 driver Salil Mehta
2017-12-04  1:29 ` [PATCH V2 net-next 1/3] net: hns3: Refactor of the reset interrupt handling logic Salil Mehta
2017-12-04  1:29 ` [PATCH V2 net-next 2/3] net: hns3: Add reset service task for handling reset requests Salil Mehta
2017-12-04  1:29 ` [PATCH V2 net-next 3/3] net: hns3: Refactors the requested reset & pending reset handling code Salil Mehta
2017-12-05 16:48 ` [PATCH V2 net-next 0/3] net: hns3: Refactors "reset" handling code in HCLGE layer of HNS3 driver David Miller
2017-12-05 16:59   ` Salil Mehta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).