* Re: [v2 3/4] regulator: qcom: Add labibb driver
From: Mark Brown @ 2020-05-27 16:39 UTC (permalink / raw)
To: Sumit Semwal
Cc: agross, Bjorn Andersson, lgirdwood, robh+dt, Nisha Kumari,
linux-arm-msm, LKML, devicetree, kgunda, Rajendra Nayak
In-Reply-To: <CAO_48GF0tjZDmTS+Fa4fv+cfH4skFZP_a9A=P7D0b_si4AFj5A@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 722 bytes --]
On Wed, May 27, 2020 at 10:01:27PM +0530, Sumit Semwal wrote:
> On Thu, 14 May 2020 at 16:57, Sumit Semwal <sumit.semwal@linaro.org> wrote:
> > > If this is useful factor it out into a helper or the core, other devices
> > > also have status bits saying if the regulator is enabled. It looks like
> > > this may be mainly trying to open code something like enable_time, with
> > > possibly some issues where the time taken to enable varies a lot.
> > Makes sense; I am not terribly familiar with the regulator core and
> > helpers, so let me look and refactor accordingly.
> Does something like this make sense, or did I misunderstand your
> suggestion completely? I'll send the updated patches accordingly.
I guess.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v5 03/14] PCI: cadence: Convert all r/w accessors to perform only 32-bit accesses
From: Rob Herring @ 2020-05-27 16:37 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Tom Joseph, Lorenzo Pieralisi, Bjorn Helgaas, PCI,
linux-kernel@vger.kernel.org, Arnd Bergmann, Greg Kroah-Hartman,
devicetree, linux-omap,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <b3663862-44df-867f-0824-28802909f224@ti.com>
On Wed, May 27, 2020 at 4:49 AM Kishon Vijay Abraham I <kishon@ti.com> wrote:
>
> Hi Rob,
>
> On 5/26/2020 8:42 PM, Rob Herring wrote:
> > On Sun, May 24, 2020 at 9:30 PM Kishon Vijay Abraham I <kishon@ti.com> wrote:
> >>
> >> Hi Rob,
> >>
> >> On 5/22/2020 9:24 PM, Rob Herring wrote:
> >>> On Thu, May 21, 2020 at 9:37 PM Kishon Vijay Abraham I <kishon@ti.com> wrote:
> >>>>
> >>>> Certain platforms like TI's J721E using Cadence PCIe IP can perform only
> >>>> 32-bit accesses for reading or writing to Cadence registers. Convert all
> >>>> read and write accesses to 32-bit in Cadence PCIe driver in preparation
> >>>> for adding PCIe support in TI's J721E SoC.
> >>>
> >>> Looking more closely I don't think cdns_pcie_ep_assert_intx is okay
> >>> with this and never can be given the PCI_COMMAND and PCI_STATUS
> >>> registers are in the same word (IIRC, that's the main reason 32-bit
> >>> config space accesses are broken). So this isn't going to work at
> >>
> >> right, PCI_STATUS has write '1' to clear bits and there's a chance that it
> >> could be reset while raising legacy interrupt. While this cannot be avoided for
> >> TI's J721E, other platforms doesn't have to have this limitation.
> >>> least for EP accesses. And maybe you need a custom .raise_irq() hook
> >>> to minimize any problems (such as making the RMW atomic at least from
> >>> the endpoint's perspective).
> >>
> >> This is to make sure EP doesn't update in-consistent state when RC is updating
> >> the PCI_STATUS register? Since this involves two different systems, how do we
> >> make this atomic?
> >
> > You can't make it atomic WRT both systems, but is there locking around
> > each RMW? Specifically, are preemption and interrupts disabled to
> > ensure time between a read and write are minimized? You wouldn't want
> > interrupts disabled during the delay too though (i.e. around
> > .raise_irq()).
>
> Okay, I'll add spin spin_lock_irqsave() in cdns_pcie_write_sz(). As you also
> pointed below that delay for legacy interrupt is wrong and it has to be fixed
> (with a later series).
But you don't need a lock everywhere. You need locks in the callers
(and only sometimes).
> How do you want to handle cdns_pcie_ep_fn_writew() now? Because now we are
> changing the default implementation to perform only 32-bit access (used for
> legacy interrupt, msi-x interrupt and while writing standard headers) and it's
> not okay only for legacy interrupts for platforms other than TI.
Now I'm wondering how set_msi is not racy in the current code with the
host setting/clearing PCI_MSI_FLAGS_ENABLE? Maybe that bit is RO from
the EP side?
Ultimately I think you're going to have to provide your own endpoint
functions or you need accessors for specific registers like
PCI_MSI_FLAGS. Then for example, you just rely on the 2 bytes before
PCI_MSI_FLAGS being reserved and do a 32-bit access without a RMW.
Trying to abstract this at the register read/write level is going to
be fragile.
Rob
^ permalink raw reply
* RE: [PATCH 16/17] dt-bindings: watchdog: renesas,wdt: Document r8a7742 support
From: Prabhakar Mahadev Lad @ 2020-05-27 16:33 UTC (permalink / raw)
To: Rob Herring, Lad, Prabhakar
Cc: Geert Uytterhoeven, Jens Axboe, Wolfram Sang, Ulf Hansson,
Sergei Shtylyov, David S. Miller, Wim Van Sebroeck, Guenter Roeck,
open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers),
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML,
Linux I2C, Linux MMC List, netdev, Linux-Renesas,
Linux Watchdog Mailing List
In-Reply-To: <CAL_JsqJUn9iOy5FT6VRmsC-uAhSdN8_Sne0Vn_7Q1dHudbzopw@mail.gmail.com>
Hi Rob,
> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 27 May 2020 15:38
> To: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; Geert Uytterhoeven <geert+renesas@glider.be>; Jens Axboe
> <axboe@kernel.dk>; Wolfram Sang <wsa+renesas@sang-engineering.com>; Ulf Hansson <ulf.hansson@linaro.org>; Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com>; David S. Miller <davem@davemloft.net>; Wim Van Sebroeck <wim@linux-watchdog.org>;
> Guenter Roeck <linux@roeck-us.net>; open list:LIBATA SUBSYSTEM (Serial and Parallel ATA drivers) <linux-ide@vger.kernel.org>; open
> list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>; LKML <linux-kernel@vger.kernel.org>; Linux
> I2C <linux-i2c@vger.kernel.org>; Linux MMC List <linux-mmc@vger.kernel.org>; netdev <netdev@vger.kernel.org>; Linux-Renesas <linux-
> renesas-soc@vger.kernel.org>; Linux Watchdog Mailing List <linux-watchdog@vger.kernel.org>
> Subject: Re: [PATCH 16/17] dt-bindings: watchdog: renesas,wdt: Document r8a7742 support
>
> On Wed, May 27, 2020 at 5:23 AM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> >
> > Hi Rob,
> >
> > On Wed, May 27, 2020 at 2:31 AM Rob Herring <robh@kernel.org> wrote:
> > >
> > > On Fri, May 15, 2020 at 04:08:56PM +0100, Lad Prabhakar wrote:
> > > > RZ/G1H (R8A7742) watchdog implementation is compatible with R-Car Gen2,
> > > > therefore add relevant documentation.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
> > > > ---
> > > > Documentation/devicetree/bindings/watchdog/renesas,wdt.txt | 1 +
> > > > 1 file changed, 1 insertion(+)
> > >
> > > Meanwhile in the DT tree, converting this schema landed. Can you prepare
> > > a version based on the schema.
> > >
> > This was kindly taken care by Stephen during merge in linux-next [1].
>
> Yes, I'm aware of that. I was hoping for a better commit message which
> stands on its own (essentially the one here).
>
As requested I have posted a patch [1].
[1] https://lore.kernel.org/patchwork/patch/1248597/
Cheers,
--Prabhakar
Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647
^ permalink raw reply
* Re: [v2 3/4] regulator: qcom: Add labibb driver
From: Sumit Semwal @ 2020-05-27 16:31 UTC (permalink / raw)
To: Mark Brown
Cc: agross, Bjorn Andersson, lgirdwood, robh+dt, Nisha Kumari,
linux-arm-msm, LKML, devicetree, kgunda, Rajendra Nayak
In-Reply-To: <CAO_48GFGpHeu_xb9XT9CFMOSUOJgRrb-z_KZ3-r3X78s-2ddjw@mail.gmail.com>
Hello Mark,
On Thu, 14 May 2020 at 16:57, Sumit Semwal <sumit.semwal@linaro.org> wrote:
>
> Hello Mark,
>
> Thank you for your review comments!
> On Mon, 11 May 2020 at 16:09, Mark Brown <broonie@kernel.org> wrote:
> >
> > On Sat, May 09, 2020 at 02:11:59AM +0530, Sumit Semwal wrote:
> >
> > > + ret = regmap_bulk_read(reg->regmap, reg->base +
> > > + REG_LABIBB_STATUS1, &val, 1);
> > > + if (ret < 0) {
> > > + dev_err(reg->dev, "Read register failed ret = %d\n", ret);
> > > + return ret;
> > > + }
> >
> > Why a bulk read of a single register?
> Right, will change.
> >
> > > +static int _check_enabled_with_retries(struct regulator_dev *rdev,
> > > + int retries, int enabled)
> > > +{
> >
> > This is not retrying, this is polling to see if the regulator actually
> > enabled.
> Yes, will update accordingly.
>
> >
> > > +static int qcom_labibb_regulator_enable(struct regulator_dev *rdev)
> > > +{
> >
> > > + ret = _check_enabled_with_retries(rdev, retries, 1);
> > > + if (ret < 0) {
> > > + dev_err(reg->dev, "retries exhausted: enable %s regulator\n",
> > > + reg->desc.name);
> > > + return ret;
> > > + }
> >
> > If this is useful factor it out into a helper or the core, other devices
> > also have status bits saying if the regulator is enabled. It looks like
> > this may be mainly trying to open code something like enable_time, with
> > possibly some issues where the time taken to enable varies a lot.
> >
> Makes sense; I am not terribly familiar with the regulator core and
> helpers, so let me look and refactor accordingly.
Does something like this make sense, or did I misunderstand your
suggestion completely? I'll send the updated patches accordingly.
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -2353,6 +2353,7 @@ static int _regulator_do_enable(struct
regulator_dev *rdev)
+ /* If max_time_poll_enabled is set for the regulator,
+ * Poll upto max_time_poll_enabled time to see if the regulator
+ * actually got enabled.
+ * For each iteration, wait for the enable_time delay calculated
+ * above already.
+ * If the regulator isn't enabled after max_time_poll_enabled has
+ * expired, return -REG_ENABLED_CHECK_FAILED.
+ */
+ if (rdev->desc->max_time_poll_enabled) {
+ unsigned int remaining_time_to_poll =
rdev->desc->max_time_poll_enabled;
+
+ while (remaining_time_to_poll > 0) {
+ /* We've already waited for enable_time above;
+ * so we can start with immediate check of the
+ * status of the regulator.
+ */
+ if (rdev->desc->ops->is_enabled(rdev))
+ break;
+
+ _regulator_enable_delay(delay);
+ remaining_time_to_poll -= delay;
+ }
+
+ if (remaining_time_to_poll <= 0) {
+ rdev_err(rdev, "Enabled check failed.\n");
+ return -REG_ENABLED_CHECK_FAILED;
+ }
+ }
+
Since atleast in my use case, the delay is really enable_time (time
before we could check the status register), we could reuse the
already-calculated 'delay' based on enable_time?
>
<snip>
Best,
Sumit.
^ permalink raw reply
* [PATCH v2] dt-bindings: watchdog: renesas,wdt: Document r8a7742 support
From: Lad Prabhakar @ 2020-05-27 16:29 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck, Rob Herring, linux-watchdog,
devicetree, linux-kernel
Cc: Lad Prabhakar
RZ/G1H (R8A7742) watchdog implementation is compatible with R-Car Gen2,
therefore add relevant documentation.
Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
Hi,
This patch is part of series [1], as requested by Rob [1] I have
reabsed my changes on-top json-schema conversion patch.
[1] https://www.spinics.net/lists/netdev/msg653258.html
[2] https://patchwork.kernel.org/patch/11552335/
Cheers,
Prabhakar
---
Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml b/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml
index 27e8c4a..572f4c9 100644
--- a/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml
+++ b/Documentation/devicetree/bindings/watchdog/renesas,wdt.yaml
@@ -24,6 +24,7 @@ properties:
- items:
- enum:
+ - renesas,r8a7742-wdt # RZ/G1H
- renesas,r8a7743-wdt # RZ/G1M
- renesas,r8a7744-wdt # RZ/G1N
- renesas,r8a7745-wdt # RZ/G1E
--
2.7.4
^ permalink raw reply related
* [PATCH v6 5/5] drivers/tty/serial: add LiteUART driver
From: Mateusz Holenko @ 2020-05-27 16:27 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Jiri Slaby,
devicetree, linux-serial
Cc: Stafford Horne, Karol Gugala, Mateusz Holenko,
Mauro Carvalho Chehab, David S. Miller, Paul E. McKenney,
Filip Kokosinski, Pawel Czarnecki, Joel Stanley, Jonathan Cameron,
Maxime Ripard, Shawn Guo, Heiko Stuebner, Sam Ravnborg,
Icenowy Zheng, Laurent Pinchart, linux-kernel, Gabriel L. Somlo
In-Reply-To: <20200527182545.3859622-0-mholenko@antmicro.com>
From: Filip Kokosinski <fkokosinski@antmicro.com>
This commit adds driver for the FPGA-based LiteUART serial controller
from LiteX SoC builder.
The current implementation supports LiteUART configured
for 32 bit data width and 8 bit CSR bus width.
It does not support IRQ.
Signed-off-by: Filip Kokosinski <fkokosinski@antmicro.com>
Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
---
Notes:
Changes in v6:
- LiteUART ports now stored in xArray
- removed PORT_LITEUART
- fixed formatting
- removed some unnecessary defines
No changes in v5.
Changes in v4:
- fixed copyright header
- removed a wrong dependency on UARTLITE from Kconfig
- added a dependency on LITEX_SOC_CONTROLLER to LITEUART in Kconfig
Changes in v3:
- aliases made optional
- used litex_get_reg/litex_set_reg functions instead of macros
- SERIAL_LITEUART_NR_PORTS renamed to SERIAL_LITEUART_MAX_PORTS
- PORT_LITEUART changed from 122 to 123
- added dependency on LITEX_SOC_CONTROLLER
- patch number changed from 4 to 5
No changes in v2.
MAINTAINERS | 1 +
drivers/tty/serial/Kconfig | 31 +++
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/liteuart.c | 404 ++++++++++++++++++++++++++++++++++
4 files changed, 437 insertions(+)
create mode 100644 drivers/tty/serial/liteuart.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 51d2d6a61fb0..d855fe807833 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9846,6 +9846,7 @@ M: Mateusz Holenko <mholenko@antmicro.com>
S: Maintained
F: Documentation/devicetree/bindings/*/litex,*.yaml
F: drivers/soc/litex/litex_soc_ctrl.c
+F: drivers/tty/serial/liteuart.c
F: include/linux/litex.h
LIVE PATCHING
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index adf9e80e7dc9..17aaf0afb27a 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1562,6 +1562,37 @@ config SERIAL_MILBEAUT_USIO_CONSOLE
receives all kernel messages and warnings and which allows logins in
single user mode).
+config SERIAL_LITEUART
+ tristate "LiteUART serial port support"
+ depends on HAS_IOMEM
+ depends on OF || COMPILE_TEST
+ depends on LITEX_SOC_CONTROLLER
+ select SERIAL_CORE
+ help
+ This driver is for the FPGA-based LiteUART serial controller from LiteX
+ SoC builder.
+
+ Say 'Y' here if you wish to use the LiteUART serial controller.
+ Otherwise, say 'N'.
+
+config SERIAL_LITEUART_MAX_PORTS
+ int "Maximum number of LiteUART ports"
+ depends on SERIAL_LITEUART
+ default "1"
+ help
+ Set this to the maximum number of serial ports you want the driver
+ to support.
+
+config SERIAL_LITEUART_CONSOLE
+ bool "LiteUART serial port console support"
+ depends on SERIAL_LITEUART=y
+ select SERIAL_CORE_CONSOLE
+ help
+ Say 'Y' here if you wish to use the FPGA-based LiteUART serial controller
+ from LiteX SoC builder as the system console (the system console is the
+ device which receives all kernel messages and warnings and which allows
+ logins in single user mode). Otherwise, say 'N'.
+
endmenu
config SERIAL_MCTRL_GPIO
diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
index d056ee6cca33..9f8ba419ff3b 100644
--- a/drivers/tty/serial/Makefile
+++ b/drivers/tty/serial/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_SERIAL_OWL) += owl-uart.o
obj-$(CONFIG_SERIAL_RDA) += rda-uart.o
obj-$(CONFIG_SERIAL_MILBEAUT_USIO) += milbeaut_usio.o
obj-$(CONFIG_SERIAL_SIFIVE) += sifive.o
+obj-$(CONFIG_SERIAL_LITEUART) += liteuart.o
# GPIOLIB helpers for modem control lines
obj-$(CONFIG_SERIAL_MCTRL_GPIO) += serial_mctrl_gpio.o
diff --git a/drivers/tty/serial/liteuart.c b/drivers/tty/serial/liteuart.c
new file mode 100644
index 000000000000..22b7612c13ca
--- /dev/null
+++ b/drivers/tty/serial/liteuart.c
@@ -0,0 +1,404 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LiteUART serial controller (LiteX) Driver
+ *
+ * Copyright (C) 2019-2020 Antmicro <www.antmicro.com>
+ */
+
+#include <linux/console.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/serial.h>
+#include <linux/serial_core.h>
+#include <linux/timer.h>
+#include <linux/tty_flip.h>
+#include <linux/litex.h>
+#include <linux/xarray.h>
+
+/*
+ * CSRs definitions (base address offsets + width)
+ *
+ * The definitions below are true for LiteX SoC configured for 8-bit CSR Bus,
+ * 32-bit aligned.
+ *
+ * Supporting other configurations might require new definitions or a more
+ * generic way of indexing the LiteX CSRs.
+ *
+ * For more details on how CSRs are defined and handled in LiteX, see comments
+ * in the LiteX SoC Driver: drivers/soc/litex/litex_soc_ctrl.c
+ */
+#define OFF_RXTX 0x00
+#define OFF_TXFULL 0x04
+#define OFF_RXEMPTY 0x08
+#define OFF_EV_STATUS 0x0c
+#define OFF_EV_PENDING 0x10
+#define OFF_EV_ENABLE 0x14
+
+/* events */
+#define EV_TX 0x1
+#define EV_RX 0x2
+
+struct liteuart_port {
+ struct uart_port port;
+ struct timer_list timer;
+};
+
+#define to_liteuart_port(port) container_of(port, struct liteuart_port, port)
+
+static DEFINE_XARRAY_FLAGS(liteuart_array, XA_FLAGS_ALLOC);
+
+#ifdef CONFIG_SERIAL_LITEUART_CONSOLE
+static struct console liteuart_console;
+#endif
+
+static struct uart_driver liteuart_driver = {
+ .owner = THIS_MODULE,
+ .driver_name = "liteuart",
+ .dev_name = "ttyLXU",
+ .major = 0,
+ .minor = 0,
+ .nr = CONFIG_SERIAL_LITEUART_MAX_PORTS,
+#ifdef CONFIG_SERIAL_LITEUART_CONSOLE
+ .cons = &liteuart_console,
+#endif
+};
+
+static void liteuart_timer(struct timer_list *t)
+{
+ struct liteuart_port *uart = from_timer(uart, t, timer);
+ struct uart_port *port = &uart->port;
+ unsigned char __iomem *membase = port->membase;
+ unsigned int flg = TTY_NORMAL;
+ int ch;
+ unsigned long status;
+
+ while ((status = !litex_get_reg(membase + OFF_RXEMPTY, 1)) == 1) {
+ ch = litex_get_reg(membase + OFF_RXTX, 1);
+ port->icount.rx++;
+
+ /* necessary for RXEMPTY to refresh its value */
+ litex_set_reg(membase + OFF_EV_PENDING, 1, EV_TX | EV_RX);
+
+ /* no overflow bits in status */
+ if (!(uart_handle_sysrq_char(port, ch)))
+ uart_insert_char(port, status, 0, ch, flg);
+
+ tty_flip_buffer_push(&port->state->port);
+ }
+
+ mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
+}
+
+static void liteuart_putchar(struct uart_port *port, int ch)
+{
+ while (litex_get_reg(port->membase + OFF_TXFULL, 1))
+ cpu_relax();
+
+ litex_set_reg(port->membase + OFF_RXTX, 1, ch);
+}
+
+static unsigned int liteuart_tx_empty(struct uart_port *port)
+{
+ /* not really tx empty, just checking if tx is not full */
+ if (!litex_get_reg(port->membase + OFF_TXFULL, 1))
+ return TIOCSER_TEMT;
+
+ return 0;
+}
+
+static void liteuart_set_mctrl(struct uart_port *port, unsigned int mctrl)
+{
+ /* modem control register is not present in LiteUART */
+}
+
+static unsigned int liteuart_get_mctrl(struct uart_port *port)
+{
+ return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
+}
+
+static void liteuart_stop_tx(struct uart_port *port)
+{
+}
+
+static void liteuart_start_tx(struct uart_port *port)
+{
+ struct circ_buf *xmit = &port->state->xmit;
+ unsigned char ch;
+
+ if (unlikely(port->x_char)) {
+ litex_set_reg(port->membase + OFF_RXTX, 1, port->x_char);
+ port->icount.tx++;
+ port->x_char = 0;
+ } else if (!uart_circ_empty(xmit)) {
+ while (xmit->head != xmit->tail) {
+ ch = xmit->buf[xmit->tail];
+ xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+ port->icount.tx++;
+ liteuart_putchar(port, ch);
+ }
+ }
+
+ if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
+ uart_write_wakeup(port);
+}
+
+static void liteuart_stop_rx(struct uart_port *port)
+{
+ struct liteuart_port *uart = to_liteuart_port(port);
+
+ /* just delete timer */
+ del_timer(&uart->timer);
+}
+
+static void liteuart_break_ctl(struct uart_port *port, int break_state)
+{
+ /* LiteUART doesn't support sending break signal */
+}
+
+static int liteuart_startup(struct uart_port *port)
+{
+ struct liteuart_port *uart = to_liteuart_port(port);
+
+ /* disable events */
+ litex_set_reg(port->membase + OFF_EV_ENABLE, 1, 0);
+
+ /* prepare timer for polling */
+ timer_setup(&uart->timer, liteuart_timer, 0);
+ mod_timer(&uart->timer, jiffies + uart_poll_timeout(port));
+
+ return 0;
+}
+
+static void liteuart_shutdown(struct uart_port *port)
+{
+}
+
+static void liteuart_set_termios(struct uart_port *port, struct ktermios *new,
+ struct ktermios *old)
+{
+ unsigned int baud;
+ unsigned long flags;
+
+ spin_lock_irqsave(&port->lock, flags);
+
+ /* update baudrate */
+ baud = uart_get_baud_rate(port, new, old, 0, 460800);
+ uart_update_timeout(port, new->c_cflag, baud);
+
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static const char *liteuart_type(struct uart_port *port)
+{
+ return "liteuart";
+}
+
+static void liteuart_release_port(struct uart_port *port)
+{
+}
+
+static int liteuart_request_port(struct uart_port *port)
+{
+ return 0;
+}
+
+static void liteuart_config_port(struct uart_port *port, int flags)
+{
+ /*
+ * Driver core for serial ports forces a non-zero value for port type.
+ * Write an arbitrary value here to accommodate the serial core driver,
+ * as ID part of UAPI is redundant.
+ */
+ port->type = 1;
+}
+
+static int liteuart_verify_port(struct uart_port *port,
+ struct serial_struct *ser)
+{
+ if (port->type != PORT_UNKNOWN && ser->type != 1)
+ return -EINVAL;
+
+ return 0;
+}
+
+static const struct uart_ops liteuart_ops = {
+ .tx_empty = liteuart_tx_empty,
+ .set_mctrl = liteuart_set_mctrl,
+ .get_mctrl = liteuart_get_mctrl,
+ .stop_tx = liteuart_stop_tx,
+ .start_tx = liteuart_start_tx,
+ .stop_rx = liteuart_stop_rx,
+ .break_ctl = liteuart_break_ctl,
+ .startup = liteuart_startup,
+ .shutdown = liteuart_shutdown,
+ .set_termios = liteuart_set_termios,
+ .type = liteuart_type,
+ .release_port = liteuart_release_port,
+ .request_port = liteuart_request_port,
+ .config_port = liteuart_config_port,
+ .verify_port = liteuart_verify_port,
+};
+
+static int liteuart_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct liteuart_port *uart;
+ struct uart_port *port;
+ struct xa_limit limit;
+ int dev_id, ret;
+
+ /* no device tree */
+ if (!np)
+ return -ENODEV;
+
+ if (!litex_check_accessors())
+ return -EPROBE_DEFER;
+
+ /* look for aliases; auto-enumerate for free index if not found */
+ dev_id = of_alias_get_id(np, "serial");
+ if (dev_id < 0)
+ limit = XA_LIMIT(0, CONFIG_SERIAL_LITEUART_MAX_PORTS);
+ else
+ limit = XA_LIMIT(dev_id, dev_id);
+
+ uart = kzalloc(sizeof(struct liteuart_port), GFP_KERNEL);
+ if (!uart)
+ return -ENOMEM;
+
+ ret = xa_alloc(&liteuart_array, &dev_id, uart, limit, GFP_KERNEL);
+ if (ret)
+ return ret;
+
+ port = &uart->port;
+
+ /* get membase */
+ port->membase = devm_platform_get_and_ioremap_resource(pdev, 0, NULL);
+ if (!port->membase)
+ return -ENXIO;
+
+ /* values not from device tree */
+ port->dev = &pdev->dev;
+ port->iotype = UPIO_MEM;
+ port->flags = UPF_BOOT_AUTOCONF;
+ port->ops = &liteuart_ops;
+ port->regshift = 2;
+ port->fifosize = 16;
+ port->iobase = 1;
+ port->type = PORT_UNKNOWN;
+ port->line = dev_id;
+ spin_lock_init(&port->lock);
+
+ return uart_add_one_port(&liteuart_driver, &uart->port);
+}
+
+static int liteuart_remove(struct platform_device *pdev)
+{
+ return 0;
+}
+
+static const struct of_device_id liteuart_of_match[] = {
+ { .compatible = "litex,liteuart" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, liteuart_of_match);
+
+static struct platform_driver liteuart_platform_driver = {
+ .probe = liteuart_probe,
+ .remove = liteuart_remove,
+ .driver = {
+ .name = "liteuart",
+ .of_match_table = liteuart_of_match,
+ },
+};
+
+#ifdef CONFIG_SERIAL_LITEUART_CONSOLE
+
+static void liteuart_console_write(struct console *co, const char *s,
+ unsigned int count)
+{
+ struct liteuart_port *uart;
+ struct uart_port *port;
+ unsigned long flags;
+
+ uart = (struct liteuart_port *)xa_load(&liteuart_array, co->index);
+ port = &uart->port;
+
+ spin_lock_irqsave(&port->lock, flags);
+ uart_console_write(port, s, count, liteuart_putchar);
+ spin_unlock_irqrestore(&port->lock, flags);
+}
+
+static int liteuart_console_setup(struct console *co, char *options)
+{
+ struct liteuart_port *uart;
+ struct uart_port *port;
+ int baud = 115200;
+ int bits = 8;
+ int parity = 'n';
+ int flow = 'n';
+
+ uart = (struct liteuart_port *)xa_load(&liteuart_array, co->index);
+ if (!uart)
+ return -ENODEV;
+
+ port = &uart->port;
+ if (!port->membase)
+ return -ENODEV;
+
+ if (options)
+ uart_parse_options(options, &baud, &parity, &bits, &flow);
+
+ return uart_set_options(port, co, baud, parity, bits, flow);
+}
+
+static struct console liteuart_console = {
+ .name = "liteuart",
+ .write = liteuart_console_write,
+ .device = uart_console_device,
+ .setup = liteuart_console_setup,
+ .flags = CON_PRINTBUFFER,
+ .index = -1,
+ .data = &liteuart_driver,
+};
+
+static int __init liteuart_console_init(void)
+{
+ register_console(&liteuart_console);
+
+ return 0;
+}
+console_initcall(liteuart_console_init);
+#endif /* CONFIG_SERIAL_LITEUART_CONSOLE */
+
+static int __init liteuart_init(void)
+{
+ int res;
+
+ res = uart_register_driver(&liteuart_driver);
+ if (res)
+ return res;
+
+ res = platform_driver_register(&liteuart_platform_driver);
+ if (res) {
+ uart_unregister_driver(&liteuart_driver);
+ return res;
+ }
+
+ return 0;
+}
+
+static void __exit liteuart_exit(void)
+{
+ platform_driver_unregister(&liteuart_platform_driver);
+ uart_unregister_driver(&liteuart_driver);
+}
+
+module_init(liteuart_init);
+module_exit(liteuart_exit);
+
+MODULE_AUTHOR("Antmicro <www.antmicro.com>");
+MODULE_DESCRIPTION("LiteUART serial driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform: liteuart");
--
2.20.1
^ permalink raw reply related
* [PATCH v6 4/5] dt-bindings: serial: document LiteUART bindings
From: Mateusz Holenko @ 2020-05-27 16:26 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Jiri Slaby,
devicetree, linux-serial
Cc: Stafford Horne, Karol Gugala, Mateusz Holenko,
Mauro Carvalho Chehab, David S. Miller, Paul E. McKenney,
Filip Kokosinski, Pawel Czarnecki, Joel Stanley, Jonathan Cameron,
Maxime Ripard, Shawn Guo, Heiko Stuebner, Sam Ravnborg,
Icenowy Zheng, Laurent Pinchart, linux-kernel, Gabriel L. Somlo
In-Reply-To: <20200527182545.3859622-0-mholenko@antmicro.com>
From: Filip Kokosinski <fkokosinski@antmicro.com>
Add documentation for LiteUART devicetree bindings.
Signed-off-by: Filip Kokosinski <fkokosinski@antmicro.com>
Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Notes:
Changes in v6:
- fixed license header
No changes in v5.
No changes in v4.
Changes in v3:
- added Reviewed-by tag
- patch number changed from 3 to 4
- removed changes in MAINTAINERS file (moved to patch #2)
Changes in v2:
- binding description rewritten to a yaml schema file
- added interrupt line
- fixed unit address
- patch number changed from 2 to 3
.../bindings/serial/litex,liteuart.yaml | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/litex,liteuart.yaml
diff --git a/Documentation/devicetree/bindings/serial/litex,liteuart.yaml b/Documentation/devicetree/bindings/serial/litex,liteuart.yaml
new file mode 100644
index 000000000000..87bf846b170a
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/litex,liteuart.yaml
@@ -0,0 +1,38 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serial/litex,liteuart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LiteUART serial controller
+
+maintainers:
+ - Karol Gugala <kgugala@antmicro.com>
+ - Mateusz Holenko <mholenko@antmicro.com>
+
+description: |
+ LiteUART serial controller is a part of LiteX FPGA SoC builder. It supports
+ multiple CPU architectures, currently including e.g. OpenRISC and RISC-V.
+
+properties:
+ compatible:
+ const: litex,liteuart
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+examples:
+ - |
+ uart0: serial@e0001800 {
+ compatible = "litex,liteuart";
+ reg = <0xe0001800 0x100>;
+ interrupts = <2>;
+ };
--
2.25.1
^ permalink raw reply related
* [PATCH v6 3/5] drivers/soc/litex: add LiteX SoC Controller driver
From: Mateusz Holenko @ 2020-05-27 16:26 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Jiri Slaby,
devicetree, linux-serial
Cc: Stafford Horne, Karol Gugala, Mateusz Holenko,
Mauro Carvalho Chehab, David S. Miller, Paul E. McKenney,
Filip Kokosinski, Pawel Czarnecki, Joel Stanley, Jonathan Cameron,
Maxime Ripard, Shawn Guo, Heiko Stuebner, Sam Ravnborg,
Icenowy Zheng, Laurent Pinchart, linux-kernel, Gabriel L. Somlo
In-Reply-To: <20200527182545.3859622-0-mholenko@antmicro.com>
From: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
This commit adds driver for the FPGA-based LiteX SoC
Controller from LiteX SoC builder.
Co-developed-by: Mateusz Holenko <mholenko@antmicro.com>
Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
Signed-off-by: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
---
Notes:
Changes in v6:
- added dependency on OF || COMPILE_TEST
- used le32_to_cpu(readl(addr)) instead of __raw_readl
and writel(cpu_to_le32(value), addr) instead of __raw_writel
to take advantage of memory barriers provided by readl/writel
Changes in v5:
- removed helper accessors and used __raw_readl/__raw_writel instead
- fixed checking for errors in litex_soc_ctrl_probe
Changes in v4:
- fixed indent in Kconfig's help section
- fixed copyright header
- changed compatible to "litex,soc-controller"
- simplified litex_soc_ctrl_probe
- removed unnecessary litex_soc_ctrl_remove
This commit has been introduced in v3 of the patchset.
It includes a simplified version of common 'litex.h'
header introduced in v2 of the patchset.
MAINTAINERS | 2 +
drivers/soc/Kconfig | 1 +
drivers/soc/Makefile | 1 +
drivers/soc/litex/Kconfig | 15 +++
drivers/soc/litex/Makefile | 3 +
drivers/soc/litex/litex_soc_ctrl.c | 197 +++++++++++++++++++++++++++++
include/linux/litex.h | 45 +++++++
7 files changed, 264 insertions(+)
create mode 100644 drivers/soc/litex/Kconfig
create mode 100644 drivers/soc/litex/Makefile
create mode 100644 drivers/soc/litex/litex_soc_ctrl.c
create mode 100644 include/linux/litex.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 39be98db7418..4d70a1b22a87 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9840,6 +9840,8 @@ M: Karol Gugala <kgugala@antmicro.com>
M: Mateusz Holenko <mholenko@antmicro.com>
S: Maintained
F: Documentation/devicetree/bindings/*/litex,*.yaml
+F: drivers/soc/litex/litex_soc_ctrl.c
+F: include/linux/litex.h
LIVE PATCHING
M: Josh Poimboeuf <jpoimboe@redhat.com>
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index 425ab6f7e375..d097d070f579 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -9,6 +9,7 @@ source "drivers/soc/bcm/Kconfig"
source "drivers/soc/fsl/Kconfig"
source "drivers/soc/imx/Kconfig"
source "drivers/soc/ixp4xx/Kconfig"
+source "drivers/soc/litex/Kconfig"
source "drivers/soc/mediatek/Kconfig"
source "drivers/soc/qcom/Kconfig"
source "drivers/soc/renesas/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 36452bed86ef..0b16108823ef 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_ARCH_GEMINI) += gemini/
obj-y += imx/
obj-$(CONFIG_ARCH_IXP4XX) += ixp4xx/
obj-$(CONFIG_SOC_XWAY) += lantiq/
+obj-$(CONFIG_LITEX_SOC_CONTROLLER) += litex/
obj-y += mediatek/
obj-y += amlogic/
obj-y += qcom/
diff --git a/drivers/soc/litex/Kconfig b/drivers/soc/litex/Kconfig
new file mode 100644
index 000000000000..c974ec3846bc
--- /dev/null
+++ b/drivers/soc/litex/Kconfig
@@ -0,0 +1,15 @@
+# SPDX-License_Identifier: GPL-2.0
+
+menu "Enable LiteX SoC Builder specific drivers"
+
+config LITEX_SOC_CONTROLLER
+ tristate "Enable LiteX SoC Controller driver"
+ depends on OF || COMPILE_TEST
+ help
+ This option enables the SoC Controller Driver which verifies
+ LiteX CSR access and provides common litex_get_reg/litex_set_reg
+ accessors.
+ All drivers that use functions from litex.h must depend on
+ LITEX_SOC_CONTROLLER.
+
+endmenu
diff --git a/drivers/soc/litex/Makefile b/drivers/soc/litex/Makefile
new file mode 100644
index 000000000000..98ff7325b1c0
--- /dev/null
+++ b/drivers/soc/litex/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License_Identifier: GPL-2.0
+
+obj-$(CONFIG_LITEX_SOC_CONTROLLER) += litex_soc_ctrl.o
diff --git a/drivers/soc/litex/litex_soc_ctrl.c b/drivers/soc/litex/litex_soc_ctrl.c
new file mode 100644
index 000000000000..319be92fc63f
--- /dev/null
+++ b/drivers/soc/litex/litex_soc_ctrl.c
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * LiteX SoC Controller Driver
+ *
+ * Copyright (C) 2020 Antmicro <www.antmicro.com>
+ *
+ */
+
+#include <linux/litex.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/printk.h>
+#include <linux/module.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+
+/*
+ * The parameters below are true for LiteX SoC
+ * configured for 8-bit CSR Bus, 32-bit aligned.
+ *
+ * Supporting other configurations will require
+ * extending the logic in this header.
+ */
+#define LITEX_REG_SIZE 0x4
+#define LITEX_SUBREG_SIZE 0x1
+#define LITEX_SUBREG_SIZE_BIT (LITEX_SUBREG_SIZE * 8)
+
+static DEFINE_SPINLOCK(csr_lock);
+
+/*
+ * LiteX SoC Generator, depending on the configuration,
+ * can split a single logical CSR (Control & Status Register)
+ * into a series of consecutive physical registers.
+ *
+ * For example, in the configuration with 8-bit CSR Bus,
+ * 32-bit aligned (the default one for 32-bit CPUs) a 32-bit
+ * logical CSR will be generated as four 32-bit physical registers,
+ * each one containing one byte of meaningful data.
+ *
+ * For details see: https://github.com/enjoy-digital/litex/wiki/CSR-Bus
+ *
+ * The purpose of `litex_set_reg`/`litex_get_reg` is to implement
+ * the logic of writing to/reading from the LiteX CSR in a single
+ * place that can be then reused by all LiteX drivers.
+ */
+void litex_set_reg(void __iomem *reg, unsigned long reg_size,
+ unsigned long val)
+{
+ unsigned long shifted_data, shift, i;
+ unsigned long flags;
+
+ spin_lock_irqsave(&csr_lock, flags);
+
+ for (i = 0; i < reg_size; ++i) {
+ shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
+ shifted_data = val >> shift;
+
+ writel(cpu_to_le32(shifted_data), reg + (LITEX_REG_SIZE * i));
+ }
+
+ spin_unlock_irqrestore(&csr_lock, flags);
+}
+
+unsigned long litex_get_reg(void __iomem *reg, unsigned long reg_size)
+{
+ unsigned long shifted_data, shift, i;
+ unsigned long result = 0;
+ unsigned long flags;
+
+ spin_lock_irqsave(&csr_lock, flags);
+
+ for (i = 0; i < reg_size; ++i) {
+ shifted_data = le32_to_cpu(readl(reg + (LITEX_REG_SIZE * i)));
+
+ shift = ((reg_size - i - 1) * LITEX_SUBREG_SIZE_BIT);
+ result |= (shifted_data << shift);
+ }
+
+ spin_unlock_irqrestore(&csr_lock, flags);
+
+ return result;
+}
+
+static int accessors_ok;
+
+/*
+ * Check if accessors are safe to be used by other drivers
+ * returns true if yes - false if not
+ */
+int litex_check_accessors(void)
+{
+ return accessors_ok;
+}
+
+#define SCRATCH_REG_OFF 0x04
+#define SCRATCH_REG_SIZE 4
+#define SCRATCH_REG_VALUE 0x12345678
+#define SCRATCH_TEST_VALUE 0xdeadbeef
+
+/*
+ * Check LiteX CSR read/write access
+ *
+ * This function reads and writes a scratch register in order
+ * to verify if CSR access works.
+ *
+ * In case any problems are detected, the driver should panic
+ * and not set `accessors_ok` flag. As a result no other
+ * LiteX driver should access CSR bus.
+ *
+ * Access to the LiteX CSR is, by design, done in CPU native
+ * endianness. The driver should not dynamically configure
+ * access functions when the endianness mismatch is detected.
+ * Such situation indicates problems in the soft SoC design
+ * and should be solved at the LiteX generator level,
+ * not in the software.
+ */
+static int litex_check_csr_access(void __iomem *reg_addr)
+{
+ unsigned long reg;
+
+ reg = litex_get_reg(reg_addr + SCRATCH_REG_OFF, SCRATCH_REG_SIZE);
+
+ if (reg != SCRATCH_REG_VALUE) {
+ panic("Scratch register read error! Expected: 0x%x but got: 0x%lx",
+ SCRATCH_REG_VALUE, reg);
+ return -EINVAL;
+ }
+
+ litex_set_reg(reg_addr + SCRATCH_REG_OFF,
+ SCRATCH_REG_SIZE, SCRATCH_TEST_VALUE);
+ reg = litex_get_reg(reg_addr + SCRATCH_REG_OFF, SCRATCH_REG_SIZE);
+
+ if (reg != SCRATCH_TEST_VALUE) {
+ panic("Scratch register write error! Expected: 0x%x but got: 0x%lx",
+ SCRATCH_TEST_VALUE, reg);
+ return -EINVAL;
+ }
+
+ /* restore original value of the SCRATCH register */
+ litex_set_reg(reg_addr + SCRATCH_REG_OFF,
+ SCRATCH_REG_SIZE, SCRATCH_REG_VALUE);
+
+ /* Set flag for other drivers */
+ accessors_ok = 1;
+ pr_info("LiteX SoC Controller driver initialized");
+
+ return 0;
+}
+
+struct litex_soc_ctrl_device {
+ void __iomem *base;
+};
+
+static const struct of_device_id litex_soc_ctrl_of_match[] = {
+ {.compatible = "litex,soc-controller"},
+ {},
+};
+
+MODULE_DEVICE_TABLE(of, litex_soc_ctrl_of_match);
+
+static int litex_soc_ctrl_probe(struct platform_device *pdev)
+{
+ struct device *dev;
+ struct device_node *node;
+ struct litex_soc_ctrl_device *soc_ctrl_dev;
+
+ dev = &pdev->dev;
+ node = dev->of_node;
+ if (!node)
+ return -ENODEV;
+
+ soc_ctrl_dev = devm_kzalloc(dev, sizeof(*soc_ctrl_dev), GFP_KERNEL);
+ if (!soc_ctrl_dev)
+ return -ENOMEM;
+
+ soc_ctrl_dev->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(soc_ctrl_dev->base))
+ return PTR_ERR(soc_ctrl_dev->base);
+
+ return litex_check_csr_access(soc_ctrl_dev->base);
+}
+
+static struct platform_driver litex_soc_ctrl_driver = {
+ .driver = {
+ .name = "litex-soc-controller",
+ .of_match_table = of_match_ptr(litex_soc_ctrl_of_match)
+ },
+ .probe = litex_soc_ctrl_probe,
+};
+
+module_platform_driver(litex_soc_ctrl_driver);
+MODULE_DESCRIPTION("LiteX SoC Controller driver");
+MODULE_AUTHOR("Antmicro <www.antmicro.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/litex.h b/include/linux/litex.h
new file mode 100644
index 000000000000..f31062436273
--- /dev/null
+++ b/include/linux/litex.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Common LiteX header providing
+ * helper functions for accessing CSRs.
+ *
+ * Implementation of the functions is provided by
+ * the LiteX SoC Controller driver.
+ *
+ * Copyright (C) 2019-2020 Antmicro <www.antmicro.com>
+ */
+
+#ifndef _LINUX_LITEX_H
+#define _LINUX_LITEX_H
+
+#include <linux/io.h>
+#include <linux/types.h>
+#include <linux/compiler_types.h>
+
+/*
+ * litex_check_accessors is a function implemented in
+ * drivers/soc/litex/litex_soc_controller.c
+ * checking if the common LiteX CSR accessors
+ * are safe to be used by the drivers;
+ * returns true (1) if yes - false (0) if not
+ *
+ * Important: All drivers that use litex_set_reg/litex_get_reg
+ * functions should make sure that LiteX SoC Controller driver
+ * has verified LiteX CSRs read and write operations before
+ * issuing any read/writes to the LiteX peripherals.
+ *
+ * Exemplary snippet that can be used at the beginning
+ * of the driver's probe() function to ensure that LiteX
+ * SoC Controller driver is properely initialized:
+ *
+ * if (!litex_check_accessors())
+ * return -EPROBE_DEFER;
+ */
+int litex_check_accessors(void);
+
+void litex_set_reg(void __iomem *reg, unsigned long reg_sz, unsigned long val);
+
+unsigned long litex_get_reg(void __iomem *reg, unsigned long reg_sz);
+
+
+#endif /* _LINUX_LITEX_H */
--
2.25.1
^ permalink raw reply related
* [PATCH v6 2/5] dt-bindings: soc: document LiteX SoC Controller bindings
From: Mateusz Holenko @ 2020-05-27 16:26 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Jiri Slaby,
devicetree, linux-serial
Cc: Stafford Horne, Karol Gugala, Mateusz Holenko,
Mauro Carvalho Chehab, David S. Miller, Paul E. McKenney,
Filip Kokosinski, Pawel Czarnecki, Joel Stanley, Jonathan Cameron,
Maxime Ripard, Shawn Guo, Heiko Stuebner, Sam Ravnborg,
Icenowy Zheng, Laurent Pinchart, linux-kernel, Gabriel L. Somlo
In-Reply-To: <20200527182545.3859622-0-mholenko@antmicro.com>
From: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
Add documentation for LiteX SoC Controller bindings.
Signed-off-by: Pawel Czarnecki <pczarnecki@internships.antmicro.com>
Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
Notes:
Changes in v6:
- fixed license header
Changes in v5:
- added reviewed-by tag
Changes in v4:
- changes compatible to "litex,soc-controller"
- fixed yaml's header
- removed unnecessary sections from yaml
- fixed indentation in yaml
This commit has been introduced in v3 of the patchset.
.../soc/litex/litex,soc-controller.yaml | 39 +++++++++++++++++++
MAINTAINERS | 6 +++
2 files changed, 45 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/litex/litex,soc-controller.yaml
diff --git a/Documentation/devicetree/bindings/soc/litex/litex,soc-controller.yaml b/Documentation/devicetree/bindings/soc/litex/litex,soc-controller.yaml
new file mode 100644
index 000000000000..b118ddbf04a4
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/litex/litex,soc-controller.yaml
@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+# Copyright 2020 Antmicro <www.antmicro.com>
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/litex/litex,soc-controller.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: LiteX SoC Controller driver
+
+description: |
+ This is the SoC Controller driver for the LiteX SoC Builder.
+ It's purpose is to verify LiteX CSR (Control&Status Register) access
+ operations and provide function for other drivers to read/write CSRs
+ and to check if those accessors are ready to use.
+
+maintainers:
+ - Karol Gugala <kgugala@antmicro.com>
+ - Mateusz Holenko <mholenko@antmicro.com>
+
+properties:
+ compatible:
+ const: litex,soc-controller
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+examples:
+ - |
+ soc_ctrl0: soc-controller@f0000000 {
+ compatible = "litex,soc-controller";
+ reg = <0x0 0xf0000000 0x0 0xC>;
+ status = "okay";
+ };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 7b58ca29cc80..39be98db7418 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9835,6 +9835,12 @@ L: kunit-dev@googlegroups.com
S: Maintained
F: lib/list-test.c
+LITEX PLATFORM
+M: Karol Gugala <kgugala@antmicro.com>
+M: Mateusz Holenko <mholenko@antmicro.com>
+S: Maintained
+F: Documentation/devicetree/bindings/*/litex,*.yaml
+
LIVE PATCHING
M: Josh Poimboeuf <jpoimboe@redhat.com>
M: Jiri Kosina <jikos@kernel.org>
--
2.25.1
^ permalink raw reply related
* [PATCH v6 1/5] dt-bindings: vendor: add vendor prefix for LiteX
From: Mateusz Holenko @ 2020-05-27 16:26 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Jiri Slaby,
devicetree, linux-serial
Cc: Stafford Horne, Karol Gugala, Mateusz Holenko,
Mauro Carvalho Chehab, David S. Miller, Paul E. McKenney,
Filip Kokosinski, Pawel Czarnecki, Joel Stanley, Jonathan Cameron,
Maxime Ripard, Shawn Guo, Heiko Stuebner, Sam Ravnborg,
Icenowy Zheng, Laurent Pinchart, linux-kernel, Gabriel L. Somlo
In-Reply-To: <20200527182545.3859622-0-mholenko@antmicro.com>
From: Filip Kokosinski <fkokosinski@antmicro.com>
Add vendor prefix for LiteX SoC builder.
Signed-off-by: Filip Kokosinski <fkokosinski@antmicro.com>
Signed-off-by: Mateusz Holenko <mholenko@antmicro.com>
Acked-by: Rob Herring <robh@kernel.org>
---
Notes:
No changes in v6.
No changes in v5.
No changes in v4.
Changes in v3:
- added Acked-by tag
No changes in v2.
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index d3891386d671..9aae6c56d7a3 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -571,6 +571,8 @@ patternProperties:
description: Linux-specific binding
"^linx,.*":
description: Linx Technologies
+ "^litex,.*":
+ description: LiteX SoC builder
"^lltc,.*":
description: Linear Technology Corporation
"^logicpd,.*":
--
2.25.1
^ permalink raw reply related
* [PATCH v6 0/5] LiteX SoC controller and LiteUART serial driver
From: Mateusz Holenko @ 2020-05-27 16:25 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, Greg Kroah-Hartman, Jiri Slaby,
devicetree, linux-serial
Cc: Stafford Horne, Karol Gugala, Mateusz Holenko,
Mauro Carvalho Chehab, David S. Miller, Paul E. McKenney,
Filip Kokosinski, Pawel Czarnecki, Joel Stanley, Jonathan Cameron,
Maxime Ripard, Shawn Guo, Heiko Stuebner, Sam Ravnborg,
Icenowy Zheng, Laurent Pinchart, linux-kernel, Gabriel L. Somlo
This patchset introduces support for LiteX SoC Controller
and LiteUART - serial device from LiteX SoC builder
(https://github.com/enjoy-digital/litex).
In the following patchset I will add
a new mor1kx-based (OpenRISC) platform that
uses this device.
Later I plan to extend this platform by
adding support for more devices from LiteX suite.
Changes in v6:
- changed accessors in SoC Controller's driver
- reworked UART driver
Changes in v5:
- added Reviewed-by tag
- removed custom accessors from SoC Controller's driver
- fixed error checking in SoC Controller's driver
Changes in v4:
- fixed copyright headers
- fixed SoC Controller's yaml
- simplified SoC Controller's driver
Changes in v3:
- added Acked-by and Reviewed-by tags
- introduced LiteX SoC Controller driver
- removed endianness detection (handled now by LiteX SoC Controller driver)
- modified litex.h header
- DTS aliases for LiteUART made optional
- renamed SERIAL_LITEUART_NR_PORTS to SERIAL_LITEUART_MAX_PORTS
- changed PORT_LITEUART from 122 to 123
Changes in v2:
- binding description rewritten to a yaml schema file
- added litex.h header with common register access functions
Filip Kokosinski (3):
dt-bindings: vendor: add vendor prefix for LiteX
dt-bindings: serial: document LiteUART bindings
drivers/tty/serial: add LiteUART driver
Pawel Czarnecki (2):
dt-bindings: soc: document LiteX SoC Controller bindings
drivers/soc/litex: add LiteX SoC Controller driver
.../bindings/serial/litex,liteuart.yaml | 38 ++
.../soc/litex/litex,soc-controller.yaml | 39 ++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
MAINTAINERS | 9 +
drivers/soc/Kconfig | 1 +
drivers/soc/Makefile | 1 +
drivers/soc/litex/Kconfig | 15 +
drivers/soc/litex/Makefile | 3 +
drivers/soc/litex/litex_soc_ctrl.c | 197 +++++++++
drivers/tty/serial/Kconfig | 31 ++
drivers/tty/serial/Makefile | 1 +
drivers/tty/serial/liteuart.c | 399 ++++++++++++++++++
include/linux/litex.h | 45 ++
13 files changed, 781 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/litex,liteuart.yaml
create mode 100644 Documentation/devicetree/bindings/soc/litex/litex,soc-controller.yaml
create mode 100644 drivers/soc/litex/Kconfig
create mode 100644 drivers/soc/litex/Makefile
create mode 100644 drivers/soc/litex/litex_soc_ctrl.c
create mode 100644 drivers/tty/serial/liteuart.c
create mode 100644 include/linux/litex.h
--
2.25.1
^ permalink raw reply
* Re: [PATCH v3 3/3] hwmon: Add Baikal-T1 PVT sensor driver
From: Guenter Roeck @ 2020-05-27 16:25 UTC (permalink / raw)
To: Serge Semin
Cc: Jean Delvare, Jonathan Corbet, Serge Semin, Maxim Kaurkin,
Alexey Malahov, Thomas Bogendoerfer, Arnd Bergmann, Rob Herring,
linux-mips, devicetree, linux-hwmon, linux-doc, linux-kernel
In-Reply-To: <20200526133823.20466-4-Sergey.Semin@baikalelectronics.ru>
On Tue, May 26, 2020 at 04:38:23PM +0300, Serge Semin wrote:
> Baikal-T1 SoC provides an embedded process, voltage and temperature
> sensor to monitor an internal SoC environment (chip temperature, supply
> voltage and process monitor) and on time detect critical situations,
> which may cause the system instability and even damages. The IP-block
> is based on the Analog Bits PVT sensor, but is equipped with a
> dedicated control wrapper, which provides a MMIO registers-based access
> to the sensor core functionality (APB3-bus based) and exposes an
> additional functions like thresholds/data ready interrupts, its status
> and masks, measurements timeout. All of these is used to create a hwmon
> driver being added to the kernel by this commit.
>
> The driver implements support for the hardware monitoring capabilities
> of Baikal-T1 process, voltage and temperature sensors. PVT IP-core
> consists of one temperature and four voltage sensors, each of which is
> implemented as a dedicated hwmon channel config.
>
> The driver can optionally provide the hwmon alarms for each sensor the
> PVT controller supports. The alarms functionality is made compile-time
> configurable due to the hardware interface implementation peculiarity,
> which is connected with an ability to convert data from only one sensor
> at a time. Additional limitation is that the controller performs the
> thresholds checking synchronously with the data conversion procedure.
> Due to these limitations in order to have the hwmon alarms
> automatically detected the driver code must switch from one sensor to
> another, read converted data and manually check the threshold status
> bits. Depending on the measurements timeout settings this design may
> cause additional burden on the system performance. By default if the
> alarms kernel config is disabled the data conversion is performed by
> the driver on demand when read operation is requested via corresponding
> _input-file.
>
> Co-developed-by: Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
> Signed-off-by: Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> Cc: Alexey Malahov <Alexey.Malahov@baikalelectronics.ru>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: linux-mips@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> ---
>
> Changelog v2:
> - Discard handwritten IO-access wrappers. Use normal readl/writel instead.
> - Use generic FIELD_{GET,PREP} macros instead of handwritten ones.
> - Since the driver depends on the OF config we can remove of_match_ptr()
> macro utilization.
> - Don't print error-message if no platform IRQ found. Just return an error.
> - Remove probe-status info string printout.
>
> Changelog v3:
> - Add bt1-pvt into the Documentation/hwmon/index.rst file.
> - Discard explicit "default n" from the SENSORS_BT1_PVT_ALARMS config.
> - Use "depends on SENSORS_BT1_PVT" statement instead of if-endif kbuild
> config clause.
> - Alphabetically order the include macro operators.
> - Discard unneeded include macro in the header file.
> - Use new generic interface of the hwmon alarms notifications introduced
> in the first patch (based on hwmon_notify_event()).
> - Add more descriptive information regarding the temp1_trim attribute.
> - Discard setting the platforms device private data by using
> platform_set_drvdata(). It's redundant since unused in the driver.
> - Pass "pvt" hwmon name instead of dev_name(dev) to
> devm_hwmon_device_register_with_info().
> - Add "baikal,pvt-temp-trim-millicelsius" temperature trim DT property
> support.
> - Discard kernel log warnings printed from the ISR when either min or
> max threshold levels are crossed.
> - Discard CONFIG_OF dependency since there is non at compile-time.
> ---
> Documentation/hwmon/bt1-pvt.rst | 116 ++++
> Documentation/hwmon/index.rst | 1 +
> drivers/hwmon/Kconfig | 25 +
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/bt1-pvt.c | 1155 +++++++++++++++++++++++++++++++
> drivers/hwmon/bt1-pvt.h | 244 +++++++
> 6 files changed, 1542 insertions(+)
> create mode 100644 Documentation/hwmon/bt1-pvt.rst
> create mode 100644 drivers/hwmon/bt1-pvt.c
> create mode 100644 drivers/hwmon/bt1-pvt.h
>
> diff --git a/Documentation/hwmon/bt1-pvt.rst b/Documentation/hwmon/bt1-pvt.rst
> new file mode 100644
> index 000000000000..f5f47891d87a
> --- /dev/null
> +++ b/Documentation/hwmon/bt1-pvt.rst
> @@ -0,0 +1,116 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +Kernel driver bt1-pvt
> +=====================
> +
> +Supported chips:
> +
> + * Baikal-T1 PVT sensor (in SoC)
> +
> + Prefix: 'bt1-pvt'
> +
> + Addresses scanned: -
> +
> + Datasheet: Provided by BAIKAL ELECTRONICS upon request and under NDA
> +
> +Authors:
> + Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
> + Serge Semin <Sergey.Semin@baikalelectronics.ru>
> +
> +Description
> +-----------
> +
> +This driver implements support for the hardware monitoring capabilities of the
> +embedded into Baikal-T1 process, voltage and temperature sensors. PVT IP-core
> +consists of one temperature and four voltage sensors, which can be used to
> +monitor the chip internal environment like heating, supply voltage and
> +transistors performance. The driver can optionally provide the hwmon alarms
> +for each sensor the PVT controller supports. The alarms functionality is made
> +compile-time configurable due to the hardware interface implementation
> +peculiarity, which is connected with an ability to convert data from only one
> +sensor at a time. Additional limitation is that the controller performs the
> +thresholds checking synchronously with the data conversion procedure. Due to
> +these in order to have the hwmon alarms automatically detected the driver code
> +must switch from one sensor to another, read converted data and manually check
> +the threshold status bits. Depending on the measurements timeout settings
> +(update_interval sysfs node value) this design may cause additional burden on
> +the system performance. So in case if alarms are unnecessary in your system
> +design it's recommended to have them disabled to prevent the PVT IRQs being
> +periodically raised to get the data cache/alarms status up to date. By default
> +in alarm-less configuration the data conversion is performed by the driver
> +on demand when read operation is requested via corresponding _input-file.
> +
> +Temperature Monitoring
> +----------------------
> +
> +Temperature is measured with 10-bit resolution and reported in millidegree
> +Celsius. The driver performs all the scaling by itself therefore reports true
> +temperatures that don't need any user-space adjustments. While the data
> +translation formulae isn't linear, which gives us non-linear discreteness,
> +it's close to one, but giving a bit better accuracy for higher temperatures.
> +The temperature input is mapped as follows (the last column indicates the input
> +ranges)::
> +
> + temp1: CPU embedded diode -48.38C - +147.438C
> +
> +In case if the alarms kernel config is enabled in the driver the temperature input
> +has associated min and max limits which trigger an alarm when crossed.
> +
> +Voltage Monitoring
> +------------------
> +
> +The voltage inputs are also sampled with 10-bit resolution and reported in
> +millivolts. But in this case the data translation formulae is linear, which
> +provides a constant measurements discreteness. The data scaling is also
> +performed by the driver, so returning true millivolts. The voltage inputs are
> +mapped as follows (the last column indicates the input ranges)::
> +
> + in0: VDD (processor core) 0.62V - 1.168V
> + in1: Low-Vt (low voltage threshold) 0.62V - 1.168V
> + in2: High-Vt (high voltage threshold) 0.62V - 1.168V
> + in3: Standard-Vt (standard voltage threshold) 0.62V - 1.168V
> +
> +In case if the alarms configis enabled in the driver the voltage inputs
> +have associated min and max limits which trigger an alarm when crossed.
> +
> +Sysfs Attributes
> +----------------
> +
> +Following is a list of all sysfs attributes that the driver provides, their
> +permissions and a short description:
> +
> +=============================== ======= =======================================
> +Name Perm Description
> +=============================== ======= =======================================
> +update_interval RW Measurements update interval per
> + sensor.
> +temp1_type RO Sensor type (always 1 as CPU embedded
> + diode).
> +temp1_label RO CPU Core Temperature sensor.
> +temp1_input RO Measured temperature in millidegree
> + Celsius.
> +temp1_min RW Low limit for temp input.
> +temp1_max RW High limit for temp input.
> +temp1_min_alarm RO Temperature input alarm. Returns 1 if
> + temperature input went below min limit,
> + 0 otherwise.
> +temp1_max_alarm RO Temperature input alarm. Returns 1 if
> + temperature input went above max limit,
> + 0 otherwise.
> +temp1_trim RW Temperature sensor trimming factor in
> + millidegree Celsius. It can be used to
> + manually adjust the temperature
> + measurements within 7.130 degrees
> + Celsius.
vs. standard ABI:
temp[1-*]_offset`
Temperature offset which is added to the temperature reading
by the chip.
Unit: millidegree Celsius
If you really think this is necessary, why not use the standard ABI ?
Guenter
^ permalink raw reply
* RE: [PATCH v9 08/10] dt-bindings: ufs: Add DT binding documentation for ufs
From: Alim Akhtar @ 2020-05-27 16:23 UTC (permalink / raw)
To: 'Rob Herring'
Cc: devicetree, linux-scsi, krzk, avri.altman, martin.petersen,
kwmad.kim, stanley.chu, cang, linux-samsung-soc, linux-arm-kernel,
linux-kernel
In-Reply-To: <20200526180843.GA81537@bogus>
Hi Rob,
> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 26 May 2020 23:39
> To: Alim Akhtar <alim.akhtar@samsung.com>
> Cc: devicetree@vger.kernel.org; linux-scsi@vger.kernel.org;
krzk@kernel.org;
> avri.altman@wdc.com; martin.petersen@oracle.com;
> kwmad.kim@samsung.com; stanley.chu@mediatek.com;
> cang@codeaurora.org; linux-samsung-soc@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v9 08/10] dt-bindings: ufs: Add DT binding
documentation
> for ufs
>
> On Thu, May 14, 2020 at 06:09:12AM +0530, Alim Akhtar wrote:
> > This patch adds DT binding for samsung ufs hci
>
> Subject should indicate this is for Samsung in some way.
>
Sure will update the Subject as suggested by you.
> >
> > Signed-off-by: Alim Akhtar <alim.akhtar@samsung.com>
> > ---
> > .../bindings/ufs/samsung,exynos-ufs.yaml | 91 +++++++++++++++++++
> > 1 file changed, 91 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml
> > b/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml
> > new file mode 100644
> > index 000000000000..eaa64cc32d52
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/ufs/samsung,exynos-ufs.yaml
> > @@ -0,0 +1,91 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
> > +---
> > +$id:
> > +https://protect2.fireeye.com/url?k=9995443c-c4461d82-9994cf73-0cc47a3
> > +1ba82-
> 2c9d6322e4bc35a5&q=1&u=http%3A%2F%2Fdevicetree.org%2Fschemas%2F
> > +ufs%2Fsamsung%2Cexynos-ufs.yaml%23
> > +$schema:
> > +https://protect2.fireeye.com/url?k=70bd56cd-2d6e0f73-70bcdd82-0cc47a3
> > +1ba82-7865215595a4146c&q=1&u=http%3A%2F%2Fdevicetree.org%2Fmeta-
> schem
> > +as%2Fcore.yaml%23
> > +
> > +title: Samsung SoC series UFS host controller Device Tree Bindings
> > +
> > +maintainers:
> > + - Alim Akhtar <alim.akhtar@samsung.com>
> > +
> > +description: |
> > + Each Samsung UFS host controller instance should have its own node.
> > + This binding define Samsung specific binding other then what is
> > +used
> > + in the common ufshcd bindings
> > + [1] Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
> > +
> > +properties:
> > +
> > + compatible:
> > + enum:
> > + - samsung,exynos7-ufs
> > +
> > + reg:
> > + items:
> > + - description: HCI register
> > + - description: vendor specific register
> > + - description: unipro register
> > + - description: UFS protector register
> > +
> > + reg-names:
> > + items:
> > + - const: hci
> > + - const: vs_hci
> > + - const: unipro
> > + - const: ufsp
> > +
> > + clocks:
> > + maxItems: 2
>
> maxItems is redundant.
>
Will drop it.
> > + items:
> > + - description: ufs link core clock
> > + - description: unipro main clock
> > +
> > + clock-names:
> > + maxItems: 2
>
> Here too.
Will drop it.
>
> > + items:
> > + - const: core_clk
> > + - const: sclk_unipro_main
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + phys:
> > + maxItems: 1
> > +
> > + phy-names:
> > + maxItems: 1
>
> What's the name? (Though a name is kind of pointless when there is only
> 1.)
Not sure are you suggesting to drop the phy-names completely? Or just keep
"phy-names:" only.
I looked into how other bindings has handle it, I will change this as
phy-names:
const: ufs-phy
Hope you are ok with this.
>
> With those fixed,
>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
With adding "phy-names" entry, I will adds your Reviewed-by tag, will post
the updated changes soon.
Thank you!!
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - phys
> > + - phy-names
> > + - clocks
> > + - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/clock/exynos7-clk.h>
> > +
> > + ufs: ufs@15570000 {
> > + compatible = "samsung,exynos7-ufs";
> > + reg = <0x15570000 0x100>,
> > + <0x15570100 0x100>,
> > + <0x15571000 0x200>,
> > + <0x15572000 0x300>;
> > + reg-names = "hci", "vs_hci", "unipro", "ufsp";
> > + interrupts = <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>;
> > + clocks = <&clock_fsys1 ACLK_UFS20_LINK>,
> > + <&clock_fsys1 SCLK_UFSUNIPRO20_USER>;
> > + clock-names = "core_clk", "sclk_unipro_main";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&ufs_rst_n &ufs_refclk_out>;
> > + phys = <&ufs_phy>;
> > + phy-names = "ufs-phy";
> > + };
> > +...
> > --
> > 2.17.1
> >
^ permalink raw reply
* Re: [PATCH v6 3/4] arm64: dts: imx8mq: enable Hantro G1/G2 VPU
From: Philipp Zabel @ 2020-05-27 16:19 UTC (permalink / raw)
To: Shawn Guo, linux-media
Cc: Ezequiel Garcia, Mauro Carvalho Chehab, Hans Verkuil, Rob Herring,
kernel, devicetree
In-Reply-To: <20200320131256.23294-4-p.zabel@pengutronix.de>
Hi Shawn,
On Fri, 2020-03-20 at 14:12 +0100, Philipp Zabel wrote:
> Add the i.MX8MQ VPU module which comprises Hantro G1 and G2 video
> decoder cores and a reset/control block.
>
> Hook up the bus clock to the VPU power domain to enable handshakes, and
> configure the core clocks to 600 MHz and the bus clock to 800 MHz by
> default.
>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
could you pick up this patch? The driver and binding parts have been
merged in media/master.
regards
Philipp
> ---
> New in v6.
> ---
> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 27 +++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> index 6a1e83922c71..98e464ecb68a 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -666,6 +666,7 @@
> pgc_vpu: power-domain@6 {
> #power-domain-cells = <0>;
> reg = <IMX8M_POWER_DOMAIN_VPU>;
> + clocks = <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
> };
>
> pgc_disp: power-domain@7 {
> @@ -1130,6 +1131,32 @@
> status = "disabled";
> };
>
> + vpu: video-codec@38300000 {
> + compatible = "nxp,imx8mq-vpu";
> + reg = <0x38300000 0x10000>,
> + <0x38310000 0x10000>,
> + <0x38320000 0x10000>;
> + reg-names = "g1", "g2", "ctrl";
> + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
> + <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "g1", "g2";
> + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>,
> + <&clk IMX8MQ_CLK_VPU_G2_ROOT>,
> + <&clk IMX8MQ_CLK_VPU_DEC_ROOT>;
> + clock-names = "g1", "g2", "bus";
> + assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>,
> + <&clk IMX8MQ_CLK_VPU_G2>,
> + <&clk IMX8MQ_CLK_VPU_BUS>,
> + <&clk IMX8MQ_VPU_PLL_BYPASS>;
> + assigned-clock-parents = <&clk IMX8MQ_VPU_PLL_OUT>,
> + <&clk IMX8MQ_VPU_PLL_OUT>,
> + <&clk IMX8MQ_SYS1_PLL_800M>,
> + <&clk IMX8MQ_VPU_PLL>;
> + assigned-clock-rates = <600000000>, <600000000>,
> + <800000000>, <0>;
> + power-domains = <&pgc_vpu>;
> + };
> +
> pcie0: pcie@33800000 {
> compatible = "fsl,imx8mq-pcie";
> reg = <0x33800000 0x400000>,
^ permalink raw reply
* Re: [PATCH v5 00/11] i2c: designeware: Add Baikal-T1 System I2C support
From: Serge Semin @ 2020-05-27 16:15 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Jarkko Nikula, Wolfram Sang, Alexey Malahov,
Maxim Kaurkin, Pavel Parkhomenko, Ramil Zaripov,
Ekaterina Skachko, Vadim Vlasov, Alexey Kolotnikov,
Thomas Bogendoerfer, Mika Westerberg, Rob Herring, linux-mips,
linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200527160847.GN1634618@smile.fi.intel.com>
On Wed, May 27, 2020 at 07:08:47PM +0300, Andy Shevchenko wrote:
> On Wed, May 27, 2020 at 06:30:35PM +0300, Serge Semin wrote:
> > Jarkko, Wolfram, the merge window is upon us, please review/merge in/whatever
> > the patchset.
> >
> > Initially this has been a small patchset which embedded the Baikal-T1
> > System I2C support into the DW APB I2C driver as is by using a simplest
> > way. After a short discussion with Andy we decided to implement what he
> > suggested (introduce regmap-based accessors and create a glue driver) and
> > even more than that to provide some cleanups of the code. So here is what
> > this patchset consists of.
> >
> > First of all we've found out that current implementation of scripts/dtc
> > didn't support i2c dt nodes with 10bit and slave flags set in the
> > reg property. You'll see an error if you try to dt_binding_check it.
> > So the very first patch fixes the problem by adding these flags support
> > into the check_i2c_bus_reg() method.
> >
> > Traditionally we converted the plain text-based DT binding to the DT schema
> > and added Baikal-T1 System I2C device support there. This required to mark
> > the reg property redundant for Baikal-T1 I2C since its reg-space is
> > indirectly accessed by means of the System Controller cmd/read/write
> > registers.
> >
> > Then as Andy suggested we replaced the Synopsys DW APB I2C common driver
> > registers IO accessors into the regmap API methods. This doesn't change
> > the code logic much, though in two places we managed to replace some bulky
> > peaces of code with a ready-to-use regmap methods.
> >
> > Additionally before adding the glue layer API we initiated a set of cleanups:
> > - Define components of the multi-object drivers (like i2c-designware-core.o
> > and i2c-designware-paltform.o) with using `-y` suffixed makefile
> > variables instead of `-objs` suffixed one. This is encouraged by
> > Documentation/kbuild/makefiles.rst text since `-objs` is supposed to be used
> > to build host programs.
> > - Make DW I2C slave driver depended on the DW I2C core code instead of the
> > platform one, which it really is.
> > - Move Intel Baytrail semaphore feature to the platform if-clause of the
> > kernel config.
> >
> > After this we finally can introduce the glue layer API for the DW APB I2C
> > platform driver. So there are three methods exported from the driver:
> > i2c_dw_plat_setup(), i2c_dw_plat_clear(), &i2c_dw_plat_dev_pm_ops to
> > setup, cleanup and add PM operations to the glue driven I2C device. Before
> > setting the platform DW I2C device up the glue probe code is supposed to
> > create an instance of DW I2C device generic object and pre-initialize
> > its `struct device` pointer together with optional platform-specific
> > flags. In addition to that we converted the MSCC Ocelot SoC I2C specific
> > code into the glue layer seeing it's really too specific and, which is more
> > important, isn't that complicated so we could unpin it without much of
> > worrying to break something.
> >
> > Meanwhile we discovered that MODEL_CHERRYTRAIL and MODEL_MASK actually
> > were no longer used in the code. MODEL_MSCC flag has been discarded since
> > the MSCC Ocelot I2C code conversion to the glue driver. So now we can get
> > rid of all the MODEL-specific flags.
> >
> > Finally we introduced a glue driver with Baikal-T1 System I2C device
> > support. The driver probe tries to find a syscon regmap, creates the DW
> > APB I2C regmap based on it and passes it further to the DW I2C device
> > descriptor. Then it does normal DW APB I2C platform setup by calling a
> > generic setup method. Cleanup is straightforward. It's just calling a
> > generic DW APB I2C clean method.
>
> Thank you for an update.
>
> > This patchset is rebased and tested on the i2c/for-next (5.7-rc7):
>
> Hmm...
>
> > base-commit: 228f95c14949 ("Merge remote-tracking branch 'spi/for-5.8' into spi-next")
>
> ...this is strange.
Yeah, my mistake. It should have been:
base-commit: 2a41d0f91443 ("Merge branch 'i2c/for-5.8' into i2c/for-next")
-Sergey
>
> > Changelog v5:
> > - Replace or-assignment with just assignment operator when getting
> > the quirk flags.
> > - Keep alphabetical order of the include statements.
> > - Discard explicit u16-type cast in the dw_reg_write_word() method.
>
> Patches 8 and 11 have been commented solely due to previous comments for
> patches 6 and 7, which I think should be addressed.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH V3] dt-bindings: reset: Convert i.MX reset to json-schema
From: Philipp Zabel @ 2020-05-27 16:14 UTC (permalink / raw)
To: Anson Huang, robh+dt, shawnguo, s.hauer, kernel, festevam,
devicetree, linux-arm-kernel, linux-kernel
Cc: Linux-imx
In-Reply-To: <1589859747-12926-1-git-send-email-Anson.Huang@nxp.com>
Hi Anson,
On Tue, 2020-05-19 at 11:42 +0800, Anson Huang wrote:
> Convert the i.MX reset binding to DT schema format using json-schema.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
Thank you, applied to reset/next.
regards
Philipp
^ permalink raw reply
* Re: [PATCH V2] dt-bindings: reset: Convert i.MX7 reset to json-schema
From: Philipp Zabel @ 2020-05-27 16:14 UTC (permalink / raw)
To: Anson Huang, robh+dt, shawnguo, s.hauer, kernel, festevam,
andrew.smirnov, devicetree, linux-arm-kernel, linux-kernel
Cc: Linux-imx
In-Reply-To: <1589198262-21372-1-git-send-email-Anson.Huang@nxp.com>
Hi Anson,
On Mon, 2020-05-11 at 19:57 +0800, Anson Huang wrote:
> Convert the i.MX7 reset binding to DT schema format using json-schema.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Thank you, applied to reset/next with Rob's and Dong's R-b.
regards
Philipp
^ permalink raw reply
* Re: [PATCH v14 1/2] media: dt-bindings: media: xilinx: Add Xilinx MIPI CSI-2 Rx Subsystem
From: Laurent Pinchart @ 2020-05-27 16:11 UTC (permalink / raw)
To: Vishal Sagar
Cc: Hyun Kwon, mchehab, robh+dt, mark.rutland, Michal Simek,
linux-media, devicetree, hans.verkuil, linux-arm-kernel,
linux-kernel, Dinesh Kumar, Sandip Kothari, Luca Ceresoli,
Jacopo Mondi
In-Reply-To: <1590587839-129558-2-git-send-email-vishal.sagar@xilinx.com>
Hi Vishal,
Thank you for the patch.
On Wed, May 27, 2020 at 07:27:18PM +0530, Vishal Sagar wrote:
> Add bindings documentation for Xilinx MIPI CSI-2 Rx Subsystem.
>
> The Xilinx MIPI CSI-2 Rx Subsystem consists of a CSI-2 Rx controller, a
> D-PHY in Rx mode and a Video Format Bridge.
>
> Signed-off-by: Vishal Sagar <vishal.sagar@xilinx.com>
> Reviewed-by: Hyun Kwon <hyun.kwon@xilinx.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> v14
> - Removed xlnx,csi-pxl-format from required properties
> - Added dependency of xlnx,csi-pxl-format on xlnx,vfb
> - End the yaml file with ...
> - Added Reviewed by Laurent
>
> v13
> - Based on Laurent's suggestions
> - Fixed the datatypes values as minimum and maximum
> - condition added for en-vcx property
>
> v12
> - Moved to yaml format
> - Update CSI-2 and D-PHY
> - Mention that bindings for D-PHY not here
> - reset -> video-reset
>
> v11
> - Modify compatible string from 4.0 to 5.0
>
> v10
> - No changes
>
> v9
> - Fix xlnx,vfb description.
> - s/Optional/Required endpoint property.
> - Move data-lanes description from Ports to endpoint property section.
>
> v8
> - Added reset-gpios optional property to assert video_aresetn
>
> v7
> - Removed the control name from dt bindings
> - Updated the example dt node name to csi2rx
>
> v6
> - Added "control" after V4L2_CID_XILINX_MIPICSISS_ACT_LANES as suggested by Luca
> - Added reviewed by Rob Herring
>
> v5
> - Incorporated comments by Luca Cersoli
> - Removed DPHY clock from description and example
> - Removed bayer pattern from device tree MIPI CSI IP
> doesn't deal with bayer pattern.
>
> v4
> - Added reviewed by Hyun Kwon
>
> v3
> - removed interrupt parent as suggested by Rob
> - removed dphy clock
> - moved vfb to optional properties
> - Added required and optional port properties section
> - Added endpoint property section
>
> v2
> - updated the compatible string to latest version supported
> - removed DPHY related parameters
> - added CSI v2.0 related property (including VCX for supporting upto 16
> virtual channels).
> - modified csi-pxl-format from string to unsigned int type where the value
> is as per the CSI specification
> - Defined port 0 and port 1 as sink and source ports.
> - Removed max-lanes property as suggested by Rob and Sakari
>
> .../bindings/media/xilinx/xlnx,csi2rxss.yaml | 237 +++++++++++++++++++++
> 1 file changed, 237 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml b/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> new file mode 100644
> index 0000000..2282231
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml
> @@ -0,0 +1,237 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/xilinx/xlnx,csi2rxss.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx MIPI CSI-2 Receiver Subsystem
> +
> +maintainers:
> + - Vishal Sagar <vishal.sagar@xilinx.com>
> +
> +description: |
> + The Xilinx MIPI CSI-2 Receiver Subsystem is used to capture MIPI CSI-2
> + traffic from compliant camera sensors and send the output as AXI4 Stream
> + video data for image processing.
> + The subsystem consists of a MIPI D-PHY in slave mode which captures the
> + data packets. This is passed along the MIPI CSI-2 Rx IP which extracts the
> + packet data. The optional Video Format Bridge (VFB) converts this data to
> + AXI4 Stream video data.
> + For more details, please refer to PG232 Xilinx MIPI CSI-2 Receiver Subsystem.
> + Please note that this bindings includes only the MIPI CSI-2 Rx controller
> + and Video Format Bridge and not D-PHY.
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - xlnx,mipi-csi2-rx-subsystem-5.0
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + description: List of clock specifiers
> + items:
> + - description: AXI Lite clock
> + - description: Video clock
> +
> + clock-names:
> + items:
> + - const: lite_aclk
> + - const: video_aclk
> +
> + xlnx,csi-pxl-format:
> + description: |
> + This denotes the CSI Data type selected in hw design.
> + Packets other than this data type (except for RAW8 and
> + User defined data types) will be filtered out.
> + Possible values are as below -
> + 0x1e - YUV4228B
> + 0x1f - YUV42210B
> + 0x20 - RGB444
> + 0x21 - RGB555
> + 0x22 - RGB565
> + 0x23 - RGB666
> + 0x24 - RGB888
> + 0x28 - RAW6
> + 0x29 - RAW7
> + 0x2a - RAW8
> + 0x2b - RAW10
> + 0x2c - RAW12
> + 0x2d - RAW14
> + 0x2e - RAW16
> + 0x2f - RAW20
> + allOf:
> + - $ref: /schemas/types.yaml#/definitions/uint32
> + - anyOf:
> + - minimum: 0x1e
> + - maximum: 0x24
> + - minimum: 0x28
> + - maximum: 0x2f
> +
> + xlnx,vfb:
> + type: boolean
> + description: Present when Video Format Bridge is enabled in IP configuration
> +
> + xlnx,en-csi-v2-0:
> + type: boolean
> + description: Present if CSI v2 is enabled in IP configuration.
> +
> + xlnx,en-vcx:
> + type: boolean
> + description: |
> + When present, there are maximum 16 virtual channels, else only 4.
> +
> + xlnx,en-active-lanes:
> + type: boolean
> + description: |
> + Present if the number of active lanes can be re-configured at
> + runtime in the Protocol Configuration Register. Otherwise all lanes,
> + as set in IP configuration, are always active.
> +
> + video-reset-gpios:
> + description: Optional specifier for a GPIO that asserts video_aresetn.
> + maxItems: 1
> +
> + ports:
> + type: object
> +
> + properties:
> + port@0:
> + type: object
> + description: |
> + Input / sink port node, single endpoint describing the
> + CSI-2 transmitter.
> +
> + properties:
> + reg:
> + const: 0
> +
> + endpoint:
> + type: object
> +
> + properties:
> +
> + data-lanes:
> + description: |
> + This is required only in the sink port 0 endpoint which
> + connects to MIPI CSI-2 source like sensor.
> + The possible values are -
> + 1 - For 1 lane enabled in IP.
> + 1 2 - For 2 lanes enabled in IP.
> + 1 2 3 - For 3 lanes enabled in IP.
> + 1 2 3 4 - For 4 lanes enabled in IP.
> + items:
> + - const: 1
> + - const: 2
> + - const: 3
> + - const: 4
> +
> + remote-endpoint: true
> +
> + required:
> + - data-lanes
> + - remote-endpoint
> +
> + additionalProperties: false
> +
> + additionalProperties: false
> +
> + port@1:
> + type: object
> + description: |
> + Output / source port node, endpoint describing modules
> + connected the CSI-2 receiver.
> +
> + properties:
> +
> + reg:
> + const: 1
> +
> + endpoint:
> + type: object
> +
> + properties:
> +
> + remote-endpoint: true
> +
> + required:
> + - remote-endpoint
> +
> + additionalProperties: false
> +
> + additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> + - ports
> +
> +allOf:
> + - if:
> + required:
> + - xlnx,vfb
> + then:
> + required:
> + - xlnx,csi-pxl-format
> + else:
> + properties:
> + xlnx,csi-pxl-format: false
> +
> + - if:
> + not:
> + required:
> + - xlnx,en-csi-v2-0
> + then:
> + properties:
> + xlnx,en-vcx: false
There's an indentation problem here, it should be
allOf:
- if:
required:
- xlnx,vfb
then:
required:
- xlnx,csi-pxl-format
else:
properties:
xlnx,csi-pxl-format: false
- if:
not:
required:
- xlnx,en-csi-v2-0
then:
properties:
xlnx,en-vcx: false
Have you run the bindings checks ?
make DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/xilinx/xlnx,csi2rxss.yaml dt_binding_check
It would have caught the issue.
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + xcsi2rxss_1: csi2rx@a0020000 {
> + compatible = "xlnx,mipi-csi2-rx-subsystem-5.0";
> + reg = <0x0 0xa0020000 0x0 0x10000>;
I think I mentioned in a previous review that this should be
reg = <0xa0020000 0x10000>;
even if it doesn't match what the real values, as dt_binding_check
compiles the examples in the context of a bus that has #address-cells =
<1> and #size-cells = <1>.
I can fix these when applying the patches to my tree if that's OK with
you, and send a pull request.
> + interrupt-parent = <&gic>;
> + interrupts = <0 95 4>;
> + xlnx,csi-pxl-format = <0x2a>;
> + xlnx,vfb;
> + xlnx,en-active-lanes;
> + xlnx,en-csi-v2-0;
> + xlnx,en-vcx;
> + clock-names = "lite_aclk", "video_aclk";
> + clocks = <&misc_clk_0>, <&misc_clk_1>;
> + video-reset-gpios = <&gpio 86 GPIO_ACTIVE_LOW>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + /* Sink port */
> + reg = <0>;
> + csiss_in: endpoint {
> + data-lanes = <1 2 3 4>;
> + /* MIPI CSI-2 Camera handle */
> + remote-endpoint = <&camera_out>;
> + };
> + };
> + port@1 {
> + /* Source port */
> + reg = <1>;
> + csiss_out: endpoint {
> + remote-endpoint = <&vproc_in>;
> + };
> + };
> + };
> + };
> +...
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH v5 11/11] i2c: designware: Add Baikal-T1 System I2C support
From: Serge Semin @ 2020-05-27 16:10 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Jarkko Nikula, Wolfram Sang, Mika Westerberg,
Alexey Malahov, Thomas Bogendoerfer, Rob Herring, linux-mips,
devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527160544.GM1634618@smile.fi.intel.com>
On Wed, May 27, 2020 at 07:05:44PM +0300, Andy Shevchenko wrote:
> On Wed, May 27, 2020 at 06:30:46PM +0300, Serge Semin wrote:
> > Baikal-T1 System Controller is equipped with a dedicated I2C Controller
> > which functionality is based on the DW APB I2C IP-core, the only
> > difference in a way it' registers are accessed. There are three access
> > register provided in the System Controller registers map, which indirectly
> > address the normal DW APB I2C registers space. So in order to have the
> > Baikal-T1 System I2C Controller supported by the common DW APB I2C driver
> > we created a dedicated Dw I2C controller model quirk, which retrieves the
> > syscon regmap from the parental dt node and creates a new regmap based on
> > it.
>
> ...
>
> > +static struct regmap_config bt1_i2c_cfg = {
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .fast_io = true,
> > + .reg_read = bt1_i2c_read,
> > + .reg_write = bt1_i2c_write,
>
> > + .max_register = DW_IC_COMP_TYPE
>
> Perhaps leave comma here.
If v6 is required, I'll fix it there.
-Sergey
>
> > +};
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v5 08/11] i2c: designware: Convert driver to using regmap API
From: Serge Semin @ 2020-05-27 16:09 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Jarkko Nikula, Wolfram Sang, Mika Westerberg,
Alexey Malahov, Thomas Bogendoerfer, Rob Herring, devicetree,
linux-mips, linux-i2c, linux-kernel
In-Reply-To: <20200527160337.GL1634618@smile.fi.intel.com>
On Wed, May 27, 2020 at 07:03:37PM +0300, Andy Shevchenko wrote:
> On Wed, May 27, 2020 at 06:30:43PM +0300, Serge Semin wrote:
> > Seeing the DW I2C driver is using flags-based accessors with two
> > conditional clauses it would be better to replace them with the regmap
> > API IO methods and to initialize the regmap object with read/write
> > callbacks specific to the controller registers map implementation. This
> > will be also handy for the drivers with non-standard registers mapping
> > (like an embedded into the Baikal-T1 System Controller DW I2C block, which
> > glue-driver is a part of this series).
> >
> > As before the driver tries to detect the mapping setup at probe stage and
> > creates a regmap object accordingly, which will be used by the rest of the
> > code to correctly access the controller registers. In two places it was
> > appropriate to convert the hand-written read-modify-write and
> > read-poll-loop design patterns to the corresponding regmap API
> > ready-to-use methods.
> >
> > Note the regmap IO methods return value is checked only at the probe
> > stage. The rest of the code won't do this because basically we have
> > MMIO-based regmap so non of the read/write methods can fail (this also
> > won't be needed for the Baikal-T1-specific I2C controller).
>
> ...
>
> > + struct regmap_config map_cfg = {
> > + .reg_bits = 32,
> > + .val_bits = 32,
> > + .reg_stride = 4,
> > + .disable_locking = true,
> > + .reg_read = dw_reg_read,
> > + .reg_write = dw_reg_write,
>
> > + .max_register = DW_IC_COMP_TYPE
>
> Perhaps leave comma here as well.
If v6 is required, it'll be fixed there.
-Sergey
>
> > + };
>
> ...
>
> > + /*
> > + * Note we'll check the return value of the regmap IO accessors only
> > + * at the probe stage. The rest of the code won't do this because
> > + * basically we have MMIO-based regmap so non of the read/write methods
> > + * can fail.
> > + */
> > + dev->map = devm_regmap_init(dev->dev, NULL, dev, &map_cfg);
> > + if (IS_ERR(dev->map)) {
> > + dev_err(dev->dev, "Failed to init the registers map\n");
>
> > + return PTR_ERR(dev->map);
> > + }
> > +
> > return 0;
>
> return PTR_ERR_OR_ZERO(dev->map);
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v5 00/11] i2c: designeware: Add Baikal-T1 System I2C support
From: Andy Shevchenko @ 2020-05-27 16:08 UTC (permalink / raw)
To: Serge Semin
Cc: Jarkko Nikula, Wolfram Sang, Serge Semin, Alexey Malahov,
Maxim Kaurkin, Pavel Parkhomenko, Ramil Zaripov,
Ekaterina Skachko, Vadim Vlasov, Alexey Kolotnikov,
Thomas Bogendoerfer, Mika Westerberg, Rob Herring, linux-mips,
linux-i2c, devicetree, linux-kernel
In-Reply-To: <20200527153046.6172-1-Sergey.Semin@baikalelectronics.ru>
On Wed, May 27, 2020 at 06:30:35PM +0300, Serge Semin wrote:
> Jarkko, Wolfram, the merge window is upon us, please review/merge in/whatever
> the patchset.
>
> Initially this has been a small patchset which embedded the Baikal-T1
> System I2C support into the DW APB I2C driver as is by using a simplest
> way. After a short discussion with Andy we decided to implement what he
> suggested (introduce regmap-based accessors and create a glue driver) and
> even more than that to provide some cleanups of the code. So here is what
> this patchset consists of.
>
> First of all we've found out that current implementation of scripts/dtc
> didn't support i2c dt nodes with 10bit and slave flags set in the
> reg property. You'll see an error if you try to dt_binding_check it.
> So the very first patch fixes the problem by adding these flags support
> into the check_i2c_bus_reg() method.
>
> Traditionally we converted the plain text-based DT binding to the DT schema
> and added Baikal-T1 System I2C device support there. This required to mark
> the reg property redundant for Baikal-T1 I2C since its reg-space is
> indirectly accessed by means of the System Controller cmd/read/write
> registers.
>
> Then as Andy suggested we replaced the Synopsys DW APB I2C common driver
> registers IO accessors into the regmap API methods. This doesn't change
> the code logic much, though in two places we managed to replace some bulky
> peaces of code with a ready-to-use regmap methods.
>
> Additionally before adding the glue layer API we initiated a set of cleanups:
> - Define components of the multi-object drivers (like i2c-designware-core.o
> and i2c-designware-paltform.o) with using `-y` suffixed makefile
> variables instead of `-objs` suffixed one. This is encouraged by
> Documentation/kbuild/makefiles.rst text since `-objs` is supposed to be used
> to build host programs.
> - Make DW I2C slave driver depended on the DW I2C core code instead of the
> platform one, which it really is.
> - Move Intel Baytrail semaphore feature to the platform if-clause of the
> kernel config.
>
> After this we finally can introduce the glue layer API for the DW APB I2C
> platform driver. So there are three methods exported from the driver:
> i2c_dw_plat_setup(), i2c_dw_plat_clear(), &i2c_dw_plat_dev_pm_ops to
> setup, cleanup and add PM operations to the glue driven I2C device. Before
> setting the platform DW I2C device up the glue probe code is supposed to
> create an instance of DW I2C device generic object and pre-initialize
> its `struct device` pointer together with optional platform-specific
> flags. In addition to that we converted the MSCC Ocelot SoC I2C specific
> code into the glue layer seeing it's really too specific and, which is more
> important, isn't that complicated so we could unpin it without much of
> worrying to break something.
>
> Meanwhile we discovered that MODEL_CHERRYTRAIL and MODEL_MASK actually
> were no longer used in the code. MODEL_MSCC flag has been discarded since
> the MSCC Ocelot I2C code conversion to the glue driver. So now we can get
> rid of all the MODEL-specific flags.
>
> Finally we introduced a glue driver with Baikal-T1 System I2C device
> support. The driver probe tries to find a syscon regmap, creates the DW
> APB I2C regmap based on it and passes it further to the DW I2C device
> descriptor. Then it does normal DW APB I2C platform setup by calling a
> generic setup method. Cleanup is straightforward. It's just calling a
> generic DW APB I2C clean method.
Thank you for an update.
> This patchset is rebased and tested on the i2c/for-next (5.7-rc7):
Hmm...
> base-commit: 228f95c14949 ("Merge remote-tracking branch 'spi/for-5.8' into spi-next")
...this is strange.
> Changelog v5:
> - Replace or-assignment with just assignment operator when getting
> the quirk flags.
> - Keep alphabetical order of the include statements.
> - Discard explicit u16-type cast in the dw_reg_write_word() method.
Patches 8 and 11 have been commented solely due to previous comments for
patches 6 and 7, which I think should be addressed.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v5 07/11] i2c: designware: Discard Cherry Trail model flag
From: Serge Semin @ 2020-05-27 16:07 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Jarkko Nikula, Wolfram Sang, Mika Westerberg,
Alexey Malahov, Thomas Bogendoerfer, Rob Herring, linux-mips,
devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527155725.GK1634618@smile.fi.intel.com>
On Wed, May 27, 2020 at 06:57:25PM +0300, Andy Shevchenko wrote:
> On Wed, May 27, 2020 at 06:30:42PM +0300, Serge Semin wrote:
> > A PM workaround activated by the flag MODEL_CHERRYTRAIL has been removed
> > since commit 9cbeeca05049 ("i2c: designware: Remove Cherry Trail PMIC I2C
> > bus pm_disabled workaround"), but the flag most likely by mistake has been
> > left in the Dw I2C drivers. Let's remove it.
>
> Same comment as per previous version.
>
> > Since MODEL_MSCC_OCELOT is
> > the only model-flag left, redefine it to be 0x100 so setting a very first
> > bit in the MODEL_MASK bits range.
>
> Cool, but why should we care?
>
> > -#define MODEL_MSCC_OCELOT 0x00000200
> > +#define MODEL_MSCC_OCELOT 0x00000100
>
> We have 4 bits for that, and you are going to reuse this anyway.
>
> I consider this unneeded churn.
I'll leave it as is since prefer keeping the bit setting in the
historical order. Let's see what Jarkko thinks.
-Sergey
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v5 11/11] i2c: designware: Add Baikal-T1 System I2C support
From: Andy Shevchenko @ 2020-05-27 16:05 UTC (permalink / raw)
To: Serge Semin
Cc: Jarkko Nikula, Wolfram Sang, Mika Westerberg, Serge Semin,
Alexey Malahov, Thomas Bogendoerfer, Rob Herring, linux-mips,
devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527153046.6172-12-Sergey.Semin@baikalelectronics.ru>
On Wed, May 27, 2020 at 06:30:46PM +0300, Serge Semin wrote:
> Baikal-T1 System Controller is equipped with a dedicated I2C Controller
> which functionality is based on the DW APB I2C IP-core, the only
> difference in a way it' registers are accessed. There are three access
> register provided in the System Controller registers map, which indirectly
> address the normal DW APB I2C registers space. So in order to have the
> Baikal-T1 System I2C Controller supported by the common DW APB I2C driver
> we created a dedicated Dw I2C controller model quirk, which retrieves the
> syscon regmap from the parental dt node and creates a new regmap based on
> it.
...
> +static struct regmap_config bt1_i2c_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .fast_io = true,
> + .reg_read = bt1_i2c_read,
> + .reg_write = bt1_i2c_write,
> + .max_register = DW_IC_COMP_TYPE
Perhaps leave comma here.
> +};
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v5 06/11] i2c: designware: Add Baytrail sem config DW I2C platform dependency
From: Serge Semin @ 2020-05-27 16:05 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Serge Semin, Jarkko Nikula, Wolfram Sang, Alexey Malahov,
Thomas Bogendoerfer, Mika Westerberg, Rob Herring, linux-mips,
devicetree, linux-i2c, linux-kernel
In-Reply-To: <20200527155533.GJ1634618@smile.fi.intel.com>
On Wed, May 27, 2020 at 06:55:33PM +0300, Andy Shevchenko wrote:
> On Wed, May 27, 2020 at 06:30:41PM +0300, Serge Semin wrote:
> > Currently Intel Baytrail I2C semaphore is a feature of the DW APB I2C
> > platform driver. It's a bit confusing to see it's config in the menu at
> > some separated place with no reference to the platform code. Let's move the
> > config definition to be below the I2C_DESIGNWARE_PLATFORM config and mark
> > it with "depends on I2C_DESIGNWARE_PLATFORM" statement. By doing so the
> > config menu will display the feature right below the DW I2C platform
> > driver item and will indent it to the right so signifying its belonging.
>
> Same comment as per previous version.
>
> > config I2C_DESIGNWARE_BAYTRAIL
> > bool "Intel Baytrail I2C semaphore support"
> > depends on ACPI
> > + depends on I2C_DESIGNWARE_PLATFORM
> > depends on (I2C_DESIGNWARE_PLATFORM=m && IOSF_MBI) || \
> > (I2C_DESIGNWARE_PLATFORM=y && IOSF_MBI=y)
>
> i.e. this change is not needed.
See my response to you in the previous version.
-Sergey
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply
* Re: [PATCH v5 08/11] i2c: designware: Convert driver to using regmap API
From: Andy Shevchenko @ 2020-05-27 16:03 UTC (permalink / raw)
To: Serge Semin
Cc: Jarkko Nikula, Wolfram Sang, Mika Westerberg, Serge Semin,
Alexey Malahov, Thomas Bogendoerfer, Rob Herring, devicetree,
linux-mips, linux-i2c, linux-kernel
In-Reply-To: <20200527153046.6172-9-Sergey.Semin@baikalelectronics.ru>
On Wed, May 27, 2020 at 06:30:43PM +0300, Serge Semin wrote:
> Seeing the DW I2C driver is using flags-based accessors with two
> conditional clauses it would be better to replace them with the regmap
> API IO methods and to initialize the regmap object with read/write
> callbacks specific to the controller registers map implementation. This
> will be also handy for the drivers with non-standard registers mapping
> (like an embedded into the Baikal-T1 System Controller DW I2C block, which
> glue-driver is a part of this series).
>
> As before the driver tries to detect the mapping setup at probe stage and
> creates a regmap object accordingly, which will be used by the rest of the
> code to correctly access the controller registers. In two places it was
> appropriate to convert the hand-written read-modify-write and
> read-poll-loop design patterns to the corresponding regmap API
> ready-to-use methods.
>
> Note the regmap IO methods return value is checked only at the probe
> stage. The rest of the code won't do this because basically we have
> MMIO-based regmap so non of the read/write methods can fail (this also
> won't be needed for the Baikal-T1-specific I2C controller).
...
> + struct regmap_config map_cfg = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> + .disable_locking = true,
> + .reg_read = dw_reg_read,
> + .reg_write = dw_reg_write,
> + .max_register = DW_IC_COMP_TYPE
Perhaps leave comma here as well.
> + };
...
> + /*
> + * Note we'll check the return value of the regmap IO accessors only
> + * at the probe stage. The rest of the code won't do this because
> + * basically we have MMIO-based regmap so non of the read/write methods
> + * can fail.
> + */
> + dev->map = devm_regmap_init(dev->dev, NULL, dev, &map_cfg);
> + if (IS_ERR(dev->map)) {
> + dev_err(dev->dev, "Failed to init the registers map\n");
> + return PTR_ERR(dev->map);
> + }
> +
> return 0;
return PTR_ERR_OR_ZERO(dev->map);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox