Re: [PATCH 3/3] media: rc: nuvoton: Keep device enabled during reg init

[Sean Young <sean@mess.org>]

On Fri, May 25, 2018 at 03:35:23PM +0200, Michał Winiarski wrote:
> On Thu, May 24, 2018 at 12:31:40PM +0100, Sean Young wrote:
> > On Mon, May 21, 2018 at 04:38:03PM +0200, Michał Winiarski wrote:
> > > Doing writes when the device is disabled seems to be a NOOP.
> > > Let's enable the device, write the values, and then disable it on init.
> > > This changes the behavior for wake device, which is now being disabled
> > > after init.
> > 
> > I don't have the datasheet so I might be misunderstanding this. We want
> > the IR wakeup to work fine even after kernel crash/power loss, right?
> 
> [snip]
> 
> Right, that makes sense. I completely ignored this scenario.
>  
> > > -	/* enable the CIR WAKE logical device */
> > > -	nvt_enable_logical_dev(nvt, LOGICAL_DEV_CIR_WAKE);
> > > +	nvt_disable_logical_dev(nvt, LOGICAL_DEV_CIR);
> > 
> > The way I read this is that the CIR, not CIR_WAKE, is being disabled,
> > which seems contrary to what the commit message says.
> > 
> 
> That's a typo. And by accident it makes the wake_device work correctly :)
> I think that registers init logic was still broken though, operating under the
> assumption that the device is enabled on module load...
> 
> I guess we should just remove disable(LOGICAL_DEV_CIR) from wake_regs_init.
> 
> Have you already included this in any non-rebasing tree?

Nothing has been applied yet.

> Should I send a v2 or fixup on top?

I don't have the hardware to test this, a v2 would be appreciated.

We're late in the release cycle and I'm wondering if this patch would also
solve the nuvoton probe problem:

https://patchwork.linuxtv.org/patch/49874/

Thanks,

Sean