Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/705504/?format=api
{ "id": 705504, "url": "http://patchwork.ozlabs.org/api/patches/705504/?format=api", "web_url": "http://patchwork.ozlabs.org/project/intel-wired-lan/patch/148165657213.18335.13594356065946487859.stgit@mdrustad-wks.jf.intel.com/", "project": { "id": 46, "url": "http://patchwork.ozlabs.org/api/projects/46/?format=api", "name": "Intel Wired Ethernet development", "link_name": "intel-wired-lan", "list_id": "intel-wired-lan.osuosl.org", "list_email": "intel-wired-lan@osuosl.org", "web_url": "", "scm_url": "", "webscm_url": "", "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<148165657213.18335.13594356065946487859.stgit@mdrustad-wks.jf.intel.com>", "list_archive_url": null, "date": "2016-12-13T19:16:12", "name": "[V5,1/4] ixgbe: Fix issues with EEPROM access", "commit_ref": null, "pull_url": null, "state": "changes-requested", "archived": false, "hash": "db9121896316423f619f1c9bac56c4ebf0e56187", "submitter": { "id": 13252, "url": "http://patchwork.ozlabs.org/api/people/13252/?format=api", "name": "Rustad, Mark D", "email": "mark.d.rustad@intel.com" }, "delegate": { "id": 68, "url": "http://patchwork.ozlabs.org/api/users/68/?format=api", "username": "jtkirshe", "first_name": "Jeff", "last_name": "Kirsher", "email": "jeffrey.t.kirsher@intel.com" }, "mbox": "http://patchwork.ozlabs.org/project/intel-wired-lan/patch/148165657213.18335.13594356065946487859.stgit@mdrustad-wks.jf.intel.com/mbox/", "series": [], "comments": "http://patchwork.ozlabs.org/api/patches/705504/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/705504/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<intel-wired-lan-bounces@lists.osuosl.org>", "X-Original-To": [ "incoming@patchwork.ozlabs.org", "intel-wired-lan@lists.osuosl.org" ], "Delivered-To": [ "patchwork-incoming@bilbo.ozlabs.org", "intel-wired-lan@lists.osuosl.org" ], "Received": [ "from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3tdTwS52YYz9sDG\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 14 Dec 2016 06:16:56 +1100 (AEDT)", "from localhost (localhost [127.0.0.1])\n\tby silver.osuosl.org (Postfix) with ESMTP id 48C692DC72;\n\tTue, 13 Dec 2016 19:16:55 +0000 (UTC)", "from silver.osuosl.org ([127.0.0.1])\n\tby localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id V0-5yNSdej+t; Tue, 13 Dec 2016 19:16:48 +0000 (UTC)", "from ash.osuosl.org (ash.osuosl.org [140.211.166.34])\n\tby silver.osuosl.org (Postfix) with ESMTP id CE14F2D673;\n\tTue, 13 Dec 2016 19:16:48 +0000 (UTC)", "from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137])\n\tby ash.osuosl.org (Postfix) with ESMTP id 9024B1C0640\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tTue, 13 Dec 2016 19:16:41 +0000 (UTC)", "from localhost (localhost [127.0.0.1])\n\tby fraxinus.osuosl.org (Postfix) with ESMTP id 8A1228786B\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tTue, 13 Dec 2016 19:16:41 +0000 (UTC)", "from fraxinus.osuosl.org ([127.0.0.1])\n\tby localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id eCkWksibF0-A for <intel-wired-lan@lists.osuosl.org>;\n\tTue, 13 Dec 2016 19:16:38 +0000 (UTC)", "from mga09.intel.com (mga09.intel.com [134.134.136.24])\n\tby fraxinus.osuosl.org (Postfix) with ESMTPS id 6769D8780D\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tTue, 13 Dec 2016 19:16:38 +0000 (UTC)", "from fmsmga004.fm.intel.com ([10.253.24.48])\n\tby orsmga102.jf.intel.com with ESMTP; 13 Dec 2016 11:16:12 -0800", "from mdrustad-wks.jf.intel.com ([134.134.3.111])\n\tby fmsmga004.fm.intel.com with ESMTP; 13 Dec 2016 11:16:12 -0800" ], "X-Virus-Scanned": [ "amavisd-new at osuosl.org", "amavisd-new at osuosl.org" ], "X-Greylist": "domain auto-whitelisted by SQLgrey-1.7.6", "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos;i=\"5.33,342,1477983600\"; d=\"scan'208\";a=\"202201021\"", "From": "Mark D Rustad <mark.d.rustad@intel.com>", "To": "intel-wired-lan@lists.osuosl.org", "Date": "Tue, 13 Dec 2016 11:16:12 -0800", "Message-ID": "<148165657213.18335.13594356065946487859.stgit@mdrustad-wks.jf.intel.com>", "In-Reply-To": "<148165652623.18335.5542391508119084017.stgit@mdrustad-wks.jf.intel.com>", "References": "<148165652623.18335.5542391508119084017.stgit@mdrustad-wks.jf.intel.com>", "User-Agent": "StGit/unknown-version", "MIME-Version": "1.0", "Subject": "[Intel-wired-lan] [PATCH V5 1/4] ixgbe: Fix issues with EEPROM\n\taccess", "X-BeenThere": "intel-wired-lan@lists.osuosl.org", "X-Mailman-Version": "2.1.18-1", "Precedence": "list", "List-Id": "Intel Wired Ethernet Linux Kernel Driver Development\n\t<intel-wired-lan.lists.osuosl.org>", "List-Unsubscribe": "<http://lists.osuosl.org/mailman/options/intel-wired-lan>, \n\t<mailto:intel-wired-lan-request@lists.osuosl.org?subject=unsubscribe>", "List-Archive": "<http://lists.osuosl.org/pipermail/intel-wired-lan/>", "List-Post": "<mailto:intel-wired-lan@lists.osuosl.org>", "List-Help": "<mailto:intel-wired-lan-request@lists.osuosl.org?subject=help>", "List-Subscribe": "<http://lists.osuosl.org/mailman/listinfo/intel-wired-lan>, \n\t<mailto:intel-wired-lan-request@lists.osuosl.org?subject=subscribe>", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "intel-wired-lan-bounces@lists.osuosl.org", "Sender": "\"Intel-wired-lan\" <intel-wired-lan-bounces@lists.osuosl.org>" }, "content": "There are two problems with EEPROM access. One is that it needs to\nhold the semaphore until the entire response is read or else the\nresponse can be corrupted by other firmware accesses. The second\nproblem is that acquiring and releasing the semaphore is slow, so\nit should be taken and released once when multiple EEPROM accesses\nwill be done.\n\nBoth of these issues can be solved by adding a new function,\nixgbe_hic_unlocked, to issue firmware commands that will assume\nthat the caller has acquired the needed semaphore.\n\nSigned-off-by: Mark Rustad <mark.d.rustad@intel.com>\n---\n drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 97 +++++++++++++++--------\n drivers/net/ethernet/intel/ixgbe/ixgbe_common.h | 1 \n drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 75 +++++++-----------\n 3 files changed, 91 insertions(+), 82 deletions(-)", "diff": "diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c\nindex 5d8f1ae125a5..851f48555506 100644\n--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c\n+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c\n@@ -3593,43 +3593,29 @@ u8 ixgbe_calculate_checksum(u8 *buffer, u32 length)\n }\n \n /**\n- * ixgbe_host_interface_command - Issue command to manageability block\n+ * ixgbe_hic_unlocked - Issue command to manageability block unlocked\n * @hw: pointer to the HW structure\n- * @buffer: contains the command to write and where the return status will\n- * be placed\n+ * @buffer: command to write and where the return status will be placed\n * @length: length of buffer, must be multiple of 4 bytes\n * @timeout: time in ms to wait for command completion\n- * @return_data: read and return data from the buffer (true) or not (false)\n- * Needed because FW structures are big endian and decoding of\n- * these fields can be 8 bit or 16 bit based on command. Decoding\n- * is not easily understood without making a table of commands.\n- * So we will leave this up to the caller to read back the data\n- * in these cases.\n *\n- * Communicates with the manageability block. On success return 0\n- * else return IXGBE_ERR_HOST_INTERFACE_COMMAND.\n+ * Communicates with the manageability block. On success return 0\n+ * else returns semaphore error when encountering an error acquiring\n+ * semaphore or IXGBE_ERR_HOST_INTERFACE_COMMAND when command fails.\n+ *\n+ * This function assumes that the IXGBE_GSSR_SW_MNG_SM semaphore is held\n+ * by the caller.\n **/\n-s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, void *buffer,\n-\t\t\t\t u32 length, u32 timeout,\n-\t\t\t\t bool return_data)\n+s32 ixgbe_hic_unlocked(struct ixgbe_hw *hw, u32 *buffer, u32 length,\n+\t\t u32 timeout)\n {\n-\tu32 hdr_size = sizeof(struct ixgbe_hic_hdr);\n-\tu32 hicr, i, bi, fwsts;\n-\tu16 buf_len, dword_len;\n-\tunion {\n-\t\tstruct ixgbe_hic_hdr hdr;\n-\t\tu32 u32arr[1];\n-\t} *bp = buffer;\n-\ts32 status;\n+\tu32 hicr, i, fwsts;\n+\tu16 dword_len;\n \n \tif (!length || length > IXGBE_HI_MAX_BLOCK_BYTE_LENGTH) {\n \t\thw_dbg(hw, \"Buffer length failure buffersize-%d.\\n\", length);\n \t\treturn IXGBE_ERR_HOST_INTERFACE_COMMAND;\n \t}\n-\t/* Take management host interface semaphore */\n-\tstatus = hw->mac.ops.acquire_swfw_sync(hw, IXGBE_GSSR_SW_MNG_SM);\n-\tif (status)\n-\t\treturn status;\n \n \t/* Set bit 9 of FWSTS clearing FW reset indication */\n \tfwsts = IXGBE_READ_REG(hw, IXGBE_FWSTS);\n@@ -3639,15 +3625,13 @@ s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, void *buffer,\n \thicr = IXGBE_READ_REG(hw, IXGBE_HICR);\n \tif (!(hicr & IXGBE_HICR_EN)) {\n \t\thw_dbg(hw, \"IXGBE_HOST_EN bit disabled.\\n\");\n-\t\tstatus = IXGBE_ERR_HOST_INTERFACE_COMMAND;\n-\t\tgoto rel_out;\n+\t\treturn IXGBE_ERR_HOST_INTERFACE_COMMAND;\n \t}\n \n \t/* Calculate length in DWORDs. We must be DWORD aligned */\n \tif (length % sizeof(u32)) {\n \t\thw_dbg(hw, \"Buffer length failure, not aligned to dword\");\n-\t\tstatus = IXGBE_ERR_INVALID_ARGUMENT;\n-\t\tgoto rel_out;\n+\t\treturn IXGBE_ERR_INVALID_ARGUMENT;\n \t}\n \n \tdword_len = length >> 2;\n@@ -3657,7 +3641,7 @@ s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, void *buffer,\n \t */\n \tfor (i = 0; i < dword_len; i++)\n \t\tIXGBE_WRITE_REG_ARRAY(hw, IXGBE_FLEX_MNG,\n-\t\t\t\t i, cpu_to_le32(bp->u32arr[i]));\n+\t\t\t\t i, cpu_to_le32(buffer[i]));\n \n \t/* Setting this bit tells the ARC that a new command is pending. */\n \tIXGBE_WRITE_REG(hw, IXGBE_HICR, hicr | IXGBE_HICR_C);\n@@ -3671,11 +3655,54 @@ s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, void *buffer,\n \n \t/* Check command successful completion. */\n \tif ((timeout && i == timeout) ||\n-\t !(IXGBE_READ_REG(hw, IXGBE_HICR) & IXGBE_HICR_SV)) {\n-\t\thw_dbg(hw, \"Command has failed with no status valid.\\n\");\n-\t\tstatus = IXGBE_ERR_HOST_INTERFACE_COMMAND;\n-\t\tgoto rel_out;\n+\t !(IXGBE_READ_REG(hw, IXGBE_HICR) & IXGBE_HICR_SV))\n+\t\treturn IXGBE_ERR_HOST_INTERFACE_COMMAND;\n+\n+\treturn 0;\n+}\n+\n+/**\n+ * ixgbe_host_interface_command - Issue command to manageability block\n+ * @hw: pointer to the HW structure\n+ * @buffer: contains the command to write and where the return status will\n+ * be placed\n+ * @length: length of buffer, must be multiple of 4 bytes\n+ * @timeout: time in ms to wait for command completion\n+ * @return_data: read and return data from the buffer (true) or not (false)\n+ * Needed because FW structures are big endian and decoding of\n+ * these fields can be 8 bit or 16 bit based on command. Decoding\n+ * is not easily understood without making a table of commands.\n+ * So we will leave this up to the caller to read back the data\n+ * in these cases.\n+ *\n+ * Communicates with the manageability block. On success return 0\n+ * else return IXGBE_ERR_HOST_INTERFACE_COMMAND.\n+ **/\n+s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, void *buffer,\n+\t\t\t\t u32 length, u32 timeout,\n+\t\t\t\t bool return_data)\n+{\n+\tu32 hdr_size = sizeof(struct ixgbe_hic_hdr);\n+\tunion {\n+\t\tstruct ixgbe_hic_hdr hdr;\n+\t\tu32 u32arr[1];\n+\t} *bp = buffer;\n+\tu16 buf_len, dword_len;\n+\ts32 status;\n+\tu32 bi;\n+\n+\tif (!length || length > IXGBE_HI_MAX_BLOCK_BYTE_LENGTH) {\n+\t\thw_dbg(hw, \"Buffer length failure buffersize-%d.\\n\", length);\n+\t\treturn IXGBE_ERR_HOST_INTERFACE_COMMAND;\n \t}\n+\t/* Take management host interface semaphore */\n+\tstatus = hw->mac.ops.acquire_swfw_sync(hw, IXGBE_GSSR_SW_MNG_SM);\n+\tif (status)\n+\t\treturn status;\n+\n+\tstatus = ixgbe_hic_unlocked(hw, buffer, length, timeout);\n+\tif (status)\n+\t\tgoto rel_out;\n \n \tif (!return_data)\n \t\tgoto rel_out;\ndiff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h\nindex 393aeabe2d8f..671a0cdf6935 100644\n--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h\n+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.h\n@@ -115,6 +115,7 @@ s32 ixgbe_set_fw_drv_ver_generic(struct ixgbe_hw *hw, u8 maj, u8 min,\n u8 ixgbe_calculate_checksum(u8 *buffer, u32 length);\n s32 ixgbe_host_interface_command(struct ixgbe_hw *hw, void *, u32 length,\n \t\t\t\t u32 timeout, bool return_data);\n+s32 ixgbe_hic_unlocked(struct ixgbe_hw *hw, u32 *buffer, u32 len, u32 timeout);\n void ixgbe_clear_tx_pending(struct ixgbe_hw *hw);\n bool ixgbe_mng_present(struct ixgbe_hw *hw);\n bool ixgbe_mng_enabled(struct ixgbe_hw *hw);\ndiff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c\nindex 80be82dd82ce..16e1aaf74a63 100644\n--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c\n+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c\n@@ -624,41 +624,6 @@ static s32 ixgbe_read_iosf_sb_reg_x550a(struct ixgbe_hw *hw, u32 reg_addr,\n \treturn status;\n }\n \n-/** ixgbe_read_ee_hostif_data_X550 - Read EEPROM word using a host interface\n- * command assuming that the semaphore is already obtained.\n- * @hw: pointer to hardware structure\n- * @offset: offset of word in the EEPROM to read\n- * @data: word read from the EEPROM\n- *\n- * Reads a 16 bit word from the EEPROM using the hostif.\n- **/\n-static s32 ixgbe_read_ee_hostif_data_X550(struct ixgbe_hw *hw, u16 offset,\n-\t\t\t\t\t u16 *data)\n-{\n-\ts32 status;\n-\tstruct ixgbe_hic_read_shadow_ram buffer;\n-\n-\tbuffer.hdr.req.cmd = FW_READ_SHADOW_RAM_CMD;\n-\tbuffer.hdr.req.buf_lenh = 0;\n-\tbuffer.hdr.req.buf_lenl = FW_READ_SHADOW_RAM_LEN;\n-\tbuffer.hdr.req.checksum = FW_DEFAULT_CHECKSUM;\n-\n-\t/* convert offset from words to bytes */\n-\tbuffer.address = cpu_to_be32(offset * 2);\n-\t/* one word */\n-\tbuffer.length = cpu_to_be16(sizeof(u16));\n-\n-\tstatus = ixgbe_host_interface_command(hw, &buffer, sizeof(buffer),\n-\t\t\t\t\t IXGBE_HI_COMMAND_TIMEOUT, false);\n-\tif (status)\n-\t\treturn status;\n-\n-\t*data = (u16)IXGBE_READ_REG_ARRAY(hw, IXGBE_FLEX_MNG,\n-\t\t\t\t\t FW_NVM_DATA_OFFSET);\n-\n-\treturn 0;\n-}\n-\n /** ixgbe_read_ee_hostif_buffer_X550- Read EEPROM word(s) using hostif\n * @hw: pointer to hardware structure\n * @offset: offset of word in the EEPROM to read\n@@ -670,6 +635,7 @@ static s32 ixgbe_read_ee_hostif_data_X550(struct ixgbe_hw *hw, u16 offset,\n static s32 ixgbe_read_ee_hostif_buffer_X550(struct ixgbe_hw *hw,\n \t\t\t\t\t u16 offset, u16 words, u16 *data)\n {\n+\tconst u32 mask = IXGBE_GSSR_SW_MNG_SM | IXGBE_GSSR_EEP_SM;\n \tstruct ixgbe_hic_read_shadow_ram buffer;\n \tu32 current_word = 0;\n \tu16 words_to_read;\n@@ -677,7 +643,7 @@ static s32 ixgbe_read_ee_hostif_buffer_X550(struct ixgbe_hw *hw,\n \tu32 i;\n \n \t/* Take semaphore for the entire operation. */\n-\tstatus = hw->mac.ops.acquire_swfw_sync(hw, IXGBE_GSSR_EEP_SM);\n+\tstatus = hw->mac.ops.acquire_swfw_sync(hw, mask);\n \tif (status) {\n \t\thw_dbg(hw, \"EEPROM read buffer - semaphore failed\\n\");\n \t\treturn status;\n@@ -698,10 +664,8 @@ static s32 ixgbe_read_ee_hostif_buffer_X550(struct ixgbe_hw *hw,\n \t\tbuffer.address = cpu_to_be32((offset + current_word) * 2);\n \t\tbuffer.length = cpu_to_be16(words_to_read * 2);\n \n-\t\tstatus = ixgbe_host_interface_command(hw, &buffer,\n-\t\t\t\t\t\t sizeof(buffer),\n-\t\t\t\t\t\t IXGBE_HI_COMMAND_TIMEOUT,\n-\t\t\t\t\t\t false);\n+\t\tstatus = ixgbe_hic_unlocked(hw, (u32 *)&buffer, sizeof(buffer),\n+\t\t\t\t\t IXGBE_HI_COMMAND_TIMEOUT);\n \t\tif (status) {\n \t\t\thw_dbg(hw, \"Host interface command failed\\n\");\n \t\t\tgoto out;\n@@ -725,7 +689,7 @@ static s32 ixgbe_read_ee_hostif_buffer_X550(struct ixgbe_hw *hw,\n \t}\n \n out:\n-\thw->mac.ops.release_swfw_sync(hw, IXGBE_GSSR_EEP_SM);\n+\thw->mac.ops.release_swfw_sync(hw, mask);\n \treturn status;\n }\n \n@@ -896,15 +860,32 @@ static s32 ixgbe_calc_eeprom_checksum_X550(struct ixgbe_hw *hw)\n **/\n static s32 ixgbe_read_ee_hostif_X550(struct ixgbe_hw *hw, u16 offset, u16 *data)\n {\n-\ts32 status = 0;\n+\tconst u32 mask = IXGBE_GSSR_SW_MNG_SM | IXGBE_GSSR_EEP_SM;\n+\tstruct ixgbe_hic_read_shadow_ram buffer;\n+\ts32 status;\n \n-\tif (hw->mac.ops.acquire_swfw_sync(hw, IXGBE_GSSR_EEP_SM) == 0) {\n-\t\tstatus = ixgbe_read_ee_hostif_data_X550(hw, offset, data);\n-\t\thw->mac.ops.release_swfw_sync(hw, IXGBE_GSSR_EEP_SM);\n-\t} else {\n-\t\tstatus = IXGBE_ERR_SWFW_SYNC;\n+\tbuffer.hdr.req.cmd = FW_READ_SHADOW_RAM_CMD;\n+\tbuffer.hdr.req.buf_lenh = 0;\n+\tbuffer.hdr.req.buf_lenl = FW_READ_SHADOW_RAM_LEN;\n+\tbuffer.hdr.req.checksum = FW_DEFAULT_CHECKSUM;\n+\n+\t/* convert offset from words to bytes */\n+\tbuffer.address = cpu_to_be32(offset * 2);\n+\t/* one word */\n+\tbuffer.length = cpu_to_be16(sizeof(u16));\n+\n+\tstatus = hw->mac.ops.acquire_swfw_sync(hw, mask);\n+\tif (status)\n+\t\treturn status;\n+\n+\tstatus = ixgbe_hic_unlocked(hw, (u32 *)&buffer, sizeof(buffer),\n+\t\t\t\t IXGBE_HI_COMMAND_TIMEOUT);\n+\tif (!status) {\n+\t\t*data = (u16)IXGBE_READ_REG_ARRAY(hw, IXGBE_FLEX_MNG,\n+\t\t\t\t\t\t FW_NVM_DATA_OFFSET);\n \t}\n \n+\thw->mac.ops.release_swfw_sync(hw, mask);\n \treturn status;\n }\n \n", "prefixes": [ "V5", "1/4" ] }