diff mbox series

[v2,1/6] media: mach-pxa: Register the camera sensor fixed-rate clock

Message ID 20210112194919.50176-2-ezequiel@collabora.com (mailing list archive)
State Accepted, archived
Delegated to: Sakari Ailus
Headers
Series Remove last users of v4l2-clk and remove v4l2-clk |

Commit Message

Ezequiel Garcia Jan. 12, 2021, 7:49 p.m. UTC 
  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(+)

Comments

Petr Cvek Jan. 14, 2021, 11:23 a.m. UTC | #1 
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);
>  }
>  
>
Sakari Ailus Jan. 18, 2021, 4:36 p.m. UTC | #2 
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);
>  }
>
Ezequiel Garcia Jan. 18, 2021, 8:21 p.m. UTC | #3 
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);
> >  }
> >  
>
Sakari Ailus Jan. 19, 2021, 8:01 a.m. UTC | #4 
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);
> > >  }
> > >  
> > 
> 
>
diff mbox series

Patch

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);
 }