[{"id":1797367,"web_url":"http://patchwork.ozlabs.org/comment/1797367/","msgid":"","list_archive_url":null,"date":"2017-11-01T16:50:30","subject":"Re: [PATCH FEAT] ibmvnic: Feature implementation of Vital Product\n\tData (VPD) for the ibmvnic driver","submitter":{"id":47927,"url":"http://patchwork.ozlabs.org/api/people/47927/","name":"Thomas Falcon","email":"tlfalcon@linux.vnet.ibm.com"},"content":"On 11/01/2017 09:39 AM, Desnes Augusto Nunes do Rosario wrote:\n> This patch implements and enables VDP support for the ibmvnic driver. Moreover, it includes the implementation of suitable structs, signal transmission/handling and fuctions which allows the retrival of firmware information from the ibmvnic card.\n>\n> Co-Authored-By: Thomas Falcon \n> ---\n\nHi, you should include a Signed-off-by tag in the commit message. You should also include the branch the patch is meant for in the PATCH field. In this case, it would be net-next. The commit message should also be wrapped (75 char per line), and 'fuctions' is missing an n.\n\n> drivers/net/ethernet/ibm/ibmvnic.c | 140 ++++++++++++++++++++++++++++++++++++-\n> drivers/net/ethernet/ibm/ibmvnic.h | 27 ++++++-\n> 2 files changed, 164 insertions(+), 3 deletions(-)\n>\n> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c\n> index d0cff28..120f3c0 100644\n> --- a/drivers/net/ethernet/ibm/ibmvnic.c\n> +++ b/drivers/net/ethernet/ibm/ibmvnic.c\n> @@ -107,6 +107,9 @@ static union sub_crq *ibmvnic_next_scrq(struct ibmvnic_adapter *,\n> \t\t\t\t\tstruct ibmvnic_sub_crq_queue *);\n> static int ibmvnic_poll(struct napi_struct *napi, int data);\n> static void send_map_query(struct ibmvnic_adapter *adapter);\n> +static int ibmvnic_get_vpd(struct ibmvnic_adapter *);\n> +static void handle_vpd_size_rsp(union ibmvnic_crq *, struct ibmvnic_adapter *);\n> +static void handle_vpd_rsp(union ibmvnic_crq *,struct ibmvnic_adapter *);\n\nThere should be a space after the comma.\n\n> static void send_request_map(struct ibmvnic_adapter *, dma_addr_t, __be32, u8);\n> static void send_request_unmap(struct ibmvnic_adapter *, u8);\n> static void send_login(struct ibmvnic_adapter *adapter);\n> @@ -573,6 +576,15 @@ static int reset_tx_pools(struct ibmvnic_adapter *adapter)\n> \treturn 0;\n> }\n>\n> +static void release_vpd_data(struct ibmvnic_adapter *adapter)\n> +{\n> +\tif(!adapter->vpd)\n\nThere should be a space between the if here. There are also some style checks that were picked up by checkpatch.pl. It's a good idea to run your patch through that before submission.\n\nThanks,\nTom\n\n> +\t\treturn;\n> +\n> +\tkfree(adapter->vpd->buff);\n> +\tkfree(adapter->vpd);\n> +}\n> +\n> static void release_tx_pools(struct ibmvnic_adapter *adapter)\n> {\n> \tstruct ibmvnic_tx_pool *tx_pool;\n> @@ -753,6 +765,8 @@ static void release_resources(struct ibmvnic_adapter *adapter)\n> {\n> \tint i;\n>\n> +\trelease_vpd_data(adapter);\n> +\n> \trelease_tx_pools(adapter);\n> \trelease_rx_pools(adapter);\n>\n> @@ -850,6 +864,10 @@ static int init_resources(struct ibmvnic_adapter *adapter)\n> \tif (rc)\n> \t\treturn rc;\n>\n> +\tadapter->vpd = kzalloc(sizeof(struct ibmvnic_vpd), GFP_KERNEL);\n> +\tif (!adapter->vpd)\n> +\t\treturn -ENOMEM;\n> +\n> \tadapter->map_id = 1;\n> \tadapter->napi = kcalloc(adapter->req_rx_queues,\n> \t\t\t\tsizeof(struct napi_struct), GFP_KERNEL);\n> @@ -950,6 +968,10 @@ static int ibmvnic_open(struct net_device *netdev)\n>\n> \trc = __ibmvnic_open(netdev);\n> \tnetif_carrier_on(netdev);\n> +\n> +\t/* Vital Product Data (VPD) */\n> +\tibmvnic_get_vpd(adapter);\n> +\n> \tmutex_unlock(&adapter->reset_lock);\n>\n> \treturn rc;\n> @@ -1878,11 +1900,15 @@ static int ibmvnic_get_link_ksettings(struct net_device *netdev,\n> \treturn 0;\n> }\n>\n> -static void ibmvnic_get_drvinfo(struct net_device *dev,\n> +static void ibmvnic_get_drvinfo(struct net_device *netdev,\n> \t\t\t\tstruct ethtool_drvinfo *info)\n> {\n> +\tstruct ibmvnic_adapter *adapter = netdev_priv(netdev);\n> +\n> \tstrlcpy(info->driver, ibmvnic_driver_name, sizeof(info->driver));\n> \tstrlcpy(info->version, IBMVNIC_DRIVER_VERSION, sizeof(info->version));\n> +\tstrlcpy(info->fw_version, adapter->fw_version,\n> +\t\tsizeof(info->fw_version));\n> }\n>\n> static u32 ibmvnic_get_msglevel(struct net_device *netdev)\n> @@ -2923,6 +2947,110 @@ static void send_login(struct ibmvnic_adapter *adapter)\n> \treturn;\n> }\n>\n> +static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)\n> +{\n> +\tstruct device *dev = &adapter->vdev->dev;\n> +\tunion ibmvnic_crq crq;\n> +\tdma_addr_t dma_addr;\n> +\tint len;\n> +\n> +\tif (adapter->vpd->buff)\n> +\t\tlen = adapter->vpd->len;\n> +\n> +\treinit_completion(&adapter->fw_done);\n> +\tcrq.get_vpd_size.first = IBMVNIC_CRQ_CMD;\n> +\tcrq.get_vpd_size.cmd = GET_VPD_SIZE;\n> +\tibmvnic_send_crq(adapter, &crq);\n> +\twait_for_completion(&adapter->fw_done);\n> +\n> +\tif (!adapter->vpd->buff)\n> +\t\tadapter->vpd->buff = kzalloc(adapter->vpd->len, GFP_KERNEL);\n> +\telse if (adapter->vpd->len != len)\n> +\t\tadapter->vpd->buff =\n> +\t\t\tkrealloc(adapter->vpd->buff,\n> +\t\t\t\t adapter->vpd->len, GFP_KERNEL);\n> +\n> +\tif (!adapter->vpd->buff) {\n> +\t\tdev_err(dev, \"Could allocate VPD buffer\\n\");\n> +\t\treturn -ENOMEM;\n> +\t}\n> +\n> +\tadapter->vpd->dma_addr =\n> +\t\tdma_map_single(dev, adapter->vpd->buff, adapter->vpd->len,\n> +\t\t\t DMA_FROM_DEVICE);\n> +\tif (dma_mapping_error(dev, dma_addr)) {\n> +\t\tdev_err(dev, \"Could not map VPD buffer\\n\");\n> +\t\treturn -ENOMEM;\n> +\t}\n> +\n> +\treinit_completion(&adapter->fw_done);\n> +\tcrq.get_vpd.first = IBMVNIC_CRQ_CMD;\n> +\tcrq.get_vpd.cmd = GET_VPD;\n> +\tcrq.get_vpd.ioba = cpu_to_be32(adapter->vpd->dma_addr);\n> +\tcrq.get_vpd.len = cpu_to_be32((u32)adapter->vpd->len);\n> +\tibmvnic_send_crq(adapter, &crq);\n> +\twait_for_completion(&adapter->fw_done);\n> +\n> +\treturn 0;\n> +}\n> +\n> +static void handle_vpd_size_rsp(union ibmvnic_crq *crq,\n> +\t\t\t\t struct ibmvnic_adapter *adapter)\n> +{\n> +\tstruct device *dev = &adapter->vdev->dev;\n> +\n> +\tif (crq->get_vpd_size_rsp.rc.code) {\n> +\t\tdev_err(dev, \"Error retrieving VPD size, rc=%x\\n\",\n> +\t\t\tcrq->get_vpd_size_rsp.rc.code);\n> +\t\tcomplete(&adapter->fw_done);\n> +\t\treturn;\n> +\t}\n> +\n> +\tadapter->vpd->len = be64_to_cpu(crq->get_vpd_size_rsp.len);\n> +\tcomplete(&adapter->fw_done);\n> +}\n> +\n> +static void handle_vpd_rsp(union ibmvnic_crq *crq,\n> +\t\t\t struct ibmvnic_adapter *adapter)\n> +{\n> +\tstruct device *dev = &adapter->vdev->dev;\n> +\tchar *substr = NULL, *ptr = NULL;\n> +\tu8 fw_level_len = 0;\n> +\n> +\tdma_unmap_single(dev, adapter->vpd->dma_addr, adapter->vpd->len,\n> +\t\t\t DMA_FROM_DEVICE);\n> +\n> +\tif (crq->get_vpd_rsp.rc.code) {\n> +\t\tdev_err(dev, \"Error retrieving VPD, rc=%x\\n\",\n> +\t\t\tcrq->get_vpd_rsp.rc.code);\n> +\t\tcomplete(&adapter->fw_done);\n> +\t\treturn;\n> +\t}\n> +\n> +\t/* get the position of the firmware version info\n> +\t * located after the ASCII 'RM' substring in the buffer\n> +\t */\n> +\tsubstr = strnstr(adapter->vpd->buff, \"RM\", adapter->vpd->len);\n> +\tif (substr == NULL) {\n> +\t\tdev_info(dev, \"No FW level provided by VPD\\n\");\n> +\t\tcomplete(&adapter->fw_done);\n> +\t\treturn;\n> +\t}\n> +\n> +\t/* get length of firmware level ASCII substring */\n> +\tfw_level_len = *(substr + 2);\n> +\n> +\t/* copy firmware version string from vpd into adapter */\n> +\tptr = strncpy((char *)adapter->fw_version,\n> +\t\t\t substr + 3, fw_level_len);\n> +\tif (!ptr) {\n> +\t\tdev_err(dev, \"Failed to isolate FW level string\\n\");\n> +\t\tmemset(adapter->fw_version, 0, 32);\n> +\t}\n> +\n> +\tcomplete(&adapter->fw_done);\n> +}\n> +\n> static void send_request_map(struct ibmvnic_adapter *adapter, dma_addr_t addr,\n> \t\t\t u32 len, u8 map_id)\n> {\n> @@ -3807,6 +3939,12 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,\n> \t\tnetdev_dbg(netdev, \"Got Collect firmware trace Response\\n\");\n> \t\tcomplete(&adapter->fw_done);\n> \t\tbreak;\n> +\tcase GET_VPD_SIZE_RSP:\n> +\t\thandle_vpd_size_rsp(crq, adapter);\n> +\t\tbreak;\n> +\tcase GET_VPD_RSP:\n> +\t\thandle_vpd_rsp(crq, adapter);\n> +\t\tbreak;\n> \tdefault:\n> \t\tnetdev_err(netdev, \"Got an invalid cmd type 0x%02x\\n\",\n> \t\t\t gen_crq->cmd);\n> diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h\n> index 4670af8..d3a6959 100644\n> --- a/drivers/net/ethernet/ibm/ibmvnic.h\n> +++ b/drivers/net/ethernet/ibm/ibmvnic.h\n> @@ -558,6 +558,12 @@ struct ibmvnic_multicast_ctrl {\n> \tstruct ibmvnic_rc rc;\n> } __packed __aligned(8);\n>\n> +struct ibmvnic_get_vpd_size {\n> +\tu8 first;\n> +\tu8 cmd;\n> +\tu8 reserved[14];\n> +} __packed __aligned(8);\n> +\n> struct ibmvnic_get_vpd_size_rsp {\n> \tu8 first;\n> \tu8 cmd;\n> @@ -575,6 +581,13 @@ struct ibmvnic_get_vpd {\n> \tu8 reserved[4];\n> } __packed __aligned(8);\n>\n> +struct ibmvnic_get_vpd_rsp {\n> +\tu8 first;\n> +\tu8 cmd;\n> +\tu8 reserved[10];\n> +\tstruct ibmvnic_rc rc;\n> +} __packed __aligned(8);\n> +\n> struct ibmvnic_acl_change_indication {\n> \tu8 first;\n> \tu8 cmd;\n> @@ -700,10 +713,10 @@ union ibmvnic_crq {\n> \tstruct ibmvnic_change_mac_addr change_mac_addr_rsp;\n> \tstruct ibmvnic_multicast_ctrl multicast_ctrl;\n> \tstruct ibmvnic_multicast_ctrl multicast_ctrl_rsp;\n> -\tstruct ibmvnic_generic_crq get_vpd_size;\n> +\tstruct ibmvnic_get_vpd_size get_vpd_size;\n> \tstruct ibmvnic_get_vpd_size_rsp get_vpd_size_rsp;\n> \tstruct ibmvnic_get_vpd get_vpd;\n> -\tstruct ibmvnic_generic_crq get_vpd_rsp;\n> +\tstruct ibmvnic_get_vpd_rsp get_vpd_rsp;\n> \tstruct ibmvnic_acl_change_indication acl_change_indication;\n> \tstruct ibmvnic_acl_query acl_query;\n> \tstruct ibmvnic_generic_crq acl_query_rsp;\n> @@ -937,6 +950,12 @@ struct ibmvnic_error_buff {\n> \t__be32 error_id;\n> };\n>\n> +struct ibmvnic_vpd {\n> +\tunsigned char *buff;\n> +\tdma_addr_t dma_addr;\n> +\tu64 len;\n> +};\n> +\n> enum vnic_state {VNIC_PROBING = 1,\n> \t\t VNIC_PROBED,\n> \t\t VNIC_OPENING,\n> @@ -978,6 +997,10 @@ struct ibmvnic_adapter {\n> \tdma_addr_t ip_offload_ctrl_tok;\n> \tu32 msg_enable;\n>\n> +\t/* Vital Product Data (VPD) */\n> +\tstruct ibmvnic_vpd *vpd;\n> +\tchar fw_version[32];\n> +\n> \t/* Statistics */\n> \tstruct ibmvnic_statistics stats;\n> \tdma_addr_t stats_token;","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yRvNb5Hmmz9sPs\n\tfor ;\n\tThu, 2 Nov 2017 03:50:39 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1755003AbdKAQui (ORCPT );\n\tWed, 1 Nov 2017 12:50:38 -0400","from mx0a-001b2d01.pphosted.com ([148.163.156.1]:57928 \"EHLO\n\tmx0a-001b2d01.pphosted.com\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1754879AbdKAQuh (ORCPT\n\t); Wed, 1 Nov 2017 12:50:37 -0400","from pps.filterd (m0098404.ppops.net [127.0.0.1])\n\tby mx0a-001b2d01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id\n\tvA1GiWq7054734\n\tfor ; Wed, 1 Nov 2017 12:50:36 -0400","from e11.ny.us.ibm.com (e11.ny.us.ibm.com [129.33.205.201])\n\tby mx0a-001b2d01.pphosted.com with ESMTP id 2dyh3gusf9-1\n\t(version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT)\n\tfor ; Wed, 01 Nov 2017 12:50:35 -0400","from localhost\n\tby e11.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use\n\tOnly! Violators will be prosecuted\n\tfor from ;\n\tWed, 1 Nov 2017 12:50:34 -0400","from b01cxnp22035.gho.pok.ibm.com (9.57.198.25)\n\tby e11.ny.us.ibm.com (146.89.104.198) with IBM ESMTP SMTP Gateway:\n\tAuthorized Use Only! Violators will be prosecuted; \n\tWed, 1 Nov 2017 12:50:32 -0400","from b01ledav004.gho.pok.ibm.com (b01ledav004.gho.pok.ibm.com\n\t[9.57.199.109])\n\tby b01cxnp22035.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP\n\tid vA1GoVcx49610840; Wed, 1 Nov 2017 16:50:31 GMT","from b01ledav004.gho.pok.ibm.com (unknown [127.0.0.1])\n\tby IMSVA (Postfix) with ESMTP id 56151112034;\n\tWed, 1 Nov 2017 12:50:00 -0400 (EDT)","from [9.41.92.147] (unknown [9.41.92.147])\n\tby b01ledav004.gho.pok.ibm.com (Postfix) with ESMTP id D832A112047;\n\tWed, 1 Nov 2017 12:49:59 -0400 (EDT)"],"Subject":"Re: [PATCH FEAT] ibmvnic: Feature implementation of Vital Product\n\tData (VPD) for the ibmvnic driver","To":"Desnes Augusto Nunes do Rosario ,\n\tnetdev@vger.kernel.org","References":"<20171101143959.14992-1-desnesn@linux.vnet.ibm.com>","Cc":"linuxppc-dev@lists.ozlabs.org, nfont@linux.vnet.ibm.com,\n\tjallen@linux.vnet.ibm.com","From":"Thomas Falcon ","Date":"Wed, 1 Nov 2017 11:50:30 -0500","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101\n\tThunderbird/45.8.0","MIME-Version":"1.0","In-Reply-To":"<20171101143959.14992-1-desnesn@linux.vnet.ibm.com>","Content-Type":"text/plain; charset=windows-1252","Content-Transfer-Encoding":"7bit","X-TM-AS-GCONF":"00","x-cbid":"17110116-2213-0000-0000-00000234D43B","X-IBM-SpamModules-Scores":"","X-IBM-SpamModules-Versions":"BY=3.00007992; HX=3.00000241; KW=3.00000007;\n\tPH=3.00000004; SC=3.00000239; SDB=6.00939665; UDB=6.00473767;\n\tIPR=6.00719927; \n\tBA=6.00005666; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009;\n\tZB=6.00000000; \n\tZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00017824;\n\tXFM=3.00000015; UTC=2017-11-01 16:50:33","X-IBM-AV-DETECTION":"SAVI=unused REMOTE=unused XFE=unused","x-cbparentid":"17110116-2214-0000-0000-000058081FAE","Message-Id":"","X-Proofpoint-Virus-Version":"vendor=fsecure engine=2.50.10432:, ,\n\tdefinitions=2017-11-01_03:, , signatures=0","X-Proofpoint-Spam-Details":"rule=outbound_notspam policy=outbound score=0\n\tspamscore=0 suspectscore=0\n\tmalwarescore=0 lowpriorityscore=0 phishscore=0 adultscore=0\n\tbulkscore=0\n\tclassifier=spam adjust=0 reason=mlx scancount=1\n\tengine=8.0.1-1707230000\n\tdefinitions=main-1711010225","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":1797382,"web_url":"http://patchwork.ozlabs.org/comment/1797382/","msgid":"<20171101171708.GA24320@lunn.ch>","list_archive_url":null,"date":"2017-11-01T17:17:08","subject":"Re: [PATCH FEAT] ibmvnic: Feature implementation of Vital Product\n\tData (VPD) for the ibmvnic driver","submitter":{"id":13608,"url":"http://patchwork.ozlabs.org/api/people/13608/","name":"Andrew Lunn","email":"andrew@lunn.ch"},"content":"> > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c\n> > index d0cff28..120f3c0 100644\n> > --- a/drivers/net/ethernet/ibm/ibmvnic.c\n> > +++ b/drivers/net/ethernet/ibm/ibmvnic.c\n> > @@ -107,6 +107,9 @@ static union sub_crq *ibmvnic_next_scrq(struct ibmvnic_adapter *,\n> > \t\t\t\t\tstruct ibmvnic_sub_crq_queue *);\n> > static int ibmvnic_poll(struct napi_struct *napi, int data);\n> > static void send_map_query(struct ibmvnic_adapter *adapter);\n> > +static int ibmvnic_get_vpd(struct ibmvnic_adapter *);\n> > +static void handle_vpd_size_rsp(union ibmvnic_crq *, struct ibmvnic_adapter *);\n> > +static void handle_vpd_rsp(union ibmvnic_crq *,struct ibmvnic_adapter *);\n> \n> There should be a space after the comma.\n\nAnd you should try to avoid forward declarations. Move the code around\nso they are not needed.\n\n Andrew","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=netdev-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3yRvzG6dKJz9s8J\n\tfor ;\n\tThu, 2 Nov 2017 04:17:14 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754089AbdKARRM (ORCPT );\n\tWed, 1 Nov 2017 13:17:12 -0400","from vps0.lunn.ch ([185.16.172.187]:36774 \"EHLO vps0.lunn.ch\"\n\trhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP\n\tid S1751682AbdKARRL (ORCPT );\n\tWed, 1 Nov 2017 13:17:11 -0400","from andrew by vps0.lunn.ch with local (Exim 4.84_2)\n\t(envelope-from )\n\tid 1e9wdY-0007EK-4l; Wed, 01 Nov 2017 18:17:08 +0100"],"Date":"Wed, 1 Nov 2017 18:17:08 +0100","From":"Andrew Lunn ","To":"Thomas Falcon ","Cc":"Desnes Augusto Nunes do Rosario ,\n\tnetdev@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,\n\tnfont@linux.vnet.ibm.com, jallen@linux.vnet.ibm.com","Subject":"Re: [PATCH FEAT] ibmvnic: Feature implementation of Vital Product\n\tData (VPD) for the ibmvnic driver","Message-ID":"<20171101171708.GA24320@lunn.ch>","References":"<20171101143959.14992-1-desnesn@linux.vnet.ibm.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"","User-Agent":"Mutt/1.5.23 (2014-03-12)","Sender":"netdev-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]