2023-07-24 09:39:37

by Potthuri, Sai Krishna

[permalink] [raw]
Subject: [PATCH v2 0/4] pinctrl: pinctrl-zynqmp: Add tri-state configuration support

Add pinctrl driver support to handle 'output-enable' and
'bias-high-impedance' configurations with proper Configuration Set version
check. This will ensure system not to crash even if older Xilinx ZynqMP
Platform Management Firmware is used.
Initial Commit details:
Commit 133ad0d9af99bdca9070 ("dt-bindings: pinctrl-zynqmp: Add
output-enable configuration").
Commit ad2bea79ef0144043721 ("pinctrl: pinctrl-zynqmp: Add support
for output-enable and bias-high-impedance").

With the above patches, using these pinctrl properties in the device-tree
cause system hang issues with older Xilinx ZynqMP Platform Management
Firmware, hence reverted the patches.
Reverted Commit details:
Commit ff8356060e3a5e126abb ("Revert "dt-bindings: pinctrl-zynqmp: Add
output-enable configuration"").
Commit 9989bc33c4894e075167 ("Revert "pinctrl: pinctrl-zynqmp: Add support
for output-enable and bias-high-impedance"").
With the latest firmware and driver changes, driver will ask firmware if
that feature is supported or not by checking the version. This way it
works with all Xilinx firmwares.

changes in v2:
- Updated commit description in 3/4 patch as suggested by Conor Dooley.

Dhaval Shah (1):
firmware: xilinx: Add support to get platform information

Sai Krishna Potthuri (3):
firmware: xilinx: Add version check for TRISTATE configuration
dt-bindings: pinctrl-zynqmp: Add output-enable configuration
pinctrl: pinctrl-zynqmp: Add support for output-enable and bias-high
impedance

.../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 4 ++
drivers/firmware/xilinx/zynqmp.c | 51 +++++++++++++++++++
drivers/pinctrl/pinctrl-zynqmp.c | 9 ++++
include/linux/firmware/xlnx-zynqmp.h | 15 ++++++
4 files changed, 79 insertions(+)

--
2.25.1



2023-07-24 09:41:40

by Potthuri, Sai Krishna

[permalink] [raw]
Subject: [PATCH v2 4/4] pinctrl: pinctrl-zynqmp: Add support for output-enable and bias-high impedance

Add support to handle 'output-enable' and 'bias-high-impedance'
configurations.

Using these pinctrl properties observed hang issues with older PMUFW(Xilinx
ZynqMP Platform Management Firmware), hence reverted the patch.
Commit 9989bc33c4894e075167 ("Revert "pinctrl: pinctrl-zynqmp: Add support
for output-enable and bias-high-impedance"").

Support for configuring these properties added in PMUFW Configuration Set
version 2.0. When there is a request for these configurations from pinctrl
driver for ZynqMP platform, xilinx firmware driver checks for this version
before configuring these properties to avoid the hang issue and proceeds
further only when firmware version is >=2 otherwise it returns error.

Signed-off-by: Sai Krishna Potthuri <[email protected]>
---
drivers/pinctrl/pinctrl-zynqmp.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/drivers/pinctrl/pinctrl-zynqmp.c b/drivers/pinctrl/pinctrl-zynqmp.c
index 8d2cb0999f2f..f2be341f73e1 100644
--- a/drivers/pinctrl/pinctrl-zynqmp.c
+++ b/drivers/pinctrl/pinctrl-zynqmp.c
@@ -415,6 +415,10 @@ static int zynqmp_pinconf_cfg_set(struct pinctrl_dev *pctldev,

break;
case PIN_CONFIG_BIAS_HIGH_IMPEDANCE:
+ param = PM_PINCTRL_CONFIG_TRI_STATE;
+ arg = PM_PINCTRL_TRI_STATE_ENABLE;
+ ret = zynqmp_pm_pinctrl_set_config(pin, param, arg);
+ break;
case PIN_CONFIG_MODE_LOW_POWER:
/*
* These cases are mentioned in dts but configurable
@@ -423,6 +427,11 @@ static int zynqmp_pinconf_cfg_set(struct pinctrl_dev *pctldev,
*/
ret = 0;
break;
+ case PIN_CONFIG_OUTPUT_ENABLE:
+ param = PM_PINCTRL_CONFIG_TRI_STATE;
+ arg = PM_PINCTRL_TRI_STATE_DISABLE;
+ ret = zynqmp_pm_pinctrl_set_config(pin, param, arg);
+ break;
default:
dev_warn(pctldev->dev,
"unsupported configuration parameter '%u'\n",
--
2.25.1


2023-07-24 09:47:19

by Potthuri, Sai Krishna

[permalink] [raw]
Subject: [PATCH v2 2/4] firmware: xilinx: Add version check for TRISTATE configuration

Support for configuring TRISTATE parameter is added in ZYNQMP PMUFW(Xilinx
ZynqMP Platform Management Firmware) Configuration Param Set version 2.0.
If the requested configuration is TRISTATE and platform is ZYNQMP then
check the version before requesting Xilinx firmware to set the
configuration.

Signed-off-by: Sai Krishna Potthuri <[email protected]>
---
drivers/firmware/xilinx/zynqmp.c | 9 +++++++++
include/linux/firmware/xlnx-zynqmp.h | 2 ++
2 files changed, 11 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index f9498e7ea694..307717f24a98 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -1150,6 +1150,15 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_get_config);
int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
u32 value)
{
+ int ret;
+
+ if (pm_family_code == ZYNQMP_FAMILY_CODE &&
+ param == PM_PINCTRL_CONFIG_TRI_STATE) {
+ ret = zynqmp_pm_feature(PM_PINCTRL_CONFIG_PARAM_SET);
+ if (ret < PM_PINCTRL_PARAM_SET_VERSION)
+ return -EOPNOTSUPP;
+ }
+
return zynqmp_pm_invoke_fn(PM_PINCTRL_CONFIG_PARAM_SET, pin,
param, value, 0, NULL);
}
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index d7f94b42ad4c..6359eeea8dd7 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -34,6 +34,8 @@
/* PM API versions */
#define PM_API_VERSION_2 2

+#define PM_PINCTRL_PARAM_SET_VERSION 2
+
#define ZYNQMP_FAMILY_CODE 0x23
#define VERSAL_FAMILY_CODE 0x26

--
2.25.1


2023-07-24 11:05:15

by Michal Simek

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] firmware: xilinx: Add version check for TRISTATE configuration



On 7/24/23 10:45, Sai Krishna Potthuri wrote:
> Support for configuring TRISTATE parameter is added in ZYNQMP PMUFW(Xilinx
> ZynqMP Platform Management Firmware) Configuration Param Set version 2.0.
> If the requested configuration is TRISTATE and platform is ZYNQMP then
> check the version before requesting Xilinx firmware to set the
> configuration.
>
> Signed-off-by: Sai Krishna Potthuri <[email protected]>
> ---
> drivers/firmware/xilinx/zynqmp.c | 9 +++++++++
> include/linux/firmware/xlnx-zynqmp.h | 2 ++
> 2 files changed, 11 insertions(+)
>
> diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
> index f9498e7ea694..307717f24a98 100644
> --- a/drivers/firmware/xilinx/zynqmp.c
> +++ b/drivers/firmware/xilinx/zynqmp.c
> @@ -1150,6 +1150,15 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_get_config);
> int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
> u32 value)
> {
> + int ret;
> +
> + if (pm_family_code == ZYNQMP_FAMILY_CODE &&
> + param == PM_PINCTRL_CONFIG_TRI_STATE) {
> + ret = zynqmp_pm_feature(PM_PINCTRL_CONFIG_PARAM_SET);
> + if (ret < PM_PINCTRL_PARAM_SET_VERSION)
> + return -EOPNOTSUPP;
> + }
> +
> return zynqmp_pm_invoke_fn(PM_PINCTRL_CONFIG_PARAM_SET, pin,
> param, value, 0, NULL);
> }
> diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
> index d7f94b42ad4c..6359eeea8dd7 100644
> --- a/include/linux/firmware/xlnx-zynqmp.h
> +++ b/include/linux/firmware/xlnx-zynqmp.h
> @@ -34,6 +34,8 @@
> /* PM API versions */
> #define PM_API_VERSION_2 2
>
> +#define PM_PINCTRL_PARAM_SET_VERSION 2
> +
> #define ZYNQMP_FAMILY_CODE 0x23
> #define VERSAL_FAMILY_CODE 0x26
>

Reviewed-by: Michal Simek <[email protected]>

Thanks,
Michal

2023-07-27 05:50:38

by Potthuri, Sai Krishna

[permalink] [raw]
Subject: RE: [PATCH v2 2/4] firmware: xilinx: Add version check for TRISTATE configuration



From: Shah, Tanmay <[email protected]>
Sent: Monday, July 24, 2023 8:57 PM
To: Potthuri, Sai Krishna <[email protected]>; Linus Walleij <[email protected]>; Simek, Michal <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski <[email protected]>; Conor Dooley <[email protected]>; Mathieu Poirier <[email protected]>; Levinsky, Ben <[email protected]>; Marek Vasut <[email protected]>; Roman Gushchin <[email protected]>; Arnd Bergmann <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; git (AMD-Xilinx) <[email protected]>
Subject: Re: [PATCH v2 2/4] firmware: xilinx: Add version check for TRISTATE configuration


On 7/24/23 3:45 AM, Sai Krishna Potthuri wrote:
Support for configuring TRISTATE parameter is added in ZYNQMP PMUFW(Xilinx
ZynqMP Platform Management Firmware) Configuration Param Set version 2.0.
If the requested configuration is TRISTATE and platform is ZYNQMP then
check the version before requesting Xilinx firmware to set the
configuration.

