| Message ID | 20211021214331.1188787-15-djrscally@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Extensions to ov8865 driver | expand |
Hi Dan, On 10/21/21 23:43, Daniel Scally wrote: > There is a chance that regulator_get() returns -EPROBE_DEFER, in which > case printing an error message is undesirable. To avoid spurious messages > in dmesg in the event that -EPROBE_DEFER is returned, use dev_err_probe() > on error paths for regulator_get(). > > Signed-off-by: Daniel Scally <djrscally@gmail.com> > --- > Changes in v3: > > - None > > drivers/media/i2c/ov8865.c | 46 +++++++++++++++++--------------------- > 1 file changed, 20 insertions(+), 26 deletions(-) > > diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c > index 572b9818e950..685539744041 100644 > --- a/drivers/media/i2c/ov8865.c > +++ b/drivers/media/i2c/ov8865.c > @@ -2955,6 +2955,26 @@ static int ov8865_probe(struct i2c_client *client) > sensor->dev = dev; > sensor->i2c_client = client; > > + /* Regulators */ > + > + /* DVDD: digital core */ > + sensor->dvdd = devm_regulator_get(dev, "dvdd"); > + if (IS_ERR(sensor->dvdd)) > + return dev_err_probe(dev, PTR_ERR(sensor->dvdd), > + "cannot get DVDD regulator\n"); > + > + /* DOVDD: digital I/O */ > + sensor->dovdd = devm_regulator_get(dev, "dovdd"); > + if (IS_ERR(sensor->dovdd)) > + return dev_err_probe(dev, PTR_ERR(sensor->dovdd), > + "cannot get DOVDD regulator\n"); > + > + /* AVDD: analog */ > + sensor->avdd = devm_regulator_get(dev, "avdd"); > + if (IS_ERR(sensor->avdd)) > + return dev_err_probe(dev, PTR_ERR(sensor->avdd), > + "cannot get AVDD (analog) regulator\n"); > + > /* Graph Endpoint */ > > handle = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL); > @@ -2985,32 +3005,6 @@ static int ov8865_probe(struct i2c_client *client) > goto error_endpoint; > } > > - /* Regulators */ > - > - /* DVDD: digital core */ > - sensor->dvdd = devm_regulator_get(dev, "dvdd"); > - if (IS_ERR(sensor->dvdd)) { > - dev_err(dev, "cannot get DVDD (digital core) regulator\n"); > - ret = PTR_ERR(sensor->dvdd); > - goto error_endpoint; > - } > - > - /* DOVDD: digital I/O */ > - sensor->dovdd = devm_regulator_get(dev, "dovdd"); > - if (IS_ERR(sensor->dovdd)) { > - dev_err(dev, "cannot get DOVDD (digital I/O) regulator\n"); > - ret = PTR_ERR(sensor->dovdd); > - goto error_endpoint; > - } > - > - /* AVDD: analog */ > - sensor->avdd = devm_regulator_get(dev, "avdd"); > - if (IS_ERR(sensor->avdd)) { > - dev_err(dev, "cannot get AVDD (analog) regulator\n"); > - ret = PTR_ERR(sensor->avdd); > - goto error_endpoint; > - } > - > /* External Clock */ > This line: > sensor->extclk = devm_clk_get(dev, "tps68470-clk"); Does not exist in the upstream repos, instead it is: sensor->extclk = devm_clk_get(dev, NULL); I guess you still had your hack to deal with the clk issues we've been working on in place in your tree on which you based this series. Unfortunately this means that this patch (and thus this series) will not apply cleanly. Can you send a v4 fixing this? Regards, Hans
Hi Hans On 28/10/2021 15:01, Hans de Goede wrote: > Hi Dan, > > On 10/21/21 23:43, Daniel Scally wrote: >> There is a chance that regulator_get() returns -EPROBE_DEFER, in which >> case printing an error message is undesirable. To avoid spurious messages >> in dmesg in the event that -EPROBE_DEFER is returned, use dev_err_probe() >> on error paths for regulator_get(). >> >> Signed-off-by: Daniel Scally <djrscally@gmail.com> > > >> --- >> Changes in v3: >> >> - None >> >> drivers/media/i2c/ov8865.c | 46 +++++++++++++++++--------------------- >> 1 file changed, 20 insertions(+), 26 deletions(-) >> >> diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c >> index 572b9818e950..685539744041 100644 >> --- a/drivers/media/i2c/ov8865.c >> +++ b/drivers/media/i2c/ov8865.c >> @@ -2955,6 +2955,26 @@ static int ov8865_probe(struct i2c_client *client) >> sensor->dev = dev; >> sensor->i2c_client = client; >> >> + /* Regulators */ >> + >> + /* DVDD: digital core */ >> + sensor->dvdd = devm_regulator_get(dev, "dvdd"); >> + if (IS_ERR(sensor->dvdd)) >> + return dev_err_probe(dev, PTR_ERR(sensor->dvdd), >> + "cannot get DVDD regulator\n"); >> + >> + /* DOVDD: digital I/O */ >> + sensor->dovdd = devm_regulator_get(dev, "dovdd"); >> + if (IS_ERR(sensor->dovdd)) >> + return dev_err_probe(dev, PTR_ERR(sensor->dovdd), >> + "cannot get DOVDD regulator\n"); >> + >> + /* AVDD: analog */ >> + sensor->avdd = devm_regulator_get(dev, "avdd"); >> + if (IS_ERR(sensor->avdd)) >> + return dev_err_probe(dev, PTR_ERR(sensor->avdd), >> + "cannot get AVDD (analog) regulator\n"); >> + >> /* Graph Endpoint */ >> >> handle = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL); >> @@ -2985,32 +3005,6 @@ static int ov8865_probe(struct i2c_client *client) >> goto error_endpoint; >> } >> >> - /* Regulators */ >> - >> - /* DVDD: digital core */ >> - sensor->dvdd = devm_regulator_get(dev, "dvdd"); >> - if (IS_ERR(sensor->dvdd)) { >> - dev_err(dev, "cannot get DVDD (digital core) regulator\n"); >> - ret = PTR_ERR(sensor->dvdd); >> - goto error_endpoint; >> - } >> - >> - /* DOVDD: digital I/O */ >> - sensor->dovdd = devm_regulator_get(dev, "dovdd"); >> - if (IS_ERR(sensor->dovdd)) { >> - dev_err(dev, "cannot get DOVDD (digital I/O) regulator\n"); >> - ret = PTR_ERR(sensor->dovdd); >> - goto error_endpoint; >> - } >> - >> - /* AVDD: analog */ >> - sensor->avdd = devm_regulator_get(dev, "avdd"); >> - if (IS_ERR(sensor->avdd)) { >> - dev_err(dev, "cannot get AVDD (analog) regulator\n"); >> - ret = PTR_ERR(sensor->avdd); >> - goto error_endpoint; >> - } >> - >> /* External Clock */ >> > This line: > >> sensor->extclk = devm_clk_get(dev, "tps68470-clk"); > Does not exist in the upstream repos, instead it is: > > sensor->extclk = devm_clk_get(dev, NULL); > > I guess you still had your hack to deal with the clk issues we've > been working on in place in your tree on which you based this series. > > Unfortunately this means that this patch (and thus this series) > will not apply cleanly. > > Can you send a v4 fixing this? > Oops! My bad, sorry about that. I'll post a v4 later to clean that up
diff --git a/drivers/media/i2c/ov8865.c b/drivers/media/i2c/ov8865.c index 572b9818e950..685539744041 100644 --- a/drivers/media/i2c/ov8865.c +++ b/drivers/media/i2c/ov8865.c @@ -2955,6 +2955,26 @@ static int ov8865_probe(struct i2c_client *client) sensor->dev = dev; sensor->i2c_client = client; + /* Regulators */ + + /* DVDD: digital core */ + sensor->dvdd = devm_regulator_get(dev, "dvdd"); + if (IS_ERR(sensor->dvdd)) + return dev_err_probe(dev, PTR_ERR(sensor->dvdd), + "cannot get DVDD regulator\n"); + + /* DOVDD: digital I/O */ + sensor->dovdd = devm_regulator_get(dev, "dovdd"); + if (IS_ERR(sensor->dovdd)) + return dev_err_probe(dev, PTR_ERR(sensor->dovdd), + "cannot get DOVDD regulator\n"); + + /* AVDD: analog */ + sensor->avdd = devm_regulator_get(dev, "avdd"); + if (IS_ERR(sensor->avdd)) + return dev_err_probe(dev, PTR_ERR(sensor->avdd), + "cannot get AVDD (analog) regulator\n"); + /* Graph Endpoint */ handle = fwnode_graph_get_next_endpoint(dev_fwnode(dev), NULL); @@ -2985,32 +3005,6 @@ static int ov8865_probe(struct i2c_client *client) goto error_endpoint; } - /* Regulators */ - - /* DVDD: digital core */ - sensor->dvdd = devm_regulator_get(dev, "dvdd"); - if (IS_ERR(sensor->dvdd)) { - dev_err(dev, "cannot get DVDD (digital core) regulator\n"); - ret = PTR_ERR(sensor->dvdd); - goto error_endpoint; - } - - /* DOVDD: digital I/O */ - sensor->dovdd = devm_regulator_get(dev, "dovdd"); - if (IS_ERR(sensor->dovdd)) { - dev_err(dev, "cannot get DOVDD (digital I/O) regulator\n"); - ret = PTR_ERR(sensor->dovdd); - goto error_endpoint; - } - - /* AVDD: analog */ - sensor->avdd = devm_regulator_get(dev, "avdd"); - if (IS_ERR(sensor->avdd)) { - dev_err(dev, "cannot get AVDD (analog) regulator\n"); - ret = PTR_ERR(sensor->avdd); - goto error_endpoint; - } - /* External Clock */ sensor->extclk = devm_clk_get(dev, "tps68470-clk");
There is a chance that regulator_get() returns -EPROBE_DEFER, in which case printing an error message is undesirable. To avoid spurious messages in dmesg in the event that -EPROBE_DEFER is returned, use dev_err_probe() on error paths for regulator_get(). Signed-off-by: Daniel Scally <djrscally@gmail.com> --- Changes in v3: - None drivers/media/i2c/ov8865.c | 46 +++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 26 deletions(-)