linux-kernel - Re: [PATCH 2/2] regulator: uniphier: add regulator driver for UniPhier SoC

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: <20180629110341.GA6556@sirena.org.uk>
Date:   Fri, 29 Jun 2018 12:03:41 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
Cc:     Liam Girdwood <lgirdwood@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org,
        Masami Hiramatsu <masami.hiramatsu@...aro.org>,
        Jassi Brar <jaswinder.singh@...aro.org>
Subject: Re: [PATCH 2/2] regulator: uniphier: add regulator driver for
 UniPhier SoC

On Fri, Jun 29, 2018 at 05:22:13PM +0900, Kunihiko Hayashi wrote:

> +++ b/drivers/regulator/uniphier-regulator.c
> @@ -0,0 +1,251 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Regulator controller driver for UniPhier SoC
> + * Copyright 2018 Socionext Inc.
> + * Author: Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>
> + */

Please make the entire header a C++ comment so that it looks intentional
rather than mixing C and C++ like this.

> +static int uniphier_regulator_enable(struct regulator_dev *rdev)
> +{
> +	struct uniphier_regulator_priv *priv = rdev_get_drvdata(rdev);
> +	u32 val;
> +
> +	val = readl_relaxed(priv->base + rdev->desc->enable_reg);
> +	val &= ~rdev->desc->enable_mask;
> +	val |= rdev->desc->enable_val;
> +	writel_relaxed(val, priv->base + rdev->desc->enable_reg);
> +
> +	return 0;
> +}

Could you use a MMIO regmap for this driver?  All the operations look
like they're just straight up operations of the sort the standard
helpers support them and it means the driver will be able to take
advantage of any improvements the core makes for free.

Otherwise this looks great, and the above two issues can be fixed as
followup patches so I'll apply.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)