Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1392234

Re: [RESEND PATCH v4 4/4] drm: bridge: anx78xx: Add anx78xx driver support.

From Thierry Reding <thierry.reding@gmail.com>
Newsgroups linux.kernel
Subject Re: [RESEND PATCH v4 4/4] drm: bridge: anx78xx: Add anx78xx driver support.
Date 2016-05-02 16:30 +0200
Message-ID <run1g-4Vp-9@gated-at.bofh.it> (permalink)
References <rugVP-7SW-7@gated-at.bofh.it> <rugVQ-7SW-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


[Multipart message — attachments visible in raw view] - view raw

On Mon, May 02, 2016 at 09:54:26AM +0200, Enric Balletbo i Serra wrote:
[...]
> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
[...]
> +static int anx78xx_init_pdata(struct anx78xx *anx78xx)
> +{
> +	struct device *dev = &anx78xx->client->dev;
> +	struct anx78xx_platform_data *pdata = &anx78xx->pdata;
> +
> +	/* 1.0V digital core power regulator (optional) */
> +	pdata->dvdd10 = devm_regulator_get(dev, "dvdd10");
> +	if (IS_ERR(pdata->dvdd10)) {
> +		DRM_INFO("DVDD10 regulator not found\n");
> +		pdata->dvdd10 = NULL;
> +	}

I'm almost sure that this isn't what you want. What if the regulator is
hooked up but the bridge driver probes before the regulator. I think
what you really want is to simply propagate the error code via:

	return PTR_ERR(pdata->dvdd10);

My understanding is that the regulator core will give you a dummy one if
there's really nothing hooked up. I think you're also supposed to call
regulator_get_optional() (or the devm_*() equivalent) if this is truly
an optional supply. Given that it's the "core power" regulator I doubt
that it's really optional; it's more likely that you may not be able to
control it, and that it's therefore always on. In that case you're
supposed to model it in DT as a fixed regulator that's always on.

This is fairly minor and it's really the only thing I could find, so no
need to respin just for that. If you're fine with the above solution (to
propagate the error code) I can make the change manually while applying.

Thierry

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[RESEND PATCH v4 0/4] Add ANX7814 I2C bridge driver Enric Balletbo i Serra <enric.balletbo@collabora.com> - 2016-05-02 10:00 +0200
  Re: [RESEND PATCH v4 4/4] drm: bridge: anx78xx: Add anx78xx driver  support. Thierry Reding <thierry.reding@gmail.com> - 2016-05-02 16:30 +0200
    Re: [RESEND PATCH v4 4/4] drm: bridge: anx78xx: Add anx78xx driver support. Enric Balletbo Serra <eballetbo@gmail.com> - 2016-05-02 17:30 +0200

csiph-web