* Re: [PATCH 4/4] arm64: dts: add support for A1 based Amlogic AD401
From: Jianxin Pan @ 2019-09-03 8:16 UTC (permalink / raw)
To: Neil Armstrong, Kevin Hilman, linux-amlogic
Cc: devicetree, Hanjie Lin, Victor Wan, Martin Blumenstingl,
linux-kernel, Qiufang Dai, Rob Herring, Jian Hu, Xingyu Chen,
Carlo Caione, Tao Zeng, linux-arm-kernel, Jerome Brunet
In-Reply-To: <97a462d6-d98e-f778-96d5-bacd4801df6b@baylibre.com>
Hi Neil,
Thanks for your time.
Please see my comments below.
On 2019/9/3 15:42, Neil Armstrong wrote:
> Hi,
>
> On 03/09/2019 08:51, Jianxin Pan wrote:
>> Add basic support for the Amlogic A1 based Amlogic AD401 board:
>> which describe components as follows: Reserve Memory, CPU, GIC, IRQ,
[...]
>> + chosen {
>> + stdout-path = "serial0:115200n8";
>> + };
>> + memory@0 {
>> + device_type = "memory";
>> + linux,usable-memory = <0x0 0x0 0x0 0x8000000>;
>
> I'll prefer usage of reg, it's handled the same but linux,usable-memory
> is not documented.
>
OK, I will fix it in the next version. Thanks for your review.
>> + };
>> +};
>> +
>> +&uart_AO_B {
>> + status = "okay";
>> + /*pinctrl-0 = <&uart_ao_a_pins>;*/
>> + /*pinctrl-names = "default";*/
>
> Please remove these lines instead of commenting them.
>
OK, I will fix it in the next version.
>> +};
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>> new file mode 100644
>> index 00000000..b98d648
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>> @@ -0,0 +1,121 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>> + */
>> +
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +/ {
[...]
>> +
>> + reserved-memory {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + ranges;
>
> Isn't there secmon reserved memory ?
>
A1 uses internal SRAM as secmon memory.
And there is no secmon reserved memory in ddr side.
>> +
>> + linux,cma {
>> + compatible = "shared-dma-pool";
>> + reusable;
>> + size = <0x0 0x800000>;
[...]
>>
>
> Thanks,
> Neil
>
> .
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCHv3 01/10] dt-bindings: omap: add new binding for PRM instances
From: Tero Kristo @ 2019-09-03 8:14 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, Tony Lindgren, Philipp Zabel, Santosh Shilimkar,
linux-omap,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAL_Jsq+AJj1bgOQYG=c86A5HC_g2UZph387oVEKZyP4M18kURw@mail.gmail.com>
On 03/09/2019 11:10, Rob Herring wrote:
> On Tue, Sep 3, 2019 at 8:26 AM Tero Kristo <t-kristo@ti.com> wrote:
>>
>> On 02/09/2019 16:39, Rob Herring wrote:
>>> On Fri, Aug 30, 2019 at 03:18:07PM +0300, Tero Kristo wrote:
>>>> Add new binding for OMAP PRM (Power and Reset Manager) instances. Each
>>>> of these will act as a power domain controller and potentially as a reset
>>>> provider.
>>>>
>>>
>>> Converting this to schema would be nice.
>>
>> Do you have documentation about schema somewhere? Basically what I need
>> to do to fix this.
>
> Documentation/devicetree/writing-schema.md (.rst in -next)
> Documentation/devicetree/bindings/example-schema.yaml
>
>>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>>> ---
>>>> .../devicetree/bindings/arm/omap/prm-inst.txt | 31 +++++++++++++++++++
>>>
>>> bindings/reset/
>>
>> I did not put this under reset, because this is basically a
>> multi-purpose function. Reset just happens to be the first functionality
>> it is going to provide. It will be followed by power domain support
>> later on.
>>
>> Any thoughts?
>
> I prefer that bindings be complete as possible even if driver support
> is not there yet. Adding power domain support may only mean adding
> '#power-domain-cells'.
>
> The location is fine then.
Yeah, I assume just adding power-domain-cells should be enough. I am not
too sure before I start trying this out though so did not want to add it
yet.
>
>>>> 1 file changed, 31 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/arm/omap/prm-inst.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/omap/prm-inst.txt b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt
>>>> new file mode 100644
>>>> index 000000000000..7c7527c37734
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt
>>>> @@ -0,0 +1,31 @@
>>>> +OMAP PRM instance bindings
>>>> +
>>>> +Power and Reset Manager is an IP block on OMAP family of devices which
>>>> +handle the power domains and their current state, and provide reset
>>>> +handling for the domains and/or separate IP blocks under the power domain
>>>> +hierarchy.
>>>> +
>>>> +Required properties:
>>>> +- compatible: Must be one of:
>>>> + "ti,am3-prm-inst"
>>>> + "ti,am4-prm-inst"
>>>> + "ti,omap4-prm-inst"
>>>> + "ti,omap5-prm-inst"
>>>> + "ti,dra7-prm-inst"
>>>
>>> '-inst' seems a bit redundant.
>>
>> ti,xyz-prm is already reserved by the parent node of all these.
>>
>> The hierarchy is basically like this (omap4 as example):
>>
>> prm: prm@4a306000 {
>> compatible = "ti,omap4-prm";
>> ...
>>
>> prm_dsp: prm@400 {
>> compatible = "ti,omap4-prm-inst";
>> ...
>> };
>>
>> prm_device: prm@1b00 {
>> compatible = "ti,omap4-prm-inst";
>> ...
>> };
>>
>> ...
>> };
>
> Okay. Then you need to state this binding must be a child of PRM. The
> schema would need to take this into account too, so probably best to
> not convert this yet.
>
Ok thanks, I'll make the necessary updates and post v4.
-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3] drm/mcde: Fix DSI transfers
From: Linus Walleij @ 2019-09-03 8:11 UTC (permalink / raw)
To: dri-devel, Maarten Lankhorst, Maxime Ripard, Sean Paul
Cc: Linus Walleij, Stephan Gerhold, kbuild test robot,
linux-arm-kernel
There were bugs in the DSI transfer (read and write) function
as it was only tested with displays ever needing a single byte
to be written. Fixed it up and tested so we can now write
messages of up to 16 bytes and read up to 4 bytes from the
display.
Tested with a Sony ACX424AKP display: this display now self-
identifies and can control backlight in command mode.
Reported-by: kbuild test robot <lkp@intel.com>
Fixes: 5fc537bfd000 ("drm/mcde: Add new driver for ST-Ericsson MCDE")
Reviewed-by: Stephan Gerhold <stephan@gerhold.net>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v2->v3:
- Fix an error message to indicate reading error rather than
writing error.
- Use the local variable for underflow print.
- Collected Stephan's reviewed-by.
ChangeLog v1->v2:
- Fix a print modifier for dev_err() found by the build robot.
---
drivers/gpu/drm/mcde/mcde_dsi.c | 70 ++++++++++++++++++++++-----------
1 file changed, 47 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/mcde/mcde_dsi.c b/drivers/gpu/drm/mcde/mcde_dsi.c
index 07f7090d08b3..cd261c266f35 100644
--- a/drivers/gpu/drm/mcde/mcde_dsi.c
+++ b/drivers/gpu/drm/mcde/mcde_dsi.c
@@ -178,22 +178,26 @@ static ssize_t mcde_dsi_host_transfer(struct mipi_dsi_host *host,
const u32 loop_delay_us = 10; /* us */
const u8 *tx = msg->tx_buf;
u32 loop_counter;
- size_t txlen;
+ size_t txlen = msg->tx_len;
+ size_t rxlen = msg->rx_len;
u32 val;
int ret;
int i;
- txlen = msg->tx_len;
- if (txlen > 12) {
+ if (txlen > 16) {
dev_err(d->dev,
- "dunno how to write more than 12 bytes yet\n");
+ "dunno how to write more than 16 bytes yet\n");
+ return -EIO;
+ }
+ if (rxlen > 4) {
+ dev_err(d->dev,
+ "dunno how to read more than 4 bytes yet\n");
return -EIO;
}
dev_dbg(d->dev,
- "message to channel %d, %zd bytes",
- msg->channel,
- txlen);
+ "message to channel %d, write %zd bytes read %zd bytes\n",
+ msg->channel, txlen, rxlen);
/* Command "nature" */
if (MCDE_DSI_HOST_IS_READ(msg->type))
@@ -210,9 +214,7 @@ static ssize_t mcde_dsi_host_transfer(struct mipi_dsi_host *host,
if (mipi_dsi_packet_format_is_long(msg->type))
val |= DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_LONGNOTSHORT;
val |= 0 << DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_ID_SHIFT;
- /* Add one to the length for the MIPI DCS command */
- val |= txlen
- << DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_SIZE_SHIFT;
+ val |= txlen << DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_SIZE_SHIFT;
val |= DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_LP_EN;
val |= msg->type << DSI_DIRECT_CMD_MAIN_SETTINGS_CMD_HEAD_SHIFT;
writel(val, d->regs + DSI_DIRECT_CMD_MAIN_SETTINGS);
@@ -249,17 +251,36 @@ static ssize_t mcde_dsi_host_transfer(struct mipi_dsi_host *host,
writel(1, d->regs + DSI_DIRECT_CMD_SEND);
loop_counter = 1000 * 1000 / loop_delay_us;
- while (!(readl(d->regs + DSI_DIRECT_CMD_STS) &
- DSI_DIRECT_CMD_STS_WRITE_COMPLETED)
- && --loop_counter)
- usleep_range(loop_delay_us, (loop_delay_us * 3) / 2);
-
- if (!loop_counter) {
- dev_err(d->dev, "DSI write timeout!\n");
- return -ETIME;
+ if (MCDE_DSI_HOST_IS_READ(msg->type)) {
+ /* Read command */
+ while (!(readl(d->regs + DSI_DIRECT_CMD_STS) &
+ (DSI_DIRECT_CMD_STS_READ_COMPLETED |
+ DSI_DIRECT_CMD_STS_READ_COMPLETED_WITH_ERR))
+ && --loop_counter)
+ usleep_range(loop_delay_us, (loop_delay_us * 3) / 2);
+ if (!loop_counter) {
+ dev_err(d->dev, "DSI write timeout!\n");
+ return -ETIME;
+ }
+ } else {
+ /* Writing only */
+ while (!(readl(d->regs + DSI_DIRECT_CMD_STS) &
+ DSI_DIRECT_CMD_STS_WRITE_COMPLETED)
+ && --loop_counter)
+ usleep_range(loop_delay_us, (loop_delay_us * 3) / 2);
+
+ if (!loop_counter) {
+ dev_err(d->dev, "DSI read timeout!\n");
+ return -ETIME;
+ }
}
val = readl(d->regs + DSI_DIRECT_CMD_STS);
+ if (val & DSI_DIRECT_CMD_STS_READ_COMPLETED_WITH_ERR) {
+ dev_err(d->dev, "read completed with error\n");
+ writel(1, d->regs + DSI_DIRECT_CMD_RD_INIT);
+ return -EIO;
+ }
if (val & DSI_DIRECT_CMD_STS_ACKNOWLEDGE_WITH_ERR_RECEIVED) {
val >>= DSI_DIRECT_CMD_STS_ACK_VAL_SHIFT;
dev_err(d->dev, "error during transmission: %04x\n",
@@ -269,10 +290,7 @@ static ssize_t mcde_dsi_host_transfer(struct mipi_dsi_host *host,
if (!MCDE_DSI_HOST_IS_READ(msg->type)) {
/* Return number of bytes written */
- if (mipi_dsi_packet_format_is_long(msg->type))
- ret = 4 + txlen;
- else
- ret = 4;
+ ret = txlen;
} else {
/* OK this is a read command, get the response */
u32 rdsz;
@@ -282,7 +300,13 @@ static ssize_t mcde_dsi_host_transfer(struct mipi_dsi_host *host,
rdsz = readl(d->regs + DSI_DIRECT_CMD_RD_PROPERTY);
rdsz &= DSI_DIRECT_CMD_RD_PROPERTY_RD_SIZE_MASK;
rddat = readl(d->regs + DSI_DIRECT_CMD_RDDAT);
- for (i = 0; i < 4 && i < rdsz; i++)
+ if (rdsz < rxlen) {
+ dev_err(d->dev, "read error, requested %zd got %zd\n",
+ msg->rx_len, rxlen);
+ return -EIO;
+ }
+ /* FIXME: read more than 4 bytes */
+ for (i = 0; i < 4 && i < rxlen; i++)
rx[i] = (rddat >> (i * 8)) & 0xff;
ret = rdsz;
}
--
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: [PATCHv3 01/10] dt-bindings: omap: add new binding for PRM instances
From: Rob Herring @ 2019-09-03 8:10 UTC (permalink / raw)
To: Tero Kristo
Cc: devicetree, Tony Lindgren, Philipp Zabel, Santosh Shilimkar,
linux-omap,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <e8d700cd-8f3c-5cea-a022-b20a595fc1e1@ti.com>
On Tue, Sep 3, 2019 at 8:26 AM Tero Kristo <t-kristo@ti.com> wrote:
>
> On 02/09/2019 16:39, Rob Herring wrote:
> > On Fri, Aug 30, 2019 at 03:18:07PM +0300, Tero Kristo wrote:
> >> Add new binding for OMAP PRM (Power and Reset Manager) instances. Each
> >> of these will act as a power domain controller and potentially as a reset
> >> provider.
> >>
> >
> > Converting this to schema would be nice.
>
> Do you have documentation about schema somewhere? Basically what I need
> to do to fix this.
Documentation/devicetree/writing-schema.md (.rst in -next)
Documentation/devicetree/bindings/example-schema.yaml
> >> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> >> ---
> >> .../devicetree/bindings/arm/omap/prm-inst.txt | 31 +++++++++++++++++++
> >
> > bindings/reset/
>
> I did not put this under reset, because this is basically a
> multi-purpose function. Reset just happens to be the first functionality
> it is going to provide. It will be followed by power domain support
> later on.
>
> Any thoughts?
I prefer that bindings be complete as possible even if driver support
is not there yet. Adding power domain support may only mean adding
'#power-domain-cells'.
The location is fine then.
> >> 1 file changed, 31 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/arm/omap/prm-inst.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/arm/omap/prm-inst.txt b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt
> >> new file mode 100644
> >> index 000000000000..7c7527c37734
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt
> >> @@ -0,0 +1,31 @@
> >> +OMAP PRM instance bindings
> >> +
> >> +Power and Reset Manager is an IP block on OMAP family of devices which
> >> +handle the power domains and their current state, and provide reset
> >> +handling for the domains and/or separate IP blocks under the power domain
> >> +hierarchy.
> >> +
> >> +Required properties:
> >> +- compatible: Must be one of:
> >> + "ti,am3-prm-inst"
> >> + "ti,am4-prm-inst"
> >> + "ti,omap4-prm-inst"
> >> + "ti,omap5-prm-inst"
> >> + "ti,dra7-prm-inst"
> >
> > '-inst' seems a bit redundant.
>
> ti,xyz-prm is already reserved by the parent node of all these.
>
> The hierarchy is basically like this (omap4 as example):
>
> prm: prm@4a306000 {
> compatible = "ti,omap4-prm";
> ...
>
> prm_dsp: prm@400 {
> compatible = "ti,omap4-prm-inst";
> ...
> };
>
> prm_device: prm@1b00 {
> compatible = "ti,omap4-prm-inst";
> ...
> };
>
> ...
> };
Okay. Then you need to state this binding must be a child of PRM. The
schema would need to take this into account too, so probably best to
not convert this yet.
Rob
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 2/3] ARM: dts: imx6ull-colibri: add phy-supply and respective regulator
From: Philippe Schenker @ 2019-09-03 8:03 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, Mark Brown, Shawn Guo, Sascha Hauer,
Liam Girdwood
Cc: Mark Rutland, devicetree@vger.kernel.org, Luka Pivk, Stefan Agner,
Marcel Ziswiler, Philippe Schenker, Rob Herring, NXP Linux Team,
Max Krummenacher, Fabio Estevam, Pengutronix Kernel Team,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190903080336.32288-1-philippe.schenker@toradex.com>
This adds regulator-fixed-clock, a fixed-regulator that turns on and
off with a clock and add it to the phy.
Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
---
arch/arm/boot/dts/imx6ull-colibri.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/imx6ull-colibri.dtsi b/arch/arm/boot/dts/imx6ull-colibri.dtsi
index d56728f03c35..76021b842a97 100644
--- a/arch/arm/boot/dts/imx6ull-colibri.dtsi
+++ b/arch/arm/boot/dts/imx6ull-colibri.dtsi
@@ -47,6 +47,17 @@
states = <1800000 0x1 3300000 0x0>;
vin-supply = <®_module_3v3>;
};
+
+ reg_eth_phy: regulator-eth-phy {
+ compatible = "regulator-fixed-clock";
+ regulator-boot-on;
+ regulator-name = "eth_phy";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ clocks = <&clks IMX6UL_CLK_ENET2_REF_125M>;
+ startup-delay-us = <150000>;
+ vin-supply = <®_module_3v3>;
+ };
};
&adc1 {
@@ -66,6 +77,7 @@
pinctrl-0 = <&pinctrl_enet2>;
phy-mode = "rmii";
phy-handle = <ðphy1>;
+ phy-supply = <®_eth_phy>;
status = "okay";
mdio {
--
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
* [PATCH 0/3] Add new binding regulator-fixed-clock to regulator-fixed
From: Philippe Schenker @ 2019-09-03 8:03 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, Mark Brown, Shawn Guo, Sascha Hauer,
Liam Girdwood
Cc: Mark Rutland, devicetree@vger.kernel.org, Luka Pivk, Stefan Agner,
Marcel Ziswiler, Philippe Schenker, Rob Herring, NXP Linux Team,
Max Krummenacher, Fabio Estevam, Pengutronix Kernel Team,
linux-arm-kernel@lists.infradead.org
Our hardware has a FET that is switching power rail of the ethernet PHY
on and off. This switching enable signal is a clock from the SoC.
There is no possibility in regulator subsystem to have this hardware
reflected in software.
I already discussed with Mark Brown about possible solutions and he
suggested to create at least a new compatible. [1]
This discussion includes also a better explanation of our circuit as
well as schematics. So please refer to that link if you have questions
about that.
In this first attempt I created a new binding "regulator-fixed-clock"
that can take a clock from devicetree. This is a simple addition to
regulator-fixed. If the binding regulator-fixed-clock is given, the
clock is simply enabled on regulator enable and disabled on regulator
disable.
To be able to have multiple consumers a counter variable is also given
that tells how many consumers need power from this regulator.
Best regards,
Philippe
[1] https://lkml.org/lkml/2019/8/7/78
Philippe Schenker (3):
regulator: fixed: add possibility to enable by clock
ARM: dts: imx6ull-colibri: add phy-supply and respective regulator
dt-bindings: regulator: add regulator-fixed-clock binding
.../bindings/regulator/fixed-regulator.yaml | 18 +++-
arch/arm/boot/dts/imx6ull-colibri.dtsi | 12 +++
drivers/regulator/fixed.c | 86 ++++++++++++++++++-
3 files changed, 112 insertions(+), 4 deletions(-)
--
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
* Re: [PATCH v4 00/10] arm64: Stolen time support
From: Andrew Jones @ 2019-09-03 8:03 UTC (permalink / raw)
To: Steven Price
Cc: Mark Rutland, kvm, Radim Krčmář, Marc Zyngier,
Suzuki K Pouloze, linux-doc, Russell King, linux-kernel,
James Morse, Julien Thierry, Catalin Marinas, Paolo Bonzini,
Will Deacon, kvmarm, linux-arm-kernel
In-Reply-To: <20190830084255.55113-1-steven.price@arm.com>
On Fri, Aug 30, 2019 at 09:42:45AM +0100, Steven Price wrote:
> This series add support for paravirtualized time for arm64 guests and
> KVM hosts following the specification in Arm's document DEN 0057A:
>
> https://developer.arm.com/docs/den0057/a
>
> It implements support for stolen time, allowing the guest to
> identify time when it is forcibly not executing.
>
> It doesn't implement support for Live Physical Time (LPT) as there are
> some concerns about the overheads and approach in the above
> specification, and I expect an updated version of the specification to
> be released soon with just the stolen time parts.
>
> NOTE: Patches 8 and 9 will conflict with Mark Rutland's series[1] cleaning
> up the SMCCC conduit. I do feel that the addition of an _invoke() call
> makes a number of call sites cleaner and it should be possible to
> integrate both this and Mark's other cleanups.
>
> [1] https://lore.kernel.org/linux-arm-kernel/20190809132245.43505-1-mark.rutland@arm.com/
>
> Also available as a git tree:
> git://linux-arm.org/linux-sp.git stolen_time/v4
>
> Changes from v3:
> https://lore.kernel.org/lkml/20190821153656.33429-1-steven.price@arm.com/
> * There's no longer a PV_TIME device, instead there are attributes on
> the VCPU. This allows the stolen time structures to be places
> arbitrarily by user space (subject to 64 byte alignment).
> * Split documentation between information on the hypercalls and the
> attributes on the VCPU
> * Fixed the type of SMCCC functions to return long not int
>
> Changes from v2:
> https://lore.kernel.org/lkml/20190819140436.12207-1-steven.price@arm.com/
> * Switched from using gfn_to_hva_cache to a new macro kvm_put_guest()
> that can provide the single-copy atomicity required (on arm64). This
> macro is added in patch 4.
> * Tidied up the locking for kvm_update_stolen_time().
> pagefault_disable() was unnecessary and the caller didn't need to
> take kvm->srcu as the function does it itself.
> * Removed struct kvm_arch_pvtime from the arm implementation, replaced
> instead with inline static functions which are empty for arm.
> * Fixed a few checkpatch --strict warnings.
>
> Changes from v1:
> https://lore.kernel.org/lkml/20190802145017.42543-1-steven.price@arm.com/
> * Host kernel no longer allocates the stolen time structure, instead it
> is allocated by user space. This means the save/restore functionality
> can be removed.
> * Refactored the code so arm has stub implementations and to avoid
> initcall
> * Rebased to pick up Documentation/{virt->virtual} change
> * Bunch of typo fixes
>
> Christoffer Dall (1):
> KVM: arm/arm64: Factor out hypercall handling from PSCI code
>
> Steven Price (9):
> KVM: arm64: Document PV-time interface
> KVM: arm64: Implement PV_FEATURES call
> KVM: Implement kvm_put_guest()
> KVM: arm64: Support stolen time reporting via shared structure
> KVM: Allow kvm_device_ops to be const
> KVM: arm64: Provide VCPU attributes for stolen time
> arm/arm64: Provide a wrapper for SMCCC 1.1 calls
> arm/arm64: Make use of the SMCCC 1.1 wrapper
> arm64: Retrieve stolen time as paravirtualized guest
>
> Documentation/virt/kvm/arm/pvtime.txt | 64 ++++++++++
> Documentation/virt/kvm/devices/vcpu.txt | 14 +++
> arch/arm/include/asm/kvm_host.h | 26 +++++
> arch/arm/kvm/Makefile | 2 +-
> arch/arm/kvm/handle_exit.c | 2 +-
> arch/arm/mm/proc-v7-bugs.c | 13 +--
> arch/arm64/include/asm/kvm_host.h | 30 ++++-
> arch/arm64/include/asm/paravirt.h | 9 +-
> arch/arm64/include/asm/pvclock-abi.h | 17 +++
> arch/arm64/include/uapi/asm/kvm.h | 2 +
> arch/arm64/kernel/cpu_errata.c | 80 +++++--------
> arch/arm64/kernel/paravirt.c | 148 ++++++++++++++++++++++++
> arch/arm64/kernel/time.c | 3 +
> arch/arm64/kvm/Kconfig | 1 +
> arch/arm64/kvm/Makefile | 2 +
> arch/arm64/kvm/guest.c | 9 ++
> arch/arm64/kvm/handle_exit.c | 4 +-
> include/kvm/arm_hypercalls.h | 43 +++++++
> include/kvm/arm_psci.h | 2 +-
> include/linux/arm-smccc.h | 58 ++++++++++
> include/linux/cpuhotplug.h | 1 +
> include/linux/kvm_host.h | 26 ++++-
> include/linux/kvm_types.h | 2 +
> include/uapi/linux/kvm.h | 2 +
> virt/kvm/arm/arm.c | 11 ++
> virt/kvm/arm/hypercalls.c | 68 +++++++++++
> virt/kvm/arm/psci.c | 84 +-------------
> virt/kvm/arm/pvtime.c | 124 ++++++++++++++++++++
> virt/kvm/kvm_main.c | 6 +-
> 29 files changed, 699 insertions(+), 154 deletions(-)
> create mode 100644 Documentation/virt/kvm/arm/pvtime.txt
> create mode 100644 arch/arm64/include/asm/pvclock-abi.h
> create mode 100644 include/kvm/arm_hypercalls.h
> create mode 100644 virt/kvm/arm/hypercalls.c
> create mode 100644 virt/kvm/arm/pvtime.c
>
> --
> 2.20.1
>
Hi Steven,
I had some fun testing this series with the KVM selftests framework. It
looks like it works to me, so you may add
Tested-by: Andrew Jones <drjones@redhat.com>
if you like. And below is the test I came up with.
Thanks,
drew
From: Andrew Jones <drjones@redhat.com>
Date: Tue, 3 Sep 2019 03:45:08 -0400
Subject: [PATCH] selftests: kvm: aarch64 stolen-time test
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
tools/testing/selftests/kvm/Makefile | 1 +
.../selftests/kvm/aarch64/stolen-time.c | 208 ++++++++++++++++++
2 files changed, 209 insertions(+)
create mode 100644 tools/testing/selftests/kvm/aarch64/stolen-time.c
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index ba7849751989..3151264039ad 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -28,6 +28,7 @@ TEST_GEN_PROGS_x86_64 += clear_dirty_log_test
TEST_GEN_PROGS_x86_64 += dirty_log_test
TEST_GEN_PROGS_x86_64 += kvm_create_max_vcpus
+TEST_GEN_PROGS_aarch64 += aarch64/stolen-time
TEST_GEN_PROGS_aarch64 += clear_dirty_log_test
TEST_GEN_PROGS_aarch64 += dirty_log_test
TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
diff --git a/tools/testing/selftests/kvm/aarch64/stolen-time.c b/tools/testing/selftests/kvm/aarch64/stolen-time.c
new file mode 100644
index 000000000000..36df2f6baa17
--- /dev/null
+++ b/tools/testing/selftests/kvm/aarch64/stolen-time.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AArch64 PV stolen time test
+ *
+ * Copyright (C) 2019, Red Hat, Inc.
+ */
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <time.h>
+#include <sched.h>
+#include <pthread.h>
+#include <sys/syscall.h>
+#include "kvm_util.h"
+
+#define ST_IPA_BASE (1 << 30)
+#define MIN_STOLEN_TIME 200000
+
+struct st_time {
+ uint32_t rev;
+ uint32_t attr;
+ uint64_t st_time;
+};
+
+static uint64_t st_ipa_offset[4];
+static uint64_t guest_stolen_time[4];
+
+static void guest_code(void)
+{
+ struct st_time *st_time;
+ uint64_t cpu;
+ int64_t ipa;
+ int32_t ret;
+
+ asm volatile("mrs %0, mpidr_el1" : "=r" (cpu));
+ cpu &= 0x3;
+
+ asm volatile(
+ "mov x0, %1\n"
+ "mov x1, %2\n"
+ "hvc #0\n"
+ "mov %0, x0\n"
+ : "=r" (ret) : "r" (0x80000001), "r" (0xc5000020) :
+ "x0", "x1", "x2", "x3");
+
+ GUEST_ASSERT(ret == 0);
+
+ asm volatile(
+ "mov x0, %1\n"
+ "mov x1, %2\n"
+ "hvc #0\n"
+ "mov %0, x0\n"
+ : "=r" (ret) : "r" (0xc5000020), "r" (0xc5000022) :
+ "x0", "x1", "x2", "x3");
+
+ GUEST_ASSERT(ret == 0);
+
+ asm volatile(
+ "mov x0, %1\n"
+ "hvc #0\n"
+ "mov %0, x0\n"
+ : "=r" (ipa) : "r" (0xc5000022) :
+ "x0", "x1", "x2", "x3");
+
+ GUEST_ASSERT(ipa == ST_IPA_BASE + st_ipa_offset[cpu]);
+
+ st_time = (struct st_time *)ipa;
+ GUEST_ASSERT(st_time->rev == 0);
+ GUEST_ASSERT(st_time->attr == 0);
+
+ guest_stolen_time[cpu] = st_time->st_time;
+ GUEST_SYNC(0);
+
+ guest_stolen_time[cpu] = st_time->st_time;
+ GUEST_DONE();
+}
+
+static long get_run_delay(void)
+{
+ char path[64];
+ long val[2];
+ FILE *fp;
+
+ sprintf(path, "/proc/%ld/schedstat", syscall(SYS_gettid));
+ fp = fopen(path, "r");
+ fscanf(fp, "%ld %ld ", &val[0], &val[1]);
+ fclose(fp);
+
+ return val[1];
+}
+
+static void *steal_time(void *arg)
+{
+ uint64_t nsecs_per_sec = 1000000000ul;
+ uint64_t sec, nsec;
+ struct timespec ts;
+
+ clock_gettime(CLOCK_MONOTONIC, &ts);
+ sec = ts.tv_sec;
+ nsec = ts.tv_nsec + MIN_STOLEN_TIME;
+ if (nsec > nsecs_per_sec) {
+ sec += 1;
+ nsec -= nsecs_per_sec;
+ }
+
+ while (1) {
+ clock_gettime(CLOCK_MONOTONIC, &ts);
+ if (ts.tv_sec > sec || (ts.tv_sec == sec && ts.tv_nsec >= nsec))
+ break;
+ }
+
+ return NULL;
+}
+
+static void run_vcpu(struct kvm_vm *vm, uint32_t vcpuid)
+{
+ struct ucall uc;
+
+ vcpu_ioctl(vm, vcpuid, KVM_RUN, NULL);
+
+ switch (get_ucall(vm, vcpuid, &uc)) {
+ case UCALL_SYNC:
+ case UCALL_DONE:
+ break;
+ case UCALL_ABORT:
+ TEST_ASSERT(false, "%s at %s:%d", (const char *)uc.args[0],
+ __FILE__, uc.args[1]);
+ default:
+ TEST_ASSERT(false, "Unexpected exit: %s",
+ exit_reason_str(vcpu_state(vm, vcpuid)->exit_reason));
+ }
+}
+
+int main(int ac, char **av)
+{
+ struct kvm_device_attr dev = {
+ .group = KVM_ARM_VCPU_PVTIME_CTRL,
+ .attr = KVM_ARM_VCPU_PVTIME_SET_IPA,
+ };
+ uint64_t pvtime_memslot_size;
+ struct kvm_vm *vm;
+ pthread_attr_t attr;
+ pthread_t thread;
+ cpu_set_t cpuset;
+ long stolen_time;
+ int i;
+
+ CPU_ZERO(&cpuset);
+ CPU_SET(0, &cpuset);
+ pthread_attr_init(&attr);
+ pthread_attr_setaffinity_np(&attr, sizeof(cpu_set_t), &cpuset);
+ pthread_setaffinity_np(pthread_self(), sizeof(cpu_set_t), &cpuset);
+
+ pvtime_memslot_size = 64 * 1024; /* one maximum-sized host page */
+
+ /* create a one-vcpu guest and the pvtime memslot */
+ vm = vm_create_default(0, 0, guest_code);
+ vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, ST_IPA_BASE, 1,
+ 16 /* vm uses 4k pages */, 0);
+ virt_map(vm, ST_IPA_BASE, ST_IPA_BASE, pvtime_memslot_size, 0);
+ ucall_init(vm, UCALL_MMIO, NULL);
+
+ /* add 3 more vcpus */
+ for (i = 1; i < 4; ++i) {
+ vm_vcpu_add_default(vm, i, guest_code);
+ st_ipa_offset[i] = i * 64;
+ sync_global_to_guest(vm, st_ipa_offset[i]);
+ }
+
+ /* add pvtime to each vcpu */
+ for (i = 0; i < 4; ++i) {
+ uint64_t st_ipa = ST_IPA_BASE + st_ipa_offset[i];
+ dev.addr = (uint64_t)&st_ipa;
+ vcpu_ioctl(vm, i, KVM_HAS_DEVICE_ATTR, &dev);
+ vcpu_ioctl(vm, i, KVM_SET_DEVICE_ATTR, &dev);
+ }
+
+ /* run the tests on each vcpu */
+ for (i = 0; i < 4; ++i) {
+ /* first vcpu run */
+ run_vcpu(vm, i);
+ sync_global_from_guest(vm, guest_stolen_time[i]);
+ TEST_ASSERT(guest_stolen_time[i] == 0, "Expected stolen_time = 0");
+
+ /* steal time from the vcpu */
+ stolen_time = get_run_delay();
+ pthread_create(&thread, &attr, steal_time, NULL);
+ pthread_yield();
+ pthread_join(thread, NULL);
+ stolen_time = get_run_delay() - stolen_time;
+ TEST_ASSERT(stolen_time >= MIN_STOLEN_TIME,
+ "Expected stolen time >= %ld, got %ld",
+ MIN_STOLEN_TIME, stolen_time);
+
+ /* run vcpu again and check the stolen time */
+ run_vcpu(vm, i);
+ sync_global_from_guest(vm, guest_stolen_time[i]);
+ TEST_ASSERT(guest_stolen_time[i] >= stolen_time,
+ "Expected stolen_time >= %ld, got %ld",
+ stolen_time, guest_stolen_time[i]);
+
+ printf("CPU%d: %ld", i, guest_stolen_time[i]);
+ if (stolen_time == guest_stolen_time[i])
+ printf(" (BONUS: guest stolen_time even exactly matches run_delay)");
+ printf("\n");
+ }
+
+ return 0;
+}
--
2.18.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
* [PATCH 1/1] mm/pgtable/debug: Add test validating architecture page table helpers
From: Anshuman Khandual @ 2019-09-03 8:01 UTC (permalink / raw)
To: linux-mm
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, Dave Hansen,
Paul Mackerras, sparclinux, Thomas Gleixner, linux-s390,
Michael Ellerman, x86, Russell King - ARM Linux, Matthew Wilcox,
Steven Price, Jason Gunthorpe, linux-arm-kernel, linux-snps-arc,
Kees Cook, Anshuman Khandual, Masahiro Yamada, Mark Brown,
Dan Williams, Vlastimil Babka, Sri Krishna chowdary,
Ard Biesheuvel, Greg Kroah-Hartman, linux-mips, Ralf Baechle,
linux-kernel, Paul Burton, Mike Rapoport, Vineet Gupta,
Martin Schwidefsky, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <1567497706-8649-1-git-send-email-anshuman.khandual@arm.com>
This adds a test module which will validate architecture page table helpers
and accessors regarding compliance with generic MM semantics expectations.
This will help various architectures in validating changes to the existing
page table helpers or addition of new ones.
Test page table and memory pages creating it's entries at various level are
all allocated from system memory with required alignments. If memory pages
with required size and alignment could not be allocated, then all depending
individual tests are skipped.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steven Price <Steven.Price@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Sri Krishna chowdary <schowdary@nvidia.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: James Hogan <jhogan@kernel.org>
Cc: Paul Burton <paul.burton@mips.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-mips@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ia64@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org
Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
mm/Kconfig.debug | 14 ++
mm/Makefile | 1 +
mm/arch_pgtable_test.c | 425 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 440 insertions(+)
create mode 100644 mm/arch_pgtable_test.c
diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
index 327b3ebf23bf..ce9c397f7b07 100644
--- a/mm/Kconfig.debug
+++ b/mm/Kconfig.debug
@@ -117,3 +117,17 @@ config DEBUG_RODATA_TEST
depends on STRICT_KERNEL_RWX
---help---
This option enables a testcase for the setting rodata read-only.
+
+config DEBUG_ARCH_PGTABLE_TEST
+ bool "Test arch page table helpers for semantics compliance"
+ depends on MMU
+ depends on DEBUG_KERNEL
+ help
+ This options provides a kernel module which can be used to test
+ architecture page table helper functions on various platform in
+ verifying if they comply with expected generic MM semantics. This
+ will help architectures code in making sure that any changes or
+ new additions of these helpers will still conform to generic MM
+ expected semantics.
+
+ If unsure, say N.
diff --git a/mm/Makefile b/mm/Makefile
index d996846697ef..bb572c5aa8c5 100644
--- a/mm/Makefile
+++ b/mm/Makefile
@@ -86,6 +86,7 @@ obj-$(CONFIG_HWPOISON_INJECT) += hwpoison-inject.o
obj-$(CONFIG_DEBUG_KMEMLEAK) += kmemleak.o
obj-$(CONFIG_DEBUG_KMEMLEAK_TEST) += kmemleak-test.o
obj-$(CONFIG_DEBUG_RODATA_TEST) += rodata_test.o
+obj-$(CONFIG_DEBUG_ARCH_PGTABLE_TEST) += arch_pgtable_test.o
obj-$(CONFIG_PAGE_OWNER) += page_owner.o
obj-$(CONFIG_CLEANCACHE) += cleancache.o
obj-$(CONFIG_MEMORY_ISOLATION) += page_isolation.o
diff --git a/mm/arch_pgtable_test.c b/mm/arch_pgtable_test.c
new file mode 100644
index 000000000000..f15be8a73723
--- /dev/null
+++ b/mm/arch_pgtable_test.c
@@ -0,0 +1,425 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * This kernel module validates architecture page table helpers &
+ * accessors and helps in verifying their continued compliance with
+ * generic MM semantics.
+ *
+ * Copyright (C) 2019 ARM Ltd.
+ *
+ * Author: Anshuman Khandual <anshuman.khandual@arm.com>
+ */
+#define pr_fmt(fmt) "arch_pgtable_test: %s " fmt, __func__
+
+#include <linux/kernel.h>
+#include <linux/hugetlb.h>
+#include <linux/mm.h>
+#include <linux/mman.h>
+#include <linux/mm_types.h>
+#include <linux/module.h>
+#include <linux/printk.h>
+#include <linux/swap.h>
+#include <linux/swapops.h>
+#include <linux/pfn_t.h>
+#include <linux/gfp.h>
+#include <linux/spinlock.h>
+#include <linux/sched/mm.h>
+#include <asm/pgalloc.h>
+#include <asm/pgtable.h>
+
+/*
+ * Basic operations
+ *
+ * mkold(entry) = An old and not a young entry
+ * mkyoung(entry) = A young and not an old entry
+ * mkdirty(entry) = A dirty and not a clean entry
+ * mkclean(entry) = A clean and not a dirty entry
+ * mkwrite(entry) = A write and not a write protected entry
+ * wrprotect(entry) = A write protected and not a write entry
+ * pxx_bad(entry) = A mapped and non-table entry
+ * pxx_same(entry1, entry2) = Both entries hold the exact same value
+ */
+#define VADDR_TEST (PGDIR_SIZE + PUD_SIZE + PMD_SIZE + PAGE_SIZE)
+#define VMA_TEST_FLAGS (VM_READ|VM_WRITE|VM_EXEC)
+#define RANDOM_NZVALUE (0xbe)
+
+static bool pud_aligned;
+static bool pmd_aligned;
+
+extern struct mm_struct *mm_alloc(void);
+
+static void pte_basic_tests(struct page *page, pgprot_t prot)
+{
+ pte_t pte = mk_pte(page, prot);
+
+ WARN_ON(!pte_same(pte, pte));
+ WARN_ON(!pte_young(pte_mkyoung(pte)));
+ WARN_ON(!pte_dirty(pte_mkdirty(pte)));
+ WARN_ON(!pte_write(pte_mkwrite(pte)));
+ WARN_ON(pte_young(pte_mkold(pte)));
+ WARN_ON(pte_dirty(pte_mkclean(pte)));
+ WARN_ON(pte_write(pte_wrprotect(pte)));
+}
+
+#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE
+static void pmd_basic_tests(struct page *page, pgprot_t prot)
+{
+ pmd_t pmd;
+
+ /*
+ * Memory block here must be PMD_SIZE aligned. Abort this
+ * test in case we could not allocate such a memory block.
+ */
+ if (!pmd_aligned) {
+ pr_warn("Could not proceed with PMD tests\n");
+ return;
+ }
+
+ pmd = mk_pmd(page, prot);
+ WARN_ON(!pmd_same(pmd, pmd));
+ WARN_ON(!pmd_young(pmd_mkyoung(pmd)));
+ WARN_ON(!pmd_dirty(pmd_mkdirty(pmd)));
+ WARN_ON(!pmd_write(pmd_mkwrite(pmd)));
+ WARN_ON(pmd_young(pmd_mkold(pmd)));
+ WARN_ON(pmd_dirty(pmd_mkclean(pmd)));
+ WARN_ON(pmd_write(pmd_wrprotect(pmd)));
+ /*
+ * A huge page does not point to next level page table
+ * entry. Hence this must qualify as pmd_bad().
+ */
+ WARN_ON(!pmd_bad(pmd_mkhuge(pmd)));
+}
+#else
+static void pmd_basic_tests(struct page *page, pgprot_t prot) { }
+#endif
+
+#ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
+static void pud_basic_tests(struct page *page, pgprot_t prot)
+{
+ pud_t pud;
+
+ /*
+ * Memory block here must be PUD_SIZE aligned. Abort this
+ * test in case we could not allocate such a memory block.
+ */
+ if (!pud_aligned) {
+ pr_warn("Could not proceed with PUD tests\n");
+ return;
+ }
+
+ pud = pfn_pud(page_to_pfn(page), prot);
+ WARN_ON(!pud_same(pud, pud));
+ WARN_ON(!pud_young(pud_mkyoung(pud)));
+ WARN_ON(!pud_write(pud_mkwrite(pud)));
+ WARN_ON(pud_write(pud_wrprotect(pud)));
+ WARN_ON(pud_young(pud_mkold(pud)));
+
+#if !defined(__PAGETABLE_PMD_FOLDED) && !defined(__ARCH_HAS_4LEVEL_HACK)
+ /*
+ * A huge page does not point to next level page table
+ * entry. Hence this must qualify as pud_bad().
+ */
+ WARN_ON(!pud_bad(pud_mkhuge(pud)));
+#endif
+}
+#else
+static void pud_basic_tests(struct page *page, pgprot_t prot) { }
+#endif
+
+static void p4d_basic_tests(struct page *page, pgprot_t prot)
+{
+ p4d_t p4d;
+
+ memset(&p4d, RANDOM_NZVALUE, sizeof(p4d_t));
+ WARN_ON(!p4d_same(p4d, p4d));
+}
+
+static void pgd_basic_tests(struct page *page, pgprot_t prot)
+{
+ pgd_t pgd;
+
+ memset(&pgd, RANDOM_NZVALUE, sizeof(pgd_t));
+ WARN_ON(!pgd_same(pgd, pgd));
+}
+
+#if !defined(__PAGETABLE_PMD_FOLDED) && !defined(__ARCH_HAS_4LEVEL_HACK)
+static void pud_clear_tests(pud_t *pudp)
+{
+ memset(pudp, RANDOM_NZVALUE, sizeof(pud_t));
+ pud_clear(pudp);
+ WARN_ON(!pud_none(READ_ONCE(*pudp)));
+}
+
+static void pud_populate_tests(struct mm_struct *mm, pud_t *pudp, pmd_t *pmdp)
+{
+ /*
+ * This entry points to next level page table page.
+ * Hence this must not qualify as pud_bad().
+ */
+ pmd_clear(pmdp);
+ pud_clear(pudp);
+ pud_populate(mm, pudp, pmdp);
+ WARN_ON(pud_bad(READ_ONCE(*pudp)));
+}
+#else
+static void pud_clear_tests(pud_t *pudp) { }
+static void pud_populate_tests(struct mm_struct *mm, pud_t *pudp, pmd_t *pmdp)
+{
+}
+#endif
+
+#if !defined(__PAGETABLE_PUD_FOLDED) && !defined(__ARCH_HAS_5LEVEL_HACK)
+static void p4d_clear_tests(p4d_t *p4dp)
+{
+ memset(p4dp, RANDOM_NZVALUE, sizeof(p4d_t));
+ p4d_clear(p4dp);
+ WARN_ON(!p4d_none(READ_ONCE(*p4dp)));
+}
+
+static void p4d_populate_tests(struct mm_struct *mm, p4d_t *p4dp, pud_t *pudp)
+{
+ /*
+ * This entry points to next level page table page.
+ * Hence this must not qualify as p4d_bad().
+ */
+ pud_clear(pudp);
+ p4d_clear(p4dp);
+ p4d_populate(mm, p4dp, pudp);
+ WARN_ON(p4d_bad(READ_ONCE(*p4dp)));
+}
+#else
+static void p4d_clear_tests(p4d_t *p4dp) { }
+static void p4d_populate_tests(struct mm_struct *mm, p4d_t *p4dp, pud_t *pudp)
+{
+}
+#endif
+
+#ifndef __PAGETABLE_P4D_FOLDED
+static void pgd_clear_tests(pgd_t *pgdp)
+{
+ memset(pgdp, RANDOM_NZVALUE, sizeof(pgd_t));
+ pgd_clear(pgdp);
+ WARN_ON(!pgd_none(READ_ONCE(*pgdp)));
+}
+
+static void pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp, p4d_t *p4dp)
+{
+ /*
+ * This entry points to next level page table page.
+ * Hence this must not qualify as pgd_bad().
+ */
+ p4d_clear(p4dp);
+ pgd_clear(pgdp);
+ pgd_populate(mm, pgdp, p4dp);
+ WARN_ON(pgd_bad(READ_ONCE(*pgdp)));
+}
+#else
+static void pgd_clear_tests(pgd_t *pgdp) { }
+static void pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp, p4d_t *p4dp)
+{
+}
+#endif
+
+static void pte_clear_tests(pte_t *ptep)
+{
+ memset(ptep, RANDOM_NZVALUE, sizeof(pte_t));
+ pte_clear(NULL, 0, ptep);
+ WARN_ON(!pte_none(READ_ONCE(*ptep)));
+}
+
+static void pmd_clear_tests(pmd_t *pmdp)
+{
+ memset(pmdp, RANDOM_NZVALUE, sizeof(pmd_t));
+ pmd_clear(pmdp);
+ WARN_ON(!pmd_none(READ_ONCE(*pmdp)));
+}
+
+static void pmd_populate_tests(struct mm_struct *mm, pmd_t *pmdp,
+ pgtable_t pgtable)
+{
+ /*
+ * This entry points to next level page table page.
+ * Hence this must not qualify as pmd_bad().
+ */
+ pmd_clear(pmdp);
+ pmd_populate(mm, pmdp, pgtable);
+ WARN_ON(pmd_bad(READ_ONCE(*pmdp)));
+}
+
+static bool pfn_range_valid(struct zone *z, unsigned long start_pfn,
+ unsigned long nr_pages)
+{
+ unsigned long i, end_pfn = start_pfn + nr_pages;
+ struct page *page;
+
+ for (i = start_pfn; i < end_pfn; i++) {
+ if (!pfn_valid(i))
+ return false;
+
+ page = pfn_to_page(i);
+
+ if (page_zone(page) != z)
+ return false;
+
+ if (PageReserved(page))
+ return false;
+
+ if (page_count(page) > 0)
+ return false;
+
+ if (PageHuge(page))
+ return false;
+ }
+ return true;
+}
+
+static struct page *alloc_gigantic_page(nodemask_t *nodemask,
+ int nid, gfp_t gfp_mask, int order)
+{
+ struct zonelist *zonelist;
+ struct zone *zone;
+ struct zoneref *z;
+ enum zone_type zonesel;
+ unsigned long ret, pfn, flags, nr_pages;
+
+ nr_pages = 1UL << order;
+ zonesel = gfp_zone(gfp_mask);
+ zonelist = node_zonelist(nid, gfp_mask);
+ for_each_zone_zonelist_nodemask(zone, z, zonelist, zonesel, nodemask) {
+ spin_lock_irqsave(&zone->lock, flags);
+ pfn = ALIGN(zone->zone_start_pfn, nr_pages);
+ while (zone_spans_pfn(zone, pfn + nr_pages - 1)) {
+ if (pfn_range_valid(zone, pfn, nr_pages)) {
+ spin_unlock_irqrestore(&zone->lock, flags);
+ ret = alloc_contig_range(pfn, pfn + nr_pages,
+ MIGRATE_MOVABLE,
+ gfp_mask);
+ if (!ret)
+ return pfn_to_page(pfn);
+ spin_lock_irqsave(&zone->lock, flags);
+ }
+ pfn += nr_pages;
+ }
+ spin_unlock_irqrestore(&zone->lock, flags);
+ }
+ return NULL;
+}
+
+static struct page *alloc_mapped_page(void)
+{
+ gfp_t gfp_mask = GFP_KERNEL | __GFP_ZERO;
+ struct page *page = NULL;
+
+ page = alloc_gigantic_page(&node_states[N_MEMORY], first_memory_node,
+ gfp_mask, get_order(PUD_SIZE));
+ if (page) {
+ pud_aligned = true;
+ pmd_aligned = true;
+ return page;
+ }
+
+ page = alloc_pages(gfp_mask, get_order(PMD_SIZE));
+ if (page) {
+ pmd_aligned = true;
+ return page;
+ }
+ return alloc_page(gfp_mask);
+}
+
+static void free_mapped_page(struct page *page)
+{
+ if (pud_aligned) {
+ unsigned long pfn = page_to_pfn(page);
+
+ free_contig_range(pfn, 1ULL << get_order(PUD_SIZE));
+ return;
+ }
+
+ if (pmd_aligned) {
+ int order = get_order(PMD_SIZE);
+
+ free_pages((unsigned long)page_address(page), order);
+ return;
+ }
+ free_page((unsigned long)page_address(page));
+}
+
+static int __init arch_pgtable_tests_init(void)
+{
+ struct mm_struct *mm;
+ struct page *page;
+ pgd_t *pgdp;
+ p4d_t *p4dp, *saved_p4dp;
+ pud_t *pudp, *saved_pudp;
+ pmd_t *pmdp, *saved_pmdp;
+ pte_t *ptep, *saved_ptep;
+ pgprot_t prot = vm_get_page_prot(VMA_TEST_FLAGS);
+ unsigned long vaddr = VADDR_TEST;
+
+ mm = mm_alloc();
+ if (!mm) {
+ pr_err("mm_struct allocation failed\n");
+ return 1;
+ }
+
+ page = alloc_mapped_page();
+ if (!page) {
+ pr_err("memory allocation failed\n");
+ return 1;
+ }
+
+ pgdp = pgd_offset(mm, vaddr);
+ p4dp = p4d_alloc(mm, pgdp, vaddr);
+ pudp = pud_alloc(mm, p4dp, vaddr);
+ pmdp = pmd_alloc(mm, pudp, vaddr);
+ ptep = pte_alloc_map(mm, pmdp, vaddr);
+
+ /*
+ * Save all the page table page addresses as the page table
+ * entries will be used for testing with random or garbage
+ * values. These saved addresses will be used for freeing
+ * page table pages.
+ */
+ saved_p4dp = p4d_offset(pgdp, 0UL);
+ saved_pudp = pud_offset(p4dp, 0UL);
+ saved_pmdp = pmd_offset(pudp, 0UL);
+ saved_ptep = pte_offset_map(pmdp, 0UL);
+
+ pte_basic_tests(page, prot);
+ pmd_basic_tests(page, prot);
+ pud_basic_tests(page, prot);
+ p4d_basic_tests(page, prot);
+ pgd_basic_tests(page, prot);
+
+ pte_clear_tests(ptep);
+ pmd_clear_tests(pmdp);
+ pud_clear_tests(pudp);
+ p4d_clear_tests(p4dp);
+ pgd_clear_tests(pgdp);
+
+ pmd_populate_tests(mm, pmdp, (pgtable_t) page);
+ pud_populate_tests(mm, pudp, pmdp);
+ p4d_populate_tests(mm, p4dp, pudp);
+ pgd_populate_tests(mm, pgdp, p4dp);
+
+ p4d_free(mm, saved_p4dp);
+ pud_free(mm, saved_pudp);
+ pmd_free(mm, saved_pmdp);
+ pte_free(mm, (pgtable_t) virt_to_page(saved_ptep));
+
+ mm_dec_nr_puds(mm);
+ mm_dec_nr_pmds(mm);
+ mm_dec_nr_ptes(mm);
+ __mmdrop(mm);
+
+ free_mapped_page(page);
+ return 0;
+}
+
+static void __exit arch_pgtable_tests_exit(void) { }
+
+module_init(arch_pgtable_tests_init);
+module_exit(arch_pgtable_tests_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Anshuman Khandual <anshuman.khandual@arm.com>");
+MODULE_DESCRIPTION("Test archicture page table helpers");
--
2.20.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
* [PATCH 0/1] mm/debug: Add tests for architecture exported page table helpers
From: Anshuman Khandual @ 2019-09-03 8:01 UTC (permalink / raw)
To: linux-mm
Cc: Mark Rutland, linux-ia64, linux-sh, Peter Zijlstra, James Hogan,
Tetsuo Handa, Heiko Carstens, Michal Hocko, Dave Hansen,
Paul Mackerras, sparclinux, Thomas Gleixner, linux-s390,
Michael Ellerman, x86, Russell King - ARM Linux, Matthew Wilcox,
Steven Price, Jason Gunthorpe, linux-arm-kernel, linux-snps-arc,
Kees Cook, Anshuman Khandual, Masahiro Yamada, Mark Brown,
Dan Williams, Vlastimil Babka, Sri Krishna chowdary,
Ard Biesheuvel, Greg Kroah-Hartman, linux-mips, Ralf Baechle,
linux-kernel, Paul Burton, Mike Rapoport, Vineet Gupta,
Martin Schwidefsky, Andrew Morton, linuxppc-dev, David S. Miller
This series adds a test validation for architecture exported page table
helpers. Patch in the series adds basic transformation tests at various
levels of the page table.
This test was originally suggested by Catalin during arm64 THP migration
RFC discussion earlier. Going forward it can include more specific tests
with respect to various generic MM functions like THP, HugeTLB etc and
platform specific tests.
https://lore.kernel.org/linux-mm/20190628102003.GA56463@arrakis.emea.arm.com/
Questions:
Should alloc_gigantic_page() be made available as an interface for general
use in the kernel. The test module here uses very similar implementation from
HugeTLB to allocate a PUD aligned memory block. Similar for mm_alloc() which
needs to be exported through a header.
Matthew Wilcox had expressed concerns regarding memory allocation for mapped
page table entries at various level. He also suggested using synethetic pfns
which can be derived from virtual address of a known kernel text symbol like
kernel_init(). But as discussed previously, it seems like allocated memory
might still outweigh synthetic pfns. This proposal goes with allocated memory
but if there is a broader agreement with respect to synthetic pfns, will be
happy to rework the test.
Testing:
Build and boot tested on arm64 and x86 platforms. While arm64 clears all
these tests, following errors were reported on x86.
1. WARN_ON(pud_bad(pud)) in pud_populate_tests()
2. WARN_ON(p4d_bad(p4d)) in p4d_populate_tests()
I would really appreciate if folks can help validate this test on other
platforms and report back problems if any. Suggestions, comments and
inputs welcome. Thank you.
Changes in V3:
- Added fallback mechanism for PMD aligned memory allocation failure
Changes in V2:
https://lore.kernel.org/linux-mm/1565335998-22553-1-git-send-email-anshuman.khandual@arm.com/T/#u
- Moved test module and it's config from lib/ to mm/
- Renamed config TEST_ARCH_PGTABLE as DEBUG_ARCH_PGTABLE_TEST
- Renamed file from test_arch_pgtable.c to arch_pgtable_test.c
- Added relevant MODULE_DESCRIPTION() and MODULE_AUTHOR() details
- Dropped loadable module config option
- Basic tests now use memory blocks with required size and alignment
- PUD aligned memory block gets allocated with alloc_contig_range()
- If PUD aligned memory could not be allocated it falls back on PMD aligned
memory block from page allocator and pud_* tests are skipped
- Clear and populate tests now operate on real in memory page table entries
- Dummy mm_struct gets allocated with mm_alloc()
- Dummy page table entries get allocated with [pud|pmd|pte]_alloc_[map]()
- Simplified [p4d|pgd]_basic_tests(), now has random values in the entries
RFC V1:
https://lore.kernel.org/linux-mm/1564037723-26676-1-git-send-email-anshuman.khandual@arm.com/
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mark Brown <broonie@kernel.org>
Cc: Steven Price <Steven.Price@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: Sri Krishna chowdary <schowdary@nvidia.com>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: James Hogan <jhogan@kernel.org>
Cc: Paul Burton <paul.burton@mips.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-mips@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-ia64@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-s390@vger.kernel.org
Cc: linux-sh@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org
Anshuman Khandual (1):
mm/pgtable/debug: Add test validating architecture page table helpers
mm/Kconfig.debug | 14 ++
mm/Makefile | 1 +
mm/arch_pgtable_test.c | 425 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 440 insertions(+)
create mode 100644 mm/arch_pgtable_test.c
--
2.20.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 4/4] arm64: dts: add support for A1 based Amlogic AD401
From: Jianxin Pan @ 2019-09-03 8:00 UTC (permalink / raw)
To: Jerome Brunet, Kevin Hilman, linux-amlogic
Cc: devicetree, Hanjie Lin, Victor Wan, Neil Armstrong,
Martin Blumenstingl, linux-kernel, Qiufang Dai, Rob Herring,
Jian Hu, Xingyu Chen, Carlo Caione, Tao Zeng, linux-arm-kernel
In-Reply-To: <1jef0xrg5d.fsf@starbuckisacylon.baylibre.com>
Hi Jerome,
Thanks for your suggestion.
I will fix them in the next version.
On 2019/9/3 15:30, Jerome Brunet wrote:
> On Tue 03 Sep 2019 at 02:51, Jianxin Pan <jianxin.pan@amlogic.com> wrote:
>
>> Add basic support for the Amlogic A1 based Amlogic AD401 board:
>> which describe components as follows: Reserve Memory, CPU, GIC, IRQ,
>> Timer, UART. It's capable of booting up into the serial console.
>>
>> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
>> ---
>> arch/arm64/boot/dts/amlogic/Makefile | 1 +
>> arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts | 30 ++++++
>> arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 121 +++++++++++++++++++++++++
>> 3 files changed, 152 insertions(+)
>> create mode 100644 arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts
>> create mode 100644 arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
>> index edbf128..1720c45 100644
>> --- a/arch/arm64/boot/dts/amlogic/Makefile
>> +++ b/arch/arm64/boot/dts/amlogic/Makefile
>> @@ -36,3 +36,4 @@ dtb-$(CONFIG_ARCH_MESON) += meson-gxm-rbox-pro.dtb
>> dtb-$(CONFIG_ARCH_MESON) += meson-gxm-vega-s96.dtb
>> dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
>> dtb-$(CONFIG_ARCH_MESON) += meson-sm1-khadas-vim3l.dtb
>> +dtb-$(CONFIG_ARCH_MESON) += meson-a1-ad401.dtb
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts b/arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts
>> new file mode 100644
>> index 00000000..3c05cc0
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts
>> @@ -0,0 +1,30 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include "meson-a1.dtsi"
>> +
>> +/ {
>> + compatible = "amlogic,ad401", "amlogic,a1";
>> + model = "Amlogic Meson A1 AD401 Development Board";
>> +
>> + aliases {
>> + serial0 = &uart_AO_B;
>> + };
>
> Newline here please
>
>> + chosen {
>> + stdout-path = "serial0:115200n8";
>> + };
>
> same
>
>> + memory@0 {
>> + device_type = "memory";
>> + linux,usable-memory = <0x0 0x0 0x0 0x8000000>;
>> + };
>> +};
>> +
>> +&uart_AO_B {
>> + status = "okay";
>> + /*pinctrl-0 = <&uart_ao_a_pins>;*/
>> + /*pinctrl-names = "default";*/
>
> Remove the commented code please
>
>> +};
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>> new file mode 100644
>> index 00000000..b98d648
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>> @@ -0,0 +1,121 @@
>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>> +/*
>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>> + */
>> +
>> +#include <dt-bindings/interrupt-controller/irq.h>
>> +#include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> +/ {
>> + compatible = "amlogic,a1";
>> +
>> + interrupt-parent = <&gic>;
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + cpus {
>> + #address-cells = <0x2>;
>> + #size-cells = <0x0>;
>> +
>> + cpu0: cpu@0 {
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a35";
>> + reg = <0x0 0x0>;
>> + enable-method = "psci";
>> + next-level-cache = <&l2>;
>> + };
>> +
>> + cpu1: cpu@1 {
>> + device_type = "cpu";
>> + compatible = "arm,cortex-a35";
>> + reg = <0x0 0x1>;
>> + enable-method = "psci";
>> + next-level-cache = <&l2>;
>> + };
>> +
>> + l2: l2-cache0 {
>> + compatible = "cache";
>> + };
>> + };
>
> New line here please
>
> With this minor comments adressed, looks good.
>
> Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
>
>> + psci {
>> + compatible = "arm,psci-1.0";
>> + method = "smc";
>> + };
>> +
>> + reserved-memory {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + ranges;
>> +
>> + linux,cma {
>> + compatible = "shared-dma-pool";
>> + reusable;
>> + size = <0x0 0x800000>;
>> + alignment = <0x0 0x400000>;
>> + linux,cma-default;
>> + };
>> + };
>> +
>> + sm: secure-monitor {
>> + compatible = "amlogic,meson-gxbb-sm";
>> + };
>> +
>> + soc {
>> + compatible = "simple-bus";
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + ranges;
>> +
>> + uart_AO: serial@fe001c00 {
>> + compatible = "amlogic,meson-gx-uart",
>> + "amlogic,meson-ao-uart";
>> + reg = <0x0 0xfe001c00 0x0 0x18>;
>> + interrupts = <GIC_SPI 25 IRQ_TYPE_EDGE_RISING>;
>> + clocks = <&xtal>, <&xtal>, <&xtal>;
>> + clock-names = "xtal", "pclk", "baud";
>> + status = "disabled";
>> + };
>> +
>> + uart_AO_B: serial@fe002000 {
>> + compatible = "amlogic,meson-gx-uart",
>> + "amlogic,meson-ao-uart";
>> + reg = <0x0 0xfe002000 0x0 0x18>;
>> + interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
>> + clocks = <&xtal>, <&xtal>, <&xtal>;
>> + clock-names = "xtal", "pclk", "baud";
>> + status = "disabled";
>> + };
>> +
>> + gic: interrupt-controller@ff901000 {
>> + compatible = "arm,gic-400";
>> + reg = <0x0 0xff901000 0x0 0x1000>,
>> + <0x0 0xff902000 0x0 0x2000>,
>> + <0x0 0xff904000 0x0 0x2000>,
>> + <0x0 0xff906000 0x0 0x2000>;
>> + interrupt-controller;
>> + interrupts = <GIC_PPI 9
>> + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
>> + #interrupt-cells = <3>;
>> + #address-cells = <0>;
>> + };
>> + };
>> +
>> + timer {
>> + compatible = "arm,armv8-timer";
>> + interrupts = <GIC_PPI 13
>> + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>,
>> + <GIC_PPI 14
>> + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>,
>> + <GIC_PPI 11
>> + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>,
>> + <GIC_PPI 10
>> + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>;
>> + };
>> +
>> + xtal: xtal-clk {
>> + compatible = "fixed-clock";
>> + clock-frequency = <24000000>;
>> + clock-output-names = "xtal";
>> + #clock-cells = <0>;
>> + };
>> +};
>> --
>> 2.7.4
>
> .
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 5/9] crypto: ccree - Rename arrays to avoid conflict with crypto/sha256.h
From: Gilad Ben-Yossef @ 2019-09-03 7:59 UTC (permalink / raw)
To: Hans de Goede
Cc: linux-efi, Catalin Marinas, Heiko Carstens, H . Peter Anvin,
Will Deacon, Atul Gupta, linux-s390, Herbert Xu, x86,
Russell King, Eric Biggers, Christian Borntraeger, Ingo Molnar,
Vasily Gorbik, Marc Zyngier, Borislav Petkov, Andy Lutomirski,
Thomas Gleixner, Linux ARM, Ard Biesheuvel,
Linux kernel mailing list, Linux Crypto Mailing List,
David S . Miller
In-Reply-To: <0d55a6a7-9cca-38cb-97a2-558280fdc122@redhat.com>
On Tue, Sep 3, 2019 at 10:51 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 03-09-19 09:45, Gilad Ben-Yossef wrote:
> > On Sun, Sep 1, 2019 at 11:36 PM Hans de Goede <hdegoede@redhat.com> wrote:
> >>
> >> Rename the algo_init arrays to cc_algo_init so that they do not conflict
> >> with the functions declared in crypto/sha256.h.
> >>
> >> This is a preparation patch for folding crypto/sha256.h into crypto/sha.h.
> >
> > I'm fine with the renaming.
> >
> > Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>
> Your Signed-off-by is only used when the patches passes through your hands,
> since Herbert will likely apply this directly that is not the case.
>
> You want either Acked-by or Reviewed-by to signal that you are ok with this patch.
>
Yes, you are right of course. Wrong macro... sorry about that.
Acked-by: Gilad Ben-Yossef <gilad@benyossef.com>
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC 5/9] dt-bindings: arm: samsung: Convert Exynos PMU bindings to json-schema
From: Krzysztof Kozlowski @ 2019-09-03 7:58 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, Alessandro Zummo, Alexandre Belloni,
Lars-Peter Clausen, Arnd Bergmann, devicetree,
open list:IIO SUBSYSTEM AND DRIVERS, Marek Szyprowski,
linux-kernel@vger.kernel.org, Tomasz Figa, linux-samsung-soc,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Peter Meerwald-Stadler, Hartmut Knaack, Olof Johansson,
open list:REAL TIME CLOCK (RTC) SUBSYSTEM, notify,
Jonathan Cameron, Paweł Chmiel
In-Reply-To: <CAL_JsqJybT41cEqiTriLMywUQj1BtAG_9muJ4=84OkF23y53CA@mail.gmail.com>
On Mon, 26 Aug 2019 at 13:54, Rob Herring <robh+dt@kernel.org> wrote:
>
> On Fri, Aug 23, 2019 at 9:54 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > Convert Samsung Exynos Power Management Unit (PMU) bindings to DT schema
> > format using json-schema.
> >
> > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > ---
> > .../devicetree/bindings/arm/samsung/pmu.txt | 72 --------------
> > .../devicetree/bindings/arm/samsung/pmu.yaml | 93 +++++++++++++++++++
> > 2 files changed, 93 insertions(+), 72 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/arm/samsung/pmu.txt
> > create mode 100644 Documentation/devicetree/bindings/arm/samsung/pmu.yaml
>
>
> > diff --git a/Documentation/devicetree/bindings/arm/samsung/pmu.yaml b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > new file mode 100644
> > index 000000000000..818c6f3488ef
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/samsung/pmu.yaml
> > @@ -0,0 +1,93 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/arm/samsung/pmu.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Samsung Exynos SoC series Power Management Unit (PMU)
> > +
> > +maintainers:
> > + - Krzysztof Kozlowski <krzk@kernel.org>
> > +
> > +properties:
> > + compatible:
> > + items:
> > + - enum:
> > + - samsung,exynos3250-pmu
> > + - samsung,exynos4210-pmu
> > + - samsung,exynos4412-pmu
> > + - samsung,exynos5250-pmu
> > + - samsung,exynos5260-pmu
> > + - samsung,exynos5410-pmu
> > + - samsung,exynos5420-pmu
> > + - samsung,exynos5433-pmu
> > + - samsung,exynos7-pmu
> > + - const: syscon
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + '#clock-cells':
> > + const: 1
> > +
> > + clock-names:
> > + description:
> > + list of clock names for particular CLKOUT mux inputs
> > + # TODO: what is the maximum number of elements (mux inputs)?
> > + minItems: 1
> > + maxItems: 32
> > + items:
> > + - enum:
>
> This isn't correct as you are only defining possible names for the
> first item. Drop the '-' (making items a schema instead of a list) and
> then it applies to all. However, doing that will cause a meta-schema
> error which I need to fix to allow. Or if there's a small set of
> possibilities of number of inputs, you can list them under a 'oneOf'
> list.
Mhmm, I cannot test it or I have an error in the schema. if I
understand correctly, this would be:
clock-names:
description:
List of clock names for particular CLKOUT mux inputs
minItems: 1
maxItems: 16
items:
clkout0
clkout1
clkout2
clkout3
clkout4
clkout5
clkout6
clkout7
clkout8
clkout9
clkout10
clkout11
clkout12
clkout13
clkout14
clkout15
clkout16
Now it produces the error "ignoring, error in schema 'items'" but
maybe it is expected with current meta-schema?
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 5/9] crypto: ccree - Rename arrays to avoid conflict with crypto/sha256.h
From: Hans de Goede @ 2019-09-03 7:51 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: linux-efi, Catalin Marinas, Heiko Carstens, H . Peter Anvin,
Will Deacon, Atul Gupta, linux-s390, Herbert Xu, x86,
Russell King, Eric Biggers, Christian Borntraeger, Ingo Molnar,
Vasily Gorbik, Marc Zyngier, Borislav Petkov, Andy Lutomirski,
Thomas Gleixner, Linux ARM, Ard Biesheuvel,
Linux kernel mailing list, Linux Crypto Mailing List,
David S . Miller
In-Reply-To: <CAOtvUMdd+V5pesw+O-kk9_JB5YpxUM+hU+Uu=kiMvOL9d0AziQ@mail.gmail.com>
Hi,
On 03-09-19 09:45, Gilad Ben-Yossef wrote:
> On Sun, Sep 1, 2019 at 11:36 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Rename the algo_init arrays to cc_algo_init so that they do not conflict
>> with the functions declared in crypto/sha256.h.
>>
>> This is a preparation patch for folding crypto/sha256.h into crypto/sha.h.
>
> I'm fine with the renaming.
>
> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Your Signed-off-by is only used when the patches passes through your hands,
since Herbert will likely apply this directly that is not the case.
You want either Acked-by or Reviewed-by to signal that you are ok with this patch.
Regards,
Hans
_______________________________________________
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 5/9] crypto: ccree - Rename arrays to avoid conflict with crypto/sha256.h
From: Gilad Ben-Yossef @ 2019-09-03 7:45 UTC (permalink / raw)
To: Hans de Goede
Cc: linux-efi, Catalin Marinas, Heiko Carstens, H . Peter Anvin,
Will Deacon, Atul Gupta, linux-s390, Herbert Xu, x86,
Russell King, Eric Biggers, Christian Borntraeger, Ingo Molnar,
Vasily Gorbik, Marc Zyngier, Borislav Petkov, Andy Lutomirski,
Thomas Gleixner, Linux ARM, Ard Biesheuvel,
Linux kernel mailing list, Linux Crypto Mailing List,
David S . Miller
In-Reply-To: <20190901203532.2615-6-hdegoede@redhat.com>
On Sun, Sep 1, 2019 at 11:36 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Rename the algo_init arrays to cc_algo_init so that they do not conflict
> with the functions declared in crypto/sha256.h.
>
> This is a preparation patch for folding crypto/sha256.h into crypto/sha.h.
I'm fine with the renaming.
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Thanks,
Gilad
_______________________________________________
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/5] input: keyboard: imx_sc: Add i.MX system controller power key support
From: Oleksij Rempel @ 2019-09-03 7:43 UTC (permalink / raw)
To: Anson Huang, robh+dt@kernel.org, mark.rutland@arm.com,
shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
catalin.marinas@arm.com, will@kernel.org,
dmitry.torokhov@gmail.com, Aisheng Dong, ulf.hansson@linaro.org,
Andy Duan, Peng Fan, Daniel Baluta, Leonard Crestez,
mripard@kernel.org, olof@lixom.net, arnd@arndb.de,
jagan@amarulasolutions.com, bjorn.andersson@linaro.org,
dinguyen@kernel.org, marcin.juszkiewicz@linaro.org,
stefan@agner.ch, gregkh@linuxfoundation.org,
andriy.shevchenko@linux.intel.com, yuehaibing@huawei.com,
tglx@linutronix.de, ronald@innovation.ch, m.felsch@pengutronix.de,
Jacky Bai, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-input@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <DB3PR0402MB3916FB4618F86DD891013FEEF5B90@DB3PR0402MB3916.eurprd04.prod.outlook.com>
On 03.09.19 09:35, Anson Huang wrote:
> Hi, Oleksij
>
>> On 03.09.19 08:48, Anson Huang wrote:
>>> Hi, Oleksij
>>>
>>>> On 03.09.19 16:03, Anson Huang wrote:
>>>>> i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller
>>>>> inside, the system controller is in charge of controlling power,
>>>>> clock and power key etc..
>>>>>
>>>>> Adds i.MX system controller power key driver support, Linux kernel
>>>>> has to communicate with system controller via MU (message unit) IPC
>>>>> to get power key's status.
>>>>>
>>>>> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>>>>> ---
>>>>> Changes since V1:
>>>>> - remove "wakeup-source" property operation, scu power key uses
>>>> generic scu irq,
>>>>> no need to have this property for device wakeup operation.
>>>>> ---
>>>>> drivers/input/keyboard/Kconfig | 7 ++
>>>>> drivers/input/keyboard/Makefile | 1 +
>>>>> drivers/input/keyboard/imx_sc_pwrkey.c | 169
>>>> +++++++++++++++++++++++++++++++++
>>>>> 3 files changed, 177 insertions(+)
>>>>> create mode 100644 drivers/input/keyboard/imx_sc_pwrkey.c
>>>>>
>>>>> diff --git a/drivers/input/keyboard/Kconfig
>>>>> b/drivers/input/keyboard/Kconfig index 2e6d288..3aaeb9c 100644
>>>>> --- a/drivers/input/keyboard/Kconfig
>>>>> +++ b/drivers/input/keyboard/Kconfig
>>>>> @@ -469,6 +469,13 @@ config KEYBOARD_IMX
>>>>> To compile this driver as a module, choose M here: the
>>>>> module will be called imx_keypad.
>>>>>
>>>>> +config KEYBOARD_IMX_SC_PWRKEY
>>>>> + tristate "IMX SCU Power Key Driver"
>>>>> + depends on IMX_SCU
>>>>> + help
>>>>> + This is the system controller powerkey driver for NXP i.MX SoCs with
>>>>> + system controller inside.
>>>>
>>>> The KEY is configurable over devicetree, why is it called PWRKEY? It
>>>> looks for me as generic SCU key handler.
>>>
>>> We always use it as power key, NOT a generic key, as it has HW
>>> function designed for power key purpose.
>>
>> gpio-key driver is mostly used for power or reboot key. And it is still called
>> gpio-key driver. If it is used for power key only, why is it configurable? I can
>> configure it as KEY_ENTER or some thing different. This driver has not
>> KEY_POWER specific any thing.
>
> Understood, I am making the V3 with all "power" removed, just using the "key".
>
>>
>>>
>>>>
>>>>> config KEYBOARD_NEWTON
>>>>> tristate "Newton keyboard"
>>>>> select SERIO
>>>>> diff --git a/drivers/input/keyboard/Makefile
>>>>> b/drivers/input/keyboard/Makefile index 9510325..9ea5585 100644
>>>>> --- a/drivers/input/keyboard/Makefile
>>>>> +++ b/drivers/input/keyboard/Makefile
>>>>> @@ -29,6 +29,7 @@ obj-$(CONFIG_KEYBOARD_HIL) +=
>> hil_kbd.o
>>>>> obj-$(CONFIG_KEYBOARD_HIL_OLD) += hilkbd.o
>>>>> obj-$(CONFIG_KEYBOARD_IPAQ_MICRO) += ipaq-micro-keys.o
>>>>> obj-$(CONFIG_KEYBOARD_IMX) += imx_keypad.o
>>>>> +obj-$(CONFIG_KEYBOARD_IMX_SC_PWRKEY) += imx_sc_pwrkey.o
>>>>> obj-$(CONFIG_KEYBOARD_HP6XX) += jornada680_kbd.o
>>>>> obj-$(CONFIG_KEYBOARD_HP7XX) += jornada720_kbd.o
>>>>> obj-$(CONFIG_KEYBOARD_LKKBD) += lkkbd.o
>>>>> diff --git a/drivers/input/keyboard/imx_sc_pwrkey.c
>>>>> b/drivers/input/keyboard/imx_sc_pwrkey.c
>>>>> new file mode 100644
>>>>> index 0000000..53aa9a4
>>>>> --- /dev/null
>>>>> +++ b/drivers/input/keyboard/imx_sc_pwrkey.c
>>>>> @@ -0,0 +1,169 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>> +/*
>>>>> + * Copyright 2019 NXP.
>>>>> + */
>>>>> +
>>>>> +#include <linux/device.h>
>>>>> +#include <linux/err.h>
>>>>> +#include <linux/firmware/imx/sci.h> #include <linux/init.h>
>>>>> +#include <linux/input.h> #include <linux/interrupt.h> #include
>>>>> +<linux/jiffies.h> #include <linux/kernel.h> #include
>>>>> +<linux/module.h> #include <linux/of.h> #include
>>>>> +<linux/of_address.h> #include <linux/platform_device.h>
>>>>> +
>>>>> +#define DEBOUNCE_TIME 100
>>>>> +#define REPEAT_INTERVAL 60
>>>>> +
>>>>> +#define SC_IRQ_BUTTON 1
>>>>> +#define SC_IRQ_GROUP_WAKE 3
>>>>> +#define IMX_SC_MISC_FUNC_GET_BUTTON_STATUS 18
>>>>> +
>>>>> +struct imx_pwrkey_drv_data {
>>>>> + int keycode;
>>>>> + bool keystate; /* 1: pressed, 0: release */
>>>>> + bool delay_check;
>>>>> + struct delayed_work check_work;
>>>>> + struct input_dev *input;
>>>>> +};
>>>>> +
>>>>> +struct imx_sc_msg_pwrkey {
>>>>> + struct imx_sc_rpc_msg hdr;
>>>>> + u8 state;
>>>>> +};
>>>>> +static struct imx_pwrkey_drv_data *pdata;
>>>>
>>>> Why is it global struct? It seems to be flexible configurable over devicetree.
>>>> So I would assume it should be able to handle more then one button.
>>>> Please remove global variables, make it allocatable per OF node.
>>>
>>> There is ONLY one button available for SC key, but yes, I think I can
>>> make the structure private and get all necessary data from the structure
>> using container_of.
>>
>> And we will never need more then 640 kB RAM ;)
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.wi
>> kiquote.org%2Fwiki%2FTalk%3ABill_Gates&data=02%7C01%7Canson.hu
>> ang%40nxp.com%7C4d4d7458087747e0d8f008d7304057e9%7C686ea1d3bc2
>> b4c6fa92cd99c5c301635%7C0%7C0%7C637030925236150243&sdata=w
>> %2FGXBaHfnBdLwjTxjbzWSPeIw3ExL%2Fs9IMOgF1onL6A%3D&reserved
>> =0
>>
>>>
>>>>
>>>> Please use different name "pdata" is usually used as platform data.
>>>> Please, use "priv".
>>>
>>> OK.
>>>
>>>>
>>>>> +static struct imx_sc_ipc *pwrkey_ipc_handle;
>>>>
>>>> same as before, no global variables.
>>>
>>> Will move it into private platform data structure.
>>>
>>>>
>>>>> +
>>>>> +static int imx_sc_pwrkey_notify(struct notifier_block *nb,
>>>>> + unsigned long event, void *group) {
>>>>> + if ((event & SC_IRQ_BUTTON) && (*(u8 *)group ==
>>>> SC_IRQ_GROUP_WAKE)
>>>>> + && !pdata->delay_check) {
>>>>> + pdata->delay_check = 1;
>>>>> + schedule_delayed_work(&pdata->check_work,
>>>>> + msecs_to_jiffies(REPEAT_INTERVAL));
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static void imx_sc_check_for_events(struct work_struct *work) {
>>>>> + struct input_dev *input = pdata->input;
>>>>> + struct imx_sc_msg_pwrkey msg;
>>>>> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
>>>>> + bool state;
>>>>> +
>>>>> + hdr->ver = IMX_SC_RPC_VERSION;
>>>>> + hdr->svc = IMX_SC_RPC_SVC_MISC;
>>>>> + hdr->func = IMX_SC_MISC_FUNC_GET_BUTTON_STATUS;
>>>>> + hdr->size = 1;
>>>>> +
>>>>> + /*
>>>>> + * Current SCU firmware does NOT have return value for
>>>>> + * this API, that means it is always successful.
>>>>> + */
>>>>
>>>> It is not true for the kernel part:
>>>> https://elixir.
>>>>
>> bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Ffirmware%2Fimx%2F
>>>> imx-
>>>>
>> scu.c%23L157&data=02%7C01%7Canson.huang%40nxp.com%7C7a5ed3
>>>>
>> ef3b2541e61be808d7303810a9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
>>>>
>> 0%7C0%7C637030889669489141&sdata=d3uw6x6WCPeavJu3QYf9o9cxx
>>>> Rx4mJar04fQFLF9EhE%3D&reserved=0
>>>>
>>>> imx_scu_call_rpc() may fail in different ways and provide proper error
>> value.
>>>> Please use it.
>>>
>>> There are about 3 APIs are special, this API is one of them, this API
>>> has no return value from SCU FW API, but it has response data from it,
>>> so that means if we set the response to false, the stack will be free
>>> and mailbox will have NULL pointer issue when response data passed
>>> from SCU FW. If we set the response to true, as the SCU FW has no
>>> return value, the return value will be the msg->func which will be
>>> already failed, that is why we have to skip the return value check. This is
>> one restriction/bug of SCU FW, we will notify SCU FW owner to fix/improve.
>>
>> Ok, I see. imx_scu_call_rpc() can return kernel side errors, for example from
>> imx-scu.c framework EINVAL or ETIMEDOUT or what ever error mbox
>> framework may also provide.
>> Aaaannnndd... it can extract an error from SCU package and return it over
>> same way as other errors.
>>
>> And current SCU version has some bugs, so it is providing wrong error value.
>> Soo... as usual the NXP has decided to make the linux kernel a bit more
>> worse to make the SCU firmware happy? Is it what you trying to describe?
>> Really ?! :D
>>
>> Please. Fix the SCU first. The provide fixed kernel patch.
>
> Understood, I will notify SCU owner to fix it, meanwhile it does NOT block this driver,
> I will add return value check in this driver.
It is great! Thank you!
Kind regards,
Oleksij Rempel
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
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 4/4] arm64: dts: add support for A1 based Amlogic AD401
From: Neil Armstrong @ 2019-09-03 7:42 UTC (permalink / raw)
To: Jianxin Pan, Kevin Hilman, linux-amlogic
Cc: devicetree, Hanjie Lin, Victor Wan, Martin Blumenstingl,
linux-kernel, Qiufang Dai, Rob Herring, Jian Hu, Xingyu Chen,
Carlo Caione, Tao Zeng, linux-arm-kernel, Jerome Brunet
In-Reply-To: <1567493475-75451-5-git-send-email-jianxin.pan@amlogic.com>
Hi,
On 03/09/2019 08:51, Jianxin Pan wrote:
> Add basic support for the Amlogic A1 based Amlogic AD401 board:
> which describe components as follows: Reserve Memory, CPU, GIC, IRQ,
> Timer, UART. It's capable of booting up into the serial console.
>
> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
> ---
> arch/arm64/boot/dts/amlogic/Makefile | 1 +
> arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts | 30 ++++++
> arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 121 +++++++++++++++++++++++++
> 3 files changed, 152 insertions(+)
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>
> diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
> index edbf128..1720c45 100644
> --- a/arch/arm64/boot/dts/amlogic/Makefile
> +++ b/arch/arm64/boot/dts/amlogic/Makefile
> @@ -36,3 +36,4 @@ dtb-$(CONFIG_ARCH_MESON) += meson-gxm-rbox-pro.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxm-vega-s96.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-sm1-khadas-vim3l.dtb
> +dtb-$(CONFIG_ARCH_MESON) += meson-a1-ad401.dtb
> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts b/arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts
> new file mode 100644
> index 00000000..3c05cc0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include "meson-a1.dtsi"
> +
> +/ {
> + compatible = "amlogic,ad401", "amlogic,a1";
> + model = "Amlogic Meson A1 AD401 Development Board";
> +
> + aliases {
> + serial0 = &uart_AO_B;
> + };
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> + memory@0 {
> + device_type = "memory";
> + linux,usable-memory = <0x0 0x0 0x0 0x8000000>;
I'll prefer usage of reg, it's handled the same but linux,usable-memory
is not documented.
> + };
> +};
> +
> +&uart_AO_B {
> + status = "okay";
> + /*pinctrl-0 = <&uart_ao_a_pins>;*/
> + /*pinctrl-names = "default";*/
Please remove these lines instead of commenting them.
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> new file mode 100644
> index 00000000..b98d648
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> @@ -0,0 +1,121 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
> + */
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> + compatible = "amlogic,a1";
> +
> + interrupt-parent = <&gic>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + cpus {
> + #address-cells = <0x2>;
> + #size-cells = <0x0>;
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a35";
> + reg = <0x0 0x0>;
> + enable-method = "psci";
> + next-level-cache = <&l2>;
> + };
> +
> + cpu1: cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a35";
> + reg = <0x0 0x1>;
> + enable-method = "psci";
> + next-level-cache = <&l2>;
> + };
> +
> + l2: l2-cache0 {
> + compatible = "cache";
> + };
> + };
> + psci {
> + compatible = "arm,psci-1.0";
> + method = "smc";
> + };
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
Isn't there secmon reserved memory ?
> +
> + linux,cma {
> + compatible = "shared-dma-pool";
> + reusable;
> + size = <0x0 0x800000>;
> + alignment = <0x0 0x400000>;
> + linux,cma-default;
> + };
> + };
> +
> + sm: secure-monitor {
> + compatible = "amlogic,meson-gxbb-sm";
> + };
> +
> + soc {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + uart_AO: serial@fe001c00 {
> + compatible = "amlogic,meson-gx-uart",
> + "amlogic,meson-ao-uart";
> + reg = <0x0 0xfe001c00 0x0 0x18>;
> + interrupts = <GIC_SPI 25 IRQ_TYPE_EDGE_RISING>;
> + clocks = <&xtal>, <&xtal>, <&xtal>;
> + clock-names = "xtal", "pclk", "baud";
> + status = "disabled";
> + };
> +
> + uart_AO_B: serial@fe002000 {
> + compatible = "amlogic,meson-gx-uart",
> + "amlogic,meson-ao-uart";
> + reg = <0x0 0xfe002000 0x0 0x18>;
> + interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
> + clocks = <&xtal>, <&xtal>, <&xtal>;
> + clock-names = "xtal", "pclk", "baud";
> + status = "disabled";
> + };
> +
> + gic: interrupt-controller@ff901000 {
> + compatible = "arm,gic-400";
> + reg = <0x0 0xff901000 0x0 0x1000>,
> + <0x0 0xff902000 0x0 0x2000>,
> + <0x0 0xff904000 0x0 0x2000>,
> + <0x0 0xff906000 0x0 0x2000>;
> + interrupt-controller;
> + interrupts = <GIC_PPI 9
> + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> + #interrupt-cells = <3>;
> + #address-cells = <0>;
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts = <GIC_PPI 13
> + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 14
> + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 11
> + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 10
> + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>;
> + };
> +
> + xtal: xtal-clk {
> + compatible = "fixed-clock";
> + clock-frequency = <24000000>;
> + clock-output-names = "xtal";
> + #clock-cells = <0>;
> + };
> +};
>
Thanks,
Neil
_______________________________________________
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/2] ARM: dts: opos6ul/opos6uldev: rework device tree to support i.MX6ULL
From: Sébastien Szymanski @ 2019-09-03 7:38 UTC (permalink / raw)
To: Shawn Guo
Cc: Mark Rutland, devicetree, Sascha Hauer, Rob Herring,
NXP Linux Team, Pengutronix Kernel Team, Fabio Estevam,
linux-arm-kernel
In-Reply-To: <20190724120623.2385-1-sebastien.szymanski@armadeus.com>
Hello,
On 7/24/19 2:06 PM, Sébastien Szymanski wrote:
> Rework the device trees of the OPOS6UL and OPOS6ULDev boards to support
> the OPOS6UL SoM with an i.MX6ULL SoC. The device trees are now as
> following:
>
> - imx6ul-imx6ull-opos6ul.dtsi
> common for both i.MX6UL and i.MX6ULL OPOS6UL SoM.
> - imx6ul-opos6ul.dtsi
> for i.MX6UL OPOS6UL SoM. It includes imx6ul.dtsi and
> imx6ul-imx6ull-opos6ul.dtsi.
> - imx6ull-opos6ul.dtsi
> for i.MX6ULL OPOS6UL SoM. It includes imx6ull.dtsi and
> imx6ul-imx6ull-opos6ul.dtsi.
>
> - imx6ul-imx6ull-opos6uldev.dtsi
> OPOS6ULDev base device tree.
> - imx6ul-opos6uldev.dts
> OPOS6ULDev board with an i.MX6UL OPOS6UL SoM. It includes
> imx6ul-opos6ul.dtsi and imx6ul-imx6ull-opos6uldevdtsi.
> - imx6ull-opos6uldev.dts
> OPOS6ULDev board with an i.MX6ULL OPOS6UL SoM. It includes
> imx6ull-opos6ul.dtsi and imx6ul-imx6ull-opos6uldevdtsi.
>
> Signed-off-by: Sébastien Szymanski <sebastien.szymanski@armadeus.com>
> ---
>
> Changes for v2:
> - explain the file hierarchy in the commit log
> - use MIT license instead of X11
> - Change compatible properties to "armadeus,imx6{ul,ull}-opos6ul" and
> "armadeus,imx6{ul,ull}-opos6uldev" to follow the bindings of the
> Armadeus boards already supported.
gentle ping...
Regards,
Sébastien
_______________________________________________
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] dt-bindings: gpu: mali-midgard: Add samsung exynos5250 compatible
From: Krzysztof Kozlowski @ 2019-09-03 7:36 UTC (permalink / raw)
To: Guillaume Gardet
Cc: Mark Rutland, devicetree, linux-samsung-soc@vger.kernel.org,
Rob Herring, Kukjin Kim, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20190903073300.5927-1-guillaume.gardet@arm.com>
On Tue, 3 Sep 2019 at 09:33, Guillaume Gardet <guillaume.gardet@arm.com> wrote:
>
> Add "samsung,exynos5250-mali" binding.
>
> Signed-off-by: Guillaume Gardet <guillaume.gardet@arm.com>
>
> Cc: Kukjin Kim <kgene@kernel.org>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Marek Szyprowski <m.szyprowski@samsung.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
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/5] input: keyboard: imx_sc: Add i.MX system controller power key support
From: Anson Huang @ 2019-09-03 7:35 UTC (permalink / raw)
To: Oleksij Rempel, robh+dt@kernel.org, mark.rutland@arm.com,
shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
catalin.marinas@arm.com, will@kernel.org,
dmitry.torokhov@gmail.com, Aisheng Dong, ulf.hansson@linaro.org,
Andy Duan, Peng Fan, Daniel Baluta, Leonard Crestez,
mripard@kernel.org, olof@lixom.net, arnd@arndb.de,
jagan@amarulasolutions.com, bjorn.andersson@linaro.org,
dinguyen@kernel.org, marcin.juszkiewicz@linaro.org,
stefan@agner.ch, gregkh@linuxfoundation.org,
andriy.shevchenko@linux.intel.com, yuehaibing@huawei.com,
tglx@linutronix.de, ronald@innovation.ch, m.felsch@pengutronix.de,
Jacky Bai, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-input@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <dbe0ba0a-29bc-ee96-541d-244b3dbf0b81@pengutronix.de>
Hi, Oleksij
> On 03.09.19 08:48, Anson Huang wrote:
> > Hi, Oleksij
> >
> >> On 03.09.19 16:03, Anson Huang wrote:
> >>> i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller
> >>> inside, the system controller is in charge of controlling power,
> >>> clock and power key etc..
> >>>
> >>> Adds i.MX system controller power key driver support, Linux kernel
> >>> has to communicate with system controller via MU (message unit) IPC
> >>> to get power key's status.
> >>>
> >>> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> >>> ---
> >>> Changes since V1:
> >>> - remove "wakeup-source" property operation, scu power key uses
> >> generic scu irq,
> >>> no need to have this property for device wakeup operation.
> >>> ---
> >>> drivers/input/keyboard/Kconfig | 7 ++
> >>> drivers/input/keyboard/Makefile | 1 +
> >>> drivers/input/keyboard/imx_sc_pwrkey.c | 169
> >> +++++++++++++++++++++++++++++++++
> >>> 3 files changed, 177 insertions(+)
> >>> create mode 100644 drivers/input/keyboard/imx_sc_pwrkey.c
> >>>
> >>> diff --git a/drivers/input/keyboard/Kconfig
> >>> b/drivers/input/keyboard/Kconfig index 2e6d288..3aaeb9c 100644
> >>> --- a/drivers/input/keyboard/Kconfig
> >>> +++ b/drivers/input/keyboard/Kconfig
> >>> @@ -469,6 +469,13 @@ config KEYBOARD_IMX
> >>> To compile this driver as a module, choose M here: the
> >>> module will be called imx_keypad.
> >>>
> >>> +config KEYBOARD_IMX_SC_PWRKEY
> >>> + tristate "IMX SCU Power Key Driver"
> >>> + depends on IMX_SCU
> >>> + help
> >>> + This is the system controller powerkey driver for NXP i.MX SoCs with
> >>> + system controller inside.
> >>
> >> The KEY is configurable over devicetree, why is it called PWRKEY? It
> >> looks for me as generic SCU key handler.
> >
> > We always use it as power key, NOT a generic key, as it has HW
> > function designed for power key purpose.
>
> gpio-key driver is mostly used for power or reboot key. And it is still called
> gpio-key driver. If it is used for power key only, why is it configurable? I can
> configure it as KEY_ENTER or some thing different. This driver has not
> KEY_POWER specific any thing.
Understood, I am making the V3 with all "power" removed, just using the "key".
>
> >
> >>
> >>> config KEYBOARD_NEWTON
> >>> tristate "Newton keyboard"
> >>> select SERIO
> >>> diff --git a/drivers/input/keyboard/Makefile
> >>> b/drivers/input/keyboard/Makefile index 9510325..9ea5585 100644
> >>> --- a/drivers/input/keyboard/Makefile
> >>> +++ b/drivers/input/keyboard/Makefile
> >>> @@ -29,6 +29,7 @@ obj-$(CONFIG_KEYBOARD_HIL) +=
> hil_kbd.o
> >>> obj-$(CONFIG_KEYBOARD_HIL_OLD) += hilkbd.o
> >>> obj-$(CONFIG_KEYBOARD_IPAQ_MICRO) += ipaq-micro-keys.o
> >>> obj-$(CONFIG_KEYBOARD_IMX) += imx_keypad.o
> >>> +obj-$(CONFIG_KEYBOARD_IMX_SC_PWRKEY) += imx_sc_pwrkey.o
> >>> obj-$(CONFIG_KEYBOARD_HP6XX) += jornada680_kbd.o
> >>> obj-$(CONFIG_KEYBOARD_HP7XX) += jornada720_kbd.o
> >>> obj-$(CONFIG_KEYBOARD_LKKBD) += lkkbd.o
> >>> diff --git a/drivers/input/keyboard/imx_sc_pwrkey.c
> >>> b/drivers/input/keyboard/imx_sc_pwrkey.c
> >>> new file mode 100644
> >>> index 0000000..53aa9a4
> >>> --- /dev/null
> >>> +++ b/drivers/input/keyboard/imx_sc_pwrkey.c
> >>> @@ -0,0 +1,169 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * Copyright 2019 NXP.
> >>> + */
> >>> +
> >>> +#include <linux/device.h>
> >>> +#include <linux/err.h>
> >>> +#include <linux/firmware/imx/sci.h> #include <linux/init.h>
> >>> +#include <linux/input.h> #include <linux/interrupt.h> #include
> >>> +<linux/jiffies.h> #include <linux/kernel.h> #include
> >>> +<linux/module.h> #include <linux/of.h> #include
> >>> +<linux/of_address.h> #include <linux/platform_device.h>
> >>> +
> >>> +#define DEBOUNCE_TIME 100
> >>> +#define REPEAT_INTERVAL 60
> >>> +
> >>> +#define SC_IRQ_BUTTON 1
> >>> +#define SC_IRQ_GROUP_WAKE 3
> >>> +#define IMX_SC_MISC_FUNC_GET_BUTTON_STATUS 18
> >>> +
> >>> +struct imx_pwrkey_drv_data {
> >>> + int keycode;
> >>> + bool keystate; /* 1: pressed, 0: release */
> >>> + bool delay_check;
> >>> + struct delayed_work check_work;
> >>> + struct input_dev *input;
> >>> +};
> >>> +
> >>> +struct imx_sc_msg_pwrkey {
> >>> + struct imx_sc_rpc_msg hdr;
> >>> + u8 state;
> >>> +};
> >>> +static struct imx_pwrkey_drv_data *pdata;
> >>
> >> Why is it global struct? It seems to be flexible configurable over devicetree.
> >> So I would assume it should be able to handle more then one button.
> >> Please remove global variables, make it allocatable per OF node.
> >
> > There is ONLY one button available for SC key, but yes, I think I can
> > make the structure private and get all necessary data from the structure
> using container_of.
>
> And we will never need more then 640 kB RAM ;)
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.wi
> kiquote.org%2Fwiki%2FTalk%3ABill_Gates&data=02%7C01%7Canson.hu
> ang%40nxp.com%7C4d4d7458087747e0d8f008d7304057e9%7C686ea1d3bc2
> b4c6fa92cd99c5c301635%7C0%7C0%7C637030925236150243&sdata=w
> %2FGXBaHfnBdLwjTxjbzWSPeIw3ExL%2Fs9IMOgF1onL6A%3D&reserved
> =0
>
> >
> >>
> >> Please use different name "pdata" is usually used as platform data.
> >> Please, use "priv".
> >
> > OK.
> >
> >>
> >>> +static struct imx_sc_ipc *pwrkey_ipc_handle;
> >>
> >> same as before, no global variables.
> >
> > Will move it into private platform data structure.
> >
> >>
> >>> +
> >>> +static int imx_sc_pwrkey_notify(struct notifier_block *nb,
> >>> + unsigned long event, void *group) {
> >>> + if ((event & SC_IRQ_BUTTON) && (*(u8 *)group ==
> >> SC_IRQ_GROUP_WAKE)
> >>> + && !pdata->delay_check) {
> >>> + pdata->delay_check = 1;
> >>> + schedule_delayed_work(&pdata->check_work,
> >>> + msecs_to_jiffies(REPEAT_INTERVAL));
> >>> + }
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> +static void imx_sc_check_for_events(struct work_struct *work) {
> >>> + struct input_dev *input = pdata->input;
> >>> + struct imx_sc_msg_pwrkey msg;
> >>> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
> >>> + bool state;
> >>> +
> >>> + hdr->ver = IMX_SC_RPC_VERSION;
> >>> + hdr->svc = IMX_SC_RPC_SVC_MISC;
> >>> + hdr->func = IMX_SC_MISC_FUNC_GET_BUTTON_STATUS;
> >>> + hdr->size = 1;
> >>> +
> >>> + /*
> >>> + * Current SCU firmware does NOT have return value for
> >>> + * this API, that means it is always successful.
> >>> + */
> >>
> >> It is not true for the kernel part:
> >> https://elixir.
> >>
> bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Ffirmware%2Fimx%2F
> >> imx-
> >>
> scu.c%23L157&data=02%7C01%7Canson.huang%40nxp.com%7C7a5ed3
> >>
> ef3b2541e61be808d7303810a9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
> >>
> 0%7C0%7C637030889669489141&sdata=d3uw6x6WCPeavJu3QYf9o9cxx
> >> Rx4mJar04fQFLF9EhE%3D&reserved=0
> >>
> >> imx_scu_call_rpc() may fail in different ways and provide proper error
> value.
> >> Please use it.
> >
> > There are about 3 APIs are special, this API is one of them, this API
> > has no return value from SCU FW API, but it has response data from it,
> > so that means if we set the response to false, the stack will be free
> > and mailbox will have NULL pointer issue when response data passed
> > from SCU FW. If we set the response to true, as the SCU FW has no
> > return value, the return value will be the msg->func which will be
> > already failed, that is why we have to skip the return value check. This is
> one restriction/bug of SCU FW, we will notify SCU FW owner to fix/improve.
>
> Ok, I see. imx_scu_call_rpc() can return kernel side errors, for example from
> imx-scu.c framework EINVAL or ETIMEDOUT or what ever error mbox
> framework may also provide.
> Aaaannnndd... it can extract an error from SCU package and return it over
> same way as other errors.
>
> And current SCU version has some bugs, so it is providing wrong error value.
> Soo... as usual the NXP has decided to make the linux kernel a bit more
> worse to make the SCU firmware happy? Is it what you trying to describe?
> Really ?! :D
>
> Please. Fix the SCU first. The provide fixed kernel patch.
Understood, I will notify SCU owner to fix it, meanwhile it does NOT block this driver,
I will add return value check in this driver.
Thanks,
Anson
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] dt-bindings: gpu: mali-midgard: Add samsung exynos5250 compatible
From: Guillaume Gardet @ 2019-09-03 7:33 UTC (permalink / raw)
To: linux-samsung-soc
Cc: Mark Rutland, devicetree, Guillaume Gardet, Krzysztof Kozlowski,
Rob Herring, Kukjin Kim, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <20190830104502.7128-2-guillaume.gardet@arm.com>
Add "samsung,exynos5250-mali" binding.
Signed-off-by: Guillaume Gardet <guillaume.gardet@arm.com>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
---
Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml
index b99a43bb471a..47bc1ac36426 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml
@@ -30,12 +30,15 @@ properties:
- enum:
- rockchip,rk3399-mali
- const: arm,mali-t860
+ - items:
+ - enum:
+ - samsung,exynos5250-mali
+ - const: arm,mali-t604
- items:
- enum:
- samsung,exynos5433-mali
- const: arm,mali-t760
- # "arm,mali-t604"
# "arm,mali-t624"
# "arm,mali-t628"
# "arm,mali-t830"
--
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
* Re: [PATCH 4/4] arm64: dts: add support for A1 based Amlogic AD401
From: Jerome Brunet @ 2019-09-03 7:30 UTC (permalink / raw)
To: Jianxin Pan, Kevin Hilman, linux-amlogic
Cc: devicetree, Hanjie Lin, Victor Wan, Jianxin Pan, Neil Armstrong,
Martin Blumenstingl, linux-kernel, Qiufang Dai, Rob Herring,
Jian Hu, Xingyu Chen, Carlo Caione, Tao Zeng, linux-arm-kernel
In-Reply-To: <1567493475-75451-5-git-send-email-jianxin.pan@amlogic.com>
On Tue 03 Sep 2019 at 02:51, Jianxin Pan <jianxin.pan@amlogic.com> wrote:
> Add basic support for the Amlogic A1 based Amlogic AD401 board:
> which describe components as follows: Reserve Memory, CPU, GIC, IRQ,
> Timer, UART. It's capable of booting up into the serial console.
>
> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
> ---
> arch/arm64/boot/dts/amlogic/Makefile | 1 +
> arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts | 30 ++++++
> arch/arm64/boot/dts/amlogic/meson-a1.dtsi | 121 +++++++++++++++++++++++++
> 3 files changed, 152 insertions(+)
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts
> create mode 100644 arch/arm64/boot/dts/amlogic/meson-a1.dtsi
>
> diff --git a/arch/arm64/boot/dts/amlogic/Makefile b/arch/arm64/boot/dts/amlogic/Makefile
> index edbf128..1720c45 100644
> --- a/arch/arm64/boot/dts/amlogic/Makefile
> +++ b/arch/arm64/boot/dts/amlogic/Makefile
> @@ -36,3 +36,4 @@ dtb-$(CONFIG_ARCH_MESON) += meson-gxm-rbox-pro.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-gxm-vega-s96.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-sm1-sei610.dtb
> dtb-$(CONFIG_ARCH_MESON) += meson-sm1-khadas-vim3l.dtb
> +dtb-$(CONFIG_ARCH_MESON) += meson-a1-ad401.dtb
> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts b/arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts
> new file mode 100644
> index 00000000..3c05cc0
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-a1-ad401.dts
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
> + */
> +
> +/dts-v1/;
> +
> +#include "meson-a1.dtsi"
> +
> +/ {
> + compatible = "amlogic,ad401", "amlogic,a1";
> + model = "Amlogic Meson A1 AD401 Development Board";
> +
> + aliases {
> + serial0 = &uart_AO_B;
> + };
Newline here please
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
same
> + memory@0 {
> + device_type = "memory";
> + linux,usable-memory = <0x0 0x0 0x0 0x8000000>;
> + };
> +};
> +
> +&uart_AO_B {
> + status = "okay";
> + /*pinctrl-0 = <&uart_ao_a_pins>;*/
> + /*pinctrl-names = "default";*/
Remove the commented code please
> +};
> diff --git a/arch/arm64/boot/dts/amlogic/meson-a1.dtsi b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> new file mode 100644
> index 00000000..b98d648
> --- /dev/null
> +++ b/arch/arm64/boot/dts/amlogic/meson-a1.dtsi
> @@ -0,0 +1,121 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
> + */
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +/ {
> + compatible = "amlogic,a1";
> +
> + interrupt-parent = <&gic>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + cpus {
> + #address-cells = <0x2>;
> + #size-cells = <0x0>;
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a35";
> + reg = <0x0 0x0>;
> + enable-method = "psci";
> + next-level-cache = <&l2>;
> + };
> +
> + cpu1: cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a35";
> + reg = <0x0 0x1>;
> + enable-method = "psci";
> + next-level-cache = <&l2>;
> + };
> +
> + l2: l2-cache0 {
> + compatible = "cache";
> + };
> + };
New line here please
With this minor comments adressed, looks good.
Reviewed-by: Jerome Brunet <jbrunet@baylibre.com>
> + psci {
> + compatible = "arm,psci-1.0";
> + method = "smc";
> + };
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + linux,cma {
> + compatible = "shared-dma-pool";
> + reusable;
> + size = <0x0 0x800000>;
> + alignment = <0x0 0x400000>;
> + linux,cma-default;
> + };
> + };
> +
> + sm: secure-monitor {
> + compatible = "amlogic,meson-gxbb-sm";
> + };
> +
> + soc {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + uart_AO: serial@fe001c00 {
> + compatible = "amlogic,meson-gx-uart",
> + "amlogic,meson-ao-uart";
> + reg = <0x0 0xfe001c00 0x0 0x18>;
> + interrupts = <GIC_SPI 25 IRQ_TYPE_EDGE_RISING>;
> + clocks = <&xtal>, <&xtal>, <&xtal>;
> + clock-names = "xtal", "pclk", "baud";
> + status = "disabled";
> + };
> +
> + uart_AO_B: serial@fe002000 {
> + compatible = "amlogic,meson-gx-uart",
> + "amlogic,meson-ao-uart";
> + reg = <0x0 0xfe002000 0x0 0x18>;
> + interrupts = <GIC_SPI 26 IRQ_TYPE_EDGE_RISING>;
> + clocks = <&xtal>, <&xtal>, <&xtal>;
> + clock-names = "xtal", "pclk", "baud";
> + status = "disabled";
> + };
> +
> + gic: interrupt-controller@ff901000 {
> + compatible = "arm,gic-400";
> + reg = <0x0 0xff901000 0x0 0x1000>,
> + <0x0 0xff902000 0x0 0x2000>,
> + <0x0 0xff904000 0x0 0x2000>,
> + <0x0 0xff906000 0x0 0x2000>;
> + interrupt-controller;
> + interrupts = <GIC_PPI 9
> + (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_HIGH)>;
> + #interrupt-cells = <3>;
> + #address-cells = <0>;
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts = <GIC_PPI 13
> + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 14
> + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 11
> + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>,
> + <GIC_PPI 10
> + (GIC_CPU_MASK_RAW(0xff) | IRQ_TYPE_LEVEL_LOW)>;
> + };
> +
> + xtal: xtal-clk {
> + compatible = "fixed-clock";
> + clock-frequency = <24000000>;
> + clock-output-names = "xtal";
> + #clock-cells = <0>;
> + };
> +};
> --
> 2.7.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH V2 2/5] input: keyboard: imx_sc: Add i.MX system controller power key support
From: Oleksij Rempel @ 2019-09-03 7:28 UTC (permalink / raw)
To: Anson Huang, robh+dt@kernel.org, mark.rutland@arm.com,
shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com,
catalin.marinas@arm.com, will@kernel.org,
dmitry.torokhov@gmail.com, Aisheng Dong, ulf.hansson@linaro.org,
Andy Duan, Peng Fan, Daniel Baluta, Leonard Crestez,
mripard@kernel.org, olof@lixom.net, arnd@arndb.de,
jagan@amarulasolutions.com, bjorn.andersson@linaro.org,
dinguyen@kernel.org, marcin.juszkiewicz@linaro.org,
stefan@agner.ch, gregkh@linuxfoundation.org,
andriy.shevchenko@linux.intel.com, yuehaibing@huawei.com,
tglx@linutronix.de, ronald@innovation.ch, m.felsch@pengutronix.de,
Jacky Bai, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, linux-input@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <DB3PR0402MB391602C6B425DD7EBFB9AF1DF5B90@DB3PR0402MB3916.eurprd04.prod.outlook.com>
On 03.09.19 08:48, Anson Huang wrote:
> Hi, Oleksij
>
>> On 03.09.19 16:03, Anson Huang wrote:
>>> i.MX8QXP is an ARMv8 SoC which has a Cortex-M4 system controller
>>> inside, the system controller is in charge of controlling power, clock
>>> and power key etc..
>>>
>>> Adds i.MX system controller power key driver support, Linux kernel has
>>> to communicate with system controller via MU (message unit) IPC to get
>>> power key's status.
>>>
>>> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>>> ---
>>> Changes since V1:
>>> - remove "wakeup-source" property operation, scu power key uses
>> generic scu irq,
>>> no need to have this property for device wakeup operation.
>>> ---
>>> drivers/input/keyboard/Kconfig | 7 ++
>>> drivers/input/keyboard/Makefile | 1 +
>>> drivers/input/keyboard/imx_sc_pwrkey.c | 169
>> +++++++++++++++++++++++++++++++++
>>> 3 files changed, 177 insertions(+)
>>> create mode 100644 drivers/input/keyboard/imx_sc_pwrkey.c
>>>
>>> diff --git a/drivers/input/keyboard/Kconfig
>>> b/drivers/input/keyboard/Kconfig index 2e6d288..3aaeb9c 100644
>>> --- a/drivers/input/keyboard/Kconfig
>>> +++ b/drivers/input/keyboard/Kconfig
>>> @@ -469,6 +469,13 @@ config KEYBOARD_IMX
>>> To compile this driver as a module, choose M here: the
>>> module will be called imx_keypad.
>>>
>>> +config KEYBOARD_IMX_SC_PWRKEY
>>> + tristate "IMX SCU Power Key Driver"
>>> + depends on IMX_SCU
>>> + help
>>> + This is the system controller powerkey driver for NXP i.MX SoCs with
>>> + system controller inside.
>>
>> The KEY is configurable over devicetree, why is it called PWRKEY? It looks for
>> me as generic SCU key handler.
>
> We always use it as power key, NOT a generic key, as it has HW function designed
> for power key purpose.
gpio-key driver is mostly used for power or reboot key. And it is still called gpio-key
driver. If it is used for power key only, why is it configurable? I can configure it as
KEY_ENTER or some thing different. This driver has not KEY_POWER specific any thing.
>
>>
>>> config KEYBOARD_NEWTON
>>> tristate "Newton keyboard"
>>> select SERIO
>>> diff --git a/drivers/input/keyboard/Makefile
>>> b/drivers/input/keyboard/Makefile index 9510325..9ea5585 100644
>>> --- a/drivers/input/keyboard/Makefile
>>> +++ b/drivers/input/keyboard/Makefile
>>> @@ -29,6 +29,7 @@ obj-$(CONFIG_KEYBOARD_HIL) += hil_kbd.o
>>> obj-$(CONFIG_KEYBOARD_HIL_OLD) += hilkbd.o
>>> obj-$(CONFIG_KEYBOARD_IPAQ_MICRO) += ipaq-micro-keys.o
>>> obj-$(CONFIG_KEYBOARD_IMX) += imx_keypad.o
>>> +obj-$(CONFIG_KEYBOARD_IMX_SC_PWRKEY) += imx_sc_pwrkey.o
>>> obj-$(CONFIG_KEYBOARD_HP6XX) += jornada680_kbd.o
>>> obj-$(CONFIG_KEYBOARD_HP7XX) += jornada720_kbd.o
>>> obj-$(CONFIG_KEYBOARD_LKKBD) += lkkbd.o
>>> diff --git a/drivers/input/keyboard/imx_sc_pwrkey.c
>>> b/drivers/input/keyboard/imx_sc_pwrkey.c
>>> new file mode 100644
>>> index 0000000..53aa9a4
>>> --- /dev/null
>>> +++ b/drivers/input/keyboard/imx_sc_pwrkey.c
>>> @@ -0,0 +1,169 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright 2019 NXP.
>>> + */
>>> +
>>> +#include <linux/device.h>
>>> +#include <linux/err.h>
>>> +#include <linux/firmware/imx/sci.h>
>>> +#include <linux/init.h>
>>> +#include <linux/input.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/jiffies.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#define DEBOUNCE_TIME 100
>>> +#define REPEAT_INTERVAL 60
>>> +
>>> +#define SC_IRQ_BUTTON 1
>>> +#define SC_IRQ_GROUP_WAKE 3
>>> +#define IMX_SC_MISC_FUNC_GET_BUTTON_STATUS 18
>>> +
>>> +struct imx_pwrkey_drv_data {
>>> + int keycode;
>>> + bool keystate; /* 1: pressed, 0: release */
>>> + bool delay_check;
>>> + struct delayed_work check_work;
>>> + struct input_dev *input;
>>> +};
>>> +
>>> +struct imx_sc_msg_pwrkey {
>>> + struct imx_sc_rpc_msg hdr;
>>> + u8 state;
>>> +};
>>> +static struct imx_pwrkey_drv_data *pdata;
>>
>> Why is it global struct? It seems to be flexible configurable over devicetree.
>> So I would assume it should be able to handle more then one button. Please
>> remove global variables, make it allocatable per OF node.
>
> There is ONLY one button available for SC key, but yes, I think I can make the structure
> private and get all necessary data from the structure using container_of.
And we will never need more then 640 kB RAM ;)
https://en.wikiquote.org/wiki/Talk:Bill_Gates
>
>>
>> Please use different name "pdata" is usually used as platform data. Please,
>> use "priv".
>
> OK.
>
>>
>>> +static struct imx_sc_ipc *pwrkey_ipc_handle;
>>
>> same as before, no global variables.
>
> Will move it into private platform data structure.
>
>>
>>> +
>>> +static int imx_sc_pwrkey_notify(struct notifier_block *nb,
>>> + unsigned long event, void *group) {
>>> + if ((event & SC_IRQ_BUTTON) && (*(u8 *)group ==
>> SC_IRQ_GROUP_WAKE)
>>> + && !pdata->delay_check) {
>>> + pdata->delay_check = 1;
>>> + schedule_delayed_work(&pdata->check_work,
>>> + msecs_to_jiffies(REPEAT_INTERVAL));
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void imx_sc_check_for_events(struct work_struct *work) {
>>> + struct input_dev *input = pdata->input;
>>> + struct imx_sc_msg_pwrkey msg;
>>> + struct imx_sc_rpc_msg *hdr = &msg.hdr;
>>> + bool state;
>>> +
>>> + hdr->ver = IMX_SC_RPC_VERSION;
>>> + hdr->svc = IMX_SC_RPC_SVC_MISC;
>>> + hdr->func = IMX_SC_MISC_FUNC_GET_BUTTON_STATUS;
>>> + hdr->size = 1;
>>> +
>>> + /*
>>> + * Current SCU firmware does NOT have return value for
>>> + * this API, that means it is always successful.
>>> + */
>>
>> It is not true for the kernel part:
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.
>> bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Ffirmware%2Fimx%2F
>> imx-
>> scu.c%23L157&data=02%7C01%7Canson.huang%40nxp.com%7C7a5ed3
>> ef3b2541e61be808d7303810a9%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
>> 0%7C0%7C637030889669489141&sdata=d3uw6x6WCPeavJu3QYf9o9cxx
>> Rx4mJar04fQFLF9EhE%3D&reserved=0
>>
>> imx_scu_call_rpc() may fail in different ways and provide proper error value.
>> Please use it.
>
> There are about 3 APIs are special, this API is one of them, this API has no return value
> from SCU FW API, but it has response data from it, so that means if we set the response
> to false, the stack will be free and mailbox will have NULL pointer issue when response
> data passed from SCU FW. If we set the response to true, as the SCU FW has no return value,
> the return value will be the msg->func which will be already failed, that is why we have to skip
> the return value check. This is one restriction/bug of SCU FW, we will notify SCU FW owner to
> fix/improve.
Ok, I see. imx_scu_call_rpc() can return kernel side errors, for example from imx-scu.c
framework EINVAL or ETIMEDOUT or what ever error mbox framework may also provide.
Aaaannnndd... it can extract an error from SCU package and return it over same way as
other errors.
And current SCU version has some bugs, so it is providing wrong error value. Soo... as
usual the NXP has decided to make the linux kernel a bit more worse to make the SCU
firmware happy? Is it what you trying to describe? Really ?! :D
Please. Fix the SCU first. The provide fixed kernel patch.
>>
>>> + imx_scu_call_rpc(pwrkey_ipc_handle, &msg, true); > + state =
>> msg.state;
>>
>> the conversation u8 to bool should be done here.
>
> OK.
>
>>
>>> +
>>> + if (!state && !pdata->keystate)
>>> + state = true;
>>> +
>>> + if (state ^ pdata->keystate) {
>>> + pm_wakeup_event(input->dev.parent, 0);
>>> + pdata->keystate = !!state;
>>
>> the state is already bool. Why do you need extra
>> conversations?
>
> Will remove it.
>
>>
>>> + input_event(input, EV_KEY, pdata->keycode, !!state);
>>
>> same here.
>
> Will remove it.
>
>>
>>> + input_sync(input);
>>> + if (!state)
>>> + pdata->delay_check = 0;
>>> + pm_relax(pdata->input->dev.parent);
>>> + }
>>> +
>>> + if (state)
>>> + schedule_delayed_work(&pdata->check_work,
>>> + msecs_to_jiffies(DEBOUNCE_TIME)); }
>>> +
>>> +static struct notifier_block imx_sc_pwrkey_notifier = {
>>> + .notifier_call = imx_sc_pwrkey_notify, };
>>> +
>>> +static int imx_sc_pwrkey_probe(struct platform_device *pdev) {
>>> + struct device_node *np = pdev->dev.of_node;
>>> + struct input_dev *input;
>>> + int ret;
>>> +
>>> + ret = imx_scu_get_handle(&pwrkey_ipc_handle);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
>>> + if (!pdata)
>>> + return -ENOMEM;
>>> +
>>> + if (of_property_read_u32(np, "linux,keycode", &pdata->keycode) > +
>> pdata->keycode = KEY_POWER;
>>
>> According binding documentation, linux,keycode is requered parameter, in
>> this case you should fail if it is not set.
>
> Agreed, will add it in V3.
>
> Thanks,
> Anson.
>
Kind regards,
Oleksij Rempel
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCHv3 01/10] dt-bindings: omap: add new binding for PRM instances
From: Tero Kristo @ 2019-09-03 7:25 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, tony, p.zabel, ssantosh, linux-omap, linux-arm-kernel
In-Reply-To: <20190902042631.GA22055@bogus>
On 02/09/2019 16:39, Rob Herring wrote:
> On Fri, Aug 30, 2019 at 03:18:07PM +0300, Tero Kristo wrote:
>> Add new binding for OMAP PRM (Power and Reset Manager) instances. Each
>> of these will act as a power domain controller and potentially as a reset
>> provider.
>>
>
> Converting this to schema would be nice.
Do you have documentation about schema somewhere? Basically what I need
to do to fix this.
>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> ---
>> .../devicetree/bindings/arm/omap/prm-inst.txt | 31 +++++++++++++++++++
>
> bindings/reset/
I did not put this under reset, because this is basically a
multi-purpose function. Reset just happens to be the first functionality
it is going to provide. It will be followed by power domain support
later on.
Any thoughts?
>
>> 1 file changed, 31 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/arm/omap/prm-inst.txt
>>
>> diff --git a/Documentation/devicetree/bindings/arm/omap/prm-inst.txt b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt
>> new file mode 100644
>> index 000000000000..7c7527c37734
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/omap/prm-inst.txt
>> @@ -0,0 +1,31 @@
>> +OMAP PRM instance bindings
>> +
>> +Power and Reset Manager is an IP block on OMAP family of devices which
>> +handle the power domains and their current state, and provide reset
>> +handling for the domains and/or separate IP blocks under the power domain
>> +hierarchy.
>> +
>> +Required properties:
>> +- compatible: Must be one of:
>> + "ti,am3-prm-inst"
>> + "ti,am4-prm-inst"
>> + "ti,omap4-prm-inst"
>> + "ti,omap5-prm-inst"
>> + "ti,dra7-prm-inst"
>
> '-inst' seems a bit redundant.
ti,xyz-prm is already reserved by the parent node of all these.
The hierarchy is basically like this (omap4 as example):
prm: prm@4a306000 {
compatible = "ti,omap4-prm";
...
prm_dsp: prm@400 {
compatible = "ti,omap4-prm-inst";
...
};
prm_device: prm@1b00 {
compatible = "ti,omap4-prm-inst";
...
};
...
};
>
>> +- reg: Contains PRM instance register address range
>> + (base address and length)
>> +
>> +Optional properties:
>> +- #reset-cells: Should be 1 if the PRM instance in question supports resets.
>> +- clocks: Associated clocks for the reset signals if any. Certain reset
>> + signals can't be toggled properly without functional clock
>> + being active for them.
>> +
>> +Example:
>> +
>> +prm_dsp2: prm@1b00 {
>
> reset-controller@...
Well, as said, the same node is going to be also power domain provider
later on...
>
>> + compatible = "ti,dra7-prm-inst";
>> + reg = <0x1b00 0x40>;
>> + #reset-cells = <1>;
>> + clocks = <&dsp2_clkctrl DRA7_DSP2_MMU0_DSP2_CLKCTRL 0>;
>> +};
>> --
>> 2.17.1
>>
>> --
>
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
_______________________________________________
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] soc: amlogic: meson-gx-socinfo: Add A1 and A113L IDs
From: Neil Armstrong @ 2019-09-03 7:12 UTC (permalink / raw)
To: Jianxin Pan, Kevin Hilman, linux-amlogic
Cc: devicetree, Hanjie Lin, Victor Wan, Martin Blumenstingl,
linux-kernel, Qiufang Dai, Rob Herring, Jian Hu, Xingyu Chen,
Carlo Caione, Tao Zeng, linux-arm-kernel, Jerome Brunet
In-Reply-To: <1567493475-75451-2-git-send-email-jianxin.pan@amlogic.com>
Hi,
On 03/09/2019 08:51, Jianxin Pan wrote:
> Add the SoC IDs for the A113L Amlogic A1 SoC.
>
> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
> ---
> drivers/soc/amlogic/meson-gx-socinfo.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/soc/amlogic/meson-gx-socinfo.c b/drivers/soc/amlogic/meson-gx-socinfo.c
> index 6d0d04f..3c86d8d 100644
> --- a/drivers/soc/amlogic/meson-gx-socinfo.c
> +++ b/drivers/soc/amlogic/meson-gx-socinfo.c
> @@ -40,6 +40,7 @@ static const struct meson_gx_soc_id {
> { "G12A", 0x28 },
> { "G12B", 0x29 },
> { "SM1", 0x2b },
> + { "A1", 0x2c },
> };
>
> static const struct meson_gx_package_id {
> @@ -68,6 +69,7 @@ static const struct meson_gx_package_id {
> { "S922X", 0x29, 0x40, 0xf0 },
> { "A311D", 0x29, 0x10, 0xf0 },
> { "S905X3", 0x2b, 0x5, 0xf },
> + { "A113L", 0x2c, 0x0, 0xf8 },
> };
>
> static inline unsigned int socinfo_to_major(u32 socinfo)
>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Thanks,
Neil
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [RFC PATCH V2 4/4] platform: mtk-isp: Add Mediatek FD driver
From: Tomasz Figa @ 2019-09-03 7:04 UTC (permalink / raw)
To: Jerry-ch Chen
Cc: devicetree@vger.kernel.org, Sean Cheng (鄭昇弘),
laurent.pinchart+renesas@ideasonboard.com,
Rynn Wu (吳育恩), srv_heupstream,
Po-Yang Huang (黃柏陽), mchehab@kernel.org,
suleiman@chromium.org, shik@chromium.org,
Jungo Lin (林明俊),
Sj Huang (黃信璋), yuzhao@chromium.org,
linux-mediatek@lists.infradead.org, zwisler@chromium.org,
matthias.bgg@gmail.com, Christie Yu (游雅惠),
Frederic Chen (陳俊元), hans.verkuil@cisco.com,
linux-arm-kernel@lists.infradead.org, linux-media@vger.kernel.org
In-Reply-To: <1567493081.18318.49.camel@mtksdccf07>
On Tue, Sep 3, 2019 at 3:44 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
>
> On Tue, 2019-09-03 at 13:19 +0800, Tomasz Figa wrote:
> > On Mon, Sep 2, 2019 at 8:47 PM Jerry-ch Chen <Jerry-ch.Chen@mediatek.com> wrote:
> > >
> > > Hi Tomasz,
> > >
> > > On Fri, 2019-08-30 at 16:33 +0800, Tomasz Figa wrote:
> > > > On Wed, Aug 28, 2019 at 11:00 AM Jerry-ch Chen
> > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > >
> > > > > Hi Tomasz,
> > > > >
> > > > > On Mon, 2019-08-26 at 14:36 +0800, Tomasz Figa wrote:
> > > > > > Hi Jerry,
> > > > > >
> > > > > > On Sun, Aug 25, 2019 at 6:18 PM Jerry-ch Chen
> > > > > > <Jerry-ch.Chen@mediatek.com> wrote:
> > > > > > >
> > > > > > > Hi Tomasz,
> > > > > > >
> > > > > > > On Fri, 2019-08-02 at 16:28 +0800, Tomasz Figa wrote:
> > > > > > > > Hi Jerry,
> > > > > > > >
> > > > > > > > On Tue, Jul 09, 2019 at 04:41:12PM +0800, Jerry-ch Chen wrote:
> > [snip]
> > > > > static int mtk_fd_vb2_queue_setup(struct vb2_queue *vq,
> > > > > unsigned int *num_buffers,
> > > > > unsigned int *num_planes,
> > > > > unsigned int sizes[],
> > > > > struct device *alloc_devs[])
> > > > > {
> > > > > struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > > > struct device *dev = ctx->dev;
> > > > > unsigned int size[2];
> > > > >
> > > > > switch (vq->type) {
> > > > > case V4L2_BUF_TYPE_META_CAPTURE:
> > > > > size[0] = ctx->dst_fmt.buffersize;
> > > > > break;
> > > > > case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> > > > > size[0] = ctx->src_fmt.plane_fmt[0].sizeimage;
> > > > > if (*num_planes == 2)
> > > > > size[1] = ctx->src_fmt.plane_fmt[1].sizeimage;
> > > > > break;
> > > > > }
> > > > >
> > > > > if (*num_planes == 1) {
> > > > > if (sizes[0] < size[0])
> > > > > return -EINVAL;
> > > > > } else if (*num_planes == 2) {
> > > > > if ((sizes[0] < size[0]) && (sizes[1] < size[1]))
> > > > > return -EINVAL;
> > > >
> > > > Can we just use a loop here and combine the 2 cases above?
> > > >
> > > > Also, we need to fail with -EINVAL if *num_planes is > 2.
> > > >
> > > > > } else {
> > > > > *num_planes = 1;
> > > > > sizes[0] = size[0];
> > > >
> > > > This should be the case if *num_planes == 0 and the number of planes
> > > > and sizes should match the currently active format.
> > > >
> > > I appreciate your comments,
> > >
> > > Ok, I will update as following:
> > > static int mtk_fd_vb2_queue_setup(struct vb2_queue *vq,
> > > unsigned int *num_buffers,
> > > unsigned int *num_planes,
> > > unsigned int sizes[],
> > > struct device *alloc_devs[])
> > > {
> > > struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > unsigned int size[2];
> > > unsigned int plane;
> > >
> > > switch (vq->type) {
> > > case V4L2_BUF_TYPE_META_CAPTURE:
> > > size[0] = ctx->dst_fmt.buffersize;
> > > break;
> > > case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> > > size[0] = ctx->src_fmt.plane_fmt[0].sizeimage;
> > > if (*num_planes == 2)
> > > size[1] = ctx->src_fmt.plane_fmt[1].sizeimage;
> > > break;
> > > }
> > >
> > > if (*num_planes > 2)
> > > return -EINVAL;
> > > if (*num_planes == 0) {
> > > if (vq->type == V4L2_BUF_TYPE_META_CAPTURE) {
> > > sizes[0] = ctx->dst_fmt.buffersize;
> > > *num_planes = 1;
> > > return 0;
> > > }
> > >
> > > *num_planes = ctx->src_fmt.num_planes;
> > > for (plane = 0; plane < *num_planes; plane++)
> > > sizes[plane] = ctx->src_fmt.plane_fmt[plane].sizeimage;
> > > return 0;
> > > }
> > >
> > > for (plane = 0; plane < *num_planes; plane++) {
> > > if(sizes[plane] < size[plane])
> > > return -EINVAL;
> > > }
> > > return 0;
> > > }
> > >
> >
> > Looks good, thanks!
> >
> > > > > }
> > > > >
> > > > > return 0;
> > > > > }
> > > > >
> > > > > > [snip]
> > > > > >
> > > > > > > > > +static void mtk_fd_vb2_stop_streaming(struct vb2_queue *vq)
> > > > > > > > > +{
> > > > > > > > > + struct mtk_fd_ctx *ctx = vb2_get_drv_priv(vq);
> > > > > > > > > + struct vb2_buffer *vb;
> > > > > > > >
> > > > > > > > How do we guarantee here that the hardware isn't still accessing the buffers
> > > > > > > > removed below?
> > > > > > > >
> > > > > > > Maybe we can check the driver state flag and aborting the unfinished
> > > > > > > jobs?
> > > > > > > (fd_hw->state == FD_ENQ)
> > > > > > >
> > > > > >
> > > > > > Yes, we need to either cancel or wait for the currently processing
> > > > > > job. It depends on hardware capabilities, but cancelling is generally
> > > > > > preferred for the lower latency.
> > > > > >
> > > > > Ok, it the state is ENQ, then we can disable the FD hw by controlling
> > > > > the registers.
> > > > >
> > > > > for example:
> > > > > writel(0x0, fd->fd_base + FD_HW_ENABLE);
> > > > > writel(0x0, fd->fd_base + FD_INT_EN);
> > > > >
> > > >
> > > > What's exactly the effect of writing 0 to FD_HW_ENABLE?
> > > >
> > > Sorry, my last reply didn't solve the question,
> > > we should implement a mtk_fd_job_abort() for v4l2_m2m_ops().
> > >
> > > which is able to readl_poll_timeout_atomic()
> > > and check the HW busy bits in the register FD_INT_EN;
> > >
> > > if they are not cleared until timeout, we could handle the last
> > > processing job.
> > > Otherwise, the FD irq handler should have handled the last processing
> > > job and we could continue the stop_streaming().
> > >
> > > For job_abort():
> > > static void mtk_fd_job_abort(void *priv)
> > > {
> > > struct mtk_fd_ctx *ctx = priv;
> > > struct mtk_fd_dev *fd = ctx->fd_dev;
> > > u32 val;
> > > u32 ret;
> > >
> > > ret = readl_poll_timeout_atomic(fd->fd_base + MTK_FD_REG_OFFSET_INT_EN,
> > > val,
> > > (val & MTK_FD_HW_BUSY_MASK) ==
> > > MTK_FD_HW_STATE_IS_BUSY,
> > > USEC_PER_MSEC, MTK_FD_STOP_HW_TIMEOUT);
> >
> > Hmm, would it be possible to avoid the busy wait by having a
> > completion that could be signalled from the interrupt handler?
> >
> > Best regards,
> > Tomasz
>
> I suppose that would be wakeup a wait queue in the interrupt handler,
> the the wait_event_interrupt_timeout() will be used in here and system
> suspend e.g. mtk_fd_suspend().
Yes, that should work.
> Or do you suggest to wait_event_interrupt_timeout() every frame in the
> mtk_fd_ipi_handler()?
Nope, we shouldn't need that.
> I think maybe the readl_poll_timeout_atomic would be good enough.
>
Not really. Busy waiting should be avoided as much as possible. What's
the point of entering suspend if you end up burning the power by
spinning the CPU for some milliseconds?
>
> One more thing, for the mtk_fd_video_device_register()
> Sorry that I would need to use intermediate variable here since the 80
> columns check.
>
> function = MEDIA_ENT_F_PROC_VIDEO_STATISTICS;
> ret = v4l2_m2m_register_media_controller(m2m_dev, vfd, function);
Why not just make it like this:
ret = v4l2_m2m_register_media_controller(m2m_dev,
MEDIA_ENT_F_PROC_VIDEO_STATISTICS);
The above line is aligned using tabs so that its end is as close to
the 80 character boundary as possible.
Best regards,
Tomasz
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox