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
Changes in v4:
- modified the spi driver to use cs gpio support form spi subsystem
- fixed dma transfers for serial driver
- squashed binding for spi and serial and moved them to mfd/atmel-usart.txt
Radu Pirea (6):
MAINTAINERS: add at91 usart mfd driver
dt-bindings: add binding for atmel-usart in SPI mode
mfd: at91-usart: added mfd driver for usart
MAINTAINERS: add at91 usart spi driver
spi: at91-usart: add driver for at91-usart as spi
tty/serial: atmel: changed the driver to work under at91-usart mfd
.../bindings/{serial => mfd}/atmel-usart.txt | 25 +-
MAINTAINERS | 16 +
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 | 431 ++++++++++++++++++
drivers/tty/serial/Kconfig | 1 +
drivers/tty/serial/atmel_serial.c | 40 +-
include/dt-bindings/mfd/at91-usart.h | 17 +
11 files changed, 606 insertions(+), 20 deletions(-)
rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
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
This patch moves the bindings for serial from serial/atmel-usart.txt to
mfd/atmel-usart.txt and adds bindings for USART in SPI mode.
Signed-off-by: Radu Pirea <[email protected]>
---
.../bindings/{serial => mfd}/atmel-usart.txt | 25 +++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/mfd/atmel-usart.txt
similarity index 76%
rename from Documentation/devicetree/bindings/serial/atmel-usart.txt
rename to Documentation/devicetree/bindings/mfd/atmel-usart.txt
index 7c0d6b2f53e4..3b9e18642c3b 100644
--- a/Documentation/devicetree/bindings/serial/atmel-usart.txt
+++ b/Documentation/devicetree/bindings/mfd/atmel-usart.txt
@@ -1,6 +1,6 @@
* Atmel Universal Synchronous Asynchronous Receiver/Transmitter (USART)
-Required properties:
+Required properties for USART:
- compatible: Should be "atmel,<chip>-usart" or "atmel,<chip>-dbgu"
The compatible <chip> indicated will be the first SoC to support an
additional mode or an USART new feature.
@@ -11,7 +11,13 @@ Required properties:
Required elements: "usart"
- clocks: phandles to input clocks.
-Optional properties:
+Required properties for USART in SPI mode:
+- #size-cells : Must be <0>
+- #address-cells : Must be <1>
+- cs-gpios: chipselects (internal cs not supported)
+- atmel,usart-mode : Must be <USART_MODE_SPI> (found in dt-bindings/mfd/at91-usart.h)
+
+Optional properties in serial mode:
- atmel,use-dma-rx: use of PDC or DMA for receiving data
- atmel,use-dma-tx: use of PDC or DMA for transmitting data
- {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD line respectively.
@@ -62,3 +68,18 @@ Example:
dma-names = "tx", "rx";
atmel,fifo-size = <32>;
};
+
+- SPI mode:
+ #include <dt-bindings/mfd/at91-usart.h>
+
+ spi0: spi@f001c000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "atmel,at91rm9200-usart", "atmel,at91sam9260-usart";
+ atmel,usart-mode = <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
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 <[email protected]>
---
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..0535a256a3c0
--- /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 <[email protected]>
+ *
+ */
+
+#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, "atmel,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 <[email protected]>");
+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 <[email protected]>
+ *
+ */
+
+#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
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 <[email protected]>
---
drivers/spi/Kconfig | 9 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-at91-usart.c | 431 +++++++++++++++++++++++++++++++++++
3 files changed, 441 insertions(+)
create mode 100644 drivers/spi/spi-at91-usart.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 6fb0347a24f2..1a002a32d7aa 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 SPI driver"
+ 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..3ca0fab6691e
--- /dev/null
+++ b/drivers/spi/spi-at91-usart.c
@@ -0,0 +1,431 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for AT91 USART Controllers as SPI
+ *
+ * Copyright (C) 2018 Microchip Technology Inc.
+ * Author: Radu Pirea <[email protected]>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.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_RESET (US_CR_RSTRX | US_CR_RSTTX)
+#define US_DISABLE (US_CR_RXDIS | US_CR_TXDIS)
+#define US_ENABLE (US_CR_RXEN | US_CR_TXEN)
+#define US_OVRE_RXRDY_IRQS (US_IR_OVRE | US_IR_RXRDY)
+
+#define US_INIT (US_MR_SPI_MASTER | US_MR_CHRL | US_MR_CLKO | \
+ US_MR_WRDBT)
+
+/* 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;
+};
+
+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 (remaining)
+ if (at91_usart_spi_tx_ready(aus)) {
+ spi_writeb(aus, THR, tx_buf[len - remaining]);
+ aus->current_tx_remaining_bytes--;
+ }
+}
+
+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 (remaining) {
+ rx_buf[len - remaining] = spi_readb(aus, RHR);
+ aus->current_rx_remaining_bytes--;
+ }
+}
+
+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);
+ u32 *ausd = spi->controller_state;
+ 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) {
+ ausd = kzalloc(sizeof(*ausd), GFP_KERNEL);
+ if (!ausd)
+ return -ENOMEM;
+
+ spi->controller_state = ausd;
+ }
+
+ *ausd = mr;
+
+ dev_dbg(&spi->dev,
+ "setup: bpw %u mode 0x%x -> mr %d %08x\n",
+ bits, spi->mode, spi->chip_select, mr);
+
+ return 0;
+}
+
+int at91_usart_spi_transfer_one(struct spi_controller *ctlr,
+ struct spi_device *spi,
+ struct spi_transfer *xfer)
+{
+ struct at91_usart_spi *aus = spi_master_get_devdata(ctlr);
+
+ 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;
+
+ 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;
+ }
+
+ return 0;
+}
+
+int at91_usart_spi_prepare_message(struct spi_controller *ctlr,
+ struct spi_message *message)
+{
+ struct at91_usart_spi *aus = spi_master_get_devdata(ctlr);
+ struct spi_device *spi = message->spi;
+ u32 *ausd = spi->controller_state;
+
+ spi_writel(aus, CR, US_ENABLE);
+ spi_writel(aus, IER, US_OVRE_RXRDY_IRQS);
+ spi_writel(aus, MR, *ausd);
+
+ return 0;
+}
+
+int at91_usart_spi_unprepare_message(struct spi_controller *ctlr,
+ struct spi_message *message)
+{
+ struct at91_usart_spi *aus = spi_master_get_devdata(ctlr);
+
+ spi_writel(aus, CR, US_RESET | US_DISABLE);
+ spi_writel(aus, IDR, US_OVRE_RXRDY_IRQS);
+ return 0;
+}
+
+static void at91_usart_spi_cleanup(struct spi_device *spi)
+{
+ struct at91_usart_spi_device *ausd = spi->controller_state;
+
+ spi->controller_state = NULL;
+ kfree(ausd);
+}
+
+static void at91_usart_spi_init(struct at91_usart_spi *aus)
+{
+ spi_writel(aus, MR, US_INIT);
+ spi_writel(aus, CR, US_RESET | US_DISABLE);
+}
+
+static int at91_usart_gpio_setup(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.parent->of_node;
+ int i;
+ int ret = 0;
+ int nb = 0;
+
+ if (!np)
+ return -EINVAL;
+
+ nb = of_gpio_named_count(np, "cs-gpios");
+ for (i = 0; i < nb; i++) {
+ int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
+
+ if (cs_gpio < 0)
+ return cs_gpio;
+
+ if (gpio_is_valid(cs_gpio)) {
+ ret = devm_gpio_request_one(&pdev->dev, cs_gpio,
+ GPIOF_DIR_OUT,
+ dev_name(&pdev->dev));
+ if (ret)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+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 -EINVAL;
+
+ 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;
+
+ ret = at91_usart_gpio_setup(pdev);
+ if (ret)
+ 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->setup = at91_usart_spi_setup;
+ controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
+ controller->transfer_one = at91_usart_spi_transfer_one;
+ controller->prepare_message = at91_usart_spi_prepare_message;
+ controller->unprepare_message = at91_usart_spi_unprepare_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 = 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 <[email protected]>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:at91_usart_spi");
--
2.17.0
This patch modifies the place where resources and device tree properties
are searched.
Signed-off-by: Radu Pirea <[email protected]>
---
drivers/tty/serial/Kconfig | 1 +
drivers/tty/serial/atmel_serial.c | 40 +++++++++++++++++--------------
2 files changed, 23 insertions(+), 18 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..740c7d7f4a0c 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
@@ -915,6 +915,7 @@ static void atmel_tx_dma(struct uart_port *port)
static int atmel_prepare_tx_dma(struct uart_port *port)
{
struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ struct device *mfd_dev = port->dev->parent;
dma_cap_mask_t mask;
struct dma_slave_config config;
int ret, nent;
@@ -922,7 +923,7 @@ static int atmel_prepare_tx_dma(struct uart_port *port)
dma_cap_zero(mask);
dma_cap_set(DMA_SLAVE, mask);
- atmel_port->chan_tx = dma_request_slave_channel(port->dev, "tx");
+ atmel_port->chan_tx = dma_request_slave_channel(mfd_dev, "tx");
if (atmel_port->chan_tx == NULL)
goto chan_err;
dev_info(port->dev, "using %s for tx DMA transfers\n",
@@ -1093,6 +1094,7 @@ static void atmel_rx_from_dma(struct uart_port *port)
static int atmel_prepare_rx_dma(struct uart_port *port)
{
struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+ struct device *mfd_dev = port->dev->parent;
struct dma_async_tx_descriptor *desc;
dma_cap_mask_t mask;
struct dma_slave_config config;
@@ -1104,7 +1106,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
dma_cap_zero(mask);
dma_cap_set(DMA_CYCLIC, mask);
- atmel_port->chan_rx = dma_request_slave_channel(port->dev, "rx");
+ atmel_port->chan_rx = dma_request_slave_channel(mfd_dev, "rx");
if (atmel_port->chan_rx == NULL)
goto chan_err;
dev_info(port->dev, "using %s for rx DMA transfers\n",
@@ -1631,7 +1633,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")) {
@@ -2222,8 +2224,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;
+ struct platform_device *mpdev = to_platform_device(port->dev->parent);
+ int size = resource_size(mpdev->resource);
release_mem_region(port->mapbase, size);
@@ -2238,8 +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;
+ struct platform_device *mpdev = to_platform_device(port->dev->parent);
+ int size = resource_size(mpdev->resource);
if (!request_mem_region(port->mapbase, size, "atmel_serial"))
return -EBUSY;
@@ -2341,27 +2343,28 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
{
int ret;
struct uart_port *port = &atmel_port->uart;
+ struct platform_device *mpdev = to_platform_device(pdev->dev.parent);
atmel_init_property(atmel_port, pdev);
atmel_set_ops(port);
- uart_get_rs485_mode(&pdev->dev, &port->rs485);
+ uart_get_rs485_mode(&mpdev->dev, &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 = mpdev->resource[0].start;
+ port->irq = mpdev->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(&mpdev->dev, "usart");
if (IS_ERR(atmel_port->clk)) {
ret = PTR_ERR(atmel_port->clk);
atmel_port->clk = NULL;
@@ -2652,11 +2655,13 @@ static int atmel_serial_resume(struct platform_device *pdev)
static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
struct platform_device *pdev)
{
+ struct device *dev = pdev->dev.parent;
+
atmel_port->fifo_size = 0;
atmel_port->rts_low = 0;
atmel_port->rts_high = 0;
- if (of_property_read_u32(pdev->dev.of_node,
+ if (of_property_read_u32(dev->of_node,
"atmel,fifo-size",
&atmel_port->fifo_size))
return;
@@ -2694,11 +2699,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 +2849,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
Added entry for at91 usart mfd driver.
Signed-off-by: Radu Pirea <[email protected]>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 12203d07c6af..dae31df711fb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9201,6 +9201,13 @@ F: drivers/mfd/at91-usart.c
F: include/dt-bindings/mfd/at91-usart.h
F: Documentation/devicetree/bindings/mfd/atmel-usart.txt
+MICROCHIP AT91 USART SPI DRIVER
+M: Radu Pirea <[email protected]>
+L: [email protected]
+S: Supported
+F: drivers/spi/spi-at91-usart.c
+F: Documentation/devicetree/bindings/mfd/atmel-usart.txt
+
MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
M: Woojung Huh <[email protected]>
M: Microchip Linux Driver Support <[email protected]>
--
2.17.0
Added entry for at91 usart mfd driver.
Signed-off-by: Radu Pirea <[email protected]>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 8e2a2fddbd19..12203d07c6af 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9160,6 +9160,7 @@ M: Richard Genoud <[email protected]>
S: Maintained
F: drivers/tty/serial/atmel_serial.c
F: drivers/tty/serial/atmel_serial.h
+F: Documentation/devicetree/bindings/mfd/atmel-usart.txt
MICROCHIP / ATMEL DMA DRIVER
M: Ludovic Desroches <[email protected]>
@@ -9192,6 +9193,14 @@ S: Supported
F: drivers/mtd/nand/raw/atmel/*
F: Documentation/devicetree/bindings/mtd/atmel-nand.txt
+MICROCHIP AT91 USART MFD DRIVER
+M: Radu Pirea <[email protected]>
+L: [email protected]
+S: Supported
+F: drivers/mfd/at91-usart.c
+F: include/dt-bindings/mfd/at91-usart.h
+F: Documentation/devicetree/bindings/mfd/atmel-usart.txt
+
MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
M: Woojung Huh <[email protected]>
M: Microchip Linux Driver Support <[email protected]>
--
2.17.0
On Fri, May 25, 2018 at 8:19 PM, Radu Pirea <[email protected]> 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.
> + depends on OF
What makes this driver OF specific?
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
Ditto.
> +#include <linux/pinctrl/pinctrl.h>
How exactly is this used?
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
Ditto.
> +#include <linux/types.h>
> +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, "atmel,usart-mode", &opmode);
Check unified device property API.
> + return mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO, &cell, 1,
> + NULL, 0, NULL);
No devm_ and no ->remove(). Why to leak resources?
--
With Best Regards,
Andy Shevchenko
On Fri, May 25, 2018 at 8:19 PM, Radu Pirea <[email protected]> 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.
>
> The driver was tested on sama5d3-xplained and sama5d4-xplained boards with
> enc28j60 ethernet controller as slave.
> +#include <linux/of_gpio.h>
What is the use of it?
> +#define US_INIT (US_MR_SPI_MASTER | US_MR_CHRL | US_MR_CLKO | \
> + US_MR_WRDBT)
Don't split lines like this, it's hard to read.
#define FOO \
(BAR1 | BAR2)
I think I already told this to someone recently, maybe to you.
> +/* 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)
Names are too generic. You better to use the same prefix as for the
rest, i.e. at91_spi_
> + /*used in interrupt to protect data reading*/
Comment style.
You need to read some existing code, perhaps, to see how it's done.
> +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 (remaining)
> + if (at91_usart_spi_tx_ready(aus)) {
if (x) {
if (y) {
...
}
}
is equivalent to if (x && y) {}.
Though, considering your intention here, I would rather go with better
pattern, i.e.
if (!remaining)
return;
> + spi_writeb(aus, THR, tx_buf[len - remaining]);
> + aus->current_tx_remaining_bytes--;
> + }
> +}
> +
> +static inline void at91_usart_spi_rx(struct at91_usart_spi *aus)
> +{
> + if (remaining) {
> + rx_buf[len - remaining] = spi_readb(aus, RHR);
> + aus->current_rx_remaining_bytes--;
> + }
Ditto.
> +}
> +static int at91_usart_gpio_setup(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.parent->of_node;
Your driver is not OF specific as far as I can see. Drop all these
device_node stuff and change API calls respectively.
> + int i;
> + int ret = 0;
> + int nb = 0;
What happened to indentation?
Redundnant assignment for both.
> + if (!np)
> + return -EINVAL;
> +
> + nb = of_gpio_named_count(np, "cs-gpios");
> + for (i = 0; i < nb; i++) {
> + int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
> +
> + if (cs_gpio < 0)
> + return cs_gpio;
> +
> + if (gpio_is_valid(cs_gpio)) {
> + ret = devm_gpio_request_one(&pdev->dev, cs_gpio,
> + GPIOF_DIR_OUT,
> + dev_name(&pdev->dev));
> + if (ret)
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int at91_usart_spi_probe(struct platform_device *pdev)
> +{
> + regs = platform_get_resource(to_platform_device(pdev->dev.parent),
> + IORESOURCE_MEM, 0);
> + if (!regs)
> + return -EINVAL;
This looks weird. Supply resource to _this_ device in your MFD code.
> + 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);
I think I already told you, don't use explicit casting when print.
If it wasn't you, do you homework then. But above is no go.
> + return 0;
> +static struct platform_driver at91_usart_spi_driver = {
> + .driver = {
> + .name = "at91_usart_spi",
> + .of_match_table = of_match_ptr(at91_usart_spi_dt_ids),
Drop of_match_ptr(). It's not needed.
> + },
> + .probe = at91_usart_spi_probe,
> + .remove = at91_usart_spi_remove, };
Already told ya, split lines correctly.
--
With Best Regards,
Andy Shevchenko
On 25/05/2018 19:19, Radu Pirea wrote:
> This patch modifies the place where resources and device tree properties
> are searched.
>
> Signed-off-by: Radu Pirea <[email protected]>
> ---
> drivers/tty/serial/Kconfig | 1 +
> drivers/tty/serial/atmel_serial.c | 40 +++++++++++++++++--------------
> 2 files changed, 23 insertions(+), 18 deletions(-)
the stdout-path property of the chosen node is still broken in this verion.
if :
stdout-path = "serial0:115200n8";
is set in the DTS, the console output should go on serial0 (aka dbgu)
cf Documentation/devicetree/bindings/chosen.txt
With this patch applied, this is not the case anymore.
Adding console=ttyS0,115200 in the chosen node is not the solution here.
regards,
Richard.
On 05/28/2018 01:08 PM, Richard Genoud wrote:
> On 25/05/2018 19:19, Radu Pirea wrote:
>> This patch modifies the place where resources and device tree properties
>> are searched.
>>
>> Signed-off-by: Radu Pirea <[email protected]>
>> ---
>> drivers/tty/serial/Kconfig | 1 +
>> drivers/tty/serial/atmel_serial.c | 40 +++++++++++++++++--------------
>> 2 files changed, 23 insertions(+), 18 deletions(-)
> the stdout-path property of the chosen node is still broken in this verion.
> if :
> stdout-path = "serial0:115200n8";
> is set in the DTS, the console output should go on serial0 (aka dbgu)
> cf Documentation/devicetree/bindings/chosen.txt
>
> With this patch applied, this is not the case anymore.
> Adding console=ttyS0,115200 in the chosen node is not the solution here.
>
I fixed the issue few minutes ago :)
I hope DMA setup is ok now.
>
> regards,
> Richard.
>
On 28/05/2018 11:00:49+0300, Andy Shevchenko wrote:
> On Fri, May 25, 2018 at 8:19 PM, Radu Pirea <[email protected]> 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.
>
> > + depends on OF
>
> What makes this driver OF specific?
>
> > + err = of_property_read_u32(np, "atmel,usart-mode", &opmode);
>
> Check unified device property API.
>
We don't care, this makes the driver OF specific and this is the correct
thing to do.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
On 05/28/2018 11:21 AM, Andy Shevchenko wrote:
> On Fri, May 25, 2018 at 8:19 PM, Radu Pirea <[email protected]> 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.
>>
>> The driver was tested on sama5d3-xplained and sama5d4-xplained boards with
>> enc28j60 ethernet controller as slave.
>
>> +#include <linux/of_gpio.h>
>
> What is the use of it?
I need of_gpio.h for of_gpio_named_count, of_get_named_gpio and
devm_gpio_request_one(found in gpio.h)
>
>> +#define US_INIT (US_MR_SPI_MASTER | US_MR_CHRL | US_MR_CLKO | \
>> + US_MR_WRDBT)
>
> Don't split lines like this, it's hard to read.
>
> #define FOO \
> (BAR1 | BAR2)
I'll fix it.
>
> I think I already told this to someone recently, maybe to you.
>
>> +/* 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)
>
> Names are too generic. You better to use the same prefix as for the
> rest, i.e. at91_spi_
Good ideea. I will change the names.
>
>> + /*used in interrupt to protect data reading*/
>
> Comment style.
>
> You need to read some existing code, perhaps, to see how it's done.
Ok. I will add the comment.
>
>> +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 (remaining)
>> + if (at91_usart_spi_tx_ready(aus)) {
>
> if (x) {
> if (y) {
> ...
> }
> }
>
> is equivalent to if (x && y) {}.
>
> Though, considering your intention here, I would rather go with better
> pattern, i.e.
>
> if (!remaining)
> return;
Thank for suggestion. I will change.
>
>> + spi_writeb(aus, THR, tx_buf[len - remaining]);
>> + aus->current_tx_remaining_bytes--;
>> + }
>> +}
>> +
>> +static inline void at91_usart_spi_rx(struct at91_usart_spi *aus)
>> +{
>
>> + if (remaining) {
>> + rx_buf[len - remaining] = spi_readb(aus, RHR);
>> + aus->current_rx_remaining_bytes--;
>> + }
>
> Ditto.
>
>> +}
>
>
>> +static int at91_usart_gpio_setup(struct platform_device *pdev)
>> +{
>
>> + struct device_node *np = pdev->dev.parent->of_node;
>
> Your driver is not OF specific as far as I can see. Drop all these
> device_node stuff and change API calls respectively.
Ok. What do you suggest to use instead of OF API to get the count of
cs-gpios and to read their values one by one?
>
>> + int i;
>
>> + int ret = 0;
>> + int nb = 0;
>
> What happened to indentation?
>
> Redundnant assignment for both.
>
>> + if (!np)
>> + return -EINVAL;
>> +
>> + nb = of_gpio_named_count(np, "cs-gpios");
>> + for (i = 0; i < nb; i++) {
>> + int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
>> +
>> + if (cs_gpio < 0)
>> + return cs_gpio;
>> +
>> + if (gpio_is_valid(cs_gpio)) {
>> + ret = devm_gpio_request_one(&pdev->dev, cs_gpio,
>> + GPIOF_DIR_OUT,
>> + dev_name(&pdev->dev));
>> + if (ret)
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int at91_usart_spi_probe(struct platform_device *pdev)
>> +{
>
>> + regs = platform_get_resource(to_platform_device(pdev->dev.parent),
>> + IORESOURCE_MEM, 0);
>> + if (!regs)
>> + return -EINVAL;
>
> This looks weird. Supply resource to _this_ device in your MFD code.
I know weird, but is the safest way to pass the resource and the of_node.
>
>> + 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);
>
> I think I already told you, don't use explicit casting when print.
> If it wasn't you, do you homework then. But above is no go. >
>> + return 0;
>
>> +static struct platform_driver at91_usart_spi_driver = {
>> + .driver = {
>> + .name = "at91_usart_spi",
>
>> + .of_match_table = of_match_ptr(at91_usart_spi_dt_ids),
>
> Drop of_match_ptr(). It's not needed.
>
>> + },
>> + .probe = at91_usart_spi_probe,
>
>> + .remove = at91_usart_spi_remove, };
>
> Already told ya, split lines correctly.
>
On 29/05/2018 at 16:28, Radu Pirea wrote:
>
>
> On 05/28/2018 11:21 AM, Andy Shevchenko wrote:
>> On Fri, May 25, 2018 at 8:19 PM, Radu Pirea <[email protected]> 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.
[..]
>>> +static int at91_usart_gpio_setup(struct platform_device *pdev)
>>> +{
>>
>>> + struct device_node *np = pdev->dev.parent->of_node;
>>
>> Your driver is not OF specific as far as I can see. Drop all these
>> device_node stuff and change API calls respectively.
>
> Ok. What do you suggest to use instead of OF API to get the count of
> cs-gpios and to read their values one by one?
As Alexandre said, we can make this driver OF specific.
What could be interesting is to use the gpio descriptors API and not the
older one. This would allow us to have far more control over the gpio
that we use in our drivers (Ludovic is converting our drivers to only
use gpiod structures).
Regards,
Nicolas
>
>>
>>> + int i;
>>
>>> + int ret = 0;
>>> + int nb = 0;
>>
>> What happened to indentation?
>>
>> Redundnant assignment for both.
>>
>>> + if (!np)
>>> + return -EINVAL;
>>> +
>>> + nb = of_gpio_named_count(np, "cs-gpios");
>>> + for (i = 0; i < nb; i++) {
>>> + int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
>>> +
>>> + if (cs_gpio < 0)
>>> + return cs_gpio;
>>> +
>>> + if (gpio_is_valid(cs_gpio)) {
>>> + ret = devm_gpio_request_one(&pdev->dev, cs_gpio,
>>> + GPIOF_DIR_OUT,
>>> + dev_name(&pdev->dev));
>>> + if (ret)
>>> + return ret;
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
[..]
--
Nicolas Ferre
On 29/05/2018 at 16:34, Nicolas Ferre wrote:
> On 29/05/2018 at 16:28, Radu Pirea wrote:
>>
>>
>> On 05/28/2018 11:21 AM, Andy Shevchenko wrote:
>>> On Fri, May 25, 2018 at 8:19 PM, Radu Pirea <[email protected]> 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.
>
> [..]
>
>>>> +static int at91_usart_gpio_setup(struct platform_device *pdev)
>>>> +{
>>>
>>>> + struct device_node *np = pdev->dev.parent->of_node;
>>>
>>> Your driver is not OF specific as far as I can see. Drop all these
>>> device_node stuff and change API calls respectively.
>>
>> Ok. What do you suggest to use instead of OF API to get the count of
>> cs-gpios and to read their values one by one?
>
> As Alexandre said, we can make this driver OF specific.
>
> What could be interesting is to use the gpio descriptors API and not the
> older one. This would allow us to have far more control over the gpio
> that we use in our drivers (Ludovic is converting our drivers to only
> use gpiod structures).
Oh, but we already said that we cannot.
Disregard my comment then, sorry for the noise.
> Regards,
> Nicolas
>
>>
>>>
>>>> + int i;
>>>
>>>> + int ret = 0;
>>>> + int nb = 0;
>>>
>>> What happened to indentation?
>>>
>>> Redundnant assignment for both.
>>>
>>>> + if (!np)
>>>> + return -EINVAL;
>>>> +
>>>> + nb = of_gpio_named_count(np, "cs-gpios");
>>>> + for (i = 0; i < nb; i++) {
>>>> + int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
>>>> +
>>>> + if (cs_gpio < 0)
>>>> + return cs_gpio;
>>>> +
>>>> + if (gpio_is_valid(cs_gpio)) {
>>>> + ret = devm_gpio_request_one(&pdev->dev, cs_gpio,
>>>> + GPIOF_DIR_OUT,
>>>> + dev_name(&pdev->dev));
>>>> + if (ret)
>>>> + return ret;
>>>> + }
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
> [..]
>
--
Nicolas Ferre
On Fri, May 25, 2018 at 08:19:37PM +0300, Radu Pirea wrote:
> This patch moves the bindings for serial from serial/atmel-usart.txt to
> mfd/atmel-usart.txt and adds bindings for USART in SPI mode.
>
> Signed-off-by: Radu Pirea <[email protected]>
> ---
> .../bindings/{serial => mfd}/atmel-usart.txt | 25 +++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
> rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
Reviewed-by: Rob Herring <[email protected]>
On Fri, May 25, 2018 at 08:19:38PM +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 <[email protected]>
> ---
> drivers/mfd/Kconfig | 10 ++++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/at91-usart.c | 75 ++++++++++++++++++++++++++++
> include/dt-bindings/mfd/at91-usart.h | 17 +++++++
This really should be part of the binding patch.
Acked-by: Rob Herring <[email protected]>
> 4 files changed, 103 insertions(+)
> create mode 100644 drivers/mfd/at91-usart.c
> create mode 100644 include/dt-bindings/mfd/at91-usart.h
On Fri, 25 May 2018, Radu Pirea wrote:
> This patch moves the bindings for serial from serial/atmel-usart.txt to
> mfd/atmel-usart.txt and adds bindings for USART in SPI mode.
>
> Signed-off-by: Radu Pirea <[email protected]>
> ---
> .../bindings/{serial => mfd}/atmel-usart.txt | 25 +++++++++++++++++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
> rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
For my own reference:
Acked-for-MFD-by: Lee Jones <[email protected]>
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog