From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753682AbbISFxt (ORCPT ); Sat, 19 Sep 2015 01:53:49 -0400 Received: from mout.gmx.net ([212.227.17.22]:49170 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751521AbbISFxs (ORCPT ); Sat, 19 Sep 2015 01:53:48 -0400 Subject: Re: Possible Spam [PATCH v2 2/2] ARM: irqchip: mxs: add Alpascale ASM9260 support To: Marc Zyngier References: <1442567922-14538-1-git-send-email-linux@rempel-privat.de> <1442567922-14538-3-git-send-email-linux@rempel-privat.de> <20150918114256.74ab823a@arm.com> Cc: linux-kernel@vger.kernel.org, jason@lakedaemon.net, tglx@linutronix.de, Oleksij Rempel From: Oleksij Rempel Message-ID: <55FCF85E.8060909@rempel-privat.de> Date: Sat, 19 Sep 2015 07:53:34 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20150918114256.74ab823a@arm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="tUaSiVU8qkm14xC3Ea5gXGtiPckrI5hAh" X-Provags-ID: V03:K0:UqgYRIwuyv0STm8OzF2j35NaSrfORmLPixQLU7+TVeS+N4aeuRK Qq/bvnoTGaeOLWbFIC0CY9Y/Q93xokp5vS4WOeCA3LACtppKrr2QdbJBmPFUt4Yxz+BOB7K F1o1CBWlKc5Sehkn0fVEjUSGAJo+jVIJVZQlNZxoWdqqqypO9JlzPodIPib92mvbpSVcnGR 1o6HdSomb3VT6bl9GEluA== X-UI-Out-Filterresults: notjunk:1;V01:K0:ce81mGE/APg=:DYZwKwkF5rY+SnvGNqTJIw me4g7R7YwtYdwGbCVAg52Ihoa7iNL0SbQuUxBp7tenjjR1YTk/yPtnyHdVqK60IsntymSfEwP RBTVyiCEG4wxwLI3uxxxUrpEf1NkZZIwcW+tNSPVU4qY9HT0dRuE1MYVlKBImv6YXonT1DY91 Ehe7bQ2tdNWBLSlKGkahM5BK3RtHPqzAxfbi/bZCNKRsoBO9gALWow3M4/SflCr4lalBJ9cW3 sp0XTb830A0kRuLpnMERhSbe3AKrpsRuEmK4V8pyVxXR7Dkkhs5qNYgf9Pc8qoCRPm3sTwjfj U9WPv2zu0oo6zceHM5wDILa8qfP53sJe+p8b4PKBC14Go0FC5Z/DdDAeixuTN8rbVwHgKxLET LJhSbmu4ZNzOpWgbGAZQ3omzaFXQm2EaX7kFk5LUxkptD1VZ8KTWc8LFFgbdKbMErLDiNJFHv yAA/aI7aFv5MRneykpWiF9SOkjZ/+oUQZLY3NlP1dOnwIi23eiTgGfcFAjWzXemt49KxR+QRN 7hg1ocgdC27O9+qS+4dMdvsz5EFcQ4F0VEcQ+Ainw8V2GTbh+xYDe043A/aLWj3q76PIX8MZK ZzRBglLX5uwTfmMBd4H6pN8ewXAkb/FHOUoexXLWUitDQZBYanVGohRf5wwFOazqRoi0xzfyy JXhy330VvFw49YArbu63jZyIlH10M9qYSuozuvy7s2KNcvENpgjqi5K4VMtn7vV5vXpIR+cVm Ovi+cpwv+xCdD/ZJ 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) --tUaSiVU8qkm14xC3Ea5gXGtiPckrI5hAh Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Am 18.09.2015 um 12:42 schrieb Marc Zyngier: > On Fri, 18 Sep 2015 11:18:42 +0200 > Oleksij Rempel wrote: >=20 >> From: Oleksij Rempel >> >> Freescale iMX23/iMX28 and Alphascale ASM9260 have similar >=20 > Is it Alphascale or Alpascale? You may need to fix the patch title. ok. >=20 >> interrupt collectors. It makes easy to reuse irq-mxs code for ASM9260.= >> Differences between this devices are fallowing: >> - different register offsets >> - different count of intterupt lines per register >> - ASM9260 don't provide reset bit >> - ASM9260 don't support FIQ. >> >> Signed-off-by: Oleksij Rempel >> --- >> drivers/irqchip/Kconfig | 5 ++ >> drivers/irqchip/Makefile | 2 +- >> drivers/irqchip/alphascale_asm9260-icoll.h | 109 ++++++++++++++++++++= +++++++++ >> drivers/irqchip/irq-mxs.c | 106 ++++++++++++++++++++= +++++++- >> 4 files changed, 220 insertions(+), 2 deletions(-) >> create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h >> >=20 > [...] >=20 >> diff --git a/drivers/irqchip/irq-mxs.c b/drivers/irqchip/irq-mxs.c >> index 14374de..1470087 100644 >> --- a/drivers/irqchip/irq-mxs.c >> +++ b/drivers/irqchip/irq-mxs.c >> @@ -1,5 +1,7 @@ >> /* >> * Copyright (C) 2009-2010 Freescale Semiconductor, Inc. All Rights R= eserved. >> + * Copyright (C) 2014 Oleksij Rempel >> + * Add Alphascale ASM9260 support. >> * >> * This program is free software; you can redistribute it and/or modi= fy >> * it under the terms of the GNU General Public License as published = by >> @@ -28,6 +30,8 @@ >> #include >> #include >> =20 >> +#include "alphascale_asm9260-icoll.h" >> + >> /* >> * this device provide 4 offsets for each register: >> * 0x0 - plain read write mode >> @@ -49,6 +53,11 @@ >> =20 >> #define ICOLL_NUM_IRQS 128 >> =20 >> +enum icoll_type { >> + ICOLL, >> + ASM9260_ICOLL, >> +}; >> + >> struct icoll_priv { >> void __iomem *vector; >> void __iomem *levelack; >> @@ -58,10 +67,38 @@ struct icoll_priv { >> /* number of interrupts per register */ >> int ; >> void __iomem *clear; >> + enum icoll_type type; >> }; >> =20 >> static struct icoll_priv icoll_priv; >> static struct irq_domain *icoll_domain; >> +static DEFINE_RAW_SPINLOCK(icoll_lock); >> + >> +/* calculate bit offset depending on number of intterupt per register= */ >> +static u32 icoll_intr_bitshift(struct irq_data *d, u32 bit) >> +{ >> + /* >> + * We expect intr_per_reg to be 4 or 1, it means >> + * "n" will be 3 or 0. >> + */ >> + int n =3D icoll_priv.intr_per_reg - 1; >> + >> + /* >> + * If n =3D 0, "bit" is never shifted. >> + * If n =3D 3, mask lower part of hwirq to convert it >> + * in 0, 1, 2 or 3 and then multiply it by 8 (or shift by 3) >> + */ >> + return bit << ((d->hwirq & n) << n); >> +} >> + >> +/* calculate mem offset depending on number of intterupt per register= */ >> +static void __iomem *icoll_intr_reg(struct irq_data *d) >> +{ >> + int n =3D icoll_priv.intr_per_reg >> 1; >> + >> + /* offset =3D hwirq / intr_per_reg * 0x10 */ >> + return icoll_priv.intr + ((d->hwirq >> n) * 0x10); >> +} >=20 > Please correct me if I'm wrong, but it looks like these function are > only useful when used on ams9260. So why do we need intr_per_reg at > all? MXS doesn't need it (always 1), and ams9260 always need it (always= > 4). Save yourself some previous cycles and simplify the whole thing. ok. >> =20 >> static void icoll_ack_irq(struct irq_data *d) >> { >> @@ -86,12 +123,38 @@ static void icoll_unmask_irq(struct irq_data *d) >> icoll_priv.intr + SET_REG + HW_ICOLL_INTERRUPTn(d->hwirq)); >> } >> =20 >> +static void asm9260_mask_irq(struct irq_data *d) >> +{ >> + raw_spin_lock(&icoll_lock); >> + __raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE), >> + icoll_intr_reg(d) + CLR_REG); >> + raw_spin_unlock(&icoll_lock); >> +} >> + >> +static void asm9260_unmask_irq(struct irq_data *d) >> +{ >> + raw_spin_lock(&icoll_lock); >> + __raw_writel(ASM9260_BM_CLEAR_BIT(d->hwirq), >> + icoll_priv.clear + >> + ASM9260_HW_ICOLL_CLEARn(d->hwirq)); >> + >> + __raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE), >> + icoll_intr_reg(d) + SET_REG); >> + raw_spin_unlock(&icoll_lock); >> +} >=20 > Can you please explain the rational for this lock? mask/unmask use > different registers, and it is not obvious to me what race you are > trying to avoid here. Uff... in one of earliest reviews i was asked to add lock.. I also was asked to add asm9260 to some existing driver. Not sure if it is still making sense. --=20 Regards, Oleksij --tUaSiVU8qkm14xC3Ea5gXGtiPckrI5hAh 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 iF4EAREIAAYFAlX8+F4ACgkQHwImuRkmbWnb1AD+MJWAM0QYSN3GyTtu62vRD/E2 nRmox/prWn2Jz/VmAoMBAJocohApoRlktX9XKaz5sbp2MqTE9aOvbOat9Y9fWbj9 =hTD0 -----END PGP SIGNATURE----- --tUaSiVU8qkm14xC3Ea5gXGtiPckrI5hAh--