In D58027#1342667, @mhorne wrote:

In D58027#1342622, @afscoelho_gmail.com wrote:

In D58027#1342399, @aokblast wrote:

Ok! I would like to confirm something with you before committing:

1. Author info: Andre Silva <andasilv@amd.com>
2. Sponsored by: Advanced Micro Devices (AMD)

Are the both desired information you expected?

Yes, they are. Thanks!

Sorry to interfere @aokblast, but I was ready to commit the whole series and just did so.

Ok! I would like to confirm something with you before committing:

In D55035#1341099, @bz wrote:

In D55035#1341066, @aokblast wrote:

In D55035#1341051, @bz wrote:

Thanks for the comment; I have a way larger branch I think than what is in review. Do you want to have a look at the entire bits? I'd be thrilled if someone will review them :) and more than happy to extract the next days.

Do you mean https://reviews.freebsd.org/D54987? I was typing something while you leave your comment:)

No, I mean more changes to be added to the stack; when I first did this it was a/multiple year(s) ago. I put some of that out earlier this year in order to get the de-coupling of LinuxKPI from the native stack structures out but that got into controversial discussions. In the follow-up I added more stuff for more USB wifi drivers ... which I never uploaded to Phab.

I'll go through what you have and would love to put a new stack up next week, if that's okay?

In D55035#1341051, @bz wrote:

Thanks for the comment; I have a way larger branch I think than what is in review. Do you want to have a look at the entire bits? I'd be thrilled if someone will review them :) and more than happy to extract the next days.

LGTM. Waiting for the feedback from @glebius :)

Plus these points, usb_unlink_urb_sub bypass unanchored in not started yet branch. Causing a forever run if the xfer hasn't start yet

hi, it is moved to D57759, right? I can commit that patch for you.

In D54218#1339119, @unitrunker_protonmail.com wrote:

usbconfig stopped reporting my webcam and fingerprint reader. Instead I get 'usbconfig: could not open device: Bad file descriptor'. Problem goes away when these changes are reverted. A very strange side note: libusb20_int.h is missing from base.txz for 15.1-RELEASE (but is in src.txz). I think the problem is I'm testing against 15.1-RELEASE which is too far behind current. I need to find some hardware onto which I can run current.

I would just abandon it as somebody has much more decent works.

LGTM. Sorry that I though it was copy and paste from line 633. It should work. Thanks!

Reflect @markj's comment

Also, we should use peek flag in native in here.

LGTM with the style change. I can commit this after @glebius accept.

In D58301#1337958, @jrtc27 wrote:

In D58301#1337950, @aokblast wrote:

In D58301#1337946, @jrtc27 wrote:

In D58301#1337933, @aokblast wrote:

In D58301#1337930, @jrtc27 wrote:

In D58301#1337928, @aokblast wrote:

In D58301#1337926, @jrtc27 wrote:

bus_read_8 has been replace to bus_space_read series in sys/bus.h.

Not sure what "bus_space_read series" is though, should that not be bus_space_read_8?

Sorry, it should be bus_space_{read,write} series.

But why series? I don't understand. Why not just use the _8 suffix everywhere?

The _8 suffix remains unchanged before and after my patch. The purpose of this change is to replace the original bus_ series with the bus_space_ series.

Ok, I think I now can decipher what you're trying to get at. What you're trying to say, I think, is something like the following?

bus_{read,write}_8 are macro wrappers around the corresponding bus_space functions in sys/bus.h, so implementing bus_{read,write}_8 won't work. Implement the underlying bus_space functions instead.

Yes, sorry about my bad english expression:)

No worries; normally these things are understandable even if they don't read as perfect English, but in this case for whatever reason I couldn't quite follow it :')

Might I suggest you use my interpretation as inspiration for amending your own message? (I mean, you can just copy the whole thing if you want, but you may prefer it to sound more like you!)

In D58301#1337946, @jrtc27 wrote:

In D58301#1337933, @aokblast wrote:

In D58301#1337930, @jrtc27 wrote:

In D58301#1337928, @aokblast wrote:

In D58301#1337926, @jrtc27 wrote:

bus_read_8 has been replace to bus_space_read series in sys/bus.h.

Not sure what "bus_space_read series" is though, should that not be bus_space_read_8?

Sorry, it should be bus_space_{read,write} series.

But why series? I don't understand. Why not just use the _8 suffix everywhere?

The _8 suffix remains unchanged before and after my patch. The purpose of this change is to replace the original bus_ series with the bus_space_ series.

Ok, I think I now can decipher what you're trying to get at. What you're trying to say, I think, is something like the following?

bus_{read,write}_8 are macro wrappers around the corresponding bus_space functions in sys/bus.h, so implementing bus_{read,write}_8 won't work. Implement the underlying bus_space functions instead.

Yes, sorry about my bad english expression:)

In D58301#1337930, @jrtc27 wrote:

In D58301#1337928, @aokblast wrote:

In D58301#1337926, @jrtc27 wrote:

bus_read_8 has been replace to bus_space_read series in sys/bus.h.

Not sure what "bus_space_read series" is though, should that not be bus_space_read_8?

Sorry, it should be bus_space_{read,write} series.

But why series? I don't understand. Why not just use the _8 suffix everywhere?

In D58301#1337926, @jrtc27 wrote:

bus_read_8 has been replace to bus_space_read series in sys/bus.h.

Not sure what "bus_space_read series" is though, should that not be bus_space_read_8?

Sorry, it should be bus_space_{read,write} series.

Also should probably be using read/write not just read everywhere in the commit message.

Have some patches upstream.

HI, my patch is only the library part. It needs extra work to become a driver.

In D58302#1337163, @kib wrote:

Could f6ee1d20d36813e990691 fixed your issue?

Anyway, the useful tool is gdb' pinfo /o type.

Reflect comments

In D58302#1337157, @kib wrote:

Why? p_asig and p_tree_refcount are at the end of the struct proc.

Remove useless patch

@hs.freebsd_id.omnilan.de I have sent you a mail to ask your desired author and mail few days ago. As others might want to compile on i386, I will use Harry Schmalzb. <freebsd@omnilan.de> and commit tomorrow.

In D57885#1335409, @dch wrote:

By default this is the permissions the log file is created with:

-rw-------  1 root wheel  153B Jul 13 23:35 buildkite.log

If this is what you intend to happen then let's commit this as is. If you're expecting it to be created readable by buildkite_account then it would need changing to have a precmd script.

Tested with yubikey.

In D51865#1334521, @kevans wrote:

In D51865#1334517, @adrian wrote:

hi!

So why are a whole lot of functions getting an fd added to them?

Do the FDs change during the runtime of the tool? Or are they something that could be set
once during some init phase (maybe in a new function which would call cap_enter, rather than
the owner having to call it) and then pulled out of some the libusb backend state?

IMO we really shouldn't have libraries entering capablity mode- libusb may be just one of many, and setting a precedence like that where some will insist on entering the sandbox is bound to end up with some really annoying circumstances.

In D51865#1334517, @adrian wrote:

hi!

So why are a whole lot of functions getting an fd added to them?

As we need to provide the require file descriptor that need to be opened before cap_enter.

Do the FDs change during the runtime of the tool? Or are they something that could be set
once during some init phase (maybe in a new function which would call cap_enter, rather than
the owner having to call it) and then pulled out of some the libusb backend state?

In D57885#1334360, @dch wrote:

Sigh, it's already set in L28... but if the file isn't present, it needs to be created with correct permissions, otherwise it won't be set. Can you add that please?

In D57885#1334350, @dch wrote:

• needs a PORTREVISION bump

In D52166#1330369, @adrian wrote:

Hey, any further traction on this? What's left before it can be landed?

Thanks for your review @bnovkov

Reflect bnovkov's review

LGTM!

Fixes

reflect olce@'s comment

Reflect olce@'s comment

In D57825#1333484, @olce wrote:

Looks good. One suggestion in an inline comment. Only thing now is the dependency on the CPPC lib, I'll review the other revisions and come back to here.

In D55474#1333882, @olce wrote:

Two more things to consider. First, you finally reduce items expected to be read-only to an integer (field value in struct acpi_cppc_rdonly_item), but some of these are dynamic and may change during runtime, in which case we have to keep the full register specification. Second, in the _CPC example I sent you offline for an AMD machine where processors do not report the CPPC extended feature flag, most present items have the PCC access method, so won't be covered by the code here. In the same example, there are only two FFH, for the ReferencePerformanceCounterRegister and DeliveredPerformanceCounterRegister, and the addresses seem to correspond to the MPERF and APERF MSR numbers respectively.

In D55474#1333742, @olce wrote:

As noted on previous revisions, SYS_RES_FFH should not exist, nor should acpi_ffh_write() and acpi_ffh_read(). So, basically, this needs a full rework.