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

[Florian Fainelli <f.fainelli@gmail.com>]

On 1/5/20 5:34 PM, Vladimir Oltean wrote:
> From: Claudiu Manoil <claudiu.manoil@nxp.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