Linux-IIO Archive mirror
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Matti Vaittinen <mazziesaccount@gmail.com>
Cc: Chenyuan Yang <chenyuan0y@gmail.com>,
	Lars-Peter Clausen <lars@metafoo.de>,
	linux-iio <linux-iio@vger.kernel.org>
Subject: Re: [PATCH] iio: Fix the sorting functionality in iio_gts_build_avail_time_table
Date: Sun, 28 Apr 2024 17:57:35 +0100	[thread overview]
Message-ID: <20240428175735.667b3101@jic23-huawei> (raw)
In-Reply-To: <CANhJrGPsdY+ReiOrVsXCCvTCoVEV0vqcyFyEMB8AS+ae5JBBOQ@mail.gmail.com>

On Fri, 26 Apr 2024 08:52:54 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Hi Jonathan, Chenyuan,
> 
> to 11. huhtik. 2024 klo 15.20 Matti Vaittinen
> (mazziesaccount@gmail.com) kirjoitti:
> >
> > On 3/23/24 20:13, Jonathan Cameron wrote:  
> > > On Wed, 20 Mar 2024 09:02:46 +0200
> > > Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> > >  
> > >> Hi Jonathan, all.
> > >>
> > >> I am resending this as I don't think I found the mail from lore. I wrote
> > >> this using my phone so maybe it was sent as HTML and stuck to some
> > >> filters. If you receive this twice - I am sorry...
> > >>
> > >> la 16. maalisk. 2024 klo 15.40 Jonathan Cameron <jic23@kernel.org
> > >> <mailto:jic23@kernel.org>> kirjoitti:
> > >>  
> > >>   > On Fri, 15 Mar 2024 15:49:10 -0500
> > >>   > Chenyuan Yang <chenyuan0y@gmail.com <mailto:chenyuan0y@gmail.com>>
> > >>   > wrote:
> > >>   >  
> > >>   > > Hi Matti,
> > >>   > >
> > >>   > > Thanks for your reply!
> > >>   > >  
> > >>   > > > I think the suggested-by tag is a bit of an overkill :) I don't feel
> > >>   > > > like taking the credit - you spotted the problem and fixed it!  
> > >>   > >
> > >>   > > You did help me figure out the real issue here and how to fix it :)
> > >>   > >  
> > >>   > > > Do you think you could fix the removal of the duplicates too?  
> > >>   > >
> > >>   > > Sure, I can help to implement the deduplication logic.
> > >>   > > Here is a potential patch for it based on your help.
> > >>   > > Besides, I changed the stop condition in the inner loop to `j < idx`
> > >>   > > since the current last index should be `idx - 1`.  
> > >>   >
> > >>   > Matti, I didn't follow why duplicates are a problem?  
> > >>
> > >> The function here builds the tables for available integration times.
> > >> These are shown to users via sysfs (if I'm not mistaken) - and while the
> > >> user-space algorithms may tolerate dublicates, they are ugly (in my
> > >> opinon) when available times are manually listed.
> > >>
> > >>  
> > >>   > Sure the code is less efficient, but do we end up with a wrong
> > >>   > answer as a result (I've not checked logic, but I'd expect either
> > >>   > first or last of repeating values to be used depending on the alg).  
> > >>
> > >> If we discuss completely omitting duplicated times from the driver
> > >> (which was one thing I referred in my previous mail) - then we are
> > >> likely to face problems as there can be register values, which then
> > >> can't be translated to times, read from a HW.
> > >>
> > >> Eg, we need to have everything described in driver tables used for
> > >> driver's computations - but (in my opinion) we should drop duplicates
> > >> from these tables which we hand over via sysfs.
> > >>  
> > > All makes sense. Thanks for the explanation.  
> >
> > Hi Chenyuan
> >
> > I don't think I have seen a patch fixing this merged? Do you know if I
> > have just missed it? If not, do you still plan to send a fix to this or
> > should I do it?  
> 
> Jonathan, seems like Chenyuan has gone offline. I'd like to get this
> fixed as it'll bite us sooner or later if left like this. What's the
> right way to proceed? Should I craft a patch and add signed-off or
> suggested-by from Chenyuan as he has done most of this already?

If it's closely related to Chenyuan's original patch, keep the authorship
the same and add a Co-developed-by + sign off for yourself.

If you end more or less writing a fresh patch, then a Suggested-by is fine.

Jonathan

> 
> Yours,
>    -- Matti
> 
> > --
> > Matti Vaittinen
> > Linux kernel developer at ROHM Semiconductors
> > Oulu Finland
> >
> > ~~ When things go utterly wrong vim users can always type :help! ~~
> >  
> 
> 


      reply	other threads:[~2024-04-28 16:57 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-13 15:57 [PATCH] iio: Fix the sorting functionality in iio_gts_build_avail_time_table Chenyuan Yang
2024-03-14 15:36 ` Jonathan Cameron
2024-03-14 17:05   ` Matti Vaittinen
2024-03-15  7:55 ` Matti Vaittinen
2024-03-15 20:49   ` Chenyuan Yang
2024-03-16 13:40     ` Jonathan Cameron
2024-03-20  7:02       ` Matti Vaittinen
2024-03-23 18:13         ` Jonathan Cameron
2024-04-11 12:19           ` Matti Vaittinen
2024-04-26  5:52             ` Matti Vaittinen
2024-04-28 16:57               ` Jonathan Cameron [this message]

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=20240428175735.667b3101@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=chenyuan0y@gmail.com \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=mazziesaccount@gmail.com \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).