Re: [PATCH 1/3] sp2: Add I2C driver for CIMaX SP2 common interface module
On 08/06/2014 10:05 AM, Olli Salonen wrote:
Driver for the CIMaX SP2 common interface chip. It is very much based on
the existing cimax2 driver for cx23885, but should be more reusable. The
product has been sold with name Atmel T90FJR as well and the data sheets
for that chip seem to be publicly available.
It seems that the USB device that I have and the cx23885 based devices will
need to interact differently with the chip for the CAM operations. Thus
there is one callback function that is passed on to the sp2 driver
(see function sp2_ci_op_cam for that one).
IRQ functionality is not included currently (not needed by USB devices
and I don't have a PCIe device for development).
Signed-off-by: Olli Salonen olli.salo...@iki.fi
---
drivers/media/dvb-frontends/Makefile | 1 +
drivers/media/dvb-frontends/sp2.c | 411 +
drivers/media/dvb-frontends/sp2.h | 54 +
drivers/media/dvb-frontends/sp2_priv.h | 49
4 files changed, 515 insertions(+)
create mode 100644 drivers/media/dvb-frontends/sp2.c
create mode 100644 drivers/media/dvb-frontends/sp2.h
create mode 100644 drivers/media/dvb-frontends/sp2_priv.h
diff --git a/drivers/media/dvb-frontends/Makefile
b/drivers/media/dvb-frontends/Makefile
index edf103d..3498b95 100644
--- a/drivers/media/dvb-frontends/Makefile
+++ b/drivers/media/dvb-frontends/Makefile
@@ -107,6 +107,7 @@ obj-$(CONFIG_DVB_DRXK) += drxk.o
obj-$(CONFIG_DVB_TDA18271C2DD) += tda18271c2dd.o
obj-$(CONFIG_DVB_SI2165) += si2165.o
obj-$(CONFIG_DVB_A8293) += a8293.o
+obj-$(CONFIG_DVB_SP2) += sp2.o
obj-$(CONFIG_DVB_TDA10071) += tda10071.o
obj-$(CONFIG_DVB_RTL2830) += rtl2830.o
obj-$(CONFIG_DVB_RTL2832) += rtl2832.o
diff --git a/drivers/media/dvb-frontends/sp2.c
b/drivers/media/dvb-frontends/sp2.c
new file mode 100644
index 000..c1b4d7e
--- /dev/null
+++ b/drivers/media/dvb-frontends/sp2.c
@@ -0,0 +1,411 @@
+/*
+ * CIMaX SP2/SP2HF (Atmel T90FJR) CI driver
+ *
+ * Copyright (C) 2014 Olli Salonen olli.salo...@iki.fi
+ *
+ * Heavily based on CIMax2(R) SP2 driver in conjunction with NetUp Dual
+ * DVB-S2 CI card (cimax2) with following copyrights:
+ *
+ * Copyright (C) 2009 NetUP Inc.
+ * Copyright (C) 2009 Igor M. Liplianin liplia...@netup.ru
+ * Copyright (C) 2009 Abylay Ospan aos...@netup.ru
+ *
+ *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 program is distributed in the hope that it will be useful,
+ *but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ *GNU General Public License for more details.
+ */
+
+#include sp2_priv.h
+
+static int sp2_read_i2c(struct sp2 *s, u8 reg, u8 *buf, int len)
+{
+ int ret;
+ struct i2c_client *client = s-client;
+ struct i2c_adapter *adap = client-adapter;
+ struct i2c_msg msg[] = {
+ {
+ .addr = client-addr,
+ .flags = 0,
+ .buf = reg,
+ .len = 1
+ }, {
+ .addr = client-addr,
+ .flags = I2C_M_RD,
+ .buf = buf,
+ .len = len
+ }
+ };
+
+ ret = i2c_transfer(adap, msg, 2);
+
+ if (ret != 2) {
+ dev_err(client-dev, i2c read error, reg = 0x%02x, status =
%d\n,
+ reg, ret);
+ return -1;
+ }
Could you define error code here? I know a lot of old drivers were using
just -1, but for new driver we could try do better. Return possible
error from i2c_transfer() and if it returns wrong amount of messages (0
or 1 in that case) then change -EIO.
I looked from I2C documentation and I think -EIO fits best
Documentation/i2c/fault-codes
EIO
This rather vague error means something went wrong when
performing an I/O operation. Use a more specific fault
code when you can.
+
+ dev_dbg(s-client-dev, addr=0x%04x, reg = 0x%02x, data = %02x\n,
+ client-addr, reg, buf[0]);
+
+ return 0;
+}
+
+static int sp2_write_i2c(struct sp2 *s, u8 reg, u8 *buf, int len)
+{
+ int ret;
+ u8 buffer[35];
+ struct i2c_client *client = s-client;
+ struct i2c_adapter *adap = client-adapter;
+ struct i2c_msg msg = {
+ .addr = client-addr,
+ .flags = 0,
+ .buf = buffer[0],
+ .len = len + 1
+ };
+
+ if ((len + 1) sizeof(buffer)) {
+ dev_err(client-dev, i2c wr reg=%02x: len=%d is too big!\n,
+ reg, len);
+ return -EINVAL;
+ }
+
+ buffer[0] = reg;
+
Re: [PATCH 1/3] sp2: Add I2C driver for CIMaX SP2 common interface module
Thank you Antti for the review. I'll submit another version of the patch in the coming days. Cheers, -olli On 7 August 2014 19:28, Antti Palosaari cr...@iki.fi wrote: Reviewed-by: Antti Palosaari cr...@iki.fi None of those findings are critical. However I hope you double check and fix if there is any relevant enough. regards Antti -- http://palosaari.fi/ -- 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
[PATCH 1/3] sp2: Add I2C driver for CIMaX SP2 common interface module
Driver for the CIMaX SP2 common interface chip. It is very much based on
the existing cimax2 driver for cx23885, but should be more reusable. The
product has been sold with name Atmel T90FJR as well and the data sheets
for that chip seem to be publicly available.
It seems that the USB device that I have and the cx23885 based devices will
need to interact differently with the chip for the CAM operations. Thus
there is one callback function that is passed on to the sp2 driver
(see function sp2_ci_op_cam for that one).
IRQ functionality is not included currently (not needed by USB devices
and I don't have a PCIe device for development).
Signed-off-by: Olli Salonen olli.salo...@iki.fi
---
drivers/media/dvb-frontends/Makefile | 1 +
drivers/media/dvb-frontends/sp2.c | 411 +
drivers/media/dvb-frontends/sp2.h | 54 +
drivers/media/dvb-frontends/sp2_priv.h | 49
4 files changed, 515 insertions(+)
create mode 100644 drivers/media/dvb-frontends/sp2.c
create mode 100644 drivers/media/dvb-frontends/sp2.h
create mode 100644 drivers/media/dvb-frontends/sp2_priv.h
diff --git a/drivers/media/dvb-frontends/Makefile
b/drivers/media/dvb-frontends/Makefile
index edf103d..3498b95 100644
--- a/drivers/media/dvb-frontends/Makefile
+++ b/drivers/media/dvb-frontends/Makefile
@@ -107,6 +107,7 @@ obj-$(CONFIG_DVB_DRXK) += drxk.o
obj-$(CONFIG_DVB_TDA18271C2DD) += tda18271c2dd.o
obj-$(CONFIG_DVB_SI2165) += si2165.o
obj-$(CONFIG_DVB_A8293) += a8293.o
+obj-$(CONFIG_DVB_SP2) += sp2.o
obj-$(CONFIG_DVB_TDA10071) += tda10071.o
obj-$(CONFIG_DVB_RTL2830) += rtl2830.o
obj-$(CONFIG_DVB_RTL2832) += rtl2832.o
diff --git a/drivers/media/dvb-frontends/sp2.c
b/drivers/media/dvb-frontends/sp2.c
new file mode 100644
index 000..c1b4d7e
--- /dev/null
+++ b/drivers/media/dvb-frontends/sp2.c
@@ -0,0 +1,411 @@
+/*
+ * CIMaX SP2/SP2HF (Atmel T90FJR) CI driver
+ *
+ * Copyright (C) 2014 Olli Salonen olli.salo...@iki.fi
+ *
+ * Heavily based on CIMax2(R) SP2 driver in conjunction with NetUp Dual
+ * DVB-S2 CI card (cimax2) with following copyrights:
+ *
+ * Copyright (C) 2009 NetUP Inc.
+ * Copyright (C) 2009 Igor M. Liplianin liplia...@netup.ru
+ * Copyright (C) 2009 Abylay Ospan aos...@netup.ru
+ *
+ *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 program is distributed in the hope that it will be useful,
+ *but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ *GNU General Public License for more details.
+ */
+
+#include sp2_priv.h
+
+static int sp2_read_i2c(struct sp2 *s, u8 reg, u8 *buf, int len)
+{
+ int ret;
+ struct i2c_client *client = s-client;
+ struct i2c_adapter *adap = client-adapter;
+ struct i2c_msg msg[] = {
+ {
+ .addr = client-addr,
+ .flags = 0,
+ .buf = reg,
+ .len = 1
+ }, {
+ .addr = client-addr,
+ .flags = I2C_M_RD,
+ .buf = buf,
+ .len = len
+ }
+ };
+
+ ret = i2c_transfer(adap, msg, 2);
+
+ if (ret != 2) {
+ dev_err(client-dev, i2c read error, reg = 0x%02x, status =
%d\n,
+ reg, ret);
+ return -1;
+ }
+
+ dev_dbg(s-client-dev, addr=0x%04x, reg = 0x%02x, data = %02x\n,
+ client-addr, reg, buf[0]);
+
+ return 0;
+}
+
+static int sp2_write_i2c(struct sp2 *s, u8 reg, u8 *buf, int len)
+{
+ int ret;
+ u8 buffer[35];
+ struct i2c_client *client = s-client;
+ struct i2c_adapter *adap = client-adapter;
+ struct i2c_msg msg = {
+ .addr = client-addr,
+ .flags = 0,
+ .buf = buffer[0],
+ .len = len + 1
+ };
+
+ if ((len + 1) sizeof(buffer)) {
+ dev_err(client-dev, i2c wr reg=%02x: len=%d is too big!\n,
+ reg, len);
+ return -EINVAL;
+ }
+
+ buffer[0] = reg;
+ memcpy(buffer[1], buf, len);
+
+ ret = i2c_transfer(adap, msg, 1);
+
+ if (ret != 1) {
+ dev_err(client-dev, i2c write error, reg = 0x%02x, status =
%d\n,
+ reg, ret);
+ return -1;
+ }
+
+ return 0;
+}
+
+static int sp2_ci_op_cam(struct dvb_ca_en50221 *en50221, int slot, u8 acs,
+ u8 read, int addr, u8 data)
+{
+ struct sp2 *s = en50221-data;
+ u8 store;
+ int mem, ret;
+ int (*ci_op_cam)(void*, u8, int, u8, int*) = s-ci_control;
+
+
