linux-kernel - Re: [PATCH v4] i2c: i801: Register optional lis3lv02d I2C device on Dell machines

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] [day] [month] [year] [list]

Date:   Thu, 6 Jun 2019 19:37:15 +0200
From:   Jean Delvare <jdelvare@...e.de>
To:     Pali Rohár <pali.rohar@...il.com>
Cc:     Wolfram Sang <wsa@...-dreams.de>,
        Michał Kępień <kernel@...pniu.pl>,
        Steven Honeyman <stevenhoneyman@...il.com>,
        Valdis.Kletnieks@...edu,
        Jochen Eisinger <jochen@...guin-breeder.org>,
        Gabriele Mazzotta <gabriele.mzt@...il.com>,
        Andy Lutomirski <luto@...nel.org>, Mario_Limonciello@...l.com,
        Alex Hung <alex.hung@...onical.com>,
        Takashi Iwai <tiwai@...e.de>, linux-i2c@...r.kernel.org,
        linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org
Subject: Re: [PATCH v4] i2c: i801: Register optional lis3lv02d I2C device on
 Dell machines

On Thu, 6 Jun 2019 17:48:00 +0200, Pali Rohár wrote:
> On Thursday 06 June 2019 16:53:09 Jean Delvare wrote:
> > This is testing that *either* bit is set. Is it what you intend to
> > achieve, or would you rather want to ensure that *all* these bits are
> > set?  
> 
> Of course, it is wrong. Thanks for catch. We should ignore apci devices
> which are not present, which are disabled or which are not functioning.
> 
> Now I looked into acpi_get_devices() implementation and it call
> acpi_ns_get_device_callback() function callback for every device. At the
> end that function calls user supplied check_acpi_smo88xx_device
> function.
> 
> And acpi_ns_get_device_callback() already ignores acpi devices which do
> not have ACPI_STA_DEVICE_PRESENT or ACPI_STA_DEVICE_FUNCTIONING flag.
> 
> According to acpi documentation when ACPI_STA_DEVICE_PRESENT is not set
> then ACPI_STA_DEVICE_ENABLED also cannot be set.
> 
> So the whole acpi_bus_get_status_handle() is not needed at all as
> acpi_get_devices() via acpi_ns_get_device_callback() already filter
> unsuitable acpi devices.
> 
> I guess that I already did this investigation in past and added comment
> "exists and is enabled" which is below near acpi_get_devices() call. But
> as I wrote this patch more then year ago I forgot about it.

Sorry for confusing you :-(

> I will remove that check. Do you have any suggestion what to write into
> comment so other readers in future would know that we do not need to
> check anything with _STA acpi method?

Well, if you manage to squeeze a short version of the above explanation
at the relevant place in the driver, that should work. Doesn't have to
be verbose really, anything that says that disabled devices will be
properly skipped for us will be good enough.

-- 
Jean Delvare
SUSE L3 Support