Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 02/20] dt-bindings: arm: Convert Marvell MMP board/soc bindings to json-schema
From: Lubomir Rintel @ 2019-08-22  9:26 UTC (permalink / raw)
  To: Olof Johansson
  Cc: Mark Rutland, devicetree, Jason Cooper, Stephen Boyd,
	Marc Zyngier, Michael Turquette, Russell King,
	Kishon Vijay Abraham I, Lubomir Rintel, Rob Herring,
	linux-arm-kernel, Thomas Gleixner, linux-clk, linux-kernel
In-Reply-To: <20190822092643.593488-1-lkundrak@v3.sk>

Convert Marvell MMP SoC bindings to DT schema format using json-schema.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

---
Changes since v1:
- Added this patch

 .../devicetree/bindings/arm/mrvl/mrvl.txt     | 14 ---------
 .../devicetree/bindings/arm/mrvl/mrvl.yaml    | 31 +++++++++++++++++++
 2 files changed, 31 insertions(+), 14 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/arm/mrvl/mrvl.txt
 create mode 100644 Documentation/devicetree/bindings/arm/mrvl/mrvl.yaml

diff --git a/Documentation/devicetree/bindings/arm/mrvl/mrvl.txt b/Documentation/devicetree/bindings/arm/mrvl/mrvl.txt
deleted file mode 100644
index 951687528efb0..0000000000000
--- a/Documentation/devicetree/bindings/arm/mrvl/mrvl.txt
+++ /dev/null
@@ -1,14 +0,0 @@
-Marvell Platforms Device Tree Bindings
-----------------------------------------------------
-
-PXA168 Aspenite Board
-Required root node properties:
-	- compatible = "mrvl,pxa168-aspenite", "mrvl,pxa168";
-
-PXA910 DKB Board
-Required root node properties:
-	- compatible = "mrvl,pxa910-dkb";
-
-MMP2 Brownstone Board
-Required root node properties:
-	- compatible = "mrvl,mmp2-brownstone", "mrvl,mmp2";
diff --git a/Documentation/devicetree/bindings/arm/mrvl/mrvl.yaml b/Documentation/devicetree/bindings/arm/mrvl/mrvl.yaml
new file mode 100644
index 0000000000000..dc9de506ac6e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mrvl/mrvl.yaml
@@ -0,0 +1,31 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/mrvl/mrvl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell Platforms Device Tree Bindings
+
+maintainers:
+  - Lubomir Rintel <lkundrak@v3.sk>
+
+properties:
+  $nodename:
+    const: '/'
+  compatible:
+    oneOf:
+      - description: PXA168 Aspenite Board
+        items:
+          - enum:
+              - mrvl,pxa168-aspenite
+          - const: mrvl,pxa168
+      - description: PXA910 DKB Board
+        items:
+          - enum:
+              - mrvl,pxa910-dkb
+      - description: MMP2 Brownstone Board
+        items:
+          - enum:
+              - mrvl,mmp2-brownstone
+          - const: mrvl,mmp2
+...
-- 
2.21.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] arm64: dts: mt8183: fix pwrap gic number
From: Matthias Brugger @ 2019-08-22  9:26 UTC (permalink / raw)
  To: Hsin-Hsiung Wang
  Cc: Mark Rutland, devicetree, srv_heupstream, linux-kernel,
	Rob Herring, linux-mediatek, linux-arm-kernel
In-Reply-To: <1566464140-26977-1-git-send-email-hsin-hsiung.wang@mediatek.com>



On 22/08/2019 10:55, Hsin-Hsiung Wang wrote:
> The correct gic number of pwrap is 185 instead of 209. This patch fixes
> it to avoid triggering error interrupt.
> 
> Fixes: e526c9bc11f8 ("arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and Makefile")
> 
> Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
> ---
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> index c2749c4..afb0996 100644
> --- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
> @@ -248,7 +248,7 @@
>  			compatible = "mediatek,mt8183-pwrap";
>  			reg = <0 0x1000d000 0 0x1000>;
>  			reg-names = "pwrap";
> -			interrupts = <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupts = <GIC_SPI 185 IRQ_TYPE_LEVEL_HIGH>;
>  			clocks = <&topckgen CLK_TOP_MUX_PMICSPI>,
>  				 <&infracfg CLK_INFRA_PMIC_AP>;
>  			clock-names = "spi", "wrap";
> 

Applied, thanks.

_______________________________________________
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/4] rtc: Add support for the MediaTek MT2712 RTC
From: Alexandre Belloni @ 2019-08-22  9:20 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Mark Rutland, Alessandro Zummo, Linus Walleij, Flora Fu,
	srv_heupstream, devicetree, Greg Kroah-Hartman, Ran Bi, Sean Wang,
	linux-kernel, YT Shen, Rob Herring, linux-mediatek,
	Jonathan Cameron, Mauro Carvalho Chehab, Yingjoe Chen,
	Eddie Huang, David S . Miller, linux-arm-kernel, linux-rtc
In-Reply-To: <c4e8b041-4a35-578e-07a3-2ebc99848ee2@gmail.com>

On 22/08/2019 11:12:29+0200, Matthias Brugger wrote:
> 
> 
> On 01/08/2019 13:01, Ran Bi wrote:
> > This add support for the MediaTek MT2712 RTC. It was SoC based RTC, but
> > had different architecture compared with MT7622 RTC.
> > 
> > Signed-off-by: Ran Bi <ran.bi@mediatek.com>
> > ---
> >  drivers/rtc/Kconfig      |  10 +
> >  drivers/rtc/Makefile     |   1 +
> >  drivers/rtc/rtc-mt2712.c | 444 +++++++++++++++++++++++++++++++++++++++
> 
> Can't we just adjust rtc-mt7622.c (and rename it) to unify the source for both
> devices. What is the difference that we need to write a driver of our own?
> 

If they are compatible, this is the way to go but the file can't be
renamed (and that is fine).


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.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 2/2] pwm: pwm-mediatek: Add MT8516 SoC support
From: Uwe Kleine-König @ 2019-08-22  9:18 UTC (permalink / raw)
  To: Fabien Parent
  Cc: mark.rutland, devicetree, linux-pwm, linux-kernel, robh+dt,
	thierry.reding, linux-mediatek, matthias.bgg, linux-arm-kernel
In-Reply-To: <20190805125848.15751-2-fparent@baylibre.com>

On Mon, Aug 05, 2019 at 02:58:48PM +0200, Fabien Parent wrote:
> Add the compatible and the platform data to support PWM on the MT8516
> SoC.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>

Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

The change looks fine, there is however another series currently waiting
for application for this driver that conflicts with this one (I think).

Maybe it would be sensible to join your forces and produce a single
series without conflicts?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
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] clk: imx: pll14xx: avoid glitch when set rate
From: Peng Fan @ 2019-08-22  9:18 UTC (permalink / raw)
  To: Leonard Crestez, sboyd@kernel.org, Jacky Bai
  Cc: Abel Vesa, Anson Huang, shawnguo@kernel.org,
	mturquette@baylibre.com, linux-kernel@vger.kernel.org,
	linux-clk@vger.kernel.org, dl-linux-imx, kernel@pengutronix.de,
	festevam@gmail.com, s.hauer@pengutronix.de,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <VI1PR04MB7023C1017F60BF132B6A3F8CEEA50@VI1PR04MB7023.eurprd04.prod.outlook.com>

> Subject: Re: [PATCH] clk: imx: pll14xx: avoid glitch when set rate
> 
> On 20.08.2019 05:17, Peng Fan wrote:
> > According to PLL1443XA and PLL1416X spec, "When BYPASS is 0 and RESETB
> > is changed from 0 to 1, FOUT starts to output unstable clock until
> > lock time passes. PLL1416X/PLL1443XA may generate a glitch at FOUT."
> >
> > So set BYPASS when RESETB is changed from 0 to 1 to avoid glitch.
> > In the end of set rate, BYPASS will be cleared.
> >
> > @@ -191,6 +191,10 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw,
> unsigned long drate,
> >   	tmp &= ~RST_MASK;
> >   	writel_relaxed(tmp, pll->base);
> >
> > +	/* Enable BYPASS */
> > +	tmp |= BYPASS_MASK;
> > +	writel(tmp, pll->base);
> > +
> 
> Shouldn't BYPASS be set before reset?

No. the glitch happens when RESET changes from 0 to 1, not from 1 to 0.

> 
> Also, isn't a similar bypass/unbypass dance also needed in
> clk_pll14xx_prepare? As far as I understand that could also output glitches
> until the PLL is locked. It could be a separate patch.

Yes, that might also output glitch. Fix in v2.

> 
> It's strange that this BYPASS bit is also handled by muxes like
> audio_pll1_bypass in clk-imx8mm.c but that's a separate issue not strictly
> related to the glitches you're trying to fix here.

Yes, need use EXT_BYPASS for the mux usage.

Regards,
Peng.

> 
> --
> Regards,
> Leonard

_______________________________________________
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 1/4] bindings: rtc: add bindings for MT2712 RTC
From: Matthias Brugger @ 2019-08-22  9:17 UTC (permalink / raw)
  To: Ran Bi, Alexandre Belloni, Rob Herring
  Cc: Mark Rutland, Alessandro Zummo, Flora Fu, srv_heupstream,
	devicetree, Greg Kroah-Hartman, Linus Walleij, Sean Wang,
	linux-kernel, YT Shen, linux-mediatek, Jonathan Cameron,
	Mauro Carvalho Chehab, Yingjoe Chen, Eddie Huang,
	David S . Miller, linux-arm-kernel, linux-rtc
In-Reply-To: <20190801110122.26834-2-ran.bi@mediatek.com>



On 01/08/2019 13:01, Ran Bi wrote:
> Document the binding for MT2712 RTC implemented by rtc-mt2712.
> 
> Signed-off-by: Ran Bi <ran.bi@mediatek.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  .../devicetree/bindings/rtc/rtc-mt2712.txt         | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-mt2712.txt
> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-mt2712.txt b/Documentation/devicetree/bindings/rtc/rtc-mt2712.txt
> new file mode 100644
> index 000000000000..c33d87e5e753
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/rtc-mt2712.txt
> @@ -0,0 +1,14 @@
> +Device-Tree bindings for MediaTek SoC based RTC
> +
> +Required properties:
> +- compatible	    : Should be "mediatek,mt2712-rtc" : for MT2712 SoC
> +- reg 		    : Specifies base physical address and size of the registers;
> +- interrupts	    : Should contain the interrupt for RTC alarm;

No clocks for the RTC? What about CLK_TOP_RTC_SEL from the clk driver?

Regards,
Matthias

> +
> +Example:
> +
> +rtc: rtc@10011000 {
> +	compatible = "mediatek,mt2712-rtc";
> +	reg = <0 0x10011000 0 0x1000>;
> +	interrupts = <GIC_SPI 239 IRQ_TYPE_LEVEL_LOW>;
> +};
> 

_______________________________________________
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 4/6] arm64: dts: meson: sei510: Add minimal thermal zone
From: Neil Armstrong @ 2019-08-22  9:15 UTC (permalink / raw)
  To: Kevin Hilman, Guillaume La Roque, rui.zhang, edubezval,
	daniel.lezcano
  Cc: devicetree, linux-pm, linux-kernel, linux-arm-kernel,
	linux-amlogic
In-Reply-To: <7hsgpu5c7j.fsf@baylibre.com>

On 22/08/2019 01:29, Kevin Hilman wrote:
> Guillaume La Roque <glaroque@baylibre.com> writes:
> 
>> Add minimal thermal zone for two temperature sensor
>> One is located close to the DDR and the other one is
>> located close to the PLLs (between the CPU and GPU)
>>
>> Signed-off-by: Guillaume La Roque <glaroque@baylibre.com>
>> Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> ---
>>  .../boot/dts/amlogic/meson-g12a-sei510.dts    | 70 +++++++++++++++++++
>>  1 file changed, 70 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
>> index c9fa23a56562..35d2ebbd6d4e 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
>> +++ b/arch/arm64/boot/dts/amlogic/meson-g12a-sei510.dts
>> @@ -10,6 +10,7 @@
>>  #include <dt-bindings/input/input.h>
>>  #include <dt-bindings/gpio/meson-g12a-gpio.h>
>>  #include <dt-bindings/sound/meson-g12a-tohdmitx.h>
>> +#include <dt-bindings/thermal/thermal.h>
>>  
>>  / {
>>  	compatible = "seirobotics,sei510", "amlogic,g12a";
>> @@ -33,6 +34,67 @@
>>  		ethernet0 = &ethmac;
>>  	};
>>  
>> +	thermal-zones {
>> +		cpu-thermal {
>> +			polling-delay = <1000>;
>> +			polling-delay-passive = <100>;
>> +			thermal-sensors = <&cpu_temp>;
>> +
>> +			trips {
>> +				cpu_hot: cpu-hot {
>> +					temperature = <85000>; /* millicelsius */
>> +					hysteresis = <2000>; /* millicelsius */
>> +					type = "hot";
>> +				};
>> +
>> +				cpu_critical: cpu-critical {
>> +					temperature = <110000>; /* millicelsius */
>> +					hysteresis = <2000>; /* millicelsius */
>> +					type = "critical";
>> +				};
>> +			};
>> +
>> +			cooling-maps {
>> +				map0 {
>> +					trip = <&cpu_hot>;
>> +					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> +							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> +							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> +							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>> +				};
>> +
>> +				map1 {
>> +					trip = <&cpu_critical>;
>> +					cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> +							 <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> +							 <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
>> +							 <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>> +				};
>> +			};
>> +		};
>> +
>> +		ddr-thermal {
>> +			polling-delay = <1000>;
>> +			polling-delay-passive = <100>;
>> +			thermal-sensors = <&ddr_temp>;
>> +
>> +			trips {
>> +				ddr_critical: ddr-critical {
>> +					temperature = <110000>; /* millicelsius */
>> +					hysteresis = <2000>; /* millicelsius */
>> +					type = "critical";
>> +				};
>> +			};
>> +
>> +			cooling-maps {
>> +				map {
>> +					trip = <&ddr_critical>;
>> +					cooling-device = <&mali THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
>> +				};
>> +			};
>> +		};
>> +	};
>> +
>>  	mono_dac: audio-codec-0 {
>>  		compatible = "maxim,max98357a";
>>  		#sound-dai-cells = <0>;
>> @@ -321,6 +383,7 @@
>>  	operating-points-v2 = <&cpu_opp_table>;
>>  	clocks = <&clkc CLKID_CPU_CLK>;
>>  	clock-latency = <50000>;
>> +	#cooling-cells = <2>;
>>  };
>>  
>>  &cpu1 {
>> @@ -328,6 +391,7 @@
>>  	operating-points-v2 = <&cpu_opp_table>;
>>  	clocks = <&clkc CLKID_CPU_CLK>;
>>  	clock-latency = <50000>;
>> +	#cooling-cells = <2>;
>>  };
>>  
>>  &cpu2 {
>> @@ -335,6 +399,7 @@
>>  	operating-points-v2 = <&cpu_opp_table>;
>>  	clocks = <&clkc CLKID_CPU_CLK>;
>>  	clock-latency = <50000>;
>> +	#cooling-cells = <2>;
>>  };
>>  
>>  &cpu3 {
>> @@ -342,6 +407,7 @@
>>  	operating-points-v2 = <&cpu_opp_table>;
>>  	clocks = <&clkc CLKID_CPU_CLK>;
>>  	clock-latency = <50000>;
>> +	#cooling-cells = <2>;
>>  };
>>  
>>  &cvbs_vdac_port {
>> @@ -368,6 +434,10 @@
>>  	status = "okay";
>>  };
>>  
>> +&mali {
>> +	#cooling-cells = <2>;
>> +};
>> +
> 
> Is there a reason these #cooling-cells properties belong in the SoC
> .dtsi and not the board .dts.  Seems like you'll have to repeat this in
> every board .dts which doesn't seem necessary.

I asked him to keep the cooling-cells in the boards until we add the thermal
in all the remaining boards.

Seemed to be safer way at the time...

Neil

> 
> Same comment for patch 5/6
> 
> Kevin
> 
> _______________________________________________
> linux-amlogic mailing list
> linux-amlogic@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-amlogic
> 


_______________________________________________
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] watchdog/aspeed: add support for dual boot
From: Ivan Mikhaylov @ 2019-08-22  9:15 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-watchdog, linux-aspeed, Andrew Jeffery, Alexander Amelkin,
	linux-kernel, Joel Stanley, Wim Van Sebroeck, linux-arm-kernel
In-Reply-To: <20190821163220.GA11547@roeck-us.net>

On Wed, 2019-08-21 at 09:32 -0700, Guenter Roeck wrote:
> 
> > +	writel(WDT_CLEAR_TIMEOUT_AND_BOOT_CODE_SELECTION,
> > +			wdt->base + WDT_CLEAR_TIMEOUT_STATUS);
> > +	wdt->wdd.bootstatus |= WDIOF_EXTERN1;
> 
> The variable reflects the _boot status_. It should not change after booting.

Okay, then perhaps may we set 'status' handler for watchdog device and check 
'status' file? Right now 'bootstatus' and 'status' are same because there is no
handler for 'status'.

> > +
> > +	return size;
> > +}
> > +static DEVICE_ATTR_WO(access_cs0);
> > +
> > +static struct attribute *bswitch_attrs[] = {
> > +	&dev_attr_access_cs0.attr,
> > +	NULL
> > +};
> > +ATTRIBUTE_GROUPS(bswitch);
> > +
> >  static const struct watchdog_ops aspeed_wdt_ops = {
> >  	.start		= aspeed_wdt_start,
> >  	.stop		= aspeed_wdt_stop,
> > @@ -223,6 +248,9 @@ static int aspeed_wdt_probe(struct platform_device
> > *pdev)
> >  
> >  	wdt->ctrl = WDT_CTRL_1MHZ_CLK;
> >  
> > +	if (of_property_read_bool(np, "aspeed,alt-boot"))
> > +		wdt->wdd.groups = bswitch_groups;
> > +
> Why does this have to be separate to the existing evaluation of
> aspeed,alt-boot, and why does the existing code not work ?
> 
> Also, is it guaranteed that this does not interfer with existing
> support for alt-boot ?

It doesn't, it just provides for ast2400 switch to cs0 at side 1(cs1). Problem
is that only one flash chip(side 1/cs1) is accessible on alternate boot, there
is citation from the documentation in commit body. So if by some reason side 0
is corrupted, need to switch into alternate boot to cs1, do the load from it,
drop that bit to make side 0 accessible and do the flash of first side. On
ast2500/2600 this problem is solved already, in alternate boot there both flash
chips are present. It's additional requirement for alternate boot on ast2400, to
make the possibility to access at all side 0 flash chip after we boot to the
alternate side.


_______________________________________________
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] drm: meson: use match data to detect vpu compatibility
From: Neil Armstrong @ 2019-08-22  9:12 UTC (permalink / raw)
  To: Julien Masson, Kevin Hilman
  Cc: linux-amlogic, dri-devel, linux-arm-kernel, linux-kernel
In-Reply-To: <87o90hzi3x.fsf@masson.i-did-not-set--mail-host-address--so-tickle-me>

Hi Julien,

On 22/08/2019 11:03, Julien Masson wrote:
> This patch introduce new enum which contains all VPU family (GXBB,
> GXL, GXM and G12A).
> This enum is used to detect the VPU compatible with the device.
> 
> We only need to set .data to the corresponding enum in the device
> table, no need to check .compatible string anymore.
> 
> Signed-off-by: Julien Masson <jmasson@baylibre.com>
> ---
>  drivers/gpu/drm/meson/meson_crtc.c      |  2 +-
>  drivers/gpu/drm/meson/meson_drv.c       | 12 +++--
>  drivers/gpu/drm/meson/meson_drv.h       | 15 +++++-
>  drivers/gpu/drm/meson/meson_dw_hdmi.c   |  2 +-
>  drivers/gpu/drm/meson/meson_overlay.c   |  2 +-
>  drivers/gpu/drm/meson/meson_plane.c     | 10 ++--
>  drivers/gpu/drm/meson/meson_vclk.c      | 64 ++++++++++++-------------
>  drivers/gpu/drm/meson/meson_venc.c      |  2 +-
>  drivers/gpu/drm/meson/meson_venc_cvbs.c | 10 ++--
>  drivers/gpu/drm/meson/meson_viu.c       | 10 ++--
>  drivers/gpu/drm/meson/meson_vpp.c       | 10 ++--
>  11 files changed, 77 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c
> index bba25325aa9c..57ae1c13d1e6 100644
> --- a/drivers/gpu/drm/meson/meson_crtc.c
> +++ b/drivers/gpu/drm/meson/meson_crtc.c
> @@ -575,7 +575,7 @@ int meson_crtc_create(struct meson_drm *priv)
>  		return ret;
>  	}
>  
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>  		meson_crtc->enable_osd1 = meson_g12a_crtc_enable_osd1;
>  		meson_crtc->enable_vd1 = meson_g12a_crtc_enable_vd1;
>  		meson_crtc->viu_offset = MESON_G12A_VIU_OFFSET;
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index ae0166181606..97e9945f66c0 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -380,10 +380,14 @@ static int compare_of(struct device *dev, void *data)
>  
>  /* Possible connectors nodes to ignore */
>  static const struct of_device_id connectors_match[] = {
> -	{ .compatible = "composite-video-connector" },
> -	{ .compatible = "svideo-connector" },
> -	{ .compatible = "hdmi-connector" },
> -	{ .compatible = "dvi-connector" },
> +	{ .compatible = "amlogic,meson-gxbb-vpu",
> +	  .data       = (void *)VPU_COMPATIBLE_GXBB },
> +	{ .compatible = "amlogic,meson-gxl-vpu",
> +	  .data       = (void *)VPU_COMPATIBLE_GXL },
> +	{ .compatible = "amlogic,meson-gxm-vpu",
> +	  .data       = (void *)VPU_COMPATIBLE_GXM },
> +	{ .compatible = "amlogic,meson-g12a-vpu",
> +	  .data       = (void *)VPU_COMPATIBLE_G12A },
>  	{}
>  };
>  
> diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
> index c9aaec1a846e..eab8c710c4e3 100644
> --- a/drivers/gpu/drm/meson/meson_drv.h
> +++ b/drivers/gpu/drm/meson/meson_drv.h
> @@ -9,6 +9,7 @@
>  
>  #include <linux/device.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
>  #include <linux/regmap.h>
>  
>  struct drm_crtc;
> @@ -16,6 +17,13 @@ struct drm_device;
>  struct drm_plane;
>  struct meson_drm;
>  
> +enum vpu_compatible {
> +	VPU_COMPATIBLE_GXBB = 0,
> +	VPU_COMPATIBLE_GXL  = 1,
> +	VPU_COMPATIBLE_GXM  = 2,
> +	VPU_COMPATIBLE_G12A = 3,
> +};
> +
>  struct meson_drm {
>  	struct device *dev;
>  	void __iomem *io_base;
> @@ -116,9 +124,12 @@ struct meson_drm {
>  };
>  
>  static inline int meson_vpu_is_compatible(struct meson_drm *priv,
> -					  const char *compat)
> +					  enum vpu_compatible family)
>  {
> -	return of_device_is_compatible(priv->dev->of_node, compat);
> +	enum vpu_compatible compat =
> +		(enum vpu_compatible)of_device_get_match_data(priv->dev);

Can you store the family into struct meson_drm at probe then check the variable here instead ?

Otherwise the rest looks fine.

Neil

> +
> +	return compat == family;
>  }
>  
>  #endif /* __MESON_DRV_H */
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index f893ebd0b799..68bbd987147b 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -937,7 +937,7 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>  	reset_control_reset(meson_dw_hdmi->hdmitx_phy);
>  
>  	/* Enable APB3 fail on error */
> -	if (!meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +	if (!meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>  		writel_bits_relaxed(BIT(15), BIT(15),
>  				    meson_dw_hdmi->hdmitx + HDMITX_TOP_CTRL_REG);
>  		writel_bits_relaxed(BIT(15), BIT(15),
> diff --git a/drivers/gpu/drm/meson/meson_overlay.c b/drivers/gpu/drm/meson/meson_overlay.c
> index 5aa9dcb4b35e..2468b0212d52 100644
> --- a/drivers/gpu/drm/meson/meson_overlay.c
> +++ b/drivers/gpu/drm/meson/meson_overlay.c
> @@ -513,7 +513,7 @@ static void meson_overlay_atomic_disable(struct drm_plane *plane,
>  	priv->viu.vd1_enabled = false;
>  
>  	/* Disable VD1 */
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>  		writel_relaxed(0, priv->io_base + _REG(VD1_BLEND_SRC_CTRL));
>  		writel_relaxed(0, priv->io_base + _REG(VD2_BLEND_SRC_CTRL));
>  		writel_relaxed(0, priv->io_base + _REG(VD1_IF0_GEN_REG + 0x17b0));
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index b9e1e117fb85..ed543227b00d 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -138,7 +138,7 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
>  				      OSD_ENDIANNESS_LE);
>  
>  	/* On GXBB, Use the old non-HDR RGB2YUV converter */
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
>  		priv->viu.osd1_blk0_cfg[0] |= OSD_OUTPUT_COLOR_RGB;
>  
>  	switch (fb->format->format) {
> @@ -292,7 +292,7 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
>  	priv->viu.osd1_blk0_cfg[3] = ((dest.x2 - 1) << 16) | dest.x1;
>  	priv->viu.osd1_blk0_cfg[4] = ((dest.y2 - 1) << 16) | dest.y1;
>  
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>  		priv->viu.osd_blend_din0_scope_h = ((dest.x2 - 1) << 16) | dest.x1;
>  		priv->viu.osd_blend_din0_scope_v = ((dest.y2 - 1) << 16) | dest.y1;
>  		priv->viu.osb_blend0_size = dst_h << 16 | dst_w;
> @@ -308,8 +308,8 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
>  
>  	if (!meson_plane->enabled) {
>  		/* Reset OSD1 before enabling it on GXL+ SoCs */
> -		if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> -		    meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
> +		if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> +		    meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
>  			meson_viu_osd1_reset(priv);
>  
>  		meson_plane->enabled = true;
> @@ -327,7 +327,7 @@ static void meson_plane_atomic_disable(struct drm_plane *plane,
>  	struct meson_drm *priv = meson_plane->priv;
>  
>  	/* Disable OSD1 */
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
>  		writel_bits_relaxed(VIU_OSD1_POSTBLD_SRC_OSD1, 0,
>  				    priv->io_base + _REG(OSD1_BLEND_SRC_CTRL));
>  	else
> diff --git a/drivers/gpu/drm/meson/meson_vclk.c b/drivers/gpu/drm/meson/meson_vclk.c
> index 869231c93617..ac491a781952 100644
> --- a/drivers/gpu/drm/meson/meson_vclk.c
> +++ b/drivers/gpu/drm/meson/meson_vclk.c
> @@ -242,7 +242,7 @@ static void meson_venci_cvbs_clock_config(struct meson_drm *priv)
>  	unsigned int val;
>  
>  	/* Setup PLL to output 1.485GHz */
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x5800023d);
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, 0x00404e00);
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0x0d5c5091);
> @@ -254,8 +254,8 @@ static void meson_venci_cvbs_clock_config(struct meson_drm *priv)
>  		/* Poll for lock bit */
>  		regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
>  					 (val & HDMI_PLL_LOCK), 10, 0);
> -	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> -		   meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
> +	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> +		   meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL)) {
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x4000027b);
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, 0x800cb300);
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0xa6212844);
> @@ -272,7 +272,7 @@ static void meson_venci_cvbs_clock_config(struct meson_drm *priv)
>  		/* Poll for lock bit */
>  		regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
>  					 (val & HDMI_PLL_LOCK), 10, 0);
> -	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x1a0504f7);
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, 0x00010000);
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0x00000000);
> @@ -300,7 +300,7 @@ static void meson_venci_cvbs_clock_config(struct meson_drm *priv)
>  				VCLK2_DIV_MASK, (55 - 1));
>  
>  	/* select vid_pll for vclk2 */
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
>  		regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL,
>  					VCLK2_SEL_MASK, (0 << VCLK2_SEL_SHIFT));
>  	else
> @@ -455,7 +455,7 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv, unsigned int m,
>  {
>  	unsigned int val;
>  
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x58000200 | m);
>  		if (frac)
>  			regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2,
> @@ -475,8 +475,8 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv, unsigned int m,
>  		/* Poll for lock bit */
>  		regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL,
>  					 val, (val & HDMI_PLL_LOCK), 10, 0);
> -	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> -		   meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
> +	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> +		   meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL)) {
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x40000200 | m);
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, 0x800cb000 | frac);
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0x860f30c4);
> @@ -493,7 +493,7 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv, unsigned int m,
>  		/* Poll for lock bit */
>  		regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
>  				(val & HDMI_PLL_LOCK), 10, 0);
> -	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>  		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x0b3a0400 | m);
>  
>  		/* Enable and reset */
> @@ -545,36 +545,36 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv, unsigned int m,
>  		} while(1);
>  	}
>  
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
>  		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL2,
>  				3 << 16, pll_od_to_reg(od1) << 16);
> -	else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> -		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
> +	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> +		 meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
>  		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL3,
>  				3 << 21, pll_od_to_reg(od1) << 21);
> -	else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> +	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
>  		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
>  				3 << 16, pll_od_to_reg(od1) << 16);
>  
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
>  		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL2,
>  				3 << 22, pll_od_to_reg(od2) << 22);
> -	else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> -		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
> +	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> +		 meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
>  		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL3,
>  				3 << 23, pll_od_to_reg(od2) << 23);
> -	else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> +	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
>  		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
>  				3 << 18, pll_od_to_reg(od2) << 18);
>  
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
>  		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL2,
>  				3 << 18, pll_od_to_reg(od3) << 18);
> -	else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> -		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
> +	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> +		 meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
>  		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL3,
>  				3 << 19, pll_od_to_reg(od3) << 19);
> -	else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> +	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
>  		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
>  				3 << 20, pll_od_to_reg(od3) << 20);
>  }
> @@ -585,7 +585,7 @@ static unsigned int meson_hdmi_pll_get_m(struct meson_drm *priv,
>  					 unsigned int pll_freq)
>  {
>  	/* The GXBB PLL has a /2 pre-multiplier */
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
>  		pll_freq /= 2;
>  
>  	return pll_freq / XTAL_FREQ;
> @@ -605,12 +605,12 @@ static unsigned int meson_hdmi_pll_get_frac(struct meson_drm *priv,
>  	unsigned int frac;
>  
>  	/* The GXBB PLL has a /2 pre-multiplier and a larger FRAC width */
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
>  		frac_max = HDMI_FRAC_MAX_GXBB;
>  		parent_freq *= 2;
>  	}
>  
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
>  		frac_max = HDMI_FRAC_MAX_G12A;
>  
>  	/* We can have a perfect match !*/
> @@ -631,15 +631,15 @@ static bool meson_hdmi_pll_validate_params(struct meson_drm *priv,
>  					   unsigned int m,
>  					   unsigned int frac)
>  {
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
>  		/* Empiric supported min/max dividers */
>  		if (m < 53 || m > 123)
>  			return false;
>  		if (frac >= HDMI_FRAC_MAX_GXBB)
>  			return false;
> -	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> -		   meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu") ||
> -		   meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> +		   meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL) ||
> +		   meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>  		/* Empiric supported min/max dividers */
>  		if (m < 106 || m > 247)
>  			return false;
> @@ -759,7 +759,7 @@ static void meson_vclk_set(struct meson_drm *priv, unsigned int pll_base_freq,
>  	/* Set HDMI PLL rate */
>  	if (!od1 && !od2 && !od3) {
>  		meson_hdmi_pll_generic_set(priv, pll_base_freq);
> -	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
> +	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
>  		switch (pll_base_freq) {
>  		case 2970000:
>  			m = 0x3d;
> @@ -776,8 +776,8 @@ static void meson_vclk_set(struct meson_drm *priv, unsigned int pll_base_freq,
>  		}
>  
>  		meson_hdmi_pll_set_params(priv, m, frac, od1, od2, od3);
> -	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> -		   meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
> +	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> +		   meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL)) {
>  		switch (pll_base_freq) {
>  		case 2970000:
>  			m = 0x7b;
> @@ -794,7 +794,7 @@ static void meson_vclk_set(struct meson_drm *priv, unsigned int pll_base_freq,
>  		}
>  
>  		meson_hdmi_pll_set_params(priv, m, frac, od1, od2, od3);
> -	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>  		switch (pll_base_freq) {
>  		case 2970000:
>  			m = 0x7b;
> diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c
> index 679d2274531c..4efd7864d5bf 100644
> --- a/drivers/gpu/drm/meson/meson_venc.c
> +++ b/drivers/gpu/drm/meson/meson_venc.c
> @@ -1759,7 +1759,7 @@ void meson_venc_disable_vsync(struct meson_drm *priv)
>  void meson_venc_init(struct meson_drm *priv)
>  {
>  	/* Disable CVBS VDAC */
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>  		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0);
>  		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 8);
>  	} else {
> diff --git a/drivers/gpu/drm/meson/meson_venc_cvbs.c b/drivers/gpu/drm/meson/meson_venc_cvbs.c
> index 6dc130a24070..9ab27aecfcf3 100644
> --- a/drivers/gpu/drm/meson/meson_venc_cvbs.c
> +++ b/drivers/gpu/drm/meson/meson_venc_cvbs.c
> @@ -155,7 +155,7 @@ static void meson_venc_cvbs_encoder_disable(struct drm_encoder *encoder)
>  	struct meson_drm *priv = meson_venc_cvbs->priv;
>  
>  	/* Disable CVBS VDAC */
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>  		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0);
>  		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 0);
>  	} else {
> @@ -174,14 +174,14 @@ static void meson_venc_cvbs_encoder_enable(struct drm_encoder *encoder)
>  	writel_bits_relaxed(VENC_VDAC_SEL_ATV_DMD, 0,
>  			    priv->io_base + _REG(VENC_VDAC_DACSEL0));
>  
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
>  		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 1);
>  		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
> -	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> -		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
> +	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> +		 meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL)) {
>  		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0xf0001);
>  		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
> -	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>  		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0x906001);
>  		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 0);
>  	}
> diff --git a/drivers/gpu/drm/meson/meson_viu.c b/drivers/gpu/drm/meson/meson_viu.c
> index e70cd55d56c9..68cf2c2eca5f 100644
> --- a/drivers/gpu/drm/meson/meson_viu.c
> +++ b/drivers/gpu/drm/meson/meson_viu.c
> @@ -353,10 +353,10 @@ void meson_viu_init(struct meson_drm *priv)
>  			    priv->io_base + _REG(VIU_OSD2_CTRL_STAT));
>  
>  	/* On GXL/GXM, Use the 10bit HDR conversion matrix */
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
> -	    meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
> +	    meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
>  		meson_viu_load_matrix(priv);
> -	else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> +	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
>  		meson_viu_set_g12a_osd1_matrix(priv, RGB709_to_YUV709l_coeff,
>  					       true);
>  
> @@ -367,7 +367,7 @@ void meson_viu_init(struct meson_drm *priv)
>  		VIU_OSD_WORDS_PER_BURST(4) | /* 4 words in 1 burst */
>  		VIU_OSD_FIFO_LIMITS(2);      /* fifo_lim: 2*16=32 */
>  
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
>  		reg |= meson_viu_osd_burst_length_reg(32);
>  	else
>  		reg |= meson_viu_osd_burst_length_reg(64);
> @@ -394,7 +394,7 @@ void meson_viu_init(struct meson_drm *priv)
>  	writel_relaxed(0x00FF00C0,
>  			priv->io_base + _REG(VD2_IF0_LUMA_FIFO_SIZE));
>  
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>  		writel_relaxed(VIU_OSD_BLEND_REORDER(0, 1) |
>  			       VIU_OSD_BLEND_REORDER(1, 0) |
>  			       VIU_OSD_BLEND_REORDER(2, 0) |
> diff --git a/drivers/gpu/drm/meson/meson_vpp.c b/drivers/gpu/drm/meson/meson_vpp.c
> index 1429f3be6028..154837688ab0 100644
> --- a/drivers/gpu/drm/meson/meson_vpp.c
> +++ b/drivers/gpu/drm/meson/meson_vpp.c
> @@ -91,20 +91,20 @@ static void meson_vpp_write_vd_scaling_filter_coefs(struct meson_drm *priv,
>  void meson_vpp_init(struct meson_drm *priv)
>  {
>  	/* set dummy data default YUV black */
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
>  		writel_relaxed(0x108080, priv->io_base + _REG(VPP_DUMMY_DATA1));
> -	else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu")) {
> +	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM)) {
>  		writel_bits_relaxed(0xff << 16, 0xff << 16,
>  				    priv->io_base + _REG(VIU_MISC_CTRL1));
>  		writel_relaxed(VPP_PPS_DUMMY_DATA_MODE,
>  			       priv->io_base + _REG(VPP_DOLBY_CTRL));
>  		writel_relaxed(0x1020080,
>  				priv->io_base + _REG(VPP_DUMMY_DATA1));
> -	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> +	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
>  		writel_relaxed(0xf, priv->io_base + _REG(DOLBY_PATH_CTRL));
>  
>  	/* Initialize vpu fifo control registers */
> -	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
> +	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
>  		writel_relaxed(VPP_OFIFO_SIZE_DEFAULT,
>  			       priv->io_base + _REG(VPP_OFIFO_SIZE));
>  	else
> @@ -113,7 +113,7 @@ void meson_vpp_init(struct meson_drm *priv)
>  	writel_relaxed(VPP_POSTBLEND_HOLD_LINES(4) | VPP_PREBLEND_HOLD_LINES(4),
>  		       priv->io_base + _REG(VPP_HOLD_LINES));
>  
> -	if (!meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
> +	if (!meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
>  		/* Turn off preblend */
>  		writel_bits_relaxed(VPP_PREBLEND_ENABLE, 0,
>  				    priv->io_base + _REG(VPP_MISC));
> 

_______________________________________________
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/4] rtc: Add support for the MediaTek MT2712 RTC
From: Matthias Brugger @ 2019-08-22  9:12 UTC (permalink / raw)
  To: Ran Bi, Alexandre Belloni, Rob Herring
  Cc: Mark Rutland, Alessandro Zummo, Flora Fu, srv_heupstream,
	devicetree, Greg Kroah-Hartman, Linus Walleij, Sean Wang,
	linux-kernel, YT Shen, linux-mediatek, Jonathan Cameron,
	Mauro Carvalho Chehab, Yingjoe Chen, Eddie Huang,
	David S . Miller, linux-arm-kernel, linux-rtc
In-Reply-To: <20190801110122.26834-3-ran.bi@mediatek.com>



On 01/08/2019 13:01, Ran Bi wrote:
> This add support for the MediaTek MT2712 RTC. It was SoC based RTC, but
> had different architecture compared with MT7622 RTC.
> 
> Signed-off-by: Ran Bi <ran.bi@mediatek.com>
> ---
>  drivers/rtc/Kconfig      |  10 +
>  drivers/rtc/Makefile     |   1 +
>  drivers/rtc/rtc-mt2712.c | 444 +++++++++++++++++++++++++++++++++++++++

Can't we just adjust rtc-mt7622.c (and rename it) to unify the source for both
devices. What is the difference that we need to write a driver of our own?

Regards,
Matthias

>  3 files changed, 455 insertions(+)
>  create mode 100644 drivers/rtc/rtc-mt2712.c
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index e72f65b61176..977d0f480dc7 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1772,6 +1772,16 @@ config RTC_DRV_MOXART
>  	   This driver can also be built as a module. If so, the module
>  	   will be called rtc-moxart
>  
> +config RTC_DRV_MT2712
> +	tristate "MediaTek MT2712 SoC based RTC"
> +	depends on ARCH_MEDIATEK || COMPILE_TEST
> +	help
> +	  This enables support for the real time clock built in the MediaTek
> +	  SoCs for MT2712.
> +
> +	  This drive can also be built as a module. If so, the module
> +	  will be called rtc-mt2712.
> +
>  config RTC_DRV_MT6397
>  	tristate "MediaTek PMIC based RTC"
>  	depends on MFD_MT6397 || (COMPILE_TEST && IRQ_DOMAIN)
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 6b09c21dc1b6..7c6cf70af281 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -108,6 +108,7 @@ obj-$(CONFIG_RTC_DRV_MESON)	+= rtc-meson.o
>  obj-$(CONFIG_RTC_DRV_MOXART)	+= rtc-moxart.o
>  obj-$(CONFIG_RTC_DRV_MPC5121)	+= rtc-mpc5121.o
>  obj-$(CONFIG_RTC_DRV_MSM6242)	+= rtc-msm6242.o
> +obj-$(CONFIG_RTC_DRV_MT2712)	+= rtc-mt2712.o
>  obj-$(CONFIG_RTC_DRV_MT6397)	+= rtc-mt6397.o
>  obj-$(CONFIG_RTC_DRV_MT7622)	+= rtc-mt7622.o
>  obj-$(CONFIG_RTC_DRV_MV)	+= rtc-mv.o
> diff --git a/drivers/rtc/rtc-mt2712.c b/drivers/rtc/rtc-mt2712.c
> new file mode 100644
> index 000000000000..1eb71ca64c2c
> --- /dev/null
> +++ b/drivers/rtc/rtc-mt2712.c
> @@ -0,0 +1,444 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019 MediaTek Inc.
> + * Author: Ran Bi <ran.bi@mediatek.com>
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/rtc.h>
> +
> +#define MTK_RTC_DEV		KBUILD_MODNAME
> +
> +#define MT2712_BBPU		0x0000
> +#define MT2712_BBPU_CLRPKY	BIT(4)
> +#define MT2712_BBPU_RELOAD	BIT(5)
> +#define MT2712_BBPU_CBUSY	BIT(6)
> +#define MT2712_BBPU_KEY		(0x43 << 8)
> +
> +#define MT2712_IRQ_STA		0x0004
> +#define MT2712_IRQ_STA_AL	BIT(0)
> +#define MT2712_IRQ_STA_TC	BIT(1)
> +
> +#define MT2712_IRQ_EN		0x0008
> +#define MT2712_IRQ_EN_AL	BIT(0)
> +#define MT2712_IRQ_EN_TC	BIT(1)
> +#define MT2712_IRQ_EN_ONESHOT	BIT(2)
> +#define MT2712_IRQ_EN_ONESHOT_AL \
> +				(MT2712_IRQ_EN_ONESHOT | MT2712_IRQ_EN_AL)
> +
> +#define MT2712_CII_EN		0x000c
> +
> +#define MT2712_AL_MASK		0x0010
> +#define MT2712_AL_MASK_DOW	BIT(4)
> +
> +#define MT2712_TC_SEC		0x0014
> +#define MT2712_TC_MIN		0x0018
> +#define MT2712_TC_HOU		0x001c
> +#define MT2712_TC_DOM		0x0020
> +#define MT2712_TC_DOW		0x0024
> +#define MT2712_TC_MTH		0x0028
> +#define MT2712_TC_YEA		0x002c
> +
> +#define MT2712_AL_SEC		0x0030
> +#define MT2712_AL_MIN		0x0034
> +#define MT2712_AL_HOU		0x0038
> +#define MT2712_AL_DOM		0x003c
> +#define MT2712_AL_DOW		0x0040
> +#define MT2712_AL_MTH		0x0044
> +#define MT2712_AL_YEA		0x0048
> +
> +#define MT2712_SEC_MASK		0x003f
> +#define MT2712_MIN_MASK		0x003f
> +#define MT2712_HOU_MASK		0x001f
> +#define MT2712_DOM_MASK		0x001f
> +#define MT2712_DOW_MASK		0x0007
> +#define MT2712_MTH_MASK		0x000f
> +#define MT2712_YEA_MASK		0x007f
> +
> +#define MT2712_POWERKEY1	0x004c
> +#define MT2712_POWERKEY2	0x0050
> +#define MT2712_POWERKEY1_KEY	0xa357
> +#define MT2712_POWERKEY2_KEY	0x67d2
> +
> +#define MT2712_CON0		0x005c
> +#define MT2712_CON1		0x0060
> +
> +#define MT2712_PROT		0x0070
> +#define MT2712_PROT_UNLOCK1	0x9136
> +#define MT2712_PROT_UNLOCK2	0x586a
> +
> +#define MT2712_WRTGR		0x0078
> +
> +/* we map HW YEAR 0 to 2000 because 2000 is the leap year */
> +#define MT2712_MIN_YEAR		2000
> +#define MT2712_BASE_YEAR	1900
> +#define MT2712_MIN_YEAR_OFFSET	(MT2712_MIN_YEAR - MT2712_BASE_YEAR)
> +#define MT2712_MAX_YEAR_OFFSET	(MT2712_MIN_YEAR_OFFSET + 127)
> +
> +struct mt2712_rtc {
> +	struct device		*dev;
> +	struct rtc_device	*rtc_dev;
> +	void __iomem		*base;
> +	int			irq;
> +	u8			irq_wake_enabled;
> +};
> +
> +static inline u32 mt2712_readl(struct mt2712_rtc *rtc, u32 reg)
> +{
> +	return readl(rtc->base + reg);
> +}
> +
> +static inline void mt2712_writel(struct mt2712_rtc *rtc, u32 reg, u32 val)
> +{
> +	writel(val, rtc->base + reg);
> +}
> +
> +static void mt2712_rtc_write_trigger(struct mt2712_rtc *rtc)
> +{
> +	unsigned long timeout = jiffies + HZ/10;
> +
> +	mt2712_writel(rtc, MT2712_WRTGR, 1);
> +	while (1) {
> +		if (!(mt2712_readl(rtc, MT2712_BBPU) & MT2712_BBPU_CBUSY))
> +			break;
> +
> +		if (time_after(jiffies, timeout)) {
> +			dev_err(rtc->dev, "%s time out!\n", __func__);
> +			break;
> +		}
> +		cpu_relax();
> +	}
> +}
> +
> +static void mt2712_rtc_writeif_unlock(struct mt2712_rtc *rtc)
> +{
> +	mt2712_writel(rtc, MT2712_PROT, MT2712_PROT_UNLOCK1);
> +	mt2712_rtc_write_trigger(rtc);
> +	mt2712_writel(rtc, MT2712_PROT, MT2712_PROT_UNLOCK2);
> +	mt2712_rtc_write_trigger(rtc);
> +}
> +
> +static irqreturn_t rtc_irq_handler_thread(int irq, void *data)
> +{
> +	struct mt2712_rtc *rtc = data;
> +	u16 irqsta, irqen;
> +
> +	mutex_lock(&rtc->rtc_dev->ops_lock);
> +
> +	irqsta = mt2712_readl(rtc, MT2712_IRQ_STA);
> +	if (irqsta & MT2712_IRQ_STA_AL) {
> +		rtc_update_irq(rtc->rtc_dev, 1, RTC_IRQF | RTC_AF);
> +		irqen = irqsta & ~MT2712_IRQ_EN_AL;
> +
> +		mt2712_writel(rtc, MT2712_IRQ_EN, irqen);
> +		mt2712_rtc_write_trigger(rtc);
> +
> +		mutex_unlock(&rtc->rtc_dev->ops_lock);
> +		return IRQ_HANDLED;
> +	}
> +
> +	mutex_unlock(&rtc->rtc_dev->ops_lock);
> +	return IRQ_NONE;
> +}
> +
> +static void __mt2712_rtc_read_time(struct mt2712_rtc *rtc,
> +				   struct rtc_time *tm, int *sec)
> +{
> +	tm->tm_sec  = mt2712_readl(rtc, MT2712_TC_SEC) & MT2712_SEC_MASK;
> +	tm->tm_min  = mt2712_readl(rtc, MT2712_TC_MIN) & MT2712_MIN_MASK;
> +	tm->tm_hour = mt2712_readl(rtc, MT2712_TC_HOU) & MT2712_HOU_MASK;
> +	tm->tm_mday = mt2712_readl(rtc, MT2712_TC_DOM) & MT2712_DOM_MASK;
> +	tm->tm_mon  = mt2712_readl(rtc, MT2712_TC_MTH) & MT2712_MTH_MASK;
> +	tm->tm_year = mt2712_readl(rtc, MT2712_TC_YEA) & MT2712_YEA_MASK;
> +
> +	*sec = mt2712_readl(rtc, MT2712_TC_SEC) & MT2712_SEC_MASK;
> +}
> +
> +static int mt2712_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	struct mt2712_rtc *rtc = dev_get_drvdata(dev);
> +	int sec;
> +
> +	do {
> +		__mt2712_rtc_read_time(rtc, tm, &sec);
> +	} while (sec < tm->tm_sec);	/* SEC has carried */
> +
> +	/* HW register use 7 bits to store year data, minus
> +	 * MT2712_MIN_YEAR_OFFSET brfore write year data to register, and plus
> +	 * MT2712_MIN_YEAR_OFFSET back after read year from register
> +	 */
> +	tm->tm_year += MT2712_MIN_YEAR_OFFSET;
> +
> +	/* HW register start mon from one, but tm_mon start from zero. */
> +	tm->tm_mon--;
> +
> +	if (rtc_valid_tm(tm)) {
> +		dev_dbg(rtc->dev, "%s: invalid time %ptR\n", __func__, tm);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mt2712_rtc_set_time(struct device *dev, struct rtc_time *tm)
> +{
> +	struct mt2712_rtc *rtc = dev_get_drvdata(dev);
> +
> +	if (tm->tm_year > MT2712_MAX_YEAR_OFFSET) {
> +		dev_dbg(rtc->dev, "Set year %d out of range. (%d - %d)\n",
> +			1900 + tm->tm_year, 1900 + MT2712_MIN_YEAR_OFFSET,
> +			1900 + MT2712_MAX_YEAR_OFFSET);
> +		return -EINVAL;
> +	}
> +
> +	tm->tm_year -= MT2712_MIN_YEAR_OFFSET;
> +	tm->tm_mon++;
> +
> +	mt2712_writel(rtc, MT2712_TC_SEC, tm->tm_sec  & MT2712_SEC_MASK);
> +	mt2712_writel(rtc, MT2712_TC_MIN, tm->tm_min  & MT2712_MIN_MASK);
> +	mt2712_writel(rtc, MT2712_TC_HOU, tm->tm_hour & MT2712_HOU_MASK);
> +	mt2712_writel(rtc, MT2712_TC_DOM, tm->tm_mday & MT2712_DOM_MASK);
> +	mt2712_writel(rtc, MT2712_TC_MTH, tm->tm_mon  & MT2712_MTH_MASK);
> +	mt2712_writel(rtc, MT2712_TC_YEA, tm->tm_year & MT2712_YEA_MASK);
> +
> +	mt2712_rtc_write_trigger(rtc);
> +
> +	return 0;
> +}
> +
> +static int mt2712_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
> +{
> +	struct mt2712_rtc *rtc = dev_get_drvdata(dev);
> +	struct rtc_time *tm = &alm->time;
> +	u16 irqen;
> +
> +	irqen = mt2712_readl(rtc, MT2712_IRQ_EN);
> +	alm->enabled = !!(irqen & MT2712_IRQ_EN_AL);
> +
> +	tm->tm_sec  = mt2712_readl(rtc, MT2712_AL_SEC) & MT2712_SEC_MASK;
> +	tm->tm_min  = mt2712_readl(rtc, MT2712_AL_MIN) & MT2712_MIN_MASK;
> +	tm->tm_hour = mt2712_readl(rtc, MT2712_AL_HOU) & MT2712_HOU_MASK;
> +	tm->tm_mday = mt2712_readl(rtc, MT2712_AL_DOM) & MT2712_DOM_MASK;
> +	tm->tm_mon  = mt2712_readl(rtc, MT2712_AL_MTH) & MT2712_MTH_MASK;
> +	tm->tm_year = mt2712_readl(rtc, MT2712_AL_YEA) & MT2712_YEA_MASK;
> +
> +	tm->tm_year += MT2712_MIN_YEAR_OFFSET;
> +	tm->tm_mon--;
> +
> +	if (rtc_valid_tm(tm)) {
> +		dev_dbg(rtc->dev, "%s: invalid alarm %ptR\n", __func__, tm);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mt2712_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
> +{
> +	struct mt2712_rtc *rtc = dev_get_drvdata(dev);
> +	struct rtc_time *tm = &alm->time;
> +	u16 irqen;
> +
> +	if (tm->tm_year > MT2712_MAX_YEAR_OFFSET) {
> +		dev_dbg(rtc->dev, "Set year %d out of range. (%d - %d)\n",
> +			1900 + tm->tm_year, 1900 + MT2712_MIN_YEAR_OFFSET,
> +			1900 + MT2712_MAX_YEAR_OFFSET);
> +		return -EINVAL;
> +	}
> +
> +	dev_dbg(rtc->dev, "set al time: %ptR, alm en: %d\n", tm, alm->enabled);
> +
> +	tm->tm_year -= MT2712_MIN_YEAR_OFFSET;
> +	tm->tm_mon++;
> +
> +	irqen = mt2712_readl(rtc, MT2712_IRQ_EN) & ~(MT2712_IRQ_EN_ONESHOT_AL);
> +	mt2712_writel(rtc, MT2712_IRQ_EN, irqen);
> +	mt2712_rtc_write_trigger(rtc);
> +
> +	mt2712_writel(rtc, MT2712_AL_SEC,
> +		      (mt2712_readl(rtc, MT2712_AL_SEC) & ~(MT2712_SEC_MASK)) |
> +		      (tm->tm_sec  & MT2712_SEC_MASK));
> +	mt2712_writel(rtc, MT2712_AL_MIN,
> +		      (mt2712_readl(rtc, MT2712_AL_MIN) & ~(MT2712_MIN_MASK)) |
> +		      (tm->tm_min  & MT2712_MIN_MASK));
> +	mt2712_writel(rtc, MT2712_AL_HOU,
> +		      (mt2712_readl(rtc, MT2712_AL_HOU) & ~(MT2712_HOU_MASK)) |
> +		      (tm->tm_hour & MT2712_HOU_MASK));
> +	mt2712_writel(rtc, MT2712_AL_DOM,
> +		      (mt2712_readl(rtc, MT2712_AL_DOM) & ~(MT2712_DOM_MASK)) |
> +		      (tm->tm_mday & MT2712_DOM_MASK));
> +	mt2712_writel(rtc, MT2712_AL_MTH,
> +		      (mt2712_readl(rtc, MT2712_AL_MTH) & ~(MT2712_MTH_MASK)) |
> +		      (tm->tm_mon  & MT2712_MTH_MASK));
> +	mt2712_writel(rtc, MT2712_AL_YEA,
> +		      (mt2712_readl(rtc, MT2712_AL_YEA) & ~(MT2712_YEA_MASK)) |
> +		      (tm->tm_year & MT2712_YEA_MASK));
> +
> +	mt2712_writel(rtc, MT2712_AL_MASK, MT2712_AL_MASK_DOW);	/* mask DOW */
> +
> +	if (alm->enabled) {
> +		irqen = mt2712_readl(rtc, MT2712_IRQ_EN) |
> +				     MT2712_IRQ_EN_ONESHOT_AL;
> +		mt2712_writel(rtc, MT2712_IRQ_EN, irqen);
> +	} else {
> +		irqen = mt2712_readl(rtc, MT2712_IRQ_EN) &
> +				     ~(MT2712_IRQ_EN_ONESHOT_AL);
> +		mt2712_writel(rtc, MT2712_IRQ_EN, irqen);
> +	}
> +	mt2712_rtc_write_trigger(rtc);
> +
> +	return 0;
> +}
> +
> +/* Init RTC register */
> +static void mt2712_rtc_hw_init(struct mt2712_rtc *rtc)
> +{
> +	u32 p1, p2;
> +
> +	mt2712_writel(rtc, MT2712_BBPU, MT2712_BBPU_KEY | MT2712_BBPU_RELOAD);
> +
> +	mt2712_writel(rtc, MT2712_CII_EN, 0);
> +	mt2712_writel(rtc, MT2712_AL_MASK, 0);
> +	/* necessary before set MT2712_POWERKEY */
> +	mt2712_writel(rtc, MT2712_CON0, 0x4848);
> +	mt2712_writel(rtc, MT2712_CON1, 0x0048);
> +
> +	mt2712_rtc_write_trigger(rtc);
> +
> +	mt2712_readl(rtc, MT2712_IRQ_STA);	/* read clear */
> +
> +	p1 = mt2712_readl(rtc, MT2712_POWERKEY1);
> +	p2 = mt2712_readl(rtc, MT2712_POWERKEY2);
> +	if (p1 != MT2712_POWERKEY1_KEY || p2 != MT2712_POWERKEY2_KEY)
> +		dev_dbg(rtc->dev, "powerkey not set (lost power)\n");
> +
> +	/* RTC need POWERKEY1/2 match, then goto normal work mode */
> +	mt2712_writel(rtc, MT2712_POWERKEY1, MT2712_POWERKEY1_KEY);
> +	mt2712_writel(rtc, MT2712_POWERKEY2, MT2712_POWERKEY2_KEY);
> +	mt2712_rtc_write_trigger(rtc);
> +
> +	mt2712_rtc_writeif_unlock(rtc);
> +}
> +
> +static const struct rtc_class_ops mt2712_rtc_ops = {
> +	.read_time	= mt2712_rtc_read_time,
> +	.set_time	= mt2712_rtc_set_time,
> +	.read_alarm	= mt2712_rtc_read_alarm,
> +	.set_alarm	= mt2712_rtc_set_alarm,
> +};
> +
> +static int mt2712_rtc_probe(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	struct mt2712_rtc *rtc;
> +	int ret;
> +
> +	rtc = devm_kzalloc(&pdev->dev, sizeof(struct mt2712_rtc), GFP_KERNEL);
> +	if (!rtc)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	rtc->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(rtc->base))
> +		return PTR_ERR(rtc->base);
> +
> +	rtc->irq = platform_get_irq(pdev, 0);
> +	if (rtc->irq < 0) {
> +		dev_err(&pdev->dev, "No IRQ resource\n");
> +		return rtc->irq;
> +	}
> +
> +	rtc->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, rtc);
> +
> +	rtc->rtc_dev = devm_rtc_allocate_device(rtc->dev);
> +	if (IS_ERR(rtc->rtc_dev))
> +		return PTR_ERR(rtc->rtc_dev);
> +
> +	ret = devm_request_threaded_irq(&pdev->dev, rtc->irq, NULL,
> +					rtc_irq_handler_thread,
> +					IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> +					dev_name(rtc->dev), rtc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
> +			rtc->irq, ret);
> +		return ret;
> +	}
> +
> +	/* rtc hw init */
> +	mt2712_rtc_hw_init(rtc);
> +
> +	device_init_wakeup(&pdev->dev, true);
> +
> +	rtc->rtc_dev->ops = &mt2712_rtc_ops;
> +
> +	ret = rtc_register_device(rtc->rtc_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "register rtc device failed\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mt2712_rtc_suspend(struct device *dev)
> +{
> +	int wake_status = 0;
> +	struct mt2712_rtc *rtc = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev)) {
> +		wake_status = enable_irq_wake(rtc->irq);
> +		if (!wake_status)
> +			rtc->irq_wake_enabled = true;
> +	}
> +
> +	return 0;
> +}
> +
> +static int mt2712_rtc_resume(struct device *dev)
> +{
> +	int wake_status = 0;
> +	struct mt2712_rtc *rtc = dev_get_drvdata(dev);
> +
> +	if (device_may_wakeup(dev) && rtc->irq_wake_enabled) {
> +		wake_status = disable_irq_wake(rtc->irq);
> +		if (!wake_status)
> +			rtc->irq_wake_enabled = false;
> +	}
> +
> +	return 0;
> +}
> +
> +static SIMPLE_DEV_PM_OPS(mt2712_pm_ops, mt2712_rtc_suspend,
> +			 mt2712_rtc_resume);
> +#endif
> +
> +static const struct of_device_id mt2712_rtc_of_match[] = {
> +	{ .compatible = "mediatek,mt2712-rtc", },
> +	{ },
> +};
> +
> +MODULE_DEVICE_TABLE(of, mt2712_rtc_of_match)
> +
> +static struct platform_driver mt2712_rtc_driver = {
> +	.driver = {
> +		.name = MTK_RTC_DEV,
> +		.of_match_table = mt2712_rtc_of_match,
> +		.pm = &mt2712_pm_ops,
> +	},
> +	.probe  = mt2712_rtc_probe,
> +};
> +
> +module_platform_driver(mt2712_rtc_driver);
> +
> +MODULE_DESCRIPTION("MediaTek MT2712 SoC based RTC Driver");
> +MODULE_AUTHOR("Ran Bi <ran.bi@mediatek.com>");
> +MODULE_LICENSE("GPL");
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] drm: meson: use match data to detect vpu compatibility
From: Julien Masson @ 2019-08-22  9:03 UTC (permalink / raw)
  To: Kevin Hilman, Neil Armstrong
  Cc: Julien Masson, linux-amlogic, dri-devel, linux-arm-kernel,
	linux-kernel

This patch introduce new enum which contains all VPU family (GXBB,
GXL, GXM and G12A).
This enum is used to detect the VPU compatible with the device.

We only need to set .data to the corresponding enum in the device
table, no need to check .compatible string anymore.

Signed-off-by: Julien Masson <jmasson@baylibre.com>
---
 drivers/gpu/drm/meson/meson_crtc.c      |  2 +-
 drivers/gpu/drm/meson/meson_drv.c       | 12 +++--
 drivers/gpu/drm/meson/meson_drv.h       | 15 +++++-
 drivers/gpu/drm/meson/meson_dw_hdmi.c   |  2 +-
 drivers/gpu/drm/meson/meson_overlay.c   |  2 +-
 drivers/gpu/drm/meson/meson_plane.c     | 10 ++--
 drivers/gpu/drm/meson/meson_vclk.c      | 64 ++++++++++++-------------
 drivers/gpu/drm/meson/meson_venc.c      |  2 +-
 drivers/gpu/drm/meson/meson_venc_cvbs.c | 10 ++--
 drivers/gpu/drm/meson/meson_viu.c       | 10 ++--
 drivers/gpu/drm/meson/meson_vpp.c       | 10 ++--
 11 files changed, 77 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_crtc.c b/drivers/gpu/drm/meson/meson_crtc.c
index bba25325aa9c..57ae1c13d1e6 100644
--- a/drivers/gpu/drm/meson/meson_crtc.c
+++ b/drivers/gpu/drm/meson/meson_crtc.c
@@ -575,7 +575,7 @@ int meson_crtc_create(struct meson_drm *priv)
 		return ret;
 	}
 
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
 		meson_crtc->enable_osd1 = meson_g12a_crtc_enable_osd1;
 		meson_crtc->enable_vd1 = meson_g12a_crtc_enable_vd1;
 		meson_crtc->viu_offset = MESON_G12A_VIU_OFFSET;
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index ae0166181606..97e9945f66c0 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -380,10 +380,14 @@ static int compare_of(struct device *dev, void *data)
 
 /* Possible connectors nodes to ignore */
 static const struct of_device_id connectors_match[] = {
-	{ .compatible = "composite-video-connector" },
-	{ .compatible = "svideo-connector" },
-	{ .compatible = "hdmi-connector" },
-	{ .compatible = "dvi-connector" },
+	{ .compatible = "amlogic,meson-gxbb-vpu",
+	  .data       = (void *)VPU_COMPATIBLE_GXBB },
+	{ .compatible = "amlogic,meson-gxl-vpu",
+	  .data       = (void *)VPU_COMPATIBLE_GXL },
+	{ .compatible = "amlogic,meson-gxm-vpu",
+	  .data       = (void *)VPU_COMPATIBLE_GXM },
+	{ .compatible = "amlogic,meson-g12a-vpu",
+	  .data       = (void *)VPU_COMPATIBLE_G12A },
 	{}
 };
 
diff --git a/drivers/gpu/drm/meson/meson_drv.h b/drivers/gpu/drm/meson/meson_drv.h
index c9aaec1a846e..eab8c710c4e3 100644
--- a/drivers/gpu/drm/meson/meson_drv.h
+++ b/drivers/gpu/drm/meson/meson_drv.h
@@ -9,6 +9,7 @@
 
 #include <linux/device.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/regmap.h>
 
 struct drm_crtc;
@@ -16,6 +17,13 @@ struct drm_device;
 struct drm_plane;
 struct meson_drm;
 
+enum vpu_compatible {
+	VPU_COMPATIBLE_GXBB = 0,
+	VPU_COMPATIBLE_GXL  = 1,
+	VPU_COMPATIBLE_GXM  = 2,
+	VPU_COMPATIBLE_G12A = 3,
+};
+
 struct meson_drm {
 	struct device *dev;
 	void __iomem *io_base;
@@ -116,9 +124,12 @@ struct meson_drm {
 };
 
 static inline int meson_vpu_is_compatible(struct meson_drm *priv,
-					  const char *compat)
+					  enum vpu_compatible family)
 {
-	return of_device_is_compatible(priv->dev->of_node, compat);
+	enum vpu_compatible compat =
+		(enum vpu_compatible)of_device_get_match_data(priv->dev);
+
+	return compat == family;
 }
 
 #endif /* __MESON_DRV_H */
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index f893ebd0b799..68bbd987147b 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -937,7 +937,7 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 	reset_control_reset(meson_dw_hdmi->hdmitx_phy);
 
 	/* Enable APB3 fail on error */
-	if (!meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+	if (!meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
 		writel_bits_relaxed(BIT(15), BIT(15),
 				    meson_dw_hdmi->hdmitx + HDMITX_TOP_CTRL_REG);
 		writel_bits_relaxed(BIT(15), BIT(15),
diff --git a/drivers/gpu/drm/meson/meson_overlay.c b/drivers/gpu/drm/meson/meson_overlay.c
index 5aa9dcb4b35e..2468b0212d52 100644
--- a/drivers/gpu/drm/meson/meson_overlay.c
+++ b/drivers/gpu/drm/meson/meson_overlay.c
@@ -513,7 +513,7 @@ static void meson_overlay_atomic_disable(struct drm_plane *plane,
 	priv->viu.vd1_enabled = false;
 
 	/* Disable VD1 */
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
 		writel_relaxed(0, priv->io_base + _REG(VD1_BLEND_SRC_CTRL));
 		writel_relaxed(0, priv->io_base + _REG(VD2_BLEND_SRC_CTRL));
 		writel_relaxed(0, priv->io_base + _REG(VD1_IF0_GEN_REG + 0x17b0));
diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
index b9e1e117fb85..ed543227b00d 100644
--- a/drivers/gpu/drm/meson/meson_plane.c
+++ b/drivers/gpu/drm/meson/meson_plane.c
@@ -138,7 +138,7 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
 				      OSD_ENDIANNESS_LE);
 
 	/* On GXBB, Use the old non-HDR RGB2YUV converter */
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
 		priv->viu.osd1_blk0_cfg[0] |= OSD_OUTPUT_COLOR_RGB;
 
 	switch (fb->format->format) {
@@ -292,7 +292,7 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
 	priv->viu.osd1_blk0_cfg[3] = ((dest.x2 - 1) << 16) | dest.x1;
 	priv->viu.osd1_blk0_cfg[4] = ((dest.y2 - 1) << 16) | dest.y1;
 
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
 		priv->viu.osd_blend_din0_scope_h = ((dest.x2 - 1) << 16) | dest.x1;
 		priv->viu.osd_blend_din0_scope_v = ((dest.y2 - 1) << 16) | dest.y1;
 		priv->viu.osb_blend0_size = dst_h << 16 | dst_w;
@@ -308,8 +308,8 @@ static void meson_plane_atomic_update(struct drm_plane *plane,
 
 	if (!meson_plane->enabled) {
 		/* Reset OSD1 before enabling it on GXL+ SoCs */
-		if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
-		    meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
+		if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
+		    meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
 			meson_viu_osd1_reset(priv);
 
 		meson_plane->enabled = true;
@@ -327,7 +327,7 @@ static void meson_plane_atomic_disable(struct drm_plane *plane,
 	struct meson_drm *priv = meson_plane->priv;
 
 	/* Disable OSD1 */
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
 		writel_bits_relaxed(VIU_OSD1_POSTBLD_SRC_OSD1, 0,
 				    priv->io_base + _REG(OSD1_BLEND_SRC_CTRL));
 	else
diff --git a/drivers/gpu/drm/meson/meson_vclk.c b/drivers/gpu/drm/meson/meson_vclk.c
index 869231c93617..ac491a781952 100644
--- a/drivers/gpu/drm/meson/meson_vclk.c
+++ b/drivers/gpu/drm/meson/meson_vclk.c
@@ -242,7 +242,7 @@ static void meson_venci_cvbs_clock_config(struct meson_drm *priv)
 	unsigned int val;
 
 	/* Setup PLL to output 1.485GHz */
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x5800023d);
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, 0x00404e00);
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0x0d5c5091);
@@ -254,8 +254,8 @@ static void meson_venci_cvbs_clock_config(struct meson_drm *priv)
 		/* Poll for lock bit */
 		regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
 					 (val & HDMI_PLL_LOCK), 10, 0);
-	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
-		   meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
+	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
+		   meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL)) {
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x4000027b);
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, 0x800cb300);
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0xa6212844);
@@ -272,7 +272,7 @@ static void meson_venci_cvbs_clock_config(struct meson_drm *priv)
 		/* Poll for lock bit */
 		regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
 					 (val & HDMI_PLL_LOCK), 10, 0);
-	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x1a0504f7);
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, 0x00010000);
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0x00000000);
@@ -300,7 +300,7 @@ static void meson_venci_cvbs_clock_config(struct meson_drm *priv)
 				VCLK2_DIV_MASK, (55 - 1));
 
 	/* select vid_pll for vclk2 */
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
 		regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL,
 					VCLK2_SEL_MASK, (0 << VCLK2_SEL_SHIFT));
 	else
@@ -455,7 +455,7 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv, unsigned int m,
 {
 	unsigned int val;
 
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x58000200 | m);
 		if (frac)
 			regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2,
@@ -475,8 +475,8 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv, unsigned int m,
 		/* Poll for lock bit */
 		regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL,
 					 val, (val & HDMI_PLL_LOCK), 10, 0);
-	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
-		   meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
+	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
+		   meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL)) {
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x40000200 | m);
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL2, 0x800cb000 | frac);
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL3, 0x860f30c4);
@@ -493,7 +493,7 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv, unsigned int m,
 		/* Poll for lock bit */
 		regmap_read_poll_timeout(priv->hhi, HHI_HDMI_PLL_CNTL, val,
 				(val & HDMI_PLL_LOCK), 10, 0);
-	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
 		regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL, 0x0b3a0400 | m);
 
 		/* Enable and reset */
@@ -545,36 +545,36 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv, unsigned int m,
 		} while(1);
 	}
 
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
 		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL2,
 				3 << 16, pll_od_to_reg(od1) << 16);
-	else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
-		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
+	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
+		 meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
 		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL3,
 				3 << 21, pll_od_to_reg(od1) << 21);
-	else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
+	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
 		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
 				3 << 16, pll_od_to_reg(od1) << 16);
 
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
 		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL2,
 				3 << 22, pll_od_to_reg(od2) << 22);
-	else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
-		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
+	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
+		 meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
 		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL3,
 				3 << 23, pll_od_to_reg(od2) << 23);
-	else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
+	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
 		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
 				3 << 18, pll_od_to_reg(od2) << 18);
 
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
 		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL2,
 				3 << 18, pll_od_to_reg(od3) << 18);
-	else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
-		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
+	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
+		 meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
 		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL3,
 				3 << 19, pll_od_to_reg(od3) << 19);
-	else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
+	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
 		regmap_update_bits(priv->hhi, HHI_HDMI_PLL_CNTL,
 				3 << 20, pll_od_to_reg(od3) << 20);
 }
@@ -585,7 +585,7 @@ static unsigned int meson_hdmi_pll_get_m(struct meson_drm *priv,
 					 unsigned int pll_freq)
 {
 	/* The GXBB PLL has a /2 pre-multiplier */
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu"))
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB))
 		pll_freq /= 2;
 
 	return pll_freq / XTAL_FREQ;
@@ -605,12 +605,12 @@ static unsigned int meson_hdmi_pll_get_frac(struct meson_drm *priv,
 	unsigned int frac;
 
 	/* The GXBB PLL has a /2 pre-multiplier and a larger FRAC width */
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
 		frac_max = HDMI_FRAC_MAX_GXBB;
 		parent_freq *= 2;
 	}
 
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
 		frac_max = HDMI_FRAC_MAX_G12A;
 
 	/* We can have a perfect match !*/
@@ -631,15 +631,15 @@ static bool meson_hdmi_pll_validate_params(struct meson_drm *priv,
 					   unsigned int m,
 					   unsigned int frac)
 {
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
 		/* Empiric supported min/max dividers */
 		if (m < 53 || m > 123)
 			return false;
 		if (frac >= HDMI_FRAC_MAX_GXBB)
 			return false;
-	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
-		   meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu") ||
-		   meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
+		   meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL) ||
+		   meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
 		/* Empiric supported min/max dividers */
 		if (m < 106 || m > 247)
 			return false;
@@ -759,7 +759,7 @@ static void meson_vclk_set(struct meson_drm *priv, unsigned int pll_base_freq,
 	/* Set HDMI PLL rate */
 	if (!od1 && !od2 && !od3) {
 		meson_hdmi_pll_generic_set(priv, pll_base_freq);
-	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
+	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
 		switch (pll_base_freq) {
 		case 2970000:
 			m = 0x3d;
@@ -776,8 +776,8 @@ static void meson_vclk_set(struct meson_drm *priv, unsigned int pll_base_freq,
 		}
 
 		meson_hdmi_pll_set_params(priv, m, frac, od1, od2, od3);
-	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
-		   meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
+	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
+		   meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL)) {
 		switch (pll_base_freq) {
 		case 2970000:
 			m = 0x7b;
@@ -794,7 +794,7 @@ static void meson_vclk_set(struct meson_drm *priv, unsigned int pll_base_freq,
 		}
 
 		meson_hdmi_pll_set_params(priv, m, frac, od1, od2, od3);
-	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
 		switch (pll_base_freq) {
 		case 2970000:
 			m = 0x7b;
diff --git a/drivers/gpu/drm/meson/meson_venc.c b/drivers/gpu/drm/meson/meson_venc.c
index 679d2274531c..4efd7864d5bf 100644
--- a/drivers/gpu/drm/meson/meson_venc.c
+++ b/drivers/gpu/drm/meson/meson_venc.c
@@ -1759,7 +1759,7 @@ void meson_venc_disable_vsync(struct meson_drm *priv)
 void meson_venc_init(struct meson_drm *priv)
 {
 	/* Disable CVBS VDAC */
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
 		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0);
 		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 8);
 	} else {
diff --git a/drivers/gpu/drm/meson/meson_venc_cvbs.c b/drivers/gpu/drm/meson/meson_venc_cvbs.c
index 6dc130a24070..9ab27aecfcf3 100644
--- a/drivers/gpu/drm/meson/meson_venc_cvbs.c
+++ b/drivers/gpu/drm/meson/meson_venc_cvbs.c
@@ -155,7 +155,7 @@ static void meson_venc_cvbs_encoder_disable(struct drm_encoder *encoder)
 	struct meson_drm *priv = meson_venc_cvbs->priv;
 
 	/* Disable CVBS VDAC */
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
 		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0);
 		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 0);
 	} else {
@@ -174,14 +174,14 @@ static void meson_venc_cvbs_encoder_enable(struct drm_encoder *encoder)
 	writel_bits_relaxed(VENC_VDAC_SEL_ATV_DMD, 0,
 			    priv->io_base + _REG(VENC_VDAC_DACSEL0));
 
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxbb-vpu")) {
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXBB)) {
 		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 1);
 		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
-	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
-		 meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu")) {
+	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
+		 meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL)) {
 		regmap_write(priv->hhi, HHI_VDAC_CNTL0, 0xf0001);
 		regmap_write(priv->hhi, HHI_VDAC_CNTL1, 0);
-	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
 		regmap_write(priv->hhi, HHI_VDAC_CNTL0_G12A, 0x906001);
 		regmap_write(priv->hhi, HHI_VDAC_CNTL1_G12A, 0);
 	}
diff --git a/drivers/gpu/drm/meson/meson_viu.c b/drivers/gpu/drm/meson/meson_viu.c
index e70cd55d56c9..68cf2c2eca5f 100644
--- a/drivers/gpu/drm/meson/meson_viu.c
+++ b/drivers/gpu/drm/meson/meson_viu.c
@@ -353,10 +353,10 @@ void meson_viu_init(struct meson_drm *priv)
 			    priv->io_base + _REG(VIU_OSD2_CTRL_STAT));
 
 	/* On GXL/GXM, Use the 10bit HDR conversion matrix */
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu") ||
-	    meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
+	    meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
 		meson_viu_load_matrix(priv);
-	else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
+	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
 		meson_viu_set_g12a_osd1_matrix(priv, RGB709_to_YUV709l_coeff,
 					       true);
 
