Re: [PATCH v3 1/5] ACPI / blacklist: add acpi_match_platform_list()
From: Rafael J. Wysocki
Date: Mon Aug 21 2017 - 16:31:32 EST
On Mon, Aug 21, 2017 at 7:36 PM, Borislav Petkov <bp@xxxxxxxxx> wrote:
> On Mon, Aug 21, 2017 at 05:23:37PM +0000, Kani, Toshimitsu wrote:
>> > > 'data' here is private to the caller. So, I do not think we need
>> > > to define the bits. Shall I change the name to 'driver_data' to
>> > > make it more explicit?
>> >
>> > You changed it to 'data'. It was a u32-used-as-boolean
>> > is_critical_error before.
>> >
>> > So you can just as well make it into flags and people can extend
>> > those flags if needed. A flag bit should be enough in most cases
>> > anyway. If they really need driver_data, then they can add a void *
>> > member.
>>
>> Hmm.. In patch 2, intel_pstate_platform_pwr_mgmt_exists() uses this
>> field for PSS and PCC, which are enum values. I think we should allow
>> drivers to set any values here. I agree that it may need to be void *
>> if we also allow drivers to set a pointer here.
>
> Let's see what Rafael prefers.
I would retain the is_critical_error field and use that for printing
the recoverable / non-recoverable message. This is kind of orthogonal
to whether or not any extra data is needed and that can be an
additional field. In that case unsigned long should be sufficient to
accommodate a pointer if need be.
Thanks,
Rafael