From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id D6BDBC05027 for ; Wed, 8 Feb 2023 12:56:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230434AbjBHM4h (ORCPT ); Wed, 8 Feb 2023 07:56:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48066 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229630AbjBHM4h (ORCPT ); Wed, 8 Feb 2023 07:56:37 -0500 Received: from vps0.lunn.ch (vps0.lunn.ch [156.67.10.101]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2FA5630EB6; Wed, 8 Feb 2023 04:56:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Disposition:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:From:Sender:Reply-To:Subject: Date:Message-ID:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Content-Disposition:In-Reply-To:References; bh=5oTXr0EPcDd/e3wIuQRZ5u10MSlrlS9A+107BrjVmHs=; b=lZXrR+cpLuZWFm1TV1kwOBAI8X 3orKSsD9snMKmhpFBtZ2MknaMyc6+SXMyBUch2d47V7zsB7267Y3jphpG0NH5u1HSgxXsgUtg7JXd sKM1ffrc3/sDrSfUlDxW/5wVK3gKYtCgGo/8QX7RMPNu2WwRBAbx058NOymLLOcQIlF8=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1pPjzn-004Oze-F9; Wed, 08 Feb 2023 13:56:19 +0100 Date: Wed, 8 Feb 2023 13:56:19 +0100 From: Andrew Lunn To: Md Danish Anwar Cc: Roger Quadros , MD Danish Anwar , "Andrew F. Davis" , Suman Anna , YueHaibing , Vignesh Raghavendra , Krzysztof Kozlowski , Rob Herring , Paolo Abeni , Jakub Kicinski , Eric Dumazet , "David S. Miller" , nm@ti.com, ssantosh@kernel.org, srk@ti.com, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, netdev@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [EXTERNAL] Re: [EXTERNAL] Re: [PATCH v4 2/2] net: ti: icssg-prueth: Add ICSSG ethernet driver Message-ID: References: <20230206060708.3574472-1-danishanwar@ti.com> <20230206060708.3574472-3-danishanwar@ti.com> <42503a0d-b434-bbcc-553d-a326af5b4918@ti.com> <4438fb71-7e20-6532-a858-b688bc64e826@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4438fb71-7e20-6532-a858-b688bc64e826@ti.com> Precedence: bulk List-ID: X-Mailing-List: linux-omap@vger.kernel.org > >>>> +static int prueth_config_rgmiidelay(struct prueth *prueth, > >>>> + struct device_node *eth_np, > >>>> + phy_interface_t phy_if) > >>>> +{ > >>> > >>> ... > >>> > >>>> + if (phy_if == PHY_INTERFACE_MODE_RGMII_ID || > >>>> + phy_if == PHY_INTERFACE_MODE_RGMII_TXID) > >>>> + rgmii_tx_id |= ICSSG_CTRL_RGMII_ID_MODE; > >>>> + > >>>> + regmap_update_bits(ctrl_mmr, icssgctrl_reg, ICSSG_CTRL_RGMII_ID_MODE, rgmii_tx_id); > > > > This is only applicable to some devices so you need to restrict this only > > to those devices. > > > > Currently ICSSG driver is getting upstreamed for AM65 SR2.0 device, so I don't > think there is any need for any device related restriction. Once support for > other devices are enabled for upstream, we can modify this accordingly. The problem is, this is a board property, not a SoC property. What if somebody designs a board with extra long clock lines in order to add the delay? > I checked the latest Technical Reference Manual [1] (Section 5.1.3.4.49, Table > 5-624) for AM65 Silicon Revision 2.0. > > Below is the description in Table 5-624 > > BIT : 24 > Field : RGMII0_ID_MODE > Type : R/W > Reset : 0h > Description : Controls the PRU_ICSSG0 RGMII0 port internal transmit delay > 0h - Internal transmit delay is enabled > 1h - Reserved > > The TX internal delay is always enabled and couldn't be disabled as 1h is > reserved. So hardware support for disabling TX internal delay is not there. So if somebody passes a phy-mode which requires it disabled, you need to return -EINVAL, to indicate the hardware cannot actually do it. > As, TX internal delay is always there, there is no need to enable it in MAC or > PHY. So no need of API prueth_config_rgmiidelay(). > > My approach to handle delay would be as below. > > *) Keep phy-mode = "rgmii-id" in DT as asked by Andrew. As i said this depends on the board, not the SoC. In theory, you could design a board with an extra long RX clock line, and then use phy-mode rgmii-txid, meaning the MAC/PHY combination needs to add the TX delay. > *) Let TX internal delay enabled in Hardware. > *) Let PHY configure RX internal delay. > *) Remove prueth_config_rgmiidelay() API is there is no use of this. TX > Internal delay is always enabled. > *) Instead of calling prueth_config_rgmiidelay() API in prueth_netdev_init() > API, add below if condition. > > if(emac->phy_if == PHY_INTERFACE_MODE_RGMII_ID) > emac->phy_if == PHY_INTERFACE_MODE_RGMII_RXID You should handle all cases where a TX delay is requested, not just ID. Andrew From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 4A2CDC05027 for ; Wed, 8 Feb 2023 12:57:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=UGgxzAJopmZGLK7g9wbEifzjE2JpK6VVbiFP3hBtDf4=; b=JC1kis2baP1KgA oiaWrT/6A7ri/20ZSGOk8KrIgGU8g6mFIxAMEBk98DBhWBx/pVyf3313Z39c/i4vvuWMXTMCfOej4 by/ywdT4qEUn5i0fmazXR657XR/BbagL6Ooz/Xvv7fm8zDIgMa6MfBT5GEsvTk2O67WMDrNITNGLq kieuWEhIPxn+l/S7JFiplRzC56QOnWUrUBhDgQTilcZXAadrBoTFySWhcOzeDMicQRR0lzXYJ9Ugj RDrkj/+EICazFmshKy2Y8hfWXVD/pfd0I4cqooFDtmr09odBsYCXxPPwW72PSENutPdFRhortDcFi n7S+mR+RmuY/5P1hDNFQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pPk0C-00FfLK-3W; Wed, 08 Feb 2023 12:56:44 +0000 Received: from vps0.lunn.ch ([156.67.10.101]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1pPk09-00FfJy-LO for linux-arm-kernel@lists.infradead.org; Wed, 08 Feb 2023 12:56:43 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lunn.ch; s=20171124; h=In-Reply-To:Content-Disposition:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:From:Sender:Reply-To:Subject: Date:Message-ID:To:Cc:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Content-Disposition:In-Reply-To:References; bh=5oTXr0EPcDd/e3wIuQRZ5u10MSlrlS9A+107BrjVmHs=; b=lZXrR+cpLuZWFm1TV1kwOBAI8X 3orKSsD9snMKmhpFBtZ2MknaMyc6+SXMyBUch2d47V7zsB7267Y3jphpG0NH5u1HSgxXsgUtg7JXd sKM1ffrc3/sDrSfUlDxW/5wVK3gKYtCgGo/8QX7RMPNu2WwRBAbx058NOymLLOcQIlF8=; Received: from andrew by vps0.lunn.ch with local (Exim 4.94.2) (envelope-from ) id 1pPjzn-004Oze-F9; Wed, 08 Feb 2023 13:56:19 +0100 Date: Wed, 8 Feb 2023 13:56:19 +0100 From: Andrew Lunn To: Md Danish Anwar Subject: Re: [EXTERNAL] Re: [EXTERNAL] Re: [PATCH v4 2/2] net: ti: icssg-prueth: Add ICSSG ethernet driver Message-ID: References: <20230206060708.3574472-1-danishanwar@ti.com> <20230206060708.3574472-3-danishanwar@ti.com> <42503a0d-b434-bbcc-553d-a326af5b4918@ti.com> <4438fb71-7e20-6532-a858-b688bc64e826@ti.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <4438fb71-7e20-6532-a858-b688bc64e826@ti.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230208_045641_722192_059C132E X-CRM114-Status: GOOD ( 22.03 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: nm@ti.com, srk@ti.com, Paolo Abeni , Vignesh Raghavendra , linux-omap@vger.kernel.org, devicetree@vger.kernel.org, netdev@vger.kernel.org, YueHaibing , linux-kernel@vger.kernel.org, MD Danish Anwar , "Andrew F. Davis" , Roger Quadros , Eric Dumazet , Rob Herring , Krzysztof Kozlowski , ssantosh@kernel.org, Jakub Kicinski , "David S. Miller" , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org > >>>> +static int prueth_config_rgmiidelay(struct prueth *prueth, > >>>> + struct device_node *eth_np, > >>>> + phy_interface_t phy_if) > >>>> +{ > >>> > >>> ... > >>> > >>>> + if (phy_if == PHY_INTERFACE_MODE_RGMII_ID || > >>>> + phy_if == PHY_INTERFACE_MODE_RGMII_TXID) > >>>> + rgmii_tx_id |= ICSSG_CTRL_RGMII_ID_MODE; > >>>> + > >>>> + regmap_update_bits(ctrl_mmr, icssgctrl_reg, ICSSG_CTRL_RGMII_ID_MODE, rgmii_tx_id); > > > > This is only applicable to some devices so you need to restrict this only > > to those devices. > > > > Currently ICSSG driver is getting upstreamed for AM65 SR2.0 device, so I don't > think there is any need for any device related restriction. Once support for > other devices are enabled for upstream, we can modify this accordingly. The problem is, this is a board property, not a SoC property. What if somebody designs a board with extra long clock lines in order to add the delay? > I checked the latest Technical Reference Manual [1] (Section 5.1.3.4.49, Table > 5-624) for AM65 Silicon Revision 2.0. > > Below is the description in Table 5-624 > > BIT : 24 > Field : RGMII0_ID_MODE > Type : R/W > Reset : 0h > Description : Controls the PRU_ICSSG0 RGMII0 port internal transmit delay > 0h - Internal transmit delay is enabled > 1h - Reserved > > The TX internal delay is always enabled and couldn't be disabled as 1h is > reserved. So hardware support for disabling TX internal delay is not there. So if somebody passes a phy-mode which requires it disabled, you need to return -EINVAL, to indicate the hardware cannot actually do it. > As, TX internal delay is always there, there is no need to enable it in MAC or > PHY. So no need of API prueth_config_rgmiidelay(). > > My approach to handle delay would be as below. > > *) Keep phy-mode = "rgmii-id" in DT as asked by Andrew. As i said this depends on the board, not the SoC. In theory, you could design a board with an extra long RX clock line, and then use phy-mode rgmii-txid, meaning the MAC/PHY combination needs to add the TX delay. > *) Let TX internal delay enabled in Hardware. > *) Let PHY configure RX internal delay. > *) Remove prueth_config_rgmiidelay() API is there is no use of this. TX > Internal delay is always enabled. > *) Instead of calling prueth_config_rgmiidelay() API in prueth_netdev_init() > API, add below if condition. > > if(emac->phy_if == PHY_INTERFACE_MODE_RGMII_ID) > emac->phy_if == PHY_INTERFACE_MODE_RGMII_RXID You should handle all cases where a TX delay is requested, not just ID. Andrew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel