netdev - Re: [PATCH v5 net-next 5/9] enetc: Make MDIO accessors more generic and export to include/linux/fsl

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8718ea22-d1aa-fe58-bd69-521eeee5190a@gmail.com>
Date:   Mon, 6 Jan 2020 11:35:14 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>, davem@...emloft.net,
        jakub.kicinski@...ronome.com, linux@...linux.org.uk,
        andrew@...n.ch, vivien.didelot@...il.com
Cc:     alexandru.marginean@....com, claudiu.manoil@....com,
        xiaoliang.yang_1@....com, yangbo.lu@....com,
        netdev@...r.kernel.org, alexandre.belloni@...tlin.com,
        horatiu.vultur@...rochip.com, UNGLinuxDriver@...rochip.com,
        Vladimir Oltean <vladimir.oltean@....com>
Subject: Re: [PATCH v5 net-next 5/9] enetc: Make MDIO accessors more generic
 and export to include/linux/fsl

On 1/5/20 5:34 PM, Vladimir Oltean wrote:
> From: Claudiu Manoil <claudiu.manoil@....com>
> 
> Within the LS1028A SoC, the register map for the ENETC MDIO controller
> is instantiated a few times: for the central (external) MDIO controller,
> for the internal bus of each standalone ENETC port, and for the internal
> bus of the Felix switch.
> 
> Refactoring is needed to support multiple MDIO buses from multiple
> drivers. The enetc_hw structure is made an opaque type and a smaller
> enetc_mdio_priv is created.
> 
> 'mdio_base' - MDIO registers base address - is being parameterized, to
> be able to work with different MDIO register bases.
> 
> The ENETC MDIO bus operations are exported from the fsl-enetc-mdio
> kernel object, the same that registers the central MDIO controller (the
> dedicated PF). The ENETC main driver has been changed to select it, and
> use its exported helpers to further register its private MDIO bus. The
> DSA Felix driver will do the same.

This series has already been applied so this may be food for thought at
this point, but why was not the solution to create a standalone mii_bus
driver and have all consumers be pointed it?

It is not uncommon for MDIO controllers to be re-used and integrated
within a larger block and when that happens whoever owns the largest
address space, say the Ethernet MAC can request the large resource
region and the MDIO bus controler can work on that premise, that's what
we did with genet/bcmmii.c and mdio-bcm-unimac.c for instance (so we
only do an ioremap, not request_mem_region + ioremap).

Your commit message does not provide a justification for why this
abstraction (mii_bus) was not suitable or considered here. Do you think
that could be changed?

Thanks!
-- 
Florian