All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rémi Cardona" <remi.cardona@smartjog.com>
To: Antti Palosaari <crope@iki.fi>
Cc: linux-media@vger.kernel.org
Subject: Re: [PATCH 2/2] [media] ds3000: properly report firmware loading issues
Date: Thu, 30 Aug 2012 17:21:25 +0200	[thread overview]
Message-ID: <503F84F5.9010304@smartjog.com> (raw)
In-Reply-To: <503F6D18.2060804@iki.fi>

Hi Antti,

On 08/30/2012 03:39 PM, Antti Palosaari wrote:
> As I understand firmware downloading failure is coming from the fact
> that register read fails => fails to detect if firmware is already
> running or not.

Well we actually see 2 cases:

 - the register read failure (when ds3000_readreg() returns negative
values). This case is fairly rare, and no changes we've done to the
driver allowed us to make those cards work.

 - the register read returning 0. Looking at the current code, it looks
like the 0xb2 register is supposed to mean that a firmware is loaded.
This case is fairly common: we've had many cards randomly saying that a
firmware was loaded when none had been. Often, a simple reboot will do
the trick. But sometimes, forcing the firmware upload (ie, bypassing the
0xb2 register check) allows the stubborn cards to function properly.

> Original behavior to expect firmware is loaded and running when register
> read fails is very stupid and your fix seems much better.

Well, this patch should not really change the behavior. It just
propagates register read errors to ds3000_initfe(). It'll just fail earlier.

> So first priority should be try fix that issue with register read. Is it
> coming from the USB stack (eg. error 110 timeout) or some other error
> coming from the fact chip answers wrong?

The cards we're using are PCIe (and not the ones with an embedded USB
controller).

> Do you see other register I/O failing too?

I'll see if I can get you an answer for that, since the cards are
shipped with the appliance we send to our customers. Remote debugging is
somewhat tricky.

> Does adding few usec sleep help?

I'm not quite sure where to add those sleeps. In the register
reading/writing functions? 10us? 100us?

Many thanks

Rémi

  reply	other threads:[~2012-08-30 15:21 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-30  9:36 [PATCH RFC 0/2] ds3000 firmware loading improvements Rémi Cardona
2012-08-30  9:36 ` [PATCH 1/2] [media] ds3000: Remove useless 'locking' Rémi Cardona
2012-09-03 14:13   ` Rémi Cardona
2012-08-30  9:36 ` [PATCH 2/2] [media] ds3000: properly report firmware loading issues Rémi Cardona
2012-08-30 13:39   ` Antti Palosaari
2012-08-30 15:21     ` Rémi Cardona [this message]
2012-08-30 16:00       ` Antti Palosaari
2012-09-03 13:27         ` Rémi Cardona
2012-08-31  8:29       ` Re: [PATCH 2/2] [media] ds3000: properly report firmware loadingissues nibble.max
2012-09-03 14:11         ` Rémi Cardona
2012-09-04  2:19         ` nibble.max

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=503F84F5.9010304@smartjog.com \
    --to=remi.cardona@smartjog.com \
    --cc=crope@iki.fi \
    --cc=linux-media@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.