Re: [PATCH 1/3] firmware: zynqmp: Add support to access efuses

[Stefan Roese <sr@denx.de>]

Hi Lukas,

On 5/14/24 16:04, lukas.funke-oss@weidmueller.com wrote:
> From: Lukas Funke <lukas.funke@weidmueller.com>
> 
> Add functions to access efuses through PMU firmware
> interface.
> 
> Signed-off-by: Lukas Funke <lukas.funke@weidmueller.com>
> ---
> 
>   drivers/firmware/firmware-zynqmp.c | 31 ++++++++++++++++++++++++++++++
>   include/zynqmp_firmware.h          |  2 ++
>   2 files changed, 33 insertions(+)
> 
> diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
> index f99507d86c6..7483f2a8709 100644
> --- a/drivers/firmware/firmware-zynqmp.c
> +++ b/drivers/firmware/firmware-zynqmp.c
> @@ -210,6 +210,37 @@ int zynqmp_pm_feature(const u32 api_id)
>   	return ret_payload[1] & FIRMWARE_VERSION_MASK;
>   }
>   
> +int zynqmp_pm_get_chipid(u32 *idcode, u32 *version)
> +{
> +	int ret;
> +	u32 ret_payload[PAYLOAD_ARG_CNT];

Reverse x-mas tree ordering looks better IMHO.

> +
> +	if (!idcode || !version)
> +		return -EINVAL;
> +
> +	ret = xilinx_pm_request(PM_GET_CHIPID, 0, 0, 0, 0, ret_payload);
> +	*idcode = ret_payload[1];
> +	*version = ret_payload[2];

You don't check ret for an error above but still pass the return values
here. Perhaps it makes sense to return with error above instead?

> +
> +	return ret;
> +}
> +
> +int zynqmp_pm_efuse_access(const u64 address, u32 *out)
> +{
> +	int ret;
> +	u32 ret_payload[PAYLOAD_ARG_CNT];
> +
> +	if (!out)
> +		return -EINVAL;
> +
> +	ret = xilinx_pm_request(PM_EFUSE_ACCESS, upper_32_bits(address),
> +				lower_32_bits(address), 0, 0, ret_payload);

Same here.

Thanks,
Stefan

> +
> +	*out = ret_payload[1];
> +
> +	return ret;
> +}
> +
>   int zynqmp_pm_is_function_supported(const u32 api_id, const u32 id)
>   {
>   	int ret;
> diff --git a/include/zynqmp_firmware.h b/include/zynqmp_firmware.h
> index 73198a6a6ea..7f18b4d59bf 100644
> --- a/include/zynqmp_firmware.h
> +++ b/include/zynqmp_firmware.h
> @@ -453,6 +453,8 @@ int xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
>   int zynqmp_pm_set_sd_config(u32 node, enum pm_sd_config_type config, u32 value);
>   int zynqmp_pm_set_gem_config(u32 node, enum pm_gem_config_type config,
>   			     u32 value);
> +int zynqmp_pm_get_chipid(u32 *idcode, u32 *version);
> +int zynqmp_pm_efuse_access(const u64 address, u32 *out);
>   int zynqmp_pm_is_function_supported(const u32 api_id, const u32 id);
>   int zynqmp_mmio_read(const u32 address, u32 *value);
>   int zynqmp_mmio_write(const u32 address, const u32 mask, const u32 value);

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de