Message ID | 4B039265.1020906@samsung.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-path: <linux-media-owner@vger.kernel.org> Envelope-to: mchehab@infradead.org Delivery-date: Wed, 18 Nov 2009 06:21:28 +0000 Received: from bombadil.infradead.org [18.85.46.34] by pedra.chehab.org with IMAP (fetchmail-6.3.6) for <mchehab@localhost> (single-drop); Wed, 18 Nov 2009 04:24:41 -0200 (BRST) Received: from vger.kernel.org ([209.132.176.167]) by bombadil.infradead.org with esmtp (Exim 4.69 #1 (Red Hat Linux)) id 1NAduy-00064Z-AD; Wed, 18 Nov 2009 06:21:28 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752203AbZKRGVU (ORCPT <rfc822; kmpark@infradead.org> + 1 other); Wed, 18 Nov 2009 01:21:20 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752127AbZKRGVU (ORCPT <rfc822;linux-media-outgoing>); Wed, 18 Nov 2009 01:21:20 -0500 Received: from mailout1.samsung.com ([203.254.224.24]:44891 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752014AbZKRGVU (ORCPT <rfc822;linux-media@vger.kernel.org>); Wed, 18 Nov 2009 01:21:20 -0500 Received: from epmmp2 (mailout1.samsung.com [203.254.224.24]) by mailout1.samsung.com (iPlanet Messaging Server 5.2 Patch 2 (built Jul 14 2004)) with ESMTP id <0KTA006XTKBO99@mailout1.samsung.com> for linux-media@vger.kernel.org; Wed, 18 Nov 2009 15:21:25 +0900 (KST) Received: from TNRNDGASPAPP1.tn.corp.samsungelectronics.net ([165.213.149.150]) by mmp2.samsung.com (iPlanet Messaging Server 5.2 Patch 2 (built Jul 14 2004)) with ESMTPA id <0KTA001NUKBO7X@mmp2.samsung.com> for linux-media@vger.kernel.org; Wed, 18 Nov 2009 15:21:24 +0900 (KST) Received: from [10.89.8.132] ([10.89.8.132]) by TNRNDGASPAPP1.tn.corp.samsungelectronics.net with Microsoft SMTPSVC(6.0.3790.3959); Wed, 18 Nov 2009 15:21:24 +0900 Date: Wed, 18 Nov 2009 15:21:25 +0900 From: Joonyoung Shim <jy0922.shim@samsung.com> Subject: [PATCH 1/3] radio-si470x: fix SYSCONFIG1 register set on si470x_start() To: linux-media@vger.kernel.org Cc: tobias.lorenz@gmx.net, mchehab@infradead.org, kyungmin.park@samsung.com Message-id: <4B039265.1020906@samsung.com> MIME-version: 1.0 Content-type: text/plain; charset=UTF-8 Content-transfer-encoding: 7BIT User-Agent: Thunderbird 2.0.0.21 (Windows/20090302) X-OriginalArrivalTime: 18 Nov 2009 06:21:24.0950 (UTC) FILETIME=[5ABF3F60:01CA6817] Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org |
Commit Message
Joonyoung Shim
Nov. 18, 2009, 6:21 a.m. UTC
We should use the or operation to set value to the SYSCONFIG1 register
on si470x_start().
Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com>
---
drivers/media/radio/si470x/radio-si470x-common.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
Comments
Hi, what is the advantage in not setting SYSCONFIG1 into a known state? Bye, Toby Am Mittwoch 18 November 2009 07:21:25 schrieb Joonyoung Shim: > We should use the or operation to set value to the SYSCONFIG1 register > on si470x_start(). > > Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com> > --- > drivers/media/radio/si470x/radio-si470x-common.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/drivers/media/radio/si470x/radio-si470x-common.c b/drivers/media/radio/si470x/radio-si470x-common.c > index f33315f..09f631a 100644 > --- a/drivers/media/radio/si470x/radio-si470x-common.c > +++ b/drivers/media/radio/si470x/radio-si470x-common.c > @@ -357,7 +357,7 @@ int si470x_start(struct si470x_device *radio) > goto done; > > /* sysconfig 1 */ > - radio->registers[SYSCONFIG1] = SYSCONFIG1_DE; > + radio->registers[SYSCONFIG1] |= SYSCONFIG1_DE; > retval = si470x_set_register(radio, SYSCONFIG1); > if (retval < 0) > goto done; > -- 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
Hi, Tobias. On 12/2/2009 8:39 AM, Tobias Lorenz wrote: > Hi, > > what is the advantage in not setting SYSCONFIG1 into a known state? > At patch 3/3, i am setting the SYSCONFIG1 register for RDS interrupt in i2c probe function, so i need this patch. Do you have other idea? > Bye, > Toby > > Am Mittwoch 18 November 2009 07:21:25 schrieb Joonyoung Shim: >> We should use the or operation to set value to the SYSCONFIG1 register >> on si470x_start(). >> >> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com> >> --- >> drivers/media/radio/si470x/radio-si470x-common.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/drivers/media/radio/si470x/radio-si470x-common.c b/drivers/media/radio/si470x/radio-si470x-common.c >> index f33315f..09f631a 100644 >> --- a/drivers/media/radio/si470x/radio-si470x-common.c >> +++ b/drivers/media/radio/si470x/radio-si470x-common.c >> @@ -357,7 +357,7 @@ int si470x_start(struct si470x_device *radio) >> goto done; >> >> /* sysconfig 1 */ >> - radio->registers[SYSCONFIG1] = SYSCONFIG1_DE; >> + radio->registers[SYSCONFIG1] |= SYSCONFIG1_DE; >> retval = si470x_set_register(radio, SYSCONFIG1); >> if (retval < 0) >> goto done; >> > -- 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
Hi, ok, understood this problem. So, why not set this in si470x_fops_open directly after the si470x_start? It seems more appropriate to enable the RDS interrupt after starting the radio. Bye the way, you pointed me to a bug. Instead of always setting de-emphasis in si470x_start: radio->registers[SYSCONFIG1] = SYSCONFIG1_DE; This should only be done, if requested by module parameter: radio->registers[SYSCONFIG1] = (de << 11) & SYSCONFIG1_DE; /* DE */ Bye, Toby Am Mittwoch 02 Dezember 2009 00:54:15 schrieb Joonyoung Shim: > Hi, Tobias. > > On 12/2/2009 8:39 AM, Tobias Lorenz wrote: > > Hi, > > > > what is the advantage in not setting SYSCONFIG1 into a known state? > > > > At patch 3/3, i am setting the SYSCONFIG1 register for RDS interrupt in > i2c probe function, so i need this patch. Do you have other idea? > > > Bye, > > Toby > > > > Am Mittwoch 18 November 2009 07:21:25 schrieb Joonyoung Shim: > >> We should use the or operation to set value to the SYSCONFIG1 register > >> on si470x_start(). > >> > >> Signed-off-by: Joonyoung Shim <jy0922.shim@samsung.com> > >> --- > >> drivers/media/radio/si470x/radio-si470x-common.c | 2 +- > >> 1 files changed, 1 insertions(+), 1 deletions(-) > >> > >> diff --git a/drivers/media/radio/si470x/radio-si470x-common.c b/drivers/media/radio/si470x/radio-si470x-common.c > >> index f33315f..09f631a 100644 > >> --- a/drivers/media/radio/si470x/radio-si470x-common.c > >> +++ b/drivers/media/radio/si470x/radio-si470x-common.c > >> @@ -357,7 +357,7 @@ int si470x_start(struct si470x_device *radio) > >> goto done; > >> > >> /* sysconfig 1 */ > >> - radio->registers[SYSCONFIG1] = SYSCONFIG1_DE; > >> + radio->registers[SYSCONFIG1] |= SYSCONFIG1_DE; > >> retval = si470x_set_register(radio, SYSCONFIG1); > >> if (retval < 0) > >> goto done; > >> > > > -- 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
On 12/2/2009 9:12 AM, Tobias Lorenz wrote: > Hi, > > ok, understood this problem. > So, why not set this in si470x_fops_open directly after the si470x_start? > It seems more appropriate to enable the RDS interrupt after starting the radio. > OK, it makes sense. I will move it in si470x_fops_open. > Bye the way, you pointed me to a bug. Instead of always setting de-emphasis in si470x_start: > radio->registers[SYSCONFIG1] = SYSCONFIG1_DE; > This should only be done, if requested by module parameter: > radio->registers[SYSCONFIG1] = (de << 11) & SYSCONFIG1_DE; /* DE */ > Ah, That's right. Thanks. -- 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
diff --git a/drivers/media/radio/si470x/radio-si470x-common.c b/drivers/media/radio/si470x/radio-si470x-common.c index f33315f..09f631a 100644 --- a/drivers/media/radio/si470x/radio-si470x-common.c +++ b/drivers/media/radio/si470x/radio-si470x-common.c @@ -357,7 +357,7 @@ int si470x_start(struct si470x_device *radio) goto done; /* sysconfig 1 */ - radio->registers[SYSCONFIG1] = SYSCONFIG1_DE; + radio->registers[SYSCONFIG1] |= SYSCONFIG1_DE; retval = si470x_set_register(radio, SYSCONFIG1); if (retval < 0) goto done;