[PATCH 00/12] arm64: qcom: autodetect firmware paths

[PATCH 00/12] arm64: qcom: autodetect firmware paths

[Dmitry Baryshkov]

This is a followup to the discussion during the Linaro Connect. Remove
most of the firmware-name properties from the board DT by using
root node compatible to detect firmware path.

The most obvious change is that the drivers now have to look for the
MBN firmware files by default, so this might break the case of the user
simply mounting vendor's firmware partition to /lib/firmware and
expecting it to work.

Also things are slightly more complex for the platforms like DB845c and
Qualcomm RB5. These platforms have generic SoC firmware in qcom/sdm845
and qcom/sm8250 and also the board-specific firmware at
qcom/sdm845/Thundercomm/DB845C and qcom/sm8250/Thundercomm/RB5
respectively. Making these boards follow up the scheme would require
additional symlinks in the firmware dir.

+Link: qcom/sdm845/Thundercomm/db845c/a630_zap.mbn -> ../../a630_zap.mbn
+Link: qcom/sm8250/Thundercomm/RB5/a650_zap.mbn -> ../../a650_zap.mbn
+Link: qcom/sdm845/Thundercomm/db845c/adsp.mbn -> ../../adsp.mbn
+Link: qcom/sdm845/Thundercomm/db845c/adspr.jsn -> ../../adspr.jsn
+Link: qcom/sdm845/Thundercomm/db845c/adspua.jsn -> ../../adspua.jsn
+Link: qcom/sdm845/Thundercomm/db845c/cdsp.mbn -> ../../cdsp.mbn
+Link: qcom/sdm845/Thundercomm/db845c/cdspr.jsn -> ../../cdspr.jsn
+Link: qcom/sm8250/Thundercomm/RB5/adsp.mbn -> ../../adsp.mbn
+Link: qcom/sm8250/Thundercomm/RB5/adspr.jsn -> ../../adspr.jsn
+Link: qcom/sm8250/Thundercomm/RB5/adspua.jsn -> ../../adspua.jsn
+Link: qcom/sm8250/Thundercomm/RB5/cdsp.mbn -> ../../cdsp.mbn
+Link: qcom/sm8250/Thundercomm/RB5/cdspr.jsn -> ../../cdspr.jsn

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
Dmitry Baryshkov (12):
      soc: qcom: add firmware name helper
      wifi: wcn36xx: make use of QCOM_FW_HELPER
      soc: qcom: wcnss_ctrl: make use of QCOM_FW_HELPER
      remoteproc: qcom_q6v5_mss: switch to mbn files by default
      remoteproc: qcom_q6v5_mss: make use of QCOM_FW_HELPER
      remoteproc: qcom_q6v5_pas: switch to mbn files by default
      remoteproc: qcom_q6v5_pas: make use of QCOM_FW_HELPER
      remoteproc: qcom_wcnss: switch to mbn files by default
      remoteproc: qcom_wcnss: make use of QCOM_FW_HELPER
      remoteproc: qcom_wcnss: make use of QCOM_FW_HELPER
      arm64: dts: qcom: apq8016-sbc: drop firmware-name properties
      arm64: dts: qcom: apq8096-db820c: drop firmware-name properties

 arch/arm64/boot/dts/qcom/apq8016-sbc.dts    |  5 +-
 arch/arm64/boot/dts/qcom/apq8096-db820c.dts |  2 -
 drivers/net/wireless/ath/wcn36xx/Kconfig    |  1 +
 drivers/net/wireless/ath/wcn36xx/main.c     |  5 ++
 drivers/remoteproc/Kconfig                  |  3 +
 drivers/remoteproc/qcom_q6v5_mss.c          | 12 +++-
 drivers/remoteproc/qcom_q6v5_pas.c          | 85 +++++++++++++++-------------
 drivers/remoteproc/qcom_wcnss.c             |  8 ++-
 drivers/soc/qcom/Kconfig                    |  6 ++
 drivers/soc/qcom/Makefile                   |  1 +
 drivers/soc/qcom/qcom_fw_helper.c           | 86 +++++++++++++++++++++++++++++
 drivers/soc/qcom/wcnss_ctrl.c               |  9 +++
 include/linux/soc/qcom/fw_helper.h          | 10 ++++
 13 files changed, 187 insertions(+), 46 deletions(-)
---
base-commit: 632483ea8004edfadd035de36e1ab2c7c4f53158
change-id: 20240520-qcom-firmware-name-aeef265a753a

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

[PATCH 01/12] soc: qcom: add firmware name helper

[Dmitry Baryshkov]

Qualcomm platforms have different sets of the firmware files, which
differ from platform to platform (and from board to board, due to the
embedded signatures). Rather than listing all the firmware files,
including full paths, in the DT, provide a way to determine firmware
path based on the root DT node compatible.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/soc/qcom/Kconfig           |  5 +++
 drivers/soc/qcom/Makefile          |  1 +
 drivers/soc/qcom/qcom_fw_helper.c  | 86 ++++++++++++++++++++++++++++++++++++++
 include/linux/soc/qcom/fw_helper.h | 10 +++++
 4 files changed, 102 insertions(+)

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 5af33b0e3470..b663774d65f8 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -62,6 +62,11 @@ config QCOM_MDT_LOADER
 	tristate
 	select QCOM_SCM
 
+config QCOM_FW_HELPER
+	tristate "NONE FW HELPER"
+	help
+	  Helpers to return platform-specific location for the firmware files.
+
 config QCOM_OCMEM
 	tristate "Qualcomm On Chip Memory (OCMEM) driver"
 	depends on ARCH_QCOM
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index ca0bece0dfff..e612bee5b955 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_QCOM_GENI_SE) +=	qcom-geni-se.o
 obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
 obj-$(CONFIG_QCOM_GSBI)	+=	qcom_gsbi.o
 obj-$(CONFIG_QCOM_MDT_LOADER)	+= mdt_loader.o
+obj-$(CONFIG_QCOM_FW_HELPER)	+= qcom_fw_helper.o
 obj-$(CONFIG_QCOM_OCMEM)	+= ocmem.o
 obj-$(CONFIG_QCOM_PDR_HELPERS)	+= pdr_interface.o
 obj-$(CONFIG_QCOM_PMIC_GLINK)	+= pmic_glink.o
diff --git a/drivers/soc/qcom/qcom_fw_helper.c b/drivers/soc/qcom/qcom_fw_helper.c
new file mode 100644
index 000000000000..13123c2514b8
--- /dev/null
+++ b/drivers/soc/qcom/qcom_fw_helper.c
@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Qualcomm Firmware loading data
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ */
+
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/soc/qcom/fw_helper.h>
+
+static DEFINE_MUTEX(qcom_fw_mutex);
+static const char *fw_path;
+
+static const struct of_device_id qcom_fw_paths[] = {
+	/* device-specific entries */
+	{ .compatible = "thundercomm,db845c", .data = "qcom/sdm845/Thundercomm/db845c", },
+	{ .compatible = "qcom,qrb5165-rb5", .data = "qcom/sm8250/Thundercomm/RB5", },
+	/* SoC default entries */
+	{ .compatible = "qcom,apq8016", .data = "qcom/apq8016", },
+	{ .compatible = "qcom,apq8096", .data = "qcom/apq8096", },
+	{ .compatible = "qcom,sdm845", .data = "qcom/sdm845", },
+	{ .compatible = "qcom,sm8250", .data = "qcom/sm8250", },
+	{ .compatible = "qcom,sm8350", .data = "qcom/sm8350", },
+	{ .compatible = "qcom,sm8450", .data = "qcom/sm8450", },
+	{ .compatible = "qcom,sm8550", .data = "qcom/sm8550", },
+	{ .compatible = "qcom,sm8650", .data = "qcom/sm8650", },
+	{},
+};
+
+static int qcom_fw_ensure_init(void)
+{
+	const struct of_device_id *match;
+	struct device_node *root;
+
+	if (fw_path)
+		return 0;
+
+	root = of_find_node_by_path("/");
+	if (!root)
+		return -ENODEV;
+
+	match = of_match_node(qcom_fw_paths, root);
+	of_node_put(root);
+	if (!match || !match->data) {
+		pr_notice("Platform not supported by qcom_fw_helper\n");
+		return -ENODEV;
+	}
+
+	fw_path = match->data;
+
+	return 0;
+}
+
+const char *qcom_get_board_fw(const char *firmware)
+{
+	if (strchr(firmware, '/'))
+		return kstrdup(firmware, GFP_KERNEL);
+
+	scoped_guard(mutex, &qcom_fw_mutex) {
+		if (!qcom_fw_ensure_init())
+			return kasprintf(GFP_KERNEL, "%s/%s", fw_path, firmware);
+	}
+
+	return kstrdup(firmware, GFP_KERNEL);
+}
+EXPORT_SYMBOL_GPL(qcom_get_board_fw);
+
+const char *devm_qcom_get_board_fw(struct device *dev, const char *firmware)
+{
+	if (strchr(firmware, '/'))
+		return devm_kstrdup(dev, firmware, GFP_KERNEL);
+
+	scoped_guard(mutex, &qcom_fw_mutex) {
+		if (!qcom_fw_ensure_init())
+			return devm_kasprintf(dev, GFP_KERNEL, "%s/%s", fw_path, firmware);
+	}
+
+	return devm_kstrdup(dev, firmware, GFP_KERNEL);
+}
+EXPORT_SYMBOL_GPL(devm_qcom_get_board_fw);
+
+MODULE_DESCRIPTION("Firmware helpers for Qualcomm devices");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/soc/qcom/fw_helper.h b/include/linux/soc/qcom/fw_helper.h
new file mode 100644
index 000000000000..755645386bba
--- /dev/null
+++ b/include/linux/soc/qcom/fw_helper.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __QCOM_FW_HELPER_H__
+#define __QCOM_FW_HELPER_H__
+
+struct device;
+
+const char *qcom_get_board_fw(const char *firmware);
+const char *devm_qcom_get_board_fw(struct device *dev, const char *firmware);
+
+#endif

-- 
2.39.2

Re: [PATCH 01/12] soc: qcom: add firmware name helper

[neil.armstrong]

On 21/05/2024 11:45, Dmitry Baryshkov wrote:
> Qualcomm platforms have different sets of the firmware files, which
> differ from platform to platform (and from board to board, due to the
> embedded signatures). Rather than listing all the firmware files,
> including full paths, in the DT, provide a way to determine firmware
> path based on the root DT node compatible.

Ok this looks quite over-engineered but necessary to handle the legacy,
but I really think we should add a way to look for a board-specific path
first and fallback to those SoC specific paths.

Neil

