| Message ID | 20220616090106.2154906-2-sudeep.holla@arm.com |
|---|---|
| State | Accepted |
| Commit | ee64b827a9af7905cb8b84d882320ecc91640192 |
| Headers | show |
| Series | ACPI: Implement FFH OpRegion support | expand |
On Thu, Jun 16, 2022 at 11:01 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > FFH(Fixed Function Hardware) Opregion is approved to be added in ACPIC 6.5 via s/ACPIC/ACPI/ > code first approach[1]. It requires special context data similar to GPIO and > Generic Serial Bus as it needs to know platform specific offset and length. > > Add support for the special context data needed by FFH Opregion. > > FFH OpRegion enables advanced use of FFH on some architectures. For example, > it could be used to easily proxy AML code to architecture-specific behavior > (to ensure it is OS initiated) > > Actual behavior of FFH is ofcourse architecture specific and depends on > the FFH bindings. The offset and length could have arch specific meaning > or usage. > > [1] https://bugzilla.tianocore.org/show_bug.cgi?id=3598 > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> This looks reasonable to me and I see that you've already submitted a pull request to the upstream ACPICA. > --- > drivers/acpi/acpica/evregion.c | 8 ++++++++ > drivers/acpi/acpica/exfield.c | 8 ++++++-- > drivers/acpi/acpica/exserial.c | 6 ++++++ > include/acpi/acconfig.h | 2 ++ > include/acpi/actypes.h | 7 +++++++ > 5 files changed, 29 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/acpica/evregion.c b/drivers/acpi/acpica/evregion.c > index b96b3a7e78e5..d5aed3249630 100644 > --- a/drivers/acpi/acpica/evregion.c > +++ b/drivers/acpi/acpica/evregion.c > @@ -172,6 +172,14 @@ acpi_ev_address_space_dispatch(union acpi_operand_object *region_obj, > ctx->subspace_id = (u8)region_obj->region.address; > } > > + if (region_obj->region.space_id == > + ACPI_ADR_SPACE_FIXED_HARDWARE) { > + struct acpi_ffh_info *ctx = > + handler_desc->address_space.context; > + > + ctx->offset = region_obj->region.address; > + ctx->length = region_obj->region.length; > + } > /* > * We must exit the interpreter because the region setup will > * potentially execute control methods (for example, the _REG method > diff --git a/drivers/acpi/acpica/exfield.c b/drivers/acpi/acpica/exfield.c > index 2b89a496de65..657f4002f9dc 100644 > --- a/drivers/acpi/acpica/exfield.c > +++ b/drivers/acpi/acpica/exfield.c > @@ -141,7 +141,9 @@ acpi_ex_read_data_from_field(struct acpi_walk_state *walk_state, > || obj_desc->field.region_obj->region.space_id == > ACPI_ADR_SPACE_IPMI > || obj_desc->field.region_obj->region.space_id == > - ACPI_ADR_SPACE_PLATFORM_RT)) { > + ACPI_ADR_SPACE_PLATFORM_RT > + || obj_desc->field.region_obj->region.space_id == > + ACPI_ADR_SPACE_FIXED_HARDWARE)) { > > /* SMBus, GSBus, IPMI serial */ > > @@ -305,7 +307,9 @@ acpi_ex_write_data_to_field(union acpi_operand_object *source_desc, > || obj_desc->field.region_obj->region.space_id == > ACPI_ADR_SPACE_IPMI > || obj_desc->field.region_obj->region.space_id == > - ACPI_ADR_SPACE_PLATFORM_RT)) { > + ACPI_ADR_SPACE_PLATFORM_RT > + || obj_desc->field.region_obj->region.space_id == > + ACPI_ADR_SPACE_FIXED_HARDWARE)) { > > /* SMBus, GSBus, IPMI serial */ > > diff --git a/drivers/acpi/acpica/exserial.c b/drivers/acpi/acpica/exserial.c > index 4da20d7845df..fd63f2042514 100644 > --- a/drivers/acpi/acpica/exserial.c > +++ b/drivers/acpi/acpica/exserial.c > @@ -323,6 +323,12 @@ acpi_ex_write_serial_bus(union acpi_operand_object *source_desc, > function = ACPI_WRITE; > break; > > + case ACPI_ADR_SPACE_FIXED_HARDWARE: > + > + buffer_length = ACPI_FFH_INPUT_BUFFER_SIZE; > + function = ACPI_WRITE; > + break; > + > default: > return_ACPI_STATUS(AE_AML_INVALID_SPACE_ID); > } > diff --git a/include/acpi/acconfig.h b/include/acpi/acconfig.h > index c3ae3ea88e17..02e57191c1fd 100644 > --- a/include/acpi/acconfig.h > +++ b/include/acpi/acconfig.h > @@ -190,6 +190,8 @@ > > #define ACPI_PRM_INPUT_BUFFER_SIZE 26 > > +#define ACPI_FFH_INPUT_BUFFER_SIZE 144 > + > /* _sx_d and _sx_w control methods */ > > #define ACPI_NUM_sx_d_METHODS 4 > diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h > index 3491e454b2ab..1b4f81f1ac5d 100644 > --- a/include/acpi/actypes.h > +++ b/include/acpi/actypes.h > @@ -1115,6 +1115,13 @@ struct acpi_pcc_info { > u8 *internal_buffer; > }; > > +/* Special Context data for FFH Opregion (ACPI 6.5) */ > + > +struct acpi_ffh_info { > + u64 offset; > + u64 length; > +}; > + > typedef > acpi_status (*acpi_adr_space_setup) (acpi_handle region_handle, > u32 function, > -- > 2.36.1 >
On Wed, Jun 22, 2022 at 02:50:08PM +0200, Rafael J. Wysocki wrote: > On Thu, Jun 16, 2022 at 11:01 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > FFH(Fixed Function Hardware) Opregion is approved to be added in ACPIC 6.5 via > > s/ACPIC/ACPI/ > Fixed and pushed in ACPICA PR. > > code first approach[1]. It requires special context data similar to GPIO and > > Generic Serial Bus as it needs to know platform specific offset and length. > > > > Add support for the special context data needed by FFH Opregion. > > > > FFH OpRegion enables advanced use of FFH on some architectures. For example, > > it could be used to easily proxy AML code to architecture-specific behavior > > (to ensure it is OS initiated) > > > > Actual behavior of FFH is ofcourse architecture specific and depends on > > the FFH bindings. The offset and length could have arch specific meaning > > or usage. > > > > [1] https://bugzilla.tianocore.org/show_bug.cgi?id=3598 > > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > This looks reasonable to me and I see that you've already submitted a > pull request to the upstream ACPICA. I assume you would prefer me to post the other 2 patches once this lands in your -next. Worst case I would like to get the generic patch in along with ACPICA changes, I can then route the arm64 specific next cycle if it gets too late for v5.20
On Wed, Jun 22, 2022 at 4:01 PM Sudeep Holla <sudeep.holla@arm.com> wrote: > > On Wed, Jun 22, 2022 at 02:50:08PM +0200, Rafael J. Wysocki wrote: > > On Thu, Jun 16, 2022 at 11:01 AM Sudeep Holla <sudeep.holla@arm.com> wrote: > > > > > > FFH(Fixed Function Hardware) Opregion is approved to be added in ACPIC 6.5 via > > > > s/ACPIC/ACPI/ > > > > Fixed and pushed in ACPICA PR. > > > > code first approach[1]. It requires special context data similar to GPIO and > > > Generic Serial Bus as it needs to know platform specific offset and length. > > > > > > Add support for the special context data needed by FFH Opregion. > > > > > > FFH OpRegion enables advanced use of FFH on some architectures. For example, > > > it could be used to easily proxy AML code to architecture-specific behavior > > > (to ensure it is OS initiated) > > > > > > Actual behavior of FFH is ofcourse architecture specific and depends on > > > the FFH bindings. The offset and length could have arch specific meaning > > > or usage. > > > > > > [1] https://bugzilla.tianocore.org/show_bug.cgi?id=3598 > > > > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > > > This looks reasonable to me and I see that you've already submitted a > > pull request to the upstream ACPICA. > > I assume you would prefer me to post the other 2 patches once this lands > in your -next. That would be ideal, but technically I can apply an ACPICA patch in advance once the corresponding pull request has been integrated upstream. > Worst case I would like to get the generic patch in along > with ACPICA changes, I can then route the arm64 specific next cycle if it > gets too late for v5.20 Let's try the normal flow and worry about workarounds if it gets late.
diff --git a/drivers/acpi/acpica/evregion.c b/drivers/acpi/acpica/evregion.c index b96b3a7e78e5..d5aed3249630 100644 --- a/drivers/acpi/acpica/evregion.c +++ b/drivers/acpi/acpica/evregion.c @@ -172,6 +172,14 @@ acpi_ev_address_space_dispatch(union acpi_operand_object *region_obj, ctx->subspace_id = (u8)region_obj->region.address; } + if (region_obj->region.space_id == + ACPI_ADR_SPACE_FIXED_HARDWARE) { + struct acpi_ffh_info *ctx = + handler_desc->address_space.context; + + ctx->offset = region_obj->region.address; + ctx->length = region_obj->region.length; + } /* * We must exit the interpreter because the region setup will * potentially execute control methods (for example, the _REG method diff --git a/drivers/acpi/acpica/exfield.c b/drivers/acpi/acpica/exfield.c index 2b89a496de65..657f4002f9dc 100644 --- a/drivers/acpi/acpica/exfield.c +++ b/drivers/acpi/acpica/exfield.c @@ -141,7 +141,9 @@ acpi_ex_read_data_from_field(struct acpi_walk_state *walk_state, || obj_desc->field.region_obj->region.space_id == ACPI_ADR_SPACE_IPMI || obj_desc->field.region_obj->region.space_id == - ACPI_ADR_SPACE_PLATFORM_RT)) { + ACPI_ADR_SPACE_PLATFORM_RT + || obj_desc->field.region_obj->region.space_id == + ACPI_ADR_SPACE_FIXED_HARDWARE)) { /* SMBus, GSBus, IPMI serial */ @@ -305,7 +307,9 @@ acpi_ex_write_data_to_field(union acpi_operand_object *source_desc, || obj_desc->field.region_obj->region.space_id == ACPI_ADR_SPACE_IPMI || obj_desc->field.region_obj->region.space_id == - ACPI_ADR_SPACE_PLATFORM_RT)) { + ACPI_ADR_SPACE_PLATFORM_RT + || obj_desc->field.region_obj->region.space_id == + ACPI_ADR_SPACE_FIXED_HARDWARE)) { /* SMBus, GSBus, IPMI serial */ diff --git a/drivers/acpi/acpica/exserial.c b/drivers/acpi/acpica/exserial.c index 4da20d7845df..fd63f2042514 100644 --- a/drivers/acpi/acpica/exserial.c +++ b/drivers/acpi/acpica/exserial.c @@ -323,6 +323,12 @@ acpi_ex_write_serial_bus(union acpi_operand_object *source_desc, function = ACPI_WRITE; break; + case ACPI_ADR_SPACE_FIXED_HARDWARE: + + buffer_length = ACPI_FFH_INPUT_BUFFER_SIZE; + function = ACPI_WRITE; + break; + default: return_ACPI_STATUS(AE_AML_INVALID_SPACE_ID); } diff --git a/include/acpi/acconfig.h b/include/acpi/acconfig.h index c3ae3ea88e17..02e57191c1fd 100644 --- a/include/acpi/acconfig.h +++ b/include/acpi/acconfig.h @@ -190,6 +190,8 @@ #define ACPI_PRM_INPUT_BUFFER_SIZE 26 +#define ACPI_FFH_INPUT_BUFFER_SIZE 144 + /* _sx_d and _sx_w control methods */ #define ACPI_NUM_sx_d_METHODS 4 diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h index 3491e454b2ab..1b4f81f1ac5d 100644 --- a/include/acpi/actypes.h +++ b/include/acpi/actypes.h @@ -1115,6 +1115,13 @@ struct acpi_pcc_info { u8 *internal_buffer; }; +/* Special Context data for FFH Opregion (ACPI 6.5) */ + +struct acpi_ffh_info { + u64 offset; + u64 length; +}; + typedef acpi_status (*acpi_adr_space_setup) (acpi_handle region_handle, u32 function,
FFH(Fixed Function Hardware) Opregion is approved to be added in ACPIC 6.5 via code first approach[1]. It requires special context data similar to GPIO and Generic Serial Bus as it needs to know platform specific offset and length. Add support for the special context data needed by FFH Opregion. FFH OpRegion enables advanced use of FFH on some architectures. For example, it could be used to easily proxy AML code to architecture-specific behavior (to ensure it is OS initiated) Actual behavior of FFH is ofcourse architecture specific and depends on the FFH bindings. The offset and length could have arch specific meaning or usage. [1] https://bugzilla.tianocore.org/show_bug.cgi?id=3598 Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/acpi/acpica/evregion.c | 8 ++++++++ drivers/acpi/acpica/exfield.c | 8 ++++++-- drivers/acpi/acpica/exserial.c | 6 ++++++ include/acpi/acconfig.h | 2 ++ include/acpi/actypes.h | 7 +++++++ 5 files changed, 29 insertions(+), 2 deletions(-)