linux-kernel - Re: [PATCH v1 1/2] scsi: ufs: core: Add UFS RTC support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <18f826db-3146-46ca-9214-eb400588131e@t-8ch.de>
Date:   Tue, 14 Nov 2023 19:53:08 +0100
From:   Thomas Weißschuh <thomas@...ch.de>
To:     Bean Huo <beanhuo@...pp.de>
Cc:     avri.altman@....com, bvanassche@....org, alim.akhtar@...sung.com,
        jejb@...ux.ibm.com, martin.petersen@...cle.com,
        stanley.chu@...iatek.com, mani@...nel.org, quic_cang@...cinc.com,
        quic_asutoshd@...cinc.com, beanhuo@...ron.com,
        linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
        mikebi@...ron.com, lporzio@...ron.com
Subject: Re: [PATCH v1 1/2] scsi: ufs: core: Add UFS RTC support

On 2023-11-14 19:27:37+0100, Bean Huo wrote:
> Thank you for your review. I will resolve the highlighted issue in the
> upcoming version. Two separate questions that require individual
> answers as below: 

Thanks for taking the feedback into account!

> 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.

Thanks for the info!
Could you mention this somewhere in the commit message?

> > >         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’

Then it should be "#include <linux/time64.h>", which is more specific.


Thomas