lkml.org 
[lkml]   [2021]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH 0/2] Add software node support to regulator framework
    From
    Date
    Hi Laurent

    On 10/07/2021 23:28, Laurent Pinchart wrote:
    > Hi Dan,
    >
    > On Thu, Jul 08, 2021 at 11:42:24PM +0100, Daniel Scally wrote:
    >> Hello all
    >>
    >> See previous series for some background context [1]
    >>
    >> Some x86 laptops with ACPI tables designed for Windows have a TPS68470
    >> PMIC providing regulators and clocks to camera modules. The DSDT tables for
    >> those cameras lack any power control methods, declaring only a
    >> dependency on the ACPI device representing the TPS68470. This leaves the
    >> regulator framework with no means of determining appropriate voltages for the
    >> regulators provided by the PMIC, or of determining which regulators relate to
    >> which of the sensor's requested supplies.
    >>
    >> This series is a prototype of an emulation of the device tree regulator
    >> initialisation and lookup functions, using software nodes. Software nodes
    >> relating to each regulator are registered as children of the TPS68470's ACPI
    >> firmware node. Those regulators have properties describing their constraints
    >> (for example "regulator-min-microvolt"). Similarly, software nodes are
    >> registered and assigned as secondary to the Camera's firmware node - these
    >> software nodes have reference properties named after the supply in the same
    >> way as device tree's phandles, for example "avdd-supply", and linking to the
    >> software node assigned to the appropriate regulator. We can then use those
    >> constraints to specify the appropriate voltages and the references to allow the
    >> camera drivers to look up the correct regulator device.
    >>
    >> Although not included in this series, I would plan to use a similar method for
    >> linking the clocks provided by the TPS68470 to the sensor so that it can be
    >> discovered too.
    >>
    >> I'm posting this to see if people agree it's a good approach for tackling the
    >> problem; I may be overthinking this and there's a much easier way that I should
    >> be looking at instead. It will have knock-ons in the cio2-bridge code [2], as
    >> that is adding software nodes to the same sensors to connect them to the media
    >> graph. Similarly, is the board file an acceptable solution, or should we just
    >> define the configuration for these devices (there's three orf our laptop models
    >> in scope) in int3472-tps68470 instead?
    > I may have missed something, but if you load the SGo2 board file, won't
    > it create the regulator software nodes if it finds an INT3472,
    > regardless of whether the device is an SGo2 ? If you happen to do so on
    > a machine that requires different voltages, that sounds dangerous.


    Ah, yes - hadn't thought of that. If a driver registered regulators with
    those names, it would try to apply those voltages during registration.
    Good point.

    > Given that INT3472 models the virtual "Intel Skylake and Kabylake camera
    > PMIC", I think moving device-specific information to the int3472 driver
    > may make sense. I'm unsure what option is best though, having all the
    > data (regulators, clocks, but also data currently stored in the
    > cio2-bridge driver) in a single file (or a single file per machine) is
    > tempting.


    It is tempting, particularly because (assuming we do end up using this
    approach) setting the references to the supplies in a board file like
    this complicated the cio2-bridge code quite a bit, since it then needs
    to extend the properties array against an already-existing software node
    rather than registering a new one. But then, I don't particularly want
    to handle that aspect of the problem in two separate places.

    >> [1] https://lore.kernel.org/lkml/20210603224007.120560-1-djrscally@gmail.com/
    >> [2] https://elixir.bootlin.com/linux/latest/source/drivers/media/pci/intel/ipu3/cio2-bridge.c#L166
    >>
    >>
    >> Daniel Scally (2):
    >> regulator: Add support for software node connections
    >> platform/surface: Add Surface Go 2 board file
    >>
    >> MAINTAINERS | 6 +
    >> drivers/platform/surface/Kconfig | 10 ++
    >> drivers/platform/surface/Makefile | 1 +
    >> drivers/platform/surface/surface_go_2.c | 135 +++++++++++++++++++++
    >> drivers/regulator/Kconfig | 6 +
    >> drivers/regulator/Makefile | 1 +
    >> drivers/regulator/core.c | 23 ++++
    >> drivers/regulator/swnode_regulator.c | 111 +++++++++++++++++
    >> include/linux/regulator/swnode_regulator.h | 33 +++++
    >> 9 files changed, 326 insertions(+)
    >> create mode 100644 drivers/platform/surface/surface_go_2.c
    >> create mode 100644 drivers/regulator/swnode_regulator.c
    >> create mode 100644 include/linux/regulator/swnode_regulator.h

    \
     
     \ /
      Last update: 2021-07-11 00:55    [W:3.062 / U:0.216 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site