Re: [PATCH v2 2/3] pinctrl: amd: Get and update IOMUX details

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

Hi Basavaraj,

thanks for your patch!

On Tue, May 24, 2022 at 9:40 AM Basavaraj Natikar
<Basavaraj.Natikar@xxxxxxx> wrote:

> Presently there is no way to change pinmux configuration runtime.
> Hence add IOMUX details which can be used to configure IOMUX
> gpio pins runtime to different functionalities.
>
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@xxxxxxx>

Interesting!

> +static acpi_status acpi_get_iomux_region(acpi_handle handle, u32 level,
> +                                        void *ctx, void **return_value)
> +{
> +       struct acpi_namespace_node *node = handle;
> +       union acpi_operand_object *region_obj;
> +       struct amd_gpio *gpio_dev = ctx;
> +
> +       /* Already mapped the IOMUX base */
> +       if (gpio_dev->iomux_base)
> +               return AE_OK;
> +
> +       /* Valid object */
> +       if (!node || !node->object)
> +               return AE_OK;
> +
> +       /* Valid operand or namespace node*/
> +       if ((ACPI_GET_DESCRIPTOR_TYPE(node->object) != ACPI_DESC_TYPE_OPERAND) &&
> +           (ACPI_GET_DESCRIPTOR_TYPE(node->object) != ACPI_DESC_TYPE_NAMED))
> +               return AE_OK;
> +
> +       /* Valid object type*/
> +       if (node->object->common.type == ACPI_TYPE_LOCAL_DATA)
> +               return AE_OK;
> +
> +       region_obj = node->object;
> +       if (!region_obj->region.handler)
> +               return AE_OK;
> +
> +       if (region_obj->region.space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
> +               return AE_OK;
> +
> +       if (strncmp("IOMX", region_obj->region.node->name.ascii, strlen("IOMX")))
> +               return AE_OK;
> +
> +       gpio_dev->iomux_base = devm_ioremap(&gpio_dev->pdev->dev,
> +                                           region_obj->region.address,
> +                                           region_obj->region.length);
> +       if (!gpio_dev->iomux_base)
> +               dev_err(&gpio_dev->pdev->dev, "failed to devm_ioremap() iomux_base\n");
> +
> +       return AE_OK;
> +}
> +
> +static void amd_update_iomux_info(struct amd_gpio *gpio_dev)
> +{
> +       acpi_handle sys_bus_handle;
> +       int status = acpi_get_handle(NULL, "\\_SB", &sys_bus_handle);
> +
> +       if (ACPI_FAILURE(status)) {
> +               dev_err(&gpio_dev->pdev->dev, "Failed to get SB handle\n");
> +               return;
> +       }
> +
> +       status = acpi_walk_namespace(ACPI_TYPE_REGION, sys_bus_handle, ACPI_UINT32_MAX,
> +                                    acpi_get_iomux_region, NULL, gpio_dev, NULL);
> +
> +       if (ACPI_FAILURE(status)) {
> +               dev_err(&gpio_dev->pdev->dev, "Failed to get acpi_get_iomux_region\n");
> +               return;
> +       }
> +}

Oh this looks scary to me, make sure you get the review from the GPIO
ACPI experts, Andy Shevchenko and/or Mika Westerberg. (Added on the
To: line)

Yours,
Linus Walleij