mbox series

[v6,00/13] mtd: rawnand: brcmnand: driver and doc updates

Message ID 20240223034758.13753-1-william.zhang@broadcom.com (mailing list archive)
Headers show
Series mtd: rawnand: brcmnand: driver and doc updates | expand

Message

William Zhang Feb. 23, 2024, 3:47 a.m. UTC
This patch series is an update from the previous version [1] after
exex_op support and fixes (patch 1 to 4 from the previous version.)

It updates all the BCMBCA SoC to support the nand controller and add
functions to handle BCMBCA specific needs on ECC and Write Protection
usage. The device tree document is also updated accordingly with the new
properties needed by the driver.

In addition there is a bug fix for exec_op helper functions, log level
adjustment on uncorrectable ECC error and some coding style fixes.

[1] https://lore.kernel.org/lkml/20230606231252.94838-1-william.zhang@broadcom.com/

Changes in v6:
- Moved style fixes to a separate patch
- Fix style issue
- Add reviewed-by tags
- Add other nand ecc properties to the exclude check list
- Update the brcm,nand-ecc-use-strap property description
- Combine the ecc step size and ecc strength into one get function
- Treat it as error condition if both brcm,nand-ecc-use-strap and nand
ecc dts properties are set
- Add intermediate steps to get the sector size bitfield

Changes in v5:
- Update the commit message
- Add reviewed-by tag
- Update the description of this new property
- Update the description for this ecc strap property
- Add check to make sure brcm,nand-ecc-use-strap and
  nand-ecc-strength/brcm,nand-oob-sector-size can not be used at the
  same time

Changes in v4:
- Fix the commit id in the fixes tag
- Revert the log level change for correctable ecc error
- Split the yaml changes into three patches. This is the first one
- Move the WP pin property to this separate patch and change it to
boolean type.
- Move ecc strap property to this separate patch and remove some
non-binding related text from the description
- Move the board related dts setting from SoC dtsi to board dts
- Move the board related dts setting from SoC dtsi to board dts
- Update the comments for ecc setting selection
- Use the new brcm,wp-not-connected property based on the dts binding
change

Changes in v3:
- Update brcm,nand-use-wp description
- Revert the description change to BCM63168 SoC-specific NAND controller
- Updated bcmbca_read_data_bus comment

Changes in v2:
- Added to patch series
- Added to patch series
- Revert the new compatible string nand-bcmbca
- Drop the BCM63168 compatible fix to avoid any potential ABI
incompatibility issue
- Simplify the explanation for brcm,nand-use-wp
- Keep the interrupt name requirement when interrupt number is specified
- Add nand controller node label for 4908 so it is consistent with other
SoCs and can be referenced by board dts file
- Drop the is_param argument to the read data bus function now that we
have the exec_op API to read the parameter page and ONFI data
- Remove be32_to_cpu from brcmnand_read_data_bus
- Minor cosmetic fixes

David Regan (2):
  mtd: rawnand: brcmnand: exec_op helper functions return type fixes
  mtd: rawnand: brcmnand: update log level messages

William Zhang (11):
  mtd: rawnand: brcmnand: fix style issues
  dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs
  dt-bindings: mtd: brcmnand: Add WP pin connection property
  dt-bindings: mtd: brcmnand: Add ecc strap property
  ARM: dts: broadcom: bcmbca: Add NAND controller node
  arm64: dts: broadcom: bcmbca: Add NAND controller node
  arm64: dts: broadcom: bcmbca: Update router boards
  mtd: rawnand: brcmnand: Rename bcm63138 nand driver
  mtd: rawnand: brcmnand: Add BCMBCA read data bus interface
  mtd: rawnand: brcmnand: Add support for getting ecc setting from strap
  mtd: rawnand: brcmnand: Support write protection setting from dts

 .../bindings/mtd/brcm,brcmnand.yaml           |  44 +++++-
 arch/arm/boot/dts/broadcom/bcm47622.dtsi      |  14 ++
 arch/arm/boot/dts/broadcom/bcm63138.dtsi      |   7 +-
 arch/arm/boot/dts/broadcom/bcm63148.dtsi      |  14 ++
 arch/arm/boot/dts/broadcom/bcm63178.dtsi      |  14 ++
 arch/arm/boot/dts/broadcom/bcm6756.dtsi       |  14 ++
 arch/arm/boot/dts/broadcom/bcm6846.dtsi       |  14 ++
 arch/arm/boot/dts/broadcom/bcm6855.dtsi       |  14 ++
 arch/arm/boot/dts/broadcom/bcm6878.dtsi       |  14 ++
 arch/arm/boot/dts/broadcom/bcm947622.dts      |  10 ++
 arch/arm/boot/dts/broadcom/bcm963138.dts      |  10 ++
 arch/arm/boot/dts/broadcom/bcm963138dvt.dts   |  14 +-
 arch/arm/boot/dts/broadcom/bcm963148.dts      |  10 ++
 arch/arm/boot/dts/broadcom/bcm963178.dts      |  10 ++
 arch/arm/boot/dts/broadcom/bcm96756.dts       |  10 ++
 arch/arm/boot/dts/broadcom/bcm96846.dts       |  10 ++
 arch/arm/boot/dts/broadcom/bcm96855.dts       |  10 ++
 arch/arm/boot/dts/broadcom/bcm96878.dts       |  10 ++
 .../bcmbca/bcm4906-netgear-r8000p.dts         |   5 +
 .../bcmbca/bcm4906-tplink-archer-c2300-v1.dts |   5 +
 .../bcmbca/bcm4908-asus-gt-ac5300.dts         |   6 +-
 .../boot/dts/broadcom/bcmbca/bcm4908.dtsi     |   4 +-
 .../boot/dts/broadcom/bcmbca/bcm4912.dtsi     |  14 ++
 .../boot/dts/broadcom/bcmbca/bcm63146.dtsi    |  14 ++
 .../boot/dts/broadcom/bcmbca/bcm63158.dtsi    |  14 ++
 .../boot/dts/broadcom/bcmbca/bcm6813.dtsi     |  14 ++
 .../boot/dts/broadcom/bcmbca/bcm6856.dtsi     |  14 ++
 .../boot/dts/broadcom/bcmbca/bcm6858.dtsi     |  14 ++
 .../boot/dts/broadcom/bcmbca/bcm94908.dts     |  10 ++
 .../boot/dts/broadcom/bcmbca/bcm94912.dts     |  10 ++
 .../boot/dts/broadcom/bcmbca/bcm963146.dts    |  10 ++
 .../boot/dts/broadcom/bcmbca/bcm963158.dts    |  10 ++
 .../boot/dts/broadcom/bcmbca/bcm96813.dts     |  10 ++
 .../boot/dts/broadcom/bcmbca/bcm96856.dts     |  10 ++
 .../boot/dts/broadcom/bcmbca/bcm96858.dts     |  10 ++
 drivers/mtd/nand/raw/brcmnand/Makefile        |   2 +-
 drivers/mtd/nand/raw/brcmnand/bcm63138_nand.c |  99 ------------
 drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c   | 126 +++++++++++++++
 drivers/mtd/nand/raw/brcmnand/brcmnand.c      | 148 ++++++++++++++----
 drivers/mtd/nand/raw/brcmnand/brcmnand.h      |   2 +
 40 files changed, 650 insertions(+), 144 deletions(-)
 delete mode 100644 drivers/mtd/nand/raw/brcmnand/bcm63138_nand.c
 create mode 100644 drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c

Comments

Florian Fainelli Feb. 23, 2024, 4:20 a.m. UTC | #1
On 2/22/2024 7:47 PM, William Zhang wrote:
> From: David Regan <dregan@broadcom.com>
> 
> Fix return types for exec_op reset and status helper functions.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: http://lists.infradead.org/pipermail/linux-mtd/2023-December/102423.html
> Fixes: 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op implementation")
> Signed-off-by: David Regan <dregan@broadcom.com>
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> Reviewed-by: William Zhang <william.zhang@broadcom.com>

Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Florian Fainelli Feb. 26, 2024, 5:36 p.m. UTC | #2
On 2/22/24 19:47, William Zhang wrote:
> This patch series is an update from the previous version [1] after
> exex_op support and fixes (patch 1 to 4 from the previous version.)
> 
> It updates all the BCMBCA SoC to support the nand controller and add
> functions to handle BCMBCA specific needs on ECC and Write Protection
> usage. The device tree document is also updated accordingly with the new
> properties needed by the driver.
> 
> In addition there is a bug fix for exec_op helper functions, log level
> adjustment on uncorrectable ECC error and some coding style fixes.
> 
> [1] https://lore.kernel.org/lkml/20230606231252.94838-1-william.zhang@broadcom.com/

Miquel, thanks for having applied the patches, we should have discussed 
ahead of time whether you should take the SoC/board-level DTS changes 
through your tree or mine, but it's fine either way and should not lead 
to conflicts in Linus' tree.
Miquel Raynal Feb. 29, 2024, 9:11 a.m. UTC | #3
Hi Florian,

florian.fainelli@broadcom.com wrote on Mon, 26 Feb 2024 09:36:02 -0800:

> On 2/22/24 19:47, William Zhang wrote:
> > This patch series is an update from the previous version [1] after
> > exex_op support and fixes (patch 1 to 4 from the previous version.)
> > 
> > It updates all the BCMBCA SoC to support the nand controller and add
> > functions to handle BCMBCA specific needs on ECC and Write Protection
> > usage. The device tree document is also updated accordingly with the new
> > properties needed by the driver.
> > 
> > In addition there is a bug fix for exec_op helper functions, log level
> > adjustment on uncorrectable ECC error and some coding style fixes.
> > 
> > [1] https://lore.kernel.org/lkml/20230606231252.94838-1-william.zhang@broadcom.com/  
> 
> Miquel, thanks for having applied the patches, we should have discussed ahead of time whether you should take the SoC/board-level DTS changes through your tree or mine, but it's fine either way and should not lead to conflicts in Linus' tree.

I'm sorry for not thinking about this ahead of time, I was also not
Cced on the other patches, I noticed it (told Willliam) and just forgot
about this when I applied the series.

It is currently living in -next so if there is any problem I can still
act.

However for this kind of change I usually apply the bindings and .c
changes independently from the DT patches. I believe there is no
problem having one or the other being merged first, or do I overlook
something?

Cheers,
Miquèl
Florian Fainelli Feb. 29, 2024, 5:32 p.m. UTC | #4
Hi Miquel,

On 2/29/24 01:11, Miquel Raynal wrote:
> Hi Florian,
> 
> florian.fainelli@broadcom.com wrote on Mon, 26 Feb 2024 09:36:02 -0800:
> 
>> On 2/22/24 19:47, William Zhang wrote:
>>> This patch series is an update from the previous version [1] after
>>> exex_op support and fixes (patch 1 to 4 from the previous version.)
>>>
>>> It updates all the BCMBCA SoC to support the nand controller and add
>>> functions to handle BCMBCA specific needs on ECC and Write Protection
>>> usage. The device tree document is also updated accordingly with the new
>>> properties needed by the driver.
>>>
>>> In addition there is a bug fix for exec_op helper functions, log level
>>> adjustment on uncorrectable ECC error and some coding style fixes.
>>>
>>> [1] https://lore.kernel.org/lkml/20230606231252.94838-1-william.zhang@broadcom.com/
>>
>> Miquel, thanks for having applied the patches, we should have discussed ahead of time whether you should take the SoC/board-level DTS changes through your tree or mine, but it's fine either way and should not lead to conflicts in Linus' tree.
> 
> I'm sorry for not thinking about this ahead of time, I was also not
> Cced on the other patches, I noticed it (told Willliam) and just forgot
> about this when I applied the series.

Not a problem.

> 
> It is currently living in -next so if there is any problem I can still
> act.
> 
> However for this kind of change I usually apply the bindings and .c
> changes independently from the DT patches. I believe there is no
> problem having one or the other being merged first, or do I overlook
> something?

That is totally fine my concern was more with you also applying the DTS 
changes which could easily conflict with changes queued up in my ARM SoC 
tree, and also did not have my Signed-off-by/Acked-by tag on them (I was 
waiting on the bindings patch to be Acked-by before giving my own). 
Anyway, let's not make this more complicated than it needs to be, and 
thanks for working with William on these changes!
Miquel Raynal March 14, 2024, 10:04 p.m. UTC | #5
Hi Florian,

miquel.raynal@bootlin.com wrote on Thu, 29 Feb 2024 10:11:01 +0100:

> Hi Florian,
> 
> florian.fainelli@broadcom.com wrote on Mon, 26 Feb 2024 09:36:02 -0800:
> 
> > On 2/22/24 19:47, William Zhang wrote:  
> > > This patch series is an update from the previous version [1] after
> > > exex_op support and fixes (patch 1 to 4 from the previous version.)
> > > 
> > > It updates all the BCMBCA SoC to support the nand controller and add
> > > functions to handle BCMBCA specific needs on ECC and Write Protection
> > > usage. The device tree document is also updated accordingly with the new
> > > properties needed by the driver.
> > > 
> > > In addition there is a bug fix for exec_op helper functions, log level
> > > adjustment on uncorrectable ECC error and some coding style fixes.
> > > 
> > > [1] https://lore.kernel.org/lkml/20230606231252.94838-1-william.zhang@broadcom.com/    
> > 
> > Miquel, thanks for having applied the patches, we should have discussed ahead of time whether you should take the SoC/board-level DTS changes through your tree or mine, but it's fine either way and should not lead to conflicts in Linus' tree.  
> 
> I'm sorry for not thinking about this ahead of time, I was also not
> Cced on the other patches, I noticed it (told Willliam) and just forgot
> about this when I applied the series.
> 
> It is currently living in -next so if there is any problem I can still
> act.
> 
> However for this kind of change I usually apply the bindings and .c
> changes independently from the DT patches. I believe there is no
> problem having one or the other being merged first, or do I overlook
> something?

What the heck /o\ I just understand now my mistake, I am very truly
sorry for that...

You were telling me I should sync with you before taking DT changes,
and I was so convinced I _did_not_ take the DT, when I looked at the
branch I did not understand your point. But I am totally sorry I
actually did take the DTs by mistake and I truly did not notice it.
Confirmation bias I suppose. My very sincere apologies.

As mentioned previously, I was not CC'ed on the DT patches, but I
believe the linux-mtd list was, so the patches didn't appear in my
inbox, and once I was happy with the binding/driver changes I applied
it all without noticing the DT changes had sneaked in.

I'm finally preparing the PR for Linus and I see it now...

I believe the SoC tree is closed now so it's up to you what I should do
with them. Let me know if you want me to keep them in my tree and
forward them to Linus or if I should drop them and you'll take them for
the next cycle. Also, if I keep them, shall I add some tag of yours on
these 3 patches? For the record I did not review them.

Thanks and again, I'm confused. I never apply DT patches like that,
your initial remark was more than legitimate.

