On 01/07/2022 08:47, Claudiu Beznea - M18063 wrote:
> On 01.07.2022 09:58, Conor Dooley wrote:
>> To date, the Microchip PolarFire SoC (MPFS) has been using the
>> cdns,macb compatible, however the generic device does not have reset
>> support. Add a new compatible & .data for MPFS to hook into the reset
>> functionality added for zynqmp support (and make the zynqmp init
>> function generic in the process).
>>
>> Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
>> ---
>> drivers/net/ethernet/cadence/macb_main.c | 25 +++++++++++++++++-------
>> 1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>> index d89098f4ede8..325f0463fd42 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -4689,33 +4689,32 @@ static const struct macb_config np4_config = {
>> .usrio = &macb_default_usrio,
>> };
>>
>> -static int zynqmp_init(struct platform_device *pdev)
>> +static int init_reset_optional(struct platform_device *pdev)
>
> It doesn't sound like a good name for this function but I don't have
> something better to propose.
It's better than zynqmp_init, but yeah...
>
>> {
>> struct net_device *dev = platform_get_drvdata(pdev);
>> struct macb *bp = netdev_priv(dev);
>> int ret;
>>
>> if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII) {
>> - /* Ensure PS-GTR PHY device used in SGMII mode is ready */
>> + /* Ensure PHY device used in SGMII mode is ready */
>> bp->sgmii_phy = devm_phy_optional_get(&pdev->dev, NULL);
>>
>> if (IS_ERR(bp->sgmii_phy)) {
>> ret = PTR_ERR(bp->sgmii_phy);
>> dev_err_probe(&pdev->dev, ret,
>> - "failed to get PS-GTR PHY\n");
>> + "failed to get SGMII PHY\n");
>> return ret;
>> }
>>
>> ret = phy_init(bp->sgmii_phy);
>> if (ret) {
>> - dev_err(&pdev->dev, "failed to init PS-GTR PHY: %d\n",
>> + dev_err(&pdev->dev, "failed to init SGMII PHY: %d\n",
>> ret);
>> return ret;
>> }
>> }
>>
>> - /* Fully reset GEM controller at hardware level using zynqmp-reset driver,
>> - * if mapped in device tree.
>> + /* Fully reset controller at hardware level if mapped in device tree
>> */
>
> The new comment can fit on a single line.
>
>> ret = device_reset_optional(&pdev->dev);
>> if (ret) {
>> @@ -4737,7 +4736,7 @@ static const struct macb_config zynqmp_config = {
>> MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH,
>> .dma_burst_length = 16,
>> .clk_init = macb_clk_init,
>> - .init = zynqmp_init,
>> + .init = init_reset_optional,
>> .jumbo_max_len = 10240,
>> .usrio = &macb_default_usrio,
>> };
>> @@ -4751,6 +4750,17 @@ static const struct macb_config zynq_config = {
>> .usrio = &macb_default_usrio,
>> };
>>
>> +static const struct macb_config mpfs_config = {
>> + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
>> + MACB_CAPS_JUMBO |
>> + MACB_CAPS_GEM_HAS_PTP,
>
> Except for zynqmp and default_gem_config the rest of the capabilities for
> other SoCs are aligned something like this:
>
> + .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
> + MACB_CAPS_JUMBO |
> + MACB_CAPS_GEM_HAS_PTP,
>
> To me it looks better to have you caps aligned this way.
Yeah, I picked that b/c I copied straight from the default config.
I have no preference, but if you're not a fan of the default...
