diff mbox

smiapp: add CCP2 support

Message ID 20170208131127.GA29237@amd (mailing list archive)
State New, archived
Headers show

Commit Message

Pavel Machek Feb. 8, 2017, 1:11 p.m. UTC 
Add support for CCP2 connected SMIA sensors as found
on the Nokia N900.

Signed-off-by: Sebastian Reichel <sre@kernel.org>
Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>

Comments

Sakari Ailus Feb. 11, 2017, 10:07 p.m. UTC | #1 
Thanks, Pavel! :-)

Besides this patch, what else is needed? The CSI-2 / CCP2 support is
missing in V4L2 OF at least. It'd be better to have this all in the same
set.

I pushed the two DT patches here:

<URL:https://git.linuxtv.org/sailus/media_tree.git/commit/?h=ccp2>

On Wed, Feb 08, 2017 at 02:11:27PM +0100, Pavel Machek wrote:
> 
> Add support for CCP2 connected SMIA sensors as found
> on the Nokia N900.
> 
> Signed-off-by: Sebastian Reichel <sre@kernel.org>
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> 
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
> index 44f8c7e..c217bc6 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -2997,13 +2997,19 @@ static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev)
>  	switch (bus_cfg->bus_type) {
>  	case V4L2_MBUS_CSI2:
>  		hwcfg->csi_signalling_mode = SMIAPP_CSI_SIGNALLING_MODE_CSI2;
> +		hwcfg->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
> +		break;
> +	case V4L2_MBUS_CCP2:
> +		hwcfg->csi_signalling_mode = (bus_cfg->bus.mipi_csi1.strobe) ?
> +		SMIAPP_CSI_SIGNALLING_MODE_CCP2_DATA_STROBE :
> +		SMIAPP_CSI_SIGNALLING_MODE_CCP2_DATA_CLOCK;
> +		hwcfg->lanes = 1;
>  		break;
> -		/* FIXME: add CCP2 support. */
>  	default:
> +		dev_err(dev, "unknown bus protocol\n");

It's rather unsupported --- V4L2 OF framework picks one from enum
v4l2_mbus_type. You might want to print the value, too. Up to you.

>  		goto out_err;
>  	}
>  
> -	hwcfg->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
>  	dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
>  
>  	/* NVM size is not mandatory */
> @@ -3017,8 +3023,8 @@ static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev)
>  		goto out_err;
>  	}
>  
> -	dev_dbg(dev, "nvm %d, clk %d, csi %d\n", hwcfg->nvm_size,
> -		hwcfg->ext_clk, hwcfg->csi_signalling_mode);
> +	dev_dbg(dev, "nvm %d, clk %d, mode %d\n",
> +		hwcfg->nvm_size, hwcfg->ext_clk, hwcfg->csi_signalling_mode);
>  
>  	if (!bus_cfg->nr_of_link_frequencies) {
>  		dev_warn(dev, "no link frequencies defined\n");
>
Pavel Machek Feb. 11, 2017, 11:22 p.m. UTC | #2 
Hi!

> Besides this patch, what else is needed? The CSI-2 / CCP2 support is
> missing in V4L2 OF at least. It'd be better to have this all in the same
> set.

Quite a lot of is needed.

> I pushed the two DT patches here:
> 
> <URL:https://git.linuxtv.org/sailus/media_tree.git/commit/?h=ccp2>

Thanks for a branch. If you could the two patches that look ok there,
it would mean less work for me, I could just mark those two as applied
here.

Core changes for CSI2 support are needed.

There are core changes in notifier locking, and subdev support.

I need video-bus-switch, at least for testing.

I need subdev support for omap3isp, so that we can attach flash and
focus devices.

Finally dts support on N900 can be enabled.

Thanks,

								Pavel
kernel test robot Feb. 12, 2017, 2:43 p.m. UTC | #3 
Hi Pavel,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.10-rc7 next-20170210]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Pavel-Machek/smiapp-add-CCP2-support/20170208-214729
base:   git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-it0-02122030 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/media/i2c/smiapp/smiapp-core.c: In function 'smiapp_get_hwconfig':
   drivers/media/i2c/smiapp/smiapp-core.c:2812:7: error: 'V4L2_MBUS_CCP2' undeclared (first use in this function)
     case V4L2_MBUS_CCP2:
          ^
   drivers/media/i2c/smiapp/smiapp-core.c:2812:7: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/media/i2c/smiapp/smiapp-core.c:2813:45: error: 'union <anonymous>' has no member named 'mipi_csi1'
      hwcfg->csi_signalling_mode = (bus_cfg->bus.mipi_csi1.strobe) ?
                                                ^

vim +2813 drivers/media/i2c/smiapp/smiapp-core.c

  2806	
  2807		switch (bus_cfg->bus_type) {
  2808		case V4L2_MBUS_CSI2:
  2809			hwcfg->csi_signalling_mode = SMIAPP_CSI_SIGNALLING_MODE_CSI2;
  2810			hwcfg->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
  2811			break;
> 2812		case V4L2_MBUS_CCP2:
> 2813			hwcfg->csi_signalling_mode = (bus_cfg->bus.mipi_csi1.strobe) ?
  2814			SMIAPP_CSI_SIGNALLING_MODE_CCP2_DATA_STROBE :
  2815			SMIAPP_CSI_SIGNALLING_MODE_CCP2_DATA_CLOCK;
  2816			hwcfg->lanes = 1;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Sakari Ailus Feb. 12, 2017, 10:10 p.m. UTC | #4 
Hi Pavel,

On Sun, Feb 12, 2017 at 12:22:58AM +0100, Pavel Machek wrote:
> Hi!
> 
> > Besides this patch, what else is needed? The CSI-2 / CCP2 support is
> > missing in V4L2 OF at least. It'd be better to have this all in the same
> > set.
> 
> Quite a lot of is needed.
> 
> > I pushed the two DT patches here:
> > 
> > <URL:https://git.linuxtv.org/sailus/media_tree.git/commit/?h=ccp2>
> 
> Thanks for a branch. If you could the two patches that look ok there,
> it would mean less work for me, I could just mark those two as applied
> here.

I think a verb could be missing from the sentence. :-) I'll send a pull
request for the entire set, containing more than just the DT changes. Feel
free to base yours on top of this.

A word of warning: I have patches to replace the V4L2 OF framework by V4L2
fwnode. The preliminary set (which is still missing V4L2 OF removal) is
here, I'll post a refresh soon:

<URL:http://www.spinics.net/lists/linux-media/msg106160.html>

Let's see what the order ends up to be in the end. If the fwnode set is
applicable first, then I'd like to rebase the lane parsing changes on top of
that rather than the other way around --- it's easier that way.

> 
> Core changes for CSI2 support are needed.

CCP2? We could get these and the smiapp and possibly also the omap3isp
patches in first, to avoid having to manage a large patchset. What do you
think?

The rest could come later.

> 
> There are core changes in notifier locking, and subdev support.
> 
> I need video-bus-switch, at least for testing.
> 
> I need subdev support for omap3isp, so that we can attach flash and
> focus devices.
> 
> Finally dts support on N900 can be enabled.

Yes! 8-)

I don't know if any euros were saved by using that video bus switch but it
sure has caused a lot of hassle (and perhaps some gray hair) for software
engineers. X-)
Pavel Machek Feb. 15, 2017, 10:48 a.m. UTC | #5 
Hi!

> > > I pushed the two DT patches here:
> > > 
> > > <URL:https://git.linuxtv.org/sailus/media_tree.git/commit/?h=ccp2>
> > 
> > Thanks for a branch. If you could the two patches that look ok there,
> > it would mean less work for me, I could just mark those two as applied
> > here.
> 
> I think a verb could be missing from the sentence. :-) I'll send a pull
> request for the entire set, containing more than just the DT changes. Feel
> free to base yours on top of this.
> 
> A word of warning: I have patches to replace the V4L2 OF framework by V4L2
> fwnode. The preliminary set (which is still missing V4L2 OF removal) is
> here, I'll post a refresh soon:
> 
> <URL:http://www.spinics.net/lists/linux-media/msg106160.html>
> 
> Let's see what the order ends up to be in the end. If the fwnode set is
> applicable first, then I'd like to rebase the lane parsing changes on top of
> that rather than the other way around --- it's easier that way.
> 
> > 
> > Core changes for CSI2 support are needed.
> 
> CCP2? We could get these and the smiapp and possibly also the omap3isp
> patches in first, to avoid having to manage a large patchset. What do you
> think?

Well... anything that reduces the ammount of patches I need to
maintain to keep camera working is welcome.

> > There are core changes in notifier locking, and subdev support.
> > 
> > I need video-bus-switch, at least for testing.
> > 
> > I need subdev support for omap3isp, so that we can attach flash and
> > focus devices.
> > 
> > Finally dts support on N900 can be enabled.
> 
> Yes! 8-)
> 
> I don't know if any euros were saved by using that video bus switch but it
> sure has caused a lot of hassle (and perhaps some gray hair) for software
> engineers. X-)

Yes, switch is one of the problems. OTOH... Nokia did a great thing by
creating phone with reasonable design...
									
									Pavel
diff mbox

Patch

diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
index 44f8c7e..c217bc6 100644
--- a/drivers/media/i2c/smiapp/smiapp-core.c
+++ b/drivers/media/i2c/smiapp/smiapp-core.c
@@ -2997,13 +2997,19 @@  static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev)
 	switch (bus_cfg->bus_type) {
 	case V4L2_MBUS_CSI2:
 		hwcfg->csi_signalling_mode = SMIAPP_CSI_SIGNALLING_MODE_CSI2;
+		hwcfg->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
+		break;
+	case V4L2_MBUS_CCP2:
+		hwcfg->csi_signalling_mode = (bus_cfg->bus.mipi_csi1.strobe) ?
+		SMIAPP_CSI_SIGNALLING_MODE_CCP2_DATA_STROBE :
+		SMIAPP_CSI_SIGNALLING_MODE_CCP2_DATA_CLOCK;
+		hwcfg->lanes = 1;
 		break;
-		/* FIXME: add CCP2 support. */
 	default:
+		dev_err(dev, "unknown bus protocol\n");
 		goto out_err;
 	}
 
-	hwcfg->lanes = bus_cfg->bus.mipi_csi2.num_data_lanes;
 	dev_dbg(dev, "lanes %u\n", hwcfg->lanes);
 
 	/* NVM size is not mandatory */
@@ -3017,8 +3023,8 @@  static struct smiapp_hwconfig *smiapp_get_hwconfig(struct device *dev)
 		goto out_err;
 	}
 
-	dev_dbg(dev, "nvm %d, clk %d, csi %d\n", hwcfg->nvm_size,
-		hwcfg->ext_clk, hwcfg->csi_signalling_mode);
+	dev_dbg(dev, "nvm %d, clk %d, mode %d\n",
+		hwcfg->nvm_size, hwcfg->ext_clk, hwcfg->csi_signalling_mode);
 
 	if (!bus_cfg->nr_of_link_frequencies) {
 		dev_warn(dev, "no link frequencies defined\n");