From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Thu, 12 May 2016 11:50:27 +0200
Subject: [U-Boot] [PATCH] i2c: mvtwsi: avoid writing to
 twsi_control_flags prior to relocation
In-Reply-To: <1463021759-9966-2-git-send-email-judge.packham@gmail.com>
References: <1463021759-9966-1-git-send-email-judge.packham@gmail.com>
 <1463021759-9966-2-git-send-email-judge.packham@gmail.com>
Message-ID: <573451E3.2030005@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Chris,

On 12.05.2016 04:55, Chris Packham wrote:
> In a system where the initial u-boot location is genuinely NOR flash (as
> opposed to RAM or a cache-line setup by a pre-bootloader) writes to the
> data section are problematic. At best these writes have no effect at
> worse they put the flash memory into a status mode which changes the
> executable code underneath us.
>
> Only write to twsi_control_flags once we know we've relocated to RAM.
>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
>
>   drivers/i2c/mvtwsi.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
> index 221ff4f..aee28c4 100644
> --- a/drivers/i2c/mvtwsi.c
> +++ b/drivers/i2c/mvtwsi.c
> @@ -28,6 +28,8 @@
>   #error Driver mvtwsi not supported by SoC or board
>   #endif
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
>   /*
>    * TWSI register structure
>    */
> @@ -297,7 +299,8 @@ static void twsi_reset(struct i2c_adapter *adap)
>   {
>   	struct mvtwsi_registers *twsi = twsi_get_base(adap);
>   	/* ensure controller will be enabled by any twsi*() function */
> -	twsi_control_flags = MVTWSI_CONTROL_TWSIEN;
> +	if (gd->flags & GD_FLG_RELOC)
> +		twsi_control_flags = MVTWSI_CONTROL_TWSIEN;
>   	/* reset controller */
>   	writel(0, &twsi->soft_reset);
>   	/* wait 2 ms -- this is what the Marvell LSP does */

I've stumbled over this global data variable also before and would
very much like to get rid of it. Can't you move this variable into
a (newly created) private data struct instead? This is how it needs
to be done in DM (Driver Model) later as well.

BTW: Your platform does not support DM (yet), right?

Thanks,
Stefan