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 X-Spam-Level: X-Spam-Status: No, score=-8.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 70A0AC1551B for ; Thu, 3 Dec 2020 13:51:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 18C11206F9 for ; Thu, 3 Dec 2020 13:51:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729140AbgLCNu5 (ORCPT ); Thu, 3 Dec 2020 08:50:57 -0500 Received: from fllv0015.ext.ti.com ([198.47.19.141]:55388 "EHLO fllv0015.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726366AbgLCNu4 (ORCPT ); Thu, 3 Dec 2020 08:50:56 -0500 Received: from fllv0034.itg.ti.com ([10.64.40.246]) by fllv0015.ext.ti.com (8.15.2/8.15.2) with ESMTP id 0B3Dnqdr070125; Thu, 3 Dec 2020 07:49:52 -0600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1607003393; bh=heQpcHf6Hj5/Jowm9lAGLrqIo4IcPttU+vpaQxrvHsY=; h=Subject:To:CC:References:From:Date:In-Reply-To; b=f7Cg6d1tPUtilSaAddetBpqCrhecrHzcoCdFosdrxW1FqVaM0FiOg0g04zUS8lL9t WB7Ev3dMp6Vzl/DqhX+M0lT0eAZaKXAtgJeu+eVJmVZuY4A6SsI10fVBmIjeI3uehg 8Rxa1MD5RiM0Svg4p02XK0TjYjsDZpx+Nc7cO28k= Received: from DLEE112.ent.ti.com (dlee112.ent.ti.com [157.170.170.23]) by fllv0034.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 0B3Dnqao114126 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 3 Dec 2020 07:49:52 -0600 Received: from DLEE108.ent.ti.com (157.170.170.38) by DLEE112.ent.ti.com (157.170.170.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1979.3; Thu, 3 Dec 2020 07:49:52 -0600 Received: from lelv0327.itg.ti.com (10.180.67.183) by DLEE108.ent.ti.com (157.170.170.38) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1979.3 via Frontend Transport; Thu, 3 Dec 2020 07:49:52 -0600 Received: from [10.250.233.179] (ileax41-snat.itg.ti.com [10.172.224.153]) by lelv0327.itg.ti.com (8.15.2/8.15.2) with ESMTP id 0B3DnnJI076106; Thu, 3 Dec 2020 07:49:49 -0600 Subject: Re: [PATCH 3/4] net: ti: am65-cpsw-nuss: Add switchdev support To: Andrew Lunn CC: "David S . Miller" , Jakub Kicinski , Grygorii Strashko , Jonathan Corbet , Jiri Pirko , , , , Linux ARM Mailing List References: <20201130082046.16292-1-vigneshr@ti.com> <20201130082046.16292-4-vigneshr@ti.com> <20201130172034.GF2073444@lunn.ch> From: Vignesh Raghavendra Message-ID: Date: Thu, 3 Dec 2020 19:19:48 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20201130172034.GF2073444@lunn.ch> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/30/20 10:50 PM, Andrew Lunn wrote: >> +static int am65_cpsw_port_stp_state_set(struct am65_cpsw_port *port, >> + struct switchdev_trans *trans, u8 state) >> +{ >> + struct am65_cpsw_common *cpsw = port->common; >> + u8 cpsw_state; >> + int ret = 0; >> + >> + if (switchdev_trans_ph_prepare(trans)) >> + return 0; >> + >> + switch (state) { >> + case BR_STATE_FORWARDING: >> + cpsw_state = ALE_PORT_STATE_FORWARD; >> + break; >> + case BR_STATE_LEARNING: >> + cpsw_state = ALE_PORT_STATE_LEARN; >> + break; >> + case BR_STATE_DISABLED: >> + cpsw_state = ALE_PORT_STATE_DISABLE; >> + break; >> + case BR_STATE_LISTENING: >> + case BR_STATE_BLOCKING: >> + cpsw_state = ALE_PORT_STATE_BLOCK; >> + break; >> + default: >> + return -EOPNOTSUPP; >> + } > > Strictly speaking, the: > >> + if (switchdev_trans_ph_prepare(trans)) >> + return 0; > > should be here. In the prepare phase, you are suppose to validate you > can do the requested action, and return an error is not. In second > phase, actually carrying out the action, you then never return an > error. > > But in this case, you are handling all the bridge states, so it should > not matter. > Yeah, since driver is interested in all STP states, I preferred to terminate the function early for prepare phase. Adding switch statement with just "return 0" for all states during prepare phase looked redundant to me. Thanks for the review! Regards Vignesh 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 X-Spam-Level: X-Spam-Status: No, score=-6.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 27278C433C1 for ; Thu, 3 Dec 2020 14:45:53 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A875322D50 for ; Thu, 3 Dec 2020 14:45:52 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A875322D50 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=ti.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; 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:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=2ZJ9ZvYpk1riKBcSjND4Epr7pGFW1DC/DkKMCpuXVBI=; b=oDgR1Xd9OsjXkynIXOwZPsiU0 zqktcsz5/tyt9179LCpOvxs+znedARh3MWYZ1dn3qY74xWsO+3pLC/ad0ufqnBSWnqXyVTBdZnQa/ NIwbJADFQTSJDI851F9dFxBgcg4CspKAF53sWPyPayQSrMH2VZ/2I5Lm5QIUjhxD4VUbuPwzLiUKr LhglM43stg4S2fMI0AO2eGl/PSFBdBYjW3MY7jQZfY3M5F+jk+XZ2Xu2gDyTbhyrfJWw7W5KG41p+ ACq2Cp56j+noomuyIYJj5PfE6Ka3LLgAzX61+dldEIh/xNbo/AL6Pz+SKWrpQIQWLx6TUfCWbQIws SlOXCZUeg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kkozi-0001oC-W9; Thu, 03 Dec 2020 13:50:03 +0000 Received: from fllv0015.ext.ti.com ([198.47.19.141]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kkozf-0001nT-0S for linux-arm-kernel@lists.infradead.org; Thu, 03 Dec 2020 13:50:00 +0000 Received: from fllv0034.itg.ti.com ([10.64.40.246]) by fllv0015.ext.ti.com (8.15.2/8.15.2) with ESMTP id 0B3Dnqdr070125; Thu, 3 Dec 2020 07:49:52 -0600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1607003393; bh=heQpcHf6Hj5/Jowm9lAGLrqIo4IcPttU+vpaQxrvHsY=; h=Subject:To:CC:References:From:Date:In-Reply-To; b=f7Cg6d1tPUtilSaAddetBpqCrhecrHzcoCdFosdrxW1FqVaM0FiOg0g04zUS8lL9t WB7Ev3dMp6Vzl/DqhX+M0lT0eAZaKXAtgJeu+eVJmVZuY4A6SsI10fVBmIjeI3uehg 8Rxa1MD5RiM0Svg4p02XK0TjYjsDZpx+Nc7cO28k= Received: from DLEE112.ent.ti.com (dlee112.ent.ti.com [157.170.170.23]) by fllv0034.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 0B3Dnqao114126 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 3 Dec 2020 07:49:52 -0600 Received: from DLEE108.ent.ti.com (157.170.170.38) by DLEE112.ent.ti.com (157.170.170.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1979.3; Thu, 3 Dec 2020 07:49:52 -0600 Received: from lelv0327.itg.ti.com (10.180.67.183) by DLEE108.ent.ti.com (157.170.170.38) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1979.3 via Frontend Transport; Thu, 3 Dec 2020 07:49:52 -0600 Received: from [10.250.233.179] (ileax41-snat.itg.ti.com [10.172.224.153]) by lelv0327.itg.ti.com (8.15.2/8.15.2) with ESMTP id 0B3DnnJI076106; Thu, 3 Dec 2020 07:49:49 -0600 Subject: Re: [PATCH 3/4] net: ti: am65-cpsw-nuss: Add switchdev support To: Andrew Lunn References: <20201130082046.16292-1-vigneshr@ti.com> <20201130082046.16292-4-vigneshr@ti.com> <20201130172034.GF2073444@lunn.ch> From: Vignesh Raghavendra Message-ID: Date: Thu, 3 Dec 2020 19:19:48 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <20201130172034.GF2073444@lunn.ch> Content-Language: en-US X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201203_084959_169962_98ABEA7B X-CRM114-Status: GOOD ( 19.46 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Grygorii Strashko , Jonathan Corbet , netdev@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, Jiri Pirko , Jakub Kicinski , "David S . Miller" , Linux ARM Mailing List 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 On 11/30/20 10:50 PM, Andrew Lunn wrote: >> +static int am65_cpsw_port_stp_state_set(struct am65_cpsw_port *port, >> + struct switchdev_trans *trans, u8 state) >> +{ >> + struct am65_cpsw_common *cpsw = port->common; >> + u8 cpsw_state; >> + int ret = 0; >> + >> + if (switchdev_trans_ph_prepare(trans)) >> + return 0; >> + >> + switch (state) { >> + case BR_STATE_FORWARDING: >> + cpsw_state = ALE_PORT_STATE_FORWARD; >> + break; >> + case BR_STATE_LEARNING: >> + cpsw_state = ALE_PORT_STATE_LEARN; >> + break; >> + case BR_STATE_DISABLED: >> + cpsw_state = ALE_PORT_STATE_DISABLE; >> + break; >> + case BR_STATE_LISTENING: >> + case BR_STATE_BLOCKING: >> + cpsw_state = ALE_PORT_STATE_BLOCK; >> + break; >> + default: >> + return -EOPNOTSUPP; >> + } > > Strictly speaking, the: > >> + if (switchdev_trans_ph_prepare(trans)) >> + return 0; > > should be here. In the prepare phase, you are suppose to validate you > can do the requested action, and return an error is not. In second > phase, actually carrying out the action, you then never return an > error. > > But in this case, you are handling all the bridge states, so it should > not matter. > Yeah, since driver is interested in all STP states, I preferred to terminate the function early for prepare phase. Adding switch statement with just "return 0" for all states during prepare phase looked redundant to me. Thanks for the review! Regards Vignesh _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel