From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756725AbcEaWeA (ORCPT ); Tue, 31 May 2016 18:34:00 -0400 Received: from mout.kundenserver.de ([217.72.192.75]:62088 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752788AbcEaWd6 (ORCPT ); Tue, 31 May 2016 18:33:58 -0400 From: Arnd Bergmann To: David Miller Cc: Yuval.Mintz@qlogic.com, manish.chopra@qlogic.com, Sudarsana.Kalluru@qlogic.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Ariel.Elior@qlogic.com Subject: Re: [PATCH] qed: fix qed_fill_link() error handling Date: Wed, 01 Jun 2016 00:34:20 +0200 Message-ID: <5288285.fZuaAytaxX@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-22-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <20160531.142046.2026659803711147043.davem@davemloft.net> References: <1464623197-2084229-1-git-send-email-arnd@arndb.de> <20160531.142046.2026659803711147043.davem@davemloft.net> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:LZFd7uiOhDJsttj14BCKWO4TwmXUKM7VHuHR6V8cW5MZ6LHsku8 l7JYwAbScSV3Z2VdmiZUPNCFFPZhTBUbRCAYuk5uzoUq2JorkV9XMvDfmCWj1yO0Ktybf1v ieA4mxYLoRSQztm91TZwo+3mY4F9pK9RnmOfbumKX7tgLA9KueqHY22poLwab2rACBXCxgg tKjxtGmDUctolRgKmjidQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:gBPCCFWsdK0=:qi3BfFcOyCtEwAvYW5Tefz LTn+wnRal5OrdZ2DfSDzyfEPN6nyCqd827H1/FeI+kGSXzb//ihS1r9dA+tHydYjc6Hm/YPw/ 9t90tTNTW0JUWOY734vd8fQBNm1c34sN2mD2E5Gwx80tLwp6oE0uNapHmjz9ktDAF1iIDIp7X QzxcQURPAIDbvo6LtGUiXcJiXPzkV0LZYt77rPOGBTyZiWka/77R1Alan8R5etdGIsIHxRGdP ZheRqWDbTjPY1k+aOGeJs29sEMcsvjWmd7/BXCHm/BenbxJL7zM7zX43mfbSO7Cfp39jKTqFI 7WP3AfyC/WhXAIkyldXiVTNYvDZbSQZ1UstacRIOjjlK1bF2Qn95LTtZXckOkuQfukdSElIHw ml1O0hw9Emt08bS9lk/pSEq9BE/9op9AYLkyD9H3D8tF58ZsT6FdMneni4pnOlitv+IB+zqUi suTH7jDOIpyvroTnAF5RhZRElbvxCxkAPuogoXksAKGyZ8QxyhOV9dPWDZkopf5RlcvEnOuDv 4E5N+Tgp2RqL1GTQLzBKUoYxrulFOHV5809CvTMhd4+Ddkr7uUpkUC74llzAl786Lr41qPsV/ WdiMODuU7pccHNpZvTXYV1JLn3IbWP+17IARyDZb3pWkPFXqV5a8JlZrPAdLZda13hrJp1cqK dYwmGs1pCVPH6zDCjpFIEry2OdqSSH4aCdltuUHvHOKMY5QxidbhdbCCsd0htxWK2/yvUY6bR K2P3gp7YMKbw96c5 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday, May 31, 2016 2:20:46 PM CEST David Miller wrote: > From: Yuval Mintz > Date: Mon, 30 May 2016 16:24:07 +0000 > > >> + if (IS_ENABLED(CONFIG_QED_SRIOV) && !IS_PF(hwfn->cdev)) { > >> + qed_vf_get_link_params(hwfn, params); > >> + qed_vf_get_link_state(hwfn, link); > >> + qed_vf_get_link_caps(hwfn, link_caps); > >> + > >> + return 0; > >> + } > > > > The IS_ENABLED here seems a bit wasteful to me - we have empty implementation > > under qed_vf.h just for this case [I.e., that SRIOV isn't enabled for qed]. > > If all we're trying achieve is removing these gcc warnings, I think we can simply > > memset the structs in the currently-empty qed_vf_get_link_* functions. Adding a memset() to those functions would add a bit of overhead in code size because that ends up being unused in practice without a way for the compiler to know, I added the IS_ENABLED() check to reduce the object code size here by also eliminating the check for IS_PF(). > I think both solutions are equally valid/elegant. > > Arnd? I think we can just remove the IS_ENABLED() check there and define the IS_PF() macro conditionally to become 'true' if CONFIG_QED_SRIOV is not set, like some other drivers do diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c index 287f61c20c19..756176525cf9 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_main.c +++ b/drivers/net/ethernet/qlogic/qed/qed_main.c @@ -1110,7 +1110,7 @@ static int qed_get_link_data(struct qed_hwfn *hwfn, { void *p; - if (IS_ENABLED(CONFIG_QED_SRIOV) && !IS_PF(hwfn->cdev)) { + if (!IS_PF(hwfn->cdev)) { qed_vf_get_link_params(hwfn, params); qed_vf_get_link_state(hwfn, link); qed_vf_get_link_caps(hwfn, link_caps); diff --git a/drivers/net/ethernet/qlogic/qed/qed_sriov.h b/drivers/net/ethernet/qlogic/qed/qed_sriov.h index c8667c65e685..c90b2b6ad969 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_sriov.h +++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.h @@ -12,11 +12,13 @@ #include "qed_vf.h" #define QED_VF_ARRAY_LENGTH (3) +#ifdef CONFIG_QED_SRIOV #define IS_VF(cdev) ((cdev)->b_is_vf) #define IS_PF(cdev) (!((cdev)->b_is_vf)) -#ifdef CONFIG_QED_SRIOV #define IS_PF_SRIOV(p_hwfn) (!!((p_hwfn)->cdev->p_iov_info)) #else +#define IS_VF(cdev) (0) +#define IS_PF(cdev) (1) #define IS_PF_SRIOV(p_hwfn) (0) #endif #define IS_PF_SRIOV_ALLOC(p_hwfn) (!!((p_hwfn)->pf_iov_info)) I don't see why that isn't already the case actually. If this is ok, I'll send an updated patch. For the PF case, we still need to fix the qed_mcp_get_link_params() failure case, so the rest of my patch is needed anyway, regardless of how we address the warning. Arnd