* [PATCH 1/3] radio-si470x: fix SYSCONFIG1 register set on si470x_start()
@ 2009-11-18 6:21 Joonyoung Shim
2009-12-01 23:39 ` Tobias Lorenz
0 siblings, 1 reply; 5+ messages in thread
From: Joonyoung Shim @ 2009-11-18 6:21 UTC (permalink / raw)
To: linux-media; +Cc: tobias.lorenz, mchehab, kyungmin.park
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;
--
1.6.0.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] radio-si470x: fix SYSCONFIG1 register set on si470x_start()
2009-11-18 6:21 [PATCH 1/3] radio-si470x: fix SYSCONFIG1 register set on si470x_start() Joonyoung Shim
@ 2009-12-01 23:39 ` Tobias Lorenz
2009-12-01 23:54 ` Joonyoung Shim
0 siblings, 1 reply; 5+ messages in thread
From: Tobias Lorenz @ 2009-12-01 23:39 UTC (permalink / raw)
To: Joonyoung Shim; +Cc: linux-media, mchehab, kyungmin.park
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;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] radio-si470x: fix SYSCONFIG1 register set on si470x_start()
2009-12-01 23:39 ` Tobias Lorenz
@ 2009-12-01 23:54 ` Joonyoung Shim
2009-12-02 0:12 ` Tobias Lorenz
0 siblings, 1 reply; 5+ messages in thread
From: Joonyoung Shim @ 2009-12-01 23:54 UTC (permalink / raw)
To: Tobias Lorenz; +Cc: linux-media, mchehab, kyungmin.park
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;
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] radio-si470x: fix SYSCONFIG1 register set on si470x_start()
2009-12-01 23:54 ` Joonyoung Shim
@ 2009-12-02 0:12 ` Tobias Lorenz
2009-12-02 0:32 ` Joonyoung Shim
0 siblings, 1 reply; 5+ messages in thread
From: Tobias Lorenz @ 2009-12-02 0:12 UTC (permalink / raw)
To: Joonyoung Shim; +Cc: linux-media, mchehab, kyungmin.park
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;
> >>
> >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/3] radio-si470x: fix SYSCONFIG1 register set on si470x_start()
2009-12-02 0:12 ` Tobias Lorenz
@ 2009-12-02 0:32 ` Joonyoung Shim
0 siblings, 0 replies; 5+ messages in thread
From: Joonyoung Shim @ 2009-12-02 0:32 UTC (permalink / raw)
To: Tobias Lorenz; +Cc: linux-media, mchehab, kyungmin.park
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-12-02 0:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-11-18 6:21 [PATCH 1/3] radio-si470x: fix SYSCONFIG1 register set on si470x_start() Joonyoung Shim
2009-12-01 23:39 ` Tobias Lorenz
2009-12-01 23:54 ` Joonyoung Shim
2009-12-02 0:12 ` Tobias Lorenz
2009-12-02 0:32 ` Joonyoung Shim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).