Messages in this thread | | | Date | Tue, 24 Nov 2020 08:54:25 -0600 | From | Bjorn Andersson <> | Subject | Re: [PATCH 2/2] hwspinlock: add sunxi hardware spinlock support |
| |
On Mon 23 Nov 12:17 CST 2020, Wilken Gottwalt wrote:
> On Sat, 21 Nov 2020 23:19:00 -0600 > Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > > +static int hwlocks_inuse_show(struct seq_file *seqf, void *unused) > > > +{ > > > + struct sunxi_hwspinlock_data *priv = seqf->private; > > > + int inuse; > > > + > > > + /* getting the status of only the main 32 spinlocks is supported */ > > > + inuse = hweight32(readl(priv->io_base + SPINLOCK_STATUS_REG)); > > > > So this returns how many of the locks are taken? How is that useful? > > It is a way to see if locks were taken from linux or the arisc core without > touching the actual hwspinlock abi or the locks. So it is a nice way to debug > hwspinlocks, hence it is part of debugfs. >
So in a scenario where two remote processors ping-pong the lock between them, this will always read 1 and you won't know why?
> > > + seq_printf(seqf, "%d\n", inuse); [..] > > > + > > > + return 0; > > > +} > > > + > > > +static const struct of_device_id sunxi_hwspinlock_ids[] = { > > > + { .compatible = "allwinner,sun8i-hwspinlock", }, > > > + { .compatible = "allwinner,sun50i-hwspinlock", }, > > > + {}, > > > +}; > > > +MODULE_DEVICE_TABLE(of, sunxi_hwspinlock_ids); > > > + > > > +static struct platform_driver sunxi_hwspinlock_driver = { > > > + .probe = sunxi_hwspinlock_probe, > > > + .remove = sunxi_hwspinlock_remove, > > > + .driver = { > > > + .name = DRIVER_NAME, > > > + .of_match_table = of_match_ptr(sunxi_hwspinlock_ids), > > > > Please avoid of_match_ptr, as this will cause warnings about unused > > variables when COMPILE_TEST without OF. > > So did you mean to leave it out completely? >
Yes, "worst case" is that you include the reference to sunxi_hwspinlock_ids on a build without CONFIG_OF and wasting a little bit of memory.
Using of_match_ptr() with CONFIG_OF=n will result in NULL and as such we'll get a compile warning that nothing references sunxi_hwspinlock_ids - so then that will have to be marked __maybe_unused, or wrapped in an #if...
So better just leave it as: .of_match_table = sunxi_hwspinlock_ids,
Regards, Bjorn
| |