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=-5.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS 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 EEA35C4338F for ; Wed, 25 Aug 2021 14:23:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D280F610CD for ; Wed, 25 Aug 2021 14:23:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241318AbhHYOXt (ORCPT ); Wed, 25 Aug 2021 10:23:49 -0400 Received: from perceval.ideasonboard.com ([213.167.242.64]:41392 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241314AbhHYOXs (ORCPT ); Wed, 25 Aug 2021 10:23:48 -0400 Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 30F2D24F; Wed, 25 Aug 2021 16:23:01 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1629901381; bh=PSIK20B8tbTzNH+tojgaixK9PnBAHo/jHaLwhljixbI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=DFFuvJcdFUK6QQFKrRsH3wXTFOa/8UzMXOMwKhiCMwchqZBufCp5HGE5KY7LoJk+o 7LPGWzLYmqYVK+rf88RmSoKZcQ+5dV6dXCxd4hJ+Ab9Q43GOGoQ3OxDmI+KCmo9+Ww CLGc7/CHg0kapQRM5sjZoFhoDKQASrDahBOqgH0g= Date: Wed, 25 Aug 2021 17:22:49 +0300 From: Laurent Pinchart To: Daniel Scally Cc: Mark Brown , Andy Shevchenko , Linux Kernel Mailing List , Platform Driver , Liam Girdwood , Hans de Goede , Mark Gross , Maximilian Luz , Kieran Bingham , Sakari Ailus Subject: Re: [RFC PATCH v2 1/3] regulator: core: Add regulator_lookup_list Message-ID: References: <20210824230620.1003828-1-djrscally@gmail.com> <20210824230620.1003828-2-djrscally@gmail.com> <20210825103301.GC5186@sirena.org.uk> <20210825113013.GD5186@sirena.org.uk> <20210825131139.GG5186@sirena.org.uk> <4ac0acb9-83ea-7fcd-cde3-669ba3b699c6@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <4ac0acb9-83ea-7fcd-cde3-669ba3b699c6@gmail.com> Precedence: bulk List-ID: X-Mailing-List: platform-driver-x86@vger.kernel.org Hi Dan, On Wed, Aug 25, 2021 at 03:12:25PM +0100, Daniel Scally wrote: > On 25/08/2021 14:59, Laurent Pinchart wrote: > > Hello, > > > > CC'ing Sakari. > > > > On Wed, Aug 25, 2021 at 02:11:39PM +0100, Mark Brown wrote: > >> On Wed, Aug 25, 2021 at 03:26:37PM +0300, Andy Shevchenko wrote: > >>> On Wed, Aug 25, 2021 at 2:30 PM Mark Brown wrote: > >>>> No, what was proposed for regulator was to duplicate all the the DT > >>>> binding code in the regulator framework so it parses fwnodes then have > >>>> an API for encoding fwnodes from C data structures at runtime. The bit > >>>> where the data gets joined up with the devices isn't the problem, it's > >>>> the duplication and fragility introduced by encoding everything into > >>>> an intermediate representation that has no purpose and passing that > >>>> around which is the problem. > >>> The whole exercise with swnode is to minimize the driver intrusion and > >>> evolving a unified way for (some) of the device properties. V4L2 won't > >> The practical implementation for regulators was to duplicate a > >> substantial amount of code in the core in order to give us a less type > >> safe and more indirect way of passing data from onen C file in the > >> kernel to another. This proposal is a lot better in that it uses the > >> existing init_data and avoids the huge amounts of duplication, it's just > >> not clear from the changelog why it's doing this in a regulator specific > >> manner. > >> > >> *Please* stop trying to force swnodes in everywhere, take on board the > >> feedback about why the swnode implementation is completely inappropriate > >> for regulators. I don't understand why you continue to push this so > >> hard. swnodes and fwnodes are a solution to a specific problem, they're > >> not the answer to every problem out there and having to rehash this > >> continually is getting in the way of actually discussing practical > >> workarounds for these poorly implemented ACPI platforms. > >> > >>> like what you are suggesting exactly because they don't like the idea > >>> of spreading the board code over the drivers. In some cases it might > >>> even be not so straightforward and easy. > >>> Laurent, do I understand correctly the v4l2 expectations? > >> There will be some cases where swnodes make sense, for example where the > >> data is going to be read through the fwnode API since the binding is > >> firmware neutral which I think is the v4l case. On the other hand > >> having a direct C representation is a very common way of implementing > >> DMI quirk tables, and we have things like the regulator API where > >> there's off the shelf platform data support and we actively don't want > >> to support fwnode. > > From a camera sensor point of view, we want to avoid code duplication. > > Having to look for regulators using OF lookups *and* platform data in > > every single sensor driver is not a good solution. This means that, from > > a camera sensor driver point of view, we want to call regulator_get() > > (or the devm_ version) with a name, without caring about who establishes > > the mapping and how the lookup is performed. I don't care much > > personally if this would be implemented through swnode or a different > > mechanism, as long as the implementation can be centralized. > > I think rather than sensor drivers, the idea would be just to have the > tps68470-regulator driver check platform data for the init_data instead, > like the tps65023-regulator driver [1] does. I'm sure that'll work, but > it's not particularly centralized from the regulator driver's point of > view. That would obviously be less of an issue from a V4L2 point of view :-) Given that the situation we're facing doesn't affect (to our knowledge) a very large number of regulators, it may not be too bad in practice. If I were to maintain the regulator subsystem I'd probably want a centralized implementation there, but that's certainly a personal preference, at least partly. On a side note, this RFC looks quite similar to what the GPIO subsystem does, which I personally consider nice as differences between regulator and GPIO in these areas are confusing for users. > [1] > https://elixir.bootlin.com/linux/latest/source/drivers/regulator/tps65023-regulator.c#L268 -- Regards, Laurent Pinchart