* [PATCH] crypto: sun8i-ce - fix runtime pm imbalance on error
From: Dinghao Liu @ 2020-05-20 7:54 UTC (permalink / raw)
To: dinghao.liu, kjlu
Cc: Herbert Xu, Chen Zhou, linux-kernel, Maxime Ripard, Chen-Yu Tsai,
Corentin Labbe, linux-crypto, Colin Ian King, David S. Miller,
linux-arm-kernel
pm_runtime_get_sync() increments the runtime PM usage counter even
it returns an error code. Thus a pairing decrement is needed on
the error handling path to keep the counter balanced.
Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
---
drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
index 3e4e4bbda34c..18f0f251204c 100644
--- a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
+++ b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-core.c
@@ -598,8 +598,10 @@ static int sun8i_ce_probe(struct platform_device *pdev)
goto error_alg;
err = pm_runtime_get_sync(ce->dev);
- if (err < 0)
+ if (err < 0) {
+ pm_runtime_put_sync(ce->dev);
goto error_alg;
+ }
v = readl(ce->base + CE_CTR);
v >>= CE_DIE_ID_SHIFT;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH 2/2] perf/imx_ddr: Add stop counter support for i.MX8MP
From: Will Deacon @ 2020-05-20 7:51 UTC (permalink / raw)
To: Joakim Zhang
Cc: mark.rutland@arm.com, robin.murphy@arm.com, dl-linux-imx,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <DB8PR04MB67952EC95A5BE39C60F7F1FCE6D80@DB8PR04MB6795.eurprd04.prod.outlook.com>
On Thu, Apr 16, 2020 at 09:51:13AM +0000, Joakim Zhang wrote:
> Any comments about this issue? Thanks a lot!
You didn't really answer any of my questions, so I don't really know what to
do with this.
- The locking appears to be broken. Your solution was to remove it
entirely.
- It appears to be a user visible change and you haven't explained how it
continues to work with old userspace
- Perf core is not aware of you stopping counters and you haven't said why
that's not an issue.
While these issues are outstanding, I cannot merge the patch. Sorry.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH] ARM: dts: imx: Make tempmon node as child of anatop node
From: Anson Huang @ 2020-05-20 7:46 UTC (permalink / raw)
To: Aisheng Dong, robh+dt@kernel.org, shawnguo@kernel.org,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <AM6PR04MB49663B517C218072B2845DBF80B60@AM6PR04MB4966.eurprd04.prod.outlook.com>
> Subject: RE: [PATCH] ARM: dts: imx: Make tempmon node as child of anatop
> node
>
> > From: Anson Huang <Anson.Huang@nxp.com>
> > Sent: Wednesday, May 20, 2020 2:30 PM
> >
> > i.MX6/7 SoCs' temperature sensor is inside anatop module from HW
> > perspective, so it should be a child node of anatop.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>
> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
>
> BTW, I think you also need a binding doc for this change.
The binding doc is the imx-thermal.yaml I sent out, it is suggested by Rob
to move tempmon into anatop node, that is why I did this patch to align
with the binding doc.
Anson
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH] ARM: dts: imx: Make tempmon node as child of anatop node
From: Aisheng Dong @ 2020-05-20 7:43 UTC (permalink / raw)
To: Anson Huang, robh+dt@kernel.org, shawnguo@kernel.org,
s.hauer@pengutronix.de, kernel@pengutronix.de, festevam@gmail.com,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <1589956216-22499-1-git-send-email-Anson.Huang@nxp.com>
> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Wednesday, May 20, 2020 2:30 PM
>
> i.MX6/7 SoCs' temperature sensor is inside anatop module from HW perspective,
> so it should be a child node of anatop.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
BTW, I think you also need a binding doc for this change.
Regards
Aisheng
> ---
> arch/arm/boot/dts/imx6qdl.dtsi | 22 +++++++++++-----------
> arch/arm/boot/dts/imx6sl.dtsi | 20 ++++++++++----------
> arch/arm/boot/dts/imx6sll.dtsi | 20 ++++++++++----------
> arch/arm/boot/dts/imx6sx.dtsi | 20 ++++++++++----------
> arch/arm/boot/dts/imx6ul.dtsi | 20 ++++++++++----------
> arch/arm/boot/dts/imx7s.dtsi | 20 ++++++++++----------
> 6 files changed, 61 insertions(+), 61 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 39d4afd..43d44d5 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -69,17 +69,6 @@
> };
> };
>
> - tempmon: tempmon {
> - compatible = "fsl,imx6q-tempmon";
> - interrupt-parent = <&gpc>;
> - interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>;
> - fsl,tempmon = <&anatop>;
> - nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
> - nvmem-cell-names = "calib", "temp_grade";
> - clocks = <&clks IMX6QDL_CLK_PLL3_USB_OTG>;
> - #thermal-sensor-cells = <0>;
> - };
> -
> ldb: ldb {
> #address-cells = <1>;
> #size-cells = <0>;
> @@ -795,6 +784,17 @@
> anatop-min-voltage = <725000>;
> anatop-max-voltage = <1450000>;
> };
> +
> + tempmon: tempmon {
> + compatible = "fsl,imx6q-tempmon";
> + interrupt-parent = <&gpc>;
> + interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>;
> + fsl,tempmon = <&anatop>;
> + nvmem-cells = <&tempmon_calib>,
> <&tempmon_temp_grade>;
> + nvmem-cell-names = "calib", "temp_grade";
> + clocks = <&clks IMX6QDL_CLK_PLL3_USB_OTG>;
> + #thermal-sensor-cells = <0>;
> + };
> };
>
> usbphy1: usbphy@20c9000 {
> diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
> index 911d8cf..d8efc0a 100644
> --- a/arch/arm/boot/dts/imx6sl.dtsi
> +++ b/arch/arm/boot/dts/imx6sl.dtsi
> @@ -93,16 +93,6 @@
> };
> };
>
> - tempmon: tempmon {
> - compatible = "fsl,imx6q-tempmon";
> - interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>;
> - interrupt-parent = <&gpc>;
> - fsl,tempmon = <&anatop>;
> - nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
> - nvmem-cell-names = "calib", "temp_grade";
> - clocks = <&clks IMX6SL_CLK_PLL3_USB_OTG>;
> - };
> -
> pmu {
> compatible = "arm,cortex-a9-pmu";
> interrupt-parent = <&gpc>;
> @@ -628,6 +618,16 @@
> anatop-min-voltage = <725000>;
> anatop-max-voltage = <1450000>;
> };
> +
> + tempmon: tempmon {
> + compatible = "fsl,imx6q-tempmon";
> + interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-parent = <&gpc>;
> + fsl,tempmon = <&anatop>;
> + nvmem-cells = <&tempmon_calib>,
> <&tempmon_temp_grade>;
> + nvmem-cell-names = "calib", "temp_grade";
> + clocks = <&clks IMX6SL_CLK_PLL3_USB_OTG>;
> + };
> };
>
> usbphy1: usbphy@20c9000 {
> diff --git a/arch/arm/boot/dts/imx6sll.dtsi b/arch/arm/boot/dts/imx6sll.dtsi
> index edd3abb..bf7f048 100644
> --- a/arch/arm/boot/dts/imx6sll.dtsi
> +++ b/arch/arm/boot/dts/imx6sll.dtsi
> @@ -105,16 +105,6 @@
> clock-output-names = "ipp_di1";
> };
>
> - tempmon: temperature-sensor {
> - compatible = "fsl,imx6sll-tempmon", "fsl,imx6sx-tempmon";
> - interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> - interrupt-parent = <&gpc>;
> - fsl,tempmon = <&anatop>;
> - nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
> - nvmem-cell-names = "calib", "temp_grade";
> - clocks = <&clks IMX6SLL_CLK_PLL3_USB_OTG>;
> - };
> -
> soc {
> #address-cells = <1>;
> #size-cells = <1>;
> @@ -531,6 +521,16 @@
> anatop-max-voltage = <3400000>;
> anatop-enable-bit = <0>;
> };
> +
> + tempmon: temperature-sensor {
> + compatible = "fsl,imx6sll-tempmon",
> "fsl,imx6sx-tempmon";
> + interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-parent = <&gpc>;
> + fsl,tempmon = <&anatop>;
> + nvmem-cells = <&tempmon_calib>,
> <&tempmon_temp_grade>;
> + nvmem-cell-names = "calib", "temp_grade";
> + clocks = <&clks IMX6SLL_CLK_PLL3_USB_OTG>;
> + };
> };
>
> usbphy1: usb-phy@20c9000 {
> diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
> index e031337..8c4473b 100644
> --- a/arch/arm/boot/dts/imx6sx.dtsi
> +++ b/arch/arm/boot/dts/imx6sx.dtsi
> @@ -134,16 +134,6 @@
> clock-output-names = "anaclk2";
> };
>
> - tempmon: tempmon {
> - compatible = "fsl,imx6sx-tempmon", "fsl,imx6q-tempmon";
> - interrupt-parent = <&gpc>;
> - interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> - fsl,tempmon = <&anatop>;
> - nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
> - nvmem-cell-names = "calib", "temp_grade";
> - clocks = <&clks IMX6SX_CLK_PLL3_USB_OTG>;
> - };
> -
> pmu {
> compatible = "arm,cortex-a9-pmu";
> interrupt-parent = <&gpc>;
> @@ -696,6 +686,16 @@
> anatop-min-voltage = <725000>;
> anatop-max-voltage = <1450000>;
> };
> +
> + tempmon: tempmon {
> + compatible = "fsl,imx6sx-tempmon",
> "fsl,imx6q-tempmon";
> + interrupt-parent = <&gpc>;
> + interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> + fsl,tempmon = <&anatop>;
> + nvmem-cells = <&tempmon_calib>,
> <&tempmon_temp_grade>;
> + nvmem-cell-names = "calib", "temp_grade";
> + clocks = <&clks IMX6SX_CLK_PLL3_USB_OTG>;
> + };
> };
>
> usbphy1: usbphy@20c9000 {
> diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
> index 35e7301..505fd4f 100644
> --- a/arch/arm/boot/dts/imx6ul.dtsi
> +++ b/arch/arm/boot/dts/imx6ul.dtsi
> @@ -131,16 +131,6 @@
> clock-output-names = "ipp_di1";
> };
>
> - tempmon: tempmon {
> - compatible = "fsl,imx6ul-tempmon", "fsl,imx6sx-tempmon";
> - interrupt-parent = <&gpc>;
> - interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> - fsl,tempmon = <&anatop>;
> - nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
> - nvmem-cell-names = "calib", "temp_grade";
> - clocks = <&clks IMX6UL_CLK_PLL3_USB_OTG>;
> - };
> -
> pmu {
> compatible = "arm,cortex-a7-pmu";
> interrupt-parent = <&gpc>;
> @@ -611,6 +601,16 @@
> anatop-min-voltage = <725000>;
> anatop-max-voltage = <1450000>;
> };
> +
> + tempmon: tempmon {
> + compatible = "fsl,imx6ul-tempmon",
> "fsl,imx6sx-tempmon";
> + interrupt-parent = <&gpc>;
> + interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> + fsl,tempmon = <&anatop>;
> + nvmem-cells = <&tempmon_calib>,
> <&tempmon_temp_grade>;
> + nvmem-cell-names = "calib", "temp_grade";
> + clocks = <&clks IMX6UL_CLK_PLL3_USB_OTG>;
> + };
> };
>
> usbphy1: usbphy@20c9000 {
> diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi index
> 8bac491..3904558 100644
> --- a/arch/arm/boot/dts/imx7s.dtsi
> +++ b/arch/arm/boot/dts/imx7s.dtsi
> @@ -147,16 +147,6 @@
> };
> };
>
> - tempmon: tempmon {
> - compatible = "fsl,imx7d-tempmon";
> - interrupt-parent = <&gpc>;
> - interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> - fsl,tempmon = <&anatop>;
> - nvmem-cells = <&tempmon_calib>, <&fuse_grade>;
> - nvmem-cell-names = "calib", "temp_grade";
> - clocks = <&clks IMX7D_PLL_SYS_MAIN_CLK>;
> - };
> -
> timer {
> compatible = "arm,armv7-timer";
> interrupt-parent = <&intc>;
> @@ -586,6 +576,16 @@
> anatop-max-voltage = <1300000>;
> anatop-enable-bit = <0>;
> };
> +
> + tempmon: tempmon {
> + compatible = "fsl,imx7d-tempmon";
> + interrupt-parent = <&gpc>;
> + interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> + fsl,tempmon = <&anatop>;
> + nvmem-cells = <&tempmon_calib>,
> <&fuse_grade>;
> + nvmem-cell-names = "calib", "temp_grade";
> + clocks = <&clks IMX7D_PLL_SYS_MAIN_CLK>;
> + };
> };
>
> snvs: snvs@30370000 {
> --
> 2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH V2] dt-bindings: thermal: Convert i.MX to json-schema
From: Aisheng Dong @ 2020-05-20 7:42 UTC (permalink / raw)
To: Anson Huang, rui.zhang@intel.com, daniel.lezcano@linaro.org,
amit.kucheria@verdurent.com, robh+dt@kernel.org,
shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <1589954560-10810-1-git-send-email-Anson.Huang@nxp.com>
> From: Anson Huang <Anson.Huang@nxp.com>
> Sent: Wednesday, May 20, 2020 2:03 PM
>
> Convert the i.MX thermal binding to DT schema format using json-schema
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V1:
> - move tempmon node into its parent node anatop in example;
> - improve "fsl,tempmon" description.
> ---
> .../devicetree/bindings/thermal/imx-thermal.txt | 61 -------------
> .../devicetree/bindings/thermal/imx-thermal.yaml | 100
[...]
> +title: NXP i.MX Thermal Binding
> +
> +maintainers:
> + - Shawn Guo <shawn.guo@linaro.org>
> + - Anson Huang <Anson.Huang@nxp.com>
> +
> +properties:
> + compatible:
> + oneOf:
> + - items:
Drop Unnecessary properties
> + - enum:
> + - fsl,imx6q-tempmon
> + - fsl,imx6sx-tempmon
> + - fsl,imx7d-tempmon
> +
> + interrupts:
> + description: |
> + The interrupt output of the controller, the IRQ will be triggered
> + when temperature is higher than high threshold.
> + maxItems: 1
You'd better explain why interrupts number is changed in the new binding compared to
the original one. Probably add in commit message if really needed.
> +
> + nvmem-cells:
> + description: |
> + Phandle to the calibration cells provided by ocotp for calibration
> + data and temperature grade.
Better describe for each of them as you did for clocks
> + maxItems: 2
> +
> + nvmem-cell-names:
> + maxItems: 2
> + items:
> + - const: calib
> + - const: temp_grade
> +
> + fsl,tempmon:
> + $ref: '/schemas/types.yaml#/definitions/phandle'
> + description: Phandle to the register map node.
What register map? A bit ambiguous..
Regards
Aisheng
> +
> + fsl,tempmon-data:
> + $ref: '/schemas/types.yaml#/definitions/phandle'
> + description: |
> + Deprecated property, phandle pointer to fuse controller that contains
> + TEMPMON calibration data, e.g. OCOTP on imx6q. The details about
> + calibration data can be found in SoC Reference Manual.
> + deprecated: true
> +
> + clocks:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - interrupts
> + - fsl,tempmon
> + - nvmem-cells
> + - nvmem-cell-names
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/imx6sx-clock.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + efuse@21bc000 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "fsl,imx6sx-ocotp", "syscon";
> + reg = <0x021bc000 0x4000>;
> + clocks = <&clks IMX6SX_CLK_OCOTP>;
> +
> + tempmon_calib: calib@38 {
> + reg = <0x38 4>;
> + };
> +
> + tempmon_temp_grade: temp-grade@20 {
> + reg = <0x20 4>;
> + };
> + };
> +
> + anatop@20c8000 {
> + compatible = "fsl,imx6q-anatop", "syscon", "simple-mfd";
> + reg = <0x020c8000 0x1000>;
> + interrupts = <0 49 IRQ_TYPE_LEVEL_HIGH>,
> + <0 54 IRQ_TYPE_LEVEL_HIGH>,
> + <0 127 IRQ_TYPE_LEVEL_HIGH>;
> +
> + tempmon {
> + compatible = "fsl,imx6sx-tempmon";
> + interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
> + fsl,tempmon = <&anatop>;
> + nvmem-cells = <&tempmon_calib>,
> <&tempmon_temp_grade>;
> + nvmem-cell-names = "calib", "temp_grade";
> + clocks = <&clks IMX6SX_CLK_PLL3_USB_OTG>;
> + };
> + };
> --
> 2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH V2 2/3] perf/imx_ddr: Add system PMU identifier for userspace
From: Will Deacon @ 2020-05-20 7:41 UTC (permalink / raw)
To: Joakim Zhang
Cc: mark.rutland, devicetree, john.garry, linux-kernel, robh+dt,
linux-imx, shawnguo, linux-arm-kernel
In-Reply-To: <20200520025619.687-3-qiangqing.zhang@nxp.com>
On Wed, May 20, 2020 at 10:56:18AM +0800, Joakim Zhang wrote:
> The DDR Perf for i.MX8 is a system PMU whose axi id would different from
> SoC to SoC. Need expose system PMU identifier for userspace which refer
> to /sys/bus/event_source/devices/<PMU DEVICE>/identifier.
>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
> drivers/perf/fsl_imx8_ddr_perf.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
> index 90884d14f95f..ba523a94f4d7 100644
> --- a/drivers/perf/fsl_imx8_ddr_perf.c
> +++ b/drivers/perf/fsl_imx8_ddr_perf.c
> @@ -76,6 +76,7 @@ struct ddr_pmu {
> unsigned int cpu;
> struct hlist_node node;
> struct device *dev;
> + const char *identifier;
> struct perf_event *events[NUM_COUNTERS];
> int active_events;
> enum cpuhp_state cpuhp_state;
> @@ -84,6 +85,27 @@ struct ddr_pmu {
> int id;
> };
>
> +static ssize_t ddr_perf_identifier_show(struct device *dev,
> + struct device_attribute *attr,
> + char *page)
> +{
> + struct ddr_pmu *pmu = dev_get_drvdata(dev);
> +
> + return sprintf(page, "%s\n", pmu->identifier);
> +}
> +
> +static struct device_attribute ddr_perf_identifier_attr =
> + __ATTR(identifier, 0444, ddr_perf_identifier_show, NULL);
> +
> +static struct attribute *ddr_perf_identifier_attrs[] = {
> + &ddr_perf_identifier_attr.attr,
> + NULL,
> +};
> +
> +static struct attribute_group ddr_perf_identifier_attr_group = {
> + .attrs = ddr_perf_identifier_attrs,
> +};
> +
> enum ddr_perf_filter_capabilities {
> PERF_CAP_AXI_ID_FILTER = 0,
> PERF_CAP_AXI_ID_FILTER_ENHANCED,
> @@ -237,6 +259,7 @@ static const struct attribute_group *attr_groups[] = {
> &ddr_perf_format_attr_group,
> &ddr_perf_cpumask_attr_group,
> &ddr_perf_filter_cap_attr_group,
> + &ddr_perf_identifier_attr_group,
> NULL,
> };
>
> @@ -601,6 +624,7 @@ static int ddr_perf_probe(struct platform_device *pdev)
> struct ddr_pmu *pmu;
> struct device_node *np;
> void __iomem *base;
> + const char *identifier = NULL;
> char *name;
> int num;
> int ret;
> @@ -620,6 +644,11 @@ static int ddr_perf_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, pmu);
>
> + ret = of_property_read_string(np, "identifier", &identifier);
> + if (ret < 0)
> + dev_warn(&pdev->dev, "Failed to get identifier\n");
> + pmu->identifier = identifier;
I think this is exactly what Rob was objecting to when he said "yet another
way to identify the SoC from userspace". I've asked him on the other thread
as to what the best way to do this is.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 5/5] scsi: ufs: Fix possible VCC power drain during runtime suspend
From: Stanley Chu @ 2020-05-20 7:23 UTC (permalink / raw)
To: Asutosh Das (asd)
Cc: linux-scsi, martin.petersen, andy.teng, jejb, chun-hung.wu,
kuohong.wang, linux-kernel, avri.altman, cang, linux-mediatek,
peter.wang, alim.akhtar, matthias.bgg, bvanassche,
linux-arm-kernel, beanhuo
In-Reply-To: <6d32fba1-f7c3-f043-42b6-0da065e9795b@codeaurora.org>
Hi, Asutosh,
Thanks for your review.
On Tue, 2020-05-19 at 09:27 -0700, Asutosh Das (asd) wrote:
> Hi Stanley,
>
> On 5/16/2020 10:46 AM, Stanley Chu wrote:
> > The commit "scsi: ufs: Fix WriteBooster flush during runtime
> > suspend" promises essential resource, i.e., for UFS devices doing
> > WriteBooster buffer flush and Auto BKOPs. However if device
> > finishes its job but not resumed for a very long time, system
> > will have unnecessary power drain because VCC is still supplied.
> >
> > To fix this, a method to recheck the threshold of keeping VCC
> > supply is required. However, the threshold recheck needs to
> > re-activate the link because the decision depends on the device
> > status.
> >
> > Introduce a delayed work to force runtime resume after a certain
> > delay during runtime suspend. This makes threshold recheck simpler
> > which will be done in the next runtime-suspend.
> >
> > Signed-off-by: Stanley Chu <stanley.chu@mediatek.com>
> > ---
>
> Is there a reason to have this code as a separate patch?
> [1] Commit: "scsi: ufs: Fix WriteBooster flush during runtime suspend"
> introduces 'keep_curr_dev_pwr_mode' and the very next change (this one)
> removes it.
> Do you think this change and [1] should be merged?
Yes, these 2 patches shall be merged. I will do it in next version.
>
> > drivers/scsi/ufs/ufs.h | 1 +
> > drivers/scsi/ufs/ufshcd.c | 43 ++++++++++++++++++++++++++++++++++-----
> > drivers/scsi/ufs/ufshcd.h | 1 +
> > 3 files changed, 40 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
> > index db07eedfed96..c70845d41449 100644
> > --- a/drivers/scsi/ufs/ufs.h
> > +++ b/drivers/scsi/ufs/ufs.h
> > @@ -574,6 +574,7 @@ struct ufs_dev_info {
> > u32 d_ext_ufs_feature_sup;
> > u8 b_wb_buffer_type;
> > u32 d_wb_alloc_units;
> > + bool b_rpm_dev_flush_capable;
> > u8 b_presrv_uspc_en;
> > };
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index f4f2c7b5ab0a..a137553f9b41 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -94,6 +94,9 @@
> > /* default delay of autosuspend: 2000 ms */
> > #define RPM_AUTOSUSPEND_DELAY_MS 2000
> >
> > +/* Default delay of RPM device flush delayed work */
> > +#define RPM_DEV_FLUSH_RECHECK_WORK_DELAY_MS 5000
> > +
> > /* Default value of wait time before gating device ref clock */
> > #define UFSHCD_REF_CLK_GATING_WAIT_US 0xFF /* microsecs */
> >
> > @@ -5310,7 +5313,7 @@ static bool ufshcd_wb_presrv_usrspc_keep_vcc_on(struct ufs_hba *hba,
> > return false;
> > }
> >
> > -static bool ufshcd_wb_keep_vcc_on(struct ufs_hba *hba)
> > +static bool ufshcd_wb_need_flush(struct ufs_hba *hba)
> > {
> > int ret;
> > u32 avail_buf;
> > @@ -5348,6 +5351,21 @@ static bool ufshcd_wb_keep_vcc_on(struct ufs_hba *hba)
> > return ufshcd_wb_presrv_usrspc_keep_vcc_on(hba, avail_buf);
> > }
> >
> > +static void ufshcd_rpm_dev_flush_recheck_work(struct work_struct *work)
> > +{
> > + struct ufs_hba *hba = container_of(to_delayed_work(work),
> > + struct ufs_hba,
> > + rpm_dev_flush_recheck_work);
> > + /*
> > + * To prevent unnecessary VCC power drain after device finishes
> > + * WriteBooster buffer flush or Auto BKOPs, force runtime resume
> > + * after a certain delay to recheck the threshold by next runtime
> > + * supsend.
> > + */
> > + pm_runtime_get_sync(hba->dev);
> > + pm_runtime_put_sync(hba->dev);
> > +}
> > +
> > /**
> > * ufshcd_exception_event_handler - handle exceptions raised by device
> > * @work: pointer to work data
> > @@ -8164,7 +8182,6 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> > enum ufs_pm_level pm_lvl;
> > enum ufs_dev_pwr_mode req_dev_pwr_mode;
> > enum uic_link_state req_link_state;
> > - bool keep_curr_dev_pwr_mode = false;
> >
> > hba->pm_op_in_progress = 1;
> > if (!ufshcd_is_shutdown_pm(pm_op)) {
> > @@ -8224,11 +8241,12 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> > * Hibern8, keep device power mode as "active power mode"
> > * and VCC supply.
> > */
> > - keep_curr_dev_pwr_mode = hba->auto_bkops_enabled ||
> > + hba->dev_info.b_rpm_dev_flush_capable =
> > + hba->auto_bkops_enabled ||
> > (((req_link_state == UIC_LINK_HIBERN8_STATE) ||
> > ((req_link_state == UIC_LINK_ACTIVE_STATE) &&
> > ufshcd_is_auto_hibern8_enabled(hba))) &&
> > - ufshcd_wb_keep_vcc_on(hba));
> > + ufshcd_wb_need_flush(hba));
> > }
> >
> > if (req_dev_pwr_mode != hba->curr_dev_pwr_mode) {
> > @@ -8238,7 +8256,7 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> > ufshcd_disable_auto_bkops(hba);
> > }
> >
> > - if (!keep_curr_dev_pwr_mode) {
> > + if (!hba->dev_info.b_rpm_dev_flush_capable) {
> > ret = ufshcd_set_dev_pwr_mode(hba, req_dev_pwr_mode);
> > if (ret)
> > goto enable_gating;
> > @@ -8295,9 +8313,16 @@ static int ufshcd_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> > if (hba->clk_scaling.is_allowed)
> > ufshcd_resume_clkscaling(hba);
> > hba->clk_gating.is_suspended = false;
> > + hba->dev_info.b_rpm_dev_flush_capable = false;
> > ufshcd_release(hba);
> > out:
> > + if (hba->dev_info.b_rpm_dev_flush_capable) {
> > + schedule_delayed_work(&hba->rpm_dev_flush_recheck_work,
> > + msecs_to_jiffies(RPM_DEV_FLUSH_RECHECK_WORK_DELAY_MS));
> > + }
> > +
> > hba->pm_op_in_progress = 0;
> > +
> Nitpick; newline, perhaps?
Thanks, I Will remove it.
>
> > if (ret)
> > ufshcd_update_reg_hist(&hba->ufs_stats.suspend_err, (u32)ret);
> > return ret;
> > @@ -8386,6 +8411,11 @@ static int ufshcd_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op)
> > /* Enable Auto-Hibernate if configured */
> > ufshcd_auto_hibern8_enable(hba);
> >
> > + if (hba->dev_info.b_rpm_dev_flush_capable) {
> > + hba->dev_info.b_rpm_dev_flush_capable = false;
> > + cancel_delayed_work(&hba->rpm_dev_flush_recheck_work);
> > + }
> > +
> > /* Schedule clock gating in case of no access to UFS device yet */
> > ufshcd_release(hba);
> >
> > @@ -8859,6 +8889,9 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq)
> > UFS_SLEEP_PWR_MODE,
> > UIC_LINK_HIBERN8_STATE);
> >
> > + INIT_DELAYED_WORK(&hba->rpm_dev_flush_recheck_work,
> > + ufshcd_rpm_dev_flush_recheck_work);
> > +
> > /* Set the default auto-hiberate idle timer value to 150 ms */
> > if (ufshcd_is_auto_hibern8_supported(hba) && !hba->ahit) {
> > hba->ahit = FIELD_PREP(UFSHCI_AHIBERN8_TIMER_MASK, 150) |
> > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
> > index 8db7a6101892..9acd437037e8 100644
> > --- a/drivers/scsi/ufs/ufshcd.h
> > +++ b/drivers/scsi/ufs/ufshcd.h
> > @@ -745,6 +745,7 @@ struct ufs_hba {
> > struct request_queue *bsg_queue;
> > bool wb_buf_flush_enabled;
> > bool wb_enabled;
> > + struct delayed_work rpm_dev_flush_recheck_work;
> > };
> >
> > /* Returns true if clocks can be gated. Otherwise false */
> >
>
>
Thanks,
Stanley Chu
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] dt-bindings: input: touchscreen: edt-ft5x06: change reg property
From: Johan Jonker @ 2020-05-20 7:33 UTC (permalink / raw)
To: heiko
Cc: devicetree, dmitry.torokhov, linux-kernel, linux-rockchip,
robh+dt, linux-input, linux-arm-kernel
A test with the command below gives this error:
arch/arm/boot/dts/rk3188-bqedison2qc.dt.yaml:
touchscreen@3e: reg:0:0: 56 was expected
The touchscreen chip on 'rk3188-bqedison2qc' and other BQ models
was shipped with different addresses then the binding currently allows.
Change the reg property that any address will pass.
make ARCH=arm dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/input/touchscreen/
edt-ft5x06.yaml
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
index 383d64a91..baa8e8f7e 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
@@ -42,7 +42,7 @@ properties:
- focaltech,ft6236
reg:
- const: 0x38
+ maxItems: 1
interrupts:
maxItems: 1
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH V1 RESEND 1/3] perf/imx_ddr: Add system PMU identifier for userspace
From: Will Deacon @ 2020-05-20 7:33 UTC (permalink / raw)
To: Rob Herring
Cc: mark.rutland, devicetree, john.garry, Joakim Zhang, linux-kernel,
linux-imx, shawnguo, linux-arm-kernel
In-Reply-To: <20200519185125.GB453195@bogus>
On Tue, May 19, 2020 at 12:51:25PM -0600, Rob Herring wrote:
> On Tue, May 12, 2020 at 03:31:13PM +0800, Joakim Zhang wrote:
> > +static ssize_t ddr_perf_identifier_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *page)
> > +{
> > + struct ddr_pmu *pmu = dev_get_drvdata(dev);
> > +
> > + return sprintf(page, "%s\n", pmu->devtype_data->identifier);
>
> Why do we need yet another way to identify the SoC from userspace?
I also really dislike this. What's the preferred way to identify the SoC
from userspace? It's needed so that the perf userspace tool can describe
perf events that are supported for the PMU, as this isn't probe-able
directly from the hardware. We have the same issue with the SMMUv3 PMCG [1],
and so we need to solve the problem for both DT and ACPI.
Will
[1] https://lore.kernel.org/r/1587120634-19666-1-git-send-email-john.garry@huawei.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 07/15] PCI: brcmstb: Add control of rescal reset
From: Philipp Zabel @ 2020-05-20 7:27 UTC (permalink / raw)
To: Jim Quinlan
Cc: moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Rob Herring, Lorenzo Pieralisi,
open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS, open list,
Florian Fainelli, maintainer:BROADCOM BCM7XXX ARM ARCHITECTURE,
moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE,
Bjorn Helgaas, Nicolas Saenz Julienne
In-Reply-To: <20200519203419.12369-8-james.quinlan@broadcom.com>
Hi Jim,
On Tue, May 19, 2020 at 04:34:05PM -0400, Jim Quinlan wrote:
> From: Jim Quinlan <jquinlan@broadcom.com>
>
> Some STB chips have a special purpose reset controller named
> RESCAL (reset calibration). This commit adds the control
> of RESCAL as well as the ability to start and stop its
> operation for PCIe HW.
>
> Signed-off-by: Jim Quinlan <jquinlan@broadcom.com>
> ---
> drivers/pci/controller/pcie-brcmstb.c | 81 ++++++++++++++++++++++++++-
> 1 file changed, 80 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 2c470104ba38..0787e8f6f7e5 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
[...]
> @@ -1100,6 +1164,21 @@ static int brcm_pcie_probe(struct platform_device *pdev)
> dev_err(&pdev->dev, "could not enable clock\n");
> return ret;
> }
> + pcie->rescal = devm_reset_control_get_shared(&pdev->dev, "rescal");
> + if (IS_ERR(pcie->rescal)) {
> + if (PTR_ERR(pcie->rescal) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + pcie->rescal = NULL;
This is effectively an optional reset control, so it is better to use:
↵
pcie->rescal = devm_reset_control_get_optional_shared(&pdev->dev,
"rescal");↵
if (IS_ERR(pcie->rescal))
return PTR_ERR(pcie->rescal);
> + } else {
> + ret = reset_control_deassert(pcie->rescal);
> + if (ret)
> + dev_err(&pdev->dev, "failed to deassert 'rescal'\n");
> + }
reset_control_* can handle rstc == NULL parameters for optional reset
controls, so this can be done unconditionally:
ret = reset_control_deassert(pcie->rescal);↵
if (ret)↵
dev_err(&pdev->dev, "failed to deassert 'rescal'\n");↵
Is rescal handled by the reset-brcmstb-rescal driver? Since that only
implements the .reset op, I would expect reset_control_reset() here.
Otherwise this looks like it'd be missing a reset_control_assert in
remove.
regards
Philipp
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [Linux-stm32] [RFC 0/3] Introduce cpufreq minimum load QoS
From: Benjamin GAIGNARD @ 2020-05-20 7:29 UTC (permalink / raw)
To: Valentin Schneider
Cc: len.brown@intel.com, mcoquelin.stm32@gmail.com,
linux-pm@vger.kernel.org, viresh.kumar@linaro.org,
Rafael J. Wysocki, Rafael J. Wysocki,
linux-kernel@vger.kernel.org, Patrick Bellasi, pavel@ucw.cz,
mchehab@kernel.org, linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <b8757472-c973-a32d-d5c9-a584d7d703f8@st.com>
On 5/4/20 11:17 AM, Benjamin GAIGNARD wrote:
>
> On 4/30/20 5:50 PM, Valentin Schneider wrote:
>> On 30/04/20 16:37, Benjamin GAIGNARD wrote:
>>> On 4/30/20 4:33 PM, Valentin Schneider wrote:
>>>> On 30/04/20 14:46, Benjamin GAIGNARD wrote:
>>>>>> That's not what I meant.
>>>>>>
>>>>>> I suppose that the interrupt processing in question takes place in
>>>>>> process context and so you may set the lower clamp on the utilization
>>>>>> of the task carrying that out.
>>>>> I have try to add this code when starting streaming (before the first
>>>>> interrupt) the frames from the sensor:
>>>>> const struct sched_attr sched_attr = {
>>>>> .sched_util_min = 10000, /* 100% of usage */
>>>> Unless you play with SCHED_CAPACITY_SHIFT, the max should be 1024 -
>>>> i.e. SCHED_CAPACITY_SCALE. That's a really big boost, but that's for you to
>>>> benchmark.
>>>>
>>>>> .sched_flags = SCHED_FLAG_UTIL_CLAMP_MIN,
>>>>> };
>>>>>
>>>>> sched_setattr(current, &sched_attr);
>>>>>
>>>>> I don't see any benefices maybe there is some configuration flags to set.
>>>>>
>>>>> How changing sched_util_min could impact cpufreq ondemand governor ?
>>>>> Does it change the value returned when the governor check the idle time ?
>>>>>
>>>> You'll have to use the schedutil governor for uclamp to have an effect. And
>>>> arguably that's what you should be using, unless something explicitly
>>>> prevents you from doing that.
>>> Even with schedutil and SCHED_CAPACITY_SCALE that it doesn't work.
>>> cpufreq/cpuinfo_cur_freq values are always on the max value even if the
>>> stats show transitions between the available frequencies.
>>>
>>> I see two possibles reasons to explain that:
>>> - sched_setattr() is called in userland process context, but the
>>> threaded irq handler is running in another process.
>> Ah yes, this only works if the task you boost is the one that will handle
>> whatever work you care about (in this case handling the irq). That said, if
>> you do use threaded IRQs, that should give you a SCHED_FIFO thread, which
>> should drive the frequency to its max when using schedutil (unrelated to
>> uclamp).
> Can I conclude that sched_setattr() isn't the good way to solve this
> problem ?
> Does my patches make sense in this case ?
Gentle up on this series beacause I haven't found any other way to solve
this problem.
Thanks,
Benjamin
>
>>> - because this use case is almost running all in hardware the process
>>> isn't doing anything so the scheduler doesn't take care of it.
>>>
>>>>>> Alternatively, that task may be a deadline one.
> _______________________________________________
> Linux-stm32 mailing list
> Linux-stm32@st-md-mailman.stormreply.com
> https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 00/12] Add cpufreq and cci devfreq for mt8183, and SVS support
From: andrew-sh.cheng @ 2020-05-20 7:10 UTC (permalink / raw)
To: Chanwoo Choi
Cc: Mark Rutland, Nishanth Menon, srv_heupstream, linux-pm,
Stephen Boyd, Viresh Kumar, Mark Brown, Rafael J . Wysocki,
Liam Girdwood, Rob Herring, linux-kernel, Kyungmin Park,
MyungJoo Ham, linux-mediatek, linux-arm-kernel, Matthias Brugger,
devicetree
In-Reply-To: <852af745-dfb7-6bb5-da7e-798d166a3370@samsung.com>
On Wed, 2020-05-20 at 15:24 +0900, Chanwoo Choi wrote:
> Hi,
>
> On 5/20/20 2:36 PM, andrew-sh.cheng wrote:
> > On Wed, 2020-05-20 at 13:10 +0900, Chanwoo Choi wrote:
> >> Hi Andrew,
> >>
> >> Could you explain the base commit of these patches?
> >> When I tried to apply them to v5.7-rc1 for testing,
> >> the merge conflict occurs.
> >>
> >> Thanks,
>
> >> Chanwoo Choi
> >
> > Hi Chanwoo Choi,
> >
> > My base commit is
> > commit 8f3d9f354286745c751374f5f1fcafee6b3f3136
> > Author: Linus Torvalds <torvalds@linux-foundation.org>
> > Date: Sun Apr 12 12:35:55 2020 -0700
> >
> > Linux 5.7-rc1
> >
> > Could you show me the conflict error?
>
>
> When I tried to apply first patch with 'git am',
> the merge conflict occurred.
>
> git am \[PATCH\ 01_12\]\ OPP\:\ Allow\ required-opps\ even\ if\ the\ device\ doesn\'t\ have\ power-domains.eml
> Applying: OPP: Allow required-opps even if the device doesn't have power-domains
> error: patch failed: drivers/opp/core.c:755
> error: drivers/opp/core.c: patch does not apply
> error: patch failed: drivers/opp/of.c:195
> error: drivers/opp/of.c: patch does not apply
> Patch failed at 0001 OPP: Allow required-opps even if the device doesn't have power-domains
> Use 'git am --show-current-patch' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> Regards,
> Chanwoo Choi
Hi Chanwoo,
I just make a new folder to get code and check.
Below is my command list.
Please help check the different with you.
505 repo init -u http://gerrit.mediatek.inc:8080/cros-kernel/manifest
-b upstream
506 repo sync -j8
507 repo start kern-dev --all
508 git remote add main
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
509 git remote add main
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
510 ls
511 cd kernel/mediatek/
512 git remote add main
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
513 git fetch main
514 git checkout v5.7-rc1
515 git am
Add-cpufreq-and-cci-devfreq-for-mt8183-and-SVS-support.patch
516 history
BR,
Andrew-sh.Cheng
>
> >
> > BR,
> > Andrew-sh.Cheng
> >>
> >> On 5/20/20 12:42 PM, Andrew-sh.Cheng wrote:
> >>> MT8183 supports CPU DVFS and CCI DVFS, and LITTLE cpus and CCI are in the same voltage domain.
> >>> So, this series is to add drivers to handle the voltage coupling between CPU and CCI DVFS.
> >>>
> >>> For SVS support, need OPP_EVENT_ADJUST_VOLTAGE and corresponding reaction.
> >>>
> >>> Change since v5:
> >>> - Changing dt-binding format to yaml.
> >>> - Extending current devfreq passive_governor instead of create a new one.
> >>> - Resend depending patches of Sravana Kannan base on kernel-5.7
> >>>
> >>>
> >>> Andrew-sh.Cheng (6):
> >>> cpufreq: mediatek: add clock and regulator enable for intermediate
> >>> clock
> >>> dt-bindings: devfreq: add compatible for mt8183 cci devfreq
> >>> devfreq: add mediatek cci devfreq
> >>> opp: Modify opp API, dev_pm_opp_get_freq(), find freq in opp, even it
> >>> is disabled
> >>> cpufreq: mediatek: add opp notification for SVS support
> >>> devfreq: mediatek: cci devfreq register opp notification for SVS
> >>> support
> >>>
> >>> Saravana Kannan (6):
> >>> OPP: Allow required-opps even if the device doesn't have power-domains
> >>> OPP: Add function to look up required OPP's for a given OPP
> >>> OPP: Improve required-opps linking
> >>> PM / devfreq: Cache OPP table reference in devfreq
> >>> PM / devfreq: Add required OPPs support to passive governor
> >>> PM / devfreq: Add cpu based scaling support to passive_governor
> >>>
> >>> .../devicetree/bindings/devfreq/mt8183-cci.yaml | 51 ++++
> >>> drivers/cpufreq/mediatek-cpufreq.c | 122 ++++++++-
> >>> drivers/devfreq/Kconfig | 12 +
> >>> drivers/devfreq/Makefile | 1 +
> >>> drivers/devfreq/devfreq.c | 6 +
> >>> drivers/devfreq/governor_passive.c | 298 +++++++++++++++++++--
> >>> drivers/devfreq/mt8183-cci-devfreq.c | 233 ++++++++++++++++
> >>> drivers/opp/core.c | 85 +++++-
> >>> drivers/opp/of.c | 108 ++++----
> >>> drivers/opp/opp.h | 5 +
> >>> include/linux/devfreq.h | 42 ++-
> >>> include/linux/pm_opp.h | 11 +
> >>> 12 files changed, 874 insertions(+), 100 deletions(-)
> >>> create mode 100644 Documentation/devicetree/bindings/devfreq/mt8183-cci.yaml
> >>> create mode 100644 drivers/devfreq/mt8183-cci-devfreq.c
> >>>
> >
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
From: Ard Biesheuvel @ 2020-05-20 7:09 UTC (permalink / raw)
To: Stephan Mueller; +Cc: Eric Biggers, Linux Crypto Mailing List, Linux ARM
In-Reply-To: <2010567.jSmZeKYv2B@tauon.chronox.de>
On Wed, 20 May 2020 at 09:01, Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Mittwoch, 20. Mai 2020, 08:54:10 CEST schrieb Ard Biesheuvel:
>
> Hi Ard,
>
> > On Wed, 20 May 2020 at 08:47, Stephan Mueller <smueller@chronox.de> wrote:
...
> > > The state of all block chaining modes we currently have is defined with
> > > the
> > > IV. That is the reason why I mentioned it can be implemented stateless
> > > when I am able to get the IV output from the previous operation.
> >
> > But it is simply the same as the penultimate block of ciphertext. So
> > you can simply capture it after encrypt, or before decrypt. There is
> > really no need to rely on the CTS transformation to pass it back to
> > you via the buffer that is only specified to provide an input to the
> > CTS transform.
>
> Let me recheck that as I am not fully sure on that one. But if it can be
> handled that way, it would make life easier.
Please refer to patch 2. The .iv_out test vectors were all simply
copied from the appropriate offset into the associated .ctext member.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 07/13] firmware: arm_scmi: Add notification dispatch and delivery
From: Cristian Marussi @ 2020-05-20 7:09 UTC (permalink / raw)
To: Lukasz Luba, linux-kernel, linux-arm-kernel
Cc: cristian.marussi, sudeep.holla
In-Reply-To: <20200313190224.GA5808@e120937-lin>
On Mon, Mar 16, 2020 at 02:46:05PM +0000, Cristian Marussi wrote:
> On Thu, Mar 12, 2020 at 09:43:31PM +0000, Lukasz Luba wrote:
> >
> >
Hi Lukasz,
I went back looking deeper into the possible race issue you pointed out a
while ago understanding it a bit better down below.
> > On 3/12/20 6:34 PM, Cristian Marussi wrote:
> > > On 12/03/2020 13:51, Lukasz Luba wrote:
> > > > Hi Cristian,
> > > >
> Hi Lukasz
>
> > > > just one comment below...
> [snip]
> > > > > + eh.timestamp = ts;
> > > > > + eh.evt_id = evt_id;
> > > > > + eh.payld_sz = len;
> > > > > + kfifo_in(&r_evt->proto->equeue.kfifo, &eh, sizeof(eh));
> > > > > + kfifo_in(&r_evt->proto->equeue.kfifo, buf, len);
> > > > > + queue_work(r_evt->proto->equeue.wq,
> > > > > + &r_evt->proto->equeue.notify_work);
> > > >
> > > > Is it safe to ignore the return value from the queue_work here?
> > > >
> > >
> > > In fact yes, we do not want to care: it returns true or false depending on the
> > > fact that the specific work was or not already queued, and we just rely on
> > > this behavior to keep kicking the worker only when needed but never kick
> > > more than one instance of it per-queue (so that there's only one reader
> > > wq and one writer here in the scmi_notify)...explaining better:
> > >
> > > 1. we push an event (hdr+payld) to the protocol queue if we found that there was
> > > enough space on the queue
> > >
> > > 2a. if at the time of the kfifo_in( ) the worker was already running
> > > (queue not empty) it will process our new event sooner or later and here
> > > the queue_work will return false, but we do not care in fact ... we
> > > tried to kick it just in case
> > >
> > > 2b. if instead at the time of the kfifo_in() the queue was empty the worker would
> > > have probably already gone to the sleep and this queue_work() will return true and
> > > so this time it will effectively wake up the worker to process our items
> > >
> > > The important thing here is that we are sure to wakeup the worker when needed
> > > but we are equally sure we are never causing the scheduling of more than one worker
> > > thread consuming from the same queue (because that would break the one reader/one writer
> > > assumption which let us use the fifo in a lockless manner): this is possible because
> > > queue_work checks if the required work item is already pending and in such a case backs
> > > out returning false and we have one work_item (notify_work) defined per-protocol and
> > > so per-queue.
> >
> > I see. That's a good assumption: one work_item per protocol and simplify
> > the locking. What if there would be an edge case scenario when the
> > consumer (work_item) has handled the last item (there was NULL from
> > scmi_process_event_header()), while in meantime scmi_notify put into
> > the fifo new event but couldn't kick the queue_work. Would it stay there
> > till the next IRQ which triggers queue_work to consume two events (one
> > potentially a bit old)? Or we can ignore such race situation assuming
> > that cleaning of work item is instant and kfifo_in is slow?
> >
>
> In fact, this is a very good point, since between the moment the worker
> determines that the queue is empty and the moment in which the worker
> effectively exits (and it's marked as no more pending by the Kernel cmwq)
> there is a window of opportunity for a race in which the ISR could fill
> the queue with one more event and then fail to kick with queue_work() since
> the work is in fact still nominally marked as pending from the point of view
> of Kernel cmwq, as below:
>
> ISR (core N) | WQ (core N+1) cmwq flags queued events
> ------------------------------------------------------------------------------------------------
> | if (queue_is_empty) - WORK_PENDING 0 events queued
> + ... - WORK_PENDING 0 events queued
> + } while (scmi_process_event_payload);
> +}// worker function exit
> kfifo_in() + ...cmwq backing out - WORK_PENDING 1 events queued
> kfifo_in() + ...cmwq backing out - WORK_PENDING 1 events queued
> queue_work() + ...cmwq backing out - WORK_PENDING 1 events queued
> -> FALSE (pending) + ...cmwq backing out - WORK_PENDING 1 events queued
> + ...cmwq backing out - WORK_PENDING 1 events queued
> + ...cmwq backing out - WORK_PENDING 1 events queued
> | ---- WORKER THREAD EXIT - !WORK_PENDING 1 events queued
> | - !WORK_PENDING 1 events queued
> kfifo_in() | - !WORK_PENDING 2 events queued
> kfifo_in() | - !WORK_PENDING 2 events queued
> queue_work() | - !WORK_PENDING 2 events queued
> -> TRUE | --- WORKER ENTER - WORK_PENDING 2 events queued
> | - WORK_PENDING 2 events consumed
>
> where effectively the last event queued won't be consumed till the next
> iteration once another event is queued.
>
In summary, looking better at Kernel cmwq code, my explanation above about
how the possible race could be exposed by a particular tricky limit condition
and the values assumed by the WORK_STRUCT_PENDING_BIT was ... bullshit :D
In fact there's no race at all because Kernel cmwq takes care to clear the above
PENDING flag BEFORE the user-provided worker-function starts to finally run:
such flag is active only when a work instance is queued pending for execution
but it is cleared just before execution effctively starts.
kernel/workqueue.c:process_one_work()
set_work_pool_and_clear_pending(work, pool->id);
....
worker->current_func(work);
As a consequence in the racy scenario above where the ISR pushes events on the
queues after the worker has already determined the queue to be empty but while
the worker func is still being deactivated in terms of Kernel cmwq internal
handling, it is not a problem since the worker while running is already NO more
marked pending so the queue_work succeeds and a new work will simply be queued
and run once the current instance terminates fully and it is removed from pool.
On the other side in the normal non racy scenario, when the worker is processing
normally a non-empty queue, we'll end-up anyway queueing new items and a new work
from the ISR even if the currently executing one will in fact consume already
naturally the queued items: this will result (it's what I observe in fact) in a
final un-needed quick worker activation/deactivation processing zero items (empty
queue) which is in fact harmless.
Basically the racy condition is taken care by the Kernel cmwq itself, and in fact
there is an extensive explanation also of the barriers employed to properly
realize this in the comments around set_work_pool_and_clear_pending()
I'll add a comment in v8 just to note this behaviour.
Thanks
Cristian
> Given the fact that the ISR and the dedicated WQ on an SMP run effectively
> in parallel I do not think unfortunately that we can simply count on the fact
> the worker exit is faster than the kifos_in, enough to close the race window
> opportunity. (even if rare)
>
> On the other side considering the impact of such scenario, I can imagine that
> it's not simply that we could only have a delayed delivery, but we must consider
> that if the delayed event is effectively the last one ever it would remain
> undelivered forever; this is particularly worrying in a scenario in which such
> last event is particularly important: imagine a system shutdown where a last
> system-power-off remains undelivered.
>
> As a consequence I think this rare racy condition should be addressed somehow.
>
> Looking at this scenario, it seems the classic situation in which you want to
> use some sort of completion to avoid missing out on events delivery, BUT in our
> usecase:
>
> - placing the workers loaned from cmwq into an unbounded wait_for_completion()
> once the queue is empty seems not the best to use resources (and probably
> frowned upon)....using a few dedicated kernel threads to simply let them idle
> waiting most of the time seems equally frowned upon (I could be wrong...))
> - the needed complete() in the ISR would introduce a spinlock_irqsave into the
> interrupt path (there's already one inside queue_work in fact) so it is not
> desirable, at least not if used on a regular base (for each event notified)
>
> So I was thinking to try to reduce sensibly the above race window, more
> than eliminate it completely, by adding an early flag to be checked under
> specific conditions in order to retry the queue_work a few times when the race
> is hit, something like:
>
> ISR (core N) | WQ (core N+1)
> -------------------------------------------------------------------------------
> | atomic_set(&exiting, 0);
> |
> | do {
> | ...
> | if (queue_is_empty) - WORK_PENDING 0 events queued
> + atomic_set(&exiting, 1) - WORK_PENDING 0 events queued
> static int cnt=3 | --> breakout of while - WORK_PENDING 0 events queued
> kfifo_in() | ....
> | } while (scmi_process_event_payload);
> kfifo_in() |
> exiting = atomic_read() | ...cmwq backing out - WORK_PENDING 1 events queued
> do { | ...cmwq backing out - WORK_PENDING 1 events queued
> ret = queue_work() | ...cmwq backing out - WORK_PENDING 1 events queued
> if (ret || !exiting)| ...cmwq backing out - WORK_PENDING 1 events queued
> break; | ...cmwq backing out - WORK_PENDING 1 events queued
> mdelay(5); | ...cmwq backing out - WORK_PENDING 1 events queued
> exiting = | ...cmwq backing out - WORK_PENDING 1 events queued
> atomic_read; | ...cmwq backing out - WORK_PENDING 1 events queued
> } while (--cnt); | ...cmwq backing out - WORK_PENDING 1 events queued
> | ---- WORKER EXIT - !WORK_PENDING 0 events queued
>
> like down below between the scissors.
>
> Not tested or tried....I could be missing something...and the mdelay is horrible (and not
> the cleanest thing you've ever seen probably :D)...I'll have a chat with Sudeep too.
>
> > >
> > > Now probably I wrote too much of an explanation and confuse stuff more ... :D
> >
> > No, thank you for the detailed explanation. I will continue my review.
> >
>
> Thanks
>
> Regards
>
> Cristian
>
>
>
> -->8-----------------
> diff --git a/drivers/firmware/arm_scmi/notify.c b/drivers/firmware/arm_scmi/notify.c
> index 9eb6b8b71bac..8719e077358c 100644
> --- a/drivers/firmware/arm_scmi/notify.c
> +++ b/drivers/firmware/arm_scmi/notify.c
> @@ -223,6 +223,7 @@ struct scmi_notify_instance {
> */
> struct events_queue {
> size_t sz;
> + atomic_t exiting;
> struct kfifo kfifo;
> struct work_struct notify_work;
> struct workqueue_struct *wq;
> @@ -406,11 +407,16 @@ scmi_process_event_header(struct events_queue *eq,
>
> outs = kfifo_out(&eq->kfifo, pd->eh,
> sizeof(struct scmi_event_header));
> - if (!outs)
> + if (!outs) {
> + atomic_set(&eq->exiting, 1);
> + smp_mb__after_atomic();
> return NULL;
> + }
> if (outs != sizeof(struct scmi_event_header)) {
> pr_err("SCMI Notifications: corrupted EVT header. Flush.\n");
> kfifo_reset_out(&eq->kfifo);
> + atomic_set(&eq->exiting, 1);
> + smp_mb__after_atomic();
> return NULL;
> }
>
> @@ -446,6 +452,8 @@ scmi_process_event_payload(struct events_queue *eq,
> outs = kfifo_out(&eq->kfifo, pd->eh->payld, pd->eh->payld_sz);
> if (unlikely(!outs)) {
> pr_warn("--- EMPTY !!!!\n");
> + atomic_set(&eq->exiting, 1);
> + smp_mb__after_atomic();
> return false;
> }
>
> @@ -455,6 +463,8 @@ scmi_process_event_payload(struct events_queue *eq,
> if (unlikely(outs != pd->eh->payld_sz)) {
> pr_err("SCMI Notifications: corrupted EVT Payload. Flush.\n");
> kfifo_reset_out(&eq->kfifo);
> + atomic_set(&eq->exiting, 1);
> + smp_mb__after_atomic();
> return false;
> }
>
> @@ -526,6 +536,8 @@ static void scmi_events_dispatcher(struct work_struct *work)
> mdelay(200);
>
> eq = container_of(work, struct events_queue, notify_work);
> + atomic_set(&eq->exiting, 0);
> + smp_mb__after_atomic();
> pd = container_of(eq, struct scmi_registered_protocol_events_desc,
> equeue);
> /*
> @@ -579,6 +591,8 @@ static void scmi_events_dispatcher(struct work_struct *work)
> int scmi_notify(const struct scmi_handle *handle, u8 proto_id, u8 evt_id,
> const void *buf, size_t len, u64 ts)
> {
> + bool exiting;
> + static u8 cnt = 3;
> struct scmi_registered_event *r_evt;
> struct scmi_event_header eh;
> struct scmi_notify_instance *ni = handle->notify_priv;
> @@ -616,8 +630,20 @@ int scmi_notify(const struct scmi_handle *handle, u8 proto_id, u8 evt_id,
> kfifo_in(&r_evt->proto->equeue.kfifo, &eh, sizeof(eh));
> mdelay(30);
> kfifo_in(&r_evt->proto->equeue.kfifo, buf, len);
> - queue_work(r_evt->proto->equeue.wq,
> - &r_evt->proto->equeue.notify_work);
> +
> + smp_mb__before_atomic();
> + exiting = atomic_read(&r_evt->proto->equeue.exiting);
> + do {
> + bool ret;
> +
> + ret = queue_work(r_evt->proto->equeue.wq,
> + &r_evt->proto->equeue.notify_work);
> + if (likely(ret || !exiting))
> + break;
> + mdelay(5);
> + smp_mb__before_atomic();
> + exiting = atomic_read(&r_evt->proto->equeue.exiting);
> + } while (--cnt);
>
> return 0;
> }
> @@ -655,6 +681,7 @@ static int scmi_initialize_events_queue(struct scmi_notify_instance *ni,
> &equeue->kfifo);
> if (ret)
> return ret;
> + atomic_set(&equeue->exiting, 0);
>
> INIT_WORK(&equeue->notify_work, scmi_events_dispatcher);
> equeue->wq = ni->notify_wq;
> --<8-----------------------
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ARM: OMAP2+: remove unneeded variable "errata" in configure_dma_errata()
From: Peter Ujfalusi @ 2020-05-20 7:08 UTC (permalink / raw)
To: Jason Yan, Russell King - ARM Linux admin
Cc: tony, linux-omap, linux-kernel, linux-arm-kernel
In-Reply-To: <c24dfb33-bbce-732e-c242-f4f4e6a1e04f@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 834 bytes --]
On 06/05/2020 11.43, Jason Yan wrote:
>
>
> 在 2020/5/6 16:29, Russell King - ARM Linux admin 写道:
>> On Wed, May 06, 2020 at 02:19:00PM +0800, Jason Yan wrote:
>>> Fix the following coccicheck warning:
>>>
>>> arch/arm/mach-omap2/dma.c:82:10-16: Unneeded variable: "errata". Return
>>> "0" on line 161
>>
>> NAK. Look closer at what the code is doing, thanks.
>>
>> This warning is basically incorrect.
>>
>
> OK, the macro SET_DMA_ERRATA is using this variable.
How can a patch from you can be trusted if you did not even try to
compile-test?
Please try to _test_ your changes before sending, or add a disclaimer
that it might not even compile.
Thanks,
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 1783 bytes --]
[-- Attachment #3: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
From: Stephan Mueller @ 2020-05-20 7:01 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Eric Biggers, Linux Crypto Mailing List, Linux ARM
In-Reply-To: <CAMj1kXF=Duh1AsAQy+aLWMcJPQ4RFL5p9-Mnmn-XAiCkzyGFbg@mail.gmail.com>
Am Mittwoch, 20. Mai 2020, 08:54:10 CEST schrieb Ard Biesheuvel:
Hi Ard,
> On Wed, 20 May 2020 at 08:47, Stephan Mueller <smueller@chronox.de> wrote:
> > Am Mittwoch, 20. Mai 2020, 08:40:57 CEST schrieb Ard Biesheuvel:
> >
> > Hi Ard,
> >
> > > On Wed, 20 May 2020 at 08:03, Stephan Mueller <smueller@chronox.de>
wrote:
> > > > Am Dienstag, 19. Mai 2020, 21:02:09 CEST schrieb Ard Biesheuvel:
> > > >
> > > > Hi Ard,
> > > >
> > > > > Stephan reports that the arm64 implementation of cts(cbc(aes))
> > > > > deviates
> > > > > from the generic implementation in what it returns as the output IV.
> > > > > So
> > > > > fix this, and add some test vectors to catch other non-compliant
> > > > > implementations.
> > > > >
> > > > > Stephan, could you provide a reference for the NIST validation tool
> > > > > and
> > > > > how it flags this behaviour as non-compliant? Thanks.
> > > >
> > > > The test definition that identified the inconsistent behavior is
> > > > specified
> > > > with [1]. Note, this testing is intended to become an RFC standard.
> > >
> > > Are you referring to the line
> > >
> > > CT[j] = AES_CBC_CS_ENCRYPT(Key[i], PT[j])
> > >
> > > where the CTS transform is invoked without an IV altogether?
> >
> > Precisely.
> >
> > > That
> > > simply seems like a bug to me. In an abstract specification like this,
> > > it would be insane for pseudocode functions to be stateful objects,
> > > and there is nothing in the pseudocode that explicitly captures the
> > > 'output IV' of that function call.
> >
> > I think the description may be updated by simply refer to IV[j-1]. Then
> > you
> > would not have a stateful operation, but you rest on the IV of the
> > previous
> > operation.
>
> But that value is not the value you are using now, right? I suspect
> that the line
>
> IV[i+1] = MSB(CT[j], IV.length)
Yes, such a line would be needed.
>
> needs to be duplicated in the inner loop for j, although that would
> require different versions for CS1/2/3
Correct in the sense to specify exactly what the IV after a cipher operation
actually is.
>
> > The state of all block chaining modes we currently have is defined with
> > the
> > IV. That is the reason why I mentioned it can be implemented stateless
> > when I am able to get the IV output from the previous operation.
>
> But it is simply the same as the penultimate block of ciphertext. So
> you can simply capture it after encrypt, or before decrypt. There is
> really no need to rely on the CTS transformation to pass it back to
> you via the buffer that is only specified to provide an input to the
> CTS transform.
Let me recheck that as I am not fully sure on that one. But if it can be
handled that way, it would make life easier.
>
> > > > To facilitate that testing, NIST offers an internet service, the ACVP
> > > > server, that allows obtaining test vectors and uploading responses.
> > > > You
> > > > see the large number of concluded testing with [2]. A particular
> > > > completion of the CTS testing I finished yesterday is given in [3].
> > > > That
> > > > particular testing was also performed on an ARM system with CE where
> > > > the
> > > > issue was detected.
> > > >
> > > > I am performing the testing with [4] that has an extension to test the
> > > > kernel crypto API.
> > >
> > > OK. So given that that neither the CTS spec nor this document makes
> > > any mention of an output IV or what its value should be, my suggestion
> > > would be to capture the IV directly from the ciphertext, rather than
> > > relying on some unspecified behavior to give you the right data. Note
> > > that we have other implementations of cts(cbc(aes)) in the kernel as
> > > well (h/w accelerated ones) and if there is no specification that
> > > defines this behavior, you really shouldn't be relying on it.
> >
> > Agreed, but all I need is the IV from the previous round without relying
> > on
> > any state.
>
> So just grab it from the ciphertext of the previous round.
>
> > > That 'specification' invokes AES_CBC_CS_ENCRYPT() twice using a
> > > different prototype, without any mention whatsoever what the implied
> > > value of IV[] is if it is missing. This is especially problematic
> > > given that it seems to cover all of CS1/2/3, and the relation between
> > > next IV and ciphertext is not even the same between those for inputs
> > > that are a multiple of the blocksize.
> >
> > I will relay that comment back to the authors for update.
>
> Thanks.
Thank you for your ideas!
>
> > > > [1]
> > > > https://github.com/usnistgov/ACVP/blob/master/artifacts/draft-celi-acv
> > > > p-b
> > > > lock-ciph-00.txt#L366
> > > >
> > > > [2]
> > > > https://csrc.nist.gov/projects/cryptographic-algorithm-validation-prog
> > > > ram
> > > > /
> > > > validation-search?searchMode=validation&family=1&productType=-1&ipp=2
> > > > 5
> > > >
> > > > [3]
> > > > https://csrc.nist.gov/projects/cryptographic-algorithm-validation-prog
> > > > ram
> > > > / details?validation=32608
> > > >
> > > > [4] https://github.com/smuellerDD/acvpparser
> > > >
> > > > > Cc: Stephan Mueller <smueller@chronox.de>
> > > > >
> > > > > Ard Biesheuvel (2):
> > > > > crypto: arm64/aes - align output IV with generic CBC-CTS driver
> > > > > crypto: testmgr - add output IVs for AES-CBC with ciphertext
> > > > > stealing
> > > > >
> > > > > arch/arm64/crypto/aes-modes.S | 2 ++
> > > > > crypto/testmgr.h | 12 ++++++++++++
> > > > > 2 files changed, 14 insertions(+)
> > > >
> > > > Ciao
> > > > Stephan
> >
> > Ciao
> > Stephan
Ciao
Stephan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH 0/3] arm64: perf: Add support for Perf NMI interrupts
From: Song Bao Hua @ 2020-05-20 6:55 UTC (permalink / raw)
To: Alexandru Elisei, Mark Rutland, Lecopzer Chen
Cc: Sumit Garg, jolsa@redhat.com, Jian-Lin Chen,
alexander.shishkin@linux.intel.com, Catalin Marinas,
yj.chiang@mediatek.com, Linux Kernel Mailing List,
acme@kernel.org, Linuxarm, Peter Zijlstra, mingo@redhat.com,
linux-mediatek@lists.infradead.org, julien.thierry.kdev@gmail.com,
matthias.bgg@gmail.com, namhyung@kernel.org, Will Deacon,
linux-arm-kernel
In-Reply-To: <a9002b5e-aec5-b6e0-7174-87b93351d60c@arm.com>
>
> On 5/18/20 11:45 AM, Mark Rutland wrote:
> > Hi all,
> >
> > On Mon, May 18, 2020 at 02:26:00PM +0800, Lecopzer Chen wrote:
> >> HI Sumit,
> >>
> >> Thanks for your information.
> >>
> >> I've already implemented IPI (same as you did [1], little difference
> >> in detail), hardlockup detector and perf in last year(2019) for
> >> debuggability.
> >> And now we tend to upstream to reduce kernel maintaining effort.
> >> I'm glad if someone in ARM can do this work :)
> >>
> >> Hi Julien,
> >>
> >> Does any Arm maintainers can proceed this action?
> > Alexandru (Cc'd) has been rebasing and reworking Julien's patches,
> > which is my preferred approach.
> >
> > I understand that's not quite ready for posting since he's
> > investigating some of the nastier subtleties (e.g. mutual exclusion
> > with the NMI), but maybe we can put the work-in-progress patches
> > somewhere in the mean time.
> >
> > Alexandru, do you have an idea of what needs to be done, and/or when
> > you expect you could post that?
>
> I'm currently working on rebasing the patches on top of 5.7-rc5, when I have
> something usable I'll post a link (should be a couple of days). After that I will
> address the review comments, and I plan to do a thorough testing because I'm
> not 100% confident that some of the assumptions around the locks that were
> removed are correct. My guess is this will take a few weeks.
+1
I would be awesome if perf NMI patches could be re-activated. Right now, it seems it is hard to
do "perf annotate" on a kernel function with local_irq disabled.
func()
{
local_irq_save();
.....
....
local_irq_restore();
return;
}
Perf will report all cycles are used by the last moment of the func().
Thanks,
Barry
>
> Thanks,
> Alex
> >
> > Thanks,
> > Mark.
> >
> >> This is really useful in debugging.
> >> Thank you!!
> >>
> >>
> >>
> >> [1] https://lkml.org/lkml/2020/4/24/328
> >>
> >>
> >> Lecopzer
> >>
> >> Sumit Garg <sumit.garg@linaro.org> 於 2020年5月18日 週一 下午
> 1:46寫道：
> >>> + Julien
> >>>
> >>> Hi Lecopzer,
> >>>
> >>> On Sat, 16 May 2020 at 18:20, Lecopzer Chen <lecopzer@gmail.com>
> wrote:
> >>>> These series implement Perf NMI funxtionality and depends on Pseudo
> >>>> NMI [1] which has been upstreamed.
> >>>>
> >>>> In arm64 with GICv3, Pseudo NMI was implemented for NMI-like
> interruts.
> >>>> That can be extended to Perf NMI which is the prerequisite for
> >>>> hard-lockup detector which had already a standard interface inside Linux.
> >>>>
> >>>> Thus the first step we need to implement perf NMI interface and
> >>>> make sure it works fine.
> >>>>
> >>> This is something that is already implemented via Julien's patch-set
> >>> [1]. Its v4 has been floating since July, 2019 and I couldn't find
> >>> any major blocking comments but not sure why things haven't
> >>> progressed further.
> >>>
> >>> Maybe Julien or Arm maintainers can provide updates on existing
> >>> patch-set [1] and how we should proceed further with this
> >>> interesting feature.
> >>>
> >>> And regarding hard-lockup detection, I have been able to enable it
> >>> based on perf NMI events using Julien's perf patch-set [1]. Have a
> >>> look at the patch here [2].
> >>>
> >>> [1] https://patchwork.kernel.org/cover/11047407/
> >>> [2]
> >>> http://lists.infradead.org/pipermail/linux-arm-kernel/2020-May/73222
> >>> 7.html
> >>>
> >>> -Sumit
> >>>
> >>>> Perf NMI has been test by dd if=/dev/urandom of=/dev/null like the
> >>>> link [2] did.
> >>>>
> >>>> [1] https://lkml.org/lkml/2019/1/31/535
> >>>> [2] https://www.linaro.org/blog/debugging-arm-kernels-using-nmifiq
> >>>>
> >>>>
> >>>> Lecopzer Chen (3):
> >>>> arm_pmu: Add support for perf NMI interrupts registration
> >>>> arm64: perf: Support NMI context for perf event ISR
> >>>> arm64: Kconfig: Add support for the Perf NMI
> >>>>
> >>>> arch/arm64/Kconfig | 10 +++++++
> >>>> arch/arm64/kernel/perf_event.c | 36 ++++++++++++++++++------
> >>>> drivers/perf/arm_pmu.c | 51
> ++++++++++++++++++++++++++++++----
> >>>> include/linux/perf/arm_pmu.h | 6 ++++
> >>>> 4 files changed, 88 insertions(+), 15 deletions(-)
> >>>>
> >>>> --
> >>>> 2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] iommu: Don't call .probe_finalize() under group->mutex
From: Yong Wu @ 2020-05-20 6:53 UTC (permalink / raw)
To: Joerg Roedel
Cc: Joerg Roedel, linux-kernel, iommu, linux-mediatek,
Matthias Brugger, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20200519132824.15163-1-joro@8bytes.org>
On Tue, 2020-05-19 at 15:28 +0200, Joerg Roedel wrote:
> From: Joerg Roedel <jroedel@suse.de>
>
> The .probe_finalize() call-back of some IOMMU drivers calls into
> arm_iommu_attach_device(). This function will call back into the
> IOMMU core code, where it tries to take group->mutex again, resulting
> in a deadlock.
>
> As there is no reason why .probe_finalize() needs to be called under
> that mutex, move it after the lock has been released to fix the
> deadlock.
>
> Cc: Yong Wu <yong.wu@mediatek.com>
> Reported-by: Yong Wu <yong.wu@mediatek.com>
> Fixes: deac0b3bed26 ("iommu: Split off default domain allocation from group assignment")
> Signed-off-by: Joerg Roedel <jroedel@suse.de>
Tested-by: Yong Wu <yong.wu@mediatek.com>
Tested on MediaTek-v1 mt2701 evb board.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
From: Ard Biesheuvel @ 2020-05-20 6:54 UTC (permalink / raw)
To: Stephan Mueller; +Cc: Eric Biggers, Linux Crypto Mailing List, Linux ARM
In-Reply-To: <16394356.0UTfFWEGjO@tauon.chronox.de>
On Wed, 20 May 2020 at 08:47, Stephan Mueller <smueller@chronox.de> wrote:
>
> Am Mittwoch, 20. Mai 2020, 08:40:57 CEST schrieb Ard Biesheuvel:
>
> Hi Ard,
>
> > On Wed, 20 May 2020 at 08:03, Stephan Mueller <smueller@chronox.de> wrote:
> > > Am Dienstag, 19. Mai 2020, 21:02:09 CEST schrieb Ard Biesheuvel:
> > >
> > > Hi Ard,
> > >
> > > > Stephan reports that the arm64 implementation of cts(cbc(aes)) deviates
> > > > from the generic implementation in what it returns as the output IV. So
> > > > fix this, and add some test vectors to catch other non-compliant
> > > > implementations.
> > > >
> > > > Stephan, could you provide a reference for the NIST validation tool and
> > > > how it flags this behaviour as non-compliant? Thanks.
> > >
> > > The test definition that identified the inconsistent behavior is specified
> > > with [1]. Note, this testing is intended to become an RFC standard.
> >
> > Are you referring to the line
> >
> > CT[j] = AES_CBC_CS_ENCRYPT(Key[i], PT[j])
> >
> > where the CTS transform is invoked without an IV altogether?
>
> Precisely.
>
> > That
> > simply seems like a bug to me. In an abstract specification like this,
> > it would be insane for pseudocode functions to be stateful objects,
> > and there is nothing in the pseudocode that explicitly captures the
> > 'output IV' of that function call.
>
> I think the description may be updated by simply refer to IV[j-1]. Then you
> would not have a stateful operation, but you rest on the IV of the previous
> operation.
>
But that value is not the value you are using now, right? I suspect
that the line
IV[i+1] = MSB(CT[j], IV.length)
needs to be duplicated in the inner loop for j, although that would
require different versions for CS1/2/3
> The state of all block chaining modes we currently have is defined with the
> IV. That is the reason why I mentioned it can be implemented stateless when I
> am able to get the IV output from the previous operation.
>
But it is simply the same as the penultimate block of ciphertext. So
you can simply capture it after encrypt, or before decrypt. There is
really no need to rely on the CTS transformation to pass it back to
you via the buffer that is only specified to provide an input to the
CTS transform.
> >
> > > To facilitate that testing, NIST offers an internet service, the ACVP
> > > server, that allows obtaining test vectors and uploading responses. You
> > > see the large number of concluded testing with [2]. A particular
> > > completion of the CTS testing I finished yesterday is given in [3]. That
> > > particular testing was also performed on an ARM system with CE where the
> > > issue was detected.
> > >
> > > I am performing the testing with [4] that has an extension to test the
> > > kernel crypto API.
> >
> > OK. So given that that neither the CTS spec nor this document makes
> > any mention of an output IV or what its value should be, my suggestion
> > would be to capture the IV directly from the ciphertext, rather than
> > relying on some unspecified behavior to give you the right data. Note
> > that we have other implementations of cts(cbc(aes)) in the kernel as
> > well (h/w accelerated ones) and if there is no specification that
> > defines this behavior, you really shouldn't be relying on it.
>
> Agreed, but all I need is the IV from the previous round without relying on
> any state.
So just grab it from the ciphertext of the previous round.
> >
> >
> > That 'specification' invokes AES_CBC_CS_ENCRYPT() twice using a
> > different prototype, without any mention whatsoever what the implied
> > value of IV[] is if it is missing. This is especially problematic
> > given that it seems to cover all of CS1/2/3, and the relation between
> > next IV and ciphertext is not even the same between those for inputs
> > that are a multiple of the blocksize.
>
> I will relay that comment back to the authors for update.
Thanks.
> >
> > > [1]
> > > https://github.com/usnistgov/ACVP/blob/master/artifacts/draft-celi-acvp-b
> > > lock-ciph-00.txt#L366
> > >
> > > [2]
> > > https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program
> > > / validation-search?searchMode=validation&family=1&productType=-1&ipp=25
> > >
> > > [3]
> > > https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program
> > > / details?validation=32608
> > >
> > > [4] https://github.com/smuellerDD/acvpparser
> > >
> > > > Cc: Stephan Mueller <smueller@chronox.de>
> > > >
> > > > Ard Biesheuvel (2):
> > > > crypto: arm64/aes - align output IV with generic CBC-CTS driver
> > > > crypto: testmgr - add output IVs for AES-CBC with ciphertext stealing
> > > >
> > > > arch/arm64/crypto/aes-modes.S | 2 ++
> > > > crypto/testmgr.h | 12 ++++++++++++
> > > > 2 files changed, 14 insertions(+)
> > >
> > > Ciao
> > > Stephan
>
>
> Ciao
> Stephan
>
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH 3/3] arm64: dts: imx8mp: add mu node
From: Aisheng Dong @ 2020-05-20 6:50 UTC (permalink / raw)
To: Fabio Estevam, Peng Fan
Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Stephen Boyd, Daniel Baluta, linux-kernel, Rob Herring,
dl-linux-imx, Sascha Hauer, Fabio Estevam, Leonard Crestez,
Shawn Guo, linux-clk,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Lucas Stach
In-Reply-To: <CAOMZO5DmiQbfJjTLKPiv6uUNaQ8ae-0h7uOpPf2J9ZMwDoE+Eg@mail.gmail.com>
> From: Fabio Estevam <festevam@gmail.com>
> Sent: Wednesday, May 20, 2020 11:07 AM
>
> Hi Peng,
>
> On Wed, May 20, 2020 at 12:01 AM Peng Fan <peng.fan@nxp.com> wrote:
>
> > Nothing specific in i.MX8MP for the mu part, so do we really need add
> > "fsl,imx8mp-mu"?
>
> It is good practice to add a more specific option.
>
> Let's say in future a bug is found that affects imx8mp MU, then you could fix the
> MU driver and keep the dtb compatibility.
+1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RFC PATCH] arm64: dts: rockchip: fix dmas dma-names for rk3308 i2s node
From: Johan Jonker @ 2020-05-20 6:48 UTC (permalink / raw)
To: heiko; +Cc: devicetree, robh+dt, linux-kernel, linux-arm-kernel,
linux-rockchip
One of the current rk3308 'i2s' nodes has a different dma layout
with only 1 item. Table 9-2 DMAC1 Request Mapping Table shows that
there 2 dma sources available, so fix the dmas and dma-names
for the rk3308 'i2s' node.
10 I2S/PCM_2CH_1 tx High level
11 I2S/PCM_2CH_1 rx High level
Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
arch/arm64/boot/dts/rockchip/rk3308.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/rockchip/rk3308.dtsi b/arch/arm64/boot/dts/rockchip/rk3308.dtsi
index ac7f69407..79c1dd1fe 100644
--- a/arch/arm64/boot/dts/rockchip/rk3308.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3308.dtsi
@@ -564,8 +564,8 @@
interrupts = <GIC_SPI 53 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cru SCLK_I2S1_2CH>, <&cru HCLK_I2S1_2CH>;
clock-names = "i2s_clk", "i2s_hclk";
- dmas = <&dmac1 11>;
- dma-names = "rx";
+ dmas = <&dmac1 10>, <&dmac1 11>;
+ dma-names = "tx", "rx";
resets = <&cru SRST_I2S1_2CH_M>, <&cru SRST_I2S1_2CH_H>;
reset-names = "reset-m", "reset-h";
status = "disabled";
--
2.11.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [RFC/RFT PATCH 0/2] crypto: add CTS output IVs for arm64 and testmgr
From: Stephan Mueller @ 2020-05-20 6:47 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Eric Biggers, Linux Crypto Mailing List, Linux ARM
In-Reply-To: <CAMj1kXFDcHncnb=aUkWnX22Co2r4g3DSM+wug0FQ231Gv7J01Q@mail.gmail.com>
Am Mittwoch, 20. Mai 2020, 08:40:57 CEST schrieb Ard Biesheuvel:
Hi Ard,
> On Wed, 20 May 2020 at 08:03, Stephan Mueller <smueller@chronox.de> wrote:
> > Am Dienstag, 19. Mai 2020, 21:02:09 CEST schrieb Ard Biesheuvel:
> >
> > Hi Ard,
> >
> > > Stephan reports that the arm64 implementation of cts(cbc(aes)) deviates
> > > from the generic implementation in what it returns as the output IV. So
> > > fix this, and add some test vectors to catch other non-compliant
> > > implementations.
> > >
> > > Stephan, could you provide a reference for the NIST validation tool and
> > > how it flags this behaviour as non-compliant? Thanks.
> >
> > The test definition that identified the inconsistent behavior is specified
> > with [1]. Note, this testing is intended to become an RFC standard.
>
> Are you referring to the line
>
> CT[j] = AES_CBC_CS_ENCRYPT(Key[i], PT[j])
>
> where the CTS transform is invoked without an IV altogether?
Precisely.
> That
> simply seems like a bug to me. In an abstract specification like this,
> it would be insane for pseudocode functions to be stateful objects,
> and there is nothing in the pseudocode that explicitly captures the
> 'output IV' of that function call.
I think the description may be updated by simply refer to IV[j-1]. Then you
would not have a stateful operation, but you rest on the IV of the previous
operation.
The state of all block chaining modes we currently have is defined with the
IV. That is the reason why I mentioned it can be implemented stateless when I
am able to get the IV output from the previous operation.
>
> > To facilitate that testing, NIST offers an internet service, the ACVP
> > server, that allows obtaining test vectors and uploading responses. You
> > see the large number of concluded testing with [2]. A particular
> > completion of the CTS testing I finished yesterday is given in [3]. That
> > particular testing was also performed on an ARM system with CE where the
> > issue was detected.
> >
> > I am performing the testing with [4] that has an extension to test the
> > kernel crypto API.
>
> OK. So given that that neither the CTS spec nor this document makes
> any mention of an output IV or what its value should be, my suggestion
> would be to capture the IV directly from the ciphertext, rather than
> relying on some unspecified behavior to give you the right data. Note
> that we have other implementations of cts(cbc(aes)) in the kernel as
> well (h/w accelerated ones) and if there is no specification that
> defines this behavior, you really shouldn't be relying on it.
Agreed, but all I need is the IV from the previous round without relying on
any state.
>
>
> That 'specification' invokes AES_CBC_CS_ENCRYPT() twice using a
> different prototype, without any mention whatsoever what the implied
> value of IV[] is if it is missing. This is especially problematic
> given that it seems to cover all of CS1/2/3, and the relation between
> next IV and ciphertext is not even the same between those for inputs
> that are a multiple of the blocksize.
I will relay that comment back to the authors for update.
>
> > [1]
> > https://github.com/usnistgov/ACVP/blob/master/artifacts/draft-celi-acvp-b
> > lock-ciph-00.txt#L366
> >
> > [2]
> > https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program
> > / validation-search?searchMode=validation&family=1&productType=-1&ipp=25
> >
> > [3]
> > https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program
> > / details?validation=32608
> >
> > [4] https://github.com/smuellerDD/acvpparser
> >
> > > Cc: Stephan Mueller <smueller@chronox.de>
> > >
> > > Ard Biesheuvel (2):
> > > crypto: arm64/aes - align output IV with generic CBC-CTS driver
> > > crypto: testmgr - add output IVs for AES-CBC with ciphertext stealing
> > >
> > > arch/arm64/crypto/aes-modes.S | 2 ++
> > > crypto/testmgr.h | 12 ++++++++++++
> > > 2 files changed, 14 insertions(+)
> >
> > Ciao
> > Stephan
Ciao
Stephan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH 2/3] clk: imx8mp: add mu root clk
From: Aisheng Dong @ 2020-05-20 6:46 UTC (permalink / raw)
To: Peng Fan, shawnguo@kernel.org, Fabio Estevam,
kernel@pengutronix.de, robh+dt@kernel.org, sboyd@kernel.org
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
dl-linux-imx, Leonard Crestez, Daniel Baluta,
linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
l.stach@pengutronix.de
In-Reply-To: <1589940278-3680-3-git-send-email-peng.fan@nxp.com>
> From: Peng Fan <peng.fan@nxp.com>
> Sent: Wednesday, May 20, 2020 10:05 AM
>
> Add mu root clk for mu mailbox usage.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
Regards
Aisheng
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [PATCH 1/3] arm64: dts: imx8m: add mu node
From: Aisheng Dong @ 2020-05-20 6:45 UTC (permalink / raw)
To: Peng Fan, shawnguo@kernel.org, Fabio Estevam,
kernel@pengutronix.de, robh+dt@kernel.org, sboyd@kernel.org
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
dl-linux-imx, Leonard Crestez, Daniel Baluta,
linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
l.stach@pengutronix.de
In-Reply-To: <1589940278-3680-2-git-send-email-peng.fan@nxp.com>
> From: Peng Fan <peng.fan@nxp.com>
> Sent: Wednesday, May 20, 2020 10:05 AM
>
> Add mu node to let A53 could communicate with M Core.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
> arch/arm64/boot/dts/freescale/imx8mm.dtsi | 9 +++++++++
> arch/arm64/boot/dts/freescale/imx8mn.dtsi | 9 +++++++++
> arch/arm64/boot/dts/freescale/imx8mq.dtsi | 9 +++++++++
> 3 files changed, 27 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> index f3bbefe3e59f..9722f76d8c3f 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> @@ -771,6 +771,15 @@
> status = "disabled";
> };
>
> + mu: mailbox@30aa0000 {
> + compatible = "fsl,imx6sx-mu";
Usually we also add current SoC compatible string.
compatible = "fsl,imx8mm-mu", "fsl,imx6sx-mu"
> + reg = <0x30aa0000 0x10000>;
> + interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MM_CLK_MU_ROOT>;
> + clock-names = "mu";
Undocumented property, drop it
> + #mbox-cells = <2>;
> + };
> +
> usdhc1: mmc@30b40000 {
> compatible = "fsl,imx8mm-usdhc", "fsl,imx7d-usdhc";
> reg = <0x30b40000 0x10000>;
> diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> index fb63a98fdff5..5f30f1d50460 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
> @@ -671,6 +671,15 @@
> status = "disabled";
> };
>
> + mu: mailbox@30aa0000 {
> + compatible = "fsl,imx6sx-mu";
> + reg = <0x30aa0000 0x10000>;
> + interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MN_CLK_MU_ROOT>;
> + clock-names = "mu";
> + #mbox-cells = <2>;
> + };
> +
> usdhc1: mmc@30b40000 {
> compatible = "fsl,imx8mn-usdhc", "fsl,imx7d-usdhc";
> reg = <0x30b40000 0x10000>;
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> index 1d15680a4962..e969fcbbd15f 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -956,6 +956,15 @@
> status = "disabled";
> };
>
> + mu: mailbox@30aa0000 {
> + compatible = "fsl,imx6sx-mu";
> + reg = <0x30aa0000 0x10000>;
> + interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clk IMX8MQ_CLK_MU_ROOT>;
> + clock-names = "mu";
> + #mbox-cells = <2>;
> + };
> +
> usdhc1: mmc@30b40000 {
> compatible = "fsl,imx8mq-usdhc",
> "fsl,imx7d-usdhc";
> --
> 2.16.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v7 04/24] iommu: Add a page fault handler
From: Lu Baolu @ 2020-05-20 6:42 UTC (permalink / raw)
To: Jean-Philippe Brucker, iommu, devicetree, linux-arm-kernel,
linux-pci, linux-mm
Cc: fenghua.yu, kevin.tian, jacob.jun.pan, jgg, catalin.marinas, joro,
robin.murphy, hch, zhangfei.gao, Jonathan.Cameron, felix.kuehling,
xuzaibo, will, christian.koenig, baolu.lu
In-Reply-To: <20200519175502.2504091-5-jean-philippe@linaro.org>
Hi Jean,
On 2020/5/20 1:54, Jean-Philippe Brucker wrote:
> Some systems allow devices to handle I/O Page Faults in the core mm. For
> example systems implementing the PCIe PRI extension or Arm SMMU stall
> model. Infrastructure for reporting these recoverable page faults was
> added to the IOMMU core by commit 0c830e6b3282 ("iommu: Introduce device
> fault report API"). Add a page fault handler for host SVA.
>
> IOMMU driver can now instantiate several fault workqueues and link them
> to IOPF-capable devices. Drivers can choose between a single global
> workqueue, one per IOMMU device, one per low-level fault queue, one per
> domain, etc.
>
> When it receives a fault event, supposedly in an IRQ handler, the IOMMU
> driver reports the fault using iommu_report_device_fault(), which calls
> the registered handler. The page fault handler then calls the mm fault
> handler, and reports either success or failure with iommu_page_response().
> When the handler succeeded, the IOMMU retries the access.
>
> The iopf_param pointer could be embedded into iommu_fault_param. But
> putting iopf_param into the iommu_param structure allows us not to care
> about ordering between calls to iopf_queue_add_device() and
> iommu_register_device_fault_handler().
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
> v6->v7: Fix leak in iopf_queue_discard_partial()
> ---
> drivers/iommu/Kconfig | 4 +
> drivers/iommu/Makefile | 1 +
> include/linux/iommu.h | 51 +++++
> drivers/iommu/io-pgfault.c | 459 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 515 insertions(+)
> create mode 100644 drivers/iommu/io-pgfault.c
>
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index d9fa5b410015..15e9dc4e503c 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -107,6 +107,10 @@ config IOMMU_SVA
> bool
> select IOASID
>
> +config IOMMU_PAGE_FAULT
> + bool
> + select IOMMU_SVA
> +
> config FSL_PAMU
> bool "Freescale IOMMU support"
> depends on PCI
> diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
> index 40c800dd4e3e..bf5cb4ee8409 100644
> --- a/drivers/iommu/Makefile
> +++ b/drivers/iommu/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_IOMMU_API) += iommu-traces.o
> obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
> obj-$(CONFIG_IOMMU_DEBUGFS) += iommu-debugfs.o
> obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
> +obj-$(CONFIG_IOMMU_PAGE_FAULT) += io-pgfault.o
> obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
> obj-$(CONFIG_IOMMU_IO_PGTABLE_ARMV7S) += io-pgtable-arm-v7s.o
> obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
[SNIP]
> +
> +static enum iommu_page_response_code
> +iopf_handle_single(struct iopf_fault *iopf)
> +{
> + vm_fault_t ret;
> + struct mm_struct *mm;
> + struct vm_area_struct *vma;
> + unsigned int access_flags = 0;
> + unsigned int fault_flags = FAULT_FLAG_REMOTE;
> + struct iommu_fault_page_request *prm = &iopf->fault.prm;
> + enum iommu_page_response_code status = IOMMU_PAGE_RESP_INVALID;
> +
> + if (!(prm->flags & IOMMU_FAULT_PAGE_REQUEST_PASID_VALID))
> + return status;
> +
> + mm = iommu_sva_find(prm->pasid);
> + if (IS_ERR_OR_NULL(mm))
> + return status;
> +
> + down_read(&mm->mmap_sem);
> +
> + vma = find_extend_vma(mm, prm->addr);
> + if (!vma)
> + /* Unmapped area */
> + goto out_put_mm;
> +
> + if (prm->perm & IOMMU_FAULT_PERM_READ)
> + access_flags |= VM_READ;
> +
> + if (prm->perm & IOMMU_FAULT_PERM_WRITE) {
> + access_flags |= VM_WRITE;
> + fault_flags |= FAULT_FLAG_WRITE;
> + }
> +
> + if (prm->perm & IOMMU_FAULT_PERM_EXEC) {
> + access_flags |= VM_EXEC;
> + fault_flags |= FAULT_FLAG_INSTRUCTION;
> + }
> +
> + if (!(prm->perm & IOMMU_FAULT_PERM_PRIV))
> + fault_flags |= FAULT_FLAG_USER;
> +
> + if (access_flags & ~vma->vm_flags)
> + /* Access fault */
> + goto out_put_mm;
> +
> + ret = handle_mm_fault(vma, prm->addr, fault_flags);
> + status = ret & VM_FAULT_ERROR ? IOMMU_PAGE_RESP_INVALID :
Do you mind telling why it's IOMMU_PAGE_RESP_INVALID but not
IOMMU_PAGE_RESP_FAILURE?
> + IOMMU_PAGE_RESP_SUCCESS;
> +
> +out_put_mm:
> + up_read(&mm->mmap_sem);
> + mmput(mm);
> +
> + return status;
> +}
> +
> +static void iopf_handle_group(struct work_struct *work)
> +{
> + struct iopf_group *group;
> + struct iopf_fault *iopf, *next;
> + enum iommu_page_response_code status = IOMMU_PAGE_RESP_SUCCESS;
> +
> + group = container_of(work, struct iopf_group, work);
> +
> + list_for_each_entry_safe(iopf, next, &group->faults, list) {
> + /*
> + * For the moment, errors are sticky: don't handle subsequent
> + * faults in the group if there is an error.
> + */
> + if (status == IOMMU_PAGE_RESP_SUCCESS)
> + status = iopf_handle_single(iopf);
> +
> + if (!(iopf->fault.prm.flags &
> + IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE))
> + kfree(iopf);
> + }
> +
> + iopf_complete_group(group->dev, &group->last_fault, status);
> + kfree(group);
> +}
> +
> +/**
> + * iommu_queue_iopf - IO Page Fault handler
> + * @evt: fault event
@fault?
> + * @cookie: struct device, passed to iommu_register_device_fault_handler.
> + *
> + * Add a fault to the device workqueue, to be handled by mm.
> + *
> + * This module doesn't handle PCI PASID Stop Marker; IOMMU drivers must discard
> + * them before reporting faults. A PASID Stop Marker (LRW = 0b100) doesn't
> + * expect a response. It may be generated when disabling a PASID (issuing a
> + * PASID stop request) by some PCI devices.
> + *
> + * The PASID stop request is issued by the device driver before unbind(). Once
> + * it completes, no page request is generated for this PASID anymore and
> + * outstanding ones have been pushed to the IOMMU (as per PCIe 4.0r1.0 - 6.20.1
> + * and 10.4.1.2 - Managing PASID TLP Prefix Usage). Some PCI devices will wait
> + * for all outstanding page requests to come back with a response before
> + * completing the PASID stop request. Others do not wait for page responses, and
> + * instead issue this Stop Marker that tells us when the PASID can be
> + * reallocated.
> + *
> + * It is safe to discard the Stop Marker because it is an optimization.
> + * a. Page requests, which are posted requests, have been flushed to the IOMMU
> + * when the stop request completes.
> + * b. We flush all fault queues on unbind() before freeing the PASID.
> + *
> + * So even though the Stop Marker might be issued by the device *after* the stop
> + * request completes, outstanding faults will have been dealt with by the time
> + * we free the PASID.
> + *
> + * Return: 0 on success and <0 on error.
> + */
> +int iommu_queue_iopf(struct iommu_fault *fault, void *cookie)
> +{
> + int ret;
> + struct iopf_group *group;
> + struct iopf_fault *iopf, *next;
> + struct iopf_device_param *iopf_param;
> +
> + struct device *dev = cookie;
> + struct dev_iommu *param = dev->iommu;
> +
> + lockdep_assert_held(¶m->lock);
> +
> + if (fault->type != IOMMU_FAULT_PAGE_REQ)
> + /* Not a recoverable page fault */
> + return -EOPNOTSUPP;
> +
> + /*
> + * As long as we're holding param->lock, the queue can't be unlinked
> + * from the device and therefore cannot disappear.
> + */
> + iopf_param = param->iopf_param;
> + if (!iopf_param)
> + return -ENODEV;
> +
> + if (!(fault->prm.flags & IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE)) {
> + iopf = kzalloc(sizeof(*iopf), GFP_KERNEL);
> + if (!iopf)
> + return -ENOMEM;
> +
> + iopf->fault = *fault;
> +
> + /* Non-last request of a group. Postpone until the last one */
> + list_add(&iopf->list, &iopf_param->partial);
> +
> + return 0;
> + }
> +
> + group = kzalloc(sizeof(*group), GFP_KERNEL);
> + if (!group) {
> + /*
> + * The caller will send a response to the hardware. But we do
> + * need to clean up before leaving, otherwise partial faults
> + * will be stuck.
> + */
> + ret = -ENOMEM;
> + goto cleanup_partial;
> + }
> +
> + group->dev = dev;
> + group->last_fault.fault = *fault;
> + INIT_LIST_HEAD(&group->faults);
> + list_add(&group->last_fault.list, &group->faults);
> + INIT_WORK(&group->work, iopf_handle_group);
> +
> + /* See if we have partial faults for this group */
> + list_for_each_entry_safe(iopf, next, &iopf_param->partial, list) {
> + if (iopf->fault.prm.grpid == fault->prm.grpid)
> + /* Insert *before* the last fault */
> + list_move(&iopf->list, &group->faults);
> + }
> +
> + queue_work(iopf_param->queue->wq, &group->work);
> + return 0;
> +
> +cleanup_partial:
> + list_for_each_entry_safe(iopf, next, &iopf_param->partial, list) {
> + if (iopf->fault.prm.grpid == fault->prm.grpid) {
> + list_del(&iopf->list);
> + kfree(iopf);
> + }
> + }
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iommu_queue_iopf);
[SNIP]
> +
> +
> +/**
> + * iopf_queue_add_device - Add producer to the fault queue
> + * @queue: IOPF queue
> + * @dev: device to add
> + *
> + * Return: 0 on success and <0 on error.
> + */
> +int iopf_queue_add_device(struct iopf_queue *queue, struct device *dev)
> +{
> + int ret = -EBUSY;
> + struct iopf_device_param *iopf_param;
> + struct dev_iommu *param = dev->iommu;
> +
> + if (!param)
> + return -ENODEV;
> +
> + iopf_param = kzalloc(sizeof(*iopf_param), GFP_KERNEL);
> + if (!iopf_param)
> + return -ENOMEM;
> +
> + INIT_LIST_HEAD(&iopf_param->partial);
> + iopf_param->queue = queue; iopf_param->dev = dev;
Two lines?
> +
> + mutex_lock(&queue->lock);
> + mutex_lock(¶m->lock);
> + if (!param->iopf_param) {
> + list_add(&iopf_param->queue_list, &queue->devices);
> + param->iopf_param = iopf_param;
> + ret = 0;
> + }
> + mutex_unlock(¶m->lock);
> + mutex_unlock(&queue->lock);
> +
> + if (ret)
> + kfree(iopf_param);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(iopf_queue_add_device);
> +
[SNIP]
Best regards,
baolu
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox