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=-2.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,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 025EFC2D0E5 for ; Fri, 27 Mar 2020 13:37:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id CA58520838 for ; Fri, 27 Mar 2020 13:37:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727345AbgC0Nh3 (ORCPT ); Fri, 27 Mar 2020 09:37:29 -0400 Received: from retiisi.org.uk ([95.216.213.190]:45640 "EHLO hillosipuli.retiisi.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726275AbgC0Nh3 (ORCPT ); Fri, 27 Mar 2020 09:37:29 -0400 Received: from valkosipuli.localdomain (valkosipuli.retiisi.org.uk [IPv6:2a01:4f9:c010:4572::80:2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by hillosipuli.retiisi.org.uk (Postfix) with ESMTPS id 14E41634C90; Fri, 27 Mar 2020 15:37:06 +0200 (EET) Received: from sailus by valkosipuli.localdomain with local (Exim 4.92) (envelope-from ) id 1jHpAX-0000og-4s; Fri, 27 Mar 2020 15:37:05 +0200 Date: Fri, 27 Mar 2020 15:37:05 +0200 From: Sakari Ailus To: Robert Foss Cc: Dongchun Zhu , Fabio Estevam , Andy Shevchenko , Tomasz Figa , linux-media , "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" , linux-kernel , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" Subject: Re: [v2 2/3] media: ov8856: Add devicetree support Message-ID: <20200327133705.GC2394@valkosipuli.retiisi.org.uk> References: <20200313110350.10864-1-robert.foss@linaro.org> <20200313110350.10864-3-robert.foss@linaro.org> <20200313121746.GC5730@valkosipuli.retiisi.org.uk> <20200326144725.GA2394@valkosipuli.retiisi.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hi Robert, On Fri, Mar 27, 2020 at 11:32:29AM +0100, Robert Foss wrote: > On Thu, 26 Mar 2020 at 15:47, Sakari Ailus wrote: > > > > Hi Robert, > > > > On Thu, Mar 26, 2020 at 12:56:37PM +0100, Robert Foss wrote: > > ... > > > > > +static int __ov8856_power_on(struct ov8856 *ov8856) > > > > > +{ > > > > > + struct i2c_client *client = v4l2_get_subdevdata(&ov8856->sd); > > > > > + int ret; > > > > > + > > > > > + ret = clk_prepare_enable(ov8856->xvclk); > > > > > + if (ret < 0) { > > > > > + dev_err(&client->dev, "failed to enable xvclk\n"); > > > > > + return ret; > > > > > + } > > > > > + > > > > > + gpiod_set_value_cansleep(ov8856->reset_gpio, GPIOD_OUT_HIGH); > > > > > + > > > > > + ret = regulator_bulk_enable(ARRAY_SIZE(ov8856_supply_names), > > > > > + ov8856->supplies); > > > > > + if (ret < 0) { > > > > > + dev_err(&client->dev, "failed to enable regulators\n"); > > > > > + goto disable_clk; > > > > > + } > > > > > + > > > > > + gpiod_set_value_cansleep(ov8856->reset_gpio, GPIOD_OUT_LOW); > > > > > + > > > > > + usleep_range(1500, 1800); > > > > > > > > I think you could omit the delay on ACPI based systems. Or just bail out > > > > early in that case. > > > > > > I'll add a check for reset_gpio being NULL, and skip the sleep for that case. > > > > There could also be a regulator but no GPIO. > > > > I think if you don't have either, then certainly there's no need for a > > delay. > > Removing the delay if no action is taken makes sense, but I'm not sure > how best to do it. > If there are no regulators dummy ones are created automatically, which > makes distinguishing between a little bit cumbersome. The regulator > structs could of course all be inspected, and if all are dummy ones, > the delay could be skipped. But is there a neater way of doing this? > Manually inspecting the regs strikes me as a bit inelegant. I guess the cleanest, easy way to make this right, albeit slightly unoptimal in very rare cases where you have none of the above resources in a DT system, is to bail out if you're running on an ACPI based system. I.e. checking for e.g. is_acpi_node(dev->fwnode). -- Sakari Ailus