mbox series

[0/8] media: ov08x40: Various improvements

Message ID 20241219134940.15472-1-hdegoede@redhat.com
Headers show
Series media: ov08x40: Various improvements | expand

Message

Hans de Goede Dec. 19, 2024, 1:49 p.m. UTC
Hi All,

Here is a new ov08x40 patch series replacing my previous series:
https://lore.kernel.org/linux-media/20241012115236.53998-1-hdegoede@redhat.com/

I have not marked this as a v2 series since after rebasing on top
of Bryan O'Donoghue changes this essentially is a whole new series.

The below series has been tested on a HP Spectre x360 14-eu0xxx laptop,
but unfortunately the sensor will still not power-on (identify() fails
with -121) after this series.

Still I believe that these are all good improvements to have.

Bryan, can you test this on your ov08x40 setup and check that it does
not cause any regressions there?

Regards,

Hans


Hans de Goede (8):
  media: ov08x40: Properly turn sensor on/off when runtime-suspended
  media: ov08x40: Move fwnode_graph_get_next_endpoint() call up
  media: ov08x40: Get reset GPIO and regulators on ACPI platforms too
  media: ov08x40: Get clock on ACPI platforms too
  media: ov08x40: Move ov08x40_identify_module() function up
  media: ov08x40: Improve ov08x40_identify_module() error logging
  media: ov08x40: Improve ov08x40_[read|write]_reg() error returns
  media: ov08x40: Add missing ov08x40_identify_module() call on
    stream-start

 drivers/media/i2c/ov08x40.c | 157 +++++++++++++++++++-----------------
 1 file changed, 82 insertions(+), 75 deletions(-)

Comments

Bryan O'Donoghue Dec. 20, 2024, 1:51 p.m. UTC | #1
On 20/12/2024 13:47, Hans de Goede wrote:
>>>    }
>>>    +static DEFINE_RUNTIME_DEV_PM_OPS(ov08x40_pm_ops, ov08x40_power_on,
>>> +                 ov08x40_power_off, NULL);
>>> +
> Ugh I have on/off swapped here, second argument of the macro is the suspend
> function so that should be ov08x40_power_off. IOW this should be:
> 
> static DEFINE_RUNTIME_DEV_PM_OPS(ov08x40_pm_ops, ov08x40_power_off,
> 				 ov08x40_power_on, NULL);
> 
> Can you give this a try with that change?
> 
> Regards,
> 
> Hans

Heh.

That'd do it, works now.

---
bod
Bryan O'Donoghue Dec. 20, 2024, 2:27 p.m. UTC | #2
On 20/12/2024 13:59, Hans de Goede wrote:
> I guess you are going to test the rest of the series and then provided
> a Tested-by?
> 
> If yes then I'll wait with posting v2 until you're done testing.

I can give you Tested-by: for the series now + the change we discussed.

---
bod
Bryan O'Donoghue Dec. 20, 2024, 2:31 p.m. UTC | #3
On 19/12/2024 13:49, Hans de Goede wrote:
> ov08x40_identify_module() already logs an error if the read ID mismatches,
> so having its caller also log an error results in 2 errors in this case.
> 
> Add error logging to the ID register read in ov08x40_identify_module() and
> drop the error logging in the caller.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Bryan O'Donoghue Dec. 20, 2024, 2:32 p.m. UTC | #4
On 19/12/2024 13:49, Hans de Goede wrote:
> The driver might skip the ov08x40_identify_module() on probe() based on
> the acpi_dev_state_d0() check done in probe().
> 
> If the ov08x40_identify_module() call is skipped on probe() it should
> be done on the first stream start. Add the missing call.
> 
> Note ov08x40_identify_module() will only do something on its first call,
> subsequent calls are no-ops.
> 
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>