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

2021-04-15 Thread Sai Krishna Potthuri
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 
---
 drivers/pinctrl/Kconfig  |  14 +
 drivers/pinctrl/Makefile |   1 +
 drivers/pinctrl/pinctrl-zynqmp.c | 906 +++
 3 files changed, 921 insertions(+)
 create mode 100644 drivers/pinctrl/pinctrl-zynqmp.c

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 815095326e2d..736f5230590b 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -341,6 +341,20 @@ config PINCTRL_ZYNQ
help
  This selects the pinctrl driver for Xilinx Zynq.
 
+config PINCTRL_ZYNQMP
+   tristate "Pinctrl driver for Xilinx ZynqMP"
+   depends on ZYNQMP_FIRMWARE
+   select PINMUX
+   select GENERIC_PINCONF
+   default ZYNQMP_FIRMWARE
+   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..cd28423fd8d5 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 ..e5f3f5794c66
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-zynqmp.c
@@ -0,0 +1,906 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ZynqMP pin controller
+ *
+ * Copyright (C) 2020 Xilinx, Inc.
+ *
+ * Sai Krishna Potthuri 
+ * Rajan Vaja 
+ */
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include "core.h"
+#include "pinctrl-utils.h"
+
+#define ZYNQMP_PIN_PREFIX  "MIO"
+#define PINCTRL_GET_FUNC_NAME_RESP_LEN 16
+#define MAX_FUNC_NAME_LEN  16
+#define MAX_GROUP_PIN  50
+#define MAX_PIN_GROUPS 50
+#define END_OF_FUNCTIONS   "END_OF_FUNCTIONS"
+#define NUM_GROUPS_PER_RESP6
+
+#define PINCTRL_GET_FUNC_GROUPS_RESP_LEN   12
+#define PINCTRL_GET_PIN_GROUPS_RESP_LEN12
+#define NA_GROUP   0x
+#define RESERVED_GROUP 0xFFFE
+
+#define DRIVE_STRENGTH_2MA 2
+#define DRIVE_STRENGTH_4MA 4
+#define DRIVE_STRENGTH_8MA 8
+#define DRIVE_STRENGTH_12MA12
+
+/**
+ * struct zynqmp_pmux_function - a pinmux function
+ * @name:  Name of the pin mux function
+ * @groups:List of pin groups for this function
+ * @ngroups:   Number of entries in @groups
+ * @node:  Firmware node matching with the function
+ *
+ * This structure holds information about pin control function
+ * and function group names supporting that function.
+ */
+struct zynqmp_pmux_function {
+   char name[MAX_FUNC_NAME_LEN];
+   const char * const *groups;
+   unsigned int ngroups;
+};
+
+/**
+ * struct zynqmp_pinctrl - driver data
+ * @pctrl: Pin control device
+ * @groups:Pin groups
+ * @ngroups:   Number of @groups
+ * @funcs: Pin mux functions
+ * @nfuncs:Number of @funcs
+ *
+ * This struct is stored as driver data and used to retrieve
+ * information regarding pin control functions, groups and
+ * group pins.
+ */
+struct zynqmp_pinctrl {
+   struct pinctrl_dev *pctrl;
+   const struct zynqmp_pctrl_group *groups;
+   unsigned int ngroups;
+   const struct zynqmp_pmux_function *funcs;
+   unsigned int nfuncs;
+};
+
+/**
+ * struct zynqmp_pctrl_group - Pin control group info
+ * @name:  Group name
+ * @pins:  Group pin numbers
+ * @npins: Number of pins in the group
+ */
+struct zynqmp_pctrl_group {
+   const char *name;
+   unsigned int pins[MAX_GROUP_PIN];
+   unsigned int npins;
+};
+
+static struct pinctrl_desc zynqmp_desc;
+
+static int zynqmp_pctrl_get_groups_count(struct pinctrl_dev *pctldev)
+{
+   struct zynqmp_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+
+   return pctrl->ngroups;
+}
+
+static const char *zynqmp_pctrl_get_group_name(struct pinctrl_dev *p

[PATCH v5 2/3] dt-bindings: pinctrl: Add binding for ZynqMP pinctrl driver

2021-04-15 Thread Sai Krishna Potthuri
Adding documentation and dt-bindings file which contains MIO pin
configuration defines for Xilinx ZynqMP pinctrl driver.

Signed-off-by: Sai Krishna Potthuri 
Reviewed-by: Rob Herring 
---
 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 336 ++
 include/dt-bindings/pinctrl/pinctrl-zynqmp.h  |  19 +
 2 files changed, 355 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
 create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h

diff --git a/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml 
b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
new file mode 100644
index ..8ef0d07d35fe
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
@@ -0,0 +1,336 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx ZynqMP Pinctrl
+
+maintainers:
+  - Sai Krishna Potthuri 
+  - Rajan Vaja 
+
+description: |
+  Please refer to pinctrl-bindings.txt in this directory for details of the
+  common pinctrl bindings used by client devices, including the meaning of the
+  phrase "pin configuration node".
+
+  ZynqMP's pin configuration nodes act as a container for an arbitrary number 
of
+  subnodes. Each of these subnodes represents some desired configuration for a
+  pin, a group, or a list of pins or groups. This 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.
+
+  Each configuration node can consist of multiple nodes describing the pinmux 
and
+  pinconf options. Those nodes can be pinmux nodes or pinconf nodes.
+
+  The name of each subnode is not important; all subnodes should be enumerated
+  and processed purely based on their content.
+
+properties:
+  compatible:
+const: xlnx,zynqmp-pinctrl
+
+patternProperties:
+  '^(.*-)?(default|gpio)$':
+type: object
+patternProperties:
+  '^mux':
+type: object
+description:
+  Pinctrl node's client devices use subnodes for pin muxes,
+  which in turn use below standard properties.
+$ref: pinmux-node.yaml#
+
+properties:
+  groups:
+description:
+  List of groups to select (either this or "pins" must be
+  specified), available groups for this subnode.
+items:
+  enum: [ethernet0_0_grp, ethernet1_0_grp, ethernet2_0_grp,
+ ethernet3_0_grp, gemtsu0_0_grp, gemtsu0_1_grp,
+ gemtsu0_2_grp, mdio0_0_grp, mdio1_0_grp,
+ mdio1_1_grp, mdio2_0_grp, mdio3_0_grp,
+ qspi0_0_grp, qspi_ss_0_grp, qspi_fbclk_0_grp,
+ spi0_0_grp, spi0_ss_0_grp, spi0_ss_1_grp,
+ spi0_ss_2_grp, spi0_1_grp, spi0_ss_3_grp,
+ spi0_ss_4_grp, spi0_ss_5_grp, spi0_2_grp,
+ spi0_ss_6_grp, spi0_ss_7_grp, spi0_ss_8_grp,
+ spi0_3_grp, spi0_ss_9_grp, spi0_ss_10_grp,
+ spi0_ss_11_grp, spi0_4_grp, spi0_ss_12_grp,
+ spi0_ss_13_grp, spi0_ss_14_grp, spi0_5_grp,
+ spi0_ss_15_grp, spi0_ss_16_grp, spi0_ss_17_grp,
+ spi1_0_grp, spi1_ss_0_grp, spi1_ss_1_grp,
+ spi1_ss_2_grp, spi1_1_grp, spi1_ss_3_grp,
+ spi1_ss_4_grp, spi1_ss_5_grp, spi1_2_grp,
+ spi1_ss_6_grp, spi1_ss_7_grp, spi1_ss_8_grp,
+ spi1_3_grp, spi1_ss_9_grp, spi1_ss_10_grp,
+ spi1_ss_11_grp, spi1_4_grp, spi1_ss_12_grp,
+ spi1_ss_13_grp, spi1_ss_14_grp, spi1_5_grp,
+ spi1_ss_15_grp, spi1_ss_16_grp, spi1_ss_17_grp,
+ sdio0_0_grp, sdio0_1_grp, sdio0_2_grp,
+ sdio0_3_grp, sdio0_4_grp, sdio0_5_grp,
+ sdio0_6_grp, sdio0_7_grp, sdio0_8_grp,
+ sdio0_9_grp, sdio0_10_grp, sdio0_11_grp,
+ sdio0_12_grp, sdio0_13_grp, sdio0_14_grp,
+ sdio0_15_grp, sdio0_16_grp, sdio0_17_grp,
+ sdio0_18_grp, sdio0_19_grp, sdio0_20_grp,
+ sdio0_21_grp, sdio0_22_grp, sdio0_23_grp,
+ sdio0_24_grp, sdio0_25_grp, sdio0_26_grp,
+ sdio0_27_grp, sdio0_28_grp, sdio0_29_grp,
+ sdio0_30_grp, sdio0_31_grp, sdio0_32_grp,
+ sdio0_pc_0_grp, sdio0_cd_0_grp, sdio0_wp_0_grp,
+ sdio0_pc_1_grp, sdio0_cd_1_grp, sdio0_wp_1_grp,
+ sdio0_pc_2_grp, sdio0_cd_2_grp, sdio0_wp_2_grp,
+ sdio1_0_grp, sdio1_1_grp, sdio1_2_grp,
+ sdio1_3_grp, sdio

[PATCH v5 0/3] Add ZynqMP pinctrl driver

2021-04-15 Thread Sai Krishna Potthuri
Add support for Xilinx ZynqMP pinctrl driver and also update
the Xilinx firmware driver to support pinctrl functionality.
This driver queries the pin information from the firmware and
allow configuring the pins as per the request.

changes in v5:
- Used generic property 'power-source' instead of driver specific for
configuring the IO voltages, updated the same in dt-binding.
- Added support to build driver as a module.
- Used error codes returned by the Xilinx firmware instead of shadowing the
error codes in the driver.
- Fixed comments from Andy related to spell checks, NULL checks, explicit
typecast, header inclusion ordering, removing kernel docs for the
obvious ones.

changes in v4:
- Added comment for ignoring the return value for GET_FUNCTION_NAME qid.
- Updated the zynqmp_pinctrl_get_function_name() API prototype to void as
it always returns zero.

changes in v3:
- Fixed binding doc comments from Rob.
- Used 'maxItems' for groups and pins properties.
- Updated commit subject and description to have present tense statements.

changes in v2:
- Use pattern for pin names in yaml file.
- Updated to support multiple groups and pins.
- Added type ref for the vendor specific properties.
- Removed 'schmitt-cmos', instead used common properties.
- Removed macros for drive-strength property.

Sai Krishna Potthuri (3):
  firmware: xilinx: Add pinctrl support
  dt-bindings: pinctrl: Add binding for ZynqMP pinctrl driver
  pinctrl: Add Xilinx ZynqMP pinctrl driver support

 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 336 +++
 drivers/firmware/xilinx/zynqmp.c  | 114 +++
 drivers/pinctrl/Kconfig   |  14 +
 drivers/pinctrl/Makefile  |   1 +
 drivers/pinctrl/pinctrl-zynqmp.c  | 906 ++
 include/dt-bindings/pinctrl/pinctrl-zynqmp.h  |  19 +
 include/linux/firmware/xlnx-zynqmp.h  |  90 ++
 7 files changed, 1480 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
 create mode 100644 drivers/pinctrl/pinctrl-zynqmp.c
 create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h

-- 
2.17.1



[PATCH v5 1/3] firmware: xilinx: Add pinctrl support

2021-04-15 Thread Sai Krishna Potthuri
Adding pinctrl support to query platform specific information (pins)
from firmware.

Signed-off-by: Sai Krishna Potthuri 
Acked-by: Michal Simek 
---
 drivers/firmware/xilinx/zynqmp.c | 114 +++
 include/linux/firmware/xlnx-zynqmp.h |  90 +
 2 files changed, 204 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index efb8a66efc68..299c3d5a9ebd 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -784,6 +784,120 @@ int zynqmp_pm_fpga_get_status(u32 *value)
 }
 EXPORT_SYMBOL_GPL(zynqmp_pm_fpga_get_status);
 
+/**
+ * zynqmp_pm_pinctrl_request - Request Pin from firmware
+ * @pin: Pin number to request
+ *
+ * This function requests pin from firmware.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_request(const u32 pin)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_REQUEST, pin, 0, 0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_request);
+
+/**
+ * zynqmp_pm_pinctrl_release - Inform firmware that Pin control is released
+ * @pin: Pin number to release
+ *
+ * This function release pin from firmware.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_release(const u32 pin)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_RELEASE, pin, 0, 0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_release);
+
+/**
+ * zynqmp_pm_pinctrl_get_function - Read function id set for the given pin
+ * @pin: Pin number
+ * @id: Buffer to store function ID
+ *
+ * This function provides the function currently set for the given pin.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+int zynqmp_pm_pinctrl_get_function(const u32 pin, u32 *id)
+{
+   u32 ret_payload[PAYLOAD_ARG_CNT];
+   int ret;
+
+   if (!id)
+   return -EINVAL;
+
+   ret = zynqmp_pm_invoke_fn(PM_PINCTRL_GET_FUNCTION, pin, 0,
+ 0, 0, ret_payload);
+   *id = ret_payload[1];
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_get_function);
+
+/**
+ * zynqmp_pm_pinctrl_set_function - Set requested function for the pin
+ * @pin: Pin number
+ * @id: Function ID to set
+ *
+ * This function sets requested function for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_set_function(const u32 pin, const u32 id)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_SET_FUNCTION, pin, id,
+  0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_function);
+
+/**
+ * zynqmp_pm_pinctrl_get_config - Get configuration parameter for the pin
+ * @pin: Pin number
+ * @param: Parameter to get
+ * @value: Buffer to store parameter value
+ *
+ * This function gets requested configuration parameter for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_get_config(const u32 pin, const u32 param,
+u32 *value)
+{
+   u32 ret_payload[PAYLOAD_ARG_CNT];
+   int ret;
+
+   if (!value)
+   return -EINVAL;
+
+   ret = zynqmp_pm_invoke_fn(PM_PINCTRL_CONFIG_PARAM_GET, pin, param,
+ 0, 0, ret_payload);
+   *value = ret_payload[1];
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_get_config);
+
+/**
+ * zynqmp_pm_pinctrl_set_config - Set configuration parameter for the pin
+ * @pin: Pin number
+ * @param: Parameter to set
+ * @value: Parameter value to set
+ *
+ * This function sets requested configuration parameter for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
+u32 value)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_CONFIG_PARAM_SET, pin,
+  param, value, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_config);
+
 /**
  * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller
  *master has initialized its own power management
diff --git a/include/linux/firmware/xlnx-zynqmp.h 
b/include/linux/firmware/xlnx-zynqmp.h
index 5968df82b991..75aa6a5afa28 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -74,6 +74,12 @@ enum pm_api_id {
PM_FPGA_LOAD,
PM_FPGA_GET_STATUS,
PM_GET_CHIPID = 24,
+   PM_PINCTRL_REQUEST = 28,
+   PM_PINCTRL_RELEASE = 29,
+   PM_PINCTRL_GET_FUNCTION = 30,
+   PM_PINCTRL_SET_FUNCTION = 31,
+   PM_PINCTRL_CONFIG_PARAM_GET = 32,
+   PM_PINCTRL_CONFIG_PARAM_SET = 33,
PM_IOCTL = 34,
PM_QUERY_DATA,
PM_CLOCK_ENABLE,
@@ -125,6 +131,12 @@ enum pm_query_id {
PM_QID_CLOCK_GET_FIXEDFACTOR_PARAMS,
PM_QID_CLOCK_GET_PARENTS,
PM_QID_CLOCK_GET_ATTRIBUTES

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

2021-03-25 Thread Sai Krishna Potthuri
Hi Linus,

> -Original Message-
> From: Linus Walleij 
> Sent: Thursday, March 25, 2021 2:27 PM
> To: Sai Krishna Potthuri 
> Cc: Andy Shevchenko ; Rob Herring
> ; Michal Simek ; Greg Kroah-
> Hartman ; linux-arm Mailing List  ker...@lists.infradead.org>; Linux Kernel Mailing List  ker...@vger.kernel.org>; devicetree ; open
> list:GPIO SUBSYSTEM ; git ;
> saikrishna12...@gmail.com
> Subject: Re: [PATCH v4 3/3] pinctrl: Add Xilinx ZynqMP pinctrl driver support
> 
> On Mon, Mar 22, 2021 at 4:25 PM Sai Krishna Potthuri
>  wrote:
> > [Andy]
> 
> > > > > > +   PIN_CONFIG_IOSTANDARD = PIN_CONFIG_END + 1, };
> > > > >
> > > > > I'm lost here. What is IO standard exactly? Why it can't be in
> > > > > generic headers?
> > > > It represents LVCMOS 3.3 volts/ LVCMOS 1.8 volts.
> > > > Since this is specific to Xilinx ZynqMP platform, considered to be
> > > > added in the driver file.
> > >
> > > So, why can't we create a couple of bits to represent this voltages
> > > in the generic header and gain usability for others as well?
> >
> > I see some drivers are maintaining the configuration list in the
> > driver file, if the configuration is specific to the driver.
> >
> > We can move this to generic header if it is used by others as well.
> > Ok, will wait for Linus to comment.
> > >
> > > Linus?
> 
> While it is fine to add custom pin config options to pin controllers for
> hopelessly idiomatic stuff, this does look like it should be using
> PIN_CONFIG_POWER_SOURCE with the voltage rail as parameter, see
> include/linux/pinctrl/pinconf-generic.h
Thanks.
I will update the driver to use 'power-source' option.

Regards
Sai Krishna
> 
> If you're not using that then tell us why.
> 
> Yours,
> Linus Walleij


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

2021-03-23 Thread Sai Krishna Potthuri
Hi Andy Shevchenko,

> -Original Message-
> From: Andy Shevchenko 
> Sent: Monday, March 22, 2021 10:47 PM
> To: Sai Krishna Potthuri 
> Cc: Linus Walleij ; Rob Herring
> ; Michal Simek ; Greg Kroah-
> Hartman ; linux-arm Mailing List  ker...@lists.infradead.org>; Linux Kernel Mailing List  ker...@vger.kernel.org>; devicetree ; open
> list:GPIO SUBSYSTEM ; git ;
> saikrishna12...@gmail.com
> Subject: Re: [PATCH v4 3/3] pinctrl: Add Xilinx ZynqMP pinctrl driver support
> 
> On Mon, Mar 22, 2021 at 5:25 PM Sai Krishna Potthuri
>  wrote:
> > > From: Andy Shevchenko 
> > > Sent: Friday, March 19, 2021 3:53 PM On Thu, Mar 18, 2021 at 4:42 PM
> > > Sai Krishna Potthuri 
> > > wrote:
> > > > > From: Andy Shevchenko 
> > > > > Sent: Wednesday, March 17, 2021 6:26 PM On Wed, Mar 17, 2021 at
> > > > > 10:27 AM Sai Krishna Potthuri
> > > > >  wrote:
> 
> ...
> 
> > > > #include 
> > >
> > > Looking into other drivers with similar includes, shouldn't it go
> > > first in the file before any other linux/* asm/* etc?
> > I see some drivers are including this header before linux/* and some
> > are using after linux/*.
> 
> The rule of thumb is that: more generic headers are going first.
> 
> I consider dt/* ones are more generic than linux/* ones because they are
> covering more than just the Linux kernel.
Ok, I will reorder accordingly.
> 
> ...
> 
> > > > > I'm lost here. What is IO standard exactly? Why it can't be in
> > > > > generic headers?
> > > > It represents LVCMOS 3.3 volts/ LVCMOS 1.8 volts.
> > > > Since this is specific to Xilinx ZynqMP platform, considered to be
> > > > added in the driver file.
> > >
> > > So, why can't we create a couple of bits to represent this voltages
> > > in the generic header and gain usability for others as well?
> > I see some drivers are maintaining the configuration list in the
> > driver file, if the configuration is specific to the driver.
> 
> Yes, my point is that this case doesn't sound too specific to the driver. Many
> pin control buffers (in hardware way of speaking) have properties to be
> different voltage tolerant / produce.
Ok, you want me to proceed with this change or shall we wait for
Linus to comment?
> 
> > We can move this to generic header if it is used by others as well.
> > Ok, will wait for Linus to comment.
> > >
> > > Linus?
> 
> ...
> 
> > > > > > +   ret = zynqmp_pm_pinctrl_request(pin);
> > > > > > +   if (ret) {
> > > > > > +   dev_err(pctldev->dev, "request failed for pin
> > > > > > + %u\n", pin);
> > > > >
> > > > > > +   return -EIO;
> > > > >
> > > > > Why shadowing error code?
> > >
> > > So, any comments on the initial Q?
> > Xilinx low level secure firmware error codes are different from Linux error
> codes.
> > Secure firmware maintains list of error codes (positive values other than
> zero).
> > Hence we return -EIO, if the return value from firmware is non-zero.
> 
> Why the zynqmp_pm_pinctrl_request() can't return codes in Linux error
> code space?
I cross checked with the Xilinx firmware team, firmware driver is now returning
Linux error codes in the latest kernel but while developing the driver it was a
different approach. I will update the driver to simply return the values from
firmware calls.
> 
> > > >>  Since it's the only possible error, why is it not
> > > > > reflected in the kernel doc?
> > > > I will update the kernel doc with the error value for such cases.
> > > > >
> > > > > > +   }
> 
> --
> With Best Regards,
> Andy Shevchenko


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

2021-03-22 Thread Sai Krishna Potthuri
Hi Andy Shevchenko,

> -Original Message-
> From: Andy Shevchenko 
> Sent: Friday, March 19, 2021 3:53 PM
> To: Sai Krishna Potthuri 
> Cc: Linus Walleij ; Rob Herring
> ; Michal Simek ; Greg Kroah-
> Hartman ; linux-arm Mailing List  ker...@lists.infradead.org>; Linux Kernel Mailing List  ker...@vger.kernel.org>; devicetree ; open
> list:GPIO SUBSYSTEM ; git ;
> saikrishna12...@gmail.com
> Subject: Re: [PATCH v4 3/3] pinctrl: Add Xilinx ZynqMP pinctrl driver support
> 
> On Thu, Mar 18, 2021 at 4:42 PM Sai Krishna Potthuri 
> wrote:
> > > From: Andy Shevchenko 
> > > Sent: Wednesday, March 17, 2021 6:26 PM On Wed, Mar 17, 2021 at
> > > 10:27 AM Sai Krishna Potthuri
> > >  wrote:
> 
> ...
> 
> > > > +config PINCTRL_ZYNQMP
> > > > +   bool "Pinctrl driver for Xilinx ZynqMP"
> > >
> > > Why not module?
> > As most of the Xilinx drivers depending on the pin controller driver
> > for configuring the MIO pins, we are not supporting to build this
> > driver as a module.
> 
> OK.
> 
> > > > +   depends on ARCH_ZYNQMP
> > > > +   select PINMUX
> > > > +   select GENERIC_PINCONF
> 
> ...
> 
> > > > +#include 
> > > > +#include 
> > >
> > > > +#include  #include
> > > > +
> > >
> > > Can you move this group of headers after linux/* ?
> > >
> > > > +#include 
> > >
> > > Can it be moved outside of these headers
> > >
> > > > +#include  #include
> > > > +
> > >
> > > Ordering (for all groups of headers)?
> > Ok, I will order the headers in the below order #include 
> > #include 
> 
> + blank line
> 
> > #include 
> 
> + blank line
Ok, I will add above two blank lines.
> 
> > #include 
> 
> Looking into other drivers with similar includes, shouldn't it go first in 
> the file
> before any other linux/* asm/* etc?
I see some drivers are including this header before linux/* and some are using
after linux/*.
> 
> > > > +#include "core.h"
> > > > +#include "pinctrl-utils.h"
> 
> ...
> 
> > > > +   PIN_CONFIG_IOSTANDARD = PIN_CONFIG_END + 1, };
> > >
> > > I'm lost here. What is IO standard exactly? Why it can't be in
> > > generic headers?
> > It represents LVCMOS 3.3 volts/ LVCMOS 1.8 volts.
> > Since this is specific to Xilinx ZynqMP platform, considered to be
> > added in the driver file.
> 
> So, why can't we create a couple of bits to represent this voltages in the
> generic header and gain usability for others as well?
I see some drivers are maintaining the configuration list in the driver file, if
the configuration is specific to the driver.
We can move this to generic header if it is used by others as well.
Ok, will wait for Linus to comment.
> 
> Linus?
> 
> ...
> 
> > > > +   ret = zynqmp_pm_pinctrl_request(pin);
> > > > +   if (ret) {
> > > > +   dev_err(pctldev->dev, "request failed for pin
> > > > + %u\n", pin);
> > >
> > > > +   return -EIO;
> > >
> > > Why shadowing error code?
> 
> So, any comments on the initial Q?
Xilinx low level secure firmware error codes are different from Linux error 
codes.
Secure firmware maintains list of error codes (positive values other than zero).
Hence we return -EIO, if the return value from firmware is non-zero. 
> 
> >>  Since it's the only possible error, why is it not
> > > reflected in the kernel doc?
> > I will update the kernel doc with the error value for such cases.
> > >
> > > > +   }
> 
> ...
> 
> > > > +   default:
> > > > +   /* Invalid drive strength */
> > > > +   dev_warn(pctldev->dev,
> > > > +"Invalid drive strength for pin %d\n",
> > > > +pin);
> > > > +   return -EINVAL;
> > > > +   }
> > > > +   break;
> > > > +   default:
> > > > +   ret = -EOPNOTSUPP;
> > > > +   break;
> > > > +   }
> > > > +
> > > > +   param = pinconf_to_config_param(*config);
> > > > +   *config = pinconf_to_config_packed(param, arg);
> > > > +
> > > > +   return ret;
>

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

2021-03-18 Thread Sai Krishna Potthuri
Hi Andy Shevchenko,

Thanks for the review.

> -Original Message-
> From: Andy Shevchenko 
> Sent: Wednesday, March 17, 2021 6:26 PM
> To: Sai Krishna Potthuri 
> Cc: Linus Walleij ; Rob Herring
> ; Michal Simek ; Greg Kroah-
> Hartman ; linux-arm Mailing List  ker...@lists.infradead.org>; Linux Kernel Mailing List  ker...@vger.kernel.org>; devicetree ; open
> list:GPIO SUBSYSTEM ; git ;
> saikrishna12...@gmail.com
> Subject: Re: [PATCH v4 3/3] pinctrl: Add Xilinx ZynqMP pinctrl driver support
> 
> On Wed, Mar 17, 2021 at 10:27 AM Sai Krishna Potthuri
>  wrote:
> >
> > 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
> > 
> > ---
> >  drivers/pinctrl/Kconfig  |   13 +
> >  drivers/pinctrl/Makefile |1 +
> >  drivers/pinctrl/pinctrl-zynqmp.c | 1030
> > ++
> >  3 files changed, 1044 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"
> 
> Why not module?
As most of the Xilinx drivers depending on the pin controller driver for
configuring the MIO pins, we are not supporting to build this driver as
a module. 
> 
> > +   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 ..63edde400e85
> > --- /dev/null
> > +++ b/drivers/pinctrl/pinctrl-zynqmp.c
> > @@ -0,0 +1,1030 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * ZynqMP pin controller
> > + *
> > + *  Copyright (C) 2020 Xilinx, Inc.
> > + *
> > + *  Sai Krishna Potthuri 
> > + *  Rajan Vaja 
> > + */
> > +
> > +#include 
> > +#include 
> 
> > +#include 
> > +#include 
> 
> Can you move this group of headers after linux/* ?
> 
> > +#include 
> 
> Can it be moved outside of these headers
> 
> > +#include 
> > +#include 
> 
> Ordering (for all groups of headers)?
Ok, I will order the headers in the below order
#include 
#include 
#include 
#include 
> 
> > +#include "core.h"
> > +#include "pinctrl-utils.h"
> > +
> > +#define ZYNQMP_PIN_PREFIX  "MIO"
> > +#define PINCTRL_GET_FUNC_NAME_RESP_LEN 16
> > +#define MAX_FUNC_NAME_LEN  16
> > +#define MAX_GROUP_PIN  50
> > +#define END_OF_FUNCTIONS   "END_OF_FUNCTIONS"
> > +#define NUM_GROUPS_PER_RESP6
> > +
> > +#define PINCTRL_GET_FUNC_GROUPS_RESP_LEN   12
> > +#define PINCTRL_GET_PIN_GROUPS_RESP_LEN12
> > +#define NA_GROUP   -1
> > +#define RESERVED_GROUP  

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

2021-03-17 Thread Sai Krishna Potthuri
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 
---
 drivers/pinctrl/Kconfig  |   13 +
 drivers/pinctrl/Makefile |1 +
 drivers/pinctrl/pinctrl-zynqmp.c | 1030 ++
 3 files changed, 1044 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 ..63edde400e85
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-zynqmp.c
@@ -0,0 +1,1030 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ZynqMP pin controller
+ *
+ *  Copyright (C) 2020 Xilinx, Inc.
+ *
+ *  Sai Krishna Potthuri 
+ *  Rajan Vaja 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "core.h"
+#include "pinctrl-utils.h"
+
+#define ZYNQMP_PIN_PREFIX  "MIO"
+#define PINCTRL_GET_FUNC_NAME_RESP_LEN 16
+#define MAX_FUNC_NAME_LEN  16
+#define MAX_GROUP_PIN  50
+#define END_OF_FUNCTIONS   "END_OF_FUNCTIONS"
+#define NUM_GROUPS_PER_RESP6
+
+#define PINCTRL_GET_FUNC_GROUPS_RESP_LEN   12
+#define PINCTRL_GET_PIN_GROUPS_RESP_LEN12
+#define NA_GROUP   -1
+#define RESERVED_GROUP -2
+
+#define DRIVE_STRENGTH_2MA 2
+#define DRIVE_STRENGTH_4MA 4
+#define DRIVE_STRENGTH_8MA 8
+#define DRIVE_STRENGTH_12MA12
+
+/**
+ * struct zynqmp_pmux_function - a pinmux function
+ * @name:  Name of the pinmux function
+ * @groups:List of pingroups for this function
+ * @ngroups:   Number of entries in @groups
+ * @node:` Firmware node matching with for function
+ *
+ * This structure holds information about pin control function
+ * and function group names supporting that function.
+ */
+struct zynqmp_pmux_function {
+   char name[MAX_FUNC_NAME_LEN];
+   const char * const *groups;
+   unsigned int ngroups;
+};
+
+/**
+ * struct zynqmp_pinctrl - driver data
+ * @pctrl: Pinctrl device
+ * @groups:Pingroups
+ * @ngroups:   Number of @groups
+ * @funcs: Pinmux functions
+ * @nfuncs:Number of @funcs
+ *
+ * This struct is stored as driver data and used to retrieve
+ * information regarding pin control functions, groups and
+ * group pins.
+ */
+struct zynqmp_pinctrl {
+   struct pinctrl_dev *pctrl;
+   const struct zynqmp_pctrl_group *groups;
+   unsigned int ngroups;
+   const struct zynqmp_pmux_function *funcs;
+   unsigned int nfuncs;
+};
+
+/**
+ * struct zynqmp_pctrl_group - Pin control group info
+ * @name:  Group name
+ * @pins:  Group pin numbers
+ * @npins: Number of pins in group
+ */
+struct zynqmp_pctrl_group {
+   const char *name;
+   unsigned int pins[MAX_GROUP_PIN];
+   unsigned int npins;
+};
+
+/**
+ * enum zynqmp_pin_config_param - possible pin configuration parameters
+ * @PIN_CONFIG_IOSTANDARD: if the pin can select an IO standard,
+ * the argument to this parameter (on a
+ * custom format) tells the driver which
+ * alternative IO standard to use
+ */
+enum zynqmp_pin_config_param {
+   PIN_CONFIG

[PATCH v4 2/3] dt-bindings: pinctrl: Add binding for ZynqMP pinctrl driver

2021-03-17 Thread Sai Krishna Potthuri
Adding documentation and dt-bindings file which contains MIO pin
configuration defines for Xilinx ZynqMP pinctrl driver.

Signed-off-by: Sai Krishna Potthuri 
Reviewed-by: Rob Herring 
---
 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 339 ++
 include/dt-bindings/pinctrl/pinctrl-zynqmp.h  |  19 +
 2 files changed, 358 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
 create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h

diff --git a/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml 
b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
new file mode 100644
index ..0c1149706f8b
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
@@ -0,0 +1,339 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx ZynqMP Pinctrl
+
+maintainers:
+  - Sai Krishna Potthuri 
+  - Rajan Vaja 
+
+description: |
+  Please refer to pinctrl-bindings.txt in this directory for details of the
+  common pinctrl bindings used by client devices, including the meaning of the
+  phrase "pin configuration node".
+
+  ZynqMP's pin configuration nodes act as a container for an arbitrary number 
of
+  subnodes. Each of these subnodes represents some desired configuration for a
+  pin, a group, or a list of pins or groups. This 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.
+
+  Each configuration node can consist of multiple nodes describing the pinmux 
and
+  pinconf options. Those nodes can be pinmux nodes or pinconf nodes.
+
+  The name of each subnode is not important; all subnodes should be enumerated
+  and processed purely based on their content.
+
+properties:
+  compatible:
+const: xlnx,zynqmp-pinctrl
+
+patternProperties:
+  '^(.*-)?(default|gpio)$':
+type: object
+patternProperties:
+  '^mux':
+type: object
+description:
+  Pinctrl node's client devices use subnodes for pin muxes,
+  which in turn use below standard properties.
+$ref: pinmux-node.yaml#
+
+properties:
+  groups:
+description:
+  List of groups to select (either this or "pins" must be
+  specified), available groups for this subnode.
+items:
+  enum: [ethernet0_0_grp, ethernet1_0_grp, ethernet2_0_grp,
+ ethernet3_0_grp, gemtsu0_0_grp, gemtsu0_1_grp,
+ gemtsu0_2_grp, mdio0_0_grp, mdio1_0_grp,
+ mdio1_1_grp, mdio2_0_grp, mdio3_0_grp,
+ qspi0_0_grp, qspi_ss_0_grp, qspi_fbclk_0_grp,
+ spi0_0_grp, spi0_ss_0_grp, spi0_ss_1_grp,
+ spi0_ss_2_grp, spi0_1_grp, spi0_ss_3_grp,
+ spi0_ss_4_grp, spi0_ss_5_grp, spi0_2_grp,
+ spi0_ss_6_grp, spi0_ss_7_grp, spi0_ss_8_grp,
+ spi0_3_grp, spi0_ss_9_grp, spi0_ss_10_grp,
+ spi0_ss_11_grp, spi0_4_grp, spi0_ss_12_grp,
+ spi0_ss_13_grp, spi0_ss_14_grp, spi0_5_grp,
+ spi0_ss_15_grp, spi0_ss_16_grp, spi0_ss_17_grp,
+ spi1_0_grp, spi1_ss_0_grp, spi1_ss_1_grp,
+ spi1_ss_2_grp, spi1_1_grp, spi1_ss_3_grp,
+ spi1_ss_4_grp, spi1_ss_5_grp, spi1_2_grp,
+ spi1_ss_6_grp, spi1_ss_7_grp, spi1_ss_8_grp,
+ spi1_3_grp, spi1_ss_9_grp, spi1_ss_10_grp,
+ spi1_ss_11_grp, spi1_4_grp, spi1_ss_12_grp,
+ spi1_ss_13_grp, spi1_ss_14_grp, spi1_5_grp,
+ spi1_ss_15_grp, spi1_ss_16_grp, spi1_ss_17_grp,
+ sdio0_0_grp, sdio0_1_grp, sdio0_2_grp,
+ sdio0_3_grp, sdio0_4_grp, sdio0_5_grp,
+ sdio0_6_grp, sdio0_7_grp, sdio0_8_grp,
+ sdio0_9_grp, sdio0_10_grp, sdio0_11_grp,
+ sdio0_12_grp, sdio0_13_grp, sdio0_14_grp,
+ sdio0_15_grp, sdio0_16_grp, sdio0_17_grp,
+ sdio0_18_grp, sdio0_19_grp, sdio0_20_grp,
+ sdio0_21_grp, sdio0_22_grp, sdio0_23_grp,
+ sdio0_24_grp, sdio0_25_grp, sdio0_26_grp,
+ sdio0_27_grp, sdio0_28_grp, sdio0_29_grp,
+ sdio0_30_grp, sdio0_31_grp, sdio0_32_grp,
+ sdio0_pc_0_grp, sdio0_cd_0_grp, sdio0_wp_0_grp,
+ sdio0_pc_1_grp, sdio0_cd_1_grp, sdio0_wp_1_grp,
+ sdio0_pc_2_grp, sdio0_cd_2_grp, sdio0_wp_2_grp,
+ sdio1_0_grp, sdio1_1_grp, sdio1_2_grp,
+ sdio1_3_grp, sdio

[PATCH v4 1/3] firmware: xilinx: Add pinctrl support

2021-03-17 Thread Sai Krishna Potthuri
Adding pinctrl support to query platform specific information (pins)
from firmware.

Signed-off-by: Sai Krishna Potthuri 
Acked-by: Michal Simek 
---
 drivers/firmware/xilinx/zynqmp.c | 114 +++
 include/linux/firmware/xlnx-zynqmp.h |  90 +
 2 files changed, 204 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index efb8a66efc68..299c3d5a9ebd 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -784,6 +784,120 @@ int zynqmp_pm_fpga_get_status(u32 *value)
 }
 EXPORT_SYMBOL_GPL(zynqmp_pm_fpga_get_status);
 
+/**
+ * zynqmp_pm_pinctrl_request - Request Pin from firmware
+ * @pin: Pin number to request
+ *
+ * This function requests pin from firmware.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_request(const u32 pin)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_REQUEST, pin, 0, 0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_request);
+
+/**
+ * zynqmp_pm_pinctrl_release - Inform firmware that Pin control is released
+ * @pin: Pin number to release
+ *
+ * This function release pin from firmware.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_release(const u32 pin)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_RELEASE, pin, 0, 0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_release);
+
+/**
+ * zynqmp_pm_pinctrl_get_function - Read function id set for the given pin
+ * @pin: Pin number
+ * @id: Buffer to store function ID
+ *
+ * This function provides the function currently set for the given pin.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+int zynqmp_pm_pinctrl_get_function(const u32 pin, u32 *id)
+{
+   u32 ret_payload[PAYLOAD_ARG_CNT];
+   int ret;
+
+   if (!id)
+   return -EINVAL;
+
+   ret = zynqmp_pm_invoke_fn(PM_PINCTRL_GET_FUNCTION, pin, 0,
+ 0, 0, ret_payload);
+   *id = ret_payload[1];
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_get_function);
+
+/**
+ * zynqmp_pm_pinctrl_set_function - Set requested function for the pin
+ * @pin: Pin number
+ * @id: Function ID to set
+ *
+ * This function sets requested function for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_set_function(const u32 pin, const u32 id)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_SET_FUNCTION, pin, id,
+  0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_function);
+
+/**
+ * zynqmp_pm_pinctrl_get_config - Get configuration parameter for the pin
+ * @pin: Pin number
+ * @param: Parameter to get
+ * @value: Buffer to store parameter value
+ *
+ * This function gets requested configuration parameter for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_get_config(const u32 pin, const u32 param,
+u32 *value)
+{
+   u32 ret_payload[PAYLOAD_ARG_CNT];
+   int ret;
+
+   if (!value)
+   return -EINVAL;
+
+   ret = zynqmp_pm_invoke_fn(PM_PINCTRL_CONFIG_PARAM_GET, pin, param,
+ 0, 0, ret_payload);
+   *value = ret_payload[1];
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_get_config);
+
+/**
+ * zynqmp_pm_pinctrl_set_config - Set configuration parameter for the pin
+ * @pin: Pin number
+ * @param: Parameter to set
+ * @value: Parameter value to set
+ *
+ * This function sets requested configuration parameter for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
+u32 value)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_CONFIG_PARAM_SET, pin,
+  param, value, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_config);
+
 /**
  * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller
  *master has initialized its own power management
diff --git a/include/linux/firmware/xlnx-zynqmp.h 
b/include/linux/firmware/xlnx-zynqmp.h
index 5968df82b991..75aa6a5afa28 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -74,6 +74,12 @@ enum pm_api_id {
PM_FPGA_LOAD,
PM_FPGA_GET_STATUS,
PM_GET_CHIPID = 24,
+   PM_PINCTRL_REQUEST = 28,
+   PM_PINCTRL_RELEASE = 29,
+   PM_PINCTRL_GET_FUNCTION = 30,
+   PM_PINCTRL_SET_FUNCTION = 31,
+   PM_PINCTRL_CONFIG_PARAM_GET = 32,
+   PM_PINCTRL_CONFIG_PARAM_SET = 33,
PM_IOCTL = 34,
PM_QUERY_DATA,
PM_CLOCK_ENABLE,
@@ -125,6 +131,12 @@ enum pm_query_id {
PM_QID_CLOCK_GET_FIXEDFACTOR_PARAMS,
PM_QID_CLOCK_GET_PARENTS,
PM_QID_CLOCK_GET_ATTRIBUTES

[PATCH v4 0/3] Add ZynqMP pinctrl driver

2021-03-17 Thread Sai Krishna Potthuri
Add support for Xilinx ZynqMP pinctrl driver and also update the Xilinx
firmware driver to support pinctrl functionality.
This driver queries the pin information from the firmware and allow
configuring the pins as per the request.

changes in v4:
- Added comment for ignoring the return value for GET_FUNCTION_NAME qid.
- Updated the zynqmp_pinctrl_get_function_name() API prototype to void
as it always returns zero.

changes in v3:
- Fixed binding doc comments from Rob.
- Used 'maxItems' for groups and pins properties.
- Updated commit subject and description to have present tense statements.

changes in v2:
- Use pattern for pin names in yaml file.
- Updated to support multiple groups and pins.
- Added type ref for the vendor specific properties.
- Removed 'schmitt-cmos', instead used common properties.
- Removed macros for drive-strength property.

Sai Krishna Potthuri (3):
  firmware: xilinx: Add pinctrl support
  dt-bindings: pinctrl: Add binding for ZynqMP pinctrl driver
  pinctrl: Add Xilinx ZynqMP pinctrl driver support

 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml |  339 ++
 drivers/firmware/xilinx/zynqmp.c  |  114 ++
 drivers/pinctrl/Kconfig   |   13 +
 drivers/pinctrl/Makefile  |1 +
 drivers/pinctrl/pinctrl-zynqmp.c  | 1030 +
 include/dt-bindings/pinctrl/pinctrl-zynqmp.h  |   19 +
 include/linux/firmware/xlnx-zynqmp.h  |   90 ++
 7 files changed, 1606 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
 create mode 100644 drivers/pinctrl/pinctrl-zynqmp.c
 create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h

-- 
2.17.1



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

2021-03-01 Thread Sai Krishna Potthuri
Hi Nobuhiro,

> -Original Message-
> From: Nobuhiro Iwamatsu 
> Sent: Sunday, February 28, 2021 6:19 AM
> To: Sai Krishna Potthuri 
> Cc: Linus Walleij ; Rob Herring
> ; Michal Simek ; Greg Kroah-
> Hartman ; linux ARM  ker...@lists.infradead.org>; Linux Kernel Mailing List  ker...@vger.kernel.org>; devicet...@vger.kernel.org; linux-
> g...@vger.kernel.org; git ; saikrishna12...@gmail.com
> Subject: Re: [PATCH v3 3/3] pinctrl: Add Xilinx ZynqMP pinctrl driver support
> 
> Hi,
> 
> 2021年2月12日(金) 21:10 Sai Krishna Potthuri
> :
> >
> > 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
> > 
> > ---
> >  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 ..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 
> > + *  Rajan Vaja 
> > + */
> 
> 
> 
> > +/**
> > + * 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


[PATCH v3 2/3] dt-bindings: pinctrl: Add binding for ZynqMP pinctrl driver

2021-02-12 Thread Sai Krishna Potthuri
Adding documentation and dt-bindings file which contains MIO pin
configuration defines for Xilinx ZynqMP pinctrl driver.

Signed-off-by: Sai Krishna Potthuri 
---
 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 339 ++
 include/dt-bindings/pinctrl/pinctrl-zynqmp.h  |  19 +
 2 files changed, 358 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
 create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h

diff --git a/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml 
b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
new file mode 100644
index ..0c1149706f8b
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
@@ -0,0 +1,339 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx ZynqMP Pinctrl
+
+maintainers:
+  - Sai Krishna Potthuri 
+  - Rajan Vaja 
+
+description: |
+  Please refer to pinctrl-bindings.txt in this directory for details of the
+  common pinctrl bindings used by client devices, including the meaning of the
+  phrase "pin configuration node".
+
+  ZynqMP's pin configuration nodes act as a container for an arbitrary number 
of
+  subnodes. Each of these subnodes represents some desired configuration for a
+  pin, a group, or a list of pins or groups. This 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.
+
+  Each configuration node can consist of multiple nodes describing the pinmux 
and
+  pinconf options. Those nodes can be pinmux nodes or pinconf nodes.
+
+  The name of each subnode is not important; all subnodes should be enumerated
+  and processed purely based on their content.
+
+properties:
+  compatible:
+const: xlnx,zynqmp-pinctrl
+
+patternProperties:
+  '^(.*-)?(default|gpio)$':
+type: object
+patternProperties:
+  '^mux':
+type: object
+description:
+  Pinctrl node's client devices use subnodes for pin muxes,
+  which in turn use below standard properties.
+$ref: pinmux-node.yaml#
+
+properties:
+  groups:
+description:
+  List of groups to select (either this or "pins" must be
+  specified), available groups for this subnode.
+items:
+  enum: [ethernet0_0_grp, ethernet1_0_grp, ethernet2_0_grp,
+ ethernet3_0_grp, gemtsu0_0_grp, gemtsu0_1_grp,
+ gemtsu0_2_grp, mdio0_0_grp, mdio1_0_grp,
+ mdio1_1_grp, mdio2_0_grp, mdio3_0_grp,
+ qspi0_0_grp, qspi_ss_0_grp, qspi_fbclk_0_grp,
+ spi0_0_grp, spi0_ss_0_grp, spi0_ss_1_grp,
+ spi0_ss_2_grp, spi0_1_grp, spi0_ss_3_grp,
+ spi0_ss_4_grp, spi0_ss_5_grp, spi0_2_grp,
+ spi0_ss_6_grp, spi0_ss_7_grp, spi0_ss_8_grp,
+ spi0_3_grp, spi0_ss_9_grp, spi0_ss_10_grp,
+ spi0_ss_11_grp, spi0_4_grp, spi0_ss_12_grp,
+ spi0_ss_13_grp, spi0_ss_14_grp, spi0_5_grp,
+ spi0_ss_15_grp, spi0_ss_16_grp, spi0_ss_17_grp,
+ spi1_0_grp, spi1_ss_0_grp, spi1_ss_1_grp,
+ spi1_ss_2_grp, spi1_1_grp, spi1_ss_3_grp,
+ spi1_ss_4_grp, spi1_ss_5_grp, spi1_2_grp,
+ spi1_ss_6_grp, spi1_ss_7_grp, spi1_ss_8_grp,
+ spi1_3_grp, spi1_ss_9_grp, spi1_ss_10_grp,
+ spi1_ss_11_grp, spi1_4_grp, spi1_ss_12_grp,
+ spi1_ss_13_grp, spi1_ss_14_grp, spi1_5_grp,
+ spi1_ss_15_grp, spi1_ss_16_grp, spi1_ss_17_grp,
+ sdio0_0_grp, sdio0_1_grp, sdio0_2_grp,
+ sdio0_3_grp, sdio0_4_grp, sdio0_5_grp,
+ sdio0_6_grp, sdio0_7_grp, sdio0_8_grp,
+ sdio0_9_grp, sdio0_10_grp, sdio0_11_grp,
+ sdio0_12_grp, sdio0_13_grp, sdio0_14_grp,
+ sdio0_15_grp, sdio0_16_grp, sdio0_17_grp,
+ sdio0_18_grp, sdio0_19_grp, sdio0_20_grp,
+ sdio0_21_grp, sdio0_22_grp, sdio0_23_grp,
+ sdio0_24_grp, sdio0_25_grp, sdio0_26_grp,
+ sdio0_27_grp, sdio0_28_grp, sdio0_29_grp,
+ sdio0_30_grp, sdio0_31_grp, sdio0_32_grp,
+ sdio0_pc_0_grp, sdio0_cd_0_grp, sdio0_wp_0_grp,
+ sdio0_pc_1_grp, sdio0_cd_1_grp, sdio0_wp_1_grp,
+ sdio0_pc_2_grp, sdio0_cd_2_grp, sdio0_wp_2_grp,
+ sdio1_0_grp, sdio1_1_grp, sdio1_2_grp,
+ sdio1_3_grp, sdio1_4_grp, sdio1_5_grp,
+

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

2021-02-12 Thread Sai Krishna Potthuri
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 
---
 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 ..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 
+ *  Rajan Vaja 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "core.h"
+#include "pinctrl-utils.h"
+
+#define ZYNQMP_PIN_PREFIX  "MIO"
+#define PINCTRL_GET_FUNC_NAME_RESP_LEN 16
+#define MAX_FUNC_NAME_LEN  16
+#define MAX_GROUP_PIN  50
+#define END_OF_FUNCTIONS   "END_OF_FUNCTIONS"
+#define NUM_GROUPS_PER_RESP6
+
+#define PINCTRL_GET_FUNC_GROUPS_RESP_LEN   12
+#define PINCTRL_GET_PIN_GROUPS_RESP_LEN12
+#define NA_GROUP   -1
+#define RESERVED_GROUP -2
+
+#define DRIVE_STRENGTH_2MA 2
+#define DRIVE_STRENGTH_4MA 4
+#define DRIVE_STRENGTH_8MA 8
+#define DRIVE_STRENGTH_12MA12
+
+/**
+ * struct zynqmp_pmux_function - a pinmux function
+ * @name:  Name of the pinmux function
+ * @groups:List of pingroups for this function
+ * @ngroups:   Number of entries in @groups
+ * @node:` Firmware node matching with for function
+ *
+ * This structure holds information about pin control function
+ * and function group names supporting that function.
+ */
+struct zynqmp_pmux_function {
+   char name[MAX_FUNC_NAME_LEN];
+   const char * const *groups;
+   unsigned int ngroups;
+};
+
+/**
+ * struct zynqmp_pinctrl - driver data
+ * @pctrl: Pinctrl device
+ * @groups:Pingroups
+ * @ngroups:   Number of @groups
+ * @funcs: Pinmux functions
+ * @nfuncs:Number of @funcs
+ *
+ * This struct is stored as driver data and used to retrieve
+ * information regarding pin control functions, groups and
+ * group pins.
+ */
+struct zynqmp_pinctrl {
+   struct pinctrl_dev *pctrl;
+   const struct zynqmp_pctrl_group *groups;
+   unsigned int ngroups;
+   const struct zynqmp_pmux_function *funcs;
+   unsigned int nfuncs;
+};
+
+/**
+ * struct zynqmp_pctrl_group - Pin control group info
+ * @name:  Group name
+ * @pins:  Group pin numbers
+ * @npins: Number of pins in group
+ */
+struct zynqmp_pctrl_group {
+   const char *name;
+   unsigned int pins[MAX_GROUP_PIN];
+   unsigned int npins;
+};
+
+/**
+ * enum zynqmp_pin_config_param - possible pin configuration parameters
+ * @PIN_CONFIG_IOSTANDARD: if the pin can select an IO standard,
+ * the argument to this parameter (on a
+ * custom format) tells the driver which
+ * alternative IO standard to use
+ */
+enum zynqmp_pin_config_param {
+   PIN_CONFIG

[PATCH v3 1/3] firmware: xilinx: Add pinctrl support

2021-02-12 Thread Sai Krishna Potthuri
Adding pinctrl support to query platform specific information (pins)
from firmware.

Signed-off-by: Sai Krishna Potthuri 
Acked-by: Michal Simek 
---
 drivers/firmware/xilinx/zynqmp.c | 114 +++
 include/linux/firmware/xlnx-zynqmp.h |  90 +
 2 files changed, 204 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index efb8a66efc68..299c3d5a9ebd 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -784,6 +784,120 @@ int zynqmp_pm_fpga_get_status(u32 *value)
 }
 EXPORT_SYMBOL_GPL(zynqmp_pm_fpga_get_status);
 
+/**
+ * zynqmp_pm_pinctrl_request - Request Pin from firmware
+ * @pin: Pin number to request
+ *
+ * This function requests pin from firmware.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_request(const u32 pin)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_REQUEST, pin, 0, 0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_request);
+
+/**
+ * zynqmp_pm_pinctrl_release - Inform firmware that Pin control is released
+ * @pin: Pin number to release
+ *
+ * This function release pin from firmware.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_release(const u32 pin)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_RELEASE, pin, 0, 0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_release);
+
+/**
+ * zynqmp_pm_pinctrl_get_function - Read function id set for the given pin
+ * @pin: Pin number
+ * @id: Buffer to store function ID
+ *
+ * This function provides the function currently set for the given pin.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+int zynqmp_pm_pinctrl_get_function(const u32 pin, u32 *id)
+{
+   u32 ret_payload[PAYLOAD_ARG_CNT];
+   int ret;
+
+   if (!id)
+   return -EINVAL;
+
+   ret = zynqmp_pm_invoke_fn(PM_PINCTRL_GET_FUNCTION, pin, 0,
+ 0, 0, ret_payload);
+   *id = ret_payload[1];
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_get_function);
+
+/**
+ * zynqmp_pm_pinctrl_set_function - Set requested function for the pin
+ * @pin: Pin number
+ * @id: Function ID to set
+ *
+ * This function sets requested function for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_set_function(const u32 pin, const u32 id)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_SET_FUNCTION, pin, id,
+  0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_function);
+
+/**
+ * zynqmp_pm_pinctrl_get_config - Get configuration parameter for the pin
+ * @pin: Pin number
+ * @param: Parameter to get
+ * @value: Buffer to store parameter value
+ *
+ * This function gets requested configuration parameter for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_get_config(const u32 pin, const u32 param,
+u32 *value)
+{
+   u32 ret_payload[PAYLOAD_ARG_CNT];
+   int ret;
+
+   if (!value)
+   return -EINVAL;
+
+   ret = zynqmp_pm_invoke_fn(PM_PINCTRL_CONFIG_PARAM_GET, pin, param,
+ 0, 0, ret_payload);
+   *value = ret_payload[1];
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_get_config);
+
+/**
+ * zynqmp_pm_pinctrl_set_config - Set configuration parameter for the pin
+ * @pin: Pin number
+ * @param: Parameter to set
+ * @value: Parameter value to set
+ *
+ * This function sets requested configuration parameter for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
+u32 value)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_CONFIG_PARAM_SET, pin,
+  param, value, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_config);
+
 /**
  * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller
  *master has initialized its own power management
diff --git a/include/linux/firmware/xlnx-zynqmp.h 
b/include/linux/firmware/xlnx-zynqmp.h
index 5968df82b991..75aa6a5afa28 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -74,6 +74,12 @@ enum pm_api_id {
PM_FPGA_LOAD,
PM_FPGA_GET_STATUS,
PM_GET_CHIPID = 24,
+   PM_PINCTRL_REQUEST = 28,
+   PM_PINCTRL_RELEASE = 29,
+   PM_PINCTRL_GET_FUNCTION = 30,
+   PM_PINCTRL_SET_FUNCTION = 31,
+   PM_PINCTRL_CONFIG_PARAM_GET = 32,
+   PM_PINCTRL_CONFIG_PARAM_SET = 33,
PM_IOCTL = 34,
PM_QUERY_DATA,
PM_CLOCK_ENABLE,
@@ -125,6 +131,12 @@ enum pm_query_id {
PM_QID_CLOCK_GET_FIXEDFACTOR_PARAMS,
PM_QID_CLOCK_GET_PARENTS,
PM_QID_CLOCK_GET_ATTRIBUTES

[PATCH v3 0/3] Add ZynqMP pinctrl driver

2021-02-12 Thread Sai Krishna Potthuri
Add support for Xilinx ZynqMP pinctrl driver and also update the
Xilinx firmware driver to support pinctrl functionality.
This driver queries the pin information from the firmware and
allow configuring the pins as per the request.

changes in v3:
- Fixed binding doc comments from Rob.
- Used 'maxItems' for groups and pins properties.
- Updated commit subject and description to have
present tense statements.

changes in v2:
- Use pattern for pin names in yaml file.
- Updated to support multiple groups and pins.
- Added type ref for the vendor specific properties.
- Removed 'schmitt-cmos', instead used common properties.
- Removed macros for drive-strength property.

Sai Krishna Potthuri (3):
  firmware: xilinx: Add pinctrl support
  dt-bindings: pinctrl: Add binding for ZynqMP pinctrl driver
  pinctrl: Add Xilinx ZynqMP pinctrl driver support

 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml |  339 ++
 drivers/firmware/xilinx/zynqmp.c  |  114 ++
 drivers/pinctrl/Kconfig   |   13 +
 drivers/pinctrl/Makefile  |1 +
 drivers/pinctrl/pinctrl-zynqmp.c  | 1031 +
 include/dt-bindings/pinctrl/pinctrl-zynqmp.h  |   19 +
 include/linux/firmware/xlnx-zynqmp.h  |   90 ++
 7 files changed, 1607 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
 create mode 100644 drivers/pinctrl/pinctrl-zynqmp.c
 create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h

-- 
2.17.1



RE: [PATCH v2 2/3] dt-bindings: pinctrl: Added binding for ZynqMP pinctrl driver

2021-02-09 Thread Sai Krishna Potthuri
Hi Rob,

> -Original Message-
> From: Rob Herring 
> Sent: Tuesday, February 9, 2021 7:27 PM
> To: Sai Krishna Potthuri 
> Cc: Linus Walleij ; Michal Simek
> ; Greg Kroah-Hartman ;
> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicet...@vger.kernel.org; linux-g...@vger.kernel.org; git
> ; saikrishna12...@gmail.com
> Subject: Re: [PATCH v2 2/3] dt-bindings: pinctrl: Added binding for ZynqMP
> pinctrl driver
> 
> On Tue, Feb 9, 2021 at 2:17 AM Sai Krishna Potthuri 
> wrote:
> >
> > Hi Rob,
> >
> > Thanks for the review.
> >
> > > -Original Message-
> > > From: Rob Herring 
> > > Sent: Tuesday, February 9, 2021 7:51 AM
> > > To: Sai Krishna Potthuri 
> > > Cc: Linus Walleij ; Michal Simek
> > > ; Greg Kroah-Hartman
> > > ; linux-arm-ker...@lists.infradead.org;
> > > linux-kernel@vger.kernel.org; devicet...@vger.kernel.org;
> > > linux-g...@vger.kernel.org; git ;
> > > saikrishna12...@gmail.com
> > > Subject: Re: [PATCH v2 2/3] dt-bindings: pinctrl: Added binding for
> > > ZynqMP pinctrl driver
> > >
> > > On Tue, Jan 19, 2021 at 10:57:33AM +0530, Sai Krishna Potthuri wrote:
> > > > Added documentation and dt-bindings file which contains MIO pin
> > > > configuration defines for Xilinx ZynqMP pinctrl driver.
> > > >
> > > > Signed-off-by: Sai Krishna Potthuri
> > > > 
> > > > ---
> > > >  .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 337
> > > > ++  include/dt-bindings/pinctrl/pinctrl-zynqmp.h
> > > > ++ |
> > > > 23 ++
> > > >  2 files changed, 360 insertions(+)  create mode 100644
> > > > Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
> > > >  create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.ya
> > > > ml
> > > > b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.ya
> > > > ml
> > > > new file mode 100644
> > > > index ..9f2efbafcaa4
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctr
> > > > +++ l.ya
> > > > +++ ml
> > > > @@ -0,0 +1,337 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
> > > > +---
> > > > +$id:
> > > > +http://devicetree.org/schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Xilinx ZynqMP Pinctrl
> > > > +
> > > > +maintainers:
> > > > +  - Sai Krishna Potthuri
> > > > +
> > > > +  - Rajan Vaja 
> > > > +
> > > > +description: |
> > > > +  Please refer to pinctrl-bindings.txt in this directory for
> > > > +details of the
> > > > +  common pinctrl bindings used by client devices, including the
> > > > +meaning of the
> > > > +  phrase "pin configuration node".
> > > > +
> > > > +  ZynqMP's pin configuration nodes act as a container for an
> > > > + arbitrary number of  subnodes. Each of these subnodes represents
> > > > + some desired configuration for a  pin, a group, or a list of
> > > > + pins or groups. This 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.
> > > > +
> > > > +  Each configuration node can consist of multiple nodes
> > > > + describing the pinmux and  pinconf options. Those nodes can be
> > > > + pinmux nodes or
> > > pinconf nodes.
> > > > +
> > > > +  The name of each subnode is not important; all subnodes should
> > > > + be enumerated  and processed purely based on their content.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +const: xlnx,zynqmp-pinctrl
> > > > +
> > > > +patternProperties:
> > > > +  '^(.*-)?(default|gpio)$':
> > > > +type: object
> > > > +patternProperties:
> > > > +  '^mux(.*)$':
> > >
> > > '^mux' is equivalent.
> > I will fix in v3.
> &g

RE: [PATCH v2 2/3] dt-bindings: pinctrl: Added binding for ZynqMP pinctrl driver

2021-02-09 Thread Sai Krishna Potthuri
Hi Rob,

Thanks for the review.

> -Original Message-
> From: Rob Herring 
> Sent: Tuesday, February 9, 2021 7:51 AM
> To: Sai Krishna Potthuri 
> Cc: Linus Walleij ; Michal Simek
> ; Greg Kroah-Hartman ;
> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicet...@vger.kernel.org; linux-g...@vger.kernel.org; git
> ; saikrishna12...@gmail.com
> Subject: Re: [PATCH v2 2/3] dt-bindings: pinctrl: Added binding for ZynqMP
> pinctrl driver
> 
> On Tue, Jan 19, 2021 at 10:57:33AM +0530, Sai Krishna Potthuri wrote:
> > Added documentation and dt-bindings file which contains MIO pin
> > configuration defines for Xilinx ZynqMP pinctrl driver.
> >
> > Signed-off-by: Sai Krishna Potthuri
> > 
> > ---
> >  .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 337
> > ++  include/dt-bindings/pinctrl/pinctrl-zynqmp.h  |
> > 23 ++
> >  2 files changed, 360 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
> >  create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h
> >
> > diff --git
> > a/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
> > b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
> > new file mode 100644
> > index ..9f2efbafcaa4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.ya
> > +++ ml
> > @@ -0,0 +1,337 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Xilinx ZynqMP Pinctrl
> > +
> > +maintainers:
> > +  - Sai Krishna Potthuri 
> > +  - Rajan Vaja 
> > +
> > +description: |
> > +  Please refer to pinctrl-bindings.txt in this directory for details
> > +of the
> > +  common pinctrl bindings used by client devices, including the
> > +meaning of the
> > +  phrase "pin configuration node".
> > +
> > +  ZynqMP's pin configuration nodes act as a container for an
> > + arbitrary number of  subnodes. Each of these subnodes represents
> > + some desired configuration for a  pin, a group, or a list of pins or
> > + groups. This 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.
> > +
> > +  Each configuration node can consist of multiple nodes describing
> > + the pinmux and  pinconf options. Those nodes can be pinmux nodes or
> pinconf nodes.
> > +
> > +  The name of each subnode is not important; all subnodes should be
> > + enumerated  and processed purely based on their content.
> > +
> > +properties:
> > +  compatible:
> > +const: xlnx,zynqmp-pinctrl
> > +
> > +patternProperties:
> > +  '^(.*-)?(default|gpio)$':
> > +type: object
> > +patternProperties:
> > +  '^mux(.*)$':
> 
> '^mux' is equivalent.
I will fix in v3.

> 
> > +type: object
> > +description:
> > +  Pinctrl node's client devices use subnodes for pin muxes,
> > +  which in turn use below standard properties.
> > +$ref: pinmux-node.yaml#
> > +
> > +properties:
> > +  groups:
> > +description:
> > +  List of groups to select (either this or "pins" must be
> > +  specified), available groups for this subnode.
> > +items:
> > +  oneOf:
> > +- enum: [ethernet0_0_grp, ethernet1_0_grp,
> > + ethernet2_0_grp,
> 
> Don't need 'oneOf' for a single item.
Here we have a possibility to have more than one group item as below,
hence used 'oneOf'.
groups = "uart0_4_grp", "uart0_5_grp";
Please suggest me if there is a better/another way to represent this.

> 
> > + ethernet3_0_grp, gemtsu0_0_grp, gemtsu0_1_grp,
> > + gemtsu0_2_grp, mdio0_0_grp, mdio1_0_grp,
> > + mdio1_1_grp, mdio2_0_grp, mdio3_0_grp,
> > + qspi0_0_grp, qspi_ss_0_grp, qspi_fbclk_0_grp,
> > + spi0_0_grp, spi0_ss_0_grp, spi0_ss_1_grp,
> > + spi0_ss_2_grp, spi0_1_grp, spi0_ss_3_grp,
> > + spi0_ss_4_grp, spi0_ss_5_grp, spi0_2_grp,
> > + spi0_ss_6_grp, spi0_

[PATCH v2 0/3] Added ZynqMP pinctrl driver

2021-01-18 Thread Sai Krishna Potthuri
Added support for Xilinx ZynqMP pinctrl driver support and also
updated the Xilinx firmware driver to support pinctrl functionality.
This driver will query the pin information from the firmware and
allow configuring the pins as per the request.

changes in v2:
- Use pattern for pin names in yaml file.
- Updated to support multiple groups and pins.
- Added type ref for the vendor specific properties.
- Removed 'schmitt-cmos', instead used common properties.
- Removed macros for drive-strength property.

This might be resend for some people, as i had some trouble with my
email server earlier.

Sai Krishna Potthuri (3):
  firmware: xilinx: Added pinctrl support
  dt-bindings: pinctrl: Added binding for ZynqMP pinctrl driver
  pinctrl: Added Xilinx ZynqMP pinctrl driver support

 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml |  337 ++
 drivers/firmware/xilinx/zynqmp.c  |  114 ++
 drivers/pinctrl/Kconfig   |   13 +
 drivers/pinctrl/Makefile  |1 +
 drivers/pinctrl/pinctrl-zynqmp.c  | 1031 +
 include/dt-bindings/pinctrl/pinctrl-zynqmp.h  |   23 +
 include/linux/firmware/xlnx-zynqmp.h  |   90 ++
 7 files changed, 1609 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
 create mode 100644 drivers/pinctrl/pinctrl-zynqmp.c
 create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h

-- 
2.17.1



[PATCH v2 1/3] firmware: xilinx: Added pinctrl support

2021-01-18 Thread Sai Krishna Potthuri
Add pinctrl support to query platform specific information (pins)
from firmware.

Signed-off-by: Sai Krishna Potthuri 
---
 drivers/firmware/xilinx/zynqmp.c | 114 +++
 include/linux/firmware/xlnx-zynqmp.h |  90 +
 2 files changed, 204 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index efb8a66efc68..299c3d5a9ebd 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -784,6 +784,120 @@ int zynqmp_pm_fpga_get_status(u32 *value)
 }
 EXPORT_SYMBOL_GPL(zynqmp_pm_fpga_get_status);
 
+/**
+ * zynqmp_pm_pinctrl_request - Request Pin from firmware
+ * @pin: Pin number to request
+ *
+ * This function requests pin from firmware.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_request(const u32 pin)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_REQUEST, pin, 0, 0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_request);
+
+/**
+ * zynqmp_pm_pinctrl_release - Inform firmware that Pin control is released
+ * @pin: Pin number to release
+ *
+ * This function release pin from firmware.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_release(const u32 pin)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_RELEASE, pin, 0, 0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_release);
+
+/**
+ * zynqmp_pm_pinctrl_get_function - Read function id set for the given pin
+ * @pin: Pin number
+ * @id: Buffer to store function ID
+ *
+ * This function provides the function currently set for the given pin.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+int zynqmp_pm_pinctrl_get_function(const u32 pin, u32 *id)
+{
+   u32 ret_payload[PAYLOAD_ARG_CNT];
+   int ret;
+
+   if (!id)
+   return -EINVAL;
+
+   ret = zynqmp_pm_invoke_fn(PM_PINCTRL_GET_FUNCTION, pin, 0,
+ 0, 0, ret_payload);
+   *id = ret_payload[1];
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_get_function);
+
+/**
+ * zynqmp_pm_pinctrl_set_function - Set requested function for the pin
+ * @pin: Pin number
+ * @id: Function ID to set
+ *
+ * This function sets requested function for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_set_function(const u32 pin, const u32 id)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_SET_FUNCTION, pin, id,
+  0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_function);
+
+/**
+ * zynqmp_pm_pinctrl_get_config - Get configuration parameter for the pin
+ * @pin: Pin number
+ * @param: Parameter to get
+ * @value: Buffer to store parameter value
+ *
+ * This function gets requested configuration parameter for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_get_config(const u32 pin, const u32 param,
+u32 *value)
+{
+   u32 ret_payload[PAYLOAD_ARG_CNT];
+   int ret;
+
+   if (!value)
+   return -EINVAL;
+
+   ret = zynqmp_pm_invoke_fn(PM_PINCTRL_CONFIG_PARAM_GET, pin, param,
+ 0, 0, ret_payload);
+   *value = ret_payload[1];
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_get_config);
+
+/**
+ * zynqmp_pm_pinctrl_set_config - Set configuration parameter for the pin
+ * @pin: Pin number
+ * @param: Parameter to set
+ * @value: Parameter value to set
+ *
+ * This function sets requested configuration parameter for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
+u32 value)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_CONFIG_PARAM_SET, pin,
+  param, value, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_config);
+
 /**
  * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller
  *master has initialized its own power management
diff --git a/include/linux/firmware/xlnx-zynqmp.h 
b/include/linux/firmware/xlnx-zynqmp.h
index 5968df82b991..75aa6a5afa28 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -74,6 +74,12 @@ enum pm_api_id {
PM_FPGA_LOAD,
PM_FPGA_GET_STATUS,
PM_GET_CHIPID = 24,
+   PM_PINCTRL_REQUEST = 28,
+   PM_PINCTRL_RELEASE = 29,
+   PM_PINCTRL_GET_FUNCTION = 30,
+   PM_PINCTRL_SET_FUNCTION = 31,
+   PM_PINCTRL_CONFIG_PARAM_GET = 32,
+   PM_PINCTRL_CONFIG_PARAM_SET = 33,
PM_IOCTL = 34,
PM_QUERY_DATA,
PM_CLOCK_ENABLE,
@@ -125,6 +131,12 @@ enum pm_query_id {
PM_QID_CLOCK_GET_FIXEDFACTOR_PARAMS,
PM_QID_CLOCK_GET_PARENTS,
PM_QID_CLOCK_GET_ATTRIBUTES,
+   PM_QID_PINCTRL_GET_NUM_PINS = 6

[PATCH v2 2/3] dt-bindings: pinctrl: Added binding for ZynqMP pinctrl driver

2021-01-18 Thread Sai Krishna Potthuri
Added documentation and dt-bindings file which contains MIO pin
configuration defines for Xilinx ZynqMP pinctrl driver.

Signed-off-by: Sai Krishna Potthuri 
---
 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 337 ++
 include/dt-bindings/pinctrl/pinctrl-zynqmp.h  |  23 ++
 2 files changed, 360 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
 create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h

diff --git a/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml 
b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
new file mode 100644
index ..9f2efbafcaa4
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
@@ -0,0 +1,337 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx ZynqMP Pinctrl
+
+maintainers:
+  - Sai Krishna Potthuri 
+  - Rajan Vaja 
+
+description: |
+  Please refer to pinctrl-bindings.txt in this directory for details of the
+  common pinctrl bindings used by client devices, including the meaning of the
+  phrase "pin configuration node".
+
+  ZynqMP's pin configuration nodes act as a container for an arbitrary number 
of
+  subnodes. Each of these subnodes represents some desired configuration for a
+  pin, a group, or a list of pins or groups. This 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.
+
+  Each configuration node can consist of multiple nodes describing the pinmux 
and
+  pinconf options. Those nodes can be pinmux nodes or pinconf nodes.
+
+  The name of each subnode is not important; all subnodes should be enumerated
+  and processed purely based on their content.
+
+properties:
+  compatible:
+const: xlnx,zynqmp-pinctrl
+
+patternProperties:
+  '^(.*-)?(default|gpio)$':
+type: object
+patternProperties:
+  '^mux(.*)$':
+type: object
+description:
+  Pinctrl node's client devices use subnodes for pin muxes,
+  which in turn use below standard properties.
+$ref: pinmux-node.yaml#
+
+properties:
+  groups:
+description:
+  List of groups to select (either this or "pins" must be
+  specified), available groups for this subnode.
+items:
+  oneOf:
+- enum: [ethernet0_0_grp, ethernet1_0_grp, ethernet2_0_grp,
+ ethernet3_0_grp, gemtsu0_0_grp, gemtsu0_1_grp,
+ gemtsu0_2_grp, mdio0_0_grp, mdio1_0_grp,
+ mdio1_1_grp, mdio2_0_grp, mdio3_0_grp,
+ qspi0_0_grp, qspi_ss_0_grp, qspi_fbclk_0_grp,
+ spi0_0_grp, spi0_ss_0_grp, spi0_ss_1_grp,
+ spi0_ss_2_grp, spi0_1_grp, spi0_ss_3_grp,
+ spi0_ss_4_grp, spi0_ss_5_grp, spi0_2_grp,
+ spi0_ss_6_grp, spi0_ss_7_grp, spi0_ss_8_grp,
+ spi0_3_grp, spi0_ss_9_grp, spi0_ss_10_grp,
+ spi0_ss_11_grp, spi0_4_grp, spi0_ss_12_grp,
+ spi0_ss_13_grp, spi0_ss_14_grp, spi0_5_grp,
+ spi0_ss_15_grp, spi0_ss_16_grp, spi0_ss_17_grp,
+ spi1_0_grp, spi1_ss_0_grp, spi1_ss_1_grp,
+ spi1_ss_2_grp, spi1_1_grp, spi1_ss_3_grp,
+ spi1_ss_4_grp, spi1_ss_5_grp, spi1_2_grp,
+ spi1_ss_6_grp, spi1_ss_7_grp, spi1_ss_8_grp,
+ spi1_3_grp, spi1_ss_9_grp, spi1_ss_10_grp,
+ spi1_ss_11_grp, spi1_4_grp, spi1_ss_12_grp,
+ spi1_ss_13_grp, spi1_ss_14_grp, spi1_5_grp,
+ spi1_ss_15_grp, spi1_ss_16_grp, spi1_ss_17_grp,
+ sdio0_0_grp, sdio0_1_grp, sdio0_2_grp,
+ sdio0_3_grp, sdio0_4_grp, sdio0_5_grp,
+ sdio0_6_grp, sdio0_7_grp, sdio0_8_grp,
+ sdio0_9_grp, sdio0_10_grp, sdio0_11_grp,
+ sdio0_12_grp, sdio0_13_grp, sdio0_14_grp,
+ sdio0_15_grp, sdio0_16_grp, sdio0_17_grp,
+ sdio0_18_grp, sdio0_19_grp, sdio0_20_grp,
+ sdio0_21_grp, sdio0_22_grp, sdio0_23_grp,
+ sdio0_24_grp, sdio0_25_grp, sdio0_26_grp,
+ sdio0_27_grp, sdio0_28_grp, sdio0_29_grp,
+ sdio0_30_grp, sdio0_31_grp, sdio0_32_grp,
+ sdio0_pc_0_grp, sdio0_cd_0_grp, sdio0_wp_0_grp,
+ sdio0_pc_1_grp, sdio0_cd_1_grp, sdio0_wp_1_grp,
+ sdio0_pc_2

[PATCH v2 3/3] pinctrl: Added Xilinx ZynqMP pinctrl driver support

2021-01-18 Thread Sai Krishna Potthuri
Added pinctrl driver for Xilinx ZynqMP platform.
This driver queries pin information from firmware and registers
pin control accordingly.

Signed-off-by: Sai Krishna Potthuri 
---
 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 ..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 
+ *  Rajan Vaja 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "core.h"
+#include "pinctrl-utils.h"
+
+#define ZYNQMP_PIN_PREFIX  "MIO"
+#define PINCTRL_GET_FUNC_NAME_RESP_LEN 16
+#define MAX_FUNC_NAME_LEN  16
+#define MAX_GROUP_PIN  50
+#define END_OF_FUNCTIONS   "END_OF_FUNCTIONS"
+#define NUM_GROUPS_PER_RESP6
+
+#define PINCTRL_GET_FUNC_GROUPS_RESP_LEN   12
+#define PINCTRL_GET_PIN_GROUPS_RESP_LEN12
+#define NA_GROUP   -1
+#define RESERVED_GROUP -2
+
+#define DRIVE_STRENGTH_2MA 2
+#define DRIVE_STRENGTH_4MA 4
+#define DRIVE_STRENGTH_8MA 8
+#define DRIVE_STRENGTH_12MA12
+
+/**
+ * struct zynqmp_pmux_function - a pinmux function
+ * @name:  Name of the pinmux function
+ * @groups:List of pingroups for this function
+ * @ngroups:   Number of entries in @groups
+ * @node:` Firmware node matching with for function
+ *
+ * This structure holds information about pin control function
+ * and function group names supporting that function.
+ */
+struct zynqmp_pmux_function {
+   char name[MAX_FUNC_NAME_LEN];
+   const char * const *groups;
+   unsigned int ngroups;
+};
+
+/**
+ * struct zynqmp_pinctrl - driver data
+ * @pctrl: Pinctrl device
+ * @groups:Pingroups
+ * @ngroups:   Number of @groups
+ * @funcs: Pinmux functions
+ * @nfuncs:Number of @funcs
+ *
+ * This struct is stored as driver data and used to retrieve
+ * information regarding pin control functions, groups and
+ * group pins.
+ */
+struct zynqmp_pinctrl {
+   struct pinctrl_dev *pctrl;
+   const struct zynqmp_pctrl_group *groups;
+   unsigned int ngroups;
+   const struct zynqmp_pmux_function *funcs;
+   unsigned int nfuncs;
+};
+
+/**
+ * struct zynqmp_pctrl_group - Pin control group info
+ * @name:  Group name
+ * @pins:  Group pin numbers
+ * @npins: Number of pins in group
+ */
+struct zynqmp_pctrl_group {
+   const char *name;
+   unsigned int pins[MAX_GROUP_PIN];
+   unsigned int npins;
+};
+
+/**
+ * enum zynqmp_pin_config_param - possible pin configuration parameters
+ * @PIN_CONFIG_IOSTANDARD: if the pin can select an IO standard,
+ * the argument to this parameter (on a
+ * custom format) tells the driver which
+ * alternative IO standard to use
+ */
+enum zynqmp_pin_config_param {
+   PIN_CONFIG

RE: [PATCH 2/3] dt-bindings: pinctrl: Added binding for ZynqMP pinctrl driver

2020-12-10 Thread Sai Krishna Potthuri
Hi Rob,

Thanks for the review

> -Original Message-
> From: Rob Herring 
> Sent: Wednesday, December 9, 2020 10:33 PM
> To: Sai Krishna Potthuri 
> Cc: Linus Walleij ; Michal Simek
> ; Greg Kroah-Hartman ;
> linux-arm-ker...@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicet...@vger.kernel.org; linux-g...@vger.kernel.org; git
> ; saikrishna12...@gmail.com
> Subject: Re: [PATCH 2/3] dt-bindings: pinctrl: Added binding for ZynqMP
> pinctrl driver
> 
> On Wed, Dec 09, 2020 at 02:14:13PM +0530, Sai Krishna Potthuri wrote:
> > Added documentation and dt-bindings file which contains MIO pin
> > configuration defines for Xilinx ZynqMP pinctrl driver.
> >
> > Signed-off-by: Sai Krishna Potthuri
> > 
> > ---
> >  .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 329
> > ++  include/dt-bindings/pinctrl/pinctrl-zynqmp.h  |
> > 29 ++
> >  2 files changed, 358 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
> >  create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h
> >
> > diff --git
> > a/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
> > b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
> > new file mode 100644
> > index ..dd0c8c12714f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.ya
> > +++ ml
> > @@ -0,0 +1,329 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Xilinx ZynqMP Pinctrl
> > +
> > +maintainers:
> > +  - Sai Krishna Potthuri 
> > +  - Rajan Vaja 
> > +
> > +description: |
> > +  Please refer to pinctrl-bindings.txt in this directory for details
> > +of the
> > +  common pinctrl bindings used by client devices, including the
> > +meaning of the
> > +  phrase "pin configuration node".
> > +
> > +  ZynqMP's pin configuration nodes act as a container for an
> > + arbitrary number of  subnodes. Each of these subnodes represents
> > + some desired configuration for a  pin, a group, or a list of pins or
> > + groups. This 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.
> > +
> > +  Each configuration node can consist of multiple nodes describing
> > + the pinmux and  pinconf options. Those nodes can be pinmux nodes or
> pinconf nodes.
> > +
> > +  The name of each subnode is not important; all subnodes should be
> > + enumerated  and processed purely based on their content.
> > +
> > +properties:
> > +  compatible:
> > +const: xlnx,zynqmp-pinctrl
> > +
> > +patternProperties:
> > +  '^(.*-)?(default|gpio)$':
> > +type: object
> > +patternProperties:
> > +  '^(.*-)?mux$':
> > +type: object
> > +description:
> > +  Pinctrl node's client devices use subnodes for pin muxes,
> > +  which in turn use below standard properties.
> > +$ref: pinmux-node.yaml#
> > +
> > +properties:
> > +  groups:
> > +description:
> > +  List of groups to select (either this or "pins" must be
> > +  specified), available groups for this subnode.
> > +enum: [ethernet0_0_grp, ethernet1_0_grp, ethernet2_0_grp,
> > +   ethernet3_0_grp, gemtsu0_0_grp, gemtsu0_1_grp,
> > +   gemtsu0_2_grp, mdio0_0_grp, mdio1_0_grp,
> > +   mdio1_1_grp, mdio2_0_grp, mdio3_0_grp,
> > +   qspi0_0_grp, qspi_ss_0_grp, qspi_fbclk_0_grp,
> > +   spi0_0_grp, spi0_ss_0_grp, spi0_ss_1_grp,
> > +   spi0_ss_2_grp, spi0_1_grp, spi0_ss_3_grp,
> > +   spi0_ss_4_grp, spi0_ss_5_grp, spi0_2_grp,
> > +   spi0_ss_6_grp, spi0_ss_7_grp, spi0_ss_8_grp,
> > +   spi0_3_grp, spi0_ss_9_grp, spi0_ss_10_grp,
> > +   spi0_ss_11_grp, spi0_4_grp, spi0_ss_12_grp,
> > +   spi0_ss_13_grp, spi0_ss_14_grp, spi0_5_grp,
> > +   spi0_ss_15_grp, spi0_ss_16_grp, spi0_ss_17_grp,
> > +   spi1_0_grp, spi1_ss_0_grp, spi1_ss_1_grp,
> > +   spi1_ss_2_grp, spi1_1_grp, spi1_ss_3_grp,
>

[PATCH 2/3] dt-bindings: pinctrl: Added binding for ZynqMP pinctrl driver

2020-12-09 Thread Sai Krishna Potthuri
Added documentation and dt-bindings file which contains MIO pin
configuration defines for Xilinx ZynqMP pinctrl driver.

Signed-off-by: Sai Krishna Potthuri 
---
 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml | 329 ++
 include/dt-bindings/pinctrl/pinctrl-zynqmp.h  |  29 ++
 2 files changed, 358 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
 create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h

diff --git a/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml 
b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
new file mode 100644
index ..dd0c8c12714f
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
@@ -0,0 +1,329 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/xlnx,zynqmp-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx ZynqMP Pinctrl
+
+maintainers:
+  - Sai Krishna Potthuri 
+  - Rajan Vaja 
+
+description: |
+  Please refer to pinctrl-bindings.txt in this directory for details of the
+  common pinctrl bindings used by client devices, including the meaning of the
+  phrase "pin configuration node".
+
+  ZynqMP's pin configuration nodes act as a container for an arbitrary number 
of
+  subnodes. Each of these subnodes represents some desired configuration for a
+  pin, a group, or a list of pins or groups. This 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.
+
+  Each configuration node can consist of multiple nodes describing the pinmux 
and
+  pinconf options. Those nodes can be pinmux nodes or pinconf nodes.
+
+  The name of each subnode is not important; all subnodes should be enumerated
+  and processed purely based on their content.
+
+properties:
+  compatible:
+const: xlnx,zynqmp-pinctrl
+
+patternProperties:
+  '^(.*-)?(default|gpio)$':
+type: object
+patternProperties:
+  '^(.*-)?mux$':
+type: object
+description:
+  Pinctrl node's client devices use subnodes for pin muxes,
+  which in turn use below standard properties.
+$ref: pinmux-node.yaml#
+
+properties:
+  groups:
+description:
+  List of groups to select (either this or "pins" must be
+  specified), available groups for this subnode.
+enum: [ethernet0_0_grp, ethernet1_0_grp, ethernet2_0_grp,
+   ethernet3_0_grp, gemtsu0_0_grp, gemtsu0_1_grp,
+   gemtsu0_2_grp, mdio0_0_grp, mdio1_0_grp,
+   mdio1_1_grp, mdio2_0_grp, mdio3_0_grp,
+   qspi0_0_grp, qspi_ss_0_grp, qspi_fbclk_0_grp,
+   spi0_0_grp, spi0_ss_0_grp, spi0_ss_1_grp,
+   spi0_ss_2_grp, spi0_1_grp, spi0_ss_3_grp,
+   spi0_ss_4_grp, spi0_ss_5_grp, spi0_2_grp,
+   spi0_ss_6_grp, spi0_ss_7_grp, spi0_ss_8_grp,
+   spi0_3_grp, spi0_ss_9_grp, spi0_ss_10_grp,
+   spi0_ss_11_grp, spi0_4_grp, spi0_ss_12_grp,
+   spi0_ss_13_grp, spi0_ss_14_grp, spi0_5_grp,
+   spi0_ss_15_grp, spi0_ss_16_grp, spi0_ss_17_grp,
+   spi1_0_grp, spi1_ss_0_grp, spi1_ss_1_grp,
+   spi1_ss_2_grp, spi1_1_grp, spi1_ss_3_grp,
+   spi1_ss_4_grp, spi1_ss_5_grp, spi1_2_grp,
+   spi1_ss_6_grp, spi1_ss_7_grp, spi1_ss_8_grp,
+   spi1_3_grp, spi1_ss_9_grp, spi1_ss_10_grp,
+   spi1_ss_11_grp, spi1_4_grp, spi1_ss_12_grp,
+   spi1_ss_13_grp, spi1_ss_14_grp, spi1_5_grp,
+   spi1_ss_15_grp, spi1_ss_16_grp, spi1_ss_17_grp,
+   sdio0_0_grp, sdio0_1_grp, sdio0_2_grp,
+   sdio0_3_grp, sdio0_4_grp, sdio0_5_grp,
+   sdio0_6_grp, sdio0_7_grp, sdio0_8_grp,
+   sdio0_9_grp, sdio0_10_grp, sdio0_11_grp,
+   sdio0_12_grp, sdio0_13_grp, sdio0_14_grp,
+   sdio0_15_grp, sdio0_16_grp, sdio0_17_grp,
+   sdio0_18_grp, sdio0_19_grp, sdio0_20_grp,
+   sdio0_21_grp, sdio0_22_grp, sdio0_23_grp,
+   sdio0_24_grp, sdio0_25_grp, sdio0_26_grp,
+   sdio0_27_grp, sdio0_28_grp, sdio0_29_grp,
+   sdio0_30_grp, sdio0_31_grp, sdio0_32_grp,
+   sdio0_pc_0_grp, sdio0_cd_0_grp, sdio0_wp_0_grp,
+   sdio0_pc_1_grp, sdio0_cd_1_grp, sdio0_wp_1_grp,
+   sdio0_pc_2_grp, sdio0_cd_2_grp, sdio0_wp_2_grp,
+   sdio1_0_grp, sdio1_1_grp, sdio1_2_grp,
+   sdio1_3_grp, sdio1_4_grp, sdio1_5_grp,
+   sdio1_6_grp, sdio1_7_grp, sdio1_8_grp,
+   sdio1_9_grp, sdio1_

[PATCH 3/3] pinctrl: Added Xilinx ZynqMP pinctrl driver support

2020-12-09 Thread Sai Krishna Potthuri
Added pinctrl driver for Xilinx ZynqMP platform.
This driver queries pin information from firmware and registers
pin control accordingly.

Signed-off-by: Sai Krishna Potthuri 
---
 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 ..0694cd6fc753
--- /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 
+ *  Rajan Vaja 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "core.h"
+#include "pinctrl-utils.h"
+
+#define ZYNQMP_PIN_PREFIX  "MIO"
+#define PINCTRL_GET_FUNC_NAME_RESP_LEN 16
+#define MAX_FUNC_NAME_LEN  16
+#define MAX_GROUP_PIN  50
+#define END_OF_FUNCTIONS   "END_OF_FUNCTIONS"
+#define NUM_GROUPS_PER_RESP6
+
+#define PINCTRL_GET_FUNC_GROUPS_RESP_LEN   12
+#define PINCTRL_GET_PIN_GROUPS_RESP_LEN12
+#define NA_GROUP   -1
+#define RESERVED_GROUP -2
+
+/**
+ * struct zynqmp_pmux_function - a pinmux function
+ * @name:  Name of the pinmux function
+ * @groups:List of pingroups for this function
+ * @ngroups:   Number of entries in @groups
+ * @node:` Firmware node matching with for function
+ *
+ * This structure holds information about pin control function
+ * and function group names supporting that function.
+ */
+struct zynqmp_pmux_function {
+   char name[MAX_FUNC_NAME_LEN];
+   const char * const *groups;
+   unsigned int ngroups;
+};
+
+/**
+ * struct zynqmp_pinctrl - driver data
+ * @pctrl: Pinctrl device
+ * @groups:Pingroups
+ * @ngroups:   Number of @groups
+ * @funcs: Pinmux functions
+ * @nfuncs:Number of @funcs
+ *
+ * This struct is stored as driver data and used to retrieve
+ * information regarding pin control functions, groups and
+ * group pins.
+ */
+struct zynqmp_pinctrl {
+   struct pinctrl_dev *pctrl;
+   const struct zynqmp_pctrl_group *groups;
+   unsigned int ngroups;
+   const struct zynqmp_pmux_function *funcs;
+   unsigned int nfuncs;
+};
+
+/**
+ * struct zynqmp_pctrl_group - Pin control group info
+ * @name:  Group name
+ * @pins:  Group pin numbers
+ * @npins: Number of pins in group
+ */
+struct zynqmp_pctrl_group {
+   const char *name;
+   unsigned int pins[MAX_GROUP_PIN];
+   unsigned int npins;
+};
+
+/**
+ * enum zynqmp_pin_config_param - possible pin configuration parameters
+ * @PIN_CONFIG_IOSTANDARD: if the pin can select an IO standard,
+ * the argument to this parameter (on a
+ * custom format) tells the driver which
+ * alternative IO standard to use
+ * @PIN_CONFIG_SCHMITTCMOS:this parameter (on a custom format) allows
+ * to select schmitt or cmos input for MIO pins
+ */
+enum zynqmp_pin_config_param {
+   PIN_CONFIG_IOS

[PATCH 0/3] pinctrl: Added ZynqMP pinctrl driver

2020-12-09 Thread Sai Krishna Potthuri
Added support for Xilinx ZynqMP pinctrl driver support and
also updated the Xilinx firmware driver to support pinctrl
functionality.
This driver will query the pin information from the firmware
and allow configuring the pins as per the request.

Sai Krishna Potthuri (3):
  firmware: xilinx: Added pinctrl support
  dt-bindings: pinctrl: Added binding for ZynqMP pinctrl driver
  pinctrl: Added Xilinx ZynqMP pinctrl driver support

 .../bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml |  329 ++
 drivers/firmware/xilinx/zynqmp.c  |  114 ++
 drivers/pinctrl/Kconfig   |   13 +
 drivers/pinctrl/Makefile  |1 +
 drivers/pinctrl/pinctrl-zynqmp.c  | 1031 +
 include/dt-bindings/pinctrl/pinctrl-zynqmp.h  |   29 +
 include/linux/firmware/xlnx-zynqmp.h  |   90 ++
 7 files changed, 1607 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/pinctrl/xlnx,zynqmp-pinctrl.yaml
 create mode 100644 drivers/pinctrl/pinctrl-zynqmp.c
 create mode 100644 include/dt-bindings/pinctrl/pinctrl-zynqmp.h

-- 
2.17.1



[PATCH 1/3] firmware: xilinx: Added pinctrl support

2020-12-09 Thread Sai Krishna Potthuri
Add pinctrl support to query platform specific information (pins)
from firmware.

Signed-off-by: Sai Krishna Potthuri 
---
 drivers/firmware/xilinx/zynqmp.c | 114 +++
 include/linux/firmware/xlnx-zynqmp.h |  90 +
 2 files changed, 204 insertions(+)

diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index efb8a66efc68..299c3d5a9ebd 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -784,6 +784,120 @@ int zynqmp_pm_fpga_get_status(u32 *value)
 }
 EXPORT_SYMBOL_GPL(zynqmp_pm_fpga_get_status);
 
+/**
+ * zynqmp_pm_pinctrl_request - Request Pin from firmware
+ * @pin: Pin number to request
+ *
+ * This function requests pin from firmware.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_request(const u32 pin)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_REQUEST, pin, 0, 0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_request);
+
+/**
+ * zynqmp_pm_pinctrl_release - Inform firmware that Pin control is released
+ * @pin: Pin number to release
+ *
+ * This function release pin from firmware.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_release(const u32 pin)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_RELEASE, pin, 0, 0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_release);
+
+/**
+ * zynqmp_pm_pinctrl_get_function - Read function id set for the given pin
+ * @pin: Pin number
+ * @id: Buffer to store function ID
+ *
+ * This function provides the function currently set for the given pin.
+ *
+ * Return: Returns status, either success or error+reason
+ */
+int zynqmp_pm_pinctrl_get_function(const u32 pin, u32 *id)
+{
+   u32 ret_payload[PAYLOAD_ARG_CNT];
+   int ret;
+
+   if (!id)
+   return -EINVAL;
+
+   ret = zynqmp_pm_invoke_fn(PM_PINCTRL_GET_FUNCTION, pin, 0,
+ 0, 0, ret_payload);
+   *id = ret_payload[1];
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_get_function);
+
+/**
+ * zynqmp_pm_pinctrl_set_function - Set requested function for the pin
+ * @pin: Pin number
+ * @id: Function ID to set
+ *
+ * This function sets requested function for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_set_function(const u32 pin, const u32 id)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_SET_FUNCTION, pin, id,
+  0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_function);
+
+/**
+ * zynqmp_pm_pinctrl_get_config - Get configuration parameter for the pin
+ * @pin: Pin number
+ * @param: Parameter to get
+ * @value: Buffer to store parameter value
+ *
+ * This function gets requested configuration parameter for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_get_config(const u32 pin, const u32 param,
+u32 *value)
+{
+   u32 ret_payload[PAYLOAD_ARG_CNT];
+   int ret;
+
+   if (!value)
+   return -EINVAL;
+
+   ret = zynqmp_pm_invoke_fn(PM_PINCTRL_CONFIG_PARAM_GET, pin, param,
+ 0, 0, ret_payload);
+   *value = ret_payload[1];
+
+   return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_get_config);
+
+/**
+ * zynqmp_pm_pinctrl_set_config - Set configuration parameter for the pin
+ * @pin: Pin number
+ * @param: Parameter to set
+ * @value: Parameter value to set
+ *
+ * This function sets requested configuration parameter for the given pin.
+ *
+ * Return: Returns status, either success or error+reason.
+ */
+int zynqmp_pm_pinctrl_set_config(const u32 pin, const u32 param,
+u32 value)
+{
+   return zynqmp_pm_invoke_fn(PM_PINCTRL_CONFIG_PARAM_SET, pin,
+  param, value, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_pinctrl_set_config);
+
 /**
  * zynqmp_pm_init_finalize() - PM call to inform firmware that the caller
  *master has initialized its own power management
diff --git a/include/linux/firmware/xlnx-zynqmp.h 
b/include/linux/firmware/xlnx-zynqmp.h
index 5968df82b991..75aa6a5afa28 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -74,6 +74,12 @@ enum pm_api_id {
PM_FPGA_LOAD,
PM_FPGA_GET_STATUS,
PM_GET_CHIPID = 24,
+   PM_PINCTRL_REQUEST = 28,
+   PM_PINCTRL_RELEASE = 29,
+   PM_PINCTRL_GET_FUNCTION = 30,
+   PM_PINCTRL_SET_FUNCTION = 31,
+   PM_PINCTRL_CONFIG_PARAM_GET = 32,
+   PM_PINCTRL_CONFIG_PARAM_SET = 33,
PM_IOCTL = 34,
PM_QUERY_DATA,
PM_CLOCK_ENABLE,
@@ -125,6 +131,12 @@ enum pm_query_id {
PM_QID_CLOCK_GET_FIXEDFACTOR_PARAMS,
PM_QID_CLOCK_GET_PARENTS,
PM_QID_CLOCK_GET_ATTRIBUTES,
+   PM_QID_PINCTRL_GET_NUM_PINS = 6

[PATCH v2 2/2] reset: reset-zynqmp: Added support for Versal platform

2020-07-22 Thread Sai Krishna Potthuri
Updated the reset driver to support Versal platform.
As part of adding Versal support
- Added Versal specific compatible string.
- Reset Id and number of resets are different for Versal and ZynqMP,
hence taken care of these two based on compatible string.

Signed-off-by: Sai Krishna Potthuri 
---
 drivers/reset/reset-zynqmp.c | 50 +++-
 1 file changed, 44 insertions(+), 6 deletions(-)

diff --git a/drivers/reset/reset-zynqmp.c b/drivers/reset/reset-zynqmp.c
index 373ea8d4f7a1..ebd433fa09dd 100644
--- a/drivers/reset/reset-zynqmp.c
+++ b/drivers/reset/reset-zynqmp.c
@@ -9,12 +9,20 @@
 #include 
 #include 
 #include 
+#include 
 
 #define ZYNQMP_NR_RESETS (ZYNQMP_PM_RESET_END - ZYNQMP_PM_RESET_START)
 #define ZYNQMP_RESET_ID ZYNQMP_PM_RESET_START
+#define VERSAL_NR_RESETS   95
+
+struct zynqmp_reset_soc_data {
+   u32 reset_id;
+   u32 num_resets;
+};
 
 struct zynqmp_reset_data {
struct reset_controller_dev rcdev;
+   const struct zynqmp_reset_soc_data *data;
 };
 
 static inline struct zynqmp_reset_data *
@@ -26,23 +34,28 @@ to_zynqmp_reset_data(struct reset_controller_dev *rcdev)
 static int zynqmp_reset_assert(struct reset_controller_dev *rcdev,
   unsigned long id)
 {
-   return zynqmp_pm_reset_assert(ZYNQMP_RESET_ID + id,
+   struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
+
+   return zynqmp_pm_reset_assert(priv->data->reset_id + id,
  PM_RESET_ACTION_ASSERT);
 }
 
 static int zynqmp_reset_deassert(struct reset_controller_dev *rcdev,
 unsigned long id)
 {
-   return zynqmp_pm_reset_assert(ZYNQMP_RESET_ID + id,
+   struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
+
+   return zynqmp_pm_reset_assert(priv->data->reset_id + id,
  PM_RESET_ACTION_RELEASE);
 }
 
 static int zynqmp_reset_status(struct reset_controller_dev *rcdev,
   unsigned long id)
 {
+   struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
int val, err;
 
-   err = zynqmp_pm_reset_get_status(ZYNQMP_RESET_ID + id, );
+   err = zynqmp_pm_reset_get_status(priv->data->reset_id + id, );
if (err)
return err;
 
@@ -52,10 +65,28 @@ static int zynqmp_reset_status(struct reset_controller_dev 
*rcdev,
 static int zynqmp_reset_reset(struct reset_controller_dev *rcdev,
  unsigned long id)
 {
-   return zynqmp_pm_reset_assert(ZYNQMP_RESET_ID + id,
+   struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
+
+   return zynqmp_pm_reset_assert(priv->data->reset_id + id,
  PM_RESET_ACTION_PULSE);
 }
 
+static int zynqmp_reset_of_xlate(struct reset_controller_dev *rcdev,
+const struct of_phandle_args *reset_spec)
+{
+   return reset_spec->args[0];
+}
+
+static const struct zynqmp_reset_soc_data zynqmp_reset_data = {
+   .reset_id = ZYNQMP_RESET_ID,
+   .num_resets = ZYNQMP_NR_RESETS,
+};
+
+static const struct zynqmp_reset_soc_data versal_reset_data = {
+.reset_id = 0,
+.num_resets = VERSAL_NR_RESETS,
+};
+
 static const struct reset_control_ops zynqmp_reset_ops = {
.reset = zynqmp_reset_reset,
.assert = zynqmp_reset_assert,
@@ -71,18 +102,25 @@ static int zynqmp_reset_probe(struct platform_device *pdev)
if (!priv)
return -ENOMEM;
 
+   priv->data = of_device_get_match_data(>dev);
+   if (!priv->data)
+   return -EINVAL;
+
platform_set_drvdata(pdev, priv);
 
priv->rcdev.ops = _reset_ops;
priv->rcdev.owner = THIS_MODULE;
priv->rcdev.of_node = pdev->dev.of_node;
-   priv->rcdev.nr_resets = ZYNQMP_NR_RESETS;
+   priv->rcdev.nr_resets = priv->data->num_resets;
+   priv->rcdev.of_reset_n_cells = 1;
+   priv->rcdev.of_xlate = zynqmp_reset_of_xlate;
 
return devm_reset_controller_register(>dev, >rcdev);
 }
 
 static const struct of_device_id zynqmp_reset_dt_ids[] = {
-   { .compatible = "xlnx,zynqmp-reset", },
+   { .compatible = "xlnx,zynqmp-reset", .data = _reset_data, },
+   { .compatible = "xlnx,versal-reset", .data = _reset_data, },
{ /* sentinel */ },
 };
 
-- 
2.17.1



[PATCH v2 0/2] reset: reset-zynqmp: Added Versal platform support

2020-07-22 Thread Sai Krishna Potthuri
Extended the ZynqMP reset driver to support Versal platform, accordingly
updated the dt-binding with the Versal platform specific information
like compatible string and reset indices.

Changes in v2:
 - Updated 1/2 patch commit description with some explanation about
reset indices.
 - 2/2 patch, Created SOC specific data structure and initialize the
const data based on of_device_get_match_data.
 - Defined driver specific of_xlate callback.

Sai Krishna Potthuri (2):
  dt-bindings: reset: Updated binding for Versal reset driver
  reset: reset-zynqmp: Added support for Versal platform

 .../bindings/reset/xlnx,zynqmp-reset.txt  |  11 +-
 drivers/reset/reset-zynqmp.c  |  50 -
 .../dt-bindings/reset/xlnx-versal-resets.h| 105 ++
 3 files changed, 156 insertions(+), 10 deletions(-)
 create mode 100644 include/dt-bindings/reset/xlnx-versal-resets.h

-- 
2.17.1



[PATCH v2 1/2] dt-bindings: reset: Updated binding for Versal reset driver

2020-07-22 Thread Sai Krishna Potthuri
Added documentation and Versal reset indices to describe
about Versal reset driver bindings.
In Versal all reset indices includes Class, SubClass, Type, Index
information whereas class refers to clock, reset, power etc.,
Underlying firmware in Versal have such classification and expects
the ID to be this way.
[13:0] - Index bits
[19:14] - Type bits
[25:20] - SubClass bits
[31:26] - Class bits.

Signed-off-by: Sai Krishna Potthuri 
---
 .../bindings/reset/xlnx,zynqmp-reset.txt  |  11 +-
 .../dt-bindings/reset/xlnx-versal-resets.h| 105 ++
 2 files changed, 112 insertions(+), 4 deletions(-)
 create mode 100644 include/dt-bindings/reset/xlnx-versal-resets.h

diff --git a/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.txt 
b/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.txt
index 27a45fe5ecf1..ed836868dbf1 100644
--- a/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.txt
+++ b/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.txt
@@ -1,7 +1,7 @@
 --
- =  Zynq UltraScale+ MPSoC reset driver binding =
+ =  Zynq UltraScale+ MPSoC and Versal reset driver binding =
 --
-The Zynq UltraScale+ MPSoC has several different resets.
+The Zynq UltraScale+ MPSoC and Versal has several different resets.
 
 See Chapter 36 of the Zynq UltraScale+ MPSoC TRM (UG) for more information
 about zynqmp resets.
@@ -10,7 +10,8 @@ Please also refer to reset.txt in this directory for common 
reset
 controller binding usage.
 
 Required Properties:
-- compatible:  "xlnx,zynqmp-reset"
+- compatible:  "xlnx,zynqmp-reset" for Zynq UltraScale+ MPSoC platform
+   "xlnx,versal-reset" for Versal platform
 - #reset-cells:Specifies the number of cells needed to encode reset
line, should be 1
 
@@ -37,8 +38,10 @@ Device nodes that need access to reset lines should
 specify them as a reset phandle in their corresponding node as
 specified in reset.txt.
 
-For list of all valid reset indicies see
+For list of all valid reset indices for Zynq UltraScale+ MPSoC see
 
+For list of all valid reset indices for Versal see
+
 
 Example:
 
diff --git a/include/dt-bindings/reset/xlnx-versal-resets.h 
b/include/dt-bindings/reset/xlnx-versal-resets.h
new file mode 100644
index ..895424e9b0e5
--- /dev/null
+++ b/include/dt-bindings/reset/xlnx-versal-resets.h
@@ -0,0 +1,105 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *  Copyright (C) 2020 Xilinx, Inc.
+ */
+
+#ifndef _DT_BINDINGS_VERSAL_RESETS_H
+#define _DT_BINDINGS_VERSAL_RESETS_H
+
+#define VERSAL_RST_PMC_POR (0xc30c001U)
+#define VERSAL_RST_PMC (0xc410002U)
+#define VERSAL_RST_PS_POR  (0xc30c003U)
+#define VERSAL_RST_PL_POR  (0xc30c004U)
+#define VERSAL_RST_NOC_POR (0xc30c005U)
+#define VERSAL_RST_FPD_POR (0xc30c006U)
+#define VERSAL_RST_ACPU_0_POR  (0xc30c007U)
+#define VERSAL_RST_ACPU_1_POR  (0xc30c008U)
+#define VERSAL_RST_OCM2_POR(0xc30c009U)
+#define VERSAL_RST_PS_SRST (0xc41000aU)
+#define VERSAL_RST_PL_SRST (0xc41000bU)
+#define VERSAL_RST_NOC (0xc41000cU)
+#define VERSAL_RST_NPI (0xc41000dU)
+#define VERSAL_RST_SYS_RST_1   (0xc41000eU)
+#define VERSAL_RST_SYS_RST_2   (0xc41000fU)
+#define VERSAL_RST_SYS_RST_3   (0xc410010U)
+#define VERSAL_RST_FPD (0xc410011U)
+#define VERSAL_RST_PL0 (0xc410012U)
+#define VERSAL_RST_PL1 (0xc410013U)
+#define VERSAL_RST_PL2 (0xc410014U)
+#define VERSAL_RST_PL3 (0xc410015U)
+#define VERSAL_RST_APU (0xc410016U)
+#define VERSAL_RST_ACPU_0  (0xc410017U)
+#define VERSAL_RST_ACPU_1  (0xc410018U)
+#define VERSAL_RST_ACPU_L2 (0xc410019U)
+#define VERSAL_RST_ACPU_GIC(0xc41001aU)
+#define VERSAL_RST_RPU_ISLAND  (0xc41001bU)
+#define VERSAL_RST_RPU_AMBA(0xc41001cU)
+#define VERSAL_RST_R5_0(0xc41001dU)
+#define VERSAL_RST_R5_1(0xc41001eU)
+#define VERSAL_RST_SYSMON_PMC_SEQ_RST  (0xc41001fU)
+#define VERSAL_RST_SYSMON_PMC_CFG_RST  (0xc410020U)
+#define VERSAL_RST_SYSMON_FPD_CFG_RST  (0xc410021U)
+#define VERSAL_RST_SYSMON_FPD_SEQ_RST  (0xc410022U)
+#define VERSAL_RST_SYSMON_LPD  (0xc410023U)
+#define VERSAL_RST_PDMA_RST1   (0xc410024U)
+#define VERSAL_RST_PDMA_RST0   (0xc410

RE: [PATCH 1/2] dt-bindings: reset: Updated binding for Versal reset driver

2020-07-21 Thread Sai Krishna Potthuri
Hi Rob,

Thanks for the review

> -Original Message-
> From: Rob Herring 
> Sent: Tuesday, July 21, 2020 7:38 AM
> To: Sai Krishna Potthuri 
> Cc: Philipp Zabel ; Michal Simek
> ; devicet...@vger.kernel.org; linux-arm-
> ker...@lists.infradead.org; linux-kernel@vger.kernel.org; git
> ; saikrishna12...@gmail.com
> Subject: Re: [PATCH 1/2] dt-bindings: reset: Updated binding for Versal reset
> driver
> 
> On Tue, Jul 14, 2020 at 11:59:08AM +0530, Sai Krishna Potthuri wrote:
> > Added documentation and Versal reset indices to describe about Versal
> > reset driver bindings.
> >
> > Signed-off-by: Sai Krishna Potthuri
> > 
> > ---
> >  .../bindings/reset/xlnx,zynqmp-reset.txt  |  11 +-
> >  .../dt-bindings/reset/xlnx-versal-resets.h| 105 ++
> >  2 files changed, 112 insertions(+), 4 deletions(-)  create mode
> > 100644 include/dt-bindings/reset/xlnx-versal-resets.h
> >
> > diff --git
> > a/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.txt
> > b/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.txt
> > index 27a45fe5ecf1..ed836868dbf1 100644
> > --- a/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.txt
> > +++ b/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.txt
> > @@ -1,7 +1,7 @@
> >
> > --
> > 
> > - =  Zynq UltraScale+ MPSoC reset driver binding =
> > + =  Zynq UltraScale+ MPSoC and Versal reset driver binding =
> >
> > --
> >  -The Zynq UltraScale+ MPSoC has several different resets.
> > +The Zynq UltraScale+ MPSoC and Versal has several different resets.
> >
> >  See Chapter 36 of the Zynq UltraScale+ MPSoC TRM (UG) for more
> > information  about zynqmp resets.
> > @@ -10,7 +10,8 @@ Please also refer to reset.txt in this directory for
> > common reset  controller binding usage.
> >
> >  Required Properties:
> > -- compatible:  "xlnx,zynqmp-reset"
> > +- compatible:  "xlnx,zynqmp-reset" for Zynq UltraScale+ MPSoC
> platform
> > +   "xlnx,versal-reset" for Versal platform
> >  - #reset-cells:Specifies the number of cells needed to encode reset
> > line, should be 1
> >
> > @@ -37,8 +38,10 @@ Device nodes that need access to reset lines should
> > specify them as a reset phandle in their corresponding node as
> > specified in reset.txt.
> >
> > -For list of all valid reset indicies see
> > +For list of all valid reset indices for Zynq UltraScale+ MPSoC see
> >  
> > +For list of all valid reset indices for Versal see
> > +
> >
> >  Example:
> >
> > diff --git a/include/dt-bindings/reset/xlnx-versal-resets.h
> > b/include/dt-bindings/reset/xlnx-versal-resets.h
> > new file mode 100644
> > index ..895424e9b0e5
> > --- /dev/null
> > +++ b/include/dt-bindings/reset/xlnx-versal-resets.h
> > @@ -0,0 +1,105 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + *  Copyright (C) 2020 Xilinx, Inc.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_VERSAL_RESETS_H
> > +#define _DT_BINDINGS_VERSAL_RESETS_H
> > +
> > +#define VERSAL_RST_PMC_POR (0xc30c001U)
> 
> Perhaps some explanation on the numbering as ZynqMP seems to be just an
> index.
In versal, all ID's includes Class, SubClass, Type, Index information whereas 
class
refers to clock, reset, power etc., Underlying firmware in Versal have such
classification and expects the ID to be this way.
[13:0] - Index bits
[19:14] - Type bits
[25:20] - SubClass bits
[31:26] - Class bits
I will add this information in this file in v2.

Regards
Sai Krishna
> 
> > +#define VERSAL_RST_PMC (0xc410002U)
> > +#define VERSAL_RST_PS_POR  (0xc30c003U)
> > +#define VERSAL_RST_PL_POR  (0xc30c004U)
> > +#define VERSAL_RST_NOC_POR (0xc30c005U)
> > +#define VERSAL_RST_FPD_POR (0xc30c006U)
> > +#define VERSAL_RST_ACPU_0_POR  (0xc30c007U)
> > +#define VERSAL_RST_ACPU_1_POR  (0xc30c008U)
> > +#define VERSAL_RST_OCM2_POR(0xc30c009U)
> > +#define VERSAL_RST_PS_SRST (0xc41000aU)
> > +#define VERSAL_RST_PL_SRST (0xc41000bU)
> > +#define VERSAL_RST_NOC (0xc41000cU)
> > +#define VERSAL_RST_NPI (0xc41000dU)
> > +#define

RE: [PATCH 2/2] reset: reset-zynqmp: Added support for Versal platform

2020-07-20 Thread Sai Krishna Potthuri
Hi Philipp,

Thanks for the review.

> -Original Message-
> From: Philipp Zabel 
> Sent: Monday, July 20, 2020 3:18 PM
> To: Sai Krishna Potthuri ; Rob Herring
> ; Michal Simek 
> Cc: devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; linux-
> ker...@vger.kernel.org; git ; saikrishna12...@gmail.com
> Subject: Re: [PATCH 2/2] reset: reset-zynqmp: Added support for Versal
> platform
> 
> On Tue, 2020-07-14 at 11:59 +0530, Sai Krishna Potthuri wrote:
> > Updated the reset driver to support Versal platform.
> > As part of adding Versal support
> > - Added Versal specific compatible string.
> > - Reset Id and number of resets are different for Versal and ZynqMP,
> > hence taken care of these two based on compatible string.
> >
> > Signed-off-by: Sai Krishna Potthuri
> > 
> > ---
> >  drivers/reset/reset-zynqmp.c | 24 
> >  1 file changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/reset/reset-zynqmp.c
> > b/drivers/reset/reset-zynqmp.c index 373ea8d4f7a1..17aa4532ec5e
> 100644
> > --- a/drivers/reset/reset-zynqmp.c
> > +++ b/drivers/reset/reset-zynqmp.c
> > @@ -12,9 +12,11 @@
> >
> >  #define ZYNQMP_NR_RESETS (ZYNQMP_PM_RESET_END -
> > ZYNQMP_PM_RESET_START)  #define ZYNQMP_RESET_ID
> ZYNQMP_PM_RESET_START
> > +#define VERSAL_NR_RESETS   95
> >
> >  struct zynqmp_reset_data {
> > struct reset_controller_dev rcdev;
> > +   u32 reset_id;
> >  };
> >
> >  static inline struct zynqmp_reset_data * @@ -26,23 +28,28 @@
> > to_zynqmp_reset_data(struct reset_controller_dev *rcdev)  static int
> > zynqmp_reset_assert(struct reset_controller_dev *rcdev,
> >unsigned long id)
> >  {
> > -   return zynqmp_pm_reset_assert(ZYNQMP_RESET_ID + id,
> > +   struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
> > +
> > +   return zynqmp_pm_reset_assert(priv->reset_id + id,
> >   PM_RESET_ACTION_ASSERT);
> >  }
> >
> >  static int zynqmp_reset_deassert(struct reset_controller_dev *rcdev,
> >  unsigned long id)
> >  {
> > -   return zynqmp_pm_reset_assert(ZYNQMP_RESET_ID + id,
> > +   struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
> > +
> > +   return zynqmp_pm_reset_assert(priv->reset_id + id,
> >   PM_RESET_ACTION_RELEASE);
> >  }
> >
> >  static int zynqmp_reset_status(struct reset_controller_dev *rcdev,
> >unsigned long id)
> >  {
> > +   struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
> > int val, err;
> >
> > -   err = zynqmp_pm_reset_get_status(ZYNQMP_RESET_ID + id, );
> > +   err = zynqmp_pm_reset_get_status(priv->reset_id + id, );
> > if (err)
> > return err;
> >
> > @@ -52,7 +59,9 @@ static int zynqmp_reset_status(struct
> > reset_controller_dev *rcdev,  static int zynqmp_reset_reset(struct
> reset_controller_dev *rcdev,
> >   unsigned long id)
> >  {
> > -   return zynqmp_pm_reset_assert(ZYNQMP_RESET_ID + id,
> > +   struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
> > +
> > +   return zynqmp_pm_reset_assert(priv->reset_id + id,
> >   PM_RESET_ACTION_PULSE);
> >  }
> >
> > @@ -76,13 +85,20 @@ static int zynqmp_reset_probe(struct
> platform_device *pdev)
> > priv->rcdev.ops = _reset_ops;
> > priv->rcdev.owner = THIS_MODULE;
> > priv->rcdev.of_node = pdev->dev.of_node;
> > +   priv->reset_id = ZYNQMP_RESET_ID;
> > priv->rcdev.nr_resets = ZYNQMP_NR_RESETS;
> > +   if (of_device_is_compatible(pdev->dev.of_node,
> > +   "xlnx,versal-reset")) {
> 
> It would be better to use of_match_device and static const initalization data
> for this.
Will create soc specific initialization data structure and assign based on
of_device_get_match_data().

> 
> > +   priv->reset_id = 0;
> > +   priv->rcdev.nr_resets = VERSAL_NR_RESETS;
> 
> This won't work. All your reset ids are greater than 95, and this driver is 
> using
> the default of_xlate callback, so of_reset_simple_xlate will fail all reset
> control requests with -EINVAL.
Will create of_xlate callback that will simply return the reset line number
without any checks. We have underlying secure library which will validate
the reset line number.

Regards
Sai Krishna
> 
> regards
> Philipp


[PATCH 1/2] dt-bindings: reset: Updated binding for Versal reset driver

2020-07-14 Thread Sai Krishna Potthuri
Added documentation and Versal reset indices to describe
about Versal reset driver bindings.

Signed-off-by: Sai Krishna Potthuri 
---
 .../bindings/reset/xlnx,zynqmp-reset.txt  |  11 +-
 .../dt-bindings/reset/xlnx-versal-resets.h| 105 ++
 2 files changed, 112 insertions(+), 4 deletions(-)
 create mode 100644 include/dt-bindings/reset/xlnx-versal-resets.h

diff --git a/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.txt 
b/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.txt
index 27a45fe5ecf1..ed836868dbf1 100644
--- a/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.txt
+++ b/Documentation/devicetree/bindings/reset/xlnx,zynqmp-reset.txt
@@ -1,7 +1,7 @@
 --
- =  Zynq UltraScale+ MPSoC reset driver binding =
+ =  Zynq UltraScale+ MPSoC and Versal reset driver binding =
 --
-The Zynq UltraScale+ MPSoC has several different resets.
+The Zynq UltraScale+ MPSoC and Versal has several different resets.
 
 See Chapter 36 of the Zynq UltraScale+ MPSoC TRM (UG) for more information
 about zynqmp resets.
@@ -10,7 +10,8 @@ Please also refer to reset.txt in this directory for common 
reset
 controller binding usage.
 
 Required Properties:
-- compatible:  "xlnx,zynqmp-reset"
+- compatible:  "xlnx,zynqmp-reset" for Zynq UltraScale+ MPSoC platform
+   "xlnx,versal-reset" for Versal platform
 - #reset-cells:Specifies the number of cells needed to encode reset
line, should be 1
 
@@ -37,8 +38,10 @@ Device nodes that need access to reset lines should
 specify them as a reset phandle in their corresponding node as
 specified in reset.txt.
 
-For list of all valid reset indicies see
+For list of all valid reset indices for Zynq UltraScale+ MPSoC see
 
+For list of all valid reset indices for Versal see
+
 
 Example:
 
diff --git a/include/dt-bindings/reset/xlnx-versal-resets.h 
b/include/dt-bindings/reset/xlnx-versal-resets.h
new file mode 100644
index ..895424e9b0e5
--- /dev/null
+++ b/include/dt-bindings/reset/xlnx-versal-resets.h
@@ -0,0 +1,105 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *  Copyright (C) 2020 Xilinx, Inc.
+ */
+
+#ifndef _DT_BINDINGS_VERSAL_RESETS_H
+#define _DT_BINDINGS_VERSAL_RESETS_H
+
+#define VERSAL_RST_PMC_POR (0xc30c001U)
+#define VERSAL_RST_PMC (0xc410002U)
+#define VERSAL_RST_PS_POR  (0xc30c003U)
+#define VERSAL_RST_PL_POR  (0xc30c004U)
+#define VERSAL_RST_NOC_POR (0xc30c005U)
+#define VERSAL_RST_FPD_POR (0xc30c006U)
+#define VERSAL_RST_ACPU_0_POR  (0xc30c007U)
+#define VERSAL_RST_ACPU_1_POR  (0xc30c008U)
+#define VERSAL_RST_OCM2_POR(0xc30c009U)
+#define VERSAL_RST_PS_SRST (0xc41000aU)
+#define VERSAL_RST_PL_SRST (0xc41000bU)
+#define VERSAL_RST_NOC (0xc41000cU)
+#define VERSAL_RST_NPI (0xc41000dU)
+#define VERSAL_RST_SYS_RST_1   (0xc41000eU)
+#define VERSAL_RST_SYS_RST_2   (0xc41000fU)
+#define VERSAL_RST_SYS_RST_3   (0xc410010U)
+#define VERSAL_RST_FPD (0xc410011U)
+#define VERSAL_RST_PL0 (0xc410012U)
+#define VERSAL_RST_PL1 (0xc410013U)
+#define VERSAL_RST_PL2 (0xc410014U)
+#define VERSAL_RST_PL3 (0xc410015U)
+#define VERSAL_RST_APU (0xc410016U)
+#define VERSAL_RST_ACPU_0  (0xc410017U)
+#define VERSAL_RST_ACPU_1  (0xc410018U)
+#define VERSAL_RST_ACPU_L2 (0xc410019U)
+#define VERSAL_RST_ACPU_GIC(0xc41001aU)
+#define VERSAL_RST_RPU_ISLAND  (0xc41001bU)
+#define VERSAL_RST_RPU_AMBA(0xc41001cU)
+#define VERSAL_RST_R5_0(0xc41001dU)
+#define VERSAL_RST_R5_1(0xc41001eU)
+#define VERSAL_RST_SYSMON_PMC_SEQ_RST  (0xc41001fU)
+#define VERSAL_RST_SYSMON_PMC_CFG_RST  (0xc410020U)
+#define VERSAL_RST_SYSMON_FPD_CFG_RST  (0xc410021U)
+#define VERSAL_RST_SYSMON_FPD_SEQ_RST  (0xc410022U)
+#define VERSAL_RST_SYSMON_LPD  (0xc410023U)
+#define VERSAL_RST_PDMA_RST1   (0xc410024U)
+#define VERSAL_RST_PDMA_RST0   (0xc410025U)
+#define VERSAL_RST_ADMA(0xc410026U)
+#define VERSAL_RST_TIMESTAMP   (0xc410027U)
+#define VERSAL_RST_OCM (0xc410028U)
+#define VERSAL_RST_OCM2_RST(0xc410029U)
+#define VERSAL_RST_IPI (0xc41

[PATCH 2/2] reset: reset-zynqmp: Added support for Versal platform

2020-07-14 Thread Sai Krishna Potthuri
Updated the reset driver to support Versal platform.
As part of adding Versal support
- Added Versal specific compatible string.
- Reset Id and number of resets are different for Versal and ZynqMP,
hence taken care of these two based on compatible string.

Signed-off-by: Sai Krishna Potthuri 
---
 drivers/reset/reset-zynqmp.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/reset/reset-zynqmp.c b/drivers/reset/reset-zynqmp.c
index 373ea8d4f7a1..17aa4532ec5e 100644
--- a/drivers/reset/reset-zynqmp.c
+++ b/drivers/reset/reset-zynqmp.c
@@ -12,9 +12,11 @@
 
 #define ZYNQMP_NR_RESETS (ZYNQMP_PM_RESET_END - ZYNQMP_PM_RESET_START)
 #define ZYNQMP_RESET_ID ZYNQMP_PM_RESET_START
+#define VERSAL_NR_RESETS   95
 
 struct zynqmp_reset_data {
struct reset_controller_dev rcdev;
+   u32 reset_id;
 };
 
 static inline struct zynqmp_reset_data *
@@ -26,23 +28,28 @@ to_zynqmp_reset_data(struct reset_controller_dev *rcdev)
 static int zynqmp_reset_assert(struct reset_controller_dev *rcdev,
   unsigned long id)
 {
-   return zynqmp_pm_reset_assert(ZYNQMP_RESET_ID + id,
+   struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
+
+   return zynqmp_pm_reset_assert(priv->reset_id + id,
  PM_RESET_ACTION_ASSERT);
 }
 
 static int zynqmp_reset_deassert(struct reset_controller_dev *rcdev,
 unsigned long id)
 {
-   return zynqmp_pm_reset_assert(ZYNQMP_RESET_ID + id,
+   struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
+
+   return zynqmp_pm_reset_assert(priv->reset_id + id,
  PM_RESET_ACTION_RELEASE);
 }
 
 static int zynqmp_reset_status(struct reset_controller_dev *rcdev,
   unsigned long id)
 {
+   struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
int val, err;
 
-   err = zynqmp_pm_reset_get_status(ZYNQMP_RESET_ID + id, );
+   err = zynqmp_pm_reset_get_status(priv->reset_id + id, );
if (err)
return err;
 
@@ -52,7 +59,9 @@ static int zynqmp_reset_status(struct reset_controller_dev 
*rcdev,
 static int zynqmp_reset_reset(struct reset_controller_dev *rcdev,
  unsigned long id)
 {
-   return zynqmp_pm_reset_assert(ZYNQMP_RESET_ID + id,
+   struct zynqmp_reset_data *priv = to_zynqmp_reset_data(rcdev);
+
+   return zynqmp_pm_reset_assert(priv->reset_id + id,
  PM_RESET_ACTION_PULSE);
 }
 
@@ -76,13 +85,20 @@ static int zynqmp_reset_probe(struct platform_device *pdev)
priv->rcdev.ops = _reset_ops;
priv->rcdev.owner = THIS_MODULE;
priv->rcdev.of_node = pdev->dev.of_node;
+   priv->reset_id = ZYNQMP_RESET_ID;
priv->rcdev.nr_resets = ZYNQMP_NR_RESETS;
+   if (of_device_is_compatible(pdev->dev.of_node,
+   "xlnx,versal-reset")) {
+   priv->reset_id = 0;
+   priv->rcdev.nr_resets = VERSAL_NR_RESETS;
+   }
 
return devm_reset_controller_register(>dev, >rcdev);
 }
 
 static const struct of_device_id zynqmp_reset_dt_ids[] = {
{ .compatible = "xlnx,zynqmp-reset", },
+   { .compatible = "xlnx,versal-reset", },
{ /* sentinel */ },
 };
 
-- 
2.17.1



[PATCH 0/2] reset: reset-zynqmp: Added Versal platform support

2020-07-14 Thread Sai Krishna Potthuri
Extended the ZynqMP reset driver to support Versal platform, accordingly
updated the dt-binding with the Versal platform specific information
like compatible string and reset indices.

Sai Krishna Potthuri (2):
  dt-bindings: reset: Updated binding for Versal reset driver
  reset: reset-zynqmp: Added support for Versal platform

 .../bindings/reset/xlnx,zynqmp-reset.txt  |  11 +-
 drivers/reset/reset-zynqmp.c  |  24 +++-
 .../dt-bindings/reset/xlnx-versal-resets.h| 105 ++
 3 files changed, 132 insertions(+), 8 deletions(-)
 create mode 100644 include/dt-bindings/reset/xlnx-versal-resets.h

-- 
2.17.1



RE: [LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the spi_transfer structure.

2016-04-12 Thread Lakshmi Sai Krishna Potthuri
Hi Cyrille and Mark,

>-Original Message-
>From: Cyrille Pitchen [mailto:cyrille.pitc...@atmel.com]
>Sent: Thursday, April 07, 2016 8:33 PM
>To: Lakshmi Sai Krishna Potthuri <laksh...@xilinx.com>; Michal Simek
><mich...@xilinx.com>; Soren Brinkmann <sor...@xilinx.com>; David
>Woodhouse <dw...@infradead.org>; Brian Norris
><computersforpe...@gmail.com>; Mark Brown <broo...@kernel.org>;
>Javier Martinez Canillas <jav...@osg.samsung.com>; Boris Brezillon
><boris.brezil...@free-electrons.com>; Stephen Warren
><swar...@nvidia.com>; Geert Uytterhoeven <geert+rene...@glider.be>;
>Andrew F. Davis <a...@ti.com>; Marek Vasut <ma...@denx.de>; Jagan Teki
><jt...@openedev.com>; Rafał Miłecki <zaj...@gmail.com>
>Cc: linux-...@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
>s...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Harini Katakam
><hari...@xilinx.com>; Punnaiah Choudary Kalluri <punn...@xilinx.com>;
>Anirudha Sarangi <anir...@xilinx.com>; Lakshmi Sai Krishna Potthuri
><laksh...@xilinx.com>; R, Vignesh <vigne...@ti.com>
>Subject: Re: [LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the
>spi_transfer structure.
>
>Hi all,
>
>Le 07/04/2016 16:39, P L Sai Krishna a écrit :
>> This patch adds dummy_cycles entry in the spi_transfer structure.
>> len field in the transfer structure contains dummy bytes along with
>> actual data bytes, controllers which requires dummy bytes use len
>> field and simply Ignore the dummy_cycles field. Controllers which
>> expects dummy cycles won't work directly by using len field because
>> host driver doesn't know that len field of a particular transfer
>> includes dummy bytes or not (and also number of dummy bytes included
>> in len field). In such cases host driver use this dummy_cycles field
>> to identify the number of dummy cycles and based on that it will send
>> the required number of dummy cycles.
>
>Dummy cycles are only used with SPI NOR flashes, aren't they?

Yes, with SPI NOR flashes.

>I guess so since there is also a patch dedicated to the m25p80 driver.
>
>So why not using the spi_flash_read() API already introduced by Vignesh
>in the SPI layer?

ZynqMP GQSPI controller driver use "transfer_one" hook. Since this is generic
Controller majorly interfaced to flash devices and it might extend to NAND
flashes in future. This patch does not disturb the existing implementation of
accessing the flash or other slave devices. It adds an additional optional
feature. So there is no harm to controllers that don't need dummy cycles -
they can ignore it and still work with the existing implementation.

Regards
Sai Krishna
>
>struct spi_flash_read_message already includes a 'dummy_bytes' member.
>
>>
>> Signed-off-by: P L Sai Krishna <laksh...@xilinx.com>
>> ---
>> v2:
>>  - Changed the structure member name from dummy to dummy_cycles.
>>  - Updated the documentation of dummy_cycles.
>>  - m25p80 changes split into another patch.
>>
>>  include/linux/spi/spi.h | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
>> index 857a9a1..63135b3 100644
>> --- a/include/linux/spi/spi.h
>> +++ b/include/linux/spi/spi.h
>> @@ -664,6 +664,9 @@ extern void spi_res_release(struct spi_master
>*master,
>>   * @len: size of rx and tx buffers (in bytes)
>>   * @speed_hz: Select a speed other than the device default for this
>>   *  transfer. If 0 the default (from @spi_device) is used.
>> + * @dummy_cycles: number of dummy cycles. If host controller requires
>> + *  dummy cycles rather than dummy bytes which send along with Cmd
>> + *  and address then this dummy_cycles is used.
>>   * @bits_per_word: select a bits_per_word other than the device default
>>   *  for this transfer. If 0 the default (from @spi_device) is used.
>>   * @cs_change: affects chipselect after this transfer completes
>> @@ -752,6 +755,7 @@ struct spi_transfer {
>>  u8  bits_per_word;
>>  u16 delay_usecs;
>>  u32 speed_hz;
>> +u32 dummy_cycles;
>>
>>  struct list_head transfer_list;
>>  };
>>
>
>Best regards,
>
>Cyrille


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.



RE: [LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the spi_transfer structure.

2016-04-12 Thread Lakshmi Sai Krishna Potthuri
Hi Cyrille and Mark,

>-Original Message-
>From: Cyrille Pitchen [mailto:cyrille.pitc...@atmel.com]
>Sent: Thursday, April 07, 2016 8:33 PM
>To: Lakshmi Sai Krishna Potthuri ; Michal Simek
>; Soren Brinkmann ; David
>Woodhouse ; Brian Norris
>; Mark Brown ;
>Javier Martinez Canillas ; Boris Brezillon
>; Stephen Warren
>; Geert Uytterhoeven ;
>Andrew F. Davis ; Marek Vasut ; Jagan Teki
>; Rafał Miłecki 
>Cc: linux-...@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
>s...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Harini Katakam
>; Punnaiah Choudary Kalluri ;
>Anirudha Sarangi ; Lakshmi Sai Krishna Potthuri
>; R, Vignesh 
>Subject: Re: [LINUX PATCH v2 1/3] spi: Added dummy_cycle entry in the
>spi_transfer structure.
>
>Hi all,
>
>Le 07/04/2016 16:39, P L Sai Krishna a écrit :
>> This patch adds dummy_cycles entry in the spi_transfer structure.
>> len field in the transfer structure contains dummy bytes along with
>> actual data bytes, controllers which requires dummy bytes use len
>> field and simply Ignore the dummy_cycles field. Controllers which
>> expects dummy cycles won't work directly by using len field because
>> host driver doesn't know that len field of a particular transfer
>> includes dummy bytes or not (and also number of dummy bytes included
>> in len field). In such cases host driver use this dummy_cycles field
>> to identify the number of dummy cycles and based on that it will send
>> the required number of dummy cycles.
>
>Dummy cycles are only used with SPI NOR flashes, aren't they?

Yes, with SPI NOR flashes.

>I guess so since there is also a patch dedicated to the m25p80 driver.
>
>So why not using the spi_flash_read() API already introduced by Vignesh
>in the SPI layer?

ZynqMP GQSPI controller driver use "transfer_one" hook. Since this is generic
Controller majorly interfaced to flash devices and it might extend to NAND
flashes in future. This patch does not disturb the existing implementation of
accessing the flash or other slave devices. It adds an additional optional
feature. So there is no harm to controllers that don't need dummy cycles -
they can ignore it and still work with the existing implementation.

Regards
Sai Krishna
>
>struct spi_flash_read_message already includes a 'dummy_bytes' member.
>
>>
>> Signed-off-by: P L Sai Krishna 
>> ---
>> v2:
>>  - Changed the structure member name from dummy to dummy_cycles.
>>  - Updated the documentation of dummy_cycles.
>>  - m25p80 changes split into another patch.
>>
>>  include/linux/spi/spi.h | 4 
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
>> index 857a9a1..63135b3 100644
>> --- a/include/linux/spi/spi.h
>> +++ b/include/linux/spi/spi.h
>> @@ -664,6 +664,9 @@ extern void spi_res_release(struct spi_master
>*master,
>>   * @len: size of rx and tx buffers (in bytes)
>>   * @speed_hz: Select a speed other than the device default for this
>>   *  transfer. If 0 the default (from @spi_device) is used.
>> + * @dummy_cycles: number of dummy cycles. If host controller requires
>> + *  dummy cycles rather than dummy bytes which send along with Cmd
>> + *  and address then this dummy_cycles is used.
>>   * @bits_per_word: select a bits_per_word other than the device default
>>   *  for this transfer. If 0 the default (from @spi_device) is used.
>>   * @cs_change: affects chipselect after this transfer completes
>> @@ -752,6 +755,7 @@ struct spi_transfer {
>>  u8  bits_per_word;
>>  u16 delay_usecs;
>>  u32 speed_hz;
>> +u32 dummy_cycles;
>>
>>  struct list_head transfer_list;
>>  };
>>
>
>Best regards,
>
>Cyrille


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.



RE: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer structure.

2016-03-31 Thread Lakshmi Sai Krishna Potthuri
Hi,

>-Original Message-
>From: geert.uytterhoe...@gmail.com
>[mailto:geert.uytterhoe...@gmail.com] On Behalf Of Geert Uytterhoeven
>Sent: Thursday, March 31, 2016 1:58 PM
>To: Lakshmi Sai Krishna Potthuri <laksh...@xilinx.com>
>Cc: Mark Brown <broo...@kernel.org>; Michal Simek <mich...@xilinx.com>;
>Soren Brinkmann <sor...@xilinx.com>; David Woodhouse
><dw...@infradead.org>; Brian Norris <computersforpe...@gmail.com>;
>Javier Martinez Canillas <jav...@osg.samsung.com>; Boris Brezillon
><boris.brezil...@free-electrons.com>; Stephen Warren
><swar...@nvidia.com>; Geert Uytterhoeven <geert+rene...@glider.be>;
>Andrew F. Davis <a...@ti.com>; Marek Vasut <ma...@denx.de>; Jagan Teki
><jt...@openedev.com>; Rafał Miłecki <zaj...@gmail.com>; linux-
>m...@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
>s...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Harini Katakam
><hari...@xilinx.com>; Punnaiah Choudary Kalluri <punn...@xilinx.com>;
>Anirudha Sarangi <anir...@xilinx.com>; saikrishna12...@gmail.com
>Subject: Re: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer
>structure.
>
>On Thu, Mar 31, 2016 at 8:14 AM, Lakshmi Sai Krishna Potthuri
><lakshmi.sai.krishna.potth...@xilinx.com> wrote:
>>>> >This is really not what I'd expect to happen, I'd expect that these
>dummy
>>>> >cycles would be in addition to the actual data (see my request for better
>>>> >documentation...).  If they overlap with the data then what is the point
>in
>>>> >specifying this?  It's more work for the host, what benefit do we get
>from
>>>> >doing it over just handing it like a normal byte?
>>>
>>>> len field in the transfer structure contains dummy bytes along with actual
>>>data
>>>> bytes, controllers which requires dummy bytes use len field and simply
>>>Ignore
>>>> the dummy field (contains only no of cycles)added in this patch.
>Controllers
>>>> (like ZynqMP GQSPI) expects dummy in cycles won't work directly by
>using
>>>> len field because host driver doesn't know that len field of a particular
>>>transfer
>>>> includes dummy bytes or not (and also number of dummy bytes included
>in
>>>len
>>>> field). In such cases driver use this dummy field to identify the number of
>>>dummy
>>>> cycles and based on that it will send the required number of dummy
>cycles
>>>(which
>>>> i did in the second patch).
>>>
>>>This doesn't make any sense at all to me.  Why does the controller care
>>>what the bytes being sent to and from the device mean?
>>
>> From the flash point of view, it expects the controller to send the dummy
>> on 1/2/4 lines based on the command. For Quad commands, flash expects
>> 4 lines to be active during dummy phase. Similarly, 2 lines for dual
>> Commands and 1 line for normal/fast commands.
>> Since len field contains total number of cmd+addr+dummy bytes,
>> host driver should take care of sending these bytes on their respective
>> bus widths. Knowing when the dummy is being sent also helps in
>> the correct switching of IO pads (since the data lines are bidirectional)
>> ZynqMP GQSPI is a generic controller, majorly interfaced to flash devices.
>> It seems reasonable for it to know the above information from
>> the flash layer. Adding "dummy" cycles entry should be useful to any
>> controller that cares about it without affecting other spi/qspi controllers.
>
>The m25p80 driver already uses dummy cycles, using real spi_transfer
>structs, which have tx_nbits/rx_nbits fields to indicate how many data lines
>to use.

m25p80 implementation just send command, address and dummy together
with tx_nbit field as 1 and host driver can't differentiate between them.
Command and address go on one line and dummy should send based
on the command as explained my previous mail.

Regards
Sai Krishna


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.



RE: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer structure.

2016-03-31 Thread Lakshmi Sai Krishna Potthuri
Hi,

>-Original Message-
>From: geert.uytterhoe...@gmail.com
>[mailto:geert.uytterhoe...@gmail.com] On Behalf Of Geert Uytterhoeven
>Sent: Thursday, March 31, 2016 1:58 PM
>To: Lakshmi Sai Krishna Potthuri 
>Cc: Mark Brown ; Michal Simek ;
>Soren Brinkmann ; David Woodhouse
>; Brian Norris ;
>Javier Martinez Canillas ; Boris Brezillon
>; Stephen Warren
>; Geert Uytterhoeven ;
>Andrew F. Davis ; Marek Vasut ; Jagan Teki
>; Rafał Miłecki ; linux-
>m...@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
>s...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Harini Katakam
>; Punnaiah Choudary Kalluri ;
>Anirudha Sarangi ; saikrishna12...@gmail.com
>Subject: Re: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer
>structure.
>
>On Thu, Mar 31, 2016 at 8:14 AM, Lakshmi Sai Krishna Potthuri
> wrote:
>>>> >This is really not what I'd expect to happen, I'd expect that these
>dummy
>>>> >cycles would be in addition to the actual data (see my request for better
>>>> >documentation...).  If they overlap with the data then what is the point
>in
>>>> >specifying this?  It's more work for the host, what benefit do we get
>from
>>>> >doing it over just handing it like a normal byte?
>>>
>>>> len field in the transfer structure contains dummy bytes along with actual
>>>data
>>>> bytes, controllers which requires dummy bytes use len field and simply
>>>Ignore
>>>> the dummy field (contains only no of cycles)added in this patch.
>Controllers
>>>> (like ZynqMP GQSPI) expects dummy in cycles won't work directly by
>using
>>>> len field because host driver doesn't know that len field of a particular
>>>transfer
>>>> includes dummy bytes or not (and also number of dummy bytes included
>in
>>>len
>>>> field). In such cases driver use this dummy field to identify the number of
>>>dummy
>>>> cycles and based on that it will send the required number of dummy
>cycles
>>>(which
>>>> i did in the second patch).
>>>
>>>This doesn't make any sense at all to me.  Why does the controller care
>>>what the bytes being sent to and from the device mean?
>>
>> From the flash point of view, it expects the controller to send the dummy
>> on 1/2/4 lines based on the command. For Quad commands, flash expects
>> 4 lines to be active during dummy phase. Similarly, 2 lines for dual
>> Commands and 1 line for normal/fast commands.
>> Since len field contains total number of cmd+addr+dummy bytes,
>> host driver should take care of sending these bytes on their respective
>> bus widths. Knowing when the dummy is being sent also helps in
>> the correct switching of IO pads (since the data lines are bidirectional)
>> ZynqMP GQSPI is a generic controller, majorly interfaced to flash devices.
>> It seems reasonable for it to know the above information from
>> the flash layer. Adding "dummy" cycles entry should be useful to any
>> controller that cares about it without affecting other spi/qspi controllers.
>
>The m25p80 driver already uses dummy cycles, using real spi_transfer
>structs, which have tx_nbits/rx_nbits fields to indicate how many data lines
>to use.

m25p80 implementation just send command, address and dummy together
with tx_nbit field as 1 and host driver can't differentiate between them.
Command and address go on one line and dummy should send based
on the command as explained my previous mail.

Regards
Sai Krishna


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.



RE: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer structure.

2016-03-31 Thread Lakshmi Sai Krishna Potthuri


>-Original Message-
>From: Mark Brown [mailto:broo...@kernel.org]
>Sent: Friday, March 25, 2016 8:31 PM
>To: Lakshmi Sai Krishna Potthuri <laksh...@xilinx.com>
>Cc: Michal Simek <mich...@xilinx.com>; Soren Brinkmann
><sor...@xilinx.com>; David Woodhouse <dw...@infradead.org>; Brian
>Norris <computersforpe...@gmail.com>; Javier Martinez Canillas
><jav...@osg.samsung.com>; Boris Brezillon <boris.brezillon@free-
>electrons.com>; Stephen Warren <swar...@nvidia.com>; Geert
>Uytterhoeven <geert+rene...@glider.be>; Andrew F. Davis <a...@ti.com>;
>Marek Vasut <ma...@denx.de>; Jagan Teki <jt...@openedev.com>; Rafał
>Miłecki <zaj...@gmail.com>; linux-...@lists.infradead.org; linux-
>ker...@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
>ker...@lists.infradead.org; Harini Katakam <hari...@xilinx.com>; Punnaiah
>Choudary Kalluri <punn...@xilinx.com>; Anirudha Sarangi
><anir...@xilinx.com>
>Subject: Re: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer
>structure.
>
>On Fri, Mar 25, 2016 at 01:41:16PM +, Lakshmi Sai Krishna Potthuri wrote:
>
>As I'm fairly sure I've said before please fix your mail client to word
>wrap within paragraphs at something substantially less than 80 columns.
>Doing this makes your messages much easier to read and reply to.
>

Sorry, there was some issue with my mail client, now I corrected it.

>> >This is really not what I'd expect to happen, I'd expect that these dummy
>> >cycles would be in addition to the actual data (see my request for better
>> >documentation...).  If they overlap with the data then what is the point in
>> >specifying this?  It's more work for the host, what benefit do we get from
>> >doing it over just handing it like a normal byte?
>
>> len field in the transfer structure contains dummy bytes along with actual
>data
>> bytes, controllers which requires dummy bytes use len field and simply
>Ignore
>> the dummy field (contains only no of cycles)added in this patch. Controllers
>> (like ZynqMP GQSPI) expects dummy in cycles won't work directly by using
>> len field because host driver doesn't know that len field of a particular
>transfer
>> includes dummy bytes or not (and also number of dummy bytes included in
>len
>> field). In such cases driver use this dummy field to identify the number of
>dummy
>> cycles and based on that it will send the required number of dummy cycles
>(which
>> i did in the second patch).
>
>This doesn't make any sense at all to me.  Why does the controller care
>what the bytes being sent to and from the device mean?

>From the flash point of view, it expects the controller to send the dummy
on 1/2/4 lines based on the command. For Quad commands, flash expects
4 lines to be active during dummy phase. Similarly, 2 lines for dual
Commands and 1 line for normal/fast commands.
Since len field contains total number of cmd+addr+dummy bytes,
host driver should take care of sending these bytes on their respective
bus widths. Knowing when the dummy is being sent also helps in
the correct switching of IO pads (since the data lines are bidirectional)
ZynqMP GQSPI is a generic controller, majorly interfaced to flash devices.
It seems reasonable for it to know the above information from
the flash layer. Adding "dummy" cycles entry should be useful to any
controller that cares about it without affecting other spi/qspi controllers.

Regards
Sai Krishna



This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.



RE: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer structure.

2016-03-31 Thread Lakshmi Sai Krishna Potthuri


>-Original Message-
>From: Mark Brown [mailto:broo...@kernel.org]
>Sent: Friday, March 25, 2016 8:31 PM
>To: Lakshmi Sai Krishna Potthuri 
>Cc: Michal Simek ; Soren Brinkmann
>; David Woodhouse ; Brian
>Norris ; Javier Martinez Canillas
>; Boris Brezillon electrons.com>; Stephen Warren ; Geert
>Uytterhoeven ; Andrew F. Davis ;
>Marek Vasut ; Jagan Teki ; Rafał
>Miłecki ; linux-...@lists.infradead.org; linux-
>ker...@vger.kernel.org; linux-...@vger.kernel.org; linux-arm-
>ker...@lists.infradead.org; Harini Katakam ; Punnaiah
>Choudary Kalluri ; Anirudha Sarangi
>
>Subject: Re: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer
>structure.
>
>On Fri, Mar 25, 2016 at 01:41:16PM +, Lakshmi Sai Krishna Potthuri wrote:
>
>As I'm fairly sure I've said before please fix your mail client to word
>wrap within paragraphs at something substantially less than 80 columns.
>Doing this makes your messages much easier to read and reply to.
>

Sorry, there was some issue with my mail client, now I corrected it.

>> >This is really not what I'd expect to happen, I'd expect that these dummy
>> >cycles would be in addition to the actual data (see my request for better
>> >documentation...).  If they overlap with the data then what is the point in
>> >specifying this?  It's more work for the host, what benefit do we get from
>> >doing it over just handing it like a normal byte?
>
>> len field in the transfer structure contains dummy bytes along with actual
>data
>> bytes, controllers which requires dummy bytes use len field and simply
>Ignore
>> the dummy field (contains only no of cycles)added in this patch. Controllers
>> (like ZynqMP GQSPI) expects dummy in cycles won't work directly by using
>> len field because host driver doesn't know that len field of a particular
>transfer
>> includes dummy bytes or not (and also number of dummy bytes included in
>len
>> field). In such cases driver use this dummy field to identify the number of
>dummy
>> cycles and based on that it will send the required number of dummy cycles
>(which
>> i did in the second patch).
>
>This doesn't make any sense at all to me.  Why does the controller care
>what the bytes being sent to and from the device mean?

>From the flash point of view, it expects the controller to send the dummy
on 1/2/4 lines based on the command. For Quad commands, flash expects
4 lines to be active during dummy phase. Similarly, 2 lines for dual
Commands and 1 line for normal/fast commands.
Since len field contains total number of cmd+addr+dummy bytes,
host driver should take care of sending these bytes on their respective
bus widths. Knowing when the dummy is being sent also helps in
the correct switching of IO pads (since the data lines are bidirectional)
ZynqMP GQSPI is a generic controller, majorly interfaced to flash devices.
It seems reasonable for it to know the above information from
the flash layer. Adding "dummy" cycles entry should be useful to any
controller that cares about it without affecting other spi/qspi controllers.

Regards
Sai Krishna



This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.



RE: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer structure.

2016-03-25 Thread Lakshmi Sai Krishna Potthuri
Hi Mark,

>-Original Message-
>From: Mark Brown [mailto:broo...@kernel.org]
>Sent: Tuesday, March 22, 2016 3:36 PM
>To: Lakshmi Sai Krishna Potthuri
>Cc: Michal Simek; Soren Brinkmann; David Woodhouse; Brian Norris; Javier
>Martinez Canillas; Boris Brezillon; Stephen Warren; Geert Uytterhoeven;
>Andrew F. Davis; Marek Vasut; Jagan Teki; Rafał Miłecki; linux-
>m...@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
>s...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Harini Katakam;
>Punnaiah Choudary Kalluri; Anirudha Sarangi
>Subject: Re: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer
>structure.
>
>On Tue, Mar 22, 2016 at 06:39:51AM +, Lakshmi Sai Krishna Potthuri
>wrote:
>
>Please fix your mail client to word wrap within paragraphs at something
>substantially less than 80 columns.  Doing this makes your messages much
>easier to read and reply to.  Please also avoid reflowing other text into 
>longer
>lengths, this makes things worse.
>
>> >This isn't enough to add the feature - a client driver trying to make
>> >use of this needs to be able to tell if the cycles are actually going
>> >to be inserted.  I'd expect to see a capability flag that can be
>> >checked and some error checking so that if we try to do a transfer
>> >with dummy cycles and can't support it we don't silently ignore the
>> >dummy cycles, ideally also something that'll handle multiples of 8
>> >bits with SPI controllers that don't otherwise support this feature.
>
>> Currently, all fast reads use 8 cycles or 1 byte of dummy. This generally
>works.
>> But it can be vary based on the flash and the type of read command.
>> Dummy bytes are taken care of in m25p80.c by adjusting the len field:
>> Length = size of (command + address + dummy byte)
>
>> There might be controllers (like ZynqMP GQSPI) that would be able to
>> use the information that dummy byte(s) were added and the precise
>> number of dummy cycles. This patch does not disturb the existing
>> implementation of adjusting length (as described above). It adds an
>additional optional feature.
>> So there is no harm to controllers that can't support it - they can
>> ignore it and still work with the existing "length adjustment"
>implementation.
>> If you think there value in adding a capability flag, please let me know.
>
>This is really not what I'd expect to happen, I'd expect that these dummy
>cycles would be in addition to the actual data (see my request for better
>documentation...).  If they overlap with the data then what is the point in
>specifying this?  It's more work for the host, what benefit do we get from
>doing it over just handing it like a normal byte?

len field in the transfer structure contains dummy bytes along with actual data
bytes, controllers which requires dummy bytes use len field and simply Ignore
the dummy field (contains only no of cycles)added in this patch. Controllers
(like ZynqMP GQSPI) expects dummy in cycles won't work directly by using
len field because host driver doesn't know that len field of a particular 
transfer
includes dummy bytes or not (and also number of dummy bytes included in len
field). In such cases driver use this dummy field to identify the number of 
dummy
cycles and based on that it will send the required number of dummy cycles (which
i did in the second patch).

Regards
Sai Krishna


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.



RE: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer structure.

2016-03-25 Thread Lakshmi Sai Krishna Potthuri
Hi Mark,

>-Original Message-
>From: Mark Brown [mailto:broo...@kernel.org]
>Sent: Tuesday, March 22, 2016 3:36 PM
>To: Lakshmi Sai Krishna Potthuri
>Cc: Michal Simek; Soren Brinkmann; David Woodhouse; Brian Norris; Javier
>Martinez Canillas; Boris Brezillon; Stephen Warren; Geert Uytterhoeven;
>Andrew F. Davis; Marek Vasut; Jagan Teki; Rafał Miłecki; linux-
>m...@lists.infradead.org; linux-kernel@vger.kernel.org; linux-
>s...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Harini Katakam;
>Punnaiah Choudary Kalluri; Anirudha Sarangi
>Subject: Re: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer
>structure.
>
>On Tue, Mar 22, 2016 at 06:39:51AM +, Lakshmi Sai Krishna Potthuri
>wrote:
>
>Please fix your mail client to word wrap within paragraphs at something
>substantially less than 80 columns.  Doing this makes your messages much
>easier to read and reply to.  Please also avoid reflowing other text into 
>longer
>lengths, this makes things worse.
>
>> >This isn't enough to add the feature - a client driver trying to make
>> >use of this needs to be able to tell if the cycles are actually going
>> >to be inserted.  I'd expect to see a capability flag that can be
>> >checked and some error checking so that if we try to do a transfer
>> >with dummy cycles and can't support it we don't silently ignore the
>> >dummy cycles, ideally also something that'll handle multiples of 8
>> >bits with SPI controllers that don't otherwise support this feature.
>
>> Currently, all fast reads use 8 cycles or 1 byte of dummy. This generally
>works.
>> But it can be vary based on the flash and the type of read command.
>> Dummy bytes are taken care of in m25p80.c by adjusting the len field:
>> Length = size of (command + address + dummy byte)
>
>> There might be controllers (like ZynqMP GQSPI) that would be able to
>> use the information that dummy byte(s) were added and the precise
>> number of dummy cycles. This patch does not disturb the existing
>> implementation of adjusting length (as described above). It adds an
>additional optional feature.
>> So there is no harm to controllers that can't support it - they can
>> ignore it and still work with the existing "length adjustment"
>implementation.
>> If you think there value in adding a capability flag, please let me know.
>
>This is really not what I'd expect to happen, I'd expect that these dummy
>cycles would be in addition to the actual data (see my request for better
>documentation...).  If they overlap with the data then what is the point in
>specifying this?  It's more work for the host, what benefit do we get from
>doing it over just handing it like a normal byte?

len field in the transfer structure contains dummy bytes along with actual data
bytes, controllers which requires dummy bytes use len field and simply Ignore
the dummy field (contains only no of cycles)added in this patch. Controllers
(like ZynqMP GQSPI) expects dummy in cycles won't work directly by using
len field because host driver doesn't know that len field of a particular 
transfer
includes dummy bytes or not (and also number of dummy bytes included in len
field). In such cases driver use this dummy field to identify the number of 
dummy
cycles and based on that it will send the required number of dummy cycles (which
i did in the second patch).

Regards
Sai Krishna


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.



RE: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer structure.

2016-03-22 Thread Lakshmi Sai Krishna Potthuri
Hi Mark,


>On Mon, Mar 21, 2016 at 05:50:08PM +0530, P L Sai Krishna wrote:
>> This patch does following things.
>> 1. Added dummy entry in the spi_transfer structure.
>> 2. Assigned dummy cycles to dummy member in the transfer structure
>> during read operation.
>
>Please try to follow the patch submission process covered in
>SubmittingPatches, in particular please use subject lines reflecting the style
>for the subsystem (which helps people identify relevant changes to
>review) and...
>
>>  drivers/mtd/devices/m25p80.c | 1 +
>>  include/linux/spi/spi.h  | 2 ++
>>  2 files changed, 3 insertions(+)
>
>...split things up into individual patches, for example here you're both 
>adding a
>new feature and adding a user of that feature in a single patch.

I will split the patch into two with appropriate commit messages.

>
>> + * @dummy: number of dummy cycles.
>
>This needs to be clearer about what a dummy cycle is and where it gets
>inserted.  We probably also want a better name, just "dummy" makes it look
>like a padding field in the structure.  How about dummy_cycles?

dummy_cycles is a better idea.
I will change it and add the usage of this in the description in a detailed 
manner.

>
>> @@ -752,6 +753,7 @@ struct spi_transfer {
>>  u8  bits_per_word;
>>  u16 delay_usecs;
>>  u32 speed_hz;
>> +u32 dummy;
>>
>>  struct list_head transfer_list;
>>  };
>
>This isn't enough to add the feature - a client driver trying to make use of 
>this
>needs to be able to tell if the cycles are actually going to be inserted.  I'd
>expect to see a capability flag that can be checked and some error checking so
>that if we try to do a transfer with dummy cycles and can't support it we don't
>silently ignore the dummy cycles, ideally also something that'll handle
>multiples of 8 bits with SPI controllers that don't otherwise support this
>feature.

Currently, all fast reads use 8 cycles or 1 byte of dummy. This generally works.
But it can be vary based on the flash and the type of read command.
Dummy bytes are taken care of in m25p80.c by adjusting the len field:
Length = size of (command + address + dummy byte)

There might be controllers (like ZynqMP GQSPI) that would be able to use
the information that dummy byte(s) were added and the precise number
of dummy cycles. This patch does not disturb the existing implementation
of adjusting length (as described above). It adds an additional optional 
feature.
So there is no harm to controllers that can't support it - they can ignore it 
and
still work with the existing "length adjustment" implementation.
If you think there value in adding a capability flag, please let me know.

Thanks for your review.

Regards,
Sai Krishna


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.



RE: [LINUX PATCH 1/2] mtd: Added dummy entry in the spi_transfer structure.

2016-03-22 Thread Lakshmi Sai Krishna Potthuri
Hi Mark,


>On Mon, Mar 21, 2016 at 05:50:08PM +0530, P L Sai Krishna wrote:
>> This patch does following things.
>> 1. Added dummy entry in the spi_transfer structure.
>> 2. Assigned dummy cycles to dummy member in the transfer structure
>> during read operation.
>
>Please try to follow the patch submission process covered in
>SubmittingPatches, in particular please use subject lines reflecting the style
>for the subsystem (which helps people identify relevant changes to
>review) and...
>
>>  drivers/mtd/devices/m25p80.c | 1 +
>>  include/linux/spi/spi.h  | 2 ++
>>  2 files changed, 3 insertions(+)
>
>...split things up into individual patches, for example here you're both 
>adding a
>new feature and adding a user of that feature in a single patch.

I will split the patch into two with appropriate commit messages.

>
>> + * @dummy: number of dummy cycles.
>
>This needs to be clearer about what a dummy cycle is and where it gets
>inserted.  We probably also want a better name, just "dummy" makes it look
>like a padding field in the structure.  How about dummy_cycles?

dummy_cycles is a better idea.
I will change it and add the usage of this in the description in a detailed 
manner.

>
>> @@ -752,6 +753,7 @@ struct spi_transfer {
>>  u8  bits_per_word;
>>  u16 delay_usecs;
>>  u32 speed_hz;
>> +u32 dummy;
>>
>>  struct list_head transfer_list;
>>  };
>
>This isn't enough to add the feature - a client driver trying to make use of 
>this
>needs to be able to tell if the cycles are actually going to be inserted.  I'd
>expect to see a capability flag that can be checked and some error checking so
>that if we try to do a transfer with dummy cycles and can't support it we don't
>silently ignore the dummy cycles, ideally also something that'll handle
>multiples of 8 bits with SPI controllers that don't otherwise support this
>feature.

Currently, all fast reads use 8 cycles or 1 byte of dummy. This generally works.
But it can be vary based on the flash and the type of read command.
Dummy bytes are taken care of in m25p80.c by adjusting the len field:
Length = size of (command + address + dummy byte)

There might be controllers (like ZynqMP GQSPI) that would be able to use
the information that dummy byte(s) were added and the precise number
of dummy cycles. This patch does not disturb the existing implementation
of adjusting length (as described above). It adds an additional optional 
feature.
So there is no harm to controllers that can't support it - they can ignore it 
and
still work with the existing "length adjustment" implementation.
If you think there value in adding a capability flag, please let me know.

Thanks for your review.

Regards,
Sai Krishna


This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.