Am 11.10.2015 um 19:58 schrieb Thomas Gleixner:
> Oleksij,
>
> On Sat, 10 Oct 2015, Oleksij Rempel wrote:
>
> The proper subject line starts with:
>
> irqchip/mxs:
>
>> Some HW has similar functionality but different register offsets.
>> Make sure we can change offsets dynamically.
>
> The patch does way more than that. I told you in V2 already:
>
>>> You forgot to mention the other preparatory changes.
>
> There is still nothing in the changelog.
>
>> +static void __init icoll_add_domain(struct device_node *np,
>> + int num)
>> +{
>> + icoll_domain = irq_domain_add_linear(np, num,
>> + &icoll_irq_domain_ops, NULL);
>> +
>> + if (!icoll_domain)
>> + panic("%s: unable add irq domain", np->full_name);
>
> This splitout should be a separate patch with an explanation why you
> add
I assume i can remove this both lines. irq_set_default_host is an
artefact from old code.
>> + irq_set_default_host(icoll_domain);
>
> and
>
>> + set_handle_irq(icoll_handle_irq);
>
> The latter is already done via the DT_MACHINE_START magic. So you
> should it remove there, because otherwise that call is just
> pointless. See the implementation of set_handle_irq.
>
>> +}
>> +
>> +static void __iomem * __init icoll_init_iobase(struct device_node *np)
>> +{
>> + void __iomem *icoll_base;
>> +
>> + icoll_base = of_io_request_and_map(np, 0, np->name);
>> + if (!icoll_base)
>> + panic("%s: unable to map resource", np->full_name);
>> + return icoll_base;
>> +}
>
> The panic() is actually a bug fix, because the old code had a
> WARN_ON() and then happily dereferenced the NULL pointer. So this
> wants to go into a separate patch as well.
>
>> + icoll_add_domain(np, ICOLL_NUM_IRQS);
>>
>> - icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
>> - &icoll_irq_domain_ops, NULL);
>> return icoll_domain ? 0 : -ENODEV;
>
> In case of !icoll_domain this return is not reached as you paniced
> already. So why would we still check icoll_domain?
I'm paniced on !icoll_base, icoll_domain will give only warning.
http://lxr.free-electrons.com/source/kernel/irq/irqdomain.c#L52
Or do i miss some thing?
>
> Thanks,
>
> tglx
>
--
Regards,
Oleksij
signature.asc
Description: OpenPGP digital signature
