Devicetree
 help / color / mirror / Atom feed
* [PATCH v14 02/11] dt-bindings: document devicetree bindings for mux-controllers and gpio-mux
From: Peter Rosin @ 2017-04-24  8:36 UTC (permalink / raw)
  To: linux-kernel, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet, linux-i2c, devicetree,
	linux-iio, linux-doc, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel
In-Reply-To: <1493022995-16917-1-git-send-email-peda@axentia.se>

Allow specifying that a single multiplexer controller can be used to
control several parallel multiplexers, thus enabling sharing of the
multiplexer controller by different consumers.

Add a binding for a first mux controller in the form of a GPIO based mux
controller.

Acked-by: Jonathan Cameron <jic23@kernel.org>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 Documentation/devicetree/bindings/mux/gpio-mux.txt |  69 +++++++++
 .../devicetree/bindings/mux/mux-controller.txt     | 157 +++++++++++++++++++++
 MAINTAINERS                                        |   6 +
 include/dt-bindings/mux/mux.h                      |  16 +++
 4 files changed, 248 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mux/gpio-mux.txt
 create mode 100644 Documentation/devicetree/bindings/mux/mux-controller.txt
 create mode 100644 include/dt-bindings/mux/mux.h

diff --git a/Documentation/devicetree/bindings/mux/gpio-mux.txt b/Documentation/devicetree/bindings/mux/gpio-mux.txt
new file mode 100644
index 000000000000..b8f746344d80
--- /dev/null
+++ b/Documentation/devicetree/bindings/mux/gpio-mux.txt
@@ -0,0 +1,69 @@
+GPIO-based multiplexer controller bindings
+
+Define what GPIO pins are used to control a multiplexer. Or several
+multiplexers, if the same pins control more than one multiplexer.
+
+Required properties:
+- compatible : "gpio-mux"
+- mux-gpios : list of gpios used to control the multiplexer, least
+	      significant bit first.
+- #mux-control-cells : <0>
+* Standard mux-controller bindings as decribed in mux-controller.txt
+
+Optional properties:
+- idle-state : if present, the state the mux will have when idle. The
+	       special state MUX_IDLE_AS_IS is the default.
+
+The multiplexer state is defined as the number represented by the
+multiplexer GPIO pins, where the first pin is the least significant
+bit. An active pin is a binary 1, an inactive pin is a binary 0.
+
+Example:
+
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+	};
+
+	adc-mux {
+		compatible = "io-channel-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		mux-controls = <&mux>;
+
+		channels = "sync-1", "in", "out", "sync-2";
+	};
+
+	i2c-mux {
+		compatible = "i2c-mux";
+		i2c-parent = <&i2c1>;
+
+		mux-controls = <&mux>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			ssd1307: oled@3c {
+				/* ... */
+			};
+		};
+
+		i2c@3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			pca9555: pca9555@20 {
+				/* ... */
+			};
+		};
+	};
diff --git a/Documentation/devicetree/bindings/mux/mux-controller.txt b/Documentation/devicetree/bindings/mux/mux-controller.txt
new file mode 100644
index 000000000000..4f47e4bd2fa0
--- /dev/null
+++ b/Documentation/devicetree/bindings/mux/mux-controller.txt
@@ -0,0 +1,157 @@
+Common multiplexer controller bindings
+======================================
+
+A multiplexer (or mux) controller will have one, or several, consumer devices
+that uses the mux controller. Thus, a mux controller can possibly control
+several parallel multiplexers. Presumably there will be at least one
+multiplexer needed by each consumer, but a single mux controller can of course
+control several multiplexers for a single consumer.
+
+A mux controller provides a number of states to its consumers, and the state
+space is a simple zero-based enumeration. I.e. 0-1 for a 2-way multiplexer,
+0-7 for an 8-way multiplexer, etc.
+
+
+Consumers
+---------
+
+Mux controller consumers should specify a list of mux controllers that they
+want to use with a property containing a 'mux-ctrl-list':
+
+	mux-ctrl-list ::= <single-mux-ctrl> [mux-ctrl-list]
+	single-mux-ctrl ::= <mux-ctrl-phandle> [mux-ctrl-specifier]
+	mux-ctrl-phandle : phandle to mux controller node
+	mux-ctrl-specifier : array of #mux-control-cells specifying the
+			     given mux controller (controller specific)
+
+Mux controller properties should be named "mux-controls". The exact meaning of
+each mux controller property must be documented in the device tree binding for
+each consumer. An optional property "mux-control-names" may contain a list of
+strings to label each of the mux controllers listed in the "mux-controls"
+property.
+
+Drivers for devices that use more than a single mux controller can use the
+"mux-control-names" property to map the name of the requested mux controller
+to an index into the list given by the "mux-controls" property.
+
+mux-ctrl-specifier typically encodes the chip-relative mux controller number.
+If the mux controller chip only provides a single mux controller, the
+mux-ctrl-specifier can typically be left out.
+
+Example:
+
+	/* One consumer of a 2-way mux controller (one GPIO-line) */
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>;
+	};
+
+	adc-mux {
+		compatible = "io-channel-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		mux-controls = <&mux>;
+		mux-control-names = "adc";
+
+		channels = "sync", "in";
+	};
+
+Note that in the example above, specifying the "mux-control-names" is redundant
+because there is only one mux controller in the list. However, if the driver
+for the consumer node in fact asks for a named mux controller, that name is of
+course still required.
+
+	/*
+	 * Two consumers (one for an ADC line and one for an i2c bus) of
+	 * parallel 4-way multiplexers controlled by the same two GPIO-lines.
+	 */
+	mux: mux-controller {
+		compatible = "gpio-mux";
+		#mux-control-cells = <0>;
+
+		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
+			    <&pioA 1 GPIO_ACTIVE_HIGH>;
+	};
+
+	adc-mux {
+		compatible = "io-channel-mux";
+		io-channels = <&adc 0>;
+		io-channel-names = "parent";
+
+		mux-controls = <&mux>;
+
+		channels = "sync-1", "in", "out", "sync-2";
+	};
+
+	i2c-mux {
+		compatible = "i2c-mux";
+		i2c-parent = <&i2c1>;
+
+		mux-controls = <&mux>;
+
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			ssd1307: oled@3c {
+				/* ... */
+			};
+		};
+
+		i2c@3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			pca9555: pca9555@20 {
+				/* ... */
+			};
+		};
+	};
+
+
+Mux controller nodes
+--------------------
+
+Mux controller nodes must specify the number of cells used for the
+specifier using the '#mux-control-cells' property.
+
+Optionally, mux controller nodes can also specify the state the mux should
+have when it is idle. The idle-state property is used for this. If the
+idle-state is not present, the mux controller is typically left as is when
+it is idle. For multiplexer chips that expose several mux controllers, the
+idle-state property is an array with one idle state for each mux controller.
+
+The special value (-1) may be used to indicate that the mux should be left
+as is when it is idle. This is the default, but can still be useful for
+mux controller chips with more than one mux controller, particularly when
+there is a need to "step past" a mux controller and set some other idle
+state for a mux controller with a higher index.
+
+Some mux controllers have the ability to disconnect the input/output of the
+multiplexer. Using this disconnected high-impedance state as the idle state
+is indicated with idle state (-2).
+
+These constants are available in
+
+      #include <dt-bindings/mux/mux.h>
+
+as MUX_IDLE_AS_IS (-1) and MUX_IDLE_DISCONNECT (-2).
+
+An example mux controller node look like this (the adg972a chip is a triple
+4-way multiplexer):
+
+	mux: mux-controller@50 {
+		compatible = "adi,adg792a";
+		reg = <0x50>;
+		#mux-control-cells = <1>;
+
+		idle-state = <MUX_IDLE_DISCONNECT MUX_IDLE_AS_IS 2>;
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index c265a5fe4848..7fc06739c8ad 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8558,6 +8558,12 @@ S:	Orphan
 F:	drivers/mmc/host/mmc_spi.c
 F:	include/linux/spi/mmc_spi.h
 
+MULTIPLEXER SUBSYSTEM
+M:	Peter Rosin <peda@axentia.se>
+S:	Maintained
+F:	Documentation/devicetree/bindings/mux/
+F:	include/linux/dt-bindings/mux/
+
 MULTISOUND SOUND DRIVER
 M:	Andrew Veliath <andrewtv@usa.net>
 S:	Maintained
diff --git a/include/dt-bindings/mux/mux.h b/include/dt-bindings/mux/mux.h
new file mode 100644
index 000000000000..c8e855c4a609
--- /dev/null
+++ b/include/dt-bindings/mux/mux.h
@@ -0,0 +1,16 @@
+/*
+ * This header provides constants for most Multiplexer bindings.
+ *
+ * Most Multiplexer bindings specify an idle state. In most cases, the
+ * the multiplexer can be left as is when idle, and in some cases it can
+ * disconnect the input/output and leave the multiplexer in a high
+ * impedance state.
+ */
+
+#ifndef _DT_BINDINGS_MUX_MUX_H
+#define _DT_BINDINGS_MUX_MUX_H
+
+#define MUX_IDLE_AS_IS      (-1)
+#define MUX_IDLE_DISCONNECT (-2)
+
+#endif
-- 
2.1.4


^ permalink raw reply related

* [PATCH v14 01/11] devres: trivial whitespace fix
From: Peter Rosin @ 2017-04-24  8:36 UTC (permalink / raw)
  To: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Jonathan Corbet,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <1493022995-16917-1-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

Everything else is indented with two spaces, so fix the odd one out.

Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
---
 Documentation/driver-model/devres.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
index bf34d5b3a733..efb8200819d6 100644
--- a/Documentation/driver-model/devres.txt
+++ b/Documentation/driver-model/devres.txt
@@ -335,7 +335,7 @@ MEM
   devm_kzalloc()
 
 MFD
- devm_mfd_add_devices()
+  devm_mfd_add_devices()
 
 PER-CPU MEM
   devm_alloc_percpu()
-- 
2.1.4

^ permalink raw reply related

* [PATCH v14 00/11] mux controller abstraction and iio/i2c muxes
From: Peter Rosin @ 2017-04-24  8:36 UTC (permalink / raw)
  To: Jonathan Cameron, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	Greg Kroah-Hartman
  Cc: Peter Rosin, Wolfram Sang, Rob Herring, Mark Rutland,
	Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
	Jonathan Corbet, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, Colin Ian King,
	Paul Gortmaker, Philipp Zabel, kernel-bIcnvbaLZ9MEGnE8C9+IrQ

Hi!

The big change since v13 is that the mux state is now locked with a mutex
instead of an rwsem. Other that that, it is mostly restructuring and doc
changes. There are a few other "real" changes as well, but those changes
feel kind of minor. I guess what I'm trying to say is that although the
list of changes for v14 is longish, it's still basically the same as last
time.

Jonathan, I hope it was OK to keep your Reviewed-by tag on patch 03/11
(mux-core.c plus support) and to also add it to 04/11 (mux-gpio.c) even
if the previously reviewed patch (mux-core.c + mux-gpio.c) was split in
two?

v13 -> v14 changes
- Go back to a mutex instead of a rwsem to protect the mux state. [Greg K-H]
- Add __must_check to the mux_control_select function. [Greg, was this what
  you meant with "sparse markings"? I assume not, but I don't know...]
- Don't return hint on mux state changes when calling mux_control_select. [Greg]
- Add mux_control_try_select function. [Philipp Zabel]
- Split the mux.h #include into mux/driver.h and mux/consumer.h [Philipp]
- Add mux_control_states function to be able to completely hide struct
  mux_control details from mux consumers.
- Include the constants from the DT bindings include, instead of
  redefining them. [Greg]
- Make mux_chip_alloc (and devm_mux_chip_alloc) return ERR_PTR instead
  of NULL on error. [Greg]
- Document why mux_chip_alloc and mux_chip_register are split up, and
  what is supposed to happen between the two calls, [Greg] and...
- ...specifically mention that mux drivers should never clobber
  cached_state [Philipp].
- Support module unloading of the core (and initialize mux_ida on init). [Greg]
- Do not mention link order when commenting on why there is a subsys_initcall
  instead of an ordinare module_init in the mux-core. [Greg]
- Use __func__ instead of spelling out the function name. [Greg]
- Drop cargo cult usage of WARN_ON. [Greg]
- Drop unused forward declaration of struct platform_device.
- Move kernel doc from the function declarations to the definitions. [Greg]
- Use unsigned values for the state in the consumer interface. [Philipp]
- WARN if consumers request an invalid state. [Philipp]
- Split out mux-gpio from the core patch (03/10 -> 03/11 and 04/11). [Greg]
- Drop OF Kconfig dep from mux-gpio. [Greg]
- Fail the adg792a probe if the I2C adapter does not support
  I2C_FUNC_SMBUS_BYTE_DATA.
- For clarity, handle the adg792a active low reset bit in a helper function.
- Make use of the preexisting mux variable in the adg792a driver instead of
  chasing the same pointers a second time.
- Use device_property_read_* instead of of_property_read_* in adg792a.
- Added a "thank you" section to the patch adding the mux core (patch 03/11).


This adds a new mux controller subsystem with an interface for accessing
mux controllers, along with two drivers providing the interface (gpio
and adg792) and two consumers (iio and i2c). This is done in such a way
that several consumers can independently access the same mux controller
if one controller controls several multiplexers, thus allowing sharing.
But sharing is by no means required, of course. It is perfectly fine to
have a single consumer of a dedicated mux controller controlling only
one mux for said consumer.

The prediction is that the typical use case will be for gpio-based muxing
(which is also what drove the development), where the below schematics
show the flexibility with one gpio-based mux controller being shared by
the iio-mux and i2c-mux-gpmux drivers.

    .----.
    |GPO0|----------.
    |GPO1|--------. |
    |    |        | |
    |    |     .-------.
    |    |     |dg4052a|
    |    |     |       |
    |ADC0|-----|X    X0|---- signal X0
    |    |     |     X1|---- signal X1
    |    |     |     X2|---- signal X2
    |    |     |     X3|---- signal X3
    |    |     |       |
    |SDA0|-----|Y    Y0|---- i2c segment Y0
    |SCL0|--.  |     Y1|---- i2c segment Y1
    '----'  |  |     Y2|---- i2c segment Y2
            |  |     Y3|---- i2c segment Y3
            |  '-------'
            |                0 1 2 3   (feed SCL0 to each of
            |                | | | |    the 4 muxed segments)
            '----------------+-+-+-'

GPO0 and GPO1 may also be fed to further parallel muxers, which is perhaps
desired in a real application to minimize digital noise from the I2C Y
channel leaking into the analog X channel. I.e. it might be a good idea
to separate the analog and digital signals...

And the below hypothetical schematics indicate something similar but using
the I2C-based adg792a multiplexer instead.

    .----.
    |SDA0|----------.
    |SCL0|--------. |
    |    |        | |
    |    |     .-------.
    |    |     |adg792a|
    |    |     |       |
    |ADC0|-----|D1  S1A|---- signal S1A
    |    |     |    S1B|---- signal S1B
    |    |     |    S1C|---- signal S1C
    |    |     |    S1D|---- signal S1D
    |    |     |       |
    |SDA1|---+-|D2  S2A|---- i2c segment S2A
    |SCL1|-. | |    S2B|---- i2c segment S2B
    '----' | | |    S2C|---- i2c segment S2C
           | | |    S2D|---- i2c segment S2D
           | | |       |
           | '-|D3  S3A|---- i2c segment S3A
           |   |    S3B|---- i2c segment S3B
           |   |    S3C|---- i2c segment S3C
           |   |    S3D|---- i2c segment S3D
           |   '-------'
           |                 A B C D   A B C D  (feed SCL1 to each of
           |                 | | | |   | | | |   the 8 muxed segments)
           '-----------------+-+-+-+---+-+-+-'


Background:

I have a piece of hardware that is using the same 3 GPIO pins
to control four 8-way muxes. Three of them control ADC lines
to an ADS1015 chip with an iio driver, and the last one
controls the SDA line of an i2c bus. We have some deployed
code to handle this, but you do not want to see it or ever
hear about it. I'm not sure why I even mention it. Anyway,
the situation has nagged me to no end for quite some time.

So, after first getting more intimate with the i2c muxing code
and later discovering the drivers/iio/inkern.c file and
writing a couple of drivers making use of it, I came up with
what I think is an acceptable solution; add a generic mux
controller driver (and subsystem) that is shared between all
instances, and combine that with an iio mux driver and a new
general purpose i2c mux driver that is only hooking the i2c
muxing and the new mux controller.

One thing that I would like to do, but don't see a solution
for, is to move the mux control code that is present in
various drivers in drivers/i2c/muxes to this new minimalistic
muxing subsystem, thus converting all present i2c muxes (but
perhaps not gates and arbitrators).

I'm using an ordinary mutex to lock the mux, but that isn't
really a perfect fit since multiple consumers of the same
mux controller need not lock each other out when they want
the same mux state. I had a rwsem for a long while, but that
still degenerated to the above mutex case both when there was
contention and for a race when uncontended.

Also, the "mux" name feels a bit ambitious, there are many muxes
in the world, and this tiny bit of code is probably not good
enough to be a nice fit for all...


Older changes:

v12 -> v13 changes:  https://lkml.org/lkml/2017/4/13/493
- Philipp Zabel noticed a bad compatible string in the gpio muxer.
  I amended patch 3/10 with his patch.
- Fixed reversed sense of the reset bit in the adg792 muxer (patch 10/10).

v11 -> v12 changes:  https://lkml.org/lkml/2017/3/27/464
- patches 11 and 12 folded into patches 6 and 3 respectively.

v10 -> v11 changes:  https://lkml.org/lkml/2017/3/27/336
- added a fixes-tag and an ack from Jonathan on patch 11
- added a new patch (12) with a fix for messed up error path reported
  by Paul Gortmaker.
- fixed some editorial nitpicks in the documentation comments in patch 3.

v9 -> v10 changes:  https://lkml.org/lkml/2017/3/10/411
- rebased onto v4.11-rc1
- added reviewed-by tags from Rob on patches 7 and 9
- added a new patch (11) with a fix for an unsigned compare with less than
  zero detected by CoverityScan and reported by Colin Ian King
- allowed the mux core to be built as a module, after discussion with Paul
  Gortmaker
- added explicit includes of linux/export.h and linux/init.h from the mux
  core, also noted by Paul
- fixed trivial whitespace issue in drivers/mux/Makefile
- added trailing '>' after my mail address in MODULE_AUTHOR, which was missing
  in all new modules in drivers/mux

v8 -> v9 changes:  https://lkml.org/lkml/2017/2/8/394
- dropped the suffix from the compatible string of the i2c-mux-simple
  binding (was ,mux-locked or ,parent-locked) and add an optional
  mux-locked property instead to change the desired locking behavior
  from the default parent-locked
- add description of the difference between mux-locked and parent-locked
- renamed i2c-mux-simple to i2c-mux (bindings for this general purpose
  i2c mux are in i2c-mux-gpmux.txt since i2c-mux.txt is already occupied
  by the common i2c-mux bindings)
- changed compatible from mux-gpio to gpio-mux
- changed bindings for idle-state back to a single array, but add defines
  for as-is and hi-Z thus avoiding magic numbers
- make use of the above defines in the code as well
- make idle-state a common mux property described in mux-controller.txt
  instead of repeating the info in individual mux controller bindings
- drop the adi,parallel property from the adg792 bindings and piggy-back
  on the #mux-control-cells property
- refrain from using the compatible string as node name
- dropped the simplified bindings for single-user gpio mux
- added acks on patches 2/10 and 5/10 from Rob

v7 -> v8 changes:  https://lkml.org/lkml/2017/1/18/745
- cleanup the implementation of the simplified gpio bindings, but still...
- ...reorder patches so that they appear last in the series (patches 11
  and 12 were patches 4 and 5 in v7) since Jonathan convinced me that
  they were perhaps not such a good idea after all. But I still wanted
  to show the last version I had and I'm still a bit undecided...
- added some words to the remaining otherwise empty commit messages
- added various acks/reviews from Jonathan and Wolfram.
- move mux last in drivers/Kconfig and drivers/Makefile
- bump copyright years
- centralize error reporting of common operatinons to the mux core
- add WARN_ON for faulty usage of mux_chip_register
- simplify code for other WARN_ON call sites

v6 -> v7 changes:  https://lkml.org/lkml/2017/1/4/315
- move from drivers/misc/mux-* to drivers/mux/
  [I will remove Cc:s to Arnd and Greg for v8, when/if that happens]
- add managed versions of mux_chip_alloc and mux_chip_register
- use the above in mux-gpio.c and mux-adg792a.c
- also use them to support a simplified binding of a gpio mux for the common
  case where there is a single consumer (and no specific idle requirements)
- new binding for describing idle behaviour of mux-adg792a
- add bindings for the gpo-pins on the mux-adg792g
- use device_property_read_u32 instead of of_property_read_u32 in mux-gpio.c
- rename iio mux compatible to io-channel-mux (was iio-mux)
- remove linuxism in the bindings (it was mentioning a function name)
- add missing quote in the example in the io-channel-mux binding
- factor out preparatory cleanup of devres docs to its own patch
- add blank line in mux_chip_free
- use SIMPLE_{PARENT,MUX}_LOCKED instead of magic numbers {0,1} in
  i2c-mux-simple.c
- add some acks and a reviewed-by from Jonathan

v5 -> v6 changes:  https://lkml.org/lkml/2016/11/30/466
- fix stupidity in mux_chip_priv, mux_gpio_remove and adg792a_remove.
- change the devicetree bindings for the iio-mux to use a list of strings
  (channels property) instead of a list children.

v4 -> v5 changes:  https://lkml.org/lkml/2016/11/29/224
- remove support for fancier dt layouts and go back to the phandle
  approach from v2 and before, killing the horrible non-working
  refcounting crap from v4 and avoiding a bunch of life-time issues
  in v3.
- introduce the concept of a mux-chip, that can hold one or more
  mux-controllers (inspired by the pwm subsystem).
- add dt #mux-control-cells property needed to get to the desired
  mux controller if a mux chip provides more than one.
- take away the option to build the mux-core as a module.
- if the mux controller has an idle state, make sure the mux controller
  is set up in the idle state initially (when it should be idle).
- do not use a variable length array on the stack in mux_gpio_set to
  temporarily store the gpio state, preallocate space instead.
- fix resource leak on one failure path in mux_gpio_probe.
- driver for Analog Devices ADG792A/G, literally the first mux chip
  I found on the Internet with an i2c interface (that was not a
  dedicated i2c multiplexer like PCA9547) which I used to verify
  that the abstractions in the mux core are up to the task. Untested,
  just proof of concept that at least looks pretty and compiles...
- various touch-ups.

v3 -> v4 changes:  https://lkml.org/lkml/2016/11/24/664
- rebased onto next-20161122 (depends on recent _available iio changes).
- added support for having the mux-controller in a child node of a
  mux-consumer if it is a sole consumer, to hopefully even further satisfy
  the complaint from Rob (and later Lars-Peter) about dt complexity.
- the above came at the cost of some rather horrible refcounting code,
  please review and suggest how it should be done...
- changed to register a device class instead of a bus.
- pass in the parent device into mux_control_alloc and require less
  work from mux-control drivers.
- changed device names from mux:control%d to mux%d
- move kernel-doc from mux-core.c to mux.h (and add some bits).
- give the gpio driver a chance to update all mux pins at once.
- factor out iio ext_info lookup into new helper function. /Lars-Peter
- use an unsigned type for the iio ext_info count. /Lars-Peter
- unified "brag strings" in the file headers.

v2 -> v3 changes:  https://lkml.org/lkml/2016/11/21/332
- have the mux-controller in the parent node of any mux-controller consumer,
  to hopefully satisfy complaint from Rob about dt complexity.
- improve commit message of the mux subsystem commit, making it more
  general, as requested by Jonathan.
- remove priv member from struct mux_control and calculate it on the
  fly. /Jonathan
- make the function comments in mux-core.c kernel doc. /Jonathan
- add devm_mux_control_* to Documentation/driver.model/devres.txt. /Jonathan
- add common dt bindings for mux-controllers, refer to them from the
  mux-gpio bindings. /Rob
- clarify how the gpio pins map to the mux state. /Rob
- separate CONFIG_ variables for the mux core and the mux gpio driver.
- improve Kconfig help texts.
- make CONFIG_MUX_GPIO depend on CONFIG_GPIOLIB.
- keep track of the number of mux states in the mux core.
- since the iio channel number is used as mux state, it was possible
  to drop the state member from the mux_child struct.
- cleanup dt bindings for i2c-mux-simple, it had some of copy-paste
  problems from ots origin (i2c-mux-gpio).
- select the mux control subsystem in config for the i2c-mux-simple driver.
- add entries to MAINTAINERS and my sign-off, I'm now satisfied and know
  nothing in this to be ashamed of.

v1 -> v2 changes:  https://lkml.org/lkml/2016/11/17/918
- fixup export of mux_control_put reported by kbuild
- drop devicetree iio-ext-info property as noted by Lars-Peter,
  and replace the functionality by exposing all ext_info
  attributes of the parent channel for each of the muxed
  channels. A cache on top of that and each muxed channel
  gets its own view of the ext_info of the parent channel.
- implement idle-state for muxes
- clear out the cache on failure in order to force a mux
  update on the following use
- cleanup the probe of i2c-mux-simple driver
- fix a bug in the i2c-mux-simple driver, where failure in
  the selection of the mux caused a deadlock when the mux
  was later unconditionally deselected.

v1:  https://lkml.org/lkml/2016/11/16/812

Cheers,
peda

Peter Rosin (11):
  devres: trivial whitespace fix
  dt-bindings: document devicetree bindings for mux-controllers and
    gpio-mux
  mux: minimal mux subsystem
  mux: gpio: add mux controller driver for gpio based multiplexers
  iio: inkern: api for manipulating ext_info of iio channels
  dt-bindings: iio: io-channel-mux: document io-channel-mux bindings
  iio: multiplexer: new iio category and iio-mux driver
  dt-bindings: i2c: i2c-mux: document general purpose i2c-mux bindings
  i2c: i2c-mux-gpmux: new driver
  dt-bindings: mux-adg792a: document devicetree bindings for ADG792A/G
    mux
  mux: adg792a: add mux controller driver for ADG792A/G

 Documentation/ABI/testing/sysfs-class-mux          |  16 +
 .../devicetree/bindings/i2c/i2c-mux-gpmux.txt      |  99 ++++
 .../bindings/iio/multiplexer/io-channel-mux.txt    |  39 ++
 .../devicetree/bindings/mux/adi,adg792a.txt        |  75 +++
 Documentation/devicetree/bindings/mux/gpio-mux.txt |  69 +++
 .../devicetree/bindings/mux/mux-controller.txt     | 157 ++++++
 Documentation/driver-model/devres.txt              |  10 +-
 MAINTAINERS                                        |  16 +
 drivers/Kconfig                                    |   2 +
 drivers/Makefile                                   |   1 +
 drivers/i2c/muxes/Kconfig                          |  13 +
 drivers/i2c/muxes/Makefile                         |   1 +
 drivers/i2c/muxes/i2c-mux-gpmux.c                  | 173 ++++++
 drivers/iio/Kconfig                                |   1 +
 drivers/iio/Makefile                               |   1 +
 drivers/iio/inkern.c                               |  60 +++
 drivers/iio/multiplexer/Kconfig                    |  18 +
 drivers/iio/multiplexer/Makefile                   |   6 +
 drivers/iio/multiplexer/iio-mux.c                  | 459 ++++++++++++++++
 drivers/mux/Kconfig                                |  46 ++
 drivers/mux/Makefile                               |   7 +
 drivers/mux/mux-adg792a.c                          | 157 ++++++
 drivers/mux/mux-core.c                             | 593 +++++++++++++++++++++
 drivers/mux/mux-gpio.c                             | 114 ++++
 include/dt-bindings/mux/mux.h                      |  16 +
 include/linux/iio/consumer.h                       |  37 ++
 include/linux/mux/consumer.h                       |  33 ++
 include/linux/mux/driver.h                         | 111 ++++
 28 files changed, 2329 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-mux
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-gpmux.txt
 create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/io-channel-mux.txt
 create mode 100644 Documentation/devicetree/bindings/mux/adi,adg792a.txt
 create mode 100644 Documentation/devicetree/bindings/mux/gpio-mux.txt
 create mode 100644 Documentation/devicetree/bindings/mux/mux-controller.txt
 create mode 100644 drivers/i2c/muxes/i2c-mux-gpmux.c
 create mode 100644 drivers/iio/multiplexer/Kconfig
 create mode 100644 drivers/iio/multiplexer/Makefile
 create mode 100644 drivers/iio/multiplexer/iio-mux.c
 create mode 100644 drivers/mux/Kconfig
 create mode 100644 drivers/mux/Makefile
 create mode 100644 drivers/mux/mux-adg792a.c
 create mode 100644 drivers/mux/mux-core.c
 create mode 100644 drivers/mux/mux-gpio.c
 create mode 100644 include/dt-bindings/mux/mux.h
 create mode 100644 include/linux/mux/consumer.h
 create mode 100644 include/linux/mux/driver.h

-- 
2.1.4

^ permalink raw reply

* Re: [PATCH V5 3/7] ARM: exynos: Use - instead of @ for DT OPP entries
From: Krzysztof Kozlowski @ 2017-04-24  8:32 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Viresh Kumar, arm, Chanwoo Choi, MyungJoo Ham, Kyungmin Park,
	Kukjin Kim, Javier Martinez Canillas, Rob Herring, Mark Rutland,
	linaro-kernel, linux-arm-kernel, linux-pm, Rafael Wysocki,
	Rob Herring, linux-samsung-soc, devicetree,
	Linux Kernel Mailing List
In-Reply-To: <CAK7LNAQChtyRzKoSWCFakg+3_Qp_NZoXP1pEt6edDQrnL9FQ2w@mail.gmail.com>

On Mon, Apr 24, 2017 at 10:30 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> 2017-04-21 1:09 GMT+09:00 Krzysztof Kozlowski <krzk@kernel.org>:
>> On Thu, Apr 20, 2017 at 04:25:07PM +0530, Viresh Kumar wrote:
>>> Compiling the DT file with W=1, DTC warns like follows:
>>>
>>> Warning (unit_address_vs_reg): Node /opp_table0/opp@1000000000 has a
>>> unit name, but no reg property
>>>
>>> Fix this by replacing '@' with '-' as the OPP nodes will never have a
>>> "reg" property.
>>>
>>> Reported-by: Krzysztof Kozlowski <krzk@kernel.org>
>>> Reported-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
>>> Acked-by: Rob Herring <robh@kernel.org>
>>> ---
>>>  .../devicetree/bindings/devfreq/exynos-bus.txt     | 46 +++++++--------
>>>  arch/arm/boot/dts/exynos3250.dtsi                  | 46 +++++++--------
>>>  arch/arm/boot/dts/exynos4210.dtsi                  | 32 +++++------
>>>  arch/arm/boot/dts/exynos4412-prime.dtsi            |  4 +-
>>>  arch/arm/boot/dts/exynos4412.dtsi                  | 66 +++++++++++-----------
>>>  arch/arm/boot/dts/exynos5420.dtsi                  | 40 ++++++-------
>>>  arch/arm/boot/dts/exynos5800.dtsi                  | 56 +++++++++---------
>>>  arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi     | 48 ++++++++--------
>>>  arch/arm64/boot/dts/exynos/exynos5433.dtsi         | 50 ++++++++--------
>>>  9 files changed, 194 insertions(+), 194 deletions(-)
>>
>>
>> Thanks, split ARM64 from ARM and applied (for v4.12). arm-soc keeps DTS
>> separated between ARM architectures. Technically this should require a
>> resend but we had way too many resends for this simple patch.
>>
>
>
> Minor note:
>
> For arm64 part, Krzysztof's Signed-off-by is doubled.
> (commit 5dccc9873c4af60f4478b3ef54267353f25cc301)

Thanks for spotting it.

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH V5 3/7] ARM: exynos: Use - instead of @ for DT OPP entries
From: Masahiro Yamada @ 2017-04-24  8:30 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Viresh Kumar, arm, Chanwoo Choi, MyungJoo Ham, Kyungmin Park,
	Kukjin Kim, Javier Martinez Canillas, Rob Herring, Mark Rutland,
	linaro-kernel, linux-arm-kernel, linux-pm, Rafael Wysocki,
	Rob Herring, linux-samsung-soc, devicetree,
	Linux Kernel Mailing List
In-Reply-To: <20170420160930.halyszjunt4q2634@kozik-lap>

2017-04-21 1:09 GMT+09:00 Krzysztof Kozlowski <krzk@kernel.org>:
> On Thu, Apr 20, 2017 at 04:25:07PM +0530, Viresh Kumar wrote:
>> Compiling the DT file with W=1, DTC warns like follows:
>>
>> Warning (unit_address_vs_reg): Node /opp_table0/opp@1000000000 has a
>> unit name, but no reg property
>>
>> Fix this by replacing '@' with '-' as the OPP nodes will never have a
>> "reg" property.
>>
>> Reported-by: Krzysztof Kozlowski <krzk@kernel.org>
>> Reported-by: Masahiro Yamada <yamada.masahiro@socionext.com>
>> Suggested-by: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
>> Acked-by: Rob Herring <robh@kernel.org>
>> ---
>>  .../devicetree/bindings/devfreq/exynos-bus.txt     | 46 +++++++--------
>>  arch/arm/boot/dts/exynos3250.dtsi                  | 46 +++++++--------
>>  arch/arm/boot/dts/exynos4210.dtsi                  | 32 +++++------
>>  arch/arm/boot/dts/exynos4412-prime.dtsi            |  4 +-
>>  arch/arm/boot/dts/exynos4412.dtsi                  | 66 +++++++++++-----------
>>  arch/arm/boot/dts/exynos5420.dtsi                  | 40 ++++++-------
>>  arch/arm/boot/dts/exynos5800.dtsi                  | 56 +++++++++---------
>>  arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi     | 48 ++++++++--------
>>  arch/arm64/boot/dts/exynos/exynos5433.dtsi         | 50 ++++++++--------
>>  9 files changed, 194 insertions(+), 194 deletions(-)
>
>
> Thanks, split ARM64 from ARM and applied (for v4.12). arm-soc keeps DTS
> separated between ARM architectures. Technically this should require a
> resend but we had way too many resends for this simple patch.
>


Minor note:

For arm64 part, Krzysztof's Signed-off-by is doubled.
(commit 5dccc9873c4af60f4478b3ef54267353f25cc301)






-- 
Best Regards
Masahiro Yamada

^ permalink raw reply

* Re: [PATCH] ARM: at91/at91-pinctrl documentation: fix spelling mistake: "contoller" -> "controller"
From: Nicolas Ferre @ 2017-04-24  8:19 UTC (permalink / raw)
  To: Colin King, Linus Walleij, Rob Herring, Mark Rutland,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170421120713.4939-1-colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

Le 21/04/2017 à 14:07, Colin King a écrit :
> From: Colin Ian King <colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
> 
> trivial fix to spelling mistake in documentation
> 
> Signed-off-by: Colin Ian King <colin.king-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

Acked-by: Nicolas Ferre <nicolas.ferre-UWL1GkI3JZL3oGB3hsPCZA@public.gmane.org>

> ---
>  Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> index 9a8a45d9d8ab..590e60378be3 100644
> --- a/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> +++ b/Documentation/devicetree/bindings/pinctrl/atmel,at91-pinctrl.txt
> @@ -4,7 +4,7 @@ The AT91 Pinmux Controller, enables the IC
>  to share one PAD to several functional blocks. The sharing is done by
>  multiplexing the PAD input/output signals. For each PAD there are up to
>  8 muxing options (called periph modes). Since different modules require
> -different PAD settings (like pull up, keeper, etc) the contoller controls
> +different PAD settings (like pull up, keeper, etc) the controller controls
>  also the PAD settings parameters.
>  
>  Please refer to pinctrl-bindings.txt in this directory for details of the
> 


-- 
Nicolas Ferre
--
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/RFC 0/5] arm64: dts: renesas: Break out common board support
From: Geert Uytterhoeven @ 2017-04-24  7:25 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, Magnus Damm, Kuninori Morimoto, Yoshihiro Shimoda,
	Rob Herring, Mark Rutland, Linux-Renesas,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <1492779321-23939-1-git-send-email-geert+renesas@glider.be>

On Fri, Apr 21, 2017 at 2:55 PM, Geert Uytterhoeven
<geert+renesas@glider.be> wrote:
> Geert Uytterhoeven (5):
>   arm64: renesas: r8a7796: Add external audio clocks
>   arm64: renesas: r8a7796: Add external PCIe bus clock

Oops, the above should have had the proper "arm64: dts:" prefix.

>   [RFC] arm64: dts: r8a7796: Add placeholders for various devices
>   [RFC] arm64: dts: renesas: Extract common Salvator-X board support
>   [RFC] arm64: dts: renesas: Extract common ULCB board support

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* Re: [PATCH v2 6/9] staging: ccree: add FIPS support
From: Stephan Müller @ 2017-04-24  7:23 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: Herbert Xu, David S. Miller, Rob Herring, Mark Rutland,
	Greg Kroah-Hartman, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux kernel mailing list,
	Gilad Ben-Yossef, Binoy Jayan, Ofir Drang, Stuart Yoder
In-Reply-To: <CAOtvUMdYtLa10dUQ--5hxeYOAi+vw9OTLZF=WPmDS27cr6KvJg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Am Montag, 24. April 2017, 09:07:45 CEST schrieb Gilad Ben-Yossef:

Hi Gilad,

> I guess we could change the function to indicate that a key is valid
> for decryption but not encryption
> and have the implementation limiting based on that if there is an
> interest in SP800-131A compliance.

