* Re: [PATCH 02/10] iio: gyro: hid-sensor-gyro-3d: use local struct device
From: Andy Shevchenko @ 2026-07-20 19:35 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko, linux-input, linux-iio,
linux-kernel
In-Reply-To: <1E009860-0A0F-4F8D-8017-CE5739F22144@gmail.com>
On Mon, Jul 20, 2026 at 10:20:28PM +0530, Sanjay Chitroda wrote:
> On 20 July 2026 5:24:33 pm IST, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> >On Mon, Jul 20, 2026 at 03:10:14PM +0530, Sanjay Chitroda via B4 Relay wrote:
...
> >> - dev_err(&pdev->dev, "failed to setup common attributes\n");
> >> + dev_err(dev, "failed to setup common attributes\n");
> >> return ret;
> >
> >Nope, First one (to eliminate a lot of churn) should be to convert this and
> >similar to use
> >
> > return dev_err_probe(...);
>
> Thank you for your input.
>
> I'm planning to have a incremental series: first introduce a local "dev"
> pointer, then convert the resource management to "devm_*", and finally
> replace the remaining error reporting with "dev_err_probe()".
>
> This keeps each series focused and avoids introducing intermediate patterns such as:
>
> ret = dev_err_probe(dev, ret, ...);
> goto error;
>
> Once the cleanup paths are removed, those sites naturally become:
>
> return dev_err_probe(dev, ret, ...);
>
> Does this approach sound reasonable?
No. First you should drop the dead code for sure (see above).
Second, try to convert to dev_err_probe() as many drivers as you can without an
intermediate change. If the driver requires an intermediate thing like the above
it means it requires bigger refactoring. So, you should not have intermediate
churn. With your current approach there are drivers that will have it, and for
them you need to have different strategy (see my proposal in the above lines).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2] HID: intel-ish-hid: ignore post-init ENUM_DEVICES from firmware
From: srinivas pandruvada @ 2026-07-20 20:59 UTC (permalink / raw)
To: Wahid7852, linux-input; +Cc: jikos, Lixu Zhang
In-Reply-To: <20260720175656.559351-1-wahidzk0091@gmail.com>
On Mon, 2026-07-20 at 23:26 +0530, Wahid7852 wrote:
> From: Wahid Khan <wahidzk0091@gmail.com>
>
> Some ISH firmware versions (observed on Tiger Lake LP, 8086:a0fc,
> GUID {33AECD58-B679-4E54-9BD9-A04D34F0C226}) periodically re-send an
> unsolicited HOSTIF_DM_ENUM_DEVICES response roughly every 79 seconds.
>
> The current guard collapses two distinct cases into one condition:
>
> if (!(response_flag) || init_done) -> bad packet + ish_hw_reset
>
> This incorrectly treats a valid post-init firmware announcement as a
> corrupted packet, triggering an ISH soft-reset and a full ISHTP
> re-initialisation cycle on each occurrence (~1100 times per day on
> affected hardware).
>
> Split the check: reject messages with no response flag as before, but
> simply discard valid ENUM_DEVICES messages that arrive after init is
> complete.
>
> Signed-off-by: Wahid Khan <wahidzk0091@gmail.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Lixu, please add your tested by. I think you already tested this.
Thanks,
Srinivas
> ---
> v2: added proper sign off
>
> drivers/hid/intel-ish-hid/ishtp-hid-client.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> index 6d64008f2..ec5e8afe7 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> @@ -113,8 +113,7 @@ static void process_recv(struct ishtp_cl
> *hid_ishtp_cl, void *recv_buf,
>
> switch (recv_msg->hdr.command & CMD_MASK) {
> case HOSTIF_DM_ENUM_DEVICES:
> - if ((!(recv_msg->hdr.command & ~CMD_MASK) ||
> - client_data->init_done)) {
> + if (!(recv_msg->hdr.command & ~CMD_MASK)) {
> ++client_data->bad_recv_cnt;
> report_bad_packet(hid_ishtp_cl,
> recv_msg,
> cur_pos,
> @@ -122,6 +121,8 @@ static void process_recv(struct ishtp_cl
> *hid_ishtp_cl, void *recv_buf,
> ish_hw_reset(ishtp_get_ishtp_device(
> hid_ishtp_cl));
> break;
> }
> + if (client_data->init_done)
> + break;
> client_data->hid_dev_count = (unsigned
> int)*payload;
> if (!client_data->hid_devices)
> client_data->hid_devices =
> devm_kcalloc(
> --
> 2.54.0
^ permalink raw reply
page: | 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