| Message ID | 20230627131830.54601-15-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: ov2680: Bugfixes + ACPI + selection(crop-tgt) API support | expand |
Morning Hans On 27/06/2023 15:18, Hans de Goede wrote: > On ACPI systems the following 2 scenarios are possible: > > 1. The xvclk is fully controlled by ACPI powermanagement, so there > is no "xvclk" for the driver to get (since it is abstracted away). > In this case there will be a "clock-frequency" device property > to tell the driver the xvclk rate. > > 2. There is a xvclk modelled in the clk framework for the driver, > but the clk-generator may not be set to the right frequency > yet. In this case there will also be a "clock-frequency" device > property and the driver is expected to set the rate of the xvclk > through this frequency through the clk framework. > > Handle both these scenarios by switching to devm_clk_get_optional() > and checking for a "clock-frequency" device property. > > This is modelled after how the same issue was fixed for the ov8865 in > commit 73dcffeb2ff9 ("media: i2c: Support 19.2MHz input clock in ov8865"). > > Acked-by: Rui Miguel Silva <rmfrfs@gmail.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com> > drivers/media/i2c/ov2680.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c > index b7c23286700e..a6a83f0e53f3 100644 > --- a/drivers/media/i2c/ov2680.c > +++ b/drivers/media/i2c/ov2680.c > @@ -698,6 +698,7 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) > { > struct device *dev = sensor->dev; > struct gpio_desc *gpio; > + unsigned int rate = 0; > int ret; > > /* > @@ -718,13 +719,34 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) > > sensor->pwdn_gpio = gpio; > > - sensor->xvclk = devm_clk_get(dev, "xvclk"); > + sensor->xvclk = devm_clk_get_optional(dev, "xvclk"); > if (IS_ERR(sensor->xvclk)) { > dev_err(dev, "xvclk clock missing or invalid\n"); > return PTR_ERR(sensor->xvclk); > } > > - sensor->xvclk_freq = clk_get_rate(sensor->xvclk); > + /* > + * We could have either a 24MHz or 19.2MHz clock rate from either DT or > + * ACPI... but we also need to support the weird IPU3 case which will > + * have an external clock AND a clock-frequency property. Check for the > + * clock-frequency property and if found, set that rate if we managed > + * to acquire a clock. This should cover the ACPI case. If the system > + * uses devicetree then the configured rate should already be set, so > + * we can just read it. > + */ > + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", > + &rate); > + if (ret && !sensor->xvclk) > + return dev_err_probe(dev, ret, "invalid clock config\n"); > + > + if (!ret && sensor->xvclk) { > + ret = clk_set_rate(sensor->xvclk, rate); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to set clock rate\n"); > + } > + > + sensor->xvclk_freq = rate ?: clk_get_rate(sensor->xvclk); > if (sensor->xvclk_freq != OV2680_XVCLK_VALUE) { > dev_err(dev, "wrong xvclk frequency %d HZ, expected: %d Hz\n", > sensor->xvclk_freq, OV2680_XVCLK_VALUE);
Hi Hans, On Tue, Jun 27, 2023 at 03:18:15PM +0200, Hans de Goede wrote: > On ACPI systems the following 2 scenarios are possible: > > 1. The xvclk is fully controlled by ACPI powermanagement, so there > is no "xvclk" for the driver to get (since it is abstracted away). > In this case there will be a "clock-frequency" device property > to tell the driver the xvclk rate. > > 2. There is a xvclk modelled in the clk framework for the driver, > but the clk-generator may not be set to the right frequency > yet. In this case there will also be a "clock-frequency" device > property and the driver is expected to set the rate of the xvclk > through this frequency through the clk framework. > > Handle both these scenarios by switching to devm_clk_get_optional() > and checking for a "clock-frequency" device property. > > This is modelled after how the same issue was fixed for the ov8865 in > commit 73dcffeb2ff9 ("media: i2c: Support 19.2MHz input clock in ov8865"). > > Acked-by: Rui Miguel Silva <rmfrfs@gmail.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/media/i2c/ov2680.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c > index b7c23286700e..a6a83f0e53f3 100644 > --- a/drivers/media/i2c/ov2680.c > +++ b/drivers/media/i2c/ov2680.c > @@ -698,6 +698,7 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) > { > struct device *dev = sensor->dev; > struct gpio_desc *gpio; > + unsigned int rate = 0; > int ret; > > /* > @@ -718,13 +719,34 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) > > sensor->pwdn_gpio = gpio; > > - sensor->xvclk = devm_clk_get(dev, "xvclk"); > + sensor->xvclk = devm_clk_get_optional(dev, "xvclk"); > if (IS_ERR(sensor->xvclk)) { > dev_err(dev, "xvclk clock missing or invalid\n"); > return PTR_ERR(sensor->xvclk); > } > > - sensor->xvclk_freq = clk_get_rate(sensor->xvclk); > + /* > + * We could have either a 24MHz or 19.2MHz clock rate from either DT or > + * ACPI... but we also need to support the weird IPU3 case which will > + * have an external clock AND a clock-frequency property. Check for the > + * clock-frequency property and if found, set that rate if we managed > + * to acquire a clock. This should cover the ACPI case. If the system > + * uses devicetree then the configured rate should already be set, so > + * we can just read it. > + */ Nice comment :) > + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", > + &rate); > + if (ret && !sensor->xvclk) > + return dev_err_probe(dev, ret, "invalid clock config\n"); > + > + if (!ret && sensor->xvclk) { > + ret = clk_set_rate(sensor->xvclk, rate); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to set clock rate\n"); > + } > + > + sensor->xvclk_freq = rate ?: clk_get_rate(sensor->xvclk); > if (sensor->xvclk_freq != OV2680_XVCLK_VALUE) { > dev_err(dev, "wrong xvclk frequency %d HZ, expected: %d Hz\n", > sensor->xvclk_freq, OV2680_XVCLK_VALUE); Looks good to me. Reviewed-by: Tommaso Merciai <tomm.merciai@gmail.com> Regards, Tommaso > -- > 2.41.0 >
Hi Hans, On Tue, Jun 27, 2023 at 03:18:15PM +0200, Hans de Goede wrote: > On ACPI systems the following 2 scenarios are possible: > > 1. The xvclk is fully controlled by ACPI powermanagement, so there > is no "xvclk" for the driver to get (since it is abstracted away). > In this case there will be a "clock-frequency" device property > to tell the driver the xvclk rate. > > 2. There is a xvclk modelled in the clk framework for the driver, > but the clk-generator may not be set to the right frequency > yet. In this case there will also be a "clock-frequency" device > property and the driver is expected to set the rate of the xvclk > through this frequency through the clk framework. > > Handle both these scenarios by switching to devm_clk_get_optional() > and checking for a "clock-frequency" device property. > > This is modelled after how the same issue was fixed for the ov8865 in > commit 73dcffeb2ff9 ("media: i2c: Support 19.2MHz input clock in ov8865"). > > Acked-by: Rui Miguel Silva <rmfrfs@gmail.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/media/i2c/ov2680.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c > index b7c23286700e..a6a83f0e53f3 100644 > --- a/drivers/media/i2c/ov2680.c > +++ b/drivers/media/i2c/ov2680.c > @@ -698,6 +698,7 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) > { > struct device *dev = sensor->dev; > struct gpio_desc *gpio; > + unsigned int rate = 0; > int ret; > > /* > @@ -718,13 +719,34 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) > > sensor->pwdn_gpio = gpio; > > - sensor->xvclk = devm_clk_get(dev, "xvclk"); > + sensor->xvclk = devm_clk_get_optional(dev, "xvclk"); > if (IS_ERR(sensor->xvclk)) { > dev_err(dev, "xvclk clock missing or invalid\n"); > return PTR_ERR(sensor->xvclk); > } > > - sensor->xvclk_freq = clk_get_rate(sensor->xvclk); > + /* > + * We could have either a 24MHz or 19.2MHz clock rate from either DT or > + * ACPI... but we also need to support the weird IPU3 case which will > + * have an external clock AND a clock-frequency property. Check for the Where does this happen? This puts the driver in an awful situation. :-( > + * clock-frequency property and if found, set that rate if we managed > + * to acquire a clock. This should cover the ACPI case. If the system > + * uses devicetree then the configured rate should already be set, so > + * we can just read it. > + */ > + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", > + &rate); > + if (ret && !sensor->xvclk) > + return dev_err_probe(dev, ret, "invalid clock config\n"); > + > + if (!ret && sensor->xvclk) { > + ret = clk_set_rate(sensor->xvclk, rate); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to set clock rate\n"); > + } > + > + sensor->xvclk_freq = rate ?: clk_get_rate(sensor->xvclk); > if (sensor->xvclk_freq != OV2680_XVCLK_VALUE) { > dev_err(dev, "wrong xvclk frequency %d HZ, expected: %d Hz\n", > sensor->xvclk_freq, OV2680_XVCLK_VALUE);
Hi, On 7/31/23 14:44, Sakari Ailus wrote: > Hi Hans, > > On Tue, Jun 27, 2023 at 03:18:15PM +0200, Hans de Goede wrote: >> On ACPI systems the following 2 scenarios are possible: >> >> 1. The xvclk is fully controlled by ACPI powermanagement, so there >> is no "xvclk" for the driver to get (since it is abstracted away). >> In this case there will be a "clock-frequency" device property >> to tell the driver the xvclk rate. >> >> 2. There is a xvclk modelled in the clk framework for the driver, >> but the clk-generator may not be set to the right frequency >> yet. In this case there will also be a "clock-frequency" device >> property and the driver is expected to set the rate of the xvclk >> through this frequency through the clk framework. >> >> Handle both these scenarios by switching to devm_clk_get_optional() >> and checking for a "clock-frequency" device property. >> >> This is modelled after how the same issue was fixed for the ov8865 in >> commit 73dcffeb2ff9 ("media: i2c: Support 19.2MHz input clock in ov8865"). >> >> Acked-by: Rui Miguel Silva <rmfrfs@gmail.com> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/media/i2c/ov2680.c | 26 ++++++++++++++++++++++++-- >> 1 file changed, 24 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c >> index b7c23286700e..a6a83f0e53f3 100644 >> --- a/drivers/media/i2c/ov2680.c >> +++ b/drivers/media/i2c/ov2680.c >> @@ -698,6 +698,7 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) >> { >> struct device *dev = sensor->dev; >> struct gpio_desc *gpio; >> + unsigned int rate = 0; >> int ret; >> >> /* >> @@ -718,13 +719,34 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) >> >> sensor->pwdn_gpio = gpio; >> >> - sensor->xvclk = devm_clk_get(dev, "xvclk"); >> + sensor->xvclk = devm_clk_get_optional(dev, "xvclk"); >> if (IS_ERR(sensor->xvclk)) { >> dev_err(dev, "xvclk clock missing or invalid\n"); >> return PTR_ERR(sensor->xvclk); >> } >> >> - sensor->xvclk_freq = clk_get_rate(sensor->xvclk); >> + /* >> + * We could have either a 24MHz or 19.2MHz clock rate from either DT or >> + * ACPI... but we also need to support the weird IPU3 case which will >> + * have an external clock AND a clock-frequency property. Check for the > > Where does this happen? This puts the driver in an awful situation. :-( This happens on IPU3 setups where the INT3472 device represents an actual i2c attached sensor PMIC (rather then just some GPIOs) in this case there is a clk generator inside the PMIC which is used and that is programmable, so the driver needs to set the clk-rate. Note this patch is pretty much a 1:1 copy of the same patch for the ov8865 and ov7251 drivers. I guess it might be good to start a discussion about doing this more elegantly but that seems out of scope for this series. Regards, Hans > >> + * clock-frequency property and if found, set that rate if we managed >> + * to acquire a clock. This should cover the ACPI case. If the system >> + * uses devicetree then the configured rate should already be set, so >> + * we can just read it. >> + */ >> + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", >> + &rate); >> + if (ret && !sensor->xvclk) >> + return dev_err_probe(dev, ret, "invalid clock config\n"); >> + >> + if (!ret && sensor->xvclk) { >> + ret = clk_set_rate(sensor->xvclk, rate); >> + if (ret) >> + return dev_err_probe(dev, ret, >> + "failed to set clock rate\n"); >> + } >> + >> + sensor->xvclk_freq = rate ?: clk_get_rate(sensor->xvclk); >> if (sensor->xvclk_freq != OV2680_XVCLK_VALUE) { >> dev_err(dev, "wrong xvclk frequency %d HZ, expected: %d Hz\n", >> sensor->xvclk_freq, OV2680_XVCLK_VALUE); >
Hi Hans, On Mon, Jul 31, 2023 at 02:54:13PM +0200, Hans de Goede wrote: > Hi, > > On 7/31/23 14:44, Sakari Ailus wrote: > > Hi Hans, > > > > On Tue, Jun 27, 2023 at 03:18:15PM +0200, Hans de Goede wrote: > >> On ACPI systems the following 2 scenarios are possible: > >> > >> 1. The xvclk is fully controlled by ACPI powermanagement, so there > >> is no "xvclk" for the driver to get (since it is abstracted away). > >> In this case there will be a "clock-frequency" device property > >> to tell the driver the xvclk rate. > >> > >> 2. There is a xvclk modelled in the clk framework for the driver, > >> but the clk-generator may not be set to the right frequency > >> yet. In this case there will also be a "clock-frequency" device > >> property and the driver is expected to set the rate of the xvclk > >> through this frequency through the clk framework. > >> > >> Handle both these scenarios by switching to devm_clk_get_optional() > >> and checking for a "clock-frequency" device property. > >> > >> This is modelled after how the same issue was fixed for the ov8865 in > >> commit 73dcffeb2ff9 ("media: i2c: Support 19.2MHz input clock in ov8865"). > >> > >> Acked-by: Rui Miguel Silva <rmfrfs@gmail.com> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >> --- > >> drivers/media/i2c/ov2680.c | 26 ++++++++++++++++++++++++-- > >> 1 file changed, 24 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c > >> index b7c23286700e..a6a83f0e53f3 100644 > >> --- a/drivers/media/i2c/ov2680.c > >> +++ b/drivers/media/i2c/ov2680.c > >> @@ -698,6 +698,7 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) > >> { > >> struct device *dev = sensor->dev; > >> struct gpio_desc *gpio; > >> + unsigned int rate = 0; > >> int ret; > >> > >> /* > >> @@ -718,13 +719,34 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) > >> > >> sensor->pwdn_gpio = gpio; > >> > >> - sensor->xvclk = devm_clk_get(dev, "xvclk"); > >> + sensor->xvclk = devm_clk_get_optional(dev, "xvclk"); > >> if (IS_ERR(sensor->xvclk)) { > >> dev_err(dev, "xvclk clock missing or invalid\n"); > >> return PTR_ERR(sensor->xvclk); > >> } > >> > >> - sensor->xvclk_freq = clk_get_rate(sensor->xvclk); > >> + /* > >> + * We could have either a 24MHz or 19.2MHz clock rate from either DT or > >> + * ACPI... but we also need to support the weird IPU3 case which will > >> + * have an external clock AND a clock-frequency property. Check for the > > > > Where does this happen? This puts the driver in an awful situation. :-( > > This happens on IPU3 setups where the INT3472 device represents an actual > i2c attached sensor PMIC (rather then just some GPIOs) in this case > there is a clk generator inside the PMIC which is used and that is programmable, > so the driver needs to set the clk-rate. > > Note this patch is pretty much a 1:1 copy of the same patch for the ov8865 > and ov7251 drivers. > > I guess it might be good to start a discussion about doing this more > elegantly but that seems out of scope for this series. Works for me. Do you happen to know which systems use the clock generator feature of the PMIC? I guess it could be as simple as putting this to tps68470 platform data for the clock. And then hope no other PMICs will be used with this format.
Hi, On 7/31/23 15:35, Sakari Ailus wrote: > Hi Hans, > > On Mon, Jul 31, 2023 at 02:54:13PM +0200, Hans de Goede wrote: >> Hi, >> >> On 7/31/23 14:44, Sakari Ailus wrote: >>> Hi Hans, >>> >>> On Tue, Jun 27, 2023 at 03:18:15PM +0200, Hans de Goede wrote: >>>> On ACPI systems the following 2 scenarios are possible: >>>> >>>> 1. The xvclk is fully controlled by ACPI powermanagement, so there >>>> is no "xvclk" for the driver to get (since it is abstracted away). >>>> In this case there will be a "clock-frequency" device property >>>> to tell the driver the xvclk rate. >>>> >>>> 2. There is a xvclk modelled in the clk framework for the driver, >>>> but the clk-generator may not be set to the right frequency >>>> yet. In this case there will also be a "clock-frequency" device >>>> property and the driver is expected to set the rate of the xvclk >>>> through this frequency through the clk framework. >>>> >>>> Handle both these scenarios by switching to devm_clk_get_optional() >>>> and checking for a "clock-frequency" device property. >>>> >>>> This is modelled after how the same issue was fixed for the ov8865 in >>>> commit 73dcffeb2ff9 ("media: i2c: Support 19.2MHz input clock in ov8865"). >>>> >>>> Acked-by: Rui Miguel Silva <rmfrfs@gmail.com> >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>> --- >>>> drivers/media/i2c/ov2680.c | 26 ++++++++++++++++++++++++-- >>>> 1 file changed, 24 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c >>>> index b7c23286700e..a6a83f0e53f3 100644 >>>> --- a/drivers/media/i2c/ov2680.c >>>> +++ b/drivers/media/i2c/ov2680.c >>>> @@ -698,6 +698,7 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) >>>> { >>>> struct device *dev = sensor->dev; >>>> struct gpio_desc *gpio; >>>> + unsigned int rate = 0; >>>> int ret; >>>> >>>> /* >>>> @@ -718,13 +719,34 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) >>>> >>>> sensor->pwdn_gpio = gpio; >>>> >>>> - sensor->xvclk = devm_clk_get(dev, "xvclk"); >>>> + sensor->xvclk = devm_clk_get_optional(dev, "xvclk"); >>>> if (IS_ERR(sensor->xvclk)) { >>>> dev_err(dev, "xvclk clock missing or invalid\n"); >>>> return PTR_ERR(sensor->xvclk); >>>> } >>>> >>>> - sensor->xvclk_freq = clk_get_rate(sensor->xvclk); >>>> + /* >>>> + * We could have either a 24MHz or 19.2MHz clock rate from either DT or >>>> + * ACPI... but we also need to support the weird IPU3 case which will >>>> + * have an external clock AND a clock-frequency property. Check for the >>> >>> Where does this happen? This puts the driver in an awful situation. :-( >> >> This happens on IPU3 setups where the INT3472 device represents an actual >> i2c attached sensor PMIC (rather then just some GPIOs) in this case >> there is a clk generator inside the PMIC which is used and that is programmable, >> so the driver needs to set the clk-rate. >> >> Note this patch is pretty much a 1:1 copy of the same patch for the ov8865 >> and ov7251 drivers. >> >> I guess it might be good to start a discussion about doing this more >> elegantly but that seems out of scope for this series. > > Works for me. > > Do you happen to know which systems use the clock generator feature of the > PMIC? This is used at least on the Microsoft Surface Go devices most folks use for IPU3 development. and have the tps68470 > > I guess it could be as simple as putting this to tps68470 platform data for > the clock. And then hope no other PMICs will be used with this format. Right, after your email from earlier today I was actually thinking along the following lines to fix this: 1. There already is a struct tps68470_clk_platform_data which currently just contains the consumer dev_name() + con-id, we could extend this with an init_clk_rate member 2. Have the int3472 glue code fill init_clk_rate with info from the sensor's SSDB. This does require the int3472 code to make an extra SSDB() ACPI call. The ssdb struct definition has moved to include/media/ipu-bridge.h now, so that is already shared. 3. Make the tps68470 driver set the clk-rate to init_clk_rate if that is set to non 0 Then the clk_set_rate() call can be dropped from the drivers, note we do still need the other complexity with getting the clk + then getting the rate from the clk with a fallback to the property. AFAICT doing this in a follow up series seems quite doable. Regards, Hans
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c index b7c23286700e..a6a83f0e53f3 100644 --- a/drivers/media/i2c/ov2680.c +++ b/drivers/media/i2c/ov2680.c @@ -698,6 +698,7 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) { struct device *dev = sensor->dev; struct gpio_desc *gpio; + unsigned int rate = 0; int ret; /* @@ -718,13 +719,34 @@ static int ov2680_parse_dt(struct ov2680_dev *sensor) sensor->pwdn_gpio = gpio; - sensor->xvclk = devm_clk_get(dev, "xvclk"); + sensor->xvclk = devm_clk_get_optional(dev, "xvclk"); if (IS_ERR(sensor->xvclk)) { dev_err(dev, "xvclk clock missing or invalid\n"); return PTR_ERR(sensor->xvclk); } - sensor->xvclk_freq = clk_get_rate(sensor->xvclk); + /* + * We could have either a 24MHz or 19.2MHz clock rate from either DT or + * ACPI... but we also need to support the weird IPU3 case which will + * have an external clock AND a clock-frequency property. Check for the + * clock-frequency property and if found, set that rate if we managed + * to acquire a clock. This should cover the ACPI case. If the system + * uses devicetree then the configured rate should already be set, so + * we can just read it. + */ + ret = fwnode_property_read_u32(dev_fwnode(dev), "clock-frequency", + &rate); + if (ret && !sensor->xvclk) + return dev_err_probe(dev, ret, "invalid clock config\n"); + + if (!ret && sensor->xvclk) { + ret = clk_set_rate(sensor->xvclk, rate); + if (ret) + return dev_err_probe(dev, ret, + "failed to set clock rate\n"); + } + + sensor->xvclk_freq = rate ?: clk_get_rate(sensor->xvclk); if (sensor->xvclk_freq != OV2680_XVCLK_VALUE) { dev_err(dev, "wrong xvclk frequency %d HZ, expected: %d Hz\n", sensor->xvclk_freq, OV2680_XVCLK_VALUE);
On ACPI systems the following 2 scenarios are possible: 1. The xvclk is fully controlled by ACPI powermanagement, so there is no "xvclk" for the driver to get (since it is abstracted away). In this case there will be a "clock-frequency" device property to tell the driver the xvclk rate. 2. There is a xvclk modelled in the clk framework for the driver, but the clk-generator may not be set to the right frequency yet. In this case there will also be a "clock-frequency" device property and the driver is expected to set the rate of the xvclk through this frequency through the clk framework. Handle both these scenarios by switching to devm_clk_get_optional() and checking for a "clock-frequency" device property. This is modelled after how the same issue was fixed for the ov8865 in commit 73dcffeb2ff9 ("media: i2c: Support 19.2MHz input clock in ov8865"). Acked-by: Rui Miguel Silva <rmfrfs@gmail.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/media/i2c/ov2680.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)