Hi Dmitry,

Subject: Re: [PATCH v7 7/7] input: keyboard: support i.MX95 BBM
module

On Thu, Aug 01, 2024 at 01:36:10AM +0000, Peng Fan wrote:

quoted

Hi Dmitry,

quoted

Subject: Re: [PATCH v7 7/7] input: keyboard: support i.MX95 BBM
module

Hi Peng,

On Wed, Jul 31, 2024 at 03:37:18PM +0000, Peng Fan wrote:

quoted

Hi Cristian,

quoted

Subject: Re: [PATCH v7 7/7] input: keyboard: support i.MX95

BBM

quoted

quoted

quoted

quoted

module

On Wed, Jul 31, 2024 at 08:56:11PM +0800, Peng Fan (OSS)

wrote:

quoted

quoted

quoted

quoted

quoted

From: Peng Fan <peng.fan@nxp.com>

The BBM module provides BUTTON feature. To i.MX95, this

module is

quoted

quoted

quoted

managed by System Manager and exported using System

Management Control

quoted

Interface(SCMI). Linux could use i.MX SCMI BBM Extension

protocol

quoted

quoted

to

quoted

use BUTTON feature.

This driver is to use SCMI interface to enable pwrkey.

+}
+
+static void scmi_imx_bbm_key_remove(struct scmi_device

*sdev) {

quoted

quoted

quoted

+	struct device *dev = &sdev->dev;
+	struct scmi_imx_bbm *bbnsm = dev_get_drvdata(dev);
+
+	device_init_wakeup(dev, false);

I do not believe you need to reset the wakeup flag on driver unbind,
as well as in the error handling path of probe(). If this is needed
then driver core should do this cleanup (maybe it already does?).

I just check the driver core code, you are right, there is no need do
this.

DevAttrError:
 device_pm_remove-> device_wakeup_disable(dev);

dpm_sysfs_remove

quoted

quoted

quoted

quoted

quoted

+
+	cancel_delayed_work_sync(&bbnsm->check_work);
+}
+

..so in v6 I asked you to add a cancel_delayed_work_sync() on
the removal path, BUT I missed, my bad, that indeed above

there

quoted

quoted

quoted

quoted

was already a call to cancel_delayed_work_sync() associated to

a

quoted

quoted

quoted

quoted

devm_add_action_or_reset....so now we have 2....also you

should

quoted

quoted

try

quoted

quoted

not to mix devm_add_action_or_reset and plain .remove

methods..use

quoted

quoted

one or the other.

Thanks for your detailed reviewing on this. I will wait to see if
Sudeep has any comments to patch 1-4. If no comments, I will not
do

a

quoted

new version to this patchset.

If v7 patch 1-4 are good for Sudeep to pick up, I will separate
this patch out as a standalone one for input subsystem maintainer.

If you remove the duplicated cancel_delayed_work_sync() in

remove()

quoted

quoted

and unneded device_init_wakeup(dev, false); then you can merge

the

quoted

quoted

input patch with the rest of them with my:

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks for your Ack. But I think patch 1-4 needs go to arm-scmi tree,
Patch 5 to arm imx tree, patch 6 to rtc tree, patch 7 to input tree.

I put the patches together in a patchset is to let reviewers could get
a full picture how the whole stuff work.

For patch 7, I will send out it as a separate patch with fix and tag
after patch 1-4 is ready in arm-scmi tree.

Right, but to accelerate getting support for your part into the mainline I
am OK with input piece not going through the input tree but together
with the rest of the patches through some other tree, probably through
arm-scmi.

Thanks for your supporting on this patchset. I appreciate! 

 If they are not willing to take it then we will have to wait till

core support lands in mainline and then I can pick up the input piece
and move it through my tree.

There is no rush here, I still need to wait Sudeep's comments on
the scmi parts. So this patch probably needs go through your tree in
the end.

Thanks,
Peng.

Thanks.

--
Dmitry