* Re: [RESEND PATCH v2 5/7] drm/vc4: Document VEC DT binding
From: Rob Herring @ 2016-12-09 21:00 UTC (permalink / raw)
To: Boris Brezillon
Cc: David Airlie, Daniel Vetter,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Florian Fainelli,
Ray Jui, Scott Branden,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Stephen Warren,
Lee Jones, Eric Anholt,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1480686493-4813-6-git-send-email-boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
On Fri, Dec 02, 2016 at 02:48:11PM +0100, Boris Brezillon wrote:
> Document the DT binding for the VEC (Video EnCoder) IP.
>
> Signed-off-by: Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/3] nvmem: imx-ocotp: Add support for i.MX6UL
From: Rob Herring @ 2016-12-09 21:02 UTC (permalink / raw)
To: Daniel Schultz
Cc: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
mark.rutland-5wv7dgnIgG8, shawnguo-DgEjT+Ai2ygdnm+yROfE0A,
kernel-bIcnvbaLZ9MEGnE8C9+IrQ, fabio.estevam-3arQi8VN3Tc,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <1480689949-17957-1-git-send-email-d.schultz-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
On Fri, Dec 02, 2016 at 03:45:47PM +0100, Daniel Schultz wrote:
> This patch adds OCOTP support for the i.MX6UL SoC.
>
> Signed-off-by: Daniel Schultz <d.schultz-guT5V/WYfQezQB+pC5nmwQ@public.gmane.org>
> ---
> Documentation/devicetree/bindings/nvmem/imx-ocotp.txt | 5 +++--
> drivers/nvmem/imx-ocotp.c | 1 +
> 2 files changed, 4 insertions(+), 2 deletions(-)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] devicetree: add vendor prefix for National Instruments
From: Rob Herring @ 2016-12-09 21:04 UTC (permalink / raw)
To: Nathan Sullivan
Cc: mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480692708-22009-1-git-send-email-nathan.sullivan-acOepvfBmUk@public.gmane.org>
On Fri, Dec 02, 2016 at 09:31:48AM -0600, Nathan Sullivan wrote:
> Signed-off-by: Nathan Sullivan <nathan.sullivan-acOepvfBmUk@public.gmane.org>
> ---
> This is required by "gpio: mmio: add support for NI 169445 NAND GPIO"
>
> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> 1 file changed, 1 insertion(+)
Applied, thanks.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] gpio: mmio: add support for NI 169445 NAND GPIO
From: Rob Herring @ 2016-12-09 21:06 UTC (permalink / raw)
To: Nathan Sullivan
Cc: linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
gnurou-Re5JQEeQqe8AvxtiuMwx3w, mark.rutland-5wv7dgnIgG8,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-gpio-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480693022-22110-1-git-send-email-nathan.sullivan-acOepvfBmUk@public.gmane.org>
On Fri, Dec 02, 2016 at 09:37:02AM -0600, Nathan Sullivan wrote:
> The GPIO-based NAND controller on National Instruments 169445 hardware
> exposes a set of simple lines for the control signals.
>
> Signed-off-by: Nathan Sullivan <nathan.sullivan-acOepvfBmUk@public.gmane.org>
> ---
> "devicetree: add vendor prefix for National Instruments" added the ni vendor prefix.
>
> This patch is needed for "MIPS: NI 169445 board support", so that GPIO NAND can work.
>
> .../bindings/gpio/ni,169445-nand-gpio.txt | 36 ++++++++++++++++++++++
> drivers/gpio/gpio-mmio.c | 1 +
> 2 files changed, 37 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/ni,169445-nand-gpio.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/ni,169445-nand-gpio.txt b/Documentation/devicetree/bindings/gpio/ni,169445-nand-gpio.txt
> new file mode 100644
> index 0000000..ca2c14f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/ni,169445-nand-gpio.txt
> @@ -0,0 +1,36 @@
> +Bindings for the National Instruments 169445 GPIO NAND controller
> +
> +The 169445 GPIO NAND controller has two memory mapped GPIO registers, one
> +for input (the ready signal) and one for output (control signals). It is
> +intended to be used with the GPIO NAND driver.
> +
> +Required properties:
> + - compatible: should be "ni,169445-nand-gpio"
> + - reg-names: must contain
> + "dat" - data register
-names with a single entry is pointless.
> + - reg: address + size pairs describing the GPIO register sets;
> + order must correspond with the order of entries in reg-names
> + - #gpio-cells: must be set to 2. The first cell is the pin number and
> + the second cell is used to specify the gpio polarity:
> + 0 = active high
> + 1 = active low
> + - gpio-controller: Marks the device node as a gpio controller.
> +
> +Examples:
> + gpio1: nand-gpio-out@1f300010 {
gpio-controller@...
> + compatible = "ni,169445-nand-gpio";
> + reg = <0x1f300010 0x4>;
> + reg-names = "dat";
> + gpio-controller;
> + #gpio-cells = <2>;
> + ngpios = <5>;
> + };
> +
> + gpio2: nand-gpio-in@1f300014 {
ditto
> + compatible = "ni,169445-nand-gpio";
> + reg = <0x1f300014 0x4>;
> + reg-names = "dat";
> + gpio-controller;
> + #gpio-cells = <2>;
> + ngpios = <1>;
> + };
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] MIPS: NI 169445 board support
From: Rob Herring @ 2016-12-09 21:18 UTC (permalink / raw)
To: Nathan Sullivan
Cc: ralf-6z/3iImG2C8G8FEW9MqTrA, mark.rutland-5wv7dgnIgG8,
linux-mips-6z/3iImG2C8G8FEW9MqTrA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480693329-22265-1-git-send-email-nathan.sullivan-acOepvfBmUk@public.gmane.org>
On Fri, Dec 02, 2016 at 09:42:09AM -0600, Nathan Sullivan wrote:
> Support the National Instruments 169445 board.
>
> Signed-off-by: Nathan Sullivan <nathan.sullivan-acOepvfBmUk@public.gmane.org>
> ---
> "gpio: mmio: add support for NI 169445 NAND GPIO" and
> "devicetree: add vendor prefix for National Instruments" are required for the
> NAND on this board to work.
These should have been a series, but I already applied the first one.
>
> Documentation/devicetree/bindings/mips/ni.txt | 7 ++
> arch/mips/Kbuild.platforms | 1 +
> arch/mips/Kconfig | 26 ++++++
> arch/mips/boot/dts/Makefile | 1 +
> arch/mips/boot/dts/ni/169445.dts | 99 +++++++++++++++++++++
> arch/mips/boot/dts/ni/Makefile | 9 ++
> arch/mips/configs/ni169445_defconfig | 120 ++++++++++++++++++++++++++
> arch/mips/ni169445/169445-console.c | 38 ++++++++
> arch/mips/ni169445/169445-init.c | 39 +++++++++
> arch/mips/ni169445/169445-int.c | 34 ++++++++
> arch/mips/ni169445/169445-setup.c | 58 +++++++++++++
> arch/mips/ni169445/169445-time.c | 35 ++++++++
> arch/mips/ni169445/Makefile | 9 ++
> arch/mips/ni169445/Platform | 6 ++
> 14 files changed, 482 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mips/ni.txt
> create mode 100644 arch/mips/boot/dts/ni/169445.dts
> create mode 100644 arch/mips/boot/dts/ni/Makefile
> create mode 100644 arch/mips/configs/ni169445_defconfig
> create mode 100644 arch/mips/ni169445/169445-console.c
> create mode 100644 arch/mips/ni169445/169445-init.c
> create mode 100644 arch/mips/ni169445/169445-int.c
> create mode 100644 arch/mips/ni169445/169445-setup.c
> create mode 100644 arch/mips/ni169445/169445-time.c
> create mode 100644 arch/mips/ni169445/Makefile
> create mode 100644 arch/mips/ni169445/Platform
>
> diff --git a/Documentation/devicetree/bindings/mips/ni.txt b/Documentation/devicetree/bindings/mips/ni.txt
> new file mode 100644
> index 0000000..722bf2d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mips/ni.txt
> @@ -0,0 +1,7 @@
> +National Instruments MIPS platforms
> +
> +required root node properties:
> + - compatible: must be "ni,169445"
> +
> +CPU Nodes
> + - compatible: must be "mti,mips14KEc"
> diff --git a/arch/mips/Kbuild.platforms b/arch/mips/Kbuild.platforms
> index f5f1bdb..f2d7b5c 100644
> --- a/arch/mips/Kbuild.platforms
> +++ b/arch/mips/Kbuild.platforms
> @@ -20,6 +20,7 @@ platforms += loongson32
> platforms += loongson64
> platforms += mti-malta
> platforms += netlogic
> +platforms += ni169445
> platforms += paravirt
> platforms += pic32
> platforms += pistachio
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index b3c5bde..d24d11f 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -574,6 +574,32 @@ config NXP_STB225
> help
> Support for NXP Semiconductors STB225 Development Board.
>
> +config NI_169445
> + bool "NI 169445 board"
> + select ARCH_WANT_OPTIONAL_GPIOLIB
> + select BOOT_ELF32
> + select BOOT_RAW
> + select BUILTIN_DTB
> + select CEVT_R4K
> + select CSRC_R4K
> + select CPU_MIPSR2_IRQ_VI
> + select CPU_MIPSR2_IRQ_EI
> + select DMA_NONCOHERENT
> + select IRQ_MIPS_CPU
> + select LIBFDT
> + select MIPS_MSC
> + select SYS_HAS_CPU_MIPS32_R1
> + select SYS_HAS_CPU_MIPS32_R2
> + select SYS_HAS_EARLY_PRINTK
> + select SYS_SUPPORTS_32BIT_KERNEL
> + select SYS_SUPPORTS_LITTLE_ENDIAN
> + select USE_OF
> + select COMMON_CLK
> + help
> + This enables support for the National Instruments 169445A
> + board.
> +
> +
> config PMC_MSP
> bool "PMC-Sierra MSP chipsets"
> select CEVT_R4K
> diff --git a/arch/mips/boot/dts/Makefile b/arch/mips/boot/dts/Makefile
> index fc7a0a9..65a0ad8 100644
> --- a/arch/mips/boot/dts/Makefile
> +++ b/arch/mips/boot/dts/Makefile
> @@ -3,6 +3,7 @@ dts-dirs += cavium-octeon
> dts-dirs += ingenic
> dts-dirs += lantiq
> dts-dirs += mti
> +dts-dirs += ni
> dts-dirs += netlogic
> dts-dirs += pic32
> dts-dirs += qca
> diff --git a/arch/mips/boot/dts/ni/169445.dts b/arch/mips/boot/dts/ni/169445.dts
> new file mode 100644
> index 0000000..a2b49f9
> --- /dev/null
> +++ b/arch/mips/boot/dts/ni/169445.dts
> @@ -0,0 +1,99 @@
> +/dts-v1/;
> +
> +/ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "ni,169445";
> +
> + cpus {
> + mips-hpt-frequency = <25000000>;
> +
> + cpu@0 {
> + compatible = "mti,mips14KEc";
> + };
> + };
> +
> + memory {
memory@0
> + device_type = "memory";
> + reg = <0x0 0x08000000>;
> + };
> +
> + clocks {
> + baseclk: baseclock {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <50000000>;
> + };
> + };
> +
> + cpu_intc: cpu_intc {
> + #address-cells = <0>;
> + compatible = "mti,cpu-interrupt-controller";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + };
> +
> + ahb@0 {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
If everything is under 0x1f3xxxxx, then add in the ranges here (and the
unit address).
> +
> + gpio1: nand-gpio-out@1f300010 {
As in the binding example: gpio-controller@...
> + compatible = "ni,169445-nand-gpio";
> + reg = <0x1f300010 0x4>;
> + reg-names = "dat";
> + gpio-controller;
> + #gpio-cells = <2>;
> + ngpios = <5>;
> + };
> +
> + gpio2: nand-gpio-in@1f300014 {
ditto
> + compatible = "ni,169445-nand-gpio";
> + reg = <0x1f300014 0x4>;
> + reg-names = "dat";
> + gpio-controller;
> + #gpio-cells = <2>;
> + ngpios = <1>;
> + };
> +
> + nand@1f300000 {
> + compatible = "gpio-control-nand";
> + nand-on-flash-bbt;
> + nand-ecc-mode = "soft_bch";
> + nand-ecc-step-size = <512>;
> + nand-ecc-strength = <4>;
> + reg = <0x1f300000 4>;
> + gpios = <&gpio2 0 0>, /* rdy */
> + <&gpio1 1 0>, /* nce */
> + <&gpio1 2 0>, /* ale */
> + <&gpio1 3 0>, /* cle */
> + <&gpio1 4 0>; /* nwp */
> + };
> +
> + serial@1f380000 {
> + compatible = "ns16550a";
> + reg = <0x1f380000 0x1000>;
> + interrupt-parent = <&cpu_intc>;
> + interrupts = <6>;
> + clocks = <&baseclk>;
> + reg-shift = <0>;
> + };
> +
> + ethernet@1f340000 {
> + compatible = "snps,dwc-qos-ethernet-4.10";
> + interrupt-parent = <&cpu_intc>;
> + interrupts = <5>;
> + reg = <0x1f340000 0x2000>;
> + clock-names = "apb_pclk", "phy_ref_clk";
> + clocks = <&baseclk>, <&baseclk>;
> +
> + phy-mode = "rgmii";
> +
> + fixed-link {
> + speed = <1000>;
> + full-duplex;
> + };
> + };
> + };
> +};
[...]
> diff --git a/arch/mips/ni169445/169445-setup.c b/arch/mips/ni169445/169445-setup.c
> new file mode 100644
> index 0000000..80a5c91
> --- /dev/null
> +++ b/arch/mips/ni169445/169445-setup.c
> @@ -0,0 +1,58 @@
> +/* Copyright 2016 National Instruments Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +#include <linux/init.h>
> +#include <linux/clk-provider.h>
> +#include <linux/libfdt.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_fdt.h>
> +
> +#include <asm/prom.h>
> +#include <asm/fw/fw.h>
> +
> +#include <asm/mips-boards/generic.h>
> +
> +const char *get_system_type(void)
> +{
> + return "NI 169445 FPGA";
Perhaps this should come from model property. There's a patch in flight
to add a function to get machine name.
> +}
> +
> +void __init plat_mem_setup(void)
> +{
> + /*
> + * Load the builtin devicetree. This causes the chosen node to be
> + * parsed resulting in our memory appearing
> + */
> + __dt_setup_arch(__dtb_start);
> +}
> +
> +void __init device_tree_init(void)
> +{
> + if (!initial_boot_params)
> + return;
> +
> + unflatten_and_copy_device_tree();
> +}
> +
> +static int __init customize_machine(void)
> +{
> + of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
This should happen by default now.
> + return 0;
> +}
> +arch_initcall(customize_machine);
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/3] crypto: brcm: DT documentation for Broadcom SPU driver
From: Rob Herring @ 2016-12-09 21:26 UTC (permalink / raw)
To: Rob Rice
Cc: Herbert Xu, David S. Miller, Mark Rutland,
linux-crypto-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Ray Jui, Scott Branden,
Jon Mason, bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
Catalin Marinas, Will Deacon,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Steve Lin
In-Reply-To: <1480714499-1476-2-git-send-email-rob.rice-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
On Fri, Dec 02, 2016 at 04:34:57PM -0500, Rob Rice wrote:
> Device tree documentation for Broadcom Secure Processing Unit
> (SPU) crypto driver.
>
> Signed-off-by: Steve Lin <steven.lin1-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Rob Rice <rob.rice-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> ---
> .../devicetree/bindings/crypto/brcm,spu-crypto.txt | 25 ++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>
> diff --git a/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
> new file mode 100644
> index 0000000..e5fe942
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
> @@ -0,0 +1,25 @@
> +The Broadcom Secure Processing Unit (SPU) driver supports symmetric
Bindings describe h/w, not drivers.
> +cryptographic offload for Broadcom SoCs with SPU hardware. A SoC may have
> +multiple SPU hardware blocks.
> +
> +Required properties:
> +- compatible : Should be "brcm,spum-crypto" for devices with SPU-M hardware
Additionally, you should have SoC specific compatible here.
> + (e.g., Northstar2) or "brcm,spum-nsp-crypto" for the Northstar Plus variant
> + of the SPU-M hardware.
> +
> +- reg: Should contain SPU registers location and length.
> +- mboxes: A list of mailbox channels to be used by the kernel driver. Mailbox
> +channels correspond to DMA rings on the device.
What determines the mbox assignment?
Needs to specify how many.
> +
> +Example:
> + spu-crypto@612d0000 {
Just crypto@...
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCHv4 11/15] clk: ti: clockdomain: add clock provider support to clockdomains
From: Michael Turquette @ 2016-12-09 21:28 UTC (permalink / raw)
To: Tony Lindgren
Cc: devicetree, Stephen Boyd, Tero Kristo, Rob Herring, linux-omap,
linux-clk, linux-arm-kernel
In-Reply-To: <20161209204015.GL4920@atomide.com>
Quoting Tony Lindgren (2016-12-09 12:40:16)
> * Michael Turquette <mturquette@baylibre.com> [161209 12:02]:
> > Quoting Tony Lindgren (2016-12-05 07:25:34)
> > > * Tero Kristo <t-kristo@ti.com> [161205 02:09]:
> ...
> <snip>
>
> > I had a recent conversation with Kevin Hilman about a related issue
> > (we were not discussing this thread or this series) and we both agreed
> > that most drivers don't even need to managed their clocks directly, so
> > much as they need to manage their on/off resources. Clocks are just one
> > part of that, and if we can hide that stuff inside of an attached genpd
> > then it would be better than having the driver manage clocks explicitly.
> >
> > Obviously some devices such as audio codec or uart will need to manage
> > clocks directly, but this is mostly the exception, not the rule.
>
> Yes. And we do that already for clkctrl clocks via PM runtime where
> hwmod manages them. Tero's series still has hwmod manage the clocks,
> but the clock register handling is moved to live under drivers/clock.
>
> > > > > > There is certainly no API for that in the clock framework, but for genpd
> > > > > > your runtime_pm_get() callback for clkdm_A could call runtime_pm_get
> > > > > > against clkdm_B, which would satisfy the requirement. See section
> > > > > > 3.1.1.1.7 Clock Domain Dependency in the OMAP4 TRM, version AB.
> > > >
> > > > For static dependencies the apis genpd_add/remove_subdomain could probably
> > > > be used.
> > > >
> > > > > To me it seems the API is just clk_get() :) Do you have some
> > > > > specific example we can use to check? My guess is that the
> > > > > TRM "Clock Domain Dependency" is just the usual parent child
> > > > > relationship between clocks that are the clockdomains..
> >
> > clk_get() only fetches a pointer to the clk. I guess you mean
> > clk_prepare_enable() to actually increment the use count?
>
> Right, with clocks that's all we should need to do :)
>
> > If we used the clk framework here is that it would look something like
> > this:
> >
> > clk_enable(clk_a)
> > -> .enable(clk_a_hw)
> > -> clk_enable(clk_b)
> >
> > However, clk_a and clk_b do not have a parent-child relationship in the
> > clock tree. This is purely a functional relationship between IP blocks.
> > Modeling this sort of thing in the clk framework would be wrong, and
> > genpd is a much better place to establish these arbitrary relationships.
>
> Hmm yes, and I don't mean the clock framework should do anything more
> complex beyond what it already does.
>
> We just want to represent the clocks as clocks, then have the
> interconnect code manage those clocks. That's currently hwmod, eventually
> it will be genpd.
>
> > > > > If there is something more magical there certainly that should
> > > > > be considered though.
> > > >
> > > > The hwmods could be transformed to individual genpds also I guess. On DT
> > > > level though, we would still need a clock pointer to the main clock and a
> > > > genpd pointer in addition to that.
> > >
> > > Hmm a genpd pointer to where exactly? AFAIK each interconnect
> > > instance should be a genpd provider, and the individual interconnect
> > > target modules should be consumers for that genpd.
> >
> > I was thinking that the clock domains would be modeled as genpd objects
> > with the interconnect target modules attached as struct devices.
>
> I think clock domains should be just clocks, then we let the interconnect
> code and eventually genpd manage them.
>
> > > > Tony, any thoughts on that? Would this break up the plans for the
> > > > interconnect completely?
> > >
> > > Does using genpd for clockdomains cause issues for using genpd for
> > > interconnect instances and the target modules?
> >
> > Can they be the same object in Linux? If there is a one-to-one mapping
> > between clock domains and the interconnect port then maybe you can just
> > model them together.
>
> I'm thinking that it should be the interconnect code implementing
> genpd, and use clk_request_enable().
>
> > > The thing I'd be worried about there is that the clockdomains and
> > > their child clocks are just devices sitting on the interconnect,
> > > so we could easily end up with genpd modeling something that does
> > > not represent the hardware.
> > >
> > > For example, on 4430 we have:
> > >
> > > l4_cfg interconnect
> > > ...
> > > segment@0
> > > ...
> > > target_module@4000
> > > cm1: cm1@0
> >
> > How about:
> >
> > l4_cfg interconnect
> > ...
> > segment@0
> > ...
> > cm1@4000
> > module: foo_module@0
>
> That's the wrong way around from hardware point of view. There's
> a generic interconnect wrapper module with it's own registers,
> then cm1 (and possibly other devices) are children of that target
> module.
>
> > I don't know much about the segments. Do they map one-to-one with the
> > clock domains?
>
> I need to check, it's been a while, but I recall some interconnects
> are partioned to segments based on voltages or clocks.
>
> > If my quick-and-dirty DT above makes sense, then the target modules
> > (e.g. io controller) would not get clocks anymore, but just
> > pm_runtime_get(). The genpd backing object would call clk_enable/disable
> > as needed.
>
> Yeah that's what we already have with hwmod and PM runtime for the
> clockctrl register. But hwmod currently directly manages the clkctrl
> register, we just want to move that part to be a clock driver.
>
> The children of the interconnect target modules just need to use
> PM runtime, but the interconnect target module driver needs to know
> it's clkctrl clock.
OK, that sounds good to me but I'm not quite sure about the difference
between "children of interconnect target modules" versus just the
"interconnect target module".
Can you give an example on each? I just want to understand for my own
curiosity.
>
> > If fine grained control of a clock is needed (e.g. for clk_set_rate)
> > then the driver can still clk_get it. Whether or not the clockdomain
> > provides that clock or if it comes from the clock generator (e.g. cm1,
> > cm2, prm, etc) isn't as important to me, but I prefer for the
> > clockdomain to not be a clock provider if possible.
>
> Yeah I totally agree with that, and that's already what we mostly
> have.
>
> > > I don't at least yet
> > > follow what we need to do with the clockdomains with genpd :)
> >
> > Use the clockdomain genpd to call clk_enable/disable under the hood.
> > Don't use them as clock providers to the target modules. Clockdomain
> > genpds would be the clock consumers.
>
> I don't think the clockdomain should be a genpd provider because
> that creates a genpd network of dependencies instead of a tree
> structure. If we end up setting the clockdomains with genpd, then
> only the other clockdomains should use them, but I don't know how
> we ever keep drivers from directly tinkering with them..
Genpd is set up as an arbitrary graph, not strictly a tree, so these
types of dependencies should be OK.
>
> IMO, the clockdomain clock driver should just provides clocks, then
> we can have the interconnect target module driver deal with the
> clockdomain dependencies.
>
> > > Wouldn't just doing clk_get() from one clockdomain clock to
> > > another clockdomain clock (or it's output) be enough to
> > > represent the clockdomain dependencies?
> >
> > s/clk_get/clk_prepare_enable/
> >
> > Yes, but you're stuffing functional dependencies into the clock tree,
> > which sucks. genpd was created to model these arbitrary dependencies.
>
> Well let's not stuff anything beyond clock framework to the
> clockdomain clock drivers. We already have the clockdomain
> dependencies handled by the interconnect code (hwmod), and there
> should be no problem moving those to be handled by genpd and the
> interconnect target driver instances.
>
> Care to take another look at Tero's patches with the assumption
> that the clockdomain clocks stay just as a clocks?
Sure.
Regards,
Mike
>
> Regards,
>
> Tony
^ permalink raw reply
* Re: [PATCH net-next v3 1/2] net: dt-bindings: add RGMII TX delay configuration to meson8b-dwmac
From: Rob Herring @ 2016-12-09 21:31 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
mark.rutland-5wv7dgnIgG8, carlo-KA+7E9HrN00dnm+yROfE0A,
khilman-rdvid1DuHRBWk0Htik3J/w, peppe.cavallaro-qxv4g6HH51o,
alexandre.torgue-qxv4g6HH51o,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161202233238.17561-2-martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
On Sat, Dec 03, 2016 at 12:32:37AM +0100, Martin Blumenstingl wrote:
> This allows configuring the RGMII TX clock delay. The RGMII clock is
> generated by underlying hardware of the the Meson 8b / GXBB DWMAC glue.
> The configuration depends on the actual hardware (no delay may be
> needed due to the design of the actual circuit, the PHY might add this
> delay, etc.).
>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
> Tested-by: Neil Armstrong <narmstrong-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
> ---
> Documentation/devicetree/bindings/net/meson-dwmac.txt | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: usb:xhci: support disable usb2 LPM Remote Wakeup
From: Rob Herring @ 2016-12-09 21:36 UTC (permalink / raw)
To: Thang Q. Nguyen
Cc: Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Mathias Nyman,
Greg Kroah-Hartman, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, Phong Vo, Loc Ho, Vu Nguyen,
patches-qTEPVZfXA3Y
In-Reply-To: <1480855321-5047-1-git-send-email-tqnguyen-qTEPVZfXA3Y@public.gmane.org>
On Sun, Dec 04, 2016 at 07:42:01PM +0700, Thang Q. Nguyen wrote:
> From: Thang Nguyen <tqnguyen-qTEPVZfXA3Y@public.gmane.org>
>
> As per USB 2.0 link power management addendum ECN, table 1-2, page 4,
> device or host initiated via resume signaling; device-initiated resumes
> can be optionally enabled/disabled by software. This patch adds support
> to control enabling the USB2 RWE feature via DT/ACPI attribute.
>
> Signed-off-by: Vu Nguyen <vnguyen-qTEPVZfXA3Y@public.gmane.org>
> Signed-off-by: Thang Nguyen <tqnguyen-qTEPVZfXA3Y@public.gmane.org>
> ---
> Documentation/devicetree/bindings/usb/usb-xhci.txt | 1 +
> drivers/usb/host/xhci-plat.c | 3 +++
> drivers/usb/host/xhci.c | 5 ++++-
> drivers/usb/host/xhci.h | 1 +
> 4 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> index 966885c..9b4cd14 100644
> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> @@ -25,6 +25,7 @@ Required properties:
>
> Optional properties:
> - clocks: reference to a clock
> + - usb2-rwe-disable: disable USB2 LPM Remote Wakeup capable
Remote wakeup has been around since USB 1.0 days. Does this need to be
USB2 or XHCI specific?
> - usb3-lpm-capable: determines if platform is USB3 LPM capable
>
> Example:
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] dt: pwm: bcm2835: fix typo in clocks property name
From: Rob Herring @ 2016-12-09 21:38 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Thierry Reding, Stephen Warren, Eric Anholt, Florian Fainelli,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
linux-pwm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161205001025.13909-1-vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
On Mon, Dec 05, 2016 at 02:10:25AM +0200, Vladimir Zapolskiy wrote:
> According to the examples of BCM2835 PWM device nodes there is a typo in
> 'clocks' property name, which is a common property name on clock consumer
> side to store a phandle to an input clock.
>
> Signed-off-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
> ---
> Documentation/devicetree/bindings/pwm/pwm-bcm2835.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Applied, thanks.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/3] dt: pwm: lpc32xx: add description of clocks and #pwm-cells properties
From: Rob Herring @ 2016-12-09 21:41 UTC (permalink / raw)
To: Vladimir Zapolskiy
Cc: Thierry Reding, Sylvain Lemieux, linux-pwm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20161205014237.1689-2-vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
On Mon, Dec 05, 2016 at 03:42:37AM +0200, Vladimir Zapolskiy wrote:
> NXP LPC32xx SoCs have two simple independent PWM controllers with a single
> output each, in this case there is no need to specify PWM channel argument
> on client side, one cell for setting PWM output frequency is sufficient.
>
> Another added to the description property 'clocks' has a standard meaning
> of a controller supply clock, in the LPC32xx User's Manual the clock is
> denoted as PWM1_CLK or PWM2_CLK clock.
>
> Signed-off-by: Vladimir Zapolskiy <vz-ChpfBGZJDbMAvxtiuMwx3w@public.gmane.org>
> ---
> Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
> index 74b5bc5..523d796 100644
> --- a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
> +++ b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
> @@ -3,15 +3,22 @@ LPC32XX PWM controller
> Required properties:
> - compatible: should be "nxp,lpc3220-pwm"
> - reg: physical base address and length of the controller's registers
> +- clocks: clock phandle and clock specifier pair
> +- #pwm-cells: should be 1, the cell is used to specify the period in
> + nanoseconds.
This use of the cell is a bit odd as the period is s/w config and this
would typically be a channel selection or such.
What if I want user specified/changed periods?
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v7 1/5] mmc: dt-bindings: add ZTE ZX296718 MMC bindings
From: Rob Herring @ 2016-12-09 21:47 UTC (permalink / raw)
To: Jun Nie
Cc: mark.rutland, shawn.guo, xie.baoyou, devicetree, ulf.hansson,
jh80.chung, jason.liu, chen.chaokai, lai.binz, linux-mmc
In-Reply-To: <1480904976-7081-2-git-send-email-jun.nie@linaro.org>
On Mon, Dec 05, 2016 at 10:29:32AM +0800, Jun Nie wrote:
> Document the device-tree binding of ZTE MMC host on
> ZX296718 SoC.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
> .../devicetree/bindings/mmc/zx-dw-mshc.txt | 35 ++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
>
> diff --git a/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
> new file mode 100644
> index 0000000..c175c4b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mmc/zx-dw-mshc.txt
> @@ -0,0 +1,35 @@
> +* ZTE specific extensions to the Synopsys Designware Mobile Storage
> + Host Controller
> +
> +The Synopsys designware mobile storage host controller is used to interface
> +a SoC with storage medium such as eMMC or SD/MMC cards. This file documents
> +differences between the core Synopsys dw mshc controller properties described
> +by synopsys-dw-mshc.txt and the properties used by the ZTE specific
> +extensions to the Synopsys Designware Mobile Storage Host Controller.
> +
> +Required Properties:
> +
> +* compatible: should be
> + - "zte,zx296718-dw-mshc": for ZX SoCs
> +
> +Example:
> +
> + mmc1: mmc@1110000 {
> + compatible = "zte,zx296718-dw-mshc";
> + #address-cells = <1>;
> + #size-cells = <0>;
These aren't needed unless you have child nodes with reg property. The
DW binding says you should have at least one child.
> + reg = <0x01110000 0x1000>;
> + interrupts = <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
> + fifo-depth = <32>;
> + data-addr = <0x200>;
> + fifo-watermark-aligned;
Custom properties should have vendor prefix.
> + bus-width = <4>;
> + clock-frequency = <50000000>;
> + clocks = <&topcrm SD0_AHB>, <&topcrm SD0_WCLK>;
> + clock-names = "biu", "ciu";
> + num-slots = <1>;
> + max-frequency = <50000000>;
> + cap-sdio-irq;
> + cap-sd-highspeed;
> + status = "disabled";
> + };
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH v7 3/5] Documentation: synopsys-dw-mshc: add binding for fifo quirks
From: Rob Herring @ 2016-12-09 21:51 UTC (permalink / raw)
To: Jun Nie
Cc: mark.rutland, shawn.guo, xie.baoyou, devicetree, ulf.hansson,
jh80.chung, jason.liu, chen.chaokai, lai.binz, linux-mmc
In-Reply-To: <1480904976-7081-4-git-send-email-jun.nie@linaro.org>
On Mon, Dec 05, 2016 at 10:29:34AM +0800, Jun Nie wrote:
> Add fifo-addr property and fifo-watermark-quirk property to
> synopsys-dw-mshc bindings. It is intended to provide more
> dt interface to support SoCs specific configuration.
>
> Signed-off-by: Jun Nie <jun.nie@linaro.org>
> ---
> Documentation/devicetree/bindings/mmc/synopsys-dw-mshc.txt | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
This patch should come before patch 1 since you use these there.
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCHv4 11/15] clk: ti: clockdomain: add clock provider support to clockdomains
From: Tony Lindgren @ 2016-12-09 21:58 UTC (permalink / raw)
To: Michael Turquette
Cc: Tero Kristo, Stephen Boyd, linux-omap, linux-clk,
linux-arm-kernel, devicetree, Rob Herring
In-Reply-To: <148131892585.16621.5033331614135070003@resonance>
* Michael Turquette <mturquette@baylibre.com> [161209 13:28]:
> Quoting Tony Lindgren (2016-12-09 12:40:16)
> > Yeah that's what we already have with hwmod and PM runtime for the
> > clockctrl register. But hwmod currently directly manages the clkctrl
> > register, we just want to move that part to be a clock driver.
> >
> > The children of the interconnect target modules just need to use
> > PM runtime, but the interconnect target module driver needs to know
> > it's clkctrl clock.
>
> OK, that sounds good to me but I'm not quite sure about the difference
> between "children of interconnect target modules" versus just the
> "interconnect target module".
The interconnect target module specific registers are "sysc", "syss"
and "revision". Those are usually stuffed to random addresses, and
currently managed by hwmod. Each interconnect target module has a
module gate clock "clkctrl" in some clockdomain. Each clockdomain
has multiple similar "clckctrl" clocks.
> Can you give an example on each? I just want to understand for my own
> curiosity.
For example, musb on am335x has these as children of a single
interconnect target module:
- Two instances of musb wrapper IP
- Two instances of musb IP
- One instance of CPP41 DMA
The on omap4, the whole system control module is just a single
interconnect target module with tens of devices in it like padconf,
clocks, pbias regulator and whatever random devices.
> > I don't think the clockdomain should be a genpd provider because
> > that creates a genpd network of dependencies instead of a tree
> > structure. If we end up setting the clockdomains with genpd, then
> > only the other clockdomains should use them, but I don't know how
> > we ever keep drivers from directly tinkering with them..
>
> Genpd is set up as an arbitrary graph, not strictly a tree, so these
> types of dependencies should be OK.
Well maybe that works then, worth checking for sure.
Regards,
Tony
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: drm/bridge: adv7511: Add regulator bindings
From: Rob Herring @ 2016-12-09 22:11 UTC (permalink / raw)
To: Archit Taneja; +Cc: laurent.pinchart, linux-arm-msm, dri-devel, devicetree
In-Reply-To: <1480924435-20882-2-git-send-email-architt@codeaurora.org>
On Mon, Dec 05, 2016 at 01:23:54PM +0530, Archit Taneja wrote:
> Add the regulator supply properties needed by ADV7511 and ADV7533.
>
> Cc: devicetree@vger.kernel.org
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Archit Taneja <architt@codeaurora.org>
> ---
> Documentation/devicetree/bindings/display/bridge/adi,adv7511.txt | 8 ++++++++
> 1 file changed, 8 insertions(+)
Didn't I ack this already? Anyway,
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 1/4] net: hix5hd2_gmac: add generic compatible string
From: Rob Herring @ 2016-12-09 22:35 UTC (permalink / raw)
To: Dongpo Li
Cc: mark.rutland, mturquette, sboyd, linux, zhangfei.gao,
yisen.zhuang, salil.mehta, davem, arnd, andrew, xuejiancheng,
benjamin.chenhao, caizhiyong, netdev, devicetree, linux-kernel
In-Reply-To: <1480944481-118803-2-git-send-email-lidongpo@hisilicon.com>
On Mon, Dec 05, 2016 at 09:27:58PM +0800, Dongpo Li wrote:
> The "hix5hd2" is SoC name, add the generic ethernet driver name.
> The "hisi-gemac-v1" is the basic version and "hisi-gemac-v2" adds
> the SG/TXCSUM/TSO/UFO features.
>
> Signed-off-by: Dongpo Li <lidongpo@hisilicon.com>
> ---
> .../devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt | 9 +++++++--
> drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 15 +++++++++++----
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
> index 75d398b..75920f0 100644
> --- a/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
> +++ b/Documentation/devicetree/bindings/net/hisilicon-hix5hd2-gmac.txt
> @@ -1,7 +1,12 @@
> Hisilicon hix5hd2 gmac controller
>
> Required properties:
> -- compatible: should be "hisilicon,hix5hd2-gmac".
> +- compatible: should contain one of the following SoC strings:
> + * "hisilicon,hix5hd2-gemac"
> + * "hisilicon,hi3798cv200-gemac"
> + and one of the following version string:
> + * "hisilicon,hisi-gemac-v1"
> + * "hisilicon,hisi-gemac-v2"
What combinations are valid? I assume both chips don't have both v1 and
v2. 2 SoCs and 2 versions so far, I don't think there is much point to
have the v1 and v2 compatible strings.
> - reg: specifies base physical address(s) and size of the device registers.
> The first region is the MAC register base and size.
> The second region is external interface control register.
> @@ -20,7 +25,7 @@ Required properties:
>
> Example:
> gmac0: ethernet@f9840000 {
> - compatible = "hisilicon,hix5hd2-gmac";
> + compatible = "hisilicon,hix5hd2-gemac", "hisilicon,hisi-gemac-v1";
You can't just change compatible strings.
> reg = <0xf9840000 0x1000>,<0xf984300c 0x4>;
> interrupts = <0 71 4>;
> #address-cells = <1>;
^ permalink raw reply
* Re: [PATCH v3 1/6] mfd: dt: Fix "indicates" typo in mfd bindings document
From: Rob Herring @ 2016-12-09 22:42 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Lee Jones, Mark Rutland, Linus Walleij, Corey Minyard,
Cédric Le Goater, Joel Stanley,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161206025321.1792-2-andrew-zrmu5oMJ5Fs@public.gmane.org>
On Tue, Dec 06, 2016 at 01:53:16PM +1100, Andrew Jeffery wrote:
> Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
> ---
> Documentation/devicetree/bindings/mfd/mfd.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 2/6] mfd: dt: ranges, #address-cells and #size-cells as optional properties
From: Rob Herring @ 2016-12-09 22:49 UTC (permalink / raw)
To: Andrew Jeffery
Cc: Lee Jones, Mark Rutland, Linus Walleij, Corey Minyard,
Cédric Le Goater, Joel Stanley,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161206025321.1792-3-andrew-zrmu5oMJ5Fs@public.gmane.org>
On Tue, Dec 06, 2016 at 01:53:17PM +1100, Andrew Jeffery wrote:
> Whilst describing a device and not a bus, simple-mfd is modelled on
> simple-bus where child nodes are iterated and registered as platform
> devices. Some complex devices, e.g. the Aspeed LPC controller, can
> benefit from address space mapping such that child nodes can use the
> regs property to describe their resources within the multi-function
> device.
>
> Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
> ---
> Documentation/devicetree/bindings/mfd/mfd.txt | 10 ++++++++++
> 1 file changed, 10 insertions(+)
No objections to this, but this is all implied by having a reg property.
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 2/6] mfd: dt: ranges, #address-cells and #size-cells as optional properties
From: Andrew Jeffery @ 2016-12-09 22:55 UTC (permalink / raw)
To: Rob Herring
Cc: Lee Jones, Mark Rutland, Linus Walleij, Corey Minyard,
Cédric Le Goater, Joel Stanley,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161209224927.3indhfdjgyndtjid@rob-hp-laptop>
[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]
On Fri, 2016-12-09 at 16:49 -0600, Rob Herring wrote:
> On Tue, Dec 06, 2016 at 01:53:17PM +1100, Andrew Jeffery wrote:
> > Whilst describing a device and not a bus, simple-mfd is modelled on
> > simple-bus where child nodes are iterated and registered as platform
> > devices. Some complex devices, e.g. the Aspeed LPC controller, can
> > benefit from address space mapping such that child nodes can use the
> > regs property to describe their resources within the multi-function
> > device.
> >
> > > > Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
> > ---
> > Documentation/devicetree/bindings/mfd/mfd.txt | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
>
> No objections to this, but this is all implied by having a reg property.
Thanks for clarifying. I wasn't sure so I wrote the patch with the
thought that we could drop it if it wasn't necessary. Regardless, I
think being explicit about the properties is nice.
>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Thanks,
Andrew
>
> Rob
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH 2/6] net: ethernet: ti: cpts: add support for ext rftclk selection
From: Grygorii Strashko @ 2016-12-09 23:29 UTC (permalink / raw)
To: Stephen Boyd
Cc: Richard Cochran, Murali Karicheri, David S. Miller,
netdev-u79uwXL29TY76Z2rM5mHXA, Mugunthan V N, Sekhar Nori,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-omap-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA, Wingman Kwok,
linux-clk-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161209004745.GJ5423-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 12/08/2016 06:47 PM, Stephen Boyd wrote:
> On 12/06, Grygorii Strashko wrote:
>> Subject: [PATCH] cpts refclk sel
>>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko-l0cyMroinI0@public.gmane.org>
>> ---
>> arch/arm/boot/dts/keystone-k2e-netcp.dtsi | 10 +++++-
>> drivers/net/ethernet/ti/cpts.c | 52 ++++++++++++++++++++++++++++++-
>> 2 files changed, 60 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
>> index 919e655..b27aa22 100644
>> --- a/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
>> +++ b/arch/arm/boot/dts/keystone-k2e-netcp.dtsi
>> @@ -138,7 +138,7 @@ netcp: netcp@24000000 {
>> /* NetCP address range */
>> ranges = <0 0x24000000 0x1000000>;
>>
>> - clocks = <&clkpa>, <&clkcpgmac>, <&chipclk12>;
>> + clocks = <&clkpa>, <&clkcpgmac>, <&cpts_mux>;
^^ mux clock used here
>> clock-names = "pa_clk", "ethss_clk", "cpts";
>> dma-coherent;
>>
>> @@ -162,6 +162,14 @@ netcp: netcp@24000000 {
>> cpts-ext-ts-inputs = <6>;
>> cpts-ts-comp-length;
>>
>> + cpts_mux: cpts_refclk_mux {
>> + #clock-cells = <0>;
>> + clocks = <&chipclk12>, <&chipclk13>;
>> + cpts-mux-tbl = <0>, <1>;
>> + assigned-clocks = <&cpts_mux>;
>> + assigned-clock-parents = <&chipclk12>;
>
> Is there a binding update?
this was pure RFC-DEV patch just to check the possibility of modeling
CPTS_RFTCLK_SEL register as mux clock.
Original patch:
https://lkml.org/lkml/2016/11/28/780
I've plan to resend it using clk framework.
Why the subnode?
Sry, I did not get this question - is there another way to pas phandle on clock
in clocks list property? Am I missing smth.?
Sry, this is my first clock :)
> Why not have it as part of the netcp node?
cpts is part of gbe ethss, which is part of netcp.
Only netcp is modeled as DD - cpts and gbe ethss implemented without using DD model,
so generic resources acquired by netcp and then passed to cpts and gbe ethss.
CPTS has register to control an external multiplexer that selects
one of up to 32 clocks for time sync reference (RFTCLK)
> Does the cpts-mux-tbl property change?
On Keystone 2 66AK2e (as example) the following list of clocks can be selected
as ref clocks (list is different for other SoCs):
0000 = SYSCLK2
0001 = SYSCLK3
0010 = TIMI0
0011 = TIMI1
0100 = TSIPCLKA
1000 = TSREFCLK
1100 = TSIPCLKB
Others = Reserved
and only 0 and 1 are internal, other external and board specific
(parameters unknown and corresponding inputs can be used for other purposes),
so I can't define all parent clocks, only internal:
clocks = <&chipclk12>, <&chipclk13>;
cpts-mux-tbl = <0>, <1>;
to use another, external, clock - it should be explicitly defined in board file the board file
timi1clk: timi1clk {
#clock-cells = <0>;
compatible = "fixed-clock";
...
&cpts_mux {
clocks = <&chipclk12>, <&chipclk13>, <timi1clk>;
^^^ i can't predict value here
cpts-mux-tbl = <0>, <1>, <3>;
^^i can't predict value here
assigned-clocks = <&cpts_mux>;
assigned-clock-parents = <&timi1clk>;
};
or I understood your question wrongly?
>
>> + };
>> +
>> interfaces {
>> gbe0: interface-0 {
>> slave-port = <0>;
>> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
>> index 938de22..ef94316 100644
>> --- a/drivers/net/ethernet/ti/cpts.c
>> +++ b/drivers/net/ethernet/ti/cpts.c
>> @@ -17,6 +17,7 @@
>> * along with this program; if not, write to the Free Software
>> * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
>> */
>> +#include <linux/clk-provider.h>
>> #include <linux/err.h>
>> #include <linux/if.h>
>> #include <linux/hrtimer.h>
>> @@ -672,6 +673,7 @@ int cpts_register(struct cpts *cpts)
>> cpts->phc_index = ptp_clock_index(cpts->clock);
>>
>> schedule_delayed_work(&cpts->overflow_work, cpts->ov_check_period);
>> +
>
> Maybe in another patch.
>
sure
>> return 0;
>>
>> err_ptp:
>> @@ -741,6 +743,54 @@ static void cpts_calc_mult_shift(struct cpts *cpts)
>> freq, cpts->cc_mult, cpts->cc.shift, (ns - NSEC_PER_SEC));
>> }
>>
...
>> +
>> + reg = &cpts->reg->rftclk_sel;
>> +
>> + clk = clk_register_mux_table(cpts->dev, refclk_np->name,
>> + parent_names, num_parents,
>> + 0, reg, 0, 0x1F, 0, mux_table, NULL);
>> + if (IS_ERR(clk))
>> + return PTR_ERR(clk);
>> +
>> + return of_clk_add_provider(refclk_np, of_clk_src_simple_get, clk);
>
> Can you please use the clk_hw APIs instead?
>
ok
--
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RESEND PATCH v2 5/7] drm/vc4: Document VEC DT binding
From: Eric Anholt @ 2016-12-09 23:37 UTC (permalink / raw)
To: Rob Herring, Boris Brezillon
Cc: Mark Rutland, devicetree, Ian Campbell, Florian Fainelli,
Pawel Moll, Scott Branden, Stephen Warren, Ray Jui, Lee Jones,
dri-devel, bcm-kernel-feedback-list, linux-rpi-kernel, Kumar Gala,
linux-arm-kernel
In-Reply-To: <20161209210028.ehylahb6o4mv5eil@rob-hp-laptop>
[-- Attachment #1.1: Type: text/plain, Size: 458 bytes --]
Rob Herring <robh@kernel.org> writes:
> On Fri, Dec 02, 2016 at 02:48:11PM +0100, Boris Brezillon wrote:
>> Document the DT binding for the VEC (Video EnCoder) IP.
>>
>> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
>> ---
>> Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>
> Acked-by: Rob Herring <robh@kernel.org>
Thanks. Pulling the series now.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 1/3] dt: pwm: lpc32xx: add description of clocks and #pwm-cells properties
From: Vladimir Zapolskiy @ 2016-12-09 23:51 UTC (permalink / raw)
To: Rob Herring
Cc: Thierry Reding, Sylvain Lemieux, linux-pwm, devicetree,
linux-arm-kernel
In-Reply-To: <20161209214137.2hdugl5rmnwjrus4@rob-hp-laptop>
Hi Rob,
On 12/09/2016 11:41 PM, Rob Herring wrote:
> On Mon, Dec 05, 2016 at 03:42:37AM +0200, Vladimir Zapolskiy wrote:
>> NXP LPC32xx SoCs have two simple independent PWM controllers with a single
>> output each, in this case there is no need to specify PWM channel argument
>> on client side, one cell for setting PWM output frequency is sufficient.
>>
>> Another added to the description property 'clocks' has a standard meaning
>> of a controller supply clock, in the LPC32xx User's Manual the clock is
>> denoted as PWM1_CLK or PWM2_CLK clock.
>>
>> Signed-off-by: Vladimir Zapolskiy <vz@mleia.com>
>> ---
>> Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
>> index 74b5bc5..523d796 100644
>> --- a/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
>> +++ b/Documentation/devicetree/bindings/pwm/lpc32xx-pwm.txt
>> @@ -3,15 +3,22 @@ LPC32XX PWM controller
>> Required properties:
>> - compatible: should be "nxp,lpc3220-pwm"
>> - reg: physical base address and length of the controller's registers
>> +- clocks: clock phandle and clock specifier pair
>> +- #pwm-cells: should be 1, the cell is used to specify the period in
>> + nanoseconds.
>
> This use of the cell is a bit odd as the period is s/w config and this
> would typically be a channel selection or such.
this is a classic PWM channel configuration property for PWM consumers
described in DT, for instance PWM frequency for display panel backlight
on boot.
I think >90% of PWM controllers with device tree bindings have this
argument in #pwm-cells, from bindings/pwm/pwm.txt :
pwm-specifier typically encodes the chip-relative PWM number and
the PWM period in nanoseconds.
You also may skim through phandle arguments of 'pwms' property,
commonly the second argument is the requested frequency.
In this particular case I just drop PWM channel number, because
the LPC32xx PWM controller has a single output channel.
> What if I want user specified/changed periods?
>
The preset period still can be changed over sysfs in runtime.
--
With best wishes,
Vladimir
^ permalink raw reply
* Re: [PATCH 1/2] of: base: add support to get machine model name
From: Frank Rowand @ 2016-12-09 23:54 UTC (permalink / raw)
To: Rob Herring, Sudeep Holla
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Arnd Bergmann, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqKioaP6JvihmJ3HK6cEfCgFRrtYr+EbJ2KvptxcrWCLnA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 12/09/16 08:03, Rob Herring wrote:
> On Wed, Nov 23, 2016 at 4:25 AM, Sudeep Holla <sudeep.holla-5wv7dgnIgG8@public.gmane.org> wrote:
>>
>>
>> On 22/11/16 21:35, Rob Herring wrote:
>>>
>>> On Tue, Nov 22, 2016 at 12:44 PM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> wrote:
>>
>>
>> [...]
>>
>>>>
>>>> This patch adds a function that leads to conflating the "model" property
>>>> and the "compatible" property. This leads to opaque, confusing and
>>>> unclear
>>>> code where ever it is used. I think it is not good for the device tree
>>>> framework to contribute to writing unclear code.
>>>>
>>>> Further, only two of the proposed users of this new function appear to
>>>> be proper usage. I do not think that the small amount of reduced lines
>>>> of code is a good trade off for the reduced code clarity and for the
>>>> potential for future mis-use of this function.
>>>>
>>>> Can I convince you to revert this patch?
>>>
>>>
>>> Yes, I will revert.
>
> I looked at this again and the users. They are all informational, so
A comment in the function docbook header stating that the intent of the
returned value is for informational use only would make me happy.
There is at least on proposed use in patch 2/2 that is not just
informational. init_octeon_system_type() sometimes uses the value of
the model property to create the value of variable octeon_system_type.
octeon_pcie_pcibios_map_irq() checks the value of octeon_system_type
(via the function octeon_board_type_string()) to determine whether
to apply a fixup:
int __init octeon_pcie_pcibios_map_irq(const struct pci_dev *dev,
u8 slot, u8 pin)
{
/*
* The EBH5600 board with the PCI to PCIe bridge mistakenly
* wires the first slot for both device id 2 and interrupt
* A. According to the PCI spec, device id 2 should be C. The
* following kludge attempts to fix this.
*/
if (strstr(octeon_board_type_string(), "EBH5600") &&
dev->bus && dev->bus->parent) {
> I'm not worried if a compatible string could be returned with this
> change. The function returns the best name for the machine and having
> consistency is a good thing.
>
> I was considering not reverting (as I'd not yet gotten around to it),
> but I'm still going to revert for the naming.
>
>>>
>>>> If not, will you accept a patch to change the function name to more
>>>> clearly indicate what it does? (One possible name would be
>>>> of_model_or_1st_compatible().)
>>>
>>>
>>> I took it as there's already the FDT equivalent function.
>>
>>
>> Yes it was mainly for non of_flat_* replacement for
>> of_flat_dt_get_machine_name
>
> I would suggest just of_get_machine_name().
>
> You might also add a fallback to return "unknown", and drop some of
> the custom strings. I don't think anyone should care about the actual
> string. However, it's an error to have a DT with no model or top level
> compatible, so maybe a WARN.
The name and other suggestions sound fine to me.
-Frank
>
> Rob
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 1/5] mtd: spi-nor: add memory controllers for the Aspeed AST2500 SoC
From: Marek Vasut @ 2016-12-10 4:01 UTC (permalink / raw)
To: Cédric Le Goater, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: David Woodhouse, Brian Norris, Boris Brezillon,
Richard Weinberger, Cyrille Pitchen,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
Joel Stanley
In-Reply-To: <1481302167-28044-2-git-send-email-clg-Bxea+6Xhats@public.gmane.org>
On 12/09/2016 05:49 PM, Cédric Le Goater wrote:
[...]
> +static int aspeed_smc_read_from_ahb(void *buf, const void __iomem *src,
> + size_t len)
> +{
> + if (IS_ALIGNED((u32)src, sizeof(u32)) &&
> + IS_ALIGNED((u32)buf, sizeof(u32)) &&
> + IS_ALIGNED(len, sizeof(u32))) {
Did you try compiling this on any 64bit system ?
> + while (len > 3) {
> + *(u32 *)buf = readl(src);
ioread32_rep() to avoid open-coding the loop .
> + buf += 4;
> + src += 4;
> + len -= 4;
> + }
> + }
> +
> + while (len--) {
> + *(u8 *)buf = readb(src);
> + buf += 1;
> + src += 1;
> + }
> + return 0;
> +}
> +
> +static int aspeed_smc_write_to_ahb(void __iomem *dst, const void *buf,
> + size_t len)
> +{
> + if (IS_ALIGNED((u32)dst, sizeof(u32)) &&
> + IS_ALIGNED((u32)buf, sizeof(u32)) &&
> + IS_ALIGNED(len, sizeof(u32))) {
> + while (len > 3) {
> + u32 val = *(u32 *)buf;
> +
> + writel(val, dst);
iowrite32_rep()
> + buf += 4;
> + dst += 4;
> + len -= 4;
> + }
> + }
> +
> + while (len--) {
> + u8 val = *(u8 *)buf;
> +
> + writeb(val, dst);
> + buf += 1;
> + dst += 1;
> + }
> + return 0;
> +}
> +
> +/*
> + * The driver only support SPI flash
> + */
> +enum aspeed_smc_flash_type {
> + smc_type_nor = 0,
> + smc_type_nand = 1,
> + smc_type_spi = 2,
> +};
> +
> +struct aspeed_smc_chip;
> +
> +struct aspeed_smc_info {
> + u32 maxsize; /* maximum size of chip window */
> + u8 nce; /* number of chip enables */
> + bool hastype; /* flash type field exists in config reg */
> + u8 we0; /* shift for write enable bit for CE0 */
> + u8 ctl0; /* offset in regs of ctl for CE0 */
> +
> + void (*set_4b)(struct aspeed_smc_chip *chip);
> +};
Move all the structs to the beginning of the driver please.
> +static void aspeed_smc_chip_set_4b(struct aspeed_smc_chip *chip);
> +
> +static const struct aspeed_smc_info fmc_2500_info = {
> + .maxsize = 256 * 1024 * 1024,
> + .nce = 3,
> + .hastype = true,
> + .we0 = 16,
> + .ctl0 = 0x10,
> + .set_4b = aspeed_smc_chip_set_4b,
> +};
> +
> +static const struct aspeed_smc_info spi_2500_info = {
> + .maxsize = 128 * 1024 * 1024,
> + .nce = 2,
> + .hastype = false,
> + .we0 = 16,
> + .ctl0 = 0x10,
> + .set_4b = aspeed_smc_chip_set_4b,
> +};
> +
> +enum aspeed_smc_ctl_reg_value {
> + smc_base, /* base value without mode for other commands */
> + smc_read, /* command reg for (maybe fast) reads */
> + smc_write, /* command reg for writes */
> + smc_max,
> +};
[...]
> +#define CONTROL_CE_STOP_ACTIVE_CONTROL BIT(2)
> +#define CONTROL_COMMAND_MODE_MASK GENMASK(1, 0)
> +#define CONTROL_COMMAND_MODE_NORMAL (0)
> +#define CONTROL_COMMAND_MODE_FREAD (1)
> +#define CONTROL_COMMAND_MODE_WRITE (2)
> +#define CONTROL_COMMAND_MODE_USER (3)
Drop the parenthesis around constants :)
> +#define CONTROL_KEEP_MASK \
> + (CONTROL_AAF_MODE | CONTROL_CE_INACTIVE_MASK | CONTROL_CLK_DIV4 | \
> + CONTROL_IO_DUMMY_MASK | CONTROL_CLOCK_FREQ_SEL_MASK | \
> + CONTROL_LSB_FIRST | CONTROL_CLOCK_MODE_3)
> +
> +/*
> + * Segment Address Registers. Start and end addresses are encoded
> + * using 8MB units
> + */
> +#define SEGMENT_ADDR_REG0 0x30
> +#define SEGMENT_ADDR_START(_r) ((((_r) >> 16) & 0xFF) << 23)
is that ((r) & 0xff0000) << 7 ?
> +#define SEGMENT_ADDR_END(_r) ((((_r) >> 24) & 0xFF) << 23)
((r) & 0xff000000) >> 1 ?
> +static inline u32 aspeed_smc_chip_write_bit(struct aspeed_smc_chip *chip)
> +{
> + return BIT(chip->controller->info->we0 + chip->cs);
> +}
[...]
> +static int aspeed_smc_chip_setup_init(struct aspeed_smc_chip *chip,
> + struct resource *res)
> +{
> + struct aspeed_smc_controller *controller = chip->controller;
> + const struct aspeed_smc_info *info = controller->info;
> + u32 reg, base_reg;
> +
> + /*
> + * Always turn on the write enable bit to allow opcodes to be
> + * sent in user mode.
> + */
> + aspeed_smc_chip_enable_write(chip);
> +
> + /* The driver only supports SPI type flash */
> + if (info->hastype)
> + aspeed_smc_chip_set_type(chip, smc_type_spi);
> +
> + /*
> + * Configure chip base address in memory
> + */
> + chip->ahb_base = aspeed_smc_chip_base(chip, res);
> + if (!chip->ahb_base) {
> + dev_warn(chip->nor.dev, "CE segment window closed.\n");
> + return -1;
return -EINVAL ? Check whether you always use errno.h macros in returns.
> + }
> +
> + /*
> + * Get value of the inherited control register. U-Boot usually
> + * does some timing calibration on the FMC chip, so it's good
> + * to keep them. In the future, we should handle calibration
> + * from Linux.
> + */
> + reg = readl(chip->ctl);
> + dev_dbg(controller->dev, "control register: %08x\n", reg);
> +
> + base_reg = reg & CONTROL_KEEP_MASK;
> + if (base_reg != reg) {
> + dev_info(controller->dev,
> + "control register changed to: %08x\n",
> + base_reg);
> + }
> + chip->ctl_val[smc_base] = base_reg;
[...]
Mostly minor nits, looks nice otherwise, thanks :)
--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 2/5] mtd: aspeed: add memory controllers for the Aspeed AST2400 SoC
From: Marek Vasut @ 2016-12-10 4:03 UTC (permalink / raw)
To: Cédric Le Goater, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: David Woodhouse, Brian Norris, Boris Brezillon,
Richard Weinberger, Cyrille Pitchen,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
Joel Stanley
In-Reply-To: <1481302167-28044-3-git-send-email-clg-Bxea+6Xhats@public.gmane.org>
On 12/09/2016 05:49 PM, Cédric Le Goater wrote:
> This driver adds mtd support for the Aspeed AST2400 SoC static memory
> controllers:
>
> * New Static Memory Controller (referred as FMC)
> . BMC firmware
> . AST2500 compatible register set
> . 5 chip select pins (CE0 ∼ CE4)
> . supports NOR flash, NAND flash and SPI flash memory.
>
> * SPI Flash Controller (SPI)
> . host Firmware
> . slightly different register set, between AST2500 and the legacy
> controller
> . supports SPI flash memory
> . 1 chip select pin (CE0)
>
> The legacy static memory controller (referred as SMC) is not
> supported, as well as types other than SPI.
>
> Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>
Well this is a nice split :-)
> ---
> drivers/mtd/spi-nor/Kconfig | 2 +-
> drivers/mtd/spi-nor/aspeed-smc.c | 33 +++++++++++++++++++++++++++++++++
> 2 files changed, 34 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
> index 5c0efbd9dd89..22bea563f9bc 100644
> --- a/drivers/mtd/spi-nor/Kconfig
> +++ b/drivers/mtd/spi-nor/Kconfig
> @@ -35,7 +35,7 @@ config SPI_ASPEED
> depends on HAS_IOMEM && OF
> help
> This enables support for the Firmware Memory controller (FMC)
> - in the Aspeed AST2500 SoC when attached to SPI NOR chips,
> + in the Aspeed AST2500/AST2400 SoCs when attached to SPI NOR chips,
> and support for the SPI flash memory controller (SPI) for
> the host firmware. The implementation only supports SPI NOR.
>
> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
> index 6f9244f07aef..99302b0d7786 100644
> --- a/drivers/mtd/spi-nor/aspeed-smc.c
> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
> @@ -119,8 +119,27 @@ struct aspeed_smc_info {
> void (*set_4b)(struct aspeed_smc_chip *chip);
> };
>
> +static void aspeed_smc_chip_set_4b_spi_2400(struct aspeed_smc_chip *chip);
> static void aspeed_smc_chip_set_4b(struct aspeed_smc_chip *chip);
>
> +static const struct aspeed_smc_info fmc_2400_info = {
> + .maxsize = 64 * 1024 * 1024,
> + .nce = 5,
> + .hastype = true,
> + .we0 = 16,
> + .ctl0 = 0x10,
> + .set_4b = aspeed_smc_chip_set_4b,
> +};
> +
> +static const struct aspeed_smc_info spi_2400_info = {
> + .maxsize = 64 * 1024 * 1024,
> + .nce = 1,
> + .hastype = false,
> + .we0 = 0,
> + .ctl0 = 0x04,
> + .set_4b = aspeed_smc_chip_set_4b_spi_2400,
> +};
> +
> static const struct aspeed_smc_info fmc_2500_info = {
> .maxsize = 256 * 1024 * 1024,
> .nce = 3,
> @@ -210,6 +229,7 @@ struct aspeed_smc_controller {
> #define CONTROL_IO_DUMMY_HI BIT(14)
> #define CONTROL_IO_DUMMY_HI_SHIFT 14
> #define CONTROL_CLK_DIV4 BIT(13) /* others */
> +#define CONTROL_IO_ADDRESS_4B BIT(13) /* AST2400 SPI */
> #define CONTROL_RW_MERGE BIT(12)
> #define CONTROL_IO_DUMMY_LO_SHIFT 6
> #define CONTROL_IO_DUMMY_LO GENMASK(7, \
> @@ -406,6 +426,8 @@ static int aspeed_smc_remove(struct platform_device *dev)
> }
>
> static const struct of_device_id aspeed_smc_matches[] = {
> + { .compatible = "aspeed,ast2400-fmc", .data = &fmc_2400_info },
> + { .compatible = "aspeed,ast2400-spi", .data = &spi_2400_info },
> { .compatible = "aspeed,ast2500-fmc", .data = &fmc_2500_info },
> { .compatible = "aspeed,ast2500-spi", .data = &spi_2500_info },
> { }
> @@ -479,6 +501,17 @@ static void aspeed_smc_chip_set_4b(struct aspeed_smc_chip *chip)
> }
> }
>
> +/*
> + * The AST2400 SPI flash controller does not have a CE Control
> + * register. It uses the CE0 control register to set 4Byte mode at the
> + * controller level.
> + */
> +static void aspeed_smc_chip_set_4b_spi_2400(struct aspeed_smc_chip *chip)
> +{
> + chip->ctl_val[smc_base] |= CONTROL_IO_ADDRESS_4B;
> + chip->ctl_val[smc_read] |= CONTROL_IO_ADDRESS_4B;
How do you know the SPI NOR is in 4B mode ?
> +}
> +
> static int aspeed_smc_chip_setup_init(struct aspeed_smc_chip *chip,
> struct resource *res)
> {
>
--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ 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