Page MenuHomeFreeBSD

Add RTL8366SR support at etherswitch driver
ClosedPublic

Authored by yamori813_yahoo.co.jp on Jun 10 2016, 2:35 AM.

Details

Summary

Add RTL8366SR support at etherswitch driver

Test Plan

RTL8366RB and RTL8366SR

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

yamori813_yahoo.co.jp retitled this revision from to Add RTL8366SR support at etherswitch driver.
yamori813_yahoo.co.jp updated this object.
yamori813_yahoo.co.jp edited the test plan for this revision. (Show Details)
adrian added a subscriber: adrian.Jun 10 2016, 3:43 AM

it looks good! Let's just move chipid into the driver softc and then we're ready to commit it to -HEAD!

sys/dev/etherswitch/rtl8366/rtl8366rb.c
387 ↗(On Diff #17490)

Ideally chipid would be a flag in the softc, rather than a global. I know, it's highly unlikely that someone'll put more than one of these on a board. It's just the "better" way to do it.

ray requested changes to this revision.Jun 10 2016, 6:53 AM
ray added a reviewer: ray.
ray added a subscriber: ray.
ray added inline comments.
sys/dev/etherswitch/rtl8366/rtl8366rbvar.h
95 ↗(On Diff #17492)

As guy who worked with number of switch chips, I have to ask:
What will be here, when code will support 5 similar Ethernet switches?
(==1)?(1):(==2)?(2):(==3)?(3):(==4)?(4):(5)
IMO, better to name different registers with model prefix and cache registers address in softc, or even make submodules, which override default methods of driver, based on chip id.

This revision now requires changes to proceed.Jun 10 2016, 6:53 AM
yamori813_yahoo.co.jp edited edge metadata.
  • fixed non initialize soft use and softc initialze is first in function.
yamori813_yahoo.co.jp edited edge metadata.
  • modify by style(9) formant

hi,

the current diff isn't against head, it looks like it's against a previous diff :) can you upload a fresh diff against head?

that way I can see what needs to be tidied up before it's commited.

Thanks!

-adrian

Hi,

The latest diff is /still/ not against an un-modified freebsd-head tree. :-)

Sorry! Would you please diff your directory against the latest freebsd-head rather than your local repo and send up a diff?

Thanks!

-a

back to base and make diff

  • delete not use line
mizhka accepted this revision.Nov 15 2016, 4:29 PM
mizhka added a reviewer: mizhka.
mizhka added a subscriber: mizhka.

(*) Small improvements are welcome (@ray, @adrian comments)
(+) Patch is fine against current -HEAD
(+) Build has been compiled via freebsd-wifi-build/tl-wr1043nd

sys/dev/etherswitch/rtl8366/rtl8366rb.c
475–476 ↗(On Diff #18833)

Please use /* */ comments

sys/dev/etherswitch/rtl8366/rtl8366rbvar.h
164–173 ↗(On Diff #18833)

As @ray mentioned, could you please move condition to SC/HAL layer.

adrian accepted this revision.Nov 15 2016, 9:40 PM
adrian added a reviewer: adrian.

This looks fine to me. :)

Would someone be able to land this to -HEAD?

This revision was automatically updated to reflect the committed changes.