Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 8bit
Date:   Wed, 02 Dec 2020 12:25:49 +0100
From:   Michael Walle <michael@walle.cc>
To:     Tudor.Ambarus@microchip.com
Cc:     linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
        miquel.raynal@bootlin.com, richard@nod.at, vigneshr@ti.com,
        boris.brezillon@collabora.com
Subject: Re: [PATCH v6 5/5] mtd: spi-nor: keep lock bits if they are
 non-volatile
In-Reply-To: <8e0a6a20-2779-9397-eedf-02518b4a0e5a@microchip.com>
References: <20201126202614.5710-1-michael@walle.cc>
 <20201126202614.5710-6-michael@walle.cc>
 <432b31a7-2560-3b83-44d2-aa82c2e322ae@microchip.com>
 <46c99138eb6ce251bc741d358388c219@walle.cc>
 <8e0a6a20-2779-9397-eedf-02518b4a0e5a@microchip.com>
User-Agent: Roundcube Webmail/1.4.9
Message-ID: <fe04f234584c2f459e865955b0d09303@walle.cc>
Precedence: bulk

Am 2020-12-02 12:10, schrieb Tudor.Ambarus@microchip.com:
> On 11/30/20 4:38 PM, Michael Walle wrote:
[..]
>>>> +        * indicated by SNOR_F_WP_IS_VOLATILE.
>>>> +        */
>>>> +       if (IS_ENABLED(CONFIG_MTD_SPI_NOR_WP_DISABLE) ||
>>>> +           (IS_ENABLED(CONFIG_MTD_SPI_NOR_WP_DISABLE_ON_VOLATILE) 
>>>> &&
>>>> +            nor->flags & SNOR_F_WP_IS_VOLATILE)) {
>>>> +               err = spi_nor_unlock_all(nor);
>>>> +               if (err) {
>>>> +                       dev_err(nor->dev, "Failed to unlock the 
>>>> entire
>>>> flash memory array\n");
>>> 
>>> dev_dbg for low level info
>> 
>> Is this low level info or an actual error? Which raises the question:
>> should spi_nor_unlock_all() in case SWRD couldn't be cleared and thus
>> should all the spi_nor_init fail of this? Or should it rather be a
> 
> yes, it should, because the flash will not work as expected/requested.

One counterargument: take our sl28 board, it has a hardware 
write-protected
SPI flash. It actually works right now because the write_sr_and_check()
doesn't work as intended and doesn't check what is written. So if you'd
fix that (and these changes would be backported to the stable trees), 
you'd
basically break spi-nor on these boards. And this _must_ be the case for
all boards which are actually using (hard- or sofware) write-protection.
That is the only way write-protection makes sense prior to this patch
series. Because linux will happily unlock every flash on startup. 
Therefore,
the hardware write protection is the only measure against this.

-michael