On Wed, Nov 01, 2023 at 05:57:50PM +0100, Heiner Kallweit wrote: > On 01.11.2023 13:57, Christian Marangi wrote: > > On Wed, Nov 01, 2023 at 02:01:33PM +0100, Heiner Kallweit wrote: > >> On 01.11.2023 13:36, Christian Marangi wrote: > >>> From: Robert Marko <robimarko@xxxxxxxxx> > >>> > >>> Aquantia PHY-s require firmware to be loaded before they start operating. > >>> It can be automatically loaded in case when there is a SPI-NOR connected > >>> to Aquantia PHY-s or can be loaded from the host via MDIO. > >>> > >>> This patch adds support for loading the firmware via MDIO as in most cases > >>> there is no SPI-NOR being used to save on cost. > >>> Firmware loading code itself is ported from mainline U-boot with cleanups. > >>> > >>> The firmware has mixed values both in big and little endian. > >>> PHY core itself is big-endian but it expects values to be in little-endian. > >>> The firmware is little-endian but CRC-16 value for it is stored at the end > >>> of firmware in big-endian. > >>> > >>> It seems the PHY does the conversion internally from firmware that is > >>> little-endian to the PHY that is big-endian on using the mailbox > >>> but mailbox returns a big-endian CRC-16 to verify the written data > >>> integrity. > >>> > >>> Co-developed-by: Christian Marangi <ansuelsmth@xxxxxxxxx> > >>> Signed-off-by: Robert Marko <robimarko@xxxxxxxxx> > >>> Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx> > >>> --- > >>> Changes v2: > >>> - Move out of RFC > >>> - Address sanity check for offsets > >>> - Add additional comments on firmware load check > >>> - Fix some typo > >>> - Capitalize CRC in comments > >>> - Rename load_sysfs to load_fs > >>> > >> > >> To make the driver better maintainable: can the firmware handling code > >> be placed in a separate source code file, similar to what has been done > >> for the hwmon part? > >> If yes, then this could also be the right time to move the aquantia > >> driver to an own subdirectory. > >> > > > > Sure! Np for me just is it really worth it? hwmod is a bigger one but > > this is really a few functions. > > > r8169_firmware.c is even smaller and I've never regretted having it factored > out. Whether it makes sense depends on how much you share with the main module > and how the API is structured that you provide to the main module. > So I don't say you have to do it, I'm just saying it's worth considering it. > Already done! Will be part of this series with v3 :D > > Anyway if requested, I will move in v3 the driver to a dedicated > > directory and move the function to a separate file! > > > -- Ansuel