Re: [PATCH v3 00/23] Unrestricted media entity ID range support

2015-12-28 Thread Mauro Carvalho Chehab 
Em Sun, 27 Dec 2015 19:11:36 +0200
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> On Wednesday 23 December 2015 10:32:42 Mauro Carvalho Chehab wrote:
> > Em Wed, 16 Dec 2015 16:03:01 +0200 Sakari Ailus escreveu:
> > > On Wed, Dec 16, 2015 at 03:32:15PM +0200, Sakari Ailus wrote:
> > > > This is the third version of the unrestricted media entity ID range
> > > > support set. I've taken Mauro's comments into account and fixed a number
> > > > of bugs as well (omap3isp memory leak and omap4iss stream start).
> > > 
> > > Javier: Mauro told me you might have OMAP4 hardware. Would you be able to
> > > test the OMAP4 ISS with these patches?
> > > 
> > > Thanks.
> > 
> > Sakari,
> > 
> > Testing with OMAP4 is not possible. The driver is broken: it doesn't
> > support DT, and the required pdata definition is missing.
> 
> What do you mean by missing ? struct iss_platform_data is defined in 
> include/media/omap4iss.h.
> 

As this driver is not DT, the platform data has to be part of the Kernel
tree for the driver to work. However, there are no board-specific data nor
any documentation about how to do that inside the Kernel tree. 
That means  that this driver won't work without some OOT patch.

So, on its current state, it is broken. It should either be
converted to DT and have the needed board definitions added to the
existing dts files or be removed.

Another possible approach would be to have a patch like the one
Javier and I tried to craft by adding the needed platform data
into arch/arm/mach-omap2/pdata-quirks.c, but I guess it is better
to just use DT instead.

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 00/23] Unrestricted media entity ID range support

2015-12-27 Thread Laurent Pinchart 
Hi Mauro,

On Wednesday 23 December 2015 10:32:42 Mauro Carvalho Chehab wrote:
> Em Wed, 16 Dec 2015 16:03:01 +0200 Sakari Ailus escreveu:
> > On Wed, Dec 16, 2015 at 03:32:15PM +0200, Sakari Ailus wrote:
> > > This is the third version of the unrestricted media entity ID range
> > > support set. I've taken Mauro's comments into account and fixed a number
> > > of bugs as well (omap3isp memory leak and omap4iss stream start).
> > 
> > Javier: Mauro told me you might have OMAP4 hardware. Would you be able to
> > test the OMAP4 ISS with these patches?
> > 
> > Thanks.
> 
> Sakari,
> 
> Testing with OMAP4 is not possible. The driver is broken: it doesn't
> support DT, and the required pdata definition is missing.

What do you mean by missing ? struct iss_platform_data is defined in 
include/media/omap4iss.h.

