Re: [PATCH 2/2] hwspinlock: add sunxi hardware spinlock support

[Wilken Gottwalt <wilken.gottwalt@posteo.net>]

Hi Maxime,

On Wed, 18 Nov 2020 16:37:33 +0100
Maxime Ripard <maxime@cerno.tech> wrote:
> Hi Wilken,
> 
> On Wed, Nov 18, 2020 at 11:02:40AM +0100, Wilken Gottwalt wrote:
> > Adds the sunxi_hwspinlock driver and updates makefiles/maintainers.
> > 
> > Signed-off-by: Wilken Gottwalt <wilken.gottwalt@posteo.net>
> 
> A more descriptive commit log would be welcome here, for example
> containing on which SoC this driver can be used, and on which it was
> tested.

can you help me here a bit? I still try to figure out how to do patch sets
properly. Some kernel submitting documentation says everything goes into the
coverletter and other documentation only tells how to split the patches. So
what would be the right way? A quick example based on my patch set would be
really helpful.

> This is the third attempt at that driver, and you can find the previous
> versions here:
> https://patchwork.kernel.org/project/linux-arm-kernel/cover/20200210170143.20007-1-nborisov@suse.com/
> https://lore.kernel.org/patchwork/patch/706512/
> 
> Most of the comments on those series still apply to yours.

Oh, I wrote my driver 2-3 years ago and just prepared it for mainline. I
wasn't aware of other attempts. I really should have checked this. Though,
I really want to get to the point where this driver is good enough for
mainline. Hmmm, it is interesting how similar these drivers are. Looks like
the other developers also got inspired by the already existing hwspinlock
drivers. :D

> Most importantly, this hwspinlock is used to synchronize the ARM cores
> and the ARISC. How did you test this driver?

Yes, you are right, I should have mentioned this. I have a simple test kernel
module for this. But I must admit, testing the ARISC is very hard and I have
no real idea how to do it. Testing the hwspinlocks in general seems to work
with my test kernel module, but I'm not sure if this is really sufficient. I
can provide the code for it if you like. What would be the best way? Github?
Just mailing a patch?

The test module produces these results:

# insmod /lib/modules/5.9.8/kernel/drivers/hwspinlock/sunxi_hwspinlock_test.ko 
[   45.395672] [init] sunxi hwspinlock test driver start
[   45.400775] [init] start test locks
[   45.404263] [run ] testing 32 locks
[   45.407804] [test] testing lock 0 -----
[   45.411652] [test] taking lock attempt #0 succeded
[   45.416438] [test] try taken lock attempt #0
[   45.420735] [test] unlock/take attempt #0
[   45.424752] [test] taking lock attempt #1 succeded
[   45.429556] [test] try taken lock attempt #1
[   45.433823] [test] unlock/take attempt #1
[   45.437862] [test] testing lock 1 -----
[   45.441699] [test] taking lock attempt #0 succeded
[   45.446484] [test] try taken lock attempt #0
[   45.450768] [test] unlock/take attempt #0
[   45.454774] [test] taking lock attempt #1 succeded
[   45.459576] [test] try taken lock attempt #1
[   45.463843] [test] unlock/take attempt #1
.
.
.
[   46.309925] [test] testing lock 30 -----
[   46.313852] [test] taking lock attempt #0 succeded
[   46.318654] [test] try taken lock attempt #0
[   46.322920] [test] unlock/take attempt #0
[   46.326944] [test] taking lock attempt #1 succeded
[   46.331729] [test] try taken lock attempt #1
[   46.335994] [test] unlock/take attempt #1
[   46.340021] [test] testing lock 31 -----
[   46.343947] [test] taking lock attempt #0 succeded
[   46.348749] [test] try taken lock attempt #0
[   46.353016] [test] unlock/take attempt #0
[   46.357040] [test] taking lock attempt #1 succeded
[   46.361825] [test] try taken lock attempt #1
[   46.366090] [test] unlock/take attempt #1
[   46.370112] [init] end test locks

Greetings,
Wilken