From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751994AbbJLJ3O (ORCPT ); Mon, 12 Oct 2015 05:29:14 -0400 Received: from mout.gmx.net ([212.227.17.21]:61969 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751178AbbJLJ3L (ORCPT ); Mon, 12 Oct 2015 05:29:11 -0400 Subject: Re: [PATCH v6 1/2] ARM: irqchip: mxs: prepare driver for HW with different offsets To: Thomas Gleixner References: <1444482507-6748-1-git-send-email-linux@rempel-privat.de> <1444482507-6748-2-git-send-email-linux@rempel-privat.de> Cc: linux-kernel@vger.kernel.org, marc.zyngier@arm.com, jason@lakedaemon.net From: Oleksij Rempel Message-ID: <561B7D60.4040808@rempel-privat.de> Date: Mon, 12 Oct 2015 11:29:04 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="G5rrsq8GIJJTUiOHdmKt1h1cdCOl30NWi" X-Provags-ID: V03:K0:D0/E6zvtXXxAK2cDgUuvPss5cS2gZl9CjrMLEEFXJbBHoS9pGJV 1dUaVREh3RUKGuZLW3Ey1gm7aweIYbJ8ybqPNe9KehsSPtniXIwVwZVH6vC/4b32hm3jrUJ ffKzQP6vbh7Xb8TJgIE+flDaiiMBC17xm7wE+34yqr66NR7oJy+D2jqgWbpxu3pfYJSdR2U wOgZWbXhQjOZJlb4j0pFA== X-UI-Out-Filterresults: notjunk:1;V01:K0:LlY1l55E89s=:gx5qYhbHmlYdxLhVgsNBTi KWS/DgYC4me+fQQVnWYQBCqDEJvUsresFoAHwErbL7+WT83aKsX5Em2XjG4Hdlvo06mcGX2E+ IfoCTr3ruMIeC4TUxUt3LFxZz8zZJsTKk6lzD+hKbeK4SNk0USpLK3skEC25IxkJE52NThGNh LCTtWkWWUgdpYtqV0NjyzfpXfngpV6H/8VYe+K3gl0tz9diy4oy1DMTWu7gSBFCG0SatQ7f2h 9w+cnOZvRlkFL1jyWRL0dxADwArNQGiXc+cQfVEXcE+YRKqVE+5XbPPSu69TDz5j7FmfLFplJ 3XysUNUTUR7KgOog/lGc6Pfa9WQRDV/aszbq1DUbPcYepAS0Nz8pj6haVEOrhZsh5Be2cy73L xQ/rziFjNPxXIJvH35RlDBVwiRtBUwrHMT/Jog81VzuVWZ8mQkxGad2jeOgv604wBkg7V01Q8 XSOz1lgi7ZbtiY4ZghPwmkJc/fqkD2oiTnuWtWzki55OC0pXkSXtmC0NB4vfx52XaXfwG/p5U blMFVun8SeinDc7pUc3pIoOWt78AaxC0L5hD3xu/B4fvy66BtBgTT2CDErZg98BZ42UweafeJ +39gOCpJ/moVfWRZ0DjUOXyJiwSfyM+scqwq6ef0o5zasaSiq2iofv31dKZxX48eky7DrEElV D5uxGYYV+9KUHMxJKp7PiX/sFCsWCQgbdBjlS1gWjwwHozayWmufJAcRL0s+uMM4+emWa2814 /VTmO+mzY7T/0aMITbA4m5kPdrAMOfURBlR5GwSUGpuyp7fy944aAcc+GXo= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --G5rrsq8GIJJTUiOHdmKt1h1cdCOl30NWi Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Am 11.10.2015 um 19:58 schrieb Thomas Gleixner: > Oleksij, >=20 > On Sat, 10 Oct 2015, Oleksij Rempel wrote: >=20 > The proper subject line starts with: >=20 > irqchip/mxs:=20 >=20 >> Some HW has similar functionality but different register offsets. >> Make sure we can change offsets dynamically. >=20 > The patch does way more than that. I told you in V2 already: >=20 >>> You forgot to mention the other preparatory changes. >=20 > There is still nothing in the changelog. >=20 >> +static void __init icoll_add_domain(struct device_node *np, >> + int num) >> +{ >> + icoll_domain =3D irq_domain_add_linear(np, num, >> + &icoll_irq_domain_ops, NULL); >> + >> + if (!icoll_domain) >> + panic("%s: unable add irq domain", np->full_name); >=20 > 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); >=20 > and >=20 >> + set_handle_irq(icoll_handle_irq); >=20 > 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. >=20 >> +} >> + >> +static void __iomem * __init icoll_init_iobase(struct device_node *np= ) >> +{ >> + void __iomem *icoll_base; >> + >> + icoll_base =3D of_io_request_and_map(np, 0, np->name); >> + if (!icoll_base) >> + panic("%s: unable to map resource", np->full_name); >> + return icoll_base; >> +} >=20 > 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. >=20 >> + icoll_add_domain(np, ICOLL_NUM_IRQS); >> =20 >> - icoll_domain =3D irq_domain_add_linear(np, ICOLL_NUM_IRQS, >> - &icoll_irq_domain_ops, NULL); >> return icoll_domain ? 0 : -ENODEV; >=20 > 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? >=20 > Thanks, >=20 > tglx >=20 --=20 Regards, Oleksij --G5rrsq8GIJJTUiOHdmKt1h1cdCOl30NWi Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iF4EAREIAAYFAlYbfWAACgkQHwImuRkmbWkJlwD+O1eGvuWwBrBE0oAdmO5NC51q Xb1fgoYkf3FThwEUsSgA/jZr4fa/H5E+nHNwvnkPwJcNRvK/4STBzyXOK8zZufkf =xNp6 -----END PGP SIGNATURE----- --G5rrsq8GIJJTUiOHdmKt1h1cdCOl30NWi--