From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> To: Marc Zyngier <marc.zyngier@arm.com> Cc: "Linus Walleij" <linus.walleij@linaro.org>, "Olof Johansson" <olof@lixom.net>, "Arnd Bergmann" <arnd@arndb.de>, "Rob Herring" <robh+dt@kernel.org>, "Thomas Gleixner" <tglx@linutronix.de>, "Jason Cooper" <jason@lakedaemon.net>, "Daniel Lezcano" <daniel.lezcano@linaro.org>, "Linux ARM" <linux-arm-kernel@lists.infradead.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@vger.kernel.org>, "Amit Kucheria" <amit.kucheria@linaro.org>, zhao_steven@263.net, "Andreas Färber" <afaerber@suse.de> Subject: Re: [PATCH 12/16] clocksource: Add clock driver for RDA8810PL SoC Date: Tue, 20 Nov 2018 17:39:10 +0530 [thread overview] Message-ID: <20181120120910.GA13485@mani> (raw) In-Reply-To: <e732ad15-b778-1c84-ad47-5770ee66688b@arm.com> On Tue, Nov 20, 2018 at 11:05:41AM +0000, Marc Zyngier wrote: > On 20/11/2018 08:56, Linus Walleij wrote: > > On Tue, Nov 20, 2018 at 9:17 AM Marc Zyngier <marc.zyngier@arm.com> wrote: > > > >> How does this change anything with the fact that the above code is > >> broken? 56 or 64 bit, you cannot read this counter with a single > >> access, or two. The canonical way of reading such a counter is > >> something like this: > >> > >> do { > >> lo = readl_relaxed(LO); > >> hi = readl_relaxed(HI); > >> } while (hi != read_relaxed(HI)); > > > > To be fair, I have seen hardware that employ a logic latch > > such that when a read access is done to the LO register, > > the value of the whole counter is latched, also for the HI > > register, so when you read the HI register in the second > > step, it is never subject to wrapping. (Conversely reading > > the HI before the LO will always give you insane values > > :D) > > I've seen such HW indeed, and I've also seen it being broken... ;-) > > It this timer is built around such a (non-broken) logic, I'd really like > to see it spelled out. It will otherwise be a real pain to debug... > There is no information about HW latch in datasheet and vendor code. But the vendor driver doesn't use any logic to prevent wrapping. However, this doesn't mean that we can assume that the hardware is capable of preventing overrun. So I guess it is best to go with Marc's suggestion here. Thanks, Mani > > However the above code should be fine unless you know > > for sure the hardware was constructed with a clever latch. > > Let's find out! > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny...
WARNING: multiple messages have this Message-ID (diff)
From: manivannan.sadhasivam@linaro.org (Manivannan Sadhasivam) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 12/16] clocksource: Add clock driver for RDA8810PL SoC Date: Tue, 20 Nov 2018 17:39:10 +0530 [thread overview] Message-ID: <20181120120910.GA13485@mani> (raw) In-Reply-To: <e732ad15-b778-1c84-ad47-5770ee66688b@arm.com> On Tue, Nov 20, 2018 at 11:05:41AM +0000, Marc Zyngier wrote: > On 20/11/2018 08:56, Linus Walleij wrote: > > On Tue, Nov 20, 2018 at 9:17 AM Marc Zyngier <marc.zyngier@arm.com> wrote: > > > >> How does this change anything with the fact that the above code is > >> broken? 56 or 64 bit, you cannot read this counter with a single > >> access, or two. The canonical way of reading such a counter is > >> something like this: > >> > >> do { > >> lo = readl_relaxed(LO); > >> hi = readl_relaxed(HI); > >> } while (hi != read_relaxed(HI)); > > > > To be fair, I have seen hardware that employ a logic latch > > such that when a read access is done to the LO register, > > the value of the whole counter is latched, also for the HI > > register, so when you read the HI register in the second > > step, it is never subject to wrapping. (Conversely reading > > the HI before the LO will always give you insane values > > :D) > > I've seen such HW indeed, and I've also seen it being broken... ;-) > > It this timer is built around such a (non-broken) logic, I'd really like > to see it spelled out. It will otherwise be a real pain to debug... > There is no information about HW latch in datasheet and vendor code. But the vendor driver doesn't use any logic to prevent wrapping. However, this doesn't mean that we can assume that the hardware is capable of preventing overrun. So I guess it is best to go with Marc's suggestion here. Thanks, Mani > > However the above code should be fine unless you know > > for sure the hardware was constructed with a clever latch. > > Let's find out! > > Thanks, > > M. > -- > Jazz is not dead. It just smells funny...
next prev parent reply other threads:[~2018-11-20 12:09 UTC|newest] Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-19 17:09 [PATCH 00/16] Add initial RDA8810PL SoC and Orange Pi boards support Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 01/16] dt-bindings: Add RDA Micro vendor prefix Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:22 ` Andreas Färber 2018-11-19 17:22 ` Andreas Färber 2018-11-19 17:29 ` Manivannan Sadhasivam 2018-11-19 17:29 ` Manivannan Sadhasivam 2018-11-20 2:51 ` Manivannan Sadhasivam 2018-11-20 2:51 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 02/16] dt-bindings: arm: Document RDA8810PL and reference boards Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 03/16] ARM: Prepare RDA8810PL SoC Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 04/16] arm: dts: Add devicetree for " Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 18:25 ` Rob Herring 2018-11-19 18:25 ` Rob Herring 2018-11-20 19:31 ` Manivannan Sadhasivam 2018-11-20 19:31 ` Manivannan Sadhasivam 2018-11-19 19:37 ` Arnd Bergmann 2018-11-19 19:37 ` Arnd Bergmann 2018-11-20 19:32 ` Manivannan Sadhasivam 2018-11-20 19:32 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 05/16] arm: dts: Add devicetree for OrangePi 2G IoT board Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 06/16] arm: dts: Add devicetree for OrangePi i96 board Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 07/16] dt-bindings: interrupt-controller: Document RDA8810PL intc Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 08/16] arm: dts: rda8810pl: Add interrupt controller support Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 18:29 ` Rob Herring 2018-11-19 18:29 ` Rob Herring 2018-11-20 19:28 ` Manivannan Sadhasivam 2018-11-20 19:28 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 09/16] irqchip: Add RDA8810PL interrupt driver Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:36 ` Marc Zyngier 2018-11-19 17:36 ` Marc Zyngier 2018-11-20 3:19 ` Manivannan Sadhasivam 2018-11-20 3:19 ` Manivannan Sadhasivam 2018-11-20 8:10 ` Marc Zyngier 2018-11-20 8:10 ` Marc Zyngier 2018-11-19 17:09 ` [PATCH 10/16] dt-bindings: timer: Document RDA8810PL SoC timer Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 11/16] arm: dts: rda8810pl: Add timer support Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 12/16] clocksource: Add clock driver for RDA8810PL SoC Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:57 ` Marc Zyngier 2018-11-19 17:57 ` Marc Zyngier 2018-11-20 5:06 ` Manivannan Sadhasivam 2018-11-20 5:06 ` Manivannan Sadhasivam 2018-11-20 8:16 ` Marc Zyngier 2018-11-20 8:16 ` Marc Zyngier 2018-11-20 8:56 ` Linus Walleij 2018-11-20 8:56 ` Linus Walleij 2018-11-20 11:05 ` Marc Zyngier 2018-11-20 11:05 ` Marc Zyngier 2018-11-20 12:09 ` Manivannan Sadhasivam [this message] 2018-11-20 12:09 ` Manivannan Sadhasivam 2018-11-20 10:32 ` Daniel Lezcano 2018-11-20 10:32 ` Daniel Lezcano 2018-11-20 12:11 ` Manivannan Sadhasivam 2018-11-20 12:11 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 13/16] dt-bindings: serial: Document RDA Micro UART Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 14/16] arm: dts: rda8810pl: Add interrupt support for UART Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 15/16] tty: serial: Add RDA8810PL UART driver Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam 2018-11-19 17:09 ` [PATCH 16/16] MAINTAINERS: Add entry for RDA Micro SoC architecture Manivannan Sadhasivam 2018-11-19 17:09 ` Manivannan Sadhasivam
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=20181120120910.GA13485@mani \ --to=manivannan.sadhasivam@linaro.org \ --cc=afaerber@suse.de \ --cc=amit.kucheria@linaro.org \ --cc=arnd@arndb.de \ --cc=daniel.lezcano@linaro.org \ --cc=devicetree@vger.kernel.org \ --cc=jason@lakedaemon.net \ --cc=linus.walleij@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=marc.zyngier@arm.com \ --cc=olof@lixom.net \ --cc=robh+dt@kernel.org \ --cc=tglx@linutronix.de \ --cc=zhao_steven@263.net \ /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: linkBe 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.