Page MenuHomeFreeBSD
Differential D6796

Add RTL8366SR support at etherswitch driver
ClosedPublic
Actions

Authored by yamori813_yahoo.co.jp on Jun 10 2016, 2:35 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jan 24, 12:14 AM
Unknown Object (File)
Sun, Jan 19, 11:10 AM
Unknown Object (File)
Jan 17 2025, 11:49 PM
Unknown Object (File)
Jan 15 2025, 1:36 PM
Unknown Object (File)
Jan 1 2025, 3:53 PM
Unknown Object (File)
Dec 28 2024, 10:54 PM
Unknown Object (File)
Dec 25 2024, 10:11 PM
Unknown Object (File)
Dec 25 2024, 6:03 AM
View All Files
Subscribers
adrian
imp
mizhka
ray
Contributor Reviews (src)

Details

Reviewers
ray
mizhka
adrian
Commits
rS315330: MFC r308700:
rS308700: [etherswitch] add RTL8366SR support
Summary

Add RTL8366SR support at etherswitch driver

Test Plan

RTL8366RB and RTL8366SR

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

yamori813_yahoo.co.jp updated this revision to Diff 17490.Jun 10 2016, 2:35 AM
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)
Herald added subscribers: Contributor Reviews (src), imp. · View Herald TranscriptJun 10 2016, 2:35 AM
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.

yamori813_yahoo.co.jp updated this revision to Diff 17492.Jun 10 2016, 6:00 AM

refict review

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 updated this revision to Diff 17788.Jun 23 2016, 3:28 AM
yamori813_yahoo.co.jp edited edge metadata.
  • fixed non initialize soft use and softc initialze is first in function.
yamori813_yahoo.co.jp updated this revision to Diff 17845.Jun 24 2016, 1:55 AM
yamori813_yahoo.co.jp edited edge metadata.
  • modify by style(9) formant
adrian added a comment.Jul 25 2016, 5:17 AM

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

yamori813_yahoo.co.jp updated this revision to Diff 18745.Jul 26 2016, 1:13 AM
  • update diff
adrian added a comment.Jul 26 2016, 4:43 PM

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

yamori813_yahoo.co.jp updated this revision to Diff 18781.Jul 27 2016, 12:15 AM

back to base and make diff

yamori813_yahoo.co.jp updated this revision to Diff 18833.Jul 28 2016, 2:06 AM
  • 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?

Closed by commit rS308700: [etherswitch] add RTL8366SR support (authored by mizhka). · Explain WhyNov 15 2016, 9:58 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
head/
sys/
conf/
3 lines
dev/
etherswitch/
rtl8366/
364 lines
193 lines

Diff 22234

View Options

head/sys/conf/options

Loading...
View Options

head/sys/dev/etherswitch/rtl8366/rtl8366rb.c

Loading...
View Options

head/sys/dev/etherswitch/rtl8366/rtl8366rbvar.h

Loading...