linux-kernel - Re: [PATCH] ARM: dts: at91: sama5d4_xplained: Mark optional AT25DF321A as disabled

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]

Message-ID: <f8fcace3-fc18-ff3a-8d85-0fc10f672d5e@microchip.com>
Date:   Thu, 24 Jun 2021 15:22:24 +0000
From:   <Tudor.Ambarus@...rochip.com>
To:     <Nicolas.Ferre@...rochip.com>, <alexandre.belloni@...tlin.com>,
        <Ludovic.Desroches@...rochip.com>
CC:     <robh+dt@...nel.org>, <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ARM: dts: at91: sama5d4_xplained: Mark optional
 AT25DF321A as disabled

On 6/22/21 4:53 PM, Nicolas Ferre wrote:
> Hi Tudor,
> 
> On 22/06/2021 at 15:42, Tudor Ambarus wrote:
>> sama5d4_xplained has an optional on-board serial DataFlash (AT25DF321A),
>> which does not come populated on board by default. Mark it as disabled
>> to avoid the following probe error:
>> spi-nor spi0.0: unrecognized JEDEC id bytes: 00 00 00 00 00 00
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@...rochip.com>
>> ---
>>   arch/arm/boot/dts/at91-sama5d4_xplained.dts | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/at91-sama5d4_xplained.dts b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
>> index 0b3ad1b580b8..f397e2a68e5c 100644
>> --- a/arch/arm/boot/dts/at91-sama5d4_xplained.dts
>> +++ b/arch/arm/boot/dts/at91-sama5d4_xplained.dts
>> @@ -40,7 +40,7 @@ uart0: serial@...04000 {
>>                 spi0: spi@...10000 {
>>                   cs-gpios = <&pioC 3 0>, <0>, <0>, <0>;
>> -                status = "okay";
>> +                status = "disabled";
>>                   m25p80@0 {
>>                       compatible = "atmel,at25df321a";
>>                       spi-max-frequency = <50000000>;
> 
> What about keeping SPI enabled and just having the status of m25p80 as disabled: honestly, I don't know if it works and if it's desirable, but might be closer to reality.
> 
> Tell me what you think.

I think it will work, because child status is checked with __of_device_is_available,
and the child will be skipped when marked as disabled.

But I feel that spi0 was enabled just because at25df321a was assumed to be populated.
Wouldn't it be strange to have spi0 marked as okay, but the only child as disabled?
How about removing spi0 node entirely from sama5d4_xplained dts?

Cheers,
ta