> 
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/soc/qcom/Kconfig           |  5 +++
>   drivers/soc/qcom/Makefile          |  1 +
>   drivers/soc/qcom/qcom_fw_helper.c  | 86 ++++++++++++++++++++++++++++++++++++++
>   include/linux/soc/qcom/fw_helper.h | 10 +++++
>   4 files changed, 102 insertions(+)
> 
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 5af33b0e3470..b663774d65f8 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -62,6 +62,11 @@ config QCOM_MDT_LOADER
>   	tristate
>   	select QCOM_SCM
>   
> +config QCOM_FW_HELPER
> +	tristate "NONE FW HELPER"
> +	help
> +	  Helpers to return platform-specific location for the firmware files.
> +
>   config QCOM_OCMEM
>   	tristate "Qualcomm On Chip Memory (OCMEM) driver"
>   	depends on ARCH_QCOM
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index ca0bece0dfff..e612bee5b955 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_QCOM_GENI_SE) +=	qcom-geni-se.o
>   obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
>   obj-$(CONFIG_QCOM_GSBI)	+=	qcom_gsbi.o
>   obj-$(CONFIG_QCOM_MDT_LOADER)	+= mdt_loader.o
> +obj-$(CONFIG_QCOM_FW_HELPER)	+= qcom_fw_helper.o
>   obj-$(CONFIG_QCOM_OCMEM)	+= ocmem.o
>   obj-$(CONFIG_QCOM_PDR_HELPERS)	+= pdr_interface.o
>   obj-$(CONFIG_QCOM_PMIC_GLINK)	+= pmic_glink.o
> diff --git a/drivers/soc/qcom/qcom_fw_helper.c b/drivers/soc/qcom/qcom_fw_helper.c
> new file mode 100644
> index 000000000000..13123c2514b8
> --- /dev/null
> +++ b/drivers/soc/qcom/qcom_fw_helper.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Qualcomm Firmware loading data
> + *
> + * Copyright (C) 2024 Linaro Ltd
> + */
> +
> +#include <linux/cleanup.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/soc/qcom/fw_helper.h>
> +
> +static DEFINE_MUTEX(qcom_fw_mutex);
> +static const char *fw_path;
> +
> +static const struct of_device_id qcom_fw_paths[] = {
> +	/* device-specific entries */
> +	{ .compatible = "thundercomm,db845c", .data = "qcom/sdm845/Thundercomm/db845c", },
> +	{ .compatible = "qcom,qrb5165-rb5", .data = "qcom/sm8250/Thundercomm/RB5", },
> +	/* SoC default entries */
> +	{ .compatible = "qcom,apq8016", .data = "qcom/apq8016", },
> +	{ .compatible = "qcom,apq8096", .data = "qcom/apq8096", },
> +	{ .compatible = "qcom,sdm845", .data = "qcom/sdm845", },
> +	{ .compatible = "qcom,sm8250", .data = "qcom/sm8250", },
> +	{ .compatible = "qcom,sm8350", .data = "qcom/sm8350", },
> +	{ .compatible = "qcom,sm8450", .data = "qcom/sm8450", },
> +	{ .compatible = "qcom,sm8550", .data = "qcom/sm8550", },
> +	{ .compatible = "qcom,sm8650", .data = "qcom/sm8650", },
> +	{},
> +};
> +
> +static int qcom_fw_ensure_init(void)
> +{
> +	const struct of_device_id *match;
> +	struct device_node *root;
> +
> +	if (fw_path)
> +		return 0;
> +
> +	root = of_find_node_by_path("/");
> +	if (!root)
> +		return -ENODEV;
> +
> +	match = of_match_node(qcom_fw_paths, root);
> +	of_node_put(root);
> +	if (!match || !match->data) {
> +		pr_notice("Platform not supported by qcom_fw_helper\n");
> +		return -ENODEV;
> +	}
> +
> +	fw_path = match->data;
> +
> +	return 0;
> +}
> +
> +const char *qcom_get_board_fw(const char *firmware)
> +{
> +	if (strchr(firmware, '/'))
> +		return kstrdup(firmware, GFP_KERNEL);
> +
> +	scoped_guard(mutex, &qcom_fw_mutex) {
> +		if (!qcom_fw_ensure_init())
> +			return kasprintf(GFP_KERNEL, "%s/%s", fw_path, firmware);
> +	}
> +
> +	return kstrdup(firmware, GFP_KERNEL);
> +}
> +EXPORT_SYMBOL_GPL(qcom_get_board_fw);
> +
> +const char *devm_qcom_get_board_fw(struct device *dev, const char *firmware)
> +{
> +	if (strchr(firmware, '/'))
> +		return devm_kstrdup(dev, firmware, GFP_KERNEL);
> +
> +	scoped_guard(mutex, &qcom_fw_mutex) {
> +		if (!qcom_fw_ensure_init())
> +			return devm_kasprintf(dev, GFP_KERNEL, "%s/%s", fw_path, firmware);
> +	}
> +
> +	return devm_kstrdup(dev, firmware, GFP_KERNEL);
> +}
> +EXPORT_SYMBOL_GPL(devm_qcom_get_board_fw);
> +
> +MODULE_DESCRIPTION("Firmware helpers for Qualcomm devices");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/soc/qcom/fw_helper.h b/include/linux/soc/qcom/fw_helper.h
> new file mode 100644
> index 000000000000..755645386bba
> --- /dev/null
> +++ b/include/linux/soc/qcom/fw_helper.h
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __QCOM_FW_HELPER_H__
> +#define __QCOM_FW_HELPER_H__
> +
> +struct device;
> +
> +const char *qcom_get_board_fw(const char *firmware);
> +const char *devm_qcom_get_board_fw(struct device *dev, const char *firmware);
> +
> +#endif
> 

Re: [PATCH 01/12] soc: qcom: add firmware name helper

[Dmitry Baryshkov]

On Tue, 21 May 2024 at 12:52, <neil.armstrong@linaro.org> wrote:
>
> On 21/05/2024 11:45, Dmitry Baryshkov wrote:
> > Qualcomm platforms have different sets of the firmware files, which
> > differ from platform to platform (and from board to board, due to the
> > embedded signatures). Rather than listing all the firmware files,
> > including full paths, in the DT, provide a way to determine firmware
> > path based on the root DT node compatible.
>
> Ok this looks quite over-engineered but necessary to handle the legacy,
> but I really think we should add a way to look for a board-specific path
> first and fallback to those SoC specific paths.

Again, CONFIG_FW_LOADER_USER_HELPER => delays.

>
> Neil
>
> >
> > Suggested-by: Arnd Bergmann <arnd@arndb.de>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >   drivers/soc/qcom/Kconfig           |  5 +++
> >   drivers/soc/qcom/Makefile          |  1 +
> >   drivers/soc/qcom/qcom_fw_helper.c  | 86 ++++++++++++++++++++++++++++++++++++++
> >   include/linux/soc/qcom/fw_helper.h | 10 +++++
> >   4 files changed, 102 insertions(+)
> >
> > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> > index 5af33b0e3470..b663774d65f8 100644
> > --- a/drivers/soc/qcom/Kconfig
> > +++ b/drivers/soc/qcom/Kconfig
> > @@ -62,6 +62,11 @@ config QCOM_MDT_LOADER
> >       tristate
> >       select QCOM_SCM
> >
> > +config QCOM_FW_HELPER
> > +     tristate "NONE FW HELPER"
> > +     help
> > +       Helpers to return platform-specific location for the firmware files.
> > +
> >   config QCOM_OCMEM
> >       tristate "Qualcomm On Chip Memory (OCMEM) driver"
> >       depends on ARCH_QCOM
> > diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> > index ca0bece0dfff..e612bee5b955 100644
> > --- a/drivers/soc/qcom/Makefile
> > +++ b/drivers/soc/qcom/Makefile
> > @@ -6,6 +6,7 @@ obj-$(CONFIG_QCOM_GENI_SE) += qcom-geni-se.o
> >   obj-$(CONFIG_QCOM_COMMAND_DB) += cmd-db.o
> >   obj-$(CONFIG_QCOM_GSBI)     +=      qcom_gsbi.o
> >   obj-$(CONFIG_QCOM_MDT_LOADER)       += mdt_loader.o
> > +obj-$(CONFIG_QCOM_FW_HELPER) += qcom_fw_helper.o
> >   obj-$(CONFIG_QCOM_OCMEM)    += ocmem.o
> >   obj-$(CONFIG_QCOM_PDR_HELPERS)      += pdr_interface.o
> >   obj-$(CONFIG_QCOM_PMIC_GLINK)       += pmic_glink.o
> > diff --git a/drivers/soc/qcom/qcom_fw_helper.c b/drivers/soc/qcom/qcom_fw_helper.c
> > new file mode 100644
> > index 000000000000..13123c2514b8
> > --- /dev/null
> > +++ b/drivers/soc/qcom/qcom_fw_helper.c
> > @@ -0,0 +1,86 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Qualcomm Firmware loading data
> > + *
> > + * Copyright (C) 2024 Linaro Ltd
> > + */
> > +
> > +#include <linux/cleanup.h>
> > +#include <linux/device.h>
> > +#include <linux/mutex.h>
> > +#include <linux/of.h>
> > +#include <linux/soc/qcom/fw_helper.h>
> > +
> > +static DEFINE_MUTEX(qcom_fw_mutex);
> > +static const char *fw_path;
> > +
> > +static const struct of_device_id qcom_fw_paths[] = {
> > +     /* device-specific entries */
> > +     { .compatible = "thundercomm,db845c", .data = "qcom/sdm845/Thundercomm/db845c", },
> > +     { .compatible = "qcom,qrb5165-rb5", .data = "qcom/sm8250/Thundercomm/RB5", },
> > +     /* SoC default entries */
> > +     { .compatible = "qcom,apq8016", .data = "qcom/apq8016", },
> > +     { .compatible = "qcom,apq8096", .data = "qcom/apq8096", },
> > +     { .compatible = "qcom,sdm845", .data = "qcom/sdm845", },
> > +     { .compatible = "qcom,sm8250", .data = "qcom/sm8250", },
> > +     { .compatible = "qcom,sm8350", .data = "qcom/sm8350", },
> > +     { .compatible = "qcom,sm8450", .data = "qcom/sm8450", },
> > +     { .compatible = "qcom,sm8550", .data = "qcom/sm8550", },
> > +     { .compatible = "qcom,sm8650", .data = "qcom/sm8650", },
> > +     {},
> > +};
> > +
> > +static int qcom_fw_ensure_init(void)
> > +{
> > +     const struct of_device_id *match;
> > +     struct device_node *root;
> > +
> > +     if (fw_path)
> > +             return 0;
> > +
> > +     root = of_find_node_by_path("/");
> > +     if (!root)
> > +             return -ENODEV;
> > +
> > +     match = of_match_node(qcom_fw_paths, root);
> > +     of_node_put(root);
> > +     if (!match || !match->data) {
> > +             pr_notice("Platform not supported by qcom_fw_helper\n");
> > +             return -ENODEV;
> > +     }
> > +
> > +     fw_path = match->data;
> > +
> > +     return 0;
> > +}
> > +
> > +const char *qcom_get_board_fw(const char *firmware)
> > +{
> > +     if (strchr(firmware, '/'))
> > +             return kstrdup(firmware, GFP_KERNEL);
> > +
> > +     scoped_guard(mutex, &qcom_fw_mutex) {
> > +             if (!qcom_fw_ensure_init())
> > +                     return kasprintf(GFP_KERNEL, "%s/%s", fw_path, firmware);
> > +     }
> > +
> > +     return kstrdup(firmware, GFP_KERNEL);
> > +}
> > +EXPORT_SYMBOL_GPL(qcom_get_board_fw);
> > +
> > +const char *devm_qcom_get_board_fw(struct device *dev, const char *firmware)
> > +{
> > +     if (strchr(firmware, '/'))
> > +             return devm_kstrdup(dev, firmware, GFP_KERNEL);
> > +
> > +     scoped_guard(mutex, &qcom_fw_mutex) {
> > +             if (!qcom_fw_ensure_init())
> > +                     return devm_kasprintf(dev, GFP_KERNEL, "%s/%s", fw_path, firmware);
> > +     }
> > +
> > +     return devm_kstrdup(dev, firmware, GFP_KERNEL);
> > +}
> > +EXPORT_SYMBOL_GPL(devm_qcom_get_board_fw);
> > +
> > +MODULE_DESCRIPTION("Firmware helpers for Qualcomm devices");
> > +MODULE_LICENSE("GPL");
> > diff --git a/include/linux/soc/qcom/fw_helper.h b/include/linux/soc/qcom/fw_helper.h
> > new file mode 100644
> > index 000000000000..755645386bba
> > --- /dev/null
> > +++ b/include/linux/soc/qcom/fw_helper.h
> > @@ -0,0 +1,10 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __QCOM_FW_HELPER_H__
> > +#define __QCOM_FW_HELPER_H__
> > +
> > +struct device;
> > +
> > +const char *qcom_get_board_fw(const char *firmware);
> > +const char *devm_qcom_get_board_fw(struct device *dev, const char *firmware);
> > +
> > +#endif
> >
>


-- 
With best wishes
Dmitry

Re: [PATCH 01/12] soc: qcom: add firmware name helper

[Kalle Valo]

Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:

> On Tue, 21 May 2024 at 12:52, <neil.armstrong@linaro.org> wrote:
>>
>> On 21/05/2024 11:45, Dmitry Baryshkov wrote:
>> > Qualcomm platforms have different sets of the firmware files, which
>> > differ from platform to platform (and from board to board, due to the
>> > embedded signatures). Rather than listing all the firmware files,
>> > including full paths, in the DT, provide a way to determine firmware
>> > path based on the root DT node compatible.
>>
>> Ok this looks quite over-engineered but necessary to handle the legacy,
>> but I really think we should add a way to look for a board-specific path
>> first and fallback to those SoC specific paths.
>
> Again, CONFIG_FW_LOADER_USER_HELPER => delays.

To me this also looks like very over-engineered, can you elaborate more
why this is needed? Concrete examples would help to understand better.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 01/12] soc: qcom: add firmware name helper

