* [PATCH 3/5] watchdog: sp805: set WDOG_HW_RUNNING when appropriate
From: Ray Jui @ 2018-05-22 23:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180522205457.GA16363@roeck-us.net>
Hi Guenter,
On 5/22/2018 1:54 PM, Guenter Roeck wrote:
> On Tue, May 22, 2018 at 11:47:18AM -0700, Ray Jui wrote:
>> If the watchdog hardware is already enabled during the boot process,
>> when the Linux watchdog driver loads, it should reset the watchdog and
>> tell the watchdog framework. As a result, ping can be generated from
>> the watchdog framework, until the userspace watchdog daemon takes over
>> control
>>
>> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>> ---
>> drivers/watchdog/sp805_wdt.c | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
>> index 1484609..408ffbe 100644
>> --- a/drivers/watchdog/sp805_wdt.c
>> +++ b/drivers/watchdog/sp805_wdt.c
>> @@ -42,6 +42,7 @@
>> /* control register masks */
>> #define INT_ENABLE (1 << 0)
>> #define RESET_ENABLE (1 << 1)
>> + #define ENABLE_MASK (INT_ENABLE | RESET_ENABLE)
>> #define WDTINTCLR 0x00C
>> #define WDTRIS 0x010
>> #define WDTMIS 0x014
>> @@ -74,6 +75,18 @@ module_param(nowayout, bool, 0);
>> MODULE_PARM_DESC(nowayout,
>> "Set to 1 to keep watchdog running after device release");
>>
>> +/* returns true if wdt is running; otherwise returns false */
>> +static bool wdt_is_running(struct watchdog_device *wdd)
>> +{
>> + struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
>> +
>> + if ((readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK) ==
>> + ENABLE_MASK)
>> + return true;
>> + else
>> + return false;
>
> return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
>
Note ENABLE_MASK contains two bits (INT_ENABLE and RESET_ENABLE);
therefore, a simple !!(expression) would not work? That is, the masked
result needs to be compared against the mask again to ensure both bits
are set, right?
Thanks,
Ray
^ permalink raw reply
* [RFC PATCH 09/10] arm64: dts: rk3399: Add dfi and dmc nodes.
From: Rob Herring @ 2018-05-22 22:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180514211610.26618-10-enric.balletbo@collabora.com>
On Mon, May 14, 2018 at 11:16:09PM +0200, Enric Balletbo i Serra wrote:
> From: Lin Huang <hl@rock-chips.com>
>
> These are required to support DDR DVFS on rk3399 platform. The patch also
> introduces two new files (rk3399-dram.h and rk3399-dram-default-timing)
> with default DRAM settings.
>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>
> .../rockchip/rk3399-dram-default-timing.dtsi | 38 ++++++++++
> arch/arm64/boot/dts/rockchip/rk3399-dram.h | 73 +++++++++++++++++++
> .../boot/dts/rockchip/rk3399-op1-opp.dtsi | 29 ++++++++
> arch/arm64/boot/dts/rockchip/rk3399.dtsi | 20 +++++
> 4 files changed, 160 insertions(+)
> create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-dram-default-timing.dtsi
> create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-dram.h
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-dram-default-timing.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-dram-default-timing.dtsi
> new file mode 100644
> index 000000000000..4dfe3e1d8bdf
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-dram-default-timing.dtsi
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR X11)
> +/*
> + * Copyright (c) 2016-2018, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * Author: Lin Huang <hl@rock-chips.com>
> + */
> +
> +#include "rk3399-dram.h"
> +
> +rockchip,ddr3_speed_bin = <21>;
> +rockchip,pd_idle = <0x40>;
> +rockchip,sr_idle = <0x2>;
Don't do includes this way please. These should go under a node.
> +rockchip,sr_mc_gate_idle = <0x3>;
> +rockchip,srpd_lite_idle = <0x4>;
> +rockchip,standby_idle = <0x2000>;
> +rockchip,dram_dll_dis_freq = <300000000>;
> +rockchip,phy_dll_dis_freq = <125000000>;
> +rockchip,auto_pd_dis_freq = <666000000>;
> +rockchip,ddr3_odt_dis_freq = <333000000>;
> +rockchip,ddr3_drv = <DDR3_DS_40ohm>;
> +rockchip,ddr3_odt = <DDR3_ODT_120ohm>;
> +rockchip,phy_ddr3_ca_drv = <PHY_DRV_ODT_40>;
> +rockchip,phy_ddr3_dq_drv = <PHY_DRV_ODT_40>;
> +rockchip,phy_ddr3_odt = <PHY_DRV_ODT_240>;
> +rockchip,lpddr3_odt_dis_freq = <333000000>;
> +rockchip,lpddr3_drv = <LP3_DS_34ohm>;
> +rockchip,lpddr3_odt = <LP3_ODT_240ohm>;
> +rockchip,phy_lpddr3_ca_drv = <PHY_DRV_ODT_40>;
> +rockchip,phy_lpddr3_dq_drv = <PHY_DRV_ODT_40>;
> +rockchip,phy_lpddr3_odt = <PHY_DRV_ODT_240>;
> +rockchip,lpddr4_odt_dis_freq = <333000000>;
> +rockchip,lpddr4_drv = <LP4_PDDS_60ohm>;
> +rockchip,lpddr4_dq_odt = <LP4_DQ_ODT_40ohm>;
> +rockchip,lpddr4_ca_odt = <LP4_CA_ODT_40ohm>;
> +rockchip,phy_lpddr4_ca_drv = <PHY_DRV_ODT_40>;
> +rockchip,phy_lpddr4_ck_cs_drv = <PHY_DRV_ODT_80>;
> +rockchip,phy_lpddr4_dq_drv = <PHY_DRV_ODT_80>;
> +rockchip,phy_lpddr4_odt = <PHY_DRV_ODT_60>;
^ permalink raw reply
* [PATCH v3 1/2] regulator: dt-bindings: add QCOM RPMh regulator bindings
From: David Collins @ 2018-05-22 22:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=XARtQNo5Cyg78XuT26JUFgn=7BjWRnXPjP566=a-sF1w@mail.gmail.com>
On 05/22/2018 09:43 AM, Doug Anderson wrote:
> On Mon, May 21, 2018 at 5:00 PM, David Collins <collinsd@codeaurora.org> wrote:
...
>> Returning the cached (but not sent) initial voltage equal to the min
>> constraint voltage in get_voltage() calls should not cause any problems.
>> This represents the highest voltage that is guaranteed to be output by the
>> regulator. Consumer's should call regulator_set_voltage() to specify
>> their voltage needs. If they simply call regulator_enable(), then the
>> cached voltage will be sent along with the enable request.
>
> I'm still not seeing the argument for initial-voltage here. If we
> added a feature like you describe where we don't send the voltage
> until the first enable couldn't we use the minimum voltage here? If a
> consumer calls regulator_enable() without setting a voltage (which
> seems like a terrible idea for something where the voltage could vary)
> then it would end up at the minimum voltage.
I wasn't proposing the voltage caching feature to be used in the upstream
qcom-rpmh-regulator. I was explaining exactly how the downstream
rpmh-regulator driver works.
However, if the voltage caching feature is acceptable for upstream usage,
then I could add it. With that in place, I see less of a need for the
qcom,regulator-initial-microvolt property and would be ok with removing it
for now.
>>> BTW: have I already said how terrible of a design it is that you can't
>>> read back the voltages that the BIOS set? If we could just read back
>>> what the BIOS set then everything would work great and we could stop
>>> talking about this.
>>
>> Even if such reading were feasible, it would not help the situation
>> substantially. Very few requests are made via the AP RSC before Linux
>> kernel boot, so 0 V values would still be read back for most regulators.
>
> Sure, but all the regulators we're talking about are ones where this
> would help. Said another way: are there any rails that are not
> touched by the bootloader where it's bad to set the minimum voltage?
I'm not sure about this.
> OK, so how's this for a proposal then:
>
> 1. For RPMh-regulator whenever we see a "set voltage" but Linux hasn't
> specified that the regulator is enabled then we don't send the
> voltage, we just cache it.
>
> 2. When we see the first enable then we first send the cached voltage
> and then do the enable.
>
> 3. We don't need an "initial voltage" because any rails that are
> expected to be variable voltage the client should be choosing a
> voltage.
>
>
> ...taking the SD card case as an example: if the regulator framework
> says "set to the minimum: 1.8V" we'll cache this but not apply it yet
> because the rail is off as far as Linux is concerned. Then when the
> SD card framework starts up it will set the voltage to 3.3V which will
> overwrite the cache. Then the SD card framework will enable the
> regulator and RPMh will set the voltage to 3.3V and enable.
I am ok with implementing this feature.
However, should the voltage be cached instead of sent immediately any time
that Linux has not explicitly requested the regulator to be enabled, or
only before the first time that an enable request is sent?
1. regulator_set_voltage(reg, 2960000, 2960000)
--> cache voltage=2960 mV
2. regulator_enable(reg)
--> Send voltage=2960 then enable=1
3. regulator_disable(reg)
--> Send enable=0
4. regulator_set_voltage(reg, 1800000, 2960000)
--> A. Send voltage=1800 OR B. cache voltage=1800?
Option A is used on the downstream rpmh-regulator driver in order to avoid
cases where AP votes to disable a regulator that is kept enabled by
another subsystem but then does not lower the voltage requested thanks to
regulator_set_voltage() being called after regulator_disable(). I plan to
go with option A for qcom-rpmh-regulator unless there are objections.
> This whole thing makes me worry about another problem, though. You
> say that the bootloader left the SD card rail on, right? ...but as
> far as Linux is concerned the SD card rail is off (argh, it can't read
> the state that the bootloader left the rail in!)
>
> ...now imagine any of the following:
>
> * The user boots up a kernel where the SD card driver is disabled.
>
> * The user ejects the SD card right after the bootloader used it but
> before the kernel driver started.
>
> When the kernel comes up it will believe that the SD card rail is
> disabled so it won't try to disable it. So the voltage will be left
> on.
We have not encountered issues with regulators getting left on
indefinitely because Linux devices failed to take control of them during
boot. I don't think that this hypothetical issue needs to be addressed in
the first qcom-rpmh-regulator driver patch if at all.
> You can even come up with a less contrived use case here. One could
> argue that the SD card framework really ought to be ensuring VMMC and
> VQMMC are off before it starts tweaking with them just in case the
> bootloader left them on. Thus, it should do:
>
> A) Turn off VMMC and VQMMC
> B) Program VMMC and VQMMC to defaults
> C) Turn on VMMC and VQMMC
>
> ...right now we bootup and pretend to Linux that VMMC and VQMMC start
> off, so step A) will be no-op. Sigh.
Step A) would not work because the regulator's use_count would be 0 and
regulator_disable() can only be called successfully if use_count > 0. The
call would have no impact and it would return an error.
I don't think that this is an avenue that we want to pursue. Consumers
should not be expected to call regulator_disable() before regulator_enable().
> Do we need to have ".is_enabled" return -ENOTRECOVERABLE to help the
> regulator framework understand that we don't know the state? I think
> that might require a pile of patches to the regulator framework, but
> it can't be helped unless we can somehow get RPMh to give us back the
> status of how the bootloader left us (if we had that, we could return
> 0 for anything the bootloader didn't touch and that would be correct
> from the point of view of the AP).
I'm not following what the regulator framework would do as a result of
is_enabled() returning -ENOTRECOVERABLE. If it saw this while processing
a regulator_enable() call then it would continue to enable the regulator.
This value could not be seen while handling a regulator_disable() call
since the is_enabled() callback is not invoked in the disable call-path.
This also seems like an optimization for a problem that we are not
encountering now (or likely to ever encounter). Therefore, I would
suggest that we not try to work this into the initial qcom-rpmh-regulator
patch.
Thanks,
David
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [RFC PATCH 02/10] dt-bindings: devfreq: rk3399_dmc: Add rockchip,pmu phandle.
From: Rob Herring @ 2018-05-22 22:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180514211610.26618-3-enric.balletbo@collabora.com>
On Mon, May 14, 2018 at 11:16:02PM +0200, Enric Balletbo i Serra wrote:
> The Rockchip DMC (Dynamic Memory Interface) needs to access to the PMU
> general register files to know the DRAM type, so add a phandle to the
> syscon that manages these registers.
>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>
> Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt | 2 ++
> 1 file changed, 2 insertions(+)
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH 1/2] ARM: dts: imx51-babbage: Fix USB PHY duplicate unit-address
From: Rob Herring @ 2018-05-22 22:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180515080624.GR26863@dragon>
On Tue, May 15, 2018 at 04:06:26PM +0800, Shawn Guo wrote:
> On Mon, May 14, 2018 at 03:29:35PM -0300, Fabio Estevam wrote:
> > From: Fabio Estevam <fabio.estevam@nxp.com>
> >
> > Currently the following DTC warning is seen with W=1:
> >
> > arch/arm/boot/dts/imx51-babbage.dtb: Warning (unique_unit_address): /usbphy/usbphy at 0: duplicate unit-address (also used in node /usbphy/usbh1phy at 0)
> >
> > Fix it by moving the USB PHY node outside of simple-bus and drop the
> > unneeded unit-address.
> >
> > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> > ---
> > arch/arm/boot/dts/imx51-babbage.dts | 21 +++++++--------------
> > 1 file changed, 7 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/imx51-babbage.dts b/arch/arm/boot/dts/imx51-babbage.dts
> > index b8ca73d..de46906 100644
> > --- a/arch/arm/boot/dts/imx51-babbage.dts
> > +++ b/arch/arm/boot/dts/imx51-babbage.dts
> > @@ -170,20 +170,13 @@
> > mux-ext-port = <3>;
> > };
> >
> > - usbphy {
> > - #address-cells = <1>;
> > - #size-cells = <0>;
> > - compatible = "simple-bus";
> > -
> > - usbh1phy: usbh1phy at 0 {
> > - compatible = "usb-nop-xceiv";
> > - reg = <0>;
> > - clocks = <&clk_usb>;
> > - clock-names = "main_clk";
> > - reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
> > - vcc-supply = <&vusb_reg>;
> > - #phy-cells = <0>;
> > - };
> > + usbh1phy: usbphy1 {
> > + compatible = "usb-nop-xceiv";
> > + clocks = <&clk_usb>;
> > + clock-names = "main_clk";
> > + reset-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
> > + vcc-supply = <&vusb_reg>;
> > + #phy-cells = <0>;
>
> This should be considered as a whole together with usbphy in imx51.dtsi.
> Also, I would like to get some input from DT folks on how we should name
> the node uniquely. @Rob.
'usbphy1' is fine. I don't have a better suggestion...
Rob
^ permalink raw reply
* [PATCH 2/2] soc: bcm: brcmstb: Add missing DDR MEMC compatible strings
From: Rob Herring @ 2018-05-22 22:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511220242.837-3-f.fainelli@gmail.com>
On Fri, May 11, 2018 at 03:02:42PM -0700, Florian Fainelli wrote:
> We would not be matching the following chip/compatible strings
> combinations, which would lead to not setting the warm boot flag
> correctly, fix that:
>
> 7260A0/B0: brcm,brcmstb-memc-ddr-rev-b.2.1
> 7255A0: brcm,brcmstb-memc-ddr-rev-b.2.3
> 7278Bx: brcm,brcmstb-memc-ddr-rev-b.3.1
>
> The B2.1 core (which is in 7260 A0 and B0) doesn't have the
> SHIMPHY_ADDR_CNTL_0_DDR_PAD_CNTRL setup in the memsys init code, nor
> does it have the warm boot flag re-definition on entry. Those changes
> were for B2.2 and later MEMSYS cores. Fall back to the previous S2/S3
> entry method for these specific chips.
>
> Fixes: 0b741b8234c8 ("soc: bcm: brcmstb: Add support for S2/S3/S5 suspend states (ARM)")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt | 3 +++
> drivers/soc/bcm/brcmstb/pm/pm-arm.c | 12 ++++++++++++
> 2 files changed, 15 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH 1/2] soc: bcm: brcmstb: pm: Add support for newer rev B3.0 controllers
From: Rob Herring @ 2018-05-22 22:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180511220242.837-2-f.fainelli@gmail.com>
On Fri, May 11, 2018 at 03:02:41PM -0700, Florian Fainelli wrote:
> From: Doug Berger <opendmb@gmail.com>
>
> Update the Device Tree binding document and add a matching entry for the
> MEMC DDR controller revision B3.0 which is found on chips like 7278A0
> and newer.
>
> Signed-off-by: Doug Berger <opendmb@gmail.com>
> [florian: tweak commit message, make it apply to upstream kernel]
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> Documentation/devicetree/bindings/arm/bcm/brcm,brcmstb.txt | 1 +
> drivers/soc/bcm/brcmstb/pm/pm-arm.c | 4 ++++
> 2 files changed, 5 insertions(+)
Reviewed-by: Rob Herring <robh@kernel.org>
Side note: this should really move out of bindings/arm/ to
bindings/memory-controllers/ or at least to its own file.
Rob
^ permalink raw reply
* [PATCH 6/9] ARM: dts: wheat: Drop MTD partitioning from DT
From: Marek Vasut @ 2018-05-22 22:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdXRnoyhNz=3v_=ZCV=FWX4TwKqx7MpH0g3c7GwtXKP1tg@mail.gmail.com>
On 05/22/2018 04:43 PM, Geert Uytterhoeven wrote:
> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> Drop the MTD partitioning from DT, since it does not describe HW
>> and to give way to a more flexible kernel command line partition
>> passing.
>>
>> To retain the original partitioning, assure you have enabled
>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
>> following to your kernel command line:
>>
>> mtdparts=spi0.0:256k at 0(loader),4096k(user),-(flash)
>
> I think the "@0" can be dropped, as it's optional?
> 4m?
My take on this is that the loader is actually at offset 0x0 of the MTD
device and we explicitly state that in the mtdparts to anchor the first
partition within the MTD device and all the other partitions are at
offset +(sum of the sizes of all partitions listed before the current
one) relative to that first partition.
Removing the @0 feels fragile at best and it seems to depend on the
current behavior of the code.
> (Gaining more knowledge during reviewing ;-)
>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>
> Gr{oetje,eeting}s,
>
> Geert
>
--
Best regards,
Marek Vasut
^ permalink raw reply
* [PATCH] arm64: vdso32: Use full path to Clang instead of relying on PATH
From: Nathan Chancellor @ 2018-05-22 21:59 UTC (permalink / raw)
To: linux-arm-kernel
Currently, in order to build the compat VDSO with Clang, this format
has to be used:
PATH=${BIN_FOLDER}:${PATH} make CC=clang
Prior to the addition of this file, this format would also be
acceptable:
make CC=${BIN_FOLDER}/clang
This is because the vdso32 Makefile uses cc-name instead of CC. After
this path, CC will still evaluate to clang for the first case as
expected but now the second case will use the specified Clang, rather
than the host's copy, which may not be compatible as shown below.
/usr/bin/as: unrecognized option '-mfloat-abi=soft'
clang-6.0: error: assembler command failed with exit code 1
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
Hi everyone,
I noticed this issue when building the Pixel 2's kernel. Since this
is a supplement to https://patchwork.kernel.org/patch/10186671/, I
was instructed by Mark to push it here. I could not find a public git
tree for this patch, I am not sure if it has been applied or not so I
couldn't add a fixes tag. If there are any other issues, please let me
know!
Nathan
arch/arm64/kernel/vdso32/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/kernel/vdso32/Makefile b/arch/arm64/kernel/vdso32/Makefile
index 6d44d972e89d..837e877a326b 100644
--- a/arch/arm64/kernel/vdso32/Makefile
+++ b/arch/arm64/kernel/vdso32/Makefile
@@ -5,7 +5,7 @@
# A mix between the arm64 and arm vDSO Makefiles.
ifeq ($(cc-name),clang)
- CC_ARM32 := $(cc-name) $(CLANG_TARGET_ARM32) -no-integrated-as
+ CC_ARM32 := $(CC) $(CLANG_TARGET_ARM32) -no-integrated-as
else
CC_ARM32 := $(CROSS_COMPILE_ARM32)$(cc-name)
endif
--
2.17.0
^ permalink raw reply related
* v4.17-rc1: regressions on N900, N950
From: Aaro Koskinen @ 2018-05-22 21:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180522205824.GA24927@amd>
Hi,
On Tue, May 22, 2018 at 10:58:26PM +0200, Pavel Machek wrote:
> On Tue 2018-05-22 22:41:39, Aaro Koskinen wrote:
> > My device worked with v4.17-rc1 (haven't found time to test newer kernels),
> > but if you say the probe order is random then we must find some proper way
> > to express the dependency.
>
> I started bisect, but.. that will probably not be useful.
>
> If your device works ok in v4.17-rc1, it probably works in newer -rcs,
> too.
Actually, my statement may be bogus... Now I tried again with -rc1
(and also -rc6) and it fails... But v4.16 works.
> Thanks for the ordering hint, I'll try to figure out what is going on
> there.
My bisection pointed to 6fa7324ac5489ad43c4b6351355b869bc5458bef which
doesn't seem to make any sense...?! So maybe there really is something
random stuff going on? :-(
A.
^ permalink raw reply
* [PATCH 2/9] ARM: dts: lager: Drop MTD partitioning from DT
From: Marek Vasut @ 2018-05-22 21:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAMuHMdV1TFH_QwTv4fsOGnUOCNftC9369_1AQmZDK+NBLDmw-Q@mail.gmail.com>
On 05/22/2018 04:32 PM, Geert Uytterhoeven wrote:
> On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> Drop the MTD partitioning from DT, since it does not describe HW
>> and to give way to a more flexible kernel command line partition
>> passing.
>>
>> To retain the original partitioning, assure you have enabled
>> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
>> following to your kernel command line:
>>
>> mtdparts=spi0.0:256k at 0(loader),4096k(user),-(flash)
>
> I guess s/4096k/4m/ works, too?
Yes, but all the other boards use nnn k, so let's stick with k . I don't
want one value to stick out like a sore thumb.
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Gr{oetje,eeting}s,
>
> Geert
>
--
Best regards,
Marek Vasut
^ permalink raw reply
* [PATCH 6/6] coresight: allow to build as modules
From: Mathieu Poirier @ 2018-05-22 21:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180518012024.22645-6-kim.phillips@arm.com>
On Thu, May 17, 2018 at 08:20:24PM -0500, Kim Phillips wrote:
> Allow to build coresight as modules. This greatly enhances developer
> efficiency by allowing the development to take place exclusively on the
> target, and without needing to reboot in between changes.
>
> - Kconfig bools become tristates, to allow =m
>
> - use -objs to denote merge object directives in Makefile, adds a
> coresight-core nomenclature for the base module.
>
> - Export core functions so as to be able to be used by
> non-core modules.
>
> - add a coresight_exit() that unregisters the coresight bus, add
> remove fns for most others.
>
> - fix up modules with ID tables for autoloading on boot
>
> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Kim Phillips <kim.phillips@arm.com>
> ---
> drivers/hwtracing/coresight/Kconfig | 48 +++++++++++++++----
> drivers/hwtracing/coresight/Makefile | 28 +++++++----
> .../hwtracing/coresight/coresight-cpu-debug.c | 2 +
> .../coresight/coresight-dynamic-replicator.c | 26 ++++++++--
> drivers/hwtracing/coresight/coresight-etb10.c | 27 +++++++++--
> .../hwtracing/coresight/coresight-etm-perf.c | 9 +++-
> .../coresight/coresight-etm3x-sysfs.c | 1 +
> drivers/hwtracing/coresight/coresight-etm3x.c | 32 +++++++++++--
> .../coresight/coresight-etm4x-sysfs.c | 1 +
> drivers/hwtracing/coresight/coresight-etm4x.c | 33 +++++++++++--
> .../hwtracing/coresight/coresight-funnel.c | 26 ++++++++--
> drivers/hwtracing/coresight/coresight-priv.h | 1 -
> .../coresight/coresight-replicator.c | 28 +++++++++--
> drivers/hwtracing/coresight/coresight-stm.c | 23 ++++++++-
> drivers/hwtracing/coresight/coresight-tmc.c | 18 ++++++-
> drivers/hwtracing/coresight/coresight-tpiu.c | 26 ++++++++--
> drivers/hwtracing/coresight/coresight.c | 14 ++++++
> 17 files changed, 299 insertions(+), 44 deletions(-)
For the next revision please split the work based on files.
>
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index f9abdef5b0d9..4512885f7a3e 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -2,7 +2,7 @@
> # Coresight configuration
> #
> menuconfig CORESIGHT
> - bool "CoreSight Tracing Support"
> + tristate "CoreSight Tracing Support"
> select ARM_AMBA
> select PERF_EVENTS
> help
> @@ -12,17 +12,23 @@ menuconfig CORESIGHT
> specification and configure the right series of components when a
> trace source gets enabled.
>
> + To compile this code as a module, choose M here: the
> + module will be called coresight-core.
> +
> if CORESIGHT
> config CORESIGHT_LINKS_AND_SINKS
> - bool "CoreSight Link and Sink drivers"
> + tristate "CoreSight Link and Sink drivers"
> help
> This enables support for CoreSight link and sink drivers that are
> responsible for transporting and collecting the trace data
> respectively. Link and sinks are dynamically aggregated with a trace
> entity at run time to form a complete trace path.
>
> + To compile this code as modules, choose M here: the
> + modules will be called coresight-funnel and coresight-replicator.
> +
> config CORESIGHT_LINK_AND_SINK_TMC
> - bool "Coresight generic TMC driver"
> + tristate "Coresight generic TMC driver"
> help
> This enables support for the Trace Memory Controller driver.
> Depending on its configuration the device can act as a link (embedded
> @@ -30,8 +36,11 @@ config CORESIGHT_LINK_AND_SINK_TMC
> complies with the generic implementation of the component without
> special enhancement or added features.
>
> + To compile this code as a module, choose M here: the
> + module will be called coresight-tmc-core.
> +
> config CORESIGHT_SINK_TPIU
> - bool "Coresight generic TPIU driver"
> + tristate "Coresight generic TPIU driver"
> help
> This enables support for the Trace Port Interface Unit driver,
> responsible for bridging the gap between the on-chip coresight
> @@ -40,15 +49,21 @@ config CORESIGHT_SINK_TPIU
> connected to an external host for use case capturing more traces than
> the on-board coresight memory can handle.
>
> + To compile this code as a module, choose M here: the
> + module will be called coresight-tpiu.
> +
> config CORESIGHT_SINK_ETBV10
> - bool "Coresight ETBv1.0 driver"
> + tristate "Coresight ETBv1.0 driver"
> help
> This enables support for the Embedded Trace Buffer version 1.0 driver
> that complies with the generic implementation of the component without
> special enhancement or added features.
>
> + To compile this code as a module, choose M here: the
> + module will be called coresight-etb10.
> +
> config CORESIGHT_SOURCE_ETM3X
> - bool "CoreSight Embedded Trace Macrocell 3.x driver"
> + tristate "CoreSight Embedded Trace Macrocell 3.x driver"
> depends on !ARM64
> help
> This driver provides support for processor ETM3.x and PTM1.x modules,
> @@ -56,8 +71,11 @@ config CORESIGHT_SOURCE_ETM3X
> This is primarily useful for instruction level tracing. Depending
> the ETM version data tracing may also be available.
>
> + To compile this code as a module, choose M here: the
> + module will be called coresight-etm3x-core.
> +
> config CORESIGHT_SOURCE_ETM4X
> - bool "CoreSight Embedded Trace Macrocell 4.x driver"
> + tristate "CoreSight Embedded Trace Macrocell 4.x driver"
> depends on ARM64
> help
> This driver provides support for the ETM4.x tracer module, tracing the
> @@ -65,15 +83,21 @@ config CORESIGHT_SOURCE_ETM4X
> for instruction level tracing. Depending on the implemented version
> data tracing may also be available.
>
> + To compile this code as a module, choose M here: the
> + module will be called coresight-etm4x-core.
> +
> config CORESIGHT_DYNAMIC_REPLICATOR
> - bool "CoreSight Programmable Replicator driver"
> + tristate "CoreSight Programmable Replicator driver"
> help
> This enables support for dynamic CoreSight replicator link driver.
> The programmable ATB replicator allows independent filtering of the
> trace data based on the traceid.
>
> + To compile this code as a module, choose M here: the
> + module will be called coresight-dynamic-replicator.
> +
> config CORESIGHT_STM
> - bool "CoreSight System Trace Macrocell driver"
> + tristate "CoreSight System Trace Macrocell driver"
> depends on STM && ((ARM && !(CPU_32v3 || CPU_32v4 || CPU_32v4T)) || ARM64)
> help
> This driver provides support for hardware assisted software
> @@ -81,6 +105,9 @@ config CORESIGHT_STM
> logging useful software events or data coming from various entities
> in the system, possibly running different OSs
>
> + To compile this code as a module, choose M here: the
> + module will be called coresight-stm.
> +
> config CORESIGHT_CPU_DEBUG
> tristate "CoreSight CPU Debug driver"
> depends on ARM || ARM64
> @@ -95,4 +122,7 @@ config CORESIGHT_CPU_DEBUG
> properly, please refer Documentation/trace/coresight-cpu-debug.txt
> for detailed description and the example for usage.
>
> + To compile this code as a module, choose M here: the
> + module will be called coresight-cpu-debug.
> +
> endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 61db9dd0d571..5990710289c2 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -2,19 +2,29 @@
> #
> # Makefile for CoreSight drivers.
> #
> -obj-$(CONFIG_CORESIGHT) += coresight.o coresight-etm-perf.o
> -obj-$(CONFIG_OF) += of_coresight.o
> -obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc.o \
> - coresight-tmc-etf.o \
> - coresight-tmc-etr.o
> +obj-$(CONFIG_CORESIGHT) += coresight-core.o
> +coresight-core-objs := coresight.o \
> + of_coresight.o
> +
> +obj-$(CONFIG_CORESIGHT) += coresight-etm-perf.o
> +
> +obj-$(CONFIG_CORESIGHT_LINK_AND_SINK_TMC) += coresight-tmc-core.o
> +coresight-tmc-core-objs := coresight-tmc.o \
> + coresight-tmc-etf.o \
> + coresight-tmc-etr.o
> obj-$(CONFIG_CORESIGHT_SINK_TPIU) += coresight-tpiu.o
> obj-$(CONFIG_CORESIGHT_SINK_ETBV10) += coresight-etb10.o
> obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \
> coresight-replicator.o
> -obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o coresight-etm-cp14.o \
> - coresight-etm3x-sysfs.o
> -obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o \
> - coresight-etm4x-sysfs.o
> +
> +obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x-core.o
> +coresight-etm3x-core-objs := coresight-etm3x.o \
> + coresight-etm-cp14.o \
> + coresight-etm3x-sysfs.o
> +
> +obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x-core.o
> +coresight-etm4x-core-objs := coresight-etm4x.o coresight-etm4x-sysfs.o
> +
> obj-$(CONFIG_CORESIGHT_DYNAMIC_REPLICATOR) += coresight-dynamic-replicator.o
> obj-$(CONFIG_CORESIGHT_STM) += coresight-stm.o
> obj-$(CONFIG_CORESIGHT_CPU_DEBUG) += coresight-cpu-debug.o
> diff --git a/drivers/hwtracing/coresight/coresight-cpu-debug.c b/drivers/hwtracing/coresight/coresight-cpu-debug.c
> index 45b2460f3166..1efe9626eb6c 100644
> --- a/drivers/hwtracing/coresight/coresight-cpu-debug.c
> +++ b/drivers/hwtracing/coresight/coresight-cpu-debug.c
> @@ -671,6 +671,8 @@ static const struct amba_id debug_ids[] = {
> { 0, 0 },
> };
>
> +MODULE_DEVICE_TABLE(amba, debug_ids);
> +
> static struct amba_driver debug_driver = {
> .drv = {
> .name = "coresight-cpu-debug",
> diff --git a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
> index fc742215ab05..bc42b8022556 100644
> --- a/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-dynamic-replicator.c
> @@ -37,7 +37,12 @@ struct replicator_state {
> static int replicator_enable(struct coresight_device *csdev, int inport,
> int outport)
> {
> - struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct replicator_state *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
> +
> + if (!try_module_get(module))
> + return -ENODEV;
>
> CS_UNLOCK(drvdata->base);
>
> @@ -63,7 +68,9 @@ static int replicator_enable(struct coresight_device *csdev, int inport,
> static void replicator_disable(struct coresight_device *csdev, int inport,
> int outport)
> {
> - struct replicator_state *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct replicator_state *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
>
> CS_UNLOCK(drvdata->base);
>
> @@ -75,6 +82,7 @@ static void replicator_disable(struct coresight_device *csdev, int inport,
>
> CS_LOCK(drvdata->base);
>
> + module_put(module);
> dev_info(drvdata->dev, "REPLICATOR disabled\n");
> }
>
> @@ -159,6 +167,15 @@ static int replicator_probe(struct amba_device *adev, const struct amba_id *id)
> return PTR_ERR_OR_ZERO(drvdata->csdev);
> }
>
> +static int __exit replicator_remove(struct amba_device *adev)
> +{
> + struct replicator_state *drvdata = dev_get_drvdata(&adev->dev);
> +
> + coresight_unregister(drvdata->csdev);
> +
> + return 0;
> +}
> +
> #ifdef CONFIG_PM
> static int replicator_runtime_suspend(struct device *dev)
> {
> @@ -200,6 +217,8 @@ static const struct amba_id replicator_ids[] = {
> { 0, 0 },
> };
>
> +MODULE_DEVICE_TABLE(amba, replicator_ids);
> +
> static struct amba_driver replicator_driver = {
> .drv = {
> .name = "coresight-dynamic-replicator",
> @@ -207,9 +226,10 @@ static struct amba_driver replicator_driver = {
> .suppress_bind_attrs = true,
> },
> .probe = replicator_probe,
> + .remove = replicator_remove,
> .id_table = replicator_ids,
> };
> -builtin_amba_driver(replicator_driver);
> +module_amba_driver(replicator_driver);
>
> MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
> MODULE_DESCRIPTION("ARM Coresight Dynamic Replicator Driver");
> diff --git a/drivers/hwtracing/coresight/coresight-etb10.c b/drivers/hwtracing/coresight/coresight-etb10.c
> index a3dac5a8b37c..8825a3e4e47a 100644
> --- a/drivers/hwtracing/coresight/coresight-etb10.c
> +++ b/drivers/hwtracing/coresight/coresight-etb10.c
> @@ -135,7 +135,12 @@ static int etb_enable(struct coresight_device *csdev, u32 mode)
> {
> u32 val;
> unsigned long flags;
> - struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct etb_drvdata *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
> +
> + if (!try_module_get(module))
> + return -ENODEV;
>
> val = local_cmpxchg(&drvdata->mode,
> CS_MODE_DISABLED, mode);
> @@ -256,7 +261,9 @@ static void etb_dump_hw(struct etb_drvdata *drvdata)
>
> static void etb_disable(struct coresight_device *csdev)
> {
> - struct etb_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct etb_drvdata *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
> unsigned long flags;
>
> spin_lock_irqsave(&drvdata->spinlock, flags);
> @@ -266,6 +273,7 @@ static void etb_disable(struct coresight_device *csdev)
>
> local_set(&drvdata->mode, CS_MODE_DISABLED);
>
> + module_put(module);
> dev_info(drvdata->dev, "ETB disabled\n");
> }
>
> @@ -712,6 +720,16 @@ static int etb_probe(struct amba_device *adev, const struct amba_id *id)
> return ret;
> }
>
> +static int __exit etb_remove(struct amba_device *adev)
> +{
> + struct etb_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> + misc_deregister(&drvdata->miscdev);
> + coresight_unregister(drvdata->csdev);
> +
> + return 0;
> +}
> +
> #ifdef CONFIG_PM
> static int etb_runtime_suspend(struct device *dev)
> {
> @@ -746,6 +764,8 @@ static const struct amba_id etb_ids[] = {
> { 0, 0},
> };
>
> +MODULE_DEVICE_TABLE(amba, etb_ids);
> +
> static struct amba_driver etb_driver = {
> .drv = {
> .name = "coresight-etb10",
> @@ -755,9 +775,10 @@ static struct amba_driver etb_driver = {
>
> },
> .probe = etb_probe,
> + .remove = etb_remove,
> .id_table = etb_ids,
> };
> -builtin_amba_driver(etb_driver);
> +module_amba_driver(etb_driver);
>
> MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
> MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c
> index ad0ef8d27111..feb287083ba5 100644
> --- a/drivers/hwtracing/coresight/coresight-etm-perf.c
> +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c
> @@ -466,6 +466,7 @@ int etm_perf_symlink(struct coresight_device *csdev, bool link)
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(etm_perf_symlink);
>
> static int __init etm_perf_init(void)
> {
> @@ -493,7 +494,13 @@ static int __init etm_perf_init(void)
>
> return ret;
> }
> -device_initcall(etm_perf_init);
> +module_init(etm_perf_init);
> +
> +static void __exit etm_perf_exit(void)
> +{
> + perf_pmu_unregister(&etm_pmu);
> +}
> +module_exit(etm_perf_exit);
>
> MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> MODULE_DESCRIPTION("Arm CoreSight tracer perf driver");
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> index 91a2a23143d8..84fa5e0fe07b 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> @@ -7,6 +7,7 @@
> #include <linux/pid_namespace.h>
> #include <linux/pm_runtime.h>
> #include <linux/sysfs.h>
> +#include <linux/coresight.h>
Why do we need this?
> #include "coresight-etm.h"
> #include "coresight-priv.h"
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x.c b/drivers/hwtracing/coresight/coresight-etm3x.c
> index 7ca73a15c735..a2357b26b3a2 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x.c
> @@ -514,7 +514,12 @@ static int etm_enable(struct coresight_device *csdev,
> {
> int ret;
> u32 val;
> - struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct etm_drvdata *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
> +
> + if (!try_module_get(module))
> + return -ENODEV;
>
> val = local_cmpxchg(&drvdata->mode, CS_MODE_DISABLED, mode);
>
> @@ -611,7 +616,9 @@ static void etm_disable(struct coresight_device *csdev,
> struct perf_event *event)
> {
> u32 mode;
> - struct etm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct etm_drvdata *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
>
> /*
> * For as long as the tracer isn't disabled another entity can't
> @@ -636,6 +643,8 @@ static void etm_disable(struct coresight_device *csdev,
>
> if (mode)
> local_set(&drvdata->mode, CS_MODE_DISABLED);
> +
> + module_put(module);
> }
>
> static const struct coresight_ops_source etm_source_ops = {
> @@ -864,6 +873,20 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
> return ret;
> }
>
> +static int __exit etm_remove(struct amba_device *adev)
> +{
> + struct etm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> + etm_perf_symlink(drvdata->csdev, false);
> +
> + cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
> + cpuhp_remove_state_nocalls(hp_online);
> +
> + coresight_unregister(drvdata->csdev);
> +
> + return 0;
> +}
> +
> #ifdef CONFIG_PM
> static int etm_runtime_suspend(struct device *dev)
> {
> @@ -924,6 +947,8 @@ static const struct amba_id etm_ids[] = {
> { 0, 0},
> };
>
> +MODULE_DEVICE_TABLE(amba, etm_ids);
> +
> static struct amba_driver etm_driver = {
> .drv = {
> .name = "coresight-etm3x",
> @@ -932,9 +957,10 @@ static struct amba_driver etm_driver = {
> .suppress_bind_attrs = true,
> },
> .probe = etm_probe,
> + .remove = etm_remove,
> .id_table = etm_ids,
> };
> -builtin_amba_driver(etm_driver);
> +module_amba_driver(etm_driver);
>
> MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
> MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> index 577a38673444..ee0cbada45d6 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x-sysfs.c
> @@ -2173,6 +2173,7 @@ const struct attribute_group *coresight_etmv4_groups[] = {
> &coresight_etmv4_trcidr_group,
> NULL,
> };
> +EXPORT_SYMBOL_GPL(coresight_etmv4_groups);
>From where I stand this is not needed.
>
> MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> MODULE_DESCRIPTION("Arm CoreSight Program Flow Trace v4 sysfs driver");
> diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c b/drivers/hwtracing/coresight/coresight-etm4x.c
> index ba10f5302a55..a6ff152ab61d 100644
> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> @@ -280,7 +280,12 @@ static int etm4_enable(struct coresight_device *csdev,
> {
> int ret;
> u32 val;
> - struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct etmv4_drvdata *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
> +
> + if (!try_module_get(module))
> + return -ENODEV;
>
> val = local_cmpxchg(&drvdata->mode, CS_MODE_DISABLED, mode);
>
> @@ -387,7 +392,9 @@ static void etm4_disable(struct coresight_device *csdev,
> struct perf_event *event)
> {
> u32 mode;
> - struct etmv4_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct etmv4_drvdata *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
>
> /*
> * For as long as the tracer isn't disabled another entity can't
> @@ -409,6 +416,8 @@ static void etm4_disable(struct coresight_device *csdev,
>
> if (mode)
> local_set(&drvdata->mode, CS_MODE_DISABLED);
> +
> + module_put(module);
> }
>
> static const struct coresight_ops_source etm4_source_ops = {
> @@ -1045,6 +1054,20 @@ static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
> return ret;
> }
>
> +static int __exit etm4_remove(struct amba_device *adev)
> +{
> + struct etmv4_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> + etm_perf_symlink(drvdata->csdev, false);
> +
> + cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING);
> + cpuhp_remove_state_nocalls(hp_online);
> +
> + coresight_unregister(drvdata->csdev);
> +
> + return 0;
> +}
> +
> static const struct amba_id etm4_ids[] = {
> { /* ETM 4.0 - Cortex-A53 */
> .id = 0x000bb95d,
> @@ -1064,15 +1087,19 @@ static const struct amba_id etm4_ids[] = {
> { 0, 0},
> };
>
> +MODULE_DEVICE_TABLE(amba, etm4_ids);
> +
> static struct amba_driver etm4x_driver = {
> .drv = {
> .name = "coresight-etm4x",
> + .owner = THIS_MODULE,
> .suppress_bind_attrs = true,
> },
> .probe = etm4_probe,
> + .remove = etm4_remove,
> .id_table = etm4_ids,
> };
> -builtin_amba_driver(etm4x_driver);
> +module_amba_driver(etm4x_driver);
>
> MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
> MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> diff --git a/drivers/hwtracing/coresight/coresight-funnel.c b/drivers/hwtracing/coresight/coresight-funnel.c
> index 1e497a75b956..c355a66bcc51 100644
> --- a/drivers/hwtracing/coresight/coresight-funnel.c
> +++ b/drivers/hwtracing/coresight/coresight-funnel.c
> @@ -61,7 +61,12 @@ static void funnel_enable_hw(struct funnel_drvdata *drvdata, int port)
> static int funnel_enable(struct coresight_device *csdev, int inport,
> int outport)
> {
> - struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct funnel_drvdata *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
> +
> + if (!try_module_get(module))
> + return -ENODEV;
>
> funnel_enable_hw(drvdata, inport);
>
> @@ -85,10 +90,13 @@ static void funnel_disable_hw(struct funnel_drvdata *drvdata, int inport)
> static void funnel_disable(struct coresight_device *csdev, int inport,
> int outport)
> {
> - struct funnel_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct funnel_drvdata *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
>
> funnel_disable_hw(drvdata, inport);
>
> + module_put(module);
> dev_info(drvdata->dev, "FUNNEL inport %d disabled\n", inport);
> }
>
> @@ -211,6 +219,15 @@ static int funnel_probe(struct amba_device *adev, const struct amba_id *id)
> return PTR_ERR_OR_ZERO(drvdata->csdev);
> }
>
> +static int __exit funnel_remove(struct amba_device *adev)
> +{
> + struct funnel_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> + coresight_unregister(drvdata->csdev);
> +
> + return 0;
> +}
> +
> #ifdef CONFIG_PM
> static int funnel_runtime_suspend(struct device *dev)
> {
> @@ -250,6 +267,8 @@ static const struct amba_id funnel_ids[] = {
> { 0, 0},
> };
>
> +MODULE_DEVICE_TABLE(amba, funnel_ids);
> +
> static struct amba_driver funnel_driver = {
> .drv = {
> .name = "coresight-funnel",
> @@ -258,9 +277,10 @@ static struct amba_driver funnel_driver = {
> .suppress_bind_attrs = true,
> },
> .probe = funnel_probe,
> + .remove = funnel_remove,
> .id_table = funnel_ids,
> };
> -builtin_amba_driver(funnel_driver);
> +module_amba_driver(funnel_driver);
>
> MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> MODULE_DESCRIPTION("ARM Coresight Funnel Driver");
> diff --git a/drivers/hwtracing/coresight/coresight-priv.h b/drivers/hwtracing/coresight/coresight-priv.h
> index 45de8c15b687..896958c2dd44 100644
> --- a/drivers/hwtracing/coresight/coresight-priv.h
> +++ b/drivers/hwtracing/coresight/coresight-priv.h
> @@ -65,7 +65,6 @@ static DEVICE_ATTR_RO(name)
> static const u32 barrier_pkt[5] = {0x7fffffff, 0x7fffffff,
> 0x7fffffff, 0x7fffffff, 0x0};
>
> -
No need for that.
> enum etm_addr_type {
> ETM_ADDR_TYPE_NONE,
> ETM_ADDR_TYPE_SINGLE,
> diff --git a/drivers/hwtracing/coresight/coresight-replicator.c b/drivers/hwtracing/coresight/coresight-replicator.c
> index 9ef539893eaa..6f16dcd7e107 100644
> --- a/drivers/hwtracing/coresight/coresight-replicator.c
> +++ b/drivers/hwtracing/coresight/coresight-replicator.c
> @@ -33,7 +33,12 @@ struct replicator_drvdata {
> static int replicator_enable(struct coresight_device *csdev, int inport,
> int outport)
> {
> - struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct replicator_drvdata *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
> +
> + if (!try_module_get(module))
> + return -ENODEV;
>
> dev_info(drvdata->dev, "REPLICATOR enabled\n");
> return 0;
> @@ -42,8 +47,11 @@ static int replicator_enable(struct coresight_device *csdev, int inport,
> static void replicator_disable(struct coresight_device *csdev, int inport,
> int outport)
> {
> - struct replicator_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct replicator_drvdata *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
>
> + module_put(module);
> dev_info(drvdata->dev, "REPLICATOR disabled\n");
> }
>
> @@ -112,6 +120,17 @@ static int replicator_probe(struct platform_device *pdev)
> return ret;
> }
>
> +static int __exit replicator_remove(struct platform_device *pdev)
> +{
> + struct replicator_drvdata *drvdata = dev_get_drvdata(&pdev->dev);
> +
> + coresight_unregister(drvdata->csdev);
> +
> + pm_runtime_disable(&pdev->dev);
> +
> + return 0;
> +}
> +
> #ifdef CONFIG_PM
> static int replicator_runtime_suspend(struct device *dev)
> {
> @@ -144,8 +163,11 @@ static const struct of_device_id replicator_match[] = {
> {}
> };
>
> +MODULE_DEVICE_TABLE(of, replicator_match);
> +
> static struct platform_driver replicator_driver = {
> .probe = replicator_probe,
> + .remove = replicator_remove,
> .driver = {
> .name = "coresight-replicator",
> .of_match_table = replicator_match,
> @@ -153,7 +175,7 @@ static struct platform_driver replicator_driver = {
> .suppress_bind_attrs = true,
> },
> };
> -builtin_platform_driver(replicator_driver);
> +module_platform_driver(replicator_driver);
>
> MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
> MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> index 30eae52a8757..9997ba0dbd54 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -194,7 +194,12 @@ static int stm_enable(struct coresight_device *csdev,
> struct perf_event *event, u32 mode)
> {
> u32 val;
> - struct stm_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct stm_drvdata *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
> +
> + if (!try_module_get(module))
> + return -ENODEV;
>
> if (mode != CS_MODE_SYSFS)
> return -EINVAL;
Function stm_disable() would likely benefit from a module_put().
> @@ -882,6 +887,17 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id)
> return ret;
> }
>
> +static int __exit stm_remove(struct amba_device *adev)
> +{
> + struct stm_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> + coresight_unregister(drvdata->csdev);
> +
> + stm_unregister_device(&drvdata->stm);
> +
> + return 0;
> +}
> +
> #ifdef CONFIG_PM
> static int stm_runtime_suspend(struct device *dev)
> {
> @@ -922,6 +938,8 @@ static const struct amba_id stm_ids[] = {
> { 0, 0},
> };
>
> +MODULE_DEVICE_TABLE(amba, stm_ids);
> +
> static struct amba_driver stm_driver = {
> .drv = {
> .name = "coresight-stm",
> @@ -930,10 +948,11 @@ static struct amba_driver stm_driver = {
> .suppress_bind_attrs = true,
> },
> .probe = stm_probe,
> + .remove = stm_remove,
> .id_table = stm_ids,
> };
>
> -builtin_amba_driver(stm_driver);
> +module_amba_driver(stm_driver);
>
> MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
> MODULE_DESCRIPTION("Arm CoreSight System Trace Macrocell driver");
> diff --git a/drivers/hwtracing/coresight/coresight-tmc.c b/drivers/hwtracing/coresight/coresight-tmc.c
> index 176a5aeab20e..eb3cdb832f84 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc.c
> @@ -429,6 +429,19 @@ static int tmc_probe(struct amba_device *adev, const struct amba_id *id)
> return ret;
> }
>
> +static int __exit tmc_remove(struct amba_device *adev)
> +{
> + struct tmc_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> + /* free ETB/ETF or ETR memory */
> + tmc_read_unprepare(drvdata);
> +
> + misc_deregister(&drvdata->miscdev);
> + coresight_unregister(drvdata->csdev);
> +
> + return 0;
> +}
> +
Right now I can remove the module for a TMC link or sink when part of an active
session, something I pointed out during an earlier revision.
I also think we need to deal with driver removal cases when the TMC buffer
(ETR or ETF) is being read from sysFS.
> static const struct amba_id tmc_ids[] = {
> {
> .id = 0x000bb961,
> @@ -453,6 +466,8 @@ static const struct amba_id tmc_ids[] = {
> { 0, 0},
> };
>
> +MODULE_DEVICE_TABLE(amba, tmc_ids);
> +
> static struct amba_driver tmc_driver = {
> .drv = {
> .name = "coresight-tmc",
> @@ -460,9 +475,10 @@ static struct amba_driver tmc_driver = {
> .suppress_bind_attrs = true,
> },
> .probe = tmc_probe,
> + .remove = tmc_remove,
> .id_table = tmc_ids,
> };
> -builtin_amba_driver(tmc_driver);
> +module_amba_driver(tmc_driver);
>
> MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
> MODULE_DESCRIPTION("Arm CoreSight Trace Memory Controller driver");
> diff --git a/drivers/hwtracing/coresight/coresight-tpiu.c b/drivers/hwtracing/coresight/coresight-tpiu.c
> index f3b154e150b3..9622f2a5a451 100644
> --- a/drivers/hwtracing/coresight/coresight-tpiu.c
> +++ b/drivers/hwtracing/coresight/coresight-tpiu.c
> @@ -69,7 +69,12 @@ static void tpiu_enable_hw(struct tpiu_drvdata *drvdata)
>
> static int tpiu_enable(struct coresight_device *csdev, u32 mode)
> {
> - struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct tpiu_drvdata *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
> +
> + if (!try_module_get(module))
> + return -ENODEV;
>
> tpiu_enable_hw(drvdata);
>
> @@ -95,10 +100,13 @@ static void tpiu_disable_hw(struct tpiu_drvdata *drvdata)
>
> static void tpiu_disable(struct coresight_device *csdev)
> {
> - struct tpiu_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> + struct device *parent_dev = csdev->dev.parent;
> + struct tpiu_drvdata *drvdata = dev_get_drvdata(parent_dev);
> + struct module *module = parent_dev->driver->owner;
>
> tpiu_disable_hw(drvdata);
>
> + module_put(module);
> dev_info(drvdata->dev, "TPIU disabled\n");
> }
>
> @@ -164,6 +172,15 @@ static int tpiu_probe(struct amba_device *adev, const struct amba_id *id)
> return PTR_ERR_OR_ZERO(drvdata->csdev);
> }
>
> +static int __exit tpiu_remove(struct amba_device *adev)
> +{
> + struct tpiu_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> + coresight_unregister(drvdata->csdev);
> +
> + return 0;
> +}
> +
> #ifdef CONFIG_PM
> static int tpiu_runtime_suspend(struct device *dev)
> {
> @@ -207,6 +224,8 @@ static const struct amba_id tpiu_ids[] = {
> { 0, 0},
> };
>
> +MODULE_DEVICE_TABLE(amba, tpiu_ids);
> +
> static struct amba_driver tpiu_driver = {
> .drv = {
> .name = "coresight-tpiu",
> @@ -215,9 +234,10 @@ static struct amba_driver tpiu_driver = {
> .suppress_bind_attrs = true,
> },
> .probe = tpiu_probe,
> + .remove = tpiu_remove,
> .id_table = tpiu_ids,
> };
> -builtin_amba_driver(tpiu_driver);
> +module_amba_driver(tpiu_driver);
>
> MODULE_AUTHOR("Pratik Patel <pratikp@codeaurora.org>");
> MODULE_AUTHOR("Mathieu Poirier <mathieu.poirier@linaro.org>");
> diff --git a/drivers/hwtracing/coresight/coresight.c b/drivers/hwtracing/coresight/coresight.c
> index 406899f316e4..c00229b0db52 100644
> --- a/drivers/hwtracing/coresight/coresight.c
> +++ b/drivers/hwtracing/coresight/coresight.c
> @@ -302,6 +302,7 @@ void coresight_disable_path(struct list_head *path)
> }
> }
> }
> +EXPORT_SYMBOL_GPL(coresight_disable_path);
>
> int coresight_enable_path(struct list_head *path, u32 mode)
> {
> @@ -353,6 +354,7 @@ int coresight_enable_path(struct list_head *path, u32 mode)
> coresight_disable_path(path);
> goto out;
> }
> +EXPORT_SYMBOL_GPL(coresight_enable_path);
>
> struct coresight_device *coresight_get_sink(struct list_head *path)
> {
> @@ -368,6 +370,7 @@ struct coresight_device *coresight_get_sink(struct list_head *path)
>
> return csdev;
> }
> +EXPORT_SYMBOL_GPL(coresight_get_sink);
>
> static int coresight_enabled_sink(struct device *dev, void *data)
> {
> @@ -392,6 +395,7 @@ static int coresight_enabled_sink(struct device *dev, void *data)
>
> return 0;
> }
> +EXPORT_SYMBOL_GPL(coresight_enabled_sink);
>
> /**
> * coresight_get_enabled_sink - returns the first enabled sink found on the bus
> @@ -414,6 +418,7 @@ struct coresight_device *coresight_get_enabled_sink(bool deactivate)
>
> return dev ? to_coresight_device(dev) : NULL;
> }
> +EXPORT_SYMBOL_GPL(coresight_get_enabled_sink);
>
> /**
> * _coresight_build_path - recursively build a path from a @csdev to a sink.
> @@ -493,6 +498,7 @@ struct list_head *coresight_build_path(struct coresight_device *source,
>
> return path;
> }
> +EXPORT_SYMBOL_GPL(coresight_build_path);
>
> /**
> * coresight_release_path - release a previously built path.
> @@ -517,6 +523,7 @@ void coresight_release_path(struct list_head *path)
> kfree(path);
> path = NULL;
> }
> +EXPORT_SYMBOL_GPL(coresight_release_path);
>
> /** coresight_validate_source - make sure a source has the right credentials
> * @csdev: the device structure for a source.
> @@ -933,6 +940,7 @@ int coresight_timeout(void __iomem *addr, u32 offset, int position, int value)
>
> return -EAGAIN;
> }
> +EXPORT_SYMBOL_GPL(coresight_timeout);
>
> struct bus_type coresight_bustype = {
> .name = "coresight",
> @@ -944,6 +952,12 @@ static int __init coresight_init(void)
> }
> postcore_initcall(coresight_init);
>
> +static void __exit coresight_exit(void)
> +{
> + bus_unregister(&coresight_bustype);
> +}
> +module_exit(coresight_exit);
> +
> struct coresight_device *coresight_register(struct coresight_desc *desc)
> {
> int i;
> --
> 2.17.0
>
^ permalink raw reply
* [linux-next PATCH 0/4] Enable network driver on K2G ICE and GP EVMs
From: Murali Karicheri @ 2018-05-22 21:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8e1aa492-ae7a-de2b-f5d2-8756b10ece79@oracle.com>
On 05/20/2018 11:17 PM, santosh.shilimkar at oracle.com wrote:
> On 5/11/18 12:29 PM, Murali Karicheri wrote:
>> Now that NetCP driver patches for K2G SoC is merged to linux-next master
>> this series add patches to enable network driver on K2G ICE and GP EVMs.
>>
>> Thanks
>>
>> Applied the patches on top of latest linux-next master, built kernel and
>> booted up on both EVMs. The logs are below
>>
>> K2G GP EVM: https://pastebin.ubuntu.com/p/ycZDnZXYPx/
>> K2G ICE EVM: https://pastebin.ubuntu.com/p/bdCpzgdrXr/
>>
>> Murali Karicheri (4):
>> ARM: dts: k2g: add dt bindings to support network driver
>> ARM: dts: keystone-k2g-evm: Enable netcp network driver
>> ARM: dts: keystone-k2g-ice: Enable netcp network driver
>> ARM: keystone: k2g: enable micrel and dp83867 phys
>>
>> arch/arm/boot/dts/keystone-k2g-evm.dts | 53 +++++++++++
>> arch/arm/boot/dts/keystone-k2g-ice.dts | 59 ++++++++++++
>> arch/arm/boot/dts/keystone-k2g-netcp.dtsi | 147 ++++++++++++++++++++++++++++++
>> arch/arm/boot/dts/keystone-k2g.dtsi | 13 +++
>> arch/arm/configs/keystone_defconfig | 2 +
>> 5 files changed, 274 insertions(+)
>> create mode 100644 arch/arm/boot/dts/keystone-k2g-netcp.dtsi
>>
> Looks good. Will add this to the queue for 4.19. Thanks !!
>
> Regards,
> Santosh
>
Thanks Santosh!
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply
* v4.17-rc1: regressions on N900, N950
From: Pavel Machek @ 2018-05-22 20:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180522194139.GB2299@darkstar.musicnaut.iki.fi>
On Tue 2018-05-22 22:41:39, Aaro Koskinen wrote:
> Hi,
>
> On Tue, May 22, 2018 at 10:02:50AM +0200, Pali Roh?r wrote:
> > Hi! I remember that in time of migration from platform board code to
> > device tree structures there appeared some bug which caused that
> > sometimes display were not initialized. And somebody figured out that
> > display initialization is failing when some other SPI devices are
> > initialized before or after display... This behavior was observed only
> > on real N900 hardware, not in qemu.
>
> Touchscreen needs to be initialized before display. This is documented
> in the DTS, see arch/arm/boot/dts/omap3-n900.dts:
>
> * For some reason, touchscreen is necessary for screen to work at
> * all on real hw. It works well without it on emulator.
> *
> * Also... order in the device tree actually matters here.
>
> > Real reason was never explained. In old platform board code there was
> > hardcoded order of SPI devices in which initialization happened. And in
> > device tree it is probably in (pseudo)-random order. Enabling/disabling
> > various config option can affect some timings and order in which kernel
> > starts probing and initializing devices...
>
> The issue was also somewhat present with platform/board code, see e.g.
> commit e65f131a14726e5f1b880a528271a52428e5b3a5.
>
> My device worked with v4.17-rc1 (haven't found time to test newer kernels),
> but if you say the probe order is random then we must find some proper way
> to express the dependency.
I started bisect, but.. that will probably not be useful.
If your device works ok in v4.17-rc1, it probably works in newer -rcs,
too.
Thanks for the ordering hint, I'll try to figure out what is going on
there.
Pavel
# bad: [60cc43fc888428bb2f18f08997432d426a243338] Linux 4.17-rc1
# good: [0adb32858b0bddf4ada5f364a84ed60b196dbcda] Linux 4.16
git bisect start 'v4.17-rc1' 'v4.16'
# bad: [ac9053d2dcb9e8c3fa35ce458dfca8fddc141680] Merge tag 'usb-4.17-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
git bisect bad ac9053d2dcb9e8c3fa35ce458dfca8fddc141680
# bad: [bb2407a7219760926760f0448fddf00d625e5aec] Merge tag 'docs-4.17' of git://git.lwn.net/linux
git bisect bad bb2407a7219760926760f0448fddf00d625e5aec
# bad: [1c7095d2836baafd84e596dd34ba1a1293a4faa9] Merge airlied/drm-next into drm-misc-next
git bisect bad 1c7095d2836baafd84e596dd34ba1a1293a4faa9
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180522/1ed84105/attachment.sig>
^ permalink raw reply
* [PATCH] media: rcar-vin: Drop unnecessary register properties from example vin port
From: Rob Herring @ 2018-05-22 20:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180509184558.14960-1-horms+renesas@verge.net.au>
On Wed, May 09, 2018 at 08:45:58PM +0200, Simon Horman wrote:
> The example vin port node does not have an address and thus does not
> need address-cells or address size-properties.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> ---
> Documentation/devicetree/bindings/media/rcar_vin.txt | 3 ---
> 1 file changed, 3 deletions(-)
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH 2/5] watchdog: sp805: add 'timeout-sec' DT property support
From: Guenter Roeck @ 2018-05-22 20:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527014840-21236-3-git-send-email-ray.jui@broadcom.com>
On Tue, May 22, 2018 at 11:47:17AM -0700, Ray Jui wrote:
> Add support for optional devicetree property 'timeout-sec'.
> 'timeout-sec' is used in the driver if specified in devicetree.
> Otherwise, fall back to driver default, i.e., 60 seconds
>
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/watchdog/sp805_wdt.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
> index 03805bc..1484609 100644
> --- a/drivers/watchdog/sp805_wdt.c
> +++ b/drivers/watchdog/sp805_wdt.c
> @@ -230,7 +230,14 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
> spin_lock_init(&wdt->lock);
> watchdog_set_nowayout(&wdt->wdd, nowayout);
> watchdog_set_drvdata(&wdt->wdd, wdt);
> - wdt_setload(&wdt->wdd, DEFAULT_TIMEOUT);
> +
> + /*
> + * If 'timeout-sec' devicetree property is specified, use that.
> + * Otherwise, use DEFAULT_TIMEOUT
> + */
> + wdt->wdd.timeout = DEFAULT_TIMEOUT;
> + watchdog_init_timeout(&wdt->wdd, 0, &adev->dev);
> + wdt_setload(&wdt->wdd, wdt->wdd.timeout);
>
> ret = watchdog_register_device(&wdt->wdd);
> if (ret) {
> --
> 2.1.4
>
^ permalink raw reply
* [PATCH 1/5] Documentation: DT: Add optional 'timeout-sec' property for sp805
From: Guenter Roeck @ 2018-05-22 20:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527014840-21236-2-git-send-email-ray.jui@broadcom.com>
On Tue, May 22, 2018 at 11:47:16AM -0700, Ray Jui wrote:
> Update the SP805 binding document to add optional 'timeout-sec'
> devicetree property
>
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Documentation/devicetree/bindings/watchdog/sp805-wdt.txt | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> index edc4f0e..f898a86 100644
> --- a/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> +++ b/Documentation/devicetree/bindings/watchdog/sp805-wdt.txt
> @@ -19,6 +19,8 @@ Required properties:
>
> Optional properties:
> - interrupts : Should specify WDT interrupt number.
> +- timeout-sec : Should specify default WDT timeout in seconds. If unset, the
> + default timeout is 30 seconds
>
> Examples:
>
> --
> 2.1.4
>
^ permalink raw reply
* [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Jon Hunter @ 2018-05-22 20:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAPDyKFr_7hbs-MC1jQn5uAgSnb1UGufXZdEmUqNTM63KemTe1g@mail.gmail.com>
On 22/05/18 15:47, Ulf Hansson wrote:
> [...]
>
>>>
>>> +/**
>>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>>> + * @dev: Device to attach.
>>> + * @index: The index of the PM domain.
>>> + *
>>> + * Parse device's OF node to find a PM domain specifier at the provided @index.
>>> + * If such is found, allocates a new device and attaches it to retrieved
>>> + * pm_domain ops.
>>> + *
>>> + * Returns the allocated device if successfully attached PM domain, NULL when
>>> + * the device don't need a PM domain or have a single PM domain, else PTR_ERR()
>>> + * in case of failures. Note that if a power-domain exists for the device, but
>>> + * cannot be found or turned on, then return PTR_ERR(-EPROBE_DEFER) to ensure
>>> + * that the device is not probed and to re-try again later.
>>> + */
>>> +struct device *genpd_dev_pm_attach_by_id(struct device *dev,
>>> + unsigned int index)
>>> +{
>>> + struct device *genpd_dev;
>>> + int num_domains;
>>> + int ret;
>>> +
>>> + if (!dev->of_node)
>>> + return NULL;
>>> +
>>> + /* Deal only with devices using multiple PM domains. */
>>> + num_domains = of_count_phandle_with_args(dev->of_node, "power-domains",
>>> + "#power-domain-cells");
>>> + if (num_domains < 2 || index >= num_domains)
>>> + return NULL;
>>> +
>>> + /* Allocate and register device on the genpd bus. */
>>> + genpd_dev = kzalloc(sizeof(*genpd_dev), GFP_KERNEL);
>>> + if (!genpd_dev)
>>> + return ERR_PTR(-ENOMEM);
>>> +
>>> + dev_set_name(genpd_dev, "genpd:%u:%s", index, dev_name(dev));
>>> + genpd_dev->bus = &genpd_bus_type;
>>> + genpd_dev->release = genpd_release_dev;
>>> +
>>> + ret = device_register(genpd_dev);
>>> + if (ret) {
>>> + kfree(genpd_dev);
>>> + return ERR_PTR(ret);
>>> + }
>>> +
>>> + /* Try to attach the device to the PM domain at the specified index. */
>>> + ret = __genpd_dev_pm_attach(genpd_dev, dev->of_node, index);
>>> + if (ret < 1) {
>>> + device_unregister(genpd_dev);
>>> + return ret ? ERR_PTR(ret) : NULL;
>>> + }
>>> +
>>> + pm_runtime_set_active(genpd_dev);
>>> + pm_runtime_enable(genpd_dev);
>>> +
>>> + return genpd_dev;
>>> +}
>>> +EXPORT_SYMBOL_GPL(genpd_dev_pm_attach_by_id);
>>
>> Thanks for sending this. Believe it or not this has still been on my to-do list
>> and so we definitely need a solution for Tegra.
>>
>> Looking at the above it appears that additional power-domains exposed as devices
>> to the client device. So I assume that this means that the drivers for devices
>> with multiple power-domains will need to call RPM APIs for each of these
>> additional power-domains. Is that correct?
>
> They can, but should not!
>
> Instead, the driver shall use device_link_add() and device_link_del(),
> dynamically, depending on what PM domain that their original device
> needs for the current running use case.
>
> In that way, they keep existing runtime PM deployment, operating on
> its original device.
OK, sounds good. Any reason why the linking cannot be handled by the
above API? Is there a use-case where you would not want it linked?
Thanks
Jon
--
nvpublic
^ permalink raw reply
* [PATCH 3/5] watchdog: sp805: set WDOG_HW_RUNNING when appropriate
From: Guenter Roeck @ 2018-05-22 20:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1527014840-21236-4-git-send-email-ray.jui@broadcom.com>
On Tue, May 22, 2018 at 11:47:18AM -0700, Ray Jui wrote:
> If the watchdog hardware is already enabled during the boot process,
> when the Linux watchdog driver loads, it should reset the watchdog and
> tell the watchdog framework. As a result, ping can be generated from
> the watchdog framework, until the userspace watchdog daemon takes over
> control
>
> Signed-off-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> ---
> drivers/watchdog/sp805_wdt.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
> index 1484609..408ffbe 100644
> --- a/drivers/watchdog/sp805_wdt.c
> +++ b/drivers/watchdog/sp805_wdt.c
> @@ -42,6 +42,7 @@
> /* control register masks */
> #define INT_ENABLE (1 << 0)
> #define RESET_ENABLE (1 << 1)
> + #define ENABLE_MASK (INT_ENABLE | RESET_ENABLE)
> #define WDTINTCLR 0x00C
> #define WDTRIS 0x010
> #define WDTMIS 0x014
> @@ -74,6 +75,18 @@ module_param(nowayout, bool, 0);
> MODULE_PARM_DESC(nowayout,
> "Set to 1 to keep watchdog running after device release");
>
> +/* returns true if wdt is running; otherwise returns false */
> +static bool wdt_is_running(struct watchdog_device *wdd)
> +{
> + struct sp805_wdt *wdt = watchdog_get_drvdata(wdd);
> +
> + if ((readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK) ==
> + ENABLE_MASK)
> + return true;
> + else
> + return false;
return !!(readl_relaxed(wdt->base + WDTCONTROL) & ENABLE_MASK));
> +}
> +
> /* This routine finds load value that will reset system in required timout */
> static int wdt_setload(struct watchdog_device *wdd, unsigned int timeout)
> {
> @@ -239,6 +252,15 @@ sp805_wdt_probe(struct amba_device *adev, const struct amba_id *id)
> watchdog_init_timeout(&wdt->wdd, 0, &adev->dev);
> wdt_setload(&wdt->wdd, wdt->wdd.timeout);
>
> + /*
> + * If HW is already running, enable/reset the wdt and set the running
> + * bit to tell the wdt subsystem
> + */
> + if (wdt_is_running(&wdt->wdd)) {
> + wdt_enable(&wdt->wdd);
> + set_bit(WDOG_HW_RUNNING, &wdt->wdd.status);
> + }
> +
> ret = watchdog_register_device(&wdt->wdd);
> if (ret) {
> dev_err(&adev->dev, "watchdog_register_device() failed: %d\n",
> --
> 2.1.4
>
^ permalink raw reply
* [PATCH v7 2/2] drivers: soc: Add LLCC driver
From: Andy Shevchenko @ 2018-05-22 20:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <19968f96da0c548dd7d96e7520ce899e@codeaurora.org>
On Tue, May 22, 2018 at 11:40 PM, <rishabhb@codeaurora.org> wrote:
> On 2018-05-22 12:38, Andy Shevchenko wrote:
>> On Tue, May 22, 2018 at 9:33 PM, <rishabhb@codeaurora.org> wrote:
>>> On 2018-05-18 14:01, Andy Shevchenko wrote:
>>>>> +struct llcc_slice_desc *llcc_slice_getd(u32 uid)
>>>>> +{
>>>>> + const struct llcc_slice_config *cfg;
>>>>> + struct llcc_slice_desc *desc;
>>>>> + u32 sz, count = 0;
>>>>> +
>>>>> + cfg = drv_data->cfg;
>>>>> + sz = drv_data->cfg_size;
>>>>> +
>>>>
>>>>
>>>>
>>>>> + while (cfg && count < sz) {
>>>>> + if (cfg->usecase_id == uid)
>>>>> + break;
>>>>> + cfg++;
>>>>> + count++;
>>>>> + }
>>>>> + if (cfg == NULL || count == sz)
>>>>> + return ERR_PTR(-ENODEV);
>>>> if (!cfg)
>>>> return ERR_PTR(-ENODEV);
>>>>
>>>> while (cfg->... != uid) {
>>>> cfg++;
>>>> count++;
>>>> }
>>>>
>>>> if (count == sz)
>>>> return ...
>>>>
>>>> Though I would rather put it to for () loop.
>>>>
>>> In each while loop iteration the cfg pointer needs to be checked for
>>> NULL. What if the usecase id never matches the uid passed by client
>>> and we keep iterating. At some point it will crash.
>> do {
>> if (!cfg || count == sz)
>> return ...(-ENODEV);
>> ...
>> } while (...);
>>
>> Though, as I said for-loop will look slightly better I think.
>
> Is this fine?
> for (count = 0; count < sz; count++) {
> if (!cfg)
> return ERR_PTR(-ENODEV);
> if (cfg->usecase_id == uid)
> break;
> cfg++;
> }
> if (count == sz)
> return ERR_PTR(-ENODEV);
for (count = 0; cfg && count < sz; count++, cfg++)
if (_id == uid)
break;
if (!cfg || count == sz)
return ERR_PTR(-ENODEV);
Simpler ?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v7 2/2] drivers: soc: Add LLCC driver
From: rishabhb at codeaurora.org @ 2018-05-22 20:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAHp75Vd8HZU+BT38-OfXHiihv1yZG6YBeMWyfweBA+kAwk6HUw@mail.gmail.com>
On 2018-05-22 12:38, Andy Shevchenko wrote:
> On Tue, May 22, 2018 at 9:33 PM, <rishabhb@codeaurora.org> wrote:
>> On 2018-05-18 14:01, Andy Shevchenko wrote:
>>> On Wed, May 16, 2018 at 8:43 PM, Rishabh Bhatnagar
>>> <rishabhb@codeaurora.org> wrote:
>
>>>> +#define ACTIVATE 0x1
>>>> +#define DEACTIVATE 0x2
>>>> +#define ACT_CTRL_OPCODE_ACTIVATE 0x1
>>>> +#define ACT_CTRL_OPCODE_DEACTIVATE 0x2
>>>> +#define ACT_CTRL_ACT_TRIG 0x1
>>>
>>>
>>> Are these bits? Perhaps BIT() ?
>>>
>> isn't it just better to use fixed size as u suggest in the next
>> comment?
>
> If the are bits, use BIT() macro.
>
>>>> +struct llcc_slice_desc *llcc_slice_getd(u32 uid)
>>>> +{
>>>> + const struct llcc_slice_config *cfg;
>>>> + struct llcc_slice_desc *desc;
>>>> + u32 sz, count = 0;
>>>> +
>>>> + cfg = drv_data->cfg;
>>>> + sz = drv_data->cfg_size;
>>>> +
>>>
>>>
>>>> + while (cfg && count < sz) {
>>>> + if (cfg->usecase_id == uid)
>>>> + break;
>>>> + cfg++;
>>>> + count++;
>>>> + }
>>>> + if (cfg == NULL || count == sz)
>>>> + return ERR_PTR(-ENODEV);
>>>
>>>
>>> if (!cfg)
>>> return ERR_PTR(-ENODEV);
>>>
>>> while (cfg->... != uid) {
>>> cfg++;
>>> count++;
>>> }
>>>
>>> if (count == sz)
>>> return ...
>>>
>>> Though I would rather put it to for () loop.
>>>
>> In each while loop iteration the cfg pointer needs to be checked for
>> NULL. What if the usecase id never matches the uid passed by client
>> and we keep iterating. At some point it will crash.
>
> do {
> if (!cfg || count == sz)
> return ...(-ENODEV);
> ...
> } while (...);
>
> Though, as I said for-loop will look slightly better I think.
Is this fine?
for (count = 0; count < sz; count++) {
if (!cfg)
return ERR_PTR(-ENODEV);
if (cfg->usecase_id == uid)
break;
cfg++;
}
if (count == sz)
return ERR_PTR(-ENODEV);
>
>>>> + ret = llcc_update_act_ctrl(desc->slice_id, act_ctrl_val,
>>>> + DEACTIVATE);
>>>
>>>
>>> Perhaps one line (~83 characters here is OK) ?
>>
>> The checkpatch script complains about such lines.
>
> So what if it just 3 characters out?
>
Other reviewers sometimes are not okay if the checkpatch complains.
Because I have gotten many reviews previously concerning about line
length. Not sure how to proceed here.
>>>> + ret = llcc_update_act_ctrl(desc->slice_id, act_ctrl_val,
>>>> + ACTIVATE);
>
>>> Ditto.
>
>>>> + attr1_cfg = bcast_off +
>>>> +
>>>> LLCC_TRP_ATTR1_CFGn(llcc_table[i].slice_id);
>>>> + attr0_cfg = bcast_off +
>>>> +
>>>> LLCC_TRP_ATTR0_CFGn(llcc_table[i].slice_id);
>
>>> Ditto.
>
>>>> + attr1_val |= llcc_table[i].probe_target_ways <<
>>>> + ATTR1_PROBE_TARGET_WAYS_SHIFT;
>>>> + attr1_val |= llcc_table[i].fixed_size <<
>>>> + ATTR1_FIXED_SIZE_SHIFT;
>>>> + attr1_val |= llcc_table[i].priority <<
>>>> ATTR1_PRIORITY_SHIFT;
>
>>> foo |=
>>> bar << SHIFT;
>>>
>>> would look slightly better.
>
> Did you consider this option ?
Yes, forgot to mention.
^ permalink raw reply
* [PATCH v2] ARM: pxa: dts: add pin definitions for extended GPIOs
From: Daniel Mack @ 2018-05-22 20:22 UTC (permalink / raw)
To: linux-arm-kernel
The PXA3xx series features some extended GPIO banks which are named GPIO0_2,
GPIO1_2 etc. The PXA300, PXA310 and PXA320 have different numbers of such
pins, and they also have variant-specific register offsets.
Signed-off-by: Daniel Mack <daniel@zonque.org>
---
arch/arm/boot/dts/pxa3xx.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm/boot/dts/pxa3xx.dtsi b/arch/arm/boot/dts/pxa3xx.dtsi
index a13ac52e4fd2..99c3687e89d3 100644
--- a/arch/arm/boot/dts/pxa3xx.dtsi
+++ b/arch/arm/boot/dts/pxa3xx.dtsi
@@ -8,6 +8,10 @@
(gpio <= 98) ? (0x0400 + 4 * (gpio - 27)) : \
(gpio <= 127) ? (0x0600 + 4 * (gpio - 99)) : \
0)
+#define MFP_PIN_PXA300_2(gpio) \
+ ((gpio <= 1) ? (0x674 + 4 * gpio) : \
+ (gpio <= 6) ? (0x2dc + 4 * gpio) : \
+ 0)
#define MFP_PIN_PXA310(gpio) \
((gpio <= 2) ? (0x00b4 + 4 * gpio) : \
@@ -18,6 +22,11 @@
(gpio <= 262) ? 0 : \
(gpio <= 268) ? (0x052c + 4 * (gpio - 263)) : \
0)
+#define MFP_PIN_PXA310_2(gpio) \
+ ((gpio <= 1) ? (0x674 + 4 * gpio) : \
+ (gpio <= 6) ? (0x2dc + 4 * gpio) : \
+ (gpio <= 10) ? (0x52c + 4 * gpio) : \
+ 0)
#define MFP_PIN_PXA320(gpio) \
((gpio <= 4) ? (0x0124 + 4 * gpio) : \
@@ -30,6 +39,10 @@
(gpio <= 98) ? (0x04f0 + 4 * (gpio - 74)) : \
(gpio <= 127) ? (0x0600 + 4 * (gpio - 99)) : \
0)
+#define MFP_PIN_PXA320_2(gpio) \
+ ((gpio <= 3) ? (0x674 + 4 * gpio) : \
+ (gpio <= 5) ? (0x284 + 4 * gpio) : \
+ 0)
/*
* MFP Alternate functions for pins having a gpio.
--
2.14.3
^ permalink raw reply related
* [patch v21 2/4] drivers: jtag: Add Aspeed SoC 24xx and 25xx families JTAG master driver
From: Andy Shevchenko @ 2018-05-22 20:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <AM5PR0501MB244981D83DD33BD29FD64924B1940@AM5PR0501MB2449.eurprd05.prod.outlook.com>
On Tue, May 22, 2018 at 6:00 PM, Oleksandr Shamray
<oleksandrs@mellanox.com> wrote:
> Ok. Changed to:
> #define ASPEED_JTAG_IOUT_LEN(len) \
> (ASPEED_JTAG_CTL_ENG_EN | \
> ASPEED_JTAG_CTL_ENG_OUT_EN | \
> ASPEED_JTAG_CTL_INST_LEN(len))
>
> #define ASPEED_JTAG_DOUT_LEN(len) \
> (ASPEED_JTAG_CTL_ENG_EN | \
> ASPEED_JTAG_CTL_ENG_OUT_EN | \
> ASPEED_JTAG_CTL_DATA_LEN(len))
What about
#define _JTAG_OUT_ENABLE \
( _ENG_EN | _ENG_OUT_EN)
#define _IOUT_LEN(len) \
(_ENABLE | _INST_LEN(len))
#define _DOUT_LEN(len) \
...
?
>> > + apb_frq = clk_get_rate(aspeed_jtag->pclk);
>>
>> > + div = (apb_frq % freq == 0) ? (apb_frq / freq) - 1 :
>> > + (apb_frq / freq);
>>
>> Isn't it the same as
>>
>> div = (apb_frq - 1) / freq;
>>
>> ?
> Seems it is same. Thanks.
Though be careful if apb_frq == 0.
In either case the hw will be screwed, but differently.
>> > + if (xfer->direction == JTAG_READ_XFER)
>> > + tdi = UINT_MAX;
>> > + else
>> > + tdi = data[index];
>>
>> > + if (xfer->direction == JTAG_READ_XFER)
>> > + tdi = UINT_MAX;
>> > + else
>> > + tdi = data[index];
>>
>> Take your time to think how the above duplication can be avoided.
>>
>
> In both cases data[] is different, so I should check it twice, but I will
> change it to, macro like:
>
> #define ASPEED_JTAG_GET_TDI(direction, data) \
> (direction == JTAG_READ_XFER) ? UNIT_MAX : data
Perhaps choose better name for data, b/c in the above you are using data[index].
>> > + dev_err(aspeed_jtag->dev, "irq status:%x\n",
>> > + status);
>> Huh, really?! SPAM.
> I will review and delete redundant debug messages.
Just to be sure you got a point. This is interrupt context. Imagine
what might go wrong.
>> > + err = jtag_register(jtag);
>>
>> Perhaps we might have devm_ variant of this. Check how SPI framework
>> deal with a such.
>>
>
> Jtag driver uses miscdevice and related misc_register and misc_deregister
> calls for creation and destruction. There is no device object prior
> to call to misc_register, which could be used in devm_jtag_register.
Same question as per previous patch.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH 2/8] dt-bindings: serial: Add bindings for nvidia,tegra194-tcu
From: Rob Herring @ 2018-05-22 20:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180508114403.14499-3-mperttunen@nvidia.com>
On Tue, May 08, 2018 at 02:43:57PM +0300, Mikko Perttunen wrote:
> Add bindings for the Tegra Combined UART device used to talk to the
> UART console on Tegra194 systems.
>
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
> .../bindings/serial/nvidia,tegra194-tcu.txt | 35 ++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
>
> diff --git a/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
> new file mode 100644
> index 000000000000..86763bc5d74f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
> @@ -0,0 +1,35 @@
> +NVIDIA Tegra Combined UART (TCU)
> +
> +The TCU is a system for sharing a hardware UART instance among multiple
> +systems withing the Tegra SoC. It is implemented through a mailbox-
s/withing/within/
Otherwise,
Reviewed-by: Rob Herring <robh@kernel.org>
> +based protocol where each "virtual UART" has a pair of mailboxes, one
> +for transmitting and one for receiving, that is used to communicate
> +with the hardware implementing the TCU.
> +
> +Required properties:
> +- name : Should be tcu
> +- compatible
> + Array of strings
> + One of:
> + - "nvidia,tegra194-tcu"
> +- mbox-names:
> + "rx" - Mailbox for receiving data from hardware UART
> + "tx" - Mailbox for transmitting data to hardware UART
> +- mboxes: Mailboxes corresponding to the mbox-names.
> +
> +This node is a mailbox consumer. See the following files for details of
> +the mailbox subsystem, and the specifiers implemented by the relevant
> +provider(s):
> +
> +- .../mailbox/mailbox.txt
> +- .../mailbox/nvidia,tegra186-hsp.txt
> +
> +Example bindings:
> +-----------------
> +
> +tcu: tcu {
> + compatible = "nvidia,tegra194-tcu";
> + mboxes = <&hsp_top0 TEGRA_HSP_MBOX_TYPE_SM 0>,
> + <&hsp_aon TEGRA_HSP_MBOX_TYPE_SM 1>;
> + mbox-names = "rx", "tx";
> +};
> --
> 2.16.1
>
^ permalink raw reply
* [PATCH 2/8] dt-bindings: serial: Add bindings for nvidia,tegra194-tcu
From: Rob Herring @ 2018-05-22 20:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3ddaafbd-d8cb-3cca-be4e-8c5c53fd9734@nvidia.com>
On Tue, May 22, 2018 at 04:15:09PM +0100, Jon Hunter wrote:
>
> On 08/05/18 12:43, Mikko Perttunen wrote:
> > Add bindings for the Tegra Combined UART device used to talk to the
> > UART console on Tegra194 systems.
> >
> > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > ---
> > .../bindings/serial/nvidia,tegra194-tcu.txt | 35 ++++++++++++++++++++++
> > 1 file changed, 35 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
> >
> > diff --git a/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
> > new file mode 100644
> > index 000000000000..86763bc5d74f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/serial/nvidia,tegra194-tcu.txt
> > @@ -0,0 +1,35 @@
> > +NVIDIA Tegra Combined UART (TCU)
> > +
> > +The TCU is a system for sharing a hardware UART instance among multiple
> > +systems withing the Tegra SoC. It is implemented through a mailbox-
> > +based protocol where each "virtual UART" has a pair of mailboxes, one
> > +for transmitting and one for receiving, that is used to communicate
> > +with the hardware implementing the TCU.
> > +
> > +Required properties:
> > +- name : Should be tcu
> > +- compatible
> > + Array of strings
> > + One of:
> > + - "nvidia,tegra194-tcu"
>
> Nit. We should say what device the above compatibility is applicable for ...
>
> - "nvidia,tegra194-tcu": for Tegra194
Yeah, everyone seems to do that, but I find it to be redundant.
Rob
^ 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