Re: [PATCH 2/3] net: qcom/emac: do not reset the EMAC during initialization
From: Timur Tabi Date: Fri, 23 Jun 2017 13:37:58 -0500 > On 06/23/2017 01:00 PM, David Miller wrote: >> What if the boot loader or something else left the chip in >> a weird state? > > We depend on the boot loader leaving the NIC in a very specific state > already, otherwise the driver can't initialize the hardware. The > firmware has to pre-initialize the EMAC for us. > > Not only that, but the driver was resetting the MAC *after* > programming the clocks (on non-ACPI systems) and initializing both > PHYs. > >> I'm not applying this. >> If it's correct, the explanation in this commit message need >> to be imporved. The change must be better justified. > > Since this is for ACPI systems, I could do this: > > if (!has_acpi_companion(&pdev->dev)) > emac_mac_reset(adpt); > > But at the very least, the call should be moved to earlier in the > function. Please just explain the ACPI situation in the commit log message and resubmit the series. Thanks.
Re: [PATCH 2/3] net: qcom/emac: do not reset the EMAC during initialization
On 06/23/2017 01:00 PM, David Miller wrote: What if the boot loader or something else left the chip in a weird state? We depend on the boot loader leaving the NIC in a very specific state already, otherwise the driver can't initialize the hardware. The firmware has to pre-initialize the EMAC for us. Not only that, but the driver was resetting the MAC *after* programming the clocks (on non-ACPI systems) and initializing both PHYs. I'm not applying this. If it's correct, the explanation in this commit message need to be imporved. The change must be better justified. Since this is for ACPI systems, I could do this: if (!has_acpi_companion(&pdev->dev)) emac_mac_reset(adpt); But at the very least, the call should be moved to earlier in the function. -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH 2/3] net: qcom/emac: do not reset the EMAC during initialization
From: Timur Tabi Date: Thu, 22 Jun 2017 13:05:31 -0500 > It doesn't make sense to reset the EMAC in the middle of initializing > it during the probe. > > Tested-by: Richard Ruigrok > Signed-off-by: Timur Tabi Why not? What if the boot loader or something else left the chip in a weird state? I'm not applying this. If it's correct, the explanation in this commit message need to be imporved. The change must be better justified.
[PATCH 2/3] net: qcom/emac: do not reset the EMAC during initialization
It doesn't make sense to reset the EMAC in the middle of initializing it during the probe. Tested-by: Richard Ruigrok Signed-off-by: Timur Tabi --- drivers/net/ethernet/qualcomm/emac/emac.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c index 77c5c92..746d94e 100644 --- a/drivers/net/ethernet/qualcomm/emac/emac.c +++ b/drivers/net/ethernet/qualcomm/emac/emac.c @@ -683,8 +683,6 @@ static int emac_probe(struct platform_device *pdev) goto err_undo_mdiobus; } - emac_mac_reset(adpt); - /* set hw features */ netdev->features = NETIF_F_SG | NETIF_F_HW_CSUM | NETIF_F_RXCSUM | NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_HW_VLAN_CTAG_RX | -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
