Re: [PATCHv7 14/15] cec: s5p-cec: Add s5p-cec driver

2015-07-23 Thread Kamil Debski
Hi,

On 21 July 2015 at 15:03, Marek Szyprowski m.szyprow...@samsung.com wrote:
 Hello,

 On 2015-07-16 15:09, Hans Verkuil wrote:

 Marek, Kamil,

 On 06/29/15 12:14, Hans Verkuil wrote:

 From: Kamil Debski ka...@wypas.org

 Add CEC interface driver present in the Samsung Exynos range of
 SoCs.

 The following files were based on work by SangPil Moon:
 - exynos_hdmi_cec.h
 - exynos_hdmi_cecctl.c

 Signed-off-by: Kamil Debski ka...@wypas.org
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---

 snip

 diff --git a/drivers/media/platform/s5p-cec/s5p_cec.c
 b/drivers/media/platform/s5p-cec/s5p_cec.c
 new file mode 100644
 index 000..0f16d00
 --- /dev/null
 +++ b/drivers/media/platform/s5p-cec/s5p_cec.c
 @@ -0,0 +1,283 @@
 +/* drivers/media/platform/s5p-cec/s5p_cec.c
 + *
 + * Samsung S5P CEC driver
 + *
 + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This driver is based on the cec interface driver for exynos soc by
 + * SangPil Moon.
 + */
 +
 +#include linux/clk.h
 +#include linux/interrupt.h
 +#include linux/kernel.h
 +#include linux/mfd/syscon.h
 +#include linux/module.h
 +#include linux/of.h
 +#include linux/platform_device.h
 +#include linux/pm_runtime.h
 +#include linux/timer.h
 +#include linux/version.h
 +#include linux/workqueue.h
 +#include media/cec.h
 +
 +#include exynos_hdmi_cec.h
 +#include regs-cec.h
 +#include s5p_cec.h
 +
 +#define CEC_NAME   s5p-cec
 +
 +static int debug;
 +module_param(debug, int, 0644);
 +MODULE_PARM_DESC(debug, debug level (0-2));
 +
 +static int s5p_cec_enable(struct cec_adapter *adap, bool enable)
 +{
 +   struct s5p_cec_dev *cec = container_of(adap, struct s5p_cec_dev,
 adap);
 +   int ret;
 +
 +   if (enable) {
 +   ret = pm_runtime_get_sync(cec-dev);
 +
 +   adap-phys_addr = 0x100b;

 This is a bogus physical address. The actual physical address has to be
 derived
 from the EDID that is read by the HDMI transmitter.

 I think in the case of this driver it will have to be userspace that
 assigns
 the physical address after reading the EDID from drm/kms?

 How did you test this, Kamil?


 If I remember correctly, physical address has been derived from EDID in the
 userspace (it is available in /sys/class/drm/*) and passed to s5p-cec driver
 by
 appropriate ioctl.

 I don't know what is the reason for the above 'adap-phys_addr = 0x100b'
 assignment.

At some point there was an idea to read the address from the EDID in
kernel. This static address was a hack until the code that reads the
EDID is written. As you say, it is much better to leave the address to
be set by the userspace. So this assignment serves no purpose anymore.


 Best regards
 --
 Marek Szyprowski, PhD
 Samsung RD Institute Poland


Best wishes,
Kamil
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv7 14/15] cec: s5p-cec: Add s5p-cec driver

2015-07-23 Thread Hans Verkuil

On 07/23/2015 06:39 PM, Kamil Debski wrote:

Hi,

On 21 July 2015 at 15:03, Marek Szyprowski m.szyprow...@samsung.com wrote:

Hello,

On 2015-07-16 15:09, Hans Verkuil wrote:


Marek, Kamil,

On 06/29/15 12:14, Hans Verkuil wrote:


From: Kamil Debski ka...@wypas.org

Add CEC interface driver present in the Samsung Exynos range of
SoCs.

The following files were based on work by SangPil Moon:
- exynos_hdmi_cec.h
- exynos_hdmi_cecctl.c

Signed-off-by: Kamil Debski ka...@wypas.org
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---


snip


diff --git a/drivers/media/platform/s5p-cec/s5p_cec.c
b/drivers/media/platform/s5p-cec/s5p_cec.c
new file mode 100644
index 000..0f16d00
--- /dev/null
+++ b/drivers/media/platform/s5p-cec/s5p_cec.c
@@ -0,0 +1,283 @@
+/* drivers/media/platform/s5p-cec/s5p_cec.c
+ *
+ * Samsung S5P CEC driver
+ *
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This driver is based on the cec interface driver for exynos soc by
+ * SangPil Moon.
+ */
+
+#include linux/clk.h
+#include linux/interrupt.h
+#include linux/kernel.h
+#include linux/mfd/syscon.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include linux/pm_runtime.h
+#include linux/timer.h
+#include linux/version.h
+#include linux/workqueue.h
+#include media/cec.h
+
+#include exynos_hdmi_cec.h
+#include regs-cec.h
+#include s5p_cec.h
+
+#define CEC_NAME   s5p-cec
+
+static int debug;
+module_param(debug, int, 0644);
+MODULE_PARM_DESC(debug, debug level (0-2));
+
+static int s5p_cec_enable(struct cec_adapter *adap, bool enable)
+{
+   struct s5p_cec_dev *cec = container_of(adap, struct s5p_cec_dev,
adap);
+   int ret;
+
+   if (enable) {
+   ret = pm_runtime_get_sync(cec-dev);
+
+   adap-phys_addr = 0x100b;


This is a bogus physical address. The actual physical address has to be
derived
from the EDID that is read by the HDMI transmitter.

I think in the case of this driver it will have to be userspace that
assigns
the physical address after reading the EDID from drm/kms?

How did you test this, Kamil?



If I remember correctly, physical address has been derived from EDID in the
userspace (it is available in /sys/class/drm/*) and passed to s5p-cec driver
by
appropriate ioctl.

I don't know what is the reason for the above 'adap-phys_addr = 0x100b'
assignment.


At some point there was an idea to read the address from the EDID in
kernel. This static address was a hack until the code that reads the
EDID is written. As you say, it is much better to leave the address to
be set by the userspace. So this assignment serves no purpose anymore.


Thank you, that's what I thought. It's fixed in my current tree. Still
working on the CEC framework: I'm chasing race conditions and I suspect
that there may be a bug in the adv7604 or adv7511 CEC implementation.

Once I've sorted that I post a new version which has been tested a lot
more thoroughly and should be complete except for the documentation.

Regards,

Hans

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


Re: [PATCHv7 14/15] cec: s5p-cec: Add s5p-cec driver

2015-07-21 Thread Marek Szyprowski

Hello,

On 2015-07-16 15:09, Hans Verkuil wrote:

Marek, Kamil,

On 06/29/15 12:14, Hans Verkuil wrote:

From: Kamil Debski ka...@wypas.org

Add CEC interface driver present in the Samsung Exynos range of
SoCs.

The following files were based on work by SangPil Moon:
- exynos_hdmi_cec.h
- exynos_hdmi_cecctl.c

Signed-off-by: Kamil Debski ka...@wypas.org
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---

snip


diff --git a/drivers/media/platform/s5p-cec/s5p_cec.c 
b/drivers/media/platform/s5p-cec/s5p_cec.c
new file mode 100644
index 000..0f16d00
--- /dev/null
+++ b/drivers/media/platform/s5p-cec/s5p_cec.c
@@ -0,0 +1,283 @@
+/* drivers/media/platform/s5p-cec/s5p_cec.c
+ *
+ * Samsung S5P CEC driver
+ *
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This driver is based on the cec interface driver for exynos soc by
+ * SangPil Moon.
+ */
+
+#include linux/clk.h
+#include linux/interrupt.h
+#include linux/kernel.h
+#include linux/mfd/syscon.h
+#include linux/module.h
+#include linux/of.h
+#include linux/platform_device.h
+#include linux/pm_runtime.h
+#include linux/timer.h
+#include linux/version.h
+#include linux/workqueue.h
+#include media/cec.h
+
+#include exynos_hdmi_cec.h
+#include regs-cec.h
+#include s5p_cec.h
+
+#define CEC_NAME   s5p-cec
+
+static int debug;
+module_param(debug, int, 0644);
+MODULE_PARM_DESC(debug, debug level (0-2));
+
+static int s5p_cec_enable(struct cec_adapter *adap, bool enable)
+{
+   struct s5p_cec_dev *cec = container_of(adap, struct s5p_cec_dev, adap);
+   int ret;
+
+   if (enable) {
+   ret = pm_runtime_get_sync(cec-dev);
+
+   adap-phys_addr = 0x100b;

This is a bogus physical address. The actual physical address has to be derived
from the EDID that is read by the HDMI transmitter.

I think in the case of this driver it will have to be userspace that assigns
the physical address after reading the EDID from drm/kms?

How did you test this, Kamil?


If I remember correctly, physical address has been derived from EDID in the
userspace (it is available in /sys/class/drm/*) and passed to s5p-cec 
driver by

appropriate ioctl.

I don't know what is the reason for the above 'adap-phys_addr = 0x100b' 
assignment.


Best regards
--
Marek Szyprowski, PhD
Samsung RD Institute Poland

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


Re: [PATCHv7 14/15] cec: s5p-cec: Add s5p-cec driver

2015-07-16 Thread Hans Verkuil
Marek, Kamil,



On 06/29/15 12:14, Hans Verkuil wrote:
 From: Kamil Debski ka...@wypas.org
 
 Add CEC interface driver present in the Samsung Exynos range of
 SoCs.
 
 The following files were based on work by SangPil Moon:
 - exynos_hdmi_cec.h
 - exynos_hdmi_cecctl.c
 
 Signed-off-by: Kamil Debski ka...@wypas.org
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 ---

snip

 diff --git a/drivers/media/platform/s5p-cec/s5p_cec.c 
 b/drivers/media/platform/s5p-cec/s5p_cec.c
 new file mode 100644
 index 000..0f16d00
 --- /dev/null
 +++ b/drivers/media/platform/s5p-cec/s5p_cec.c
 @@ -0,0 +1,283 @@
 +/* drivers/media/platform/s5p-cec/s5p_cec.c
 + *
 + * Samsung S5P CEC driver
 + *
 + * Copyright (c) 2014 Samsung Electronics Co., Ltd.
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This driver is based on the cec interface driver for exynos soc by
 + * SangPil Moon.
 + */
 +
 +#include linux/clk.h
 +#include linux/interrupt.h
 +#include linux/kernel.h
 +#include linux/mfd/syscon.h
 +#include linux/module.h
 +#include linux/of.h
 +#include linux/platform_device.h
 +#include linux/pm_runtime.h
 +#include linux/timer.h
 +#include linux/version.h
 +#include linux/workqueue.h
 +#include media/cec.h
 +
 +#include exynos_hdmi_cec.h
 +#include regs-cec.h
 +#include s5p_cec.h
 +
 +#define CEC_NAME s5p-cec
 +
 +static int debug;
 +module_param(debug, int, 0644);
 +MODULE_PARM_DESC(debug, debug level (0-2));
 +
 +static int s5p_cec_enable(struct cec_adapter *adap, bool enable)
 +{
 + struct s5p_cec_dev *cec = container_of(adap, struct s5p_cec_dev, adap);
 + int ret;
 +
 + if (enable) {
 + ret = pm_runtime_get_sync(cec-dev);
 +
 + adap-phys_addr = 0x100b;

This is a bogus physical address. The actual physical address has to be derived
from the EDID that is read by the HDMI transmitter.

I think in the case of this driver it will have to be userspace that assigns
the physical address after reading the EDID from drm/kms?

How did you test this, Kamil?

Regards,

Hans

 + s5p_cec_reset(cec);
 +
 + s5p_cec_set_divider(cec);
 + s5p_cec_threshold(cec);
 +
 + s5p_cec_unmask_tx_interrupts(cec);
 + s5p_cec_unmask_rx_interrupts(cec);
 + s5p_cec_enable_rx(cec);
 + } else {
 + s5p_cec_mask_tx_interrupts(cec);
 + s5p_cec_mask_rx_interrupts(cec);
 + pm_runtime_disable(cec-dev);
 + }
 +
 + return 0;
 +}
 +
 +static int s5p_cec_log_addr(struct cec_adapter *adap, u8 addr)
 +{
 + struct s5p_cec_dev *cec = container_of(adap, struct s5p_cec_dev, adap);
 +
 + s5p_cec_set_addr(cec, addr);
 + return 0;
 +}
 +
 +static int s5p_cec_transmit(struct cec_adapter *adap, struct cec_msg *msg)
 +{
 + struct s5p_cec_dev *cec = container_of(adap, struct s5p_cec_dev, adap);
 +
 + s5p_cec_copy_packet(cec, msg-msg, msg-len);
 + return 0;
 +}
 +
 +static void s5p_cec_transmit_timed_out(struct cec_adapter *adap)
 +{
 +
 +}
 +
 +static irqreturn_t s5p_cec_irq_handler(int irq, void *priv)
 +{
 + struct s5p_cec_dev *cec = priv;
 + u32 status = 0;
 +
 + status = s5p_cec_get_status(cec);
 +
 + dev_dbg(cec-dev, irq received\n);
 +
 + if (status  CEC_STATUS_TX_DONE) {
 + if (status  CEC_STATUS_TX_ERROR) {
 + dev_dbg(cec-dev, CEC_STATUS_TX_ERROR set\n);
 + cec-tx = STATE_ERROR;
 + } else {
 + dev_dbg(cec-dev, CEC_STATUS_TX_DONE\n);
 + cec-tx = STATE_DONE;
 + }
 + s5p_clr_pending_tx(cec);
 + }
 +
 + if (status  CEC_STATUS_RX_DONE) {
 + if (status  CEC_STATUS_RX_ERROR) {
 + dev_dbg(cec-dev, CEC_STATUS_RX_ERROR set\n);
 + s5p_cec_rx_reset(cec);
 + s5p_cec_enable_rx(cec);
 + } else {
 + dev_dbg(cec-dev, CEC_STATUS_RX_DONE set\n);
 + if (cec-rx != STATE_IDLE)
 + dev_dbg(cec-dev, Buffer overrun (worker did 
 not process previous message)\n);
 + cec-rx = STATE_BUSY;
 + cec-msg.len = status  24;
 + cec-msg.status = CEC_RX_STATUS_READY;
 + s5p_cec_get_rx_buf(cec, cec-msg.len,
 + cec-msg.msg);
 + cec-rx = STATE_DONE;
 + s5p_cec_enable_rx(cec);
 + }
 + /* Clear interrupt pending bit */
 + s5p_clr_pending_rx(cec);
 + }
 + return IRQ_WAKE_THREAD;
 +}
 +
 +static irqreturn_t s5p_cec_irq_handler_thread(int irq, void *priv)
 +{
 + struct s5p_cec_dev *cec = priv;
 

[PATCHv7 14/15] cec: s5p-cec: Add s5p-cec driver

2015-06-29 Thread Hans Verkuil
From: Kamil Debski ka...@wypas.org

Add CEC interface driver present in the Samsung Exynos range of
SoCs.

The following files were based on work by SangPil Moon:
- exynos_hdmi_cec.h
- exynos_hdmi_cecctl.c

Signed-off-by: Kamil Debski ka...@wypas.org
Signed-off-by: Hans Verkuil hans.verk...@cisco.com
---
 .../devicetree/bindings/media/s5p-cec.txt  |  31 +++
 drivers/media/platform/Kconfig |  10 +
 drivers/media/platform/Makefile|   1 +
 drivers/media/platform/s5p-cec/Makefile|   2 +
 drivers/media/platform/s5p-cec/exynos_hdmi_cec.h   |  37 +++
 .../media/platform/s5p-cec/exynos_hdmi_cecctrl.c   | 208 +++
 drivers/media/platform/s5p-cec/regs-cec.h  |  96 +++
 drivers/media/platform/s5p-cec/s5p_cec.c   | 283 +
 drivers/media/platform/s5p-cec/s5p_cec.h   |  76 ++
 9 files changed, 744 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/s5p-cec.txt
 create mode 100644 drivers/media/platform/s5p-cec/Makefile
 create mode 100644 drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
 create mode 100644 drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
 create mode 100644 drivers/media/platform/s5p-cec/regs-cec.h
 create mode 100644 drivers/media/platform/s5p-cec/s5p_cec.c
 create mode 100644 drivers/media/platform/s5p-cec/s5p_cec.h

diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt 
b/Documentation/devicetree/bindings/media/s5p-cec.txt
new file mode 100644
index 000..da63a4c
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
@@ -0,0 +1,31 @@
+* Samsung HDMI CEC driver
+
+The HDMI CEC module is present is Samsung SoCs and its purpose is to
+handle communication between HDMI connected devices over the CEC bus.
+
+Required properties:
+  - compatible : value should be follwoing
+   samsung,s5p-cec
+
+  - reg : Physical base address of the IP registers and length of memory
+ mapped region.
+
+  - interrupts : HDMI CEC interrupt number to the CPU.
+  - clocks : from common clock binding: handle to HDMI CEC clock.
+  - clock-names : from common clock binding: must contain hdmicec,
+ corresponding to entry in the clocks property.
+  - samsung,syscon-phandle - phandle to the PMU system controller
+
+Example:
+
+hdmicec: cec@100B {
+   compatible = samsung,s5p-cec;
+   reg = 0x100B 0x200;
+   interrupts = 0 114 0;
+   clocks = clock CLK_HDMI_CEC;
+   clock-names = hdmicec;
+   samsung,syscon-phandle = pmu_system_controller;
+   pinctrl-names = default;
+   pinctrl-0 = hdmi_cec;
+   status = okay;
+};
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 4776a8c..d3843fc 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -157,6 +157,16 @@ config VIDEO_MEM2MEM_DEINTERLACE
help
Generic deinterlacing V4L2 driver.
 
+config VIDEO_SAMSUNG_S5P_CEC
+   tristate Samsung S5P CEC driver
+   depends on CEC  VIDEO_DEV  VIDEO_V4L2  (PLAT_S5P || ARCH_EXYNOS)
+   default n
+   ---help---
+ This is a driver for Samsung S5P HDMI CEC interface. It uses the
+ generic CEC framework interface.
+ CEC bus is present in the HDMI connector and enables communication
+ between compatible devices.
+
 config VIDEO_SAMSUNG_S5P_G2D
tristate Samsung S5P and EXYNOS4 G2D 2d graphics accelerator driver
depends on VIDEO_DEV  VIDEO_V4L2
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 114f9ab..95dd78d 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_VIDEO_MEM2MEM_DEINTERLACE)   += 
m2m-deinterlace.o
 
 obj-$(CONFIG_VIDEO_S3C_CAMIF)  += s3c-camif/
 obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS4_IS) += exynos4-is/
+obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC)+= s5p-cec/
 obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG)   += s5p-jpeg/
 obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC)+= s5p-mfc/
 obj-$(CONFIG_VIDEO_SAMSUNG_S5P_TV) += s5p-tv/
diff --git a/drivers/media/platform/s5p-cec/Makefile 
b/drivers/media/platform/s5p-cec/Makefile
new file mode 100644
index 000..0e2cf45
--- /dev/null
+++ b/drivers/media/platform/s5p-cec/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC)+= s5p-cec.o
+s5p-cec-y += s5p_cec.o exynos_hdmi_cecctrl.o
diff --git a/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h 
b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
new file mode 100644
index 000..d008695
--- /dev/null
+++ b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
@@ -0,0 +1,37 @@
+/* drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
+ *
+ * Copyright (c) 2010, 2014 Samsung Electronics
+ * http://www.samsung.com/
+ *
+ * Header file for interface of Samsung Exynos hdmi cec hardware
+ *
+ * This program is free software; you can redistribute it and/or modify
+ *