Page MenuHomeFreeBSD

ACPI support for USB , mainly hub.

Authored by takawata on Jun 13 2019, 3:41 PM.



ACPI supports USB related features.
This patch will do

  1. detects USB root hub if host controller has ACPI handle.
  2. Enumulate ACPI handles of USB ports and attach to device driver structure.
  3. Evaluate _UPC and _PLD objects under the ACPI objects of USB ports and parse them.

I think this patch is premature for merge, but I want to hear your opinion.

Diff Detail

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

Event Timeline

takawata created this revision.Jun 13 2019, 3:41 PM
imp added a comment.Jun 13 2019, 3:47 PM

I've not looked deeply into this, but it seems to my limited understanding in the right direction, though I'll defer to hps@ should his opinion differ.
Just noted a couple of nits, plus there's some minor style issues with some of the code that we can get into once other comments are made to the source.

8 ↗(On Diff #58589)

The project template now omits "all rights reserved," and it's preferred that new code not have that unless the author really wants it. It is your choice, though.


# This script should be run on every USB C- and H- file before
# commit to enforce correct style.

[ -z "$1" ] && (echo "Please specify a filename.") && exit

# The "-ta" option is an extension to indent.
# Contact to receive a
# patch that adds support for this option to
# indent !

# sed -E 's/&\(([^, \)]*)\)/\&\1/' |

for F in $*

echo "Now styling $F"

(cat $F | indent -Toss_mixerinfo -TFILE -Tu_char -Tu_int -Tu_long \
 -Tu_short -Tfd_set -ta -st -bad -bap -nbbb -nbc -br -nbs \
 -c41 -cd41 -cdb -ce -ci4 -cli0 -d0 -di8 -ndj -ei -nfc1 \
 -nfcb -i8 -ip8 -l79 -lc77 -ldi0 -nlp -npcs -psl -sc \
 -nsob -nv | 
 sed -e "s/_HEAD [(]/_HEAD(/g" |
 sed -e "s/_ENTRY [(]/_ENTRY(/g" |
 sed -e "s/     __packed/ __packed/g" | 
 sed -e "s/     __aligned/ __aligned/g" |
 sed -e "s/^#define /#define    /g") > temp

(diff temp $F > /dev/null) || (cp temp $F)


Can you pass the new files through the style script above?

^^^ only .c and .h files

hselasky added inline comments.Jun 13 2019, 4:22 PM
110 ↗(On Diff #58589)

remove this commented line

124 ↗(On Diff #58589)

/* Found */

153 ↗(On Diff #58589)

Spelling error Propritary

takawata updated this revision to Diff 58609.Jun 14 2019, 3:35 AM

Resolve comments.

hselasky added inline comments.Jun 14 2019, 9:55 AM
79 ↗(On Diff #58609)

Remove one empty line.

84 ↗(On Diff #58609)

Insert an empty line before static UINT32 ...

104 ↗(On Diff #58609)

uint8_t nports;

131 ↗(On Diff #58609)

Add empty line before static int

145 ↗(On Diff #58609)

Indent this line properly.

204 ↗(On Diff #58609)

Wrap long line.

252 ↗(On Diff #58609)

Can you move the curly bracket at the end of the line to the next line.

261 ↗(On Diff #58609)

Address is 64-bit? Use cast when comparing.

devinfo->Address <= (uint64_t)sc->nports)

262 ↗(On Diff #58609)

Looks off-by-one:
sc->porthandle[devinfo->Address - 1] = ah;

293 ↗(On Diff #58609)

/* success prior than non - acpi hub */

318 ↗(On Diff #58609)

Use device_printf() ??

347 ↗(On Diff #58609)

This print does not look needed.

414 ↗(On Diff #58609)

Remove this empty line.

425 ↗(On Diff #58609)

Remove this empty line.

446 ↗(On Diff #58609)

Remove one empty line here.

37 ↗(On Diff #58609)

Insert empty line before UHUB_INTR_INTERVAL.

Also use TAB after #define, before UHUB_INTR_INTERVAL.

42 ↗(On Diff #58609)

Will this build for all platforms? ARM, RISC, MIPS, SPARC ...

takawata updated this revision to Diff 58674.Jun 15 2019, 4:20 PM

Resolve comments.
Fix off-by-one error in acpi_usb_hub_port_probe_cb
Hide verbose outputs under debug flag.

hselasky added inline comments.Jun 17 2019, 6:55 PM
311 ↗(On Diff #58674)

Use /**/ comments.

hselasky accepted this revision.Jun 17 2019, 6:58 PM

Fix the last few comments I've added and this is ready to go.

This revision is now accepted and ready to land.Jun 17 2019, 6:58 PM
This revision was automatically updated to reflect the committed changes.