[GIT PULL] Mediatek: 32-bit DT update for v4.15

[GIT PULL] Mediatek: 32-bit DT update for v4.15

[Matthias Brugger]

Hi Arnd and Olof,

Please pull the following commits.

Thanks
Matthias

---
The following changes since commit 2bd6bf03f4c1c59381d62c61d03f6cc3fe71f66e:

  Linux 4.14-rc1 (2017-09-16 15:47:51 -0700)

are available in the git repository at:

  https://github.com/mbgg/linux-mediatek.git tags/v4.14-next-dts32

for you to fetch changes up to 355d73e3b2e20d42195268e539a299dedbcc4f59:

  arm: dts: mt7623: remove unused compatible string for pio node (2017-10-20
13:15:59 +0200)

----------------------------------------------------------------
- mt76233 add PCIe node
- mt7623 update nodes to binding description
- mt2701 add display pwn nodes
- mt2701 update audio node description

----------------------------------------------------------------
Ryder Lee (5):
      arm: dts: mediatek: update audio node for mt2701 and mt7623
      arm: dts: mt7623: add PCIe related nodes
      arm: dts: mt7623: update crypto node
      arm: dts: mt7623: update usb related nodes
      arm: dts: mt7623: remove unused compatible string for pio node

Weiqing Kong (2):
      arm: dts: mt2701: add pwm backlight device node
      arm: dts: mt2701: enable display pwm backlight

 arch/arm/boot/dts/mt2701-evb.dts              |  23 +++++
 arch/arm/boot/dts/mt2701.dtsi                 |  13 ++-
 arch/arm/boot/dts/mt7623.dtsi                 | 138 +++++++++++++++++++++++---
 arch/arm/boot/dts/mt7623n-bananapi-bpi-r2.dts |  30 ++++++
 4 files changed, 190 insertions(+), 14 deletions(-)

Re: [GIT PULL] Mediatek: 32-bit DT update for v4.15

[Arnd Bergmann]

On Mon, Oct 23, 2017 at 12:06 AM, Matthias Brugger
<matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> ----------------------------------------------------------------
> - mt76233 add PCIe node

Could you clarify what the subnodes in the PCI node are for? It seems odd
to have "interrupt-map" properties in both the pcie controller and its child
nodes, and I want to ensure this is following the standard PCIe binding before
I pull it.

       Arnd
--
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

Re: [GIT PULL] Mediatek: 32-bit DT update for v4.15

[Ryder Lee]

Hi Arnd,

We have 3 root ports in MT7623, but this is a bug in this chip where the
HW designers wired the IRQs in a nonstandard way. We've tried to
statically assign the bus portion of the address part in the parent
interrupt-map before, but this approach cannot handle the case - if we
attach the device in random order.

Thanks.

On Mon, 2017-10-30 at 13:42 +0100, Arnd Bergmann wrote:
> On Mon, Oct 23, 2017 at 12:06 AM, Matthias Brugger
> <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >
> > ----------------------------------------------------------------
> > - mt76233 add PCIe node
> 
> Could you clarify what the subnodes in the PCI node are for? It seems odd
> to have "interrupt-map" properties in both the pcie controller and its child
> nodes, and I want to ensure this is following the standard PCIe binding before
> I pull it.
> 
>        Arnd
> 


--
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

Re: [GIT PULL] Mediatek: 32-bit DT update for v4.15

[Matthias Brugger]

Hi Arnd,

On 10/31/2017 05:19 AM, Ryder Lee wrote:
> Hi Arnd,
> 
> We have 3 root ports in MT7623, but this is a bug in this chip where the
> HW designers wired the IRQs in a nonstandard way. We've tried to
> statically assign the bus portion of the address part in the parent
> interrupt-map before, but this approach cannot handle the case - if we
> attach the device in random order.
> 

Ryder, please don't top post :)

> Thanks.
> 
> On Mon, 2017-10-30 at 13:42 +0100, Arnd Bergmann wrote:
>> On Mon, Oct 23, 2017 at 12:06 AM, Matthias Brugger
>> <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>
>>> ----------------------------------------------------------------
>>> - mt76233 add PCIe node
>>
>> Could you clarify what the subnodes in the PCI node are for? It seems odd
>> to have "interrupt-map" properties in both the pcie controller and its child
>> nodes, and I want to ensure this is following the standard PCIe binding before
>> I pull it.
>>

Arnd, I didn't found the pull request in your next/dt branch.
Is there more clarification needed from our side?

Regards,
Matthias
--
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

Re: [GIT PULL] Mediatek: 32-bit DT update for v4.15

[Arnd Bergmann]

On Thu, Nov 2, 2017 at 12:47 PM, Matthias Brugger
<matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Arnd,
>
> On 10/31/2017 05:19 AM, Ryder Lee wrote:
>> Hi Arnd,
>>
>> We have 3 root ports in MT7623, but this is a bug in this chip where the
>> HW designers wired the IRQs in a nonstandard way. We've tried to
>> statically assign the bus portion of the address part in the parent
>> interrupt-map before, but this approach cannot handle the case - if we
>> attach the device in random order.
>>
>
> Ryder, please don't top post :)
>
>> Thanks.
>>
>> On Mon, 2017-10-30 at 13:42 +0100, Arnd Bergmann wrote:
>>> On Mon, Oct 23, 2017 at 12:06 AM, Matthias Brugger
>>> <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>
>>>> ----------------------------------------------------------------
>>>> - mt76233 add PCIe node
>>>
>>> Could you clarify what the subnodes in the PCI node are for? It seems odd
>>> to have "interrupt-map" properties in both the pcie controller and its child
>>> nodes, and I want to ensure this is following the standard PCIe binding before
>>> I pull it.
>>>
>
> Arnd, I didn't found the pull request in your next/dt branch.
> Is there more clarification needed from our side?

I'm still unsure about it., this looks like exactly the thing that the top-level
interrupt-map is supposed to handle fine.

Can you send a pull request for the series without the pci child nodes for the
moment while we figure out what the exact problem is? We can take
whatever fix we come up with during the v4.15 -rc cycle then.

I would assume that either the parent interrupt-map is wrong, or something
broke the parser, but that it's not actually something that's wrong in the
hardware design in a way that we can't already handle in a standard way.

Ryder, can you be more specific how the interrupts are wired up?
Is there one IRQ per slot that is connected to all of IntA/IntB/IntC/IntD
and gets propagated through the bridges like that, or is it something else?

     Arnd
--
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

Re: [GIT PULL] Mediatek: 32-bit DT update for v4.15

[Matthias Brugger]

On 11/02/2017 05:05 PM, Arnd Bergmann wrote:
> On Thu, Nov 2, 2017 at 12:47 PM, Matthias Brugger
> <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Hi Arnd,
>>
>> On 10/31/2017 05:19 AM, Ryder Lee wrote:
>>> Hi Arnd,
>>>
>>> We have 3 root ports in MT7623, but this is a bug in this chip where the
>>> HW designers wired the IRQs in a nonstandard way. We've tried to
>>> statically assign the bus portion of the address part in the parent
>>> interrupt-map before, but this approach cannot handle the case - if we
>>> attach the device in random order.
>>>
>>
>> Ryder, please don't top post :)
>>
>>> Thanks.
>>>
>>> On Mon, 2017-10-30 at 13:42 +0100, Arnd Bergmann wrote:
>>>> On Mon, Oct 23, 2017 at 12:06 AM, Matthias Brugger
>>>> <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>>>
>>>>> ----------------------------------------------------------------
>>>>> - mt76233 add PCIe node
>>>>
>>>> Could you clarify what the subnodes in the PCI node are for? It seems odd
>>>> to have "interrupt-map" properties in both the pcie controller and its child
>>>> nodes, and I want to ensure this is following the standard PCIe binding before
>>>> I pull it.
>>>>
>>
>> Arnd, I didn't found the pull request in your next/dt branch.
>> Is there more clarification needed from our side?
> 
> I'm still unsure about it., this looks like exactly the thing that the top-level
> interrupt-map is supposed to handle fine.
> 
> Can you send a pull request for the series without the pci child nodes for the
> moment while we figure out what the exact problem is? We can take
> whatever fix we come up with during the v4.15 -rc cycle then.

Sure. I'll provide a new tag v4.14-next-dts32-2 without the patch.
I will send a pull request shortly.

Regards,
Matthias

> 
> I would assume that either the parent interrupt-map is wrong, or something
> broke the parser, but that it's not actually something that's wrong in the
> hardware design in a way that we can't already handle in a standard way.
> 
> Ryder, can you be more specific how the interrupts are wired up?
> Is there one IRQ per slot that is connected to all of IntA/IntB/IntC/IntD
> and gets propagated through the bridges like that, or is it something else?
> 
>      Arnd
> 
--
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

Re: [GIT PULL] Mediatek: 32-bit DT update for v4.15

[Arnd Bergmann]

On Thu, Nov 2, 2017 at 7:47 PM, Matthias Brugger <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>>>
>>> Arnd, I didn't found the pull request in your next/dt branch.
>>> Is there more clarification needed from our side?
>>
>> I'm still unsure about it., this looks like exactly the thing that the top-level
>> interrupt-map is supposed to handle fine.
>>
>> Can you send a pull request for the series without the pci child nodes for the
>> moment while we figure out what the exact problem is? We can take
>> whatever fix we come up with during the v4.15 -rc cycle then.
>
> Sure. I'll provide a new tag v4.14-next-dts32-2 without the patch.
> I will send a pull request shortly.

I was thinking with the patch but without the duplicated interrupt-map
properties
(or without the extra child nodes) but dropping the patch works for me too.

I was hoping that we could figure out whether it's a code bug that prevents
us from just parsing the parent interrupt-map correctly, or whether there is
something wrong with that map itself.

       Arnd
--
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

Re: [GIT PULL] Mediatek: 32-bit DT update for v4.15

[Ryder Lee]

Hi,

On Thu, 2017-11-02 at 17:05 +0100, Arnd Bergmann wrote:
> On Thu, Nov 2, 2017 at 12:47 PM, Matthias Brugger
> <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > Hi Arnd,
> >
> > On 10/31/2017 05:19 AM, Ryder Lee wrote:
> >> Hi Arnd,
> >>
> >> We have 3 root ports in MT7623, but this is a bug in this chip where the
> >> HW designers wired the IRQs in a nonstandard way. We've tried to
> >> statically assign the bus portion of the address part in the parent
> >> interrupt-map before, but this approach cannot handle the case - if we
> >> attach the device in random order.
> >>
> >
> > Ryder, please don't top post :)

Oh. My bad!

> >> Thanks.
> >>
> >> On Mon, 2017-10-30 at 13:42 +0100, Arnd Bergmann wrote:
> >>> On Mon, Oct 23, 2017 at 12:06 AM, Matthias Brugger
> >>> <matthias.bgg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>>>
> >>>> ----------------------------------------------------------------
> >>>> - mt76233 add PCIe node
> >>>
> >>> Could you clarify what the subnodes in the PCI node are for? It seems odd
> >>> to have "interrupt-map" properties in both the pcie controller and its child
> >>> nodes, and I want to ensure this is following the standard PCIe binding before
> >>> I pull it.
> >>>
> >
> > Arnd, I didn't found the pull request in your next/dt branch.
> > Is there more clarification needed from our side?
> 
> I'm still unsure about it., this looks like exactly the thing that the top-level
> interrupt-map is supposed to handle fine.
> 
> Can you send a pull request for the series without the pci child nodes for the
> moment while we figure out what the exact problem is? We can take
> whatever fix we come up with during the v4.15 -rc cycle then.
> 
> I would assume that either the parent interrupt-map is wrong, or something
> broke the parser, but that it's not actually something that's wrong in the
> hardware design in a way that we can't already handle in a standard way.
> 
> Ryder, can you be more specific how the interrupts are wired up?
> Is there one IRQ per slot that is connected to all of IntA/IntB/IntC/IntD
> and gets propagated through the bridges like that, or is it something else?

Yes, that's what I mean - we only have one IRQ which is connected to all
INTx for each slot, and I'm not sure if there is any better way to solve
this problem. 

>      Arnd
> 


--
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

Re: [GIT PULL] Mediatek: 32-bit DT update for v4.15

[Arnd Bergmann]

On Fri, Nov 3, 2017 at 2:37 AM, Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
>> Ryder, can you be more specific how the interrupts are wired up?
>> Is there one IRQ per slot that is connected to all of IntA/IntB/IntC/IntD
>> and gets propagated through the bridges like that, or is it something else?
>
> Yes, that's what I mean - we only have one IRQ which is connected to all
> INTx for each slot, and I'm not sure if there is any better way to solve
> this problem.

Ok. Your parent interrupt-map seems entirely reasonable for that case as far
as I can tell (maybe someone else can find a problem):

+               interrupt-map-mask = <0xf800 0 0 0>;
+               interrupt-map = <0x0000 0 0 0 &sysirq GIC_SPI 193
IRQ_TYPE_LEVEL_LOW>,
+                               <0x0800 0 0 0 &sysirq GIC_SPI 194
IRQ_TYPE_LEVEL_LOW>,
+                               <0x1000 0 0 0 &sysirq GIC_SPI 195
IRQ_TYPE_LEVEL_LOW>;

However, I can't find any other example of a machine using
interrupt-map-mask = <0xf800 0 0 0>; in the kernel tree, so it's possible
that we have a parser bug. We do have other boards that list all four
interrupts for each slot, and that seems to work fine. Can you try this
map in the parent while leaving out the chilren?

               interrupt-map-mask = <0xf800 0 0 7>;
               interrupt-map = <0x0000 0 0 1 &sysirq GIC_SPI 193
IRQ_TYPE_LEVEL_LOW>,
                                        <0x0000 0 0 2 &sysirq GIC_SPI
193 IRQ_TYPE_LEVEL_LOW>,
                                        <0x0000 0 0 3 &sysirq GIC_SPI
193 IRQ_TYPE_LEVEL_LOW>,
                                        <0x0000 0 0 4 &sysirq GIC_SPI
193 IRQ_TYPE_LEVEL_LOW>,
                                        <0x0800 0 0 1 &sysirq GIC_SPI
194 IRQ_TYPE_LEVEL_LOW>,
                                        <0x0800 0 0 2 &sysirq GIC_SPI
194 IRQ_TYPE_LEVEL_LOW>,
                                        <0x0800 0 0 3 &sysirq GIC_SPI
194 IRQ_TYPE_LEVEL_LOW>,
                                        <0x0800 0 0 4 &sysirq GIC_SPI
194 IRQ_TYPE_LEVEL_LOW>,
                                        <0x1000 0 0 1 &sysirq GIC_SPI
195 IRQ_TYPE_LEVEL_LOW>,
                                        <0x1000 0 0 2 &sysirq GIC_SPI
195 IRQ_TYPE_LEVEL_LOW>,
                                        <0x1000 0 0 3 &sysirq GIC_SPI
195 IRQ_TYPE_LEVEL_LOW>,
                                        <0x1000 0 0 4 &sysirq GIC_SPI
195 IRQ_TYPE_LEVEL_LOW>;

This should have the exact same effect as what you have in your tree,
but if that works,
we can merge that version and try to figure out why the kernel thinks
they are different.

      Arnd
--
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

Re: [GIT PULL] Mediatek: 32-bit DT update for v4.15

[Ryder Lee]

On Fri, 2017-11-03 at 10:40 +0100, Arnd Bergmann wrote:
> On Fri, Nov 3, 2017 at 2:37 AM, Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> >> Ryder, can you be more specific how the interrupts are wired up?
> >> Is there one IRQ per slot that is connected to all of IntA/IntB/IntC/IntD
> >> and gets propagated through the bridges like that, or is it something else?
> >
> > Yes, that's what I mean - we only have one IRQ which is connected to all
> > INTx for each slot, and I'm not sure if there is any better way to solve
> > this problem.
> 
> Ok. Your parent interrupt-map seems entirely reasonable for that case as far
> as I can tell (maybe someone else can find a problem):
> 
> +               interrupt-map-mask = <0xf800 0 0 0>;
> +               interrupt-map = <0x0000 0 0 0 &sysirq GIC_SPI 193
> IRQ_TYPE_LEVEL_LOW>,
> +                               <0x0800 0 0 0 &sysirq GIC_SPI 194
> IRQ_TYPE_LEVEL_LOW>,
> +                               <0x1000 0 0 0 &sysirq GIC_SPI 195
> IRQ_TYPE_LEVEL_LOW>;
> 
> However, I can't find any other example of a machine using
> interrupt-map-mask = <0xf800 0 0 0>; in the kernel tree, so it's possible
> that we have a parser bug. We do have other boards that list all four
> interrupts for each slot, and that seems to work fine. Can you try this
> map in the parent while leaving out the chilren?
> 
>                interrupt-map-mask = <0xf800 0 0 7>;
>                interrupt-map = <0x0000 0 0 1 &sysirq GIC_SPI 193
> IRQ_TYPE_LEVEL_LOW>,
>                                         <0x0000 0 0 2 &sysirq GIC_SPI
> 193 IRQ_TYPE_LEVEL_LOW>,
>                                         <0x0000 0 0 3 &sysirq GIC_SPI
> 193 IRQ_TYPE_LEVEL_LOW>,
>                                         <0x0000 0 0 4 &sysirq GIC_SPI
> 193 IRQ_TYPE_LEVEL_LOW>,
>                                         <0x0800 0 0 1 &sysirq GIC_SPI
> 194 IRQ_TYPE_LEVEL_LOW>,
>                                         <0x0800 0 0 2 &sysirq GIC_SPI
> 194 IRQ_TYPE_LEVEL_LOW>,
>                                         <0x0800 0 0 3 &sysirq GIC_SPI
> 194 IRQ_TYPE_LEVEL_LOW>,
>                                         <0x0800 0 0 4 &sysirq GIC_SPI
> 194 IRQ_TYPE_LEVEL_LOW>,
>                                         <0x1000 0 0 1 &sysirq GIC_SPI
> 195 IRQ_TYPE_LEVEL_LOW>,
>                                         <0x1000 0 0 2 &sysirq GIC_SPI
> 195 IRQ_TYPE_LEVEL_LOW>,
>                                         <0x1000 0 0 3 &sysirq GIC_SPI
> 195 IRQ_TYPE_LEVEL_LOW>,
>                                         <0x1000 0 0 4 &sysirq GIC_SPI
> 195 IRQ_TYPE_LEVEL_LOW>;
> 
> This should have the exact same effect as what you have in your tree,
> but if that works,
> we can merge that version and try to figure out why the kernel thinks
> they are different.

I've tried this approach by using the 4-ports network card and got the
result:
pcieport 0000:00:01.0: assign IRQ: got 220
pcieport 0000:00:01.0: assigning IRQ 220
pcieport 0000:00:01.0: enabling device (0140 -> 0142)
pcieport 0000:00:01.0: enabling bus mastering
pcieport 0000:00:01.0: Signaling PME with IRQ 220
....
igb 0000:01:00.0: assign IRQ: got 221
igb 0000:01:00.0: assigning IRQ 221
igb 0000:01:00.1: assign IRQ: got 221
igb 0000:01:00.1: assigning IRQ 221
igb 0000:01:00.2: assign IRQ: got 221
igb 0000:01:00.2: assigning IRQ 221
igb 0000:01:00.3: assign IRQ: got 221
igb 0000:01:00.3: assigning IRQ 221

I think slot 1 should share its IRQ (220) with every device in the
hierarchy below this root port.

Ryder.



--
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

Re: [GIT PULL] Mediatek: 32-bit DT update for v4.15

[Arnd Bergmann]

On Fri, Nov 3, 2017 at 12:52 PM, Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> On Fri, 2017-11-03 at 10:40 +0100, Arnd Bergmann wrote:
>> On Fri, Nov 3, 2017 at 2:37 AM, Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:

>> This should have the exact same effect as what you have in your tree,
>> but if that works,
>> we can merge that version and try to figure out why the kernel thinks
>> they are different.
>
> I've tried this approach by using the 4-ports network card and got the
> result:
> pcieport 0000:00:01.0: assign IRQ: got 220
> pcieport 0000:00:01.0: assigning IRQ 220
> pcieport 0000:00:01.0: enabling device (0140 -> 0142)
> pcieport 0000:00:01.0: enabling bus mastering
> pcieport 0000:00:01.0: Signaling PME with IRQ 220
> ....
> igb 0000:01:00.0: assign IRQ: got 221
> igb 0000:01:00.0: assigning IRQ 221
> igb 0000:01:00.1: assign IRQ: got 221
> igb 0000:01:00.1: assigning IRQ 221
> igb 0000:01:00.2: assign IRQ: got 221
> igb 0000:01:00.2: assigning IRQ 221
> igb 0000:01:00.3: assign IRQ: got 221
> igb 0000:01:00.3: assigning IRQ 221
>
> I think slot 1 should share its IRQ (220) with every device in the
> hierarchy below this root port.

