LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Stefan Richter <stefanr@s5r6.in-berlin.de>
To: Philipp Beyer <philipp.beyer@alliedvisiontec.com>
Cc: linux-kernel@vger.kernel.org, linux1394-devel@lists.sourceforge.net
Subject: Re: ieee1394 feature needed: overwrite SPLIT_TIMEOUT from userspace
Date: Sat, 13 Jan 2007 19:15:36 +0100 (CET)	[thread overview]
Message-ID: <tkrat.0ae1f576575bc02e@s5r6.in-berlin.de> (raw)
In-Reply-To: <1168602157.5074.4.camel@ahr-pbe-lx.avtnet.local>

(full quote for linux1394-devel)

On 12 Jan, Philipp Beyer wrote to linux-kernel:
> Hi,
> 
> I'm investigating an unwanted behaviour of our firewire devices in 
> connection with the ieee1394 kernel module.
> 
> The problem is caused by a non standard-conform behaviour of our
> devices. Anyway, changes on the device-side dont seem to be the
> best solution, so I'm looking for a workaround in terms of a
> kernel patch.
> 
> The problem:
> Our devices exceed the SPLIT_TIMEOUT for write requests in some
> situations, where write accesses to the devices flash memory are 
> triggered.

There are certainly a number of ways to implement this in your
device in a conforming way. For example, if it is too costly to
avoid the transaction timeout, you could add a register to your
device to be polled by the requester after it initiated a lengthy
operation. The extra register would become responsive when the
operation finished and could even show whether the operation
succeeded.

But then, why not support lengthier timeouts in Linux if it can be
done with minimum overhead.

> The SPLIT_TIMEOUT could be adjusted as it's part of the 
> CSR layout, but the longest interval possible is 8 seconds. We need
> a substantial longer interval to assure failure-free operation.
> (the maximum timeout needed may be around 120 seconds)
> 
> The presumed solution:
> These long timeouts are only needed in a few rare situations like
> writing user presets to flash or firmware updates. As far as I've
> examined the kernel code it would be the best thing to have a
> function (ioctl?) accessible from userspace that overwrites the
> stored SPLIT_TIMEOUT for a certain connected device. This way
> there should not be any interferences in case of normal operation.
> Until (rare) write accesses to the flash memory are performed, a
> reasonable short timeout could be used.

I have an alternative suggestion:

 - Keep a global timeout for split transactions for all nodes.
   Tracking different timeouts per node would add significant code
   footprint.
 - Control the timeout like before via a write request to the
   SPLIT_TIMEOUT CSR.
 - Allow the local node to write a nonstandard value of >7 to
   SPLIT_TIMEOUT_HI. This would not be compliant with IEEE 1394(a)
   but at least with IEEE 1212.

This suggestion may fall short if a bus manager is present. Also,
I have concerns to add such a non-conforming feature to mainline.
(Not that our drivers were fully compliant to the spec now or that
100% by-the-book behavior would be desirable in the first place...)

> Since I don't have any real experience in kernel hacking yet,
> this should be interpreted as a feature request at first:
> If the described feature is easy to implement I would appreciate
> if someone could do this.

I could post a patch which works as I outlined if it fits your
requirements.

> Otherwise I'm confident that I'm able to write a patch on my own.
> In this case the critical part would be to meet the standards
> of the kernel community, since we would like to have the patch
> included in the mainline.
> 
> Therefore I'm also interested in any kind of advices about how
> to realize an appropriate patch.
> 
> Thanks,
> 
> Philipp Beyer
> 
> Software Development
> Allied Vision Technologies

See Documentation/SubmittingPatches in the Linux kernel source
distribution for advice on code submission.
-- 
Stefan Richter
-=====-=-=== ---= -==-=
http://arcgraph.de/sr/


  reply	other threads:[~2007-01-13 18:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-01-12 11:42 Philipp Beyer
2007-01-13 18:15 ` Stefan Richter [this message]
2007-01-15 13:21   ` Philipp Beyer
2007-01-15 14:06     ` Stefan Richter
2007-01-15 14:54       ` Philipp Beyer
2007-01-15 18:30         ` Stefan Richter
2007-01-15 18:54     ` Kristian Høgsberg

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=tkrat.0ae1f576575bc02e@s5r6.in-berlin.de \
    --to=stefanr@s5r6.in-berlin.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux1394-devel@lists.sourceforge.net \
    --cc=philipp.beyer@alliedvisiontec.com \
    --subject='Re: ieee1394 feature needed: overwrite SPLIT_TIMEOUT from userspace' \
    /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

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).