Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 22 Nov 2018 16:59:44 +0100
From:   Maxime Ripard <maxime.ripard@bootlin.com>
To:     Mesih Kilinc <mesihkilinc@gmail.com>
Cc:     devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org, linux-clk@vger.kernel.org,
        linux-gpio@vger.kernel.org, linux-sunxi@googlegroups.com,
        Chen-Yu Tsai <wens@csie.org>,
        Russell King <linux@armlinux.org.uk>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Marc Zyngier <marc.zyngier@arm.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        Icenowy Zheng <icenowy@aosc.io>,
        Rob Herring <robh+dt@kernel.org>,
        Julian Calaby <julian.calaby@gmail.com>
Subject: Re: [RFC PATCH v3 05/17] irqchip/sun4i: add support for suniv
 interrupt controller
Message-ID: <20181122155944.vkx2u4iwwedheor2@flea>
References: <cover.1542824904.git.mesihkilinc@gmail.com>
 <08b40429e46626f4caf8e4d2287b5c4d354e3b7f.1542824904.git.mesihkilinc@gmail.com>
 <20181122083547.srnopylueqx6p2qj@flea>
 <20181122150200.p6e22sp3v4npb42w@ThinkPad>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="h2z3qr6y2agvh63m"
Content-Disposition: inline
In-Reply-To: <20181122150200.p6e22sp3v4npb42w@ThinkPad>
User-Agent: NeoMutt/20180716
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--h2z3qr6y2agvh63m
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Nov 22, 2018 at 06:02:00PM +0300, Mesih Kilinc wrote:
> > > diff --git a/drivers/irqchip/irq-sun4i.c b/drivers/irqchip/irq-sun4i.c
> > > index e3e5b91..7ca4a4d 100644
> > > --- a/drivers/irqchip/irq-sun4i.c
> > > +++ b/drivers/irqchip/irq-sun4i.c
> > > @@ -28,11 +28,21 @@
> > >  #define SUN4I_IRQ_NMI_CTRL_REG		0x0c
> > >  #define SUN4I_IRQ_PENDING_REG(x)	(0x10 + 0x4 * x)
> > >  #define SUN4I_IRQ_FIQ_PENDING_REG(x)	(0x20 + 0x4 * x)
> > > -#define SUN4I_IRQ_ENABLE_REG(x)		(0x40 + 0x4 * x)
> > > -#define SUN4I_IRQ_MASK_REG(x)		(0x50 + 0x4 * x)
> > > +#define SUN4I_IRQ_ENABLE_REG(x)		(irq_ic_data->enable_req_offset + 0=
x4 * x)
> > > +#define SUN4I_IRQ_MASK_REG(x)		(irq_ic_data->mask_req_offset + 0x4 *=
 x)
> >=20
> > You shouldn't have all the values you use passed as argument, so
> > irq_ic_data should be one of them here.
> >
>
> Could you elaborate it a little bit?

You should change your macro to something like:

SUN4I_IRQ_ENABLE_REG(data, x) ((data)->enable_req_offset + 0x4 * x)

That way, you are transparent about the parameters that the macro
expects, and don't rely on some unwritten rule about the variable
name.

> > > +
> > > +static int __init suniv_ic_of_init(struct device_node *node,
> > > +				   struct device_node *parent)
> > > +{
> > > +	irq_ic_data =3D kzalloc(sizeof(struct sunxi_irq_chip_data), GFP_KER=
NEL);
> > > +	if (!irq_ic_data) {
> > > +		pr_err("kzalloc failed!\n");
> > > +		return -ENOMEM;
> > > +	}
> > > +
> > > +	irq_ic_data->enable_req_offset =3D SUNIV_IRQ_ENABLE_REG_OFFSET;
> > > +	irq_ic_data->mask_req_offset =3D SUNIV_IRQ_MASK_REG_OFFSET;
> > > +
> > > +	return sun4i_of_init(node, parent);
> > > +}
> > > +
> > > +IRQCHIP_DECLARE(allwinner_sunvi_ic, "allwinner,suniv-f1c100s-ic", su=
niv_ic_of_init);
> >=20
> > You can even split that addition to a new patch as well.
>=20
> OK. I will do 3 patches. First one will add a struct that holds only
> base and domain. Second one will add register offsets to that struct.=20
> Third one will add f1c100s support. Is that ok?

Sounds perfect :)

Thanks!
Maxime

--=20
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

--h2z3qr6y2agvh63m
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCW/bScAAKCRDj7w1vZxhR
xeTNAQCVVvFSY83AwPAdDSiPgA7y2KdvvK38YF4s6yokph6yXgEAh+yXGenXIuaz
Bx6K/lyvdgkoeZ2bStF5yFF56RD8HwQ=
=FloS
-----END PGP SIGNATURE-----

--h2z3qr6y2agvh63m--