> -----Original Message-----
> From: Maximilian Luz <luzmaximilian@xxxxxxxxx>
> Sent: Wednesday, December 4, 2019 11:34 AM
> To: Kaneda, Erik <erik.kaneda@xxxxxxxxx>; Moore, Robert
> <robert.moore@xxxxxxxxx>; Wysocki, Rafael J <rafael.j.wysocki@xxxxxxxxx>
> Cc: linux-acpi@xxxxxxxxxxxxxxx; devel@xxxxxxxxxx
> Subject: Re: PROBLEM: Calling ObjectType on buffer field reports type integer
>
> On 12/3/19 11:21 PM, Kaneda, Erik wrote:
> > Hi Maximilian,
> >
> > Please try the patch below:
>
> Hi,
>
> I've tested this on 5.4.1 and the current upstream master now.
> Unfortunately it doesn't seem to fix the issue.
>
> Specifically, it seems like the flag doesn't get set here (printk-debugging
> indicates that the code in the if never runs):
>
> >
> >
> /******************************************************************
> ***
> > ********* diff -Nurp linux.before_name/drivers/acpi/acpica/dsutils.c
> > linux.after_name/drivers/acpi/acpica/dsutils.c
> > --- linux.before_name/drivers/acpi/acpica/dsutils.c 2019-12-03
> 13:36:21.782578718 -0800
> > +++ linux.after_name/drivers/acpi/acpica/dsutils.c 2019-12-03
> 13:36:17.765579110 -0800
> > @@ -470,6 +470,9 @@ acpi_ds_create_operand(struct acpi_walk_
> > ACPI_CAST_PTR(union acpi_operand_object,
> > walk_state->deferred_node);
> > status = AE_OK;
> > + if (walk_state->opcode == AML_CREATE_FIELD_OP) {
> > + obj_desc->buffer_field.is_create_field = TRUE;
> > + }
> > } else { /* All other opcodes */
> >
> > /*
>
> That of course means that the if condition here is still not fulfilled, so the
> problem is not fixed.
>
> > diff -Nurp linux.before_name/drivers/acpi/acpica/exfield.c
> linux.after_name/drivers/acpi/acpica/exfield.c
> > --- linux.before_name/drivers/acpi/acpica/exfield.c 2019-12-03
> 13:36:21.791578717 -0800
> > +++ linux.after_name/drivers/acpi/acpica/exfield.c 2019-12-03
> 13:36:18.257579062 -0800
> > @@ -153,12 +154,17 @@ acpi_ex_read_data_from_field(struct acpi
> > * the use of arithmetic operators on the returned value if the
> > * field size is equal or smaller than an Integer.
> > *
> > + * However, all buffer fields created by create_field operator needs to
> > + * remain as a buffer to match other AML interpreter implementations.
> > + *
> > * Note: Field.length is in bits.
> > */
> > buffer_length =
> >
> > (acpi_size)ACPI_ROUND_BITS_UP_TO_BYTES(obj_desc->field.bit_length);
> >
> > - if (buffer_length > acpi_gbl_integer_byte_width) {
> > + if (buffer_length > acpi_gbl_integer_byte_width ||
> > + (obj_desc->common.type == ACPI_TYPE_BUFFER_FIELD &&
> > + obj_desc->buffer_field.is_create_field)) {
> >
> > /* Field is too large for an Integer, create a Buffer instead */
> >
>
> If I'm not mistaken I've proposed a patch here that is conceptually quite similar
> (but may have some flaws or cause other issues, as I'm not really familiar with
> the ACPICA code-base). My idea was to modify acpi_ds_init_buffer_field
> (dsopcode.c) instead of acpi_ds_create_operand. As said, I'm not sure however
> if this may have any other consequences that I'm unaware of.
>
> Also I've avoided creating an is_create_field flag by changing the field access
> from AML_FIELD_ACCESS_BYTE to AML_FIELD_ACCESS_BUFFER (which doesn't
> seem to be used anywhere else) and checking that, but the general idea of
> extending the if condition in acpi_ex_read_data_from_field is the same.
>
> Link to the patch:
> https://github.com/qzed/linux-surfacegen5-
> acpi/blob/2014964bac52f138109443de3e92dc2c6cff5e83/patches/5.3/0001-
> ACPI-Fix-buffer-integer-type-mismatch.patch
Your solution looks very similar to my patch.
In AcpiDsInitBufferField, I would prefer to use a new field "is_create_field" or something like that. AML_FIELD_ACCESS_BUFFER
does get used in other parts of ACPICA so let's create and use a new field in the object union.
Go ahead and re-submit the patch with the above changes. I'll edit your explanation with more details and include this in
the next ACPICA release. It'll land in linux after the release.
Thanks!
Erik
>
> Thanks,
> Maximilian
