Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: m.karthikeyan@mobiveil.co.in, linux-pci@vger.kernel.org,
	mingkai.hu@nxp.com, mark.rutland@arm.com, minghuan.lian@nxp.com,
	zhiqiang.hou@nxp.com, l.subrahmanya@mobiveil.co.in
Subject: Re: [PATCH] PCI: mobiveil: Modified the Device tree bindings interrupt-map example
Date: Thu, 2 Jul 2020 16:29:04 +0100
Message-ID: <20200702152904.GA25591@e121166-lin.cambridge.arm.com> (raw)
In-Reply-To: <20191029224055.GA117186@google.com>

On Tue, Oct 29, 2019 at 05:40:55PM -0500, Bjorn Helgaas wrote:
> On Tue, Oct 29, 2019 at 09:23:42PM +0530, m.karthikeyan@mobiveil.co.in wrote:
> > From: Karthikeyan Mitran <m.karthikeyan@mobiveil.co.in>
> 
> *All* patches modify something, so the subject line isn't very
> informative.  I think you're actually fixing a bug:
> 
> > -		interrupt-map = <0 0 0 0 &pci_express 0>,
> > +		interrupt-map = <0 0 0 1 &pci_express 0>,
> 
> and *that* should be clear in the subject.  Maybe something like:
> 
>   dt-bindings: PCI: mobiveil: Correct INTx mapping
> 
> I don't know the implications of this for backwards compatibility.

Yes that has to be tested but nonetheless this binding is still
broken (and probably the driver was made to work with it so I
need to check it).

It is certain that the current binding can't work with a PCI device
requiring an INTD.

Lorenzo

> 
> > Legacy IRQs Interrupt pins map 01h, 02h, 03h, and 04h while value of 00h
> > indicates Function uses no legacy interrupt Message
> > 
> > Signed-off-by: Karthikeyan Mitran <m.karthikeyan@mobiveil.co.in>
> > ---
> >  .../devicetree/bindings/pci/mobiveil-pcie.txt | 19 ++++++++++++-------
> >  1 file changed, 12 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/mobiveil-pcie.txt b/Documentation/devicetree/bindings/pci/mobiveil-pcie.txt
> > index 64156993e05..b9dcb0ddc19 100644
> > --- a/Documentation/devicetree/bindings/pci/mobiveil-pcie.txt
> > +++ b/Documentation/devicetree/bindings/pci/mobiveil-pcie.txt
> > @@ -31,9 +31,14 @@ Required properties:
> >  - interrupts: The interrupt line of the PCIe controller
> >  		last cell of this field is set to 4 to
> >  		denote it as IRQ_TYPE_LEVEL_HIGH type interrupt.
> > -- interrupt-map-mask,
> > -	interrupt-map: standard PCI properties to define the mapping of the
> > -	PCI interface to interrupt numbers.
> > +- interrupt-map-mask:
> > +		Its a 4-tuple like structure denoting phys.hi, phys.mid,
> > +		phys.low and interrupt-cell
> > +- interrupt-map: standard PCI properties to define the mapping of the
> > +		PCI interface to interrupt numbers. Here the first 4-tuple
> > +		are represented similar to interrupt-map-mask representation
> > +		while the next fields represents Interrupt controller phandle
> > +		and its #interrupt-cells fields
> 
> The original text was basically the same as all the other bindings, so
> I don't really see the point of changing this to be different from all
> the rest.
> 
> A few (mediatek, nvidia) refer to the "standard PCI bus binding
> document" for more details.
> 
> Maybe there should be a common place in the Linux source for
> describing these "standard properties" so it's not repeated
> everywhere?
> 
> >  - ranges: ranges for the PCI memory regions (I/O space region is not
> >  	supported by hardware)
> >  	Please refer to the standard PCI bus binding document for a more
> > @@ -63,10 +68,10 @@ Example:
> >  		#interrupt-cells = <1>;
> >  		interrupts = < 0 89 4 >;
> >  		interrupt-map-mask = <0 0 0 7>;
> > -		interrupt-map = <0 0 0 0 &pci_express 0>,
> > -				<0 0 0 1 &pci_express 1>,
> > -				<0 0 0 2 &pci_express 2>,
> > -				<0 0 0 3 &pci_express 3>;
> > +		interrupt-map = <0 0 0 1 &pci_express 0>,
> > +				<0 0 0 2 &pci_express 1>,
> > +				<0 0 0 3 &pci_express 2>,
> > +				<0 0 0 4 &pci_express 3>;
> 
> Above you say the first 4-tuple in interrupt-map is similar to
> interrupt-map-mask, but these all look the same and they don't look
> like interrupt-map-mask.
> 
> Oh, I guess you mean the "0 0 0 1" is a 4-tuple and the
> "&pci_express 0" part is the "next fields".  I would have called that
> a 6-tuple.  But I'm not a DT person, so maybe I just don't know the
> terminology.
> 
> >  		ranges = < 0x83000000 0 0x00000000 0xa8000000 0 0x8000000>;
> >  
> >  	};
> > -- 
> > 2.17.1
> > 
> > 
> > -- 
> > Mobiveil INC., CONFIDENTIALITY NOTICE: This e-mail message, including any 
> > attachments, is for the sole use of the intended recipient(s) and may 
> > contain proprietary confidential or privileged information or otherwise be 
> > protected by law. Any unauthorized review, use, disclosure or distribution 
> > is prohibited. If you are not the intended recipient, please notify the 
> > sender and destroy all copies and the original message.
> 
> You should try to avoid confidentiality notices like this in email to
> the public mailing lists.  I don't know whether we could apply a patch
> with this notice on it or not.

      parent reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-29 15:53 m.karthikeyan
2019-10-29 22:40 ` Bjorn Helgaas
2019-11-01 16:10   ` Andrew Murray
2020-07-02 15:29   ` Lorenzo Pieralisi [this message]

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=20200702152904.GA25591@e121166-lin.cambridge.arm.com \
    --to=lorenzo.pieralisi@arm.com \
    --cc=helgaas@kernel.org \
    --cc=l.subrahmanya@mobiveil.co.in \
    --cc=linux-pci@vger.kernel.org \
    --cc=m.karthikeyan@mobiveil.co.in \
    --cc=mark.rutland@arm.com \
    --cc=minghuan.lian@nxp.com \
    --cc=mingkai.hu@nxp.com \
    --cc=zhiqiang.hou@nxp.com \
    /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

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org
	public-inbox-index linux-pci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git