Agreed, that is what I expected, too. I've looked at the of_pci_irq.c
in more detail but couldn't come up with what happened. Can you
try running with this patch or something like that to figure out where
it takes the wrong turn?

https://pastebin.com/MrMzwcmw

     Arnd
--
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

Re: [GIT PULL] Mediatek: 32-bit DT update for v4.15

[Ryder Lee]

On Fri, 2017-11-03 at 16:21 +0100, Arnd Bergmann wrote:
> On Fri, Nov 3, 2017 at 12:52 PM, Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> > On Fri, 2017-11-03 at 10:40 +0100, Arnd Bergmann wrote:
> >> On Fri, Nov 3, 2017 at 2:37 AM, Ryder Lee <ryder.lee-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> wrote:
> 
> >> This should have the exact same effect as what you have in your tree,
> >> but if that works,
> >> we can merge that version and try to figure out why the kernel thinks
> >> they are different.
> >
> > I've tried this approach by using the 4-ports network card and got the
> > result:
> > pcieport 0000:00:01.0: assign IRQ: got 220
> > pcieport 0000:00:01.0: assigning IRQ 220
> > pcieport 0000:00:01.0: enabling device (0140 -> 0142)
> > pcieport 0000:00:01.0: enabling bus mastering
> > pcieport 0000:00:01.0: Signaling PME with IRQ 220
> > ....
> > igb 0000:01:00.0: assign IRQ: got 221
> > igb 0000:01:00.0: assigning IRQ 221
> > igb 0000:01:00.1: assign IRQ: got 221
> > igb 0000:01:00.1: assigning IRQ 221
> > igb 0000:01:00.2: assign IRQ: got 221
> > igb 0000:01:00.2: assigning IRQ 221
> > igb 0000:01:00.3: assign IRQ: got 221
> > igb 0000:01:00.3: assigning IRQ 221
> >
> > I think slot 1 should share its IRQ (220) with every device in the
> > hierarchy below this root port.
> 
> Agreed, that is what I expected, too. I've looked at the of_pci_irq.c
> in more detail but couldn't come up with what happened. Can you
> try running with this patch or something like that to figure out where
> it takes the wrong turn?
> 
> https://pastebin.com/MrMzwcmw
> 

[    4.466527] pcieport 0000:00:01.0: swizzle found slot 1 pin 1
[    4.472252] pcieport 0000:00:01.0: direct parse found irq
3747445820:3747445848
[    4.479515] pcieport 0000:00:01.0: parse pin 1
[    4.483920] null ppdev, node dfff3744, pin 1
[    4.488151] null ppdev, parse node dfff3744 pin 1
[    4.492825] of_irq_parse_raw:  /pcie-controller@1a140000:00000001
[    4.498885] OF: of_irq_parse_raw: ipar=/pcie-controller@1a140000,
size=1
[    4.505530] OF:  -> addrsize=3
[    4.508569] OF:  -> match=0 (imaplen=92)
[    4.512481] OF:  -> newintsize=3, newaddrsize=0
[    4.516969] OF:  -> imaplen=88
[    4.520007] OF:  -> match=0 (imaplen=84)
[    4.523899] OF:  -> newintsize=3, newaddrsize=0
[    4.528400] OF:  -> imaplen=80
[    4.531425] OF:  -> match=0 (imaplen=76)
[    4.535317] OF:  -> newintsize=3, newaddrsize=0
[    4.539817] OF:  -> imaplen=72
[    4.542842] OF:  -> match=0 (imaplen=68)
[    4.546734] OF:  -> newintsize=3, newaddrsize=0
[    4.551234] OF:  -> imaplen=64
[    4.554259] OF:  -> match=1 (imaplen=60)
[    4.558151] OF:  -> newintsize=3, newaddrsize=0
[    4.562651] OF:  -> imaplen=56
[    4.565676] OF:  -> new parent: /interrupt-controller@10200100
[    4.571469] OF:  -> got it !
[    4.574322] null ppdev, parse node dfff3744 irq 0:194
[    4.579413] pcieport 0000:00:01.0: assign IRQ: got 220

