* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-11 14:35 ` Arnd Bergmann
0 siblings, 0 replies; 45+ messages in thread
From: Arnd Bergmann @ 2014-06-11 14:35 UTC (permalink / raw)
To: gregkh
Cc: Tony Lindgren, Mauro Carvalho Chehab, Greg Kroah-Hartman,
Laurent Pinchart, linux-omap, linux-media, linux-arm-kernel, arm
The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
of which can be loadable modules. This causes build failures
if we want the camera driver to be built-in.
This can be solved by turning the option into "tristate",
which unfortunately causes another problem, because the
driver incorrectly calls a platform-internal interface
for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
To work around that, we can export those symbols, but
that isn't really the correct solution, as we should not
have dependencies on platform code this way.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This is one of just two patches we currently need to get
'make allmodconfig' to build again on ARM.
diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 751f354..05d2d98 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
{
return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
}
+EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
void omap4_ctrl_pad_writel(u32 val, u16 offset)
{
writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
}
+EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
#ifdef CONFIG_ARCH_OMAP3
diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
index 78b0fba..0c3e3c1 100644
--- a/drivers/staging/media/omap4iss/Kconfig
+++ b/drivers/staging/media/omap4iss/Kconfig
@@ -1,5 +1,5 @@
config VIDEO_OMAP4
- bool "OMAP 4 Camera support"
+ tristate "OMAP 4 Camera support"
depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
select VIDEOBUF2_DMA_CONTIG
---help---
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-11 14:35 ` Arnd Bergmann
0 siblings, 0 replies; 45+ messages in thread
From: Arnd Bergmann @ 2014-06-11 14:35 UTC (permalink / raw)
Cc: Tony Lindgren, Mauro Carvalho Chehab, Greg Kroah-Hartman,
Laurent Pinchart, linux-omap, linux-media, linux-arm-kernel, arm
The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
of which can be loadable modules. This causes build failures
if we want the camera driver to be built-in.
This can be solved by turning the option into "tristate",
which unfortunately causes another problem, because the
driver incorrectly calls a platform-internal interface
for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
To work around that, we can export those symbols, but
that isn't really the correct solution, as we should not
have dependencies on platform code this way.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This is one of just two patches we currently need to get
'make allmodconfig' to build again on ARM.
diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 751f354..05d2d98 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
{
return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
}
+EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
void omap4_ctrl_pad_writel(u32 val, u16 offset)
{
writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
}
+EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
#ifdef CONFIG_ARCH_OMAP3
diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
index 78b0fba..0c3e3c1 100644
--- a/drivers/staging/media/omap4iss/Kconfig
+++ b/drivers/staging/media/omap4iss/Kconfig
@@ -1,5 +1,5 @@
config VIDEO_OMAP4
- bool "OMAP 4 Camera support"
+ tristate "OMAP 4 Camera support"
depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
select VIDEOBUF2_DMA_CONTIG
---help---
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-11 14:35 ` Arnd Bergmann
0 siblings, 0 replies; 45+ messages in thread
From: Arnd Bergmann @ 2014-06-11 14:35 UTC (permalink / raw)
To: linux-arm-kernel
The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
of which can be loadable modules. This causes build failures
if we want the camera driver to be built-in.
This can be solved by turning the option into "tristate",
which unfortunately causes another problem, because the
driver incorrectly calls a platform-internal interface
for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
To work around that, we can export those symbols, but
that isn't really the correct solution, as we should not
have dependencies on platform code this way.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This is one of just two patches we currently need to get
'make allmodconfig' to build again on ARM.
diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
index 751f354..05d2d98 100644
--- a/arch/arm/mach-omap2/control.c
+++ b/arch/arm/mach-omap2/control.c
@@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
{
return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
}
+EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
void omap4_ctrl_pad_writel(u32 val, u16 offset)
{
writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
}
+EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
#ifdef CONFIG_ARCH_OMAP3
diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
index 78b0fba..0c3e3c1 100644
--- a/drivers/staging/media/omap4iss/Kconfig
+++ b/drivers/staging/media/omap4iss/Kconfig
@@ -1,5 +1,5 @@
config VIDEO_OMAP4
- bool "OMAP 4 Camera support"
+ tristate "OMAP 4 Camera support"
depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
select VIDEOBUF2_DMA_CONTIG
---help---
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-11 14:35 ` Arnd Bergmann
(?)
@ 2014-06-11 14:42 ` Nishanth Menon
-1 siblings, 0 replies; 45+ messages in thread
From: Nishanth Menon @ 2014-06-11 14:42 UTC (permalink / raw)
To: Arnd Bergmann, gregkh
Cc: Tony Lindgren, Mauro Carvalho Chehab, Laurent Pinchart,
linux-omap, linux-media, linux-arm-kernel, arm, Laurent Pinchart
On 06/11/2014 09:35 AM, Arnd Bergmann wrote:
> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> of which can be loadable modules. This causes build failures
> if we want the camera driver to be built-in.
>
> This can be solved by turning the option into "tristate",
> which unfortunately causes another problem, because the
> driver incorrectly calls a platform-internal interface
> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> To work around that, we can export those symbols, but
> that isn't really the correct solution, as we should not
> have dependencies on platform code this way.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> This is one of just two patches we currently need to get
> 'make allmodconfig' to build again on ARM.
>
> diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> index 751f354..05d2d98 100644
> --- a/arch/arm/mach-omap2/control.c
> +++ b/arch/arm/mach-omap2/control.c
> @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
> {
> return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
> }
> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
>
> void omap4_ctrl_pad_writel(u32 val, u16 offset)
> {
> writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
> }
> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
>
> #ifdef CONFIG_ARCH_OMAP3
>
> diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
> index 78b0fba..0c3e3c1 100644
> --- a/drivers/staging/media/omap4iss/Kconfig
> +++ b/drivers/staging/media/omap4iss/Kconfig
> @@ -1,5 +1,5 @@
> config VIDEO_OMAP4
> - bool "OMAP 4 Camera support"
> + tristate "OMAP 4 Camera support"
> depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> select VIDEOBUF2_DMA_CONTIG
> ---help---
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
This was discussed in detail here:
http://marc.info/?t=140198692500001&r=1&w=2
Direct dependency from a staging driver to mach-omap2 driver is not
something we'd like, right?
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-11 14:42 ` Nishanth Menon
0 siblings, 0 replies; 45+ messages in thread
From: Nishanth Menon @ 2014-06-11 14:42 UTC (permalink / raw)
To: Arnd Bergmann, gregkh
Cc: Tony Lindgren, Mauro Carvalho Chehab, Laurent Pinchart,
linux-omap, linux-media, linux-arm-kernel, arm
On 06/11/2014 09:35 AM, Arnd Bergmann wrote:
> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> of which can be loadable modules. This causes build failures
> if we want the camera driver to be built-in.
>
> This can be solved by turning the option into "tristate",
> which unfortunately causes another problem, because the
> driver incorrectly calls a platform-internal interface
> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> To work around that, we can export those symbols, but
> that isn't really the correct solution, as we should not
> have dependencies on platform code this way.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> This is one of just two patches we currently need to get
> 'make allmodconfig' to build again on ARM.
>
> diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> index 751f354..05d2d98 100644
> --- a/arch/arm/mach-omap2/control.c
> +++ b/arch/arm/mach-omap2/control.c
> @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
> {
> return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
> }
> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
>
> void omap4_ctrl_pad_writel(u32 val, u16 offset)
> {
> writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
> }
> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
>
> #ifdef CONFIG_ARCH_OMAP3
>
> diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
> index 78b0fba..0c3e3c1 100644
> --- a/drivers/staging/media/omap4iss/Kconfig
> +++ b/drivers/staging/media/omap4iss/Kconfig
> @@ -1,5 +1,5 @@
> config VIDEO_OMAP4
> - bool "OMAP 4 Camera support"
> + tristate "OMAP 4 Camera support"
> depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> select VIDEOBUF2_DMA_CONTIG
> ---help---
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
This was discussed in detail here:
http://marc.info/?t=140198692500001&r=1&w=2
Direct dependency from a staging driver to mach-omap2 driver is not
something we'd like, right?
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-11 14:42 ` Nishanth Menon
0 siblings, 0 replies; 45+ messages in thread
From: Nishanth Menon @ 2014-06-11 14:42 UTC (permalink / raw)
To: linux-arm-kernel
On 06/11/2014 09:35 AM, Arnd Bergmann wrote:
> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> of which can be loadable modules. This causes build failures
> if we want the camera driver to be built-in.
>
> This can be solved by turning the option into "tristate",
> which unfortunately causes another problem, because the
> driver incorrectly calls a platform-internal interface
> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> To work around that, we can export those symbols, but
> that isn't really the correct solution, as we should not
> have dependencies on platform code this way.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> This is one of just two patches we currently need to get
> 'make allmodconfig' to build again on ARM.
>
> diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> index 751f354..05d2d98 100644
> --- a/arch/arm/mach-omap2/control.c
> +++ b/arch/arm/mach-omap2/control.c
> @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
> {
> return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
> }
> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
>
> void omap4_ctrl_pad_writel(u32 val, u16 offset)
> {
> writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
> }
> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
>
> #ifdef CONFIG_ARCH_OMAP3
>
> diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
> index 78b0fba..0c3e3c1 100644
> --- a/drivers/staging/media/omap4iss/Kconfig
> +++ b/drivers/staging/media/omap4iss/Kconfig
> @@ -1,5 +1,5 @@
> config VIDEO_OMAP4
> - bool "OMAP 4 Camera support"
> + tristate "OMAP 4 Camera support"
> depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> select VIDEOBUF2_DMA_CONTIG
> ---help---
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
This was discussed in detail here:
http://marc.info/?t=140198692500001&r=1&w=2
Direct dependency from a staging driver to mach-omap2 driver is not
something we'd like, right?
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-11 14:35 ` Arnd Bergmann
@ 2014-06-11 14:47 ` Tony Lindgren
-1 siblings, 0 replies; 45+ messages in thread
From: Tony Lindgren @ 2014-06-11 14:47 UTC (permalink / raw)
To: Arnd Bergmann
Cc: gregkh, Mauro Carvalho Chehab, Laurent Pinchart, linux-omap,
linux-media, linux-arm-kernel, arm
* Arnd Bergmann <arnd@arndb.de> [140611 07:37]:
> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> of which can be loadable modules. This causes build failures
> if we want the camera driver to be built-in.
That's good news, but let's not fix it this way.
> This can be solved by turning the option into "tristate",
> which unfortunately causes another problem, because the
> driver incorrectly calls a platform-internal interface
> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> To work around that, we can export those symbols, but
> that isn't really the correct solution, as we should not
> have dependencies on platform code this way.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> This is one of just two patches we currently need to get
> 'make allmodconfig' to build again on ARM.
>
> diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> index 751f354..05d2d98 100644
> --- a/arch/arm/mach-omap2/control.c
> +++ b/arch/arm/mach-omap2/control.c
> @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
> {
> return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
> }
> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
>
> void omap4_ctrl_pad_writel(u32 val, u16 offset)
> {
> writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
> }
> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
>
> #ifdef CONFIG_ARCH_OMAP3
Exporting these will likely cause immediate misuse in other
drivers all over the place.
These should just use either pinctrl-single.c instead for muxing.
Or if they are not mux registers, we do have the syscon mapping
available in omap4.dtsi that pbias-regulator.c is already using.
Laurent, got any better ideas?
Regards,
Tony
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-11 14:47 ` Tony Lindgren
0 siblings, 0 replies; 45+ messages in thread
From: Tony Lindgren @ 2014-06-11 14:47 UTC (permalink / raw)
To: linux-arm-kernel
* Arnd Bergmann <arnd@arndb.de> [140611 07:37]:
> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> of which can be loadable modules. This causes build failures
> if we want the camera driver to be built-in.
That's good news, but let's not fix it this way.
> This can be solved by turning the option into "tristate",
> which unfortunately causes another problem, because the
> driver incorrectly calls a platform-internal interface
> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> To work around that, we can export those symbols, but
> that isn't really the correct solution, as we should not
> have dependencies on platform code this way.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> This is one of just two patches we currently need to get
> 'make allmodconfig' to build again on ARM.
>
> diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> index 751f354..05d2d98 100644
> --- a/arch/arm/mach-omap2/control.c
> +++ b/arch/arm/mach-omap2/control.c
> @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
> {
> return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
> }
> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
>
> void omap4_ctrl_pad_writel(u32 val, u16 offset)
> {
> writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
> }
> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
>
> #ifdef CONFIG_ARCH_OMAP3
Exporting these will likely cause immediate misuse in other
drivers all over the place.
These should just use either pinctrl-single.c instead for muxing.
Or if they are not mux registers, we do have the syscon mapping
available in omap4.dtsi that pbias-regulator.c is already using.
Laurent, got any better ideas?
Regards,
Tony
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-11 14:42 ` Nishanth Menon
@ 2014-06-11 14:49 ` Arnd Bergmann
-1 siblings, 0 replies; 45+ messages in thread
From: Arnd Bergmann @ 2014-06-11 14:49 UTC (permalink / raw)
To: Nishanth Menon
Cc: gregkh, Tony Lindgren, Mauro Carvalho Chehab, Laurent Pinchart,
linux-omap, linux-media, linux-arm-kernel, arm
On Wednesday 11 June 2014 09:42:04 Nishanth Menon wrote:
> On 06/11/2014 09:35 AM, Arnd Bergmann wrote:
> > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > of which can be loadable modules. This causes build failures
> > if we want the camera driver to be built-in.
> >
> > This can be solved by turning the option into "tristate",
> > which unfortunately causes another problem, because the
> > driver incorrectly calls a platform-internal interface
> > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > To work around that, we can export those symbols, but
> > that isn't really the correct solution, as we should not
> > have dependencies on platform code this way.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > This is one of just two patches we currently need to get
> > 'make allmodconfig' to build again on ARM.
> >
> > diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> > index 751f354..05d2d98 100644
> > --- a/arch/arm/mach-omap2/control.c
> > +++ b/arch/arm/mach-omap2/control.c
> > @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
> > {
> > return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
> > }
> > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
> >
> > void omap4_ctrl_pad_writel(u32 val, u16 offset)
> > {
> > writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
> > }
> > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
> >
> > #ifdef CONFIG_ARCH_OMAP3
> >
> > diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
> > index 78b0fba..0c3e3c1 100644
> > --- a/drivers/staging/media/omap4iss/Kconfig
> > +++ b/drivers/staging/media/omap4iss/Kconfig
> > @@ -1,5 +1,5 @@
> > config VIDEO_OMAP4
> > - bool "OMAP 4 Camera support"
> > + tristate "OMAP 4 Camera support"
> > depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> > select VIDEOBUF2_DMA_CONTIG
> > ---help---
> >
>
> This was discussed in detail here:
> http://marc.info/?t=140198692500001&r=1&w=2
> Direct dependency from a staging driver to mach-omap2 driver is not
> something we'd like, right?
So it was decided to just leave ARM allmodconfig broken?
Why not at least do this instead?
8<----
>From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 21 Jan 2014 09:32:43 +0100
Subject: [PATCH] [media] staging: tighten omap4iss dependencies
The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
of which can be loadable modules. This causes build failures
if we want the camera driver to be built-in.
This can be solved by turning the option into "tristate",
which unfortunately causes another problem, because the
driver incorrectly calls a platform-internal interface
for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
Instead, this patch just forbids the invalid configurations
and ensures that the driver can only be built if all its
dependencies are built-in.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
index 78b0fba..8afc6fe 100644
--- a/drivers/staging/media/omap4iss/Kconfig
+++ b/drivers/staging/media/omap4iss/Kconfig
@@ -1,6 +1,6 @@
config VIDEO_OMAP4
bool "OMAP 4 Camera support"
- depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
+ depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y && ARCH_OMAP4
select VIDEOBUF2_DMA_CONTIG
---help---
Driver for an OMAP 4 ISS controller.
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-11 14:49 ` Arnd Bergmann
0 siblings, 0 replies; 45+ messages in thread
From: Arnd Bergmann @ 2014-06-11 14:49 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 11 June 2014 09:42:04 Nishanth Menon wrote:
> On 06/11/2014 09:35 AM, Arnd Bergmann wrote:
> > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > of which can be loadable modules. This causes build failures
> > if we want the camera driver to be built-in.
> >
> > This can be solved by turning the option into "tristate",
> > which unfortunately causes another problem, because the
> > driver incorrectly calls a platform-internal interface
> > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > To work around that, we can export those symbols, but
> > that isn't really the correct solution, as we should not
> > have dependencies on platform code this way.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > This is one of just two patches we currently need to get
> > 'make allmodconfig' to build again on ARM.
> >
> > diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> > index 751f354..05d2d98 100644
> > --- a/arch/arm/mach-omap2/control.c
> > +++ b/arch/arm/mach-omap2/control.c
> > @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
> > {
> > return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
> > }
> > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
> >
> > void omap4_ctrl_pad_writel(u32 val, u16 offset)
> > {
> > writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
> > }
> > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
> >
> > #ifdef CONFIG_ARCH_OMAP3
> >
> > diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
> > index 78b0fba..0c3e3c1 100644
> > --- a/drivers/staging/media/omap4iss/Kconfig
> > +++ b/drivers/staging/media/omap4iss/Kconfig
> > @@ -1,5 +1,5 @@
> > config VIDEO_OMAP4
> > - bool "OMAP 4 Camera support"
> > + tristate "OMAP 4 Camera support"
> > depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> > select VIDEOBUF2_DMA_CONTIG
> > ---help---
> >
>
> This was discussed in detail here:
> http://marc.info/?t=140198692500001&r=1&w=2
> Direct dependency from a staging driver to mach-omap2 driver is not
> something we'd like, right?
So it was decided to just leave ARM allmodconfig broken?
Why not at least do this instead?
8<----
>From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 21 Jan 2014 09:32:43 +0100
Subject: [PATCH] [media] staging: tighten omap4iss dependencies
The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
of which can be loadable modules. This causes build failures
if we want the camera driver to be built-in.
This can be solved by turning the option into "tristate",
which unfortunately causes another problem, because the
driver incorrectly calls a platform-internal interface
for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
Instead, this patch just forbids the invalid configurations
and ensures that the driver can only be built if all its
dependencies are built-in.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
index 78b0fba..8afc6fe 100644
--- a/drivers/staging/media/omap4iss/Kconfig
+++ b/drivers/staging/media/omap4iss/Kconfig
@@ -1,6 +1,6 @@
config VIDEO_OMAP4
bool "OMAP 4 Camera support"
- depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
+ depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y && ARCH_OMAP4
select VIDEOBUF2_DMA_CONTIG
---help---
Driver for an OMAP 4 ISS controller.
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-11 14:49 ` Arnd Bergmann
(?)
@ 2014-06-11 14:53 ` Nishanth Menon
-1 siblings, 0 replies; 45+ messages in thread
From: Nishanth Menon @ 2014-06-11 14:53 UTC (permalink / raw)
To: Arnd Bergmann
Cc: gregkh, Tony Lindgren, Mauro Carvalho Chehab, Laurent Pinchart,
linux-omap, linux-media, linux-arm-kernel, arm
On 06/11/2014 09:49 AM, Arnd Bergmann wrote:
> On Wednesday 11 June 2014 09:42:04 Nishanth Menon wrote:
>> On 06/11/2014 09:35 AM, Arnd Bergmann wrote:
>>> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
>>> of which can be loadable modules. This causes build failures
>>> if we want the camera driver to be built-in.
>>>
>>> This can be solved by turning the option into "tristate",
>>> which unfortunately causes another problem, because the
>>> driver incorrectly calls a platform-internal interface
>>> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
>>> To work around that, we can export those symbols, but
>>> that isn't really the correct solution, as we should not
>>> have dependencies on platform code this way.
>>>
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>> ---
>>> This is one of just two patches we currently need to get
>>> 'make allmodconfig' to build again on ARM.
>>>
>>> diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
>>> index 751f354..05d2d98 100644
>>> --- a/arch/arm/mach-omap2/control.c
>>> +++ b/arch/arm/mach-omap2/control.c
>>> @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
>>> {
>>> return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
>>> }
>>> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
>>>
>>> void omap4_ctrl_pad_writel(u32 val, u16 offset)
>>> {
>>> writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
>>> }
>>> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
>>>
>>> #ifdef CONFIG_ARCH_OMAP3
>>>
>>> diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
>>> index 78b0fba..0c3e3c1 100644
>>> --- a/drivers/staging/media/omap4iss/Kconfig
>>> +++ b/drivers/staging/media/omap4iss/Kconfig
>>> @@ -1,5 +1,5 @@
>>> config VIDEO_OMAP4
>>> - bool "OMAP 4 Camera support"
>>> + tristate "OMAP 4 Camera support"
>>> depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
>>> select VIDEOBUF2_DMA_CONTIG
>>> ---help---
>>>
>>
>> This was discussed in detail here:
>> http://marc.info/?t=140198692500001&r=1&w=2
>> Direct dependency from a staging driver to mach-omap2 driver is not
>> something we'd like, right?
>
> So it was decided to just leave ARM allmodconfig broken?
>
> Why not at least do this instead?
>
> 8<----
> From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, 21 Jan 2014 09:32:43 +0100
> Subject: [PATCH] [media] staging: tighten omap4iss dependencies
>
> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> of which can be loadable modules. This causes build failures
> if we want the camera driver to be built-in.
>
> This can be solved by turning the option into "tristate",
> which unfortunately causes another problem, because the
> driver incorrectly calls a platform-internal interface
> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
>
> Instead, this patch just forbids the invalid configurations
> and ensures that the driver can only be built if all its
> dependencies are built-in.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
> index 78b0fba..8afc6fe 100644
> --- a/drivers/staging/media/omap4iss/Kconfig
> +++ b/drivers/staging/media/omap4iss/Kconfig
> @@ -1,6 +1,6 @@
> config VIDEO_OMAP4
> bool "OMAP 4 Camera support"
> - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> + depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y && ARCH_OMAP4
> select VIDEOBUF2_DMA_CONTIG
> ---help---
> Driver for an OMAP 4 ISS controller.
>
I am ok with this if Tony and Laurent have no issues. Considering that
Laurent was working on coverting iss driver to dt, the detailed
discussion could take place at that point in time.
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-11 14:53 ` Nishanth Menon
0 siblings, 0 replies; 45+ messages in thread
From: Nishanth Menon @ 2014-06-11 14:53 UTC (permalink / raw)
To: Arnd Bergmann
Cc: gregkh, Tony Lindgren, Mauro Carvalho Chehab, Laurent Pinchart,
linux-omap, linux-media, linux-arm-kernel, arm
On 06/11/2014 09:49 AM, Arnd Bergmann wrote:
> On Wednesday 11 June 2014 09:42:04 Nishanth Menon wrote:
>> On 06/11/2014 09:35 AM, Arnd Bergmann wrote:
>>> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
>>> of which can be loadable modules. This causes build failures
>>> if we want the camera driver to be built-in.
>>>
>>> This can be solved by turning the option into "tristate",
>>> which unfortunately causes another problem, because the
>>> driver incorrectly calls a platform-internal interface
>>> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
>>> To work around that, we can export those symbols, but
>>> that isn't really the correct solution, as we should not
>>> have dependencies on platform code this way.
>>>
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>> ---
>>> This is one of just two patches we currently need to get
>>> 'make allmodconfig' to build again on ARM.
>>>
>>> diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
>>> index 751f354..05d2d98 100644
>>> --- a/arch/arm/mach-omap2/control.c
>>> +++ b/arch/arm/mach-omap2/control.c
>>> @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
>>> {
>>> return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
>>> }
>>> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
>>>
>>> void omap4_ctrl_pad_writel(u32 val, u16 offset)
>>> {
>>> writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
>>> }
>>> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
>>>
>>> #ifdef CONFIG_ARCH_OMAP3
>>>
>>> diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
>>> index 78b0fba..0c3e3c1 100644
>>> --- a/drivers/staging/media/omap4iss/Kconfig
>>> +++ b/drivers/staging/media/omap4iss/Kconfig
>>> @@ -1,5 +1,5 @@
>>> config VIDEO_OMAP4
>>> - bool "OMAP 4 Camera support"
>>> + tristate "OMAP 4 Camera support"
>>> depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
>>> select VIDEOBUF2_DMA_CONTIG
>>> ---help---
>>>
>>
>> This was discussed in detail here:
>> http://marc.info/?t=140198692500001&r=1&w=2
>> Direct dependency from a staging driver to mach-omap2 driver is not
>> something we'd like, right?
>
> So it was decided to just leave ARM allmodconfig broken?
>
> Why not at least do this instead?
>
> 8<----
> From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, 21 Jan 2014 09:32:43 +0100
> Subject: [PATCH] [media] staging: tighten omap4iss dependencies
>
> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> of which can be loadable modules. This causes build failures
> if we want the camera driver to be built-in.
>
> This can be solved by turning the option into "tristate",
> which unfortunately causes another problem, because the
> driver incorrectly calls a platform-internal interface
> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
>
> Instead, this patch just forbids the invalid configurations
> and ensures that the driver can only be built if all its
> dependencies are built-in.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
> index 78b0fba..8afc6fe 100644
> --- a/drivers/staging/media/omap4iss/Kconfig
> +++ b/drivers/staging/media/omap4iss/Kconfig
> @@ -1,6 +1,6 @@
> config VIDEO_OMAP4
> bool "OMAP 4 Camera support"
> - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> + depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y && ARCH_OMAP4
> select VIDEOBUF2_DMA_CONTIG
> ---help---
> Driver for an OMAP 4 ISS controller.
>
I am ok with this if Tony and Laurent have no issues. Considering that
Laurent was working on coverting iss driver to dt, the detailed
discussion could take place at that point in time.
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-11 14:53 ` Nishanth Menon
0 siblings, 0 replies; 45+ messages in thread
From: Nishanth Menon @ 2014-06-11 14:53 UTC (permalink / raw)
To: linux-arm-kernel
On 06/11/2014 09:49 AM, Arnd Bergmann wrote:
> On Wednesday 11 June 2014 09:42:04 Nishanth Menon wrote:
>> On 06/11/2014 09:35 AM, Arnd Bergmann wrote:
>>> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
>>> of which can be loadable modules. This causes build failures
>>> if we want the camera driver to be built-in.
>>>
>>> This can be solved by turning the option into "tristate",
>>> which unfortunately causes another problem, because the
>>> driver incorrectly calls a platform-internal interface
>>> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
>>> To work around that, we can export those symbols, but
>>> that isn't really the correct solution, as we should not
>>> have dependencies on platform code this way.
>>>
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>> ---
>>> This is one of just two patches we currently need to get
>>> 'make allmodconfig' to build again on ARM.
>>>
>>> diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
>>> index 751f354..05d2d98 100644
>>> --- a/arch/arm/mach-omap2/control.c
>>> +++ b/arch/arm/mach-omap2/control.c
>>> @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
>>> {
>>> return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
>>> }
>>> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
>>>
>>> void omap4_ctrl_pad_writel(u32 val, u16 offset)
>>> {
>>> writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
>>> }
>>> +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
>>>
>>> #ifdef CONFIG_ARCH_OMAP3
>>>
>>> diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
>>> index 78b0fba..0c3e3c1 100644
>>> --- a/drivers/staging/media/omap4iss/Kconfig
>>> +++ b/drivers/staging/media/omap4iss/Kconfig
>>> @@ -1,5 +1,5 @@
>>> config VIDEO_OMAP4
>>> - bool "OMAP 4 Camera support"
>>> + tristate "OMAP 4 Camera support"
>>> depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
>>> select VIDEOBUF2_DMA_CONTIG
>>> ---help---
>>>
>>
>> This was discussed in detail here:
>> http://marc.info/?t=140198692500001&r=1&w=2
>> Direct dependency from a staging driver to mach-omap2 driver is not
>> something we'd like, right?
>
> So it was decided to just leave ARM allmodconfig broken?
>
> Why not at least do this instead?
>
> 8<----
> From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, 21 Jan 2014 09:32:43 +0100
> Subject: [PATCH] [media] staging: tighten omap4iss dependencies
>
> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> of which can be loadable modules. This causes build failures
> if we want the camera driver to be built-in.
>
> This can be solved by turning the option into "tristate",
> which unfortunately causes another problem, because the
> driver incorrectly calls a platform-internal interface
> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
>
> Instead, this patch just forbids the invalid configurations
> and ensures that the driver can only be built if all its
> dependencies are built-in.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
> index 78b0fba..8afc6fe 100644
> --- a/drivers/staging/media/omap4iss/Kconfig
> +++ b/drivers/staging/media/omap4iss/Kconfig
> @@ -1,6 +1,6 @@
> config VIDEO_OMAP4
> bool "OMAP 4 Camera support"
> - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> + depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y && ARCH_OMAP4
> select VIDEOBUF2_DMA_CONTIG
> ---help---
> Driver for an OMAP 4 ISS controller.
>
I am ok with this if Tony and Laurent have no issues. Considering that
Laurent was working on coverting iss driver to dt, the detailed
discussion could take place at that point in time.
--
Regards,
Nishanth Menon
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-11 14:49 ` Arnd Bergmann
@ 2014-06-11 15:02 ` Tony Lindgren
-1 siblings, 0 replies; 45+ messages in thread
From: Tony Lindgren @ 2014-06-11 15:02 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Nishanth Menon, gregkh, Mauro Carvalho Chehab, Laurent Pinchart,
linux-omap, linux-media, linux-arm-kernel, arm
* Arnd Bergmann <arnd@arndb.de> [140611 07:51]:
> On Wednesday 11 June 2014 09:42:04 Nishanth Menon wrote:
> > On 06/11/2014 09:35 AM, Arnd Bergmann wrote:
> > > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > > of which can be loadable modules. This causes build failures
> > > if we want the camera driver to be built-in.
> > >
> > > This can be solved by turning the option into "tristate",
> > > which unfortunately causes another problem, because the
> > > driver incorrectly calls a platform-internal interface
> > > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > > To work around that, we can export those symbols, but
> > > that isn't really the correct solution, as we should not
> > > have dependencies on platform code this way.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > > This is one of just two patches we currently need to get
> > > 'make allmodconfig' to build again on ARM.
> > >
> > > diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> > > index 751f354..05d2d98 100644
> > > --- a/arch/arm/mach-omap2/control.c
> > > +++ b/arch/arm/mach-omap2/control.c
> > > @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
> > > {
> > > return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
> > > }
> > > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
> > >
> > > void omap4_ctrl_pad_writel(u32 val, u16 offset)
> > > {
> > > writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
> > > }
> > > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
> > >
> > > #ifdef CONFIG_ARCH_OMAP3
> > >
> > > diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
> > > index 78b0fba..0c3e3c1 100644
> > > --- a/drivers/staging/media/omap4iss/Kconfig
> > > +++ b/drivers/staging/media/omap4iss/Kconfig
> > > @@ -1,5 +1,5 @@
> > > config VIDEO_OMAP4
> > > - bool "OMAP 4 Camera support"
> > > + tristate "OMAP 4 Camera support"
> > > depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> > > select VIDEOBUF2_DMA_CONTIG
> > > ---help---
> > >
> >
> > This was discussed in detail here:
> > http://marc.info/?t=140198692500001&r=1&w=2
> > Direct dependency from a staging driver to mach-omap2 driver is not
> > something we'd like, right?
>
> So it was decided to just leave ARM allmodconfig broken?
>
> Why not at least do this instead?
>
> 8<----
> From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, 21 Jan 2014 09:32:43 +0100
> Subject: [PATCH] [media] staging: tighten omap4iss dependencies
>
> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> of which can be loadable modules. This causes build failures
> if we want the camera driver to be built-in.
>
> This can be solved by turning the option into "tristate",
> which unfortunately causes another problem, because the
> driver incorrectly calls a platform-internal interface
> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
>
> Instead, this patch just forbids the invalid configurations
> and ensures that the driver can only be built if all its
> dependencies are built-in.
Makes sense to me if the media people are OK with this:
Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
> index 78b0fba..8afc6fe 100644
> --- a/drivers/staging/media/omap4iss/Kconfig
> +++ b/drivers/staging/media/omap4iss/Kconfig
> @@ -1,6 +1,6 @@
> config VIDEO_OMAP4
> bool "OMAP 4 Camera support"
> - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> + depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y && ARCH_OMAP4
> select VIDEOBUF2_DMA_CONTIG
> ---help---
> Driver for an OMAP 4 ISS controller.
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-11 15:02 ` Tony Lindgren
0 siblings, 0 replies; 45+ messages in thread
From: Tony Lindgren @ 2014-06-11 15:02 UTC (permalink / raw)
To: linux-arm-kernel
* Arnd Bergmann <arnd@arndb.de> [140611 07:51]:
> On Wednesday 11 June 2014 09:42:04 Nishanth Menon wrote:
> > On 06/11/2014 09:35 AM, Arnd Bergmann wrote:
> > > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > > of which can be loadable modules. This causes build failures
> > > if we want the camera driver to be built-in.
> > >
> > > This can be solved by turning the option into "tristate",
> > > which unfortunately causes another problem, because the
> > > driver incorrectly calls a platform-internal interface
> > > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > > To work around that, we can export those symbols, but
> > > that isn't really the correct solution, as we should not
> > > have dependencies on platform code this way.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > > This is one of just two patches we currently need to get
> > > 'make allmodconfig' to build again on ARM.
> > >
> > > diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> > > index 751f354..05d2d98 100644
> > > --- a/arch/arm/mach-omap2/control.c
> > > +++ b/arch/arm/mach-omap2/control.c
> > > @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
> > > {
> > > return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
> > > }
> > > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
> > >
> > > void omap4_ctrl_pad_writel(u32 val, u16 offset)
> > > {
> > > writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
> > > }
> > > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
> > >
> > > #ifdef CONFIG_ARCH_OMAP3
> > >
> > > diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
> > > index 78b0fba..0c3e3c1 100644
> > > --- a/drivers/staging/media/omap4iss/Kconfig
> > > +++ b/drivers/staging/media/omap4iss/Kconfig
> > > @@ -1,5 +1,5 @@
> > > config VIDEO_OMAP4
> > > - bool "OMAP 4 Camera support"
> > > + tristate "OMAP 4 Camera support"
> > > depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> > > select VIDEOBUF2_DMA_CONTIG
> > > ---help---
> > >
> >
> > This was discussed in detail here:
> > http://marc.info/?t=140198692500001&r=1&w=2
> > Direct dependency from a staging driver to mach-omap2 driver is not
> > something we'd like, right?
>
> So it was decided to just leave ARM allmodconfig broken?
>
> Why not at least do this instead?
>
> 8<----
> From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, 21 Jan 2014 09:32:43 +0100
> Subject: [PATCH] [media] staging: tighten omap4iss dependencies
>
> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> of which can be loadable modules. This causes build failures
> if we want the camera driver to be built-in.
>
> This can be solved by turning the option into "tristate",
> which unfortunately causes another problem, because the
> driver incorrectly calls a platform-internal interface
> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
>
> Instead, this patch just forbids the invalid configurations
> and ensures that the driver can only be built if all its
> dependencies are built-in.
Makes sense to me if the media people are OK with this:
Acked-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> diff --git a/drivers/staging/media/omap4iss/Kconfig b/drivers/staging/media/omap4iss/Kconfig
> index 78b0fba..8afc6fe 100644
> --- a/drivers/staging/media/omap4iss/Kconfig
> +++ b/drivers/staging/media/omap4iss/Kconfig
> @@ -1,6 +1,6 @@
> config VIDEO_OMAP4
> bool "OMAP 4 Camera support"
> - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> + depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y && ARCH_OMAP4
> select VIDEOBUF2_DMA_CONTIG
> ---help---
> Driver for an OMAP 4 ISS controller.
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-11 14:49 ` Arnd Bergmann
@ 2014-06-12 14:12 ` Laurent Pinchart
-1 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-12 14:12 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Nishanth Menon, gregkh, Tony Lindgren, Mauro Carvalho Chehab,
linux-omap, linux-media, linux-arm-kernel, arm
Hi Arnd,
On Wednesday 11 June 2014 16:49:31 Arnd Bergmann wrote:
> On Wednesday 11 June 2014 09:42:04 Nishanth Menon wrote:
> > On 06/11/2014 09:35 AM, Arnd Bergmann wrote:
> > > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > > of which can be loadable modules. This causes build failures
> > > if we want the camera driver to be built-in.
> > >
> > > This can be solved by turning the option into "tristate",
> > > which unfortunately causes another problem, because the
> > > driver incorrectly calls a platform-internal interface
> > > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > > To work around that, we can export those symbols, but
> > > that isn't really the correct solution, as we should not
> > > have dependencies on platform code this way.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > > This is one of just two patches we currently need to get
> > > 'make allmodconfig' to build again on ARM.
> > >
> > > diff --git a/arch/arm/mach-omap2/control.c
> > > b/arch/arm/mach-omap2/control.c
> > > index 751f354..05d2d98 100644
> > > --- a/arch/arm/mach-omap2/control.c
> > > +++ b/arch/arm/mach-omap2/control.c
> > > @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
> > > {
> > > return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
> > > }
> > > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
> > >
> > > void omap4_ctrl_pad_writel(u32 val, u16 offset)
> > > {
> > > writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
> > > }
> > > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
> > >
> > > #ifdef CONFIG_ARCH_OMAP3
> > >
> > > diff --git a/drivers/staging/media/omap4iss/Kconfig
> > > b/drivers/staging/media/omap4iss/Kconfig index 78b0fba..0c3e3c1 100644
> > > --- a/drivers/staging/media/omap4iss/Kconfig
> > > +++ b/drivers/staging/media/omap4iss/Kconfig
> > > @@ -1,5 +1,5 @@
> > > config VIDEO_OMAP4
> > > - bool "OMAP 4 Camera support"
> > > + tristate "OMAP 4 Camera support"
> > > depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> > > select VIDEOBUF2_DMA_CONTIG
> > > ---help---
> >
> > This was discussed in detail here:
> > http://marc.info/?t=140198692500001&r=1&w=2
> > Direct dependency from a staging driver to mach-omap2 driver is not
> > something we'd like, right?
>
> So it was decided to just leave ARM allmodconfig broken?
>
> Why not at least do this instead?
>
> 8<----
> From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, 21 Jan 2014 09:32:43 +0100
> Subject: [PATCH] [media] staging: tighten omap4iss dependencies
>
> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> of which can be loadable modules. This causes build failures
> if we want the camera driver to be built-in.
>
> This can be solved by turning the option into "tristate",
> which unfortunately causes another problem, because the
> driver incorrectly calls a platform-internal interface
> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
>
> Instead, this patch just forbids the invalid configurations
> and ensures that the driver can only be built if all its
> dependencies are built-in.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Should I take this in my tree for v3.17 or would you like to fast-track it ?
> diff --git a/drivers/staging/media/omap4iss/Kconfig
> b/drivers/staging/media/omap4iss/Kconfig index 78b0fba..8afc6fe 100644
> --- a/drivers/staging/media/omap4iss/Kconfig
> +++ b/drivers/staging/media/omap4iss/Kconfig
> @@ -1,6 +1,6 @@
> config VIDEO_OMAP4
> bool "OMAP 4 Camera support"
> - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> + depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y && ARCH_OMAP4
> select VIDEOBUF2_DMA_CONTIG
> ---help---
> Driver for an OMAP 4 ISS controller.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-12 14:12 ` Laurent Pinchart
0 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-12 14:12 UTC (permalink / raw)
To: linux-arm-kernel
Hi Arnd,
On Wednesday 11 June 2014 16:49:31 Arnd Bergmann wrote:
> On Wednesday 11 June 2014 09:42:04 Nishanth Menon wrote:
> > On 06/11/2014 09:35 AM, Arnd Bergmann wrote:
> > > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > > of which can be loadable modules. This causes build failures
> > > if we want the camera driver to be built-in.
> > >
> > > This can be solved by turning the option into "tristate",
> > > which unfortunately causes another problem, because the
> > > driver incorrectly calls a platform-internal interface
> > > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > > To work around that, we can export those symbols, but
> > > that isn't really the correct solution, as we should not
> > > have dependencies on platform code this way.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > > This is one of just two patches we currently need to get
> > > 'make allmodconfig' to build again on ARM.
> > >
> > > diff --git a/arch/arm/mach-omap2/control.c
> > > b/arch/arm/mach-omap2/control.c
> > > index 751f354..05d2d98 100644
> > > --- a/arch/arm/mach-omap2/control.c
> > > +++ b/arch/arm/mach-omap2/control.c
> > > @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
> > > {
> > > return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
> > > }
> > > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
> > >
> > > void omap4_ctrl_pad_writel(u32 val, u16 offset)
> > > {
> > > writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
> > > }
> > > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
> > >
> > > #ifdef CONFIG_ARCH_OMAP3
> > >
> > > diff --git a/drivers/staging/media/omap4iss/Kconfig
> > > b/drivers/staging/media/omap4iss/Kconfig index 78b0fba..0c3e3c1 100644
> > > --- a/drivers/staging/media/omap4iss/Kconfig
> > > +++ b/drivers/staging/media/omap4iss/Kconfig
> > > @@ -1,5 +1,5 @@
> > > config VIDEO_OMAP4
> > > - bool "OMAP 4 Camera support"
> > > + tristate "OMAP 4 Camera support"
> > > depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> > > select VIDEOBUF2_DMA_CONTIG
> > > ---help---
> >
> > This was discussed in detail here:
> > http://marc.info/?t=140198692500001&r=1&w=2
> > Direct dependency from a staging driver to mach-omap2 driver is not
> > something we'd like, right?
>
> So it was decided to just leave ARM allmodconfig broken?
>
> Why not at least do this instead?
>
> 8<----
> From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Tue, 21 Jan 2014 09:32:43 +0100
> Subject: [PATCH] [media] staging: tighten omap4iss dependencies
>
> The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> of which can be loadable modules. This causes build failures
> if we want the camera driver to be built-in.
>
> This can be solved by turning the option into "tristate",
> which unfortunately causes another problem, because the
> driver incorrectly calls a platform-internal interface
> for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
>
> Instead, this patch just forbids the invalid configurations
> and ensures that the driver can only be built if all its
> dependencies are built-in.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Should I take this in my tree for v3.17 or would you like to fast-track it ?
> diff --git a/drivers/staging/media/omap4iss/Kconfig
> b/drivers/staging/media/omap4iss/Kconfig index 78b0fba..8afc6fe 100644
> --- a/drivers/staging/media/omap4iss/Kconfig
> +++ b/drivers/staging/media/omap4iss/Kconfig
> @@ -1,6 +1,6 @@
> config VIDEO_OMAP4
> bool "OMAP 4 Camera support"
> - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> + depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y && ARCH_OMAP4
> select VIDEOBUF2_DMA_CONTIG
> ---help---
> Driver for an OMAP 4 ISS controller.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-12 14:12 ` Laurent Pinchart
@ 2014-06-12 14:15 ` Arnd Bergmann
-1 siblings, 0 replies; 45+ messages in thread
From: Arnd Bergmann @ 2014-06-12 14:15 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Laurent Pinchart, Nishanth Menon, Tony Lindgren, gregkh,
Mauro Carvalho Chehab, arm, linux-omap, linux-media
On Thursday 12 June 2014 16:12:17 Laurent Pinchart wrote:
> > From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> > From: Arnd Bergmann <arnd@arndb.de>
> > Date: Tue, 21 Jan 2014 09:32:43 +0100
> > Subject: [PATCH] [media] staging: tighten omap4iss dependencies
> >
> > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > of which can be loadable modules. This causes build failures
> > if we want the camera driver to be built-in.
> >
> > This can be solved by turning the option into "tristate",
> > which unfortunately causes another problem, because the
> > driver incorrectly calls a platform-internal interface
> > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> >
> > Instead, this patch just forbids the invalid configurations
> > and ensures that the driver can only be built if all its
> > dependencies are built-in.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Should I take this in my tree for v3.17 or would you like to fast-track it ?
>
I'd actually like to see it in 3.15 as a stable backport if possible,
but definitely in 3.16. What is the normal path for staging/media
but fix patches?
Arnd
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-12 14:15 ` Arnd Bergmann
0 siblings, 0 replies; 45+ messages in thread
From: Arnd Bergmann @ 2014-06-12 14:15 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 12 June 2014 16:12:17 Laurent Pinchart wrote:
> > From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> > From: Arnd Bergmann <arnd@arndb.de>
> > Date: Tue, 21 Jan 2014 09:32:43 +0100
> > Subject: [PATCH] [media] staging: tighten omap4iss dependencies
> >
> > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > of which can be loadable modules. This causes build failures
> > if we want the camera driver to be built-in.
> >
> > This can be solved by turning the option into "tristate",
> > which unfortunately causes another problem, because the
> > driver incorrectly calls a platform-internal interface
> > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> >
> > Instead, this patch just forbids the invalid configurations
> > and ensures that the driver can only be built if all its
> > dependencies are built-in.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Should I take this in my tree for v3.17 or would you like to fast-track it ?
>
I'd actually like to see it in 3.15 as a stable backport if possible,
but definitely in 3.16. What is the normal path for staging/media
but fix patches?
Arnd
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-12 14:15 ` Arnd Bergmann
@ 2014-06-12 14:25 ` Greg KH
-1 siblings, 0 replies; 45+ messages in thread
From: Greg KH @ 2014-06-12 14:25 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Laurent Pinchart, Nishanth Menon,
Tony Lindgren, Mauro Carvalho Chehab, arm, linux-omap,
linux-media
On Thu, Jun 12, 2014 at 04:15:32PM +0200, Arnd Bergmann wrote:
> On Thursday 12 June 2014 16:12:17 Laurent Pinchart wrote:
> > > From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> > > From: Arnd Bergmann <arnd@arndb.de>
> > > Date: Tue, 21 Jan 2014 09:32:43 +0100
> > > Subject: [PATCH] [media] staging: tighten omap4iss dependencies
> > >
> > > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > > of which can be loadable modules. This causes build failures
> > > if we want the camera driver to be built-in.
> > >
> > > This can be solved by turning the option into "tristate",
> > > which unfortunately causes another problem, because the
> > > driver incorrectly calls a platform-internal interface
> > > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > >
> > > Instead, this patch just forbids the invalid configurations
> > > and ensures that the driver can only be built if all its
> > > dependencies are built-in.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Should I take this in my tree for v3.17 or would you like to fast-track it ?
> >
>
> I'd actually like to see it in 3.15 as a stable backport if possible,
It's not stable material, sorry.
> but definitely in 3.16. What is the normal path for staging/media
> but fix patches?
Through Mauro's tree.
greg k-h
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-12 14:25 ` Greg KH
0 siblings, 0 replies; 45+ messages in thread
From: Greg KH @ 2014-06-12 14:25 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 12, 2014 at 04:15:32PM +0200, Arnd Bergmann wrote:
> On Thursday 12 June 2014 16:12:17 Laurent Pinchart wrote:
> > > From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> > > From: Arnd Bergmann <arnd@arndb.de>
> > > Date: Tue, 21 Jan 2014 09:32:43 +0100
> > > Subject: [PATCH] [media] staging: tighten omap4iss dependencies
> > >
> > > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > > of which can be loadable modules. This causes build failures
> > > if we want the camera driver to be built-in.
> > >
> > > This can be solved by turning the option into "tristate",
> > > which unfortunately causes another problem, because the
> > > driver incorrectly calls a platform-internal interface
> > > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > >
> > > Instead, this patch just forbids the invalid configurations
> > > and ensures that the driver can only be built if all its
> > > dependencies are built-in.
> > >
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Should I take this in my tree for v3.17 or would you like to fast-track it ?
> >
>
> I'd actually like to see it in 3.15 as a stable backport if possible,
It's not stable material, sorry.
> but definitely in 3.16. What is the normal path for staging/media
> but fix patches?
Through Mauro's tree.
greg k-h
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-12 14:25 ` Greg KH
@ 2014-06-12 14:28 ` Arnd Bergmann
-1 siblings, 0 replies; 45+ messages in thread
From: Arnd Bergmann @ 2014-06-12 14:28 UTC (permalink / raw)
To: Greg KH
Cc: linux-arm-kernel, Laurent Pinchart, Nishanth Menon,
Tony Lindgren, Mauro Carvalho Chehab, arm, linux-omap,
linux-media
On Thursday 12 June 2014 07:25:15 Greg KH wrote:
> On Thu, Jun 12, 2014 at 04:15:32PM +0200, Arnd Bergmann wrote:
> > On Thursday 12 June 2014 16:12:17 Laurent Pinchart wrote:
> > > > From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> > > > From: Arnd Bergmann <arnd@arndb.de>
> > > > Date: Tue, 21 Jan 2014 09:32:43 +0100
> > > > Subject: [PATCH] [media] staging: tighten omap4iss dependencies
> > > >
> > > > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > > > of which can be loadable modules. This causes build failures
> > > > if we want the camera driver to be built-in.
> > > >
> > > > This can be solved by turning the option into "tristate",
> > > > which unfortunately causes another problem, because the
> > > > driver incorrectly calls a platform-internal interface
> > > > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > > >
> > > > Instead, this patch just forbids the invalid configurations
> > > > and ensures that the driver can only be built if all its
> > > > dependencies are built-in.
> > > >
> > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > >
> > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >
> > > Should I take this in my tree for v3.17 or would you like to fast-track it ?
> > >
> >
> > I'd actually like to see it in 3.15 as a stable backport if possible,
>
> It's not stable material, sorry.
To clarify, I was talking about second version of the patch,
not the original one. It just does this:
> config VIDEO_OMAP4
> bool "OMAP 4 Camera support"
> - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> + depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y && ARCH_OMAP4
> select VIDEOBUF2_DMA_CONTIG
> ---help---
> Driver for an OMAP 4 ISS controller.
which enforces that configurations that cannot be compiled
will not be selectable in Kconfig, so we can have allmodconfig
working. I thought that was ok for -stable.
> > but definitely in 3.16. What is the normal path for staging/media
> > but fix patches?
>
> Through Mauro's tree.
Ok.
Arnd
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-12 14:28 ` Arnd Bergmann
0 siblings, 0 replies; 45+ messages in thread
From: Arnd Bergmann @ 2014-06-12 14:28 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 12 June 2014 07:25:15 Greg KH wrote:
> On Thu, Jun 12, 2014 at 04:15:32PM +0200, Arnd Bergmann wrote:
> > On Thursday 12 June 2014 16:12:17 Laurent Pinchart wrote:
> > > > From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> > > > From: Arnd Bergmann <arnd@arndb.de>
> > > > Date: Tue, 21 Jan 2014 09:32:43 +0100
> > > > Subject: [PATCH] [media] staging: tighten omap4iss dependencies
> > > >
> > > > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > > > of which can be loadable modules. This causes build failures
> > > > if we want the camera driver to be built-in.
> > > >
> > > > This can be solved by turning the option into "tristate",
> > > > which unfortunately causes another problem, because the
> > > > driver incorrectly calls a platform-internal interface
> > > > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > > >
> > > > Instead, this patch just forbids the invalid configurations
> > > > and ensures that the driver can only be built if all its
> > > > dependencies are built-in.
> > > >
> > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > >
> > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >
> > > Should I take this in my tree for v3.17 or would you like to fast-track it ?
> > >
> >
> > I'd actually like to see it in 3.15 as a stable backport if possible,
>
> It's not stable material, sorry.
To clarify, I was talking about second version of the patch,
not the original one. It just does this:
> config VIDEO_OMAP4
> bool "OMAP 4 Camera support"
> - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> + depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y && ARCH_OMAP4
> select VIDEOBUF2_DMA_CONTIG
> ---help---
> Driver for an OMAP 4 ISS controller.
which enforces that configurations that cannot be compiled
will not be selectable in Kconfig, so we can have allmodconfig
working. I thought that was ok for -stable.
> > but definitely in 3.16. What is the normal path for staging/media
> > but fix patches?
>
> Through Mauro's tree.
Ok.
Arnd
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-11 14:47 ` Tony Lindgren
@ 2014-06-12 14:52 ` Laurent Pinchart
-1 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-12 14:52 UTC (permalink / raw)
To: Tony Lindgren
Cc: Arnd Bergmann, gregkh, Mauro Carvalho Chehab, linux-omap,
linux-media, linux-arm-kernel, arm
Hi Tony,
On Wednesday 11 June 2014 07:47:54 Tony Lindgren wrote:
> * Arnd Bergmann <arnd@arndb.de> [140611 07:37]:
> > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > of which can be loadable modules. This causes build failures
> > if we want the camera driver to be built-in.
>
> That's good news, but let's not fix it this way.
>
> > This can be solved by turning the option into "tristate",
> > which unfortunately causes another problem, because the
> > driver incorrectly calls a platform-internal interface
> > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > To work around that, we can export those symbols, but
> > that isn't really the correct solution, as we should not
> > have dependencies on platform code this way.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > This is one of just two patches we currently need to get
> > 'make allmodconfig' to build again on ARM.
> >
> > diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> > index 751f354..05d2d98 100644
> > --- a/arch/arm/mach-omap2/control.c
> > +++ b/arch/arm/mach-omap2/control.c
> > @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
> > {
> > return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
> > }
> > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
> >
> > void omap4_ctrl_pad_writel(u32 val, u16 offset)
> > {
> > writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
> > }
> > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
> >
> > #ifdef CONFIG_ARCH_OMAP3
>
> Exporting these will likely cause immediate misuse in other
> drivers all over the place.
>
> These should just use either pinctrl-single.c instead for muxing.
> Or if they are not mux registers, we do have the syscon mapping
> available in omap4.dtsi that pbias-regulator.c is already using.
>
> Laurent, got any better ideas?
The ISS driver needs to write a single register, which contains several
independent fields. They thus need to be controlled by a single driver. Some
of them might be considered to be related to pinmuxing (although I disagree on
that), others are certainly not about muxing (there are clock gate bits for
instance).
Using the syscon mapping seems like the best option. I'll give it a try.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-12 14:52 ` Laurent Pinchart
0 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-12 14:52 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tony,
On Wednesday 11 June 2014 07:47:54 Tony Lindgren wrote:
> * Arnd Bergmann <arnd@arndb.de> [140611 07:37]:
> > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > of which can be loadable modules. This causes build failures
> > if we want the camera driver to be built-in.
>
> That's good news, but let's not fix it this way.
>
> > This can be solved by turning the option into "tristate",
> > which unfortunately causes another problem, because the
> > driver incorrectly calls a platform-internal interface
> > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > To work around that, we can export those symbols, but
> > that isn't really the correct solution, as we should not
> > have dependencies on platform code this way.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> > This is one of just two patches we currently need to get
> > 'make allmodconfig' to build again on ARM.
> >
> > diff --git a/arch/arm/mach-omap2/control.c b/arch/arm/mach-omap2/control.c
> > index 751f354..05d2d98 100644
> > --- a/arch/arm/mach-omap2/control.c
> > +++ b/arch/arm/mach-omap2/control.c
> > @@ -190,11 +190,13 @@ u32 omap4_ctrl_pad_readl(u16 offset)
> > {
> > return readl_relaxed(OMAP4_CTRL_PAD_REGADDR(offset));
> > }
> > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_readl);
> >
> > void omap4_ctrl_pad_writel(u32 val, u16 offset)
> > {
> > writel_relaxed(val, OMAP4_CTRL_PAD_REGADDR(offset));
> > }
> > +EXPORT_SYMBOL_GPL(omap4_ctrl_pad_writel);
> >
> > #ifdef CONFIG_ARCH_OMAP3
>
> Exporting these will likely cause immediate misuse in other
> drivers all over the place.
>
> These should just use either pinctrl-single.c instead for muxing.
> Or if they are not mux registers, we do have the syscon mapping
> available in omap4.dtsi that pbias-regulator.c is already using.
>
> Laurent, got any better ideas?
The ISS driver needs to write a single register, which contains several
independent fields. They thus need to be controlled by a single driver. Some
of them might be considered to be related to pinmuxing (although I disagree on
that), others are certainly not about muxing (there are clock gate bits for
instance).
Using the syscon mapping seems like the best option. I'll give it a try.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-12 14:28 ` Arnd Bergmann
@ 2014-06-12 15:00 ` Laurent Pinchart
-1 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-12 15:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Greg KH, linux-arm-kernel, Nishanth Menon, Tony Lindgren,
Mauro Carvalho Chehab, arm, linux-omap, linux-media
Hi Arnd,
On Thursday 12 June 2014 16:28:39 Arnd Bergmann wrote:
> On Thursday 12 June 2014 07:25:15 Greg KH wrote:
> > On Thu, Jun 12, 2014 at 04:15:32PM +0200, Arnd Bergmann wrote:
> > > On Thursday 12 June 2014 16:12:17 Laurent Pinchart wrote:
> > > > > From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00
> > > > > 2001
> > > > > From: Arnd Bergmann <arnd@arndb.de>
> > > > > Date: Tue, 21 Jan 2014 09:32:43 +0100
> > > > > Subject: [PATCH] [media] staging: tighten omap4iss dependencies
> > > > >
> > > > > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > > > > of which can be loadable modules. This causes build failures
> > > > > if we want the camera driver to be built-in.
> > > > >
> > > > > This can be solved by turning the option into "tristate",
> > > > > which unfortunately causes another problem, because the
> > > > > driver incorrectly calls a platform-internal interface
> > > > > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > > > >
> > > > > Instead, this patch just forbids the invalid configurations
> > > > > and ensures that the driver can only be built if all its
> > > > > dependencies are built-in.
> > > > >
> > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > >
> > > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > > Should I take this in my tree for v3.17 or would you like to
> > > > fast-track it ?> >
> > > I'd actually like to see it in 3.15 as a stable backport if possible,
> >
> > It's not stable material, sorry.
>
> To clarify, I was talking about second version of the patch,
> not the original one. It just does this:
>
> > config VIDEO_OMAP4
> > bool "OMAP 4 Camera support"
> > - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> > + depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y &&
> > ARCH_OMAP4
> > select VIDEOBUF2_DMA_CONTIG
> > ---help---
> > Driver for an OMAP 4 ISS controller.
>
> which enforces that configurations that cannot be compiled
> will not be selectable in Kconfig, so we can have allmodconfig
> working. I thought that was ok for -stable.
>
> > > but definitely in 3.16. What is the normal path for staging/media
> > > but fix patches?
> >
> > Through Mauro's tree.
>
> Ok.
I've applied the patch to my tree and will send a pull request to Mauro for
v3.16 as soon as you reach an agreement with Greg on whether I should add CC:
stable or not.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-12 15:00 ` Laurent Pinchart
0 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-12 15:00 UTC (permalink / raw)
To: linux-arm-kernel
Hi Arnd,
On Thursday 12 June 2014 16:28:39 Arnd Bergmann wrote:
> On Thursday 12 June 2014 07:25:15 Greg KH wrote:
> > On Thu, Jun 12, 2014 at 04:15:32PM +0200, Arnd Bergmann wrote:
> > > On Thursday 12 June 2014 16:12:17 Laurent Pinchart wrote:
> > > > > From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00
> > > > > 2001
> > > > > From: Arnd Bergmann <arnd@arndb.de>
> > > > > Date: Tue, 21 Jan 2014 09:32:43 +0100
> > > > > Subject: [PATCH] [media] staging: tighten omap4iss dependencies
> > > > >
> > > > > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > > > > of which can be loadable modules. This causes build failures
> > > > > if we want the camera driver to be built-in.
> > > > >
> > > > > This can be solved by turning the option into "tristate",
> > > > > which unfortunately causes another problem, because the
> > > > > driver incorrectly calls a platform-internal interface
> > > > > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > > > >
> > > > > Instead, this patch just forbids the invalid configurations
> > > > > and ensures that the driver can only be built if all its
> > > > > dependencies are built-in.
> > > > >
> > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > >
> > > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > > Should I take this in my tree for v3.17 or would you like to
> > > > fast-track it ?> >
> > > I'd actually like to see it in 3.15 as a stable backport if possible,
> >
> > It's not stable material, sorry.
>
> To clarify, I was talking about second version of the patch,
> not the original one. It just does this:
>
> > config VIDEO_OMAP4
> > bool "OMAP 4 Camera support"
> > - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> > + depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y &&
> > ARCH_OMAP4
> > select VIDEOBUF2_DMA_CONTIG
> > ---help---
> > Driver for an OMAP 4 ISS controller.
>
> which enforces that configurations that cannot be compiled
> will not be selectable in Kconfig, so we can have allmodconfig
> working. I thought that was ok for -stable.
>
> > > but definitely in 3.16. What is the normal path for staging/media
> > > but fix patches?
> >
> > Through Mauro's tree.
>
> Ok.
I've applied the patch to my tree and will send a pull request to Mauro for
v3.16 as soon as you reach an agreement with Greg on whether I should add CC:
stable or not.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-12 14:52 ` Laurent Pinchart
@ 2014-06-12 15:15 ` Tony Lindgren
-1 siblings, 0 replies; 45+ messages in thread
From: Tony Lindgren @ 2014-06-12 15:15 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Arnd Bergmann, gregkh, Mauro Carvalho Chehab, linux-omap,
linux-media, linux-arm-kernel, arm
* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 07:52]:
> On Wednesday 11 June 2014 07:47:54 Tony Lindgren wrote:
> >
> > These should just use either pinctrl-single.c instead for muxing.
> > Or if they are not mux registers, we do have the syscon mapping
> > available in omap4.dtsi that pbias-regulator.c is already using.
> >
> > Laurent, got any better ideas?
>
> The ISS driver needs to write a single register, which contains several
> independent fields. They thus need to be controlled by a single driver. Some
> of them might be considered to be related to pinmuxing (although I disagree on
> that), others are certainly not about muxing (there are clock gate bits for
> instance).
>
> Using the syscon mapping seems like the best option. I'll give it a try.
OK if it's not strictly pinctrl related then let's not use
pinctrl-single,bits for it. You may be able to implement one or more
framework drivers for it for pinctrl/regulator/clock/transceiver
whatever that register is doing.
In any case it's best to have that handling in a separate helper driver
somewhere as it's a separate piece of hardware from the camera module.
If it does not fit into any existing frameworks then it's best to have
it in a separate driver with the camera driver.
Regards,
Tony
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-12 15:15 ` Tony Lindgren
0 siblings, 0 replies; 45+ messages in thread
From: Tony Lindgren @ 2014-06-12 15:15 UTC (permalink / raw)
To: linux-arm-kernel
* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 07:52]:
> On Wednesday 11 June 2014 07:47:54 Tony Lindgren wrote:
> >
> > These should just use either pinctrl-single.c instead for muxing.
> > Or if they are not mux registers, we do have the syscon mapping
> > available in omap4.dtsi that pbias-regulator.c is already using.
> >
> > Laurent, got any better ideas?
>
> The ISS driver needs to write a single register, which contains several
> independent fields. They thus need to be controlled by a single driver. Some
> of them might be considered to be related to pinmuxing (although I disagree on
> that), others are certainly not about muxing (there are clock gate bits for
> instance).
>
> Using the syscon mapping seems like the best option. I'll give it a try.
OK if it's not strictly pinctrl related then let's not use
pinctrl-single,bits for it. You may be able to implement one or more
framework drivers for it for pinctrl/regulator/clock/transceiver
whatever that register is doing.
In any case it's best to have that handling in a separate helper driver
somewhere as it's a separate piece of hardware from the camera module.
If it does not fit into any existing frameworks then it's best to have
it in a separate driver with the camera driver.
Regards,
Tony
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-12 15:15 ` Tony Lindgren
@ 2014-06-12 15:31 ` Laurent Pinchart
-1 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-12 15:31 UTC (permalink / raw)
To: Tony Lindgren
Cc: Arnd Bergmann, gregkh, Mauro Carvalho Chehab, linux-omap,
linux-media, linux-arm-kernel, arm
Hi Tony,
On Thursday 12 June 2014 08:15:35 Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 07:52]:
> > On Wednesday 11 June 2014 07:47:54 Tony Lindgren wrote:
> > > These should just use either pinctrl-single.c instead for muxing.
> > > Or if they are not mux registers, we do have the syscon mapping
> > > available in omap4.dtsi that pbias-regulator.c is already using.
> > >
> > > Laurent, got any better ideas?
> >
> > The ISS driver needs to write a single register, which contains several
> > independent fields. They thus need to be controlled by a single driver.
> > Some of them might be considered to be related to pinmuxing (although I
> > disagree on that), others are certainly not about muxing (there are clock
> > gate bits for instance).
> >
> > Using the syscon mapping seems like the best option. I'll give it a try.
>
> OK if it's not strictly pinctrl related then let's not use
> pinctrl-single,bits for it. You may be able to implement one or more
> framework drivers for it for pinctrl/regulator/clock/transceiver
> whatever that register is doing.
>
> In any case it's best to have that handling in a separate helper driver
> somewhere as it's a separate piece of hardware from the camera module.
> If it does not fit into any existing frameworks then it's best to have
> it in a separate driver with the camera driver.
The register contains the following fields that control the two CSI2 PHYs
(PHY1 and PHY2).
31 CAMERARX_CSI22_LANEENABLE2 PHY2 Lane 2 (CSI22_DX2, CSI22_DY2) Enable
30 CAMERARX_CSI22_LANEENABLE1 PHY2 Lane 1 (CSI22_DX1, CSI22_DY1) Enable
29 CAMERARX_CSI22_LANEENABLE0 PHY2 Lane 0 (CSI22_DX0, CSI22_DY0) Enable
28 CAMERARX_CSI21_LANEENABLE4 PHY1 Lane 4 (CSI21_DX4, CSI21_DY4) Enable
27 CAMERARX_CSI21_LANEENABLE3 PHY1 Lane 3 (CSI21_DX3, CSI21_DY3) Enable
26 CAMERARX_CSI21_LANEENABLE2 PHY1 Lane 2 (CSI21_DX2, CSI21_DY2) Enable
25 CAMERARX_CSI21_LANEENABLE1 PHY1 Lane 1 (CSI21_DX1, CSI21_DY1) Enable
24 CAMERARX_CSI21_LANEENABLE0 PHY1 Lane 0 (CSI21_DX0, CSI21_DY0) Enable
21 CAMERARX_CSI22_CTRLCLKEN PHY2 Clock Enable
20:19 CAMERARX_CSI22_CAMMODE PHY2 Mode (CCP2, CSI1, CSI2)
18 CAMERARX_CSI21_CTRLCLKEN PHY1 Clock Enable
17:16 CAMERARX_CSI21_CAMMODE PHY1 Mode (CCP2, CSI1, CSI2)
Bits 18 and 21 could be exposed through CCF. Bits 24 to 31 enable/disable the
CSI2 lanes, so it could be argued that they could be exposed through the
pinctrl framework. However, they need to be configured independently, possibly
at runtime. I'm thus not sure pinctrl would be a good idea. Bits 17:16 and
20:19 don't fit in existing frameworks.
Given that this register is specific to the ISS, I think handling it as a
separate device through a separate driver would only complicate the
implementation without any real benefit.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-12 15:31 ` Laurent Pinchart
0 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-12 15:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tony,
On Thursday 12 June 2014 08:15:35 Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 07:52]:
> > On Wednesday 11 June 2014 07:47:54 Tony Lindgren wrote:
> > > These should just use either pinctrl-single.c instead for muxing.
> > > Or if they are not mux registers, we do have the syscon mapping
> > > available in omap4.dtsi that pbias-regulator.c is already using.
> > >
> > > Laurent, got any better ideas?
> >
> > The ISS driver needs to write a single register, which contains several
> > independent fields. They thus need to be controlled by a single driver.
> > Some of them might be considered to be related to pinmuxing (although I
> > disagree on that), others are certainly not about muxing (there are clock
> > gate bits for instance).
> >
> > Using the syscon mapping seems like the best option. I'll give it a try.
>
> OK if it's not strictly pinctrl related then let's not use
> pinctrl-single,bits for it. You may be able to implement one or more
> framework drivers for it for pinctrl/regulator/clock/transceiver
> whatever that register is doing.
>
> In any case it's best to have that handling in a separate helper driver
> somewhere as it's a separate piece of hardware from the camera module.
> If it does not fit into any existing frameworks then it's best to have
> it in a separate driver with the camera driver.
The register contains the following fields that control the two CSI2 PHYs
(PHY1 and PHY2).
31 CAMERARX_CSI22_LANEENABLE2 PHY2 Lane 2 (CSI22_DX2, CSI22_DY2) Enable
30 CAMERARX_CSI22_LANEENABLE1 PHY2 Lane 1 (CSI22_DX1, CSI22_DY1) Enable
29 CAMERARX_CSI22_LANEENABLE0 PHY2 Lane 0 (CSI22_DX0, CSI22_DY0) Enable
28 CAMERARX_CSI21_LANEENABLE4 PHY1 Lane 4 (CSI21_DX4, CSI21_DY4) Enable
27 CAMERARX_CSI21_LANEENABLE3 PHY1 Lane 3 (CSI21_DX3, CSI21_DY3) Enable
26 CAMERARX_CSI21_LANEENABLE2 PHY1 Lane 2 (CSI21_DX2, CSI21_DY2) Enable
25 CAMERARX_CSI21_LANEENABLE1 PHY1 Lane 1 (CSI21_DX1, CSI21_DY1) Enable
24 CAMERARX_CSI21_LANEENABLE0 PHY1 Lane 0 (CSI21_DX0, CSI21_DY0) Enable
21 CAMERARX_CSI22_CTRLCLKEN PHY2 Clock Enable
20:19 CAMERARX_CSI22_CAMMODE PHY2 Mode (CCP2, CSI1, CSI2)
18 CAMERARX_CSI21_CTRLCLKEN PHY1 Clock Enable
17:16 CAMERARX_CSI21_CAMMODE PHY1 Mode (CCP2, CSI1, CSI2)
Bits 18 and 21 could be exposed through CCF. Bits 24 to 31 enable/disable the
CSI2 lanes, so it could be argued that they could be exposed through the
pinctrl framework. However, they need to be configured independently, possibly
at runtime. I'm thus not sure pinctrl would be a good idea. Bits 17:16 and
20:19 don't fit in existing frameworks.
Given that this register is specific to the ISS, I think handling it as a
separate device through a separate driver would only complicate the
implementation without any real benefit.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-12 14:28 ` Arnd Bergmann
@ 2014-06-12 15:59 ` Greg KH
-1 siblings, 0 replies; 45+ messages in thread
From: Greg KH @ 2014-06-12 15:59 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, Laurent Pinchart, Nishanth Menon,
Tony Lindgren, Mauro Carvalho Chehab, arm, linux-omap,
linux-media
On Thu, Jun 12, 2014 at 04:28:39PM +0200, Arnd Bergmann wrote:
> On Thursday 12 June 2014 07:25:15 Greg KH wrote:
> > On Thu, Jun 12, 2014 at 04:15:32PM +0200, Arnd Bergmann wrote:
> > > On Thursday 12 June 2014 16:12:17 Laurent Pinchart wrote:
> > > > > From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> > > > > From: Arnd Bergmann <arnd@arndb.de>
> > > > > Date: Tue, 21 Jan 2014 09:32:43 +0100
> > > > > Subject: [PATCH] [media] staging: tighten omap4iss dependencies
> > > > >
> > > > > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > > > > of which can be loadable modules. This causes build failures
> > > > > if we want the camera driver to be built-in.
> > > > >
> > > > > This can be solved by turning the option into "tristate",
> > > > > which unfortunately causes another problem, because the
> > > > > driver incorrectly calls a platform-internal interface
> > > > > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > > > >
> > > > > Instead, this patch just forbids the invalid configurations
> > > > > and ensures that the driver can only be built if all its
> > > > > dependencies are built-in.
> > > > >
> > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > >
> > > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > > Should I take this in my tree for v3.17 or would you like to fast-track it ?
> > > >
> > >
> > > I'd actually like to see it in 3.15 as a stable backport if possible,
> >
> > It's not stable material, sorry.
>
> To clarify, I was talking about second version of the patch,
> not the original one. It just does this:
>
> > config VIDEO_OMAP4
> > bool "OMAP 4 Camera support"
> > - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> > + depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y && ARCH_OMAP4
> > select VIDEOBUF2_DMA_CONTIG
> > ---help---
> > Driver for an OMAP 4 ISS controller.
>
> which enforces that configurations that cannot be compiled
> will not be selectable in Kconfig, so we can have allmodconfig
> working. I thought that was ok for -stable.
Ah, yes, that one works, sorry, I was thinking of the first one.
greg k-h
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-12 15:59 ` Greg KH
0 siblings, 0 replies; 45+ messages in thread
From: Greg KH @ 2014-06-12 15:59 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 12, 2014 at 04:28:39PM +0200, Arnd Bergmann wrote:
> On Thursday 12 June 2014 07:25:15 Greg KH wrote:
> > On Thu, Jun 12, 2014 at 04:15:32PM +0200, Arnd Bergmann wrote:
> > > On Thursday 12 June 2014 16:12:17 Laurent Pinchart wrote:
> > > > > From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
> > > > > From: Arnd Bergmann <arnd@arndb.de>
> > > > > Date: Tue, 21 Jan 2014 09:32:43 +0100
> > > > > Subject: [PATCH] [media] staging: tighten omap4iss dependencies
> > > > >
> > > > > The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
> > > > > of which can be loadable modules. This causes build failures
> > > > > if we want the camera driver to be built-in.
> > > > >
> > > > > This can be solved by turning the option into "tristate",
> > > > > which unfortunately causes another problem, because the
> > > > > driver incorrectly calls a platform-internal interface
> > > > > for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
> > > > >
> > > > > Instead, this patch just forbids the invalid configurations
> > > > > and ensures that the driver can only be built if all its
> > > > > dependencies are built-in.
> > > > >
> > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > >
> > > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > >
> > > > Should I take this in my tree for v3.17 or would you like to fast-track it ?
> > > >
> > >
> > > I'd actually like to see it in 3.15 as a stable backport if possible,
> >
> > It's not stable material, sorry.
>
> To clarify, I was talking about second version of the patch,
> not the original one. It just does this:
>
> > config VIDEO_OMAP4
> > bool "OMAP 4 Camera support"
> > - depends on VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API && I2C && ARCH_OMAP4
> > + depends on VIDEO_V4L2=y && VIDEO_V4L2_SUBDEV_API && I2C=y && ARCH_OMAP4
> > select VIDEOBUF2_DMA_CONTIG
> > ---help---
> > Driver for an OMAP 4 ISS controller.
>
> which enforces that configurations that cannot be compiled
> will not be selectable in Kconfig, so we can have allmodconfig
> working. I thought that was ok for -stable.
Ah, yes, that one works, sorry, I was thinking of the first one.
greg k-h
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-12 15:31 ` Laurent Pinchart
@ 2014-06-13 5:30 ` Tony Lindgren
-1 siblings, 0 replies; 45+ messages in thread
From: Tony Lindgren @ 2014-06-13 5:30 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Arnd Bergmann, gregkh, Mauro Carvalho Chehab, linux-omap,
linux-media, linux-arm-kernel, arm
* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 08:32]:
> Hi Tony,
>
> On Thursday 12 June 2014 08:15:35 Tony Lindgren wrote:
> > * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 07:52]:
> > > On Wednesday 11 June 2014 07:47:54 Tony Lindgren wrote:
> > > > These should just use either pinctrl-single.c instead for muxing.
> > > > Or if they are not mux registers, we do have the syscon mapping
> > > > available in omap4.dtsi that pbias-regulator.c is already using.
> > > >
> > > > Laurent, got any better ideas?
> > >
> > > The ISS driver needs to write a single register, which contains several
> > > independent fields. They thus need to be controlled by a single driver.
> > > Some of them might be considered to be related to pinmuxing (although I
> > > disagree on that), others are certainly not about muxing (there are clock
> > > gate bits for instance).
> > >
> > > Using the syscon mapping seems like the best option. I'll give it a try.
> >
> > OK if it's not strictly pinctrl related then let's not use
> > pinctrl-single,bits for it. You may be able to implement one or more
> > framework drivers for it for pinctrl/regulator/clock/transceiver
> > whatever that register is doing.
> >
> > In any case it's best to have that handling in a separate helper driver
> > somewhere as it's a separate piece of hardware from the camera module.
> > If it does not fit into any existing frameworks then it's best to have
> > it in a separate driver with the camera driver.
>
> The register contains the following fields that control the two CSI2 PHYs
> (PHY1 and PHY2).
>
> 31 CAMERARX_CSI22_LANEENABLE2 PHY2 Lane 2 (CSI22_DX2, CSI22_DY2) Enable
> 30 CAMERARX_CSI22_LANEENABLE1 PHY2 Lane 1 (CSI22_DX1, CSI22_DY1) Enable
> 29 CAMERARX_CSI22_LANEENABLE0 PHY2 Lane 0 (CSI22_DX0, CSI22_DY0) Enable
> 28 CAMERARX_CSI21_LANEENABLE4 PHY1 Lane 4 (CSI21_DX4, CSI21_DY4) Enable
> 27 CAMERARX_CSI21_LANEENABLE3 PHY1 Lane 3 (CSI21_DX3, CSI21_DY3) Enable
> 26 CAMERARX_CSI21_LANEENABLE2 PHY1 Lane 2 (CSI21_DX2, CSI21_DY2) Enable
> 25 CAMERARX_CSI21_LANEENABLE1 PHY1 Lane 1 (CSI21_DX1, CSI21_DY1) Enable
> 24 CAMERARX_CSI21_LANEENABLE0 PHY1 Lane 0 (CSI21_DX0, CSI21_DY0) Enable
> 21 CAMERARX_CSI22_CTRLCLKEN PHY2 Clock Enable
> 20:19 CAMERARX_CSI22_CAMMODE PHY2 Mode (CCP2, CSI1, CSI2)
> 18 CAMERARX_CSI21_CTRLCLKEN PHY1 Clock Enable
> 17:16 CAMERARX_CSI21_CAMMODE PHY1 Mode (CCP2, CSI1, CSI2)
>
> Bits 18 and 21 could be exposed through CCF. Bits 24 to 31 enable/disable the
> CSI2 lanes, so it could be argued that they could be exposed through the
> pinctrl framework. However, they need to be configured independently, possibly
> at runtime. I'm thus not sure pinctrl would be a good idea. Bits 17:16 and
> 20:19 don't fit in existing frameworks.
OK thanks for the info. Sounds like drivers/phy might be the right location
for it then and then the phy driver can use the syscon regmap.
> Given that this register is specific to the ISS, I think handling it as a
> separate device through a separate driver would only complicate the
> implementation without any real benefit.
Even though it's one register, it shoud still be treated separately from
the camera driver. The problems with keeping the register access to the
control module in the camera driver are at least following:
1. They live in separate hardware modules that can be clocked separately
2. Doing a read-back to flush a posted write in one hardware module most
likely won't flush the write to other and that can lead into hard to
find mysterious bugs
3. If we ever have a common system control module driver, we need to
rewrite all the system control module register tinkering in the drivers
So it's best to try to use an existing framework for it. That avoids
tons of pain later on ;)
Regards,
Tony
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-13 5:30 ` Tony Lindgren
0 siblings, 0 replies; 45+ messages in thread
From: Tony Lindgren @ 2014-06-13 5:30 UTC (permalink / raw)
To: linux-arm-kernel
* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 08:32]:
> Hi Tony,
>
> On Thursday 12 June 2014 08:15:35 Tony Lindgren wrote:
> > * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 07:52]:
> > > On Wednesday 11 June 2014 07:47:54 Tony Lindgren wrote:
> > > > These should just use either pinctrl-single.c instead for muxing.
> > > > Or if they are not mux registers, we do have the syscon mapping
> > > > available in omap4.dtsi that pbias-regulator.c is already using.
> > > >
> > > > Laurent, got any better ideas?
> > >
> > > The ISS driver needs to write a single register, which contains several
> > > independent fields. They thus need to be controlled by a single driver.
> > > Some of them might be considered to be related to pinmuxing (although I
> > > disagree on that), others are certainly not about muxing (there are clock
> > > gate bits for instance).
> > >
> > > Using the syscon mapping seems like the best option. I'll give it a try.
> >
> > OK if it's not strictly pinctrl related then let's not use
> > pinctrl-single,bits for it. You may be able to implement one or more
> > framework drivers for it for pinctrl/regulator/clock/transceiver
> > whatever that register is doing.
> >
> > In any case it's best to have that handling in a separate helper driver
> > somewhere as it's a separate piece of hardware from the camera module.
> > If it does not fit into any existing frameworks then it's best to have
> > it in a separate driver with the camera driver.
>
> The register contains the following fields that control the two CSI2 PHYs
> (PHY1 and PHY2).
>
> 31 CAMERARX_CSI22_LANEENABLE2 PHY2 Lane 2 (CSI22_DX2, CSI22_DY2) Enable
> 30 CAMERARX_CSI22_LANEENABLE1 PHY2 Lane 1 (CSI22_DX1, CSI22_DY1) Enable
> 29 CAMERARX_CSI22_LANEENABLE0 PHY2 Lane 0 (CSI22_DX0, CSI22_DY0) Enable
> 28 CAMERARX_CSI21_LANEENABLE4 PHY1 Lane 4 (CSI21_DX4, CSI21_DY4) Enable
> 27 CAMERARX_CSI21_LANEENABLE3 PHY1 Lane 3 (CSI21_DX3, CSI21_DY3) Enable
> 26 CAMERARX_CSI21_LANEENABLE2 PHY1 Lane 2 (CSI21_DX2, CSI21_DY2) Enable
> 25 CAMERARX_CSI21_LANEENABLE1 PHY1 Lane 1 (CSI21_DX1, CSI21_DY1) Enable
> 24 CAMERARX_CSI21_LANEENABLE0 PHY1 Lane 0 (CSI21_DX0, CSI21_DY0) Enable
> 21 CAMERARX_CSI22_CTRLCLKEN PHY2 Clock Enable
> 20:19 CAMERARX_CSI22_CAMMODE PHY2 Mode (CCP2, CSI1, CSI2)
> 18 CAMERARX_CSI21_CTRLCLKEN PHY1 Clock Enable
> 17:16 CAMERARX_CSI21_CAMMODE PHY1 Mode (CCP2, CSI1, CSI2)
>
> Bits 18 and 21 could be exposed through CCF. Bits 24 to 31 enable/disable the
> CSI2 lanes, so it could be argued that they could be exposed through the
> pinctrl framework. However, they need to be configured independently, possibly
> at runtime. I'm thus not sure pinctrl would be a good idea. Bits 17:16 and
> 20:19 don't fit in existing frameworks.
OK thanks for the info. Sounds like drivers/phy might be the right location
for it then and then the phy driver can use the syscon regmap.
> Given that this register is specific to the ISS, I think handling it as a
> separate device through a separate driver would only complicate the
> implementation without any real benefit.
Even though it's one register, it shoud still be treated separately from
the camera driver. The problems with keeping the register access to the
control module in the camera driver are at least following:
1. They live in separate hardware modules that can be clocked separately
2. Doing a read-back to flush a posted write in one hardware module most
likely won't flush the write to other and that can lead into hard to
find mysterious bugs
3. If we ever have a common system control module driver, we need to
rewrite all the system control module register tinkering in the drivers
So it's best to try to use an existing framework for it. That avoids
tons of pain later on ;)
Regards,
Tony
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-13 5:30 ` Tony Lindgren
@ 2014-06-13 6:47 ` Laurent Pinchart
-1 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-13 6:47 UTC (permalink / raw)
To: Tony Lindgren
Cc: Arnd Bergmann, gregkh, Mauro Carvalho Chehab, linux-omap,
linux-media, linux-arm-kernel, arm
Hi Tony,
On Thursday 12 June 2014 22:30:44 Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 08:32]:
> > On Thursday 12 June 2014 08:15:35 Tony Lindgren wrote:
> >> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 07:52]:
> >>> On Wednesday 11 June 2014 07:47:54 Tony Lindgren wrote:
> >>>> These should just use either pinctrl-single.c instead for muxing.
> >>>> Or if they are not mux registers, we do have the syscon mapping
> >>>> available in omap4.dtsi that pbias-regulator.c is already using.
> >>>>
> >>>> Laurent, got any better ideas?
> >>>
> >>> The ISS driver needs to write a single register, which contains
> >>> several independent fields. They thus need to be controlled by a
> >>> single driver. Some of them might be considered to be related to
> >>> pinmuxing (although I disagree on that), others are certainly not
> >>> about muxing (there are clock gate bits for instance).
> >>>
> >>> Using the syscon mapping seems like the best option. I'll give it a
> >>> try.
> >>
> >> OK if it's not strictly pinctrl related then let's not use
> >> pinctrl-single,bits for it. You may be able to implement one or more
> >> framework drivers for it for pinctrl/regulator/clock/transceiver
> >> whatever that register is doing.
> >>
> >> In any case it's best to have that handling in a separate helper driver
> >> somewhere as it's a separate piece of hardware from the camera module.
> >> If it does not fit into any existing frameworks then it's best to have
> >> it in a separate driver with the camera driver.
> >
> > The register contains the following fields that control the two CSI2 PHYs
> > (PHY1 and PHY2).
> >
> > 31 CAMERARX_CSI22_LANEENABLE2 PHY2 Lane 2 (CSI22_DX2, CSI22_DY2)
> > Enable
> > 30 CAMERARX_CSI22_LANEENABLE1 PHY2 Lane 1 (CSI22_DX1, CSI22_DY1)
> > Enable
> > 29 CAMERARX_CSI22_LANEENABLE0 PHY2 Lane 0 (CSI22_DX0, CSI22_DY0)
> > Enable
> > 28 CAMERARX_CSI21_LANEENABLE4 PHY1 Lane 4 (CSI21_DX4, CSI21_DY4)
> > Enable
> > 27 CAMERARX_CSI21_LANEENABLE3 PHY1 Lane 3 (CSI21_DX3, CSI21_DY3)
> > Enable
> > 26 CAMERARX_CSI21_LANEENABLE2 PHY1 Lane 2 (CSI21_DX2, CSI21_DY2)
> > Enable
> > 25 CAMERARX_CSI21_LANEENABLE1 PHY1 Lane 1 (CSI21_DX1, CSI21_DY1)
> > Enable
> > 24 CAMERARX_CSI21_LANEENABLE0 PHY1 Lane 0 (CSI21_DX0, CSI21_DY0)
> > Enable
> > 21 CAMERARX_CSI22_CTRLCLKEN PHY2 Clock Enable
> > 20:19 CAMERARX_CSI22_CAMMODE PHY2 Mode (CCP2, CSI1, CSI2)
> > 18 CAMERARX_CSI21_CTRLCLKEN PHY1 Clock Enable
> > 17:16 CAMERARX_CSI21_CAMMODE PHY1 Mode (CCP2, CSI1, CSI2)
> >
> > Bits 18 and 21 could be exposed through CCF. Bits 24 to 31 enable/disable
> > the CSI2 lanes, so it could be argued that they could be exposed through
> > the pinctrl framework. However, they need to be configured independently,
> > possibly at runtime. I'm thus not sure pinctrl would be a good idea. Bits
> > 17:16 and 20:19 don't fit in existing frameworks.
>
> OK thanks for the info. Sounds like drivers/phy might be the right location
> for it then and then the phy driver can use the syscon regmap.
>
> > Given that this register is specific to the ISS, I think handling it as a
> > separate device through a separate driver would only complicate the
> > implementation without any real benefit.
>
> Even though it's one register, it shoud still be treated separately from
> the camera driver. The problems with keeping the register access to the
> control module in the camera driver are at least following:
>
> 1. They live in separate hardware modules that can be clocked separately
Actually I don't think that's true. The CSI2 PHY is part of the camera device,
with all its registers but the one above in the camera device register space.
For some weird reason a couple of bits were pushed to the control module, but
that doesn't make the CSI2 PHY itself a separate device.
> 2. Doing a read-back to flush a posted write in one hardware module most
> likely won't flush the write to other and that can lead into hard to
> find mysterious bugs
The OMAP4 ISS driver can just read back the CAMERA_RX register, can't it ?
> 3. If we ever have a common system control module driver, we need to
> rewrite all the system control module register tinkering in the drivers
Sure, but that's already the case today, as the OMAP4 ISS driver already
accesses the control module register directly. I won't make that worse :-)
> So it's best to try to use an existing framework for it. That avoids tons of
> pain later on ;)
I agree, but I don't think the PHY framework would be the right abstraction.
As explained above the CSI2 PHY is part of the OMAP4 ISS, so modeling its
single control module register as a PHY would be a hack.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-13 6:47 ` Laurent Pinchart
0 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-13 6:47 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tony,
On Thursday 12 June 2014 22:30:44 Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 08:32]:
> > On Thursday 12 June 2014 08:15:35 Tony Lindgren wrote:
> >> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 07:52]:
> >>> On Wednesday 11 June 2014 07:47:54 Tony Lindgren wrote:
> >>>> These should just use either pinctrl-single.c instead for muxing.
> >>>> Or if they are not mux registers, we do have the syscon mapping
> >>>> available in omap4.dtsi that pbias-regulator.c is already using.
> >>>>
> >>>> Laurent, got any better ideas?
> >>>
> >>> The ISS driver needs to write a single register, which contains
> >>> several independent fields. They thus need to be controlled by a
> >>> single driver. Some of them might be considered to be related to
> >>> pinmuxing (although I disagree on that), others are certainly not
> >>> about muxing (there are clock gate bits for instance).
> >>>
> >>> Using the syscon mapping seems like the best option. I'll give it a
> >>> try.
> >>
> >> OK if it's not strictly pinctrl related then let's not use
> >> pinctrl-single,bits for it. You may be able to implement one or more
> >> framework drivers for it for pinctrl/regulator/clock/transceiver
> >> whatever that register is doing.
> >>
> >> In any case it's best to have that handling in a separate helper driver
> >> somewhere as it's a separate piece of hardware from the camera module.
> >> If it does not fit into any existing frameworks then it's best to have
> >> it in a separate driver with the camera driver.
> >
> > The register contains the following fields that control the two CSI2 PHYs
> > (PHY1 and PHY2).
> >
> > 31 CAMERARX_CSI22_LANEENABLE2 PHY2 Lane 2 (CSI22_DX2, CSI22_DY2)
> > Enable
> > 30 CAMERARX_CSI22_LANEENABLE1 PHY2 Lane 1 (CSI22_DX1, CSI22_DY1)
> > Enable
> > 29 CAMERARX_CSI22_LANEENABLE0 PHY2 Lane 0 (CSI22_DX0, CSI22_DY0)
> > Enable
> > 28 CAMERARX_CSI21_LANEENABLE4 PHY1 Lane 4 (CSI21_DX4, CSI21_DY4)
> > Enable
> > 27 CAMERARX_CSI21_LANEENABLE3 PHY1 Lane 3 (CSI21_DX3, CSI21_DY3)
> > Enable
> > 26 CAMERARX_CSI21_LANEENABLE2 PHY1 Lane 2 (CSI21_DX2, CSI21_DY2)
> > Enable
> > 25 CAMERARX_CSI21_LANEENABLE1 PHY1 Lane 1 (CSI21_DX1, CSI21_DY1)
> > Enable
> > 24 CAMERARX_CSI21_LANEENABLE0 PHY1 Lane 0 (CSI21_DX0, CSI21_DY0)
> > Enable
> > 21 CAMERARX_CSI22_CTRLCLKEN PHY2 Clock Enable
> > 20:19 CAMERARX_CSI22_CAMMODE PHY2 Mode (CCP2, CSI1, CSI2)
> > 18 CAMERARX_CSI21_CTRLCLKEN PHY1 Clock Enable
> > 17:16 CAMERARX_CSI21_CAMMODE PHY1 Mode (CCP2, CSI1, CSI2)
> >
> > Bits 18 and 21 could be exposed through CCF. Bits 24 to 31 enable/disable
> > the CSI2 lanes, so it could be argued that they could be exposed through
> > the pinctrl framework. However, they need to be configured independently,
> > possibly at runtime. I'm thus not sure pinctrl would be a good idea. Bits
> > 17:16 and 20:19 don't fit in existing frameworks.
>
> OK thanks for the info. Sounds like drivers/phy might be the right location
> for it then and then the phy driver can use the syscon regmap.
>
> > Given that this register is specific to the ISS, I think handling it as a
> > separate device through a separate driver would only complicate the
> > implementation without any real benefit.
>
> Even though it's one register, it shoud still be treated separately from
> the camera driver. The problems with keeping the register access to the
> control module in the camera driver are at least following:
>
> 1. They live in separate hardware modules that can be clocked separately
Actually I don't think that's true. The CSI2 PHY is part of the camera device,
with all its registers but the one above in the camera device register space.
For some weird reason a couple of bits were pushed to the control module, but
that doesn't make the CSI2 PHY itself a separate device.
> 2. Doing a read-back to flush a posted write in one hardware module most
> likely won't flush the write to other and that can lead into hard to
> find mysterious bugs
The OMAP4 ISS driver can just read back the CAMERA_RX register, can't it ?
> 3. If we ever have a common system control module driver, we need to
> rewrite all the system control module register tinkering in the drivers
Sure, but that's already the case today, as the OMAP4 ISS driver already
accesses the control module register directly. I won't make that worse :-)
> So it's best to try to use an existing framework for it. That avoids tons of
> pain later on ;)
I agree, but I don't think the PHY framework would be the right abstraction.
As explained above the CSI2 PHY is part of the OMAP4 ISS, so modeling its
single control module register as a PHY would be a hack.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-13 6:47 ` Laurent Pinchart
@ 2014-06-13 7:53 ` Tony Lindgren
-1 siblings, 0 replies; 45+ messages in thread
From: Tony Lindgren @ 2014-06-13 7:53 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Arnd Bergmann, gregkh, Mauro Carvalho Chehab, linux-omap,
linux-media, linux-arm-kernel, arm
* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 23:48]:
> On Thursday 12 June 2014 22:30:44 Tony Lindgren wrote:
> >
> > 1. They live in separate hardware modules that can be clocked separately
>
> Actually I don't think that's true. The CSI2 PHY is part of the camera device,
> with all its registers but the one above in the camera device register space.
> For some weird reason a couple of bits were pushed to the control module, but
> that doesn't make the CSI2 PHY itself a separate device.
Yes they are separate. Anything in the system control module is
a separate hardware module from the other devices. So in this case
the CSI2 PHY is part of the system control module, not the camera
module.
> > 2. Doing a read-back to flush a posted write in one hardware module most
> > likely won't flush the write to other and that can lead into hard to
> > find mysterious bugs
>
> The OMAP4 ISS driver can just read back the CAMERA_RX register, can't it ?
Right, but you would have to do readbacks both from the phy register and
camera register to ensure writes get written. It's best to keep the
logic completely separate especially considering that they can be
clocked separately.
> > 3. If we ever have a common system control module driver, we need to
> > rewrite all the system control module register tinkering in the drivers
>
> Sure, but that's already the case today, as the OMAP4 ISS driver already
> accesses the control module register directly. I won't make that worse :-)
Well it's in staging for a reason :)
> > So it's best to try to use an existing framework for it. That avoids tons of
> > pain later on ;)
>
> I agree, but I don't think the PHY framework would be the right abstraction.
> As explained above the CSI2 PHY is part of the OMAP4 ISS, so modeling its
> single control module register as a PHY would be a hack.
Well that register belongs to the system control module, not the
camera module. It's not like the camera IO space is out of registers
or something! :)
We're already handling similar control module phy cases, see for
example drivers/phy/phy-omap-control.c. Maybe you have most of the
code already there?
Regards,
Tony
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-13 7:53 ` Tony Lindgren
0 siblings, 0 replies; 45+ messages in thread
From: Tony Lindgren @ 2014-06-13 7:53 UTC (permalink / raw)
To: linux-arm-kernel
* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 23:48]:
> On Thursday 12 June 2014 22:30:44 Tony Lindgren wrote:
> >
> > 1. They live in separate hardware modules that can be clocked separately
>
> Actually I don't think that's true. The CSI2 PHY is part of the camera device,
> with all its registers but the one above in the camera device register space.
> For some weird reason a couple of bits were pushed to the control module, but
> that doesn't make the CSI2 PHY itself a separate device.
Yes they are separate. Anything in the system control module is
a separate hardware module from the other devices. So in this case
the CSI2 PHY is part of the system control module, not the camera
module.
> > 2. Doing a read-back to flush a posted write in one hardware module most
> > likely won't flush the write to other and that can lead into hard to
> > find mysterious bugs
>
> The OMAP4 ISS driver can just read back the CAMERA_RX register, can't it ?
Right, but you would have to do readbacks both from the phy register and
camera register to ensure writes get written. It's best to keep the
logic completely separate especially considering that they can be
clocked separately.
> > 3. If we ever have a common system control module driver, we need to
> > rewrite all the system control module register tinkering in the drivers
>
> Sure, but that's already the case today, as the OMAP4 ISS driver already
> accesses the control module register directly. I won't make that worse :-)
Well it's in staging for a reason :)
> > So it's best to try to use an existing framework for it. That avoids tons of
> > pain later on ;)
>
> I agree, but I don't think the PHY framework would be the right abstraction.
> As explained above the CSI2 PHY is part of the OMAP4 ISS, so modeling its
> single control module register as a PHY would be a hack.
Well that register belongs to the system control module, not the
camera module. It's not like the camera IO space is out of registers
or something! :)
We're already handling similar control module phy cases, see for
example drivers/phy/phy-omap-control.c. Maybe you have most of the
code already there?
Regards,
Tony
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-13 7:53 ` Tony Lindgren
@ 2014-06-13 10:29 ` Laurent Pinchart
-1 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-13 10:29 UTC (permalink / raw)
To: Tony Lindgren
Cc: Arnd Bergmann, gregkh, Mauro Carvalho Chehab, linux-omap,
linux-media, linux-arm-kernel, arm
Hi Tony,
On Friday 13 June 2014 00:53:25 Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 23:48]:
> > On Thursday 12 June 2014 22:30:44 Tony Lindgren wrote:
> > > 1. They live in separate hardware modules that can be clocked separately
> >
> > Actually I don't think that's true. The CSI2 PHY is part of the camera
> > device, with all its registers but the one above in the camera device
> > register space. For some weird reason a couple of bits were pushed to the
> > control module, but that doesn't make the CSI2 PHY itself a separate
> > device.
>
> Yes they are separate. Anything in the system control module is
> a separate hardware module from the other devices. So in this case
> the CSI2 PHY is part of the system control module, not the camera
> module.
Section 8.2.3 ("ISS CSI2 PHY") of the OMAP4460 TRM (revision AA) documents the
CSI2 PHY is being part of the ISS, with three PHY registers in the ISS
register space (not counting the PHY interrupt and status bits in several
other ISS registers) and one register in the system control module register
space. It's far from clear which power domain(s) is (are) involved.
> > > 2. Doing a read-back to flush a posted write in one hardware module most
> > > likely won't flush the write to other and that can lead into hard to
> > > find mysterious bugs
> >
> > The OMAP4 ISS driver can just read back the CAMERA_RX register, can't it ?
>
> Right, but you would have to do readbacks both from the phy register and
> camera register to ensure writes get written. It's best to keep the
> logic completely separate especially considering that they can be
> clocked separately.
>
> > > 3. If we ever have a common system control module driver, we need to
> > > rewrite all the system control module register tinkering in the
> > > drivers
> >
> > Sure, but that's already the case today, as the OMAP4 ISS driver already
> > accesses the control module register directly. I won't make that worse :-)
>
> Well it's in staging for a reason :)
>
> > > So it's best to try to use an existing framework for it. That avoids
> > > tons of pain later on ;)
> >
> > I agree, but I don't think the PHY framework would be the right
> > abstraction. As explained above the CSI2 PHY is part of the OMAP4 ISS, so
> > modeling its single control module register as a PHY would be a hack.
>
> Well that register belongs to the system control module, not the
> camera module. It's not like the camera IO space is out of registers
> or something! :)
The PHY has 3 registers in the ISS I/O space and one register in the control
module I/O space. I have no idea why they've split it that way. The clock
enable bits are especially "interested", the source clock (CAM_PHY_CTRL_FCLK)
comes from the ISS as documented in section 8.1.1 ("ISS Integration"), is
gated by the control module (the gated clock is called CTRLCLK) and then goes
back to the ISS CSI2 PHY (it's mentioned in the CSI2 PHY "REGISTER1"
documentation).
> We're already handling similar control module phy cases, see for
> example drivers/phy/phy-omap-control.c. Maybe you have most of the
> code already there?
I'm afraid not. For PHYs that are in the system control module that solution
is perfectly fine, but the CSI2 PHY isn't (or at least not all of it).
I would be fine with writing a separate PHY driver if the PHY was completely
separate. As the documentation doesn't make it clear which part of the
hardware belongs to which module, matching the software implementation with an
unknown hardware implementation would be pretty difficult :-)
If you have a couple of minutes to spare and can look at the CSI2 PHY
documentation in the TRM, you might be more successful than me figuring out
how the hardware is implemented.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-13 10:29 ` Laurent Pinchart
0 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-13 10:29 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tony,
On Friday 13 June 2014 00:53:25 Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 23:48]:
> > On Thursday 12 June 2014 22:30:44 Tony Lindgren wrote:
> > > 1. They live in separate hardware modules that can be clocked separately
> >
> > Actually I don't think that's true. The CSI2 PHY is part of the camera
> > device, with all its registers but the one above in the camera device
> > register space. For some weird reason a couple of bits were pushed to the
> > control module, but that doesn't make the CSI2 PHY itself a separate
> > device.
>
> Yes they are separate. Anything in the system control module is
> a separate hardware module from the other devices. So in this case
> the CSI2 PHY is part of the system control module, not the camera
> module.
Section 8.2.3 ("ISS CSI2 PHY") of the OMAP4460 TRM (revision AA) documents the
CSI2 PHY is being part of the ISS, with three PHY registers in the ISS
register space (not counting the PHY interrupt and status bits in several
other ISS registers) and one register in the system control module register
space. It's far from clear which power domain(s) is (are) involved.
> > > 2. Doing a read-back to flush a posted write in one hardware module most
> > > likely won't flush the write to other and that can lead into hard to
> > > find mysterious bugs
> >
> > The OMAP4 ISS driver can just read back the CAMERA_RX register, can't it ?
>
> Right, but you would have to do readbacks both from the phy register and
> camera register to ensure writes get written. It's best to keep the
> logic completely separate especially considering that they can be
> clocked separately.
>
> > > 3. If we ever have a common system control module driver, we need to
> > > rewrite all the system control module register tinkering in the
> > > drivers
> >
> > Sure, but that's already the case today, as the OMAP4 ISS driver already
> > accesses the control module register directly. I won't make that worse :-)
>
> Well it's in staging for a reason :)
>
> > > So it's best to try to use an existing framework for it. That avoids
> > > tons of pain later on ;)
> >
> > I agree, but I don't think the PHY framework would be the right
> > abstraction. As explained above the CSI2 PHY is part of the OMAP4 ISS, so
> > modeling its single control module register as a PHY would be a hack.
>
> Well that register belongs to the system control module, not the
> camera module. It's not like the camera IO space is out of registers
> or something! :)
The PHY has 3 registers in the ISS I/O space and one register in the control
module I/O space. I have no idea why they've split it that way. The clock
enable bits are especially "interested", the source clock (CAM_PHY_CTRL_FCLK)
comes from the ISS as documented in section 8.1.1 ("ISS Integration"), is
gated by the control module (the gated clock is called CTRLCLK) and then goes
back to the ISS CSI2 PHY (it's mentioned in the CSI2 PHY "REGISTER1"
documentation).
> We're already handling similar control module phy cases, see for
> example drivers/phy/phy-omap-control.c. Maybe you have most of the
> code already there?
I'm afraid not. For PHYs that are in the system control module that solution
is perfectly fine, but the CSI2 PHY isn't (or at least not all of it).
I would be fine with writing a separate PHY driver if the PHY was completely
separate. As the documentation doesn't make it clear which part of the
hardware belongs to which module, matching the software implementation with an
unknown hardware implementation would be pretty difficult :-)
If you have a couple of minutes to spare and can look at the CSI2 PHY
documentation in the TRM, you might be more successful than me figuring out
how the hardware is implemented.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-13 10:29 ` Laurent Pinchart
@ 2014-06-13 11:10 ` Tony Lindgren
-1 siblings, 0 replies; 45+ messages in thread
From: Tony Lindgren @ 2014-06-13 11:10 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Arnd Bergmann, gregkh, Mauro Carvalho Chehab, linux-omap,
linux-media, linux-arm-kernel, arm
* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140613 03:30]:
> Hi Tony,
>
> On Friday 13 June 2014 00:53:25 Tony Lindgren wrote:
> > * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 23:48]:
> > > On Thursday 12 June 2014 22:30:44 Tony Lindgren wrote:
> > > > 1. They live in separate hardware modules that can be clocked separately
> > >
> > > Actually I don't think that's true. The CSI2 PHY is part of the camera
> > > device, with all its registers but the one above in the camera device
> > > register space. For some weird reason a couple of bits were pushed to the
> > > control module, but that doesn't make the CSI2 PHY itself a separate
> > > device.
> >
> > Yes they are separate. Anything in the system control module is
> > a separate hardware module from the other devices. So in this case
> > the CSI2 PHY is part of the system control module, not the camera
> > module.
>
> Section 8.2.3 ("ISS CSI2 PHY") of the OMAP4460 TRM (revision AA) documents the
> CSI2 PHY is being part of the ISS, with three PHY registers in the ISS
> register space (not counting the PHY interrupt and status bits in several
> other ISS registers) and one register in the system control module register
> space. It's far from clear which power domain(s) is (are) involved.
OK I see. The register in the system control module just contains some
pin and clock related resources for the phy.
> > > > 2. Doing a read-back to flush a posted write in one hardware module most
> > > > likely won't flush the write to other and that can lead into hard to
> > > > find mysterious bugs
> > >
> > > The OMAP4 ISS driver can just read back the CAMERA_RX register, can't it ?
> >
> > Right, but you would have to do readbacks both from the phy register and
> > camera register to ensure writes get written. It's best to keep the
> > logic completely separate especially considering that they can be
> > clocked separately.
> >
> > > > 3. If we ever have a common system control module driver, we need to
> > > > rewrite all the system control module register tinkering in the
> > > > drivers
> > >
> > > Sure, but that's already the case today, as the OMAP4 ISS driver already
> > > accesses the control module register directly. I won't make that worse :-)
> >
> > Well it's in staging for a reason :)
> >
> > > > So it's best to try to use an existing framework for it. That avoids
> > > > tons of pain later on ;)
> > >
> > > I agree, but I don't think the PHY framework would be the right
> > > abstraction. As explained above the CSI2 PHY is part of the OMAP4 ISS, so
> > > modeling its single control module register as a PHY would be a hack.
> >
> > Well that register belongs to the system control module, not the
> > camera module. It's not like the camera IO space is out of registers
> > or something! :)
>
> The PHY has 3 registers in the ISS I/O space and one register in the control
> module I/O space. I have no idea why they've split it that way. The clock
> enable bits are especially "interested", the source clock (CAM_PHY_CTRL_FCLK)
> comes from the ISS as documented in section 8.1.1 ("ISS Integration"), is
> gated by the control module (the gated clock is called CTRLCLK) and then goes
> back to the ISS CSI2 PHY (it's mentioned in the CSI2 PHY "REGISTER1"
> documentation).
>
> > We're already handling similar control module phy cases, see for
> > example drivers/phy/phy-omap-control.c. Maybe you have most of the
> > code already there?
>
> I'm afraid not. For PHYs that are in the system control module that solution
> is perfectly fine, but the CSI2 PHY isn't (or at least not all of it).
>
> I would be fine with writing a separate PHY driver if the PHY was completely
> separate. As the documentation doesn't make it clear which part of the
> hardware belongs to which module, matching the software implementation with an
> unknown hardware implementation would be pretty difficult :-)
Yeah it seems the phy driver would still have to use the pin resources
in the system control module.
> If you have a couple of minutes to spare and can look at the CSI2 PHY
> documentation in the TRM, you might be more successful than me figuring out
> how the hardware is implemented.
Took a look and it seems the phy is split into two parts. So probably
using the syscon mapping for the register in scm are is a good start.
At least then there's some protection from drivers tinkering directly
with the system control modules.
Maybe s ee what drivers/regulator/pbias-regulator.c is doing with
syscon to see if that works? Moving that to some phy driver later on
should be trivial if needed :)
Regards,
Tony
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-13 11:10 ` Tony Lindgren
0 siblings, 0 replies; 45+ messages in thread
From: Tony Lindgren @ 2014-06-13 11:10 UTC (permalink / raw)
To: linux-arm-kernel
* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140613 03:30]:
> Hi Tony,
>
> On Friday 13 June 2014 00:53:25 Tony Lindgren wrote:
> > * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 23:48]:
> > > On Thursday 12 June 2014 22:30:44 Tony Lindgren wrote:
> > > > 1. They live in separate hardware modules that can be clocked separately
> > >
> > > Actually I don't think that's true. The CSI2 PHY is part of the camera
> > > device, with all its registers but the one above in the camera device
> > > register space. For some weird reason a couple of bits were pushed to the
> > > control module, but that doesn't make the CSI2 PHY itself a separate
> > > device.
> >
> > Yes they are separate. Anything in the system control module is
> > a separate hardware module from the other devices. So in this case
> > the CSI2 PHY is part of the system control module, not the camera
> > module.
>
> Section 8.2.3 ("ISS CSI2 PHY") of the OMAP4460 TRM (revision AA) documents the
> CSI2 PHY is being part of the ISS, with three PHY registers in the ISS
> register space (not counting the PHY interrupt and status bits in several
> other ISS registers) and one register in the system control module register
> space. It's far from clear which power domain(s) is (are) involved.
OK I see. The register in the system control module just contains some
pin and clock related resources for the phy.
> > > > 2. Doing a read-back to flush a posted write in one hardware module most
> > > > likely won't flush the write to other and that can lead into hard to
> > > > find mysterious bugs
> > >
> > > The OMAP4 ISS driver can just read back the CAMERA_RX register, can't it ?
> >
> > Right, but you would have to do readbacks both from the phy register and
> > camera register to ensure writes get written. It's best to keep the
> > logic completely separate especially considering that they can be
> > clocked separately.
> >
> > > > 3. If we ever have a common system control module driver, we need to
> > > > rewrite all the system control module register tinkering in the
> > > > drivers
> > >
> > > Sure, but that's already the case today, as the OMAP4 ISS driver already
> > > accesses the control module register directly. I won't make that worse :-)
> >
> > Well it's in staging for a reason :)
> >
> > > > So it's best to try to use an existing framework for it. That avoids
> > > > tons of pain later on ;)
> > >
> > > I agree, but I don't think the PHY framework would be the right
> > > abstraction. As explained above the CSI2 PHY is part of the OMAP4 ISS, so
> > > modeling its single control module register as a PHY would be a hack.
> >
> > Well that register belongs to the system control module, not the
> > camera module. It's not like the camera IO space is out of registers
> > or something! :)
>
> The PHY has 3 registers in the ISS I/O space and one register in the control
> module I/O space. I have no idea why they've split it that way. The clock
> enable bits are especially "interested", the source clock (CAM_PHY_CTRL_FCLK)
> comes from the ISS as documented in section 8.1.1 ("ISS Integration"), is
> gated by the control module (the gated clock is called CTRLCLK) and then goes
> back to the ISS CSI2 PHY (it's mentioned in the CSI2 PHY "REGISTER1"
> documentation).
>
> > We're already handling similar control module phy cases, see for
> > example drivers/phy/phy-omap-control.c. Maybe you have most of the
> > code already there?
>
> I'm afraid not. For PHYs that are in the system control module that solution
> is perfectly fine, but the CSI2 PHY isn't (or at least not all of it).
>
> I would be fine with writing a separate PHY driver if the PHY was completely
> separate. As the documentation doesn't make it clear which part of the
> hardware belongs to which module, matching the software implementation with an
> unknown hardware implementation would be pretty difficult :-)
Yeah it seems the phy driver would still have to use the pin resources
in the system control module.
> If you have a couple of minutes to spare and can look at the CSI2 PHY
> documentation in the TRM, you might be more successful than me figuring out
> how the hardware is implemented.
Took a look and it seems the phy is split into two parts. So probably
using the syscon mapping for the register in scm are is a good start.
At least then there's some protection from drivers tinkering directly
with the system control modules.
Maybe s ee what drivers/regulator/pbias-regulator.c is doing with
syscon to see if that works? Moving that to some phy driver later on
should be trivial if needed :)
Regards,
Tony
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [media] staging: allow omap4iss to be modular
2014-06-13 11:10 ` Tony Lindgren
@ 2014-06-13 13:17 ` Laurent Pinchart
-1 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-13 13:17 UTC (permalink / raw)
To: Tony Lindgren
Cc: Arnd Bergmann, gregkh, Mauro Carvalho Chehab, linux-omap,
linux-media, linux-arm-kernel, arm
Hi Tony,
On Friday 13 June 2014 04:10:12 Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140613 03:30]:
> > On Friday 13 June 2014 00:53:25 Tony Lindgren wrote:
> > > * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 23:48]:
> > > > On Thursday 12 June 2014 22:30:44 Tony Lindgren wrote:
> > > > > 1. They live in separate hardware modules that can be clocked
> > > > > separately
> > > >
> > > > Actually I don't think that's true. The CSI2 PHY is part of the camera
> > > > device, with all its registers but the one above in the camera device
> > > > register space. For some weird reason a couple of bits were pushed to
> > > > the control module, but that doesn't make the CSI2 PHY itself a
> > > > separate device.
> > >
> > > Yes they are separate. Anything in the system control module is
> > > a separate hardware module from the other devices. So in this case
> > > the CSI2 PHY is part of the system control module, not the camera
> > > module.
> >
> > Section 8.2.3 ("ISS CSI2 PHY") of the OMAP4460 TRM (revision AA) documents
> > the CSI2 PHY is being part of the ISS, with three PHY registers in the
> > ISS register space (not counting the PHY interrupt and status bits in
> > several other ISS registers) and one register in the system control
> > module register space. It's far from clear which power domain(s) is (are)
> > involved.
>
> OK I see. The register in the system control module just contains some
> pin and clock related resources for the phy.
And the configuration of the PHY mode (CCP2, CSI1 or CSI2). It really seems
like random bits :-)
> > > > > 2. Doing a read-back to flush a posted write in one hardware module
> > > > > most likely won't flush the write to other and that can lead into
> > > > > hard to find mysterious bugs
> > > >
> > > > The OMAP4 ISS driver can just read back the CAMERA_RX register, can't
> > > > it ?
> > >
> > > Right, but you would have to do readbacks both from the phy register and
> > > camera register to ensure writes get written. It's best to keep the
> > > logic completely separate especially considering that they can be
> > > clocked separately.
> > >
> > > > > 3. If we ever have a common system control module driver, we need to
> > > > > rewrite all the system control module register tinkering in the
> > > > > drivers
> > > >
> > > > Sure, but that's already the case today, as the OMAP4 ISS driver
> > > > already accesses the control module register directly. I won't make
> > > > that worse :-)
> > >
> > > Well it's in staging for a reason :)
> > >
> > > > > So it's best to try to use an existing framework for it. That avoids
> > > > > tons of pain later on ;)
> > > >
> > > > I agree, but I don't think the PHY framework would be the right
> > > > abstraction. As explained above the CSI2 PHY is part of the OMAP4 ISS,
> > > > so modeling its single control module register as a PHY would be a
> > > > hack.
> > >
> > > Well that register belongs to the system control module, not the
> > > camera module. It's not like the camera IO space is out of registers
> > > or something! :)
> >
> > The PHY has 3 registers in the ISS I/O space and one register in the
> > control module I/O space. I have no idea why they've split it that way.
> > The clock enable bits are especially "interested", the source clock
> > (CAM_PHY_CTRL_FCLK) comes from the ISS as documented in section 8.1.1
> > ("ISS Integration"), is gated by the control module (the gated clock is
> > called CTRLCLK) and then goes back to the ISS CSI2 PHY (it's mentioned in
> > the CSI2 PHY "REGISTER1" documentation).
> >
> > > We're already handling similar control module phy cases, see for
> > > example drivers/phy/phy-omap-control.c. Maybe you have most of the
> > > code already there?
> >
> > I'm afraid not. For PHYs that are in the system control module that
> > solution is perfectly fine, but the CSI2 PHY isn't (or at least not all
> > of it).
> >
> > I would be fine with writing a separate PHY driver if the PHY was
> > completely separate. As the documentation doesn't make it clear which
> > part of the hardware belongs to which module, matching the software
> > implementation with an unknown hardware implementation would be pretty
> > difficult :-)
>
> Yeah it seems the phy driver would still have to use the pin resources
> in the system control module.
>
> > If you have a couple of minutes to spare and can look at the CSI2 PHY
> > documentation in the TRM, you might be more successful than me figuring
> > out how the hardware is implemented.
>
> Took a look and it seems the phy is split into two parts. So probably
> using the syscon mapping for the register in scm are is a good start.
> At least then there's some protection from drivers tinkering directly
> with the system control modules.
That's my plan.
> Maybe s ee what drivers/regulator/pbias-regulator.c is doing with
> syscon to see if that works? Moving that to some phy driver later on
> should be trivial if needed :)
I'll have a look, but I'm not sure whether the same approach will be possible.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [media] staging: allow omap4iss to be modular
@ 2014-06-13 13:17 ` Laurent Pinchart
0 siblings, 0 replies; 45+ messages in thread
From: Laurent Pinchart @ 2014-06-13 13:17 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tony,
On Friday 13 June 2014 04:10:12 Tony Lindgren wrote:
> * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140613 03:30]:
> > On Friday 13 June 2014 00:53:25 Tony Lindgren wrote:
> > > * Laurent Pinchart <laurent.pinchart@ideasonboard.com> [140612 23:48]:
> > > > On Thursday 12 June 2014 22:30:44 Tony Lindgren wrote:
> > > > > 1. They live in separate hardware modules that can be clocked
> > > > > separately
> > > >
> > > > Actually I don't think that's true. The CSI2 PHY is part of the camera
> > > > device, with all its registers but the one above in the camera device
> > > > register space. For some weird reason a couple of bits were pushed to
> > > > the control module, but that doesn't make the CSI2 PHY itself a
> > > > separate device.
> > >
> > > Yes they are separate. Anything in the system control module is
> > > a separate hardware module from the other devices. So in this case
> > > the CSI2 PHY is part of the system control module, not the camera
> > > module.
> >
> > Section 8.2.3 ("ISS CSI2 PHY") of the OMAP4460 TRM (revision AA) documents
> > the CSI2 PHY is being part of the ISS, with three PHY registers in the
> > ISS register space (not counting the PHY interrupt and status bits in
> > several other ISS registers) and one register in the system control
> > module register space. It's far from clear which power domain(s) is (are)
> > involved.
>
> OK I see. The register in the system control module just contains some
> pin and clock related resources for the phy.
And the configuration of the PHY mode (CCP2, CSI1 or CSI2). It really seems
like random bits :-)
> > > > > 2. Doing a read-back to flush a posted write in one hardware module
> > > > > most likely won't flush the write to other and that can lead into
> > > > > hard to find mysterious bugs
> > > >
> > > > The OMAP4 ISS driver can just read back the CAMERA_RX register, can't
> > > > it ?
> > >
> > > Right, but you would have to do readbacks both from the phy register and
> > > camera register to ensure writes get written. It's best to keep the
> > > logic completely separate especially considering that they can be
> > > clocked separately.
> > >
> > > > > 3. If we ever have a common system control module driver, we need to
> > > > > rewrite all the system control module register tinkering in the
> > > > > drivers
> > > >
> > > > Sure, but that's already the case today, as the OMAP4 ISS driver
> > > > already accesses the control module register directly. I won't make
> > > > that worse :-)
> > >
> > > Well it's in staging for a reason :)
> > >
> > > > > So it's best to try to use an existing framework for it. That avoids
> > > > > tons of pain later on ;)
> > > >
> > > > I agree, but I don't think the PHY framework would be the right
> > > > abstraction. As explained above the CSI2 PHY is part of the OMAP4 ISS,
> > > > so modeling its single control module register as a PHY would be a
> > > > hack.
> > >
> > > Well that register belongs to the system control module, not the
> > > camera module. It's not like the camera IO space is out of registers
> > > or something! :)
> >
> > The PHY has 3 registers in the ISS I/O space and one register in the
> > control module I/O space. I have no idea why they've split it that way.
> > The clock enable bits are especially "interested", the source clock
> > (CAM_PHY_CTRL_FCLK) comes from the ISS as documented in section 8.1.1
> > ("ISS Integration"), is gated by the control module (the gated clock is
> > called CTRLCLK) and then goes back to the ISS CSI2 PHY (it's mentioned in
> > the CSI2 PHY "REGISTER1" documentation).
> >
> > > We're already handling similar control module phy cases, see for
> > > example drivers/phy/phy-omap-control.c. Maybe you have most of the
> > > code already there?
> >
> > I'm afraid not. For PHYs that are in the system control module that
> > solution is perfectly fine, but the CSI2 PHY isn't (or at least not all
> > of it).
> >
> > I would be fine with writing a separate PHY driver if the PHY was
> > completely separate. As the documentation doesn't make it clear which
> > part of the hardware belongs to which module, matching the software
> > implementation with an unknown hardware implementation would be pretty
> > difficult :-)
>
> Yeah it seems the phy driver would still have to use the pin resources
> in the system control module.
>
> > If you have a couple of minutes to spare and can look at the CSI2 PHY
> > documentation in the TRM, you might be more successful than me figuring
> > out how the hardware is implemented.
>
> Took a look and it seems the phy is split into two parts. So probably
> using the syscon mapping for the register in scm are is a good start.
> At least then there's some protection from drivers tinkering directly
> with the system control modules.
That's my plan.
> Maybe s ee what drivers/regulator/pbias-regulator.c is doing with
> syscon to see if that works? Moving that to some phy driver later on
> should be trivial if needed :)
I'll have a look, but I'm not sure whether the same approach will be possible.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 45+ messages in thread
end of thread, other threads:[~2014-06-13 13:17 UTC | newest]
Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-11 14:35 [PATCH] [media] staging: allow omap4iss to be modular Arnd Bergmann
2014-06-11 14:35 ` Arnd Bergmann
2014-06-11 14:35 ` Arnd Bergmann
2014-06-11 14:42 ` Nishanth Menon
2014-06-11 14:42 ` Nishanth Menon
2014-06-11 14:42 ` Nishanth Menon
2014-06-11 14:49 ` Arnd Bergmann
2014-06-11 14:49 ` Arnd Bergmann
2014-06-11 14:53 ` Nishanth Menon
2014-06-11 14:53 ` Nishanth Menon
2014-06-11 14:53 ` Nishanth Menon
2014-06-11 15:02 ` Tony Lindgren
2014-06-11 15:02 ` Tony Lindgren
2014-06-12 14:12 ` Laurent Pinchart
2014-06-12 14:12 ` Laurent Pinchart
2014-06-12 14:15 ` Arnd Bergmann
2014-06-12 14:15 ` Arnd Bergmann
2014-06-12 14:25 ` Greg KH
2014-06-12 14:25 ` Greg KH
2014-06-12 14:28 ` Arnd Bergmann
2014-06-12 14:28 ` Arnd Bergmann
2014-06-12 15:00 ` Laurent Pinchart
2014-06-12 15:00 ` Laurent Pinchart
2014-06-12 15:59 ` Greg KH
2014-06-12 15:59 ` Greg KH
2014-06-11 14:47 ` Tony Lindgren
2014-06-11 14:47 ` Tony Lindgren
2014-06-12 14:52 ` Laurent Pinchart
2014-06-12 14:52 ` Laurent Pinchart
2014-06-12 15:15 ` Tony Lindgren
2014-06-12 15:15 ` Tony Lindgren
2014-06-12 15:31 ` Laurent Pinchart
2014-06-12 15:31 ` Laurent Pinchart
2014-06-13 5:30 ` Tony Lindgren
2014-06-13 5:30 ` Tony Lindgren
2014-06-13 6:47 ` Laurent Pinchart
2014-06-13 6:47 ` Laurent Pinchart
2014-06-13 7:53 ` Tony Lindgren
2014-06-13 7:53 ` Tony Lindgren
2014-06-13 10:29 ` Laurent Pinchart
2014-06-13 10:29 ` Laurent Pinchart
2014-06-13 11:10 ` Tony Lindgren
2014-06-13 11:10 ` Tony Lindgren
2014-06-13 13:17 ` Laurent Pinchart
2014-06-13 13:17 ` Laurent Pinchart
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.