Re: [net-next v5 2/6] ARM: dts: nxp: mxs: Adjust the imx28.dtsi L2 switch description

[Lukasz Majewski <lukma@denx.de>]

[-- Attachment #1: Type: text/plain, Size: 3031 bytes --]

Hi Stefan,

> Hi Andrew,
> 
> Am 16.04.25 um 23:58 schrieb Andrew Lunn:
> >>> -		eth_switch: switch@800f8000 {
> >>> -			reg = <0x800f8000 0x8000>;
> >>> +		eth_switch: switch@800f0000 {
> >>> +			compatible = "nxp,imx28-mtip-switch";
> >>> +			reg = <0x800f0000 0x20000>;
> >>> +			interrupts = <100>, <101>, <102>;
> >>> +			clocks = <&clks 57>, <&clks 57>, <&clks
> >>> 64>, <&clks 35>;
> >>> +			clock-names = "ipg", "ahb", "enet_out",
> >>> "ptp"; status = "disabled";  
> >> from my understanding of device tree this file should describe the
> >> hardware, not the software implementation. After this change the
> >> switch memory region overlaps the existing mac0 and mac1 nodes.
> >>
> >> Definition in the i.MX28 reference manual:
> >> ENET MAC0 ENET 0x800F0000 - 0x800F3FFF 16KB
> >> ENET MAC1 ENET 0x800F4000 - 0x800F7FFF 16KB
> >> ENT Switch SWITCH 0x800F8000 - 0x800FFFFF 32KB
> >>
> >> I'm not the expert how to solve this properly. Maybe two node
> >> references to mac0 and mac1 under eth_switch in order to allocate
> >> the memory regions separately.  
> > I get what you are saying about describing the hardware, but...
> >
> > The hardware can be used in two different ways.
> >
> > 1) Two FEC devices, and the switch it left unused.
> >
> > For this, it makes sense that each FEC has its own memory range,
> > there are two entries, and each has a compatible, since there are
> > two devices.
> >
> > 2) A switch and MAC conglomerate device, which makes use of all
> > three blocks in a single driver.
> >
> > The three hardware blocks have to be used as one consistent whole,
> > by a single driver. There is one compatible for the whole. Given the
> > ranges are contiguous, it makes little sense to map them
> > individually, it would just make the driver needlessly more complex.
> >
> > It should also be noted that 1) and 2) are mutually exclusive, so i
> > don't think it matters the address ranges overlap. Bad things are
> > going to happen independent of this if you enable both at once.
> >
> >        Andrew  
> i wasn't aware how critical possible overlapping memory regions are.
> I was just surprised because it wasn't mention in the commit message.
> As long as everyone is fine with this approach, please ignore my last
> comment.
> 

Just for the record - there was an attempt to "re-use" FEC enet driver
in switch [1], but this approach has been rejected as one not very
robust and "clear by design" (i.e. similar to cpsw_new.c driver).

And I do agree with Andrew - that approach presented in this patch set
is the correct one.

Links:
[1] -
https://source.denx.de/linux/linux-imx28-l2switch/-/commits/imx28-v5.12-L2-upstream-switchdev-RFC_v1

> Regards
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]