Re: [PATCH v3 3/3] pinctrl: Add Xilinx ZynqMP pinctrl driver support

[Nobuhiro Iwamatsu <iwamatsu@xxxxxxxxxxx>]

Hi,

2021年2月12日(金) 21:10 Sai Krishna Potthuri
<lakshmi.sai.krishna.potthuri@xxxxxxxxxx>:
>
> Adding pinctrl driver for Xilinx ZynqMP platform.
> This driver queries pin information from firmware and registers
> pin control accordingly.
>
> Signed-off-by: Sai Krishna Potthuri <lakshmi.sai.krishna.potthuri@xxxxxxxxxx>
> ---
>  drivers/pinctrl/Kconfig          |   13 +
>  drivers/pinctrl/Makefile         |    1 +
>  drivers/pinctrl/pinctrl-zynqmp.c | 1031 ++++++++++++++++++++++++++++++
>  3 files changed, 1045 insertions(+)
>  create mode 100644 drivers/pinctrl/pinctrl-zynqmp.c
>
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 815095326e2d..25d3c7208975 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -341,6 +341,19 @@ config PINCTRL_ZYNQ
>         help
>           This selects the pinctrl driver for Xilinx Zynq.
>
> +config PINCTRL_ZYNQMP
> +       bool "Pinctrl driver for Xilinx ZynqMP"
> +       depends on ARCH_ZYNQMP
> +       select PINMUX
> +       select GENERIC_PINCONF
> +       help
> +         This selects the pinctrl driver for Xilinx ZynqMP platform.
> +         This driver will query the pin information from the firmware
> +         and allow configuring the pins.
> +         Configuration can include the mux function to select on those
> +         pin(s)/group(s), and various pin configuration parameters
> +         such as pull-up, slew rate, etc.
> +
>  config PINCTRL_INGENIC
>         bool "Pinctrl driver for the Ingenic JZ47xx SoCs"
>         default MACH_INGENIC
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index f53933b2ff02..7e058739f0d5 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_PINCTRL_TB10X)   += pinctrl-tb10x.o
>  obj-$(CONFIG_PINCTRL_ST)       += pinctrl-st.o
>  obj-$(CONFIG_PINCTRL_STMFX)    += pinctrl-stmfx.o
>  obj-$(CONFIG_PINCTRL_ZYNQ)     += pinctrl-zynq.o
> +obj-$(CONFIG_PINCTRL_ZYNQMP)    += pinctrl-zynqmp.o
>  obj-$(CONFIG_PINCTRL_INGENIC)  += pinctrl-ingenic.o
>  obj-$(CONFIG_PINCTRL_RK805)    += pinctrl-rk805.o
>  obj-$(CONFIG_PINCTRL_OCELOT)   += pinctrl-ocelot.o
> diff --git a/drivers/pinctrl/pinctrl-zynqmp.c b/drivers/pinctrl/pinctrl-zynqmp.c
> new file mode 100644
> index 000000000000..ec0a5d0e22d5
> --- /dev/null
> +++ b/drivers/pinctrl/pinctrl-zynqmp.c
> @@ -0,0 +1,1031 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ZynqMP pin controller
> + *
> + *  Copyright (C) 2020 Xilinx, Inc.
> + *
> + *  Sai Krishna Potthuri <lakshmi.sai.krishna.potthuri@xxxxxxxxxx>
> + *  Rajan Vaja <rajanv@xxxxxxxxxx>
> + */

<snip>

> +/**
> + * zynqmp_pinctrl_get_function_name() - get function name
> + * @fid:       Function ID.
> + * @name:      Function name
> + *
> + * Call firmware API to get name of given function.
> + *
> + * Return: 0 on success else error code.
> + */
> +static int zynqmp_pinctrl_get_function_name(u32 fid, char *name)
> +{
> +       struct zynqmp_pm_query_data qdata = {0};
> +       u32 ret_payload[PAYLOAD_ARG_CNT];
> +
> +       qdata.qid = PM_QID_PINCTRL_GET_FUNCTION_NAME;
> +       qdata.arg1 = fid;
> +
> +       zynqmp_pm_query_data(qdata, ret_payload);

Please check the return value here as well as other functions.

I know that when we used zynqmp_pm_query_data with
PM_QID_PINCTRL_GET_FUNCTION_NAME,
it returns -22 error code.
How about adding processing with zynqmp_pm_query_data like
PM_QID_CLOCK_GET_NAME or
writing a comment here?

Best regards,
  Nobuhiro

-- 
Nobuhiro Iwamatsu
   iwamatsu at {nigauri.org / debian.org}
   GPG ID: 40AD1FA6