Some background about this.
On original qsdk ipq8064 based firmware there was a big separation from
boot partition and user partition. With boot partition we refer to
partition used to init the router (bootloader, spm firmware and other
internal stuff) With user partition we refer to linux partition and data
partition not used to init the router.
When someone had to write to these boot partition a special mode was
needed, to switch the nand driver to this special configuration.
Upstream version of the nandc driver totally dropped this and the result
is that if someone try to read data from these partition a CRC warning
is printed and if someone try to write that (if for example someone
wants to replace the bootloader) result is a broken system as the data
is badly written.
This series comes to fix this.
A user can declare offset and size of these special partition using the
qcom,boot-pages binding.
An initial implementation of this assumed that the boot-pages started
from the start of the nand but we discover that some device have backup
of these special partition and we can have situation where we have this
partition scheme
- APPSBL (require special mode)
- APPSBLENV (doesn't require special mode)
- ART
- APPSBLBK (back of APPSBL require special mode)
- APPSBLENVBK (back of APPSBLENV doesn't require special mode)
With this configuration we need to declare sparse boot page and we can't
assume boot-pages always starts from the start of the nand.
A user can use this form to declare sparse boot pages
qcom,boot-partitions = <0x0 0x0c80000 0x0c80000 0x0500000>;
The driver internally will parse this array, convert it to nand pages
and check internally on every read/write if this special configuration
should used for that page or the normal one.
The reason for all of this is that qcom FOR SOME REASON, disable ECC for
spare data only for these boot partition and we need to reflect this
special configuration to mute these warning and to permit actually
writing to these pages.
v6:
- Add additional comments on boot partition check
- First reorder struct then make change
- Add additional changes request from Manivannan
- Add review tag for dt commit
v5:
- Rename boot-pages to boot-partitions
- Add additional check to parsing function
- Rename unprotect_spare_data to codeword_fixup
- Add additional info from Manivannan
- Add patch to remove holes in qcom_nand_host struct
v4:
- Fix wrong compatible set for boot-pages (ipq8074 instead of ipq806x)
v3:
- Fix typo in Docmunetation commit desription
- Add items description for uint32-matrix
v2:
- Add fixes from Krzysztof in Documentation
Ansuel Smith (3):
mtd: nand: raw: qcom_nandc: reorder qcom_nand_host struct
mtd: nand: raw: qcom_nandc: add support for unprotected spare data
pages
dt-bindings: mtd: qcom_nandc: document qcom,boot-partitions binding
.../devicetree/bindings/mtd/qcom,nandc.yaml | 27 +++
drivers/mtd/nand/raw/qcom_nandc.c | 214 +++++++++++++++++-
2 files changed, 232 insertions(+), 9 deletions(-)
--
2.36.1
Document new qcom,boot-partition binding used to apply special
read/write layout to boot partitions.
QCOM apply a special layout where spare data is not protected
by ECC for some special pages (used for boot partition). Add
Documentation on how to declare these special pages.
Signed-off-by: Ansuel Smith <[email protected]>
Reviewed-by: Manivannan Sadhasivam <[email protected]>
---
.../devicetree/bindings/mtd/qcom,nandc.yaml | 27 +++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml b/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
index 84ad7ff30121..482a2c068740 100644
--- a/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
+++ b/Documentation/devicetree/bindings/mtd/qcom,nandc.yaml
@@ -102,6 +102,31 @@ allOf:
- const: rx
- const: cmd
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,ipq806x-nand
+
+ then:
+ properties:
+ qcom,boot-partitions:
+ $ref: /schemas/types.yaml#/definitions/uint32-matrix
+ items:
+ items:
+ - description: offset
+ - description: size
+ description:
+ Boot partition use a different layout where the 4 bytes of spare
+ data are not protected by ECC. Use this to declare these special
+ partitions by defining first the offset and then the size.
+
+ It's in the form of <offset1 size1 offset2 size2 offset3 ...>
+ and should be declared in ascending order.
+
+ Refer to the ipq8064 example on how to use this special binding.
+
required:
- compatible
- reg
@@ -135,6 +160,8 @@ examples:
nand-ecc-strength = <4>;
nand-bus-width = <8>;
+ qcom,boot-partitions = <0x0 0x58a0000>;
+
partitions {
compatible = "fixed-partitions";
#address-cells = <1>;
--
2.36.1
Reorder qcom_nand_host to save holes in the struct.
Signed-off-by: Ansuel Smith <[email protected]>
---
drivers/mtd/nand/raw/qcom_nandc.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 1a77542c6d67..7fbbd3e7784c 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -431,11 +431,12 @@ struct qcom_nand_controller {
* and reserved bytes
* @cw_data: the number of bytes within a codeword protected
* by ECC
+ * @ecc_bytes_hw: ECC bytes used by controller hardware for this
+ * chip
+ *
* @use_ecc: request the controller to use ECC for the
* upcoming read/write
* @bch_enabled: flag to tell whether BCH ECC mode is used
- * @ecc_bytes_hw: ECC bytes used by controller hardware for this
- * chip
* @status: value to be returned if NAND_CMD_STATUS command
* is executed
* @last_command: keeps track of last command on this chip. used
@@ -452,11 +453,12 @@ struct qcom_nand_host {
int cs;
int cw_size;
int cw_data;
- bool use_ecc;
- bool bch_enabled;
int ecc_bytes_hw;
int spare_bytes;
int bbm_size;
+
+ bool use_ecc;
+ bool bch_enabled;
u8 status;
int last_command;
--
2.36.1
On Thu, Jun 09, 2022 at 10:37:22PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Jun 09, 2022 at 03:23:42PM +0200, Ansuel Smith wrote:
> > Reorder qcom_nand_host to save holes in the struct.
>
> You forgot to reorder other structs also as I requested :/
>
> Thanks,
> Mani
>
Mhhh I didn't find obvius hole in other struct.
Think I will pass this with dwarf to better check them. Sorry!
Feel free to point them if you notice obvius hole that I didn't notice.
> >
> > Signed-off-by: Ansuel Smith <[email protected]>
> > ---
> > drivers/mtd/nand/raw/qcom_nandc.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> > index 1a77542c6d67..7fbbd3e7784c 100644
> > --- a/drivers/mtd/nand/raw/qcom_nandc.c
> > +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> > @@ -431,11 +431,12 @@ struct qcom_nand_controller {
> > * and reserved bytes
> > * @cw_data: the number of bytes within a codeword protected
> > * by ECC
> > + * @ecc_bytes_hw: ECC bytes used by controller hardware for this
> > + * chip
> > + *
> > * @use_ecc: request the controller to use ECC for the
> > * upcoming read/write
> > * @bch_enabled: flag to tell whether BCH ECC mode is used
> > - * @ecc_bytes_hw: ECC bytes used by controller hardware for this
> > - * chip
> > * @status: value to be returned if NAND_CMD_STATUS command
> > * is executed
> > * @last_command: keeps track of last command on this chip. used
> > @@ -452,11 +453,12 @@ struct qcom_nand_host {
> > int cs;
> > int cw_size;
> > int cw_data;
> > - bool use_ecc;
> > - bool bch_enabled;
> > int ecc_bytes_hw;
> > int spare_bytes;
> > int bbm_size;
> > +
> > + bool use_ecc;
> > + bool bch_enabled;
> > u8 status;
> > int last_command;
> >
> > --
> > 2.36.1
> >
>
> --
> மணிவண்ணன் சதாசிவம்
--
Ansuel
On Thu, Jun 09, 2022 at 03:23:42PM +0200, Ansuel Smith wrote:
> Reorder qcom_nand_host to save holes in the struct.
You forgot to reorder other structs also as I requested :/
Thanks,
Mani
>
> Signed-off-by: Ansuel Smith <[email protected]>
> ---
> drivers/mtd/nand/raw/qcom_nandc.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> index 1a77542c6d67..7fbbd3e7784c 100644
> --- a/drivers/mtd/nand/raw/qcom_nandc.c
> +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> @@ -431,11 +431,12 @@ struct qcom_nand_controller {
> * and reserved bytes
> * @cw_data: the number of bytes within a codeword protected
> * by ECC
> + * @ecc_bytes_hw: ECC bytes used by controller hardware for this
> + * chip
> + *
> * @use_ecc: request the controller to use ECC for the
> * upcoming read/write
> * @bch_enabled: flag to tell whether BCH ECC mode is used
> - * @ecc_bytes_hw: ECC bytes used by controller hardware for this
> - * chip
> * @status: value to be returned if NAND_CMD_STATUS command
> * is executed
> * @last_command: keeps track of last command on this chip. used
> @@ -452,11 +453,12 @@ struct qcom_nand_host {
> int cs;
> int cw_size;
> int cw_data;
> - bool use_ecc;
> - bool bch_enabled;
> int ecc_bytes_hw;
> int spare_bytes;
> int bbm_size;
> +
> + bool use_ecc;
> + bool bch_enabled;
> u8 status;
> int last_command;
>
> --
> 2.36.1
>
--
மணிவண்ணன் சதாசிவம்
On Thu, Jun 09, 2022 at 10:37:22PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Jun 09, 2022 at 03:23:42PM +0200, Ansuel Smith wrote:
> > Reorder qcom_nand_host to save holes in the struct.
>
> You forgot to reorder other structs also as I requested :/
>
> Thanks,
> Mani
>
Hi, I run this commit with pahole tools and it didn't reorder anything
else aside from what i already reordered. Am I missing something here?
> >
> > Signed-off-by: Ansuel Smith <[email protected]>
> > ---
> > drivers/mtd/nand/raw/qcom_nandc.c | 10 ++++++----
> > 1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> > index 1a77542c6d67..7fbbd3e7784c 100644
> > --- a/drivers/mtd/nand/raw/qcom_nandc.c
> > +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> > @@ -431,11 +431,12 @@ struct qcom_nand_controller {
> > * and reserved bytes
> > * @cw_data: the number of bytes within a codeword protected
> > * by ECC
> > + * @ecc_bytes_hw: ECC bytes used by controller hardware for this
> > + * chip
> > + *
> > * @use_ecc: request the controller to use ECC for the
> > * upcoming read/write
> > * @bch_enabled: flag to tell whether BCH ECC mode is used
> > - * @ecc_bytes_hw: ECC bytes used by controller hardware for this
> > - * chip
> > * @status: value to be returned if NAND_CMD_STATUS command
> > * is executed
> > * @last_command: keeps track of last command on this chip. used
> > @@ -452,11 +453,12 @@ struct qcom_nand_host {
> > int cs;
> > int cw_size;
> > int cw_data;
> > - bool use_ecc;
> > - bool bch_enabled;
> > int ecc_bytes_hw;
> > int spare_bytes;
> > int bbm_size;
> > +
> > + bool use_ecc;
> > + bool bch_enabled;
> > u8 status;
> > int last_command;
> >
> > --
> > 2.36.1
> >
>
> --
> மணிவண்ணன் சதாசிவம்
--
Ansuel
On Thu, 09 Jun 2022 15:23:44 +0200, Ansuel Smith wrote:
> Document new qcom,boot-partition binding used to apply special
> read/write layout to boot partitions.
>
> QCOM apply a special layout where spare data is not protected
> by ECC for some special pages (used for boot partition). Add
> Documentation on how to declare these special pages.
>
> Signed-off-by: Ansuel Smith <[email protected]>
> Reviewed-by: Manivannan Sadhasivam <[email protected]>
> ---
> .../devicetree/bindings/mtd/qcom,nandc.yaml | 27 +++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
Reviewed-by: Rob Herring <[email protected]>
On Thu, Jun 09, 2022 at 07:10:33PM +0200, Ansuel Smith wrote:
> On Thu, Jun 09, 2022 at 10:37:22PM +0530, Manivannan Sadhasivam wrote:
> > On Thu, Jun 09, 2022 at 03:23:42PM +0200, Ansuel Smith wrote:
> > > Reorder qcom_nand_host to save holes in the struct.
> >
> > You forgot to reorder other structs also as I requested :/
> >
> > Thanks,
> > Mani
> >
>
> Mhhh I didn't find obvius hole in other struct.
> Think I will pass this with dwarf to better check them. Sorry!
> Feel free to point them if you notice obvius hole that I didn't notice.
>
Sorry, I should be explicit. Please rearrange the members in other structs such
that we could avoid holes (in future also). For instance, in
"struct bam_transaction" u32's and bool are mixed in the middle. You could
organize them like,
struct pointer
struct
u32
bool
And this goes same for all other structs as well.
Thanks,
Mani
> > >
> > > Signed-off-by: Ansuel Smith <[email protected]>
> > > ---
> > > drivers/mtd/nand/raw/qcom_nandc.c | 10 ++++++----
> > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
> > > index 1a77542c6d67..7fbbd3e7784c 100644
> > > --- a/drivers/mtd/nand/raw/qcom_nandc.c
> > > +++ b/drivers/mtd/nand/raw/qcom_nandc.c
> > > @@ -431,11 +431,12 @@ struct qcom_nand_controller {
> > > * and reserved bytes
> > > * @cw_data: the number of bytes within a codeword protected
> > > * by ECC
> > > + * @ecc_bytes_hw: ECC bytes used by controller hardware for this
> > > + * chip
> > > + *
> > > * @use_ecc: request the controller to use ECC for the
> > > * upcoming read/write
> > > * @bch_enabled: flag to tell whether BCH ECC mode is used
> > > - * @ecc_bytes_hw: ECC bytes used by controller hardware for this
> > > - * chip
> > > * @status: value to be returned if NAND_CMD_STATUS command
> > > * is executed
> > > * @last_command: keeps track of last command on this chip. used
> > > @@ -452,11 +453,12 @@ struct qcom_nand_host {
> > > int cs;
> > > int cw_size;
> > > int cw_data;
> > > - bool use_ecc;
> > > - bool bch_enabled;
> > > int ecc_bytes_hw;
> > > int spare_bytes;
> > > int bbm_size;
> > > +
> > > + bool use_ecc;
> > > + bool bch_enabled;
> > > u8 status;
> > > int last_command;
> > >
> > > --
> > > 2.36.1
> > >
> >
> > --
> > மணிவண்ணன் சதாசிவம்
>
> --
> Ansuel
--
மணிவண்ணன் சதாசிவம்