2020-06-22 10:07:36

by Dariusz Marcinkiewicz

[permalink] [raw]
Subject: [PATCH RESEND] media: cros-ec-cec: do not bail on device_init_wakeup failure

Do not fail probing when device_init_wakeup fails.

device_init_wakeup fails when the device is already enabled as wakeup
device. Hence, the driver fails to probe the device if:
- The device has already been enabled for wakeup (via e.g. sysfs)
- The driver has been unloaded and is being loaded again.

This goal of the patch is to fix the above cases.

Overwhelming majority of the drivers do not check device_init_wakeup
return value.

Signed-off-by: Dariusz Marcinkiewicz <[email protected]>
---
drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
index 0e7e2772f08f..2d95e16cd248 100644
--- a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
+++ b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
@@ -277,11 +277,7 @@ static int cros_ec_cec_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, cros_ec_cec);
cros_ec_cec->cros_ec = cros_ec;

- ret = device_init_wakeup(&pdev->dev, 1);
- if (ret) {
- dev_err(&pdev->dev, "failed to initialize wakeup\n");
- return ret;
- }
+ device_init_wakeup(&pdev->dev, 1);

cros_ec_cec->adap = cec_allocate_adapter(&cros_ec_cec_ops, cros_ec_cec,
DRV_NAME,
--
2.27.0.290.gba653c62da-goog


2020-06-22 10:25:55

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH RESEND] media: cros-ec-cec: do not bail on device_init_wakeup failure

On 22/06/2020 12:05, Dariusz Marcinkiewicz wrote:
> Do not fail probing when device_init_wakeup fails.
>
> device_init_wakeup fails when the device is already enabled as wakeup
> device. Hence, the driver fails to probe the device if:
> - The device has already been enabled for wakeup (via e.g. sysfs)
> - The driver has been unloaded and is being loaded again.
>
> This goal of the patch is to fix the above cases.
>
> Overwhelming majority of the drivers do not check device_init_wakeup
> return value.
>
> Signed-off-by: Dariusz Marcinkiewicz <[email protected]>

This can be CCed to stable, I guess?

Can you provide a Fixes: tag as well?

Regards,

Hans

> ---
> drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
> index 0e7e2772f08f..2d95e16cd248 100644
> --- a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
> +++ b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
> @@ -277,11 +277,7 @@ static int cros_ec_cec_probe(struct platform_device *pdev)
> platform_set_drvdata(pdev, cros_ec_cec);
> cros_ec_cec->cros_ec = cros_ec;
>
> - ret = device_init_wakeup(&pdev->dev, 1);
> - if (ret) {
> - dev_err(&pdev->dev, "failed to initialize wakeup\n");
> - return ret;
> - }
> + device_init_wakeup(&pdev->dev, 1);
>
> cros_ec_cec->adap = cec_allocate_adapter(&cros_ec_cec_ops, cros_ec_cec,
> DRV_NAME,
>

2020-06-24 12:44:42

by Dariusz Marcinkiewicz

[permalink] [raw]
Subject: Re: [PATCH RESEND] media: cros-ec-cec: do not bail on device_init_wakeup failure

Hello.

I realized that I sent the previous answer using HTML, and as a
consequence it was blocked from mailing lists. Sending it again
(apologies for double posting).

On Mon, Jun 22, 2020 at 12:23 PM Hans Verkuil <[email protected]> wrote:
>
> On 22/06/2020 12:05, Dariusz Marcinkiewicz wrote:
> > Do not fail probing when device_init_wakeup fails.
> >
> > device_init_wakeup fails when the device is already enabled as wakeup
> > device. Hence, the driver fails to probe the device if:
> > - The device has already been enabled for wakeup (via e.g. sysfs)
> > - The driver has been unloaded and is being loaded again.
> >
> > This goal of the patch is to fix the above cases.
> >
> > Overwhelming majority of the drivers do not check device_init_wakeup
> > return value.
> >
> > Signed-off-by: Dariusz Marcinkiewicz <[email protected]>
>
> This can be CCed to stable, I guess?
>

That issue is not a recent regression but has been in there since the
very beginning. So it might be argued that is it not severe enough to
warrant cc'ing stable. Happy to do that anyways if you think
otherwise.

> Can you provide a Fixes: tag as well?
>

Done, submitted v2 with that a couple of days ago.

Thank you and best regards.

2020-06-24 14:46:51

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH RESEND] media: cros-ec-cec: do not bail on device_init_wakeup failure

On Wed, Jun 24, 2020 at 5:40 AM Dariusz Marcinkiewicz <[email protected]> wrote:
>
> Hello.
>
> I realized that I sent the previous answer using HTML, and as a
> consequence it was blocked from mailing lists. Sending it again
> (apologies for double posting).
>
> On Mon, Jun 22, 2020 at 12:23 PM Hans Verkuil <[email protected]> wrote:
> >
> > On 22/06/2020 12:05, Dariusz Marcinkiewicz wrote:
> > > Do not fail probing when device_init_wakeup fails.
> > >
> > > device_init_wakeup fails when the device is already enabled as wakeup
> > > device. Hence, the driver fails to probe the device if:
> > > - The device has already been enabled for wakeup (via e.g. sysfs)
> > > - The driver has been unloaded and is being loaded again.
> > >
> > > This goal of the patch is to fix the above cases.
> > >
> > > Overwhelming majority of the drivers do not check device_init_wakeup
> > > return value.
> > >
> > > Signed-off-by: Dariusz Marcinkiewicz <[email protected]>
> >
> > This can be CCed to stable, I guess?
> >
>
> That issue is not a recent regression but has been in there since the
> very beginning. So it might be argued that is it not severe enough to
> warrant cc'ing stable. Happy to do that anyways if you think
> otherwise.
>

Confused. Internally you would like to have this patch applied to
chromeos-4.4. Here you suggest that it may not be important enough to
apply to stable releases. Which one is it ?

Thanks,
Guenter

> > Can you provide a Fixes: tag as well?
> >
>
> Done, submitted v2 with that a couple of days ago.
>
> Thank you and best regards.

2020-06-24 16:40:27

by Dariusz Marcinkiewicz

[permalink] [raw]
Subject: Re: [PATCH RESEND] media: cros-ec-cec: do not bail on device_init_wakeup failure

Hi.

On Wed, Jun 24, 2020 at 4:45 PM Guenter Roeck <[email protected]> wrote:

> > On Mon, Jun 22, 2020 at 12:23 PM Hans Verkuil <[email protected]> wrote:
> > >
> > > > Signed-off-by: Dariusz Marcinkiewicz <[email protected]>
> > >
> > > This can be CCed to stable, I guess?
> > >
> >
> > That issue is not a recent regression but has been in there since the
> > very beginning. So it might be argued that is it not severe enough to
> > warrant cc'ing stable. Happy to do that anyways if you think
> > otherwise.
> >
>
> Confused. Internally you would like to have this patch applied to
> chromeos-4.4. Here you suggest that it may not be important enough to
> apply to stable releases. Which one is it ?
>

It affects us on 4.4, hence the backport to 4.4. I have nothing
against this being merged into the mainline stable. I simply wasn't
sure if that should be considered severe enough to be backported
there. As said before, I am happy to CC this to stable.

Thank you and best regards.

(apologies for double post again)