Re: [PATCH v2 20/23] staging: qlge: Fix CHECK: usleep_range is preferred over udelay
Hello!
On 11.12.2019 21:12, Scott Schafer wrote:
chage udelay() to usleep_range()
Change?
Signed-off-by: Scott Schafer
---
drivers/staging/qlge/qlge_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index e18aa335c899..9427386e4a1e 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -147,7 +147,7 @@ int ql_sem_spinlock(struct ql_adapter *qdev, u32 sem_mask)
do {
if (!ql_sem_trylock(qdev, sem_mask))
return 0;
- udelay(100);
+ usleep_range(100, 200);
I hope you're not in atomic context...
} while (--wait_count);
return -ETIMEDOUT;
}
MBR, Sergei
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH for-5.6 2/4] staging: bcm2835-audio: Use managed buffer allocation
On Tue, 2019-12-10 at 15:13 +0100, Takashi Iwai wrote: > Clean up the driver with the new managed buffer allocation API. > The hw_params and hw_free callbacks became superfluous and dropped. > > Signed-off-by: Takashi Iwai Reviewed-by: Nicolas Saenz Julienne Thanks! signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH for-5.6 4/4] staging: bcm2835-audio: Drop superfluous ioctl PCM ops
On Tue, 2019-12-10 at 15:13 +0100, Takashi Iwai wrote: > PCM core deals the empty ioctl field now as default. > Let's kill the redundant lines. > > Signed-off-by: Takashi Iwai Reviewed-by: Nicolas Saenz Julienne Thanks! signature.asc Description: This is a digitally signed message part ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 20/23] staging: qlge: Fix CHECK: usleep_range is preferred over udelay
On Thu, Dec 12, 2019 at 01:45:57PM +0300, Sergei Shtylyov wrote:
> Hello!
>
> On 11.12.2019 21:12, Scott Schafer wrote:
>
> > chage udelay() to usleep_range()
>
>Change?
>
> > Signed-off-by: Scott Schafer
> > ---
> > drivers/staging/qlge/qlge_main.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/qlge/qlge_main.c
> > b/drivers/staging/qlge/qlge_main.c
> > index e18aa335c899..9427386e4a1e 100644
> > --- a/drivers/staging/qlge/qlge_main.c
> > +++ b/drivers/staging/qlge/qlge_main.c
> > @@ -147,7 +147,7 @@ int ql_sem_spinlock(struct ql_adapter *qdev, u32
> > sem_mask)
> > do {
> > if (!ql_sem_trylock(qdev, sem_mask))
> > return 0;
> > - udelay(100);
> > + usleep_range(100, 200);
>
>I hope you're not in atomic context...
>
> > } while (--wait_count);
> > return -ETIMEDOUT;
> > }
>
> MBR, Sergei
Im not quite what you mean by "I hope you're not in atomic context",
could you please explain why you said this?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 20/23] staging: qlge: Fix CHECK: usleep_range is preferred over udelay
On Thu, Dec 12, 2019 at 01:45:57PM +0300, Sergei Shtylyov wrote:
> Hello!
>
> On 11.12.2019 21:12, Scott Schafer wrote:
>
> > chage udelay() to usleep_range()
>
>Change?
>
> > Signed-off-by: Scott Schafer
> > ---
> > drivers/staging/qlge/qlge_main.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/qlge/qlge_main.c
> > b/drivers/staging/qlge/qlge_main.c
> > index e18aa335c899..9427386e4a1e 100644
> > --- a/drivers/staging/qlge/qlge_main.c
> > +++ b/drivers/staging/qlge/qlge_main.c
> > @@ -147,7 +147,7 @@ int ql_sem_spinlock(struct ql_adapter *qdev, u32
> > sem_mask)
> > do {
> > if (!ql_sem_trylock(qdev, sem_mask))
> > return 0;
> > - udelay(100);
> > + usleep_range(100, 200);
>
>I hope you're not in atomic context...
I have an unpublished Smatch check which says that we aren't in atomic
context, but still this has spin_lock() in the name so you're right, it
shouldn't sleep.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 20/23] staging: qlge: Fix CHECK: usleep_range is preferred over udelay
On Thu, Dec 12, 2019 at 05:00:57AM -0600, Scott Schafer wrote:
> On Thu, Dec 12, 2019 at 01:45:57PM +0300, Sergei Shtylyov wrote:
> > Hello!
> >
> > On 11.12.2019 21:12, Scott Schafer wrote:
> >
> > > chage udelay() to usleep_range()
> >
> >Change?
> >
> > > Signed-off-by: Scott Schafer
> > > ---
> > > drivers/staging/qlge/qlge_main.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/qlge/qlge_main.c
> > > b/drivers/staging/qlge/qlge_main.c
> > > index e18aa335c899..9427386e4a1e 100644
> > > --- a/drivers/staging/qlge/qlge_main.c
> > > +++ b/drivers/staging/qlge/qlge_main.c
> > > @@ -147,7 +147,7 @@ int ql_sem_spinlock(struct ql_adapter *qdev, u32
> > > sem_mask)
> > > do {
> > > if (!ql_sem_trylock(qdev, sem_mask))
> > > return 0;
> > > - udelay(100);
> > > + usleep_range(100, 200);
> >
> >I hope you're not in atomic context...
> >
> > > } while (--wait_count);
> > > return -ETIMEDOUT;
> > > }
> >
> > MBR, Sergei
>
> Im not quite what you mean by "I hope you're not in atomic context",
> could you please explain why you said this?
You can't sleep from certain IRQs or when you are holding certain locks
(spin_locks and rwlocks). The we have preempt_disable() then you can't
sleep.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: imx7-mipi-csis: Add the missed v4l2_async_notifier_cleanup in remove
Steve, Philipp, I'd like one (or both) of you to look over this first. It looks as if the subdev_notifier field of struct csi_state is never used, except by the existing v4l2_async_notifier_unregister() call. If I am right, then the real issue is that that field should be removed. Regards, Hans On 12/11/19 11:59 AM, Rui Miguel Silva wrote: > Hi Chuhong, > Thanks for the patch. > > On Mon, Dec 09, 2019 at 04:58:28PM +0800, Chuhong Yuan wrote: >> All drivers in imx call v4l2_async_notifier_cleanup() after unregistering >> the notifier except this driver. >> This should be a miss and we need to add the call to fix it. >> >> Signed-off-by: Chuhong Yuan > > Reviewed-by: Rui Miguel Silva > > -- > Cheers, > Rui >> --- >> drivers/staging/media/imx/imx7-mipi-csis.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c >> b/drivers/staging/media/imx/imx7-mipi-csis.c >> index 99166afca071..2bfa85bb84e7 100644 >> --- a/drivers/staging/media/imx/imx7-mipi-csis.c >> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c >> @@ -1105,6 +1105,7 @@ static int mipi_csis_remove(struct platform_device >> *pdev) >> mipi_csis_debugfs_exit(state); >> v4l2_async_unregister_subdev(&state->mipi_sd); >> v4l2_async_notifier_unregister(&state->subdev_notifier); >> +v4l2_async_notifier_cleanup(&state->subdev_notifier); >> >> pm_runtime_disable(&pdev->dev); >> mipi_csis_pm_suspend(&pdev->dev, true); >> -- >> 2.24.0 >> ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] media: allegro: add the missed check for v4l2_m2m_ctx_init
On 12/10/19 4:15 AM, Chuhong Yuan wrote:
> allegro_open() misses a check for v4l2_m2m_ctx_init().
> Add a check and error handling code to fix it.
>
> Fixes: f20387dfd065 ("media: allegro: add Allegro DVT video IP core driver")
> Signed-off-by: Chuhong Yuan
> ---
> Changes in v2:
> - Fix the use-after-free in v1.
>
> drivers/staging/media/allegro-dvt/allegro-core.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/staging/media/allegro-dvt/allegro-core.c
> b/drivers/staging/media/allegro-dvt/allegro-core.c
> index 6f0cd0784786..66736beb67af 100644
> --- a/drivers/staging/media/allegro-dvt/allegro-core.c
> +++ b/drivers/staging/media/allegro-dvt/allegro-core.c
> @@ -2270,6 +2270,7 @@ static int allegro_open(struct file *file)
> struct allegro_channel *channel = NULL;
> struct v4l2_ctrl_handler *handler;
> u64 mask;
> + int ret;
>
> channel = kzalloc(sizeof(*channel), GFP_KERNEL);
> if (!channel)
> @@ -2341,6 +2342,14 @@ static int allegro_open(struct file *file)
> channel->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, channel,
> allegro_queue_init);
>
> + if (IS_ERR(channel->fh.m2m_ctx)) {
> + ret = PTR_ERR(channel->fh.m2m_ctx);
> + v4l2_fh_del(&channel->fh);
> + v4l2_fh_exit(&channel->fh);
Just move the v4l2_fh_init/add calls to just before the return 0, i.e. when
everything
is right. Then you don't need to call del/exit on error.
Also, I see that the result of all the v4l2_ctrl_new* calls isn't checked.
Just after the last v4l2_ctrl_new_std() call you need to check handler->error:
if not 0, then there was an error and you need to call v4l2_ctrl_handler_free
to clean it up.
Regards,
Hans
> + kfree(channel);
> + return ret;
> + }
> +
> return 0;
> }
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: imx7-mipi-csis: Add the missed v4l2_async_notifier_cleanup in remove
On Mon, Dec 09, 2019 at 04:58:28PM +0800, Chuhong Yuan wrote: > All drivers in imx call v4l2_async_notifier_cleanup() after unregistering > the notifier except this driver. > This should be a miss and we need to add the call to fix it. > > Signed-off-by: Chuhong Yuan > --- > drivers/staging/media/imx/imx7-mipi-csis.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c > b/drivers/staging/media/imx/imx7-mipi-csis.c > index 99166afca071..2bfa85bb84e7 100644 > --- a/drivers/staging/media/imx/imx7-mipi-csis.c > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > @@ -1105,6 +1105,7 @@ static int mipi_csis_remove(struct platform_device > *pdev) > mipi_csis_debugfs_exit(state); > v4l2_async_unregister_subdev(&state->mipi_sd); > v4l2_async_notifier_unregister(&state->subdev_notifier); > + v4l2_async_notifier_cleanup(&state->subdev_notifier); > In this case the "state->subdev_notifier" was never initialized or used so both v4l2_async_notifier_unregister() and v4l2_async_notifier_cleanup() are no-ops. We should just delete "subdev_notifier". regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: imx7-mipi-csis: Add the missed v4l2_async_notifier_cleanup in remove
On Thu, 2019-12-12 at 14:51 +0300, Dan Carpenter wrote: > On Mon, Dec 09, 2019 at 04:58:28PM +0800, Chuhong Yuan wrote: > > All drivers in imx call v4l2_async_notifier_cleanup() after unregistering > > the notifier except this driver. > > This should be a miss and we need to add the call to fix it. > > > > Signed-off-by: Chuhong Yuan > > --- > > drivers/staging/media/imx/imx7-mipi-csis.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c > > b/drivers/staging/media/imx/imx7-mipi-csis.c > > index 99166afca071..2bfa85bb84e7 100644 > > --- a/drivers/staging/media/imx/imx7-mipi-csis.c > > +++ b/drivers/staging/media/imx/imx7-mipi-csis.c > > @@ -1105,6 +1105,7 @@ static int mipi_csis_remove(struct platform_device > > *pdev) > > mipi_csis_debugfs_exit(state); > > v4l2_async_unregister_subdev(&state->mipi_sd); > > v4l2_async_notifier_unregister(&state->subdev_notifier); > > + v4l2_async_notifier_cleanup(&state->subdev_notifier); > > > > In this case the "state->subdev_notifier" was never initialized or used > so both v4l2_async_notifier_unregister() and v4l2_async_notifier_cleanup() > are no-ops. > > We should just delete "subdev_notifier". I agree with Dan and Hans, the subdev_notifier field and the v4l2_async_notifier_unregister() call should be removed. Since this issue was there from the start, the patch can be tagged as fixing commit 7807063b862b. regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 11/23] staging: qlge: Fix CHECK: braces {} should be used on all arms of this statement
On Wed, Dec 11, 2019 at 12:12:40PM -0600, Scott Schafer wrote:
> @@ -351,8 +352,9 @@ static int ql_aen_lost(struct ql_adapter *qdev, struct
> mbox_params *mbcp)
> mbcp->out_count = 6;
>
> status = ql_get_mb_sts(qdev, mbcp);
> - if (status)
> + if (status) {
> netif_err(qdev, drv, qdev->ndev, "Lost AEN broken!\n");
> + }
> else {
The close } should be on the same line as the else.
> int i;
>
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 11/23] staging: qlge: Fix CHECK: braces {} should be used on all arms of this statement
On Thu, Dec 12, 2019 at 03:12:06PM +0300, Dan Carpenter wrote:
> On Wed, Dec 11, 2019 at 12:12:40PM -0600, Scott Schafer wrote:
> > @@ -351,8 +352,9 @@ static int ql_aen_lost(struct ql_adapter *qdev, struct
> > mbox_params *mbcp)
> > mbcp->out_count = 6;
> >
> > status = ql_get_mb_sts(qdev, mbcp);
> > - if (status)
> > + if (status) {
> > netif_err(qdev, drv, qdev->ndev, "Lost AEN broken!\n");
> > + }
> > else {
>
> The close } should be on the same line as the else.
>
> > int i;
> >
>
> regards,
> dan carpenter
this was fixed in patch 22
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 11/23] staging: qlge: Fix CHECK: braces {} should be used on all arms of this statement
On Thu, Dec 12, 2019 at 09:02:00AM -0600, Scott Schafer wrote:
> On Thu, Dec 12, 2019 at 03:12:06PM +0300, Dan Carpenter wrote:
> > On Wed, Dec 11, 2019 at 12:12:40PM -0600, Scott Schafer wrote:
> > > @@ -351,8 +352,9 @@ static int ql_aen_lost(struct ql_adapter *qdev,
> > > struct mbox_params *mbcp)
> > > mbcp->out_count = 6;
> > >
> > > status = ql_get_mb_sts(qdev, mbcp);
> > > - if (status)
> > > + if (status) {
> > > netif_err(qdev, drv, qdev->ndev, "Lost AEN broken!\n");
> > > + }
> > > else {
> >
> > The close } should be on the same line as the else.
> >
> > > int i;
> > >
> >
> > regards,
> > dan carpenter
>
> this was fixed in patch 22
Don't add a warning that you later fix up :)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 11/23] staging: qlge: Fix CHECK: braces {} should be used on all arms of this statement
On Thu, Dec 12, 2019 at 09:02:00AM -0600, Scott Schafer wrote:
> On Thu, Dec 12, 2019 at 03:12:06PM +0300, Dan Carpenter wrote:
> > On Wed, Dec 11, 2019 at 12:12:40PM -0600, Scott Schafer wrote:
> > > @@ -351,8 +352,9 @@ static int ql_aen_lost(struct ql_adapter *qdev,
> > > struct mbox_params *mbcp)
> > > mbcp->out_count = 6;
> > >
> > > status = ql_get_mb_sts(qdev, mbcp);
> > > - if (status)
> > > + if (status) {
> > > netif_err(qdev, drv, qdev->ndev, "Lost AEN broken!\n");
> > > + }
> > > else {
> >
> > The close } should be on the same line as the else.
> >
> > > int i;
> > >
> >
> > regards,
> > dan carpenter
>
> this was fixed in patch 22
The truth is that I don't care at all about tiny typos like this, but
in the future then the right way to fix these is to create a separate
patch for this, and the use git rebase to fold it back into this patch.
It's a pretty straight forward process.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: imx7-mipi-csis: Add the missed v4l2_async_notifier_cleanup in remove
Hi Dan, Thanks for the inputs. On Thu, Dec 12, 2019 at 02:51:34PM +0300, Dan Carpenter wrote: > On Mon, Dec 09, 2019 at 04:58:28PM +0800, Chuhong Yuan wrote: > > All drivers in imx call v4l2_async_notifier_cleanup() after > > unregistering the notifier except this driver. This should be a > > miss and we need to add the call to fix it. > > > > Signed-off-by: Chuhong Yuan --- > > drivers/staging/media/imx/imx7-mipi-csis.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c > > b/drivers/staging/media/imx/imx7-mipi-csis.c index > > 99166afca071..2bfa85bb84e7 100644 --- > > a/drivers/staging/media/imx/imx7-mipi-csis.c +++ > > b/drivers/staging/media/imx/imx7-mipi-csis.c @@ -1105,6 +1105,7 @@ > > static int mipi_csis_remove(struct platform_device *pdev) > > mipi_csis_debugfs_exit(state); > > v4l2_async_unregister_subdev(&state->mipi_sd); > > v4l2_async_notifier_unregister(&state->subdev_notifier); + > > v4l2_async_notifier_cleanup(&state->subdev_notifier); > > > > In this case the "state->subdev_notifier" was never initialized or > used so both v4l2_async_notifier_unregister() and > v4l2_async_notifier_cleanup() are no-ops. I have applied this patch on top of Steve's series [0], since by the timeline I was expecting to be applied before this one, that series adds a bound notifier, even though, it is not named the same, eheh. That trigged me to think that this cleanup was correct since a notifier was initialized in probe. But as you say, it is a no-ops in the end. @Steve, that said, it looks that in [0], you will need to add some unregister and cleanup for the notifiers that you are adding in several places. A patch to fix this will follow. -- Cheers, Rui [0]: https://patchwork.kernel.org/project/linux-media/list/?series=207517 > > We should just delete "subdev_notifier". > > regards, dan carpenter > > ___ devel mailing list > de...@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] media: imx7-mipi-csis: remove subdev_notifier
It was defined a notifier in the csi_state structure that is never
allocated. And besides that it's unregister in the remove, even though
it is a no-op, just remove both.
Fixes: 7807063b862b ("media: staging/imx7: add MIPI CSI-2 receiver subdev for
i.MX7")
Reported-by: Hans Verkuil
Suggested-by: Dan Carpenter
Suggested-by: Philipp Zabel
Signed-off-by: Rui Miguel Silva
---
drivers/staging/media/imx/imx7-mipi-csis.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c
b/drivers/staging/media/imx/imx7-mipi-csis.c
index 99166afca071..383abecb3bec 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -251,8 +251,6 @@ struct csi_state {
struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
- struct v4l2_async_notifier subdev_notifier;
-
struct csis_hw_reset hw_reset;
struct regulator *mipi_phy_regulator;
bool sink_linked;
@@ -1104,7 +1102,6 @@ static int mipi_csis_remove(struct platform_device *pdev)
mipi_csis_debugfs_exit(state);
v4l2_async_unregister_subdev(&state->mipi_sd);
- v4l2_async_notifier_unregister(&state->subdev_notifier);
pm_runtime_disable(&pdev->dev);
mipi_csis_pm_suspend(&pdev->dev, true);
--
2.24.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 11/23] staging: qlge: Fix CHECK: braces {} should be used on all arms of this statement
From: Scott Schafer
Date: Thu, 12 Dec 2019 09:02:00 -0600
> On Thu, Dec 12, 2019 at 03:12:06PM +0300, Dan Carpenter wrote:
>> On Wed, Dec 11, 2019 at 12:12:40PM -0600, Scott Schafer wrote:
>> > @@ -351,8 +352,9 @@ static int ql_aen_lost(struct ql_adapter *qdev, struct
>> > mbox_params *mbcp)
>> >mbcp->out_count = 6;
>> >
>> >status = ql_get_mb_sts(qdev, mbcp);
>> > - if (status)
>> > + if (status) {
>> >netif_err(qdev, drv, qdev->ndev, "Lost AEN broken!\n");
>> > + }
>> >else {
>>
>> The close } should be on the same line as the else.
>>
>> >int i;
>> >
>>
>> regards,
>> dan carpenter
>
> this was fixed in patch 22
It should not be introduced in the first place. Therefore it needs to be dealt
with
in this patch.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[driver-core:debugfs_remove_return_value 2/2] drivers/crypto/hisilicon/hpre/hpre_main.c:598:6: error: void value not ignored as it ought to be
tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git
debugfs_remove_return_value
head: ccebb9244d66f0fe7d7fd6a6fe45d0d5a812bd60
commit: ccebb9244d66f0fe7d7fd6a6fe45d0d5a812bd60 [2/2] debugfs: remove return
value of debugfs_create_regset32()
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.5.0-1) 7.5.0
reproduce:
git checkout ccebb9244d66f0fe7d7fd6a6fe45d0d5a812bd60
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
drivers/crypto/hisilicon/hpre/hpre_main.c: In function
'hpre_pf_comm_regs_debugfs_init':
>> drivers/crypto/hisilicon/hpre/hpre_main.c:598:6: error: void value not
>> ignored as it ought to be
tmp = debugfs_create_regset32("regs", 0444, debug->debug_root, regset);
^
drivers/crypto/hisilicon/hpre/hpre_main.c: In function
'hpre_cluster_debugfs_init':
drivers/crypto/hisilicon/hpre/hpre_main.c:630:7: error: void value not
ignored as it ought to be
tmp = debugfs_create_regset32("regs", 0444, tmp_d, regset);
^
sparse warnings: (new ones prefixed by >>)
>> drivers/crypto/hisilicon/hpre/hpre_main.c:598:13: sparse: sparse: incorrect
>> type in assignment (different base types)
drivers/crypto/hisilicon/hpre/hpre_main.c:598:13: sparse:expected struct
dentry *tmp
drivers/crypto/hisilicon/hpre/hpre_main.c:598:13: sparse:got void
drivers/crypto/hisilicon/hpre/hpre_main.c:630:21: sparse: sparse: incorrect
type in assignment (different base types)
drivers/crypto/hisilicon/hpre/hpre_main.c:630:21: sparse:expected struct
dentry *tmp
drivers/crypto/hisilicon/hpre/hpre_main.c:630:21: sparse:got void
vim +598 drivers/crypto/hisilicon/hpre/hpre_main.c
8489741516182d Zaibo Xu 2019-09-30 581
8489741516182d Zaibo Xu 2019-09-30 582 static int
hpre_pf_comm_regs_debugfs_init(struct hpre_debug *debug)
8489741516182d Zaibo Xu 2019-09-30 583 {
8489741516182d Zaibo Xu 2019-09-30 584 struct hpre *hpre =
container_of(debug, struct hpre, debug);
8489741516182d Zaibo Xu 2019-09-30 585 struct hisi_qm *qm = &hpre->qm;
8489741516182d Zaibo Xu 2019-09-30 586 struct device *dev =
&qm->pdev->dev;
8489741516182d Zaibo Xu 2019-09-30 587 struct debugfs_regset32 *regset;
8489741516182d Zaibo Xu 2019-09-30 588 struct dentry *tmp;
8489741516182d Zaibo Xu 2019-09-30 589
8489741516182d Zaibo Xu 2019-09-30 590 regset = devm_kzalloc(dev,
sizeof(*regset), GFP_KERNEL);
8489741516182d Zaibo Xu 2019-09-30 591 if (!regset)
8489741516182d Zaibo Xu 2019-09-30 592 return -ENOMEM;
8489741516182d Zaibo Xu 2019-09-30 593
8489741516182d Zaibo Xu 2019-09-30 594 regset->regs =
hpre_com_dfx_regs;
8489741516182d Zaibo Xu 2019-09-30 595 regset->nregs =
ARRAY_SIZE(hpre_com_dfx_regs);
8489741516182d Zaibo Xu 2019-09-30 596 regset->base = qm->io_base;
8489741516182d Zaibo Xu 2019-09-30 597
8489741516182d Zaibo Xu 2019-09-30 @598 tmp =
debugfs_create_regset32("regs", 0444, debug->debug_root, regset);
8489741516182d Zaibo Xu 2019-09-30 599 if (!tmp)
8489741516182d Zaibo Xu 2019-09-30 600 return -ENOENT;
8489741516182d Zaibo Xu 2019-09-30 601
8489741516182d Zaibo Xu 2019-09-30 602 return 0;
8489741516182d Zaibo Xu 2019-09-30 603 }
8489741516182d Zaibo Xu 2019-09-30 604
:: The code at line 598 was first introduced by commit
:: 8489741516182d8ac57a69e9f4ca963450607351 crypto: hisilicon - Add debugfs
for HPRE
:: TO: Zaibo Xu
:: CC: Herbert Xu
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org Intel Corporation
.config.gz
Description: application/gzip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: imx7-mipi-csis: Add the missed v4l2_async_notifier_cleanup in remove
On 12/12/19 11:08 AM, Rui Miguel Silva wrote: Hi Dan, Thanks for the inputs. On Thu, Dec 12, 2019 at 02:51:34PM +0300, Dan Carpenter wrote: On Mon, Dec 09, 2019 at 04:58:28PM +0800, Chuhong Yuan wrote: All drivers in imx call v4l2_async_notifier_cleanup() after unregistering the notifier except this driver. This should be a miss and we need to add the call to fix it. Signed-off-by: Chuhong Yuan --- drivers/staging/media/imx/imx7-mipi-csis.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c b/drivers/staging/media/imx/imx7-mipi-csis.c index 99166afca071..2bfa85bb84e7 100644 --- a/drivers/staging/media/imx/imx7-mipi-csis.c +++ b/drivers/staging/media/imx/imx7-mipi-csis.c @@ -1105,6 +1105,7 @@ static int mipi_csis_remove(struct platform_device *pdev) mipi_csis_debugfs_exit(state); v4l2_async_unregister_subdev(&state->mipi_sd); v4l2_async_notifier_unregister(&state->subdev_notifier); + v4l2_async_notifier_cleanup(&state->subdev_notifier); In this case the "state->subdev_notifier" was never initialized or used so both v4l2_async_notifier_unregister() and v4l2_async_notifier_cleanup() are no-ops. I have applied this patch on top of Steve's series [0], since by the timeline I was expecting to be applied before this one, that series adds a bound notifier, even though, it is not named the same, eheh. That trigged me to think that this cleanup was correct since a notifier was initialized in probe. But as you say, it is a no-ops in the end. @Steve, that said, it looks that in [0], you will need to add some unregister and cleanup for the notifiers that you are adding in several places. Well, turns out I had failed to notice that an async notifier was already included in 'struct imx7_csi' as 'subdev_notifier', even though it was unused. So I ended up creating a duplicate 'notifier'. I'll cleaning that up in v3 of [0]. Steve A patch to fix this will follow. -- Cheers, Rui [0]: https://patchwork.kernel.org/project/linux-media/list/?series=207517 We should just delete "subdev_notifier". regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: imx7-mipi-csis: remove subdev_notifier
On 12/12/19 11:17 AM, Rui Miguel Silva wrote:
It was defined a notifier in the csi_state structure that is never
allocated. And besides that it's unregister in the remove, even though
it is a no-op, just remove both.
Fixes: 7807063b862b ("media: staging/imx7: add MIPI CSI-2 receiver subdev for
i.MX7")
Reported-by: Hans Verkuil
Suggested-by: Dan Carpenter
Suggested-by: Philipp Zabel
Signed-off-by: Rui Miguel Silva
Reviewed-by: Steve Longerbeam
---
drivers/staging/media/imx/imx7-mipi-csis.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c
b/drivers/staging/media/imx/imx7-mipi-csis.c
index 99166afca071..383abecb3bec 100644
--- a/drivers/staging/media/imx/imx7-mipi-csis.c
+++ b/drivers/staging/media/imx/imx7-mipi-csis.c
@@ -251,8 +251,6 @@ struct csi_state {
struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
- struct v4l2_async_notifier subdev_notifier;
-
struct csis_hw_reset hw_reset;
struct regulator *mipi_phy_regulator;
bool sink_linked;
@@ -1104,7 +1102,6 @@ static int mipi_csis_remove(struct platform_device *pdev)
mipi_csis_debugfs_exit(state);
v4l2_async_unregister_subdev(&state->mipi_sd);
- v4l2_async_notifier_unregister(&state->subdev_notifier);
pm_runtime_disable(&pdev->dev);
mipi_csis_pm_suspend(&pdev->dev, true);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: meson: add missing allocation failure check on new_buf
On Thu, Dec 5, 2019 at 9:06 AM Sergey Senozhatsky wrote: > > On (19/12/04 14:11), Colin King wrote: > [..] > > diff --git a/drivers/staging/media/meson/vdec/vdec.c > > b/drivers/staging/media/meson/vdec/vdec.c > > index 0a1a04fd5d13..8dd1396909d7 100644 > > --- a/drivers/staging/media/meson/vdec/vdec.c > > +++ b/drivers/staging/media/meson/vdec/vdec.c > > @@ -133,6 +133,8 @@ vdec_queue_recycle(struct amvdec_session *sess, struct > > vb2_buffer *vb) > > struct amvdec_buffer *new_buf; > > > > new_buf = kmalloc(sizeof(*new_buf), GFP_KERNEL); > > + if (!new_buf) > > + return; > > new_buf->vb = vb; Thanks for the patch Colin. > > So the buffer is not getting recycled? IOW is leaked? > > -ss The "recycle" mechanism in the meson vdec is a way to tell the firmware that "hey, both userspace and kernel are done using this buffer, you can start using it again". Not queuing it for recycling means that the firmware won't use this buffer again, it's not desirable of course, but if there is no memory left to allocate a simple list element then there are bigger problems at hand. Either way, failing this allocation and returning instantly doesn't leak anything or do any damage kernel-side. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] media: imx7-mipi-csis: remove subdev_notifier
On 12/12/19 8:17 PM, Rui Miguel Silva wrote:
> It was defined a notifier in the csi_state structure that is never
> allocated. And besides that it's unregister in the remove, even though
> it is a no-op, just remove both.
>
> Fixes: 7807063b862b ("media: staging/imx7: add MIPI CSI-2 receiver subdev for
> i.MX7")
> Reported-by: Hans Verkuil
> Suggested-by: Dan Carpenter
> Suggested-by: Philipp Zabel
> Signed-off-by: Rui Miguel Silva
Mismatch between this Signed-off-by and your email address.
Is it OK if I use your linaro email in this Signed-off-by?
Regards,
Hans
> ---
> drivers/staging/media/imx/imx7-mipi-csis.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c
> b/drivers/staging/media/imx/imx7-mipi-csis.c
> index 99166afca071..383abecb3bec 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -251,8 +251,6 @@ struct csi_state {
>
> struct mipi_csis_event events[MIPI_CSIS_NUM_EVENTS];
>
> - struct v4l2_async_notifier subdev_notifier;
> -
> struct csis_hw_reset hw_reset;
> struct regulator *mipi_phy_regulator;
> bool sink_linked;
> @@ -1104,7 +1102,6 @@ static int mipi_csis_remove(struct platform_device
> *pdev)
>
> mipi_csis_debugfs_exit(state);
> v4l2_async_unregister_subdev(&state->mipi_sd);
> - v4l2_async_notifier_unregister(&state->subdev_notifier);
>
> pm_runtime_disable(&pdev->dev);
> mipi_csis_pm_suspend(&pdev->dev, true);
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
