[PATCH v1 1/3] mtd: physmap: add dual die entry in devicetree binding

[PATCH v1 1/3] mtd: physmap: add dual die entry in devicetree binding

[Bean Huo]

From: beanhuo <beanhuo@micron.com>

This commit adds the dual die entry in the devicetree binding
for the daul die stacked parallel NOR.

Signed-off-by: beanhuo <beanhuo@micron.com>
---
 Documentation/devicetree/bindings/mtd/mtd-physmap.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
index 4a0a48b..782bacc 100644
--- a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
+++ b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
@@ -29,6 +29,8 @@ file systems on embedded devices.
  - use-advanced-sector-protection: boolean to enable support for the
    advanced sector protection (Spansion: PPB - Persistent Protection
    Bits) locking.
+ - dual-die-stack: boolean to enable support for the devices with the
+   two dies in stack.
 
 For JEDEC compatible devices, the following additional properties
 are defined:
-- 
2.7.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v1 1/3] mtd: physmap: add dual die entry in devicetree binding

[Bean Huo]

From: beanhuo <beanhuo@micron.com>

This commit adds the dual die entry in the devicetree binding
for the daul die stacked parallel NOR.

Signed-off-by: beanhuo <beanhuo@micron.com>
---
 Documentation/devicetree/bindings/mtd/mtd-physmap.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
index 4a0a48b..782bacc 100644
--- a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
+++ b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
@@ -29,6 +29,8 @@ file systems on embedded devices.
  - use-advanced-sector-protection: boolean to enable support for the
    advanced sector protection (Spansion: PPB - Persistent Protection
    Bits) locking.
+ - dual-die-stack: boolean to enable support for the devices with the
+   two dies in stack.
 
 For JEDEC compatible devices, the following additional properties
 are defined:
-- 
2.7.4

Re: [PATCH v1 1/3] mtd: physmap: add dual die entry in devicetree binding

[Boris Brezillon]

On Wed, 21 Feb 2018 19:32:32 +0000
Bean Huo <beanhuo@outlook.com> wrote:

> From: beanhuo <beanhuo@micron.com>
> 
> This commit adds the dual die entry in the devicetree binding
> for the daul die stacked parallel NOR.
> 
> Signed-off-by: beanhuo <beanhuo@micron.com>
> ---
>  Documentation/devicetree/bindings/mtd/mtd-physmap.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
> index 4a0a48b..782bacc 100644
> --- a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
> +++ b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
> @@ -29,6 +29,8 @@ file systems on embedded devices.
>   - use-advanced-sector-protection: boolean to enable support for the
>     advanced sector protection (Spansion: PPB - Persistent Protection
>     Bits) locking.
> + - dual-die-stack: boolean to enable support for the devices with the
> +   two dies in stack.

How about making that more future proof and adding a property that
directly contains the number of dies (num-dies)?

>  
>  For JEDEC compatible devices, the following additional properties
>  are defined:



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 1/3] mtd: physmap: add dual die entry in devicetree binding

[Boris Brezillon]

On Wed, 21 Feb 2018 19:32:32 +0000
Bean Huo <beanhuo@outlook.com> wrote:

> From: beanhuo <beanhuo@micron.com>
> 
> This commit adds the dual die entry in the devicetree binding
> for the daul die stacked parallel NOR.
> 
> Signed-off-by: beanhuo <beanhuo@micron.com>
> ---
>  Documentation/devicetree/bindings/mtd/mtd-physmap.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
> index 4a0a48b..782bacc 100644
> --- a/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
> +++ b/Documentation/devicetree/bindings/mtd/mtd-physmap.txt
> @@ -29,6 +29,8 @@ file systems on embedded devices.
>   - use-advanced-sector-protection: boolean to enable support for the
>     advanced sector protection (Spansion: PPB - Persistent Protection
>     Bits) locking.
> + - dual-die-stack: boolean to enable support for the devices with the
> +   two dies in stack.

How about making that more future proof and adding a property that
directly contains the number of dies (num-dies)?

>  
>  For JEDEC compatible devices, the following additional properties
>  are defined:



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v1 1/3] mtd: physmap: add dual die entry in devicetree binding

[Bean Huo]

Boris,
thanks for the review.

>> + - dual-die-stack: boolean to enable support for the devices with the
>> +   two dies in stack.

>How about making that more future proof and adding a property that
>directly contains the number of dies (num-dies)?

Exactly, I also thought here should add a property, through which it can be compatible
with all kinds of PNOR for the future more dies stacked device.
But currently, it doesn't exist  PNOR device with more than two dies, and we
have no idea about 4/6 dies stacked device operation sequence. so I simply add a boolean
entry and try to only enable dual-die stacked PNOR firstly.

Maybe I can try to do that in next version patch. the software implementation is so flexible.

>>
>>  For JEDEC compatible devices, the following additional properties
>>  are defined:


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 1/3] mtd: physmap: add dual die entry in devicetree binding

[Bean Huo]

Boris,
thanks for the review.

>> + - dual-die-stack: boolean to enable support for the devices with the
>> +   two dies in stack.

>How about making that more future proof and adding a property that
>directly contains the number of dies (num-dies)?

Exactly, I also thought here should add a property, through which it can be compatible
with all kinds of PNOR for the future more dies stacked device.
But currently, it doesn't exist  PNOR device with more than two dies, and we
have no idea about 4/6 dies stacked device operation sequence. so I simply add a boolean
entry and try to only enable dual-die stacked PNOR firstly.

Maybe I can try to do that in next version patch. the software implementation is so flexible.

>>
>>  For JEDEC compatible devices, the following additional properties
>>  are defined:

Re: [PATCH v1 1/3] mtd: physmap: add dual die entry in devicetree binding

[Boris Brezillon]

On Fri, 23 Feb 2018 11:07:17 +0000
Bean Huo <beanhuo@outlook.com> wrote:

> Boris,
> thanks for the review.
> 
> >> + - dual-die-stack: boolean to enable support for the devices with the
> >> +   two dies in stack.  
> 
> >How about making that more future proof and adding a property that
> >directly contains the number of dies (num-dies)?  
> 
> Exactly, I also thought here should add a property, through which it can be compatible
> with all kinds of PNOR for the future more dies stacked device.
> But currently, it doesn't exist  PNOR device with more than two dies, and we
> have no idea about 4/6 dies stacked device operation sequence. so I simply add a boolean
> entry and try to only enable dual-die stacked PNOR firstly.

Actually, I think it's better to think about future evolutions now than
adding a new property every time a vendor decides to add more dies (I
guess the number of dies will always be a power-of-2).

> 
> Maybe I can try to do that in next version patch. the software implementation is so flexible.
> 
> >>
> >>  For JEDEC compatible devices, the following additional properties
> >>  are defined:  
> 



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 1/3] mtd: physmap: add dual die entry in devicetree binding

[Boris Brezillon]

On Fri, 23 Feb 2018 11:07:17 +0000
Bean Huo <beanhuo@outlook.com> wrote:

> Boris,
> thanks for the review.
> 
> >> + - dual-die-stack: boolean to enable support for the devices with the
> >> +   two dies in stack.  
> 
> >How about making that more future proof and adding a property that
> >directly contains the number of dies (num-dies)?  
> 
> Exactly, I also thought here should add a property, through which it can be compatible
> with all kinds of PNOR for the future more dies stacked device.
> But currently, it doesn't exist  PNOR device with more than two dies, and we
> have no idea about 4/6 dies stacked device operation sequence. so I simply add a boolean
> entry and try to only enable dual-die stacked PNOR firstly.

Actually, I think it's better to think about future evolutions now than
adding a new property every time a vendor decides to add more dies (I
guess the number of dies will always be a power-of-2).

> 
> Maybe I can try to do that in next version patch. the software implementation is so flexible.
> 
> >>
> >>  For JEDEC compatible devices, the following additional properties
> >>  are defined:  
> 



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v1 2/3] mtd: probe: probe dual die entry from devicetree binding

[Bean Huo]

From: beanhuo <beanhuo@micron.com>

The device_stack property now will be initialized when devicetree
has an entry compatible with "dual_die_stack".

Signed-off-by: beanhuo <beanhuo@micron.com>
---
 drivers/mtd/chips/cfi_probe.c | 7 +++++++
 include/linux/mtd/cfi.h       | 8 ++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
index e8d0164..6182406 100644
--- a/drivers/mtd/chips/cfi_probe.c
+++ b/drivers/mtd/chips/cfi_probe.c
@@ -17,6 +17,7 @@
 #include <linux/mtd/map.h>
 #include <linux/mtd/cfi.h>
 #include <linux/mtd/gen_probe.h>
+#include <linux/of_platform.h>
 
 //#define DEBUG_CFI
 
@@ -159,6 +160,7 @@ static int __xipram cfi_chip_setup(struct map_info *map,
 	int num_erase_regions = cfi_read_query(map, base + (0x10 + 28)*ofs_factor);
 	int i;
 	int addr_unlock1 = 0x555, addr_unlock2 = 0x2AA;
+	struct device_node __maybe_unused *np = map->device_node;
 
 	xip_enable(base, map, cfi);
 #ifdef DEBUG_CFI
@@ -177,6 +179,11 @@ static int __xipram cfi_chip_setup(struct map_info *map,
 
 	cfi->sector_erase_cmd = CMD(0x30);
 
+	cfi->device_stack = CFI_DEVICESTACK_1DIE;
+#ifdef CONFIG_OF
+	if (np && of_property_read_bool(np, "dual-die-stack"))
+		cfi->device_stack = CFI_DEVICESTACK_2DIE;
+#endif
 	/* Read the CFI info structure */
 	xip_disable_qry(base, map, cfi);
 	for (i=0; i<(sizeof(struct cfi_ident) + num_erase_regions * 4); i++)
diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h
index 9b57a9b..bda37a4 100644
--- a/include/linux/mtd/cfi.h
+++ b/include/linux/mtd/cfi.h
@@ -277,11 +277,19 @@ struct cfi_bri_query {
 #define CFI_MODE_CFI	1
 #define CFI_MODE_JEDEC	0
 
+/* These values represent the die count of chip stack.
+ * So far, there only exists two type of stack device,
+ * single or dual die.
+ */
+#define CFI_DEVICESTACK_1DIE	1
+#define CFI_DEVICESTACK_2DIE	2
+
 struct cfi_private {
 	uint16_t cmdset;
 	void *cmdset_priv;
 	int interleave;
 	int device_type;
+	int device_stack;	/* Device stack type, single die or dual die. */
 	int cfi_mode;		/* Are we a JEDEC device pretending to be CFI? */
 	int addr_unlock1;
 	int addr_unlock2;
-- 
2.7.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v1 2/3] mtd: probe: probe dual die entry from devicetree binding

[Bean Huo]

From: beanhuo <beanhuo@micron.com>

The device_stack property now will be initialized when devicetree
has an entry compatible with "dual_die_stack".

Signed-off-by: beanhuo <beanhuo@micron.com>
---
 drivers/mtd/chips/cfi_probe.c | 7 +++++++
 include/linux/mtd/cfi.h       | 8 ++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
index e8d0164..6182406 100644
--- a/drivers/mtd/chips/cfi_probe.c
+++ b/drivers/mtd/chips/cfi_probe.c
@@ -17,6 +17,7 @@
 #include <linux/mtd/map.h>
 #include <linux/mtd/cfi.h>
 #include <linux/mtd/gen_probe.h>
+#include <linux/of_platform.h>
 
 //#define DEBUG_CFI
 
@@ -159,6 +160,7 @@ static int __xipram cfi_chip_setup(struct map_info *map,
 	int num_erase_regions = cfi_read_query(map, base + (0x10 + 28)*ofs_factor);
 	int i;
 	int addr_unlock1 = 0x555, addr_unlock2 = 0x2AA;
+	struct device_node __maybe_unused *np = map->device_node;
 
 	xip_enable(base, map, cfi);
 #ifdef DEBUG_CFI
@@ -177,6 +179,11 @@ static int __xipram cfi_chip_setup(struct map_info *map,
 
 	cfi->sector_erase_cmd = CMD(0x30);
 
+	cfi->device_stack = CFI_DEVICESTACK_1DIE;
+#ifdef CONFIG_OF
+	if (np && of_property_read_bool(np, "dual-die-stack"))
+		cfi->device_stack = CFI_DEVICESTACK_2DIE;
+#endif
 	/* Read the CFI info structure */
 	xip_disable_qry(base, map, cfi);
 	for (i=0; i<(sizeof(struct cfi_ident) + num_erase_regions * 4); i++)
diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h
index 9b57a9b..bda37a4 100644
--- a/include/linux/mtd/cfi.h
+++ b/include/linux/mtd/cfi.h
@@ -277,11 +277,19 @@ struct cfi_bri_query {
 #define CFI_MODE_CFI	1
 #define CFI_MODE_JEDEC	0
 
+/* These values represent the die count of chip stack.
+ * So far, there only exists two type of stack device,
+ * single or dual die.
+ */
+#define CFI_DEVICESTACK_1DIE	1
+#define CFI_DEVICESTACK_2DIE	2
+
 struct cfi_private {
 	uint16_t cmdset;
 	void *cmdset_priv;
 	int interleave;
 	int device_type;
+	int device_stack;	/* Device stack type, single die or dual die. */
 	int cfi_mode;		/* Are we a JEDEC device pretending to be CFI? */
 	int addr_unlock1;
 	int addr_unlock2;
-- 
2.7.4

Re: [PATCH v1 2/3] mtd: probe: probe dual die entry from devicetree binding

[Boris Brezillon]

On Wed, 21 Feb 2018 19:32:34 +0000
Bean Huo <beanhuo@outlook.com> wrote:

> From: beanhuo <beanhuo@micron.com>
> 
> The device_stack property now will be initialized when devicetree
> has an entry compatible with "dual_die_stack".
> 
> Signed-off-by: beanhuo <beanhuo@micron.com>
> ---
>  drivers/mtd/chips/cfi_probe.c | 7 +++++++
>  include/linux/mtd/cfi.h       | 8 ++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
> index e8d0164..6182406 100644
> --- a/drivers/mtd/chips/cfi_probe.c
> +++ b/drivers/mtd/chips/cfi_probe.c
> @@ -17,6 +17,7 @@
>  #include <linux/mtd/map.h>
>  #include <linux/mtd/cfi.h>
>  #include <linux/mtd/gen_probe.h>
> +#include <linux/of_platform.h>
>  
>  //#define DEBUG_CFI
>  
> @@ -159,6 +160,7 @@ static int __xipram cfi_chip_setup(struct map_info *map,
>  	int num_erase_regions = cfi_read_query(map, base + (0x10 + 28)*ofs_factor);
>  	int i;
>  	int addr_unlock1 = 0x555, addr_unlock2 = 0x2AA;
> +	struct device_node __maybe_unused *np = map->device_node;

You can drop this variable...

>  
>  	xip_enable(base, map, cfi);
>  #ifdef DEBUG_CFI
> @@ -177,6 +179,11 @@ static int __xipram cfi_chip_setup(struct map_info *map,
>  
>  	cfi->sector_erase_cmd = CMD(0x30);
>  
> +	cfi->device_stack = CFI_DEVICESTACK_1DIE;
> +#ifdef CONFIG_OF
> +	if (np && of_property_read_bool(np, "dual-die-stack"))
> +		cfi->device_stack = CFI_DEVICESTACK_2DIE;
> +#endif

and simply do:

	if (of_property_read_bool(map->device_node, "dual-die-stack"))
		cfi->device_stack = CFI_DEVICESTACK_2DIE;

>  	/* Read the CFI info structure */
>  	xip_disable_qry(base, map, cfi);
>  	for (i=0; i<(sizeof(struct cfi_ident) + num_erase_regions * 4); i++)
> diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h
> index 9b57a9b..bda37a4 100644
> --- a/include/linux/mtd/cfi.h
> +++ b/include/linux/mtd/cfi.h
> @@ -277,11 +277,19 @@ struct cfi_bri_query {
>  #define CFI_MODE_CFI	1
>  #define CFI_MODE_JEDEC	0
>  
> +/* These values represent the die count of chip stack.
> + * So far, there only exists two type of stack device,
> + * single or dual die.
> + */
> +#define CFI_DEVICESTACK_1DIE	1
> +#define CFI_DEVICESTACK_2DIE	2

Why are these macros needed?

> +
>  struct cfi_private {
>  	uint16_t cmdset;
>  	void *cmdset_priv;
>  	int interleave;
>  	int device_type;
> +	int device_stack;	/* Device stack type, single die or dual die. */

Why not simply:

	int num_dies;

which would contain the number of dies.

>  	int cfi_mode;		/* Are we a JEDEC device pretending to be CFI? */
>  	int addr_unlock1;
>  	int addr_unlock2;



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 2/3] mtd: probe: probe dual die entry from devicetree binding

[Boris Brezillon]

On Wed, 21 Feb 2018 19:32:34 +0000
Bean Huo <beanhuo@outlook.com> wrote:

> From: beanhuo <beanhuo@micron.com>
> 
> The device_stack property now will be initialized when devicetree
> has an entry compatible with "dual_die_stack".
> 
> Signed-off-by: beanhuo <beanhuo@micron.com>
> ---
>  drivers/mtd/chips/cfi_probe.c | 7 +++++++
>  include/linux/mtd/cfi.h       | 8 ++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
> index e8d0164..6182406 100644
> --- a/drivers/mtd/chips/cfi_probe.c
> +++ b/drivers/mtd/chips/cfi_probe.c
> @@ -17,6 +17,7 @@
>  #include <linux/mtd/map.h>
>  #include <linux/mtd/cfi.h>
>  #include <linux/mtd/gen_probe.h>
> +#include <linux/of_platform.h>
>  
>  //#define DEBUG_CFI
>  
> @@ -159,6 +160,7 @@ static int __xipram cfi_chip_setup(struct map_info *map,
>  	int num_erase_regions = cfi_read_query(map, base + (0x10 + 28)*ofs_factor);
>  	int i;
>  	int addr_unlock1 = 0x555, addr_unlock2 = 0x2AA;
> +	struct device_node __maybe_unused *np = map->device_node;

You can drop this variable...

>  
>  	xip_enable(base, map, cfi);
>  #ifdef DEBUG_CFI
> @@ -177,6 +179,11 @@ static int __xipram cfi_chip_setup(struct map_info *map,
>  
>  	cfi->sector_erase_cmd = CMD(0x30);
>  
> +	cfi->device_stack = CFI_DEVICESTACK_1DIE;
> +#ifdef CONFIG_OF
> +	if (np && of_property_read_bool(np, "dual-die-stack"))
> +		cfi->device_stack = CFI_DEVICESTACK_2DIE;
> +#endif

and simply do:

	if (of_property_read_bool(map->device_node, "dual-die-stack"))
		cfi->device_stack = CFI_DEVICESTACK_2DIE;

>  	/* Read the CFI info structure */
>  	xip_disable_qry(base, map, cfi);
>  	for (i=0; i<(sizeof(struct cfi_ident) + num_erase_regions * 4); i++)
> diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h
> index 9b57a9b..bda37a4 100644
> --- a/include/linux/mtd/cfi.h
> +++ b/include/linux/mtd/cfi.h
> @@ -277,11 +277,19 @@ struct cfi_bri_query {
>  #define CFI_MODE_CFI	1
>  #define CFI_MODE_JEDEC	0
>  
> +/* These values represent the die count of chip stack.
> + * So far, there only exists two type of stack device,
> + * single or dual die.
> + */
> +#define CFI_DEVICESTACK_1DIE	1
> +#define CFI_DEVICESTACK_2DIE	2

Why are these macros needed?

> +
>  struct cfi_private {
>  	uint16_t cmdset;
>  	void *cmdset_priv;
>  	int interleave;
>  	int device_type;
> +	int device_stack;	/* Device stack type, single die or dual die. */

Why not simply:

	int num_dies;

which would contain the number of dies.

>  	int cfi_mode;		/* Are we a JEDEC device pretending to be CFI? */
>  	int addr_unlock1;
>  	int addr_unlock2;



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v1 2/3] mtd: probe: probe dual die entry from devicetree binding

[Bean Huo]

Below suggested points will be involved in next patch. Thanks Boris.

>>       int addr_unlock1 = 0x555, addr_unlock2 = 0x2AA;
>> +     struct device_node __maybe_unused *np = map->device_node;

>You can drop this variable...

>>
>>       xip_enable(base, map, cfi);
>>  #ifdef DEBUG_CFI
>> @@ -177,6 +179,11 @@ static int __xipram cfi_chip_setup(struct map_info *map,
>>
>>       cfi->sector_erase_cmd = CMD(0x30);
>>
>> +     cfi->device_stack = CFI_DEVICESTACK_1DIE;
>> +#ifdef CONFIG_OF
>> +     if (np && of_property_read_bool(np, "dual-die-stack"))
>> +             cfi->device_stack = CFI_DEVICESTACK_2DIE;
>> +#endif
>and simply do:

>      if (of_property_read_bool(map->device_node, "dual-die-stack"))
>             cfi->device_stack = CFI_DEVICESTACK_2DIE;

>>       /* Read the CFI info structure */
>>       xip_disable_qry(base, map, cfi);
>>       for (i=0; i<(sizeof(struct cfi_ident) + num_erase_regions * 4); i++)
>> diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h
>> index 9b57a9b..bda37a4 100644
>> --- a/include/linux/mtd/cfi.h
>> +++ b/include/linux/mtd/cfi.h
>> @@ -277,11 +277,19 @@ struct cfi_bri_query {
>>  #define CFI_MODE_CFI 1
>>  #define CFI_MODE_JEDEC       0
>>
>> +/* These values represent the die count of chip stack.
>> + * So far, there only exists two type of stack device,
>> + * single or dual die.
>> + */
>> +#define CFI_DEVICESTACK_1DIE 1
>> +#define CFI_DEVICESTACK_2DIE 2

>Why are these macros needed?

>> +
>>  struct cfi_private {
>>       uint16_t cmdset;
>>       void *cmdset_priv;
>>       int interleave;
>>       int device_type;
>> +     int device_stack;       /* Device stack type, single die or dual die. */

>Why not simply:

>       int num_dies;

>which would contain the number of dies.

>>       int cfi_mode;           /* Are we a JEDEC device pretending to be CFI? */
>>       int addr_unlock1;
>>       int addr_unlock2;

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 2/3] mtd: probe: probe dual die entry from devicetree binding

[Bean Huo]

Below suggested points will be involved in next patch. Thanks Boris.

>>       int addr_unlock1 = 0x555, addr_unlock2 = 0x2AA;
>> +     struct device_node __maybe_unused *np = map->device_node;

>You can drop this variable...

>>
>>       xip_enable(base, map, cfi);
>>  #ifdef DEBUG_CFI
>> @@ -177,6 +179,11 @@ static int __xipram cfi_chip_setup(struct map_info *map,
>>
>>       cfi->sector_erase_cmd = CMD(0x30);
>>
>> +     cfi->device_stack = CFI_DEVICESTACK_1DIE;
>> +#ifdef CONFIG_OF
>> +     if (np && of_property_read_bool(np, "dual-die-stack"))
>> +             cfi->device_stack = CFI_DEVICESTACK_2DIE;
>> +#endif
>and simply do:

>      if (of_property_read_bool(map->device_node, "dual-die-stack"))
>             cfi->device_stack = CFI_DEVICESTACK_2DIE;

>>       /* Read the CFI info structure */
>>       xip_disable_qry(base, map, cfi);
>>       for (i=0; i<(sizeof(struct cfi_ident) + num_erase_regions * 4); i++)
>> diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h
>> index 9b57a9b..bda37a4 100644
>> --- a/include/linux/mtd/cfi.h
>> +++ b/include/linux/mtd/cfi.h
>> @@ -277,11 +277,19 @@ struct cfi_bri_query {
>>  #define CFI_MODE_CFI 1
>>  #define CFI_MODE_JEDEC       0
>>
>> +/* These values represent the die count of chip stack.
>> + * So far, there only exists two type of stack device,
>> + * single or dual die.
>> + */
>> +#define CFI_DEVICESTACK_1DIE 1
>> +#define CFI_DEVICESTACK_2DIE 2

>Why are these macros needed?

>> +
>>  struct cfi_private {
>>       uint16_t cmdset;
>>       void *cmdset_priv;
>>       int interleave;
>>       int device_type;
>> +     int device_stack;       /* Device stack type, single die or dual die. */

>Why not simply:

>       int num_dies;

>which would contain the number of dies.

>>       int cfi_mode;           /* Are we a JEDEC device pretending to be CFI? */
>>       int addr_unlock1;
>>       int addr_unlock2;

[PATCH v1 3/3] drivers: mtd: chips: add support for the dual die stacked PNOR

[Bean Huo]

From: beanhuo <beanhuo@micron.com>

For the dual die stacked parallel NOR flash (PNOR), it comprises
two dies in the stack. These two dies can receive the commands in
parallel, but there is only one die can be selected or accept
commands according to the maximum address line A[max].
For example,Micron parallel NOR MT28FW02GBBA1HPC and Cypress
S70GL02GS, both are dual-die stacked PNOR with 2Gbit density,
and maximum address line A[26], when A[26] == 0, the lower 1Gb die
is selected, when A[26] == 1, the upper 1Gb is selected.

This patch is to check the accessing offset in the chip and rebase
the unlock cycle command base addresses.

Signed-off-by: beanhuo <beanhuo@micron.com>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 144 ++++++++++++++++++++++++++++--------
 1 file changed, 113 insertions(+), 31 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 56aa6b7..8fa2193 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1104,6 +1104,27 @@ do {  \
 
 #endif
 
+/*
+ * The dual die stacked device comprises two identical dies which connected
+ * in parallel.But there is only one die being selected each time according
+ * to maximum address line A[max]. When A[max] == 0, the lower die is selected,
+ * when A[max] == 1, the upper die is selected. This function will reture the
+ * CFI unlock-command base address accrording to accessing address.
+ */
+static loff_t get_cmd_base_address(struct map_info *map, struct flchip *chip,
+		loff_t offset)
+{
+	struct cfi_private *cfi = map->fldrv_priv;
+	unsigned long cmd_base_addr = chip->start;
+
+	if (cfi->device_stack == CFI_DEVICESTACK_2DIE) {
+		if (offset >= (1 << (cfi->cfiq->DevSize - 1)))
+			cmd_base_addr += (1 << (cfi->cfiq->DevSize - 1));
+	}
+
+	return cmd_base_addr;
+}
+
 static inline int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
 {
 	unsigned long cmd_addr;
@@ -1180,12 +1201,18 @@ static inline void otp_enter(struct map_info *map, struct flchip *chip,
 			     loff_t adr, size_t len)
 {
 	struct cfi_private *cfi = map->fldrv_priv;
+	unsigned long  cmd_base_addr;
 
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr);
+
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x88, cfi->addr_unlock1, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x88, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
 
 	INVALIDATE_CACHED_RANGE(map, chip->start + adr, len);
@@ -1195,14 +1222,20 @@ static inline void otp_exit(struct map_info *map, struct flchip *chip,
 			    loff_t adr, size_t len)
 {
 	struct cfi_private *cfi = map->fldrv_priv;
+	unsigned long cmd_base_addr;
 
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr);
+
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x90, cfi->addr_unlock1, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x90, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x00, cfi->addr_unlock1, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x00, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
 
 	INVALIDATE_CACHED_RANGE(map, chip->start + adr, len);
@@ -1566,6 +1599,7 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
 	int ret = 0;
 	map_word oldd;
 	int retry_cnt = 0;
+	unsigned long cmd_base_addr;
 
 	adr += chip->start;
 
@@ -1599,10 +1633,18 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
 	ENABLE_VPP(map);
 	xip_disable(map, chip, adr);
 
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr - chip->start);
+
  retry:
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0xA0, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0xA0, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
 	map_write(map, datum, adr);
 	chip->state = mode;
 
@@ -1807,6 +1849,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 	unsigned long cmd_adr;
 	int z, words;
 	map_word datum;
+	unsigned long cmd_base_addr;
 
 	adr += chip->start;
 	cmd_adr = adr;
@@ -1827,8 +1870,15 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 	ENABLE_VPP(map);
 	xip_disable(map, chip, cmd_adr);
 
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi, cfi->device_type, NULL);
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr - chip->start);
+
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
 
 	/* Write Buffer Load */
 	map_write(map, CMD(0x25), cmd_adr);
@@ -1896,11 +1946,17 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 	 * See e.g.
 	 * http://www.spansion.com/Support/Application%20Notes/MirrorBit_Write_Buffer_Prog_Page_Buffer_Read_AN.pdf
 	 */
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
+
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr - chip->start);
+
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0xF0, cfi->addr_unlock1, chip->start, map, cfi,
+	cfi_send_gen_cmd(0xF0, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
 	xip_enable(map, chip, adr);
 	/* FIXME - should have reset delay before continuing */
@@ -2063,6 +2119,7 @@ static int do_panic_write_oneword(struct map_info *map, struct flchip *chip,
 	map_word oldd;
 	int ret = 0;
 	int i;
+	unsigned long cmd_base_addr;
 
 	adr += chip->start;
 
@@ -2087,10 +2144,18 @@ static int do_panic_write_oneword(struct map_info *map, struct flchip *chip,
 
 	ENABLE_VPP(map);
 
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr - chip->start);
+
 retry:
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0xA0, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0xA0, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
 	map_write(map, datum, adr);
 
 	for (i = 0; i < jiffies_to_usecs(uWriteTimeout); i++) {
@@ -2329,6 +2394,7 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip,
 	unsigned long timeo = jiffies + HZ;
 	DECLARE_WAITQUEUE(wait, current);
 	int ret = 0;
+	unsigned long cmd_base_addr;
 
 	adr += chip->start;
 
@@ -2346,11 +2412,21 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip,
 	ENABLE_VPP(map);
 	xip_disable(map, chip, adr);
 
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x80, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi, cfi->device_type, NULL);
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr - chip->start);
+
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x80, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
 	map_write(map, cfi->sector_erase_cmd, adr);
 
 	chip->state = FL_ERASING;
@@ -2399,7 +2475,7 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip,
 	/* Did we succeed? */
 	if (!chip_good(map, adr, map_word_ff(map))) {
 		/* reset on all failures. */
-		map_write( map, CMD(0xF0), chip->start );
+		map_write(map, CMD(0xF0), cmd_base_addr);
 		/* FIXME - should have reset delay before continuing */
 
 		ret = -EIO;
@@ -2549,6 +2625,7 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
 	struct cfi_private *cfi = map->fldrv_priv;
 	unsigned long timeo;
 	int ret;
+	unsigned long cmd_base_addr;
 
 	mutex_lock(&chip->mutex);
 	ret = get_chip(map, chip, adr + chip->start, FL_LOCKING);
@@ -2559,12 +2636,17 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
 
 	pr_debug("MTD %s(): XXLOCK 0x%08lx len %d\n", __func__, adr, len);
 
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr);
+
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
 	/* PPB entry command */
-	cfi_send_gen_cmd(0xC0, cfi->addr_unlock1, chip->start, map, cfi,
+	cfi_send_gen_cmd(0xC0, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
 
 	if (thunk == DO_XXLOCK_ONEBLOCK_LOCK) {
@@ -2577,8 +2659,8 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
 		 * have to unlock all sectors of this device instead
 		 */
 		chip->state = FL_UNLOCKING;
-		map_write(map, CMD(0x80), chip->start);
-		map_write(map, CMD(0x30), chip->start);
+		map_write(map, CMD(0x80), cmd_base_addr);
+		map_write(map, CMD(0x30), cmd_base_addr);
 	} else if (thunk == DO_XXLOCK_ONEBLOCK_GETLOCK) {
 		chip->state = FL_JEDEC_QUERY;
 		/* Return locked status: 0->locked, 1->unlocked */
@@ -2604,8 +2686,8 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
 	}
 
 	/* Exit BC commands */
-	map_write(map, CMD(0x90), chip->start);
-	map_write(map, CMD(0x00), chip->start);
+	map_write(map, CMD(0x90), cmd_base_addr);
+	map_write(map, CMD(0x00), cmd_base_addr);
 
 	chip->state = FL_READY;
 	put_chip(map, chip, adr + chip->start);
-- 
2.7.4


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v1 3/3] drivers: mtd: chips: add support for the dual die stacked PNOR

[Bean Huo]

From: beanhuo <beanhuo@micron.com>

For the dual die stacked parallel NOR flash (PNOR), it comprises
two dies in the stack. These two dies can receive the commands in
parallel, but there is only one die can be selected or accept
commands according to the maximum address line A[max].
For example,Micron parallel NOR MT28FW02GBBA1HPC and Cypress
S70GL02GS, both are dual-die stacked PNOR with 2Gbit density,
and maximum address line A[26], when A[26] == 0, the lower 1Gb die
is selected, when A[26] == 1, the upper 1Gb is selected.

This patch is to check the accessing offset in the chip and rebase
the unlock cycle command base addresses.

Signed-off-by: beanhuo <beanhuo@micron.com>
---
 drivers/mtd/chips/cfi_cmdset_0002.c | 144 ++++++++++++++++++++++++++++--------
 1 file changed, 113 insertions(+), 31 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 56aa6b7..8fa2193 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -1104,6 +1104,27 @@ do {  \
 
 #endif
 
+/*
+ * The dual die stacked device comprises two identical dies which connected
+ * in parallel.But there is only one die being selected each time according
+ * to maximum address line A[max]. When A[max] == 0, the lower die is selected,
+ * when A[max] == 1, the upper die is selected. This function will reture the
+ * CFI unlock-command base address accrording to accessing address.
+ */
+static loff_t get_cmd_base_address(struct map_info *map, struct flchip *chip,
+		loff_t offset)
+{
+	struct cfi_private *cfi = map->fldrv_priv;
+	unsigned long cmd_base_addr = chip->start;
+
+	if (cfi->device_stack == CFI_DEVICESTACK_2DIE) {
+		if (offset >= (1 << (cfi->cfiq->DevSize - 1)))
+			cmd_base_addr += (1 << (cfi->cfiq->DevSize - 1));
+	}
+
+	return cmd_base_addr;
+}
+
 static inline int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
 {
 	unsigned long cmd_addr;
@@ -1180,12 +1201,18 @@ static inline void otp_enter(struct map_info *map, struct flchip *chip,
 			     loff_t adr, size_t len)
 {
 	struct cfi_private *cfi = map->fldrv_priv;
+	unsigned long  cmd_base_addr;
 
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr);
+
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x88, cfi->addr_unlock1, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x88, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
 
 	INVALIDATE_CACHED_RANGE(map, chip->start + adr, len);
@@ -1195,14 +1222,20 @@ static inline void otp_exit(struct map_info *map, struct flchip *chip,
 			    loff_t adr, size_t len)
 {
 	struct cfi_private *cfi = map->fldrv_priv;
+	unsigned long cmd_base_addr;
 
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr);
+
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x90, cfi->addr_unlock1, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x90, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x00, cfi->addr_unlock1, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x00, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
 
 	INVALIDATE_CACHED_RANGE(map, chip->start + adr, len);
@@ -1566,6 +1599,7 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
 	int ret = 0;
 	map_word oldd;
 	int retry_cnt = 0;
+	unsigned long cmd_base_addr;
 
 	adr += chip->start;
 
@@ -1599,10 +1633,18 @@ static int __xipram do_write_oneword(struct map_info *map, struct flchip *chip,
 	ENABLE_VPP(map);
 	xip_disable(map, chip, adr);
 
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr - chip->start);
+
  retry:
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0xA0, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0xA0, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
 	map_write(map, datum, adr);
 	chip->state = mode;
 
@@ -1807,6 +1849,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 	unsigned long cmd_adr;
 	int z, words;
 	map_word datum;
+	unsigned long cmd_base_addr;
 
 	adr += chip->start;
 	cmd_adr = adr;
@@ -1827,8 +1870,15 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 	ENABLE_VPP(map);
 	xip_disable(map, chip, cmd_adr);
 
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi, cfi->device_type, NULL);
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr - chip->start);
+
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
 
 	/* Write Buffer Load */
 	map_write(map, CMD(0x25), cmd_adr);
@@ -1896,11 +1946,17 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
 	 * See e.g.
 	 * http://www.spansion.com/Support/Application%20Notes/MirrorBit_Write_Buffer_Prog_Page_Buffer_Read_AN.pdf
 	 */
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
+
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr - chip->start);
+
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0xF0, cfi->addr_unlock1, chip->start, map, cfi,
+	cfi_send_gen_cmd(0xF0, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
 	xip_enable(map, chip, adr);
 	/* FIXME - should have reset delay before continuing */
@@ -2063,6 +2119,7 @@ static int do_panic_write_oneword(struct map_info *map, struct flchip *chip,
 	map_word oldd;
 	int ret = 0;
 	int i;
+	unsigned long cmd_base_addr;
 
 	adr += chip->start;
 
@@ -2087,10 +2144,18 @@ static int do_panic_write_oneword(struct map_info *map, struct flchip *chip,
 
 	ENABLE_VPP(map);
 
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr - chip->start);
+
 retry:
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0xA0, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0xA0, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
 	map_write(map, datum, adr);
 
 	for (i = 0; i < jiffies_to_usecs(uWriteTimeout); i++) {
@@ -2329,6 +2394,7 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip,
 	unsigned long timeo = jiffies + HZ;
 	DECLARE_WAITQUEUE(wait, current);
 	int ret = 0;
+	unsigned long cmd_base_addr;
 
 	adr += chip->start;
 
@@ -2346,11 +2412,21 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip,
 	ENABLE_VPP(map);
 	xip_disable(map, chip, adr);
 
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x80, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi, cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi, cfi->device_type, NULL);
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr - chip->start);
+
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x80, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
+			cfi->device_type, NULL);
 	map_write(map, cfi->sector_erase_cmd, adr);
 
 	chip->state = FL_ERASING;
@@ -2399,7 +2475,7 @@ static int __xipram do_erase_oneblock(struct map_info *map, struct flchip *chip,
 	/* Did we succeed? */
 	if (!chip_good(map, adr, map_word_ff(map))) {
 		/* reset on all failures. */
-		map_write( map, CMD(0xF0), chip->start );
+		map_write(map, CMD(0xF0), cmd_base_addr);
 		/* FIXME - should have reset delay before continuing */
 
 		ret = -EIO;
@@ -2549,6 +2625,7 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
 	struct cfi_private *cfi = map->fldrv_priv;
 	unsigned long timeo;
 	int ret;
+	unsigned long cmd_base_addr;
 
 	mutex_lock(&chip->mutex);
 	ret = get_chip(map, chip, adr + chip->start, FL_LOCKING);
@@ -2559,12 +2636,17 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
 
 	pr_debug("MTD %s(): XXLOCK 0x%08lx len %d\n", __func__, adr, len);
 
-	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
+	/* For the dual die device, rebase the command base address according
+	 * to accessing address.
+	 */
+	cmd_base_addr = get_cmd_base_address(map, chip, adr);
+
+	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
-	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, chip->start, map, cfi,
+	cfi_send_gen_cmd(0x55, cfi->addr_unlock2, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
 	/* PPB entry command */
-	cfi_send_gen_cmd(0xC0, cfi->addr_unlock1, chip->start, map, cfi,
+	cfi_send_gen_cmd(0xC0, cfi->addr_unlock1, cmd_base_addr, map, cfi,
 			 cfi->device_type, NULL);
 
 	if (thunk == DO_XXLOCK_ONEBLOCK_LOCK) {
@@ -2577,8 +2659,8 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
 		 * have to unlock all sectors of this device instead
 		 */
 		chip->state = FL_UNLOCKING;
-		map_write(map, CMD(0x80), chip->start);
-		map_write(map, CMD(0x30), chip->start);
+		map_write(map, CMD(0x80), cmd_base_addr);
+		map_write(map, CMD(0x30), cmd_base_addr);
 	} else if (thunk == DO_XXLOCK_ONEBLOCK_GETLOCK) {
 		chip->state = FL_JEDEC_QUERY;
 		/* Return locked status: 0->locked, 1->unlocked */
@@ -2604,8 +2686,8 @@ static int __maybe_unused do_ppb_xxlock(struct map_info *map,
 	}
 
 	/* Exit BC commands */
-	map_write(map, CMD(0x90), chip->start);
-	map_write(map, CMD(0x00), chip->start);
+	map_write(map, CMD(0x90), cmd_base_addr);
+	map_write(map, CMD(0x00), cmd_base_addr);
 
 	chip->state = FL_READY;
 	put_chip(map, chip, adr + chip->start);
-- 
2.7.4

Re: [PATCH v1 3/3] drivers: mtd: chips: add support for the dual die stacked PNOR

[Boris Brezillon]

On Wed, 21 Feb 2018 19:32:37 +0000
Bean Huo <beanhuo@outlook.com> wrote:

> From: beanhuo <beanhuo@micron.com>
> 
> For the dual die stacked parallel NOR flash (PNOR), it comprises
> two dies in the stack. These two dies can receive the commands in
> parallel, but there is only one die can be selected or accept
> commands according to the maximum address line A[max].
> For example,Micron parallel NOR MT28FW02GBBA1HPC and Cypress
> S70GL02GS, both are dual-die stacked PNOR with 2Gbit density,
> and maximum address line A[26], when A[26] == 0, the lower 1Gb die
> is selected, when A[26] == 1, the upper 1Gb is selected.
> 
> This patch is to check the accessing offset in the chip and rebase
> the unlock cycle command base addresses.
> 
> Signed-off-by: beanhuo <beanhuo@micron.com>
> ---
>  drivers/mtd/chips/cfi_cmdset_0002.c | 144 ++++++++++++++++++++++++++++--------
>  1 file changed, 113 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 56aa6b7..8fa2193 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -1104,6 +1104,27 @@ do {  \
>  
>  #endif
>  
> +/*
> + * The dual die stacked device comprises two identical dies which connected

								    ^are

> + * in parallel.But there is only one die being selected each time according

		  ^ missing space after the period

> + * to maximum address line A[max]. When A[max] == 0, the lower die is selected,
> + * when A[max] == 1, the upper die is selected. This function will reture the

								      ^return

> + * CFI unlock-command base address accrording to accessing address.

Is it really just about unlock commands?

> + */
> +static loff_t get_cmd_base_address(struct map_info *map, struct flchip *chip,
> +		loff_t offset)
> +{
> +	struct cfi_private *cfi = map->fldrv_priv;
> +	unsigned long cmd_base_addr = chip->start;
> +
> +	if (cfi->device_stack == CFI_DEVICESTACK_2DIE) {
> +		if (offset >= (1 << (cfi->cfiq->DevSize - 1)))
> +			cmd_base_addr += (1 << (cfi->cfiq->DevSize - 1));
> +	}

Could be done in a more generic way:

	unsigned long die = offset >> (cfi->cfiq->DevSize - 1);

	if (cfi->device_stack == 1) {
		WARN_ON(die);
		return cmd_base_addr;
	}

	cmd_base_addr += die << (cfi->cfiq->DevSize - 1);

> +
> +	return cmd_base_addr;
> +}
> +
>  static inline int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
>  {
>  	unsigned long cmd_addr;
> @@ -1180,12 +1201,18 @@ static inline void otp_enter(struct map_info *map, struct flchip *chip,
>  			     loff_t adr, size_t len)
>  {
>  	struct cfi_private *cfi = map->fldrv_priv;
> +	unsigned long  cmd_base_addr;
>  
> -	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
> +	/* For the dual die device, rebase the command base address according

Please do not use net-style comments.

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v1 3/3] drivers: mtd: chips: add support for the dual die stacked PNOR

[Boris Brezillon]

On Wed, 21 Feb 2018 19:32:37 +0000
Bean Huo <beanhuo@outlook.com> wrote:

> From: beanhuo <beanhuo@micron.com>
> 
> For the dual die stacked parallel NOR flash (PNOR), it comprises
> two dies in the stack. These two dies can receive the commands in
> parallel, but there is only one die can be selected or accept
> commands according to the maximum address line A[max].
> For example,Micron parallel NOR MT28FW02GBBA1HPC and Cypress
> S70GL02GS, both are dual-die stacked PNOR with 2Gbit density,
> and maximum address line A[26], when A[26] == 0, the lower 1Gb die
> is selected, when A[26] == 1, the upper 1Gb is selected.
> 
> This patch is to check the accessing offset in the chip and rebase
> the unlock cycle command base addresses.
> 
> Signed-off-by: beanhuo <beanhuo@micron.com>
> ---
>  drivers/mtd/chips/cfi_cmdset_0002.c | 144 ++++++++++++++++++++++++++++--------
>  1 file changed, 113 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index 56aa6b7..8fa2193 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -1104,6 +1104,27 @@ do {  \
>  
>  #endif
>  
> +/*
> + * The dual die stacked device comprises two identical dies which connected

								    ^are

> + * in parallel.But there is only one die being selected each time according

		  ^ missing space after the period

> + * to maximum address line A[max]. When A[max] == 0, the lower die is selected,
> + * when A[max] == 1, the upper die is selected. This function will reture the

								      ^return

> + * CFI unlock-command base address accrording to accessing address.

Is it really just about unlock commands?

> + */
> +static loff_t get_cmd_base_address(struct map_info *map, struct flchip *chip,
> +		loff_t offset)
> +{
> +	struct cfi_private *cfi = map->fldrv_priv;
> +	unsigned long cmd_base_addr = chip->start;
> +
> +	if (cfi->device_stack == CFI_DEVICESTACK_2DIE) {
> +		if (offset >= (1 << (cfi->cfiq->DevSize - 1)))
> +			cmd_base_addr += (1 << (cfi->cfiq->DevSize - 1));
> +	}

Could be done in a more generic way:

	unsigned long die = offset >> (cfi->cfiq->DevSize - 1);

	if (cfi->device_stack == 1) {
		WARN_ON(die);
		return cmd_base_addr;
	}

	cmd_base_addr += die << (cfi->cfiq->DevSize - 1);

> +
> +	return cmd_base_addr;
> +}
> +
>  static inline int do_read_onechip(struct map_info *map, struct flchip *chip, loff_t adr, size_t len, u_char *buf)
>  {
>  	unsigned long cmd_addr;
> @@ -1180,12 +1201,18 @@ static inline void otp_enter(struct map_info *map, struct flchip *chip,
>  			     loff_t adr, size_t len)
>  {
>  	struct cfi_private *cfi = map->fldrv_priv;
> +	unsigned long  cmd_base_addr;
>  
> -	cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
> +	/* For the dual die device, rebase the command base address according

Please do not use net-style comments.

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v1 3/3] drivers: mtd: chips: add support for the dual die stacked PNOR

[Bean Huo]

Boris,
>>
>> +/*
>> + * The dual die stacked device comprises two identical dies which connected
>                                                                   ^are
fixed in next patch.

>> + * in parallel.But there is only one die being selected each time according

   >               ^ missing space after the period
fixed in next patch.

>> + * to maximum address line A[max]. When A[max] == 0, the lower die is selected,
>> + * when A[max] == 1, the upper die is selected. This function will reture the
>
 >                                                               ^return
>> + * CFI unlock-command base address according to accessing address.
>Is it really just about unlock commands?

Except unlock commands, other commands also need but don't need to add
0x555/0x2AA. And current parallel nor driver already does this way.
so we don't need to change.

>> + */
>> +static loff_t get_cmd_base_address(struct map_info *map, struct flchip *chip,
>> +             loff_t offset)
>> +{
>> +     struct cfi_private *cfi = map->fldrv_priv;
>>
>>+     unsigned long cmd_base_addr = chip->start;
>>+
>> +     if (cfi->device_stack == CFI_DEVICESTACK_2DIE) {
>>+             if (offset >= (1 << (cfi->cfiq->DevSize - 1)))
>>+                     cmd_base_addr += (1 << (cfi->cfiq->DevSize - 1));
>> +     }

>Could be done in a more generic way:

 >    unsigned long die = offset >> (cfi->cfiq->DevSize - 1);

>        if (cfi->device_stack == 1) {
 >               WARN_ON(die);
  >              return cmd_base_addr;
  >      }

   >     cmd_base_addr += die << (cfi->cfiq->DevSize - 1);

it will be included in next patch, thanks.

...
>> -     cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
>> +     /* For the dual die device, rebase the command base address according

>Please do not use net-style comments.

To be honest, I don't know what is net-style comments.

>--
>Boris Brezillon, Bootlin (formerly Free Electrons)
>Embedded Linux and Kernel engineering
>https://bootlin.com

Re: [PATCH v1 3/3] drivers: mtd: chips: add support for the dual die stacked PNOR

[Boris Brezillon]

On Fri, 23 Feb 2018 11:43:21 +0000
Bean Huo <beanhuo@outlook.com> wrote:

> Boris,
> >>
> >> +/*
> >> + * The dual die stacked device comprises two identical dies which connected  
> >                                                                   ^are  
> fixed in next patch.
> 
> >> + * in parallel.But there is only one die being selected each time according  
> 
>    >               ^ missing space after the period  
> fixed in next patch.
> 
> >> + * to maximum address line A[max]. When A[max] == 0, the lower die is selected,
> >> + * when A[max] == 1, the upper die is selected. This function will reture the  
> >
>  >                                                               ^return  
> >> + * CFI unlock-command base address according to accessing address.  
> >Is it really just about unlock commands?  
> 
> Except unlock commands, other commands also need but don't need to add
> 0x555/0x2AA. And current parallel nor driver already does this way.
> so we don't need to change.

Sorry, I don't understand, probably because I lack some background on
CFI. I'll try to document myself if I find some time.

> 
> >> + */
> >> +static loff_t get_cmd_base_address(struct map_info *map, struct flchip *chip,
> >> +             loff_t offset)
> >> +{
> >> +     struct cfi_private *cfi = map->fldrv_priv;
> >>
> >>+     unsigned long cmd_base_addr = chip->start;
> >>+
> >> +     if (cfi->device_stack == CFI_DEVICESTACK_2DIE) {
> >>+             if (offset >= (1 << (cfi->cfiq->DevSize - 1)))
> >>+                     cmd_base_addr += (1 << (cfi->cfiq->DevSize - 1));
> >> +     }  
> 
> >Could be done in a more generic way:  
> 
>  >    unsigned long die = offset >> (cfi->cfiq->DevSize - 1);  
> 
> >        if (cfi->device_stack == 1) {
>  >               WARN_ON(die);
>   >              return cmd_base_addr;
>   >      }  
> 
>    >     cmd_base_addr += die << (cfi->cfiq->DevSize - 1);  
> 
> it will be included in next patch, thanks.
> 
> ...
> >> -     cfi_send_gen_cmd(0xAA, cfi->addr_unlock1, chip->start, map, cfi,
> >> +     /* For the dual die device, rebase the command base address according  
> 
> >Please do not use net-style comments.  
> 
> To be honest, I don't know what is net-style comments.

Net-style comment:

/* blablabla
 * blablabla
 */

Kernel-style comment:

/*
 * blablabla
 * blablabla
 */

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com