subject:"Re\: \[PATCH v3 2\/2\] dt\-bindings\: pcie\: Add documentation for Mediatek PCIe"

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-13 Thread Ryder Lee


Hi Arnd,

Sorry to bother you again.

On Thu, 2017-05-11 at 20:11 +0800, Ryder Lee wrote:
> > interrupt-map-mask = <0xff800 0 0 0>;
> > interrupt-map = <0x 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
> >  <0x0800 0 0 0  GIC_SPI 194 IRQ_TYPE_NONE>,
> >  <0x1000 0 0 0  GIC_SPI 195 IRQ_TYPE_NONE>,
> > 
> >  /* workaround here*/
> >  <0x1 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
> >  <0x2 0 0 0  GIC_SPI 194 IRQ_TYPE_NONE>,
> >  <0x3 0 0 0  GIC_SPI 195 IRQ_TYPE_NONE>;
> > 
> > It works well. But how could we handle the situation if root port0
> > status = "disabled" ? I think we cannot assign child bus number
> > dynamically from binding.
> 
> That is to say, we route it statically if port0 (or port1) is
> unavailable. The PCI child bus enumeration should look something like
> this:
> 
>  pci :00:01.0: fixup irq: got 224
>  pci :00:01.0: assigning IRQ 224
>  pci :00:02.0: fixup irq: got 225
>  pci :00:02.0: assigning IRQ 225
>  
>  Go wrong here! IRQ 223/224 should be assigned to the devices behind
> port0 and port1.
>  pci :01:00.0: fixup irq: got 223
>  pci :01:00.0: assigning IRQ 223
>  pci :02:00.0: fixup irq: got 224
>  pci :02:00.0: assigning IRQ 224

What I thought was wrong. I have misunderstood something in previous
discussion. Actually it could work for the situation that I mentioned
before. However, I'm not sure whether this is a proper representation
you want to see.

> > > >> On a related note, I see that you still list
> > > >>
> > > >> > +- interrupts: Three interrupt outputs of the controller. Must 
> > > >> > contain an
> > > >> > +  entry for each entry in the interrupt-names property.
> > > >> > +- interrupt-names: Must include the following names
> > > >> > +  - "pcie-int0"
> > > >> > +  - "pcie-int1"
> > > >> > +  - "pcie-int2"
> > > >>
> > > >> This seems to be an artifact from the older version and should be
> > > >> removed as the driver correctly ignores the properties now.
> > > >
> > > > Actually, everything works fine without these properties however when it
> > > > loads we see a few weird error message:
> > > >
> > > > pcieport :00:01.0: Signaling PME with IRQ 232
> > > > pcieport :00:02.0: enabling device (0140 -> 0142)
> > > > pcieport :00:02.0: enabling bus mastering
> > > > irq 232: nobody cared (try booting with the "irqpoll" option)
> > > > ...
> > > > [] (pcie_pme_probe) from [] (pcie_port_probe_service
> > > > +0x44/0x6c)
> > > > (pcie_port_probe_service) from [] (driver_probe_device
> > > > +0x280/0x470)
> > > > ...
> > > > (pcie_port_device_register) from [] (pcie_portdrv_probe
> > > > +0x3c/0xb4)
> > > > (pcie_portdrv_probe) from [] (pci_device_probe+0x98/0xfc)
> > > > (pci_device_probe) from [] (driver_probe_device+0x280/0x470)
> > > > handlers:
> > > > [] pcie_pme_irq
> > > > Disabling IRQ #233
> > > >
> > > > I haven't dig it out yet, but just keep them here to solve that.
> > > 
> > > Something is going very wrong if adding the properties helps. I can't
> > > think of what that is, but we have to find out before the binding can
> > > be merged.
> > 
> > Not really understand PME service. But I will find the reason here.
> 
> I have do some test here. PME needs port IRQs, which interrupt type was
> set correctly(IRQ_TYPE_LEVEL_LOW). But we cannot set it from
> interrupt-map, according to gic_set_type() /* SPIs have restrictions on
> the supported types */ .
> 
> So we need to add additional interrupt properties.

I could use iPerf to test my WLAN card normally. But just ignore this
exception message. I would definitely appreciate if someone could give
me some hint on how to properly solve it. 

Thanks a lot.

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-13 Thread Ryder Lee


Hi Arnd,

Sorry to bother you again.

On Thu, 2017-05-11 at 20:11 +0800, Ryder Lee wrote:
> > interrupt-map-mask = <0xff800 0 0 0>;
> > interrupt-map = <0x 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
> >  <0x0800 0 0 0  GIC_SPI 194 IRQ_TYPE_NONE>,
> >  <0x1000 0 0 0  GIC_SPI 195 IRQ_TYPE_NONE>,
> > 
> >  /* workaround here*/
> >  <0x1 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
> >  <0x2 0 0 0  GIC_SPI 194 IRQ_TYPE_NONE>,
> >  <0x3 0 0 0  GIC_SPI 195 IRQ_TYPE_NONE>;
> > 
> > It works well. But how could we handle the situation if root port0
> > status = "disabled" ? I think we cannot assign child bus number
> > dynamically from binding.
> 
> That is to say, we route it statically if port0 (or port1) is
> unavailable. The PCI child bus enumeration should look something like
> this:
> 
>  pci :00:01.0: fixup irq: got 224
>  pci :00:01.0: assigning IRQ 224
>  pci :00:02.0: fixup irq: got 225
>  pci :00:02.0: assigning IRQ 225
>  
>  Go wrong here! IRQ 223/224 should be assigned to the devices behind
> port0 and port1.
>  pci :01:00.0: fixup irq: got 223
>  pci :01:00.0: assigning IRQ 223
>  pci :02:00.0: fixup irq: got 224
>  pci :02:00.0: assigning IRQ 224

What I thought was wrong. I have misunderstood something in previous
discussion. Actually it could work for the situation that I mentioned
before. However, I'm not sure whether this is a proper representation
you want to see.

> > > >> On a related note, I see that you still list
> > > >>
> > > >> > +- interrupts: Three interrupt outputs of the controller. Must 
> > > >> > contain an
> > > >> > +  entry for each entry in the interrupt-names property.
> > > >> > +- interrupt-names: Must include the following names
> > > >> > +  - "pcie-int0"
> > > >> > +  - "pcie-int1"
> > > >> > +  - "pcie-int2"
> > > >>
> > > >> This seems to be an artifact from the older version and should be
> > > >> removed as the driver correctly ignores the properties now.
> > > >
> > > > Actually, everything works fine without these properties however when it
> > > > loads we see a few weird error message:
> > > >
> > > > pcieport :00:01.0: Signaling PME with IRQ 232
> > > > pcieport :00:02.0: enabling device (0140 -> 0142)
> > > > pcieport :00:02.0: enabling bus mastering
> > > > irq 232: nobody cared (try booting with the "irqpoll" option)
> > > > ...
> > > > [] (pcie_pme_probe) from [] (pcie_port_probe_service
> > > > +0x44/0x6c)
> > > > (pcie_port_probe_service) from [] (driver_probe_device
> > > > +0x280/0x470)
> > > > ...
> > > > (pcie_port_device_register) from [] (pcie_portdrv_probe
> > > > +0x3c/0xb4)
> > > > (pcie_portdrv_probe) from [] (pci_device_probe+0x98/0xfc)
> > > > (pci_device_probe) from [] (driver_probe_device+0x280/0x470)
> > > > handlers:
> > > > [] pcie_pme_irq
> > > > Disabling IRQ #233
> > > >
> > > > I haven't dig it out yet, but just keep them here to solve that.
> > > 
> > > Something is going very wrong if adding the properties helps. I can't
> > > think of what that is, but we have to find out before the binding can
> > > be merged.
> > 
> > Not really understand PME service. But I will find the reason here.
> 
> I have do some test here. PME needs port IRQs, which interrupt type was
> set correctly(IRQ_TYPE_LEVEL_LOW). But we cannot set it from
> interrupt-map, according to gic_set_type() /* SPIs have restrictions on
> the supported types */ .
> 
> So we need to add additional interrupt properties.

I could use iPerf to test my WLAN card normally. But just ignore this
exception message. I would definitely appreciate if someone could give
me some hint on how to properly solve it. 

Thanks a lot.

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-11 Thread Ryder Lee

Hi Arnd,

I want to further explain what I have discussed in previous mail.


On Thu, 2017-05-11 at 17:08 +0800, Ryder Lee wrote:
> On Thu, 2017-05-11 at 09:17 +0200, Arnd Bergmann wrote:
> > On Thu, May 11, 2017 at 4:44 AM, Ryder Lee  wrote:
> > > On Wed, 2017-05-10 at 12:01 +0200, Arnd Bergmann wrote:
> > >> On Wed, May 10, 2017 at 11:31 AM, Ryder Lee  
> > >> wrote:
> > >> > On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote:
> > 
> > >> >> > +Required properties:
> > >> >> > +- device_type: Must be "pci"
> > >> >> > +- assigned-addresses: Address and size of the port configuration 
> > >> >> > registers
> > >> >> > +- reg: Only the first four bytes are used to refer to the correct 
> > >> >> > bus number
> > >> >> > +  and device number.
> > >> >> > +- #address-cells: Must be 3
> > >> >> > +- #size-cells: Must be 2
> > >> >> > +- #interrupt-cells: Must be 1
> > >> >> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping 
> > >> >> > properties
> > >> >> > +  Please refer to the standard PCI bus binding document for a more 
> > >> >> > detailed
> > >> >> > +  explanation.
> > >> >>
> > >> >> Child nodes do not normally have interrupt-map properties. Isn't this
> > >> >> already covered by the interrupt-map in the parent?
> > >> >>
> > >> >
> > >> > I have one Intel 4 port ethernet card(:00:01) and MTK WLAN card
> > >> > (:00:02), probe message looks good to me.
> > >> >
> > >> > pci :00:01.0: fixup irq: got 224
> > >> > pci :00:01.0: assigning IRQ 224
> > >> > pci :00:02.0: fixup irq: got 225
> > >> > pci :00:02.0: assigning IRQ 225
> > >> >
> > >> > pci :01:00.0: fixup irq: got 224
> > >> > pci :01:00.0: assigning IRQ 224
> > >> > pci :01:00.1: fixup irq: got 224
> > >> > pci :01:00.1: assigning IRQ 224
> > >> > pci :01:00.2: fixup irq: got 224
> > >> > pci :01:00.2: assigning IRQ 224
> > >> > pci :01:00.3: fixup irq: got 224
> > >> > pci :01:00.3: assigning IRQ 224
> > >> >
> > >> > pci :02:00.0: fixup irq: got 225
> > >> > pci :02:00.0: assigning IRQ 225
> > >> >
> > >> >
> > >> > But child nodes without interrupt-map properties:
> > >> > It seems incorrect.
> > >> >
> > >> > pci :00:01.0: fixup irq: got 224
> > >> > pci :00:01.0: assigning IRQ 224
> > >> > pci :00:02.0: fixup irq: got 225
> > >> > pci :00:02.0: assigning IRQ 225
> > >> >
> > >> > pci :01:00.0: fixup irq: got 223
> > >> > pci :01:00.0: assigning IRQ 223
> > >>
> > >> Not entirely sure what happens here, but I guess the problem
> > >> is that the 'reg' portion of the parent interrupt-map refers to
> > >> the port devices, not the devices attached the devices behind
> > >> them.
> > >
> > > I agree with you. That's why I need additional interrupt-map properties
> > > to resolve IRQ correctly for the devices behind root ports.
> > >
> > > Not sure whether other platforms have similar case like me here.
> > 
> > I think it's just a bug in this specific chip where the HW designers
> > wired the IRQs in a nonstandard way.
> > 
> > However, you really should not need the interrupt-map properties
> > in the child nodes, just change the address part in the parent
> > interrupt-map. Specifically, the 'bus' portion of the device address
> > in the interrupt-map would have to be nonzero to refer to
> > child devices.
> 
> This is what I modify for the parent node and remove interrupt-map
> properties from child..
> 
> interrupt-map-mask = <0xff800 0 0 0>;
> interrupt-map = <0x 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
><0x0800 0 0 0  GIC_SPI 194 IRQ_TYPE_NONE>,
><0x1000 0 0 0  GIC_SPI 195 IRQ_TYPE_NONE>,
> 
>/* workaround here*/
><0x1 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
><0x2 0 0 0  GIC_SPI 194 IRQ_TYPE_NONE>,
><0x3 0 0 0  GIC_SPI 195 IRQ_TYPE_NONE>;
> 
> It works well. But how could we handle the situation if root port0
> status = "disabled" ? I think we cannot assign child bus number
> dynamically from binding.

That is to say, we route it statically if port0 (or port1) is
unavailable. The PCI child bus enumeration should look something like
this:

 pci :00:01.0: fixup irq: got 224
 pci :00:01.0: assigning IRQ 224
 pci :00:02.0: fixup irq: got 225
 pci :00:02.0: assigning IRQ 225
 
 Go wrong here! IRQ 223/224 should be assigned to the devices behind
port0 and port1.
 pci :01:00.0: fixup irq: got 223
 pci :01:00.0: assigning IRQ 223
 pci :02:00.0: fixup irq: got 224
 pci :02:00.0: assigning IRQ 224

> > >> On a related note, I see that you still list
> > >>
> > >> > +- interrupts: Three interrupt outputs of the controller. Must contain 
> > >> > an
> > >> > +  entry for each entry in the interrupt-names property.
> > >> > +- interrupt-names: Must include the following names
> > >> > +  - "pcie-int0"
> > >> > +  - "pcie-int1"
> > >> > +  - "pcie-int2"
> > >>
> >

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-11 Thread Ryder Lee

Hi Arnd,

I want to further explain what I have discussed in previous mail.


On Thu, 2017-05-11 at 17:08 +0800, Ryder Lee wrote:
> On Thu, 2017-05-11 at 09:17 +0200, Arnd Bergmann wrote:
> > On Thu, May 11, 2017 at 4:44 AM, Ryder Lee  wrote:
> > > On Wed, 2017-05-10 at 12:01 +0200, Arnd Bergmann wrote:
> > >> On Wed, May 10, 2017 at 11:31 AM, Ryder Lee  
> > >> wrote:
> > >> > On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote:
> > 
> > >> >> > +Required properties:
> > >> >> > +- device_type: Must be "pci"
> > >> >> > +- assigned-addresses: Address and size of the port configuration 
> > >> >> > registers
> > >> >> > +- reg: Only the first four bytes are used to refer to the correct 
> > >> >> > bus number
> > >> >> > +  and device number.
> > >> >> > +- #address-cells: Must be 3
> > >> >> > +- #size-cells: Must be 2
> > >> >> > +- #interrupt-cells: Must be 1
> > >> >> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping 
> > >> >> > properties
> > >> >> > +  Please refer to the standard PCI bus binding document for a more 
> > >> >> > detailed
> > >> >> > +  explanation.
> > >> >>
> > >> >> Child nodes do not normally have interrupt-map properties. Isn't this
> > >> >> already covered by the interrupt-map in the parent?
> > >> >>
> > >> >
> > >> > I have one Intel 4 port ethernet card(:00:01) and MTK WLAN card
> > >> > (:00:02), probe message looks good to me.
> > >> >
> > >> > pci :00:01.0: fixup irq: got 224
> > >> > pci :00:01.0: assigning IRQ 224
> > >> > pci :00:02.0: fixup irq: got 225
> > >> > pci :00:02.0: assigning IRQ 225
> > >> >
> > >> > pci :01:00.0: fixup irq: got 224
> > >> > pci :01:00.0: assigning IRQ 224
> > >> > pci :01:00.1: fixup irq: got 224
> > >> > pci :01:00.1: assigning IRQ 224
> > >> > pci :01:00.2: fixup irq: got 224
> > >> > pci :01:00.2: assigning IRQ 224
> > >> > pci :01:00.3: fixup irq: got 224
> > >> > pci :01:00.3: assigning IRQ 224
> > >> >
> > >> > pci :02:00.0: fixup irq: got 225
> > >> > pci :02:00.0: assigning IRQ 225
> > >> >
> > >> >
> > >> > But child nodes without interrupt-map properties:
> > >> > It seems incorrect.
> > >> >
> > >> > pci :00:01.0: fixup irq: got 224
> > >> > pci :00:01.0: assigning IRQ 224
> > >> > pci :00:02.0: fixup irq: got 225
> > >> > pci :00:02.0: assigning IRQ 225
> > >> >
> > >> > pci :01:00.0: fixup irq: got 223
> > >> > pci :01:00.0: assigning IRQ 223
> > >>
> > >> Not entirely sure what happens here, but I guess the problem
> > >> is that the 'reg' portion of the parent interrupt-map refers to
> > >> the port devices, not the devices attached the devices behind
> > >> them.
> > >
> > > I agree with you. That's why I need additional interrupt-map properties
> > > to resolve IRQ correctly for the devices behind root ports.
> > >
> > > Not sure whether other platforms have similar case like me here.
> > 
> > I think it's just a bug in this specific chip where the HW designers
> > wired the IRQs in a nonstandard way.
> > 
> > However, you really should not need the interrupt-map properties
> > in the child nodes, just change the address part in the parent
> > interrupt-map. Specifically, the 'bus' portion of the device address
> > in the interrupt-map would have to be nonzero to refer to
> > child devices.
> 
> This is what I modify for the parent node and remove interrupt-map
> properties from child..
> 
> interrupt-map-mask = <0xff800 0 0 0>;
> interrupt-map = <0x 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
><0x0800 0 0 0  GIC_SPI 194 IRQ_TYPE_NONE>,
><0x1000 0 0 0  GIC_SPI 195 IRQ_TYPE_NONE>,
> 
>/* workaround here*/
><0x1 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
><0x2 0 0 0  GIC_SPI 194 IRQ_TYPE_NONE>,
><0x3 0 0 0  GIC_SPI 195 IRQ_TYPE_NONE>;
> 
> It works well. But how could we handle the situation if root port0
> status = "disabled" ? I think we cannot assign child bus number
> dynamically from binding.

That is to say, we route it statically if port0 (or port1) is
unavailable. The PCI child bus enumeration should look something like
this:

 pci :00:01.0: fixup irq: got 224
 pci :00:01.0: assigning IRQ 224
 pci :00:02.0: fixup irq: got 225
 pci :00:02.0: assigning IRQ 225
 
 Go wrong here! IRQ 223/224 should be assigned to the devices behind
port0 and port1.
 pci :01:00.0: fixup irq: got 223
 pci :01:00.0: assigning IRQ 223
 pci :02:00.0: fixup irq: got 224
 pci :02:00.0: assigning IRQ 224

> > >> On a related note, I see that you still list
> > >>
> > >> > +- interrupts: Three interrupt outputs of the controller. Must contain 
> > >> > an
> > >> > +  entry for each entry in the interrupt-names property.
> > >> > +- interrupt-names: Must include the following names
> > >> > +  - "pcie-int0"
> > >> > +  - "pcie-int1"
> > >> > +  - "pcie-int2"
> > >>
> > >> This seems to be an artifact from the older

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-11 Thread Ryder Lee

On Thu, 2017-05-11 at 09:17 +0200, Arnd Bergmann wrote:
> On Thu, May 11, 2017 at 4:44 AM, Ryder Lee  wrote:
> > On Wed, 2017-05-10 at 12:01 +0200, Arnd Bergmann wrote:
> >> On Wed, May 10, 2017 at 11:31 AM, Ryder Lee  wrote:
> >> > On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote:
> 
> >> >> > +Required properties:
> >> >> > +- device_type: Must be "pci"
> >> >> > +- assigned-addresses: Address and size of the port configuration 
> >> >> > registers
> >> >> > +- reg: Only the first four bytes are used to refer to the correct 
> >> >> > bus number
> >> >> > +  and device number.
> >> >> > +- #address-cells: Must be 3
> >> >> > +- #size-cells: Must be 2
> >> >> > +- #interrupt-cells: Must be 1
> >> >> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping 
> >> >> > properties
> >> >> > +  Please refer to the standard PCI bus binding document for a more 
> >> >> > detailed
> >> >> > +  explanation.
> >> >>
> >> >> Child nodes do not normally have interrupt-map properties. Isn't this
> >> >> already covered by the interrupt-map in the parent?
> >> >>
> >> >
> >> > I have one Intel 4 port ethernet card(:00:01) and MTK WLAN card
> >> > (:00:02), probe message looks good to me.
> >> >
> >> > pci :00:01.0: fixup irq: got 224
> >> > pci :00:01.0: assigning IRQ 224
> >> > pci :00:02.0: fixup irq: got 225
> >> > pci :00:02.0: assigning IRQ 225
> >> >
> >> > pci :01:00.0: fixup irq: got 224
> >> > pci :01:00.0: assigning IRQ 224
> >> > pci :01:00.1: fixup irq: got 224
> >> > pci :01:00.1: assigning IRQ 224
> >> > pci :01:00.2: fixup irq: got 224
> >> > pci :01:00.2: assigning IRQ 224
> >> > pci :01:00.3: fixup irq: got 224
> >> > pci :01:00.3: assigning IRQ 224
> >> >
> >> > pci :02:00.0: fixup irq: got 225
> >> > pci :02:00.0: assigning IRQ 225
> >> >
> >> >
> >> > But child nodes without interrupt-map properties:
> >> > It seems incorrect.
> >> >
> >> > pci :00:01.0: fixup irq: got 224
> >> > pci :00:01.0: assigning IRQ 224
> >> > pci :00:02.0: fixup irq: got 225
> >> > pci :00:02.0: assigning IRQ 225
> >> >
> >> > pci :01:00.0: fixup irq: got 223
> >> > pci :01:00.0: assigning IRQ 223
> >>
> >> Not entirely sure what happens here, but I guess the problem
> >> is that the 'reg' portion of the parent interrupt-map refers to
> >> the port devices, not the devices attached the devices behind
> >> them.
> >
> > I agree with you. That's why I need additional interrupt-map properties
> > to resolve IRQ correctly for the devices behind root ports.
> >
> > Not sure whether other platforms have similar case like me here.
> 
> I think it's just a bug in this specific chip where the HW designers
> wired the IRQs in a nonstandard way.
> 
> However, you really should not need the interrupt-map properties
> in the child nodes, just change the address part in the parent
> interrupt-map. Specifically, the 'bus' portion of the device address
> in the interrupt-map would have to be nonzero to refer to
> child devices.

This is what I modify for the parent node and remove interrupt-map
properties from child..

interrupt-map-mask = <0xff800 0 0 0>;
interrupt-map = <0x 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
 <0x0800 0 0 0  GIC_SPI 194 IRQ_TYPE_NONE>,
 <0x1000 0 0 0  GIC_SPI 195 IRQ_TYPE_NONE>,

 /* workaround here*/
 <0x1 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
 <0x2 0 0 0  GIC_SPI 194 IRQ_TYPE_NONE>,
 <0x3 0 0 0  GIC_SPI 195 IRQ_TYPE_NONE>;

It works well. But how could we handle the situation if root port0
status = "disabled" ? I think we cannot assign child bus number
dynamically from binding.

> >> On a related note, I see that you still list
> >>
> >> > +- interrupts: Three interrupt outputs of the controller. Must contain an
> >> > +  entry for each entry in the interrupt-names property.
> >> > +- interrupt-names: Must include the following names
> >> > +  - "pcie-int0"
> >> > +  - "pcie-int1"
> >> > +  - "pcie-int2"
> >>
> >> This seems to be an artifact from the older version and should be
> >> removed as the driver correctly ignores the properties now.
> >
> > Actually, everything works fine without these properties however when it
> > loads we see a few weird error message:
> >
> > pcieport :00:01.0: Signaling PME with IRQ 232
> > pcieport :00:02.0: enabling device (0140 -> 0142)
> > pcieport :00:02.0: enabling bus mastering
> > irq 232: nobody cared (try booting with the "irqpoll" option)
> > ...
> > [] (pcie_pme_probe) from [] (pcie_port_probe_service
> > +0x44/0x6c)
> > (pcie_port_probe_service) from [] (driver_probe_device
> > +0x280/0x470)
> > ...
> > (pcie_port_device_register) from [] (pcie_portdrv_probe
> > +0x3c/0xb4)
> > (pcie_portdrv_probe) from [] (pci_device_probe+0x98/0xfc)
> > (pci_device_probe) from [] (driver_probe_device+0x280/0x470)
> >

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-11 Thread Ryder Lee

On Thu, 2017-05-11 at 09:17 +0200, Arnd Bergmann wrote:
> On Thu, May 11, 2017 at 4:44 AM, Ryder Lee  wrote:
> > On Wed, 2017-05-10 at 12:01 +0200, Arnd Bergmann wrote:
> >> On Wed, May 10, 2017 at 11:31 AM, Ryder Lee  wrote:
> >> > On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote:
> 
> >> >> > +Required properties:
> >> >> > +- device_type: Must be "pci"
> >> >> > +- assigned-addresses: Address and size of the port configuration 
> >> >> > registers
> >> >> > +- reg: Only the first four bytes are used to refer to the correct 
> >> >> > bus number
> >> >> > +  and device number.
> >> >> > +- #address-cells: Must be 3
> >> >> > +- #size-cells: Must be 2
> >> >> > +- #interrupt-cells: Must be 1
> >> >> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping 
> >> >> > properties
> >> >> > +  Please refer to the standard PCI bus binding document for a more 
> >> >> > detailed
> >> >> > +  explanation.
> >> >>
> >> >> Child nodes do not normally have interrupt-map properties. Isn't this
> >> >> already covered by the interrupt-map in the parent?
> >> >>
> >> >
> >> > I have one Intel 4 port ethernet card(:00:01) and MTK WLAN card
> >> > (:00:02), probe message looks good to me.
> >> >
> >> > pci :00:01.0: fixup irq: got 224
> >> > pci :00:01.0: assigning IRQ 224
> >> > pci :00:02.0: fixup irq: got 225
> >> > pci :00:02.0: assigning IRQ 225
> >> >
> >> > pci :01:00.0: fixup irq: got 224
> >> > pci :01:00.0: assigning IRQ 224
> >> > pci :01:00.1: fixup irq: got 224
> >> > pci :01:00.1: assigning IRQ 224
> >> > pci :01:00.2: fixup irq: got 224
> >> > pci :01:00.2: assigning IRQ 224
> >> > pci :01:00.3: fixup irq: got 224
> >> > pci :01:00.3: assigning IRQ 224
> >> >
> >> > pci :02:00.0: fixup irq: got 225
> >> > pci :02:00.0: assigning IRQ 225
> >> >
> >> >
> >> > But child nodes without interrupt-map properties:
> >> > It seems incorrect.
> >> >
> >> > pci :00:01.0: fixup irq: got 224
> >> > pci :00:01.0: assigning IRQ 224
> >> > pci :00:02.0: fixup irq: got 225
> >> > pci :00:02.0: assigning IRQ 225
> >> >
> >> > pci :01:00.0: fixup irq: got 223
> >> > pci :01:00.0: assigning IRQ 223
> >>
> >> Not entirely sure what happens here, but I guess the problem
> >> is that the 'reg' portion of the parent interrupt-map refers to
> >> the port devices, not the devices attached the devices behind
> >> them.
> >
> > I agree with you. That's why I need additional interrupt-map properties
> > to resolve IRQ correctly for the devices behind root ports.
> >
> > Not sure whether other platforms have similar case like me here.
> 
> I think it's just a bug in this specific chip where the HW designers
> wired the IRQs in a nonstandard way.
> 
> However, you really should not need the interrupt-map properties
> in the child nodes, just change the address part in the parent
> interrupt-map. Specifically, the 'bus' portion of the device address
> in the interrupt-map would have to be nonzero to refer to
> child devices.

This is what I modify for the parent node and remove interrupt-map
properties from child..

interrupt-map-mask = <0xff800 0 0 0>;
interrupt-map = <0x 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
 <0x0800 0 0 0  GIC_SPI 194 IRQ_TYPE_NONE>,
 <0x1000 0 0 0  GIC_SPI 195 IRQ_TYPE_NONE>,

 /* workaround here*/
 <0x1 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
 <0x2 0 0 0  GIC_SPI 194 IRQ_TYPE_NONE>,
 <0x3 0 0 0  GIC_SPI 195 IRQ_TYPE_NONE>;

It works well. But how could we handle the situation if root port0
status = "disabled" ? I think we cannot assign child bus number
dynamically from binding.

> >> On a related note, I see that you still list
> >>
> >> > +- interrupts: Three interrupt outputs of the controller. Must contain an
> >> > +  entry for each entry in the interrupt-names property.
> >> > +- interrupt-names: Must include the following names
> >> > +  - "pcie-int0"
> >> > +  - "pcie-int1"
> >> > +  - "pcie-int2"
> >>
> >> This seems to be an artifact from the older version and should be
> >> removed as the driver correctly ignores the properties now.
> >
> > Actually, everything works fine without these properties however when it
> > loads we see a few weird error message:
> >
> > pcieport :00:01.0: Signaling PME with IRQ 232
> > pcieport :00:02.0: enabling device (0140 -> 0142)
> > pcieport :00:02.0: enabling bus mastering
> > irq 232: nobody cared (try booting with the "irqpoll" option)
> > ...
> > [] (pcie_pme_probe) from [] (pcie_port_probe_service
> > +0x44/0x6c)
> > (pcie_port_probe_service) from [] (driver_probe_device
> > +0x280/0x470)
> > ...
> > (pcie_port_device_register) from [] (pcie_portdrv_probe
> > +0x3c/0xb4)
> > (pcie_portdrv_probe) from [] (pci_device_probe+0x98/0xfc)
> > (pci_device_probe) from [] (driver_probe_device+0x280/0x470)
> > handlers:
> > [] pcie_pme_irq
> > Disabling IRQ #233
>

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-11 Thread Arnd Bergmann

On Thu, May 11, 2017 at 4:44 AM, Ryder Lee  wrote:
> On Wed, 2017-05-10 at 12:01 +0200, Arnd Bergmann wrote:
>> On Wed, May 10, 2017 at 11:31 AM, Ryder Lee  wrote:
>> > On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote:

>> >> > +Required properties:
>> >> > +- device_type: Must be "pci"
>> >> > +- assigned-addresses: Address and size of the port configuration 
>> >> > registers
>> >> > +- reg: Only the first four bytes are used to refer to the correct bus 
>> >> > number
>> >> > +  and device number.
>> >> > +- #address-cells: Must be 3
>> >> > +- #size-cells: Must be 2
>> >> > +- #interrupt-cells: Must be 1
>> >> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping 
>> >> > properties
>> >> > +  Please refer to the standard PCI bus binding document for a more 
>> >> > detailed
>> >> > +  explanation.
>> >>
>> >> Child nodes do not normally have interrupt-map properties. Isn't this
>> >> already covered by the interrupt-map in the parent?
>> >>
>> >
>> > I have one Intel 4 port ethernet card(:00:01) and MTK WLAN card
>> > (:00:02), probe message looks good to me.
>> >
>> > pci :00:01.0: fixup irq: got 224
>> > pci :00:01.0: assigning IRQ 224
>> > pci :00:02.0: fixup irq: got 225
>> > pci :00:02.0: assigning IRQ 225
>> >
>> > pci :01:00.0: fixup irq: got 224
>> > pci :01:00.0: assigning IRQ 224
>> > pci :01:00.1: fixup irq: got 224
>> > pci :01:00.1: assigning IRQ 224
>> > pci :01:00.2: fixup irq: got 224
>> > pci :01:00.2: assigning IRQ 224
>> > pci :01:00.3: fixup irq: got 224
>> > pci :01:00.3: assigning IRQ 224
>> >
>> > pci :02:00.0: fixup irq: got 225
>> > pci :02:00.0: assigning IRQ 225
>> >
>> >
>> > But child nodes without interrupt-map properties:
>> > It seems incorrect.
>> >
>> > pci :00:01.0: fixup irq: got 224
>> > pci :00:01.0: assigning IRQ 224
>> > pci :00:02.0: fixup irq: got 225
>> > pci :00:02.0: assigning IRQ 225
>> >
>> > pci :01:00.0: fixup irq: got 223
>> > pci :01:00.0: assigning IRQ 223
>>
>> Not entirely sure what happens here, but I guess the problem
>> is that the 'reg' portion of the parent interrupt-map refers to
>> the port devices, not the devices attached the devices behind
>> them.
>
> I agree with you. That's why I need additional interrupt-map properties
> to resolve IRQ correctly for the devices behind root ports.
>
> Not sure whether other platforms have similar case like me here.

I think it's just a bug in this specific chip where the HW designers
wired the IRQs in a nonstandard way.

However, you really should not need the interrupt-map properties
in the child nodes, just change the address part in the parent
interrupt-map. Specifically, the 'bus' portion of the device address
in the interrupt-map would have to be nonzero to refer to
child devices.

>> On a related note, I see that you still list
>>
>> > +- interrupts: Three interrupt outputs of the controller. Must contain an
>> > +  entry for each entry in the interrupt-names property.
>> > +- interrupt-names: Must include the following names
>> > +  - "pcie-int0"
>> > +  - "pcie-int1"
>> > +  - "pcie-int2"
>>
>> This seems to be an artifact from the older version and should be
>> removed as the driver correctly ignores the properties now.
>
> Actually, everything works fine without these properties however when it
> loads we see a few weird error message:
>
> pcieport :00:01.0: Signaling PME with IRQ 232
> pcieport :00:02.0: enabling device (0140 -> 0142)
> pcieport :00:02.0: enabling bus mastering
> irq 232: nobody cared (try booting with the "irqpoll" option)
> ...
> [] (pcie_pme_probe) from [] (pcie_port_probe_service
> +0x44/0x6c)
> (pcie_port_probe_service) from [] (driver_probe_device
> +0x280/0x470)
> ...
> (pcie_port_device_register) from [] (pcie_portdrv_probe
> +0x3c/0xb4)
> (pcie_portdrv_probe) from [] (pci_device_probe+0x98/0xfc)
> (pci_device_probe) from [] (driver_probe_device+0x280/0x470)
> handlers:
> [] pcie_pme_irq
> Disabling IRQ #233
>
> I haven't dig it out yet, but just keep them here to solve that.

Something is going very wrong if adding the properties helps. I can't
think of what that is, but we have to find out before the binding can
be merged.

  Arnd

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-11 Thread Arnd Bergmann

On Thu, May 11, 2017 at 4:44 AM, Ryder Lee  wrote:
> On Wed, 2017-05-10 at 12:01 +0200, Arnd Bergmann wrote:
>> On Wed, May 10, 2017 at 11:31 AM, Ryder Lee  wrote:
>> > On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote:

>> >> > +Required properties:
>> >> > +- device_type: Must be "pci"
>> >> > +- assigned-addresses: Address and size of the port configuration 
>> >> > registers
>> >> > +- reg: Only the first four bytes are used to refer to the correct bus 
>> >> > number
>> >> > +  and device number.
>> >> > +- #address-cells: Must be 3
>> >> > +- #size-cells: Must be 2
>> >> > +- #interrupt-cells: Must be 1
>> >> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping 
>> >> > properties
>> >> > +  Please refer to the standard PCI bus binding document for a more 
>> >> > detailed
>> >> > +  explanation.
>> >>
>> >> Child nodes do not normally have interrupt-map properties. Isn't this
>> >> already covered by the interrupt-map in the parent?
>> >>
>> >
>> > I have one Intel 4 port ethernet card(:00:01) and MTK WLAN card
>> > (:00:02), probe message looks good to me.
>> >
>> > pci :00:01.0: fixup irq: got 224
>> > pci :00:01.0: assigning IRQ 224
>> > pci :00:02.0: fixup irq: got 225
>> > pci :00:02.0: assigning IRQ 225
>> >
>> > pci :01:00.0: fixup irq: got 224
>> > pci :01:00.0: assigning IRQ 224
>> > pci :01:00.1: fixup irq: got 224
>> > pci :01:00.1: assigning IRQ 224
>> > pci :01:00.2: fixup irq: got 224
>> > pci :01:00.2: assigning IRQ 224
>> > pci :01:00.3: fixup irq: got 224
>> > pci :01:00.3: assigning IRQ 224
>> >
>> > pci :02:00.0: fixup irq: got 225
>> > pci :02:00.0: assigning IRQ 225
>> >
>> >
>> > But child nodes without interrupt-map properties:
>> > It seems incorrect.
>> >
>> > pci :00:01.0: fixup irq: got 224
>> > pci :00:01.0: assigning IRQ 224
>> > pci :00:02.0: fixup irq: got 225
>> > pci :00:02.0: assigning IRQ 225
>> >
>> > pci :01:00.0: fixup irq: got 223
>> > pci :01:00.0: assigning IRQ 223
>>
>> Not entirely sure what happens here, but I guess the problem
>> is that the 'reg' portion of the parent interrupt-map refers to
>> the port devices, not the devices attached the devices behind
>> them.
>
> I agree with you. That's why I need additional interrupt-map properties
> to resolve IRQ correctly for the devices behind root ports.
>
> Not sure whether other platforms have similar case like me here.

I think it's just a bug in this specific chip where the HW designers
wired the IRQs in a nonstandard way.

However, you really should not need the interrupt-map properties
in the child nodes, just change the address part in the parent
interrupt-map. Specifically, the 'bus' portion of the device address
in the interrupt-map would have to be nonzero to refer to
child devices.

>> On a related note, I see that you still list
>>
>> > +- interrupts: Three interrupt outputs of the controller. Must contain an
>> > +  entry for each entry in the interrupt-names property.
>> > +- interrupt-names: Must include the following names
>> > +  - "pcie-int0"
>> > +  - "pcie-int1"
>> > +  - "pcie-int2"
>>
>> This seems to be an artifact from the older version and should be
>> removed as the driver correctly ignores the properties now.
>
> Actually, everything works fine without these properties however when it
> loads we see a few weird error message:
>
> pcieport :00:01.0: Signaling PME with IRQ 232
> pcieport :00:02.0: enabling device (0140 -> 0142)
> pcieport :00:02.0: enabling bus mastering
> irq 232: nobody cared (try booting with the "irqpoll" option)
> ...
> [] (pcie_pme_probe) from [] (pcie_port_probe_service
> +0x44/0x6c)
> (pcie_port_probe_service) from [] (driver_probe_device
> +0x280/0x470)
> ...
> (pcie_port_device_register) from [] (pcie_portdrv_probe
> +0x3c/0xb4)
> (pcie_portdrv_probe) from [] (pci_device_probe+0x98/0xfc)
> (pci_device_probe) from [] (driver_probe_device+0x280/0x470)
> handlers:
> [] pcie_pme_irq
> Disabling IRQ #233
>
> I haven't dig it out yet, but just keep them here to solve that.

Something is going very wrong if adding the properties helps. I can't
think of what that is, but we have to find out before the binding can
be merged.

  Arnd

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Ryder Lee

On Wed, 2017-05-10 at 12:01 +0200, Arnd Bergmann wrote:
> On Wed, May 10, 2017 at 11:31 AM, Ryder Lee  wrote:
> > On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote:
> >> On Wed, May 10, 2017 at 4:07 AM, Ryder Lee  wrote:
> >>
> >> > +- ranges:
> >> > +  - The first three entries are expected to translate the addresses for 
> >> > the root
> >> > +port registers, which are referenced by the assigned-addresses 
> >> > property of
> >> > +the root port nodes (see below).
> >>
> >> I don't understand this part. Why do you need a static translation for 
> >> these?
> >> Shouldn't they just be listed in the 'reg' property of the parent node now 
> >> that
> >> you have the clk/reset/phy properties in the parent as well?
> >
> > At first, I did like that. But I noticed that someone suggest it's
> > better to use 'assigned-addresses' to handle per-port registers, the
> > same path as tegra and marvell did, in other platform discussion thread.
> > So I just put shared register in root node. It could be rolled back if
> > you feel this is inappropriate.
> 
> The marvell case is not a good example for your case: their top-level
> device is made up by the OS to help with the shared resource allocation,
> while in your case the bus bridge actually exists in hardware.
> 
> I'm not too familiar with the Tegra case, and haven't looked at that here,
> but it could be an artifact of how for a while we used to list the config
> space access in the top-level "ranges" instead of the "reg" property.
> 
> I'd vote for moving it back, for consistency with the other port specific
> properties that are now in the root node. Once you do that, the port
> nodes can be removed completely, which is what I was aiming for with
> the comments on the previous version.
 
I'll move it back.

> >> > +Required properties:
> >> > +- device_type: Must be "pci"
> >> > +- assigned-addresses: Address and size of the port configuration 
> >> > registers
> >> > +- reg: Only the first four bytes are used to refer to the correct bus 
> >> > number
> >> > +  and device number.
> >> > +- #address-cells: Must be 3
> >> > +- #size-cells: Must be 2
> >> > +- #interrupt-cells: Must be 1
> >> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping 
> >> > properties
> >> > +  Please refer to the standard PCI bus binding document for a more 
> >> > detailed
> >> > +  explanation.
> >>
> >> Child nodes do not normally have interrupt-map properties. Isn't this
> >> already covered by the interrupt-map in the parent?
> >>
> >
> > I have one Intel 4 port ethernet card(:00:01) and MTK WLAN card
> > (:00:02), probe message looks good to me.
> >
> > pci :00:01.0: fixup irq: got 224
> > pci :00:01.0: assigning IRQ 224
> > pci :00:02.0: fixup irq: got 225
> > pci :00:02.0: assigning IRQ 225
> >
> > pci :01:00.0: fixup irq: got 224
> > pci :01:00.0: assigning IRQ 224
> > pci :01:00.1: fixup irq: got 224
> > pci :01:00.1: assigning IRQ 224
> > pci :01:00.2: fixup irq: got 224
> > pci :01:00.2: assigning IRQ 224
> > pci :01:00.3: fixup irq: got 224
> > pci :01:00.3: assigning IRQ 224
> >
> > pci :02:00.0: fixup irq: got 225
> > pci :02:00.0: assigning IRQ 225
> >
> >
> > But child nodes without interrupt-map properties:
> > It seems incorrect.
> >
> > pci :00:01.0: fixup irq: got 224
> > pci :00:01.0: assigning IRQ 224
> > pci :00:02.0: fixup irq: got 225
> > pci :00:02.0: assigning IRQ 225
> >
> > pci :01:00.0: fixup irq: got 223
> > pci :01:00.0: assigning IRQ 223
> 
> Not entirely sure what happens here, but I guess the problem
> is that the 'reg' portion of the parent interrupt-map refers to
> the port devices, not the devices attached the devices behind
> them.

I agree with you. That's why I need additional interrupt-map properties
to resolve IRQ correctly for the devices behind root ports.

Not sure whether other platforms have similar case like me here.

> On a related note, I see that you still list
> 
> > +- interrupts: Three interrupt outputs of the controller. Must contain an
> > +  entry for each entry in the interrupt-names property.
> > +- interrupt-names: Must include the following names
> > +  - "pcie-int0"
> > +  - "pcie-int1"
> > +  - "pcie-int2"
> 
> This seems to be an artifact from the older version and should be
> removed as the driver correctly ignores the properties now.

Actually, everything works fine without these properties however when it
loads we see a few weird error message:

pcieport :00:01.0: Signaling PME with IRQ 232
pcieport :00:02.0: enabling device (0140 -> 0142)
pcieport :00:02.0: enabling bus mastering
irq 232: nobody cared (try booting with the "irqpoll" option)
...
[] (pcie_pme_probe) from [] (pcie_port_probe_service
+0x44/0x6c)
(pcie_port_probe_service) from [] (driver_probe_device
+0x280/0x470)
...
(pcie_port_device_register) from [] (pcie_portdrv_probe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Ryder Lee

On Wed, 2017-05-10 at 12:01 +0200, Arnd Bergmann wrote:
> On Wed, May 10, 2017 at 11:31 AM, Ryder Lee  wrote:
> > On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote:
> >> On Wed, May 10, 2017 at 4:07 AM, Ryder Lee  wrote:
> >>
> >> > +- ranges:
> >> > +  - The first three entries are expected to translate the addresses for 
> >> > the root
> >> > +port registers, which are referenced by the assigned-addresses 
> >> > property of
> >> > +the root port nodes (see below).
> >>
> >> I don't understand this part. Why do you need a static translation for 
> >> these?
> >> Shouldn't they just be listed in the 'reg' property of the parent node now 
> >> that
> >> you have the clk/reset/phy properties in the parent as well?
> >
> > At first, I did like that. But I noticed that someone suggest it's
> > better to use 'assigned-addresses' to handle per-port registers, the
> > same path as tegra and marvell did, in other platform discussion thread.
> > So I just put shared register in root node. It could be rolled back if
> > you feel this is inappropriate.
> 
> The marvell case is not a good example for your case: their top-level
> device is made up by the OS to help with the shared resource allocation,
> while in your case the bus bridge actually exists in hardware.
> 
> I'm not too familiar with the Tegra case, and haven't looked at that here,
> but it could be an artifact of how for a while we used to list the config
> space access in the top-level "ranges" instead of the "reg" property.
> 
> I'd vote for moving it back, for consistency with the other port specific
> properties that are now in the root node. Once you do that, the port
> nodes can be removed completely, which is what I was aiming for with
> the comments on the previous version.
 
I'll move it back.

> >> > +Required properties:
> >> > +- device_type: Must be "pci"
> >> > +- assigned-addresses: Address and size of the port configuration 
> >> > registers
> >> > +- reg: Only the first four bytes are used to refer to the correct bus 
> >> > number
> >> > +  and device number.
> >> > +- #address-cells: Must be 3
> >> > +- #size-cells: Must be 2
> >> > +- #interrupt-cells: Must be 1
> >> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping 
> >> > properties
> >> > +  Please refer to the standard PCI bus binding document for a more 
> >> > detailed
> >> > +  explanation.
> >>
> >> Child nodes do not normally have interrupt-map properties. Isn't this
> >> already covered by the interrupt-map in the parent?
> >>
> >
> > I have one Intel 4 port ethernet card(:00:01) and MTK WLAN card
> > (:00:02), probe message looks good to me.
> >
> > pci :00:01.0: fixup irq: got 224
> > pci :00:01.0: assigning IRQ 224
> > pci :00:02.0: fixup irq: got 225
> > pci :00:02.0: assigning IRQ 225
> >
> > pci :01:00.0: fixup irq: got 224
> > pci :01:00.0: assigning IRQ 224
> > pci :01:00.1: fixup irq: got 224
> > pci :01:00.1: assigning IRQ 224
> > pci :01:00.2: fixup irq: got 224
> > pci :01:00.2: assigning IRQ 224
> > pci :01:00.3: fixup irq: got 224
> > pci :01:00.3: assigning IRQ 224
> >
> > pci :02:00.0: fixup irq: got 225
> > pci :02:00.0: assigning IRQ 225
> >
> >
> > But child nodes without interrupt-map properties:
> > It seems incorrect.
> >
> > pci :00:01.0: fixup irq: got 224
> > pci :00:01.0: assigning IRQ 224
> > pci :00:02.0: fixup irq: got 225
> > pci :00:02.0: assigning IRQ 225
> >
> > pci :01:00.0: fixup irq: got 223
> > pci :01:00.0: assigning IRQ 223
> 
> Not entirely sure what happens here, but I guess the problem
> is that the 'reg' portion of the parent interrupt-map refers to
> the port devices, not the devices attached the devices behind
> them.

I agree with you. That's why I need additional interrupt-map properties
to resolve IRQ correctly for the devices behind root ports.

Not sure whether other platforms have similar case like me here.

> On a related note, I see that you still list
> 
> > +- interrupts: Three interrupt outputs of the controller. Must contain an
> > +  entry for each entry in the interrupt-names property.
> > +- interrupt-names: Must include the following names
> > +  - "pcie-int0"
> > +  - "pcie-int1"
> > +  - "pcie-int2"
> 
> This seems to be an artifact from the older version and should be
> removed as the driver correctly ignores the properties now.

Actually, everything works fine without these properties however when it
loads we see a few weird error message:

pcieport :00:01.0: Signaling PME with IRQ 232
pcieport :00:02.0: enabling device (0140 -> 0142)
pcieport :00:02.0: enabling bus mastering
irq 232: nobody cared (try booting with the "irqpoll" option)
...
[] (pcie_pme_probe) from [] (pcie_port_probe_service
+0x44/0x6c)
(pcie_port_probe_service) from [] (driver_probe_device
+0x280/0x470)
...
(pcie_port_device_register) from [] (pcie_portdrv_probe
+0x3c/0xb4)
(pcie_portdrv_probe) from []

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Arnd Bergmann

On Wed, May 10, 2017 at 11:31 AM, Ryder Lee  wrote:
> On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote:
>> On Wed, May 10, 2017 at 4:07 AM, Ryder Lee  wrote:
>>
>> > +- ranges:
>> > +  - The first three entries are expected to translate the addresses for 
>> > the root
>> > +port registers, which are referenced by the assigned-addresses 
>> > property of
>> > +the root port nodes (see below).
>>
>> I don't understand this part. Why do you need a static translation for these?
>> Shouldn't they just be listed in the 'reg' property of the parent node now 
>> that
>> you have the clk/reset/phy properties in the parent as well?
>
> At first, I did like that. But I noticed that someone suggest it's
> better to use 'assigned-addresses' to handle per-port registers, the
> same path as tegra and marvell did, in other platform discussion thread.
> So I just put shared register in root node. It could be rolled back if
> you feel this is inappropriate.

The marvell case is not a good example for your case: their top-level
device is made up by the OS to help with the shared resource allocation,
while in your case the bus bridge actually exists in hardware.

I'm not too familiar with the Tegra case, and haven't looked at that here,
but it could be an artifact of how for a while we used to list the config
space access in the top-level "ranges" instead of the "reg" property.

I'd vote for moving it back, for consistency with the other port specific
properties that are now in the root node. Once you do that, the port
nodes can be removed completely, which is what I was aiming for with
the comments on the previous version.

>> > +Required properties:
>> > +- device_type: Must be "pci"
>> > +- assigned-addresses: Address and size of the port configuration registers
>> > +- reg: Only the first four bytes are used to refer to the correct bus 
>> > number
>> > +  and device number.
>> > +- #address-cells: Must be 3
>> > +- #size-cells: Must be 2
>> > +- #interrupt-cells: Must be 1
>> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping 
>> > properties
>> > +  Please refer to the standard PCI bus binding document for a more 
>> > detailed
>> > +  explanation.
>>
>> Child nodes do not normally have interrupt-map properties. Isn't this
>> already covered by the interrupt-map in the parent?
>>
>
> I have one Intel 4 port ethernet card(:00:01) and MTK WLAN card
> (:00:02), probe message looks good to me.
>
> pci :00:01.0: fixup irq: got 224
> pci :00:01.0: assigning IRQ 224
> pci :00:02.0: fixup irq: got 225
> pci :00:02.0: assigning IRQ 225
>
> pci :01:00.0: fixup irq: got 224
> pci :01:00.0: assigning IRQ 224
> pci :01:00.1: fixup irq: got 224
> pci :01:00.1: assigning IRQ 224
> pci :01:00.2: fixup irq: got 224
> pci :01:00.2: assigning IRQ 224
> pci :01:00.3: fixup irq: got 224
> pci :01:00.3: assigning IRQ 224
>
> pci :02:00.0: fixup irq: got 225
> pci :02:00.0: assigning IRQ 225
>
>
> But child nodes without interrupt-map properties:
> It seems incorrect.
>
> pci :00:01.0: fixup irq: got 224
> pci :00:01.0: assigning IRQ 224
> pci :00:02.0: fixup irq: got 225
> pci :00:02.0: assigning IRQ 225
>
> pci :01:00.0: fixup irq: got 223
> pci :01:00.0: assigning IRQ 223

Not entirely sure what happens here, but I guess the problem
is that the 'reg' portion of the parent interrupt-map refers to
the port devices, not the devices attached the devices behind
them.

On a related note, I see that you still list

> +- interrupts: Three interrupt outputs of the controller. Must contain an
> +  entry for each entry in the interrupt-names property.
> +- interrupt-names: Must include the following names
> +  - "pcie-int0"
> +  - "pcie-int1"
> +  - "pcie-int2"

This seems to be an artifact from the older version and should be
removed as the driver correctly ignores the properties now.

  Arnd

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Arnd Bergmann

On Wed, May 10, 2017 at 11:31 AM, Ryder Lee  wrote:
> On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote:
>> On Wed, May 10, 2017 at 4:07 AM, Ryder Lee  wrote:
>>
>> > +- ranges:
>> > +  - The first three entries are expected to translate the addresses for 
>> > the root
>> > +port registers, which are referenced by the assigned-addresses 
>> > property of
>> > +the root port nodes (see below).
>>
>> I don't understand this part. Why do you need a static translation for these?
>> Shouldn't they just be listed in the 'reg' property of the parent node now 
>> that
>> you have the clk/reset/phy properties in the parent as well?
>
> At first, I did like that. But I noticed that someone suggest it's
> better to use 'assigned-addresses' to handle per-port registers, the
> same path as tegra and marvell did, in other platform discussion thread.
> So I just put shared register in root node. It could be rolled back if
> you feel this is inappropriate.

The marvell case is not a good example for your case: their top-level
device is made up by the OS to help with the shared resource allocation,
while in your case the bus bridge actually exists in hardware.

I'm not too familiar with the Tegra case, and haven't looked at that here,
but it could be an artifact of how for a while we used to list the config
space access in the top-level "ranges" instead of the "reg" property.

I'd vote for moving it back, for consistency with the other port specific
properties that are now in the root node. Once you do that, the port
nodes can be removed completely, which is what I was aiming for with
the comments on the previous version.

>> > +Required properties:
>> > +- device_type: Must be "pci"
>> > +- assigned-addresses: Address and size of the port configuration registers
>> > +- reg: Only the first four bytes are used to refer to the correct bus 
>> > number
>> > +  and device number.
>> > +- #address-cells: Must be 3
>> > +- #size-cells: Must be 2
>> > +- #interrupt-cells: Must be 1
>> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping 
>> > properties
>> > +  Please refer to the standard PCI bus binding document for a more 
>> > detailed
>> > +  explanation.
>>
>> Child nodes do not normally have interrupt-map properties. Isn't this
>> already covered by the interrupt-map in the parent?
>>
>
> I have one Intel 4 port ethernet card(:00:01) and MTK WLAN card
> (:00:02), probe message looks good to me.
>
> pci :00:01.0: fixup irq: got 224
> pci :00:01.0: assigning IRQ 224
> pci :00:02.0: fixup irq: got 225
> pci :00:02.0: assigning IRQ 225
>
> pci :01:00.0: fixup irq: got 224
> pci :01:00.0: assigning IRQ 224
> pci :01:00.1: fixup irq: got 224
> pci :01:00.1: assigning IRQ 224
> pci :01:00.2: fixup irq: got 224
> pci :01:00.2: assigning IRQ 224
> pci :01:00.3: fixup irq: got 224
> pci :01:00.3: assigning IRQ 224
>
> pci :02:00.0: fixup irq: got 225
> pci :02:00.0: assigning IRQ 225
>
>
> But child nodes without interrupt-map properties:
> It seems incorrect.
>
> pci :00:01.0: fixup irq: got 224
> pci :00:01.0: assigning IRQ 224
> pci :00:02.0: fixup irq: got 225
> pci :00:02.0: assigning IRQ 225
>
> pci :01:00.0: fixup irq: got 223
> pci :01:00.0: assigning IRQ 223

Not entirely sure what happens here, but I guess the problem
is that the 'reg' portion of the parent interrupt-map refers to
the port devices, not the devices attached the devices behind
them.

On a related note, I see that you still list

> +- interrupts: Three interrupt outputs of the controller. Must contain an
> +  entry for each entry in the interrupt-names property.
> +- interrupt-names: Must include the following names
> +  - "pcie-int0"
> +  - "pcie-int1"
> +  - "pcie-int2"

This seems to be an artifact from the older version and should be
removed as the driver correctly ignores the properties now.

  Arnd

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Ryder Lee

On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote:
> On Wed, May 10, 2017 at 4:07 AM, Ryder Lee  wrote:
> 
> > +- ranges:
> > +  - The first three entries are expected to translate the addresses for 
> > the root
> > +port registers, which are referenced by the assigned-addresses 
> > property of
> > +the root port nodes (see below).
> 
> I don't understand this part. Why do you need a static translation for these?
> Shouldn't they just be listed in the 'reg' property of the parent node now 
> that
> you have the clk/reset/phy properties in the parent as well?

At first, I did like that. But I noticed that someone suggest it's
better to use 'assigned-addresses' to handle per-port registers, the
same path as tegra and marvell did, in other platform discussion thread.
So I just put shared register in root node. It could be rolled back if
you feel this is inappropriate.

> > +Required properties:
> > +- device_type: Must be "pci"
> > +- assigned-addresses: Address and size of the port configuration registers
> > +- reg: Only the first four bytes are used to refer to the correct bus 
> > number
> > +  and device number.
> > +- #address-cells: Must be 3
> > +- #size-cells: Must be 2
> > +- #interrupt-cells: Must be 1
> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
> > +  Please refer to the standard PCI bus binding document for a more detailed
> > +  explanation.
> 
> Child nodes do not normally have interrupt-map properties. Isn't this
> already covered by the interrupt-map in the parent?
> 

I have one Intel 4 port ethernet card(:00:01) and MTK WLAN card
(:00:02), probe message looks good to me.

pci :00:01.0: fixup irq: got 224
pci :00:01.0: assigning IRQ 224
pci :00:02.0: fixup irq: got 225
pci :00:02.0: assigning IRQ 225

pci :01:00.0: fixup irq: got 224
pci :01:00.0: assigning IRQ 224
pci :01:00.1: fixup irq: got 224
pci :01:00.1: assigning IRQ 224
pci :01:00.2: fixup irq: got 224
pci :01:00.2: assigning IRQ 224
pci :01:00.3: fixup irq: got 224
pci :01:00.3: assigning IRQ 224

pci :02:00.0: fixup irq: got 225
pci :02:00.0: assigning IRQ 225


But child nodes without interrupt-map properties:
It seems incorrect.

pci :00:01.0: fixup irq: got 224
pci :00:01.0: assigning IRQ 224
pci :00:02.0: fixup irq: got 225
pci :00:02.0: assigning IRQ 225

pci :01:00.0: fixup irq: got 223
pci :01:00.0: assigning IRQ 223
pci :01:00.1: fixup irq: got 223
pci :01:00.1: assigning IRQ 223
pci :01:00.2: fixup irq: got 223
pci :01:00.2: assigning IRQ 223
pci :01:00.3: fixup irq: got 223
pci :01:00.3: assigning IRQ 223

pci :02:00.0: fixup irq: got 223
pci :02:00.0: assigning IRQ 223

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Ryder Lee

On Wed, 2017-05-10 at 10:08 +0200, Arnd Bergmann wrote:
> On Wed, May 10, 2017 at 4:07 AM, Ryder Lee  wrote:
> 
> > +- ranges:
> > +  - The first three entries are expected to translate the addresses for 
> > the root
> > +port registers, which are referenced by the assigned-addresses 
> > property of
> > +the root port nodes (see below).
> 
> I don't understand this part. Why do you need a static translation for these?
> Shouldn't they just be listed in the 'reg' property of the parent node now 
> that
> you have the clk/reset/phy properties in the parent as well?

At first, I did like that. But I noticed that someone suggest it's
better to use 'assigned-addresses' to handle per-port registers, the
same path as tegra and marvell did, in other platform discussion thread.
So I just put shared register in root node. It could be rolled back if
you feel this is inappropriate.

> > +Required properties:
> > +- device_type: Must be "pci"
> > +- assigned-addresses: Address and size of the port configuration registers
> > +- reg: Only the first four bytes are used to refer to the correct bus 
> > number
> > +  and device number.
> > +- #address-cells: Must be 3
> > +- #size-cells: Must be 2
> > +- #interrupt-cells: Must be 1
> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
> > +  Please refer to the standard PCI bus binding document for a more detailed
> > +  explanation.
> 
> Child nodes do not normally have interrupt-map properties. Isn't this
> already covered by the interrupt-map in the parent?
> 

I have one Intel 4 port ethernet card(:00:01) and MTK WLAN card
(:00:02), probe message looks good to me.

pci :00:01.0: fixup irq: got 224
pci :00:01.0: assigning IRQ 224
pci :00:02.0: fixup irq: got 225
pci :00:02.0: assigning IRQ 225

pci :01:00.0: fixup irq: got 224
pci :01:00.0: assigning IRQ 224
pci :01:00.1: fixup irq: got 224
pci :01:00.1: assigning IRQ 224
pci :01:00.2: fixup irq: got 224
pci :01:00.2: assigning IRQ 224
pci :01:00.3: fixup irq: got 224
pci :01:00.3: assigning IRQ 224

pci :02:00.0: fixup irq: got 225
pci :02:00.0: assigning IRQ 225


But child nodes without interrupt-map properties:
It seems incorrect.

pci :00:01.0: fixup irq: got 224
pci :00:01.0: assigning IRQ 224
pci :00:02.0: fixup irq: got 225
pci :00:02.0: assigning IRQ 225

pci :01:00.0: fixup irq: got 223
pci :01:00.0: assigning IRQ 223
pci :01:00.1: fixup irq: got 223
pci :01:00.1: assigning IRQ 223
pci :01:00.2: fixup irq: got 223
pci :01:00.2: assigning IRQ 223
pci :01:00.3: fixup irq: got 223
pci :01:00.3: assigning IRQ 223

pci :02:00.0: fixup irq: got 223
pci :02:00.0: assigning IRQ 223

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Ryder Lee

On Wed, 2017-05-10 at 09:58 +0200, Matthias Brugger wrote:
> 
> On 10/05/17 04:07, Ryder Lee wrote:
> > Add documentation for PCIe host driver available in MT7623
> > series SoCs.
> > 
> > Signed-off-by: Ryder Lee 
> > Acked-by: Rob Herring 
> > ---
> >   .../bindings/pci/mediatek,mt7623-pcie.txt  | 149 
> > +
> >   1 file changed, 149 insertions(+)
> >   create mode 100644 
> > Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt 
> > b/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
> > new file mode 100644
> > index 000..a9393ce
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
> > @@ -0,0 +1,149 @@
> > +Mediatek Gen2 PCIe controller which is available on MT7623 series SoCs
> > +
> > +PCIe subsys supports single root complex (RC) with 3 Root Ports. Each root
> > +ports supports a Gen2 1-lane Link and has PIPE interface to PHY.
> > +
> > +Required properties:
> > +- compatible: Should contain "mediatek,mt7623-pcie".
> > +- device_type: Must be "pci"
> > +- reg: Base addresses and lengths of the PCIe controller.
> > +- #address-cells: Address representation for root ports (must be 3)
> > +- #size-cells: Size representation for root ports (must be 2)
> > +- #interrupt-cells: Size representation for interrupts (must be 1)
> > +- interrupts: Three interrupt outputs of the controller. Must contain an
> > +  entry for each entry in the interrupt-names property.
> > +- interrupt-names: Must include the following names
> > +  - "pcie-int0"
> > +  - "pcie-int1"
> > +  - "pcie-int2"
> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
> > +  Please refer to the standard PCI bus binding document for a more detailed
> > +  explanation.
> > +- clocks: Must contain an entry for each entry in clock-names.
> > +  See ../clocks/clock-bindings.txt for details.
> > +- clock-names: Must include the following entries:
> > +  - free_ck :for reference clock of PCIe subsys
> > +  - sys_ck0 :for clock of Port0
> > +  - sys_ck1 :for clock of Port1
> > +  - sys_ck2 :for clock of Port2
> > +- resets: Must contain an entry for each entry in reset-names.
> > +  See ../reset/reset.txt for details.
> > +- reset-names: Must include the following entries:
> > +  - pcie-rst0 :port0 reset
> > +  - pcie-rst1 :port1 reset
> > +  - pcie-rst2 :port2 reset
> > +- phys: list of PHY specifiers (used by generic PHY framework).
> > +- phy-names : must be "pcie-phy0", "pcie-phy1", "pcie-phyN".. based on the
> > +  number of PHYs as specified in *phys* property.
> > +- power-domains: A phandle and power domain specifier pair to the power 
> > domain
> > +  which is responsible for collapsing and restoring power to the 
> > peripheral.
> > +- bus-range: Range of bus numbers associated with this controller.
> > +- ranges:
> > +  - The first three entries are expected to translate the addresses for 
> > the root
> > +port registers, which are referenced by the assigned-addresses 
> > property of
> > +the root port nodes (see below).
> > +  - The remaining entries setup the mapping for the standard I/O and memory
> > +   regions.
> > +  Please refer to the standard PCI bus binding document for a more detailed
> > +  explanation.
> > +
> > +In addition, the device tree node must have sub-nodes describing each
> > +PCIe port interface, having the following mandatory properties:
> > +
> > +Required properties:
> > +- device_type: Must be "pci"
> > +- assigned-addresses: Address and size of the port configuration registers
> > +- reg: Only the first four bytes are used to refer to the correct bus 
> > number
> > +  and device number.
> > +- #address-cells: Must be 3
> > +- #size-cells: Must be 2
> > +- #interrupt-cells: Must be 1
> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
> > +  Please refer to the standard PCI bus binding document for a more detailed
> > +  explanation.
> > +- ranges: Sub-ranges distributed from the PCIe controller node. An empty
> > +  property is sufficient.
> > +- num-lanes: Number of lanes to use for this port.
> > +
> > +Examples:
> > +
> > +   hifsys: syscon@1a00 {
> > +   compatible = "mediatek,mt7623-hifsys", "syscon";
> 
> If you want to put the clock controller subsystem node to the example, 
> please use a valid compatible, for example:
> 
>   compatible = "mediatek,mt7623-hifsys",
>"mediatek,mt2701-hifsys",
>"syscon";
> 
> Thanks,
> Matthias

OK i will correct it.
> 
> > +   reg = <0 0x1a00 0 0x1000>;
> > +   #clock-cells = <1>;
> > +   #reset-cells = <1>;
> > +   };
> > +
> > +   pcie: pcie-controller@1a14 {
> > +   compatible = "mediatek,mt7623-pcie";
> > +   device_type = "pci";
> > +   reg = <0

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Ryder Lee

On Wed, 2017-05-10 at 09:58 +0200, Matthias Brugger wrote:
> 
> On 10/05/17 04:07, Ryder Lee wrote:
> > Add documentation for PCIe host driver available in MT7623
> > series SoCs.
> > 
> > Signed-off-by: Ryder Lee 
> > Acked-by: Rob Herring 
> > ---
> >   .../bindings/pci/mediatek,mt7623-pcie.txt  | 149 
> > +
> >   1 file changed, 149 insertions(+)
> >   create mode 100644 
> > Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt 
> > b/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
> > new file mode 100644
> > index 000..a9393ce
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
> > @@ -0,0 +1,149 @@
> > +Mediatek Gen2 PCIe controller which is available on MT7623 series SoCs
> > +
> > +PCIe subsys supports single root complex (RC) with 3 Root Ports. Each root
> > +ports supports a Gen2 1-lane Link and has PIPE interface to PHY.
> > +
> > +Required properties:
> > +- compatible: Should contain "mediatek,mt7623-pcie".
> > +- device_type: Must be "pci"
> > +- reg: Base addresses and lengths of the PCIe controller.
> > +- #address-cells: Address representation for root ports (must be 3)
> > +- #size-cells: Size representation for root ports (must be 2)
> > +- #interrupt-cells: Size representation for interrupts (must be 1)
> > +- interrupts: Three interrupt outputs of the controller. Must contain an
> > +  entry for each entry in the interrupt-names property.
> > +- interrupt-names: Must include the following names
> > +  - "pcie-int0"
> > +  - "pcie-int1"
> > +  - "pcie-int2"
> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
> > +  Please refer to the standard PCI bus binding document for a more detailed
> > +  explanation.
> > +- clocks: Must contain an entry for each entry in clock-names.
> > +  See ../clocks/clock-bindings.txt for details.
> > +- clock-names: Must include the following entries:
> > +  - free_ck :for reference clock of PCIe subsys
> > +  - sys_ck0 :for clock of Port0
> > +  - sys_ck1 :for clock of Port1
> > +  - sys_ck2 :for clock of Port2
> > +- resets: Must contain an entry for each entry in reset-names.
> > +  See ../reset/reset.txt for details.
> > +- reset-names: Must include the following entries:
> > +  - pcie-rst0 :port0 reset
> > +  - pcie-rst1 :port1 reset
> > +  - pcie-rst2 :port2 reset
> > +- phys: list of PHY specifiers (used by generic PHY framework).
> > +- phy-names : must be "pcie-phy0", "pcie-phy1", "pcie-phyN".. based on the
> > +  number of PHYs as specified in *phys* property.
> > +- power-domains: A phandle and power domain specifier pair to the power 
> > domain
> > +  which is responsible for collapsing and restoring power to the 
> > peripheral.
> > +- bus-range: Range of bus numbers associated with this controller.
> > +- ranges:
> > +  - The first three entries are expected to translate the addresses for 
> > the root
> > +port registers, which are referenced by the assigned-addresses 
> > property of
> > +the root port nodes (see below).
> > +  - The remaining entries setup the mapping for the standard I/O and memory
> > +   regions.
> > +  Please refer to the standard PCI bus binding document for a more detailed
> > +  explanation.
> > +
> > +In addition, the device tree node must have sub-nodes describing each
> > +PCIe port interface, having the following mandatory properties:
> > +
> > +Required properties:
> > +- device_type: Must be "pci"
> > +- assigned-addresses: Address and size of the port configuration registers
> > +- reg: Only the first four bytes are used to refer to the correct bus 
> > number
> > +  and device number.
> > +- #address-cells: Must be 3
> > +- #size-cells: Must be 2
> > +- #interrupt-cells: Must be 1
> > +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
> > +  Please refer to the standard PCI bus binding document for a more detailed
> > +  explanation.
> > +- ranges: Sub-ranges distributed from the PCIe controller node. An empty
> > +  property is sufficient.
> > +- num-lanes: Number of lanes to use for this port.
> > +
> > +Examples:
> > +
> > +   hifsys: syscon@1a00 {
> > +   compatible = "mediatek,mt7623-hifsys", "syscon";
> 
> If you want to put the clock controller subsystem node to the example, 
> please use a valid compatible, for example:
> 
>   compatible = "mediatek,mt7623-hifsys",
>"mediatek,mt2701-hifsys",
>"syscon";
> 
> Thanks,
> Matthias

OK i will correct it.
> 
> > +   reg = <0 0x1a00 0 0x1000>;
> > +   #clock-cells = <1>;
> > +   #reset-cells = <1>;
> > +   };
> > +
> > +   pcie: pcie-controller@1a14 {
> > +   compatible = "mediatek,mt7623-pcie";
> > +   device_type = "pci";
> > +   reg = <0 0x1a14 0 0x1000>; /* PCIe shared

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Arnd Bergmann

On Wed, May 10, 2017 at 4:07 AM, Ryder Lee  wrote:

> +- ranges:
> +  - The first three entries are expected to translate the addresses for the 
> root
> +port registers, which are referenced by the assigned-addresses property 
> of
> +the root port nodes (see below).

I don't understand this part. Why do you need a static translation for these?
Shouldn't they just be listed in the 'reg' property of the parent node now that
you have the clk/reset/phy properties in the parent as well?

> +Required properties:
> +- device_type: Must be "pci"
> +- assigned-addresses: Address and size of the port configuration registers
> +- reg: Only the first four bytes are used to refer to the correct bus number
> +  and device number.
> +- #address-cells: Must be 3
> +- #size-cells: Must be 2
> +- #interrupt-cells: Must be 1
> +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
> +  Please refer to the standard PCI bus binding document for a more detailed
> +  explanation.

Child nodes do not normally have interrupt-map properties. Isn't this
already covered by the interrupt-map in the parent?

  Arnd

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Arnd Bergmann

On Wed, May 10, 2017 at 4:07 AM, Ryder Lee  wrote:

> +- ranges:
> +  - The first three entries are expected to translate the addresses for the 
> root
> +port registers, which are referenced by the assigned-addresses property 
> of
> +the root port nodes (see below).

I don't understand this part. Why do you need a static translation for these?
Shouldn't they just be listed in the 'reg' property of the parent node now that
you have the clk/reset/phy properties in the parent as well?

> +Required properties:
> +- device_type: Must be "pci"
> +- assigned-addresses: Address and size of the port configuration registers
> +- reg: Only the first four bytes are used to refer to the correct bus number
> +  and device number.
> +- #address-cells: Must be 3
> +- #size-cells: Must be 2
> +- #interrupt-cells: Must be 1
> +- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
> +  Please refer to the standard PCI bus binding document for a more detailed
> +  explanation.

Child nodes do not normally have interrupt-map properties. Isn't this
already covered by the interrupt-map in the parent?

  Arnd

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Matthias Brugger




On 10/05/17 04:07, Ryder Lee wrote:

Add documentation for PCIe host driver available in MT7623
series SoCs.

Signed-off-by: Ryder Lee 
Acked-by: Rob Herring 
---
  .../bindings/pci/mediatek,mt7623-pcie.txt  | 149 +
  1 file changed, 149 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt 
b/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
new file mode 100644
index 000..a9393ce
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
@@ -0,0 +1,149 @@
+Mediatek Gen2 PCIe controller which is available on MT7623 series SoCs
+
+PCIe subsys supports single root complex (RC) with 3 Root Ports. Each root
+ports supports a Gen2 1-lane Link and has PIPE interface to PHY.
+
+Required properties:
+- compatible: Should contain "mediatek,mt7623-pcie".
+- device_type: Must be "pci"
+- reg: Base addresses and lengths of the PCIe controller.
+- #address-cells: Address representation for root ports (must be 3)
+- #size-cells: Size representation for root ports (must be 2)
+- #interrupt-cells: Size representation for interrupts (must be 1)
+- interrupts: Three interrupt outputs of the controller. Must contain an
+  entry for each entry in the interrupt-names property.
+- interrupt-names: Must include the following names
+  - "pcie-int0"
+  - "pcie-int1"
+  - "pcie-int2"
+- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
+  Please refer to the standard PCI bus binding document for a more detailed
+  explanation.
+- clocks: Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names: Must include the following entries:
+  - free_ck :for reference clock of PCIe subsys
+  - sys_ck0 :for clock of Port0
+  - sys_ck1 :for clock of Port1
+  - sys_ck2 :for clock of Port2
+- resets: Must contain an entry for each entry in reset-names.
+  See ../reset/reset.txt for details.
+- reset-names: Must include the following entries:
+  - pcie-rst0 :port0 reset
+  - pcie-rst1 :port1 reset
+  - pcie-rst2 :port2 reset
+- phys: list of PHY specifiers (used by generic PHY framework).
+- phy-names : must be "pcie-phy0", "pcie-phy1", "pcie-phyN".. based on the
+  number of PHYs as specified in *phys* property.
+- power-domains: A phandle and power domain specifier pair to the power domain
+  which is responsible for collapsing and restoring power to the peripheral.
+- bus-range: Range of bus numbers associated with this controller.
+- ranges:
+  - The first three entries are expected to translate the addresses for the 
root
+port registers, which are referenced by the assigned-addresses property of
+the root port nodes (see below).
+  - The remaining entries setup the mapping for the standard I/O and memory
+   regions.
+  Please refer to the standard PCI bus binding document for a more detailed
+  explanation.
+
+In addition, the device tree node must have sub-nodes describing each
+PCIe port interface, having the following mandatory properties:
+
+Required properties:
+- device_type: Must be "pci"
+- assigned-addresses: Address and size of the port configuration registers
+- reg: Only the first four bytes are used to refer to the correct bus number
+  and device number.
+- #address-cells: Must be 3
+- #size-cells: Must be 2
+- #interrupt-cells: Must be 1
+- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
+  Please refer to the standard PCI bus binding document for a more detailed
+  explanation.
+- ranges: Sub-ranges distributed from the PCIe controller node. An empty
+  property is sufficient.
+- num-lanes: Number of lanes to use for this port.
+
+Examples:
+
+   hifsys: syscon@1a00 {
+   compatible = "mediatek,mt7623-hifsys", "syscon";


If you want to put the clock controller subsystem node to the example, 
please use a valid compatible, for example:


compatible = "mediatek,mt7623-hifsys",
 "mediatek,mt2701-hifsys",
 "syscon";

Thanks,
Matthias


+   reg = <0 0x1a00 0 0x1000>;
+   #clock-cells = <1>;
+   #reset-cells = <1>;
+   };
+
+   pcie: pcie-controller@1a14 {
+   compatible = "mediatek,mt7623-pcie";
+   device_type = "pci";
+   reg = <0 0x1a14 0 0x1000>; /* PCIe shared registers */
+   #address-cells = <3>;
+   #size-cells = <2>;
+   #interrupt-cells = <1>;
+   interrupts = ,
+,
+;
+   interrupt-names = "pcie-int0", "pcie-int1", "pcie-int2";
+   interrupt-map-mask = <0xf800 0 0 0>;
+   interrupt-map = <0x 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
+

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

2017-05-10 Thread Matthias Brugger




On 10/05/17 04:07, Ryder Lee wrote:

Add documentation for PCIe host driver available in MT7623
series SoCs.

Signed-off-by: Ryder Lee 
Acked-by: Rob Herring 
---
  .../bindings/pci/mediatek,mt7623-pcie.txt  | 149 +
  1 file changed, 149 insertions(+)
  create mode 100644 
Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt 
b/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
new file mode 100644
index 000..a9393ce
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/mediatek,mt7623-pcie.txt
@@ -0,0 +1,149 @@
+Mediatek Gen2 PCIe controller which is available on MT7623 series SoCs
+
+PCIe subsys supports single root complex (RC) with 3 Root Ports. Each root
+ports supports a Gen2 1-lane Link and has PIPE interface to PHY.
+
+Required properties:
+- compatible: Should contain "mediatek,mt7623-pcie".
+- device_type: Must be "pci"
+- reg: Base addresses and lengths of the PCIe controller.
+- #address-cells: Address representation for root ports (must be 3)
+- #size-cells: Size representation for root ports (must be 2)
+- #interrupt-cells: Size representation for interrupts (must be 1)
+- interrupts: Three interrupt outputs of the controller. Must contain an
+  entry for each entry in the interrupt-names property.
+- interrupt-names: Must include the following names
+  - "pcie-int0"
+  - "pcie-int1"
+  - "pcie-int2"
+- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
+  Please refer to the standard PCI bus binding document for a more detailed
+  explanation.
+- clocks: Must contain an entry for each entry in clock-names.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names: Must include the following entries:
+  - free_ck :for reference clock of PCIe subsys
+  - sys_ck0 :for clock of Port0
+  - sys_ck1 :for clock of Port1
+  - sys_ck2 :for clock of Port2
+- resets: Must contain an entry for each entry in reset-names.
+  See ../reset/reset.txt for details.
+- reset-names: Must include the following entries:
+  - pcie-rst0 :port0 reset
+  - pcie-rst1 :port1 reset
+  - pcie-rst2 :port2 reset
+- phys: list of PHY specifiers (used by generic PHY framework).
+- phy-names : must be "pcie-phy0", "pcie-phy1", "pcie-phyN".. based on the
+  number of PHYs as specified in *phys* property.
+- power-domains: A phandle and power domain specifier pair to the power domain
+  which is responsible for collapsing and restoring power to the peripheral.
+- bus-range: Range of bus numbers associated with this controller.
+- ranges:
+  - The first three entries are expected to translate the addresses for the 
root
+port registers, which are referenced by the assigned-addresses property of
+the root port nodes (see below).
+  - The remaining entries setup the mapping for the standard I/O and memory
+   regions.
+  Please refer to the standard PCI bus binding document for a more detailed
+  explanation.
+
+In addition, the device tree node must have sub-nodes describing each
+PCIe port interface, having the following mandatory properties:
+
+Required properties:
+- device_type: Must be "pci"
+- assigned-addresses: Address and size of the port configuration registers
+- reg: Only the first four bytes are used to refer to the correct bus number
+  and device number.
+- #address-cells: Must be 3
+- #size-cells: Must be 2
+- #interrupt-cells: Must be 1
+- interrupt-map-mask and interrupt-map: Standard PCI IRQ mapping properties
+  Please refer to the standard PCI bus binding document for a more detailed
+  explanation.
+- ranges: Sub-ranges distributed from the PCIe controller node. An empty
+  property is sufficient.
+- num-lanes: Number of lanes to use for this port.
+
+Examples:
+
+   hifsys: syscon@1a00 {
+   compatible = "mediatek,mt7623-hifsys", "syscon";


If you want to put the clock controller subsystem node to the example, 
please use a valid compatible, for example:


compatible = "mediatek,mt7623-hifsys",
 "mediatek,mt2701-hifsys",
 "syscon";

Thanks,
Matthias


+   reg = <0 0x1a00 0 0x1000>;
+   #clock-cells = <1>;
+   #reset-cells = <1>;
+   };
+
+   pcie: pcie-controller@1a14 {
+   compatible = "mediatek,mt7623-pcie";
+   device_type = "pci";
+   reg = <0 0x1a14 0 0x1000>; /* PCIe shared registers */
+   #address-cells = <3>;
+   #size-cells = <2>;
+   #interrupt-cells = <1>;
+   interrupts = ,
+,
+;
+   interrupt-names = "pcie-int0", "pcie-int1", "pcie-int2";
+   interrupt-map-mask = <0xf800 0 0 0>;
+   interrupt-map = <0x 0 0 0  GIC_SPI 193 IRQ_TYPE_NONE>,
+   <0x0800 0 0 0  GIC_SPI 194

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

Re: [PATCH v3 2/2] dt-bindings: pcie: Add documentation for Mediatek PCIe

20 matches

Site Navigation

Mail list logo

Footer information