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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7C39EC433EF for ; Tue, 5 Jul 2022 15:38:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232050AbiGEPi1 (ORCPT ); Tue, 5 Jul 2022 11:38:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35376 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231171AbiGEPiY (ORCPT ); Tue, 5 Jul 2022 11:38:24 -0400 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5EBD0296 for ; Tue, 5 Jul 2022 08:38:23 -0700 (PDT) Received: from fraeml739-chm.china.huawei.com (unknown [172.18.147.226]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4Lcmy56GL3z67Pf9; Tue, 5 Jul 2022 23:37:17 +0800 (CST) Received: from lhreml724-chm.china.huawei.com (10.201.108.75) by fraeml739-chm.china.huawei.com (10.206.15.220) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Tue, 5 Jul 2022 17:38:20 +0200 Received: from [10.126.171.232] (10.126.171.232) by lhreml724-chm.china.huawei.com (10.201.108.75) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2375.24; Tue, 5 Jul 2022 16:38:20 +0100 Message-ID: <08a2a92d-caac-2472-05e0-e6749bcdd15b@huawei.com> Date: Tue, 5 Jul 2022 16:38:17 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.6.1 Subject: Re: [PATCH v1 1/4] bus: hisi_lpc: Don't dereference fwnode handle To: Andy Shevchenko CC: Andy Shevchenko , "Rafael J. Wysocki" , Linux Kernel Mailing List References: <20220705114312.86164-1-andriy.shevchenko@linux.intel.com> From: John Garry In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.126.171.232] X-ClientProxiedBy: lhreml703-chm.china.huawei.com (10.201.108.52) To lhreml724-chm.china.huawei.com (10.201.108.75) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 05/07/2022 16:23, Andy Shevchenko wrote: > On Tue, Jul 5, 2022 at 5:11 PM John Garry wrote: >> On 05/07/2022 12:43, Andy Shevchenko wrote: >>> Use dev_fwnode() and acpi_fwnode_handle() instead of dereferencing >>> an fwnode handle directly. >> ...which is a better coding practice, right? If so, it would be nice to >> mention it - well at least I think so. > Not only. In the case of fwnode it's a long story behind its corner > case(s) where in the future we might switch from embedded structure to > linked list, for example, in order to address those corner cases. > Should I write a paragraph for that as well? > If you just say that it's a better coding practice to use available APIs to access structure members rather than access them directly, then that is good enough. Or maybe you can think of something better along the lines of what you wrote above. My issue is that there was no reason. So I'll leave it to you. > ... > >> Apart from above and nit, below: > See below my answer. > >> Acked-by: John Garry > Thanks. > > ... > >>> - sys_port = logic_pio_trans_hwaddr(&host->fwnode, res->start, len); >>> + sys_port = logic_pio_trans_hwaddr(acpi_fwnode_handle(host), res->start, len); >> nit: currently the driver keeps to the old 80 character line limit. >> While the rules may have been relaxed, I'd rather we still maintain it. > First of all, even before the 100 characters era the rule had two exceptions: > 1) the string literals; Sure > 2) the readability over strictness of the 80 characters rule. > > While I agree in general with you, in this case I think keeping > strictness makes readability worse. ok, fine. I was going to suggest introduce a varible to hold acpi_fwnode_handle(host) but then we may not want a variable of fwnode_handle* type hanging around. Anyway I don't feel too strongly about it. Thanks, John