Message ID | 20230425054429.3956535-3-s-vadapalli@ti.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp3191075vqo; Mon, 24 Apr 2023 22:47:47 -0700 (PDT) X-Google-Smtp-Source: AKy350bO5ya3Vsew7KAKvMq/WTBYVHg/krN4qrjDME858LwzKMOJQHGJcJAwbOxZXaya1MRuY+1S X-Received: by 2002:a17:90b:1643:b0:247:6be7:8cc0 with SMTP id il3-20020a17090b164300b002476be78cc0mr14910522pjb.35.1682401667346; Mon, 24 Apr 2023 22:47:47 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1682401667; cv=none; d=google.com; s=arc-20160816; b=n/LOKWEq0nO8BYkgEJwOabjwz8rzCSSK6PrwIAfjJMsHjQXnMd8VYitXwU7tQF/svJ WgP3uIOqBPzHGfSpWUoqOjfL+0NFqppH81tIs7XYeViVADqhHh+ft5P6qhbB1/sOIXm8 kFKqV7y6LeRBUeG8NUQC57twJqEeUj0wpUkPS8dw8pxDO/V6vCclGD7qDh7EmEtlwFkT vpLDKCpKo0J/sbt0qChN1c6IJX0y9CI7xHdcJnKZHiI0t5jry0e7BGGU5IPeWNrb+WNT 6jt+rRWcCzfkT1CGZPL7cg0B3dcr9KlZGd0ZKwjX9K4ftYn9oXFX2b0jlmBCaHb6goFC wv3A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=3wDxwAR+CCOnwWR6upvy6Zihsi8yqla39FCimDo+1BM=; b=pThYAkE6AAH8U8cgyThhZHrjFlewX5SDWgMqfUJec8IH+1zu3q5NObGttms3cL6aLa FKZEnwpmAmjId+ZJV17nFKErePbCvSMhfrHhGrhFKiFSvUWzXTq5+oPjyJBae9xyXTKB NB9ACttmS5SX8F/xeS00ZrSt02xuXMTqf9rIwYs1pbRTUXD+LZc4mfM+XL/Sut0SSC2U W4OSQOSX5CSbpHFsh4BvSIDijBiFaJEDHfCtARaCLmwb+EzFrTIDdNyAmY8cHB9yhYUg U6ftPAhezmtJxc8nqnnkk/sICxUJEquD19WHZMoVQJ6DCH/T5OG6HG2dsKZWtimTAslz q6cg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@ti.com header.s=ti-com-17Q1 header.b=BV74RoKd; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=NONE dis=NONE) header.from=ti.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id ne4-20020a17090b374400b0024695095bb3si1122625pjb.189.2023.04.24.22.47.32; Mon, 24 Apr 2023 22:47:47 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@ti.com header.s=ti-com-17Q1 header.b=BV74RoKd; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=NONE dis=NONE) header.from=ti.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233333AbjDYFpF (ORCPT <rfc822;zxc52fgh@gmail.com> + 99 others); Tue, 25 Apr 2023 01:45:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55780 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233167AbjDYFpD (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 25 Apr 2023 01:45:03 -0400 Received: from fllv0015.ext.ti.com (fllv0015.ext.ti.com [198.47.19.141]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 93F4F83EC; Mon, 24 Apr 2023 22:45:00 -0700 (PDT) Received: from fllv0035.itg.ti.com ([10.64.41.0]) by fllv0015.ext.ti.com (8.15.2/8.15.2) with ESMTP id 33P5ifmp104119; Tue, 25 Apr 2023 00:44:41 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1682401481; bh=3wDxwAR+CCOnwWR6upvy6Zihsi8yqla39FCimDo+1BM=; h=From:To:CC:Subject:Date:In-Reply-To:References; b=BV74RoKdPWYIXkTDAic5TBueevO2QWIMX8XZ6Bagx8t6RJ8jpKD88H9U+d9PcdMtx c1Xgp40xzaq7Mzsg1HOH+2ipGSi8w7BgLK6LVNdO+bMd9xABEJHUa9faV3v3r5A/dP 2vFPp3l8XcP+jE73IOwxCvj9uF/8Ulg9lv0kszmM= Received: from DFLE113.ent.ti.com (dfle113.ent.ti.com [10.64.6.34]) by fllv0035.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 33P5if1g112335 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 25 Apr 2023 00:44:41 -0500 Received: from DFLE110.ent.ti.com (10.64.6.31) by DFLE113.ent.ti.com (10.64.6.34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.16; Tue, 25 Apr 2023 00:44:41 -0500 Received: from lelv0327.itg.ti.com (10.180.67.183) by DFLE110.ent.ti.com (10.64.6.31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.16 via Frontend Transport; Tue, 25 Apr 2023 00:44:41 -0500 Received: from uda0492258.dhcp.ti.com (ileaxei01-snat2.itg.ti.com [10.180.69.6]) by lelv0327.itg.ti.com (8.15.2/8.15.2) with ESMTP id 33P5iURp124283; Tue, 25 Apr 2023 00:44:38 -0500 From: Siddharth Vadapalli <s-vadapalli@ti.com> To: <andrew@lunn.ch>, <hkallweit1@gmail.com>, <linux@armlinux.org.uk>, <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com> CC: <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-kernel@lists.infradead.org>, <srk@ti.com>, <s-vadapalli@ti.com> Subject: [RFC PATCH 2/2] net: phy: dp83869: fix mii mode when rgmii strap cfg is used Date: Tue, 25 Apr 2023 11:14:29 +0530 Message-ID: <20230425054429.3956535-3-s-vadapalli@ti.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20230425054429.3956535-1-s-vadapalli@ti.com> References: <20230425054429.3956535-1-s-vadapalli@ti.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1764126010753797002?= X-GMAIL-MSGID: =?utf-8?q?1764126010753797002?= |
Series |
DP83867/DP83869 Ethernet PHY workaround/fix
|
|
Commit Message
Siddharth Vadapalli
April 25, 2023, 5:44 a.m. UTC
From: Grygorii Strashko <grygorii.strashko@ti.com> The DP83869 PHY on TI's k3-am642-evm supports both MII and RGMII interfaces and is configured by default to use RGMII interface (strap). However, the board design allows switching dynamically to MII interface for testing purposes by applying different set of pinmuxes. To support switching to MII interface, update the DP83869 PHY driver to configure OP_MODE_DECODE.RGMII_MII_SEL(bit 5) properly when MII PHY interface mode is requested. Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> --- drivers/net/phy/dp83869.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Comments
On Tue, Apr 25, 2023 at 11:14:29AM +0530, Siddharth Vadapalli wrote: > From: Grygorii Strashko <grygorii.strashko@ti.com> > > The DP83869 PHY on TI's k3-am642-evm supports both MII and RGMII > interfaces and is configured by default to use RGMII interface (strap). > However, the board design allows switching dynamically to MII interface > for testing purposes by applying different set of pinmuxes. Only for testing? So nobody should actually design a board to use MII and use software to change the interface from RGMII to MII? This does not seem to be a fix, it is a new feature. So please submit to net-next, in two weeks time when it opens again. > @@ -692,8 +692,11 @@ static int dp83869_configure_mode(struct phy_device *phydev, > /* Below init sequence for each operational mode is defined in > * section 9.4.8 of the datasheet. > */ > + phy_ctrl_val = dp83869->mode; > + if (phydev->interface == PHY_INTERFACE_MODE_MII) > + phy_ctrl_val |= DP83869_OP_MODE_MII; Should there be some validation here with dp83869->mode? DP83869_RGMII_COPPER_ETHERNET, DP83869_RGMII_SGMII_BRIDGE etc don't make sense if MII is being used. DP83869_100M_MEDIA_CONVERT and maybe DP83869_RGMII_100_BASE seem to be the only valid modes with MII? Andrew
On 25/04/23 17:48, Andrew Lunn wrote: > On Tue, Apr 25, 2023 at 11:14:29AM +0530, Siddharth Vadapalli wrote: >> From: Grygorii Strashko <grygorii.strashko@ti.com> >> >> The DP83869 PHY on TI's k3-am642-evm supports both MII and RGMII >> interfaces and is configured by default to use RGMII interface (strap). >> However, the board design allows switching dynamically to MII interface >> for testing purposes by applying different set of pinmuxes. > > Only for testing? So nobody should actually design a board to use MII > and use software to change the interface from RGMII to MII? > > This does not seem to be a fix, it is a new feature. So please submit > to net-next, in two weeks time when it opens again. Sure. I will split this patch from the series and post the v2 of this patch with the subject "RFC PATCH net-next" for requesting further feedback on this patch if needed. Following that, I will post it to net-next as a new patch. > >> @@ -692,8 +692,11 @@ static int dp83869_configure_mode(struct phy_device *phydev, >> /* Below init sequence for each operational mode is defined in >> * section 9.4.8 of the datasheet. >> */ >> + phy_ctrl_val = dp83869->mode; >> + if (phydev->interface == PHY_INTERFACE_MODE_MII) >> + phy_ctrl_val |= DP83869_OP_MODE_MII; > > Should there be some validation here with dp83869->mode? > > DP83869_RGMII_COPPER_ETHERNET, DP83869_RGMII_SGMII_BRIDGE etc don't > make sense if MII is being used. DP83869_100M_MEDIA_CONVERT and maybe > DP83869_RGMII_100_BASE seem to be the only valid modes with MII? The DP83869_OP_MODE_MII macro corresponds to BIT(5) which is the RGMII_MII_SEL bit in the OP_MODE_DECODE register. If the RGMII_MII_SEL bit is set, MII mode is selected. If the bit is cleared, which is the default value, RGMII mode is selected. As pointed out by you, there are modes which aren't valid with MII mode. However, a mode which isn't valid with RGMII mode (default value of the RGMII_MII_SEL bit) also exists: DP83869_SGMII_COPPER_ETHERNET. For this reason, I believe that setting the bit when MII mode is requested shouldn't cause any issues. > > Andrew
> >> @@ -692,8 +692,11 @@ static int dp83869_configure_mode(struct phy_device *phydev, > >> /* Below init sequence for each operational mode is defined in > >> * section 9.4.8 of the datasheet. > >> */ > >> + phy_ctrl_val = dp83869->mode; > >> + if (phydev->interface == PHY_INTERFACE_MODE_MII) > >> + phy_ctrl_val |= DP83869_OP_MODE_MII; > > > > Should there be some validation here with dp83869->mode? > > > > DP83869_RGMII_COPPER_ETHERNET, DP83869_RGMII_SGMII_BRIDGE etc don't > > make sense if MII is being used. DP83869_100M_MEDIA_CONVERT and maybe > > DP83869_RGMII_100_BASE seem to be the only valid modes with MII? > > The DP83869_OP_MODE_MII macro corresponds to BIT(5) which is the RGMII_MII_SEL > bit in the OP_MODE_DECODE register. If the RGMII_MII_SEL bit is set, MII mode is > selected. If the bit is cleared, which is the default value, RGMII mode is > selected. As pointed out by you, there are modes which aren't valid with MII > mode. However, a mode which isn't valid with RGMII mode (default value of the > RGMII_MII_SEL bit) also exists: DP83869_SGMII_COPPER_ETHERNET. For this reason, > I believe that setting the bit when MII mode is requested shouldn't cause any > issues. If you say so. I was just thinking you could give the poor software engineer a hint the hardware engineer has put on strapping resistors which means the PHY is not going to work. Andrew
On 26/04/23 18:11, Andrew Lunn wrote: >>>> @@ -692,8 +692,11 @@ static int dp83869_configure_mode(struct phy_device *phydev, >>>> /* Below init sequence for each operational mode is defined in >>>> * section 9.4.8 of the datasheet. >>>> */ >>>> + phy_ctrl_val = dp83869->mode; >>>> + if (phydev->interface == PHY_INTERFACE_MODE_MII) >>>> + phy_ctrl_val |= DP83869_OP_MODE_MII; >>> >>> Should there be some validation here with dp83869->mode? >>> >>> DP83869_RGMII_COPPER_ETHERNET, DP83869_RGMII_SGMII_BRIDGE etc don't >>> make sense if MII is being used. DP83869_100M_MEDIA_CONVERT and maybe >>> DP83869_RGMII_100_BASE seem to be the only valid modes with MII? >> >> The DP83869_OP_MODE_MII macro corresponds to BIT(5) which is the RGMII_MII_SEL >> bit in the OP_MODE_DECODE register. If the RGMII_MII_SEL bit is set, MII mode is >> selected. If the bit is cleared, which is the default value, RGMII mode is >> selected. As pointed out by you, there are modes which aren't valid with MII >> mode. However, a mode which isn't valid with RGMII mode (default value of the >> RGMII_MII_SEL bit) also exists: DP83869_SGMII_COPPER_ETHERNET. For this reason, >> I believe that setting the bit when MII mode is requested shouldn't cause any >> issues. > > If you say so. I was just thinking you could give the poor software > engineer a hint the hardware engineer has put on strapping resistors > which means the PHY is not going to work. I understand now. I will update this patch to add a print if the MII mode is not valid with the configured "dp83869->mode". Would you suggest using a dev_err() or a dev_dbg()? Thank you for the feedback on this series.
> > If you say so. I was just thinking you could give the poor software > > engineer a hint the hardware engineer has put on strapping resistors > > which means the PHY is not going to work. > > I understand now. I will update this patch to add a print if the MII mode is not > valid with the configured "dp83869->mode". Would you suggest using a dev_err() > or a dev_dbg()? dev_err(). And you can return -EINVAL when asked to set the interface mode. Andrew
diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c index 9ab5eff502b7..8dbc502bcd9e 100644 --- a/drivers/net/phy/dp83869.c +++ b/drivers/net/phy/dp83869.c @@ -692,8 +692,11 @@ static int dp83869_configure_mode(struct phy_device *phydev, /* Below init sequence for each operational mode is defined in * section 9.4.8 of the datasheet. */ + phy_ctrl_val = dp83869->mode; + if (phydev->interface == PHY_INTERFACE_MODE_MII) + phy_ctrl_val |= DP83869_OP_MODE_MII; ret = phy_write_mmd(phydev, DP83869_DEVADDR, DP83869_OP_MODE, - dp83869->mode); + phy_ctrl_val); if (ret) return ret;