| 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: <37f73812-be0c-43b2-a5bc-a41e09db85ea@ti.com>
Date: Thu, 7 Aug 2025 12:24:29 -0500
From: Andrew Davis <afd@...com>
To: Judith Mendez <jm@...com>, Nishanth Menon <nm@...com>,
Tero Kristo
<kristo@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski
<krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Adrian Hunter
<adrian.hunter@...el.com>,
Ulf Hansson <ulf.hansson@...aro.org>
CC: Vignesh Raghavendra <vigneshr@...com>,
Santosh Shilimkar
<ssantosh@...nel.org>,
<linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-mmc@...r.kernel.org>
Subject: Re: [PATCH 2/4] soc: ti: k3-socinfo: Add support for AM62P variants
On 8/5/25 6:49 PM, Judith Mendez wrote:
> This adds a support for detecting AM62P SR1.0, SR1.1, SR1.2.
>
> On AM62P, silicon revision is discovered with GP_SW1 instead of JTAGID
> register, so introduce GP_SW register range to determine SoC revision.
>
> Signed-off-by: Judith Mendez <jm@...com>
> ---
> drivers/soc/ti/k3-socinfo.c | 82 +++++++++++++++++++++++++++++++++----
> 1 file changed, 74 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/soc/ti/k3-socinfo.c b/drivers/soc/ti/k3-socinfo.c
> index d716be113c84..9daeced656d6 100644
> --- a/drivers/soc/ti/k3-socinfo.c
> +++ b/drivers/soc/ti/k3-socinfo.c
> @@ -15,6 +15,7 @@
> #include <linux/sys_soc.h>
>
> #define CTRLMMR_WKUP_JTAGID_REG 0
> +#define CTRLMMR_WKUP_GP_SW1_REG 4
> /*
> * Bits:
> * 31-28 VARIANT Device variant
> @@ -62,10 +63,63 @@ static const struct k3_soc_id {
> { JTAG_ID_PARTNO_AM62LX, "AM62LX" },
> };
>
> +static const struct regmap_config k3_chipinfo_regmap_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> +};
> +
> static const char * const j721e_rev_string_map[] = {
> "1.0", "1.1", "2.0",
> };
>
> +static const char * const am62p_gpsw_rev_string_map[] = {
> + "1.0", "1.1", "1.2",
> +};
> +
> +static int
> +k3_chipinfo_get_variant_alternate(struct platform_device *pdev, unsigned int partno, u32 *variant)
> +{
> + struct device *dev = &pdev->dev;
> + struct regmap *regmap;
> + void __iomem *base;
> + u32 offset;
> + int ret;
> +
> + base = devm_platform_ioremap_resource(pdev, 1);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + regmap = regmap_init_mmio(dev, base, &k3_chipinfo_regmap_cfg);
devm_regmap_init_mmio()
Otherwise this regmap never gets freed up in the error paths.
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + switch (partno) {
> + case JTAG_ID_PARTNO_AM62PX:
> + offset = CTRLMMR_WKUP_GP_SW1_REG;
> + break;
> + default:
> + offset = CTRLMMR_WKUP_GP_SW1_REG;
This switch case does nothing, if offset ever can get any other
value, then you can add it back.
> + }
> +
> + ret = regmap_read(regmap, offset, variant);
> +
Extra newline
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +static bool k3_chipinfo_variant_in_gp_sw(unsigned int partno)
> +{
> + switch (partno) {
> + case JTAG_ID_PARTNO_AM62PX:
Do you really need a function just to hold a switch-case? You also
have a switch-case that checks for PARTNO_AM62PX in the below function
that makes use of the extra "gp_sw1", why not inside that case fetch
the value. Would avoid passing in a variable "gp_sw1" which is unused
in all but the PARTNO_AM62PX case.
Andrew
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> static int
> k3_chipinfo_partno_to_names(unsigned int partno,
> struct soc_device_attribute *soc_dev_attr)
> @@ -83,8 +137,10 @@ k3_chipinfo_partno_to_names(unsigned int partno,
>
> static int
> k3_chipinfo_variant_to_sr(unsigned int partno, unsigned int variant,
> - struct soc_device_attribute *soc_dev_attr)
> + struct soc_device_attribute *soc_dev_attr, u32 gp_sw1)
> {
> + u32 gpsw_variant = gp_sw1 % 16;
> +
> switch (partno) {
> case JTAG_ID_PARTNO_J721E:
> if (variant >= ARRAY_SIZE(j721e_rev_string_map))
> @@ -92,6 +148,13 @@ k3_chipinfo_variant_to_sr(unsigned int partno, unsigned int variant,
> soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
> j721e_rev_string_map[variant]);
> break;
> + case JTAG_ID_PARTNO_AM62PX:
> + /* Always parse AM62P variant from GP_SW1 */
> + if (gpsw_variant >= ARRAY_SIZE(am62p_gpsw_rev_string_map))
> + goto err_unknown_variant;
> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%s",
> + am62p_gpsw_rev_string_map[gpsw_variant]);
> + break;
> default:
> variant++;
> soc_dev_attr->revision = kasprintf(GFP_KERNEL, "SR%x.0",
> @@ -107,12 +170,6 @@ k3_chipinfo_variant_to_sr(unsigned int partno, unsigned int variant,
> return -ENODEV;
> }
>
> -static const struct regmap_config k3_chipinfo_regmap_cfg = {
> - .reg_bits = 32,
> - .val_bits = 32,
> - .reg_stride = 4,
> -};
> -
> static int k3_chipinfo_probe(struct platform_device *pdev)
> {
> struct device_node *node = pdev->dev.of_node;
> @@ -121,6 +178,7 @@ static int k3_chipinfo_probe(struct platform_device *pdev)
> struct soc_device *soc_dev;
> struct regmap *regmap;
> void __iomem *base;
> + u32 gp_sw1_val = 0;
> u32 partno_id;
> u32 variant;
> u32 jtag_id;
> @@ -163,7 +221,15 @@ static int k3_chipinfo_probe(struct platform_device *pdev)
> goto err;
> }
>
> - ret = k3_chipinfo_variant_to_sr(partno_id, variant, soc_dev_attr);
> + if (k3_chipinfo_variant_in_gp_sw(partno_id)) {
> + ret = k3_chipinfo_get_variant_alternate(pdev, partno_id, &gp_sw1_val);
> + if (ret < 0) {
> + dev_err(dev, "Failed to read GP_SW1: %d\n", ret);
> + goto err;
> + }
> + }
> +
> + ret = k3_chipinfo_variant_to_sr(partno_id, variant, soc_dev_attr, gp_sw1_val);
> if (ret) {
> dev_err(dev, "Unknown SoC SR[0x%08X]: %d\n", jtag_id, ret);
> goto err;
Powered by blists - more mailing lists