On 24/01/2025 11:53, Viken Dadhaniya wrote:
> /* Common SE registers */
> @@ -891,6 +896,445 @@ int geni_icc_disable(struct geni_se *se)
> }
> EXPORT_SYMBOL_GPL(geni_icc_disable);
>
> +/**
> + * elf_phdr_valid: Function to validate elf header.
> + * @phdr: A pointer to a elf header.
> + *
> + * This function validates elf header by comparing fields
Drop "This function" and use imperative. It's redundant and you keep
using it everywherre here
...
> +static int qup_fw_load(struct qup_se_rsc *rsc, const char *fw_name)
> +{
> + int ret;
> + const struct firmware *fw;
> + struct device *dev = rsc->se->dev;
> +
> + ret = request_firmware(&fw, fw_name, dev);
> + if (ret) {
> + dev_err(dev, "request_firmware failed for %d: %d\n", rsc->protocol, ret);
> + return ret;
> + }
> +
> + ret = (rsc->protocol != GENI_SE_NONE) ? geni_load_se_fw(rsc, fw) : -EINVAL;
Drop ternary operator. Not easy to read.
> +
> + release_firmware(fw);
> +
> + return ret;
> +}
> +
> +/**
> + * geni_load_se_firmware: Function to initiate firmware loading.
> + * @se: Serial engine details.
> + * @protocol: protocol from spi, i2c or uart for which firmware to
> + * be loaded
> + *
> + * This function is called from the probe function of protocol driver.
> + * if dtsi properties are configured to load QUP firmware and firmware
> + * is already not loaded, it will start firmware loading. if dtsi
> + * properties are not defined,it will skip loading firmware assuming
> + * it is already loaded by TZ.
> + *
> + * return: Return 0 if no error, else return error value.
> + */
> +int geni_load_se_firmware(struct geni_se *se,
> + enum geni_se_protocol_type protocol)
> +{
> + struct qup_se_rsc rsc;
> + const char *fw_name;
> + int ret;
> +
> + ret = device_property_read_string(se->wrapper->dev, "firmware-name", &fw_name);
> + if (ret)
> + return -EINVAL;
> +
> + rsc.se = se;
> + rsc.protocol = protocol;
> +
> + /* Set default xfer mode to FIFO*/
> + rsc.mode = GENI_SE_FIFO;
> + of_property_read_u32(se->dev->of_node, "qcom,xfer-mode", &rsc.mode);
> + switch (rsc.mode) {
> + case GENI_SE_FIFO:
> + case GENI_SE_DMA:
How value of 2 is acceptable? Your bindings said it is not.
> + case GENI_GPI_DMA:
> + break;
> + default:
> + dev_err(se->dev, "Invalid xfer mode specified: %d\n", rsc.mode);
> + return -EINVAL;
> + }
> +
> + ret = qup_fw_load(&rsc, fw_name);
> + if (ret) {
> + dev_err(se->dev, "Firmware Loading failed for proto: %s Error: %d\n",
> + protocol_name[rsc.protocol], ret);
Aren't you printing same error multiple times?
> + return ret;
> + }
> +
> + dev_dbg(se->dev, "Firmware load for %s protocol is Success for xfer mode %d\n",
> + protocol_name[rsc.protocol], rsc.mode);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(geni_load_se_firmware);
> +
> static int geni_se_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> diff --git a/include/linux/soc/qcom/geni-se.h b/include/linux/soc/qcom/geni-se.h
> index 2996a3c28ef3..289fa6675d2b 100644
> --- a/include/linux/soc/qcom/geni-se.h
> +++ b/include/linux/soc/qcom/geni-se.h
> @@ -1,6 +1,7 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> /*
> * Copyright (c) 2017-2018, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2023-2025 Qualcomm Innovation Center, Inc. All rights reserved.
> */
>
> #ifndef _LINUX_QCOM_GENI_SE
> @@ -72,6 +73,19 @@ struct geni_se {
> struct geni_icc_path icc_paths[3];
> };
>
> +/**
> + * struct qup_se_rsc - Structure containing se details protocol and xfer mode
> + *
> + * @mode: transfer mode se fifo, dma or gsi.
> + * @protocol: Protocol spi or i2c or serial.
> + * @se: Pointer to the concerned serial engine.
> + */
> +struct qup_se_rsc {
> + struct geni_se *se;
> + enum geni_se_xfer_mode mode;
> + enum geni_se_protocol_type protocol;
> +};
> +
> /* Common SE registers */
> #define GENI_FORCE_DEFAULT_REG 0x20
> #define GENI_OUTPUT_CTRL 0x24
> @@ -531,5 +545,8 @@ void geni_icc_set_tag(struct geni_se *se, u32 tag);
> int geni_icc_enable(struct geni_se *se);
>
> int geni_icc_disable(struct geni_se *se);
> +
> +int geni_load_se_firmware(struct geni_se *se,
> + enum geni_se_protocol_type protocol);
> #endif
> #endif
> diff --git a/include/linux/soc/qcom/qup-fw-load.h b/include/linux/soc/qcom/qup-fw-load.h
> new file mode 100644
> index 000000000000..b9b58e81f5cb
> --- /dev/null
> +++ b/include/linux/soc/qcom/qup-fw-load.h
> @@ -0,0 +1,179 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +#ifndef _LINUX_QCOM_QUP_FW_LOAD
> +#define _LINUX_QCOM_QUP_FW_LOAD
> +
> +#include <linux/device.h>
> +#include <linux/elf.h>
> +#include <linux/firmware.h>
> +#include <linux/kernel.h>
> +
> +/*Magic numbers*/
> +#define MAGIC_NUM_SE 0x57464553
> +
> +/* Common SE registers*/
> +#define GENI_INIT_CFG_REVISION 0x0
> +#define GENI_S_INIT_CFG_REVISION 0x4
> +#define GENI_FORCE_DEFAULT_REG 0x20
> +#define GENI_CGC_CTRL 0x28
> +#define GENI_CFG_REG0 0x100
> +
> +#define QUPV3_SE_HW_PARAM_1 0xE28
Drop indentation after 'define'
Best regards,
Krzysztof
