This reverts commit cd7a38c40b.
When submitting the change above, it was thought that the origin of the
init_data should be a clear choice, from the driver or from DT but not
both.
It turns out some devices, such as qcom-msm8974-lge-nexus5-hammerhead,
relied on the old behaviour to override the init_data provided by the
driver, making it some kind of default if none is provided by the platform.
Using the init_data provided by the driver when it is present broke these
devices so revert the change to fixup the situation and add a comment
to make things a bit more clear
Reported-by: Luca Weiss <luca@lucaweiss.eu>
Closes: https://lore.kernel.org/lkml/5857103.DvuYhMxLoT@lucaweiss.eu
Fixes: cd7a38c40b ("regulator: core: do not silently ignore provided init_data")
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Link: https://patch.msgid.link/20250211-regulator-init-data-fixup-v1-1-5ce1c6cff990@baylibre.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Introduce power budget management for the regulator device. Enable tracking
of available power capacity by providing helpers to request and release
power budget allocations.
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
Link: https://patch.msgid.link/20250115-feature_regulator_pw_budget-v2-1-0a44b949e6bc@bootlin.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Previously, the regulator core resolved its supply only from the parent
device or its children, ignoring the of_node specified in the
regulator_config structure.
This behavior causes issues in scenarios where multiple regulator devices
are registered for components described as children of a controller, each
with their own specific regulator supply.
For instance, in a PSE controller with multiple PIs (Power Interfaces),
each PI may have a distinct regulator supply. However, the regulator core
would incorrectly use the PSE controller node or its first child to look up
the regulator supply, rather than the node specified by the
regulator_config->of_node for the PI.
This update modifies the behavior to prioritize the of_node in
regulator_config for resolving the supply. This ensures correct resolution
of the power supply for each device. If no supply is found in the provided
of_node, the core falls back to searching within the parent device as
before.
Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de>
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
Link: https://patch.msgid.link/20250109-b4-feature_poe_arrange-v2-13-55ded947b510@bootlin.com
Signed-off-by: Mark Brown <broonie@kernel.org>
We should only consider max_uA constraints if they are explicitly defined.
In cases where it is not set, we should assume the regulator has no current
limit.
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
Link: https://patch.msgid.link/20241121-feature_poe_port_prio-v3-2-83299fa6967c@bootlin.com
Signed-off-by: Mark Brown <broonie@kernel.org>
_regulator_delay_helper() implements the recommondation of the outdated
documentation which sleep mechanism should be used. There is already a
function in place which does everything and also maps to reality called
fsleep().
Use fsleep() directly.
Cc: Liam Girdwood <lgirdwood@gmail.com>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de>
Reviewed-by: Frederic Weisbecker <frederic@kernel.org>
Link: https://patch.msgid.link/20241014-devel-anna-maria-b4-timers-flseep-v3-11-dc8b907cb62f@linutronix.de
Signed-off-by: Mark Brown <broonie@kernel.org>
The machine specific regulator_init() appears to be unused.
It does not allow a lot of interaction with the regulator framework,
since nothing from the framework is passed along (desc, config,
etc ...)
Machine specific init may also be done with the added init_cb() in
the regulator description, so remove regulator_init().
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Link: https://patch.msgid.link/20241008-regulator-ignored-data-v2-3-d1251e0ee507@baylibre.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Provide an initialisation callback to handle runtime parameters.
The idea is similar to the regulator_init() callback, but it provides
regulator specific structures, instead of just the driver specific data.
As an example, this allows the driver to amend the regulator constraints
based on runtime parameters if necessary.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Link: https://patch.msgid.link/20241008-regulator-ignored-data-v2-2-d1251e0ee507@baylibre.com
Signed-off-by: Mark Brown <broonie@kernel.org>
On DT platforms, if a regulator init_data is provided in config, it is
silently ignored in favor of the DT parsing done by the framework, if
of_match is set.
of_match is an indication that init_data is expected to be set based on DT
and the parsing should be done by the regulator framework.
If the regulator provider passed init_data it must be because it is useful
somehow, in such case of_match should be clear.
If the driver expects the framework to initialize this data on its
own, it should leave init_data clear.
Warn if both init_data and of_match are set, then default to the provided
init_data.
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Link: https://patch.msgid.link/20241008-regulator-ignored-data-v2-1-d1251e0ee507@baylibre.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The to-be-introduced I2C component prober needs to enable regulator
supplies (and toggle GPIO pins) for the various components it intends
to probe. To support this, a new "pure DT lookup" method for getting
regulator supplies is needed, since the device normally requesting
the supply won't get created until after the component is probed to
be available.
Add a new of_regulator_get_optional() function for this. This mirrors
the existing regulator_get_optional() function, but is OF-specific.
The underlying code that supports the existing regulator_get*()
functions has been reworked in previous patches to support this
specific case.
Also convert an existing usage of "dev && dev->of_node" to
"dev_of_node(dev)".
Link: https://lore.kernel.org/all/20231220203537.83479-2-jernej.skrabec@gmail.com/ [1]
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Link: https://patch.msgid.link/20240925093807.1026949-2-wenst@chromium.org
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
_regulator_get() contains a lot of common code doing checks prior to the
regulator lookup and housekeeping work after the lookup. Almost all the
code could be shared with a OF-specific variant of _regulator_get().
Split out the common parts so that they can be reused. The OF-specific
version of _regulator_get() will be added in a subsequent patch.
No functional changes were made.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://patch.msgid.link/20240911072751.365361-4-wenst@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
The behavior of regulator_dev_lookup() for non-DT way has been broken
since the commit b8c3255457 ("regulator: Move OF-specific regulator
lookup code to of_regulator.c").
Before the commit, of_get_regulator() was used to get the regulator,
which returns NULL if the regulator is not found. So the regulator
will be looked up through regulator_lookup_by_name() if no matching
regulator is found in regulator_map_list.
However, currently, of_regulator_dev_lookup() is used to instead of
of_get_regulator(), but the variable 'r' is set to ERR_PTR(-ENODEV)
instead of NULL if the regulator is not found. In this case, if no
regulator is found in regulator_map_list, the variable 'r' is still
ERR_PTR(-ENODEV), So regulator_dev_lookup() returns the value of 'r'
directly instead of continuing to look up the regulator through
regulator_lookup_by_name().
Fixes: b8c3255457 ("regulator: Move OF-specific regulator lookup code to of_regulator.c")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
Link: https://patch.msgid.link/20240911120338.526384-1-wei.fang@nxp.com
Signed-off-by: Mark Brown <broonie@kernel.org>
There's still a bit of OF-specific code in the regulator device lookup
function.
Move those bits of code over to of_regulator.c, and create a new
function of_regulator_dev_lookup() to encapsulate the code moved out of
regulator_dev_lookup().
Also mark of_find_regulator_by_node() as static, since there are no
other users in other compile units.
There are no functional changes. A line alignment was also fixed.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Link: https://patch.msgid.link/20240904090016.2841572-4-wenst@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
Previous commits cleaning up kerneldoc used the term "negative error
number" to refer to error condition return values. Update remaining
instances of other terminology such as "error code" or "errno" as
well so the whole regulator subsystem is unified.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://patch.msgid.link/20240829085131.1361701-11-wenst@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
kernel-doc complains about missing "Return" section for many documented
functions in the regulator core. Some with free-form return value
descriptions have been fixed in the previous patch. The remaining are
completely missing any mention of return values.
Add "Return" sections to these kerneldoc blocks with basic descriptions.
In a few cases where the functions don't call even more functions and
the error numbers are known, those are documented in detail.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://patch.msgid.link/20240829085131.1361701-5-wenst@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
kernel-doc complains about missing "Return" section for many documented
functions in the regulator core. Many of them actually have descriptions
about the return values, just not in the format kernel-doc wants.
Convert these to use the proper "Return:" section header. The existing
descriptions have been reworded and moved around to fit the grammar and
formatting.
In a few cases where the functions don't call even more functions
and the error numbers are known, those are documented in detail.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://patch.msgid.link/20240829085131.1361701-4-wenst@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
The kerneldoc for regulator_is_supported_voltage() states that the
return value is a boolean. That is not correct, as it could return an
error number if the check failed.
Fix the description by expanding it to cover the valid return values and
error conditions. The description is also converted to a proper "Return"
section.
Fixes: c5f3939b8f ("regulator: core: Support fixed voltages in regulator_is_supported_voltage()")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://patch.msgid.link/20240829085131.1361701-3-wenst@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
kernel-doc complains that _regulator_check_status_enabled() is missing a
short description.
Since the current description is already quite short, just trim it a bit
more and use it as the short description.
Fixes: f7d7ad42a9 ("regulator: Allow regulators to verify enabled during enable()")
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Reported-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://patch.msgid.link/20240829085131.1361701-2-wenst@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
The original error message simply said "get() with no identifier"
without any context as to what was requested or what device the
request was related to. The only thing the user or developer could
do was grep for the message in the full source tree.
Amend the error message to be more specific, and also use dev_*
to associate the error message with a device.
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
Link: https://patch.msgid.link/20240822072047.3097740-2-wenst@chromium.org
Signed-off-by: Mark Brown <broonie@kernel.org>
Add a helper function that allow regulator consumers to allow low-level
HW access, in order to enable/disable regulator in atomic context.
The use-case for RZ/G2L SoC is to enable VBUS selection register based
on vbus detection that happens in interrupt context.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Link: https://patch.msgid.link/20240616105402.45211-4-biju.das.jz@bp.renesas.com
Signed-off-by: Mark Brown <broonie@kernel.org>
There's one API update here, a new API factoring out a common pattern
for reference voltage supplies. These are supplies uses as a reference
by analogue circuits where the consumer requests and enables the supply,
reads the voltage to calibrate the user and then never touches it again.
This is factored out into a single operation which just returns the
voltage and uses devm_ to manage the request and enable portion.
Otherwise this has been a very quiet release, we've got some new device
support, some small fixes, housekeeping and cleanup work but nothing
substantial.
There's also some non-regulator changes in here, a number of users for
the new reference voltage API were merged along with it and some MFD
changes were pulled in as dependencies for new driver work.
Highlights:
- Add a new API for single operation handling of reference voltages.
- Support for Allwinner AXP717 and D1, and NXP PCA9561A.
-----BEGIN PGP SIGNATURE-----
iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmZB3mMACgkQJNaLcl1U
h9DRmAf9Etp/ydeKGrbYD4K34Ptk4ZwchkTfvPEiNmJEDBlbcazRoeCF9asUVsDR
ByggQCahCWhEa+exT+IBQYswpwFRd1oZChSgGUmIS/7pvQDdaPL+53Fnw8SfzmdS
QygBEMNN8TGIu2Y+OqEEqINJ407NLbv2SbIlFX1CywZ5hDvCtvFdqhX/xCxTedBN
wXlyj2BLe/xmbTGxr3Ssb+l8a0oe1BZvLO6ddrg8DtS92l1zJ0sEk+fegHSQGPRI
wI4QehjEgphBsoGhybMGz3ny0nvSs3JkpgRiG/ly3cT97Cx6KSGctMtqjmy7Ynh5
TM3Qq5DS37pBKIm79zmQFbL1z2A9Bw==
=wV3x
-----END PGP SIGNATURE-----
Merge tag 'regulator-v6.10' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator
Pull regulator updates from Mark Brown:
"There's one API update here, a new API factoring out a common pattern
for reference voltage supplies. These are supplies used as a reference
by analogue circuits where the consumer requests and enables the
supply, reads the voltage to calibrate the user and then never touches
it again. This is factored out into a single operation which just
returns the voltage and uses devm_ to manage the request and enable
portion.
Otherwise this has been a very quiet release, we've got some new
device support, some small fixes, housekeeping and cleanup work but
nothing substantial.
There's also some non-regulator changes in here, a number of users for
the new reference voltage API were merged along with it and some MFD
changes were pulled in as dependencies for new driver work.
Highlights:
- Add a new API for single operation handling of reference voltages
- Support for Allwinner AXP717 and D1, and NXP PCA9561A"
* tag 'regulator-v6.10' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator: (33 commits)
regulator: sun20i: Add Allwinner D1 LDOs driver
regulator: dt-bindings: Add Allwinner D1 system LDOs
regulator: Mention regulator id in error message about dummy supplies
staging: iio: impedance-analyzer: ad5933: Use devm_regulator_get_enable_read_voltage()
iio: frequency: admv1013: Use devm_regulator_get_enable_read_voltage()
iio: addac: ad74115: Use devm_regulator_get_enable_read_voltage()
hwmon: (adc128d818) simplify final return in probe
regulator: devres: fix devm_regulator_get_enable_read_voltage() return
hwmon: (da9052) Use devm_regulator_get_enable_read_voltage()
hwmon: (adc128d818) Use devm_regulator_get_enable_read_voltage()
regulator: devres: add API for reference voltage supplies
regulator: rtq2208: Fix LDO discharge register and add vsel setting
regulator: dt-bindings: fixed-regulator: Add a preferred node name
regulator: axp20x: add support for the AXP717
mfd: axp20x: Add support for AXP717 PMIC
dt-bindings: mfd: x-powers,axp152: Document AXP717
regulator: axp20x: fix typo-ed identifier
dt-bindings: regulator: qcom,usb-vbus-regulator: Add PM7250B compatible
regulator: pca9450: add pca9451a support
regulator: dt-bindings: pca9450: add pca9451a support
...
regulator_get() may sometimes be called more than once for the same
consumer device, something which before commit dbe954d8f1 ("regulator:
core: Avoid debugfs: Directory ... already present! error") resulted in
errors being logged.
A couple of recent commits broke the handling of such cases so that
attributes are now erroneously created in the debugfs root directory the
second time a regulator is requested and the log is filled with errors
like:
debugfs: File 'uA_load' in directory '/' already present!
debugfs: File 'min_uV' in directory '/' already present!
debugfs: File 'max_uV' in directory '/' already present!
debugfs: File 'constraint_flags' in directory '/' already present!
on any further calls.
Fixes: 2715bb11cf ("regulator: core: Fix more error checking for debugfs_create_dir()")
Fixes: 08880713ce ("regulator: core: Streamline debugfs operations")
Cc: stable@vger.kernel.org
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Link: https://lore.kernel.org/r/20240509133304.8883-1-johan+linaro@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
Previously, performing an exclusive get on an already-enabled regulator
resulted in inconsistent state initialization between child and parent
regulators. While the child's counts were updated, its parent's counters
remained unaffected.
Consequently, attempting to disable an already-enabled exclusive regulator
triggered unbalanced disables warnings from its parent regulator.
This commit addresses the issue by propagating the enable state to the
parent regulator using a regulator_enable call. This ensures consistent
state management across the regulator hierarchy, preventing warnings!
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
Link: https://msgid.link/r/20240312091638.1266167-1-kory.maincent@bootlin.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Since commit 43a7206b09 ("driver core: class: make class_register() take
a const *"), the driver core allows for struct class to be in read-only
memory, so move the regulator_class structure to be declared at build time
placing it into read-only memory, instead of having to be dynamically
allocated at boot time.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
Link: https://msgid.link/r/20240305-class_cleanup-regulator-v1-1-4950345d6d8f@marliere.net
Signed-off-by: Mark Brown <broonie@kernel.org>
The variable possible_uV being assigned a value that is never read, the
control flow via the following goto statement takes a path where the
variable is not accessed. The assignment is redundant and can be removed.
Cleans up clang scan build warning:
drivers/regulator/core.c:3935:3: warning: Value stored to 'possible_uV'
is never read [deadcode.DeadStores]
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Link: https://msgid.link/r/20240216134918.2108262-1-colin.i.king@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
This commit introduces netlink event support to the regulator subsystem.
Changes:
- Introduce event.c and regnl.h for netlink event handling.
- Implement reg_generate_netlink_event to broadcast regulator events.
- Update Makefile to include the new event.c file.
Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com>
Link: https://lore.kernel.org/r/20231205105207.1262928-1-naresh.solanki@9elements.com
Signed-off-by: Mark Brown <broonie@kernel.org>
The use_count of a regulator should only be incremented when the
enable_count changes from 0 to 1. Similarly, the use_count should
only be decremented when the enable_count changes from 1 to 0.
In the previous implementation, use_count was sometimes decremented
to 0 when some consumer called unbalanced disable,
leading to unexpected disable even the regulator is enabled by
other consumers. With this change, the use_count accurately reflects
the number of users which the regulator is enabled.
This should make things more robust in the case where a consumer does
leak references.
Signed-off-by: Rui Zhang <zr.zhang@vivo.com>
Link: https://lore.kernel.org/r/20231103074231.8031-1-zr.zhang@vivo.com
Signed-off-by: Mark Brown <broonie@kernel.org>
This may be useful for debugging and develompent purposes, when there are
drivers that depend on regulators to be enabled but do not request them.
It is inspired from the clk_ignore_unused and pd_ignore_unused parameters,
that are used to keep firmware-enabled clocks and power domains on even if
these are not used by drivers.
The parameter is not expected to be used in normal cases and should not be
needed on a platform with proper driver support.
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Brian Masney <bmasney@redhat.com>
Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Link: https://lore.kernel.org/r/20231107190926.1185326-1-javierm@redhat.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Add 'uv_survival_time' field to regulation_constraints for specifying
survival time post critical under-voltage event. Update the regulator
notifier call chain and Device Tree property parsing to use this new
field, allowing a configurable timeout before emergency shutdown.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Link: https://lore.kernel.org/r/20231026144824.4065145-6-o.rempel@pengutronix.de
Signed-off-by: Mark Brown <broonie@kernel.org>
Handle under-voltage events for crucial regulators to maintain system
stability and avoid issues during power drops.
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Link: https://lore.kernel.org/r/20231026144824.4065145-3-o.rempel@pengutronix.de
Signed-off-by: Mark Brown <broonie@kernel.org>
When fixing a memory leak in commit d3c731564e ("regulator: plug
of_node leak in regulator_register()'s error path") it moved the
device_initialize() call earlier, but did not move the `dev->class`
initialization. The bug was spotted and fixed by reverting part of
the commit (in commit 5f4b204b6b "regulator: core: fix kobject
release warning and memory leak in regulator_register()") but
introducing a different bug: now early error paths use `kfree(dev)`
instead of `put_device()` for an already initialized `struct device`.
Move the missing assignments to just after `device_initialize()`.
Fixes: d3c731564e ("regulator: plug of_node leak in regulator_register()'s error path")
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
Link: https://lore.kernel.org/r/b5b19cb458c40c9d02f3d5a7bd1ba7d97ba17279.1695077303.git.mirq-linux@rere.qmqm.pl
Signed-off-by: Mark Brown <broonie@kernel.org>
If CONFIG_DEBUG_FS is not set:
regulator: Failed to create debugfs directory
...
regulator-dummy: Failed to create debugfs directory
As per the comments for debugfs_create_dir(), errors returned by this
function should be expected, and ignored:
* If debugfs is not enabled in the kernel, the value -%ENODEV will be
* returned.
*
* NOTE: it's expected that most callers should _ignore_ the errors returned
* by this function. Other debugfs functions handle the fact that the "dentry"
* passed to them could be an error and they don't crash in that case.
* Drivers should generally work fine even if debugfs fails to init anyway.
Adhere to the debugfs spirit, and streamline all operations by:
1. Demoting the importance of the printed error messages to debug
level, like is already done in create_regulator(),
2. Further ignoring any returned errors, as by design, all debugfs
functions are no-ops when passed an error pointer.
Fixes: 2bf1c45be3 ("regulator: Fix error checking for debugfs_create_dir")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/2f8bb6e113359ddfab7b59e4d4274bd4c06d6d0a.1685013051.git.geert+renesas@glider.be
Signed-off-by: Mark Brown <broonie@kernel.org>
In case of failure, debugfs_create_dir() does not return NULL, but an
error pointer. Most incorrect error checks were fixed, but the one in
create_regulator() was forgotten.
Fix the remaining error check.
Fixes: 2bf1c45be3 ("regulator: Fix error checking for debugfs_create_dir")
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Link: https://lore.kernel.org/r/ee980a108b5854dd8ce3630f8f673e784e057d17.1685013051.git.geert+renesas@glider.be
Signed-off-by: Mark Brown <broonie@kernel.org>
This patch fixes the error checking in core.c in debugfs_create_dir.
The correct way to check if an error occurred is 'IS_ERR' inline function.
Signed-off-by: Osama Muhammad <osmtendev@gmail.com
Suggested-by: Ivan Orlov <ivan.orlov0322@gmail.com
Link: https://lore.kernel.org/r/20230515172938.13338-1-osmtendev@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org
An automated bot told me that there was a potential lockdep problem
with regulators. This was on the chromeos-5.15 kernel, but I see
nothing that would be different downstream compared to upstream. The
bot said:
============================================
WARNING: possible recursive locking detected
5.15.104-lockdep-17461-gc1e499ed6604 #1 Not tainted
--------------------------------------------
kworker/u16:4/115 is trying to acquire lock:
ffffff8083110170 (regulator_ww_class_mutex){+.+.}-{3:3}, at: create_regulator+0x398/0x7ec
but task is already holding lock:
ffffff808378e170 (regulator_ww_class_mutex){+.+.}-{3:3}, at: ww_mutex_trylock+0x3c/0x7b8
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(regulator_ww_class_mutex);
lock(regulator_ww_class_mutex);
*** DEADLOCK ***
May be due to missing lock nesting notation
4 locks held by kworker/u16:4/115:
#0: ffffff808006a948 ((wq_completion)events_unbound){+.+.}-{0:0}, at: process_one_work+0x520/0x1348
#1: ffffffc00e0a7cc0 ((work_completion)(&entry->work)){+.+.}-{0:0}, at: process_one_work+0x55c/0x1348
#2: ffffff80828a2260 (&dev->mutex){....}-{3:3}, at: __device_attach_async_helper+0xd0/0x2a4
#3: ffffff808378e170 (regulator_ww_class_mutex){+.+.}-{3:3}, at: ww_mutex_trylock+0x3c/0x7b8
stack backtrace:
CPU: 2 PID: 115 Comm: kworker/u16:4 Not tainted 5.15.104-lockdep-17461-gc1e499ed6604 #1 9292e52fa83c0e23762b2b3aa1bacf5787a4d5da
Hardware name: Google Quackingstick (rev0+) (DT)
Workqueue: events_unbound async_run_entry_fn
Call trace:
dump_backtrace+0x0/0x4ec
show_stack+0x34/0x50
dump_stack_lvl+0xdc/0x11c
dump_stack+0x1c/0x48
__lock_acquire+0x16d4/0x6c74
lock_acquire+0x208/0x750
__mutex_lock_common+0x11c/0x11f8
ww_mutex_lock+0xc0/0x440
create_regulator+0x398/0x7ec
regulator_resolve_supply+0x654/0x7c4
regulator_register_resolve_supply+0x30/0x120
class_for_each_device+0x1b8/0x230
regulator_register+0x17a4/0x1f40
devm_regulator_register+0x60/0xd0
reg_fixed_voltage_probe+0x728/0xaec
platform_probe+0x150/0x1c8
really_probe+0x274/0xa20
__driver_probe_device+0x1dc/0x3f4
driver_probe_device+0x78/0x1c0
__device_attach_driver+0x1ac/0x2c8
bus_for_each_drv+0x11c/0x190
__device_attach_async_helper+0x1e4/0x2a4
async_run_entry_fn+0xa0/0x3ac
process_one_work+0x638/0x1348
worker_thread+0x4a8/0x9c4
kthread+0x2e4/0x3a0
ret_from_fork+0x10/0x20
The problem was first reported soon after we made many of the
regulators probe asynchronously, though nothing I've seen implies that
the problems couldn't have also happened even without that.
I haven't personally been able to reproduce the lockdep issue, but the
issue does look somewhat legitimate. Specifically, it looks like in
regulator_resolve_supply() we are holding a "rdev" lock while calling
set_supply() -> create_regulator() which grabs the lock of a
_different_ "rdev" (the one for our supply). This is not necessarily
safe from a lockdep perspective since there is no documented ordering
between these two locks.
In reality, we should always be locking a regulator before the
supplying regulator, so I don't expect there to be any real deadlocks
in practice. However, the regulator framework in general doesn't
express this to lockdep.
Let's fix the issue by simply grabbing the two locks involved in the
same way we grab multiple locks elsewhere in the regulator framework:
using the "wound/wait" mechanisms.
Fixes: eaa7995c52 ("regulator: core: avoid regulator_resolve_supply() race condition")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Link: https://lore.kernel.org/r/20230329143317.RFC.v2.2.I30d8e1ca10cfbe5403884cdd192253a2e063eb9e@changeid
Signed-off-by: Mark Brown <broonie@kernel.org>
When a codepath locks a rdev using ww_mutex_lock_slow() directly then
that codepath is responsible for incrementing the "ref_cnt" and also
setting the "mutex_owner" to "current".
The regulator core consistently got that right for "ref_cnt" but
didn't always get it right for "mutex_owner". Let's fix this.
It's unlikely that this truly matters because the "mutex_owner" is
only needed if we're going to do subsequent locking of the same
rdev. However, even though it's not truly needed it seems less
surprising if we consistently set "mutex_owner" properly.
Fixes: f8702f9e4a ("regulator: core: Use ww_mutex for regulators locking")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Link: https://lore.kernel.org/r/20230329143317.RFC.v2.1.I4e9d433ea26360c06dd1381d091c82bb1a4ce843@changeid
Signed-off-by: Mark Brown <broonie@kernel.org>
This is very close to a straight revert of commit 218320fec2
("regulator: core: Fix off-on-delay-us for always-on/boot-on
regulators"). We've identified that patch as causing a boot speed
regression on sc7180-trogdor boards. While boot speed certainly isn't
more important than making sure that power sequencing is correct,
looking closely at the original change it doesn't seem to have been
fully justified. It mentions "cycling issues" without describing
exactly what the issues were. That means it's possible that the
cycling issues were really a problem that should be fixed in a
different way.
Let's take a careful look at how we should handle regulators that have
an off-on-delay and that are boot-on or always-on. Linux currently
doesn't have any way to identify whether a GPIO regulator was already
on when the kernel booted. That means that when the kernel boots we
probe a regulator, see that it wants boot-on / always-on we, and then
turn the regulator on. We could be in one of two cases when we do
this:
a) The regulator might have been left on by the bootloader and we're
ensuring that it stays on.
b) The regulator might have been left off by the bootloader and we're
just now turning it on.
For case a) we definitely don't need any sort of delay. For case b) we
_might_ need some delay in case the bootloader turned the regulator
off _right_ before booting the kernel. To get the proper delay for
case b) then we can just assume a `last_off` of 0, which is what it
gets initialized to by default.
As per above, we can't tell whether we're in case a) or case b) so
we'll assume the longer delay (case b). This basically puts the code
to how it was before commit 218320fec2 ("regulator: core: Fix
off-on-delay-us for always-on/boot-on regulators"). However, we add
one important change: we make sure that the delay is actually honored
if `last_off` is 0. Though the original "cycling issues" cited were
vague, I'm hopeful that this important extra change will be enough to
fix the issues that the initial commit mentioned.
With this fix, I've confined that on a sc7180-trogdor board the delay
at boot goes down from 500 ms to ~250 ms. That's not as good as the 0
ms that we had prior to commit 218320fec2 ("regulator: core: Fix
off-on-delay-us for always-on/boot-on regulators"), but it's probably
safer because we don't know if the bootloader turned the regulator off
right before booting.
One note is that it's possible that we could be in a state that's not
a) or b) if there are other issues in the kernel. The only one I can
think of is related to pinctrl. If the pinctrl driver being used on a
board isn't careful about avoiding glitches when setting up a pin then
it's possible that setting up a pin could cause the regulator to "turn
off" briefly immediately before the regulator probes. If this is
indeed causing problems then the pinctrl driver should be fixed,
perhaps in a similar way to what was done in commit d21f4b7ffc
("pinctrl: qcom: Avoid glitching lines when we first mux to output")
Fixes: 218320fec2 ("regulator: core: Fix off-on-delay-us for always-on/boot-on regulators")
Cc: Christian Kohlschütter <christian@kohlschutter.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Link: https://lore.kernel.org/r/20230313111806.1.I2eaad872be0932a805c239a7c7a102233fb0b03b@changeid
Signed-off-by: Mark Brown <broonie@kernel.org>
For regulators with 'off-on-delay-us' the regulator framework currently
uses ktime_get() to determine how long the regulator has been off
before re-enabling it (after a delay if needed). A problem with using
ktime_get() is that it doesn't account for the time the system is
suspended. As a result a regulator with a longer 'off-on-delay' (e.g.
500ms) that was switched off during suspend might still incurr in a
delay on resume before it is re-enabled, even though the regulator
might have been off for hours. ktime_get_boottime() accounts for
suspend time, use it instead of ktime_get().
Fixes: a8ce7bd896 ("regulator: core: Fix off_on_delay handling")
Cc: stable@vger.kernel.org # 5.13+
Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/20230223003301.v2.1.I9719661b8eb0a73b8c416f9c26cf5bd8c0563f99@changeid
Signed-off-by: Mark Brown <broonie@kernel.org>
Two core fixes here, one for a long standing race which some Qualcomm
systems have started triggering with their UFS driver and another fixing
a problem with supply lookup introduced by the fixes for devm related
use after free issues that were introduced in this merge window.
-----BEGIN PGP SIGNATURE-----
iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmOmHd4ACgkQJNaLcl1U
h9A8hQf/fTezN966/Rp9tvP9YJOPmluqoqDxp/kbyfs3wXlhGR51rytXk/h0DDUD
U8VYIzIyJ7RyqNRtbSqoaNKTCq73Z2/eWl3O/Tsw/mVbpsPHbgDFG4qmM3wE88Rm
u6tOFHouTMQPSJ1yDWJrnR627uRQsnjmJbElSkSWXGWVtJnBqbj+QPGal+hXR/lv
0Fo3nvKvLG5W7GkRh7rRTBu+reJAM4piFQzWxWStiyQwgRLZzpPbgXmpPfto7J5L
WeRMEJkprUVdps+DuylnsHHPD1EA0sBwvs5FQ7yKFuWoejZLC/loW51uColwiIOw
uhQlLzm8WwLePjYQPatHDegxu056fg==
=YhNZ
-----END PGP SIGNATURE-----
Merge tag 'regulator-fix-v6.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator
Pull regulator fixes from Mark Brown:
"Two core fixes here, one for a long standing race which some Qualcomm
systems have started triggering with their UFS driver and another
fixing a problem with supply lookup introduced by the fixes for devm
related use after free issues that were introduced in this merge
window"
* tag 'regulator-fix-v6.2-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator:
regulator: core: fix deadlock on regulator enable
regulator: core: Fix resolve supply lookup issue
When updating the operating mode as part of regulator enable, the caller
has already locked the regulator tree and drms_uA_update() must not try
to do the same in order not to trigger a deadlock.
The lock inversion is reported by lockdep as:
======================================================
WARNING: possible circular locking dependency detected
6.1.0-next-20221215 #142 Not tainted
------------------------------------------------------
udevd/154 is trying to acquire lock:
ffffc11f123d7e50 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent+0x54/0x280
but task is already holding lock:
ffff80000e4c36e8 (regulator_ww_class_acquire){+.+.}-{0:0}, at: regulator_enable+0x34/0x80
which lock already depends on the new lock.
...
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(regulator_ww_class_acquire);
lock(regulator_list_mutex);
lock(regulator_ww_class_acquire);
lock(regulator_list_mutex);
*** DEADLOCK ***
just before probe of a Qualcomm UFS controller (occasionally) deadlocks
when enabling one of its regulators.
Fixes: 9243a195be ("regulator: core: Change voltage setting path")
Fixes: f8702f9e4a ("regulator: core: Use ww_mutex for regulators locking")
Cc: stable@vger.kernel.org # 5.0
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Link: https://lore.kernel.org/r/20221215104646.19818-1-johan+linaro@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
From Marek's log, the previous change modify the parent of rdev.
https://lore.kernel.org/all/58b92e75-f373-dae7-7031-8abd465bb874@samsung.com/
In 'regulator_resolve_supply', it uses the parent DT node of rdev as the
DT-lookup starting node. But the parent DT node may not exist. This will
cause the NULL supply issue.
This patch modify the parent of rdev back to the device that provides
from 'regulator_config' in 'regulator_register'.
Fixes: 8f3cbcd6b4 ("regulator: core: Use different devices for resource allocation and DT lookup")
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
Link: https://lore.kernel.org/r/1670981831-12583-1-git-send-email-u0084500@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Quite a quiet release for regulator, the diffstat is dominated by the
I2C migration to probe_new() and the newly added MT6357 driver. We've
just one framework addition and the rest is all new device support,
fixes and cleanups.
The framework addition is an API for requesting all regulators defined
in DT, this isn't great practice but has reasonable applications when
there is generic code handling devices on buses where the bus
specification doesn't include power. The immediate application is MDIO
but I believe there's others, it's another API that'll need an eye
keeping on it for undesirable usage.
- An API for requesting all regulators defined in DT.
- Conversion of lots of drivers to the I2C probe_new() API.
- Support for Mediatek MT6357, Qualcomm PM8550, PMR735a and
Richtek RT6190.
There's a cross tree merge with the I2C tree in order to use the new
i2c_client_get_device_id() helper in the conversions to probe_new().
-----BEGIN PGP SIGNATURE-----
iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmOXIIcACgkQJNaLcl1U
h9DW8Af/SkLjUIhuMYrln1QBKvJfMOzAIOo8LeCRCU6E8IxsDu/zccEva6dqr/M/
DFkq5VBCH7cX7VIvaHLjED/VdM0n+JG35tzwv83TVuAohLW/qsRdO6RE9IRBJcUd
7wUj2gjUBjjnCwbpI0hTygqQHvIxO0deQVYoQsxF8VwJTQ+ufpA3TJ3tmxqeWvbd
N/qkLohCk9NoFgiMjzBxBonacaZEvHkjUtqRthHm/nx8Mdu9NhBb6ai6KIpN7EJD
CaY+nUOx/cW8YJWBaZ32bcnvRgtanJGWn+p49/JmUCu0lIEOX2r7jds/kBo/hWWl
akGDUTcmvZY0CfrJfbx+FJxcA1A7Jg==
=Dmpg
-----END PGP SIGNATURE-----
Merge tag 'regulator-v6.2' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator
Pull regulator updates from Mark Brown:
"Quite a quiet release for regulator, the diffstat is dominated by the
I2C migration to probe_new() and the newly added MT6357 driver. We've
just one framework addition and the rest is all new device support,
fixes and cleanups.
The framework addition is an API for requesting all regulators defined
in DT, this isn't great practice but has reasonable applications when
there is generic code handling devices on buses where the bus
specification doesn't include power. The immediate application is MDIO
but I believe there's others, it's another API that'll need an eye
keeping on it for undesirable usage.
Summary:
- An API for requesting all regulators defined in DT
- Conversion of lots of drivers to the I2C probe_new() API
- Support for Mediatek MT6357, Qualcomm PM8550, PMR735a and Richtek
RT6190"
* tag 'regulator-v6.2' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator: (56 commits)
regulator: core: Use different devices for resource allocation and DT lookup
dt-bindings: Add missing 'unevaluatedProperties' to regulator nodes
regulator: qcom-labibb: Fix missing of_node_put() in qcom_labibb_regulator_probe()
regulator: add mt6357 regulator
regulator: dt-bindings: Add binding schema for mt6357 regulators
regulator: core: fix resource leak in regulator_register()
regulator: core: fix module refcount leak in set_supply()
regulator: core: fix use_count leakage when handling boot-on
regulator: rk808: Use dev_err_probe
regulator: rk808: reduce 'struct rk808' usage
regulator: Drop obsolete dependencies on COMPILE_TEST
regulator: pv88080-regulator: Convert to i2c's .probe_new()
regulator: pfuze100-regulator: Convert to i2c's .probe_new()
regulator: isl6271a-regulator: Convert to i2c's .probe_new()
regulator: fan53555: Convert to i2c's .probe_new()
regulator: act8865-regulator: Convert to i2c's .probe_new()
regulator: qcom-rpmh: Add support for PM8550 regulators
regulator: dt-bindings: qcom,rpmh: Add compatible for PM8550
regulator: tps65023-regulator: Convert to i2c's .probe_new()
regulator: tps62360-regulator: Convert to i2c's .probe_new()
...
Following by the below discussion, there's the potential UAF issue
between regulator and mfd.
https://lore.kernel.org/all/20221128143601.1698148-1-yangyingliang@huawei.com/
From the analysis of Yingliang
CPU A |CPU B
mt6370_probe() |
devm_mfd_add_devices() |
|mt6370_regulator_probe()
| regulator_register()
| //allocate init_data and add it to devres
| regulator_of_get_init_data()
i2c_unregister_device() |
device_del() |
devres_release_all() |
// init_data is freed |
release_nodes() |
| // using init_data causes UAF
| regulator_register()
It's common to use mfd core to create child device for the regulator.
In order to do the DT lookup for init data, the child that registered
the regulator would pass its parent as the parameter. And this causes
init data resource allocated to its parent, not itself. The issue happen
when parent device is going to release and regulator core is still doing
some operation of init data constraint for the regulator of child device.
To fix it, this patch expand 'regulator_register' API to use the
different devices for init data allocation and DT lookup.
Reported-by: Yang Yingliang <yangyingliang@huawei.com>
Signed-off-by: ChiYuan Huang <cy_huang@richtek.com>
Link: https://lore.kernel.org/r/1670311341-32664-1-git-send-email-u0084500@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org>
