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: <cc65098e-b459-b20a-f6e2-ee521fc20ca7@redhat.com>
Date:   Wed, 25 Aug 2021 16:48:15 +0200
From:   Hans de Goede <hdegoede@...hat.com>
To:     Mark Brown <broonie@...nel.org>,
        Daniel Scally <djrscally@...il.com>
Cc:     linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Gross <mgross@...ux.intel.com>,
        Maximilian Luz <luzmaximilian@...il.com>,
        Kieran Bingham <kieran.bingham@...asonboard.com>,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: Re: [RFC PATCH v2 1/3] regulator: core: Add regulator_lookup_list

Hi all,

On 8/25/21 12:33 PM, Mark Brown wrote:
> On Wed, Aug 25, 2021 at 12:06:18AM +0100, Daniel Scally wrote:
>> In some situations regulator devices can be enumerated via either
>> devicetree or ACPI and bound to regulator drivers but without any
>> init data being provided in firmware. This leaves their consumers
>> unable to acquire them via regulator_get().
>>
>> To fix the issue, add the ability to register a lookup table to a
>> list within regulator core, which will allow board files to provide
>> init data via matching against the regulator name and device name in
>> the same fashion as the gpiod lookup table.
> 
> This is the wrong level to do this I think, this is a generic problem
> that affects all kinds of platform data so if we're not going to scatter
> DMI quirks throughout the drivers like we currently do then we should
> have a way for boards to just store generic platform data for a device
> and then have that platform data joined up with the device later.  This
> could for example also be used by all the laptop audio subsystems which
> need DMI quirk tables in drivers for their components to figure out how
> they're wired up and avoids the need to go through subsystems adding new
> APIs.

Daniel, I believe that what Mark wants here is something similar to what
we already do for the 5v boost converter regulator in the TI bq24190 charger
chip used on some Cherry Trail devices.

There the entire i2c-client is instantiated by platform code here:
drivers/i2c/busses/i2c-cht-wc.c

This attaches a struct bq24190_platform_data as platform data to
the i2c-client, this struct contains a single 

const struct regulator_init_data *regulator_init_data

member which then gets consumed (if there is platform data set) by
the regulator code in:

drivers/power/supply/bq24190_charger.c

For the tps68470 regulator code the platform_data then would need to
have 3 const struct regulator_init_data * members one for each of the
3 regulators.

This platform_data could then be directly set (based on a DMI match table)
from intel_skl_int3472_tps68470.c avoiding probe-ordering issues (1) with
the lookup solution and will allow the code containing the DMI and
regulator_init_data tables to be build as a module.

So all in all I think that this will be a better solution.

Regards,

Hans


1) You are forcing the DMI matching driver adding the lookups to be builtin
but what if the tps68740-MFD + regulatorcode is also builtin, then there
still is no guarantee the lookups will be added before the regulator drivers'
probe function runs


p.s.

I see that you mention something similar later in the thread referring to
the tps65023-regulator driver. I did not check, but assuming that uses what
I describe above; then yes the idea would be to do something similar for
the tps68740-code, setting the platform_data when (just before) the MFD-cells
are instantiated from intel_skl_int3472_tps68470.c