On Wed, Oct 24, 2018 at 08:26:51AM -0600, Spencer Olson wrote:
> On Wed, Oct 24, 2018 at 8:18 AM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> >
> > On Wed, Oct 24, 2018 at 07:59:45AM -0600, Spencer E. Olson wrote:
> > > Changes implementation of INSN_CONFIG_GET_CMD_TIMING_CONSTRAINTS for
> > > ni_mio devices to scale the result by the number of channels being used.
> >
> > I really can't understand this statement at all. What changes the
> > implementation?
> >
> > > The user is already required to indicate which channels (and how many
> > > obviously) are intended to be used. There is no point of not using this
> > > information--the analog input cards already similarly scale the timing
> > > results based on the number of channels.
> >
> > This sounds like it's just an optimization but I think this patch is a
> > behavior change or a bug fix, right? It's not totally clear to me from
> > the patch description what the user visible effect of this patch is.
> > Could you spell that out a little bit more clearly and resend the
> > patch? (It's also possible that I just don't understand Comedi well
> > enough to understand the patch description).
> >
> > >
> > > Signed-off-by: Spencer E. Olson <olsonse@xxxxxxxxx>
> > > ---
> > > This patch is made in reference to the last set of patches adding the timing
> > > constraint facility in pci_mio_common
> > > (51fd3673838396844f15de0e906be5333bfbbc8d).
> >
> >
> > I feel like we should be using the Fixes tag for this. Like so:
> >
> > Fixes: 51fd36738383 ("staging: comedi: ni_mio_common: implement INSN_CONFIG_GET_CMD_TIMING_CONSTRAINTS")
> > Signed-off-by: Your <email.com>
>
> I did consider submitting it as a fix instead, but I couldn't actually
> remember what my intentions were when I wrote the original code from
> the earlier patch. On the other hand, after reading more on the
> interface documentation and user code that I have written (not yet
> submitted to the comedi community), I think you are right and that
> this should be submitted as a fix. Thanks
>
That kind of information is actually really interesting to me when I'm
reviewing the code. The changelog could say something like this:
I noticed a bug in the API <blah blah blah we weren't scaling the
timing by the number of channels or something>. I haven't quite
published the user space code for this so this patch doesn't cause
any regressions.
Then I would feel very good about a patch like that.
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
