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;
Date:   Fri, 7 May 2021 19:26:33 +0530
From:   Pratyush Yadav <p.yadav@ti.com>
To:     Mark Brown <broonie@kernel.org>
CC:     Tudor Ambarus <tudor.ambarus@microchip.com>,
        Michael Walle <michael@walle.cc>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        Richard Weinberger <richard@nod.at>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        <linux-mtd@lists.infradead.org>, <linux-kernel@vger.kernel.org>,
        <linux-spi@vger.kernel.org>
Subject: Re: [PATCH 4/6] spi: spi-mem: reject partial cycle transfers in
 8D-8D-8D mode
Message-ID: <20210507135631.maue7gorfzsv4qpk@ti.com>
References: <20210506191829.8271-1-p.yadav@ti.com>
 <20210506191829.8271-5-p.yadav@ti.com>
 <20210507125533.GA6383@sirena.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20210507125533.GA6383@sirena.org.uk>
User-Agent: NeoMutt/20171215
Precedence: bulk

On 07/05/21 01:55PM, Mark Brown wrote:
> On Fri, May 07, 2021 at 12:48:27AM +0530, Pratyush Yadav wrote:
> > In 8D-8D-8D mode two bytes are transferred per cycle. So an odd number
> > of bytes cannot be transferred because it would leave a residual half
> > cycle at the end. Consider such a transfer invalid and reject it.
> > 
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > 
> > ---
> > This patch should go through the SPI tree but I have included it in this
> > series because if it goes in before patches 1-3, Micron MT35XU and
> > Cypress S28HS flashes will stop working correctly.
> 
> It seems like this should probably even go in as a fix even if nothing
> is broken with mainline right now, it's the sort of thing some out of
> tree patch could easily trigger.  Unless anyone objects I'll do that.

If it does get backported to stable branches, patches 1-3 would have to 
go in _before_ this one. Otherwise it will break the two 8D-8D-8D 
flashes: Micron MT35XU512ABA and Cypress S28HS512T. Without patch 1 
8D-8D-8D mode will not be selected correctly on these two flashes. The 
supports_op() will reject it because of 1 data byte. This is a 
relatively safe patch for backporting to a stable branch.

Patches 2 and 3 are a slightly different matter. They add an extra 
register write. But most controllers I've come across don't support 
1-byte writes in 8D mode. It is likely that they are sending 
bogus/undefined values in the second byte and deasserting CS only after 
the cycle is done. So they should _in theory_ change undefined behaviour 
to defined behaviour.

Still, they introduce an extra register write. I'm not sure how 
risk-tolerant you want to be for stable backports. I will leave the 
judgement to you or Tudor or Vignesh.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.