Re: [PATCH] Fix missing sysfs vendor/model entries for some devices
Brought to you by:
aeb,
bencollins
Menu
▾
▴
Re: [PATCH] Fix missing sysfs vendor/model entries for some devices
|
From: Takashi S. <o-t...@sa...> - 2023-11-27 10:56:01
|
On Sun, Nov 26, 2023 at 02:37:56AM -0800, Adam Goldman wrote: > Hi, > > When the Sony DVMC-DA1 is connected, /sys/bus/firewire/devices/fw1/ has > a vendor entry, but is missing vendor_name, model, and model_name. > > This is the DVMC-DA1's config ROM: > > 00000000: fbe7 1e04 3439 3331 0040 64e0 0346 0008 ....4931.@d..F.. > 00000010: 3c19 1400 81b6 0600 4600 0803 c083 000c <.......F....... > 00000020: 0a00 008d 0300 00d1 0500 00c3 0a00 0081 ................ > 00000030: bfcd 0200 2da0 0012 0000 0113 fe0c 0200 ....-........... > 00000040: 0000 fa17 0800 0081 6ec6 0200 0346 0008 ........n....F.. > 00000050: 3c19 1400 269e 0300 0000 0000 0000 0000 <...&........... > 00000060: 796e 6f53 1d00 0500 0000 0000 0000 0000 ynoS............ > 00000070: 434d 5644 3141 442d 0000 0000 CMVD1AD-.... > > Note that the root directory contains two entries for vendor. The first > is a Vendor_ID immediate value, and the second is a Vendor_Info > directory. The descriptor for vendor_name is after the Vendor_Info > entry. However, search_leaf compares the directory entry high byte > against the desired key_id without masking off the type bits, so it > doesn't recognize the Vendor_Info entry. > > Also, the model and model_name are not in the root directory, but > instead are in the vendor directory. search_leaf and show_immediate > don't see these, because they only search the root directory. > > The following patch fixes the above problems. > > The new behavior is conformant with Annex A of 1394TA Document 1999027, > Configuration ROM for AV/C Devices 1.0: "Some legacy devices may have > their Model_IDs and textual descriptors in minimal ASCII subset in their > vendor directories. It is recommended for a controller to read the > Model_ID and its descriptor in the vendor directory of a target only if > the Model_ID and its descriptor are not present in the root directory of > the target." > > In addition to the DVMC-DA1, other devices are also affected. This patch > also fixes the missing sysfs entries for the Sony DCR-TRV120 and > Panasonic AG-DV1DC. The Sony DCR-TRV310 has been reported to have the > same problem and should be fixed by the patch, but I don't have one for > testing. Thanks for the patch. Indeed, for the legacy layout of Configuration ROM, current implementation of core functionality does not pick up enough information from the vendor directory. I think you already know the similar case in the issue filed in systemd project[1]. The patch format is not compliant to Linux kernel development, while I would evaluate it. However, I need more actual examples. Would I ask you to provide images of Configuration ROM retrieved by your test devices? I think you own Sony DCR-TRV120 and Panasonic AG-DV1DC. I maintain collections of Configuration ROM[2] and it is preferable to receive merge request in github.com service. Of cource, you can send them directly to me, instead. I have a plan to move the collection to git.kernel.org, thus we take enough care of image license. I think it better to use CC0[3]. Well, I note that you can decode the content of Configuration ROM by 'config-rom-pretty-printer' in linux-firewire-utils. The tool is written by C language and can be build in your system, I believe: https://git.kernel.org/pub/scm/utils/ieee1394/linux-firewire-utils.git/ For example: ``` $ cat rom.img | config-rom-pretty-printer ROM header and bus information block ----------------------------------------------------------------- 1024 041ee7fb bus_info_length 4, crc_length 30, crc 59387 1028 31333934 bus_name "1394" 1032 e0644000 irmc 1, cmc 1, isc 1, bmc 0, cyc_clk_acc 100, max_rec 4 (32) 1036 08004603 company_id 080046 | 1040 0014193c device_id 12886219068 | EUI-64 576537731003586876 root directory ----------------------------------------------------------------- 1044 0006b681 directory_length 6, crc 46721 1048 03080046 vendor 1052 0c0083c0 node capabilities: per IEEE 1394 1056 8d00000a --> eui-64 leaf at 1096 1060 d1000003 --> unit directory at 1072 1064 c3000005 --> vendor directory at 1084 1068 8100000a --> descriptor leaf at 1108 unit directory at 1072 ----------------------------------------------------------------- 1072 0002cdbf directory_length 2, crc 52671 1076 1200a02d specifier id 1080 13010000 version vendor directory at 1084 ----------------------------------------------------------------- 1084 00020cfe directory_length 2, crc 3326 1088 17fa0000 model 1092 81000008 --> descriptor leaf at 1124 eui-64 leaf at 1096 ----------------------------------------------------------------- 1096 0002c66e leaf_length 2, crc 50798 1100 08004603 company_id 080046 | 1104 0014193c device_id 12886219068 | EUI-64 576537731003586876 descriptor leaf at 1108 ----------------------------------------------------------------- 1108 00039e26 leaf_length 3, crc 40486 1112 00000000 textual descriptor 1116 00000000 minimal ASCII 1120 536f6e79 "Sony" descriptor leaf at 1124 ----------------------------------------------------------------- 1124 0005001d leaf_length 5, crc 29 1128 00000000 textual descriptor 1132 00000000 minimal ASCII 1136 44564d43 "DVMC" 1140 2d444131 "-DA1" 1144 00000000 ``` Finally, I apologize in advance that my patch review would be delayed since I'm involved to critical kernel regression in the combination of AMD Ryzen machine and a kind of 1394 OHCI cards[4]. It takes more days to review your patch, sorry. [1] IEEE1394_UNIT_FUNCTION_VIDEO not set on camcorder (because it does not have ATTR{model}?) https://github.com/systemd/systemd/issues/25029 [2] https://github.com/takaswie/am-config-roms/ [3] https://creativecommons.org/publicdomain/zero/1.0/ [4] https://lore.kernel.org/lkml/20231105144852.GA165906@workstation.local/ Thanks Takashi Sakamoto |