[    4.613400] igb 0000:01:00.0: swizzle pin 1
[    4.617551] pcieport 0000:00:01.0: swizzle found slot 1 pin 1
[    4.623276] igb 0000:01:00.0: parse pin 1
[    4.627251] pcieport 0000:00:01.0: node dfff3e6c pin 1
[    4.632359] pcieport 0000:00:01.0: parse node dfff3e6c pin 1
[    4.637970]
of_irq_parse_raw:  /pcie-controller@1a140000/pcie@1,0:00000001
[    4.644807] OF: of_irq_parse_raw: ipar=/pcie-controller@1a140000,
size=1
[    4.651463] OF:  -> addrsize=3
[    4.654491] OF:  -> match=1 (imaplen=92)
[    4.658404] OF:  -> newintsize=3, newaddrsize=0
[    4.662893] OF:  -> imaplen=88
[    4.665917] OF:  -> new parent: /interrupt-controller@10200100
[    4.671711] OF:  -> got it !
[    4.674567] pcieport 0000:00:01.0: parse node dfff3e6c irq 0:193
[    4.680596] igb 0000:01:00.0: assign IRQ: got 221

[    4.806020] igb 0000:01:00.1: swizzle pin 2
[    4.810201] pcieport 0000:00:01.0: swizzle found slot 1 pin 2
[    4.815902] igb 0000:01:00.1: parse pin 2
[    4.819891] pcieport 0000:00:01.0: node dfff3e6c pin 2
[    4.824987] pcieport 0000:00:01.0: parse node dfff3e6c pin 2
[    4.830609]
of_irq_parse_raw:  /pcie-controller@1a140000/pcie@1,0:00000002
[    4.837437] OF: of_irq_parse_raw: ipar=/pcie-controller@1a140000,
size=1
[    4.844093] OF:  -> addrsize=3
[    4.847121] OF:  -> match=0 (imaplen=92)
[    4.851042] OF:  -> newintsize=3, newaddrsize=0
[    4.855532] OF:  -> imaplen=88
[    4.858570] OF:  -> match=1 (imaplen=84)
[    4.862463] OF:  -> newintsize=3, newaddrsize=0
[    4.866952] OF:  -> imaplen=80
[    4.869988] OF:  -> new parent: /interrupt-controller@10200100
[    4.875769] OF:  -> got it !
[    4.878637] pcieport 0000:00:01.0: parse node dfff3e6c irq 0:193
[    4.884601] igb 0000:01:00.1: assign IRQ: got 221


But if I add "interrupt-map properties" in both the pcie controller and
its child node:

[    4.466979] pcieport 0000:00:01.0: swizzle found slot 1 pin 1
[    4.472705] pcieport 0000:00:01.0: direct parse found irq
3747437628:3747437656
[    4.479969] pcieport 0000:00:01.0: parse pin 1
[    4.484372] null ppdev, node dfff3570, pin 1
[    4.488603] null ppdev, parse node dfff3570 pin 1
[    4.493278] of_irq_parse_raw:  /pcie-controller@1a140000:00000001
[    4.499338] OF: of_irq_parse_raw: ipar=/pcie-controller@1a140000,
size=1
[    4.505983] OF:  -> addrsize=3
[    4.509023] OF:  -> match=0 (imaplen=20)
[    4.512935] OF:  -> newintsize=3, newaddrsize=0
[    4.517424] OF:  -> imaplen=16
[    4.520464] OF:  -> match=1 (imaplen=12)
[    4.524356] OF:  -> newintsize=3, newaddrsize=0
[    4.528857] OF:  -> imaplen=8
[    4.531797] OF:  -> new parent: /interrupt-controller@10200100
[    4.537578] OF:  -> got it !
[    4.540442] null ppdev, parse node dfff3570 irq 0:194
[    4.545534] pcieport 0000:00:01.0: assign IRQ: got 220

[    4.579510] igb 0000:01:00.0: swizzle pin 1
[    4.583660] pcieport 0000:00:01.0: swizzle found slot 1 pin 1
[    4.589384] igb 0000:01:00.0: parse pin 1
[    4.593359] pcieport 0000:00:01.0: node dfff3d34 pin 1
[    4.598453] pcieport 0000:00:01.0: parse node dfff3d34 pin 1
[    4.604080]
of_irq_parse_raw:  /pcie-controller@1a140000/pcie@1,0:00000001
[    4.610918] OF: of_irq_parse_raw:
ipar=/pcie-controller@1a140000/pcie@1,0, size=1
[    4.618337] OF:  -> addrsize=3
[    4.621377] OF:  -> match=1 (imaplen=4)
[    4.625193] OF:  -> newintsize=3, newaddrsize=0
[    4.629695] OF:  -> imaplen=0
[    4.632633] OF:  -> new parent: /interrupt-controller@10200100
[    4.638414] OF:  -> got it !
[    4.641282] pcieport 0000:00:01.0: parse node dfff3d34 irq 0:194
[    4.647242] igb 0000:01:00.0: assign IRQ: got 220

[    4.772710] igb 0000:01:00.1: swizzle pin 2
[    4.776860] pcieport 0000:00:01.0: swizzle found slot 1 pin 2
[    4.782583] igb 0000:01:00.1: parse pin 2
[    4.786559] pcieport 0000:00:01.0: node dfff3d34 pin 2
[    4.791668] pcieport 0000:00:01.0: parse node dfff3d34 pin 2
[    4.834485] pcieport 0000:00:01.0: parse node dfff3d34 irq 0:194
[    4.840460] igb 0000:01:00.1: assign IRQ: got 220

I'm trying to understand what happened in the walking function.

Ryder

--
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

Re: [GIT PULL] Mediatek: 32-bit DT update for v4.15

[Ryder Lee]

On Fri, 2017-11-03 at 16:21 +0100, Arnd Bergmann wrote:
> On Fri, Nov 3, 2017 at 12:52 PM, Ryder Lee <ryder.lee@mediatek.com> wrote:
> > On Fri, 2017-11-03 at 10:40 +0100, Arnd Bergmann wrote:
> >> On Fri, Nov 3, 2017 at 2:37 AM, Ryder Lee <ryder.lee@mediatek.com> wrote:
> 
> >> This should have the exact same effect as what you have in your tree,
> >> but if that works,
> >> we can merge that version and try to figure out why the kernel thinks
> >> they are different.
> >
> > I've tried this approach by using the 4-ports network card and got the
> > result:
> > pcieport 0000:00:01.0: assign IRQ: got 220
> > pcieport 0000:00:01.0: assigning IRQ 220
> > pcieport 0000:00:01.0: enabling device (0140 -> 0142)
> > pcieport 0000:00:01.0: enabling bus mastering
> > pcieport 0000:00:01.0: Signaling PME with IRQ 220
> > ....
> > igb 0000:01:00.0: assign IRQ: got 221
> > igb 0000:01:00.0: assigning IRQ 221
> > igb 0000:01:00.1: assign IRQ: got 221
> > igb 0000:01:00.1: assigning IRQ 221
> > igb 0000:01:00.2: assign IRQ: got 221
> > igb 0000:01:00.2: assigning IRQ 221
> > igb 0000:01:00.3: assign IRQ: got 221
> > igb 0000:01:00.3: assigning IRQ 221
> >
> > I think slot 1 should share its IRQ (220) with every device in the
> > hierarchy below this root port.
> 
> Agreed, that is what I expected, too. I've looked at the of_pci_irq.c
> in more detail but couldn't come up with what happened. Can you
> try running with this patch or something like that to figure out where
> it takes the wrong turn?
> 
> https://pastebin.com/MrMzwcmw
> 

Hi Arnd,

I guess the problem is the first device attached to downstream side of a
link must be device 0 - it means that the 'slot' portion of pdev->devfn
should be 0 when PCIe enumeration. That's why we always mismatch to
interrupt-map = <0x0000 0 0 ... GIC_SPI 193 ...>

However, if we want to list all four interrupts for each slot, that
could work if fallback to device tree parsing:

https://elixir.free-electrons.com/linux/v4.15-rc7/source/drivers/of/of_pci_irq.c#L89

Then replace 'pdev->devfn' with 'of_pci_get_devfn(out_irq->np)'

But I still not sure the proper way to solve it.

Ryder.