Re: [PATCH v5 net-next 5/9] enetc: Make MDIO accessors more generic and export to include/linux/fsl
From: Florian Fainelli <f.fainelli@gmail.com>
Date: 2020-01-06 19:35:25
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