From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161555AbcHEPu4 (ORCPT ); Fri, 5 Aug 2016 11:50:56 -0400 Received: from mout.kundenserver.de ([217.72.192.73]:56342 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161444AbcHEPuz (ORCPT ); Fri, 5 Aug 2016 11:50:55 -0400 From: Arnd Bergmann To: linux-arm-kernel@lists.infradead.org Cc: Philipp Zabel , Masahiro Yamada , Axel Lin , Linux Kernel Mailing List , Hans de Goede , Maxime Ripard , Lee Jones Subject: Re: Why do we need reset_control_get_optional() ? Date: Fri, 05 Aug 2016 17:50:10 +0200 Message-ID: <1859714.7lR6dsl2IG@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.4.0-31-generic; KDE/5.18.0; x86_64; ; ) In-Reply-To: <1469703649.12835.34.camel@pengutronix.de> References: <1469703649.12835.34.camel@pengutronix.de> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:W5Zh1QhI7PlwpkJYz4B0zzWXrt2t/1/8Ef1He5DfRli4sf8a7ft rcBaq0T1I8qH2HEXeIY+bbZKRaRlkHc4cMgeOFxNbZqp4lxTOdPfEzhQQpgoqqsRGuDWj7X WavaWwm6dnZwjA5P5Yb50KF6R/LDixwOmGMdUnseLyJhpvuO26u76nHZHqyQRTLqe/fL9Z/ y4WBbvPQbgWspnqY1XtOQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:PTUtnOkBd+I=:oqTSSGyYwd+0K7v6/r8T5D /LDRWELYkneIreUyuhRARp1a0x9Cw4znCA/DHCOZ3RuUbsoMUtvDPSFOUrMgt5lzRxWBvYdAB zC4gRX1Bv+mZ7Zd/Ldqe9sXkTXO1VKB/4wrzs47TijtEocAe+rEQ+ydpdq88I4G2QAsnOfLLV V+g+n6G9ZNKQ5prpmAc5yXs5Sz+sNAP3o0M1yyKegzfZcczuyGpwEj1/crdkH8JqWw9lCD67Z R1NbKfCHn2DuPBaPECTW6RACqLk0AiSQYEYrfh7y0LC9cFF5vwsVQ98ojO2POMakeHVIfPoPt kMxVgBKgHil4z8KcTKpnKb3afWOB+LD5SYmG8yMmaLhaYUfFAN6w+gFezOXTti75FnYNPs3CY Nwn5RLIhW9+rlj2WcWsmavGoPre77vVAiOjsX06d/DF1GV/1sS6KZxPGVCuDUbHEDMMVBQdRU bDt0V0Wfj9bXFFlFXMkGz7W5JBlksggINrLB1lGxDgu9AiAD3zQJxD5JcOFnqL0tDGbgHFvNU RXW5pZLgiLbXOlPqspnekNUo1X17BRfCZtUKXoOajPOlKd7PdxoZYlTaT/UaQ4E4gH6a4vGrU 1num/rOuQn6UZWKD36Bf+LODahsG4JxKaSwoB8U+m9HtV/+LlN2TBsaCejqjebazOUuZtcR3e 28PpgizrCVMf35jBYX4NAgjMZ1eZP2QNfgRilJNn6CvanOHlwuXBR4H8ib8HvcYJkn3dn68Cb 4Nyn6ATiLSZFTpjU Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thursday, July 28, 2016 1:00:49 PM CEST Philipp Zabel wrote: > Am Donnerstag, den 28.07.2016, 19:52 +0900 schrieb Masahiro Yamada: > > > In my experimental patch, I make the _optional functions > > > return NULL if no "resets" property is provided but return > > > an error if there are reset lines but the subsystem is > > > disabled, i.e. an optional reset must be used if it's in the > > > DT, but can be ignored otherwise. > > > > I do not like this idea. > > > > reset_control_get() (or variants) should not return NULL, it is ambiguous. > > It should return ERR_PTR(-ENOENT) if no "resets" property. > > > > I only want two types for functions that return a pointer. > > > > [1] return a valid pointer on success, or return NULL on failure > > (for example, kmalloc()) > > [2] return a valid pointer on success, or return error pointer on failure > > (many of _register() functions) > > > > Mixing [1] and [2] will be a mess. Ah, right. I was thinking only of the case where the reset subsystem is completely disabled here, so returning NULL could be considered a valid return code that can in turn be passed into the other functions. However, I agree that returning NULL as a valid result from ..._get_optional() would be bad style, so let's drop my idea there. > I too would prefer to keep that as-is. The reset_control_get_optional > stub could return -ENOENT if there is no resets device tree property. Now I'm also confused about what we really need reset_control_get_optional() for, and which error codes the callers are supposed to check. This is the matrix I think you mean for _get_optional: CONFIG_RESET_CONTROLLER=y, dt entry present: valid pointer (or other error) CONFIG_RESET_CONTROLLER=n, dt entry present: -EOPNOTSUPP CONFIG_RESET_CONTROLLER=y, dt entry missing: -ENOENT CONFIG_RESET_CONTROLLER=n, dt entry missing: -ENOENT Is this what you had in mind? If so, what is the value of the added runtime warning for reset_control_get? Any caller of that function would already check for errors, the only difference I see is that callers of _optional can ignore -ENOENT. Arnd