On Wed, 27 Sep 2017 10:35:51 +0200
Romain Izard <romain.izard.pro@xxxxxxxxx> wrote:
> During backup mode, the contents of all registers will be cleared as the
> SoC will be completely powered down. For a product that boots on NAND
> Flash memory, the bootloader will obviously use the related controller
> to read the Flash and correct any detected error in the memory, before
> handling back control to the kernel's resuming entry point.
>
> But it does not clean the NAND controller registers after use and on its
> side the kernel driver expects the error locator to be powered down and
> in a clean state. Add a resume hook for the PMECC error locator, and
> reset its registers.
>
> Signed-off-by: Romain Izard <romain.izard.pro@xxxxxxxxx>
> ---
> Change in v3:
> * keep the PMECC disabled when not in use, and use atmel_pmecc_resume to
> reset the controller after the bootloader has left it enabled.
>
> drivers/mtd/nand/atmel/nand-controller.c | 3 +++
> drivers/mtd/nand/atmel/pmecc.c | 22 ++++++++++++++--------
> drivers/mtd/nand/atmel/pmecc.h | 1 +
> 3 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mtd/nand/atmel/nand-controller.c b/drivers/mtd/nand/atmel/nand-controller.c
> index f25eca79f4e5..86c2199380c2 100644
> --- a/drivers/mtd/nand/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/atmel/nand-controller.c
> @@ -2530,6 +2530,9 @@ static __maybe_unused int atmel_nand_controller_resume(struct device *dev)
> struct atmel_nand_controller *nc = dev_get_drvdata(dev);
> struct atmel_nand *nand;
>
> + if (nand->pmecc)
> + atmel_pmecc_resume(nand->pmecc);
> +
nand is used uninitialized here, and atmel_pmecc_resume() should be
passed a atmel_pmecc object not a atmel_pmecc_user.
if (nc->pmecc)
atmel_pmecc_resume(nc->pmecc);
> list_for_each_entry(nand, &nc->chips, node) {
> int i;
>
> diff --git a/drivers/mtd/nand/atmel/pmecc.c b/drivers/mtd/nand/atmel/pmecc.c
> index 146af8218314..ff09c0f25dd4 100644
> --- a/drivers/mtd/nand/atmel/pmecc.c
> +++ b/drivers/mtd/nand/atmel/pmecc.c
> @@ -765,6 +765,12 @@ void atmel_pmecc_get_generated_eccbytes(struct atmel_pmecc_user *user,
> }
> EXPORT_SYMBOL_GPL(atmel_pmecc_get_generated_eccbytes);
>
> +void atmel_pmecc_reset(struct atmel_pmecc *pmecc)
> +{
> + writel(PMECC_CTRL_RST, pmecc->regs.base + ATMEL_PMECC_CTRL);
> + writel(PMECC_CTRL_DISABLE, pmecc->regs.base + ATMEL_PMECC_CTRL);
> +}
It's not used outside of this file, so it should have a static
specifier. Anyway, I wonder why you don't expose atmel_pmecc_reset()
directly instead of creating this atmel_pmecc_resume() wrapper.
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
