2023-11-15 14:02:44

by Bean Huo

[permalink] [raw]
Subject: Re: [PATCH v1 1/2] scsi: ufs: core: Add UFS RTC support

Hi Thomas,

Thank you for your review. I will resolve the highlighted issue in the
upcoming version. Two separate questions that require individual
answers as below:


On Thu, 2023-11-09 at 15:05 +0100, Thomas Weißschuh wrote:
> >   static int ufs_get_device_desc(struct ufs_hba *hba)
> >   {
> >         int err;
> > @@ -8237,6 +8321,8 @@ static int ufs_get_device_desc(struct ufs_hba
> > *hba)
> >  
> >         ufshcd_temp_notif_probe(hba, desc_buf);
> >  
> > +       ufs_init_rtc(hba, desc_buf);
> > +
>
> As somebody with no idea and no access to the specs:
>
> Is this available for all devices and all protocol versions?
>
> >
I would like to mention that while I cannot confirm that RTC works on
all protocol versions, it has been consistently functional on all
devices in the market since the introduction of UFS 2.0, which also
introduced RTC. I am not aware of any UFS version lower than 2.0
currently available on the market. In the event that a vendor has a
product with a lower UFS version, we can consider implementing a
version check.

> >
> >

> >
>
> >         goto out;
> >  
> >   set_link_active:
> > @@ -9840,6 +9930,8 @@ static int __ufshcd_wl_resume(struct ufs_hba
> > *hba, enum ufs_pm_op pm_op)
> >                 if (ret)
> >                         goto set_old_link_state;
> >                 ufshcd_set_timestamp_attr(hba);
> > +               schedule_delayed_work(&hba->ufs_rtc_delayed_work,
> > +                                                       msecs_to_ji
> > ffies(UFS_RTC_UPDATE_EVERY_MS));
> >         }
> >  
> >         if (ufshcd_keep_autobkops_enabled_except_suspend(hba))
> > diff --git a/include/ufs/ufs.h b/include/ufs/ufs.h
> > index e77ab1786856..18b39c6b3a97 100644
> > --- a/include/ufs/ufs.h
> > +++ b/include/ufs/ufs.h
> > @@ -14,6 +14,7 @@
> >   #include <linux/bitops.h>
> >   #include <linux/types.h>
> >   #include <uapi/scsi/scsi_bsg_ufs.h>
> > +#include <linux/rtc.h>
>
> Seems unnecessary.

seems it's needed, otherwise, I will get:
./include/ufs/ufs.h:599:9: error: unknown type name ‘time64_t’


Kind regards,
Bean