Hi Ezequiel, On Mon, Jan 18, 2021 at 05:21:12PM -0300, Ezequiel Garcia wrote: > 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? It isn't. But that's what the clock framework is for. > > Moreover, doing the proper clock conversion in the PXA driver > doesn't seem trivial and I don't have hardware to test it. I guess it's possible to do that later, too. The platform appears to rely still on platform data, too. This set is still a major improvement, to get rid of v4l2-clk. > > 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); > > > } > > > > > > > -- Sakari Ailus