| Message ID | 56C4A46C.50506@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wednesday 17 February 2016 16:48:44 Vladimir Murzin wrote: > On 16/02/16 16:10, Vladimir Murzin wrote: > > On 16/02/16 11:01, Arnd Bergmann wrote: > >> On Tuesday 16 February 2016 10:08:14 Vladimir Murzin wrote: > >>> + > >>> + ethernet@40200000 { > >>> + compatible = "smsc,lan9220", "smsc,lan9115"; > >>> + reg = <0x40200000 0x10000>; > >>> + interrupts = <13>; > >>> + interrupt-parent = <&nvic>; > >>> + smsc,irq-active-high; > >>> + }; > >>> +}; > >>> + > >>> > >> > >> This node seems slightly misplaced. Is there some external bus interface > >> that this is connected to? The address suggests that it should be somewhere > >> below the /soc node, and you probably want to list the external bus > >> interface with a "ranges" property that identifies the addresses visibile > >> there, and put the external chip under there. > >> > > > > I might messed it up since the MAC/PHY connects to the same 16-bit > > interface as the 16MB PSRAM external memory and both connected via AHB. > > > > Not sure how it should be expressed, so some help form DT camp would be > > appreciated. > > Does following fixup address your point on where/how ethernet node > should be placed? > diff --git a/arch/arm/boot/dts/mps2-an385.dts b/arch/arm/boot/dts/an385.dts > index 976f86d..50c8d24 100644 > --- a/arch/arm/boot/dts/mps2-an385.dts > +++ b/arch/arm/boot/dts/mps2-an385.dts > @@ -63,14 +63,10 @@ > device_type = "memory"; > reg = <0x21000000 0x1000000>; > }; > +}; > > - ethernet@40200000 { > - compatible = "smsc,lan9220", "smsc,lan9115"; > - reg = <0x40200000 0x10000>; > - interrupts = <13>; > - interrupt-parent = <&nvic>; > - smsc,irq-active-high; > - }; > +&mb { > + ranges = <0 0x40200000 0x10000>; > }; How is the range being set here? The way I read this is: "There is an external bus controller whose single CPU physical address for MMIO is configurable. The chip always connects a lan9220 device to it (as that is in the dtsi file) and nothing else is possible, and the bootloader in this version of the machine has configured the window to be at address 0x40200000." Is that what the hardware does? I would have expected the opposite, with the external bus interface being hardwired to one or more physical addresses (more than one if you have multiple chip-selects), and then allow to connect different devices, which are in the .dts file, while the bus controller is defined in the .dtsi file. Arnd
On 17/02/16 16:58, Arnd Bergmann wrote: > On Wednesday 17 February 2016 16:48:44 Vladimir Murzin wrote: >> On 16/02/16 16:10, Vladimir Murzin wrote: >>> On 16/02/16 11:01, Arnd Bergmann wrote: >>>> On Tuesday 16 February 2016 10:08:14 Vladimir Murzin wrote: >>>>> + >>>>> + ethernet@40200000 { >>>>> + compatible = "smsc,lan9220", "smsc,lan9115"; >>>>> + reg = <0x40200000 0x10000>; >>>>> + interrupts = <13>; >>>>> + interrupt-parent = <&nvic>; >>>>> + smsc,irq-active-high; >>>>> + }; >>>>> +}; >>>>> + >>>>> >>>> >>>> This node seems slightly misplaced. Is there some external bus interface >>>> that this is connected to? The address suggests that it should be somewhere >>>> below the /soc node, and you probably want to list the external bus >>>> interface with a "ranges" property that identifies the addresses visibile >>>> there, and put the external chip under there. >>>> >>> >>> I might messed it up since the MAC/PHY connects to the same 16-bit >>> interface as the 16MB PSRAM external memory and both connected via AHB. >>> >>> Not sure how it should be expressed, so some help form DT camp would be >>> appreciated. >> >> Does following fixup address your point on where/how ethernet node >> should be placed? > > > >> diff --git a/arch/arm/boot/dts/mps2-an385.dts b/arch/arm/boot/dts/an385.dts >> index 976f86d..50c8d24 100644 >> --- a/arch/arm/boot/dts/mps2-an385.dts >> +++ b/arch/arm/boot/dts/mps2-an385.dts >> @@ -63,14 +63,10 @@ >> device_type = "memory"; >> reg = <0x21000000 0x1000000>; >> }; >> +}; >> >> - ethernet@40200000 { >> - compatible = "smsc,lan9220", "smsc,lan9115"; >> - reg = <0x40200000 0x10000>; >> - interrupts = <13>; >> - interrupt-parent = <&nvic>; >> - smsc,irq-active-high; >> - }; >> +&mb { >> + ranges = <0 0x40200000 0x10000>; >> }; > > How is the range being set here? The way I read this is: > > "There is an external bus controller whose single CPU physical > address for MMIO is configurable. The chip always connects > a lan9220 device to it (as that is in the dtsi file) and > nothing else is possible, and the bootloader in this > version of the machine has configured the window to be > at address 0x40200000." > > Is that what the hardware does? > > I would have expected the opposite, with the external bus > interface being hardwired to one or more physical addresses > (more than one if you have multiple chip-selects), and > then allow to connect different devices, which are in the > .dts file, while the bus controller is defined in the > .dtsi file. > Right, I thought in a wrong way, in opposite it makes more sense now. .dtsi /* below the soc/ */ smb { compatible = "simple-bus"; #address-cells = <2>; #size-cells = <1>; ranges = <0 0 0x40200000 0x10000>, <1 0 0xa0000000 0x10000>; }; .dts smb { ethernet@0,0 { compatible = "smsc,lan9220", "smsc,lan9115"; reg = <0 0x0 0x10000>; interrupts = <13>; interrupt-parent = <&nvic>; smsc,irq-active-high; }; and looking again at .dtsi it seems to me that fpgaio should be moved below the soc/ under separate bus interface which would hosts audio and spi too or I keep missing things around device-tree? I appreciate your help on this, thanks! Vladimir > Arnd > > >
On Thursday 18 February 2016 10:11:37 Vladimir Murzin wrote: > > Right, I thought in a wrong way, in opposite it makes more sense now. > > .dtsi > > /* below the soc/ */ > smb { > compatible = "simple-bus"; > #address-cells = <2>; > #size-cells = <1>; > ranges = <0 0 0x40200000 0x10000>, > <1 0 0xa0000000 0x10000>; > }; That looks good, yes. Is 0x10000 the correct maximum addressable size of the external bus in both cases? Intuitively, I would guess that the 0xa0000000 range might be much wider. > .dts > > smb { > ethernet@0,0 { > compatible = "smsc,lan9220", "smsc,lan9115"; > reg = <0 0x0 0x10000>; > interrupts = <13>; > interrupt-parent = <&nvic>; > smsc,irq-active-high; > }; > > > and looking again at .dtsi it seems to me that fpgaio should be moved > below the soc/ under separate bus interface which would hosts audio and > spi too or I keep missing things around device-tree? > I don't see the audio and spi nodes, so I'm not sure where exactly you would put them. Ideally those things should be visible from a block diagram in the datasheet. Arnd
On 18/02/16 10:45, Arnd Bergmann wrote: > On Thursday 18 February 2016 10:11:37 Vladimir Murzin wrote: >> >> Right, I thought in a wrong way, in opposite it makes more sense now. >> >> .dtsi >> >> /* below the soc/ */ >> smb { >> compatible = "simple-bus"; >> #address-cells = <2>; >> #size-cells = <1>; >> ranges = <0 0 0x40200000 0x10000>, >> <1 0 0xa0000000 0x10000>; >> }; > > That looks good, yes. > > Is 0x10000 the correct maximum addressable size of the external bus > in both cases? > > Intuitively, I would guess that the 0xa0000000 range might > be much wider. There is only Ethernet connected to this bus (apart from PSRAM), so it might be wider, but there is no indication of this in documentation. > >> .dts >> >> smb { >> ethernet@0,0 { >> compatible = "smsc,lan9220", "smsc,lan9115"; >> reg = <0 0x0 0x10000>; >> interrupts = <13>; >> interrupt-parent = <&nvic>; >> smsc,irq-active-high; >> }; >> >> >> and looking again at .dtsi it seems to me that fpgaio should be moved >> below the soc/ under separate bus interface which would hosts audio and >> spi too or I keep missing things around device-tree? >> > > I don't see the audio and spi nodes, so I'm not sure where exactly > you would put them. I just keep things simple ;) > > Ideally those things should be visible from a block diagram in the > datasheet. Indeed, block diagram indicates all them as a "FPGA APB subsystem" and clearly draws a line indicating a bus those devices are connected to. After your point about lan9220, it looks clearer to me to express that subsystem outside of soc/ node indicating bus interface, so it would match to those drawings closely. Cheers Vladimir > > Arnd > > >
On Thursday 18 February 2016 11:13:06 Vladimir Murzin wrote: > On 18/02/16 10:45, Arnd Bergmann wrote: > > On Thursday 18 February 2016 10:11:37 Vladimir Murzin wrote: > >> > >> Right, I thought in a wrong way, in opposite it makes more sense now. > >> > >> .dtsi > >> > >> /* below the soc/ */ > >> smb { > >> compatible = "simple-bus"; > >> #address-cells = <2>; > >> #size-cells = <1>; > >> ranges = <0 0 0x40200000 0x10000>, > >> <1 0 0xa0000000 0x10000>; > >> }; > > > > That looks good, yes. > > > > Is 0x10000 the correct maximum addressable size of the external bus > > in both cases? > > > > Intuitively, I would guess that the 0xa0000000 range might > > be much wider. > > There is only Ethernet connected to this bus (apart from PSRAM), so it > might be wider, but there is no indication of this in documentation. I see this called "ahb_to_extmem16" in the documentation, which indicates that it might be use 16 bits of address space, which would match the 64K you listed. For SSRAM1 / SSRAM2 / SSRAM3, a 8 MB address space is mentioned and 16 MB for external PSRAM at 0x21000000. Arnd
On 18/02/16 12:16, Arnd Bergmann wrote: > On Thursday 18 February 2016 11:13:06 Vladimir Murzin wrote: >> On 18/02/16 10:45, Arnd Bergmann wrote: >>> On Thursday 18 February 2016 10:11:37 Vladimir Murzin wrote: >>>> >>>> Right, I thought in a wrong way, in opposite it makes more sense now. >>>> >>>> .dtsi >>>> >>>> /* below the soc/ */ >>>> smb { >>>> compatible = "simple-bus"; >>>> #address-cells = <2>; >>>> #size-cells = <1>; >>>> ranges = <0 0 0x40200000 0x10000>, >>>> <1 0 0xa0000000 0x10000>; >>>> }; >>> >>> That looks good, yes. >>> >>> Is 0x10000 the correct maximum addressable size of the external bus >>> in both cases? >>> >>> Intuitively, I would guess that the 0xa0000000 range might >>> be much wider. >> >> There is only Ethernet connected to this bus (apart from PSRAM), so it >> might be wider, but there is no indication of this in documentation. > > I see this called "ahb_to_extmem16" in the documentation, which indicates > that it might be use 16 bits of address space, which would match > the 64K you listed. > > For SSRAM1 / SSRAM2 / SSRAM3, a 8 MB address space is mentioned > and 16 MB for external PSRAM at 0x21000000. > Right. I'll incorporate changes in the next version. Thanks for lessons given and your time! Cheers Vladimir > Arnd > > > >
diff --git a/arch/arm/boot/dts/mps2-an385.dts b/arch/arm/boot/dts/an385.dts index 976f86d..50c8d24 100644 --- a/arch/arm/boot/dts/mps2-an385.dts +++ b/arch/arm/boot/dts/mps2-an385.dts @@ -63,14 +63,10 @@ device_type = "memory"; reg = <0x21000000 0x1000000>; }; +}; - ethernet@40200000 { - compatible = "smsc,lan9220", "smsc,lan9115"; - reg = <0x40200000 0x10000>; - interrupts = <13>; - interrupt-parent = <&nvic>; - smsc,irq-active-high; - }; +&mb { + ranges = <0 0x40200000 0x10000>; }; &uart0 { diff --git a/arch/arm/boot/dts/mps2.dtsi b/arch/arm/boot/dts/mps2.dtsi index 5d2c539..1673472 100644 --- a/arch/arm/boot/dts/mps2.dtsi +++ b/arch/arm/boot/dts/mps2.dtsi @@ -223,5 +223,21 @@ default-state = "off"; }; }; + + }; + + mb: smb { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges; + + ethernet@0 { + compatible = "smsc,lan9220", "smsc,lan9115"; + reg = <0x0 0x10000>; + interrupts = <13>; + interrupt-parent = <&nvic>; + smsc,irq-active-high; + }; }; };