[Dmitry Baryshkov]

On Tue, 21 May 2024 at 13:20, Kalle Valo <kvalo@kernel.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>
> > On Tue, 21 May 2024 at 12:52, <neil.armstrong@linaro.org> wrote:
> >>
> >> On 21/05/2024 11:45, Dmitry Baryshkov wrote:
> >> > Qualcomm platforms have different sets of the firmware files, which
> >> > differ from platform to platform (and from board to board, due to the
> >> > embedded signatures). Rather than listing all the firmware files,
> >> > including full paths, in the DT, provide a way to determine firmware
> >> > path based on the root DT node compatible.
> >>
> >> Ok this looks quite over-engineered but necessary to handle the legacy,
> >> but I really think we should add a way to look for a board-specific path
> >> first and fallback to those SoC specific paths.
> >
> > Again, CONFIG_FW_LOADER_USER_HELPER => delays.
>
> To me this also looks like very over-engineered, can you elaborate more
> why this is needed? Concrete examples would help to understand better.

Sure. During the meeting last week Arnd suggested evaluating if we can
drop firmware-name from the board DT files. Several reasons for that:
- DT should describe the hardware, not the Linux-firmware locations
- having firmware name in DT complicates updating the tree to use
different firmware API (think of mbn vs mdt vs any other format)
- If the DT gets supplied by the vendor (e.g. for
SystemReady-certified devices), there should be a sync between the
vendor's DT, linux kernel and the rootfs. Dropping firmware names from
DT solves that by removing one piece of the equation

Now for the complexity of the solution. Each SoC family has their own
firmware set. This includes firmware for the DSPs, for modem, WiFi
bits, GPU shader, etc.
For the development boards these devices are signed by the testing key
and the actual signature is not validated against the root of trust
certificate.
For the end-user devices the signature is actually validated against
the bits fused to the SoC during manufacturing process. CA certificate
(and thus the fuses) differ from vendor to vendor (and from the device
to device)

Not all of the firmware files are a part of the public linux-firmware
tree. However we need to support the rootfs bundled with the firmware
for different platforms (both public and vendor). The non-signed files
come from the Adreno GPU and can be shared between platforms. All
other files are SoC-specific and in some cases device-specific.

So for example the SDM845 db845c (open device) loads following firmware files:
Not signed:
- qcom/a630_sqe.fw
- qcom/a630_gmu.bin

Signed, will work for any non-secured sdm845 device:
- qcom/sdm845/a630_zap.mbn
- qcom/sdm845/adsp.mbn
- qcom/sdm845/cdsp.mbn
- qcom/sdm485/mba.mbn
- qcom/sdm845/modem.mbn
- qcom/sdm845/wlanmdsp.mbn (loaded via TQFTP)
- qcom/venus-5.2/venus.mbn

Signed, works only for DB845c.
- qcom/sdm845/Thundercomm/db845c/slpi.mbn

In comparison, the SDM845 Pixel-3 phone (aka blueline) should load the
following firmware files:
- qcom/a630_sqe.fw (the same, non-signed file)
- qcom/a630_gmu.bin (the same, non-signed file)
- qcom/sdm845/Google/blueline/a630_zap.mbn
- qcom/sdm845/Google/blueline/adsp.mbn
- qcom/sdm845/Google/blueline/cdsp.mbn
- qcom/sdm845/Google/blueline/ipa_fws.mbn
- qcom/sdm845/Google/blueline/mba.mbn
- qcom/sdm845/Google/blueline/modem.mbn
- qcom/sdm845/Google/blueline/venus.mbn
- qcom/sdm845/Google/blueline/wlanmdsp.mbn
- qcom/sdm845/Google/blueline/slpi.mbn

The Lenovo Yoga C630 WoS laptop (SDM850 is a variant of SDM845) uses
another set of files:
- qcom/a630_sqe.fw (the same, non-signed file)
- qcom/a630_gmu.bin (the same, non-signed file)
- qcom/sdm850/LENOVO/81JL/qcdxkmsuc850.mbn
- qcom/sdm850/LENOVO/81JL/qcadsp850.mbn
- qcom/sdm850/LENOVO/81JL/qccdsp850.mbn
- qcom/sdm850/LENOVO/81JL/ipa_fws.elf
- qcom/sdm850/LENOVO/81JL/qcdsp1v2850.mbn
- qcom/sdm850/LENOVO/81JL/qcdsp2850.mbn
- qcom/sdm850/LENOVO/81JL/qcvss850.mbn
- qcom/sdm850/LENOVO/81JL/wlanmdsp.mbn
- qcom/sdm850/LENOVO/81JL/qcslpi850.mbn

If we look at one of the recent platforms, e.g. SM8650-QRD, this list
also grows up:
- qcom/gen70900_sqe.fw (generic, non-signed)
- qcom/gmu_gen70900.bin (generic, non-signed)
- qcom/sm8650/gen70900_zap.mbn
- qcom/sm8650/adsp.mbn
- qcom/sm8650/adsp_dtb.mbn
- qcom/sm8650/cdsp.mbn
- qcom/sm8650/cdsp_dtb.mbn
- qcom/sm8650/ipa_fws.mbn
- qcom/sm8650/modem.mbn
- qcom/sm8650/modem_dtb.mbn
- qcom/sm8650/vpu33_4v.mbn (or maybe qcom/vpu-33/vpu_4v.mbn)

-- 
With best wishes
Dmitry

Re: [PATCH 01/12] soc: qcom: add firmware name helper

[Bjorn Andersson]

On Tue, May 21, 2024 at 03:08:31PM +0200, Dmitry Baryshkov wrote:
> On Tue, 21 May 2024 at 13:20, Kalle Valo <kvalo@kernel.org> wrote:
> >
> > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> >
> > > On Tue, 21 May 2024 at 12:52, <neil.armstrong@linaro.org> wrote:
> > >>
> > >> On 21/05/2024 11:45, Dmitry Baryshkov wrote:
> > >> > Qualcomm platforms have different sets of the firmware files, which
> > >> > differ from platform to platform (and from board to board, due to the
> > >> > embedded signatures). Rather than listing all the firmware files,
> > >> > including full paths, in the DT, provide a way to determine firmware
> > >> > path based on the root DT node compatible.
> > >>
> > >> Ok this looks quite over-engineered but necessary to handle the legacy,
> > >> but I really think we should add a way to look for a board-specific path
> > >> first and fallback to those SoC specific paths.
> > >
> > > Again, CONFIG_FW_LOADER_USER_HELPER => delays.
> >
> > To me this also looks like very over-engineered, can you elaborate more
> > why this is needed? Concrete examples would help to understand better.
> 
> Sure. During the meeting last week Arnd suggested evaluating if we can
> drop firmware-name from the board DT files. Several reasons for that:
> - DT should describe the hardware, not the Linux-firmware locations
> - having firmware name in DT complicates updating the tree to use
> different firmware API (think of mbn vs mdt vs any other format)
> - If the DT gets supplied by the vendor (e.g. for
> SystemReady-certified devices), there should be a sync between the
> vendor's DT, linux kernel and the rootfs. Dropping firmware names from
> DT solves that by removing one piece of the equation
> 
> Now for the complexity of the solution. Each SoC family has their own
> firmware set. This includes firmware for the DSPs, for modem, WiFi
> bits, GPU shader, etc.
> For the development boards these devices are signed by the testing key
> and the actual signature is not validated against the root of trust
> certificate.
> For the end-user devices the signature is actually validated against
> the bits fused to the SoC during manufacturing process. CA certificate
> (and thus the fuses) differ from vendor to vendor (and from the device
> to device)
> 
> Not all of the firmware files are a part of the public linux-firmware
> tree. However we need to support the rootfs bundled with the firmware
> for different platforms (both public and vendor). The non-signed files
> come from the Adreno GPU and can be shared between platforms. All
> other files are SoC-specific and in some cases device-specific.
> 
> So for example the SDM845 db845c (open device) loads following firmware files:
> Not signed:
> - qcom/a630_sqe.fw
> - qcom/a630_gmu.bin
> 
> Signed, will work for any non-secured sdm845 device:
> - qcom/sdm845/a630_zap.mbn
> - qcom/sdm845/adsp.mbn
> - qcom/sdm845/cdsp.mbn
> - qcom/sdm485/mba.mbn
> - qcom/sdm845/modem.mbn
> - qcom/sdm845/wlanmdsp.mbn (loaded via TQFTP)
> - qcom/venus-5.2/venus.mbn
> 
> Signed, works only for DB845c.
> - qcom/sdm845/Thundercomm/db845c/slpi.mbn
> 
> In comparison, the SDM845 Pixel-3 phone (aka blueline) should load the
> following firmware files:
> - qcom/a630_sqe.fw (the same, non-signed file)
> - qcom/a630_gmu.bin (the same, non-signed file)
> - qcom/sdm845/Google/blueline/a630_zap.mbn

How do you get from "a630_zap.mbn" to this? By extending the lookup
table for every target, or what am I missing?

Regards,
Bjorn

> - qcom/sdm845/Google/blueline/adsp.mbn
> - qcom/sdm845/Google/blueline/cdsp.mbn
> - qcom/sdm845/Google/blueline/ipa_fws.mbn
> - qcom/sdm845/Google/blueline/mba.mbn
> - qcom/sdm845/Google/blueline/modem.mbn
> - qcom/sdm845/Google/blueline/venus.mbn
> - qcom/sdm845/Google/blueline/wlanmdsp.mbn
> - qcom/sdm845/Google/blueline/slpi.mbn
> 
> The Lenovo Yoga C630 WoS laptop (SDM850 is a variant of SDM845) uses
> another set of files:
> - qcom/a630_sqe.fw (the same, non-signed file)
> - qcom/a630_gmu.bin (the same, non-signed file)
> - qcom/sdm850/LENOVO/81JL/qcdxkmsuc850.mbn
> - qcom/sdm850/LENOVO/81JL/qcadsp850.mbn
> - qcom/sdm850/LENOVO/81JL/qccdsp850.mbn
> - qcom/sdm850/LENOVO/81JL/ipa_fws.elf
> - qcom/sdm850/LENOVO/81JL/qcdsp1v2850.mbn
> - qcom/sdm850/LENOVO/81JL/qcdsp2850.mbn
> - qcom/sdm850/LENOVO/81JL/qcvss850.mbn
> - qcom/sdm850/LENOVO/81JL/wlanmdsp.mbn
> - qcom/sdm850/LENOVO/81JL/qcslpi850.mbn
> 
> If we look at one of the recent platforms, e.g. SM8650-QRD, this list
> also grows up:
> - qcom/gen70900_sqe.fw (generic, non-signed)
> - qcom/gmu_gen70900.bin (generic, non-signed)
> - qcom/sm8650/gen70900_zap.mbn
> - qcom/sm8650/adsp.mbn
> - qcom/sm8650/adsp_dtb.mbn
> - qcom/sm8650/cdsp.mbn
> - qcom/sm8650/cdsp_dtb.mbn
> - qcom/sm8650/ipa_fws.mbn
> - qcom/sm8650/modem.mbn
> - qcom/sm8650/modem_dtb.mbn
> - qcom/sm8650/vpu33_4v.mbn (or maybe qcom/vpu-33/vpu_4v.mbn)
> 
> -- 
> With best wishes
> Dmitry
> 
> 
> 
> 
> 
> 
> 
> 
> 

Re: [PATCH 01/12] soc: qcom: add firmware name helper

[Dmitry Baryshkov]

On Thu, 23 May 2024 at 01:48, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>
> On Tue, May 21, 2024 at 03:08:31PM +0200, Dmitry Baryshkov wrote:
> > On Tue, 21 May 2024 at 13:20, Kalle Valo <kvalo@kernel.org> wrote:
> > >
> > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> > >
> > > > On Tue, 21 May 2024 at 12:52, <neil.armstrong@linaro.org> wrote:
> > > >>
> > > >> On 21/05/2024 11:45, Dmitry Baryshkov wrote:
> > > >> > Qualcomm platforms have different sets of the firmware files, which
> > > >> > differ from platform to platform (and from board to board, due to the
> > > >> > embedded signatures). Rather than listing all the firmware files,
> > > >> > including full paths, in the DT, provide a way to determine firmware
> > > >> > path based on the root DT node compatible.
> > > >>
> > > >> Ok this looks quite over-engineered but necessary to handle the legacy,
> > > >> but I really think we should add a way to look for a board-specific path
> > > >> first and fallback to those SoC specific paths.
> > > >
> > > > Again, CONFIG_FW_LOADER_USER_HELPER => delays.
> > >
> > > To me this also looks like very over-engineered, can you elaborate more
> > > why this is needed? Concrete examples would help to understand better.
> >
> > Sure. During the meeting last week Arnd suggested evaluating if we can
> > drop firmware-name from the board DT files. Several reasons for that:
> > - DT should describe the hardware, not the Linux-firmware locations
> > - having firmware name in DT complicates updating the tree to use
> > different firmware API (think of mbn vs mdt vs any other format)
> > - If the DT gets supplied by the vendor (e.g. for
> > SystemReady-certified devices), there should be a sync between the
> > vendor's DT, linux kernel and the rootfs. Dropping firmware names from
> > DT solves that by removing one piece of the equation
> >
> > Now for the complexity of the solution. Each SoC family has their own
> > firmware set. This includes firmware for the DSPs, for modem, WiFi
> > bits, GPU shader, etc.
> > For the development boards these devices are signed by the testing key
> > and the actual signature is not validated against the root of trust
> > certificate.
> > For the end-user devices the signature is actually validated against
> > the bits fused to the SoC during manufacturing process. CA certificate
> > (and thus the fuses) differ from vendor to vendor (and from the device
> > to device)
> >
> > Not all of the firmware files are a part of the public linux-firmware
> > tree. However we need to support the rootfs bundled with the firmware
> > for different platforms (both public and vendor). The non-signed files
> > come from the Adreno GPU and can be shared between platforms. All
> > other files are SoC-specific and in some cases device-specific.
> >
> > So for example the SDM845 db845c (open device) loads following firmware files:
> > Not signed:
> > - qcom/a630_sqe.fw
> > - qcom/a630_gmu.bin
> >
> > Signed, will work for any non-secured sdm845 device:
> > - qcom/sdm845/a630_zap.mbn
> > - qcom/sdm845/adsp.mbn
> > - qcom/sdm845/cdsp.mbn
> > - qcom/sdm485/mba.mbn
> > - qcom/sdm845/modem.mbn
> > - qcom/sdm845/wlanmdsp.mbn (loaded via TQFTP)
> > - qcom/venus-5.2/venus.mbn
> >
> > Signed, works only for DB845c.
> > - qcom/sdm845/Thundercomm/db845c/slpi.mbn
> >
> > In comparison, the SDM845 Pixel-3 phone (aka blueline) should load the
> > following firmware files:
> > - qcom/a630_sqe.fw (the same, non-signed file)
> > - qcom/a630_gmu.bin (the same, non-signed file)
> > - qcom/sdm845/Google/blueline/a630_zap.mbn
>
> How do you get from "a630_zap.mbn" to this? By extending the lookup
> table for every target, or what am I missing?

More or less so. Matching the root OF node gives us the firmware
location, then it gets prepended to all firmware targets. Not an ideal
solution, as there is no fallback support, but at least it gives us
some points to discuss (and to decide whether to move to some
particular direction or to abandon the idea completely, making Arnd
unhappy again).

>
> Regards,
> Bjorn
>
> > - qcom/sdm845/Google/blueline/adsp.mbn
> > - qcom/sdm845/Google/blueline/cdsp.mbn
> > - qcom/sdm845/Google/blueline/ipa_fws.mbn
> > - qcom/sdm845/Google/blueline/mba.mbn
> > - qcom/sdm845/Google/blueline/modem.mbn
> > - qcom/sdm845/Google/blueline/venus.mbn
> > - qcom/sdm845/Google/blueline/wlanmdsp.mbn
> > - qcom/sdm845/Google/blueline/slpi.mbn
> >
> > The Lenovo Yoga C630 WoS laptop (SDM850 is a variant of SDM845) uses
> > another set of files:
> > - qcom/a630_sqe.fw (the same, non-signed file)
> > - qcom/a630_gmu.bin (the same, non-signed file)
> > - qcom/sdm850/LENOVO/81JL/qcdxkmsuc850.mbn
> > - qcom/sdm850/LENOVO/81JL/qcadsp850.mbn
> > - qcom/sdm850/LENOVO/81JL/qccdsp850.mbn
> > - qcom/sdm850/LENOVO/81JL/ipa_fws.elf
> > - qcom/sdm850/LENOVO/81JL/qcdsp1v2850.mbn
> > - qcom/sdm850/LENOVO/81JL/qcdsp2850.mbn
> > - qcom/sdm850/LENOVO/81JL/qcvss850.mbn
> > - qcom/sdm850/LENOVO/81JL/wlanmdsp.mbn
> > - qcom/sdm850/LENOVO/81JL/qcslpi850.mbn
> >
> > If we look at one of the recent platforms, e.g. SM8650-QRD, this list
> > also grows up:
> > - qcom/gen70900_sqe.fw (generic, non-signed)
> > - qcom/gmu_gen70900.bin (generic, non-signed)
> > - qcom/sm8650/gen70900_zap.mbn
> > - qcom/sm8650/adsp.mbn
> > - qcom/sm8650/adsp_dtb.mbn
> > - qcom/sm8650/cdsp.mbn
> > - qcom/sm8650/cdsp_dtb.mbn
> > - qcom/sm8650/ipa_fws.mbn
> > - qcom/sm8650/modem.mbn
> > - qcom/sm8650/modem_dtb.mbn
> > - qcom/sm8650/vpu33_4v.mbn (or maybe qcom/vpu-33/vpu_4v.mbn)
> >
> > --
> > With best wishes
> > Dmitry
> >
> >
> >
> >
> >
> >
> >
> >
> >



-- 
With best wishes
Dmitry

Re: [PATCH 01/12] soc: qcom: add firmware name helper

[Bjorn Andersson]

On Mon, May 27, 2024 at 02:42:44PM GMT, Dmitry Baryshkov wrote:
> On Thu, 23 May 2024 at 01:48, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
> >
> > On Tue, May 21, 2024 at 03:08:31PM +0200, Dmitry Baryshkov wrote:
> > > On Tue, 21 May 2024 at 13:20, Kalle Valo <kvalo@kernel.org> wrote:
> > > >
> > > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> > > >
> > > > > On Tue, 21 May 2024 at 12:52, <neil.armstrong@linaro.org> wrote:
> > > > >>
> > > > >> On 21/05/2024 11:45, Dmitry Baryshkov wrote:
> > > > >> > Qualcomm platforms have different sets of the firmware files, which
> > > > >> > differ from platform to platform (and from board to board, due to the
> > > > >> > embedded signatures). Rather than listing all the firmware files,
> > > > >> > including full paths, in the DT, provide a way to determine firmware
> > > > >> > path based on the root DT node compatible.
> > > > >>
> > > > >> Ok this looks quite over-engineered but necessary to handle the legacy,
> > > > >> but I really think we should add a way to look for a board-specific path
> > > > >> first and fallback to those SoC specific paths.
> > > > >
> > > > > Again, CONFIG_FW_LOADER_USER_HELPER => delays.
> > > >
> > > > To me this also looks like very over-engineered, can you elaborate more
> > > > why this is needed? Concrete examples would help to understand better.
> > >
> > > Sure. During the meeting last week Arnd suggested evaluating if we can
> > > drop firmware-name from the board DT files. Several reasons for that:
> > > - DT should describe the hardware, not the Linux-firmware locations
> > > - having firmware name in DT complicates updating the tree to use
> > > different firmware API (think of mbn vs mdt vs any other format)
> > > - If the DT gets supplied by the vendor (e.g. for
> > > SystemReady-certified devices), there should be a sync between the
> > > vendor's DT, linux kernel and the rootfs. Dropping firmware names from
> > > DT solves that by removing one piece of the equation
> > >
> > > Now for the complexity of the solution. Each SoC family has their own
> > > firmware set. This includes firmware for the DSPs, for modem, WiFi
> > > bits, GPU shader, etc.
> > > For the development boards these devices are signed by the testing key
> > > and the actual signature is not validated against the root of trust
> > > certificate.
> > > For the end-user devices the signature is actually validated against
> > > the bits fused to the SoC during manufacturing process. CA certificate
> > > (and thus the fuses) differ from vendor to vendor (and from the device
> > > to device)
> > >
> > > Not all of the firmware files are a part of the public linux-firmware
> > > tree. However we need to support the rootfs bundled with the firmware
> > > for different platforms (both public and vendor). The non-signed files
> > > come from the Adreno GPU and can be shared between platforms. All
> > > other files are SoC-specific and in some cases device-specific.
> > >
> > > So for example the SDM845 db845c (open device) loads following firmware files:
> > > Not signed:
> > > - qcom/a630_sqe.fw
> > > - qcom/a630_gmu.bin
> > >
> > > Signed, will work for any non-secured sdm845 device:
> > > - qcom/sdm845/a630_zap.mbn
> > > - qcom/sdm845/adsp.mbn
> > > - qcom/sdm845/cdsp.mbn
> > > - qcom/sdm485/mba.mbn
> > > - qcom/sdm845/modem.mbn
> > > - qcom/sdm845/wlanmdsp.mbn (loaded via TQFTP)
> > > - qcom/venus-5.2/venus.mbn
> > >
> > > Signed, works only for DB845c.
> > > - qcom/sdm845/Thundercomm/db845c/slpi.mbn
> > >
> > > In comparison, the SDM845 Pixel-3 phone (aka blueline) should load the
> > > following firmware files:
> > > - qcom/a630_sqe.fw (the same, non-signed file)
> > > - qcom/a630_gmu.bin (the same, non-signed file)
> > > - qcom/sdm845/Google/blueline/a630_zap.mbn
> >
> > How do you get from "a630_zap.mbn" to this? By extending the lookup
> > table for every target, or what am I missing?
> 
> More or less so. Matching the root OF node gives us the firmware
> location, then it gets prepended to all firmware targets. Not an ideal
> solution, as there is no fallback support, but at least it gives us
> some points to discuss (and to decide whether to move to some
> particular direction or to abandon the idea completely, making Arnd
> unhappy again).
> 

I understand the desire to not put linux-firmware-specific paths in the
DeviceTree, but I think I'm less keen on having a big lookup table in
the kernel...

Regards,
Bjorn

Re: [PATCH 01/12] soc: qcom: add firmware name helper

[Kalle Valo]

Bjorn Andersson <andersson@kernel.org> writes:

