| Message ID | 20250504101336.18748-3-hdegoede@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | media: mt9m114: Changes to make it work with atomisp devices | expand |
Hi Hans, On Sun, May 04, 2025 at 12:13:23PM +0200, Hans de Goede wrote: > Add support for platforms that do not have a clock provider, but instead > specify the clock frequency by using the "clock-frequency" property. > > E.g. ACPI platforms turn the clock on/off through ACPI power-resources > depending on the runtime-pm state, so there is no clock provider. https://lore.kernel.org/r/20250321130329.342236-1-mehdi.djait@linux.intel.com is a solution to this problem that will scale better. > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/media/i2c/mt9m114.c | 20 ++++++++++++++++---- > 1 file changed, 16 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c > index 5f0b0ad8f885..be1d2ec64b89 100644 > --- a/drivers/media/i2c/mt9m114.c > +++ b/drivers/media/i2c/mt9m114.c > @@ -377,6 +377,7 @@ struct mt9m114 { > struct gpio_desc *reset; > struct regulator_bulk_data supplies[3]; > struct v4l2_fwnode_endpoint bus_cfg; > + u32 clk_freq; > > struct { > unsigned int m; > @@ -2122,14 +2123,13 @@ static int mt9m114_power_on(struct mt9m114 *sensor) > > /* Perform a hard reset if available, or a soft reset otherwise. */ > if (sensor->reset) { > - long freq = clk_get_rate(sensor->clk); > unsigned int duration; > > /* > * The minimum duration is 50 clock cycles, thus typically > * around 2µs. Double it to be safe. > */ > - duration = DIV_ROUND_UP(2 * 50 * 1000000, freq); > + duration = DIV_ROUND_UP(2 * 50 * 1000000, sensor->clk_freq); > > gpiod_set_value(sensor->reset, 1); > fsleep(duration); > @@ -2249,7 +2249,7 @@ static int mt9m114_clk_init(struct mt9m114 *sensor) > * for 16-bit per pixel, transmitted in DDR over a single lane. For > * parallel mode, the sensor ouputs one pixel in two PIXCLK cycles. > */ > - sensor->pixrate = clk_get_rate(sensor->clk) * sensor->pll.m > + sensor->pixrate = sensor->clk_freq * sensor->pll.m > / ((sensor->pll.n + 1) * (sensor->pll.p + 1)); > > link_freq = sensor->bus_cfg.bus_type == V4L2_MBUS_CSI2_DPHY > @@ -2360,13 +2360,25 @@ static int mt9m114_probe(struct i2c_client *client) > return ret; > > /* Acquire clocks, GPIOs and regulators. */ > - sensor->clk = devm_clk_get(dev, NULL); > + sensor->clk = devm_clk_get_optional(dev, NULL); > if (IS_ERR(sensor->clk)) { > ret = PTR_ERR(sensor->clk); > dev_err_probe(dev, ret, "Failed to get clock\n"); > goto error_ep_free; > } > > + if (sensor->clk) { > + sensor->clk_freq = clk_get_rate(sensor->clk); > + } else { > + ret = fwnode_property_read_u32(dev_fwnode(dev), > + "clock-frequency", > + &sensor->clk_freq); > + if (ret) { > + dev_err_probe(dev, ret, "Failed to read clock-freq\n"); > + goto error_ep_free; > + } > + } > + > sensor->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); > if (IS_ERR(sensor->reset)) { > ret = PTR_ERR(sensor->reset);
Hi, On 9-May-25 8:51 PM, Laurent Pinchart wrote: > Hi Hans, > > On Sun, May 04, 2025 at 12:13:23PM +0200, Hans de Goede wrote: >> Add support for platforms that do not have a clock provider, but instead >> specify the clock frequency by using the "clock-frequency" property. >> >> E.g. ACPI platforms turn the clock on/off through ACPI power-resources >> depending on the runtime-pm state, so there is no clock provider. > > https://lore.kernel.org/r/20250321130329.342236-1-mehdi.djait@linux.intel.com > is a solution to this problem that will scale better. I agree. Depending on how the timing wrt upstreaming works out, we may still need this patch before merging 13/13 (adding of the actual ACPI hw-id table). I'll move this to be patch 12/13 for v2. So that we can at least merge 1-11 of v2 (assuming there will be no more review comments on v2). Laurent, if the patch you linked to is not ready yet would you then be ok with temporarily taking this patch? I'm happy to commit to moving this driver over to the new helper once the helper is upstream. Regards, Hans > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/media/i2c/mt9m114.c | 20 ++++++++++++++++---- >> 1 file changed, 16 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c >> index 5f0b0ad8f885..be1d2ec64b89 100644 >> --- a/drivers/media/i2c/mt9m114.c >> +++ b/drivers/media/i2c/mt9m114.c >> @@ -377,6 +377,7 @@ struct mt9m114 { >> struct gpio_desc *reset; >> struct regulator_bulk_data supplies[3]; >> struct v4l2_fwnode_endpoint bus_cfg; >> + u32 clk_freq; >> >> struct { >> unsigned int m; >> @@ -2122,14 +2123,13 @@ static int mt9m114_power_on(struct mt9m114 *sensor) >> >> /* Perform a hard reset if available, or a soft reset otherwise. */ >> if (sensor->reset) { >> - long freq = clk_get_rate(sensor->clk); >> unsigned int duration; >> >> /* >> * The minimum duration is 50 clock cycles, thus typically >> * around 2µs. Double it to be safe. >> */ >> - duration = DIV_ROUND_UP(2 * 50 * 1000000, freq); >> + duration = DIV_ROUND_UP(2 * 50 * 1000000, sensor->clk_freq); >> >> gpiod_set_value(sensor->reset, 1); >> fsleep(duration); >> @@ -2249,7 +2249,7 @@ static int mt9m114_clk_init(struct mt9m114 *sensor) >> * for 16-bit per pixel, transmitted in DDR over a single lane. For >> * parallel mode, the sensor ouputs one pixel in two PIXCLK cycles. >> */ >> - sensor->pixrate = clk_get_rate(sensor->clk) * sensor->pll.m >> + sensor->pixrate = sensor->clk_freq * sensor->pll.m >> / ((sensor->pll.n + 1) * (sensor->pll.p + 1)); >> >> link_freq = sensor->bus_cfg.bus_type == V4L2_MBUS_CSI2_DPHY >> @@ -2360,13 +2360,25 @@ static int mt9m114_probe(struct i2c_client *client) >> return ret; >> >> /* Acquire clocks, GPIOs and regulators. */ >> - sensor->clk = devm_clk_get(dev, NULL); >> + sensor->clk = devm_clk_get_optional(dev, NULL); >> if (IS_ERR(sensor->clk)) { >> ret = PTR_ERR(sensor->clk); >> dev_err_probe(dev, ret, "Failed to get clock\n"); >> goto error_ep_free; >> } >> >> + if (sensor->clk) { >> + sensor->clk_freq = clk_get_rate(sensor->clk); >> + } else { >> + ret = fwnode_property_read_u32(dev_fwnode(dev), >> + "clock-frequency", >> + &sensor->clk_freq); >> + if (ret) { >> + dev_err_probe(dev, ret, "Failed to read clock-freq\n"); >> + goto error_ep_free; >> + } >> + } >> + >> sensor->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); >> if (IS_ERR(sensor->reset)) { >> ret = PTR_ERR(sensor->reset); >
diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c index 5f0b0ad8f885..be1d2ec64b89 100644 --- a/drivers/media/i2c/mt9m114.c +++ b/drivers/media/i2c/mt9m114.c @@ -377,6 +377,7 @@ struct mt9m114 { struct gpio_desc *reset; struct regulator_bulk_data supplies[3]; struct v4l2_fwnode_endpoint bus_cfg; + u32 clk_freq; struct { unsigned int m; @@ -2122,14 +2123,13 @@ static int mt9m114_power_on(struct mt9m114 *sensor) /* Perform a hard reset if available, or a soft reset otherwise. */ if (sensor->reset) { - long freq = clk_get_rate(sensor->clk); unsigned int duration; /* * The minimum duration is 50 clock cycles, thus typically * around 2µs. Double it to be safe. */ - duration = DIV_ROUND_UP(2 * 50 * 1000000, freq); + duration = DIV_ROUND_UP(2 * 50 * 1000000, sensor->clk_freq); gpiod_set_value(sensor->reset, 1); fsleep(duration); @@ -2249,7 +2249,7 @@ static int mt9m114_clk_init(struct mt9m114 *sensor) * for 16-bit per pixel, transmitted in DDR over a single lane. For * parallel mode, the sensor ouputs one pixel in two PIXCLK cycles. */ - sensor->pixrate = clk_get_rate(sensor->clk) * sensor->pll.m + sensor->pixrate = sensor->clk_freq * sensor->pll.m / ((sensor->pll.n + 1) * (sensor->pll.p + 1)); link_freq = sensor->bus_cfg.bus_type == V4L2_MBUS_CSI2_DPHY @@ -2360,13 +2360,25 @@ static int mt9m114_probe(struct i2c_client *client) return ret; /* Acquire clocks, GPIOs and regulators. */ - sensor->clk = devm_clk_get(dev, NULL); + sensor->clk = devm_clk_get_optional(dev, NULL); if (IS_ERR(sensor->clk)) { ret = PTR_ERR(sensor->clk); dev_err_probe(dev, ret, "Failed to get clock\n"); goto error_ep_free; } + if (sensor->clk) { + sensor->clk_freq = clk_get_rate(sensor->clk); + } else { + ret = fwnode_property_read_u32(dev_fwnode(dev), + "clock-frequency", + &sensor->clk_freq); + if (ret) { + dev_err_probe(dev, ret, "Failed to read clock-freq\n"); + goto error_ep_free; + } + } + sensor->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); if (IS_ERR(sensor->reset)) { ret = PTR_ERR(sensor->reset);
Add support for platforms that do not have a clock provider, but instead specify the clock frequency by using the "clock-frequency" property. E.g. ACPI platforms turn the clock on/off through ACPI power-resources depending on the runtime-pm state, so there is no clock provider. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/media/i2c/mt9m114.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)