On Sat, Feb 14, 2015 at 06:09:39PM +0100, Andrew Lunn wrote:

Hi Russell

quoted

+static struct reset_control_ops pmu_reset_ops = {
+	.reset = pmu_reset_reset,
+	.assert = pmu_reset_assert,
+	.deassert = pmu_reset_deassert,
+};
+
+static struct reset_controller_dev pmu_reset __initdata = {
+	.ops = &pmu_reset_ops,
+	.owner = THIS_MODULE,

Dumb question: Is this still needed? There have been a lot of patches
from Wolfram Sang removing similar THIS_MODULE statements.

If this is not set, then .owner remains NULL; reset_controller_register()
is not wrapped to hide the initialisation of this member.  (IMHO, that's
exactly why hiding it is bad - it has created inconsistencies because
some APIs need an explicit initialisation, others don't.)

quoted

+ * pmu {
+ *	compatible = "marvell,pmu";

Is this maybe too generic? I'm sure Marvell has more than one pmu.
Maybe "marvell,dove-pmu"

Changed.

quoted

+int __init dove_init_pmu(void)
+{
+	struct device_node *np_pmu, *np;
+	struct pmu_data *pmu;
+	int ret, parent_irq;
+
+	/* Lookup the PMU node */
+	np_pmu = of_find_compatible_node(NULL, NULL, "marvell,pmu");
+	if (!np_pmu)
+		return 0;
+
+	pmu = kzalloc(sizeof(*pmu), GFP_KERNEL);
+	if (!pmu)
+		return -ENOMEM;
+
+	spin_lock_init(&pmu->lock);
+	pmu->of_node = np_pmu;
+	pmu->pmc_base = of_iomap(pmu->of_node, 0);
+	pmu->pmu_base = of_iomap(pmu->of_node, 1);
+	if (!pmu->pmc_base || !pmu->pmu_base) {
+		pr_err("%s: failed to map PMU\n", np_pmu->name);
+		iounmap(pmu->pmu_base);
+		iounmap(pmu->pmc_base);
+		kfree(pmu);
+		return -ENOMEM;
+	}
+
+	parent_irq = irq_of_parse_and_map(pmu->of_node, 0);
+	if (!parent_irq)
+		pr_err("%s: no interrupt specified\n", np_pmu->name);
+

Is it not fatal to be missing the interrupt? Seems like return -EINVAL
would be a good idea?

I don't think so.  The lack of parent interrupt shouldn't stop the
rest of the PMU code initialising - least of all only because the
only user of this right now is the RTC.

It may make sense to avoid initialising the PMU's IRQ support in
that case though.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.