Page MenuHomeFreeBSD

Maintenance's commit of Microsemi smartpqi driver
ClosedPublic

Authored by deepak.ukey_microsemi.com on Jun 29 2018, 11:38 AM.

Details

Summary

This maintenance commit contains fixes and features for Microsemi's smartpqi module.

Test Plan

Smartpqi driver is tested and qualified by internal quality assurance team.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mav added a comment.Jul 3 2018, 5:03 PM

On a quick look I spotted number of style issues you may fix.

sys/dev/smartpqi/smartpqi_defines.h
909 ↗(On Diff #44630)

This looks like a null change, but if you wish so, so be it.

920 ↗(On Diff #44630)

Here would be good to add braces around rcb on the rigth side.

948 ↗(On Diff #44630)

Here should be tab after define. Also it may be not very good to change flags meaning without significant reason.

sys/dev/smartpqi/smartpqi_discovery.c
48 ↗(On Diff #44630)

Here extra tabs.

sys/dev/smartpqi/smartpqi_init.c
568 ↗(On Diff #44630)

Extra tabs.

579 ↗(On Diff #44630)

Something happened to formatting.

681 ↗(On Diff #44630)

FreeBSD style(9) recommends to wrap the line before the function name and before the opening curly bracket. And also some extra tabs.

762 ↗(On Diff #44630)

Here some mess with tabs/spaces.

sys/dev/smartpqi/smartpqi_mem.c
61 ↗(On Diff #44630)

Very "useful" function. ;)

Incorporated the changes according to review comments.

mav accepted this revision.Jul 9 2018, 8:18 PM

I have no objections.

This revision is now accepted and ready to land.Jul 9 2018, 8:18 PM
sbruno accepted this revision.Jul 11 2018, 4:42 PM
This revision was automatically updated to reflect the committed changes.
cem added a subscriber: cem.Jul 13 2018, 10:40 PM

Please stop adding redundant prototypes for the same function — it breaks the GCC build. The tagged lines below are not exhaustive.

head/sys/dev/smartpqi/smartpqi_prototypes.h
143

This one's got a dupe too

144

E.g., this is defined again 100 lines lower in the same header. Why???

145

Ditto.

241–242

We don't need both