Bug 8092

Summary: transaction code (tcode) not set for some hpsb packets
Product: Drivers Reporter: arsene (s_lu_3)
Component: IEEE1394Assignee: Stefan Richter (stefanr)
Status: REJECTED INVALID    
Severity: normal    
Priority: P2    
Hardware: i386   
OS: Linux   
Kernel Version: 2.6.20 Subsystem:
Regression: --- Bisected commit-id:

Description arsene 2007-02-26 17:57:31 UTC
Distribution: Linux-2.6.20
Problem Description:
I was reading source code and felt not sure if what described following is a 
bug.

It seems that, before a hpsb_packet is sent out (drivers/ieee1394), its 
`tcode' field, representing transaction code, should be set. This holds for 
all types of hpsb packet, except one case:

In file drivers/ieee1394/ieee1394_transactions.c, function hpsb_make_phypacket
(struct hpsb_host *host, quadlet_t data) does not fill the 'tcode' field of 
the generated hpsb_packet, which will be sent out without further modification.

For all other types of hpsb packet, the generated packet has all the important 
fields set, including tcode field. (refer to hpsb_make_readpacket, 
hpsb_make_writepacket, hpsb_make_streampacket, hpsb_make_lockpacket, 
hpsb_make_lock64packet,hpsb_make_isopacket) Only in hpsb_make_phypacket, 
packet->tcode is not set. Is this a bug?

Following is the code related to phypacket:

File drivers/ieee1394/ieee1394_transactions.c 
459 struct hpsb_packet *hpsb_make_phypacket(struct hpsb_host *host, quadlet_t 
data)
460 {
461	struct hpsb_packet *p;
462
463	p = hpsb_alloc_packet(0);
464	if (!p)
465		return NULL;
466
467	p->host = host;
468	fill_phy_packet(p, data);
469
470	return p;
471 }

File drivers/ieee1394/ieee1394_transactions.c  
99  static void fill_phy_packet(struct hpsb_packet *packet, quadlet_t data)
100 {
101	packet->header[0] = data;
102	packet->header[1] = ~data;
103	packet->header_size = 8;
104	packet->data_size = 0;
105	packet->expect_response = 0;
106	packet->type = hpsb_raw;	/* No CRC added */
107	packet->speed_code = IEEE1394_SPEED_100;	/* Force speed to be 
100Mbps */
108 }

In above functions, neither hpsb_alloc_packet(..) nor fill_phy_packet(..) has 
filled the packet->tcode. Based on the code of other type of hpsb packets, 
packet->tcode should (usually) be filled by fill_xx_packet function (like fill 
fill_iso_packet, fill_async_stream_packet, etc.). However, fill_phy_packet 
doesnot. 


Maybe tcode is not necessary for `phy_packet'. However, it seems that whenever 
a packet is received, the packet's tcode will be checked. If the tcode is not 
correctly set, there would be some error message  ( void hpsb_packet_received
(...) function in file drivers/ieee1394/ieee1394_core.c). 

Above observation makes me suspect fill_phy_packet function is not correct in 
not filling the packet->tcode, but I am not sure about it.
Comment 1 Stefan Richter 2007-02-26 21:50:43 UTC
PHY packets are different from transaction request packets or transaction
reponse packets. They don't contain a transaction code header field.
Furthermore, we re-use some of the transaction send code to send PHY packets,
but not to receive PHY packets. Thus there is also no need for internal pseudo
transaction codes.