| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <35ae8415-9308-4cbc-b14e-c3cdc0a2318a@oss.qualcomm.com>
Date: Mon, 27 Oct 2025 13:17:21 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Jishnu Prakash <jishnu.prakash@....qualcomm.com>,
Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>,
David Collins <david.collins@....qualcomm.com>
Cc: linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, aiqun.yu@....qualcomm.com,
kamal.wadhwa@....qualcomm.com, jingyi.wang@....qualcomm.com
Subject: Re: [PATCH v3 3/3] spmi: spmi-pmic-arb: add support for PMIC arbiter
v8
On 10/24/25 11:33 AM, Jishnu Prakash wrote:
> From: David Collins <david.collins@....qualcomm.com>
>
> PMIC arbiter v8 supports up to 4 SPMI buses and up to 8192 PMIC
> peripherals. Its register map differs from v7 as several fields
> increased in size. Add support for PMIC arbiter version 8.
>
> Signed-off-by: David Collins <david.collins@....qualcomm.com>
> Signed-off-by: Kamal Wadhwa <kamal.wadhwa@....qualcomm.com>
> Signed-off-by: Jishnu Prakash <jishnu.prakash@....qualcomm.com>
> ---
> drivers/spmi/spmi-pmic-arb.c | 324 +++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 294 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> index 91581974ef84..612736973e4b 100644
> --- a/drivers/spmi/spmi-pmic-arb.c
> +++ b/drivers/spmi/spmi-pmic-arb.c
> @@ -3,6 +3,7 @@
> * Copyright (c) 2012-2015, 2017, 2021, The Linux Foundation. All rights reserved.
> */
> #include <linux/bitmap.h>
> +#include <linux/bitfield.h>
bit'f'ield < bit'm'ap
[...]
> #define spec_to_hwirq(slave_id, periph_id, irq_id, apid) \
> - ((((slave_id) & 0xF) << 28) | \
> - (((periph_id) & 0xFF) << 20) | \
> - (((irq_id) & 0x7) << 16) | \
> - (((apid) & 0x3FF) << 0))
> + (FIELD_PREP(GENMASK(28, 24), (slave_id)) | \
> + FIELD_PREP(GENMASK(23, 16), (periph_id)) | \
> + FIELD_PREP(GENMASK(15, 13), (irq_id)) | \
> + FIELD_PREP(GENMASK(12, 0), (apid)))
I think this could be further improved by:
#define SOMETHING_SLAVE_ID_SOMETHING GENMASK(28, 24)
and then below:
[...]
> - if (intspec[0] > 0xF || intspec[1] > 0xFF || intspec[2] > 0x7)
> + if (intspec[0] > 0x1F || intspec[1] > 0xFF || intspec[2] > 0x7)
> return -EINVAL;
we can use FIELD_MAX(SOMETHING...)
[...]
> +static int pmic_arb_get_core_resources_v8(struct platform_device *pdev,
> + void __iomem *core)
> +{
> + struct spmi_pmic_arb *pmic_arb = platform_get_drvdata(pdev);
> +
> + pmic_arb->apid_map = devm_platform_ioremap_resource_byname(pdev,
> + "chnl_map");
Feel free to unwrap this line
> + if (IS_ERR(pmic_arb->apid_map))
> + return PTR_ERR(pmic_arb->apid_map);
> +
> + pmic_arb->core = core;
> +
> + pmic_arb->max_periphs = PMIC_ARB_MAX_PERIPHS_V8;
> +
> + return pmic_arb_get_obsrvr_chnls_v2(pdev);
> +}
> +
> +static int pmic_arb_get_bus_resources_v8(struct platform_device *pdev,
> + struct device_node *node,
> + struct spmi_pmic_arb_bus *bus)
> +{
> + int index;
> +
> + index = of_property_match_string(node, "reg-names", "chnl_owner");
> + if (index < 0) {
> + dev_err(&pdev->dev, "chnl_owner reg region missing\n");
> + return -EINVAL;
> + }
> +
> + bus->apid_owner = devm_of_iomap(&pdev->dev, node, index, NULL);
> +
> + return PTR_ERR_OR_ZERO(bus->apid_owner);
Is this any different chan using devm_platform_ioremap_resource_byname()
like you did above?
> +}
> +
> +static int pmic_arb_read_apid_map_v8(struct spmi_pmic_arb_bus *bus)
> +{
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> + struct apid_data *apidd;
> + struct apid_data *prev_apidd;
> + u16 i, apid, ppid, apid_max;
> + bool valid, is_irq_ee;
> + u32 regval, offset;
> +
> + /*
> + * In order to allow multiple EEs to write to a single PPID in arbiter
> + * version 8, there can be more than one APID mapped to each PPID. The
> + * owner field for each of these mappings specifies the EE which is
> + * allowed to write to the APID. The owner of the last (highest) APID
> + * which has the IRQ owner bit set for a given PPID will receive
> + * interrupts from the PPID.
> + *
> + * In arbiter version 8, the APID numbering space is divided between
> + * the SPMI buses according to this mapping:
> + * APID = 0 to N-1 --> bus 0
> + * APID = N to N+M-1 --> bus 1
> + * APID = N+M to N+M+P-1 --> bus 2
> + * APID = N+M+P to N+M+P+Q-1 --> bus 3
> + * where N = number of APIDs supported by bus 0
> + * M = number of APIDs supported by bus 1
> + * P = number of APIDs supported by bus 2
> + * Q = number of APIDs supported by bus 3
> + */
> + apidd = &bus->apid_data[bus->base_apid];
> + apid_max = bus->base_apid + bus->apid_count;
> + for (i = bus->base_apid; i < apid_max; i++, apidd++) {
> + offset = pmic_arb->ver_ops->apid_map_offset(i);
> + regval = readl_relaxed(pmic_arb->apid_map + offset);
> + if (!regval)
> + continue;
> + ppid = regval & PMIC_ARB_V8_PPID_MASK;
> + is_irq_ee = PMIC_ARB_V8_CHAN_IS_IRQ_OWNER(regval);
[...]
If you parametrize the masks, the diff against pmic_arb_read_apid_map_v5
is 3 lines of code instead
> +
> + return 0;
> +}
> +
> +static int pmic_arb_init_apid_v8(struct spmi_pmic_arb_bus *bus, int index)
> +{
> + struct spmi_pmic_arb *pmic_arb = bus->pmic_arb;
> + int ret, i;
> +
> + if (index < 0 || index >= PMIC_ARB_MAX_BUSES_V8) {
> + dev_err(&bus->spmic->dev, "Unsupported bus index %d detected\n",
> + index);
> + return -EINVAL;
> + }
> +
> + bus->base_apid = 0;
> + bus->apid_count = 0;
> + for (i = 0; i <= index; i++) {
> + bus->base_apid += bus->apid_count;
> + bus->apid_count = readl_relaxed(pmic_arb->core + PMIC_ARB_FEATURES + i * 4) &
> + PMIC_ARB_FEATURES_V8_PERIPH_MASK;
You can almost replace pmic_arb_init_apid_v7 with this impl
[...]
> +static void __iomem *
> +pmic_arb_apid_owner_v8(struct spmi_pmic_arb_bus *bus, u16 n)
> +{
> + return bus->apid_owner + 0x4 * (n - bus->base_apid);
> +}
This is identical to pmic_arb_apid_owner_v7
Konrad
Powered by blists - more mailing lists