linux-kernel - Re: [PATCH v10] i2c: virtio: add a virtio i2c frontend driver

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:   Wed, 24 Mar 2021 09:17:58 +0800
From:   Jie Deng <jie.deng@...el.com>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Viresh Kumar <viresh.kumar@...aro.org>,
        Linux I2C <linux-i2c@...r.kernel.org>,
        virtualization@...ts.linux-foundation.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Wolfram Sang <wsa@...nel.org>,
        Jason Wang <jasowang@...hat.com>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        conghui.chen@...el.com, kblaiech@...lanox.com,
        jarkko.nikula@...ux.intel.com,
        Sergey Semin <Sergey.Semin@...kalelectronics.ru>,
        Mike Rapoport <rppt@...nel.org>, loic.poulain@...aro.org,
        Tali Perry <tali.perry1@...il.com>,
        Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        yu1.wang@...el.com, shuo.a.liu@...el.com,
        Stefan Hajnoczi <stefanha@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH v10] i2c: virtio: add a virtio i2c frontend driver


On 2021/3/23 17:27, Arnd Bergmann wrote:
> On Tue, Mar 23, 2021 at 9:33 AM Jie Deng <jie.deng@...el.com> wrote:
>> On 2021/3/23 15:27, Viresh Kumar wrote:
>>
>>> On 23-03-21, 22:19, Jie Deng wrote:
>>>> +static int __maybe_unused virtio_i2c_freeze(struct virtio_device *vdev)
>>>> +{
>>>> +    virtio_i2c_del_vqs(vdev);
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int __maybe_unused virtio_i2c_restore(struct virtio_device *vdev)
>>>> +{
>>>> +    return virtio_i2c_setup_vqs(vdev->priv);
>>>> +}
>>> Sorry for not looking at this earlier, but shouldn't we enclose the above two
>>> within #ifdef CONFIG_PM_SLEEP instead and drop the __maybe_unused ?
>>
>> I remembered I was suggested to use "__maybe_unused" instead of "#ifdef".
>>
>> You may check this https://lore.kernel.org/patchwork/patch/732981/
>>
>> The reason may be something like that.
> I usually recommend the use of __maybe_unused for the suspend/resume
> callbacks for drivers that use SIMPLE_DEV_PM_OPS() or similar helpers
> that hide the exact conditions under which the functions get called.
>
> In this driver, there is an explicit #ifdef in the reference to the
> functions, so
> it would make sense to use the same #ifdef around the definition.
>
> A better question to ask is whether you could use the helpers instead,
> and drop the other #ifdef.
>
>         Arnd


I didn't see the "struct virtio_driver" has a member "struct dev_pm_ops *pm"

It defines its own hooks (freeze and restore) though it includes "struct 
device_driver"

which has a "struct dev_pm_ops *pm".

I just follow other virtio drivers to directly use the hooks defined in 
"struct virtio_driver".

For this driver, Both __maybe_unused and #ifdef are OK to me.