From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753304AbcANJoJ (ORCPT ); Thu, 14 Jan 2016 04:44:09 -0500 Received: from mga04.intel.com ([192.55.52.120]:4776 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752448AbcANJnd (ORCPT ); Thu, 14 Jan 2016 04:43:33 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,293,1449561600"; d="scan'208";a="860318008" Message-ID: <1452764635.2521.28.camel@linux.intel.com> Subject: Re: [PATCH v2 next-next] net: hns: enet specifies a reference to dsaf From: Andy Shevchenko To: Yisen Zhuang , Kejian Yan , davem@davemloft.net, robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, catalin.marinas@arm.com, will.deacon@arm.com, huangdaode@hisilicon.com, liguozhu@huawei.com, arnd@arndb.de, fengguang.wu@intel.com, salil.mehta@huawei.com, lisheng011@huawei.com Cc: devicetree@vger.kernel.org, netdev@vger.kernel.org, linuxarm@huawei.com, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Date: Thu, 14 Jan 2016 11:43:55 +0200 In-Reply-To: <56970F48.9060504@huawei.com> References: <1452654880-28980-1-git-send-email-yankejian@huawei.com> <56970F48.9060504@huawei.com> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.3-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2016-01-14 at 11:00 +0800, Yisen Zhuang wrote: > > 在 2016/1/13 11:14, Kejian Yan 写道: > > This patch replace the assoication between dsaf and enet from > > string > > matching to object reference. It requires the DTS to be updated > > within > > BIOS. Thanks god it can be done for all released boards. > > > > Hi kejian, > > This patch is fine to me. There are few thing below. >  > > --- a/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c > > +++ b/drivers/net/ethernet/hisilicon/hns/hns_ae_adapt.c > > @@ -847,6 +847,7 @@ static struct hnae_ae_ops hns_dsaf_ops = { > >  int hns_dsaf_ae_init(struct dsaf_device *dsaf_dev) > >  { > >   struct hnae_ae_dev *ae_dev = &dsaf_dev->ae_dev; > > + static atomic_t id = ATOMIC_INIT(-1); > >   > >   switch (dsaf_dev->dsaf_ver) { > >   case AE_VERSION_1: > > @@ -858,6 +859,9 @@ int hns_dsaf_ae_init(struct dsaf_device > > *dsaf_dev) > >   default: > >   break; > >   } > > + > > + snprintf(ae_dev->name, AE_NAME_SIZE, "%s%d", > > DSAF_DEVICE_NAME, > > +  (int)atomic_inc_return(&id)); If you bind/unbind device enough times you may get an overflow and end up with name of existing device (if you have 1+ of them in the system). To avoid such situation better to use IDA/IDR framework. > >  > > --- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c > > +++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c > > @@ -1802,7 +1802,7 @@ static int hns_nic_try_get_ae(struct > > net_device *ndev) > >   int ret; > >   > >   h = hnae_get_handle(&priv->netdev->dev, > > -     priv->ae_name, priv->port_id, NULL); > > +     priv->ae_node, priv->port_id, NULL); > >   if (IS_ERR_OR_NULL(h)) { > >   ret = PTR_ERR(h); > >   dev_dbg(priv->dev, "has not handle, register > > notifier!\n"); > > @@ -1880,9 +1880,12 @@ static int hns_nic_dev_probe(struct > > platform_device *pdev) > >   else > >   priv->enet_ver = AE_VERSION_2; > >   > > - ret = of_property_read_string(node, "ae-name", &priv- > > >ae_name); > > - if (ret) > > - goto out_read_string_fail; (1) > > + priv->ae_node = (void *)of_parse_phandle(node, "ae- > > handle", 0); > > + if (IS_ERR_OR_NULL(priv->ae_node)) { > > + ret = PTR_ERR(priv->ae_node); > > + dev_err(dev, "not find ae-handle\n"); > > + goto out_read_handle_fai; (2) > > + } > >   > >   ret = of_property_read_u32(node, "port-id", &priv- > > >port_id); > >   if (ret) > > @@ -1945,6 +1948,8 @@ static int hns_nic_dev_probe(struct > > platform_device *pdev) > >   > >  out_notify_fail: > >   (void)cancel_work_sync(&priv->service_task); > > +out_read_handle_fai: > > + Redundant line > >  out_read_string_fail: Leftover? (see (1) and (2) ) -- Andy Shevchenko Intel Finland Oy