| 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
| ||
|
Message-ID: <562744A9.1070705@redhat.com>
Date: Wed, 21 Oct 2015 09:54:17 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Chen-Yu Tsai <wens@...e.org>,
Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>,
Tomi Valkeinen <tomi.valkeinen@...com>,
Rob Herring <robh+dt@...nel.org>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Kumar Gala <galak@...eaurora.org>
Cc: linux-fbdev@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH RFC v2 2/2] simplefb: Claim and enable regulators
Hi,
On 21-10-15 07:59, Chen-Yu Tsai wrote:
> This claims and enables regulators listed in the simple framebuffer dt
> node. This is needed so that regulators powering the display pipeline
> and external hardware, described in the device node and known by the
> kernel code, will remain properly enabled.
>
> Signed-off-by: Chen-Yu Tsai <wens@...e.org>
> ---
> drivers/video/fbdev/simplefb.c | 122 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 121 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> index 52c5c7e63b52..c4ee10d83a70 100644
> --- a/drivers/video/fbdev/simplefb.c
> +++ b/drivers/video/fbdev/simplefb.c
> @@ -28,7 +28,10 @@
> #include <linux/platform_device.h>
> #include <linux/clk.h>
> #include <linux/clk-provider.h>
> +#include <linux/of.h>
> #include <linux/of_platform.h>
> +#include <linux/parser.h>
> +#include <linux/regulator/consumer.h>
>
> static struct fb_fix_screeninfo simplefb_fix = {
> .id = "simple",
> @@ -174,6 +177,10 @@ struct simplefb_par {
> int clk_count;
> struct clk **clks;
> #endif
> +#if defined CONFIG_OF && defined CONFIG_REGULATOR
> + u32 regulator_count;
> + struct regulator **regulators;
> +#endif
> };
>
> #if defined CONFIG_OF && defined CONFIG_COMMON_CLK
> @@ -269,6 +276,112 @@ static int simplefb_clocks_init(struct simplefb_par *par,
> static void simplefb_clocks_destroy(struct simplefb_par *par) { }
> #endif
>
> +#if defined CONFIG_OF && defined CONFIG_REGULATOR
> +
> +#define SUPPLY_SUFFIX "-supply"
> +
> +/*
> + * Regulator handling code.
> + *
> + * Here we handle the num-supplies and vin*-supply properties of our
> + * "simple-framebuffer" dt node. This is necessary so that we can make sure
> + * that any regulators needed by the display hardware that the bootloader
> + * set up for us (and for which it provided a simplefb dt node), stay up,
> + * for the life of the simplefb driver.
> + *
> + * When the driver unloads, we cleanly disable, and then release the
> + * regulators.
> + *
> + * We only complain about errors here, no action is taken as the most likely
> + * error can only happen due to a mismatch between the bootloader which set
> + * up simplefb, and the regulator definitions in the device tree. Chances are
> + * that there are no adverse effects, and if there are, a clean teardown of
> + * the fb probe will not help us much either. So just complain and carry on,
> + * and hope that the user actually gets a working fb at the end of things.
> + */
> +static int simplefb_regulators_init(struct simplefb_par *par,
> + struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct property *prop;
> + struct regulator *regulator;
> + const char *p;
> + int count = 0, i = 0, ret;
> +
> + if (dev_get_platdata(&pdev->dev) || !np)
> + return 0;
> +
> + /* Count the number of regulator supplies */
> + for_each_property_of_node(np, prop) {
> + p = strstr(prop->name, SUPPLY_SUFFIX);
> + if (p && p != prop->name)
> + count++;
> + }
> +
> + if (!count)
> + return 0;
> +
> + par->regulators = devm_kcalloc(&pdev->dev, count,
> + sizeof(struct regulator *), GFP_KERNEL);
> + if (!par->regulators)
> + return -ENOMEM;
> +
> + /* Get all the regulators */
> + for_each_property_of_node(np, prop) {
> + char name[32]; /* 32 is max size of property name */
> +
> + p = strstr(prop->name, SUPPLY_SUFFIX);
> + if (p && p != prop->name)
> + continue;
> +
> + strlcpy(name, prop->name,
> + strlen(prop->name) - sizeof(SUPPLY_SUFFIX) + 1);
> + regulator = devm_regulator_get_optional(&pdev->dev, name);
> + if (IS_ERR(regulator)) {
> + if (PTR_ERR(regulator) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + dev_err(&pdev->dev, "regulator %s not found: %ld\n",
> + name, PTR_ERR(regulator));
> + continue;
> + }
> + par->regulators[i++] = regulator;
So you only fill slots when the regulator_get has succeeded
> + }
> + par->regulator_count = i;
and regulator_count now is the amount of successfully gotten regulators
(which may be different from count).
> +
> + /* Enable all the regulators */
> + for (i = 0; i < par->regulator_count; i++) {
> + if (par->regulators[i]) {
That means that this "if" is not necessary, it will always be true.
> + ret = regulator_enable(par->regulators[i]);
> + if (ret) {
> + dev_err(&pdev->dev,
> + "failed to enable regulator %d: %d\n",
> + i, ret);
> + devm_regulator_put(par->regulators[i]);
> + par->regulators[i] = NULL;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> +static void simplefb_regulators_destroy(struct simplefb_par *par)
> +{
> + int i;
> +
> + if (!par->regulators)
> + return;
> +
> + for (i = 0; i < par->regulator_count; i++)
> + if (par->regulators[i])
And idem for this if.
Other then that this patch looks good and is:
Reviewed-by: Hans de Goede <hdegoede@...hat.com>
Regards,
Hans
> + regulator_disable(par->regulators[i]);
> +}
> +#else
> +static int simplefb_regulators_init(struct simplefb_par *par,
> + struct platform_device *pdev) { return 0; }
> +static void simplefb_regulators_destroy(struct simplefb_par *par) { }
> +#endif
> +
> static int simplefb_probe(struct platform_device *pdev)
> {
> int ret;
> @@ -340,6 +453,10 @@ static int simplefb_probe(struct platform_device *pdev)
> if (ret < 0)
> goto error_unmap;
>
> + ret = simplefb_regulators_init(par, pdev);
> + if (ret < 0)
> + goto error_clocks;
> +
> dev_info(&pdev->dev, "framebuffer at 0x%lx, 0x%x bytes, mapped to 0x%p\n",
> info->fix.smem_start, info->fix.smem_len,
> info->screen_base);
> @@ -351,13 +468,15 @@ static int simplefb_probe(struct platform_device *pdev)
> ret = register_framebuffer(info);
> if (ret < 0) {
> dev_err(&pdev->dev, "Unable to register simplefb: %d\n", ret);
> - goto error_clocks;
> + goto error_regulators;
> }
>
> dev_info(&pdev->dev, "fb%d: simplefb registered!\n", info->node);
>
> return 0;
>
> +error_regulators:
> + simplefb_regulators_destroy(par);
> error_clocks:
> simplefb_clocks_destroy(par);
> error_unmap:
> @@ -373,6 +492,7 @@ static int simplefb_remove(struct platform_device *pdev)
> struct simplefb_par *par = info->par;
>
> unregister_framebuffer(info);
> + simplefb_regulators_destroy(par);
> simplefb_clocks_destroy(par);
> framebuffer_release(info);
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists