Hi Petr,
Thanks a lot for reviewing and testing the series.
On Tue, 2021-01-05 at 17:41 +0100, Petr Cvek wrote:
>
> Dne 04. 01. 21 v 17:57 Ezequiel Garcia napsal(a):
> > The pxa-camera capture driver currently registers a v4l2-clk
> > clock, named "mclk", to represent the mt9m111 sensor clock.
> >
> > Register a proper fixed-rate clock using the generic clock framework,
> > which will allow to remove the v4l2-clk clock in the pxa-camera
> > driver in a follow-up commit.
> >
>
> BTW the mclk output to a sensor is actually a variable rate, divided from lcdclk (which can be changed too). PXA camera driver is using variable
> pcdev->mclk_divisor to generate the mclk from lcdclk.
>
Hm, now that I look at this, I see the pxa-camera driver
is requiring a clock:
pcdev->clk = devm_clk_get(&pdev->dev, NULL);
if (IS_ERR(pcdev->clk))
return PTR_ERR(pcdev->clk);
Where is this clock registered in the non-devicetree case?
> The rate change is done in pxa_camera_activate():
>
> https://elixir.bootlin.com/linux/v5.11-rc2/source/drivers/media/platform/pxa_camera.c#L1136
>
> __raw_writel(pcdev->mclk_divisor | cicr4, pcdev->base + CICR4);
>
> Would it be possible to register a correct clock type with possibility to change the divisor by the standard way?
>
Right, so you mean the pxa-camera driver is the one providing the clock for the sensors?
In that case, I guess the pxa-camera driver should be the one registering
a CCF clock. Other drivers are doing this, through clk_register for instance.
However, for the sake of this series, which is meant to get rid
of the v4l2-clk API, I would say it's fine to just register a fixed-rate.
This is similar to what v4l2_clk_register was doing, which was registering
a dummy clock.
Having said that, as I mentioned above, I'm wondering if the mach-pxa
boards are really working, given I'm not seeing the clock for pxa-camera.
Maybe the best way forward is to just accept that pxa-camera
is only supported for the device tree platforms, and therefore drop the
support from mach-pxa/ boards.
Thanks,
Ezequiel
> Petr
>
>
> > Cc: Arnd Bergmann <arnd@xxxxxxxx>
> > Cc: Robert Jarzmik <robert.jarzmik@xxxxxxx>
> > Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> > ---
> > arch/arm/mach-pxa/devices.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/arch/arm/mach-pxa/devices.c b/arch/arm/mach-pxa/devices.c
> > index 524d6093e0c7..09b8495f3fd9 100644
> > --- a/arch/arm/mach-pxa/devices.c
> > +++ b/arch/arm/mach-pxa/devices.c
> > @@ -4,6 +4,7 @@
> > #include <linux/init.h>
> > #include <linux/platform_device.h>
> > #include <linux/clkdev.h>
> > +#include <linux/clk-provider.h>
> > #include <linux/dma-mapping.h>
> > #include <linux/dmaengine.h>
> > #include <linux/spi/pxa2xx_spi.h>
> > @@ -634,6 +635,13 @@ static struct platform_device pxa27x_device_camera = {
> >
> > void __init pxa_set_camera_info(struct pxacamera_platform_data *info)
> > {
> > + struct clk *mclk;
> > +
> > + /* Register a fixed-rate clock for camera sensors. */
> > + mclk = clk_register_fixed_rate(NULL, "pxa_camera_clk", NULL, 0,
> > + info->mclk_10khz * 10000);
> > + if (!IS_ERR(mclk))
> > + clkdev_create(mclk, "mclk", NULL);
> > pxa_register_device(&pxa27x_device_camera, info);
> > }
> >
> >
