[PATCH v2 0/5] staging/fwserial: Address reviewer comments

[PATCH v2 0/5] staging/fwserial: Address reviewer comments

[Peter Hurley]

Overdue respin.
v2 changes:
   Don't use 'card' when referring to firewire node
   Removed lower clamp in link_speed_to_max_payload()
   Ripped out the bandwidth limit logic
   Drop suggestion to integrate link_speed_to_max_payload()
     & device_max_receive()
   Note required minimum max_rec value for OHCI-compliant devices

Peter Hurley (6):
  staging/fwserial: Refine Kconfig help text
  staging/fwserial: Remove bandwidth limit logic
  staging/fwserial: Limit tx/rx to 1394-2008 spec maximum
  staging/fwserial: Update TODO file per reviewer comments
  staging/fwserial: Assume firmware is OHCI-complaint
  staging/fwserial: Drop suggestion for helper fn integration

 drivers/staging/fwserial/Kconfig    |  4 +++-
 drivers/staging/fwserial/TODO       | 16 +++-------------
 drivers/staging/fwserial/fwserial.c | 15 ++++++++-------
 drivers/staging/fwserial/fwserial.h | 17 +++++++----------
 4 files changed, 21 insertions(+), 31 deletions(-)

-- 
1.8.0.1

[PATCH v2 1/6] staging/fwserial: Refine Kconfig help text

[Peter Hurley]

Users should be informed upfront that this is a Linux-only affair
currently.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/Kconfig | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/fwserial/Kconfig b/drivers/staging/fwserial/Kconfig
index 580406c..b2f8331 100644
--- a/drivers/staging/fwserial/Kconfig
+++ b/drivers/staging/fwserial/Kconfig
@@ -3,7 +3,9 @@ config FIREWIRE_SERIAL
        depends on FIREWIRE
        help
           This enables TTY over IEEE 1394, providing high-speed serial
-	  connectivity to cabled peers.
+	  connectivity to cabled peers. This driver implements a
+	  ad-hoc transport protocol and is currently limited to
+	  Linux-to-Linux communication.
 
 	  To compile this driver as a module, say M here:  the module will
 	  be called firewire-serial.
-- 
1.8.0.1

[PATCH v2 2/6] staging/fwserial: Remove bandwidth limit logic

[Peter Hurley]

Self-limiting asynchronous bandwidth (via reducing the payload)
is not necessary and does not work, because
 1) asynchronous traffic will absorb all available bandwidth (less that
    being used for isochronous traffic)
 2) isochronous arbitration always wins.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/fwserial.c |  3 ---
 drivers/staging/fwserial/fwserial.h | 15 ++++++---------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index 61ee290..be5db8a 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -40,12 +40,10 @@ static int num_ttys = 4;	    /* # of std ttys to create per fw_card    */
 				    /* - doubles as loopback port index       */
 static bool auto_connect = true;    /* try to VIRT_CABLE to every peer        */
 static bool create_loop_dev = true; /* create a loopback device for each card */
-bool limit_bw;			    /* limit async bandwidth to 20% of max    */
 
 module_param_named(ttys, num_ttys, int, S_IRUGO | S_IWUSR);
 module_param_named(auto, auto_connect, bool, S_IRUGO | S_IWUSR);
 module_param_named(loop, create_loop_dev, bool, S_IRUGO | S_IWUSR);
-module_param(limit_bw, bool, S_IRUGO | S_IWUSR);
 
 /*
  * Threshold below which the tty is woken for writing
@@ -2940,4 +2938,3 @@ MODULE_DEVICE_TABLE(ieee1394, fwserial_id_table);
 MODULE_PARM_DESC(ttys, "Number of ttys to create for each local firewire node");
 MODULE_PARM_DESC(auto, "Auto-connect a tty to each firewire node discovered");
 MODULE_PARM_DESC(loop, "Create a loopback device, fwloop<n>, with ttys");
-MODULE_PARM_DESC(limit_bw, "Limit bandwidth utilization to 20%.");
diff --git a/drivers/staging/fwserial/fwserial.h b/drivers/staging/fwserial/fwserial.h
index 8b572ed..cb0eea0 100644
--- a/drivers/staging/fwserial/fwserial.h
+++ b/drivers/staging/fwserial/fwserial.h
@@ -351,7 +351,6 @@ struct fw_serial {
 #define TTY_DEV_NAME		    "fwtty"	/* ttyFW was taken           */
 static const char tty_dev_name[] =  TTY_DEV_NAME;
 static const char loop_dev_name[] = "fwloop";
