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=-0.9 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED 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 8ABA6C2BA1E for ; Mon, 6 Apr 2020 15:26:14 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 57B94248A5 for ; Mon, 6 Apr 2020 15:26:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="XBPXHInV"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="rM5SW1yc" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 57B94248A5 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=AVYWEa6EUVek9vOTH9Xjz0Z/9SPJR5k/CFiTNcUngUs=; b=XBPXHInVwvMwMN K4oEBy9EOL1WaUYdERASRRPZmLJIXfkdavDvoR2FVGMzifi87J3sI0xG9C+2A9rVwBl0RtH5c/1s3 11ZMZmE6iSfNzZW78/1yvwbOXJk+snCqSb78JAL/XjTD6MPT4HXjptmGzi0XMcVs0Cl7gPw/boXqa 7PM7GSick0az3GXopv5dwm1c69aksi8kA/ILiJoB0kW8kwl57pNSPz5eNrcnuy66ze+Q/1Qm9qznW DOwnrLMv6QBV5ObR+cD/igEt3yKm5XChZ1mWMvEWyTU3/MSXz7t5MThOdh0rS+p9osXnh7DBNuGFB NplV447xB6cO0rrGNqvg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jLTdd-0004FV-Ut; Mon, 06 Apr 2020 15:26:13 +0000 Received: from mail-ot1-x342.google.com ([2607:f8b0:4864:20::342]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jLTdb-0004FC-Ab for linux-arm-kernel@lists.infradead.org; Mon, 06 Apr 2020 15:26:12 +0000 Received: by mail-ot1-x342.google.com with SMTP id g23so15729989otq.4 for ; Mon, 06 Apr 2020 08:26:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=mLkFZg4uKjsIykk7BTwvQ1u38EFobfLrFUiH9V7o1sk=; b=rM5SW1yckf32vIPkw90xBqCzGHKCy/iHgCoXE6hqaPFebS750iSbilzPRpBtU5Twz1 b2EPRdimrtONwJqOGXXuG7cCPfzkuJI8GXEitZW1DZvfdhnT+acAbHbzVSl8DwHhX8eb SApEs/7LmbMvmXeBcvhGT+g+A9YosMhrHBoZRH4R+i6X4axPNRraO1HMVjVhgvOB0GKH WPncSG4ks6mq3alyrF7YtXCdmZfEJzBoLgaNXPzH6SWsiJwYvEENNhoU/8vX/V/x+0az qPVZh46IQouMAk0ky8JWRrmDASEJ5HZtxWIWRo/hnssv1o7eOH9jGhV+0d+i44iYN0/t 8uhQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=mLkFZg4uKjsIykk7BTwvQ1u38EFobfLrFUiH9V7o1sk=; b=ATdBjcYCNm2lbDUnnfE7Aa+QHPo+II3tcbQf1vzzN2stZNo/j8g074Rh23nzuKzneU 0+SzhbUSXVSIQk9wyF6Pu+APKEOdGaT4W5eVKJvG7P73zWA9nvZ6Gp1fBlQzONQE8Uq4 0Wsp4BvmuS4+3/pALNinmcfIMa4mo1oUVIGCUNR5E8epW+qlerdaJWUTU94p7SHb/izh URkN2YlkzbiJh/VJBalJ2hAcNMDWn4rWVvWtzKggmmTQxOLhT5O2DjivmSMIjFyQsQLr Vec+YXQt0C1vATX43PSZ2CrquQ2uQy3Tai0W2Hrv5C4Pm0yumGCImrqagQbNzvu6Co32 vVgQ== X-Gm-Message-State: AGi0PuZZr2bxB2BPcLaA//LM27SQ2ssoQxChnz+GSTH4XZSVavTP6oEv gzq/YwE4iqHUFukwMtbd16/5Pl3Xkr7CXpkaZnxZSQ== X-Google-Smtp-Source: APiQypKLKKYLmPxZPCftzf+0SBlZXHZMIwDor9EtVAxYxuQ+W8VWcYMwc6GcICK2n6IQIOMhasirVAdBnijIMJ7TltU= X-Received: by 2002:a4a:c595:: with SMTP id x21mr17831239oop.10.1586186770118; Mon, 06 Apr 2020 08:26:10 -0700 (PDT) MIME-Version: 1.0 References: <20200331133346.372517-1-robert.foss@linaro.org> <20200331133346.372517-3-robert.foss@linaro.org> <20200406150626.GW3676135@smile.fi.intel.com> In-Reply-To: <20200406150626.GW3676135@smile.fi.intel.com> From: Robert Foss Date: Mon, 6 Apr 2020 17:25:58 +0200 Message-ID: Subject: Re: [PATCH v3 2/3] media: ov8856: Add devicetree support To: Andy Shevchenko X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200406_082611_394233_5D39E93B X-CRM114-Status: GOOD ( 26.49 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree , Linux Kernel Mailing List , Tomasz Figa , Sakari Ailus , Dongchun Zhu , Fabio Estevam , linux-arm Mailing List , Linux Media Mailing List Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, 6 Apr 2020 at 17:06, Andy Shevchenko wrote: > > On Mon, Apr 06, 2020 at 03:37:24PM +0200, Robert Foss wrote: > > On Tue, 31 Mar 2020 at 16:01, Andy Shevchenko wrote: > > > On Tue, Mar 31, 2020 at 4:36 PM Robert Foss wrote: > > ... > > > > > + if (ret < 0) { > > > > > > Do you need all ' < 0' parts all over the series? > > > > Some checks are needed due to ACPI and DT support co-existing. > > Maybe it would be better to just split the probing into an ACPI path > > and a DT path. > > > > I'll have a look through the series for redundant retval checks. > > Drop where it is redundant. > > ... > > > > > - ret = fwnode_property_read_u32(fwnode, "clock-frequency", &mclk); > > > > - if (ret) > > > > - return ret; > > > > > > Where is it gone? Why? > > > > It was replaced by a clk_get_rate call, which as Sakari pointed out, > > isn't correct. > > I'll rework the clock handling for v4. > > If it was in the driver it should stay -- properties is an ABI (between firmware and kernel). Ack. > > > > > + ov8856->xvclk = devm_clk_get_optional(dev, "xvclk"); > > > > + if (IS_ERR(ov8856->xvclk)) { > > > > > > > + dev_err(dev, "could not get xvclk clock (%ld)\n", > > > > + PTR_ERR(ov8856->xvclk)); > > > > > > Also you may use %pe here and in similar cases. > > > > Weirdly checkpatch complains about this. > > But it builds and runs cleanly, so I'll add it in v4. > > %pe requires pointer, PTR_ERR converts pointer to integer. Ack. > > ... > > > > > + ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(ov8856_supply_names), > > > > + ov8856->supplies); > > > > + if (ret) { > > > > > > > + dev_warn(dev, "failed to get regulators\n"); > > > > > > If it's a warning, why we return from here? > > > Same question to all other places with same issue. > > > > The issue I was seeing was the driver having to return a EDEFER here, > > so this warning sheds some light on which exact component is returning > > an EDEFER. > > > > [ 15.962623] ov8856 16-0010: Dropping the link to regulator.29 > > [ 15.968464] ov8856 16-0010: failed to get regulators > > [ 15.973493] ov8856 16-0010: failed to get HW configuration: -517 > > [ 15.979591] ov8856 16-0010: removing from PM domain titan_top_gdsc > > [ 15.985855] ov8856 16-0010: genpd_remove_device() > > [ 15.990672] i2c 16-0010: Driver ov8856 requests probe deferral > > > > Personally I found it helpful to speed up debugging, but I'll happily > > remove it if you prefer no warning. > > My point is that you have it in align: > - if it is an error, print as an error and bail out, otherwise > - if it is a warning, print it and continue. I see what you're saying now, let's remove it then :) I guess in the specific case of EDEFER, it doesn't fit neatly into either of those categories, in the sense that the way you continue is to return and then try to probe again later. There are some other locations where this is handled wrong, I'll align them properly for v4. > > > > > + return ret; > > > > } > > -- > With Best Regards, > Andy Shevchenko > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel