* Re: [PATCH net-next v4 0/2] dt-bindings: net: meson-dwmac: convert to yaml
From: David Miller @ 2019-08-22 22:42 UTC (permalink / raw)
To: narmstrong
Cc: devicetree, martin.blumenstingl, netdev, linux-kernel, robh+dt,
linux-amlogic, linux-arm-kernel
In-Reply-To: <20190820075742.14857-1-narmstrong@baylibre.com>
From: Neil Armstrong <narmstrong@baylibre.com>
Date: Tue, 20 Aug 2019 09:57:40 +0200
> This patchsets converts the Amlogic Meson DWMAC glue bindings over to
> YAML schemas using the already converted dwmac bindings.
>
> The first patch is needed because the Amlogic glue needs a supplementary
> reg cell to access the DWMAC glue registers.
>
> Changes since v3:
> - Specified net-next target tree
>
> Changes since v2:
> - Added review tags
> - Updated allwinner,sun7i-a20-gmac.yaml reg maxItems
Series applied, thanks.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 00/14] arm64: dts: meson: fixes following YAML bindings schemas conversion
From: Kevin Hilman @ 2019-08-22 22:26 UTC (permalink / raw)
To: Neil Armstrong
Cc: linux-amlogic, Neil Armstrong, linux-kernel, linux-arm-kernel,
devicetree
In-Reply-To: <20190821142043.14649-1-narmstrong@baylibre.com>
Neil Armstrong <narmstrong@baylibre.com> writes:
> This is the first set of DT fixes following the first YAML bindings conversion
> at [1], [2] and [3].
>
> After this set of fixes, the remaining errors are :
> meson-axg-s400.dt.yaml: sound: 'clocks' is a dependency of 'assigned-clocks'
> meson-g12a-sei510.dt.yaml: sound: 'clocks' is a dependency of 'assigned-clocks'
> meson-g12b-odroid-n2.dt.yaml: usb-hub: gpios:0:0: 20 is not valid under any of the given schemas
> meson-g12b-odroid-n2.dt.yaml: sound: 'clocks' is a dependency of 'assigned-clocks'
> meson-g12a-x96-max.dt.yaml: sound: 'clocks' is a dependency of 'assigned-clocks'
>
> These are only cosmetic changes, and should not break drivers implementation
> following the bindings.
Any chance you can rebase this on top of my v5.4/dt64 branch?
Thanks,
Kevin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 2/2] coresight: tmc-etr: Add barrier packet when moving offset forward
From: Mathieu Poirier @ 2019-08-22 22:09 UTC (permalink / raw)
To: yabinc, suzuki.poulose, leo.yan
Cc: alexander.shishkin, linux-arm-kernel, mike.leach, linux-kernel
In-Reply-To: <20190822220915.8876-1-mathieu.poirier@linaro.org>
This patch adds barrier packets in the trace stream when the offset in the
data buffer needs to be moved forward. Otherwise the decoder isn't aware
of the break in the stream and can't synchronise itself with the trace
data.
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
.../hwtracing/coresight/coresight-tmc-etr.c | 43 ++++++++++++++-----
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
index 4f000a03152e..0e4cd6ec5f28 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
@@ -946,10 +946,6 @@ static void tmc_sync_etr_buf(struct tmc_drvdata *drvdata)
WARN_ON(!etr_buf->ops || !etr_buf->ops->sync);
etr_buf->ops->sync(etr_buf, rrp, rwp);
-
- /* Insert barrier packets at the beginning, if there was an overflow */
- if (etr_buf->full)
- tmc_etr_buf_insert_barrier_packet(etr_buf, etr_buf->offset);
}
static void __tmc_etr_enable_hw(struct tmc_drvdata *drvdata)
@@ -1415,10 +1411,11 @@ static void tmc_free_etr_buffer(void *config)
* buffer to the perf ring buffer.
*/
static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf,
+ unsigned long src_offset,
unsigned long to_copy)
{
long bytes;
- long pg_idx, pg_offset, src_offset;
+ long pg_idx, pg_offset;
unsigned long head = etr_perf->head;
char **dst_pages, *src_buf;
struct etr_buf *etr_buf = etr_perf->etr_buf;
@@ -1427,7 +1424,6 @@ static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf,
pg_idx = head >> PAGE_SHIFT;
pg_offset = head & (PAGE_SIZE - 1);
dst_pages = (char **)etr_perf->pages;
- src_offset = etr_buf->offset + etr_buf->len - to_copy;
while (to_copy > 0) {
/*
@@ -1475,7 +1471,7 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
void *config)
{
bool lost = false;
- unsigned long flags, size = 0;
+ unsigned long flags, offset, size = 0;
struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
struct etr_perf_buffer *etr_perf = config;
struct etr_buf *etr_buf = etr_perf->etr_buf;
@@ -1503,11 +1499,39 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
spin_unlock_irqrestore(&drvdata->spinlock, flags);
size = etr_buf->len;
+ offset = etr_buf->offset;
+ lost |= etr_buf->full;
+
+ /*
+ * The ETR buffer may be bigger than the space available in the
+ * perf ring buffer (handle->size). If so advance the offset so that we
+ * get the latest trace data. In snapshot mode none of that matters
+ * since we are expected to clobber stale data in favour of the latest
+ * traces.
+ */
if (!etr_perf->snapshot && size > handle->size) {
- size = handle->size;
+ u32 mask = tmc_get_memwidth_mask(drvdata);
+
+ /*
+ * Make sure the new size is aligned in accordance with the
+ * requirement explained in function tmc_get_memwidth_mask().
+ */
+ size = handle->size & mask;
+ offset = etr_buf->offset + etr_buf->len - size;
+
+ if (offset >= etr_buf->size)
+ offset -= etr_buf->size;
lost = true;
}
- tmc_etr_sync_perf_buffer(etr_perf, size);
+
+ /*
+ * Insert barrier packets at the beginning, if there was an overflow
+ * or if the offset had to be brought forward.
+ */
+ if (lost)
+ tmc_etr_buf_insert_barrier_packet(etr_buf, offset);
+
+ tmc_etr_sync_perf_buffer(etr_perf, offset, size);
/*
* In snapshot mode we simply increment the head by the number of byte
@@ -1518,7 +1542,6 @@ tmc_update_etr_buffer(struct coresight_device *csdev,
if (etr_perf->snapshot)
handle->head += size;
- lost |= etr_buf->full;
out:
/*
* Don't set the TRUNCATED flag in snapshot mode because 1) the
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 0/2] coresight: Add barrier packet when moving offset forward
From: Mathieu Poirier @ 2019-08-22 22:09 UTC (permalink / raw)
To: yabinc, suzuki.poulose, leo.yan
Cc: alexander.shishkin, linux-arm-kernel, mike.leach, linux-kernel
Hi Yabin,
When doing more tests on your patch that adjust the offset to fit the
available space in the perf ring buffer[1], I noticed the decoder wasn't
able to decode the traces that had been collected. The issue was observed
in CPU wide scenarios but I also suspect they would have showed up in
per-thread mode given the right conditions.
I traced the problem to the moving forward of the offset in the trace
buffer. Doing so skips over the barrier packets originally inserted in
function tmc_sync_etr_buf(), which in turn prevents the decoder from
properly synchronising with the trace packets.
I fixed the condition by inserting barrier packets once the offset has been
moved forward, making sure that alignment rules are respected.
I'd be grateful if you could review and test my changes to make sure things
still work on your side.
Applies cleanly on the coresight next branch.
Best regards,
Mathieu
[1]. https://lkml.org/lkml/2019/8/14/1336
Mathieu Poirier (2):
coresight: tmc: Make memory width mask computation into a function
coresight: tmc-etr: Add barrier packet when moving offset forward
.../hwtracing/coresight/coresight-tmc-etf.c | 23 +---------
.../hwtracing/coresight/coresight-tmc-etr.c | 43 ++++++++++++++-----
drivers/hwtracing/coresight/coresight-tmc.c | 28 ++++++++++++
drivers/hwtracing/coresight/coresight-tmc.h | 1 +
4 files changed, 64 insertions(+), 31 deletions(-)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 1/2] coresight: tmc: Make memory width mask computation into a function
From: Mathieu Poirier @ 2019-08-22 22:09 UTC (permalink / raw)
To: yabinc, suzuki.poulose, leo.yan
Cc: alexander.shishkin, linux-arm-kernel, mike.leach, linux-kernel
In-Reply-To: <20190822220915.8876-1-mathieu.poirier@linaro.org>
Make the computation of a memory ask representing the width of the memory
bus into a function so that it can be re-used by the ETR driver.
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
---
.../hwtracing/coresight/coresight-tmc-etf.c | 23 ++-------------
drivers/hwtracing/coresight/coresight-tmc.c | 28 +++++++++++++++++++
drivers/hwtracing/coresight/coresight-tmc.h | 1 +
3 files changed, 31 insertions(+), 21 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
index 23b7ff00af5c..807416b75ecc 100644
--- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
+++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
@@ -479,30 +479,11 @@ static unsigned long tmc_update_etf_buffer(struct coresight_device *csdev,
* traces.
*/
if (!buf->snapshot && to_read > handle->size) {
- u32 mask = 0;
-
- /*
- * The value written to RRP must be byte-address aligned to
- * the width of the trace memory databus _and_ to a frame
- * boundary (16 byte), whichever is the biggest. For example,
- * for 32-bit, 64-bit and 128-bit wide trace memory, the four
- * LSBs must be 0s. For 256-bit wide trace memory, the five
- * LSBs must be 0s.
- */
- switch (drvdata->memwidth) {
- case TMC_MEM_INTF_WIDTH_32BITS:
- case TMC_MEM_INTF_WIDTH_64BITS:
- case TMC_MEM_INTF_WIDTH_128BITS:
- mask = GENMASK(31, 4);
- break;
- case TMC_MEM_INTF_WIDTH_256BITS:
- mask = GENMASK(31, 5);
- break;
- }
+ u32 mask = tmc_get_memwidth_mask(drvdata);
/*
* Make sure the new size is aligned in accordance with the
- * requirement explained above.
+ * requirement explained in function tmc_get_memwidth_mask().
*/
to_read = handle->size & mask;
/* Move the RAM read pointer up */
diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
index 3055bf8e2236..1cf82fa58289 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.c
+++ b/drivers/hwtracing/coresight/coresight-tmc.c
@@ -70,6 +70,34 @@ void tmc_disable_hw(struct tmc_drvdata *drvdata)
writel_relaxed(0x0, drvdata->base + TMC_CTL);
}
+u32 tmc_get_memwidth_mask(struct tmc_drvdata *drvdata)
+{
+ u32 mask = 0;
+
+ /*
+ * When moving RRP or an offset address forward, the new values must
+ * be byte-address aligned to the width of the trace memory databus
+ * _and_ to a frame boundary (16 byte), whichever is the biggest. For
+ * example, for 32-bit, 64-bit and 128-bit wide trace memory, the four
+ * LSBs must be 0s. For 256-bit wide trace memory, the five LSBs must
+ * be 0s.
+ */
+ switch (drvdata->memwidth) {
+ case TMC_MEM_INTF_WIDTH_32BITS:
+ /* fallthrough */
+ case TMC_MEM_INTF_WIDTH_64BITS:
+ /* fallthrough */
+ case TMC_MEM_INTF_WIDTH_128BITS:
+ mask = GENMASK(31, 4);
+ break;
+ case TMC_MEM_INTF_WIDTH_256BITS:
+ mask = GENMASK(31, 5);
+ break;
+ }
+
+ return mask;
+}
+
static int tmc_read_prepare(struct tmc_drvdata *drvdata)
{
int ret = 0;
diff --git a/drivers/hwtracing/coresight/coresight-tmc.h b/drivers/hwtracing/coresight/coresight-tmc.h
index 9dbcdf453e22..71de978575f3 100644
--- a/drivers/hwtracing/coresight/coresight-tmc.h
+++ b/drivers/hwtracing/coresight/coresight-tmc.h
@@ -255,6 +255,7 @@ void tmc_wait_for_tmcready(struct tmc_drvdata *drvdata);
void tmc_flush_and_stop(struct tmc_drvdata *drvdata);
void tmc_enable_hw(struct tmc_drvdata *drvdata);
void tmc_disable_hw(struct tmc_drvdata *drvdata);
+u32 tmc_get_memwidth_mask(struct tmc_drvdata *drvdata);
/* ETB/ETF functions */
int tmc_read_prepare_etb(struct tmc_drvdata *drvdata);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC v4 07/18] objtool: Introduce INSN_UNKNOWN type
From: Josh Poimboeuf @ 2019-08-22 21:51 UTC (permalink / raw)
To: Julien
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
linux-kernel, Raphael Gault, linux-arm-kernel
In-Reply-To: <3c4e3227-eeb3-371a-d015-a0e0e60e5332@gmail.com>
On Thu, Aug 22, 2019 at 09:45:00PM +0100, Julien wrote:
> Hi Josh,
>
> On 22/08/19 21:04, Josh Poimboeuf wrote:
> > On Fri, Aug 16, 2019 at 01:23:52PM +0100, Raphael Gault wrote:
> > > On arm64 some object files contain data stored in the .text section.
> > > This data is interpreted by objtool as instruction but can't be
> > > identified as a valid one. In order to keep analysing those files we
> > > introduce INSN_UNKNOWN type. The "unknown instruction" warning will thus
> > > only be raised if such instructions are uncountered while validating an
> > > execution branch.
> > >
> > > This change doesn't impact the x86 decoding logic since 0 is still used
> > > as a way to specify an unknown type, raising the "unknown instruction"
> > > warning during the decoding phase still.
> > >
> > > Signed-off-by: Raphael Gault <raphael.gault@arm.com>
> >
> > Is there a reason such data can't be moved to .rodata? That would seem
> > like the proper fix.
> >
>
> Raphaël can confirm, if I remember correctly, that issue was encountered on
> assembly files implementing crypto algorithms were some words/double-words
> of data were in the middle of the .text. I think it is done this way to make
> sure the data can be loaded in a single instruction. So moving it to another
> section could impact the crypto performance depending on the relocations.
>
> That was my understanding at least.
Thanks. If that's the case then that would be useful information to put
in the patch description. A code excerpt of an example code site would
be useful too.
I'm not sure INSN_UNKNOWN is the right name though, since the decoder
does actually know about it. Maybe INSN_DATA or something?
--
Josh
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] ARM: dts: pbab01: correct rtc vendor
From: Alexandre Belloni @ 2019-08-22 21:35 UTC (permalink / raw)
To: Shawn Guo
Cc: Alexandre Belloni, linux-kernel, NXP Linux Team, kernel,
Fabio Estevam, linux-arm-kernel
The rtc8564 is made by Epson but is similar to the NXP pcf8563. Use the
correct vendor name.
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
arch/arm/boot/dts/imx6qdl-phytec-pbab01.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/imx6qdl-phytec-pbab01.dtsi b/arch/arm/boot/dts/imx6qdl-phytec-pbab01.dtsi
index 82802f8ce7a0..d434868e870a 100644
--- a/arch/arm/boot/dts/imx6qdl-phytec-pbab01.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-phytec-pbab01.dtsi
@@ -128,7 +128,7 @@
};
rtc@51 {
- compatible = "nxp,rtc8564";
+ compatible = "epson,rtc8564";
reg = <0x51>;
};
--
2.21.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* page_alloc.shuffle=1 + CONFIG_PROVE_LOCKING=y = arm64 hang
From: Qian Cai @ 2019-08-22 21:33 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon
Cc: Peter Zijlstra, linux-mm, Dan Williams, linux-kernel,
linux-arm-kernel
https://raw.githubusercontent.com/cailca/linux-mm/master/arm64.config
Booting an arm64 ThunderX2 server with page_alloc.shuffle=1 [1] +
CONFIG_PROVE_LOCKING=y results in hanging.
[1] https://lore.kernel.org/linux-mm/154899811208.3165233.17623209031065121886.s
tgit@dwillia2-desk3.amr.corp.intel.com/
...
[ 125.142689][ T1] arm-smmu-v3 arm-smmu-v3.2.auto: option mask 0x2
[ 125.149687][ T1] arm-smmu-v3 arm-smmu-v3.2.auto: ias 44-bit, oas 44-bit
(features 0x0000170d)
[ 125.165198][ T1] arm-smmu-v3 arm-smmu-v3.2.auto: allocated 524288 entries
for cmdq
[ 125.239425][ [ 125.251484][ T1] arm-smmu-v3 arm-smmu-v3.3.auto: option
mask 0x2
[ 125.258233][ T1] arm-smmu-v3 arm-smmu-v3.3.auto: ias 44-bit, oas 44-bit
(features 0x0000170d)
[ 125.282750][ T1] arm-smmu-v3 arm-smmu-v3.3.auto: allocated 524288 entries
for cmdq
[ 125.320097][ T1] arm-smmu-v3 arm-smmu-v3.3.auto: allocated 524288 entries
for evtq
[ 125.332667][ T1] arm-smmu-v3 arm-smmu-v3.4.auto: option mask 0x2
[ 125.339427][ T1] arm-smmu-v3 arm-smmu-v3.4.auto: ias 44-bit, oas 44-bit
(features 0x0000170d)
[ 125.354846][ T1] arm-smmu-v3 arm-smmu-v3.4.auto: allocated 524288 entries
for cmdq
[ 125.375295][ T1] arm-smmu-v3 arm-smmu-v3.4.auto: allocated 524288 entries
for evtq
[ 125.387371][ T1] arm-smmu-v3 arm-smmu-v3.5.auto: option mask 0x2
[ 125.393955][ T1] arm-smmu-v3 arm-smmu-v3.5.auto: ias 44-bit, oas 44-bit
(features 0x0000170d)
[ 125.522605][ T1] arm-smmu-v3 arm-smmu-v3.5.auto: allocated 524288 entries
for cmdq
[ 125.543338][ T1] arm-smmu-v3 arm-smmu-v3.5.auto: allocated 524288 entries
for evtq
[ 126.694742][ T1] EFI Variables Facility v0.08 2004-May-17
[ 126.799291][ T1] NET: Registered protocol family 17
[ 126.978632][ T1] zswap: loaded using pool lzo/zbud
[ 126.989168][ T1] kmemleak: Kernel memory leak detector initialized
[ 126.989191][ T1577] kmemleak: Automatic memory scanning thread started
[ 127.044079][ T1335] pcieport 0000:0f:00.0: Adding to iommu group 0
[ 127.388074][ T1] Freeing unused kernel memory: 22528K
[ 133.527005][ T1] Checked W+X mappings: passed, no W+X pages found
[ 133.533474][ T1] Run /init as init process
[ 133.727196][ T1] systemd[1]: System time before build time, advancing
clock.
[ 134.576021][ T1587] modprobe (1587) used greatest stack depth: 27056 bytes
left
[ 134.764026][ T1] systemd[1]: systemd 239 running in system mode. (+PAM
+AUDIT +SELINUX +IMA -APPARMOR +SMACK +SYSVINIT +UTMP +LIBCRYPTSETUP +GCRYPT
+GNUTLS +ACL +XZ +LZ4 +SECCOMP +BLKID +ELFUTILS +KMOD +IDN2 -IDN +PCRE2 default-
hierarchy=legacy)
[ 134.799044][ T1] systemd[1]: Detected architecture arm64.
[ 134.804818][ T1] systemd[1]: Running in initial RAM disk.
<...hang...>
Fix it by either set page_alloc.shuffle=0 or CONFIG_PROVE_LOCKING=n which allow
it to continue successfully.
[ 121.093846][ T1] systemd[1]: Set hostname to <hpe-apollo-cn99xx>.
[ 123.157524][ T1] random: systemd: uninitialized urandom read (16 bytes
read)
[ 123.168562][ T1] systemd[1]: Listening on Journal Socket.
[ OK ] Listening on Journal Socket.
[ 123.203932][ T1] random: systemd: uninitialized urandom read (16 bytes
read)
[ 123.212813][ T1] systemd[1]: Listening on udev Kernel Socket.
[ OK ] Listening on udev Kernel Socket.
...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/3] rtc: sun6i: Add support for H6 RTC
From: Alexandre Belloni @ 2019-08-22 21:16 UTC (permalink / raw)
To: megous
Cc: Mark Rutland, Alessandro Zummo, devicetree, Maxime Ripard,
linux-sunxi, linux-kernel, Chen-Yu Tsai, Rob Herring,
linux-arm-kernel, linux-rtc
In-Reply-To: <20190820151934.3860-3-megous@megous.com>
On 20/08/2019 17:19:33+0200, megous@megous.com wrote:
> From: Ondrej Jirman <megous@megous.com>
>
> RTC on H6 is mostly the same as on H5 and H3. It has slight differences
> mostly in features that are not yet supported by this driver.
>
> Some differences are already stated in the comments in existing code.
> One other difference is that H6 has extra bit in LOSC_CTRL_REG, called
> EXT_LOSC_EN to enable/disable external low speed crystal oscillator.
>
> It also has bit EXT_LOSC_STA in LOSC_AUTO_SWT_STA_REG, to check whether
> external low speed oscillator is working correctly.
>
> This patch adds support for enabling LOSC when necessary:
>
> - during reparenting
> - when probing the clock
>
> H6 also has capacbility to automatically reparent RTC clock from
> external crystal oscillator, to internal RC oscillator, if external
> oscillator fails. This is enabled by default. Disable it during
> probe.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> Reviewed-by: Chen-Yu Tsai <wens@csie.org>
> ---
> drivers/rtc/rtc-sun6i.c | 40 ++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 38 insertions(+), 2 deletions(-)
>
Applied, thanks.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: Add compatible for H6 RTC
From: Alexandre Belloni @ 2019-08-22 21:15 UTC (permalink / raw)
To: megous
Cc: Mark Rutland, Alessandro Zummo, devicetree, Maxime Ripard,
linux-sunxi, linux-kernel, Chen-Yu Tsai, Rob Herring,
linux-arm-kernel, linux-rtc
In-Reply-To: <20190820151934.3860-2-megous@megous.com>
On 20/08/2019 17:19:32+0200, megous@megous.com wrote:
> From: Ondrej Jirman <megous@megous.com>
>
> RTC on H6 is similar to the one on H5 SoC, but incompatible in small
> details. See the driver for description of differences. For example
> H6 RTC needs to enable the external low speed oscillator. Add new
> compatible for this RTC.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
> .../bindings/rtc/allwinner,sun6i-a31-rtc.yaml | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
Applied, thanks.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH net-next v2 2/3] net: ethernet: mediatek: Re-add support SGMII
From: Russell King - ARM Linux admin @ 2019-08-22 21:11 UTC (permalink / raw)
To: René van Dorst
Cc: Nelson Chang, Frank Wunderlich, netdev, Sean Wang, linux-mips,
linux-mediatek, John Crispin, Matthias Brugger, Stefan Roese,
David S . Miller, linux-arm-kernel
In-Reply-To: <20190822195033.Horde.hEW8FBGNfFrugQOCv0gaDfx@www.vdorst.com>
Hi Rene,
On Thu, Aug 22, 2019 at 07:50:33PM +0000, René van Dorst wrote:
> Quoting Russell King - ARM Linux admin <linux@armlinux.org.uk>:
> > Isn't this set for Cisco SGMII as well as for 802.3 1000BASE-X and
> > the up-clocked 2500BASE-X modes?
> >
> > If so, is there a reason why 10Mbps and 100Mbps speeds aren't
> > supported on Cisco SGMII links?
>
> I can only tell a bit about the mt7622 SOC, datasheet tells me that:
>
> The SGMII is the interface between 10/100/1000/2500 Mbps PHY and Ethernet MAC,
> the spec is raised by Cisco in 1999, which is aims for pin reduction compare
> with the GMII. It uses 2 differential data pair for TX and RX with clock
> embedded bit stream to convey frame data and port ability information.
> The core leverages the 1000Base-X PCS and Auto-Negotiation from IEEE 802.3
> specification (clause 36/37). This IP can support up to 3.125G baud for
> 2.5Gbps
> (proprietary 2500Base-X) data rate of MAC by overclocking.
>
> Also features tells me: Support 10/100/1000/2500 Mbps in full duplex mode and
> 10/100 Mbps in half duplex mode.
Yep, that is what I'd expect. Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 2/2] rtc: Add Amlogic Virtual Wake RTC
From: Alexandre Belloni @ 2019-08-22 21:11 UTC (permalink / raw)
To: Kevin Hilman
Cc: linux-rtc, linux-amlogic, devicetree, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20190812232850.8016-3-khilman@kernel.org>
On 12/08/2019 16:28:50-0700, Kevin Hilman wrote:
> From: Neil Armstrong <narmstrong@baylibre.com>
>
> The Amlogic Meson GX SoCs uses a special register to store the
> time in seconds to wakeup after a system suspend.
>
> In order to be able to reuse the RTC wakealarm feature, this
> driver implements a fake RTC device which uses the system time
> to deduce a suspend delay.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> [khilman: rebase to v5.3-rc, rework and modernization]
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
> MAINTAINERS | 1 +
> drivers/rtc/Kconfig | 11 +++
> drivers/rtc/Makefile | 1 +
> drivers/rtc/rtc-meson-vrtc.c | 156 +++++++++++++++++++++++++++++++++++
> 4 files changed, 169 insertions(+)
> create mode 100644 drivers/rtc/rtc-meson-vrtc.c
>
Applied, thanks.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: rtc: new binding for Amlogic VRTC
From: Alexandre Belloni @ 2019-08-22 21:11 UTC (permalink / raw)
To: Kevin Hilman
Cc: linux-rtc, linux-amlogic, devicetree, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20190812232850.8016-2-khilman@kernel.org>
On 12/08/2019 16:28:49-0700, Kevin Hilman wrote:
> From: Kevin Hilman <khilman@baylibre.com>
>
> Add binding fo the new VRTC driver for Amlogic SoCs. The 64-bit
> family of SoCs only has an RTC managed by firmware, and this VRTC
> driver provides the simple, one-register firmware interface.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
> ---
> .../bindings/rtc/rtc-meson-vrtc.txt | 22 +++++++++++++++++++
> 1 file changed, 22 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/rtc/rtc-meson-vrtc.txt
>
Applied, thanks.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 0/2] rtc: add new Amlogic Virtual Wake RTC
From: Kevin Hilman @ 2019-08-22 20:52 UTC (permalink / raw)
To: Kevin Hilman, Alexandre Belloni, linux-rtc
Cc: linux-amlogic, devicetree, linux-arm-kernel, Neil Armstrong
In-Reply-To: <20190812232850.8016-1-khilman@kernel.org>
Kevin Hilman <khilman@kernel.org> writes:
> From: Kevin Hilman <khilman@baylibre.com>
>
> Add a new driver for the virtual wake RTC on Amlogic SoCs.
>
> The RTC is virtual from the Linux side because it's a hardware timer
> managed by firmware on the secure co-processor (SCP.) The interface
> is 1 register where a wakeup time (in seconds) is written. The SCP then
> uses this value to program an always-on timer.
Just FYI... this was originally tested on G12A and G12B SoCs, but has
now also been tested to work unmodified on the new SM1 SoC as well.
Kevin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 6/6] arm64: dts: meson-sm1-sei610: enable DVFS
From: Kevin Hilman @ 2019-08-22 20:49 UTC (permalink / raw)
To: Neil Armstrong, jbrunet
Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20190822142455.12506-7-narmstrong@baylibre.com>
Neil Armstrong <narmstrong@baylibre.com> writes:
> This enables DVFS for the Amlogic SM1 based SEI610 board by:
> - Adding the SM1 SoC OPPs taken from the vendor tree
> - Selecting the SM1 Clock controller instead of the G12A one
> - Adding the CPU rail regulator, PWM and OPPs for each CPU nodes.
>
> Each power supply can achieve 0.69V to 1.05V using a single PWM
> output clocked at 666KHz with an inverse duty-cycle.
>
> DVFS has been tested by running the arm64 cpuburn at [1] and cycling
> between all the possible cpufreq translations of each cluster and
> checking the final frequency using the clock-measurer, script at [2].
>
> [1] https://github.com/ssvb/cpuburn-arm/blob/master/cpuburn-a53.S
> [2] https://gist.github.com/superna9999/d4de964dbc0f84b7d527e1df2ddea25f
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Kevin Hilman <khilman@baylibre.com>
Tested-by: Kevin Hilman <khilman@baylibre.com>
Tested on meson-sm1-sei610 board using the userspace govenor to manually
walk through the available frequencies.
I'll queue this up when there's a stable clock tag I can use for patch
5/6.
Kevin
> ---
> .../boot/dts/amlogic/meson-sm1-sei610.dts | 59 ++++++++++++++--
> arch/arm64/boot/dts/amlogic/meson-sm1.dtsi | 69 +++++++++++++++++++
> 2 files changed, 124 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts b/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
> index 36ac2e4b970d..69966e2e0611 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
> +++ b/arch/arm64/boot/dts/amlogic/meson-sm1-sei610.dts
> @@ -19,10 +19,6 @@
> ethernet0 = ðmac;
> };
>
> - chosen {
> - stdout-path = "serial0:115200n8";
> - };
> -
> emmc_pwrseq: emmc-pwrseq {
> compatible = "mmc-pwrseq-emmc";
> reset-gpios = <&gpio BOOT_12 GPIO_ACTIVE_LOW>;
> @@ -136,6 +132,25 @@
> regulator-always-on;
> };
>
> + vddcpu: regulator-vddcpu {
> + /*
> + * SY8120B1ABC DC/DC Regulator.
> + */
> + compatible = "pwm-regulator";
> +
> + regulator-name = "VDDCPU";
> + regulator-min-microvolt = <690000>;
> + regulator-max-microvolt = <1050000>;
> +
> + vin-supply = <&dc_in>;
> +
> + pwms = <&pwm_AO_cd 1 1500 0>;
> + pwm-dutycycle-range = <100 0>;
> +
> + regulator-boot-on;
> + regulator-always-on;
> + };
> +
> vddio_ao1v8: regulator-vddio_ao1v8 {
> compatible = "regulator-fixed";
> regulator-name = "VDDIO_AO1V8";
> @@ -182,6 +197,34 @@
> hdmi-phandle = <&hdmi_tx>;
> };
>
> +&cpu0 {
> + cpu-supply = <&vddcpu>;
> + operating-points-v2 = <&cpu_opp_table>;
> + clocks = <&clkc CLKID_CPU_CLK>;
> + clock-latency = <50000>;
> +};
> +
> +&cpu1 {
> + cpu-supply = <&vddcpu>;
> + operating-points-v2 = <&cpu_opp_table>;
> + clocks = <&clkc CLKID_CPU1_CLK>;
> + clock-latency = <50000>;
> +};
> +
> +&cpu2 {
> + cpu-supply = <&vddcpu>;
> + operating-points-v2 = <&cpu_opp_table>;
> + clocks = <&clkc CLKID_CPU2_CLK>;
> + clock-latency = <50000>;
> +};
> +
> +&cpu3 {
> + cpu-supply = <&vddcpu>;
> + operating-points-v2 = <&cpu_opp_table>;
> + clocks = <&clkc CLKID_CPU3_CLK>;
> + clock-latency = <50000>;
> +};
> +
> ðmac {
> status = "okay";
> phy-handle = <&internal_ephy>;
> @@ -220,6 +263,14 @@
> clock-names = "clkin0";
> };
>
> +&pwm_AO_cd {
> + pinctrl-0 = <&pwm_ao_d_e_pins>;
> + pinctrl-names = "default";
> + clocks = <&xtal>;
> + clock-names = "clkin1";
> + status = "okay";
> +};
> +
> &pwm_ef {
> status = "okay";
> pinctrl-0 = <&pwm_e_pins>;
> diff --git a/arch/arm64/boot/dts/amlogic/meson-sm1.dtsi b/arch/arm64/boot/dts/amlogic/meson-sm1.dtsi
> index 37064d7f66c1..2b61406b0610 100644
> --- a/arch/arm64/boot/dts/amlogic/meson-sm1.dtsi
> +++ b/arch/arm64/boot/dts/amlogic/meson-sm1.dtsi
> @@ -50,6 +50,71 @@
> compatible = "cache";
> };
> };
> +
> + cpu_opp_table: opp-table {
> + compatible = "operating-points-v2";
> + opp-shared;
> +
> + opp-100000000 {
> + opp-hz = /bits/ 64 <100000000>;
> + opp-microvolt = <730000>;
> + };
> +
> + opp-250000000 {
> + opp-hz = /bits/ 64 <250000000>;
> + opp-microvolt = <730000>;
> + };
> +
> + opp-500000000 {
> + opp-hz = /bits/ 64 <500000000>;
> + opp-microvolt = <730000>;
> + };
> +
> + opp-667000000 {
> + opp-hz = /bits/ 64 <666666666>;
> + opp-microvolt = <750000>;
> + };
> +
> + opp-1000000000 {
> + opp-hz = /bits/ 64 <1000000000>;
> + opp-microvolt = <770000>;
> + };
> +
> + opp-1200000000 {
> + opp-hz = /bits/ 64 <1200000000>;
> + opp-microvolt = <780000>;
> + };
> +
> + opp-1404000000 {
> + opp-hz = /bits/ 64 <1404000000>;
> + opp-microvolt = <790000>;
> + };
> +
> + opp-1512000000 {
> + opp-hz = /bits/ 64 <1500000000>;
> + opp-microvolt = <800000>;
> + };
> +
> + opp-1608000000 {
> + opp-hz = /bits/ 64 <1608000000>;
> + opp-microvolt = <810000>;
> + };
> +
> + opp-1704000000 {
> + opp-hz = /bits/ 64 <1704000000>;
> + opp-microvolt = <850000>;
> + };
> +
> + opp-1800000000 {
> + opp-hz = /bits/ 64 <1800000000>;
> + opp-microvolt = <900000>;
> + };
> +
> + opp-1908000000 {
> + opp-hz = /bits/ 64 <1908000000>;
> + opp-microvolt = <950000>;
> + };
> + };
> };
>
> &cecb_AO {
> @@ -60,6 +125,10 @@
> compatible = "amlogic,meson-sm1-clk-measure";
> };
>
> +&clkc {
> + compatible = "amlogic,sm1-clkc";
> +};
> +
> ðmac {
> power-domains = <&pwrc PWRC_SM1_ETH_ID>;
> };
> --
> 2.22.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: clk: meson: add sm1 periph clock controller bindings
From: Kevin Hilman @ 2019-08-22 20:49 UTC (permalink / raw)
To: Neil Armstrong, jbrunet, devicetree
Cc: linux-amlogic, linux-kernel, linux-clk, linux-arm-kernel,
Neil Armstrong
In-Reply-To: <20190822142455.12506-2-narmstrong@baylibre.com>
Neil Armstrong <narmstrong@baylibre.com> writes:
> Update the documentation to support clock driver for the Amlogic SM1 SoC.
>
> SM1 clock tree is very close, the main differences are :
> - each CPU core can achieve a different frequency, albeit a common PLL
> - a similar tree as the clock tree has been added for the DynamIQ Shared Unit
> - has a new GP1 PLL used for the DynamIQ Shared Unit
> - SM1 has additional clocks like for CSI, NanoQ an other components
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Kevin Hilman <khilman@baylibre.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC v4 07/18] objtool: Introduce INSN_UNKNOWN type
From: Julien @ 2019-08-22 20:45 UTC (permalink / raw)
To: Josh Poimboeuf, Raphael Gault
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
linux-kernel, linux-arm-kernel
In-Reply-To: <20190822200406.jc3yf77pomxxwep6@treble>
Hi Josh,
On 22/08/19 21:04, Josh Poimboeuf wrote:
> On Fri, Aug 16, 2019 at 01:23:52PM +0100, Raphael Gault wrote:
>> On arm64 some object files contain data stored in the .text section.
>> This data is interpreted by objtool as instruction but can't be
>> identified as a valid one. In order to keep analysing those files we
>> introduce INSN_UNKNOWN type. The "unknown instruction" warning will thus
>> only be raised if such instructions are uncountered while validating an
>> execution branch.
>>
>> This change doesn't impact the x86 decoding logic since 0 is still used
>> as a way to specify an unknown type, raising the "unknown instruction"
>> warning during the decoding phase still.
>>
>> Signed-off-by: Raphael Gault <raphael.gault@arm.com>
>
> Is there a reason such data can't be moved to .rodata? That would seem
> like the proper fix.
>
Raphaël can confirm, if I remember correctly, that issue was encountered
on assembly files implementing crypto algorithms were some
words/double-words of data were in the middle of the .text. I think it
is done this way to make sure the data can be loaded in a single
instruction. So moving it to another section could impact the crypto
performance depending on the relocations.
That was my understanding at least.
Cheers,
--
Julien Thierry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 2/5] soc: amlogic: Add support for Everything-Else power domains controller
From: Kevin Hilman @ 2019-08-22 20:32 UTC (permalink / raw)
To: Neil Armstrong, ulf.hansson
Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-pm
In-Reply-To: <b6cfb770-76eb-00b1-e088-1a73b7978f33@baylibre.com>
Neil Armstrong <narmstrong@baylibre.com> writes:
> On 22/08/2019 01:16, Kevin Hilman wrote:
>> Neil Armstrong <narmstrong@baylibre.com> writes:
>>
>>> Add support for the General Purpose Amlogic Everything-Else Power controller,
>>> with the first support for G12A and SM1 SoCs dedicated to the VPU, PCIe,
>>> USB, NNA, GE2D and Ethernet Power Domains.
>>>
>>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>>
>> Nice! Thanks for generalizing this.
>>
>> A few comments/concerns below, but this is mostly ready.
[...]
>>> +#define VPU_PD(__name, __resets, __clks, __top_pd, __mem, __get_power) \
>>> + { \
>>> + .name = __name, \
>>> + .reset_names_count = ARRAY_SIZE(__resets), \
>>> + .reset_names = __resets, \
>>> + .clk_names_count = ARRAY_SIZE(__clks), \
>>> + .clk_names = __clks, \
>>> + .top_pd = __top_pd, \
>>> + .mem_pd_count = ARRAY_SIZE(__mem), \
>>> + .mem_pd = __mem, \
>>> + .get_power = __get_power, \
>>> + }
>>> +
>>> +#define TOP_PD(__name, __top_pd, __mem) \
>>> + { \
>>> + .name = __name, \
>>> + .top_pd = __top_pd, \
>>> + .mem_pd_count = ARRAY_SIZE(__mem), \
>>> + .mem_pd = __mem, \
>>> + }
>>
>> Why can't the TOP_PD domains also have a __get_power(). Shouldn't we
>> just be able to check the sleep_reg/sleep_mask like in the VPU case?
>
> It can, I can add it later, do we need it for this version ?
Yes please. Seems a pretty simple addition, let's have it from the
beginning.
>> Also, for readability, I think the arguments to VPU_PD and TOP_PD to
>> have the same order, at least for the common ones. IOW, __name,
>> __top_pd, __mem should be first.
>
> Sure, will fix
and you can add __get_power to the common list too.
[...]
>>> +static int meson_ee_clk_disable(struct meson_ee_pwrc_domain *pwrc_domain)
>>> +{
>>> + int i;
>>> +
>>> + for (i = 0 ; i < pwrc_domain->num_clks ; ++i)
>>> + clk_disable(pwrc_domain->clks[i]);
>>> +
>>> + for (i = 0 ; i < pwrc_domain->num_clks ; ++i)
>>> + clk_unprepare(pwrc_domain->clks[i]);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_ee_clk_enable(struct meson_ee_pwrc_domain *pwrc_domain)
>>> +{
>>> + int i, ret;
>>> +
>>> + for (i = 0 ; i < pwrc_domain->num_clks ; ++i) {
>>> + ret = clk_prepare(pwrc_domain->clks[i]);
>>> + if (ret)
>>> + goto fail_prepare;
>>> + }
>>> +
>>> + for (i = 0 ; i < pwrc_domain->num_clks ; ++i) {
>>> + ret = clk_enable(pwrc_domain->clks[i]);
>>> + if (ret)
>>> + goto fail_enable;
>>> + }
>>> +
>>> + return 0;
>>> +fail_enable:
>>> + while (--i)
>>> + clk_disable(pwrc_domain->clks[i]);
>>> +
>>> + /* Unprepare all clocks */
>>> + i = pwrc_domain->num_clks;
>>> +
>>> +fail_prepare:
>>> + while (--i)
>>> + clk_unprepare(pwrc_domain->clks[i]);
>>> +
>>> + return ret;
>>> +}
>>
>> Both the clk enable and disable functions above are just open-coding of
>> the clk_bulk equivalents. Please use clk_bulk_*, then you don't need
>> these helpers. (c.f. the RFT patch I did to convert the old driver to
>> clk_bulk[1])
>
> Yes, but clk_bulk takes _all_ the clocks from the node, you canot specify
> a list of names, maybe it's overengineered but I wanted to specify the
> exact resets and clocks for each power domain, clk_bulk doesn't provide this.
I've been going on the assumption that there's no reason to list clocks
in the pwrc DT node that you don't want managed by the driver. This
also seems to match the exisiting driver, and this new one.
What is the case where you would list clocks in the DT node for the
power-domain, but not want to manage them in the driver?
If there's no good reason to do that, then clk_bulk greatly simplifies
this code.
>>> +static int meson_ee_pwrc_off(struct generic_pm_domain *domain)
>>> +{
>>> + struct meson_ee_pwrc_domain *pwrc_domain =
>>> + container_of(domain, struct meson_ee_pwrc_domain, base);
>>> + int i;
>>> +
>>> + if (pwrc_domain->desc.top_pd)
>>> + regmap_update_bits(pwrc_domain->pwrc->regmap_ao,
>>> + pwrc_domain->desc.top_pd->sleep_reg,
>>> + pwrc_domain->desc.top_pd->sleep_mask,
>>> + pwrc_domain->desc.top_pd->sleep_mask);
>>> + udelay(20);
>>> +
>>> + for (i = 0 ; i < pwrc_domain->desc.mem_pd_count ; ++i)
>>> + regmap_update_bits(pwrc_domain->pwrc->regmap_hhi,
>>> + pwrc_domain->desc.mem_pd[i].reg,
>>> + pwrc_domain->desc.mem_pd[i].mask,
>>> + pwrc_domain->desc.mem_pd[i].mask);
>>> +
>>> + udelay(20);
>>> +
>>> + if (pwrc_domain->desc.top_pd)
>>> + regmap_update_bits(pwrc_domain->pwrc->regmap_ao,
>>> + pwrc_domain->desc.top_pd->iso_reg,
>>> + pwrc_domain->desc.top_pd->iso_mask,
>>> + pwrc_domain->desc.top_pd->iso_mask);
>>> +
>>> + if (pwrc_domain->num_clks) {
>>> + msleep(20);
>>> + meson_ee_clk_disable(pwrc_domain);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int meson_ee_pwrc_on(struct generic_pm_domain *domain)
>>> +{
>>> + struct meson_ee_pwrc_domain *pwrc_domain =
>>> + container_of(domain, struct meson_ee_pwrc_domain, base);
>>> + int i, ret;
>>> +
>>> + if (pwrc_domain->desc.top_pd)
>>> + regmap_update_bits(pwrc_domain->pwrc->regmap_ao,
>>> + pwrc_domain->desc.top_pd->sleep_reg,
>>> + pwrc_domain->desc.top_pd->sleep_mask, 0);
>>> + udelay(20);
>>> +
>>> + for (i = 0 ; i < pwrc_domain->desc.mem_pd_count ; ++i)
>>> + regmap_update_bits(pwrc_domain->pwrc->regmap_hhi,
>>> + pwrc_domain->desc.mem_pd[i].reg,
>>> + pwrc_domain->desc.mem_pd[i].mask, 0);
>>> +
>>> + udelay(20);
>>> +
>>> + ret = meson_ee_reset_assert(pwrc_domain);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (pwrc_domain->desc.top_pd)
>>> + regmap_update_bits(pwrc_domain->pwrc->regmap_ao,
>>> + pwrc_domain->desc.top_pd->iso_reg,
>>> + pwrc_domain->desc.top_pd->iso_mask, 0);
>>> +
>>> + ret = meson_ee_reset_deassert(pwrc_domain);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + return meson_ee_clk_enable(pwrc_domain);
>>> +}
>>> +
>>> +static int meson_ee_pwrc_init_domain(struct platform_device *pdev,
>>> + struct meson_ee_pwrc *sm1_pwrc,
>>> + struct meson_ee_pwrc_domain *dom)
>>> +{
>>> + dom->pwrc = sm1_pwrc;
>>> + dom->num_rstc = dom->desc.reset_names_count;
>>> + dom->num_clks = dom->desc.clk_names_count;
>>> +
>>> + if (dom->num_rstc) {
>>> + int rst;
>>> +
>>> + dom->rstc = devm_kcalloc(&pdev->dev, dom->num_rstc,
>>> + sizeof(struct reset_control *), GFP_KERNEL);
>>> + if (!dom->rstc)
>>> + return -ENOMEM;
>>> +
>>> + for (rst = 0 ; rst < dom->num_rstc ; ++rst) {
>>> + dom->rstc[rst] = devm_reset_control_get_exclusive(
>>> + &pdev->dev,
>>> + dom->desc.reset_names[rst]);
>>> + if (IS_ERR(dom->rstc[rst]))
>>> + return PTR_ERR(dom->rstc[rst]);
>>> + }
>>
>> Why not simplify and use the helpers that get multiple reset lines (like
>> devm_reset_control_array_get() used in meson-gx-pwrc-vpu.c)?
>
> Same comment as clk_bulk, we cannot be sure we select the right reset lines.
Again, what is the case for listing resets in the power-domain node that
you don't want to be managed by the driver?
>> You could also use reset_control_get_count() and compare to the expected
>> number (dom->num_rstc).
>
> This seems oversimplified
Similar to above, if you're always going to manage all the reset lines
in the DT node, then simple is good.
If there are reasons to list reset lines in the DT node that will not be
managed by the driver, I'm defintiely curious why.
If not, using the reset API helpers is much more readable and
maintainble IMO.
>>
>>> + }
>>> +
>>> + if (dom->num_clks) {
>>> + int clk;
>>> +
>>> + dom->clks = devm_kcalloc(&pdev->dev, dom->num_clks,
>>> + sizeof(struct clk *), GFP_KERNEL);
>>> + if (!dom->clks)
>>> + return -ENOMEM;
>>> +
>>> + for (clk = 0 ; clk < dom->num_clks ; ++clk) {
>>> + dom->clks[clk] = devm_clk_get(&pdev->dev,
>>> + dom->desc.clk_names[clk]);
>>> + if (IS_ERR(dom->clks[clk]))
>>> + return PTR_ERR(dom->clks[clk]);
>>> + }
>>> + }
>>
>> Please use clk_bulk API, and then just double-check that the number of
>> clocks found matches the expected number.
>
> Same, I'll either take all the clks and resets for the vpu power domain,
> or keep this code to make sure we get the right clocks and resets.
Similar to above. IMO, we should be sure to put the "right clocks and
resets" into the DT, and then simplify the code.
>>
>>> + dom->base.name = dom->desc.name;
>>> + dom->base.power_on = meson_ee_pwrc_on;
>>> + dom->base.power_off = meson_ee_pwrc_off;
>>> +
>>> + if (dom->desc.get_power) {
>>> + bool powered_off = dom->desc.get_power(dom);
>>
>> nit: insert blank line here
>>
>> More importantly, we defintely will have problem here in the
>> !powered_off case. TL;DR; the driver's state does not match the actual
>> hardware state.
>>
>> When powered_off = false, you're telling the genpd core that this domain
>> is already turned on. However, you haven't called _pwrc_on() yet for
>> the domain, which means internal state of the driver for this domain
>> (e.g. clock enables, resets, etc.) is not in sync with the HW. On
>> SEI610 this case is happending for the VPU, which seems to be enabled by
>> u-boot, so this driver detects it as already on, which is fine. But...
>>
>> Remember that the ->power_off() function will be called during suspend,
>> and will lead to the clk unprepare/disable calls. However, for domains
>> that are detected as on (!powered_off), clk prepare/enable will never
>> have been called, so that when suspend happens, you'll get "already
>> unprepared" errors from the clock core
>>
>> IOW, I think you need something like this here:
>>
>> if (!powered_off)
>> meson_ee_pwrc_on(&dom->base);
>>
>> so that the internal state of clock fwk etc. matches the detected state
>> of the HW. The problem with that simple fix, at least for the VPU is
>> that it might cause us to lose any existing display or framebuffer
>> console that's on-going. Probably needs some testing.
>
> Yes, I forgot to take the _shutdown() function from gx_pwrc_vpu driver :
>
> 349 static void meson_gx_pwrc_vpu_shutdown(struct platform_device *pdev)
> 350 {
> 351 struct meson_gx_pwrc_vpu *vpu_pd = platform_get_drvdata(pdev);
> 352 bool powered_off;
> 353
> 354 powered_off = meson_gx_pwrc_vpu_get_power(vpu_pd);
> 355 if (!powered_off)
> 356 vpu_pd->genpd.power_off(&vpu_pd->genpd);
> 357 }
OK, yeah, I hadn't noticed that in the original driver. I tested
something simliar with suspend/resume on SEI610 and it gets rid of the
"already unprepared" splats from the clock core.
>>
>> Anyways, to see what I mean, try suspend/resume (you can test this
>> series on my integ branch with "rtcwake -d rtc0 -m mem -s4") and you'll
>> see error splats from the clock core during suspend.
>>
>>
>>
>>> + pm_genpd_init(&dom->base, &pm_domain_always_on_gov,
>>> + powered_off);
>>
>>> + } else
>>> + pm_genpd_init(&dom->base, NULL, true);
>>
>> nit: the else clause should also have {} to match the if
>> (c.f. CodingStyle)
>
> OK
>
>>
>> Why do you force the always-on governor in the case where ->get_power()
>> exists, but not the other?
>>
>> If you force that, then for any devices connected to these domains that
>> use runtime PM, they will never turn off the domain when it's idle.
>> IOW, these domains will only ever be turned off on system-wide
>> suspend/resume.
>>
>> IMO, unless there's a good reason not to, you should pass NULL for the
>> governor.
>
> It's for legacy when VPU is initialized from vendor U-Boot, look at commit :
> 339cd0ea082287ea8e2b7e7159a5a33665a2cbe3 "soc: amlogic: meson-gx-pwrc-vpu: fix power-off when powered by bootloader"
>
> In the case the VPU power domain has been powered on by the bootloader
> and no driver are attached to this power domain, the genpd will power it
> off after a certain amount of time, but the clocks hasn't been enabled
> by the kernel itself and the power-off will trigger some faults.
> This patch enable the clocks to have a coherent state for an eventual
> poweroff and switches to the pm_domain_always_on_gov governor.
The key phrase there being "and no driver is attached". Now that we
have a driver, it claims this domain so I don't think it will be
powered off:
# cat /sys/kernel/debug/pm_genpd/pm_genpd_summary
domain status slaves
/device runtime status
----------------------------------------------------------------------
ETH on
/devices/platform/soc/ff3f0000.ethernet unsupported
AUDIO off-0
GE2D off-0
PCI off-0
USB on
/devices/platform/soc/ffe09000.usb active
NNA off-0
VPU on
/devices/platform/soc/ff900000.vpu unsupported
In my tests with a framebuffer console (over HDMI), I don't see the
display being powered off.
> I could set always-on governor only if the domain was already enabled,
> what do you think ?
I don't think that's necessary now that we have a driver. We really
want to be able to power-down this domain when the display is not in
use, and if you use always_on, that will never happen.
> And seems I'm also missing the "This patch enable the clocks".
I'm not sure what patch you're referring to.
Kevin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC v4 16/18] arm64: crypto: Add exceptions for crypto object to prevent stack analysis
From: Josh Poimboeuf @ 2019-08-22 20:19 UTC (permalink / raw)
To: Raphael Gault
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
linux-kernel, linux-arm-kernel, julien.thierry.kdev
In-Reply-To: <20190816122403.14994-17-raphael.gault@arm.com>
On Fri, Aug 16, 2019 at 01:24:01PM +0100, Raphael Gault wrote:
> Some crypto modules contain `.word` of data in the .text section.
> Since objtool can't make the distinction between data and incorrect
> instruction, it gives a warning about the instruction beeing unknown
> and stops the analysis of the object file.
>
> The exception can be removed if the data are moved to another section
> or if objtool is tweaked to handle this particular case.
>
> Signed-off-by: Raphael Gault <raphael.gault@arm.com>
If the data can be moved to .rodata then I think that would be a much
better solution.
> ---
> arch/arm64/crypto/Makefile | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm64/crypto/Makefile b/arch/arm64/crypto/Makefile
> index 0435f2a0610e..e2a25919ebaa 100644
> --- a/arch/arm64/crypto/Makefile
> +++ b/arch/arm64/crypto/Makefile
> @@ -43,9 +43,11 @@ aes-neon-blk-y := aes-glue-neon.o aes-neon.o
>
> obj-$(CONFIG_CRYPTO_SHA256_ARM64) += sha256-arm64.o
> sha256-arm64-y := sha256-glue.o sha256-core.o
> +OBJECT_FILES_NON_STANDARD_sha256-core.o := y
>
> obj-$(CONFIG_CRYPTO_SHA512_ARM64) += sha512-arm64.o
> sha512-arm64-y := sha512-glue.o sha512-core.o
> +OBJECT_FILES_NON_STANDARD_sha512-core.o := y
>
> obj-$(CONFIG_CRYPTO_CHACHA20_NEON) += chacha-neon.o
> chacha-neon-y := chacha-neon-core.o chacha-neon-glue.o
> @@ -58,6 +60,7 @@ aes-arm64-y := aes-cipher-core.o aes-cipher-glue.o
>
> obj-$(CONFIG_CRYPTO_AES_ARM64_BS) += aes-neon-bs.o
> aes-neon-bs-y := aes-neonbs-core.o aes-neonbs-glue.o
> +OBJECT_FILES_NON_STANDARD_aes-neonbs-core.o := y
>
> CFLAGS_aes-glue-ce.o := -DUSE_V8_CRYPTO_EXTENSIONS
>
> --
> 2.17.1
>
--
Josh
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC v4 05/18] objtool: special: Adapt special section handling
From: Julien @ 2019-08-22 20:18 UTC (permalink / raw)
To: Raphael Gault, linux-arm-kernel, linux-kernel, jpoimboe
Cc: peterz, catalin.marinas, will.deacon, raph.gault+kdev
In-Reply-To: <20190816122403.14994-6-raphael.gault@arm.com>
Hi Raphaël,
On 16/08/19 13:23, Raphael Gault wrote:
> This patch abstracts the few architecture dependent tests that are
> perform when handling special section and switch tables. It enables any
> architecture to ignore a particular CPU feature or not to handle switch
> tables.
I think it would be better if this patch only focused on CPU features
and leave dealing with switch table to subsequent patches.
>
> Signed-off-by: Raphael Gault <raphael.gault@arm.com>
> ---
> tools/objtool/arch/arm64/Build | 1 +
> tools/objtool/arch/arm64/arch_special.c | 22 +++++++++++++++
> .../objtool/arch/arm64/include/arch_special.h | 10 +++++--
> tools/objtool/arch/x86/Build | 1 +
> tools/objtool/arch/x86/arch_special.c | 28 +++++++++++++++++++
> tools/objtool/arch/x86/include/arch_special.h | 9 ++++++
> tools/objtool/check.c | 24 ++++++++++++++--
> tools/objtool/special.c | 9 ++----
> tools/objtool/special.h | 3 ++
> 9 files changed, 96 insertions(+), 11 deletions(-)
> create mode 100644 tools/objtool/arch/arm64/arch_special.c
> create mode 100644 tools/objtool/arch/x86/arch_special.c
>
> diff --git a/tools/objtool/arch/arm64/Build b/tools/objtool/arch/arm64/Build
> index bf7a32c2b9e9..3d09be745a84 100644
> --- a/tools/objtool/arch/arm64/Build
> +++ b/tools/objtool/arch/arm64/Build
> @@ -1,3 +1,4 @@
> +objtool-y += arch_special.o
> objtool-y += decode.o
> objtool-y += orc_dump.o
> objtool-y += orc_gen.o
> diff --git a/tools/objtool/arch/arm64/arch_special.c b/tools/objtool/arch/arm64/arch_special.c
> new file mode 100644
> index 000000000000..a21d28876317
> --- /dev/null
> +++ b/tools/objtool/arch/arm64/arch_special.c
> @@ -0,0 +1,22 @@
> +/*
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "../../special.h"
> +#include "arch_special.h"
> +
> +void arch_force_alt_path(unsigned short feature,
> + bool uaccess,
> + struct special_alt *alt)
> +{
> +}
> diff --git a/tools/objtool/arch/arm64/include/arch_special.h b/tools/objtool/arch/arm64/include/arch_special.h
> index 63da775d0581..185103be8a51 100644
> --- a/tools/objtool/arch/arm64/include/arch_special.h
> +++ b/tools/objtool/arch/arm64/include/arch_special.h
> @@ -30,7 +30,13 @@
> #define ALT_ORIG_LEN_OFFSET 10
> #define ALT_NEW_LEN_OFFSET 11
>
> -#define X86_FEATURE_POPCNT (4 * 32 + 23)
> -#define X86_FEATURE_SMAP (9 * 32 + 20)
> +static inline bool arch_should_ignore_feature(unsigned short feature)
> +{
> + return false;
> +}
>
> +static inline bool arch_support_switch_table(void)
> +{
> + return false;
If I understand correctly , this gets later (patch 8) replaced by
arch_find_switch_table() which can return NULL if unable to find a
switch table.
So, is it necessary to introduce this function at this stage of the
patchset? Can't we just have directly arch_find_switch_table() ?
Also, I believe that in the end you keep the two
arch_support_switch_table() implementations (x86 and arm64) despite
getting rid of the only caller (in patch 8).
> +}
> #endif /* _ARM64_ARCH_SPECIAL_H */
> diff --git a/tools/objtool/arch/x86/Build b/tools/objtool/arch/x86/Build
> index 1f11b45999d0..63e167775bc8 100644
> --- a/tools/objtool/arch/x86/Build
> +++ b/tools/objtool/arch/x86/Build
> @@ -1,3 +1,4 @@
> +objtool-y += arch_special.o
> objtool-y += decode.o
> objtool-y += orc_dump.o
> objtool-y += orc_gen.o
> diff --git a/tools/objtool/arch/x86/arch_special.c b/tools/objtool/arch/x86/arch_special.c
> new file mode 100644
> index 000000000000..6583a1770bb2
> --- /dev/null
> +++ b/tools/objtool/arch/x86/arch_special.c
> @@ -0,0 +1,28 @@
> +/*
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#include "../../special.h"
> +#include "arch_special.h"
> +
> +void arch_force_alt_path(unsigned short feature,
> + bool uaccess,
Maybe you could have something like:
void arch_select_alt_path(unsigned short feature, struct special_alt *alt);
or arch_process_alt_inst(...);
This way you just let the arch decide what to do for alternative
instructions associated with their features, and call it in the "if
(entry->feature)" branch of get_alt_entry().
You don't need to pass the "uaccess" as that info is globally accessible
from builtin.h.
> + struct special_alt *alt)
> +{
> + if (feature == X86_FEATURE_SMAP) {
> + if (uaccess)
> + alt->skip_orig = true;
> + else
> + alt->skip_alt = true;
> + }
> +}
> diff --git a/tools/objtool/arch/x86/include/arch_special.h b/tools/objtool/arch/x86/include/arch_special.h
> index 424ce47013e3..fce2b1193194 100644
> --- a/tools/objtool/arch/x86/include/arch_special.h
> +++ b/tools/objtool/arch/x86/include/arch_special.h
> @@ -33,4 +33,13 @@
> #define X86_FEATURE_POPCNT (4 * 32 + 23)
> #define X86_FEATURE_SMAP (9 * 32 + 20)
>
> +static inline bool arch_should_ignore_feature(unsigned short feature)
> +{
> + return feature == X86_FEATURE_POPCNT;
> +}
With my above suggestion you shouldn't need that and just use the arch
specific alternative handling set the alt->skip_orig.
> +
> +static inline bool arch_support_switch_table(void)
> +{
> + return true;
> +}
> #endif /* _X86_ARCH_SPECIAL_H */
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 30e147391dcb..4af6422d3428 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -729,7 +729,7 @@ static int handle_group_alt(struct objtool_file *file,
> last_orig_insn = insn;
> }
>
> - if (next_insn_same_sec(file, last_orig_insn)) {
> + if (last_orig_insn && next_insn_same_sec(file, last_orig_insn)) {
> fake_jump = malloc(sizeof(*fake_jump));
> if (!fake_jump) {
> WARN("malloc failed");
> @@ -1061,6 +1061,26 @@ static struct rela *find_jump_table(struct objtool_file *file,
> table_rela = find_rela_by_dest(table_sec, table_offset);
> if (!table_rela)
> continue;
> + /*
> + * If we are on arm64 architecture, we now that we
> + * are in presence of a switch table thanks to
> + * the `br <Xn>` insn. but we can't retrieve it yet.
> + * So we just ignore unreachable for this file.
> + */
> + if (!arch_support_switch_table()) {
> + file->ignore_unreachables = true;
> + return NULL;
> + }
> +
> + rodata_rela = find_rela_by_dest(rodata_sec, table_offset);
> + if (rodata_rela) {
> + /*
> + * Use of RIP-relative switch jumps is quite rare, and
> + * indicates a rare GCC quirk/bug which can leave dead
> + * code behind.
> + */
> + if (text_rela->type == R_X86_64_PC32)
> + file->ignore_unreachables = true;
>
> /*
> * Use of RIP-relative switch jumps is quite rare, and
> @@ -1864,7 +1884,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
> insn = first;
> sec = insn->sec;
>
> - if (insn->alt_group && list_empty(&insn->alts)) {
> + if (!insn->visited && insn->alt_group && list_empty(&insn->alts)) {
> WARN_FUNC("don't know how to handle branch to middle of alternative instruction group",
> sec, insn->offset);
> return 1;
> diff --git a/tools/objtool/special.c b/tools/objtool/special.c
> index b8ccee1b5382..7a0092d6e5b3 100644
> --- a/tools/objtool/special.c
> +++ b/tools/objtool/special.c
> @@ -81,7 +81,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
> * feature path which is a "very very small percentage of
> * machines".
> */
> - if (feature == X86_FEATURE_POPCNT)
> + if (arch_should_ignore_feature(feature))
> alt->skip_orig = true;
>
> /*
> @@ -93,12 +93,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
> * find paths that see the STAC but take the NOP instead of
> * CLAC and the other way around.
> */
> - if (feature == X86_FEATURE_SMAP) {
> - if (uaccess)
> - alt->skip_orig = true;
> - else
> - alt->skip_alt = true;
> - }
> + arch_force_alt_path(feature, uaccess, alt);
> }
>
> orig_rela = find_rela_by_dest(sec, offset + entry->orig);
> diff --git a/tools/objtool/special.h b/tools/objtool/special.h
> index 35061530e46e..90626a7e41cf 100644
> --- a/tools/objtool/special.h
> +++ b/tools/objtool/special.h
> @@ -27,5 +27,8 @@ struct special_alt {
> };
>
> int special_get_alts(struct elf *elf, struct list_head *alts);
> +void arch_force_alt_path(unsigned short feature,
> + bool uaccess,
> + struct special_alt *alt);
>
> #endif /* _SPECIAL_H */
>
CHeers,
--
Julien Thierry
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC v4 13/18] arm64: sleep: Prevent stack frame warnings from objtool
From: Josh Poimboeuf @ 2019-08-22 20:16 UTC (permalink / raw)
To: Raphael Gault
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
linux-kernel, linux-arm-kernel, julien.thierry.kdev
In-Reply-To: <20190816122403.14994-14-raphael.gault@arm.com>
On Fri, Aug 16, 2019 at 01:23:58PM +0100, Raphael Gault wrote:
> This code doesn't respect the Arm PCS but it is intended this
> way. Adapting it to respect the PCS would result in altering the
> behaviour.
>
> In order to suppress objtool's warnings, we setup a stack frame
> for __cpu_suspend_enter and annotate cpu_resume and _cpu_resume
> as having non-standard stack frames.
>
> Signed-off-by: Raphael Gault <raphael.gault@arm.com>
> ---
> arch/arm64/kernel/sleep.S | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S
> index f5b04dd8a710..55c7c099d32c 100644
> --- a/arch/arm64/kernel/sleep.S
> +++ b/arch/arm64/kernel/sleep.S
> @@ -1,5 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> #include <linux/errno.h>
> +#include <linux/frame.h>
> #include <linux/linkage.h>
> #include <asm/asm-offsets.h>
> #include <asm/assembler.h>
> @@ -90,6 +91,7 @@ ENTRY(__cpu_suspend_enter)
> str x0, [x1]
> add x0, x0, #SLEEP_STACK_DATA_SYSTEM_REGS
> stp x29, lr, [sp, #-16]!
> + mov x29, sp
> bl cpu_do_suspend
> ldp x29, lr, [sp], #16
> mov x0, #1
> @@ -146,3 +148,6 @@ ENTRY(_cpu_resume)
> mov x0, #0
> ret
> ENDPROC(_cpu_resume)
> +
> + asm_stack_frame_non_standard cpu_resume
> + asm_stack_frame_non_standard _cpu_resume
We usually put these annotations immediately after the functions they're
annotating. And they should resemble the C macros like
STACK_FRAME_NON_STANDARD(cpu_resume)
--
Josh
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC v4 12/18] arm64: assembler: Add macro to annotate asm function having non standard stack-frame.
From: Josh Poimboeuf @ 2019-08-22 20:10 UTC (permalink / raw)
To: Raphael Gault
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
linux-kernel, linux-arm-kernel, julien.thierry.kdev
In-Reply-To: <20190816122403.14994-13-raphael.gault@arm.com>
On Fri, Aug 16, 2019 at 01:23:57PM +0100, Raphael Gault wrote:
> Some functions don't have standard stack-frames but are intended
> this way. In order for objtool to ignore those particular cases
> we add a macro that enables us to annotate the cases we chose
> to mark as particular.
>
> Signed-off-by: Raphael Gault <raphael.gault@arm.com>
> ---
> include/linux/frame.h | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/frame.h b/include/linux/frame.h
> index 02d3ca2d9598..1e35e58ab259 100644
> --- a/include/linux/frame.h
> +++ b/include/linux/frame.h
> @@ -11,14 +11,31 @@
> *
> * For more information, see tools/objtool/Documentation/stack-validation.txt.
> */
> +#ifndef __ASSEMBLY__
> #define STACK_FRAME_NON_STANDARD(func) \
> static void __used __section(.discard.func_stack_frame_non_standard) \
> *__func_stack_frame_non_standard_##func = func
> +#else
> + /*
> + * This macro is the arm64 assembler equivalent of the
> + * macro STACK_FRAME_NON_STANDARD define at
> + * ~/include/linux/frame.h
> + */
This comment is a bit confusing as it's referring to its own header
file. And it's not arm64-specific. I don't think we really need a
comment here anyway.
> + .macro asm_stack_frame_non_standard func
> + .pushsection ".discard.func_stack_frame_non_standard"
> + .quad \func
> + .popsection
> + .endm
Can you call it STACK_FRAME_NON_STANDARD for consistency with the
non-asm version?
--
Josh
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ARM: imx: Drop imx_anatop_init()
From: Andrey Smirnov @ 2019-08-22 20:06 UTC (permalink / raw)
To: Leonard Crestez
Cc: Aisheng Dong, Peter Chen, Shawn Guo, linux-kernel@vger.kernel.org,
dl-linux-imx, Chris Healy, Fabio Estevam,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <VI1PR04MB7023AE3910B261877892EEABEEA50@VI1PR04MB7023.eurprd04.prod.outlook.com>
On Thu, Aug 22, 2019 at 10:33 AM Leonard Crestez
<leonard.crestez@nxp.com> wrote:
>
> On 31.07.2019 21:01, Andrey Smirnov wrote:
> > With commit b5bbe2235361 ("usb: phy: mxs: Disable external charger
> > detect in mxs_phy_hw_init()") in tree all of the necessary charger
> > setup is done by the USB PHY driver which covers all of the affected
> > i.MX6 SoCs.
> >
> > NOTE: Imx_anatop_init() was also called for i.MX7D, but looking at its
> > datasheet it appears to have a different USB PHY IP block, so
> > executing i.MX6 charger disable configuration seems unnecessary.
> >
> > -void __init imx_anatop_init(void)
> > -{
> > - anatop = syscon_regmap_lookup_by_compatible("fsl,imx6q-anatop");
> > - if (IS_ERR(anatop)) {
> > - pr_err("%s: failed to find imx6q-anatop regmap!\n", __func__);
> > - return;
> > - }
>
> This patch breaks suspend on imx6 in linux-next because the "anatop"
> regmap is no longer initialized. This was found via bisect but
> no_console_suspend prints a helpful stack anyway:
>
> (regmap_read) from [<c01226e4>] (imx_anatop_enable_weak2p5+0x28/0x70)
> (imx_anatop_enable_weak2p5) from [<c0122744>]
> (imx_anatop_pre_suspend+0x18/0x64)
> (imx_anatop_pre_suspend) from [<c0124434>] (imx6q_pm_enter+0x60/0x16c)
> (imx6q_pm_enter) from [<c018c8a4>] (suspend_devices_and_enter+0x7d4/0xcbc)
> (suspend_devices_and_enter) from [<c018d544>] (pm_suspend+0x7b8/0x904)
> (pm_suspend) from [<c018b1b4>] (state_store+0x68/0xc8)
>
My bad, completely missed that fact that anatop was a global variable
in imx_anatop_init(). Sorry about that.
> Minimal fix looks like this:
>
> --- arch/arm/mach-imx/anatop.c
> +++ arch/arm/mach-imx/anatop.c
> @@ -111,6 +111,12 @@ void __init imx_init_revision_from_anatop(void)
> digprog = readl_relaxed(anatop_base + offset);
> iounmap(anatop_base);
>
> + anatop = syscon_regmap_lookup_by_compatible("fsl,imx6q-anatop");
> + if (IS_ERR(anatop)) {
> + pr_err("failed to find imx6q-anatop regmap!\n");
> + return;
> + }
>
> Since all SOCs that called imx_anatop_init also call
> imx_init_revision_from_anatop this might be an acceptable solution,
> unless there is some limitation preventing early regmap lookup.
>
Would making every function that uses anatop explicitly request it via
syscon_regmap_lookup_by_compatible("fsl,imx6q-anatop") be too much of
a code duplication? This way we won't need to worry if
imx_init_revision_from_anatop() was called before any of them are
used.
Thanks,
Andrey Smirnov
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC v4 09/18] gcc-plugins: objtool: Add plugin to detect switch table on arm64
From: Josh Poimboeuf @ 2019-08-22 20:05 UTC (permalink / raw)
To: Raphael Gault
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
linux-kernel, linux-arm-kernel, julien.thierry.kdev
In-Reply-To: <20190816122403.14994-10-raphael.gault@arm.com>
On Fri, Aug 16, 2019 at 01:23:54PM +0100, Raphael Gault wrote:
> This plugins comes into play before the final 2 RTL passes of GCC and
"plugin"
> detects switch-tables that are to be outputed in the ELF and writes
> information in an "objtool_data" section which will be used by objtool.
The section should probably have a ".discard" prefix
(.discard.objtool_data) so it gets discarded at link time.
Also, "objtool_data" is a bit generic. How about
".discard.switch_tables" or something.
>
> Signed-off-by: Raphael Gault <raphael.gault@arm.com>
> ---
> scripts/Makefile.gcc-plugins | 2 +
> scripts/gcc-plugins/Kconfig | 9 +++
> .../arm64_switch_table_detection_plugin.c | 58 +++++++++++++++++++
> 3 files changed, 69 insertions(+)
> create mode 100644 scripts/gcc-plugins/arm64_switch_table_detection_plugin.c
>
> diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
> index 5f7df50cfe7a..a56736df9dc2 100644
> --- a/scripts/Makefile.gcc-plugins
> +++ b/scripts/Makefile.gcc-plugins
> @@ -44,6 +44,8 @@ ifdef CONFIG_GCC_PLUGIN_ARM_SSP_PER_TASK
> endif
> export DISABLE_ARM_SSP_PER_TASK_PLUGIN
>
> +gcc-plugin-$(CONFIG_GCC_PLUGIN_SWITCH_TABLES) += arm64_switch_table_detection_plugin.so
> +
> # All the plugin CFLAGS are collected here in case a build target needs to
> # filter them out of the KBUILD_CFLAGS.
> GCC_PLUGINS_CFLAGS := $(strip $(addprefix -fplugin=$(objtree)/scripts/gcc-plugins/, $(gcc-plugin-y)) $(gcc-plugin-cflags-y))
> diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> index d33de0b9f4f5..1daeffb55dce 100644
> --- a/scripts/gcc-plugins/Kconfig
> +++ b/scripts/gcc-plugins/Kconfig
> @@ -113,4 +113,13 @@ config GCC_PLUGIN_ARM_SSP_PER_TASK
> bool
> depends on GCC_PLUGINS && ARM
>
> +config GCC_PLUGIN_SWITCH_TABLES
> + bool "GCC Plugin: Identify switch tables at compile time"
> + default y
> + depends on STACK_VALIDATION && ARM64
> + help
> + Plugin to identify switch tables generated at compile time and store
> + them in a .objtool_data section. Objtool will then use that section
> + to analyse the different execution path of the switch table.
This isn't something you want to ask the user about, as objtool for
arm64 requires it. For the same reason, instead of
GCC_PLUGIN_SWITCH_TABLES depending on STACK_VALIDATION, arm64
HAVE_STACK_VALIDATION should depend on GCC_PLUGIN_SWITCH_TABLES.
--
Josh
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC v4 08/18] objtool: Refactor switch-tables code to support other architectures
From: Josh Poimboeuf @ 2019-08-22 20:04 UTC (permalink / raw)
To: Raphael Gault
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
linux-kernel, linux-arm-kernel, julien.thierry.kdev
In-Reply-To: <20190816122403.14994-9-raphael.gault@arm.com>
On Fri, Aug 16, 2019 at 01:23:53PM +0100, Raphael Gault wrote:
> The way to identify switch-tables and retrieves all the data necessary
> to handle the different execution branches is not the same on all
> architecture. In order to be able to add other architecture support,
> this patch defines arch-dependent functions to process jump-tables.
>
> Signed-off-by: Raphael Gault <raphael.gault@arm.com>
> ---
> tools/objtool/arch/arm64/arch_special.c | 15 ++++
> tools/objtool/arch/arm64/decode.c | 4 +-
> tools/objtool/arch/x86/arch_special.c | 79 ++++++++++++++++++++
> tools/objtool/check.c | 95 +------------------------
> tools/objtool/check.h | 7 ++
> tools/objtool/special.h | 10 ++-
> 6 files changed, 114 insertions(+), 96 deletions(-)
>
> diff --git a/tools/objtool/arch/arm64/arch_special.c b/tools/objtool/arch/arm64/arch_special.c
> index a21d28876317..17a8a06aac2a 100644
> --- a/tools/objtool/arch/arm64/arch_special.c
> +++ b/tools/objtool/arch/arm64/arch_special.c
> @@ -20,3 +20,18 @@ void arch_force_alt_path(unsigned short feature,
> struct special_alt *alt)
> {
> }
> +
> +int arch_add_jump_table(struct objtool_file *file, struct instruction *insn,
> + struct rela *table, struct rela *next_table)
> +{
> + return 0;
> +}
> +
> +struct rela *arch_find_switch_table(struct objtool_file *file,
> + struct rela *text_rela,
> + struct section *rodata_sec,
> + unsigned long table_offset)
> +{
> + file->ignore_unreachables = true;
> + return NULL;
> +}
If this refactoring is done before adding arm64 support then you won't
need intermediate hacks like this.
--
Josh
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox