tda18271 driver power consumption

  On Wed, Jul 25, 2012 at 11:18 PM, Michael Krufky <mkrufky@linuxtv.org> wrote:
> On Tue, Jul 24, 2012 at 8:43 PM, Antti Palosaari <crope@iki.fi> wrote:
>> On 07/25/2012 03:15 AM, Michael Krufky wrote:
>>>
>>> On Tue, Jul 24, 2012 at 6:17 PM, Michael Krufky <mkrufky@linuxtv.org>
>>> wrote:
>>>>
>>>> On Tue, Jul 24, 2012 at 6:12 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>>
>>>>> On 07/25/2012 12:55 AM, Michael Krufky wrote:
>>>>>>
>>>>>>
>>>>>> On Sun, Jul 22, 2012 at 3:59 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>>>>
>>>>>>>
>>>>>>> Moi Michael,
>>>>>>> I just realized tda18271 driver eats 160mA too much current after
>>>>>>> attach.
>>>>>>> This means, there is power management bug.
>>>>>>>
>>>>>>> When I plug my nanoStick it eats total 240mA, after tda18271 sleep is
>>>>>>> called
>>>>>>> it eats only 80mA total which is reasonable. If I use Digital Devices
>>>>>>> tda18271c2dd driver it is total 110mA after attach, which is also
>>>>>>> quite
>>>>>>> OK.
>>>>>>
>>>>>>
>>>>>>
>>>>>> Thanks for the report -- I will take a look at it.
>>>>>>
>>>>>> ...patches are welcome, of course :-)
>>>>>
>>>>>
>>>>>
>>>>> I suspect it does some tweaking on attach() and chip leaves powered (I
>>>>> saw
>>>>> demod debugs at calls I2C-gate control quite many times thus this
>>>>> suspicion). When chip is powered-up it is usually in some sleep state by
>>>>> default. Also, on attach() there should be no I/O unless very good
>>>>> reason.
>>>>> For example chip ID is allowed to read and download firmware in case it
>>>>> is
>>>>> really needed to continue - like for tuner communication.
>>>>>
>>>>>
>>>>> What I found quickly testing few DVB USB sticks there seems to be very
>>>>> much
>>>>> power management problems... I am now waiting for new multimeter in
>>>>> order to
>>>>> make better measurements and likely return fixing these issues later.
>>>>
>>>>
>>>> The driver does some calibration during attach, some of which is a
>>>> one-time initialization to determine a temperature differential for
>>>> tune calculation later on, which can take some time on slower USB
>>>> buses.  The "fix" for the power usage issue would just be to make sure
>>>> to sleep the device before exiting the attach() function.
>>>>
>>>> I'm not looking to remove the calibration from the attach -- this was
>>>> done on purpose.
>>>>
>>>
>>> Antti,
>>>
>>> After looking again, I realize that we are purposefully not sleeping
>>> the device before we exit the attach() function.
>>>
>>> The tda18271 is commonly found in multi-chip designs that may or may
>>> not include an analog demodulator and / or other tda18271 tuners.  In
>>> such designs, the chips tend to be daisy-chained to each other, using
>>> the xtal output and loop-thru features of the tda18271.  We set the
>>> required features in the attach-time configuration structure.
>>> However, we must keep in mind that this is a hybrid tuner chip, and
>>> the analog side of the bridge driver may actually come up before the
>>> digital side.  Since the actual configuration tends to be done in the
>>> digital bring-up, the analog side is brought up within tuner.ko using
>>> the most generic one-size-fits all configuration, which gets
>>> overridden when the digital side initializes.
>>>
>>> It is absolutely crucial that if we actually need the xtal output
>>> feature enabled, that it must *never* be turned off, otherwise the i2c
>>> bus may get wedged unrecoverably.  So, we make sure to leave this
>>> feature enabled during the attach function, since we don't yet know at
>>> that point whether there is another "instance" of this same tuner yet
>>> to be initialized.  It is not safe to power off that feature until
>>> after we are sure that the bridge has completely initialized.
>>>
>>> In order to rectify this issue from within your driver, you should
>>> call sleep after you complete the attach.  For instance, this is what
>>> we do in the cx23885 driver:
>>>
>>> if (fe0->dvb.frontend->ops.analog_ops.standby)
>>>
>>> fe0->dvb.frontend->ops.analog_ops.standby(fe0->dvb.frontend);
>>>
>>>
>>> ...except you should call into the tuner_ops->sleep() function instead
>>> of analog_demod_ops->standby()
>>>
>>> Does this clear things up for you?
>>
>>
>> Surely this is possible and it will resolve power drain issue. But it is not
>> nice looking and causes more deviation compared to others.
>>
>> Could you add configuration option "bool do_not_powerdown_on_attach" ?
>>
>> I have quite many tda18271 devices here and all those are DVB only? (OK,
>> PCTV 520e is DVB + analog, but analog is not supported). Having
>> configuration parameter sounds like better plan.
>
> Come to think of it, since the generic "one-size-fits-all"
> configuration leaves the loop thru and xtal output enabled, it should
> be safe to go to the lowest power level allowed (based on the
> configuration) at the end of the attach() function.  I'll put up a
> patch within the next few days...  Thanks for noticing this, Antti.
> :-)
>
> We wont need to add any new configuration option :-)
>
> -Mike

Antti,

This small patch should do the trick -- can you test it?


The following changes since commit 0c7d5a6da75caecc677be1fda207b7578936770d:

  Linux 3.5-rc5 (2012-07-03 22:57:41 +0300)

are available in the git repository at:

  git://git.linuxtv.org/mkrufky/tuners tda18271

for you to fetch changes up to 782b28e20d3b253d317cc71879639bf3c108b200:

  tda18271: enter low-power standby mode at the end of
tda18271_attach() (2012-07-26 08:34:37 -0400)

----------------------------------------------------------------
Michael Krufky (1):
      tda18271: enter low-power standby mode at the end of tda18271_attach()

 drivers/media/common/tuners/tda18271-fe.c |    3 +++
 1 file changed, 3 insertions(+)





Cheers,

Mike
  

On 07/26/2012 03:48 PM, Michael Krufky wrote:
> On Wed, Jul 25, 2012 at 11:18 PM, Michael Krufky <mkrufky@linuxtv.org> wrote:
>> On Tue, Jul 24, 2012 at 8:43 PM, Antti Palosaari <crope@iki.fi> wrote:
>>> On 07/25/2012 03:15 AM, Michael Krufky wrote:
>>>>
>>>> On Tue, Jul 24, 2012 at 6:17 PM, Michael Krufky <mkrufky@linuxtv.org>
>>>> wrote:
>>>>>
>>>>> On Tue, Jul 24, 2012 at 6:12 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>>>
>>>>>> On 07/25/2012 12:55 AM, Michael Krufky wrote:
>>>>>>>
>>>>>>>
>>>>>>> On Sun, Jul 22, 2012 at 3:59 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> Moi Michael,
>>>>>>>> I just realized tda18271 driver eats 160mA too much current after
>>>>>>>> attach.
>>>>>>>> This means, there is power management bug.
>>>>>>>>
>>>>>>>> When I plug my nanoStick it eats total 240mA, after tda18271 sleep is
>>>>>>>> called
>>>>>>>> it eats only 80mA total which is reasonable. If I use Digital Devices
>>>>>>>> tda18271c2dd driver it is total 110mA after attach, which is also
>>>>>>>> quite
>>>>>>>> OK.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Thanks for the report -- I will take a look at it.
>>>>>>>
>>>>>>> ...patches are welcome, of course :-)
>>>>>>
>>>>>>
>>>>>>
>>>>>> I suspect it does some tweaking on attach() and chip leaves powered (I
>>>>>> saw
>>>>>> demod debugs at calls I2C-gate control quite many times thus this
>>>>>> suspicion). When chip is powered-up it is usually in some sleep state by
>>>>>> default. Also, on attach() there should be no I/O unless very good
>>>>>> reason.
>>>>>> For example chip ID is allowed to read and download firmware in case it
>>>>>> is
>>>>>> really needed to continue - like for tuner communication.
>>>>>>
>>>>>>
>>>>>> What I found quickly testing few DVB USB sticks there seems to be very
>>>>>> much
>>>>>> power management problems... I am now waiting for new multimeter in
>>>>>> order to
>>>>>> make better measurements and likely return fixing these issues later.
>>>>>
>>>>>
>>>>> The driver does some calibration during attach, some of which is a
>>>>> one-time initialization to determine a temperature differential for
>>>>> tune calculation later on, which can take some time on slower USB
>>>>> buses.  The "fix" for the power usage issue would just be to make sure
>>>>> to sleep the device before exiting the attach() function.
>>>>>
>>>>> I'm not looking to remove the calibration from the attach -- this was
>>>>> done on purpose.
>>>>>
>>>>
>>>> Antti,
>>>>
>>>> After looking again, I realize that we are purposefully not sleeping
>>>> the device before we exit the attach() function.
>>>>
>>>> The tda18271 is commonly found in multi-chip designs that may or may
>>>> not include an analog demodulator and / or other tda18271 tuners.  In
>>>> such designs, the chips tend to be daisy-chained to each other, using
>>>> the xtal output and loop-thru features of the tda18271.  We set the
>>>> required features in the attach-time configuration structure.
>>>> However, we must keep in mind that this is a hybrid tuner chip, and
>>>> the analog side of the bridge driver may actually come up before the
>>>> digital side.  Since the actual configuration tends to be done in the
>>>> digital bring-up, the analog side is brought up within tuner.ko using
>>>> the most generic one-size-fits all configuration, which gets
>>>> overridden when the digital side initializes.
>>>>
>>>> It is absolutely crucial that if we actually need the xtal output
>>>> feature enabled, that it must *never* be turned off, otherwise the i2c
>>>> bus may get wedged unrecoverably.  So, we make sure to leave this
>>>> feature enabled during the attach function, since we don't yet know at
>>>> that point whether there is another "instance" of this same tuner yet
>>>> to be initialized.  It is not safe to power off that feature until
>>>> after we are sure that the bridge has completely initialized.
>>>>
>>>> In order to rectify this issue from within your driver, you should
>>>> call sleep after you complete the attach.  For instance, this is what
>>>> we do in the cx23885 driver:
>>>>
>>>> if (fe0->dvb.frontend->ops.analog_ops.standby)
>>>>
>>>> fe0->dvb.frontend->ops.analog_ops.standby(fe0->dvb.frontend);
>>>>
>>>>
>>>> ...except you should call into the tuner_ops->sleep() function instead
>>>> of analog_demod_ops->standby()
>>>>
>>>> Does this clear things up for you?
>>>
>>>
>>> Surely this is possible and it will resolve power drain issue. But it is not
>>> nice looking and causes more deviation compared to others.
>>>
>>> Could you add configuration option "bool do_not_powerdown_on_attach" ?
>>>
>>> I have quite many tda18271 devices here and all those are DVB only? (OK,
>>> PCTV 520e is DVB + analog, but analog is not supported). Having
>>> configuration parameter sounds like better plan.
>>
>> Come to think of it, since the generic "one-size-fits-all"
>> configuration leaves the loop thru and xtal output enabled, it should
>> be safe to go to the lowest power level allowed (based on the
>> configuration) at the end of the attach() function.  I'll put up a
>> patch within the next few days...  Thanks for noticing this, Antti.
>> :-)
>>
>> We wont need to add any new configuration option :-)
>>
>> -Mike
>
> Antti,
>
> This small patch should do the trick -- can you test it?


Tested-by: Antti Palosaari <crope@iki.fi>

>
>
> The following changes since commit 0c7d5a6da75caecc677be1fda207b7578936770d:
>
>    Linux 3.5-rc5 (2012-07-03 22:57:41 +0300)
>
> are available in the git repository at:
>
>    git://git.linuxtv.org/mkrufky/tuners tda18271
>
> for you to fetch changes up to 782b28e20d3b253d317cc71879639bf3c108b200:
>
>    tda18271: enter low-power standby mode at the end of
> tda18271_attach() (2012-07-26 08:34:37 -0400)
>
> ----------------------------------------------------------------
> Michael Krufky (1):
>        tda18271: enter low-power standby mode at the end of tda18271_attach()
>
>   drivers/media/common/tuners/tda18271-fe.c |    3 +++
>   1 file changed, 3 insertions(+)
>
>
>
>
>
> Cheers,
>
> Mike
>
  

Em Thu, 26 Jul 2012 08:48:58 -0400
Michael Krufky <mkrufky@linuxtv.org> escreveu:

> Antti,
> 
> This small patch should do the trick -- can you test it?
> 
> 
> The following changes since commit 0c7d5a6da75caecc677be1fda207b7578936770d:
> 
>   Linux 3.5-rc5 (2012-07-03 22:57:41 +0300)
> 
> are available in the git repository at:
> 
>   git://git.linuxtv.org/mkrufky/tuners tda18271
> 
> for you to fetch changes up to 782b28e20d3b253d317cc71879639bf3c108b200:
> 
>   tda18271: enter low-power standby mode at the end of
> tda18271_attach() (2012-07-26 08:34:37 -0400)
> 
> ----------------------------------------------------------------
> Michael Krufky (1):
>       tda18271: enter low-power standby mode at the end of tda18271_attach()
> 
>  drivers/media/common/tuners/tda18271-fe.c |    3 +++
>  1 file changed, 3 insertions(+)


Mike,

Despite patchwork's way of handling, thinking that this is a pull request,
I suspect that your intention here were simply offer some patches for Antti
to test.

In any case, please always send the patches via email to the ML before
sending a pull request. This was always a rule, but some developers are
lazy with this duty, and, as I didn't use to have a tool to double check,
bad things happen.

I'm now finally able to check with a simple script if weather a patch
went to the ML or not. My script checks both reply-to/references email
tags and it looks for the same patch subject at the ML Inbox.
So, I'll be now be more grumpy with that ;) [1]

So, please be sure to post those patches at the ML, with Antti's tested-by:
tag, before sending a pull request.

Thanks!
Mauro

[1] Side note: it is not actually a matter of being grumpy; posted patches
receive a lot more attention/review than simple pull requests. From time 
to time, patches that went via the wrong way (e. g. without a previous post) 
caused troubles for other developers. So, enforcing it is actually a matter
of improving Kernel quality and avoiding regressions.

-

$ test_patch
testing if patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch applies
patch -p1 -i patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch --dry-run -t -N
patching file drivers/media/tuners/tda18271-fe.c
 drivers/media/tuners/tda18271-fe.c |    3 +++
 1 file changed, 3 insertions(+)
Subject: tda18271: enter low-power standby mode at the end of  tda18271_attach()
From: Michael Krufky <mkrufky@linuxtv.org>
Date: Thu, 26 Jul 2012 08:34:37 -0400
Patch applies OK
total: 0 errors, 0 warnings, 9 lines checked

patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch has no obvious style problems and is ready for submission.
Didn't find any message with subject equal to 'tda18271: enter low-power standby mode at the end of tda18271_attach()'
Duplicated md5sum patches
Likely duplicated patches (need manual check)
--
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 09/27/2012 10:19 PM, Mauro Carvalho Chehab wrote:
> Em Thu, 26 Jul 2012 08:48:58 -0400
> Michael Krufky <mkrufky@linuxtv.org> escreveu:
>
>> Antti,
>>
>> This small patch should do the trick -- can you test it?
>>
>>
>> The following changes since commit 0c7d5a6da75caecc677be1fda207b7578936770d:
>>
>>    Linux 3.5-rc5 (2012-07-03 22:57:41 +0300)
>>
>> are available in the git repository at:
>>
>>    git://git.linuxtv.org/mkrufky/tuners tda18271
>>
>> for you to fetch changes up to 782b28e20d3b253d317cc71879639bf3c108b200:
>>
>>    tda18271: enter low-power standby mode at the end of
>> tda18271_attach() (2012-07-26 08:34:37 -0400)
>>
>> ----------------------------------------------------------------
>> Michael Krufky (1):
>>        tda18271: enter low-power standby mode at the end of tda18271_attach()
>>
>>   drivers/media/common/tuners/tda18271-fe.c |    3 +++
>>   1 file changed, 3 insertions(+)
>
>
> Mike,
>
> Despite patchwork's way of handling, thinking that this is a pull request,
> I suspect that your intention here were simply offer some patches for Antti
> to test.
>
> In any case, please always send the patches via email to the ML before
> sending a pull request. This was always a rule, but some developers are
> lazy with this duty, and, as I didn't use to have a tool to double check,
> bad things happen.
>
> I'm now finally able to check with a simple script if weather a patch
> went to the ML or not. My script checks both reply-to/references email
> tags and it looks for the same patch subject at the ML Inbox.
> So, I'll be now be more grumpy with that ;) [1]
>
> So, please be sure to post those patches at the ML, with Antti's tested-by:
> tag, before sending a pull request.
>
> Thanks!
> Mauro
>
> [1] Side note: it is not actually a matter of being grumpy; posted patches
> receive a lot more attention/review than simple pull requests. From time
> to time, patches that went via the wrong way (e. g. without a previous post)
> caused troubles for other developers. So, enforcing it is actually a matter
> of improving Kernel quality and avoiding regressions.
>
> -
>
> $ test_patch
> testing if patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch applies
> patch -p1 -i patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch --dry-run -t -N
> patching file drivers/media/tuners/tda18271-fe.c
>   drivers/media/tuners/tda18271-fe.c |    3 +++
>   1 file changed, 3 insertions(+)
> Subject: tda18271: enter low-power standby mode at the end of  tda18271_attach()
> From: Michael Krufky <mkrufky@linuxtv.org>
> Date: Thu, 26 Jul 2012 08:34:37 -0400
> Patch applies OK
> total: 0 errors, 0 warnings, 9 lines checked
>
> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch has no obvious style problems and is ready for submission.
> Didn't find any message with subject equal to 'tda18271: enter low-power standby mode at the end of tda18271_attach()'
> Duplicated md5sum patches
> Likely duplicated patches (need manual check)

If that tda18271 patch is not applied then these two should be:

https://patchwork.kernel.org/patch/1481901/
https://patchwork.kernel.org/patch/1481911/

regards
Antti
  

