linux-kernel - RE: [PATCH 2/2] i2c: smbus: Use device_ functions instead of of

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]

Message-ID: <BN9PR12MB52732B801C0D15BBBA71B8DDC0779@BN9PR12MB5273.namprd12.prod.outlook.com>
Date:   Thu, 16 Dec 2021 16:08:27 +0000
From:   Akhil R <akhilrajeev@...dia.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
CC:     Wolfram Sang <wsa@...nel.org>,
        Sumit Semwal <sumit.semwal@...aro.org>,
        Christian Koenig <christian.koenig@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Dmitry Osipenko <digetx@...il.com>,
        Laxman Dewangan <ldewangan@...dia.com>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        linux-i2c <linux-i2c@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-tegra <linux-tegra@...r.kernel.org>,
        Krishna Yarlagadda <kyarlagadda@...dia.com>,
        Suresh Mangipudi <smangipudi@...dia.com>
Subject: RE: [PATCH 2/2] i2c: smbus: Use device_ functions instead of of_

> On Thu, Dec 16, 2021 at 3:14 PM Akhil R <akhilrajeev@...dia.com> wrote:
> >
> > Change of_ functions to device_ for firmware agnostic usage.
> 
> of_*()
> device_*()
> 
> > This allows to have smbus_alert interrupt without any changes in the
> > controller drivers using ACPI table.
> 
> ...
> 
> > -       irq = of_property_match_string(adapter->dev.of_node, "interrupt-
> names",
> > -                                      "smbus_alert");
> > +       irq = device_property_match_string(adapter->dev.parent, "interrupt-
> names",
> > +                                          "smbus_alert");
> 
> Hmm... Adapter device node is not the same as the node for its parent.
> Do you have some code that propagates of_node from parent to child?
Adapter device does not have an of_node unless the adapter driver
sets it, I guess. I see all the adapter drivers add the of_node and 
parent for adapter. Also, there are many places in i2c-core-base and 
i2c-core-acpi where adapter->dev.parent is referred to as the adapter 
driver device.

Basically, adapter->dev.parent and adapter->dev.of_node would 
ultimately refer to the same device (or the of_node of that device), 
as far as I understand.
> 
> I.o.w. I would expect to see
> 
>        irq = device_property_match_string(&adapter->dev, "interrupt-names",
> 
> here.
It would then require adding the fw_node as well from the adapter driver.
I felt it made more sense to refer adapter->dev.parent here as most of the
(or rather all of the) adapter drivers already sets it.
> 
> >         if (irq == -EINVAL || irq == -ENODATA)
> >                 return 0;
> >         else if (irq < 0)
> 
> TBH the entire code smells. "Interesting" way of getting an optional named
> interrupt.
I felt it useful to have it this way as it would remain agnostic to device tree and 
the ACPI. We would not have to add redundant codes in adapter drivers that
are using ACPI table.

Named interrupts for the ACPI as well, I feel would be a useful addition that can
prove to be of value more than this change; I believe.

Thanks,
Akhil