| Message ID | 1723089163-28983-4-git-send-email-shawn.lin@rock-chips.com |
|---|---|
| State | New |
| Headers | show |
| Series | Init support for RK3576 UFS controller | expand |
On Mon, Aug 12, 2024 at 09:28:26AM +0800, Shawn Lin wrote: > JHi Mani, > > 在 2024/8/10 17:28, Manivannan Sadhasivam 写道: > > On Fri, Aug 09, 2024 at 04:16:41PM +0800, Shawn Lin wrote: > > > > [...] > > > > > > > +static int ufs_rockchip_hce_enable_notify(struct ufs_hba *hba, > > > > > + enum ufs_notify_change_status status) > > > > > +{ > > > > > + int err = 0; > > > > > + > > > > > + if (status == PRE_CHANGE) { > > > > > + int retry_outer = 3; > > > > > + int retry_inner; > > > > > +start: > > > > > + if (ufshcd_is_hba_active(hba)) > > > > > + /* change controller state to "reset state" */ > > > > > + ufshcd_hba_stop(hba); > > > > > + > > > > > + /* UniPro link is disabled at this point */ > > > > > + ufshcd_set_link_off(hba); > > > > > + > > > > > + /* start controller initialization sequence */ > > > > > + ufshcd_writel(hba, CONTROLLER_ENABLE, REG_CONTROLLER_ENABLE); > > > > > + > > > > > + usleep_range(100, 200); > > > > > + > > > > > + /* wait for the host controller to complete initialization */ > > > > > + retry_inner = 50; > > > > > + while (!ufshcd_is_hba_active(hba)) { > > > > > + if (retry_inner) { > > > > > + retry_inner--; > > > > > + } else { > > > > > + dev_err(hba->dev, > > > > > + "Controller enable failed\n"); > > > > > + if (retry_outer) { > > > > > + retry_outer--; > > > > > + goto start; > > > > > + } > > > > > + return -EIO; > > > > > + } > > > > > + usleep_range(1000, 1100); > > > > > + } > > > > > > > > You just duplicated ufshcd_hba_execute_hce() here. Why? This doesn't make sense. > > > > > > Since we set UFSHCI_QUIRK_BROKEN_HCE, and we also need to do someting > > > which is very similar to ufshcd_hba_execute_hce(), before calling > > > ufshcd_dme_reset(). Similar but not totally the same. I'll try to see if > > > we can export ufshcd_hba_execute_hce() to make full use of it. > > > > > > > But you are starting the controller using REG_CONTROLLER_ENABLE. Isn't that > > supposed to be broken if you set UFSHCI_QUIRK_BROKEN_HCE? Or I am > > misunderstanding the quirk? > > > > Our controller doesn't work with exiting code, whether setting > UFSHCI_QUIRK_BROKEN_HCE or not. > Okay. Then this means you do not need this quirk at all. > > For UFSHCI_QUIRK_BROKEN_HCE case, it calls ufshcd_dme_reset（）first, > but we need to set REG_CONTROLLER_ENABLE first. > > For !UFSHCI_QUIRK_BROKEN_HCE case, namly ufshcd_hba_execute_hce, it > sets REG_CONTROLLER_ENABLE first but never send DMA_RESET and calls > ufshcd_dme_enable. > I don't see where ufshcd_dme_enable() is getting called for !UFSHCI_QUIRK_BROKEN_HCE case. > So the closet code path is to go through UFSHCI_QUIRK_BROKEN_HCE case, > and set REG_CONTROLLER_ENABLE by adding hce_enable_notify hook. > No, that is abusing the quirk. But I'm confused about why your controller wants resetting the unipro stack _after_ enabling the controller? Why can't it be reset before? > > > > > > > > > + } else { /* POST_CHANGE */ > > > > > + err = ufshcd_vops_phy_initialization(hba); > > > > > + } > > > > > + > > > > > + return err; > > > > > +} > > > > > + [...] > > > > > +static const struct dev_pm_ops ufs_rockchip_pm_ops = { > > > > > + SET_SYSTEM_SLEEP_PM_OPS(ufs_rockchip_suspend, ufs_rockchip_resume) > > > > > + SET_RUNTIME_PM_OPS(ufs_rockchip_runtime_suspend, ufs_rockchip_runtime_resume, NULL) > > > > > > > > Why can't you use ufshcd PM ops as like other vendor drivers? > > > > > > It doesn't work from the test. We have many use case to power down the > > > controller and device, so there is no flow to recovery the link. Only > > > when the first accessing to UFS fails, the ufshcd error handle recovery the > > > link. This is not what we expect. > > > > > > > What tests? The existing UFS controller drivers are used in production devices > > and they never had a usecase to invent their own PM callbacks. So if your > > controller is special, then you need to justify it more elaborately. If > > something is missing in ufshcd callbacks, then we can add them. > > > > All the register got lost each time as we power down both controller & PHY > and devices in suspend. Which suspend? runtime or system suspend? I believe system suspend. > So we have to restore the necessary > registers and link. I didn't see where the code recovery the controller > settings in ufshcd_resume, except ufshcd_err_handler（）triggers that. > Am I missing any thing? Can you explain what is causing the powerdown of the controller and PHY? Because, ufshcd_suspend() just turns off the clocks and regulators (if UFSHCD_CAP_AGGR_POWER_COLLAPSE is set) and spm_lvl 3 set by this driver only puts the device in sleep mode and link in hibern8 state. - Mani > Below is the dump we get if using > SET_SYSTEM_SLEEP_PM_OPS(ufshcd_system_suspend, ufshcd_system_resume). > It can work as ufshcd_err_handler () will fix the link, but we have to > suffer from getting the error log each time. Moreover, we need to gate > 26MHz refclk for device when RPM is called. So our own rpm callback is > needed. > > [ 14.318444] <<GTP-INF>>[gt1x_wakeup_sleep:964] Wakeup by poweron > [ 14.439723] ufshcd-rockchip 2a2d0000.ufs: Controller not ready to accept > UIC commands > [ 14.439730] ufshcd-rockchip 2a2d0000.ufs: pwr ctrl cmd 0x18 with mode 0x0 > uic error -5 > [ 14.439736] ufshcd-rockchip 2a2d0000.ufs: UFS Host state=1 > [ 14.439740] ufshcd-rockchip 2a2d0000.ufs: outstanding reqs=0x0 tasks=0x0 > [ 14.439744] ufshcd-rockchip 2a2d0000.ufs: saved_err=0x0, > saved_uic_err=0x0 > [ 14.439748] ufshcd-rockchip 2a2d0000.ufs: Device power mode=2, UIC link > state=2 > [ 14.439753] ufshcd-rockchip 2a2d0000.ufs: PM in progress=1, sys. > suspended=1 > [ 14.439758] ufshcd-rockchip 2a2d0000.ufs: Auto BKOPS=0, Host self-block=0 > [ 14.439762] ufshcd-rockchip 2a2d0000.ufs: Clk gate=1 > [ 14.439766] ufshcd-rockchip 2a2d0000.ufs: last_hibern8_exit_tstamp at 0 > us, hibern8_exit_cnt=0 > [ 14.439770] ufshcd-rockchip 2a2d0000.ufs: last intr at 10807625 us, last > intr status=0x440 > [ 14.439775] ufshcd-rockchip 2a2d0000.ufs: error handling flags=0x0, req. > abort count=0 > [ 14.439779] ufshcd-rockchip 2a2d0000.ufs: hba->ufs_version=0x200, Host > capabilities=0x187011f, caps=0x48c > [ 14.439785] ufshcd-rockchip 2a2d0000.ufs: quirks=0x2100, dev. quirks=0xc4 > [ 14.439790] ufshcd-rockchip 2a2d0000.ufs: UFS dev info: SAMSUNG > KLUDG2R1DE-B0F1 rev 0100 > [ 14.439796] ufshcd-rockchip 2a2d0000.ufs: clk: core, rate: 50000000 > [ 14.439822] host_regs: 00000000: 0187011f 00000000 00000200 00000000 > [ 14.439827] host_regs: 00000010: 00000000 000005e6 00000000 00000000 > [ 14.439831] host_regs: 00000020: 00000000 00000000 00000000 00000000 > [ 14.439835] host_regs: 00000030: 00000000 00000000 00000000 00000000 > [ 14.439839] host_regs: 00000040: 00000000 00000000 00000000 00000000 > [ 14.439843] host_regs: 00000050: 00000000 00000000 00000000 00000000 > [ 14.439847] host_regs: 00000060: 00000000 00000000 00000000 00000000 > [ 14.439851] host_regs: 00000070: 00000000 00000000 00000000 00000000 > [ 14.439855] host_regs: 00000080: 00000000 00000000 00000000 00000000 > [ 14.439859] host_regs: 00000090: 00000000 00000000 00000000 00000000 > [ 14.439863] ufshcd-rockchip 2a2d0000.ufs: No record of pa_err > [ 14.439867] ufshcd-rockchip 2a2d0000.ufs: No record of dl_err > [ 14.439871] ufshcd-rockchip 2a2d0000.ufs: No record of nl_err > [ 14.439876] ufshcd-rockchip 2a2d0000.ufs: No record of tl_err > [ 14.439880] ufshcd-rockchip 2a2d0000.ufs: No record of dme_err > [ 14.439884] ufshcd-rockchip 2a2d0000.ufs: No record of auto_hibern8_err > [ 14.439888] ufshcd-rockchip 2a2d0000.ufs: No record of fatal_err > [ 14.439892] ufshcd-rockchip 2a2d0000.ufs: No record of link_startup_fail > [ 14.439896] ufshcd-rockchip 2a2d0000.ufs: No record of resume_fail > [ 14.439900] ufshcd-rockchip 2a2d0000.ufs: No record of suspend_fail > [ 14.439905] ufshcd-rockchip 2a2d0000.ufs: dev_reset[0] = 0x0 at 1418763 > us > [ 14.439910] ufshcd-rockchip 2a2d0000.ufs: dev_reset: total cnt=1 > [ 14.439914] ufshcd-rockchip 2a2d0000.ufs: No record of host_reset > [ 14.439918] ufshcd-rockchip 2a2d0000.ufs: No record of task_abort > [ 14.439930] ufshcd-rockchip 2a2d0000.ufs: ufshcd_uic_hibern8_exit: > hibern8 exit failed. ret = -5 > [ 14.439935] ufshcd-rockchip 2a2d0000.ufs: __ufshcd_wl_resume: hibern8 > exit failed -5 > [ 14.439944] ufs_device_wlun 0:0:0:49488: ufshcd_wl_resume failed: -5 > [ 14.439950] ufs_device_wlun 0:0:0:49488: PM: dpm_run_callback(): > scsi_bus_resume+0x0/0xa8 returns -5 > [ 14.440003] ufshcd-rockchip 2a2d0000.ufs: ufshcd_err_handler started; HBA > state eh_fatal; powered 1; shutting down 0; saved_err = 0; saved_uic_err = > 0; force_reset = 0; link is broken > [ 14.440017] ufs_device_wlun 0:0:0:49488: PM: failed to resume async: > error -5 > > > - Mani > >
On Mon, Aug 12, 2024 at 02:24:31PM +0800, Shawn Lin wrote: > 在 2024/8/12 12:10, Manivannan Sadhasivam 写道: > > On Mon, Aug 12, 2024 at 09:28:26AM +0800, Shawn Lin wrote: > > > JHi Mani, > > > > > > 在 2024/8/10 17:28, Manivannan Sadhasivam 写道: > > > > On Fri, Aug 09, 2024 at 04:16:41PM +0800, Shawn Lin wrote: > > > > > > > > [...] > > > > > > > > > > > +static int ufs_rockchip_hce_enable_notify(struct ufs_hba *hba, > > > > > > > + enum ufs_notify_change_status status) > > > > > > > +{ > > > > > > > + int err = 0; > > > > > > > + > > > > > > > + if (status == PRE_CHANGE) { > > > > > > > + int retry_outer = 3; > > > > > > > + int retry_inner; > > > > > > > +start: > > > > > > > + if (ufshcd_is_hba_active(hba)) > > > > > > > + /* change controller state to "reset state" */ > > > > > > > + ufshcd_hba_stop(hba); > > > > > > > + > > > > > > > + /* UniPro link is disabled at this point */ > > > > > > > + ufshcd_set_link_off(hba); > > > > > > > + > > > > > > > + /* start controller initialization sequence */ > > > > > > > + ufshcd_writel(hba, CONTROLLER_ENABLE, REG_CONTROLLER_ENABLE); > > > > > > > + > > > > > > > + usleep_range(100, 200); > > > > > > > + > > > > > > > + /* wait for the host controller to complete initialization */ > > > > > > > + retry_inner = 50; > > > > > > > + while (!ufshcd_is_hba_active(hba)) { > > > > > > > + if (retry_inner) { > > > > > > > + retry_inner--; > > > > > > > + } else { > > > > > > > + dev_err(hba->dev, > > > > > > > + "Controller enable failed\n"); > > > > > > > + if (retry_outer) { > > > > > > > + retry_outer--; > > > > > > > + goto start; > > > > > > > + } > > > > > > > + return -EIO; > > > > > > > + } > > > > > > > + usleep_range(1000, 1100); > > > > > > > + } > > > > > > > > > > > > You just duplicated ufshcd_hba_execute_hce() here. Why? This doesn't make sense. > > > > > > > > > > Since we set UFSHCI_QUIRK_BROKEN_HCE, and we also need to do someting > > > > > which is very similar to ufshcd_hba_execute_hce(), before calling > > > > > ufshcd_dme_reset(). Similar but not totally the same. I'll try to see if > > > > > we can export ufshcd_hba_execute_hce() to make full use of it. > > > > > > > > > > > > > But you are starting the controller using REG_CONTROLLER_ENABLE. Isn't that > > > > supposed to be broken if you set UFSHCI_QUIRK_BROKEN_HCE? Or I am > > > > misunderstanding the quirk? > > > > > > > > > > Our controller doesn't work with exiting code, whether setting > > > UFSHCI_QUIRK_BROKEN_HCE or not. > > > > > > > Okay. Then this means you do not need this quirk at all. > > > > > > > > For UFSHCI_QUIRK_BROKEN_HCE case, it calls ufshcd_dme_reset（）first, > > > but we need to set REG_CONTROLLER_ENABLE first. > > > > > > For !UFSHCI_QUIRK_BROKEN_HCE case, namly ufshcd_hba_execute_hce, it > > > sets REG_CONTROLLER_ENABLE first but never send DMA_RESET and calls > > > ufshcd_dme_enable. > > > > > > > I don't see where ufshcd_dme_enable() is getting called for > > !UFSHCI_QUIRK_BROKEN_HCE case. > > > > > So the closet code path is to go through UFSHCI_QUIRK_BROKEN_HCE case, > > > and set REG_CONTROLLER_ENABLE by adding hce_enable_notify hook. > > > > > > > No, that is abusing the quirk. But I'm confused about why your controller wants > > resetting the unipro stack _after_ enabling the controller? Why can't it be > > reset before? > > > > It can't be. The DME_RESET to reset the unipro stack will be failed > without enabling REG_CONTROLLER_ENABLE. And the controller does want us > to reset the unipro stack before other coming UICs. > > So I considered it's a kind of broken HCE case as well. Should I add a > new quirk or add a new hba_enable hook in ufs_hba_variant_ops? Or just > use UFSHCI_QUIRK_BROKEN_HCE ? > IMO, you should add a new quirk and use it directly in ufshcd_hba_execute_hce(). But you need to pick the quirk name as per the actual quirky behavior of the controller. > > > > > > > > > > > > > + } else { /* POST_CHANGE */ > > > > > > > + err = ufshcd_vops_phy_initialization(hba); > > > > > > > + } > > > > > > > + > > > > > > > + return err; > > > > > > > +} > > > > > > > + > > > > [...] > > > > > > > > > +static const struct dev_pm_ops ufs_rockchip_pm_ops = { > > > > > > > + SET_SYSTEM_SLEEP_PM_OPS(ufs_rockchip_suspend, ufs_rockchip_resume) > > > > > > > + SET_RUNTIME_PM_OPS(ufs_rockchip_runtime_suspend, ufs_rockchip_runtime_resume, NULL) > > > > > > > > > > > > Why can't you use ufshcd PM ops as like other vendor drivers? > > > > > > > > > > It doesn't work from the test. We have many use case to power down the > > > > > controller and device, so there is no flow to recovery the link. Only > > > > > when the first accessing to UFS fails, the ufshcd error handle recovery the > > > > > link. This is not what we expect. > > > > > > > > > > > > > What tests? The existing UFS controller drivers are used in production devices > > > > and they never had a usecase to invent their own PM callbacks. So if your > > > > controller is special, then you need to justify it more elaborately. If > > > > something is missing in ufshcd callbacks, then we can add them. > > > > > > > > > > All the register got lost each time as we power down both controller & PHY > > > and devices in suspend. > > > > Which suspend? runtime or system suspend? I believe system suspend. > > Both. > With {rpm/spm}_lvl = 3, you should not power down the controller. > > > > > So we have to restore the necessary > > > registers and link. I didn't see where the code recovery the controller > > > settings in ufshcd_resume, except ufshcd_err_handler（）triggers that. > > > Am I missing any thing? > > > > Can you explain what is causing the powerdown of the controller and PHY? > > Because, ufshcd_suspend() just turns off the clocks and regulators (if > > UFSHCD_CAP_AGGR_POWER_COLLAPSE is set) and spm_lvl 3 set by this driver only > > puts the device in sleep mode and link in hibern8 state. > > > > For runtime PM case, it's the power-domain driver will power down the > controller and PHY if UFS stack is not active any more（autosuspend）. > > For system PM case, it's the SoC's firmware to cutting of all the power > for controller/PHY and device. > Both cases are not matching the expectations of {rpm/spm}_lvl. So the platform (power domain or the firmware) should be fixed. What if the user sets the {rpm/spm}_lvl to 1? Will the platform power down the controller even then? If so, then I'd say that the platform is broken and should be fixed. - Mani
Hi Mani, 在 2024/8/13 0:55, Manivannan Sadhasivam 写道: > On Mon, Aug 12, 2024 at 02:24:31PM +0800, Shawn Lin wrote: >> 在 2024/8/12 12:10, Manivannan Sadhasivam 写道: >>> On Mon, Aug 12, 2024 at 09:28:26AM +0800, Shawn Lin wrote: >>>> JHi Mani, >>>> >>>> 在 2024/8/10 17:28, Manivannan Sadhasivam 写道: >>>>> On Fri, Aug 09, 2024 at 04:16:41PM +0800, Shawn Lin wrote: >>>>> >>>>> [...] >>>>> >>>>>>>> +static int ufs_rockchip_hce_enable_notify(struct ufs_hba *hba, >>>>>>>> + enum ufs_notify_change_status status) >>>>>>>> +{ >>>>>>>> + int err = 0; >>>>>>>> + >>>>>>>> + if (status == PRE_CHANGE) { >>>>>>>> + int retry_outer = 3; >>>>>>>> + int retry_inner; >>>>>>>> +start: >>>>>>>> + if (ufshcd_is_hba_active(hba)) >>>>>>>> + /* change controller state to "reset state" */ >>>>>>>> + ufshcd_hba_stop(hba); >>>>>>>> + >>>>>>>> + /* UniPro link is disabled at this point */ >>>>>>>> + ufshcd_set_link_off(hba); >>>>>>>> + >>>>>>>> + /* start controller initialization sequence */ >>>>>>>> + ufshcd_writel(hba, CONTROLLER_ENABLE, REG_CONTROLLER_ENABLE); >>>>>>>> + >>>>>>>> + usleep_range(100, 200); >>>>>>>> + >>>>>>>> + /* wait for the host controller to complete initialization */ >>>>>>>> + retry_inner = 50; >>>>>>>> + while (!ufshcd_is_hba_active(hba)) { >>>>>>>> + if (retry_inner) { >>>>>>>> + retry_inner--; >>>>>>>> + } else { >>>>>>>> + dev_err(hba->dev, >>>>>>>> + "Controller enable failed\n"); >>>>>>>> + if (retry_outer) { >>>>>>>> + retry_outer--; >>>>>>>> + goto start; >>>>>>>> + } >>>>>>>> + return -EIO; >>>>>>>> + } >>>>>>>> + usleep_range(1000, 1100); >>>>>>>> + } >>>>>>> >>>>>>> You just duplicated ufshcd_hba_execute_hce() here. Why? This doesn't make sense. >>>>>> >>>>>> Since we set UFSHCI_QUIRK_BROKEN_HCE, and we also need to do someting >>>>>> which is very similar to ufshcd_hba_execute_hce(), before calling >>>>>> ufshcd_dme_reset(). Similar but not totally the same. I'll try to see if >>>>>> we can export ufshcd_hba_execute_hce() to make full use of it. >>>>>> >>>>> >>>>> But you are starting the controller using REG_CONTROLLER_ENABLE. Isn't that >>>>> supposed to be broken if you set UFSHCI_QUIRK_BROKEN_HCE? Or I am >>>>> misunderstanding the quirk? >>>>> >>>> >>>> Our controller doesn't work with exiting code, whether setting >>>> UFSHCI_QUIRK_BROKEN_HCE or not. >>>> >>> >>> Okay. Then this means you do not need this quirk at all. >>> >>>> >>>> For UFSHCI_QUIRK_BROKEN_HCE case, it calls ufshcd_dme_reset（）first, >>>> but we need to set REG_CONTROLLER_ENABLE first. >>>> >>>> For !UFSHCI_QUIRK_BROKEN_HCE case, namly ufshcd_hba_execute_hce, it >>>> sets REG_CONTROLLER_ENABLE first but never send DMA_RESET and calls >>>> ufshcd_dme_enable. >>>> >>> >>> I don't see where ufshcd_dme_enable() is getting called for >>> !UFSHCI_QUIRK_BROKEN_HCE case. >>> >>>> So the closet code path is to go through UFSHCI_QUIRK_BROKEN_HCE case, >>>> and set REG_CONTROLLER_ENABLE by adding hce_enable_notify hook. >>>> >>> >>> No, that is abusing the quirk. But I'm confused about why your controller wants >>> resetting the unipro stack _after_ enabling the controller? Why can't it be >>> reset before? >>> >> >> It can't be. The DME_RESET to reset the unipro stack will be failed >> without enabling REG_CONTROLLER_ENABLE. And the controller does want us >> to reset the unipro stack before other coming UICs. >> >> So I considered it's a kind of broken HCE case as well. Should I add a >> new quirk or add a new hba_enable hook in ufs_hba_variant_ops? Or just >> use UFSHCI_QUIRK_BROKEN_HCE ? >> > > IMO, you should add a new quirk and use it directly in ufshcd_hba_execute_hce(). > But you need to pick the quirk name as per the actual quirky behavior of the > controller. > Thanks, Main. I'll add a new quirk for ufshcd_hba_execute_hce() as per the actual quirky behavour. >>>>>>> >>>>>>>> + } else { /* POST_CHANGE */ >>>>>>>> + err = ufshcd_vops_phy_initialization(hba); >>>>>>>> + } >>>>>>>> + >>>>>>>> + return err; >>>>>>>> +} >>>>>>>> + >>> >>> [...] >>> >>>>>>>> +static const struct dev_pm_ops ufs_rockchip_pm_ops = { >>>>>>>> + SET_SYSTEM_SLEEP_PM_OPS(ufs_rockchip_suspend, ufs_rockchip_resume) >>>>>>>> + SET_RUNTIME_PM_OPS(ufs_rockchip_runtime_suspend, ufs_rockchip_runtime_resume, NULL) >>>>>>> >>>>>>> Why can't you use ufshcd PM ops as like other vendor drivers? >>>>>> >>>>>> It doesn't work from the test. We have many use case to power down the >>>>>> controller and device, so there is no flow to recovery the link. Only >>>>>> when the first accessing to UFS fails, the ufshcd error handle recovery the >>>>>> link. This is not what we expect. >>>>>> >>>>> >>>>> What tests? The existing UFS controller drivers are used in production devices >>>>> and they never had a usecase to invent their own PM callbacks. So if your >>>>> controller is special, then you need to justify it more elaborately. If >>>>> something is missing in ufshcd callbacks, then we can add them. >>>>> >>>> >>>> All the register got lost each time as we power down both controller & PHY >>>> and devices in suspend. >>> >>> Which suspend? runtime or system suspend? I believe system suspend. >> >> Both. >> > > With {rpm/spm}_lvl = 3, you should not power down the controller. > >>> >>>> So we have to restore the necessary >>>> registers and link. I didn't see where the code recovery the controller >>>> settings in ufshcd_resume, except ufshcd_err_handler（）triggers that. >>>> Am I missing any thing? >>> >>> Can you explain what is causing the powerdown of the controller and PHY? >>> Because, ufshcd_suspend() just turns off the clocks and regulators (if >>> UFSHCD_CAP_AGGR_POWER_COLLAPSE is set) and spm_lvl 3 set by this driver only >>> puts the device in sleep mode and link in hibern8 state. >>> >> >> For runtime PM case, it's the power-domain driver will power down the >> controller and PHY if UFS stack is not active any more（autosuspend）. >> >> For system PM case, it's the SoC's firmware to cutting of all the power >> for controller/PHY and device. >> > > Both cases are not matching the expectations of {rpm/spm}_lvl. So the platform > (power domain or the firmware) should be fixed. What if the user sets the > {rpm/spm}_lvl to 1? Will the platform power down the controller even then? If > so, then I'd say that the platform is broken and should be fixed. Ok, it seems I need to set {rpm/spm}_lvl = 6 if I want platform to power down the controller for ultra power-saving. But I still need to add my own system PM callback in that case to recovery the link first. Do I misunderstand it? And for the user who sets the rpm/spm level via ufs_sysfs_pm_lvl_store(), I think there is no way to block it currently, except that we need to fix the power-domain driver and Firmware to respect the level and choose correct policy. So in summary for what the next step I should to: (1) Set {rpm/spm}_lvl = 6 in host driver to reflect the expectation (2) Add own PM callbacks to recovery the link to meet the expectation (3) Fix the broken behaviour of PD or Firmware to respect the actual desired pm level if user changes the pm level. Does that sound feasible to you? Thanks. > > - Mani >
在 2024/8/13 11:52, Shawn Lin 写道: > Hi Mani, > > 在 2024/8/13 0:55, Manivannan Sadhasivam 写道: >> On Mon, Aug 12, 2024 at 02:24:31PM +0800, Shawn Lin wrote: >>> 在 2024/8/12 12:10, Manivannan Sadhasivam 写道: >>>> On Mon, Aug 12, 2024 at 09:28:26AM +0800, Shawn Lin wrote: >>>>> JHi Mani, >>>>> >>>>> 在 2024/8/10 17:28, Manivannan Sadhasivam 写道: >>>>>> On Fri, Aug 09, 2024 at 04:16:41PM +0800, Shawn Lin wrote: >>>>>> >>>>>> [...] >>>>>> >>>>>>>>> +static int ufs_rockchip_hce_enable_notify(struct ufs_hba *hba, >>>>>>>>> + enum ufs_notify_change_status status) >>>>>>>>> +{ >>>>>>>>> + int err = 0; >>>>>>>>> + >>>>>>>>> + if (status == PRE_CHANGE) { >>>>>>>>> + int retry_outer = 3; >>>>>>>>> + int retry_inner; >>>>>>>>> +start: >>>>>>>>> + if (ufshcd_is_hba_active(hba)) >>>>>>>>> + /* change controller state to "reset state" */ >>>>>>>>> + ufshcd_hba_stop(hba); >>>>>>>>> + >>>>>>>>> + /* UniPro link is disabled at this point */ >>>>>>>>> + ufshcd_set_link_off(hba); >>>>>>>>> + >>>>>>>>> + /* start controller initialization sequence */ >>>>>>>>> + ufshcd_writel(hba, CONTROLLER_ENABLE, >>>>>>>>> REG_CONTROLLER_ENABLE); >>>>>>>>> + >>>>>>>>> + usleep_range(100, 200); >>>>>>>>> + >>>>>>>>> + /* wait for the host controller to complete >>>>>>>>> initialization */ >>>>>>>>> + retry_inner = 50; >>>>>>>>> + while (!ufshcd_is_hba_active(hba)) { >>>>>>>>> + if (retry_inner) { >>>>>>>>> + retry_inner--; >>>>>>>>> + } else { >>>>>>>>> + dev_err(hba->dev, >>>>>>>>> + "Controller enable failed\n"); >>>>>>>>> + if (retry_outer) { >>>>>>>>> + retry_outer--; >>>>>>>>> + goto start; >>>>>>>>> + } >>>>>>>>> + return -EIO; >>>>>>>>> + } >>>>>>>>> + usleep_range(1000, 1100); >>>>>>>>> + } >>>>>>>> >>>>>>>> You just duplicated ufshcd_hba_execute_hce() here. Why? This >>>>>>>> doesn't make sense. >>>>>>> >>>>>>> Since we set UFSHCI_QUIRK_BROKEN_HCE, and we also need to do >>>>>>> someting >>>>>>> which is very similar to ufshcd_hba_execute_hce(), before calling >>>>>>> ufshcd_dme_reset(). Similar but not totally the same. I'll try to >>>>>>> see if >>>>>>> we can export ufshcd_hba_execute_hce() to make full use of it. >>>>>>> >>>>>> >>>>>> But you are starting the controller using REG_CONTROLLER_ENABLE. >>>>>> Isn't that >>>>>> supposed to be broken if you set UFSHCI_QUIRK_BROKEN_HCE? Or I am >>>>>> misunderstanding the quirk? >>>>>> >>>>> >>>>> Our controller doesn't work with exiting code, whether setting >>>>> UFSHCI_QUIRK_BROKEN_HCE or not. >>>>> >>>> >>>> Okay. Then this means you do not need this quirk at all. >>>> >>>>> >>>>> For UFSHCI_QUIRK_BROKEN_HCE case, it calls ufshcd_dme_reset（）first, >>>>> but we need to set REG_CONTROLLER_ENABLE first. >>>>> >>>>> For !UFSHCI_QUIRK_BROKEN_HCE case, namly ufshcd_hba_execute_hce, it >>>>> sets REG_CONTROLLER_ENABLE first but never send DMA_RESET and calls >>>>> ufshcd_dme_enable. >>>>> >>>> >>>> I don't see where ufshcd_dme_enable() is getting called for >>>> !UFSHCI_QUIRK_BROKEN_HCE case. >>>> >>>>> So the closet code path is to go through UFSHCI_QUIRK_BROKEN_HCE case, >>>>> and set REG_CONTROLLER_ENABLE by adding hce_enable_notify hook. >>>>> >>>> >>>> No, that is abusing the quirk. But I'm confused about why your >>>> controller wants >>>> resetting the unipro stack _after_ enabling the controller? Why >>>> can't it be >>>> reset before? >>>> >>> >>> It can't be. The DME_RESET to reset the unipro stack will be failed >>> without enabling REG_CONTROLLER_ENABLE. And the controller does want us >>> to reset the unipro stack before other coming UICs. >>> >>> So I considered it's a kind of broken HCE case as well. Should I add a >>> new quirk or add a new hba_enable hook in ufs_hba_variant_ops? Or just >>> use UFSHCI_QUIRK_BROKEN_HCE ? >>> >> >> IMO, you should add a new quirk and use it directly in >> ufshcd_hba_execute_hce(). >> But you need to pick the quirk name as per the actual quirky behavior >> of the >> controller. >> > > Thanks, Main. I'll add a new quirk for > ufshcd_hba_execute_hce() as per the actual quirky behavour. > >>>>>>>> >>>>>>>>> + } else { /* POST_CHANGE */ >>>>>>>>> + err = ufshcd_vops_phy_initialization(hba); >>>>>>>>> + } >>>>>>>>> + >>>>>>>>> + return err; >>>>>>>>> +} >>>>>>>>> + >>>> >>>> [...] >>>> >>>>>>>>> +static const struct dev_pm_ops ufs_rockchip_pm_ops = { >>>>>>>>> + SET_SYSTEM_SLEEP_PM_OPS(ufs_rockchip_suspend, >>>>>>>>> ufs_rockchip_resume) >>>>>>>>> + SET_RUNTIME_PM_OPS(ufs_rockchip_runtime_suspend, >>>>>>>>> ufs_rockchip_runtime_resume, NULL) >>>>>>>> >>>>>>>> Why can't you use ufshcd PM ops as like other vendor drivers? >>>>>>> >>>>>>> It doesn't work from the test. We have many use case to power >>>>>>> down the >>>>>>> controller and device, so there is no flow to recovery the link. >>>>>>> Only >>>>>>> when the first accessing to UFS fails, the ufshcd error handle >>>>>>> recovery the >>>>>>> link. This is not what we expect. >>>>>>> >>>>>> >>>>>> What tests? The existing UFS controller drivers are used in >>>>>> production devices >>>>>> and they never had a usecase to invent their own PM callbacks. So >>>>>> if your >>>>>> controller is special, then you need to justify it more >>>>>> elaborately. If >>>>>> something is missing in ufshcd callbacks, then we can add them. >>>>>> >>>>> >>>>> All the register got lost each time as we power down both >>>>> controller & PHY >>>>> and devices in suspend. >>>> >>>> Which suspend? runtime or system suspend? I believe system suspend. >>> >>> Both. >>> >> >> With {rpm/spm}_lvl = 3, you should not power down the controller. >> >>>> >>>>> So we have to restore the necessary >>>>> registers and link. I didn't see where the code recovery the >>>>> controller >>>>> settings in ufshcd_resume, except ufshcd_err_handler（）triggers that. >>>>> Am I missing any thing? >>>> >>>> Can you explain what is causing the powerdown of the controller and >>>> PHY? >>>> Because, ufshcd_suspend() just turns off the clocks and regulators (if >>>> UFSHCD_CAP_AGGR_POWER_COLLAPSE is set) and spm_lvl 3 set by this >>>> driver only >>>> puts the device in sleep mode and link in hibern8 state. >>>> >>> >>> For runtime PM case, it's the power-domain driver will power down the >>> controller and PHY if UFS stack is not active any more（autosuspend）. >>> >>> For system PM case, it's the SoC's firmware to cutting of all the power >>> for controller/PHY and device. >>> >> >> Both cases are not matching the expectations of {rpm/spm}_lvl. So the >> platform >> (power domain or the firmware) should be fixed. What if the user sets the >> {rpm/spm}_lvl to 1? Will the platform power down the controller even >> then? If >> so, then I'd say that the platform is broken and should be fixed. > > Ok, it seems I need to set {rpm/spm}_lvl = 6 if I want platform to power > down the controller for ultra power-saving. But I still need to add my > own system PM callback in that case to recovery the link first. Do I > misunderstand it? > > And for the user who sets the rpm/spm level via > ufs_sysfs_pm_lvl_store(), I think there is no way to block it currently, > except that we need to fix the power-domain driver and Firmware to > respect the level and choose correct policy. > > > So in summary for what the next step I should to: > (1) Set {rpm/spm}_lvl = 6 in host driver to reflect the expectation > (2) Add own PM callbacks to recovery the link to meet the expectation > (3) Fix the broken behaviour of PD or Firmware to respect the actual > desired pm level if user changes the pm level. > > Sorry, I misunderstood your comment, so the action should be (1) Set {rpm/spm}_lvl = 5 in host driver to reflect the expectation (2) Use ufshcd_system_suspend/resume, but keep our own runtime PM callbacks as we need a extra step to gate refclk. (3) Fix the broken behaviour of PD or Firmware to respect the actual desired pm level if user changes the pm level. > Does that sound feasible to you? > > > Thanks. > >> >> - Mani >>
On Tue, Aug 13, 2024 at 03:22:45PM +0800, Shawn Lin wrote: [...] > > > > For runtime PM case, it's the power-domain driver will power down the > > > > controller and PHY if UFS stack is not active any more（autosuspend）. > > > > > > > > For system PM case, it's the SoC's firmware to cutting of all the power > > > > for controller/PHY and device. > > > > > > > > > > Both cases are not matching the expectations of {rpm/spm}_lvl. So > > > the platform > > > (power domain or the firmware) should be fixed. What if the user sets the > > > {rpm/spm}_lvl to 1? Will the platform power down the controller even > > > then? If > > > so, then I'd say that the platform is broken and should be fixed. > > > > Ok, it seems I need to set {rpm/spm}_lvl = 6 if I want platform to power > > down the controller for ultra power-saving. But I still need to add my > > own system PM callback in that case to recovery the link first. Do I > > misunderstand it? > > > > And for the user who sets the rpm/spm level via > > ufs_sysfs_pm_lvl_store(), I think there is no way to block it currently, > > except that we need to fix the power-domain driver and Firmware to > > respect the level and choose correct policy. > > > > > > So in summary for what the next step I should to: > > (1) Set {rpm/spm}_lvl = 6 in host driver to reflect the expectation > > (2) Add own PM callbacks to recovery the link to meet the expectation > > (3) Fix the broken behaviour of PD or Firmware to respect the actual > > desired pm level if user changes the pm level. > > > > > > Sorry, I misunderstood your comment, so the action should be > (1) Set {rpm/spm}_lvl = 5 in host driver to reflect the expectation > (2) Use ufshcd_system_suspend/resume, but keep our own runtime PM > callbacks as we need a extra step to gate refclk. Ok. > (3) Fix the broken behaviour of PD or Firmware to respect the actual > desired pm level if user changes the pm level. If you do this, then you don't need (1), don't you? - Mani
diff --git a/drivers/ufs/host/Kconfig b/drivers/ufs/host/Kconfig index 580c8d0..fafaa33 100644 --- a/drivers/ufs/host/Kconfig +++ b/drivers/ufs/host/Kconfig @@ -142,3 +142,15 @@ config SCSI_UFS_SPRD Select this if you have UFS controller on Unisoc chipset. If unsure, say N. + +config SCSI_UFS_ROCKCHIP + tristate "Rockchip specific hooks to UFS controller platform driver" + depends on SCSI_UFSHCD_PLATFORM && (ARCH_ROCKCHIP || COMPILE_TEST) + help + This selects the Rockchip specific additions to UFSHCD platform driver. + UFS host on Rockchip needs some vendor specific configuration before + accessing the hardware which includes PHY configuration and vendor + specific registers. + + Select this if you have UFS controller on Rockchip chipset. + If unsure, say N. diff --git a/drivers/ufs/host/Makefile b/drivers/ufs/host/Makefile index 4573aea..2f97feb 100644 --- a/drivers/ufs/host/Makefile +++ b/drivers/ufs/host/Makefile @@ -10,5 +10,6 @@ obj-$(CONFIG_SCSI_UFSHCD_PLATFORM) += ufshcd-pltfrm.o obj-$(CONFIG_SCSI_UFS_HISI) += ufs-hisi.o obj-$(CONFIG_SCSI_UFS_MEDIATEK) += ufs-mediatek.o obj-$(CONFIG_SCSI_UFS_RENESAS) += ufs-renesas.o +obj-$(CONFIG_SCSI_UFS_ROCKCHIP) += ufs-rockchip.o obj-$(CONFIG_SCSI_UFS_SPRD) += ufs-sprd.o obj-$(CONFIG_SCSI_UFS_TI_J721E) += ti-j721e-ufs.o diff --git a/drivers/ufs/host/ufs-rockchip.c b/drivers/ufs/host/ufs-rockchip.c new file mode 100644 index 0000000..46c90d6 --- /dev/null +++ b/drivers/ufs/host/ufs-rockchip.c @@ -0,0 +1,438 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Rockchip UFS Host Controller driver + * + * Copyright (C) 2024 Rockchip Electronics Co.Ltd. + */ + +#include <linux/clk.h> +#include <linux/gpio.h> +#include <linux/mfd/syscon.h> +#include <linux/of.h> +#include <linux/platform_device.h> +#include <linux/regmap.h> +#include <linux/reset.h> + +#include <ufs/ufshcd.h> +#include <ufs/unipro.h> +#include "ufshcd-pltfrm.h" +#include "ufshcd-dwc.h" +#include "ufs-rockchip.h" + +static inline bool ufshcd_is_device_present(struct ufs_hba *hba) +{ + return ufshcd_readl(hba, REG_CONTROLLER_STATUS) & DEVICE_PRESENT; +} + +static int ufs_rockchip_hce_enable_notify(struct ufs_hba *hba, + enum ufs_notify_change_status status) +{ + int err = 0; + + if (status == PRE_CHANGE) { + int retry_outer = 3; + int retry_inner; +start: + if (ufshcd_is_hba_active(hba)) + /* change controller state to "reset state" */ + ufshcd_hba_stop(hba); + + /* UniPro link is disabled at this point */ + ufshcd_set_link_off(hba); + + /* start controller initialization sequence */ + ufshcd_writel(hba, CONTROLLER_ENABLE, REG_CONTROLLER_ENABLE); + + usleep_range(100, 200); + + /* wait for the host controller to complete initialization */ + retry_inner = 50; + while (!ufshcd_is_hba_active(hba)) { + if (retry_inner) { + retry_inner--; + } else { + dev_err(hba->dev, + "Controller enable failed\n"); + if (retry_outer) { + retry_outer--; + goto start; + } + return -EIO; + } + usleep_range(1000, 1100); + } + } else { /* POST_CHANGE */ + err = ufshcd_vops_phy_initialization(hba); + } + + return err; +} + +static void ufs_rockchip_set_pm_lvl(struct ufs_hba *hba) +{ + hba->rpm_lvl = UFS_PM_LVL_1; + hba->spm_lvl = UFS_PM_LVL_3; +} + +static int ufs_rockchip_rk3576_phy_init(struct ufs_hba *hba) +{ + struct ufs_rockchip_host *host = ufshcd_get_variant(hba); + + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(PA_LOCAL_TX_LCC_ENABLE, 0x0), 0x0); + /* enable the mphy DME_SET cfg */ + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0x200, 0x0), 0x40); + for (int i = 0; i < 2; i++) { + /* Configuration M-TX */ + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0xaa, SEL_TX_LANE0 + i), 0x06); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0xa9, SEL_TX_LANE0 + i), 0x02); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0xad, SEL_TX_LANE0 + i), 0x44); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0xac, SEL_TX_LANE0 + i), 0xe6); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0xab, SEL_TX_LANE0 + i), 0x07); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0x94, SEL_TX_LANE0 + i), 0x93); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0x93, SEL_TX_LANE0 + i), 0xc9); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0x7f, SEL_TX_LANE0 + i), 0x00); + /* Configuration M-RX */ + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0x12, SEL_RX_LANE0 + i), 0x06); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0x11, SEL_RX_LANE0 + i), 0x00); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0x1d, SEL_RX_LANE0 + i), 0x58); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0x1c, SEL_RX_LANE0 + i), 0x8c); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0x1b, SEL_RX_LANE0 + i), 0x02); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0x25, SEL_RX_LANE0 + i), 0xf6); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0x2f, SEL_RX_LANE0 + i), 0x69); + } + /* disable the mphy DME_SET cfg */ + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(0x200, 0x0), 0x00); + + ufs_sys_writel(host->mphy_base, 0x80, 0x08C); + ufs_sys_writel(host->mphy_base, 0xB5, 0x110); + ufs_sys_writel(host->mphy_base, 0xB5, 0x250); + + ufs_sys_writel(host->mphy_base, 0x03, 0x134); + ufs_sys_writel(host->mphy_base, 0x03, 0x274); + + ufs_sys_writel(host->mphy_base, 0x38, 0x0E0); + ufs_sys_writel(host->mphy_base, 0x38, 0x220); + + ufs_sys_writel(host->mphy_base, 0x50, 0x164); + ufs_sys_writel(host->mphy_base, 0x50, 0x2A4); + + ufs_sys_writel(host->mphy_base, 0x80, 0x178); + ufs_sys_writel(host->mphy_base, 0x80, 0x2B8); + + ufs_sys_writel(host->mphy_base, 0x18, 0x1B0); + ufs_sys_writel(host->mphy_base, 0x18, 0x2F0); + + ufs_sys_writel(host->mphy_base, 0x03, 0x128); + ufs_sys_writel(host->mphy_base, 0x03, 0x268); + + ufs_sys_writel(host->mphy_base, 0x20, 0x12C); + ufs_sys_writel(host->mphy_base, 0x20, 0x26C); + + ufs_sys_writel(host->mphy_base, 0xC0, 0x120); + ufs_sys_writel(host->mphy_base, 0xC0, 0x260); + + ufs_sys_writel(host->mphy_base, 0x03, 0x094); + + ufs_sys_writel(host->mphy_base, 0x03, 0x1B4); + ufs_sys_writel(host->mphy_base, 0x03, 0x2F4); + + ufs_sys_writel(host->mphy_base, 0xC0, 0x08C); + udelay(1); + ufs_sys_writel(host->mphy_base, 0x00, 0x08C); + + udelay(200); + /* start link up */ + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(MIB_T_DBG_CPORT_TX_ENDIAN, 0), 0x0); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(MIB_T_DBG_CPORT_RX_ENDIAN, 0), 0x0); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(N_DEVICEID, 0), 0x0); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(N_DEVICEID_VALID, 0), 0x1); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(T_PEERDEVICEID, 0), 0x1); + ufshcd_dme_set(hba, UIC_ARG_MIB_SEL(T_CONNECTIONSTATE, 0), 0x1); + + return 0; +} + +static int ufs_rockchip_common_init(struct ufs_hba *hba) +{ + struct device *dev = hba->dev; + struct platform_device *pdev = to_platform_device(dev); + struct ufs_rockchip_host *host; + int err = 0; + + host = devm_kzalloc(dev, sizeof(*host), GFP_KERNEL); + if (!host) + return -ENOMEM; + + /* system control register for hci */ + host->ufs_sys_ctrl = devm_platform_ioremap_resource_byname(pdev, "hci_grf"); + if (IS_ERR(host->ufs_sys_ctrl)) + return dev_err_probe(dev, PTR_ERR(host->ufs_sys_ctrl), + "cannot ioremap for hci system control register\n"); + + /* system control register for mphy */ + host->ufs_phy_ctrl = devm_platform_ioremap_resource_byname(pdev, "mphy_grf"); + if (IS_ERR(host->ufs_phy_ctrl)) + return dev_err_probe(dev, PTR_ERR(host->ufs_phy_ctrl), + "cannot ioremap for mphy system control register\n"); + + /* mphy base register */ + host->mphy_base = devm_platform_ioremap_resource_byname(pdev, "mphy"); + if (IS_ERR(host->mphy_base)) + return dev_err_probe(dev, PTR_ERR(host->mphy_base), + "cannot ioremap for mphy base register\n"); + + host->rst = devm_reset_control_array_get_exclusive(dev); + if (IS_ERR(host->rst)) + return dev_err_probe(dev, PTR_ERR(host->rst), "failed to get reset control\n"); + + reset_control_assert(host->rst); + udelay(1); + reset_control_deassert(host->rst); + + host->ref_out_clk = devm_clk_get(dev, "ref_out"); + if (IS_ERR(host->ref_out_clk)) + return dev_err_probe(dev, PTR_ERR(host->ref_out_clk), "ciu-drive not available\n"); + + err = clk_prepare_enable(host->ref_out_clk); + if (err) + return dev_err_probe(dev, err, "failed to enable ref out clock\n"); + + host->rst_gpio = devm_gpiod_get(&pdev->dev, "reset", GPIOD_OUT_LOW); + if (IS_ERR(host->rst_gpio)) { + dev_err_probe(&pdev->dev, PTR_ERR(host->rst_gpio), + "invalid reset-gpios property in node\n"); + err = PTR_ERR(host->rst_gpio); + goto out; + } + udelay(20); + gpiod_set_value_cansleep(host->rst_gpio, 1); + + host->clks[0].id = "core"; + host->clks[1].id = "pclk"; + host->clks[2].id = "pclk_mphy"; + err = devm_clk_bulk_get_optional(dev, UFS_MAX_CLKS, host->clks); + if (err) { + dev_err_probe(dev, err, "failed to get clocks\n"); + goto out; + } + + err = clk_bulk_prepare_enable(UFS_MAX_CLKS, host->clks); + if (err) { + dev_err_probe(dev, err, "failed to enable clocks\n"); + goto out; + } + + pm_runtime_set_active(&pdev->dev); + + host->hba = hba; + ufs_rockchip_set_pm_lvl(hba); + + ufshcd_set_variant(hba, host); + + return 0; +out: + clk_disable_unprepare(host->ref_out_clk); + return err; +} + +static int ufs_rockchip_rk3576_init(struct ufs_hba *hba) +{ + int ret = 0; + struct device *dev = hba->dev; + + hba->quirks = UFSHCI_QUIRK_BROKEN_HCE | UFSHCD_QUIRK_SKIP_DEF_UNIPRO_TIMEOUT_SETTING; + + /* Enable BKOPS when suspend */ + hba->caps |= UFSHCD_CAP_AUTO_BKOPS_SUSPEND; + /* Enable putting device into deep sleep */ + hba->caps |= UFSHCD_CAP_DEEPSLEEP; + /* Enable devfreq of UFS */ + hba->caps |= UFSHCD_CAP_CLK_SCALING; + /* Enable WriteBooster */ + hba->caps |= UFSHCD_CAP_WB_EN; + + ret = ufs_rockchip_common_init(hba); + if (ret) + return dev_err_probe(dev, ret, "ufs common init fail\n"); + + return 0; +} + +static int ufs_rockchip_device_reset(struct ufs_hba *hba) +{ + struct ufs_rockchip_host *host = ufshcd_get_variant(hba); + + if (!host->rst_gpio) + return -EOPNOTSUPP; + + gpiod_set_value_cansleep(host->rst_gpio, 0); + udelay(20); + + gpiod_set_value_cansleep(host->rst_gpio, 1); + udelay(20); + + return 0; +} + +static const struct ufs_hba_variant_ops ufs_hba_rk3576_vops = { + .name = "rk3576", + .init = ufs_rockchip_rk3576_init, + .device_reset = ufs_rockchip_device_reset, + .hce_enable_notify = ufs_rockchip_hce_enable_notify, + .phy_initialization = ufs_rockchip_rk3576_phy_init, +}; + +static const struct of_device_id ufs_rockchip_of_match[] = { + { .compatible = "rockchip,rk3576-ufs", .data = &ufs_hba_rk3576_vops}, + {}, +}; +MODULE_DEVICE_TABLE(of, ufs_rockchip_of_match); + +static int ufs_rockchip_probe(struct platform_device *pdev) +{ + int err = 0; + struct device *dev = &pdev->dev; + const struct ufs_hba_variant_ops *vops; + + vops = device_get_match_data(dev); + err = ufshcd_pltfrm_init(pdev, vops); + if (err) + dev_err_probe(dev, err, "ufshcd_pltfrm_init failed\n"); + + return err; +} + +static void ufs_rockchip_remove(struct platform_device *pdev) +{ + struct ufs_hba *hba = platform_get_drvdata(pdev); + struct ufs_rockchip_host *host = ufshcd_get_variant(hba); + + pm_runtime_forbid(&pdev->dev); + pm_runtime_get_noresume(&pdev->dev); + ufshcd_remove(hba); + ufshcd_dealloc_host(hba); + clk_disable_unprepare(host->ref_out_clk); +} + +static int ufs_rockchip_restore_link(struct ufs_hba *hba, bool is_store) +{ + struct ufs_rockchip_host *host = ufshcd_get_variant(hba); + int err, retry = 3; + + if (is_store) { + host->ie = ufshcd_readl(hba, REG_INTERRUPT_ENABLE); + host->ahit = ufshcd_readl(hba, REG_AUTO_HIBERNATE_IDLE_TIMER); + return 0; + } + + /* Enable controller */ + err = ufshcd_hba_enable(hba); + if (err) + return err; + + /* Link startup and wait for DP */ + do { + err = ufshcd_dme_link_startup(hba); + if (!err && ufshcd_is_device_present(hba)) { + dev_dbg_ratelimited(hba->dev, "rockchip link startup successfully.\n"); + break; + } + } while (retry--); + + if (retry < 0) { + dev_err(hba->dev, "rockchip link startup failed.\n"); + return -ENXIO; + } + + /* Restore negotiated power mode */ + err = ufshcd_config_pwr_mode(hba, &(hba->pwr_info)); + if (err) + dev_err(hba->dev, "Failed to restore power mode, err = %d\n", err); + + /* Restore task and transfer list */ + ufshcd_writel(hba, 0xffffffff, REG_INTERRUPT_STATUS); + ufshcd_make_hba_operational(hba); + + /* Restore lost regs */ + ufshcd_writel(hba, host->ie, REG_INTERRUPT_ENABLE); + ufshcd_writel(hba, host->ahit, REG_AUTO_HIBERNATE_IDLE_TIMER); + ufshcd_writel(hba, 0x1, REG_UTP_TRANSFER_REQ_LIST_RUN_STOP); + ufshcd_writel(hba, 0x1, REG_UTP_TASK_REQ_LIST_RUN_STOP); + + return err; +} + +static int ufs_rockchip_runtime_suspend(struct device *dev) +{ + struct ufs_hba *hba = dev_get_drvdata(dev); + struct ufs_rockchip_host *host = ufshcd_get_variant(hba); + + clk_disable_unprepare(host->ref_out_clk); + return ufs_rockchip_restore_link(hba, true); +} + +static int ufs_rockchip_runtime_resume(struct device *dev) +{ + struct ufs_hba *hba = dev_get_drvdata(dev); + struct ufs_rockchip_host *host = ufshcd_get_variant(hba); + int err; + + err = clk_prepare_enable(host->ref_out_clk); + if (err) { + dev_err(hba->dev, "failed to enable ref out clock %d\n", err); + return err; + } + + reset_control_assert(host->rst); + udelay(1); + reset_control_deassert(host->rst); + + return ufs_rockchip_restore_link(hba, false); +} + +static int ufs_rockchip_suspend(struct device *dev) +{ + struct ufs_hba *hba = dev_get_drvdata(dev); + + if (pm_runtime_suspended(hba->dev)) + return 0; + + ufs_rockchip_restore_link(hba, true); + + return 0; +} + +static int ufs_rockchip_resume(struct device *dev) +{ + struct ufs_hba *hba = dev_get_drvdata(dev); + + if (pm_runtime_suspended(hba->dev)) + return 0; + + /* Reset device if possible */ + ufs_rockchip_device_reset(hba); + ufs_rockchip_restore_link(hba, false); + + return 0; +} + +static const struct dev_pm_ops ufs_rockchip_pm_ops = { + SET_SYSTEM_SLEEP_PM_OPS(ufs_rockchip_suspend, ufs_rockchip_resume) + SET_RUNTIME_PM_OPS(ufs_rockchip_runtime_suspend, ufs_rockchip_runtime_resume, NULL) + .prepare = ufshcd_suspend_prepare, + .complete = ufshcd_resume_complete, +}; + +static struct platform_driver ufs_rockchip_pltform = { + .probe = ufs_rockchip_probe, + .remove = ufs_rockchip_remove, + .driver = { + .name = "ufshcd-rockchip", + .pm = &ufs_rockchip_pm_ops, + .of_match_table = ufs_rockchip_of_match, + }, +}; +module_platform_driver(ufs_rockchip_pltform); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("Rockchip UFS Host Driver"); diff --git a/drivers/ufs/host/ufs-rockchip.h b/drivers/ufs/host/ufs-rockchip.h new file mode 100644 index 0000000..9eb80e8 --- /dev/null +++ b/drivers/ufs/host/ufs-rockchip.h @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Rockchip UFS Host Controller driver + * + * Copyright (C) 2024 Rockchip Electronics Co.Ltd. + */ + +#ifndef _UFS_ROCKCHIP_H_ +#define _UFS_ROCKCHIP_H_ + +#define UFS_MAX_CLKS 3 + +#define SEL_TX_LANE0 0x0 +#define SEL_TX_LANE1 0x1 +#define SEL_TX_LANE2 0x2 +#define SEL_TX_LANE3 0x3 +#define SEL_RX_LANE0 0x4 +#define SEL_RX_LANE1 0x5 +#define SEL_RX_LANE2 0x6 +#define SEL_RX_LANE3 0x7 + +#define MIB_T_DBG_CPORT_TX_ENDIAN 0xc022 +#define MIB_T_DBG_CPORT_RX_ENDIAN 0xc023 + +struct ufs_rockchip_host { + struct ufs_hba *hba; + void __iomem *ufs_phy_ctrl; + void __iomem *ufs_sys_ctrl; + void __iomem *mphy_base; + struct gpio_desc *rst_gpio; + struct reset_control *rst; + struct clk *ref_out_clk; + struct clk_bulk_data clks[UFS_MAX_CLKS]; + uint64_t caps; + bool in_suspend; + u32 ie; + u32 ahit; +}; + +#define ufs_sys_writel(base, val, reg) \ + writel((val), (base) + (reg)) +#define ufs_sys_readl(base, reg) readl((base) + (reg)) +#define ufs_sys_set_bits(base, mask, reg) \ + ufs_sys_writel( \ + (base), ((mask) | (ufs_sys_readl((base), (reg)))), (reg)) +#define ufs_sys_ctrl_clr_bits(base, mask, reg) \ + ufs_sys_writel((base), \ + ((~(mask)) & (ufs_sys_readl((base), (reg)))), \ + (reg)) + +#endif /* _UFS_ROCKCHIP_H_ */
RK3576 contains a UFS controller, add init support fot it. Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com> --- Changes in v2: - use dev_probe_err - remove ufs-phy-config-mode as it's not used - drop of_match_ptr drivers/ufs/host/Kconfig | 12 ++ drivers/ufs/host/Makefile | 1 + drivers/ufs/host/ufs-rockchip.c | 438 ++++++++++++++++++++++++++++++++++++++++ drivers/ufs/host/ufs-rockchip.h | 51 +++++ 4 files changed, 502 insertions(+) create mode 100644 drivers/ufs/host/ufs-rockchip.c create mode 100644 drivers/ufs/host/ufs-rockchip.h