Hi,
1. Keem Bay: in subject is wrong. Tools are working with it and you
should just use keembay: instead.
2. This should come first before actual change to keep the tree bisectable.
On 06. 10. 20 17:55, muhammad.husaini.zulkifli@xxxxxxxxx wrote:
> From: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@xxxxxxxxx>
>
> Add header file to handle API function for device driver to communicate
> with Arm Trusted Firmware.
>
> Signed-off-by: Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@xxxxxxxxx>
> ---
> .../linux/firmware/intel/keembay_firmware.h | 46 +++++++++++++++++++
> 1 file changed, 46 insertions(+)
> create mode 100644 include/linux/firmware/intel/keembay_firmware.h
>
> diff --git a/include/linux/firmware/intel/keembay_firmware.h b/include/linux/firmware/intel/keembay_firmware.h
> new file mode 100644
> index 000000000000..9adb8c87b788
> --- /dev/null
> +++ b/include/linux/firmware/intel/keembay_firmware.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Intel Keembay SOC Firmware API Layer
> + *
> + * Copyright (C) 2020-2021, Intel Corporation
> + *
> + * Muhammad Husaini Zulkifli <Muhammad.Husaini.Zulkifli@xxxxxxxxx>
> + */
> +
> +#ifndef __FIRMWARE_KEEMBAY_SMC_H__
> +#define __FIRMWARE_KEEMBAY_SMC_H__
> +
> +#include <linux/arm-smccc.h>
> +
> +/**
This is not a kernel doc comment. Just use /*
> + * This file defines API function that can be called by device driver in order to
> + * communicate with Arm Trusted Firmware.
> + */
> +
> +/* Setting for Keem Bay IO Pad Line Voltage Selection */
> +#define KEEMBAY_SET_SD_VOLTAGE_FUNC_ID 0x8200ff26
Sudeep: Don't we have any macros for composing these IDs?
nit: IMHO composing these IDs from macros would make more sense to me.
> +#define KEEMBAY_SET_1V8_VOLT 0x01
0x01 is just 1
> +#define KEEMBAY_SET_3V3_VOLT 0x00
0x00 is just 0
> +
> +#if IS_ENABLED(CONFIG_HAVE_ARM_SMCCC_DISCOVERY)
> +static int do_fw_invoke(u64 func_id, u64 arg0, u64 arg1)
> +{
> + struct arm_smccc_res res;
> +
> + arm_smccc_1_1_invoke(func_id, arg0, arg1, &res);
> +
> + return res.a0;
I am not big fan of this error propagation in case of failure.
If smc fails you get via res.a0 SMCCC_RET_NOT_SUPPORTED which is defined
as -1 which is based on errno-base.h defined as EPERM.
That driver which Sudeep pointed you to is using EINVAL instead.
It means I would add a code to check it.
> +}
> +
> +int keembay_sd_voltage_selection(int volt)
as was reported by robot
> +{
> + return do_fw_invoke(KEEMBAY_SET_SD_VOLTAGE_FUNC_ID, volt, 0);
> +}
> +#else
> +static inline int keembay_sd_voltage_selection(int volt)
> +{
> + return -ENODEV;
> +}
> +#endif
> +
> +#endif /* __FIRMWARE_KEEMBAY_SMC_H__ */
>
M