Signed-off-by: Sai Krishna Potthuri mailto:[email protected]
---
drivers/firmware/xilinx/zynqmp.c | 9 +++++++++
include/linux/firmware/xlnx-zynqmp.h | 2 ++
2 files changed, 11 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index f9498e7ea694..307717f24a98 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -1150,6 +1150,15 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_get_config);
int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
u32 value)
{
+ int ret;
+
+ if (pm_family_code == ZYNQMP_FAMILY_CODE &&
+ param == PM_PINCTRL_CONFIG_TRI_STATE) {
+ ret = zynqmp_pm_feature(PM_PINCTRL_CONFIG_PARAM_SET);
+ if (ret < PM_PINCTRL_PARAM_SET_VERSION)
+ return -EOPNOTSUPP;
Hi Sai,
If you get version 1 as response of feature_check then, it is possible to handle call as per old payload arguments?
something as following:
if (ret == 2) {
    //hanlde modified payload as per v2 of this call
} else if (ret == 1) {
    //handle original payload of this call as firmware is old.
} else {
    //fail here with -EOPNOTSUPP as we don't support this version.
}
This way we can maintain backward compatibility with old firmware.

Hi Tanmay,

TRISTATE feature support added in version 2.0, hence we are checking the SET PARAM
version if the requested configuration is TRISTATE. If the version is 2.0 then we are
allowing the call otherwise will return error. This change will not impact the other
configurations and not impact the backward compatibility as we are checking the
version only for the TRISTATE configuration.

Regards
Sai Krishna


+ }
+
return zynqmp_pm_invoke_fn(PM_PINCTRL_CONFIG_PARAM_SET, pin,
param, value, 0, NULL);
}
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index d7f94b42ad4c..6359eeea8dd7 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -34,6 +34,8 @@
/* PM API versions */
#define PM_API_VERSION_2 2

+#define PM_PINCTRL_PARAM_SET_VERSION 2
+
#define ZYNQMP_FAMILY_CODE 0x23
#define VERSAL_FAMILY_CODE 0x26