[PATCH v3 0/6] Driver for at91 usart in spi mode

[PATCH v3 0/6] Driver for at91 usart in spi mode

[Radu Pirea]

Hello,

This is the second version of driver. I added a mfd driver which by
default probes atmel_serial driver and if in dt is specified to probe
the spi driver, then the spi-at91-usart driver will be probed. The
compatible for atmel_serial is now the compatible for at91-usart mfd
driver and compatilbe for atmel_serial driver was changed in order to
keep the bindings for serial as they are.

Changes in v1:
- added spi-at91-usart driver

Changes in v2:
- added at91-usart mfd driver
- modified spi-at91-usart driver to work as mfd driver child
- modified atmel_serial driver to work as mfd driver child

Changes in v3:
- fixed spi slaves probing

Radu Pirea (6):
  MAINTAINERS: add at91 usart mfd driver
  mfd: at91-usart: added mfd driver for usart
  MAINTAINERS: add at91 usart spi driver
  dt-bindings: add binding for at91-usart in spi mode
  spi: at91-usart: add driver for at91-usart as spi
  tty/serial: atmel: changed the driver to work under at91-usart mfd

 .../bindings/spi/microchip,at91-usart-spi.txt |  28 +
 MAINTAINERS                                   |  14 +
 drivers/mfd/Kconfig                           |  10 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/at91-usart.c                      |  75 +++
 drivers/spi/Kconfig                           |   9 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-at91-usart.c                  | 544 ++++++++++++++++++
 drivers/tty/serial/Kconfig                    |   1 +
 drivers/tty/serial/atmel_serial.c             |  29 +-
 include/dt-bindings/mfd/at91-usart.h          |  17 +
 11 files changed, 715 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
 create mode 100644 drivers/mfd/at91-usart.c
 create mode 100644 drivers/spi/spi-at91-usart.c
 create mode 100644 include/dt-bindings/mfd/at91-usart.h

-- 
2.17.0

[PATCH v3 0/6] Driver for at91 usart in spi mode

[Radu Pirea]

Hello,

This is the second version of driver. I added a mfd driver which by
default probes atmel_serial driver and if in dt is specified to probe
the spi driver, then the spi-at91-usart driver will be probed. The
compatible for atmel_serial is now the compatible for at91-usart mfd
driver and compatilbe for atmel_serial driver was changed in order to
keep the bindings for serial as they are.

Changes in v1:
- added spi-at91-usart driver

Changes in v2:
- added at91-usart mfd driver
- modified spi-at91-usart driver to work as mfd driver child
- modified atmel_serial driver to work as mfd driver child

Changes in v3:
- fixed spi slaves probing

Radu Pirea (6):
  MAINTAINERS: add at91 usart mfd driver
  mfd: at91-usart: added mfd driver for usart
  MAINTAINERS: add at91 usart spi driver
  dt-bindings: add binding for at91-usart in spi mode
  spi: at91-usart: add driver for at91-usart as spi
  tty/serial: atmel: changed the driver to work under at91-usart mfd

 .../bindings/spi/microchip,at91-usart-spi.txt |  28 +
 MAINTAINERS                                   |  14 +
 drivers/mfd/Kconfig                           |  10 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/at91-usart.c                      |  75 +++
 drivers/spi/Kconfig                           |   9 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-at91-usart.c                  | 544 ++++++++++++++++++
 drivers/tty/serial/Kconfig                    |   1 +
 drivers/tty/serial/atmel_serial.c             |  29 +-
 include/dt-bindings/mfd/at91-usart.h          |  17 +
 11 files changed, 715 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
 create mode 100644 drivers/mfd/at91-usart.c
 create mode 100644 drivers/spi/spi-at91-usart.c
 create mode 100644 include/dt-bindings/mfd/at91-usart.h

-- 
2.17.0

[PATCH v3 0/6] Driver for at91 usart in spi mode

[Radu Pirea]

Hello,

This is the second version of driver. I added a mfd driver which by
default probes atmel_serial driver and if in dt is specified to probe
the spi driver, then the spi-at91-usart driver will be probed. The
compatible for atmel_serial is now the compatible for at91-usart mfd
driver and compatilbe for atmel_serial driver was changed in order to
keep the bindings for serial as they are.

Changes in v1:
- added spi-at91-usart driver

Changes in v2:
- added at91-usart mfd driver
- modified spi-at91-usart driver to work as mfd driver child
- modified atmel_serial driver to work as mfd driver child

Changes in v3:
- fixed spi slaves probing

Radu Pirea (6):
  MAINTAINERS: add at91 usart mfd driver
  mfd: at91-usart: added mfd driver for usart
  MAINTAINERS: add at91 usart spi driver
  dt-bindings: add binding for at91-usart in spi mode
  spi: at91-usart: add driver for at91-usart as spi
  tty/serial: atmel: changed the driver to work under at91-usart mfd

 .../bindings/spi/microchip,at91-usart-spi.txt |  28 +
 MAINTAINERS                                   |  14 +
 drivers/mfd/Kconfig                           |  10 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/at91-usart.c                      |  75 +++
 drivers/spi/Kconfig                           |   9 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-at91-usart.c                  | 544 ++++++++++++++++++
 drivers/tty/serial/Kconfig                    |   1 +
 drivers/tty/serial/atmel_serial.c             |  29 +-
 include/dt-bindings/mfd/at91-usart.h          |  17 +
 11 files changed, 715 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
 create mode 100644 drivers/mfd/at91-usart.c
 create mode 100644 drivers/spi/spi-at91-usart.c
 create mode 100644 include/dt-bindings/mfd/at91-usart.h

-- 
2.17.0

[PATCH v3 1/6] MAINTAINERS: add at91 usart mfd driver

[Radu Pirea]

Added entry for at91 usart mfd driver.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8e2a2fddbd19..ca06c6f58299 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9192,6 +9192,13 @@ S:	Supported
 F:	drivers/mtd/nand/raw/atmel/*
 F:	Documentation/devicetree/bindings/mtd/atmel-nand.txt
 
+MICROCHIP AT91 USART MFD DRIVER
+M:	Radu Pirea <radu.pirea@microchip.com>
+L:	linux-kernel@vger.kernel.org
+S:	Supported
+F:	drivers/mfd/at91-usart.c
+F:	include/dt-bindings/mfd/at91-usart.h
+
 MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
 M:	Woojung Huh <Woojung.Huh@microchip.com>
 M:	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
-- 
2.17.0

[PATCH v3 1/6] MAINTAINERS: add at91 usart mfd driver

[Radu Pirea]

Added entry for at91 usart mfd driver.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8e2a2fddbd19..ca06c6f58299 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9192,6 +9192,13 @@ S:	Supported
 F:	drivers/mtd/nand/raw/atmel/*
 F:	Documentation/devicetree/bindings/mtd/atmel-nand.txt
 
+MICROCHIP AT91 USART MFD DRIVER
+M:	Radu Pirea <radu.pirea@microchip.com>
+L:	linux-kernel@vger.kernel.org
+S:	Supported
+F:	drivers/mfd/at91-usart.c
+F:	include/dt-bindings/mfd/at91-usart.h
+
 MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
 M:	Woojung Huh <Woojung.Huh@microchip.com>
 M:	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
-- 
2.17.0

[PATCH v3 1/6] MAINTAINERS: add at91 usart mfd driver

[Radu Pirea]

Added entry for at91 usart mfd driver.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8e2a2fddbd19..ca06c6f58299 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9192,6 +9192,13 @@ S:	Supported
 F:	drivers/mtd/nand/raw/atmel/*
 F:	Documentation/devicetree/bindings/mtd/atmel-nand.txt
 
+MICROCHIP AT91 USART MFD DRIVER
+M:	Radu Pirea <radu.pirea@microchip.com>
+L:	linux-kernel at vger.kernel.org
+S:	Supported
+F:	drivers/mfd/at91-usart.c
+F:	include/dt-bindings/mfd/at91-usart.h
+
 MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
 M:	Woojung Huh <Woojung.Huh@microchip.com>
 M:	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
-- 
2.17.0

[PATCH v3 2/6] mfd: at91-usart: added mfd driver for usart

[Radu Pirea]

This mfd driver is just a wrapper over atmel_serial driver and
spi-at91-usart driver. Selection of one of the drivers is based on a
property from device tree. If the property is not specified, the default
driver is atmel_serial.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 drivers/mfd/Kconfig                  | 10 ++++
 drivers/mfd/Makefile                 |  1 +
 drivers/mfd/at91-usart.c             | 75 ++++++++++++++++++++++++++++
 include/dt-bindings/mfd/at91-usart.h | 17 +++++++
 4 files changed, 103 insertions(+)
 create mode 100644 drivers/mfd/at91-usart.c
 create mode 100644 include/dt-bindings/mfd/at91-usart.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b860eb5aa194..de99b79061b7 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -99,6 +99,16 @@ config MFD_AAT2870_CORE
 	  additional drivers must be enabled in order to use the
 	  functionality of the device.
 
+config MFD_AT91_USART
+	tristate "AT91 USART Driver"
+	select MFD_CORE
+	depends on OF
+	help
+	  Select this to get support for AT91 USART IP. This is a wrapper
+	  over at91-usart-serial driver and usart-spi-driver. Only one function
+	  can be used at a time. The choice is done at boot time by the probe
+	  function of this MFD driver according to a device tree property.
+
 config MFD_ATMEL_FLEXCOM
 	tristate "Atmel Flexcom (Flexible Serial Communication Unit)"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index d9d2cf0d32ef..db1332aa96db 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -185,6 +185,7 @@ obj-$(CONFIG_MFD_SPMI_PMIC)	+= qcom-spmi-pmic.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
 obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
 obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
+obj-$(CONFIG_MFD_AT91_USART)	+= at91-usart.o
 obj-$(CONFIG_MFD_ATMEL_FLEXCOM)	+= atmel-flexcom.o
 obj-$(CONFIG_MFD_ATMEL_HLCDC)	+= atmel-hlcdc.o
 obj-$(CONFIG_MFD_ATMEL_SMC)	+= atmel-smc.o
diff --git a/drivers/mfd/at91-usart.c b/drivers/mfd/at91-usart.c
new file mode 100644
index 000000000000..87094463f8f4
--- /dev/null
+++ b/drivers/mfd/at91-usart.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for AT91 USART
+ *
+ * Copyright (C) 2018 Microchip Technology
+ *
+ * Author: Radu Pirea <radu.pirea@microchip.com>
+ *
+ */
+
+#include <dt-bindings/mfd/at91-usart.h>
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mfd/core.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+static struct mfd_cell at91_usart_spi_subdev = {
+		.name = "at91_usart_spi",
+		.of_compatible = "microchip,at91sam9g45-usart-spi",
+	};
+
+static struct mfd_cell at91_usart_serial_subdev = {
+		.name = "atmel_usart_serial",
+		.of_compatible = "atmel,at91rm9200-usart-serial",
+	};
+
+static int at91_usart_mode_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct mfd_cell cell;
+	u32 opmode;
+	int err;
+
+	err = of_property_read_u32(np, "at91,usart-mode", &opmode);
+
+	switch (opmode) {
+	case AT91_USART_MODE_SPI:
+		cell = at91_usart_spi_subdev;
+		break;
+	case AT91_USART_MODE_SERIAL:
+	default:
+		cell = at91_usart_serial_subdev;
+	}
+
+	return mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO, &cell, 1,
+			      NULL, 0, NULL);
+}
+
+static const struct of_device_id at91_usart_mode_of_match[] = {
+	{ .compatible = "atmel,at91rm9200-usart" },
+	{ .compatible = "atmel,at91sam9260-usart" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, at91_flexcom_of_match);
+
+static struct platform_driver at91_usart_mfd = {
+	.probe	= at91_usart_mode_probe,
+	.driver	= {
+		.name		= "at91_usart_mode",
+		.of_match_table	= at91_usart_mode_of_match,
+	},
+};
+
+module_platform_driver(at91_usart_mfd);
+
+MODULE_AUTHOR("Radu Pirea <radu.pirea@microchip.com>");
+MODULE_DESCRIPTION("AT91 USART MFD driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/dt-bindings/mfd/at91-usart.h b/include/dt-bindings/mfd/at91-usart.h
new file mode 100644
index 000000000000..ac811628a42d
--- /dev/null
+++ b/include/dt-bindings/mfd/at91-usart.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides macros for AT91 USART DT bindings.
+ *
+ * Copyright (C) 2018 Microchip Technology
+ *
+ * Author: Radu Pirea <radu.pirea@microchip.com>
+ *
+ */
+
+#ifndef __DT_BINDINGS_AT91_USART_H__
+#define __DT_BINDINGS_AT91_USART_H__
+
+#define AT91_USART_MODE_SERIAL	1
+#define AT91_USART_MODE_SPI	2
+
+#endif /* __DT_BINDINGS_AT91_USART_H__ */
-- 
2.17.0

[PATCH v3 2/6] mfd: at91-usart: added mfd driver for usart

[Radu Pirea]

This mfd driver is just a wrapper over atmel_serial driver and
spi-at91-usart driver. Selection of one of the drivers is based on a
property from device tree. If the property is not specified, the default
driver is atmel_serial.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 drivers/mfd/Kconfig                  | 10 ++++
 drivers/mfd/Makefile                 |  1 +
 drivers/mfd/at91-usart.c             | 75 ++++++++++++++++++++++++++++
 include/dt-bindings/mfd/at91-usart.h | 17 +++++++
 4 files changed, 103 insertions(+)
 create mode 100644 drivers/mfd/at91-usart.c
 create mode 100644 include/dt-bindings/mfd/at91-usart.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b860eb5aa194..de99b79061b7 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -99,6 +99,16 @@ config MFD_AAT2870_CORE
 	  additional drivers must be enabled in order to use the
 	  functionality of the device.
 
+config MFD_AT91_USART
+	tristate "AT91 USART Driver"
+	select MFD_CORE
+	depends on OF
+	help
+	  Select this to get support for AT91 USART IP. This is a wrapper
+	  over at91-usart-serial driver and usart-spi-driver. Only one function
+	  can be used at a time. The choice is done at boot time by the probe
+	  function of this MFD driver according to a device tree property.
+
 config MFD_ATMEL_FLEXCOM
 	tristate "Atmel Flexcom (Flexible Serial Communication Unit)"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index d9d2cf0d32ef..db1332aa96db 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -185,6 +185,7 @@ obj-$(CONFIG_MFD_SPMI_PMIC)	+= qcom-spmi-pmic.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
 obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
 obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
+obj-$(CONFIG_MFD_AT91_USART)	+= at91-usart.o
 obj-$(CONFIG_MFD_ATMEL_FLEXCOM)	+= atmel-flexcom.o
 obj-$(CONFIG_MFD_ATMEL_HLCDC)	+= atmel-hlcdc.o
 obj-$(CONFIG_MFD_ATMEL_SMC)	+= atmel-smc.o
diff --git a/drivers/mfd/at91-usart.c b/drivers/mfd/at91-usart.c
new file mode 100644
index 000000000000..87094463f8f4
--- /dev/null
+++ b/drivers/mfd/at91-usart.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for AT91 USART
+ *
+ * Copyright (C) 2018 Microchip Technology
+ *
+ * Author: Radu Pirea <radu.pirea@microchip.com>
+ *
+ */
+
+#include <dt-bindings/mfd/at91-usart.h>
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mfd/core.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+static struct mfd_cell at91_usart_spi_subdev = {
+		.name = "at91_usart_spi",
+		.of_compatible = "microchip,at91sam9g45-usart-spi",
+	};
+
+static struct mfd_cell at91_usart_serial_subdev = {
+		.name = "atmel_usart_serial",
+		.of_compatible = "atmel,at91rm9200-usart-serial",
+	};
+
+static int at91_usart_mode_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct mfd_cell cell;
+	u32 opmode;
+	int err;
+
+	err = of_property_read_u32(np, "at91,usart-mode", &opmode);
+
+	switch (opmode) {
+	case AT91_USART_MODE_SPI:
+		cell = at91_usart_spi_subdev;
+		break;
+	case AT91_USART_MODE_SERIAL:
+	default:
+		cell = at91_usart_serial_subdev;
+	}
+
+	return mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO, &cell, 1,
+			      NULL, 0, NULL);
+}
+
+static const struct of_device_id at91_usart_mode_of_match[] = {
+	{ .compatible = "atmel,at91rm9200-usart" },
+	{ .compatible = "atmel,at91sam9260-usart" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, at91_flexcom_of_match);
+
+static struct platform_driver at91_usart_mfd = {
+	.probe	= at91_usart_mode_probe,
+	.driver	= {
+		.name		= "at91_usart_mode",
+		.of_match_table	= at91_usart_mode_of_match,
+	},
+};
+
+module_platform_driver(at91_usart_mfd);
+
+MODULE_AUTHOR("Radu Pirea <radu.pirea@microchip.com>");
+MODULE_DESCRIPTION("AT91 USART MFD driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/dt-bindings/mfd/at91-usart.h b/include/dt-bindings/mfd/at91-usart.h
new file mode 100644
index 000000000000..ac811628a42d
--- /dev/null
+++ b/include/dt-bindings/mfd/at91-usart.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides macros for AT91 USART DT bindings.
+ *
+ * Copyright (C) 2018 Microchip Technology
+ *
+ * Author: Radu Pirea <radu.pirea@microchip.com>
+ *
+ */
+
+#ifndef __DT_BINDINGS_AT91_USART_H__
+#define __DT_BINDINGS_AT91_USART_H__
+
+#define AT91_USART_MODE_SERIAL	1
+#define AT91_USART_MODE_SPI	2
+
+#endif /* __DT_BINDINGS_AT91_USART_H__ */
-- 
2.17.0

[PATCH v3 2/6] mfd: at91-usart: added mfd driver for usart

[Radu Pirea]

This mfd driver is just a wrapper over atmel_serial driver and
spi-at91-usart driver. Selection of one of the drivers is based on a
property from device tree. If the property is not specified, the default
driver is atmel_serial.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 drivers/mfd/Kconfig                  | 10 ++++
 drivers/mfd/Makefile                 |  1 +
 drivers/mfd/at91-usart.c             | 75 ++++++++++++++++++++++++++++
 include/dt-bindings/mfd/at91-usart.h | 17 +++++++
 4 files changed, 103 insertions(+)
 create mode 100644 drivers/mfd/at91-usart.c
 create mode 100644 include/dt-bindings/mfd/at91-usart.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b860eb5aa194..de99b79061b7 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -99,6 +99,16 @@ config MFD_AAT2870_CORE
 	  additional drivers must be enabled in order to use the
 	  functionality of the device.
 
+config MFD_AT91_USART
+	tristate "AT91 USART Driver"
+	select MFD_CORE
+	depends on OF
+	help
+	  Select this to get support for AT91 USART IP. This is a wrapper
+	  over at91-usart-serial driver and usart-spi-driver. Only one function
+	  can be used at a time. The choice is done at boot time by the probe
+	  function of this MFD driver according to a device tree property.
+
 config MFD_ATMEL_FLEXCOM
 	tristate "Atmel Flexcom (Flexible Serial Communication Unit)"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index d9d2cf0d32ef..db1332aa96db 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -185,6 +185,7 @@ obj-$(CONFIG_MFD_SPMI_PMIC)	+= qcom-spmi-pmic.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
 obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
 obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
+obj-$(CONFIG_MFD_AT91_USART)	+= at91-usart.o
 obj-$(CONFIG_MFD_ATMEL_FLEXCOM)	+= atmel-flexcom.o
 obj-$(CONFIG_MFD_ATMEL_HLCDC)	+= atmel-hlcdc.o
 obj-$(CONFIG_MFD_ATMEL_SMC)	+= atmel-smc.o
diff --git a/drivers/mfd/at91-usart.c b/drivers/mfd/at91-usart.c
new file mode 100644
index 000000000000..87094463f8f4
--- /dev/null
+++ b/drivers/mfd/at91-usart.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for AT91 USART
+ *
+ * Copyright (C) 2018 Microchip Technology
+ *
+ * Author: Radu Pirea <radu.pirea@microchip.com>
+ *
+ */
+
+#include <dt-bindings/mfd/at91-usart.h>
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mfd/core.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+static struct mfd_cell at91_usart_spi_subdev = {
+		.name = "at91_usart_spi",
+		.of_compatible = "microchip,at91sam9g45-usart-spi",
+	};
+
+static struct mfd_cell at91_usart_serial_subdev = {
+		.name = "atmel_usart_serial",
+		.of_compatible = "atmel,at91rm9200-usart-serial",
+	};
+
+static int at91_usart_mode_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct mfd_cell cell;
+	u32 opmode;
+	int err;
+
+	err = of_property_read_u32(np, "at91,usart-mode", &opmode);
+
+	switch (opmode) {
+	case AT91_USART_MODE_SPI:
+		cell = at91_usart_spi_subdev;
+		break;
+	case AT91_USART_MODE_SERIAL:
+	default:
+		cell = at91_usart_serial_subdev;
+	}
+
+	return mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO, &cell, 1,
+			      NULL, 0, NULL);
+}
+
+static const struct of_device_id at91_usart_mode_of_match[] = {
+	{ .compatible = "atmel,at91rm9200-usart" },
+	{ .compatible = "atmel,at91sam9260-usart" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, at91_flexcom_of_match);
+
+static struct platform_driver at91_usart_mfd = {
+	.probe	= at91_usart_mode_probe,
+	.driver	= {
+		.name		= "at91_usart_mode",
+		.of_match_table	= at91_usart_mode_of_match,
+	},
+};
+
+module_platform_driver(at91_usart_mfd);
+
+MODULE_AUTHOR("Radu Pirea <radu.pirea@microchip.com>");
+MODULE_DESCRIPTION("AT91 USART MFD driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/dt-bindings/mfd/at91-usart.h b/include/dt-bindings/mfd/at91-usart.h
new file mode 100644
index 000000000000..ac811628a42d
--- /dev/null
+++ b/include/dt-bindings/mfd/at91-usart.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides macros for AT91 USART DT bindings.
+ *
+ * Copyright (C) 2018 Microchip Technology
+ *
+ * Author: Radu Pirea <radu.pirea@microchip.com>
+ *
+ */
+
+#ifndef __DT_BINDINGS_AT91_USART_H__
+#define __DT_BINDINGS_AT91_USART_H__
+
+#define AT91_USART_MODE_SERIAL	1
+#define AT91_USART_MODE_SPI	2
+
+#endif /* __DT_BINDINGS_AT91_USART_H__ */
-- 
2.17.0

[PATCH v3 3/6] MAINTAINERS: add at91 usart spi driver

[Radu Pirea]

Added entry for at91 usart mfd driver.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ca06c6f58299..9243b9007966 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9199,6 +9199,13 @@ S:	Supported
 F:	drivers/mfd/at91-usart.c
 F:	include/dt-bindings/mfd/at91-usart.h
 
+MICROCHIP AT91 USART SPI DRIVER
+M:	Radu Pirea <radu.pirea@microchip.com>
+L:	linux-spi@vger.kernel.org
+S:	Supported
+F:	drivers/spi/spi-at91-usart.c
+F:	Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
+
 MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
 M:	Woojung Huh <Woojung.Huh@microchip.com>
 M:	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
-- 
2.17.0

[PATCH v3 3/6] MAINTAINERS: add at91 usart spi driver

[Radu Pirea]

Added entry for at91 usart mfd driver.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ca06c6f58299..9243b9007966 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9199,6 +9199,13 @@ S:	Supported
 F:	drivers/mfd/at91-usart.c
 F:	include/dt-bindings/mfd/at91-usart.h
 
+MICROCHIP AT91 USART SPI DRIVER
+M:	Radu Pirea <radu.pirea@microchip.com>
+L:	linux-spi@vger.kernel.org
+S:	Supported
+F:	drivers/spi/spi-at91-usart.c
+F:	Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
+
 MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
 M:	Woojung Huh <Woojung.Huh@microchip.com>
 M:	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
-- 
2.17.0

[PATCH v3 3/6] MAINTAINERS: add at91 usart spi driver

[Radu Pirea]

Added entry for at91 usart mfd driver.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ca06c6f58299..9243b9007966 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9199,6 +9199,13 @@ S:	Supported
 F:	drivers/mfd/at91-usart.c
 F:	include/dt-bindings/mfd/at91-usart.h
 
+MICROCHIP AT91 USART SPI DRIVER
+M:	Radu Pirea <radu.pirea@microchip.com>
+L:	linux-spi at vger.kernel.org
+S:	Supported
+F:	drivers/spi/spi-at91-usart.c
+F:	Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
+
 MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
 M:	Woojung Huh <Woojung.Huh@microchip.com>
 M:	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
-- 
2.17.0

[PATCH v3 4/6] dt-bindings: add binding for at91-usart in spi mode

[Radu Pirea]

These are bindings for at91-usart IP in spi spi mode. There is no support for
internal chip select. Only kind of chip selects available are gpio chip
selects.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 .../bindings/spi/microchip,at91-usart-spi.txt | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt

diff --git a/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
new file mode 100644
index 000000000000..b68a3bec4121
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
@@ -0,0 +1,28 @@
+* Universal Synchronous Asynchronous Receiver/Transmitter (USART) in SPI mode
+
+Required properties:
+- #size-cells      : Must be <0>
+- #address-cells   : Must be <1>
+- compatible: Should be "atmel,at91rm9200-usart" or "atmel,at91sam9260-usart"
+- reg: Should contain registers location and length
+- interrupts: Should contain interrupt
+- clocks: phandles to input clocks.
+- clock-names: tuple listing input clock names.
+	Required elements: "usart"
+- cs-gpios: chipselects (internal cs not supported)
+- at91,usart-mode: AT91_USART_MODE_SPI (found in dt-bindings/mfd/at91-usart.h)
+
+Example:
+	#include <dt-bindings/mfd/at91-usart.h>
+
+	spi0: spi@f001c000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "atmel,at91rm9200-usart", "atmel,at91sam9260-usart";
+		at91,usart-mode = <AT91_USART_MODE_SPI>;
+		reg = <0xf001c000 0x100>;
+		interrupts = <12 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&usart0_clk>;
+		clock-names = "usart";
+		cs-gpios = <&pioB 3 0>;
+	};
-- 
2.17.0

[PATCH v3 4/6] dt-bindings: add binding for at91-usart in spi mode

[Radu Pirea]

These are bindings for at91-usart IP in spi spi mode. There is no support for
internal chip select. Only kind of chip selects available are gpio chip
selects.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 .../bindings/spi/microchip,at91-usart-spi.txt | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt

diff --git a/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
new file mode 100644
index 000000000000..b68a3bec4121
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
@@ -0,0 +1,28 @@
+* Universal Synchronous Asynchronous Receiver/Transmitter (USART) in SPI mode
+
+Required properties:
+- #size-cells      : Must be <0>
+- #address-cells   : Must be <1>
+- compatible: Should be "atmel,at91rm9200-usart" or "atmel,at91sam9260-usart"
+- reg: Should contain registers location and length
+- interrupts: Should contain interrupt
+- clocks: phandles to input clocks.
+- clock-names: tuple listing input clock names.
+	Required elements: "usart"
+- cs-gpios: chipselects (internal cs not supported)
+- at91,usart-mode: AT91_USART_MODE_SPI (found in dt-bindings/mfd/at91-usart.h)
+
+Example:
+	#include <dt-bindings/mfd/at91-usart.h>
+
+	spi0: spi@f001c000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "atmel,at91rm9200-usart", "atmel,at91sam9260-usart";
+		at91,usart-mode = <AT91_USART_MODE_SPI>;
+		reg = <0xf001c000 0x100>;
+		interrupts = <12 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&usart0_clk>;
+		clock-names = "usart";
+		cs-gpios = <&pioB 3 0>;
+	};
-- 
2.17.0

[PATCH v3 4/6] dt-bindings: add binding for at91-usart in spi mode

[Radu Pirea]

These are bindings for at91-usart IP in spi spi mode. There is no support for
internal chip select. Only kind of chip selects available are gpio chip
selects.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 .../bindings/spi/microchip,at91-usart-spi.txt | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt

diff --git a/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
new file mode 100644
index 000000000000..b68a3bec4121
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
@@ -0,0 +1,28 @@
+* Universal Synchronous Asynchronous Receiver/Transmitter (USART) in SPI mode
+
+Required properties:
+- #size-cells      : Must be <0>
+- #address-cells   : Must be <1>
+- compatible: Should be "atmel,at91rm9200-usart" or "atmel,at91sam9260-usart"
+- reg: Should contain registers location and length
+- interrupts: Should contain interrupt
+- clocks: phandles to input clocks.
+- clock-names: tuple listing input clock names.
+	Required elements: "usart"
+- cs-gpios: chipselects (internal cs not supported)
+- at91,usart-mode: AT91_USART_MODE_SPI (found in dt-bindings/mfd/at91-usart.h)
+
+Example:
+	#include <dt-bindings/mfd/at91-usart.h>
+
+	spi0: spi at f001c000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "atmel,at91rm9200-usart", "atmel,at91sam9260-usart";
+		at91,usart-mode = <AT91_USART_MODE_SPI>;
+		reg = <0xf001c000 0x100>;
+		interrupts = <12 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&usart0_clk>;
+		clock-names = "usart";
+		cs-gpios = <&pioB 3 0>;
+	};
-- 
2.17.0

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

This is the driver for at91-usart in spi mode. The USART IP can be configured
to work in many modes and one of them is SPI.

The driver was tested on sama5d3-xplained and sama5d4-xplained boards with
enc28j60 ethernet controller as slave.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 drivers/spi/Kconfig          |   9 +
 drivers/spi/Makefile         |   1 +
 drivers/spi/spi-at91-usart.c | 544 +++++++++++++++++++++++++++++++++++
 3 files changed, 554 insertions(+)
 create mode 100644 drivers/spi/spi-at91-usart.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 6fb0347a24f2..c675e6b8dd5a 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -77,6 +77,15 @@ config SPI_ATMEL
 	  This selects a driver for the Atmel SPI Controller, present on
 	  many AT91 (ARM) chips.
 
+config SPI_AT91_USART
+        tristate "Atmel USART Controller as SPI"
+	depends on HAS_DMA
+	depends on (ARCH_AT91 || COMPILE_TEST)
+        select MFD_AT91_USART
+	help
+	  This selects a driver for the AT91 USART Controller as SPI Master,
+	  present on AT91 and SAMA5 SoC series.
+
 config SPI_AU1550
 	tristate "Au1550/Au1200/Au1300 SPI Controller"
 	depends on MIPS_ALCHEMY
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 34c5f2832ddf..fb6cb42f4eaa 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_SPI_LOOPBACK_TEST)		+= spi-loopback-test.o
 obj-$(CONFIG_SPI_ALTERA)		+= spi-altera.o
 obj-$(CONFIG_SPI_ARMADA_3700)		+= spi-armada-3700.o
 obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
+obj-$(CONFIG_SPI_AT91_USART)		+= spi-at91-usart.o
 obj-$(CONFIG_SPI_ATH79)			+= spi-ath79.o
 obj-$(CONFIG_SPI_AU1550)		+= spi-au1550.o
 obj-$(CONFIG_SPI_AXI_SPI_ENGINE)	+= spi-axi-spi-engine.o
diff --git a/drivers/spi/spi-at91-usart.c b/drivers/spi/spi-at91-usart.c
new file mode 100644
index 000000000000..79a59759d2ee
--- /dev/null
+++ b/drivers/spi/spi-at91-usart.c
@@ -0,0 +1,544 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for AT91 USART Controllers as SPI
+ *
+ * Copyright (C) 2018 Microchip Technology Inc.
+ * Author: Radu Pirea <radu.pirea@microchip.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+
+#include <linux/pinctrl/consumer.h>
+
+#include <linux/spi/spi.h>
+
+#define US_CR			0x00
+#define US_MR			0x04
+#define US_IER			0x08
+#define US_IDR			0x0C
+#define US_CSR			0x14
+#define US_RHR			0x18
+#define US_THR			0x1C
+#define US_BRGR			0x20
+#define US_VERSION		0xFC
+
+#define US_CR_RSTRX		BIT(2)
+#define US_CR_RSTTX		BIT(3)
+#define US_CR_RXEN		BIT(4)
+#define US_CR_RXDIS		BIT(5)
+#define US_CR_TXEN		BIT(6)
+#define US_CR_TXDIS		BIT(7)
+
+#define US_MR_SPI_MASTER	0x0E
+#define US_MR_CHRL		GENMASK(7, 6)
+#define US_MR_CPHA		BIT(8)
+#define US_MR_CPOL		BIT(16)
+#define US_MR_CLKO		BIT(18)
+#define US_MR_WRDBT		BIT(20)
+#define US_MR_LOOP		BIT(15)
+
+#define US_IR_RXRDY		BIT(0)
+#define US_IR_TXRDY		BIT(1)
+#define US_IR_OVRE		BIT(5)
+
+#define US_BRGR_SIZE		BIT(16)
+
+#define US_MIN_CLK_DIV		0x06
+#define US_MAX_CLK_DIV		BIT(16)
+
+#define US_DUMMY_TX		0xFF
+
+/* Register access macros */
+#define spi_readl(port, reg) \
+	readl_relaxed((port)->regs + US_##reg)
+#define spi_writel(port, reg, value) \
+	writel_relaxed((value), (port)->regs + US_##reg)
+
+#define spi_readb(port, reg) \
+	readb_relaxed((port)->regs + US_##reg)
+#define spi_writeb(port, reg, value) \
+	writeb_relaxed((value), (port)->regs + US_##reg)
+
+struct at91_usart_spi {
+	struct spi_transfer	*current_transfer;
+	void __iomem		*regs;
+	struct device		*dev;
+	struct clk		*clk;
+
+	/*used in interrupt to protect data reading*/
+	spinlock_t		lock;
+
+	int			irq;
+	unsigned int		current_tx_remaining_bytes;
+	unsigned int		current_rx_remaining_bytes;
+	int			done_status;
+
+	u32			spi_clk;
+	u32			status;
+
+	bool			xfer_failed;
+	bool			keep_cs;
+	bool			cs_active;
+};
+
+struct at91_usart_spi_device {
+	struct gpio_desc	*npcs_pin;
+	u32			mr;
+};
+
+static inline u32 at91_usart_spi_tx_ready(struct at91_usart_spi *aus)
+{
+	return aus->status & US_IR_TXRDY;
+}
+
+static inline u32 at91_usart_spi_rx_ready(struct at91_usart_spi *aus)
+{
+	return aus->status & US_IR_RXRDY;
+}
+
+static inline u32 at91_usart_spi_check_overrun(struct at91_usart_spi *aus)
+{
+	return aus->status & US_IR_OVRE;
+}
+
+static inline u32 at91_usart_spi_read_status(struct at91_usart_spi *aus)
+{
+	aus->status = spi_readl(aus, CSR);
+	return aus->status;
+}
+
+static inline void at91_usart_spi_tx(struct at91_usart_spi *aus)
+{
+	unsigned int len = aus->current_transfer->len;
+	unsigned int remaining = aus->current_tx_remaining_bytes;
+	const u8  *tx_buf = aus->current_transfer->tx_buf;
+
+	if (tx_buf && remaining) {
+		if (at91_usart_spi_tx_ready(aus))
+			spi_writel(aus, THR, tx_buf[len - remaining]);
+			aus->current_tx_remaining_bytes--;
+	} else {
+		if (at91_usart_spi_tx_ready(aus))
+			spi_writel(aus, THR, US_DUMMY_TX);
+	}
+}
+
+static inline void at91_usart_spi_rx(struct at91_usart_spi *aus)
+{
+	int len = aus->current_transfer->len;
+	int remaining = aus->current_rx_remaining_bytes;
+	u8  *rx_buf = aus->current_transfer->rx_buf;
+
+	if (aus->current_rx_remaining_bytes) {
+		rx_buf[len - remaining] = spi_readb(aus, RHR);
+		aus->current_rx_remaining_bytes--;
+	} else {
+		spi_readb(aus, RHR);
+	}
+}
+
+static inline void at91_usart_spi_cs_activate(struct spi_device *spi)
+{
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+	struct at91_usart_spi *aus = spi_master_get_devdata(spi->controller);
+	u32 active = spi->mode & SPI_CS_HIGH;
+
+	gpiod_set_value(ausd->npcs_pin, active);
+	aus->cs_active = true;
+}
+
+static inline void at91_usart_spi_cs_deactivate(struct spi_device *spi)
+{
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+	struct at91_usart_spi *aus = spi_master_get_devdata(spi->controller);
+	u32 active = spi->mode & SPI_CS_HIGH;
+
+	gpiod_set_value(ausd->npcs_pin, !active);
+	aus->cs_active = false;
+}
+
+static inline void at91_usart_spi_set_mode_register(struct spi_device *spi)
+{
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+	struct at91_usart_spi *aus = spi_master_get_devdata(spi->controller);
+
+	spi_writel(aus, MR, ausd->mr);
+}
+
+static inline void
+at91_usart_spi_enable_irq_and_hw(struct at91_usart_spi *aus)
+{
+	spi_writel(aus, CR, US_CR_RXEN | US_CR_TXEN);
+	spi_writel(aus, IER, US_IR_OVRE | US_IR_RXRDY);
+}
+
+static inline void
+at91_usart_spi_disable_irq_and_hw(struct at91_usart_spi *aus)
+{
+	spi_writel(aus, CR, US_CR_RXDIS | US_CR_TXDIS |
+		   US_CR_RSTRX | US_CR_RSTTX);
+	spi_writel(aus, IDR, US_IR_OVRE | US_IR_RXRDY);
+}
+
+static inline void
+at91_usart_spi_set_xfer_speed(struct at91_usart_spi *aus,
+			      struct spi_transfer *xfer)
+{
+	spi_writel(aus, BRGR,
+		   DIV_ROUND_UP(aus->spi_clk, xfer->speed_hz));
+}
+
+static irqreturn_t at91_usart_spi_interrupt(int irq, void *dev_id)
+{
+	struct spi_controller *controller = dev_id;
+	struct at91_usart_spi *aus = spi_master_get_devdata(controller);
+
+	spin_lock(&aus->lock);
+
+	at91_usart_spi_read_status(aus);
+
+	if (at91_usart_spi_check_overrun(aus)) {
+		aus->xfer_failed = true;
+		aus->done_status = -EIO;
+		spi_writel(aus, IDR, US_IR_OVRE | US_IR_RXRDY);
+		spin_unlock(&aus->lock);
+		return IRQ_HANDLED;
+	}
+
+	if (at91_usart_spi_rx_ready(aus)) {
+		at91_usart_spi_rx(aus);
+		spin_unlock(&aus->lock);
+		return IRQ_HANDLED;
+	}
+	spin_unlock(&aus->lock);
+
+	return IRQ_NONE;
+}
+
+static int at91_usart_spi_setup(struct spi_device *spi)
+{
+	struct at91_usart_spi *aus = spi_master_get_devdata(spi->controller);
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+	struct gpio_desc *npcs_pin;
+	unsigned int mr = spi_readl(aus, MR);
+	u8 bits = spi->bits_per_word;
+
+	if (bits != 8) {
+		dev_dbg(&spi->dev, "Only 8 bits per word are supported\n");
+		return -EINVAL;
+	}
+
+	if (spi->mode & SPI_CPOL)
+		mr |= US_MR_CPOL;
+	else
+		mr &= ~US_MR_CPOL;
+
+	if (spi->mode & SPI_CPHA)
+		mr |= US_MR_CPHA;
+	else
+		mr &= ~US_MR_CPHA;
+
+	if (spi->mode & SPI_LOOP)
+		mr |= US_MR_LOOP;
+	else
+		mr &= ~US_MR_LOOP;
+
+	if (!ausd) {
+		if (gpio_is_valid(spi->cs_gpio)) {
+			npcs_pin = gpio_to_desc(spi->cs_gpio);
+		} else {
+			dev_dbg(&spi->dev, "Invalid chip select\n");
+			return -EINVAL;
+		}
+
+		ausd = kzalloc(sizeof(*ausd), GFP_KERNEL);
+		if (!ausd)
+			return -ENOMEM;
+		gpiod_direction_output(npcs_pin, !(spi->mode & SPI_CS_HIGH));
+
+		ausd->npcs_pin = npcs_pin;
+		spi->controller_state = ausd;
+	}
+
+	ausd->mr = mr;
+
+	dev_dbg(&spi->dev,
+		"setup: bpw %u mode 0x%x -> mr %d %08x\n",
+		bits, spi->mode, spi->chip_select, mr);
+
+	return 0;
+}
+
+static int at91_usart_spi_one_transfer(struct spi_controller *controller,
+				       struct spi_message *msg,
+				       struct spi_transfer *xfer)
+{
+	struct at91_usart_spi *aus = spi_master_get_devdata(controller);
+	struct spi_device *spi = msg->spi;
+	const u8 *tx_buf = xfer->tx_buf;
+	u8 *rx_buf = xfer->rx_buf;
+
+	if (!(xfer->tx_buf || xfer->rx_buf) && xfer->len) {
+		dev_dbg(&spi->dev, "missing rx and tx buf\n");
+		return -EINVAL;
+	}
+
+	at91_usart_spi_set_xfer_speed(aus, xfer);
+	aus->done_status = 0;
+	aus->xfer_failed = false;
+	aus->current_transfer = xfer;
+	aus->current_tx_remaining_bytes = xfer->len;
+	aus->current_rx_remaining_bytes = xfer->len;
+	if (!tx_buf)
+		aus->current_tx_remaining_bytes = 0;
+	if (!rx_buf)
+		aus->current_rx_remaining_bytes = 0;
+
+	while ((aus->current_tx_remaining_bytes ||
+		aus->current_rx_remaining_bytes) && !aus->xfer_failed) {
+		at91_usart_spi_read_status(aus);
+		at91_usart_spi_tx(aus);
+		cpu_relax();
+	}
+	if (aus->xfer_failed) {
+		dev_err(aus->dev, "Overrun!\n");
+		return -EIO;
+	}
+
+	if (xfer->delay_usecs)
+		udelay(xfer->delay_usecs);
+
+	if (xfer->cs_change) {
+		if (list_is_last(&xfer->transfer_list, &msg->transfers)) {
+			aus->keep_cs = true;
+		} else {
+			aus->cs_active = !aus->cs_active;
+			if (aus->cs_active)
+				at91_usart_spi_cs_activate(spi);
+			else
+				at91_usart_spi_cs_deactivate(spi);
+		}
+	}
+
+	return 0;
+}
+
+static int
+at91_usart_spi_transfer_one_message(struct spi_controller *controller,
+				    struct spi_message *msg)
+{
+	struct at91_usart_spi *aus = spi_master_get_devdata(controller);
+	struct spi_transfer *xfer;
+	struct spi_device *spi = msg->spi;
+	int ret;
+
+	dev_dbg(&spi->dev, "new message %p submitted for %s\n",
+		msg, dev_name(&spi->dev));
+	at91_usart_spi_enable_irq_and_hw(aus);
+	at91_usart_spi_set_mode_register(spi);
+	at91_usart_spi_cs_activate(spi);
+
+	aus->keep_cs = false;
+
+	msg->status = 0;
+	msg->actual_length = 0;
+
+	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		ret = at91_usart_spi_one_transfer(controller, msg, xfer);
+		if (ret)
+			goto msg_done;
+	}
+
+msg_done:
+
+	if (!aus->keep_cs)
+		at91_usart_spi_cs_deactivate(spi);
+
+	at91_usart_spi_disable_irq_and_hw(aus);
+
+	msg->status = aus->done_status;
+	spi_finalize_current_message(spi->master);
+
+	return ret;
+}
+
+static void at91_usart_spi_cleanup(struct spi_device *spi)
+{
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+
+	if (!ausd)
+		return;
+
+	spi->controller_state = NULL;
+	kfree(ausd);
+}
+
+static int at91_usart_spi_gpio_cs(struct platform_device *pdev)
+{
+	struct spi_controller *controller = platform_get_drvdata(pdev);
+	struct device_node *np = controller->dev.parent->of_node;
+	struct gpio_desc *cs_gpio;
+	int nb;
+	int i;
+
+	if (!np)
+		return 0;
+
+	nb = of_gpio_named_count(np, "cs-gpios");
+	for (i = 0; i < nb; i++) {
+		cs_gpio = devm_gpiod_get_from_of_node(&pdev->dev,
+						      pdev->dev.parent->of_node,
+						      "cs-gpios",
+						      i, GPIOD_OUT_HIGH,
+						      dev_name(&pdev->dev));
+		if (IS_ERR(cs_gpio))
+			return PTR_ERR(cs_gpio);
+	}
+
+	controller->num_chipselect = nb;
+
+	return 0;
+}
+
+static void at91_usart_spi_init(struct at91_usart_spi *aus)
+{
+	spi_writel(aus, MR, US_MR_SPI_MASTER | US_MR_CHRL | US_MR_CLKO |
+			US_MR_WRDBT);
+	spi_writel(aus, CR, US_CR_RXDIS | US_CR_TXDIS | US_CR_RSTRX |
+			US_CR_RSTTX);
+}
+
+static int at91_usart_spi_probe(struct platform_device *pdev)
+{
+	struct resource *regs;
+	struct spi_controller *controller;
+	struct at91_usart_spi *aus;
+	struct clk *clk;
+	int irq;
+	int ret;
+
+	regs = platform_get_resource(to_platform_device(pdev->dev.parent),
+				     IORESOURCE_MEM, 0);
+	if (!regs)
+		return -ENXIO;
+
+	irq = platform_get_irq(to_platform_device(pdev->dev.parent), 0);
+	if (irq < 0)
+		return irq;
+
+	clk = devm_clk_get(pdev->dev.parent, "usart");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	ret = -ENOMEM;
+	controller = spi_alloc_master(&pdev->dev, sizeof(*aus));
+	if (!controller)
+		goto at91_usart_spi_probe_fail;
+
+	controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP | SPI_CS_HIGH;
+	controller->dev.of_node = pdev->dev.parent->of_node;
+	controller->bits_per_word_mask = SPI_BPW_MASK(8);
+	controller->num_chipselect = 0;
+	controller->setup = at91_usart_spi_setup;
+	controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
+	controller->transfer_one_message = at91_usart_spi_transfer_one_message;
+	controller->cleanup = at91_usart_spi_cleanup;
+	controller->max_speed_hz = DIV_ROUND_UP(clk_get_rate(clk),
+						US_MIN_CLK_DIV);
+	controller->min_speed_hz = DIV_ROUND_UP(clk_get_rate(clk),
+						US_MAX_CLK_DIV);
+	platform_set_drvdata(pdev, controller);
+
+	aus = spi_master_get_devdata(controller);
+
+	aus->dev = &pdev->dev;
+	aus->regs = devm_ioremap_resource(&pdev->dev, regs);
+	if (IS_ERR(aus->regs)) {
+		ret = PTR_ERR(aus->regs);
+		goto at91_usart_spi_probe_fail;
+	}
+
+	aus->irq = irq;
+	aus->clk = clk;
+
+	ret = at91_usart_spi_gpio_cs(pdev);
+	if (ret)
+		goto at91_usart_spi_probe_fail;
+
+	ret = devm_request_irq(&pdev->dev, irq, at91_usart_spi_interrupt, 0,
+			       dev_name(&pdev->dev), controller);
+	if (ret)
+		goto at91_usart_spi_probe_fail;
+
+	ret = clk_prepare_enable(clk);
+	if (ret)
+		goto at91_usart_spi_probe_fail;
+
+	aus->spi_clk = clk_get_rate(clk);
+	at91_usart_spi_init(aus);
+
+	spin_lock_init(&aus->lock);
+	ret = devm_spi_register_master(&pdev->dev, controller);
+	if (ret)
+		goto fail_register_master;
+
+	dev_info(&pdev->dev,
+		 "Atmel USART SPI Controller version 0x%x at 0x%08lx (irq %d)\n",
+		 spi_readl(aus, VERSION),
+		 (unsigned long)regs->start, irq);
+
+	return 0;
+
+fail_register_master:
+	clk_disable_unprepare(clk);
+at91_usart_spi_probe_fail:
+	spi_master_put(controller);
+	return ret;
+}
+
+static int at91_usart_spi_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct at91_usart_spi *aus = spi_master_get_devdata(master);
+
+	clk_disable_unprepare(aus->clk);
+
+	return 0;
+}
+
+static const struct of_device_id at91_usart_spi_dt_ids[] = {
+	{ .compatible = "microchip,sama5d3-usart-spi"},
+	{ .compatible = "microchip,sama5d4-usart-spi"},
+	{ .compatible = "microchip,at91sam9g45-usart-spi"},
+	{ /* sentinel */}
+};
+
+MODULE_DEVICE_TABLE(of, at91_usart_spi_dt_ids);
+
+static struct platform_driver at91_usart_spi_driver = {
+	.driver = {
+		.name = "at91_usart_spi",
+		.of_match_table = of_match_ptr(at91_usart_spi_dt_ids),
+	},
+	.probe = at91_usart_spi_probe,
+	.remove = at91_usart_spi_remove, };
+module_platform_driver(at91_usart_spi_driver);
+
+MODULE_DESCRIPTION("Microchip AT91 USART SPI Controller driver");
+MODULE_AUTHOR("Radu Pirea <radu.pirea@microchip.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:at91_usart_spi");
-- 
2.17.0

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

This is the driver for at91-usart in spi mode. The USART IP can be configured
to work in many modes and one of them is SPI.

The driver was tested on sama5d3-xplained and sama5d4-xplained boards with
enc28j60 ethernet controller as slave.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 drivers/spi/Kconfig          |   9 +
 drivers/spi/Makefile         |   1 +
 drivers/spi/spi-at91-usart.c | 544 +++++++++++++++++++++++++++++++++++
 3 files changed, 554 insertions(+)
 create mode 100644 drivers/spi/spi-at91-usart.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 6fb0347a24f2..c675e6b8dd5a 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -77,6 +77,15 @@ config SPI_ATMEL
 	  This selects a driver for the Atmel SPI Controller, present on
 	  many AT91 (ARM) chips.
 
+config SPI_AT91_USART
+        tristate "Atmel USART Controller as SPI"
+	depends on HAS_DMA
+	depends on (ARCH_AT91 || COMPILE_TEST)
+        select MFD_AT91_USART
+	help
+	  This selects a driver for the AT91 USART Controller as SPI Master,
+	  present on AT91 and SAMA5 SoC series.
+
 config SPI_AU1550
 	tristate "Au1550/Au1200/Au1300 SPI Controller"
 	depends on MIPS_ALCHEMY
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 34c5f2832ddf..fb6cb42f4eaa 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_SPI_LOOPBACK_TEST)		+= spi-loopback-test.o
 obj-$(CONFIG_SPI_ALTERA)		+= spi-altera.o
 obj-$(CONFIG_SPI_ARMADA_3700)		+= spi-armada-3700.o
 obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
+obj-$(CONFIG_SPI_AT91_USART)		+= spi-at91-usart.o
 obj-$(CONFIG_SPI_ATH79)			+= spi-ath79.o
 obj-$(CONFIG_SPI_AU1550)		+= spi-au1550.o
 obj-$(CONFIG_SPI_AXI_SPI_ENGINE)	+= spi-axi-spi-engine.o
diff --git a/drivers/spi/spi-at91-usart.c b/drivers/spi/spi-at91-usart.c
new file mode 100644
index 000000000000..79a59759d2ee
--- /dev/null
+++ b/drivers/spi/spi-at91-usart.c
@@ -0,0 +1,544 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for AT91 USART Controllers as SPI
+ *
+ * Copyright (C) 2018 Microchip Technology Inc.
+ * Author: Radu Pirea <radu.pirea@microchip.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+
+#include <linux/pinctrl/consumer.h>
+
+#include <linux/spi/spi.h>
+
+#define US_CR			0x00
+#define US_MR			0x04
+#define US_IER			0x08
+#define US_IDR			0x0C
+#define US_CSR			0x14
+#define US_RHR			0x18
+#define US_THR			0x1C
+#define US_BRGR			0x20
+#define US_VERSION		0xFC
+
+#define US_CR_RSTRX		BIT(2)
+#define US_CR_RSTTX		BIT(3)
+#define US_CR_RXEN		BIT(4)
+#define US_CR_RXDIS		BIT(5)
+#define US_CR_TXEN		BIT(6)
+#define US_CR_TXDIS		BIT(7)
+
+#define US_MR_SPI_MASTER	0x0E
+#define US_MR_CHRL		GENMASK(7, 6)
+#define US_MR_CPHA		BIT(8)
+#define US_MR_CPOL		BIT(16)
+#define US_MR_CLKO		BIT(18)
+#define US_MR_WRDBT		BIT(20)
+#define US_MR_LOOP		BIT(15)
+
+#define US_IR_RXRDY		BIT(0)
+#define US_IR_TXRDY		BIT(1)
+#define US_IR_OVRE		BIT(5)
+
+#define US_BRGR_SIZE		BIT(16)
+
+#define US_MIN_CLK_DIV		0x06
+#define US_MAX_CLK_DIV		BIT(16)
+
+#define US_DUMMY_TX		0xFF
+
+/* Register access macros */
+#define spi_readl(port, reg) \
+	readl_relaxed((port)->regs + US_##reg)
+#define spi_writel(port, reg, value) \
+	writel_relaxed((value), (port)->regs + US_##reg)
+
+#define spi_readb(port, reg) \
+	readb_relaxed((port)->regs + US_##reg)
+#define spi_writeb(port, reg, value) \
+	writeb_relaxed((value), (port)->regs + US_##reg)
+
+struct at91_usart_spi {
+	struct spi_transfer	*current_transfer;
+	void __iomem		*regs;
+	struct device		*dev;
+	struct clk		*clk;
+
+	/*used in interrupt to protect data reading*/
+	spinlock_t		lock;
+
+	int			irq;
+	unsigned int		current_tx_remaining_bytes;
+	unsigned int		current_rx_remaining_bytes;
+	int			done_status;
+
+	u32			spi_clk;
+	u32			status;
+
+	bool			xfer_failed;
+	bool			keep_cs;
+	bool			cs_active;
+};
+
+struct at91_usart_spi_device {
+	struct gpio_desc	*npcs_pin;
+	u32			mr;
+};
+
+static inline u32 at91_usart_spi_tx_ready(struct at91_usart_spi *aus)
+{
+	return aus->status & US_IR_TXRDY;
+}
+
+static inline u32 at91_usart_spi_rx_ready(struct at91_usart_spi *aus)
+{
+	return aus->status & US_IR_RXRDY;
+}
+
+static inline u32 at91_usart_spi_check_overrun(struct at91_usart_spi *aus)
+{
+	return aus->status & US_IR_OVRE;
+}
+
+static inline u32 at91_usart_spi_read_status(struct at91_usart_spi *aus)
+{
+	aus->status = spi_readl(aus, CSR);
+	return aus->status;
+}
+
+static inline void at91_usart_spi_tx(struct at91_usart_spi *aus)
+{
+	unsigned int len = aus->current_transfer->len;
+	unsigned int remaining = aus->current_tx_remaining_bytes;
+	const u8  *tx_buf = aus->current_transfer->tx_buf;
+
+	if (tx_buf && remaining) {
+		if (at91_usart_spi_tx_ready(aus))
+			spi_writel(aus, THR, tx_buf[len - remaining]);
+			aus->current_tx_remaining_bytes--;
+	} else {
+		if (at91_usart_spi_tx_ready(aus))
+			spi_writel(aus, THR, US_DUMMY_TX);
+	}
+}
+
+static inline void at91_usart_spi_rx(struct at91_usart_spi *aus)
+{
+	int len = aus->current_transfer->len;
+	int remaining = aus->current_rx_remaining_bytes;
+	u8  *rx_buf = aus->current_transfer->rx_buf;
+
+	if (aus->current_rx_remaining_bytes) {
+		rx_buf[len - remaining] = spi_readb(aus, RHR);
+		aus->current_rx_remaining_bytes--;
+	} else {
+		spi_readb(aus, RHR);
+	}
+}
+
+static inline void at91_usart_spi_cs_activate(struct spi_device *spi)
+{
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+	struct at91_usart_spi *aus = spi_master_get_devdata(spi->controller);
+	u32 active = spi->mode & SPI_CS_HIGH;
+
+	gpiod_set_value(ausd->npcs_pin, active);
+	aus->cs_active = true;
+}
+
+static inline void at91_usart_spi_cs_deactivate(struct spi_device *spi)
+{
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+	struct at91_usart_spi *aus = spi_master_get_devdata(spi->controller);
+	u32 active = spi->mode & SPI_CS_HIGH;
+
+	gpiod_set_value(ausd->npcs_pin, !active);
+	aus->cs_active = false;
+}
+
+static inline void at91_usart_spi_set_mode_register(struct spi_device *spi)
+{
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+	struct at91_usart_spi *aus = spi_master_get_devdata(spi->controller);
+
+	spi_writel(aus, MR, ausd->mr);
+}
+
+static inline void
+at91_usart_spi_enable_irq_and_hw(struct at91_usart_spi *aus)
+{
+	spi_writel(aus, CR, US_CR_RXEN | US_CR_TXEN);
+	spi_writel(aus, IER, US_IR_OVRE | US_IR_RXRDY);
+}
+
+static inline void
+at91_usart_spi_disable_irq_and_hw(struct at91_usart_spi *aus)
+{
+	spi_writel(aus, CR, US_CR_RXDIS | US_CR_TXDIS |
+		   US_CR_RSTRX | US_CR_RSTTX);
+	spi_writel(aus, IDR, US_IR_OVRE | US_IR_RXRDY);
+}
+
+static inline void
+at91_usart_spi_set_xfer_speed(struct at91_usart_spi *aus,
+			      struct spi_transfer *xfer)
+{
+	spi_writel(aus, BRGR,
+		   DIV_ROUND_UP(aus->spi_clk, xfer->speed_hz));
+}
+
+static irqreturn_t at91_usart_spi_interrupt(int irq, void *dev_id)
+{
+	struct spi_controller *controller = dev_id;
+	struct at91_usart_spi *aus = spi_master_get_devdata(controller);
+
+	spin_lock(&aus->lock);
+
+	at91_usart_spi_read_status(aus);
+
+	if (at91_usart_spi_check_overrun(aus)) {
+		aus->xfer_failed = true;
+		aus->done_status = -EIO;
+		spi_writel(aus, IDR, US_IR_OVRE | US_IR_RXRDY);
+		spin_unlock(&aus->lock);
+		return IRQ_HANDLED;
+	}
+
+	if (at91_usart_spi_rx_ready(aus)) {
+		at91_usart_spi_rx(aus);
+		spin_unlock(&aus->lock);
+		return IRQ_HANDLED;
+	}
+	spin_unlock(&aus->lock);
+
+	return IRQ_NONE;
+}
+
+static int at91_usart_spi_setup(struct spi_device *spi)
+{
+	struct at91_usart_spi *aus = spi_master_get_devdata(spi->controller);
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+	struct gpio_desc *npcs_pin;
+	unsigned int mr = spi_readl(aus, MR);
+	u8 bits = spi->bits_per_word;
+
+	if (bits != 8) {
+		dev_dbg(&spi->dev, "Only 8 bits per word are supported\n");
+		return -EINVAL;
+	}
+
+	if (spi->mode & SPI_CPOL)
+		mr |= US_MR_CPOL;
+	else
+		mr &= ~US_MR_CPOL;
+
+	if (spi->mode & SPI_CPHA)
+		mr |= US_MR_CPHA;
+	else
+		mr &= ~US_MR_CPHA;
+
+	if (spi->mode & SPI_LOOP)
+		mr |= US_MR_LOOP;
+	else
+		mr &= ~US_MR_LOOP;
+
+	if (!ausd) {
+		if (gpio_is_valid(spi->cs_gpio)) {
+			npcs_pin = gpio_to_desc(spi->cs_gpio);
+		} else {
+			dev_dbg(&spi->dev, "Invalid chip select\n");
+			return -EINVAL;
+		}
+
+		ausd = kzalloc(sizeof(*ausd), GFP_KERNEL);
+		if (!ausd)
+			return -ENOMEM;
+		gpiod_direction_output(npcs_pin, !(spi->mode & SPI_CS_HIGH));
+
+		ausd->npcs_pin = npcs_pin;
+		spi->controller_state = ausd;
+	}
+
+	ausd->mr = mr;
+
+	dev_dbg(&spi->dev,
+		"setup: bpw %u mode 0x%x -> mr %d %08x\n",
+		bits, spi->mode, spi->chip_select, mr);
+
+	return 0;
+}
+
+static int at91_usart_spi_one_transfer(struct spi_controller *controller,
+				       struct spi_message *msg,
+				       struct spi_transfer *xfer)
+{
+	struct at91_usart_spi *aus = spi_master_get_devdata(controller);
+	struct spi_device *spi = msg->spi;
+	const u8 *tx_buf = xfer->tx_buf;
+	u8 *rx_buf = xfer->rx_buf;
+
+	if (!(xfer->tx_buf || xfer->rx_buf) && xfer->len) {
+		dev_dbg(&spi->dev, "missing rx and tx buf\n");
+		return -EINVAL;
+	}
+
+	at91_usart_spi_set_xfer_speed(aus, xfer);
+	aus->done_status = 0;
+	aus->xfer_failed = false;
+	aus->current_transfer = xfer;
+	aus->current_tx_remaining_bytes = xfer->len;
+	aus->current_rx_remaining_bytes = xfer->len;
+	if (!tx_buf)
+		aus->current_tx_remaining_bytes = 0;
+	if (!rx_buf)
+		aus->current_rx_remaining_bytes = 0;
+
+	while ((aus->current_tx_remaining_bytes ||
+		aus->current_rx_remaining_bytes) && !aus->xfer_failed) {
+		at91_usart_spi_read_status(aus);
+		at91_usart_spi_tx(aus);
+		cpu_relax();
+	}
+	if (aus->xfer_failed) {
+		dev_err(aus->dev, "Overrun!\n");
+		return -EIO;
+	}
+
+	if (xfer->delay_usecs)
+		udelay(xfer->delay_usecs);
+
+	if (xfer->cs_change) {
+		if (list_is_last(&xfer->transfer_list, &msg->transfers)) {
+			aus->keep_cs = true;
+		} else {
+			aus->cs_active = !aus->cs_active;
+			if (aus->cs_active)
+				at91_usart_spi_cs_activate(spi);
+			else
+				at91_usart_spi_cs_deactivate(spi);
+		}
+	}
+
+	return 0;
+}
+
+static int
+at91_usart_spi_transfer_one_message(struct spi_controller *controller,
+				    struct spi_message *msg)
+{
+	struct at91_usart_spi *aus = spi_master_get_devdata(controller);
+	struct spi_transfer *xfer;
+	struct spi_device *spi = msg->spi;
+	int ret;
+
+	dev_dbg(&spi->dev, "new message %p submitted for %s\n",
+		msg, dev_name(&spi->dev));
+	at91_usart_spi_enable_irq_and_hw(aus);
+	at91_usart_spi_set_mode_register(spi);
+	at91_usart_spi_cs_activate(spi);
+
+	aus->keep_cs = false;
+
+	msg->status = 0;
+	msg->actual_length = 0;
+
+	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		ret = at91_usart_spi_one_transfer(controller, msg, xfer);
+		if (ret)
+			goto msg_done;
+	}
+
+msg_done:
+
+	if (!aus->keep_cs)
+		at91_usart_spi_cs_deactivate(spi);
+
+	at91_usart_spi_disable_irq_and_hw(aus);
+
+	msg->status = aus->done_status;
+	spi_finalize_current_message(spi->master);
+
+	return ret;
+}
+
+static void at91_usart_spi_cleanup(struct spi_device *spi)
+{
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+
+	if (!ausd)
+		return;
+
+	spi->controller_state = NULL;
+	kfree(ausd);
+}
+
+static int at91_usart_spi_gpio_cs(struct platform_device *pdev)
+{
+	struct spi_controller *controller = platform_get_drvdata(pdev);
+	struct device_node *np = controller->dev.parent->of_node;
+	struct gpio_desc *cs_gpio;
+	int nb;
+	int i;
+
+	if (!np)
+		return 0;
+
+	nb = of_gpio_named_count(np, "cs-gpios");
+	for (i = 0; i < nb; i++) {
+		cs_gpio = devm_gpiod_get_from_of_node(&pdev->dev,
+						      pdev->dev.parent->of_node,
+						      "cs-gpios",
+						      i, GPIOD_OUT_HIGH,
+						      dev_name(&pdev->dev));
+		if (IS_ERR(cs_gpio))
+			return PTR_ERR(cs_gpio);
+	}
+
+	controller->num_chipselect = nb;
+
+	return 0;
+}
+
+static void at91_usart_spi_init(struct at91_usart_spi *aus)
+{
+	spi_writel(aus, MR, US_MR_SPI_MASTER | US_MR_CHRL | US_MR_CLKO |
+			US_MR_WRDBT);
+	spi_writel(aus, CR, US_CR_RXDIS | US_CR_TXDIS | US_CR_RSTRX |
+			US_CR_RSTTX);
+}
+
+static int at91_usart_spi_probe(struct platform_device *pdev)
+{
+	struct resource *regs;
+	struct spi_controller *controller;
+	struct at91_usart_spi *aus;
+	struct clk *clk;
+	int irq;
+	int ret;
+
+	regs = platform_get_resource(to_platform_device(pdev->dev.parent),
+				     IORESOURCE_MEM, 0);
+	if (!regs)
+		return -ENXIO;
+
+	irq = platform_get_irq(to_platform_device(pdev->dev.parent), 0);
+	if (irq < 0)
+		return irq;
+
+	clk = devm_clk_get(pdev->dev.parent, "usart");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	ret = -ENOMEM;
+	controller = spi_alloc_master(&pdev->dev, sizeof(*aus));
+	if (!controller)
+		goto at91_usart_spi_probe_fail;
+
+	controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP | SPI_CS_HIGH;
+	controller->dev.of_node = pdev->dev.parent->of_node;
+	controller->bits_per_word_mask = SPI_BPW_MASK(8);
+	controller->num_chipselect = 0;
+	controller->setup = at91_usart_spi_setup;
+	controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
+	controller->transfer_one_message = at91_usart_spi_transfer_one_message;
+	controller->cleanup = at91_usart_spi_cleanup;
+	controller->max_speed_hz = DIV_ROUND_UP(clk_get_rate(clk),
+						US_MIN_CLK_DIV);
+	controller->min_speed_hz = DIV_ROUND_UP(clk_get_rate(clk),
+						US_MAX_CLK_DIV);
+	platform_set_drvdata(pdev, controller);
+
+	aus = spi_master_get_devdata(controller);
+
+	aus->dev = &pdev->dev;
+	aus->regs = devm_ioremap_resource(&pdev->dev, regs);
+	if (IS_ERR(aus->regs)) {
+		ret = PTR_ERR(aus->regs);
+		goto at91_usart_spi_probe_fail;
+	}
+
+	aus->irq = irq;
+	aus->clk = clk;
+
+	ret = at91_usart_spi_gpio_cs(pdev);
+	if (ret)
+		goto at91_usart_spi_probe_fail;
+
+	ret = devm_request_irq(&pdev->dev, irq, at91_usart_spi_interrupt, 0,
+			       dev_name(&pdev->dev), controller);
+	if (ret)
+		goto at91_usart_spi_probe_fail;
+
+	ret = clk_prepare_enable(clk);
+	if (ret)
+		goto at91_usart_spi_probe_fail;
+
+	aus->spi_clk = clk_get_rate(clk);
+	at91_usart_spi_init(aus);
+
+	spin_lock_init(&aus->lock);
+	ret = devm_spi_register_master(&pdev->dev, controller);
+	if (ret)
+		goto fail_register_master;
+
+	dev_info(&pdev->dev,
+		 "Atmel USART SPI Controller version 0x%x at 0x%08lx (irq %d)\n",
+		 spi_readl(aus, VERSION),
+		 (unsigned long)regs->start, irq);
+
+	return 0;
+
+fail_register_master:
+	clk_disable_unprepare(clk);
+at91_usart_spi_probe_fail:
+	spi_master_put(controller);
+	return ret;
+}
+
+static int at91_usart_spi_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct at91_usart_spi *aus = spi_master_get_devdata(master);
+
+	clk_disable_unprepare(aus->clk);
+
+	return 0;
+}
+
+static const struct of_device_id at91_usart_spi_dt_ids[] = {
+	{ .compatible = "microchip,sama5d3-usart-spi"},
+	{ .compatible = "microchip,sama5d4-usart-spi"},
+	{ .compatible = "microchip,at91sam9g45-usart-spi"},
+	{ /* sentinel */}
+};
+
+MODULE_DEVICE_TABLE(of, at91_usart_spi_dt_ids);
+
+static struct platform_driver at91_usart_spi_driver = {
+	.driver = {
+		.name = "at91_usart_spi",
+		.of_match_table = of_match_ptr(at91_usart_spi_dt_ids),
+	},
+	.probe = at91_usart_spi_probe,
+	.remove = at91_usart_spi_remove, };
+module_platform_driver(at91_usart_spi_driver);
+
+MODULE_DESCRIPTION("Microchip AT91 USART SPI Controller driver");
+MODULE_AUTHOR("Radu Pirea <radu.pirea@microchip.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:at91_usart_spi");
-- 
2.17.0

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

This is the driver for at91-usart in spi mode. The USART IP can be configured
to work in many modes and one of them is SPI.

The driver was tested on sama5d3-xplained and sama5d4-xplained boards with
enc28j60 ethernet controller as slave.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 drivers/spi/Kconfig          |   9 +
 drivers/spi/Makefile         |   1 +
 drivers/spi/spi-at91-usart.c | 544 +++++++++++++++++++++++++++++++++++
 3 files changed, 554 insertions(+)
 create mode 100644 drivers/spi/spi-at91-usart.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 6fb0347a24f2..c675e6b8dd5a 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -77,6 +77,15 @@ config SPI_ATMEL
 	  This selects a driver for the Atmel SPI Controller, present on
 	  many AT91 (ARM) chips.
 
+config SPI_AT91_USART
+        tristate "Atmel USART Controller as SPI"
+	depends on HAS_DMA
+	depends on (ARCH_AT91 || COMPILE_TEST)
+        select MFD_AT91_USART
+	help
+	  This selects a driver for the AT91 USART Controller as SPI Master,
+	  present on AT91 and SAMA5 SoC series.
+
 config SPI_AU1550
 	tristate "Au1550/Au1200/Au1300 SPI Controller"
 	depends on MIPS_ALCHEMY
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 34c5f2832ddf..fb6cb42f4eaa 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_SPI_LOOPBACK_TEST)		+= spi-loopback-test.o
 obj-$(CONFIG_SPI_ALTERA)		+= spi-altera.o
 obj-$(CONFIG_SPI_ARMADA_3700)		+= spi-armada-3700.o
 obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
+obj-$(CONFIG_SPI_AT91_USART)		+= spi-at91-usart.o
 obj-$(CONFIG_SPI_ATH79)			+= spi-ath79.o
 obj-$(CONFIG_SPI_AU1550)		+= spi-au1550.o
 obj-$(CONFIG_SPI_AXI_SPI_ENGINE)	+= spi-axi-spi-engine.o
diff --git a/drivers/spi/spi-at91-usart.c b/drivers/spi/spi-at91-usart.c
new file mode 100644
index 000000000000..79a59759d2ee
--- /dev/null
+++ b/drivers/spi/spi-at91-usart.c
@@ -0,0 +1,544 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for AT91 USART Controllers as SPI
+ *
+ * Copyright (C) 2018 Microchip Technology Inc.
+ * Author: Radu Pirea <radu.pirea@microchip.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+
+#include <linux/pinctrl/consumer.h>
+
+#include <linux/spi/spi.h>
+
+#define US_CR			0x00
+#define US_MR			0x04
+#define US_IER			0x08
+#define US_IDR			0x0C
+#define US_CSR			0x14
+#define US_RHR			0x18
+#define US_THR			0x1C
+#define US_BRGR			0x20
+#define US_VERSION		0xFC
+
+#define US_CR_RSTRX		BIT(2)
+#define US_CR_RSTTX		BIT(3)
+#define US_CR_RXEN		BIT(4)
+#define US_CR_RXDIS		BIT(5)
+#define US_CR_TXEN		BIT(6)
+#define US_CR_TXDIS		BIT(7)
+
+#define US_MR_SPI_MASTER	0x0E
+#define US_MR_CHRL		GENMASK(7, 6)
+#define US_MR_CPHA		BIT(8)
+#define US_MR_CPOL		BIT(16)
+#define US_MR_CLKO		BIT(18)
+#define US_MR_WRDBT		BIT(20)
+#define US_MR_LOOP		BIT(15)
+
+#define US_IR_RXRDY		BIT(0)
+#define US_IR_TXRDY		BIT(1)
+#define US_IR_OVRE		BIT(5)
+
+#define US_BRGR_SIZE		BIT(16)
+
+#define US_MIN_CLK_DIV		0x06
+#define US_MAX_CLK_DIV		BIT(16)
+
+#define US_DUMMY_TX		0xFF
+
+/* Register access macros */
+#define spi_readl(port, reg) \
+	readl_relaxed((port)->regs + US_##reg)
+#define spi_writel(port, reg, value) \
+	writel_relaxed((value), (port)->regs + US_##reg)
+
+#define spi_readb(port, reg) \
+	readb_relaxed((port)->regs + US_##reg)
+#define spi_writeb(port, reg, value) \
+	writeb_relaxed((value), (port)->regs + US_##reg)
+
+struct at91_usart_spi {
+	struct spi_transfer	*current_transfer;
+	void __iomem		*regs;
+	struct device		*dev;
+	struct clk		*clk;
+
+	/*used in interrupt to protect data reading*/
+	spinlock_t		lock;
+
+	int			irq;
+	unsigned int		current_tx_remaining_bytes;
+	unsigned int		current_rx_remaining_bytes;
+	int			done_status;
+
+	u32			spi_clk;
+	u32			status;
+
+	bool			xfer_failed;
+	bool			keep_cs;
+	bool			cs_active;
+};
+
+struct at91_usart_spi_device {
+	struct gpio_desc	*npcs_pin;
+	u32			mr;
+};
+
+static inline u32 at91_usart_spi_tx_ready(struct at91_usart_spi *aus)
+{
+	return aus->status & US_IR_TXRDY;
+}
+
+static inline u32 at91_usart_spi_rx_ready(struct at91_usart_spi *aus)
+{
+	return aus->status & US_IR_RXRDY;
+}
+
+static inline u32 at91_usart_spi_check_overrun(struct at91_usart_spi *aus)
+{
+	return aus->status & US_IR_OVRE;
+}
+
+static inline u32 at91_usart_spi_read_status(struct at91_usart_spi *aus)
+{
+	aus->status = spi_readl(aus, CSR);
+	return aus->status;
+}
+
+static inline void at91_usart_spi_tx(struct at91_usart_spi *aus)
+{
+	unsigned int len = aus->current_transfer->len;
+	unsigned int remaining = aus->current_tx_remaining_bytes;
+	const u8  *tx_buf = aus->current_transfer->tx_buf;
+
+	if (tx_buf && remaining) {
+		if (at91_usart_spi_tx_ready(aus))
+			spi_writel(aus, THR, tx_buf[len - remaining]);
+			aus->current_tx_remaining_bytes--;
+	} else {
+		if (at91_usart_spi_tx_ready(aus))
+			spi_writel(aus, THR, US_DUMMY_TX);
+	}
+}
+
+static inline void at91_usart_spi_rx(struct at91_usart_spi *aus)
+{
+	int len = aus->current_transfer->len;
+	int remaining = aus->current_rx_remaining_bytes;
+	u8  *rx_buf = aus->current_transfer->rx_buf;
+
+	if (aus->current_rx_remaining_bytes) {
+		rx_buf[len - remaining] = spi_readb(aus, RHR);
+		aus->current_rx_remaining_bytes--;
+	} else {
+		spi_readb(aus, RHR);
+	}
+}
+
+static inline void at91_usart_spi_cs_activate(struct spi_device *spi)
+{
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+	struct at91_usart_spi *aus = spi_master_get_devdata(spi->controller);
+	u32 active = spi->mode & SPI_CS_HIGH;
+
+	gpiod_set_value(ausd->npcs_pin, active);
+	aus->cs_active = true;
+}
+
+static inline void at91_usart_spi_cs_deactivate(struct spi_device *spi)
+{
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+	struct at91_usart_spi *aus = spi_master_get_devdata(spi->controller);
+	u32 active = spi->mode & SPI_CS_HIGH;
+
+	gpiod_set_value(ausd->npcs_pin, !active);
+	aus->cs_active = false;
+}
+
+static inline void at91_usart_spi_set_mode_register(struct spi_device *spi)
+{
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+	struct at91_usart_spi *aus = spi_master_get_devdata(spi->controller);
+
+	spi_writel(aus, MR, ausd->mr);
+}
+
+static inline void
+at91_usart_spi_enable_irq_and_hw(struct at91_usart_spi *aus)
+{
+	spi_writel(aus, CR, US_CR_RXEN | US_CR_TXEN);
+	spi_writel(aus, IER, US_IR_OVRE | US_IR_RXRDY);
+}
+
+static inline void
+at91_usart_spi_disable_irq_and_hw(struct at91_usart_spi *aus)
+{
+	spi_writel(aus, CR, US_CR_RXDIS | US_CR_TXDIS |
+		   US_CR_RSTRX | US_CR_RSTTX);
+	spi_writel(aus, IDR, US_IR_OVRE | US_IR_RXRDY);
+}
+
+static inline void
+at91_usart_spi_set_xfer_speed(struct at91_usart_spi *aus,
+			      struct spi_transfer *xfer)
+{
+	spi_writel(aus, BRGR,
+		   DIV_ROUND_UP(aus->spi_clk, xfer->speed_hz));
+}
+
+static irqreturn_t at91_usart_spi_interrupt(int irq, void *dev_id)
+{
+	struct spi_controller *controller = dev_id;
+	struct at91_usart_spi *aus = spi_master_get_devdata(controller);
+
+	spin_lock(&aus->lock);
+
+	at91_usart_spi_read_status(aus);
+
+	if (at91_usart_spi_check_overrun(aus)) {
+		aus->xfer_failed = true;
+		aus->done_status = -EIO;
+		spi_writel(aus, IDR, US_IR_OVRE | US_IR_RXRDY);
+		spin_unlock(&aus->lock);
+		return IRQ_HANDLED;
+	}
+
+	if (at91_usart_spi_rx_ready(aus)) {
+		at91_usart_spi_rx(aus);
+		spin_unlock(&aus->lock);
+		return IRQ_HANDLED;
+	}
+	spin_unlock(&aus->lock);
+
+	return IRQ_NONE;
+}
+
+static int at91_usart_spi_setup(struct spi_device *spi)
+{
+	struct at91_usart_spi *aus = spi_master_get_devdata(spi->controller);
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+	struct gpio_desc *npcs_pin;
+	unsigned int mr = spi_readl(aus, MR);
+	u8 bits = spi->bits_per_word;
+
+	if (bits != 8) {
+		dev_dbg(&spi->dev, "Only 8 bits per word are supported\n");
+		return -EINVAL;
+	}
+
+	if (spi->mode & SPI_CPOL)
+		mr |= US_MR_CPOL;
+	else
+		mr &= ~US_MR_CPOL;
+
+	if (spi->mode & SPI_CPHA)
+		mr |= US_MR_CPHA;
+	else
+		mr &= ~US_MR_CPHA;
+
+	if (spi->mode & SPI_LOOP)
+		mr |= US_MR_LOOP;
+	else
+		mr &= ~US_MR_LOOP;
+
+	if (!ausd) {
+		if (gpio_is_valid(spi->cs_gpio)) {
+			npcs_pin = gpio_to_desc(spi->cs_gpio);
+		} else {
+			dev_dbg(&spi->dev, "Invalid chip select\n");
+			return -EINVAL;
+		}
+
+		ausd = kzalloc(sizeof(*ausd), GFP_KERNEL);
+		if (!ausd)
+			return -ENOMEM;
+		gpiod_direction_output(npcs_pin, !(spi->mode & SPI_CS_HIGH));
+
+		ausd->npcs_pin = npcs_pin;
+		spi->controller_state = ausd;
+	}
+
+	ausd->mr = mr;
+
+	dev_dbg(&spi->dev,
+		"setup: bpw %u mode 0x%x -> mr %d %08x\n",
+		bits, spi->mode, spi->chip_select, mr);
+
+	return 0;
+}
+
+static int at91_usart_spi_one_transfer(struct spi_controller *controller,
+				       struct spi_message *msg,
+				       struct spi_transfer *xfer)
+{
+	struct at91_usart_spi *aus = spi_master_get_devdata(controller);
+	struct spi_device *spi = msg->spi;
+	const u8 *tx_buf = xfer->tx_buf;
+	u8 *rx_buf = xfer->rx_buf;
+
+	if (!(xfer->tx_buf || xfer->rx_buf) && xfer->len) {
+		dev_dbg(&spi->dev, "missing rx and tx buf\n");
+		return -EINVAL;
+	}
+
+	at91_usart_spi_set_xfer_speed(aus, xfer);
+	aus->done_status = 0;
+	aus->xfer_failed = false;
+	aus->current_transfer = xfer;
+	aus->current_tx_remaining_bytes = xfer->len;
+	aus->current_rx_remaining_bytes = xfer->len;
+	if (!tx_buf)
+		aus->current_tx_remaining_bytes = 0;
+	if (!rx_buf)
+		aus->current_rx_remaining_bytes = 0;
+
+	while ((aus->current_tx_remaining_bytes ||
+		aus->current_rx_remaining_bytes) && !aus->xfer_failed) {
+		at91_usart_spi_read_status(aus);
+		at91_usart_spi_tx(aus);
+		cpu_relax();
+	}
+	if (aus->xfer_failed) {
+		dev_err(aus->dev, "Overrun!\n");
+		return -EIO;
+	}
+
+	if (xfer->delay_usecs)
+		udelay(xfer->delay_usecs);
+
+	if (xfer->cs_change) {
+		if (list_is_last(&xfer->transfer_list, &msg->transfers)) {
+			aus->keep_cs = true;
+		} else {
+			aus->cs_active = !aus->cs_active;
+			if (aus->cs_active)
+				at91_usart_spi_cs_activate(spi);
+			else
+				at91_usart_spi_cs_deactivate(spi);
+		}
+	}
+
+	return 0;
+}
+
+static int
+at91_usart_spi_transfer_one_message(struct spi_controller *controller,
+				    struct spi_message *msg)
+{
+	struct at91_usart_spi *aus = spi_master_get_devdata(controller);
+	struct spi_transfer *xfer;
+	struct spi_device *spi = msg->spi;
+	int ret;
+
+	dev_dbg(&spi->dev, "new message %p submitted for %s\n",
+		msg, dev_name(&spi->dev));
+	at91_usart_spi_enable_irq_and_hw(aus);
+	at91_usart_spi_set_mode_register(spi);
+	at91_usart_spi_cs_activate(spi);
+
+	aus->keep_cs = false;
+
+	msg->status = 0;
+	msg->actual_length = 0;
+
+	list_for_each_entry(xfer, &msg->transfers, transfer_list) {
+		ret = at91_usart_spi_one_transfer(controller, msg, xfer);
+		if (ret)
+			goto msg_done;
+	}
+
+msg_done:
+
+	if (!aus->keep_cs)
+		at91_usart_spi_cs_deactivate(spi);
+
+	at91_usart_spi_disable_irq_and_hw(aus);
+
+	msg->status = aus->done_status;
+	spi_finalize_current_message(spi->master);
+
+	return ret;
+}
+
+static void at91_usart_spi_cleanup(struct spi_device *spi)
+{
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+
+	if (!ausd)
+		return;
+
+	spi->controller_state = NULL;
+	kfree(ausd);
+}
+
+static int at91_usart_spi_gpio_cs(struct platform_device *pdev)
+{
+	struct spi_controller *controller = platform_get_drvdata(pdev);
+	struct device_node *np = controller->dev.parent->of_node;
+	struct gpio_desc *cs_gpio;
+	int nb;
+	int i;
+
+	if (!np)
+		return 0;
+
+	nb = of_gpio_named_count(np, "cs-gpios");
+	for (i = 0; i < nb; i++) {
+		cs_gpio = devm_gpiod_get_from_of_node(&pdev->dev,
+						      pdev->dev.parent->of_node,
+						      "cs-gpios",
+						      i, GPIOD_OUT_HIGH,
+						      dev_name(&pdev->dev));
+		if (IS_ERR(cs_gpio))
+			return PTR_ERR(cs_gpio);
+	}
+
+	controller->num_chipselect = nb;
+
+	return 0;
+}
+
+static void at91_usart_spi_init(struct at91_usart_spi *aus)
+{
+	spi_writel(aus, MR, US_MR_SPI_MASTER | US_MR_CHRL | US_MR_CLKO |
+			US_MR_WRDBT);
+	spi_writel(aus, CR, US_CR_RXDIS | US_CR_TXDIS | US_CR_RSTRX |
+			US_CR_RSTTX);
+}
+
+static int at91_usart_spi_probe(struct platform_device *pdev)
+{
+	struct resource *regs;
+	struct spi_controller *controller;
+	struct at91_usart_spi *aus;
+	struct clk *clk;
+	int irq;
+	int ret;
+
+	regs = platform_get_resource(to_platform_device(pdev->dev.parent),
+				     IORESOURCE_MEM, 0);
+	if (!regs)
+		return -ENXIO;
+
+	irq = platform_get_irq(to_platform_device(pdev->dev.parent), 0);
+	if (irq < 0)
+		return irq;
+
+	clk = devm_clk_get(pdev->dev.parent, "usart");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	ret = -ENOMEM;
+	controller = spi_alloc_master(&pdev->dev, sizeof(*aus));
+	if (!controller)
+		goto at91_usart_spi_probe_fail;
+
+	controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP | SPI_CS_HIGH;
+	controller->dev.of_node = pdev->dev.parent->of_node;
+	controller->bits_per_word_mask = SPI_BPW_MASK(8);
+	controller->num_chipselect = 0;
+	controller->setup = at91_usart_spi_setup;
+	controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
+	controller->transfer_one_message = at91_usart_spi_transfer_one_message;
+	controller->cleanup = at91_usart_spi_cleanup;
+	controller->max_speed_hz = DIV_ROUND_UP(clk_get_rate(clk),
+						US_MIN_CLK_DIV);
+	controller->min_speed_hz = DIV_ROUND_UP(clk_get_rate(clk),
+						US_MAX_CLK_DIV);
+	platform_set_drvdata(pdev, controller);
+
+	aus = spi_master_get_devdata(controller);
+
+	aus->dev = &pdev->dev;
+	aus->regs = devm_ioremap_resource(&pdev->dev, regs);
+	if (IS_ERR(aus->regs)) {
+		ret = PTR_ERR(aus->regs);
+		goto at91_usart_spi_probe_fail;
+	}
+
+	aus->irq = irq;
+	aus->clk = clk;
+
+	ret = at91_usart_spi_gpio_cs(pdev);
+	if (ret)
+		goto at91_usart_spi_probe_fail;
+
+	ret = devm_request_irq(&pdev->dev, irq, at91_usart_spi_interrupt, 0,
+			       dev_name(&pdev->dev), controller);
+	if (ret)
+		goto at91_usart_spi_probe_fail;
+
+	ret = clk_prepare_enable(clk);
+	if (ret)
+		goto at91_usart_spi_probe_fail;
+
+	aus->spi_clk = clk_get_rate(clk);
+	at91_usart_spi_init(aus);
+
+	spin_lock_init(&aus->lock);
+	ret = devm_spi_register_master(&pdev->dev, controller);
+	if (ret)
+		goto fail_register_master;
+
+	dev_info(&pdev->dev,
+		 "Atmel USART SPI Controller version 0x%x at 0x%08lx (irq %d)\n",
+		 spi_readl(aus, VERSION),
+		 (unsigned long)regs->start, irq);
+
+	return 0;
+
+fail_register_master:
+	clk_disable_unprepare(clk);
+at91_usart_spi_probe_fail:
+	spi_master_put(controller);
+	return ret;
+}
+
+static int at91_usart_spi_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct at91_usart_spi *aus = spi_master_get_devdata(master);
+
+	clk_disable_unprepare(aus->clk);
+
+	return 0;
+}
+
+static const struct of_device_id at91_usart_spi_dt_ids[] = {
+	{ .compatible = "microchip,sama5d3-usart-spi"},
+	{ .compatible = "microchip,sama5d4-usart-spi"},
+	{ .compatible = "microchip,at91sam9g45-usart-spi"},
+	{ /* sentinel */}
+};
+
+MODULE_DEVICE_TABLE(of, at91_usart_spi_dt_ids);
+
+static struct platform_driver at91_usart_spi_driver = {
+	.driver = {
+		.name = "at91_usart_spi",
+		.of_match_table = of_match_ptr(at91_usart_spi_dt_ids),
+	},
+	.probe = at91_usart_spi_probe,
+	.remove = at91_usart_spi_remove, };
+module_platform_driver(at91_usart_spi_driver);
+
+MODULE_DESCRIPTION("Microchip AT91 USART SPI Controller driver");
+MODULE_AUTHOR("Radu Pirea <radu.pirea@microchip.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:at91_usart_spi");
-- 
2.17.0

[PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Radu Pirea]

This patch modifies the place where resources and device tree properties
are searched.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 drivers/tty/serial/Kconfig        |  1 +
 drivers/tty/serial/atmel_serial.c | 29 +++++++++++++++--------------
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 3682fd3e960c..25e55332f8b1 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -119,6 +119,7 @@ config SERIAL_ATMEL
 	depends on ARCH_AT91 || COMPILE_TEST
 	select SERIAL_CORE
 	select SERIAL_MCTRL_GPIO if GPIOLIB
+	select MFD_AT91_USART
 	help
 	  This enables the driver for the on-chip UARTs of the Atmel
 	  AT91 processors.
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index df46a9e88c34..6b4494352853 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -193,8 +193,8 @@ static struct console atmel_console;
 
 #if defined(CONFIG_OF)
 static const struct of_device_id atmel_serial_dt_ids[] = {
-	{ .compatible = "atmel,at91rm9200-usart" },
-	{ .compatible = "atmel,at91sam9260-usart" },
+	{ .compatible = "atmel,at91rm9200-usart-serial" },
+	{ .compatible = "atmel,at91sam9260-usart-serial" },
 	{ /* sentinel */ }
 };
 #endif
@@ -1631,7 +1631,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
 static void atmel_init_property(struct atmel_uart_port *atmel_port,
 				struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = pdev->dev.parent->of_node;
 
 	/* DMA/PDC usage specification */
 	if (of_property_read_bool(np, "atmel,use-dma-rx")) {
@@ -2223,7 +2223,8 @@ static const char *atmel_type(struct uart_port *port)
 static void atmel_release_port(struct uart_port *port)
 {
 	struct platform_device *pdev = to_platform_device(port->dev);
-	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
+	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
+		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
 
 	release_mem_region(port->mapbase, size);
 
@@ -2239,7 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
 static int atmel_request_port(struct uart_port *port)
 {
 	struct platform_device *pdev = to_platform_device(port->dev);
-	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
+	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
+		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
 
 	if (!request_mem_region(port->mapbase, size, "atmel_serial"))
 		return -EBUSY;
@@ -2345,23 +2347,23 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
 	atmel_init_property(atmel_port, pdev);
 	atmel_set_ops(port);
 
-	uart_get_rs485_mode(&pdev->dev, &port->rs485);
+	uart_get_rs485_mode(pdev->dev.parent, &port->rs485);
 
 	port->iotype		= UPIO_MEM;
 	port->flags		= UPF_BOOT_AUTOCONF | UPF_IOREMAP;
 	port->ops		= &atmel_pops;
 	port->fifosize		= 1;
 	port->dev		= &pdev->dev;
-	port->mapbase	= pdev->resource[0].start;
-	port->irq	= pdev->resource[1].start;
+	port->mapbase		= to_platform_device(pdev->dev.parent)->resource[0].start;
+	port->irq		= to_platform_device(pdev->dev.parent)->resource[1].start;
 	port->rs485_config	= atmel_config_rs485;
-	port->membase	= NULL;
+	port->membase		= NULL;
 
 	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
 
 	/* for console, the clock could already be configured */
 	if (!atmel_port->clk) {
-		atmel_port->clk = clk_get(&pdev->dev, "usart");
+		atmel_port->clk = clk_get(pdev->dev.parent, "usart");
 		if (IS_ERR(atmel_port->clk)) {
 			ret = PTR_ERR(atmel_port->clk);
 			atmel_port->clk = NULL;
@@ -2656,7 +2658,7 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
 	atmel_port->rts_low = 0;
 	atmel_port->rts_high = 0;
 
-	if (of_property_read_u32(pdev->dev.of_node,
+	if (of_property_read_u32(pdev->dev.parent->of_node,
 				 "atmel,fifo-size",
 				 &atmel_port->fifo_size))
 		return;
@@ -2694,11 +2696,10 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
 static int atmel_serial_probe(struct platform_device *pdev)
 {
 	struct atmel_uart_port *atmel_port;
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = pdev->dev.parent->of_node;
 	void *data;
 	int ret = -ENODEV;
 	bool rs485_enabled;
-
 	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
 
 	ret = of_alias_get_id(np, "serial");
@@ -2845,7 +2846,7 @@ static struct platform_driver atmel_serial_driver = {
 	.suspend	= atmel_serial_suspend,
 	.resume		= atmel_serial_resume,
 	.driver		= {
-		.name			= "atmel_usart",
+		.name			= "atmel_usart_serial",
 		.of_match_table		= of_match_ptr(atmel_serial_dt_ids),
 	},
 };
-- 
2.17.0

[PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Radu Pirea]

This patch modifies the place where resources and device tree properties
are searched.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 drivers/tty/serial/Kconfig        |  1 +
 drivers/tty/serial/atmel_serial.c | 29 +++++++++++++++--------------
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 3682fd3e960c..25e55332f8b1 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -119,6 +119,7 @@ config SERIAL_ATMEL
 	depends on ARCH_AT91 || COMPILE_TEST
 	select SERIAL_CORE
 	select SERIAL_MCTRL_GPIO if GPIOLIB
+	select MFD_AT91_USART
 	help
 	  This enables the driver for the on-chip UARTs of the Atmel
 	  AT91 processors.
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index df46a9e88c34..6b4494352853 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -193,8 +193,8 @@ static struct console atmel_console;
 
 #if defined(CONFIG_OF)
 static const struct of_device_id atmel_serial_dt_ids[] = {
-	{ .compatible = "atmel,at91rm9200-usart" },
-	{ .compatible = "atmel,at91sam9260-usart" },
+	{ .compatible = "atmel,at91rm9200-usart-serial" },
+	{ .compatible = "atmel,at91sam9260-usart-serial" },
 	{ /* sentinel */ }
 };
 #endif
@@ -1631,7 +1631,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
 static void atmel_init_property(struct atmel_uart_port *atmel_port,
 				struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = pdev->dev.parent->of_node;
 
 	/* DMA/PDC usage specification */
 	if (of_property_read_bool(np, "atmel,use-dma-rx")) {
@@ -2223,7 +2223,8 @@ static const char *atmel_type(struct uart_port *port)
 static void atmel_release_port(struct uart_port *port)
 {
 	struct platform_device *pdev = to_platform_device(port->dev);
-	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
+	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
+		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
 
 	release_mem_region(port->mapbase, size);
 
@@ -2239,7 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
 static int atmel_request_port(struct uart_port *port)
 {
 	struct platform_device *pdev = to_platform_device(port->dev);
-	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
+	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
+		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
 
 	if (!request_mem_region(port->mapbase, size, "atmel_serial"))
 		return -EBUSY;
@@ -2345,23 +2347,23 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
 	atmel_init_property(atmel_port, pdev);
 	atmel_set_ops(port);
 
-	uart_get_rs485_mode(&pdev->dev, &port->rs485);
+	uart_get_rs485_mode(pdev->dev.parent, &port->rs485);
 
 	port->iotype		= UPIO_MEM;
 	port->flags		= UPF_BOOT_AUTOCONF | UPF_IOREMAP;
 	port->ops		= &atmel_pops;
 	port->fifosize		= 1;
 	port->dev		= &pdev->dev;
-	port->mapbase	= pdev->resource[0].start;
-	port->irq	= pdev->resource[1].start;
+	port->mapbase		= to_platform_device(pdev->dev.parent)->resource[0].start;
+	port->irq		= to_platform_device(pdev->dev.parent)->resource[1].start;
 	port->rs485_config	= atmel_config_rs485;
-	port->membase	= NULL;
+	port->membase		= NULL;
 
 	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
 
 	/* for console, the clock could already be configured */
 	if (!atmel_port->clk) {
-		atmel_port->clk = clk_get(&pdev->dev, "usart");
+		atmel_port->clk = clk_get(pdev->dev.parent, "usart");
 		if (IS_ERR(atmel_port->clk)) {
 			ret = PTR_ERR(atmel_port->clk);
 			atmel_port->clk = NULL;
@@ -2656,7 +2658,7 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
 	atmel_port->rts_low = 0;
 	atmel_port->rts_high = 0;
 
-	if (of_property_read_u32(pdev->dev.of_node,
+	if (of_property_read_u32(pdev->dev.parent->of_node,
 				 "atmel,fifo-size",
 				 &atmel_port->fifo_size))
 		return;
@@ -2694,11 +2696,10 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
 static int atmel_serial_probe(struct platform_device *pdev)
 {
 	struct atmel_uart_port *atmel_port;
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = pdev->dev.parent->of_node;
 	void *data;
 	int ret = -ENODEV;
 	bool rs485_enabled;
-
 	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
 
 	ret = of_alias_get_id(np, "serial");
@@ -2845,7 +2846,7 @@ static struct platform_driver atmel_serial_driver = {
 	.suspend	= atmel_serial_suspend,
 	.resume		= atmel_serial_resume,
 	.driver		= {
-		.name			= "atmel_usart",
+		.name			= "atmel_usart_serial",
 		.of_match_table		= of_match_ptr(atmel_serial_dt_ids),
 	},
 };
-- 
2.17.0

[PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Radu Pirea]

This patch modifies the place where resources and device tree properties
are searched.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 drivers/tty/serial/Kconfig        |  1 +
 drivers/tty/serial/atmel_serial.c | 29 +++++++++++++++--------------
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 3682fd3e960c..25e55332f8b1 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -119,6 +119,7 @@ config SERIAL_ATMEL
 	depends on ARCH_AT91 || COMPILE_TEST
 	select SERIAL_CORE
 	select SERIAL_MCTRL_GPIO if GPIOLIB
+	select MFD_AT91_USART
 	help
 	  This enables the driver for the on-chip UARTs of the Atmel
 	  AT91 processors.
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index df46a9e88c34..6b4494352853 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -193,8 +193,8 @@ static struct console atmel_console;
 
 #if defined(CONFIG_OF)
 static const struct of_device_id atmel_serial_dt_ids[] = {
-	{ .compatible = "atmel,at91rm9200-usart" },
-	{ .compatible = "atmel,at91sam9260-usart" },
+	{ .compatible = "atmel,at91rm9200-usart-serial" },
+	{ .compatible = "atmel,at91sam9260-usart-serial" },
 	{ /* sentinel */ }
 };
 #endif
@@ -1631,7 +1631,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
 static void atmel_init_property(struct atmel_uart_port *atmel_port,
 				struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = pdev->dev.parent->of_node;
 
 	/* DMA/PDC usage specification */
 	if (of_property_read_bool(np, "atmel,use-dma-rx")) {
@@ -2223,7 +2223,8 @@ static const char *atmel_type(struct uart_port *port)
 static void atmel_release_port(struct uart_port *port)
 {
 	struct platform_device *pdev = to_platform_device(port->dev);
-	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
+	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
+		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
 
 	release_mem_region(port->mapbase, size);
 
@@ -2239,7 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
 static int atmel_request_port(struct uart_port *port)
 {
 	struct platform_device *pdev = to_platform_device(port->dev);
-	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
+	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
+		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
 
 	if (!request_mem_region(port->mapbase, size, "atmel_serial"))
 		return -EBUSY;
@@ -2345,23 +2347,23 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
 	atmel_init_property(atmel_port, pdev);
 	atmel_set_ops(port);
 
-	uart_get_rs485_mode(&pdev->dev, &port->rs485);
+	uart_get_rs485_mode(pdev->dev.parent, &port->rs485);
 
 	port->iotype		= UPIO_MEM;
 	port->flags		= UPF_BOOT_AUTOCONF | UPF_IOREMAP;
 	port->ops		= &atmel_pops;
 	port->fifosize		= 1;
 	port->dev		= &pdev->dev;
-	port->mapbase	= pdev->resource[0].start;
-	port->irq	= pdev->resource[1].start;
+	port->mapbase		= to_platform_device(pdev->dev.parent)->resource[0].start;
+	port->irq		= to_platform_device(pdev->dev.parent)->resource[1].start;
 	port->rs485_config	= atmel_config_rs485;
-	port->membase	= NULL;
+	port->membase		= NULL;
 
 	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
 
 	/* for console, the clock could already be configured */
 	if (!atmel_port->clk) {
-		atmel_port->clk = clk_get(&pdev->dev, "usart");
+		atmel_port->clk = clk_get(pdev->dev.parent, "usart");
 		if (IS_ERR(atmel_port->clk)) {
 			ret = PTR_ERR(atmel_port->clk);
 			atmel_port->clk = NULL;
@@ -2656,7 +2658,7 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
 	atmel_port->rts_low = 0;
 	atmel_port->rts_high = 0;
 
-	if (of_property_read_u32(pdev->dev.of_node,
+	if (of_property_read_u32(pdev->dev.parent->of_node,
 				 "atmel,fifo-size",
 				 &atmel_port->fifo_size))
 		return;
@@ -2694,11 +2696,10 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
 static int atmel_serial_probe(struct platform_device *pdev)
 {
 	struct atmel_uart_port *atmel_port;
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = pdev->dev.parent->of_node;
 	void *data;
 	int ret = -ENODEV;
 	bool rs485_enabled;
-
 	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
 
 	ret = of_alias_get_id(np, "serial");
@@ -2845,7 +2846,7 @@ static struct platform_driver atmel_serial_driver = {
 	.suspend	= atmel_serial_suspend,
 	.resume		= atmel_serial_resume,
 	.driver		= {
-		.name			= "atmel_usart",
+		.name			= "atmel_usart_serial",
 		.of_match_table		= of_match_ptr(atmel_serial_dt_ids),
 	},
 };
-- 
2.17.0

Re: [PATCH v3 3/6] MAINTAINERS: add at91 usart spi driver

[Andy Shevchenko]

On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip.com> wrote:
> Added entry for at91 usart mfd driver.
>

You are adding a record for not existing file?

> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
>  MAINTAINERS | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ca06c6f58299..9243b9007966 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9199,6 +9199,13 @@ S:       Supported
>  F:     drivers/mfd/at91-usart.c
>  F:     include/dt-bindings/mfd/at91-usart.h
>
> +MICROCHIP AT91 USART SPI DRIVER
> +M:     Radu Pirea <radu.pirea@microchip.com>
> +L:     linux-spi@vger.kernel.org
> +S:     Supported
> +F:     drivers/spi/spi-at91-usart.c
> +F:     Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
> +
>  MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
>  M:     Woojung Huh <Woojung.Huh@microchip.com>
>  M:     Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> --
> 2.17.0
>



-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 3/6] MAINTAINERS: add at91 usart spi driver

[Andy Shevchenko]

On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip.com> wrote:
> Added entry for at91 usart mfd driver.
>

You are adding a record for not existing file?

> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
>  MAINTAINERS | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ca06c6f58299..9243b9007966 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9199,6 +9199,13 @@ S:       Supported
>  F:     drivers/mfd/at91-usart.c
>  F:     include/dt-bindings/mfd/at91-usart.h
>
> +MICROCHIP AT91 USART SPI DRIVER
> +M:     Radu Pirea <radu.pirea@microchip.com>
> +L:     linux-spi at vger.kernel.org
> +S:     Supported
> +F:     drivers/spi/spi-at91-usart.c
> +F:     Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
> +
>  MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
>  M:     Woojung Huh <Woojung.Huh@microchip.com>
>  M:     Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
> --
> 2.17.0
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Andy Shevchenko]

On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip.com> wrote:
> This is the driver for at91-usart in spi mode. The USART IP can be configured
> to work in many modes and one of them is SPI.

> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>

Here is something wrong. You need to use latter one in new code.

> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>

Hmm... Do you need all of them?

> +static inline void at91_usart_spi_cs_activate(struct spi_device *spi)
> +{
...
> +       gpiod_set_value(ausd->npcs_pin, active);
> +       aus->cs_active = true;
> +}
> +
> +static inline void at91_usart_spi_cs_deactivate(struct spi_device *spi)
> +{
...
> +       gpiod_set_value(ausd->npcs_pin, !active);
> +       aus->cs_active = false;
> +}
...
> +       if (!ausd) {
> +               if (gpio_is_valid(spi->cs_gpio)) {
> +                       npcs_pin = gpio_to_desc(spi->cs_gpio);
...
> +               }
...
> +               gpiod_direction_output(npcs_pin, !(spi->mode & SPI_CS_HIGH));
> +
> +               ausd->npcs_pin = npcs_pin;
...
> +       }

I will refer to above as (1) later on.

> +       dev_dbg(&spi->dev, "new message %p submitted for %s\n",
> +               msg, dev_name(&spi->dev));

%p does make a very little sense.

> +       list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> +               ret = at91_usart_spi_one_transfer(controller, msg, xfer);
> +               if (ret)
> +                       goto msg_done;
> +       }

Cant SPI core do this for your?

> +static void at91_usart_spi_cleanup(struct spi_device *spi)
> +{
> +       struct at91_usart_spi_device *ausd = spi->controller_state;
> +

> +       if (!ausd)
> +               return;

Is it even possible?

Anyway the code below will work fine even if it's the case.

> +
> +       spi->controller_state = NULL;
> +       kfree(ausd);
> +}

> +static int at91_usart_spi_gpio_cs(struct platform_device *pdev)
> +{
> +       struct spi_controller *controller = platform_get_drvdata(pdev);
> +       struct device_node *np = controller->dev.parent->of_node;
> +       struct gpio_desc *cs_gpio;
> +       int nb;
> +       int i;
> +
> +       if (!np)
> +               return 0;
> +
> +       nb = of_gpio_named_count(np, "cs-gpios");
> +       for (i = 0; i < nb; i++) {
> +               cs_gpio = devm_gpiod_get_from_of_node(&pdev->dev,
> +                                                     pdev->dev.parent->of_node,
> +                                                     "cs-gpios",
> +                                                     i, GPIOD_OUT_HIGH,
> +                                                     dev_name(&pdev->dev));
> +               if (IS_ERR(cs_gpio))
> +                       return PTR_ERR(cs_gpio);
> +       }
> +
> +       controller->num_chipselect = nb;
> +
> +       return 0;
> +}

The question is, why you didn't utilize what SPI core provides you?

> +       spi_writel(aus, MR, US_MR_SPI_MASTER | US_MR_CHRL | US_MR_CLKO |
> +                       US_MR_WRDBT);
> +       spi_writel(aus, CR, US_CR_RXDIS | US_CR_TXDIS | US_CR_RSTRX |
> +                       US_CR_RSTTX);

I didn't check over, but it seems like you might have duplication in
these bitwise ORs. Consider to unify them into another (shorter)
definitions and reuse all over the code.

> +       regs = platform_get_resource(to_platform_device(pdev->dev.parent),
> +                                    IORESOURCE_MEM, 0);
> +       if (!regs)
> +               return -ENXIO;

Strange error code for getting MMIO resource. ENOMEM sounds better.

> +       dev_info(&pdev->dev,
> +                "Atmel USART SPI Controller version 0x%x at 0x%08lx (irq %d)\n",
> +                spi_readl(aus, VERSION),
> +                (unsigned long)regs->start, irq);

If you do explicit casting when printing something you are doing wrong.
Please use %pR or %pr in this case.

> +static struct platform_driver at91_usart_spi_driver = {
> +       .driver = {
> +               .name = "at91_usart_spi",

> +               .of_match_table = of_match_ptr(at91_usart_spi_dt_ids),

Can it work as pure platform driver? If no, of_match_ptr() is redundant.

> +       },
> +       .probe = at91_usart_spi_probe,
> +       .remove = at91_usart_spi_remove, };

Two lines at one. Split.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Andy Shevchenko]

On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip.com> wrote:
> This is the driver for at91-usart in spi mode. The USART IP can be configured
> to work in many modes and one of them is SPI.

> +#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>

Here is something wrong. You need to use latter one in new code.

> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_gpio.h>

Hmm... Do you need all of them?

> +static inline void at91_usart_spi_cs_activate(struct spi_device *spi)
> +{
...
> +       gpiod_set_value(ausd->npcs_pin, active);
> +       aus->cs_active = true;
> +}
> +
> +static inline void at91_usart_spi_cs_deactivate(struct spi_device *spi)
> +{
...
> +       gpiod_set_value(ausd->npcs_pin, !active);
> +       aus->cs_active = false;
> +}
...
> +       if (!ausd) {
> +               if (gpio_is_valid(spi->cs_gpio)) {
> +                       npcs_pin = gpio_to_desc(spi->cs_gpio);
...
> +               }
...
> +               gpiod_direction_output(npcs_pin, !(spi->mode & SPI_CS_HIGH));
> +
> +               ausd->npcs_pin = npcs_pin;
...
> +       }

I will refer to above as (1) later on.

> +       dev_dbg(&spi->dev, "new message %p submitted for %s\n",
> +               msg, dev_name(&spi->dev));

%p does make a very little sense.

> +       list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> +               ret = at91_usart_spi_one_transfer(controller, msg, xfer);
> +               if (ret)
> +                       goto msg_done;
> +       }

Cant SPI core do this for your?

> +static void at91_usart_spi_cleanup(struct spi_device *spi)
> +{
> +       struct at91_usart_spi_device *ausd = spi->controller_state;
> +

> +       if (!ausd)
> +               return;

Is it even possible?

Anyway the code below will work fine even if it's the case.

> +
> +       spi->controller_state = NULL;
> +       kfree(ausd);
> +}

> +static int at91_usart_spi_gpio_cs(struct platform_device *pdev)
> +{
> +       struct spi_controller *controller = platform_get_drvdata(pdev);
> +       struct device_node *np = controller->dev.parent->of_node;
> +       struct gpio_desc *cs_gpio;
> +       int nb;
> +       int i;
> +
> +       if (!np)
> +               return 0;
> +
> +       nb = of_gpio_named_count(np, "cs-gpios");
> +       for (i = 0; i < nb; i++) {
> +               cs_gpio = devm_gpiod_get_from_of_node(&pdev->dev,
> +                                                     pdev->dev.parent->of_node,
> +                                                     "cs-gpios",
> +                                                     i, GPIOD_OUT_HIGH,
> +                                                     dev_name(&pdev->dev));
> +               if (IS_ERR(cs_gpio))
> +                       return PTR_ERR(cs_gpio);
> +       }
> +
> +       controller->num_chipselect = nb;
> +
> +       return 0;
> +}

The question is, why you didn't utilize what SPI core provides you?

> +       spi_writel(aus, MR, US_MR_SPI_MASTER | US_MR_CHRL | US_MR_CLKO |
> +                       US_MR_WRDBT);
> +       spi_writel(aus, CR, US_CR_RXDIS | US_CR_TXDIS | US_CR_RSTRX |
> +                       US_CR_RSTTX);

I didn't check over, but it seems like you might have duplication in
these bitwise ORs. Consider to unify them into another (shorter)
definitions and reuse all over the code.

> +       regs = platform_get_resource(to_platform_device(pdev->dev.parent),
> +                                    IORESOURCE_MEM, 0);
> +       if (!regs)
> +               return -ENXIO;

Strange error code for getting MMIO resource. ENOMEM sounds better.

> +       dev_info(&pdev->dev,
> +                "Atmel USART SPI Controller version 0x%x at 0x%08lx (irq %d)\n",
> +                spi_readl(aus, VERSION),
> +                (unsigned long)regs->start, irq);

If you do explicit casting when printing something you are doing wrong.
Please use %pR or %pr in this case.

> +static struct platform_driver at91_usart_spi_driver = {
> +       .driver = {
> +               .name = "at91_usart_spi",

> +               .of_match_table = of_match_ptr(at91_usart_spi_dt_ids),

Can it work as pure platform driver? If no, of_match_ptr() is redundant.

> +       },
> +       .probe = at91_usart_spi_probe,
> +       .remove = at91_usart_spi_remove, };

Two lines at one. Split.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Andy Shevchenko]

> I will refer to above as (1) later on.

> The question is, why you didn't utilize what SPI core provides you?

Here I should have referred to (1).

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Andy Shevchenko]

> I will refer to above as (1) later on.

> The question is, why you didn't utilize what SPI core provides you?

Here I should have referred to (1).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Richard Genoud]

Hi,

On 11/05/2018 12:38, Radu Pirea wrote:
> This patch modifies the place where resources and device tree properties
> are searched.
> 
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
>  drivers/tty/serial/Kconfig        |  1 +
>  drivers/tty/serial/atmel_serial.c | 29 +++++++++++++++--------------
>  2 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 3682fd3e960c..25e55332f8b1 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -119,6 +119,7 @@ config SERIAL_ATMEL
>  	depends on ARCH_AT91 || COMPILE_TEST
>  	select SERIAL_CORE
>  	select SERIAL_MCTRL_GPIO if GPIOLIB
> +	select MFD_AT91_USART
>  	help
>  	  This enables the driver for the on-chip UARTs of the Atmel
>  	  AT91 processors.
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index df46a9e88c34..6b4494352853 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -193,8 +193,8 @@ static struct console atmel_console;
>  
>  #if defined(CONFIG_OF)
>  static const struct of_device_id atmel_serial_dt_ids[] = {
> -	{ .compatible = "atmel,at91rm9200-usart" },
> -	{ .compatible = "atmel,at91sam9260-usart" },
> +	{ .compatible = "atmel,at91rm9200-usart-serial" },
> +	{ .compatible = "atmel,at91sam9260-usart-serial" },
>  	{ /* sentinel */ }
>  };
>  #endif
> @@ -1631,7 +1631,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
>  static void atmel_init_property(struct atmel_uart_port *atmel_port,
>  				struct platform_device *pdev)
>  {
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.parent->of_node;
>  
>  	/* DMA/PDC usage specification */
>  	if (of_property_read_bool(np, "atmel,use-dma-rx")) {
> @@ -2223,7 +2223,8 @@ static const char *atmel_type(struct uart_port *port)
>  static void atmel_release_port(struct uart_port *port)
>  {
>  	struct platform_device *pdev = to_platform_device(port->dev);
> -	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> +	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
> +		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
I think it may be simpler with something like:
+	struct platform_device *mfd_pdev = to_platform_device(port->dev->parent);
+	int size = mfd_pdev->resource[0].end - mfd_pdev->resource[0].start + 1;

>  
>  	release_mem_region(port->mapbase, size);
>  
> @@ -2239,7 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
>  static int atmel_request_port(struct uart_port *port)
>  {
>  	struct platform_device *pdev = to_platform_device(port->dev);
> -	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> +	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
> +		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
>  
ditto

>  	if (!request_mem_region(port->mapbase, size, "atmel_serial"))
>  		return -EBUSY;
> @@ -2345,23 +2347,23 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
Here, we could also add:
+	struct device *mfd_dev = pdev->dev.parent;
+	struct platform_device *mfd_pdev = to_platform_device(mfd_dev);

>  	atmel_init_property(atmel_port, pdev);
>  	atmel_set_ops(port);
>  
> -	uart_get_rs485_mode(&pdev->dev, &port->rs485);
> +	uart_get_rs485_mode(pdev->dev.parent, &port->rs485);
...and use them here

>  
>  	port->iotype		= UPIO_MEM;
>  	port->flags		= UPF_BOOT_AUTOCONF | UPF_IOREMAP;
>  	port->ops		= &atmel_pops;
>  	port->fifosize		= 1;
>  	port->dev		= &pdev->dev;
> -	port->mapbase	= pdev->resource[0].start;
> -	port->irq	= pdev->resource[1].start;
> +	port->mapbase		= to_platform_device(pdev->dev.parent)->resource[0].start;
> +	port->irq		= to_platform_device(pdev->dev.parent)->resource[1].start;
and here
I think it would be easier to read.

>  	port->rs485_config	= atmel_config_rs485;
> -	port->membase	= NULL;
> +	port->membase		= NULL;
>  
>  	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
>  
>  	/* for console, the clock could already be configured */
>  	if (!atmel_port->clk) {
> -		atmel_port->clk = clk_get(&pdev->dev, "usart");
> +		atmel_port->clk = clk_get(pdev->dev.parent, "usart");
and here

>  		if (IS_ERR(atmel_port->clk)) {
>  			ret = PTR_ERR(atmel_port->clk);
>  			atmel_port->clk = NULL;
> @@ -2656,7 +2658,7 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
>  	atmel_port->rts_low = 0;
>  	atmel_port->rts_high = 0;
>  
> -	if (of_property_read_u32(pdev->dev.of_node,
> +	if (of_property_read_u32(pdev->dev.parent->of_node,
>  				 "atmel,fifo-size",
>  				 &atmel_port->fifo_size))
>  		return;
> @@ -2694,11 +2696,10 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
>  static int atmel_serial_probe(struct platform_device *pdev)
>  {
>  	struct atmel_uart_port *atmel_port;
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.parent->of_node;
>  	void *data;
>  	int ret = -ENODEV;
>  	bool rs485_enabled;
> -
I think this line feed wasn't so bad.

>  	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
>  
>  	ret = of_alias_get_id(np, "serial");
> @@ -2845,7 +2846,7 @@ static struct platform_driver atmel_serial_driver = {
>  	.suspend	= atmel_serial_suspend,
>  	.resume		= atmel_serial_resume,
>  	.driver		= {
> -		.name			= "atmel_usart",
> +		.name			= "atmel_usart_serial",
>  		.of_match_table		= of_match_ptr(atmel_serial_dt_ids),
>  	},
>  };
> 

After your patch, the DMA is not selected anymore:
atmel_usart_serial atmel_usart_serial.0.auto: TX channel not available, switch to pio
instead of:
atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers

And the kernel doesn't log anymore on the serial console, despite the loglevel=8
(after reverting this series, the kernel logs reappears on the serial console)

(tests done on sam9g35)

regards,
Richard

Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Richard Genoud]

Hi,

On 11/05/2018 12:38, Radu Pirea wrote:
> This patch modifies the place where resources and device tree properties
> are searched.
> 
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
>  drivers/tty/serial/Kconfig        |  1 +
>  drivers/tty/serial/atmel_serial.c | 29 +++++++++++++++--------------
>  2 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 3682fd3e960c..25e55332f8b1 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -119,6 +119,7 @@ config SERIAL_ATMEL
>  	depends on ARCH_AT91 || COMPILE_TEST
>  	select SERIAL_CORE
>  	select SERIAL_MCTRL_GPIO if GPIOLIB
> +	select MFD_AT91_USART
>  	help
>  	  This enables the driver for the on-chip UARTs of the Atmel
>  	  AT91 processors.
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index df46a9e88c34..6b4494352853 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -193,8 +193,8 @@ static struct console atmel_console;
>  
>  #if defined(CONFIG_OF)
>  static const struct of_device_id atmel_serial_dt_ids[] = {
> -	{ .compatible = "atmel,at91rm9200-usart" },
> -	{ .compatible = "atmel,at91sam9260-usart" },
> +	{ .compatible = "atmel,at91rm9200-usart-serial" },
> +	{ .compatible = "atmel,at91sam9260-usart-serial" },
>  	{ /* sentinel */ }
>  };
>  #endif
> @@ -1631,7 +1631,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
>  static void atmel_init_property(struct atmel_uart_port *atmel_port,
>  				struct platform_device *pdev)
>  {
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.parent->of_node;
>  
>  	/* DMA/PDC usage specification */
>  	if (of_property_read_bool(np, "atmel,use-dma-rx")) {
> @@ -2223,7 +2223,8 @@ static const char *atmel_type(struct uart_port *port)
>  static void atmel_release_port(struct uart_port *port)
>  {
>  	struct platform_device *pdev = to_platform_device(port->dev);
> -	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> +	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
> +		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
I think it may be simpler with something like:
+	struct platform_device *mfd_pdev = to_platform_device(port->dev->parent);
+	int size = mfd_pdev->resource[0].end - mfd_pdev->resource[0].start + 1;

>  
>  	release_mem_region(port->mapbase, size);
>  
> @@ -2239,7 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
>  static int atmel_request_port(struct uart_port *port)
>  {
>  	struct platform_device *pdev = to_platform_device(port->dev);
> -	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> +	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
> +		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
>  
ditto

>  	if (!request_mem_region(port->mapbase, size, "atmel_serial"))
>  		return -EBUSY;
> @@ -2345,23 +2347,23 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
Here, we could also add:
+	struct device *mfd_dev = pdev->dev.parent;
+	struct platform_device *mfd_pdev = to_platform_device(mfd_dev);

>  	atmel_init_property(atmel_port, pdev);
>  	atmel_set_ops(port);
>  
> -	uart_get_rs485_mode(&pdev->dev, &port->rs485);
> +	uart_get_rs485_mode(pdev->dev.parent, &port->rs485);
...and use them here

>  
>  	port->iotype		= UPIO_MEM;
>  	port->flags		= UPF_BOOT_AUTOCONF | UPF_IOREMAP;
>  	port->ops		= &atmel_pops;
>  	port->fifosize		= 1;
>  	port->dev		= &pdev->dev;
> -	port->mapbase	= pdev->resource[0].start;
> -	port->irq	= pdev->resource[1].start;
> +	port->mapbase		= to_platform_device(pdev->dev.parent)->resource[0].start;
> +	port->irq		= to_platform_device(pdev->dev.parent)->resource[1].start;
and here
I think it would be easier to read.

>  	port->rs485_config	= atmel_config_rs485;
> -	port->membase	= NULL;
> +	port->membase		= NULL;
>  
>  	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
>  
>  	/* for console, the clock could already be configured */
>  	if (!atmel_port->clk) {
> -		atmel_port->clk = clk_get(&pdev->dev, "usart");
> +		atmel_port->clk = clk_get(pdev->dev.parent, "usart");
and here

>  		if (IS_ERR(atmel_port->clk)) {
>  			ret = PTR_ERR(atmel_port->clk);
>  			atmel_port->clk = NULL;
> @@ -2656,7 +2658,7 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
>  	atmel_port->rts_low = 0;
>  	atmel_port->rts_high = 0;
>  
> -	if (of_property_read_u32(pdev->dev.of_node,
> +	if (of_property_read_u32(pdev->dev.parent->of_node,
>  				 "atmel,fifo-size",
>  				 &atmel_port->fifo_size))
>  		return;
> @@ -2694,11 +2696,10 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
>  static int atmel_serial_probe(struct platform_device *pdev)
>  {
>  	struct atmel_uart_port *atmel_port;
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.parent->of_node;
>  	void *data;
>  	int ret = -ENODEV;
>  	bool rs485_enabled;
> -
I think this line feed wasn't so bad.

>  	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
>  
>  	ret = of_alias_get_id(np, "serial");
> @@ -2845,7 +2846,7 @@ static struct platform_driver atmel_serial_driver = {
>  	.suspend	= atmel_serial_suspend,
>  	.resume		= atmel_serial_resume,
>  	.driver		= {
> -		.name			= "atmel_usart",
> +		.name			= "atmel_usart_serial",
>  		.of_match_table		= of_match_ptr(atmel_serial_dt_ids),
>  	},
>  };
> 

After your patch, the DMA is not selected anymore:
atmel_usart_serial atmel_usart_serial.0.auto: TX channel not available, switch to pio
instead of:
atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers

And the kernel doesn't log anymore on the serial console, despite the loglevel=8
(after reverting this series, the kernel logs reappears on the serial console)

(tests done on sam9g35)

regards,
Richard

[PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Richard Genoud]

Hi,

On 11/05/2018 12:38, Radu Pirea wrote:
> This patch modifies the place where resources and device tree properties
> are searched.
> 
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
>  drivers/tty/serial/Kconfig        |  1 +
>  drivers/tty/serial/atmel_serial.c | 29 +++++++++++++++--------------
>  2 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 3682fd3e960c..25e55332f8b1 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -119,6 +119,7 @@ config SERIAL_ATMEL
>  	depends on ARCH_AT91 || COMPILE_TEST
>  	select SERIAL_CORE
>  	select SERIAL_MCTRL_GPIO if GPIOLIB
> +	select MFD_AT91_USART
>  	help
>  	  This enables the driver for the on-chip UARTs of the Atmel
>  	  AT91 processors.
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index df46a9e88c34..6b4494352853 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -193,8 +193,8 @@ static struct console atmel_console;
>  
>  #if defined(CONFIG_OF)
>  static const struct of_device_id atmel_serial_dt_ids[] = {
> -	{ .compatible = "atmel,at91rm9200-usart" },
> -	{ .compatible = "atmel,at91sam9260-usart" },
> +	{ .compatible = "atmel,at91rm9200-usart-serial" },
> +	{ .compatible = "atmel,at91sam9260-usart-serial" },
>  	{ /* sentinel */ }
>  };
>  #endif
> @@ -1631,7 +1631,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
>  static void atmel_init_property(struct atmel_uart_port *atmel_port,
>  				struct platform_device *pdev)
>  {
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.parent->of_node;
>  
>  	/* DMA/PDC usage specification */
>  	if (of_property_read_bool(np, "atmel,use-dma-rx")) {
> @@ -2223,7 +2223,8 @@ static const char *atmel_type(struct uart_port *port)
>  static void atmel_release_port(struct uart_port *port)
>  {
>  	struct platform_device *pdev = to_platform_device(port->dev);
> -	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> +	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
> +		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
I think it may be simpler with something like:
+	struct platform_device *mfd_pdev = to_platform_device(port->dev->parent);
+	int size = mfd_pdev->resource[0].end - mfd_pdev->resource[0].start + 1;

>  
>  	release_mem_region(port->mapbase, size);
>  
> @@ -2239,7 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
>  static int atmel_request_port(struct uart_port *port)
>  {
>  	struct platform_device *pdev = to_platform_device(port->dev);
> -	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
> +	int size = to_platform_device(pdev->dev.parent)->resource[0].end -
> +		to_platform_device(pdev->dev.parent)->resource[0].start + 1;
>  
ditto

>  	if (!request_mem_region(port->mapbase, size, "atmel_serial"))
>  		return -EBUSY;
> @@ -2345,23 +2347,23 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
Here, we could also add:
+	struct device *mfd_dev = pdev->dev.parent;
+	struct platform_device *mfd_pdev = to_platform_device(mfd_dev);

>  	atmel_init_property(atmel_port, pdev);
>  	atmel_set_ops(port);
>  
> -	uart_get_rs485_mode(&pdev->dev, &port->rs485);
> +	uart_get_rs485_mode(pdev->dev.parent, &port->rs485);
...and use them here

>  
>  	port->iotype		= UPIO_MEM;
>  	port->flags		= UPF_BOOT_AUTOCONF | UPF_IOREMAP;
>  	port->ops		= &atmel_pops;
>  	port->fifosize		= 1;
>  	port->dev		= &pdev->dev;
> -	port->mapbase	= pdev->resource[0].start;
> -	port->irq	= pdev->resource[1].start;
> +	port->mapbase		= to_platform_device(pdev->dev.parent)->resource[0].start;
> +	port->irq		= to_platform_device(pdev->dev.parent)->resource[1].start;
and here
I think it would be easier to read.

>  	port->rs485_config	= atmel_config_rs485;
> -	port->membase	= NULL;
> +	port->membase		= NULL;
>  
>  	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
>  
>  	/* for console, the clock could already be configured */
>  	if (!atmel_port->clk) {
> -		atmel_port->clk = clk_get(&pdev->dev, "usart");
> +		atmel_port->clk = clk_get(pdev->dev.parent, "usart");
and here

>  		if (IS_ERR(atmel_port->clk)) {
>  			ret = PTR_ERR(atmel_port->clk);
>  			atmel_port->clk = NULL;
> @@ -2656,7 +2658,7 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
>  	atmel_port->rts_low = 0;
>  	atmel_port->rts_high = 0;
>  
> -	if (of_property_read_u32(pdev->dev.of_node,
> +	if (of_property_read_u32(pdev->dev.parent->of_node,
>  				 "atmel,fifo-size",
>  				 &atmel_port->fifo_size))
>  		return;
> @@ -2694,11 +2696,10 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
>  static int atmel_serial_probe(struct platform_device *pdev)
>  {
>  	struct atmel_uart_port *atmel_port;
> -	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *np = pdev->dev.parent->of_node;
>  	void *data;
>  	int ret = -ENODEV;
>  	bool rs485_enabled;
> -
I think this line feed wasn't so bad.

>  	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
>  
>  	ret = of_alias_get_id(np, "serial");
> @@ -2845,7 +2846,7 @@ static struct platform_driver atmel_serial_driver = {
>  	.suspend	= atmel_serial_suspend,
>  	.resume		= atmel_serial_resume,
>  	.driver		= {
> -		.name			= "atmel_usart",
> +		.name			= "atmel_usart_serial",
>  		.of_match_table		= of_match_ptr(atmel_serial_dt_ids),
>  	},
>  };
> 

After your patch, the DMA is not selected anymore:
atmel_usart_serial atmel_usart_serial.0.auto: TX channel not available, switch to pio
instead of:
atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers

And the kernel doesn't log anymore on the serial console, despite the loglevel=8
(after reverting this series, the kernel logs reappears on the serial console)

(tests done on sam9g35)

regards,
Richard

Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Andy Shevchenko]

On Mon, May 14, 2018 at 1:57 PM, Richard Genoud
<richard.genoud@gmail.com> wrote:
> On 11/05/2018 12:38, Radu Pirea wrote:
>> This patch modifies the place where resources and device tree properties
>> are searched.

> I think it may be simpler with something like:

> +       int size = mfd_pdev->resource[0].end - mfd_pdev->resource[0].start + 1;

Isn't resource_size() macro for this very purpose?

>> +     int size = to_platform_device(pdev->dev.parent)->resource[0].end -
>> +             to_platform_device(pdev->dev.parent)->resource[0].start + 1;
>>
> ditto

Ditto.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Andy Shevchenko]

On Mon, May 14, 2018 at 1:57 PM, Richard Genoud
<richard.genoud@gmail.com> wrote:
> On 11/05/2018 12:38, Radu Pirea wrote:
>> This patch modifies the place where resources and device tree properties
>> are searched.

> I think it may be simpler with something like:

> +       int size = mfd_pdev->resource[0].end - mfd_pdev->resource[0].start + 1;

Isn't resource_size() macro for this very purpose?

>> +     int size = to_platform_device(pdev->dev.parent)->resource[0].end -
>> +             to_platform_device(pdev->dev.parent)->resource[0].start + 1;
>>
> ditto

Ditto.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Andy Shevchenko]

First of all, do not remove mailing lists from Cc and people if you
are not sure they do not need your stuff.

On Mon, May 14, 2018 at 11:11 AM, Radu Pirea <radu.pirea@microchip.com> wrote:
> On Sun, 2018-05-13 at 16:33 +0300, Andy Shevchenko wrote:
>> On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip.com
>> > wrote:

>> > +static void at91_usart_spi_cleanup(struct spi_device *spi)
>> > +{
>> > +       struct at91_usart_spi_device *ausd = spi->controller_state;
>> > +
>> > +       if (!ausd)
>> > +               return;
>>
>> Is it even possible?
> Theoretically yes.

I would like to know real circumstances when it might happen.

>>
>> Anyway the code below will work fine even if it's the case.
>>
>> > +
>> > +       spi->controller_state = NULL;
>> > +       kfree(ausd);
>> > +}

>> The question is, why you didn't utilize what SPI core provides you?
> I tried, but it did not work the way I expected.

So, what is not going as expected in "SPI core takes care of CSs" case?
Did you use oscilloscope for that?

-- 
With Best Regards,
Andy Shevchenko

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Andy Shevchenko]

First of all, do not remove mailing lists from Cc and people if you
are not sure they do not need your stuff.

On Mon, May 14, 2018 at 11:11 AM, Radu Pirea <radu.pirea@microchip.com> wrote:
> On Sun, 2018-05-13 at 16:33 +0300, Andy Shevchenko wrote:
>> On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip.com
>> > wrote:

>> > +static void at91_usart_spi_cleanup(struct spi_device *spi)
>> > +{
>> > +       struct at91_usart_spi_device *ausd = spi->controller_state;
>> > +
>> > +       if (!ausd)
>> > +               return;
>>
>> Is it even possible?
> Theoretically yes.

I would like to know real circumstances when it might happen.

>>
>> Anyway the code below will work fine even if it's the case.
>>
>> > +
>> > +       spi->controller_state = NULL;
>> > +       kfree(ausd);
>> > +}

>> The question is, why you didn't utilize what SPI core provides you?
> I tried, but it did not work the way I expected.

So, what is not going as expected in "SPI core takes care of CSs" case?
Did you use oscilloscope for that?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Richard Genoud]

On 14/05/2018 18:56, Andy Shevchenko wrote:
> On Mon, May 14, 2018 at 1:57 PM, Richard Genoud
> <richard.genoud@gmail.com> wrote:
>> On 11/05/2018 12:38, Radu Pirea wrote:
>>> This patch modifies the place where resources and device tree properties
>>> are searched.
> 
>> I think it may be simpler with something like:
> 
>> +       int size = mfd_pdev->resource[0].end - mfd_pdev->resource[0].start + 1;
> 
> Isn't resource_size() macro for this very purpose?
Indeed.
+       int size = resource_size(mfd_pdev->resource);
would be even simpler !

> 
>>> +     int size = to_platform_device(pdev->dev.parent)->resource[0].end -
>>> +             to_platform_device(pdev->dev.parent)->resource[0].start + 1;
>>>
>> ditto
> 
> Ditto.
> 

Thanks !

Richard.

[PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Richard Genoud]

On 14/05/2018 18:56, Andy Shevchenko wrote:
> On Mon, May 14, 2018 at 1:57 PM, Richard Genoud
> <richard.genoud@gmail.com> wrote:
>> On 11/05/2018 12:38, Radu Pirea wrote:
>>> This patch modifies the place where resources and device tree properties
>>> are searched.
> 
>> I think it may be simpler with something like:
> 
>> +       int size = mfd_pdev->resource[0].end - mfd_pdev->resource[0].start + 1;
> 
> Isn't resource_size() macro for this very purpose?
Indeed.
+       int size = resource_size(mfd_pdev->resource);
would be even simpler !

> 
>>> +     int size = to_platform_device(pdev->dev.parent)->resource[0].end -
>>> +             to_platform_device(pdev->dev.parent)->resource[0].start + 1;
>>>
>> ditto
> 
> Ditto.
> 

Thanks !

Richard.

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

On Mon, 2018-05-14 at 20:38 +0300, Andy Shevchenko wrote:
> First of all, do not remove mailing lists from Cc and people if you
> are not sure they do not need your stuff.
> 
Sorry. My mistake. 
> On Mon, May 14, 2018 at 11:11 AM, Radu Pirea
> <radu.pirea@microchip.com> wrote:
> > On Sun, 2018-05-13 at 16:33 +0300, Andy Shevchenko wrote:
> > > On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip
> > > .com
> > > > wrote:
> > > > +static void at91_usart_spi_cleanup(struct spi_device *spi)
> > > > +{
> > > > +       struct at91_usart_spi_device *ausd = spi-
> > > > >controller_state;
> > > > +
> > > > +       if (!ausd)
> > > > +               return;
> > > 
> > > Is it even possible?
> > 
> > Theoretically yes.
> 
> I would like to know real circumstances when it might happen.
That check was used in debug stage of driver. I will remove.
> 
> > > 
> > > Anyway the code below will work fine even if it's the case.
> > > 
> > > > +
> > > > +       spi->controller_state = NULL;
> > > > +       kfree(ausd);
> > > > +}
> > > The question is, why you didn't utilize what SPI core provides
> > > you?
> > 
> > I tried, but it did not work the way I expected.
> 
> So, what is not going as expected in "SPI core takes care of CSs"
> case?
> Did you use oscilloscope for that?
Yes, I used and CSs was not asserted. Anyway, I will will try again.
> 

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

On Mon, 2018-05-14 at 20:38 +0300, Andy Shevchenko wrote:
> First of all, do not remove mailing lists from Cc and people if you
> are not sure they do not need your stuff.
> 
Sorry. My mistake. 
> On Mon, May 14, 2018 at 11:11 AM, Radu Pirea
> <radu.pirea@microchip.com> wrote:
> > On Sun, 2018-05-13 at 16:33 +0300, Andy Shevchenko wrote:
> > > On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip
> > > .com
> > > > wrote:
> > > > +static void at91_usart_spi_cleanup(struct spi_device *spi)
> > > > +{
> > > > +       struct at91_usart_spi_device *ausd = spi-
> > > > >controller_state;
> > > > +
> > > > +       if (!ausd)
> > > > +               return;
> > > 
> > > Is it even possible?
> > 
> > Theoretically yes.
> 
> I would like to know real circumstances when it might happen.
That check was used in debug stage of driver. I will remove.
> 
> > > 
> > > Anyway the code below will work fine even if it's the case.
> > > 
> > > > +
> > > > +       spi->controller_state = NULL;
> > > > +       kfree(ausd);
> > > > +}
> > > The question is, why you didn't utilize what SPI core provides
> > > you?
> > 
> > I tried, but it did not work the way I expected.
> 
> So, what is not going as expected in "SPI core takes care of CSs"
> case?
> Did you use oscilloscope for that?
Yes, I used and CSs was not asserted. Anyway, I will will try again.
> 

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

On Mon, 2018-05-14 at 20:38 +0300, Andy Shevchenko wrote:
> First of all, do not remove mailing lists from Cc and people if you
> are not sure they do not need your stuff.
> 
Sorry. My mistake. 
> On Mon, May 14, 2018 at 11:11 AM, Radu Pirea
> <radu.pirea@microchip.com> wrote:
> > On Sun, 2018-05-13 at 16:33 +0300, Andy Shevchenko wrote:
> > > On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip
> > > .com
> > > > wrote:
> > > > +static void at91_usart_spi_cleanup(struct spi_device *spi)
> > > > +{
> > > > +       struct at91_usart_spi_device *ausd = spi-
> > > > >controller_state;
> > > > +
> > > > +       if (!ausd)
> > > > +               return;
> > > 
> > > Is it even possible?
> > 
> > Theoretically yes.
> 
> I would like to know real circumstances when it might happen.
That check was used in debug stage of driver. I will remove.
> 
> > > 
> > > Anyway the code below will work fine even if it's the case.
> > > 
> > > > +
> > > > +       spi->controller_state = NULL;
> > > > +       kfree(ausd);
> > > > +}
> > > The question is, why you didn't utilize what SPI core provides
> > > you?
> > 
> > I tried, but it did not work the way I expected.
> 
> So, what is not going as expected in "SPI core takes care of CSs"
> case?
> Did you use oscilloscope for that?
Yes, I used and CSs was not asserted. Anyway, I will will try again.
> 

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

On Sun, 2018-05-13 at 16:33 +0300, Andy Shevchenko wrote:
> On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip.com
> > wrote:
> > This is the driver for at91-usart in spi mode. The USART IP can be
> > configured
> > to work in many modes and one of them is SPI.
> > +#include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> 
> Here is something wrong. You need to use latter one in new code.
> 
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_gpio.h>
> 
> Hmm... Do you need all of them?
> 
> > +static inline void at91_usart_spi_cs_activate(struct spi_device
> > *spi)
> > +{
> 
> ...
> > +       gpiod_set_value(ausd->npcs_pin, active);
> > +       aus->cs_active = true;
> > +}
> > +
> > +static inline void at91_usart_spi_cs_deactivate(struct spi_device
> > *spi)
> > +{
> 
> ...
> > +       gpiod_set_value(ausd->npcs_pin, !active);
> > +       aus->cs_active = false;
> > +}
> 
> ...
> > +       if (!ausd) {
> > +               if (gpio_is_valid(spi->cs_gpio)) {
> > +                       npcs_pin = gpio_to_desc(spi->cs_gpio);
> 
> ...
> > +               }
> 
> ...
> > +               gpiod_direction_output(npcs_pin, !(spi->mode &
> > SPI_CS_HIGH));
> > +
> > +               ausd->npcs_pin = npcs_pin;
> 
> ...
> > +       }
> 
> I will refer to above as (1) later on.
> 
> > +       dev_dbg(&spi->dev, "new message %p submitted for %s\n",
> > +               msg, dev_name(&spi->dev));
> 
> %p does make a very little sense.
> 
> > +       list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> > +               ret = at91_usart_spi_one_transfer(controller, msg,
> > xfer);
> > +               if (ret)
> > +                       goto msg_done;
> > +       }
> 
> Cant SPI core do this for your?
> 
> > +static void at91_usart_spi_cleanup(struct spi_device *spi)
> > +{
> > +       struct at91_usart_spi_device *ausd = spi->controller_state;
> > +
> > +       if (!ausd)
> > +               return;
> 
> Is it even possible?
> 
> Anyway the code below will work fine even if it's the case.
> 
> > +
> > +       spi->controller_state = NULL;
> > +       kfree(ausd);
> > +}
> > +static int at91_usart_spi_gpio_cs(struct platform_device *pdev)
> > +{
> > +       struct spi_controller *controller =
> > platform_get_drvdata(pdev);
> > +       struct device_node *np = controller->dev.parent->of_node;
> > +       struct gpio_desc *cs_gpio;
> > +       int nb;
> > +       int i;
> > +
> > +       if (!np)
> > +               return 0;
> > +
> > +       nb = of_gpio_named_count(np, "cs-gpios");
> > +       for (i = 0; i < nb; i++) {
> > +               cs_gpio = devm_gpiod_get_from_of_node(&pdev->dev,
> > +                                                     pdev-
> > >dev.parent->of_node,
> > +                                                     "cs-gpios",
> > +                                                     i,
> > GPIOD_OUT_HIGH,
> > +                                                     dev_name(&pde
> > v->dev));
> > +               if (IS_ERR(cs_gpio))
> > +                       return PTR_ERR(cs_gpio);
> > +       }
> > +
> > +       controller->num_chipselect = nb;
> > +
> > +       return 0;
> > +}
> 
> The question is, why you didn't utilize what SPI core provides you?
> 
> > +       spi_writel(aus, MR, US_MR_SPI_MASTER | US_MR_CHRL |
> > US_MR_CLKO |
> > +                       US_MR_WRDBT);
> > +       spi_writel(aus, CR, US_CR_RXDIS | US_CR_TXDIS | US_CR_RSTRX
> > |
> > +                       US_CR_RSTTX);
> 
> I didn't check over, but it seems like you might have duplication in
> these bitwise ORs. Consider to unify them into another (shorter)
> definitions and reuse all over the code.
> 
> > +       regs = platform_get_resource(to_platform_device(pdev-
> > >dev.parent),
> > +                                    IORESOURCE_MEM, 0);
> > +       if (!regs)
> > +               return -ENXIO;
> 
> Strange error code for getting MMIO resource. ENOMEM sounds better.
> 
> > +       dev_info(&pdev->dev,
> > +                "Atmel USART SPI Controller version 0x%x at
> > 0x%08lx (irq %d)\n",
> > +                spi_readl(aus, VERSION),
> > +                (unsigned long)regs->start, irq);
> 
> If you do explicit casting when printing something you are doing
> wrong.
> Please use %pR or %pr in this case.
> 
> > +static struct platform_driver at91_usart_spi_driver = {
> > +       .driver = {
> > +               .name = "at91_usart_spi",
> > +               .of_match_table =
> > of_match_ptr(at91_usart_spi_dt_ids),
> 
> Can it work as pure platform driver? If no, of_match_ptr() is
> redundant.

This driver can not work as pure platform driver, but I the way I used
to probe it from MFD(by compatbile string).
Do you know another way?

> 
> > +       },
> > +       .probe = at91_usart_spi_probe,
> > +       .remove = at91_usart_spi_remove, };
> 
> Two lines at one. Split.
> 

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

On Sun, 2018-05-13 at 16:33 +0300, Andy Shevchenko wrote:
> On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip.com
> > wrote:
> > This is the driver for at91-usart in spi mode. The USART IP can be
> > configured
> > to work in many modes and one of them is SPI.
> > +#include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> 
> Here is something wrong. You need to use latter one in new code.
> 
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_gpio.h>
> 
> Hmm... Do you need all of them?
> 
> > +static inline void at91_usart_spi_cs_activate(struct spi_device
> > *spi)
> > +{
> 
> ...
> > +       gpiod_set_value(ausd->npcs_pin, active);
> > +       aus->cs_active = true;
> > +}
> > +
> > +static inline void at91_usart_spi_cs_deactivate(struct spi_device
> > *spi)
> > +{
> 
> ...
> > +       gpiod_set_value(ausd->npcs_pin, !active);
> > +       aus->cs_active = false;
> > +}
> 
> ...
> > +       if (!ausd) {
> > +               if (gpio_is_valid(spi->cs_gpio)) {
> > +                       npcs_pin = gpio_to_desc(spi->cs_gpio);
> 
> ...
> > +               }
> 
> ...
> > +               gpiod_direction_output(npcs_pin, !(spi->mode &
> > SPI_CS_HIGH));
> > +
> > +               ausd->npcs_pin = npcs_pin;
> 
> ...
> > +       }
> 
> I will refer to above as (1) later on.
> 
> > +       dev_dbg(&spi->dev, "new message %p submitted for %s\n",
> > +               msg, dev_name(&spi->dev));
> 
> %p does make a very little sense.
> 
> > +       list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> > +               ret = at91_usart_spi_one_transfer(controller, msg,
> > xfer);
> > +               if (ret)
> > +                       goto msg_done;
> > +       }
> 
> Cant SPI core do this for your?
> 
> > +static void at91_usart_spi_cleanup(struct spi_device *spi)
> > +{
> > +       struct at91_usart_spi_device *ausd = spi->controller_state;
> > +
> > +       if (!ausd)
> > +               return;
> 
> Is it even possible?
> 
> Anyway the code below will work fine even if it's the case.
> 
> > +
> > +       spi->controller_state = NULL;
> > +       kfree(ausd);
> > +}
> > +static int at91_usart_spi_gpio_cs(struct platform_device *pdev)
> > +{
> > +       struct spi_controller *controller =
> > platform_get_drvdata(pdev);
> > +       struct device_node *np = controller->dev.parent->of_node;
> > +       struct gpio_desc *cs_gpio;
> > +       int nb;
> > +       int i;
> > +
> > +       if (!np)
> > +               return 0;
> > +
> > +       nb = of_gpio_named_count(np, "cs-gpios");
> > +       for (i = 0; i < nb; i++) {
> > +               cs_gpio = devm_gpiod_get_from_of_node(&pdev->dev,
> > +                                                     pdev-
> > >dev.parent->of_node,
> > +                                                     "cs-gpios",
> > +                                                     i,
> > GPIOD_OUT_HIGH,
> > +                                                     dev_name(&pde
> > v->dev));
> > +               if (IS_ERR(cs_gpio))
> > +                       return PTR_ERR(cs_gpio);
> > +       }
> > +
> > +       controller->num_chipselect = nb;
> > +
> > +       return 0;
> > +}
> 
> The question is, why you didn't utilize what SPI core provides you?
> 
> > +       spi_writel(aus, MR, US_MR_SPI_MASTER | US_MR_CHRL |
> > US_MR_CLKO |
> > +                       US_MR_WRDBT);
> > +       spi_writel(aus, CR, US_CR_RXDIS | US_CR_TXDIS | US_CR_RSTRX
> > |
> > +                       US_CR_RSTTX);
> 
> I didn't check over, but it seems like you might have duplication in
> these bitwise ORs. Consider to unify them into another (shorter)
> definitions and reuse all over the code.
> 
> > +       regs = platform_get_resource(to_platform_device(pdev-
> > >dev.parent),
> > +                                    IORESOURCE_MEM, 0);
> > +       if (!regs)
> > +               return -ENXIO;
> 
> Strange error code for getting MMIO resource. ENOMEM sounds better.
> 
> > +       dev_info(&pdev->dev,
> > +                "Atmel USART SPI Controller version 0x%x at
> > 0x%08lx (irq %d)\n",
> > +                spi_readl(aus, VERSION),
> > +                (unsigned long)regs->start, irq);
> 
> If you do explicit casting when printing something you are doing
> wrong.
> Please use %pR or %pr in this case.
> 
> > +static struct platform_driver at91_usart_spi_driver = {
> > +       .driver = {
> > +               .name = "at91_usart_spi",
> > +               .of_match_table =
> > of_match_ptr(at91_usart_spi_dt_ids),
> 
> Can it work as pure platform driver? If no, of_match_ptr() is
> redundant.

This driver can not work as pure platform driver, but I the way I used
to probe it from MFD(by compatbile string).
Do you know another way?

> 
> > +       },
> > +       .probe = at91_usart_spi_probe,
> > +       .remove = at91_usart_spi_remove, };
> 
> Two lines at one. Split.
> 

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

On Sun, 2018-05-13 at 16:33 +0300, Andy Shevchenko wrote:
> On Fri, May 11, 2018 at 1:38 PM, Radu Pirea <radu.pirea@microchip.com
> > wrote:
> > This is the driver for at91-usart in spi mode. The USART IP can be
> > configured
> > to work in many modes and one of them is SPI.
> > +#include <linux/gpio.h>
> > +#include <linux/gpio/consumer.h>
> 
> Here is something wrong. You need to use latter one in new code.
> 
> > +#include <linux/interrupt.h>
> > +#include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_gpio.h>
> 
> Hmm... Do you need all of them?
> 
> > +static inline void at91_usart_spi_cs_activate(struct spi_device
> > *spi)
> > +{
> 
> ...
> > +       gpiod_set_value(ausd->npcs_pin, active);
> > +       aus->cs_active = true;
> > +}
> > +
> > +static inline void at91_usart_spi_cs_deactivate(struct spi_device
> > *spi)
> > +{
> 
> ...
> > +       gpiod_set_value(ausd->npcs_pin, !active);
> > +       aus->cs_active = false;
> > +}
> 
> ...
> > +       if (!ausd) {
> > +               if (gpio_is_valid(spi->cs_gpio)) {
> > +                       npcs_pin = gpio_to_desc(spi->cs_gpio);
> 
> ...
> > +               }
> 
> ...
> > +               gpiod_direction_output(npcs_pin, !(spi->mode &
> > SPI_CS_HIGH));
> > +
> > +               ausd->npcs_pin = npcs_pin;
> 
> ...
> > +       }
> 
> I will refer to above as (1) later on.
> 
> > +       dev_dbg(&spi->dev, "new message %p submitted for %s\n",
> > +               msg, dev_name(&spi->dev));
> 
> %p does make a very little sense.
> 
> > +       list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> > +               ret = at91_usart_spi_one_transfer(controller, msg,
> > xfer);
> > +               if (ret)
> > +                       goto msg_done;
> > +       }
> 
> Cant SPI core do this for your?
> 
> > +static void at91_usart_spi_cleanup(struct spi_device *spi)
> > +{
> > +       struct at91_usart_spi_device *ausd = spi->controller_state;
> > +
> > +       if (!ausd)
> > +               return;
> 
> Is it even possible?
> 
> Anyway the code below will work fine even if it's the case.
> 
> > +
> > +       spi->controller_state = NULL;
> > +       kfree(ausd);
> > +}
> > +static int at91_usart_spi_gpio_cs(struct platform_device *pdev)
> > +{
> > +       struct spi_controller *controller =
> > platform_get_drvdata(pdev);
> > +       struct device_node *np = controller->dev.parent->of_node;
> > +       struct gpio_desc *cs_gpio;
> > +       int nb;
> > +       int i;
> > +
> > +       if (!np)
> > +               return 0;
> > +
> > +       nb = of_gpio_named_count(np, "cs-gpios");
> > +       for (i = 0; i < nb; i++) {
> > +               cs_gpio = devm_gpiod_get_from_of_node(&pdev->dev,
> > +                                                     pdev-
> > >dev.parent->of_node,
> > +                                                     "cs-gpios",
> > +                                                     i,
> > GPIOD_OUT_HIGH,
> > +                                                     dev_name(&pde
> > v->dev));
> > +               if (IS_ERR(cs_gpio))
> > +                       return PTR_ERR(cs_gpio);
> > +       }
> > +
> > +       controller->num_chipselect = nb;
> > +
> > +       return 0;
> > +}
> 
> The question is, why you didn't utilize what SPI core provides you?
> 
> > +       spi_writel(aus, MR, US_MR_SPI_MASTER | US_MR_CHRL |
> > US_MR_CLKO |
> > +                       US_MR_WRDBT);
> > +       spi_writel(aus, CR, US_CR_RXDIS | US_CR_TXDIS | US_CR_RSTRX
> > |
> > +                       US_CR_RSTTX);
> 
> I didn't check over, but it seems like you might have duplication in
> these bitwise ORs. Consider to unify them into another (shorter)
> definitions and reuse all over the code.
> 
> > +       regs = platform_get_resource(to_platform_device(pdev-
> > >dev.parent),
> > +                                    IORESOURCE_MEM, 0);
> > +       if (!regs)
> > +               return -ENXIO;
> 
> Strange error code for getting MMIO resource. ENOMEM sounds better.
> 
> > +       dev_info(&pdev->dev,
> > +                "Atmel USART SPI Controller version 0x%x at
> > 0x%08lx (irq %d)\n",
> > +                spi_readl(aus, VERSION),
> > +                (unsigned long)regs->start, irq);
> 
> If you do explicit casting when printing something you are doing
> wrong.
> Please use %pR or %pr in this case.
> 
> > +static struct platform_driver at91_usart_spi_driver = {
> > +       .driver = {
> > +               .name = "at91_usart_spi",
> > +               .of_match_table =
> > of_match_ptr(at91_usart_spi_dt_ids),
> 
> Can it work as pure platform driver? If no, of_match_ptr() is
> redundant.

This driver can not work as pure platform driver, but I the way I used
to probe it from MFD(by compatbile string).
Do you know another way?

> 
> > +       },
> > +       .probe = at91_usart_spi_probe,
> > +       .remove = at91_usart_spi_remove, };
> 
> Two lines at one. Split.
> 

Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Radu Pirea]

[-- Attachment #1: Type: text/plain, Size: 6813 bytes --]

On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote:
> Hi,
> 
> On 11/05/2018 12:38, Radu Pirea wrote:
> > This patch modifies the place where resources and device tree
> > properties
> > are searched.
> > 
> > Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> > ---
> >  drivers/tty/serial/Kconfig        |  1 +
> >  drivers/tty/serial/atmel_serial.c | 29 +++++++++++++++----------
> > ----
> >  2 files changed, 16 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/Kconfig
> > b/drivers/tty/serial/Kconfig
> > index 3682fd3e960c..25e55332f8b1 100644
> > --- a/drivers/tty/serial/Kconfig
> > +++ b/drivers/tty/serial/Kconfig
> > @@ -119,6 +119,7 @@ config SERIAL_ATMEL
> >  	depends on ARCH_AT91 || COMPILE_TEST
> >  	select SERIAL_CORE
> >  	select SERIAL_MCTRL_GPIO if GPIOLIB
> > +	select MFD_AT91_USART
> >  	help
> >  	  This enables the driver for the on-chip UARTs of the
> > Atmel
> >  	  AT91 processors.
> > diff --git a/drivers/tty/serial/atmel_serial.c
> > b/drivers/tty/serial/atmel_serial.c
> > index df46a9e88c34..6b4494352853 100644
> > --- a/drivers/tty/serial/atmel_serial.c
> > +++ b/drivers/tty/serial/atmel_serial.c
> > @@ -193,8 +193,8 @@ static struct console atmel_console;
> >  
> >  #if defined(CONFIG_OF)
> >  static const struct of_device_id atmel_serial_dt_ids[] = {
> > -	{ .compatible = "atmel,at91rm9200-usart" },
> > -	{ .compatible = "atmel,at91sam9260-usart" },
> > +	{ .compatible = "atmel,at91rm9200-usart-serial" },
> > +	{ .compatible = "atmel,at91sam9260-usart-serial" },
> >  	{ /* sentinel */ }
> >  };
> >  #endif
> > @@ -1631,7 +1631,7 @@ static void atmel_tasklet_tx_func(unsigned
> > long data)
> >  static void atmel_init_property(struct atmel_uart_port
> > *atmel_port,
> >  				struct platform_device *pdev)
> >  {
> > -	struct device_node *np = pdev->dev.of_node;
> > +	struct device_node *np = pdev->dev.parent->of_node;
> >  
> >  	/* DMA/PDC usage specification */
> >  	if (of_property_read_bool(np, "atmel,use-dma-rx")) {
> > @@ -2223,7 +2223,8 @@ static const char *atmel_type(struct
> > uart_port *port)
> >  static void atmel_release_port(struct uart_port *port)
> >  {
> >  	struct platform_device *pdev = to_platform_device(port-
> > >dev);
> > -	int size = pdev->resource[0].end - pdev->resource[0].start 
> > + 1;
> > +	int size = to_platform_device(pdev->dev.parent)-
> > >resource[0].end -
> > +		to_platform_device(pdev->dev.parent)-
> > >resource[0].start + 1;
> 
> I think it may be simpler with something like:
> +	struct platform_device *mfd_pdev = to_platform_device(port-
> >dev->parent);
> +	int size = mfd_pdev->resource[0].end - mfd_pdev-
> >resource[0].start + 1;
> 
> >  
> >  	release_mem_region(port->mapbase, size);
> >  
> > @@ -2239,7 +2240,8 @@ static void atmel_release_port(struct
> > uart_port *port)
> >  static int atmel_request_port(struct uart_port *port)
> >  {
> >  	struct platform_device *pdev = to_platform_device(port-
> > >dev);
> > -	int size = pdev->resource[0].end - pdev->resource[0].start 
> > + 1;
> > +	int size = to_platform_device(pdev->dev.parent)-
> > >resource[0].end -
> > +		to_platform_device(pdev->dev.parent)-
> > >resource[0].start + 1;
> >  
> 
> ditto
> 
> >  	if (!request_mem_region(port->mapbase, size,
> > "atmel_serial"))
> >  		return -EBUSY;
> > @@ -2345,23 +2347,23 @@ static int atmel_init_port(struct
> > atmel_uart_port *atmel_port,
> 
> Here, we could also add:
> +	struct device *mfd_dev = pdev->dev.parent;
> +	struct platform_device *mfd_pdev =
> to_platform_device(mfd_dev);
> 
> >  	atmel_init_property(atmel_port, pdev);
> >  	atmel_set_ops(port);
> >  
> > -	uart_get_rs485_mode(&pdev->dev, &port->rs485);
> > +	uart_get_rs485_mode(pdev->dev.parent, &port->rs485);
> 
> ...and use them here
> 
> >  
> >  	port->iotype		= UPIO_MEM;
> >  	port->flags		= UPF_BOOT_AUTOCONF |
> > UPF_IOREMAP;
> >  	port->ops		= &atmel_pops;
> >  	port->fifosize		= 1;
> >  	port->dev		= &pdev->dev;
> > -	port->mapbase	= pdev->resource[0].start;
> > -	port->irq	= pdev->resource[1].start;
> > +	port->mapbase		= to_platform_device(pdev-
> > >dev.parent)->resource[0].start;
> > +	port->irq		= to_platform_device(pdev-
> > >dev.parent)->resource[1].start;
> 
> and here
> I think it would be easier to read.
> 
> >  	port->rs485_config	= atmel_config_rs485;
> > -	port->membase	= NULL;
> > +	port->membase		= NULL;
> >  
> >  	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port-
> > >rx_ring));
> >  
> >  	/* for console, the clock could already be configured */
> >  	if (!atmel_port->clk) {
> > -		atmel_port->clk = clk_get(&pdev->dev, "usart");
> > +		atmel_port->clk = clk_get(pdev->dev.parent,
> > "usart");
> 
> and here
> 
> >  		if (IS_ERR(atmel_port->clk)) {
> >  			ret = PTR_ERR(atmel_port->clk);
> >  			atmel_port->clk = NULL;
> > @@ -2656,7 +2658,7 @@ static void atmel_serial_probe_fifos(struct
> > atmel_uart_port *atmel_port,
> >  	atmel_port->rts_low = 0;
> >  	atmel_port->rts_high = 0;
> >  
> > -	if (of_property_read_u32(pdev->dev.of_node,
> > +	if (of_property_read_u32(pdev->dev.parent->of_node,
> >  				 "atmel,fifo-size",
> >  				 &atmel_port->fifo_size))
> >  		return;
> > @@ -2694,11 +2696,10 @@ static void atmel_serial_probe_fifos(struct
> > atmel_uart_port *atmel_port,
> >  static int atmel_serial_probe(struct platform_device *pdev)
> >  {
> >  	struct atmel_uart_port *atmel_port;
> > -	struct device_node *np = pdev->dev.of_node;
> > +	struct device_node *np = pdev->dev.parent->of_node;
> >  	void *data;
> >  	int ret = -ENODEV;
> >  	bool rs485_enabled;
> > -
> 
> I think this line feed wasn't so bad.
> 
> >  	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE &
> > (ATMEL_SERIAL_RINGSIZE - 1));
> >  
> >  	ret = of_alias_get_id(np, "serial");
> > @@ -2845,7 +2846,7 @@ static struct platform_driver
> > atmel_serial_driver = {
> >  	.suspend	= atmel_serial_suspend,
> >  	.resume		= atmel_serial_resume,
> >  	.driver		= {
> > -		.name			= "atmel_usart",
> > +		.name			=
> > "atmel_usart_serial",
> >  		.of_match_table		=
> > of_match_ptr(atmel_serial_dt_ids),
> >  	},
> >  };
> > 
> 
> After your patch, the DMA is not selected anymore:
> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not
> available, switch to pio
> instead of:
> atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers
> 
Fixed.
> And the kernel doesn't log anymore on the serial console, despite the
> loglevel=8
> (after reverting this series, the kernel logs reappears on the serial
> console)
> 
Which serial are you using as console? 
> (tests done on sam9g35)
> 
I will consider the rest of suggestions. 
> regards,
> Richard
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Radu Pirea]

[-- Attachment #1: Type: text/plain, Size: 6813 bytes --]

On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote:
> Hi,
> 
> On 11/05/2018 12:38, Radu Pirea wrote:
> > This patch modifies the place where resources and device tree
> > properties
> > are searched.
> > 
> > Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> > ---
> >  drivers/tty/serial/Kconfig        |  1 +
> >  drivers/tty/serial/atmel_serial.c | 29 +++++++++++++++----------
> > ----
> >  2 files changed, 16 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/Kconfig
> > b/drivers/tty/serial/Kconfig
> > index 3682fd3e960c..25e55332f8b1 100644
> > --- a/drivers/tty/serial/Kconfig
> > +++ b/drivers/tty/serial/Kconfig
> > @@ -119,6 +119,7 @@ config SERIAL_ATMEL
> >  	depends on ARCH_AT91 || COMPILE_TEST
> >  	select SERIAL_CORE
> >  	select SERIAL_MCTRL_GPIO if GPIOLIB
> > +	select MFD_AT91_USART
> >  	help
> >  	  This enables the driver for the on-chip UARTs of the
> > Atmel
> >  	  AT91 processors.
> > diff --git a/drivers/tty/serial/atmel_serial.c
> > b/drivers/tty/serial/atmel_serial.c
> > index df46a9e88c34..6b4494352853 100644
> > --- a/drivers/tty/serial/atmel_serial.c
> > +++ b/drivers/tty/serial/atmel_serial.c
> > @@ -193,8 +193,8 @@ static struct console atmel_console;
> >  
> >  #if defined(CONFIG_OF)
> >  static const struct of_device_id atmel_serial_dt_ids[] = {
> > -	{ .compatible = "atmel,at91rm9200-usart" },
> > -	{ .compatible = "atmel,at91sam9260-usart" },
> > +	{ .compatible = "atmel,at91rm9200-usart-serial" },
> > +	{ .compatible = "atmel,at91sam9260-usart-serial" },
> >  	{ /* sentinel */ }
> >  };
> >  #endif
> > @@ -1631,7 +1631,7 @@ static void atmel_tasklet_tx_func(unsigned
> > long data)
> >  static void atmel_init_property(struct atmel_uart_port
> > *atmel_port,
> >  				struct platform_device *pdev)
> >  {
> > -	struct device_node *np = pdev->dev.of_node;
> > +	struct device_node *np = pdev->dev.parent->of_node;
> >  
> >  	/* DMA/PDC usage specification */
> >  	if (of_property_read_bool(np, "atmel,use-dma-rx")) {
> > @@ -2223,7 +2223,8 @@ static const char *atmel_type(struct
> > uart_port *port)
> >  static void atmel_release_port(struct uart_port *port)
> >  {
> >  	struct platform_device *pdev = to_platform_device(port-
> > >dev);
> > -	int size = pdev->resource[0].end - pdev->resource[0].start 
> > + 1;
> > +	int size = to_platform_device(pdev->dev.parent)-
> > >resource[0].end -
> > +		to_platform_device(pdev->dev.parent)-
> > >resource[0].start + 1;
> 
> I think it may be simpler with something like:
> +	struct platform_device *mfd_pdev = to_platform_device(port-
> >dev->parent);
> +	int size = mfd_pdev->resource[0].end - mfd_pdev-
> >resource[0].start + 1;
> 
> >  
> >  	release_mem_region(port->mapbase, size);
> >  
> > @@ -2239,7 +2240,8 @@ static void atmel_release_port(struct
> > uart_port *port)
> >  static int atmel_request_port(struct uart_port *port)
> >  {
> >  	struct platform_device *pdev = to_platform_device(port-
> > >dev);
> > -	int size = pdev->resource[0].end - pdev->resource[0].start 
> > + 1;
> > +	int size = to_platform_device(pdev->dev.parent)-
> > >resource[0].end -
> > +		to_platform_device(pdev->dev.parent)-
> > >resource[0].start + 1;
> >  
> 
> ditto
> 
> >  	if (!request_mem_region(port->mapbase, size,
> > "atmel_serial"))
> >  		return -EBUSY;
> > @@ -2345,23 +2347,23 @@ static int atmel_init_port(struct
> > atmel_uart_port *atmel_port,
> 
> Here, we could also add:
> +	struct device *mfd_dev = pdev->dev.parent;
> +	struct platform_device *mfd_pdev =
> to_platform_device(mfd_dev);
> 
> >  	atmel_init_property(atmel_port, pdev);
> >  	atmel_set_ops(port);
> >  
> > -	uart_get_rs485_mode(&pdev->dev, &port->rs485);
> > +	uart_get_rs485_mode(pdev->dev.parent, &port->rs485);
> 
> ...and use them here
> 
> >  
> >  	port->iotype		= UPIO_MEM;
> >  	port->flags		= UPF_BOOT_AUTOCONF |
> > UPF_IOREMAP;
> >  	port->ops		= &atmel_pops;
> >  	port->fifosize		= 1;
> >  	port->dev		= &pdev->dev;
> > -	port->mapbase	= pdev->resource[0].start;
> > -	port->irq	= pdev->resource[1].start;
> > +	port->mapbase		= to_platform_device(pdev-
> > >dev.parent)->resource[0].start;
> > +	port->irq		= to_platform_device(pdev-
> > >dev.parent)->resource[1].start;
> 
> and here
> I think it would be easier to read.
> 
> >  	port->rs485_config	= atmel_config_rs485;
> > -	port->membase	= NULL;
> > +	port->membase		= NULL;
> >  
> >  	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port-
> > >rx_ring));
> >  
> >  	/* for console, the clock could already be configured */
> >  	if (!atmel_port->clk) {
> > -		atmel_port->clk = clk_get(&pdev->dev, "usart");
> > +		atmel_port->clk = clk_get(pdev->dev.parent,
> > "usart");
> 
> and here
> 
> >  		if (IS_ERR(atmel_port->clk)) {
> >  			ret = PTR_ERR(atmel_port->clk);
> >  			atmel_port->clk = NULL;
> > @@ -2656,7 +2658,7 @@ static void atmel_serial_probe_fifos(struct
> > atmel_uart_port *atmel_port,
> >  	atmel_port->rts_low = 0;
> >  	atmel_port->rts_high = 0;
> >  
> > -	if (of_property_read_u32(pdev->dev.of_node,
> > +	if (of_property_read_u32(pdev->dev.parent->of_node,
> >  				 "atmel,fifo-size",
> >  				 &atmel_port->fifo_size))
> >  		return;
> > @@ -2694,11 +2696,10 @@ static void atmel_serial_probe_fifos(struct
> > atmel_uart_port *atmel_port,
> >  static int atmel_serial_probe(struct platform_device *pdev)
> >  {
> >  	struct atmel_uart_port *atmel_port;
> > -	struct device_node *np = pdev->dev.of_node;
> > +	struct device_node *np = pdev->dev.parent->of_node;
> >  	void *data;
> >  	int ret = -ENODEV;
> >  	bool rs485_enabled;
> > -
> 
> I think this line feed wasn't so bad.
> 
> >  	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE &
> > (ATMEL_SERIAL_RINGSIZE - 1));
> >  
> >  	ret = of_alias_get_id(np, "serial");
> > @@ -2845,7 +2846,7 @@ static struct platform_driver
> > atmel_serial_driver = {
> >  	.suspend	= atmel_serial_suspend,
> >  	.resume		= atmel_serial_resume,
> >  	.driver		= {
> > -		.name			= "atmel_usart",
> > +		.name			=
> > "atmel_usart_serial",
> >  		.of_match_table		=
> > of_match_ptr(atmel_serial_dt_ids),
> >  	},
> >  };
> > 
> 
> After your patch, the DMA is not selected anymore:
> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not
> available, switch to pio
> instead of:
> atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers
> 
Fixed.
> And the kernel doesn't log anymore on the serial console, despite the
> loglevel=8
> (after reverting this series, the kernel logs reappears on the serial
> console)
> 
Which serial are you using as console? 
> (tests done on sam9g35)
> 
I will consider the rest of suggestions. 
> regards,
> Richard
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Radu Pirea]

On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote:
> Hi,
> 
> On 11/05/2018 12:38, Radu Pirea wrote:
> > This patch modifies the place where resources and device tree
> > properties
> > are searched.
> > 
> > Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> > ---
> >  drivers/tty/serial/Kconfig        |  1 +
> >  drivers/tty/serial/atmel_serial.c | 29 +++++++++++++++----------
> > ----
> >  2 files changed, 16 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/Kconfig
> > b/drivers/tty/serial/Kconfig
> > index 3682fd3e960c..25e55332f8b1 100644
> > --- a/drivers/tty/serial/Kconfig
> > +++ b/drivers/tty/serial/Kconfig
> > @@ -119,6 +119,7 @@ config SERIAL_ATMEL
> >  	depends on ARCH_AT91 || COMPILE_TEST
> >  	select SERIAL_CORE
> >  	select SERIAL_MCTRL_GPIO if GPIOLIB
> > +	select MFD_AT91_USART
> >  	help
> >  	  This enables the driver for the on-chip UARTs of the
> > Atmel
> >  	  AT91 processors.
> > diff --git a/drivers/tty/serial/atmel_serial.c
> > b/drivers/tty/serial/atmel_serial.c
> > index df46a9e88c34..6b4494352853 100644
> > --- a/drivers/tty/serial/atmel_serial.c
> > +++ b/drivers/tty/serial/atmel_serial.c
> > @@ -193,8 +193,8 @@ static struct console atmel_console;
> >  
> >  #if defined(CONFIG_OF)
> >  static const struct of_device_id atmel_serial_dt_ids[] = {
> > -	{ .compatible = "atmel,at91rm9200-usart" },
> > -	{ .compatible = "atmel,at91sam9260-usart" },
> > +	{ .compatible = "atmel,at91rm9200-usart-serial" },
> > +	{ .compatible = "atmel,at91sam9260-usart-serial" },
> >  	{ /* sentinel */ }
> >  };
> >  #endif
> > @@ -1631,7 +1631,7 @@ static void atmel_tasklet_tx_func(unsigned
> > long data)
> >  static void atmel_init_property(struct atmel_uart_port
> > *atmel_port,
> >  				struct platform_device *pdev)
> >  {
> > -	struct device_node *np = pdev->dev.of_node;
> > +	struct device_node *np = pdev->dev.parent->of_node;
> >  
> >  	/* DMA/PDC usage specification */
> >  	if (of_property_read_bool(np, "atmel,use-dma-rx")) {
> > @@ -2223,7 +2223,8 @@ static const char *atmel_type(struct
> > uart_port *port)
> >  static void atmel_release_port(struct uart_port *port)
> >  {
> >  	struct platform_device *pdev = to_platform_device(port-
> > >dev);
> > -	int size = pdev->resource[0].end - pdev->resource[0].start 
> > + 1;
> > +	int size = to_platform_device(pdev->dev.parent)-
> > >resource[0].end -
> > +		to_platform_device(pdev->dev.parent)-
> > >resource[0].start + 1;
> 
> I think it may be simpler with something like:
> +	struct platform_device *mfd_pdev = to_platform_device(port-
> >dev->parent);
> +	int size = mfd_pdev->resource[0].end - mfd_pdev-
> >resource[0].start + 1;
> 
> >  
> >  	release_mem_region(port->mapbase, size);
> >  
> > @@ -2239,7 +2240,8 @@ static void atmel_release_port(struct
> > uart_port *port)
> >  static int atmel_request_port(struct uart_port *port)
> >  {
> >  	struct platform_device *pdev = to_platform_device(port-
> > >dev);
> > -	int size = pdev->resource[0].end - pdev->resource[0].start 
> > + 1;
> > +	int size = to_platform_device(pdev->dev.parent)-
> > >resource[0].end -
> > +		to_platform_device(pdev->dev.parent)-
> > >resource[0].start + 1;
> >  
> 
> ditto
> 
> >  	if (!request_mem_region(port->mapbase, size,
> > "atmel_serial"))
> >  		return -EBUSY;
> > @@ -2345,23 +2347,23 @@ static int atmel_init_port(struct
> > atmel_uart_port *atmel_port,
> 
> Here, we could also add:
> +	struct device *mfd_dev = pdev->dev.parent;
> +	struct platform_device *mfd_pdev =
> to_platform_device(mfd_dev);
> 
> >  	atmel_init_property(atmel_port, pdev);
> >  	atmel_set_ops(port);
> >  
> > -	uart_get_rs485_mode(&pdev->dev, &port->rs485);
> > +	uart_get_rs485_mode(pdev->dev.parent, &port->rs485);
> 
> ...and use them here
> 
> >  
> >  	port->iotype		= UPIO_MEM;
> >  	port->flags		= UPF_BOOT_AUTOCONF |
> > UPF_IOREMAP;
> >  	port->ops		= &atmel_pops;
> >  	port->fifosize		= 1;
> >  	port->dev		= &pdev->dev;
> > -	port->mapbase	= pdev->resource[0].start;
> > -	port->irq	= pdev->resource[1].start;
> > +	port->mapbase		= to_platform_device(pdev-
> > >dev.parent)->resource[0].start;
> > +	port->irq		= to_platform_device(pdev-
> > >dev.parent)->resource[1].start;
> 
> and here
> I think it would be easier to read.
> 
> >  	port->rs485_config	= atmel_config_rs485;
> > -	port->membase	= NULL;
> > +	port->membase		= NULL;
> >  
> >  	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port-
> > >rx_ring));
> >  
> >  	/* for console, the clock could already be configured */
> >  	if (!atmel_port->clk) {
> > -		atmel_port->clk = clk_get(&pdev->dev, "usart");
> > +		atmel_port->clk = clk_get(pdev->dev.parent,
> > "usart");
> 
> and here
> 
> >  		if (IS_ERR(atmel_port->clk)) {
> >  			ret = PTR_ERR(atmel_port->clk);
> >  			atmel_port->clk = NULL;
> > @@ -2656,7 +2658,7 @@ static void atmel_serial_probe_fifos(struct
> > atmel_uart_port *atmel_port,
> >  	atmel_port->rts_low = 0;
> >  	atmel_port->rts_high = 0;
> >  
> > -	if (of_property_read_u32(pdev->dev.of_node,
> > +	if (of_property_read_u32(pdev->dev.parent->of_node,
> >  				 "atmel,fifo-size",
> >  				 &atmel_port->fifo_size))
> >  		return;
> > @@ -2694,11 +2696,10 @@ static void atmel_serial_probe_fifos(struct
> > atmel_uart_port *atmel_port,
> >  static int atmel_serial_probe(struct platform_device *pdev)
> >  {
> >  	struct atmel_uart_port *atmel_port;
> > -	struct device_node *np = pdev->dev.of_node;
> > +	struct device_node *np = pdev->dev.parent->of_node;
> >  	void *data;
> >  	int ret = -ENODEV;
> >  	bool rs485_enabled;
> > -
> 
> I think this line feed wasn't so bad.
> 
> >  	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE &
> > (ATMEL_SERIAL_RINGSIZE - 1));
> >  
> >  	ret = of_alias_get_id(np, "serial");
> > @@ -2845,7 +2846,7 @@ static struct platform_driver
> > atmel_serial_driver = {
> >  	.suspend	= atmel_serial_suspend,
> >  	.resume		= atmel_serial_resume,
> >  	.driver		= {
> > -		.name			= "atmel_usart",
> > +		.name			=
> > "atmel_usart_serial",
> >  		.of_match_table		=
> > of_match_ptr(atmel_serial_dt_ids),
> >  	},
> >  };
> > 
> 
> After your patch, the DMA is not selected anymore:
> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not
> available, switch to pio
> instead of:
> atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers
> 
Fixed.
> And the kernel doesn't log anymore on the serial console, despite the
> loglevel=8
> (after reverting this series, the kernel logs reappears on the serial
> console)
> 
Which serial are you using as console? 
> (tests done on sam9g35)
> 
I will consider the rest of suggestions. 
> regards,
> Richard
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180515/f865516c/attachment.sig>

Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Richard Genoud]

On 15/05/2018 14:47, Radu Pirea wrote:
> On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote:
>> After your patch, the DMA is not selected anymore:
>> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not
>> available, switch to pio
>> instead of:
>> atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers
>>
> Fixed.
>> And the kernel doesn't log anymore on the serial console, despite the
>> loglevel=8
>> (after reverting this series, the kernel logs reappears on the serial
>> console)
>>
> Which serial are you using as console? 
fffff200.serial (sam9g35-cm)
( stdout-path = "serial0:115200n8"; in the DTS )

With this series applied, all the kernel log goes on the screen.
Without, it goes on the serial debug.

>> (tests done on sam9g35)
>>
> I will consider the rest of suggestions. 
>> regards,
>> Richard

[PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Richard Genoud]

On 15/05/2018 14:47, Radu Pirea wrote:
> On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote:
>> After your patch, the DMA is not selected anymore:
>> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not
>> available, switch to pio
>> instead of:
>> atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers
>>
> Fixed.
>> And the kernel doesn't log anymore on the serial console, despite the
>> loglevel=8
>> (after reverting this series, the kernel logs reappears on the serial
>> console)
>>
> Which serial are you using as console? 
fffff200.serial (sam9g35-cm)
( stdout-path = "serial0:115200n8"; in the DTS )

With this series applied, all the kernel log goes on the screen.
Without, it goes on the serial debug.

>> (tests done on sam9g35)
>>
> I will consider the rest of suggestions. 
>> regards,
>> Richard

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 526 bytes --]

On Tue, May 15, 2018 at 12:22:24PM +0300, Radu Pirea wrote:
> On Mon, 2018-05-14 at 20:38 +0300, Andy Shevchenko wrote:

> > So, what is not going as expected in "SPI core takes care of CSs"
> > case?
> > Did you use oscilloscope for that?

> Yes, I used and CSs was not asserted. Anyway, I will will try again.

If the core chip select handling is not working properly for some reason
then the core chip select handling should be fixed rather than just open
coding in your driver - probably it's also broken for other users.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Mark Brown]

On Tue, May 15, 2018 at 12:22:24PM +0300, Radu Pirea wrote:
> On Mon, 2018-05-14 at 20:38 +0300, Andy Shevchenko wrote:

> > So, what is not going as expected in "SPI core takes care of CSs"
> > case?
> > Did you use oscilloscope for that?

> Yes, I used and CSs was not asserted. Anyway, I will will try again.

If the core chip select handling is not working properly for some reason
then the core chip select handling should be fixed rather than just open
coding in your driver - probably it's also broken for other users.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180517/795c8270/attachment-0001.sig>

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1903 bytes --]

On Fri, May 11, 2018 at 01:38:21PM +0300, Radu Pirea wrote:

> +config SPI_AT91_USART
> +        tristate "Atmel USART Controller as SPI"
> +	depends on HAS_DMA
> +	depends on (ARCH_AT91 || COMPILE_TEST)
> +        select MFD_AT91_USART
> +	help
> +	  This selects a driver for the AT91 USART Controller as SPI Master,
> +	  present on AT91 and SAMA5 SoC series.
> +

This looks like there's some tab/space mixing going on here.

> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for AT91 USART Controllers as SPI
> + *
> + * Copyright (C) 2018 Microchip Technology Inc.

Make the entire block a C++ comment so it looks more intentional rather
tha mixing C and C++.

> +static inline void at91_usart_spi_tx(struct at91_usart_spi *aus)
> +{
> +	unsigned int len = aus->current_transfer->len;
> +	unsigned int remaining = aus->current_tx_remaining_bytes;
> +	const u8  *tx_buf = aus->current_transfer->tx_buf;
> +
> +	if (tx_buf && remaining) {
> +		if (at91_usart_spi_tx_ready(aus))
> +			spi_writel(aus, THR, tx_buf[len - remaining]);
> +			aus->current_tx_remaining_bytes--;

Missing braces here - we only write to the FIFO if there's space but we
unconditionally decrement the counter.

> +	} else {
> +		if (at91_usart_spi_tx_ready(aus))
> +			spi_writel(aus, THR, US_DUMMY_TX);
> +	}
> +}

This looks like you're open coding SPI_CONTROLLER_MUST_TX

> +	int len = aus->current_transfer->len;
> +	int remaining = aus->current_rx_remaining_bytes;
> +	u8  *rx_buf = aus->current_transfer->rx_buf;
> +
> +	if (aus->current_rx_remaining_bytes) {
> +		rx_buf[len - remaining] = spi_readb(aus, RHR);
> +		aus->current_rx_remaining_bytes--;
> +	} else {
> +		spi_readb(aus, RHR);
> +	}

Similarly for _MUST_RX.

> +	controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;

You're actually setting both flags...  this means that the handling for
cases with missing TX or RX buffers can't happen.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Mark Brown]

On Fri, May 11, 2018 at 01:38:21PM +0300, Radu Pirea wrote:

> +config SPI_AT91_USART
> +        tristate "Atmel USART Controller as SPI"
> +	depends on HAS_DMA
> +	depends on (ARCH_AT91 || COMPILE_TEST)
> +        select MFD_AT91_USART
> +	help
> +	  This selects a driver for the AT91 USART Controller as SPI Master,
> +	  present on AT91 and SAMA5 SoC series.
> +

This looks like there's some tab/space mixing going on here.

> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for AT91 USART Controllers as SPI
> + *
> + * Copyright (C) 2018 Microchip Technology Inc.

Make the entire block a C++ comment so it looks more intentional rather
tha mixing C and C++.

> +static inline void at91_usart_spi_tx(struct at91_usart_spi *aus)
> +{
> +	unsigned int len = aus->current_transfer->len;
> +	unsigned int remaining = aus->current_tx_remaining_bytes;
> +	const u8  *tx_buf = aus->current_transfer->tx_buf;
> +
> +	if (tx_buf && remaining) {
> +		if (at91_usart_spi_tx_ready(aus))
> +			spi_writel(aus, THR, tx_buf[len - remaining]);
> +			aus->current_tx_remaining_bytes--;

Missing braces here - we only write to the FIFO if there's space but we
unconditionally decrement the counter.

> +	} else {
> +		if (at91_usart_spi_tx_ready(aus))
> +			spi_writel(aus, THR, US_DUMMY_TX);
> +	}
> +}

This looks like you're open coding SPI_CONTROLLER_MUST_TX

> +	int len = aus->current_transfer->len;
> +	int remaining = aus->current_rx_remaining_bytes;
> +	u8  *rx_buf = aus->current_transfer->rx_buf;
> +
> +	if (aus->current_rx_remaining_bytes) {
> +		rx_buf[len - remaining] = spi_readb(aus, RHR);
> +		aus->current_rx_remaining_bytes--;
> +	} else {
> +		spi_readb(aus, RHR);
> +	}

Similarly for _MUST_RX.

> +	controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;

You're actually setting both flags...  this means that the handling for
cases with missing TX or RX buffers can't happen.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180517/75f75572/attachment-0001.sig>

Re: [PATCH v3 4/6] dt-bindings: add binding for at91-usart in spi mode

[Rob Herring]

On Fri, May 11, 2018 at 01:38:20PM +0300, Radu Pirea wrote:
> These are bindings for at91-usart IP in spi spi mode. There is no support for

s/spi spi/SPI/

> internal chip select. Only kind of chip selects available are gpio chip

GPIO

> selects.
> 
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
>  .../bindings/spi/microchip,at91-usart-spi.txt | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
> 
> diff --git a/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
> new file mode 100644
> index 000000000000..b68a3bec4121
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt

So now we have 2 copies of the same thing that varies by 2 properties?
Please make this one doc.

> @@ -0,0 +1,28 @@
> +* Universal Synchronous Asynchronous Receiver/Transmitter (USART) in SPI mode
> +
> +Required properties:
> +- #size-cells      : Must be <0>
> +- #address-cells   : Must be <1>
> +- compatible: Should be "atmel,at91rm9200-usart" or "atmel,at91sam9260-usart"
> +- reg: Should contain registers location and length
> +- interrupts: Should contain interrupt
> +- clocks: phandles to input clocks.
> +- clock-names: tuple listing input clock names.
> +	Required elements: "usart"
> +- cs-gpios: chipselects (internal cs not supported)
> +- at91,usart-mode: AT91_USART_MODE_SPI (found in dt-bindings/mfd/at91-usart.h)

at91 is not a vendor.


> +
> +Example:
> +	#include <dt-bindings/mfd/at91-usart.h>
> +
> +	spi0: spi@f001c000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "atmel,at91rm9200-usart", "atmel,at91sam9260-usart";
> +		at91,usart-mode = <AT91_USART_MODE_SPI>;
> +		reg = <0xf001c000 0x100>;
> +		interrupts = <12 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&usart0_clk>;
> +		clock-names = "usart";
> +		cs-gpios = <&pioB 3 0>;
> +	};
> -- 
> 2.17.0
> 

[PATCH v3 4/6] dt-bindings: add binding for at91-usart in spi mode

[Rob Herring]

On Fri, May 11, 2018 at 01:38:20PM +0300, Radu Pirea wrote:
> These are bindings for at91-usart IP in spi spi mode. There is no support for

s/spi spi/SPI/

> internal chip select. Only kind of chip selects available are gpio chip

GPIO

> selects.
> 
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
>  .../bindings/spi/microchip,at91-usart-spi.txt | 28 +++++++++++++++++++
>  1 file changed, 28 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
> 
> diff --git a/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt
> new file mode 100644
> index 000000000000..b68a3bec4121
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/microchip,at91-usart-spi.txt

So now we have 2 copies of the same thing that varies by 2 properties?
Please make this one doc.

> @@ -0,0 +1,28 @@
> +* Universal Synchronous Asynchronous Receiver/Transmitter (USART) in SPI mode
> +
> +Required properties:
> +- #size-cells      : Must be <0>
> +- #address-cells   : Must be <1>
> +- compatible: Should be "atmel,at91rm9200-usart" or "atmel,at91sam9260-usart"
> +- reg: Should contain registers location and length
> +- interrupts: Should contain interrupt
> +- clocks: phandles to input clocks.
> +- clock-names: tuple listing input clock names.
> +	Required elements: "usart"
> +- cs-gpios: chipselects (internal cs not supported)
> +- at91,usart-mode: AT91_USART_MODE_SPI (found in dt-bindings/mfd/at91-usart.h)

at91 is not a vendor.


> +
> +Example:
> +	#include <dt-bindings/mfd/at91-usart.h>
> +
> +	spi0: spi at f001c000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "atmel,at91rm9200-usart", "atmel,at91sam9260-usart";
> +		at91,usart-mode = <AT91_USART_MODE_SPI>;
> +		reg = <0xf001c000 0x100>;
> +		interrupts = <12 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&usart0_clk>;
> +		clock-names = "usart";
> +		cs-gpios = <&pioB 3 0>;
> +	};
> -- 
> 2.17.0
> 

Re: [PATCH v3 2/6] mfd: at91-usart: added mfd driver for usart

[Rob Herring]

On Fri, May 11, 2018 at 01:38:18PM +0300, Radu Pirea wrote:
> This mfd driver is just a wrapper over atmel_serial driver and
> spi-at91-usart driver. Selection of one of the drivers is based on a
> property from device tree. If the property is not specified, the default
> driver is atmel_serial.
> 
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
>  drivers/mfd/Kconfig                  | 10 ++++
>  drivers/mfd/Makefile                 |  1 +
>  drivers/mfd/at91-usart.c             | 75 ++++++++++++++++++++++++++++
>  include/dt-bindings/mfd/at91-usart.h | 17 +++++++
>  4 files changed, 103 insertions(+)
>  create mode 100644 drivers/mfd/at91-usart.c
>  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> 

> +#ifndef __DT_BINDINGS_AT91_USART_H__
> +#define __DT_BINDINGS_AT91_USART_H__
> +
> +#define AT91_USART_MODE_SERIAL	1
> +#define AT91_USART_MODE_SPI	2

Won't this require a DT update for serial mode to add the mode property? 
That breaks compatibility.

Rob

[PATCH v3 2/6] mfd: at91-usart: added mfd driver for usart

[Rob Herring]

On Fri, May 11, 2018 at 01:38:18PM +0300, Radu Pirea wrote:
> This mfd driver is just a wrapper over atmel_serial driver and
> spi-at91-usart driver. Selection of one of the drivers is based on a
> property from device tree. If the property is not specified, the default
> driver is atmel_serial.
> 
> Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> ---
>  drivers/mfd/Kconfig                  | 10 ++++
>  drivers/mfd/Makefile                 |  1 +
>  drivers/mfd/at91-usart.c             | 75 ++++++++++++++++++++++++++++
>  include/dt-bindings/mfd/at91-usart.h | 17 +++++++
>  4 files changed, 103 insertions(+)
>  create mode 100644 drivers/mfd/at91-usart.c
>  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> 

> +#ifndef __DT_BINDINGS_AT91_USART_H__
> +#define __DT_BINDINGS_AT91_USART_H__
> +
> +#define AT91_USART_MODE_SERIAL	1
> +#define AT91_USART_MODE_SPI	2

Won't this require a DT update for serial mode to add the mode property? 
That breaks compatibility.

Rob

Re: [PATCH v3 2/6] mfd: at91-usart: added mfd driver for usart

[Alexandre Belloni]

On 18/05/2018 17:19:49-0500, Rob Herring wrote:
> On Fri, May 11, 2018 at 01:38:18PM +0300, Radu Pirea wrote:
> > This mfd driver is just a wrapper over atmel_serial driver and
> > spi-at91-usart driver. Selection of one of the drivers is based on a
> > property from device tree. If the property is not specified, the default
> > driver is atmel_serial.
> > 
> > Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> > ---
> >  drivers/mfd/Kconfig                  | 10 ++++
> >  drivers/mfd/Makefile                 |  1 +
> >  drivers/mfd/at91-usart.c             | 75 ++++++++++++++++++++++++++++
> >  include/dt-bindings/mfd/at91-usart.h | 17 +++++++
> >  4 files changed, 103 insertions(+)
> >  create mode 100644 drivers/mfd/at91-usart.c
> >  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> > 
> 
> > +#ifndef __DT_BINDINGS_AT91_USART_H__
> > +#define __DT_BINDINGS_AT91_USART_H__
> > +
> > +#define AT91_USART_MODE_SERIAL	1
> > +#define AT91_USART_MODE_SPI	2
> 
> Won't this require a DT update for serial mode to add the mode property? 
> That breaks compatibility.
> 

If the mode property is not present, it defaults to serial to keep
compatibility.


-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v3 2/6] mfd: at91-usart: added mfd driver for usart

[Alexandre Belloni]

On 18/05/2018 17:19:49-0500, Rob Herring wrote:
> On Fri, May 11, 2018 at 01:38:18PM +0300, Radu Pirea wrote:
> > This mfd driver is just a wrapper over atmel_serial driver and
> > spi-at91-usart driver. Selection of one of the drivers is based on a
> > property from device tree. If the property is not specified, the default
> > driver is atmel_serial.
> > 
> > Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
> > ---
> >  drivers/mfd/Kconfig                  | 10 ++++
> >  drivers/mfd/Makefile                 |  1 +
> >  drivers/mfd/at91-usart.c             | 75 ++++++++++++++++++++++++++++
> >  include/dt-bindings/mfd/at91-usart.h | 17 +++++++
> >  4 files changed, 103 insertions(+)
> >  create mode 100644 drivers/mfd/at91-usart.c
> >  create mode 100644 include/dt-bindings/mfd/at91-usart.h
> > 
> 
> > +#ifndef __DT_BINDINGS_AT91_USART_H__
> > +#define __DT_BINDINGS_AT91_USART_H__
> > +
> > +#define AT91_USART_MODE_SERIAL	1
> > +#define AT91_USART_MODE_SPI	2
> 
> Won't this require a DT update for serial mode to add the mode property? 
> That breaks compatibility.
> 

If the mode property is not present, it defaults to serial to keep
compatibility.


-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

On 05/17/2018 08:04 AM, Mark Brown wrote:
> On Fri, May 11, 2018 at 01:38:21PM +0300, Radu Pirea wrote:
> 
>> +config SPI_AT91_USART
>> +        tristate "Atmel USART Controller as SPI"
>> +	depends on HAS_DMA
>> +	depends on (ARCH_AT91 || COMPILE_TEST)
>> +        select MFD_AT91_USART
>> +	help
>> +	  This selects a driver for the AT91 USART Controller as SPI Master,
>> +	  present on AT91 and SAMA5 SoC series.
>> +
> 
> This looks like there's some tab/space mixing going on here.
> 
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for AT91 USART Controllers as SPI
>> + *
>> + * Copyright (C) 2018 Microchip Technology Inc.
> 
> Make the entire block a C++ comment so it looks more intentional rather
> tha mixing C and C++.

Hi Mark,

I know it's ugly, but SPDX license identifier must be in a separate 
comment block.

> 
>> +static inline void at91_usart_spi_tx(struct at91_usart_spi *aus)
>> +{
>> +	unsigned int len = aus->current_transfer->len;
>> +	unsigned int remaining = aus->current_tx_remaining_bytes;
>> +	const u8  *tx_buf = aus->current_transfer->tx_buf;
>> +
>> +	if (tx_buf && remaining) {
>> +		if (at91_usart_spi_tx_ready(aus))
>> +			spi_writel(aus, THR, tx_buf[len - remaining]);
>> +			aus->current_tx_remaining_bytes--;
> 
> Missing braces here - we only write to the FIFO if there's space but we
> unconditionally decrement the counter.
> 

Thanks. I will fix it.

>> +	} else {
>> +		if (at91_usart_spi_tx_ready(aus))
>> +			spi_writel(aus, THR, US_DUMMY_TX);
>> +	}
>> +}
> 
> This looks like you're open coding SPI_CONTROLLER_MUST_TX
> 
>> +	int len = aus->current_transfer->len;
>> +	int remaining = aus->current_rx_remaining_bytes;
>> +	u8  *rx_buf = aus->current_transfer->rx_buf;
>> +
>> +	if (aus->current_rx_remaining_bytes) {
>> +		rx_buf[len - remaining] = spi_readb(aus, RHR);
>> +		aus->current_rx_remaining_bytes--;
>> +	} else {
>> +		spi_readb(aus, RHR);
>> +	}
> 
> Similarly for _MUST_RX.
> 
>> +	controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
> 
> You're actually setting both flags...  this means that the handling for
> cases with missing TX or RX buffers can't happen.

Sorry. My mistake. I will remove unnecessary code.

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

On 05/17/2018 08:04 AM, Mark Brown wrote:
> On Fri, May 11, 2018 at 01:38:21PM +0300, Radu Pirea wrote:
> 
>> +config SPI_AT91_USART
>> +        tristate "Atmel USART Controller as SPI"
>> +	depends on HAS_DMA
>> +	depends on (ARCH_AT91 || COMPILE_TEST)
>> +        select MFD_AT91_USART
>> +	help
>> +	  This selects a driver for the AT91 USART Controller as SPI Master,
>> +	  present on AT91 and SAMA5 SoC series.
>> +
> 
> This looks like there's some tab/space mixing going on here.
> 
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for AT91 USART Controllers as SPI
>> + *
>> + * Copyright (C) 2018 Microchip Technology Inc.
> 
> Make the entire block a C++ comment so it looks more intentional rather
> tha mixing C and C++.

Hi Mark,

I know it's ugly, but SPDX license identifier must be in a separate 
comment block.

> 
>> +static inline void at91_usart_spi_tx(struct at91_usart_spi *aus)
>> +{
>> +	unsigned int len = aus->current_transfer->len;
>> +	unsigned int remaining = aus->current_tx_remaining_bytes;
>> +	const u8  *tx_buf = aus->current_transfer->tx_buf;
>> +
>> +	if (tx_buf && remaining) {
>> +		if (at91_usart_spi_tx_ready(aus))
>> +			spi_writel(aus, THR, tx_buf[len - remaining]);
>> +			aus->current_tx_remaining_bytes--;
> 
> Missing braces here - we only write to the FIFO if there's space but we
> unconditionally decrement the counter.
> 

Thanks. I will fix it.

>> +	} else {
>> +		if (at91_usart_spi_tx_ready(aus))
>> +			spi_writel(aus, THR, US_DUMMY_TX);
>> +	}
>> +}
> 
> This looks like you're open coding SPI_CONTROLLER_MUST_TX
> 
>> +	int len = aus->current_transfer->len;
>> +	int remaining = aus->current_rx_remaining_bytes;
>> +	u8  *rx_buf = aus->current_transfer->rx_buf;
>> +
>> +	if (aus->current_rx_remaining_bytes) {
>> +		rx_buf[len - remaining] = spi_readb(aus, RHR);
>> +		aus->current_rx_remaining_bytes--;
>> +	} else {
>> +		spi_readb(aus, RHR);
>> +	}
> 
> Similarly for _MUST_RX.
> 
>> +	controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
> 
> You're actually setting both flags...  this means that the handling for
> cases with missing TX or RX buffers can't happen.

Sorry. My mistake. I will remove unnecessary code.

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

On 05/17/2018 08:04 AM, Mark Brown wrote:
> On Fri, May 11, 2018 at 01:38:21PM +0300, Radu Pirea wrote:
> 
>> +config SPI_AT91_USART
>> +        tristate "Atmel USART Controller as SPI"
>> +	depends on HAS_DMA
>> +	depends on (ARCH_AT91 || COMPILE_TEST)
>> +        select MFD_AT91_USART
>> +	help
>> +	  This selects a driver for the AT91 USART Controller as SPI Master,
>> +	  present on AT91 and SAMA5 SoC series.
>> +
> 
> This looks like there's some tab/space mixing going on here.
> 
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Driver for AT91 USART Controllers as SPI
>> + *
>> + * Copyright (C) 2018 Microchip Technology Inc.
> 
> Make the entire block a C++ comment so it looks more intentional rather
> tha mixing C and C++.

Hi Mark,

I know it's ugly, but SPDX license identifier must be in a separate 
comment block.

> 
>> +static inline void at91_usart_spi_tx(struct at91_usart_spi *aus)
>> +{
>> +	unsigned int len = aus->current_transfer->len;
>> +	unsigned int remaining = aus->current_tx_remaining_bytes;
>> +	const u8  *tx_buf = aus->current_transfer->tx_buf;
>> +
>> +	if (tx_buf && remaining) {
>> +		if (at91_usart_spi_tx_ready(aus))
>> +			spi_writel(aus, THR, tx_buf[len - remaining]);
>> +			aus->current_tx_remaining_bytes--;
> 
> Missing braces here - we only write to the FIFO if there's space but we
> unconditionally decrement the counter.
> 

Thanks. I will fix it.

>> +	} else {
>> +		if (at91_usart_spi_tx_ready(aus))
>> +			spi_writel(aus, THR, US_DUMMY_TX);
>> +	}
>> +}
> 
> This looks like you're open coding SPI_CONTROLLER_MUST_TX
> 
>> +	int len = aus->current_transfer->len;
>> +	int remaining = aus->current_rx_remaining_bytes;
>> +	u8  *rx_buf = aus->current_transfer->rx_buf;
>> +
>> +	if (aus->current_rx_remaining_bytes) {
>> +		rx_buf[len - remaining] = spi_readb(aus, RHR);
>> +		aus->current_rx_remaining_bytes--;
>> +	} else {
>> +		spi_readb(aus, RHR);
>> +	}
> 
> Similarly for _MUST_RX.
> 
>> +	controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
> 
> You're actually setting both flags...  this means that the handling for
> cases with missing TX or RX buffers can't happen.

Sorry. My mistake. I will remove unnecessary code.

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

On Wed, May 23, 2018 at 11:10:28AM +0300, Radu Pirea wrote:
> On 05/17/2018 08:04 AM, Mark Brown wrote:

> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Driver for AT91 USART Controllers as SPI
> > > + *
> > > + * Copyright (C) 2018 Microchip Technology Inc.

> > Make the entire block a C++ comment so it looks more intentional rather
> > tha mixing C and C++.

> I know it's ugly, but SPDX license identifier must be in a separate comment
> block.

No, it doesn't - it just needs to be the first line of the file.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Mark Brown]

On Wed, May 23, 2018 at 11:10:28AM +0300, Radu Pirea wrote:
> On 05/17/2018 08:04 AM, Mark Brown wrote:

> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Driver for AT91 USART Controllers as SPI
> > > + *
> > > + * Copyright (C) 2018 Microchip Technology Inc.

> > Make the entire block a C++ comment so it looks more intentional rather
> > tha mixing C and C++.

> I know it's ugly, but SPDX license identifier must be in a separate comment
> block.

No, it doesn't - it just needs to be the first line of the file.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180523/77fafae7/attachment.sig>

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

On 05/17/2018 07:54 AM, Mark Brown wrote:
> On Tue, May 15, 2018 at 12:22:24PM +0300, Radu Pirea wrote:
>> On Mon, 2018-05-14 at 20:38 +0300, Andy Shevchenko wrote:
> 
>>> So, what is not going as expected in "SPI core takes care of CSs"
>>> case?
>>> Did you use oscilloscope for that?
> 
>> Yes, I used and CSs was not asserted. Anyway, I will will try again.
> 
> If the core chip select handling is not working properly for some reason
> then the core chip select handling should be fixed rather than just open
> coding in your driver - probably it's also broken for other users.
> 

Hi Mark,

I found the fix for cs-gpios. If I change spi_add_device function like 
this(see below) everything is ok.

int spi_add_device(struct spi_device *spi)

...

     if (ctlr->cs_gpios){
         spi->cs_gpio = ctlr->cs_gpios[spi->chip_select];
         if(gpio_is_valid(spi->cs_gpio))
             gpio_direction_output(spi->cs_gpio, !(spi->mode & 
SPI_CS_HIGH));

     }

...

     return status;
}

In the subsystem gpio direction of pins is never set and 
gpio_set_value() don't set the direction.
In my opinion gpio_direction_output() set direction should be called in 
spi_add_device. What do you think? Is ok?

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

On 05/17/2018 07:54 AM, Mark Brown wrote:
> On Tue, May 15, 2018 at 12:22:24PM +0300, Radu Pirea wrote:
>> On Mon, 2018-05-14 at 20:38 +0300, Andy Shevchenko wrote:
> 
>>> So, what is not going as expected in "SPI core takes care of CSs"
>>> case?
>>> Did you use oscilloscope for that?
> 
>> Yes, I used and CSs was not asserted. Anyway, I will will try again.
> 
> If the core chip select handling is not working properly for some reason
> then the core chip select handling should be fixed rather than just open
> coding in your driver - probably it's also broken for other users.
> 

Hi Mark,

I found the fix for cs-gpios. If I change spi_add_device function like 
this(see below) everything is ok.

int spi_add_device(struct spi_device *spi)

...

     if (ctlr->cs_gpios){
         spi->cs_gpio = ctlr->cs_gpios[spi->chip_select];
         if(gpio_is_valid(spi->cs_gpio))
             gpio_direction_output(spi->cs_gpio, !(spi->mode & 
SPI_CS_HIGH));

     }

...

     return status;
}

In the subsystem gpio direction of pins is never set and 
gpio_set_value() don't set the direction.
In my opinion gpio_direction_output() set direction should be called in 
spi_add_device. What do you think? Is ok?

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Radu Pirea]

On 05/17/2018 07:54 AM, Mark Brown wrote:
> On Tue, May 15, 2018 at 12:22:24PM +0300, Radu Pirea wrote:
>> On Mon, 2018-05-14 at 20:38 +0300, Andy Shevchenko wrote:
> 
>>> So, what is not going as expected in "SPI core takes care of CSs"
>>> case?
>>> Did you use oscilloscope for that?
> 
>> Yes, I used and CSs was not asserted. Anyway, I will will try again.
> 
> If the core chip select handling is not working properly for some reason
> then the core chip select handling should be fixed rather than just open
> coding in your driver - probably it's also broken for other users.
> 

Hi Mark,

I found the fix for cs-gpios. If I change spi_add_device function like 
this(see below) everything is ok.

int spi_add_device(struct spi_device *spi)

...

     if (ctlr->cs_gpios){
         spi->cs_gpio = ctlr->cs_gpios[spi->chip_select];
         if(gpio_is_valid(spi->cs_gpio))
             gpio_direction_output(spi->cs_gpio, !(spi->mode & 
SPI_CS_HIGH));

     }

...

     return status;
}

In the subsystem gpio direction of pins is never set and 
gpio_set_value() don't set the direction.
In my opinion gpio_direction_output() set direction should be called in 
spi_add_device. What do you think? Is ok?

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Alexandre Belloni]

Hi,

On 24/05/2018 19:04:11+0300, Radu Pirea wrote:
> 
> 
> On 05/17/2018 07:54 AM, Mark Brown wrote:
> > On Tue, May 15, 2018 at 12:22:24PM +0300, Radu Pirea wrote:
> > > On Mon, 2018-05-14 at 20:38 +0300, Andy Shevchenko wrote:
> > 
> > > > So, what is not going as expected in "SPI core takes care of CSs"
> > > > case?
> > > > Did you use oscilloscope for that?
> > 
> > > Yes, I used and CSs was not asserted. Anyway, I will will try again.
> > 
> > If the core chip select handling is not working properly for some reason
> > then the core chip select handling should be fixed rather than just open
> > coding in your driver - probably it's also broken for other users.
> > 
> 
> Hi Mark,
> 
> I found the fix for cs-gpios. If I change spi_add_device function like
> this(see below) everything is ok.
> 
> int spi_add_device(struct spi_device *spi)
> 
> ...
> 
>     if (ctlr->cs_gpios){
>         spi->cs_gpio = ctlr->cs_gpios[spi->chip_select];
>         if(gpio_is_valid(spi->cs_gpio))
>             gpio_direction_output(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
> 
>     }
> 
> ...
> 
>     return status;
> }
> 
> In the subsystem gpio direction of pins is never set and gpio_set_value()
> don't set the direction.
> In my opinion gpio_direction_output() set direction should be called in
> spi_add_device. What do you think? Is ok?

Back in 2014, I was suggesting using devm_gpio_request_one() in
of_spi_register_master(). That would take care of setting the direction
of the GPIO:

https://www.spinics.net/lists/arm-kernel/msg351251.html

I never took the time to create the patch and test though.

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Alexandre Belloni]

Hi,

On 24/05/2018 19:04:11+0300, Radu Pirea wrote:
> 
> 
> On 05/17/2018 07:54 AM, Mark Brown wrote:
> > On Tue, May 15, 2018 at 12:22:24PM +0300, Radu Pirea wrote:
> > > On Mon, 2018-05-14 at 20:38 +0300, Andy Shevchenko wrote:
> > 
> > > > So, what is not going as expected in "SPI core takes care of CSs"
> > > > case?
> > > > Did you use oscilloscope for that?
> > 
> > > Yes, I used and CSs was not asserted. Anyway, I will will try again.
> > 
> > If the core chip select handling is not working properly for some reason
> > then the core chip select handling should be fixed rather than just open
> > coding in your driver - probably it's also broken for other users.
> > 
> 
> Hi Mark,
> 
> I found the fix for cs-gpios. If I change spi_add_device function like
> this(see below) everything is ok.
> 
> int spi_add_device(struct spi_device *spi)
> 
> ...
> 
>     if (ctlr->cs_gpios){
>         spi->cs_gpio = ctlr->cs_gpios[spi->chip_select];
>         if(gpio_is_valid(spi->cs_gpio))
>             gpio_direction_output(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
> 
>     }
> 
> ...
> 
>     return status;
> }
> 
> In the subsystem gpio direction of pins is never set and gpio_set_value()
> don't set the direction.
> In my opinion gpio_direction_output() set direction should be called in
> spi_add_device. What do you think? Is ok?

Back in 2014, I was suggesting using devm_gpio_request_one() in
of_spi_register_master(). That would take care of setting the direction
of the GPIO:

https://www.spinics.net/lists/arm-kernel/msg351251.html

I never took the time to create the patch and test though.

-- 
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 526 bytes --]

On Thu, May 24, 2018 at 07:04:11PM +0300, Radu Pirea wrote:

>     if (ctlr->cs_gpios){
>         spi->cs_gpio = ctlr->cs_gpios[spi->chip_select];
>         if(gpio_is_valid(spi->cs_gpio))
>             gpio_direction_output(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
> 
>     }

You're expected to request the GPIOs in your driver using one of the
modern request functions like gpio_request_one() (or ideally the GPIO
descriptor APIs now) which combine the direction setting and request
into a single operation.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Mark Brown]

On Thu, May 24, 2018 at 07:04:11PM +0300, Radu Pirea wrote:

>     if (ctlr->cs_gpios){
>         spi->cs_gpio = ctlr->cs_gpios[spi->chip_select];
>         if(gpio_is_valid(spi->cs_gpio))
>             gpio_direction_output(spi->cs_gpio, !(spi->mode & SPI_CS_HIGH));
> 
>     }

You're expected to request the GPIOs in your driver using one of the
modern request functions like gpio_request_one() (or ideally the GPIO
descriptor APIs now) which combine the direction setting and request
into a single operation.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/afe4d42a/attachment.sig>

Re: [PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 565 bytes --]

On Thu, May 24, 2018 at 07:56:20PM +0200, Alexandre Belloni wrote:

> Back in 2014, I was suggesting using devm_gpio_request_one() in
> of_spi_register_master(). That would take care of setting the direction
> of the GPIO:

> https://www.spinics.net/lists/arm-kernel/msg351251.html

> I never took the time to create the patch and test though.

Right, this'd be ideal but unfortunately it does mean we'd have to
transition all the existing users that open code their requests which is
a pain.  It'd be really nice if someone had the time/enthusiam to do it
though.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v3 5/6] spi: at91-usart: add driver for at91-usart as spi

[Mark Brown]

On Thu, May 24, 2018 at 07:56:20PM +0200, Alexandre Belloni wrote:

> Back in 2014, I was suggesting using devm_gpio_request_one() in
> of_spi_register_master(). That would take care of setting the direction
> of the GPIO:

> https://www.spinics.net/lists/arm-kernel/msg351251.html

> I never took the time to create the patch and test though.

Right, this'd be ideal but unfortunately it does mean we'd have to
transition all the existing users that open code their requests which is
a pain.  It'd be really nice if someone had the time/enthusiam to do it
though.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180524/4bbd58ed/attachment.sig>

Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Radu Pirea]

On 05/15/2018 04:14 PM, Richard Genoud wrote:
> On 15/05/2018 14:47, Radu Pirea wrote:
>> On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote:
>>> After your patch, the DMA is not selected anymore:
>>> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not
>>> available, switch to pio
>>> instead of:
>>> atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers
>>>
>> Fixed.
>>> And the kernel doesn't log anymore on the serial console, despite the
>>> loglevel=8
>>> (after reverting this series, the kernel logs reappears on the serial
>>> console)
>>>
>> Which serial are you using as console?
> fffff200.serial (sam9g35-cm)
> ( stdout-path = "serial0:115200n8"; in the DTS )
> 
> With this series applied, all the kernel log goes on the screen.
> Without, it goes on the serial debug.
> 
I tested again with archlinux arm and poky-linux4sam release as distros 
and kernel log goes on the serial debug. Can you give me more details 
like cmdline?
>>> (tests done on sam9g35)
>>>
>> I will consider the rest of suggestions.
>>> regards,
>>> Richard
> 

Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Radu Pirea]

On 05/15/2018 04:14 PM, Richard Genoud wrote:
> On 15/05/2018 14:47, Radu Pirea wrote:
>> On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote:
>>> After your patch, the DMA is not selected anymore:
>>> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not
>>> available, switch to pio
>>> instead of:
>>> atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers
>>>
>> Fixed.
>>> And the kernel doesn't log anymore on the serial console, despite the
>>> loglevel=8
>>> (after reverting this series, the kernel logs reappears on the serial
>>> console)
>>>
>> Which serial are you using as console?
> fffff200.serial (sam9g35-cm)
> ( stdout-path = "serial0:115200n8"; in the DTS )
> 
> With this series applied, all the kernel log goes on the screen.
> Without, it goes on the serial debug.
> 
I tested again with archlinux arm and poky-linux4sam release as distros 
and kernel log goes on the serial debug. Can you give me more details 
like cmdline?
>>> (tests done on sam9g35)
>>>
>> I will consider the rest of suggestions.
>>> regards,
>>> Richard
> 

[PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Radu Pirea]

On 05/15/2018 04:14 PM, Richard Genoud wrote:
> On 15/05/2018 14:47, Radu Pirea wrote:
>> On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote:
>>> After your patch, the DMA is not selected anymore:
>>> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not
>>> available, switch to pio
>>> instead of:
>>> atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers
>>>
>> Fixed.
>>> And the kernel doesn't log anymore on the serial console, despite the
>>> loglevel=8
>>> (after reverting this series, the kernel logs reappears on the serial
>>> console)
>>>
>> Which serial are you using as console?
> fffff200.serial (sam9g35-cm)
> ( stdout-path = "serial0:115200n8"; in the DTS )
> 
> With this series applied, all the kernel log goes on the screen.
> Without, it goes on the serial debug.
> 
I tested again with archlinux arm and poky-linux4sam release as distros 
and kernel log goes on the serial debug. Can you give me more details 
like cmdline?
>>> (tests done on sam9g35)
>>>
>> I will consider the rest of suggestions.
>>> regards,
>>> Richard
> 

Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Richard Genoud]

On 25/05/2018 14:17, Radu Pirea wrote:
> 
> 
> On 05/15/2018 04:14 PM, Richard Genoud wrote:
>> On 15/05/2018 14:47, Radu Pirea wrote:
>>> On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote:
>>>> After your patch, the DMA is not selected anymore:
>>>> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not
>>>> available, switch to pio
>>>> instead of:
>>>> atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers
>>>>
>>> Fixed.
>>>> And the kernel doesn't log anymore on the serial console, despite the
>>>> loglevel=8
>>>> (after reverting this series, the kernel logs reappears on the serial
>>>> console)
>>>>
>>> Which serial are you using as console?
>> fffff200.serial (sam9g35-cm)
>> ( stdout-path = "serial0:115200n8"; in the DTS )
>>
>> With this series applied, all the kernel log goes on the screen.
>> Without, it goes on the serial debug.
>>
> I tested again with archlinux arm and poky-linux4sam release as distros
> and kernel log goes on the serial debug. Can you give me more details
> like cmdline?
I used kernel 4.17-rc6
at91_dt_defconfig
at91sam9g35ek.dtb

Kernel command line: root=/dev/mtdblock1 rw rootfstype=ubifs ubi.mtd=1 root=ubi0:rootfs
( the one from the DTS )

Detailed instructions:

git checkout v4.17-rc6 
ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 at91_dt_defconfig
ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 uImage at91sam9g35ek.dtb
cp arch/arm/boot/uImage arch/arm/boot/dts/at91sam9g35ek.dtb /tftpboot/

>From uboot:
tftpboot 0x20007FC0 uImage
tftpboot 0x26400000 at91sam9g35ek.dtb
bootm 0x20007FC0 - 0x26400000

[ I see the logs on the serial debug ]

git am \[PATCH\ v3\ [1-6]*

ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 uImage at91sam9g35ek.dtb
cp arch/arm/boot/uImage arch/arm/boot/dts/at91sam9g35ek.dtb /tftpboot/

>From uboot:
tftpboot 0x20007FC0 uImage
tftpboot 0x26400000 at91sam9g35ek.dtb
bootm 0x20007FC0 - 0x26400000

[ I don't see the logs on the serial debug anymore ]


>>>> (tests done on sam9g35)
>>>>
>>> I will consider the rest of suggestions.
>>>> regards,
>>>> Richard
>>

[PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Richard Genoud]

On 25/05/2018 14:17, Radu Pirea wrote:
> 
> 
> On 05/15/2018 04:14 PM, Richard Genoud wrote:
>> On 15/05/2018 14:47, Radu Pirea wrote:
>>> On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote:
>>>> After your patch, the DMA is not selected anymore:
>>>> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not
>>>> available, switch to pio
>>>> instead of:
>>>> atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers
>>>>
>>> Fixed.
>>>> And the kernel doesn't log anymore on the serial console, despite the
>>>> loglevel=8
>>>> (after reverting this series, the kernel logs reappears on the serial
>>>> console)
>>>>
>>> Which serial are you using as console?
>> fffff200.serial (sam9g35-cm)
>> ( stdout-path = "serial0:115200n8"; in the DTS )
>>
>> With this series applied, all the kernel log goes on the screen.
>> Without, it goes on the serial debug.
>>
> I tested again with archlinux arm and poky-linux4sam release as distros
> and kernel log goes on the serial debug. Can you give me more details
> like cmdline?
I used kernel 4.17-rc6
at91_dt_defconfig
at91sam9g35ek.dtb

Kernel command line: root=/dev/mtdblock1 rw rootfstype=ubifs ubi.mtd=1 root=ubi0:rootfs
( the one from the DTS )

Detailed instructions:

git checkout v4.17-rc6 
ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 at91_dt_defconfig
ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 uImage at91sam9g35ek.dtb
cp arch/arm/boot/uImage arch/arm/boot/dts/at91sam9g35ek.dtb /tftpboot/

>From uboot:
tftpboot 0x20007FC0 uImage
tftpboot 0x26400000 at91sam9g35ek.dtb
bootm 0x20007FC0 - 0x26400000

[ I see the logs on the serial debug ]

git am \[PATCH\ v3\ [1-6]*

ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 uImage at91sam9g35ek.dtb
cp arch/arm/boot/uImage arch/arm/boot/dts/at91sam9g35ek.dtb /tftpboot/

>From uboot:
tftpboot 0x20007FC0 uImage
tftpboot 0x26400000 at91sam9g35ek.dtb
bootm 0x20007FC0 - 0x26400000

[ I don't see the logs on the serial debug anymore ]


>>>> (tests done on sam9g35)
>>>>
>>> I will consider the rest of suggestions.
>>>> regards,
>>>> Richard
>>

Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Radu Pirea]

On 05/25/2018 04:35 PM, Richard Genoud wrote:
> On 25/05/2018 14:17, Radu Pirea wrote:
>>
>>
>> On 05/15/2018 04:14 PM, Richard Genoud wrote:
>>> On 15/05/2018 14:47, Radu Pirea wrote:
>>>> On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote:
>>>>> After your patch, the DMA is not selected anymore:
>>>>> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not
>>>>> available, switch to pio
>>>>> instead of:
>>>>> atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers
>>>>>
>>>> Fixed.
>>>>> And the kernel doesn't log anymore on the serial console, despite the
>>>>> loglevel=8
>>>>> (after reverting this series, the kernel logs reappears on the serial
>>>>> console)
>>>>>
>>>> Which serial are you using as console?
>>> fffff200.serial (sam9g35-cm)
>>> ( stdout-path = "serial0:115200n8"; in the DTS )
>>>
>>> With this series applied, all the kernel log goes on the screen.
>>> Without, it goes on the serial debug.
>>>
>> I tested again with archlinux arm and poky-linux4sam release as distros
>> and kernel log goes on the serial debug. Can you give me more details
>> like cmdline?
> I used kernel 4.17-rc6
> at91_dt_defconfig
> at91sam9g35ek.dtb
> 
> Kernel command line: root=/dev/mtdblock1 rw rootfstype=ubifs ubi.mtd=1 root=ubi0:rootfs
I reproduced your issue.
If in cmdline console is not set like this "console=ttyS0,115200", the 
console goes to the lcd.
> ( the one from the DTS )
> 
> Detailed instructions:
> 
> git checkout v4.17-rc6
> ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 at91_dt_defconfig
> ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 uImage at91sam9g35ek.dtb
> cp arch/arm/boot/uImage arch/arm/boot/dts/at91sam9g35ek.dtb /tftpboot/
> 
>  From uboot:
> tftpboot 0x20007FC0 uImage
> tftpboot 0x26400000 at91sam9g35ek.dtb
> bootm 0x20007FC0 - 0x26400000
> 
> [ I see the logs on the serial debug ]
> 
> git am \[PATCH\ v3\ [1-6]*
> 
> ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 uImage at91sam9g35ek.dtb
> cp arch/arm/boot/uImage arch/arm/boot/dts/at91sam9g35ek.dtb /tftpboot/
> 
>  From uboot:
> tftpboot 0x20007FC0 uImage
> tftpboot 0x26400000 at91sam9g35ek.dtb
> bootm 0x20007FC0 - 0x26400000
> 
> [ I don't see the logs on the serial debug anymore ]
> 
> 
>>>>> (tests done on sam9g35)
>>>>>
>>>> I will consider the rest of suggestions.
>>>>> regards,
>>>>> Richard
>>>
> 

Re: [PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Radu Pirea]

On 05/25/2018 04:35 PM, Richard Genoud wrote:
> On 25/05/2018 14:17, Radu Pirea wrote:
>>
>>
>> On 05/15/2018 04:14 PM, Richard Genoud wrote:
>>> On 15/05/2018 14:47, Radu Pirea wrote:
>>>> On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote:
>>>>> After your patch, the DMA is not selected anymore:
>>>>> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not
>>>>> available, switch to pio
>>>>> instead of:
>>>>> atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers
>>>>>
>>>> Fixed.
>>>>> And the kernel doesn't log anymore on the serial console, despite the
>>>>> loglevel=8
>>>>> (after reverting this series, the kernel logs reappears on the serial
>>>>> console)
>>>>>
>>>> Which serial are you using as console?
>>> fffff200.serial (sam9g35-cm)
>>> ( stdout-path = "serial0:115200n8"; in the DTS )
>>>
>>> With this series applied, all the kernel log goes on the screen.
>>> Without, it goes on the serial debug.
>>>
>> I tested again with archlinux arm and poky-linux4sam release as distros
>> and kernel log goes on the serial debug. Can you give me more details
>> like cmdline?
> I used kernel 4.17-rc6
> at91_dt_defconfig
> at91sam9g35ek.dtb
> 
> Kernel command line: root=/dev/mtdblock1 rw rootfstype=ubifs ubi.mtd=1 root=ubi0:rootfs
I reproduced your issue.
If in cmdline console is not set like this "console=ttyS0,115200", the 
console goes to the lcd.
> ( the one from the DTS )
> 
> Detailed instructions:
> 
> git checkout v4.17-rc6
> ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 at91_dt_defconfig
> ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 uImage at91sam9g35ek.dtb
> cp arch/arm/boot/uImage arch/arm/boot/dts/at91sam9g35ek.dtb /tftpboot/
> 
>  From uboot:
> tftpboot 0x20007FC0 uImage
> tftpboot 0x26400000 at91sam9g35ek.dtb
> bootm 0x20007FC0 - 0x26400000
> 
> [ I see the logs on the serial debug ]
> 
> git am \[PATCH\ v3\ [1-6]*
> 
> ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 uImage at91sam9g35ek.dtb
> cp arch/arm/boot/uImage arch/arm/boot/dts/at91sam9g35ek.dtb /tftpboot/
> 
>  From uboot:
> tftpboot 0x20007FC0 uImage
> tftpboot 0x26400000 at91sam9g35ek.dtb
> bootm 0x20007FC0 - 0x26400000
> 
> [ I don't see the logs on the serial debug anymore ]
> 
> 
>>>>> (tests done on sam9g35)
>>>>>
>>>> I will consider the rest of suggestions.
>>>>> regards,
>>>>> Richard
>>>
> 

[PATCH v3 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd

[Radu Pirea]

On 05/25/2018 04:35 PM, Richard Genoud wrote:
> On 25/05/2018 14:17, Radu Pirea wrote:
>>
>>
>> On 05/15/2018 04:14 PM, Richard Genoud wrote:
>>> On 15/05/2018 14:47, Radu Pirea wrote:
>>>> On Mon, 2018-05-14 at 12:57 +0200, Richard Genoud wrote:
>>>>> After your patch, the DMA is not selected anymore:
>>>>> atmel_usart_serial atmel_usart_serial.0.auto: TX channel not
>>>>> available, switch to pio
>>>>> instead of:
>>>>> atmel_usart fffff200.serial: using dma1chan2 for tx DMA transfers
>>>>>
>>>> Fixed.
>>>>> And the kernel doesn't log anymore on the serial console, despite the
>>>>> loglevel=8
>>>>> (after reverting this series, the kernel logs reappears on the serial
>>>>> console)
>>>>>
>>>> Which serial are you using as console?
>>> fffff200.serial (sam9g35-cm)
>>> ( stdout-path = "serial0:115200n8"; in the DTS )
>>>
>>> With this series applied, all the kernel log goes on the screen.
>>> Without, it goes on the serial debug.
>>>
>> I tested again with archlinux arm and poky-linux4sam release as distros
>> and kernel log goes on the serial debug. Can you give me more details
>> like cmdline?
> I used kernel 4.17-rc6
> at91_dt_defconfig
> at91sam9g35ek.dtb
> 
> Kernel command line: root=/dev/mtdblock1 rw rootfstype=ubifs ubi.mtd=1 root=ubi0:rootfs
I reproduced your issue.
If in cmdline console is not set like this "console=ttyS0,115200", the 
console goes to the lcd.
> ( the one from the DTS )
> 
> Detailed instructions:
> 
> git checkout v4.17-rc6
> ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 at91_dt_defconfig
> ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 uImage at91sam9g35ek.dtb
> cp arch/arm/boot/uImage arch/arm/boot/dts/at91sam9g35ek.dtb /tftpboot/
> 
>  From uboot:
> tftpboot 0x20007FC0 uImage
> tftpboot 0x26400000 at91sam9g35ek.dtb
> bootm 0x20007FC0 - 0x26400000
> 
> [ I see the logs on the serial debug ]
> 
> git am \[PATCH\ v3\ [1-6]*
> 
> ARCH=arm CROSS_COMPILE=path_to_my_Xchain/arm-linux- LOADADDR=0x20008000 make -j 12 uImage at91sam9g35ek.dtb
> cp arch/arm/boot/uImage arch/arm/boot/dts/at91sam9g35ek.dtb /tftpboot/
> 
>  From uboot:
> tftpboot 0x20007FC0 uImage
> tftpboot 0x26400000 at91sam9g35ek.dtb
> bootm 0x20007FC0 - 0x26400000
> 
> [ I don't see the logs on the serial debug anymore ]
> 
> 
>>>>> (tests done on sam9g35)
>>>>>
>>>> I will consider the rest of suggestions.
>>>>> regards,
>>>>> Richard
>>>
>