* [PATCH v2 2/2] trace, ras: add ARM RAS extension trace event
From: Ruidong Tian @ 2024-03-21 2:53 UTC (permalink / raw)
To: catalin.marinas, will, lpieralisi, guohanjun, sudeep.holla,
xueshuai, baolin.wang, linux-kernel, linux-acpi, linux-arm-kernel,
rafael, lenb, tony.luck, bp, linux-edac
Cc: tianruidond, Tyler Baicar, Ruidong Tian
In-Reply-To: <20240321025317.114621-1-tianruidong@linux.alibaba.com>
From: Tyler Baicar <baicar@os.amperecomputing.com>
Add a trace event for hardware errors reported by the ARMv8
RAS extension registers.
Signed-off-by: Tyler Baicar <baicar@os.amperecomputing.com>
Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
---
drivers/acpi/arm64/aest.c | 5 ++++
include/ras/ras_event.h | 55 +++++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/drivers/acpi/arm64/aest.c b/drivers/acpi/arm64/aest.c
index ab17aa5f5997..0cfe7fb9d4b9 100644
--- a/drivers/acpi/arm64/aest.c
+++ b/drivers/acpi/arm64/aest.c
@@ -15,6 +15,8 @@
#include <acpi/actbl.h>
#include <asm/ras.h>
+#include <ras/ras_event.h>
+
#undef pr_fmt
#define pr_fmt(fmt) "ACPI AEST: " fmt
@@ -153,6 +155,9 @@ static void aest_print(struct aest_node_llist *lnode)
pr_err("%s ERR%uMISC2: 0x%llx\n", pfx_seq, index, regs->err_misc[2]);
pr_err("%s ERR%uMISC3: 0x%llx\n", pfx_seq, index, regs->err_misc[3]);
}
+
+ trace_arm_ras_ext_event(lnode->type, lnode->id0, lnode->id1, index,
+ lnode->regs);
}
static void aest_handle_memory_failure(struct aest_node_llist *lnode)
diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h
index cbd3ddd7c33d..6003cab65ae4 100644
--- a/include/ras/ras_event.h
+++ b/include/ras/ras_event.h
@@ -338,6 +338,61 @@ TRACE_EVENT(aer_event,
"Not available")
);
+/*
+ * ARM RAS Extension Events Report
+ *
+ * This event is generated when an error reported by the ARM RAS extension
+ * hardware is detected.
+ */
+
+#ifdef CONFIG_ARM64_RAS_EXTN
+#include <asm/ras.h>
+TRACE_EVENT(arm_ras_ext_event,
+
+ TP_PROTO(u8 type, u32 id0, u32 id1, u32 index, struct ras_ext_regs *regs),
+
+ TP_ARGS(type, id0, id1, index, regs),
+
+ TP_STRUCT__entry(
+ __field(u8, type)
+ __field(u32, id0)
+ __field(u32, id1)
+ __field(u32, index)
+ __field(u64, err_fr)
+ __field(u64, err_ctlr)
+ __field(u64, err_status)
+ __field(u64, err_addr)
+ __field(u64, err_misc0)
+ __field(u64, err_misc1)
+ __field(u64, err_misc2)
+ __field(u64, err_misc3)
+ ),
+
+ TP_fast_assign(
+ __entry->type = type;
+ __entry->id0 = id0;
+ __entry->id1 = id1;
+ __entry->index = index;
+ __entry->err_fr = regs->err_fr;
+ __entry->err_ctlr = regs->err_ctlr;
+ __entry->err_status = regs->err_status;
+ __entry->err_addr = regs->err_addr;
+ __entry->err_misc0 = regs->err_misc[0];
+ __entry->err_misc1 = regs->err_misc[1];
+ __entry->err_misc2 = regs->err_misc[2];
+ __entry->err_misc3 = regs->err_misc[3];
+ ),
+
+ TP_printk("type: %d; id0: %d; id1: %d; index: %d; ERR_FR: %llx; ERR_CTLR: %llx; "
+ "ERR_STATUS: %llx; ERR_ADDR: %llx; ERR_MISC0: %llx; ERR_MISC1: %llx; "
+ "ERR_MISC2: %llx; ERR_MISC3: %llx",
+ __entry->type, __entry->id0, __entry->id1, __entry->index, __entry->err_fr,
+ __entry->err_ctlr, __entry->err_status, __entry->err_addr,
+ __entry->err_misc0, __entry->err_misc1, __entry->err_misc2,
+ __entry->err_misc3)
+);
+#endif
+
/*
* memory-failure recovery action result event
*
--
2.33.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: [PATCH v2 3/8] dt-bindings: opp: Describe H616 OPPs and opp-supported-hw
From: Viresh Kumar @ 2024-03-21 3:09 UTC (permalink / raw)
To: Andre Przywara
Cc: Rob Herring, Yangtao Li, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Rafael J . Wysocki, linux-pm,
devicetree, linux-sunxi, linux-arm-kernel, Brandon Cheo Fusi,
Martin Botka, Martin Botka
In-Reply-To: <20240320153738.3e2410bf@donnerap.manchester.arm.com>
On 20-03-24, 15:37, Andre Przywara wrote:
> On Wed, 20 Mar 2024 10:02:28 -0500
> Rob Herring <robh@kernel.org> wrote:
> > On Mon, Mar 18, 2024 at 01:12:23AM +0000, Andre Przywara wrote:
> > > From: Martin Botka <martin.botka@somainline.org>
> > > - opp-1080000000 {
> > > + opp-792000000-l {
> > > clock-latency-ns = <244144>; /* 8 32k periods */
> > > - opp-hz = /bits/ 64 <1080000000>;
> > > + opp-hz = /bits/ 64 <792000000>;
> > >
> > > - opp-microvolt-speed0 = <1060000>;
> > > - opp-microvolt-speed1 = <880000>;
> > > - opp-microvolt-speed2 = <840000>;
> > > + opp-microvolt = <900000>;
> > > + opp-supported-hw = <0x02>;
> > > };
> > >
> > > - opp-1320000000 {
> > > + opp-792000000-h {
> > > clock-latency-ns = <244144>; /* 8 32k periods */
> > > - opp-hz = /bits/ 64 <1320000000>;
> > > + opp-hz = /bits/ 64 <792000000>;
> > >
> > > - opp-microvolt-speed0 = <1160000>;
> > > - opp-microvolt-speed1 = <940000>;
> > > - opp-microvolt-speed2 = <900000>;
> > > + opp-microvolt = <940000>;
> > > + opp-supported-hw = <0x10>;
> >
> > So far, we've avoided multiple entries for a single frequency. I think
> > it would be good to maintain that.
>
> Fair, I wasn't super happy with that either, but it still seemed better
> than the alternatives.
>
> > Couldn't you just do:
> >
> > opp-supported-hw = <0>, <0x10>, <0x02>;
> >
> > Where the index corresponds to speed0, speed1, speed2.
> >
> > If not, then I don't understand how multiple entries of opp-supported-hw
> > are supposed to work.
>
> If I got this correctly, multiple cells in opp-supported-hw are to
> describe various levels of hierarchy for a chip version, so like silicon
> mask, metal layer revision, bin, I guess? The binding doc speaks of "cuts,
> substrate and process", not really sure what that means exactly.
Right. That basically translates to hardware versions the OPP will be parsed
for.
> I think currently we cannot easily combine microvolt suffixes and
> opp-supported-hw in one OPP node?
It should be fine.
> I think it bails out if one
> microvolt-speed<x> property is missing, but I have to double check.
> But IIRC v1 of this series somehow pulled that off, so we can maybe bring
> it back? To end up with:
> opp-792 {
> opp-hz = <792000000>;
> opp-microvolt-speed1 = <900000>;
> opp-microvolt-speed4 = <940000>;
> opp-supported-hw = <0x12>;
> };
That's what I thought too while reading your email.. Just populate the OPP for
both 0x10 and 0x02 versions and let the speedN thing get you the right voltage.
--
viresh
_______________________________________________
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/5] ASoC: fsl: Support register and unregister rpmsg sound card through remoteproc
From: Shengjiu Wang @ 2024-03-21 3:26 UTC (permalink / raw)
To: Chancel Liu
Cc: Xiubo.Lee, festevam, nicoleotsuka, lgirdwood, broonie, perex,
tiwai, shawnguo, s.hauer, kernel, linux-imx, alsa-devel,
linuxppc-dev, linux-sound, linux-kernel, linux-arm-kernel
In-Reply-To: <20240311111349.723256-1-chancel.liu@nxp.com>
On Mon, Mar 11, 2024 at 7:14 PM Chancel Liu <chancel.liu@nxp.com> wrote:
>
> echo /lib/firmware/fw.elf > /sys/class/remoteproc/remoteproc0/firmware
> (A) echo start > /sys/class/remoteproc/remoteproc0/state
> (B) echo stop > /sys/class/remoteproc/remoteproc0/state
>
> The rpmsg sound card is registered in (A) and unregistered in (B).
> After "start", imx-audio-rpmsg registers devices for ASoC platform driver
> and machine driver. Then sound card is registered. After "stop",
> imx-audio-rpmsg unregisters devices for ASoC platform driver and machine
> driver. Then sound card is unregistered.
Acked-by: Shengjiu Wang <shengjiu.wang@gmail.com>
Best regards
Shengjiu Wang
>
> changes in v2
> - Fix build errors reported by kernel test robot
>
> changes in v3
> - Add a new patch for fsl_rpmsg to register CPU DAI with rpmsg channel
> name
> - Update imx-rpmsg.c to get DT node of ASoC CPU DAI device with rpmsg
> channel name instead of using undocumented bindings
>
> Chancel Liu (5):
> ASoC: fsl: imx-pcm-rpmsg: Register component with rpmsg channel name
> ASoC: fsl: imx-audio-rpmsg: Register device with rpmsg channel name
> ASoC: fsl: Let imx-audio-rpmsg register platform device for card
> ASoC: fsl: fsl_rpmsg: Register CPU DAI with name of rpmsg channel
> ASoC: fsl: imx-rpmsg: Update to correct DT node
>
> sound/soc/fsl/fsl_rpmsg.c | 43 ++++++++++++++++++++-------------
> sound/soc/fsl/imx-audio-rpmsg.c | 21 +++++++++++++---
> sound/soc/fsl/imx-pcm-rpmsg.c | 11 ++++++---
> sound/soc/fsl/imx-rpmsg.c | 28 ++++++++++++++-------
> 4 files changed, 71 insertions(+), 32 deletions(-)
>
> --
> 2.43.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 v2 1/2] ACPI/AEST: Initial AEST driver
From: Baolin Wang @ 2024-03-21 3:52 UTC (permalink / raw)
To: Ruidong Tian, catalin.marinas, will, lpieralisi, guohanjun,
sudeep.holla, xueshuai, linux-kernel, linux-acpi,
linux-arm-kernel, rafael, lenb, tony.luck, bp, linux-edac
Cc: tianruidond, Tyler Baicar
In-Reply-To: <20240321025317.114621-2-tianruidong@linux.alibaba.com>
On 2024/3/21 10:53, Ruidong Tian wrote:
> From: Tyler Baicar <baicar@os.amperecomputing.com>
>
> Add support for parsing the ARM Error Source Table and basic handling of
> errors reported through both memory mapped and system register interfaces.
>
> Assume system register interfaces are only registered with private
> peripheral interrupts (PPIs); otherwise there is no guarantee the
> core handling the error is the core which took the error and has the
> syndrome info in its system registers.
>
> In kernel-first mode, all configuration is controlled by kernel, include
> CE ce_threshold and interrupt enable/disable.
>
> All detected errors will be processed as follow:
> - CE, DE: use a workqueue to log this hardware errors.
> - UER, UEO: log it and call memory_failure in workquee.
> - UC, UEU: panic in irq context.
>
> Signed-off-by: Tyler Baicar <baicar@os.amperecomputing.com>
> Signed-off-by: Ruidong Tian <tianruidong@linux.alibaba.com>
> ---
> MAINTAINERS | 11 +
> arch/arm64/include/asm/ras.h | 71 +++
> drivers/acpi/arm64/Kconfig | 10 +
> drivers/acpi/arm64/Makefile | 1 +
> drivers/acpi/arm64/aest.c | 834 +++++++++++++++++++++++++++++++++++
> include/linux/acpi_aest.h | 92 ++++
> include/linux/cpuhotplug.h | 1 +
> 7 files changed, 1020 insertions(+)
> create mode 100644 arch/arm64/include/asm/ras.h
> create mode 100644 drivers/acpi/arm64/aest.c
> create mode 100644 include/linux/acpi_aest.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dd5de540ec0b..34900d4bb677 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -330,6 +330,17 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> S: Maintained
> F: drivers/acpi/arm64
>
> +ACPI AEST
> +M: Tyler Baicar <baicar@os.amperecomputing.com>
> +M: Ruidong Tian <tianruidond@linux.alibaba.com>
> +L: linux-acpi@vger.kernel.org
> +L: linux-arm-kernel@lists.infradead.org
> +S: Supported
> +F: arch/arm64/include/asm/ras.h
> +F: drivers/acpi/arm64/aest.c
> +F: include/linux/acpi_aest.h
> +
> +
> ACPI FOR RISC-V (ACPI/riscv)
> M: Sunil V L <sunilvl@ventanamicro.com>
> L: linux-acpi@vger.kernel.org
> diff --git a/arch/arm64/include/asm/ras.h b/arch/arm64/include/asm/ras.h
> new file mode 100644
> index 000000000000..04667f0de30f
> --- /dev/null
> +++ b/arch/arm64/include/asm/ras.h
> @@ -0,0 +1,71 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_RAS_H
> +#define __ASM_RAS_H
> +
> +#include <linux/types.h>
> +#include <linux/bits.h>
> +
> +/* ERR<n>FR */
> +#define ERR_FR_RP BIT(15)
> +#define ERR_FR_CEC GENMASK_ULL(14, 12)
> +
> +#define ERR_FR_RP_SINGLE_COUNTER 0
> +#define ERR_FR_RP_DOUBLE_COUNTER 1
> +
> +#define ERR_FR_CEC_0B_COUNTER 0
> +#define ERR_FR_CEC_8B_COUNTER BIT(1)
> +#define ERR_FR_CEC_16B_COUNTER BIT(2)
> +
> +/* ERR<n>STATUS */
> +#define ERR_STATUS_AV BIT(31)
> +#define ERR_STATUS_V BIT(30)
> +#define ERR_STATUS_UE BIT(29)
> +#define ERR_STATUS_ER BIT(28)
> +#define ERR_STATUS_OF BIT(27)
> +#define ERR_STATUS_MV BIT(26)
> +#define ERR_STATUS_CE (BIT(25) | BIT(24))
> +#define ERR_STATUS_DE BIT(23)
> +#define ERR_STATUS_PN BIT(22)
> +#define ERR_STATUS_UET (BIT(21) | BIT(20))
> +#define ERR_STATUS_CI BIT(19)
> +#define ERR_STATUS_IERR GENMASK_ULL(15, 8)
> +#define ERR_STATUS_SERR GENMASK_ULL(7, 0)
> +
> +/* These bit is write-one-to-clear */
> +#define ERR_STATUS_W1TC (ERR_STATUS_AV | ERR_STATUS_V | ERR_STATUS_UE | \
> + ERR_STATUS_ER | ERR_STATUS_OF | ERR_STATUS_MV | \
> + ERR_STATUS_CE | ERR_STATUS_DE | ERR_STATUS_PN | \
> + ERR_STATUS_UET | ERR_STATUS_CI)
> +
> +#define ERR_STATUS_UET_UC 0
> +#define ERR_STATUS_UET_UEU 1
> +#define ERR_STATUS_UET_UER 2
> +#define ERR_STATUS_UET_UEO 3
> +
> +/* ERR<n>CTLR */
> +#define ERR_CTLR_FI BIT(3)
> +#define ERR_CTLR_UI BIT(2)
> +
> +/* ERR<n>ADDR */
> +#define ERR_ADDR_AI BIT(61)
> +#define ERR_ADDR_PADDR GENMASK_ULL(55, 0)
> +
> +/* ERR<n>MISC0 */
> +
> +/* ERR<n>FR.CEC == 0b010, ERR<n>FR.RP == 0 */
> +#define ERR_MISC0_8B_OF BIT(39)
> +#define ERR_MISC0_8B_CEC GENMASK_ULL(38, 32)
> +
> +/* ERR<n>FR.CEC == 0b100, ERR<n>FR.RP == 0 */
> +#define ERR_MISC0_16B_OF BIT(47)
> +#define ERR_MISC0_16B_CEC GENMASK_ULL(46, 32)
> +
> +struct ras_ext_regs {
> + u64 err_fr;
> + u64 err_ctlr;
> + u64 err_status;
> + u64 err_addr;
> + u64 err_misc[4];
> +};
> +
> +#endif /* __ASM_RAS_H */
> diff --git a/drivers/acpi/arm64/Kconfig b/drivers/acpi/arm64/Kconfig
> index b3ed6212244c..639db671c5cf 100644
> --- a/drivers/acpi/arm64/Kconfig
> +++ b/drivers/acpi/arm64/Kconfig
> @@ -21,3 +21,13 @@ config ACPI_AGDI
>
> config ACPI_APMT
> bool
> +
> +config ACPI_AEST
> + bool "ARM Error Source Table Support"
> +
> + help
> + The Arm Error Source Table (AEST) provides details on ACPI
> + extensions that enable kernel-first handling of errors in a
> + system that supports the Armv8 RAS extensions.
> +
> + If set, the kernel will report and log hardware errors.
> diff --git a/drivers/acpi/arm64/Makefile b/drivers/acpi/arm64/Makefile
> index 143debc1ba4a..b5b740058c46 100644
> --- a/drivers/acpi/arm64/Makefile
> +++ b/drivers/acpi/arm64/Makefile
> @@ -5,3 +5,4 @@ obj-$(CONFIG_ACPI_GTDT) += gtdt.o
> obj-$(CONFIG_ACPI_APMT) += apmt.o
> obj-$(CONFIG_ARM_AMBA) += amba.o
> obj-y += dma.o init.o
> +obj-$(CONFIG_ACPI_AEST) += aest.o
> diff --git a/drivers/acpi/arm64/aest.c b/drivers/acpi/arm64/aest.c
> new file mode 100644
> index 000000000000..ab17aa5f5997
> --- /dev/null
> +++ b/drivers/acpi/arm64/aest.c
> @@ -0,0 +1,834 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ARM Error Source Table Support
> + *
> + * Copyright (c) 2021, Ampere Computing LLC
> + * Copyright (c) 2021-2024, Alibaba Group.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/acpi_aest.h>
> +#include <linux/cpuhotplug.h>
> +#include <linux/kernel.h>
> +#include <linux/genalloc.h>
> +#include <linux/llist.h>
> +#include <acpi/actbl.h>
> +#include <asm/ras.h>
> +
> +#undef pr_fmt
> +#define pr_fmt(fmt) "ACPI AEST: " fmt
> +
> +#define CASE_READ(res, x) \
> + case (x): { \
> + res = read_sysreg_s(SYS_##x##_EL1); \
> + break; \
> + }
> +
> +#define CASE_WRITE(val, x) \
> + case (x): { \
> + write_sysreg_s((val), SYS_##x##_EL1); \
> + break; \
> + }
> +
> +#define for_each_implemented_record(index, node) \
> + for ((index) = node->interface.record_start; \
> + (index) < node->interface.record_end; \
> + (index)++)
> +
> +#define AEST_LOG_PREFIX_BUFFER 64
> +
> +/*
> + * This memory pool is only to be used to save AEST node in AEST irq context.
> + * There can be 500 AEST node at most.
> + */
> +#define AEST_NODE_ALLOCED_MAX 500
> +
> +static struct acpi_table_header *aest_table;
> +
> +static struct aest_node __percpu **aest_ppi_data;
> +
> +static int *ppi_irqs;
> +static u8 num_ppi;
> +static u8 ppi_idx;
> +
> +static struct work_struct aest_work;
> +
> +static struct gen_pool *aest_node_pool;
> +static struct llist_head aest_node_llist;
> +
> +static u64 aest_sysreg_read(u64 __unused, u32 offset)
> +{
> + u64 res;
> +
> + switch (offset) {
> + CASE_READ(res, ERXFR)
> + CASE_READ(res, ERXCTLR)
> + CASE_READ(res, ERXSTATUS)
> + CASE_READ(res, ERXADDR)
> + CASE_READ(res, ERXMISC0)
> + CASE_READ(res, ERXMISC1)
> + CASE_READ(res, ERXMISC2)
> + CASE_READ(res, ERXMISC3)
> + default :
> + res = 0;
> + }
> + return res;
> +}
> +
> +static void aest_sysreg_write(u64 base, u32 offset, u64 val)
> +{
> + switch (offset) {
> + CASE_WRITE(val, ERXFR)
> + CASE_WRITE(val, ERXCTLR)
> + CASE_WRITE(val, ERXSTATUS)
> + CASE_WRITE(val, ERXADDR)
> + CASE_WRITE(val, ERXMISC0)
> + CASE_WRITE(val, ERXMISC1)
> + CASE_WRITE(val, ERXMISC2)
> + CASE_WRITE(val, ERXMISC3)
> + default :
> + return;
> + }
> +}
> +
> +static u64 aest_iomem_read(u64 base, u32 offset)
> +{
> + return readq_relaxed((void *)(base + offset));
> +}
> +
> +static void aest_iomem_write(u64 base, u32 offset, u64 val)
> +{
> + writeq_relaxed(val, (void *)(base + offset));
> +}
> +
> +static void aest_print(struct aest_node_llist *lnode)
> +{
> + static atomic_t seqno = { 0 };
> + unsigned int curr_seqno;
> + char pfx_seq[AEST_LOG_PREFIX_BUFFER];
> + int index;
> + struct ras_ext_regs *regs;
> +
> + curr_seqno = atomic_inc_return(&seqno);
> + snprintf(pfx_seq, sizeof(pfx_seq), "{%u}" HW_ERR, curr_seqno);
> + pr_info("%sHardware error from %s\n", pfx_seq, lnode->node_name);
> +
> + switch (lnode->type) {
> + case ACPI_AEST_PROCESSOR_ERROR_NODE:
> + pr_err("%s Error from CPU%d\n", pfx_seq, lnode->id0);
> + break;
> + case ACPI_AEST_MEMORY_ERROR_NODE:
> + pr_err("%s Error from memory at SRAT proximity domain 0x%x\n",
> + pfx_seq, lnode->id0);
> + break;
> + case ACPI_AEST_SMMU_ERROR_NODE:
> + pr_err("%s Error from SMMU IORT node 0x%x subcomponent 0x%x\n",
> + pfx_seq, lnode->id0, lnode->id1);
> + break;
> + case ACPI_AEST_VENDOR_ERROR_NODE:
> + pr_err("%s Error from vendor hid 0x%x uid 0x%x\n",
> + pfx_seq, lnode->id0, lnode->id1);
> + break;
> + case ACPI_AEST_GIC_ERROR_NODE:
> + pr_err("%s Error from GIC type 0x%x instance 0x%x\n",
> + pfx_seq, lnode->id0, lnode->id1);
> + break;
> + default:
> + pr_err("%s Unknown AEST node type\n", pfx_seq);
> + return;
> + }
> +
> + index = lnode->index;
> + regs = lnode->regs;
> +
> + pr_err("%s ERR%uFR: 0x%llx\n", pfx_seq, index, regs->err_fr);
> + pr_err("%s ERR%uCTRL: 0x%llx\n", pfx_seq, index, regs->err_ctlr);
> + pr_err("%s ERR%uSTATUS: 0x%llx\n", pfx_seq, index, regs->err_status);
> + if (regs->err_status & ERR_STATUS_AV)
> + pr_err("%s ERR%uADDR: 0x%llx\n", pfx_seq, index, regs->err_addr);
> +
> + if (regs->err_status & ERR_STATUS_MV) {
> + pr_err("%s ERR%uMISC0: 0x%llx\n", pfx_seq, index, regs->err_misc[0]);
> + pr_err("%s ERR%uMISC1: 0x%llx\n", pfx_seq, index, regs->err_misc[1]);
> + pr_err("%s ERR%uMISC2: 0x%llx\n", pfx_seq, index, regs->err_misc[2]);
> + pr_err("%s ERR%uMISC3: 0x%llx\n", pfx_seq, index, regs->err_misc[3]);
> + }
> +}
> +
> +static void aest_handle_memory_failure(struct aest_node_llist *lnode)
> +{
> + unsigned long pfn;
> + u64 addr;
> +
> + if (test_bit(lnode->index, &lnode->addressing_mode) ||
> + (lnode->regs->err_addr & ERR_ADDR_AI))
> + return;
> +
> + addr = lnode->regs->err_addr & (1UL << CONFIG_ARM64_PA_BITS);
> + pfn = PHYS_PFN(addr);
> +
> + if (!pfn_valid(pfn)) {
> + pr_warn(HW_ERR "Invalid physical address: %#llx\n", addr);
> + return;
> + }
> +
> + memory_failure(pfn, 0);
> +}
> +
> +static void aest_node_pool_process(struct work_struct *__unused)
> +{
> + struct llist_node *head;
> + struct aest_node_llist *lnode, *tmp;
> + u64 status;
> +
> + head = llist_del_all(&aest_node_llist);
> + if (!head)
> + return;
> +
> + head = llist_reverse_order(head);
> + llist_for_each_entry_safe(lnode, tmp, head, llnode) {
> + aest_print(lnode);
> +
> + status = lnode->regs->err_status;
> + if ((status & ERR_STATUS_UE) &&
> + (status & ERR_STATUS_UET) > ERR_STATUS_UET_UEU)
> + aest_handle_memory_failure(lnode);
> + gen_pool_free(aest_node_pool, (unsigned long)lnode,
> + sizeof(*lnode));
> + }
> +}
> +
> +static int aest_node_gen_pool_add(struct aest_node *node, int index,
> + struct ras_ext_regs *regs)
> +{
> + struct aest_node_llist *list;
> +
> + if (!aest_node_pool)
> + return -EINVAL;
> +
> + list = (void *)gen_pool_alloc(aest_node_pool, sizeof(*list));
> + if (!list)
> + return -ENOMEM;
> +
> + list->type = node->type;
> + list->node_name = node->name;
> + switch (node->type) {
> + case ACPI_AEST_PROCESSOR_ERROR_NODE:
> + list->id0 = node->spec.processor.processor_id;
> + if (node->spec.processor.flags & (ACPI_AEST_PROC_FLAG_SHARED |
> + ACPI_AEST_PROC_FLAG_GLOBAL))
> + list->id0 = smp_processor_id();
> +
> + list->id1 = node->spec.processor.resource_type;
> + break;
> + case ACPI_AEST_MEMORY_ERROR_NODE:
> + list->id0 = node->spec.memory.srat_proximity_domain;
> + break;
> + case ACPI_AEST_SMMU_ERROR_NODE:
> + list->id0 = node->spec.smmu.iort_node_reference;
> + list->id1 = node->spec.smmu.subcomponent_reference;
> + break;
> + case ACPI_AEST_VENDOR_ERROR_NODE:
> + list->id0 = node->spec.vendor.acpi_hid;
> + list->id1 = node->spec.vendor.acpi_uid;
> + break;
> + case ACPI_AEST_GIC_ERROR_NODE:
> + list->id0 = node->spec.gic.interface_type;
> + list->id1 = node->spec.gic.instance_id;
> + break;
> + default:
> + list->id0 = 0;
> + list->id1 = 0;
> + }
> +
> + list->regs = regs;
> + list->index = index;
> + list->addressing_mode = node->interface.addressing_mode;
> + llist_add(&list->llnode, &aest_node_llist);
> +
> + return 0;
> +}
> +
> +static int aest_node_pool_init(void)
> +{
> + unsigned long addr, size;
> + int rc;
> +
> + if (aest_node_pool)
> + return 0;
> +
> + size = ilog2(sizeof(struct aest_node_llist));
> + aest_node_pool = gen_pool_create(size, -1);
> + if (!aest_node_pool)
> + return -ENOMEM;
> +
> + addr = (unsigned long)vmalloc(PAGE_ALIGN(size * AEST_NODE_ALLOCED_MAX));
> + if (!addr)
> + goto err_pool_alloc;
> +
> + rc = gen_pool_add(aest_node_pool, addr, size, -1);
> + if (rc)
> + goto err_pool_add;
> +
> + return 0;
> +
> +err_pool_add:
> + vfree((void *)addr);
> +
> +err_pool_alloc:
> + gen_pool_destroy(aest_node_pool);
> +
> + return -ENOMEM;
> +}
> +
> +static void aest_log(struct aest_node *node, int index, struct ras_ext_regs *regs)
> +{
> + if (!aest_node_gen_pool_add(node, index, regs))
> + schedule_work(&aest_work);
> +}
> +
> +/*
> + * Each PE may has multi error record, you must selects an error record to
> + * be accessed through the Error Record System registers.
> + */
> +static inline void aest_select_record(struct aest_node *node, int i)
> +{
> + if (node->interface.type == ACPI_AEST_NODE_SYSTEM_REGISTER) {
> + write_sysreg_s(i, SYS_ERRSELR_EL1);
> + isb();
> + }
> +}
> +
> +/* Ensure all writes has taken effect. */
> +static inline void aest_sync(struct aest_node *node)
> +{
> + if (node->interface.type == ACPI_AEST_NODE_SYSTEM_REGISTER)
> + isb();
> +}
> +
> +static int aest_proc(struct aest_node *node)
> +{
> + struct ras_ext_regs regs = {0};
> + struct aest_access *access;
> + int i, count = 0;
> + u64 regs_p;
> +
> + for_each_implemented_record(i, node) {
> +
> + /* 1b: Error record at i index is not implemented */
> + if (test_bit(i, &node->interface.record_implemented))
> + continue;
> +
> + aest_select_record(node, i);
> +
> + access = node->access;
> + regs_p = (u64)&node->interface.regs[i];
> +
> + regs.err_status = access->read(regs_p, ERXSTATUS);
> + if (!(regs.err_status & ERR_STATUS_V))
> + continue;
> +
> + count++;
> +
> + if (regs.err_status & ERR_STATUS_AV)
> + regs.err_addr = access->read(regs_p, ERXADDR);
> +
> + regs.err_fr = access->read(regs_p, ERXFR);
> + regs.err_ctlr = access->read(regs_p, ERXCTLR);
> +
> + if (regs.err_status & ERR_STATUS_MV) {
> + regs.err_misc[0] = access->read(regs_p, ERXMISC0);
> + regs.err_misc[1] = access->read(regs_p, ERXMISC1);
> + regs.err_misc[2] = access->read(regs_p, ERXMISC2);
> + regs.err_misc[3] = access->read(regs_p, ERXMISC3);
> + }
> +
> + if (node->interface.flags & ACPI_AEST_INTERFACE_CLEAR_MISC) {
> + access->write(regs_p, ERXMISC0, 0);
> + access->write(regs_p, ERXMISC1, 0);
> + access->write(regs_p, ERXMISC2, 0);
> + access->write(regs_p, ERXMISC3, 0);
> + } else
> + access->write(regs_p, ERXMISC0,
> + node->interface.ce_threshold[i]);
> +
> + aest_log(node, i, ®s);
> +
> + /* panic if unrecoverable and uncontainable error encountered */
> + if ((regs.err_status & ERR_STATUS_UE) &&
> + (regs.err_status & ERR_STATUS_UET) < ERR_STATUS_UET_UER)
> + panic("AEST: unrecoverable error encountered");
> +
> + /* Write-one-to-clear the bits we've seen */
> + regs.err_status &= ERR_STATUS_W1TC;
> +
> + /* Multi bit filed need to write all-ones to clear. */
> + if (regs.err_status & ERR_STATUS_CE)
> + regs.err_status |= ERR_STATUS_CE;
> +
> + /* Multi bit filed need to write all-ones to clear. */
> + if (regs.err_status & ERR_STATUS_UET)
> + regs.err_status |= ERR_STATUS_UET;
> +
> + access->write(regs_p, ERXSTATUS, regs.err_status);
> +
> + aest_sync(node);
> + }
> +
> + return count;
> +}
> +
> +static irqreturn_t aest_irq_func(int irq, void *input)
> +{
> + struct aest_node *node = input;
> +
> + if (aest_proc(node))
> + return IRQ_HANDLED;
> +
> + return IRQ_NONE;
> +}
> +
> +static int __init aest_register_gsi(u32 gsi, int trigger, void *data,
> + irq_handler_t aest_irq_func)
> +{
> + int cpu, irq;
> +
> + irq = acpi_register_gsi(NULL, gsi, trigger, ACPI_ACTIVE_HIGH);
> +
> + if (irq == -EINVAL) {
> + pr_err("failed to map AEST GSI %d\n", gsi);
> + return -EINVAL;
> + }
IMO, should be:
if (irq < 0) {
pr_err("failed to map AEST GSI %d\n", gsi);
return irq;
}
> +
> + if (irq_is_percpu_devid(irq)) {
> + ppi_irqs[ppi_idx] = irq;
> + for_each_possible_cpu(cpu) {
> + memcpy(per_cpu_ptr(aest_ppi_data[ppi_idx], cpu), data,
> + sizeof(struct aest_node));
> + }
> + if (request_percpu_irq(irq, aest_irq_func, "AEST",
> + aest_ppi_data[ppi_idx++])) {
> + pr_err("failed to register AEST IRQ %d\n", irq);
> + return -EINVAL;
Do not override the error number.
> + }
> + } else {
> + if (request_irq(irq, aest_irq_func, IRQF_SHARED, "AEST",
> + data)) {
> + pr_err("failed to register AEST IRQ %d\n", irq);
> + return -EINVAL;
ditto.
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int __init aest_init_interrupts(struct acpi_aest_hdr *hdr,
> + struct aest_node *node)
> +{
> + struct acpi_aest_node_interrupt *interrupt;
> + int i, trigger, ret = 0, err_ctlr, regs_p;
> +
> + interrupt = ACPI_ADD_PTR(struct acpi_aest_node_interrupt, hdr,
> + hdr->node_interrupt_offset);
> +
> + for (i = 0; i < hdr->node_interrupt_count; i++, interrupt++) {
> + trigger = (interrupt->flags & AEST_INTERRUPT_MODE) ?
> + ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE;
> + if (aest_register_gsi(interrupt->gsiv, trigger, node,
> + aest_irq_func))
> + ret = -EINVAL;
Do not override the error number.
> + }
> +
> + /* Ensure RAS interrupt is enabled */
> + for_each_implemented_record(i, node) {
> + /* 1b: Error record at i index is not implemented */
> + if (test_bit(i, &node->interface.record_implemented))
> + continue;
> +
> + aest_select_record(node, i);
> +
> + regs_p = (u64)&node->interface.regs[i];
> +
> + err_ctlr = node->access->read(regs_p, ERXCTLR);
> +
> + if (interrupt->type == ACPI_AEST_NODE_FAULT_HANDLING)
> + err_ctlr |= ERR_CTLR_FI;
> + if (interrupt->type == ACPI_AEST_NODE_ERROR_RECOVERY)
> + err_ctlr |= ERR_CTLR_UI;
> +
> + node->access->write(regs_p, ERXCTLR, err_ctlr);
> +
> + aest_sync(node);
> + }
> +
> + return ret;
> +}
> +
> +static void __init set_aest_node_name(struct aest_node *node)
> +{
> + switch (node->type) {
> + case ACPI_AEST_PROCESSOR_ERROR_NODE:
> + node->name = kasprintf(GFP_KERNEL, "AEST-CPU%d",
> + node->spec.processor.processor_id);
> + break;
> + case ACPI_AEST_MEMORY_ERROR_NODE:
> + case ACPI_AEST_SMMU_ERROR_NODE:
> + case ACPI_AEST_VENDOR_ERROR_NODE:
> + case ACPI_AEST_GIC_ERROR_NODE:
> + node->name = kasprintf(GFP_KERNEL, "AEST-%llx",
> + node->interface.phy_addr);
> + break;
> + default:
> + node->name = kasprintf(GFP_KERNEL, "AEST-Unkown-Node");
IMO, better to check the return value for memory allocation.
> + }
> +}
> +
> +/* access type is decided by AEST interface type. */
> +static struct aest_access aest_access[] = {
> + [ACPI_AEST_NODE_SYSTEM_REGISTER] = {
> + .read = aest_sysreg_read,
> + .write = aest_sysreg_write,
> + },
> +
> + [ACPI_AEST_NODE_MEMORY_MAPPED] = {
> + .read = aest_iomem_read,
> + .write = aest_iomem_write,
> + },
> + { }
> +};
> +
> +/* In kernel-first mode, kernel will report every CE by default. */
> +static void __init aest_set_ce_threshold(struct aest_node *node)
> +{
> + u64 regs_p, err_fr, err_fr_cec, err_fr_rp, err_misc0, ce_threshold;
> + int i;
> +
> + for_each_implemented_record(i, node) {
> + /* 1b: Error record at i index is not implemented */
> + if (test_bit(i, &node->interface.record_implemented))
> + continue;
> +
> + aest_select_record(node, i);
> + regs_p = (u64)&node->interface.regs[i];
> +
> + err_fr = node->access->read(regs_p, ERXFR);
> + err_fr_cec = FIELD_GET(ERR_FR_CEC, err_fr);
> + err_fr_rp = FIELD_GET(ERR_FR_RP, err_fr);
> + err_misc0 = node->access->read(regs_p, ERXMISC0);
> +
> + if (err_fr_cec == ERR_FR_CEC_0B_COUNTER)
> + pr_debug("%s-%d do not support CE threshold!\n",
> + node->name, i);
> + else if (err_fr_cec == ERR_FR_CEC_8B_COUNTER &&
> + err_fr_rp == ERR_FR_RP_SINGLE_COUNTER) {
> + pr_debug("%s-%d support 8 bit CE threshold!\n",
> + node->name, i);
> + ce_threshold = err_misc0 | ERR_MISC0_8B_CEC;
> + } else if (err_fr_cec == ERR_FR_CEC_16B_COUNTER &&
> + err_fr_rp == ERR_FR_RP_SINGLE_COUNTER) {
> + pr_debug("%s-%d support 16 bit CE threshold!\n",
> + node->name, i);
> + ce_threshold = err_misc0 | ERR_MISC0_16B_CEC;
> + } else
> + pr_debug("%s-%d do not support double counter yet!\n",
> + node->name, i);
Change to 'switch' statement will be more readable.
> +
> + node->access->write(regs_p, ERXMISC0, ce_threshold);
> + node->interface.ce_threshold[i] = ce_threshold;
> +
> + aest_sync(node);
> + }
> +}
> +
> +static int __init aest_init_interface(struct acpi_aest_hdr *hdr,
> + struct aest_node *node)
> +{
> + struct acpi_aest_node_interface *interface;
> + struct resource *res;
> + int size;
> +
> + interface = ACPI_ADD_PTR(struct acpi_aest_node_interface, hdr,
> + hdr->node_interface_offset);
> +
> + if (interface->type >= ACPI_AEST_XFACE_RESERVED) {
> + pr_err("invalid interface type: %d\n", interface->type);
> + return -EINVAL;
> + }
> +
> + node->interface.type = interface->type;
> + node->interface.phy_addr = interface->address;
> + node->interface.record_start = interface->error_record_index;
> + node->interface.record_end = interface->error_record_index +
> + interface->error_record_count;
> + node->interface.flags = interface->flags;
> + node->interface.record_implemented = interface->error_record_implemented;
> + node->interface.status_reporting = interface->error_status_reporting;
> + node->interface.addressing_mode = interface->addressing_mode;
> + node->access = &aest_access[interface->type];
> +
> + /*
> + * Currently SR based handling is done through the architected
> + * discovery exposed through SRs. That may change in the future
> + * if there is supplemental information in the AEST that is
> + * needed.
> + */
> + if (interface->type == ACPI_AEST_NODE_SYSTEM_REGISTER)
> + return 0;
> +
> + res = kzalloc(sizeof(struct resource), GFP_KERNEL);
> + if (!res)
> + return -ENOMEM;
> +
> + size = interface->error_record_count * sizeof(struct ras_ext_regs);
> + res->name = "AEST";
> + res->start = interface->address;
> + res->end = res->start + size;
> + res->flags = IORESOURCE_MEM;
> +
> + if (insert_resource(&iomem_resource, res)) {
> + pr_notice("request region conflict with %s\n",
> + res->name);
> + }
> +
> + node->interface.regs = ioremap(res->start, size);
> + if (!node->interface.regs) {
> + pr_err("Ioremap for %s failed!\n", node->name);
> + kfree(res);
> + return -EINVAL;
return -ENOMEM;
> + }
> +
> + node->interface.ce_threshold = kzalloc(sizeof(u64) *
> + interface->error_record_count, GFP_KERNEL);
> + if (!node->interface.ce_threshold)
kfree(res) and iounmap()
> + return -ENOMEM;
> +
> + aest_set_ce_threshold(node);
> +
> + return 0;
> +}
> +
> +static int __init aest_init_common(struct acpi_aest_hdr *hdr,
> + struct aest_node *node)
> +{
> + int ret;
> +
> + set_aest_node_name(node);
> +
> + ret = aest_init_interface(hdr, node);
> + if (ret) {
> + pr_err("failed to init interface\n");
> + return ret;
I did not see you free the node->name before returning an error.
> + }
> +
> + return aest_init_interrupts(hdr, node);
> +}
> +
> +static int __init aest_init_node_default(struct acpi_aest_hdr *hdr)
> +{
> + struct aest_node *node;
> + union aest_node_spec *node_spec;
> + int ret;
> +
> + node = kzalloc(sizeof(struct aest_node), GFP_KERNEL);
> + if (!node)
> + return -ENOMEM;
> +
> + node->type = hdr->type;
> + node_spec = ACPI_ADD_PTR(union aest_node_spec, hdr,
> + hdr->node_specific_offset);
> +
> + memcpy(&node->spec, node_spec,
> + hdr->node_interface_offset - hdr->node_specific_offset);
> +
> + ret = aest_init_common(hdr, node);
> + if (ret)
> + kfree(node);
> +
> + return ret;
> +}
> +
> +static int __init aest_init_processor_node(struct acpi_aest_hdr *hdr)
> +{
> + struct aest_node *node;
> + union aest_node_spec *node_spec;
> + union aest_node_processor *proc;
> + int ret;
> +
> + node = kzalloc(sizeof(struct aest_node), GFP_KERNEL);
> + if (!node)
> + return -ENOMEM;
> +
> + node->type = hdr->type;
> + node_spec = ACPI_ADD_PTR(union aest_node_spec, hdr,
> + hdr->node_specific_offset);
> +
> + memcpy(&node->spec, node_spec,
> + hdr->node_interface_offset - hdr->node_specific_offset);
> +
> + proc = ACPI_ADD_PTR(union aest_node_processor, node_spec,
> + sizeof(acpi_aest_processor));
> +
> + switch (node->spec.processor.resource_type) {
> + case ACPI_AEST_CACHE_RESOURCE:
> + memcpy(&node->proc, proc,
> + sizeof(struct acpi_aest_processor_cache));
> + break;
> + case ACPI_AEST_TLB_RESOURCE:
> + memcpy(&node->proc, proc,
> + sizeof(struct acpi_aest_processor_tlb));
> + break;
> + case ACPI_AEST_GENERIC_RESOURCE:
> + memcpy(&node->proc, proc,
> + sizeof(struct acpi_aest_processor_generic));
> + break;
> + }
> +
> + ret = aest_init_common(hdr, node);
> + if (ret)
> + kfree(node);
> +
> + return ret;
> +}
> +
> +static int __init aest_init_node(struct acpi_aest_hdr *node)
> +{
> + switch (node->type) {
> + case ACPI_AEST_PROCESSOR_ERROR_NODE:
> + return aest_init_processor_node(node);
> + case ACPI_AEST_MEMORY_ERROR_NODE:
> + case ACPI_AEST_VENDOR_ERROR_NODE:
> + case ACPI_AEST_SMMU_ERROR_NODE:
> + case ACPI_AEST_GIC_ERROR_NODE:
> + return aest_init_node_default(node);
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static void __init aest_count_ppi(struct acpi_aest_hdr *header)
> +{
> + struct acpi_aest_node_interrupt *interrupt;
> + int i;
> +
> + interrupt = ACPI_ADD_PTR(struct acpi_aest_node_interrupt, header,
> + header->node_interrupt_offset);
> +
> + for (i = 0; i < header->node_interrupt_count; i++, interrupt++) {
> + if (interrupt->gsiv >= 16 && interrupt->gsiv < 32)
> + num_ppi++;
> + }
> +}
> +
> +static int aest_starting_cpu(unsigned int cpu)
> +{
> + int i;
> +
> + for (i = 0; i < num_ppi; i++)
> + enable_percpu_irq(ppi_irqs[i], IRQ_TYPE_NONE);
> +
> + return 0;
> +}
> +
> +static int aest_dying_cpu(unsigned int cpu)
> +{
> + int i;
> +
> + for (i = 0; i < num_ppi; i++)
> + disable_percpu_irq(ppi_irqs[i]);
> +
> + return 0;
> +}
> +
> +int __init acpi_aest_init(void)
Should be 'static'.
> +{
> + struct acpi_aest_hdr *aest_node, *aest_end;
> + struct acpi_table_aest *aest;
> + int i, ret = 0;
> +
> + if (acpi_disabled)
> + return 0;
> +
> + if (!IS_ENABLED(CONFIG_ARM64_RAS_EXTN))
> + return 0;
I think you can move this into Kconfig file, that makes ACPI_AEST
dependent on this CONFIG_ARM64_RAS_EXTN?
> +
> + if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_AEST, 0, &aest_table)))
> + return -EINVAL;
> +
> + ret = aest_node_pool_init();
> + if (ret) {
> + pr_err("Failed init aest node pool.\n");
> + goto fail;
Just return ret;
> + }
> +
> + INIT_WORK(&aest_work, aest_node_pool_process);
> +
> + aest = (struct acpi_table_aest *)aest_table;
> +
> + /* Get the first AEST node */
> + aest_node = ACPI_ADD_PTR(struct acpi_aest_hdr, aest,
> + sizeof(struct acpi_table_header));
> + /* Pointer to the end of the AEST table */
> + aest_end = ACPI_ADD_PTR(struct acpi_aest_hdr, aest,
> + aest_table->length);
> +
> + while (aest_node < aest_end) {
> + if (((u64)aest_node + aest_node->length) > (u64)aest_end) {
> + pr_err("AEST node pointer overflow, bad table.\n");
> + return -EINVAL;
You should destroy the node pool before returning errors.
> + }
> +
> + aest_count_ppi(aest_node);
> +
> + aest_node = ACPI_ADD_PTR(struct acpi_aest_hdr, aest_node,
> + aest_node->length);
> + }
> +
> + aest_ppi_data = kcalloc(num_ppi, sizeof(struct aest_node_data *),
> + GFP_KERNEL);
> + if (!aest_ppi_data) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> + ppi_irqs = kcalloc(num_ppi, sizeof(int), GFP_KERNEL);
> + if (!ppi_irqs) {
> + ret = -ENOMEM;
> + goto fail;
> + }
> +
> + for (i = 0; i < num_ppi; i++) {
> + aest_ppi_data[i] = alloc_percpu(struct aest_node);
> + if (!aest_ppi_data[i]) {
> + pr_err("Failed percpu allocation.\n");
> + ret = -ENOMEM;
> + goto fail;
> + }
> + }
> +
> + aest_node = ACPI_ADD_PTR(struct acpi_aest_hdr, aest,
> + sizeof(struct acpi_table_header));
> +
> + while (aest_node < aest_end) {
> + ret = aest_init_node(aest_node);
> + if (ret) {
> + pr_err("failed to init node: %d", ret);
> + goto fail;
> + }
> +
> + aest_node = ACPI_ADD_PTR(struct acpi_aest_hdr, aest_node,
> + aest_node->length);
> + }
> +
> +
> +
> + return cpuhp_setup_state(CPUHP_AP_ARM_AEST_STARTING,
> + "drivers/acpi/arm64/aest:starting",
> + aest_starting_cpu, aest_dying_cpu);
Need free the resources you requested if an error occurs.
> +
> +fail:
> + for (i = 0; i < num_ppi; i++)
> + free_percpu(aest_ppi_data[i]);
> + kfree(aest_ppi_data);
> + return ret;
> +}
> +subsys_initcall(acpi_aest_init);
> diff --git a/include/linux/acpi_aest.h b/include/linux/acpi_aest.h
> new file mode 100644
> index 000000000000..679187505dc6
> --- /dev/null
> +++ b/include/linux/acpi_aest.h
> @@ -0,0 +1,92 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef AEST_H
> +#define AEST_H
> +
> +#include <acpi/actbl.h>
> +#include <asm/ras.h>
> +
> +#define AEST_INTERRUPT_MODE BIT(0)
> +
> +#define ACPI_AEST_PROC_FLAG_GLOBAL (1<<0)
> +#define ACPI_AEST_PROC_FLAG_SHARED (1<<1)
> +
> +#define ACPI_AEST_INTERFACE_CLEAR_MISC (1<<0)
> +
> +#define ERXFR 0x0
> +#define ERXCTLR 0x8
> +#define ERXSTATUS 0x10
> +#define ERXADDR 0x18
> +#define ERXMISC0 0x20
> +#define ERXMISC1 0x28
> +#define ERXMISC2 0x30
> +#define ERXMISC3 0x38
> +
> +struct aest_node_interface {
> + u8 type;
> + u64 phy_addr;
> + u16 record_start;
> + u16 record_end;
> + u32 flags;
> + unsigned long record_implemented;
> + unsigned long status_reporting;
> + unsigned long addressing_mode;
> + struct ras_ext_regs *regs;
> + u64 *ce_threshold;
> +};
> +
> +union aest_node_processor {
> + struct acpi_aest_processor_cache cache_data;
> + struct acpi_aest_processor_tlb tlb_data;
> + struct acpi_aest_processor_generic generic_data;
> +};
> +
> +union aest_node_spec {
> + struct acpi_aest_processor processor;
> + struct acpi_aest_memory memory;
> + struct acpi_aest_smmu smmu;
> + struct acpi_aest_vendor vendor;
> + struct acpi_aest_gic gic;
> +};
> +
> +struct aest_access {
> + u64 (*read)(u64 base, u32 offset);
> + void (*write)(u64 base, u32 offset, u64 val);
> +};
> +
> +struct aest_node {
> + char *name;
> + u8 type;
> + struct aest_node_interface interface;
> + union aest_node_spec spec;
> + union aest_node_processor proc;
> + struct aest_access *access;
> +};
> +
> +struct aest_node_llist {
> + struct llist_node llnode;
> + char *node_name;
> + int type;
> + /*
> + * Different nodes have different meanings:
> + * - Processor node : processor number.
> + * - Memory node : SRAT proximity domain.
> + * - SMMU node : IORT proximity domain.
> + * - Vendor node : hardware ID.
> + * - GIC node : interface type.
> + */
> + u32 id0;
> + /*
> + * Different nodes have different meanings:
> + * - Processor node : processor resource type.
> + * - Memory node : Non.
> + * - SMMU node : subcomponent reference.
> + * - Vendor node : Unique ID.
> + * - GIC node : instance identifier.
> + */
> + u32 id1;
> + int index;
> + unsigned long addressing_mode;
> + struct ras_ext_regs *regs;
> +};
These are only aest-related structures? If so, I think they should be in
aest.c file.
> +
> +#endif /* AEST_H */
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index 624d4a38c358..f0dda08dbad2 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -186,6 +186,7 @@ enum cpuhp_state {
> CPUHP_AP_CSKY_TIMER_STARTING,
> CPUHP_AP_TI_GP_TIMER_STARTING,
> CPUHP_AP_HYPERV_TIMER_STARTING,
> + CPUHP_AP_ARM_AEST_STARTING,
> /* Must be the last timer callback */
> CPUHP_AP_DUMMY_TIMER_STARTING,
> CPUHP_AP_ARM_XEN_STARTING,
_______________________________________________
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 PATCH v3 5/5] mm: support large folios swapin as a whole
From: Huang, Ying @ 2024-03-21 4:23 UTC (permalink / raw)
To: Barry Song
Cc: Ryan Roberts, Matthew Wilcox, akpm, linux-mm, chengming.zhou,
chrisl, david, hannes, kasong, linux-arm-kernel, linux-kernel,
mhocko, nphamcs, shy828301, steven.price, surenb, wangkefeng.wang,
xiang, yosryahmed, yuzhao, Chuanhua Han, Barry Song
In-Reply-To: <CAGsJ_4wP_Zy9LZxZA-5KG13S3sQOak4LJK7rOhs3M=ETNJppNw@mail.gmail.com>
Barry Song <21cnbao@gmail.com> writes:
> On Wed, Mar 20, 2024 at 7:22 PM Huang, Ying <ying.huang@intel.com> wrote:
>>
>> Barry Song <21cnbao@gmail.com> writes:
>>
>> > On Wed, Mar 20, 2024 at 3:20 PM Huang, Ying <ying.huang@intel.com> wrote:
>> >>
>> >> Ryan Roberts <ryan.roberts@arm.com> writes:
>> >>
>> >> > On 19/03/2024 09:20, Huang, Ying wrote:
>> >> >> Ryan Roberts <ryan.roberts@arm.com> writes:
>> >> >>
>> >> >>>>>> I agree phones are not the only platform. But Rome wasn't built in a
>> >> >>>>>> day. I can only get
>> >> >>>>>> started on a hardware which I can easily reach and have enough hardware/test
>> >> >>>>>> resources on it. So we may take the first step which can be applied on
>> >> >>>>>> a real product
>> >> >>>>>> and improve its performance, and step by step, we broaden it and make it
>> >> >>>>>> widely useful to various areas in which I can't reach :-)
>> >> >>>>>
>> >> >>>>> We must guarantee the normal swap path runs correctly and has no
>> >> >>>>> performance regression when developing SWP_SYNCHRONOUS_IO optimization.
>> >> >>>>> So we have to put some effort on the normal path test anyway.
>> >> >>>>>
>> >> >>>>>> so probably we can have a sysfs "enable" entry with default "n" or
>> >> >>>>>> have a maximum
>> >> >>>>>> swap-in order as Ryan's suggestion [1] at the beginning,
>> >> >>>>>>
>> >> >>>>>> "
>> >> >>>>>> So in the common case, swap-in will pull in the same size of folio as was
>> >> >>>>>> swapped-out. Is that definitely the right policy for all folio sizes? Certainly
>> >> >>>>>> it makes sense for "small" large folios (e.g. up to 64K IMHO). But I'm not sure
>> >> >>>>>> it makes sense for 2M THP; As the size increases the chances of actually needing
>> >> >>>>>> all of the folio reduces so chances are we are wasting IO. There are similar
>> >> >>>>>> arguments for CoW, where we currently copy 1 page per fault - it probably makes
>> >> >>>>>> sense to copy the whole folio up to a certain size.
>> >> >>>>>> "
>> >> >>>
>> >> >>> I thought about this a bit more. No clear conclusions, but hoped this might help
>> >> >>> the discussion around policy:
>> >> >>>
>> >> >>> The decision about the size of the THP is made at first fault, with some help
>> >> >>> from user space and in future we might make decisions to split based on
>> >> >>> munmap/mremap/etc hints. In an ideal world, the fact that we have had to swap
>> >> >>> the THP out at some point in its lifetime should not impact on its size. It's
>> >> >>> just being moved around in the system and the reason for our original decision
>> >> >>> should still hold.
>> >> >>>
>> >> >>> So from that PoV, it would be good to swap-in to the same size that was
>> >> >>> swapped-out.
>> >> >>
>> >> >> Sorry, I don't agree with this. It's better to swap-in and swap-out in
>> >> >> smallest size if the page is only accessed seldom to avoid to waste
>> >> >> memory.
>> >> >
>> >> > If we want to optimize only for memory consumption, I'm sure there are many
>> >> > things we would do differently. We need to find a balance between memory and
>> >> > performance. The benefits of folios are well documented and the kernel is
>> >> > heading in the direction of managing memory in variable-sized blocks. So I don't
>> >> > think it's as simple as saying we should always swap-in the smallest possible
>> >> > amount of memory.
>> >>
>> >> It's conditional, that is,
>> >>
>> >> "if the page is only accessed seldom"
>> >>
>> >> Then, the page swapped-in will be swapped-out soon and adjacent pages in
>> >> the same large folio will not be accessed during this period.
>> >>
>> >> So, I suggest to create an algorithm to decide swap-in order based on
>> >> swap-readahead information automatically. It can detect the situation
>> >> above via reduced swap readahead window size. And, if the page is
>> >> accessed for quite long time, and the adjacent pages in the same large
>> >> folio are accessed too, swap-readahead window will increase and large
>> >> swap-in order will be used.
>> >
>> > The original size of do_anonymous_page() should be honored, considering it
>> > embodies a decision influenced by not only sysfs settings and per-vma
>> > HUGEPAGE hints but also architectural characteristics, for example
>> > CONT-PTE.
>> >
>> > The model you're proposing may offer memory-saving benefits or reduce I/O,
>> > but it entirely disassociates the size of the swap in from the size prior to the
>> > swap out.
>>
>> Readahead isn't the only factor to determine folio order. For example,
>> we must respect "never" policy to allocate order-0 folio always.
>> There's no requirements to use swap-out order in swap-in too. Memory
>> allocation has different performance character of storage reading.
>
> Still quite unclear.
>
> If users have only enabled 64KiB (4-ORDER) large folios in sysfs, and the
> readahead algorithm requires 16KiB, what should be set as the large folio size?
> Setting it to 16KiB doesn't align with users' requirements, while
> setting it to 64KiB
> would be wasteful according to your criteria.
IIUC, enabling 64KB means you can use 64KB mTHP if appropriate, doesn't
mean that you must use 64KB mTHP. If so, we should use 16KB mTHP in
that situation.
>> > Moreover, there's no guarantee that the large folio generated by
>> > the readahead window is contiguous in the swap and can be added to the
>> > swap cache, as we are currently dealing with folio->swap instead of
>> > subpage->swap.
>>
>> Yes. We can optimize only when all conditions are satisfied. Just like
>> other optimization.
>>
>> > Incidentally, do_anonymous_page() serves as the initial location for allocating
>> > large folios. Given that memory conservation is a significant consideration in
>> > do_swap_page(), wouldn't it be even more crucial in do_anonymous_page()?
>>
>> Yes. We should consider that too. IIUC, that is why mTHP support is
>> off by default for now. After we find a way to solve the memory usage
>> issue. We may make default "on".
>
> It's challenging to establish a universal solution because various systems
> exhibit diverse hardware characteristics, and VMAs may require different
> alignments. The current sysfs and per-vma hints allow users the opportunity
> o customize settings according to their specific requirements.
IIUC, Linux kernel is trying to provide a reasonable default behavior in
all situations. We are trying to optimize default behavior in the first
place, only introduce customization if we fail to do that. I don't
think that it's a good idea to introduce too much customization if we
haven't tried to optimize the default behavior.
>>
>> > A large folio, by its nature, represents a high-quality resource that has the
>> > potential to leverage hardware characteristics for the benefit of the
>> > entire system.
>>
>> But not at the cost of memory wastage.
>>
>> > Conversely, I don't believe that a randomly determined size dictated by the
>> > readahead window possesses the same advantageous qualities.
>>
>> There's a readahead algorithm which is not pure random.
>>
>> > SWP_SYNCHRONOUS_IO devices are not reliant on readahead whatsoever,
>> > their needs should also be respected.
>>
>> I understand that there are special requirements for SWP_SYNCHRONOUS_IO
>> devices. I just suggest to work on general code before specific
>> optimization.
>
> I disagree with your definition of "special" and "general". According
> to your logic,
> non-SWP_SYNCHRONOUS_IO devices could also be classified as "special".
SWP_SYNCHRONOUS_IO devices also use general code path. They just use
some special optimization in some special situation (__swap_count(entry)
== 1). Optimization in general path benefits everyone.
> Furthermore, the number of systems running SWP_SYNCHRONOUS_IO is
> significantly greater than those running non-SWP_SYNCHRONOUS_IO,
> contradicting your assertion.
>
> SWP_SYNCHRONOUS_IO devices have a minor chance of being involved
> in readahead.
Then it loses an opportunity to determine the appropriate folio order.
We can consider how to balance between the overhead and benefit of
readahead. IIUC, compared with original SWP_SYNCHRONOUS_IO swap-in,
mTHP is a kind of readahead too.
BTW, because we have added more and more swap cache related operations
(swapcache_prepare(), clear_shadow_from_swap_cache(), swapcache_clear(),
etc.) in SWP_SYNCHRONOUS_IO code path, I suspect whether the benefit of
SWP_SYNCHRONOUS_IO is still large enough. We may need to re-evaluate
it.
> However, in OPPO's code, which hasn't been sent in the
> LKML yet, we use the exact same size as do_anonymous_page for readahead.
> Without a clear description of how you want the new readahead
> algorithm to balance memory waste and users' hints from sysfs and
> per-vma flags, it appears to be an ambiguous area to address.
>
> Please provide a clear description of how you would like the new readahead
> algorithm to function. I believe this clarity will facilitate others
> in attempting to
> implement it.
For example, if __swapin_nr_pages() > 4, we can try to allocate an
order-2 mTHP if other conditions are satisfied.
>>
>> >> > You also said we should swap *out* in smallest size possible. Have I
>> >> > misunderstood you? I thought the case for swapping-out a whole folio without
>> >> > splitting was well established and non-controversial?
>> >>
>> >> That is conditional too.
>> >>
>> >> >>
>> >> >>> But we only kind-of keep that information around, via the swap
>> >> >>> entry contiguity and alignment. With that scheme it is possible that multiple
>> >> >>> virtually adjacent but not physically contiguous folios get swapped-out to
>> >> >>> adjacent swap slot ranges and then they would be swapped-in to a single, larger
>> >> >>> folio. This is not ideal, and I think it would be valuable to try to maintain
>> >> >>> the original folio size information with the swap slot. One way to do this would
>> >> >>> be to store the original order for which the cluster was allocated in the
>> >> >>> cluster. Then we at least know that a given swap slot is either for a folio of
>> >> >>> that order or an order-0 folio (due to cluster exhaustion/scanning). Can we
>> >> >>> steal a bit from swap_map to determine which case it is? Or are there better
>> >> >>> approaches?
>> >> >>
>> >> >> [snip]
>>
--
Best Regards,
Huang, Ying
_______________________________________________
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 PATCH v3 5/5] mm: support large folios swapin as a whole
From: Barry Song @ 2024-03-21 5:12 UTC (permalink / raw)
To: Huang, Ying
Cc: Ryan Roberts, Matthew Wilcox, akpm, linux-mm, chengming.zhou,
chrisl, david, hannes, kasong, linux-arm-kernel, linux-kernel,
mhocko, nphamcs, shy828301, steven.price, surenb, wangkefeng.wang,
xiang, yosryahmed, yuzhao, Chuanhua Han, Barry Song
In-Reply-To: <87a5msrz5e.fsf@yhuang6-desk2.ccr.corp.intel.com>
On Thu, Mar 21, 2024 at 5:25 PM Huang, Ying <ying.huang@intel.com> wrote:
>
> Barry Song <21cnbao@gmail.com> writes:
>
> > On Wed, Mar 20, 2024 at 7:22 PM Huang, Ying <ying.huang@intel.com> wrote:
> >>
> >> Barry Song <21cnbao@gmail.com> writes:
> >>
> >> > On Wed, Mar 20, 2024 at 3:20 PM Huang, Ying <ying.huang@intel.com> wrote:
> >> >>
> >> >> Ryan Roberts <ryan.roberts@arm.com> writes:
> >> >>
> >> >> > On 19/03/2024 09:20, Huang, Ying wrote:
> >> >> >> Ryan Roberts <ryan.roberts@arm.com> writes:
> >> >> >>
> >> >> >>>>>> I agree phones are not the only platform. But Rome wasn't built in a
> >> >> >>>>>> day. I can only get
> >> >> >>>>>> started on a hardware which I can easily reach and have enough hardware/test
> >> >> >>>>>> resources on it. So we may take the first step which can be applied on
> >> >> >>>>>> a real product
> >> >> >>>>>> and improve its performance, and step by step, we broaden it and make it
> >> >> >>>>>> widely useful to various areas in which I can't reach :-)
> >> >> >>>>>
> >> >> >>>>> We must guarantee the normal swap path runs correctly and has no
> >> >> >>>>> performance regression when developing SWP_SYNCHRONOUS_IO optimization.
> >> >> >>>>> So we have to put some effort on the normal path test anyway.
> >> >> >>>>>
> >> >> >>>>>> so probably we can have a sysfs "enable" entry with default "n" or
> >> >> >>>>>> have a maximum
> >> >> >>>>>> swap-in order as Ryan's suggestion [1] at the beginning,
> >> >> >>>>>>
> >> >> >>>>>> "
> >> >> >>>>>> So in the common case, swap-in will pull in the same size of folio as was
> >> >> >>>>>> swapped-out. Is that definitely the right policy for all folio sizes? Certainly
> >> >> >>>>>> it makes sense for "small" large folios (e.g. up to 64K IMHO). But I'm not sure
> >> >> >>>>>> it makes sense for 2M THP; As the size increases the chances of actually needing
> >> >> >>>>>> all of the folio reduces so chances are we are wasting IO. There are similar
> >> >> >>>>>> arguments for CoW, where we currently copy 1 page per fault - it probably makes
> >> >> >>>>>> sense to copy the whole folio up to a certain size.
> >> >> >>>>>> "
> >> >> >>>
> >> >> >>> I thought about this a bit more. No clear conclusions, but hoped this might help
> >> >> >>> the discussion around policy:
> >> >> >>>
> >> >> >>> The decision about the size of the THP is made at first fault, with some help
> >> >> >>> from user space and in future we might make decisions to split based on
> >> >> >>> munmap/mremap/etc hints. In an ideal world, the fact that we have had to swap
> >> >> >>> the THP out at some point in its lifetime should not impact on its size. It's
> >> >> >>> just being moved around in the system and the reason for our original decision
> >> >> >>> should still hold.
> >> >> >>>
> >> >> >>> So from that PoV, it would be good to swap-in to the same size that was
> >> >> >>> swapped-out.
> >> >> >>
> >> >> >> Sorry, I don't agree with this. It's better to swap-in and swap-out in
> >> >> >> smallest size if the page is only accessed seldom to avoid to waste
> >> >> >> memory.
> >> >> >
> >> >> > If we want to optimize only for memory consumption, I'm sure there are many
> >> >> > things we would do differently. We need to find a balance between memory and
> >> >> > performance. The benefits of folios are well documented and the kernel is
> >> >> > heading in the direction of managing memory in variable-sized blocks. So I don't
> >> >> > think it's as simple as saying we should always swap-in the smallest possible
> >> >> > amount of memory.
> >> >>
> >> >> It's conditional, that is,
> >> >>
> >> >> "if the page is only accessed seldom"
> >> >>
> >> >> Then, the page swapped-in will be swapped-out soon and adjacent pages in
> >> >> the same large folio will not be accessed during this period.
> >> >>
> >> >> So, I suggest to create an algorithm to decide swap-in order based on
> >> >> swap-readahead information automatically. It can detect the situation
> >> >> above via reduced swap readahead window size. And, if the page is
> >> >> accessed for quite long time, and the adjacent pages in the same large
> >> >> folio are accessed too, swap-readahead window will increase and large
> >> >> swap-in order will be used.
> >> >
> >> > The original size of do_anonymous_page() should be honored, considering it
> >> > embodies a decision influenced by not only sysfs settings and per-vma
> >> > HUGEPAGE hints but also architectural characteristics, for example
> >> > CONT-PTE.
> >> >
> >> > The model you're proposing may offer memory-saving benefits or reduce I/O,
> >> > but it entirely disassociates the size of the swap in from the size prior to the
> >> > swap out.
> >>
> >> Readahead isn't the only factor to determine folio order. For example,
> >> we must respect "never" policy to allocate order-0 folio always.
> >> There's no requirements to use swap-out order in swap-in too. Memory
> >> allocation has different performance character of storage reading.
> >
> > Still quite unclear.
> >
> > If users have only enabled 64KiB (4-ORDER) large folios in sysfs, and the
> > readahead algorithm requires 16KiB, what should be set as the large folio size?
> > Setting it to 16KiB doesn't align with users' requirements, while
> > setting it to 64KiB
> > would be wasteful according to your criteria.
>
> IIUC, enabling 64KB means you can use 64KB mTHP if appropriate, doesn't
> mean that you must use 64KB mTHP. If so, we should use 16KB mTHP in
> that situation.
A specific large folio size inherently denotes a high-quality
resource. For example,
a 64KiB folio necessitates only one TLB on ARM64, just as a 2MB large folio
accommodates only one TLB. I am skeptical whether a size determined by
readahead offers any tangible advantages over simply having small folios.
>
> >> > Moreover, there's no guarantee that the large folio generated by
> >> > the readahead window is contiguous in the swap and can be added to the
> >> > swap cache, as we are currently dealing with folio->swap instead of
> >> > subpage->swap.
> >>
> >> Yes. We can optimize only when all conditions are satisfied. Just like
> >> other optimization.
> >>
> >> > Incidentally, do_anonymous_page() serves as the initial location for allocating
> >> > large folios. Given that memory conservation is a significant consideration in
> >> > do_swap_page(), wouldn't it be even more crucial in do_anonymous_page()?
> >>
> >> Yes. We should consider that too. IIUC, that is why mTHP support is
> >> off by default for now. After we find a way to solve the memory usage
> >> issue. We may make default "on".
> >
> > It's challenging to establish a universal solution because various systems
> > exhibit diverse hardware characteristics, and VMAs may require different
> > alignments. The current sysfs and per-vma hints allow users the opportunity
> > o customize settings according to their specific requirements.
>
> IIUC, Linux kernel is trying to provide a reasonable default behavior in
> all situations. We are trying to optimize default behavior in the first
> place, only introduce customization if we fail to do that. I don't
> think that it's a good idea to introduce too much customization if we
> haven't tried to optimize the default behavior.
I've never been opposed to the readahead case, but I feel it's a second step.
My point is to begin with the simplest and most practical approaches
that can generate
genuine value and contribution. The SWP_SYNCHRONOUS_IO case has been
implemented on millions of OPPO's phones and has demonstrated product success.
>
> >>
> >> > A large folio, by its nature, represents a high-quality resource that has the
> >> > potential to leverage hardware characteristics for the benefit of the
> >> > entire system.
> >>
> >> But not at the cost of memory wastage.
> >>
> >> > Conversely, I don't believe that a randomly determined size dictated by the
> >> > readahead window possesses the same advantageous qualities.
> >>
> >> There's a readahead algorithm which is not pure random.
> >>
> >> > SWP_SYNCHRONOUS_IO devices are not reliant on readahead whatsoever,
> >> > their needs should also be respected.
> >>
> >> I understand that there are special requirements for SWP_SYNCHRONOUS_IO
> >> devices. I just suggest to work on general code before specific
> >> optimization.
> >
> > I disagree with your definition of "special" and "general". According
> > to your logic,
> > non-SWP_SYNCHRONOUS_IO devices could also be classified as "special".
>
> SWP_SYNCHRONOUS_IO devices also use general code path. They just use
> some special optimization in some special situation (__swap_count(entry)
> == 1). Optimization in general path benefits everyone.
>
> > Furthermore, the number of systems running SWP_SYNCHRONOUS_IO is
> > significantly greater than those running non-SWP_SYNCHRONOUS_IO,
> > contradicting your assertion.
> >
> > SWP_SYNCHRONOUS_IO devices have a minor chance of being involved
> > in readahead.
>
> Then it loses an opportunity to determine the appropriate folio order.
> We can consider how to balance between the overhead and benefit of
> readahead. IIUC, compared with original SWP_SYNCHRONOUS_IO swap-in,
> mTHP is a kind of readahead too.
>
> BTW, because we have added more and more swap cache related operations
> (swapcache_prepare(), clear_shadow_from_swap_cache(), swapcache_clear(),
> etc.) in SWP_SYNCHRONOUS_IO code path, I suspect whether the benefit of
> SWP_SYNCHRONOUS_IO is still large enough. We may need to re-evaluate
> it.
Obviously SWP_SYNCHRONOUS_IO is still quite valuable as Kairui has the data
in his commit 13ddaf26be324a ("mm/swap: fix race when skipping swapcache")
"Performance overhead is minimal, microbenchmark swapin 10G from 32G zram:
Before: 10934698 us
After: 11157121 us
Cached: 13155355 us (Dropping SWP_SYNCHRONOUS_IO flag) "
>
> > However, in OPPO's code, which hasn't been sent in the
> > LKML yet, we use the exact same size as do_anonymous_page for readahead.
> > Without a clear description of how you want the new readahead
> > algorithm to balance memory waste and users' hints from sysfs and
> > per-vma flags, it appears to be an ambiguous area to address.
> >
> > Please provide a clear description of how you would like the new readahead
> > algorithm to function. I believe this clarity will facilitate others
> > in attempting to
> > implement it.
>
> For example, if __swapin_nr_pages() > 4, we can try to allocate an
> order-2 mTHP if other conditions are satisfied.
There is no evidence suggesting that an order-2 or any other orders
determined by readahead are superior to having four small folios.
>
> >>
> >> >> > You also said we should swap *out* in smallest size possible. Have I
> >> >> > misunderstood you? I thought the case for swapping-out a whole folio without
> >> >> > splitting was well established and non-controversial?
> >> >>
> >> >> That is conditional too.
> >> >>
> >> >> >>
> >> >> >>> But we only kind-of keep that information around, via the swap
> >> >> >>> entry contiguity and alignment. With that scheme it is possible that multiple
> >> >> >>> virtually adjacent but not physically contiguous folios get swapped-out to
> >> >> >>> adjacent swap slot ranges and then they would be swapped-in to a single, larger
> >> >> >>> folio. This is not ideal, and I think it would be valuable to try to maintain
> >> >> >>> the original folio size information with the swap slot. One way to do this would
> >> >> >>> be to store the original order for which the cluster was allocated in the
> >> >> >>> cluster. Then we at least know that a given swap slot is either for a folio of
> >> >> >>> that order or an order-0 folio (due to cluster exhaustion/scanning). Can we
> >> >> >>> steal a bit from swap_map to determine which case it is? Or are there better
> >> >> >>> approaches?
> >> >> >>
> >> >> >> [snip]
> >>
>
> --
> Best Regards,
> Huang, Ying
Thanks
Barry
_______________________________________________
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/4] dt-bindings: display/xlnx/zynqmp-dpsub: Add audio DMAs
From: Tomi Valkeinen @ 2024-03-21 5:52 UTC (permalink / raw)
To: Rob Herring
Cc: Lars-Peter Clausen, Jaroslav Kysela, Takashi Iwai, Liam Girdwood,
Mark Brown, Laurent Pinchart, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Daniel Vetter,
Krzysztof Kozlowski, Conor Dooley, Michal Simek, linux-sound,
linux-kernel, dri-devel, devicetree, linux-arm-kernel,
Vishal Sagar, Anatoliy Klymenko, Péter Ujfalusi
In-Reply-To: <20240320153725.GA1771730-robh@kernel.org>
On 20/03/2024 17:37, Rob Herring wrote:
> On Tue, Mar 19, 2024 at 10:22:37AM +0200, Tomi Valkeinen wrote:
>> The DP subsystem for ZynqMP support audio via two channels, and the DP
>> DMA has dma-engines for those channels. For some reason the DT binding
>> has not specified those channels, even if the picture included in
>> xlnx,zynqmp-dpsub.yaml shows "2 x aud" DMAs.
>
> New required entries is an ABI change. This message kind of indicates it
> was a mistake, but should be a lot more explicit. Are things broken
> without the entries? Need 'Fixes'?
I'll improve the desc for the next version.
So, yes, it's an ABI change, and as far as I can guess (I can't figure
out any other reason), the audio DMAs were left out by mistake or
misunderstanding. The Linux driver has not supported audio, so this has
not been an issue and nothing is broken.
Now that this series adds the audio support, I had to add the audio
DMAs. I considered making the DMAs optional in the DT, but that doesn't
sound right, even if that would keep the ABI compatibility (wouldn't
it?). The driver I add in this series does consider the audio DMAs as
optional, though. If they're not present, the driver will continue
without audio support.
So, strictly speaking I think this is a fix to the original commit that
adds the DT node, but as the driver using the audio DMAs comes in only
now, I think there's no need for the 'Fixes' and backporting.
I'm happy to change the approach if you think some other way is better.
Tomi
>>
>> Add the two audio DMAs to the binding.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>> .../devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml
>> index 554f9d5809d4..6b754d4f260e 100644
>> --- a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml
>> +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.yaml
>> @@ -100,12 +100,16 @@ properties:
>> - description: Video layer, plane 1 (U/V or U)
>> - description: Video layer, plane 2 (V)
>> - description: Graphics layer
>> + - description: Audio channel 0
>> + - description: Audio channel 1
>> dma-names:
>> items:
>> - const: vid0
>> - const: vid1
>> - const: vid2
>> - const: gfx0
>> + - const: aud0
>> + - const: aud1
>>
>> phys:
>> description: PHYs for the DP data lanes
>> @@ -194,11 +198,13 @@ examples:
>> power-domains = <&pd_dp>;
>> resets = <&reset ZYNQMP_RESET_DP>;
>>
>> - dma-names = "vid0", "vid1", "vid2", "gfx0";
>> + dma-names = "vid0", "vid1", "vid2", "gfx0", "aud0", "aud1";
>> dmas = <&xlnx_dpdma 0>,
>> <&xlnx_dpdma 1>,
>> <&xlnx_dpdma 2>,
>> - <&xlnx_dpdma 3>;
>> + <&xlnx_dpdma 3>,
>> + <&xlnx_dpdma 4>,
>> + <&xlnx_dpdma 5>;
>>
>> phys = <&psgtr 1 PHY_TYPE_DP 0 3>,
>> <&psgtr 0 PHY_TYPE_DP 1 3>;
>>
>> --
>> 2.34.1
>>
_______________________________________________
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 v4 1/5] dt-bindings: misc: Add mikrobus-connector
From: Vaishnav Achath @ 2024-03-21 6:30 UTC (permalink / raw)
To: Conor Dooley
Cc: Andrew Lunn, Ayush Singh, Michael Walle, open list, jkridner,
robertcnelson, lorforlinux, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Nishanth Menon, Vignesh Raghavendra, Tero Kristo,
Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
Mark Brown, Johan Hovold, Alex Elder,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM/TEXAS INSTRUMENTS K3 ARCHITECTURE,
open list:SPI SUBSYSTEM, moderated list:GREYBUS SUBSYSTEM,
Vaishnav M A
In-Reply-To: <20240319-unmoral-map-3ab9a467b637@spud>
Hi Conor,
On 19/03/24 23:49, Conor Dooley wrote:
> On Tue, Mar 19, 2024 at 11:05:37PM +0530, Vaishnav Achath wrote:
>> Hi Andrew,
>>
>> On 19/03/24 17:55, Andrew Lunn wrote:
>>>> The device tree defines the SPI controller associated with mikroBUS SPI
>>>> pins. The driver on match queries and takes a reference to the SPI
>>>> controller but does nothing with it. Once a mikroBUS add-on board is
>>>> detected (by passing manifest using sysfs or reading from 1-wire EEPROM),
>>>> the driver parses the manifest, and if it detects an SPI device in manifest,
>>>> it registers SPI device along with setting properties such as `chip_select`,
>>>> `max_speed_hz`, `mode`, etc.,
>>>
>>> How complex can the description of the hardware be in the manifest?
>>>
>>> Could i describe an SPI to I2C converter? And then a few temperature
>>> sensors, a fan controller, and a GPIO controller on that I2C bus? And
>>> the GPIO controller is then used for LEDs and a push button? DT
>>> overlays could describe that. Can the manifest?
>>
>> No, it cannot describe such complex hardware, it can only describe simple
>> devices (sensors/displays .etc) on a standard mikroBUS add-on board, we did
>> a analysis on what mikroBUS add-on boards have driver support in Linux and
>> then noticed that most devices does not need this kind of complex
>> description to work:
>> https://elinux.org/MikroEClicks_with_Linux_Support
>
> What happens to the devices that fall outside of the "do not need a
> complex description" category? Do you expect that those would be
> described by a dt overlay?
>
Yes, those would need a device tree overlay, but most mikroBUS add-on
boards does not need this, almost all the boards need the standard bus
properties (SPI/I2C properties), IRQ, named-gpios, named properties,
regulators, clocks and the current implementation supports this.
Looking at the example Andrew provided above (SPI-I2C converter with
sensors .etc on the I2C bus and GPIO controller) - usually you will not
find such a mikroBUS add-on board, because if there are I2C devices they
would directly be on the mikroBUS I2C bus rather than on the converter,
now someone can do this in their custom solution but then it is no
different than an I2C adapter on USB/PCIe, does the standard discovery
mechanism on those buses help instantiate devices on the I2C? my
understanding is NO and you will need to write a custom device tree
overlay for the same and same will be the case here.
>> The greybus manifest already is being used in the greybus susbystem for
>> describing an interface and there are already greybus controllers (SPI/I2C
>> .etc) being created according to the manifest contents, all this driver does
>> is to extend that format to be able to instantiate devices on these buses.
>> The primary goals for introducing the driver for mikroBUS add-on boards are:
>>
>> 1) A way to isolate platform specific information from add-on board specific
>> information - so that each permutation of connecting the add-on board on
>> different ports on different board does not require a new overlay.
>> 2) A way to instantiate add-on boards on greybus created virtual mikroBUS
>> ports.
>> 3) Both 1 and 2 should use the same add-on board description format.
>>
>> Standard device tree overlays did not help to achieve this and that is why
>> the standard interface discovery mechanism in greybus, the manifest was
>> extended even though it is not the most optimal way to describe hardware.
>>
>> The greybus manifest extensions were made with the following things in mind
>> and three new descriptor were introduced:
>> 1) mikrobus descriptor - pinmux/port state
>> 2) device descriptor - contains information which is a superset of struct
>> i2c_board_info , struct spi_board_info .etc
>> 3) property descriptor - to describe named properties of the types defined
>> under https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/property.h#n22
>>
>> With these we were able to test around 150 add-on boards with corresponding
>> drivers in Linux :
>> https://github.com/MikroElektronika/click_id/tree/main/manifests
>>
>> The mechanism is not as robust a device tree and should not be compared, the
>
> Why not? You're suggesting this as a method for describing devices and you
> seem to have extended the manifest to support more complex properties, why
> shouldn't someone question make that comparison?
>
Agreed, but the comparison need to limited to the expansion interface
(mikroBUS) under discussion as the idea is not to create an alternate
interface for describing generic devices, the class of add-on boards
that can fit in the mikroBUS add-on board form factor and on the buses
exposed by mikroBUS requires only simple descriptions - namely standard
bus properties (SPI/I2C properties), IRQ, named-gpios, named properties,
regulators, clocks and the extensions to manifest were made for those
properties only. Also the extensions were done to support the properties
under unified device property interface under include/linux/property.h
Thanks and Regards,
Vaishnav
>> intent was not to create a new hardware description format, but extend the
>> existing greybus manifest format to be able to instantiate devices on the
>> greybus SPI/I2C/GPIO/ (mikroBUS)
>
_______________________________________________
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] spi: spi-mt65xx: Rename a variable in interrupt handler
From: Fei Shao @ 2024-03-21 6:41 UTC (permalink / raw)
To: Mark Brown, AngeloGioacchino Del Regno
Cc: Fei Shao, Matthias Brugger, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-spi
In-Reply-To: <20240321064313.1385316-1-fshao@chromium.org>
All the spi_transfer variables in this file use the name "xfer" except
the one in mtk_spi_interrupt(). Align the naming for consistency and
easier searching.
While at it, reformat one memcpy() usage since the coding style allows
100 column lines today.
This commit has no functional change.
Signed-off-by: Fei Shao <fshao@chromium.org>
---
drivers/spi/spi-mt65xx.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index 86ea822c942b..aaa0006a02a3 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -748,7 +748,7 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
u32 cmd, reg_val, cnt, remainder, len;
struct spi_controller *host = dev_id;
struct mtk_spi *mdata = spi_controller_get_devdata(host);
- struct spi_transfer *trans = mdata->cur_transfer;
+ struct spi_transfer *xfer = mdata->cur_transfer;
reg_val = readl(mdata->base + SPI_STATUS0_REG);
if (reg_val & MTK_SPI_PAUSE_INT_STATUS)
@@ -762,42 +762,40 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
- if (!host->can_dma(host, NULL, trans)) {
- if (trans->rx_buf) {
+ if (!host->can_dma(host, NULL, xfer)) {
+ if (xfer->rx_buf) {
cnt = mdata->xfer_len / 4;
ioread32_rep(mdata->base + SPI_RX_DATA_REG,
- trans->rx_buf + mdata->num_xfered, cnt);
+ xfer->rx_buf + mdata->num_xfered, cnt);
remainder = mdata->xfer_len % 4;
if (remainder > 0) {
reg_val = readl(mdata->base + SPI_RX_DATA_REG);
- memcpy(trans->rx_buf +
- mdata->num_xfered +
- (cnt * 4),
+ memcpy(xfer->rx_buf + (cnt * 4) + mdata->num_xfered,
®_val,
remainder);
}
}
mdata->num_xfered += mdata->xfer_len;
- if (mdata->num_xfered == trans->len) {
+ if (mdata->num_xfered == xfer->len) {
spi_finalize_current_transfer(host);
return IRQ_HANDLED;
}
- len = trans->len - mdata->num_xfered;
+ len = xfer->len - mdata->num_xfered;
mdata->xfer_len = min(MTK_SPI_MAX_FIFO_SIZE, len);
mtk_spi_setup_packet(host);
- if (trans->tx_buf) {
+ if (xfer->tx_buf) {
cnt = mdata->xfer_len / 4;
iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
- trans->tx_buf + mdata->num_xfered, cnt);
+ xfer->tx_buf + mdata->num_xfered, cnt);
remainder = mdata->xfer_len % 4;
if (remainder > 0) {
reg_val = 0;
memcpy(®_val,
- trans->tx_buf + (cnt * 4) + mdata->num_xfered,
+ xfer->tx_buf + (cnt * 4) + mdata->num_xfered,
remainder);
writel(reg_val, mdata->base + SPI_TX_DATA_REG);
}
@@ -808,21 +806,21 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
}
if (mdata->tx_sgl)
- trans->tx_dma += mdata->xfer_len;
+ xfer->tx_dma += mdata->xfer_len;
if (mdata->rx_sgl)
- trans->rx_dma += mdata->xfer_len;
+ xfer->rx_dma += mdata->xfer_len;
if (mdata->tx_sgl && (mdata->tx_sgl_len == 0)) {
mdata->tx_sgl = sg_next(mdata->tx_sgl);
if (mdata->tx_sgl) {
- trans->tx_dma = sg_dma_address(mdata->tx_sgl);
+ xfer->tx_dma = sg_dma_address(mdata->tx_sgl);
mdata->tx_sgl_len = sg_dma_len(mdata->tx_sgl);
}
}
if (mdata->rx_sgl && (mdata->rx_sgl_len == 0)) {
mdata->rx_sgl = sg_next(mdata->rx_sgl);
if (mdata->rx_sgl) {
- trans->rx_dma = sg_dma_address(mdata->rx_sgl);
+ xfer->rx_dma = sg_dma_address(mdata->rx_sgl);
mdata->rx_sgl_len = sg_dma_len(mdata->rx_sgl);
}
}
@@ -840,7 +838,7 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
mtk_spi_update_mdata_len(host);
mtk_spi_setup_packet(host);
- mtk_spi_setup_dma_addr(host, trans);
+ mtk_spi_setup_dma_addr(host, xfer);
mtk_spi_enable_transfer(host);
return IRQ_HANDLED;
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
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: [PATCH] drm/atmel-hlcdc: Release CRTC commit when destroying plane state
From: Ludovic.Desroches @ 2024-03-21 6:45 UTC (permalink / raw)
To: pl.dourneau
Cc: Manikandan.M, alexandre.belloni, linux-kernel, daniel, bbrezillon,
sam, maarten.lankhorst, mripard, claudiu.beznea, n.georges,
dri-devel, tzimmermann, b.alcaina, airlied, a.lahache,
linux-arm-kernel
In-Reply-To: <20240315122527.344534-1-pl.dourneau@klervi.com>
On 3/15/24 13:25, Pierre-Louis Dourneau wrote:
> [You don't often get email from pl.dourneau@klervi.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 3/12/24, Pierre-Louis Dourneau <pl.dourneau@klervi.com> wrote:
>> On 3/8/24, Ludovic.Desroches@microchip.com <Ludovic.Desroches@microchip.com> wrote:
>>> This patch fixes the memory leak but introduces a crash on my side when
>>> exiting a graphics app using the Microchip graphics library.
>>
>> We've tried to reproduce your crash with 6.1.22-linux4microchip-2023.04,
>> to no avail. We'll try to upgrade to 6.1.55-linux4microchip-2023.10 (your
>> version) and test again.
>
> I was able to test a few more recent kernel versions[0] with the patch
> applied. None yielded any crash, be it running Microchip's EGT samples[1]
> or libdrm's modetest. Although, what I did manage to reproduce was a
> refcount underflow similar to the one you had:
>
> # modetest -M atmel-hlcdc -s 32:#0 -P 33@47:800x400@XR24 -a
> setting mode 1024x600-65.48Hz on connectors 32, crtc 47
> testing 800x400@XR24 on plane 33, crtc 47
> [ 75.736699] ------------[ cut here ]------------
> [ 75.741353] WARNING: CPU: 0 PID: 200 at lib/refcount.c:28 refcount_warn_saturate+0xf0/0x15c
> [ 75.749731] refcount_t: underflow; use-after-free.
> [ 75.754574] CPU: 0 PID: 200 Comm: modetest Not tainted 6.1.55-linux4microchip-2023.10 #4
> [ 75.762915] Hardware name: Microchip SAM9X60
> [ 75.767198] unwind_backtrace from show_stack+0x10/0x18
> [ 75.772423] show_stack from dump_stack_lvl+0x28/0x34
> [ 75.777479] dump_stack_lvl from __warn+0x8c/0xc0
> [ 75.782187] __warn from warn_slowpath_fmt+0x74/0xa8
> [ 75.787158] warn_slowpath_fmt from refcount_warn_saturate+0xf0/0x15c
> [ 75.793611] refcount_warn_saturate from __drm_atomic_helper_plane_destroy_state+0xd0/0xd4
> [ 75.801894] __drm_atomic_helper_plane_destroy_state from atmel_hlcdc_plane_atomic_destroy_state+0x38/0x48
> [ 75.811573] atmel_hlcdc_plane_atomic_destroy_state from drm_atomic_state_default_clear+0x1c4/0x2fc
> [ 75.820642] drm_atomic_state_default_clear from __drm_atomic_state_free+0x7c/0xb0
> [ 75.828228] __drm_atomic_state_free from drm_mode_atomic_ioctl+0x868/0xb88
> [ 75.835204] drm_mode_atomic_ioctl from drm_ioctl+0x200/0x3c4
> [ 75.840960] drm_ioctl from sys_ioctl+0x240/0xb48
> [ 75.845669] sys_ioctl from ret_fast_syscall+0x0/0x44
> [ 75.850725] Exception stack(0xc8c91fa8 to 0xc8c91ff0)
> [ 75.855794] 1fa0: 004b0ec0 00000003 00000003 c03864bc becd7bc8 becd7b98
> [ 75.863992] 1fc0: 004b0ec0 00000003 becd7bc8 00000036 00000003 00000003 b6f22f80 00000400
> [ 75.872183] 1fe0: b6f21e74 becd7a68 b6f07494 b6f61cc8
> [ 75.877289] ---[ end trace 0000000000000000 ]---
> Atomic Commit failed [1]
>
> Same but without using the atomic uAPI (`-a` option removed):
>
> # modetest -M atmel-hlcdc -s 32:#0 -P 33@47:800x400@XR24
> setting mode 1024x600-65.48Hz on connectors 32, crtc 47
> testing 800x400@XR24 overlay plane 33
> failed to enable plane: Invalid argument
>
> [ 98.542958] ------------[ cut here ]------------
> [ 98.547547] WARNING: CPU: 0 PID: 28 at lib/refcount.c:28 refcount_warn_saturate+0xf0/0x15c
> [ 98.555902] refcount_t: underflow; use-after-free.
> [ 98.560698] CPU: 0 PID: 28 Comm: kworker/0:7 Tainted: G W 6.1.55-linux4microchip-2023.10 #6
> [ 98.570695] Hardware name: Microchip SAM9X60
> [ 98.574972] Workqueue: events drm_mode_rmfb_work_fn
> [ 98.579859] unwind_backtrace from show_stack+0x10/0x18
> [ 98.587615] show_stack from dump_stack_lvl+0x28/0x34
> [ 98.595201] dump_stack_lvl from __warn+0x8c/0xc0
> [ 98.602438] __warn from warn_slowpath_fmt+0x74/0xa8
> [ 98.609937] warn_slowpath_fmt from refcount_warn_saturate+0xf0/0x15c
> [ 98.618919] refcount_warn_saturate from __drm_atomic_helper_plane_destroy_state+0xd0/0xd4
> [ 98.629740] __drm_atomic_helper_plane_destroy_state from atmel_hlcdc_plane_atomic_destroy_state+0x38/0x48
> [ 98.641947] atmel_hlcdc_plane_atomic_destroy_state from drm_atomic_state_default_clear+0x1c4/0x2fc
> [ 98.653545] drm_atomic_state_default_clear from __drm_atomic_state_free+0x7c/0xb0
> [ 98.663660] __drm_atomic_state_free from drm_framebuffer_remove+0x48c/0x540
> [ 98.673252] drm_framebuffer_remove from drm_mode_rmfb_work_fn+0x68/0x84
> [ 98.682495] drm_mode_rmfb_work_fn from process_one_work+0x1b4/0x3f4
> [ 98.691390] process_one_work from worker_thread+0x214/0x4e8
> [ 98.699587] worker_thread from kthread+0xb4/0xd8
> [ 98.706824] kthread from ret_from_fork+0x14/0x28
> [ 98.714060] Exception stack(0xc88adfb0 to 0xc88adff8)
> [ 98.719125] dfa0: 00000000 00000000 00000000 00000000
> [ 98.727327] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> [ 98.735520] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000
> [ 98.742219] ---[ end trace 0000000000000000 ]---
>
> The first one is not deterministic, you have to try a few times to trigger
> it. The second one is a hit every time.
>
> Same commands on a kernel without the patch don't report any underflow.
> Note the commit in the first command also fails on a kernel without the
> patch, which I guess is expected as plane 33 is the primary plane and I'm
> trying set dimensions that do not match the size of the display. The commit
> succeeds when invoking with the correct dimensions, but then I can't make
> it produce an underflow. Same with the second command.
>
> It seems to only trigger once per boot. Running the commands again does not
> yield another underflow.
>
> Looking at the disassembly, this is an underflow of the drm_crtc_commit's
> refcount this time. In the warning you had, it was on a framebuffer object.
>
> Anyway, I'll go back to the drawing board, study more closely the resource
> release part of the driver. Thanks for having brought up the issues with
> the patch.
Hello,
Sorry for the late reply, I was off previous weeks. Late me know if you
need more information or testing on my side.
From the top of my head, the crash was systematic on my side and I
think I get it with any EGT application.
I use an EGT app to check if the memory leak is still here. It
continuously change the text of a button causing many calls of
drm_atomic_helper_setup_commit. After a while, we can see that the
virtual memory is constant while the system memory decreases. At least,
your patch sounds to fix this memory leak.
Regards,
Ludovic
>
> [0]: Namely, linux4microchip-2023.10 (6.1.55), v6.8, and drm-next-2023-03-13
> [1]: https://github.com/linux4sam/egt/tree/master/examples
>
> Pierre-Louis
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 0/2] Fixes NULL pointer access in spi-mt65xx.c
From: Fei Shao @ 2024-03-21 6:41 UTC (permalink / raw)
To: Mark Brown, AngeloGioacchino Del Regno
Cc: Fei Shao, Daniel Kurtz, Matthias Brugger, linux-arm-kernel,
linux-kernel, linux-mediatek, linux-spi
Hi,
This series contains two patches for spi-mt65xx.c, both focusing on its
interrupt handler mtk_spi_interrupt().
The first patch is to fix a NULL pointer access in the interrupt
handler, which is first found on a MT8186 Chromebook device when the
system tries to establish host communication with its embedded
controller.
The second one is a decorative clean-up when I'm working on the first
patch, which simply renames a variable to better follow the rest of the
code.
I put this after the first fix because I think that will make
maintainers and users slightly easier to only backport the fix if
needed.
Looking forward to any feedback, thank you.
Regards,
Fei
Fei Shao (2):
spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
spi: spi-mt65xx: Rename a variable in interrupt handler
drivers/spi/spi-mt65xx.c | 47 ++++++++++++++++++++--------------------
1 file changed, 23 insertions(+), 24 deletions(-)
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
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] spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
From: Fei Shao @ 2024-03-21 6:41 UTC (permalink / raw)
To: Mark Brown, AngeloGioacchino Del Regno
Cc: Fei Shao, Daniel Kurtz, Matthias Brugger, linux-arm-kernel,
linux-kernel, linux-mediatek, linux-spi
In-Reply-To: <20240321064313.1385316-1-fshao@chromium.org>
The TX buffer in spi_transfer can be a NULL pointer, so the interrupt
handler may end up writing to the invalid memory and cause crashes.
Add a check to xfer->tx_buf before using it.
Fixes: 1ce24864bff4 ("spi: mediatek: Only do dma for 4-byte aligned buffers")
Signed-off-by: Fei Shao <fshao@chromium.org>
---
drivers/spi/spi-mt65xx.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index 8d4633b353ee..86ea822c942b 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -788,17 +788,18 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
mdata->xfer_len = min(MTK_SPI_MAX_FIFO_SIZE, len);
mtk_spi_setup_packet(host);
- cnt = mdata->xfer_len / 4;
- iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
- trans->tx_buf + mdata->num_xfered, cnt);
+ if (trans->tx_buf) {
+ cnt = mdata->xfer_len / 4;
+ iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
+ trans->tx_buf + mdata->num_xfered, cnt);
- remainder = mdata->xfer_len % 4;
- if (remainder > 0) {
- reg_val = 0;
- memcpy(®_val,
- trans->tx_buf + (cnt * 4) + mdata->num_xfered,
- remainder);
- writel(reg_val, mdata->base + SPI_TX_DATA_REG);
+ remainder = mdata->xfer_len % 4;
+ if (remainder > 0) {
+ reg_val = 0;
+ memcpy(®_val,
+ trans->tx_buf + (cnt * 4) + mdata->num_xfered,
+ remainder);
+ writel(reg_val, mdata->base + SPI_TX_DATA_REG);
}
mtk_spi_enable_transfer(host);
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
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: [PATCH 0/2] Fixes NULL pointer access in spi-mt65xx.c
From: Fei Shao @ 2024-03-21 6:56 UTC (permalink / raw)
To: Mark Brown, AngeloGioacchino Del Regno
Cc: Daniel Kurtz, Matthias Brugger, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-spi
In-Reply-To: <20240321064313.1385316-1-fshao@chromium.org>
On Thu, Mar 21, 2024 at 2:43 PM Fei Shao <fshao@chromium.org> wrote:
>
> Hi,
>
> This series contains two patches for spi-mt65xx.c, both focusing on its
> interrupt handler mtk_spi_interrupt().
>
> The first patch is to fix a NULL pointer access in the interrupt
> handler, which is first found on a MT8186 Chromebook device when the
> system tries to establish host communication with its embedded
> controller.
>
> The second one is a decorative clean-up when I'm working on the first
> patch, which simply renames a variable to better follow the rest of the
> code.
> I put this after the first fix because I think that will make
> maintainers and users slightly easier to only backport the fix if
> needed.
>
> Looking forward to any feedback, thank you.
>
> Regards,
> Fei
Sorry, I found I messed things up in the last rebase and this doesn't compile.
I'll send a v2 soon so please disregard this series.
Regards,
Fei
>
>
> Fei Shao (2):
> spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
> spi: spi-mt65xx: Rename a variable in interrupt handler
>
> drivers/spi/spi-mt65xx.c | 47 ++++++++++++++++++++--------------------
> 1 file changed, 23 insertions(+), 24 deletions(-)
>
> --
> 2.44.0.396.g6e790dbe36-goog
>
_______________________________________________
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 v4 2/6] arm64: dts: ti: k3-j784s4: Add alias to MCU CPSW2G
From: Chintan Vankar @ 2024-03-21 7:01 UTC (permalink / raw)
To: Andrew Davis, Peter Rosin, Greg Kroah-Hartman, Conor Dooley,
Krzysztof Kozlowski, Rob Herring, Tero Kristo,
Vignesh Raghavendra, Nishanth Menon
Cc: linux-kernel, devicetree, linux-arm-kernel, srk, s-vadapalli,
r-gunasekaran, danishanwar
In-Reply-To: <c4b91154-7a8b-4642-a642-6ae93b448115@ti.com>
On 19/03/24 21:05, Andrew Davis wrote:
> On 3/11/24 5:44 AM, Chintan Vankar wrote:
>>
>>
>> On 31/01/24 21:06, Andrew Davis wrote:
>>> On 1/31/24 4:14 AM, Chintan Vankar wrote:
>>>> Add alias for the MCU CPSW2G port to enable Linux to fetch MAC Address
>>>> for the port directly from U-Boot.
>>>
>>> Could you explain *how* this alias allows Linux to fetch a MAC
>>> address from U-Boot? Sounds like we are doing something hacky here..
>>>
>> Using "probe_daughtercards()" function U-Boot parses MAC addresses from
>> EEPROM, then it internally calls "eth_env_set_enetaddr_by_index()"
>> function which stores these MAC addresses into environment variables
>> ethaddr, eth1addr, eth2addr and so on based on number of ports.
>>
>> U-Boot loads DTB during boot process, and it calls
>> "fdt_fixup_ethernet()" function, which uses environment variables to
>> update MAC addresses of ethernet ports as specified in the aliases
>> section.
>>
>
> So maybe a better question would by why does it need to use aliases
> for this?
>
Since "probe_daughtercards()" in U-Boot is implemented in a way that
it gets the MAC addresses fromm EEPROM and "fdt_fixup_ethernet()"
function configures MAC addresses for the ethernet ports as specified
in aliases section.
>>> Why can't Linux fetch the MAC from efuses the same way U-Boot does,
>>
>> Linux can fetch the MAC address from efuses if "ti,syscon-efuse"
>> property is enabled.
>>
>
> Then let's do it this way always.
>
Yes, Linux reads MAC addresses from efuse this way only, but the
functionality of Linux getting the MAC addresses from EEPROM is
not implemented.
>>> what happens if I don't use U-Boot to boot?
>>
>> If you don't use U-Boot to boot then the equivalent of
>> "probe_daughtercards()" has to be implemented which is currently
>> missing.
>>
>
> Or we just let Linux fetch it instead of implementing that function
> in all the possible bootloaders. This would also remove a DTB fixup.
> Those fixups should be avoided if at all possible.
>
Yes, we can let Linux fetch MAC addresses, but right now the Ethernet
driver in Linux can only fetch MAC addresses from "EFUSE" and not from
"EEPROM", and since the functionality to fetch MAC addresses from
"EEPROM" is implemented in U-Boot we are utilizing it.
> Andrew
>
>>>
>>> Andrew
>>>
>>>> ---
>>>> arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 1 +
>>>> 1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
>>>> b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
>>>> index f34b92acc56d..b74f7d3025de 100644
>>>> --- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
>>>> +++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
>>>> @@ -27,6 +27,7 @@ aliases {
>>>> mmc1 = &main_sdhci1;
>>>> i2c0 = &wkup_i2c0;
>>>> i2c3 = &main_i2c0;
>>>> + ethernet0 = &mcu_cpsw_port1;
>>>> };
>>>> memory@80000000 {
_______________________________________________
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 v4 1/5] dt-bindings: misc: Add mikrobus-connector
From: Vaishnav Achath @ 2024-03-21 7:07 UTC (permalink / raw)
To: Andrew Lunn
Cc: Ayush Singh, Michael Walle, open list, jkridner, robertcnelson,
lorforlinux, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Derek Kiernan,
Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman, Mark Brown,
Johan Hovold, Alex Elder,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM/TEXAS INSTRUMENTS K3 ARCHITECTURE,
open list:SPI SUBSYSTEM, moderated list:GREYBUS SUBSYSTEM,
Vaishnav M A
In-Reply-To: <4c299d42-84c7-46fc-952f-292cef1bb4b4@lunn.ch>
Hi Andrew,
On 20/03/24 00:53, Andrew Lunn wrote:
> On Tue, Mar 19, 2024 at 11:05:37PM +0530, Vaishnav Achath wrote:
>> Hi Andrew,
>>
>> On 19/03/24 17:55, Andrew Lunn wrote:
>>>> The device tree defines the SPI controller associated with mikroBUS SPI
>>>> pins. The driver on match queries and takes a reference to the SPI
>>>> controller but does nothing with it. Once a mikroBUS add-on board is
>>>> detected (by passing manifest using sysfs or reading from 1-wire EEPROM),
>>>> the driver parses the manifest, and if it detects an SPI device in manifest,
>>>> it registers SPI device along with setting properties such as `chip_select`,
>>>> `max_speed_hz`, `mode`, etc.,
>>>
>>> How complex can the description of the hardware be in the manifest?
>>>
>>> Could i describe an SPI to I2C converter? And then a few temperature
>>> sensors, a fan controller, and a GPIO controller on that I2C bus? And
>>> the GPIO controller is then used for LEDs and a push button? DT
>>> overlays could describe that. Can the manifest?
>>
>> No, it cannot describe such complex hardware, it can only describe simple
>> devices (sensors/displays .etc) on a standard mikroBUS add-on board, we did
>> a analysis on what mikroBUS add-on boards have driver support in Linux and
>> then noticed that most devices does not need this kind of complex
>> description to work:
>> https://elinux.org/MikroEClicks_with_Linux_Support
>
> Is that because the current software support is too limited? Are there
> manufactures who want to create more complex designed, but are limited
> by what can be described in the manifest?
>
most mikroBUS add-on boards in production lies in the category of
sensors, displays, connectivity, mixed signal (ADC/DAC .etc) and if you
look at the existing bindings under bindings/iio/ , most devices need
only simple descriptions and the properties are mainly standard bus
properties (SPI/I2C properties), IRQ, named-gpios, named properties,
regulators, clocks the extension to manifest was made taking this into
account and the named property description interface just maps the
manifest entries to the unified device property interface under
include/linux/property.h
> Do you have a list of boards without Linux support? Why do they not
> have Linux support? Is there a "vendor crap" driver which makes them
> work? Does it make them work by working around the manifest
> limitations?
>
I did the survey in 2020, close to 600 board did not have Linux support
and 150 board had support then, the boards which did not have Linux
support was mostly because there was no corresponding driver present and
a lot of these were similar to the 150 that had support (IIO sensors,
ADC, DACs .etc), there is no vendor(Example MikroElektronika) drivers
being maintained, so I am not sure if there are drivers working around
limitations of manifests , but for the 150 boards that we have tested
support we never had to make any changes to the underlying device
drivers to be supported.
>> The greybus manifest already is being used in the greybus susbystem for
>> describing an interface and there are already greybus controllers (SPI/I2C
>> .etc) being created according to the manifest contents, all this driver does
>> is to extend that format to be able to instantiate devices on these buses.
>
> I don't know anything about greybus, so let me ask a few background
> questions. Are these SPI and I2C controller plain Linux SPI and I2C
> controllers? They fit the usual device model, they appear in
> /sys/class/bus etc? Are the GPIO controllers also just plain Linux
> GPIO controllers? All the drivers have a bottom interface which uses
> greybus to perform some sort of RPC, but the top interface is standard
> Linux. So in fact they are not so different to I2C over USB, SPI over
> USB, GPIO over USB?
They are very similar and all the details you mentioned are correct, I
will provide some comments on the DT proposal you made and why we could
not implement that approach initially, primarily it is because PCIe and
USB has OF device tree support and USB interface nodes are children of
USB device nodes and there is some hardware parent we can tie USB
interface to and share/derive the of_node, but in case of greybus we
could not find such mapping - looking at your proposal that is more
maintainable in the long term, have some doubts regarding the proposal
will post in the other thread.
Thanks and Regards,
Vaishnav
>
> Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 0/2] Fixes NULL pointer access in spi-mt65xx.c
From: Fei Shao @ 2024-03-21 7:08 UTC (permalink / raw)
To: Mark Brown, AngeloGioacchino Del Regno
Cc: Fei Shao, Daniel Kurtz, Matthias Brugger, linux-arm-kernel,
linux-kernel, linux-mediatek, linux-spi
Hi,
This series contains two patches for spi-mt65xx.c, both focusing on its
interrupt handler mtk_spi_interrupt().
The first patch is to fix a NULL pointer access in the interrupt
handler, which is first found on a MT8186 Chromebook device when the
system tries to establish host communication with its embedded
controller.
The second one is a decorative clean-up when I'm working on the first
patch, which simply renames a variable to better follow the rest of the
code.
I put this after the first fix because I think that will make
maintainers and users slightly easier to only backport the fix if
needed.
Looking forward to any feedback, thank you.
Regards,
Fei
Changes in v2:
- Restore a missing curly brace being dropped during rebase
- Fix a typo in commit message (trans, not xfer)
Fei Shao (2):
spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
spi: spi-mt65xx: Rename a variable in interrupt handler
drivers/spi/spi-mt65xx.c | 48 ++++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 24 deletions(-)
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 2/2] spi: spi-mt65xx: Rename a variable in interrupt handler
From: Fei Shao @ 2024-03-21 7:08 UTC (permalink / raw)
To: Mark Brown, AngeloGioacchino Del Regno
Cc: Fei Shao, Matthias Brugger, linux-arm-kernel, linux-kernel,
linux-mediatek, linux-spi
In-Reply-To: <20240321070942.1587146-1-fshao@chromium.org>
All the spi_transfer variables in this file use the name "xfer" except
the one in mtk_spi_interrupt(). Align the naming for consistency and
easier searching.
While at it, reformat one memcpy() usage since the coding style allows
100 column lines today.
This commit has no functional change.
Signed-off-by: Fei Shao <fshao@chromium.org>
---
(no changes since v1)
drivers/spi/spi-mt65xx.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index e4cb22fe0075..36c2f52cd6b8 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -748,7 +748,7 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
u32 cmd, reg_val, cnt, remainder, len;
struct spi_controller *host = dev_id;
struct mtk_spi *mdata = spi_controller_get_devdata(host);
- struct spi_transfer *trans = mdata->cur_transfer;
+ struct spi_transfer *xfer = mdata->cur_transfer;
reg_val = readl(mdata->base + SPI_STATUS0_REG);
if (reg_val & MTK_SPI_PAUSE_INT_STATUS)
@@ -762,42 +762,40 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
return IRQ_HANDLED;
}
- if (!host->can_dma(host, NULL, trans)) {
- if (trans->rx_buf) {
+ if (!host->can_dma(host, NULL, xfer)) {
+ if (xfer->rx_buf) {
cnt = mdata->xfer_len / 4;
ioread32_rep(mdata->base + SPI_RX_DATA_REG,
- trans->rx_buf + mdata->num_xfered, cnt);
+ xfer->rx_buf + mdata->num_xfered, cnt);
remainder = mdata->xfer_len % 4;
if (remainder > 0) {
reg_val = readl(mdata->base + SPI_RX_DATA_REG);
- memcpy(trans->rx_buf +
- mdata->num_xfered +
- (cnt * 4),
+ memcpy(xfer->rx_buf + (cnt * 4) + mdata->num_xfered,
®_val,
remainder);
}
}
mdata->num_xfered += mdata->xfer_len;
- if (mdata->num_xfered == trans->len) {
+ if (mdata->num_xfered == xfer->len) {
spi_finalize_current_transfer(host);
return IRQ_HANDLED;
}
- len = trans->len - mdata->num_xfered;
+ len = xfer->len - mdata->num_xfered;
mdata->xfer_len = min(MTK_SPI_MAX_FIFO_SIZE, len);
mtk_spi_setup_packet(host);
- if (trans->tx_buf) {
+ if (xfer->tx_buf) {
cnt = mdata->xfer_len / 4;
iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
- trans->tx_buf + mdata->num_xfered, cnt);
+ xfer->tx_buf + mdata->num_xfered, cnt);
remainder = mdata->xfer_len % 4;
if (remainder > 0) {
reg_val = 0;
memcpy(®_val,
- trans->tx_buf + (cnt * 4) + mdata->num_xfered,
+ xfer->tx_buf + (cnt * 4) + mdata->num_xfered,
remainder);
writel(reg_val, mdata->base + SPI_TX_DATA_REG);
}
@@ -809,21 +807,21 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
}
if (mdata->tx_sgl)
- trans->tx_dma += mdata->xfer_len;
+ xfer->tx_dma += mdata->xfer_len;
if (mdata->rx_sgl)
- trans->rx_dma += mdata->xfer_len;
+ xfer->rx_dma += mdata->xfer_len;
if (mdata->tx_sgl && (mdata->tx_sgl_len == 0)) {
mdata->tx_sgl = sg_next(mdata->tx_sgl);
if (mdata->tx_sgl) {
- trans->tx_dma = sg_dma_address(mdata->tx_sgl);
+ xfer->tx_dma = sg_dma_address(mdata->tx_sgl);
mdata->tx_sgl_len = sg_dma_len(mdata->tx_sgl);
}
}
if (mdata->rx_sgl && (mdata->rx_sgl_len == 0)) {
mdata->rx_sgl = sg_next(mdata->rx_sgl);
if (mdata->rx_sgl) {
- trans->rx_dma = sg_dma_address(mdata->rx_sgl);
+ xfer->rx_dma = sg_dma_address(mdata->rx_sgl);
mdata->rx_sgl_len = sg_dma_len(mdata->rx_sgl);
}
}
@@ -841,7 +839,7 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
mtk_spi_update_mdata_len(host);
mtk_spi_setup_packet(host);
- mtk_spi_setup_dma_addr(host, trans);
+ mtk_spi_setup_dma_addr(host, xfer);
mtk_spi_enable_transfer(host);
return IRQ_HANDLED;
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
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 v2 1/2] spi: spi-mt65xx: Fix NULL pointer access in interrupt handler
From: Fei Shao @ 2024-03-21 7:08 UTC (permalink / raw)
To: Mark Brown, AngeloGioacchino Del Regno
Cc: Fei Shao, Daniel Kurtz, Matthias Brugger, linux-arm-kernel,
linux-kernel, linux-mediatek, linux-spi
In-Reply-To: <20240321070942.1587146-1-fshao@chromium.org>
The TX buffer in spi_transfer can be a NULL pointer, so the interrupt
handler may end up writing to the invalid memory and cause crashes.
Add a check to trans->tx_buf before using it.
Fixes: 1ce24864bff4 ("spi: mediatek: Only do dma for 4-byte aligned buffers")
Signed-off-by: Fei Shao <fshao@chromium.org>
---
Changes in v2:
- Restore a missing curly brace being dropped during rebase
- Fix a typo in commit message (trans, not xfer)
drivers/spi/spi-mt65xx.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c
index 8d4633b353ee..e4cb22fe0075 100644
--- a/drivers/spi/spi-mt65xx.c
+++ b/drivers/spi/spi-mt65xx.c
@@ -788,17 +788,19 @@ static irqreturn_t mtk_spi_interrupt(int irq, void *dev_id)
mdata->xfer_len = min(MTK_SPI_MAX_FIFO_SIZE, len);
mtk_spi_setup_packet(host);
- cnt = mdata->xfer_len / 4;
- iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
- trans->tx_buf + mdata->num_xfered, cnt);
+ if (trans->tx_buf) {
+ cnt = mdata->xfer_len / 4;
+ iowrite32_rep(mdata->base + SPI_TX_DATA_REG,
+ trans->tx_buf + mdata->num_xfered, cnt);
- remainder = mdata->xfer_len % 4;
- if (remainder > 0) {
- reg_val = 0;
- memcpy(®_val,
- trans->tx_buf + (cnt * 4) + mdata->num_xfered,
- remainder);
- writel(reg_val, mdata->base + SPI_TX_DATA_REG);
+ remainder = mdata->xfer_len % 4;
+ if (remainder > 0) {
+ reg_val = 0;
+ memcpy(®_val,
+ trans->tx_buf + (cnt * 4) + mdata->num_xfered,
+ remainder);
+ writel(reg_val, mdata->base + SPI_TX_DATA_REG);
+ }
}
mtk_spi_enable_transfer(host);
--
2.44.0.396.g6e790dbe36-goog
_______________________________________________
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: [PATCH v4 1/5] dt-bindings: misc: Add mikrobus-connector
From: Vaishnav Achath @ 2024-03-21 7:35 UTC (permalink / raw)
To: Andrew Lunn, Ayush Singh
Cc: Michael Walle, Krzysztof Kozlowski, open list, jkridner,
robertcnelson, lorforlinux, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Nishanth Menon, Vignesh Raghavendra, Tero Kristo,
Derek Kiernan, Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
Mark Brown, Johan Hovold, Alex Elder,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
moderated list:ARM/TEXAS INSTRUMENTS K3 ARCHITECTURE,
open list:SPI SUBSYSTEM, moderated list:GREYBUS SUBSYSTEM,
Vaishnav M A
In-Reply-To: <c368ee3b-1b80-46b1-9aa7-b7fc0094e3a1@lunn.ch>
Hi Andrew,
On 21/03/24 00:14, Andrew Lunn wrote:
> On Wed, Mar 20, 2024 at 10:09:05PM +0530, Ayush Singh wrote:
>> On 3/20/24 01:02, Andrew Lunn wrote:
>>
>>>> Yes, after discussion with Vaishnav and trying to brainstorm some way to do
>>>> the same thing with dt overlays, it seems that trying to use dt overlays
>>>> will mean need to have completely separate implementation of mikroBUS for
>>>> local ports and mikroBUS over greybus.
>>> Could you explain why please?
>>>
>>> Are greybus I2C bus masters different from physical I2C bus masters?
>>> Are greybus SPI bus masters different from physical SPI bus masters?
>>
>> Well, they are virtual, so they are not declared in the device tree. I have
>> linked the greybus i2c implementation. It basically allocates an i2c_adpater
>> and then adds it using `i2c_add_adapter` method. This adapter can then be
>> passed to say mikroBUS driver where it can be used as a normal i2c_adapter,
>> and we can register the device to it.
>
> Being virtual does not really stop it being added to the DT.
>
> I'm making this all up, but i assume it will look something like this:
>
> greybus@42 {
> compatible = "acme,greybus";
> reg = <0x42 0x100>;
>
> This would represent the greybus host controller.
>
> module@0 {
> reg = <0>;
>
> This would represent a module discovered on the bus. I assume there is
> some sort of addressing? The greybus core code dynamically creates the
> node in DT to describe the modules it has discovered. This is not too
> different to USB. You can already describe USB devices in DT, but the
> assumption is you know they exists, e.g. because they are hard wired,
> not hot-plugable. The USB core will associate the USB device with the
> node in DT. But actually creating a node in DT is not too big a jump.
>
> interface@0 {
> compatible = "greybus,i2c";
> reg = <0>;
> }
> interface@1 {
> compatible = "greybus,spi";
> reg = <1>;
> }
> interface@10 {
> compatible = "greybus,gpio";
> reg = <10>;
> }
>
> It can then enumerate the interfaces on the module, and create the I2C
> node, SPI bus node, the gpio controller etc. Again, the greybus core
> can add nodes to DT to described the discovered hardware, and
> associate them to the linux devices which are created.
>
This proposal looks great and would be the ideal solution, but we met
with few challenges when initially trying to implement something like
this and had to drop and take the route with minimal development effort
to just instantiate mikroBUS devices.
From what we understand, you are recommending to change the manifest
description format used by greybus to device tree and also add of_bus
support for greybus - now this will not only solve instantiating
mikrobus devices on greybus but even complex devices on greybus making
it a robust solution and using standard tools and support DT offers.
However we have a few doubts:
* For USB or PCIe, to add OF device tree support the parent devices are
physically present, for example USB device is a child node of USB
controller (physically description available in a SoC DT) and USB
interfaces are child of USB devices, how would that hierarchy look for
greybus devices?
Would it be
USB/UART/transport controller -> AP Bridge host controller -> Module ->
interface -> bundle -> CPort ?
When this mikrobus driver was initially implemented we could not think
of such an approach as the SVC and Control functionality were
implemented in userspace with gbridge (
https://github.com/anobli/gbridge ) with a netlink interface to kernel
greybus, but today there are references to do it completely in kernel (
drivers/greybus/gb-beagleplay.c) and your proposal is implementable.
Also with this the manifesto tool which is not very well maintained is
not necessary : https://github.com/projectara/manifesto
> That gives you what you need to load a DT overlay to make use of these
> devices. That overlay would contain one of your virtual mikroBUS
> controllers. This virtual controller is basically a phandle-proxy. The
> virtual mikroBUS controllers is a consumer of phandles to an I2C bus,
> an SPI bus, GPIO bus which makes up the pins routed to the mikroBUS
> connector. The virtual mikroBUS controllers is also a provider of an
> I2C bus, an SPI bus, GPIO controller. The mikroBUS device consumes
> these I2C bus, SPI bus etc. The virtual mikroBUS controllers makes it
> simpler for the device to find the resources it needs, since they are
> all in one place. For a physical mikroBUS you have a DT node with
> phandles to the physical devices. For greybus you create a virtual
> device with phandles to the virtual devices added to the DT bus.
>
> You then have everything you need to describe the mikroBUS
> devices. For very simple devices you convert the manifest to a DT
> overlay and load it. For complex devices you directly use a DT
> overlay.
>
> I also don't see any need to do the manifest to DT overlay conversion
> on the fly. You have a database of manifests. They could be converted
> to DT and then added to the linux-firmware repo, for example. If
> device with an unknown manifest is found,
How do we know if we found a device with unknown manifest if we don't
read the EEPROM?
it should be possible to
> read the manifest in userspace via its eeprom in /sys/class/. An tool
> could create DT blob and add it to /lib/firmware to get it working
> locally, and provide suggestions how to contribute it to the linux
> firmware project?
Agreed, but on what basis will you load the particular manifest for a
add-on board if you are not reading the DT overlay (or manifest blob)
from the EEPROM?
Thanks and Regards,
Vaishnav
>
> Andrew
_______________________________________________
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] dt-bindings: display: samsung,exynos5-dp: convert to DT Schema
From: Krzysztof Kozlowski @ 2024-03-21 7:37 UTC (permalink / raw)
To: Conor Dooley
Cc: Inki Dae, Seung-Woo Kim, Kyungmin Park, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Alim Akhtar,
dri-devel, devicetree, linux-arm-kernel, linux-samsung-soc,
linux-kernel
In-Reply-To: <20240320-reanalyze-koala-4c086a24fee0@spud>
On 20/03/2024 18:04, Conor Dooley wrote:
> On Wed, Mar 13, 2024 at 07:28:55PM +0100, Krzysztof Kozlowski wrote:
>
>> + clock-names:
>> + items:
>> + - const: dp
>
>> + phy-names:
>> + items:
>> + - const: dp
>
> The items lists here are redundant when you only have a single item, no?
> Isnt it just
> phy-names:
> const: dp
Somehow the convention for properties was to define the list. Unlike for
compatible where we use shorter syntax like you propose. Shall we change
the approach and use shorter syntax in general?
Best regards,
Krzysztof
_______________________________________________
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/3] dt-bindings: remoteproc: add Versal-NET platform
From: Krzysztof Kozlowski @ 2024-03-21 7:39 UTC (permalink / raw)
To: Tanmay Shah, andersson, mathieu.poirier, robh+dt,
krzysztof.kozlowski+dt, conor+dt, michal.simek, ben.levinsky
Cc: linux-remoteproc, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <72f3caa2-30ec-4191-9477-2d2a5e7f2f4b@amd.com>
On 20/03/2024 16:14, Tanmay Shah wrote:
>
>
> On 3/20/24 2:40 AM, Krzysztof Kozlowski wrote:
>> On 19/03/2024 15:42, Tanmay Shah wrote:
>>>
>>>
>>> On 3/19/24 12:30 AM, Krzysztof Kozlowski wrote:
>>>> On 19/03/2024 01:51, Tanmay Shah wrote:
>>>>> Hello Krzysztof,
>>>>>
>>>>> Thanks for reviews. Please find my comments below.
>>>>>
>>>>> On 3/17/24 1:53 PM, Krzysztof Kozlowski wrote:
>>>>>> On 15/03/2024 22:15, Tanmay Shah wrote:
>>>>>>> AMD-Xilinx Versal-NET platform is successor of Versal platform. It
>>>>>>> contains multiple clusters of cortex-R52 real-time processing units.
>>>>>>> Each cluster contains two cores of cortex-R52 processors. Each cluster
>>>>>>> can be configured in lockstep mode or split mode.
>>>>>>>
>>>>>>> Each R52 core is assigned 128KB of TCM memory. ATCM memory is 64KB, BTCM
>>>>>>> and CTCM memoreis are 32KB each. Each TCM memory has its own dedicated
>>>>>>> power-domain that needs to be requested before using it.
>>>>>>>
>>>>>>> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
>>>>>>> ---
>>>>>>> .../remoteproc/xlnx,zynqmp-r5fss.yaml | 220 +++++++++++++++---
>>>>>>> 1 file changed, 184 insertions(+), 36 deletions(-)
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
>>>>>>> index 711da0272250..55654ee02eef 100644
>>>>>>> --- a/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
>>>>>>> +++ b/Documentation/devicetree/bindings/remoteproc/xlnx,zynqmp-r5fss.yaml
>>>>>>> @@ -18,7 +18,9 @@ description: |
>>>>>>>
>>>>>>> properties:
>>>>>>> compatible:
>>>>>>> - const: xlnx,zynqmp-r5fss
>>>>>>> + enum:
>>>>>>> + - xlnx,zynqmp-r5fss
>>>>>>> + - xlnx,versal-net-r52fss
>>>>>>>
>>>>>>> "#address-cells":
>>>>>>> const: 2
>>>>>>> @@ -64,7 +66,9 @@ patternProperties:
>>>>>>>
>>>>>>> properties:
>>>>>>> compatible:
>>>>>>> - const: xlnx,zynqmp-r5f
>>>>>>> + enum:
>>>>>>> + - xlnx,zynqmp-r5f
>>>>>>> + - xlnx,versal-net-r52f
>>>>>>>
>>>>>>> reg:
>>>>>>> minItems: 1
>>>>>>> @@ -135,9 +139,11 @@ required:
>>>>>>> allOf:
>>>>>>> - if:
>>>>>>> properties:
>>>>>>> - xlnx,cluster-mode:
>>>>>>> - enum:
>>>>>>> - - 1
>>>>>>> + compatible:
>>>>>>> + contains:
>>>>>>> + enum:
>>>>>>> + - xlnx,versal-net-r52fss
>>>>>>
>>>>>> Why do you touch these lines?
>>>>>>
>>>>>>> +
>>>>>>> then:
>>>>>>> patternProperties:
>>>>>>> "^r5f@[0-9a-f]+$":
>>>>>>> @@ -149,16 +155,14 @@ allOf:
>>>>>>> items:
>>>>>>> - description: ATCM internal memory
>>>>>>> - description: BTCM internal memory
>>>>>>> - - description: extra ATCM memory in lockstep mode
>>>>>>> - - description: extra BTCM memory in lockstep mode
>>>>>>> + - description: CTCM internal memory
>>>>>>>
>>>>>>> reg-names:
>>>>>>> minItems: 1
>>>>>>> items:
>>>>>>> - - const: atcm0
>>>>>>> - - const: btcm0
>>>>>>> - - const: atcm1
>>>>>>> - - const: btcm1
>>>>>>> + - const: atcm
>>>>>>> + - const: btcm
>>>>>>> + - const: ctcm
>>>>>>>
>>>>>>> power-domains:
>>>>>>> minItems: 2
>>>>>>> @@ -166,33 +170,70 @@ allOf:
>>>>>>> - description: RPU core power domain
>>>>>>> - description: ATCM power domain
>>>>>>> - description: BTCM power domain
>>>>>>> - - description: second ATCM power domain
>>>>>>> - - description: second BTCM power domain
>>>>>>> + - description: CTCM power domain
>>>>>>>
>>>>>>> else:
>>>>>>> - patternProperties:
>>>>>>> - "^r5f@[0-9a-f]+$":
>>>>>>> - type: object
>>>>>>> -
>>>>>>> - properties:
>>>>>>> - reg:
>>>>>>> - minItems: 1
>>>>>>> - items:
>>>>>>> - - description: ATCM internal memory
>>>>>>> - - description: BTCM internal memory
>>>>>>> -
>>>>>>> - reg-names:
>>>>>>> - minItems: 1
>>>>>>> - items:
>>>>>>> - - const: atcm0
>>>>>>> - - const: btcm0
>>>>>>> -
>>>>>>> - power-domains:
>>>>>>> - minItems: 2
>>>>>>> - items:
>>>>>>> - - description: RPU core power domain
>>>>>>> - - description: ATCM power domain
>>>>>>> - - description: BTCM power domain
>>>>>>> + allOf:
>>>>>>> + - if:
>>>>>>> + properties:
>>>>>>> + xlnx,cluster-mode:
>>>>>>> + enum:
>>>>>>> + - 1
>>>>>>
>>>>>> Whatever you did here, is not really readable. You have now multiple
>>>>>> if:then:if:then embedded.
>>>>>
>>>>> For ZynqMP platform, TCM can be configured differently in lockstep mode
>>>>> and split mode.
>>>>>
>>>>> For Versal-NET no such configuration is available, but new CTCM memory
>>>>> is added.
>>>>>
>>>>> So, I am trying to achieve following representation of TCM for both:
>>>>>
>>>>> if: versal-net compatible
>>>>> then:
>>>>> ATCM - 64KB
>>>>> BTCM - 32KB
>>>>> CTCM - 32KB
>>>>>
>>>>> else: (ZynqMP compatible)
>>>>> if:
>>>>> xlnx,cluster-mode (lockstep mode)
>>>>> then:
>>>>> ATCM0 - 64KB
>>>>> BTCM0 - 64KB
>>>>> ATCM1 - 64KB
>>>>> BTCM1 - 64KB
>>>>> else: (split mode)
>>>>> ATCM0 - 64KB
>>>>> BTCM0 - 64KB
>>>>>
>>>>>
>>>>> If bindings are getting complicated, does it make sense to introduce
>>>>> new file for Versal-NET bindings? Let me know how you would like me
>>>>> to proceed.
>>>>
>>>> All this is broken in your previous patchset, but now we nicely see.
>>>>
>>>> No, this does not work like this. You do not have entirely different
>>>> programming models in one device, don't you?
>>>>
>>>
>>> I don't understand what do you mean? Programming model is same. Only number
>>> of TCMs are changing based on configuration and platform. I can certainly
>>> list different compatible for different platforms as requested. But other than
>>> that not sure what needs to be fixed.
>>
>> You cannot have same programming model with different memory mappings.
>> Anyway, please follow writing bindings rules: all of your different
>> devices must have dedicated compatible. I really though we talked about
>> two IPs on same SoC...
>
> I agree that Versal compatible should be added, I will do that in next revision.
>
> For ZynqMP case, it is two IPs on same SOC. In lockstep mode and split mode,
> same SOC is configuring TCM differently.
>
> How this should be resolved for Versal-NET ? Driver avoids such TCM configuration
> for Versal-NET.
Binding should describe the hardware, not what driver is doing
currently, so the question is: does your device have such properties or
not? Anyway, you need compatible per each variant and each SoC
implementation.
Best regards,
Krzysztof
_______________________________________________
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: thermal: convert st,stih407-thermal to DT schema
From: Krzysztof Kozlowski @ 2024-03-21 7:41 UTC (permalink / raw)
To: Raphael Gallais-Pou, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
Lukasz Luba, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Patrice Chotard, Lee Jones
Cc: linux-pm, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20240320-thermal-v3-1-700296694c4a@gmail.com>
On 20/03/2024 22:33, Raphael Gallais-Pou wrote:
> 'st,passive_colling_temp' does not appear in the device-tree, 'reg' and
> '#thermal-sensor-cells' are also missing in the device description.
>
> Convert st,stih407-thermal binding to DT schema format in order to clean
> unused 'st,passive_cooling_temp' and add missing properties.
>
> Signed-off-by: Raphael Gallais-Pou <rgallaispou@gmail.com>
> ---
> Changes in v3:
> - Specify const value for '#thermal-sensor-cells'
> - Add 'maxItems' for 'interrupts' property
> - Change commit log accordingly
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
_______________________________________________
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/4] dt-bindings: iommu: rockchip: Fix rk3588 variant
From: Krzysztof Kozlowski @ 2024-03-21 8:14 UTC (permalink / raw)
To: Emmanuel Gil Peyrot, linux-kernel
Cc: Ezequiel Garcia, Philipp Zabel, Mauro Carvalho Chehab,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner,
Joerg Roedel, Will Deacon, Robin Murphy, Sebastian Reichel,
Cristian Ciocaltea, Dragan Simic, Shreeya Patel, Chris Morgan,
Andy Yan, Nicolas Frattaroli, linux-media, linux-rockchip,
devicetree, linux-arm-kernel, iommu
In-Reply-To: <20240320173736.2720778-2-linkmauve@linkmauve.fr>
On 20/03/2024 18:37, Emmanuel Gil Peyrot wrote:
> The documentation got added in f8aa519976b38e67aae02d2db3e2998513305e80,
Please use commit SHA () syntax (see submitting patches).
> but it hasn’t been added to the driver so it was unused.
Eh? That's not how this works.
>
> Signed-off-by: Emmanuel Gil Peyrot <linkmauve@linkmauve.fr>
> ---
> arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 2 +-
That's DTS, not bindings.
> drivers/iommu/rockchip-iommu.c | 3 +++
Driver code cannot be combined with DTS in one patch.
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> index 87b83c87bd55..2a23b4dc36e4 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
> @@ -704,7 +704,7 @@ vp3: port@3 {
> };
>
> vop_mmu: iommu@fdd97e00 {
> - compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
> + compatible = "rockchip,rk3588-iommu";
NAK.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v10 01/11] dt-bindings: usb: usbmisc-imx: add fsl,imx8ulp-usbmisc compatible
From: Xu Yang @ 2024-03-21 8:14 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, shawnguo, conor+dt
Cc: s.hauer, kernel, festevam, linux-imx, peter.chen, xu.yang_2,
jun.li, linux-usb, devicetree, linux-arm-kernel, imx,
linux-kernel
Add "fsl,imx8ulp-usbmisc" compatible.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
Changes in v3:
- new patch due to missed this little one
Changes in v4:
- no changes
Changes in v5:
- add Acked-by tag
Changes in v6:
- no changes
Changes in v7:
- no changes
Changes in v8:
- no changes
Changes in v9:
- no changes
Changes in v10:
- no changes
---
Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml b/Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml
index 2d3589d284b2..0a6e7ac1b37e 100644
--- a/Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml
+++ b/Documentation/devicetree/bindings/usb/fsl,usbmisc.yaml
@@ -33,6 +33,7 @@ properties:
- fsl,imx7ulp-usbmisc
- fsl,imx8mm-usbmisc
- fsl,imx8mn-usbmisc
+ - fsl,imx8ulp-usbmisc
- const: fsl,imx7d-usbmisc
- const: fsl,imx6q-usbmisc
- items:
--
2.34.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 v10 02/11] arm64: dts: imx8ulp: add usb nodes
From: Xu Yang @ 2024-03-21 8:14 UTC (permalink / raw)
To: gregkh, robh+dt, krzysztof.kozlowski+dt, shawnguo, conor+dt
Cc: s.hauer, kernel, festevam, linux-imx, peter.chen, xu.yang_2,
jun.li, linux-usb, devicetree, linux-arm-kernel, imx,
linux-kernel
In-Reply-To: <20240321081439.541799-1-xu.yang_2@nxp.com>
Add USB nodes on i.MX8ULP platform which has 2 USB controllers.
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
Changes in v2:
- no changes
Changes in v3:
- no changes
Changes in v4:
- no changes
Changes in v5:
- no changes
Changes in v6:
- drop usbphy aliases
Changes in v7:
- no changes
Changes in v8:
- no changes
Changes in v9:
- no changes
Changes in v10:
- no changes
---
arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 62 ++++++++++++++++++++++
1 file changed, 62 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index c4a0082f30d3..7da9461a5745 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -472,6 +472,68 @@ usdhc2: mmc@298f0000 {
status = "disabled";
};
+ usbotg1: usb@29900000 {
+ compatible = "fsl,imx8ulp-usb", "fsl,imx7ulp-usb", "fsl,imx6ul-usb";
+ reg = <0x29900000 0x200>;
+ interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&pcc4 IMX8ULP_CLK_USB0>;
+ power-domains = <&scmi_devpd IMX8ULP_PD_USB0>;
+ phys = <&usbphy1>;
+ fsl,usbmisc = <&usbmisc1 0>;
+ ahb-burst-config = <0x0>;
+ tx-burst-size-dword = <0x8>;
+ rx-burst-size-dword = <0x8>;
+ status = "disabled";
+ };
+
+ usbmisc1: usbmisc@29900200 {
+ compatible = "fsl,imx8ulp-usbmisc", "fsl,imx7d-usbmisc",
+ "fsl,imx6q-usbmisc";
+ #index-cells = <1>;
+ reg = <0x29900200 0x200>;
+ status = "disabled";
+ };
+
+ usbphy1: usb-phy@29910000 {
+ compatible = "fsl,imx8ulp-usbphy", "fsl,imx7ulp-usbphy";
+ reg = <0x29910000 0x10000>;
+ interrupts = <GIC_SPI 104 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&pcc4 IMX8ULP_CLK_USB0_PHY>;
+ #phy-cells = <0>;
+ status = "disabled";
+ };
+
+ usbotg2: usb@29920000 {
+ compatible = "fsl,imx8ulp-usb", "fsl,imx7ulp-usb", "fsl,imx6ul-usb";
+ reg = <0x29920000 0x200>;
+ interrupts = <GIC_SPI 105 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&pcc4 IMX8ULP_CLK_USB1>;
+ power-domains = <&scmi_devpd IMX8ULP_PD_USDHC2_USB1>;
+ phys = <&usbphy2>;
+ fsl,usbmisc = <&usbmisc2 0>;
+ ahb-burst-config = <0x0>;
+ tx-burst-size-dword = <0x8>;
+ rx-burst-size-dword = <0x8>;
+ status = "disabled";
+ };
+
+ usbmisc2: usbmisc@29920200 {
+ compatible = "fsl,imx8ulp-usbmisc", "fsl,imx7d-usbmisc",
+ "fsl,imx6q-usbmisc";
+ #index-cells = <1>;
+ reg = <0x29920200 0x200>;
+ status = "disabled";
+ };
+
+ usbphy2: usb-phy@29930000 {
+ compatible = "fsl,imx8ulp-usbphy", "fsl,imx7ulp-usbphy";
+ reg = <0x29930000 0x10000>;
+ interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&pcc4 IMX8ULP_CLK_USB1_PHY>;
+ #phy-cells = <0>;
+ status = "disabled";
+ };
+
fec: ethernet@29950000 {
compatible = "fsl,imx8ulp-fec", "fsl,imx6ul-fec", "fsl,imx6q-fec";
reg = <0x29950000 0x10000>;
--
2.34.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
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