linux-kernel - Re: [PATCH 2/4] iio: dac: ad5755: mark OF related data as maybe unused

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:   Sun, 12 Mar 2023 11:11:17 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Jonathan Cameron <jic23@...nel.org>
Cc:     Marek Vasut <marek.vasut@...il.com>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Michael Hennerich <Michael.Hennerich@...log.com>,
        Robert Eshleman <bobbyeshleman@...il.com>,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] iio: dac: ad5755: mark OF related data as maybe
 unused

On 11/03/2023 19:31, Jonathan Cameron wrote:
> On Sat, 11 Mar 2023 13:25:33 +0100
> Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> wrote:
> 
>> On 11/03/2023 13:22, Jonathan Cameron wrote:
>>> On Sat, 11 Mar 2023 12:14:55 +0100
>>> Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org> wrote:
>>>   
>>>> The driver can be compile tested with !CONFIG_OF making certain data
>>>> unused (of_device_id is not used for device matching):  
>>>
>>> It should be used for device matching I think, so I'd rather see
>>> it assigned for that purpose than hiding the issue.  
>>
>> That would require testing and changes. The device matching is via SPI
>> table which has device data. Probably adding OF matching would require
>> bigger changes to for handling the match data.
>>
>> This was intentional design in this driver, so we are not hiding here
>> anything.
> 
> I doubt it was intentional. Mostly people do this because the magic
> fallbacks to find the spi_device_id entry work.
> 
> If we'd noticed at review time it would not have gone in like this.
> Note that the spi_match_id() use of_modalias_node() which has stripped the
> vendor id off the compatible then matches against the spi_device_id
> table.
> 
> So it 'should' just work.  Now ideally we'd switch to
> spi_get_device_match_data() but that needs more significant changes.
> Though simple enough ones that review would be enough.
> 
> Just need to use pointers to the ad75755_chip_info_tbl entries
> rather than the enum in both the spi id table and the of one - this
> avoids the issue with the enum value of 0 counting as a failed match.

It's not that simple change... maybe you are right that adding match
data to OF table would not break anything, but to me it is something
substantial and requiring testing, which obviously I cannot do.
Therefore I am going to skip this one (thus the error stays).

Best regards,
Krzysztof