From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752064AbdJDLTn (ORCPT ); Wed, 4 Oct 2017 07:19:43 -0400 Received: from mail-oi0-f66.google.com ([209.85.218.66]:38038 "EHLO mail-oi0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751731AbdJDLTl (ORCPT ); Wed, 4 Oct 2017 07:19:41 -0400 X-Google-Smtp-Source: AOwi7QBb2bQOFgKMiodJ82Aq8AKPh7VZVr3geqmB1cy7K9OIT2z3x8dDPLeQmYoysCnnG2fI0kE4rUuxU30LIE5KVsk= MIME-Version: 1.0 In-Reply-To: <1506604306-20739-5-git-send-email-sudeep.holla@arm.com> References: <1506604306-20739-1-git-send-email-sudeep.holla@arm.com> <1506604306-20739-5-git-send-email-sudeep.holla@arm.com> From: Arnd Bergmann Date: Wed, 4 Oct 2017 13:19:40 +0200 X-Google-Sender-Auth: lgQzid01jhDk2Uf7OU2NDcJ1SiA Message-ID: Subject: Re: [PATCH v3 04/22] firmware: arm_scmi: add basic driver infrastructure for SCMI To: Sudeep Holla Cc: ALKML , LKML , DTML , Roy Franz , Harb Abdulhamid , Nishanth Menon , Loc Ho , Alexey Klimov , Ryan Harkin , Jassi Brar Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Sep 28, 2017 at 3:11 PM, Sudeep Holla wrote: > +static int scmi_mbox_free_channel(struct scmi_info *info) > +{ > + if (!IS_ERR_OR_NULL(info->tx_chan)) { > + mbox_free_channel(info->tx_chan); > + info->tx_chan = NULL; > + } > + > + return 0; > +} Any use of IS_ERR_OR_NULL() tends to be an indication of a bad API design. Please make a decision about what you want to store in info->tx_chan and stick with it. Usually, we don't store error pointers in permanent data structures. If you can deal with tx_chan being absent here, then you can just store NULL into it when you don't have one, otherwise you should make sure you never call this and fail the probe function earlier. Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 From: Arnd Bergmann Subject: Re: [PATCH v3 04/22] firmware: arm_scmi: add basic driver infrastructure for SCMI Date: Wed, 4 Oct 2017 13:19:40 +0200 Message-ID: References: <1506604306-20739-1-git-send-email-sudeep.holla@arm.com> <1506604306-20739-5-git-send-email-sudeep.holla@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: <1506604306-20739-5-git-send-email-sudeep.holla-5wv7dgnIgG8@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Sudeep Holla Cc: ALKML , LKML , DTML , Roy Franz , Harb Abdulhamid , Nishanth Menon , Loc Ho , Alexey Klimov , Ryan Harkin , Jassi Brar List-Id: devicetree@vger.kernel.org On Thu, Sep 28, 2017 at 3:11 PM, Sudeep Holla wrote: > +static int scmi_mbox_free_channel(struct scmi_info *info) > +{ > + if (!IS_ERR_OR_NULL(info->tx_chan)) { > + mbox_free_channel(info->tx_chan); > + info->tx_chan = NULL; > + } > + > + return 0; > +} Any use of IS_ERR_OR_NULL() tends to be an indication of a bad API design. Please make a decision about what you want to store in info->tx_chan and stick with it. Usually, we don't store error pointers in permanent data structures. If you can deal with tx_chan being absent here, then you can just store NULL into it when you don't have one, otherwise you should make sure you never call this and fail the probe function earlier. Arnd -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: arnd@arndb.de (Arnd Bergmann) Date: Wed, 4 Oct 2017 13:19:40 +0200 Subject: [PATCH v3 04/22] firmware: arm_scmi: add basic driver infrastructure for SCMI In-Reply-To: <1506604306-20739-5-git-send-email-sudeep.holla@arm.com> References: <1506604306-20739-1-git-send-email-sudeep.holla@arm.com> <1506604306-20739-5-git-send-email-sudeep.holla@arm.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, Sep 28, 2017 at 3:11 PM, Sudeep Holla wrote: > +static int scmi_mbox_free_channel(struct scmi_info *info) > +{ > + if (!IS_ERR_OR_NULL(info->tx_chan)) { > + mbox_free_channel(info->tx_chan); > + info->tx_chan = NULL; > + } > + > + return 0; > +} Any use of IS_ERR_OR_NULL() tends to be an indication of a bad API design. Please make a decision about what you want to store in info->tx_chan and stick with it. Usually, we don't store error pointers in permanent data structures. If you can deal with tx_chan being absent here, then you can just store NULL into it when you don't have one, otherwise you should make sure you never call this and fail the probe function earlier. Arnd