* Re: [PATCH 01/22] Input: ad714x - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 18:47 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel
In-Reply-To: <20240904044244.1042174-2-dmitry.torokhov@gmail.com>
On 04/09/2024 06:42, Dmitry Torokhov wrote:
> Using guard notation makes the code more compact and error handling
> more robust by ensuring that mutexes are released in all code paths
> when control leaves critical section.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/misc/ad714x.c | 12 +++---------
> 1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/input/misc/ad714x.c b/drivers/input/misc/ad714x.c
> index 1acd8429c56c..d106f37df6bc 100644
> --- a/drivers/input/misc/ad714x.c
> +++ b/drivers/input/misc/ad714x.c
> @@ -941,7 +941,7 @@ static irqreturn_t ad714x_interrupt_thread(int irq, void *data)
> struct ad714x_chip *ad714x = data;
> int i;
>
> - mutex_lock(&ad714x->mutex);
> + guard(mutex)(&ad714x->mutex);
>
> ad714x->read(ad714x, STG_LOW_INT_STA_REG, &ad714x->l_state, 3);
>
> @@ -954,8 +954,6 @@ static irqreturn_t ad714x_interrupt_thread(int irq, void *data)
> for (i = 0; i < ad714x->hw->touchpad_num; i++)
> ad714x_touchpad_state_machine(ad714x, i);
>
> - mutex_unlock(&ad714x->mutex);
> -
> return IRQ_HANDLED;
> }
>
> @@ -1169,13 +1167,11 @@ static int ad714x_suspend(struct device *dev)
>
> dev_dbg(ad714x->dev, "%s enter\n", __func__);
>
> - mutex_lock(&ad714x->mutex);
> + guard(mutex)(&ad714x->mutex);
>
> data = ad714x->hw->sys_cfg_reg[AD714X_PWR_CTRL] | 0x3;
> ad714x->write(ad714x, AD714X_PWR_CTRL, data);
>
> - mutex_unlock(&ad714x->mutex);
> -
> return 0;
> }
>
> @@ -1184,7 +1180,7 @@ static int ad714x_resume(struct device *dev)
> struct ad714x_chip *ad714x = dev_get_drvdata(dev);
> dev_dbg(ad714x->dev, "%s enter\n", __func__);
>
> - mutex_lock(&ad714x->mutex);
> + guard(mutex)(&ad714x->mutex);
>
> /* resume to non-shutdown mode */
>
> @@ -1197,8 +1193,6 @@ static int ad714x_resume(struct device *dev)
>
> ad714x->read(ad714x, STG_LOW_INT_STA_REG, &ad714x->l_state, 3);
>
> - mutex_unlock(&ad714x->mutex);
> -
> return 0;
> }
>
Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
^ permalink raw reply
* Re: [PATCH 15/22] Input: iqs7222 - use cleanup facility for fwnodes
From: Javier Carrasco @ 2024-09-04 18:46 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Michael Hennerich, Ville Syrjala, Support Opensource,
Eddie James, Andrey Moiseev, Hans de Goede, Jeff LaBundy,
linux-kernel
In-Reply-To: <ZtimWmQ2B_WlcvTw@google.com>
On 04/09/2024 20:26, Dmitry Torokhov wrote:
> On Wed, Sep 04, 2024 at 12:50:44PM +0200, Javier Carrasco wrote:
>> Hi Dmitry,
>>
>> On 04/09/2024 06:48, Dmitry Torokhov wrote:
>>> Use __free(fwnode_handle) cleanup facility to ensure that references to
>>> acquired fwnodes are dropped at appropriate times automatically.
>>>
>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>> ---
>>> drivers/input/misc/iqs7222.c | 30 ++++++++++++++----------------
>>> 1 file changed, 14 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
>>> index 9ca5a743f19f..d9b87606ff7a 100644
>>> --- a/drivers/input/misc/iqs7222.c
>>> +++ b/drivers/input/misc/iqs7222.c
>>
>> ...
>>
>>> @@ -2818,9 +2813,9 @@ static int iqs7222_parse_reg_grp(struct iqs7222_private *iqs7222,
>>> int reg_grp_index)
>>> {
>>> struct i2c_client *client = iqs7222->client;
>>> - struct fwnode_handle *reg_grp_node;
>>> int error;
>>>
>>
>> Nit: reg_grp_node could stay at the top (where it used to be), as you
>> are assigning it to NULL because there are no sensible assignments at
>> this point.
>>
>>> + struct fwnode_handle *reg_grp_node __free(fwnode_handle) = NULL;
>>> if (iqs7222_reg_grp_names[reg_grp]) {
>>> char reg_grp_name[16];
>
> I think this follows Linus' guidance (in spirit) to combine declaration
> and initialization for objects using __cleanup(). If it was Rust I'd
> written it as
>
> let reg_grp_node = if let Some(...) { ... } else { ... };
>
> so declaration and initialization would be the same, but with C this is
> the closest I could come up with.
>
> Thanks.
>
Combining the declaration and initialization was right, no doubt about
that. I was just nitpicking that the variable declaration could have
been done at the top, as it used to be. The same as you did, but not
separating the declaration from the rest as there are no instructions in
between.
My second thought was that you might be attempting to declare the
variable as near as possible to the "some" initialization, so you moved
it a little bit to get it closer :)
Either way, if you did that on purpose, then
Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
^ permalink raw reply
* Re: [PATCH 12/22] Input: iqs269a - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 18:41 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Michael Hennerich, Ville Syrjala, Support Opensource,
Eddie James, Andrey Moiseev, Hans de Goede, Jeff LaBundy,
linux-kernel
In-Reply-To: <ZtilRLKICDSXKyEp@google.com>
On 04/09/2024 20:21, Dmitry Torokhov wrote:
> Hi Javier,
>
> On Wed, Sep 04, 2024 at 03:53:40PM +0200, Javier Carrasco wrote:
>> On 04/09/2024 06:47, Dmitry Torokhov wrote:
>>> Using guard notation makes the code more compact and error handling
>>> more robust by ensuring that mutexes are released in all code paths
>>> when control leaves critical section.
>>>
>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>> ---
>>> drivers/input/misc/iqs269a.c | 46 +++++++++++++-----------------------
>>> 1 file changed, 16 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/drivers/input/misc/iqs269a.c b/drivers/input/misc/iqs269a.c
>>> index 843f8a3f3410..c34d847fa4af 100644
>>> --- a/drivers/input/misc/iqs269a.c
>>> +++ b/drivers/input/misc/iqs269a.c
>>
>> ...
>>
>>> @@ -453,9 +449,9 @@ static int iqs269_ati_base_get(struct iqs269_private *iqs269,
>>> if (ch_num >= IQS269_NUM_CH)
>>> return -EINVAL;
>>>
>>> - mutex_lock(&iqs269->lock);
>>> + guard(mutex)(&iqs269->lock);
>>> +
>>> engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
>>
>> maybe scoped_guard() to keep the scope of the mutex as it used to be?
>
> Thank you for looking over patches.
>
> It is just a few computations extra, so I decided not to use
> scoped_guard(). Note that original code was forced to release mutex
> early to avoid having to unlock it in all switch branches.
>
>>
>>> - mutex_unlock(&iqs269->lock);
>>>
>>> switch (engine_b & IQS269_CHx_ENG_B_ATI_BASE_MASK) {
>>> case IQS269_CHx_ENG_B_ATI_BASE_75:
>>> @@ -491,7 +487,7 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
>>> if (target > IQS269_CHx_ENG_B_ATI_TARGET_MAX)
>>> return -EINVAL;
>>>
>>> - mutex_lock(&iqs269->lock);
>>> + guard(mutex)(&iqs269->lock);
>>>
>>> engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
>>>
>>> @@ -501,8 +497,6 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
>>> ch_reg[ch_num].engine_b = cpu_to_be16(engine_b);
>>> iqs269->ati_current = false;
>>>
>>> - mutex_unlock(&iqs269->lock);
>>> -
>>> return 0;
>>> }
>>>
>>> @@ -515,10 +509,9 @@ static int iqs269_ati_target_get(struct iqs269_private *iqs269,
>>> if (ch_num >= IQS269_NUM_CH)
>>> return -EINVAL;
>>>
>>> - mutex_lock(&iqs269->lock);
>>> - engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
>>> - mutex_unlock(&iqs269->lock);
>>> + guard(mutex)(&iqs269->lock);
>>
>> same here?
>>
>>>
>>> + engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
>>> *target = (engine_b & IQS269_CHx_ENG_B_ATI_TARGET_MASK) * 32;
>
> Same here, calculating the line above will take no time at all...
>
> Thanks.
>
As you pointed out, in reality the extra locked instructions will not
make any difference. But as the conversion added instructions to be
locked by the mutex without mentioning it, I thought it should be either
left as it used to be with scoped_guard(), or explicitly mentioned in
the description.
No strong feelings against it, but out of curiosity, why would you
rather use guard()? I think scoped_guard() is a better way to
self-document what has to be accessed via mutex, and what not.
Best regards,
Javier Carrasco
^ permalink raw reply
* Re: Question for submitting a dt-binding besides an existing one covering enough for our device
From: Dmitry Torokhov @ 2024-09-04 18:40 UTC (permalink / raw)
To: Tobita, Tatsunosuke
Cc: devicetree@vger.kernel.org, Cheng, Ping, Chen, Martin,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Linux Input
In-Reply-To: <PA4PR07MB7407FF98C5EA3F2E1CFA5D8E879C2@PA4PR07MB7407.eurprd07.prod.outlook.com>
Hi Tatsunosuke,
On Wed, Sep 04, 2024 at 06:26:40AM +0000, Tobita, Tatsunosuke wrote:
> Hi,
>
> Wacom is currently working on to add our dt-binding at
> /Documentation/devicetree/bindings/input/touchscreen. While doing
> that, we found another dt-binding which was already sufficient to
> detect our device and load the driver for it[1].
You pointed to a whole directory of bindings, based on the context
below I assume you are talking about this:
Documentation/devicetree/bindings/input/hid-over-i2c.yaml
>
> In this case, what can we do? hid-over-i2c dt-binding([1]) covers
> information enough for our device to be used under I2C interface.
> Therefore, the difference is seemingly going to be at "description".
> Do we just provide a yaml file containing only the some of the
> headers, such as id, schema, tiltle, and description items to submit
> our dt-binding?
If your controller is fully covered by hid-over-i2c.yaml binding then
I think you just need to add a new compatible to it. We already have
"wacom,w9013" there.
Otherwise follow examples such as:
Documentation/devicetree/bindings/input/elan,ekth6915.yaml
Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
that are also hid-over-i2c devices but require special power up/down
handling.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 15/22] Input: iqs7222 - use cleanup facility for fwnodes
From: Dmitry Torokhov @ 2024-09-04 18:26 UTC (permalink / raw)
To: Javier Carrasco
Cc: linux-input, Michael Hennerich, Ville Syrjala, Support Opensource,
Eddie James, Andrey Moiseev, Hans de Goede, Jeff LaBundy,
linux-kernel
In-Reply-To: <f0956e79-8261-4bd5-96ca-3795bbe1faaf@gmail.com>
On Wed, Sep 04, 2024 at 12:50:44PM +0200, Javier Carrasco wrote:
> Hi Dmitry,
>
> On 04/09/2024 06:48, Dmitry Torokhov wrote:
> > Use __free(fwnode_handle) cleanup facility to ensure that references to
> > acquired fwnodes are dropped at appropriate times automatically.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/input/misc/iqs7222.c | 30 ++++++++++++++----------------
> > 1 file changed, 14 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
> > index 9ca5a743f19f..d9b87606ff7a 100644
> > --- a/drivers/input/misc/iqs7222.c
> > +++ b/drivers/input/misc/iqs7222.c
>
> ...
>
> > @@ -2818,9 +2813,9 @@ static int iqs7222_parse_reg_grp(struct iqs7222_private *iqs7222,
> > int reg_grp_index)
> > {
> > struct i2c_client *client = iqs7222->client;
> > - struct fwnode_handle *reg_grp_node;
> > int error;
> >
>
> Nit: reg_grp_node could stay at the top (where it used to be), as you
> are assigning it to NULL because there are no sensible assignments at
> this point.
>
> > + struct fwnode_handle *reg_grp_node __free(fwnode_handle) = NULL;
> > if (iqs7222_reg_grp_names[reg_grp]) {
> > char reg_grp_name[16];
I think this follows Linus' guidance (in spirit) to combine declaration
and initialization for objects using __cleanup(). If it was Rust I'd
written it as
let reg_grp_node = if let Some(...) { ... } else { ... };
so declaration and initialization would be the same, but with C this is
the closest I could come up with.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 12/22] Input: iqs269a - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 18:21 UTC (permalink / raw)
To: Javier Carrasco
Cc: linux-input, Michael Hennerich, Ville Syrjala, Support Opensource,
Eddie James, Andrey Moiseev, Hans de Goede, Jeff LaBundy,
linux-kernel
In-Reply-To: <9cc5b106-88dc-4539-b831-3cc6cb3ef860@gmail.com>
Hi Javier,
On Wed, Sep 04, 2024 at 03:53:40PM +0200, Javier Carrasco wrote:
> On 04/09/2024 06:47, Dmitry Torokhov wrote:
> > Using guard notation makes the code more compact and error handling
> > more robust by ensuring that mutexes are released in all code paths
> > when control leaves critical section.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > drivers/input/misc/iqs269a.c | 46 +++++++++++++-----------------------
> > 1 file changed, 16 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/input/misc/iqs269a.c b/drivers/input/misc/iqs269a.c
> > index 843f8a3f3410..c34d847fa4af 100644
> > --- a/drivers/input/misc/iqs269a.c
> > +++ b/drivers/input/misc/iqs269a.c
>
> ...
>
> > @@ -453,9 +449,9 @@ static int iqs269_ati_base_get(struct iqs269_private *iqs269,
> > if (ch_num >= IQS269_NUM_CH)
> > return -EINVAL;
> >
> > - mutex_lock(&iqs269->lock);
> > + guard(mutex)(&iqs269->lock);
> > +
> > engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
>
> maybe scoped_guard() to keep the scope of the mutex as it used to be?
Thank you for looking over patches.
It is just a few computations extra, so I decided not to use
scoped_guard(). Note that original code was forced to release mutex
early to avoid having to unlock it in all switch branches.
>
> > - mutex_unlock(&iqs269->lock);
> >
> > switch (engine_b & IQS269_CHx_ENG_B_ATI_BASE_MASK) {
> > case IQS269_CHx_ENG_B_ATI_BASE_75:
> > @@ -491,7 +487,7 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
> > if (target > IQS269_CHx_ENG_B_ATI_TARGET_MAX)
> > return -EINVAL;
> >
> > - mutex_lock(&iqs269->lock);
> > + guard(mutex)(&iqs269->lock);
> >
> > engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
> >
> > @@ -501,8 +497,6 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
> > ch_reg[ch_num].engine_b = cpu_to_be16(engine_b);
> > iqs269->ati_current = false;
> >
> > - mutex_unlock(&iqs269->lock);
> > -
> > return 0;
> > }
> >
> > @@ -515,10 +509,9 @@ static int iqs269_ati_target_get(struct iqs269_private *iqs269,
> > if (ch_num >= IQS269_NUM_CH)
> > return -EINVAL;
> >
> > - mutex_lock(&iqs269->lock);
> > - engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
> > - mutex_unlock(&iqs269->lock);
> > + guard(mutex)(&iqs269->lock);
>
> same here?
>
> >
> > + engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
> > *target = (engine_b & IQS269_CHx_ENG_B_ATI_TARGET_MASK) * 32;
Same here, calculating the line above will take no time at all...
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH -next 13/19] hwmon: Use devm_hid_hw_start_and_open in rog_ryujin_probe()
From: Guenter Roeck @ 2024-09-04 14:20 UTC (permalink / raw)
To: Li Zetao, jikos, bentiss, michael.zaidman, gupt21, djogorchock,
rrameshbabu, bonbons, roderick.colenbrander, david, savicaleksa83,
me, jdelvare, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-14-lizetao1@huawei.com>
On 9/4/24 05:36, Li Zetao wrote:
> Currently, the rog_ryujin module needs to maintain hid resources
> by itself. Consider using devm_hid_hw_start_and_open helper to ensure
> that hid resources are consistent with the device life cycle, and release
> hid resources before device is released. At the same time, it can avoid
> the goto-release encoding, drop the fail_and_close and fail_and_stop
> lables, and directly return the error code when an error occurs.
>
> Signed-off-by: Li Zetao <lizetao1@huawei.com>
> ---
> drivers/hwmon/asus_rog_ryujin.c | 29 +++++------------------------
> 1 file changed, 5 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/hwmon/asus_rog_ryujin.c b/drivers/hwmon/asus_rog_ryujin.c
> index f8b20346a995..da03ba3b4e0f 100644
> --- a/drivers/hwmon/asus_rog_ryujin.c
> +++ b/drivers/hwmon/asus_rog_ryujin.c
> @@ -520,23 +520,13 @@ static int rog_ryujin_probe(struct hid_device *hdev, const struct hid_device_id
> }
>
> /* Enable hidraw so existing user-space tools can continue to work */
> - ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> - if (ret) {
> - hid_err(hdev, "hid hw start failed with %d\n", ret);
> + ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
> + if (ret)
> return ret;
> - }
> -
> - ret = hid_hw_open(hdev);
> - if (ret) {
> - hid_err(hdev, "hid hw open failed with %d\n", ret);
> - goto fail_and_stop;
> - }
>
> priv->buffer = devm_kzalloc(&hdev->dev, MAX_REPORT_LENGTH, GFP_KERNEL);
> - if (!priv->buffer) {
> - ret = -ENOMEM;
> - goto fail_and_close;
> - }
> + if (!priv->buffer)
> + return -ENOMEM;
>
> mutex_init(&priv->status_report_request_mutex);
> mutex_init(&priv->buffer_lock);
> @@ -553,16 +543,10 @@ static int rog_ryujin_probe(struct hid_device *hdev, const struct hid_device_id
> if (IS_ERR(priv->hwmon_dev)) {
> ret = PTR_ERR(priv->hwmon_dev);
> hid_err(hdev, "hwmon registration failed with %d\n", ret);
> - goto fail_and_close;
> + return ret;
> }
struct device *hwmon_dev;
hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "rog_ryujin",
priv, &rog_ryujin_chip_info, NULL);
return PTR_ERR_OR_ZERO(hwmon_dev);
and drop the remove function.
Thanks,
Guenter
>
> return 0;
> -
> -fail_and_close:
> - hid_hw_close(hdev);
> -fail_and_stop:
> - hid_hw_stop(hdev);
> - return ret;
> }
>
> static void rog_ryujin_remove(struct hid_device *hdev)
> @@ -570,9 +554,6 @@ static void rog_ryujin_remove(struct hid_device *hdev)
> struct rog_ryujin_data *priv = hid_get_drvdata(hdev);
>
> hwmon_device_unregister(priv->hwmon_dev);
> -
> - hid_hw_close(hdev);
> - hid_hw_stop(hdev);
> }
>
> static const struct hid_device_id rog_ryujin_table[] = {
^ permalink raw reply
* Re: [PATCH -next 17/19] hwmon: (nzxt-kraken2) Use devm_hid_hw_start_and_open in kraken2_probe()
From: Guenter Roeck @ 2024-09-04 14:17 UTC (permalink / raw)
To: Li Zetao, jikos, bentiss, michael.zaidman, gupt21, djogorchock,
rrameshbabu, bonbons, roderick.colenbrander, david, savicaleksa83,
me, jdelvare, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-18-lizetao1@huawei.com>
On 9/4/24 05:36, Li Zetao wrote:
> Currently, the nzxt-kraken2 module needs to maintain hid resources
> by itself. Consider using devm_hid_hw_start_and_open helper to ensure
> that hid resources are consistent with the device life cycle, and release
> hid resources before device is released. At the same time, it can avoid
> the goto-release encoding, drop the fail_and_close and fail_and_stop
> lables, and directly return the error code when an error occurs.
>
> Signed-off-by: Li Zetao <lizetao1@huawei.com>
> ---
> drivers/hwmon/nzxt-kraken2.c | 23 +++--------------------
> 1 file changed, 3 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/hwmon/nzxt-kraken2.c b/drivers/hwmon/nzxt-kraken2.c
> index 7caf387eb144..aaaf857b42ed 100644
> --- a/drivers/hwmon/nzxt-kraken2.c
> +++ b/drivers/hwmon/nzxt-kraken2.c
> @@ -158,17 +158,9 @@ static int kraken2_probe(struct hid_device *hdev,
> /*
> * Enable hidraw so existing user-space tools can continue to work.
> */
> - ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> - if (ret) {
> - hid_err(hdev, "hid hw start failed with %d\n", ret);
> + ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
> + if (ret)
> return ret;
> - }
> -
> - ret = hid_hw_open(hdev);
> - if (ret) {
> - hid_err(hdev, "hid hw open failed with %d\n", ret);
> - goto fail_and_stop;
> - }
>
> priv->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "kraken2",
> priv, &kraken2_chip_info,
> @@ -176,16 +168,10 @@ static int kraken2_probe(struct hid_device *hdev,
> if (IS_ERR(priv->hwmon_dev)) {
> ret = PTR_ERR(priv->hwmon_dev);
> hid_err(hdev, "hwmon registration failed with %d\n", ret);
> - goto fail_and_close;
> + return ret;
> }
>
> return 0;
struct device *hwmon_dev; // also drop from struct kraken2_priv_data
...
hwmon_dev = devm_hwmon_device_register_with_info(...);
return PTR_ERR_OR_ZERO(hwmon_dev);
and drop the remove function.
Thanks,
Guenter
> -
> -fail_and_close:
> - hid_hw_close(hdev);
> -fail_and_stop:
> - hid_hw_stop(hdev);
> - return ret;
> }
>
> static void kraken2_remove(struct hid_device *hdev)
> @@ -193,9 +179,6 @@ static void kraken2_remove(struct hid_device *hdev)
> struct kraken2_priv_data *priv = hid_get_drvdata(hdev);
>
> hwmon_device_unregister(priv->hwmon_dev);
> -
> - hid_hw_close(hdev);
> - hid_hw_stop(hdev);
> }
>
> static const struct hid_device_id kraken2_table[] = {
^ permalink raw reply
* Re: [PATCH -next 19/19] hwmon: (nzxt-smart2) Use devm_hid_hw_start_and_open in nzxt_smart2_hid_probe()
From: Guenter Roeck @ 2024-09-04 14:14 UTC (permalink / raw)
To: Li Zetao, jikos, bentiss, michael.zaidman, gupt21, djogorchock,
rrameshbabu, bonbons, roderick.colenbrander, david, savicaleksa83,
me, jdelvare, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-20-lizetao1@huawei.com>
On 9/4/24 05:36, Li Zetao wrote:
> Currently, the nzxt-smart2 module needs to maintain hid resources
> by itself. Consider using devm_hid_hw_start_and_open helper to ensure
For all patches:
s/Consider using/Use/
> that hid resources are consistent with the device life cycle, and release
> hid resources before device is released. At the same time, it can avoid
> the goto-release encoding, drop the out_hw_close and out_hw_stop
> lables, and directly return the error code when an error occurs.
>
> Signed-off-by: Li Zetao <lizetao1@huawei.com>
> ---
> drivers/hwmon/nzxt-smart2.c | 22 +++-------------------
> 1 file changed, 3 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/hwmon/nzxt-smart2.c b/drivers/hwmon/nzxt-smart2.c
> index df6fa72a6b59..b5721a42c0d3 100644
> --- a/drivers/hwmon/nzxt-smart2.c
> +++ b/drivers/hwmon/nzxt-smart2.c
> @@ -750,14 +750,10 @@ static int nzxt_smart2_hid_probe(struct hid_device *hdev,
> if (ret)
> return ret;
>
> - ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> + ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
> if (ret)
> return ret;
>
> - ret = hid_hw_open(hdev);
> - if (ret)
> - goto out_hw_stop;
> -
> hid_device_io_start(hdev);
>
> init_device(drvdata, UPDATE_INTERVAL_DEFAULT_MS);
> @@ -765,19 +761,10 @@ static int nzxt_smart2_hid_probe(struct hid_device *hdev,
> drvdata->hwmon =
> hwmon_device_register_with_info(&hdev->dev, "nzxtsmart2", drvdata,
> &nzxt_smart2_chip_info, NULL);
> - if (IS_ERR(drvdata->hwmon)) {
> - ret = PTR_ERR(drvdata->hwmon);
> - goto out_hw_close;
> - }
> + if (IS_ERR(drvdata->hwmon))
> + return PTR_ERR(drvdata->hwmon);
>
> return 0;
return PTR_ERR_OR_ZERO(drvdata->hwmon);
Also, this can be optimized further.
struct device *hwmon; // and drop from struct drvdata
...
hwmon = devm_hwmon_device_register_with_info(&hdev->dev, "nzxtsmart2", drvdata,
&nzxt_smart2_chip_info, NULL);
return PTR_ERR_OR_ZERO(hwmon);
and drop the remove function entirely.
Thanks,
Guenter
> -
> -out_hw_close:
> - hid_hw_close(hdev);
> -
> -out_hw_stop:
> - hid_hw_stop(hdev);
> - return ret;
> }
>
> static void nzxt_smart2_hid_remove(struct hid_device *hdev)
> @@ -785,9 +772,6 @@ static void nzxt_smart2_hid_remove(struct hid_device *hdev)
> struct drvdata *drvdata = hid_get_drvdata(hdev);
>
> hwmon_device_unregister(drvdata->hwmon);
> -
> - hid_hw_close(hdev);
> - hid_hw_stop(hdev);
> }
>
> static const struct hid_device_id nzxt_smart2_hid_id_table[] = {
^ permalink raw reply
* Re: [PATCH 1/1] dt-bindings: input: convert rotary-encoder to yaml
From: Frank Li @ 2024-09-04 14:10 UTC (permalink / raw)
To: Rob Herring (Arm), Dmitry Torokhov
Cc: linux-kernel, Krzysztof Kozlowski, Conor Dooley, linux-input, imx,
Dmitry Torokhov, devicetree
In-Reply-To: <172358311160.2028662.8871129494345919782.robh@kernel.org>
On Tue, Aug 13, 2024 at 03:05:12PM -0600, Rob Herring (Arm) wrote:
>
> On Sun, 11 Aug 2024 17:46:55 -0400, Frank Li wrote:
> > Convert binding doc rotary-encoder.txt to yaml format.
> >
> > Additional change:
> > - Only keep one example.
> >
> > Fix below warning:
> > arch/arm64/boot/dts/freescale/imx8mn-dimonoff-gateway-evk.dtb: /rotary-encoder:
> > failed to match any schema with compatible: ['rotary-encoder']
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > .../bindings/input/rotary-encoder.txt | 50 -----------
> > .../bindings/input/rotary-encoder.yaml | 90 +++++++++++++++++++
> > 2 files changed, 90 insertions(+), 50 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/input/rotary-encoder.txt
> > create mode 100644 Documentation/devicetree/bindings/input/rotary-encoder.yaml
> >
>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Dmitry:
Could you take care this patch?
Frank
>
^ permalink raw reply
* Re: [PATCH 12/22] Input: iqs269a - use guard notation when acquiring mutex
From: Javier Carrasco @ 2024-09-04 13:53 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel
In-Reply-To: <20240904044756.1047629-1-dmitry.torokhov@gmail.com>
On 04/09/2024 06:47, Dmitry Torokhov wrote:
> Using guard notation makes the code more compact and error handling
> more robust by ensuring that mutexes are released in all code paths
> when control leaves critical section.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/misc/iqs269a.c | 46 +++++++++++++-----------------------
> 1 file changed, 16 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/input/misc/iqs269a.c b/drivers/input/misc/iqs269a.c
> index 843f8a3f3410..c34d847fa4af 100644
> --- a/drivers/input/misc/iqs269a.c
> +++ b/drivers/input/misc/iqs269a.c
...
> @@ -453,9 +449,9 @@ static int iqs269_ati_base_get(struct iqs269_private *iqs269,
> if (ch_num >= IQS269_NUM_CH)
> return -EINVAL;
>
> - mutex_lock(&iqs269->lock);
> + guard(mutex)(&iqs269->lock);
> +
> engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
maybe scoped_guard() to keep the scope of the mutex as it used to be?
> - mutex_unlock(&iqs269->lock);
>
> switch (engine_b & IQS269_CHx_ENG_B_ATI_BASE_MASK) {
> case IQS269_CHx_ENG_B_ATI_BASE_75:
> @@ -491,7 +487,7 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
> if (target > IQS269_CHx_ENG_B_ATI_TARGET_MAX)
> return -EINVAL;
>
> - mutex_lock(&iqs269->lock);
> + guard(mutex)(&iqs269->lock);
>
> engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
>
> @@ -501,8 +497,6 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
> ch_reg[ch_num].engine_b = cpu_to_be16(engine_b);
> iqs269->ati_current = false;
>
> - mutex_unlock(&iqs269->lock);
> -
> return 0;
> }
>
> @@ -515,10 +509,9 @@ static int iqs269_ati_target_get(struct iqs269_private *iqs269,
> if (ch_num >= IQS269_NUM_CH)
> return -EINVAL;
>
> - mutex_lock(&iqs269->lock);
> - engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
> - mutex_unlock(&iqs269->lock);
> + guard(mutex)(&iqs269->lock);
same here?
>
> + engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
> *target = (engine_b & IQS269_CHx_ENG_B_ATI_TARGET_MASK) * 32;
>
> return 0;
Best regards,
Javier Carrasco
^ permalink raw reply
* [PATCH -next 19/19] hwmon: (nzxt-smart2) Use devm_hid_hw_start_and_open in nzxt_smart2_hid_probe()
From: Li Zetao @ 2024-09-04 12:36 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the nzxt-smart2 module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the out_hw_close and out_hw_stop
lables, and directly return the error code when an error occurs.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hwmon/nzxt-smart2.c | 22 +++-------------------
1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/drivers/hwmon/nzxt-smart2.c b/drivers/hwmon/nzxt-smart2.c
index df6fa72a6b59..b5721a42c0d3 100644
--- a/drivers/hwmon/nzxt-smart2.c
+++ b/drivers/hwmon/nzxt-smart2.c
@@ -750,14 +750,10 @@ static int nzxt_smart2_hid_probe(struct hid_device *hdev,
if (ret)
return ret;
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
if (ret)
return ret;
- ret = hid_hw_open(hdev);
- if (ret)
- goto out_hw_stop;
-
hid_device_io_start(hdev);
init_device(drvdata, UPDATE_INTERVAL_DEFAULT_MS);
@@ -765,19 +761,10 @@ static int nzxt_smart2_hid_probe(struct hid_device *hdev,
drvdata->hwmon =
hwmon_device_register_with_info(&hdev->dev, "nzxtsmart2", drvdata,
&nzxt_smart2_chip_info, NULL);
- if (IS_ERR(drvdata->hwmon)) {
- ret = PTR_ERR(drvdata->hwmon);
- goto out_hw_close;
- }
+ if (IS_ERR(drvdata->hwmon))
+ return PTR_ERR(drvdata->hwmon);
return 0;
-
-out_hw_close:
- hid_hw_close(hdev);
-
-out_hw_stop:
- hid_hw_stop(hdev);
- return ret;
}
static void nzxt_smart2_hid_remove(struct hid_device *hdev)
@@ -785,9 +772,6 @@ static void nzxt_smart2_hid_remove(struct hid_device *hdev)
struct drvdata *drvdata = hid_get_drvdata(hdev);
hwmon_device_unregister(drvdata->hwmon);
-
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
static const struct hid_device_id nzxt_smart2_hid_id_table[] = {
--
2.34.1
^ permalink raw reply related
* [PATCH -next 18/19] hwmon: (nzxt-kraken3) Use devm_hid_hw_start_and_open in kraken3_probe()
From: Li Zetao @ 2024-09-04 12:36 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the nzxt-kraken2 module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the fail_and_close and fail_and_stop
lables, and directly return the error code when an error occurs.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hwmon/nzxt-kraken3.c | 34 +++++++---------------------------
1 file changed, 7 insertions(+), 27 deletions(-)
diff --git a/drivers/hwmon/nzxt-kraken3.c b/drivers/hwmon/nzxt-kraken3.c
index 00f3ac90a290..71b8c21cfe1b 100644
--- a/drivers/hwmon/nzxt-kraken3.c
+++ b/drivers/hwmon/nzxt-kraken3.c
@@ -897,17 +897,9 @@ static int kraken3_probe(struct hid_device *hdev, const struct hid_device_id *id
}
/* Enable hidraw so existing user-space tools can continue to work */
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
- if (ret) {
- hid_err(hdev, "hid hw start failed with %d\n", ret);
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
+ if (ret)
return ret;
- }
-
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "hid hw open failed with %d\n", ret);
- goto fail_and_stop;
- }
switch (hdev->product) {
case USB_PRODUCT_ID_X53:
@@ -928,15 +920,12 @@ static int kraken3_probe(struct hid_device *hdev, const struct hid_device_id *id
device_name = "kraken2023elite";
break;
default:
- ret = -ENODEV;
- goto fail_and_close;
+ return -ENODEV;
}
priv->buffer = devm_kzalloc(&hdev->dev, MAX_REPORT_LENGTH, GFP_KERNEL);
- if (!priv->buffer) {
- ret = -ENOMEM;
- goto fail_and_close;
- }
+ if (!priv->buffer)
+ return -ENOMEM;
mutex_init(&priv->buffer_lock);
mutex_init(&priv->z53_status_request_lock);
@@ -948,7 +937,7 @@ static int kraken3_probe(struct hid_device *hdev, const struct hid_device_id *id
ret = kraken3_init_device(hdev);
if (ret < 0) {
hid_err(hdev, "device init failed with %d\n", ret);
- goto fail_and_close;
+ return ret;
}
ret = kraken3_get_fw_ver(hdev);
@@ -960,18 +949,12 @@ static int kraken3_probe(struct hid_device *hdev, const struct hid_device_id *id
if (IS_ERR(priv->hwmon_dev)) {
ret = PTR_ERR(priv->hwmon_dev);
hid_err(hdev, "hwmon registration failed with %d\n", ret);
- goto fail_and_close;
+ return ret;
}
kraken3_debugfs_init(priv, device_name);
return 0;
-
-fail_and_close:
- hid_hw_close(hdev);
-fail_and_stop:
- hid_hw_stop(hdev);
- return ret;
}
static void kraken3_remove(struct hid_device *hdev)
@@ -980,9 +963,6 @@ static void kraken3_remove(struct hid_device *hdev)
debugfs_remove_recursive(priv->debugfs);
hwmon_device_unregister(priv->hwmon_dev);
-
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
static const struct hid_device_id kraken3_table[] = {
--
2.34.1
^ permalink raw reply related
* [PATCH -next 17/19] hwmon: (nzxt-kraken2) Use devm_hid_hw_start_and_open in kraken2_probe()
From: Li Zetao @ 2024-09-04 12:36 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the nzxt-kraken2 module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the fail_and_close and fail_and_stop
lables, and directly return the error code when an error occurs.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hwmon/nzxt-kraken2.c | 23 +++--------------------
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/drivers/hwmon/nzxt-kraken2.c b/drivers/hwmon/nzxt-kraken2.c
index 7caf387eb144..aaaf857b42ed 100644
--- a/drivers/hwmon/nzxt-kraken2.c
+++ b/drivers/hwmon/nzxt-kraken2.c
@@ -158,17 +158,9 @@ static int kraken2_probe(struct hid_device *hdev,
/*
* Enable hidraw so existing user-space tools can continue to work.
*/
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
- if (ret) {
- hid_err(hdev, "hid hw start failed with %d\n", ret);
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
+ if (ret)
return ret;
- }
-
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "hid hw open failed with %d\n", ret);
- goto fail_and_stop;
- }
priv->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "kraken2",
priv, &kraken2_chip_info,
@@ -176,16 +168,10 @@ static int kraken2_probe(struct hid_device *hdev,
if (IS_ERR(priv->hwmon_dev)) {
ret = PTR_ERR(priv->hwmon_dev);
hid_err(hdev, "hwmon registration failed with %d\n", ret);
- goto fail_and_close;
+ return ret;
}
return 0;
-
-fail_and_close:
- hid_hw_close(hdev);
-fail_and_stop:
- hid_hw_stop(hdev);
- return ret;
}
static void kraken2_remove(struct hid_device *hdev)
@@ -193,9 +179,6 @@ static void kraken2_remove(struct hid_device *hdev)
struct kraken2_priv_data *priv = hid_get_drvdata(hdev);
hwmon_device_unregister(priv->hwmon_dev);
-
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
static const struct hid_device_id kraken2_table[] = {
--
2.34.1
^ permalink raw reply related
* [PATCH -next 15/19] hwmon: (corsair-psu) Use devm_hid_hw_start_and_open in corsairpsu_probe()
From: Li Zetao @ 2024-09-04 12:36 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the corsair-psu module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the fail_and_close and fail_and_stop
lables, and directly return the error code when an error occurs.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hwmon/corsair-psu.c | 24 +++++-------------------
1 file changed, 5 insertions(+), 19 deletions(-)
diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c
index f8f22b8a67cd..b574ec9cd00f 100644
--- a/drivers/hwmon/corsair-psu.c
+++ b/drivers/hwmon/corsair-psu.c
@@ -787,14 +787,10 @@ static int corsairpsu_probe(struct hid_device *hdev, const struct hid_device_id
if (ret)
return ret;
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
if (ret)
return ret;
- ret = hid_hw_open(hdev);
- if (ret)
- goto fail_and_stop;
-
priv->hdev = hdev;
hid_set_drvdata(hdev, priv);
mutex_init(&priv->lock);
@@ -805,13 +801,13 @@ static int corsairpsu_probe(struct hid_device *hdev, const struct hid_device_id
ret = corsairpsu_init(priv);
if (ret < 0) {
dev_err(&hdev->dev, "unable to initialize device (%d)\n", ret);
- goto fail_and_stop;
+ return ret;
}
ret = corsairpsu_fwinfo(priv);
if (ret < 0) {
dev_err(&hdev->dev, "unable to query firmware (%d)\n", ret);
- goto fail_and_stop;
+ return ret;
}
corsairpsu_get_criticals(priv);
@@ -820,20 +816,12 @@ static int corsairpsu_probe(struct hid_device *hdev, const struct hid_device_id
priv->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "corsairpsu", priv,
&corsairpsu_chip_info, NULL);
- if (IS_ERR(priv->hwmon_dev)) {
- ret = PTR_ERR(priv->hwmon_dev);
- goto fail_and_close;
- }
+ if (IS_ERR(priv->hwmon_dev))
+ return PTR_ERR(priv->hwmon_dev);
corsairpsu_debugfs_init(priv);
return 0;
-
-fail_and_close:
- hid_hw_close(hdev);
-fail_and_stop:
- hid_hw_stop(hdev);
- return ret;
}
static void corsairpsu_remove(struct hid_device *hdev)
@@ -842,8 +830,6 @@ static void corsairpsu_remove(struct hid_device *hdev)
debugfs_remove_recursive(priv->debugfs);
hwmon_device_unregister(priv->hwmon_dev);
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
static int corsairpsu_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data,
--
2.34.1
^ permalink raw reply related
* [PATCH -next 16/19] hwmon: (gigabyte_waterforce) Use devm_hid_hw_start_and_open in waterforce_probe()
From: Li Zetao @ 2024-09-04 12:36 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the waterforce module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the fail_and_close and fail_and_stop
lables, and directly return the error code when an error occurs.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hwmon/gigabyte_waterforce.c | 29 +++++------------------------
1 file changed, 5 insertions(+), 24 deletions(-)
diff --git a/drivers/hwmon/gigabyte_waterforce.c b/drivers/hwmon/gigabyte_waterforce.c
index 8129d7b3ceaf..9052d1c3d5aa 100644
--- a/drivers/hwmon/gigabyte_waterforce.c
+++ b/drivers/hwmon/gigabyte_waterforce.c
@@ -337,23 +337,13 @@ static int waterforce_probe(struct hid_device *hdev, const struct hid_device_id
/*
* Enable hidraw so existing user-space tools can continue to work.
*/
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
- if (ret) {
- hid_err(hdev, "hid hw start failed with %d\n", ret);
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
+ if (ret)
return ret;
- }
-
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "hid hw open failed with %d\n", ret);
- goto fail_and_stop;
- }
priv->buffer = devm_kzalloc(&hdev->dev, MAX_REPORT_LENGTH, GFP_KERNEL);
- if (!priv->buffer) {
- ret = -ENOMEM;
- goto fail_and_close;
- }
+ if (!priv->buffer)
+ return -ENOMEM;
mutex_init(&priv->status_report_request_mutex);
mutex_init(&priv->buffer_lock);
@@ -371,18 +361,12 @@ static int waterforce_probe(struct hid_device *hdev, const struct hid_device_id
if (IS_ERR(priv->hwmon_dev)) {
ret = PTR_ERR(priv->hwmon_dev);
hid_err(hdev, "hwmon registration failed with %d\n", ret);
- goto fail_and_close;
+ return ret;
}
waterforce_debugfs_init(priv);
return 0;
-
-fail_and_close:
- hid_hw_close(hdev);
-fail_and_stop:
- hid_hw_stop(hdev);
- return ret;
}
static void waterforce_remove(struct hid_device *hdev)
@@ -391,9 +375,6 @@ static void waterforce_remove(struct hid_device *hdev)
debugfs_remove_recursive(priv->debugfs);
hwmon_device_unregister(priv->hwmon_dev);
-
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
static const struct hid_device_id waterforce_table[] = {
--
2.34.1
^ permalink raw reply related
* [PATCH -next 14/19] hwmon: (corsair-cpro) Use devm_hid_hw_start_and_open in ccp_probe()
From: Li Zetao @ 2024-09-04 12:36 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the corsair-cpro module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the out_hw_close and hid_hw_stop
lables, and directly return the error code when an error occurs.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hwmon/corsair-cpro.c | 24 +++++-------------------
1 file changed, 5 insertions(+), 19 deletions(-)
diff --git a/drivers/hwmon/corsair-cpro.c b/drivers/hwmon/corsair-cpro.c
index e1a7f7aa7f80..7bba30840f32 100644
--- a/drivers/hwmon/corsair-cpro.c
+++ b/drivers/hwmon/corsair-cpro.c
@@ -601,14 +601,10 @@ static int ccp_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (ret)
return ret;
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
if (ret)
return ret;
- ret = hid_hw_open(hdev);
- if (ret)
- goto out_hw_stop;
-
ccp->hdev = hdev;
hid_set_drvdata(hdev, ccp);
@@ -621,28 +617,20 @@ static int ccp_probe(struct hid_device *hdev, const struct hid_device_id *id)
/* temp and fan connection status only updates when device is powered on */
ret = get_temp_cnct(ccp);
if (ret)
- goto out_hw_close;
+ return ret;
ret = get_fan_cnct(ccp);
if (ret)
- goto out_hw_close;
+ return ret;
ccp_debugfs_init(ccp);
ccp->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "corsaircpro",
ccp, &ccp_chip_info, NULL);
- if (IS_ERR(ccp->hwmon_dev)) {
- ret = PTR_ERR(ccp->hwmon_dev);
- goto out_hw_close;
- }
+ if (IS_ERR(ccp->hwmon_dev))
+ return PTR_ERR(ccp->hwmon_dev);
return 0;
-
-out_hw_close:
- hid_hw_close(hdev);
-out_hw_stop:
- hid_hw_stop(hdev);
- return ret;
}
static void ccp_remove(struct hid_device *hdev)
@@ -651,8 +639,6 @@ static void ccp_remove(struct hid_device *hdev)
debugfs_remove_recursive(ccp->debugfs);
hwmon_device_unregister(ccp->hwmon_dev);
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
static const struct hid_device_id ccp_devices[] = {
--
2.34.1
^ permalink raw reply related
* [PATCH -next 13/19] hwmon: Use devm_hid_hw_start_and_open in rog_ryujin_probe()
From: Li Zetao @ 2024-09-04 12:36 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the rog_ryujin module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the fail_and_close and fail_and_stop
lables, and directly return the error code when an error occurs.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hwmon/asus_rog_ryujin.c | 29 +++++------------------------
1 file changed, 5 insertions(+), 24 deletions(-)
diff --git a/drivers/hwmon/asus_rog_ryujin.c b/drivers/hwmon/asus_rog_ryujin.c
index f8b20346a995..da03ba3b4e0f 100644
--- a/drivers/hwmon/asus_rog_ryujin.c
+++ b/drivers/hwmon/asus_rog_ryujin.c
@@ -520,23 +520,13 @@ static int rog_ryujin_probe(struct hid_device *hdev, const struct hid_device_id
}
/* Enable hidraw so existing user-space tools can continue to work */
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
- if (ret) {
- hid_err(hdev, "hid hw start failed with %d\n", ret);
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
+ if (ret)
return ret;
- }
-
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "hid hw open failed with %d\n", ret);
- goto fail_and_stop;
- }
priv->buffer = devm_kzalloc(&hdev->dev, MAX_REPORT_LENGTH, GFP_KERNEL);
- if (!priv->buffer) {
- ret = -ENOMEM;
- goto fail_and_close;
- }
+ if (!priv->buffer)
+ return -ENOMEM;
mutex_init(&priv->status_report_request_mutex);
mutex_init(&priv->buffer_lock);
@@ -553,16 +543,10 @@ static int rog_ryujin_probe(struct hid_device *hdev, const struct hid_device_id
if (IS_ERR(priv->hwmon_dev)) {
ret = PTR_ERR(priv->hwmon_dev);
hid_err(hdev, "hwmon registration failed with %d\n", ret);
- goto fail_and_close;
+ return ret;
}
return 0;
-
-fail_and_close:
- hid_hw_close(hdev);
-fail_and_stop:
- hid_hw_stop(hdev);
- return ret;
}
static void rog_ryujin_remove(struct hid_device *hdev)
@@ -570,9 +554,6 @@ static void rog_ryujin_remove(struct hid_device *hdev)
struct rog_ryujin_data *priv = hid_get_drvdata(hdev);
hwmon_device_unregister(priv->hwmon_dev);
-
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
static const struct hid_device_id rog_ryujin_table[] = {
--
2.34.1
^ permalink raw reply related
* [PATCH -next 12/19] hwmon: (aquacomputer_d5next) Use devm_hid_hw_start_and_open in aqc_probe()
From: Li Zetao @ 2024-09-04 12:36 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the aquacomputer_d5next module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the fail_and_close and fail_and_stop
lables, and directly return the error code when an error occurs.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hwmon/aquacomputer_d5next.c | 39 +++++++----------------------
1 file changed, 9 insertions(+), 30 deletions(-)
diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
index 8e55cd2f46f5..9b66ff0fe6e1 100644
--- a/drivers/hwmon/aquacomputer_d5next.c
+++ b/drivers/hwmon/aquacomputer_d5next.c
@@ -1556,14 +1556,10 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
if (ret)
return ret;
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
if (ret)
return ret;
- ret = hid_hw_open(hdev);
- if (ret)
- goto fail_and_stop;
-
switch (hdev->product) {
case USB_PRODUCT_ID_AQUAERO:
/*
@@ -1577,10 +1573,8 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
* they present. The two other devices have the type of the second element in
* their respective collections set to 1, while the real device has it set to 0.
*/
- if (hdev->collection[1].type != 0) {
- ret = -ENODEV;
- goto fail_and_close;
- }
+ if (hdev->collection[1].type != 0)
+ return -ENODEV;
priv->kind = aquaero;
@@ -1740,10 +1734,8 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
* Choose the right Leakshield device, because
* the other one acts as a keyboard
*/
- if (hdev->type != 2) {
- ret = -ENODEV;
- goto fail_and_close;
- }
+ if (hdev->type != 2)
+ return -ENODEV;
priv->kind = leakshield;
@@ -1865,30 +1857,20 @@ static int aqc_probe(struct hid_device *hdev, const struct hid_device_id *id)
priv->name = aqc_device_names[priv->kind];
priv->buffer = devm_kzalloc(&hdev->dev, priv->buffer_size, GFP_KERNEL);
- if (!priv->buffer) {
- ret = -ENOMEM;
- goto fail_and_close;
- }
+ if (!priv->buffer)
+ return -ENOMEM;
mutex_init(&priv->mutex);
priv->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, priv->name, priv,
&aqc_chip_info, NULL);
- if (IS_ERR(priv->hwmon_dev)) {
- ret = PTR_ERR(priv->hwmon_dev);
- goto fail_and_close;
- }
+ if (IS_ERR(priv->hwmon_dev))
+ return PTR_ERR(priv->hwmon_dev);
aqc_debugfs_init(priv);
return 0;
-
-fail_and_close:
- hid_hw_close(hdev);
-fail_and_stop:
- hid_hw_stop(hdev);
- return ret;
}
static void aqc_remove(struct hid_device *hdev)
@@ -1897,9 +1879,6 @@ static void aqc_remove(struct hid_device *hdev)
debugfs_remove_recursive(priv->debugfs);
hwmon_device_unregister(priv->hwmon_dev);
-
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
static const struct hid_device_id aqc_table[] = {
--
2.34.1
^ permalink raw reply related
* [PATCH -next 11/19] HID: wiimote: Use devm_hid_hw_start_and_open in wiimote_hid_probe()
From: Li Zetao @ 2024-09-04 12:35 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the wiimote module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the err_close and err_stop lables.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hid/hid-wiimote-core.c | 20 +++-----------------
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
index 26167cfb696f..28cd9ccbb617 100644
--- a/drivers/hid/hid-wiimote-core.c
+++ b/drivers/hid/hid-wiimote-core.c
@@ -1780,8 +1780,6 @@ static void wiimote_destroy(struct wiimote_data *wdata)
wiimote_ext_unload(wdata);
wiimote_modules_unload(wdata);
cancel_work_sync(&wdata->queue.worker);
- hid_hw_close(wdata->hdev);
- hid_hw_stop(wdata->hdev);
kfree(wdata);
}
@@ -1806,22 +1804,14 @@ static int wiimote_hid_probe(struct hid_device *hdev,
goto err;
}
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
- if (ret) {
- hid_err(hdev, "HW start failed\n");
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
+ if (ret)
goto err;
- }
-
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "cannot start hardware I/O\n");
- goto err_stop;
- }
ret = device_create_file(&hdev->dev, &dev_attr_extension);
if (ret) {
hid_err(hdev, "cannot create sysfs attribute\n");
- goto err_close;
+ goto err;
}
ret = device_create_file(&hdev->dev, &dev_attr_devtype);
@@ -1847,10 +1837,6 @@ static int wiimote_hid_probe(struct hid_device *hdev,
err_ext:
device_remove_file(&wdata->hdev->dev, &dev_attr_extension);
-err_close:
- hid_hw_close(hdev);
-err_stop:
- hid_hw_stop(hdev);
err:
input_free_device(wdata->ir);
input_free_device(wdata->accel);
--
2.34.1
^ permalink raw reply related
* [PATCH -next 09/19] HID: playstation: Use devm_hid_hw_start_and_open in ps_probe()
From: Li Zetao @ 2024-09-04 12:35 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the playstation module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the err_close and err_stop lables, and
directly return the error code when an error occurs.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hid/hid-playstation.c | 27 ++++-----------------------
1 file changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 0d90d7ee693c..6dddb9451a37 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -2704,41 +2704,25 @@ static int ps_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
- if (ret) {
- hid_err(hdev, "Failed to start HID device\n");
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
+ if (ret)
return ret;
- }
-
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "Failed to open HID device\n");
- goto err_stop;
- }
if (id->driver_data == PS_TYPE_PS4_DUALSHOCK4) {
dev = dualshock4_create(hdev);
if (IS_ERR(dev)) {
hid_err(hdev, "Failed to create dualshock4.\n");
- ret = PTR_ERR(dev);
- goto err_close;
+ return PTR_ERR(dev);
}
} else if (id->driver_data == PS_TYPE_PS5_DUALSENSE) {
dev = dualsense_create(hdev);
if (IS_ERR(dev)) {
hid_err(hdev, "Failed to create dualsense.\n");
- ret = PTR_ERR(dev);
- goto err_close;
+ return PTR_ERR(dev);
}
}
return ret;
-
-err_close:
- hid_hw_close(hdev);
-err_stop:
- hid_hw_stop(hdev);
- return ret;
}
static void ps_remove(struct hid_device *hdev)
@@ -2750,9 +2734,6 @@ static void ps_remove(struct hid_device *hdev)
if (dev->remove)
dev->remove(dev);
-
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
static const struct hid_device_id ps_devices[] = {
--
2.34.1
^ permalink raw reply related
* [PATCH -next 10/19] HID: hid-steam: Use devm_hid_hw_start_and_open in steam_probe()
From: Li Zetao @ 2024-09-04 12:35 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the hid-steam module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the err_hw_close and err_hw_stop lables.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hid/hid-steam.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index bf8b633114be..d393762bf52f 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -1236,18 +1236,10 @@ static int steam_probe(struct hid_device *hdev,
* With the real steam controller interface, do not connect hidraw.
* Instead, create the client_hid and connect that.
*/
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDRAW);
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDRAW);
if (ret)
goto err_cancel_work;
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev,
- "%s:hid_hw_open\n",
- __func__);
- goto err_hw_stop;
- }
-
if (steam->quirks & STEAM_QUIRK_WIRELESS) {
hid_info(hdev, "Steam wireless receiver connected");
/* If using a wireless adaptor ask for connection status */
@@ -1261,7 +1253,7 @@ static int steam_probe(struct hid_device *hdev,
hid_err(hdev,
"%s:steam_register failed with error %d\n",
__func__, ret);
- goto err_hw_close;
+ goto err_cancel_work;
}
}
@@ -1283,10 +1275,6 @@ static int steam_probe(struct hid_device *hdev,
err_steam_unregister:
if (steam->connected)
steam_unregister(steam);
-err_hw_close:
- hid_hw_close(hdev);
-err_hw_stop:
- hid_hw_stop(hdev);
err_cancel_work:
cancel_work_sync(&steam->work_connect);
cancel_delayed_work_sync(&steam->mode_switch);
@@ -1312,8 +1300,6 @@ static void steam_remove(struct hid_device *hdev)
if (steam->quirks & STEAM_QUIRK_WIRELESS) {
hid_info(hdev, "Steam wireless receiver disconnected");
}
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
steam_unregister(steam);
}
--
2.34.1
^ permalink raw reply related
* [PATCH -next 08/19] HID: hid-picolcd: Use devm_hid_hw_start_and_open in picolcd_probe()
From: Li Zetao @ 2024-09-04 12:35 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the hid-picolcd module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the err_cleanup_hid_ll and
err_cleanup_hid_hw lables.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hid/hid-picolcd_core.c | 22 ++++------------------
1 file changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
index 297103be3381..973822d1b2db 100644
--- a/drivers/hid/hid-picolcd_core.c
+++ b/drivers/hid/hid-picolcd_core.c
@@ -551,23 +551,15 @@ static int picolcd_probe(struct hid_device *hdev,
goto err_cleanup_data;
}
- error = hid_hw_start(hdev, 0);
+ error = devm_hid_hw_start_and_open(hdev, 0);
if (error) {
- hid_err(hdev, "hardware start failed\n");
+ hid_err(hdev, "hardware start and open failed\n");
goto err_cleanup_data;
}
- error = hid_hw_open(hdev);
- if (error) {
- hid_err(hdev, "failed to open input interrupt pipe for key and IR events\n");
- goto err_cleanup_hid_hw;
- }
-
error = device_create_file(&hdev->dev, &dev_attr_operation_mode_delay);
- if (error) {
- hid_err(hdev, "failed to create sysfs attributes\n");
- goto err_cleanup_hid_ll;
- }
+ if (error)
+ goto err_cleanup_data;
error = device_create_file(&hdev->dev, &dev_attr_operation_mode);
if (error) {
@@ -589,10 +581,6 @@ static int picolcd_probe(struct hid_device *hdev,
device_remove_file(&hdev->dev, &dev_attr_operation_mode);
err_cleanup_sysfs1:
device_remove_file(&hdev->dev, &dev_attr_operation_mode_delay);
-err_cleanup_hid_ll:
- hid_hw_close(hdev);
-err_cleanup_hid_hw:
- hid_hw_stop(hdev);
err_cleanup_data:
kfree(data);
return error;
@@ -611,8 +599,6 @@ static void picolcd_remove(struct hid_device *hdev)
picolcd_exit_devfs(data);
device_remove_file(&hdev->dev, &dev_attr_operation_mode);
device_remove_file(&hdev->dev, &dev_attr_operation_mode_delay);
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
/* Shortcut potential pending reply that will never arrive */
spin_lock_irqsave(&data->lock, flags);
--
2.34.1
^ permalink raw reply related
* [PATCH -next 07/19] HID: shield: Use devm_hid_hw_start_and_open in shield_probe()
From: Li Zetao @ 2024-09-04 12:35 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the shield module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the err_stop and err_ts_create lables, and
directly return the error code when an error occurs.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hid/hid-nvidia-shield.c | 20 +++-----------------
1 file changed, 3 insertions(+), 17 deletions(-)
diff --git a/drivers/hid/hid-nvidia-shield.c b/drivers/hid/hid-nvidia-shield.c
index ff9078ad1961..747996a21dd9 100644
--- a/drivers/hid/hid-nvidia-shield.c
+++ b/drivers/hid/hid-nvidia-shield.c
@@ -1070,27 +1070,15 @@ static int shield_probe(struct hid_device *hdev, const struct hid_device_id *id)
ts = container_of(shield_dev, struct thunderstrike, base);
- ret = hid_hw_start(hdev, HID_CONNECT_HIDINPUT);
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDINPUT);
if (ret) {
- hid_err(hdev, "Failed to start HID device\n");
- goto err_ts_create;
- }
-
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "Failed to open HID device\n");
- goto err_stop;
+ thunderstrike_destroy(ts);
+ return ret;
}
thunderstrike_device_init_info(shield_dev);
return ret;
-
-err_stop:
- hid_hw_stop(hdev);
-err_ts_create:
- thunderstrike_destroy(ts);
- return ret;
}
static void shield_remove(struct hid_device *hdev)
@@ -1100,11 +1088,9 @@ static void shield_remove(struct hid_device *hdev)
ts = container_of(dev, struct thunderstrike, base);
- hid_hw_close(hdev);
thunderstrike_destroy(ts);
del_timer_sync(&ts->psy_stats_timer);
cancel_work_sync(&ts->hostcmd_req_work);
- hid_hw_stop(hdev);
}
static const struct hid_device_id shield_devices[] = {
--
2.34.1
^ permalink raw reply related
* [PATCH -next 06/19] HID: nintendo: Use devm_hid_hw_start_and_open in nintendo_hid_probe()
From: Li Zetao @ 2024-09-04 12:35 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the nintendo module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the err_close and err_stop lables.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hid/hid-nintendo.c | 23 ++++-------------------
1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 58cd0506e431..45ac4fd3c7ea 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -2673,31 +2673,23 @@ static int nintendo_hid_probe(struct hid_device *hdev,
*/
hdev->version |= 0x8000;
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
- if (ret) {
- hid_err(hdev, "HW start failed\n");
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
+ if (ret)
goto err_wq;
- }
-
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "cannot start hardware I/O\n");
- goto err_stop;
- }
hid_device_io_start(hdev);
ret = joycon_init(hdev);
if (ret) {
hid_err(hdev, "Failed to initialize controller; ret=%d\n", ret);
- goto err_close;
+ goto err_wq;
}
/* Initialize the leds */
ret = joycon_leds_create(ctlr);
if (ret) {
hid_err(hdev, "Failed to create leds; ret=%d\n", ret);
- goto err_close;
+ goto err_wq;
}
/* Initialize the battery power supply */
@@ -2720,10 +2712,6 @@ static int nintendo_hid_probe(struct hid_device *hdev,
err_ida:
ida_free(&nintendo_player_id_allocator, ctlr->player_id);
-err_close:
- hid_hw_close(hdev);
-err_stop:
- hid_hw_stop(hdev);
err_wq:
destroy_workqueue(ctlr->rumble_queue);
err:
@@ -2745,9 +2733,6 @@ static void nintendo_hid_remove(struct hid_device *hdev)
destroy_workqueue(ctlr->rumble_queue);
ida_free(&nintendo_player_id_allocator, ctlr->player_id);
-
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
#ifdef CONFIG_PM
--
2.34.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox