Re: [PATCH V2 15/16] MIPS: ralink: add support for periodic timer irq

[Gabor Juhos <juhosg@openwrt.org>]

2013.04.12. 9:27 keltezéssel, John Crispin írta:
> Adds a driver for the periodic timer found on Ralink SoC.
> 
> Signed-off-by: John Crispin <blogic@openwrt.org>
> ---
>  arch/mips/ralink/Makefile |    2 +-
>  arch/mips/ralink/timer.c  |  193 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 194 insertions(+), 1 deletion(-)
>  create mode 100644 arch/mips/ralink/timer.c
> 
> diff --git a/arch/mips/ralink/Makefile b/arch/mips/ralink/Makefile
> index 38cf1a8..e37e0ec 100644
> --- a/arch/mips/ralink/Makefile
> +++ b/arch/mips/ralink/Makefile
> @@ -6,7 +6,7 @@
>  # Copyright (C) 2009-2011 Gabor Juhos <juhosg@openwrt.org>
>  # Copyright (C) 2013 John Crispin <blogic@openwrt.org>
>  
> -obj-y := prom.o of.o reset.o clk.o irq.o
> +obj-y := prom.o of.o reset.o clk.o irq.o timer.o
>  
>  obj-$(CONFIG_SOC_RT288X) += rt288x.o
>  obj-$(CONFIG_SOC_RT305X) += rt305x.o
> diff --git a/arch/mips/ralink/timer.c b/arch/mips/ralink/timer.c

<...>

> +static void rt_timer_disable(struct rt_timer *rt)
> +{
> +	u32 t;
> +
> +	t = rt_timer_r32(rt, TIMER_REG_TMR0CTL);
> +	t &= ~TMR0CTL_ENABLE;
> +	rt_timer_w32(rt, TIMER_REG_TMR0CTL, t);
> +}
> +
> +static int rt_timer_probe(struct platform_device *pdev)
> +{
> +	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	struct rt_timer *rt;
> +	struct clk *clk;
> +
> +	if (!res) {
> +		dev_err(&pdev->dev, "no memory resource found\n");
> +		return -EINVAL;
> +	}
> +
> +	rt = devm_kzalloc(&pdev->dev, sizeof(*rt), GFP_KERNEL);
> +	if (!rt) {
> +		dev_err(&pdev->dev, "failed to allocate memory\n");
> +		return -ENOMEM;
> +	}
> +
> +	rt->irq = platform_get_irq(pdev, 0);
> +	if (!rt->irq) {
> +		dev_err(&pdev->dev, "failed to load irq\n");
> +		return -ENOENT;
> +	}
> +
> +	rt->membase = devm_ioremap_resource(&pdev->dev, res);
> +	if (!rt->membase) {

If devm_ioremap_resource fails, it returns with ERR_PTR thus you should use
IS_ERR(rt->membase) here ...


> +		dev_err(&pdev->dev, "failed to ioremap\n");
> +		return -ENOMEM;

... and 'return PRT_ERR(rt->membase)' here.

> +	}

Also devm_ioremap_resource() provides its own error messages so the
message can be removed.

IIRC, you have fixed this already in one of your internal trees but it seems
that you forgot to fold that fix into this patch.

-Gabor