| Message ID | 1481658930-565-1-git-send-email-timur@codeaurora.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 12/13/2016 11:55 AM, Timur Tabi wrote: > On ACPI systems, clocks are not available to drivers directly. They are > handled exclusively by ACPI and/or firmware, so there is no clock driver. > Calls to clk_get() always fail, so we should not even attempt to claim > any clocks on ACPI systems. > > Signed-off-by: Timur Tabi <timur@codeaurora.org> > --- > drivers/net/ethernet/qualcomm/emac/emac.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c > index ae32f85..b1c1cdc 100644 > --- a/drivers/net/ethernet/qualcomm/emac/emac.c > +++ b/drivers/net/ethernet/qualcomm/emac/emac.c > @@ -627,11 +627,12 @@ static int emac_probe(struct platform_device *pdev) > if (ret) > goto err_undo_netdev; > > - /* initialize clocks */ > - ret = emac_clks_phase1_init(pdev, adpt); > - if (ret) { > - dev_err(&pdev->dev, "could not initialize clocks\n"); > - goto err_undo_netdev; > + if (!has_acpi_companion(&pdev->dev)) { Is there a reason why the check is not moved down inwo emac_clks_phase{1,2}_init functions? Do you anticipate other ACPI-related changes in the future that would warrant having this check moved at a higher level?
On 12/13/2016 03:46 PM, Florian Fainelli wrote: > Is there a reason why the check is not moved down inwo > emac_clks_phase{1,2}_init functions? Do you anticipate other > ACPI-related changes in the future that would warrant having this check > moved at a higher level? No, this is the last ACPI-related change that I expect. I could move the check into those functions, but I don't see how that's any different than what I'm doing now. My way avoids calling a function altogether, your way calls into a function only to have it return immediately. But I don't have any strong feelings either way. I will change it if you want me to.
On 12/13/2016 01:54 PM, Timur Tabi wrote: > On 12/13/2016 03:46 PM, Florian Fainelli wrote: >> Is there a reason why the check is not moved down inwo >> emac_clks_phase{1,2}_init functions? Do you anticipate other >> ACPI-related changes in the future that would warrant having this check >> moved at a higher level? > > No, this is the last ACPI-related change that I expect. I could move > the check into those functions, but I don't see how that's any different > than what I'm doing now. My way avoids calling a function altogether, > your way calls into a function only to have it return immediately. > > But I don't have any strong feelings either way. I will change it if > you want me to. No strong feelings either, it just seems easier and safer to move the check down in the function and make it return success rather than potentially affecting the error path within the caller of emac_clks_phase{1,2}_init here.
On 12/13/2016 04:02 PM, Florian Fainelli wrote: > No strong feelings either, it just seems easier and safer to move the > check down in the function and make it return success rather than > potentially affecting the error path within the caller of > emac_clks_phase{1,2}_init here. I suppose that makes sense. I'll post a V2.
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c index ae32f85..b1c1cdc 100644 --- a/drivers/net/ethernet/qualcomm/emac/emac.c +++ b/drivers/net/ethernet/qualcomm/emac/emac.c @@ -627,11 +627,12 @@ static int emac_probe(struct platform_device *pdev) if (ret) goto err_undo_netdev; - /* initialize clocks */ - ret = emac_clks_phase1_init(pdev, adpt); - if (ret) { - dev_err(&pdev->dev, "could not initialize clocks\n"); - goto err_undo_netdev; + if (!has_acpi_companion(&pdev->dev)) { + ret = emac_clks_phase1_init(pdev, adpt); + if (ret) { + dev_err(&pdev->dev, "could not initialize clocks\n"); + goto err_undo_netdev; + } } netdev->watchdog_timeo = EMAC_WATCHDOG_TIME; @@ -655,11 +656,12 @@ static int emac_probe(struct platform_device *pdev) if (ret) goto err_undo_mdiobus; - /* enable clocks */ - ret = emac_clks_phase2_init(pdev, adpt); - if (ret) { - dev_err(&pdev->dev, "could not initialize clocks\n"); - goto err_undo_mdiobus; + if (!has_acpi_companion(&pdev->dev)) { + ret = emac_clks_phase2_init(pdev, adpt); + if (ret) { + dev_err(&pdev->dev, "could not initialize clocks\n"); + goto err_undo_mdiobus; + } } emac_mac_reset(adpt);
On ACPI systems, clocks are not available to drivers directly. They are handled exclusively by ACPI and/or firmware, so there is no clock driver. Calls to clk_get() always fail, so we should not even attempt to claim any clocks on ACPI systems. Signed-off-by: Timur Tabi <timur@codeaurora.org> --- drivers/net/ethernet/qualcomm/emac/emac.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)