| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 8 Aug 2012 12:39:16 +0000
From: Arnd Bergmann <arnd@...db.de>
To: Ian Molton <ian.molton@...ethink.co.uk>
Cc: David Miller <davem@...emloft.net>,
linux-arm-kernel@...ts.infradead.org, andrew@...n.ch,
thomas.petazzoni@...e-electrons.com, ben.dooks@...ethink.co.uk,
netdev@...r.kernel.org
Subject: Re: [PATCH v3 0/7] mv643xx.c: Add basic device tree support.
On Wednesday 08 August 2012, Ian Molton wrote:
> The SMI / PHY stuff should look very similar, so I'm happy with something
> like:
>
> mdio@...0 {
> #address-cells = <1>;
> #size-cells = <1>;
> device_type = "mdio";
> compatible = "marvell,mv643xx-mdio";
> phy0: ethernet-phy@0 {
> device_type = "ethernet-phy";
> compatible = "marvell,whatever";
> interrupts = <76>;
> interrupt-parent = <&mpic>;
> reg = <0 32>; // Auto probed phy addr
> };
>
> phy1: ethernet-phy@3 {
> device_type = "ethernet-phy";
> compatible = "marvell,whatever";
> interrupts = <77>;
> interrupt-parent = <&mpic>;
> reg = <3 1>; // specified phy addr
> };
>
> ... and so on.
> }
>
> Where we can use the reg parameter to allow auto-probing, by
> specifying a size of 32 (32 phy addrs max).
I don't understand the auto-probed phy address. What is the purpose of that?
If possible, I think we should keep using #size-cells=<0>, which would
make the method you describe impossible. It might still work if you just
leave out the "reg" property for that node.
I also don't understand how the phy driver would locate ethernet-phy@0
on the bus if it does not know the address.
> The ethernet driver itself is more complicated:
>
> We have the following considerations:
>
> * we have one MDIO bus, typically, shared between all the MACs / PHYs.
> * each ethernet device can multiple ports (up to three), each with its
> own MAC/PHY.
> * MAC <-> PHY mapping can be specified, probed (ugh!) or a (gah!)
> mix of the two.
> * existing D-T users, albeit not well documented / code complete.
> * some port address ranges overlap (MIB counters, MCAST / UNICAST
> tables, etc.
>
> The existing ethernet-group idea only works because the current
> platform-device based driver doesnt really do proper resource
> management, and thus the MAC registers are actually mapped by
> the MDIO driver.
>
> I don't think that preserving this bad behaviour is a good idea, which
> leaves us with two choices:
>
> 1) My preferred solution - allow each device to specify up to three
> interrupts, MACs, and PHYs. This is clean in that it doesnt require
> multiply instantiating a driver three times over the same address
> space.
>
> ethernet@...0 {
> compatible = "marvell,mv643xx-eth";
> reg = <0x2400 0x1c00>
> interrupt_parent = <&mpic>;
> ports = <3>;
> interrupts = <4>, <5>, <6>;
> phys = <&phy0>, <&phy1>, <&phy2>;
> };
>
> ethernet@...0 {
> compatible = "marvell,mv643xx-eth";
> reg = <0x6400 0x1c00>
> interrupt_parent = <&mpic>;
> ports = <1>;
> interrupts = <4>;
> phys = <&phy3>;
> };
>
> Note that the address is 2400, not 2000 - since this driver no longer
> would share its address range with the MDIO driver.
>
> This method would require a small amount of rework in the driver to
> set up <n> ports, rather than just one.
This looks quite nice, but it is still very much incompatible with the
existing binding. Obviously we can abandon an existing binding and
introduce a second one for the same hardware, but that should not
be taken lightly.
> 2) Create some kind of pseudo-ethernet group device that manages
> all the work for some sort of lightweight ethernet device, one per
> port. This can never be done cleanly since the port address ranges
> overlap:
>
> pseudo_eth@...0 {
> #address-cells = <1>;
> #size-cells = <0>;
> compatible = "marvell,mv643xx-shared-eth"
> reg = <0x2400 0x1c00>;
>
> ethernet@0 {
> compatible = "marvell,mv643xx-port";
> interrupts = <4>;
> interrupt_parent = <&mpic>;
> phy = <&phy0>;
> };
>
> ethernet@1 {
> compatible = "marvell,mv643xx-port";
> interrupts = <5>;
> interrupt_parent = <&mpic>;
> phy = <&phy1>;
> };
>
> ethernet@2 {
> compatible = "marvell,mv643xx-port";
> interrupts = <6>;
> interrupt_parent = <&mpic>;
> phy = <&phy2>;
> };
> }
> pseudo_eth@...0 {
> #address-cells = <1>;
> #size-cells = <0>;
> compatible = "marvell,mv643xx-shared-eth"
> reg = <0x6400 0x1c00>;
>
> ethernet@0 {
> compatible = "marvell,mv643xx-port";
> interrupts = <4>;
> interrupt_parent = <&mpic>;
> phy = <&phy3>;
> };
> };
This looks almost compatible with the existing binding, which is
good. I would in fact recommend to use the actual "compatible"
strings from the binding. More generally speaking, you should not
use wildcards in those strings anyway, so always use
"marvell,mv64360-eth" instead of "marvell,mv64x60-eth" or
"marvell,mv643xx-eth". If you have multiple chips that are
completely compatible, put use the identifier for the older one.
I don't fully understand your concern with the overlapping
registers, mostly because I still don't know all the combinations
that are actually valid here. Let me try to say what I understood
so far, and you can correct me if that's wrong:
* A system can have multiple instances of an mv64360 ethernet
block, with a register area of 0x2000 bytes.
* Each such block can have three MACs and three PHYs.
* The first 0x400 bytes in the register space control the three
PHYs and the remaining registers control the MACs.
* While this is meant to be used in a way that you assign
the each of the three PHYs to one of the MACs, this is not
always done, and sometimes you use a different PHY (?), or
one from a different instance of the mv64360 ethernet block
on the same SoC?.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists