linux icon indicating copy to clipboard operation
linux copied to clipboard

Adding Crypto support

Open UtsavAgarwalADI opened this issue 1 year ago • 4 comments

PR Description

  • Please replace this comment with a summary of your changes, and add any context necessary to understand them. List any dependencies required for this change.
  • To check the checkboxes below, insert a 'x' between square brackets (without any space), or simply check them after publishing the PR.
  • If you changes include a breaking change, please specify dependent PRs in the description and try to push all related PRs simultaneously.

PR Type

  • [x] Bug fix (a change that fixes an issue)
  • [x] New feature (a change that adds new functionality)
  • [ ] Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

  • [x] I have conducted a self-review of my own code changes
  • [x] I have tested the changes on the relevant hardware
  • [x] I have updated the documentation outside this repo accordingly (if there is the case)

NOTE: hardware cryptographic functions were not tested at the moment due to absence of the devcrypto engine userspace library

UtsavAgarwalADI avatar Apr 19 '24 14:04 UtsavAgarwalADI

Thank you for your review.

I have made the requested changes in addition to some indentation for the source files - to allow it being more in line with the kernel coding style(used gnu indent with kernel specific settings).

Please let me know if there are any further changes that you feel may be necessary.

UtsavAgarwalADI avatar Apr 22 '24 11:04 UtsavAgarwalADI

@UtsavAgarwalADI
I have a question here :) What is the desired level of review here?

For upstream kernel (review level), quite a bit more work would be required (to review, and re-spin). Then, when/if upstreaming this driver, upstream maintainers would still have some feedback (which is usually more about preference). For internal (non-upstream) use of this driver, the review can be more relaxed.

The initial review (which I did) was a bit more late-Friday quick-scan. I'll need to make some time for something more thorough.

commodo avatar Apr 22 '24 12:04 commodo

@commodo

Ideally these files(this and a series of others before - these are basically ports of a non-upstream LTS branch to the current kernel version) are intended to be eventually submitted to the kernel mailing list.

As you had correctly mentioned, there is a lot of work needed for re-spinning it in addition to feedback from the kernel maintainers.

Hence, if possible, I would request an upstream review, however, any feedback at all would be appreciated.

UtsavAgarwalADI avatar Apr 22 '24 13:04 UtsavAgarwalADI

I have tried addressing all of the above-mentioned concerns/suggestions. In addition, I have also attempted to try and remove any other redundant code and added checks for long/complex functions which previously could never fail.

UtsavAgarwalADI avatar Apr 25 '24 11:04 UtsavAgarwalADI