Re: [RFC/example] powerpc: add the mpic global timer support

[Scott Wood <scottwood@freescale.com>]

Kumar Gala wrote:
> +		timer@41100 {
> +			compatible = "fsl,mpic-global-timer";
> +			reg = <0x41100 0x204>;
> +			interrupts = <0xf7 0x2>;
> +			interrupt-parent = <&mpic>;
> +		};

Why is this a separate node from the MPIC?  Seems like it should at 
least be a child node, if not just implied by one of the compatibles on 
the mpic node.

> +static ssize_t mpic_tm_timeout_store(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t count)
> +{
> +	struct mpic_tm_priv *priv = dev_get_drvdata(dev);
> +	unsigned long interval = simple_strtoul(buf, NULL, 0);
> +	unsigned long temp;
> +
> +	if (interval > 0x7fffffff) {
> +		dev_dbg(dev, "mpic_tm: interval %lu (in ns) too long\n", interval);
> +		return -EINVAL;
> +	}
> +
> +	interval *= priv->ticks_per_sec;

In nanoseconds, or seconds?

> +static DEVICE_ATTR(timeout, 0660, mpic_tm_timeout_show, mpic_tm_timeout_store);
> +
> +static int __devinit mpic_tm_probe(struct of_device *dev,
> +				const struct of_device_id *match)
> +{
> +	struct device_node *np = dev->node;
> +	struct resource res;
> +	struct mpic_tm_priv *priv;
> +	struct mpic_type *type = match->data;
> +	int has_tcr = type->has_tcr;
> +	u32 busfreq = fsl_get_sys_freq();
> +	int ret = 0;
> +
> +	if (busfreq == 0) {
> +		dev_err(&dev->dev, "mpic_tm: No bus frequency in device tree.\n");
> +		return -ENODEV;
> +	}

Maybe put clock-frequency in the timer (or mpic) node itself?

This seems to try to support non-FSL MPIC timers, but fsl_get_sys_freq() 
wouldn't be appropriate in such a case.

> +	priv = kmalloc(sizeof(struct mpic_tm_priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	spin_lock_init(&priv->lock);
> +	dev_set_drvdata(&dev->dev, priv);
> +
> +	ret = of_address_to_resource(np, 0, &res);
> +	if (ret)
> +		goto out;

of_iomap()?

> +	ret = request_irq(priv->irq, mpic_tm_isr, 0, "mpic timer 0", priv);
> +	if (ret)
> +		goto out;

Hardcoded zero in string?

> +	printk("MPIC global timer init done.\n");

KERN_DEBUG?

-Scott