Cheers,
Miquèl
Florian Fainelli March 14, 2024, 11:02 p.m. UTC | #6
On 3/14/24 15:04, Miquel Raynal wrote:
> Hi Florian,
> 
> miquel.raynal@bootlin.com wrote on Thu, 29 Feb 2024 10:11:01 +0100:
> 
>> Hi Florian,
>>
>> florian.fainelli@broadcom.com wrote on Mon, 26 Feb 2024 09:36:02 -0800:
>>
>>> On 2/22/24 19:47, William Zhang wrote:
>>>> This patch series is an update from the previous version [1] after
>>>> exex_op support and fixes (patch 1 to 4 from the previous version.)
>>>>
>>>> It updates all the BCMBCA SoC to support the nand controller and add
>>>> functions to handle BCMBCA specific needs on ECC and Write Protection
>>>> usage. The device tree document is also updated accordingly with the new
>>>> properties needed by the driver.
>>>>
>>>> In addition there is a bug fix for exec_op helper functions, log level
>>>> adjustment on uncorrectable ECC error and some coding style fixes.
>>>>
>>>> [1] https://lore.kernel.org/lkml/20230606231252.94838-1-william.zhang@broadcom.com/
>>>
>>> Miquel, thanks for having applied the patches, we should have discussed ahead of time whether you should take the SoC/board-level DTS changes through your tree or mine, but it's fine either way and should not lead to conflicts in Linus' tree.
>>
>> I'm sorry for not thinking about this ahead of time, I was also not
>> Cced on the other patches, I noticed it (told Willliam) and just forgot
>> about this when I applied the series.
>>
>> It is currently living in -next so if there is any problem I can still
>> act.
>>
>> However for this kind of change I usually apply the bindings and .c
>> changes independently from the DT patches. I believe there is no
>> problem having one or the other being merged first, or do I overlook
>> something?
> 
> What the heck /o\ I just understand now my mistake, I am very truly
> sorry for that...
> 
> You were telling me I should sync with you before taking DT changes,
> and I was so convinced I _did_not_ take the DT, when I looked at the
> branch I did not understand your point. But I am totally sorry I
> actually did take the DTs by mistake and I truly did not notice it.
> Confirmation bias I suppose. My very sincere apologies.
> 
> As mentioned previously, I was not CC'ed on the DT patches, but I
> believe the linux-mtd list was, so the patches didn't appear in my
> inbox, and once I was happy with the binding/driver changes I applied
> it all without noticing the DT changes had sneaked in.
> 
> I'm finally preparing the PR for Linus and I see it now...
> 
> I believe the SoC tree is closed now so it's up to you what I should do
> with them. Let me know if you want me to keep them in my tree and
> forward them to Linus or if I should drop them and you'll take them for
> the next cycle. Also, if I keep them, shall I add some tag of yours on
> these 3 patches? For the record I did not review them.

Yes please add my:

Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>

tag, and it's fine I don't expect that we will get conflicts for those 
files.

> 
> Thanks and again, I'm confused. I never apply DT patches like that,
> your initial remark was more than legitimate.

Not a problem!
Miquel Raynal March 14, 2024, 11:03 p.m. UTC | #7
> >> I'm sorry for not thinking about this ahead of time, I was also not
> >> Cced on the other patches, I noticed it (told Willliam) and just forgot
> >> about this when I applied the series.
> >>
> >> It is currently living in -next so if there is any problem I can still
> >> act.
> >>
> >> However for this kind of change I usually apply the bindings and .c
> >> changes independently from the DT patches. I believe there is no
> >> problem having one or the other being merged first, or do I overlook
> >> something?  
> > 
> > What the heck /o\ I just understand now my mistake, I am very truly
> > sorry for that...
> > 
> > You were telling me I should sync with you before taking DT changes,
> > and I was so convinced I _did_not_ take the DT, when I looked at the
> > branch I did not understand your point. But I am totally sorry I
> > actually did take the DTs by mistake and I truly did not notice it.
> > Confirmation bias I suppose. My very sincere apologies.
> > 
> > As mentioned previously, I was not CC'ed on the DT patches, but I
> > believe the linux-mtd list was, so the patches didn't appear in my
> > inbox, and once I was happy with the binding/driver changes I applied
> > it all without noticing the DT changes had sneaked in.
> > 
> > I'm finally preparing the PR for Linus and I see it now...
> > 
> > I believe the SoC tree is closed now so it's up to you what I should do
> > with them. Let me know if you want me to keep them in my tree and
> > forward them to Linus or if I should drop them and you'll take them for
> > the next cycle. Also, if I keep them, shall I add some tag of yours on
> > these 3 patches? For the record I did not review them.  
> 
> Yes please add my:
> 
> Acked-by: Florian Fainelli <florian.fainelli@broadcom.com>
> 
> tag, and it's fine I don't expect that we will get conflicts for those files.
> 
> > 
> > Thanks and again, I'm confused. I never apply DT patches like that,
> > your initial remark was more than legitimate.  
> 
> Not a problem!

Thanks :-)

Miquèl