8000 Add Phison MSD210 (MSD210-M8320GPSCB5UFI-S122) by phison-someone · Pull Request #343 · smartmontools/smartmontools · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add Phison MSD210 (MSD210-M8320GPSCB5UFI-S122) #343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Add Phison MSD210 (MSD210-M8320GPSCB5UFI-S122) #343

wants to merge 16 commits into from

Conversation

phison-someone
Copy link
< 8000 /span>

To Whom It May Concern,

I am writing to submit a pull request to modify Phison's MSD210 series.
drivedb.h & change log have been updated.

Please let me know if you have any questions. Thank you.

@phison-someone phison-someone changed the title Phison someone patch msd210 Add Phison MSD210 May 12, 2025
@phison-someone
Copy link
Author

smartctrl

logtest001 (2).log

@chrfranke chrfranke added drivedb Entries to the drivedb.h undecided labels May 14, 2025
@chrfranke
Copy link
Contributor

The accumulated diff of this PR shows that the model family string is missing. Smartctl refuses the patched file:

$ smartctl -B ./drivedb.h -P showall 'MSD210-M8320GPSCB5UFI-S122'
./drivedb.h(192): Syntax error, ',' expected

This makes the PR useless.

Important: PLEASE test each new entry before posting as a PR.

The attributes look like a subset of the existing entry Phison Driven SSDs. Please consider to add the new drives to the regex of the existing entry or explain why this would not work.

@phison-someone
Update drivedb.h
ca4c9ed 
add model family info
@phison-someone
Update drivedb.h
cf206e0 
modify Informal string about the model family.
@chrfranke
Copy link
Contributor

Thanks.

  • Please explain why an entry separate from Phison Driven SSDs entry is needed. We do not add new entries for each new drive series if the attribute settings are similar.
  • The new entry should be located below or above the existing Phison ... entries.
  • The "SPCC Solid State Disk" // ... part is obviously block copied from Phison Driven OEM SSDs which does not make sense.
  • The firmware regex S[89ABCH]F[DMI][0-9T][0-9]\\.[0-9] is not needed unless "SPCC Solid State Disk" is really intentional. In the latter case, please fix the block copied // tested with ... comment and provide a sample smartctl -x -a output for a "SPCC Solid State Disk" device.
  • In the regex, please use [..] instead of (.|.) for single char alternatives, for example [48] instead of (4|8).
  • The ChangeLog comment should be informal only and should not contain the full regex.

@chrfranke chrfranke changed the title Add Phison MSD210 Add Phison MSD210 (MSD210-M8320GPSCB5UFI-S122) May 26, 2025
@phison-someone
Update drivedb.h
407930a 
1. below the existing Phison entries
2. delete "SPCC Solid State Disk"
3. modify firmware and model name regex
@phison-someone
Copy link
Author
  • the regex
    Phison Driven SSDs :
    "-v 173,raw16(avg16),MaxAvgErase_Ct "
    Phison MSD210 Series SSD :
    "-v 173,raw24/raw24:z10z32,Erase_Ct_Max/Avg "
    The above two (173) are different. So we need add new entries.
    Other parts have been modified.
    Thanks

Thanks.

  • Please explain why an entry separate from Phison Driven SSDs entry is needed. We do not add new entries for each new drive series if the attribute settings are similar.
  • The new entry should be located below or above the existing Phison ... entries.
  • The "SPCC Solid State Disk" // ... part is obviously block copied from Phison Driven OEM SSDs which does not make sense.
  • The firmware regex S[89ABCH]F[DMI][0-9T][0-9]\\.[0-9] is not needed unless "SPCC Solid State Disk" is really intentional. In the latter case, please fix the block copied // tested with ... comment and provide a sample smartctl -x -a output for a "SPCC Solid State Disk" device.
  • In the regex, please use [..] instead of (.|.) for single char alternatives, for example [48] instead of (4|8).
  • The ChangeLog comment should be informal only and should not contain the full regex.

@phison-someone
Copy link
Author

smartctl -B ./drivedb_git.h -P showall 'MSD210-M4320GPSCB5UFI-S121'
smartctl_msd210.txt
smartctl -x -a /dev/sda -B ./drivedb_git.h
smartctl_msd210_1.txt

@chrfranke
Copy link
Contributor

The 'master' branch is now retired, see #346. Please create a new PR for the 'main' branch.
Please also move the new entry to above or below the existing entry Phison Driven SSDs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
drivedb Entries to the drivedb.h undecided
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@phison-someone @chrfranke
0