Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Linus Walleij <linus.walleij@linaro.org>
To: Marc Zyngier <marc.zyngier@arm.com>
Cc: Jason Cooper <jason@lakedaemon.net>,
	Arnd Bergmann <arnd@arndb.de>, Tim Harvey <tharvey@gateworks.com>,
	Krzysztof Halasa <khalasa@piap.pl>,
	Olof Johansson <olof@lixom.net>,
	Thomas Gleixner <tglx@linutronix.de>,
	Imre Kaloz <kaloz@openwrt.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 04/17 v1] irqchip: Add driver for IXP4xx
Date: Mon, 11 Feb 2019 21:58:41 +0100
Message-ID: <CACRpkdZK7OU9rb-qw9tTjJLNvfQG-VmN_VBXUz9DPqVV11zuww@mail.gmail.com> (raw)
In-Reply-To: <ffc97649-bfc2-e411-149f-cd5ed9e8c77e@arm.com>

On Mon, Feb 11, 2019 at 4:30 PM Marc Zyngier <marc.zyngier@arm.com> wrote:

> > +static int ixp4xx_set_irq_type(struct irq_data *d, unsigned int type)
> > +{
> > +     /* All are level active high (asserted) here */
>
> It'd be good to return an error if type isn't LEVEL_HIGH.

OK

> > +     if (ixi->is_356 && d->hwirq >= 32) {
> > +             val = __raw_readl(ixi->irqbase + IXP4XX_ICMR2);
> > +             val &= ~BIT(d->hwirq - 32);
> > +             __raw_writel(val, ixi->irqbase + IXP4XX_ICMR2);
> > +     } else {
> > +             val = __raw_readl(ixi->irqbase + IXP4XX_ICMR);
> > +             val &= ~BIT(d->hwirq);
> > +             __raw_writel(val, ixi->irqbase + IXP4XX_ICMR);
> > +     }
> > +}
>
> This probably comes from the original code, but I'd like to be able to
> use a LE kernel on this HW (full disclosure: I have some of this crap
> stashed somewhere in the basement... ;-).

It will work. One reason I am refactoring and preserving IXP4xx
is this stuff from the datasheet:

Ambient Air Temperature (Extended)  -40° C to 85° C
Ambient Air Temperature (Commercial) 0° C to 70° C
MTBF: 60 Years at 55°C

And I don't think there is any big difference between the "extended"
and "commercial" version of this SoC, they all come in the same
sturdy packaging too. Diamonds and IXP4xx are forever.

> How about using something that enforces the endianness of the accesses,
> as I suspect the bus is hardcoded to BE?

So, IIUC (Krzysztof can confirm) IXP4xx works like this that when the
CPU is brought up in LE mode, all bytes on the address bus are
swizzled so that the access can always use the CPU native
endianness.

Isn't it crazy :D

This means __raw_writel() will always do the right thing, because
that uses native endianness, and that is why it is used quite a lot
in IXP4xx drivers.

If you want full control you need to define local helpers for
everything that change behaviour at compile-time such as:

static void foo_writel(u32 val, __iomem void *addr)
{
#if defined(CONFIG_CPU_BIG_ENDIAN)
   iowrite32be(val, addr);
#else
    writel(val, addr);
#endif
}

Which will have the same effect as __raw_writel().

So that is why we use the (ugly) macro __raw_writel() a lot
in IXP4xx drivers.

> ioread32be/iowrite32be springs
> to mind, and I can see the current IXP4xx code provides such an
> implementation in its own io.h (which you may have to make private).

The stuf in <mach/io.h> is there for indirect PCI, which is
another unrelated oddity. PCI devices are always LE (as is
proper). It is there because the PCI host on the IXP4xx can
only access > 64MB PCI memory space with certain quirks that
no other PCI host controller needs.

I will get to this when I try to move that driver to drivers/pci ...
It's unrelated to this business however.

> > +     ixi->irqchip.name = "IXP4xx";
> > +     ixi->irqchip.irq_mask = ixp4xx_irq_mask;
> > +     ixi->irqchip.irq_unmask = ixp4xx_irq_unmask;
> > +     ixi->irqchip.irq_set_type = ixp4xx_set_irq_type;
>
> Aren't you guaranteed to only have one such irqchip? If so, this could
> become a static const object, instead of allocating it dynamically. Not
> a big deal though.

Force of habit. Comes from the GPIO side of things where we
never know how many instances there are of stuff, if it's OK
I'd like to keep it.

> > +     fwnode = irq_domain_alloc_fwnode(base);
>
> I assume this is a temporary solution until the SoC gains a DT port (and
> the irqchip a DT node)?

Yes, see the later DT patch that passes the parent as
fwnode, once all boards are converted we can drop this.

BTW here is how I then use the hierarchical parent in the
GPIO chip:
https://marc.info/?l=linux-arm-kernel&m=154923023907623&w=2
https://marc.info/?l=linux-arm-kernel&m=154923038707686&w=2

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply index

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-03 21:41 [PATCH 00/17 v1] ARM: ixp4xx: Modernize and DT support Linus Walleij
2019-02-03 21:41 ` [PATCH 01/17 v1] ARM: ixp4xx: Convert to MULTI_IRQ_HANDLER Linus Walleij
2019-02-03 21:41 ` [PATCH 02/17 v1] ARM: ixp4xx: Pass IRQ resource to beeper Linus Walleij
2019-02-03 21:41 ` [PATCH 03/17 v1] ARM: ixp4xx: Convert to SPARSE_IRQ Linus Walleij
2019-02-03 21:41 ` [PATCH 04/17 v1] irqchip: Add driver for IXP4xx Linus Walleij
2019-02-11 15:30   ` Marc Zyngier
2019-02-11 20:58     ` Linus Walleij [this message]
2019-02-11 22:11       ` Marc Zyngier
2019-02-18  7:06         ` khalasa
2019-02-18  7:16           ` Linus Walleij
2019-02-18  7:35             ` khalasa
2019-02-18  9:40             ` Arnd Bergmann
2019-02-18 12:03               ` khalasa
2019-02-18 12:44                 ` Arnd Bergmann
2019-02-19  6:51                   ` khalasa
2019-02-19  9:46                     ` Arnd Bergmann
2019-02-20  7:35                       ` khalasa
2019-02-18  9:18         ` Arnd Bergmann
2019-02-03 21:41 ` [PATCH 05/17 v1] gpio: ixp4xx: Add driver for the IXP4xx GPIO Linus Walleij
2019-02-06 16:03   ` Bartosz Golaszewski
2019-02-21  8:50     ` Linus Walleij
2019-02-03 21:41 ` [PATCH 06/17 v1] ARM: ixp4xx: Switch to use new IRQ+GPIO drivers Linus Walleij
2019-02-03 21:41 ` [PATCH 07/17 v1] clocksource/drivers/ixp4xx: Add driver Linus Walleij
2019-02-03 21:41 ` [PATCH 08/17 v1] ARM: ixp4xx: Switch to use new timer driver Linus Walleij
2019-02-03 21:41 ` [PATCH 09/17 v1] irqchip: ixp4xx: Add DT bindings Linus Walleij
2019-02-18 21:25   ` Rob Herring
2019-02-03 21:41 ` [PATCH 10/17 v1] irqchip: ixp4xx: Add OF initialization support Linus Walleij
2019-02-03 21:41 ` [PATCH 11/17 v1] clocksource/drivers/ixp4xx: Add DT bindings Linus Walleij
2019-02-18 21:26   ` Rob Herring
2019-02-18 22:10     ` Daniel Lezcano
2019-02-03 21:42 ` [PATCH 12/17 v1] clocksource/drivers/ixp4xx: Add OF initialization support Linus Walleij
2019-02-11 11:26   ` Daniel Lezcano
2019-02-03 21:42 ` [PATCH 13/17 v1] gpio: ixp4xx: Add DT bindings Linus Walleij
2019-02-06 16:05   ` Bartosz Golaszewski
2019-02-18 21:27   ` Rob Herring
2019-02-03 21:42 ` [PATCH 14/17 v1] gpio: ixp4xx: Add OF probing support Linus Walleij
2019-02-06 16:13   ` Bartosz Golaszewski
2019-02-21  8:55     ` Linus Walleij
2019-02-03 21:42 ` [PATCH 15/17 v1] ARM: ixp4xx: Add DT bindings Linus Walleij
2019-02-04 15:16   ` Rob Herring
2019-02-08 19:37     ` Linus Walleij
2019-02-03 21:42 ` [PATCH 16/17 v1] ARM: ixp4xx: Add device tree boot support Linus Walleij
2019-02-03 21:42 ` [PATCH 17/17 v1] RFC: ARM: dts: Add some initial IXP4xx device trees Linus Walleij

Reply instructions:

You may reply publically 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=CACRpkdZK7OU9rb-qw9tTjJLNvfQG-VmN_VBXUz9DPqVV11zuww@mail.gmail.com \
    --to=linus.walleij@linaro.org \
    --cc=arnd@arndb.de \
    --cc=jason@lakedaemon.net \
    --cc=kaloz@openwrt.org \
    --cc=khalasa@piap.pl \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=marc.zyngier@arm.com \
    --cc=olof@lixom.net \
    --cc=tglx@linutronix.de \
    --cc=tharvey@gateworks.com \
    /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

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org infradead-linux-arm-kernel@archiver.kernel.org
	public-inbox-index linux-arm-kernel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox