From: "Yankejian (Hackim Yim)" <yankejian-hv44wF8Li93QT0dZR+AlfA@public•gmane.org>
To: Andy Shevchenko
<andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public•gmane.org>,
Yisen Zhuang
<Yisen.zhuang-hv44wF8Li93QT0dZR+AlfA@public•gmane.org>,
<davem-fT/PcQaiUtIeIZ0/mPfg9Q@public•gmane.org>,
<robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public•gmane.org>,
<pawel.moll-5wv7dgnIgG8@public•gmane.org>,
<mark.rutland-5wv7dgnIgG8@public•gmane.org>,
<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public•gmane.org>,
<galak-sgV2jX0FEOL9JmXXK+q4OQ@public•gmane.org>,
<catalin.marinas-5wv7dgnIgG8@public•gmane.org>,
<will.deacon-5wv7dgnIgG8@public•gmane.org>,
<huangdaode-C8/M+/jPZTeaMJb+Lgu22Q@public•gmane.org>,
<liguozhu-hv44wF8Li93QT0dZR+AlfA@public•gmane.org>,
<arnd-r2nGTMty4D4@public•gmane.org>,
<fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public•gmane.org>,
<salil.mehta-hv44wF8Li93QT0dZR+AlfA@public•gmane.org>,
<lisheng011-hv44wF8Li93QT0dZR+AlfA@public•gmane.org>
Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public•gmane.org>,
<netdev-u79uwXL29TY76Z2rM5mHXA@public•gmane.org>,
<linuxarm-hv44wF8Li93QT0dZR+AlfA@public•gmane.org>,
<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public•gmane.org>,
<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public•gmane.org>
Subject: Re: [PATCH v2 next-next] net: hns: enet specifies a reference to dsaf
Date: Mon, 18 Jan 2016 19:35:38 +0800 [thread overview]
Message-ID: <569CCE0A.5010005@huawei.com> (raw)
In-Reply-To: <1452764635.2521.28.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On 2016/1/14 17:43, Andy Shevchenko wrote:
> 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.
thanks
Best Regards
Kejian Yan
>>>
>>> --- 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) )
thanks.
Best Regards
Kejian Yan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public•gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2016-01-18 11:35 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-13 3:14 [PATCH v2 next-next] net: hns: enet specifies a reference to dsaf Kejian Yan
2016-01-14 3:00 ` Yisen Zhuang
[not found] ` <56970F48.9060504-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2016-01-14 9:43 ` Andy Shevchenko
[not found] ` <1452764635.2521.28.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2016-01-18 11:35 ` Yankejian (Hackim Yim) [this message]
[not found] ` <569F2DEC.9040209@huawei.com>
[not found] ` <569F2DEC.9040209-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2016-01-20 9:36 ` Andy Shevchenko
2016-01-20 11:10 ` Yankejian (Hackim Yim)
[not found] ` <1452654880-28980-1-git-send-email-yankejian-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2016-01-15 2:28 ` Rob Herring
-- strict thread matches above, loose matches on Subject: below --
2016-01-13 3:01 Kejian Yan
2016-01-13 2:52 ` Yankejian (Hackim Yim)
2016-01-13 2:54 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=569CCE0A.5010005@huawei.com \
--to=yankejian-hv44wf8li93qt0dzr+alfa@public$(echo .)gmane.org \
--cc=Yisen.zhuang-hv44wF8Li93QT0dZR+AlfA@public$(echo .)gmane.org \
--cc=andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public$(echo .)gmane.org \
--cc=arnd-r2nGTMty4D4@public$(echo .)gmane.org \
--cc=catalin.marinas-5wv7dgnIgG8@public$(echo .)gmane.org \
--cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public$(echo .)gmane.org \
--cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public$(echo .)gmane.org \
--cc=galak-sgV2jX0FEOL9JmXXK+q4OQ@public$(echo .)gmane.org \
--cc=huangdaode-C8/M+/jPZTeaMJb+Lgu22Q@public$(echo .)gmane.org \
--cc=ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public$(echo .)gmane.org \
--cc=liguozhu-hv44wF8Li93QT0dZR+AlfA@public$(echo .)gmane.org \
--cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public$(echo .)gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=linuxarm-hv44wF8Li93QT0dZR+AlfA@public$(echo .)gmane.org \
--cc=lisheng011-hv44wF8Li93QT0dZR+AlfA@public$(echo .)gmane.org \
--cc=mark.rutland-5wv7dgnIgG8@public$(echo .)gmane.org \
--cc=netdev-u79uwXL29TY76Z2rM5mHXA@public$(echo .)gmane.org \
--cc=pawel.moll-5wv7dgnIgG8@public$(echo .)gmane.org \
--cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public$(echo .)gmane.org \
--cc=salil.mehta-hv44wF8Li93QT0dZR+AlfA@public$(echo .)gmane.org \
--cc=will.deacon-5wv7dgnIgG8@public$(echo .)gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox