On Thu, Jun 22, 2017 at 11:06:55AM +0300, Mika Westerberg wrote: > On Wed, Jun 21, 2017 at 08:05:53PM +0200, Lukas Wunner wrote: > > + props = acpi_evaluate_dsm_typed(adev->handle, apple_prp_uuid, 1, 0, > > + NULL, ACPI_TYPE_BUFFER); > > + if (!props || !props->buffer.length) > > + goto out_free; > > + > > + version = props->buffer.pointer[0]; > > + ACPI_FREE(props); > > + if (version != 3) { > > + acpi_handle_err(adev->handle, > > + "unsupported properties version %u\n", version); > > I don't think this is error. More like debug if anything. It would be good to be able to google for this message to detect if there are ever Macs out there which return something different than "3" to this _DSM call. How about KERN_WARN or KERN_INFO? The AppleACPIPlatform.kext which shipped with 10.4.11 (2007) already checked for a return value of 3 and the newest Macs still return "3", so this has never changed in at least 10 years and the whole issue is thus theoretical. I just wanted to be compatible to what macOS does, they check for "3" and abort if the return value is different. > > + if (skipped) > > + acpi_handle_err(adev->handle, > > + "skipped %u properties: wrong type\n", skipped); > > Same here. This would be a firmware bug. We've got FW_BUG, FW_WARN, FW_INFO defined in include/linux/printk.h. Granted this is not high priority, but FW_WARN or at least FW_INFO would seem to be justified. > > + WARN_ON(free_space != (void *)newprops + newsize); > > Again, there is not need to scare the user if we can't parse these. We > can log an error here and give up but definitely no need to trigger > backtrace and register dump. This is not for parse errors but programming errors. free_space is a pointer into the newprops allocation. If (free_space < newprops + newsize) then the allocation was too large and we're wasting memory. If (free_space > newprops + newsize) then the allocation was too small and we've corrupted memory behind the allocation. I'm fairly sure the WARN_ON will never trigger but what do I know? :-) Thanks! Lukas -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html