All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: jeffy <jeffy.chen@rock-chips.com>
Cc: Amitkumar Karwar <akarwar@marvell.com>,
	Cathy Luo <cluo@marvell.com>,
	Nishant Sarmukadam <nishants@marvell.com>,
	rajatja@google.com, dmitry.torokhov@gmail.com,
	Bjorn Helgaas <bhelgaas@google.com>,
	Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [v4,3/3] mwifiex: Enable WoWLAN for both sdio and pcie
Date: Mon, 10 Jul 2017 15:31:40 -0700	[thread overview]
Message-ID: <20170710223138.GA145502@google.com> (raw)
In-Reply-To: <595EFA40.1080405@rock-chips.com>

+ linux-pci
- linux-wireless

On Fri, Jul 07, 2017 at 11:04:32AM +0800, Jeffy Chen wrote:
> On 07/07/2017 08:53 AM, Brian Norris wrote:
> >It kind of seems like we inadvertently conflicted with the PCI OF
> >interrupt spec [1]. There, the "interrupts" property for a device (if
> >present) is supposed to represent INT{A...D} with values of {1...4}.
> >IIUC, there should only be a single entry in this property.
> >
> >If we were to extend this properly, I guess that would mean we'd need a
> >second "interrupts" entry, with a different parent. I think we can use
> >"interrupts-extended" for that.
> >
> >So we'd need to document an optional "interrupt-names" for Marvell, and
> >have the driver try that first. The rough outline would be something
> >like this.
> >
> >For the device tree (e.g., rk3399-gru):
> >
> >-			interrupt-parent = <&gpio0>;
> >-			interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
> >+			interrupts-extended = <&pcie0 1>, <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
> >+			interrupt-names = "int-A", "wake";
> This is a great idea.
> 
> And how about also add a property to tell of_pci_irq to ignore of
> irq and force using PCI_INTERRUPT_PIN? Since there might be devices
> don't use pci irq, but using other irq(wowlan for example).

Even if they don't use the PCI IRQ, is there a way to provide an empty /
un-mapped entry?

I'm a little wary on trying to "extend" (which can sometimes be read
"break") the device tree spec here too much more. But perhaps DT or PCI
folks have a better recommendation (if they can hold their noses long
enough).

> Then we can specify this property and add a name("wake") to the wifi
> wake irq here. And interrupts-extended would still be an available
> option.

Brian

WARNING: multiple messages have this Message-ID (diff)
From: Brian Norris <briannorris-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
To: jeffy <jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Cc: Amitkumar Karwar
	<akarwar-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Cathy Luo <cluo-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	Nishant Sarmukadam
	<nishants-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>,
	rajatja-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org,
	dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	Bjorn Helgaas <bhelgaas-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>,
	Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [v4,3/3] mwifiex: Enable WoWLAN for both sdio and pcie
Date: Mon, 10 Jul 2017 15:31:40 -0700	[thread overview]
Message-ID: <20170710223138.GA145502@google.com> (raw)
In-Reply-To: <595EFA40.1080405-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

+ linux-pci
- linux-wireless

On Fri, Jul 07, 2017 at 11:04:32AM +0800, Jeffy Chen wrote:
> On 07/07/2017 08:53 AM, Brian Norris wrote:
> >It kind of seems like we inadvertently conflicted with the PCI OF
> >interrupt spec [1]. There, the "interrupts" property for a device (if
> >present) is supposed to represent INT{A...D} with values of {1...4}.
> >IIUC, there should only be a single entry in this property.
> >
> >If we were to extend this properly, I guess that would mean we'd need a
> >second "interrupts" entry, with a different parent. I think we can use
> >"interrupts-extended" for that.
> >
> >So we'd need to document an optional "interrupt-names" for Marvell, and
> >have the driver try that first. The rough outline would be something
> >like this.
> >
> >For the device tree (e.g., rk3399-gru):
> >
> >-			interrupt-parent = <&gpio0>;
> >-			interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
> >+			interrupts-extended = <&pcie0 1>, <&gpio0 8 IRQ_TYPE_LEVEL_LOW>;
> >+			interrupt-names = "int-A", "wake";
> This is a great idea.
> 
> And how about also add a property to tell of_pci_irq to ignore of
> irq and force using PCI_INTERRUPT_PIN? Since there might be devices
> don't use pci irq, but using other irq(wowlan for example).

Even if they don't use the PCI IRQ, is there a way to provide an empty /
un-mapped entry?

I'm a little wary on trying to "extend" (which can sometimes be read
"break") the device tree spec here too much more. But perhaps DT or PCI
folks have a better recommendation (if they can hold their noses long
enough).

> Then we can specify this property and add a name("wake") to the wifi
> wake irq here. And interrupts-extended would still be an available
> option.

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2017-07-10 22:31 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-15 13:36 [PATCH v4 1/3] mwifiex: Allow mwifiex early access to device structure Amitkumar Karwar
2016-11-15 13:36 ` [PATCH v4 2/3] mwifiex: Introduce mwifiex_probe_of() to parse common properties Amitkumar Karwar
2016-11-15 13:36 ` [PATCH v4 3/3] mwifiex: Enable WoWLAN for both sdio and pcie Amitkumar Karwar
2016-11-15 17:35   ` Dmitry Torokhov
2016-11-15 18:16     ` Brian Norris
2017-07-03 10:46   ` [v4,3/3] " jeffy
2017-07-07  0:53     ` Brian Norris
2017-07-07  0:53       ` Brian Norris
2017-07-07  3:04       ` jeffy
2017-07-07  3:04         ` jeffy
2017-07-10 22:31         ` Brian Norris [this message]
2017-07-10 22:31           ` Brian Norris
2016-11-18 11:22 ` [v4,1/3] mwifiex: Allow mwifiex early access to device structure Kalle Valo
2016-11-19  7:13 ` Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170710223138.GA145502@google.com \
    --to=briannorris@chromium.org \
    --cc=akarwar@marvell.com \
    --cc=bhelgaas@google.com \
    --cc=cluo@marvell.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jeffy.chen@rock-chips.com \
    --cc=linux-pci@vger.kernel.org \
    --cc=nishants@marvell.com \
    --cc=rajatja@google.com \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.