Re: [PATCH v8 3/7] rust: time: Introduce Instant type
From: FUJITA Tomonori
Date: Wed Jan 22 2025 - 08:47:07 EST
On Wed, 22 Jan 2025 13:51:21 +0100
Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
> On Wed, Jan 22, 2025 at 1:49 PM FUJITA Tomonori
> <fujita.tomonori@xxxxxxxxx> wrote:
>>
>> On Thu, 16 Jan 2025 21:06:44 +0900 (JST)
>> FUJITA Tomonori <fujita.tomonori@xxxxxxxxx> wrote:
>>
>> > On Thu, 16 Jan 2025 10:32:45 +0100
>> > Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
>> >
>> >>> -impl Ktime {
>> >>> - /// Create a `Ktime` from a raw `ktime_t`.
>> >>> +impl Instant {
>> >>> + /// Create a `Instant` from a raw `ktime_t`.
>> >>> #[inline]
>> >>> - pub fn from_raw(inner: bindings::ktime_t) -> Self {
>> >>> + fn from_raw(inner: bindings::ktime_t) -> Self {
>> >>> Self { inner }
>> >>> }
>> >>
>> >> Please keep this function public.
>> >
>> > Surely, your driver uses from_raw()?
>>
>> I checked out the C version of Binder driver and it doesn't seem like
>> the driver needs from_raw function. The Rust version [1] also doesn't
>> seem to need the function. Do you have a different use case?
>
> Not for this particular function, but I've changed functions called
> from_raw and similar from private to public so many times at this
> point that I think it should be the default.
Then can we remove from_raw()?
We don't use Instant to represent both a specific point in time and a
span of time (we add Delta) so a device driver don't need to create an
Instant from an arbitrary value, I think.
If we allow a device driver to create Instant via from_raw(), we need
to validate a value from the driver. If we create ktime_t only via
ktime_get(), we don't need the details of ktime like a valid range of
ktime_t.