Opened 9 years ago

Closed 9 years ago

#482 closed patch (fixed)

Support for Prolific PL2773

Reported by: TomVe Owned by: Christian Franke
Priority: major Milestone: Release 6.4
Component: all Version: 6.3
Keywords: Cc: tommy.vestermark@…

Description

The ICY BOX IB-120StU3 uses a Prolific PL2773 chip, which is currently not supported by smartmontools. By reverse engineering the Windows tool iSmart.exe i have made the included patch. It seems to work well and supports all of smartctl -a.

Please review and consider including in your next release.

Attachments (6)

PL2773_ver1.patch (6.4 KB ) - added by TomVe 9 years ago.
Patch against SVN rev. 4002
PL2773 reverse engineering.txt (2.3 KB ) - added by TomVe 9 years ago.
sg_raw commands replicating output from ismart.exe
PL2773 smartctl test.txt (9.6 KB ) - added by TomVe 9 years ago.
Output from running dmesg, smartctl -x and smartctl -t short
PL2773_ver2.patch (7.6 KB ) - added by TomVe 9 years ago.
Patch against SVN rev. 4002
PL277X_scsi_pass_thru_cmd_v03_Ollie.doc (50.0 KB ) - added by TomVe 9 years ago.
Documentation from ProlificUSA. Command and Status description.
PL2773_DataOut_ver1.patch (5.3 KB ) - added by TomVe 9 years ago.
Patch against SVN rev. 4009

Download all attachments as: .zip

Change History (19)

by TomVe, 9 years ago

Attachment: PL2773_ver1.patch added

Patch against SVN rev. 4002

comment:1 by Alex Samorukov, 9 years ago

Thank you for the contribution! May i ask if you tested smartmontools with -x and -t commands to see if basic functionality works fine? Also it would be great to see documentation update in the patch.

comment:2 by TomVe, 9 years ago

You are welcome :-) And thank you! I am a bit rusty in coding, but found it rather easy to get up and running on your code. Also the code seems pretty well structured, such that adding this driver is (mostly) confined to one file.

I am no SCSI/ATA/SMART wizard, but just tried to replicate the ismart.exe commands by mapping the ATA CDB content by a little googling and trial and error. Although ismart.exe only uses 4 different ATA commands it seems to work pretty well with smartctls larger featureset.

I have attached a file with output from smartctl -x and -t. Please comment whether it looks sane.

I have attached a file with my reverse engineering output. If anything obvious appears please comment.

I would also like some feedback on the following:

  • Is the code generally sane?
  • I disabled support_data_out, as it will hang the drive. I may be missing a R/W bit in the ATA command or something - any ideas?
  • I just tried adding the supports_48bit flag, and it seemed to pass a smartctl -x command with no issues. I do not completely understand how that works as the CDB only contains 3 LBA bytes. Can you see from the code whether it should be safe to enable?
  • I am mostly unsure about the Read ATA output registers functionality. I think regbuf[0] may be the status register (See attached file for some examples). Any advice for how to find the 'error' and 'sector_count' register? What is needed for proper function of smartctl/smartd?
  • Another ismart.exe ATA command (D0) seems to readout the EEPROM content (up to 256 bytes) with firmware revision etc. Does it provide any value to implement that for smartctl?
  • What do you mean by "documentation update"? Should I try to add something to smartctl.8? (I have never made man pages before :-)

If you have any ideas, I can try them out and will rework the patch to your liking.

by TomVe, 9 years ago

sg_raw commands replicating output from ismart.exe

by TomVe, 9 years ago

Attachment: PL2773 smartctl test.txt added

Output from running dmesg, smartctl -x and smartctl -t short

in reply to:  2 comment:3 by Christian Franke, 9 years ago

Milestone: Release 6.4
Owner: set to Christian Franke
Status: newaccepted
  • Is the code generally sane?

Looks very good - thanks.

One suggestion:

+//  printf("DEBUG:REG %x %x %x %x %x %x ...

Replace commented out test code by real debug messages. Check scsi_debug, print with pout():

+  if (scsi_debug > 0)
+    pout("Prolific Registers: %x %x %x %x %x %x ...
  • I disabled support_data_out, as it will hang the drive. I may be missing a R/W bit in the ATA command or something - any ideas?

Is there any reverse engineering result for a DATA OUT command?

  • I just tried adding the supports_48bit flag, and it seemed to pass a smartctl -x command with no issues. I do not completely understand how that works as the CDB only contains 3 LBA bytes. Can you see from the code whether it should be safe to enable?

For SATA bridges, it is normally safe because the SATA protocol layer (unlike PATA) makes no destinction between 28-bit and 48-bit commands (there is no "28-bit Register FIS"). It works with 3 LBA bytes in CDB if the bridge firmware sets the upper LBA bytes in the SATA Register FIS to 0. Use the "ata_device::supports_48bit_hi_null" flag for "ata_cmd_is_supported()" check. Or implement an extra "-d usbprolific,x" option to enable it, see "class usbjmicron_device" for an example.

  • I am mostly unsure about the Read ATA output registers functionality. I think regbuf[0] may be the status register (See attached file for some examples). Any advice for how to find the 'error' and 'sector_count' register? What is needed for proper function of smartctl/smartd?

SMART STATUS requires only LBA MID and HIGH. GET SCT ERC would also require SECTOR COUNT and LBA LOW but this command only works with DATA OUT support. Many interfaces use the traditional pre-IDE register byte order (inherited from some very old WDC controller :-), see "ata_in_regs" and "ata_out_regs".

  • Another ismart.exe ATA command (D0) seems to readout the EEPROM content (up to 256 bytes) with firmware revision etc. Does it provide any value to implement that for smartctl?

There is no smartctl option to handle such device specific commands.

  • What do you mean by "documentation update"? Should I try to add something to smartctl.8? (I have never made man pages before :-)

Add description for "usbprolific" parameter for "-d" option to smartctl.8.in and smartd.conf.5.in. Not mandatory, we could add this later.

by TomVe, 9 years ago

Attachment: PL2773_ver2.patch added

Patch against SVN rev. 4002

comment:4 by TomVe, 9 years ago

Thanks for the feedback!

I have made a version 2 of the patch with the following modifications:

  • Removed the debug printf's as I learned about -r scsiioctl,2 :-)
  • Added supports_48bit_hi_null, as it works fine with -x
  • Added man pages
  • Minor clean-up

The patch is tested with two disks attached to the controller, and both drives worked fine. I might experiment with Data Out support at a later time - but it is not used by ismart.exe. It seems to work OK for me at the time being. Please consider merging.

comment:5 by Christian Franke, 9 years ago

Resolution: fixed
Status: acceptedclosed

Patch applied in r4004.

The addition to drivedb.h is omitted for now because the new -d option would break the backward compatibility which is required for /usr/sbin/update-smart-drivedb. I need to create new drivedb.h branches for 6.1 - 6.3 first. You could add the Prolific entry to a local /etc/smart_drivedb.h file.

I also removed the download URL because it apparently requires a login.

comment:6 by Alex Samorukov, 9 years ago

@chrfranke here it is available w/o password and registration: http://prolificusa.com/portfolio/pl-2773-usb3-0-esataii-to-sataii-bridge-controller/

in reply to:  6 comment:7 by Christian Franke, 9 years ago

New drivedb.h branches created, Prolific entry added to trunk and merged (as unsupported) to branches: r4007, r4008, r4009.

in reply to:  6 comment:8 by Christian Franke, 9 years ago

Replying to samm2:

@chrfranke here it is available w/o password and registration: ...

Thanks. Should we add it to scsiata.cpp or to the Wiki? I am not sure.

by TomVe, 9 years ago

Documentation from ProlificUSA. Command and Status description.

comment:9 by TomVe, 9 years ago

Cc: tommy.vestermark@… added

I have asked ProlificUSA for information/verification of the pass-through command and status registers. They kindly sent me the attached document. The patch should be OK as is.

I will however try to make a new patch with added support for DATA OUT commands (rebased of r4009). When done, I will attach it to this ticket (to keep it together) and re-open the ticket - unless you want me to make a new ticket instead.

Last edited 9 years ago by TomVe (previous) (diff)

by TomVe, 9 years ago

Attachment: PL2773_DataOut_ver1.patch added

Patch against SVN rev. 4009

comment:10 by TomVe, 9 years ago

Resolution: fixed
Status: closedreopened

Made a new patch with support for DATA OUT and OUTPUT REGS and some minor clean up based on the information in the Prolific document. Works fine with '-x' and can now print temperature log and SCT. Test with '-t select' and a span also works now.

It might be possible to add support for true 48 bit LBA. Although not documented, I suspect the PREFIX might be for sending the high order bits (not tested). Furthermore it should be easy add support for readout of the high order bits in the status register. However, I am unsure of the value of doing this, as everything seems to work perfectly now. Furthermore my knowledge of the code base it probably too thin to attempt this...

Version 0, edited 9 years ago by TomVe (next)

in reply to:  10 comment:11 by Christian Franke, 9 years ago

Status: reopenedaccepted

Replying to TomVe:

Made a new patch with support for DATA OUT and OUTPUT REGS and some minor clean up based on the information in the Prolific document. Works fine with '-x' and can now print temperature log and SCT. Test with '-t select' and a span also works now.

Thanks! I will commit it soon.

It might be possible to add support for true 48 bit LBA. Although not documented, I suspect the PREFIX might be for sending the high order bits (not tested).

I presume the PREFIX is like the "previous registers" (clever:-) hack from the IDE/PATA interface. Nonzero values would be only needed for READ LOG EXT if a log contains more than 255 sectors.

Hmm... it might make sense to always issue the PREFIX command for 48-bit ATA commands to make sure that the upper bytes are properly set. See "usbsunplus_device" for an example.

Furthermore it should be easy add support for readout of the high order bits in the status register.

According to (even recent) ATA standards, the "Status field is one byte". The only use case for the upper bytes of other output registers are failed READ/WRITE EX commands which return the first failed LBA in the output registers. Such commands are not used by smartmontools.

comment:12 by TomVe, 9 years ago

If READ LOG EXT with more than 255 sectors is the only use case for true 48 bit, I must confess my motivation for risking adding issues due to misinterpreting PREFIX is low. I wouldn't even know how to test it thoroughly, so I think I will just leave it as is. It currently works pretty well and Prolifics ismart.exe does not issue these commands either.

BTW I asked Prolific for more USB IDs compatible with this interface, but have not yet received an answer. I will open a new ticket, if I can add support for more USB IDs than 0x2773 at a later time.

comment:13 by Christian Franke, 9 years ago

Resolution: fixed
Status: acceptedclosed

Patch applied in r4011.

Note: See TracTickets for help on using tickets.