Re: [PATCH v2 2/2] regulator: uniphier: add regulator driver for UniPhier SoC

From: Mark Brown
Date: Tue Jul 10 2018 - 14:39:44 EST


On Tue, Jul 10, 2018 at 10:27:17AM +0900, Kunihiko Hayashi wrote:

> +static int uniphier_regulator_enable(struct regulator_dev *rdev)
> +{
> + struct uniphier_regulator_priv *priv = rdev_get_drvdata(rdev);
> +
> + return regmap_update_bits(priv->regmap, rdev->desc->enable_reg,
> + rdev->desc->enable_mask,
> + rdev->desc->enable_val);
> +}

This is just regulator_enable_regmap() isn't it? Same for disable,
unless I'm missing something (which is possible).

> +static int uniphier_regulator_is_enabled(struct regulator_dev *rdev)
> +{
> + struct uniphier_regulator_priv *priv = rdev_get_drvdata(rdev);
> + unsigned int val;
> + int ret = -EINVAL;
> +
> + regmap_read(priv->regmap, rdev->desc->enable_reg, &val);
> + val &= rdev->desc->enable_mask;
> +
> + if (val == rdev->desc->enable_val)
> + ret = 1;
> + else if (val == rdev->desc->disable_val)
> + ret = 0;
> +
> + return ret;
> +}

This isn't *quite* regulator_is_enabled_regmap() as it explicitly checks
if the disable value is set and errors otherwise which would be a good
fix for that. Could you do a patch for that?

Attachment: signature.asc
Description: PGP signature