Hello Sascha,
On 05.01.22 10:21, Ahmad Fatoum wrote:
> Hi,
>
> On 05.01.22 10:14, Sascha Hauer wrote:
>> On Mon, Jan 03, 2022 at 01:03:36PM +0100, Ahmad Fatoum wrote:
>>> For testing regulator drivers, it can be handy to enable/disable them
>>> from the shell prompt. Extend the regulator command to support this.
>>>
>>> Signed-off-by: Ahmad Fatoum <a.fatoum@xxxxxxxxxxxxxx>
>>> ---
>>> commands/regulator.c | 38 +++++++++++++++++++++++++++++++++++---
>>> drivers/regulator/core.c | 8 ++++++++
>>> include/regulator.h | 1 +
>>> 3 files changed, 44 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/commands/regulator.c b/commands/regulator.c
>>> index 3e2595f8bfc1..e6b2f4852db4 100644
>>> --- a/commands/regulator.c
>>> +++ b/commands/regulator.c
>>> @@ -6,16 +6,48 @@
>>> #include <common.h>
>>> #include <command.h>
>>> #include <regulator.h>
>>> +#include <getopt.h>
>>>
>>> static int do_regulator(int argc, char *argv[])
>>> {
>>> - regulators_print();
>>> + struct regulator *chosen;
>>> + int opt, ret;
>>> +
>>> + while ((opt = getopt(argc, argv, "e:d:")) > 0) {
>>> + switch (opt) {
>>> + case 'e':
>>> + case 'd':
>>> + chosen = regulator_get_name(optarg);
>>> + if (IS_ERR_OR_NULL(chosen)) {
>>> + printf("regulator not found\n");
>>> + return COMMAND_ERROR;
>>> + }
>>> +
>>> + ret = opt == 'e' ? regulator_enable(chosen)
>>> + : regulator_disable(chosen);
>>> + regulator_put(chosen);
>>> + return ret;
>>
>> The barebox regulator core distinguishes between struct regulator and
>> struct regulator_internal. regulator_internal represents the physical
>> regulator whereas regulator is allocated for each consumer. If the
>> regulator core were a bit more sophisticated then a regulator would
>> have it's own enable count and you would be warned when a regulators
>> enable count goes below zero.
>>
>> I agree that controlling regulators on the command line would be useful,
>> but I also don't want to block extending the regulator core in such a
>> way.
>
> Should I move the command implementation then into drivers/regulator/core.c?
>
> That's what I did here[1], but I seem to recall that you objected to
> moving the command into drivers/ to access internals, when the API
> should suffice/be extended. I can't find the mail right now though or
> perhaps my recollection is erroneous.
>
> So how to proceed?
I keep forward-porting this patch every time I port a regulator driver.
What do I need to do to get this merged?
Cheers,
Ahmad
>
> [1]: https://lore.barebox.org/barebox/20191106094459.w32tgsl22ty34vhe@xxxxxxxxxxxxxx/#t
>
> Cheers,
> Ahmad
>
>>
>> Sascha
>>
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
