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

[Sai Krishna Potthuri <lakshmis@xxxxxxxxxx>]

Hi Nobuhiro,

> -----Original Message-----
> From: Nobuhiro Iwamatsu <iwamatsu@xxxxxxxxxxx>
> Sent: Sunday, February 28, 2021 6:19 AM
> To: Sai Krishna Potthuri <lakshmis@xxxxxxxxxx>
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>; Rob Herring
> <robh+dt@xxxxxxxxxx>; Michal Simek <michals@xxxxxxxxxx>; Greg Kroah-
> Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; linux ARM <linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx>; Linux Kernel Mailing List <linux-
> kernel@xxxxxxxxxxxxxxx>; devicetree@xxxxxxxxxxxxxxx; linux-
> gpio@xxxxxxxxxxxxxxx; git <git@xxxxxxxxxx>; saikrishna12468@xxxxxxxxx
> Subject: Re: [PATCH v3 3/3] pinctrl: Add Xilinx ZynqMP pinctrl driver support
> 
> 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?
I will add comment for this kind of intentional cases.

Regards
Sai Krishna 
> 
> Best regards,
>   Nobuhiro
> 
> --
> Nobuhiro Iwamatsu
>    iwamatsu at {nigauri.org / debian.org}
>    GPG ID: 40AD1FA6