Skip to contentSkip to conversation

ARM: S5P: Add platform helpers for camera GPIO configuration

PATCH/RFC New Sylwester Nawrocki 2 participants 8 messages Feb 11, 2011 – Mar 10, 2011

Subsystems (3)
Arm (32-bit) Arm sub-architectures Arm Samsung Exynos
lore
Share & tools
lore

Sylwester Nawrocki

author
#0 lore raw

Hello,

the following patch introduces platform helpers for the peripheral parallel camera bus configuration in S5PV210 and S5PV310 SoCs. In those SoCs multiple camera host interface IP's are available and can be attached to any of two ITU-R parrallel bus for external sensor. That's why I didn't try to pass any platform device as a pointer to those helpers. This is an initial proposal and any comments are appreciated.

[PATCH] ARM: S5P: Add platform helpers for camera GPIO configuration

Created against for-next branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/kgene/linux-samsung.git

Regards,
--
Sylwester Nawrocki
Samsung Poland R&D Center

author
#1 lore raw

PATCH 1/1 ARM: S5P: Add platform helpers for camera GPIO configuration

Add functions for the parallel camera GPIO interface configuration on S5PV210 and S5PV310 SoCs.

Signed-off-by Sylwester Nawrocki <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>
---
 arch/arm/mach-s5pv210/Kconfig           |    5 +++
 arch/arm/mach-s5pv210/Makefile          |    1 +
 arch/arm/mach-s5pv210/setup-camera.c    |   53 +++++++++++++++++++++++++++++++
 arch/arm/mach-s5pv310/Kconfig           |    5 +++
 arch/arm/mach-s5pv310/Makefile          |    1 +
 arch/arm/mach-s5pv310/setup-camera.c    |   43 +++++++++++++++++++++++++
 arch/arm/plat-s5p/include/plat/camera.h |   26 +++++++++++++++
 7 files changed, 134 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-s5pv210/setup-camera.c
 create mode 100644 arch/arm/mach-s5pv310/setup-camera.c
 create mode 100644 arch/arm/plat-s5p/include/plat/camera.h
arch/arm/mach-s5pv210/Kconfig +5 -0
index 53aabef..300993a 100644
@@ -53,6 +53,11 @@ config S5PV210_SETUP_SDHCI_GPIO
 	help
 	  Common setup code for SDHCI gpio.
 
+config S5PV210_SETUP_CAMERA
+	bool
+	help
+	  Common setup code for the camera interfaces.
+
 menu "S5PC110 Machines"
 
 config MACH_AQUILA
arch/arm/mach-s5pv210/Makefile +1 -0
index ff1a0db..d6c9f0d 100644
@@ -30,6 +30,7 @@ obj-$(CONFIG_MACH_TORBRECK)	+= mach-torbreck.o
 obj-y				+= dev-audio.o
 obj-$(CONFIG_S3C64XX_DEV_SPI)	+= dev-spi.o
 
+obj-$(CONFIG_S5PV210_SETUP_CAMERA)	+= setup-camera.o
 obj-$(CONFIG_S5PV210_SETUP_FB_24BPP)	+= setup-fb-24bpp.o
 obj-$(CONFIG_S5PV210_SETUP_I2C1) 	+= setup-i2c1.o
 obj-$(CONFIG_S5PV210_SETUP_I2C2) 	+= setup-i2c2.o
arch/arm/mach-s5pv210/setup-camera.c new file +53 -0

53 lines not on this page · raw message

arch/arm/mach-s5pv310/Kconfig +5 -0
index b2a9acc..ccd1dc4 100644
@@ -71,6 +71,11 @@ config S5PV310_DEV_SYSMMU
 	help
 	  Common setup code for SYSTEM MMU in S5PV310
 
+config S5PV310_SETUP_CAMERA
+	bool
+	help
+	  Common setup code for the camera interfaces.
+
 # machine support
 
 menu "S5PC210 Machines"
arch/arm/mach-s5pv310/Makefile +1 -0
index 036fb38..c1d6577 100644
@@ -32,6 +32,7 @@ obj-y					+= dev-audio.o
 obj-$(CONFIG_S5PV310_DEV_PD)		+= dev-pd.o
 obj-$(CONFIG_S5PV310_DEV_SYSMMU)	+= dev-sysmmu.o
 
+obj-$(CONFIG_S5PV310_SETUP_CAMERA)	+= setup-camera.o
 obj-$(CONFIG_S5PV310_SETUP_I2C1)	+= setup-i2c1.o
 obj-$(CONFIG_S5PV310_SETUP_I2C2)	+= setup-i2c2.o
 obj-$(CONFIG_S5PV310_SETUP_I2C3)	+= setup-i2c3.o
arch/arm/mach-s5pv310/setup-camera.c new file +43 -0

43 lines not on this page · raw message

arch/arm/plat-s5p/include/plat/camera.h new file +26 -0
new file mode 100644
index 0000000..f7c66ec
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2011 Samsung Electronics Co., Ltd.
+ *
+ * S5P series camera interface helper functions
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef PLAT_S5P_CAMERA_H_
+#define PLAT_S5P_CAMERA_H_ __FILE__
+
+enum s5p_camif_id {
+	S5P_CAMIF_A,
+	S5P_CAMIF_B,
+};
+
+/**
+ * s5pvX10_camif_cfg_gpio - configure IO pins of the camera A/B interface
+ * @id: id of a camera gpio interface
+ */
+int s5pv210_camif_cfg_gpio(enum s5p_camif_id id);
+int s5pv310_camif_cfg_gpio(enum s5p_camif_id id);
+
+#endif /* PLAT_S5P_CAMERA_H_ */
Signature · 1 line
1.7.4
author
#2 lore raw

PATCH 1/1 ARM: S5P: Add platform helpers for camera GPIO configuration

Quoted 3 lines · Sylwester Nawrocki

Add functions for the parallel camera GPIO interface configuration on S5PV210 and S5PV310 SoCs.

Kukjin, are you OK with general concept of those? I would probably need to rework the patches after recent S5PV310 -> EXYNOS4 renaming. And move camera.h to arch/arm/plat-samsung/include/plat/?

What's your opinion?

Thanks,

Signature · 2 lines
Sylwester Nawrocki
Samsung Poland R&D Center
#3 lore raw

PATCH 1/1 ARM: S5P: Add platform helpers for camera GPIO configuration

Quoted 7 lines · Sylwester Nawrocki
Quoted 3 lines · Sylwester Nawrocki

Add functions for the parallel camera GPIO interface configuration on S5PV210 and S5PV310 SoCs.

Quoted 6 lines

Kukjin, are you OK with general concept of those? I would probably need to rework the patches after recent S5PV310 -> EXYNOS4 renaming. And move camera.h to arch/arm/plat-samsung/include/plat/?

What's your opinion?

Oh, missed...yeah need to re-work based on exynos4 :)
And you don't need to move it into plat-samsung/ because plat-s5p/ is used for Exynos also.

Please refer to my comments on it.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

#4 lore raw

PATCH 1/1 ARM: S5P: Add platform helpers for camera GPIO configuration

In reply to Sylwester Nawrocki
Quoted 6 lines · Sylwester Nawrocki

Add functions for the parallel camera GPIO interface configuration on S5PV210 and S5PV310 SoCs.

Signed-off-by Sylwester Nawrocki <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>

Hmm...typo?

Signed-off-by Kyungmin Park <[email protected]>
Quoted 19 lines
---
 arch/arm/mach-s5pv210/Kconfig           |    5 +++
 arch/arm/mach-s5pv210/Makefile          |    1 +
 arch/arm/mach-s5pv210/setup-camera.c    |   53
+++++++++++++++++++++++++++++++
 arch/arm/mach-s5pv310/Kconfig           |    5 +++
 arch/arm/mach-s5pv310/Makefile          |    1 +
 arch/arm/mach-s5pv310/setup-camera.c    |   43 +++++++++++++++++++++++++
 arch/arm/plat-s5p/include/plat/camera.h |   26 +++++++++++++++
 7 files changed, 134 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-s5pv210/setup-camera.c
 create mode 100644 arch/arm/mach-s5pv310/setup-camera.c
 create mode 100644 arch/arm/plat-s5p/include/plat/camera.h
arch/arm/mach-s5pv210/Kconfig +0 -0
index 53aabef..300993a 100644

arch/arm/mach-s5pv210/Kconfig config S5PV210_SETUP_SDHCI_GPIO · line 56

Quoted 4 lines
arch/arm/mach-s5pv210/Kconfig +1 -0
@@ -53,6 +53,11 @@ config S5PV210_SETUP_SDHCI_GPIO
 	help
 	  Common setup code for SDHCI gpio.
 
+config S5PV210_SETUP_CAMERA

How about "S5PV210_SETUP_FIMC"?

As you know, it belong to FIMC block which is written in datasheet. And this naming is more reasonable like SETUP_I2C...

Review

Quoted 8 lines
arch/arm/mach-s5pv210/Kconfig +4 -0
+	bool
+	help
+	  Common setup code for the camera interfaces.
+
 menu "S5PC110 Machines"
 
 config MACH_AQUILA
(quoted diff) +0 -0

b/arch/arm/mach-s5pv210/Makefile

arch/arm/mach-s5pv210/setup-camera.c

Quoted 17 lines

index ff1a0db..d6c9f0d 100644

arch/arm/mach-s5pv210/Makefile +1 -0
@@ -30,6 +30,7 @@ obj-$(CONFIG_MACH_TORBRECK)	+= mach-torbreck.o
 obj-y				+= dev-audio.o
 obj-$(CONFIG_S3C64XX_DEV_SPI)	+= dev-spi.o
 
+obj-$(CONFIG_S5PV210_SETUP_CAMERA)	+= setup-camera.o
 obj-$(CONFIG_S5PV210_SETUP_FB_24BPP)	+= setup-fb-24bpp.o
 obj-$(CONFIG_S5PV210_SETUP_I2C1) 	+= setup-i2c1.o
 obj-$(CONFIG_S5PV210_SETUP_I2C2) 	+= setup-i2c2.o
arch/arm/mach- renamed from arch/arm/mach-s5pv210/setup-camera.c +0 -0

s5pv210/setup-camera.c
new file mode 100644
index 0000000..e13c354

arch/arm/mach-s5pv210/setup-camera.c new file +0 -0

So setup-fimc.c

Quoted 17 lines
arch/arm/mach-s5pv210/setup-camera.c new file +16 -0
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2011 Samsung Electronics Co., Ltd.
+ *
+ * S5PV310 camera interface GPIO configuration.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/gpio.h>
+#include <plat/gpio-cfg.h>
+#include <plat/camera.h>
+
+/*
+ * Configure the camera parallel bus pins. The parallel bus can be
Quoted 4 lines

multiplexed
+ * with any FIMC entity. Even multiple FIMC entities are allowed to be attached
+ * to a particular (A or B) gpio interface. This function should be

called

Quoted 4 lines

from
+ * a board setup code.
+ */
+int s5pv210_camif_cfg_gpio(enum s5p_camif_id id)

int s5pv210_fimc_cfg_gpio...

arch/arm/mach-s5pv210/setup-camera.c line 31

Quoted 10 lines
arch/arm/mach-s5pv210/setup-camera.c new file +10 -0
+{
+	u32 gpio8, gpio5;
+	int ret;
+	int i = 5;
+
+	switch (id) {
+	case S5P_CAMIF_A:
+		gpio8 = S5PV210_GPE0(0);
+		gpio5 = S5PV210_GPE1(0);
+		break;

Blank line?

arch/arm/mach-s5pv210/setup-camera.c line 35

Quoted 4 lines
arch/arm/mach-s5pv210/setup-camera.c new file +4 -0
+	case S5P_CAMIF_B:
+		gpio8 = S5PV210_GPJ0(0);
+		gpio5 = S5PV210_GPJ1(0);
+		break;

Same...

arch/arm/mach-s5pv210/setup-camera.c line 47

Quoted 12 lines
arch/arm/mach-s5pv210/setup-camera.c new file +12 -0
+	default:
+		WARN(1, "id: %d\n", id);
+		return -EINVAL;
+	}
+
+	ret = s3c_gpio_cfgall_range(gpio8, 8, S3C_GPIO_SFN(2),
+				    S3C_GPIO_PULL_UP);
+	if (ret)
+		return ret;
+
+	ret = s3c_gpio_cfgall_range(gpio5, 5, S3C_GPIO_SFN(2),
+				    S3C_GPIO_PULL_UP);

Where is "return ret;" ?

arch/arm/mach-s5pv210/setup-camera.c line 51

Quoted 4 lines
arch/arm/mach-s5pv210/setup-camera.c new file +4 -0
+
+	while (i-- && !ret)
+		ret = s5p_gpio_set_drvstr(S5PV210_GPE1(i),
+					  S5P_GPIO_DRVSTR_LV4);

Basically drive strength depends on each board. So I think, should be removed here.

arch/arm/mach-s5pv310/Kconfig config S5PV310_DEV_SYSMMU · line 74

Quoted 11 lines
arch/arm/mach-s5pv210/setup-camera.c new file +2 -0
+	return ret;
+}
arch/arm/mach-s5pv310/Kconfig +1 -0
index b2a9acc..ccd1dc4 100644
@@ -71,6 +71,11 @@ config S5PV310_DEV_SYSMMU
 	help
 	  Common setup code for SYSTEM MMU in S5PV310
 
+config S5PV310_SETUP_CAMERA

S5PV310_SETUP_FIMC ?

