Page MenuHomeFreeBSD
Differential D15979

Add PNP info to PCI attachment of et, iir, ignore, isab, ips, ipw, ismt, iwm, ix, ixgb, ixv drivers
ClosedPublic
Actions

Authored by lakhanshiva_gmail.com on Jun 23 2018, 11:21 AM.
Tags
None
Referenced Files
F103777976: D15979.diff
Fri, Nov 29, 7:42 AM
Unknown Object (File)
Tue, Nov 26, 7:41 AM
Unknown Object (File)
Tue, Nov 26, 7:36 AM
Unknown Object (File)
Tue, Nov 26, 7:35 AM
Unknown Object (File)
Tue, Nov 26, 5:36 AM
Unknown Object (File)
Sun, Nov 24, 1:01 AM
Unknown Object (File)
Mon, Nov 18, 8:39 PM
Unknown Object (File)
Mon, Nov 18, 12:25 PM
View All Files
Subscribers
ae

Details

Reviewers
imp
chuck
sbruno
Commits
rS336110: Add PNP info to PCI attachment of et driver
rS336109: Add PNP info to PCI attachment of ipw driver
rS336108: Add PNP info to PCI attachment of ixv driver
rS336107: Add PNP info to PCI attachment of ix driver
Summary

Update PNP info to PCI attachment of ismt driver
Update PNP info to PCI attachment of ips driver
Update PNP info to PCI attachment of iir driver
Update PNP info to PCI attachment of isab driver

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17712
Build 17514: arc lint + arc unit

Event Timeline

lakhanshiva_gmail.com created this revision.Jun 23 2018, 11:21 AM
Herald added a reviewer: sbruno. · View Herald TranscriptJun 23 2018, 11:21 AM
Herald added a subscriber: ae. · View Herald Transcript
Harbormaster completed remote builds in B17560: Diff 44341.Jun 23 2018, 11:22 AM
lakhanshiva_gmail.com updated this revision to Diff 44342.Jun 23 2018, 11:45 AM
  • Update PNP info to PCI attachment of ignore driver
Harbormaster completed remote builds in B17561: Diff 44342.Jun 23 2018, 11:45 AM
lakhanshiva_gmail.com updated this revision to Diff 44343.Jun 23 2018, 12:07 PM
  • Add PNP info to PCI attachment of ismt driver
Harbormaster completed remote builds in B17562: Diff 44343.Jun 23 2018, 12:07 PM
lakhanshiva_gmail.com retitled this revision from Add PNP info to PCI attachment of et, iir, ignore, isab, ips, ipw, iwm, ix, ixgb, ixv drivers to Add PNP info to PCI attachment of et, iir, ignore, isab, ips, ipw, ismt, iwm, ix, ixgb, ixv drivers.Jun 23 2018, 12:08 PM
lakhanshiva_gmail.com edited the summary of this revision. (Show Details)
chuck requested changes to this revision.Jun 24 2018, 12:36 AM
chuck added inline comments.
sys/dev/iir/iir_pci.c
179–180

Now that the driver has a device table, should probe be modified to use it?

sys/dev/ips/ips_pci.c
63

style(9) nits including:

  • Unary operators do not require spaces, binary operators do.
  • Indentation is an 8 character tab.
  • Blank line between variable declaration and code.
  • Space between for and open paren and between close paren and brace
235

Should this include D:#?

This revision now requires changes to proceed.Jun 24 2018, 12:36 AM
lakhanshiva_gmail.com updated this revision to Diff 44384.Jun 24 2018, 10:52 AM
  • Add PNP info to PCI attachment of isab driver
  • Add PNP info to PCI attachment of ignore driver
  • Add PNP info to PCI attachment of ix driver
  • Add PNP info to PCI attachment of ixv driver
  • Add PNP info to PCI attachment of ixgb driver
  • Add PNP info to PCI attachment of ips driver
  • Add PNP info to PCI attachment of iwm driver
  • Add PNP info to PCI attachment of ipw driver
  • Add PNP info to PCI attachment of iir driver
  • Add PNP info to PCI attachment of et driver
  • Add PNP info to PCI attachment of ismt driver
Harbormaster completed remote builds in B17589: Diff 44384.Jun 24 2018, 10:52 AM
imp added a comment.Jun 24 2018, 1:51 PM

I think we should consider doing a pci_lookup() routine. It would make a lot of this simpler...

sys/dev/et/if_et.c
191

I may have suggested this already, but a adding ;D# here would be good.

sys/dev/iir/iir_pci.c
172

Same here.

sys/dev/ipw/if_ipw.c
205

I'd add ;D# here as well.

lakhanshiva_gmail.com added a comment.Jun 24 2018, 1:57 PM

I have added a ;D:# wherever needed in this differential review. I was having a problem since yesterday with updating the diff review using arcanist. It is stuck at a step - >>> [18] <http> https://reviews.freebsd.org/api/differential.querydiffs Sometimes it works and sometimes it doesn't and i just got it to work a couple of hours back. I am up for a discussion tomorrow on pci_lookup() routine.

