Page MenuHomeFreeBSD

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

Authored by lakhanshiva_gmail.com on Jun 23 2018, 11:21 AM.

Details

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

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

  • Update PNP info to PCI attachment of ignore driver
  • Add PNP info to PCI attachment of ismt driver
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
180 ↗(On Diff #44343)

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

sys/dev/ips/ips_pci.c
63 ↗(On Diff #44343)

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 ↗(On Diff #44343)

Should this include D:#?

This revision now requires changes to proceed.Jun 24 2018, 12:36 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
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 ↗(On Diff #44343)

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

sys/dev/iir/iir_pci.c
172 ↗(On Diff #44343)

Same here.

sys/dev/ipw/if_ipw.c
205 ↗(On Diff #44343)

I'd add ;D# here as well.

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 edited the summary of this revision. (Show Details)
lakhanshiva_gmail.com marked an inline comment as done.
lakhanshiva_gmail.com edited the summary of this revision. (Show Details)
lakhanshiva_gmail.com marked an inline comment as done.
lakhanshiva_gmail.com edited the summary of this revision. (Show Details)
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 ↗(On Diff #44465)

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 ↗(On Diff #44343)

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 ↗(On Diff #44465)

style(9)

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
sys/dev/ips/ips_pci.c
63 ↗(On Diff #44343)

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 ↗(On Diff #44465)

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

sys/dev/iir/iir_pci.c
187 ↗(On Diff #44465)

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 ↗(On Diff #44343)

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

sys/dev/pci/isa_pci.c
156 ↗(On Diff #44465)

Understood, fixed it now!

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.

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
This revision was automatically updated to reflect the committed changes.