* [PATCHv4 11/15] clk: ti: clockdomain: add clock provider support to clockdomains
From: Tony Lindgren @ 2016-12-03 0:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <148072274164.32158.6012452044533845688@resonance>
* Michael Turquette <mturquette@baylibre.com> [161202 15:52]:
> Quoting Tony Lindgren (2016-12-02 15:12:40)
> > * Michael Turquette <mturquette@baylibre.com> [161202 14:34]:
> > > Quoting Tony Lindgren (2016-10-28 16:54:48)
> > > > * Stephen Boyd <sboyd@codeaurora.org> [161028 16:37]:
> > > > > On 10/28, Tony Lindgren wrote:
> > > > > > * Tero Kristo <t-kristo@ti.com> [161028 00:43]:
> > > > > > > On 28/10/16 03:50, Stephen Boyd wrote:
> > > > > > > > I suppose a PRCM is
> > > > > > > > like an MFD that has clocks and resets under it? On other
> > > > > > > > platforms we've combined that all into one node and just had
> > > > > > > > #clock-cells and #reset-cells in that node. Is there any reason
> > > > > > > > we can't do that here?
> > > > > > >
> > > > > > > For OMAPs, there are typically multiple instances of the PRCM around; OMAP4
> > > > > > > for example has:
> > > > > > >
> > > > > > > cm1 @ 0x4a004000 (clocks + clockdomains)
> > > > > > > cm2 @ 0x4a008000 (clocks + clockdomains)
> > > > > > > prm @ 0x4a306000 (few clocks + resets + power state handling)
> > > > > > > scrm @ 0x4a30a000 (few external clocks + plenty of misc stuff)
> > > > > > >
> > > > > > > These instances are also under different power/voltage domains which means
> > > > > > > their PM behavior is different.
> > > > > > >
> > > > > > > The idea behind having a clockdomain as a provider was mostly to have the
> > > > > > > topology visible : prcm-instance -> clockdomain -> clocks
> > > > > >
> > > > > > Yeah that's needed to get the interconnect hierarchy right for
> > > > > > genpd :)
> > > > > >
> > > > > > > ... but basically I think it would be possible to drop the clockdomain
> > > > > > > representation and just mark the prcm-instance as a clock provider. Tony,
> > > > > > > any thoughts on that?
> > > > > >
> > > > > > No let's not drop the clockdomains as those will be needed when we
> > > > > > move things into proper hierarchy within the interconnect instances.
> > > > > > This will then help with getting things right with genpd.
> > > > > >
> > > > > > In the long run we just want to specify clockdomain and the offset of
> > > > > > the clock instance within the clockdomain in the dts files.
> > > > > >
> > > > >
> > > > > Sorry, I have very little idea how OMAP hardware works. Do you
> > > > > mean that you will have different nodes for each clockdomain so
> > > > > that genpd can map 1:1 to the node in dts? But in hardware
> > > > > there's a prcm that allows us to control many clock domains
> > > > > through register read/writes? How is the interconnect involved?
> > > >
> > > > There are multiple clockdomains, at least one for each interconnect
> > > > instance. Once a clockdomain is idle, the related interconnect can
> > > > idle too. So yeah genpd pretty much maps 1:1 with the clockdomains.
> > > >
> > > > There's more info in for example omap4 TRM section "3.4.1 Device
> > > > Power-Management Layout" that shows the voltage/power/clock domains.
> > > > The interconnect instances are mostly named there too looking at
> > > > the L4/L3 naming.
> > >
> > > I'm confused on two points:
> > >
> > > 1) why are the clkdm's acting as clock providers? I've always hated the
> > > name "clock domain" since those bits are for managing module state, not
> > > clock state. The PRM, CM1 and CM2 provide the clocks, not the
> > > clockdomains.
> >
> > The clock domains have multiple clock inputs that are routed to multiple
> > child clocks. So it is a clock :)
> >
> > See for example omap4430 TRM "3.6.4 CD_WKUP Clock Domain" on page
> > 393 in my revision here.
> >
> > On that page "Figure 3-48" shows CD_WKUP with the four input clocks.
> > And then "Table 3-84. CD_WKUP Control and Status Parameters" shows
> > the CD_WKUP clock domain specific registers. These registers show
> > the status, I think they are all read-only registers. Then CD_WKUP
> > has multiple child clocks with configurable registers.
> >
> > From hardware register point of view, each clock domain has:
> >
> > - Read-only clockdomain status registers in the beginning of
> > the address space
> >
> > - Multiple similar clock instances register instances each
> > mapping to a specific interconnect target module
> >
> > These are documented in "3.11.16.1 WKUP_CM Register Summary".
>
> Oh, this is because you are treating the MODULEMODE bits like gate
> clocks. I never really figured out if this was the best way to model
> those bits since they do more than control a line toggling at a rate.
> For instance this bit will affect the master/slave IDLE protocol between
> the module and the PRCM.
Yes seems like there is some negotiation going on there with the
target module. But from practical point of view the CLKCTRL
register is the gate for a module functional clock.
> > From hardware point of view, we ideally want to map interconnect
> > target modules to the clock instance offset from the clock domain
> > for that interconnect segment. For example gptimer1 clocks would
> > be just:
> >
> > clocks = <&cd_wkup 0x40>;
> >
> > > 2) why aren't the clock domains modeled as genpds with their associated
> > > devices attached to them? Note that it is possible to "nest" genpd
> > > objects. This would also allow for the "Clockdomain Dependency"
> > > relationships to be properly modeled (see section 3.1.1.1.7 Clock Domain
> > > Dependency in the OMAP4 TRM).
> >
> > Clock domains only route clocks to child clocks. Power domains
> > are different registers. The power domains map roughly to
> > interconnect instances, there we have registers to disable the
> > whole interconnect when idle.
>
> I'm not talking about power islands at all, but the genpd object in
> Linux. For instance, if we treat each clock domain like a clock
> provider, how could the functional dependency between clkdm_A and
> clkdm_B be asserted?
To me it seems that some output of a clockdomain is just a input
of another clockdomain? So it's just the usual parent child
relationship once we treat a clockdomain just as a clock. Tero
probably has some input here.
> There is certainly no API for that in the clock framework, but for genpd
> your runtime_pm_get() callback for clkdm_A could call runtime_pm_get
> against clkdm_B, which would satisfy the requirement. See section
> 3.1.1.1.7 Clock Domain Dependency in the OMAP4 TRM, version AB.
To me it seems the API is just clk_get() :) Do you have some
specific example we can use to check? My guess is that the
TRM "Clock Domain Dependency" is just the usual parent child
relationship between clocks that are the clockdomains..
If there is something more magical there certainly that should
be considered though.
Regards,
Tony
^ permalink raw reply
* [PATCH v3] PCI/ACPI: xgene: Add ECAM quirk for X-Gene PCIe controller
From: Jon Masters @ 2016-12-03 0:33 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161202233943.GF9903@bhelgaas-glaptop.roam.corp.google.com>
On 12/02/2016 06:39 PM, Bjorn Helgaas wrote:
> On Thu, Dec 01, 2016 at 11:08:23PM -0500, Jon Masters wrote:
>> Let's see if I summarized this correctly...
>>
>> 1. The MMIO registers for the host bridge itself need to be described
>> somewhere, especially if we need to find those in a quirk and poke
>> them. Since those registers are very much part of the bridge device,
>> it makes sense for them to be in the _CRS for PNP0A08/PNP0A03.
>>
>> 2. The address space covering these registers MUST be described as a
>> ResourceConsumer in order to avoid accidentally exposing them as
>> available for use by downstream devices on the PCI bus.
>>
>> 3. The ACPI specification allows for resources of the type "Memory32Fixed".
>> This is a macro that doesn't have the notion of a producer or consumer.
>> HOWEVER various interpretations seem to be that this could/should
>> default to being interpreted as a consumed region.
>
> I agree; I think that per spec, Memory24, Memory32, Memory32Fixed, IO,
> and FixedIO should all be for consumed resources, not for bridge
> windows, since they don't have the notion of producer.
Ok. If we ultimately codify this somewhere as the general Linux kernel
consensus (Rafael?) then we can also go and get the various ARM server
specs updated to reflect this in (for e.g.) reference firmware builds.
> I'm pretty sure there's x86 firmware in the field that uses these for
> windows, so I think we have to accept that usage, at least on x86.
Ok. I was pondering how to even go about finding that out, but even if
I scheduled a job across RH's infra to look, that would be a drop in
the bucket of possible machines that might be out there doing this.
<snip>
> Per spec, we should ignore the Consumer/Producer bit in Word/DWord/QWord
> descriptors. In bridge devices on x86, I think we have to treat them as
> producers (windows) because that's how they've been typically used.
Ok.
>> BUT if we were to do that, it would break existing shipping systems since
>> there are quirks out there that use this form to find the base CSR:
>>
>> if (acpi_res->type == ACPI_RESOURCE_TYPE_FIXED_MEMORY32) {
>> fixed32 = &acpi_res->data.fixed_memory32;
>> port->csr_base = ioremap(fixed32->address,
>> fixed32->address_length);
>> return AE_CTRL_TERMINATE;
>> }
>
> I think this is a valid usage of FixedMemory32 in terms of the spec.
> Linux currently handles this as a window if it appears in a PNP0A03
> device because some x86 firmware used it that way.
>
> We might be able to handle it differently on arm64, e.g., by making an
> arm64 version of pci_acpi_root_prepare_resources() that checks for
> IORESOURCE_WINDOW.
This is something we should figure out the consensus on and codify.
>> 2. What would happen if we had a difference policy on arm64 for such
>> resources. x86 has an "exception" for accessing the config space
>> using IO port 0xCF8-0xCFF (a fairly reasonable exception!) and
>> we can make the rules for a new platform (i.e. actually prescribe
>> exactly what the behavior is, rather than have it not be defined).
>> This is of course terrible in that existing BIOS vendors and so on
>> won't necessarily know this when working on ARM ACPI later on.
>
>> Indeed. And in the case of m400, it is currently this in shipping systems:
>>
>> Memory32Fixed (ReadWrite,
>> 0x1F500000, // Address Base
>> 0x00010000, // Address Length
>> )
>
>>>>> [ 0.822990] pci_bus 0000:00: root bus resource [mem 0x1f2b0000-0x1f2bffff]
>>>>
>>>> I think this is wrong. The PCI core thinks [mem 0x1f2b0000-0x1f2bffff]
>>>> is available for use by devices on bus 0000:00, but I think you're
>>>> saying it is consumed by the bridge itself, not forwarded down to PCI.
>
> I think this ASL is perfectly spec-compliant, and what's wrong is the
> way Linux is interpreting it.
>
> I'm not clear on what's terrible about idea 2. I think it's basically
> what I suggested above, i.e., something like the patch below, which I
> think (hope) would keep us from thinking that region is a window.
I was guarded because I like harmony between architectures (where it
makes sense). But that said, there is nothing to prevent having a
different interpretation on ARM, as long as everyone agrees on it.
> Even without this patch, I don't think it's a show-stopper to have
> Linux mistakenly thinking this region is routed to PCI, because the
> driver does reserve it and the PCI core will never try to use it.
Ok. So are you happy with pulling in Duc's v4 patch and retaining
status quo on the bridge resources for 4.10? We can continue to
discuss this and ultimately set a direction for the spec, as well
as clean up existing and future designs (certainly the latter) to
ensure all possible resources used by a platform are described
and consumed correctly, and hopefully live with the slightly
odd little bit of address space eaten up for that RC CSR :)
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index 8a177a1..a16fc8e 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -114,6 +114,19 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
> return 0;
> }
>
> +static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci)
> +{
> + struct resource_entry *entry, *tmp;
> + int status;
> +
> + status = acpi_pci_probe_root_resources(ci);
> + resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
> + if (!(entry->res->flags & IORESOURCE_WINDOW))
> + resource_list_destroy_entry(entry);
> + }
> + return status;
> +}
> +
> /*
> * Lookup the bus range for the domain in MCFG, and set up config space
> * mapping.
> @@ -190,6 +203,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
> }
>
> root_ops->release_info = pci_acpi_generic_release_info;
> + root_ops->prepare_resources = pci_acpi_root_prepare_resources;
> root_ops->pci_ops = &ri->cfg->ops->pci_ops;
> bus = acpi_pci_root_create(root, root_ops, &ri->common, ri->cfg);
> if (!bus)
>
I can give this patch a quick boot test a bit later.
Jon.
--
Computer Architect | Sent from my Fedora powered laptop
^ permalink raw reply
* [PATCH] clk: uniphier: Fix build with gcc-4.4.
From: Vinson Lee @ 2016-12-03 0:37 UTC (permalink / raw)
To: linux-arm-kernel
gcc-4.4 has issues with anonymous unions in initializers.
CC drivers/clk/uniphier/clk-uniphier-sys.o
drivers/clk/uniphier/clk-uniphier-sys.c:45: error: unknown field ?factor? specified in initializer
Fixes: 1574d5722636 ("clk: uniphier: remove unneeded member name for union")
Signed-off-by: Vinson Lee <vlee@freedesktop.org>
---
drivers/clk/uniphier/clk-uniphier-mio.c | 4 ++--
drivers/clk/uniphier/clk-uniphier.h | 12 ++++++------
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/uniphier/clk-uniphier-mio.c b/drivers/clk/uniphier/clk-uniphier-mio.c
index 4974d38..7441eeb 100644
--- a/drivers/clk/uniphier/clk-uniphier-mio.c
+++ b/drivers/clk/uniphier/clk-uniphier-mio.c
@@ -30,7 +30,7 @@
.name = "sd" #ch "-sel", \
.type = UNIPHIER_CLK_TYPE_MUX, \
.idx = -1, \
- .mux = { \
+ { .mux = { \
.parent_names = { \
"sd-44m", \
"sd-33m", \
@@ -63,7 +63,7 @@
0x00001200, \
0x00001300, \
}, \
- }, \
+ } }, \
}, \
UNIPHIER_CLK_GATE("sd" #ch, (_idx), "sd" #ch "-sel", 0x20 + 0x200 * (ch), 8)
diff --git a/drivers/clk/uniphier/clk-uniphier.h b/drivers/clk/uniphier/clk-uniphier.h
index 81d7e5c..8735a7d 100644
--- a/drivers/clk/uniphier/clk-uniphier.h
+++ b/drivers/clk/uniphier/clk-uniphier.h
@@ -81,12 +81,12 @@ struct uniphier_clk_data {
.name = (_name), \
.type = UNIPHIER_CLK_TYPE_CPUGEAR, \
.idx = (_idx), \
- .cpugear = { \
+ { .cpugear = { \
.parent_names = { __VA_ARGS__ }, \
.num_parents = (_num_parents), \
.regbase = (_regbase), \
.mask = (_mask) \
- }, \
+ } }, \
}
#define UNIPHIER_CLK_FACTOR(_name, _idx, _parent, _mult, _div) \
@@ -94,11 +94,11 @@ struct uniphier_clk_data {
.name = (_name), \
.type = UNIPHIER_CLK_TYPE_FIXED_FACTOR, \
.idx = (_idx), \
- .factor = { \
+ { .factor = { \
.parent_name = (_parent), \
.mult = (_mult), \
.div = (_div), \
- }, \
+ } }, \
}
#define UNIPHIER_CLK_GATE(_name, _idx, _parent, _reg, _bit) \
@@ -106,11 +106,11 @@ struct uniphier_clk_data {
.name = (_name), \
.type = UNIPHIER_CLK_TYPE_GATE, \
.idx = (_idx), \
- .gate = { \
+ { .gate = { \
.parent_name = (_parent), \
.reg = (_reg), \
.bit = (_bit), \
- }, \
+ } }, \
}
#define UNIPHIER_CLK_DIV(parent, div) \
--
2.7.4
^ permalink raw reply related
* [PATCH] clk: uniphier: Fix build with gcc-4.4.
From: Masahiro Yamada @ 2016-12-03 1:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480725436-9628-1-git-send-email-vlee@freedesktop.org>
Hi Vinson,
2016-12-03 9:37 GMT+09:00 Vinson Lee <vlee@freedesktop.org>:
> gcc-4.4 has issues with anonymous unions in initializers.
>
> CC drivers/clk/uniphier/clk-uniphier-sys.o
> drivers/clk/uniphier/clk-uniphier-sys.c:45: error: unknown field ?factor? specified in initializer
>
> Fixes: 1574d5722636 ("clk: uniphier: remove unneeded member name for union")
> Signed-off-by: Vinson Lee <vlee@freedesktop.org>
This driver has COMPILE_TEST option, but kbuild test robot
did not mention about this.
This is a bad way of fixing, I think.
(what if a new member is inserted before the union in the future?)
Rather, please revert the bad commit.
> .name = "sd" #ch "-sel", \
> .type = UNIPHIER_CLK_TYPE_MUX, \
> .idx = -1, \
> - .mux = { \
> + { .mux = { \
> .parent_names = { \
> "sd-44m", \
> "sd-33m", \
> @@ -63,7 +63,7 @@
> 0x00001200, \
> 0x00001300, \
> }, \
> - }, \
> + } }, \
> }, \
No, please do not do this.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* [PATCH v9 07/16] drivers: acpi: implement acpi_dma_configure
From: Rafael J. Wysocki @ 2016-12-03 2:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161202153816.GA18290@red-moon>
On Fri, Dec 2, 2016 at 4:38 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> Rafael, Mark, Suravee,
>
> On Mon, Nov 21, 2016 at 10:01:39AM +0000, Lorenzo Pieralisi wrote:
>> On DT based systems, the of_dma_configure() API implements DMA
>> configuration for a given device. On ACPI systems an API equivalent to
>> of_dma_configure() is missing which implies that it is currently not
>> possible to set-up DMA operations for devices through the ACPI generic
>> kernel layer.
>>
>> This patch fills the gap by introducing acpi_dma_configure/deconfigure()
>> calls that for now are just wrappers around arch_setup_dma_ops() and
>> arch_teardown_dma_ops() and also updates ACPI and PCI core code to use
>> the newly introduced acpi_dma_configure/acpi_dma_deconfigure functions.
>>
>> Since acpi_dma_configure() is used to configure DMA operations, the
>> function initializes the dma/coherent_dma masks to sane default values
>> if the current masks are uninitialized (also to keep the default values
>> consistent with DT systems) to make sure the device has a complete
>> default DMA set-up.
>
> I spotted a niggle that unfortunately was hard to spot (and should not
> be a problem per se but better safe than sorry) and I am not comfortable
> with it.
>
> Following commit d0562674838c ("ACPI / scan: Parse _CCA and setup
> device coherency") in acpi_bind_one() we check if the acpi_device
> associated with a device just added supports DMA, first it was
> done with acpi_check_dma() and then commit 1831eff876bd ("device
> property: ACPI: Make use of the new DMA Attribute APIs") changed
> it to acpi_get_dma_attr().
>
> The subsequent check (attr != DEV_DMA_NOT_SUPPORTED) is always true
> on _any_ acpi device we pass to acpi_bind_one() on x86, which was
> fine because we used it to call arch_setup_dma_ops(), which is a nop
> on x86. On ARM64 a _CCA method is required to define if a device
> supports DMA so (attr != DEV_DMA_NOT_SUPPORTED) may well be false.
>
> Now, acpi_bind_one() is used to bind an acpi_device to its physical
> node also for pseudo-devices like cpus and memory nodes. For those
> objects, on x86, attr will always be != DEV_DMA_NOT_SUPPORTED.
>
> So far so good, because on x86 arch_setup_dma_ops() is empty code.
>
> With this patch, I use the (attr != DEV_DMA_NOT_SUPPORTED) check
> to call acpi_dma_configure() which is basically a nop on x86 except
> that it sets up the dma_mask/coherent_dma_mask to a sane default value
> (after all we are setting up DMA for the device so it makes sense to
> initialize the masks there if they were unset since we are configuring
> DMA for the device in question) for the given device.
>
> Problem is, as per the explanation above, we are also setting the
> default dma masks for pseudo-devices (eg CPUs) that were previously
> untouched, it should not be a problem per-se but I am not comfortable
> with that, honestly it does not make much sense.
>
> An easy "fix" would be to move the default dma masks initialization out
> of acpi_dma_configure() (as it was in previous patch versions of this
> series - I moved it to acpi_dma_configure() just a consolidation point
> for initializing the masks instead of scattering them in every
> acpi_dma_configure caller) I can send this as a fix-up patch to Joerg if
> we think that's the right thing to do (or I can send it to Rafael later
> when the code is in the merged depending on the timing) just let me
> know please.
Why can't arch_setup_dma_ops() set those masks too?
Thanks,
Rafael
^ permalink raw reply
* [PATCH 3/3] USB: OHCI: nxp: remove useless extern declaration
From: csmanjuvijay at gmail.com @ 2016-12-03 3:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480735523-23950-1-git-send-email-csmanjuvijay@gmail.com>
From: Manjunath Goudar <csmanjuvijay@gmail.com>
Remove usb_disabled() extern declaration as it is already declared
as EXPORT_SYMBOL_GPL declaration.
Signed-off-by: Manjunath Goudar <csmanjuvijay@gmail.com>
Cc: Vladimir Zapolskiy <vz@mleia.com>
Cc: Sylvain Lemieux <slemieux.tyco@gmail.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-usb at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
---
drivers/usb/host/ohci-nxp.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/usb/host/ohci-nxp.c b/drivers/usb/host/ohci-nxp.c
index b7d4756..1843f04 100644
--- a/drivers/usb/host/ohci-nxp.c
+++ b/drivers/usb/host/ohci-nxp.c
@@ -56,8 +56,6 @@ static struct hc_driver __read_mostly ohci_nxp_hc_driver;
static struct i2c_client *isp1301_i2c_client;
-extern int usb_disabled(void);
-
static struct clk *usb_host_clk;
static void isp1301_configure_lpc32xx(void)
--
2.7.4
^ permalink raw reply related
* [PATCH v2] USB: OHCI: ohci-s3c2410: remove useless functions
From: csmanjuvijay at gmail.com @ 2016-12-03 3:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480542166-7032-1-git-send-email-csmanjuvijay@gmail.com>
From: Manjunath Goudar <csmanjuvijay@gmail.com>
The ohci_hcd_s3c2410_drv_probe and ohci_hcd_s3c2410_drv_remove
functions are removed as these are useless functions except calling
usb_hcd_s3c2410_probe and usb_hcd_s3c2410_remove functions.
The usb_hcd_s3c2410_probe function renamed to ohci_hcd_s3c2410_drv_probe
and usb_hcd_s3c2410_remove function renamed to ohci_hcd_s3c2410_drv_remove
for proper naming.
Signed-off-by: Manjunath Goudar <csmanjuvijay@gmail.com>
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-samsung-soc at vger.kernel.org
Cc: linux-usb at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
---
Changelog v1 -> v2:
Removed checkpatch.pl warnings and errors cleanup code which is not related
to this patch.
drivers/usb/host/ohci-s3c2410.c | 39 ++++++++++++++-------------------------
1 file changed, 14 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/host/ohci-s3c2410.c b/drivers/usb/host/ohci-s3c2410.c
index d8e03a8..b006b93 100644
--- a/drivers/usb/host/ohci-s3c2410.c
+++ b/drivers/usb/host/ohci-s3c2410.c
@@ -43,6 +43,8 @@ static const char hcd_name[] = "ohci-s3c2410";
static struct clk *clk;
static struct clk *usb_clk;
+static struct hc_driver __read_mostly ohci_s3c2410_hc_driver;
+
/* forward definitions */
static void s3c2410_hcd_oc(struct s3c2410_hcd_info *info, int port_oc);
@@ -321,26 +323,29 @@ static void s3c2410_hcd_oc(struct s3c2410_hcd_info *info, int port_oc)
/* may be called with controller, bus, and devices active */
/*
- * usb_hcd_s3c2410_remove - shutdown processing for HCD
+ * ohci_hcd_s3c2410_remove - shutdown processing for HCD
* @dev: USB Host Controller being removed
* Context: !in_interrupt()
*
- * Reverses the effect of usb_hcd_3c2410_probe(), first invoking
+ * Reverses the effect of ohci_hcd_3c2410_probe(), first invoking
* the HCD's stop() method. It is always called from a thread
* context, normally "rmmod", "apmd", or something similar.
*
*/
-static void
-usb_hcd_s3c2410_remove(struct usb_hcd *hcd, struct platform_device *dev)
+static int
+ohci_hcd_s3c2410_remove(struct platform_device *dev)
{
+ struct usb_hcd *hcd = platform_get_drvdata(dev);
+
usb_remove_hcd(hcd);
s3c2410_stop_hc(dev);
usb_put_hcd(hcd);
+ return 0;
}
/**
- * usb_hcd_s3c2410_probe - initialize S3C2410-based HCDs
+ * ohci_hcd_s3c2410_probe - initialize S3C2410-based HCDs
* Context: !in_interrupt()
*
* Allocates basic resources for this USB host controller, and
@@ -348,8 +353,7 @@ usb_hcd_s3c2410_remove(struct usb_hcd *hcd, struct platform_device *dev)
* through the hotplug entry's driver_data.
*
*/
-static int usb_hcd_s3c2410_probe(const struct hc_driver *driver,
- struct platform_device *dev)
+static int ohci_hcd_s3c2410_probe(struct platform_device *dev)
{
struct usb_hcd *hcd = NULL;
struct s3c2410_hcd_info *info = dev_get_platdata(&dev->dev);
@@ -358,7 +362,7 @@ static int usb_hcd_s3c2410_probe(const struct hc_driver *driver,
s3c2410_usb_set_power(info, 1, 1);
s3c2410_usb_set_power(info, 2, 1);
- hcd = usb_create_hcd(driver, &dev->dev, "s3c24xx");
+ hcd = usb_create_hcd(&ohci_s3c2410_hc_driver, &dev->dev, "s3c24xx");
if (hcd == NULL)
return -ENOMEM;
@@ -404,21 +408,6 @@ static int usb_hcd_s3c2410_probe(const struct hc_driver *driver,
/*-------------------------------------------------------------------------*/
-static struct hc_driver __read_mostly ohci_s3c2410_hc_driver;
-
-static int ohci_hcd_s3c2410_drv_probe(struct platform_device *pdev)
-{
- return usb_hcd_s3c2410_probe(&ohci_s3c2410_hc_driver, pdev);
-}
-
-static int ohci_hcd_s3c2410_drv_remove(struct platform_device *pdev)
-{
- struct usb_hcd *hcd = platform_get_drvdata(pdev);
-
- usb_hcd_s3c2410_remove(hcd, pdev);
- return 0;
-}
-
#ifdef CONFIG_PM
static int ohci_hcd_s3c2410_drv_suspend(struct device *dev)
{
@@ -465,8 +454,8 @@ static const struct of_device_id ohci_hcd_s3c2410_dt_ids[] = {
MODULE_DEVICE_TABLE(of, ohci_hcd_s3c2410_dt_ids);
static struct platform_driver ohci_hcd_s3c2410_driver = {
- .probe = ohci_hcd_s3c2410_drv_probe,
- .remove = ohci_hcd_s3c2410_drv_remove,
+ .probe = ohci_hcd_s3c2410_probe,
+ .remove = ohci_hcd_s3c2410_remove,
.shutdown = usb_hcd_platform_shutdown,
.driver = {
.name = "s3c2410-ohci",
--
2.7.4
^ permalink raw reply related
* [PATCH v3] PCI/ACPI: xgene: Add ECAM quirk for X-Gene PCIe controller
From: Duc Dang @ 2016-12-03 7:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161202233943.GF9903@bhelgaas-glaptop.roam.corp.google.com>
On Fri, Dec 2, 2016 at 3:39 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Thu, Dec 01, 2016 at 11:08:23PM -0500, Jon Masters wrote:
> > Hi Bjorn, Duc, Mark,
> >
> > I switched my brain to the on mode and went and read some specs, and a few
> > tables, so here's my 2 cents on this...
> >
> > On 12/01/2016 06:22 PM, Duc Dang wrote:
> > > On Thu, Dec 1, 2016 at 3:07 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > >> On Thu, Dec 01, 2016 at 02:10:10PM -0800, Duc Dang wrote:
> >
> > >>>>> The SoC provide some number of RC bridges, each with a different base
> > >>>>> for some mmio registers. Even if segment is legitimate in MCFG, there
> > >>>>> is still a problem if a platform doesn't use the segment ordering
> > >>>>> implied by the code. But the PNP0A03 _CRS does have this base address
> > >>>>> as the first memory resource, so we could get it from there and not
> > >>>>> have hard-coded addresses and implied ording in the quirk code.
> > >>>>
> > >>>> I'm confused. Doesn't the current code treat every item in PNP0A03
> > >>>> _CRS as a window? Do you mean the first resource is handled
> > >>>> differently somehow? The Consumer/Producer bit could allow us to do
> > >>>> this by marking the RC MMIO space as "Consumer", but I didn't think
> > >>>> that strategy was quite working yet.
> >
> > Let's see if I summarized this correctly...
> >
> > 1. The MMIO registers for the host bridge itself need to be described
> > somewhere, especially if we need to find those in a quirk and poke
> > them. Since those registers are very much part of the bridge device,
> > it makes sense for them to be in the _CRS for PNP0A08/PNP0A03.
> >
> > 2. The address space covering these registers MUST be described as a
> > ResourceConsumer in order to avoid accidentally exposing them as
> > available for use by downstream devices on the PCI bus.
> >
> > 3. The ACPI specification allows for resources of the type "Memory32Fixed".
> > This is a macro that doesn't have the notion of a producer or consumer.
> > HOWEVER various interpretations seem to be that this could/should
> > default to being interpreted as a consumed region.
>
> I agree; I think that per spec, Memory24, Memory32, Memory32Fixed, IO,
> and FixedIO should all be for consumed resources, not for bridge
> windows, since they don't have the notion of producer.
>
> I'm pretty sure there's x86 firmware in the field that uses these for
> windows, so I think we have to accept that usage, at least on x86.
>
> > 4. At one point, a regression was added to the kernel:
> >
> > 63f1789ec716 ("x86/PCI/ACPI: Ignore resources consumed by
> > host bridge itself")
> >
> > Which lead to a series on conversations about what should happen
> > for bridge resources (e.g. https://lkml.org/lkml/2015/3/24/962 )
> >
> > 5. This resulted in the following commit reverting point 4:
> >
> > 2c62e8492ed7 ("x86/PCI/ACPI: Make all resources except [io 0xcf8-0xcff]
> > available on PCI bus")
> >
> > Which also stated that:
> >
> > "This solution will also ease the way to consolidate ACPI PCI host
> > bridge common code from x86, ia64 and ARM64"
> >
> > End of summary.
> >
> > So it seems that generally there is an aversion to having bridge resources
> > be described in this manner and you would like to require that they be
> > described e.g. using QWordMemory with a ResourceConsumer type?
>
> Per spec, we should ignore the Consumer/Producer bit in Word/DWord/QWord
> descriptors. In bridge devices on x86, I think we have to treat them as
> producers (windows) because that's how they've been typically used.
>
> > BUT if we were to do that, it would break existing shipping systems since
> > there are quirks out there that use this form to find the base CSR:
> >
> > if (acpi_res->type == ACPI_RESOURCE_TYPE_FIXED_MEMORY32) {
> > fixed32 = &acpi_res->data.fixed_memory32;
> > port->csr_base = ioremap(fixed32->address,
> > fixed32->address_length);
> > return AE_CTRL_TERMINATE;
> > }
>
> I think this is a valid usage of FixedMemory32 in terms of the spec.
> Linux currently handles this as a window if it appears in a PNP0A03
> device because some x86 firmware used it that way.
>
> We might be able to handle it differently on arm64, e.g., by making an
> arm64 version of pci_acpi_root_prepare_resources() that checks for
> IORESOURCE_WINDOW.
>
> > 2. What would happen if we had a difference policy on arm64 for such
> > resources. x86 has an "exception" for accessing the config space
> > using IO port 0xCF8-0xCFF (a fairly reasonable exception!) and
> > we can make the rules for a new platform (i.e. actually prescribe
> > exactly what the behavior is, rather than have it not be defined).
> > This is of course terrible in that existing BIOS vendors and so on
> > won't necessarily know this when working on ARM ACPI later on.
>
> > Indeed. And in the case of m400, it is currently this in shipping systems:
> >
> > Memory32Fixed (ReadWrite,
> > 0x1F500000, // Address Base
> > 0x00010000, // Address Length
> > )
>
> > >>> [ 0.822990] pci_bus 0000:00: root bus resource [mem 0x1f2b0000-0x1f2bffff]
> > >>
> > >> I think this is wrong. The PCI core thinks [mem 0x1f2b0000-0x1f2bffff]
> > >> is available for use by devices on bus 0000:00, but I think you're
> > >> saying it is consumed by the bridge itself, not forwarded down to PCI.
>
> I think this ASL is perfectly spec-compliant, and what's wrong is the
> way Linux is interpreting it.
>
> I'm not clear on what's terrible about idea 2. I think it's basically
> what I suggested above, i.e., something like the patch below, which I
> think (hope) would keep us from thinking that region is a window.
>
> Even without this patch, I don't think it's a show-stopper to have
> Linux mistakenly thinking this region is routed to PCI, because the
> driver does reserve it and the PCI core will never try to use it.
>
> diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> index 8a177a1..a16fc8e 100644
> --- a/arch/arm64/kernel/pci.c
> +++ b/arch/arm64/kernel/pci.c
> @@ -114,6 +114,19 @@ int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
> return 0;
> }
>
> +static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci)
> +{
> + struct resource_entry *entry, *tmp;
> + int status;
> +
> + status = acpi_pci_probe_root_resources(ci);
> + resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
> + if (!(entry->res->flags & IORESOURCE_WINDOW))
> + resource_list_destroy_entry(entry);
> + }
> + return status;
> +}
> +
> /*
> * Lookup the bus range for the domain in MCFG, and set up config space
> * mapping.
> @@ -190,6 +203,7 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
> }
>
> root_ops->release_info = pci_acpi_generic_release_info;
> + root_ops->prepare_resources = pci_acpi_root_prepare_resources;
> root_ops->pci_ops = &ri->cfg->ops->pci_ops;
> bus = acpi_pci_root_create(root, root_ops, &ri->common, ri->cfg);
> if (!bus)
I tried your patch above with my X-Gene ECAM v4 patch on Mustang, here
is the kernel boot log and output of 'cat /proc/iomem'. The PCIe core
does not print the MMIO space as a window (which is expected per your
patch above).
Booting Linux on physical CPU 0x0
Linux version 4.9.0-rc1-17008-gf18738b-dirty
(dhdang at dhdang-workstation-01) (gcc version 4.9.3 20150218
(prerelease) (APM-8.0.10-le) ) #78 SMP PREEMPT Fri Dec 2 22:32:29 PST
2016
Boot CPU: AArch64 Processor [500f0001]
earlycon: uart8250 at MMIO32 0x000000001c020000 (options '')
bootconsole [uart8250] enabled
efi: Getting EFI parameters from FDT:
efi: EFI v2.40 by X-Gene Mustang Board EFI Oct 17 2016 13:54:05
efi: ACPI=0x47fa700000 ACPI 2.0=0x47fa700014 SMBIOS
3.0=0x47fa9db000 ESRT=0x47ff006f18
esrt: Reserving ESRT space from 0x00000047ff006f18 to 0x00000047ff006f78.
cma: Reserved 256 MiB at 0x00000040f0000000
ACPI: Early table checksum verification disabled
ACPI: RSDP 0x00000047FA700014 000024 (v02 APM )
ACPI: XSDT 0x00000047FA6F00E8 000084 (v01 APM XGENE 00000003
01000013)
ACPI: FACP 0x00000047FA6C0000 00010C (v05 APM XGENE 00000003
INTL 20140724)
ACPI: DSDT 0x00000047FA6D0000 005922 (v05 APM APM88xxx 00000001
INTL 20140724)
ACPI: DBG2 0x00000047FA6E0000 0000AA (v00 APMC0D XGENEDBG 00000000
INTL 20140724)
ACPI: GTDT 0x00000047FA6A0000 000060 (v02 APM XGENE 00000001
INTL 20140724)
ACPI: MCFG 0x00000047FA690000 00003C (v01 APM XGENE 00000002
INTL 20140724)
ACPI: SPCR 0x00000047FA680000 000050 (v02 APMC0D XGENESPC 00000000
INTL 20140724)
ACPI: SSDT 0x00000047FA670000 00002D (v02 APM XGENE 00000001
INTL 20140724)
ACPI: BERT 0x00000047FA660000 000030 (v01 APM XGENE 00000002
INTL 20140724)
ACPI: HEST 0x00000047FA650000 0002A8 (v01 APM XGENE 00000002
INTL 20140724)
ACPI: APIC 0x00000047FA640000 0002A4 (v03 APM XGENE 00000003
01000013)
ACPI: SSDT 0x00000047FA630000 000063 (v02 REDHAT MACADDRS 00000001
01000013)
ACPI: SSDT 0x00000047FA620000 000032 (v02 REDHAT UARTCLKS 00000001
01000013)
ACPI: PCCT 0x00000047FA610000 000300 (v01 APM XGENE 00000003
01000013)
ACPI: SPCR: console: uart,mmio,0x1c020000,115200
On node 0 totalpages: 8388608
DMA zone: 16384 pages used for memmap
DMA zone: 0 pages reserved
DMA zone: 1048576 pages, LIFO batch:31
Normal zone: 114688 pages used for memmap
Normal zone: 7340032 pages, LIFO batch:31
psci: is not implemented in ACPI.
percpu: Embedded 21 pages/cpu @ffff8007fff16000 s48000 r8192 d29824 u86016
pcpu-alloc: s48000 r8192 d29824 u86016 alloc=21*4096
pcpu-alloc: [0] 0 [0] 1 [0] 2 [0] 3 [0] 4 [0] 5 [0] 6 [0] 7
Detected PIPT I-cache on CPU0
Built 1 zonelists in Zone order, mobility grouping on. Total pages: 8257536
Kernel command line: BOOT_IMAGE=/apm-opensource/Image
console=ttyS0,115200 earlycon=uart8250,mmio32,0x1c020000 root=/dev/ram
rw netdev=eth0 debug acpi=force
log_buf_len individual max cpu contribution: 4096 bytes
log_buf_len total cpu_extra contributions: 28672 bytes
log_buf_len min size: 16384 bytes
log_buf_len: 65536 bytes
early log buf free: 12844(78%)
PID hash table entries: 4096 (order: 3, 32768 bytes)
Dentry cache hash table entries: 4194304 (order: 13, 33554432 bytes)
Inode-cache hash table entries: 2097152 (order: 12, 16777216 bytes)
software IO TLB [mem 0x40ebfff000-0x40effff000] (64MB) mapped at
[ffff8000ebfff000-ffff8000efffefff]
Memory: 32615844K/33554432K available (8700K kernel code, 870K rwdata,
3792K rodata, 1024K init, 284K bss, 676444K reserved, 262144K
cma-reserved)
Virtual kernel memory layout:
modules : 0xffff000000000000 - 0xffff000008000000 ( 128 MB)
vmalloc : 0xffff000008000000 - 0xffff7dffbfff0000 (129022 GB)
.text : 0xffff000008080000 - 0xffff000008900000 ( 8704 KB)
.rodata : 0xffff000008900000 - 0xffff000008cc0000 ( 3840 KB)
.init : 0xffff000008cc0000 - 0xffff000008dc0000 ( 1024 KB)
.data : 0xffff000008dc0000 - 0xffff000008e99a00 ( 871 KB)
.bss : 0xffff000008e99a00 - 0xffff000008ee0bc0 ( 285 KB)
fixed : 0xffff7dfffe7fd000 - 0xffff7dfffec00000 ( 4108 KB)
PCI I/O : 0xffff7dfffee00000 - 0xffff7dffffe00000 ( 16 MB)
vmemmap : 0xffff7e0000000000 - 0xffff800000000000 ( 2048 GB maximum)
0xffff7e0000000000 - 0xffff7e0020000000 ( 512 MB actual)
memory : 0xffff800000000000 - 0xffff800800000000 ( 32768 MB)
SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=8, Nodes=1
Preemptible hierarchical RCU implementation.
Build-time adjustment of leaf fanout to 64.
RCU restricting CPUs from NR_CPUS=64 to nr_cpu_ids=8.
RCU: Adjusting geometry for rcu_fanout_leaf=64, nr_cpu_ids=8
NR_IRQS:64 nr_irqs:64 0
GIC: Using split EOI/Deactivate mode
GICv3: No distributor detected at @ffff000008010000, giving up
arm_arch_timer: Architected cp15 timer(s) running at 50.00MHz (phys).
clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles:
0xb8812736b, max_idle_ns: 440795202655 ns
sched_clock: 56 bits at 50MHz, resolution 20ns, wraps every 4398046511100ns
Console: colour dummy device 80x25
Calibrating delay loop (skipped), value calculated using timer
frequency.. 100.00 BogoMIPS (lpj=200000)
pid_max: default: 32768 minimum: 301
ACPI: Core revision 20160831
ACPI Error: Method parse/execution failed [\_SB.ET00._STA] (Node
ffff8007fa9fcdc0), AE_CTRL_PARSE_CONTINUE (20160831/psparse-543)
ACPI Error: Invalid zero thread count in method (20160831/dsmethod-796)
ACPI Error: Invalid OwnerId: 0x00 (20160831/utownerid-186)
ACPI Error: Method parse/execution failed [\_SB.ET01._STA] (Node
ffff8007fa9fe078), AE_CTRL_PARSE_CONTINUE (20160831/psparse-543)
ACPI Error: Invalid zero thread count in method (20160831/dsmethod-796)
ACPI Error: Invalid OwnerId: 0x00 (20160831/utownerid-186)
ACPI: 4 ACPI AML tables successfully acquired and loaded
Security Framework initialized
Mount-cache hash table entries: 65536 (order: 7, 524288 bytes)
Mountpoint-cache hash table entries: 65536 (order: 7, 524288 bytes)
ASID allocator initialised with 65536 entries
Remapping and enabling EFI services.
EFI remap 0x0000000010510000 => 0000000020000000
EFI remap 0x0000000010548000 => 0000000020018000
EFI remap 0x0000000017000000 => 0000000020020000
EFI remap 0x000000001c025000 => 0000000020035000
EFI remap 0x00000047fa5a0000 => 0000000020040000
EFI remap 0x00000047fa5b0000 => 0000000020050000
EFI remap 0x00000047fa5c0000 => 0000000020060000
EFI remap 0x00000047fa710000 => 0000000020070000
EFI remap 0x00000047fa730000 => 0000000020090000
EFI remap 0x00000047fa790000 => 00000000200f0000
EFI remap 0x00000047fa7a0000 => 0000000020100000
EFI remap 0x00000047fa9a0000 => 0000000020300000
EFI remap 0x00000047fa9b0000 => 0000000020310000
EFI remap 0x00000047ff9a0000 => 0000000020330000
EFI remap 0x00000047ff9c0000 => 0000000020340000
Detected PIPT I-cache on CPU1
CPU1: Booted secondary processor [500f0001]
Detected PIPT I-cache on CPU2
CPU2: Booted secondary processor [500f0001]
Detected PIPT I-cache on CPU3
CPU3: Booted secondary processor [500f0001]
Detected PIPT I-cache on CPU4
CPU4: Booted secondary processor [500f0001]
Detected PIPT I-cache on CPU5
CPU5: Booted secondary processor [500f0001]
Detected PIPT I-cache on CPU6
CPU6: Booted secondary processor [500f0001]
Detected PIPT I-cache on CPU7
CPU7: Booted secondary processor [500f0001]
Brought up 8 CPUs
SMP: Total of 8 processors activated.
CPU features: detected feature: 32-bit EL0 Support
CPU: All CPU(s) started at EL2
devtmpfs: initialized
SMBIOS 3.0.0 present.
clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff,
max_idle_ns: 7645041785100000 ns
pinctrl core: initialized pinctrl subsystem
NET: Registered protocol family 16
cpuidle: using governor menu
vdso: 2 pages (1 code @ ffff000008907000, 1 data @ ffff000008dc4000)
hw-breakpoint: found 4 breakpoint and 4 watchpoint registers.
DMA: preallocated 256 KiB pool for atomic allocations
ACPI: bus type PCI registered
Serial: AMBA PL011 UART driver
HugeTLB registered 2 MB page size, pre-allocated 0 pages
ACPI: Added _OSI(Module Device)
ACPI: Added _OSI(Processor Device)
ACPI: Added _OSI(3.0 _SCP Extensions)
ACPI: Added _OSI(Processor Aggregator Device)
ACPI: Interpreter enabled
ACPI: Using GIC for interrupt routing
ACPI: MCFG table detected, 1 entries
ACPI: Power Resource [SCVR] (on)
ACPI: PCI Root Bridge [PCI0] (domain 0000 [bus 00-ff])
acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments MSI]
acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug PME AER PCIeCapability]
acpi PNP0A08:00: MCFG quirk: ECAM at [mem 0xe0d0000000-0xe0dfffffff]
for [bus 00-ff] with xgene_v1_pcie_ecam_ops
acpi PNP0A08:00: [Firmware Bug]: ECAM area [mem
0xe0d0000000-0xe0dfffffff] not reserved in ACPI namespace
acpi PNP0A08:00: ECAM at [mem 0xe0d0000000-0xe0dfffffff] for [bus 00-ff]
Remapped I/O 0x000000e010000000 to [io 0x0000-0xffff window]
PCI host bridge to bus 0000:00
pci_bus 0000:00: root bus resource [io 0x0000-0xffff window] (bus
address [0x10000000-0x1000ffff])
pci_bus 0000:00: root bus resource [mem 0xe040000000-0xe07fffffff
window] (bus address [0x40000000-0x7fffffff])
pci_bus 0000:00: root bus resource [mem 0xf000000000-0xffffffffff window]
pci_bus 0000:00: root bus resource [bus 00-ff]
pci 0000:00:00.0: [10e8:e004] type 01 class 0x060400
pci 0000:00:00.0: supports D1 D2
pci 0000:01:00.0: [15b3:1003] type 00 class 0x020000
pci 0000:01:00.0: reg 0x10: [mem 0xe040000000-0xe0400fffff 64bit]
pci 0000:01:00.0: reg 0x18: [mem 0xe042000000-0xe043ffffff 64bit pref]
pci 0000:01:00.0: reg 0x30: [mem 0xfff00000-0xffffffff pref]
pci_bus 0000:00: on NUMA node 0
pci 0000:00:00.0: BAR 15: assigned [mem 0xf000000000-0xf001ffffff 64bit pref]
pci 0000:00:00.0: BAR 14: assigned [mem 0xe040000000-0xe0401fffff]
pci 0000:01:00.0: BAR 2: assigned [mem 0xf000000000-0xf001ffffff 64bit pref]
pci 0000:01:00.0: BAR 0: assigned [mem 0xe040000000-0xe0400fffff 64bit]
pci 0000:01:00.0: BAR 6: assigned [mem 0xe040100000-0xe0401fffff pref]
pci 0000:00:00.0: PCI bridge to [bus 01]
pci 0000:00:00.0: bridge window [mem 0xe040000000-0xe0401fffff]
pci 0000:00:00.0: bridge window [mem 0xf000000000-0xf001ffffff 64bit pref]
vgaarb: loaded
SCSI subsystem initialized
libata version 3.00 loaded.
ACPI: bus type USB registered
usbcore: registered new interface driver usbfs
usbcore: registered new interface driver hub
usbcore: registered new device driver usb
pps_core: LinuxPPS API ver. 1 registered
pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti
<giometti@linux.it>
PTP clock support registered
Registered efivars operations
Advanced Linux Sound Architecture Driver Initialized.
clocksource: Switched to clocksource arch_sys_counter
VFS: Disk quotas dquot_6.6.0
VFS: Dquot-cache hash table entries: 512 (order 0, 4096 bytes)
pnp: PnP ACPI init
pnp: PnP ACPI: found 0 devices
NET: Registered protocol family 2
TCP established hash table entries: 262144 (order: 9, 2097152 bytes)
TCP bind hash table entries: 65536 (order: 8, 1048576 bytes)
TCP: Hash tables configured (established 262144 bind 65536)
UDP hash table entries: 16384 (order: 7, 524288 bytes)
UDP-Lite hash table entries: 16384 (order: 7, 524288 bytes)
NET: Registered protocol family 1
RPC: Registered named UNIX socket transport module.
RPC: Registered udp transport module.
RPC: Registered tcp transport module.
RPC: Registered tcp NFSv4.1 backchannel transport module.
PCI: CLS 0 bytes, default 128
Unpacking initramfs...
Freeing initrd memory: 14676K (ffff8007f8767000 - ffff8007f95bc000)
kvm [1]: 8-bit VMID
kvm [1]: IDMAP page: 4000af5000
kvm [1]: HYP VA range: 800000000000:ffffffffffff
kvm [1]: Hyp mode initialized successfully
kvm [1]: vgic-v2 at 780cf000
kvm [1]: vgic interrupt IRQ1
kvm [1]: virtual timer IRQ4
futex hash table entries: 2048 (order: 6, 262144 bytes)
audit: initializing netlink subsys (disabled)
audit: type=2000 audit(4.120:1): initialized
workingset: timestamp_bits=46 max_order=23 bucket_order=0
squashfs: version 4.0 (2009/01/31) Phillip Lougher
NFS: Registering the id_resolver key type
Key type id_resolver registered
Key type id_legacy registered
nfs4filelayout_init: NFSv4 File Layout Driver Registering...
9p: Installing v9fs 9p2000 file system support
Block layer SCSI generic (bsg) driver version 0.4 loaded (major 247)
io scheduler noop registered
io scheduler cfq registered (default)
libphy: mdio_driver_register: phy-bcm-ns2-pci
xgene-gpio APMC0D14:00: X-Gene GPIO driver registered.
aer 0000:00:00.0:pcie002: service driver aer loaded
pcieport 0000:00:00.0: Signaling PME through PCIe PME interrupt
pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
pcie_pme 0000:00:00.0:pcie001: service driver pcie_pme loaded
input: Power Button as /devices/LNXSYSTM:00/PNP0C0C:00/input/input0
ACPI: Power Button [PWRB]
xenfs: not registering filesystem on non-xen platform
Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
console [ttyS0] disabled
APMC0D08:00: ttyS0 at MMIO 0x1c020000 (irq = 22, base_baud = 3125000)
is a U6_16550A
console [ttyS0] enabled
bootconsole [uart8250] disabled
APMC0D08:01: ttyS1 at MMIO 0x1c021000 (irq = 23, base_baud = 3125000)
is a U6_16550A
SuperH (H)SCI(F) driver initialized
msm_serial: driver initialized
Failed to find cpu0 device node
Unable to detect cache hierarchy from DT for CPU 0
loop: module loaded
hisi_sas: driver version v1.6
xgene-ahci APMC0D0D:00: skip clock and PHY initialization
xgene-ahci APMC0D0D:00: controller can't do NCQ, turning off CAP_NCQ
xgene-ahci APMC0D0D:00: AHCI 0001.0300 32 slots 2 ports 6 Gbps 0x3
impl platform mode
xgene-ahci APMC0D0D:00: flags: 64bit sntf pm only pmp fbs pio slum part ccc
xgene-ahci APMC0D0D:00: port 0 is not capable of FBS
xgene-ahci APMC0D0D:00: port 1 is not capable of FBS
scsi host0: xgene-ahci
scsi host1: xgene-ahci
ata1: SATA max UDMA/133 mmio [mem 0x1a400000-0x1a400fff] port 0x100 irq 27
ata2: SATA max UDMA/133 mmio [mem 0x1a400000-0x1a400fff] port 0x180 irq 27
xgene-ahci APMC0D0D:01: skip clock and PHY initialization
xgene-ahci APMC0D0D:01: controller can't do NCQ, turning off CAP_NCQ
xgene-ahci APMC0D0D:01: AHCI 0001.0300 32 slots 2 ports 6 Gbps 0x3
impl platform mode
xgene-ahci APMC0D0D:01: flags: 64bit sntf pm only pmp fbs pio slum part ccc
xgene-ahci APMC0D0D:01: port 0 is not capable of FBS
xgene-ahci APMC0D0D:01: port 1 is not capable of FBS
scsi host2: xgene-ahci
scsi host3: xgene-ahci
ata3: SATA max UDMA/133 mmio [mem 0x1a800000-0x1a800fff] port 0x100 irq 28
ata4: SATA max UDMA/133 mmio [mem 0x1a800000-0x1a800fff] port 0x180 irq 28
libphy: APM X-Gene MDIO bus: probed
libphy: Fixed MDIO Bus: probed
tun: Universal TUN/TAP device driver, 1.6
tun: (C) 1999-2004 Max Krasnyansky <maxk@qualcomm.com>
xgene-enet APMC0D05:00: clocks have been setup already
xgene-enet APMC0D30:00: clocks have been setup already
ata1: SATA link down (SStatus 0 SControl 4300)
ata2: SATA link down (SStatus 0 SControl 4300)
xgene-enet APMC0D30:01: clocks have been setup already
xgene-enet APMC0D31:00: clocks have been setup already
e1000e: Intel(R) PRO/1000 Network Driver - 3.2.6-k
e1000e: Copyright(c) 1999 - 2015 Intel Corporation.
igb: Intel(R) Gigabit Ethernet Network Driver - version 5.4.0-k
igb: Copyright (c) 2007-2014 Intel Corporation.
igbvf: Intel(R) Gigabit Virtual Function Network Driver - version 2.4.0-k
igbvf: Copyright (c) 2009 - 2012 Intel Corporation.
sky2: driver version 1.30
mlx4_core: Mellanox ConnectX core driver v2.2-1 (Feb, 2014)
ata3: SATA link down (SStatus 0 SControl 4300)
mlx4_core: Initializing 0000:01:00.0
ata4: SATA link up 6.0 Gbps (SStatus 133 SControl 4300)
ata4.00: ATA-8: SDLFOCAM-800G-1HA1, ZZ37RE92, max UDMA/133
ata4.00: 1562824368 sectors, multi 0: LBA48 NCQ (depth 0/32)
ata4.00: configured for UDMA/133
scsi 3:0:0:0: Direct-Access ATA SDLFOCAM-800G-1H RE92 PQ: 0 ANSI: 5
sd 3:0:0:0: [sda] 1562824368 512-byte logical blocks: (800 GB/745 GiB)
sd 3:0:0:0: [sda] 4096-byte physical blocks
sd 3:0:0:0: [sda] Write Protect is off
sd 3:0:0:0: [sda] Mode Sense: 00 3a 00 00
sd 3:0:0:0: [sda] Write cache: enabled, read cache: enabled, doesn't
support DPO or FUA
sda: sda1 sda2 sda3
sd 3:0:0:0: [sda] Attached SCSI disk
pcieport 0000:00:00.0: AER: Multiple Corrected error received: id=0000
pcieport 0000:00:00.0: PCIe Bus Error: severity=Corrected,
type=Physical Layer, id=0000(Receiver ID)
pcieport 0000:00:00.0: device [10e8:e004] error status/mask=00000041/00002001
pcieport 0000:00:00.0: [ 6] Bad TLP
pcieport 0000:00:00.0: AER: Corrected error received: id=0000
pcieport 0000:00:00.0: can't find device of ID0000
pcieport 0000:00:00.0: AER: Corrected error received: id=0000
pcieport 0000:00:00.0: can't find device of ID0000
pcieport 0000:00:00.0: AER: Corrected error received: id=0000
pcieport 0000:00:00.0: PCIe Bus Error: severity=Corrected,
type=Physical Layer, id=0000(Transmitter ID)
pcieport 0000:00:00.0: device [10e8:e004] error status/mask=00001041/00002001
pcieport 0000:00:00.0: [ 6] Bad TLP
pcieport 0000:00:00.0: [12] Replay Timer Timeout
pcieport 0000:00:00.0: AER: Multiple Corrected error received: id=0000
pcieport 0000:00:00.0: can't find device of ID0000
pcieport 0000:00:00.0: AER: Corrected error received: id=0000
pcieport 0000:00:00.0: can't find device of ID0000
pcieport 0000:00:00.0: AER: Corrected error received: id=0000
pcieport 0000:00:00.0: can't find device of ID0000
mlx4_core 0000:01:00.0: PCIe link speed is 8.0GT/s, device supports 8.0GT/s
mlx4_core 0000:01:00.0: PCIe link width is x8, device supports x8
mlx4_en: Mellanox ConnectX HCA Ethernet driver v2.2-1 (Feb 2014)
mlx4_en 0000:01:00.0: Activating port:1
mlx4_en: 0000:01:00.0: Port 1: Using 64 TX rings
mlx4_en: 0000:01:00.0: Port 1: Using 4 RX rings
mlx4_en: 0000:01:00.0: Port 1: frag:0 - size:1522 prefix:0 stride:1536
mlx4_en: 0000:01:00.0: Port 1: Initializing port
mlx4_en 0000:01:00.0: registered PHC clock
mlx4_en 0000:01:00.0: Activating port:2
mlx4_en: 0000:01:00.0: Port 2: Using 64 TX rings
mlx4_en: 0000:01:00.0: Port 2: Using 4 RX rings
mlx4_en: 0000:01:00.0: Port 2: frag:0 - size:1522 prefix:0 stride:1536
mlx4_en: 0000:01:00.0: Port 2: Initializing port
VFIO - User Level meta-driver version: 0.3
ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
ehci-pci: EHCI PCI platform driver
ehci-platform: EHCI generic platform driver
ehci-exynos: EHCI EXYNOS driver
ehci-msm: Qualcomm On-Chip EHCI Host Controller
ohci_hcd: USB 1.1 'Open' Host Controller (OHCI) Driver
ohci-pci: OHCI PCI platform driver
ohci-platform: OHCI generic platform driver
ohci-exynos: OHCI EXYNOS driver
xhci-hcd: probe of xhci-hcd.0.auto failed with error -5
xhci-hcd: probe of xhci-hcd.1.auto failed with error -5
usbcore: registered new interface driver usb-storage
mousedev: PS/2 mouse device common for all mice
rtc-efi rtc-efi: rtc core: registered rtc-efi as rtc0
i2c /dev entries driver
sdhci: Secure Digital Host Controller Interface driver
sdhci: Copyright(c) Pierre Ossman
Synopsys Designware Multimedia Card Interface Driver
sdhci-pltfm: SDHCI platform and OF driver helper
ledtrig-cpu: registered to indicate activity on CPUs
usbcore: registered new interface driver usbhid
usbhid: USB HID core driver
NET: Registered protocol family 17
9pnet: Installing 9P2000 support
Key type dns_resolver registered
registered taskstats version 1
rtc-efi rtc-efi: setting system clock to 2016-12-03 06:49:10 UTC (1480747750)
ALSA device list:
No soundcards found.
Freeing unused kernel memory: 1024K (ffff800000ec0000 - ffff800000fc0000)
udevd[1484]: starting version 182
random: fast init done
[root@(none) ~]# cat /proc/io mem
10000000-103fffff : APMC0D05:00
10520000-10523fff : APMC0D18:00
10524000-10527fff : APMC0D17:00
10540000-10547fff : APMC0D01:00
1054a000-1054a0ff : APMC0D43:00
1054a000-1054a01b : APMC0D41:00
17001000-170013ff : APMC0D15:00
1701c000-1701cfff : APMC0D14:00
17020000-1702d0ff : APMC0D05:00
17020000-1702d0ff : APMC0D65:00
17020000-1702d0ff : APMC0D3E:00
17020000-1702d0ff : APMC0D65:00
17030000-1703ffff : APMC0D05:00
18000000-183fffff : APMC0D31:00
18000000-183fffff : APMC0D6A:00
19000000-19007fff : 808622B7:00
1900c100-190fffff : 808622B7:00
1900c100-190fffff : 808622B7:00
19800000-19807fff : 808622B7:01
1980c100-198fffff : 808622B7:01
1980c100-198fffff : 808622B7:01
1a400000-1a400fff : APMC0D0D:00
1a400000-1a400fff : APMC0D0D:00
1a800000-1a800fff : APMC0D0D:01
1a800000-1a800fff : APMC0D0D:01
1b000000-1b3fffff : APMC0D43:00
1b000000-1b007fff : APMC0D30:01
1b000000-1b001fff : APMC0D30:00
1b00a000-1b00bfff : APMC0D41:00
1c000000-1c0000ff : APMC0D0C:00
1c020000-1c0200ff : APMC0D08:00
1c020000-1c02001f : serial
1c021000-1c0210ff : APMC0D08:01
1c021000-1c02101f : serial
1c024000-1c024fff : APMC0D07:00
1c024000-1c024fff : APMC0D07:00
1f200000-1f20ffff : APMC0D41:00
1f200000-1f20ffff : APMC0D43:00
1f200000-1f20c2ff : APMC0D30:01
1f200000-1f20c2ff : APMC0D30:00
1f210000-1f21d0ff : APMC0D30:00
1f210030-1f21d0ff : APMC0D30:01
1f220000-1f220fff : APMC0D0D:00
1f220000-1f220fff : APMC0D0D:00
1f227000-1f227fff : APMC0D0D:00
1f227000-1f227fff : APMC0D0D:00
1f22d000-1f22dfff : APMC0D0D:00
1f22d000-1f22dfff : APMC0D0D:00
1f22e000-1f22efff : APMC0D0D:00
1f22e000-1f22efff : APMC0D0D:00
1f230000-1f230fff : APMC0D0D:01
1f230000-1f230fff : APMC0D0D:01
1f23d000-1f23dfff : APMC0D0D:01
1f23d000-1f23dfff : APMC0D0D:01
1f23e000-1f23efff : APMC0D0D:01
1f23e000-1f23efff : APMC0D0D:01
1f250000-1f25ffff : APMC0D41:00
1f270000-1f27ffff : APMC0D43:00
1f280000-1f28ffff : 808622B7:00
1f290000-1f29ffff : 808622B7:01
1f2a0000-1f2a0fff : APMC0D0C:00
1f2b0000-1f2bffff : PNP0A08:00
1f600000-1f60ffff : APMC0D31:00
1f600000-1f60ffff : APMC0D6A:00
1f610000-1f61ffff : APMC0D31:00
78810000-78810fff : APMC0D5C:00
79000000-798fffff : APMC0D0E:00
7e200000-7e200fff : APMC0D5C:00
7e610000-7e610fff : APMC0D5D:00
7e700000-7e700fff : APMC0D5C:00
7e710000-7e710fff : APMC0D5F:00
7e720000-7e720fff : APMC0D5C:00
7e730000-7e730fff : APMC0D5F:01
7e810000-7e810fff : APMC0D60:00
7e850000-7e850fff : APMC0D60:01
7e890000-7e890fff : APMC0D60:02
7e8d0000-7e8d0fff : APMC0D60:03
7e940000-7e940fff : APMC0D5E:00
4000000000-40001fffff : reserved
4000200000-47fa59ffff : System RAM
4000280000-4000ebffff : Kernel code
4000fc0000-40010e6fff : Kernel data
47fa5a0000-47fa5cffff : reserved
47fa5d0000-47fa5ddfff : System RAM
47fa5de000-47fa9cffff : reserved
47fa9d0000-47fa9d9fff : System RAM
47fa9da000-47fa9dbfff : reserved
47fa9dc000-47ff99ffff : System RAM
47ff9a0000-47ff9affff : reserved
47ff9b0000-47ff9bffff : System RAM
47ff9c0000-47ff9effff : reserved
47ff9f0000-47ffffffff : System RAM
e040000000-e07fffffff : PCI Bus 0000:00
e040000000-e0401fffff : PCI Bus 0000:01
e040000000-e0400fffff : 0000:01:00.0
e040000000-e0400fffff : mlx4_core
e040100000-e0401fffff : 0000:01:00.0
e0d0000000-e0dfffffff : PCI ECAM
f000000000-ffffffffff : PCI Bus 0000:00
f000000000-f001ffffff : PCI Bus 0000:01
f000000000-f001ffffff : 0000:01:00.0
f000000000-f001ffffff : mlx4_core
[root@(none) ~]#
Regards,
Duc Dang.
^ permalink raw reply
* [PATCH v2 1/7] MFD: add bindings for stm32 general purpose timer driver
From: Jonathan Cameron @ 2016-12-03 9:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+M3ks4Y4FnJGqwyH0oitrxvzRnKNHA261wqfEOSfc1aA4am4g@mail.gmail.com>
On 29/11/16 08:48, Benjamin Gaignard wrote:
> 2016-11-27 16:41 GMT+01:00 Jonathan Cameron <jic23@kernel.org>:
>> On 27/11/16 14:10, Jonathan Cameron wrote:
>>> On 24/11/16 15:14, Benjamin Gaignard wrote:
>>>> Add bindings information for stm32 general purpose timer
>>>>
>>>> version 2:
>>>> - rename stm32-mfd-timer to stm32-gptimer
>>>> - only keep one compatible string
>>>>
>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>>> ---
>>>> .../bindings/mfd/stm32-general-purpose-timer.txt | 43 ++++++++++++++++++++++
>>>> 1 file changed, 43 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt b/Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt
>>>> new file mode 100644
>>>> index 0000000..2f10e67
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt
>>>> @@ -0,0 +1,43 @@
>>>> +STM32 general purpose timer driver
>>>> +
>>>> +Required parameters:
>>>> +- compatible: must be "st,stm32-gptimer"
>>>> +
>>>> +- reg: Physical base address and length of the controller's
>>>> + registers.
>>>> +- clock-names: Set to "clk_int".
>>>> +- clocks: Phandle to the clock used by the timer module.
>>>> + For Clk properties, please refer to ../clock/clock-bindings.txt
>>>> +
>>>> +Optional parameters:
>>>> +- resets: Phandle to the parent reset controller.
>>>> + See ..reset/st,stm32-rcc.txt
>>>> +
>>>> +Optional subnodes:
>>>> +- pwm: See ../pwm/pwm-stm32.txt
>>>> +- iiotimer: See ../iio/timer/stm32-iio-timer.txt
>>> Naming issue here. Can't mention IIO as that's a linux subsystem and all
>>> bindings must be independent of OS.
>>>
>>> Perhaps adc-trigger-timer?
>>>> +
>>>> +Example:
>>>> + gptimer1: gptimer1 at 40010000 {
>>>> + compatible = "st,stm32-gptimer";
>>>> + reg = <0x40010000 0x400>;
>>>> + clocks = <&rcc 0 160>;
>>>> + clock-names = "clk_int";
>>>> +
>>>> + pwm1 at 0 {
>>>> + compatible = "st,stm32-pwm";
>>>> + st,pwm-num-chan = <4>;
>>>> + st,breakinput;
>>>> + st,complementary;
>>>> + };
>>>> +
>>>> + iiotimer1 at 0 {
>>>> + compatible = "st,stm32-iio-timer";
>>> Again, avoid the use of iio in here (same issue you had with mfd in the previous
>>> version).
>>>> + interrupts = <27>;
>>>> + st,input-triggers-names = TIM5_TRGO,
>>> Docs for these should be introduced before they are used in an example.
>>> Same for the PWM ones above. Expand the detail fo the example as you add
>>> the other elements.
>>
>> I've just dived into the datasheet for these timers.
>> http://www.st.com/content/ccc/resource/technical/document/reference_manual/3d/6d/5a/66/b4/99/40/d4/DM00031020.pdf/files/DM00031020.pdf/jcr:content/translations/en.DM00031020.pdf
>>
>
> I really appreciate that you do this effort, thanks.
>
>>
>> I think you need a binding that describes the capabilities of each of the timers
>> explicitly. Down to the level of whether there is a repetition counter or not.
>> Each should exists as a separate entity in device tree.
>>
>> They then have an existence as timers separate to the description of what they
>> are used for.
>>
>> Here the only way we are saying they exist is by their use which doesn't feel
>> right to me.
>>
>> So I think you need to move back to what you had in the first place. The key
>> thing is that ever timer needs describing fully. They are different enough
>> that for example the datasheet doesn't even try to describe them in one section.
>> (it has 4 separate chapters covering different sets of these hardware blocks).
>> The naming isn't really based on index, we are talking different hardware
>> that the datasheet authors have decided not to give different names to!
>
> Even if the hardware are named differently in the documentation they
> all share the
> same registers definitions and mapping but configurations are
> different for each device.
>
>>
>> If they'd called them
>> advanced timers
>> generic timers
>> basic timers
>> really basic timers meant for driving the DAC (6 and 7)
>>
>> We'd all have been quite happy with different compatible strings giving away
>> what they can do.
>
> 4 compatible strings will not be enough to describe devices
> configuration, for example
> in the documentation general purpose timers could have a 16 or 32 bit
> counter, for 1 to 4
> pwm channels and triggers (accepted or generated by the device) are
> different for each device.
>
> DAC could be drive by timers 2, 4, 5, 6, 7 and 8.
> ADC could be driver by 32 triggers
My point was more about the fact that though the naming appears to (and kind
of does describe an index) these devices are really as different as for example
different part numbers would imply on a range of ADCs (say the multitude supported
by the max1363 driver - all of which are very nearly register compatible)
Hence I'd be less quick to dismiss the option of a number of compatible strings
rather than the wealth of description you'll otherwise have to put in the device
tree.
>
>> What you have here is far too specific to what you are trying to do with them
>> right now.
>>
>> These things are separately capable of timing capture (which is I guess where
>> the IIO device later comes in).
>>
>> So my expectation is that we end up potentially instantiating:
>>
>> 1) An MFD to handle the shared elements of the timers.
>> 2) Up to 12ish timers each with separate existence as a device in the driver model
>> and in device tree.
>> (nasty corner cases here are using timers as perscalers for other timers - I'd be
>> tempted to leave that for now)
>> Note that each of these devices has a different register set I think? Any shared
>> bits are handled via the mfd above (if we even need that MFD which I'm starting
>> to doubt looking at the datasheet).
>>
>
> pwm and trigger share the same registers but not the same bits.
> With regmap write functions I don't have sharing problems.
>
>> 3) Up to N pwms again with there own existence in the device model. These don't
>> do much other than wrap the timer and stick it in output mode.
>> 4) Up to N iio triggers - this is basically using the timer as a periodic interrupt
>> (though without the interrupt having visibility to the kernel) which fires off
>> sampling on associated ADCs.
>> 5) Up to N iio capture devices for all channels that support timing capture.
>> Note there is also hardware encoder capture support in these which should be
>> correctly handled as well. This comes back to an ancient discussion on the
>> TI ecap units which have similar capabilities (driver never went anywhere but
>> I think that was because the author left TI).
>>
>> Certainly for the IIO devices these should no be bound up into one instance
>> as you have done here.
>>
>> Anyhow, I fear that right now this discussion is missing the key ingredient
>> that the hardware is horrendously variable in it's capabilities and really
>> is 4-5 different types of hardware that just happen to share a few bits of
>> their offsets in their register maps.
>
> Hardware really share the same registers mapping that why I have wrote
> one only driver
> per framework. Only the configurations are different
One driver is fine, but the difference to my mind are sufficient that
we may need to use compatible strings for the various options. Worth
a go at trying to fully describe them first though!
>
>>
>> So after all that I'm almost more confused than I was at the start!
>>
>> Jonathan
>>
>>
>>>> + TIM2_TRGO,
>>>> + TIM4_TRGO,
>>>> + TIM3_TRGO;
>>>> + st,output-triggers-names = TIM1_TRGO;
>>>> + };
>>>> + };
>>>>
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo at vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>
>
>
^ permalink raw reply
* [PATCH v2 1/2] arm64: dts: zx: Fix gic GICR property
From: Shawn Guo @ 2016-12-03 9:22 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a243d381-f7ab-abed-ca21-f0951abb04fd@arm.com>
On Fri, Dec 02, 2016 at 05:02:28PM +0000, Marc Zyngier wrote:
> Just noticed this.
>
> On 13/10/16 13:31, Jun Nie wrote:
> > GICR for multiple CPU can be described with start address and stride,
> > or with multiple address. Current multiple address and stride are
> > both used. Fix it.
> >
> > vmalloc patch 727a7f5a9 triggered this bug:
> > [ 0.097146] Unable to handle kernel paging request at virtual address ffff000008060008
> > [ 0.097150] pgd = ffff000008602000
> > [ 0.097160] [ffff000008060008] *pgd=000000007fffe003, *pud=000000007fffd003, *pmd=000000007fffc003, *pte=0000000000000000
> > [ 0.097165] Internal error: Oops: 96000007 [#1] PREEMPT SMP
> > [ 0.097170] Modules linked in:
> > [ 0.097177] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0+ #1474
> > [ 0.097179] Hardware name: ZTE zx296718 evaluation board (DT)
> > [ 0.097183] task: ffff80003e8c8b80 task.stack: ffff80003e8d0000
> > [ 0.097197] PC is at gic_populate_rdist+0x74/0x15c
> > [ 0.097202] LR is at gic_starting_cpu+0xc/0x20
> > [ 0.097206] pc : [<ffff0000082b1b18>] lr : [<ffff0000082b26e0>] pstate: 600001c5
> >
> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > ---
> > arch/arm64/boot/dts/zte/zx296718.dtsi | 11 +++--------
> > 1 file changed, 3 insertions(+), 8 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/zte/zx296718.dtsi b/arch/arm64/boot/dts/zte/zx296718.dtsi
> > index a223066..6b239a3 100644
> > --- a/arch/arm64/boot/dts/zte/zx296718.dtsi
> > +++ b/arch/arm64/boot/dts/zte/zx296718.dtsi
> > @@ -239,16 +239,11 @@
> > compatible = "arm,gic-v3";
> > #interrupt-cells = <3>;
> > #address-cells = <0>;
> > - #redistributor-regions = <6>;
> > - redistributor-stride = <0x0 0x40000>;
> > + #redistributor-regions = <1>;
> > + redistributor-stride = <0x20000>;
>
> Why is that stride specified? Is the GIC implementation so busted that
> the GICR_TYPER do not report a GICv3 redistributor, which implies a
> 128kB stride?
No, it's not required per my testing. I guess it's there for
documentation purpose to make the stride setting explicit. Are you
suggesting that we simply drop it?
Also, it seems that #redistributor-regions can also be saved, since
bindings doc tells that it's only required if more than one such region
is present?
Shawn
^ permalink raw reply
* [PATCH v2 6/7] IIO: add STM32 IIO timer driver
From: Jonathan Cameron @ 2016-12-03 9:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+M3ks7ks=YsToEyLcPhPf+JfdJtekUYcihpcYLLt0h6APbcfA@mail.gmail.com>
On 29/11/16 09:46, Benjamin Gaignard wrote:
> 2016-11-27 16:42 GMT+01:00 Jonathan Cameron <jic23@kernel.org>:
>> I delved into the datasheet after trying to figure this out, so I think
>> I now sort of understand your intent, but please do answer the questions
>> inline.
>>
>> On 24/11/16 15:14, Benjamin Gaignard wrote:
>>> Timers IPs can be used to generate triggers for other IPs like
>>> DAC, ADC or other timers.
>>> Each trigger may result of timer internals signals like counter enable,
>>> reset or edge, this configuration could be done through "master_mode"
>>> device attribute.
>>>
>>> A timer device could be triggered by other timers, we use the trigger
>>> name and is_stm32_iio_timer_trigger() function to distinguish them
>>> and configure IP input switch.
>> The presence of an IIO device in here was a suprise.. What is it actually for?
>
> IIO device is needed to be able to valid the input triggers, which
> aren't the same than
> those generated by the device.
> Since I use triggers name to distinguish them I have introduced
> is_stm32_iio_timer_trigger()
> function to be sure that triggers are coming for a valid hardware and
> not from a fake one
> using the same name.
>
>>
>> I think this needs some examples of usage to make it clear what the aim is.
>
> In the hardware block there is switch in input to select which trigger
> will drive the IP.
> For example that allow to start multiple pwm exactly that the same
> time or to start/stop
> it on master edges.
Hmm. OK. We need to think about how to represent this concept of a tree
of triggers - independent of having an IIO device as that is down right
misleading.
In the first instance the tree is full supported by this one driver I think?
If so lets use it as a testbed and try and put together a simple tree between
the triggers.
So the child triggers (started on the parent firing) can perhaps have a
'parent' attribute? (might be better naming possible!)
>
>>
>> I was basically expecting to see a driver instantiating one iio trigger
>> per timer that can act as a trigger. Those would each have sampling frequency
>> controls and basica enable / disable.
>
> An hardware device could have up to 5 triggers: timX_trgo, timX_ch1, timX_ch2,
> timX_ch3, timX_ch4.
On a train so I don't have the datasheet. Which of these would actually make
sense when driving an adc scan?
> Until now I have try to simplify the problem and just use timX_trgo trigger.
> I have added a "sampling_frequency" attribute on the trigger to
> control the frequence
> and I use trigger set_state function to enable disable it.
Wise move! Makes sense to build this up in baby steps if possible.
>
>>
>> I'm seeing something much more complex here so additional explanation is
>> needed.
>>>
>>> Timer may also decide on which event (edge, level) they could
>>> be activated by a trigger, this configuration is done by writing in
>>> "slave_mode" device attribute.
>> Really? Sounds like magic numbers in sysfs which is never a good idea.
>> Please document those attributes / or break them up into elements that
>> don't require magic numbers.
>
> I would like to use strings here, it is possible to use IIO_CONST_ATTR
> to describe them ?
If it's on the iio device use the iio_ext_info stuff that has support
for enums. If you need this for the trigger feel free to add equivalent
support to the core as needed.
Note that we are still evolving IIO so if we need new stuff to support
your usecase, never be afraid of proposing it! The only element
I am keen on is keeping anything that is opaque to drivers opaque
unless there is a VERY good reason to do otherwise. Mostly this
just means using access functions etc. That makes messing around
with the core internals (as still happens from time to time) a lot
less painful!
>
>>>
>>> Since triggers could also be used by DAC or ADC their names are defined
>>> in include/dt-bindings/iio/timer/st,stm32-iio-timer.h so those IPs will be able
>>> to configure themselves in valid_trigger function
>>>
>>> Trigger have a "sampling_frequency" attribute which allow to configure
>>> timer sampling frequency without using pwm interface
>>>
>>> version 2:
>>> - keep only one compatible
>> Hmm. I'm not sure I like this as such. We are actually dealing with lots
>> of instances of a hardware block with only a small amount of shared
>> infrastrcuture (which is classic mfd teritory). So to my mind we
>> should have a separate device for each.
>
> Registers mapping and offset are the same, from triggers point of view
> only the configuration of the input switch is different.
>
>>
>>> - use st,input-triggers-names and st,output-triggers-names
>>> to know which triggers are accepted and/or create by the device
>> I'm not following why we have this cascade setup?
>>
>> These are triggers, not devices in the IIO context. We need some detailed
>> description of why you have it setup like this. This would include the
>> ABI with examples of how you are using it.
>
> I had put example of usage on the cover letter, I will duplicate them
> in this commit
> message.
Ooops. I didn't ready that ;) Sorry.
>
>>
>> Basically I don't currently understand what you are doing :(
>>
>>
>> Thanks,
>>
>> Jonathan
>>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>> ---
>>> drivers/iio/Kconfig | 2 +-
>>> drivers/iio/Makefile | 1 +
>>> drivers/iio/timer/Kconfig | 15 +
>>> drivers/iio/timer/Makefile | 1 +
>>> drivers/iio/timer/stm32-iio-timer.c | 448 +++++++++++++++++++++
>>> drivers/iio/trigger/Kconfig | 1 -
>>> include/dt-bindings/iio/timer/st,stm32-iio-timer.h | 23 ++
>>> include/linux/iio/timer/stm32-iio-timers.h | 16 +
>>> 8 files changed, 505 insertions(+), 2 deletions(-)
>>> create mode 100644 drivers/iio/timer/Kconfig
>>> create mode 100644 drivers/iio/timer/Makefile
>>> create mode 100644 drivers/iio/timer/stm32-iio-timer.c
>>> create mode 100644 include/dt-bindings/iio/timer/st,stm32-iio-timer.h
>>> create mode 100644 include/linux/iio/timer/stm32-iio-timers.h
>>>
>>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>>> index 6743b18..2de2a80 100644
>>> --- a/drivers/iio/Kconfig
>>> +++ b/drivers/iio/Kconfig
>>> @@ -90,5 +90,5 @@ source "drivers/iio/potentiometer/Kconfig"
>>> source "drivers/iio/pressure/Kconfig"
>>> source "drivers/iio/proximity/Kconfig"
>>> source "drivers/iio/temperature/Kconfig"
>>> -
>>> +source "drivers/iio/timer/Kconfig"
>>> endif # IIO
>>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>>> index 87e4c43..b797c08 100644
>>> --- a/drivers/iio/Makefile
>>> +++ b/drivers/iio/Makefile
>>> @@ -32,4 +32,5 @@ obj-y += potentiometer/
>>> obj-y += pressure/
>>> obj-y += proximity/
>>> obj-y += temperature/
>>> +obj-y += timer/
>>> obj-y += trigger/
>>> diff --git a/drivers/iio/timer/Kconfig b/drivers/iio/timer/Kconfig
>>> new file mode 100644
>>> index 0000000..7a73bc6
>>> --- /dev/null
>>> +++ b/drivers/iio/timer/Kconfig
>>> @@ -0,0 +1,15 @@
>>> +#
>>> +# Timers drivers
>>> +
>>> +menu "Timers"
>>> +
>>> +config IIO_STM32_TIMER
>>> + tristate "stm32 iio timer"
>>> + depends on ARCH_STM32
>>> + depends on OF
>>> + select IIO_TRIGGERED_EVENT
>>> + select MFD_STM32_GP_TIMER
>>> + help
>>> + Select this option to enable stm32 timers hardware IPs
>>> +
>>> +endmenu
>>> diff --git a/drivers/iio/timer/Makefile b/drivers/iio/timer/Makefile
>>> new file mode 100644
>>> index 0000000..a360c9f
>>> --- /dev/null
>>> +++ b/drivers/iio/timer/Makefile
>>> @@ -0,0 +1 @@
>>> +obj-$(CONFIG_IIO_STM32_TIMER) += stm32-iio-timer.o
>>> diff --git a/drivers/iio/timer/stm32-iio-timer.c b/drivers/iio/timer/stm32-iio-timer.c
>>> new file mode 100644
>>> index 0000000..35f2687
>>> --- /dev/null
>>> +++ b/drivers/iio/timer/stm32-iio-timer.c
>>> @@ -0,0 +1,448 @@
>>> +/*
>>> + * stm32-iio-timer.c
>>> + *
>>> + * Copyright (C) STMicroelectronics 2016
>>> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
>>> + * License terms: GNU General Public License (GPL), version 2
>>> + */
>>> +
>>> +#include <linux/iio/iio.h>
>>> +#include <linux/iio/sysfs.h>
>>> +#include <linux/iio/timer/stm32-iio-timers.h>
>>> +#include <linux/iio/trigger.h>
>>> +#include <linux/iio/triggered_event.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/mfd/stm32-gptimer.h>
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +
>>> +#define DRIVER_NAME "stm32-iio-timer"
>>> +
>>> +struct stm32_iio_timer_dev {
>>> + struct device *dev;
>>> + struct regmap *regmap;
>>> + struct clk *clk;
>>> + int irq;
>>> + bool own_timer;
>>> + unsigned int sampling_frequency;
>>> + struct iio_trigger *active_trigger;
>>> +};
>>> +
>>> +static ssize_t _store_frequency(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t len)
>>> +{
>>> + struct iio_trigger *trig = to_iio_trigger(dev);
>>> + struct stm32_iio_timer_dev *stm32 = iio_trigger_get_drvdata(trig);
>>> + unsigned int freq;
>>> + int ret;
>>> +
>>> + ret = kstrtouint(buf, 10, &freq);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + stm32->sampling_frequency = freq;
>>> +
>>> + return len;
>>> +}
>>> +
>>> +static ssize_t _read_frequency(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct iio_trigger *trig = to_iio_trigger(dev);
>>> + struct stm32_iio_timer_dev *stm32 = iio_trigger_get_drvdata(trig);
>>> + unsigned long long freq = stm32->sampling_frequency;
>>> + u32 psc, arr, cr1;
>>> +
>>> + regmap_read(stm32->regmap, TIM_CR1, &cr1);
>>> + regmap_read(stm32->regmap, TIM_PSC, &psc);
>>> + regmap_read(stm32->regmap, TIM_ARR, &arr);
>>> +
>>> + if (psc && arr && (cr1 & TIM_CR1_CEN)) {
>>> + freq = (unsigned long long)clk_get_rate(stm32->clk);
>>> + do_div(freq, psc);
>>> + do_div(freq, arr);
>>> + }
>>> +
>>> + return sprintf(buf, "%d\n", (unsigned int)freq);
>>> +}
>>> +
>>> +static IIO_DEV_ATTR_SAMP_FREQ(S_IWUSR | S_IRUGO,
>>> + _read_frequency,
>>> + _store_frequency);
>>> +
>>> +static struct attribute *stm32_trigger_attrs[] = {
>>> + &iio_dev_attr_sampling_frequency.dev_attr.attr,
>>> + NULL,
>>> +};
>>> +
>>> +static const struct attribute_group stm32_trigger_attr_group = {
>>> + .attrs = stm32_trigger_attrs,
>>> +};
>>> +
>>> +static const struct attribute_group *stm32_trigger_attr_groups[] = {
>>> + &stm32_trigger_attr_group,
>>> + NULL,
>>> +};
>>> +
>>> +static
>>> +ssize_t _show_master_mode(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct stm32_iio_timer_dev *stm32 = iio_priv(indio_dev);
>>> + u32 cr2;
>>> +
>>> + regmap_read(stm32->regmap, TIM_CR2, &cr2);
>>> +
>>> + return snprintf(buf, PAGE_SIZE, "%d\n", (cr2 >> 4) & 0x7);
>>> +}
>>> +
>>> +static
>>> +ssize_t _store_master_mode(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t len)
>>> +{
>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct stm32_iio_timer_dev *stm32 = iio_priv(indio_dev);
>>> + u8 mode;
>>> + int ret;
>>> +
>>> + ret = kstrtou8(buf, 10, &mode);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (mode > 0x7)
>>> + return -EINVAL;
>>> +
>>> + regmap_update_bits(stm32->regmap, TIM_CR2, TIM_CR2_MMS, mode << 4);
>>> +
>>> + return len;
>>> +}
>>> +
>>> +static IIO_DEVICE_ATTR(master_mode, S_IRUGO | S_IWUSR,
>>> + _show_master_mode,
>>> + _store_master_mode,
>>> + 0);
>>> +
>>> +static
>>> +ssize_t _show_slave_mode(struct device *dev,
>>> + struct device_attribute *attr, char *buf)
>>> +{
>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct stm32_iio_timer_dev *stm32 = iio_priv(indio_dev);
>>> + u32 smcr;
>>> +
>>> + regmap_read(stm32->regmap, TIM_SMCR, &smcr);
>>> +
>>> + return snprintf(buf, PAGE_SIZE, "%d\n", smcr & 0x3);
>>> +}
>>> +
>>> +static
>>> +ssize_t _store_slave_mode(struct device *dev,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t len)
>>> +{
>>> + struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>>> + struct stm32_iio_timer_dev *stm32 = iio_priv(indio_dev);
>>> + u8 mode;
>>> + int ret;
>>> +
>>> + ret = kstrtou8(buf, 10, &mode);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + if (mode > 0x7)
>>> + return -EINVAL;
>> How is something called slave mode going to be fed a number between 0 and 7?
>> Rule of thumb is no magic numbers in sysfs and right now this is looking
>> rather cryptic to say the least!
>
> I would like to use strings here, it is possible to use IIO_CONST_ATTR
> to describe them ?
> In documentation slave modes are describe that this:
> 000: Slave mode disabled - if CEN = ?1? then the prescaler is clocked
> directly by the internal clock.
> 001: Encoder mode 1 - Counter counts up/down on TI2FP1 edge depending
> on TI1FP2 level.
> 010: Encoder mode 2 - Counter counts up/down on TI1FP2 edge depending
> on TI2FP1 level.
> 011: Encoder mode 3 - Counter counts up/down on both TI1FP1 and TI2FP2
> edges depending on the level of the other input.
> 100: Reset Mode - Rising edge of the selected trigger input (TRGI)
> reinitializes the counter and generates an update of the registers.
> 101: Gated Mode - The counter clock is enabled when the trigger input
> (TRGI) is high.
> The counter stops (but is not reset) as soon as the trigger becomes low.
> Both start and stop of the counter are controlled.
> 110: Trigger Mode - The counter starts at a rising edge of the trigger
> TRGI (but it is notreset).
> Only the start of the counter is controlled.
> 111: External Clock Mode 1 - Rising edges of the selected trigger
> (TRGI) clock the counter.
>
>>> +
>>> + regmap_update_bits(stm32->regmap, TIM_SMCR, TIM_SMCR_SMS, mode);
>>> +
>>> + return len;
>>> +}
>>> +
>>> +static IIO_DEVICE_ATTR(slave_mode, S_IRUGO | S_IWUSR,
>> There is an iritating move (in terms of noise it's generating) to use values
>> directly instead fo these defines. Still if you don't fix it here I'll only
>> get a patch 'fixing' it soon after...
>
> I will fix at in version 3
>
>>
>>> + _show_slave_mode,
>>> + _store_slave_mode,
>>> + 0);
>>> +
>>> +static struct attribute *stm32_timer_attrs[] = {
>>> + &iio_dev_attr_master_mode.dev_attr.attr,
>>> + &iio_dev_attr_slave_mode.dev_attr.attr,
>> New ABI so must be documented under Documentation/ABI/testing/sysfs-bus-iio-*
>
> OK
>
>>> + NULL,
>>> +};
>>> +
>>> +static const struct attribute_group stm32_timer_attr_group = {
>>> + .attrs = stm32_timer_attrs,
>>> +};
>>> +
>>> +static int stm32_timer_start(struct stm32_iio_timer_dev *stm32)
>>> +{
>>> + unsigned long long prd, div;
>>> + int prescaler = 0;
>>> + u32 max_arr = 0xFFFF, cr1;
>>> +
>>> + if (stm32->sampling_frequency == 0)
>>> + return 0;
>>> +
>>> + /* Period and prescaler values depends of clock rate */
>>> + div = (unsigned long long)clk_get_rate(stm32->clk);
>>> +
>>> + do_div(div, stm32->sampling_frequency);
>>> +
>>> + prd = div;
>>> +
>>> + while (div > max_arr) {
>>> + prescaler++;
>>> + div = prd;
>>> + do_div(div, (prescaler + 1));
>>> + }
>>> + prd = div;
>>> +
>>> + if (prescaler > MAX_TIM_PSC) {
>>> + dev_err(stm32->dev, "prescaler exceeds the maximum value\n");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + /* Check that we own the timer */
>>> + regmap_read(stm32->regmap, TIM_CR1, &cr1);
>>> + if ((cr1 & TIM_CR1_CEN) && !stm32->own_timer)
>>> + return -EBUSY;
>>> +
>>> + if (!stm32->own_timer) {
>>> + stm32->own_timer = true;
>>> + clk_enable(stm32->clk);
>>> + }
>>> +
>>> + regmap_write(stm32->regmap, TIM_PSC, prescaler);
>>> + regmap_write(stm32->regmap, TIM_ARR, prd - 1);
>>> + regmap_update_bits(stm32->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
>>> +
>>> + /* Force master mode to update mode */
>>> + regmap_update_bits(stm32->regmap, TIM_CR2, TIM_CR2_MMS, 0x20);
>>> +
>>> + /* Make sure that registers are updated */
>>> + regmap_update_bits(stm32->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
>>> +
>>> + /* Enable interrupt */
>>> + regmap_write(stm32->regmap, TIM_SR, 0);
>>> + regmap_update_bits(stm32->regmap, TIM_DIER, TIM_DIER_UIE, TIM_DIER_UIE);
>>> +
>>> + /* Enable controller */
>>> + regmap_update_bits(stm32->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int stm32_timer_stop(struct stm32_iio_timer_dev *stm32)
>>> +{
>>> + if (!stm32->own_timer)
>>> + return 0;
>>> +
>>> + /* Stop timer */
>>> + regmap_update_bits(stm32->regmap, TIM_DIER, TIM_DIER_UIE, 0);
>>> + regmap_update_bits(stm32->regmap, TIM_CR1, TIM_CR1_CEN, 0);
>>> + regmap_write(stm32->regmap, TIM_PSC, 0);
>>> + regmap_write(stm32->regmap, TIM_ARR, 0);
>>> +
>>> + clk_disable(stm32->clk);
>>> +
>>> + stm32->own_timer = false;
>>> + stm32->active_trigger = NULL;
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int stm32_set_trigger_state(struct iio_trigger *trig, bool state)
>>> +{
>>> + struct stm32_iio_timer_dev *stm32 = iio_trigger_get_drvdata(trig);
>>> +
>>> + stm32->active_trigger = trig;
>>> +
>>> + if (state)
>>> + return stm32_timer_start(stm32);
>>> + else
>>> + return stm32_timer_stop(stm32);
>>> +}
>>> +
>>> +static irqreturn_t stm32_timer_irq_handler(int irq, void *private)
>>> +{
>>> + struct stm32_iio_timer_dev *stm32 = private;
>>> + u32 sr;
>>> +
>>> + regmap_read(stm32->regmap, TIM_SR, &sr);
>>> + regmap_write(stm32->regmap, TIM_SR, 0);
>>> +
>>> + if ((sr & TIM_SR_UIF) && stm32->active_trigger)
>>> + iio_trigger_poll(stm32->active_trigger);
>> This is acting like a trigger cascading off another trigger?
>
> Not only a trigger but ADC or DAC too.
>
>>
>> Normally this interrupt handler would be directly associated with the
>> trigger hardware - in this case the timer.
>
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static const struct iio_trigger_ops timer_trigger_ops = {
>>> + .owner = THIS_MODULE,
>>> + .set_trigger_state = stm32_set_trigger_state,
>>> +};
>>> +
>>> +static int stm32_setup_iio_triggers(struct stm32_iio_timer_dev *stm32)
>>> +{
>>> + int ret;
>>> + struct property *p;
>>> + const char *cur = NULL;
>>> +
>>> + p = of_find_property(stm32->dev->of_node,
>>> + "st,output-triggers-names", NULL);
>>> +
>>> + while ((cur = of_prop_next_string(p, cur)) != NULL) {
>>> + struct iio_trigger *trig;
>>> +
>>> + trig = devm_iio_trigger_alloc(stm32->dev, "%s", cur);
>>> + if (!trig)
>>> + return -ENOMEM;
>>> +
>>> + trig->dev.parent = stm32->dev->parent;
>>> + trig->ops = &timer_trigger_ops;
>>> + trig->dev.groups = stm32_trigger_attr_groups;
>>> + iio_trigger_set_drvdata(trig, stm32);
>>> +
>>> + ret = devm_iio_trigger_register(stm32->dev, trig);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +/**
>>> + * is_stm32_iio_timer_trigger
>>> + * @trig: trigger to be checked
>>> + *
>>> + * return true if the trigger is a valid stm32 iio timer trigger
>>> + * either return false
>>> + */
>>> +bool is_stm32_iio_timer_trigger(struct iio_trigger *trig)
>>> +{
>>> + return (trig->ops == &timer_trigger_ops);
>>> +}
>>> +EXPORT_SYMBOL(is_stm32_iio_timer_trigger);
>>> +
>>> +static int stm32_validate_trigger(struct iio_dev *indio_dev,
>>> + struct iio_trigger *trig)
>>> +{
>>> + struct stm32_iio_timer_dev *dev = iio_priv(indio_dev);
>>> + int ret;
>>> +
>>> + if (!is_stm32_iio_timer_trigger(trig))
>>> + return -EINVAL;
>>> +
>>> + ret = of_property_match_string(dev->dev->of_node,
>>> + "st,input-triggers-names",
>>> + trig->name);
>>> +
>>> + if (ret < 0)
>>> + return ret;
>>> +
>>> + regmap_update_bits(dev->regmap, TIM_SMCR, TIM_SMCR_TS, ret << 4);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct iio_info stm32_trigger_info = {
>>> + .driver_module = THIS_MODULE,
>>> + .validate_trigger = stm32_validate_trigger,
>>> + .attrs = &stm32_timer_attr_group,
>>> +};
>>> +
>>> +static struct stm32_iio_timer_dev *stm32_setup_iio_device(struct device *dev)
>>> +{
>>> + struct iio_dev *indio_dev;
>>> + int ret;
>>> +
>>> + indio_dev = devm_iio_device_alloc(dev, sizeof(struct stm32_iio_timer_dev));
>>> + if (!indio_dev)
>>> + return NULL;
>> This is 'unusual'. Why does a trigger driver need an iio_dev at all?
>
> Trigger doesn't need it but for configuring the input switch when
> validating the triggers I need a device
As suggested above, lets pull this trigger cascade clear of involving devices
at all. It's nice functionality to have anyway. Once we've figured it
out for this driver, I'd like to generalize it as I think the same stuff could
be used to do clean setup of oscilloscope sampling approaches for more
complex sensor setups...
>
>>> +
>>> + indio_dev->name = dev_name(dev);
>>> + indio_dev->dev.parent = dev;
>>> + indio_dev->info = &stm32_trigger_info;
>>> + indio_dev->modes = INDIO_EVENT_TRIGGERED;
>>> + indio_dev->num_channels = 0;
>>> + indio_dev->dev.of_node = dev->of_node;
>>> +
>>> + ret = iio_triggered_event_setup(indio_dev,
>>> + NULL,
>>> + stm32_timer_irq_handler);
>> So the iio_dev exists to provide the ability to fire this interrupt from
>> another trigger? Why do you want to do this?
>
> I need interrupt because I use set_trigger_state() to enable/disable
> the sampling frequency.
> As far I understand and test set_trigger_state() is only called when
> indio_dev->modes = INDIO_EVENT_TRIGGERED
> and iio_triggered_event_setup have been called to create register an
> irq handler.
> I just need irq declaration for this last condition, I don't need the
> irq to fire in kernel to drive other hardware block.
>
> If I could use set_trigger_state() without calling using
> iio_triggered_event_setup() I should remove all
> irq code from the driver.
>
> One possible solution would be to add calls to set_trigger_state() in
> iio_trigger_write_current() function
> at the same level than iio_trigger_detach_poll_func() or
> iio_trigger_attach_poll_func() calls:
I fear this may introduce race conditions in drivers that assume this stuff
can't change whilst the trigger is enabled.
Bit too risky a change to my mind.
If you need to add functions to explicitly do such a trigger enable, then
feel free to propose them. I never have a problem with adding core
functionality if that is the best way to solve a particular issue.
(subject to standard questions of maintainability and insisting they have
good documentation - do as I say, not as I did years ago ;)
>
> if (indio_dev->modes = INDIO_DIRECT_MODE && oldtrig->ops->set_trigger_state)
> oldtrig->ops->set_trigger_state(oldtrig, false);
>
> if (indio_dev->modes = INDIO_DIRECT_MODE &&
> indio_dev->trig->ops->set_trigger_state)
> indio_dev->trig->ops->set_trigger_state(indio_dev->trig, true);
>
> I'm to new in IIO framework to understand if that it possible or not
>
>>> + if (ret)
>>> + return NULL;
>>> +
>>> + ret = devm_iio_device_register(dev, indio_dev);
>>> + if (ret) {
>>> + iio_triggered_event_cleanup(indio_dev);
>>> + return NULL;
>>> + }
>>> +
>>> + return iio_priv(indio_dev);
>>> +}
>>> +
>>> +static int stm32_iio_timer_probe(struct platform_device *pdev)
>>> +{
>>> + struct device *dev = &pdev->dev;
>>> + struct stm32_iio_timer_dev *stm32;
>>> + struct stm32_gptimer_dev *mfd = dev_get_drvdata(pdev->dev.parent);
>>> + int ret;
>>> +
>>> + stm32 = stm32_setup_iio_device(dev);
>>> + if (!stm32)
>>> + return -ENOMEM;
>>> +
>>> + stm32->dev = dev;
>>> + stm32->regmap = mfd->regmap;
>>> + stm32->clk = mfd->clk;
>>> +
>>> + stm32->irq = platform_get_irq(pdev, 0);
>>> + if (stm32->irq < 0)
>>> + return -EINVAL;
>>> +
>>> + ret = devm_request_irq(stm32->dev, stm32->irq,
>>> + stm32_timer_irq_handler, IRQF_SHARED,
>>> + "iiotimer_event", stm32);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + ret = stm32_setup_iio_triggers(stm32);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + platform_set_drvdata(pdev, stm32);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int stm32_iio_timer_remove(struct platform_device *pdev)
>>> +{
>>> + struct stm32_iio_timer_dev *stm32 = platform_get_drvdata(pdev);
>>> +
>>> + iio_triggered_event_cleanup((struct iio_dev *)stm32);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct of_device_id stm32_trig_of_match[] = {
>>> + {
>>> + .compatible = "st,stm32-iio-timer",
>>> + },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, stm32_trig_of_match);
>>> +
>>> +static struct platform_driver stm32_iio_timer_driver = {
>>> + .probe = stm32_iio_timer_probe,
>>> + .remove = stm32_iio_timer_remove,
>>> + .driver = {
>>> + .name = DRIVER_NAME,
>>> + .of_match_table = stm32_trig_of_match,
>>> + },
>>> +};
>>> +module_platform_driver(stm32_iio_timer_driver);
>>> +
>>> +MODULE_ALIAS("platform:" DRIVER_NAME);
>>> +MODULE_DESCRIPTION("STMicroelectronics STM32 iio timer driver");
>>> +MODULE_LICENSE("GPL");
>>> diff --git a/drivers/iio/trigger/Kconfig b/drivers/iio/trigger/Kconfig
>>> index 809b2e7..f2af4fe 100644
>>> --- a/drivers/iio/trigger/Kconfig
>>> +++ b/drivers/iio/trigger/Kconfig
>>> @@ -46,5 +46,4 @@ config IIO_SYSFS_TRIGGER
>>>
>>> To compile this driver as a module, choose M here: the
>>> module will be called iio-trig-sysfs.
>>> -
>> Clear this out...
>>> endmenu
>>> diff --git a/include/dt-bindings/iio/timer/st,stm32-iio-timer.h b/include/dt-bindings/iio/timer/st,stm32-iio-timer.h
>>> new file mode 100644
>>> index 0000000..d39bf16
>>> --- /dev/null
>>> +++ b/include/dt-bindings/iio/timer/st,stm32-iio-timer.h
>>> @@ -0,0 +1,23 @@
>>> +/*
>>> + * st,stm32-iio-timer.h
>>> + *
>>> + * Copyright (C) STMicroelectronics 2016
>>> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
>>> + * License terms: GNU General Public License (GPL), version 2
>>> + */
>>> +
>>> +#ifndef _DT_BINDINGS_IIO_TIMER_H_
>>> +#define _DT_BINDINGS_IIO_TIMER_H_
>>> +
>>> +#define TIM1_TRGO "tim1_trgo"
>>> +#define TIM2_TRGO "tim2_trgo"
>>> +#define TIM3_TRGO "tim3_trgo"
>>> +#define TIM4_TRGO "tim4_trgo"
>>> +#define TIM5_TRGO "tim5_trgo"
>>> +#define TIM6_TRGO "tim6_trgo"
>>> +#define TIM7_TRGO "tim7_trgo"
>>> +#define TIM8_TRGO "tim8_trgo"
>>> +#define TIM9_TRGO "tim9_trgo"
>>> +#define TIM12_TRGO "tim12_trgo"
>>> +
>>> +#endif
>>> diff --git a/include/linux/iio/timer/stm32-iio-timers.h b/include/linux/iio/timer/stm32-iio-timers.h
>>> new file mode 100644
>>> index 0000000..5d1b86c
>>> --- /dev/null
>>> +++ b/include/linux/iio/timer/stm32-iio-timers.h
>>> @@ -0,0 +1,16 @@
>>> +/*
>>> + * stm32-iio-timers.h
>>> + *
>>> + * Copyright (C) STMicroelectronics 2016
>>> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
>>> + * License terms: GNU General Public License (GPL), version 2
>>> + */
>>> +
>>> +#ifndef _STM32_IIO_TIMERS_H_
>>> +#define _STM32_IIO_TIMERS_H_
>>> +
>>> +#include <dt-bindings/iio/timer/st,stm32-iio-timer.h>
>>> +
>>> +bool is_stm32_iio_timer_trigger(struct iio_trigger *trig);
>>> +
>>> +#endif
>>>
>>
^ permalink raw reply
* [PATCH v3 0/7] Add pwm and IIO timer drivers for stm32
From: Jonathan Cameron @ 2016-12-03 9:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480673842-20804-1-git-send-email-benjamin.gaignard@st.com>
On 02/12/16 10:17, Benjamin Gaignard wrote:
> version 3:
> - no change on mfd and pwm divers patches
> - add cross reference between bindings
> - change compatible to "st,stm32-timer-trigger"
> - fix attributes access rights
> - use string instead of int for master_mode and slave_mode
> - document device attributes in sysfs-bus-iio-timer-stm32
> - udpate DT with the new compatible
>
> version 2:
> - keep only one compatible per driver
> - use DT parameters to describe hardware block configuration:
> - pwm channels, complementary output, counter size, break input
> - triggers accepted and create by IIO timers
> - change DT to limite use of reference to the node
> - interrupt is now in IIO timer driver
> - rename stm32-mfd-timer to stm32-gptimer (for general purpose timer)
>
> The following patches enable pwm and IIO Timer features for stm32 platforms.
>
> Those two features are mixed into the registers of the same hardware block
> (named general purpose timer) which lead to introduce a multifunctions driver
> on the top of them to be able to share the registers.
>
> In stm32 14 instances of timer hardware block exist, even if they all have
> the same register mapping they could have a different number of pwm channels
> and/or different triggers capabilities. We use various parameters in DT to
> describe the differences between hardware blocks
>
> The MFD (stm32-gptimer.c) takes care of clock and register mapping
> by using regmap. stm32_gptimer_dev structure is provided to its sub-node to
> share those information.
>
> PWM driver is implemented into pwm-stm32.c. Depending of the instance we may
> have up to 4 channels, sometime with complementary outputs or 32 bits counter
> instead of 16 bits. Some hardware blocks may also have a break input function
> which allows to stop pwm depending of a level, defined in devicetree, on an
> external pin.
>
> IIO timer driver (stm32-iio-timer.c and stm32-iio-timers.h) define a list of
> hardware triggers usable by hardware blocks like ADC, DAC or other timers.
>
> The matrix of possible connections between blocks is quite complex so we use
> trigger names and is_stm32_iio_timer_trigger() function to be sure that
> triggers are valid and configure the IPs.
> Possible triggers ar listed in include/dt-bindings/iio/timer/st,stm32-iio-timer.h
>
> At run time IIO timer hardware blocks can configure (through "master_mode"
> IIO device attribute) which internal signal (counter enable, reset,
> comparison block, etc...) is used to generate the trigger.
>
> By using "slave_mode" IIO device attribute timer can also configure on which
> event (level, rising edge) of the block is enabled.
>
> Since we can use trigger from one hardware to control an other block, we can
> use a pwm to control an other one. The following example shows how to configure
> pwm1 and pwm3 to make pwm3 generate pulse only when pwm1 pulse level is high.
>
> /sys/bus/iio/devices # ls
> iio:device0 iio:device1 trigger0 trigger1
>
> configure timer1 to use pwm1 channel 0 as output trigger
> /sys/bus/iio/devices # echo 'OC1REF' > iio\:device0/master_mode
> configure timer3 to enable only when input is high
> /sys/bus/iio/devices # echo 'gated' > iio\:device1/slave_mode
> /sys/bus/iio/devices # cat trigger0/name
> tim1_trgo
> configure timer2 to use timer1 trigger is input
> /sys/bus/iio/devices # echo "tim1_trgo" > iio\:device1/trigger/current_trigger
>
> configure pwm3 channel 0 to generate a signal with a period of 100ms and a
> duty cycle of 50%
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3 at 0/pwm/pwmchip4 # echo 0 > export
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3 at 0/pwm/pwmchip4 # echo 100000000 > pwm0/period
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3 at 0/pwm/pwmchip4 # echo 50000000 > pwm0/duty_cycle
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3 at 0/pwm/pwmchip4# echo 1 > pwm0/enable
> here pwm3 channel 0, as expected, doesn't start because has to be triggered by
> pwm1 channel 0
>
> configure pwm1 channel 0 to generate a signal with a period of 1s and a
> duty cycle of 50%
> /sys/devices/platform/soc/40010000.gptimer1/40010000.gptimer1:pwm1 at 0/pwm/pwmchip0 # echo 0 > export
> /sys/devices/platform/soc/40010000.gptimer1/40010000.gptimer1:pwm1 at 0/pwm/pwmchip0 # echo 1000000000 > pwm0/period
> /sys/devices/platform/soc/40010000.gptimer1/40010000.gptimer1:pwm1 at 0/pwm/pwmchip0 # echo 500000000 > pwm0/duty_cycle
> /sys/devices/platform/soc/40010000.gptimer1/40010000.gptimer1:pwm1 at 0/pwm/pwmchip0 # echo 1 > pwm0/enable
> finally pwm1 starts and pwm3 only generates pulse when pwm1 signal is high
>
> An other example to use a timer as source of clock for another device.
> Here timer1 is used a source clock for pwm3:
>
> /sys/bus/iio/devices # echo 100000 > trigger0/sampling_frequency
> /sys/bus/iio/devices # echo tim1_trgo > iio\:device1/trigger/current_trigger
> /sys/bus/iio/devices # echo 'external_clock' > iio\:device1/slave_mode
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3 at 0/pwm/pwmchip4 # echo 0 > export
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3 at 0/pwm/pwmchip4 # echo 1000000 > pwm0/period
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3 at 0/pwm/pwmchip4 # echo 500000 > pwm0/duty_cycle
> /sys/devices/platform/soc/40000400.gptimer3/40000400.gptimer3:pwm3 at 0/pwm/pwmchip4 # echo 1 > pwm0/enable
This is good thorough documentation. Could we have an additional
patch adding just this documentation to the tree (probably under
Documentation/mfd?). Documentation in general is a bit in flux at the
moment so we'll may want to sphixify it afterwards but plain text
is fine for now.
Jonathan
>
> Benjamin Gaignard (7):
> MFD: add bindings for stm32 general purpose timer driver
> MFD: add stm32 general purpose timer driver
> PWM: add pwm-stm32 DT bindings
> PWM: add pwm driver for stm32 plaftorm
> IIO: add bindings for stm32 timer trigger driver
> IIO: add STM32 timer trigger driver
> ARM: dts: stm32: add stm32 general purpose timer driver in DT
>
> .../ABI/testing/sysfs-bus-iio-timer-stm32 | 47 ++
> .../bindings/iio/timer/stm32-timer-trigger.txt | 39 ++
> .../bindings/mfd/stm32-general-purpose-timer.txt | 47 ++
> .../devicetree/bindings/pwm/pwm-stm32.txt | 38 ++
> arch/arm/boot/dts/stm32f429.dtsi | 333 +++++++++++++-
> arch/arm/boot/dts/stm32f469-disco.dts | 28 ++
> drivers/iio/Kconfig | 2 +-
> drivers/iio/Makefile | 1 +
> drivers/iio/timer/Kconfig | 15 +
> drivers/iio/timer/Makefile | 1 +
> drivers/iio/timer/stm32-timer-trigger.c | 477 +++++++++++++++++++++
> drivers/iio/trigger/Kconfig | 1 -
> drivers/mfd/Kconfig | 10 +
> drivers/mfd/Makefile | 2 +
> drivers/mfd/stm32-gptimer.c | 73 ++++
> drivers/pwm/Kconfig | 8 +
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-stm32.c | 285 ++++++++++++
> .../iio/timer/st,stm32-timer-triggers.h | 60 +++
> include/linux/iio/timer/stm32-timer-trigger.h | 16 +
> include/linux/mfd/stm32-gptimer.h | 62 +++
> 21 files changed, 1543 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> create mode 100644 Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt
> create mode 100644 Documentation/devicetree/bindings/mfd/stm32-general-purpose-timer.txt
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-stm32.txt
> create mode 100644 drivers/iio/timer/Kconfig
> create mode 100644 drivers/iio/timer/Makefile
> create mode 100644 drivers/iio/timer/stm32-timer-trigger.c
> create mode 100644 drivers/mfd/stm32-gptimer.c
> create mode 100644 drivers/pwm/pwm-stm32.c
> create mode 100644 include/dt-bindings/iio/timer/st,stm32-timer-triggers.h
> create mode 100644 include/linux/iio/timer/stm32-timer-trigger.h
> create mode 100644 include/linux/mfd/stm32-gptimer.h
>
^ permalink raw reply
* [PATCH v3 5/7] IIO: add bindings for stm32 timer trigger driver
From: Jonathan Cameron @ 2016-12-03 9:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CA+M3ks7GhvApJwZVFK7KAbbvbAhp40sdOVvhkyZWBHZXpLfZDQ@mail.gmail.com>
On 02/12/16 14:23, Benjamin Gaignard wrote:
> 2016-12-02 14:59 GMT+01:00 Lee Jones <lee.jones@linaro.org>:
>> On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
>>
>>> Define bindings for stm32 timer trigger
>>>
>>> version 3:
>>> - change file name
>>> - add cross reference with mfd bindings
>>>
>>> version 2:
>>> - only keep one compatible
>>> - add DT parameters to set lists of the triggers:
>>> one list describe the triggers created by the device
>>> another one give the triggers accepted by the device
>>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>> ---
>>> .../bindings/iio/timer/stm32-timer-trigger.txt | 39 ++++++++++++++++++++++
>>> 1 file changed, 39 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt b/Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt
>>> new file mode 100644
>>> index 0000000..858816d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/timer/stm32-timer-trigger.txt
>>> @@ -0,0 +1,39 @@
>>> +timer trigger bindings for STM32
>>> +
>>> +Must be a sub-node of STM32 general purpose timer driver
>>> +Parent node properties are describe in ../mfd/stm32-general-purpose-timer.txt
>>> +
>>> +Required parameters:
>>> +- compatible: must be "st,stm32-iio-timer"
>>> +- interrupts: Interrupt for this device
>>> + See ../interrupt-controller/st,stm32-exti.txt
>>> +
>>> +Optional parameters:
>>> +- st,input-triggers-names: List of the possible input triggers for
>>> + the device
>>> +- st,output-triggers-names: List of the possible output triggers for
>>> + the device
>>> +
>>> +Possible triggers are defined in include/dt-bindings/iio/timer/st,stm32-timer-trigger.h
>>> +
>>> +Example:
>>> + gptimer1: gptimer1 at 40010000 {
>>> + compatible = "st,stm32-gptimer";
>>> + reg = <0x40010000 0x400>;
>>> + clocks = <&rcc 0 160>;
>>> + clock-names = "clk_int";
>>> +
>>> + timer1 at 0 {
>>> + compatible = "st,stm32-timer-trigger";
>>> + interrupts = <27>;
>>> + st,input-triggers-names = TIM5_TRGO,
>>> + TIM2_TRGO,
>>> + TIM4_TRGO,
>>> + TIM3_TRGO;
>>> + st,output-triggers-names = TIM1_TRGO,
>>> + TIM1_CH1,
>>> + TIM1_CH2,
>>> + TIM1_CH3,
>>> + TIM1_CH4;
>>
>> I see why you've done it like this now ... because it makes things
>> easier for you in the driver, since the IIO subsystem matches on names
>> such as these.
>>
>> BUT, this is a Linux-implementation-ism. Just use pairs of integers
>> and create the Linux-ism strings in the driver.
>
> The goal is not to make things easier in driver but to be able to share
> the triggers names with other drivers like DAC or ADC.
> If each driver have to create it own triggers names it will more difficult
> to keep them coherent than it they share the same definitions
Do it by documentation. This will be effectively ABI going forward
so fixed once it is defined. Should be fairly easy to tell during testing
if someone has messed it up ;)
Jonathan
>
>>
>>> + };
>>> + };
>>
>> --
>> Lee Jones
>> Linaro STMicroelectronics Landing Team Lead
>> Linaro.org ? Open source software for ARM SoCs
>> Follow Linaro: Facebook | Twitter | Blog
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer driver in DT
From: Jonathan Cameron @ 2016-12-03 9:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161202132251.GL2683@dell>
On 02/12/16 13:22, Lee Jones wrote:
> On Fri, 02 Dec 2016, Benjamin Gaignard wrote:
>
>> Add general purpose timers and it sub-nodes into DT for stm32f4.
>> Define and enable pwm1 and pwm3 for stm32f469 discovery board
>>
>> version 3:
>> - use "st,stm32-timer-trigger" in DT
>>
>> version 2:
>> - use parameters to describe hardware capabilities
>> - do not use references for pwm and iio timer subnodes
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>> arch/arm/boot/dts/stm32f429.dtsi | 333 +++++++++++++++++++++++++++++++++-
>> arch/arm/boot/dts/stm32f469-disco.dts | 28 +++
>> 2 files changed, 360 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
>> index bca491d..8c50d03 100644
>> --- a/arch/arm/boot/dts/stm32f429.dtsi
>> +++ b/arch/arm/boot/dts/stm32f429.dtsi
>> @@ -48,7 +48,7 @@
>> #include "skeleton.dtsi"
>> #include "armv7-m.dtsi"
>> #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
>> -
>> +#include <dt-bindings/iio/timer/st,stm32-timer-triggers.h>
>> / {
>> clocks {
>> clk_hse: clk-hse {
>> @@ -355,6 +355,21 @@
>> slew-rate = <2>;
>> };
>> };
>> +
>> + pwm1_pins: pwm at 1 {
>> + pins {
>> + pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
>> + <STM32F429_PB13_FUNC_TIM1_CH1N>,
>> + <STM32F429_PB12_FUNC_TIM1_BKIN>;
>> + };
>> + };
>> +
>> + pwm3_pins: pwm at 3 {
>> + pins {
>> + pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
>> + <STM32F429_PB5_FUNC_TIM3_CH2>;
>> + };
>> + };
>> };
>>
>> rcc: rcc at 40023810 {
>> @@ -426,6 +441,322 @@
>> interrupts = <80>;
>> clocks = <&rcc 0 38>;
>> };
>> +
>> + gptimer1: gptimer1 at 40010000 {
>
> timer at xxxxxxx
>
> Node names should be generic and not numbered.
>
> I suggest that this isn't actually a timer either. Is contains a
> timer (and a PWM), but in it's completeness it is not a timer per
> say.
That's just mean ;) At least suggest an alternative?
stm32-gptimerish-magic?
>
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40010000 0x400>;
>> + clocks = <&rcc 0 160>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm1 at 0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + st,breakinput;
>> + st,complementary;
>> + status = "disabled";
>> + };
>> +
>> + timer1 at 0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <27>;
>> + st,input-triggers-names = TIM5_TRGO,
>> + TIM2_TRGO,
>> + TIM4_TRGO,
>> + TIM3_TRGO;
>
> I'm still dubious with matching by strings.
>
> I'll take a look at the C code to see what the alternatives could be.
>
>> + st,output-triggers-names = TIM1_TRGO,
>> + TIM1_CH1,
>> + TIM1_CH2,
>> + TIM1_CH3,
>> + TIM1_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer2: gptimer2 at 40000000 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40000000 0x400>;
>> + clocks = <&rcc 0 128>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm2 at 0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + st,32bits-counter;
>> + status = "disabled";
>> + };
>> +
>> + timer2 at 0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <28>;
>> + st,input-triggers-names = TIM1_TRGO,
>> + TIM8_TRGO,
>> + TIM3_TRGO,
>> + TIM4_TRGO;
>> + st,output-triggers-names = TIM2_TRGO,
>> + TIM2_CH1,
>> + TIM2_CH2,
>> + TIM2_CH3,
>> + TIM2_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer3: gptimer3 at 40000400 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40000400 0x400>;
>> + clocks = <&rcc 0 129>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm3 at 0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + status = "disabled";
>> + };
>> +
>> + timer3 at 0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <29>;
>> + st,input-triggers-names = TIM1_TRGO,
>> + TIM8_TRGO,
>> + TIM5_TRGO,
>> + TIM4_TRGO;
>> + st,output-triggers-names = TIM3_TRGO,
>> + TIM3_CH1,
>> + TIM3_CH2,
>> + TIM3_CH3,
>> + TIM3_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer4: gptimer4 at 40000800 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40000800 0x400>;
>> + clocks = <&rcc 0 130>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm4 at 0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + status = "disabled";
>> + };
>> +
>> + timer4 at 0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <30>;
>> + st,input-triggers-names = TIM1_TRGO,
>> + TIM2_TRGO,
>> + TIM3_TRGO,
>> + TIM8_TRGO;
>> + st,output-triggers-names = TIM4_TRGO,
>> + TIM4_CH1,
>> + TIM4_CH2,
>> + TIM4_CH3,
>> + TIM4_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer5: gptimer5 at 40000C00 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40000C00 0x400>;
>> + clocks = <&rcc 0 131>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm5 at 0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + st,32bits-counter;
>> + status = "disabled";
>> + };
>> +
>> + timer5 at 0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <50>;
>> + st,input-triggers-names = TIM2_TRGO,
>> + TIM3_TRGO,
>> + TIM4_TRGO,
>> + TIM8_TRGO;
>> + st,output-triggers-names = TIM5_TRGO,
>> + TIM5_CH1,
>> + TIM5_CH2,
>> + TIM5_CH3,
>> + TIM5_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer6: gptimer6 at 40001000 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40001000 0x400>;
>> + clocks = <&rcc 0 132>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + timer6 at 0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <54>;
>> + st,output-triggers-names = TIM6_TRGO;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer7: gptimer7 at 40001400 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40001400 0x400>;
>> + clocks = <&rcc 0 133>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + timer7 at 0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <55>;
>> + st,output-triggers-names = TIM7_TRGO;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer8: gptimer8 at 40010400 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40010400 0x400>;
>> + clocks = <&rcc 0 161>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm8 at 0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <4>;
>> + st,complementary;
>> + st,breakinput;
>> + status = "disabled";
>> + };
>> +
>> + timer8 at 0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <46>;
>> + st,input-triggers-names = TIM1_TRGO,
>> + TIM2_TRGO,
>> + TIM4_TRGO,
>> + TIM5_TRGO;
>> + st,output-triggers-names = TIM8_TRGO,
>> + TIM8_CH1,
>> + TIM8_CH2,
>> + TIM8_CH3,
>> + TIM8_CH4;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer9: gptimer9 at 40014000 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40014000 0x400>;
>> + clocks = <&rcc 0 176>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm9 at 0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <2>;
>> + status = "disabled";
>> + };
>> +
>> + timer9 at 0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <24>;
>> + st,input-triggers-names = TIM2_TRGO,
>> + TIM3_TRGO;
>> + st,output-triggers-names = TIM9_TRGO,
>> + TIM9_CH1,
>> + TIM9_CH2;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer10: gptimer10 at 40014400 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40014400 0x400>;
>> + clocks = <&rcc 0 177>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm10 at 0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <1>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer11: gptimer11 at 40014800 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40014800 0x400>;
>> + clocks = <&rcc 0 178>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm11 at 0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <1>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer12: gptimer12 at 40001800 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40001800 0x400>;
>> + clocks = <&rcc 0 134>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm12 at 0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <2>;
>> + status = "disabled";
>> + };
>> +
>> + timer12 at 0 {
>> + compatible = "st,stm32-timer-trigger";
>> + interrupts = <43>;
>> + st,input-triggers-names = TIM4_TRGO,
>> + TIM5_TRGO;
>> + st,output-triggers-names = TIM12_TRGO,
>> + TIM12_CH1,
>> + TIM12_CH2;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer13: gptimer13 at 40001C00 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40001C00 0x400>;
>> + clocks = <&rcc 0 135>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm13 at 0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <1>;
>> + status = "disabled";
>> + };
>> + };
>> +
>> + gptimer14: gptimer14 at 40002000 {
>> + compatible = "st,stm32-gptimer";
>> + reg = <0x40002000 0x400>;
>> + clocks = <&rcc 0 136>;
>> + clock-names = "clk_int";
>> + status = "disabled";
>> +
>> + pwm14 at 0 {
>> + compatible = "st,stm32-pwm";
>> + st,pwm-num-chan = <1>;
>> + status = "disabled";
>> + };
>> + };
>> };
>> };
>>
>> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
>> index 8a163d7..df4ca7e 100644
>> --- a/arch/arm/boot/dts/stm32f469-disco.dts
>> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
>> @@ -81,3 +81,31 @@
>> &usart3 {
>> status = "okay";
>> };
>> +
>> +&gptimer1 {
>> + status = "okay";
>> +
>> + pwm1 at 0 {
>> + pinctrl-0 = <&pwm1_pins>;
>> + pinctrl-names = "default";
>> + status = "okay";
>> + };
>> +
>> + timer1 at 0 {
>> + status = "okay";
>> + };
>> +};
>
> This is a much *better* format than before.
>
> I still don't like the '&' syntax though.
>
>> +&gptimer3 {
>> + status = "okay";
>> +
>> + pwm3 at 0 {
>> + pinctrl-0 = <&pwm3_pins>;
>> + pinctrl-names = "default";
>> + status = "okay";
>> + };
>> +
>> + timer3 at 0 {
>> + status = "okay";
>> + };
>> +};
>
^ permalink raw reply
* [PATCH v3 7/7] ARM: dts: stm32: add stm32 general purpose timer driver in DT
From: Jonathan Cameron @ 2016-12-03 9:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480673842-20804-8-git-send-email-benjamin.gaignard@st.com>
On 02/12/16 10:17, Benjamin Gaignard wrote:
> Add general purpose timers and it sub-nodes into DT for stm32f4.
> Define and enable pwm1 and pwm3 for stm32f469 discovery board
>
> version 3:
> - use "st,stm32-timer-trigger" in DT
>
> version 2:
> - use parameters to describe hardware capabilities
> - do not use references for pwm and iio timer subnodes
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
This didn't come out nearly as badly as I thought it would.
Seems we don't need the multiple compatibles which is good!
> ---
> arch/arm/boot/dts/stm32f429.dtsi | 333 +++++++++++++++++++++++++++++++++-
> arch/arm/boot/dts/stm32f469-disco.dts | 28 +++
> 2 files changed, 360 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
> index bca491d..8c50d03 100644
> --- a/arch/arm/boot/dts/stm32f429.dtsi
> +++ b/arch/arm/boot/dts/stm32f429.dtsi
> @@ -48,7 +48,7 @@
> #include "skeleton.dtsi"
> #include "armv7-m.dtsi"
> #include <dt-bindings/pinctrl/stm32f429-pinfunc.h>
> -
> +#include <dt-bindings/iio/timer/st,stm32-timer-triggers.h>
> / {
> clocks {
> clk_hse: clk-hse {
> @@ -355,6 +355,21 @@
> slew-rate = <2>;
> };
> };
> +
> + pwm1_pins: pwm at 1 {
> + pins {
> + pinmux = <STM32F429_PA8_FUNC_TIM1_CH1>,
> + <STM32F429_PB13_FUNC_TIM1_CH1N>,
> + <STM32F429_PB12_FUNC_TIM1_BKIN>;
> + };
> + };
> +
> + pwm3_pins: pwm at 3 {
> + pins {
> + pinmux = <STM32F429_PB4_FUNC_TIM3_CH1>,
> + <STM32F429_PB5_FUNC_TIM3_CH2>;
> + };
> + };
> };
>
> rcc: rcc at 40023810 {
> @@ -426,6 +441,322 @@
> interrupts = <80>;
> clocks = <&rcc 0 38>;
> };
> +
> + gptimer1: gptimer1 at 40010000 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40010000 0x400>;
> + clocks = <&rcc 0 160>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm1 at 0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <4>;
> + st,breakinput;
> + st,complementary;
> + status = "disabled";
> + };
> +
> + timer1 at 0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <27>;
> + st,input-triggers-names = TIM5_TRGO,
> + TIM2_TRGO,
> + TIM4_TRGO,
> + TIM3_TRGO;
> + st,output-triggers-names = TIM1_TRGO,
> + TIM1_CH1,
> + TIM1_CH2,
> + TIM1_CH3,
> + TIM1_CH4;
> + status = "disabled";
> + };
> + };
> +
> + gptimer2: gptimer2 at 40000000 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40000000 0x400>;
> + clocks = <&rcc 0 128>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm2 at 0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <4>;
> + st,32bits-counter;
> + status = "disabled";
> + };
> +
> + timer2 at 0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <28>;
> + st,input-triggers-names = TIM1_TRGO,
> + TIM8_TRGO,
> + TIM3_TRGO,
> + TIM4_TRGO;
> + st,output-triggers-names = TIM2_TRGO,
> + TIM2_CH1,
> + TIM2_CH2,
> + TIM2_CH3,
> + TIM2_CH4;
> + status = "disabled";
> + };
> + };
> +
> + gptimer3: gptimer3 at 40000400 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40000400 0x400>;
> + clocks = <&rcc 0 129>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm3 at 0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <4>;
> + status = "disabled";
> + };
> +
> + timer3 at 0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <29>;
> + st,input-triggers-names = TIM1_TRGO,
> + TIM8_TRGO,
> + TIM5_TRGO,
> + TIM4_TRGO;
> + st,output-triggers-names = TIM3_TRGO,
> + TIM3_CH1,
> + TIM3_CH2,
> + TIM3_CH3,
> + TIM3_CH4;
> + status = "disabled";
> + };
> + };
> +
> + gptimer4: gptimer4 at 40000800 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40000800 0x400>;
> + clocks = <&rcc 0 130>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm4 at 0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <4>;
> + status = "disabled";
> + };
> +
> + timer4 at 0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <30>;
> + st,input-triggers-names = TIM1_TRGO,
> + TIM2_TRGO,
> + TIM3_TRGO,
> + TIM8_TRGO;
> + st,output-triggers-names = TIM4_TRGO,
> + TIM4_CH1,
> + TIM4_CH2,
> + TIM4_CH3,
> + TIM4_CH4;
> + status = "disabled";
> + };
> + };
> +
> + gptimer5: gptimer5 at 40000C00 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40000C00 0x400>;
> + clocks = <&rcc 0 131>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm5 at 0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <4>;
> + st,32bits-counter;
> + status = "disabled";
> + };
> +
> + timer5 at 0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <50>;
> + st,input-triggers-names = TIM2_TRGO,
> + TIM3_TRGO,
> + TIM4_TRGO,
> + TIM8_TRGO;
> + st,output-triggers-names = TIM5_TRGO,
> + TIM5_CH1,
> + TIM5_CH2,
> + TIM5_CH3,
> + TIM5_CH4;
> + status = "disabled";
> + };
> + };
> +
> + gptimer6: gptimer6 at 40001000 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40001000 0x400>;
> + clocks = <&rcc 0 132>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + timer6 at 0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <54>;
> + st,output-triggers-names = TIM6_TRGO;
> + status = "disabled";
> + };
> + };
> +
> + gptimer7: gptimer7 at 40001400 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40001400 0x400>;
> + clocks = <&rcc 0 133>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + timer7 at 0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <55>;
> + st,output-triggers-names = TIM7_TRGO;
> + status = "disabled";
> + };
> + };
> +
> + gptimer8: gptimer8 at 40010400 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40010400 0x400>;
> + clocks = <&rcc 0 161>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm8 at 0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <4>;
> + st,complementary;
> + st,breakinput;
> + status = "disabled";
> + };
> +
> + timer8 at 0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <46>;
> + st,input-triggers-names = TIM1_TRGO,
> + TIM2_TRGO,
> + TIM4_TRGO,
> + TIM5_TRGO;
> + st,output-triggers-names = TIM8_TRGO,
> + TIM8_CH1,
> + TIM8_CH2,
> + TIM8_CH3,
> + TIM8_CH4;
> + status = "disabled";
> + };
> + };
> +
> + gptimer9: gptimer9 at 40014000 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40014000 0x400>;
> + clocks = <&rcc 0 176>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm9 at 0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <2>;
> + status = "disabled";
> + };
> +
> + timer9 at 0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <24>;
> + st,input-triggers-names = TIM2_TRGO,
> + TIM3_TRGO;
> + st,output-triggers-names = TIM9_TRGO,
> + TIM9_CH1,
> + TIM9_CH2;
> + status = "disabled";
> + };
> + };
> +
> + gptimer10: gptimer10 at 40014400 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40014400 0x400>;
> + clocks = <&rcc 0 177>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm10 at 0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <1>;
> + status = "disabled";
> + };
> + };
> +
> + gptimer11: gptimer11 at 40014800 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40014800 0x400>;
> + clocks = <&rcc 0 178>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm11 at 0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <1>;
> + status = "disabled";
> + };
> + };
> +
> + gptimer12: gptimer12 at 40001800 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40001800 0x400>;
> + clocks = <&rcc 0 134>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm12 at 0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <2>;
> + status = "disabled";
> + };
> +
> + timer12 at 0 {
> + compatible = "st,stm32-timer-trigger";
> + interrupts = <43>;
> + st,input-triggers-names = TIM4_TRGO,
> + TIM5_TRGO;
> + st,output-triggers-names = TIM12_TRGO,
> + TIM12_CH1,
> + TIM12_CH2;
> + status = "disabled";
> + };
> + };
> +
> + gptimer13: gptimer13 at 40001C00 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40001C00 0x400>;
> + clocks = <&rcc 0 135>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm13 at 0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <1>;
> + status = "disabled";
> + };
> + };
> +
> + gptimer14: gptimer14 at 40002000 {
> + compatible = "st,stm32-gptimer";
> + reg = <0x40002000 0x400>;
> + clocks = <&rcc 0 136>;
> + clock-names = "clk_int";
> + status = "disabled";
> +
> + pwm14 at 0 {
> + compatible = "st,stm32-pwm";
> + st,pwm-num-chan = <1>;
> + status = "disabled";
> + };
> + };
> };
> };
>
> diff --git a/arch/arm/boot/dts/stm32f469-disco.dts b/arch/arm/boot/dts/stm32f469-disco.dts
> index 8a163d7..df4ca7e 100644
> --- a/arch/arm/boot/dts/stm32f469-disco.dts
> +++ b/arch/arm/boot/dts/stm32f469-disco.dts
> @@ -81,3 +81,31 @@
> &usart3 {
> status = "okay";
> };
> +
> +&gptimer1 {
> + status = "okay";
> +
> + pwm1 at 0 {
> + pinctrl-0 = <&pwm1_pins>;
> + pinctrl-names = "default";
> + status = "okay";
> + };
> +
> + timer1 at 0 {
> + status = "okay";
> + };
> +};
> +
> +&gptimer3 {
> + status = "okay";
> +
> + pwm3 at 0 {
> + pinctrl-0 = <&pwm3_pins>;
> + pinctrl-names = "default";
> + status = "okay";
> + };
> +
> + timer3 at 0 {
> + status = "okay";
> + };
> +};
>
^ permalink raw reply
* [PATCH v2 1/2] arm64: dts: zx: Fix gic GICR property
From: Marc Zyngier @ 2016-12-03 10:11 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161203092251.GA17375@dragon>
On Sat, Dec 03 2016 at 09:22:55 AM, Shawn Guo <shawnguo@kernel.org> wrote:
> On Fri, Dec 02, 2016 at 05:02:28PM +0000, Marc Zyngier wrote:
>> Just noticed this.
>>
>> On 13/10/16 13:31, Jun Nie wrote:
>> > GICR for multiple CPU can be described with start address and stride,
>> > or with multiple address. Current multiple address and stride are
>> > both used. Fix it.
>> >
>> > vmalloc patch 727a7f5a9 triggered this bug:
>> > [ 0.097146] Unable to handle kernel paging request at virtual address ffff000008060008
>> > [ 0.097150] pgd = ffff000008602000
>> > [ 0.097160] [ffff000008060008] *pgd=000000007fffe003, *pud=000000007fffd003, *pmd=000000007fffc003, *pte=0000000000000000
>> > [ 0.097165] Internal error: Oops: 96000007 [#1] PREEMPT SMP
>> > [ 0.097170] Modules linked in:
>> > [ 0.097177] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0+ #1474
>> > [ 0.097179] Hardware name: ZTE zx296718 evaluation board (DT)
>> > [ 0.097183] task: ffff80003e8c8b80 task.stack: ffff80003e8d0000
>> > [ 0.097197] PC is at gic_populate_rdist+0x74/0x15c
>> > [ 0.097202] LR is at gic_starting_cpu+0xc/0x20
>> > [ 0.097206] pc : [<ffff0000082b1b18>] lr : [<ffff0000082b26e0>] pstate: 600001c5
>> >
>> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
>> > ---
>> > arch/arm64/boot/dts/zte/zx296718.dtsi | 11 +++--------
>> > 1 file changed, 3 insertions(+), 8 deletions(-)
>> >
>> > diff --git a/arch/arm64/boot/dts/zte/zx296718.dtsi b/arch/arm64/boot/dts/zte/zx296718.dtsi
>> > index a223066..6b239a3 100644
>> > --- a/arch/arm64/boot/dts/zte/zx296718.dtsi
>> > +++ b/arch/arm64/boot/dts/zte/zx296718.dtsi
>> > @@ -239,16 +239,11 @@
>> > compatible = "arm,gic-v3";
>> > #interrupt-cells = <3>;
>> > #address-cells = <0>;
>> > - #redistributor-regions = <6>;
>> > - redistributor-stride = <0x0 0x40000>;
>> > + #redistributor-regions = <1>;
>> > + redistributor-stride = <0x20000>;
>>
>> Why is that stride specified? Is the GIC implementation so busted that
>> the GICR_TYPER do not report a GICv3 redistributor, which implies a
>> 128kB stride?
>
> No, it's not required per my testing. I guess it's there for
> documentation purpose to make the stride setting explicit. Are you
> suggesting that we simply drop it?
Indeed. This is only meant as a workaround for some of the most
braindead platforms out there which have a redistributor stride that
deviates from what the architecture defines (128kB for GICv3, 256kB for
GICv4). It is good to know that this implementation is not broken.
> Also, it seems that #redistributor-regions can also be saved, since
> bindings doc tells that it's only required if more than one such region
> is present?
This could be removed as well.
Thanks,
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply
* [arm64:for-next/core 52/52] arch/arm64/include/asm/probes.h:18:25: fatal error: asm/opcodes.h: No such file or directory
From: Marc Zyngier @ 2016-12-03 10:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <201612030617.rGmhD67h%fengguang.wu@intel.com>
On Fri, Dec 02 2016 at 10:05:27 PM, kbuild test robot <fengguang.wu@intel.com> wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core
> head: bca8f17f57bd76ddf2bbd2527eb890d6f588853e
> commit: bca8f17f57bd76ddf2bbd2527eb890d6f588853e [52/52] arm64: Get rid of asm/opcodes.h
> config: arm64-allmodconfig (attached as .config)
> compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
> wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> git checkout bca8f17f57bd76ddf2bbd2527eb890d6f588853e
> # save the attached .config to linux build tree
> make.cross ARCH=arm64
>
> All errors (new ones prefixed by >>):
>
> In file included from arch/arm64/include/asm/uprobes.h:14:0,
> from include/linux/uprobes.h:61,
> from include/linux/mm_types.h:13,
> from include/linux/sched.h:27,
> from arch/arm64/kernel/asm-offsets.c:21:
>>> arch/arm64/include/asm/probes.h:18:25: fatal error: asm/opcodes.h: No such file or directory
> #include <asm/opcodes.h>
> ^
> compilation terminated.
> make[2]: *** [arch/arm64/kernel/asm-offsets.s] Error 1
> make[2]: Target '__build' not remade because of errors.
> make[1]: *** [prepare0] Error 2
> make[1]: Target 'prepare' not remade because of errors.
> make: *** [sub-make] Error 2
Blah. Bloody uprobes. Just deleting this line should be enough, as I
can't see anything that actually depends on what is in the 32bit
asm/opcodes.h.
I can submit a patch if needed.
Thanks,
M.
--
Jazz is not dead. It just smells funny.
^ permalink raw reply
* [PATCH v9 07/16] drivers: acpi: implement acpi_dma_configure
From: Lorenzo Pieralisi @ 2016-12-03 10:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAJZ5v0jfwGBaHepToB9U3qQZ3KTB6XzoRDwEskV0B9fK7+OoDg@mail.gmail.com>
On Sat, Dec 03, 2016 at 03:11:09AM +0100, Rafael J. Wysocki wrote:
> On Fri, Dec 2, 2016 at 4:38 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > Rafael, Mark, Suravee,
> >
> > On Mon, Nov 21, 2016 at 10:01:39AM +0000, Lorenzo Pieralisi wrote:
> >> On DT based systems, the of_dma_configure() API implements DMA
> >> configuration for a given device. On ACPI systems an API equivalent to
> >> of_dma_configure() is missing which implies that it is currently not
> >> possible to set-up DMA operations for devices through the ACPI generic
> >> kernel layer.
> >>
> >> This patch fills the gap by introducing acpi_dma_configure/deconfigure()
> >> calls that for now are just wrappers around arch_setup_dma_ops() and
> >> arch_teardown_dma_ops() and also updates ACPI and PCI core code to use
> >> the newly introduced acpi_dma_configure/acpi_dma_deconfigure functions.
> >>
> >> Since acpi_dma_configure() is used to configure DMA operations, the
> >> function initializes the dma/coherent_dma masks to sane default values
> >> if the current masks are uninitialized (also to keep the default values
> >> consistent with DT systems) to make sure the device has a complete
> >> default DMA set-up.
> >
> > I spotted a niggle that unfortunately was hard to spot (and should not
> > be a problem per se but better safe than sorry) and I am not comfortable
> > with it.
> >
> > Following commit d0562674838c ("ACPI / scan: Parse _CCA and setup
> > device coherency") in acpi_bind_one() we check if the acpi_device
> > associated with a device just added supports DMA, first it was
> > done with acpi_check_dma() and then commit 1831eff876bd ("device
> > property: ACPI: Make use of the new DMA Attribute APIs") changed
> > it to acpi_get_dma_attr().
> >
> > The subsequent check (attr != DEV_DMA_NOT_SUPPORTED) is always true
> > on _any_ acpi device we pass to acpi_bind_one() on x86, which was
> > fine because we used it to call arch_setup_dma_ops(), which is a nop
> > on x86. On ARM64 a _CCA method is required to define if a device
> > supports DMA so (attr != DEV_DMA_NOT_SUPPORTED) may well be false.
> >
> > Now, acpi_bind_one() is used to bind an acpi_device to its physical
> > node also for pseudo-devices like cpus and memory nodes. For those
> > objects, on x86, attr will always be != DEV_DMA_NOT_SUPPORTED.
> >
> > So far so good, because on x86 arch_setup_dma_ops() is empty code.
> >
> > With this patch, I use the (attr != DEV_DMA_NOT_SUPPORTED) check
> > to call acpi_dma_configure() which is basically a nop on x86 except
> > that it sets up the dma_mask/coherent_dma_mask to a sane default value
> > (after all we are setting up DMA for the device so it makes sense to
> > initialize the masks there if they were unset since we are configuring
> > DMA for the device in question) for the given device.
> >
> > Problem is, as per the explanation above, we are also setting the
> > default dma masks for pseudo-devices (eg CPUs) that were previously
> > untouched, it should not be a problem per-se but I am not comfortable
> > with that, honestly it does not make much sense.
> >
> > An easy "fix" would be to move the default dma masks initialization out
> > of acpi_dma_configure() (as it was in previous patch versions of this
> > series - I moved it to acpi_dma_configure() just a consolidation point
> > for initializing the masks instead of scattering them in every
> > acpi_dma_configure caller) I can send this as a fix-up patch to Joerg if
> > we think that's the right thing to do (or I can send it to Rafael later
> > when the code is in the merged depending on the timing) just let me
> > know please.
>
> Why can't arch_setup_dma_ops() set those masks too?
Because the dma masks set-up is done by the caller (see
of_dma_configure()) according to firmware configuration or
platform data knowledge. I wanted to replicate the of_dma_configure()
interface on ACPI for obvious reasons (on ARM systems), I stopped
short of adding ACPI code to mirror of_dma_get_range() equivalent
(through the _DMA object) but I am really really nervous about changing
the code path on x86 because in theory all is fine, in practice even
just setting the masks to sane values can have unexpected consequences,
I just can't know (that's why I wasn't doing it in the first iterations
of this series).
Side note: DT with of_dma_configure() and ACPI with
acpi_create_platform_device() set the default dma mask for all
platform devices already _regardless_ of what they really are, though
arguably acpi_bind_one() touches ways more devices.
I really think that removing the default dma masks settings from
acpi_dma_configure() is the safer thing to do for the time being (or
moving acpi_dma_configure() to acpi_create_platform_device(), where the
DMA masks are set-up by default by core ACPI. Mark, Suravee, what was
the rationale behind calling arch_setup_dma_ops() in acpi_bind_one() ?)
Please let me know, fix-up is trivial however we decide to proceed.
Thank you !
Lorenzo
^ permalink raw reply
* [PATCH v2 1/2] arm64: dts: zx: Fix gic GICR property
From: Shawn Guo @ 2016-12-03 10:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <6379358.TRgYEOKGM8@wuerfel>
On Fri, Dec 02, 2016 at 05:38:01PM +0100, Arnd Bergmann wrote:
> On Monday, November 28, 2016 10:08:18 PM CET Shawn Guo wrote:
> > On Sat, Nov 26, 2016 at 6:03 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Monday, October 17, 2016 1:49:19 PM CET Olof Johansson wrote:
> > >> On Thu, Oct 13, 2016 at 08:31:20PM +0800, Jun Nie wrote:
> > >> > GICR for multiple CPU can be described with start address and stride,
> > >> > or with multiple address. Current multiple address and stride are
> > >> > both used. Fix it.
> > >> >
> > >> > vmalloc patch 727a7f5a9 triggered this bug:
> > >> > [ 0.097146] Unable to handle kernel paging request at virtual address ffff000008060008
> > >> > [ 0.097150] pgd = ffff000008602000
> > >> > [ 0.097160] [ffff000008060008] *pgd=000000007fffe003, *pud=000000007fffd003, *pmd=000000007fffc003, *pte=0000000000000000
> > >> > [ 0.097165] Internal error: Oops: 96000007 [#1] PREEMPT SMP
> > >> > [ 0.097170] Modules linked in:
> > >> > [ 0.097177] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.8.0+ #1474
> > >> > [ 0.097179] Hardware name: ZTE zx296718 evaluation board (DT)
> > >> > [ 0.097183] task: ffff80003e8c8b80 task.stack: ffff80003e8d0000
> > >> > [ 0.097197] PC is at gic_populate_rdist+0x74/0x15c
> > >> > [ 0.097202] LR is at gic_starting_cpu+0xc/0x20
> > >> > [ 0.097206] pc : [<ffff0000082b1b18>] lr : [<ffff0000082b26e0>] pstate: 600001c5
> > >> >
> > >> > Signed-off-by: Jun Nie <jun.nie@linaro.org>
> > >>
> > >> A Fixes: tag would be useful on a patch like this, to tell what patch
> > >> introduced the problem. Please consider using them in the future.
> > >>
> > >> I've applied this one to fixes now.
> > >
> > > Hi Olof,
> > >
> > > I happened to still have this one in my todo folder as I must have
> > > missed your reply, and I stumbled over it while looking for things
> > > that may have gone missing.
> > >
> > > I don't see it in v4.9-rc6, did it get dropped accidentally?
> >
> > Please help get this into v4.9 if possible, as it is required to get
> > v4.9 kernel boot up on ZTE ZX296718 SoC. Thanks.
> >
> > Shawn
> >
>
> Ok, applied both. Thanks,
Patch 2/2 already went to you in the pull request[1]:
[GIT PULL] ZTE arm64 device tree updates for 4.10
Shawn
[1] http://www.spinics.net/lists/arm-kernel/msg545640.html
^ permalink raw reply
* [PATCH] arm64: dts: zx: add pcu_domain node for zx296718
From: Baoyou Xie @ 2016-12-03 11:59 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds the pcu_domain node, so it can be used
by zte-soc's power domain driver.
Furthermore, it adds the document of the node.
Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
Documentation/devicetree/bindings/arm/zte.txt | 11 +++++++++++
arch/arm64/boot/dts/zte/zx296718.dtsi | 7 +++++++
2 files changed, 18 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/zte.txt b/Documentation/devicetree/bindings/arm/zte.txt
index 83369785..19a7e1b 100644
--- a/Documentation/devicetree/bindings/arm/zte.txt
+++ b/Documentation/devicetree/bindings/arm/zte.txt
@@ -27,6 +27,9 @@ System management required properties:
- compatible = "zte,zx296718-aon-sysctrl"
- compatible = "zte,zx296718-sysctrl"
+Low power management required properties:
+ - compatible = "zte,zx296718-pcu"
+
Example:
aon_sysctrl: aon-sysctrl at 116000 {
compatible = "zte,zx296718-aon-sysctrl", "syscon";
@@ -37,3 +40,11 @@ sysctrl: sysctrl at 1463000 {
compatible = "zte,zx296718-sysctrl", "syscon";
reg = <0x1463000 0x1000>;
};
+
+pcu_domain: pcu at 0x00117000 {
+ compatible = "zte,zx296718-pcu";
+ reg = <0x00117000 0x1000>;
+ #power-domain-cells = <1>;
+ status = "ok";
+};
+
diff --git a/arch/arm64/boot/dts/zte/zx296718.dtsi b/arch/arm64/boot/dts/zte/zx296718.dtsi
index b44d1d1..39e70c7 100644
--- a/arch/arm64/boot/dts/zte/zx296718.dtsi
+++ b/arch/arm64/boot/dts/zte/zx296718.dtsi
@@ -351,5 +351,12 @@
reg = <0x01480000 0x1000>;
#clock-cells = <1>;
};
+
+ pcu_domain: pcu at 0x00117000 {
+ compatible = "zte,zx296718-pcu";
+ reg = <0x00117000 0x1000>;
+ #power-domain-cells = <1>;
+ status = "ok";
+ };
};
};
--
2.7.4
^ permalink raw reply related
* [PATCH v3 11/14] ACPI: irq: introduce interrupt producer
From: G Gregory @ 2016-12-03 12:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1477408169-22217-12-git-send-email-guohanjun@huawei.com>
On Tue, Oct 25, 2016 at 11:09:26PM +0800, Hanjun Guo wrote:
> From: Hanjun Guo <hanjun.guo@linaro.org>
>
> In ACPI 6.1 spec, section 19.6.62, Interrupt Resource Descriptor Macro,
>
> Interrupt (ResourceUsage, EdgeLevel, ActiveLevel, Shared,
> ResourceSourceIndex, ResourceSource, DescriptorName)
> { InterruptList } => Buffer
>
> For the arguement ResourceUsage and DescriptorName, which means:
>
> ResourceUsage describes whether the device consumes the specified
> interrupt ( ResourceConsumer ) or produces it for use by a child
> device ( ResourceProducer ).
> If nothing is specified, then ResourceConsumer is assumed.
>
> DescriptorName evaluates to a name string which refers to the
> entire resource descriptor.
>
> So it can be used for devices connecting to a specific interrupt
> prodcucer instead of the main interrupt controller in MADT. In the
> real world, we have irqchip such as mbi-gen which connecting to
> a group of wired interrupts and then issue msi to the ITS, devices
> connecting to such interrupt controller fit this scope.
>
> For now the irq for ACPI only pointer to the main interrupt
> controller's irqdomain, for devices not connecting to those
> irqdomains, which need to present its irq parent, we can use
> following ASL code to represent it:
>
> Interrupt(ResourceConsumer,..., "\_SB.IRQP") {12,14,....}
>
> then we can parse the interrupt producer with the full
> path name "\_SB.IRQP".
>
> In order to do that, we introduce a pointer interrupt_producer
> in struct acpi_device, and fill it when scanning irq resources
> for acpi device if it specifies the interrupt producer.
>
> But for now when parsing the resources for acpi devices, we don't
> pass the acpi device for acpi_walk_resoures() in drivers/acpi/resource.c,
> so introduce a adev in struct res_proc_context to pass it as a context
> to scan the interrupt resources, then finally pass to acpi_register_gsi()
> to find its interrupt producer to get the virq from diffrent domains.
>
> With steps above ready, rework acpi_register_gsi() to get other
> interrupt producer if devices not connecting to main interrupt
> controller.
>
> Since we often pass NULL to acpi_register_gsi() and there is no interrupt
> producer for devices connect to gicd on ARM or io-apic on X86, so it will
> use the default irqdomain for those deivces and no functional changes to
> those devices.
>
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Agustin Vega-Frias <agustinv@codeaurora.org>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> ---
> drivers/acpi/gsi.c | 10 ++++--
> drivers/acpi/resource.c | 85 ++++++++++++++++++++++++++++++++++---------------
> include/acpi/acpi_bus.h | 1 +
> 3 files changed, 68 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/acpi/gsi.c b/drivers/acpi/gsi.c
> index ee9e0f2..29ee547 100644
> --- a/drivers/acpi/gsi.c
> +++ b/drivers/acpi/gsi.c
> @@ -55,13 +55,19 @@ int acpi_register_gsi(struct device *dev, u32 gsi, int trigger,
> int polarity)
> {
> struct irq_fwspec fwspec;
> + struct acpi_device *adev = dev ? to_acpi_device(dev) : NULL;
>
> - if (WARN_ON(!acpi_gsi_domain_id)) {
> + if (adev && &adev->fwnode && adev->interrupt_producer)
> + /* devices in DSDT connecting to spefic interrupt producer */
> + fwspec.fwnode = adev->interrupt_producer;
> + else if (acpi_gsi_domain_id)
> + /* devices connecting to gicd in default */
> + fwspec.fwnode = acpi_gsi_domain_id;
> + else {
> pr_warn("GSI: No registered irqchip, giving up\n");
> return -EINVAL;
> }
>
> - fwspec.fwnode = acpi_gsi_domain_id;
> fwspec.param[0] = gsi;
> fwspec.param[1] = acpi_dev_get_irq_type(trigger, polarity);
> fwspec.param_count = 2;
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 56241eb..f1371cf 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -381,7 +381,7 @@ static void acpi_dev_irqresource_disabled(struct resource *res, u32 gsi)
> res->flags = IORESOURCE_IRQ | IORESOURCE_DISABLED | IORESOURCE_UNSET;
> }
>
> -static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
> +static void acpi_dev_get_irqresource(struct acpi_device *adev, struct resource *res, u32 gsi,
> u8 triggering, u8 polarity, u8 shareable,
> bool legacy)
> {
> @@ -415,7 +415,7 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
> }
>
> res->flags = acpi_dev_irq_flags(triggering, polarity, shareable);
> - irq = acpi_register_gsi(NULL, gsi, triggering, polarity);
> + irq = acpi_register_gsi(&adev->dev, gsi, triggering, polarity);
> if (irq >= 0) {
> res->start = irq;
> res->end = irq;
> @@ -424,27 +424,9 @@ static void acpi_dev_get_irqresource(struct resource *res, u32 gsi,
> }
> }
>
> -/**
> - * acpi_dev_resource_interrupt - Extract ACPI interrupt resource information.
> - * @ares: Input ACPI resource object.
> - * @index: Index into the array of GSIs represented by the resource.
> - * @res: Output generic resource object.
> - *
> - * Check if the given ACPI resource object represents an interrupt resource
> - * and @index does not exceed the resource's interrupt count (true is returned
> - * in that case regardless of the results of the other checks)). If that's the
> - * case, register the GSI corresponding to @index from the array of interrupts
> - * represented by the resource and populate the generic resource object pointed
> - * to by @res accordingly. If the registration of the GSI is not successful,
> - * IORESOURCE_DISABLED will be set it that object's flags.
> - *
> - * Return:
> - * 1) false with res->flags setting to zero: not the expected resource type
> - * 2) false with IORESOURCE_DISABLED in res->flags: valid unassigned resource
> - * 3) true: valid assigned resource
> - */
> -bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
> - struct resource *res)
> +static bool __acpi_dev_resource_interrupt(struct acpi_device *adev,
> + struct acpi_resource *ares, int index,
> + struct resource *res)
> {
> struct acpi_resource_irq *irq;
> struct acpi_resource_extended_irq *ext_irq;
> @@ -460,7 +442,7 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
> acpi_dev_irqresource_disabled(res, 0);
> return false;
> }
> - acpi_dev_get_irqresource(res, irq->interrupts[index],
> + acpi_dev_get_irqresource(adev, res, irq->interrupts[index],
> irq->triggering, irq->polarity,
> irq->sharable, true);
> break;
> @@ -470,7 +452,31 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
> acpi_dev_irqresource_disabled(res, 0);
> return false;
> }
> - acpi_dev_get_irqresource(res, ext_irq->interrupts[index],
> +
> + /*
> + * It's a interrupt consumer device and connecting to specfic
> + * interrupt controller. For now, we only support connecting
> + * interrupts to one irq controller for a single device
> + */
> + if (ext_irq->producer_consumer == ACPI_CONSUMER
> + && ext_irq->resource_source.string_length != 0
> + && !adev->interrupt_producer) {
> + acpi_status status;
> + acpi_handle handle;
> + struct acpi_device *device;
> +
> + status = acpi_get_handle(NULL, ext_irq->resource_source.string_ptr, &handle);
> + if (ACPI_FAILURE(status))
> + return false;
> +
> + device = acpi_bus_get_acpi_device(handle);
> + if (!device)
> + return false;
> +
> + adev->interrupt_producer = &device->fwnode;
> + }
> +
> + acpi_dev_get_irqresource(adev, res, ext_irq->interrupts[index],
> ext_irq->triggering, ext_irq->polarity,
> ext_irq->sharable, false);
> break;
> @@ -481,6 +487,31 @@ bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
>
> return true;
> }
> +
> +/**
> + * acpi_dev_resource_interrupt - Extract ACPI interrupt resource information.
> + * @ares: Input ACPI resource object.
> + * @index: Index into the array of GSIs represented by the resource.
> + * @res: Output generic resource object.
> + *
> + * Check if the given ACPI resource object represents an interrupt resource
> + * and @index does not exceed the resource's interrupt count (true is returned
> + * in that case regardless of the results of the other checks)). If that's the
> + * case, register the GSI corresponding to @index from the array of interrupts
> + * represented by the resource and populate the generic resource object pointed
> + * to by @res accordingly. If the registration of the GSI is not successful,
> + * IORESOURCE_DISABLED will be set it that object's flags.
> + *
> + * Return:
> + * 1) false with res->flags setting to zero: not the expected resource type
> + * 2) false with IORESOURCE_DISABLED in res->flags: valid unassigned resource
> + * 3) true: valid assigned resource
> + */
> +bool acpi_dev_resource_interrupt(struct acpi_resource *ares, int index,
> + struct resource *res)
> +{
> + return __acpi_dev_resource_interrupt(NULL, ares, index, res);
> +}
> EXPORT_SYMBOL_GPL(acpi_dev_resource_interrupt);
You cannot do this as &adev->dev is deferenced in acpi_dev_get_irqresource
without a check that adev is NULL, and so passes an offset from NULL to
acpi_register_gsi() which breaks everything.
Graeme
>
> /**
> @@ -499,6 +530,7 @@ struct res_proc_context {
> void *preproc_data;
> int count;
> int error;
> + struct acpi_device *adev;
> };
>
> static acpi_status acpi_dev_new_resource_entry(struct resource_win *win,
> @@ -546,7 +578,7 @@ static acpi_status acpi_dev_process_resource(struct acpi_resource *ares,
> || acpi_dev_resource_ext_address_space(ares, &win))
> return acpi_dev_new_resource_entry(&win, c);
>
> - for (i = 0; acpi_dev_resource_interrupt(ares, i, res); i++) {
> + for (i = 0; __acpi_dev_resource_interrupt(c->adev, ares, i, res); i++) {
> acpi_status status;
>
> status = acpi_dev_new_resource_entry(&win, c);
> @@ -599,6 +631,7 @@ int acpi_dev_get_resources(struct acpi_device *adev, struct list_head *list,
> c.preproc_data = preproc_data;
> c.count = 0;
> c.error = 0;
> + c.adev = adev;
> status = acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
> acpi_dev_process_resource, &c);
> if (ACPI_FAILURE(status)) {
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 4242c31..5410d3b 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -355,6 +355,7 @@ struct acpi_device {
> int device_type;
> acpi_handle handle; /* no handle for fixed hardware */
> struct fwnode_handle fwnode;
> + struct fwnode_handle *interrupt_producer;
> struct acpi_device *parent;
> struct list_head children;
> struct list_head node;
> --
> 1.7.12.4
>
^ permalink raw reply
* [PATCH] arm64: dts: zte: clean up gic-v3 redistributor properties
From: Shawn Guo @ 2016-12-03 12:36 UTC (permalink / raw)
To: linux-arm-kernel
The gic-v3 property redistributor-stride is only meant as a workaround
for broken platforms that have a redistributor stride deviating what the
architecture defines, i.e. 128KiB for GICv3, 256KiB for GICv4. This is
not the case for ZX296718, and redistributor-stride is not really
necessary. Let's drop it.
Also, #redistributor-regions is only required when there is more than
one such region is present. Let's remove it as well.
Signed-off-by: Shawn Guo <shawnguo@kernel.org>
---
arch/arm64/boot/dts/zte/zx296718.dtsi | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/arm64/boot/dts/zte/zx296718.dtsi b/arch/arm64/boot/dts/zte/zx296718.dtsi
index 6b239a365f01..dd2cd73c774c 100644
--- a/arch/arm64/boot/dts/zte/zx296718.dtsi
+++ b/arch/arm64/boot/dts/zte/zx296718.dtsi
@@ -239,8 +239,6 @@
compatible = "arm,gic-v3";
#interrupt-cells = <3>;
#address-cells = <0>;
- #redistributor-regions = <1>;
- redistributor-stride = <0x20000>;
interrupt-controller;
reg = <0x02a00000 0x10000>,
<0x02b00000 0xc0000>;
--
1.9.1
^ permalink raw reply related
* [PATCH] arm64: dts: zte: clean up gic-v3 redistributor properties
From: Marc Zyngier @ 2016-12-03 13:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1480768600-20827-1-git-send-email-shawnguo@kernel.org>
On Sat, 3 Dec 2016 20:36:40 +0800
Shawn Guo <shawnguo@kernel.org> wrote:
> The gic-v3 property redistributor-stride is only meant as a workaround
> for broken platforms that have a redistributor stride deviating what the
> architecture defines, i.e. 128KiB for GICv3, 256KiB for GICv4. This is
> not the case for ZX296718, and redistributor-stride is not really
> necessary. Let's drop it.
>
> Also, #redistributor-regions is only required when there is more than
> one such region is present. Let's remove it as well.
>
> Signed-off-by: Shawn Guo <shawnguo@kernel.org>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply
* ARM: topology: Checking for a failed kcalloc() in parse_dt_topology()?
From: SF Markus Elfring @ 2016-12-03 14:00 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
Another run of a small script for the semantic patch language (Coccinelle)
pointed the implementation of the function ?parse_dt_topology? out for
further considerations.
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/arch/arm/kernel/topology.c?id=e05f574a0bb1f4502a4b2264fdb0ef6419cf3772#n285
Would it be useful to check the return value from a call of the
function ?kcalloc? also there?
Regards,
Markus
^ permalink raw reply
* [PATCH 0/2] Fix fallouts from asm/opcodes.h removal
From: Marc Zyngier @ 2016-12-03 14:05 UTC (permalink / raw)
To: linux-arm-kernel
Since the asm/opcodes.h file was removed, two bugs have cropped up:
- probes.h contains a reference to asm/opcodes.h that was missed
- An ugly (and alas familiar) bug has reappeared, leading to obscure
compilation errors when a .inst is part of an alternative and that
the "auto nop" feature is used to pad the alternative sequence (and
the whole thing is assembled with an old version of gas). This is
triggered again now that we generate SET_PSTATE_PAN/UAO using .inst.
The first bug is easy to fix, while the second requires some really
ugly (if limited) surgery using the .fill directive to replace the
"nops" macro.
Marc Zyngier (2):
arm64: Remove reference to asm/opcodes.h
arm64: alternatives: Work around NOP generation with broken assembler
arch/arm64/include/asm/alternative.h | 12 +++++++++++-
arch/arm64/include/asm/probes.h | 2 --
2 files changed, 11 insertions(+), 3 deletions(-)
--
2.10.2
^ 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