Bug 8092 - transaction code (tcode) not set for some hpsb packets
Summary: transaction code (tcode) not set for some hpsb packets
Status: REJECTED INVALID
Alias: None
Product: Drivers
Classification: Unclassified
Component: IEEE1394 (show other bugs)
Hardware: i386 Linux
: P2 normal
Assignee: Stefan Richter
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-02-26 17:57 UTC by arsene
Modified: 2007-02-26 21:50 UTC (History)
0 users

See Also:
Kernel Version: 2.6.20
Subsystem:
Regression: ---
Bisected commit-id:


Attachments

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.

Note You need to log in before you can comment on or make changes to this bug.