On Thu, Jun 16, 2016 at 10:35:12PM +0800, Lijun Ou wrote: > This patch mainly added reset flow of RoCE engine in RoCE > driver. It is necessary when RoCE is loaded and removed. > > Signed-off-by: Wei Hu > Signed-off-by: Nenglong Zhao > Signed-off-by: Lijun Ou > --- > PATCH v9/v8: > - No change over the PATCH v7 > > PATCH v7: > This fixes the comments given by Leon Romanovsky over the PATCH v6: > Link: https://lkml.org/lkml/2016/5/3/733 > > PATCH v6: > - No change over the PATCH v5 > > PATCH v5: > - The initial patch which was redesigned based on the second patch > in PATCH v4 > --- > --- > drivers/infiniband/hw/hns/hns_roce_device.h | 7 +++ > drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 72 +++++++++++++++++++++++++++++ > drivers/infiniband/hw/hns/hns_roce_hw_v1.h | 40 ++++++++++++++++ > drivers/infiniband/hw/hns/hns_roce_main.c | 17 ++++++- > 4 files changed, 135 insertions(+), 1 deletion(-) > create mode 100644 drivers/infiniband/hw/hns/hns_roce_hw_v1.c > create mode 100644 drivers/infiniband/hw/hns/hns_roce_hw_v1.h > > diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h > index 946b470..b857c76 100644 > --- a/drivers/infiniband/hw/hns/hns_roce_device.h > +++ b/drivers/infiniband/hw/hns/hns_roce_device.h > @@ -56,6 +56,10 @@ struct hns_roce_caps { > u8 num_ports; > }; > > +struct hns_roce_hw { > + int (*reset)(struct hns_roce_dev *hr_dev, bool enable); > +}; > + > struct hns_roce_dev { > struct ib_device ib_dev; > struct platform_device *pdev; > @@ -68,6 +72,9 @@ struct hns_roce_dev { > > int cmd_mod; > int loop_idc; > + struct hns_roce_hw *hw; > }; > > +extern struct hns_roce_hw hns_roce_hw_v1; > + > #endif /* _HNS_ROCE_DEVICE_H */ > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c > new file mode 100644 > index 0000000..198be3b > --- /dev/null > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c > @@ -0,0 +1,72 @@ > +/* > + * Copyright (c) 2016 Hisilicon Limited. > + * > + * This software is available to you under a choice of one of two > + * licenses. You may choose to be licensed under the terms of the GNU > + * General Public License (GPL) Version 2, available from the file > + * COPYING in the main directory of this source tree, or the > + * OpenIB.org BSD license below: > + * > + * Redistribution and use in source and binary forms, with or > + * without modification, are permitted provided that the following > + * conditions are met: > + * > + * - Redistributions of source code must retain the above > + * copyright notice, this list of conditions and the following > + * disclaimer. > + * > + * - Redistributions in binary form must reproduce the above > + * copyright notice, this list of conditions and the following > + * disclaimer in the documentation and/or other materials > + * provided with the distribution. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE > + * SOFTWARE. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include "hns_roce_device.h" > +#include "hns_roce_hw_v1.h" > + > +/** > + * hns_roce_v1_reset - reset roce > + * @hr_dev: roce device struct pointer > + * @enable: true -- drop reset, false -- reset > + * return 0 - success , negative --fail > + */ > +int hns_roce_v1_reset(struct hns_roce_dev *hr_dev, bool enable) > +{ > + struct device_node *dsaf_node; > + struct device *dev = &hr_dev->pdev->dev; > + struct device_node *np = dev->of_node; > + int ret; > + > + dsaf_node = of_parse_phandle(np, "dsaf-handle", 0); > + > + if (!enable) { > + ret = hns_dsaf_roce_reset(&dsaf_node->fwnode, false); > + } else { > + ret = hns_dsaf_roce_reset(&dsaf_node->fwnode, false); Move this line out of if-else and leave "if (enable)" part only. > + if (ret) > + return ret; > + > + msleep(SLEEP_TIME_INTERVAL); Nice, here you used define and in other places just hardcoded 20 (msleep(20)). Please give meaningful definition to 20. > + ret = hns_dsaf_roce_reset(&dsaf_node->fwnode, true); > + } > + > + return ret; Indentation > +} > + > +struct hns_roce_hw hns_roce_hw_v1 = { > + .reset = hns_roce_v1_reset, > +}; > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.h b/drivers/infiniband/hw/hns/hns_roce_hw_v1.h > new file mode 100644 > index 0000000..a8c0c1d > --- /dev/null > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.h > @@ -0,0 +1,40 @@ > +/* > + * Copyright (c) 2016 Hisilicon Limited. > + * > + * This software is available to you under a choice of one of two > + * licenses. You may choose to be licensed under the terms of the GNU > + * General Public License (GPL) Version 2, available from the file > + * COPYING in the main directory of this source tree, or the > + * OpenIB.org BSD license below: > + * > + * Redistribution and use in source and binary forms, with or > + * without modification, are permitted provided that the following > + * conditions are met: > + * > + * - Redistributions of source code must retain the above > + * copyright notice, this list of conditions and the following > + * disclaimer. > + * > + * - Redistributions in binary form must reproduce the above > + * copyright notice, this list of conditions and the following > + * disclaimer in the documentation and/or other materials > + * provided with the distribution. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE > + * SOFTWARE. > + */ > + > +#ifndef _HNS_ROCE_HW_V1_H > +#define _HNS_ROCE_HW_V1_H > + > +#define SLEEP_TIME_INTERVAL 20 > + > +extern int hns_dsaf_roce_reset(struct fwnode_handle *dsaf_fwnode, bool enable); Why did you add this extern? You already exported this function. drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:EXPORT_SYMBOL(hns_dsaf_roce_reset); > + > +#endif > diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c > index 8924ce3..d5ccce2 100644 > --- a/drivers/infiniband/hw/hns/hns_roce_main.c > +++ b/drivers/infiniband/hw/hns/hns_roce_main.c > @@ -71,7 +71,9 @@ static int hns_roce_get_cfg(struct hns_roce_dev *hr_dev) > struct platform_device *pdev = NULL; > struct resource *res; > > - if (!of_device_is_compatible(np, "hisilicon,hns-roce-v1")) { > + if (of_device_is_compatible(np, "hisilicon,hns-roce-v1")) { > + hr_dev->hw = &hns_roce_hw_v1; > + } else { > dev_err(dev, "device no compatible!\n"); > return -EINVAL; > } > @@ -118,6 +120,11 @@ static int hns_roce_get_cfg(struct hns_roce_dev *hr_dev) > return 0; > } > > +static int hns_roce_engine_reset(struct hns_roce_dev *hr_dev, bool enable) > +{ > + return hr_dev->hw->reset(hr_dev, enable); > +} > + > /** > * hns_roce_probe - RoCE driver entrance > * @pdev: pointer to platform device > @@ -156,6 +163,12 @@ static int hns_roce_probe(struct platform_device *pdev) > goto error_failed_get_cfg; > } > > + ret = hns_roce_engine_reset(hr_dev, true); Do you have better solution to sense device without doing full reset of your hardware? > + if (ret) { > + dev_err(dev, "Reset roce engine failed!\n"); > + goto error_failed_get_cfg; > + } > + > error_failed_get_cfg: > ib_dealloc_device(&hr_dev->ib_dev); > > @@ -170,6 +183,8 @@ static int hns_roce_remove(struct platform_device *pdev) > { > struct hns_roce_dev *hr_dev = platform_get_drvdata(pdev); > > + (void)hns_roce_engine_reset(hr_dev, false); Any reason to do explicit casting? > + > ib_dealloc_device(&hr_dev->ib_dev); > > return 0; > -- > 1.9.1 >