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;
Subject: Re: [RESEND PATCH 1/2] mtd: spi-nor: do not touch TB bit without
 SPI_NOR_HAS_TB
To:     Ivan Mikhaylov <i.mikhaylov@yadro.com>,
        Tudor Ambarus <tudor.ambarus@microchip.com>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        Richard Weinberger <richard@nod.at>
CC:     <linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>
References: <20200921112450.4824-1-i.mikhaylov@yadro.com>
 <20200921112450.4824-2-i.mikhaylov@yadro.com>
From:   Vignesh Raghavendra <vigneshr@ti.com>
Message-ID: <9bfb10df-64d8-4a5e-b0ad-a8b5f4efc56f@ti.com>
Date:   Wed, 30 Sep 2020 15:06:46 +0530
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20200921112450.4824-2-i.mikhaylov@yadro.com>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 9/21/20 4:54 PM, Ivan Mikhaylov wrote:
> Some chips like macronix don't have TB(Top/Bottom protection)
> bit in the status register. Do not write tb_mask inside status
> register, unless SPI_NOR_HAS_TB is present for the chip.
> 

Not entirely accurate.. Macronix chips have TB bit in config register
and is OTP and hence should not be touched ideally...

You still need to "read" that bit to determine actual scheme (Top vs
Bottom). This is needs to be done before 2/2 enables SPI_NOR_HAS_LOCK
flag for macronix flashes.

> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
> ---
>  drivers/mtd/spi-nor/core.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 0369d98b2d12..f9853dd566dc 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1735,13 +1735,18 @@ static int spi_nor_sr_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
>  			return -EINVAL;
>  	}
>  
> -	status_new = (status_old & ~mask & ~tb_mask) | val;
> +	if (nor->flags & SNOR_F_HAS_SR_TB)
> +		status_new = (status_old & ~mask & ~tb_mask) | val;
> +	else
> +		status_new = (status_old & ~mask) | val;
>  
>  	/* Disallow further writes if WP pin is asserted */
>  	status_new |= SR_SRWD;
>  

I guess macronix does not support SR_SRWD right? This needs special
treatment as well.

So either, macronix.c should implements its own locking ops or convert
this function in to more generic library so that its suitable to be
called from macronix.c file while hiding vendor specific stuff in that
driver,

Regards
Vignesh