Re: [PATCH net v2 1/3] net: hns: support deferred probe when can not obtain irq

[lipeng (Y)]

On 2017/4/24 22:47, Matthias Brugger wrote:

On 24/04/17 13:43, lipeng (Y) wrote:

On 2017/4/24 18:28, Matthias Brugger wrote:

On 21/04/17 09:44, Yankejian wrote:

From: lipeng <lipeng...@huawei.com>

In the hip06 and hip07 SoCs, the interrupt lines from the
DSAF controllers are connected to mbigen hw module.
The mbigen module is probed with module_init, and, as such,
is not guaranteed to probe before the HNS driver. So we need
to support deferred probe.

We check for probe deferral in the hw layer probe, so we not
probe into the main layer and memories, etc., to later learn
that we need to defer the probe.

Why? This looks like a hack.
From what I see, we can handle EPROBE_DEFER easily inside hns_ppe_init
checking the return value of hns_rcb_get_cfg. Like you do in 2/3 of
this series.

Regards,
Matthias

Hi Matthias,

mdio && phy is not necessary condition, and port can work well  for port
+ SFP (without mdio &&phy).

BUT irq is the necessary condition,  port can not work well without irq.

So, I check IRQ first,and do not probe dsaf if can't obtain irq(1/3 of

this series), and check mdio only when there is phy(2/3 of this series).

And thanks for your review.

I think I didn't explained myself good enough.
I was suggesting the following (not even compile tested):

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c

index eba406bea52f..be38d47bc399 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_ppe.c
@@ -510,7 +510,9 @@ int hns_ppe_init(struct dsaf_device *dsaf_dev)

                hns_ppe_get_cfg(dsaf_dev->ppe_common[i]);

-               hns_rcb_get_cfg(dsaf_dev->rcb_common[i]);
+               ret = hns_rcb_get_cfg(dsaf_dev->rcb_common[i]);
+               if (reg < 0)
+                       goto get_cfg_fail;
        }

        for (i = 0; i < HNS_PPE_COM_NUM; i++)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c

index c20a0f4f8f02..c7e801d0c3b7 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_rcb.c

@@ -492,7 +492,7 @@ static int hns_rcb_get_base_irq_idx(struct rcb_common_cb *rcb_common)

  *hns_rcb_get_cfg - get rcb config
  *@rcb_common: rcb common device
  */
-void hns_rcb_get_cfg(struct rcb_common_cb *rcb_common)
+int hns_rcb_get_cfg(struct rcb_common_cb *rcb_common)
 {
        struct ring_pair_cb *ring_pair_cb;
        u32 i;

@@ -517,10 +517,18 @@ void hns_rcb_get_cfg(struct rcb_common_cb *rcb_common)

                ring_pair_cb->virq[HNS_RCB_IRQ_IDX_RX] =

is_ver1 ? platform_get_irq(pdev, base_irq_idx + i * 2 + 1) :

                          platform_get_irq(pdev, base_irq_idx + i * 3);
+

+ if ((ring_pair_cb->virq[HNS_RCB_IRQ_IDX_TX] == -EPROBE_DEFER) || + (ring_pair_cb->virq[HNS_RCB_IRQ_IDX_RX] == -EPROBE_DEFER)) {

+                       return -EPROBE_DEFER;
+               }
+
                ring_pair_cb->q.phy_base =

RCB_COMM_BASE_TO_RING_BASE(rcb_common->phy_base, i);
                hns_rcb_ring_pair_get_cfg(ring_pair_cb);
        }
+
+       return 0;
 }

 /**


Regards,
Matthias

Thanks,  I will take your advice and test it.

lipeng

Signed-off-by: lipeng <lipeng...@huawei.com>
Reviewed-by: Yisen Zhuang <yisen.zhu...@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
index 403ea9d..2da5b42 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c
@@ -2971,6 +2971,18 @@ static int hns_dsaf_probe(struct
platform_device *pdev)
     struct dsaf_device *dsaf_dev;
     int ret;

+    /*
+     * Check if we should defer the probe before we probe the
+     * dsaf, as it's hard to defer later on.
+     */
+    ret = platform_get_irq(pdev, 0);
+    if (ret < 0) {
+        if (ret != -EPROBE_DEFER)
+            dev_err(&pdev->dev, "Cannot obtain irq\n");
+
+        return ret;
+    }
+
     dsaf_dev = hns_dsaf_alloc_dev(&pdev->dev, sizeof(struct
dsaf_drv_priv));
     if (IS_ERR(dsaf_dev)) {
         ret = PTR_ERR(dsaf_dev);

.

.