All of lore.kernel.org
 help / color / mirror / Atom feed
From: antoine.tenart@free-electrons.com (Antoine Tenart)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/6] irqchip: add the Alpine MSIX interrupt controller
Date: Mon, 8 Feb 2016 11:44:07 +0100	[thread overview]
Message-ID: <20160208104407.GB4117@kwain> (raw)
In-Reply-To: <56B86EA8.9070306@arm.com>

On Mon, Feb 08, 2016 at 10:32:08AM +0000, Marc Zyngier wrote:
> On 08/02/16 10:26, Antoine Tenart wrote:
> >>> +static int alpine_msix_init(struct device_node *node,
> >>> +			    struct device_node *parent)
> >>> +{
> >>> +	struct alpine_msix_data *priv;
> >>> +	struct resource res;
> >>> +	int ret;
> >>> +
> >>> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> >>> +	if (!priv)
> >>> +		return -ENOMEM;
> >>> +
> >>> +	spin_lock_init(&priv->msi_map_lock);
> >>> +
> >>> +	ret = of_address_to_resource(node, 0, &res);
> >>> +	if (ret) {
> >>> +		pr_err("Failed to allocate resource\n");
> >>> +		goto err_priv;
> >>> +	}
> >>> +
> >>> +	priv->addr_high = upper_32_bits((u64)res.start);
> >>> +	priv->addr_low = lower_32_bits(res.start) + ALPINE_MSIX_SPI_TARGET_CLUSTER0;
> >>
> >> This is a bit odd. If you always set bit 16, why isn't that reflected in
> >> the base address coming from the DT?
> > 
> > The 20 least significant bits of addr_low provide direct information
> > regarding the interrupt destination, so I thought it would be clearer
> > to have this explicitly in the driver so that we know what those bits
> > mean.
> 
> So what is this information? TARGET_CLUSTER0 is not very expressive, and
> doesn't show what the alternatives are. Could you please elaborate a bit
> on that front?

For now lots of bits are reserved, so there aren't many alternatives.
Bits [18:17] are used to set the GIC to which to route the MSI and bit
16 must be set when this target GIC is the primary GIC (bits [18:17] set
to 0x0). There aren't other options available for now (that I'm aware
of) for the target GIC configuration.

Antoine

-- 
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160208/845247f2/attachment.sig>

WARNING: multiple messages have this Message-ID (diff)
From: Antoine Tenart <antoine.tenart@free-electrons.com>
To: Marc Zyngier <marc.zyngier@arm.com>
Cc: Antoine Tenart <antoine.tenart@free-electrons.com>,
	tglx@linutronix.de, jason@lakedaemon.net,
	tsahee@annapurnalabs.com, rshitrit@annapurnalabs.com,
	thomas.petazzoni@free-electrons.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/6] irqchip: add the Alpine MSIX interrupt controller
Date: Mon, 8 Feb 2016 11:44:07 +0100	[thread overview]
Message-ID: <20160208104407.GB4117@kwain> (raw)
In-Reply-To: <56B86EA8.9070306@arm.com>

