* ARM, SoC: About the use DT-defined properties by 3rd-party drivers
From: Sebastian Frias @ 2016-09-13 14:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160913131208.GA23336@leverpostej>
Hi Mark,
On 09/13/2016 03:12 PM, Mark Rutland wrote:
>> Exactly, that is why I was thinking it would take less "review" time.
>> Indeed, if there is no driver, why would it matter what those bindings
>> are?
>
> If you believe that the bindings don't matter, then there is absolutely
> no reason for them to exist in the first place.
>
> If those binding matter to *anyone*, then those collating the bindings
> have some responsibility of stewardship, and that includes
> review/maintenance/etc.
The thing is that right now it seems the "responsibility of stewardship"
lies only within "Linux", whereas DT is proposed as open for everybody,
Bootloaders, FreeBSD, etc.
In that case, shouldn't the "responsibility" be shared?
Alternatively, maybe 'borders' could be created, in order to enable the
allocation of responsibility of different sections to different parties,
right?
Obviously, moving properties/nodes from one 'section' to another crossing
responsibility 'borders' would require agreements.
Shouldn't that be something good to think about?
Best regards,
Sebastian
^ permalink raw reply
* [RFCv3][PATCH 3/5] arm64: Implement ARCH_HAS_FORCE_CACHE
From: Laura Abbott @ 2016-09-13 15:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160913091946.GB7467@arm.com>
On 09/13/2016 02:19 AM, Will Deacon wrote:
> Hi Laura,
>
> On Mon, Sep 12, 2016 at 02:32:56PM -0700, Laura Abbott wrote:
>>
>> arm64 may need to guarantee the caches are synced. Implement versions of
>> the kernel_force_cache API to allow this.
>>
>> Signed-off-by: Laura Abbott <labbott@redhat.com>
>> ---
>> v3: Switch to calling cache operations directly instead of relying on
>> DMA mapping.
>> ---
>> arch/arm64/include/asm/cacheflush.h | 8 ++++++++
>> arch/arm64/mm/cache.S | 24 ++++++++++++++++++++----
>> arch/arm64/mm/flush.c | 11 +++++++++++
>> 3 files changed, 39 insertions(+), 4 deletions(-)
>
> I'm really hesitant to expose these cache routines as an API solely to
> support a driver sitting in staging/. I appreciate that there's a chicken
> and egg problem here, but we *really* don't want people using these routines
> in preference to the DMA API, and I fear that we'll simply grow a bunch
> more users of these things if we promote it as an API like you're proposing.
>
> Can the code not be contained under staging/, as part of ion?
>
I proposed that in V1 and it was suggested I make it a proper API
http://www.mail-archive.com/driverdev-devel at linuxdriverproject.org/msg47654.html
http://www.mail-archive.com/driverdev-devel at linuxdriverproject.org/msg47672.html
> Will
>
Thanks,
Laura
^ permalink raw reply
* [PATCH 1/5] clk: add support for runtime pm
From: Ulf Hansson @ 2016-09-13 15:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <76bb5b71-dd4e-e1ad-df0b-b84833812502@samsung.com>
[...]
>>
>> This needs to be clarified. I agree we need to cover system PM as
>> well, but let's try be a bit more precise about it.
>
>
> Right, I wasn't precise here. I've developed this code on older (v4.1 and
> v4.6)
> kernels, which had a code which disables runtime pm during system sleep
> transition
> time. Maybe I need to revisit it and consider your change merged to
> v4.8-rc1, which
> keeps runtime pm enabled during system sleep transitions.
Right, I see.
>>> static bool clk_core_is_prepared(struct clk_core *core)
>>> {
>>> + if (clk_pm_runtime_suspended(core))
>>> + return false;
>>> +
>>
>> This isn't safe, as even if the clock controller is runtime resumed at
>> this point, that's *not* a guarantee that is stays runtime resumed
>> while invoking the ->ops->is_prepared().
>>
>> Instead you must call a pm_runtime_get_noresume() before you check the
>> runtime PM status, as that should avoid the device from being runtime
>> suspended. Then when the ->ops->is_prepared() has been invoked, we
>> should call pm_runtime_put().
>>
>> Although, I am not sure the above change becomes entirely correct as I
>> think we are mixing the runtime PM status with the clock prepare
>> status here. In other words, the next time the clock controller
>> becomes runtime resumed, it may very well restore some register
>> context which may prepare the clock, unless someone explicitly has
>> unprepared it.
>>
>> Of course, it all depends on how clk_core_is_prepared() is used by the
>> clock framework.
>
>
> clk_core_is_prepared() is mainly used by disable_unused_tree_*. You are
> right that it mixes a bit clock prepared state with runtime pm active
> state of clock controller's, but I assumed here that clock cannot be
> prepared if runtime pm state of controller is suspended. Other approach
> here would be to call pm_runtime_get(), check status and then
> pm_runtime_put(). If you prefer such approach, I will change it.
Using pm_runtime_get|put() would work for the clk_core_is_prepared()
case, although perhaps not for the clk_core_is_enabled() case.
The reason is that I guess the clk_core_is_enabled() API may be called
from atomic context? Thus we would need to enable
pm_runtime_irq_safe() for the clock provider device, which I *really*
would like to avoid.
[...]
>>
>> I believe we are also accessing the clock controller HW from the
>> late_initcall_sync(clk_disable_unused) function.
>
>
> This was indirectly handled by the runtime pm state check in is_prepared
> and is_enabled().
I see.
Although, I was thinking that you explicitly would like to
disable/unprepare unused clocks in this phase, so then it isn't
sufficient to rely on the runtime PM status to know whether the clock
is prepared/enabled.
Perhaps, this is the only case when you actually need a
pm_runtime_get|put() around the ->is_enabled|prepared()!?
>
>> More precisely, in clk_disable_unused_subtree(), we probably need a
>> pm_runtime_get_sync() before calling clk_core_is_enabled(). And then
>> restore that with a pm_runtime_put() after the clock has been
>> disabled.
>> The similar is needed in clk_unprepare_unused_subtree().
>
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
Kind regards
Uffe
^ permalink raw reply
* [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi function device for legacy interrupts.
From: Bjorn Helgaas @ 2016-09-13 15:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D7ADA8.5060201@arm.com>
On Tue, Sep 13, 2016 at 08:41:28AM +0100, Marc Zyngier wrote:
> On 12/09/16 23:02, Bjorn Helgaas wrote:
> > On Thu, Sep 01, 2016 at 05:19:55AM +0000, Bharat Kumar Gogada wrote:
> >>>>>>> Hi Bharat,
> >>>>>>>> @@ -561,7 +561,7 @@ static int nwl_pcie_init_irq_domain(struct
> >>>>>>>> nwl_pcie
> >>>>>>> *pcie)
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> pcie->legacy_irq_domain = irq_domain_add_linear(legacy_intc_node,
> >>>>>>>> - INTX_NUM,
> >>>>>>>> + INTX_NUM + 1,
> >>>>>>>> &legacy_domain_ops,
> >>>>>>>> pcie);
> >>>>>>>
> >>>>>>> This feels like the wrong thing to do. You have INTX_NUM irqs, so
> >>>>>>> the domain allocation should reflect this. On the other hand, the
> >>>>>>> way the driver currently deals with mappings is quite broken
> >>>>>>> (consistently adding 1 to
> >>>>> the HW interrupt).
> >>>>>>>
> >>>>>> Hi Marc,
> >>>>>>
> >>>>>> Without above change I get following crash in kernel while booting.
> >>>>>>
> >>>>>> [ 2.441684] error: hwirq 0x4 is too large for dummy
> >>>>>>
> >>>>>> [ 2.441694] ------------[ cut here ]------------
> >>>>>>
> >>>>>> [ 2.441698] WARNING: at kernel/irq/irqdomain.c:344
> >>>>>>
> >>>>>> [ 2.441702] Modules linked in:
> >>>>>>
> >>>>>> [ 2.441706]
> >>>>>>
> >>>>>> [ 2.441714] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.4.0 #8
> >>>>>>
> >>>>>> [ 2.441718] Hardware name: xlnx,zynqmp (DT)
> >>>>>>
> >>>>>> [ 2.441723] task: ffffffc071886b80 ti: ffffffc071888000 task.ti:
> >>>>> ffffffc071888000
> >>>>>>
> >>>>>> [ 2.441732] PC is at irq_domain_associate+0x138/0x1c0
> >>>>>>
> >>>>>> [ 2.441738] LR is at irq_domain_associate+0x138/0x1c0
> >>>>>>
> >>>>>> In kernel/irq/irqdomain.c function irq_domain_associate
> >>>>>>
> >>>>>> if (WARN(hwirq >= domain->hwirq_max,
> >>>>>> "error: hwirq 0x%x is too large for %s\n", (int)hwirq, domain-
> >>>> name))
> >>>>>> return -EINVAL;
> >>>>>>
> >>>>>> Here the hwirq and hwirq_max are equal to 4 without the above
> >>>>>> condition
> >>>>> (INTX_NUM + 1) due to which crash is coming.
> >>>>>> This is happening as the legacy interrupts are starting from 1 (INTA).
> >>>>>
> >>>>> I understood that. I'm still persisting in saying that you have the wrong fix.
> >>>>>
> >>>>> Your domain should always allocate many interrupts as you have
> >>>>> interrupt sources. These interrupts (hwirq) should be numbered from 0 to (n-
> >>> 1).
> >>>>
> >>>> Agreed, but here comes the problem the hwirq for legacy interrupts
> >>>> will start at 0x1 to 0x4 (INTA to INTD) and these values are as per
> >>>> PCIe specification for legacy interrupts. So these cannot be numbered
> >>>> from 0. So when 0x4 (INTD) for a multi-function device comes the crash
> >>>> occurs.
> >>>
> >>> So who provides this hwirq? Who calls irq_domain_associate() with hwirq set to
> >>> 4?
> >>>
> >> PCIe subsystem invokes pcibios_add_device function in arch/arm64/kernel/pci.c for every pci device.
> >> The purpose of this function is to assign dev->irq using of_irq_parse_and_map_pci.
> >> of_irq_parse_and_map_pci invokes of_irq_parse_pci where it reads PCI_INTERRUPT_PIN from configuration space and saves it
> >> in parameter of struct of_phandle_args.
> >> This structure is passed to irq_create_of_mapping where it invokes irq_create_fwspec_mapping.
> >> irq_create_fwspec_mapping invokes irq_domain_translate and gets hwirq, here the above saved PCI_INTERRUPT_PIN value is assigned
> >> to hwirq (*hwirq = fwspec->param[0]).
> >> And then using this hwirq irq_create_mapping -> irq_domain_associate were invoked and mapping is created for virtual irq with this hwirq.
> >> So for any end point PCI_INTERRUPT_PIN value starts from 0x1 to 0x4 and so hwirq starts from 0x1 to 0x4.
> >>
> >> So the values are more generic w.r.t to protocol, that's why hwirq will range from 0x1 to 0x4.
> >> And then if you check pcie-altera.c they are doing this adding one in their handler and while creating legacy domain.
> >
> > Is this resolved yet? Marc, are you happy, or should we iterate on this
> > again?
>
> Ah, sorry to have dropped the ball on this patch.
No problem, I wasn't making forward progress anyway.
> I guess that given that the infrastructure imposes the hwirq range on
> the host drivers, Bharat's approach is the only way (and a number of
> other host drivers are already slightly broken). I'll try and have a
> look at solving this at the generic level. In the meantime:
>
> Acked-by: Marc Zyngier <marc.zyngier@arm.com>
After looking at this myself, I'm not happy with this either. It feels
like there are bugs lurking here and we're just hiding one of them.
Here are the callers of irq_domain_add_linear() for legacy INTx in
drivers/pci/host:
advk_pcie_init_irq_domain LEGACY_IRQ_NUM (4)
dra7xx_pcie_init_irq_domain 4
ks_dw_pcie_host_init MAX_LEGACY_IRQS (4)
altera_pcie_init_irq_domain INTX_NUM + 1 (5)
nwl_pcie_init_irq_domain INTX_NUM + 1 (5)
xilinx_pcie_init_irq_domain 4
I think all of these use the of_irq_parse_and_map_pci() path you
mentioned, so if the problem is in the way that path works, I would
think these should *all* be requesting the same number of interrupts
in the domain.
I agree with Marc that we should request 4 IRQs, because that's what
we need. If we can't do that for some reason, we ought to at least
make all these callers the same.
Bjorn
^ permalink raw reply
* [RESEND PATCH v2 1/4] pinctrl: amlogic: gxbb: add spi nor pins
From: Jerome Brunet @ 2016-09-13 15:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473409738-27175-1-git-send-email-jbrunet@baylibre.com>
Add EE domains pins for the SPI flash controller
Acked-by: Kevin Hilman <khilman@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
index f115638e8699..573901887cee 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
@@ -156,6 +156,11 @@ static const unsigned int emmc_clk_pins[] = { PIN(BOOT_8, EE_OFF) };
static const unsigned int emmc_cmd_pins[] = { PIN(BOOT_10, EE_OFF) };
static const unsigned int emmc_ds_pins[] = { PIN(BOOT_15, EE_OFF) };
+static const unsigned int nor_d_pins[] = { PIN(BOOT_11, EE_OFF) };
+static const unsigned int nor_q_pins[] = { PIN(BOOT_12, EE_OFF) };
+static const unsigned int nor_c_pins[] = { PIN(BOOT_13, EE_OFF) };
+static const unsigned int nor_cs_pins[] = { PIN(BOOT_15, EE_OFF) };
+
static const unsigned int sdcard_d0_pins[] = { PIN(CARD_1, EE_OFF) };
static const unsigned int sdcard_d1_pins[] = { PIN(CARD_0, EE_OFF) };
static const unsigned int sdcard_d2_pins[] = { PIN(CARD_5, EE_OFF) };
@@ -430,6 +435,10 @@ static struct meson_pmx_group meson_gxbb_periphs_groups[] = {
GROUP(emmc_clk, 4, 18),
GROUP(emmc_cmd, 4, 19),
GROUP(emmc_ds, 4, 31),
+ GROUP(nor_d, 5, 1),
+ GROUP(nor_q, 5, 3),
+ GROUP(nor_c, 5, 2),
+ GROUP(nor_cs, 5, 0),
/* Bank CARD */
GROUP(sdcard_d1, 2, 14),
@@ -517,6 +526,10 @@ static const char * const emmc_groups[] = {
"emmc_nand_d07", "emmc_clk", "emmc_cmd", "emmc_ds",
};
+static const char * const nor_groups[] = {
+ "nor_d", "nor_q", "nor_c", "nor_cs",
+};
+
static const char * const sdcard_groups[] = {
"sdcard_d0", "sdcard_d1", "sdcard_d2", "sdcard_d3",
"sdcard_cmd", "sdcard_clk",
@@ -619,6 +632,7 @@ static const char * const pwm_ao_b_groups[] = {
static struct meson_pmx_func meson_gxbb_periphs_functions[] = {
FUNCTION(gpio_periphs),
FUNCTION(emmc),
+ FUNCTION(nor),
FUNCTION(sdcard),
FUNCTION(sdio),
FUNCTION(uart_a),
--
2.7.4
^ permalink raw reply related
* [RESEND PATCH v2 2/4] pinctrl: amlogic: gxbb: add nand pins
From: Jerome Brunet @ 2016-09-13 15:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473779542-2635-1-git-send-email-jbrunet@baylibre.com>
Add EE domains pins for the NAND flash controller.
Even tough we have no driver for the NAND flash controller yet, we need
to have these pins in pinctrl as the actual pin are shared with the spifc
controller. The bootloader on the S905-P200 setup pinmux for the NAND
controller so we need the kernel to properly deactivate this if necessary.
Acked-by: Kevin Hilman <khilman@baylibre.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
drivers/pinctrl/meson/pinctrl-meson-gxbb.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
index 573901887cee..b06cc12f2500 100644
--- a/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
+++ b/drivers/pinctrl/meson/pinctrl-meson-gxbb.c
@@ -176,6 +176,15 @@ static const unsigned int sdio_cmd_pins[] = { PIN(GPIOX_4, EE_OFF) };
static const unsigned int sdio_clk_pins[] = { PIN(GPIOX_5, EE_OFF) };
static const unsigned int sdio_irq_pins[] = { PIN(GPIOX_7, EE_OFF) };
+static const unsigned int nand_ce0_pins[] = { PIN(BOOT_8, EE_OFF) };
+static const unsigned int nand_ce1_pins[] = { PIN(BOOT_9, EE_OFF) };
+static const unsigned int nand_rb0_pins[] = { PIN(BOOT_10, EE_OFF) };
+static const unsigned int nand_ale_pins[] = { PIN(BOOT_11, EE_OFF) };
+static const unsigned int nand_cle_pins[] = { PIN(BOOT_12, EE_OFF) };
+static const unsigned int nand_wen_clk_pins[] = { PIN(BOOT_13, EE_OFF) };
+static const unsigned int nand_ren_wr_pins[] = { PIN(BOOT_14, EE_OFF) };
+static const unsigned int nand_dqs_pins[] = { PIN(BOOT_15, EE_OFF) };
+
static const unsigned int uart_tx_a_pins[] = { PIN(GPIOX_12, EE_OFF) };
static const unsigned int uart_rx_a_pins[] = { PIN(GPIOX_13, EE_OFF) };
static const unsigned int uart_cts_a_pins[] = { PIN(GPIOX_14, EE_OFF) };
@@ -439,6 +448,14 @@ static struct meson_pmx_group meson_gxbb_periphs_groups[] = {
GROUP(nor_q, 5, 3),
GROUP(nor_c, 5, 2),
GROUP(nor_cs, 5, 0),
+ GROUP(nand_ce0, 4, 26),
+ GROUP(nand_ce1, 4, 27),
+ GROUP(nand_rb0, 4, 25),
+ GROUP(nand_ale, 4, 24),
+ GROUP(nand_cle, 4, 23),
+ GROUP(nand_wen_clk, 4, 22),
+ GROUP(nand_ren_wr, 4, 21),
+ GROUP(nand_dqs, 4, 20),
/* Bank CARD */
GROUP(sdcard_d1, 2, 14),
@@ -540,6 +557,11 @@ static const char * const sdio_groups[] = {
"sdio_cmd", "sdio_clk", "sdio_irq",
};
+static const char * const nand_groups[] = {
+ "nand_ce0", "nand_ce1", "nand_rb0", "nand_ale", "nand_cle",
+ "nand_wen_clk", "nand_ren_wr", "nand_dqs",
+};
+
static const char * const uart_a_groups[] = {
"uart_tx_a", "uart_rx_a", "uart_cts_a", "uart_rts_a",
};
@@ -635,6 +657,7 @@ static struct meson_pmx_func meson_gxbb_periphs_functions[] = {
FUNCTION(nor),
FUNCTION(sdcard),
FUNCTION(sdio),
+ FUNCTION(nand),
FUNCTION(uart_a),
FUNCTION(uart_b),
FUNCTION(uart_c),
--
2.7.4
^ permalink raw reply related
* [RFCv3][PATCH 3/5] arm64: Implement ARCH_HAS_FORCE_CACHE
From: Will Deacon @ 2016-09-13 15:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <27805ccf-1f2d-81ea-044e-b21207be8e7f@redhat.com>
On Tue, Sep 13, 2016 at 08:02:20AM -0700, Laura Abbott wrote:
> On 09/13/2016 02:19 AM, Will Deacon wrote:
> >On Mon, Sep 12, 2016 at 02:32:56PM -0700, Laura Abbott wrote:
> >>
> >>arm64 may need to guarantee the caches are synced. Implement versions of
> >>the kernel_force_cache API to allow this.
> >>
> >>Signed-off-by: Laura Abbott <labbott@redhat.com>
> >>---
> >>v3: Switch to calling cache operations directly instead of relying on
> >>DMA mapping.
> >>---
> >> arch/arm64/include/asm/cacheflush.h | 8 ++++++++
> >> arch/arm64/mm/cache.S | 24 ++++++++++++++++++++----
> >> arch/arm64/mm/flush.c | 11 +++++++++++
> >> 3 files changed, 39 insertions(+), 4 deletions(-)
> >
> >I'm really hesitant to expose these cache routines as an API solely to
> >support a driver sitting in staging/. I appreciate that there's a chicken
> >and egg problem here, but we *really* don't want people using these routines
> >in preference to the DMA API, and I fear that we'll simply grow a bunch
> >more users of these things if we promote it as an API like you're proposing.
> >
> >Can the code not be contained under staging/, as part of ion?
> >
>
> I proposed that in V1 and it was suggested I make it a proper API
>
> http://www.mail-archive.com/driverdev-devel at linuxdriverproject.org/msg47654.html
> http://www.mail-archive.com/driverdev-devel at linuxdriverproject.org/msg47672.html
:/ then I guess we're in disagreement. If ion really needs this stuff
(which I don't fully grok), perhaps we should be exposing something at
a higher level from the architecture, so it really can't be used for
anything other than ion.
Will
^ permalink raw reply
* [PATCH 1/3] PCI: Xilinx NWL PCIe: Expanding PCIe core errors and printing event occurred.
From: Bjorn Helgaas @ 2016-09-13 15:18 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1472553558-27215-1-git-send-email-bharatku@xilinx.com>
On Tue, Aug 30, 2016 at 04:09:16PM +0530, Bharat Kumar Gogada wrote:
> The current driver prints pcie core error, for all core events.
> Instead of just printing PCIe core error, now adding prints to show
> individual core events occurred.
>
> Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com>
I applied the first two patches to pci/host-xilinx for v4.9, thanks!
I'd like to work on the third one a little more, as I mentioned in my
response to it.
> ---
> drivers/pci/host/pcie-xilinx-nwl.c | 48 +++++++++++++++++++++++++++++++-------
> 1 file changed, 40 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pci/host/pcie-xilinx-nwl.c b/drivers/pci/host/pcie-xilinx-nwl.c
> index 3479d30..86c1834 100644
> --- a/drivers/pci/host/pcie-xilinx-nwl.c
> +++ b/drivers/pci/host/pcie-xilinx-nwl.c
> @@ -85,10 +85,15 @@
> #define MSGF_MISC_SR_MASTER_ERR BIT(5)
> #define MSGF_MISC_SR_I_ADDR_ERR BIT(6)
> #define MSGF_MISC_SR_E_ADDR_ERR BIT(7)
> -#define MSGF_MISC_SR_UR_DETECT BIT(20)
> -
> -#define MSGF_MISC_SR_PCIE_CORE GENMASK(18, 16)
> -#define MSGF_MISC_SR_PCIE_CORE_ERR GENMASK(31, 22)
> +#define MSGF_MISC_SR_FATAL_AER BIT(16)
> +#define MSGF_MISC_SR_NON_FATAL_AER BIT(17)
> +#define MSGF_MISC_SR_CORR_AER BIT(18)
> +#define MSGF_MISC_SR_UR_DETECT BIT(20)
> +#define MSGF_MISC_SR_NON_FATAL_DEV BIT(22)
> +#define MSGF_MISC_SR_FATAL_DEV BIT(23)
> +#define MSGF_MISC_SR_LINK_DOWN BIT(24)
> +#define MSGF_MSIC_SR_LINK_AUTO_BWIDTH BIT(25)
> +#define MSGF_MSIC_SR_LINK_BWIDTH BIT(26)
>
> #define MSGF_MISC_SR_MASKALL (MSGF_MISC_SR_RXMSG_AVAIL | \
> MSGF_MISC_SR_RXMSG_OVER | \
> @@ -96,9 +101,15 @@
> MSGF_MISC_SR_MASTER_ERR | \
> MSGF_MISC_SR_I_ADDR_ERR | \
> MSGF_MISC_SR_E_ADDR_ERR | \
> + MSGF_MISC_SR_FATAL_AER | \
> + MSGF_MISC_SR_NON_FATAL_AER | \
> + MSGF_MISC_SR_CORR_AER | \
> MSGF_MISC_SR_UR_DETECT | \
> - MSGF_MISC_SR_PCIE_CORE | \
> - MSGF_MISC_SR_PCIE_CORE_ERR)
> + MSGF_MISC_SR_NON_FATAL_DEV | \
> + MSGF_MISC_SR_FATAL_DEV | \
> + MSGF_MISC_SR_LINK_DOWN | \
> + MSGF_MSIC_SR_LINK_AUTO_BWIDTH | \
> + MSGF_MSIC_SR_LINK_BWIDTH)
>
> /* Legacy interrupt status mask bits */
> #define MSGF_LEG_SR_INTA BIT(0)
> @@ -291,8 +302,29 @@ static irqreturn_t nwl_pcie_misc_handler(int irq, void *data)
> dev_err(pcie->dev,
> "In Misc Egress address translation error\n");
>
> - if (misc_stat & MSGF_MISC_SR_PCIE_CORE_ERR)
> - dev_err(pcie->dev, "PCIe Core error\n");
> + if (misc_stat & MSGF_MISC_SR_FATAL_AER)
> + dev_err(pcie->dev, "Fatal Error in AER Capability\n");
> +
> + if (misc_stat & MSGF_MISC_SR_NON_FATAL_AER)
> + dev_err(pcie->dev, "Non-Fatal Error in AER Capability\n");
> +
> + if (misc_stat & MSGF_MISC_SR_CORR_AER)
> + dev_err(pcie->dev, "Correctable Error in AER Capability\n");
> +
> + if (misc_stat & MSGF_MISC_SR_UR_DETECT)
> + dev_err(pcie->dev, "Unsupported request Detected\n");
> +
> + if (misc_stat & MSGF_MISC_SR_NON_FATAL_DEV)
> + dev_err(pcie->dev, "Non-Fatal Error Detected\n");
> +
> + if (misc_stat & MSGF_MISC_SR_FATAL_DEV)
> + dev_err(pcie->dev, "Fatal Error Detected\n");
> +
> + if (misc_stat & MSGF_MSIC_SR_LINK_AUTO_BWIDTH)
> + dev_info(pcie->dev, "Link Autonomous Bandwidth Management Status bit set\n");
> +
> + if (misc_stat & MSGF_MSIC_SR_LINK_BWIDTH)
> + dev_info(pcie->dev, "Link Bandwidth Management Status bit set\n");
>
> /* Clear misc interrupt status */
> nwl_bridge_writel(pcie, misc_stat, MSGF_MISC_STATUS);
> --
> 2.1.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" 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 v5 2/9] drivers: irqchip: Add STM32 external interrupts support
From: Thomas Gleixner @ 2016-09-13 15:21 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473432124-6784-3-git-send-email-alexandre.torgue@st.com>
On Fri, 9 Sep 2016, Alexandre TORGUE wrote:
> +static void stm32_exti_free(struct irq_domain *d, unsigned int virq,
> + unsigned int nr_irqs)
> +{
> + struct irq_data *data = irq_get_irq_data(virq);
> +
> + irq_gc_mask_clr_bit(data->parent_data);
I have a hard time to understand this. The irq domain is not hierarchical.
> + irq_domain_reset_irq_data(data);
> + domain = irq_domain_add_linear(node, nr_exti,
> + &irq_exti_domain_ops, NULL);
It's a simple linear domain. So how can data->parent_data be a valid
irq_data pointer? Answer: It can't!
But it doesn't blow up in your face simply because the alloc/free callbacks
are never invoked for simple non hierarchical domains. So you should have
removed that stuff after copying some other irqchip driver.
Thanks,
tglx
^ permalink raw reply
* [PATCH v5 07/14] drivers: acpi: iort: add support for ARM SMMU platform devices creation
From: Robin Murphy @ 2016-09-13 15:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160909142343.13314-8-lorenzo.pieralisi@arm.com>
On 09/09/16 15:23, Lorenzo Pieralisi wrote:
> In ARM ACPI systems, IOMMU components are specified through static
> IORT table entries. In order to create platform devices for the
> corresponding ARM SMMU components, IORT kernel code should be made
> able to parse IORT table entries and create platform devices
> dynamically.
>
> This patch adds the generic IORT infrastructure required to create
> platform devices for ARM SMMUs.
>
> ARM SMMU versions have different resources requirement therefore this
> patch also introduces an IORT specific structure (ie iort_iommu_config)
> that contains hooks (to be defined when the corresponding ARM SMMU
> driver support is added to the kernel) to be used to define the
> platform devices names, init the IOMMUs, count their resources and
> finally initialize them.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Tomasz Nowicki <tn@semihalf.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> ---
> drivers/acpi/arm64/iort.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 131 insertions(+)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index b89b3d3..e0a9b16 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -22,6 +22,7 @@
> #include <linux/kernel.h>
> #include <linux/list.h>
> #include <linux/pci.h>
> +#include <linux/platform_device.h>
> #include <linux/slab.h>
>
> struct iort_its_msi_chip {
> @@ -424,6 +425,135 @@ struct irq_domain *iort_get_device_domain(struct device *dev, u32 req_id)
> return irq_find_matching_fwnode(handle, DOMAIN_BUS_PCI_MSI);
> }
>
> +struct iort_iommu_config {
> + const char *name;
> + int (*iommu_init)(struct acpi_iort_node *node);
> + bool (*iommu_is_coherent)(struct acpi_iort_node *node);
> + int (*iommu_count_resources)(struct acpi_iort_node *node);
> + void (*iommu_init_resources)(struct resource *res,
> + struct acpi_iort_node *node);
> +};
> +
> +static __init
> +const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
> +{
> + return NULL;
> +}
> +
> +/**
> + * iort_add_smmu_platform_device() - Allocate a platform device for SMMU
> + * @fwnode: IORT node associated fwnode handle
> + * @node: Pointer to SMMU ACPI IORT node
> + *
> + * Returns: 0 on success, <0 failure
> + */
> +static int __init iort_add_smmu_platform_device(struct fwnode_handle *fwnode,
> + struct acpi_iort_node *node)
> +{
> + struct platform_device *pdev;
> + struct resource *r;
> + enum dev_dma_attr attr;
> + int ret, count;
> + const struct iort_iommu_config *ops = iort_get_iommu_cfg(node);
> +
> + if (!ops)
> + return -ENODEV;
> +
> + pdev = platform_device_alloc(ops->name, PLATFORM_DEVID_AUTO);
> + if (!pdev)
> + return PTR_ERR(pdev);
> +
> + count = ops->iommu_count_resources(node);
> +
> + r = kcalloc(count, sizeof(*r), GFP_KERNEL);
> + if (!r) {
> + ret = -ENOMEM;
> + goto dev_put;
> + }
> +
> + ops->iommu_init_resources(r, node);
> +
> + ret = platform_device_add_resources(pdev, r, count);
> + /*
> + * Resources are duplicated in platform_device_add_resources,
> + * free their allocated memory
> + */
> + kfree(r);
> +
> + if (ret)
> + goto dev_put;
> +
> + /*
> + * Add a copy of IORT node pointer to platform_data to
> + * be used to retrieve IORT data information.
> + */
> + ret = platform_device_add_data(pdev, &node, sizeof(node));
> + if (ret)
> + goto dev_put;
> +
> + pdev->dev.dma_mask = kmalloc(sizeof(*pdev->dev.dma_mask), GFP_KERNEL);
> + if (!pdev->dev.dma_mask) {
> + ret = -ENOMEM;
> + goto dev_put;
> + }
Since this is exclusively for creating SMMUs, and we know they should
never have weird shenanigans going on requiring different masks, I'd be
inclined to just point dev.dma_mask at dev.coherent_dma_mask and be done
with it.
> +
> + pdev->dev.fwnode = fwnode;
> +
> + /*
> + * Set default dma mask value for the table walker,
> + * to be overridden on probing with correct value.
> + */
> + *pdev->dev.dma_mask = DMA_BIT_MASK(32);
> + pdev->dev.coherent_dma_mask = *pdev->dev.dma_mask;
> +
> + attr = ops->iommu_is_coherent(node) ?
> + DEV_DMA_COHERENT : DEV_DMA_NON_COHERENT;
> +
> + /* Configure DMA for the page table walker */
> + acpi_dma_configure(&pdev->dev, attr);
Oh look, some more code which would be simpler if acpi_dma_configure()
set the default mask itself ;)
Robin.
> +
> + ret = platform_device_add(pdev);
> + if (ret)
> + goto dma_deconfigure;
> +
> + return 0;
> +
> +dma_deconfigure:
> + acpi_dma_deconfigure(&pdev->dev);
> + kfree(pdev->dev.dma_mask);
> +
> +dev_put:
> + platform_device_put(pdev);
> +
> + return ret;
> +}
> +
> +static acpi_status __init iort_match_iommu_callback(struct acpi_iort_node *node,
> + void *context)
> +{
> + int ret;
> + struct fwnode_handle *fwnode;
> +
> + fwnode = iort_get_fwnode(node);
> +
> + if (!fwnode)
> + return AE_NOT_FOUND;
> +
> + ret = iort_add_smmu_platform_device(fwnode, node);
> + if (ret) {
> + pr_err("Error in platform device creation\n");
> + return AE_ERROR;
> + }
> +
> + return AE_OK;
> +}
> +
> +static void __init iort_smmu_init(void)
> +{
> + iort_scan_node(ACPI_IORT_NODE_SMMU, iort_match_iommu_callback, NULL);
> + iort_scan_node(ACPI_IORT_NODE_SMMU_V3, iort_match_iommu_callback, NULL);
> +}
> +
> void __init acpi_iort_init(void)
> {
> acpi_status status;
> @@ -436,4 +566,5 @@ void __init acpi_iort_init(void)
> }
>
> acpi_probe_device_table(iort);
> + iort_smmu_init();
> }
>
^ permalink raw reply
* [GIT PULL] STi DT updates for v4.9
From: Arnd Bergmann @ 2016-09-13 15:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <c83fda3c-5120-a55b-dfb0-f550cbaf777a@st.com>
On Friday, September 2, 2016 3:58:00 PM CEST Patrice Chotard wrote:
> git://git.kernel.org/pub/scm/linux/kernel/git/pchotard/sti.git
>
> for you to fetch changes up to ba5ba11906d5462993f5fd4e3da4d234505427a0:
>
> ARM: dts: STiH41x-b2020: Update gpio specifier (2016-09-02 15:15:14 +0200)
>
> ----------------------------------------------------------------
>
> STi dts fixes and new STi 96board support:
>
> Add thermal node for STiH407 family boards
> Add specific nodes for STMicroelectronics 96Board
> Add new B2260 STi board file = 96Board
> Fix ahci issue on STiH407 family
> Fix debugfs/pinctrl kernel warning
> Update gpio-cells and gpio specifier
>
Pulled into next/dt. The URL had no tag name, but I found the tag "sti-dt-for-v4.9-1"
in your tree with that commit ID, so I pulled that.
Not sure what went wrong on your end, it wasn't a big deal but please
try to always include the tag. Thanks!
Arnd
^ permalink raw reply
* [PATCH 4/7] phy: meson: add USB2 PHY support for Meson8b and GXBB
From: Philipp Zabel @ 2016-09-13 15:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAFBinCDohOoFP_3GC+=tt6S7sPS308ao-yf+oGhdfwo-N-JnAA@mail.gmail.com>
Hi Martin,
Am Freitag, den 09.09.2016, 22:36 +0200 schrieb Martin Blumenstingl:
> On Fri, Sep 9, 2016 at 5:33 PM, Kevin Hilman <khilman@baylibre.com> wrote:
> > Martin Blumenstingl <martin.blumenstingl@googlemail.com> writes:
> >
> >> On Thu, Sep 8, 2016 at 10:53 PM, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
> >>> On 08/09/16 21:42, Kevin Hilman wrote:
> >>>>
> >>>> Ben Dooks <ben.dooks@codethink.co.uk> writes:
> >>>>
> >>>>> On 08/09/16 20:52, Martin Blumenstingl wrote:
> >>>>>>
> >>>>>> On Thu, Sep 8, 2016 at 9:35 PM, Kevin Hilman <khilman@baylibre.com>
> >>>>>> wrote:
> >>>>>>>>
> >>>>>>>> + phy = devm_phy_create(&pdev->dev, NULL, &phy_meson_usb2_ops);
> >>>>>>>> + if (IS_ERR(phy)) {
> >>>>>>>> + dev_err(&pdev->dev, "failed to create PHY\n");
> >>>>>>>> + return PTR_ERR(phy);
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> + if (usb_reset_refcnt++ == 0) {
> >>>>>>>> + ret = device_reset(&pdev->dev);
> >>>>>>>> + if (ret) {
> >>>>>>>> + dev_err(&phy->dev, "Failed to reset USB PHY\n");
> >>>>>>>> + return ret;
> >>>>>>>> + }
> >>>>>>>> + }
> >>>>>>>
> >>>>>>>
> >>>>>>> The ref count + reset here looks like something that could/should be
> >>>>>>> handled in a runtime PM callback.
> >>>>>>
> >>>>>> Unfortunately that doesn't work (as Jerome found out) because both
> >>>>>> PHYs are sharing the same reset line.
> >>>>>> So if the second PHY would call device_reset then it would also reset
> >>>>>> the first PHY!
> >>>>>>
> >>>>>> There's a comment above the declaration of usb_reset_refcnt which
> >>>>>> tries to explain this:
> >>>>>> "The PHYs are sharing a common reset line -> we are only allowed to
> >>>>>> reset once for all PHYs."
> >>>>>> Maybe I should move this comment to the "if (usb_reset_refcnt++ == 0)
> >>>>>> {" line to make it easier to see?
> >>>>>>
> >>>>>
> >>>>> pm-runtime has refcounting in it. When one of the nodes turns on,
> >>>>> the pm-runtime will call your driver to say there is a user when
> >>>>> this first use turns up.
> >>>>>
> >>>>> If all the sub-phys turn off and drop their refcount then the driver
> >>>>> is called to say there are no more users and you can go to sleep.
> >>>>
> >>>>
> >>>> After a chat w/Martin on IRC, It turns out runtime PM wont help here.
> >>>>
> >>>> The reason is because there are physically two PHY devices[1]. Those 2
> >>>> devices will be treated independely by runtime PM, and have separate
> >>>> use-counting, which means doing what I proposed would cause a reset to
> >>>> happen when either device was probed.
> >>>>
> >>>> So, I think it's OK as it is.
> >>>
> >>>
> >>> Surely you can do pm_runtime_get/put on the phy's parent platform
> >>> device and do it that way?
> >> could you please be more specific with that (do you mean pdev->dev.parent)?
> >> so we would use pm_runtime_{get_sync,put} with the parent, while we
> >> would still define the runtime_resume in our driver.
> >
> > You'd also need to do get/put on the children, but yes, that's what Ben
> > is suggesting.
> >
> > However, the problem with all of the solutions proposed (runtime PM ones
> > included) is that we're forcing a board-specific design issue (2 devices
> > sharing a reset line) into a driver that should not have any
> > board-specific assumptions in it.
> >
> > For example, if this driver is used on another platform where different
> > PHYs have different reset lines, then one of them (the unlucky one who
> > is not probed first) will never get reset. So any form of per-device
> > ref-counting is not a portable solution.
> indeed, so in simple words we would need something like
> reset_control_do_once(rstc, RESET/ASSERT/DEASSERT) which would
> remember internally if any action has already been executed: if not it
> does a _reset, _assert or _deassert and otherwise it does nothing.
>
> > I'm not sure yet how the reset framework is supposed to handle shared
> > reset lines, but that needs some investigation. I quick glance and it
> > seems that reset controllers can have shared lines, so that should be
> > investigated.
> I added Philipp and Hans to this thread - maybe they can comment on this.
> To sum it up, our problem is:
> - there are two separate USB PHYs on Meson GXBB
> - both are sharing the same reset line (provided by the reset-meson driver)
> - during initialization of the PHYs we must only call
> reset_control_reset(rstc) once (if we do it for the first *and* second
> PHY then the first PHY gets confused once the second PHY uses the
> reset because the first PHY's state is reset as well)
If you have an initially asserted reset line and you can enable the
first module by deasserting the reset via reset_control_deassert (and
reset_control_assert to signal when the module may be disabled again
after use), shared resets are for you.
If you need a reset pulse or have no direct control over the reset line,
(device_reset), the reset framework currently has no solution for this.
The ugly thing about reset_control_once would be that it can't re-reset
modules when unloading and reloading driver modules.
A real solution for shared reset lines with reset pulses would have to
be some kind of reset request framework where if one module requests a
reset, the other module sharing the reset could be notified, and then
either veto the reset or, if possible, cease operations, store its
state, and prepare to be reset, too, and afterwards restore state. I'd
prefer not to think about this too much unless absolutely necessary.
regards
Philipp
^ permalink raw reply
* [PATCHv2] ARM: dts: explicitly mark skeleton.dtsi as deprecated
From: Arnd Bergmann @ 2016-09-13 15:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1472834090-24528-1-git-send-email-mark.rutland@arm.com>
On Friday, September 2, 2016 5:34:50 PM CEST Mark Rutland wrote:
> As noted in commit 3ebee5a2e141496b ("arm64: dts: kill skeleton.dtsi"),
> there are a number of problems with skeleton.dtsi, and it would be
> prefereable to remove it entirely. As there are a large number of
> existing users, fixing these up will take a while.
>
> This patch adds a note to arm's skeleton.dtsi noting that this is the
> case, to make this more obvious and hopefully minimize new uptake of
> skeleton.dtsi in the mean time.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Olof Johansson <olof@lixom.net>
> ---
> arch/arm/boot/dts/skeleton.dtsi | 4 ++++
> 1 file changed, 4 insertions(+)
>
> Arnd, Olof, could you please pick this up?
>
>
Applied to next/dt, thanks!
Arnd
^ permalink raw reply
* [GIT PULL] Qualcomm Device Tree Changes for v4.9
From: Arnd Bergmann @ 2016-09-13 15:30 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1472875131-18077-1-git-send-email-andy.gross@linaro.org>
On Friday, September 2, 2016 10:58:51 PM CEST Andy Gross wrote:
> Qualcomm Device Tree Changes for v4.9
>
> * Rework dr_mode on APQ8064 and Nexus7
> * Add MSM8974 BLSP1 UART1 port
> * Add AP148 SATA mapping
> * Add support for LG Nexus 5 (Hammerhead)
> * Fixup MSM8660/MSM8064 SPMI/MPP IRQs
> * Add Nexus7 IMEM/reboot reason
> * Add Honami touchscreen support
> * Add TSENS support on MSM8974, APQ8064, and APQ8084
> * Add APQ8060 Dragonboard PM8058 LEDs
> * Rework VPH PWR REG for MSM8974
>
Pulled into next/dt, thanks!
Arnd
^ permalink raw reply
* [GIT PULL] ARM: mvebu: dt for v4.9 (#1)
From: Arnd Bergmann @ 2016-09-13 15:32 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <87inubzmi7.fsf@free-electrons.com>
On Sunday, September 4, 2016 5:57:04 PM CEST Gregory CLEMENT wrote:
> mvebu dt for 4.9 (part 1)
>
> - update for Armada XP/38x allowing using direct access SPI
> - various improvement for Armada 39x platforms
> - add pinctrl information for NANd on Armada 38x
> - fix the kirkwood based Openblock A6 external GPIO pins
>
>
Pulled into next/dt, thanks!
Arnd
^ permalink raw reply
* [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi function device for legacy interrupts.
From: Bjorn Helgaas @ 2016-09-13 15:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160913150511.GC27748@localhost>
[+cc Ley Foon (altera), Thomas (aardvark), Kishon (dra7xx), Murali (keystone)]
On Tue, Sep 13, 2016 at 10:05:11AM -0500, Bjorn Helgaas wrote:
> On Tue, Sep 13, 2016 at 08:41:28AM +0100, Marc Zyngier wrote:
> > On 12/09/16 23:02, Bjorn Helgaas wrote:
> > > On Thu, Sep 01, 2016 at 05:19:55AM +0000, Bharat Kumar Gogada wrote:
> > >>>>>>> Hi Bharat,
> > >>>>>>>> @@ -561,7 +561,7 @@ static int nwl_pcie_init_irq_domain(struct
> > >>>>>>>> nwl_pcie
> > >>>>>>> *pcie)
> > >>>>>>>> }
> > >>>>>>>>
> > >>>>>>>> pcie->legacy_irq_domain = irq_domain_add_linear(legacy_intc_node,
> > >>>>>>>> - INTX_NUM,
> > >>>>>>>> + INTX_NUM + 1,
> > >>>>>>>> &legacy_domain_ops,
> > >>>>>>>> pcie);
> > >>>>>>>
> > >>>>>>> This feels like the wrong thing to do. You have INTX_NUM irqs, so
> > >>>>>>> the domain allocation should reflect this. On the other hand, the
> > >>>>>>> way the driver currently deals with mappings is quite broken
> > >>>>>>> (consistently adding 1 to
> > >>>>> the HW interrupt).
> > >>>>>>>
> > >>>>>> Hi Marc,
> > >>>>>>
> > >>>>>> Without above change I get following crash in kernel while booting.
> > >>>>>>
> > >>>>>> [ 2.441684] error: hwirq 0x4 is too large for dummy
> > >>>>>>
> > >>>>>> [ 2.441694] ------------[ cut here ]------------
> > >>>>>>
> > >>>>>> [ 2.441698] WARNING: at kernel/irq/irqdomain.c:344
> > >>>>>>
> > >>>>>> [ 2.441702] Modules linked in:
> > >>>>>>
> > >>>>>> [ 2.441706]
> > >>>>>>
> > >>>>>> [ 2.441714] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.4.0 #8
> > >>>>>>
> > >>>>>> [ 2.441718] Hardware name: xlnx,zynqmp (DT)
> > >>>>>>
> > >>>>>> [ 2.441723] task: ffffffc071886b80 ti: ffffffc071888000 task.ti:
> > >>>>> ffffffc071888000
> > >>>>>>
> > >>>>>> [ 2.441732] PC is at irq_domain_associate+0x138/0x1c0
> > >>>>>>
> > >>>>>> [ 2.441738] LR is at irq_domain_associate+0x138/0x1c0
> > >>>>>>
> > >>>>>> In kernel/irq/irqdomain.c function irq_domain_associate
> > >>>>>>
> > >>>>>> if (WARN(hwirq >= domain->hwirq_max,
> > >>>>>> "error: hwirq 0x%x is too large for %s\n", (int)hwirq, domain-
> > >>>> name))
> > >>>>>> return -EINVAL;
> > >>>>>>
> > >>>>>> Here the hwirq and hwirq_max are equal to 4 without the above
> > >>>>>> condition
> > >>>>> (INTX_NUM + 1) due to which crash is coming.
> > >>>>>> This is happening as the legacy interrupts are starting from 1 (INTA).
> > >>>>>
> > >>>>> I understood that. I'm still persisting in saying that you have the wrong fix.
> > >>>>>
> > >>>>> Your domain should always allocate many interrupts as you have
> > >>>>> interrupt sources. These interrupts (hwirq) should be numbered from 0 to (n-
> > >>> 1).
> > >>>>
> > >>>> Agreed, but here comes the problem the hwirq for legacy interrupts
> > >>>> will start at 0x1 to 0x4 (INTA to INTD) and these values are as per
> > >>>> PCIe specification for legacy interrupts. So these cannot be numbered
> > >>>> from 0. So when 0x4 (INTD) for a multi-function device comes the crash
> > >>>> occurs.
> > >>>
> > >>> So who provides this hwirq? Who calls irq_domain_associate() with hwirq set to
> > >>> 4?
> > >>>
> > >> PCIe subsystem invokes pcibios_add_device function in arch/arm64/kernel/pci.c for every pci device.
> > >> The purpose of this function is to assign dev->irq using of_irq_parse_and_map_pci.
> > >> of_irq_parse_and_map_pci invokes of_irq_parse_pci where it reads PCI_INTERRUPT_PIN from configuration space and saves it
> > >> in parameter of struct of_phandle_args.
> > >> This structure is passed to irq_create_of_mapping where it invokes irq_create_fwspec_mapping.
> > >> irq_create_fwspec_mapping invokes irq_domain_translate and gets hwirq, here the above saved PCI_INTERRUPT_PIN value is assigned
> > >> to hwirq (*hwirq = fwspec->param[0]).
> > >> And then using this hwirq irq_create_mapping -> irq_domain_associate were invoked and mapping is created for virtual irq with this hwirq.
> > >> So for any end point PCI_INTERRUPT_PIN value starts from 0x1 to 0x4 and so hwirq starts from 0x1 to 0x4.
> > >>
> > >> So the values are more generic w.r.t to protocol, that's why hwirq will range from 0x1 to 0x4.
> > >> And then if you check pcie-altera.c they are doing this adding one in their handler and while creating legacy domain.
> > >
> > > Is this resolved yet? Marc, are you happy, or should we iterate on this
> > > again?
> >
> > Ah, sorry to have dropped the ball on this patch.
>
> No problem, I wasn't making forward progress anyway.
>
> > I guess that given that the infrastructure imposes the hwirq range on
> > the host drivers, Bharat's approach is the only way (and a number of
> > other host drivers are already slightly broken). I'll try and have a
> > look at solving this at the generic level. In the meantime:
> >
> > Acked-by: Marc Zyngier <marc.zyngier@arm.com>
>
> After looking at this myself, I'm not happy with this either. It feels
> like there are bugs lurking here and we're just hiding one of them.
>
> Here are the callers of irq_domain_add_linear() for legacy INTx in
> drivers/pci/host:
>
> advk_pcie_init_irq_domain LEGACY_IRQ_NUM (4)
> dra7xx_pcie_init_irq_domain 4
> ks_dw_pcie_host_init MAX_LEGACY_IRQS (4)
> altera_pcie_init_irq_domain INTX_NUM + 1 (5)
> nwl_pcie_init_irq_domain INTX_NUM + 1 (5)
> xilinx_pcie_init_irq_domain 4
The altera change corresponding to this was 99496bd2971f ("PCI: altera: Fix
error when INTx is 4"). I should have noticed this inconsistency back
then.
Are aardvark, dra7xx, keystone, and xilinx (non-NWL) broken because they
only request 4 IRQs and only INTA, INTB, and INTC work?
> I think all of these use the of_irq_parse_and_map_pci() path you
> mentioned, so if the problem is in the way that path works, I would
> think these should *all* be requesting the same number of interrupts
> in the domain.
>
> I agree with Marc that we should request 4 IRQs, because that's what
> we need. If we can't do that for some reason, we ought to at least
> make all these callers the same.
>
> Bjorn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" 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
* [GIT PULL] LPC18xx DTS changes for v4.9 part2
From: Arnd Bergmann @ 2016-09-13 15:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160906201600.31373-1-manabian@gmail.com>
On Tuesday, September 6, 2016 10:16:00 PM CEST Joachim Eastwood wrote:
> Device Tree clean up for LPC18xx platform
>
> * Removal of skeleton.dtsi from the common armv7-m dtsi together
> with lpc18xx and efm32 platforms.
>
> * Fix for unit address warnings from the dtc on lpc18xx/efm32.
> That is made possible with skeleton.dtsi gone.
>
>
Pulled into next/dt, thanks!
Arnd
^ permalink raw reply
* [GIT PULL] Second Round of Renesas ARM Based SoC DT Updates for v4.9
From: Arnd Bergmann @ 2016-09-13 15:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1473234371.git.horms+renesas@verge.net.au>
On Wednesday, September 7, 2016 9:49:30 AM CEST Simon Horman wrote:
> Second Round of Renesas ARM Based SoC DT Updates for v4.9
>
> Fixes (for v4.9):
> * Correct PWM clock parent on r8a7794 SoC
>
> Clean-up:
> * Remove obsolete vsp1 properties from r8a779[01] SoCs
>
> New boards:
> * Add r8a7792/wheat and r7s72100/rskrza1 boards
>
> Enablement:
> * Enable LEDs, DU, SDHI on r8a7792/blanche board
> * Enable MMCIF and SDHI on r8a7794/alt board
> * Add SPI and VSP1 to r8a7792 SoC
> * Add ethernet to r7s72100 SoC
>
Pulled into next/dt, thanks!
Arnd
^ permalink raw reply
* [GIT PULL] Allwinner DT changes for 4.9
From: Arnd Bergmann @ 2016-09-13 15:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160907194322.GA7535@lukather>
On Wednesday, September 7, 2016 9:43:22 PM CEST Maxime Ripard wrote:
> Allwinner DT changes for 4.9
>
> Business as usual on our side, with a mix of new boards and new IPs enabled
> on boards:
> - Support for the GPIO found on the AXP PMIC
> - ESP8089 on the relevant boards
> - More boards converted to the reference design and q8 designs
> - New boards: Orange Pi PC Plus, Orange Pi 2, Orange Pi Plus 2E, Orange
> Pi Lite, Olimex A33-Olinuxino, Empire Electronix M712, inet-d978_rev2,
> Nano Pi Neo
>
Pulled into next/dt, thanks!
Arnd
^ permalink raw reply
* [GIT PULL 2/3] Rockchip dts32 changes for 4.9
From: Arnd Bergmann @ 2016-09-13 15:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481725.D51jRkC1a3@phil>
On Thursday, September 8, 2016 4:10:35 PM CEST Heiko Stuebner wrote:
> 32bit Rockchip devicetree changes containing two new rk3288 boards (Fennec
> and Firefly-reload), display support for the rk3288-evb, support for the
> recently added firmware reboot-flag support and some housekeeping cleanups.
>
Pulled into next/dt, thanks,
Arnd
^ permalink raw reply
* [GIT PULL 3/4] ARM: tegra: Device tree changes for v4.9-rc1
From: Arnd Bergmann @ 2016-09-13 15:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160909152032.9550-3-thierry.reding@gmail.com>
On Friday, September 9, 2016 5:20:31 PM CEST Thierry Reding wrote:
> ARM: tegra: Device tree changes for v4.9-rc1
>
> Contains a couple of cleanups for Tegra114 device trees.
>
>
Pulled into next/dt, thanks!
Arnd
^ permalink raw reply
* [PATCH] coresight: tmc: fix for trace collection bug in sysFS mode
From: Mathieu Poirier @ 2016-09-13 15:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473769245-18159-1-git-send-email-venkatesh.vivekanandan@broadcom.com>
On 13 September 2016 at 06:20, Venkatesh Vivekanandan
<venkatesh.vivekanandan@broadcom.com> wrote:
> tmc_etb_dump_hw is never called in sysFS mode to collect trace from
> hardware, because drvdata->mode is set to CS_MODE_DISABLED at
> tmc_disable_etf/etr_sink
>
> static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
> {
> .
> .
> if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
> tmc_etb_dump_hw(drvdata);
> .
> .
> }
>
> static void tmc_disable_etf_sink(struct coresight_device *csdev)
> {
> .
> .
> val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
> /* Disable the TMC only if it needs to */
> if (val != CS_MODE_DISABLED)
> tmc_etb_disable_hw(drvdata);
You are correct.
> .
> .
> }
>
> Signed-off-by: Venkatesh Vivekanandan <venkatesh.vivekanandan@broadcom.com>
> ---
> drivers/hwtracing/coresight/coresight-tmc-etf.c | 9 +++++----
> drivers/hwtracing/coresight/coresight-tmc-etr.c | 9 +++++----
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etf.c b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> index 466af86..c7fb7f7 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etf.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etf.c
> @@ -61,6 +61,8 @@ static void tmc_etb_dump_hw(struct tmc_drvdata *drvdata)
>
> static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
> {
> + long val;
> +
> CS_UNLOCK(drvdata->base);
>
> tmc_flush_and_stop(drvdata);
> @@ -68,7 +70,8 @@ static void tmc_etb_disable_hw(struct tmc_drvdata *drvdata)
> * When operating in sysFS mode the content of the buffer needs to be
> * read before the TMC is disabled.
> */
> - if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
> + val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
> + if (val == CS_MODE_SYSFS)
> tmc_etb_dump_hw(drvdata);
> tmc_disable_hw(drvdata);
>
> @@ -225,7 +228,6 @@ static int tmc_enable_etf_sink(struct coresight_device *csdev, u32 mode)
>
> static void tmc_disable_etf_sink(struct coresight_device *csdev)
> {
> - long val;
> unsigned long flags;
> struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -235,9 +237,8 @@ static void tmc_disable_etf_sink(struct coresight_device *csdev)
> return;
> }
>
> - val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
> /* Disable the TMC only if it needs to */
> - if (val != CS_MODE_DISABLED)
> + if (local_read(&drvdata->mode) != CS_MODE_DISABLED)
> tmc_etb_disable_hw(drvdata);
This would work but tmc_enable_etf_sink() and tmc_disable_etf_sink()
are no longer balanced. Another approach would be to add a "mode"
parameter to tmc_etb_disable_hw() and so something like:
if (val != CS_MODE_DISABLED)
tmc_etb_disable_hw(drvdata, val);
In tmc_etb_disable_hw(), if mode == CS_MODE_SYSFS then we can move
ahead with the dump operation. The same apply for ETR.
Thanks,
Mathieu
>
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 688be9e..480794b 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> @@ -73,6 +73,8 @@ static void tmc_etr_dump_hw(struct tmc_drvdata *drvdata)
>
> static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
> {
> + long val;
> +
> CS_UNLOCK(drvdata->base);
>
> tmc_flush_and_stop(drvdata);
> @@ -80,7 +82,8 @@ static void tmc_etr_disable_hw(struct tmc_drvdata *drvdata)
> * When operating in sysFS mode the content of the buffer needs to be
> * read before the TMC is disabled.
> */
> - if (local_read(&drvdata->mode) == CS_MODE_SYSFS)
> + val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
> + if (val == CS_MODE_SYSFS)
> tmc_etr_dump_hw(drvdata);
> tmc_disable_hw(drvdata);
>
> @@ -215,7 +218,6 @@ static int tmc_enable_etr_sink(struct coresight_device *csdev, u32 mode)
>
> static void tmc_disable_etr_sink(struct coresight_device *csdev)
> {
> - long val;
> unsigned long flags;
> struct tmc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>
> @@ -225,9 +227,8 @@ static void tmc_disable_etr_sink(struct coresight_device *csdev)
> return;
> }
>
> - val = local_xchg(&drvdata->mode, CS_MODE_DISABLED);
> /* Disable the TMC only if it needs to */
> - if (val != CS_MODE_DISABLED)
> + if (local_read(&drvdata->mode) != CS_MODE_DISABLED)
> tmc_etr_disable_hw(drvdata);
>
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> --
> 2.1.0
>
^ permalink raw reply
* [PATCH -next] net: ethernet: dwmac: fix non static symbol warning
From: David Miller @ 2016-09-13 15:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473510690-25198-1-git-send-email-weiyj.lk@gmail.com>
From: Wei Yongjun <weiyj.lk@gmail.com>
Date: Sat, 10 Sep 2016 12:31:30 +0000
> From: Wei Yongjun <weiyongjun1@huawei.com>
>
> Fixes the following sparse warning:
>
> drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c:172:1: warning:
> symbol 'stm32_dwmac_pm_ops' was not declared. Should it be static?
>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Applied.
^ permalink raw reply
* [GIT PULL 3/5] i.MX device tree updates for 4.9
From: Arnd Bergmann @ 2016-09-13 15:43 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1473670948-4265-3-git-send-email-shawnguo@kernel.org>
On Monday, September 12, 2016 5:02:26 PM CEST Shawn Guo wrote:
> i.MX device tree changes for 4.9:
> - Add SoC support for i.MX7 Solo which is a reduced version of i.MX7
> Dual.
> - New board support: Gateworks Ventana i.MX6Q/DL GW553x, Inverse Path
> i.MX53 USB armory, i.MX6Q/DL TS-4900 from Technologic Systems,
> i.MX6UL GEA M6UL modules from Engicam, i.MX7 Solo Warp7 board.
> - Add DMA and IPU CSI devices for i.MX53 SoC support.
> - Refine i.MX7 Dual SoC DTS as a preparation of i.MX7 Solo support.
> - Use of_graph dt nodes to describe the panel for vf610-colibri and
> ls1021a-twr boards.
> - Add gpio-ranges property to i.MX6 GPIO controllers, which will be
> useful when GPIO driver is changed to request pad configuration as
> GPIO function.
> - Random device additions or small changes for various board support.
>
>
Pulled into next/dt, thanks!
Arnd
^ permalink raw reply
* ARM, SoC: About the use DT-defined properties by 3rd-party drivers
From: Mark Rutland @ 2016-09-13 15:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <57D8137F.8040508@laposte.net>
On Tue, Sep 13, 2016 at 04:55:59PM +0200, Sebastian Frias wrote:
> On 09/13/2016 03:12 PM, Mark Rutland wrote:
> >> Exactly, that is why I was thinking it would take less "review" time.
> >> Indeed, if there is no driver, why would it matter what those bindings
> >> are?
> >
> > If you believe that the bindings don't matter, then there is absolutely
> > no reason for them to exist in the first place.
> >
> > If those binding matter to *anyone*, then those collating the bindings
> > have some responsibility of stewardship, and that includes
> > review/maintenance/etc.
>
> The thing is that right now it seems the "responsibility of stewardship"
> lies only within "Linux", whereas DT is proposed as open for everybody,
> Bootloaders, FreeBSD, etc.
>
> In that case, shouldn't the "responsibility" be shared?
Ideally, yes.
Which is one of the reasons devicetree.org was set up as a common forum
for projects to collaborate on devicetree.
Thanks,
Mark.
^ 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