Devicetree
 help / color / mirror / Atom feed
* [PATCH v3] spi-imx: Implements handling of the SPI_READY mode flag.
From: Leif Middelschulte @ 2017-04-23 19:19 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Leif Middelschulte

From: Leif Middelschulte <leif.middelschulte-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This patch implements consideration of the SPI_READY mode flag as
defined in spi.h. It extends the device tree bindings to support
the values defined by the reference manual for the DRCTL field.

Thus supporting edge-triggered and level-triggered bursts.

Signed-off-by: Leif Middelschulte <Leif.Middelschulte-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/spi/fsl-imx-cspi.txt         |  7 +++++++
 drivers/spi/spi-imx.c                                | 20 ++++++++++++++++++--
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
index 8bc95e2fc47f..31b5b21598ff 100644
--- a/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
+++ b/Documentation/devicetree/bindings/spi/fsl-imx-cspi.txt
@@ -23,6 +23,12 @@ See the clock consumer binding,
 Obsolete properties:
 - fsl,spi-num-chipselects : Contains the number of the chipselect
 
+Optional properties:
+- fsl,spi-rdy-drctl: Integer, representing the value of DRCTL, the register
+controlling the SPI_READY handling. Note that to enable the DRCTL consideration,
+the SPI_READY mode-flag needs to be set too.
+Valid values are: 0 (disabled), 1 (edge-triggered burst) and 2 (level-triggered burst).
+
 Example:
 
 ecspi@70010000 {
@@ -35,4 +41,5 @@ ecspi@70010000 {
 		   <&gpio3 25 0>; /* GPIO3_25 */
 	dmas = <&sdma 3 7 1>, <&sdma 4 7 2>;
 	dma-names = "rx", "tx";
+	fsl,spi-rdy-drctl = <1>;
 };
diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 9a7c62f471dc..b402530a7a9a 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -95,6 +95,7 @@ struct spi_imx_data {
 	unsigned int spi_bus_clk;
 
 	unsigned int bytes_per_word;
+	unsigned int spi_drctl;
 
 	unsigned int count;
 	void (*tx)(struct spi_imx_data *);
@@ -246,6 +247,7 @@ static bool spi_imx_can_dma(struct spi_master *master, struct spi_device *spi,
 #define MX51_ECSPI_CTRL_XCH		(1 <<  2)
 #define MX51_ECSPI_CTRL_SMC		(1 << 3)
 #define MX51_ECSPI_CTRL_MODE_MASK	(0xf << 4)
+#define MX51_ECSPI_CTRL_DRCTL(drctl)	((drctl) << 16)
 #define MX51_ECSPI_CTRL_POSTDIV_OFFSET	8
 #define MX51_ECSPI_CTRL_PREDIV_OFFSET	12
 #define MX51_ECSPI_CTRL_CS(cs)		((cs) << 18)
@@ -355,6 +357,12 @@ static int mx51_ecspi_config(struct spi_device *spi,
 	 */
 	ctrl |= MX51_ECSPI_CTRL_MODE_MASK;
 
+	/*
+	 * Enable SPI_RDY handling (falling edge/level triggered).
+	 */
+	if (spi->mode & SPI_READY)
+		ctrl |= MX51_ECSPI_CTRL_DRCTL(spi_imx->spi_drctl);
+
 	/* set clock speed */
 	ctrl |= mx51_ecspi_clkdiv(spi_imx, config->speed_hz, &clk);
 	spi_imx->spi_bus_clk = clk;
@@ -1173,7 +1181,7 @@ static int spi_imx_probe(struct platform_device *pdev)
 	struct spi_master *master;
 	struct spi_imx_data *spi_imx;
 	struct resource *res;
-	int i, ret, irq;
+	int i, ret, irq, spi_drctl;
 
 	if (!np && !mxc_platform_info) {
 		dev_err(&pdev->dev, "can't get the platform data\n");
@@ -1181,6 +1189,12 @@ static int spi_imx_probe(struct platform_device *pdev)
 	}
 
 	master = spi_alloc_master(&pdev->dev, sizeof(struct spi_imx_data));
+	ret = of_property_read_u32(np, "fsl,spi-rdy-drctl", &spi_drctl);
+	if ((ret < 0) || (spi_drctl >= 0x3)) {
+		/* '11' is reserved */
+		spi_drctl = 0;
+	}
+
 	if (!master)
 		return -ENOMEM;
 
@@ -1216,7 +1230,9 @@ static int spi_imx_probe(struct platform_device *pdev)
 	spi_imx->bitbang.master->unprepare_message = spi_imx_unprepare_message;
 	spi_imx->bitbang.master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH;
 	if (is_imx35_cspi(spi_imx) || is_imx51_ecspi(spi_imx))
-		spi_imx->bitbang.master->mode_bits |= SPI_LOOP;
+		spi_imx->bitbang.master->mode_bits |= SPI_LOOP | SPI_READY;
+
+	spi_imx->spi_drctl = spi_drctl;
 
 	init_completion(&spi_imx->xfer_done);
 
-- 
2.12.2

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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

* Re: [PATCH V4 1/9] PM / OPP: Allow OPP table to be used for power-domains
From: Kevin Hilman @ 2017-04-23 22:07 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Sudeep Holla, Nishanth Menon, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-pm-u79uwXL29TY76Z2rM5mHXA, Viresh Kumar, Rafael Wysocki,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	linaro-kernel-cunTk1MwBs8s++Sfvej+rw,
	rnayak-sgV2jX0FEOL9JmXXK+q4OQ, Stephen Boyd,
	lina.iyer-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <20170420095248.GJ5436@vireshk-i7>

Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:

> On 20-04-17, 10:43, Sudeep Holla wrote:
>> Just that the term performance is closely related to frequency, it needs
>> to be explicit on what *exactly* it means. As it stands now,
>> it can be used for OPP as I explain which controls both but as you
>> clarify that's not what it's designed for.
>
> We are talking about active states of a power domain here and
> *performance* is the best word I got.
>
> And yes we can still have
> frequency as a configurable here, just that current platforms don't
> have it.

It's not that your platforms don't have frequency, it's just that it's
hidden by firmware on an M3 in your particular case.

DT is meant to model hardware, and surely what the M3 is managing is
frequency and/or voltage (IOW, an OPP).

The way I see it, the problem you're trying to solve is complicated just
because you don't know the exat freq and/or voltage because they are
hiddent behind the firware, and all you have control over is an index.  

What if you drop the introduction of the new domain-performance-state
and just stick with OPPs (and phandles to them), and for cases where you
don't know the exact freq/volage pairs, just use indexes and comment
what they refer to:

operating-points = <
	/*
         * NOTE: actual frequency and voltages are managed by
         * firmware and are hidden from HLOS, so we simply use index
         * here to select the OPP
         */
        1 1
	2 2
	3 3
>;

Since selecting the OPP is up to the power-domain driver implementation,
this should be fine, IMO.

This would mean just updating the doc to reflect the "relaxing" of these
fields to reflect indexes in cases where the exact freq/voltages are not
known.

IMO, this would be much simpler, as it avoids adding a new property and
continues to use teriminology that people are already familiar with
around OPPs.

Kevin

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/5] arm64: renesas: r8a7796: Add external audio clocks
From: Kuninori Morimoto @ 2017-04-23 23:56 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, Magnus Damm, Yoshihiro Shimoda, Rob Herring,
	Mark Rutland, linux-renesas-soc, devicetree, linux-arm-kernel
In-Reply-To: <1492779321-23939-2-git-send-email-geert+renesas@glider.be>


Hi Geert

> Add the external audio clocks as zero Hz fixed-frequency clocks.
> Boards that provide these clocks should override them.
> 
> Based on commit 623197b90c7aa97c ("arm64: renesas: r8a7795: Sound SSI
> PIO support").
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---

Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

>  arch/arm64/boot/dts/renesas/r8a7796.dtsi | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> index 2ec1ed5f499165ad..101cd41d693a7ab5 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
> @@ -120,6 +120,29 @@
>  		clock-frequency = <0>;
>  	};
>  
> +	/*
> +	 * The external audio clocks are configured as 0 Hz fixed frequency
> +	 * clocks by default.
> +	 * Boards that provide audio clocks should override them.
> +	 */
> +	audio_clk_a: audio_clk_a {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <0>;
> +	};
> +
> +	audio_clk_b: audio_clk_b {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <0>;
> +	};
> +
> +	audio_clk_c: audio_clk_c {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <0>;
> +	};
> +
>  	/* External CAN clock - to be overridden by boards that provide it */
>  	can_clk: can {
>  		compatible = "fixed-clock";
> -- 
> 2.7.4
> 

^ permalink raw reply

* [PATCH v3] ARM: dts: at91: sama5d2: add m_can nodes
From: Wenyou Yang @ 2017-04-24  1:12 UTC (permalink / raw)
  To: Nicolas.Ferre, Alexandre Belloni, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, Russell King
  Cc: devicetree, Wenyou Yang, Oliver Hartkopp, linux-kernel, linux-can,
	Quentin Schulz, Wenyou Yang, linux-arm-kernel

Add nodes to support the Controller Area Network(M_CAN) on SAMA5D2.
The version of M_CAN IP core is 3.1.0 (CREL = 0x31040730).

As said in SAMA5D2 datasheet, the CAN clock is recommended to use
frequencies of 20, 40 or 80 MHz. To achieve these frequencies,
PMC GCLK3 must select the UPLLCK(480 MHz) as source clock and
divide by 24, 12, or 6. So, the "assigned-clock-rates" property
has three options: 20000000, 40000000, and 80000000.
The "assigned-clock-parents" property should be referred to utmi
fixedly.

The MSBs [bits 31:16] of the CAN Message RAM for CAN0 and CAN1 are
default configured in 0x00200000. To avoid conflict with SRAM map
for PM, change them to 0x00210000 in the AT91Bootstrap via setting
the CAN Memories Address-based Register(SFR_CAN) of SFR.

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
Tested-by: Quentin Schulz <quentin.schulz@free-electrons.com>
---
The patch is tested on SAMA5D2 Xplained and based on the patch set,
 1. [PATCH v4 1/7] can: m_can: Disabled Interrupt Line 1
    http://marc.info/?l=linux-can&m=149165343604033&w=2

Changes in v3:
 - Add Tested-by tag.
 - Change the number of Rx Rx Buffers, Tx Buffers and Tx Event FIFO
   to maximum.

Changes in v2:
 - Configures 10 TX Event FIFO elements and 10 TX Buffers/FIFO slots,
   because the TXE FIFO is needed to be configured.
 - Configure the offset of Message RAM for CAN1 followed from CAN0's.

 arch/arm/boot/dts/at91-sama5d2_xplained.dts | 24 +++++++++++++
 arch/arm/boot/dts/sama5d2.dtsi              | 56 +++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/arch/arm/boot/dts/at91-sama5d2_xplained.dts b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
index 9f7f8a7d8ff9..2f19b08dc226 100644
--- a/arch/arm/boot/dts/at91-sama5d2_xplained.dts
+++ b/arch/arm/boot/dts/at91-sama5d2_xplained.dts
@@ -257,6 +257,12 @@
 				status = "okay";
 			};
 
+			can0: can@f8054000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&pinctrl_can0_default>;
+				status = "okay";
+			};
+
 			uart3: serial@fc008000 {
 				atmel,use-dma-rx;
 				atmel,use-dma-tx;
@@ -321,6 +327,18 @@
 					bias-disable;
 				};
 
+				pinctrl_can0_default: can0_default {
+					pinmux = <PIN_PC10__CANTX0>,
+						 <PIN_PC11__CANRX0>;
+					bias-disable;
+				};
+
+				pinctrl_can1_default: can1_default {
+					pinmux = <PIN_PC26__CANTX1>,
+						 <PIN_PC27__CANRX1>;
+					bias-disable;
+				};
+
 				pinctrl_charger_chglev: charger_chglev {
 					pinmux = <PIN_PA12__GPIO>;
 					bias-disable;
@@ -468,6 +486,12 @@
 				};
 
 			};
+
+			can1: can@fc050000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&pinctrl_can1_default>;
+				status = "okay";
+			};
 		};
 	};
 
diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index 22332be72140..7e00fa21373e 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -762,6 +762,18 @@
 						atmel,clk-output-range = <0 83000000>;
 					};
 
+					can0_clk: can0_clk {
+						#clock-cells = <0>;
+						reg = <56>;
+						atmel,clk-output-range = <0 83000000>;
+					};
+
+					can1_clk: can1_clk {
+						#clock-cells = <0>;
+						reg = <57>;
+						atmel,clk-output-range = <0 83000000>;
+					};
+
 					classd_clk: classd_clk {
 						#clock-cells = <0>;
 						reg = <59>;
@@ -890,6 +902,18 @@
 						#clock-cells = <0>;
 						reg = <55>;
 					};
+
+					can0_gclk: can0_gclk {
+						#clock-cells = <0>;
+						reg = <56>;
+						atmel,clk-output-range = <0 80000000>;
+					};
+
+					can1_gclk: can1_gclk {
+						#clock-cells = <0>;
+						reg = <57>;
+						atmel,clk-output-range = <0 80000000>;
+					};
 				};
 			};
 
@@ -1144,6 +1168,22 @@
 				clocks = <&clk32k>;
 			};
 
+			can0: can@f8054000 {
+				compatible = "bosch,m_can";
+				reg = <0xf8054000 0x4000>, <0x210000 0x4000>;
+				reg-names = "m_can", "message_ram";
+				interrupts = <56 IRQ_TYPE_LEVEL_HIGH 7>,
+					     <64 IRQ_TYPE_LEVEL_HIGH 7>;
+				interrupt-names = "int0", "int1";
+				clocks = <&can0_clk>, <&can0_gclk>;
+				clock-names = "hclk", "cclk";
+				assigned-clocks = <&can0_gclk>;
+				assigned-clock-parents = <&utmi>;
+				assigned-clock-rates = <40000000>;
+				bosch,mram-cfg = <0x0 0 0 64 0 0 32 32>;
+				status = "disabled";
+			};
+
 			spi1: spi@fc000000 {
 				compatible = "atmel,at91rm9200-spi";
 				reg = <0xfc000000 0x100>;
@@ -1305,6 +1345,22 @@
 				status = "okay";
 			};
 
+			can1: can@fc050000 {
+				compatible = "bosch,m_can";
+				reg = <0xfc050000 0x4000>, <0x210000 0x4000>;
+				reg-names = "m_can", "message_ram";
+				interrupts = <57 IRQ_TYPE_LEVEL_HIGH 7>,
+					     <65 IRQ_TYPE_LEVEL_HIGH 7>;
+				interrupt-names = "int0", "int1";
+				clocks = <&can1_clk>, <&can1_gclk>;
+				clock-names = "hclk", "cclk";
+				assigned-clocks = <&can1_gclk>;
+				assigned-clock-parents = <&utmi>;
+				assigned-clock-rates = <40000000>;
+				bosch,mram-cfg = <0x1100 0 0 64 0 0 32 32>;
+				status = "disabled";
+			};
+
 			chipid@fc069000 {
 				compatible = "atmel,sama5d2-chipid";
 				reg = <0xfc069000 0x8>;
-- 
2.11.0

^ permalink raw reply related

* 12224 devicetree
From: arywhite007-/E1597aS9LQAvxtiuMwx3w @ 2017-04-24  2:10 UTC (permalink / raw)
  To: devicetree-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: 9062578892.zip --]
[-- Type: application/zip, Size: 1691 bytes --]

^ 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

* [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

* [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 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 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 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 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 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

* 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

* 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/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: 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 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: 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: 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: 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: 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: [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/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] 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


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