Thank you Maxim for the review

Please see: D56118.

In D55857#1283609, @kib wrote:

In D55857#1283607, @cperciva wrote:

So... it's not an easy fix and definitely not an API change I can make quickly, and I don't want to hold up "better" just because we can't do "perfect" yet.

Sometimes even a promise would do.

In D55857#1283607, @cperciva wrote:

In D55857#1283545, @kib wrote:

I still think that the error should somehow propagate up. With this series, the device is malfunctioning, with the only hint about the reason being the single cryptic line in the dmesg, which is not even attributed cleanly to the device.
It is better to fail the driver attach in this case than to offer such failure mode.

I agree in theory, but a lot of functions would need to change from (void) to (int) to do this. And there's some weird cases like what should ioapic_resume do if one of its ioapic_program_intpin calls fails. (Also, we need to program the interrupt to go nowhere rather than returning without programming it -- otherwise if an interrupt is being *re*programmed we get a panic when interrupts keep arriving at a CPU which no longer knows what to do with them.)

So... it's not an easy fix and definitely not an API change I can make quickly, and I don't want to hold up "better" just because we can't do "perfect" yet.

In D55857#1283545, @kib wrote:

I still think that the error should somehow propagate up. With this series, the device is malfunctioning, with the only hint about the reason being the single cryptic line in the dmesg, which is not even attributed cleanly to the device.
It is better to fail the driver attach in this case than to offer such failure mode.

Print intpin when warning about unsupporting APIC IDs

Fix a few build issues

• Fail attach if unit > 0
• Change device description

committed

In D56062#1283586, @obiwac wrote:

which leaves a lot to be desired

Yes, I still have no clue how I'm supposed to parse their description 😄

I think you can add to acpi_spmc_probe() a test about whether devclass_get_count() is non-zero, in which case you return ENXIO. This would allow manually detaching acpi_spmc and reattaching it "by hand" using devctl(8), if there's ever a machine with two PNP0D80 devices one day, and only one is functional...

Tried this out but devclass_get_count() gives 1 in the probe before anything was attached. i will find the time to investigate this soon

which leaves a lot to be desired

• Don't allow non-largepage shift mask
• Try to massage the manpage a bit more

Bump the iteration limit. Request review.

Updated diff to add a new ioctl for page-bank operations.
Changed the 2 drivers that might use page/bank (iflib, mce) to accept the new ioctl and to ensure they are not reading garbage page/bank fields when operating with the legacy SIOGI2C ioctl

In D55912#1282432, @kib wrote:

I mean something like this (ignore naming), in the generic ioctl handler, not in the driver code;

case SIOCGI2C_OLD:
     i2c.page = i2c.bank = 0;
     ioctl(SIOCG2C, &i2c);
     break;

I.e. drivers only need to implement the new ioctl that knows about bank/page.

Or put it differently, your original patch for mlx5en then would be correct, with this addition to the net/, instead of requiring each driver to have code to handle both ioctls.

This looks a lot better. In addition to the inline comments, I have two general ones:

Abandoning this differential revision.

Should I reroll the diff for UPDATING?
There are already several ports adding UPDATING entries and causing merge conflicts.

Probe is called a second time if it didn't return 0. Acpi does add all the children. Identify is rarely needed with acpi

I still think that the error should somehow propagate up. With this series, the device is malfunctioning, with the only hint about the reason being the single cryptic line in the dmesg, which is not even attributed cleanly to the device.
It is better to fail the driver attach in this case than to offer such failure mode.

Typo in the message: OPNOTSUPP

Could use @imp eyes on this later but looks pretty straightforward.

implemented the suggestions of bms from the previous review
and improved error handling to use the correct macro

I will try new patch with axe and axge and report it. Thanks!!

In D56117#1283524, @ivy wrote:

that sounds reasonable. note this blocks D56087, so i'm interested in the result.

that sounds reasonable. note this blocks D56087, so i'm interested in the result.

Give me some time to test it, but I think it is better if we just update share/mk/src.opts.mk (and regenerate the manual page afterwards):

.if ${MK_BLOCKLIST} == "no"
MK_BLACKLIST:=	no
.endif

Comments inline. +10 points for joining us on Phabricator.

Looks good overall. Based on Microsoft's documentation, which leaves a lot to be desired, I would have called DSM_MODERN_TURN_ON_DISPLAY before DSM_MODERN_EXIT_NOTIF exactly as you're doing.