On Thu, Sep 27, 2012 at 3:59 PM, Antti Palosaari <crope@iki.fi> wrote:
> On 09/27/2012 10:19 PM, Mauro Carvalho Chehab wrote:
>>
>> Em Thu, 26 Jul 2012 08:48:58 -0400
>> Michael Krufky <mkrufky@linuxtv.org> escreveu:
>>
>>> Antti,
>>>
>>> This small patch should do the trick -- can you test it?
>>>
>>>
>>> The following changes since commit
>>> 0c7d5a6da75caecc677be1fda207b7578936770d:
>>>
>>>    Linux 3.5-rc5 (2012-07-03 22:57:41 +0300)
>>>
>>> are available in the git repository at:
>>>
>>>    git://git.linuxtv.org/mkrufky/tuners tda18271
>>>
>>> for you to fetch changes up to 782b28e20d3b253d317cc71879639bf3c108b200:
>>>
>>>    tda18271: enter low-power standby mode at the end of
>>> tda18271_attach() (2012-07-26 08:34:37 -0400)
>>>
>>> ----------------------------------------------------------------
>>> Michael Krufky (1):
>>>        tda18271: enter low-power standby mode at the end of
>>> tda18271_attach()
>>>
>>>   drivers/media/common/tuners/tda18271-fe.c |    3 +++
>>>   1 file changed, 3 insertions(+)
>>
>>
>>
>> Mike,
>>
>> Despite patchwork's way of handling, thinking that this is a pull request,
>> I suspect that your intention here were simply offer some patches for
>> Antti
>> to test.
>>
>> In any case, please always send the patches via email to the ML before
>> sending a pull request. This was always a rule, but some developers are
>> lazy with this duty, and, as I didn't use to have a tool to double check,
>> bad things happen.
>>
>> I'm now finally able to check with a simple script if weather a patch
>> went to the ML or not. My script checks both reply-to/references email
>> tags and it looks for the same patch subject at the ML Inbox.
>> So, I'll be now be more grumpy with that ;) [1]
>>
>> So, please be sure to post those patches at the ML, with Antti's
>> tested-by:
>> tag, before sending a pull request.
>>
>> Thanks!
>> Mauro
>>
>> [1] Side note: it is not actually a matter of being grumpy; posted patches
>> receive a lot more attention/review than simple pull requests. From time
>> to time, patches that went via the wrong way (e. g. without a previous
>> post)
>> caused troubles for other developers. So, enforcing it is actually a
>> matter
>> of improving Kernel quality and avoiding regressions.
>>
>> -
>>
>> $ test_patch
>> testing if
>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>> applies
>> patch -p1 -i
>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>> --dry-run -t -N
>> patching file drivers/media/tuners/tda18271-fe.c
>>   drivers/media/tuners/tda18271-fe.c |    3 +++
>>   1 file changed, 3 insertions(+)
>> Subject: tda18271: enter low-power standby mode at the end of
>> tda18271_attach()
>> From: Michael Krufky <mkrufky@linuxtv.org>
>> Date: Thu, 26 Jul 2012 08:34:37 -0400
>> Patch applies OK
>> total: 0 errors, 0 warnings, 9 lines checked
>>
>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>> has no obvious style problems and is ready for submission.
>> Didn't find any message with subject equal to 'tda18271: enter low-power
>> standby mode at the end of tda18271_attach()'
>> Duplicated md5sum patches
>> Likely duplicated patches (need manual check)
>
>
> If that tda18271 patch is not applied then these two should be:
>
> https://patchwork.kernel.org/patch/1481901/
> https://patchwork.kernel.org/patch/1481911/
>
>
> regards
> Antti
>
> --
> http://palosaari.fi/

The tda18271 patch should indeed be applied -- I will send it to the
ML later on today and follow up with a pull request.  Thanks to all
who have commented :-)

-Mike
--
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 09/28/2012 12:20 AM, Michael Krufky wrote:
> On Thu, Sep 27, 2012 at 3:59 PM, Antti Palosaari <crope@iki.fi> wrote:
>> On 09/27/2012 10:19 PM, Mauro Carvalho Chehab wrote:
>>>
>>> Em Thu, 26 Jul 2012 08:48:58 -0400
>>> Michael Krufky <mkrufky@linuxtv.org> escreveu:
>>>
>>>> Antti,
>>>>
>>>> This small patch should do the trick -- can you test it?
>>>>
>>>>
>>>> The following changes since commit
>>>> 0c7d5a6da75caecc677be1fda207b7578936770d:
>>>>
>>>>     Linux 3.5-rc5 (2012-07-03 22:57:41 +0300)
>>>>
>>>> are available in the git repository at:
>>>>
>>>>     git://git.linuxtv.org/mkrufky/tuners tda18271
>>>>
>>>> for you to fetch changes up to 782b28e20d3b253d317cc71879639bf3c108b200:
>>>>
>>>>     tda18271: enter low-power standby mode at the end of
>>>> tda18271_attach() (2012-07-26 08:34:37 -0400)
>>>>
>>>> ----------------------------------------------------------------
>>>> Michael Krufky (1):
>>>>         tda18271: enter low-power standby mode at the end of
>>>> tda18271_attach()
>>>>
>>>>    drivers/media/common/tuners/tda18271-fe.c |    3 +++
>>>>    1 file changed, 3 insertions(+)
>>>
>>>
>>>
>>> Mike,
>>>
>>> Despite patchwork's way of handling, thinking that this is a pull request,
>>> I suspect that your intention here were simply offer some patches for
>>> Antti
>>> to test.
>>>
>>> In any case, please always send the patches via email to the ML before
>>> sending a pull request. This was always a rule, but some developers are
>>> lazy with this duty, and, as I didn't use to have a tool to double check,
>>> bad things happen.
>>>
>>> I'm now finally able to check with a simple script if weather a patch
>>> went to the ML or not. My script checks both reply-to/references email
>>> tags and it looks for the same patch subject at the ML Inbox.
>>> So, I'll be now be more grumpy with that ;) [1]
>>>
>>> So, please be sure to post those patches at the ML, with Antti's
>>> tested-by:
>>> tag, before sending a pull request.
>>>
>>> Thanks!
>>> Mauro
>>>
>>> [1] Side note: it is not actually a matter of being grumpy; posted patches
>>> receive a lot more attention/review than simple pull requests. From time
>>> to time, patches that went via the wrong way (e. g. without a previous
>>> post)
>>> caused troubles for other developers. So, enforcing it is actually a
>>> matter
>>> of improving Kernel quality and avoiding regressions.
>>>
>>> -
>>>
>>> $ test_patch
>>> testing if
>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>> applies
>>> patch -p1 -i
>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>> --dry-run -t -N
>>> patching file drivers/media/tuners/tda18271-fe.c
>>>    drivers/media/tuners/tda18271-fe.c |    3 +++
>>>    1 file changed, 3 insertions(+)
>>> Subject: tda18271: enter low-power standby mode at the end of
>>> tda18271_attach()
>>> From: Michael Krufky <mkrufky@linuxtv.org>
>>> Date: Thu, 26 Jul 2012 08:34:37 -0400
>>> Patch applies OK
>>> total: 0 errors, 0 warnings, 9 lines checked
>>>
>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>> has no obvious style problems and is ready for submission.
>>> Didn't find any message with subject equal to 'tda18271: enter low-power
>>> standby mode at the end of tda18271_attach()'
>>> Duplicated md5sum patches
>>> Likely duplicated patches (need manual check)
>>
>>
>> If that tda18271 patch is not applied then these two should be:
>>
>> https://patchwork.kernel.org/patch/1481901/
>> https://patchwork.kernel.org/patch/1481911/
>>
>>
>> regards
>> Antti
>>
>> --
>> http://palosaari.fi/
>
> The tda18271 patch should indeed be applied -- I will send it to the
> ML later on today and follow up with a pull request.  Thanks to all
> who have commented :-)

Mike, There is other problem too. PCTV 520e, which is Em28xx + DRX-K + 
TDA18271, fails to attach tuner now. Tuner is wired behind DRX-K I2C 
bus. TDA18271 driver does very much I/O during attach and I2C error is 
raised during attach now. Earlier it worked as DRX-K firmware was 
downloaded before tuner was attached, but now both DRX-K fw download and 
tuner attach happens same time leading that error.

regards
Antti
  

