From: William Zhang <william.zhang@broadcom.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Broadcom Kernel List <bcm-kernel-feedback-list@broadcom.com>,
Linux MTD List <linux-mtd@lists.infradead.org>,
f.fainelli@gmail.com, rafal@milecki.pl, kursad.oney@broadcom.com,
joel.peshkin@broadcom.com, computersforpeace@gmail.com,
anand.gore@broadcom.com, dregan@mail.com,
kamal.dasu@broadcom.com, tomer.yacoby@broadcom.com,
dan.beygelman@broadcom.com, linux-kernel@vger.kernel.org,
Vignesh Raghavendra <vigneshr@ti.com>,
Richard Weinberger <richard@nod.at>,
Kamal Dasu <kdasu.kdev@gmail.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 10/12] mtd: rawnand: brcmnand: Add BCMBCA read data bus interface
Date: Mon, 12 Jun 2023 12:18:58 -0700 [thread overview]
Message-ID: <da4cb6a6-aa7d-3747-3f64-19b5582b15e8@broadcom.com> (raw)
In-Reply-To: <20230612195305.4b097c46@xps-13>
[-- Attachment #1: Type: text/plain, Size: 4133 bytes --]
On 06/12/2023 10:53 AM, Miquel Raynal wrote:
> Hello again,
>
>>>>>>>> Perhaps we could have a single function that is statically assigned at
>>>>>>>> probe time instead of a first helper with two conditions which calls in
>>>>>>>> one case another hook... This can be simplified I guess.
>>>>>>>> >> Well this will need to be done at the SoC specific implementation level (bcm<xxx>_nand.c) and each SoC will need to have either general data bus read func with is_param option or data_bus_read_page, data_bus_read_param.
>>>>>>
>>>>>> You told me in case we would use exec_op we could avoid the param
>>>>>> cache. If that's true then the whole support can be simplified.
>>>>>> >> Correct we may possibly unified the parameter data read but exec_op is long shot and we are not fully ready for that yet. It also depends on if the low level data register has endianess difference for the parameter data between difference SoCs.
>>>>>
>>>>> So I would like to push the current implementation and we can explore the exec_op option late which will be a much big and complete different implementation.
>>>>
>>>> I am sorry but this series is totally backwards, you're trying to guess
>>>> what comes next with the 'is_param' thing, it's exactly what we are
>>>> fighting against since 2017. There are plenty of ->exec_op()
>>>> conversions out there, I don't believe this one will be harder. You
>>>> need to convert the driver to this new API and get rid of this whole
>>>> endianness non-sense to simplify a lot the driver.
>>>>
>>> I am not guessing anything but just factor out the existing common nand cache read logic into the single default function(or one for page read and another for parameter read as I mentioned in another thread) and allow SoC to overrides the implementation when needed.
>>
>> No, you are trying to guess what type of read the core is performing,
>> either a regular data page read or a parameter page read.
>>
>>> I agree ->exec_op can possibly get rid of the parameter page read function and is the way to go. But it won't help on the page read for endianess.
>>
>> You told me there is no endianess issue with the data pages, so why it
>> won't help on the page read?
>>
>>> It's not that I am against exec_op but I want to take one step a time
>>> and I'd like to get these fixes
>>
>> I don't see any fix here? Let me know if I am missing something but
>> right now I see a new version of the controller being supported with
>> its own constraints. If you are fixing existing code for already
>> supported platform, then make it clear and we can discuss this. But if
>> you just want to support the bcmbca flavor, then there is no risk
>> mitigation involved here, and a conversion is the right step :)
>>
>
> I forgot to mention: the exec_op conversion is almost ready, Boris
> worked on it but he lacked the hardware so maybe you'll just need to
> revive the few patches which target your platform and do a little bit of
> debugging?
>
> https://github.com/bbrezillon/linux/commits/nand/exec-op-conversion?after=8a3cf6fd25d5e15c6667f9e95c1fc86e4cb735e6+34&branch=nand%2Fexec-op-conversion&qualified_name=refs%2Fheads%2Fnand%2Fexec-op-conversion
>
Yes this is the patch what our exec_op work is based on. Thanks Boris!
The issue with patch is that performance is very slow for anything that
rely on nand_read_page_op as the patch implementing it using the low
level cmd and data register to transfer the data byte by byte. I
actually sent out email regarding this to Boris and he cc'ed you in sept
last year. We have to use the nand parser to match the page read from
exec_op so we can actually match and use the brcmnand_page_read fast
path. But there are many situations that we need to match so the project
to migrate exce_op are still work in progress just on our bcmbca chip as
of now. Just forward that email again to you and I appreciate it if you
have any inputs there. So IMHO it is just too risky and too big of
scope to have the exec_op added to this patch series and definitively
better to do it afterwards with a dedicated patch.
> Cheers,
> Miquèl
>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]
next prev parent reply other threads:[~2023-06-12 19:19 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-06 23:12 [PATCH 00/12] mtd: rawnand: brcmnand: driver and doc updates William Zhang
2023-06-06 23:12 ` [PATCH 01/12] mtd: rawnand: brcmnand: Fix ECC level field setting for v7.2 controller William Zhang
2023-06-07 8:06 ` Miquel Raynal
2023-06-06 23:12 ` [PATCH 02/12] mtd: rawnand: brcmnand: Fix potential false time out warning William Zhang
2023-06-06 23:12 ` [PATCH 03/12] mtd: rawnand: brcmnand: Fix crash during the panic_write William Zhang
2023-06-06 23:12 ` [PATCH 04/12] mtd: rawnand: brcmnand: Fix potential out-of-bounds access in oob write William Zhang
2023-06-07 8:09 ` Miquel Raynal
2023-06-06 23:12 ` [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs William Zhang
2023-06-07 8:14 ` Miquel Raynal
2023-06-07 20:01 ` William Zhang
2023-06-09 8:58 ` Miquel Raynal
2023-06-09 19:05 ` William Zhang
2023-06-12 17:43 ` Miquel Raynal
2023-06-14 22:46 ` Rob Herring
2023-06-15 0:40 ` William Zhang
2023-06-14 22:43 ` Rob Herring
2023-06-15 0:28 ` William Zhang
2023-06-06 23:12 ` [PATCH 06/12] ARM: dts: broadcom: bcmbca: Add NAND controller node William Zhang
2023-06-06 23:12 ` [PATCH 07/12] arm64: " William Zhang
2023-06-06 23:12 ` [PATCH 08/12] mtd: rawnand: brcmnand: Rename bcm63138 nand driver William Zhang
2023-06-06 23:12 ` [PATCH 09/12] mtd: rawnand: brcmnand: Add new compatible string William Zhang
2023-06-06 23:12 ` [PATCH 10/12] mtd: rawnand: brcmnand: Add BCMBCA read data bus interface William Zhang
2023-06-07 8:20 ` Miquel Raynal
2023-06-07 20:12 ` William Zhang
2023-06-08 6:15 ` Miquel Raynal
2023-06-08 19:04 ` William Zhang
2023-06-07 8:22 ` Miquel Raynal
2023-06-07 20:24 ` William Zhang
2023-06-08 6:18 ` Miquel Raynal
2023-06-08 19:10 ` William Zhang
2023-06-09 8:35 ` Miquel Raynal
2023-06-09 19:16 ` William Zhang
2023-06-12 17:49 ` Miquel Raynal
2023-06-12 17:53 ` Miquel Raynal
2023-06-12 19:18 ` William Zhang [this message]
2023-06-13 6:42 ` Miquel Raynal
2023-06-14 0:00 ` William Zhang
2023-06-14 6:22 ` Miquel Raynal
2023-06-14 23:52 ` William Zhang
2023-06-12 19:03 ` William Zhang
2023-06-11 9:54 ` kernel test robot
2023-06-06 23:12 ` [PATCH 11/12] mtd: rawnand: brcmnand: Add support for getting ecc setting from strap William Zhang
2023-06-07 8:25 ` Miquel Raynal
2023-06-06 23:12 ` [PATCH 12/12] mtd: rawnand: brcmnand: Support write protection setting from dts William Zhang
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=da4cb6a6-aa7d-3747-3f64-19b5582b15e8@broadcom.com \
--to=william.zhang@broadcom.com \
--cc=anand.gore@broadcom.com \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=computersforpeace@gmail.com \
--cc=dan.beygelman@broadcom.com \
--cc=dregan@mail.com \
--cc=f.fainelli@gmail.com \
--cc=joel.peshkin@broadcom.com \
--cc=kamal.dasu@broadcom.com \
--cc=kdasu.kdev@gmail.com \
--cc=kursad.oney@broadcom.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mtd@lists.infradead.org \
--cc=miquel.raynal@bootlin.com \
--cc=rafal@milecki.pl \
--cc=richard@nod.at \
--cc=tomer.yacoby@broadcom.com \
--cc=vigneshr@ti.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 read-only IMAP folder(s) and NNTP newsgroup(s).