From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752103AbdKDTiF (ORCPT ); Sat, 4 Nov 2017 15:38:05 -0400 Received: from vps0.lunn.ch ([185.16.172.187]:42091 "EHLO vps0.lunn.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750959AbdKDTiC (ORCPT ); Sat, 4 Nov 2017 15:38:02 -0400 Date: Sat, 4 Nov 2017 20:37:31 +0100 From: Andrew Lunn To: Stefan Wahren Cc: Florian Fainelli , Mark Rutland , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , Sean Wang , Martin Kaiser , Herbert Xu , Scott Branden , Ray Jui , Matt Mackall , Russell King , Krzysztof Kozlowski , linux-kernel@vger.kernel.org, Eric Anholt , Harald Freudenberger , Rob Herring , "maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE..." , "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" , PrasannaKumar Muralidharan , Steffen Trumtrar , "moderated list:BROADCOM BCM2835 ARM ARCHITECTURE" , "moderated list:BROADCOM BCM2835 ARM ARCHITECTURE" Subject: Re: [PATCH 07/12] hwrng: bcm2835-rng: Manage an optional clock Message-ID: <20171104193731.GA751@lunn.ch> References: <20171102010408.27736-1-f.fainelli@gmail.com> <20171102010408.27736-8-f.fainelli@gmail.com> <944505791.223880.1509803454887@email.1und1.de> <547786283.132202.1509819754396@email.1und1.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <547786283.132202.1509819754396@email.1und1.de> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Florian > > >> + /* Clock is optional on most platforms */ > > >> + priv->clk = devm_clk_get(dev, NULL); > > >> + if (IS_ERR(priv->clk)) > > >> + priv->clk = NULL; > > > > > > at least in case of EPROBE_DEFERED this isn't the expected behavior. Maybe we should better trigger on non-existing clock? > > > > Good point, so more like: > > > > if (IS_ERR(priv->clk) && PTR_ERR(priv->clk) == -ENODEV)? > > Unfortunately we need to return the error in all other cases. Please > take a look at devm_usb_get_phy in dwc2 [1]. AFAIK we don't need to > take care of ENXIO in our case. A few subsystems have a get_optional() call, e.g. devm_phy_optional_get(). It does not return an error when the phy is not supposed to exist, but in all other cases, it does. Maybe consider adding devm_clk_get_optional()? Andrew