Re: [PATCH v4 4/4] pvrusb2: Add Hauppauge HVR1955/1975 devices

[Sean Young <sean@mess.org>]

Hi Brad,

On Mon, Apr 08, 2019 at 01:01:43PM -0500, Brad Love wrote:
> 
> On 05/04/2019 10.29, Sean Young wrote:
> > On Fri, Apr 05, 2019 at 04:24:24PM +0100, Sean Young wrote:
> >> On Wed, Feb 27, 2019 at 01:16:06PM -0600, Brad Love wrote:
> >  > +		pr_info("%s(): resetting 160xxx demod\n", __func__);
> >>> +		/* TODO: not sure this is proper place to reset once only */
> >>> +		pvr2_issue_simple_cmd(hdw,
> >>> +				     FX2CMD_HCW_DEMOD_RESET_PIN |
> >>> +				     (1 << 8) |
> >>> +				     ((0) << 16));
> >>> +		msleep(10);
> >> usleep_range() is preferred (for delays <= 10), I think. Maybe 10ms is too
> >> long anyway and msleep(1) is enough.
> > Sorry that was wrong. usleep_range() is preferred 1ms - 20ms, see:
> >
> > https://www.kernel.org/doc/Documentation/timers/timers-howto.txt
> >
> 
> Hi Shawn,
> 
> Thanks for reviewing things. Please explain what your rationale is for
> arbitrarily reducing the delay to 1ms? I'm a Hauppauge engineer, this is
> a Hauppauge device, I'm going off Hauppauge documentation and reference
> code...and it says 10ms. I'll change to the proper sleep function,
> apologies for missing this instance, but the delay will stay at 10ms as
> that is what my documentation says.

I had not seen a device before that needed reset to be asserted for so long.
Clearly I was completely wrong there.

All I was trying to point out that either the msleep() should be reduced
if possible else usleep_range() should be used. I probably could have
expressed that more clearly.


Sean