All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Young <sean@mess.org>
To: Samuel Holland <samuel@sholland.org>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>,
	Maxime Ripard <mripard@kernel.org>, Chen-Yu Tsai <wens@csie.org>,
	Jernej Skrabec <jernej.skrabec@siol.net>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-sunxi@googlegroups.com
Subject: Re: [PATCH 1/4] media: sunxi-cir: Clean up dead register writes
Date: Wed, 13 Jan 2021 14:33:43 +0000	[thread overview]
Message-ID: <20210113143343.GA8430@gofer.mess.org> (raw)
In-Reply-To: <20210113045132.31430-2-samuel@sholland.org>

On Tue, Jan 12, 2021 at 10:51:29PM -0600, Samuel Holland wrote:
> The register writes during driver removal occur after the device is
> already put back in reset, so they never had any effect.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
>  drivers/media/rc/sunxi-cir.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> index 8555c7798706..0a7f7eab3cc3 100644
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c
> @@ -342,22 +342,12 @@ static int sunxi_ir_probe(struct platform_device *pdev)
>  
>  static int sunxi_ir_remove(struct platform_device *pdev)
>  {
> -	unsigned long flags;
>  	struct sunxi_ir *ir = platform_get_drvdata(pdev);
>  
>  	clk_disable_unprepare(ir->clk);
>  	clk_disable_unprepare(ir->apb_clk);
>  	reset_control_assert(ir->rst);
>  
> -	spin_lock_irqsave(&ir->ir_lock, flags);
> -	/* disable IR IRQ */
> -	writel(0, ir->base + SUNXI_IR_RXINT_REG);
> -	/* clear All Rx Interrupt Status */
> -	writel(REG_RXSTA_CLEARALL, ir->base + SUNXI_IR_RXSTA_REG);
> -	/* disable IR */
> -	writel(0, ir->base + SUNXI_IR_CTL_REG);
> -	spin_unlock_irqrestore(&ir->ir_lock, flags);
> -
>  	rc_unregister_device(ir->rc);
>  	return 0;
>  }

I don't think there is anything wrong with the patch, however here the
driver does rc_unregister_device() *after* disabling it. Userspace can
still hold a file descriptor open, and call e.g. LIRC_SET_REC_TIMEOUT
ioctl, which causes various writes the sunxi-cir registers.

The order should be reversed.


Sean

  reply	other threads:[~2021-01-13 14:34 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-13  4:51 [PATCH 0/4] media: sunxi-cir: Cleanup and power management Samuel Holland
2021-01-13  4:51 ` [PATCH 1/4] media: sunxi-cir: Clean up dead register writes Samuel Holland
2021-01-13 14:33   ` Sean Young [this message]
2021-01-13  4:51 ` [PATCH 2/4] media: sunxi-cir: Remove unnecessary spinlock Samuel Holland
2021-01-13  4:51 ` [PATCH 3/4] media: sunxi-cir: Factor out hardware initialization Samuel Holland
2021-01-13 14:36   ` Sean Young
2021-01-13 15:00     ` Samuel Holland
2021-01-13 15:09       ` Sean Young
2021-01-13  4:51 ` [PATCH 4/4] media: sunxi-cir: Implement suspend/resume/shutdown callbacks Samuel Holland
2021-01-13  8:26 ` [PATCH 0/4] media: sunxi-cir: Cleanup and power management Maxime Ripard

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=20210113143343.GA8430@gofer.mess.org \
    --to=sean@mess.org \
    --cc=jernej.skrabec@siol.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-sunxi@googlegroups.com \
    --cc=mchehab@kernel.org \
    --cc=mripard@kernel.org \
    --cc=samuel@sholland.org \
    --cc=wens@csie.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.