Regression in next with "mfd: twl6040: The chip does not support bulk access"

Peter Ujfalusi peter.ujfalusi at ti.com
Fri Sep 23 12:05:57 PDT 2016


On 09/23/2016 06:24 PM, Santosh Shilimkar wrote:
> On 9/23/2016 7:26 AM, Tony Lindgren wrote:
>> * Peter Ujfalusi <peter.ujfalusi at ti.com> [160923 00:21]:
>>> On 09/22/16 21:07, Tony Lindgren wrote:
> 
> [...]
> 
>>> On which linux-next version you are seeing this?
>>
>> This was with next-20160922. But testing it again this is just another
>> regression caused by "softirq: fix tasklet_kill() and its users", so adding
>> Santosh to Cc.
>>
>> So no need to revert $subject patch.
>>
> So your driver also looks like fiddling with tasklet core structures if
> you got impacted because of the core change.

How is that? None of the drivers in the twl6040 stack uses tasklets
explicitly. The core uses regmap_irq and the drivers use threaded irq when
they need interrupt handling, but no tasklet, no fiddling with internals.

-- 
Péter

> After the merge window
> am going to take stab at bad users of tasklet but below is
> quick snap shot conversation I had with Thomas and Andrew...
> 
> ------------------------------------------------------------------
> B1;2802;0cOn Wed, 21 Sep 2016, Santosh Shilimkar wrote:
>> I requested you to include this patch but now am not sure anymore.
>> Looks like there are almost 30 more users which are directly
>> tweaking 'tasklet_struct' fields and calling other APIs. Hunting them
>> and fixing them probably would be an exercise and also those changes
>> needs those changed drivers to be tested.
>>
>> What do you suggest ? At least this patch needs to be dropped as of now
>> till we can have complete coverage for those bad users.
> 
> Yes, it needs to be dropped. Stephen, can you please revert it from next?
> 
> How to fix this: The only way is to review all tasklet usage sites for
> creative abuse and then fix them one by one. This needs to be done anyway
> because those are ticking timebombs even without changes in the core
> code. I looked at one of the offenders and it's broken today, it's just
> protected by the extremly low probablity to hit the wreckage case.
> 
> What you can do to coerce the developers/maintainers of offending code into
> looking at the mess they created/merged is to implement accessors for the
> tasklet struct fields and replace the open coded fiddling with them.
> 
> Once that is done, rename the struct fields to something which is absurd
> enough to type.  But don't worry, you will find people doing that. I
> catched a few brainwrecks who actually used:
> 
>  irqdesc->core_internal_state__do_not_mess_with_it
> 
> in their code.
> 
> Now after having everything converted to accessors, you can add sanity
> checks into the accessors and emit WARN_ONCE() when they are used in the
> wrong context. That'll make them look and explain why they think that
> fiddling in the internals is a good idea.
> 
> Thanks,
> 
>     tglx
> ------------------------------------------------------------------
> 



More information about the linux-arm-kernel mailing list