From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5F664241FC for ; Thu, 14 Dec 2023 08:56:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="XeKmQtl6" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 37572C433C7; Thu, 14 Dec 2023 08:56:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1702544211; bh=nbf0dgDn+vr040wHTregcpxbzjC0V+a+rzOuDCSHWMo=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=XeKmQtl66oLbf4JrM8QOR6OkwrqPVRjYycaK+o5uOUxzH/4GpoRr3xH5hNikl+RU5 ZS7g2qGHhjZIWbXWb1BNLIXdwjoxVcHLSjnqWvxNieCOnsimInfS72ApYcgvRFAUv5 VQvpC6v/TOB429G9khJ0FIMcIMzp3E7lKqwou2i9+iJCxDihlrJHcjCqTLEEgMXESt BRTe1g3qaiSFzl+kt+DHcJpfQsPOglwPl2tdSO2sfP2BATLk+xkC3HtlNTTZBx0oYy DUbiBUyaL1ttlefvh+LHNaiWggqJpSYS0Cqb6dE8CRVrp2KLh1oh4o3HEztdZSVzo5 elENY6HaY7eSg== Precedence: bulk X-Mailing-List: linux-spi@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Date: Thu, 14 Dec 2023 09:56:47 +0100 From: Michael Walle To: Bough Chen Cc: broonie@kernel.org, Han Xu , dl-linux-imx , linux-spi@vger.kernel.org, yogeshgaur.83@gmail.com Subject: Re: [PATCH 4/5] spi: spi-nxp-fspi: add function to select sample clock source for flash reading In-Reply-To: References: <20231213091346.956789-4-haibo.chen@nxp.com> <20231213172113.2774476-1-mwalle@kernel.org> Message-ID: <1995188744e94699d0372d2c24010ab2@kernel.org> X-Sender: mwalle@kernel.org Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Hi, >> > From: Haibo Chen >> > >> > fspi define four mode for sample clock source selection. >> > >> > Here is the list of modes: >> > mode 0: Dummy Read strobe generated by FlexSPI Controller and loopback >> > internally mode 1: Dummy Read strobe generated by FlexSPI Controller >> > and loopback from DQS pad mode 2: Reserved mode 3: Flash provided Read >> > strobe and input from DQS pad >> > >> > In default, fspi use mode 0 after reset. >> > For 8-8-8-DTR mode, need to use mode 3, otherwise 8-8-8-DTR read >> > always get incorrect data. >> >> I'd say this is board dependant, right? If you now hardcode 8d8d8d to >> always use >> mode 3. I'm not sure how a board which doesn't have the DQS connected >> to the >> flash can change this to another mode again. Looks like we'd need a >> (DT) >> property which tells you if there is actually a DQS line connected to >> the flash. > > Currently we distinguish through SoC chip, not board. Like patch5. > If SoC contain the DQS, but the board do not connect it to flash > device, then this is a real issue. See below, there are alternatives to the DQS pin. But it really is frequency dependent. I'd bet you can use mode 0 with a slow frequency in 8d8d8d mode. I.e. the LS1028ARM will always refer you to mode 3 if you want to archive "the highest frequency". > But I think if user use one octal flash device which support dtr mode, > they should > connect this DQS pad if want to work in DTR mode. > If forget to connect the DQS pad, they can limit the tx/rx buswidth to > 4 or 1 in dts. > > Anyway, add a DT property seems better. > > DQS is a must requirement for octal dtr mode, if detect no DQS, we can > also disable the DTR mode support. I don't think this is true in general. I haven't checked with the FSPI controller. But DQS is used for higher frequencies which isn't necessarily related to DTR. Also, there are other methods, like manual calibration of the delay lines on the I/Os. There was once a patchset to add that calibration for a TI (?) controller, but it was never merged. I've seen the fspi also supports some DLL configuration and some kind of data learning feature. As far as I understand that, these are all alternatives. I.e. (1) don't use high frequencies (2) use DQS (driven by the flash) (3) manually/automatically set the DLL > Will add in next version. > >> Btw you don't check buswidth, so you'll enable that mode for any DTR >> mode. > > Seems current spi-nor code only support one DTR mode, that is 8d-8d-8d. Yes. But that doesn't mean the spi controller should assume that. If you don't want to support any other mode, you should probably check that in your .supports_op() callback. -michael