From: Vignesh Raghavendra <vigneshr@ti.com>
To: <masonccyang@mxic.com.tw>, Pratyush Yadav <me@yadavpratyush.com>
Cc: <tudor.ambarus@microchip.com>, <juliensu@mxic.com.tw>,
<richard@nod.at>, <miquel.raynal@bootlin.com>,
<linux-kernel@vger.kernel.org>, <linux-spi@vger.kernel.org>,
<broonie@kernel.org>, <linux-mtd@lists.infradead.org>,
Boris Brezillon <boris.brezillon@collabora.com>,
Pratyush Yadav <p.yadav@ti.com>
Subject: Re: [PATCH v2 0/5] mtd: spi-nor: Add support for Octal 8D-8D-8D mode
Date: Thu, 30 Apr 2020 13:51:47 +0530 [thread overview]
Message-ID: <3e33ffae-3267-015a-8535-6e1667b03e39@ti.com> (raw)
In-Reply-To: <OF04289CE2.B346916F-ON48258559.002280BD-48258559.00295800@mxic.com.tw>
Hi Mason,
On 29/04/20 1:01 pm, masonccyang@mxic.com.tw wrote:
>
> Hi Pratyush,
>
>
>>>>> On Tue, 21 Apr 2020 14:39:42 +0800
>>>>> Mason Yang <masonccyang@mxic.com.tw> wrote:
[...]
>>>
> https://lore.kernel.org/linux-mtd/20200424184410.8578-1-p.yadav@ti.com/
>>>
>>>
>>> Our mx25uw51245g supports BFPT DWORD-18,19 and 20 data and xSPI
> profile
>>> 1.0,
>>> and it comply with BFPT DWORD-19, octal mode enable sequences by write
> CFG
>>> Reg2
>>> with instruction 0x72. Therefore, I can't apply your patches.
>>
>> I didn't mean apply my patches directly. I meant more along the lines of
>
>> edit your patches to work on top of my series. It should be as easy as
>> adding your flash's fixup hooks and its octal DTR enable hook, but if my
>
>> series is missing something you need (like complete Profile 1.0 parsing,
>
>> which I left out because I wanted to be conservative and didn't see any
>> immediate use-case for us), let me know, and we can work together to
>> address it.
>
> yes,sure!
> let's work together to upstream the Octal 8D-8D-8D driver to mainline.
>
> The main concern is where and how to enable xSPI octal mode?
>
> Vignesh don't agree to enable it in fixup hooks and that's why I patched
> it to spi_nor_late_init_params() and confirmed the device support xSPI
> Octal mode after BFPT DWORD-19 and xSPI pf 1.0 have been parsed.
>
My suggestion was to use SFDP wherever possible.. E.g: it is possible to
get opcode extension type from BFPT...
But using BFPT DWORD-19 is not correct for switching to 8D-8D-8D mode:
Per JESD216D.01 Bits 22:20 of 19th DWORD of BFPT:
Octal Enable Requirements:
This field describes whether the device contains a Octal Enable bit used
to enable 1-1-8 and 1-
8-8 octal read or octal program operations.
So, this cannot be used for enabling 8D-8D-8D mode... Flashes that only
support 1S-1S-1S and 8D-8D-8D will set this field to 0.
There is a separate table to enable 8D mode called
"Command Sequences to Change to Octal DDR (8D-8D-8D) mode". But if flash
does not have the table or has bad data, fixup hook is the only way...
If mx25* supports above table, please build on top of Pratyush's series
to add support for parsing this table. Otherwise, macronix would have to
use a fixup hook too...
> I can't apply your patches to enable xSPI Octal mode for mx25uw51245g
> because your patches set up Octal protocol first and then using Octal
> protocol to write Configuration Register 2(CFG Reg2). I think driver
> should write CFG Reg2 in SPI 1-1-1 mode (power on state) and make sure
> write CFG Reg 2 is success and then setup Octa protocol in the last.
>
> As JESD216F description on BFPT DOWRD 19th, only two way to enable
> xSPI Octal mode;
Where is JESD216F? Latest I can find is JESD216D.01
> one is by two instruction: issue instruction 06h(WREN) and then E8h.
> the other is issue instruction 06h, then issue instruction 72h (Write
> CFG Reg2), address 0h and data 02h (8D-8D-8D).
>
> Let our patches comply with this. you may refer to my patches
> [v2,3/5] mtd: spi-nor: Parse BFPT DWORD-18, 19 and 20 for Octal 8D-8D-8D
> mode
As I pointed out earlier using above DWORDS seems wrong for 8D-8D-8D,
they can be used for 1-1-8 and 1-
8-8
>
> /* Octal mode enable sequences. */
> switch (bfpt.dwords[BFPT_DWORD(19)] &
> BFPT_DWORD19_OCTAL_SEQ_MASK) {
> case BFPT_DWORD19_TWO_INST:
> + ----> to patch here.
> break;
> case BFPT_DWORD19_CFG_REG2:
> params->xspi_enable =
> spi_nor_cfg_reg2_octal_enable;
> break;
> default:
> break;
> }
>
>
>>
>>> I quickly went through your patches but can't reply them in each your
>>> patches.
>>>
>>> i.e,.
>>> 1) [v4,03/16] spi: spi-mem: allow specifying a command's extension
>>>
>>> - u8 opcode;
>>> + u16 opcode;
>>>
>>> big/little Endian issue, right?
Is the big/little Endian issue a quirk of the flash or controller? If
its controller specific then it needs to handled in controller driver.
If this is a flash quirk, please point to the waveforms in the flash
datasheet...
>>> why not just u8 ext_opcode;
>>> No any impact for exist code and actually only xSPI device use
> extension
>>> command.
>>
>> Boris already explained the reasoning behind it.
>
> yup, I got his point and please make sure CPU data access.
>
> i.e,.
> Fix endianness of the BFPT DWORDs and xSPI in sfdp.c
>
> and your patch,
> + ext = spi_nor_get_cmd_ext(nor, op);
> + op->cmd.opcode = (op->cmd.opcode << 8) |
> ext;
> + op->cmd.nbytes = 2;
>
> I think maybe using u8 opcode[2] could avoid endianness.
>
> Moreover, Vignesh think it's fine to use u8 ext_opcode in my v1 patches.
> please check his comments on
> https://patchwork.ozlabs.org/project/linux-mtd/patch/1573808288-19365-3-git-send-email-masonccyang@mxic.com.tw/
>
>
>
> Let's open this discussion and maybe Vighesh and Tudor could have some
> comments on it.
> thanks a lot.
>
Sorry , but others clearly see having single variable to store cmd +
extension is beneficial here. So, I take back my suggestion.
Regards
Vignesh
next prev parent reply other threads:[~2020-04-30 8:22 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-21 6:39 [PATCH v2 0/5] mtd: spi-nor: Add support for Octal 8D-8D-8D mode Mason Yang
2020-04-21 6:39 ` [PATCH v2 1/5] " Mason Yang
2020-04-21 6:39 ` [PATCH v2 2/5] mtd: spi-nor: sfdp: Add support for xSPI profile 1.0 table Mason Yang
2020-04-21 6:39 ` [PATCH v2 3/5] mtd: spi-nor: Parse BFPT DWORD-18,19 and 20 for Octal 8D-8D-8D mode Mason Yang
2020-04-21 6:39 ` [PATCH v2 4/5] mtd: spi-nor: macronix: Add Octal 8D-8D-8D supports for Macronix mx25uw51245g Mason Yang
2020-04-21 6:39 ` [PATCH v2 5/5] spi: mxic: Patch for Octal 8D-8D-8D mode support Mason Yang
2020-04-24 15:41 ` kbuild test robot
2020-04-21 7:23 ` [PATCH v2 0/5] mtd: spi-nor: Add support for Octal 8D-8D-8D mode Boris Brezillon
2020-04-21 9:35 ` Vignesh Raghavendra
2020-04-21 12:17 ` Boris Brezillon
2020-04-27 17:55 ` Pratyush Yadav
2020-04-28 6:14 ` masonccyang
2020-04-28 6:34 ` Boris Brezillon
2020-04-28 8:35 ` Pratyush Yadav
2020-04-29 5:59 ` masonccyang
2020-04-28 8:54 ` Pratyush Yadav
2020-04-29 7:31 ` masonccyang
2020-04-29 8:37 ` Boris Brezillon
2020-04-29 18:18 ` Pratyush Yadav
2020-05-05 9:31 ` masonccyang
2020-05-05 9:44 ` Boris Brezillon
2020-05-05 10:01 ` Boris Brezillon
2020-05-11 6:54 ` masonccyang
2020-05-06 9:40 ` Pratyush Yadav
2020-05-15 2:26 ` masonccyang
2020-05-15 6:55 ` Pratyush Yadav
2020-04-30 8:21 ` Vignesh Raghavendra [this message]
2020-05-11 3:23 ` masonccyang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3e33ffae-3267-015a-8535-6e1667b03e39@ti.com \
--to=vigneshr@ti.com \
--cc=boris.brezillon@collabora.com \
--cc=broonie@kernel.org \
--cc=juliensu@mxic.com.tw \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=linux-spi@vger.kernel.org \
--cc=masonccyang@mxic.com.tw \
--cc=me@yadavpratyush.com \
--cc=miquel.raynal@bootlin.com \
--cc=p.yadav@ti.com \
--cc=richard@nod.at \
--cc=tudor.ambarus@microchip.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).