Re: [PATCH v4 12/16] MIPS: lantiq: Add a GPHY driver which uses the RCU syscon-mfd

[Hauke Mehrtens <hauke@xxxxxxxxxx>]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html