On 06/20/2017 01:36 AM, Andy Shevchenko wrote: > On Tue, Jun 20, 2017 at 1:26 AM, Hauke Mehrtens <hauke@xxxxxxxxxx> wrote: >> From: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> >> >> Compared to the old xrx200_phy_fw driver the new version has multiple >> enhancements. The name of the firmware files does not have to be added >> to all .dts files anymore - one now configures the GPHY mode (FE or GE) >> instead. Each GPHY can now also boot separate firmware (thus mixing of >> GE and FE GPHYs is now possible). >> The new implementation is based on the RCU syscon-mfd and uses the >> reeset_controller framework instead of raw RCU register reads/writes. >> > > Couple of comments, otherwise FWIW, > Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > >> Signed-off-by: Hauke Mehrtens <hauke@xxxxxxxxxx> >> --- >> .../devicetree/bindings/mips/lantiq/rcu-gphy.txt | 36 +++ >> arch/mips/lantiq/xway/sysctrl.c | 6 +- >> drivers/soc/lantiq/Makefile | 1 + >> drivers/soc/lantiq/gphy.c | 261 +++++++++++++++++++++ >> include/dt-bindings/mips/lantiq_rcu_gphy.h | 15 ++ >> 5 files changed, 317 insertions(+), 2 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/mips/lantiq/rcu-gphy.txt >> create mode 100644 drivers/soc/lantiq/gphy.c >> create mode 100644 include/dt-bindings/mips/lantiq_rcu_gphy.h ...... >> + >> + /* >> + * GPHY cores need the firmware code in a persistent and contiguous >> + * memory area with a 16 kB boundary aligned start address. >> + */ >> + priv->size = fw->size + XRX200_GPHY_FW_ALIGN; >> + >> + priv->fw_addr = dma_alloc_coherent(dev, priv->size, >> + &priv->dma_addr, GFP_KERNEL); > > dmam_ ? Thanks, didn't know this functions was looking for something with devm_ prefix, but dmam_ makes the code simpler. .... >> + >> + ret = xway_gphy_load(dev, priv, &fw_addr); >> + if (ret) > > You need to disable and unprepare a clock. Done >> + return ret; >> + .... > >> + dma_free_coherent(dev, priv->size, priv->fw_addr, priv->dma_addr); > > It will go away if dmam_ variant is in use. Done >> + >> + ret = unregister_reboot_notifier(&priv->gphy_reboot_nb); >> + if (ret) >> + dev_warn(dev, "Failed to unregister reboot notifier\n"); ... Hauke