On 11/3/21 11:03 AM, Rafał Miłecki wrote: > On 2021-11-03 18:50, Florian Fainelli wrote: >> On 11/3/21 8:11 AM, Rafał Miłecki wrote: >>> From: Rafał Miłecki <rafal@xxxxxxxxxx> >>> >>> Some boards may use WP-capable controller but still have WP not >>> connected. This change fixes: >>> [ 1.175550] bcm63138_nand ff801800.nand: timeout on status poll >>> (expected c0000040 got c00000c0) >>> [ 1.184524] bcm63138_nand ff801800.nand: nand #WP expected on >>> [ 1.285547] bcm63138_nand ff801800.nand: timeout on status poll >>> (expected c0000040 got c00000c0) >>> [ 1.294516] bcm63138_nand ff801800.nand: nand #WP expected on >>> [ 1.395548] bcm63138_nand ff801800.nand: timeout on status poll >>> (expected c0000040 got c00000c0) >>> [ 1.404517] bcm63138_nand ff801800.nand: nand #WP expected on >>> >>> Signed-off-by: Rafał Miłecki <rafal@xxxxxxxxxx> >>> --- >>> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c >>> b/drivers/mtd/nand/raw/brcmnand/brcmnand.c >>> index f75929783b94..8b6167457f0c 100644 >>> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c >>> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c >>> @@ -714,7 +714,8 @@ static int brcmnand_revision_init(struct >>> brcmnand_controller *ctrl) >>> if (ctrl->nand_version >= 0x0500) >>> ctrl->features |= BRCMNAND_HAS_1K_SECTORS; >>> >>> - if (ctrl->nand_version >= 0x0700) >>> + if (ctrl->nand_version >= 0x0700 && >>> + !of_property_read_bool(ctrl->dev->of_node, "no-wp")) >>> ctrl->features |= BRCMNAND_HAS_WP; >> >> Should not this be a logical OR here or rather, should it be moved out >> of the check on ctrl->nand_version entirely? What revision of the NAND >> controller do you have on that chip? > > It's NAND controller version 0x0701 (v7.1) and I think it's correct. > > I think we want WP enabled on rev 7.0+ unless it was explicitly disabled. True, I somehow got confused about the negative logic, so maybe we need to combine the logic of all of these lines into a single one: if ((ctrl->nand_version >= 0x700 || of_property_read_bool(ctrl->dev->of_node, "brcm,nand-has-wp")) && !of_property_read_bool(ctrl->dev->of_node, "no-wp") such that it is clearer, Kamal, what do you think? -- Florian
