Re: [PATCH 07/30 v2] clocksource/drivers/ixp4xx: Add driver

From: Linus Walleij <linus.walleij@linaro.org>
To: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Imre Kaloz <kaloz@openwrt.org>,
	Krzysztof Halasa <khalasa@piap.pl>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 07/30 v2] clocksource/drivers/ixp4xx: Add driver
Date: Sun, 3 Mar 2019 10:35:58 +0100	[thread overview]
Message-ID: <CACRpkdYD_QBntqV9LZqhjFHr9E5xMwh=wso_-2dtps4rtnR4fg@mail.gmail.com> (raw)
In-Reply-To: <14f5cea3-9edf-c319-57f4-c768189875c9@linaro.org>

On Thu, Feb 21, 2019 at 9:54 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 21/02/2019 16:44, Linus Walleij wrote:
> > This adds a new slightly rewritten timer driver for the
> > Intel IXP4xx clocksource, clockevent and delay timer.
> >
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > Clocksource/timer maintainers: I am requesting an ACK for
> > this once you're happy with the driver, as I intend to
> > merge all of this IXP4xx rework through ARM SoC.
> > ---
>
> Hi Linus,
>
> a couple of questions:
>
>  - why __raw_write|read ?

That is an excellent question! It was answered in another mail
about the irqchip driver:

https://marc.info/?l=linux-arm-kernel&m=154991876024540&w=2

Quoting myself:

---------
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.
---------

>  - can you consider using the timer_of API ?

Yes that is added in a later patch. We are transitioning a machine
from boardfile to DT and later on the non-DT path will be deleted.

Yours,
Linus Walleij

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