* [PATCH] HID: intel-ish-hid: loader: Add PRODUCT_FAMILY-based firmware matching
@ 2025-12-18 14:00 Vishnu Sankar
2025-12-18 15:54 ` srinivas pandruvada
0 siblings, 1 reply; 4+ messages in thread
From: Vishnu Sankar @ 2025-12-18 14:00 UTC (permalink / raw)
To: srinivas.pandruvada, jikos, bentiss, vsankar
Cc: linux-input, linux-kernel, Vishnu Sankar, Mark Pearson,
Richie Roy Jayme
Add support for firmware filenames that include the CRC32 checksum of the
DMI product_family field. Several OEMs ship ISH firmware variants shared
across a product family while product_name or product_sku may differ. This
intermediate matching granularity reduces duplication and improves firmware
selection for vendor-customized platforms.
The newly supported filename forms are checked before existing patterns:
ish_${gen}_${vendor}_${family}_${name}_${sku}.bin
ish_${gen}_${vendor}_${family}_${sku}.bin
ish_${gen}_${vendor}_${family}_${name}.bin
ish_${gen}_${vendor}_${family}.bin
The legacy product_name/product_sku rules remain unchanged and continue
to provide fallback matching.
ISH_FW_FILENAME_LEN_MAX is changed to 72 to accommodate the product_family.
Tested with X9 series and X1 series.
Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
Tested-by: Richie Roy Jayme <rjayme.jp@gmail.com>
Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
---
drivers/hid/intel-ish-hid/ishtp/loader.c | 50 +++++++++++++++++++++++-
1 file changed, 48 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/intel-ish-hid/ishtp/loader.c b/drivers/hid/intel-ish-hid/ishtp/loader.c
index f34086b29cf0..4559d460fd83 100644
--- a/drivers/hid/intel-ish-hid/ishtp/loader.c
+++ b/drivers/hid/intel-ish-hid/ishtp/loader.c
@@ -194,6 +194,11 @@ static int prepare_dma_bufs(struct ishtp_device *dev,
return 0;
}
+/* Patterns with PRODUCT_FAMILY */
+#define ISH_FW_FILE_VENDOR_FAMILY_NAME_SKU_FMT "intel/ish/ish_%s_%08x_%08x_%08x_%08x.bin"
+#define ISH_FW_FILE_VENDOR_FAMILY_SKU_FMT "intel/ish/ish_%s_%08x_%08x_%08x.bin"
+#define ISH_FW_FILE_VENDOR_FAMILY_NAME_FMT "intel/ish/ish_%s_%08x_%08x_%08x.bin"
+#define ISH_FW_FILE_VENDOR_FAMILY_FMT "intel/ish/ish_%s_%08x_%08x.bin"
#define ISH_FW_FILE_VENDOR_NAME_SKU_FMT "intel/ish/ish_%s_%08x_%08x_%08x.bin"
#define ISH_FW_FILE_VENDOR_SKU_FMT "intel/ish/ish_%s_%08x_%08x.bin"
@@ -201,7 +206,7 @@ static int prepare_dma_bufs(struct ishtp_device *dev,
#define ISH_FW_FILE_VENDOR_FMT "intel/ish/ish_%s_%08x.bin"
#define ISH_FW_FILE_DEFAULT_FMT "intel/ish/ish_%s.bin"
-#define ISH_FW_FILENAME_LEN_MAX 56
+#define ISH_FW_FILENAME_LEN_MAX 72
#define ISH_CRC_INIT (~0u)
#define ISH_CRC_XOROUT (~0u)
@@ -256,8 +261,9 @@ static int request_ish_firmware(const struct firmware **firmware_p,
struct device *dev)
{
const char *gen, *sys_vendor, *product_name, *product_sku;
+ const char *product_family;
struct ishtp_device *ishtp = dev_get_drvdata(dev);
- u32 vendor_crc, name_crc, sku_crc;
+ u32 vendor_crc, name_crc, sku_crc, family_crc;
char filename[ISH_FW_FILENAME_LEN_MAX];
int ret;
@@ -265,6 +271,7 @@ static int request_ish_firmware(const struct firmware **firmware_p,
sys_vendor = dmi_get_system_info(DMI_SYS_VENDOR);
product_name = dmi_get_system_info(DMI_PRODUCT_NAME);
product_sku = dmi_get_system_info(DMI_PRODUCT_SKU);
+ product_family = dmi_get_system_info(DMI_PRODUCT_FAMILY);
if (sys_vendor)
vendor_crc = crc32(ISH_CRC_INIT, sys_vendor, strlen(sys_vendor)) ^ ISH_CRC_XOROUT;
@@ -272,6 +279,45 @@ static int request_ish_firmware(const struct firmware **firmware_p,
name_crc = crc32(ISH_CRC_INIT, product_name, strlen(product_name)) ^ ISH_CRC_XOROUT;
if (product_sku)
sku_crc = crc32(ISH_CRC_INIT, product_sku, strlen(product_sku)) ^ ISH_CRC_XOROUT;
+ if (product_family)
+ family_crc = crc32(ISH_CRC_INIT, product_family, strlen(product_family)) ^ ISH_CRC_XOROUT;
+
+ /* PRODUCT_FAMILY-extended matching */
+ if (sys_vendor && product_family && product_name && product_sku) {
+ snprintf(filename, sizeof(filename),
+ ISH_FW_FILE_VENDOR_FAMILY_NAME_SKU_FMT,
+ gen, vendor_crc, family_crc, name_crc, sku_crc);
+ ret = _request_ish_firmware(firmware_p, filename, dev);
+ if (!ret)
+ return 0;
+ }
+
+ if (sys_vendor && product_family && product_sku) {
+ snprintf(filename, sizeof(filename),
+ ISH_FW_FILE_VENDOR_FAMILY_SKU_FMT,
+ gen, vendor_crc, family_crc, sku_crc);
+ ret = _request_ish_firmware(firmware_p, filename, dev);
+ if (!ret)
+ return 0;
+ }
+
+ if (sys_vendor && product_family && product_name) {
+ snprintf(filename, sizeof(filename),
+ ISH_FW_FILE_VENDOR_FAMILY_NAME_FMT,
+ gen, vendor_crc, family_crc, name_crc);
+ ret = _request_ish_firmware(firmware_p, filename, dev);
+ if (!ret)
+ return 0;
+ }
+
+ if (sys_vendor && product_family) {
+ snprintf(filename, sizeof(filename),
+ ISH_FW_FILE_VENDOR_FAMILY_FMT,
+ gen, vendor_crc, family_crc);
+ ret = _request_ish_firmware(firmware_p, filename, dev);
+ if (!ret)
+ return 0;
+}
if (sys_vendor && product_name && product_sku) {
snprintf(filename, sizeof(filename), ISH_FW_FILE_VENDOR_NAME_SKU_FMT, gen,
--
2.51.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] HID: intel-ish-hid: loader: Add PRODUCT_FAMILY-based firmware matching
2025-12-18 14:00 [PATCH] HID: intel-ish-hid: loader: Add PRODUCT_FAMILY-based firmware matching Vishnu Sankar
@ 2025-12-18 15:54 ` srinivas pandruvada
2025-12-19 1:26 ` Zhang, Lixu
0 siblings, 1 reply; 4+ messages in thread
From: srinivas pandruvada @ 2025-12-18 15:54 UTC (permalink / raw)
To: Vishnu Sankar, jikos, bentiss, vsankar, Zhang, Lixu, Even Xu
Cc: linux-input, linux-kernel, Mark Pearson, Richie Roy Jayme
+ Lixu
+ Even
On Thu, 2025-12-18 at 23:00 +0900, Vishnu Sankar wrote:
> Add support for firmware filenames that include the CRC32 checksum of
> the
> DMI product_family field. Several OEMs ship ISH firmware variants
> shared
> across a product family while product_name or product_sku may differ.
> This
> intermediate matching granularity reduces duplication and improves
> firmware
> selection for vendor-customized platforms.
>
> The newly supported filename forms are checked before existing
> patterns:
>
> ish_${gen}_${vendor}_${family}_${name}_${sku}.bin
> ish_${gen}_${vendor}_${family}_${sku}.bin
> ish_${gen}_${vendor}_${family}_${name}.bin
> ish_${gen}_${vendor}_${family}.bin
>
> The legacy product_name/product_sku rules remain unchanged and
> continue
> to provide fallback matching.
>
> ISH_FW_FILENAME_LEN_MAX is changed to 72 to accommodate the
> product_family.
>
> Tested with X9 series and X1 series.
>
> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> Tested-by: Richie Roy Jayme <rjayme.jp@gmail.com>
> Signed-off-by: Vishnu Sankar <vishnuocv@gmail.com>
> ---
> drivers/hid/intel-ish-hid/ishtp/loader.c | 50
> +++++++++++++++++++++++-
> 1 file changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/intel-ish-hid/ishtp/loader.c
> b/drivers/hid/intel-ish-hid/ishtp/loader.c
> index f34086b29cf0..4559d460fd83 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/loader.c
> +++ b/drivers/hid/intel-ish-hid/ishtp/loader.c
> @@ -194,6 +194,11 @@ static int prepare_dma_bufs(struct ishtp_device
> *dev,
>
> return 0;
> }
> +/* Patterns with PRODUCT_FAMILY */
> +#define ISH_FW_FILE_VENDOR_FAMILY_NAME_SKU_FMT
> "intel/ish/ish_%s_%08x_%08x_%08x_%08x.bin"
> +#define ISH_FW_FILE_VENDOR_FAMILY_SKU_FMT
> "intel/ish/ish_%s_%08x_%08x_%08x.bin"
> +#define ISH_FW_FILE_VENDOR_FAMILY_NAME_FMT
> "intel/ish/ish_%s_%08x_%08x_%08x.bin"
> +#define ISH_FW_FILE_VENDOR_FAMILY_FMT
> "intel/ish/ish_%s_%08x_%08x.bin"
>
> #define ISH_FW_FILE_VENDOR_NAME_SKU_FMT
> "intel/ish/ish_%s_%08x_%08x_%08x.bin"
> #define ISH_FW_FILE_VENDOR_SKU_FMT "intel/ish/ish_%s_%08x_%08x.bin"
> @@ -201,7 +206,7 @@ static int prepare_dma_bufs(struct ishtp_device
> *dev,
> #define ISH_FW_FILE_VENDOR_FMT "intel/ish/ish_%s_%08x.bin"
> #define ISH_FW_FILE_DEFAULT_FMT "intel/ish/ish_%s.bin"
>
> -#define ISH_FW_FILENAME_LEN_MAX 56
> +#define ISH_FW_FILENAME_LEN_MAX 72
>
> #define ISH_CRC_INIT (~0u)
> #define ISH_CRC_XOROUT (~0u)
> @@ -256,8 +261,9 @@ static int request_ish_firmware(const struct
> firmware **firmware_p,
> struct device *dev)
> {
> const char *gen, *sys_vendor, *product_name, *product_sku;
> + const char *product_family;
> struct ishtp_device *ishtp = dev_get_drvdata(dev);
> - u32 vendor_crc, name_crc, sku_crc;
> + u32 vendor_crc, name_crc, sku_crc, family_crc;
> char filename[ISH_FW_FILENAME_LEN_MAX];
> int ret;
>
> @@ -265,6 +271,7 @@ static int request_ish_firmware(const struct
> firmware **firmware_p,
> sys_vendor = dmi_get_system_info(DMI_SYS_VENDOR);
> product_name = dmi_get_system_info(DMI_PRODUCT_NAME);
> product_sku = dmi_get_system_info(DMI_PRODUCT_SKU);
> + product_family = dmi_get_system_info(DMI_PRODUCT_FAMILY);
>
> if (sys_vendor)
> vendor_crc = crc32(ISH_CRC_INIT, sys_vendor,
> strlen(sys_vendor)) ^ ISH_CRC_XOROUT;
> @@ -272,6 +279,45 @@ static int request_ish_firmware(const struct
> firmware **firmware_p,
> name_crc = crc32(ISH_CRC_INIT, product_name,
> strlen(product_name)) ^ ISH_CRC_XOROUT;
> if (product_sku)
> sku_crc = crc32(ISH_CRC_INIT, product_sku,
> strlen(product_sku)) ^ ISH_CRC_XOROUT;
> + if (product_family)
> + family_crc = crc32(ISH_CRC_INIT, product_family,
> strlen(product_family)) ^ ISH_CRC_XOROUT;
> +
> + /* PRODUCT_FAMILY-extended matching */
> + if (sys_vendor && product_family && product_name &&
> product_sku) {
> + snprintf(filename, sizeof(filename),
> + ISH_FW_FILE_VENDOR_FAMILY_NAME_SKU_FMT,
> + gen, vendor_crc, family_crc, name_crc,
> sku_crc);
> + ret = _request_ish_firmware(firmware_p, filename,
> dev);
> + if (!ret)
> + return 0;
> + }
> +
> + if (sys_vendor && product_family && product_sku) {
> + snprintf(filename, sizeof(filename),
> + ISH_FW_FILE_VENDOR_FAMILY_SKU_FMT,
> + gen, vendor_crc, family_crc, sku_crc);
> + ret = _request_ish_firmware(firmware_p, filename,
> dev);
> + if (!ret)
> + return 0;
> + }
> +
> + if (sys_vendor && product_family && product_name) {
> + snprintf(filename, sizeof(filename),
> + ISH_FW_FILE_VENDOR_FAMILY_NAME_FMT,
> + gen, vendor_crc, family_crc, name_crc);
> + ret = _request_ish_firmware(firmware_p, filename,
> dev);
> + if (!ret)
> + return 0;
> + }
> +
> + if (sys_vendor && product_family) {
> + snprintf(filename, sizeof(filename),
> + ISH_FW_FILE_VENDOR_FAMILY_FMT,
> + gen, vendor_crc, family_crc);
> + ret = _request_ish_firmware(firmware_p, filename,
> dev);
> + if (!ret)
> + return 0;
> +}
>
> if (sys_vendor && product_name && product_sku) {
> snprintf(filename, sizeof(filename),
> ISH_FW_FILE_VENDOR_NAME_SKU_FMT, gen,
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH] HID: intel-ish-hid: loader: Add PRODUCT_FAMILY-based firmware matching
2025-12-18 15:54 ` srinivas pandruvada
@ 2025-12-19 1:26 ` Zhang, Lixu
2025-12-19 5:39 ` Vishnu Sankar
0 siblings, 1 reply; 4+ messages in thread
From: Zhang, Lixu @ 2025-12-19 1:26 UTC (permalink / raw)
To: srinivas pandruvada, Vishnu Sankar, jikos@kernel.org,
bentiss@kernel.org, Sankar, Vishnu, Xu, Even
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
Mark Pearson, Richie Roy Jayme
>-----Original Message-----
>From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
>Sent: Thursday, December 18, 2025 11:55 PM
>To: Vishnu Sankar <vishnuocv@gmail.com>; jikos@kernel.org;
>bentiss@kernel.org; Sankar, Vishnu <vsankar@lenovo.com>; Zhang, Lixu
><lixu.zhang@intel.com>; Xu, Even <even.xu@intel.com>
>Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Mark Pearson
><mpearson-lenovo@squebb.ca>; Richie Roy Jayme <rjayme.jp@gmail.com>
>Subject: Re: [PATCH] HID: intel-ish-hid: loader: Add PRODUCT_FAMILY-based
>firmware matching
>
>+ Lixu
>+ Even
>
>
>On Thu, 2025-12-18 at 23:00 +0900, Vishnu Sankar wrote:
>> Add support for firmware filenames that include the CRC32 checksum of
>> the DMI product_family field. Several OEMs ship ISH firmware variants
>> shared across a product family while product_name or product_sku may
>> differ.
>> This
>> intermediate matching granularity reduces duplication and improves
>> firmware selection for vendor-customized platforms.
...
>> +
>> + if (sys_vendor && product_family) {
>> + snprintf(filename, sizeof(filename),
>> + ISH_FW_FILE_VENDOR_FAMILY_FMT,
>> + gen, vendor_crc, family_crc);
>> + ret = _request_ish_firmware(firmware_p, filename,
>> dev);
>> + if (!ret)
>> + return 0;
>> +}
Indent issue.
Hi Vishnu,
Thanks for your patch.
1. Please run the ` ./scripts/checkpatch.pl --strict --codespell [PATCH FILE]`, and fix the issue.
2. Could you please also update the document `Documentation/hid/intel-ish-hid.rst`?
Thanks,
Lixu
>>
>> if (sys_vendor && product_name && product_sku) {
>> snprintf(filename, sizeof(filename),
>> ISH_FW_FILE_VENDOR_NAME_SKU_FMT, gen,
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] HID: intel-ish-hid: loader: Add PRODUCT_FAMILY-based firmware matching
2025-12-19 1:26 ` Zhang, Lixu
@ 2025-12-19 5:39 ` Vishnu Sankar
0 siblings, 0 replies; 4+ messages in thread
From: Vishnu Sankar @ 2025-12-19 5:39 UTC (permalink / raw)
To: Zhang, Lixu
Cc: srinivas pandruvada, jikos@kernel.org, bentiss@kernel.org,
Sankar, Vishnu, Xu, Even, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, Mark Pearson, Richie Roy Jayme
Hi Lixu,
Thank you for the comments.
On Fri, Dec 19, 2025 at 10:26 AM Zhang, Lixu <lixu.zhang@intel.com> wrote:
>
> >-----Original Message-----
> >From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
> >Sent: Thursday, December 18, 2025 11:55 PM
> >To: Vishnu Sankar <vishnuocv@gmail.com>; jikos@kernel.org;
> >bentiss@kernel.org; Sankar, Vishnu <vsankar@lenovo.com>; Zhang, Lixu
> ><lixu.zhang@intel.com>; Xu, Even <even.xu@intel.com>
> >Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; Mark Pearson
> ><mpearson-lenovo@squebb.ca>; Richie Roy Jayme <rjayme.jp@gmail.com>
> >Subject: Re: [PATCH] HID: intel-ish-hid: loader: Add PRODUCT_FAMILY-based
> >firmware matching
> >
> >+ Lixu
> >+ Even
> >
> >
> >On Thu, 2025-12-18 at 23:00 +0900, Vishnu Sankar wrote:
> >> Add support for firmware filenames that include the CRC32 checksum of
> >> the DMI product_family field. Several OEMs ship ISH firmware variants
> >> shared across a product family while product_name or product_sku may
> >> differ.
> >> This
> >> intermediate matching granularity reduces duplication and improves
> >> firmware selection for vendor-customized platforms.
>
> ...
>
> >> +
> >> + if (sys_vendor && product_family) {
> >> + snprintf(filename, sizeof(filename),
> >> + ISH_FW_FILE_VENDOR_FAMILY_FMT,
> >> + gen, vendor_crc, family_crc);
> >> + ret = _request_ish_firmware(firmware_p, filename,
> >> dev);
> >> + if (!ret)
> >> + return 0;
> >> +}
> Indent issue.
>
> Hi Vishnu,
> Thanks for your patch.
> 1. Please run the ` ./scripts/checkpatch.pl --strict --codespell [PATCH FILE]`, and fix the issue.
Acked
> 2. Could you please also update the document `Documentation/hid/intel-ish-hid.rst`?
Will update the document in v2.
>
> Thanks,
> Lixu
>
> >>
> >> if (sys_vendor && product_name && product_sku) {
> >> snprintf(filename, sizeof(filename),
> >> ISH_FW_FILE_VENDOR_NAME_SKU_FMT, gen,
--
Regards,
Vishnu Sankar
+817015150407 (Japan)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-12-19 5:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-18 14:00 [PATCH] HID: intel-ish-hid: loader: Add PRODUCT_FAMILY-based firmware matching Vishnu Sankar
2025-12-18 15:54 ` srinivas pandruvada
2025-12-19 1:26 ` Zhang, Lixu
2025-12-19 5:39 ` Vishnu Sankar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox