* [PATCH] PCI: Add information about describing PCI in ACPI
From: Lorenzo Pieralisi @ 2016-11-23 12:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAKv+Gu-TW__+eZFS2p=1ZLN7fBs7pqQ+SitbEX46Z4JiS64egA@mail.gmail.com>
On Wed, Nov 23, 2016 at 07:28:12AM +0000, Ard Biesheuvel wrote:
> On 23 November 2016 at 01:06, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Tue, Nov 22, 2016 at 10:09:50AM +0000, Ard Biesheuvel wrote:
> >> On 17 November 2016 at 17:59, Bjorn Helgaas <bhelgaas@google.com> wrote:
> >
> >> > +PCI host bridges are PNP0A03 or PNP0A08 devices. Their _CRS should
> >> > +describe all the address space they consume. In principle, this would
> >> > +be all the windows they forward down to the PCI bus, as well as the
> >> > +bridge registers themselves. The bridge registers include things like
> >> > +secondary/subordinate bus registers that determine the bus range below
> >> > +the bridge, window registers that describe the apertures, etc. These
> >> > +are all device-specific, non-architected things, so the only way a
> >> > +PNP0A03/PNP0A08 driver can manage them is via _PRS/_CRS/_SRS, which
> >> > +contain the device-specific details. These bridge registers also
> >> > +include ECAM space, since it is consumed by the bridge.
> >> > +
> >> > +ACPI defined a Producer/Consumer bit that was intended to distinguish
> >> > +the bridge apertures from the bridge registers [4, 5]. However,
> >> > +BIOSes didn't use that bit correctly, and the result is that OSes have
> >> > +to assume that everything in a PCI host bridge _CRS is a window. That
> >> > +leaves no way to describe the bridge registers in the PNP0A03/PNP0A08
> >> > +device itself.
> >>
> >> Is that universally true? Or is it still possible to do the right
> >> thing here on new ACPI architectures such as arm64?
> >
> > That's a very good question. I had thought that the ACPI spec had
> > given up on Consumer/Producer completely, but I was wrong. In the 6.0
> > spec, the Consumer/Producer bit is still documented in the Extended
> > Address Space Descriptor (sec 6.4.3.5.4). It is documented as
> > "ignored" in the QWord, DWord, and Word descriptors (sec 6.4.3.5.1,2,3).
> >
> > Linux looks at the producer_consumer bit in acpi_decode_space(), which
> > I think is used for all these descriptors (QWord, DWord, Word, and
> > Extended). This doesn't quite follow the spec -- we probably should
> > ignore it except for Extended. In any event, acpi_decode_space() sets
> > IORESOURCE_WINDOW for Producer descriptors, but we don't test
> > IORESOURCE_WINDOW in the PCI host bridge code.
> >
> > x86 and ia64 supply their own pci_acpi_root_prepare_resources()
> > functions that call acpi_pci_probe_root_resources(), which parses _CRS
> > and looks at producer_consumer. Then they do a little arch-specific
> > stuff on the result.
> >
> > On arm64 we use acpi_pci_probe_root_resources() directly, with no
> > arch-specific stuff.
> >
> > On all three arches, we ignore the Consumer/Producer bit, so all the
> > resources are treated as Producers, e.g., as bridge windows.
> >
> > I think we *could* implement an arm64 version of
> > pci_acpi_root_prepare_resources() that would pay attention to the
> > Consumer/Producer bit by checking IORESOURCE_WINDOW. To be spec
> > compliant, we would have to use Extended descriptors for all bridge
> > windows, even if they would fit in a DWord or QWord.
> >
> > Should we do that? I dunno. I'd like to hear your opinion(s).
> >
>
> Yes, I think we should. If the spec allows for a way for a PNP0A03
> device to describe all of its resources unambiguously, we should not
> be relying on workarounds that were designed for another architecture
> in another decade (for, presumably, another OS)
That was the idea I floated at LPC16. We can override the
acpi_pci_root_ops prepare_resources() function pointer with a function
that checks IORESOURCE_WINDOW and filters resources accordingly (and
specific quirk "drivers" may know how to intepret resources that aren't
IORESOURCE_WINDOW - ie they can use it to describe the PCI ECAM config
space quirk region in their _CRS).
In a way that's something that makes sense anyway because given
that we are starting from a clean slate on ARM64 considering resources
that are not IORESOURCE_WINDOW as host bridge windows is just something
we are inheriting from x86, it is not really ACPI specs compliant (is
it ?).
> Just for my understanding, we will need to use extended descriptors
> for all consumed *and* produced regions, even though dword/qword are
> implicitly produced-only, due to the fact that the bit is ignored?
That's something that has to be clarified within the ASWG ie why the
consumer bit is ignored for *some* descriptors and not for others.
As things stand unfortunately the answer seems yes (I do not know
why).
> > It *would* be nice to have bridge registers in the bridge _CRS. That
> > would eliminate the need for looking up the HISI0081/PNP0C02 devices
> > to find the bridge registers. Avoiding that lookup is only a
> > temporary advantage -- the next round of bridges are supposed to fully
> > implement ECAM, and then we won't need to know where the registers
> > are.
> >
> > Apart from the lookup, there's still some advantage in describing the
> > registers in the PNP0A03 device instead of an unrelated PNP0C02
> > device, because it makes /proc/iomem more accurate and potentially
> > makes host bridge hotplug cleaner. We would have to enhance the host
> > bridge driver to do the reservations currently done by pnp/system.c.
> >
> > There's some value in doing it the same way as on x86, even though
> > that way is somewhat broken.
> >
> > Whatever we decide, I think it's very important to get it figured out
> > ASAP because it affects the ECAM quirks that we're trying to merge in
> > v4.10.
> >
>
> I agree. What exactly is the impact for the quirks mechanism as proposed?
The impact is that we could just use the PNP0A03 _CRS to report the PCI
ECAM config space quirk region through a consumer resource keeping in
mind what I say above (actually I think that's what was done on APM
firmware initially, for the records).
Lorenzo
^ permalink raw reply
* [PATCH v3 1/3] bus: da8xx-mstpri: drop the call to of_flat_dt_get_machine_name()
From: Sekhar Nori @ 2016-11-23 12:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479899187-10199-2-git-send-email-bgolaszewski@baylibre.com>
Hi Bartosz,
On Wednesday 23 November 2016 04:36 PM, Bartosz Golaszewski wrote:
> In order to avoid a section mismatch use a locally implemented routine
> instead of of_flat_dt_get_machine_name() when printing the error
> message.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
> drivers/bus/da8xx-mstpri.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bus/da8xx-mstpri.c b/drivers/bus/da8xx-mstpri.c
> index 85f0b53..064eeb9 100644
> --- a/drivers/bus/da8xx-mstpri.c
> +++ b/drivers/bus/da8xx-mstpri.c
> @@ -16,7 +16,6 @@
> #include <linux/platform_device.h>
> #include <linux/io.h>
> #include <linux/regmap.h>
> -#include <linux/of_fdt.h>
>
> /*
> * REVISIT: Linux doesn't have a good framework for the kind of performance
> @@ -190,6 +189,26 @@ static const struct da8xx_mstpri_board_priorities da8xx_mstpri_board_confs[] = {
> },
> };
>
> +/*
> + * FIXME Remove this function once of/base gets a general routine for getting
> + * the machine model/compatible string.
> + */
> +static const char *da8xx_mstpri_machine_get_compatible(void)
> +{
> + struct device_node *root;
> + const char *compatible;
> + int ret = -1;
> +
> + root = of_find_node_by_path("/");
> + if (root) {
> + ret = of_property_read_string_index(root, "compatible",
> + 0, &compatible);
> + of_node_put(root);
> + }
> +
> + return ret ? NULL : compatible;
> +}
As I just noted in the thread for v1 of this patch, calling
of_node_put(root) while keeping a reference to its compatible property
for later use sounds really broken.
I think it is safest to fix this by not including the compatible name in
error message at all. The error message will be little less descriptive,
but thats better than adding questionable code.
Thats what Frank suggested first up, but I did not realize at that time
that printing compatible name will be this much effort.
Can you please send a v4?
Thanks,
Sekhar
^ permalink raw reply
* [PATCH v2 1/2] ARM64: dts: Add support for Meson GXM
From: Martin Blumenstingl @ 2016-11-23 12:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161122100046.25899-2-narmstrong@baylibre.com>
On Tue, Nov 22, 2016 at 11:00 AM, Neil Armstrong
<narmstrong@baylibre.com> wrote:
> Following the Amlogic Linux kernel, it seem the only differences
> between the GXL and GXM SoCs are the CPU Clusters.
>
> This commit renames the gxl-s905d-p23x DTSI in a common file for
> S905D p23x and S912 q20x boards.
>
> Then adds a meson-gxm dtsi and reproduce the P23x to Q20x boards
> dts files since the S905D and S912 SoCs shares the same pinout
> and the P23x and Q20x boards are identical.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
I have tested this successfully on a Mecool BB2 (similar to the Q200
reference board).
^ permalink raw reply
* [PATCH v16 14/15] clocksource/drivers/arm_arch_timer: Add GTDT support for memory-mapped timer
From: Fu Wei @ 2016-11-23 12:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118202000.GP1197@leverpostej>
Hi Mark,
On 19 November 2016 at 04:20, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Nov 16, 2016 at 09:49:07PM +0800, fu.wei at linaro.org wrote:
>> From: Fu Wei <fu.wei@linaro.org>
>>
>> The patch add memory-mapped timer register support by using the
>> information provided by the new GTDT driver of ACPI.
>>
>> Signed-off-by: Fu Wei <fu.wei@linaro.org>
>> ---
>> drivers/clocksource/arm_arch_timer.c | 26 +++++++++++++++++++++++++-
>> 1 file changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
>> index c494ca8..0aad60a 100644
>> --- a/drivers/clocksource/arm_arch_timer.c
>> +++ b/drivers/clocksource/arm_arch_timer.c
>> @@ -1067,7 +1067,28 @@ CLOCKSOURCE_OF_DECLARE(armv7_arch_timer_mem, "arm,armv7-timer-mem",
>> arch_timer_mem_of_init);
>>
>> #ifdef CONFIG_ACPI_GTDT
>> -/* Initialize per-processor generic timer */
>> +static int __init arch_timer_mem_acpi_init(void)
>> +{
>> + struct arch_timer_mem *timer_mem;
>> + int ret = 0;
>> + int i = 0;
>> +
>> + timer_mem = kzalloc(sizeof(*timer_mem), GFP_KERNEL);
>
> Why do you need it zeroed? You don't clear it between iterations, so
> either you don't need this, or you have a bug in the loop.
>
>> + if (!timer_mem)
>> + return -ENOMEM;
>> +
>> + while (!gtdt_arch_timer_mem_init(timer_mem, i)) {
>
> Huh?
>
> Why doesn't GTDT expose a function to fill in the entire arch_timer_mem
> in one go?
Yes, that is a good idea, but I need to find a way to get the number of
GT block in GTDT, then allocate a struct arch_timer_mem array for all GT block
>
> There shouldn't be multiple instances, as far as I am aware. Am I
> mistaken?
I don't see any doc says : there is only one memory-mapped timer
in a soc.
Maybe we have more than one GT block in GTDT or
(in another word) we may have more than one memory-mapped
timer in a soc, right ??
Please correct me, If I miss something.
>
>> + ret = arch_timer_mem_init(timer_mem);
>> + if (ret)
>> + break;
>> + i++;
>> + }
>> +
>> + kfree(timer_mem);
>> + return ret;
>> +}
>
>
> Regardless, arch_timer_mem is small enough that you don't need dynamic
> allocaiton. Just put it on the stack, e.g.
>
> static int __init arch_timer_mem_acpi_init(void)
> {
> int i = 0;
> struct arch_timer_mem timer_mem;
>
> while (!gtdt_arch_timer_mem_init(timer_mem, i)) {
> int ret = arch_timer_mem_init();
> if (ret)
> return ret;
> i++
> }
>
> return 0;
> }
>
> Thanks,
> Mark.
--
Best regards,
Fu Wei
Software Engineer
Red Hat
^ permalink raw reply
* [PATCH 1/3] of: base: add support to get machine compatible string
From: Sudeep Holla @ 2016-11-23 12:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <11467504-c700-cbfa-a945-be9ec8776144@ti.com>
On 23/11/16 12:13, Sekhar Nori wrote:
> On Wednesday 23 November 2016 05:37 PM, Sudeep Holla wrote:
>>> So, the if(!of_node_get()) is just an expensive NULL pointer check. I
>>> think
>>> it is better to be explicit about it by not using of_node_get/put() at
>>> all.
>>> How about:
>>>
>>
>> Are we planning to use this in any time sensitive paths? Anyways I am
>> fine removing them.
>
> Not worried about the time taken as much as it serving as a bad example
> and getting carried over to other places where the impact might actually
> be real.
>
Ah OK, sure.
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH 3/3] arm64: dts: r8a7796: Add CAN FD support
From: Chris Paterson @ 2016-11-23 12:14 UTC (permalink / raw)
To: linux-arm-kernel
Adds CAN FD controller node for r8a7796.
Based on a patch for r8a7795 by Ramesh Shanmugasundaram.
Signed-off-by: Chris Paterson <chris.paterson2@renesas.com>
---
.../devicetree/bindings/net/can/rcar_canfd.txt | 12 ++++++-----
arch/arm64/boot/dts/renesas/r8a7796.dtsi | 24 ++++++++++++++++++++++
2 files changed, 31 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
index 22a6f10..788f273 100644
--- a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
+++ b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
@@ -5,6 +5,7 @@ Required properties:
- compatible: Must contain one or more of the following:
- "renesas,rcar-gen3-canfd" for R-Car Gen3 compatible controller.
- "renesas,r8a7795-canfd" for R8A7795 (R-Car H3) compatible controller.
+ - "renesas,r8a7796-canfd" for R8A7796 (R-Car M3) compatible controller.
When compatible with the generic version, nodes must list the
SoC-specific version corresponding to the platform first, followed by the
@@ -23,11 +24,12 @@ The name of the child nodes are "channel0" and "channel1" respectively. Each
child node supports the "status" property only, which is used to
enable/disable the respective channel.
-Required properties for "renesas,r8a7795-canfd" compatible:
-In R8A7795 SoC, canfd clock is a div6 clock and can be used by both CAN
-and CAN FD controller at the same time. It needs to be scaled to maximum
-frequency if any of these controllers use it. This is done using the
-below properties.
+Required properties for "renesas,r8a7795-canfd" and "renesas,r8a7796-canfd"
+compatible:
+In R8A7795 and R8A7796 SoCs, canfd clock is a div6 clock and can be used by both
+CAN and CAN FD controller at the same time. It needs to be scaled to maximum
+frequency if any of these controllers use it. This is done using the below
+properties:
- assigned-clocks: phandle of canfd clock.
- assigned-clock-rates: maximum frequency of this clock.
diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 95857fc..c069d77 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -392,6 +392,30 @@
status = "disabled";
};
+ canfd: can at e66c0000 {
+ compatible = "renesas,r8a7796-canfd",
+ "renesas,rcar-gen3-canfd";
+ reg = <0 0xe66c0000 0 0x8000>;
+ interrupts = <GIC_SPI 29 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 30 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD 914>,
+ <&cpg CPG_CORE R8A7796_CLK_CANFD>,
+ <&can_clk>;
+ clock-names = "fck", "canfd", "can_clk";
+ assigned-clocks = <&cpg CPG_CORE R8A7796_CLK_CANFD>;
+ assigned-clock-rates = <40000000>;
+ power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+ status = "disabled";
+
+ channel0 {
+ status = "disabled";
+ };
+
+ channel1 {
+ status = "disabled";
+ };
+ };
+
scif2: serial at e6e88000 {
compatible = "renesas,scif-r8a7796",
"renesas,rcar-gen3-scif", "renesas,scif";
--
1.9.1
^ permalink raw reply related
* [PATCH 2/3] arm64: dts: r8a7796: Add CAN support
From: Chris Paterson @ 2016-11-23 12:14 UTC (permalink / raw)
To: linux-arm-kernel
Adds CAN controller nodes for r8a7796.
Based on a patch for r8a7795 by Ramesh Shanmugasundaram.
Signed-off-by: Chris Paterson <chris.paterson2@renesas.com>
---
.../devicetree/bindings/net/can/rcar_can.txt | 12 +++++----
arch/arm64/boot/dts/renesas/r8a7796.dtsi | 30 ++++++++++++++++++++++
2 files changed, 37 insertions(+), 5 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/can/rcar_can.txt b/Documentation/devicetree/bindings/net/can/rcar_can.txt
index 8d40ab2..06bb7cc 100644
--- a/Documentation/devicetree/bindings/net/can/rcar_can.txt
+++ b/Documentation/devicetree/bindings/net/can/rcar_can.txt
@@ -10,6 +10,7 @@ Required properties:
"renesas,can-r8a7793" if CAN controller is a part of R8A7793 SoC.
"renesas,can-r8a7794" if CAN controller is a part of R8A7794 SoC.
"renesas,can-r8a7795" if CAN controller is a part of R8A7795 SoC.
+ "renesas,can-r8a7796" if CAN controller is a part of R8A7796 SoC.
"renesas,rcar-gen1-can" for a generic R-Car Gen1 compatible device.
"renesas,rcar-gen2-can" for a generic R-Car Gen2 compatible device.
"renesas,rcar-gen3-can" for a generic R-Car Gen3 compatible device.
@@ -24,11 +25,12 @@ Required properties:
- pinctrl-0: pin control group to be used for this controller.
- pinctrl-names: must be "default".
-Required properties for "renesas,can-r8a7795" compatible:
-In R8A7795 SoC, "clkp2" can be CANFD clock. This is a div6 clock and can be
-used by both CAN and CAN FD controller at the same time. It needs to be scaled
-to maximum frequency if any of these controllers use it. This is done using
-the below properties.
+Required properties for "renesas,can-r8a7795" and "renesas,can-r8a7796"
+compatible:
+In R8A7795 and R8A7796 SoCs, "clkp2" can be CANFD clock. This is a div6 clock
+and can be used by both CAN and CAN FD controller at the same time. It needs to
+be scaled to maximum frequency if any of these controllers use it. This is done
+using the below properties:
- assigned-clocks: phandle of clkp2(CANFD) clock.
- assigned-clock-rates: maximum frequency of this clock.
diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 91d716e..95857fc 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -362,6 +362,36 @@
status = "disabled";
};
+ can0: can at e6c30000 {
+ compatible = "renesas,can-r8a7796",
+ "renesas,rcar-gen3-can";
+ reg = <0 0xe6c30000 0 0x1000>;
+ interrupts = <GIC_SPI 186 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD 916>,
+ <&cpg CPG_CORE R8A7796_CLK_CANFD>,
+ <&can_clk>;
+ clock-names = "clkp1", "clkp2", "can_clk";
+ assigned-clocks = <&cpg CPG_CORE R8A7796_CLK_CANFD>;
+ assigned-clock-rates = <40000000>;
+ power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+ status = "disabled";
+ };
+
+ can1: can at e6c38000 {
+ compatible = "renesas,can-r8a7796",
+ "renesas,rcar-gen3-can";
+ reg = <0 0xe6c38000 0 0x1000>;
+ interrupts = <GIC_SPI 187 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD 915>,
+ <&cpg CPG_CORE R8A7796_CLK_CANFD>,
+ <&can_clk>;
+ clock-names = "clkp1", "clkp2", "can_clk";
+ assigned-clocks = <&cpg CPG_CORE R8A7796_CLK_CANFD>;
+ assigned-clock-rates = <40000000>;
+ power-domains = <&sysc R8A7796_PD_ALWAYS_ON>;
+ status = "disabled";
+ };
+
scif2: serial at e6e88000 {
compatible = "renesas,scif-r8a7796",
"renesas,rcar-gen3-scif", "renesas,scif";
--
1.9.1
^ permalink raw reply related
* [PATCH 1/3] arm64: dts: r8a7796: Add CAN external clock support
From: Chris Paterson @ 2016-11-23 12:14 UTC (permalink / raw)
To: linux-arm-kernel
Adds external CAN clock node for r8a7796. This clock can be used as
fCAN clock of CAN and CAN FD controller.
Based on a patch for r8a7795 by Ramesh Shanmugasundaram.
Signed-off-by: Chris Paterson <chris.paterson2@renesas.com>
---
arch/arm64/boot/dts/renesas/r8a7796.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/renesas/r8a7796.dtsi b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
index 41a050d..91d716e 100644
--- a/arch/arm64/boot/dts/renesas/r8a7796.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a7796.dtsi
@@ -69,6 +69,13 @@
clock-frequency = <0>;
};
+ /* External CAN clock - to be overridden by boards that provide it */
+ can_clk: can {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <0>;
+ };
+
/* External SCIF clock - to be overridden by boards that provide it */
scif_clk: scif {
compatible = "fixed-clock";
--
1.9.1
^ permalink raw reply related
* [PATCH 0/3] arm64: dts: r8a7796: Add CAN/CAN FD support
From: Chris Paterson @ 2016-11-23 12:14 UTC (permalink / raw)
To: linux-arm-kernel
This patch series adds CAN and CAN FD support to the r8a7796.
Based on renesas-devel-20161122-v4.9-rc6.
Chris Paterson (3):
arm64: dts: r8a7796: Add CAN external clock support
arm64: dts: r8a7796: Add CAN support
arm64: dts: r8a7796: Add CAN FD support
.../devicetree/bindings/net/can/rcar_can.txt | 12 +++--
.../devicetree/bindings/net/can/rcar_canfd.txt | 12 +++--
arch/arm64/boot/dts/renesas/r8a7796.dtsi | 61 ++++++++++++++++++++++
3 files changed, 75 insertions(+), 10 deletions(-)
--
1.9.1
^ permalink raw reply
* Tearing down DMA transfer setup after DMA client has finished
From: Måns Rullgård @ 2016-11-23 12:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <58356EA8.2010806@free.fr>
Mason <slash.tmp@free.fr> writes:
> Hello,
>
> On my platform, setting up a DMA transfer is a two-step process:
>
> 1) configure the "switch box" to connect a device to a memory channel
> 2) configure the transfer details (address, size, command)
>
> When the transfer is done, the sbox setup can be torn down,
> and the DMA driver can start another transfer.
>
> The current software architecture for my NFC (NAND Flash controller)
> driver is as follows (for one DMA transfer).
>
> sg_init_one
> dma_map_sg
> dmaengine_prep_slave_sg
> dmaengine_submit
> dma_async_issue_pending
> configure_NFC_transfer
> wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
> wait_for_NFC_idle
> dma_unmap_sg
>
> The problem is that the DMA driver tears down the sbox setup
> as soon as it receives the IRQ. However, when writing to the
> device, the interrupt only means "I have pushed all data from
> memory to the memory channel". These data have not reached
> the device yet, and may still be "in flight". Thus the sbox
> setup can only be torn down after the NFC is idle.
>
> How do I call back into the DMA driver after wait_for_NFC_idle,
> to request sbox tear down?
>
> The new architecture would become:
>
> sg_init_one
> dma_map_sg
> dmaengine_prep_slave_sg
> dmaengine_submit
> dma_async_issue_pending
> configure_NFC_transfer
> wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
> wait_for_NFC_idle
> request_sbox_tear_down /*** HOW TO DO THAT ***/
> dma_unmap_sg
>
> As far as I can tell, my NFC driver should call dmaengine_synchronize ??
> (In other words request_sbox_tear_down == dmaengine_synchronize)
>
> So the DMA driver should implement the device_synchronize hook,
> and tear the sbox down in that function.
>
> Is that correct? Or am I on the wrong track?
dmaengine_synchronize() is not meant for this. See the documentation:
/**
* dmaengine_synchronize() - Synchronize DMA channel termination
* @chan: The channel to synchronize
*
* Synchronizes to the DMA channel termination to the current context. When this
* function returns it is guaranteed that all transfers for previously issued
* descriptors have stopped and and it is safe to free the memory assoicated
* with them. Furthermore it is guaranteed that all complete callback functions
* for a previously submitted descriptor have finished running and it is safe to
* free resources accessed from within the complete callbacks.
*
* The behavior of this function is undefined if dma_async_issue_pending() has
* been called between dmaengine_terminate_async() and this function.
*
* This function must only be called from non-atomic context and must not be
* called from within a complete callback of a descriptor submitted on the same
* channel.
*/
This is for use after a dmaengine_terminate_async() call to wait for the
dma engine to finish whatever it was doing. This is not the problem
here. Your problem is that the dma engine interrupt fires before the
transfer is actually complete. Although you get an indication from the
target device when it has received all the data, there is no way to make
the dma driver wait for this.
--
M?ns Rullg?rd
^ permalink raw reply
* [PATCH 1/3] of: base: add support to get machine compatible string
From: Sekhar Nori @ 2016-11-23 12:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f0ba1a7a-8948-21c3-8a96-d46bf105bb7e@arm.com>
On Wednesday 23 November 2016 05:37 PM, Sudeep Holla wrote:
>> So, the if(!of_node_get()) is just an expensive NULL pointer check. I
>> think
>> it is better to be explicit about it by not using of_node_get/put() at
>> all.
>> How about:
>>
>
> Are we planning to use this in any time sensitive paths? Anyways I am
> fine removing them.
Not worried about the time taken as much as it serving as a bad example
and getting carried over to other places where the impact might actually
be real.
>
>> +int of_machine_get_model_name(const char **model)
>> +{
>> + int error;
>> +
>> + if (!of_root)
>> + return -EINVAL;
>> +
>> + error = of_property_read_string(of_root, "model", model);
>> + if (error)
>> + error = of_property_read_string_index(of_root,
>> "compatible",
>> + 0, model);
>> + return error;
>> +}
>> +EXPORT_SYMBOL(of_machine_get_model_name);
>>
>> I know the patch is already in -next so I guess it depends on how
>> strongly
>> Rob feels about this.
>
> Frank expressed his concerns and it may be reverted.
Didn't notice that. I will check that thread. Thanks!
Regards,
Sekhar
^ permalink raw reply
* [PATCH 1/3] of: base: add support to get machine compatible string
From: Sudeep Holla @ 2016-11-23 12:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <ff1ea88a-87c6-6721-8e63-ddb05900cc58@ti.com>
On 23/11/16 11:47, Sekhar Nori wrote:
> On Wednesday 23 November 2016 03:35 PM, Sudeep Holla wrote:
>>
>>
>> On 23/11/16 07:49, Sekhar Nori wrote:
>>> On Tuesday 22 November 2016 09:16 PM, Sudeep Holla wrote:
>>>> Hi Sekhar,
>>>>
>>>> On 22/11/16 15:06, Sekhar Nori wrote:
>>>>> Hi Sudeep,
>>>>>
>>>>> On Tuesday 22 November 2016 04:23 PM, Sudeep Holla wrote:
>>>>>>
>>>>>>
>>>>>> On 22/11/16 10:41, Bartosz Golaszewski wrote:
>>>>>>> Add a function allowing to retrieve the compatible string of the root
>>>>>>> node of the device tree.
>>>>>>>
>>>>>>
>>>>>> Rob has queued [1] and it's in -next today. You can reuse that if you
>>>>>> are planning to target this for v4.11 or just use open coding in your
>>>>>> driver for v4.10 and target this move for v4.11 to avoid cross tree
>>>>>> dependencies as I already mentioned in your previous thread.
>>>>>
>>>>> I dont have your original patch in my mailbox, but I wonder if
>>>>> returning a pointer to property string for a node whose reference has
>>>>> already been released is safe to do? Probably not an issue for the root
>>>>> node, but still feels counter-intuitive.
>>>>>
>>>>
>>>> I am not sure if I understand the issue here. Are you referring a case
>>>> where of_root is freed ?
>>>
>>> Yes, right, thats what I was hinting at. Since you are giving up the
>>> reference to the device node before the function returns, the user can
>>> be left with a dangling reference.
>>>
>>
>> Yes I agree.
>
> So, the if(!of_node_get()) is just an expensive NULL pointer check. I think
> it is better to be explicit about it by not using of_node_get/put() at all.
> How about:
>
Are we planning to use this in any time sensitive paths? Anyways I am
fine removing them.
> +int of_machine_get_model_name(const char **model)
> +{
> + int error;
> +
> + if (!of_root)
> + return -EINVAL;
> +
> + error = of_property_read_string(of_root, "model", model);
> + if (error)
> + error = of_property_read_string_index(of_root, "compatible",
> + 0, model);
> + return error;
> +}
> +EXPORT_SYMBOL(of_machine_get_model_name);
>
> I know the patch is already in -next so I guess it depends on how strongly
> Rob feels about this.
Frank expressed his concerns and it may be reverted.
--
Regards,
Sudeep
^ permalink raw reply
* [PATCH v2 1/3] ARM: davinci: da8xx: Fix ohci device name
From: Sekhar Nori @ 2016-11-23 12:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161103160308.29588-2-ahaslam@baylibre.com>
On Thursday 03 November 2016 09:33 PM, Axel Haslam wrote:
> While the clk lookup table is making reference to "ohci"
> other subsystems (such as phy) are trying to match "ohci.0"
>
> Since there is a single ohci instance, instead of changing
> the clk name, change the dev id to -1, and add the "-da8xx"
> postfix to match the driver name that will also be changed
> in a subsequent patch.
>
> Signed-off-by: Axel Haslam <ahaslam@baylibre.com>
Applied to v4.10/soc
Thanks,
Sekhar
^ permalink raw reply
* [PATCH v16 11/15] acpi/arm64: Add GTDT table parse driver
From: Fu Wei @ 2016-11-23 12:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118201250.GO1197@leverpostej>
Hi Mark,
On 19 November 2016 at 04:12, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Nov 16, 2016 at 09:49:04PM +0800, fu.wei at linaro.org wrote:
>
>> +#define for_each_platform_timer(_g) for (; _g; _g = next_platform_timer(_g))
>
> This doesn't fit the usual for_each_* pattern, since _g has to be
> manually initialised first. Either come up with a way of maknig this fit
> the usual pattern, or get rid of this, and use:
>
> t = however_you_get_the_first_timer();
>
> if (!t)
> bailt_out_somehow();
>
> do {
> ...
> } while (t = next_platform_timer(t));
Thanks, will do
>
>> +/*
>> + * Release the memory we have allocated in acpi_gtdt_init.
>> + * This should be called, when the driver who called "acpi_gtdt_init" previously
>> + * doesn't need the GTDT info anymore.
>> + */
>> +void __init acpi_gtdt_release(void)
>> +{
>> + kfree(timer_block);
>> + kfree(watchdog);
>> + timer_block = NULL;
>> + watchdog = NULL;
>> +}
>
> Why is this not simply in the error path of acpi_gtdt_init()?
>
>> +
>> +/*
>> + * Get some basic info from GTDT table, and init the global variables above
>> + * for all timers initialization of Generic Timer.
>> + * This function does some validation on GTDT table.
>> + */
>> +int __init acpi_gtdt_init(struct acpi_table_header *table)
>> +{
>
>> + timer_block = kcalloc(timer_count,
>> + sizeof(struct acpi_gtdt_timer_block *),
>> + GFP_KERNEL);
>> + if (!timer_block)
>> + return -ENOMEM;
>> +
>> + watchdog = kcalloc(timer_count, sizeof(struct acpi_gtdt_watchdog *),
>> + GFP_KERNEL);
>> + if (!watchdog) {
>> + kfree(timer_block);
>> + timer_block = NULL;
>> + return -ENOMEM;
>> + }
>
> Please have a common error path below, and branch to that when you need
> to free these.
OK , will do
>
>> +error:
>> + acpi_gtdt_release();
>> + return -EINVAL;
>> +}
>
> [...]
>
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index 61a3d90..a1611d1 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -577,6 +577,13 @@ enum acpi_reconfig_event {
>> int acpi_reconfig_notifier_register(struct notifier_block *nb);
>> int acpi_reconfig_notifier_unregister(struct notifier_block *nb);
>>
>> +#ifdef CONFIG_ACPI_GTDT
>> +int acpi_gtdt_init(struct acpi_table_header *table);
>> +int acpi_gtdt_map_ppi(int type);
>> +bool acpi_gtdt_c3stop(int type);
>> +void acpi_gtdt_release(void);
>
> Why do these need to be here?
>
> What possible value is ther in exporting acpi_gtdt_release() !?
because I need to release these after mem timer init,
But like your comment in 14/15, if I fill in the entire arch_timer_mem
in one go, I don't need to export acpi_gtdt_release.
Will try this way in v17
>
> Thanks,
> Mark.
--
Best regards,
Fu Wei
Software Engineer
Red Hat
^ permalink raw reply
* [PATCH v16 13/15] acpi/arm64: Add memory-mapped timer support in GTDT driver
From: Fu Wei @ 2016-11-23 11:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161118142249.GA18029@red-moon>
Hi Lorenzo,
On 18 November 2016 at 22:22, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Wed, Nov 16, 2016 at 09:49:06PM +0800, fu.wei at linaro.org wrote:
>> From: Fu Wei <fu.wei@linaro.org>
>>
>> On platforms booting with ACPI, architected memory-mapped timers'
>> configuration data is provided by firmware through the ACPI GTDT
>> static table.
>>
>> The clocksource architected timer kernel driver requires a firmware
>> interface to collect timer configuration and configure its driver.
>> this infrastructure is present for device tree systems, but it is
>> missing on systems booting with ACPI.
>>
>> Implement the kernel infrastructure required to parse the static
>> ACPI GTDT table so that the architected timer clocksource driver can
>> make use of it on systems booting with ACPI, therefore enabling
>> the corresponding timers configuration.
>>
>> Signed-off-by: Fu Wei <fu.wei@linaro.org>
>> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
>> ---
>> drivers/acpi/arm64/gtdt.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/acpi.h | 1 +
>> 2 files changed, 96 insertions(+)
>>
>> diff --git a/drivers/acpi/arm64/gtdt.c b/drivers/acpi/arm64/gtdt.c
>> index 2de79aa..08d9506 100644
>> --- a/drivers/acpi/arm64/gtdt.c
>> +++ b/drivers/acpi/arm64/gtdt.c
>> @@ -51,6 +51,14 @@ static inline bool is_timer_block(void *platform_timer)
>> return gh->type == ACPI_GTDT_TYPE_TIMER_BLOCK;
>> }
>>
>> +static inline struct acpi_gtdt_timer_block *get_timer_block(unsigned int index)
>> +{
>> + if (index >= acpi_gtdt_desc.timer_block_count || !timer_block)
>> + return NULL;
>> +
>> + return timer_block[index];
>> +}
>> +
>> static inline bool is_watchdog(void *platform_timer)
>> {
>> struct acpi_gtdt_header *gh = platform_timer;
>> @@ -214,3 +222,90 @@ int __init acpi_gtdt_init(struct acpi_table_header *table)
>> acpi_gtdt_release();
>> return -EINVAL;
>> }
>> +
>> +/*
>> + * Get ONE GT block info for memory-mapped timer from GTDT table.
>> + * @data: the GT block data (parsing result)
>> + * @index: the index number of GT block
>> + * Note: we already verify @data in caller, it can't be NULL here.
>> + * Returns 0 if success, -EINVAL/-ENODEV if error.
>> + */
>
> Documentation/kernel-doc-nano-HOWTO.txt
Great thanks , will fix all the comments :-)
>
>> +int __init gtdt_arch_timer_mem_init(struct arch_timer_mem *data,
>> + unsigned int index)
>
> Nit: acpi_arch_timer_mem_init() to make it consistent with other ACPI
> calls ?
yes, it makes sense, will do this way for the function which is exported
>
>> +{
>> + struct acpi_gtdt_timer_block *block;
>> + struct acpi_gtdt_timer_entry *frame;
>> + int i;
>> +
>> + block = get_timer_block(index);
>> + if (!block)
>> + return -ENODEV;
>> +
>> + if (!block->timer_count) {
>> + pr_err(FW_BUG "GT block present, but frame count is zero.");
>> + return -ENODEV;
>> + }
>> +
>> + if (block->timer_count > ARCH_TIMER_MEM_MAX_FRAMES) {
>> + pr_err(FW_BUG "GT block lists %d frames, ACPI spec only allows 8\n",
>> + block->timer_count);
>> + return -EINVAL;
>> + }
>
> Nit: these two checks can be carried out at GTDT init where the GTDT
> is parsed first. Actually you could have two functions one to init
this check is the verify the data in GT block, but the GTDT init
won't get into the block,
so I think that is better to keep this in GT block init function
"acpi_arch_timer_mem_init()"
> timers (say acpi_gtdt_timers_init()) and one watchdogs, that would
> eliminate the duplicated timer_block/watchdog arrays (both sized
> Platform Timer Count) and acpi_gtdt_timers_init() can return
> the number of entries found so that you can loop with a determined
> upper bound in the arm arch timer driver.
Yes, I did this way in previous patchset, but we still need to do scan
loop in both functions for
two types of platform timer.
So I decide to keep the scan loop in the acpi_gtdt_init to get the
number and the entries pointer
of each type of platform timers in one go. then we don't need to scan
GTDT in two functions.
>
> Just thinking aloud, these are just improvements I can carry them
> out later, the more important question here is the interface between the
> parsing code and the arch timer probing code which depends on other
> patches and that needs to be agreed, this patch is not really a problem.
yes, every time I modify the interface I have to change the driver :-(
>
>> + data->cntctlbase = (phys_addr_t)block->block_address;
>> + /*
>> + * We can NOT get the size info from GTDT table,
>> + * but according to "Table * CNTCTLBase memory map" of
>> + * <ARM Architecture Reference Manual> for ARMv8,
>> + * it should be 4KB(Offset 0x000 ? 0xFFC).
>
> That's the reason why it is not explicit in the GTDT table :)
>
>> + data->size = SZ_4K;
>> + data->num_frames = block->timer_count;
>> +
>> + frame = (void *)block + block->timer_offset;
>> + if (frame + block->timer_count != (void *)block + block->header.length)
>> + return -EINVAL;
>> +
>> + /*
>> + * Get the GT timer Frame data for every GT Block Timer
>> + */
>> + for (i = 0; i < block->timer_count; i++, frame++) {
>> + if (!frame->base_address || !frame->timer_interrupt)
>> + return -EINVAL;
>> +
>> + data->frame[i].phys_irq = map_gt_gsi(frame->timer_interrupt,
>> + frame->timer_flags);
>> + if (data->frame[i].phys_irq <= 0) {
>> + pr_warn("failed to map physical timer irq in frame %d.\n",
>> + i);
>> + return -EINVAL;
>> + }
>> +
>> + if (frame->virtual_timer_interrupt) {
>> + data->frame[i].virt_irq =
>> + map_gt_gsi(frame->virtual_timer_interrupt,
>> + frame->virtual_timer_flags);
>> + if (data->frame[i].virt_irq <= 0) {
>> + pr_warn("failed to map virtual timer irq in frame %d.\n",
>> + i);
>> + return -EINVAL;
>
> You should unregister phys_irq here for correctness, right ?
yup, thanks for pointing this bug out. :-)
>
>> + }
>> + }
>> +
>> + data->frame[i].frame_nr = frame->frame_number;
>> + data->frame[i].cntbase = frame->base_address;
>> + /*
>> + * We can NOT get the size info from GTDT table,
>> + * but according to "Table * CNTBaseN memory map" of
>> + * <ARM Architecture Reference Manual> for ARMv8,
>> + * it should be 4KB(Offset 0x000 ? 0xFFC).
>
> See above.
yes, you are right, I will fix both comments
>
>> + */
>> + data->frame[i].size = SZ_4K;
>> + }
>> +
>> + if (acpi_gtdt_desc.timer_block_count)
>> + pr_info("parsed No.%d of %d memory-mapped timer block(s).\n",
>> + index, acpi_gtdt_desc.timer_block_count);
>
> I am not sure how helpful this message can be honestly.
yup, I will simplify it :-)
>
>> +
>> + return 0;
>> +}
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index a1611d1..44b8c1b 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -582,6 +582,7 @@ int acpi_gtdt_init(struct acpi_table_header *table);
>> int acpi_gtdt_map_ppi(int type);
>> bool acpi_gtdt_c3stop(int type);
>> void acpi_gtdt_release(void);
>> +int gtdt_arch_timer_mem_init(struct arch_timer_mem *data, unsigned int index);
>
> See above.
>
> Overall it looks fine as long as the interface with clocksource is
> settled, which is what really needs to be agreed upon in this series.
Thanks for your help :-)
>
> Lorenzo
>
>> #endif
>>
>> #else /* !CONFIG_ACPI */
>> --
>> 2.7.4
>>
--
Best regards,
Fu Wei
Software Engineer
Red Hat
^ permalink raw reply
* [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
From: Arnd Bergmann @ 2016-11-23 11:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <bd4944d8-3cef-b3f7-4fba-6de590493047@gmail.com>
On Wednesday, November 23, 2016 8:38:00 PM CET Milo Kim wrote:
> On 11/23/2016 12:57 AM, Lee Jones wrote:
> >> pwrbutton {
> >> > - interrupts = <TPS65217_IRQ_PB>;
> >> > + interrupts = <TPS65217_IRQ_PUSHBUTTON>;
> > Push button or power button?
> >
>
> According to the datasheet, push button interrupt is correct.
>
> http://www.ti.com/lit/ds/symlink/tps65217.pdf
>
> This is used for a power button input in Beaglebone boards. In other
> words, the power button is one of push button usages.
>
> So, I'd like to keep general name for the interrupt.
Ah, the numbers come from the data sheet. Please just remove the
header then, there is no need to keep them as an ABI, in particular
when the driver doesn't even include that header today.
I see you even have names for them in the node:
charger {
- interrupts = <TPS65217_IRQ_AC>, <TPS65217_IRQ_USB>;
+ interrupts = <TPS65217_IRQ_AC_POWER>, <TPS65217_IRQ_USB_POWER>;
interrupts-names = "AC", "USB";
status = "okay";
What matters here is the binding documentation in
Documentation/devicetree/bindings/power/supply/tps65217_charger.txt
Please fix that instead to mandate that the two interrupts are
required, what their functions are, and what the interrupt-names
(not interrupts-names) property must list.
Arnd
^ permalink raw reply
* [PATCH 1/3] of: base: add support to get machine compatible string
From: Sekhar Nori @ 2016-11-23 11:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <2a644b8c-d91e-5ab1-200b-00f749a36307@arm.com>
On Wednesday 23 November 2016 03:35 PM, Sudeep Holla wrote:
>
>
> On 23/11/16 07:49, Sekhar Nori wrote:
>> On Tuesday 22 November 2016 09:16 PM, Sudeep Holla wrote:
>>> Hi Sekhar,
>>>
>>> On 22/11/16 15:06, Sekhar Nori wrote:
>>>> Hi Sudeep,
>>>>
>>>> On Tuesday 22 November 2016 04:23 PM, Sudeep Holla wrote:
>>>>>
>>>>>
>>>>> On 22/11/16 10:41, Bartosz Golaszewski wrote:
>>>>>> Add a function allowing to retrieve the compatible string of the root
>>>>>> node of the device tree.
>>>>>>
>>>>>
>>>>> Rob has queued [1] and it's in -next today. You can reuse that if you
>>>>> are planning to target this for v4.11 or just use open coding in your
>>>>> driver for v4.10 and target this move for v4.11 to avoid cross tree
>>>>> dependencies as I already mentioned in your previous thread.
>>>>
>>>> I dont have your original patch in my mailbox, but I wonder if
>>>> returning a pointer to property string for a node whose reference has
>>>> already been released is safe to do? Probably not an issue for the root
>>>> node, but still feels counter-intuitive.
>>>>
>>>
>>> I am not sure if I understand the issue here. Are you referring a case
>>> where of_root is freed ?
>>
>> Yes, right, thats what I was hinting at. Since you are giving up the
>> reference to the device node before the function returns, the user can
>> be left with a dangling reference.
>>
>
> Yes I agree.
So, the if(!of_node_get()) is just an expensive NULL pointer check. I think
it is better to be explicit about it by not using of_node_get/put() at all.
How about:
+int of_machine_get_model_name(const char **model)
+{
+ int error;
+
+ if (!of_root)
+ return -EINVAL;
+
+ error = of_property_read_string(of_root, "model", model);
+ if (error)
+ error = of_property_read_string_index(of_root, "compatible",
+ 0, model);
+ return error;
+}
+EXPORT_SYMBOL(of_machine_get_model_name);
I know the patch is already in -next so I guess it depends on how strongly
Rob feels about this.
Thanks,
Sekhar
^ permalink raw reply
* Synopsys Ethernet QoS Driver
From: Joao Pinto @ 2016-11-23 11:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <899DC02E-84BB-489E-A1FE-5D8F3BB795B6@axis.com>
On 23-11-2016 11:41, Lars Persson wrote:
>
>> 23 nov. 2016 kl. 12:11 skrev Joao Pinto <Joao.Pinto@synopsys.com>:
>>
>> Hi Peppe and Lars,
>>
>>> On 23-11-2016 10:59, Giuseppe CAVALLARO wrote:
>>> Hello Joao, Lars.
>>>
>>>> On 11/22/2016 3:16 PM, Joao Pinto wrote:
>>>>>> Ok, it makes sense.
>>>>>> Just for curiosity the target setup is the following:
>>>>>> https://www.youtube.com/watch?v=8V-LB5y2Cos
>>>>>> but instead of using internal drivers, we desire to use mainline drivers only.
>>>>>>
>>>>>> Thanks!
>>>> Regarding this subject, I am thinking of making the following adaption:
>>>>
>>>> a) delete ethernet/synopsys
>>>> b) rename ethernet/stmicro/stmmac to ethernet/synopsys
>>>>
>>>> and send you a patch for you to evaluate. Both agree with the approach?
>>>> To have a new work base would be important, because I will add to the "new"
>>>> structure some missing QoS features like Multichannel support, CBS and later TSN.
>>>
>>> IMO, we have to agree on a common strategy making the change for
>>> net-next; I imaged the following steps:
>>
>> Yes it makes totally sense.
>>
>>>
>>> - to port missing feature or fixes from ethernet/synopsys
>>> inside the stmmac taking care about the documentation too.
>>
>> @Lars: You are familiar with the synopsys qos driver. Could you please do this
>> porting. You can also make an analysis of what to port and I can do the porting
>> for you if you don't have the availability for it.
>
> As my main duty is changing diapers until March next year, please go ahead with this step if you can spend time on it before I am back in office.
Congratulations :)!
>
> Rabin Vincent can review and test that the port works properly on our Artpec-chips that use dwc_eth_qos.c today.
>
> The main porting step is to implement the device tree binding in bindings/net/snps,dwc-qos-ethernet.txt. Also our chip has a strict requirement that the phy is enabled when the SWR reset bit is set (it needs a tx clock to complete the reset).
>
> - Lars
Ok, I will do the task.
@Peppe: Agree with the plan?
>
>>
>>> - remove ethernet/synopsys
>>> - rename ethernet/stmicro/stmmac to ethernet/synopsys
>>
>> I volunteer to do this task.
>>
>>>
>>> These latest two have some relevant impacts.
>>>
>>> This change should be propagated to all the platforms that are using:
>>> CONFIG_SYNOPSYS_DWC_ETH_QOS and CONFIG_STMMAC_ETH
>>> plus device-tree compatibility.
>>
>> I volunteer to do this task also.
>>
>>>
>>> - enhance the stmmac with new features and new glue (part of these
>>> can be anticipated for sure).
>>
>> I have to implement 3 new features for now, but I will take some time for it, so
>> I would suggest to make the previous task and incrementally add features.
>>
>>>
>>> what do you think? does it make sense? If yes, we can also
>>> understand how/who starts.
>>>
>>> Regards,
>>> Peppe
>>
>> Thanks and regards.
>>
>> Joao
>>
>>>
>>>> Thanks.
>>>
>>
^ permalink raw reply
* [GIT PULL] Second Round of Renesas ARM Based SoC Drivers Updates for v4.10
From: Arnd Bergmann @ 2016-11-23 11:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161119013551.GB2543@localhost>
On Friday, November 18, 2016 5:35:51 PM CET Olof Johansson wrote:
>
> So, this pull request contains 8 patches, not 4. Seems like your pull
> request doesn't show any of the code from Geert's branch, didn't mention
> it in the tag and only in the email text above. Furthermore, Geert's
> branch modifies driver core code, so it's extra important to make sure
> it's clear that it's an unusual pull request.
>
> Given that this modifies driver core, please either merge that code
> through Greg first, or get an ack from him. If you merge through him,
> make sure it's on a standalone topic branch that we can share.
As discussed on IRC, I think it makes sense to track that branch
separately in arm-soc, as its own top-level next/* branch.
The branch was first merged into the mmc tree after we got consensus
on the approach.
Arnd
^ permalink raw reply
* Synopsys Ethernet QoS Driver
From: Lars Persson @ 2016-11-23 11:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <7c259adb-5c73-f997-6b96-5be427157b08@synopsys.com>
> 23 nov. 2016 kl. 12:11 skrev Joao Pinto <Joao.Pinto@synopsys.com>:
>
> Hi Peppe and Lars,
>
>> On 23-11-2016 10:59, Giuseppe CAVALLARO wrote:
>> Hello Joao, Lars.
>>
>>> On 11/22/2016 3:16 PM, Joao Pinto wrote:
>>>>> Ok, it makes sense.
>>>>> Just for curiosity the target setup is the following:
>>>>> https://www.youtube.com/watch?v=8V-LB5y2Cos
>>>>> but instead of using internal drivers, we desire to use mainline drivers only.
>>>>>
>>>>> Thanks!
>>> Regarding this subject, I am thinking of making the following adaption:
>>>
>>> a) delete ethernet/synopsys
>>> b) rename ethernet/stmicro/stmmac to ethernet/synopsys
>>>
>>> and send you a patch for you to evaluate. Both agree with the approach?
>>> To have a new work base would be important, because I will add to the "new"
>>> structure some missing QoS features like Multichannel support, CBS and later TSN.
>>
>> IMO, we have to agree on a common strategy making the change for
>> net-next; I imaged the following steps:
>
> Yes it makes totally sense.
>
>>
>> - to port missing feature or fixes from ethernet/synopsys
>> inside the stmmac taking care about the documentation too.
>
> @Lars: You are familiar with the synopsys qos driver. Could you please do this
> porting. You can also make an analysis of what to port and I can do the porting
> for you if you don't have the availability for it.
As my main duty is changing diapers until March next year, please go ahead with this step if you can spend time on it before I am back in office.
Rabin Vincent can review and test that the port works properly on our Artpec-chips that use dwc_eth_qos.c today.
The main porting step is to implement the device tree binding in bindings/net/snps,dwc-qos-ethernet.txt. Also our chip has a strict requirement that the phy is enabled when the SWR reset bit is set (it needs a tx clock to complete the reset).
- Lars
>
>> - remove ethernet/synopsys
>> - rename ethernet/stmicro/stmmac to ethernet/synopsys
>
> I volunteer to do this task.
>
>>
>> These latest two have some relevant impacts.
>>
>> This change should be propagated to all the platforms that are using:
>> CONFIG_SYNOPSYS_DWC_ETH_QOS and CONFIG_STMMAC_ETH
>> plus device-tree compatibility.
>
> I volunteer to do this task also.
>
>>
>> - enhance the stmmac with new features and new glue (part of these
>> can be anticipated for sure).
>
> I have to implement 3 new features for now, but I will take some time for it, so
> I would suggest to make the previous task and incrementally add features.
>
>>
>> what do you think? does it make sense? If yes, we can also
>> understand how/who starts.
>>
>> Regards,
>> Peppe
>
> Thanks and regards.
>
> Joao
>
>>
>>> Thanks.
>>
>
^ permalink raw reply
* [PATCH] dt-bindings: mfd: Improve readability for TPS65217 interrupt sources
From: Milo Kim @ 2016-11-23 11:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161122155759.GG10134@dell.home>
On 11/23/2016 12:57 AM, Lee Jones wrote:
>> pwrbutton {
>> > - interrupts = <TPS65217_IRQ_PB>;
>> > + interrupts = <TPS65217_IRQ_PUSHBUTTON>;
> Push button or power button?
>
According to the datasheet, push button interrupt is correct.
http://www.ti.com/lit/ds/symlink/tps65217.pdf
This is used for a power button input in Beaglebone boards. In other
words, the power button is one of push button usages.
So, I'd like to keep general name for the interrupt.
Best regards,
Milo
^ permalink raw reply
* [PATCH] mfd: twl-core: export twl_get_regmap
From: Rosia, Nicolae @ 2016-11-23 11:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161123111223.GG23750@n2100.armlinux.org.uk>
Hello,
On Wed, 2016-11-23 at 11:12 +0000, Russell King - ARM Linux wrote:
> I was just curious, and I took a peek at the OMAP/TWL DT files, and
> I see that it's left to DT to create the children.
I'm converting the driver to use mfd_add_devices and
mfd_remove_devices, the subdrivers will access the parent's private
data which will remain valid since in the remove method we will be
calling mfd_remove_devices first.
After removing all global calls to twl-core.c methods, I will also get
rid of the "only one instance" of twl_priv and the "ready" flag.
Thanks for your input,
Nicolae
^ permalink raw reply
* [PATCH V9 11/11] ARM64/PCI: Support for ACPI based PCI host controller
From: Tomasz Nowicki @ 2016-11-23 11:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161122231321.GA20246@bhelgaas-glaptop.roam.corp.google.com>
Hi Bjorn,
On 23.11.2016 00:13, Bjorn Helgaas wrote:
> Hi Tomasz,
>
> On Fri, Jun 10, 2016 at 09:55:19PM +0200, Tomasz Nowicki wrote:
>> Implement pci_acpi_scan_root and other arch-specific call so that ARM64
>> can start using ACPI to setup and enumerate PCI buses.
>>
>> Prior to buses enumeration the pci_acpi_scan_root() implementation looks
>> for configuration space start address (obtained through ACPI _CBA method or
>> MCFG interface). If succeed, it uses ECAM library to create new mapping.
>> Then it attaches generic ECAM ops (pci_generic_ecam_ops) which are used
>> for accessing configuration space later on.
>> ...
>
>> +static struct acpi_pci_root_ops acpi_pci_root_ops = {
>> + .release_info = pci_acpi_generic_release_info,
>> +};
>> +
>> +/* Interface called from ACPI code to setup PCI host controller */
>> struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
>> {
>> - /* TODO: Should be revisited when implementing PCI on ACPI */
>> - return NULL;
>> + int node = acpi_get_node(root->device->handle);
>> + struct acpi_pci_generic_root_info *ri;
>> + struct pci_bus *bus, *child;
>> +
>> + ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
>> + if (!ri)
>> + return NULL;
>> +
>> + ri->cfg = pci_acpi_setup_ecam_mapping(root);
>> + if (!ri->cfg) {
>> + kfree(ri);
>> + return NULL;
>> + }
>> +
>> + acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
>
> This has already been merged, but this isn't right, is it? We're
> writing a host controller-specific pointer into the single system-wide
> acpi_pci_root_ops, then passing it on to acpi_pci_root_create().
>
> Today, I think ri->cfg->ops->pci_ops is always &pci_generic_ecam_ops,
> from this path:
>
> ri->cfg = pci_acpi_setup_ecam_mapping
> cfg = pci_ecam_create(..., &pci_generic_ecam_ops)
> cfg = kzalloc(...)
> cfg->ops = ops # &pci_generic_ecam_ops
>
> But we're about to merge the ECAM quirks series, which will mean it
> may not be &pci_generic_ecam_ops. Even apart from the ECAM quirks, we
> should avoid this pattern of putting device-specific info in a single
> shared structure because it's too difficult to verify that it's
> correct.
>
Well spotted. I agree, we need to fix this. How about this:
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index fb439c7..31c0e1c 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -152,33 +152,35 @@ static void pci_acpi_generic_release_info(struct
acpi_pci_root_info *ci)
ri = container_of(ci, struct acpi_pci_generic_root_info, common);
pci_ecam_free(ri->cfg);
+ kfree(ci->ops);
kfree(ri);
}
-static struct acpi_pci_root_ops acpi_pci_root_ops = {
- .release_info = pci_acpi_generic_release_info,
-};
-
/* Interface called from ACPI code to setup PCI host controller */
struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
{
int node = acpi_get_node(root->device->handle);
struct acpi_pci_generic_root_info *ri;
struct pci_bus *bus, *child;
+ struct acpi_pci_root_ops *root_ops;
ri = kzalloc_node(sizeof(*ri), GFP_KERNEL, node);
if (!ri)
return NULL;
+ root_ops = kzalloc_node(sizeof(*root_ops), GFP_KERNEL, node);
+ if (!root_ops)
+ return NULL;
+
ri->cfg = pci_acpi_setup_ecam_mapping(root);
if (!ri->cfg) {
kfree(ri);
+ kfree(root_ops);
return NULL;
}
- acpi_pci_root_ops.pci_ops = &ri->cfg->ops->pci_ops;
- bus = acpi_pci_root_create(root, &acpi_pci_root_ops, &ri->common,
- ri->cfg);
+ root_ops->release_info = pci_acpi_generic_release_info;
+ root_ops->pci_ops = &ri->cfg->ops->pci_ops;
+ bus = acpi_pci_root_create(root, root_ops, &ri->common, ri->cfg);
if (!bus)
return NULL;
Of course, this should be the part of ECAM quirks core patches.
The other option we have is to remove "struct pci_ops *pci_ops;" from
acpi_pci_root_ops structure and pass struct pci_ops as an extra argument
to acpi_pci_root_create(). What do you think?
Thanks,
Tomasz
^ permalink raw reply related
* [PATCH 0/3] TI DA850/OMAP-L138/AM18x pinconf
From: Sekhar Nori @ 2016-11-23 11:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479871767-20160-1-git-send-email-david@lechnology.com>
On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
> This series adds a new driver and DT bindings for TI DA850/OMAP-L138/AM18x
> pinconf (bias pullup/pulldown).
>
> The motivation for this series is LEGO MINDSTORMS EV3 support. It needs most,
> if not all, internal pullup/down resistors disabled in order to work correctly.
This looks really neat to my non-pinconf trained eyes. I have sent some
minor comments. But apart from that:
Reviewed-by: Sekhar Nori <nsekhar@ti.com>
3/3 should go through my tree. If the driver patches are going to get
queued for v4.10, I can queue the DTS portion through my tree.
Thanks,
Sekhar
^ permalink raw reply
* [PATCH 3/3] ARM: dts: da850: Add node for pullup/pulldown pinconf
From: Sekhar Nori @ 2016-11-23 11:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1479871767-20160-4-git-send-email-david@lechnology.com>
On Wednesday 23 November 2016 08:59 AM, David Lechner wrote:
> This SoC has a separate pin controller for configuring pullup/pulldown
> bias on groups of pins.
>
> Signed-off-by: David Lechner <david@lechnology.com>
> ---
> arch/arm/boot/dts/da850.dtsi | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 8945815..1c0224c 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -210,6 +210,11 @@
> };
>
> };
> + pinconf: pin-controller at 22c00c {
> + compatible = "ti,da850-pupd";
> + reg = <0x22c00c 0x8>;
> + status = "disabled";
> + };
Can you please place this below the i2c1 node. I am trying to keep the
nodes sorted by unit address. I know thats broken in many places today,
but lets add the new ones where they should eventually end up.
Thanks,
Sekhar
^ 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