@@ -367,7 +367,7 @@ void meson_viu_init(struct meson_drm *priv)
 		VIU_OSD_WORDS_PER_BURST(4) | /* 4 words in 1 burst */
 		VIU_OSD_FIFO_LIMITS(2);      /* fifo_lim: 2*16=32 */
 
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
 		reg |= meson_viu_osd_burst_length_reg(32);
 	else
 		reg |= meson_viu_osd_burst_length_reg(64);
@@ -394,7 +394,7 @@ void meson_viu_init(struct meson_drm *priv)
 	writel_relaxed(0x00FF00C0,
 			priv->io_base + _REG(VD2_IF0_LUMA_FIFO_SIZE));
 
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
 		writel_relaxed(VIU_OSD_BLEND_REORDER(0, 1) |
 			       VIU_OSD_BLEND_REORDER(1, 0) |
 			       VIU_OSD_BLEND_REORDER(2, 0) |
diff --git a/drivers/gpu/drm/meson/meson_vpp.c b/drivers/gpu/drm/meson/meson_vpp.c
index 1429f3be6028..154837688ab0 100644
--- a/drivers/gpu/drm/meson/meson_vpp.c
+++ b/drivers/gpu/drm/meson/meson_vpp.c
@@ -91,20 +91,20 @@ static void meson_vpp_write_vd_scaling_filter_coefs(struct meson_drm *priv,
 void meson_vpp_init(struct meson_drm *priv)
 {
 	/* set dummy data default YUV black */
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-gxl-vpu"))
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL))
 		writel_relaxed(0x108080, priv->io_base + _REG(VPP_DUMMY_DATA1));
-	else if (meson_vpu_is_compatible(priv, "amlogic,meson-gxm-vpu")) {
+	else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM)) {
 		writel_bits_relaxed(0xff << 16, 0xff << 16,
 				    priv->io_base + _REG(VIU_MISC_CTRL1));
 		writel_relaxed(VPP_PPS_DUMMY_DATA_MODE,
 			       priv->io_base + _REG(VPP_DOLBY_CTRL));
 		writel_relaxed(0x1020080,
 				priv->io_base + _REG(VPP_DUMMY_DATA1));
-	} else if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
+	} else if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
 		writel_relaxed(0xf, priv->io_base + _REG(DOLBY_PATH_CTRL));
 
 	/* Initialize vpu fifo control registers */
-	if (meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu"))
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
 		writel_relaxed(VPP_OFIFO_SIZE_DEFAULT,
 			       priv->io_base + _REG(VPP_OFIFO_SIZE));
 	else
@@ -113,7 +113,7 @@ void meson_vpp_init(struct meson_drm *priv)
 	writel_relaxed(VPP_POSTBLEND_HOLD_LINES(4) | VPP_PREBLEND_HOLD_LINES(4),
 		       priv->io_base + _REG(VPP_HOLD_LINES));
 
-	if (!meson_vpu_is_compatible(priv, "amlogic,meson-g12a-vpu")) {
+	if (!meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
 		/* Turn off preblend */
 		writel_bits_relaxed(VPP_PREBLEND_ENABLE, 0,
 				    priv->io_base + _REG(VPP_MISC));
-- 
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] KVM: arm/arm64: vgic: Allow more than 256 vcpus for KVM_IRQ_LINE
From: Auger Eric @ 2019-08-22  9:00 UTC (permalink / raw)
  To: Marc Zyngier, Peter Maydell, James Morse, Julien Thierry,
	Suzuki K Poulose, Zenghui Yu
  Cc: qemu-arm, kvmarm, linux-arm-kernel, kvm
In-Reply-To: <20190818140710.23920-1-maz@kernel.org>

Hi Marc,

On 8/18/19 4:07 PM, Marc Zyngier wrote:
> While parts of the VGIC support a large number of vcpus (we
> bravely allow up to 512), other parts are more limited.
> 
> One of these limits is visible in the KVM_IRQ_LINE ioctl, which
> only allows 256 vcpus to be signalled when using the CPU or PPI
> types. Unfortunately, we've cornered ourselves badly by allocating
> all the bits in the irq field.
> 
> Since the irq_type subfield (8 bit wide) is currently only taking
> the values 0, 1 and 2 (and we have been careful not to allow anything
> else), let's reduce this field to only 4 bits, and allocate the
> remaining 4 bits to a vcpu2_index, which acts as a multiplier:
> 
>   vcpu_id = 256 * vcpu2_index + vcpu_index
> 
> With that, and a new capability (KVM_CAP_ARM_IRQ_LINE_LAYOUT_2)
> allowing this to be discovered, it becomes possible to inject
> PPIs to up to 4096 vcpus. But please just don't.
> 
> Reported-by: Zenghui Yu <yuzenghui@huawei.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  Documentation/virt/kvm/api.txt    | 8 ++++++--
>  arch/arm/include/uapi/asm/kvm.h   | 4 +++-
>  arch/arm64/include/uapi/asm/kvm.h | 4 +++-
>  include/uapi/linux/kvm.h          | 1 +
>  virt/kvm/arm/arm.c                | 2 ++
>  5 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/virt/kvm/api.txt b/Documentation/virt/kvm/api.txt
> index 2d067767b617..85518bfb2a99 100644
> --- a/Documentation/virt/kvm/api.txt
> +++ b/Documentation/virt/kvm/api.txt
> @@ -753,8 +753,8 @@ in-kernel irqchip (GIC), and for in-kernel irqchip can tell the GIC to
>  use PPIs designated for specific cpus.  The irq field is interpreted
>  like this:
>  
> -  bits:  | 31 ... 24 | 23  ... 16 | 15    ...    0 |
> -  field: | irq_type  | vcpu_index |     irq_id     |
> +  bits:  |  31 ... 28  | 27 ... 24 | 23  ... 16 | 15 ... 0 |
> +  field: | vcpu2_index | irq_type  | vcpu_index |  irq_id  |
>  
>  The irq_type field has the following values:
>  - irq_type[0]: out-of-kernel GIC: irq_id 0 is IRQ, irq_id 1 is FIQ
> @@ -766,6 +766,10 @@ The irq_type field has the following values:
>  
>  In both cases, level is used to assert/deassert the line.
>  
> +When KVM_CAP_ARM_IRQ_LINE_LAYOUT_2 is supported, the target vcpu is
> +identified as (256 * vcpu2_index + vcpu_index). Otherwise, vcpu2_index
> +must be zero.
> +
>  struct kvm_irq_level {
>  	union {
>  		__u32 irq;     /* GSI */
> diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h
> index a4217c1a5d01..2769360f195c 100644
> --- a/arch/arm/include/uapi/asm/kvm.h
> +++ b/arch/arm/include/uapi/asm/kvm.h
> @@ -266,8 +266,10 @@ struct kvm_vcpu_events {
>  #define   KVM_DEV_ARM_ITS_CTRL_RESET		4
>  
>  /* KVM_IRQ_LINE irq field index values */
> +#define KVM_ARM_IRQ_VCPU2_SHIFT		28
> +#define KVM_ARM_IRQ_VCPU2_MASK		0xf
>  #define KVM_ARM_IRQ_TYPE_SHIFT		24
> -#define KVM_ARM_IRQ_TYPE_MASK		0xff
> +#define KVM_ARM_IRQ_TYPE_MASK		0xf
>  #define KVM_ARM_IRQ_VCPU_SHIFT		16
>  #define KVM_ARM_IRQ_VCPU_MASK		0xff
>  #define KVM_ARM_IRQ_NUM_SHIFT		0
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 9a507716ae2f..67c21f9bdbad 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -325,8 +325,10 @@ struct kvm_vcpu_events {
>  #define   KVM_ARM_VCPU_TIMER_IRQ_PTIMER		1
>  
>  /* KVM_IRQ_LINE irq field index values */
> +#define KVM_ARM_IRQ_VCPU2_SHIFT		28
> +#define KVM_ARM_IRQ_VCPU2_MASK		0xf
>  #define KVM_ARM_IRQ_TYPE_SHIFT		24
> -#define KVM_ARM_IRQ_TYPE_MASK		0xff
> +#define KVM_ARM_IRQ_TYPE_MASK		0xf
>  #define KVM_ARM_IRQ_VCPU_SHIFT		16
>  #define KVM_ARM_IRQ_VCPU_MASK		0xff
>  #define KVM_ARM_IRQ_NUM_SHIFT		0
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 5e3f12d5359e..5414b6588fbb 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -996,6 +996,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_ARM_PTRAUTH_ADDRESS 171
>  #define KVM_CAP_ARM_PTRAUTH_GENERIC 172
>  #define KVM_CAP_PMU_EVENT_FILTER 173
> +#define KVM_CAP_ARM_IRQ_LINE_LAYOUT_2 174
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 35a069815baf..c1385911de69 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -182,6 +182,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  	int r;
>  	switch (ext) {
>  	case KVM_CAP_IRQCHIP:
> +	case KVM_CAP_ARM_IRQ_LINE_LAYOUT_2:
>  		r = vgic_present;
>  		break;
>  	case KVM_CAP_IOEVENTFD:
> @@ -888,6 +889,7 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_level,
>  
>  	irq_type = (irq >> KVM_ARM_IRQ_TYPE_SHIFT) & KVM_ARM_IRQ_TYPE_MASK;
>  	vcpu_idx = (irq >> KVM_ARM_IRQ_VCPU_SHIFT) & KVM_ARM_IRQ_VCPU_MASK;
> +	vcpu_idx += ((irq >> KVM_ARM_IRQ_VCPU2_SHIFT) & KVM_ARM_IRQ_VCPU2_MASK) * (KVM_ARM_IRQ_VCPU_MASK + 1);
>  	irq_num = (irq >> KVM_ARM_IRQ_NUM_SHIFT) & KVM_ARM_IRQ_NUM_MASK;
>  
>  	trace_kvm_irq_line(irq_type, vcpu_idx, irq_num, irq_level->level);
> 

Thank you for the patch!

Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric


_______________________________________________
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] soc: renesas: Enable ARM_ERRATA_754322 for affected Cortex-A9
From: Simon Horman @ 2019-08-22  8:59 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: linux-renesas-soc, Magnus Damm, linux-arm-kernel
In-Reply-To: <20190821124602.29317-3-geert+renesas@glider.be>

On Wed, Aug 21, 2019 at 02:46:01PM +0200, Geert Uytterhoeven wrote:
> ARM Erratum 754322 affects Cortex-A9 revisions r2p* and r3p*.
> 
> Enable support code to mitigate the erratum when compiling a kernel for
> any of the affected Renesas SoCs:
>   - RZ/A1: r3p0,
>   - R-Mobile A1: r2p4,
>   - R-Car M1A: r2p2-00rel0,
>   - R-Car H1: r3p0,
>   - SH-Mobile AG5: r2p2,
> and drop the corresponding config symbol from shmobile_defconfig.
> 
> EMMA Mobile EV2 (r1p3) and RZ/A2 (r4p1) are not affected.

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Looks like my R-Mobile A1 is actually r2p3, and the R-Car M1A in Magnus'
> farm is r2p4?
> 
>  arch/arm/configs/shmobile_defconfig | 1 -
>  drivers/soc/renesas/Kconfig         | 5 +++++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/configs/shmobile_defconfig b/arch/arm/configs/shmobile_defconfig
> index c6c70355141c38fa..123821e63873dafa 100644
> --- a/arch/arm/configs/shmobile_defconfig
> +++ b/arch/arm/configs/shmobile_defconfig
> @@ -9,7 +9,6 @@ CONFIG_PERF_EVENTS=y
>  CONFIG_SLAB=y
>  CONFIG_ARCH_RENESAS=y
>  CONFIG_PL310_ERRATA_588369=y
> -CONFIG_ARM_ERRATA_754322=y
>  CONFIG_SMP=y
>  CONFIG_SCHED_MC=y
>  CONFIG_NR_CPUS=8
> diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
> index a72d014ea37cc788..3c5e017bacbaca11 100644
> --- a/drivers/soc/renesas/Kconfig
> +++ b/drivers/soc/renesas/Kconfig
> @@ -55,6 +55,7 @@ config ARCH_EMEV2
>  
>  config ARCH_R7S72100
>  	bool "RZ/A1H (R7S72100)"
> +	select ARM_ERRATA_754322
>  	select PM
>  	select PM_GENERIC_DOMAINS
>  	select RENESAS_OSTM
> @@ -79,6 +80,7 @@ config ARCH_R8A73A4
>  config ARCH_R8A7740
>  	bool "R-Mobile A1 (R8A77400)"
>  	select ARCH_RMOBILE
> +	select ARM_ERRATA_754322
>  	select RENESAS_INTC_IRQPIN
>  
>  config ARCH_R8A7743
> @@ -108,10 +110,12 @@ config ARCH_R8A77470
>  config ARCH_R8A7778
>  	bool "R-Car M1A (R8A77781)"
>  	select ARCH_RCAR_GEN1
> +	select ARM_ERRATA_754322
>  
>  config ARCH_R8A7779
>  	bool "R-Car H1 (R8A77790)"
>  	select ARCH_RCAR_GEN1
> +	select ARM_ERRATA_754322
>  	select HAVE_ARM_SCU if SMP
>  	select HAVE_ARM_TWD if SMP
>  	select SYSC_R8A7779
> @@ -158,6 +162,7 @@ config ARCH_R9A06G032
>  config ARCH_SH73A0
>  	bool "SH-Mobile AG5 (R8A73A00)"
>  	select ARCH_RMOBILE
> +	select ARM_ERRATA_754322
>  	select HAVE_ARM_SCU if SMP
>  	select HAVE_ARM_TWD if SMP
>  	select RENESAS_INTC_IRQPIN
> -- 
> 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

* Re: [PATCH v2 2/4] nvmem: meson-efuse: bindings: Add secure-monitor phandle
From: Jerome Brunet @ 2019-08-22  8:59 UTC (permalink / raw)
  To: Rob Herring, Carlo Caione
  Cc: devicetree, narmstrong, khilman, srinivas.kandagatla,
	linux-amlogic, tglx, linux-arm-kernel
In-Reply-To: <20190821181458.GA2886@bogus>

On Wed 21 Aug 2019 at 13:14, Rob Herring <robh@kernel.org> wrote:

> On Wed, Jul 31, 2019 at 09:23:37AM +0100, Carlo Caione wrote:
>> Add a new property to link the nvmem driver to the secure-monitor. The
>> nvmem driver needs to access the secure-monitor to be able to access the
>> fuses.
>> 
>> Signed-off-by: Carlo Caione <ccaione@baylibre.com>
>> ---
>>  Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt b/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt
>> index 2e0723ab3384..f7b3ed74db54 100644
>> --- a/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt
>> +++ b/Documentation/devicetree/bindings/nvmem/amlogic-efuse.txt
>> @@ -4,6 +4,7 @@ Required properties:
>>  - compatible: should be "amlogic,meson-gxbb-efuse"
>>  - clocks: phandle to the efuse peripheral clock provided by the
>>  	  clock controller.
>> +- secure-monitor: phandle to the secure-monitor node
>>  
>>  = Data cells =
>>  Are child nodes of eFuse, bindings of which as described in
>> @@ -16,6 +17,7 @@ Example:
>>  		clocks = <&clkc CLKID_EFUSE>;
>>  		#address-cells = <1>;
>>  		#size-cells = <1>;
>> +		secure-monitor = <&sm>;
>>  
>>  		sn: sn@14 {
>>  			reg = <0x14 0x10>;
>> @@ -30,6 +32,10 @@ Example:
>>  		};
>>  	};
>>  
>> +	sm: secure-monitor {
>> +		compatible = "amlogic,meson-gxbb-sm";
>> +	};
>
> I guess I acked this a while back, but I'm not sure I would today. It 
> seems incomplete and a node with only a compatible string and no 
> resources doesn't need to be in DT. But that's already done...

It does have ressources (the shared memory) but the mistake (we should maybe think about
fixing) is not expressing these in DT

I think the shared memory is already in our DT, maybe the secure monitor
should get a phandle to it ?

>
> There's no need for 'secure-monitor' anyways. Just do 
> 'of_find_compatible_node(NULL, NULL, "amlogic,meson-gxbb-sm")' or search 
> for the driver directly. It's not like there's more than one secure 
> monitor...

IMO the two methods show different constraints:
- Carlo's proposition show that the efuse driver need a ressource, which
is *a* secure monitor, whatever the variant is.

- Your proposition shows that the efuse driver depends on a particular
  secure monitor variant, which is the one provided by
  "amlogic,meson-gxbb-sm"

Yes, we could make your proposition work by the keeping the
"amlogic,meson-gxbb-sm" last in the compatible list but it isn't great
if a newer variant is actually not compatible with it.

Carlo represent the HW the way it is. It seems more flexible to me,
without adding (unbearable) complexity

>
> Rob

_______________________________________________
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 v10 09/23] iommu/io-pgtable-arm-v7s: Extend to support PA[33:32] for MediaTek
From: Yong Wu @ 2019-08-22  8:59 UTC (permalink / raw)
  To: Robin Murphy
  Cc: youlin.pei, devicetree, Nicolas Boichat, cui.zhang,
	srv_heupstream, Tomasz Figa, Joerg Roedel, linux-kernel,
	Evan Green, chao.hao, iommu, Rob Herring, linux-mediatek,
	Matthias Brugger, ming-fan.chen, anan.sun, Will Deacon,
	Matthias Kaehlcke, linux-arm-kernel
In-Reply-To: <22a79977-5074-7af1-97b8-8a3e549b23c1@arm.com>

On Wed, 2019-08-21 at 16:34 +0100, Robin Murphy wrote:
> On 21/08/2019 16:24, Will Deacon wrote:
> > On Wed, Aug 21, 2019 at 09:53:12PM +0800, Yong Wu wrote:
> >> MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
> >> the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
> >> respectively. Meanwhile the iova still is 32bits.
> >>
> >> Regarding whether the pagetable address could be over 4GB, the mt8183
> >> support it while the previous mt8173 don't, thus keep it as is.
> >>
> >> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> >> ---
> >>   drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
> >>   include/linux/io-pgtable.h         |  7 +++----
> >>   2 files changed, 28 insertions(+), 11 deletions(-)
> > 
> > [...]
> > 
> >> @@ -731,7 +747,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> >>   {
> >>   	struct arm_v7s_io_pgtable *data;
> >>   
> >> -	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
> >> +	if (cfg->ias > ARM_V7S_ADDR_BITS ||
> >> +	    (cfg->oas > ARM_V7S_ADDR_BITS &&
> >> +	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
> > 
> > Please can you instead change arm_v7s_alloc_pgtable() so that it allows an
> > ias of up to 34 when the IO_PGTABLE_QUIRK_ARM_MTK_EXT is set?
> 
> You mean oas, right? I believe the hardware *does* actually support a 
> 32-bit ias as well, but we shouldn't pretend to support that while 
> __arm_v7s_alloc_table() still only knows how to allocate normal-sized 
> tables.

Yes. The HW double the lvl1 pgtable, thus it supports 33bit iova
actually. We may extend ias in the future.

> 
> Robin.
> 
> > 
> > With that change:
> > 
> > Acked-by: Will Deacon <will@kernel.org>
> > 
> > Will
> > 
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek



_______________________________________________
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] soc: renesas: Enable ARM_ERRATA_814220 for affected Cortex-A7
From: Simon Horman @ 2019-08-22  8:58 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: linux-renesas-soc, Magnus Damm, linux-arm-kernel
In-Reply-To: <20190821124602.29317-2-geert+renesas@glider.be>

On Wed, Aug 21, 2019 at 02:46:00PM +0200, Geert Uytterhoeven wrote:
> ARM Erratum 814220 affects Cortex-A7 revisions r0p2-r0p5.
> 
> Enable automatically support to mitigate the erratum when compiling a
> kernel for any of the affected Renesas SoCs:
>   - R-Mobile APE6: r0p2,
>   - RZ/G1E: r0p5,
>   - RZ/G1C: r0p5,
>   - R-Car H2: r0p3,
>   - R-Car E2: r0p5,
>   - RZ/N1: r0p5.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  drivers/soc/renesas/Kconfig | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
> index 2bbf49e5d441808b..a72d014ea37cc788 100644
> --- a/drivers/soc/renesas/Kconfig
> +++ b/drivers/soc/renesas/Kconfig
> @@ -72,6 +72,7 @@ config ARCH_R8A73A4
>  	bool "R-Mobile APE6 (R8A73A40)"
>  	select ARCH_RMOBILE
>  	select ARM_ERRATA_798181 if SMP
> +	select ARM_ERRATA_814220
>  	select HAVE_ARM_ARCH_TIMER
>  	select RENESAS_IRQC
>  
> @@ -95,11 +96,13 @@ config ARCH_R8A7744
>  config ARCH_R8A7745
>  	bool "RZ/G1E (R8A77450)"
>  	select ARCH_RCAR_GEN2
> +	select ARM_ERRATA_814220
>  	select SYSC_R8A7745
>  
>  config ARCH_R8A77470
>  	bool "RZ/G1C (R8A77470)"
>  	select ARCH_RCAR_GEN2
> +	select ARM_ERRATA_814220
>  	select SYSC_R8A77470
>  
>  config ARCH_R8A7778
> @@ -117,6 +120,7 @@ config ARCH_R8A7790
>  	bool "R-Car H2 (R8A77900)"
>  	select ARCH_RCAR_GEN2
>  	select ARM_ERRATA_798181 if SMP
> +	select ARM_ERRATA_814220
>  	select I2C
>  	select SYSC_R8A7790
>  
> @@ -143,11 +147,13 @@ config ARCH_R8A7793
>  config ARCH_R8A7794
>  	bool "R-Car E2 (R8A77940)"
>  	select ARCH_RCAR_GEN2
> +	select ARM_ERRATA_814220
>  	select SYSC_R8A7794
>  
>  config ARCH_R9A06G032
>  	bool "RZ/N1D (R9A06G032)"
>  	select ARCH_RZN1
> +	select ARM_ERRATA_814220
>  
>  config ARCH_SH73A0
>  	bool "SH-Mobile AG5 (R8A73A00)"
> -- 
> 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

* Re: [PATCH 3/3] [RFC] ARM: shmobile: defconfig: Disable PL310_ERRATA_588369
From: Simon Horman @ 2019-08-22  8:58 UTC (permalink / raw)
  To: Geert Uytterhoeven; +Cc: linux-renesas-soc, Magnus Damm, linux-arm-kernel
In-Reply-To: <20190821124602.29317-4-geert+renesas@glider.be>

On Wed, Aug 21, 2019 at 02:46:02PM +0200, Geert Uytterhoeven wrote:
> PL310 Erratum 588369 affects PL310 cache controller revisions older than
> r2p0.
> 
> As Renesas ARM SoCs contain the following revisions:
>   - SH-Mobile AG5: r3p1,
>   - R-Mobile A1: r3p1-50rel0,
>   - R-Car H1: r3p2,
>   - RZ/A1: r3p2,
>   - RZ/A2: r3p3,
> none of them are affected, and support for the errata can be disabled
> safely.
> 
> The EMMA Mobile EV2 documentation doesn't mention the revision of its
> PL310 cache controller, so this SoC might be affected.  However, the L2
> cache controller is not enabled by Linux.

If the controller is not enabled by Linux then I would think that the
Errata is not needed. If that is true then I agree with this patch.

Regarding making assumptions about the version of the PL310 cache
controller, I suggest that we cannot assume that it is not affected
without further information.

> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/arm/configs/shmobile_defconfig | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/configs/shmobile_defconfig b/arch/arm/configs/shmobile_defconfig
> index 123821e63873dafa..95a127cbe8e6fcd7 100644
> --- a/arch/arm/configs/shmobile_defconfig
> +++ b/arch/arm/configs/shmobile_defconfig
> @@ -8,7 +8,6 @@ CONFIG_CC_OPTIMIZE_FOR_SIZE=y
>  CONFIG_PERF_EVENTS=y
>  CONFIG_SLAB=y
>  CONFIG_ARCH_RENESAS=y
> -CONFIG_PL310_ERRATA_588369=y
>  CONFIG_SMP=y
>  CONFIG_SCHED_MC=y
>  CONFIG_NR_CPUS=8
> -- 
> 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

* Re: [PATCH v10 09/23] iommu/io-pgtable-arm-v7s: Extend to support PA[33:32] for MediaTek
From: Yong Wu @ 2019-08-22  8:56 UTC (permalink / raw)
  To: Will Deacon
  Cc: youlin.pei, devicetree, Nicolas Boichat, cui.zhang,
	srv_heupstream, Tomasz Figa, Joerg Roedel, linux-kernel,
	Evan Green, chao.hao, iommu, Rob Herring, linux-mediatek,
	Matthias Brugger, ming-fan.chen, anan.sun, Robin Murphy,
	Matthias Kaehlcke, linux-arm-kernel
In-Reply-To: <20190821152448.qmoqjh5zznfpdi6n@willie-the-truck>

On Wed, 2019-08-21 at 16:24 +0100, Will Deacon wrote:
> On Wed, Aug 21, 2019 at 09:53:12PM +0800, Yong Wu wrote:
> > MediaTek extend the arm v7s descriptor to support up to 34 bits PA where
> > the bit32 and bit33 are encoded in the bit9 and bit4 of the PTE
> > respectively. Meanwhile the iova still is 32bits.
> > 
> > Regarding whether the pagetable address could be over 4GB, the mt8183
> > support it while the previous mt8173 don't, thus keep it as is.
> > 
> > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > ---
> >  drivers/iommu/io-pgtable-arm-v7s.c | 32 +++++++++++++++++++++++++-------
> >  include/linux/io-pgtable.h         |  7 +++----
> >  2 files changed, 28 insertions(+), 11 deletions(-)
> 
> [...]
> 
> > @@ -731,7 +747,9 @@ static struct io_pgtable *arm_v7s_alloc_pgtable(struct io_pgtable_cfg *cfg,
> >  {
> >  	struct arm_v7s_io_pgtable *data;
> >  
> > -	if (cfg->ias > ARM_V7S_ADDR_BITS || cfg->oas > ARM_V7S_ADDR_BITS)
> > +	if (cfg->ias > ARM_V7S_ADDR_BITS ||
> > +	    (cfg->oas > ARM_V7S_ADDR_BITS &&
> > +	     !(cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT)))
> 
> Please can you instead change arm_v7s_alloc_pgtable() so that it allows an
> ias of up to 34 when the IO_PGTABLE_QUIRK_ARM_MTK_EXT is set?

Here I only simply skip the oas checking for our case. then which way do
your prefer?  something like you commented before:?


	if (cfg->ias > ARM_V7S_ADDR_BITS)
		return NULL;

	if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_MTK_EXT) {
		if (!IS_ENABLED(CONFIG_PHYS_ADDR_T_64BIT))
			cfg->oas = min(cfg->oas, ARM_V7S_ADDR_BITS);
		else if (cfg->oas > 34)
			return NULL;
	} else if (cfg->oas > ARM_V7S_ADDR_BITS) {
		return NULL;
	}


> 
> With that change:
> 
> Acked-by: Will Deacon <will@kernel.org>
> 
> Will
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] arm64: dts: mt8183: fix pwrap gic number
From: Hsin-Hsiung Wang @ 2019-08-22  8:55 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Mark Rutland, devicetree, srv_heupstream, linux-kernel,
	Rob Herring, linux-mediatek, Hsin-Hsiung Wang, linux-arm-kernel

The correct gic number of pwrap is 185 instead of 209. This patch fixes
it to avoid triggering error interrupt.

Fixes: e526c9bc11f8 ("arm64: dts: Add Mediatek SoC MT8183 and evaluation board dts and Makefile")

Signed-off-by: Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index c2749c4..afb0996 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -248,7 +248,7 @@
 			compatible = "mediatek,mt8183-pwrap";
 			reg = <0 0x1000d000 0 0x1000>;
 			reg-names = "pwrap";
-			interrupts = <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
+			interrupts = <GIC_SPI 185 IRQ_TYPE_LEVEL_HIGH>;
 			clocks = <&topckgen CLK_TOP_MUX_PMICSPI>,
 				 <&infracfg CLK_INFRA_PMIC_AP>;
 			clock-names = "spi", "wrap";
-- 
1.9.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] pwm: pwm-mediatek: Add MT8516 SoC support
From: Matthias Brugger @ 2019-08-22  8:52 UTC (permalink / raw)
  To: Fabien Parent, thierry.reding, robh+dt
  Cc: mark.rutland, linux-pwm, devicetree, linux-kernel, linux-mediatek,
	linux-arm-kernel
In-Reply-To: <20190805125848.15751-2-fparent@baylibre.com>



On 05/08/2019 14:58, Fabien Parent wrote:
> Add the compatible and the platform data to support PWM on the MT8516
> SoC.
> 
> Signed-off-by: Fabien Parent <fparent@baylibre.com>

Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>

> ---
>  drivers/pwm/pwm-mediatek.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/pwm/pwm-mediatek.c b/drivers/pwm/pwm-mediatek.c
> index eb6674ce995f..6697e30811e7 100644
> --- a/drivers/pwm/pwm-mediatek.c
> +++ b/drivers/pwm/pwm-mediatek.c
> @@ -302,11 +302,18 @@ static const struct mtk_pwm_platform_data mt7628_pwm_data = {
>  	.has_clks = false,
>  };
>  
> +static const struct mtk_pwm_platform_data mt8516_pwm_data = {
> +	.num_pwms = 5,
> +	.pwm45_fixup = false,
> +	.has_clks = true,
> +};
> +
>  static const struct of_device_id mtk_pwm_of_match[] = {
>  	{ .compatible = "mediatek,mt2712-pwm", .data = &mt2712_pwm_data },
>  	{ .compatible = "mediatek,mt7622-pwm", .data = &mt7622_pwm_data },
>  	{ .compatible = "mediatek,mt7623-pwm", .data = &mt7623_pwm_data },
>  	{ .compatible = "mediatek,mt7628-pwm", .data = &mt7628_pwm_data },
> +	{ .compatible = "mediatek,mt8516-pwm", .data = &mt8516_pwm_data },
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, mtk_pwm_of_match);
> 

_______________________________________________
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/4] misc: xilinx_sdfec: Fix a couple small information leaks
From: Michal Simek @ 2019-08-22  8:49 UTC (permalink / raw)
  To: Dan Carpenter, Michal Simek
  Cc: Arnd Bergmann, Greg Kroah-Hartman, Dragan Cvetic, kernel-janitors,
	linux-kernel, Derek Kiernan, linux-arm-kernel
In-Reply-To: <20190822082831.GH3964@kadam>

On 22. 08. 19 10:28, Dan Carpenter wrote:
> On Thu, Aug 22, 2019 at 10:14:12AM +0200, Michal Simek wrote:
>> Hi Dan,
>>
>> On 21. 08. 19 9:06, Dan Carpenter wrote:
>>> These structs have holes in them so we end up disclosing a few bytes of
>>> uninitialized stack data.
>>>
>>> drivers/misc/xilinx_sdfec.c:305 xsdfec_get_status() warn: check that 'status' doesn't leak information (struct has a hole after 'activity')
>>> drivers/misc/xilinx_sdfec.c:449 xsdfec_get_turbo() warn: check that 'turbo_params' doesn't leak information (struct has a hole after 'scale')
>>
>> Who is generating these warnings? Is this any new GCC or different tool?
>> I see that 3byte padding but never seen these warnings.
> 
> This is a Smatch check.

ok. It looks like I need to update it to latest version. My version is
not showing these.

Anyway thanks for patches,
Michal

_______________________________________________
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/9] crypt: Add diskcipher
From: Krzysztof Kozlowski @ 2019-08-22  8:37 UTC (permalink / raw)
  To: boojin.kim
  Cc: Ulf Hansson, Mike Snitzer, dm-devel, Andreas Dilger,
	Alasdair Kergon, Eric Biggers, linux-samsung-soc@vger.kernel.org,
	Herbert Xu, Jaehoon Chung, Kukjin Kim, linux-ext4, Chao Yu,
	linux-block, linux-fscrypt, Jaegeuk Kim, linux-arm-kernel,
	Jens Axboe, Theodore Y. Ts'o, linux-mmc,
	linux-kernel@vger.kernel.org, linux-f2fs-devel, linux-crypto,
	linux-fsdevel, David S. Miller
In-Reply-To: <003d01d557eb$8f6ca210$ae45e630$@samsung.com>

On Wed, 21 Aug 2019 at 08:42, boojin.kim <boojin.kim@samsung.com> wrote:
>
> Diskcipher supports cryptographic operations of inline crypto engines like
> FMP. Inline crypto engine refers to hardware and solutions implemented
> to encrypt data stored in storage device.
>
> When encrypting using the FMP, Additional control is required
> to carry and maintain the crypto information between
> the encryption user(fscrypt, DM-crypt) and FMP driver.
> Diskcipher provides this control.
>
> Diskcipher is a symmetric key cipher in linux crypto API to support FMP.
> FMP are registered with the cihper algorithm that uses diskcipher.
>
> Diskcipher has three major steps.
> The first step is to assign a cipher and set the key.
> The second step is to pass the cipher through the BIO to the storage
> driver.
> The third step is to get the cipher from BIO and request a crypt
> to FMP algorithm.
>
> In the first step, encryption users such as fscrypt or dm-crypt
> allocate/release a diskcipher and set key into the diskcipher.
> Diskcipher provides allocate(), free(), and setkey() that are similar
> to existing ciphers.
>
> In the second step, BIO is used to pass the diskcipher to the storage
> driver.
> The BIO submitters such as ext4, f2fs and DM-crypt set diskcipher to BIO.
> Diskcipher provides the set () API for this.
>
> In the third step, the storage driver extracts the diskcipher from the BIO
> and requests the actual encryption behavior to inline crypto engine driver.
> Diskcipher provides get() and crypt() APIs for this.
>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Boojin Kim <boojin.kim@samsung.com>
> ---
>  crypto/Kconfig              |   9 ++
>  crypto/Makefile             |   1 +
>  crypto/diskcipher.c         | 349
> ++++++++++++++++++++++++++++++++++++++++++++
>  crypto/testmgr.c            | 157 ++++++++++++++++++++
>  include/crypto/diskcipher.h | 245 +++++++++++++++++++++++++++++++
>  include/linux/crypto.h      |   1 +
>  6 files changed, 762 insertions(+)
>  create mode 100644 crypto/diskcipher.c
>  create mode 100644 include/crypto/diskcipher.h
>
> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 455a335..382d43a 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -1636,6 +1636,15 @@ config CRYPTO_TWOFISH_AVX_X86_64
>           See also:
>           <http://www.schneier.com/twofish.html>
>
> +config CRYPTO_DISKCIPHER
> +       bool "Diskcipher support"
> +       default n
> +       help
> +         Disk cipher algorithm
> +
> +         This cipher supports the crypt operation of the block host device
> +         that has inline crypto engine.
> +
>  comment "Compression"
>
>  config CRYPTO_DEFLATE
> diff --git a/crypto/Makefile b/crypto/Makefile
> index 0d2cdd5..71df76a 100644
> --- a/crypto/Makefile
> +++ b/crypto/Makefile
> @@ -165,6 +165,7 @@ obj-$(CONFIG_CRYPTO_USER_API_AEAD) += algif_aead.o
>  obj-$(CONFIG_CRYPTO_ZSTD) += zstd.o
>  obj-$(CONFIG_CRYPTO_OFB) += ofb.o
>  obj-$(CONFIG_CRYPTO_ECC) += ecc.o
> +obj-$(CONFIG_CRYPTO_DISKCIPHER) += diskcipher.o
>
>  ecdh_generic-y += ecdh.o
>  ecdh_generic-y += ecdh_helper.o
> diff --git a/crypto/diskcipher.c b/crypto/diskcipher.c
> new file mode 100644
> index 0000000..ffe95a5
> --- /dev/null
> +++ b/crypto/diskcipher.c
> @@ -0,0 +1,349 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2017 Samsung Electronics Co., Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/blkdev.h>
> +#include <linux/errno.h>
> +#include <linux/module.h>
> +#include <linux/seq_file.h>
> +#include <linux/string.h>
> +#include <linux/crypto.h>
> +#include <crypto/algapi.h>
> +#include <crypto/diskcipher.h>
> +#include <linux/delay.h>
> +#include <linux/mm_types.h>
> +#include <linux/fs.h>
> +#include <linux/fscrypt.h>
> +
> +#include "internal.h"
> +
> +static int crypto_diskcipher_check(struct bio *bio)
> +{
> +       struct crypto_diskcipher *ci = NULL;
> +       struct inode *inode = NULL;
> +       struct page *page = NULL;
> +
> +       if (!bio) {
> +               pr_err("%s: doesn't exist bio\n", __func__);
> +               return 0;
> +       }
> +
> +       /* enc without fscrypt */
> +       ci = bio->bi_aux_private;
> +       if (!ci->inode)
> +               return 0;
> +       if (ci->algo == 0)
> +               return 0;
> +
> +       page = bio->bi_io_vec[0].bv_page;
> +       if (!page || PageAnon(page) || !page->mapping ||
> !page->mapping->host)

Your patch looks corrupted - wrapped by mailer. The easiest way
usually is to use git format-patch and git send-email - then you do
not have to worry about formatting etc.

Best regards,
Krzysztof

_______________________________________________
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/4] nvmem: meson-efuse: bindings: Add secure-monitor phandle
From: Carlo Caione @ 2019-08-22  8:36 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, narmstrong, khilman, srinivas.kandagatla,
	linux-amlogic, tglx, linux-arm-kernel, jbrunet
In-Reply-To: <20190821181458.GA2886@bogus>

On 21/08/2019 19:14, Rob Herring wrote:
> On Wed, Jul 31, 2019 at 09:23:37AM +0100, Carlo Caione wrote:

> There's no need for 'secure-monitor' anyways. Just do
> 'of_find_compatible_node(NULL, NULL, "amlogic,meson-gxbb-sm")' or search
> for the driver directly. It's not like there's more than one secure
> monitor...

How is hardcoding the secure-monitor directly into the efuse driver 
better than having it referenced in the DT?

Yes, there is one single secure monitor but (even if this is not 
currently the case) several drivers can use it making the secure-monitor 
a resource to be potentially used by several devices.

--
Carlo Caione

_______________________________________________
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/5] soc: amlogic: Add support for Everything-Else power domains controller
From: Neil Armstrong @ 2019-08-22  8:35 UTC (permalink / raw)
  To: Kevin Hilman, ulf.hansson
  Cc: linux-amlogic, linux-kernel, linux-arm-kernel, linux-pm
In-Reply-To: <7hzhk25ct3.fsf@baylibre.com>

On 22/08/2019 01:16, Kevin Hilman wrote:
> Neil Armstrong <narmstrong@baylibre.com> writes:
> 
>> Add support for the General Purpose Amlogic Everything-Else Power controller,
>> with the first support for G12A and SM1 SoCs dedicated to the VPU, PCIe,
>> USB, NNA, GE2D and Ethernet Power Domains.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> 
> Nice!  Thanks for generalizing this.
> 
> A few comments/concerns below, but this is mostly ready.
> 
>> ---
>>  drivers/soc/amlogic/Kconfig         |  11 +
>>  drivers/soc/amlogic/Makefile        |   1 +
>>  drivers/soc/amlogic/meson-ee-pwrc.c | 560 ++++++++++++++++++++++++++++
>>  3 files changed, 572 insertions(+)
>>  create mode 100644 drivers/soc/amlogic/meson-ee-pwrc.c
>>
>> diff --git a/drivers/soc/amlogic/Kconfig b/drivers/soc/amlogic/Kconfig
>> index 23bfb8ef4fdb..bc2c912949bd 100644
>> --- a/drivers/soc/amlogic/Kconfig
>> +++ b/drivers/soc/amlogic/Kconfig
>> @@ -37,6 +37,17 @@ config MESON_GX_PM_DOMAINS
>>  	  Say yes to expose Amlogic Meson GX Power Domains as
>>  	  Generic Power Domains.
>>  
>> +config MESON_EE_PM_DOMAINS
>> +	bool "Amlogic Meson Everything-Else Power Domains driver"
>> +	depends on ARCH_MESON || COMPILE_TEST
>> +	depends on PM && OF
>> +	default ARCH_MESON
>> +	select PM_GENERIC_DOMAINS
>> +	select PM_GENERIC_DOMAINS_OF
>> +	help
>> +	  Say yes to expose Amlogic Meson Everything-Else Power Domains as
>> +	  Generic Power Domains.
>> +
>>  config MESON_MX_SOCINFO
>>  	bool "Amlogic Meson MX SoC Information driver"
>>  	depends on ARCH_MESON || COMPILE_TEST
>> diff --git a/drivers/soc/amlogic/Makefile b/drivers/soc/amlogic/Makefile
>> index f2e4ed171297..de79d044b545 100644
>> --- a/drivers/soc/amlogic/Makefile
>> +++ b/drivers/soc/amlogic/Makefile
>> @@ -4,3 +4,4 @@ obj-$(CONFIG_MESON_CLK_MEASURE) += meson-clk-measure.o
>>  obj-$(CONFIG_MESON_GX_SOCINFO) += meson-gx-socinfo.o
>>  obj-$(CONFIG_MESON_GX_PM_DOMAINS) += meson-gx-pwrc-vpu.o
>>  obj-$(CONFIG_MESON_MX_SOCINFO) += meson-mx-socinfo.o
>> +obj-$(CONFIG_MESON_EE_PM_DOMAINS) += meson-ee-pwrc.o
>> diff --git a/drivers/soc/amlogic/meson-ee-pwrc.c b/drivers/soc/amlogic/meson-ee-pwrc.c
>> new file mode 100644
>> index 000000000000..7159888c850b
>> --- /dev/null
>> +++ b/drivers/soc/amlogic/meson-ee-pwrc.c
>> @@ -0,0 +1,560 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (c) 2019 BayLibre, SAS
>> + * Author: Neil Armstrong <narmstrong@baylibre.com>
>> + */
>> +
>> +#include <linux/of_address.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_domain.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/regmap.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/of_device.h>
>> +#include <linux/reset-controller.h>
>> +#include <linux/reset.h>
>> +#include <linux/clk.h>
>> +#include <dt-bindings/power/meson-g12a-power.h>
>> +#include <dt-bindings/power/meson-sm1-power.h>
>> +
>> +/* AO Offsets */
>> +
>> +#define AO_RTI_GEN_PWR_SLEEP0		(0x3a << 2)
>> +#define AO_RTI_GEN_PWR_ISO0		(0x3b << 2)
>> +
>> +/* HHI Offsets */
>> +
>> +#define HHI_MEM_PD_REG0			(0x40 << 2)
>> +#define HHI_VPU_MEM_PD_REG0		(0x41 << 2)
>> +#define HHI_VPU_MEM_PD_REG1		(0x42 << 2)
>> +#define HHI_VPU_MEM_PD_REG3		(0x43 << 2)
>> +#define HHI_VPU_MEM_PD_REG4		(0x44 << 2)
>> +#define HHI_AUDIO_MEM_PD_REG0		(0x45 << 2)
>> +#define HHI_NANOQ_MEM_PD_REG0		(0x46 << 2)
>> +#define HHI_NANOQ_MEM_PD_REG1		(0x47 << 2)
>> +#define HHI_VPU_MEM_PD_REG2		(0x4d << 2)
>> +
>> +struct meson_ee_pwrc;
>> +struct meson_ee_pwrc_domain;
>> +
>> +struct meson_ee_pwrc_mem_domain {
>> +	unsigned int reg;
>> +	unsigned int mask;
>> +};
>> +
>> +struct meson_ee_pwrc_top_domain {
>> +	unsigned int sleep_reg;
>> +	unsigned int sleep_mask;
>> +	unsigned int iso_reg;
>> +	unsigned int iso_mask;
>> +};
>> +
>> +struct meson_ee_pwrc_domain_desc {
>> +	char *name;
>> +	char **reset_names;
>> +	unsigned int reset_names_count;
>> +	char **clk_names;
>> +	unsigned int clk_names_count;
>> +	struct meson_ee_pwrc_top_domain *top_pd;
>> +	unsigned int mem_pd_count;
>> +	struct meson_ee_pwrc_mem_domain *mem_pd;
>> +	bool (*get_power)(struct meson_ee_pwrc_domain *pwrc_domain);
>> +};
>> +
>> +struct meson_ee_pwrc_domain_data {
>> +	unsigned int count;
>> +	struct meson_ee_pwrc_domain_desc *domains;
>> +};
>> +
>> +/* Clock and Resets lists */
>> +
>> +static char *g12a_pwrc_vpu_resets[] = {
>> +	"viu", "venc", "vcbus", "bt656",
>> +	"rdma", "venci", "vencp", "vdac",
>> +	"vdi6", "vencl", "vid_lock",
>> +};
>> +
>> +static char *g12a_pwrc_vpu_clks[] = {
>> +	"vpu", "vapb",
>> +};
>> +
>> +/* TOP Power Domains */
>> +
>> +static struct meson_ee_pwrc_top_domain g12a_pwrc_vpu = {
>> +	.sleep_reg = AO_RTI_GEN_PWR_SLEEP0,
>> +	.sleep_mask = BIT(8),
>> +	.iso_reg = AO_RTI_GEN_PWR_SLEEP0,
>> +	.iso_mask = BIT(9),
>> +};
>> +
>> +#define SM1_EE_PD(__bit)					\
>> +	{							\
>> +		.sleep_reg = AO_RTI_GEN_PWR_SLEEP0, 		\
>> +		.sleep_mask = BIT(__bit), 			\
>> +		.iso_reg = AO_RTI_GEN_PWR_ISO0, 		\
>> +		.iso_mask = BIT(__bit), 			\
>> +	}
>> +
>> +static struct meson_ee_pwrc_top_domain sm1_pwrc_vpu = SM1_EE_PD(8);
>> +static struct meson_ee_pwrc_top_domain sm1_pwrc_nna = SM1_EE_PD(16);
>> +static struct meson_ee_pwrc_top_domain sm1_pwrc_usb = SM1_EE_PD(17);
>> +static struct meson_ee_pwrc_top_domain sm1_pwrc_pci = SM1_EE_PD(18);
>> +static struct meson_ee_pwrc_top_domain sm1_pwrc_ge2d = SM1_EE_PD(19);
>> +
>> +/* Memory PD Domains */
>> +
>> +#define VPU_MEMPD(__reg)					\
>> +	{ __reg, GENMASK(1, 0) },				\
>> +	{ __reg, GENMASK(3, 2) },				\
>> +	{ __reg, GENMASK(5, 4) },				\
>> +	{ __reg, GENMASK(7, 6) },				\
>> +	{ __reg, GENMASK(9, 8) },				\
>> +	{ __reg, GENMASK(11, 10) },				\
>> +	{ __reg, GENMASK(13, 12) },				\
>> +	{ __reg, GENMASK(15, 14) },				\
>> +	{ __reg, GENMASK(17, 16) },				\
>> +	{ __reg, GENMASK(19, 18) },				\
>> +	{ __reg, GENMASK(21, 20) },				\
>> +	{ __reg, GENMASK(23, 22) },				\
>> +	{ __reg, GENMASK(25, 24) },				\
>> +	{ __reg, GENMASK(27, 26) },				\
>> +	{ __reg, GENMASK(29, 28) },				\
>> +	{ __reg, GENMASK(31, 30) }
>> +
>> +#define VPU_HHI_MEMPD(__reg)					\
>> +	{ __reg, BIT(8) },					\
>> +	{ __reg, BIT(9) },					\
>> +	{ __reg, BIT(10) },					\
>> +	{ __reg, BIT(11) },					\
>> +	{ __reg, BIT(12) },					\
>> +	{ __reg, BIT(13) },					\
>> +	{ __reg, BIT(14) },					\
>> +	{ __reg, BIT(15) }
>> +
>> +static struct meson_ee_pwrc_mem_domain g12a_pwrc_mem_vpu[] = {
>> +	VPU_MEMPD(HHI_VPU_MEM_PD_REG0),
>> +	VPU_MEMPD(HHI_VPU_MEM_PD_REG1),
>> +	VPU_MEMPD(HHI_VPU_MEM_PD_REG2),
>> +	VPU_HHI_MEMPD(HHI_MEM_PD_REG0),
>> +};
>> +
>> +static struct meson_ee_pwrc_mem_domain g12a_pwrc_mem_eth[] = {
>> +	{ HHI_MEM_PD_REG0, GENMASK(3, 2) },
>> +};
>> +
>> +static struct meson_ee_pwrc_mem_domain sm1_pwrc_mem_vpu[] = {
>> +	VPU_MEMPD(HHI_VPU_MEM_PD_REG0),
>> +	VPU_MEMPD(HHI_VPU_MEM_PD_REG1),
>> +	VPU_MEMPD(HHI_VPU_MEM_PD_REG2),
>> +	VPU_MEMPD(HHI_VPU_MEM_PD_REG3),
>> +	{ HHI_VPU_MEM_PD_REG4, GENMASK(1, 0) },
>> +	{ HHI_VPU_MEM_PD_REG4, GENMASK(3, 2) },
>> +	{ HHI_VPU_MEM_PD_REG4, GENMASK(5, 4) },
>> +	{ HHI_VPU_MEM_PD_REG4, GENMASK(7, 6) },
>> +	VPU_HHI_MEMPD(HHI_MEM_PD_REG0),
>> +};
>> +
>> +static struct meson_ee_pwrc_mem_domain sm1_pwrc_mem_nna[] = {
>> +	{ HHI_NANOQ_MEM_PD_REG0, 0xff },
>> +	{ HHI_NANOQ_MEM_PD_REG1, 0xff },
>> +};
>> +
>> +static struct meson_ee_pwrc_mem_domain sm1_pwrc_mem_usb[] = {
>> +	{ HHI_MEM_PD_REG0, GENMASK(31, 30) },
>> +};
>> +
>> +static struct meson_ee_pwrc_mem_domain sm1_pwrc_mem_pcie[] = {
>> +	{ HHI_MEM_PD_REG0, GENMASK(29, 26) },
>> +};
>> +
>> +static struct meson_ee_pwrc_mem_domain sm1_pwrc_mem_ge2d[] = {
>> +	{ HHI_MEM_PD_REG0, GENMASK(25, 18) },
>> +};
>> +
>> +static struct meson_ee_pwrc_mem_domain sm1_pwrc_mem_audio[] = {
>> +	{ HHI_MEM_PD_REG0, GENMASK(5, 4) },
>> +	{ HHI_AUDIO_MEM_PD_REG0, GENMASK(1, 0) },
>> +	{ HHI_AUDIO_MEM_PD_REG0, GENMASK(3, 2) },
>> +	{ HHI_AUDIO_MEM_PD_REG0, GENMASK(5, 4) },
>> +	{ HHI_AUDIO_MEM_PD_REG0, GENMASK(7, 6) },
>> +	{ HHI_AUDIO_MEM_PD_REG0, GENMASK(13, 12) },
>> +	{ HHI_AUDIO_MEM_PD_REG0, GENMASK(15, 14) },
>> +	{ HHI_AUDIO_MEM_PD_REG0, GENMASK(17, 16) },
>> +	{ HHI_AUDIO_MEM_PD_REG0, GENMASK(19, 18) },
>> +	{ HHI_AUDIO_MEM_PD_REG0, GENMASK(21, 20) },
>> +	{ HHI_AUDIO_MEM_PD_REG0, GENMASK(23, 22) },
>> +	{ HHI_AUDIO_MEM_PD_REG0, GENMASK(25, 24) },
>> +	{ HHI_AUDIO_MEM_PD_REG0, GENMASK(27, 26) },
>> +};
>> +
>> +#define VPU_PD(__name, __resets, __clks, __top_pd, __mem, __get_power)	\
>> +	{								\
>> +		.name = __name,						\
>> +		.reset_names_count = ARRAY_SIZE(__resets),		\
>> +		.reset_names = __resets,				\
>> +		.clk_names_count = ARRAY_SIZE(__clks),			\
>> +		.clk_names = __clks,					\
>> +		.top_pd = __top_pd,					\
>> +		.mem_pd_count = ARRAY_SIZE(__mem),			\
>> +		.mem_pd = __mem,					\
>> +		.get_power = __get_power,				\
>> +	}
>> +
>> +#define TOP_PD(__name, __top_pd, __mem)					\
>> +	{								\
>> +		.name = __name,						\
>> +		.top_pd = __top_pd,					\
>> +		.mem_pd_count = ARRAY_SIZE(__mem),			\
>> +		.mem_pd = __mem,					\
>> +	}
> 
> Why can't the TOP_PD domains also have a __get_power().  Shouldn't we
> just be able to check the sleep_reg/sleep_mask like in the VPU case?

It can, I can add it later, do we need it for this version ?

> 
> Also, for readability, I think the arguments to VPU_PD and TOP_PD to
> have the same order, at least for the common ones.  IOW, __name,
> __top_pd, __mem should be first.

Sure, will fix

> 
>> +#define MEM_PD(__name, __mem)						\
>> +	TOP_PD(__name, NULL, __mem)
>> +
>> +static bool pwrc_vpu_get_power(struct meson_ee_pwrc_domain *pwrc_domain);
>> +
>> +static struct meson_ee_pwrc_domain_desc g12a_pwrc_domains[] = {
>> +	[PWRC_G12A_VPU_ID]  = VPU_PD("VPU", g12a_pwrc_vpu_resets,
>> +				     g12a_pwrc_vpu_clks, &g12a_pwrc_vpu,
>> +				     g12a_pwrc_mem_vpu,
>> +				     pwrc_vpu_get_power),
>> +	[PWRC_G12A_ETH_ID] = MEM_PD("ETH", g12a_pwrc_mem_eth),
>> +};
>> +
>> +static struct meson_ee_pwrc_domain_desc sm1_pwrc_domains[] = {
>> +	[PWRC_SM1_VPU_ID]  = VPU_PD("VPU", g12a_pwrc_vpu_resets,
>> +				    g12a_pwrc_vpu_clks, &sm1_pwrc_vpu,
>> +				    sm1_pwrc_mem_vpu,
>> +				    pwrc_vpu_get_power),
>> +	[PWRC_SM1_NNA_ID]  = TOP_PD("NNA", &sm1_pwrc_nna, sm1_pwrc_mem_nna),
>> +	[PWRC_SM1_USB_ID]  = TOP_PD("USB", &sm1_pwrc_usb, sm1_pwrc_mem_usb),
>> +	[PWRC_SM1_PCIE_ID] = TOP_PD("PCI", &sm1_pwrc_pci, sm1_pwrc_mem_pcie),
>> +	[PWRC_SM1_GE2D_ID] = TOP_PD("GE2D", &sm1_pwrc_ge2d, sm1_pwrc_mem_ge2d),
>> +	[PWRC_SM1_AUDIO_ID] = MEM_PD("AUDIO", sm1_pwrc_mem_audio),
>> +	[PWRC_SM1_ETH_ID] = MEM_PD("ETH", g12a_pwrc_mem_eth),
>> +};
>> +
>> +struct meson_ee_pwrc_domain {
>> +	struct generic_pm_domain base;
>> +	bool enabled;
>> +	struct meson_ee_pwrc *pwrc;
>> +	struct meson_ee_pwrc_domain_desc desc;
>> +	struct clk **clks;
>> +	int num_clks;
>> +	struct reset_control **rstc;
>> +	int num_rstc;
>> +};
>> +
>> +struct meson_ee_pwrc {
>> +	struct regmap *regmap_ao;
>> +	struct regmap *regmap_hhi;
>> +	struct meson_ee_pwrc_domain *domains;
>> +	struct genpd_onecell_data xlate;
>> +};
>> +
>> +static bool pwrc_vpu_get_power(struct meson_ee_pwrc_domain *pwrc_domain)
>> +{
>> +	u32 reg;
>> +
>> +	regmap_read(pwrc_domain->pwrc->regmap_ao,
>> +		    pwrc_domain->desc.top_pd->sleep_reg, &reg);
>> +
>> +	return (reg & pwrc_domain->desc.top_pd->sleep_mask);
>> +}
>> +
>> +static int meson_ee_reset_assert(struct meson_ee_pwrc_domain *pwrc_domain)
>> +{
>> +	int i, ret;
>> +
>> +	for (i = 0 ; i < pwrc_domain->num_rstc ; ++i) {
>> +		ret = reset_control_assert(pwrc_domain->rstc[i]);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int meson_ee_reset_deassert(struct meson_ee_pwrc_domain *pwrc_domain)
>> +{
>> +	int i, ret;
>> +
>> +	for (i = 0 ; i < pwrc_domain->num_rstc ; ++i) {
>> +		ret = reset_control_deassert(pwrc_domain->rstc[i]);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	return 0;
>> +}
> 
> You should use the reset_array functions, then you don't need these helpers.
> 
>> +static int meson_ee_clk_disable(struct meson_ee_pwrc_domain *pwrc_domain)
>> +{
>> +	int i;
>> +
>> +	for (i = 0 ; i < pwrc_domain->num_clks ; ++i)
>> +		clk_disable(pwrc_domain->clks[i]);
>> +
>> +	for (i = 0 ; i < pwrc_domain->num_clks ; ++i)
>> +		clk_unprepare(pwrc_domain->clks[i]);
>> +
>> +	return 0;
>> +}
>> +
>> +static int meson_ee_clk_enable(struct meson_ee_pwrc_domain *pwrc_domain)
>> +{
>> +	int i, ret;
>> +
>> +	for (i = 0 ; i < pwrc_domain->num_clks ; ++i) {
>> +		ret = clk_prepare(pwrc_domain->clks[i]);
>> +		if (ret)
>> +			goto fail_prepare;
>> +	}
>> +
>> +	for (i = 0 ; i < pwrc_domain->num_clks ; ++i) {
>> +		ret = clk_enable(pwrc_domain->clks[i]);
>> +		if (ret)
>> +			goto fail_enable;
>> +	}
>> +
>> +	return 0;
>> +fail_enable:
>> +	while (--i)
>> +		clk_disable(pwrc_domain->clks[i]);
>> +
>> +	/* Unprepare all clocks */
>> +	i = pwrc_domain->num_clks;
>> +
>> +fail_prepare:
>> +	while (--i)
>> +		clk_unprepare(pwrc_domain->clks[i]);
>> +
>> +	return ret;
>> +}
> 
> Both the clk enable and disable functions above are just open-coding of
> the clk_bulk equivalents.  Please use clk_bulk_*, then you don't need
> these helpers.  (c.f. the RFT patch I did to convert the old driver to
> clk_bulk[1])

Yes, but clk_bulk takes _all_ the clocks from the node, you canot specify
a list of names, maybe it's overengineered but I wanted to specify the
exact resets and clocks for each power domain, clk_bulk doesn't provide this.

> 
>> +static int meson_ee_pwrc_off(struct generic_pm_domain *domain)
>> +{
>> +	struct meson_ee_pwrc_domain *pwrc_domain =
>> +		container_of(domain, struct meson_ee_pwrc_domain, base);
>> +	int i;
>> +
>> +	if (pwrc_domain->desc.top_pd)
>> +		regmap_update_bits(pwrc_domain->pwrc->regmap_ao,
>> +				   pwrc_domain->desc.top_pd->sleep_reg,
>> +				   pwrc_domain->desc.top_pd->sleep_mask,
>> +				   pwrc_domain->desc.top_pd->sleep_mask);
>> +	udelay(20);
>> +
>> +	for (i = 0 ; i < pwrc_domain->desc.mem_pd_count ; ++i)
>> +		regmap_update_bits(pwrc_domain->pwrc->regmap_hhi,
>> +				   pwrc_domain->desc.mem_pd[i].reg,
>> +				   pwrc_domain->desc.mem_pd[i].mask,
>> +				   pwrc_domain->desc.mem_pd[i].mask);
>> +
>> +	udelay(20);
>> +
>> +	if (pwrc_domain->desc.top_pd)
>> +		regmap_update_bits(pwrc_domain->pwrc->regmap_ao,
>> +				   pwrc_domain->desc.top_pd->iso_reg,
>> +				   pwrc_domain->desc.top_pd->iso_mask,
>> +				   pwrc_domain->desc.top_pd->iso_mask);
>> +
>> +	if (pwrc_domain->num_clks) {
>> +		msleep(20);
>> +		meson_ee_clk_disable(pwrc_domain);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int meson_ee_pwrc_on(struct generic_pm_domain *domain)
>> +{
>> +	struct meson_ee_pwrc_domain *pwrc_domain =
>> +		container_of(domain, struct meson_ee_pwrc_domain, base);
>> +	int i, ret;
>> +
>> +	if (pwrc_domain->desc.top_pd)
>> +		regmap_update_bits(pwrc_domain->pwrc->regmap_ao,
>> +				   pwrc_domain->desc.top_pd->sleep_reg,
>> +				   pwrc_domain->desc.top_pd->sleep_mask, 0);
>> +	udelay(20);
>> +
>> +	for (i = 0 ; i < pwrc_domain->desc.mem_pd_count ; ++i)
>> +		regmap_update_bits(pwrc_domain->pwrc->regmap_hhi,
>> +				   pwrc_domain->desc.mem_pd[i].reg,
>> +				   pwrc_domain->desc.mem_pd[i].mask, 0);
>> +
>> +	udelay(20);
>> +
>> +	ret = meson_ee_reset_assert(pwrc_domain);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (pwrc_domain->desc.top_pd)
>> +		regmap_update_bits(pwrc_domain->pwrc->regmap_ao,
>> +				   pwrc_domain->desc.top_pd->iso_reg,
>> +				   pwrc_domain->desc.top_pd->iso_mask, 0);
>> +
>> +	ret = meson_ee_reset_deassert(pwrc_domain);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return meson_ee_clk_enable(pwrc_domain);
>> +}
>> +
>> +static int meson_ee_pwrc_init_domain(struct platform_device *pdev,
>> +				     struct meson_ee_pwrc *sm1_pwrc,
>> +				     struct meson_ee_pwrc_domain *dom)
>> +{
>> +	dom->pwrc = sm1_pwrc;
>> +	dom->num_rstc = dom->desc.reset_names_count;
>> +	dom->num_clks = dom->desc.clk_names_count;
>> +
>> +	if (dom->num_rstc) {
>> +		int rst;
>> +
>> +		dom->rstc = devm_kcalloc(&pdev->dev, dom->num_rstc,
>> +				sizeof(struct reset_control *),	GFP_KERNEL);
>> +		if (!dom->rstc)
>> +			return -ENOMEM;
>> +
>> +		for (rst = 0 ; rst < dom->num_rstc ; ++rst) {
>> +			dom->rstc[rst] = devm_reset_control_get_exclusive(
>> +					&pdev->dev,
>> +					dom->desc.reset_names[rst]);
>> +			if (IS_ERR(dom->rstc[rst]))
>> +				return PTR_ERR(dom->rstc[rst]);
>> +		}
> 
> Why not simplify and use the helpers that get multiple reset lines (like
> devm_reset_control_array_get() used in meson-gx-pwrc-vpu.c)?

Same comment as clk_bulk, we cannot be sure we select the right reset lines.

> 
> You could also use reset_control_get_count() and compare to the expected
> number (dom->num_rstc).

This seems oversimplified

> 
>> +	}
>> +
>> +	if (dom->num_clks) {
>> +		int clk;
>> +
>> +		dom->clks = devm_kcalloc(&pdev->dev, dom->num_clks,
>> +				sizeof(struct clk *), GFP_KERNEL);
>> +		if (!dom->clks)
>> +			return -ENOMEM;
>> +
>> +		for (clk = 0 ; clk < dom->num_clks ; ++clk) {
>> +			dom->clks[clk] = devm_clk_get(&pdev->dev,
>> +					dom->desc.clk_names[clk]);
>> +			if (IS_ERR(dom->clks[clk]))
>> +				return PTR_ERR(dom->clks[clk]);
>> +		}
>> +	}
> 
> Please use clk_bulk API, and then just double-check that the number of
> clocks found matches the expected number.

Same, I'll either take all the clks and resets for the vpu power domain,
or keep this code to make sure we get the right clocks and resets.

> 
>> +	dom->base.name = dom->desc.name;
>> +	dom->base.power_on = meson_ee_pwrc_on;
>> +	dom->base.power_off = meson_ee_pwrc_off;
>> +
>> +	if (dom->desc.get_power) {
>> +		bool powered_off = dom->desc.get_power(dom);
> 
> nit: insert blank line here
> 
> More importantly, we defintely will have problem here in the
> !powered_off case.  TL;DR; the driver's state does not match the actual
> hardware state.
> 
> When powered_off = false, you're telling the genpd core that this domain
> is already turned on.  However, you haven't called _pwrc_on() yet for
> the domain, which means internal state of the driver for this domain
> (e.g. clock enables, resets, etc.) is not in sync with the HW.  On
> SEI610 this case is happending for the VPU, which seems to be enabled by
> u-boot, so this driver detects it as already on, which is fine.  But...
> 
> Remember that the ->power_off() function will be called during suspend,
> and will lead to the clk unprepare/disable calls.  However, for domains
> that are detected as on (!powered_off), clk prepare/enable will never
> have been called, so that when suspend happens, you'll get "already
> unprepared" errors from the clock core
> 
> IOW, I think you need something like this here:
> 
> 		if (!powered_off)
> 			meson_ee_pwrc_on(&dom->base);
> 
> so that the internal state of clock fwk etc. matches the detected state
> of the HW.  The problem with that simple fix, at least for the VPU is
> that it might cause us to lose any existing display or framebuffer
> console that's on-going.  Probably needs some testing.

Yes, I forgot to take the _shutdown() function from gx_pwrc_vpu driver :

349 static void meson_gx_pwrc_vpu_shutdown(struct platform_device *pdev)
350 {
351         struct meson_gx_pwrc_vpu *vpu_pd = platform_get_drvdata(pdev);
352         bool powered_off;
353
354         powered_off = meson_gx_pwrc_vpu_get_power(vpu_pd);
355         if (!powered_off)
356                 vpu_pd->genpd.power_off(&vpu_pd->genpd);
357 }

> 
> Anyways, to see what I mean, try suspend/resume (you can test this
> series on my integ branch with "rtcwake -d rtc0 -m mem -s4") and you'll
> see error splats from the clock core during suspend.
> 
> 
> 
>> +		pm_genpd_init(&dom->base, &pm_domain_always_on_gov,
>> +			      powered_off);
> 
>> +	} else
>> +		pm_genpd_init(&dom->base, NULL, true);
> 
> nit: the else clause should also have {} to match the if
> (c.f. CodingStyle)

OK

> 
> Why do you force the always-on governor in the case where ->get_power()
> exists, but not the other?
> 
> If you force that, then for any devices connected to these domains that
> use runtime PM, they will never turn off the domain when it's idle.
> IOW, these domains will only ever be turned off on system-wide
> suspend/resume.
> 
> IMO, unless there's a good reason not to, you should pass NULL for the
> governor.

It's for legacy when VPU is initialized from vendor U-Boot, look at commit :
339cd0ea082287ea8e2b7e7159a5a33665a2cbe3 "soc: amlogic: meson-gx-pwrc-vpu: fix power-off when powered by bootloader"

    In the case the VPU power domain has been powered on by the bootloader
    and no driver are attached to this power domain, the genpd will power it
    off after a certain amount of time, but the clocks hasn't been enabled
    by the kernel itself and the power-off will trigger some faults.
    This patch enable the clocks to have a coherent state for an eventual
    poweroff and switches to the pm_domain_always_on_gov governor.

I could set always-on governor only if the domain was already enabled,
what do you think ?

And seems I'm also missing the "This patch enable the clocks".

> 
>> +	return 0;
>> +}
>> +
>> +static int meson_ee_pwrc_probe(struct platform_device *pdev)
>> +{
>> +	const struct meson_ee_pwrc_domain_data *match;
>> +	struct regmap *regmap_ao, *regmap_hhi;
>> +	struct meson_ee_pwrc *sm1_pwrc;
> 
> Why the sm1_ prefix throughout this code?  This is now generalized for
> more SoCs, so shouldn't be sm1.  I'm assuming this is just left-over
> from the original version.  IMO, just called it pwrc.

Indeed, it's a leftover.

> 
>> +	int i, ret;
>> +
>> +	match = of_device_get_match_data(&pdev->dev);
>> +	if (!match) {
>> +		dev_err(&pdev->dev, "failed to get match data\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	sm1_pwrc = devm_kzalloc(&pdev->dev, sizeof(*sm1_pwrc), GFP_KERNEL);
>> +	if (!sm1_pwrc)
>> +		return -ENOMEM;
>> +
>> +	sm1_pwrc->xlate.domains =
>> +		devm_kcalloc(&pdev->dev,
>> +			     match->count,
>> +			     sizeof(*sm1_pwrc->xlate.domains),
>> +			     GFP_KERNEL);
>> +	if (!sm1_pwrc->xlate.domains)
>> +		return -ENOMEM;
>> +
>> +	sm1_pwrc->domains =
>> +		devm_kcalloc(&pdev->dev,
> 
> nit: this line probably doesn't need to be wrapped.

Ok

> 
>> +			     match->count,
>> +			     sizeof(*sm1_pwrc->domains),
>> +			     GFP_KERNEL);
>> +	if (!sm1_pwrc->domains)
>> +		return -ENOMEM;
>> +
>> +	sm1_pwrc->xlate.num_domains = match->count;
>> +
>> +	regmap_hhi = syscon_node_to_regmap(of_get_parent(pdev->dev.of_node));
>> +	if (IS_ERR(regmap_hhi)) {
>> +		dev_err(&pdev->dev, "failed to get HHI regmap\n");
>> +		return PTR_ERR(regmap_hhi);
>> +	}
>> +
>> +	regmap_ao = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
>> +						    "amlogic,ao-sysctrl");
>> +	if (IS_ERR(regmap_ao)) {
>> +		dev_err(&pdev->dev, "failed to get AO regmap\n");
>> +		return PTR_ERR(regmap_ao);
>> +	}
>> +
>> +	sm1_pwrc->regmap_ao = regmap_ao;
>> +	sm1_pwrc->regmap_hhi = regmap_hhi;
>> +
>> +	platform_set_drvdata(pdev, sm1_pwrc);
>> +
>> +	for (i = 0 ; i < match->count ; ++i) {
>> +		struct meson_ee_pwrc_domain *dom = &sm1_pwrc->domains[i];
>> +
>> +		memcpy(&dom->desc, &match->domains[i], sizeof(dom->desc));
>> +
>> +		ret = meson_ee_pwrc_init_domain(pdev, sm1_pwrc, dom);
>> +		if (ret)
>> +			return ret;
>> +
>> +		sm1_pwrc->xlate.domains[i] = &dom->base;
>> +	}
>> +
>> +	of_genpd_add_provider_onecell(pdev->dev.of_node, &sm1_pwrc->xlate);
>> +
>> +	return 0;
>> +}
>> +
>> +static struct meson_ee_pwrc_domain_data meson_ee_g12a_pwrc_data = {
>> +	.count = ARRAY_SIZE(g12a_pwrc_domains),
>> +	.domains = g12a_pwrc_domains,
>> +};
>> +
>> +static struct meson_ee_pwrc_domain_data meson_ee_sm1_pwrc_data = {
>> +	.count = ARRAY_SIZE(sm1_pwrc_domains),
>> +	.domains = sm1_pwrc_domains,
>> +};
>> +
>> +static const struct of_device_id meson_ee_pwrc_match_table[] = {
>> +	{
>> +		.compatible = "amlogic,meson-g12a-pwrc",
>> +		.data = &meson_ee_g12a_pwrc_data,
>> +	},
>> +	{
>> +		.compatible = "amlogic,meson-sm1-pwrc",
>> +		.data = &meson_ee_sm1_pwrc_data,
>> +	},
>> +	{ /* sentinel */ }
>> +};
>> +
>> +static struct platform_driver meson_ee_pwrc_driver = {
>> +	.probe	= meson_ee_pwrc_probe,
>> +	.driver = {
>> +		.name		= "meson_ee_pwrc",
>> +		.of_match_table	= meson_ee_pwrc_match_table,
>> +	},
>> +};
>> +builtin_platform_driver(meson_ee_pwrc_driver);
>> -- 
>> 2.22.0

Thanks,
Neil

> 
> Kevin
> 
> [1] https://lore.kernel.org/linux-amlogic/20190809230904.28747-1-khilman@baylibre.com/
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] net/wireless: Delete unnecessary checks before the macro call “dev_kfree_skb”
From: Markus Elfring @ 2019-08-22  8:30 UTC (permalink / raw)
  To: linux-wireless, linux-arm-kernel, linux-mediatek, ath10k,
	David S. Miller, Felix Fietkau, Kalle Valo, Lorenzo Bianconi,
	Matthias Brugger, Roy Luo, Ryder Lee, Solomon Peachy,
	Stanislaw Gruszka
  Cc: kernel-janitors, LKML

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 22 Aug 2019 10:20:10 +0200

The dev_kfree_skb() function performs also input parameter validation.
Thus the test around the shown calls is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/ath/ath10k/wmi.c               | 4 +---
 drivers/net/wireless/intel/iwlegacy/3945-mac.c      | 8 ++------
 drivers/net/wireless/intel/iwlegacy/common.c        | 8 ++------
 drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c | 5 +----
 drivers/net/wireless/st/cw1200/scan.c               | 3 +--
 5 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 4f707c6394bb..d384293429b4 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -9440,7 +9440,5 @@ void ath10k_wmi_detach(struct ath10k *ar)
 	}

 	cancel_work_sync(&ar->svc_rdy_work);
-
-	if (ar->svc_rdy_skb)
-		dev_kfree_skb(ar->svc_rdy_skb);
+	dev_kfree_skb(ar->svc_rdy_skb);
 }
diff --git a/drivers/net/wireless/intel/iwlegacy/3945-mac.c b/drivers/net/wireless/intel/iwlegacy/3945-mac.c
index b82da75a9ae3..4b3b166f6f2a 100644
--- a/drivers/net/wireless/intel/iwlegacy/3945-mac.c
+++ b/drivers/net/wireless/intel/iwlegacy/3945-mac.c
@@ -2302,9 +2302,7 @@ __il3945_down(struct il_priv *il)
 	il3945_hw_txq_ctx_free(il);
 exit:
 	memset(&il->card_alive, 0, sizeof(struct il_alive_resp));
-
-	if (il->beacon_skb)
-		dev_kfree_skb(il->beacon_skb);
+	dev_kfree_skb(il->beacon_skb);
 	il->beacon_skb = NULL;

 	/* clear out any free frames */
@@ -3847,9 +3845,7 @@ il3945_pci_remove(struct pci_dev *pdev)
 	il_free_channel_map(il);
 	il_free_geos(il);
 	kfree(il->scan_cmd);
-	if (il->beacon_skb)
-		dev_kfree_skb(il->beacon_skb);
-
+	dev_kfree_skb(il->beacon_skb);
 	ieee80211_free_hw(il->hw);
 }

diff --git a/drivers/net/wireless/intel/iwlegacy/common.c b/drivers/net/wireless/intel/iwlegacy/common.c
index 73f7bbf742bc..4e7e64f46ea8 100644
--- a/drivers/net/wireless/intel/iwlegacy/common.c
+++ b/drivers/net/wireless/intel/iwlegacy/common.c
@@ -5182,8 +5182,7 @@ il_mac_reset_tsf(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	memset(&il->current_ht_config, 0, sizeof(struct il_ht_config));

 	/* new association get rid of ibss beacon skb */
-	if (il->beacon_skb)
-		dev_kfree_skb(il->beacon_skb);
+	dev_kfree_skb(il->beacon_skb);
 	il->beacon_skb = NULL;
 	il->timestamp = 0;

@@ -5302,10 +5301,7 @@ il_beacon_update(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	}

 	spin_lock_irqsave(&il->lock, flags);
-
-	if (il->beacon_skb)
-		dev_kfree_skb(il->beacon_skb);
-
+	dev_kfree_skb(il->beacon_skb);
 	il->beacon_skb = skb;

 	timestamp = ((struct ieee80211_mgmt *)skb->data)->u.beacon.timestamp;
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
index d61c686e08de..d6487cd67cca 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_beacon.c
@@ -88,10 +88,7 @@ int mt76x02_mac_set_beacon(struct mt76x02_dev *dev, u8 vif_idx,
 	for (i = 0; i < ARRAY_SIZE(dev->beacons); i++) {
 		if (vif_idx == i) {
 			force_update = !!dev->beacons[i] ^ !!skb;
-
-			if (dev->beacons[i])
-				dev_kfree_skb(dev->beacons[i]);
-
+			dev_kfree_skb(dev->beacons[i]);
 			dev->beacons[i] = skb;
 			__mt76x02_mac_set_beacon(dev, bcn_idx, skb);
 		} else if (force_update && dev->beacons[i]) {
diff --git a/drivers/net/wireless/st/cw1200/scan.c b/drivers/net/wireless/st/cw1200/scan.c
index c46b044b7f7b..988581cc134b 100644
--- a/drivers/net/wireless/st/cw1200/scan.c
+++ b/drivers/net/wireless/st/cw1200/scan.c
@@ -120,8 +120,7 @@ int cw1200_hw_scan(struct ieee80211_hw *hw,
 		++priv->scan.n_ssids;
 	}

-	if (frame.skb)
-		dev_kfree_skb(frame.skb);
+	dev_kfree_skb(frame.skb);
 	mutex_unlock(&priv->conf_mutex);
 	queue_work(priv->workqueue, &priv->scan.work);
 	return 0;
--
2.23.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


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