> On Mon, May 27, 2024 at 02:42:44PM GMT, Dmitry Baryshkov wrote:
>
>> On Thu, 23 May 2024 at 01:48, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>> >
>> > On Tue, May 21, 2024 at 03:08:31PM +0200, Dmitry Baryshkov wrote:
>> > > On Tue, 21 May 2024 at 13:20, Kalle Valo <kvalo@kernel.org> wrote:
>> > > >
>> > > > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>> > > >
>> > > > > On Tue, 21 May 2024 at 12:52, <neil.armstrong@linaro.org> wrote:
>> > > > >>
>> > > > >> On 21/05/2024 11:45, Dmitry Baryshkov wrote:
>> > > > >> > Qualcomm platforms have different sets of the firmware files, which
>> > > > >> > differ from platform to platform (and from board to board, due to the
>> > > > >> > embedded signatures). Rather than listing all the firmware files,
>> > > > >> > including full paths, in the DT, provide a way to determine firmware
>> > > > >> > path based on the root DT node compatible.
>> > > > >>
>> > > > >> Ok this looks quite over-engineered but necessary to handle the legacy,
>> > > > >> but I really think we should add a way to look for a board-specific path
>> > > > >> first and fallback to those SoC specific paths.
>> > > > >
>> > > > > Again, CONFIG_FW_LOADER_USER_HELPER => delays.
>> > > >
>> > > > To me this also looks like very over-engineered, can you elaborate more
>> > > > why this is needed? Concrete examples would help to understand better.
>> > >
>> > > Sure. During the meeting last week Arnd suggested evaluating if we can
>> > > drop firmware-name from the board DT files. Several reasons for that:
>> > > - DT should describe the hardware, not the Linux-firmware locations
>> > > - having firmware name in DT complicates updating the tree to use
>> > > different firmware API (think of mbn vs mdt vs any other format)
>> > > - If the DT gets supplied by the vendor (e.g. for
>> > > SystemReady-certified devices), there should be a sync between the
>> > > vendor's DT, linux kernel and the rootfs. Dropping firmware names from
>> > > DT solves that by removing one piece of the equation
>> > >
>> > > Now for the complexity of the solution. Each SoC family has their own
>> > > firmware set. This includes firmware for the DSPs, for modem, WiFi
>> > > bits, GPU shader, etc.
>> > > For the development boards these devices are signed by the testing key
>> > > and the actual signature is not validated against the root of trust
>> > > certificate.
>> > > For the end-user devices the signature is actually validated against
>> > > the bits fused to the SoC during manufacturing process. CA certificate
>> > > (and thus the fuses) differ from vendor to vendor (and from the device
>> > > to device)
>> > >
>> > > Not all of the firmware files are a part of the public linux-firmware
>> > > tree. However we need to support the rootfs bundled with the firmware
>> > > for different platforms (both public and vendor). The non-signed files
>> > > come from the Adreno GPU and can be shared between platforms. All
>> > > other files are SoC-specific and in some cases device-specific.
>> > >
>> > > So for example the SDM845 db845c (open device) loads following firmware files:
>> > > Not signed:
>> > > - qcom/a630_sqe.fw
>> > > - qcom/a630_gmu.bin
>> > >
>> > > Signed, will work for any non-secured sdm845 device:
>> > > - qcom/sdm845/a630_zap.mbn
>> > > - qcom/sdm845/adsp.mbn
>> > > - qcom/sdm845/cdsp.mbn
>> > > - qcom/sdm485/mba.mbn
>> > > - qcom/sdm845/modem.mbn
>> > > - qcom/sdm845/wlanmdsp.mbn (loaded via TQFTP)
>> > > - qcom/venus-5.2/venus.mbn
>> > >
>> > > Signed, works only for DB845c.
>> > > - qcom/sdm845/Thundercomm/db845c/slpi.mbn
>> > >
>> > > In comparison, the SDM845 Pixel-3 phone (aka blueline) should load the
>> > > following firmware files:
>> > > - qcom/a630_sqe.fw (the same, non-signed file)
>> > > - qcom/a630_gmu.bin (the same, non-signed file)
>> > > - qcom/sdm845/Google/blueline/a630_zap.mbn
>> >
>> > How do you get from "a630_zap.mbn" to this? By extending the lookup
>> > table for every target, or what am I missing?
>> 
>> More or less so. Matching the root OF node gives us the firmware
>> location, then it gets prepended to all firmware targets. Not an ideal
>> solution, as there is no fallback support, but at least it gives us
>> some points to discuss (and to decide whether to move to some
>> particular direction or to abandon the idea completely, making Arnd
>> unhappy again).
>> 
>
> I understand the desire to not put linux-firmware-specific paths in the
> DeviceTree

Me too.

> but I think I'm less keen on having a big lookup table in the
> kernel...

Yeah, also for me this feels wrong. But on the other hand I don't have
anything better to suggest either...

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH 02/12] wifi: wcn36xx: make use of QCOM_FW_HELPER

[Dmitry Baryshkov]

Make the driver use qcom_fw_helper to autodetect the path to the
calibration data file.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/Kconfig | 1 +
 drivers/net/wireless/ath/wcn36xx/main.c  | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/Kconfig b/drivers/net/wireless/ath/wcn36xx/Kconfig
index 5832c7ef9352..90239c89676a 100644
--- a/drivers/net/wireless/ath/wcn36xx/Kconfig
+++ b/drivers/net/wireless/ath/wcn36xx/Kconfig
@@ -4,6 +4,7 @@ config WCN36XX
 	depends on MAC80211 && HAS_DMA
 	depends on QCOM_WCNSS_CTRL || QCOM_WCNSS_CTRL=n
 	depends on RPMSG || RPMSG=n
+	select QCOM_FW_HELPER
 	help
 	  This module adds support for wireless adapters based on
 	  Qualcomm Atheros WCN3660 and WCN3680 mobile chipsets.
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index e760d8002e09..8d25db81c1d0 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -22,6 +22,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/rpmsg.h>
+#include <linux/soc/qcom/fw_helper.h>
 #include <linux/soc/qcom/smem_state.h>
 #include <linux/soc/qcom/wcnss_ctrl.h>
 #include <net/ipv6.h>
@@ -1609,6 +1610,10 @@ static int wcn36xx_probe(struct platform_device *pdev)
 		goto out_wq;
 	}
 
+	wcn->nv_file = devm_qcom_get_board_fw(wcn->dev, wcn->nv_file);
+	if (!wcn->nv_file)
+		return -ENOMEM;
+
 	wcn->smd_channel = qcom_wcnss_open_channel(wcnss, "WLAN_CTRL", wcn36xx_smd_rsp_process, hw);
 	if (IS_ERR(wcn->smd_channel)) {
 		wcn36xx_err("failed to open WLAN_CTRL channel\n");

-- 
2.39.2

Re: [PATCH 02/12] wifi: wcn36xx: make use of QCOM_FW_HELPER

[Kalle Valo]

Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:

> Make the driver use qcom_fw_helper to autodetect the path to the
> calibration data file.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Not a fan of one sentence commit messages. It would be nice to explain a
bit more in the commit message, for instance answering to the question
'why?' and maybe provide a short example how this is supposed to work?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH 03/12] soc: qcom: wcnss_ctrl: make use of QCOM_FW_HELPER

[Dmitry Baryshkov]

Make the driver use qcom_fw_helper to autodetect the path to the
calibration data file.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/soc/qcom/Kconfig      | 1 +
 drivers/soc/qcom/wcnss_ctrl.c | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index b663774d65f8..3af3f15175e4 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -238,6 +238,7 @@ config QCOM_WCNSS_CTRL
 	tristate "Qualcomm WCNSS control driver"
 	depends on ARCH_QCOM || COMPILE_TEST
 	depends on RPMSG
+	select QCOM_FW_HELPER
 	help
 	  Client driver for the WCNSS_CTRL SMD channel, used to download nv
 	  firmware to a newly booted WCNSS chip.
diff --git a/drivers/soc/qcom/wcnss_ctrl.c b/drivers/soc/qcom/wcnss_ctrl.c
index 148bcbac332d..7d1a4536226a 100644
--- a/drivers/soc/qcom/wcnss_ctrl.c
+++ b/drivers/soc/qcom/wcnss_ctrl.c
@@ -12,6 +12,7 @@
 #include <linux/platform_device.h>
 #include <linux/rpmsg.h>
 #include <linux/soc/qcom/wcnss_ctrl.h>
+#include <linux/soc/qcom/fw_helper.h>
 
 #define WCNSS_REQUEST_TIMEOUT	(5 * HZ)
 #define WCNSS_CBC_TIMEOUT	(10 * HZ)
@@ -214,11 +215,19 @@ static int wcnss_download_nv(struct wcnss_ctrl *wcnss, bool *expect_cbc)
 	if (ret < 0 && ret != -EINVAL)
 		goto free_req;
 
+	nvbin = qcom_get_board_fw(nvbin);
+	if (!nvbin) {
+		ret = -ENOMEM;
+		goto free_req;
+	}
+
 	ret = request_firmware(&fw, nvbin, dev);
 	if (ret < 0) {
 		dev_err(dev, "Failed to load nv file %s: %d\n", nvbin, ret);
+		kfree(nvbin);
 		goto free_req;
 	}
+	kfree(nvbin);
 
 	data = fw->data;
 	left = fw->size;

-- 
2.39.2

[PATCH 04/12] remoteproc: qcom_q6v5_mss: switch to mbn files by default

[Dmitry Baryshkov]

We have been pushing userspace to use mbn files by default for ages.
As a preparation for making the firmware-name optional, make the driver
use .mbn instead of .mdt files by default.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/remoteproc/qcom_q6v5_mss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 1779fc890e10..eeaae2505352 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -2003,7 +2003,7 @@ static int q6v5_probe(struct platform_device *pdev)
 	qproc = rproc->priv;
 	qproc->dev = &pdev->dev;
 	qproc->rproc = rproc;