On Thu, Sep 27, 2012 at 5:38 PM, Antti Palosaari <crope@iki.fi> wrote:
> On 09/28/2012 12:20 AM, Michael Krufky wrote:
>>
>> On Thu, Sep 27, 2012 at 3:59 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>
>>> On 09/27/2012 10:19 PM, Mauro Carvalho Chehab wrote:
>>>>
>>>>
>>>> Em Thu, 26 Jul 2012 08:48:58 -0400
>>>> Michael Krufky <mkrufky@linuxtv.org> escreveu:
>>>>
>>>>> Antti,
>>>>>
>>>>> This small patch should do the trick -- can you test it?
>>>>>
>>>>>
>>>>> The following changes since commit
>>>>> 0c7d5a6da75caecc677be1fda207b7578936770d:
>>>>>
>>>>>     Linux 3.5-rc5 (2012-07-03 22:57:41 +0300)
>>>>>
>>>>> are available in the git repository at:
>>>>>
>>>>>     git://git.linuxtv.org/mkrufky/tuners tda18271
>>>>>
>>>>> for you to fetch changes up to
>>>>> 782b28e20d3b253d317cc71879639bf3c108b200:
>>>>>
>>>>>     tda18271: enter low-power standby mode at the end of
>>>>> tda18271_attach() (2012-07-26 08:34:37 -0400)
>>>>>
>>>>> ----------------------------------------------------------------
>>>>> Michael Krufky (1):
>>>>>         tda18271: enter low-power standby mode at the end of
>>>>> tda18271_attach()
>>>>>
>>>>>    drivers/media/common/tuners/tda18271-fe.c |    3 +++
>>>>>    1 file changed, 3 insertions(+)
>>>>
>>>>
>>>>
>>>>
>>>> Mike,
>>>>
>>>> Despite patchwork's way of handling, thinking that this is a pull
>>>> request,
>>>> I suspect that your intention here were simply offer some patches for
>>>> Antti
>>>> to test.
>>>>
>>>> In any case, please always send the patches via email to the ML before
>>>> sending a pull request. This was always a rule, but some developers are
>>>> lazy with this duty, and, as I didn't use to have a tool to double
>>>> check,
>>>> bad things happen.
>>>>
>>>> I'm now finally able to check with a simple script if weather a patch
>>>> went to the ML or not. My script checks both reply-to/references email
>>>> tags and it looks for the same patch subject at the ML Inbox.
>>>> So, I'll be now be more grumpy with that ;) [1]
>>>>
>>>> So, please be sure to post those patches at the ML, with Antti's
>>>> tested-by:
>>>> tag, before sending a pull request.
>>>>
>>>> Thanks!
>>>> Mauro
>>>>
>>>> [1] Side note: it is not actually a matter of being grumpy; posted
>>>> patches
>>>> receive a lot more attention/review than simple pull requests. From time
>>>> to time, patches that went via the wrong way (e. g. without a previous
>>>> post)
>>>> caused troubles for other developers. So, enforcing it is actually a
>>>> matter
>>>> of improving Kernel quality and avoiding regressions.
>>>>
>>>> -
>>>>
>>>> $ test_patch
>>>> testing if
>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>> applies
>>>> patch -p1 -i
>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>> --dry-run -t -N
>>>> patching file drivers/media/tuners/tda18271-fe.c
>>>>    drivers/media/tuners/tda18271-fe.c |    3 +++
>>>>    1 file changed, 3 insertions(+)
>>>> Subject: tda18271: enter low-power standby mode at the end of
>>>> tda18271_attach()
>>>> From: Michael Krufky <mkrufky@linuxtv.org>
>>>> Date: Thu, 26 Jul 2012 08:34:37 -0400
>>>> Patch applies OK
>>>> total: 0 errors, 0 warnings, 9 lines checked
>>>>
>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>> has no obvious style problems and is ready for submission.
>>>> Didn't find any message with subject equal to 'tda18271: enter low-power
>>>> standby mode at the end of tda18271_attach()'
>>>> Duplicated md5sum patches
>>>> Likely duplicated patches (need manual check)
>>>
>>>
>>>
>>> If that tda18271 patch is not applied then these two should be:
>>>
>>> https://patchwork.kernel.org/patch/1481901/
>>> https://patchwork.kernel.org/patch/1481911/
>>>
>>>
>>> regards
>>> Antti
>>>
>>> --
>>> http://palosaari.fi/
>>
>>
>> The tda18271 patch should indeed be applied -- I will send it to the
>> ML later on today and follow up with a pull request.  Thanks to all
>> who have commented :-)
>
>
> Mike, There is other problem too. PCTV 520e, which is Em28xx + DRX-K +
> TDA18271, fails to attach tuner now. Tuner is wired behind DRX-K I2C bus.
> TDA18271 driver does very much I/O during attach and I2C error is raised
> during attach now. Earlier it worked as DRX-K firmware was downloaded before
> tuner was attached, but now both DRX-K fw download and tuner attach happens
> same time leading that error.

Why is the DRX-K firmware downloading at the same time as tuner
attach?  Shouldn't the demod attach be finished before the tuner
attach begins?

-Mike
--
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 09/28/2012 12:58 AM, Michael Krufky wrote:
> On Thu, Sep 27, 2012 at 5:38 PM, Antti Palosaari <crope@iki.fi> wrote:
>> On 09/28/2012 12:20 AM, Michael Krufky wrote:
>>>
>>> On Thu, Sep 27, 2012 at 3:59 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>
>>>> On 09/27/2012 10:19 PM, Mauro Carvalho Chehab wrote:
>>>>>
>>>>>
>>>>> Em Thu, 26 Jul 2012 08:48:58 -0400
>>>>> Michael Krufky <mkrufky@linuxtv.org> escreveu:
>>>>>
>>>>>> Antti,
>>>>>>
>>>>>> This small patch should do the trick -- can you test it?
>>>>>>
>>>>>>
>>>>>> The following changes since commit
>>>>>> 0c7d5a6da75caecc677be1fda207b7578936770d:
>>>>>>
>>>>>>      Linux 3.5-rc5 (2012-07-03 22:57:41 +0300)
>>>>>>
>>>>>> are available in the git repository at:
>>>>>>
>>>>>>      git://git.linuxtv.org/mkrufky/tuners tda18271
>>>>>>
>>>>>> for you to fetch changes up to
>>>>>> 782b28e20d3b253d317cc71879639bf3c108b200:
>>>>>>
>>>>>>      tda18271: enter low-power standby mode at the end of
>>>>>> tda18271_attach() (2012-07-26 08:34:37 -0400)
>>>>>>
>>>>>> ----------------------------------------------------------------
>>>>>> Michael Krufky (1):
>>>>>>          tda18271: enter low-power standby mode at the end of
>>>>>> tda18271_attach()
>>>>>>
>>>>>>     drivers/media/common/tuners/tda18271-fe.c |    3 +++
>>>>>>     1 file changed, 3 insertions(+)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Mike,
>>>>>
>>>>> Despite patchwork's way of handling, thinking that this is a pull
>>>>> request,
>>>>> I suspect that your intention here were simply offer some patches for
>>>>> Antti
>>>>> to test.
>>>>>
>>>>> In any case, please always send the patches via email to the ML before
>>>>> sending a pull request. This was always a rule, but some developers are
>>>>> lazy with this duty, and, as I didn't use to have a tool to double
>>>>> check,
>>>>> bad things happen.
>>>>>
>>>>> I'm now finally able to check with a simple script if weather a patch
>>>>> went to the ML or not. My script checks both reply-to/references email
>>>>> tags and it looks for the same patch subject at the ML Inbox.
>>>>> So, I'll be now be more grumpy with that ;) [1]
>>>>>
>>>>> So, please be sure to post those patches at the ML, with Antti's
>>>>> tested-by:
>>>>> tag, before sending a pull request.
>>>>>
>>>>> Thanks!
>>>>> Mauro
>>>>>
>>>>> [1] Side note: it is not actually a matter of being grumpy; posted
>>>>> patches
>>>>> receive a lot more attention/review than simple pull requests. From time
>>>>> to time, patches that went via the wrong way (e. g. without a previous
>>>>> post)
>>>>> caused troubles for other developers. So, enforcing it is actually a
>>>>> matter
>>>>> of improving Kernel quality and avoiding regressions.
>>>>>
>>>>> -
>>>>>
>>>>> $ test_patch
>>>>> testing if
>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>> applies
>>>>> patch -p1 -i
>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>> --dry-run -t -N
>>>>> patching file drivers/media/tuners/tda18271-fe.c
>>>>>     drivers/media/tuners/tda18271-fe.c |    3 +++
>>>>>     1 file changed, 3 insertions(+)
>>>>> Subject: tda18271: enter low-power standby mode at the end of
>>>>> tda18271_attach()
>>>>> From: Michael Krufky <mkrufky@linuxtv.org>
>>>>> Date: Thu, 26 Jul 2012 08:34:37 -0400
>>>>> Patch applies OK
>>>>> total: 0 errors, 0 warnings, 9 lines checked
>>>>>
>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>> has no obvious style problems and is ready for submission.
>>>>> Didn't find any message with subject equal to 'tda18271: enter low-power
>>>>> standby mode at the end of tda18271_attach()'
>>>>> Duplicated md5sum patches
>>>>> Likely duplicated patches (need manual check)
>>>>
>>>>
>>>>
>>>> If that tda18271 patch is not applied then these two should be:
>>>>
>>>> https://patchwork.kernel.org/patch/1481901/
>>>> https://patchwork.kernel.org/patch/1481911/
>>>>
>>>>
>>>> regards
>>>> Antti
>>>>
>>>> --
>>>> http://palosaari.fi/
>>>
>>>
>>> The tda18271 patch should indeed be applied -- I will send it to the
>>> ML later on today and follow up with a pull request.  Thanks to all
>>> who have commented :-)
>>
>>
>> Mike, There is other problem too. PCTV 520e, which is Em28xx + DRX-K +
>> TDA18271, fails to attach tuner now. Tuner is wired behind DRX-K I2C bus.
>> TDA18271 driver does very much I/O during attach and I2C error is raised
>> during attach now. Earlier it worked as DRX-K firmware was downloaded before
>> tuner was attached, but now both DRX-K fw download and tuner attach happens
>> same time leading that error.
>
> Why is the DRX-K firmware downloading at the same time as tuner
> attach?  Shouldn't the demod attach be finished before the tuner
> attach begins?

What I think all these should go in order bridge => demod => tuner. 
Attach and fw loading. I cannot see how it will never work generally 
unless those firmwares are loaded in that order.
1) bridge needs firmware up and running before demod could be attached. 
That is mostly because I2C adapter is behind bridge.

2) demod needs firmware up and running before tuner could be attached. 
That is mostly because I2C adapter/bus for tuner is behind the demod.

3) tuner needs firmware up and running as there could be firmware 
controlled GPIO bus behind tuner. There is many times LNA, LNB 
controller, LED, antenna switch. I am not surprised if there is even I2C 
bus behind the tuner to control LNB or other equipment near antenna 
connector and tuner.

Of course situation is not that bad usually - but surely there could be 
some existing device which is very near that. But as we *want* to do 
things as general as possible to avoid driver / device specific hacks 
that is the only reasonable model.

regards
Antti
  

On Thu, Sep 27, 2012 at 6:26 PM, Antti Palosaari <crope@iki.fi> wrote:
> On 09/28/2012 12:58 AM, Michael Krufky wrote:
>>
>> On Thu, Sep 27, 2012 at 5:38 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>
>>> On 09/28/2012 12:20 AM, Michael Krufky wrote:
>>>>
>>>>
>>>> On Thu, Sep 27, 2012 at 3:59 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>>
>>>>>
>>>>> On 09/27/2012 10:19 PM, Mauro Carvalho Chehab wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Em Thu, 26 Jul 2012 08:48:58 -0400
>>>>>> Michael Krufky <mkrufky@linuxtv.org> escreveu:
>>>>>>
>>>>>>> Antti,
>>>>>>>
>>>>>>> This small patch should do the trick -- can you test it?
>>>>>>>
>>>>>>>
>>>>>>> The following changes since commit
>>>>>>> 0c7d5a6da75caecc677be1fda207b7578936770d:
>>>>>>>
>>>>>>>      Linux 3.5-rc5 (2012-07-03 22:57:41 +0300)
>>>>>>>
>>>>>>> are available in the git repository at:
>>>>>>>
>>>>>>>      git://git.linuxtv.org/mkrufky/tuners tda18271
>>>>>>>
>>>>>>> for you to fetch changes up to
>>>>>>> 782b28e20d3b253d317cc71879639bf3c108b200:
>>>>>>>
>>>>>>>      tda18271: enter low-power standby mode at the end of
>>>>>>> tda18271_attach() (2012-07-26 08:34:37 -0400)
>>>>>>>
>>>>>>> ----------------------------------------------------------------
>>>>>>> Michael Krufky (1):
>>>>>>>          tda18271: enter low-power standby mode at the end of
>>>>>>> tda18271_attach()
>>>>>>>
>>>>>>>     drivers/media/common/tuners/tda18271-fe.c |    3 +++
>>>>>>>     1 file changed, 3 insertions(+)
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Mike,
>>>>>>
>>>>>> Despite patchwork's way of handling, thinking that this is a pull
>>>>>> request,
>>>>>> I suspect that your intention here were simply offer some patches for
>>>>>> Antti
>>>>>> to test.
>>>>>>
>>>>>> In any case, please always send the patches via email to the ML before
>>>>>> sending a pull request. This was always a rule, but some developers
>>>>>> are
>>>>>> lazy with this duty, and, as I didn't use to have a tool to double
>>>>>> check,
>>>>>> bad things happen.
>>>>>>
>>>>>> I'm now finally able to check with a simple script if weather a patch
>>>>>> went to the ML or not. My script checks both reply-to/references email
>>>>>> tags and it looks for the same patch subject at the ML Inbox.
>>>>>> So, I'll be now be more grumpy with that ;) [1]
>>>>>>
>>>>>> So, please be sure to post those patches at the ML, with Antti's
>>>>>> tested-by:
>>>>>> tag, before sending a pull request.
>>>>>>
>>>>>> Thanks!
>>>>>> Mauro
>>>>>>
>>>>>> [1] Side note: it is not actually a matter of being grumpy; posted
>>>>>> patches
>>>>>> receive a lot more attention/review than simple pull requests. From
>>>>>> time
>>>>>> to time, patches that went via the wrong way (e. g. without a previous
>>>>>> post)
>>>>>> caused troubles for other developers. So, enforcing it is actually a
>>>>>> matter
>>>>>> of improving Kernel quality and avoiding regressions.
>>>>>>
>>>>>> -
>>>>>>
>>>>>> $ test_patch
>>>>>> testing if
>>>>>>
>>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>>> applies
>>>>>> patch -p1 -i
>>>>>>
>>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>>> --dry-run -t -N
>>>>>> patching file drivers/media/tuners/tda18271-fe.c
>>>>>>     drivers/media/tuners/tda18271-fe.c |    3 +++
>>>>>>     1 file changed, 3 insertions(+)
>>>>>> Subject: tda18271: enter low-power standby mode at the end of
>>>>>> tda18271_attach()
>>>>>> From: Michael Krufky <mkrufky@linuxtv.org>
>>>>>> Date: Thu, 26 Jul 2012 08:34:37 -0400
>>>>>> Patch applies OK
>>>>>> total: 0 errors, 0 warnings, 9 lines checked
>>>>>>
>>>>>>
>>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>>> has no obvious style problems and is ready for submission.
>>>>>> Didn't find any message with subject equal to 'tda18271: enter
>>>>>> low-power
>>>>>> standby mode at the end of tda18271_attach()'
>>>>>> Duplicated md5sum patches
>>>>>> Likely duplicated patches (need manual check)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> If that tda18271 patch is not applied then these two should be:
>>>>>
>>>>> https://patchwork.kernel.org/patch/1481901/
>>>>> https://patchwork.kernel.org/patch/1481911/
>>>>>
>>>>>
>>>>> regards
>>>>> Antti
>>>>>
>>>>> --
>>>>> http://palosaari.fi/
>>>>
>>>>
>>>>
>>>> The tda18271 patch should indeed be applied -- I will send it to the
>>>> ML later on today and follow up with a pull request.  Thanks to all
>>>> who have commented :-)
>>>
>>>
>>>
>>> Mike, There is other problem too. PCTV 520e, which is Em28xx + DRX-K +
>>> TDA18271, fails to attach tuner now. Tuner is wired behind DRX-K I2C bus.
>>> TDA18271 driver does very much I/O during attach and I2C error is raised
>>> during attach now. Earlier it worked as DRX-K firmware was downloaded
>>> before
>>> tuner was attached, but now both DRX-K fw download and tuner attach
>>> happens
>>> same time leading that error.
>>
>>
>> Why is the DRX-K firmware downloading at the same time as tuner
>> attach?  Shouldn't the demod attach be finished before the tuner
>> attach begins?
>
>
> What I think all these should go in order bridge => demod => tuner. Attach
> and fw loading. I cannot see how it will never work generally unless those
> firmwares are loaded in that order.
> 1) bridge needs firmware up and running before demod could be attached. That
> is mostly because I2C adapter is behind bridge.
>
> 2) demod needs firmware up and running before tuner could be attached. That
> is mostly because I2C adapter/bus for tuner is behind the demod.
>
> 3) tuner needs firmware up and running as there could be firmware controlled
> GPIO bus behind tuner. There is many times LNA, LNB controller, LED, antenna
> switch. I am not surprised if there is even I2C bus behind the tuner to
> control LNB or other equipment near antenna connector and tuner.
>
> Of course situation is not that bad usually - but surely there could be some
> existing device which is very near that. But as we *want* to do things as
> general as possible to avoid driver / device specific hacks that is the only
> reasonable model.
>


I'm not sure how that relates to the problem you brought up.....
back to the issue:

I don't have the PCTV 520e schematics handy, but...  it's possible
that the DRX-K depends on XTOUT from the tda18271 --

In your struct tda18271_config, are you using the .output_opt
configuration?  for example:

struct tda18271_config hauppauge_tda18271_config = {
        .std_map = &hauppauge_tda18271_std_map,
        .gate    = TDA18271_GATE_ANALOG,
        .output_opt = TDA18271_OUTPUT_LT_OFF,
};


If so, try deleting the .output_opt line - see if that helps.

