Re: [RESEND PATCH v3 2/2] i2c: i2c-mux-gpio: Enable this driver in ACPI land

[Andy Shevchenko]

On Thu, Nov 19, 2020 at 1:40 AM Evan Green <evgreen@xxxxxxxxxxxx> wrote:
>
> Enable i2c-mux-gpio devices to be defined via ACPI. The idle-state
> property translates directly to a fwnode_property_*() call. The child
> reg property translates naturally into _ADR in ACPI.
>
> The i2c-parent binding is a relic from the days when the bindings
> dictated that all direct children of an I2C controller had to be I2C
> devices. These days that's no longer required. The i2c-mux can sit as a
> direct child of its parent controller, which is where it makes the most
> sense from a hardware description perspective. For the ACPI
> implementation we'll assume that's always how the i2c-mux-gpio is
> instantiated.

...

> +#ifdef CONFIG_ACPI
> +
> +static int i2c_mux_gpio_get_acpi_adr(struct device *dev,
> +                                    struct fwnode_handle *fwdev,
> +                                    unsigned int *adr)
> +
> +{
> +       unsigned long long adr64;
> +       acpi_status status;
> +
> +       status = acpi_evaluate_integer(ACPI_HANDLE_FWNODE(fwdev),
> +                                      METHOD_NAME__ADR,
> +                                      NULL, &adr64);
> +
> +       if (!ACPI_SUCCESS(status)) {
> +               dev_err(dev, "Cannot get address\n");
> +               return -EINVAL;
> +       }
> +
> +       *adr = adr64;
> +       if (*adr != adr64) {
> +               dev_err(dev, "Address out of range\n");
> +               return -ERANGE;
> +       }
> +
> +       return 0;
> +}
> +
> +#else
> +
> +static int i2c_mux_gpio_get_acpi_adr(struct device *dev,
> +                                    struct fwnode_handle *fwdev,
> +                                    unsigned int *adr)
> +{
> +       return -EINVAL;
> +}
> +
> +#endif

I'm wondering if you may use acpi_find_child_device() here.
Or is it a complementary function?

...

> +       device_for_each_child_node(dev, child) {
> +               if (is_of_node(child)) {
> +                       fwnode_property_read_u32(child, "reg", values + i);
> +
> +               } else if (is_acpi_node(child)) {
> +                       rc = i2c_mux_gpio_get_acpi_adr(dev, child, values + i);
> +                       if (rc)
> +                               return rc;
> +               }
> +
>                 i++;
>         }

And for this I already told in two different threads with similar code
that perhaps we need common helper that will check reg followed by
_ADR.

-- 
With Best Regards,
Andy Shevchenko