> Both Javier and I tried to fix it in the last couple days, in order to test
> it with a PandaBoard. We came with the enclosed patch, but it is still
> incomplete. Based on what's written on this e-mail:
>https://www.mail-archive.com/linux-media@vger.kernel.org/msg89247.html
> 
> It seems that this is an already known issue.
> 
> So, I'm considering this driver as BROKEN. Not much sense on doing any
> tests on it, while this doesn't get fixed.
> 
> Regards,
> Mauro
> 
> PS.: With the enclosed patch, I got this error:
>   [0.267639] platform omap4iss: failed to claim resource 2
> 
> But, even if I comment out the platform code that returns this error,
> there are still other missing things:
>   [7.131622] omap4iss omap4iss: Unable to get iss_fck clock info
>   [7.137878] omap4iss omap4iss: Unable to get clocks
> 
> ---
> 
> ARM: add a pdata quirks for OMAP4 panda camera
> 
> This is a hack to make it to believe that the pandaboard
> has a camera.
> 
> 
> diff --git a/arch/arm/mach-omap2/pdata-quirks.c
> b/arch/arm/mach-omap2/pdata-quirks.c index 1dfe34654c43..998bb6936dc0
> 100644
> --- a/arch/arm/mach-omap2/pdata-quirks.c
> +++ b/arch/arm/mach-omap2/pdata-quirks.c
> @@ -36,6 +36,8 @@
>  #include "soc.h"
>  #include "hsmmc.h"
> 
> +#include "../../../drivers/staging/media/omap4iss/iss.h"
> +
>  struct pdata_init {
>   const char *compatible;
>   void (*fn)(void);
> @@ -408,6 +410,124 @@ static void __init t410_abort_init(void)
>  }
>  #endif
> 
> +#ifdef CONFIG_ARCH_OMAP4
> +
> +static struct resource panda_iss_resource[] = {
> + {
> + .start = 0x5200,
> + .end = 0x5200 + 0x100,
> + .name = "top",
> + .flags = IORESOURCE_MEM,
> + }, {
> + .start = 0x52001000,
> + .end = 0x52001000 + 0x170,
> + .name = "csi2_a_regs1",
> + .flags = IORESOURCE_MEM,
> + }, {
> + .start = 0x52001170,
> + .end = 0x52001170 + 0x020,
> + .name = "camerarx_core1",
> + .flags = IORESOURCE_MEM,
> + }, {
> + .start = 0x52001400,
> + .end = 0x52001400 + 0x170,
> + .name = "csi2_b_regs1",
> + .flags = IORESOURCE_MEM,
> + }, {
> + .start = 0x52001570,
> + .end = 0x52001570 + 0x020,
> + .name = "camerarx_core2",
> + .flags = IORESOURCE_MEM,
> + }, {
> + .start = 0x52002000,
> + .end = 0x52002000 + 0x200,
> + .name = "bte",
> + .flags = IORESOURCE_MEM,
> + }, {
> + .start = 0x5201,
> + .end = 0x5201 + 0x0a0,
> + .name = "isp_sys1",
> + .flags = IORESOURCE_MEM,
> + }, {
> + .start = 0x52010400,
> + .end = 0x52010400 + 0x400,
> + .name = "isp_resizer",
> + .flags = IORESOURCE_MEM,
> + }, {
> + .start = 0x52010800,
> + .end = 0x52010800 + 0x800,
> + .name = "isp_ipipe",
> + .flags = IORESOURCE_MEM,
> + }, {
> + .start = 0x52011000,
> + .end = 0x52011000 + 0x200,
> + .name = "isp_isif",
> + .flags = IORESOURCE_MEM,
> + }, {
> + .start = 0x52011200,
> + .end = 0x52011200 + 0x080,
> + .name = "isp_ipipeif",
> + .flags = IORESOURCE_MEM,
> + }
> +};
> +
> +static struct i2c_board_info panda_camera_i2c_device = {
> + I2C_BOARD_INFO("smia", 0x10),
> +};
> +
> +static struct iss_subdev_i2c_board_info panda_camera_subdevs[] = {
> + {
> + .board_info = _camera_i2c_device,
> + .i2c_adapter_id = 3,
> + },
> +};
> +
> +static struct iss_v4l2_subdevs_group iss_subdevs[] = {
> + {
> + .subdevs = panda_camera_subdevs,
> + .interface = ISS_INTERFACE_CSI2A_PHY1,
> + .bus = {
> + .csi2 = {
> + .lanecfg = {
> + .clk = {
> + .pol = 0,
> +

Re: [PATCH v3 00/23] Unrestricted media entity ID range support

2015-12-27 Thread Javier Martinez Canillas 
Hello Laurent,

On 12/27/2015 02:11 PM, Laurent Pinchart wrote:
> Hi Mauro,
> 
> On Wednesday 23 December 2015 10:32:42 Mauro Carvalho Chehab wrote:
>> Em Wed, 16 Dec 2015 16:03:01 +0200 Sakari Ailus escreveu:
>>> On Wed, Dec 16, 2015 at 03:32:15PM +0200, Sakari Ailus wrote:
 This is the third version of the unrestricted media entity ID range
 support set. I've taken Mauro's comments into account and fixed a number
 of bugs as well (omap3isp memory leak and omap4iss stream start).
>>>
>>> Javier: Mauro told me you might have OMAP4 hardware. Would you be able to
>>> test the OMAP4 ISS with these patches?
>>>
>>> Thanks.
>>
>> Sakari,
>>
>> Testing with OMAP4 is not possible. The driver is broken: it doesn't
>> support DT, and the required pdata definition is missing.
> 
> What do you mean by missing ? struct iss_platform_data is defined in 
> include/media/omap4iss.h.
>

That's true but at the very least the omap4iss hwmod is broken in mainline
as you mentioned in the linux-media thread that Mauro shared below.

I think what Mauro meant is that there isn't an omap4 board supported in
mainline that makes use of the iss platform data structures. So testing the
driver in a popular omap4 board such as the pandaboard isn't possible without
adding plumbing board code. And even in that case, the driver fails to probe
due a missing iss_fck clock as Mauro mentioned in his boot log below as well.

I know is not a requirement for a driver to have mainline users but without
DT bindings, using this driver will not be possible sooner rather than later
once the mach-omap2 pdata-quirks.c workaround is removed from mainline since
the omap4 board files were deleted almost 3 years ago (3.11 according to git).
 
>> Both Javier and I tried to fix it in the last couple days, in order to test
>> it with a PandaBoard. We came with the enclosed patch, but it is still
>> incomplete. Based on what's written on this e-mail:
>>   https://www.mail-archive.com/linux-media@vger.kernel.org/msg89247.html
>>
>> It seems that this is an already known issue.
>>
>> So, I'm considering this driver as BROKEN. Not much sense on doing any
>> tests on it, while this doesn't get fixed.
>>
>> Regards,
>> Mauro
>>
>> PS.: With the enclosed patch, I got this error:
>>  [0.267639] platform omap4iss: failed to claim resource 2
>>
>> But, even if I comment out the platform code that returns this error,
>> there are still other missing things:
>>  [7.131622] omap4iss omap4iss: Unable to get iss_fck clock info
>>  [7.137878] omap4iss omap4iss: Unable to get clocks
>>
>> ---
>>
>> ARM: add a pdata quirks for OMAP4 panda camera
>>
>> This is a hack to make it to believe that the pandaboard
>> has a camera.
>>
>>
>> diff --git a/arch/arm/mach-omap2/pdata-quirks.c
>> b/arch/arm/mach-omap2/pdata-quirks.c index 1dfe34654c43..998bb6936dc0
>> 100644
>> --- a/arch/arm/mach-omap2/pdata-quirks.c
>> +++ b/arch/arm/mach-omap2/pdata-quirks.c
>> @@ -36,6 +36,8 @@
>>  #include "soc.h"
>>  #include "hsmmc.h"
>>
>> +#include "../../../drivers/staging/media/omap4iss/iss.h"
>> +
>>  struct pdata_init {
>>  const char *compatible;
>>  void (*fn)(void);
>> @@ -408,6 +410,124 @@ static void __init t410_abort_init(void)
>>  }
>>  #endif
>>
>> +#ifdef CONFIG_ARCH_OMAP4
>> +
>> +static struct resource panda_iss_resource[] = {
>> +{
>> +.start = 0x5200,
>> +.end = 0x5200 + 0x100,
>> +.name = "top",
>> +.flags = IORESOURCE_MEM,
>> +}, {
>> +.start = 0x52001000,
>> +.end = 0x52001000 + 0x170,
>> +.name = "csi2_a_regs1",
>> +.flags = IORESOURCE_MEM,
>> +}, {
>> +.start = 0x52001170,
>> +.end = 0x52001170 + 0x020,
>> +.name = "camerarx_core1",
>> +.flags = IORESOURCE_MEM,
>> +}, {
>> +.start = 0x52001400,
>> +.end = 0x52001400 + 0x170,
>> +.name = "csi2_b_regs1",
>> +.flags = IORESOURCE_MEM,
>> +}, {
>> +.start = 0x52001570,
>> +.end = 0x52001570 + 0x020,
>> +.name = "camerarx_core2",
>> +.flags = IORESOURCE_MEM,
>> +}, {
>> +.start = 0x52002000,
>> +.end = 0x52002000 + 0x200,
>> +.name = "bte",
>> +.flags = IORESOURCE_MEM,
>> +}, {
>> +.start = 0x5201,
>> +.end = 0x5201 + 0x0a0,
>> +.name = "isp_sys1",
>> +.flags = IORESOURCE_MEM,
>> +}, {
>> +.start = 0x52010400,
>> +.end = 0x52010400 + 0x400,
>> +.name = "isp_resizer",
>> +.flags = IORESOURCE_MEM,
>> +}, {
>> +.start = 0x52010800,
>> +.end = 0x52010800 + 0x800,
>> +.name = "isp_ipipe",
>> +.flags = IORESOURCE_MEM,
>> +}, {
>> +.start = 0x52011000,
>> +.end = 0x52011000 + 0x200,
>> +

Re: [PATCH v3 00/23] Unrestricted media entity ID range support

2015-12-23 Thread Mauro Carvalho Chehab 
Em Wed, 16 Dec 2015 16:03:01 +0200
Sakari Ailus  escreveu:

> Hi Javier,
> 
> On Wed, Dec 16, 2015 at 03:32:15PM +0200, Sakari Ailus wrote:
> > This is the third version of the unrestricted media entity ID range
> > support set. I've taken Mauro's comments into account and fixed a number
> > of bugs as well (omap3isp memory leak and omap4iss stream start).
> 
> Javier: Mauro told me you might have OMAP4 hardware. Would you be able to
> test the OMAP4 ISS with these patches?
> 
> Thanks.
> 

Sakari,

Testing with OMAP4 is not possible. The driver is broken: it doesn't
support DT, and the required pdata definition is missing.

Both Javier and I tried to fix it in the last couple days, in order to test
it with a PandaBoard. We came with the enclosed patch, but it is still 
incomplete. Based on what's written on this e-mail:
 https://www.mail-archive.com/linux-media@vger.kernel.org/msg89247.html

It seems that this is an already known issue.

So, I'm considering this driver as BROKEN. Not much sense on doing any
tests on it, while this doesn't get fixed.

Regards,
Mauro

PS.: With the enclosed patch, I got this error:
[0.267639] platform omap4iss: failed to claim resource 2

But, even if I comment out the platform code that returns this error,
there are still other missing things:
[7.131622] omap4iss omap4iss: Unable to get iss_fck clock info
[7.137878] omap4iss omap4iss: Unable to get clocks

---

ARM: add a pdata quirks for OMAP4 panda camera

This is a hack to make it to believe that the pandaboard
has a camera.


diff --git a/arch/arm/mach-omap2/pdata-quirks.c 
b/arch/arm/mach-omap2/pdata-quirks.c
index 1dfe34654c43..998bb6936dc0 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -36,6 +36,8 @@
 #include "soc.h"
 #include "hsmmc.h"
 
+#include "../../../drivers/staging/media/omap4iss/iss.h"
+
 struct pdata_init {
const char *compatible;
void (*fn)(void);
@@ -408,6 +410,124 @@ static void __init t410_abort_init(void)
 }
 #endif
 
+#ifdef CONFIG_ARCH_OMAP4
+
+static struct resource panda_iss_resource[] = {
+   {
+   .start = 0x5200,
+   .end = 0x5200 + 0x100,
+   .name = "top",
+   .flags = IORESOURCE_MEM,
+   }, {
+   .start = 0x52001000,
+   .end = 0x52001000 + 0x170,
+   .name = "csi2_a_regs1",
+   .flags = IORESOURCE_MEM,
+   }, {
+   .start = 0x52001170,
+   .end = 0x52001170 + 0x020,
+   .name = "camerarx_core1",
+   .flags = IORESOURCE_MEM,
+   }, {
+   .start = 0x52001400,
+   .end = 0x52001400 + 0x170,
+   .name = "csi2_b_regs1",
+   .flags = IORESOURCE_MEM,
+   }, {
+   .start = 0x52001570,
+   .end = 0x52001570 + 0x020,
+   .name = "camerarx_core2",
+   .flags = IORESOURCE_MEM,
+   }, {
+   .start = 0x52002000,
+   .end = 0x52002000 + 0x200,
+   .name = "bte",
+   .flags = IORESOURCE_MEM,
+   }, {
+   .start = 0x5201,
+   .end = 0x5201 + 0x0a0,
+   .name = "isp_sys1",
+   .flags = IORESOURCE_MEM,
+   }, {
+   .start = 0x52010400,
+   .end = 0x52010400 + 0x400,
+   .name = "isp_resizer",
+   .flags = IORESOURCE_MEM,
+   }, {
+   .start = 0x52010800,
+   .end = 0x52010800 + 0x800,
+   .name = "isp_ipipe",
+   .flags = IORESOURCE_MEM,
+   }, {
+   .start = 0x52011000,
+   .end = 0x52011000 + 0x200,
+   .name = "isp_isif",
+   .flags = IORESOURCE_MEM,
+   }, {
+   .start = 0x52011200,
+   .end = 0x52011200 + 0x080,
+   .name = "isp_ipipeif",
+   .flags = IORESOURCE_MEM,
+   }
+};
+
+static struct i2c_board_info panda_camera_i2c_device = {
+   I2C_BOARD_INFO("smia", 0x10),
+};
+
+static struct iss_subdev_i2c_board_info panda_camera_subdevs[] = {
+   {
+   .board_info = _camera_i2c_device,
+   .i2c_adapter_id = 3,
+   },
+};
+
+static struct iss_v4l2_subdevs_group iss_subdevs[] = {
+   {
+   .subdevs = panda_camera_subdevs,
+   .interface = ISS_INTERFACE_CSI2A_PHY1,
+   .bus = {
+   .csi2 = {
+   .lanecfg = {
+   .clk = {
+   .pol = 0,
+   .pos = 2,
+   },
+   .data[0] = {
+   .pol = 0,
+   .pos =