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