Skip to content

Conversation

2427054102
Copy link
Contributor

@2427054102 2427054102 commented Aug 2, 2025

ptool advice:Fix Partition Rounding, Alignment, and Erase Logic

Copy link
Contributor

@lumag lumag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No improvements since #22.

  • Each commit should be described properly in the commit message (and subject)
  • Missing Signed-off-by tags
    also:
  • Don't close PRs just to post a new iteration. Instead please push to the same branch in order to keep review visible to others
  • Don't add stray merge commits. Please rebase/reshuffle your changes in order to get rid of it.

@ricardosalveti
Copy link
Contributor

Please see go/OSSBestPractices for an overview on how to better produce commit messages.

Everything is part of the patch subject, please change to have a simplified subject line and a proper commit body with the rest of the commit message. Also please explain better the justification for the changes, not just saying what you are changing.

@2427054102 2427054102 force-pushed the v2 branch 2 times, most recently from 55b3325 to 1d3fb95 Compare August 5, 2025 01:30
@2427054102
Copy link
Contributor Author

ok, I have modified it as requested, please see if there are any other issues. Thank you

@ricardosalveti
Copy link
Contributor

Some more comments:

  • Please update your Author line, you have xiaomzh <[email protected]> as author and xiaomzh <[email protected]> as sign-off-by, please align both. DCO github action is failing.
  • Wrap the lines in your commit bodies, do not write very long lines. 80-100 columns max.

@2427054102
Copy link
Contributor Author

ok. Thanks for the feedback. I've updated the Author line to match the Signed-off-by line (xiaomzh [email protected]) to resolve the DCO check failure.
Also, all commit messages have been rewrapped to respect the 80–100 column limit.
Please let me know if anything else needs adjustment.

@2427054102 2427054102 changed the title V2 ptool advice Aug 10, 2025
@lumag
Copy link
Contributor

lumag commented Aug 11, 2025

ok. Thanks for the feedback. I've updated the Author line to match the Signed-off-by line (xiaomzh [email protected]) to resolve the DCO check failure.

This is not your real name. Please adjust your git setup and then the author and tags accordingly.

Also, all commit messages have been rewrapped to respect the 80–100 column limit. Please let me know if anything else needs adjustment.

Please use imperative language. I.e. 'Adjust foo, Fix bar, Provision baz' instead of descriptive ('This change makes foo being adjusted...').

@2427054102 2427054102 changed the title ptool advice Fix Partition Rounding, Alignment, and Erase Logic Aug 12, 2025
@2427054102
Copy link
Contributor Author

Please let me know if there are any further adjustments needed. Thanks!

Copy link
Collaborator

@vkraleti vkraleti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks much better now.

ptool.py Outdated
TempResult +=1
PhyPartition[k][j]['size_in_kb'] = (TempResult * SECTOR_SIZE_IN_BYTES)/1024
size_kb = float(PhyPartition[k][j]['size_in_kb'])
size_bytes = math.ceil(size_kb * 1024) #Count the number of bytes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the extra whitespaces at the end of the line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the extra whitespaces at the end of the lines. Thanks for the reminder!

ptool.py Outdated
PhyPartition[k][j]['size_in_kb'] = (TempResult * SECTOR_SIZE_IN_BYTES)/1024
size_kb = float(PhyPartition[k][j]['size_in_kb'])
size_bytes = math.ceil(size_kb * 1024) #Count the number of bytes
sectors = math.ceil(size_bytes / SECTOR_SIZE_IN_BYTES) #Calculate the number of sectors rounded up
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One whitespace at the end of this one as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

ptool.py Outdated
sectors = math.ceil(size_bytes / SECTOR_SIZE_IN_BYTES) #Calculate the number of sectors rounded up
PhyPartition[k][j]['size_in_kb'] = sectors * SECTOR_SIZE_IN_BYTES / 1024

##import pdb; pdb.set_trace() ## verifying sizes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commented import pdb won't be in the correct indentation after your changes, either remove (as a separated patch) or align the indentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing that out! I've aligned the indentation of the commented import pdb line in the latest commit. Please let me know if there's anything else that needs adjustment.

Fix partition sector rounding to avoid decimal sector counts.
Ensure partition memory size is rounded up to the next whole sector.
Prevent insufficient space allocation due to fractional sector values.

Signed-off-by: Xiaoming Zhao <[email protected]>
Remove incorrect condition checking ALIGN_PARTITIONS_TO_PERFORMANCE_BOUNDARY.
Ensure partitions align correctly to required boundaries such as 4KB.
Prevent logic errors caused by unnecessary alignment checks.

Signed-off-by: Xiaoming Zhao <[email protected]>
Erase main GPT sectors dynamically based on sector size.
Use 34 sectors for 512-byte sectors and 6 for 4096-byte sectors.
Improve accuracy and compatibility with different storage configurations.

Signed-off-by: Xiaoming Zhao <[email protected]>
Preserve decimal precision in partition size specification.
Avoid truncating fractional values during memory allocation.
Enable more accurate and efficient memory usage.

Signed-off-by: Xiaoming Zhao <[email protected]>
Erase MBR sector dynamically based on actual sector size.
Use 0.5KB for 512-byte sectors and 4KB for 4096-byte sectors.
Improve compatibility across devices with varying sector sizes.

Signed-off-by: Xiaoming Zhao <[email protected]>
Erase backup GPT sectors dynamically based on sector size.
Use 33 sectors for 512-byte and 5 for 4096-byte configurations.
Ensure accurate and adaptable erase operations.

Signed-off-by: Xiaoming Zhao <[email protected]>
Fix MBR sector occupancy calculation to reflect dynamic sector sizes.
Ensure MBR occupies one sector: 0.5KB or 4KB depending on configuration.
Match actual device layout for correctness.

Signed-off-by: Xiaoming Zhao <[email protected]>
Fix undefined variable reference in Partition hash instructions.
Replace incorrect variable with valid HashInstructions key.
Prevent runtime errors and improve reliability.

Signed-off-by: Xiaoming Zhao <[email protected]>
Copy link
Contributor

@ndechesne ndechesne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the clean up. Pretty nice, especially given how un-readable this piece of code is :)

@ndechesne ndechesne merged commit e71fe81 into qualcomm-linux:main Aug 19, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants