Re: [PATCH v2 00/14] ravb/sh_eth: fix sleep in atomic by reusing shared ethtool handlers

[Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>]

On 07/05/2018 08:59 AM, Vladimir Zapolskiy wrote:

>>> For ages trivial changes to RAVB and SuperH ethernet links by means of
>>> standard 'ethtool' trigger a 'sleeping function called from invalid
>>> context' bug, to visualize it on r8a7795 ULCB:
>>  ...
>>> The root cause is that an attempt to modify ECMR and GECMR registers
>>> only when RX/TX function is disabled was too overcomplicated in its
>>> original implementation, also processing of an optional Link Change
>>> interrupt added even more complexity, as a result the implementation
>>> was error prone.
>>>
>>> The new locking scheme is confirmed to be correct by dumping driver
>>> specific and generic PHY framework function calls with aid of ftrace
>>> while running more or less advanced tests.
>>>
>>> Please note that sh_eth patches from the series were built-tested only.
>>>
>>> On purpose I do not add Fixes tags, the reused PHY handlers were added
>>> way later than the fixed problems were firstly found in the drivers.
>>>
>>> Changes from v1 to v2:
>>> * the original patches are split to bugfixes and enhancements only,
>>>   both v1 and v2 series are absolutely equal in total, thus I omit
>>>   description of changes in individual patches,
>>> * the latter implies that there should be no strict need for retesting,
>>>   but because formally two series are different, I have to drop the tags
>>>   given by Geert and Andrew, please send your tags again.
>>
>> These changes look fine to me but I want to see some reviews and/or
>> testing before I apply them.
>>
> 
> Thanks to Geert for finding time to test v1 series, the sums of changes
> are word for word, https://www.spinics.net/lists/linux-renesas-soc/msg28666.html

   I might find a time to test the sh_eth patches on the R-Car V3H Starter Kit board;
I'm still on vacations, so don't have access to the R-Car gen2 boards at home...

> Andrew also gave a number of Reviewed-by tags, but they are not directly
> applicable to new patches, unfortunately.
> 
> In any case let's wait for scrupulous review completed by Sergei, I believe
> he'd like to contribute to the review process, and Sergei may highlight more
> shortcomings.

   Heh. I'm surprised DaveM haven't lectured you about not mixing fixes and cleanups
in the single series -- they go to his different 2 trees, net.git and net-next.git...
If you need your cleanups based on your fixes, you have to indicate that in the cleanups
patchset...

> --
> Best wishes,
> Vladimir

MBR, Sergei