From patchwork Mon Mar 2 17:36:05 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexey Klimov X-Patchwork-Id: 9558 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n22HZjlX015598 for ; Mon, 2 Mar 2009 17:35:45 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753904AbZCBRfp (ORCPT ); Mon, 2 Mar 2009 12:35:45 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753200AbZCBRfp (ORCPT ); Mon, 2 Mar 2009 12:35:45 -0500 Received: from mail-bw0-f178.google.com ([209.85.218.178]:36477 "EHLO mail-bw0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753904AbZCBRfo (ORCPT ); Mon, 2 Mar 2009 12:35:44 -0500 Received: by bwz26 with SMTP id 26so2027016bwz.37 for ; Mon, 02 Mar 2009 09:35:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:subject:from:to:cc :in-reply-to:references:content-type:date:message-id:mime-version :x-mailer:content-transfer-encoding; bh=OBohX/a07jWM9WYKgWrcR4qz874T+p7d5wFIKpR3Qic=; b=QrD/hfxZNpY9sL73KL8/ZkfQwv6ejKZTWnzcBwhMklOvVQqDq4y+U9YeldtrB7LI5F zciNwMlLdtluQ9KmUyKkJV4b17CuSurokk6nCXAJK54APPKwa7n1slhblqwtpsmrdF4A pczFW27hE8qFzcDnMO5w3iR0yNCx3KsZj+eck= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; b=Kpho4c2o45QeYQ7q/cTkWKMYs+1tL32iPXZBB+Q6agTKnjaedXOdf4fvo8/eMICm0P p85pzcP3nYCoXzFAlh3a++IvfnC8wrOLX9ob33MHyJA6YOXnbBS0eVv8rXV2Q/RWfUZL KFZ/9cVMcX7lISYiD1EUcyAZ4JTNPfxuuxDCw= Received: by 10.103.198.20 with SMTP id a20mr3057432muq.63.1236015340808; Mon, 02 Mar 2009 09:35:40 -0800 (PST) Received: from ?192.168.1.42? (gw.zunet.ru [217.67.117.64]) by mx.google.com with ESMTPS id u9sm1484700muf.25.2009.03.02.09.35.39 (version=SSLv3 cipher=RC4-MD5); Mon, 02 Mar 2009 09:35:40 -0800 (PST) Subject: Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-netup From: Alexey Klimov To: "Igor M. Liplianin" Cc: linux-media@vger.kernel.org, Mauro Chehab , Steven Toth In-Reply-To: <200902281632.47597.liplianin@tut.by> References: <200902281632.47597.liplianin@tut.by> Date: Mon, 02 Mar 2009 20:36:05 +0300 Message-Id: <1236015365.1726.53.camel@tux.localhost> Mime-Version: 1.0 X-Mailer: Evolution 2.24.4 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org Hello, all Igor, do you mind if i make few comments ? On Sat, 2009-02-28 at 16:32 +0200, Igor M. Liplianin wrote: > Mauro, > It is driver for NetUP Dual DVB-S2 CI PCI-e card. > You can find short description of it on linuxtv wiki. > > Please pull from http://udev.netup.ru/hg/v4l-dvb-netup > > for the following 11 changesets: > > 01/11: Add init code for NetUP Dual DVB-S2 CI card > http://udev.netup.ru/hg/v4l-dvb-netup?cmd=changeset;node=f752e18dc395 I paste first patch here: I see return -1 after call to i2c_transfer in few patches also, may be you wanna place return ret there.. And "static" also have to be checked. I'm sorry if my comments and questions are wrong. > 02/11: Add EEPROM code for NetUP Dual DVB-S2 CI card. > http://udev.netup.ru/hg/v4l-dvb-netup?cmd=changeset;node=5c764157d510 > > 03/11: Add CIMax(R) SP2 Common Interface code for NetUP Dual DVB-S2 CI card > http://udev.netup.ru/hg/v4l-dvb-netup?cmd=changeset;node=7af6715bacec > > 04/11: Add support for ST STV6110 silicon tuner. > http://udev.netup.ru/hg/v4l-dvb-netup?cmd=changeset;node=68ca5a26e7a5 > > 05/11: Add support for ST LNBH24 LNB power controller. > http://udev.netup.ru/hg/v4l-dvb-netup?cmd=changeset;node=3b65476f39a9 > > 06/11: Add headers for ST STV0900 dual demodulator. > http://udev.netup.ru/hg/v4l-dvb-netup?cmd=changeset;node=8dd572ce00ae > > 07/11: Add more headers for ST STV0900 dual demodulator. > http://udev.netup.ru/hg/v4l-dvb-netup?cmd=changeset;node=d29394751223 > > 08/11: Add core code for ST STV0900 dual demodulator. > http://udev.netup.ru/hg/v4l-dvb-netup?cmd=changeset;node=59492f22aebe > > 09/11: Add support for ST STV0900 dual demodulator. > http://udev.netup.ru/hg/v4l-dvb-netup?cmd=changeset;node=df4fbc0c5b7e > > 10/11: Add support for NetUP Dual DVB-S2 CI card > http://udev.netup.ru/hg/v4l-dvb-netup?cmd=changeset;node=5b9ba251e7dc > > 11/11: stv0900 fixes > http://udev.netup.ru/hg/v4l-dvb-netup?cmd=changeset;node=e5c9e5d75291 > > > b/linux/drivers/media/dvb/frontends/cimax2.c | 531 ++ > b/linux/drivers/media/dvb/frontends/cimax2.h | 45 > b/linux/drivers/media/dvb/frontends/lnbh24.h | 55 > b/linux/drivers/media/dvb/frontends/netup-init.c | 143 > b/linux/drivers/media/dvb/frontends/netup-init.h | 25 > b/linux/drivers/media/dvb/frontends/stv0900.h | 62 > b/linux/drivers/media/dvb/frontends/stv0900_core.c | 1961 ++++++++++ > b/linux/drivers/media/dvb/frontends/stv0900_init.h | 428 ++ > b/linux/drivers/media/dvb/frontends/stv0900_priv.h | 430 ++ > b/linux/drivers/media/dvb/frontends/stv0900_reg.h | 3787 +++++++++++++++++++++ > b/linux/drivers/media/dvb/frontends/stv0900_sw.c | 2847 +++++++++++++++ > b/linux/drivers/media/dvb/frontends/stv6110.c | 457 ++ > b/linux/drivers/media/dvb/frontends/stv6110.h | 62 > b/linux/drivers/media/video/cx23885/netup-eeprom.c | 117 > b/linux/drivers/media/video/cx23885/netup-eeprom.h | 42 > linux/Documentation/video4linux/CARDLIST.cx23885 | 1 > linux/drivers/media/dvb/frontends/Kconfig | 21 > linux/drivers/media/dvb/frontends/Makefile | 4 > linux/drivers/media/dvb/frontends/lnbp21.c | 33 > linux/drivers/media/dvb/frontends/lnbp21.h | 50 > linux/drivers/media/dvb/frontends/stv0900_core.c | 2 > linux/drivers/media/dvb/frontends/stv0900_init.h | 17 > linux/drivers/media/video/cx23885/Kconfig | 1 > linux/drivers/media/video/cx23885/Makefile | 4 > linux/drivers/media/video/cx23885/cx23885-cards.c | 50 > linux/drivers/media/video/cx23885/cx23885-dvb.c | 106 > linux/drivers/media/video/cx23885/cx23885.h | 2 > 27 files changed, 11255 insertions(+), 28 deletions(-) > > Thanks, > Igor > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/linux/drivers/media/dvb/frontends/netup-init.c Tue Jan 20 22:03:11 2009 +0200 @@ -0,0 +1,143 @@ +/* + * netup-init.c + * + * NetUP Dual DVB-S2 CI driver + * + * Copyright (C) 2009 NetUP Inc. + * Copyright (C) 2009 Igor M. Liplianin + * Copyright (C) 2009 Abylay Ospan + * + * 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. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ + +#include "cx23885.h" Well, i failed trying to find this .h file here. May be i understand process of building in wrong way. +int i2c_av_write(struct i2c_adapter *i2c, u16 reg, u8 val) +{ + int ret; + u8 buf[3]; + struct i2c_msg msg = { + .addr = 0x88 >> 1, + .flags = 0, + .buf = buf, + .len = 3 + }; + + buf[0] = reg >> 8; + buf[1] = reg & 0xff; + buf[2] = val; + + ret = i2c_transfer(i2c, &msg, 1); + + if (ret != 1) { + printk(KERN_ERR "i2c write error, Reg=[0x%02x], Status=%d\n", + reg, ret); If user see such message in dmesg he or she won't be in position to determine what module makes this error. Right ? It's better to add module name here, i think. + return -1; + } + + return 0; +} + +int i2c_av_write4(struct i2c_adapter *i2c, u16 reg, u32 val) +{ + int ret; + u8 buf[6]; + struct i2c_msg msg = { + .addr = 0x88 >> 1, + .flags = 0, + .buf = buf, + .len = 6 + }; + + buf[0] = reg >> 8; + buf[1] = reg & 0xff; + buf[2] = val & 0xff; + buf[3] = (val >> 8) & 0xff; + buf[4] = (val >> 16) & 0xff; + buf[5] = val >> 24; + + ret = i2c_transfer(i2c, &msg, 1); + + if (ret != 1) { + printk(KERN_ERR "i2c write error, Reg=[0x%02x], Status=%d\n", + reg, ret); The same thing here and below. + return -1; + } + + return 0; +} + +u8 i2c_av_read(struct i2c_adapter *i2c, u16 reg) +{ + int ret; + u8 buf[2]; + struct i2c_msg msg = { + .addr = 0x88 >> 1, + .flags = 0, + .buf = buf, + .len = 2 + }; + + buf[0] = reg >> 8; + buf[1] = reg & 0xff; + + ret = i2c_transfer(i2c, &msg, 1); + + if (ret != 1) { + printk(KERN_ERR "i2c write error, Reg=[0x%02x], Status=%d\n", + reg, ret); + return -1; + } + + msg.flags = I2C_M_RD; + msg.len = 1; + + ret = i2c_transfer(i2c, &msg, 1); + + if (ret != 1) { + printk(KERN_ERR "i2c write error, Reg=[0x%02x], Status=%d\n", + reg, ret); + return -1; Why don't return ret ? + } + + return buf[0]; +} + +int i2c_av_and_or(struct i2c_adapter *i2c, u16 reg, unsigned and_mask, + u8 or_value) +{ + return i2c_av_write(i2c, reg, + (i2c_av_read(i2c, reg) & and_mask) | + or_value); +} +/* set 27MHz on AUX_CLK */ +void netup_initialize(struct cx23885_dev *dev) +{ + struct cx23885_i2c *i2c_bus = &dev->i2c_bus[2]; + struct i2c_adapter *i2c = &i2c_bus->i2c_adap; + + /* Stop microcontroller */ + i2c_av_and_or(i2c, 0x803, ~0x10, 0x00); + + /* Aux PLL frac for 27 MHz */ + i2c_av_write4(i2c, 0x114, 0xea0eb3); + + /* Aux PLL int for 27 MHz */ + i2c_av_write4(i2c, 0x110, 0x090319); + + /* start microcontroller */ + i2c_av_and_or(i2c, 0x803, ~0x10, 0x10); Why you didn't check returned values of i2c-functions? It's better programming practice, yes? And general question - can many functions in this file be declared as static? +} --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/linux/drivers/media/dvb/frontends/netup-init.h Tue Jan 20 22:03:11 2009 +0200 @@ -0,0 +1,25 @@ +/* + * netup-init.h + * + * NetUP Dual DVB-S2 CI driver + * + * Copyright (C) 2009 NetUP Inc. + * Copyright (C) 2009 Igor M. Liplianin + * Copyright (C) 2009 Abylay Ospan + * + * 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. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. + */ +extern void netup_initialize(struct cx23885_dev *dev);