I would be delighted to see and review a patch.

Ciao
Stephan
--
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 6/9] staging: ccree: add FIPS support
From: Stephan Müller @ 2017-04-24  7:22 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: Mark Rutland, devel, Herbert Xu, devicetree, Greg Kroah-Hartman,
	Linux kernel mailing list, Binoy Jayan, Rob Herring, linux-crypto,
	Gilad Ben-Yossef, Stuart Yoder, David S. Miller, Ofir Drang
In-Reply-To: <CAOtvUMfOM0fG+NCX=FcqDZfZ0k54Fcie+dcikxRaFjmfubYkKQ@mail.gmail.com>

Am Montag, 24. April 2017, 09:04:13 CEST schrieb Gilad Ben-Yossef:

Hi Gilad,

> 
> Thanks you for the clarification. As I think is obvious by now I am
> not a FIPS expert by any stretch.
> 
> Isn't the requirements on DRBG or KDF invocations pertain to key
> generation only?
> What happens if you don't derive the keys on the system, but wish to
> use keys derived elsewhere?
> I assumed the limitations on weak keys etc. were meant to protect
> against that scenario and are
> therefore independent from key generation requirements, but I may have
> misunderstood that.

That is exactly an important question. NIST lately moved away from a pure 
cipher-only view of cryptography to a more holistic view (i.e. where are 
ciphers used).

That said, for 3DES there is no formal requirement that the 3 keys must be 
checked. NIST is fine when documentation says how the keys are generated by 
some logic outside the module.
> 
> Anyway, if I understand you correctly, what you are saying is that
> these checks might make an
> implementation RFC 2451 and SP800-131A compliant respectively but they
> are not required for
> FIPS 140-2 compliance? did I understand that correctly?

Correct. Regarding SP800-131A, it only says that a full 3key 3DES is required. 
It does not say whether/how the key shall be enforced not being identical.
> 
> If so, since two 3DES implementation in the kernel already do the RFC
> 2451 compliant check I assume
> you would not object to the ccree driver using the same function,
> disconnect from FIPS being set or
> not, just as is being done with the other 3DES implementation.

Absolutely. If possible, all 3DES implementations should use the same checking 
functions. The existing checking function regarding the prevention of 1key 
3DES should be used by your code too.

All I am saying that from a FIPS perspective, there is no need to extent the 
common function by a 3key 3DES enforcer.
> 
> In addition, would it be OK if we did an extra check in the ccree
> driver for SP800-131A key compliance
> and disable encryption (but allow decryption) if the key fails the
> check? again, this would be independent
> from FIPS mode?

My personal taste would be: changes that makes sense for all 3DES 
implementations should go to a common function. Otherwise, 3DES implementation 
A behaves differently than impl. B.

That said, having a check that all three keys are non-identical would 
certainly be good (see my Ack to the patch from a year ago). But you cannot 
use the argument that FIPS mandates it to push it through. :-)

Ciao
Stephan

PS: There is currently a new requirement being discussed for FIPS: 3DES 
operations should not allow more than 4GB of data to be encrypted with one 
key. This requirement would need technical enforcement. I am looking into this 
one for some time now.

^ permalink raw reply

* Re: Re: [PATCH v3 02/12] arm64: allwinner: a64: add NMI controller on A64
From: Maxime Ripard @ 2017-04-24  7:17 UTC (permalink / raw)
  To: icenowy-h8G6r0blFSE
  Cc: Lee Jones, Rob Herring, Chen-Yu Tsai, Liam Girdwood, Mark Brown,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <18ae853e9ce59a83bdeb6b64f96caee0-h8G6r0blFSE@public.gmane.org>

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

On Thu, Apr 20, 2017 at 03:03:38PM +0800, icenowy-h8G6r0blFSE@public.gmane.org wrote:
> 在 2017-04-20 13:58,Maxime Ripard 写道:
> > On Tue, Apr 18, 2017 at 06:56:43PM +0800, Icenowy Zheng wrote:
> > > 
> > > 
> > > 于 2017年4月18日 GMT+08:00 下午3:00:16, Maxime Ripard
> > > <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> 写到:
> > > >On Mon, Apr 17, 2017 at 07:57:37PM +0800, Icenowy Zheng wrote:
> > > >> Allwinner A64 SoC features a NMI controller, which is usually
> > > >connected
> > > >> to the AXP PMIC.
> > > >>
> > > >> Add support for it.
> > > >>
> > > >> Signed-off-by: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
> > > >> Acked-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
> > > >> ---
> > > >> Changes in v2:
> > > >> - Added Chen-Yu's ACK.
> > > >>
> > > >>  arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 8 ++++++++
> > > >>  1 file changed, 8 insertions(+)
> > > >>
> > > >> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > >b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > >> index 05ec9fc5e81f..53c18ca372ea 100644
> > > >> --- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > >> +++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
> > > >> @@ -403,6 +403,14 @@
> > > >>  				     <GIC_SPI 41 IRQ_TYPE_LEVEL_HIGH>;
> > > >>  		};
> > > >>
> > > >> +		nmi_intc: interrupt-controller@01f00c0c {
> > > >> +			compatible = "allwinner,sun6i-a31-sc-nmi";
> > > >> +			interrupt-controller;
> > > >> +			#interrupt-cells = <2>;
> > > >> +			reg = <0x01f00c0c 0x38>;
> > > >
> > > >The base address is not correct, and there's uncertainty on whether
> > > >this is this particular controller or not. Did you even test this?
> > > 
> > > Tested by axp20x-pek.
> > 
> > Still, the base address is wrong, which is yet another hint that this
> > is not the same interrupt controller, and just works by accident.
> 
> No, it's the same as other post-sun6i device trees.
> See other post-sun6i device trees: (or maybe they're all wrong, but
> as we have no document for it, we should temporarily keep them)
> 
> sun6i-a31.dtsi
> ```
> 		nmi_intc: interrupt-controller@01f00c0c {
> 			compatible = "allwinner,sun6i-a31-sc-nmi";
> 			interrupt-controller;
> 			#interrupt-cells = <2>;
> 			reg = <0x01f00c0c 0x38>;
> 			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> 		};
> ```
> 
> sun8i-a23-a33.dtsi
> ```
> 		nmi_intc: interrupt-controller@01f00c0c {
> 			compatible = "allwinner,sun6i-a31-sc-nmi";
> 			interrupt-controller;
> 			#interrupt-cells = <2>;
> 			reg = <0x01f00c0c 0x38>;
> 			interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> 		};
> ```
> 
> But according to the BSP device tree, the base address should be
> 0x01f00c00. Should I send some patch to fix all of them? (but it will
> break device tree compatibility)

I'm really not a big fan of "if we see something that is broken, just
let it rot" to be honest.

We have no idea how this controller works exactly, just like we have
no idea if it is exactly the same controller or not.

The only thing we have today is the memory map, and it tells us that
it has more registers than what you express here.

Because of the DT backward compatibility, you have to think of it the
other way around: what will happen if it turns out we need to setup
any register outside of that region you described in the DT, in
something like a year or so?

We can't, really. While if you have the full memory region from the
beginning, then you just have to add a single writel in your driver.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.

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

^ permalink raw reply

* Re: [PATCH v2 6/9] staging: ccree: add FIPS support
From: Gilad Ben-Yossef @ 2017-04-24  7:07 UTC (permalink / raw)
  To: Stephan Müller
  Cc: Herbert Xu, David S. Miller, Rob Herring, Mark Rutland,
	Greg Kroah-Hartman, devel, linux-crypto, devicetree,
	Linux kernel mailing list, Gilad Ben-Yossef, Binoy Jayan,
	Ofir Drang, Stuart Yoder
In-Reply-To: <2100092.9meQszc5SR@tauon.chronox.de>

On Mon, Apr 24, 2017 at 9:21 AM, Stephan Müller <smueller@chronox.de> wrote:
> Am Montag, 24. April 2017, 08:16:50 CEST schrieb Stephan Müller:
>
> Hi Gilad,
>
>> >
>> > int __des3_ede_setkey(u32 *expkey, u32 *flags, const u8 *key,
>> >
>> >                       unsigned int keylen)
>> >
>> > However, this does not check that k1 == k3. In this case DES3
>> > becomes 2DES (2-keys TDEA), the use of which was dropped post 2015
>> > by NIST Special Publication 800-131A*.
>>
>> It is correct that the RFC wants at least a 2key 3DES. And it is correct
>> that SP800-131A mandates 3key 3DES post 2015. All I am saying is that FIPS
>> 140-2 does *not* require a technical verification of the 3 keys being not
>> identical.
>
> One side note: we had discussed a patch to this function in the past, see [1].
>
> [1] https://patchwork.kernel.org/patch/8293441/
>

Thanks, I was not aware of that.

I guess we could change the function to indicate that a key is valid
for decryption but not encryption
and have the implementation limiting based on that if there is an
interest in SP800-131A compliance.

Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

^ permalink raw reply

* Re: [PATCH v2 6/9] staging: ccree: add FIPS support
From: Gilad Ben-Yossef @ 2017-04-24  7:04 UTC (permalink / raw)
  To: Stephan Müller
  Cc: Mark Rutland, devel, Herbert Xu, devicetree, Greg Kroah-Hartman,
	Linux kernel mailing list, Binoy Jayan, Rob Herring, linux-crypto,
	Gilad Ben-Yossef, Stuart Yoder, David S. Miller, Ofir Drang
In-Reply-To: <2108964.Kb0ivG6kmD@tauon.chronox.de>

On Mon, Apr 24, 2017 at 9:16 AM, Stephan Müller <smueller@chronox.de> wrote:
> Am Montag, 24. April 2017, 08:06:09 CEST schrieb Gilad Ben-Yossef:
>
> Hi Gilad,
>>
>> Well, it turns out there is and we do :-)
>>
>> This is from crypto/des_generic.c:
>>
>> /*
>>  * RFC2451:
>>  *
>>  *   For DES-EDE3, there is no known need to reject weak or
>>  *   complementation keys.  Any weakness is obviated by the use of
>>  *   multiple keys.
>>  *
>>  *   However, if the first two or last two independent 64-bit keys are
>>  *   equal (k1 == k2 or k2 == k3), then the DES3 operation is simply the
>>  *   same as DES.  Implementers MUST reject keys that exhibit this
>>  *   property.
>>  *
>>  */
>> int __des3_ede_setkey(u32 *expkey, u32 *flags, const u8 *key,
>>                       unsigned int keylen)
>>
>> However, this does not check that k1 == k3. In this case DES3
>> becomes 2DES (2-keys TDEA), the use of which was dropped post 2015
>> by NIST Special Publication 800-131A*.
>
> It is correct that the RFC wants at least a 2key 3DES. And it is correct that
> SP800-131A mandates 3key 3DES post 2015. All I am saying is that FIPS 140-2
> does *not* require a technical verification of the 3 keys being not identical.
>
> Note, formally, FIPS 140-2 requires that the 3 keys (i.e. all 192 bits) must
> be obtained from *one* call to a DRBG or KDF (separate independent calls to,
> say, obtain one key at a time is *not* permitted). Of course, fixing the
> parity bits is allowed after obtaining the random number.
>>
>> Would it be acceptable if I offer a patch adding this check to
>> __des3_ede_setkey()
>> and use that in the ccree driver?
>
> I am not sure it makes sense as the core requirement is the *one* invocation
> of the DRBG.


Thanks you for the clarification. As I think is obvious by now I am
not a FIPS expert by any stretch.

Isn't the requirements on DRBG or KDF invocations pertain to key
generation only?
What happens if you don't derive the keys on the system, but wish to
use keys derived elsewhere?
I assumed the limitations on weak keys etc. were meant to protect
against that scenario and are
therefore independent from key generation requirements, but I may have
misunderstood that.

Anyway, if I understand you correctly, what you are saying is that
these checks might make an
implementation RFC 2451 and SP800-131A compliant respectively but they
are not required for
FIPS 140-2 compliance? did I understand that correctly?

If so, since two 3DES implementation in the kernel already do the RFC
2451 compliant check I assume
you would not object to the ccree driver using the same function,
disconnect from FIPS being set or
not, just as is being done with the other 3DES implementation.

In addition, would it be OK if we did an extra check in the ccree
driver for SP800-131A key compliance
and disable encryption (but allow decryption) if the key fails the
check? again, this would be independent
from FIPS mode?

Thanks again,
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply

* Re: [PATCH v2 6/9] staging: ccree: add FIPS support
From: Stephan Müller @ 2017-04-24  6:21 UTC (permalink / raw)
  To: Stephan Müller
  Cc: Gilad Ben-Yossef, Herbert Xu, David S. Miller, Rob Herring,
	Mark Rutland, Greg Kroah-Hartman,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux kernel mailing list,
	Gilad Ben-Yossef, Binoy Jayan, Ofir Drang, Stuart Yoder
In-Reply-To: <2108964.Kb0ivG6kmD-b2PLbiJbNv8ftSvlWXw0+g@public.gmane.org>

Am Montag, 24. April 2017, 08:16:50 CEST schrieb Stephan Müller:

Hi Gilad,

> > 
> > int __des3_ede_setkey(u32 *expkey, u32 *flags, const u8 *key,
> > 
> >                       unsigned int keylen)
> > 
> > However, this does not check that k1 == k3. In this case DES3
> > becomes 2DES (2-keys TDEA), the use of which was dropped post 2015
> > by NIST Special Publication 800-131A*.
> 
> It is correct that the RFC wants at least a 2key 3DES. And it is correct
> that SP800-131A mandates 3key 3DES post 2015. All I am saying is that FIPS
> 140-2 does *not* require a technical verification of the 3 keys being not
> identical.

One side note: we had discussed a patch to this function in the past, see [1].

[1] https://patchwork.kernel.org/patch/8293441/

Ciao
Stephan
--
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 6/9] staging: ccree: add FIPS support
From: Stephan Müller @ 2017-04-24  6:16 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: Herbert Xu, David S. Miller, Rob Herring, Mark Rutland,
	Greg Kroah-Hartman, devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Linux kernel mailing list,
	Gilad Ben-Yossef, Binoy Jayan, Ofir Drang, Stuart Yoder
In-Reply-To: <CAOtvUMdF53Bu65oLtDaUKvnxcMCBY1h=Dq8LaPwTGOwg4YKYVg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Am Montag, 24. April 2017, 08:06:09 CEST schrieb Gilad Ben-Yossef:

Hi Gilad,
> 
> Well, it turns out there is and we do :-)
> 
> This is from crypto/des_generic.c:
> 
> /*
>  * RFC2451:
>  *
>  *   For DES-EDE3, there is no known need to reject weak or
>  *   complementation keys.  Any weakness is obviated by the use of
>  *   multiple keys.
>  *
>  *   However, if the first two or last two independent 64-bit keys are
>  *   equal (k1 == k2 or k2 == k3), then the DES3 operation is simply the
>  *   same as DES.  Implementers MUST reject keys that exhibit this
>  *   property.
>  *
>  */
> int __des3_ede_setkey(u32 *expkey, u32 *flags, const u8 *key,
>                       unsigned int keylen)
> 
> However, this does not check that k1 == k3. In this case DES3
> becomes 2DES (2-keys TDEA), the use of which was dropped post 2015
> by NIST Special Publication 800-131A*.

It is correct that the RFC wants at least a 2key 3DES. And it is correct that 
SP800-131A mandates 3key 3DES post 2015. All I am saying is that FIPS 140-2 
does *not* require a technical verification of the 3 keys being not identical.

Note, formally, FIPS 140-2 requires that the 3 keys (i.e. all 192 bits) must 
be obtained from *one* call to a DRBG or KDF (separate independent calls to, 
say, obtain one key at a time is *not* permitted). Of course, fixing the 
parity bits is allowed after obtaining the random number.
> 
> Would it be acceptable if I offer a patch adding this check to
> __des3_ede_setkey()
> and use that in the ccree driver?

I am not sure it makes sense as the core requirement is the *one* invocation 
of the DRBG.

Ciao
Stephan
--
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 0/3] Add R8A7743/SK-RZG1M PFC support
From: Simon Horman @ 2017-04-24  6:12 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Rob Herring, Mark Rutland, linux-renesas-soc, devicetree,
	Magnus Damm, Russell King, linux-arm-kernel
In-Reply-To: <20170420185132.258111787@cogentembedded.com>

On Thu, Apr 20, 2017 at 09:51:32PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
>    Here's the set of 3 patches against Simon Horman's 'renesas.git' repo,
> 'renesas-devel-20170420-v4.11-rc7' tag.  We're adding the R8A7743 PFC node
> and then describing the pins for SCIF0 and Ether devices declared earlier.
> These patches depend on the R8A7743 PFC suport in order to work properly.

Thanks. I have marked these as deferred pending acceptance of the PFC
driver changes. Please repost or otherwise let me know when dependency
is satisfied.

^ permalink raw reply

* Re: [PATCH v2 6/9] staging: ccree: add FIPS support
From: Gilad Ben-Yossef @ 2017-04-24  6:06 UTC (permalink / raw)
  To: Stephan Müller
  Cc: Mark Rutland, devel, Herbert Xu, devicetree, Greg Kroah-Hartman,
	Linux kernel mailing list, Binoy Jayan, Rob Herring, linux-crypto,
	Gilad Ben-Yossef, Stuart Yoder, David S. Miller, Ofir Drang
In-Reply-To: <CAOtvUMcOib2xr=THj1hy_uPtVhRHgJ2-4__mUBb5VwANQ1GA_A@mail.gmail.com>

On Sun, Apr 23, 2017 at 12:48 PM, Gilad Ben-Yossef <gilad@benyossef.com> wrote:
> Hi,
>
> Thank you for the review.
>
> On Thu, Apr 20, 2017 at 4:39 PM, Stephan Müller <smueller@chronox.de> wrote:
>
>>> +/* The function verifies that tdes keys are not weak.*/
>>> +static int ssi_fips_verify_3des_keys(const u8 *key, unsigned int keylen)
>>> +{
>>> +#ifdef CCREE_FIPS_SUPPORT
>>> +        tdes_keys_t *tdes_key = (tdes_keys_t*)key;
>>> +
>>> +     /* verify key1 != key2 and key3 != key2*/
>>
>> I do not think that this check is necessary. There is no FIPS requirement or
>> IG that mandates this (unlike the XTS key check).
>>
>> If there would be such requirement, we would need a common service function
>> for all TDES implementations


Well, it turns out there is and we do :-)

This is from crypto/des_generic.c:

/*
 * RFC2451:
 *
 *   For DES-EDE3, there is no known need to reject weak or
 *   complementation keys.  Any weakness is obviated by the use of
 *   multiple keys.
 *
 *   However, if the first two or last two independent 64-bit keys are
 *   equal (k1 == k2 or k2 == k3), then the DES3 operation is simply the
 *   same as DES.  Implementers MUST reject keys that exhibit this
 *   property.
 *
 */
int __des3_ede_setkey(u32 *expkey, u32 *flags, const u8 *key,
                      unsigned int keylen)

However, this does not check that k1 == k3. In this case DES3
becomes 2DES (2-keys TDEA), the use of which was dropped post 2015
by NIST Special Publication 800-131A*.

Would it be acceptable if I offer a patch adding this check to
__des3_ede_setkey()
and use that in the ccree driver?

* http://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-131Ar1.pdf


Many thanks,
Gilad

-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

^ permalink raw reply

* Re: [PATCH v2 0/9] drm/sun4i: Support multiple display pipelines
From: Maxime Ripard @ 2017-04-24  5:51 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: David Airlie, Rob Herring, Mark Rutland,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20170421083857.29636-1-wens-jdAy2FN1RRM@public.gmane.org>

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

On Fri, Apr 21, 2017 at 04:38:48PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> This is v2 of the series previously named "drm/sun4i: Support two
> display pipelines". As the name change suggests, the driver now
> supports any number of pipelines, though the hardware only has
> 2 or 3.
> 
> Changes since v1:
> 
>   - Add component endpoint ID numbering scheme to device tree binding.
> 
>   - Use lists to keep references to registered backends and tcons.
> 
>   - Save pointer to device node for backends.
> 
>   - Traverse the device tree of_graph starting from the tcons, going
>     up towards the inputs, and matching the device nodes with the
>     device nodes of registered backends, to find the one linked with
>     the tcon the search started from.
> 
>   - Copy the ID for the tcon from its upstream backend, instead of
>     trying, and possibly failing, to figure it out from the device
>     tree.
> 
>   - Split out hunk dropping trailing 0 from a backend error message.
> 
> Patch 1 adds the component endpoint ID numbering scheme to the
> device tree binding. New in v2.
> 
> Patch 2 adds lists to track registered display backends and TCONs,
> instead of just one pointer per component type. Previously added
> arrays of pointers in v1.
> 
> Patch 3 drops the trailing 0 from one of the backend's bind error
> messages. This was previously part of the patch "drm/sun4i: Support
> two display pipelines".
> 
> Patch 4 adds a function to fetch a backend's ID from the device tree.
> Unchanged.
> 
> Patch 5 adds a device node field to the backend data structure and
> saves a reference to the underlying device node of the backend.
> New in v2.
> 
> Patch 6 makes the tcon driver find its upstream backend by traversing
> the of_graph and matching device nodes against the device nodes of
> registered backends.
> New in v2.
> 
> Patch 7 makes the tcon driver use the ID from its associated backend.
> New in v2. This is not immediately used in this series, but will be
> used in similar fashion for downstream encoders to figure out IDs and
> muxing
> 
> Patch 8 adds device nodes for sun6i's second display pipeline.
> Unchanged.
> 
> Patch 9 enables sun6i's tcon0 by default.
> Unchanged.

Applied everything, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* [PATCH 2/2] of: Add unit tests for applying overlays.
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-24  5:43 UTC (permalink / raw)
  To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A, Michal Marek
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-kbuild-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493012595-696-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

Existing overlay unit tests examine individual pieces of the overlay
code.  The new tests target the entire process of applying an overlay.

Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---

There are checkpatch warnings.  I have reviewed them and feel they
can be ignored.

 drivers/of/fdt.c                            |  45 +++++
 drivers/of/of_private.h                     |   2 +
 drivers/of/unittest-data/Makefile           |  17 +-
 drivers/of/unittest-data/ot.dts             |  53 ++++++
 drivers/of/unittest-data/ot_bad_phandle.dts |  20 +++
 drivers/of/unittest-data/ot_base.dts        |  71 ++++++++
 drivers/of/unittest.c                       | 264 ++++++++++++++++++++++++++++
 7 files changed, 469 insertions(+), 3 deletions(-)
 create mode 100644 drivers/of/unittest-data/ot.dts
 create mode 100644 drivers/of/unittest-data/ot_bad_phandle.dts
 create mode 100644 drivers/of/unittest-data/ot_base.dts

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index e5ce4b59e162..54120ab8f322 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -31,6 +31,8 @@
 #include <asm/setup.h>  /* for COMMAND_LINE_SIZE */
 #include <asm/page.h>
 
+#include "of_private.h"
+
 /*
  * of_fdt_limit_memory - limit the number of regions in the /memory node
  * @limit: maximum entries
@@ -1256,11 +1258,54 @@ bool __init early_init_dt_scan(void *params)
  */
 void __init unflatten_device_tree(void)
 {
+#ifdef CONFIG_OF_UNITTEST
+	extern uint8_t __dtb_ot_base_begin[];
+	extern uint8_t __dtb_ot_base_end[];
+	struct device_node *ot_base_root;
+	void *ot_base;
+	u32 data_size;
+	u32 size;
+#endif
+
 	__unflatten_device_tree(initial_boot_params, NULL, &of_root,
 				early_init_dt_alloc_memory_arch, false);
 
 	/* Get pointer to "/chosen" and "/aliases" nodes for use everywhere */
 	of_alias_scan(early_init_dt_alloc_memory_arch);
+
+#ifdef CONFIG_OF_UNITTEST
+	/*
+	 * Base device tree for the overlay unittest.
+	 * Do as much as possible the same way as done for the normal FDT.
+	 * Have to stop before resolving phandles, because that uses kmalloc.
+	 */
+
+	data_size = __dtb_ot_base_end - __dtb_ot_base_begin;
+	if (!data_size) {
+		pr_err("No __dtb_ot_base_begin to attach\n");
+		return;
+	}
+
+	size = fdt_totalsize(__dtb_ot_base_begin);
+	if (size != data_size) {
+		pr_err("__dtb_ot_base_begin header totalsize != actual size");
+		return;
+	}
+
+	ot_base = early_init_dt_alloc_memory_arch(size,
+					     roundup_pow_of_two(FDT_V17_SIZE));
+	if (!ot_base) {
+		pr_err("alloc of ot_base failed");
+		return;
+	}
+
+	memcpy(ot_base, __dtb_ot_base_begin, size);
+
+	__unflatten_device_tree(ot_base, NULL, &ot_base_root,
+				early_init_dt_alloc_memory_arch, true);
+
+	unittest_set_ot_base_root(ot_base_root);
+#endif
 }
 
 /**
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index f4f6793d2f04..02d54da078ac 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -55,6 +55,8 @@ static inline int of_property_notify(int action, struct device_node *np,
 }
 #endif /* CONFIG_OF_DYNAMIC */
 
+extern void unittest_set_ot_base_root(struct device_node *dn);
+
 /**
  * General utilities for working with live trees.
  *
diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
index 1ac5cc01d627..6879befe29d2 100644
--- a/drivers/of/unittest-data/Makefile
+++ b/drivers/of/unittest-data/Makefile
@@ -1,7 +1,18 @@
 obj-y += testcases.dtb.o
+obj-y += ot.dtb.o
+obj-y += ot_bad_phandle.dtb.o
+obj-y += ot_base.dtb.o
 
 targets += testcases.dtb testcases.dtb.S
+targets += ot.dtb ot.dtb.S
+targets += ot_bad_phandle.dtb ot_bad_phandle.dtb.S
+targets += ot_base.dtb ot_base.dtb.S
 
-.SECONDARY: \
-	$(obj)/testcases.dtb.S \
-	$(obj)/testcases.dtb
+.PRECIOUS: \
+	$(obj)/%.dtb.S \
+	$(obj)/%.dtb
+
+# enable creation of __symbols__ node
+DTC_FLAGS_ot := -@
+DTC_FLAGS_ot_base := -@
+DTC_FLAGS_ot_bad_phandle := -@
diff --git a/drivers/of/unittest-data/ot.dts b/drivers/of/unittest-data/ot.dts
new file mode 100644
index 000000000000..37e105622b7a
--- /dev/null
+++ b/drivers/of/unittest-data/ot.dts
@@ -0,0 +1,53 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+
+	fragment@0 {
+		target = <&electric_1>;
+
+		__overlay__ {
+			status = "ok";
+
+			hvac_2: hvac_large_1 {
+				compatible = "ot,hvac-large";
+				heat-range = < 40 75 >;
+				cool-range = < 65 80 >;
+			};
+		};
+	};
+
+	fragment@1 {
+		target = <&rides_1>;
+
+		__overlay__ {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			status = "ok";
+
+			ride@200 {
+				compatible = "ot,ferris-wheel";
+				reg = < 0x00000200 0x100 >;
+				hvac-provider = < &hvac_2 >;
+				hvac-thermostat = < 27 32 > ;
+				hvac-zones = < 12 5 >;
+				hvac-zone-names = "operator", "snack-bar";
+				spin-controller = < &spin_ctrl_1 3 >;
+				spin-rph = < 30 >;
+				gondolas = < 16 >;
+				gondola-capacity = < 6 >;
+			};
+		};
+	};
+
+	fragment@2 {
+		target = <&lights_2>;
+
+		__overlay__ {
+			status = "ok";
+			color = "purple", "white", "red", "green";
+			rate = < 3 256 >;
+		};
+	};
+
+};
diff --git a/drivers/of/unittest-data/ot_bad_phandle.dts b/drivers/of/unittest-data/ot_bad_phandle.dts
new file mode 100644
index 000000000000..234d5f1dcfe4
--- /dev/null
+++ b/drivers/of/unittest-data/ot_bad_phandle.dts
@@ -0,0 +1,20 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+
+	fragment@0 {
+		target = <&electric_1>;
+
+		__overlay__ {
+
+			// This label should cause an error when the overlay
+			// is applied.  There is already a phandle value
+			// in the base tree for motor_1.
+			spin_ctrl_1_conflict: motor_1 {
+				accelerate = < 3 >;
+				decelerate = < 5 >;
+			};
+		};
+	};
+};
diff --git a/drivers/of/unittest-data/ot_base.dts b/drivers/of/unittest-data/ot_base.dts
new file mode 100644
index 000000000000..0a4fbe598b02
--- /dev/null
+++ b/drivers/of/unittest-data/ot_base.dts
@@ -0,0 +1,71 @@
+/dts-v1/;
+/plugin/;
+
+/ {
+	testcase-data-2 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		electric_1: substation@100 {
+			compatible = "ot,big-volts-control";
+			reg = < 0x00000100 0x100 >;
+			status = "disabled";
+
+			hvac_1: hvac_medium_1 {
+				compatible = "ot,hvac-medium";
+				heat-range = < 50 75 >;
+				cool-range = < 60 80 >;
+			};
+
+			spin_ctrl_1: motor_1 {
+				compatible = "ot,ferris-wheel-motor";
+				spin = "clockwise";
+			};
+
+			spin_ctrl_2: motor_8 {
+				compatible = "ot,roller-coaster-motor";
+			};
+		};
+
+		rides_1: fairway_1 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "ot,rides";
+			status = "disabled";
+			orientation = < 127 >;
+
+			ride@100 {
+				compatible = "ot,roller-coaster";
+				reg = < 0x00000100 0x100 >;
+				hvac-provider = < &hvac_1 >;
+				hvac-thermostat = < 29 > ;
+				hvac-zones = < 14 >;
+				hvac-zone-names = "operator";
+				spin-controller = < &spin_ctrl_2 5 &spin_ctrl_2 7 >;
+				spin-controller-names = "track_1", "track_2";
+				queues = < 2 >;
+			};
+		};
+
+		lights_1: lights@30000 {
+			compatible = "ot,work-lights";
+			reg = < 0x00030000 0x1000 >;
+			status = "disabled";
+		};
+
+		lights_2: lights@40000 {
+			compatible = "ot,show-lights";
+			reg = < 0x00040000 0x1000 >;
+			status = "disabled";
+			rate = < 13 138 >;
+		};
+
+		retail_1: vending@50000 {
+			reg = < 0x00050000 0x1000 >;
+			compatible = "ot,tickets";
+			status = "disabled";
+		};
+
+	};
+};
+
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 62db55b97c10..599eb10e9b40 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -8,6 +8,7 @@
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/hashtable.h>
+#include <linux/libfdt.h>
 #include <linux/of.h>
 #include <linux/of_fdt.h>
 #include <linux/of_irq.h>
@@ -42,6 +43,13 @@
 	failed; \
 })
 
+static struct device_node *ot_base_root;
+
+void unittest_set_ot_base_root(struct device_node *dn)
+{
+	ot_base_root = dn;
+}
+
 static void __init of_unittest_find_node_by_name(void)
 {
 	struct device_node *np;
@@ -1925,6 +1933,259 @@ static void __init of_unittest_overlay(void)
 static inline void __init of_unittest_overlay(void) { }
 #endif
 
+#define OVERLAY_INFO_EXTERN(name) \
+	extern uint8_t __dtb_##name##_begin[]; \
+	extern uint8_t __dtb_##name##_end[]
+
+#define OVERLAY_INFO(name, expected) \
+{	.dtb_begin	 = __dtb_##name##_begin, \
+	.dtb_end	 = __dtb_##name##_end, \
+	.expected_result = expected, \
+}
+
+struct overlay_info {
+	uint8_t		   *dtb_begin;
+	uint8_t		   *dtb_end;
+	void		   *data;
+	struct device_node *np_overlay;
+	int		   expected_result;
+	int		   overlay_id;
+};
+
+OVERLAY_INFO_EXTERN(ot);
+OVERLAY_INFO_EXTERN(ot_bad_phandle);
+
+struct overlay_info overlays[] = {
+	OVERLAY_INFO(ot, 0),
+	OVERLAY_INFO(ot_bad_phandle, -EINVAL),
+	{}
+};
+
+#ifdef CONFIG_OF_OVERLAY
+/*
+ * The purpose of of_unittest_overlay_test_data_add is to add an
+ * overlay in the normal fashion.  This is a test of the whole
+ * picture, instead of testing individual elements.
+ *
+ * A secondary purpose is to be able to verify that the contents of
+ * /proc/device-tree/ contains the updated structure and values from
+ * the overlay.  That must be verified separately in user space.
+ *
+ * Return 0 on unexpected error.
+ */
+static int __init overlay_test_data_add(int onum)
+{
+	/*
+	 * __dtb_ot_begin[] and __dtb_ot_end[] are
+	 * created by cmd_dt_S_dtb in scripts/Makefile.lib
+	 */
+	struct overlay_info *info;
+	int k;
+	int ret;
+	u32 size;
+	u32 size_from_header;
+
+	for (k = 0, info = overlays; info; info++, k++) {
+		if (k == onum)
+			break;
+	}
+	if (onum > k)
+		return 0;
+
+	size = info->dtb_end - info->dtb_begin;
+	if (!size) {
+		pr_err("no overlay to attach, %d\n", onum);
+		ret = 0;
+	}
+
+	size_from_header = fdt_totalsize(info->dtb_begin);
+	if (size_from_header != size) {
+		pr_err("overlay header totalsize != actual size, %d", onum);
+		return 0;
+	}
+
+	/*
+	 * Must create permanent copy of FDT because of_fdt_unflatten_tree()
+	 * will create pointers to the passed in FDT in the EDT.
+	 */
+	info->data = kmemdup(info->dtb_begin, size, GFP_KERNEL);
+	if (!info->data) {
+		pr_err("unable to allocate memory for data, %d\n", onum);
+		return 0;
+	}
+
+	of_fdt_unflatten_tree(info->data, NULL, &info->np_overlay);
+	if (!info->np_overlay) {
+		pr_err("unable to unflatten overlay, %d\n", onum);
+		ret = 0;
+		goto out_free_data;
+	}
+	of_node_set_flag(info->np_overlay, OF_DETACHED);
+
+	ret = of_resolve_phandles(info->np_overlay);
+	if (ret) {
+		pr_err("resolve ot phandles (ret=%d), %d\n", ret, onum);
+		goto out_free_np_overlay;
+	}
+
+	ret = of_overlay_create(info->np_overlay);
+	if (ret < 0) {
+		pr_err("of_overlay_create() (ret=%d), %d\n", ret, onum);
+		goto out_free_np_overlay;
+	} else {
+		info->overlay_id = ret;
+		ret = 0;
+	}
+
+	pr_debug("__dtb_ot_begin applied, overlay id %d\n", ret);
+
+	goto out;
+
+out_free_np_overlay:
+	/*
+	 * info->np_overlay is the unflattened device tree
+	 * It has not been spliced into the live tree.
+	 */
+
+	/* todo: function to free unflattened device tree */
+
+out_free_data:
+	kfree(info->data);
+
+out:
+	return (ret == info->expected_result);
+}
+
+/*
+ * The purpose of of_unittest_overlay_high_level is to add an overlay
+ * in the normal fashion.  This is a test of the whole picture,
+ * instead of individual elements.
+ *
+ * The first part of the function is _not_ normal overlay usage; it is
+ * finishing splicing the base overlay device tree into the live tree.
+ */
+static __init void of_unittest_overlay_high_level(void)
+{
+	struct device_node *last_sibling;
+	struct device_node *np;
+	struct device_node *of_symbols;
+	struct device_node *ot_base_symbols;
+	struct device_node **pprev;
+	struct property *prop;
+	int ret;
+
+	if (!ot_base_root) {
+		unittest(0, "ot_base_root not initialized\n");
+		return;
+	}
+
+	/*
+	 * Could not fixup phandles in unflatten_device_tree() because
+	 * kmalloc() was not yet available.
+	 */
+	of_resolve_phandles(ot_base_root);
+
+	/*
+	 * do not allow ot_base to duplicate any node already in tree,
+	 * this greatly simplifies the code
+	 */
+
+
+	/* remove ot_base_root node "__local_fixups" */
+	pprev = &ot_base_root->child;
+	for (np = ot_base_root->child; np; np = np->sibling) {
+		if (!of_node_cmp(np->name, "__local_fixups__")) {
+			*pprev = np->sibling;
+			break;
+		}
+		pprev = &np->sibling;
+	}
+
+
+	/* remove ot_base_root node "__symbols__" if in live tree*/
+	of_symbols = of_get_child_by_name(of_root, "__symbols__");
+	if (of_symbols) {
+		/* will have to graft properties from node into live tree */
+		pprev = &ot_base_root->child;
+		for (np = ot_base_root->child; np; np = np->sibling) {
+			if (!of_node_cmp(np->name, "__symbols__")) {
+				ot_base_symbols = np;
+				*pprev = np->sibling;
+				break;
+			}
+			pprev = &np->sibling;
+		}
+	}
+
+	for (np = ot_base_root->child; np; np = np->sibling) {
+		if (of_get_child_by_name(of_root, np->name)) {
+			unittest(0, "illegal node name in ot_base %s",
+				np->name);
+			return;
+		}
+	}
+
+	/*
+	 * __dtb_ot_base_begin is not allowed to have root properties,
+	 * so only need to splice nodes into main device tree.
+	 *
+	 * root node of *ot_base_root will not be freed, it is lost
+	 * memory.
+	 */
+
+	for (np = ot_base_root->child; np; np = np->sibling)
+		np->parent = of_root;
+
+	mutex_lock(&of_mutex);
+
+	for (np = of_root->child; np; np = np->sibling)
+		last_sibling = np;
+
+	if (last_sibling)
+		last_sibling->sibling = ot_base_root->child;
+	else
+		of_root->child = ot_base_root->child;
+
+	for_each_of_allnodes_from(ot_base_root, np)
+		__of_attach_node_sysfs(np);
+
+	if (of_symbols) {
+		for_each_property_of_node(ot_base_symbols, prop) {
+			ret = __of_add_property(of_symbols, prop);
+			if (ret) {
+				unittest(0,
+					 "duplicate property '%s' in ot_base node __symbols__",
+					 prop->name);
+				return;
+			}
+			ret = __of_add_property_sysfs(of_symbols, prop);
+			if (ret) {
+				unittest(0,
+					 "unable to add property '%s' in ot_base node __symbols__ to sysfs",
+					 prop->name);
+				return;
+			}
+		}
+	}
+
+	mutex_unlock(&of_mutex);
+
+
+	/* now do the normal overlay usage test */
+
+	unittest(overlay_test_data_add(0),
+		 "Adding overlay __dtb_ot_begin failed\n");
+
+	unittest(overlay_test_data_add(1),
+		 "Adding overlay phandle conflict failed\n");
+}
+
+#else
+
+static inline __init void of_unittest_overlay_high_level(void) {}
+
+#endif
+
 static int __init of_unittest(void)
 {
 	struct device_node *np;
@@ -1962,6 +2223,9 @@ static int __init of_unittest(void)
 	/* Double check linkage after removing testcase data */
 	of_unittest_check_tree_linkage();
 
+
+	of_unittest_overlay_high_level();
+
 	pr_info("end of unittest - %i passed, %i failed\n",
 		unittest_results.passed, unittest_results.failed);
 
-- 
Frank Rowand <frank.rowand-7U/KSKJipcs@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 related

* [PATCH 1/2] of: add support of dtc compiler flags for device tree overlays
From: frowand.list @ 2017-04-24  5:43 UTC (permalink / raw)
  To: Rob Herring, stephen.boyd, Michal Marek
  Cc: devicetree, linux-kernel, linux-kbuild
In-Reply-To: <1493012595-696-1-git-send-email-frowand.list@gmail.com>

From: Frank Rowand <frank.rowand@sony.com>

The dtc compiler version that adds initial support was available
in 4.11-rc1.  Add the ability to set the dtc compiler flags needed
by overlays.

Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
 scripts/Makefile.lib | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 0a07f9014944..0bbec480d323 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -283,6 +283,8 @@ ifeq ($(KBUILD_ENABLE_EXTRA_GCC_CHECKS),)
 DTC_FLAGS += -Wno-unit_address_vs_reg
 endif
 
+DTC_FLAGS += $(DTC_FLAGS_$(basetarget))
+
 # Generate an assembly file to wrap the output of the device tree compiler
 quiet_cmd_dt_S_dtb= DTB     $@
 cmd_dt_S_dtb=						\
-- 
Frank Rowand <frank.rowand@sony.com>

^ permalink raw reply related

* [PATCH 0/2] of: Add unit tests for applying overlays
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-24  5:43 UTC (permalink / raw)
  To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A, Michal Marek
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-kbuild-u79uwXL29TY76Z2rM5mHXA

From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

Existing overlay unit tests examine individual pieces of the overlay
code.  The new tests target the entire process of applying an overlay.

Frank Rowand (2):
  of: add support of dtc compiler flags for device tree overlays
  of: Add unit tests for applying overlays.

 drivers/of/fdt.c                            |  45 +++++
 drivers/of/of_private.h                     |   2 +
 drivers/of/unittest-data/Makefile           |  17 +-
 drivers/of/unittest-data/ot.dts             |  53 ++++++
 drivers/of/unittest-data/ot_bad_phandle.dts |  20 +++
 drivers/of/unittest-data/ot_base.dts        |  71 ++++++++
 drivers/of/unittest.c                       | 264 ++++++++++++++++++++++++++++
 scripts/Makefile.lib                        |   2 +
 8 files changed, 471 insertions(+), 3 deletions(-)
 create mode 100644 drivers/of/unittest-data/ot.dts
 create mode 100644 drivers/of/unittest-data/ot_bad_phandle.dts
 create mode 100644 drivers/of/unittest-data/ot_base.dts

-- 
Frank Rowand <frank.rowand-7U/KSKJipcs@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

* [PATCH 3/3] of: detect invalid phandle in overlay
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-24  5:20 UTC (permalink / raw)
  To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493011204-27635-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

Overlays are not allowed to modify phandle values of previously existing
nodes because there is no information available to allow fixup up of
properties that use the previously existing phandle.

Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
 drivers/of/overlay.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 7827786718d8..c0e4ee1cd1ba 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -132,6 +132,10 @@ static int of_overlay_apply_single_device_node(struct of_overlay *ov,
 	/* NOTE: Multiple mods of created nodes not supported */
 	tchild = of_get_child_by_name(target, cname);
 	if (tchild != NULL) {
+		/* new overlay phandle value conflicts with existing value */
+		if (child->phandle)
+			return -EINVAL;
+
 		/* apply overlay recursively */
 		ret = of_overlay_apply_one(ov, tchild, child);
 		of_node_put(tchild);
-- 
Frank Rowand <frank.rowand-7U/KSKJipcs@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 related

* [PATCH 2/3] of: make __of_attach_node() static
From: frowand.list-Re5JQEeQqe8AvxtiuMwx3w @ 2017-04-24  5:20 UTC (permalink / raw)
  To: Rob Herring, stephen.boyd-QSEj5FYQhm4dnm+yROfE0A
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1493011204-27635-1-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>

__of_attach_node() is not used outside of drivers/of/dynamic.c.  Make
it static and remove it from drivers/of/of_private.h.

Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
---
 drivers/of/dynamic.c    | 2 +-
 drivers/of/of_private.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 44963b4e7235..9dcadd704aee 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -216,7 +216,7 @@ int of_property_notify(int action, struct device_node *np,
 	return of_reconfig_notify(action, &pr);
 }
 
-void __of_attach_node(struct device_node *np)
+static void __of_attach_node(struct device_node *np)
 {
 	const __be32 *phandle;
 	int sz;
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 554394c96569..f4f6793d2f04 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -81,7 +81,6 @@ extern int __of_update_property(struct device_node *np,
 extern void __of_update_property_sysfs(struct device_node *np,
 		struct property *newprop, struct property *oldprop);
 
-extern void __of_attach_node(struct device_node *np);
 extern int __of_attach_node_sysfs(struct device_node *np);
 extern void __of_detach_node(struct device_node *np);
 extern void __of_detach_node_sysfs(struct device_node *np);
-- 
Frank Rowand <frank.rowand-7U/KSKJipcs@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 related

* [PATCH 1/3] of: overlay_adjust_phandles() - do not modify const field
From: frowand.list @ 2017-04-24  5:20 UTC (permalink / raw)
  To: Rob Herring, stephen.boyd; +Cc: devicetree, linux-kernel
In-Reply-To: <1493011204-27635-1-git-send-email-frowand.list@gmail.com>

From: Frank Rowand <frank.rowand@sony.com>

When adjusting overlay phandles to apply to the live device tree, can
not modify the property value because it is type const.

This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *.  As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.

  [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html

Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
 drivers/of/base.c       |  4 ++--
 drivers/of/dynamic.c    | 28 +++++++++++++++++++++------
 drivers/of/of_private.h |  3 +++
 drivers/of/resolver.c   | 51 ++++++++++++++++++++++++++++++-------------------
 4 files changed, 58 insertions(+), 28 deletions(-)

diff --git a/drivers/of/base.c b/drivers/of/base.c
index d7c4629a3a2d..b41650fd0fcf 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -220,8 +220,8 @@ void __init of_core_init(void)
 		proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
 }
 
-static struct property *__of_find_property(const struct device_node *np,
-					   const char *name, int *lenp)
+struct property *__of_find_property(const struct device_node *np,
+				    const char *name, int *lenp)
 {
 	struct property *pp;
 
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 888fdbc09992..44963b4e7235 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -354,17 +354,17 @@ void of_node_release(struct kobject *kobj)
 }
 
 /**
- * __of_prop_dup - Copy a property dynamically.
+ * __of_prop_alloc - Create a property dynamically.
  * @prop:	Property to copy
  * @allocflags:	Allocation flags (typically pass GFP_KERNEL)
  *
- * Copy a property by dynamically allocating the memory of both the
+ * Create a property by dynamically allocating the memory of both the
  * property structure and the property name & contents. The property's
  * flags have the OF_DYNAMIC bit set so that we can differentiate between
  * dynamically allocated properties and not.
  * Returns the newly allocated property or NULL on out of memory error.
  */
-struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
+struct property *__of_prop_alloc(char *name, void *value, int len, gfp_t allocflags)
 {
 	struct property *new;
 
@@ -378,9 +378,9 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
 	 * of zero bytes. We do this to work around the use
 	 * of of_get_property() calls on boolean values.
 	 */
-	new->name = kstrdup(prop->name, allocflags);
-	new->value = kmemdup(prop->value, prop->length, allocflags);
-	new->length = prop->length;
+	new->name = kstrdup(name, allocflags);
+	new->value = kmemdup(value, len, allocflags);
+	new->length = len;
 	if (!new->name || !new->value)
 		goto err_free;
 
@@ -397,6 +397,22 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
 }
 
 /**
+ * __of_prop_dup - Copy a property dynamically.
+ * @prop:	Property to copy
+ * @allocflags:	Allocation flags (typically pass GFP_KERNEL)
+ *
+ * Copy a property by dynamically allocating the memory of both the
+ * property structure and the property name & contents. The property's
+ * flags have the OF_DYNAMIC bit set so that we can differentiate between
+ * dynamically allocated properties and not.
+ * Returns the newly allocated property or NULL on out of memory error.
+ */
+struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
+{
+	return __of_prop_alloc(prop->name, prop->value, prop->length, allocflags);
+}
+
+/**
  * __of_node_dup() - Duplicate or create an empty device node dynamically.
  * @fmt: Format string (plus vargs) for new full name of the device node
  *
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index 18bbb4517e25..554394c96569 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -62,6 +62,7 @@ static inline int of_property_notify(int action, struct device_node *np,
  * without taking node references, so you either have to
  * own the devtree lock or work on detached trees only.
  */
+struct property *__of_prop_alloc(char *name, void *value, int len, gfp_t allocflags);
 struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags);
 __printf(2, 3) struct device_node *__of_node_dup(const struct device_node *np, const char *fmt, ...);
 
@@ -70,6 +71,8 @@ extern const void *__of_get_property(const struct device_node *np,
 extern int __of_add_property(struct device_node *np, struct property *prop);
 extern int __of_add_property_sysfs(struct device_node *np,
 		struct property *prop);
+extern struct property *__of_find_property(const struct device_node *np,
+					   const char *name, int *lenp);
 extern int __of_remove_property(struct device_node *np, struct property *prop);
 extern void __of_remove_property_sysfs(struct device_node *np,
 		struct property *prop);
diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 7ae9863cb0a4..a2d5b8f0b7bf 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -20,6 +20,8 @@
 #include <linux/errno.h>
 #include <linux/slab.h>
 
+#include "of_private.h"
+
 /* illegal phandle value (set when unresolved) */
 #define OF_PHANDLE_ILLEGAL	0xdeadbeef
 
@@ -67,36 +69,43 @@ static phandle live_tree_max_phandle(void)
 	return phandle;
 }
 
-static void adjust_overlay_phandles(struct device_node *overlay,
+static int adjust_overlay_phandles(struct device_node *overlay,
 		int phandle_delta)
 {
 	struct device_node *child;
+	struct property *newprop;
 	struct property *prop;
 	phandle phandle;
+	int ret;
 
-	/* adjust node's phandle in node */
-	if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL)
-		overlay->phandle += phandle_delta;
-
-	/* copy adjusted phandle into *phandle properties */
-	for_each_property_of_node(overlay, prop) {
+	if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL) {
 
-		if (of_prop_cmp(prop->name, "phandle") &&
-		    of_prop_cmp(prop->name, "linux,phandle"))
-			continue;
-
-		if (prop->length < 4)
-			continue;
+		overlay->phandle += phandle_delta;
 
-		phandle = be32_to_cpup(prop->value);
-		if (phandle == OF_PHANDLE_ILLEGAL)
-			continue;
+		phandle = cpu_to_be32(overlay->phandle);
+
+		prop = __of_find_property(overlay, "phandle", NULL);
+		newprop = __of_prop_alloc(prop->name, &phandle, sizeof(phandle),
+					  GFP_KERNEL);
+		if (!newprop)
+			return -ENOMEM;
+		__of_update_property(overlay, newprop, &prop);
+
+		prop = __of_find_property(overlay, "linux,phandle", NULL);
+		newprop = __of_prop_alloc(prop->name, &phandle, sizeof(phandle),
+					  GFP_KERNEL);
+		if (!newprop)
+			return -ENOMEM;
+		__of_update_property(overlay, newprop, &prop);
+	}
 
-		*(uint32_t *)prop->value = cpu_to_be32(overlay->phandle);
+	for_each_child_of_node(overlay, child) {
+		ret = adjust_overlay_phandles(child, phandle_delta);
+		if (ret)
+			return ret;
 	}
 
-	for_each_child_of_node(overlay, child)
-		adjust_overlay_phandles(child, phandle_delta);
+	return 0;
 }
 
 static int update_usages_of_a_phandle_reference(struct device_node *overlay,
@@ -306,7 +315,9 @@ int of_resolve_phandles(struct device_node *overlay)
 	}
 
 	phandle_delta = live_tree_max_phandle() + 1;
-	adjust_overlay_phandles(overlay, phandle_delta);
+	err = adjust_overlay_phandles(overlay, phandle_delta);
+	if (err)
+		goto out;
 
 	for_each_child_of_node(overlay, local_fixups)
 		if (!of_node_cmp(local_fixups->name, "__local_fixups__"))
-- 
Frank Rowand <frank.rowand@sony.com>

^ permalink raw reply related

* [PATCH 0/3] of: fix overlay modification of const variable
From: frowand.list @ 2017-04-24  5:20 UTC (permalink / raw)
  To: Rob Herring, stephen.boyd; +Cc: devicetree, linux-kernel

From: Frank Rowand <frank.rowand@sony.com>

When adjusting overlay phandles to apply to the live device tree, can
not modify the property value because it is type const.
    
This is to resolve the issue found by Stephen Boyd [1] when he changed
the type of struct property.value from void * to const void *.  As
a result of the type change, the overlay code had compile errors
where the resolver updates phandle values.
    
  [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html

Patch 1 fixes the const variable problem.

Patches 2 and 3 are minor fixups for issues that became visible
while implementing patch 1.

Frank Rowand (3):
  of: overlay_adjust_phandles() - do not modify const field
  of: make __of_attach_node() static
  of: detect invalid phandle in overlay

 drivers/of/base.c       |  4 ++--
 drivers/of/dynamic.c    | 30 ++++++++++++++++++++++-------
 drivers/of/of_private.h |  4 +++-
 drivers/of/overlay.c    |  4 ++++
 drivers/of/resolver.c   | 51 ++++++++++++++++++++++++++++++-------------------
 5 files changed, 63 insertions(+), 30 deletions(-)

-- 
Frank Rowand <frank.rowand@sony.com>

^ permalink raw reply

* RE: [PATCH 1/2 v2] dt-bindings: qoriq-clock: Add coreclk
From: Andy Tang @ 2017-04-24  3:14 UTC (permalink / raw)
  To: mturquette@baylibre.com, sboyd@codeaurora.org
  Cc: robh+dt@kernel.org, mark.rutland@arm.com,
	linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, Scott Wood
In-Reply-To: <DB6PR0402MB283701D478A1678D28894C77F30A0@DB6PR0402MB2837.eurprd04.prod.outlook.com>

Does anyone give me a clue why this patch set can't be responded after so long time?

Thanks,
Andy

-----Original Message-----
From: Andy Tang 
Sent: Monday, April 17, 2017 9:37 AM
To: 'mturquette@baylibre.com' <mturquette@baylibre.com>; 'sboyd@codeaurora.org' <sboyd@codeaurora.org>
Cc: 'robh+dt@kernel.org' <robh+dt@kernel.org>; 'mark.rutland@arm.com' <mark.rutland@arm.com>; 'linux-clk@vger.kernel.org' <linux-clk@vger.kernel.org>; 'devicetree@vger.kernel.org' <devicetree@vger.kernel.org>; 'linux-kernel@vger.kernel.org' <linux-kernel@vger.kernel.org>; 'linux-arm-kernel@lists.infradead.org' <linux-arm-kernel@lists.infradead.org>; 'Scott Wood' <oss@buserror.net>
Subject: RE: [PATCH 1/2 v2] dt-bindings: qoriq-clock: Add coreclk

Hi Stephen and Michael,

This patch set has been pending for more than two months since it was first sent.
I have not received any response from you until now.

Could you give some comments on it?

Regards,
Andy

-----Original Message-----
From: Andy Tang
Sent: Wednesday, April 05, 2017 2:16 PM
To: mturquette@baylibre.com; sboyd@codeaurora.org
Cc: robh+dt@kernel.org; mark.rutland@arm.com; linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Scott Wood <oss@buserror.net>
Subject: RE: [PATCH 1/2 v2] dt-bindings: qoriq-clock: Add coreclk

Hello 

Do you have any comments on this patch set which was acked by Rob?

Regards,
Andy

> -----Original Message-----
> From: Yuantian Tang [mailto:andy.tang@nxp.com]
> Sent: Monday, March 20, 2017 10:37 AM
> To: mturquette@baylibre.com
> Cc: sboyd@codeaurora.org; robh+dt@kernel.org; mark.rutland@arm.com; 
> linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux- 
> kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org; Scott 
> Wood <oss@buserror.net>; Andy Tang <andy.tang@nxp.com>
> Subject: [PATCH 1/2 v2] dt-bindings: qoriq-clock: Add coreclk
> 
> From: Scott Wood <oss@buserror.net>
> 
> ls1012a has separate input root clocks for core PLLs versus the 
> platform PLL, with the latter described as sysclk in the hw docs.
> Update the qoriq-clock binding to allow a second input clock, named 
> "coreclk".  If present, this clock will be used for the core PLLs.
> 
> Signed-off-by: Scott Wood <oss@buserror.net>
> Signed-off-by: Tang Yuantian <andy.tang@nxp.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> v2:
> 	-- change the author to Scott
>  Documentation/devicetree/bindings/clock/qoriq-clock.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> index aa3526f..119cafd 100644
> --- a/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> +++ b/Documentation/devicetree/bindings/clock/qoriq-clock.txt
> @@ -56,6 +56,11 @@ Optional properties:
>  - clocks: If clock-frequency is not specified, sysclk may be provided
>  	as an input clock.  Either clock-frequency or clocks must be
>  	provided.
> +	A second input clock, called "coreclk", may be provided if
> +	core PLLs are based on a different input clock from the
> +	platform PLL.
> +- clock-names: Required if a coreclk is present.  Valid names are
> +	"sysclk" and "coreclk".
> 
>  2. Clock Provider
> 
> @@ -72,6 +77,7 @@ second cell is the clock index for the specified type.
>  	2	hwaccel		index (n in CLKCGnHWACSR)
>  	3	fman		0 for fm1, 1 for fm2
>  	4	platform pll	0=pll, 1=pll/2, 2=pll/3, 3=pll/4
> +	5	coreclk		must be 0
> 
>  3. Example
> 
> --
> 2.1.0.27.g96db324


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox