* RE: [PATCH v2 4/6] firmware: arm_scmi: add initial support for i.MX MISC protocol
From: Peng Fan @ 2024-04-07 1:03 UTC (permalink / raw)
To: Marco Felsch, Peng Fan (OSS)
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, imx@lists.linux.dev
In-Reply-To: <20240405164414.t3fqhnwudxxt2ozv@pengutronix.de>
> Subject: Re: [PATCH v2 4/6] firmware: arm_scmi: add initial support for i.MX
> MISC protocol
>
> Hi Peng,
>
> On 24-04-05, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > The i.MX MISC protocol is for misc settings, such as gpio expander
> > wakeup.
>
> Can you elaborate a bit more please?
The gpio expander is under M33(SCMI firmware used core) I2C control,
But the gpio expander supports board function such as PCIE_WAKEUP,
BTN_WAKEUP. So these are managed by MISC protocol.
SAI_CLK_MSEL in WAKEUP BLK CTRL is also managed by MISC Protocol.
And etc...
I will add more info in commit log in next version later, after I get more
reviews on the patchset.
Thanks,
Peng.
>
> Regards,
> Marco
>
>
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > drivers/firmware/arm_scmi/Kconfig | 10 ++
> > drivers/firmware/arm_scmi/Makefile | 1 +
> > drivers/firmware/arm_scmi/imx-sm-misc.c | 305
> ++++++++++++++++++++++++++++++++
> > include/linux/scmi_imx_protocol.h | 17 ++
> > 4 files changed, 333 insertions(+)
> >
> > diff --git a/drivers/firmware/arm_scmi/Kconfig
> > b/drivers/firmware/arm_scmi/Kconfig
> > index 56d11c9d9f47..bfeae92f6420 100644
> > --- a/drivers/firmware/arm_scmi/Kconfig
> > +++ b/drivers/firmware/arm_scmi/Kconfig
> > @@ -191,3 +191,13 @@ config IMX_SCMI_BBM_EXT
> > and BUTTON.
> >
> > This driver can also be built as a module.
> > +
> > +config IMX_SCMI_MISC_EXT
> > + tristate "i.MX SCMI MISC EXTENSION"
> > + depends on ARM_SCMI_PROTOCOL || (COMPILE_TEST && OF)
> > + default y if ARCH_MXC
> > + help
> > + This enables i.MX System MISC control logic such as gpio expander
> > + wakeup
> > +
> > + This driver can also be built as a module.
> > diff --git a/drivers/firmware/arm_scmi/Makefile
> > b/drivers/firmware/arm_scmi/Makefile
> > index 327687acf857..a23fde721222 100644
> > --- a/drivers/firmware/arm_scmi/Makefile
> > +++ b/drivers/firmware/arm_scmi/Makefile
> > @@ -12,6 +12,7 @@ scmi-transport-
> $(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO)
> > += virtio.o
> > scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
> > scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o
> > system.o voltage.o powercap.o
> > scmi-protocols-$(CONFIG_IMX_SCMI_BBM_EXT) += imx-sm-bbm.o
> > +scmi-protocols-$(CONFIG_IMX_SCMI_MISC_EXT) += imx-sm-misc.o
> > scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y)
> > $(scmi-transport-y)
> >
> > obj-$(CONFIG_ARM_SCMI_PROTOCOL) += scmi-core.o diff --git
> > a/drivers/firmware/arm_scmi/imx-sm-misc.c
> > b/drivers/firmware/arm_scmi/imx-sm-misc.c
> > new file mode 100644
> > index 000000000000..1b0ec2281518
> > --- /dev/null
> > +++ b/drivers/firmware/arm_scmi/imx-sm-misc.c
> > @@ -0,0 +1,305 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * System control and Management Interface (SCMI) NXP MISC Protocol
> > + *
> > + * Copyright 2024 NXP
> > + */
> > +
> > +#define pr_fmt(fmt) "SCMI Notifications MISC - " fmt
> > +
> > +#include <linux/bits.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/scmi_protocol.h>
> > +#include <linux/scmi_imx_protocol.h>
> > +
> > +#include "protocols.h"
> > +#include "notify.h"
> > +
> > +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x10000
> > +
> > +enum scmi_imx_misc_protocol_cmd {
> > + SCMI_IMX_MISC_CTRL_SET = 0x3,
> > + SCMI_IMX_MISC_CTRL_GET = 0x4,
> > + SCMI_IMX_MISC_CTRL_NOTIFY = 0x8,
> > +};
> > +
> > +struct scmi_imx_misc_info {
> > + u32 version;
> > + u32 nr_dev_ctrl;
> > + u32 nr_brd_ctrl;
> > + u32 nr_reason;
> > +};
> > +
> > +struct scmi_msg_imx_misc_protocol_attributes {
> > + __le32 attributes;
> > +};
> > +
> > +#define GET_BRD_CTRLS_NR(x) le32_get_bits((x), GENMASK(31,
> 24))
> > +#define GET_REASONS_NR(x) le32_get_bits((x), GENMASK(23, 16))
> > +#define GET_DEV_CTRLS_NR(x) le32_get_bits((x), GENMASK(15, 0))
> > +#define BRD_CTRL_START_ID BIT(15)
> > +
> > +struct scmi_imx_misc_ctrl_set_in {
> > + __le32 id;
> > + __le32 num;
> > + __le32 value[MISC_MAX_VAL];
> > +};
> > +
> > +struct scmi_imx_misc_ctrl_notify_in {
> > + __le32 ctrl_id;
> > + __le32 flags;
> > +};
> > +
> > +struct scmi_imx_misc_ctrl_notify_payld {
> > + __le32 ctrl_id;
> > + __le32 flags;
> > +};
> > +
> > +struct scmi_imx_misc_ctrl_get_out {
> > + __le32 num;
> > + __le32 *val;
> > +};
> > +
> > +static int scmi_imx_misc_attributes_get(const struct scmi_protocol_handle
> *ph,
> > + struct scmi_imx_misc_info *mi)
> > +{
> > + int ret;
> > + struct scmi_xfer *t;
> > + struct scmi_msg_imx_misc_protocol_attributes *attr;
> > +
> > + ret = ph->xops->xfer_get_init(ph, PROTOCOL_ATTRIBUTES, 0,
> > + sizeof(*attr), &t);
> > + if (ret)
> > + return ret;
> > +
> > + attr = t->rx.buf;
> > +
> > + ret = ph->xops->do_xfer(ph, t);
> > + if (!ret) {
> > + mi->nr_dev_ctrl = GET_DEV_CTRLS_NR(attr->attributes);
> > + mi->nr_brd_ctrl = GET_BRD_CTRLS_NR(attr->attributes);
> > + mi->nr_reason = GET_REASONS_NR(attr->attributes);
> > + dev_info(ph->dev, "i.MX MISC NUM DEV CTRL: %d, NUM
> BRD CTRL: %d,NUM Reason: %d\n",
> > + mi->nr_dev_ctrl, mi->nr_brd_ctrl, mi->nr_reason);
> > + }
> > +
> > + ph->xops->xfer_put(ph, t);
> > +
> > + return ret;
> > +}
> > +
> > +static int scmi_imx_misc_ctrl_validate_id(const struct
> scmi_protocol_handle *ph,
> > + u32 ctrl_id)
> > +{
> > + struct scmi_imx_misc_info *mi = ph->get_priv(ph);
> > +
> > + if ((ctrl_id < BRD_CTRL_START_ID) && (ctrl_id > mi->nr_dev_ctrl))
> > + return -EINVAL;
> > + if (ctrl_id >= BRD_CTRL_START_ID + mi->nr_brd_ctrl)
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
> > +
> > +static int scmi_imx_misc_ctrl_notify(const struct scmi_protocol_handle
> *ph,
> > + u32 ctrl_id, u32 flags)
> > +{
> > + struct scmi_imx_misc_ctrl_notify_in *in;
> > + struct scmi_xfer *t;
> > + int ret;
> > +
> > + ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> > + if (ret)
> > + return ret;
> > +
> > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_NOTIFY,
> > + sizeof(*in), 0, &t);
> > + if (ret)
> > + return ret;
> > +
> > + in = t->tx.buf;
> > + in->ctrl_id = cpu_to_le32(ctrl_id);
> > + in->flags = cpu_to_le32(flags);
> > +
> > + ret = ph->xops->do_xfer(ph, t);
> > +
> > + ph->xops->xfer_put(ph, t);
> > +
> > + return ret;
> > +}
> > +
> > +static int
> > +scmi_imx_misc_ctrl_set_notify_enabled(const struct
> scmi_protocol_handle *ph,
> > + u8 evt_id, u32 src_id, bool enable) {
> > + int ret;
> > +
> > + ret = scmi_imx_misc_ctrl_notify(ph, src_id, enable ? evt_id : 0);
> > + if (ret)
> > + dev_err(ph->dev, "FAIL_ENABLED - evt[%X] src[%d] -
> ret:%d\n",
> > + evt_id, src_id, ret);
> > +
> > + return ret;
> > +}
> > +
> > +static int scmi_imx_misc_ctrl_get_num_sources(const struct
> > +scmi_protocol_handle *ph) {
> > + return GENMASK(15, 0);
> > +}
> > +
> > +static void *
> > +scmi_imx_misc_ctrl_fill_custom_report(const struct scmi_protocol_handle
> *ph,
> > + u8 evt_id, ktime_t timestamp,
> > + const void *payld, size_t payld_sz,
> > + void *report, u32 *src_id)
> > +{
> > + const struct scmi_imx_misc_ctrl_notify_payld *p = payld;
> > + struct scmi_imx_misc_ctrl_notify_report *r = report;
> > +
> > + if (sizeof(*p) != payld_sz)
> > + return NULL;
> > +
> > + r->timestamp = timestamp;
> > + r->ctrl_id = p->ctrl_id;
> > + r->flags = p->flags;
> > + *src_id = r->ctrl_id;
> > + dev_dbg(ph->dev, "%s: ctrl_id: %d flags: %d\n", __func__,
> > + r->ctrl_id, r->flags);
> > +
> > + return r;
> > +}
> > +
> > +static const struct scmi_event_ops scmi_imx_misc_event_ops = {
> > + .get_num_sources = scmi_imx_misc_ctrl_get_num_sources,
> > + .set_notify_enabled = scmi_imx_misc_ctrl_set_notify_enabled,
> > + .fill_custom_report = scmi_imx_misc_ctrl_fill_custom_report,
> > +};
> > +
> > +static const struct scmi_event scmi_imx_misc_events[] = {
> > + {
> > + .id = SCMI_EVENT_IMX_MISC_CONTROL_DISABLED,
> > + .max_payld_sz = sizeof(struct
> scmi_imx_misc_ctrl_notify_payld),
> > + .max_report_sz = sizeof(struct
> scmi_imx_misc_ctrl_notify_report),
> > + },
> > + {
> > + .id = SCMI_EVENT_IMX_MISC_CONTROL_FALLING_EDGE,
> > + .max_payld_sz = sizeof(struct
> scmi_imx_misc_ctrl_notify_payld),
> > + .max_report_sz = sizeof(struct
> scmi_imx_misc_ctrl_notify_report),
> > + },
> > + {
> > + .id = SCMI_EVENT_IMX_MISC_CONTROL_RISING_EDGE,
> > + .max_payld_sz = sizeof(struct
> scmi_imx_misc_ctrl_notify_payld),
> > + .max_report_sz = sizeof(struct
> scmi_imx_misc_ctrl_notify_report),
> > + }
> > +};
> > +
> > +static struct scmi_protocol_events scmi_imx_misc_protocol_events = {
> > + .queue_sz = SCMI_PROTO_QUEUE_SZ,
> > + .ops = &scmi_imx_misc_event_ops,
> > + .evts = scmi_imx_misc_events,
> > + .num_events = ARRAY_SIZE(scmi_imx_misc_events), };
> > +
> > +static int scmi_imx_misc_protocol_init(const struct
> > +scmi_protocol_handle *ph) {
> > + struct scmi_imx_misc_info *minfo;
> > + u32 version;
> > + int ret;
> > +
> > + ret = ph->xops->version_get(ph, &version);
> > + if (ret)
> > + return ret;
> > +
> > + dev_info(ph->dev, "NXP SM MISC Version %d.%d\n",
> > + PROTOCOL_REV_MAJOR(version),
> PROTOCOL_REV_MINOR(version));
> > +
> > + minfo = devm_kzalloc(ph->dev, sizeof(*minfo), GFP_KERNEL);
> > + if (!minfo)
> > + return -ENOMEM;
> > +
> > + ret = scmi_imx_misc_attributes_get(ph, minfo);
> > + if (ret)
> > + return ret;
> > +
> > + return ph->set_priv(ph, minfo, version); }
> > +
> > +static int scmi_imx_misc_ctrl_get(const struct scmi_protocol_handle *ph,
> > + u32 ctrl_id, u32 *num, u32 *val) {
> > + struct scmi_imx_misc_ctrl_get_out *out;
> > + struct scmi_xfer *t;
> > + int ret, i;
> > +
> > + ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> > + if (ret)
> > + return ret;
> > +
> > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_GET,
> sizeof(u32),
> > + 0, &t);
> > + if (ret)
> > + return ret;
> > +
> > + put_unaligned_le32(ctrl_id, t->tx.buf);
> > + ret = ph->xops->do_xfer(ph, t);
> > + if (!ret) {
> > + out = t->rx.buf;
> > + *num = le32_to_cpu(out->num);
> > + for (i = 0; i < *num && i < MISC_MAX_VAL; i++)
> > + val[i] = le32_to_cpu(out->val[i]);
> > + }
> > +
> > + ph->xops->xfer_put(ph, t);
> > +
> > + return ret;
> > +}
> > +
> > +static int scmi_imx_misc_ctrl_set(const struct scmi_protocol_handle *ph,
> > + u32 ctrl_id, u32 num, u32 *val) {
> > + struct scmi_imx_misc_ctrl_set_in *in;
> > + struct scmi_xfer *t;
> > + int ret, i;
> > +
> > + ret = scmi_imx_misc_ctrl_validate_id(ph, ctrl_id);
> > + if (ret)
> > + return ret;
> > +
> > + if (num > MISC_MAX_VAL)
> > + return -EINVAL;
> > +
> > + ret = ph->xops->xfer_get_init(ph, SCMI_IMX_MISC_CTRL_SET,
> sizeof(*in),
> > + 0, &t);
> > + if (ret)
> > + return ret;
> > +
> > + in = t->tx.buf;
> > + in->id = cpu_to_le32(ctrl_id);
> > + in->num = cpu_to_le32(num);
> > + for (i = 0; i < num; i++)
> > + in->value[i] = cpu_to_le32(val[i]);
> > +
> > + ret = ph->xops->do_xfer(ph, t);
> > +
> > + ph->xops->xfer_put(ph, t);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct scmi_imx_misc_proto_ops scmi_imx_misc_proto_ops =
> {
> > + .misc_ctrl_set = scmi_imx_misc_ctrl_set,
> > + .misc_ctrl_get = scmi_imx_misc_ctrl_get, };
> > +
> > +static const struct scmi_protocol scmi_imx_misc = {
> > + .id = SCMI_PROTOCOL_IMX_MISC,
> > + .owner = THIS_MODULE,
> > + .instance_init = &scmi_imx_misc_protocol_init,
> > + .ops = &scmi_imx_misc_proto_ops,
> > + .events = &scmi_imx_misc_protocol_events,
> > + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION, };
> > +module_scmi_protocol(scmi_imx_misc);
> > diff --git a/include/linux/scmi_imx_protocol.h
> > b/include/linux/scmi_imx_protocol.h
> > index 90ce011a4429..a69bd4a20f0f 100644
> > --- a/include/linux/scmi_imx_protocol.h
> > +++ b/include/linux/scmi_imx_protocol.h
> > @@ -13,8 +13,14 @@
> > #include <linux/notifier.h>
> > #include <linux/types.h>
> >
> > +#define SCMI_PAYLOAD_LEN 100
> > +
> > +#define SCMI_ARRAY(X, Y) ((SCMI_PAYLOAD_LEN - (X)) / sizeof(Y))
> > +#define MISC_MAX_VAL SCMI_ARRAY(8, uint32_t)
> > +
> > enum scmi_nxp_protocol {
> > SCMI_PROTOCOL_IMX_BBM = 0x81,
> > + SCMI_PROTOCOL_IMX_MISC = 0x84,
> > };
> >
> > struct scmi_imx_bbm_proto_ops {
> > @@ -42,4 +48,15 @@ struct scmi_imx_bbm_notif_report {
> > unsigned int rtc_id;
> > unsigned int rtc_evt;
> > };
> > +
> > +struct scmi_imx_misc_ctrl_notify_report {
> > + ktime_t timestamp;
> > + unsigned int ctrl_id;
> > + unsigned int flags;
> > +};
> > +
> > +struct scmi_imx_misc_proto_ops {
> > + int (*misc_ctrl_set)(const struct scmi_protocol_handle *ph, u32 id,
> u32 num, u32 *val);
> > + int (*misc_ctrl_get)(const struct scmi_protocol_handle *ph, u32 id,
> > +u32 *num, u32 *val); };
> > #endif
> >
> > --
> > 2.37.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 v2 2/6] dt-bindings: firmware: add i.MX SCMI Extension protocol
From: Peng Fan @ 2024-04-07 0:51 UTC (permalink / raw)
To: Krzysztof Kozlowski, Peng Fan (OSS), Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Sudeep Holla,
Cristian Marussi
Cc: devicetree@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <9aab8dab-27c4-40b6-b1d5-0a2babe0700d@kernel.org>
> Subject: Re: [PATCH v2 2/6] dt-bindings: firmware: add i.MX SCMI Extension
> protocol
>
> On 05/04/2024 14:39, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add i.MX SCMI Extension protocols bindings for:
> > - Battery Backed Secure Module(BBSM)
>
> Which is what?
I should say BBM(BBSM + BBNSM), BBM has RTC and ON/OFF
key features, but BBM is managed by SCMI firmware and exported
to agent by BBM protocol. So add bindings for i.MX BBM protocol.
Is this ok?
>
> > - MISC settings such as General Purpose Registers settings.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> > .../devicetree/bindings/firmware/imx,scmi.yaml | 80
> ++++++++++++++++++++++
> > 1 file changed, 80 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
> > b/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
> > new file mode 100644
> > index 000000000000..7ee19a661d83
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
> > @@ -0,0 +1,80 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) # Copyright 2024
> > +NXP %YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Ffirmware%2Fimx%2Cscmi.yaml%23&data=05%7
> C02%7Cp
> >
> +eng.fan%40nxp.com%7C5d16781d3eca425a342508dc562910b7%7C686ea
> 1d3bc2b4c
> >
> +6fa92cd99c5c301635%7C0%7C0%7C638479981570959816%7CUnknown%
> 7CTWFpbGZsb
> >
> +3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> 0%3D
> >
> +%7C0%7C%7C%7C&sdata=mWNwPvu2eyF18MroVOBHb%2Fjeo%2BIHfV5V
> h%2F9ebdx65MM
> > +%3D&reserved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> > +cetree.org%2Fmeta-
> schemas%2Fcore.yaml%23&data=05%7C02%7Cpeng.fan%40nx
> >
> +p.com%7C5d16781d3eca425a342508dc562910b7%7C686ea1d3bc2b4c6fa
> 92cd99c5c
> >
> +301635%7C0%7C0%7C638479981570971949%7CUnknown%7CTWFpbGZs
> b3d8eyJWIjoiM
> >
> +C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7
> C%7C%7
> >
> +C&sdata=v4XnGG00D4I8j5MJvDUVYMRTm7yRrvz0V3fUyc5KAAA%3D&reser
> ved=0
> > +
> > +title: i.MX System Control and Management Interface(SCMI) Vendor
> > +Protocols Extension
> > +
> > +maintainers:
> > + - Peng Fan <peng.fan@nxp.com>
> > +
> > +allOf:
> > + - $ref: arm,scmi.yaml#
>
> Sorry, but arm,scmi is a final schema. Is your plan to define some common
> part?
No. I just wanna add vendor extension per SCMI spec.
0x80-0xFF:
Reserved for vendor or platform-specific extensions to this interface
Each vendor may have different usage saying id 0x81, so I add
i.MX dt-schema file.
>
> > +
> > +properties:
> > + protocol@81:
> > + $ref: 'arm,scmi.yaml#/$defs/protocol-node'
> > + unevaluatedProperties: false
> > + description:
> > + The BBM Protocol is for managing Battery Backed Secure Module
> (BBSM) RTC
> > + and the ON/OFF Key
> > +
> > + properties:
> > + reg:
> > + const: 0x81
> > +
> > + required:
> > + - reg
> > +
> > + protocol@84:
> > + $ref: 'arm,scmi.yaml#/$defs/protocol-node'
> > + unevaluatedProperties: false
> > + description:
> > + The MISC Protocol is for managing SoC Misc settings, such as
> > + GPR settings
>
> Genera register is not a setting... this is a pleonasm. Please be more specific
> what is the GPR, MISC protocol etc.
The MISC Protocol is for managing SoC Misc settings, such as SAI MCLK/MQS in
Always On domain BLK CTRL, SAI_CLK_SEL in WAKEUP BLK CTRL, gpio
expanders which is under control of SCMI firmware.
> > +
> > + properties:
> > + reg:
> > + const: 0x84
> > +
> > + wakeup-sources:
> > + description:
> > + Each entry consists of 2 integers, represents the source
> > + and electric signal edge
>
> Can you answer questions from reviewers?
Sorry. Is this ok?
minItems: 1
maxItems: 32
>
> > + items:
> > + items:
> > + - description: the wakeup source
> > + - description: the wakeup electric signal edge
> > + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +
> > + required:
> > + - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + firmware {
> > + scmi {
> > + compatible = "arm,scmi";
>
> > + mboxes = <&mu2 5 0>, <&mu2 3 0>, <&mu2 3 1>;
> > + shmem = <&scmi_buf0>, <&scmi_buf1>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + protocol@81 {
> > + reg = <0x81>;
> > + };
> > +
> > + protocol@84 {
> > + reg = <0x84>;
> > + wakeup-sources = <0x8000 1
> > + 0x8001 1
> > + 0x8002 1
> > + 0x8003 1
> > + 0x8004 1>;
>
> Nothing improved... If you are going to ignore reviews, then you will only get
> NAKed.
Sorry, you mean the examples, or the whole dt-schema?
Thanks,
Peng.
>
> 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 v2 1/6] dt-bindings: firmware: arm,scmi: set additionalProperties to true
From: Peng Fan @ 2024-04-07 0:37 UTC (permalink / raw)
To: Krzysztof Kozlowski, Peng Fan (OSS), Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Shawn Guo, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Sudeep Holla,
Cristian Marussi
Cc: devicetree@vger.kernel.org, imx@lists.linux.dev,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <614b5107-656d-4d41-99c1-77941c48342c@kernel.org>
> Subject: Re: [PATCH v2 1/6] dt-bindings: firmware: arm,scmi: set
> additionalProperties to true
>
> On 05/04/2024 14:39, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > When adding vendor extension protocols, there is dt-schema warning:
> > "
> > imx,scmi.example.dtb: scmi: 'protocol@81', 'protocol@84' do not match
> > any of the regexes: 'pinctrl-[0-9]+'
> > "
> >
> > Set additionalProperties to true to address the issue.
>
> I do not see anything addressed here, except making the binding accepting
> anything anywhere...
I not wanna add vendor protocols in arm,scmi.yaml, so will introduce
a new yaml imx.scmi.yaml which add i.MX SCMI protocol extension.
With additionalProperties set to false, I not know how, please suggest.
Thanks,
Peng.
>
> 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 3/4] selftests/arm: Add elf test
From: Muhammad Usama Anjum @ 2024-04-06 21:30 UTC (permalink / raw)
To: Dev Jain, shuah, linux-arm-kernel
Cc: Muhammad Usama Anjum, linux-kselftest, linux-kernel,
Anshuman.Khandual, suzuki.poulose, ryan.roberts, rob.herring,
Catalin.Marinas, broonie, will, mark.rutland
In-Reply-To: <20240405084410.256788-4-dev.jain@arm.com>
On 4/5/24 1:44 PM, Dev Jain wrote:
> This patch introduces an ELF parsing test; the 5th byte of the ELF header
> must be 0x01 for a 32-bit process. A basic sanity check is required to ensure
> that we are actually testing a 32-bit build.
>
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
> tools/testing/selftests/arm/elf/parse_elf.c | 75 +++++++++++++++++++++
> 1 file changed, 75 insertions(+)
> create mode 100644 tools/testing/selftests/arm/elf/parse_elf.c
>
> diff --git a/tools/testing/selftests/arm/elf/parse_elf.c b/tools/testing/selftests/arm/elf/parse_elf.c
> new file mode 100644
> index 000000000000..decd65699858
> --- /dev/null
> +++ b/tools/testing/selftests/arm/elf/parse_elf.c
> @@ -0,0 +1,75 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024 ARM Limited
> + *
> + * Author : Dev Jain <dev.jain@arm.com>
> + *
> + * Parse elf header to confirm 32-bit process
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <elf.h>
> +#include <stdint.h>
> +
> +#include <kselftest.h>
> +
> +/* The ELF file header. This appears at the start of every ELF file. */
> +
> +struct elf_header {
> + unsigned char e_ident[16]; /* Magic number and other info */
> + uint16_t e_type; /* Object file type */
> + uint16_t e_machine; /* Architecture */
> + uint32_t e_version; /* Object file version */
> + uint64_t e_entry; /* Entry point virtual address */
> + uint64_t e_phoff; /* Program header table file offset */
> + uint64_t e_shoff; /* Section header table file offset */
> + uint32_t e_flags; /* Processor-specific flags */
> + uint16_t e_ehsize; /* ELF header size in bytes */
> + uint16_t e_phentsize; /* Program header table entry size */
> + uint16_t e_phnum; /* Program header table entry count */
> + uint16_t e_shentsize; /* Section header table entry size */
> + uint16_t e_shnum; /* Section header table entry count */
> + uint16_t e_shstrndx; /* Section header string table index */
> +};
> +
> +static int read_elf_header(const char *elfFile)
> +{
> + struct elf_header header;
> + FILE *file;
> +
> + file = fopen(elfFile, "r");
> + if (file) {
> +
> + /* store header in struct */
> + fread(&header, 1, sizeof(header), file);
> + fclose(file);
> +
> + /* sanity check: does it really follow ELF format */
> + if (header.e_ident[0] == 0x7f &&
> + header.e_ident[1] == 'E' &&
> + header.e_ident[2] == 'L' &&
> + header.e_ident[3] == 'F') {
> + if (header.e_ident[4] == 0x01)
> + return 0;
> + return 1;
> + }
> + ksft_exit_fail_msg("Cannot parse /proc/self/exe\n");
> + }
> + ksft_exit_fail_msg("Cannot open /proc/self/exe\n");
> + exit(EXIT_FAILURE);
Instead of failing and exiting multiple times here, use ksft_print_msg,
return error or -1 from here and fail the test case in ksft_test_result().
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + const char *file_name;
> +
> + ksft_print_header();
> + ksft_set_plan(1);
> +
> + file_name = "/proc/self/exe";
> + ksft_test_result(read_elf_header(file_name) == 0, "ELF is 32 bit\n");
> + ksft_finished();
> +}
--
BR,
Muhammad Usama Anjum
_______________________________________________
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/4] selftests/arm: Add signal tests
From: Muhammad Usama Anjum @ 2024-04-06 21:28 UTC (permalink / raw)
To: Dev Jain, shuah, linux-arm-kernel
Cc: Muhammad Usama Anjum, linux-kselftest, linux-kernel,
Anshuman.Khandual, suzuki.poulose, ryan.roberts, rob.herring,
Catalin.Marinas, broonie, will, mark.rutland
In-Reply-To: <20240405084410.256788-3-dev.jain@arm.com>
On 4/5/24 1:44 PM, Dev Jain wrote:
> This patch introduces two signal tests, and generic test wrappers similar to
> selftests/arm64/signal directory, along with the mangling testcases found
> therein. arm_cpsr, dumped by the kernel to user space in the ucontext structure
> to the signal handler, is mangled with. The kernel must spot this illegal
> attempt and the testcases are expected to terminate via SEGV.
>
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
> .../selftests/arm/signal/test_signals.c | 27 ++
> .../selftests/arm/signal/test_signals.h | 74 +++++
> .../selftests/arm/signal/test_signals_utils.c | 257 ++++++++++++++++++
> .../selftests/arm/signal/test_signals_utils.h | 128 +++++++++
> .../signal/testcases/mangle_cpsr_aif_bits.c | 33 +++
> .../mangle_cpsr_invalid_compat_toggle.c | 29 ++
Too many files/tests in one patch. Break this patch logically into multiple
tests for easy to review and follow.
> 6 files changed, 548 insertions(+)
> create mode 100644 tools/testing/selftests/arm/signal/test_signals.c
> create mode 100644 tools/testing/selftests/arm/signal/test_signals.h
> create mode 100644 tools/testing/selftests/arm/signal/test_signals_utils.c
> create mode 100644 tools/testing/selftests/arm/signal/test_signals_utils.h
> create mode 100644 tools/testing/selftests/arm/signal/testcases/mangle_cpsr_aif_bits.c
> create mode 100644 tools/testing/selftests/arm/signal/testcases/mangle_cpsr_invalid_compat_toggle.c
>
> diff --git a/tools/testing/selftests/arm/signal/test_signals.c b/tools/testing/selftests/arm/signal/test_signals.c
> new file mode 100644
> index 000000000000..1ecf1e9f041c
> --- /dev/null
> +++ b/tools/testing/selftests/arm/signal/test_signals.c
> @@ -0,0 +1,27 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024 ARM Limited
> + *
> + * Generic test wrapper for arm signal tests.
> + *
> + * Each test provides its own tde struct tdescr descriptor to link with
> + * this wrapper. Framework provides common helpers.
> + */
> +#include <kselftest.h>
> +
> +#include "test_signals.h"
> +#include "test_signals_utils.h"
> +
> +struct tdescr *current = &tde;
> +
> +int main(int argc, char *argv[])
> +{
> + ksft_print_msg("%s :: %s\n", current->name, current->descr);
> + if (test_setup(current) && test_init(current)) {
> + test_run(current);
> + test_cleanup(current);
> + }
> + test_result(current);
> +
> + return current->result;
> +}
This test isn't TAP compliant. Please make this and all tests TAP
compilant. The 1/4 patch has example of TAP usage.
> diff --git a/tools/testing/selftests/arm/signal/test_signals.h b/tools/testing/selftests/arm/signal/test_signals.h
> new file mode 100644
> index 000000000000..bbd147127d66
> --- /dev/null
> +++ b/tools/testing/selftests/arm/signal/test_signals.h
> @@ -0,0 +1,74 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2024 ARM Limited */
> +
> +#ifndef __TEST_SIGNALS_H__
> +#define __TEST_SIGNALS_H__
> +
> +#include <signal.h>
> +#include <stdbool.h>
> +#include <ucontext.h>
> +
> +/*
> + * Using ARCH specific and sanitized Kernel headers from the tree.
> + */
> +#include <asm/ptrace.h>
> +#include <asm/hwcap.h>
> +
> +/*
> + * A descriptor used to describe and configure a test case.
> + * Fields with a non-trivial meaning are described inline in the following.
> + */
> +struct tdescr {
> + /* KEEP THIS FIELD FIRST for easier lookup from assembly */
> + void *token;
> + /* when disabled token based sanity checking is skipped in handler */
> + bool sanity_disabled;
> + /* just a name for the test-case; manadatory field */
> + char *name;
> + char *descr;
> +
> + bool initialized;
> + unsigned int minsigstksz;
> + /* signum used as a test trigger. Zero if no trigger-signal is used */
> + int sig_trig;
> + /*
> + * signum considered as a successful test completion.
> + * Zero when no signal is expected on success
> + */
> + int sig_ok;
> + /* signum expected on unsupported CPU features. */
> + int sig_unsupp;
> + /* a timeout in second for test completion */
> + unsigned int timeout;
> + bool triggered;
> + bool pass;
> + unsigned int result;
> + /* optional sa_flags for the installed handler */
> + int sa_flags;
> + ucontext_t saved_uc;
> + /* used by get_current_ctx() */
> + size_t live_sz;
> + ucontext_t *live_uc;
> + volatile sig_atomic_t live_uc_valid;
> + /* optional test private data */
> + void *priv;
> +
> + /* a custom setup: called alternatively to default_setup */
> + int (*setup)(struct tdescr *td);
> + /* a custom init: called by default test init after test_setup */
> + bool (*init)(struct tdescr *td);
> + /* a custom cleanup function called before test exits */
> + void (*cleanup)(struct tdescr *td);
> + /* an optional function to be used as a trigger for starting test */
> + int (*trigger)(struct tdescr *td);
> + /*
> + * the actual test-core: invoked differently depending on the
> + * presence of the trigger function above; this is mandatory
> + */
> + int (*run)(struct tdescr *td, siginfo_t *si, ucontext_t *uc);
> + /* an optional function for custom results' processing */
> + void (*check_result)(struct tdescr *td);
> +};
> +
> +extern struct tdescr tde;
> +#endif
> diff --git a/tools/testing/selftests/arm/signal/test_signals_utils.c b/tools/testing/selftests/arm/signal/test_signals_utils.c
> new file mode 100644
> index 000000000000..96aeb11de151
> --- /dev/null
> +++ b/tools/testing/selftests/arm/signal/test_signals_utils.c
> @@ -0,0 +1,257 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (C) 2024 ARM Limited */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <signal.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <assert.h>
> +#include <sys/auxv.h>
> +#include <linux/auxvec.h>
> +#include <ucontext.h>
> +
> +#include <asm/unistd.h>
> +
> +#include <kselftest.h>
> +
> +#include "test_signals.h"
> +#include "test_signals_utils.h"
> +
> +
> +extern struct tdescr *current;
> +
> +static int sig_copyctx = SIGTRAP;
> +
> +static void unblock_signal(int signum)
> +{
> + sigset_t sset;
> +
> + sigemptyset(&sset);
> + sigaddset(&sset, signum);
> + sigprocmask(SIG_UNBLOCK, &sset, NULL);
> +}
> +
> +static void default_result(struct tdescr *td, bool force_exit)
> +{
> + if (td->result == KSFT_SKIP) {
> + fprintf(stderr, "==>> completed. SKIP.\n");
> + } else if (td->pass) {
> + fprintf(stderr, "==>> completed. PASS(1)\n");
> + td->result = KSFT_PASS;
> + } else {
> + fprintf(stdout, "==>> completed. FAIL(0)\n");
> + td->result = KSFT_FAIL;
> + }
> +
> + if (force_exit)
> + exit(td->result);
> +}
> +
> +/*
> + * The following handle_signal_* helpers are used by main default_handler
> + * and are meant to return true when signal is handled successfully:
> + * when false is returned instead, it means that the signal was somehow
> + * unexpected in that context and it was NOT handled; default_handler will
> + * take care of such unexpected situations.
> + */
> +
> +static bool handle_signal_unsupported(struct tdescr *td,
> + siginfo_t *si, void *uc)
> +{
> +
> + /* Mangling PC to avoid loops on original SIGILL */
> + ((ucontext_t *)uc)->uc_mcontext.arm_pc += 4;
> +
> + if (!td->initialized) {
> + fprintf(stderr,
> + "Got SIG_UNSUPP @test_init. Ignore.\n");
> + } else {
> + fprintf(stderr,
> + "-- RX SIG_UNSUPP on unsupported feat...OK\n");
> + td->pass = 1;
> + default_result(current, 1);
> + }
> +
> + return true;
> +}
> +
> +static bool handle_signal_trigger(struct tdescr *td,
> + siginfo_t *si, void *uc)
> +{
> + td->triggered = 1;
> +
> + /* ->run was asserted NON-NULL in test_setup() already */
> + td->run(td, si, uc);
> +
> + return true;
> +}
> +
> +static bool handle_signal_ok(struct tdescr *td,
> + siginfo_t *si, void *uc)
> +{
> +
> + /*
> + * it's a bug in the test code when this assert fail:
> + * if sig_trig was defined, it must have been used before getting here.
> + */
> + assert(!td->sig_trig || td->triggered);
> + fprintf(stderr,
> + "SIG_OK -- SP:0x%lX si_addr@:%p si_code:%d token@:%p offset:%d\n",
> + ((ucontext_t *)uc)->uc_mcontext.arm_sp,
> + si->si_addr, si->si_code, td->token, td->token - si->si_addr);
> +
> + /*
> + * Trying to narrow down the SEGV to the ones generated by Kernel itself
> + * via arm64_notify_segfault(). This is a best-effort check anyway, and
> + * the si_code check may need to change if this aspect of the kernel
> + * ABI changes.
> + */
> + if (td->sig_ok == SIGSEGV && si->si_code != SEGV_ACCERR) {
> + fprintf(stdout,
> + "si_code != SEGV_ACCERR...test is probably broken!\n");
> + abort();
> + }
> + td->pass = 1;
> + /*
> + * Some tests can lead to SEGV loops: in such a case we want to
> + * terminate immediately exiting straight away; some others are not
> + * supposed to outlive the signal handler code, due to the content of
> + * the fake sigframe which caused the signal itself.
> + */
> + default_result(current, 1);
> +
> + return true;
> +}
> +
> +static void default_handler(int signum, siginfo_t *si, void *uc)
> +{
> + if (current->sig_unsupp && signum == current->sig_unsupp &&
> + handle_signal_unsupported(current, si, uc)) {
> + fprintf(stderr, "Handled SIG_UNSUPP\n");
> + } else if (current->sig_trig && signum == current->sig_trig &&
> + handle_signal_trigger(current, si, uc)) {
> + fprintf(stderr, "Handled SIG_TRIG\n");
> + } else if (current->sig_ok && signum == current->sig_ok &&
> + handle_signal_ok(current, si, uc)) {
> + fprintf(stderr, "Handled SIG_OK\n");
> + } else if (signum == sig_copyctx && current->live_uc) {
> + fprintf(stderr, "Handled SIG_COPYCTX\n");
> + } else {
> + if (signum == SIGALRM && current->timeout) {
> + fprintf(stderr, "-- Timeout !\n");
> + } else {
> + fprintf(stderr,
> + "-- RX UNEXPECTED SIGNAL: %d code %d address %p\n",
> + signum, si->si_code, si->si_addr);
> + }
> + default_result(current, 1);
> + }
> +}
> +
> +static int default_setup(struct tdescr *td)
> +{
> + struct sigaction sa;
> +
> + sa.sa_sigaction = default_handler;
> + sa.sa_flags = SA_SIGINFO | SA_RESTART;
> + sa.sa_flags |= td->sa_flags;
> + sigemptyset(&sa.sa_mask);
> + /* uncatchable signals naturally skipped ... */
> + for (int sig = 1; sig < 32; sig++)
> + sigaction(sig, &sa, NULL);
> + /*
> + * RT Signals default disposition is Term but they cannot be
> + * generated by the Kernel in response to our tests; so just catch
> + * them all and report them as UNEXPECTED signals.
> + */
> + for (int sig = SIGRTMIN; sig <= SIGRTMAX; sig++)
> + sigaction(sig, &sa, NULL);
> +
> + /* just in case...unblock explicitly all we need */
> + if (td->sig_trig)
> + unblock_signal(td->sig_trig);
> + if (td->sig_ok)
> + unblock_signal(td->sig_ok);
> + if (td->sig_unsupp)
> + unblock_signal(td->sig_unsupp);
> +
> + if (td->timeout) {
> + unblock_signal(SIGALRM);
> + alarm(td->timeout);
> + }
> + fprintf(stderr, "Registered handlers for all signals.\n");
> +
> + return 1;
> +}
> +
> +static inline int default_trigger(struct tdescr *td)
> +{
> + return !raise(td->sig_trig);
> +}
> +
> +int test_init(struct tdescr *td)
> +{
> + if (td->sig_trig == sig_copyctx) {
> + fprintf(stdout,
> + "Signal %d is RESERVED, cannot be used as a trigger. Aborting\n",
> + sig_copyctx);
> + return 0;
> + }
> + /* just in case */
> + unblock_signal(sig_copyctx);
> +
> + td->minsigstksz = getauxval(AT_MINSIGSTKSZ);
> + if (!td->minsigstksz)
> + td->minsigstksz = MINSIGSTKSZ;
> + fprintf(stderr, "Detected MINSTKSIGSZ:%d\n", td->minsigstksz);
> +
> + /* Perform test specific additional initialization */
> + if (td->init && !td->init(td)) {
> + fprintf(stderr, "FAILED Testcase initialization.\n");
> + return 0;
> + }
> + td->initialized = 1;
> + fprintf(stderr, "Testcase initialized.\n");
> +
> + return 1;
> +}
> +
> +int test_setup(struct tdescr *td)
> +{
> + /* assert core invariants symptom of a rotten testcase */
> + assert(current);
> + assert(td);
> + assert(td->name);
> + assert(td->run);
> +
> + /* Default result is FAIL if test setup fails */
> + td->result = KSFT_FAIL;
> + if (td->setup)
> + return td->setup(td);
> + else
> + return default_setup(td);
> +}
> +
> +int test_run(struct tdescr *td)
> +{
> + if (td->trigger)
> + return td->trigger(td);
> + else if (td->sig_trig)
> + return default_trigger(td);
> + else
> + return td->run(td, NULL, NULL);
> +}
> +
> +void test_result(struct tdescr *td)
> +{
> + if (td->initialized && td->result != KSFT_SKIP && td->check_result)
> + td->check_result(td);
> + default_result(td, 0);
> +}
> +
> +void test_cleanup(struct tdescr *td)
> +{
> + if (td->cleanup)
> + td->cleanup(td);
> +}
> diff --git a/tools/testing/selftests/arm/signal/test_signals_utils.h b/tools/testing/selftests/arm/signal/test_signals_utils.h
> new file mode 100644
> index 000000000000..386dcc6c268d
> --- /dev/null
> +++ b/tools/testing/selftests/arm/signal/test_signals_utils.h
> @@ -0,0 +1,128 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (C) 2024 ARM Limited */
> +
> +#ifndef __TEST_SIGNALS_UTILS_H__
> +#define __TEST_SIGNALS_UTILS_H__
> +
> +#include <assert.h>
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include <linux/compiler.h>
> +#include "test_signals.h"
> +
> +int test_init(struct tdescr *td);
> +int test_setup(struct tdescr *td);
> +void test_cleanup(struct tdescr *td);
> +int test_run(struct tdescr *td);
> +void test_result(struct tdescr *td);
> +
> +/*
> + * Obtaining a valid and full-blown ucontext_t from userspace is tricky:
> + * libc getcontext does() not save all the regs and messes with some of
> + * them (pstate value in particular is not reliable).
> + *
> + * Here we use a service signal to grab the ucontext_t from inside a
> + * dedicated signal handler, since there, it is populated by Kernel
> + * itself in setup_sigframe(). The grabbed context is then stored and
> + * made available in td->live_uc.
> + *
> + * As service-signal is used a SIGTRAP induced by a 'brk' instruction,
> + * because here we have to avoid syscalls to trigger the signal since
> + * they would cause any SVE sigframe content (if any) to be removed.
> + *
> + * Anyway this function really serves a dual purpose:
> + *
> + * 1. grab a valid sigcontext into td->live_uc for result analysis: in
> + * such case it returns 1.
> + *
> + * 2. detect if, somehow, a previously grabbed live_uc context has been
> + * used actively with a sigreturn: in such a case the execution would have
> + * magically resumed in the middle of this function itself (seen_already==1):
> + * in such a case return 0, since in fact we have not just simply grabbed
> + * the context.
> + *
> + * This latter case is useful to detect when a fake_sigreturn test-case has
> + * unexpectedly survived without hitting a SEGV.
> + *
> + * Note that the case of runtime dynamically sized sigframes (like in SVE
> + * context) is still NOT addressed: sigframe size is supposed to be fixed
> + * at sizeof(ucontext_t).
> + */
> +static __always_inline bool get_current_context(struct tdescr *td,
> + ucontext_t *dest_uc,
> + size_t dest_sz)
> +{
> + static volatile bool seen_already;
> + int i;
> + char *uc = (char *)dest_uc;
> +
> + assert(td && dest_uc);
> + /* it's a genuine invocation..reinit */
> + seen_already = 0;
> + td->live_uc_valid = 0;
> + td->live_sz = dest_sz;
> +
> + /*
> + * This is a memset() but we don't want the compiler to
> + * optimise it into either instructions or a library call
> + * which might be incompatible with streaming mode.
> + */
> + for (i = 0; i < td->live_sz; i++) {
> + uc[i] = 0;
> + OPTIMIZER_HIDE_VAR(uc[0]);
> + }
> +
> + td->live_uc = dest_uc;
> + /*
> + * Grab ucontext_t triggering a SIGTRAP.
> + *
> + * Note that:
> + * - live_uc_valid is declared volatile sig_atomic_t in
> + * struct tdescr since it will be changed inside the
> + * sig_copyctx handler
> + * - the additional 'memory' clobber is there to avoid possible
> + * compiler's assumption on live_uc_valid and the content
> + * pointed by dest_uc, which are all changed inside the signal
> + * handler
> + * - BRK causes a debug exception which is handled by the Kernel
> + * and finally causes the SIGTRAP signal to be delivered to this
> + * test thread. Since such delivery happens on the ret_to_user()
> + * /do_notify_resume() debug exception return-path, we are sure
> + * that the registered SIGTRAP handler has been run to completion
> + * before the execution path is restored here: as a consequence
> + * we can be sure that the volatile sig_atomic_t live_uc_valid
> + * carries a meaningful result. Being in a single thread context
> + * we'll also be sure that any access to memory modified by the
> + * handler (namely ucontext_t) will be visible once returned.
> + * - note that since we are using a breakpoint instruction here
> + * to cause a SIGTRAP, the ucontext_t grabbed from the signal
> + * handler would naturally contain a PC pointing exactly to this
> + * BRK line, which means that, on return from the signal handler,
> + * or if we place the ucontext_t on the stack to fake a sigreturn,
> + * we'll end up in an infinite loop of BRK-SIGTRAP-handler.
> + * For this reason we take care to artificially move forward the
> + * PC to the next instruction while inside the signal handler.
> + */
> + asm volatile ("brk #666"
> + : "+m" (*dest_uc)
> + :
> + : "memory");
> +
> + /*
> + * If we get here with seen_already==1 it implies the td->live_uc
> + * context has been used to get back here....this probably means
> + * a test has failed to cause a SEGV...anyway live_uc does not
> + * point to a just acquired copy of ucontext_t...so return 0
> + */
> + if (seen_already) {
> + fprintf(stdout,
> + "Unexpected successful sigreturn detected: live_uc is stale !\n");
> + return 0;
> + }
> + seen_already = 1;
> +
> + return td->live_uc_valid;
> +}
> +
> +#endif
> diff --git a/tools/testing/selftests/arm/signal/testcases/mangle_cpsr_aif_bits.c b/tools/testing/selftests/arm/signal/testcases/mangle_cpsr_aif_bits.c
> new file mode 100644
> index 000000000000..f422cd11ccf2
> --- /dev/null
> +++ b/tools/testing/selftests/arm/signal/testcases/mangle_cpsr_aif_bits.c
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024 ARM Limited
> + *
> + * Try to mangle the ucontext from inside a signal handler, mangling the
> + * AIF bits in an illegal manner: this attempt must be spotted by Kernel
> + * and the test case is expected to be terminated via SEGV.
> + *
> + */
> +
> +#include "test_signals_utils.h"
> +
> +static int mangle_invalid_cpsr_run(struct tdescr *td, siginfo_t *si,
> + ucontext_t *uc)
> +{
> +
> + /*
> + * This config should trigger a SIGSEGV by Kernel when it checks
> + * the sigframe consistency in valid_user_regs() routine.
> + */
> + uc->uc_mcontext.arm_cpsr |= PSR_A_BIT | PSR_I_BIT | PSR_F_BIT;
> +
> + return 1;
> +}
> +
> +struct tdescr tde = {
> + .sanity_disabled = true,
> + .name = "MANGLE_CPSR_INVALID_AIF_BITS",
> + .descr = "Mangling uc_mcontext with INVALID AIF_BITS",
> + .sig_trig = SIGUSR1,
> + .sig_ok = SIGSEGV,
> + .run = mangle_invalid_cpsr_run,
> +};
> diff --git a/tools/testing/selftests/arm/signal/testcases/mangle_cpsr_invalid_compat_toggle.c b/tools/testing/selftests/arm/signal/testcases/mangle_cpsr_invalid_compat_toggle.c
> new file mode 100644
> index 000000000000..cb7eb8aec7f2
> --- /dev/null
> +++ b/tools/testing/selftests/arm/signal/testcases/mangle_cpsr_invalid_compat_toggle.c
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024 ARM Limited
> + *
> + * Try to mangle the ucontext from inside a signal handler, toggling
> + * the execution state bit: this attempt must be spotted by Kernel and
> + * the test case is expected to be terminated via SEGV.
> + */
> +
> +#include "test_signals_utils.h"
> +
> +static int mangle_invalid_cpsr_run(struct tdescr *td, siginfo_t *si,
> + ucontext_t *uc)
> +{
> +
> + /* This config should trigger a SIGSEGV by Kernel */
> + uc->uc_mcontext.arm_cpsr ^= MODE32_BIT;
> +
> + return 1;
> +}
> +
> +struct tdescr tde = {
> + .sanity_disabled = true,
> + .name = "MANGLE_CPSR_INVALID_STATE_TOGGLE",
> + .descr = "Mangling uc_mcontext with INVALID STATE_TOGGLE",
> + .sig_trig = SIGUSR1,
> + .sig_ok = SIGSEGV,
> + .run = mangle_invalid_cpsr_run,
> +};
--
BR,
Muhammad Usama Anjum
_______________________________________________
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] selftests/arm: Add mm test
From: Muhammad Usama Anjum @ 2024-04-06 21:23 UTC (permalink / raw)
To: Dev Jain, shuah, linux-arm-kernel
Cc: Muhammad Usama Anjum, linux-kselftest, linux-kernel,
Anshuman.Khandual, suzuki.poulose, ryan.roberts, rob.herring,
Catalin.Marinas, broonie, will, mark.rutland
In-Reply-To: <20240405084410.256788-2-dev.jain@arm.com>
On 4/5/24 1:44 PM, Dev Jain wrote:
> This patch tests the 4GB VA restriction for 32-bit processes; it is required
> to test the compat layer, whether the kernel knows that it is running a 32-bit
> process or not. Chunks are allocated until the VA gets exhausted; mmap must
> fail beyond 4GB. This is asserted against the VA mappings found
> in /proc/self/maps.
>
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
> tools/testing/selftests/arm/mm/compat_va.c | 94 ++++++++++++++++++++++
> 1 file changed, 94 insertions(+)
> create mode 100644 tools/testing/selftests/arm/mm/compat_va.c
>
> diff --git a/tools/testing/selftests/arm/mm/compat_va.c b/tools/testing/selftests/arm/mm/compat_va.c
> new file mode 100644
> index 000000000000..3a78f240bc87
> --- /dev/null
> +++ b/tools/testing/selftests/arm/mm/compat_va.c
> @@ -0,0 +1,94 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2024 ARM Limited
> + *
> + * Author : Dev Jain <dev.jain@arm.com>
> + *
> + * Tests 4GB VA restriction for 32 bit process
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <sys/mman.h>
> +
> +#include <linux/sizes.h>
> +#include <kselftest.h>
> +
> +#define MAP_CHUNK_SIZE SZ_1M
> +#define NR_CHUNKS_4G (SZ_1G / MAP_CHUNK_SIZE) * 4 /* prevent overflow */
> +
> +static int validate_address_hint(void)
> +{
> + char *ptr;
> +
> + ptr = mmap((void *) (1UL << 29), MAP_CHUNK_SIZE, PROT_READ |
> + PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +
> + if (ptr == MAP_FAILED)
> + return 0;
> +
> + return 1;
Usually we return negative value instead of positive one which indicates
error situation.
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + char *ptr[NR_CHUNKS_4G + 3];
> + char line[1000];
> + const char *file_name;
> + int chunks;
> + FILE *file;
> + int i;
> +
> + ksft_print_header();
> + ksft_set_plan(1);
There are multiple test cases. Instead of saying there is only 1 test.
There should be multiple ksft_test_result{_pass,_fail} statements for each
sub-tests.
> +
> + /* try allocation beyond 4 GB */
> + for (i = 0; i < NR_CHUNKS_4G + 3; ++i) {
> + ptr[i] = mmap(NULL, MAP_CHUNK_SIZE, PROT_READ | PROT_WRITE,
> + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
> +
> + if (ptr[i] == MAP_FAILED) {
> + if (validate_address_hint())
> + ksft_exit_fail_msg("VA exhaustion failed\n");
> + break;
> + }
> + }
> +
> + chunks = i;
> + if (chunks >= NR_CHUNKS_4G) {
> + ksft_test_result_fail("mmapped chunks beyond 4GB\n");
> + ksft_finished();
> + }
> +
> + /* parse /proc/self/maps, confirm 32 bit VA mappings */
> + file_name = "/proc/self/maps";
> + file = fopen(file_name, "r");
> + if (file == NULL)
> + ksft_exit_fail_msg("/proc/self/maps cannot be opened\n");
> +
> + while (fgets(line, sizeof(line), file)) {
> + const char *whitespace_loc, *hyphen_loc;
> +
> + hyphen_loc = strchr(line, '-');
> + whitespace_loc = strchr(line, ' ');
> +
> + if (!(hyphen_loc && whitespace_loc)) {
> + ksft_test_result_skip("Unexpected format");
> + ksft_finished();
I'm unable to follow as there are too many return statements. If you divide
the test into multiple sub-tests, you can skip/pass/fail each sub-test easily.
> + }
> +
> + if ((hyphen_loc - line > 8) ||
> + (whitespace_loc - hyphen_loc) > 9) {
> + ksft_test_result_fail("Memory map more than 32 bits\n");
> + ksft_finished();
> + }
> + }
> +
> + for (int i = 0; i < chunks; ++i)
> + munmap(ptr[i], MAP_CHUNK_SIZE);
> +
> + ksft_test_result_pass("Test\n");
> + ksft_finished();
> +}
--
BR,
Muhammad Usama Anjum
_______________________________________________
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 4/4] selftests: Add build infrastructure along with README
From: Muhammad Usama Anjum @ 2024-04-06 21:15 UTC (permalink / raw)
To: Dev Jain, shuah, linux-arm-kernel
Cc: Muhammad Usama Anjum, linux-kselftest, linux-kernel,
Anshuman.Khandual, suzuki.poulose, ryan.roberts, rob.herring,
Catalin.Marinas, broonie, will, mark.rutland
In-Reply-To: <20240405084410.256788-5-dev.jain@arm.com>
On 4/5/24 1:44 PM, Dev Jain wrote:
> Add arm target, individual Makefile targets, and instructions to build the
> tests.
>
> Signed-off-by: Dev Jain <dev.jain@arm.com>
> ---
> tools/testing/selftests/Makefile | 1 +
> tools/testing/selftests/arm/Makefile | 57 +++++++++++++++++++++
> tools/testing/selftests/arm/README | 31 +++++++++++
> tools/testing/selftests/arm/elf/Makefile | 6 +++
> tools/testing/selftests/arm/mm/Makefile | 6 +++
> tools/testing/selftests/arm/signal/Makefile | 30 +++++++++++
> 6 files changed, 131 insertions(+)
> create mode 100644 tools/testing/selftests/arm/Makefile
> create mode 100644 tools/testing/selftests/arm/README
> create mode 100644 tools/testing/selftests/arm/elf/Makefile
> create mode 100644 tools/testing/selftests/arm/mm/Makefile
> create mode 100644 tools/testing/selftests/arm/signal/Makefile
Add one recursive .gitignore file or multiple .gitignore files and put
generated object files in it to avoid clutter of generated objects in git
history.
>
> diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
> index 15b6a111c3be..8478d94cda4c 100644
> --- a/tools/testing/selftests/Makefile
> +++ b/tools/testing/selftests/Makefile
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
> TARGETS += alsa
> TARGETS += amd-pstate
> +TARGETS += arm
> TARGETS += arm64
> TARGETS += bpf
> TARGETS += breakpoints
> diff --git a/tools/testing/selftests/arm/Makefile b/tools/testing/selftests/arm/Makefile
> new file mode 100644
> index 000000000000..039224bc006e
> --- /dev/null
> +++ b/tools/testing/selftests/arm/Makefile
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# When ARCH not overridden for crosscompiling, lookup machine
> +ARCH ?= $(shell uname -m 2>/dev/null || echo not)
> +
> +ifneq (,$(filter $(ARCH),aarch64 arm64 arm armv7l armv8l))
> +ARM_SUBTARGETS ?= mm signal elf
> +else
> +ARM_SUBTARGETS :=
> +endif
> +
> +CFLAGS := -Wall -O2 -g -static
> +
> +# A proper top_srcdir is needed by KSFT(lib.mk)
> +top_srcdir = $(realpath ../../../../)
> +
> +# Additional include paths needed by kselftest.h and local headers
> +CFLAGS += -I$(top_srcdir)/tools/testing/selftests/
> +
> +CFLAGS += -I$(top_srcdir)/tools/include
Please use KHDR_INCLUDE instead of using absolute path
> +
> +export CFLAGS
> +export top_srcdir
> +
> +all:
> + @for DIR in $(ARM_SUBTARGETS); do \
> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
> + mkdir -p $$BUILD_TARGET; \
> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
> + done
> +
> +install: all
> + @for DIR in $(ARM_SUBTARGETS); do \
> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
> + done
> +
> +run_tests: all
> + @for DIR in $(ARM_SUBTARGETS); do \
> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
> + done
> +
> +# Avoid any output on non arm on emit_tests
> +emit_tests:
> + @for DIR in $(ARM_SUBTARGETS); do \
> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
> + done
> +
> +clean:
> + @for DIR in $(ARM_SUBTARGETS); do \
> + BUILD_TARGET=$(OUTPUT)/$$DIR; \
> + make OUTPUT=$$BUILD_TARGET -C $$DIR $@; \
> + done
> +
> +.PHONY: all clean install run_tests emit_tests
> diff --git a/tools/testing/selftests/arm/README b/tools/testing/selftests/arm/README
> new file mode 100644
> index 000000000000..1a05c043d7ee
> --- /dev/null
> +++ b/tools/testing/selftests/arm/README
> @@ -0,0 +1,31 @@
> +KSelfTest ARM
> +===============
> +
> +- This is a series of compatibility tests, wherein the source files are
> + built statically into a 32 bit ELF; they should pass on both 32 and 64
> + bit kernels. They are not built or run but just skipped completely when
> + env-variable ARCH is found to be different than 'arm64' or 'arm' and
> + `uname -m` reports other than 'aarch64', 'armv7l' or 'armv8l'.
> +
> +- Please ensure that the test kernel is built with CONFIG_COMPAT enabled.
Please create a config file and put all the per-requisite configurations in
that. For example, look at tools/testing/selftests/mm/config
> +
> +- Holding true the above, ARM KSFT tests can be run within the KSelfTest
> + framework using standard Linux top-level-makefile targets. Please set
> + $(CROSS_COMPILE) to 'arm-linux-gnueabi-' or 'arm-linux-gnueabihf-'.
> +
> + $ make TARGETS=arm kselftest-clean
> + $ make $(CROSS_COMPILE) TARGETS=arm kselftest
> +
> + or
> +
> + $ make $(CROSS_COMPILE) -C tools/testing/selftests TARGETS=arm \
> + INSTALL_PATH=<your-installation-path> install
> +
> + or, alternatively, only specific arm/ subtargets can be picked:
> +
> + $ make $(CROSS_COMPILE) -C tools/testing/selftests TARGETS=arm \
> + ARM_SUBTARGETS="signal" INSTALL_PATH=<your-installation-path> \
> + install
> +
> + Further details on building and running KFST can be found in:
> + Documentation/dev-tools/kselftest.rst
Thanks for this well written documentation.
> diff --git a/tools/testing/selftests/arm/elf/Makefile b/tools/testing/selftests/arm/elf/Makefile
> new file mode 100644
> index 000000000000..86636fe02994
> --- /dev/null
> +++ b/tools/testing/selftests/arm/elf/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2024 ARM Limited
> +
> +TEST_GEN_PROGS := parse_elf
> +
> +include ../../lib.mk
> diff --git a/tools/testing/selftests/arm/mm/Makefile b/tools/testing/selftests/arm/mm/Makefile
> new file mode 100644
> index 000000000000..d8bfa45df98c
> --- /dev/null
> +++ b/tools/testing/selftests/arm/mm/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2024 ARM Limited
> +
> +TEST_GEN_PROGS := compat_va
> +
> +include ../../lib.mk
> diff --git a/tools/testing/selftests/arm/signal/Makefile b/tools/testing/selftests/arm/signal/Makefile
> new file mode 100644
> index 000000000000..3540a25de75a
> --- /dev/null
> +++ b/tools/testing/selftests/arm/signal/Makefile
> @@ -0,0 +1,30 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2024 ARM Limited
> +
> +# Additional include paths needed by kselftest.h and local headers
> +CFLAGS += -D_GNU_SOURCE -std=gnu99 -I.
> +
> +SRCS := $(filter-out testcases/testcases.c,$(wildcard testcases/*.c))
> +PROGS := $(patsubst %.c,%,$(SRCS))
> +
> +# Generated binaries to be installed by top KSFT script
> +TEST_GEN_PROGS := $(notdir $(PROGS))
> +
> +# Get Kernel headers installed and use them.
> +
> +# Including KSFT lib.mk here will also mangle the TEST_GEN_PROGS list
> +# to account for any OUTPUT target-dirs optionally provided by
> +# the toplevel makefile
> +include ../../lib.mk
> +
> +$(TEST_GEN_PROGS): $(PROGS)
> + cp $(PROGS) $(OUTPUT)/
> +
> +# Common test-unit targets to build common-layout test-cases executables
> +# Needs secondary expansion to properly include the testcase c-file in pre-reqs
> +COMMON_SOURCES := test_signals.c test_signals_utils.c
> +COMMON_HEADERS := test_signals.h test_signals_utils.h
> +
> +.SECONDEXPANSION:
> +$(PROGS): $$@.c ${COMMON_SOURCES} ${COMMON_HEADERS}
> + $(CC) $(CFLAGS) ${@}.c ${COMMON_SOURCES} -o $@
--
BR,
Muhammad Usama Anjum
_______________________________________________
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 7/8] cpuidle/poll_state: replace cpu_relax with smp_cond_load_relaxed
From: Okanovic, Haris @ 2024-04-06 18:42 UTC (permalink / raw)
To: ankur.a.arora@oracle.com
Cc: joao.m.martins@oracle.com, kvm@vger.kernel.org,
dianders@chromium.org, linux-arm-kernel@lists.infradead.org,
pmladek@suse.com, wanpengli@tencent.com,
akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
catalin.marinas@arm.com, mingo@redhat.com, pbonzini@redhat.com,
tglx@linutronix.de, daniel.lezcano@linaro.org,
mihai.carabas@oracle.com, arnd@arndb.de, will@kernel.org,
hpa@zytor.com, peterz@infradead.org, mic@digikod.net,
vkuznets@redhat.com, bp@alien8.de, npiggin@gmail.com,
linux-pm@vger.kernel.org, rafael@kernel.org,
juerg.haefliger@canonical.com, x86@kernel.org,
rick.p.edgecombe@intel.com
In-Reply-To: <87r0fjtn9y.fsf@oracle.com>
On Fri, 2024-04-05 at 16:14 -0700, Ankur Arora wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> Okanovic, Haris <harisokn@amazon.com> writes:
>
> > On Thu, 2024-02-15 at 09:41 +0200, Mihai Carabas wrote:
> > > cpu_relax on ARM64 does a simple "yield". Thus we replace it with
> > > smp_cond_load_relaxed which basically does a "wfe".
> > >
> > > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > > Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com>
> > > ---
> > > drivers/cpuidle/poll_state.c | 15 ++++++++++-----
> > > 1 file changed, 10 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
> > > index 9b6d90a72601..1e45be906e72 100644
> > > --- a/drivers/cpuidle/poll_state.c
> > > +++ b/drivers/cpuidle/poll_state.c
> > > @@ -13,6 +13,7 @@
> > > static int __cpuidle poll_idle(struct cpuidle_device *dev,
> > > struct cpuidle_driver *drv, int index)
> > > {
> > > + unsigned long ret;
> > > u64 time_start;
> > >
> > > time_start = local_clock_noinstr();
> > > @@ -26,12 +27,16 @@ static int __cpuidle poll_idle(struct cpuidle_device *dev,
> > >
> > > limit = cpuidle_poll_time(drv, dev);
> > >
> > > - while (!need_resched()) {
> > > - cpu_relax();
> > > - if (loop_count++ < POLL_IDLE_RELAX_COUNT)
> > > - continue;
> > > -
> > > + for (;;) {
> > > loop_count = 0;
> > > +
> > > + ret = smp_cond_load_relaxed(¤t_thread_info()->flags,
> > > + VAL & _TIF_NEED_RESCHED ||
> > > + loop_count++ >= POLL_IDLE_RELAX_COUNT);
> >
> > Is it necessary to repeat this 200 times with a wfe poll?
>
> The POLL_IDLE_RELAX_COUNT is there because on x86 each cpu_relax()
> iteration is much shorter.
>
> With WFE, it makes less sense.
>
> > Does kvm not implement a timeout period?
>
> Not yet, but it does become more useful after a WFE haltpoll is
> available on ARM64.
Note that kvm conditionally traps WFE and WFI based on number of host
CPU tasks. VMs will sometimes see hardware behavior - potentially
polling for a long time before entering WFI.
https://elixir.bootlin.com/linux/latest/source/arch/arm64/kvm/arm.c#L459
>
> Haltpoll does have a timeout, which you should be able to tune via
> /sys/module/haltpoll/parameters/ but that, of course, won't help here.
>
> > Could you make it configurable? This patch improves certain workloads
> > on AWS Graviton instances as well, but blocks up to 6ms in 200 * 30us
> > increments before going to wfi, which is a bit excessive.
>
> Yeah, this looks like a problem. We could solve it by making it an
> architectural parameter. Though I worry about ARM platforms with
> much smaller default timeouts.
> The other possibility is using WFET in the primitive, but then we
> have that dependency and that's a bigger change.
See arm64's delay() for inspiration:
https://elixir.bootlin.com/linux/v6.9-rc2/source/arch/arm64/lib/delay.c#L26
>
> Will address this in the next version.
>
> Thanks for pointing this out.
>
> --
> ankur
_______________________________________________
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] remoteproc: zynqmp: Add coredump support
From: Leonard Crestez @ 2024-04-06 18:28 UTC (permalink / raw)
To: Tanmay Shah, Mathieu Poirier
Cc: Bjorn Andersson, Michal Simek, linux-remoteproc, linux-arm-kernel
In-Reply-To: <6adf7a7c-c9ad-40ae-9e11-cc722c8049dd@amd.com>
On 4/4/24 23:14, Tanmay Shah wrote:
> Hello,
>
> Thanks for your patch. Patch looks good to me.
> Please find some comments below.
>
> On 3/16/24 1:16 PM, Leonard Crestez wrote:
>> Supporting remoteproc coredump requires the platform-specific driver to
>> register coredump segments to be dumped. Do this by calling
>> rproc_coredump_add_segment for every carveout.
>>
>> Also call rproc_coredump_set_elf_info when then rproc is created. If the
>> ELFCLASS parameter is not provided then coredump fails with an error.
>> Other drivers seem to pass EM_NONE for the machine argument but for me
>> this shows a warning in gdb. Pass EM_ARM because this is an ARM R5.
>>
>> Signed-off-by: Leonard Crestez <cdleonard@gmail.com>
>> ---
>>
>> Tests were done by triggering an deliberate crash using remoteproc
>> debugfs: echo 2 > /sys/kernel/debug/remoteproc/remoteproc0/crash
>>
>> The R5 cache is not flushed so RAM might be out of date which is
>> actually very bad because information most relevant to determining the
>> cause of a crash is lost. Possible workaround would be to flush caches
>> in some sort of R5 crash handler? I don't think Linux can do anything
>> about this limitation.
>>
>
> Correct Linux can't. One workaround is that R5 firmware can mark
> required memory regions as non-cachable in MPU setting. This way information
> loss can be avoided.
The solution I ended up with is to add cache flushing in some sort of R5-side
crash handler.
>> The generated coredump doesn't contain registers, this seems to be a
>> limitation shared with other rproc coredumps. It's not clear how the apu
>> could access rpu registers on zynqmp, my only idea would be to use the
>> coresight dap but that sounds difficult.
>
> Linux doesn't really have access to R5 control registers due to security.
> Instead EEMI calls to platform management controller are used to control R5.
> So R5 control register dump shouldn't needed.
>
> Mathieu,
> I am okay to merge this patch.
Thanks for the review.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH net-next v11 00/13] Introduce PHY listing and link_topology tracking
From: patchwork-bot+netdevbpf @ 2024-04-06 17:30 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, thomas.petazzoni, andrew, kuba,
edumazet, pabeni, linux, linux-arm-kernel, christophe.leroy,
herve.codina, f.fainelli, hkallweit1, vladimir.oltean,
kory.maincent, jesse.brandeburg, corbet, kabel,
piergiorgio.beruto, o.rempel, nicveronese, horms, mwojtas
In-Reply-To: <20240404093004.2552221-1-maxime.chevallier@bootlin.com>
Hello:
This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:
On Thu, 4 Apr 2024 11:29:50 +0200 you wrote:
> Hello everyone,
>
> This is V11 for the link topology addition, allowing to track all PHYs
> that are linked to netdevices.
>
> This V11 addresses the various netlink-related issues that were raised
> by Jakub, and fixes some typos in the documentation.
>
> [...]
Here is the summary with links:
- [net-next,v11,01/13] net: phy: Introduce ethernet link topology representation
https://git.kernel.org/netdev/net-next/c/6916e461e793
- [net-next,v11,02/13] net: sfp: pass the phy_device when disconnecting an sfp module's PHY
https://git.kernel.org/netdev/net-next/c/0ec5ed6c130e
- [net-next,v11,03/13] net: phy: add helpers to handle sfp phy connect/disconnect
https://git.kernel.org/netdev/net-next/c/e75e4e074c44
- [net-next,v11,04/13] net: sfp: Add helper to return the SFP bus name
https://git.kernel.org/netdev/net-next/c/fdd353965b52
- [net-next,v11,05/13] net: ethtool: Allow passing a phy index for some commands
https://git.kernel.org/netdev/net-next/c/841942bc6212
- [net-next,v11,06/13] netlink: specs: add phy-index as a header parameter
(no matching commit)
- [net-next,v11,07/13] net: ethtool: Introduce a command to list PHYs on an interface
(no matching commit)
- [net-next,v11,08/13] netlink: specs: add ethnl PHY_GET command set
(no matching commit)
- [net-next,v11,09/13] net: ethtool: plca: Target the command to the requested PHY
(no matching commit)
- [net-next,v11,10/13] net: ethtool: pse-pd: Target the command to the requested PHY
(no matching commit)
- [net-next,v11,11/13] net: ethtool: cable-test: Target the command to the requested PHY
(no matching commit)
- [net-next,v11,12/13] net: ethtool: strset: Allow querying phy stats by index
(no matching commit)
- [net-next,v11,13/13] Documentation: networking: document phy_link_topology
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] arm64: dts: rockchip: rk3588s: Fix ordering of nodes
From: Diederik de Haas @ 2024-04-06 17:28 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
Cc: devicetree, linux-arm-kernel, linux-rockchip, Diederik de Haas
Fix the ordering of the main nodes by sorting them alphabetically and
then the ones with a memory address sequentially by that address.
Signed-off-by: Diederik de Haas <didi.debian@cknow.org>
---
arch/arm64/boot/dts/rockchip/rk3588s.dtsi | 304 +++++++++++-----------
1 file changed, 152 insertions(+), 152 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
index 89d40cff635f..ac5bd630f155 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588s.dtsi
@@ -347,6 +347,11 @@ l3_cache: l3-cache {
};
};
+ display_subsystem: display-subsystem {
+ compatible = "rockchip,display-subsystem";
+ ports = <&vop_out>;
+ };
+
firmware {
optee: optee {
compatible = "linaro,optee-tz";
@@ -394,11 +399,6 @@ spll: clock-0 {
#clock-cells = <0>;
};
- display_subsystem: display-subsystem {
- compatible = "rockchip,display-subsystem";
- ports = <&vop_out>;
- };
-
timer {
compatible = "arm,armv8-timer";
interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_HIGH 0>,
@@ -436,6 +436,62 @@ scmi_shmem: sram@0 {
};
};
+ gpu: gpu@fb000000 {
+ compatible = "rockchip,rk3588-mali", "arm,mali-valhall-csf";
+ reg = <0x0 0xfb000000 0x0 0x200000>;
+ #cooling-cells = <2>;
+ assigned-clocks = <&scmi_clk SCMI_CLK_GPU>;
+ assigned-clock-rates = <200000000>;
+ clocks = <&cru CLK_GPU>, <&cru CLK_GPU_COREGROUP>,
+ <&cru CLK_GPU_STACKS>;
+ clock-names = "core", "coregroup", "stacks";
+ dynamic-power-coefficient = <2982>;
+ interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH 0>;
+ interrupt-names = "job", "mmu", "gpu";
+ operating-points-v2 = <&gpu_opp_table>;
+ power-domains = <&power RK3588_PD_GPU>;
+ status = "disabled";
+
+ gpu_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-300000000 {
+ opp-hz = /bits/ 64 <300000000>;
+ opp-microvolt = <675000 675000 850000>;
+ };
+ opp-400000000 {
+ opp-hz = /bits/ 64 <400000000>;
+ opp-microvolt = <675000 675000 850000>;
+ };
+ opp-500000000 {
+ opp-hz = /bits/ 64 <500000000>;
+ opp-microvolt = <675000 675000 850000>;
+ };
+ opp-600000000 {
+ opp-hz = /bits/ 64 <600000000>;
+ opp-microvolt = <675000 675000 850000>;
+ };
+ opp-700000000 {
+ opp-hz = /bits/ 64 <700000000>;
+ opp-microvolt = <700000 700000 850000>;
+ };
+ opp-800000000 {
+ opp-hz = /bits/ 64 <800000000>;
+ opp-microvolt = <750000 750000 850000>;
+ };
+ opp-900000000 {
+ opp-hz = /bits/ 64 <900000000>;
+ opp-microvolt = <800000 800000 850000>;
+ };
+ opp-1000000000 {
+ opp-hz = /bits/ 64 <1000000000>;
+ opp-microvolt = <850000 850000 850000>;
+ };
+ };
+ };
+
usb_host0_ehci: usb@fc800000 {
compatible = "rockchip,rk3588-ehci", "generic-ehci";
reg = <0x0 0xfc800000 0x0 0x40000>;
@@ -501,62 +557,6 @@ usb_host2_xhci: usb@fcd00000 {
status = "disabled";
};
- gpu: gpu@fb000000 {
- compatible = "rockchip,rk3588-mali", "arm,mali-valhall-csf";
- reg = <0x0 0xfb000000 0x0 0x200000>;
- #cooling-cells = <2>;
- assigned-clocks = <&scmi_clk SCMI_CLK_GPU>;
- assigned-clock-rates = <200000000>;
- clocks = <&cru CLK_GPU>, <&cru CLK_GPU_COREGROUP>,
- <&cru CLK_GPU_STACKS>;
- clock-names = "core", "coregroup", "stacks";
- dynamic-power-coefficient = <2982>;
- interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH 0>,
- <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH 0>,
- <GIC_SPI 94 IRQ_TYPE_LEVEL_HIGH 0>;
- interrupt-names = "job", "mmu", "gpu";
- operating-points-v2 = <&gpu_opp_table>;
- power-domains = <&power RK3588_PD_GPU>;
- status = "disabled";
-
- gpu_opp_table: opp-table {
- compatible = "operating-points-v2";
-
- opp-300000000 {
- opp-hz = /bits/ 64 <300000000>;
- opp-microvolt = <675000 675000 850000>;
- };
- opp-400000000 {
- opp-hz = /bits/ 64 <400000000>;
- opp-microvolt = <675000 675000 850000>;
- };
- opp-500000000 {
- opp-hz = /bits/ 64 <500000000>;
- opp-microvolt = <675000 675000 850000>;
- };
- opp-600000000 {
- opp-hz = /bits/ 64 <600000000>;
- opp-microvolt = <675000 675000 850000>;
- };
- opp-700000000 {
- opp-hz = /bits/ 64 <700000000>;
- opp-microvolt = <700000 700000 850000>;
- };
- opp-800000000 {
- opp-hz = /bits/ 64 <800000000>;
- opp-microvolt = <750000 750000 850000>;
- };
- opp-900000000 {
- opp-hz = /bits/ 64 <900000000>;
- opp-microvolt = <800000 800000 850000>;
- };
- opp-1000000000 {
- opp-hz = /bits/ 64 <1000000000>;
- opp-microvolt = <850000 850000 850000>;
- };
- };
- };
-
pmu1grf: syscon@fd58a000 {
compatible = "rockchip,rk3588-pmugrf", "syscon", "simple-mfd";
reg = <0x0 0xfd58a000 0x0 0x10000>;
@@ -702,74 +702,6 @@ i2c0: i2c@fd880000 {
status = "disabled";
};
- vop: vop@fdd90000 {
- compatible = "rockchip,rk3588-vop";
- reg = <0x0 0xfdd90000 0x0 0x4200>, <0x0 0xfdd95000 0x0 0x1000>;
- reg-names = "vop", "gamma-lut";
- interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH 0>;
- clocks = <&cru ACLK_VOP>,
- <&cru HCLK_VOP>,
- <&cru DCLK_VOP0>,
- <&cru DCLK_VOP1>,
- <&cru DCLK_VOP2>,
- <&cru DCLK_VOP3>,
- <&cru PCLK_VOP_ROOT>;
- clock-names = "aclk",
- "hclk",
- "dclk_vp0",
- "dclk_vp1",
- "dclk_vp2",
- "dclk_vp3",
- "pclk_vop";
- iommus = <&vop_mmu>;
- power-domains = <&power RK3588_PD_VOP>;
- rockchip,grf = <&sys_grf>;
- rockchip,vop-grf = <&vop_grf>;
- rockchip,vo1-grf = <&vo1_grf>;
- rockchip,pmu = <&pmu>;
- status = "disabled";
-
- vop_out: ports {
- #address-cells = <1>;
- #size-cells = <0>;
-
- vp0: port@0 {
- #address-cells = <1>;
- #size-cells = <0>;
- reg = <0>;
- };
-
- vp1: port@1 {
- #address-cells = <1>;
- #size-cells = <0>;
- reg = <1>;
- };
-
- vp2: port@2 {
- #address-cells = <1>;
- #size-cells = <0>;
- reg = <2>;
- };
-
- vp3: port@3 {
- #address-cells = <1>;
- #size-cells = <0>;
- reg = <3>;
- };
- };
- };
-
- vop_mmu: iommu@fdd97e00 {
- compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
- reg = <0x0 0xfdd97e00 0x0 0x100>, <0x0 0xfdd97f00 0x0 0x100>;
- interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH 0>;
- clocks = <&cru ACLK_VOP>, <&cru HCLK_VOP>;
- clock-names = "aclk", "iface";
- #iommu-cells = <0>;
- power-domains = <&power RK3588_PD_VOP>;
- status = "disabled";
- };
-
uart0: serial@fd890000 {
compatible = "rockchip,rk3588-uart", "snps,dw-apb-uart";
reg = <0x0 0xfd890000 0x0 0x100>;
@@ -1140,6 +1072,87 @@ power-domain@RK3588_PD_SDMMC {
};
};
+ av1d: video-codec@fdc70000 {
+ compatible = "rockchip,rk3588-av1-vpu";
+ reg = <0x0 0xfdc70000 0x0 0x800>;
+ interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH 0>;
+ interrupt-names = "vdpu";
+ assigned-clocks = <&cru ACLK_AV1>, <&cru PCLK_AV1>;
+ assigned-clock-rates = <400000000>, <400000000>;
+ clocks = <&cru ACLK_AV1>, <&cru PCLK_AV1>;
+ clock-names = "aclk", "hclk";
+ power-domains = <&power RK3588_PD_AV1>;
+ resets = <&cru SRST_A_AV1>, <&cru SRST_P_AV1>, <&cru SRST_A_AV1_BIU>, <&cru SRST_P_AV1_BIU>;
+ };
+
+ vop: vop@fdd90000 {
+ compatible = "rockchip,rk3588-vop";
+ reg = <0x0 0xfdd90000 0x0 0x4200>, <0x0 0xfdd95000 0x0 0x1000>;
+ reg-names = "vop", "gamma-lut";
+ interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru ACLK_VOP>,
+ <&cru HCLK_VOP>,
+ <&cru DCLK_VOP0>,
+ <&cru DCLK_VOP1>,
+ <&cru DCLK_VOP2>,
+ <&cru DCLK_VOP3>,
+ <&cru PCLK_VOP_ROOT>;
+ clock-names = "aclk",
+ "hclk",
+ "dclk_vp0",
+ "dclk_vp1",
+ "dclk_vp2",
+ "dclk_vp3",
+ "pclk_vop";
+ iommus = <&vop_mmu>;
+ power-domains = <&power RK3588_PD_VOP>;
+ rockchip,grf = <&sys_grf>;
+ rockchip,vop-grf = <&vop_grf>;
+ rockchip,vo1-grf = <&vo1_grf>;
+ rockchip,pmu = <&pmu>;
+ status = "disabled";
+
+ vop_out: ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ vp0: port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ };
+
+ vp1: port@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ };
+
+ vp2: port@2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <2>;
+ };
+
+ vp3: port@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+ };
+ };
+ };
+
+ vop_mmu: iommu@fdd97e00 {
+ compatible = "rockchip,rk3588-iommu", "rockchip,rk3568-iommu";
+ reg = <0x0 0xfdd97e00 0x0 0x100>, <0x0 0xfdd97f00 0x0 0x100>;
+ interrupts = <GIC_SPI 156 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru ACLK_VOP>, <&cru HCLK_VOP>;
+ clock-names = "aclk", "iface";
+ #iommu-cells = <0>;
+ power-domains = <&power RK3588_PD_VOP>;
+ status = "disabled";
+ };
+
i2s4_8ch: i2s@fddc0000 {
compatible = "rockchip,rk3588-i2s-tdm";
reg = <0x0 0xfddc0000 0x0 0x1000>;
@@ -1431,6 +1444,16 @@ qos_vop_m1: qos@fdf82200 {
reg = <0x0 0xfdf82200 0x0 0x20>;
};
+ dfi: dfi@fe060000 {
+ reg = <0x00 0xfe060000 0x00 0x10000>;
+ compatible = "rockchip,rk3588-dfi";
+ interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH 0>,
+ <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH 0>;
+ rockchip,pmu = <&pmu1grf>;
+ };
+
pcie2x1l1: pcie@fe180000 {
compatible = "rockchip,rk3588-pcie", "rockchip,rk3568-pcie";
bus-range = <0x30 0x3f>;
@@ -1533,16 +1556,6 @@ pcie2x1l2_intc: legacy-interrupt-controller {
};
};
- dfi: dfi@fe060000 {
- reg = <0x00 0xfe060000 0x00 0x10000>;
- compatible = "rockchip,rk3588-dfi";
- interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH 0>,
- <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH 0>,
- <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH 0>,
- <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH 0>;
- rockchip,pmu = <&pmu1grf>;
- };
-
gmac1: ethernet@fe1c0000 {
compatible = "rockchip,rk3588-gmac", "snps,dwmac-4.20a";
reg = <0x0 0xfe1c0000 0x0 0x10000>;
@@ -2543,19 +2556,6 @@ gpio4: gpio@fec50000 {
#interrupt-cells = <2>;
};
};
-
- av1d: video-codec@fdc70000 {
- compatible = "rockchip,rk3588-av1-vpu";
- reg = <0x0 0xfdc70000 0x0 0x800>;
- interrupts = <GIC_SPI 108 IRQ_TYPE_LEVEL_HIGH 0>;
- interrupt-names = "vdpu";
- assigned-clocks = <&cru ACLK_AV1>, <&cru PCLK_AV1>;
- assigned-clock-rates = <400000000>, <400000000>;
- clocks = <&cru ACLK_AV1>, <&cru PCLK_AV1>;
- clock-names = "aclk", "hclk";
- power-domains = <&power RK3588_PD_AV1>;
- resets = <&cru SRST_A_AV1>, <&cru SRST_P_AV1>, <&cru SRST_A_AV1_BIU>, <&cru SRST_P_AV1_BIU>;
- };
};
#include "rk3588s-pinctrl.dtsi"
base-commit: c1b1f340dd7db11f273e426e110697551c9f501f
--
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 related
* Re: [PATCH v5 02/10] dt-bindings: mailbox: Add mboxes property for CMDQ secure driver
From: Jason-JH Lin (林睿祥) @ 2024-04-06 16:15 UTC (permalink / raw)
To: conor@kernel.org
Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
Houlong Wei (魏厚龙),
devicetree@vger.kernel.org, Shawn Sung (宋孝謙),
CK Hu (胡俊光), conor+dt@kernel.org,
robh@kernel.org, linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
jassisinghbrar@gmail.com, angelogioacchino.delregno@collabora.com
In-Reply-To: <20240405-remindful-galley-2dee9eec4f34@spud>
On Fri, 2024-04-05 at 17:13 +0100, Conor Dooley wrote:
> On Fri, Apr 05, 2024 at 02:33:14PM +0000, Jason-JH Lin (林睿祥) wrote:
> > On Thu, 2024-04-04 at 15:52 +0100, Conor Dooley wrote:
> > > On Thu, Apr 04, 2024 at 04:31:06AM +0000, Jason-JH Lin (林睿祥)
> > > wrote:
> > > > Hi Conor,
> > > >
> > > > Thanks for the reviews.
> > > >
> > > > On Wed, 2024-04-03 at 16:46 +0100, Conor Dooley wrote:
> > > > > On Wed, Apr 03, 2024 at 06:25:54PM +0800, Shawn Sung wrote:
> > > > > > From: "Jason-JH.Lin" <jason-jh.lin@mediatek.com>
> > > > > >
> > > > > > Add mboxes to define a GCE loopping thread as a secure irq
> > > > > > handler.
> > > > > > This property is only required if CMDQ secure driver is
> > > > > > supported.
> > > > > >
> > > > > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > > > > > Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> > > > > > ---
> > > > > > .../bindings/mailbox/mediatek,gce-mailbox.yaml |
> > > > > > 10
> > > > > > ++++++++++
> > > > > > 1 file changed, 10 insertions(+)
> > > > > >
> > > > > > diff --git
> > > > > > a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > > mailbox.yaml
> > > > > > b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > > mailbox.yaml
> > > > > > index cef9d76013985..c0d80cc770899 100644
> > > > > > ---
> > > > > > a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > > mailbox.yaml
> > > > > > +++
> > > > > > b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> > > > > > mailbox.yaml
> > > > > > @@ -49,6 +49,16 @@ properties:
> > > > > > items:
> > > > > > - const: gce
> > > > > >
> > > > > > + mediatek,gce-events:
> > > > > > + description:
> > > > > > + The event id which is mapping to the specific
> > > > > > hardware
> > > > > > event
> > > > > > signal
> > > > > > + to gce. The event id is defined in the gce header
> > > > > > + include/dt-bindings/gce/<chip>-gce.h of each chips.
> > > > >
> > > > > Missing any info here about when this should be used, hint -
> > > > > you
> > > > > have
> > > > > it
> > > > > in the commit message.
> > > > >
> > > > > > + $ref: /schemas/types.yaml#/definitions/uint32-arrayi
> > > > >
> > > > > Why is the ID used by the CMDQ service not fixed for each
> > > > > SoC?
> > > > >
> > > >
> > > > I forgot to sync with Shawn about this:
> > > > https://lore.kernel.org/all/20240124011459.12204-1-jason-
> > > > jh.lin@mediatek.com
> > > >
> > > > I'll fix it at the next version.
> > >
> > > When I say "fixed" I don't mean "this is wrong, please fix it", I
> > > mean
> > > "why is the value not static for a particular SoC". This needs to
> > > be
> > > explained in the patch (and the description for the event here
> > > needs
> > > to
> > > explain what the gce-mailbox is reserving an event for).
> > >
> >
> > Oh, I see. Thanks for noticing me.
> >
> > We do want to reserve a static event ID for gce-mailbox to
> > different
> > SoCs. There are 2 mainly reasons to why we set it in DTS:
> > 1. There are 1024 events IDs for GCE to use to execute instructions
> > in
> > the specific event happened. These events could be signaled by HW
> > or SW
> > and their value would be different in different SoC because of HW
> > event
> > IDs distribution range from 0 to 1023.
> > If we set a static event ID: 855 for mt8188, it might be conflict
> > the
> > event ID original set in mt8195.
>
> That's not a problem, we have compatibles for this purpose.
I agree that compatibles can do the same things.
>
> > 2. If we defined the event ID in DTS, we might know how many SW or
> > HW
> > event IDs are used.
> > If someone wants to use a new event ID for a new feature, they
> > could
> > find out the used event IDs in DTS easily and avoid the event ID
> > conflicting.
>
> Are the event IDs not documented in the reference manual for the SoC
> in
> question? Or in documentation for the secure world for these devices?
> A
> DTS should not be the authoritive source for this information for
> developers.
>
The event IDs were defined in:
inculde/dt-bindings/mailbox/mediatek,mt8188-gce.h.
> Additionally, the driver could very easily detect if someone does
> happen
> to put in the reserved ID. That could be generically useful (IOW,
> check
> all of them for re-use) if the ID are to not allowed to be shared.
>
> > The reason why we define a event ID is we want to get a SW signal
> > from
> > secure world. We design a GCE looping thread in gce-mailbox driver
> > to
> > wait for the GCE execute done event for each cmdq secure packets
> > from
> > secure world.
>
> This sort of information needs to be in the commit message, but I
> don't
> think this property is needed at all since it seems to be something
> detectable from the compatible.
I think put this event ID in driver data and distinguish them by
different compatibles can achieve the same thing.
However, I originally thought that align to the existing way like
MUTEX, CCORR, WDMA in
https://lore.kernel.org/all/20240124011459.12204-4-jason-jh.lin@mediatek.com
would be better choice.
I think their usage of gce-events are the same.
What do you think?
Regards,
Jason-JH.Lin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH net-next v11 08/13] netlink: specs: add ethnl PHY_GET command set
From: Andrew Lunn @ 2024-04-06 16:14 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel,
Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit,
Vladimir Oltean, Köry Maincent, Jesse Brandeburg,
Jonathan Corbet, Marek Behún, Piergiorgio Beruto,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas
In-Reply-To: <20240404093004.2552221-9-maxime.chevallier@bootlin.com>
On Thu, Apr 04, 2024 at 11:29:58AM +0200, Maxime Chevallier wrote:
> The PHY_GET command, supporting both DUMP and GET operations, is used to
> retrieve the list of PHYs connected to a netdevice, and get topology
> information to know where exactly it sits on the physical link.
>
> Add the netlink specs corresponding to that command.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
This i don't feel qualified to review this, other than i don't see any
obvious spelling mistakes etc.
Is there a tool to sanity check/validate these spec files? If so,
could it be added to NIPA? Added to Simon's list of checks?
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 net-next v11 13/13] Documentation: networking: document phy_link_topology
From: Andrew Lunn @ 2024-04-06 16:07 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel,
Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit,
Vladimir Oltean, Köry Maincent, Jesse Brandeburg,
Jonathan Corbet, Marek Behún, Piergiorgio Beruto,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas
In-Reply-To: <20240404093004.2552221-14-maxime.chevallier@bootlin.com>
On Thu, Apr 04, 2024 at 11:30:03AM +0200, Maxime Chevallier wrote:
> The newly introduced phy_link_topology tracks all ethernet PHYs that are
> attached to a netdevice. Document the base principle, internal and
> external APIs. As the phy_link_topology is expected to be extended, this
> documentation will hold any further improvements and additions made
> relative to topology handling.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
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 net-next v11 12/13] net: ethtool: strset: Allow querying phy stats by index
From: Andrew Lunn @ 2024-04-06 16:01 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel,
Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit,
Vladimir Oltean, Köry Maincent, Jesse Brandeburg,
Jonathan Corbet, Marek Behún, Piergiorgio Beruto,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas
In-Reply-To: <20240404093004.2552221-13-maxime.chevallier@bootlin.com>
On Thu, Apr 04, 2024 at 11:30:02AM +0200, Maxime Chevallier wrote:
> The ETH_SS_PHY_STATS command gets PHY statistics. Use the phydev pointer
> from the ethnl request to allow query phy stats from each PHY on the
> link.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
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 net-next v11 11/13] net: ethtool: cable-test: Target the command to the requested PHY
From: Andrew Lunn @ 2024-04-06 16:00 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel,
Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit,
Vladimir Oltean, Köry Maincent, Jesse Brandeburg,
Jonathan Corbet, Marek Behún, Piergiorgio Beruto,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas
In-Reply-To: <20240404093004.2552221-12-maxime.chevallier@bootlin.com>
On Thu, Apr 04, 2024 at 11:30:01AM +0200, Maxime Chevallier wrote:
> Cable testing is a PHY-specific command. Instead of targeting the command
> towards dev->phydev, use the request to pick the targeted PHY.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
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 net-next v11 10/13] net: ethtool: pse-pd: Target the command to the requested PHY
From: Andrew Lunn @ 2024-04-06 16:00 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel,
Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit,
Vladimir Oltean, Köry Maincent, Jesse Brandeburg,
Jonathan Corbet, Marek Behún, Piergiorgio Beruto,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas
In-Reply-To: <20240404093004.2552221-11-maxime.chevallier@bootlin.com>
On Thu, Apr 04, 2024 at 11:30:00AM +0200, Maxime Chevallier wrote:
> PSE and PD configuration is a PHY-specific command. Instead of targeting
> the command towards dev->phydev, use the request to pick the targeted
> PHY device.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
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 net-next v11 09/13] net: ethtool: plca: Target the command to the requested PHY
From: Andrew Lunn @ 2024-04-06 16:00 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel,
Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit,
Vladimir Oltean, Köry Maincent, Jesse Brandeburg,
Jonathan Corbet, Marek Behún, Piergiorgio Beruto,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas
In-Reply-To: <20240404093004.2552221-10-maxime.chevallier@bootlin.com>
On Thu, Apr 04, 2024 at 11:29:59AM +0200, Maxime Chevallier wrote:
> PLCA is a PHY-specific command. Instead of targeting the command
> towards dev->phydev, use the request to pick the targeted PHY.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
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 net-next v11 07/13] net: ethtool: Introduce a command to list PHYs on an interface
From: Andrew Lunn @ 2024-04-06 15:58 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel,
Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit,
Vladimir Oltean, Köry Maincent, Jesse Brandeburg,
Jonathan Corbet, Marek Behún, Piergiorgio Beruto,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas
In-Reply-To: <20240404093004.2552221-8-maxime.chevallier@bootlin.com>
On Thu, Apr 04, 2024 at 11:29:57AM +0200, Maxime Chevallier wrote:
> As we have the ability to track the PHYs connected to a net_device
> through the link_topology, we can expose this list to userspace. This
> allows userspace to use these identifiers for phy-specific commands and
> take the decision of which PHY to target by knowing the link topology.
>
> Add PHY_GET and PHY_DUMP, which can be a filtered DUMP operation to list
> devices on only one interface.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
It would be good if Jakub reviewed this as well, since i don't know
netlink too well. But:
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
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 net-next v11 03/13] net: phy: add helpers to handle sfp phy connect/disconnect
From: Andrew Lunn @ 2024-04-06 15:52 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel,
Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit,
Vladimir Oltean, Köry Maincent, Jesse Brandeburg,
Jonathan Corbet, Marek Behún, Piergiorgio Beruto,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas
In-Reply-To: <20240404093004.2552221-4-maxime.chevallier@bootlin.com>
On Thu, Apr 04, 2024 at 11:29:53AM +0200, Maxime Chevallier wrote:
> There are a few PHY drivers that can handle SFP modules through their
> sfp_upstream_ops. Introduce Phylib helpers to keep track of connected
> SFP PHYs in a netdevice's namespace, by adding the SFP PHY to the
> upstream PHY's netdev's namespace.
>
> By doing so, these SFP PHYs can be enumerated and exposed to users,
> which will be able to use their capabilities.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
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 net-next v11 02/13] net: sfp: pass the phy_device when disconnecting an sfp module's PHY
From: Andrew Lunn @ 2024-04-06 15:51 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel,
Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit,
Vladimir Oltean, Köry Maincent, Jesse Brandeburg,
Jonathan Corbet, Marek Behún, Piergiorgio Beruto,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas
In-Reply-To: <20240404093004.2552221-3-maxime.chevallier@bootlin.com>
On Thu, Apr 04, 2024 at 11:29:52AM +0200, Maxime Chevallier wrote:
> Pass the phy_device as a parameter to the sfp upstream .disconnect_phy
> operation. This is preparatory work to help track phy devices across
> a net_device's link.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
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 net-next v11 01/13] net: phy: Introduce ethernet link topology representation
From: Andrew Lunn @ 2024-04-06 15:51 UTC (permalink / raw)
To: Maxime Chevallier
Cc: davem, netdev, linux-kernel, thomas.petazzoni, Jakub Kicinski,
Eric Dumazet, Paolo Abeni, Russell King, linux-arm-kernel,
Christophe Leroy, Herve Codina, Florian Fainelli, Heiner Kallweit,
Vladimir Oltean, Köry Maincent, Jesse Brandeburg,
Jonathan Corbet, Marek Behún, Piergiorgio Beruto,
Oleksij Rempel, Nicolò Veronese, Simon Horman, mwojtas
In-Reply-To: <20240404093004.2552221-2-maxime.chevallier@bootlin.com>
On Thu, Apr 04, 2024 at 11:29:51AM +0200, Maxime Chevallier wrote:
> Link topologies containing multiple network PHYs attached to the same
> net_device can be found when using a PHY as a media converter for use
> with an SFP connector, on which an SFP transceiver containing a PHY can
> be used.
>
> With the current model, the transceiver's PHY can't be used for
> operations such as cable testing, timestamping, macsec offload, etc.
>
> The reason being that most of the logic for these configuration, coming
> from either ethtool netlink or ioctls tend to use netdev->phydev, which
> in multi-phy systems will reference the PHY closest to the MAC.
>
> Introduce a numbering scheme allowing to enumerate PHY devices that
> belong to any netdev, which can in turn allow userspace to take more
> precise decisions with regard to each PHY's configuration.
>
> The numbering is maintained per-netdev, in a phy_device_list.
> The numbering works similarly to a netdevice's ifindex, with
> identifiers that are only recycled once INT_MAX has been reached.
>
> This prevents races that could occur between PHY listing and SFP
> transceiver removal/insertion.
>
> The identifiers are assigned at phy_attach time, as the numbering
> depends on the netdevice the phy is attached to. The PHY index can be
> re-used for PHYs that are persistent.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
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 1/4] dt-bindings: clock: airoha: add EN7581 binding
From: Krzysztof Kozlowski @ 2024-04-06 11:50 UTC (permalink / raw)
To: Lorenzo Bianconi, linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <99734deb28889e685a764da94418f68b55ee3bdc.1712399981.git.lorenzo@kernel.org>
On 06/04/2024 12:43, Lorenzo Bianconi wrote:
> Introduce Airoha EN7581 entry in Airoha EN7523 clock binding
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> .../bindings/clock/airoha,en7523-scu.yaml | 31 +++++++++++++++++--
> 1 file changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> index 79b0752faa91..3f4266637733 100644
> --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> @@ -29,10 +29,13 @@ description: |
> properties:
> compatible:
> items:
> - - const: airoha,en7523-scu
> + - enum:
> + - airoha,en7523-scu
> + - airoha,en7581-scu
>
> reg:
> - maxItems: 2
> + minItems: 2
> + maxItems: 3
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.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 v2 2/6] dt-bindings: firmware: add i.MX SCMI Extension protocol
From: Krzysztof Kozlowski @ 2024-04-06 11:02 UTC (permalink / raw)
To: Peng Fan (OSS), Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-2-9fc9186856c2@nxp.com>
On 05/04/2024 14:39, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> Add i.MX SCMI Extension protocols bindings for:
> - Battery Backed Secure Module(BBSM)
Which is what?
> - MISC settings such as General Purpose Registers settings.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> .../devicetree/bindings/firmware/imx,scmi.yaml | 80 ++++++++++++++++++++++
> 1 file changed, 80 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/firmware/imx,scmi.yaml b/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
> new file mode 100644
> index 000000000000..7ee19a661d83
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/imx,scmi.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2024 NXP
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/imx,scmi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: i.MX System Control and Management Interface(SCMI) Vendor Protocols Extension
> +
> +maintainers:
> + - Peng Fan <peng.fan@nxp.com>
> +
> +allOf:
> + - $ref: arm,scmi.yaml#
Sorry, but arm,scmi is a final schema. Is your plan to define some
common part?
> +
> +properties:
> + protocol@81:
> + $ref: 'arm,scmi.yaml#/$defs/protocol-node'
> + unevaluatedProperties: false
> + description:
> + The BBM Protocol is for managing Battery Backed Secure Module (BBSM) RTC
> + and the ON/OFF Key
> +
> + properties:
> + reg:
> + const: 0x81
> +
> + required:
> + - reg
> +
> + protocol@84:
> + $ref: 'arm,scmi.yaml#/$defs/protocol-node'
> + unevaluatedProperties: false
> + description:
> + The MISC Protocol is for managing SoC Misc settings, such as GPR settings
Genera register is not a setting... this is a pleonasm. Please be more
specific what is the GPR, MISC protocol etc.
> +
> + properties:
> + reg:
> + const: 0x84
> +
> + wakeup-sources:
> + description:
> + Each entry consists of 2 integers, represents the source and electric signal edge
Can you answer questions from reviewers?
> + items:
> + items:
> + - description: the wakeup source
> + - description: the wakeup electric signal edge
> + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +
> + required:
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + firmware {
> + scmi {
> + compatible = "arm,scmi";
> + mboxes = <&mu2 5 0>, <&mu2 3 0>, <&mu2 3 1>;
> + shmem = <&scmi_buf0>, <&scmi_buf1>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + protocol@81 {
> + reg = <0x81>;
> + };
> +
> + protocol@84 {
> + reg = <0x84>;
> + wakeup-sources = <0x8000 1
> + 0x8001 1
> + 0x8002 1
> + 0x8003 1
> + 0x8004 1>;
Nothing improved... If you are going to ignore reviews, then you will
only get NAKed.
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 v2 1/6] dt-bindings: firmware: arm,scmi: set additionalProperties to true
From: Krzysztof Kozlowski @ 2024-04-06 10:57 UTC (permalink / raw)
To: Peng Fan (OSS), Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Sudeep Holla, Cristian Marussi
Cc: Peng Fan, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240405-imx95-bbm-misc-v2-v2-1-9fc9186856c2@nxp.com>
On 05/04/2024 14:39, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
>
> When adding vendor extension protocols, there is dt-schema warning:
> "
> imx,scmi.example.dtb: scmi: 'protocol@81', 'protocol@84' do not match any
> of the regexes: 'pinctrl-[0-9]+'
> "
>
> Set additionalProperties to true to address the issue.
I do not see anything addressed here, except making the binding
accepting anything anywhere...
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
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