Review

Quoted 8 lines
arch/arm/mach-s5pv310/Kconfig +4 -0
+	bool
+	help
+	  Common setup code for the camera interfaces.
+
 # machine support
 
 menu "S5PC210 Machines"
(quoted diff) +0 -0

b/arch/arm/mach-s5pv310/Makefile

arch/arm/mach-s5pv310/Makefile obj-y +=

Quoted 4 lines

index 036fb38..c1d6577 100644

arch/arm/mach-s5pv310/Makefile +0 -0
@@ -32,6 +32,7 @@ obj-y					+=

dev-audio.o

arch/arm/mach-s5pv310/setup-camera.c

Quoted 13 lines
arch/arm/mach-s5pv310/Makefile +1 -0
 obj-$(CONFIG_S5PV310_DEV_PD)		+= dev-pd.o
 obj-$(CONFIG_S5PV310_DEV_SYSMMU)	+= dev-sysmmu.o
 
+obj-$(CONFIG_S5PV310_SETUP_CAMERA)	+= setup-camera.o
 obj-$(CONFIG_S5PV310_SETUP_I2C1)	+= setup-i2c1.o
 obj-$(CONFIG_S5PV310_SETUP_I2C2)	+= setup-i2c2.o
 obj-$(CONFIG_S5PV310_SETUP_I2C3)	+= setup-i2c3.o
arch/arm/mach- renamed from arch/arm/mach-s5pv310/setup-camera.c +0 -0

s5pv310/setup-camera.c
new file mode 100644
index 0000000..8ab239e

arch/arm/mach-s5pv310/setup-camera.c new file +0 -0

Almost same.

(snip)

arch/arm/plat-s5p/include/plat/camera.h

Quoted 6 lines
arch/arm/plat- renamed from arch/arm/plat-s5p/include/plat/camera.h +0 -0

s5p/include/plat/camera.h
new file mode 100644
index 0000000..f7c66ec

arch/arm/plat-s5p/include/plat/camera.h new file +0 -0

How about camport.h?

Quoted 8 lines
arch/arm/plat-s5p/include/plat/camera.h new file +7 -0
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2011 Samsung Electronics Co., Ltd.
+ *
+ * S5P series camera interface helper functions
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as

arch/arm/plat-s5p/include/plat/camera.h line 17

Quoted 10 lines
arch/arm/plat-s5p/include/plat/camera.h new file +10 -0
+ * published by the Free Software Foundation.
+ */
+
+#ifndef PLAT_S5P_CAMERA_H_
+#define PLAT_S5P_CAMERA_H_ __FILE__
+
+enum s5p_camif_id {
+	S5P_CAMIF_A,
+	S5P_CAMIF_B,
+};

Because I think, "CAMPORT" is more clearly.

enum s5p_camport_id {
	S5P_CAMPORT_A,
	S5P_CAMPORT_B,
};

arch/arm/plat-s5p/include/plat/camera.h

Quoted 10 lines
arch/arm/plat-s5p/include/plat/camera.h new file +9 -1
+
+/**
+ * s5pvX10_camif_cfg_gpio - configure IO pins of the camera A/B interface
+ * @id: id of a camera gpio interface
+ */
+int s5pv210_camif_cfg_gpio(enum s5p_camif_id id);
+int s5pv310_camif_cfg_gpio(enum s5p_camif_id id);
+
+#endif /* PLAT_S5P_CAMERA_H_ */
--

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

author
#5 lore raw

PATCH 1/1 ARM: S5P: Add platform helpers for camera GPIO configuration

Quoted 9 lines · Kukjin Kim
Quoted 6 lines · Sylwester Nawrocki

Add functions for the parallel camera GPIO interface configuration on S5PV210 and S5PV310 SoCs.

Signed-off-by Sylwester Nawrocki <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>

Hmm...typo?

Err, no. Looks like someone needs a glasses here..

Quoted 24 lines
Signed-off-by Kyungmin Park <[email protected]>
Quoted 22 lines
---
 arch/arm/mach-s5pv210/Kconfig           |    5 +++
 arch/arm/mach-s5pv210/Makefile          |    1 +
 arch/arm/mach-s5pv210/setup-camera.c    |   53
+++++++++++++++++++++++++++++++
 arch/arm/mach-s5pv310/Kconfig           |    5 +++
 arch/arm/mach-s5pv310/Makefile          |    1 +
 arch/arm/mach-s5pv310/setup-camera.c    |   43 +++++++++++++++++++++++++
 arch/arm/plat-s5p/include/plat/camera.h |   26 +++++++++++++++
 7 files changed, 134 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-s5pv210/setup-camera.c
 create mode 100644 arch/arm/mach-s5pv310/setup-camera.c
 create mode 100644 arch/arm/plat-s5p/include/plat/camera.h
arch/arm/mach-s5pv210/Kconfig +0 -0
index 53aabef..300993a 100644
@@ -53,6 +53,11 @@ config S5PV210_SETUP_SDHCI_GPIO
 	help
 	  Common setup code for SDHCI gpio.
Quoted 6 lines

arch/arm/mach-s5pv210/Kconfig config S5PV210_SETUP_SDHCI_GPIO · line 56

Quoted 1 line
arch/arm/mach-s5pv210/Kconfig +1 -0
+config S5PV210_SETUP_CAMERA

How about "S5PV210_SETUP_FIMC"?

As you know, it belong to FIMC block which is written in datasheet. And this naming is more reasonable like SETUP_I2C...

Yeah, makes sense. Will change that.

Quoted 17 lines

Review

Quoted 9 lines
arch/arm/mach-s5pv210/Kconfig +4 -0
+	bool
+	help
+	  Common setup code for the camera interfaces.
+
 menu "S5PC110 Machines"
 
 config MACH_AQUILA
(quoted diff) +0 -0

b/arch/arm/mach-s5pv210/Makefile

Quoted 7 lines

index ff1a0db..d6c9f0d 100644

arch/arm/mach-s5pv210/Makefile +0 -0
@@ -30,6 +30,7 @@ obj-$(CONFIG_MACH_TORBRECK)	+= mach-torbreck.o
 obj-y				+= dev-audio.o
 obj-$(CONFIG_S3C64XX_DEV_SPI)	+= dev-spi.o
Quoted 12 lines

arch/arm/mach-s5pv210/setup-camera.c

Quoted 10 lines
arch/arm/mach-s5pv210/Makefile +1 -0
+obj-$(CONFIG_S5PV210_SETUP_CAMERA)	+= setup-camera.o
 obj-$(CONFIG_S5PV210_SETUP_FB_24BPP)	+= setup-fb-24bpp.o
 obj-$(CONFIG_S5PV210_SETUP_I2C1) 	+= setup-i2c1.o
 obj-$(CONFIG_S5PV210_SETUP_I2C2) 	+= setup-i2c2.o
arch/arm/mach- renamed from arch/arm/mach-s5pv210/setup-camera.c +0 -0

s5pv210/setup-camera.c
new file mode 100644
index 0000000..e13c354

arch/arm/mach-s5pv210/setup-camera.c new file +0 -0

So setup-fimc.c

Ok.

Quoted 23 lines
Quoted 18 lines
arch/arm/mach-s5pv210/setup-camera.c new file +16 -0
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2011 Samsung Electronics Co., Ltd.
+ *
+ * S5PV310 camera interface GPIO configuration.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/gpio.h>
+#include <plat/gpio-cfg.h>
+#include <plat/camera.h>
+
+/*
+ * Configure the camera parallel bus pins. The parallel bus can be
Quoted 4 lines

multiplexed
+ * with any FIMC entity. Even multiple FIMC entities are allowed to be attached
+ * to a particular (A or B) gpio interface. This function should be

called

Quoted 7 lines
Quoted 4 lines

from
+ * a board setup code.
+ */
+int s5pv210_camif_cfg_gpio(enum s5p_camif_id id)

int s5pv210_fimc_cfg_gpio...

Ok, that's fine with me.

Quoted 4 lines
Quoted 4 lines
arch/arm/mach-s5pv210/setup-camera.c new file +4 -0
+{
+	u32 gpio8, gpio5;
+	int ret;
+	int i = 5;
Quoted 8 lines

arch/arm/mach-s5pv210/setup-camera.c line 31

Quoted 6 lines
arch/arm/mach-s5pv210/setup-camera.c new file +6 -0
+
+	switch (id) {
+	case S5P_CAMIF_A:
+		gpio8 = S5PV210_GPE0(0);
+		gpio5 = S5PV210_GPE1(0);
+		break;

Blank line?

OK

Quoted 8 lines

arch/arm/mach-s5pv210/setup-camera.c line 35

Quoted 5 lines
arch/arm/mach-s5pv210/setup-camera.c new file +4 -0
+	case S5P_CAMIF_B:
+		gpio8 = S5PV210_GPJ0(0);
+		gpio5 = S5PV210_GPJ1(0);
+		break;

Same...

Quoted 14 lines

arch/arm/mach-s5pv210/setup-camera.c line 47

Quoted 12 lines
arch/arm/mach-s5pv210/setup-camera.c new file +12 -0
+	default:
+		WARN(1, "id: %d\n", id);
+		return -EINVAL;
+	}
+
+	ret = s3c_gpio_cfgall_range(gpio8, 8, S3C_GPIO_SFN(2),
+				    S3C_GPIO_PULL_UP);
+	if (ret)
+		return ret;
+
+	ret = s3c_gpio_cfgall_range(gpio5, 5, S3C_GPIO_SFN(2),
+				    S3C_GPIO_PULL_UP);

Where is "return ret;" ?

No need, it's being checked below, int the "while" conditional.

Quoted 8 lines

arch/arm/mach-s5pv210/setup-camera.c line 51

Quoted 5 lines
arch/arm/mach-s5pv210/setup-camera.c new file +4 -0
+
+	while (i-- && !ret)
+		ret = s5p_gpio_set_drvstr(S5PV210_GPE1(i),
+					  S5P_GPIO_DRVSTR_LV4);

Basically drive strength depends on each board. So I think, should be removed here.

Hmm, none of our boards works without this. And it might be hard to find bug when someone forgets about setting gpio drive strength. I had some trouble with this already. Without setting proper drive strength the image from sensor had been black or the video capture was not working at all.
I think it's worth to add a parameter to the above function as it is expected to fully configure the parallel camera bus pins. I prefer to have possibly everything which touches the camera bus pins in there.

How about:

int s5pv210_fimc_setup_gpio(enum s5p_camport_id id, bool use_drvstr,

			s5p_gpio_drvstr_t drvstr);

end then:
...
+
+ ret = s3c_gpio_cfgall_range(gpio5, 5, S3C_GPIO_SFN(2),
+ S3C_GPIO_PULL_UP);
+ if (!use_drvstr)
+ return ret;
+
+ while (i-- && !ret)
+ ret = s5p_gpio_set_drvstr(S5PV210_GPE1(i), drvstr);
?

Quoted 11 lines
Quoted 11 lines
arch/arm/mach-s5pv210/setup-camera.c new file +2 -0
+	return ret;
+}
arch/arm/mach-s5pv310/Kconfig +0 -0
index b2a9acc..ccd1dc4 100644
@@ -71,6 +71,11 @@ config S5PV310_DEV_SYSMMU
 	help
 	  Common setup code for SYSTEM MMU in S5PV310
Quoted 3 lines

arch/arm/mach-s5pv310/Kconfig config S5PV310_DEV_SYSMMU · line 74

Quoted 1 line
arch/arm/mach-s5pv310/Kconfig +1 -0
+config S5PV310_SETUP_CAMERA

S5PV310_SETUP_FIMC ?

OK.

Quoted 18 lines

Review

Quoted 9 lines
arch/arm/mach-s5pv310/Kconfig +4 -0
+	bool
+	help
+	  Common setup code for the camera interfaces.
+
 # machine support
 
 menu "S5PC210 Machines"
(quoted diff) +0 -0

b/arch/arm/mach-s5pv310/Makefile

arch/arm/mach-s5pv310/Makefile obj-y +=

Quoted 4 lines

index 036fb38..c1d6577 100644

arch/arm/mach-s5pv310/Makefile +0 -0
@@ -32,6 +32,7 @@ obj-y					+=

dev-audio.o

Quoted 3 lines
arch/arm/mach-s5pv310/Makefile +0 -0
 obj-$(CONFIG_S5PV310_DEV_PD)		+= dev-pd.o
 obj-$(CONFIG_S5PV310_DEV_SYSMMU)	+= dev-sysmmu.o
Quoted 12 lines

arch/arm/mach-s5pv310/setup-camera.c

Quoted 10 lines
arch/arm/mach-s5pv310/Makefile +1 -0
+obj-$(CONFIG_S5PV310_SETUP_CAMERA)	+= setup-camera.o
 obj-$(CONFIG_S5PV310_SETUP_I2C1)	+= setup-i2c1.o
 obj-$(CONFIG_S5PV310_SETUP_I2C2)	+= setup-i2c2.o
 obj-$(CONFIG_S5PV310_SETUP_I2C3)	+= setup-i2c3.o
arch/arm/mach- renamed from arch/arm/mach-s5pv310/setup-camera.c +0 -0

s5pv310/setup-camera.c
new file mode 100644
index 0000000..8ab239e

arch/arm/mach-s5pv310/setup-camera.c new file +0 -0

Almost same.

For sure it's similar. I followed how setup-i2c[N].c are done. Should I merge camera port setup into setup-fimc.c under plat-s5p and use preprocessor to switch between ARCHs? It doesn't seem right..

Quoted 11 lines

(snip)

arch/arm/plat-s5p/include/plat/camera.h

Quoted 6 lines
arch/arm/plat- renamed from arch/arm/plat-s5p/include/plat/camera.h +0 -0

s5p/include/plat/camera.h
new file mode 100644
index 0000000..f7c66ec

arch/arm/plat-s5p/include/plat/camera.h new file +0 -0

How about camport.h?

Yeah, sounds better.

Quoted 22 lines
Quoted 9 lines
arch/arm/plat-s5p/include/plat/camera.h new file +7 -0
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2011 Samsung Electronics Co., Ltd.
+ *
+ * S5P series camera interface helper functions
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as

arch/arm/plat-s5p/include/plat/camera.h line 17

Quoted 10 lines
arch/arm/plat-s5p/include/plat/camera.h new file +10 -0
+ * published by the Free Software Foundation.
+ */
+
+#ifndef PLAT_S5P_CAMERA_H_
+#define PLAT_S5P_CAMERA_H_ __FILE__
+
+enum s5p_camif_id {
+	S5P_CAMIF_A,
+	S5P_CAMIF_B,
+};

Because I think, "CAMPORT" is more clearly.

Quoted 5 lines
enum s5p_camport_id {
	S5P_CAMPORT_A,
	S5P_CAMPORT_B,
};

Ok, agreed.

Quoted 11 lines

arch/arm/plat-s5p/include/plat/camera.h

Quoted 11 lines
arch/arm/plat-s5p/include/plat/camera.h new file +9 -1
+
+/**
+ * s5pvX10_camif_cfg_gpio - configure IO pins of the camera A/B interface
+ * @id: id of a camera gpio interface
+ */
+int s5pv210_camif_cfg_gpio(enum s5p_camif_id id);
+int s5pv310_camif_cfg_gpio(enum s5p_camif_id id);
+
+#endif /* PLAT_S5P_CAMERA_H_ */
--

Regards,

Signature · 2 lines
Sylwester Nawrocki
Samsung Poland R&D Center
author
#6 lore raw

PATCH v2 1/1 ARM: S5P: Add platform helpers for camera GPIO configuration

In reply to Kukjin Kim

Add functions for configuration of the parallel camera bus pins on S5PV210 and Exynos4 SoC.

Signed-off-by Sylwester Nawrocki <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>

---

Hello,

this is a corrected version of platform helpers for the peripheral parallel camera bus configuration in S5PV210 and Exynos4 SoCs.