-Mike
--
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 09/28/2012 01:43 AM, Michael Krufky wrote:
> On Thu, Sep 27, 2012 at 6:26 PM, Antti Palosaari <crope@iki.fi> wrote:
>> On 09/28/2012 12:58 AM, Michael Krufky wrote:
>>>
>>> On Thu, Sep 27, 2012 at 5:38 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>
>>>> On 09/28/2012 12:20 AM, Michael Krufky wrote:
>>>>>
>>>>>
>>>>> On Thu, Sep 27, 2012 at 3:59 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>>>
>>>>>>
>>>>>> On 09/27/2012 10:19 PM, Mauro Carvalho Chehab wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Em Thu, 26 Jul 2012 08:48:58 -0400
>>>>>>> Michael Krufky <mkrufky@linuxtv.org> escreveu:
>>>>>>>
>>>>>>>> Antti,
>>>>>>>>
>>>>>>>> This small patch should do the trick -- can you test it?
>>>>>>>>
>>>>>>>>
>>>>>>>> The following changes since commit
>>>>>>>> 0c7d5a6da75caecc677be1fda207b7578936770d:
>>>>>>>>
>>>>>>>>       Linux 3.5-rc5 (2012-07-03 22:57:41 +0300)
>>>>>>>>
>>>>>>>> are available in the git repository at:
>>>>>>>>
>>>>>>>>       git://git.linuxtv.org/mkrufky/tuners tda18271
>>>>>>>>
>>>>>>>> for you to fetch changes up to
>>>>>>>> 782b28e20d3b253d317cc71879639bf3c108b200:
>>>>>>>>
>>>>>>>>       tda18271: enter low-power standby mode at the end of
>>>>>>>> tda18271_attach() (2012-07-26 08:34:37 -0400)
>>>>>>>>
>>>>>>>> ----------------------------------------------------------------
>>>>>>>> Michael Krufky (1):
>>>>>>>>           tda18271: enter low-power standby mode at the end of
>>>>>>>> tda18271_attach()
>>>>>>>>
>>>>>>>>      drivers/media/common/tuners/tda18271-fe.c |    3 +++
>>>>>>>>      1 file changed, 3 insertions(+)
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Mike,
>>>>>>>
>>>>>>> Despite patchwork's way of handling, thinking that this is a pull
>>>>>>> request,
>>>>>>> I suspect that your intention here were simply offer some patches for
>>>>>>> Antti
>>>>>>> to test.
>>>>>>>
>>>>>>> In any case, please always send the patches via email to the ML before
>>>>>>> sending a pull request. This was always a rule, but some developers
>>>>>>> are
>>>>>>> lazy with this duty, and, as I didn't use to have a tool to double
>>>>>>> check,
>>>>>>> bad things happen.
>>>>>>>
>>>>>>> I'm now finally able to check with a simple script if weather a patch
>>>>>>> went to the ML or not. My script checks both reply-to/references email
>>>>>>> tags and it looks for the same patch subject at the ML Inbox.
>>>>>>> So, I'll be now be more grumpy with that ;) [1]
>>>>>>>
>>>>>>> So, please be sure to post those patches at the ML, with Antti's
>>>>>>> tested-by:
>>>>>>> tag, before sending a pull request.
>>>>>>>
>>>>>>> Thanks!
>>>>>>> Mauro
>>>>>>>
>>>>>>> [1] Side note: it is not actually a matter of being grumpy; posted
>>>>>>> patches
>>>>>>> receive a lot more attention/review than simple pull requests. From
>>>>>>> time
>>>>>>> to time, patches that went via the wrong way (e. g. without a previous
>>>>>>> post)
>>>>>>> caused troubles for other developers. So, enforcing it is actually a
>>>>>>> matter
>>>>>>> of improving Kernel quality and avoiding regressions.
>>>>>>>
>>>>>>> -
>>>>>>>
>>>>>>> $ test_patch
>>>>>>> testing if
>>>>>>>
>>>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>>>> applies
>>>>>>> patch -p1 -i
>>>>>>>
>>>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>>>> --dry-run -t -N
>>>>>>> patching file drivers/media/tuners/tda18271-fe.c
>>>>>>>      drivers/media/tuners/tda18271-fe.c |    3 +++
>>>>>>>      1 file changed, 3 insertions(+)
>>>>>>> Subject: tda18271: enter low-power standby mode at the end of
>>>>>>> tda18271_attach()
>>>>>>> From: Michael Krufky <mkrufky@linuxtv.org>
>>>>>>> Date: Thu, 26 Jul 2012 08:34:37 -0400
>>>>>>> Patch applies OK
>>>>>>> total: 0 errors, 0 warnings, 9 lines checked
>>>>>>>
>>>>>>>
>>>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>>>> has no obvious style problems and is ready for submission.
>>>>>>> Didn't find any message with subject equal to 'tda18271: enter
>>>>>>> low-power
>>>>>>> standby mode at the end of tda18271_attach()'
>>>>>>> Duplicated md5sum patches
>>>>>>> Likely duplicated patches (need manual check)
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> If that tda18271 patch is not applied then these two should be:
>>>>>>
>>>>>> https://patchwork.kernel.org/patch/1481901/
>>>>>> https://patchwork.kernel.org/patch/1481911/
>>>>>>
>>>>>>
>>>>>> regards
>>>>>> Antti
>>>>>>
>>>>>> --
>>>>>> http://palosaari.fi/
>>>>>
>>>>>
>>>>>
>>>>> The tda18271 patch should indeed be applied -- I will send it to the
>>>>> ML later on today and follow up with a pull request.  Thanks to all
>>>>> who have commented :-)
>>>>
>>>>
>>>>
>>>> Mike, There is other problem too. PCTV 520e, which is Em28xx + DRX-K +
>>>> TDA18271, fails to attach tuner now. Tuner is wired behind DRX-K I2C bus.
>>>> TDA18271 driver does very much I/O during attach and I2C error is raised
>>>> during attach now. Earlier it worked as DRX-K firmware was downloaded
>>>> before
>>>> tuner was attached, but now both DRX-K fw download and tuner attach
>>>> happens
>>>> same time leading that error.
>>>
>>>
>>> Why is the DRX-K firmware downloading at the same time as tuner
>>> attach?  Shouldn't the demod attach be finished before the tuner
>>> attach begins?
>>
>>
>> What I think all these should go in order bridge => demod => tuner. Attach
>> and fw loading. I cannot see how it will never work generally unless those
>> firmwares are loaded in that order.
>> 1) bridge needs firmware up and running before demod could be attached. That
>> is mostly because I2C adapter is behind bridge.
>>
>> 2) demod needs firmware up and running before tuner could be attached. That
>> is mostly because I2C adapter/bus for tuner is behind the demod.
>>
>> 3) tuner needs firmware up and running as there could be firmware controlled
>> GPIO bus behind tuner. There is many times LNA, LNB controller, LED, antenna
>> switch. I am not surprised if there is even I2C bus behind the tuner to
>> control LNB or other equipment near antenna connector and tuner.
>>
>> Of course situation is not that bad usually - but surely there could be some
>> existing device which is very near that. But as we *want* to do things as
>> general as possible to avoid driver / device specific hacks that is the only
>> reasonable model.
>>
>
>
> I'm not sure how that relates to the problem you brought up.....
> back to the issue:
>
> I don't have the PCTV 520e schematics handy, but...  it's possible
> that the DRX-K depends on XTOUT from the tda18271 --
>
> In your struct tda18271_config, are you using the .output_opt
> configuration?  for example:
>
> struct tda18271_config hauppauge_tda18271_config = {
>          .std_map = &hauppauge_tda18271_std_map,
>          .gate    = TDA18271_GATE_ANALOG,
>          .output_opt = TDA18271_OUTPUT_LT_OFF,
> };
>
>
> If so, try deleting the .output_opt line - see if that helps.

I suspect it does not have nothing to do with tuner outputs. If I add 2 
second sleep after demod attach and before tuner attach - it works. Also 
if I use DRX-K internal firmware (not download newer) it works.

Here is the debug (drx-k & tda18271):
http://palosaari.fi/linux/v4l-dvb/em28xx_drxk_tda18271.txt

regards
Antti
  

On Thu, Sep 27, 2012 at 6:46 PM, Antti Palosaari <crope@iki.fi> wrote:
> On 09/28/2012 01:43 AM, Michael Krufky wrote:
>>
>> On Thu, Sep 27, 2012 at 6:26 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>
>>> On 09/28/2012 12:58 AM, Michael Krufky wrote:
>>>>
>>>>
>>>> On Thu, Sep 27, 2012 at 5:38 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>>
>>>>>
>>>>> On 09/28/2012 12:20 AM, Michael Krufky wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Thu, Sep 27, 2012 at 3:59 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 09/27/2012 10:19 PM, Mauro Carvalho Chehab wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Em Thu, 26 Jul 2012 08:48:58 -0400
>>>>>>>> Michael Krufky <mkrufky@linuxtv.org> escreveu:
>>>>>>>>
>>>>>>>>> Antti,
>>>>>>>>>
>>>>>>>>> This small patch should do the trick -- can you test it?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The following changes since commit
>>>>>>>>> 0c7d5a6da75caecc677be1fda207b7578936770d:
>>>>>>>>>
>>>>>>>>>       Linux 3.5-rc5 (2012-07-03 22:57:41 +0300)
>>>>>>>>>
>>>>>>>>> are available in the git repository at:
>>>>>>>>>
>>>>>>>>>       git://git.linuxtv.org/mkrufky/tuners tda18271
>>>>>>>>>
>>>>>>>>> for you to fetch changes up to
>>>>>>>>> 782b28e20d3b253d317cc71879639bf3c108b200:
>>>>>>>>>
>>>>>>>>>       tda18271: enter low-power standby mode at the end of
>>>>>>>>> tda18271_attach() (2012-07-26 08:34:37 -0400)
>>>>>>>>>
>>>>>>>>> ----------------------------------------------------------------
>>>>>>>>> Michael Krufky (1):
>>>>>>>>>           tda18271: enter low-power standby mode at the end of
>>>>>>>>> tda18271_attach()
>>>>>>>>>
>>>>>>>>>      drivers/media/common/tuners/tda18271-fe.c |    3 +++
>>>>>>>>>      1 file changed, 3 insertions(+)
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Mike,
>>>>>>>>
>>>>>>>> Despite patchwork's way of handling, thinking that this is a pull
>>>>>>>> request,
>>>>>>>> I suspect that your intention here were simply offer some patches
>>>>>>>> for
>>>>>>>> Antti
>>>>>>>> to test.
>>>>>>>>
>>>>>>>> In any case, please always send the patches via email to the ML
>>>>>>>> before
>>>>>>>> sending a pull request. This was always a rule, but some developers
>>>>>>>> are
>>>>>>>> lazy with this duty, and, as I didn't use to have a tool to double
>>>>>>>> check,
>>>>>>>> bad things happen.
>>>>>>>>
>>>>>>>> I'm now finally able to check with a simple script if weather a
>>>>>>>> patch
>>>>>>>> went to the ML or not. My script checks both reply-to/references
>>>>>>>> email
>>>>>>>> tags and it looks for the same patch subject at the ML Inbox.
>>>>>>>> So, I'll be now be more grumpy with that ;) [1]
>>>>>>>>
>>>>>>>> So, please be sure to post those patches at the ML, with Antti's
>>>>>>>> tested-by:
>>>>>>>> tag, before sending a pull request.
>>>>>>>>
>>>>>>>> Thanks!
>>>>>>>> Mauro
>>>>>>>>
>>>>>>>> [1] Side note: it is not actually a matter of being grumpy; posted
>>>>>>>> patches
>>>>>>>> receive a lot more attention/review than simple pull requests. From
>>>>>>>> time
>>>>>>>> to time, patches that went via the wrong way (e. g. without a
>>>>>>>> previous
>>>>>>>> post)
>>>>>>>> caused troubles for other developers. So, enforcing it is actually a
>>>>>>>> matter
>>>>>>>> of improving Kernel quality and avoiding regressions.
>>>>>>>>
>>>>>>>> -
>>>>>>>>
>>>>>>>> $ test_patch
>>>>>>>> testing if
>>>>>>>>
>>>>>>>>
>>>>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>>>>> applies
>>>>>>>> patch -p1 -i
>>>>>>>>
>>>>>>>>
>>>>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>>>>> --dry-run -t -N
>>>>>>>> patching file drivers/media/tuners/tda18271-fe.c
>>>>>>>>      drivers/media/tuners/tda18271-fe.c |    3 +++
>>>>>>>>      1 file changed, 3 insertions(+)
>>>>>>>> Subject: tda18271: enter low-power standby mode at the end of
>>>>>>>> tda18271_attach()
>>>>>>>> From: Michael Krufky <mkrufky@linuxtv.org>
>>>>>>>> Date: Thu, 26 Jul 2012 08:34:37 -0400
>>>>>>>> Patch applies OK
>>>>>>>> total: 0 errors, 0 warnings, 9 lines checked
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>>>>> has no obvious style problems and is ready for submission.
>>>>>>>> Didn't find any message with subject equal to 'tda18271: enter
>>>>>>>> low-power
>>>>>>>> standby mode at the end of tda18271_attach()'
>>>>>>>> Duplicated md5sum patches
>>>>>>>> Likely duplicated patches (need manual check)
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> If that tda18271 patch is not applied then these two should be:
>>>>>>>
>>>>>>> https://patchwork.kernel.org/patch/1481901/
>>>>>>> https://patchwork.kernel.org/patch/1481911/
>>>>>>>
>>>>>>>
>>>>>>> regards
>>>>>>> Antti
>>>>>>>
>>>>>>> --
>>>>>>> http://palosaari.fi/
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> The tda18271 patch should indeed be applied -- I will send it to the
>>>>>> ML later on today and follow up with a pull request.  Thanks to all
>>>>>> who have commented :-)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Mike, There is other problem too. PCTV 520e, which is Em28xx + DRX-K +
>>>>> TDA18271, fails to attach tuner now. Tuner is wired behind DRX-K I2C
>>>>> bus.
>>>>> TDA18271 driver does very much I/O during attach and I2C error is
>>>>> raised
>>>>> during attach now. Earlier it worked as DRX-K firmware was downloaded
>>>>> before
>>>>> tuner was attached, but now both DRX-K fw download and tuner attach
>>>>> happens
>>>>> same time leading that error.
>>>>
>>>>
>>>>
>>>> Why is the DRX-K firmware downloading at the same time as tuner
>>>> attach?  Shouldn't the demod attach be finished before the tuner
>>>> attach begins?
>>>
>>>
>>>
>>> What I think all these should go in order bridge => demod => tuner.
>>> Attach
>>> and fw loading. I cannot see how it will never work generally unless
>>> those
>>> firmwares are loaded in that order.
>>> 1) bridge needs firmware up and running before demod could be attached.
>>> That
>>> is mostly because I2C adapter is behind bridge.
>>>
>>> 2) demod needs firmware up and running before tuner could be attached.
>>> That
>>> is mostly because I2C adapter/bus for tuner is behind the demod.
>>>
>>> 3) tuner needs firmware up and running as there could be firmware
>>> controlled
>>> GPIO bus behind tuner. There is many times LNA, LNB controller, LED,
>>> antenna
>>> switch. I am not surprised if there is even I2C bus behind the tuner to
>>> control LNB or other equipment near antenna connector and tuner.
>>>
>>> Of course situation is not that bad usually - but surely there could be
>>> some
>>> existing device which is very near that. But as we *want* to do things as
>>> general as possible to avoid driver / device specific hacks that is the
>>> only
>>> reasonable model.
>>>
>>
>>
>> I'm not sure how that relates to the problem you brought up.....
>> back to the issue:
>>
>> I don't have the PCTV 520e schematics handy, but...  it's possible
>> that the DRX-K depends on XTOUT from the tda18271 --
>>
>> In your struct tda18271_config, are you using the .output_opt
>> configuration?  for example:
>>
>> struct tda18271_config hauppauge_tda18271_config = {
>>          .std_map = &hauppauge_tda18271_std_map,
>>          .gate    = TDA18271_GATE_ANALOG,
>>          .output_opt = TDA18271_OUTPUT_LT_OFF,
>> };
>>
>>
>> If so, try deleting the .output_opt line - see if that helps.
>
>
> I suspect it does not have nothing to do with tuner outputs. If I add 2
> second sleep after demod attach and before tuner attach - it works. Also if
> I use DRX-K internal firmware (not download newer) it works.
>
> Here is the debug (drx-k & tda18271):
> http://palosaari.fi/linux/v4l-dvb/em28xx_drxk_tda18271.txt

Antti,

This is not about tuner *output* -- The tda18271 has a crystal output
feature to drive another demod.  If the demod needs this crystal
output, then it will lockup if the tuner disables the xtout.

Please try my suggestion -- it seems to me that it's quite likely that
the DRX-K could be using the XTOUT from the tda18271.  That's why the
driver leaves XTOUT on by default.

Try rebuilding with the TDA18271_OUTPUT_LT_OFF, removed.

-Mike
--
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 09/28/2012 01:55 AM, Michael Krufky wrote:
> On Thu, Sep 27, 2012 at 6:46 PM, Antti Palosaari <crope@iki.fi> wrote:
>> On 09/28/2012 01:43 AM, Michael Krufky wrote:
>>>
>>> On Thu, Sep 27, 2012 at 6:26 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>
>>>> On 09/28/2012 12:58 AM, Michael Krufky wrote:
>>>>>
>>>>>
>>>>> On Thu, Sep 27, 2012 at 5:38 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>>>
>>>>>>
>>>>>> On 09/28/2012 12:20 AM, Michael Krufky wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Sep 27, 2012 at 3:59 PM, Antti Palosaari <crope@iki.fi> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On 09/27/2012 10:19 PM, Mauro Carvalho Chehab wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Em Thu, 26 Jul 2012 08:48:58 -0400
>>>>>>>>> Michael Krufky <mkrufky@linuxtv.org> escreveu:
>>>>>>>>>
>>>>>>>>>> Antti,
>>>>>>>>>>
>>>>>>>>>> This small patch should do the trick -- can you test it?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> The following changes since commit
>>>>>>>>>> 0c7d5a6da75caecc677be1fda207b7578936770d:
>>>>>>>>>>
>>>>>>>>>>        Linux 3.5-rc5 (2012-07-03 22:57:41 +0300)
>>>>>>>>>>
>>>>>>>>>> are available in the git repository at:
>>>>>>>>>>
>>>>>>>>>>        git://git.linuxtv.org/mkrufky/tuners tda18271
>>>>>>>>>>
>>>>>>>>>> for you to fetch changes up to
>>>>>>>>>> 782b28e20d3b253d317cc71879639bf3c108b200:
>>>>>>>>>>
>>>>>>>>>>        tda18271: enter low-power standby mode at the end of
>>>>>>>>>> tda18271_attach() (2012-07-26 08:34:37 -0400)
>>>>>>>>>>
>>>>>>>>>> ----------------------------------------------------------------
>>>>>>>>>> Michael Krufky (1):
>>>>>>>>>>            tda18271: enter low-power standby mode at the end of
>>>>>>>>>> tda18271_attach()
>>>>>>>>>>
>>>>>>>>>>       drivers/media/common/tuners/tda18271-fe.c |    3 +++
>>>>>>>>>>       1 file changed, 3 insertions(+)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Mike,
>>>>>>>>>
>>>>>>>>> Despite patchwork's way of handling, thinking that this is a pull
>>>>>>>>> request,
>>>>>>>>> I suspect that your intention here were simply offer some patches
>>>>>>>>> for
>>>>>>>>> Antti
>>>>>>>>> to test.
>>>>>>>>>
>>>>>>>>> In any case, please always send the patches via email to the ML
>>>>>>>>> before
>>>>>>>>> sending a pull request. This was always a rule, but some developers
>>>>>>>>> are
>>>>>>>>> lazy with this duty, and, as I didn't use to have a tool to double
>>>>>>>>> check,
>>>>>>>>> bad things happen.
>>>>>>>>>
>>>>>>>>> I'm now finally able to check with a simple script if weather a
>>>>>>>>> patch
>>>>>>>>> went to the ML or not. My script checks both reply-to/references
>>>>>>>>> email
>>>>>>>>> tags and it looks for the same patch subject at the ML Inbox.
>>>>>>>>> So, I'll be now be more grumpy with that ;) [1]
>>>>>>>>>
>>>>>>>>> So, please be sure to post those patches at the ML, with Antti's
>>>>>>>>> tested-by:
>>>>>>>>> tag, before sending a pull request.
>>>>>>>>>
>>>>>>>>> Thanks!
>>>>>>>>> Mauro
>>>>>>>>>
>>>>>>>>> [1] Side note: it is not actually a matter of being grumpy; posted
>>>>>>>>> patches
>>>>>>>>> receive a lot more attention/review than simple pull requests. From
>>>>>>>>> time
>>>>>>>>> to time, patches that went via the wrong way (e. g. without a
>>>>>>>>> previous
>>>>>>>>> post)
>>>>>>>>> caused troubles for other developers. So, enforcing it is actually a
>>>>>>>>> matter
>>>>>>>>> of improving Kernel quality and avoiding regressions.
>>>>>>>>>
>>>>>>>>> -
>>>>>>>>>
>>>>>>>>> $ test_patch
>>>>>>>>> testing if
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>>>>>> applies
>>>>>>>>> patch -p1 -i
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>>>>>> --dry-run -t -N
>>>>>>>>> patching file drivers/media/tuners/tda18271-fe.c
>>>>>>>>>       drivers/media/tuners/tda18271-fe.c |    3 +++
>>>>>>>>>       1 file changed, 3 insertions(+)
>>>>>>>>> Subject: tda18271: enter low-power standby mode at the end of
>>>>>>>>> tda18271_attach()
>>>>>>>>> From: Michael Krufky <mkrufky@linuxtv.org>
>>>>>>>>> Date: Thu, 26 Jul 2012 08:34:37 -0400
>>>>>>>>> Patch applies OK
>>>>>>>>> total: 0 errors, 0 warnings, 9 lines checked
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> patches/0001-tda18271-enter-low-power-standby-mode-at-the-end-of-.patch
>>>>>>>>> has no obvious style problems and is ready for submission.
>>>>>>>>> Didn't find any message with subject equal to 'tda18271: enter
>>>>>>>>> low-power
>>>>>>>>> standby mode at the end of tda18271_attach()'
>>>>>>>>> Duplicated md5sum patches
>>>>>>>>> Likely duplicated patches (need manual check)
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> If that tda18271 patch is not applied then these two should be:
>>>>>>>>
>>>>>>>> https://patchwork.kernel.org/patch/1481901/
>>>>>>>> https://patchwork.kernel.org/patch/1481911/
>>>>>>>>
>>>>>>>>
>>>>>>>> regards
>>>>>>>> Antti
>>>>>>>>
>>>>>>>> --
>>>>>>>> http://palosaari.fi/
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> The tda18271 patch should indeed be applied -- I will send it to the
>>>>>>> ML later on today and follow up with a pull request.  Thanks to all
>>>>>>> who have commented :-)
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Mike, There is other problem too. PCTV 520e, which is Em28xx + DRX-K +
>>>>>> TDA18271, fails to attach tuner now. Tuner is wired behind DRX-K I2C
>>>>>> bus.
>>>>>> TDA18271 driver does very much I/O during attach and I2C error is
>>>>>> raised
>>>>>> during attach now. Earlier it worked as DRX-K firmware was downloaded
>>>>>> before
>>>>>> tuner was attached, but now both DRX-K fw download and tuner attach
>>>>>> happens
>>>>>> same time leading that error.
>>>>>
>>>>>
>>>>>
>>>>> Why is the DRX-K firmware downloading at the same time as tuner
>>>>> attach?  Shouldn't the demod attach be finished before the tuner
>>>>> attach begins?
>>>>
>>>>
>>>>
>>>> What I think all these should go in order bridge => demod => tuner.
>>>> Attach
>>>> and fw loading. I cannot see how it will never work generally unless
>>>> those
>>>> firmwares are loaded in that order.
>>>> 1) bridge needs firmware up and running before demod could be attached.
>>>> That
>>>> is mostly because I2C adapter is behind bridge.
>>>>
>>>> 2) demod needs firmware up and running before tuner could be attached.
>>>> That
>>>> is mostly because I2C adapter/bus for tuner is behind the demod.
>>>>
>>>> 3) tuner needs firmware up and running as there could be firmware
>>>> controlled
>>>> GPIO bus behind tuner. There is many times LNA, LNB controller, LED,
>>>> antenna
>>>> switch. I am not surprised if there is even I2C bus behind the tuner to
>>>> control LNB or other equipment near antenna connector and tuner.
>>>>
>>>> Of course situation is not that bad usually - but surely there could be
>>>> some
>>>> existing device which is very near that. But as we *want* to do things as
>>>> general as possible to avoid driver / device specific hacks that is the
>>>> only
>>>> reasonable model.
>>>>
>>>
>>>
>>> I'm not sure how that relates to the problem you brought up.....
>>> back to the issue:
>>>
>>> I don't have the PCTV 520e schematics handy, but...  it's possible
>>> that the DRX-K depends on XTOUT from the tda18271 --
>>>
>>> In your struct tda18271_config, are you using the .output_opt
>>> configuration?  for example:
>>>
>>> struct tda18271_config hauppauge_tda18271_config = {
>>>           .std_map = &hauppauge_tda18271_std_map,
>>>           .gate    = TDA18271_GATE_ANALOG,
>>>           .output_opt = TDA18271_OUTPUT_LT_OFF,
>>> };
>>>
>>>
>>> If so, try deleting the .output_opt line - see if that helps.
>>
>>
>> I suspect it does not have nothing to do with tuner outputs. If I add 2
>> second sleep after demod attach and before tuner attach - it works. Also if
>> I use DRX-K internal firmware (not download newer) it works.
>>
>> Here is the debug (drx-k & tda18271):
>> http://palosaari.fi/linux/v4l-dvb/em28xx_drxk_tda18271.txt
>
> Antti,
>
> This is not about tuner *output* -- The tda18271 has a crystal output
> feature to drive another demod.  If the demod needs this crystal
> output, then it will lockup if the tuner disables the xtout.
>
> Please try my suggestion -- it seems to me that it's quite likely that
> the DRX-K could be using the XTOUT from the tda18271.  That's why the
> driver leaves XTOUT on by default.
>
> Try rebuilding with the TDA18271_OUTPUT_LT_OFF, removed.

Tested (all possible) values: 0/1/2. No effect.

regards
Antti
  

Em Thu, 27 Sep 2012 17:58:24 -0400
Michael Krufky <mkrufky@linuxtv.org> escreveu:

> On Thu, Sep 27, 2012 at 5:38 PM, Antti Palosaari <crope@iki.fi> wrote:
> > On 09/28/2012 12:20 AM, Michael Krufky wrote:

> > Mike, There is other problem too. PCTV 520e, which is Em28xx + DRX-K +
> > TDA18271, fails to attach tuner now. Tuner is wired behind DRX-K I2C bus.
> > TDA18271 driver does very much I/O during attach and I2C error is raised
> > during attach now. Earlier it worked as DRX-K firmware was downloaded before
> > tuner was attached, but now both DRX-K fw download and tuner attach happens
> > same time leading that error.
> 
> Why is the DRX-K firmware downloading at the same time as tuner
> attach?  Shouldn't the demod attach be finished before the tuner
> attach begins?

Michael,

What happens is that udev changes forced drivers to load firmware asynchronously,
as, otherwise, udev won't load any firmware at all. Also, there's no warranty that
the firmware will be loaded on 2 seconds or so (Anti's hack were to add a 2 seconds
wait after drxk atttach, to wait for firmware load).

What I suspect is that tda18271 init is being interruped in the middle, by the
drxk firmware load. If this is the case, the solution is clean and quick: just
use the new i2c_lock_adapter() way to lock the I2C bus to tda18271 during the
critical part of the code where the register init happens.
  

On 09/28/2012 02:43 PM, Mauro Carvalho Chehab wrote:
> Em Thu, 27 Sep 2012 17:58:24 -0400
> Michael Krufky <mkrufky@linuxtv.org> escreveu:
>
>> On Thu, Sep 27, 2012 at 5:38 PM, Antti Palosaari <crope@iki.fi> wrote:
>>> On 09/28/2012 12:20 AM, Michael Krufky wrote:
>
>>> Mike, There is other problem too. PCTV 520e, which is Em28xx + DRX-K +
>>> TDA18271, fails to attach tuner now. Tuner is wired behind DRX-K I2C bus.
>>> TDA18271 driver does very much I/O during attach and I2C error is raised
>>> during attach now. Earlier it worked as DRX-K firmware was downloaded before
>>> tuner was attached, but now both DRX-K fw download and tuner attach happens
>>> same time leading that error.
>>
>> Why is the DRX-K firmware downloading at the same time as tuner
>> attach?  Shouldn't the demod attach be finished before the tuner
>> attach begins?
>
> Michael,
>
> What happens is that udev changes forced drivers to load firmware asynchronously,
> as, otherwise, udev won't load any firmware at all. Also, there's no warranty that
> the firmware will be loaded on 2 seconds or so (Anti's hack were to add a 2 seconds
> wait after drxk atttach, to wait for firmware load).

IMHO whole current DRX-K FW is broken by design.
1) if fw is not really needed it should not be loaded on attach() 
instead first use case at .init()
2) if fw is needed then it must be loaded and wait chip is up and 
running and after that return from attach()

When you done it asynchronously like that, there is big you hit more 
problems depending on hardware configuration etc.

I explained that earlier too. But lets take more "real world" example.

There is USB DVB-S device. USB-bridge needs first firmware in order to 
offer I2C adapter. We need USB-bridge I2C-adapter to probe demodulator 
which sits on bridge I2C-bus. After demodulator is found we need to 
download firmware fir demodulator in order to find out tuner. Tuner sits 
behind demod I2C-bus. OK, download fw and start demod => get access for 
demod I2C-bus. Then probe used tuner. Download FW for tuner in order to 
get access for tuner GPIOs which are in that case controlled by tuner 
firmware. Finally there is LNB voltage controller which is controlled by 
tuner GPIO. We ask tuner firmware to disable LNB voltage supply.

Quite worst possible scenario, but highly possible. And it cannot be 
performed until firmware are loaded for each chip one by one.

The only place this kind "asynchronous" hack is allowed is bridge driver 
- leaving the rest as those are. And my real opinion is that this kind 
of functionality does not belong to drivers at all but somewhere more 
lower levels like USB/PCI core routines.

> What I suspect is that tda18271 init is being interruped in the middle, by the
> drxk firmware load. If this is the case, the solution is clean and quick: just
> use the new i2c_lock_adapter() way to lock the I2C bus to tda18271 during the
> critical part of the code where the register init happens.
>
>

regards
Antti