Messages in this thread | | | Date | Mon, 2 May 2016 17:22:08 +0200 | Subject | Re: [RESEND PATCH v4 4/4] drm: bridge: anx78xx: Add anx78xx driver support. | From | Enric Balletbo Serra <> |
| |
Hi Thierry,
2016-05-02 16:22 GMT+02:00 Thierry Reding <thierry.reding@gmail.com>: > 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. >
Yes, I was thinking in the case you're are not able to control it and I make it as optional, and yes, you have reason so It's fine with me the above solution.
> 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. >
It's up to you. Whichever you prefer.
Thanks, Enric
> Thierry > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
| |