netdev - Re: [PATCH net-next 06/11] net: hns3: modify firmware version display format

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 25 Jul 2019 21:32:20 +0000
From:   Saeed Mahameed <saeedm@...lanox.com>
To:     "tanhuazhong@...wei.com" <tanhuazhong@...wei.com>,
        "davem@...emloft.net" <davem@...emloft.net>
CC:     "lipeng321@...wei.com" <lipeng321@...wei.com>,
        "yisen.zhuang@...wei.com" <yisen.zhuang@...wei.com>,
        "salil.mehta@...wei.com" <salil.mehta@...wei.com>,
        "linuxarm@...wei.com" <linuxarm@...wei.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "moyufeng@...wei.com" <moyufeng@...wei.com>
Subject: Re: [PATCH net-next 06/11] net: hns3: modify firmware version display
 format

On Thu, 2019-07-25 at 10:34 +0800, tanhuazhong wrote:
> 
> On 2019/7/25 2:34, Saeed Mahameed wrote:
> > On Wed, 2019-07-24 at 11:18 +0800, Huazhong Tan wrote:
> > > From: Yufeng Mo <moyufeng@...wei.com>
> > > 
> > > This patch modifies firmware version display format in
> > > hclge(vf)_cmd_init() and hns3_get_drvinfo(). Also, adds
> > > some optimizations for firmware version display format.
> > > 
> > > Signed-off-by: Yufeng Mo <moyufeng@...wei.com>
> > > Signed-off-by: Peng Li <lipeng321@...wei.com>
> > > Signed-off-by: Huazhong Tan <tanhuazhong@...wei.com>
> > > ---
> > >   drivers/net/ethernet/hisilicon/hns3/hnae3.h              |  9
> > > +++++++++
> > >   drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c       | 15
> > > +++++++++++++--
> > >   drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c   | 10
> > > +++++++++-
> > >   drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c | 11
> > > +++++++++--
> > >   4 files changed, 40 insertions(+), 5 deletions(-)
> > > 
> > > 

[...]

> > > --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
> > > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
> > > @@ -419,7 +419,15 @@ int hclge_cmd_init(struct hclge_dev *hdev)
> > >   	}
> > >   	hdev->fw_version = version;
> > >   
> > > -	dev_info(&hdev->pdev->dev, "The firmware version is %08x\n",
> > > version);
> > > +	pr_info_once("The firmware version is %lu.%lu.%lu.%lu\n",
> > > +		     hnae3_get_field(version,
> > > HNAE3_FW_VERSION_BYTE3_MASK,
> > > +				     HNAE3_FW_VERSION_BYTE3_SHIFT),
> > > +		     hnae3_get_field(version,
> > > HNAE3_FW_VERSION_BYTE2_MASK,
> > > +				     HNAE3_FW_VERSION_BYTE2_SHIFT),
> > > +		     hnae3_get_field(version,
> > > HNAE3_FW_VERSION_BYTE1_MASK,
> > > +				     HNAE3_FW_VERSION_BYTE1_SHIFT),
> > > +		     hnae3_get_field(version,
> > > HNAE3_FW_VERSION_BYTE0_MASK,
> > > +				     HNAE3_FW_VERSION_BYTE0_SHIFT));
> > >   
> > 
> > Device name/string will not be printed now, what happens if i have
> > multiple devices ? at least print the device name as it was before
> > 
> Since on each board we only have one firmware, the firmware
> version is same per device, and will not change when running.
> So pr_info_once() looks good for this case.
> 

boards change too often to have such static assumption.

> BTW, maybe we should change below print in the end of
> hclge_init_ae_dev(), use dev_info() instead of pr_info(),
> then we can know that which device has already initialized.
> I will send other patch to do that, is it acceptable for you?
> 
> "pr_info("%s driver initialization finished.\n", HCLGE_DRIVER_NAME);"
> 

I would avoid using pr_info when i can ! if you have the option to
print with dev information as it was before that is preferable.

Thanks,
Saeed.