Hi Johon, Christophe, On Mon, 4 Mar 2024 at 14:48, Johan Hovold <johan@xxxxxxxxxx> wrote: > > On Sat, Mar 02, 2024 at 10:05:46PM +0530, Anand Moon wrote: > > On Sat, 2 Mar 2024 at 21:19, Christophe JAILLET > > <christophe.jaillet@xxxxxxxxxx> wrote: > > > Le 01/03/2024 à 20:38, Anand Moon a écrit : > > > > > The devm_clk_get_enabled() helpers: > > > > - call devm_clk_get() > > > > - call clk_prepare_enable() and register what is needed in order to > > > > call clk_disable_unprepare() when needed, as a managed resource. > > > > > > > > This simplifies the code and avoids the calls to clk_disable_unprepare(). > > > > > > > > While at it, use dev_err_probe consistently, and use its return value > > > > to return the error code. > > > > > @@ -260,25 +248,17 @@ static int exynos_ehci_suspend(struct device *dev) > > > > > > > > exynos_ehci_phy_disable(dev); > > > > > > > > - clk_disable_unprepare(exynos_ehci->clk); > > > > I don't think that removing clk_[en|dis]abble from the suspend and > > > resume function is correct. > > > > > > The goal is to stop some hardware when the system is suspended, in order > > > to save some power. > > Yes correct, > > > > > > Why did you removed it? > > > devm_clk_get_enabled function register callback for clk_prepare_enable > > and clk_disable_unprepare, so when the clock resource is not used it should get > > disabled. > > > > [0] https://elixir.bootlin.com/linux/latest/source/drivers/clk/clk-devres.c#L75 > > > > I have also tested with rtc suspend & resume and did not find any issue. > > You seem to be totally confused about how devres works, and arguing back > after Christophe points this out to you instead of going back and doing > the homework you should have done before posting these patches is really > not OK (e.g. as you're wasting other people's time). > Ok, It seems to have fallen short in my understanding.. > And you clearly did not test these patches enough to confirm that you > didn't break the driver. Ok I missed the failure of the ehci driver. while testing. [root@archl-xu4 alarm]# echo no > /sys/module/printk/parameters/console_suspend [root@archl-xu4 alarm]# echo no > /sys/module/printk/parameters/console_suspend [root@archl-xu4 alarm]# echo no > /sys/module/printk/parameters/console_suspend [root@archl-xu4 alarm]# time rtcwake -s 30 -m mem rtcwake: assuming RTC uses UTC ... rtcwake: wakeup from "mem" using /dev/rtc0 at Mon Mar 4 09:44:25 2024 [11969.792928] PM: suspend entry (deep) [11969.798423] Filesystems sync: 0.003 seconds [11969.819722] Freezing user space processes [11969.825818] Freezing user space processes completed (elapsed 0.003 seconds) [11969.831585] OOM killer disabled. [11969.834586] Freezing remaining freezable tasks [11969.841553] Freezing remaining freezable tasks completed (elapsed 0.002 seconds) [11969.919178] sd 0:0:0:0: [sda] Synchronizing SCSI cache [11970.091681] wake enabled for irq 129 (gpx0-4) [11970.135766] wake enabled for irq 149 (gpx0-3) [11970.157943] samsung-pinctrl 13400000.pinctrl: Setting external wakeup interrupt mask: 0xffffffe7 [11970.179304] Disabling non-boot CPUs ... [11970.276394] s3c2410-wdt 101d0000.watchdog: watchdog disabled [11970.281961] wake disabled for irq 149 (gpx0-3) [11970.288997] phy phy-12130000.phy.6: phy_power_on was called before phy_init [11970.358899] exynos-ohci 12120000.usb: init err (00000000 0000) [11970.363298] exynos-ohci 12120000.usb: can't restart, -75 [11970.368581] usb usb2: root hub lost power or was reset [11970.373819] wake disabled for irq 129 (gpx0-4) [11970.382641] xhci-hcd xhci-hcd.8.auto: xHC error in resume, USBSTS 0x411, Reinit [11970.383237] s3c-rtc 101e0000.rtc: rtc disabled, re-enabling [11970.383355] xhci-hcd xhci-hcd.9.auto: xHC error in resume, USBSTS 0x401, Reinit [11970.383376] usb usb5: root hub lost power or was reset [11970.383396] usb usb6: root hub lost power or was reset [11970.388471] usb usb3: root hub lost power or was reset [11970.416740] usb usb4: root hub lost power or was reset [11970.770122] usb 3-1: reset high-speed USB device number 2 using xhci-hcd [11971.100601] usb 4-1: reset SuperSpeed USB device number 3 using xhci-hcd [11971.569524] usb 3-1.2: reset high-speed USB device number 3 using xhci-hcd [11974.575262] OOM killer enabled. [11974.576964] Restarting tasks ... done. [11974.580608] r8152-cfgselector 6-1: USB disconnect, device number 4 [11974.589302] random: crng reseeded on system resumption [11974.596363] PM: suspend exit real 0m34.951s user 0m0.012s sys 0m0.259s [root@archl-xu4 alarm]# [11974.640778] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 400000Hz, actual 396825HZ div = 63) [11975.180552] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0) [11975.192142] mmc_host mmc0: Bus speed (slot 0) = 200000000Hz (slot req 200000000Hz, actual 200000000HZ div = 0) [11975.282474] mmc_host mmc0: Bus speed (slot 0) = 50000000Hz (slot req 52000000Hz, actual 50000000HZ div = 0) [11975.296174] mmc_host mmc0: Bus speed (slot 0) = 400000000Hz (slot req 200000000Hz, actual 200000000HZ div = 1) [11975.569457] usb 6-1: new SuperSpeed USB device number 5 using xhci-hcd [11975.614390] usb 6-1: New USB device found, idVendor=0bda, idProduct=8153, bcdDevice=30.00 [11975.622196] usb 6-1: New USB device strings: Mfr=1, Product=2, SerialNumber=6 [11975.629284] usb 6-1: Product: USB 10/100/1000 LAN [11975.633352] usb 6-1: Manufacturer: Realtek [11975.637458] usb 6-1: SerialNumber: 000001000000 [11975.871080] r8152-cfgselector 6-1: reset SuperSpeed USB device number 5 using xhci-hcd [11975.955112] r8152 6-1:1.0: load rtl8153a-3 v2 02/07/20 successfully [11976.032484] r8152 6-1:1.0 eth0: v1.12.13 [11976.134078] r8152 6-1:1.0 enu1: renamed from eth0 [root@archl-xu4 alarm]# [11981.522603] r8152 6-1:1.0 enu1: carrier on [root@archl-xu4 alarm]# > Ok, I will restore the clk changes in the suspend / resume functions in the next version and do thought testing. > Johan Thanks -Anand
