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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 19 Nov 2020 17:25:41 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Evan Green <evgreen@...omium.org>
Cc:     Peter Rosin <peda@...ntia.se>, Wolfram Sang <wsa@...nel.org>,
        Randy Dunlap <rdunlap@...radead.org>,
        Peter Korsgaard <peter.korsgaard@...co.com>,
        linux-i2c <linux-i2c@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RESEND PATCH v3 2/2] i2c: i2c-mux-gpio: Enable this driver in
 ACPI land

On Thu, Nov 19, 2020 at 1:40 AM Evan Green <evgreen@...omium.org> 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