* [PATCH] arm64: dts: allwinner: h5: NanoPi NEO Plus2 : add EMAC support
From: Antony Antony @ 2017-11-29 14:17 UTC (permalink / raw)
To: Maxime Ripard, Chen-Yu Tsai
Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Antony Antony,
devicetree
add arm64 H5 dwmac-sun8i support for this board
Signed-off-by: Antony Antony <antony-AVhj06Q78b5AfugRpC6u6w@public.gmane.org>
---
.../boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts
index 7c028af..01dace4 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo-plus2.dts
@@ -53,6 +53,7 @@
compatible = "friendlyarm,nanopi-neo-plus2", "allwinner,sun50i-h5";
aliases {
+ ethernet0 = &emac;
serial0 = &uart0;
};
@@ -133,6 +134,22 @@
status = "okay";
};
+&emac {
+ pinctrl-names = "default";
+ pinctrl-0 = <&emac_rgmii_pins>;
+ phy-supply = <®_gmac_3v3>;
+ phy-handle = <&ext_rgmii_phy>;
+ phy-mode = "rgmii";
+ status = "okay";
+};
+
+&external_mdio {
+ ext_rgmii_phy: ethernet-phy@7 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ reg = <7>;
+ };
+};
+
&mmc0 {
pinctrl-names = "default";
pinctrl-0 = <&mmc0_pins_a>, <&mmc0_cd_pin>;
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v4 0/8] Cavium OCTEON-III network driver.
From: David Miller @ 2017-11-29 14:18 UTC (permalink / raw)
To: david.daney
Cc: mark.rutland, linux-mips, devel, devicetree, netdev, linux-kernel,
ralf, robh+dt, andrew, steven.hill, gregkh, f.fainelli,
james.hogan
In-Reply-To: <20171129005540.28829-1-david.daney@cavium.com>
The net-next tree is closed, please resubmit this when the net-next
tree opens again.
Thank you.
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: orange-pi-zero-plus2: enable AP6212a WiFi/BT combo
From: Chen-Yu Tsai @ 2017-11-29 14:22 UTC (permalink / raw)
To: Jagan Teki
Cc: Sergey Matyukevich, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
Mark Rutland, devicetree, linux-arm-kernel
In-Reply-To: <CAMty3ZAiDvAG-P-c0J7vKaDyRWQ2CQobLA_xycRva5B20W6SOg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Nov 29, 2017 at 9:53 PM, Jagan Teki <jagan-dyjBcgdgk7Pe9wHmmfpqLFaTQe2KTcn/@public.gmane.org> wrote:
> On Sat, Nov 4, 2017 at 1:28 AM, Sergey Matyukevich <geomatsi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Enable AP6212a WiFi/BT combo chip on orange-pi-zero-plus2 board:
>> - WiFi SDIO interface is connected to MMC1
>> - WiFi REG_ON pin connected to gpio PA9: attach to mmc-pwrseq
>> - WiFi HOST_WAKE pin connected to gpio PL7
>> - BT is connected to UART1
>>
>> Signed-off-by: Sergey Matyukevich <geomatsi-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> .../allwinner/sun50i-h5-orangepi-zero-plus2.dts | 32 ++++++++++++++++++++++
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dts b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dts
>> index a42fd79a62a3..d415b7b67cce 100644
>> --- a/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dts
>> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-orangepi-zero-plus2.dts
>> @@ -64,6 +64,13 @@
>> regulator-min-microvolt = <3300000>;
>> regulator-max-microvolt = <3300000>;
>> };
>> +
>> + wifi_pwrseq: wifi_pwrseq {
>> + compatible = "mmc-pwrseq-simple";
>> + pinctrl-names = "default";
>> + reset-gpios = <&pio 0 9 GPIO_ACTIVE_LOW>; /* PA9 */
>> + post-power-on-delay-ms = <200>;
>> + };
>> };
>>
>> &mmc0 {
>> @@ -75,6 +82,25 @@
>> status = "okay";
>> };
>>
>> +&mmc1 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&mmc1_pins_a>;
>> + vmmc-supply = <®_vcc3v3>;
>> + vqmmc-supply = <®_vcc3v3>;
>> + mmc-pwrseq = <&wifi_pwrseq>;
>> + bus-width = <4>;
>> + non-removable;
>> + status = "okay";
>> +
>> + brcmf: wifi@1 {
>> + reg = <1>;
>> + compatible = "brcm,bcm4329-fmac";
>> + interrupt-parent = <&r_pio>;
>> + interrupts = <0 7 IRQ_TYPE_LEVEL_LOW>; /* PL7 */
>> + interrupt-names = "host-wake";
>> + };
>> +};
>
> Did you observe this issue, I'm using firmware from buildroot and
> couldn't find any *.txt from brcm/ I believe we need ass text file for
> firmware details, did you tried the same from BR?
The text file is not provided as part of the linux-firmware repository.
You have to fetch it from your vendor BSP. This is documented not just
for sunxi, but also for the raspberry pi 3.
ChenYu
>
> Log:
> [ 1.872235] mmc1: new high speed SDIO card at address 0001
> [ 1.880919] brcmfmac: brcmf_fw_map_chip_to_name: using
> brcm/brcmfmac43430-sdio.bin for chip 0x00a9a6(43430) rev 0x000001
> [ 1.908802] EXT4-fs (mmcblk0p1): re-mounted. Opts: data=ordered
> [ 1.927199] brcmfmac mmc1:0001:1: Direct firmware load for
> brcm/brcmfmac43430-sdio.txt failed with error -2
> [ 2.963478] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000):
> clkctl 0x50
> [ 3.971888] brcmfmac: brcmf_sdio_htclk: HT Avail timeout (1000000):
> clkctl 0x50
>
> thanks!
> --
> Jagan Teki
> Senior Linux Kernel Engineer | Amarula Solutions
> U-Boot, Linux | Upstream Maintainer
> Hyderabad, India.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] arm64: dts: renesas: r8a7795: Move nodes which have no reg property out of bus
From: Rob Herring @ 2017-11-29 14:36 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Simon Horman, Linux-Renesas, linux-arm-kernel@lists.infradead.org,
Magnus Damm, Arnd Bergmann, Geert Uytterhoeven,
devicetree@vger.kernel.org
In-Reply-To: <CAMuHMdVaL6WeOiZhBU2qfXQK7a9NjshUow2GpJJqkwXywiQmaQ@mail.gmail.com>
On Wed, Nov 29, 2017 at 7:58 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Rob,
>
> On Wed, Nov 29, 2017 at 2:35 PM, Rob Herring <robh+dt@kernel.org> wrote:
>> On Tue, Nov 28, 2017 at 3:04 AM, Geert Uytterhoeven
>> <geert@linux-m68k.org> wrote:
>>> On Tue, Nov 28, 2017 at 9:56 AM, Simon Horman <horms@verge.net.au> wrote:
>>>> On Mon, Nov 27, 2017 at 12:15:39PM +0100, Geert Uytterhoeven wrote:
>>>>> On Mon, Nov 27, 2017 at 12:04 PM, Simon Horman <horms@verge.net.au> wrote:
>>>>> > I just noticed that with this patch applied I now see:
>>>>> >
>>>>> > arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dtb: Warning (interrupts_property): Missing interrupt-parent for /pmu_a57
>>>>> > arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dtb: Warning (interrupts_property): Missing interrupt-parent for /pmu_a53
>>>>> > arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dtb: Warning (interrupts_property): Missing interrupt-parent for /pmu_a57
>>>>> > arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dtb: Warning (interrupts_property): Missing interrupt-parent for /pmu_a53arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dtb: Warning (interrupts_property): Missing interrupt-parent for /timer
>>>>> >
>>>>> > arch/arm64/boot/dts/renesas/r8a7795-h3ulcb.dtb: Warning (interrupts_property): Missing interrupt-parent for /timer
>>>>>
>>>>> Right, the "interrupt-parent = <&gic>;" inside the /soc node applies to child
>>>>> nodes of the /soc node only.
>>>>>
>>>>> You can find this in two ways:
>>>
>>> s/find/fix/
>>>
>>>>>
>>>>> 1. Add "interrupt-parent = <&gic>;" to the /pmu_a57 and /pmu_a53 nodes.
>>>>> 2. Switch those nodes from "interrupt" to "interrupts-extended", e.g. turn
>>>>>
>>>>> interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>,
>>>>>
>>>>> into
>>>>>
>>>>> interrupts-extended = <&gic GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>,
>>>>>
>>>>> The latter is what e.g. arch/arm/boot/dts/armada-375.dtsi does.
>>>>
>>>> Thanks, I took option 1 as it seems consistent with the rest of the
>>>> Renesas DT files. I also added it to the /timer node.
>>>
>>> Actually we recently had a discussion about this on IRC, triggered by a
>>> similar issue in board files (see e.g, Ethernet PHY interrupts).
>>>
>>> Given the following comment:
>>>
>>> drivers/of/irq.c: /* Try the new-style interrupts-extended first */
>>> drivers/of/irq.c: res = of_parse_phandle_with_args(device,
>>> "interrupts-extended",
>>>
>>> I think it would be better to use interrupts-extended for individual/isolated
>>> use outside the /soc node.
>>
>> I disagree. There's no point to use interrupts-extended unless you
>> have 2 or more interrupt parents. Just set interrupt-parent in the
>> root node.
>
> The on-SoC devices (all under the /soc node, except for the weird ones without
> reg properties that are now being moved out) all have "&gic" as their interrupt
> parents.
>
> Off-SoC devices use one of the on-SoC interrupt controllers for external
> interrupts ("&irqc", "&irqpin", "&irqpin0", "&irqpin1"), or one of the on-SoC
> GPIO controllers that can also serve external interrupts ("&gpioN").
>
> For the latter, I think interrupts-extended definitely makes sense.
>
> For the former (incl. the "pmu_aN" nodes discussed here), it is debatable.
> But using interrupts-extended makes it easier to catch mistakes in board files,
> as they will be flagged by dtc ("Missing interrupt-parent").
> With interrupt-parent in the root node, they may go undetected.
They would still likely get flagged since the GIC #interrupt-cells is
3 and it is likely you don't have a multiple of 3.
But I guess interrupts-extended does make sense in this case.
Rob
^ permalink raw reply
* Re: [PATCH] ARM: dts: imx6qdl-udoo: add support for powering off
From: Shawn Guo @ 2017-11-29 14:39 UTC (permalink / raw)
To: Maciej S. Szmigiero
Cc: Sascha Hauer, Fabio Estevam, linux-arm-kernel, devicetree,
linux-kernel
In-Reply-To: <475cb655-a88f-0cc2-2e00-405458f80433@maciej.szmigiero.name>
On Mon, Nov 27, 2017 at 11:22:28PM +0100, Maciej S. Szmigiero wrote:
> UDOO board has a possibility to disable most of imx6 SoC power supplies via
> "EN_5V" signal, which is connected to "NANDF_D4" pad.
>
> This reduces current consumption after shutdown from ~350 mA to ~40 mA and
> prevents a SoC watchdog from starting it again on its own after the
> watchdog timeout expires (previously it was not possible to shut down the
> SoC permanently if its watchdog was ever enabled since it was still ticking
> after the shutdown).
>
> Unfortunately, this does not extinguish the "Power" LED (as it is powered
> from an always-on voltage regulator).
>
> To power the SoC up again press the power button (SW2, the one with a long
> shaft).
>
> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v2 10/16] ARM: dts: ixm6qdl-sr-som: add support for eMMC
From: Shawn Guo @ 2017-11-29 14:46 UTC (permalink / raw)
To: Russell King
Cc: Jon Nettleton, Sascha Hauer, Fabio Estevam, Rob Herring,
Mark Rutland, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <E1eJhPK-0006O6-5I-eh5Bv4kxaXIk46pC+1QYvQNdhmdF6hFW@public.gmane.org>
On Tue, Nov 28, 2017 at 03:02:46PM +0000, Russell King wrote:
> Rev 1.5 microsoms include eMMC support on-board. Add a microsom include
> file which can be used to add support for this.
>
> Signed-off-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
s/ixm6qdl/imx6qdl in the subject.
I fixed it up and applied the whole series.
Shawn
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v4 3/5] misc serdev: Add w2sg0004 (gps receiver) power control driver
From: Andrew F. Davis @ 2017-11-29 14:47 UTC (permalink / raw)
To: H. Nikolaus Schaller
Cc: Mark Rutland, DTML, linux-omap, Arnd Bergmann, Tony Lindgren,
Greg Kroah-Hartman, kernel, Russell King,
Linux Kernel Mailing List, Neil Brown, Rob Herring, Linux ARM,
Benoît Cousson, Kevin Hilman, Marek Belisko,
Discussions about the Letux Kernel, Thierry Reding,
Andreas Färber, Jonathan Cameron
In-Reply-To: <B096DC8C-66C5-4F04-8EE4-0B0711EC1700@goldelico.com>
On 11/28/2017 03:42 PM, H. Nikolaus Schaller wrote:
> Hi Andrew,
> now as 4.15-rc1 is out I find time to continue this work and prepare [PATCH v5].
>
>> Am 23.11.2017 um 17:06 schrieb Andrew F. Davis <afd@ti.com>:
>>
>> On 11/15/2017 03:37 PM, H. Nikolaus Schaller wrote:
>>> Add driver for Wi2Wi W2SG0004/84 GPS module connected through uart.
>>>
>>> Use serdev API hooks to monitor and forward the UART traffic to /dev/ttyGPSn
>>> and turn on/off the module. It also detects if the module is turned on (sends data)
>>> but should be off, e.g. if it was already turned on during boot or power-on-reset.
>>>
>>> Additionally, rfkill block/unblock can be used to control an external LNA
>>> (and power down the module if not needed).
>>>
>>> The driver concept is based on code developed by NeilBrown <neilb@suse.de>
>>> but simplified and adapted to use the new serdev API introduced in 4.11.
>>>
>>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>>> ---
>>> drivers/misc/Kconfig | 10 +
>>> drivers/misc/Makefile | 1 +
>>> drivers/misc/w2sg0004.c | 545 ++++++++++++++++++++++++++++++++++++++++++++++++
>
> I wonder if this shouldn't better go to
>
> drivers/gps
>
> But this directory does not yet exist and has no overall maintainer.
> So it could be left in drivers/misc and moved as soon as drivers/gps
> is created elsewhere.
>
Making that directory would imply the existence of a GPS driver
framework, until one comes about (or if you would like to create one), I
think misc is the most appropriate spot for now.
>>> 3 files changed, 556 insertions(+)
>>> create mode 100644 drivers/misc/w2sg0004.c
>>>
>>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>>> index 8136dc7e863d..09d171d68408 100644
>>> --- a/drivers/misc/Kconfig
>>> +++ b/drivers/misc/Kconfig
>>> @@ -518,4 +518,14 @@ source "drivers/misc/mic/Kconfig"
>>> source "drivers/misc/genwqe/Kconfig"
>>> source "drivers/misc/echo/Kconfig"
>>> source "drivers/misc/cxl/Kconfig"
>>> +
>>> +config W2SG0004
>>> + tristate "W2SG00x4 on/off control"
>>> + depends on GPIOLIB && SERIAL_DEV_BUS
>>> + help
>>> + Enable on/off control of W2SG00x4 GPS moduled connected
>>> + to some SoC UART to allow powering up/down if the /dev/ttyGPSn
>>> + is opened/closed.
>>> + It also provides a rfkill gps name to control the LNA power.
>>> +
>>> endmenu
>>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>>> index ad0e64fdba34..abcb667e0ff0 100644
>>> --- a/drivers/misc/Makefile
>>> +++ b/drivers/misc/Makefile
>>> @@ -51,6 +51,7 @@ obj-$(CONFIG_SRAM_EXEC) += sram-exec.o
>>> obj-y += mic/
>>> obj-$(CONFIG_GENWQE) += genwqe/
>>> obj-$(CONFIG_ECHO) += echo/
>>> +obj-$(CONFIG_W2SG0004) += w2sg0004.o
>>> obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o
>>> obj-$(CONFIG_CXL_BASE) += cxl/
>>> obj-$(CONFIG_ASPEED_LPC_CTRL) += aspeed-lpc-ctrl.o
>>> diff --git a/drivers/misc/w2sg0004.c b/drivers/misc/w2sg0004.c
>>> new file mode 100644
>>> index 000000000000..12e14b5e0a99
>>> --- /dev/null
>>> +++ b/drivers/misc/w2sg0004.c
>>> @@ -0,0 +1,545 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>
>> Damn this looks ugly, oh well :/
>
> I could remove it for [PATCH v5] ... :\
>
Nah, you should leave it, this comment was just me venting, Greg KH has
spoken, so this is the correct way now.
>>
>>> +/*
>>> + * Driver for power controlling the w2sg0004/w2sg0084 GPS receiver.
>>> + *
>>
>>
>> Think you still need copyright tag here somewhere.
>
> At the bottom there is:
>
>>> +MODULE_AUTHOR("NeilBrown <neilb@suse.de>");
>>> +MODULE_DESCRIPTION("w2sg0004 GPS power management driver");
>>> +MODULE_LICENSE("GPL v2");
>
> Isn't that enough any more?
>
Not sure.. someone needs to hold the copyright to this file and it
really should be close to the top.
>>
>>> + * This receiver has an ON/OFF pin which must be toggled to
>>> + * turn the device 'on' of 'off'. A high->low->high toggle
>>> + * will switch the device on if it is off, and off if it is on.
>>> + *
>>> + * To enable receiving on/off requests we register with the
>>> + * UART power management notifications.
>>> + *
>>> + * It is not possible to directly detect the state of the device.
>>> + * However when it is on it will send characters on a UART line
>>> + * regularly.
>>> + *
>>> + * To detect that the power state is out of sync (e.g. if GPS
>>> + * was enabled before a reboot), we register for UART data received
>>> + * notifications.
>>> + *
>>> + * In addition we register as a rfkill client so that we can
>>> + * control the LNA power.
>>> + *
>>> + */
>>> +
>>> +#include <linux/delay.h>
>>> +#include <linux/err.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/irq.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/of_gpio.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/regulator/consumer.h>
>>> +#include <linux/rfkill.h>
>>> +#include <linux/serdev.h>
>>> +#include <linux/sched.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/tty.h>
>>> +#include <linux/tty_flip.h>
>>> +#include <linux/workqueue.h>
>>> +
>>> +/*
>>> + * There seems to be restrictions on how quickly we can toggle the
>>> + * on/off line. data sheets says "two rtc ticks", whatever that means.
>>> + * If we do it too soon it doesn't work.
>>> + * So we have a state machine which uses the common work queue to ensure
>>> + * clean transitions.
>>> + * When a change is requested we record that request and only act on it
>>> + * once the previous change has completed.
>>> + * A change involves a 10ms low pulse, and a 990ms raised level, so only
>>> + * one change per second.
>>> + */
>>> +
>>> +enum w2sg_state {
>>> + W2SG_IDLE, /* is not changing state */
>>> + W2SG_PULSE, /* activate on/off impulse */
>>> + W2SG_NOPULSE /* deactivate on/off impulse */
>>> +};
>>> +
>>> +struct w2sg_data {
>>> + struct rfkill *rf_kill;
>>> + struct regulator *lna_regulator;
>>> + int lna_blocked; /* rfkill block gps active */
>>> + int lna_is_off; /* LNA is currently off */
>>> + int is_on; /* current state (0/1) */
>>> + unsigned long last_toggle;
>>> + unsigned long backoff; /* time to wait since last_toggle */
>>> + int on_off_gpio; /* the on-off gpio number */
>>> + struct serdev_device *uart; /* uart connected to the chip */
>>> + struct tty_driver *tty_drv; /* this is the user space tty */
>>> + struct device *dev; /* from tty_port_register_device() */
>>> + struct tty_port port;
>>> + int open_count; /* how often we were opened */
>>> + enum w2sg_state state;
>>> + int requested; /* requested state (0/1) */
>>> + int suspended;
>>> + struct delayed_work work;
>>> + int discard_count;
>>> +};
>>> +
>>
>> Kernel doc style.
>
> Is this really needed here?
>
> For pure driver internal structs (they are not kernel infrastructure API) I usually
> consult the source code of a driver and never well formatted kernel doc. Hence I think
> readability by programmers looking into the source file is more important than serving
> kernel doc tools.
>
> Yes, there are examples like:
>
> https://elixir.free-electrons.com/linux/v4.15-rc1/source/drivers/iio/accel/mma8452.c#L113
>
> But I find them more confusing than helpful because I have to jump between code lines
> to match the comment with the data type.
>
I have no strong preference here as long as you think this is the most
readable way.
>>
>>> +static struct w2sg_data *w2sg_by_minor[1];
>>> +
>>
>> If you can only have one right now then just drop the array.
>
> w2sg_get_by_minor() is prepared to access more than one record.
> And there are plans to have more than one (but I don't know exactly
> how to provide it).
>
Just add it back when you get that functionality.
> Making it a non-array seems to be an unnecessary hurdle for such
> improvements. And regarding memory footprint, a single-element
> array is equivalent to a non-array.
>
This comment was not about memory footprint, it's about sane code practices.
>>
>>> +static int w2sg_set_lna_power(struct w2sg_data *data)
>>> +{
>>> + int ret = 0;
>>> + int off = data->suspended || !data->requested || data->lna_blocked;
>>> +
>>> + pr_debug("%s: %s\n", __func__, off ? "off" : "on");
>>> +
>>> + if (off != data->lna_is_off) {
>>> + data->lna_is_off = off;
>>> + if (!IS_ERR_OR_NULL(data->lna_regulator)) {
>>> + if (off)
>>> + regulator_disable(data->lna_regulator);
>>> + else
>>> + ret = regulator_enable(data->lna_regulator);
>>> + }
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +static void w2sg_set_power(void *pdata, int val)
>>> +{
>>> + struct w2sg_data *data = (struct w2sg_data *) pdata;
>>> +
>>> + pr_debug("%s to state=%d (requested=%d)\n", __func__, val, data->requested);
>>> +
>>> + if (val && !data->requested) {
>>> + data->requested = true;
>>> + } else if (!val && data->requested) {
>>> + data->backoff = HZ;
>>> + data->requested = false;
>>> + } else
>>> + return;
>>> +
>>> + pr_debug("w2sg00x4 scheduled for %d\n", data->requested);
>>> +
>>> + if (!data->suspended)
>>> + schedule_delayed_work(&data->work, 0);
>>> +}
>>> +
>>> +/* called each time data is received by the UART (i.e. sent by the w2sg0004) */
>>> +
>>> +static int w2sg_uart_receive_buf(struct serdev_device *serdev,
>>> + const unsigned char *rxdata,
>>> + size_t count)
>>> +{
>>> + struct w2sg_data *data =
>>> + (struct w2sg_data *) serdev_device_get_drvdata(serdev);
>>> +
>>> + if (!data->requested && !data->is_on) {
>>> + /*
>>> + * we have received characters while the w2sg
>>> + * should have been be turned off
>>> + */
>>> + data->discard_count += count;
>>> + if ((data->state == W2SG_IDLE) &&
>>> + time_after(jiffies,
>>> + data->last_toggle + data->backoff)) {
>>> + /* Should be off by now, time to toggle again */
>>> + pr_debug("w2sg00x4 has sent %d characters data although it should be off!\n",
>>> + data->discard_count);
>>> +
>>> + data->discard_count = 0;
>>> +
>>> + data->is_on = true;
>>> + data->backoff *= 2;
>>> + if (!data->suspended)
>>> + schedule_delayed_work(&data->work, 0);
>>> + }
>>> + } else if (data->open_count > 0) {
>>> + int n;
>>> +
>>> + pr_debug("w2sg00x4: push %lu chars to tty port\n",
>>> + (unsigned long) count);
>>> +
>>> + /* pass to user-space */
>>> + n = tty_insert_flip_string(&data->port, rxdata, count);
>>> + if (n != count)
>>> + pr_err("w2sg00x4: did loose %lu characters\n",
>>> + (unsigned long) (count - n));
>>> + tty_flip_buffer_push(&data->port);
>>> + return n;
>>> + }
>>> +
>>> + /* assume we have processed everything */
>>> + return count;
>>> +}
>>> +
>>> +/* try to toggle the power state by sending a pulse to the on-off GPIO */
>>> +
>>
>> drop extra line, same everywhere
>
> Ok! Was just this and one more location.
>
>>
>>> +static void toggle_work(struct work_struct *work)
>>> +{
>>> + struct w2sg_data *data = container_of(work, struct w2sg_data,
>>> + work.work);
>>> +
>>> + switch (data->state) {
>>> + case W2SG_IDLE:
>>> + if (data->requested == data->is_on)
>>> + return;
>>> +
>>> + w2sg_set_lna_power(data); /* update LNA power state */
>>> + gpio_set_value_cansleep(data->on_off_gpio, 0);
>>> + data->state = W2SG_PULSE;
>>> +
>>> + pr_debug("w2sg: power gpio ON\n");
>>> +
>>> + schedule_delayed_work(&data->work,
>>> + msecs_to_jiffies(10));
>>> + break;
>>> +
>>> + case W2SG_PULSE:
>>> + gpio_set_value_cansleep(data->on_off_gpio, 1);
>>> + data->last_toggle = jiffies;
>>> + data->state = W2SG_NOPULSE;
>>> + data->is_on = !data->is_on;
>>> +
>>> + pr_debug("w2sg: power gpio OFF\n");
>>> +
>>> + schedule_delayed_work(&data->work,
>>> + msecs_to_jiffies(10));
>>> + break;
>>> +
>>> + case W2SG_NOPULSE:
>>> + data->state = W2SG_IDLE;
>>> +
>>> + pr_debug("w2sg: idle\n");
>>> +
>>> + break;
>>> +
>>> + }
>>> +}
>>> +
>>> +static int w2sg_rfkill_set_block(void *pdata, bool blocked)
>>> +{
>>> + struct w2sg_data *data = pdata;
>>> +
>>> + pr_debug("%s: blocked: %d\n", __func__, blocked);
>>> +
>>> + data->lna_blocked = blocked;
>>> +
>>> + return w2sg_set_lna_power(data);
>>> +}
>>> +
>>> +static struct rfkill_ops w2sg0004_rfkill_ops = {
>>> + .set_block = w2sg_rfkill_set_block,
>>> +};
>>> +
>>> +static struct serdev_device_ops serdev_ops = {
>>> + .receive_buf = w2sg_uart_receive_buf,
>>> +};
>>> +
>>> +/*
>>> + * we are a man-in the middle between the user-space visible tty port
>>> + * and the serdev tty where the chip is connected.
>>> + * This allows us to recognise when the device should be powered on
>>> + * or off and handle the "false" state that data arrives while no
>>> + * users-space tty client exists.
>>> + */
>>> +
>>> +static struct w2sg_data *w2sg_get_by_minor(unsigned int minor)
>>> +{
>>> + return w2sg_by_minor[minor];
>>> +}
>>> +
>>> +static int w2sg_tty_install(struct tty_driver *driver, struct tty_struct *tty)
>>> +{
>>> + struct w2sg_data *data;
>>> + int retval;
>>> +
>>> + pr_debug("%s() tty = %p\n", __func__, tty);
>>> +
>>> + data = w2sg_get_by_minor(tty->index);
>>> +
>>> + if (!data)
>>> + return -ENODEV;
>>> +
>>> + retval = tty_standard_install(driver, tty);
>>> + if (retval)
>>> + goto error_init_termios;
>>> +
>>> + tty->driver_data = data;
>>> +
>>> + return 0;
>>> +
>>> +error_init_termios:
>>> + tty_port_put(&data->port);
>>> + return retval;
>>> +}
>>> +
>>> +static int w2sg_tty_open(struct tty_struct *tty, struct file *file)
>>> +{
>>> + struct w2sg_data *data = tty->driver_data;
>>> +
>>> + pr_debug("%s() data = %p open_count = ++%d\n", __func__, data, data->open_count);
>>> +
>>> + w2sg_set_power(data, ++data->open_count > 0);
>>> +
>>> + return tty_port_open(&data->port, tty, file);
>>> +}
>>> +
>>> +static void w2sg_tty_close(struct tty_struct *tty, struct file *file)
>>> +{
>>> + struct w2sg_data *data = tty->driver_data;
>>> +
>>> + pr_debug("%s()\n", __func__);
>>> +
>>> + w2sg_set_power(data, --data->open_count > 0);
>>> +
>>> + tty_port_close(&data->port, tty, file);
>>> +}
>>> +
>>> +static int w2sg_tty_write(struct tty_struct *tty,
>>> + const unsigned char *buffer, int count)
>>> +{
>>> + struct w2sg_data *data = tty->driver_data;
>>> +
>>> + /* simply pass down to UART */
>>> + return serdev_device_write_buf(data->uart, buffer, count);
>>> +}
>>> +
>>> +static const struct tty_operations w2sg_serial_ops = {
>>> + .install = w2sg_tty_install,
>>> + .open = w2sg_tty_open,
>>> + .close = w2sg_tty_close,
>>> + .write = w2sg_tty_write,
>>> +};
>>> +
>>> +static const struct tty_port_operations w2sg_port_ops = {
>>> +};
>>
>> drop the brackets? or use NULL below, not sure if this needs memory.
>
> This allocates a struct tty_port_operations initialized with NULL for
> all function pointers.
>
Then just drop the brackets, or allocate it with kzalloc.
> I am not sure if this is the same as providing no w2sg_port_ops at all.
>
> Rather I think the serial core is only port == NULL safe but not
> port->ops == NULL e.g.:
>
> http://elixir.free-electrons.com/linux/v4.15-rc1/source/drivers/tty/tty_port.c#L422
>
> A test confirms (see comment below):
>
>>
>>> +
>>> +static int w2sg_probe(struct serdev_device *serdev)
>>> +{
>>> + struct w2sg_data *data;
>>> + struct rfkill *rf_kill;
>>> + int err;
>>> + int minor;
>>> + enum of_gpio_flags flags;
>>> +
>>> + pr_debug("%s()\n", __func__);
>>> +
>>> + minor = 0;
>>> + if (w2sg_by_minor[minor]) {
>>> + pr_err("w2sg minor is already in use!\n");
>>> + return -ENODEV;
>>> + }
>>> +
>>> + data = devm_kzalloc(&serdev->dev, sizeof(*data), GFP_KERNEL);
>>> + if (data == NULL)
>>> + return -ENOMEM;
>>> +
>>> + w2sg_by_minor[minor] = data;
>
> While looking through this, I found a potential issue if allocating the
> gpio fails with -EPROBE_DEFER.
>
> Then, we have set w2sg_by_minor[minor] != NULL and the above test will already
> find it allocated on next deferred probe attempt.
>
> So I'll move that to a position further down.
>
>>> +
>>> + serdev_device_set_drvdata(serdev, data);
>>> +
>>> + data->on_off_gpio = of_get_named_gpio_flags(serdev->dev.of_node,
>>> + "enable-gpios", 0,
>>> + &flags);
>>> + /* defer until we have all gpios */
>>> + if (data->on_off_gpio == -EPROBE_DEFER)
>>> + return -EPROBE_DEFER;
>>> +
>>> + data->lna_regulator = devm_regulator_get_optional(&serdev->dev,
>>> + "lna");
>>> + if (IS_ERR(data->lna_regulator)) {
>>> + /* defer until we can get the regulator */
>>> + if (PTR_ERR(data->lna_regulator) == -EPROBE_DEFER)
>>> + return -EPROBE_DEFER;
>>> +
>>> + data->lna_regulator = NULL;
>>> + }
>>> + pr_debug("%s() lna_regulator = %p\n", __func__, data->lna_regulator);
>>> +
>>> + data->lna_blocked = true;
>>> + data->lna_is_off = true;
>>> +
>>> + data->is_on = false;
>>> + data->requested = false;
>>> + data->state = W2SG_IDLE;
>>> + data->last_toggle = jiffies;
>>> + data->backoff = HZ;
>>> +
>>> + data->uart = serdev;
>>> +
>>> + INIT_DELAYED_WORK(&data->work, toggle_work);
>>> +
>>> + err = devm_gpio_request(&serdev->dev, data->on_off_gpio,
>>> + "w2sg0004-on-off");
>>> + if (err < 0)
>>> + goto out;
>>> +
>>> + gpio_direction_output(data->on_off_gpio, false);
>>> +
>>> + serdev_device_set_client_ops(data->uart, &serdev_ops);
>>> + serdev_device_open(data->uart);
>>> +
>>> + serdev_device_set_baudrate(data->uart, 9600);
>>> + serdev_device_set_flow_control(data->uart, false);
>>> +
>>> + rf_kill = rfkill_alloc("GPS", &serdev->dev, RFKILL_TYPE_GPS,
>>> + &w2sg0004_rfkill_ops, data);
>>> + if (rf_kill == NULL) {
>>> + err = -ENOMEM;
>>> + goto err_rfkill;
>>> + }
>>> +
>>> + err = rfkill_register(rf_kill);
>>> + if (err) {
>>> + dev_err(&serdev->dev, "Cannot register rfkill device\n");
>>> + goto err_rfkill;
>>> + }
>>> +
>>> + data->rf_kill = rf_kill;
>>> +
>>> + /* allocate the tty driver */
>>> + data->tty_drv = alloc_tty_driver(1);
>>> + if (!data->tty_drv)
>>> + return -ENOMEM;
>>> +
>>> + /* initialize the tty driver */
>>> + data->tty_drv->owner = THIS_MODULE;
>>> + data->tty_drv->driver_name = "w2sg0004";
>>> + data->tty_drv->name = "ttyGPS";
>>> + data->tty_drv->major = 0;
>>> + data->tty_drv->minor_start = 0;
>>> + data->tty_drv->type = TTY_DRIVER_TYPE_SERIAL;
>>> + data->tty_drv->subtype = SERIAL_TYPE_NORMAL;
>>> + data->tty_drv->flags = TTY_DRIVER_REAL_RAW | TTY_DRIVER_DYNAMIC_DEV;
>>> + data->tty_drv->init_termios = tty_std_termios;
>>> + data->tty_drv->init_termios.c_cflag = B9600 | CS8 | CREAD |
>>> + HUPCL | CLOCAL;
>>> + /*
>>> + * optional:
>>> + * tty_termios_encode_baud_rate(&data->tty_drv->init_termios,
>>> + 115200, 115200);
>>> + * w2sg_tty_termios(&data->tty_drv->init_termios);
>>> + */
>>> + tty_set_operations(data->tty_drv, &w2sg_serial_ops);
>>> +
>>> + /* register the tty driver */
>>> + err = tty_register_driver(data->tty_drv);
>>> + if (err) {
>>> + pr_err("%s - tty_register_driver failed(%d)\n",
>>> + __func__, err);
>>> + put_tty_driver(data->tty_drv);
>>> + goto err_rfkill;
>>> + }
>>> +
>>> + tty_port_init(&data->port);
>>> + data->port.ops = &w2sg_port_ops;
>
> A test setting this to NULL leads to a kernel oops in:
>
> [ 3048.821258] [<c046c598>] (tty_port_open) from [<c0466328>] (tty_open+0x1e8/0x2d8)
>
> So we must define this completely empty struct w2sg_port_ops
> and pass a reference.
>
>>> +
>>> + pr_debug("w2sg call tty_port_register_device\n");
>>> +
>>> + data->dev = tty_port_register_device(&data->port,
>>> + data->tty_drv, minor, &serdev->dev);
>>> +
>>> + pr_debug("w2sg probed\n");
>>> +
>>> + /* keep off until user space requests the device */
>>> + w2sg_set_power(data, false);
>>> +
>>> + return 0;
>>> +
>>> +err_rfkill:
>>> + rfkill_destroy(rf_kill);
>>> + serdev_device_close(data->uart);
>>> +out:
>>> + return err;
>>> +}
>>> +
>>> +static void w2sg_remove(struct serdev_device *serdev)
>>> +{
>>> + struct w2sg_data *data = serdev_device_get_drvdata(serdev);
>>> + int minor;
>>> +
>>> + cancel_delayed_work_sync(&data->work);
>>> +
>>> + /* what is the right sequence to avoid problems? */
>>> + serdev_device_close(data->uart);
>>> +
>>> + minor = 0;
>>> + tty_unregister_device(data->tty_drv, minor);
>>> +
>>> + tty_unregister_driver(data->tty_drv);
>>> +}
>>> +
>>> +static int w2sg_suspend(struct device *dev)
>>
>> __maybe_unused, or if PM is disabled you will get a warning.
>
> Ok.
>
>>
>>> +{
>>> + struct w2sg_data *data = dev_get_drvdata(dev);
>>> +
>>> + data->suspended = true;
>>> +
>>> + cancel_delayed_work_sync(&data->work);
>>> +
>>> + w2sg_set_lna_power(data); /* shuts down if needed */
>>> +
>>> + if (data->state == W2SG_PULSE) {
>>> + msleep(10);
>>> + gpio_set_value_cansleep(data->on_off_gpio, 1);
>>> + data->last_toggle = jiffies;
>>> + data->is_on = !data->is_on;
>>> + data->state = W2SG_NOPULSE;
>>> + }
>>> +
>>> + if (data->state == W2SG_NOPULSE) {
>>> + msleep(10);
>>> + data->state = W2SG_IDLE;
>>> + }
>>> +
>>> + if (data->is_on) {
>>> + pr_info("GPS off for suspend %d %d %d\n", data->requested,
>>> + data->is_on, data->lna_is_off);
>>> +
>>> + gpio_set_value_cansleep(data->on_off_gpio, 0);
>>> + msleep(10);
>>> + gpio_set_value_cansleep(data->on_off_gpio, 1);
>>> + data->is_on = 0;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int w2sg_resume(struct device *dev)
>>> +{
>>> + struct w2sg_data *data = dev_get_drvdata(dev);
>>> +
>>> + data->suspended = false;
>>> +
>>> + pr_info("GPS resuming %d %d %d\n", data->requested,
>>> + data->is_on, data->lna_is_off);
>>> +
>>> + schedule_delayed_work(&data->work, 0); /* enables LNA if needed */
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct of_device_id w2sg0004_of_match[] = {
>>> + { .compatible = "wi2wi,w2sg0004" },
>>> + { .compatible = "wi2wi,w2sg0084" },
>>> + {},
>>> +};
>>> +MODULE_DEVICE_TABLE(of, w2sg0004_of_match);
>>> +
>>> +SIMPLE_DEV_PM_OPS(w2sg_pm_ops, w2sg_suspend, w2sg_resume);
>>> +
>>> +static struct serdev_device_driver w2sg_driver = {
>>> + .probe = w2sg_probe,
>>> + .remove = w2sg_remove,
>>> + .driver = {
>>> + .name = "w2sg0004",
>>> + .owner = THIS_MODULE,
>>> + .pm = &w2sg_pm_ops,
>>> + .of_match_table = of_match_ptr(w2sg0004_of_match)
>>> + },
>>> +};
>>> +
>>> +module_serdev_device_driver(w2sg_driver);
>>> +
>>> +MODULE_ALIAS("w2sg0004");
>>
>> Is this needed?
>
> Seems to be redundant.
> After removal the driver is still loaded automatically after boot:
>
> root@letux:~# modprobe -c | fgrep w2sg
> alias of:N*T*Cwi2wi,w2sg0004 w2sg0004
> alias of:N*T*Cwi2wi,w2sg0004C* w2sg0004
> alias of:N*T*Cwi2wi,w2sg0084 w2sg0004
> alias of:N*T*Cwi2wi,w2sg0084C* w2sg0004
> root@letux:~# lsmod | fgrep w2sg
> w2sg0004 16384 2
> root@letux:~#
>
>>
>>> +
>>> +MODULE_AUTHOR("NeilBrown <neilb@suse.de>");
>>
>> Who really wrote this?
>
> Good question.
>
> The problem is that with cleaning up the code and rewriting history over such
> a long time, it is no more visible.
>
> But I have searched through our older branches:
>
> Neil Brown had developed the first version for v3.7 in 2013:
> http://git.goldelico.com/?p=gta04-kernel.git;a=commit;h=5b6fad7c7f15db8bb8e2c98ae9a50da52bda8b69
>
> This is where the MODULE_AUTHOR line comes from.
>
> Later, Marek Belisko worked on the UART driver, pinmux and interrupts.
>
> And I have
> * added lna-regulator and rfkill ca. v3.12
> * added device tree support ca. v3.15
> * moved to drivers/misc
> * ported the code to serdev API ca. v4.11
> * submitted to LKML and worked in comments by reviewers ca. 4.15
>
> So this is the first version that is close to be acceptable.
>
> Neil had also submitted different versions to LKML but I am not
> sure if he still is active anywhere.
>
> I have also checked a diff between the v3.7 version and the
> current one and there are ca. 30-40% of lines original code by
> Neil.
>
> So I'd say:
>
> * Neil is the original author and a significant amount of untouched code lines and comments are still there
> * he designed the overall driver architecture
> * Hence he is the copyright holder, did license under GPL v2 and we have just made a derivative work out of it
> * Neil did submit his version of serdev ca. 2015 (quite different from this) but resigned after review feedback
> * I did add some important features but not the core code and driver architecture
> * I feel my role as maintainer and massaging everything for DT, serdev and get it upstream, but not "the author"
>
> How should we best reflect this in the AUTHOR macro?
>
Easy:
MODULE_AUTHOR("NeilBrown <neilb@suse.de>");
MODULE_AUTHOR("H. Nikolaus Schaller <hns@goldelico.com>");
you can have more than one :)
>>
>>> +MODULE_DESCRIPTION("w2sg0004 GPS power management driver");
>>> +MODULE_LICENSE("GPL v2");
>>>
>
>
> BR and thanks,
> Nikolaus Schaller
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH v4] ARM: dts: imx6qdl-nitrogen6x: Add SPI NOR partitions
From: Shawn Guo @ 2017-11-29 14:48 UTC (permalink / raw)
To: Otavio Salvador
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Gary Bisson,
Fabio Estevam, devicetree-u79uwXL29TY76Z2rM5mHXA, Sascha Hauer,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
Russell King
In-Reply-To: <20171128174924.5149-1-otavio-fKevB0iiKLMBZ+LybsDmbA@public.gmane.org>
On Tue, Nov 28, 2017 at 03:49:24PM -0200, Otavio Salvador wrote:
> This adds the partitions definition for the SPI NOR to provide
> backward compatibility with the documented[1] layout used with
> Boundary Devices BSP.
>
> 1. https://boundarydevices.com/boot-flash-access-linux/
>
> It exports to Linux:
>
> mtd0: bootloader
> mtd1: env
> mtd2: splash
>
> Signed-off-by: Otavio Salvador <otavio-fKevB0iiKLMBZ+LybsDmbA@public.gmane.org>
Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFCv2 PATCH 09/36] iommu/fault: Allow blocking fault handlers
From: Jean-Philippe Brucker @ 2017-11-29 15:01 UTC (permalink / raw)
To: Yisheng Xie,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
Cc: joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Mark Rutland,
Catalin Marinas, Will Deacon, Lorenzo Pieralisi,
hanjun.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, Sudeep Holla,
rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org,
lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Robin Murphy,
bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
tn-nYOzD4b6Jr9Wk0Htik3J/w@public.gmane.org,
liubo95-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
thunder.leizhen-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA@public.gmane.org,
nwatters
In-Reply-To: <7e1c8ea4-e568-1000-17de-62f8562c7169-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Hello,
On 29/11/17 06:15, Yisheng Xie wrote:
> Hi Jean,
>
> On 2017/10/6 21:31, Jean-Philippe Brucker wrote:
>> - if (domain->ext_handler) {
>> + if (domain->handler_flags & IOMMU_FAULT_HANDLER_ATOMIC) {
>> + fault->flags |= IOMMU_FAULT_ATOMIC;
>
> Why remove the condition of domain->ext_handler? should it be much better like:
> if ((domain->handler_flags & IOMMU_FAULT_HANDLER_ATOMIC) && domain->ext_handler)
>
> If domain->ext_handler is NULL, and (domain->handler_flags & IOMMU_FAULT_HANDLER_ATOMIC)
> is true. It will oops, right?
I removed the check because ext_handler shouldn't be NULL if handler_flags
has a bit set (as per iommu_set_ext_fault_handler). But you're right that
this is fragile, and I overlooked the case where users could call
set_ext_fault_handler to clear the fault handler.
(Note that this ext_handler will most likely be replaced by the fault
infrastructure that Jacob is working on:
https://patchwork.kernel.org/patch/10063385/ to which we should add the
atomic/blocking flags)
Thanks,
Jean
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFCv2 PATCH 05/36] iommu/process: Bind and unbind process to and from devices
From: Jean-Philippe Brucker @ 2017-11-29 15:01 UTC (permalink / raw)
To: Yisheng Xie, linux-arm-kernel@lists.infradead.org,
linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org,
devicetree@vger.kernel.org, iommu@lists.linux-foundation.org
Cc: joro@8bytes.org, robh+dt@kernel.org, Mark Rutland,
Catalin Marinas, Will Deacon, Lorenzo Pieralisi,
hanjun.guo@linaro.org, Sudeep Holla, rjw@rjwysocki.net,
lenb@kernel.org, Robin Murphy, bhelgaas@google.com,
alex.williamson@redhat.com, tn@semihalf.com, liubo95@huawei.com,
thunder.leizhen@huawei.com, gabriele.paoloni@huawei.com, nwatters
In-Reply-To: <9c364147-12c4-5a8a-9cd9-0642a6b92555@huawei.com>
On 29/11/17 06:08, Yisheng Xie wrote:
>
>
> On 2017/10/6 21:31, Jean-Philippe Brucker wrote:
>> +int iommu_process_bind_device(struct device *dev, struct task_struct *task,
>> + int *pasid, int flags)
>> +{
> [..]
>> + err = iommu_process_attach_locked(context, dev);
>> + if (err)
>> + iommu_process_put_locked(process);
> one ref for a context is enough right? so also need call iommu_process_put_locked()
> if attach ok, or will be leak if user call bind twice for the same device and task.
I wasn't sure, I think I prefer taking one ref for each bind. If user
calls bind twice, it should call unbind twice as well (in case of leak we
free the context on process exit).
Also with this implementation, user can call bind for two devices in the
same domain, which will share the same context structure. So we have to
take as many refs as bind() calls.
Thanks,
Jean
^ permalink raw reply
* Re: [PATCH v2 27/35] irqchip: Andestech Internal Vector Interrupt Controller driver
From: Greentime Hu @ 2017-11-29 15:23 UTC (permalink / raw)
To: Marc Zyngier
Cc: Greentime, Linux Kernel Mailing List, Arnd Bergmann, linux-arch,
Thomas Gleixner, Jason Cooper, Rob Herring, netdev, Vincent Chen,
DTML, Al Viro, David Howells, Will Deacon, Daniel Lezcano,
linux-serial-u79uwXL29TY76Z2rM5mHXA, Rick Chen
In-Reply-To: <c7447c93-9905-2840-e2d8-01837b9fdecd-5wv7dgnIgG8@public.gmane.org>
Hi, Marc:
2017-11-28 17:37 GMT+08:00 Marc Zyngier <marc.zyngier-5wv7dgnIgG8@public.gmane.org>:
> On 27/11/17 12:28, Greentime Hu wrote:
>> +static void ativic32_ack_irq(struct irq_data *data)
>> +{
>> + __nds32__mtsr_dsb(1 << data->hwirq, NDS32_SR_INT_PEND2);
>
> Consider writing (1 << data->hwirq) as BIT(data->hwirq).
Thanks for this suggestion. I will modify it in the next version patch.
>> +}
>> +
>> +static void ativic32_mask_irq(struct irq_data *data)
>> +{
>> + unsigned long int_mask2 = __nds32__mfsr(NDS32_SR_INT_MASK2);
>> + __nds32__mtsr_dsb(int_mask2 & (~(1 << data->hwirq)), NDS32_SR_INT_MASK2);
>
> Same here.
Thanks for this suggestion. I will modify it in the next version patch.
>> +}
>> +
>> +static void ativic32_mask_ack_irq(struct irq_data *data)
>> +{
>> + unsigned long int_mask2 = __nds32__mfsr(NDS32_SR_INT_MASK2);
>> + __nds32__mtsr_dsb(int_mask2 & (~(1 << data->hwirq)), NDS32_SR_INT_MASK2);
>> + __nds32__mtsr_dsb((1 << data->hwirq), NDS32_SR_INT_PEND2);
>
> This is effectively MASK+ACK, so you're better off just writing it as
> such. And since there is no advantage in your implementation in having
> MASK_ACK over MASK+ACK, I suggest you remove this function completely,
> and rely on the core code which will call them in sequence.
I think mask_ack is still better than mask + ack because we don't need
to do two function call.
We can save a prologue and a epilogue. It will benefit interrupt latency.
>> +
>> +}
>> +
>> +static void ativic32_unmask_irq(struct irq_data *data)
>> +{
>> + unsigned long int_mask2 = __nds32__mfsr(NDS32_SR_INT_MASK2);
>> + __nds32__mtsr_dsb(int_mask2 | (1 << data->hwirq), NDS32_SR_INT_MASK2);
>
> Same BIT() here.
>
Thanks for this suggestion. I will modify it in the next version patch.
>> +}
>> +
>> +static struct irq_chip ativic32_chip = {
>> + .name = "ativic32",
>> + .irq_ack = ativic32_ack_irq,
>> + .irq_mask = ativic32_mask_irq,
>> + .irq_mask_ack = ativic32_mask_ack_irq,
>> + .irq_unmask = ativic32_unmask_irq,
>> +};
>> +
>> +static unsigned int __initdata nivic_map[6] = { 6, 2, 10, 16, 24, 32 };
>> +
>> +static struct irq_domain *root_domain;
>> +static int ativic32_irq_domain_map(struct irq_domain *id, unsigned int virq,
>> + irq_hw_number_t hw)
>> +{
>> +
>> + unsigned long int_trigger_type;
>> + int_trigger_type = __nds32__mfsr(NDS32_SR_INT_TRIGGER);
>> + if (int_trigger_type & (1 << hw))
>
> And here.
>
Thanks for this suggestion. I will modify it in the next version patch.
>> + irq_set_chip_and_handler(virq, &ativic32_chip, handle_edge_irq);
>> + else
>> + irq_set_chip_and_handler(virq, &ativic32_chip, handle_level_irq);
>
> Since you do not express the trigger in DT, you need to tell the core
> about it by calling irqd_set_trigger_type() with the right setting.
>
Since the comments say so, I will add ativic32_set_type() for irq_set_type()
in the next version patch.
/*
* Must only be called inside irq_chip.irq_set_type() functions.
*/
static inline void irqd_set_trigger_type(struct irq_data *d, u32 type)
{
__irqd_to_state(d) &= ~IRQD_TRIGGER_MASK;
__irqd_to_state(d) |= type & IRQD_TRIGGER_MASK;
}
It will be like this.
static int ativic32_set_type(struct irq_data *data, unsigned int flow_type)
{
irqd_set_trigger_type(data, flow_type);
return IRQ_SET_MASK_OK;
}
>> +
>> + return 0;
>> +}
>> +
>> +static struct irq_domain_ops ativic32_ops = {
>> + .map = ativic32_irq_domain_map,
>> + .xlate = irq_domain_xlate_onecell
>> +};
>> +
>> +static int get_intr_src(void)
>
> I'm not sure "int" is the best return type here. I suspect something
> unsigned would be preferable, or even the irq_hw_number_t type.
Thanks for this suggestion. I will modify it in the next version patch.
>> +{
>> + return ((__nds32__mfsr(NDS32_SR_ITYPE)&ITYPE_mskVECTOR) >> ITYPE_offVECTOR)
>
> Spaces around '&'.
>
Thanks for this suggestion. I will modify it in the next version patch.
>> + - NDS32_VECTOR_offINTERRUPT;
>> +}
>> +
>> +asmlinkage void asm_do_IRQ(struct pt_regs *regs)
>> +{
>> + int hwirq = get_intr_src();
>
> irq_hw_number_t.
>
Thanks for this suggestion. I will modify it in the next version patch.
>> + handle_domain_irq(root_domain, hwirq, regs);
>> +}
>> +
>> +int __init ativic32_init_irq(struct device_node *node, struct device_node *parent)
>> +{
>> + unsigned long int_vec_base, nivic;
>> +
>> + if (WARN(parent, "non-root ativic32 are not supported"))
>> + return -EINVAL;
>> +
>> + int_vec_base = __nds32__mfsr(NDS32_SR_IVB);
>> +
>> + if (((int_vec_base & IVB_mskIVIC_VER) >> IVB_offIVIC_VER) == 0)
>> + panic("Unable to use atcivic32 for this cpu.\n");
>> +
>> + nivic = (int_vec_base & IVB_mskNIVIC) >> IVB_offNIVIC;
>> + if (nivic >= (sizeof nivic_map / sizeof nivic_map[0]))
>
> This should be:
> if (nivic >= ARRAY_SIZE(NIVIC_MAP))
>
Thanks for this suggestion. I will modify it in the next version patch.
>> + panic("The number of input for ativic32 is not supported.\n");
>> +
>> + nivic = nivic_map[nivic];
>
> I'd rather you use another variable (nr_ints?).
>
Thanks for this suggestion. I will modify it in the next version patch.
>> +
>> + root_domain = irq_domain_add_linear(node, nivic,
>> + &ativic32_ops, NULL);
>> +
>> + if (!root_domain)
>> + panic("%s: unable to create IRQ domain\n", node->full_name);
>> +
>> + return 0;
>> +}
>> +IRQCHIP_DECLARE(ativic32, "andestech,ativic32", ativic32_init_irq);
>>
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] arm64: dts: allwinner: h5: NanoPi NEO Plus2 : add EMAC support
From: Maxime Ripard @ 2017-11-29 15:30 UTC (permalink / raw)
To: Antony Antony
Cc: Chen-Yu Tsai, linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, devicetree
In-Reply-To: <20171129141716.98951-1-antony-AVhj06Q78b5AfugRpC6u6w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 319 bytes --]
On Wed, Nov 29, 2017 at 03:17:16PM +0100, Antony Antony wrote:
> add arm64 H5 dwmac-sun8i support for this board
>
> Signed-off-by: Antony Antony <antony-AVhj06Q78b5AfugRpC6u6w@public.gmane.org>
Applied, thanks
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v5 13/13] ASoC: stm32: add DFSDM DAI support
From: Mark Brown @ 2017-11-29 15:37 UTC (permalink / raw)
To: Arnaud Pouliquen
Cc: Mark Rutland, devicetree, alsa-devel, Lars-Peter Clausen,
Maxime Coquelin, Liam Girdwood, linux-iio, Takashi Iwai,
Rob Herring, linux-arm-kernel, Peter Meerwald-Stadler,
Hartmut Knaack, Jonathan Cameron, Alexandre Torgue
In-Reply-To: <1511881557-28596-14-git-send-email-arnaud.pouliquen@st.com>
[-- Attachment #1.1: Type: text/plain, Size: 350 bytes --]
On Tue, Nov 28, 2017 at 04:05:57PM +0100, Arnaud Pouliquen wrote:
> Add driver to handle DAI interface for PDM microphones connected
> to Digital Filter for Sigma Delta Modulators IP.
This is basically fine, if someone could send me a pull request and the
relevant patches when the IIO stuff is sorted out I'll give it a final
check and apply then.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [PATCH 3/5] media: i2c: Add TDA1997x HDMI receiver driver
From: Tim Harvey @ 2017-11-29 15:47 UTC (permalink / raw)
To: Hans Verkuil
Cc: linux-media, alsa-devel, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, Shawn Guo, Steve Longerbeam,
Philipp Zabel, Hans Verkuil, Mauro Carvalho Chehab
In-Reply-To: <0f5227cb-913b-1d55-0b1a-5c41d68f5bf9@xs4all.nl>
On Thu, Nov 23, 2017 at 12:08 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 11/23/2017 05:27 AM, Tim Harvey wrote:
>> On Mon, Nov 20, 2017 at 7:39 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>> Hi Tim,
>>>
>>> Some more review comments:
>>>
>>> On 11/09/2017 07:45 PM, Tim Harvey wrote:
>>>> Add support for the TDA1997x HDMI receivers.
>> <snip>
>>>> + */
>>>> +struct color_matrix_coefs {
>>>> + const char *name;
>>>> + /* Input offsets */
>>>> + s16 offint1;
>>>> + s16 offint2;
>>>> + s16 offint3;
>>>> + /* Coeficients */
>>>> + s16 p11coef;
>>>> + s16 p12coef;
>>>> + s16 p13coef;
>>>> + s16 p21coef;
>>>> + s16 p22coef;
>>>> + s16 p23coef;
>>>> + s16 p31coef;
>>>> + s16 p32coef;
>>>> + s16 p33coef;
>>>> + /* Output offsets */
>>>> + s16 offout1;
>>>> + s16 offout2;
>>>> + s16 offout3;
>>>> +};
>>>> +
>>>> +enum {
>>>> + ITU709_RGBLIMITED,
>>>> + ITU709_RGBFULL,
>>>> + ITU601_RGBLIMITED,
>>>> + ITU601_RGBFULL,
>>>> + RGBLIMITED_RGBFULL,
>>>> + RGBLIMITED_ITU601,
>>>> + RGBFULL_ITU601,
>>>
>>> This can't be right.
>>> ITU709_RGBLIMITED
>>> You have these conversions:
>>>
>>> ITU709_RGBFULL
>>> ITU601_RGBFULL
>>> RGBLIMITED_RGBFULL
>>> RGBLIMITED_ITU601
>>> RGBFULL_ITU601
>>> RGBLIMITED_ITU709
>>> RGBFULL_ITU709
>>>
>>> I.e. on the HDMI receiver side you can receive RGB full/limited or ITU601/709.
>>> On the output side you have RGB full or ITU601/709.
>>>
>>> So something like ITU709_RGBLIMITED makes no sense.
>>>
>>
>> I misunderstood the V4L2_CID_DV_RX_RGB_RANGE thinking that it allowed
>> you to configure the output range. If output to the SoC is only ever
>> full quant range for RGB then I can drop the
>> ITU709_RGBLIMITED/ITU601_RGBLIMITED conversions.
>
> Output for RGB is always full range. The reason is simply that the V4L2 API
> has no way of selecting the quantization range it wants to receive. I made
> a patch for that a few years back, but there really is no demand for it (yet).
> Userspace expects full range RGB and limited range YUV.
>
>>
>> However, If the output is YUV how do I know if I need to convert to
>> ITU709 or ITU601 and what are my conversion matrices for
>> RGBLIMITED_ITU709/RGBFULL_ITU709?
>
> You can choose yourself whether you convert to YUV 601 or 709. I would
> recommend to use 601 for SDTV resolutions (i.e. width/height <= 720x576)
> and 709 for HDTV.
>
> I made a little program that calculates the values for RGB lim/full to
> YUV 601/709:
>
> -------------------------------------
> #include <stdlib.h>
> #include <stdio.h>
>
> #define COEFF(v, r) ((v) * (r) * 16.0)
>
> static const double bt601[3][3] = {
> { COEFF(0.299, 219), COEFF(0.587, 219), COEFF(0.114, 219) },
> { COEFF(-0.1687, 224), COEFF(-0.3313, 224), COEFF(0.5, 224) },
> { COEFF(0.5, 224), COEFF(-0.4187, 224), COEFF(-0.0813, 224) },
> };
> static const double rec709[3][3] = {
> { COEFF(0.2126, 219), COEFF(0.7152, 219), COEFF(0.0722, 219) },
> { COEFF(-0.1146, 224), COEFF(-0.3854, 224), COEFF(0.5, 224) },
> { COEFF(0.5, 224), COEFF(-0.4542, 224), COEFF(-0.0458, 224) },
> };
>
> int main(int argc, char **argv)
> {
> int i, j;
> int mapi[] = { 0, 2, 1 };
> int mapj[] = { 1, 0, 2 };
>
> printf("rgb full -> 601\n");
> printf(" 0, 0, 0,\n");
> for (i = 0; i < 3; i++) {
> for (j = 0; j < 3; j++) {
> printf("%5d, ", (int)(0.5 + bt601[mapi[i]][mapj[j]]));
> }
> printf("\n");
> }
> printf(" 256, 2048, 2048,\n\n");
>
> printf("rgb lim -> 601\n");
> printf(" -256, -256, -256,\n");
> for (i = 0; i < 3; i++) {
> for (j = 0; j < 3; j++) {
> printf("%5d, ", (int)(0.5 + 255.0 / 219.0 * bt601[mapi[i]][mapj[j]]));
> }
> printf("\n");
> }
> printf(" 256, 2048, 2048,\n\n");
>
> printf("rgb full -> 709\n");
> printf(" 0, 0, 0,\n");
> for (i = 0; i < 3; i++) {
> for (j = 0; j < 3; j++) {
> printf("%5d, ", (int)(0.5 + rec709[mapi[i]][mapj[j]]));
> }
> printf("\n");
> }
> printf(" 256, 2048, 2048,\n\n");
>
> printf("rgb lim -> 709\n");
> printf(" -256, -256, -256,\n");
> for (i = 0; i < 3; i++) {
> for (j = 0; j < 3; j++) {
> printf("%5d, ", (int)(0.5 + 255.0 / 219.0 * rec709[mapi[i]][mapj[j]]));
> }
> printf("\n");
> }
> printf(" 256, 2048, 2048,\n\n");
> return 0;
> }
> -------------------------------------
>
> This should give you the needed matrices. It's up to you whether to keep the
> existing matrices for 601 or replace them with these. Probably best to keep
> them.
>
Hans,
Thanks for the code - this gives me what I need.
>>
>> Sorry for all the questions, the colorspace/colorimetry options
>> confuse the heck out of me.
>>
>>>> +};
>>>> +
>> <snip>
>>>> +
>>>> +/* parse an infoframe and do some sanity checks on it */
>>>> +static unsigned int
>>>> +tda1997x_parse_infoframe(struct tda1997x_state *state, u16 addr)
>>>> +{
>>>> + struct v4l2_subdev *sd = &state->sd;
>>>> + union hdmi_infoframe frame;
>>>> + u8 buffer[40];
>>>> + u8 reg;
>>>> + int len, err;
>>>> +
>>>> + /* read data */
>>>> + len = io_readn(sd, addr, sizeof(buffer), buffer);
>>>> + err = hdmi_infoframe_unpack(&frame, buffer);
>>>> + if (err) {
>>>> + v4l_err(state->client,
>>>> + "failed parsing %d byte infoframe: 0x%04x/0x%02x\n",
>>>> + len, addr, buffer[0]);
>>>> + return err;
>>>> + }
>>>> + hdmi_infoframe_log(KERN_INFO, &state->client->dev, &frame);
>>>> + switch (frame.any.type) {
>>>> + /* Audio InfoFrame: see HDMI spec 8.2.2 */
>>>> + case HDMI_INFOFRAME_TYPE_AUDIO:
>>>> + /* sample rate */
>>>> + switch (frame.audio.sample_frequency) {
>>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_32000:
>>>> + state->audio_samplerate = 32000;
>>>> + break;
>>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_44100:
>>>> + state->audio_samplerate = 44100;
>>>> + break;
>>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_48000:
>>>> + state->audio_samplerate = 48000;
>>>> + break;
>>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_88200:
>>>> + state->audio_samplerate = 88200;
>>>> + break;
>>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_96000:
>>>> + state->audio_samplerate = 96000;
>>>> + break;
>>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_176400:
>>>> + state->audio_samplerate = 176400;
>>>> + break;
>>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_192000:
>>>> + state->audio_samplerate = 192000;
>>>> + break;
>>>> + default:
>>>> + case HDMI_AUDIO_SAMPLE_FREQUENCY_STREAM:
>>>> + break;
>>>> + }
>>>> +
>>>> + /* sample size */
>>>> + switch (frame.audio.sample_size) {
>>>> + case HDMI_AUDIO_SAMPLE_SIZE_16:
>>>> + state->audio_samplesize = 16;
>>>> + break;
>>>> + case HDMI_AUDIO_SAMPLE_SIZE_20:
>>>> + state->audio_samplesize = 20;
>>>> + break;
>>>> + case HDMI_AUDIO_SAMPLE_SIZE_24:
>>>> + state->audio_samplesize = 24;
>>>> + break;
>>>> + case HDMI_AUDIO_SAMPLE_SIZE_STREAM:
>>>> + default:
>>>> + break;
>>>> + }
>>>> +
>>>> + /* Channel Count */
>>>> + state->audio_channels = frame.audio.channels;
>>>> + if (frame.audio.channel_allocation &&
>>>> + frame.audio.channel_allocation != state->audio_ch_alloc) {
>>>> + /* use the channel assignment from the infoframe */
>>>> + state->audio_ch_alloc = frame.audio.channel_allocation;
>>>> + tda1997x_configure_audout(sd, state->audio_ch_alloc);
>>>> + /* reset the audio FIFO */
>>>> + tda1997x_hdmi_info_reset(sd, RESET_AUDIO, false);
>>>> + }
>>>> + break;
>>>> +
>>>> + /* Auxiliary Video information (AVI) InfoFrame: see HDMI spec 8.2.1 */
>>>> + case HDMI_INFOFRAME_TYPE_AVI:
>>>> + state->colorspace = frame.avi.colorspace;
>>>> + state->colorimetry = frame.avi.colorimetry;
>>>> + state->range = frame.avi.quantization_range;
>>>
>>> This should be ignored if it is overridden by the RGB Quantization Range
>>> control, or am I missing something?
>>>
>>
>> Ok. Sounds like I should only use the range from the infoframe if
>> range == V4L2_DV_RGB_RANGE_AUTO:
>>
>> /* Quantization Range */
>> if (state->range == V4L2_DV_RGB_RANGE_AUTO)
>> state->range = frame.avi.quantization_range;
>
> Huh? You're mixing V4L2_DV_RGB_* defines with HDMI_QUANTIZATION_RANGE_*
> defines.
>
> You probably mean to check the control value here.
>
Yes, I think I've got it now and will post a v4 today.
Tim
^ permalink raw reply
* Re: [PATCH 2/5] media: dt-bindings: Add bindings for TDA1997X
From: Tim Harvey @ 2017-11-29 15:54 UTC (permalink / raw)
To: Sakari Ailus
Cc: linux-media, alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Shawn Guo,
Steve Longerbeam, Philipp Zabel, Hans Verkuil,
Mauro Carvalho Chehab, Rob Herring
In-Reply-To: <20171123082532.znxnmwpgrdjbhxbi-S+BSfZ9RZZmRSg0ZkenSGLdO1Tsj/99ntUK59QYPAWc@public.gmane.org>
On Thu, Nov 23, 2017 at 12:25 AM, Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> wrote:
> On Wed, Nov 22, 2017 at 08:37:04PM -0800, Tim Harvey wrote:
>> On Tue, Nov 21, 2017 at 11:36 PM, Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org> wrote:
>> > Hi Tim,
>> >
>> > On Thu, Nov 09, 2017 at 10:45:33AM -0800, Tim Harvey wrote:
>> >> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> >> Signed-off-by: Tim Harvey <tharvey-UMMOYl/HMS+akBO8gow8eQ@public.gmane.org>
>> >> ---
>> >> v3:
>> >> - fix typo
>> >>
>> >> v2:
>> >> - add vendor prefix and remove _ from vidout-portcfg
>> >> - remove _ from labels
>> >> - remove max-pixel-rate property
>> >> - describe and provide example for single output port
>> >> - update to new audio port bindings
>> >> ---
>> >> .../devicetree/bindings/media/i2c/tda1997x.txt | 179 +++++++++++++++++++++
>> >> 1 file changed, 179 insertions(+)
>> >> create mode 100644 Documentation/devicetree/bindings/media/i2c/tda1997x.txt
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/media/i2c/tda1997x.txt b/Documentation/devicetree/bindings/media/i2c/tda1997x.txt
>> >> new file mode 100644
>> >> index 0000000..dd37f14
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/media/i2c/tda1997x.txt
>> >> @@ -0,0 +1,179 @@
>> >> +Device-Tree bindings for the NXP TDA1997x HDMI receiver
>> >> +
>> >> +The TDA19971/73 are HDMI video receivers.
>> >> +
>> >> +The TDA19971 Video port output pins can be used as follows:
>> >> + - RGB 8bit per color (24 bits total): R[11:4] B[11:4] G[11:4]
>> >> + - YUV444 8bit per color (24 bits total): Y[11:4] Cr[11:4] Cb[11:4]
>> >> + - YUV422 semi-planar 8bit per component (16 bits total): Y[11:4] CbCr[11:4]
>> >> + - YUV422 semi-planar 10bit per component (20 bits total): Y[11:2] CbCr[11:2]
>> >> + - YUV422 semi-planar 12bit per component (24 bits total): - Y[11:0] CbCr[11:0]
>> >> + - YUV422 BT656 8bit per component (8 bits total): YCbCr[11:4] (2-cycles)
>> >> + - YUV422 BT656 10bit per component (10 bits total): YCbCr[11:2] (2-cycles)
>> >> + - YUV422 BT656 12bit per component (12 bits total): YCbCr[11:0] (2-cycles)
>> >> +
>> >> +The TDA19973 Video port output pins can be used as follows:
>> >> + - RGB 12bit per color (36 bits total): R[11:0] B[11:0] G[11:0]
>> >> + - YUV444 12bit per color (36 bits total): Y[11:0] Cb[11:0] Cr[11:0]
>> >> + - YUV422 semi-planar 12bit per component (24 bits total): Y[11:0] CbCr[11:0]
>> >> + - YUV422 BT656 12bit per component (12 bits total): YCbCr[11:0] (2-cycles)
>> >> +
>> >> +The Video port output pins are mapped via 4-bit 'pin groups' allowing
>> >> +for a variety of connection possibilities including swapping pin order within
>> >> +pin groups. The video_portcfg device-tree property consists of register mapping
>> >> +pairs which map a chip-specific VP output register to a 4-bit pin group. If
>> >> +the pin group needs to be bit-swapped you can use the *_S pin-group defines.
>> >> +
>> >> +Required Properties:
>> >> + - compatible :
>> >> + - "nxp,tda19971" for the TDA19971
>> >> + - "nxp,tda19973" for the TDA19973
>> >> + - reg : I2C slave address
>> >> + - interrupts : The interrupt number
>> >> + - DOVDD-supply : Digital I/O supply
>> >> + - DVDD-supply : Digital Core supply
>> >> + - AVDD-supply : Analog supply
>> >> + - nxp,vidout-portcfg : array of pairs mapping VP output pins to pin groups.
>> >> +
>> >> +Optional Properties:
>> >> + - nxp,audout-format : DAI bus format: "i2s" or "spdif".
>> >> + - nxp,audout-width : width of audio output data bus (1-4).
>> >> + - nxp,audout-layout : data layout (0=AP0 used, 1=AP0/AP1/AP2/AP3 used).
>> >> + - nxp,audout-mclk-fs : Multiplication factor between stream rate and codec
>> >> + mclk.
>> >> +
>> >> +The device node must contain one 'port' child node for its digital output
>> >> +video port, in accordance with the video interface bindings defined in
>> >> +Documentation/devicetree/bindings/media/video-interfaces.txt.
>> >
>> > Could you add that this port has one endpoint node as well? (Unless you
>> > support multiple, that is.)
>>
>> Sure... will clarify as:
>>
>> The device node must contain one endpoint 'port' child node for its
>> digital output
>> video port, in accordance with the video interface bindings defined in
>> Documentation/devicetree/bindings/media/video-interfaces.txt.
>
> I think it'd be clearer if you just add "The port node shall contain one
> endpoint child node".
ok
>
>>
>> >> +
>> >> +Optional Endpoint Properties:
>> >> + The following three properties are defined in video-interfaces.txt and
>> >> + are valid for source endpoints only:
>> >
>> > Transmitters? Don't you have an endpoint only in the port representing the
>> > transmitter?
>>
>> I'm not usre what you mean.
>>
>> The TDA1997x is an HDMI receiver meaning it receives HDMI and decodes
>> it to a parallel video bus. HDMI transmitters are the opposite.
>
> The parallel bus. If you mean that, then you could just say that. "Source
> endpoint" is a bit vague, or requires knowing V4L2.
>
agreed
Thanks,
Tim
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/5] PCI: cadence: Add host driver for Cadence PCIe controller
From: Bjorn Helgaas @ 2017-11-29 15:55 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Cyrille Pitchen, bhelgaas, kishon, lorenzo.pieralisi, linux-pci,
adouglas, stelford, dgary, kgopi, eandrews, sureshp, nsekhar,
linux-kernel, robh, devicetree
In-Reply-To: <20171129091929.0077ffbc@windsurf.home>
On Wed, Nov 29, 2017 at 09:19:29AM +0100, Thomas Petazzoni wrote:
> Hello,
>
> On Tue, 28 Nov 2017 14:41:14 -0600, Bjorn Helgaas wrote:
>
> > > + * struct cdns_pcie_rc_data - hardware specific data
> >
> > "cdns" is a weird abbreviation for "Cadence", since "Cadence" doesn't
> > contain an "s".
>
> cdns is the official Device Tree binding vendor prefix for Cadence:
>
> $ grep Cadence Documentation/devicetree/bindings/vendor-prefixes.txt
> cdns Cadence Design Systems Inc.
>
> And it is already widely used throughout the kernel for Cadence
> drivers. See drivers/watchdog/cadence_wdt.c, drivers/spi/spi-cadence.c,
> drivers/i2c/busses/i2c-cadence.c, etc.
Hard to argue with that! Thanks!
^ permalink raw reply
* Re: [PATCH 3/4] RFC: net: dsa: Add bindings for Realtek SMI DSAs
From: Andrew Lunn @ 2017-11-29 15:56 UTC (permalink / raw)
To: Linus Walleij
Cc: Vivien Didelot, Florian Fainelli, netdev, Antti Seppälä,
Roman Yeryomin, Colin Leitner, Gabor Juhos, devicetree
In-Reply-To: <CACRpkdbzqnbF49yN-SAqLkhufk=R2C=jpdLo-QWmL0zR_+Vj=w@mail.gmail.com>
> I have the phy-handle in the ethernet controller. This RTL8366RB
> thing is just one big PHY as far as I know.
Hi Linus
We don't model switches as PHYs. They are their own device type. And
the internal or external PHYs are just normal PHYs in the linux
model. Meaning their interrupt properties goes in the PHY node in
device tree, as documented in the phy.txt binding documentation.
Andrew
^ permalink raw reply
* Re: [PATCH v4 7/8] netdev: octeon-ethernet: Add Cavium Octeon III support.
From: Souptick Joarder @ 2017-11-29 16:07 UTC (permalink / raw)
To: David Daney
Cc: Mark Rutland, linux-mips, devel, devicetree, netdev, linux-kernel,
ralf, Carlos Munoz, Rob Herring, Andrew Lunn, Steven J. Hill,
Greg Kroah-Hartman, Florian Fainelli, James Hogan,
David S. Miller
In-Reply-To: <CAFqt6zabdQhyjUc4WsjzJ6CxMr70H3V_JdipJVwRi8LuOG54tA@mail.gmail.com>
On Wed, Nov 29, 2017 at 4:00 PM, Souptick Joarder <jrdr.linux@gmail.com> wrote:
> On Wed, Nov 29, 2017 at 6:25 AM, David Daney <david.daney@cavium.com> wrote:
>> From: Carlos Munoz <cmunoz@cavium.com>
>>
>> The Cavium OCTEON cn78xx and cn73xx SoCs have network packet I/O
>> hardware that is significantly different from previous generations of
>> the family.
>> diff --git a/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c b/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
>> new file mode 100644
>> index 000000000000..4dad35fa4270
>> --- /dev/null
>> +++ b/drivers/net/ethernet/cavium/octeon/octeon3-bgx-port.c
>> @@ -0,0 +1,2033 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2017 Cavium, Inc.
>> + *
>> + * This file is subject to the terms and conditions of the GNU General Public
>> + * License. See the file "COPYING" in the main directory of this archive
>> + * for more details.
>> + */
>> +#include <linux/platform_device.h>
>> +#include <linux/netdevice.h>
>> +#include <linux/etherdevice.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_mdio.h>
>> +#include <linux/of_net.h>
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/list.h>
>> +
>> +static void bgx_port_sgmii_set_link_down(struct bgx_port_priv *priv)
>> +{
>> + u64 data;
>> + data = oct_csr_read(BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx, priv->index));
>> + data |= BIT(11);
>> + oct_csr_write(data, BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx, priv->index));
>> + data = oct_csr_read(BGX_GMP_PCS_MISC_CTL(priv->node, priv->bgx, priv->index));
>
> Any particular reason to read immediately after write ?
>> +static int bgx_port_sgmii_set_link_speed(struct bgx_port_priv *priv, struct port_status status)
>> +{
>> + u64 data;
>> + u64 prtx;
>> + u64 miscx;
>> + int timeout;
>> +
>> +
>> + switch (status.speed) {
>> + case 10:
>
> In my opinion, instead of hard coding the value, is it fine to use ENUM ?
Similar comments applicable in other places where hard coded values are used.
>> +static int bgx_port_gser_27882(struct bgx_port_priv *priv)
>> +{
>> + u64 data;
>> + u64 addr;
>
>> + int timeout = 200;
>> +
>> + // timeout = 200;
Better to initialize the timeout value
>> +static int bgx_port_qlm_rx_equalization(struct bgx_port_priv *priv, int qlm, int lane)
>> +{
>> + lmode = oct_csr_read(GSER_LANE_MODE(priv->node, qlm));
>> + lmode &= 0xf;
>> + addr = GSER_LANE_P_MODE_1(priv->node, qlm, lmode);
>> + data = oct_csr_read(addr);
>> + /* Don't complete rx equalization if in VMA manual mode */
>> + if (data & BIT(14))
>> + return 0;
>> +
>> + /* Apply rx equalization for speed > 6250 */
>> + if (bgx_port_get_qlm_speed(priv, qlm) < 6250)
>> + return 0;
>> +
>> + /* Wait until rx data is valid (CDRLOCK) */
>> + timeout = 500;
>
> 500 us is the min required value or it can be further reduced ?
>> +static int bgx_port_init_xaui_link(struct bgx_port_priv *priv)
>> +{
>> +
>> + if (use_ber) {
>> + timeout = 10000;
>> + do {
>> + data =
>> + oct_csr_read(BGX_SPU_BR_STATUS1(priv->node, priv->bgx, priv->index));
>> + if (data & BIT(0))
>> + break;
>> + timeout--;
>> + udelay(1);
>> + } while (timeout);
>
> In my opinion, it's better to implement similar kind of loops inside macros.
>
>> + if (!timeout) {
>> + pr_debug("BGX%d:%d:%d: BLK_LOCK timeout\n",
>> + priv->bgx, priv->index, priv->node);
>> + return -1;
>> + }
>> + } else {
>> + timeout = 10000;
>> + do {
>> + data =
>> + oct_csr_read(BGX_SPU_BX_STATUS(priv->node, priv->bgx, priv->index));
>> + if (data & BIT(12))
>> + break;
>> + timeout--;
>> + udelay(1);
>> + } while (timeout);
> same here
^ permalink raw reply
* Re: [RFC 0/2] of: Add whitelist
From: Alan Tull @ 2017-11-29 16:11 UTC (permalink / raw)
To: Rob Herring
Cc: Frank Rowand, Pantelis Antoniou, Moritz Fischer,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-fpga-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAL_JsqL5gYh_++1wDLMYjO7MXKOHpzr315FZ1tuN9o5tOe7TYA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, Nov 29, 2017 at 7:31 AM, Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Nov 29, 2017 at 3:20 AM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 11/27/17 15:58, Alan Tull wrote:
>>> Here's a proposal for a whitelist to lock down the dynamic device tree.
>>>
>>> For an overlay to be accepted, all of its targets are required to be
>>> on a target node whitelist.
>>>
>>> Currently the only way I have to get on the whitelist is calling a
>>> function to add a node. That works for fpga regions, but I think
>>> other uses will need a way of having adding specific nodes from the
>>> base device tree, such as by adding a property like 'allow-overlay;'
>>> or 'allow-overlay = "okay";' If that is acceptable, I could use some
>>> advice on where that particular code should go.
>>>
>>> Alan
>>>
>>> Alan Tull (2):
>>> of: overlay: add whitelist
>>> fpga: of region: add of-fpga-region to whitelist
>>>
>>> drivers/fpga/of-fpga-region.c | 9 ++++++
>>> drivers/of/overlay.c | 73 +++++++++++++++++++++++++++++++++++++++++++
>>> include/linux/of.h | 12 +++++++
>>> 3 files changed, 94 insertions(+)
>>>
>>
>> The plan was to use connectors to restrict where an overlay could be applied.
>> I would prefer not to have multiple methods for accomplishing the same thing
>> unless there is a compelling reason to do so.
>
> Connector nodes need a mechanism to enable themselves, too. I don't
> think connector nodes are going to solve every usecase.
>
> Rob
The two methods I'm suggesting are intended to handle different cases.
There will exist some drivers that by their nature will want every
instance to be enabled for overlays, such as fpga regions. The other
case is where drivers could support overlays but that's not the
widespread use for them. So no need to enable every instance of that
driver for overlays. In that case the DT property provides some
granularity, only enabling overlays for specific instances of that
driver, leaving the rest of the DT locked down.
If we only want one method, I would choose having the DT property only
and not exporting the functions. Users would have to add the property
for every FPGA region but that's not really painful. This would have
the benefit of still keeping the DT locked down unless someone
specifically wanted to enable some regions for overlays for their
particular use.
Alan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH v2 2/3] regulator: core: Parse coupled regulators properties
From: Mark Brown @ 2017-11-29 16:25 UTC (permalink / raw)
To: Maciej Purski
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Liam Girdwood, Rob Herring,
Mark Rutland, Marek Szyprowski, Bartlomiej Zolnierkiewicz
In-Reply-To: <1508330822-8039-3-git-send-email-m.purski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1967 bytes --]
On Wed, Oct 18, 2017 at 02:47:01PM +0200, Maciej Purski wrote:
This looks mostly good, a few things below that I think should be
relatively easy to addresss.
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
> @@ -3953,6 +3953,198 @@ static int regulator_register_resolve_supply(struct device *dev, void *data)
> return 0;
> }
>
> +/* Function returns regulator coupled with the given regulator_dev */
> +static struct regulator_dev *parse_coupled_regulator(struct regulator_dev *rdev,
> + int index)
> +{
> + struct device_node *node = rdev->dev.of_node;
This is DT parsing code directly in the regulator core - we need to work
with other firmware interfaces and with board files. This code should
be in of_regulator.c to ensure that the infrastructure works for non-DT
users. The code's pretty much there, it mostly needs moving around.
The coupling voltage needs to go into the constraints data.
> + /* Regulator can't be found */
> + if (j == n_coupled && tmp_rdev != c_rdev)
> + return -1;
Please return error codes and print error messages, it'll help users
debug stuff.
> @@ -4116,6 +4308,8 @@ regulator_register(const struct regulator_desc *regulator_desc,
> dev_set_drvdata(&rdev->dev, rdev);
> rdev_init_debugfs(rdev);
>
> + regulator_resolve_coupling(rdev);
> +
> /* try to resolve regulators supply since a new one was registered */
> class_for_each_device(®ulator_class, NULL, NULL,
> regulator_register_resolve_supply);
> @@ -4129,6 +4323,7 @@ regulator_register(const struct regulator_desc *regulator_desc,
> wash:
> kfree(rdev->constraints);
> mutex_lock(®ulator_list_mutex);
> + regulator_clean_coupling(rdev);
> regulator_ena_gpio_free(rdev);
> mutex_unlock(®ulator_list_mutex);
> clean:
Shouldn't we be resolving the coupling while we hold the list lock?
We're still working with multiple regulators at this point.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [RFC V7 0/2] OPP: Allow OPP table to be used for power-domains
From: Rob Herring @ 2017-11-29 16:37 UTC (permalink / raw)
To: Viresh Kumar
Cc: Stephen Boyd, Rafael Wysocki, linux-pm@vger.kernel.org,
Nishanth Menon, Vincent Guittot, Rajendra Nayak, Sudeep Holla,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Viresh Kumar, Ulf Hansson, Kevin Hilman
In-Reply-To: <20171129041435.GG11413@vireshk-i7>
On Tue, Nov 28, 2017 at 10:14 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> On 31-10-17, 18:17, Viresh Kumar wrote:
>> Hi,
>>
>> Now that the performance state of PM domains are supported by the kernel
>> (merged in linux-next), I am trying once again to define the bindings
>> which we dropped until the code is merged first.
>
> @Rob/Stephen: Can I get Ack from you guys on this series, before I
> apply them? Ulf has already reviewed these.
I don't ack RFCs. In any case, I'm waiting to see what Stephen says.
Rob
^ permalink raw reply
* Re: [RFC V7 1/2] OPP: Allow OPP table to be used for power-domains
From: Rob Herring @ 2017-11-29 16:46 UTC (permalink / raw)
To: Viresh Kumar
Cc: Ulf Hansson, Kevin Hilman, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki,
linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Vincent Guittot,
Rajendra Nayak, Sudeep Holla,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <2b244ea0a09deaf50237fb8b7578273a8284499e.1509453284.git.viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Tue, Oct 31, 2017 at 7:47 AM, Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> Power-domains can also have their active states and this patch enhances
> the OPP binding to define those.
>
> The power domains can use the OPP bindings mostly as is. Though there
> are some changes required to support special cases:
>
> - Allow "operating-points-v2" to contain multiple phandles for power
> domain providers providing multiple domains.
>
> - A new property "power-domain-opp" is added for devices to specify the
> minimum required OPP of the master domain for the functioning of the
> device. We can add this property directly to device's node if the
> device has a fixed minimum OPP requirement from the master power
> domain. Or we can add this property to each OPP node of the device, if
> different OPP nodes have different minimum OPP requirement from the
> master power domain.
>
> Signed-off-by: Viresh Kumar <viresh.kumar-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> Documentation/devicetree/bindings/opp/opp.txt | 12 +++++
> .../devicetree/bindings/power/power_domain.txt | 62 ++++++++++++++++++++++
> 2 files changed, 74 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/opp/opp.txt b/Documentation/devicetree/bindings/opp/opp.txt
> index 9d733af26be7..203e09fe7698 100644
> --- a/Documentation/devicetree/bindings/opp/opp.txt
> +++ b/Documentation/devicetree/bindings/opp/opp.txt
> @@ -45,6 +45,11 @@ Devices supporting OPPs must set their "operating-points-v2" property with
> phandle to a OPP table in their DT node. The OPP core will use this phandle to
> find the operating points for the device.
>
> +This can contain more than one phandle for power domain providers that provide
> +multiple power domains. That is, one phandle for each power domain. If only one
> +phandle is available, then the same OPP table will be used for all power domains
> +provided by the power domain provider.
> +
> If required, this can be extended for SoC vendor specific bindings. Such bindings
> should be documented as Documentation/devicetree/bindings/power/<vendor>-opp.txt
> and should have a compatible description like: "operating-points-v2-<vendor>".
> @@ -154,6 +159,13 @@ properties.
>
> - status: Marks the node enabled/disabled.
>
> +- power-domain-opp: This contains phandle to one of the OPP nodes of the master
> + power domain. This specifies the minimum required OPP of the master domain for
> + the functioning of the device in this OPP (where this property is present).
> + This property can only be set for a device if the device node contains the
> + "power-domains" property. Also, either all or none of the OPP nodes in an OPP
> + table should have it set.
This is a "this device requires OPP n" property. Couldn't we want this
for cases other than a powerdomain OPP? What if a device has
requirements 2 different OPPs?
On the flipside, I don't think we want devices picking things like CPU
OPPs and putting policy here. But I'd rather things be extendable than
reviewing yet another OPP property next month.
Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH V2 8/9] PCI: tegra: Broadcast PME_turn_Off message before link goes to L2
From: Bjorn Helgaas @ 2017-11-29 16:51 UTC (permalink / raw)
To: Manikanta Maddireddy
Cc: thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
jonathanh-DDmLM1+adcrQT0dZR+AlfA, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
frowand.list-Re5JQEeQqe8AvxtiuMwx3w,
bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, rjw-LthD3rsA81gm4RdzfppkhA,
tglx-hfZtesqFncYOwBW4kG4KsQ, vidyas-DDmLM1+adcrQT0dZR+AlfA,
kthota-DDmLM1+adcrQT0dZR+AlfA, linux-tegra-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-pci-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1511638333-22951-9-git-send-email-mmaddireddy-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
On Sun, Nov 26, 2017 at 01:02:12AM +0530, Manikanta Maddireddy wrote:
> Per PCIe r3.0, sec 5.3.3.2.1, PCIe root port shoould broadcast PME_turn_Off
> message before PCIe link goes to L2. PME_turn_Off broadcast mechanism is
> implemented in AFI module. Each Tegra PCIe root port has its own
> PME_turn_Off and PME_TO_Ack bitmap in AFI_PME register, program this
> register to broadcast PME_turn_Off message.
s/PME_turn_Off/PME_Turn_Off/ above to match spec.
I thought PME_TO_Ack was also mis-capitalized, but it's not. Guess
that "TO" stands for "Turn Off".
> Signed-off-by: Manikanta Maddireddy <mmaddireddy-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> V2:
> * no change in this patch
>
> drivers/pci/host/pci-tegra.c | 76 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 76 insertions(+)
>
> diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
> index bbc2807bcd4a..b380958a3deb 100644
> --- a/drivers/pci/host/pci-tegra.c
> +++ b/drivers/pci/host/pci-tegra.c
> @@ -155,6 +155,8 @@
> #define AFI_INTR_EN_FPCI_TIMEOUT (1 << 7)
> #define AFI_INTR_EN_PRSNT_SENSE (1 << 8)
>
> +#define AFI_PCIE_PME 0xf0
> +
> #define AFI_PCIE_CONFIG 0x0f8
> #define AFI_PCIE_CONFIG_PCIE_DISABLE(x) (1 << ((x) + 1))
> #define AFI_PCIE_CONFIG_PCIE_DISABLE_ALL 0xe
> @@ -315,6 +317,7 @@
> #define PADS_REFCLK_CFG_DRVI_SHIFT 12 /* 15:12 */
>
> #define LINK_RETRAIN_TIMEOUT 100000
> +#define PME_ACK_TIMEOUT 10000
>
> struct tegra_msi {
> struct msi_controller chip;
> @@ -1503,6 +1506,76 @@ static int tegra_pcie_put_resources(struct tegra_pcie *pcie)
> return 0;
> }
>
> +static inline u32 get_pme_turnoff_bitmap(struct tegra_pcie_port *port)
> +{
> + struct device *dev = port->pcie->dev;
> + struct device_node *np = dev->of_node;
> + int ret = 0;
Unnecessary initialization. In fact, the variable is unnecessary; you
can just return the value directly as soon as you know it.
> +
> + switch (port->index) {
> + case 0:
> + ret = 0;
> + case 1:
> + ret = 8;
> + case 2:
> + if (of_device_is_compatible(np, "nvidia,tegra30-pcie"))
> + ret = 16;
> + else
> + ret = 12;
> + }
> + return ret;
> +}
> +
> +static inline u32 get_pme_ack_bitmap(struct tegra_pcie_port *port)
> +{
> + struct device *dev = port->pcie->dev;
> + struct device_node *np = dev->of_node;
> + int ret = 0;
Similar.
> +
> + switch (port->index) {
> + case 0:
> + ret = 5;
> + case 1:
> + ret = 10;
> + case 2:
> + if (of_device_is_compatible(np, "nvidia,tegra30-pcie"))
> + ret = 18;
> + else
> + ret = 14;
> + }
> + return ret;
> +}
> +
> +static void tegra_pcie_pme_turnoff(struct tegra_pcie_port *port)
> +{
> + struct tegra_pcie *pcie = port->pcie;
> + ktime_t deadline;
> + unsigned int data;
> +
> + data = afi_readl(pcie, AFI_PCIE_PME);
> + data |= (0x1 << get_pme_turnoff_bitmap(port));
> + afi_writel(pcie, data, AFI_PCIE_PME);
> +
> + deadline = ktime_add_us(ktime_get(), PME_ACK_TIMEOUT);
> + do {
> + data = afi_readl(pcie, AFI_PCIE_PME);
> + data &= (0x1 << get_pme_ack_bitmap(port));
> + udelay(1);
> + if (ktime_after(ktime_get(), deadline))
> + break;
> + } while (!data);
> +
> + if (data)
> + dev_err(pcie->dev, "PME Ack is not receieved on port: %d\n",
s/receieved/received/
> + port->index);
> +
> + usleep_range(10000, 11000);
> +
> + data = afi_readl(pcie, AFI_PCIE_PME);
> + data &= ~(0x1 << get_pme_turnoff_bitmap(port));
> + afi_writel(pcie, data, AFI_PCIE_PME);
> +}
> +
> static int tegra_msi_alloc(struct tegra_msi *chip)
> {
> int msi;
> @@ -2828,6 +2901,7 @@ static int tegra_pcie_remove(struct platform_device *pdev)
> {
> struct tegra_pcie *pcie = platform_get_drvdata(pdev);
> struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
> + struct tegra_pcie_port *port, *tmp;
>
> if (IS_ENABLED(CONFIG_DEBUG_FS))
> tegra_pcie_debugfs_exit(pcie);
> @@ -2835,6 +2909,8 @@ static int tegra_pcie_remove(struct platform_device *pdev)
> pci_remove_root_bus(host->bus);
> if (IS_ENABLED(CONFIG_PCI_MSI))
> tegra_pcie_disable_msi(pcie);
> + list_for_each_entry_safe(port, tmp, &pcie->ports, list)
> + tegra_pcie_pme_turnoff(port);
> tegra_pcie_disable_ports(pcie);
> tegra_pcie_free_resources(pcie);
> tegra_pcie_disable_controller(pcie);
> --
> 2.1.4
>
^ permalink raw reply
* Re: [PATCH 1/2] arm64: dts: renesas: r8a77970: use CPG core clock macros
From: Sergei Shtylyov @ 2017-11-29 17:00 UTC (permalink / raw)
To: Simon Horman
Cc: Rob Herring, Catalin Marinas, Will Deacon,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Magnus Damm, Mark Rutland,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20171129095558.wvvcvfctwfmtrkq4-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
On 11/29/2017 12:55 PM, Simon Horman wrote:
>> Now that the commit ecadea00f588 ("dt-bindings: clock: Add R8A77970 CPG
>> core clock definitions") has hit Linus' tree, we can replace the bare
>> numbers (we had to use to avoid a cross tree dependency) with these macro
>> definitions...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
>
> Thanks, applied.
Not seeing these 2 patches in the today's devel branch tho...
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 1/2] pinctrl: Allow a device to indicate when to force a state
From: Tony Lindgren @ 2017-11-29 17:01 UTC (permalink / raw)
To: Florian Fainelli
Cc: linux-gpio, Linus Walleij, Rob Herring, Mark Rutland,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list, ckeepax, ckeepax, swarren, andy.shevchenko, alcooperx,
bcm-kernel-feedback-list
In-Reply-To: <20171102231551.16220-2-f.fainelli@gmail.com>
* Florian Fainelli <f.fainelli@gmail.com> [171102 23:18]:
> It may happen that a device needs to force applying a state, e.g:
> because it only defines one state of pin states (default) but loses
> power/register contents when entering low power modes. Add a
> pinctrl_dev::flags bitmask to help describe future quirks and define
> PINCTRL_FLG_FORCE_STATE as such a settable flag.
It makes sense to tag the existing state with the context loss
information as otherwise we'll be duplicating the state in the
pinctrl driver potentially for hundreds of pins.
Maybe this patch description should clarify that it's the
pinctrl device restoring the pin state, not the pinctrl
consumer devices?
So maybe just "a pinctrl device needs to force apply a state"
instead of just device above?
Regards,
Tony
^ 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