Messages in this thread | | | From | Marco von Rosenberg <> | Subject | Re: [PATCH] net: phy: broadcom: Wire suspend/resume for BCM54612E | Date | Fri, 03 Nov 2023 18:37:00 +0100 |
| |
On Friday, November 3, 2023 4:39:55 AM CET Florian Fainelli wrote: > We have an unconditional call to __phy_resume() in phy_start() and we > should always have a call to phy_start() regardless of the path though > you have a point Andrew that we should ensure that by the time > phy_init_hw() is called we have taken the device out of IDDQ-SR. > > > I agree with all of your points. This is just one way which happens to > > solve this specific problem. Of course it might be asymmetric to see the > > patch as a solution to my problem. However is there anything > > fundamentally wrong with adding suspend/resume callbacks? I see some > > other drivers having these callbacks defined and some not (it seems a bit > > inconsistent throughout the drivers in broadcom.c to be honest). > > > > I'm wondering if I should just omit this whole "motivation" paragraph in > > the commit message and just use the commit message of commit 38b6a9073007 > > ("net: phy: broadcom: Wire suspend/resume for BCM50610 and BCM50610M") as > > a template. I mean, regardless of my motivation, I would say it makes > > sense for this PHY to support suspend and resume. > > I would remove the motivation aspect from the paragraph and we could > also improve the driver a bit to ensure that IDDQ-SR is disabled upon > config_init(). Other than that your patch is just fine with me. Can you > re-submit in a few days when net-next opens again?
Ok, I'll re-submit the patch when net-next is open again with an updated commit message. And I agree, disabling IDDQ-SR in config_init() would make sense for a future patch since this would fix this potential issue also for other PHYs.
Marco
| |