-extern bool limit_bw;
 
 struct tty_driver *fwtty_driver;
 
@@ -370,18 +369,16 @@ static inline void fwtty_bind_console(struct fwtty_port *port,
 
 /*
  * Returns the max send async payload size in bytes based on the unit device
- * link speed - if set to limit bandwidth to max 20%, use lookup table
+ * link speed. Self-limiting asynchronous bandwidth (via reducing the payload)
+ * is not necessary and does not work, because
+ *   1) asynchronous traffic will absorb all available bandwidth (less that
+ *	being used for isochronous traffic)
+ *   2) isochronous arbitration always wins.
  */
 static inline int link_speed_to_max_payload(unsigned speed)
 {
-	static const int max_async[] = { 307, 614, 1229, 2458, 4916, 9832, };
-	BUILD_BUG_ON(ARRAY_SIZE(max_async) - 1 != SCODE_3200);
-
 	speed = clamp(speed, (unsigned) SCODE_100, (unsigned) SCODE_3200);
-	if (limit_bw)
-		return max_async[speed];
-	else
-		return 1 << (speed + 9);
+	return 1 << (speed + 9);
 }
 
 #endif /* _FIREWIRE_FWSERIAL_H */
-- 
1.8.0.1

[PATCH v2 3/6] staging/fwserial: Limit tx/rx to 1394-2008 spec maximum

[Peter Hurley]

Per this conversation https://lkml.org/lkml/2012/11/27/587
limit the maximum transmission to the IEEE 1394-2008 specification
maximum size of 4096 bytes for asynchronous packets.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/TODO       | 3 ---
 drivers/staging/fwserial/fwserial.c | 8 ++++----
 drivers/staging/fwserial/fwserial.h | 4 ++--
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/fwserial/TODO b/drivers/staging/fwserial/TODO
index 7269005..ffe47d1 100644
--- a/drivers/staging/fwserial/TODO
+++ b/drivers/staging/fwserial/TODO
@@ -12,9 +12,6 @@ TODOs
 1. This driver uses the same unregistered vendor id that the firewire core does
      (0xd00d1e). Perhaps this could be exposed as a define in
      firewire-constants.h?
-2. MAX_ASYNC_PAYLOAD needs to be publicly exposed by core/ohci
-   - otherwise how will this driver know the max size of address window to
-     open for one packet write?
 3. Maybe device_max_receive() and link_speed_to_max_payload() should be
      taken up by the firewire core?
 4. To avoid dropping rx data while still limiting the maximum buffering,
diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index be5db8a..db1378d 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -174,10 +174,11 @@ static void dump_profile(struct seq_file *m, struct stats *stats)
 #define dump_profile(m, stats)
 #endif
 
-/* Returns the max receive packet size for the given card */
+/* Returns the max receive packet size for the given node */
 static inline int device_max_receive(struct fw_device *fw_device)
 {
-	return 1 <<  (clamp_t(int, fw_device->max_rec, 8U, 13U) + 1);
+	/* see IEEE 1394-2008 table 8-8 */
+	return 1 <<  (clamp_t(int, fw_device->max_rec, 8U, 11U) + 1);
 }
 
 static void fwtty_log_tx_error(struct fwtty_port *port, int rcode)
@@ -1683,8 +1684,7 @@ static void fwserial_virt_plug_complete(struct fwtty_peer *peer,
 
 	/* reconfigure tx_fifo optimally for this peer */
 	spin_lock_bh(&port->lock);
-	port->max_payload = min3(peer->max_payload, peer->fifo_len,
-				 MAX_ASYNC_PAYLOAD);
+	port->max_payload = min(peer->max_payload, peer->fifo_len);
 	dma_fifo_change_tx_limit(&port->tx_fifo, port->max_payload);
 	spin_unlock_bh(&peer->port->lock);
 
diff --git a/drivers/staging/fwserial/fwserial.h b/drivers/staging/fwserial/fwserial.h
index cb0eea0..953ece6 100644
--- a/drivers/staging/fwserial/fwserial.h
+++ b/drivers/staging/fwserial/fwserial.h
@@ -377,8 +377,8 @@ static inline void fwtty_bind_console(struct fwtty_port *port,
  */
 static inline int link_speed_to_max_payload(unsigned speed)
 {
-	speed = clamp(speed, (unsigned) SCODE_100, (unsigned) SCODE_3200);
-	return 1 << (speed + 9);
+	/* Max async payload is 4096 - see IEEE 1394-2008 tables 6-4, 16-18 */
+	return min(512 << speed, 4096);
 }
 
 #endif /* _FIREWIRE_FWSERIAL_H */
-- 
1.8.0.1

[PATCH v2 4/6] staging/fwserial: Update TODO file per reviewer comments

[Peter Hurley]

Pursuant to this review https://lkml.org/lkml/2012/11/12/500
by Stefan Richter, update the TODO file.
- Clarify purpose of TODO file
- Remove firewire item #4. As discussed in this conversation
  https://lkml.org/lkml/2012/11/13/564 knowing the AR buffer size
  is not a hard requirement. The required rx buffer size can be
  determined experimentally.
- Remove firewire item #5. This was a private note for further
  experimentation.
- Change firewire item #1. Change suggested header from uapi header
  to kernel-only header.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/TODO | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/fwserial/TODO b/drivers/staging/fwserial/TODO
index ffe47d1..8dae8fb 100644
--- a/drivers/staging/fwserial/TODO
+++ b/drivers/staging/fwserial/TODO
@@ -1,5 +1,5 @@
-TODOs
------
+TODOs prior to this driver moving out of staging
+------------------------------------------------
 1. Implement retries for RCODE_BUSY, RCODE_NO_ACK and RCODE_SEND_ERROR
    - I/O is handled asynchronously which presents some issues when error
      conditions occur.
@@ -11,14 +11,9 @@ TODOs
 -- Issues with firewire stack --
 1. This driver uses the same unregistered vendor id that the firewire core does
      (0xd00d1e). Perhaps this could be exposed as a define in
-     firewire-constants.h?
+     firewire.h?
 3. Maybe device_max_receive() and link_speed_to_max_payload() should be
      taken up by the firewire core?
-4. To avoid dropping rx data while still limiting the maximum buffering,
-     the size of the AR context must be known. How to expose this to drivers?
-5. Explore if bigger AR context will reduce RCODE_BUSY responses
-   (or auto-grow to certain max size -- but this would require major surgery
-    as the current AR is contiguously mapped)
 
 -- Issues with TTY core --
   1. Hack for alternate device name scheme
-- 
1.8.0.1

[PATCH v2 5/6] staging/fwserial: Assume firmware is OHCI-complaint

[Peter Hurley]

Devices which are OHCI v1.0/ v1.1/ v1.2-draft compliant or
RFC 2734 compliant are required by specification to support
max_rec of 8 (512 bytes) or more. Accept reported value.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/fwserial.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index db1378d..59e90e6 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -174,11 +174,15 @@ static void dump_profile(struct seq_file *m, struct stats *stats)
 #define dump_profile(m, stats)
 #endif
 
-/* Returns the max receive packet size for the given node */
+/*
+ * Returns the max receive packet size for the given node
+ * Devices which are OHCI v1.0/ v1.1/ v1.2-draft or RFC 2734 compliant
+ * are required by specification to support max_rec of 8 (512 bytes) or more.
+ */
 static inline int device_max_receive(struct fw_device *fw_device)
 {
 	/* see IEEE 1394-2008 table 8-8 */
-	return 1 <<  (clamp_t(int, fw_device->max_rec, 8U, 11U) + 1);
+	return min(2 << fw_device->max_rec, 4096);
 }
 
 static void fwtty_log_tx_error(struct fwtty_port *port, int rcode)
-- 
1.8.0.1

[PATCH v2 6/6] staging/fwserial: Drop suggestion for helper fn integration

[Peter Hurley]

The firewire core does not require or want the suggested helper fns;
drop suggestion from TODO file.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/TODO | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/fwserial/TODO b/drivers/staging/fwserial/TODO
index 8dae8fb..dc61d97 100644
--- a/drivers/staging/fwserial/TODO
+++ b/drivers/staging/fwserial/TODO
@@ -12,8 +12,6 @@ TODOs prior to this driver moving out of staging
 1. This driver uses the same unregistered vendor id that the firewire core does
      (0xd00d1e). Perhaps this could be exposed as a define in
      firewire.h?
-3. Maybe device_max_receive() and link_speed_to_max_payload() should be
-     taken up by the firewire core?
 
 -- Issues with TTY core --
   1. Hack for alternate device name scheme
-- 
1.8.0.1

Re: [PATCH v2 6/6] staging/fwserial: Drop suggestion for helper fn integration

[Stefan Richter]

On Dec 15 Peter Hurley wrote:
> The firewire core does not require or want the suggested helper fns;
> drop suggestion from TODO file.

It's not about the core, but about what the highlevel drivers need.
(I.e. protocol drivers and some higherlevel parts of the core, e.g.
the userspace driver interface.)

If it is something which two or more drivers use, it goes into the core.
(Or into a library, as Clemens did with snd-firewire-lib.)  If it is
something which needs assistance by the lowlevel driver, it goes into the
core or passes through the core, regardless whether one or more protocol
drivers need it.  (Example:  Physical DMA filtering for the SBP-2
initiator driver.)  If it is something very complicated and 1394
architecture specific, but still only needed by one highlevel driver, I
for one am more comfortable with leaving this in the respective driver
rather than moving this into the core.

The packet payload calculation is a 1394 architecture arcanum and is needed
in on form or another in more than one driver (firewire-net, -sbp2, and
now fwserial).  But as discussed, what they precisely need in the end
differs to a degree that leaves nothing substantial to share.

[That's too many words about a two- or three-line piece of code; but on the
on the other hand it is a generally relevant consideration whenever new
functionality is to be added somewhere in the driver stack.]
-- 
Stefan Richter
-=====-===-- ==-- -====
http://arcgraph.de/sr/

Re: [PATCH v2 6/6] staging/fwserial: Drop suggestion for helper fn integration

[Stefan Richter]

> On Dec 15 Peter Hurley wrote:
> > The firewire core does not require or want the suggested helper fns;
> > drop suggestion from TODO file.

PS,
the patch and the other five look good to me.
-- 
Stefan Richter
-=====-===-- ==-- -====
http://arcgraph.de/sr/

Re: [PATCH v2 0/5] staging/fwserial: Address reviewer comments

[Greg Kroah-Hartman]

On Sat, Dec 15, 2012 at 01:03:14AM -0500, Peter Hurley wrote:
> Overdue respin.
> v2 changes:
>    Don't use 'card' when referring to firewire node
>    Removed lower clamp in link_speed_to_max_payload()
>    Ripped out the bandwidth limit logic
>    Drop suggestion to integrate link_speed_to_max_payload()
>      & device_max_receive()
>    Note required minimum max_rec value for OHCI-compliant devices

Ugh, I applied the first version of this series, many apologies.  Can
you send me follow-on patches with just the differences here?

thanks,

greg k-h

Re: [PATCH v2 0/5] staging/fwserial: Address reviewer comments

[Peter Hurley]

On Mon, 2013-01-07 at 15:22 -0800, Greg Kroah-Hartman wrote:
> On Sat, Dec 15, 2012 at 01:03:14AM -0500, Peter Hurley wrote:
> > Overdue respin.
> > v2 changes:
> >    Don't use 'card' when referring to firewire node
> >    Removed lower clamp in link_speed_to_max_payload()
> >    Ripped out the bandwidth limit logic
> >    Drop suggestion to integrate link_speed_to_max_payload()
> >      & device_max_receive()
> >    Note required minimum max_rec value for OHCI-compliant devices
> 
> Ugh, I applied the first version of this series, many apologies.  Can
> you send me follow-on patches with just the differences here?

I saw that - no worries. I'll send follow-on patches today or tomorrow.

Regards,
Peter Hurley

[PATCH v3 0/6] staging/fwserial: Address reviewer comments

[Peter Hurley]

Hi Greg,

Here's the delta patches to get from v1 --> v2 of the patch series.
Patches 2,3 & 4 are the "sub-patches" from
  [PATCH v2 3/6] staging/fwserial: Limit tx/rx to 1394-2008 spec maximum

Peter Hurley (6):
  staging/fwserial: Remove bandwidth limit logic
  staging/fwserial: Refer to fw_device as "node"
  staging/fwserial: Simplify max payload calculation
  staging/fwserial: Fold constant MAX_ASYNC_PAYLOAD
  staging/fwserial: Assume firmware is OHCI-complaint
  staging/fwserial: Drop suggestion for helper fn integration

 drivers/staging/fwserial/TODO       |  2 --
 drivers/staging/fwserial/fwserial.c | 15 ++++++++-------
 drivers/staging/fwserial/fwserial.h | 17 +++++++----------
 3 files changed, 15 insertions(+), 19 deletions(-)

-- 
1.8.1.1

[PATCH v3 1/6] staging/fwserial: Remove bandwidth limit logic

[Peter Hurley]

Self-limiting asynchronous bandwidth (via reducing the payload)
is not necessary and does not work, because
 1) asynchronous traffic will absorb all available bandwidth (less that
    being used for isochronous traffic)
 2) isochronous arbitration always wins.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/fwserial.c |  3 ---
 drivers/staging/fwserial/fwserial.h | 15 ++++++---------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index e5e8f2f..233bb56 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -40,12 +40,10 @@ static int num_ttys = 4;	    /* # of std ttys to create per fw_card    */
 				    /* - doubles as loopback port index       */
 static bool auto_connect = true;    /* try to VIRT_CABLE to every peer        */
 static bool create_loop_dev = true; /* create a loopback device for each card */
-bool limit_bw;			    /* limit async bandwidth to 20% of max    */
 
 module_param_named(ttys, num_ttys, int, S_IRUGO | S_IWUSR);
 module_param_named(auto, auto_connect, bool, S_IRUGO | S_IWUSR);
 module_param_named(loop, create_loop_dev, bool, S_IRUGO | S_IWUSR);
-module_param(limit_bw, bool, S_IRUGO | S_IWUSR);
 
 /*
  * Threshold below which the tty is woken for writing
@@ -2931,4 +2929,3 @@ MODULE_DEVICE_TABLE(ieee1394, fwserial_id_table);
 MODULE_PARM_DESC(ttys, "Number of ttys to create for each local firewire node");
 MODULE_PARM_DESC(auto, "Auto-connect a tty to each firewire node discovered");
 MODULE_PARM_DESC(loop, "Create a loopback device, fwloop<n>, with ttys");
-MODULE_PARM_DESC(limit_bw, "Limit bandwidth utilization to 20%.");
diff --git a/drivers/staging/fwserial/fwserial.h b/drivers/staging/fwserial/fwserial.h
index caa1c1e..e157318 100644
--- a/drivers/staging/fwserial/fwserial.h
+++ b/drivers/staging/fwserial/fwserial.h
@@ -351,7 +351,6 @@ struct fw_serial {
 #define TTY_DEV_NAME		    "fwtty"	/* ttyFW was taken           */
 static const char tty_dev_name[] =  TTY_DEV_NAME;
 static const char loop_dev_name[] = "fwloop";
-extern bool limit_bw;
 
 struct tty_driver *fwtty_driver;
 
@@ -370,18 +369,16 @@ static inline void fwtty_bind_console(struct fwtty_port *port,
 
 /*
  * Returns the max send async payload size in bytes based on the unit device
- * link speed - if set to limit bandwidth to max 20%, use lookup table
+ * link speed. Self-limiting asynchronous bandwidth (via reducing the payload)
+ * is not necessary and does not work, because
+ *   1) asynchronous traffic will absorb all available bandwidth (less that
+ *	being used for isochronous traffic)
+ *   2) isochronous arbitration always wins.
  */
 static inline int link_speed_to_max_payload(unsigned speed)
 {
-	static const int max_async[] = { 307, 614, 1229, 2458, };
-	BUILD_BUG_ON(ARRAY_SIZE(max_async) - 1 != SCODE_800);
-
 	speed = clamp(speed, (unsigned) SCODE_100, (unsigned) SCODE_800);
-	if (limit_bw)
-		return max_async[speed];
-	else
-		return 1 << (speed + 9);
+	return 1 << (speed + 9);
 }
 
 #endif /* _FIREWIRE_FWSERIAL_H */
-- 
1.8.1.1

[PATCH v3 2/6] staging/fwserial: Refer to fw_device as "node"

[Peter Hurley]

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/fwserial.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index 233bb56..ae180a3 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -174,7 +174,7 @@ static void dump_profile(struct seq_file *m, struct stats *stats)
 #define dump_profile(m, stats)
 #endif
 
-/* Returns the max receive packet size for the given card */
+/* Returns the max receive packet size for the given node */
 static inline int device_max_receive(struct fw_device *fw_device)
 {
 	return 1 <<  (clamp_t(int, fw_device->max_rec, 8U, 11U) + 1);
-- 
1.8.1.1

[PATCH v3 3/6] staging/fwserial: Simplify max payload calculation

[Peter Hurley]

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/fwserial.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fwserial/fwserial.h b/drivers/staging/fwserial/fwserial.h
index e157318..953ece6 100644
--- a/drivers/staging/fwserial/fwserial.h
+++ b/drivers/staging/fwserial/fwserial.h
@@ -377,8 +377,8 @@ static inline void fwtty_bind_console(struct fwtty_port *port,
  */
 static inline int link_speed_to_max_payload(unsigned speed)
 {
-	speed = clamp(speed, (unsigned) SCODE_100, (unsigned) SCODE_800);
-	return 1 << (speed + 9);
+	/* Max async payload is 4096 - see IEEE 1394-2008 tables 6-4, 16-18 */
+	return min(512 << speed, 4096);
 }
 
 #endif /* _FIREWIRE_FWSERIAL_H */
-- 
1.8.1.1

[PATCH v3 4/6] staging/fwserial: Fold constant MAX_ASYNC_PAYLOAD

[Peter Hurley]

Since peer->max_payload is now limited to 1394-2008 spec maximum
of 4096, the port->max_payload limit can now be simplified.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/fwserial.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index ae180a3..940639f 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -1674,8 +1674,7 @@ static void fwserial_virt_plug_complete(struct fwtty_peer *peer,
 
 	/* reconfigure tx_fifo optimally for this peer */
 	spin_lock_bh(&port->lock);
-	port->max_payload = min3(peer->max_payload, peer->fifo_len,
-				 MAX_ASYNC_PAYLOAD);
+	port->max_payload = min(peer->max_payload, peer->fifo_len);
 	dma_fifo_change_tx_limit(&port->tx_fifo, port->max_payload);
 	spin_unlock_bh(&peer->port->lock);
 
-- 
1.8.1.1

[PATCH v3 5/6] staging/fwserial: Assume firmware is OHCI-complaint

[Peter Hurley]

Devices which are OHCI v1.0/ v1.1/ v1.2-draft compliant or
RFC 2734 compliant are required by specification to support
max_rec of 8 (512 bytes) or more. Accept reported value.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/fwserial.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index 940639f..48523eb 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -174,10 +174,15 @@ static void dump_profile(struct seq_file *m, struct stats *stats)
 #define dump_profile(m, stats)
 #endif
 
-/* Returns the max receive packet size for the given node */
+/*
+ * Returns the max receive packet size for the given node
+ * Devices which are OHCI v1.0/ v1.1/ v1.2-draft or RFC 2734 compliant
+ * are required by specification to support max_rec of 8 (512 bytes) or more.
+ */
 static inline int device_max_receive(struct fw_device *fw_device)
 {
-	return 1 <<  (clamp_t(int, fw_device->max_rec, 8U, 11U) + 1);
+	/* see IEEE 1394-2008 table 8-8 */
+	return min(2 << fw_device->max_rec, 4096);
 }
 
 static void fwtty_log_tx_error(struct fwtty_port *port, int rcode)
-- 
1.8.1.1

[PATCH v3 6/6] staging/fwserial: Drop suggestion for helper fn integration

[Peter Hurley]

The firewire core does not require or want the suggested helper fns;
drop suggestion from TODO file.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/TODO | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/fwserial/TODO b/drivers/staging/fwserial/TODO
index 8dae8fb..dc61d97 100644
--- a/drivers/staging/fwserial/TODO
+++ b/drivers/staging/fwserial/TODO
@@ -12,8 +12,6 @@ TODOs prior to this driver moving out of staging
 1. This driver uses the same unregistered vendor id that the firewire core does
      (0xd00d1e). Perhaps this could be exposed as a define in
      firewire.h?
-3. Maybe device_max_receive() and link_speed_to_max_payload() should be
-     taken up by the firewire core?
 
 -- Issues with TTY core --
   1. Hack for alternate device name scheme
-- 
1.8.1.1