From: Rodolfo Giometti <giometti@enneenne.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: Alex Komrakov <alexander.komrakov@broadcom.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4] pps: Add elapsed realtime timestamping
Date: Fri, 17 Mar 2023 17:34:28 +0100 [thread overview]
Message-ID: <f8ea9ab9-e1c1-7962-dab4-126beda74046@enneenne.com> (raw)
In-Reply-To: <ZBR3t0D0JEl8feRt@kroah.com>
On 17/03/23 15:22, Greg KH wrote:
> On Fri, Mar 17, 2023 at 03:04:31PM +0100, Rodolfo Giometti wrote:
>> On 17/03/23 10:51, Alex Komrakov wrote:
>>>> + if (!(pps->info.mode & PPS_CAPTURECLEAR))
>>>> + return 0; Why are you not returning an error?
>>> [AK] I used the style in this file sysfs.c.
>>> assert_show() and clear_show() have the same condition.
>>> When '& PPS_CAPTURECLEAR' -- 0 means no interrupt asserted and it is not error
>>> Probably Rodolfo can get more info why return 0
>>
>> It's just as Alex said, if the PPS source has no PPS_CAPTUREASSERT or
>> PPS_CAPTURECLEAR mode it should not print ASSERT and CLEAR info.
>
> But shouldn't you return an error instead of an empty string?
This is not an error... it's just a disabled capability. :)
>>> And why are these sysfs files even present if the mode is not set
>>> properly? Can the mode be set while the device is attached or is this
>>> only defined at probe time? If at probe time, just never create these
>>> files.
>>> [AK] we can understand mode is set when interrupts asserted and
>>> file assert_elapsed will be updated.
>>
>> PPS source's "mode bits" can be set at runtime via PPS_SETPARAMS.
>
> Ok, that's good to know. But I think the error return value is a better
> indication that something went wrong here and this attribute does not
> work for this device at this point in time.
I see... however I suppose several code relays on this behavior.
If we decide to change it, which should be the better way to do it? Any
suggestions are appreciated.
Ciao,
Rodolfo
--
GNU/Linux Solutions e-mail: giometti@enneenne.com
Linux Device Driver giometti@linux.it
Embedded Systems phone: +39 349 2432127
UNIX programming skype: rodolfo.giometti
next prev parent reply other threads:[~2023-03-17 16:34 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-17 7:47 Alexander Komrakov
2023-03-17 7:57 ` Greg KH
[not found] ` <CAMedr-_ssg-baCz94ExMVTeXr2Qivzop1kEpx0S4PWuQdAiGaw@mail.gmail.com>
2023-03-17 11:40 ` Greg KH
2023-03-17 14:04 ` Rodolfo Giometti
2023-03-17 14:22 ` Greg KH
2023-03-17 16:34 ` Rodolfo Giometti [this message]
2023-03-17 17:06 ` Greg KH
[not found] ` <CAMedr-9CvSRmMLhzDatdOvwHDjkxwehOfkEg5WYpxBvEcsYMig@mail.gmail.com>
2023-03-18 7:34 ` Greg KH
2023-03-20 8:59 ` Rodolfo Giometti
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f8ea9ab9-e1c1-7962-dab4-126beda74046@enneenne.com \
--to=giometti@enneenne.com \
--cc=alexander.komrakov@broadcom.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome