| Message ID | 20210112194919.50176-2-ezequiel@collabora.com |
|---|---|
| State | Accepted |
| Commit | a52e17361987782ecea7cc6ed0dc1f37a11949a8 |
| Headers | show |
| Series | Remove last users of v4l2-clk and remove v4l2-clk | expand |
Acked-by: Petr Cvek <petrcvekcz@gmail.com> Dne 12. 01. 21 v 20:49 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. > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > Acked-by: Arnd Bergmann <arnd@arndb.de> (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); > } > >
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? > > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com> > Acked-by: Arnd Bergmann <arnd@arndb.de> (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); > } > -- Kind regards, Sakari Ailus
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@collabora.com> > > Acked-by: Arnd Bergmann <arnd@arndb.de> (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); > > } > > >
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@collabora.com> > > > Acked-by: Arnd Bergmann <arnd@arndb.de> (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
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); }