From patchwork Thu Feb 25 17:31:48 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Jan_Kundr=C3=A1t?= X-Patchwork-Id: 1444588 X-Patchwork-Delegate: anthony.l.nguyen@intel.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=osuosl.org (client-ip=140.211.166.133; helo=smtp2.osuosl.org; envelope-from=intel-wired-lan-bounces@osuosl.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; secure) header.d=cesnet.cz header.i=@cesnet.cz header.a=rsa-sha256 header.s=office2-2020 header.b=Ac8Yj24N; dkim-atps=neutral Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4DmjyK5YWXz9sVV for ; Fri, 26 Feb 2021 06:48:24 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 30B354332E; Thu, 25 Feb 2021 19:48:23 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id JyfDe3X6jhX7; Thu, 25 Feb 2021 19:48:22 +0000 (UTC) Received: from ash.osuosl.org (ash.osuosl.org [140.211.166.34]) by smtp2.osuosl.org (Postfix) with ESMTP id D82E443329; Thu, 25 Feb 2021 19:48:21 +0000 (UTC) X-Original-To: intel-wired-lan@lists.osuosl.org Delivered-To: intel-wired-lan@lists.osuosl.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by ash.osuosl.org (Postfix) with ESMTP id 4A4C21BF2F1 for ; Thu, 25 Feb 2021 19:06:50 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 337F74EF53 for ; Thu, 25 Feb 2021 19:06:50 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp4.osuosl.org (amavisd-new); dkim=pass (2048-bit key) header.d=cesnet.cz Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 0g43--Zh48GX for ; Thu, 25 Feb 2021 19:06:48 +0000 (UTC) X-Greylist: from auto-whitelisted by SQLgrey-1.8.0 Received: from office2.cesnet.cz (office2.cesnet.cz [195.113.144.244]) by smtp4.osuosl.org (Postfix) with ESMTPS id DE0144EF4D for ; Thu, 25 Feb 2021 19:06:47 +0000 (UTC) Received: from localhost (e-78-106.eduroam.fit.cvut.cz [147.32.78.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by office2.cesnet.cz (Postfix) with ESMTPSA id 5CE4C400064; Thu, 25 Feb 2021 20:06:43 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cesnet.cz; s=office2-2020; t=1614280004; bh=ERMZCjkdsIX9PRSwtqVh0eO0Su7gVbK6OqiUp9Kjb64=; h=Resent-Date:Resent-From:Resent-To:Resent-Cc:From:Date:Subject:To: Cc; b=Ac8Yj24NlLzj3dW+RGbREuHTg/lKFMH39jgynesBq1psBKtX1vjXaZMH0sqKbtGR5 cHvPKjPVtBIPTKfTXKM2YRWZtQCzrQHthoM0ppyGhDHr7vj0xWaYllIETN3Tqi51xq KcSq7wp7mEIp2iR5/V2jxU78XeNczZdA1lJU9mUHQ+Ivnm7BMBnS89sjEItFpNjz1F YevdKe5JZ5NQ4UpxbbBssGnj8767yFz8FsnmHdbTDXoh7S6NuOjyI9y0luL6YUcqNW 6dYijJ4kfP7eSpX0hbmFC5K9lkIAyy4WXGvUzGDxfo1sIDOVZC247/pHkJj82YCrTY iHPJdNWeVecDw== Resent-Date: Thu, 25 Feb 2021 20:06:42 +0100 Resent-Message-ID: Resent-From: =?iso-8859-1?q?Jan_Kundr=E1t?= Resent-To: "David S. Miller" , Jakub Kicinski , Resent-Cc: Carolyn Wyborny , Jesse Brandeburg , Tony Nguyen , , , =?utf-8?b?RnJhbnRpxaFlayBSecWhw6FuZWs=?= Message-Id: <5a9f1aed25a6df32337b9ac1140339d783abb6d8.1614279918.git.jan.kundrat@cesnet.cz> From: =?utf-8?q?Jan_Kundr=C3=A1t?= Date: Thu, 25 Feb 2021 18:31:48 +0100 MIME-Version: 1.0 To: David S. Miller , Jakub Kicinski , intel-wired-lan@lists.osuosl.org X-Mailman-Approved-At: Thu, 25 Feb 2021 19:48:20 +0000 Subject: [Intel-wired-lan] [PATCH] igb: unbreak I2C bit-banging on i350 X-BeenThere: intel-wired-lan@osuosl.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel Wired Ethernet Linux Kernel Driver Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: =?unknown-8bit?b?bmV0ZGV2QHZnZXIua2VybmVsLm9yZywgRnJhbnRpxaFlayBSecWh?= =?unknown-8bit?b?w6FuZWsgPHJ5c2FuZWtAZmNjcHMuY3o+LCBsaW51eC1pMmNAdmdlci5r?= =?unknown-8bit?b?ZXJuZWwub3Jn?= Errors-To: intel-wired-lan-bounces@osuosl.org Sender: "Intel-wired-lan" The driver tried to use Linux' native software I2C bus master (i2c-algo-bits) for exporting the I2C interface that talks to the SFP cage(s) towards userspace. As-is, however, the physical SCL/SDA pins were not moving at all, staying at logical 1 all the time. The main culprit was the I2CPARAMS register where igb was not setting the I2CBB_EN bit. That meant that all the careful signal bit-banging was actually not being propagated to the chip pads (I verified this with a scope). The bit-banging was not correct either, because I2C is supposed to be an open-collector bus, and the code was driving both lines via a totem pole. The code was also trying to do operations which did not make any sense with the i2c-algo-bits, namely manipulating both SDA and SCL from igb_set_i2c_data (which is only supposed to set SDA). I'm not sure if that was meant as an optimization, or was just flat out wrong, but given that the i2c-algo-bits is set up to work with a totally dumb GPIO-ish implementation underneath, there's no need for this code to be smart. The open-drain vs. totem-pole is fixed by the usual trick where the logical zero is implemented via regular output mode and outputting a logical 0, and the logical high is implemented via the IO pad configured as an input (thus floating), and letting the mandatory pull-up resistors do the rest. Anything else is actually wrong on I2C where all devices are supposed to have open-drain connection to the bus. The missing I2CBB_EN is set (along with a safe initial value of the GPIOs) just before registering this software I2C bus. The chip datasheet mentions HW-implemented I2C transactions (SFP EEPROM reads and writes) as well, but I'm not touching these for simplicity. Tested on a LR-Link LRES2203PF-2SFP (which is an almost-miniPCIe form factor card, a cable, and a module with two SFP cages). There was one casualty, an old broken SFP we had laying around, which was used to solder some thin wires as a DIY I2C breakout. Thanks for your service. With this patch in place, I can `i2cdump -y 3 0x51 c` and read back data which make sense. Yay. Signed-off-by: Jan Kundrát See-also: https://www.spinics.net/lists/netdev/msg490554.html Reviewed-by: Jesse Brandeburg --- drivers/net/ethernet/intel/igb/igb_main.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index d9c3a6b169f9..9f3aeb636e4a 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -576,16 +576,15 @@ static void igb_set_i2c_data(void *data, int state) struct e1000_hw *hw = &adapter->hw; s32 i2cctl = rd32(E1000_I2CPARAMS); - if (state) - i2cctl |= E1000_I2C_DATA_OUT; - else + if (state) { + i2cctl |= E1000_I2C_DATA_OUT | E1000_I2C_DATA_OE_N; + } else { + i2cctl &= ~E1000_I2C_DATA_OE_N; i2cctl &= ~E1000_I2C_DATA_OUT; + } - i2cctl &= ~E1000_I2C_DATA_OE_N; - i2cctl |= E1000_I2C_CLK_OE_N; wr32(E1000_I2CPARAMS, i2cctl); wrfl(); - } /** @@ -602,8 +601,7 @@ static void igb_set_i2c_clk(void *data, int state) s32 i2cctl = rd32(E1000_I2CPARAMS); if (state) { - i2cctl |= E1000_I2C_CLK_OUT; - i2cctl &= ~E1000_I2C_CLK_OE_N; + i2cctl |= E1000_I2C_CLK_OUT | E1000_I2C_CLK_OE_N; } else { i2cctl &= ~E1000_I2C_CLK_OUT; i2cctl &= ~E1000_I2C_CLK_OE_N; @@ -2956,11 +2954,20 @@ static void igb_init_mas(struct igb_adapter *adapter) static s32 igb_init_i2c(struct igb_adapter *adapter) { s32 status = 0; + s32 i2cctl; + struct e1000_hw *hw = &adapter->hw; /* I2C interface supported on i350 devices */ if (adapter->hw.mac.type != e1000_i350) return 0; + i2cctl = rd32(E1000_I2CPARAMS); + i2cctl |= E1000_I2CBB_EN + | E1000_I2C_CLK_OUT | E1000_I2C_CLK_OE_N + | E1000_I2C_DATA_OUT | E1000_I2C_DATA_OE_N; + wr32(E1000_I2CPARAMS, i2cctl); + wrfl(); + /* Initialize the i2c bus which is controlled by the registers. * This bus will use the i2c_algo_bit structue that implements * the protocol through toggling of the 4 bits in the register.