| Message ID | 20241012115236.53998-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] media: ov08x40: Move fwnode_property_read_u32("clock-frequency") call down | expand |
On 12/10/2024 12:52, Hans de Goede wrote: > If the bridge has not yet setup the fwnode-graph then > the fwnode_property_read_u32("clock-frequency") call will fail. > > Move the fwnode_property_read_u32("clock-frequency") call below > the fwnode_graph_get_next_endpoint() call and return -EPROBE_DEFER > if the latter fails. > > While moving the fwnode_property_read_u32("clock-frequency") call also > switch to dev_err_probe() for logging when it fails so that the error > code gets logged. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/media/i2c/ov08x40.c | 35 ++++++++++++++++++----------------- > 1 file changed, 18 insertions(+), 17 deletions(-) > > diff --git a/drivers/media/i2c/ov08x40.c b/drivers/media/i2c/ov08x40.c > index 7ead3c720e0e..c0dc918edc3c 100644 > --- a/drivers/media/i2c/ov08x40.c > +++ b/drivers/media/i2c/ov08x40.c > @@ -2060,31 +2060,32 @@ static int ov08x40_check_hwcfg(struct device *dev) > int ret; > u32 ext_clk; > > - if (!fwnode) > - return -ENXIO; > - > - ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", > - &ext_clk); > - if (ret) { > - dev_err(dev, "can't get clock frequency"); > - return ret; > - } > - > - if (ext_clk != OV08X40_EXT_CLK) { > - dev_err(dev, "external clock %d is not supported", > - ext_clk); > - return -EINVAL; > - } > - > + /* > + * Sometimes the fwnode graph is initialized by the bridge driver. > + * Bridge drivers doing this also add sensor properties, wait for this. > + */ > ep = fwnode_graph_get_next_endpoint(fwnode, NULL); > if (!ep) > - return -ENXIO; > + return dev_err_probe(dev, -EPROBE_DEFER, > + "waiting for fwnode graph endpoint\n"); > > ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg); > fwnode_handle_put(ep); > if (ret) > return ret; > > + ret = fwnode_property_read_u32(fwnode, "clock-frequency", &ext_clk); > + if (ret) { > + dev_err_probe(dev, ret, "reading clock-frequency\n"); > + goto out_err; > + } > + > + if (ext_clk != OV08X40_EXT_CLK) { > + dev_err(dev, "external clock %u is not supported\n", ext_clk); > + ret = -EINVAL; > + goto out_err; > + } > + > if (bus_cfg.bus.mipi_csi2.num_data_lanes != OV08X40_DATA_LANES) { > dev_err(dev, "number of CSI2 data lanes %d is not supported", > bus_cfg.bus.mipi_csi2.num_data_lanes); 20241010-b4-master-24-11-25-ov08x40-v6-4-cf966e34e685@linaro.org One of us will have to rebase for this change. --- bod
On Sat, Oct 12, 2024 at 04:10:14PM +0100, Bryan O'Donoghue wrote:
> 20241010-b4-master-24-11-25-ov08x40-v6-4-cf966e34e685@linaro.org
Hans: could you rebase this, please? Bryan's set has been out for quite
some time (and now in my tree, devel branch).
Hi, On 1-Nov-24 12:20 PM, Sakari Ailus wrote: > On Sat, Oct 12, 2024 at 04:10:14PM +0100, Bryan O'Donoghue wrote: >> 20241010-b4-master-24-11-25-ov08x40-v6-4-cf966e34e685@linaro.org > > Hans: could you rebase this, please? Bryan's set has been out for quite > some time (and now in my tree, devel branch). Ack will do. I'm still waiting for testing feedback anyways since I don't have a laptop with this sensor myself. Regards, Hans
diff --git a/drivers/media/i2c/ov08x40.c b/drivers/media/i2c/ov08x40.c index 7ead3c720e0e..c0dc918edc3c 100644 --- a/drivers/media/i2c/ov08x40.c +++ b/drivers/media/i2c/ov08x40.c @@ -2060,31 +2060,32 @@ static int ov08x40_check_hwcfg(struct device *dev) int ret; u32 ext_clk; - if (!fwnode) - return -ENXIO; - - ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", - &ext_clk); - if (ret) { - dev_err(dev, "can't get clock frequency"); - return ret; - } - - if (ext_clk != OV08X40_EXT_CLK) { - dev_err(dev, "external clock %d is not supported", - ext_clk); - return -EINVAL; - } - + /* + * Sometimes the fwnode graph is initialized by the bridge driver. + * Bridge drivers doing this also add sensor properties, wait for this. + */ ep = fwnode_graph_get_next_endpoint(fwnode, NULL); if (!ep) - return -ENXIO; + return dev_err_probe(dev, -EPROBE_DEFER, + "waiting for fwnode graph endpoint\n"); ret = v4l2_fwnode_endpoint_alloc_parse(ep, &bus_cfg); fwnode_handle_put(ep); if (ret) return ret; + ret = fwnode_property_read_u32(fwnode, "clock-frequency", &ext_clk); + if (ret) { + dev_err_probe(dev, ret, "reading clock-frequency\n"); + goto out_err; + } + + if (ext_clk != OV08X40_EXT_CLK) { + dev_err(dev, "external clock %u is not supported\n", ext_clk); + ret = -EINVAL; + goto out_err; + } + if (bus_cfg.bus.mipi_csi2.num_data_lanes != OV08X40_DATA_LANES) { dev_err(dev, "number of CSI2 data lanes %d is not supported", bus_cfg.bus.mipi_csi2.num_data_lanes);
If the bridge has not yet setup the fwnode-graph then the fwnode_property_read_u32("clock-frequency") call will fail. Move the fwnode_property_read_u32("clock-frequency") call below the fwnode_graph_get_next_endpoint() call and return -EPROBE_DEFER if the latter fails. While moving the fwnode_property_read_u32("clock-frequency") call also switch to dev_err_probe() for logging when it fails so that the error code gets logged. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/media/i2c/ov08x40.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-)