From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DC7B847F3D6 for ; Thu, 15 Oct 2026 06:29:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1792045762; cv=none; b=UwvNj7hF5NDTdvfEF3GgkJpmnlL9lq85Pmwae9DxPrEflyjNlRFPtSLG/Nz50ou/NzaivsZri9qMz2T/rahwB/GkEt3In7o5cGcdG8SPNrrXpOTCHxFval09naTj/nG/tv0WTRhugsSLYLVXABfwf8ZIbG3fah0gFy9w5sxs7B4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1792045762; c=relaxed/simple; bh=ngMMr7wv1g49NtcMDPtMet1+NYvAn6ISxIYsQhJogC4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=EoBR0sVdfthZPScLleLa435JbG/pVLKXdh2mFouXIgdXSvsQDrfR1CeVqCCgF1G0RSKZHa2KVtdBq4k/YQddOcNOWjFdsZpuJfFotGNyde6hJXww+hJgGA3WTjiS8UUP+VJGEBEmL8/BMbYUttrusE7vl2mmo7UbGLhzX3KGEsA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com; spf=pass smtp.mailfrom=baylibre.com; dkim=pass (2048-bit key) header.d=baylibre.com header.i=@baylibre.com header.b=Cl+1VbGv; arc=none smtp.client-ip=209.85.128.43 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=baylibre.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=baylibre.com header.i=@baylibre.com header.b="Cl+1VbGv" Received: by mail-wm1-f43.google.com with SMTP id 5b1f17b1804b1-4a018dc1f98so39109795e9.3 for ; Wed, 14 Oct 2026 23:29:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre.com; s=google; t=1792045758; x=1792650558; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=CrzILkoyvowEBCBs4W94J+1nI3BRb4DDhSmm0sp3WxM=; b=Cl+1VbGve9R8Ka6N1rGjxz2HnPX569Tm1n1DR6sF7EG+5DD+4YZz0WJxP5T6R4vYaF tDrLs2VjC9vMoL5cNEwunvTnwgg+9O02W1Fa2vJK+40FuXfScaUx3ORpY8y2B1gynUHV xGSUEl9i3QVOAWrg0bwidzD6W+jodHwp9n1eg74O3erLKRyCEI9gLWcb0V1l3OtQe1Mv XAikl0A315kw862GfFaNfU5I52c3XPrqeIMHroEPfvDuuNNcB+ywmUKLesH0oirLBeY7 fVz9YjYC429S58c3ummgT8DX+Lxe/VKle/d0woSMr3McQjfga56QYq6GJ0BMc35j7mrW Eidg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1792045758; x=1792650558; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=CrzILkoyvowEBCBs4W94J+1nI3BRb4DDhSmm0sp3WxM=; b=0WmcAZ+H45NktdATo3OW73LPoRnnt1kc5cFw7h8XM5Euf6OSEcicR89Pne+XvtAVMq m6JUEJbqUoHQ4iTxNKlaxbqA3VPUqYO3AnGNdM6c4MqnfBbNU4sfj4dG0XhGQHdh3qlm 6vkUPpoZq0n8fIQ3JpaVt27jl12IJfIfNNtS2qPjlY/JgjwCMe9t01J3Fln1xxCSD6Mq 86PmhyJxdDZ7UnBWIoU97/xqo7FxCAhZjnhIxfIM4Ooy6vrRfGBoK7eiATNkPfjnRbEB M0DfDkZYXotDVzsP9LpkmBjJotTmcRKhMDWAf9s196tbY9iwu5DK8txy95/PDEiQJjOx ei8Q== X-Forwarded-Encrypted: i=1; AKwUvBxhZ3dMYQ5Ir/Krxp5qFPRBraKiBqGwGQL4K/AUiRNO2ogksR+EnUBxEDK50kfOweBtFvGu7nyB35HB@vger.kernel.org X-Gm-Message-State: AFq9FYI2avcJOV4Ztk6tjW7SB+KUdZXfUcXu8MCgMzSFlt2dP21ZDjBO ACVKyuOioG2ZFgu0OtLsTsVRg3xLxs5koS07qSDoksBg6mHBCUqJIHzY7I7kPwJV++o= X-Gm-Gg: AYBFou3TRh7WZ0iZngOyhazdjIhqdGCiU1POnQOPMukrVp69c/61FdY8MEr6nDdVT0/ qrU8WhtqGQi0Jeg7cOVnUIe/wbo3TyPrGoIzzeeWCcL3hGieyBXmzg2yBgC4u0FM82qK3V8fyQP mh3e9qbp/ACvTOFaYVCiwTAze+3oV8iAp/Z4pg2Xju/BbZlS/afWjJ2S3AK7ib6iSyR5hGdi9GK ooVQhPd2CqdavS0fKH7Hbm1QqeDLMzzauuhGs83rrMxr0XDMxnjY0qKST+bESQuqgSLbfDMbYZ5 0U4qacmnAFPOorjmUAHaPuO2thCp0/msohBR9Z/5/2m6b4k44FPOO/AFer3X1zhsV+3sy4v0Jwe IbIZxoTZeALibljhKEA/n1JD9PPekBH96T+Q9uZj1TKQVKNPivnbC9m3LoQvKZKNjgcnr6GmtW8 a97OFngiqE57Lc7pjBTdfRjONIf45aOz6gjoxwIDYM1bY2WOGQDOJ02r4DBzx/ep5DO3MM9p3UF /f1 X-Received: by 2002:a05:600c:8518:b0:49f:cbf3:551c with SMTP id 5b1f17b1804b1-4a18e4a8ca6mr326220155e9.12.1792045757868; Wed, 14 Oct 2026 23:29:17 -0700 (PDT) Received: from localhost ([2a02:8071:56d1:2de0:1d24:d58d:2b65:c291]) by smtp.gmail.com with UTF8SMTPSA id 5b1f17b1804b1-4a1bd9a8a15sm46085045e9.4.2026.10.14.23.29.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Oct 2026 23:29:17 -0700 (PDT) Date: Thu, 15 Oct 2026 08:29:15 +0200 From: Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= To: Abel Vesa Cc: Srinivas Kandagatla , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Bartosz Golaszewski , Marcel Holtmann , Luiz Augusto von Dentz , Jeff Johnson , Konrad Dybcio , Abel Vesa , Bjorn Andersson , Dmitry Baryshkov , Konrad Dybcio , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-bluetooth@vger.kernel.org, linux-wireless@vger.kernel.org, ath12k@lists.infradead.org Subject: Re: [PATCH RFC 2/5] nvmem: Add UEFI variable NVMEM provider Message-ID: References: <20261014-b4-nvmem-efivar-v1-0-187c85f65018@oss.qualcomm.com> <20261014-b4-nvmem-efivar-v1-2-187c85f65018@oss.qualcomm.com> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="zmiuyjupclul4syh" Content-Disposition: inline In-Reply-To: <20261014-b4-nvmem-efivar-v1-2-187c85f65018@oss.qualcomm.com> --zmiuyjupclul4syh Content-Type: text/plain; protected-headers=v1; charset=us-ascii Content-Disposition: inline Subject: Re: [PATCH RFC 2/5] nvmem: Add UEFI variable NVMEM provider MIME-Version: 1.0 On Wed, Oct 14, 2026 at 04:02:40PM +0300, Abel Vesa wrote: > +#include > [...] > +#include pulls in a lot of unneeded dependencies and already provides of_device_id. So please drop the former. > +static int efivar_nvmem_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + const char *name, *guid_str; > + struct nvmem_config cfg = {}; > + struct efivar_nvmem *priv; > + struct nvmem_device *nvmem; > + efi_char16_t *name16; > + efi_guid_t guid; > + guid_t g; > + unsigned long size; > + efi_status_t status; > + u32 attr; > + int i, len, ret; > + void *buf; > + > + /* > + * The EFI variable service may not be a native EFI runtime service; > + * backends such as the Qualcomm uefisecapp driver register the > + * efivars operations asynchronously. Wait for them. > + */ > + if (!efivar_is_available()) > + return -EPROBE_DEFER; > + > + ret = device_property_read_string(dev, "efivar-name", &name); > + if (ret) > + return dev_err_probe(dev, ret, "missing 'efivar-name'\n"); > + > + /* ASCII variable name -> UCS-2 as expected by the EFI service */ > + len = strlen(name); > + name16 = kcalloc(len + 1, sizeof(*name16), GFP_KERNEL); > + if (!name16) > + return -ENOMEM; > + for (i = 0; i < len; i++) > + name16[i] = name[i]; > + > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > + if (!priv) { > + kfree(name16); > + return -ENOMEM; > + } > + > + /* > + * The vendor GUID is optional: if it is not provided, look the > + * variable up by name across the whole store. A variable that has > + * not been provisioned yet is reported as not found and handled > + * below like an erased EEPROM. > + */ > + if (!device_property_read_string(dev, "efivar-guid", &guid_str)) { > + ret = guid_parse(guid_str, &g); > + if (ret) { > + kfree(name16); > + return dev_err_probe(dev, ret, "invalid 'efivar-guid'\n"); > + } > + memcpy(&guid, &g, sizeof(guid)); > + status = EFI_SUCCESS; > + } else { > + ret = efivar_nvmem_find_guid(name16, &guid); > + if (ret && ret != -ENODEV) { > + kfree(name16); > + return dev_err_probe(dev, ret, > + "failed to look up variable '%s'\n", > + name); > + } > + status = ret ? EFI_NOT_FOUND : EFI_SUCCESS; > + } > + > + /* First attempt with a modest buffer; grow if the firmware asks. */ > + size = 512; > + buf = kzalloc(size, GFP_KERNEL); > + if (!buf) { > + kfree(name16); > + return -ENOMEM; > + } > + > + if (status == EFI_SUCCESS) { > + status = efivar_get_variable(name16, &guid, &attr, &size, buf); > + if (status == EFI_BUFFER_TOO_SMALL) { > + kfree(buf); > + buf = kzalloc(size, GFP_KERNEL); > + if (!buf) { > + kfree(name16); > + return -ENOMEM; > + } > + status = efivar_get_variable(name16, &guid, &attr, > + &size, buf); > + } > + } > + kfree(name16); > + > + if (status == EFI_NOT_FOUND) { > + /* > + * Variable not provisioned yet: behave like an erased > + * EEPROM so that consumers still probe and fall back to > + * their defaults. > + */ > + kfree(buf); > + size = efivar_nvmem_cells_extent(dev); > + if (!size) > + return dev_err_probe(dev, -ENODEV, > + "variable '%s' not found and no cells described\n", > + name); > + buf = kzalloc(size, GFP_KERNEL); > + if (!buf) > + return -ENOMEM; > + dev_info(dev, "variable '%s' not provisioned, reading as zeroes\n", > + name); > + } else if (status != EFI_SUCCESS) { > + kfree(buf); > + return dev_err_probe(dev, -ENODEV, > + "GetVariable('%s') failed: 0x%lx\n", > + name, status); > + } else { > + dev_dbg(dev, "variable '%s' attr=0x%x size=%lu\n", > + name, attr, size); > + } > + > + priv->data = devm_kmemdup(dev, buf, size, GFP_KERNEL); > + kfree(buf); > + if (!priv->data) > + return -ENOMEM; > + priv->size = size; > + > + cfg.dev = dev; > + cfg.name = "efivar"; > + cfg.id = NVMEM_DEVID_AUTO; > + cfg.owner = THIS_MODULE; > + cfg.type = NVMEM_TYPE_OTP; > + cfg.read_only = true; > + cfg.word_size = 1; > + cfg.stride = 1; > + cfg.size = priv->size; > + cfg.reg_read = efivar_nvmem_reg_read; > + cfg.priv = priv; > + cfg.of_node = dev->of_node; > + cfg.add_legacy_fixed_of_cells = true; having only nearly all of these aligned is really strange. If you ask me, don't try to align =, just use a single space before it. You can write this as: cfg = (typeof(cfg)) { .dev = dev, .name = "efivar", ... }; which is a bit less repetitive and easier for the compiler to optimize. > + nvmem = devm_nvmem_register(dev, &cfg); Does nvmem_register only return success or -ENOMEM? If not, a dev_err_probe() would be nice here. > + return PTR_ERR_OR_ZERO(nvmem); > +} > + > +static const struct of_device_id efivar_nvmem_of_match[] = { > + { .compatible = "nvmem-efivar" }, > + {} s/{}/{ }/ > +}; > +MODULE_DEVICE_TABLE(of, efivar_nvmem_of_match); Best regards Uwe --zmiuyjupclul4syh Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEP4GsaTp6HlmJrf7Tj4D7WH0S/k4FAmrQcrgACgkQj4D7WH0S /k7Hggf/V5sRv/MeY3XE+UjeelKqRfTHz0evXvOJWyrUMTHIBxrDkz8QKuEN9Suk jeahg8flS8j/DDI1JqZdIFpsddA/zp4pQtQL5VgFe35gJoG3wYXsrVLdWozBylIP n/evHMBXhRXiCUnQPMB4l3th14mTR5TE1Hb94GtlMKTueC1PPc4L0jzfLBPzEsdT vnz/amCvOXWaguwBCYf2gy/PYMK/JZtJf0peAj404US7HDDFWiwqI+UdFRK4sTTy xUgWhdZ+Bj5PLXYavMI1VcPx6G1YPlfflK1GjkMA5/puxk28nPCguUFz9G9zoKwR 7QQYsNrlVCJ9O8W14hihe8LrM7cotQ== =Qsq8 -----END PGP SIGNATURE----- --zmiuyjupclul4syh--