Re: [PATCH v2 1/2] it87: Add param to ignore ACPI resource conflicts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/10/2022 19:10, Guenter Roeck wrote:
On Sun, Oct 02, 2022 at 06:43:00PM +0100, Ahmad Khalifa wrote:
Add in the parameter to ignore ACPI resource conflicts so that modprobe
can use the force_id parameter when ACPI has the same address mapped.
force_id and ignore_resource_conflict are orthogonal / unrelated to each
other. Why create a dependency or correlation where none exists ?

The reason for introducing ignore_resource_conflict in the driver was that
some systems didn't like the system wide parameter (acpi_enforce_resources)
to ignore resource conflicts and failed to boot if it was set to lax
or disabled.

They're unrelated in their purpose, but adding ignore_resource_conflict creates an unusual situation that doesn't make it safe to use on its own. Because the driver registers two platform devices, the second one will start probing a random base address (0xFFF8).

It's not random though, because 0xFFFF & ~7 gets you there. But frankly, I don't know what normally lives there, so my initial opinion was that both changes should be a single commit to stop it87_find() from confirming that a device exists.

Just to be clear, currently, without the ignore_resource_conflict param, the driver just unregisters itself after the first of the 2 registers gives the ACPI conflict. So the issue of the second non-existant device, is not an issue.

Could separate the two patches completely, but ideally they're still together. What are your thoughts here?

The third parameter is the access permission. It should be 0 or maybe 0000.
Why "false" ?

My mistake, out of tree convention. Missed that the param right above it uses a 0 convention.

+	if (err){
Formatting: Space between ) and { required.

Will fix in v3.


--
Regards,
Ahmad



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux