thunderbolt: debugfs: Show all accessible dwords
Subsystems (2)
PATCH 1/1 thunderbolt: debugfs: Show all accessible dwords
Currently, when first failure occurs while reading of the block, we stop reading the block and jump to the next capability. This doesn't cover the case of block with "holes" of inaccessible dwords, followed by accessible dwords.
This patch address this problem.
In case of failure while reading the complete block in one transaction, (because of one or more dwords is inaccessible), we read the remaining dwords of the block dword-by-dword, one dword per transaction, till the end of the block.
By doing this, we handle the case of block with "holes" of inaccessible dwords, followed by accessible dwords. The accessible dwords are shown with the fields: <offset> <relative_offset> <cap_id> <vs_cap_id> <value> E.g.:0x01eb 236 0x05 0x06 0x0000d166
While the inaccesible dwords are shown as: <offset> <not accessible> E.g.:0x01ed <not accessible>
--- drivers/thunderbolt/debugfs.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/thunderbolt/debugfs.c +3 -5
@@ -265,10 +265,8 @@ static void cap_show_by_dw(struct seq_file *s, struct tb_switch *sw,else ret =tb_sw_read (sw, &data,TB_CFG_SWITCH , cap + offset + i,1 );if (ret) { -seq_printf (s,"0x%04x <not accessible>\n" , cap + offset); -if (dwords - i >1 ) -seq_printf (s,"0x%04x ...\n" , cap + offset +1 ); -return ; +seq_printf (s,"0x%04x <not accessible>\n" , cap + offset + i); +continue ; } seq_printf(s, "0x%04x %4d 0x%02x 0x%02x 0x%08x\n", cap + offset + i,
@@ -292,7 +290,7 @@ static void cap_show(struct seq_file *s, struct tb_switch *sw,else ret =tb_sw_read (sw, data,TB_CFG_SWITCH , cap + offset, dwords);if (ret) { -cap_show_by_dw (s, sw, port, cap, offset, cap_id, vsec_id, dwords); +cap_show_by_dw (s, sw, port, cap, offset, cap_id, vsec_id, length);return ; }
Signature · 9 lines
2.17.1 --------------------------------------------------------------------- Intel Israel (74) Limited This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
Yehezkel Bernat
maintainerQuoted 10 lines · Gil Fine
Currently, when first failure occurs while reading of the block, we stop reading the block and jump to the next capability. This doesn't cover the case of block with "holes" of inaccessible dwords, followed by accessible dwords.
This patch address this problem.
In case of failure while reading the complete block in one transaction, (because of one or more dwords is inaccessible), we read the remaining dwords of the block dword-by-dword, one dword per transaction, till the end of the block.
Just wondering: is there any chance this will slow down the device adding flow if there are many inaccessible dwords, or we expect the read to return immediately for such dwords? If for some devices it may be a slow process, does it make sense to have a way to disable this feature for specific devices or by a module parameter? Maybe we don't worry about it as it's only for debugfs?
Thanks,
Yehezkel
Quoted 12 lines · Yehezkel Bernat
Quoted 10 lines · Gil Fine
Currently, when first failure occurs while reading of the block, we stop reading the block and jump to the next capability. This doesn't cover the case of block with "holes" of inaccessible dwords, followed by accessible dwords.
This patch address this problem.
In case of failure while reading the complete block in one transaction, (because of one or more dwords is inaccessible), we read the remaining dwords of the block dword-by-dword, one dword per transaction, till the end of the block.
Quoted 5 lines
Just wondering: is there any chance this will slow down the device adding flow if there are many inaccessible dwords, or we expect the read to return immediately for such dwords? If for some devices it may be a slow process, does it make sense to have a way to disable this feature for specific devices or by a module parameter? Maybe we don't worry about it as it's only for debugfs?
We don't worry about it as it is just debugfs ;-)
Mika Westerberg
AppliedQuoted 18 lines · Gil Fine
Currently, when first failure occurs while reading of the block, we stop reading the block and jump to the next capability. This doesn't cover the case of block with "holes" of inaccessible dwords, followed by accessible dwords.
This patch address this problem.
In case of failure while reading the complete block in one transaction, (because of one or more dwords is inaccessible), we read the remaining dwords of the block dword-by-dword, one dword per transaction, till the end of the block.
By doing this, we handle the case of block with "holes" of inaccessible dwords, followed by accessible dwords. The accessible dwords are shown with the fields: <offset> <relative_offset> <cap_id> <vs_cap_id> <value> E.g.:0x01eb2360x050x060x0000d166
While the inaccesible dwords are shown as: <offset> <not accessible> E.g.:0x01ed<not accessible>
Quoted 1 line
Signed-off-by Gil Fine <[email protected]>
Applied to thunderbolt.git/next, thanks!
Hi Gil,
Quoted 42 lines · Gil Fine
Currently, when first failure occurs while reading of the block, we stop reading the block and jump to the next capability. This doesn't cover the case of block with "holes" of inaccessible dwords, followed by accessible dwords.
This patch address this problem.
In case of failure while reading the complete block in one transaction, (because of one or more dwords is inaccessible), we read the remaining dwords of the block dword-by-dword, one dword per transaction, till the end of the block.
By doing this, we handle the case of block with "holes" of inaccessible dwords, followed by accessible dwords. The accessible dwords are shown with the fields: <offset> <relative_offset> <cap_id> <vs_cap_id> <value> E.g.:0x01eb2360x050x060x0000d166
While the inaccesible dwords are shown as: <offset> <not accessible> E.g.:0x01ed<not accessible>Signed-off-by Gil Fine <[email protected]>--- drivers/thunderbolt/debugfs.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)drivers/thunderbolt/debugfs.c +2
-4@@ -265,10 +265,8 @@ static void cap_show_by_dw(struct seq_file *s, struct tb_switch *sw,else ret =tb_sw_read (sw, &data,TB_CFG_SWITCH , cap + offset + i,1 );if (ret) { -seq_printf (s,"0x%04x <not accessible>\n" , cap + offset); -if (dwords - i >1 ) -seq_printf (s,"0x%04x ...\n" , cap + offset +1 ); -return ; +seq_printf (s,"0x%04x <not accessible>\n" , cap + offset + i); +continue ; } seq_printf(s, "0x%04x %4d 0x%02x 0x%02x 0x%08x\n", cap + offset + i,
drivers/thunderbolt/debugfs.c static void cap_show(struct seq_file *s, struct tb_switch *sw, · line 295
Quoted 4 lines
drivers/thunderbolt/debugfs.c +0
-1@@ -292,7 +290,7 @@ static void cap_show(struct seq_file *s, struct tb_switch *sw,else ret =tb_sw_read (sw, data,TB_CFG_SWITCH , cap + offset, dwords);if (ret) { -cap_show_by_dw (s, sw, port, cap, offset, cap_id, vsec_id, dwords);
Sorry for being late here:
Can we call cap_show_by_dw(..., dwords) directly here, instead of having the logic of doing the block read and then resorting to this if the block read fails?
Since it's debugfs, I doubt efficiency gains are paramount here, and we have a simpler invocation at the callsite.
Quoted 15 lines
drivers/thunderbolt/debugfs.c +1
-1+cap_show_by_dw (s, sw, port, cap, offset, cap_id, vsec_id, length);return ; } --2.17.1
---------------------------------------------------------------------
Intel Israel (74) LimitedThis e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
Hi,
Quoted 44 lines · Prashant Malani
Hi Gil,
Quoted 40 lines · Gil Fine
Currently, when first failure occurs while reading of the block, we stop reading the block and jump to the next capability. This doesn't cover the case of block with "holes" of inaccessible dwords, followed by accessible dwords.
This patch address this problem.
In case of failure while reading the complete block in one transaction, (because of one or more dwords is inaccessible), we read the remaining dwords of the block dword-by-dword, one dword per transaction, till the end of the block.
By doing this, we handle the case of block with "holes" of inaccessible dwords, followed by accessible dwords. The accessible dwords are shown with the fields: <offset> <relative_offset> <cap_id> <vs_cap_id> <value> E.g.:0x01eb2360x050x060x0000d166
While the inaccesible dwords are shown as: <offset> <not accessible> E.g.:0x01ed<not accessible>Signed-off-by Gil Fine <[email protected]>--- drivers/thunderbolt/debugfs.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)drivers/thunderbolt/debugfs.c +2
-4@@ -265,10 +265,8 @@ static void cap_show_by_dw(struct seq_file *s, struct tb_switch *sw,else ret =tb_sw_read (sw, &data,TB_CFG_SWITCH , cap + offset + i,1 );if (ret) { -seq_printf (s,"0x%04x <not accessible>\n" , cap + offset); -if (dwords - i >1 ) -seq_printf (s,"0x%04x ...\n" , cap + offset +1 ); -return ; +seq_printf (s,"0x%04x <not accessible>\n" , cap + offset + i); +continue ; }
Quoted 13 lines
drivers/thunderbolt/debugfs.c static void cap_show(struct seq_file *s, struct tb_switch *sw, · line 295
Quoted 6 lines
drivers/thunderbolt/debugfs.c +0
-1seq_printf(s, "0x%04x %4d 0x%02x 0x%02x 0x%08x\n", cap + offset + i,@@ -292,7 +290,7 @@ static void cap_show(struct seq_file *s, struct tb_switch *sw,else ret =tb_sw_read (sw, data,TB_CFG_SWITCH , cap + offset, dwords);if (ret) { -cap_show_by_dw (s, sw, port, cap, offset, cap_id, vsec_id, dwords);Sorry for being late here:
Can we callcap_show_by_dw(..., dwords)directly here, instead of having the logic of doing the block read and then resorting to this if the block read fails?
Since it's debugfs, I doubt efficiency gains are paramount here, and we have a simpler invocation at the callsite.
I don't think that complicates too much, and in most cases we don't expect the cap read to fail so doing block read saves us sending multiple messages over the fabric (even if we don't care about the performance here).
In any case the patch is already applied so if you want to change this please send an incremental patch.