Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752748AbdLLXDC (ORCPT ); Tue, 12 Dec 2017 18:03:02 -0500 Received: from mail-oi0-f65.google.com ([209.85.218.65]:40730 "EHLO mail-oi0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752679AbdLLXC7 (ORCPT ); Tue, 12 Dec 2017 18:02:59 -0500 X-Google-Smtp-Source: ACJfBotEoALZ28dm9WgkLTnsRiDYw9LepfDqRWwlYoGtuvvmyfCz16RNwQktGseNBwajSGJzPZhzQ2i9o0viWcSh8fM= MIME-Version: 1.0 In-Reply-To: <9bb1c58a-8d48-9952-e292-60b2bcb87a51@arm.com> References: <20171201222330.18863-1-jeremy.linton@arm.com> <20171201222330.18863-5-jeremy.linton@arm.com> <2078459.JrYtfXc8fv@aspire.rjw.lan> <5024a041-2ef4-3912-994f-b5fcc945e916@arm.com> <9bb1c58a-8d48-9952-e292-60b2bcb87a51@arm.com> From: "Rafael J. Wysocki" Date: Wed, 13 Dec 2017 00:02:58 +0100 X-Google-Sender-Auth: uAt572n-qL0bkWbH8wr_W4irxvc Message-ID: Subject: Re: [PATCH v5 4/9] drivers: base cacheinfo: Add support for ACPI based firmware tables To: Jeremy Linton Cc: "Rafael J. Wysocki" , "Rafael J. Wysocki" , ACPI Devel Maling List , "linux-arm-kernel@lists.infradead.org" , Sudeep Holla , Hanjun Guo , Lorenzo Pieralisi , Will Deacon , Catalin Marinas , Greg Kroah-Hartman , Viresh Kumar , Mark Rutland , Linux Kernel Mailing List , Linux PM , jhugo@codeaurora.org, wangxiongfeng2@huawei.com, Jonathan.Zhang@cavium.com, Al Stone , Jayachandran.Nair@cavium.com, austinwc@codeaurora.org, Len Brown Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1623 Lines: 55 On Tue, Dec 12, 2017 at 11:55 PM, Jeremy Linton wrote: > Hi, > > > On 12/12/2017 11:25 AM, Rafael J. Wysocki wrote: >> [cut] >>>> >>>> >>>> >>>> What about converting this to using struct fwnode instead of adding >>>> fields to it? >>> >>> >>> >>> I didn't really want to add another field here, but I've also pointed out >>> how I thought converting it to a fwnode wasn't a good choice. >>> >>> https://lkml.org/lkml/2017/11/20/502 >>> >>> Mostly because IMHO its even more misleading (lacking any >>> fwnode_operations) >>> than misusing the of_node as a void *. >> >> >> I'm not sure what you mean. > > > Converting the DT drivers/cacheinfo.c code to use a fwnode_handle is > straightforward. But IMHO it doesn't solve the readability problem of either > casting the ACPI/PPTT token directly to the resulting fwnode_handle *, or > alternatively an actual fwnode_handle with bogus fwnode_operations to wrap > that token. I'm not talking about that at all. >> >> Anyway, the idea is to have one pointer in there instead of two that >> cannot be used at the same time and there's no reason why of_node >> should be special. > > > Avoid two pointers for size, or readability? Because the last > version had a union with of_node, which isn't strictly necessary as I can > just cast the pptt token to of_node. There is exactly one line of code after > that which uses the token and it doesn't care about type. So call this field "token" or similar. Don't call it "of_node" and don't introduce another "firmware_node" thing in addition to that. That just is a mess, sorry. Thanks, Rafael