Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 27 Jan 2020 15:35:59 +0100
From:   Miquel Raynal <miquel.raynal@bootlin.com>
To:     Masahiro Yamada <masahiroy@kernel.org>
Cc:     linux-mtd <linux-mtd@lists.infradead.org>,
        Boris Brezillon <bbrezillon@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: How to handle write-protect pin of NAND device ?
Message-ID: <20200127153559.60a83e76@xps13>
In-Reply-To: <CAK7LNAR0FemABUg5uN5fhy5LRsOm7n5GhmFVVHE8T57knDM9Ug@mail.gmail.com>
References: <CAK7LNAR0FemABUg5uN5fhy5LRsOm7n5GhmFVVHE8T57knDM9Ug@mail.gmail.com>
Organization: Bootlin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Masahiro,

Masahiro Yamada <masahiroy@kernel.org> wrote on Mon, 27 Jan 2020
21:55:25 +0900:

> Hi.
> 
> I have a question about the
> WP_n pin of a NAND chip.
> 
> 
> As far as I see, the NAND framework does not
> handle it.

There is a nand_check_wp() which reads the status of the pin before
erasing/writing.

> 
> Instead, it is handled in a driver level.
> I see some DT-bindings that handle the WP_n pin.
> 
> $ git grep wp -- Documentation/devicetree/bindings/mtd/
> Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt:-
> brcm,nand-has-wp          : Some versions of this IP include a
> write-protect

Just checked: brcmnand de-assert WP when writing/erasing and asserts it
otherwise. IMHO this switching is useless.

> Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt:-
> wp-gpios: GPIO specifier for the write protect pin.
> Documentation/devicetree/bindings/mtd/ingenic,jz4780-nand.txt:
>          wp-gpios = <&gpf 22 GPIO_ACTIVE_LOW>;
> Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt:-
> wp-gpios: GPIO specifier for the write protect pin.
> Documentation/devicetree/bindings/mtd/nvidia-tegra20-nand.txt:
>          wp-gpios = <&gpio TEGRA_GPIO(S, 0) GPIO_ACTIVE_LOW>;

In both cases, the WP GPIO is unused in the code, just de-asserted at
boot time like what you do in the patch below.

> 
> 
> 
> I wrote a patch to avoid read-only issue in some cases:
> http://patchwork.ozlabs.org/patch/1229749/
> 
> Generally speaking, we expect NAND devices
> are writable in Linux. So, I think my patch is OK.

I think the patch is fine.

> 
> 
> However, I asked this myself:
> Is there a useful case to assert the write protect
> pin in order to make the NAND chip really read-only?
> For example, the system recovery image is stored in
> a read-only device, and the write-protect pin is
> kept asserted to assure nobody accidentally corrupts it.

It is very likely that the same device is used for RO and RW storage so
in most cases this is not possible. We already have squashfs which is
actually read-only at filesystem level, I'm not sure it is needed to
enforce this at a lower level... Anyway if there is actually a pin for
that, one might want to handle the pin directly as a GPIO, what do you
think?

> But, I am not sure if it should be handled in the
> framework level with a more generic DT-binding.
> 
> 
> Comments are appreciated.
> 
> --
> Best Regards
> Masahiro Yamada

Thanks,
Miquèl