[U-Boot] [PATCH] i2c: mvtwsi: avoid writing to twsi_control_flags prior to relocation

Stefan Roese sr at denx.de
Fri May 13 07:49:19 CEST 2016


Hi Chris,

On 13.05.2016 03:20, Chris Packham wrote:
> On Fri, May 13, 2016 at 10:35 AM, Chris Packham <judge.packham at gmail.com> wrote:
>> On Thu, May 12, 2016 at 9:50 PM, Stefan Roese <sr at denx.de> wrote:
>>> Hi Chris,
>>>
>>>
>>> On 12.05.2016 04:55, Chris Packham wrote:
> <snip>
>>>>          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?
>>
>> I'll take a look. The other deficiency with my solution is that
>> although it avoids the hang the driver still won't work because the
>> state that is reflected in twsi_control_flags will either cause a new
>> hang or not be updated.
>>
>
> Actually I might need some pointers on this. Where could I keep such a
> private data struct. About the only thing I can think of is either to
> add to arch_global_data or pass a flags variable through the call
> chain.

I've checked and you are correct. Unfortunately we are missing a private 
pointer in the legacy "struct i2c_adapter". So perhaps
the flags variable is the way to go (for the non-DM I2C driver).

Thanks,
Stefan


More information about the U-Boot mailing list