Re: [PATCH] can: c_can: use regmap_update_bits() to modify RAMINIT register

[Roger Quadros <rogerq@ti.com>]

+Mark with correct id.

On 12/01/15 11:57, Roger Quadros wrote:
> +Mark,
> 
> On 09/01/15 15:50, Tomi Valkeinen wrote:
>> On 07/01/15 16:32, Roger Quadros wrote:
>>> use of regmap_read() and regmap_write() in c_can_hw_raminit_syscon()
>>> is not safe as the RAMINIT register can be shared between different drivers
>>> at least for TI SoCs.
>>>
>>> To make the modification atomic we switch to using regmap_update_bits().
>>>
>>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>>> ---
>>>  drivers/net/can/c_can/c_can_platform.c | 9 +++++++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
>>> index f363972..364209a 100644
>>> --- a/drivers/net/can/c_can/c_can_platform.c
>>> +++ b/drivers/net/can/c_can/c_can_platform.c
>>> @@ -110,6 +110,10 @@ static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable)
>>>  	 */
>>>  	ctrl &= ~(1 << raminit->bits.start);
>>>  	ctrl |= 1 << raminit->bits.done;
>>> +
>>> +	/* we can't use regmap_update_bits here as it will bypass the write
>>> +	 * if START is already 0 and DONE is already 1.
>>> +	 */
>>>  	regmap_write(raminit->syscon, raminit->reg, ctrl);
>>
>> Can you first use update_bits to change either bit, so that this update
>> will be done?
>>
> 
> Initial condition is START=0, DONE=1.
> 
> Now setting and clearing START bit initiates the start process and will cause the
> DONE bit to be set after a while.
> So unfortunately this doesn't work for us.
> 
> Mark,
> 
> The problem here is we can't use regmap_update_bits() because we need to write a 1 to the DONE bit to clear it and _regmap_update_bits() doesn't allow us to do that because of commit
> d91e8db2c3bb regmap: Suppress noop writes in regmap_update_bits()
> 
> Is reverting it going to cause other issues? If yes then can we have a flag to specify forced update?
> 
> cheers,
> -roger
> [1] - http://lxr.free-electrons.com/source/drivers/base/regmap/regmap.c#L2332
> 
>