[-- Attachment #1: Type: text/plain, Size: 1792 bytes --]

On Mon, Feb 08, 2016 at 10:32:08AM +0000, Marc Zyngier wrote:
> On 08/02/16 10:26, Antoine Tenart wrote:
> >>> +static int alpine_msix_init(struct device_node *node,
> >>> +			    struct device_node *parent)
> >>> +{
> >>> +	struct alpine_msix_data *priv;
> >>> +	struct resource res;
> >>> +	int ret;
> >>> +
> >>> +	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> >>> +	if (!priv)
> >>> +		return -ENOMEM;
> >>> +
> >>> +	spin_lock_init(&priv->msi_map_lock);
> >>> +
> >>> +	ret = of_address_to_resource(node, 0, &res);
> >>> +	if (ret) {
> >>> +		pr_err("Failed to allocate resource\n");
> >>> +		goto err_priv;
> >>> +	}
> >>> +
> >>> +	priv->addr_high = upper_32_bits((u64)res.start);
> >>> +	priv->addr_low = lower_32_bits(res.start) + ALPINE_MSIX_SPI_TARGET_CLUSTER0;
> >>
> >> This is a bit odd. If you always set bit 16, why isn't that reflected in
> >> the base address coming from the DT?
> > 
> > The 20 least significant bits of addr_low provide direct information
> > regarding the interrupt destination, so I thought it would be clearer
> > to have this explicitly in the driver so that we know what those bits
> > mean.
> 
> So what is this information? TARGET_CLUSTER0 is not very expressive, and
> doesn't show what the alternatives are. Could you please elaborate a bit
> on that front?

For now lots of bits are reserved, so there aren't many alternatives.
Bits [18:17] are used to set the GIC to which to route the MSI and bit
16 must be set when this target GIC is the primary GIC (bits [18:17] set
to 0x0). There aren't other options available for now (that I'm aware
of) for the target GIC configuration.

Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2016-02-08 10:44 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-08  9:16 [PATCH 0/6] irqchip: introduce the Alpine MSIX driver Antoine Tenart
2016-02-08  9:16 ` Antoine Tenart
2016-02-08  9:16 ` [PATCH 1/6] irqchip: add the Alpine MSIX interrupt controller Antoine Tenart
2016-02-08  9:16   ` Antoine Tenart
2016-02-08  9:44   ` Marc Zyngier
2016-02-08  9:44     ` Marc Zyngier
2016-02-08  9:53     ` Thomas Petazzoni
2016-02-08  9:53       ` Thomas Petazzoni
2016-02-08 10:08       ` Marc Zyngier
2016-02-08 10:08         ` Marc Zyngier
2016-02-08 10:26     ` Antoine Tenart
2016-02-08 10:26       ` Antoine Tenart
2016-02-08 10:32       ` Marc Zyngier
2016-02-08 10:32         ` Marc Zyngier
2016-02-08 10:44         ` Antoine Tenart [this message]
2016-02-08 10:44           ` Antoine Tenart
2016-02-08 10:56           ` Marc Zyngier
2016-02-08 10:56             ` Marc Zyngier
2016-02-08 11:01             ` Antoine Tenart
2016-02-08 11:01               ` Antoine Tenart
2016-02-08 14:04     ` Antoine Tenart
2016-02-08 14:04       ` Antoine Tenart
2016-02-08 14:11       ` Marc Zyngier
2016-02-08 14:11         ` Marc Zyngier
2016-02-08 10:31   ` Thomas Gleixner
2016-02-08 10:31     ` Thomas Gleixner
2016-02-08 14:17     ` Antoine Tenart
2016-02-08 14:17       ` Antoine Tenart
2016-02-08 14:29       ` Marc Zyngier
2016-02-08 14:29         ` Marc Zyngier
2016-02-08 14:48         ` Antoine Tenart
2016-02-08 14:48           ` Antoine Tenart
2016-02-08 15:01           ` Marc Zyngier
2016-02-08 15:01             ` Marc Zyngier
2016-02-08  9:16 ` [PATCH 2/6] Documentation: bindings: document the Alpine MSIX driver Antoine Tenart
2016-02-08  9:16   ` Antoine Tenart
2016-02-08  9:16   ` Antoine Tenart
2016-02-08 14:55   ` Marc Zyngier
2016-02-08 14:55     ` Marc Zyngier
2016-02-08 15:05     ` Antoine Tenart
2016-02-08 15:05       ` Antoine Tenart
2016-02-08  9:16 ` [PATCH 3/6] arm64: dts: alpine: add the MSIX node in the Alpine v2 dtsi Antoine Tenart
2016-02-08  9:16   ` Antoine Tenart
2016-02-08  9:16 ` [PATCH 4/6] ARM: dts: alpine: add the MSIX node Antoine Tenart
2016-02-08  9:16   ` Antoine Tenart
2016-02-08  9:16 ` [PATCH 5/6] arm64: alpine: select the Alpine MSI controller driver Antoine Tenart
2016-02-08  9:16   ` Antoine Tenart
2016-02-08  9:16 ` [PATCH 6/6] arm: " Antoine Tenart
2016-02-08  9:16   ` Antoine Tenart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160208104407.GB4117@kwain \
    --to=antoine.tenart@free-electrons.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.