* [PATCH 1/2] dt-bindings: mtd: brcmnand: add "no-wp" property
@ 2021-11-03 15:11 ` Rafał Miłecki
0 siblings, 0 replies; 14+ messages in thread
From: Rafał Miłecki @ 2021-11-03 15:11 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Brian Norris, Kamal Dasu
Cc: linux-mtd, devicetree, Florian Fainelli,
bcm-kernel-feedback-list, Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
It's required to properly describe boards without connected WP pin (e.g.
Asus GT-AC5300).
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
index dd5a64969e37..49c7860c0dad 100644
--- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
+++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
@@ -111,6 +111,11 @@ properties:
earlier versions of this core that include WP
type: boolean
+ no-wp:
+ description:
+ This property marks boards with WP pin not connected to the NAND chip.
+ type: boolean
+
patternProperties:
"^nand@[a-f0-9]$":
type: object
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 1/2] dt-bindings: mtd: brcmnand: add "no-wp" property
@ 2021-11-03 15:11 ` Rafał Miłecki
0 siblings, 0 replies; 14+ messages in thread
From: Rafał Miłecki @ 2021-11-03 15:11 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Brian Norris, Kamal Dasu
Cc: linux-mtd, devicetree, Florian Fainelli,
bcm-kernel-feedback-list, Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
It's required to properly describe boards without connected WP pin (e.g.
Asus GT-AC5300).
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
index dd5a64969e37..49c7860c0dad 100644
--- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
+++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
@@ -111,6 +111,11 @@ properties:
earlier versions of this core that include WP
type: boolean
+ no-wp:
+ description:
+ This property marks boards with WP pin not connected to the NAND chip.
+ type: boolean
+
patternProperties:
"^nand@[a-f0-9]$":
type: object
--
2.31.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] mtd: rawnand: brcmnand: support "no-wp" DT property
2021-11-03 15:11 ` Rafał Miłecki
@ 2021-11-03 15:11 ` Rafał Miłecki
-1 siblings, 0 replies; 14+ messages in thread
From: Rafał Miłecki @ 2021-11-03 15:11 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Brian Norris, Kamal Dasu
Cc: linux-mtd, devicetree, Florian Fainelli,
bcm-kernel-feedback-list, Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
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@milecki.pl>
---
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;
else if (of_property_read_bool(ctrl->dev->of_node, "brcm,nand-has-wp"))
ctrl->features |= BRCMNAND_HAS_WP;
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] mtd: rawnand: brcmnand: support "no-wp" DT property
@ 2021-11-03 15:11 ` Rafał Miłecki
0 siblings, 0 replies; 14+ messages in thread
From: Rafał Miłecki @ 2021-11-03 15:11 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Brian Norris, Kamal Dasu
Cc: linux-mtd, devicetree, Florian Fainelli,
bcm-kernel-feedback-list, Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
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@milecki.pl>
---
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;
else if (of_property_read_bool(ctrl->dev->of_node, "brcm,nand-has-wp"))
ctrl->features |= BRCMNAND_HAS_WP;
--
2.31.1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] mtd: rawnand: brcmnand: support "no-wp" DT property
2021-11-03 15:11 ` Rafał Miłecki
@ 2021-11-03 17:50 ` Florian Fainelli
-1 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2021-11-03 17:50 UTC (permalink / raw)
To: Rafał Miłecki, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Rob Herring, Brian Norris, Kamal Dasu
Cc: linux-mtd, devicetree, Florian Fainelli,
bcm-kernel-feedback-list, Rafał Miłecki
On 11/3/21 8:11 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> 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@milecki.pl>
> ---
> 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?
> else if (of_property_read_bool(ctrl->dev->of_node, "brcm,nand-has-wp"))
> ctrl->features |= BRCMNAND_HAS_WP;
>
--
Florian
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] mtd: rawnand: brcmnand: support "no-wp" DT property
@ 2021-11-03 17:50 ` Florian Fainelli
0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2021-11-03 17:50 UTC (permalink / raw)
To: Rafał Miłecki, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Rob Herring, Brian Norris, Kamal Dasu
Cc: linux-mtd, devicetree, Florian Fainelli,
bcm-kernel-feedback-list, Rafał Miłecki
On 11/3/21 8:11 AM, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> 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@milecki.pl>
> ---
> 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?
> else if (of_property_read_bool(ctrl->dev->of_node, "brcm,nand-has-wp"))
> ctrl->features |= BRCMNAND_HAS_WP;
>
--
Florian
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] mtd: rawnand: brcmnand: support "no-wp" DT property
2021-11-03 17:50 ` Florian Fainelli
@ 2021-11-03 18:03 ` Rafał Miłecki
-1 siblings, 0 replies; 14+ messages in thread
From: Rafał Miłecki @ 2021-11-03 18:03 UTC (permalink / raw)
To: Florian Fainelli
Cc: Rafał Miłecki, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Rob Herring, Brian Norris, Kamal Dasu,
linux-mtd, devicetree, bcm-kernel-feedback-list
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@milecki.pl>
>>
>> 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@milecki.pl>
>> ---
>> 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.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] mtd: rawnand: brcmnand: support "no-wp" DT property
@ 2021-11-03 18:03 ` Rafał Miłecki
0 siblings, 0 replies; 14+ messages in thread
From: Rafał Miłecki @ 2021-11-03 18:03 UTC (permalink / raw)
To: Florian Fainelli
Cc: Rafał Miłecki, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Rob Herring, Brian Norris, Kamal Dasu,
linux-mtd, devicetree, bcm-kernel-feedback-list
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@milecki.pl>
>>
>> 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@milecki.pl>
>> ---
>> 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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] mtd: rawnand: brcmnand: support "no-wp" DT property
2021-11-03 18:03 ` Rafał Miłecki
@ 2021-11-03 18:08 ` Florian Fainelli
-1 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2021-11-03 18:08 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rafał Miłecki, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Rob Herring, Brian Norris, Kamal Dasu,
linux-mtd, devicetree, bcm-kernel-feedback-list
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@milecki.pl>
>>>
>>> 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@milecki.pl>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] mtd: rawnand: brcmnand: support "no-wp" DT property
@ 2021-11-03 18:08 ` Florian Fainelli
0 siblings, 0 replies; 14+ messages in thread
From: Florian Fainelli @ 2021-11-03 18:08 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rafał Miłecki, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Rob Herring, Brian Norris, Kamal Dasu,
linux-mtd, devicetree, bcm-kernel-feedback-list
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@milecki.pl>
>>>
>>> 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@milecki.pl>
>>> ---
>>> 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
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mtd: brcmnand: add "no-wp" property
2021-11-03 15:11 ` Rafał Miłecki
@ 2021-11-09 7:49 ` Rafał Miłecki
-1 siblings, 0 replies; 14+ messages in thread
From: Rafał Miłecki @ 2021-11-09 7:49 UTC (permalink / raw)
To: Rafał Miłecki, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Rob Herring, Brian Norris, Kamal Dasu
Cc: linux-mtd, devicetree, Florian Fainelli, bcm-kernel-feedback-list
On 03.11.2021 16:11, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> It's required to properly describe boards without connected WP pin (e.g.
> Asus GT-AC5300).
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> index dd5a64969e37..49c7860c0dad 100644
> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> @@ -111,6 +111,11 @@ properties:
> earlier versions of this core that include WP
> type: boolean
>
> + no-wp:
> + description:
> + This property marks boards with WP pin not connected to the NAND chip.
> + type: boolean
I started rethinking this. Since we already hav "brcm,nand-has-wp"
(boolean), would makes more sense:
1. Add "no-wp" boolean (as proposed in this patch)
2. Add "wp" (or similar) with [0, 1] and deprecate "brcm,nand-has-wp"
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mtd: brcmnand: add "no-wp" property
@ 2021-11-09 7:49 ` Rafał Miłecki
0 siblings, 0 replies; 14+ messages in thread
From: Rafał Miłecki @ 2021-11-09 7:49 UTC (permalink / raw)
To: Rafał Miłecki, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Rob Herring, Brian Norris, Kamal Dasu
Cc: linux-mtd, devicetree, Florian Fainelli, bcm-kernel-feedback-list
On 03.11.2021 16:11, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> It's required to properly describe boards without connected WP pin (e.g.
> Asus GT-AC5300).
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> index dd5a64969e37..49c7860c0dad 100644
> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> @@ -111,6 +111,11 @@ properties:
> earlier versions of this core that include WP
> type: boolean
>
> + no-wp:
> + description:
> + This property marks boards with WP pin not connected to the NAND chip.
> + type: boolean
I started rethinking this. Since we already hav "brcm,nand-has-wp"
(boolean), would makes more sense:
1. Add "no-wp" boolean (as proposed in this patch)
2. Add "wp" (or similar) with [0, 1] and deprecate "brcm,nand-has-wp"
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mtd: brcmnand: add "no-wp" property
2021-11-09 7:49 ` Rafał Miłecki
@ 2021-11-09 10:52 ` Miquel Raynal
-1 siblings, 0 replies; 14+ messages in thread
From: Miquel Raynal @ 2021-11-09 10:52 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rafał Miłecki, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Brian Norris, Kamal Dasu, linux-mtd, devicetree,
Florian Fainelli, bcm-kernel-feedback-list
Hello,
rafal@milecki.pl wrote on Tue, 9 Nov 2021 08:49:36 +0100:
> On 03.11.2021 16:11, Rafał Miłecki wrote:
> > From: Rafał Miłecki <rafal@milecki.pl>
> >
> > It's required to properly describe boards without connected WP pin (e.g.
> > Asus GT-AC5300).
> >
> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > ---
> > Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> > index dd5a64969e37..49c7860c0dad 100644
> > --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> > +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> > @@ -111,6 +111,11 @@ properties:
> > earlier versions of this core that include WP
> > type: boolean
> > > + no-wp:
> > + description:
> > + This property marks boards with WP pin not connected to the NAND chip.
> > + type: boolean
>
> I started rethinking this. Since we already hav "brcm,nand-has-wp"
> (boolean), would makes more sense:
> 1. Add "no-wp" boolean (as proposed in this patch)
> 2. Add "wp" (or similar) with [0, 1] and deprecate "brcm,nand-has-wp"
Maybe this should be a raw NAND wide property, at least in the bindings
for now: nand-wp (such as nand-rb) and this property should contain the
wp line id.
For me, brcm,nand-has-wp means that the nand wp is connected, not that
it "can be" connected. The fact that the controller has a wp pin or not
should be internal to the controller driver (different compatible or hw
version check).
Thanks,
Miquèl
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mtd: brcmnand: add "no-wp" property
@ 2021-11-09 10:52 ` Miquel Raynal
0 siblings, 0 replies; 14+ messages in thread
From: Miquel Raynal @ 2021-11-09 10:52 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rafał Miłecki, Richard Weinberger, Vignesh Raghavendra,
Rob Herring, Brian Norris, Kamal Dasu, linux-mtd, devicetree,
Florian Fainelli, bcm-kernel-feedback-list
Hello,
rafal@milecki.pl wrote on Tue, 9 Nov 2021 08:49:36 +0100:
> On 03.11.2021 16:11, Rafał Miłecki wrote:
> > From: Rafał Miłecki <rafal@milecki.pl>
> >
> > It's required to properly describe boards without connected WP pin (e.g.
> > Asus GT-AC5300).
> >
> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > ---
> > Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> > index dd5a64969e37..49c7860c0dad 100644
> > --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> > +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> > @@ -111,6 +111,11 @@ properties:
> > earlier versions of this core that include WP
> > type: boolean
> > > + no-wp:
> > + description:
> > + This property marks boards with WP pin not connected to the NAND chip.
> > + type: boolean
>
> I started rethinking this. Since we already hav "brcm,nand-has-wp"
> (boolean), would makes more sense:
> 1. Add "no-wp" boolean (as proposed in this patch)
> 2. Add "wp" (or similar) with [0, 1] and deprecate "brcm,nand-has-wp"
Maybe this should be a raw NAND wide property, at least in the bindings
for now: nand-wp (such as nand-rb) and this property should contain the
wp line id.
For me, brcm,nand-has-wp means that the nand wp is connected, not that
it "can be" connected. The fact that the controller has a wp pin or not
should be internal to the controller driver (different compatible or hw
version check).
Thanks,
Miquèl
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-11-09 10:53 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-03 15:11 [PATCH 1/2] dt-bindings: mtd: brcmnand: add "no-wp" property Rafał Miłecki
2021-11-03 15:11 ` Rafał Miłecki
2021-11-03 15:11 ` [PATCH 2/2] mtd: rawnand: brcmnand: support "no-wp" DT property Rafał Miłecki
2021-11-03 15:11 ` Rafał Miłecki
2021-11-03 17:50 ` Florian Fainelli
2021-11-03 17:50 ` Florian Fainelli
2021-11-03 18:03 ` Rafał Miłecki
2021-11-03 18:03 ` Rafał Miłecki
2021-11-03 18:08 ` Florian Fainelli
2021-11-03 18:08 ` Florian Fainelli
2021-11-09 7:49 ` [PATCH 1/2] dt-bindings: mtd: brcmnand: add "no-wp" property Rafał Miłecki
2021-11-09 7:49 ` Rafał Miłecki
2021-11-09 10:52 ` Miquel Raynal
2021-11-09 10:52 ` Miquel Raynal
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.