Changes since v1:
- Renamed files: camera.h/camport.h, setup-camera.c/setup-fimc.c
- Replaced "camif" with "fimc" in function and data type names
- Removed driver strength setup (it turned out than only the clock source pin needed maximum driver strength on the test boards, so I've moved its configuration to the board code)

Regards,
Sylwester Nawrocki

---
 arch/arm/mach-exynos4/Kconfig            |    5 +++
 arch/arm/mach-exynos4/Makefile           |    1 +
 arch/arm/mach-exynos4/setup-fimc.c       |   44 ++++++++++++++++++++++++++++++
 arch/arm/mach-s5pv210/Kconfig            |    5 +++
 arch/arm/mach-s5pv210/Makefile           |    1 +
 arch/arm/mach-s5pv210/setup-fimc.c       |   43 +++++++++++++++++++++++++++++
 arch/arm/plat-s5p/include/plat/camport.h |   28 +++++++++++++++++++
 7 files changed, 127 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-exynos4/setup-fimc.c
 create mode 100644 arch/arm/mach-s5pv210/setup-fimc.c
 create mode 100644 arch/arm/plat-s5p/include/plat/camport.h
arch/arm/mach-exynos4/Kconfig +5 -0
index ad03840..cfe8c33 100644
@@ -71,6 +71,11 @@ config EXYNOS4_SETUP_SDHCI_GPIO
 	help
 	  Common setup code for SDHCI gpio.
 
+config EXYNOS4_SETUP_FIMC
+	bool
+	help
+	  Common setup code for the camera interfaces.
+
 # machine support
 
 menu "EXYNOS4 Machines"
arch/arm/mach-exynos4/Makefile +1 -0
index 0558235..57770d6 100644
@@ -32,6 +32,7 @@ obj-y					+= dev-audio.o
 obj-$(CONFIG_EXYNOS4_DEV_PD)		+= dev-pd.o
 obj-$(CONFIG_EXYNOS4_DEV_SYSMMU)	+= dev-sysmmu.o
 
+obj-$(CONFIG_EXYNOS4_SETUP_FIMC)	+= setup-fimc.o
 obj-$(CONFIG_EXYNOS4_SETUP_I2C1)	+= setup-i2c1.o
 obj-$(CONFIG_EXYNOS4_SETUP_I2C2)	+= setup-i2c2.o
 obj-$(CONFIG_EXYNOS4_SETUP_I2C3)	+= setup-i2c3.o
arch/arm/mach-exynos4/setup-fimc.c new file +44 -0

44 lines not on this page · raw message

arch/arm/mach-s5pv210/Kconfig +5 -0
index 53aabef..3b6d0d4 100644
@@ -53,6 +53,11 @@ config S5PV210_SETUP_SDHCI_GPIO
 	help
 	  Common setup code for SDHCI gpio.
 
+config S5PV210_SETUP_FIMC
+	bool
+	help
+	  Common setup code for the camera interfaces.
+
 menu "S5PC110 Machines"
 
 config MACH_AQUILA
arch/arm/mach-s5pv210/Makefile +1 -0
index ff1a0db..11f1790 100644
@@ -31,6 +31,7 @@ obj-y				+= dev-audio.o
 obj-$(CONFIG_S3C64XX_DEV_SPI)	+= dev-spi.o
 
 obj-$(CONFIG_S5PV210_SETUP_FB_24BPP)	+= setup-fb-24bpp.o
+obj-$(CONFIG_S5PV210_SETUP_FIMC)	+= setup-fimc.o
 obj-$(CONFIG_S5PV210_SETUP_I2C1) 	+= setup-i2c1.o
 obj-$(CONFIG_S5PV210_SETUP_I2C2) 	+= setup-i2c2.o
 obj-$(CONFIG_S5PV210_SETUP_IDE)		+= setup-ide.o
arch/arm/mach-s5pv210/setup-fimc.c new file +43 -0

43 lines not on this page · raw message

arch/arm/plat-s5p/include/plat/camport.h new file +28 -0
new file mode 100644
index 0000000..71688c8
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2011 Samsung Electronics Co., Ltd.
+ *
+ * S5P series camera interface helper functions
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef PLAT_S5P_CAMPORT_H_
+#define PLAT_S5P_CAMPORT_H_ __FILE__
+
+enum s5p_camport_id {
+	S5P_CAMPORT_A,
+	S5P_CAMPORT_B,
+};
+
+/*
+ * The helper functions to configure GPIO for the camera parallel bus.
+ * The camera port can be multiplexed with any FIMC entity, even multiple
+ * FIMC entities are allowed to be attached to a single port simultaneously.
+ * These functions are to be used in the board setup code.
+ */
+int s5pv210_fimc_setup_gpio(enum s5p_camport_id id);
+int exynos4_fimc_setup_gpio(enum s5p_camport_id id);
+
+#endif
Signature · 1 line
1.7.4.1
#7 lore raw

PATCH v2 1/1 ARM: S5P: Add platform helpers for camera GPIO configuration

Quoted 23 lines · Sylwester Nawrocki

Add functions for configuration of the parallel camera bus pins on S5PV210 and Exynos4 SoC.

Signed-off-by Sylwester Nawrocki <[email protected]>
Signed-off-by Kyungmin Park <[email protected]>

---

Hello,

this is a corrected version of platform helpers for the peripheral parallel camera bus configuration in S5PV210 and Exynos4 SoCs.

Changes since v1:
- Renamed files: camera.h/camport.h, setup-camera.c/setup-fimc.c
- Replaced "camif" with "fimc" in function and data type names
- Removed driver strength setup (it turned out than only the clock source pin needed maximum driver strength on the test boards, so I've moved its configuration to the board code)

Regards,
Sylwester Nawrocki

Quoted 15 lines
---
 arch/arm/mach-exynos4/Kconfig            |    5 +++
 arch/arm/mach-exynos4/Makefile           |    1 +
 arch/arm/mach-exynos4/setup-fimc.c       |   44
++++++++++++++++++++++++++++++
 arch/arm/mach-s5pv210/Kconfig            |    5 +++
 arch/arm/mach-s5pv210/Makefile           |    1 +
 arch/arm/mach-s5pv210/setup-fimc.c       |   43
+++++++++++++++++++++++++++++
 arch/arm/plat-s5p/include/plat/camport.h |   28 +++++++++++++++++++
 7 files changed, 127 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/mach-exynos4/setup-fimc.c
 create mode 100644 arch/arm/mach-s5pv210/setup-fimc.c
 create mode 100644 arch/arm/plat-s5p/include/plat/camport.h

(snip)

Looks ok to me, will apply :)
Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.