-	qproc->hexagon_mdt_image = "modem.mdt";
+	qproc->hexagon_mdt_image = "modem.mbn";
 	ret = of_property_read_string_index(pdev->dev.of_node, "firmware-name",
 					    1, &qproc->hexagon_mdt_image);
 	if (ret < 0 && ret != -EINVAL) {

-- 
2.39.2

[PATCH 05/12] remoteproc: qcom_q6v5_mss: make use of QCOM_FW_HELPER

[Dmitry Baryshkov]

Make the driver use qcom_fw_helper to autodetect the path to the
calibration data file.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/remoteproc/Kconfig         |  1 +
 drivers/remoteproc/qcom_q6v5_mss.c | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 48845dc8fa85..884e1e69bbb6 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -202,6 +202,7 @@ config QCOM_Q6V5_MSS
 	depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
 	depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
 	select MFD_SYSCON
+	select QCOM_FW_HELPER
 	select QCOM_MDT_LOADER
 	select QCOM_PIL_INFO
 	select QCOM_Q6V5_COMMON
diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index eeaae2505352..1ccd5bb92952 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -26,6 +26,7 @@
 #include <linux/remoteproc.h>
 #include <linux/reset.h>
 #include <linux/soc/qcom/mdt_loader.h>
+#include <linux/soc/qcom/fw_helper.h>
 #include <linux/iopoll.h>
 #include <linux/slab.h>
 
@@ -1990,8 +1991,13 @@ static int q6v5_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	mba_image = qcom_get_board_fw(mba_image);
+	if (!mba_image)
+		return -ENOMEM;
+
 	rproc = devm_rproc_alloc(&pdev->dev, pdev->name, &q6v5_ops,
 				 mba_image, sizeof(*qproc));
+	kfree(mba_image);
 	if (!rproc) {
 		dev_err(&pdev->dev, "failed to allocate rproc\n");
 		return -ENOMEM;
@@ -2011,6 +2017,10 @@ static int q6v5_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	qproc->hexagon_mdt_image = devm_qcom_get_board_fw(&pdev->dev, qproc->hexagon_mdt_image);
+	if (!qproc->hexagon_mdt_image)
+		return -ENOMEM;
+
 	platform_set_drvdata(pdev, qproc);
 
 	qproc->has_qaccept_regs = desc->has_qaccept_regs;

-- 
2.39.2

[PATCH 06/12] remoteproc: qcom_q6v5_pas: switch to mbn files by default

[Dmitry Baryshkov]

We have been pushing userspace to use mbn files by default for ages.
As a preparation for making the firmware-name optional, make the driver
use .mbn instead of .mdt files by default.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/remoteproc/qcom_q6v5_pas.c | 76 +++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 54d8005d40a3..4694ec4f038d 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -812,7 +812,7 @@ static void adsp_remove(struct platform_device *pdev)
 
 static const struct adsp_data adsp_resource_init = {
 	.crash_reason_smem = 423,
-	.firmware_name = "adsp.mdt",
+	.firmware_name = "adsp.mbn",
 	.pas_id = 1,
 	.auto_boot = true,
 	.ssr_name = "lpass",
@@ -822,7 +822,7 @@ static const struct adsp_data adsp_resource_init = {
 
 static const struct adsp_data sdm845_adsp_resource_init = {
 	.crash_reason_smem = 423,
-	.firmware_name = "adsp.mdt",
+	.firmware_name = "adsp.mbn",
 	.pas_id = 1,
 	.auto_boot = true,
 	.load_state = "adsp",
@@ -833,7 +833,7 @@ static const struct adsp_data sdm845_adsp_resource_init = {
 
 static const struct adsp_data sm6350_adsp_resource = {
 	.crash_reason_smem = 423,
-	.firmware_name = "adsp.mdt",
+	.firmware_name = "adsp.mbn",
 	.pas_id = 1,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -849,7 +849,7 @@ static const struct adsp_data sm6350_adsp_resource = {
 
 static const struct adsp_data sm6375_mpss_resource = {
 	.crash_reason_smem = 421,
-	.firmware_name = "modem.mdt",
+	.firmware_name = "modem.mbn",
 	.pas_id = 4,
 	.minidump_id = 3,
 	.auto_boot = false,
@@ -864,7 +864,7 @@ static const struct adsp_data sm6375_mpss_resource = {
 
 static const struct adsp_data sm8150_adsp_resource = {
 	.crash_reason_smem = 423,
-	.firmware_name = "adsp.mdt",
+	.firmware_name = "adsp.mbn",
 	.pas_id = 1,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -879,7 +879,7 @@ static const struct adsp_data sm8150_adsp_resource = {
 
 static const struct adsp_data sm8250_adsp_resource = {
 	.crash_reason_smem = 423,
-	.firmware_name = "adsp.mdt",
+	.firmware_name = "adsp.mbn",
 	.pas_id = 1,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -895,7 +895,7 @@ static const struct adsp_data sm8250_adsp_resource = {
 
 static const struct adsp_data sm8350_adsp_resource = {
 	.crash_reason_smem = 423,
-	.firmware_name = "adsp.mdt",
+	.firmware_name = "adsp.mbn",
 	.pas_id = 1,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -911,7 +911,7 @@ static const struct adsp_data sm8350_adsp_resource = {
 
 static const struct adsp_data msm8996_adsp_resource = {
 	.crash_reason_smem = 423,
-	.firmware_name = "adsp.mdt",
+	.firmware_name = "adsp.mbn",
 	.pas_id = 1,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -925,7 +925,7 @@ static const struct adsp_data msm8996_adsp_resource = {
 
 static const struct adsp_data cdsp_resource_init = {
 	.crash_reason_smem = 601,
-	.firmware_name = "cdsp.mdt",
+	.firmware_name = "cdsp.mbn",
 	.pas_id = 18,
 	.auto_boot = true,
 	.ssr_name = "cdsp",
@@ -935,7 +935,7 @@ static const struct adsp_data cdsp_resource_init = {
 
 static const struct adsp_data sdm845_cdsp_resource_init = {
 	.crash_reason_smem = 601,
-	.firmware_name = "cdsp.mdt",
+	.firmware_name = "cdsp.mbn",
 	.pas_id = 18,
 	.auto_boot = true,
 	.load_state = "cdsp",
@@ -946,7 +946,7 @@ static const struct adsp_data sdm845_cdsp_resource_init = {
 
 static const struct adsp_data sm6350_cdsp_resource = {
 	.crash_reason_smem = 601,
-	.firmware_name = "cdsp.mdt",
+	.firmware_name = "cdsp.mbn",
 	.pas_id = 18,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -962,7 +962,7 @@ static const struct adsp_data sm6350_cdsp_resource = {
 
 static const struct adsp_data sm8150_cdsp_resource = {
 	.crash_reason_smem = 601,
-	.firmware_name = "cdsp.mdt",
+	.firmware_name = "cdsp.mbn",
 	.pas_id = 18,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -977,7 +977,7 @@ static const struct adsp_data sm8150_cdsp_resource = {
 
 static const struct adsp_data sm8250_cdsp_resource = {
 	.crash_reason_smem = 601,
-	.firmware_name = "cdsp.mdt",
+	.firmware_name = "cdsp.mbn",
 	.pas_id = 18,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -992,7 +992,7 @@ static const struct adsp_data sm8250_cdsp_resource = {
 
 static const struct adsp_data sc8280xp_nsp0_resource = {
 	.crash_reason_smem = 601,
-	.firmware_name = "cdsp.mdt",
+	.firmware_name = "cdsp.mbn",
 	.pas_id = 18,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -1006,7 +1006,7 @@ static const struct adsp_data sc8280xp_nsp0_resource = {
 
 static const struct adsp_data sc8280xp_nsp1_resource = {
 	.crash_reason_smem = 633,
-	.firmware_name = "cdsp.mdt",
+	.firmware_name = "cdsp.mbn",
 	.pas_id = 30,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -1020,8 +1020,8 @@ static const struct adsp_data sc8280xp_nsp1_resource = {
 
 static const struct adsp_data x1e80100_adsp_resource = {
 	.crash_reason_smem = 423,
-	.firmware_name = "adsp.mdt",
-	.dtb_firmware_name = "adsp_dtb.mdt",
+	.firmware_name = "adsp.mbn",
+	.dtb_firmware_name = "adsp_dtb.mbn",
 	.pas_id = 1,
 	.dtb_pas_id = 0x24,
 	.lite_pas_id = 0x1f,
@@ -1040,8 +1040,8 @@ static const struct adsp_data x1e80100_adsp_resource = {
 
 static const struct adsp_data x1e80100_cdsp_resource = {
 	.crash_reason_smem = 601,
-	.firmware_name = "cdsp.mdt",
-	.dtb_firmware_name = "cdsp_dtb.mdt",
+	.firmware_name = "cdsp.mbn",
+	.dtb_firmware_name = "cdsp_dtb.mbn",
 	.pas_id = 18,
 	.dtb_pas_id = 0x25,
 	.minidump_id = 7,
@@ -1060,7 +1060,7 @@ static const struct adsp_data x1e80100_cdsp_resource = {
 
 static const struct adsp_data sm8350_cdsp_resource = {
 	.crash_reason_smem = 601,
-	.firmware_name = "cdsp.mdt",
+	.firmware_name = "cdsp.mbn",
 	.pas_id = 18,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -1076,7 +1076,7 @@ static const struct adsp_data sm8350_cdsp_resource = {
 
 static const struct adsp_data mpss_resource_init = {
 	.crash_reason_smem = 421,
-	.firmware_name = "modem.mdt",
+	.firmware_name = "modem.mbn",
 	.pas_id = 4,
 	.minidump_id = 3,
 	.auto_boot = false,
@@ -1093,7 +1093,7 @@ static const struct adsp_data mpss_resource_init = {
 
 static const struct adsp_data sc8180x_mpss_resource = {
 	.crash_reason_smem = 421,
-	.firmware_name = "modem.mdt",
+	.firmware_name = "modem.mbn",
 	.pas_id = 4,
 	.auto_boot = false,
 	.proxy_pd_names = (char*[]){
@@ -1108,7 +1108,7 @@ static const struct adsp_data sc8180x_mpss_resource = {
 
 static const struct adsp_data msm8996_slpi_resource_init = {
 	.crash_reason_smem = 424,
-	.firmware_name = "slpi.mdt",
+	.firmware_name = "slpi.mbn",
 	.pas_id = 12,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -1122,7 +1122,7 @@ static const struct adsp_data msm8996_slpi_resource_init = {
 
 static const struct adsp_data sdm845_slpi_resource_init = {
 	.crash_reason_smem = 424,
-	.firmware_name = "slpi.mdt",
+	.firmware_name = "slpi.mbn",
 	.pas_id = 12,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -1138,7 +1138,7 @@ static const struct adsp_data sdm845_slpi_resource_init = {
 
 static const struct adsp_data wcss_resource_init = {
 	.crash_reason_smem = 421,
-	.firmware_name = "wcnss.mdt",
+	.firmware_name = "wcnss.mbn",
 	.pas_id = 6,
 	.auto_boot = true,
 	.ssr_name = "mpss",
@@ -1148,7 +1148,7 @@ static const struct adsp_data wcss_resource_init = {
 
 static const struct adsp_data sdx55_mpss_resource = {
 	.crash_reason_smem = 421,
-	.firmware_name = "modem.mdt",
+	.firmware_name = "modem.mbn",
 	.pas_id = 4,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -1163,7 +1163,7 @@ static const struct adsp_data sdx55_mpss_resource = {
 
 static const struct adsp_data sm8450_mpss_resource = {
 	.crash_reason_smem = 421,
-	.firmware_name = "modem.mdt",
+	.firmware_name = "modem.mbn",
 	.pas_id = 4,
 	.minidump_id = 3,
 	.auto_boot = false,
@@ -1181,8 +1181,8 @@ static const struct adsp_data sm8450_mpss_resource = {
 
 static const struct adsp_data sm8550_adsp_resource = {
 	.crash_reason_smem = 423,
-	.firmware_name = "adsp.mdt",
-	.dtb_firmware_name = "adsp_dtb.mdt",
+	.firmware_name = "adsp.mbn",
+	.dtb_firmware_name = "adsp_dtb.mbn",
 	.pas_id = 1,
 	.dtb_pas_id = 0x24,
 	.minidump_id = 5,
@@ -1200,8 +1200,8 @@ static const struct adsp_data sm8550_adsp_resource = {
 
 static const struct adsp_data sm8550_cdsp_resource = {
 	.crash_reason_smem = 601,
-	.firmware_name = "cdsp.mdt",
-	.dtb_firmware_name = "cdsp_dtb.mdt",
+	.firmware_name = "cdsp.mbn",
+	.dtb_firmware_name = "cdsp_dtb.mbn",
 	.pas_id = 18,
 	.dtb_pas_id = 0x25,
 	.minidump_id = 7,
@@ -1220,8 +1220,8 @@ static const struct adsp_data sm8550_cdsp_resource = {
 
 static const struct adsp_data sm8550_mpss_resource = {
 	.crash_reason_smem = 421,
-	.firmware_name = "modem.mdt",
-	.dtb_firmware_name = "modem_dtb.mdt",
+	.firmware_name = "modem.mbn",
+	.dtb_firmware_name = "modem_dtb.mbn",
 	.pas_id = 4,
 	.dtb_pas_id = 0x26,
 	.minidump_id = 3,
@@ -1243,7 +1243,7 @@ static const struct adsp_data sm8550_mpss_resource = {
 
 static const struct adsp_data sc7280_wpss_resource = {
 	.crash_reason_smem = 626,
-	.firmware_name = "wpss.mdt",
+	.firmware_name = "wpss.mbn",
 	.pas_id = 6,
 	.auto_boot = true,
 	.proxy_pd_names = (char*[]){
@@ -1259,8 +1259,8 @@ static const struct adsp_data sc7280_wpss_resource = {
 
 static const struct adsp_data sm8650_cdsp_resource = {
 	.crash_reason_smem = 601,
-	.firmware_name = "cdsp.mdt",
-	.dtb_firmware_name = "cdsp_dtb.mdt",
+	.firmware_name = "cdsp.mbn",
+	.dtb_firmware_name = "cdsp_dtb.mbn",
 	.pas_id = 18,
 	.dtb_pas_id = 0x25,
 	.minidump_id = 7,
@@ -1283,8 +1283,8 @@ static const struct adsp_data sm8650_cdsp_resource = {
 
 static const struct adsp_data sm8650_mpss_resource = {
 	.crash_reason_smem = 421,
-	.firmware_name = "modem.mdt",
-	.dtb_firmware_name = "modem_dtb.mdt",
+	.firmware_name = "modem.mbn",
+	.dtb_firmware_name = "modem_dtb.mbn",
 	.pas_id = 4,
 	.dtb_pas_id = 0x26,
 	.minidump_id = 3,

-- 
2.39.2

Re: [PATCH 06/12] remoteproc: qcom_q6v5_pas: switch to mbn files by default

[neil.armstrong]

On 21/05/2024 11:45, Dmitry Baryshkov wrote:
> We have been pushing userspace to use mbn files by default for ages.
> As a preparation for making the firmware-name optional, make the driver
> use .mbn instead of .mdt files by default.

I think we should have a mechanism to fallback to .mdt since downstream
uses split mdt on the devices filesystem.

Perhaps only specify .firmware_name = "adsp" and add a list of allowed extension
it will try in a loop ?

Neil

> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>   drivers/remoteproc/qcom_q6v5_pas.c | 76 +++++++++++++++++++-------------------
>   1 file changed, 38 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 54d8005d40a3..4694ec4f038d 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -812,7 +812,7 @@ static void adsp_remove(struct platform_device *pdev)
>   
>   static const struct adsp_data adsp_resource_init = {
>   	.crash_reason_smem = 423,
> -	.firmware_name = "adsp.mdt",
> +	.firmware_name = "adsp.mbn",
>   	.pas_id = 1,
>   	.auto_boot = true,
>   	.ssr_name = "lpass",
> @@ -822,7 +822,7 @@ static const struct adsp_data adsp_resource_init = {
>   
>   static const struct adsp_data sdm845_adsp_resource_init = {
>   	.crash_reason_smem = 423,
> -	.firmware_name = "adsp.mdt",
> +	.firmware_name = "adsp.mbn",
>   	.pas_id = 1,
>   	.auto_boot = true,
>   	.load_state = "adsp",
> @@ -833,7 +833,7 @@ static const struct adsp_data sdm845_adsp_resource_init = {
>   
>   static const struct adsp_data sm6350_adsp_resource = {
>   	.crash_reason_smem = 423,
> -	.firmware_name = "adsp.mdt",
> +	.firmware_name = "adsp.mbn",
>   	.pas_id = 1,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -849,7 +849,7 @@ static const struct adsp_data sm6350_adsp_resource = {
>   
>   static const struct adsp_data sm6375_mpss_resource = {
>   	.crash_reason_smem = 421,
> -	.firmware_name = "modem.mdt",
> +	.firmware_name = "modem.mbn",
>   	.pas_id = 4,
>   	.minidump_id = 3,
>   	.auto_boot = false,
> @@ -864,7 +864,7 @@ static const struct adsp_data sm6375_mpss_resource = {
>   
>   static const struct adsp_data sm8150_adsp_resource = {
>   	.crash_reason_smem = 423,
> -	.firmware_name = "adsp.mdt",
> +	.firmware_name = "adsp.mbn",
>   	.pas_id = 1,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -879,7 +879,7 @@ static const struct adsp_data sm8150_adsp_resource = {
>   
>   static const struct adsp_data sm8250_adsp_resource = {
>   	.crash_reason_smem = 423,
> -	.firmware_name = "adsp.mdt",
> +	.firmware_name = "adsp.mbn",
>   	.pas_id = 1,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -895,7 +895,7 @@ static const struct adsp_data sm8250_adsp_resource = {
>   
>   static const struct adsp_data sm8350_adsp_resource = {
>   	.crash_reason_smem = 423,
> -	.firmware_name = "adsp.mdt",
> +	.firmware_name = "adsp.mbn",
>   	.pas_id = 1,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -911,7 +911,7 @@ static const struct adsp_data sm8350_adsp_resource = {
>   
>   static const struct adsp_data msm8996_adsp_resource = {
>   	.crash_reason_smem = 423,
> -	.firmware_name = "adsp.mdt",
> +	.firmware_name = "adsp.mbn",
>   	.pas_id = 1,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -925,7 +925,7 @@ static const struct adsp_data msm8996_adsp_resource = {
>   
>   static const struct adsp_data cdsp_resource_init = {
>   	.crash_reason_smem = 601,
> -	.firmware_name = "cdsp.mdt",
> +	.firmware_name = "cdsp.mbn",
>   	.pas_id = 18,
>   	.auto_boot = true,
>   	.ssr_name = "cdsp",
> @@ -935,7 +935,7 @@ static const struct adsp_data cdsp_resource_init = {
>   
>   static const struct adsp_data sdm845_cdsp_resource_init = {
>   	.crash_reason_smem = 601,
> -	.firmware_name = "cdsp.mdt",
> +	.firmware_name = "cdsp.mbn",
>   	.pas_id = 18,
>   	.auto_boot = true,
>   	.load_state = "cdsp",
> @@ -946,7 +946,7 @@ static const struct adsp_data sdm845_cdsp_resource_init = {
>   
>   static const struct adsp_data sm6350_cdsp_resource = {
>   	.crash_reason_smem = 601,
> -	.firmware_name = "cdsp.mdt",
> +	.firmware_name = "cdsp.mbn",
>   	.pas_id = 18,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -962,7 +962,7 @@ static const struct adsp_data sm6350_cdsp_resource = {
>   
>   static const struct adsp_data sm8150_cdsp_resource = {
>   	.crash_reason_smem = 601,
> -	.firmware_name = "cdsp.mdt",
> +	.firmware_name = "cdsp.mbn",
>   	.pas_id = 18,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -977,7 +977,7 @@ static const struct adsp_data sm8150_cdsp_resource = {
>   
>   static const struct adsp_data sm8250_cdsp_resource = {
>   	.crash_reason_smem = 601,
> -	.firmware_name = "cdsp.mdt",
> +	.firmware_name = "cdsp.mbn",
>   	.pas_id = 18,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -992,7 +992,7 @@ static const struct adsp_data sm8250_cdsp_resource = {
>   
>   static const struct adsp_data sc8280xp_nsp0_resource = {
>   	.crash_reason_smem = 601,
> -	.firmware_name = "cdsp.mdt",
> +	.firmware_name = "cdsp.mbn",
>   	.pas_id = 18,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -1006,7 +1006,7 @@ static const struct adsp_data sc8280xp_nsp0_resource = {
>   
>   static const struct adsp_data sc8280xp_nsp1_resource = {
>   	.crash_reason_smem = 633,
> -	.firmware_name = "cdsp.mdt",
> +	.firmware_name = "cdsp.mbn",
>   	.pas_id = 30,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -1020,8 +1020,8 @@ static const struct adsp_data sc8280xp_nsp1_resource = {
>   
>   static const struct adsp_data x1e80100_adsp_resource = {
>   	.crash_reason_smem = 423,
> -	.firmware_name = "adsp.mdt",
> -	.dtb_firmware_name = "adsp_dtb.mdt",
> +	.firmware_name = "adsp.mbn",
> +	.dtb_firmware_name = "adsp_dtb.mbn",
>   	.pas_id = 1,
>   	.dtb_pas_id = 0x24,
>   	.lite_pas_id = 0x1f,
> @@ -1040,8 +1040,8 @@ static const struct adsp_data x1e80100_adsp_resource = {
>   
>   static const struct adsp_data x1e80100_cdsp_resource = {
>   	.crash_reason_smem = 601,
> -	.firmware_name = "cdsp.mdt",
> -	.dtb_firmware_name = "cdsp_dtb.mdt",
> +	.firmware_name = "cdsp.mbn",
> +	.dtb_firmware_name = "cdsp_dtb.mbn",
>   	.pas_id = 18,
>   	.dtb_pas_id = 0x25,
>   	.minidump_id = 7,
> @@ -1060,7 +1060,7 @@ static const struct adsp_data x1e80100_cdsp_resource = {
>   
>   static const struct adsp_data sm8350_cdsp_resource = {
>   	.crash_reason_smem = 601,
> -	.firmware_name = "cdsp.mdt",
> +	.firmware_name = "cdsp.mbn",
>   	.pas_id = 18,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -1076,7 +1076,7 @@ static const struct adsp_data sm8350_cdsp_resource = {
>   
>   static const struct adsp_data mpss_resource_init = {
>   	.crash_reason_smem = 421,
> -	.firmware_name = "modem.mdt",
> +	.firmware_name = "modem.mbn",
>   	.pas_id = 4,
>   	.minidump_id = 3,
>   	.auto_boot = false,
> @@ -1093,7 +1093,7 @@ static const struct adsp_data mpss_resource_init = {
>   
>   static const struct adsp_data sc8180x_mpss_resource = {
>   	.crash_reason_smem = 421,
> -	.firmware_name = "modem.mdt",
> +	.firmware_name = "modem.mbn",
>   	.pas_id = 4,
>   	.auto_boot = false,
>   	.proxy_pd_names = (char*[]){
> @@ -1108,7 +1108,7 @@ static const struct adsp_data sc8180x_mpss_resource = {
>   
>   static const struct adsp_data msm8996_slpi_resource_init = {
>   	.crash_reason_smem = 424,
> -	.firmware_name = "slpi.mdt",
> +	.firmware_name = "slpi.mbn",
>   	.pas_id = 12,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -1122,7 +1122,7 @@ static const struct adsp_data msm8996_slpi_resource_init = {
>   
>   static const struct adsp_data sdm845_slpi_resource_init = {
>   	.crash_reason_smem = 424,
> -	.firmware_name = "slpi.mdt",
> +	.firmware_name = "slpi.mbn",
>   	.pas_id = 12,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -1138,7 +1138,7 @@ static const struct adsp_data sdm845_slpi_resource_init = {
>   
>   static const struct adsp_data wcss_resource_init = {
>   	.crash_reason_smem = 421,
> -	.firmware_name = "wcnss.mdt",
> +	.firmware_name = "wcnss.mbn",
>   	.pas_id = 6,
>   	.auto_boot = true,
>   	.ssr_name = "mpss",
> @@ -1148,7 +1148,7 @@ static const struct adsp_data wcss_resource_init = {
>   
>   static const struct adsp_data sdx55_mpss_resource = {
>   	.crash_reason_smem = 421,
> -	.firmware_name = "modem.mdt",
> +	.firmware_name = "modem.mbn",
>   	.pas_id = 4,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -1163,7 +1163,7 @@ static const struct adsp_data sdx55_mpss_resource = {
>   
>   static const struct adsp_data sm8450_mpss_resource = {
>   	.crash_reason_smem = 421,
> -	.firmware_name = "modem.mdt",
> +	.firmware_name = "modem.mbn",
>   	.pas_id = 4,
>   	.minidump_id = 3,
>   	.auto_boot = false,
> @@ -1181,8 +1181,8 @@ static const struct adsp_data sm8450_mpss_resource = {
>   
>   static const struct adsp_data sm8550_adsp_resource = {
>   	.crash_reason_smem = 423,
> -	.firmware_name = "adsp.mdt",
> -	.dtb_firmware_name = "adsp_dtb.mdt",
> +	.firmware_name = "adsp.mbn",
> +	.dtb_firmware_name = "adsp_dtb.mbn",
>   	.pas_id = 1,
>   	.dtb_pas_id = 0x24,
>   	.minidump_id = 5,
> @@ -1200,8 +1200,8 @@ static const struct adsp_data sm8550_adsp_resource = {
>   
>   static const struct adsp_data sm8550_cdsp_resource = {
>   	.crash_reason_smem = 601,
> -	.firmware_name = "cdsp.mdt",
> -	.dtb_firmware_name = "cdsp_dtb.mdt",
> +	.firmware_name = "cdsp.mbn",
> +	.dtb_firmware_name = "cdsp_dtb.mbn",
>   	.pas_id = 18,
>   	.dtb_pas_id = 0x25,
>   	.minidump_id = 7,
> @@ -1220,8 +1220,8 @@ static const struct adsp_data sm8550_cdsp_resource = {
>   
>   static const struct adsp_data sm8550_mpss_resource = {
>   	.crash_reason_smem = 421,
> -	.firmware_name = "modem.mdt",
> -	.dtb_firmware_name = "modem_dtb.mdt",
> +	.firmware_name = "modem.mbn",
> +	.dtb_firmware_name = "modem_dtb.mbn",
>   	.pas_id = 4,
>   	.dtb_pas_id = 0x26,
>   	.minidump_id = 3,
> @@ -1243,7 +1243,7 @@ static const struct adsp_data sm8550_mpss_resource = {
>   
>   static const struct adsp_data sc7280_wpss_resource = {
>   	.crash_reason_smem = 626,
> -	.firmware_name = "wpss.mdt",
> +	.firmware_name = "wpss.mbn",
>   	.pas_id = 6,
>   	.auto_boot = true,
>   	.proxy_pd_names = (char*[]){
> @@ -1259,8 +1259,8 @@ static const struct adsp_data sc7280_wpss_resource = {
>   
>   static const struct adsp_data sm8650_cdsp_resource = {
>   	.crash_reason_smem = 601,
> -	.firmware_name = "cdsp.mdt",
> -	.dtb_firmware_name = "cdsp_dtb.mdt",
> +	.firmware_name = "cdsp.mbn",
> +	.dtb_firmware_name = "cdsp_dtb.mbn",
>   	.pas_id = 18,
>   	.dtb_pas_id = 0x25,
>   	.minidump_id = 7,
> @@ -1283,8 +1283,8 @@ static const struct adsp_data sm8650_cdsp_resource = {
>   
>   static const struct adsp_data sm8650_mpss_resource = {
>   	.crash_reason_smem = 421,
> -	.firmware_name = "modem.mdt",
> -	.dtb_firmware_name = "modem_dtb.mdt",
> +	.firmware_name = "modem.mbn",
> +	.dtb_firmware_name = "modem_dtb.mbn",
>   	.pas_id = 4,
>   	.dtb_pas_id = 0x26,
>   	.minidump_id = 3,
> 

Re: [PATCH 06/12] remoteproc: qcom_q6v5_pas: switch to mbn files by default

[Dmitry Baryshkov]

On Tue, 21 May 2024 at 12:49, <neil.armstrong@linaro.org> wrote:
>
> On 21/05/2024 11:45, Dmitry Baryshkov wrote:
> > We have been pushing userspace to use mbn files by default for ages.
> > As a preparation for making the firmware-name optional, make the driver
> > use .mbn instead of .mdt files by default.
>
> I think we should have a mechanism to fallback to .mdt since downstream
> uses split mdt on the devices filesystem.
>
> Perhaps only specify .firmware_name = "adsp" and add a list of allowed extension
> it will try in a loop ?

Such loops can cause unnecessary delays if the
CONFIG_FW_LOADER_USER_HELPER is enabled.
Since it is not possible to use vendor's firmware partition as is (you
have to either bind-mount a subdir or use a plenty of symlinks) one
might as well symlink .mbn to .mdt file.
Another option is to explicitly specify something like `firmware-name
= "./adsp.mdt";'

But yes, this whole series is a balance of pros and cons, as it was
discussed last week.

-- 
With best wishes
Dmitry

Re: [PATCH 06/12] remoteproc: qcom_q6v5_pas: switch to mbn files by default

[Bjorn Andersson]

On Tue, May 21, 2024 at 11:49:42AM +0200, neil.armstrong@linaro.org wrote:
> On 21/05/2024 11:45, Dmitry Baryshkov wrote:
> > We have been pushing userspace to use mbn files by default for ages.
> > As a preparation for making the firmware-name optional, make the driver
> > use .mbn instead of .mdt files by default.
> 
> I think we should have a mechanism to fallback to .mdt since downstream
> uses split mdt on the devices filesystem.
> 

Let's ignore and continue to move away from the split .mdt files.

Combining split files is trivial and removes a class of problems where
people mix and match their parts. (And worst case you can rename/symlink
your downstream firmware to match the requested filename)

Regards,
Bjorn

[PATCH 07/12] remoteproc: qcom_q6v5_pas: make use of QCOM_FW_HELPER

[Dmitry Baryshkov]

Make the driver use qcom_fw_helper to autodetect the path to the
calibration data file.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/remoteproc/Kconfig         | 1 +
 drivers/remoteproc/qcom_q6v5_pas.c | 9 +++++++++
 2 files changed, 10 insertions(+)

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 884e1e69bbb6..7bb22fdb64e4 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -223,6 +223,7 @@ config QCOM_Q6V5_PAS
 	depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
 	depends on QCOM_AOSS_QMP || QCOM_AOSS_QMP=n
 	select MFD_SYSCON
+	select QCOM_FW_HELPER
 	select QCOM_PIL_INFO
 	select QCOM_MDT_LOADER
 	select QCOM_Q6V5_COMMON
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 4694ec4f038d..893fda54b598 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -22,6 +22,7 @@
 #include <linux/firmware/qcom/qcom_scm.h>
 #include <linux/regulator/consumer.h>
 #include <linux/remoteproc.h>
+#include <linux/soc/qcom/fw_helper.h>
 #include <linux/soc/qcom/mdt_loader.h>
 #include <linux/soc/qcom/smem.h>
 #include <linux/soc/qcom/smem_state.h>
@@ -705,11 +706,19 @@ static int adsp_probe(struct platform_device *pdev)
 						    &dtb_fw_name);
 		if (ret < 0 && ret != -EINVAL)
 			return ret;
+
+		dtb_fw_name = devm_qcom_get_board_fw(&pdev->dev, dtb_fw_name);
+		if (!dtb_fw_name)
+			return -ENOMEM;
 	}
 
 	if (desc->minidump_id)
 		ops = &adsp_minidump_ops;
 
+	fw_name = qcom_get_board_fw(fw_name);
+	if (!fw_name)
+		return -ENOMEM;
+
 	rproc = devm_rproc_alloc(&pdev->dev, pdev->name, ops, fw_name, sizeof(*adsp));
 
 	if (!rproc) {

-- 
2.39.2

[PATCH 08/12] remoteproc: qcom_wcnss: switch to mbn files by default

[Dmitry Baryshkov]

We have been pushing userspace to use mbn files by default for ages.
As a preparation for making the firmware-name optional, make the driver
use .mbn instead of .mdt files by default.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/remoteproc/qcom_wcnss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index a7bb9da27029..421a3943a90d 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -32,7 +32,7 @@
 #include "qcom_wcnss.h"
 
 #define WCNSS_CRASH_REASON_SMEM		422
-#define WCNSS_FIRMWARE_NAME		"wcnss.mdt"
+#define WCNSS_FIRMWARE_NAME		"wcnss.mbn"
 #define WCNSS_PAS_ID			6
 #define WCNSS_SSCTL_ID			0x13
 

-- 
2.39.2

[PATCH 09/12] remoteproc: qcom_wcnss: make use of QCOM_FW_HELPER

[Dmitry Baryshkov]

Make the driver use qcom_fw_helper to autodetect the path to the
calibration data file.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/remoteproc/qcom_wcnss.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
index 421a3943a90d..45fc578ae30b 100644
--- a/drivers/remoteproc/qcom_wcnss.c
+++ b/drivers/remoteproc/qcom_wcnss.c
@@ -23,6 +23,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/remoteproc.h>
 #include <linux/soc/qcom/mdt_loader.h>
+#include <linux/soc/qcom/fw_helper.h>
 #include <linux/soc/qcom/smem.h>
 #include <linux/soc/qcom/smem_state.h>
 
@@ -555,8 +556,13 @@ static int wcnss_probe(struct platform_device *pdev)
 	if (ret < 0 && ret != -EINVAL)
 		return ret;
 
+	fw_name = qcom_get_board_fw(fw_name);
+	if (!fw_name)
+		return -ENOMEM;
+
 	rproc = devm_rproc_alloc(&pdev->dev, pdev->name, &wcnss_ops,
 				 fw_name, sizeof(*wcnss));
+	kfree(fw_name);
 	if (!rproc) {
 		dev_err(&pdev->dev, "unable to allocate remoteproc\n");
 		return -ENOMEM;

-- 
2.39.2

Re: [PATCH 09/12] remoteproc: qcom_wcnss: make use of QCOM_FW_HELPER

[Jeff Johnson]

On 5/21/2024 2:45 AM, Dmitry Baryshkov wrote:
> Make the driver use qcom_fw_helper to autodetect the path to the
> calibration data file.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/remoteproc/qcom_wcnss.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
> index 421a3943a90d..45fc578ae30b 100644
> --- a/drivers/remoteproc/qcom_wcnss.c
> +++ b/drivers/remoteproc/qcom_wcnss.c
> @@ -23,6 +23,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/remoteproc.h>
>  #include <linux/soc/qcom/mdt_loader.h>
> +#include <linux/soc/qcom/fw_helper.h>
>  #include <linux/soc/qcom/smem.h>
>  #include <linux/soc/qcom/smem_state.h>
>  
> @@ -555,8 +556,13 @@ static int wcnss_probe(struct platform_device *pdev)
>  	if (ret < 0 && ret != -EINVAL)
>  		return ret;
>  
> +	fw_name = qcom_get_board_fw(fw_name);
> +	if (!fw_name)
> +		return -ENOMEM;
> +
>  	rproc = devm_rproc_alloc(&pdev->dev, pdev->name, &wcnss_ops,
>  				 fw_name, sizeof(*wcnss));
> +	kfree(fw_name);
>  	if (!rproc) {
>  		dev_err(&pdev->dev, "unable to allocate remoteproc\n");
>  		return -ENOMEM;
> 

can you cleanly bisect to this patch? seems it depends upon patch 10.
should 09 & 10 be swapped, or perhaps squashed?

Re: [PATCH 09/12] remoteproc: qcom_wcnss: make use of QCOM_FW_HELPER

[Dmitry Baryshkov]

On Wed, 22 May 2024 at 22:22, Jeff Johnson <quic_jjohnson@quicinc.com> wrote:
>
> On 5/21/2024 2:45 AM, Dmitry Baryshkov wrote:
> > Make the driver use qcom_fw_helper to autodetect the path to the
> > calibration data file.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  drivers/remoteproc/qcom_wcnss.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
> > index 421a3943a90d..45fc578ae30b 100644
> > --- a/drivers/remoteproc/qcom_wcnss.c
> > +++ b/drivers/remoteproc/qcom_wcnss.c
> > @@ -23,6 +23,7 @@
> >  #include <linux/regulator/consumer.h>
> >  #include <linux/remoteproc.h>
> >  #include <linux/soc/qcom/mdt_loader.h>
> > +#include <linux/soc/qcom/fw_helper.h>
> >  #include <linux/soc/qcom/smem.h>
> >  #include <linux/soc/qcom/smem_state.h>
> >
> > @@ -555,8 +556,13 @@ static int wcnss_probe(struct platform_device *pdev)
> >       if (ret < 0 && ret != -EINVAL)
> >               return ret;
> >
> > +     fw_name = qcom_get_board_fw(fw_name);
> > +     if (!fw_name)
> > +             return -ENOMEM;
> > +
> >       rproc = devm_rproc_alloc(&pdev->dev, pdev->name, &wcnss_ops,
> >                                fw_name, sizeof(*wcnss));
> > +     kfree(fw_name);
> >       if (!rproc) {
> >               dev_err(&pdev->dev, "unable to allocate remoteproc\n");
> >               return -ENOMEM;
> >
>
> can you cleanly bisect to this patch? seems it depends upon patch 10.
> should 09 & 10 be swapped, or perhaps squashed?

Yes. I think I got this mixed during rebasing and squashing of the
changes. For v2, if the approach is found to be generally acceptable,
I'll squash them.

-- 
With best wishes
Dmitry

[PATCH 10/12] remoteproc: qcom_wcnss: make use of QCOM_FW_HELPER

[Dmitry Baryshkov]

Make the driver use qcom_fw_helper to autodetect the path to the
calibration data file.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/remoteproc/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 7bb22fdb64e4..e0ffcaeca03d 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -279,6 +279,7 @@ config QCOM_WCNSS_PIL
 	depends on QCOM_SMEM
 	depends on QCOM_SYSMON || QCOM_SYSMON=n
 	depends on RPMSG_QCOM_GLINK || RPMSG_QCOM_GLINK=n
+	select QCOM_FW_HELPER
 	select QCOM_MDT_LOADER
 	select QCOM_PIL_INFO
 	select QCOM_RPROC_COMMON

-- 
2.39.2

[PATCH 11/12] arm64: dts: qcom: apq8016-sbc: drop firmware-name properties

[Dmitry Baryshkov]

As the drivers default to loading the firmware from the board-specific
location, drop the firmware-name properties. In case of the WCNSS
calibration data drop the path to the file, retaining just the file
name.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/apq8016-sbc.dts | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
index aba08424aa38..24779238cc18 100644
--- a/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
+++ b/arch/arm64/boot/dts/qcom/apq8016-sbc.dts
@@ -260,8 +260,6 @@ &mdss_dsi0_out {
 
 &mpss {
 	status = "okay";
-
-	firmware-name = "qcom/apq8016/mba.mbn", "qcom/apq8016/modem.mbn";
 };
 
 &mpss_mem {
@@ -388,11 +386,10 @@ &venus_mem {
 
 &wcnss {
 	status = "okay";
-	firmware-name = "qcom/apq8016/wcnss.mbn";
 };
 
 &wcnss_ctrl {
-	firmware-name = "qcom/apq8016/WCNSS_qcom_wlan_nv_sbc.bin";
+	firmware-name = "WCNSS_qcom_wlan_nv_sbc.bin";
 };
 
 &wcnss_iris {

-- 
2.39.2

[PATCH 12/12] arm64: dts: qcom: apq8096-db820c: drop firmware-name properties

[Dmitry Baryshkov]

As the drivers default to loading the firmware from the board-specific
location, drop the firmware-name properties.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/apq8096-db820c.dts | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/apq8096-db820c.dts b/arch/arm64/boot/dts/qcom/apq8096-db820c.dts
index e8148b3d6c50..2c8a77401aa3 100644
--- a/arch/arm64/boot/dts/qcom/apq8096-db820c.dts
+++ b/arch/arm64/boot/dts/qcom/apq8096-db820c.dts
@@ -161,7 +161,6 @@ bluetooth {
 
 &adsp_pil {
 	status = "okay";
-	firmware-name = "qcom/apq8096/adsp.mbn";
 };
 
 &blsp2_i2c1 {
@@ -253,7 +252,6 @@ &mmcc {
 &mss_pil {
 	status = "okay";
 	pll-supply = <&vreg_l12a_1p8>;
-	firmware-name = "qcom/apq8096/mba.mbn", "qcom/apq8096/modem.mbn";
 };
 
 &pm8994_resin {

-- 
2.39.2