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=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_IN_DEF_DKIM_WL 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 3773FC433E6 for ; Fri, 22 Jan 2021 21:05:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0393B23B45 for ; Fri, 22 Jan 2021 21:05:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729750AbhAVVFe (ORCPT ); Fri, 22 Jan 2021 16:05:34 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51582 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730961AbhAVU7l (ORCPT ); Fri, 22 Jan 2021 15:59:41 -0500 Received: from mail-yb1-xb2e.google.com (mail-yb1-xb2e.google.com [IPv6:2607:f8b0:4864:20::b2e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 73D37C061786 for ; Fri, 22 Jan 2021 12:59:01 -0800 (PST) Received: by mail-yb1-xb2e.google.com with SMTP id p185so6737605ybg.8 for ; Fri, 22 Jan 2021 12:59:01 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=tFBOhDhZE+sffz0l6h3gmZcjhk5QjifXRo2nY423Z8U=; b=EltnYhbWOukF5okpdXiW9OaK/D6IEMnbuoQC70nY4erIGhZ1kOk/vJ9OLSs8Kgwsud /DhiwEtqJy12ahGMvuwb+wuqRQNgbnmHhSjEUe9vXI/YTfajEz8EdAhHg+6J5TXycfvZ TG3X/m4WjMQ7ncBnEPPVSJD9zhH23Yg9le/dNQ9SOzLDk+jMT5qCZA0AUoLZ63uOZy0C 84ZCmYx7FRbx5aZeFEHes29O+KC7v7piqEnqKgyXCnPUNkul58/qakvGk0hffEyyo98d 7mFdEsVFFgfsJxZfcxFJtZVoTwy0nt+71QPASazRu3magI4PS6kM1f4YrBb0ure9a2Zx a1lA== 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=tFBOhDhZE+sffz0l6h3gmZcjhk5QjifXRo2nY423Z8U=; b=oEPudsODsFdw/2cVCGfdyBX4O9sAi5AL96HGNictD8kJmpAidTqIde9hJnDWSaONA8 YApuNYgHlAJch0WEvDZacEX8Gspx5sRNfCl4uG56x8eY06FA8vcDGDn192k3KAKAfTxP VYZLEBKgv3ypckWSQsla7iJ0LhuUkAx5f18PXihG87MQrt42lJ7gBLbyTEzi/Vv0bJJ8 wQI0eUewk5YcnhmihAA0q+IJQZWtPhA54PTmagoBnZhg/BPk/yfbaXPcCCiLD642j4ZK pK4gmei20qyYfcx5TSKTWW+HCZ58gu7rONDwtTSF6KGA9XmuPHK1FkjC9TCcPCdy1FOp 3HRg== X-Gm-Message-State: AOAM533hzk3FCVSNUV0ypL0pJSaIF62FvdSYGeMNTPspqP3QTmtI2X3c 2KR6nsp5QwrJPLDkRxJGOv7RjP2ZUzvLw5WOVPIP1w== X-Google-Smtp-Source: ABdhPJxz6BmL17ilwDtkmTNCHLH83w6r5IBrNYbPHVFz4VMv0KgX+SH2FrNiRHxMyDNAIkSS99ID76Oyojyirckz9B0= X-Received: by 2002:a25:3345:: with SMTP id z66mr9201005ybz.466.1611349140491; Fri, 22 Jan 2021 12:59:00 -0800 (PST) MIME-Version: 1.0 References: <20210112134054.342-1-calvin.johnson@oss.nxp.com> <20210112134054.342-10-calvin.johnson@oss.nxp.com> <20210112180343.GI4077@smile.fi.intel.com> In-Reply-To: From: Saravana Kannan Date: Fri, 22 Jan 2021 12:58:24 -0800 Message-ID: Subject: Re: [net-next PATCH v3 09/15] device property: Introduce fwnode_get_id() To: "Rafael J. Wysocki" Cc: Andy Shevchenko , Calvin Johnson , Grant Likely , Jeremy Linton , Andrew Lunn , Florian Fainelli , Russell King - ARM Linux admin , Cristi Sovaiala , Florin Laurentiu Chiculita , Ioana Ciornei , Madalin Bucur , Heikki Krogerus , Marcin Wojtas , Pieter Jansen Van Vuuren , Jon , Diana Madalina Craciun , LKML , netdev , Laurentiu Tudor , ACPI Devel Maling List , "linux.cj" , linux-arm-kernel , Bartosz Golaszewski , Greg Kroah-Hartman , Laurent Pinchart , Randy Dunlap Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jan 22, 2021 at 8:34 AM Rafael J. Wysocki wrote: > > On Wed, Jan 20, 2021 at 9:01 PM Saravana Kannan wrote: > > > > On Wed, Jan 20, 2021 at 11:15 AM Rafael J. Wysocki wrote: > > > > > > On Wed, Jan 20, 2021 at 7:51 PM Andy Shevchenko > > > wrote: > > > > > > > > On Wed, Jan 20, 2021 at 8:18 PM Rafael J. Wysocki wrote: > > > > > On Tue, Jan 12, 2021 at 7:02 PM Andy Shevchenko > > > > > wrote: > > > > > > On Tue, Jan 12, 2021 at 09:30:31AM -0800, Saravana Kannan wrote: > > > > > > > On Tue, Jan 12, 2021 at 5:42 AM Calvin Johnson > > > > > > > wrote: > > > > > > > > ... > > > > > > > > > > > > + ret = fwnode_property_read_u32(fwnode, "reg", id); > > > > > > > > + if (!(ret && is_acpi_node(fwnode))) > > > > > > > > + return ret; > > > > > > > > + > > > > > > > > +#ifdef CONFIG_ACPI > > > > > > > > + status = acpi_evaluate_integer(ACPI_HANDLE_FWNODE(fwnode), > > > > > > > > + METHOD_NAME__ADR, NULL, &adr); > > > > > > > > + if (ACPI_FAILURE(status)) > > > > > > > > + return -EINVAL; > > > > > > > > + *id = (u32)adr; > > > > > > > > +#endif > > > > > > > > + return 0; > > > > > > > > > > > Also ACPI and DT > > > > > > > aren't mutually exclusive if I'm not mistaken. > > > > > > > > > > > > That's why we try 'reg' property for both cases first. > > > > > > > > > > > > is_acpi_fwnode() conditional is that what I don't like though. > > > > > > > > > > I'm not sure what you mean here, care to elaborate? > > > > > > > > I meant is_acpi_node(fwnode) in the conditional. > > > > > > > > I think it's redundant and we can simple do something like this: > > > > > > > > if (ret) { > > > > #ifdef ACPI > > > > ... > > > > #else > > > > return ret; > > > > #endif > > > > } > > > > return 0; > > > > > > > > -- > > > > > > Right, that should work. And I'd prefer it too. > > > > Rafael, > > > > I'd rather this new function be an ops instead of a bunch of #ifdef or > > if (acpi) checks. Thoughts? > > Well, it looks more like a helper function than like an op and I'm not > even sure how many potential users of it will expect that _ADR should > be evaluated in the absence of the "reg" property. > > It's just that the "reg" property happens to be kind of an _ADR > equivalent in this particular binding AFAICS. I agree it is not clear how useful this helper function is going to be. But in general, to me, any time the wrapper/helper functions in drivers/base/property.c need to do something like this: if (ACPI) ACPI specific code if (OF) OF specific code I think the code should be pushed to the fwnode ops. That's one of the main point of fwnode. So that firmware specific stuff is done by firmware specific code. Also, when adding support for new firmware, it's pretty clear what support the firmware needs to implement. Instead of having to go fix up a bunch of code all over the place. So fwnode_ops->get_id() would be the OP ACPI and OF would implement. And then we can have a wrapper in drivers/base/property.c. -Saravana