staging/fwserial: Address reviewer comments

PATCH Superseded Peter Hurley 3 participants 18 messages Dec 15, 2012 – Jan 29, 2013 Staging

Share & tools

Peter Hurley

author

2012-12-15 06:03 UTC #0 lore raw

v2 cover staging/fwserial: Address reviewer comments

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

Signature · 1 line

1.8.0.1

Peter Hurley

author

2012-12-15 06:03 UTC #1 lore raw

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

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

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/Kconfig | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/staging/fwserial/Kconfig +3 -1

index 580406c..b2f8331 100644
@@ -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.

Signature · 1 line

1.8.0.1

Peter Hurley

author

2012-12-15 06:04 UTC #2 lore raw

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

In reply to 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 <[email protected]>

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

drivers/staging/fwserial/fwserial.c +0 -3

index 61ee290..be5db8a 100644
@@ -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%.");

drivers/staging/fwserial/fwserial.h +6 -9

index 8b572ed..cb0eea0 100644
@@ -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 */

Signature · 1 line

1.8.0.1

Peter Hurley

author

2012-12-15 06:04 UTC #3 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/staging/fwserial/TODO       | 3 ---
 drivers/staging/fwserial/fwserial.c | 8 ++++----
 drivers/staging/fwserial/fwserial.h | 4 ++--
 3 files changed, 6 insertions(+), 9 deletions(-)

drivers/staging/fwserial/TODO +0 -3

index 7269005..ffe47d1 100644
@@ -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,

drivers/staging/fwserial/fwserial.c +4 -4

index be5db8a..db1378d 100644
@@ -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);

drivers/staging/fwserial/fwserial.h +2 -2

index cb0eea0..953ece6 100644
@@ -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 */

Signature · 1 line

1.8.0.1

Peter Hurley

author

2012-12-15 06:03 UTC #4 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/staging/fwserial/TODO | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

drivers/staging/fwserial/TODO +3 -8

index ffe47d1..8dae8fb 100644
@@ -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

Signature · 1 line

1.8.0.1

Peter Hurley

author

2012-12-15 06:03 UTC #5 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/staging/fwserial/fwserial.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/staging/fwserial/fwserial.c +6 -2

index db1378d..59e90e6 100644
@@ -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)

Signature · 1 line

1.8.0.1

Peter Hurley

author

2012-12-15 06:03 UTC #6 lore raw

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

In reply to Peter Hurley

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

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/TODO | 2 --
 1 file changed, 2 deletions(-)

drivers/staging/fwserial/TODO +0 -2

index 8dae8fb..dc61d97 100644
@@ -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

Signature · 1 line

1.8.0.1

Stefan Richter

2012-12-15 12:34 UTC #7 lore raw

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

Quoted 2 lines · 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.]

Signature · 3 lines

Stefan Richter
-=====-===-- ==-- -====
http://arcgraph.de/sr/

Stefan Richter

2012-12-15 16:09 UTC #8 lore raw

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

Quoted 3 lines · On Dec 15 Peter Hurley wrote:

Quoted 2 lines
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.

Signature · 3 lines

Stefan Richter
-=====-===-- ==-- -====
http://arcgraph.de/sr/

Greg Kroah-Hartman

maintainer

2013-01-07 23:22 UTC #9 lore raw

In reply to Peter Hurley

Quoted 8 lines · 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

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

Peter Hurley

author

2013-01-08 14:59 UTC #10 lore raw

Quoted 10 lines · Greg Kroah-Hartman

Quoted 8 lines · 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

Quoted 2 lines

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

Peter Hurley

author

2013-01-29 01:59 UTC #11 lore raw

v3 cover staging/fwserial: Address reviewer comments

In reply to Greg Kroah-Hartman

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

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 01:59 UTC #12 lore raw

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

Signed-off-by Peter Hurley <[email protected]>

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

drivers/staging/fwserial/fwserial.c +0 -3

index e5e8f2f..233bb56 100644
@@ -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%.");

drivers/staging/fwserial/fwserial.h +6 -9

index caa1c1e..e157318 100644
@@ -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 */

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 02:00 UTC #13 lore raw

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

In reply to Peter Hurley

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/fwserial.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/staging/fwserial/fwserial.c +1 -1

index 233bb56..ae180a3 100644
@@ -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);

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 01:59 UTC #14 lore raw

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

In reply to Peter Hurley

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/fwserial.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/staging/fwserial/fwserial.h +2 -2

index e157318..953ece6 100644
@@ -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 */

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 01:59 UTC #15 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/staging/fwserial/fwserial.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/staging/fwserial/fwserial.c +1 -2

index ae180a3..940639f 100644
@@ -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);

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 01:59 UTC #16 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/staging/fwserial/fwserial.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

drivers/staging/fwserial/fwserial.c +7 -2

index 940639f..48523eb 100644
@@ -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)

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 01:59 UTC #17 lore raw

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

In reply to Peter Hurley

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

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/TODO | 2 --
 1 file changed, 2 deletions(-)

drivers/staging/fwserial/TODO +0 -2

index 8dae8fb..dc61d97 100644
@@ -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

Signature · 1 line

1.8.1.1