lakhanshiva_gmail.com marked 4 inline comments as done.Jun 24 2018, 3:18 PM
lakhanshiva_gmail.com updated this revision to Diff 44459.Jun 26 2018, 12:29 PM
lakhanshiva_gmail.com edited the summary of this revision. (Show Details)
lakhanshiva_gmail.com marked an inline comment as done.
lakhanshiva_gmail.com updated this revision to Diff 44460.Jun 26 2018, 12:50 PM
lakhanshiva_gmail.com edited the summary of this revision. (Show Details)
lakhanshiva_gmail.com updated this revision to Diff 44461.Jun 26 2018, 1:01 PM
lakhanshiva_gmail.com marked an inline comment as done.
lakhanshiva_gmail.com edited the summary of this revision. (Show Details)
lakhanshiva_gmail.com updated this revision to Diff 44465.Jun 26 2018, 1:49 PM
lakhanshiva_gmail.com edited the summary of this revision. (Show Details)
chuck added inline comments.Jun 28 2018, 9:42 PM
sys/dev/iir/iir_pci.c
187

I understand why this if-else is necessary, but that same necessity makes me wonder if a table lookup for this device class is "wrong". If it was possible to encode this as a part of PNP (can you combine L16 and G16?) maybe this would make more sense.

sys/dev/ips/ips_pci.c
63

Better, but for() should be:
for (i = 0; i < nitems(ips_pci_devs) - 1; i++) {
Fix here and elsewhere.

sys/dev/pci/isa_pci.c
156

style(9)

lakhanshiva_gmail.com updated this revision to Diff 44612.Jun 29 2018, 12:53 AM
lakhanshiva_gmail.com marked 2 inline comments as done.
lakhanshiva_gmail.com edited the summary of this revision. (Show Details)
  • Update PNP info to PCI attachment of isab driver
  • Update PNP info to PCI attachment of ips driver
  • Update PNP info to PCI attachment of ismt driver
Harbormaster completed remote builds in B17712: Diff 44612.Jun 29 2018, 12:53 AM
lakhanshiva_gmail.com added inline comments.Jun 29 2018, 12:54 AM
sys/dev/ips/ips_pci.c
63

This is how i can see it. "for (i = 0; i < nitems(ips_pci_devs) - 1; i++) {"
Is it for some reason not showing you the latest code ?
However, there is no space between if and left parentheses which i fixed now.

sys/dev/pci/isa_pci.c
156

for looks fine to me.
Between if and left parenthseses there was no space, which i fixed now.

lakhanshiva_gmail.com added inline comments.Jun 29 2018, 1:11 AM
sys/dev/iir/iir_pci.c
187

I need some help from @imp to use L16 and G16 correctly.

lakhanshiva_gmail.com marked 2 inline comments as done.Jun 29 2018, 2:55 AM
lakhanshiva_gmail.com added inline comments.
sys/dev/ips/ips_pci.c
63

Understood, the space between i and nitems. My bad ! Fixed it.

sys/dev/pci/isa_pci.c
156

Understood, fixed it now!

lakhanshiva_gmail.com updated this revision to Diff 44617.Jun 29 2018, 3:03 AM
lakhanshiva_gmail.com marked 2 inline comments as done.
lakhanshiva_gmail.com edited the summary of this revision. (Show Details)
lakhanshiva_gmail.com marked 2 inline comments as done.
lakhanshiva_gmail.com updated this revision to Diff 44717.Jul 1 2018, 11:23 AM
lakhanshiva_gmail.com added a comment.Jul 4 2018, 8:51 AM

Converted spaces to tabs (inside probe functions)

This revision was not accepted when it landed; it landed in state Needs Review.Jul 8 2018, 8:40 PM
Closed by commit rS336107: Add PNP info to PCI attachment of ix driver (authored by imp). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
sys/
dev/
et/
2 lines
iir/
43 lines
ips/
41 lines
ipw/
2 lines
ismt/
41 lines
iwm/
28 lines
ixgb/
3 lines
ixgbe/
2 lines
2 lines
pci/
9 lines
73 lines

Diff 44612

View Options

sys/dev/et/if_et.c

Loading...
View Options

sys/dev/iir/iir_pci.c

Loading...
View Options

sys/dev/ips/ips_pci.c

Loading...
View Options

sys/dev/ipw/if_ipw.c

Loading...
View Options

sys/dev/ismt/ismt.c

Loading...
View Options

sys/dev/iwm/if_iwm.c

Loading...
View Options

sys/dev/ixgb/if_ixgb.c

Loading...
View Options

sys/dev/ixgbe/if_ix.c

Loading...
View Options

sys/dev/ixgbe/if_ixv.c

Loading...
View Options

sys/dev/pci/ignore_pci.c

Loading...
View Options

sys/dev/pci/isa_pci.c

Loading...