On Mon, 2021-01-18 at 18:36 +0200, Sakari Ailus wrote: > Hi Ezequiel, > > Thanks for the patch. > > On Tue, Jan 12, 2021 at 04:49:14PM -0300, Ezequiel Garcia wrote: > > 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. > > Where is the clock generated? > > If it's the same device, shouldn't it be registered in the pxa_camera > driver? > Apparently, as Petr explained, the PXA camera controller can provide a clock. However, it seems to me this is not necesarily the only way to provide a clock to a sensor, is it? Moreover, doing the proper clock conversion in the PXA driver doesn't seem trivial and I don't have hardware to test it. I'd rather keep things simple, and just register a fixed-rate clock at mach-pxa level, which will be removed anyway once these non-DT platforms are finally converted to DT or dropped. This way is at least a tiny bit less ugly than the current dummy v4l2-clk. Thanks, Ezequiel > > > > Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx> > > Acked-by: Arnd Bergmann <arnd@xxxxxxxx> (for arch/arm/mach-*/) > > --- > > Quoting Arnd: > > """ > > If there are no objections to the change itself, please take it through > > the v4l2 git tree. > > """ > > > > 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); > > } > > >