Messages in this thread | | | Date | Tue, 7 Jun 2022 16:08:39 +0200 | Subject | Re: [PATCH v5 2/4] media: ov5675: add device-tree support and support runtime PM | From | Quentin Schulz <> |
| |
Hi Jacopo,
On 5/31/22 15:16, Jacopo Mondi wrote: > Hi Quentin, > one more question > > On Wed, May 25, 2022 at 04:58:31PM +0200, Quentin Schulz wrote: >> From: Quentin Schulz <quentin.schulz@theobroma-systems.com> >> >> Until now, this driver only supported ACPI. This adds support for >> Device Tree too while enabling clock and regulators in runtime PM. >> >> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com> >> --- >> >> v5: >> - fixed -Wdeclaration-after-statement for delay_us, >> >> v4: >> - added delays based on clock cycles as specified in datasheet for >> pre-power-off and post-power-on, >> - re-arranged clk handling, shutdown toggling and regulator handling to >> better match power up/down sequence defined in datasheet, >> - added comment on need for regulator being stable before releasing >> shutdown pin, >> >> v3: >> - added linux/mod_devicetable.h include, >> - moved delay for reset pulse right after the regulators are enabled, >> - removed check on is_acpi_node in favor of checks on presence of OF >> properties (e.g. devm_clk_get_optional returns NULL), >> - moved power management out of system suspend/resume into runtime PM >> callbacks, >> - removed ACPI specific comment since it's not specific to this driver, >> - changed devm_clk_get to devm_clk_get_optional, >> - remove OF use of clock-frequency (handled by devm_clk_get_optional >> directly), >> - removed name of clock (only one, so no need for anything explicit) >> when requesting a clock from OF, >> - wrapped lines to 80 chars, >> >> v2: >> - fixed unused-const-variable warning by removing of_match_ptr in >> of_match_table, reported by kernel test robot, >> >> drivers/media/i2c/ov5675.c | 149 +++++++++++++++++++++++++++++++------ >> 1 file changed, 128 insertions(+), 21 deletions(-) >> >> diff --git a/drivers/media/i2c/ov5675.c b/drivers/media/i2c/ov5675.c >> index 82ba9f56baec8..ea801edb8e408 100644 >> --- a/drivers/media/i2c/ov5675.c >> +++ b/drivers/media/i2c/ov5675.c >> @@ -3,10 +3,14 @@ >> >> #include <asm/unaligned.h> >> #include <linux/acpi.h> >> +#include <linux/clk.h> >> #include <linux/delay.h> >> +#include <linux/gpio/consumer.h> >> #include <linux/i2c.h> >> +#include <linux/mod_devicetable.h> >> #include <linux/module.h> >> #include <linux/pm_runtime.h> >> +#include <linux/regulator/consumer.h> >> #include <media/v4l2-ctrls.h> >> #include <media/v4l2-device.h> >> #include <media/v4l2-fwnode.h> >> @@ -17,7 +21,7 @@ >> >> #define OV5675_LINK_FREQ_450MHZ 450000000ULL >> #define OV5675_SCLK 90000000LL >> -#define OV5675_MCLK 19200000 >> +#define OV5675_XVCLK_19_2 19200000 >> #define OV5675_DATA_LANES 2 >> #define OV5675_RGB_DEPTH 10 >> >> @@ -76,6 +80,14 @@ >> >> #define to_ov5675(_sd) container_of(_sd, struct ov5675, sd) >> >> +static const char * const ov5675_supply_names[] = { >> + "avdd", /* Analog power */ >> + "dovdd", /* Digital I/O power */ >> + "dvdd", /* Digital core power */ >> +}; >> + >> +#define OV5675_NUM_SUPPLIES ARRAY_SIZE(ov5675_supply_names) >> + >> enum { >> OV5675_LINK_FREQ_900MBPS, >> }; >> @@ -484,6 +496,9 @@ struct ov5675 { >> struct v4l2_subdev sd; >> struct media_pad pad; >> struct v4l2_ctrl_handler ctrl_handler; >> + struct clk *xvclk; >> + struct gpio_desc *reset_gpio; >> + struct regulator_bulk_data supplies[OV5675_NUM_SUPPLIES]; >> >> /* V4L2 Controls */ >> struct v4l2_ctrl *link_freq; >> @@ -944,6 +959,56 @@ static int ov5675_set_stream(struct v4l2_subdev *sd, int enable) >> return ret; >> } >> >> +static int ov5675_power_off(struct device *dev) > > Does this (and power_on) require __maybe_unused to avoid a warning > when compiling without CONFIG_PM support ? Have you tried that ? >
It does not, because they are called manually in the probe function and its error path (but thanks for the heads up, because I definitely forgot about this one).
I received some review outside of this mailing list telling me the error path is incorrect for pm_runtime so I'll have a look at it before sending a v6.
Cheers, Quentin
| |