linux-kernel - Re: [RFC PATCH v2 1/3] regulator: core: Add regulator_lookup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4ac0acb9-83ea-7fcd-cde3-669ba3b699c6@gmail.com>
Date:   Wed, 25 Aug 2021 15:12:25 +0100
From:   Daniel Scally <djrscally@...il.com>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Mark Brown <broonie@...nel.org>
Cc:     Andy Shevchenko <andy.shevchenko@...il.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Platform Driver <platform-driver-x86@...r.kernel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Mark Gross <mgross@...ux.intel.com>,
        Maximilian Luz <luzmaximilian@...il.com>,
        Kieran Bingham <kieran.bingham@...asonboard.com>,
        sailues@...dragon.ideasonboard.com
Subject: Re: [RFC PATCH v2 1/3] regulator: core: Add regulator_lookup_list

Hi Laurent

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 <broonie@...nel.org> 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.


[1]
https://elixir.bootlin.com/linux/latest/source/drivers/regulator/tps65023-regulator.c#L268