[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d4d4931e8c189ee5e87bf87d0090ca0c4f626e5b.camel@linux.intel.com>
Date: Tue, 23 Dec 2025 08:25:29 -0800
From: srinivas pandruvada <srinivas.pandruvada@...ux.intel.com>
To: Vishnu Sankar <vishnuocv@...il.com>, jikos@...nel.org,
bentiss@...nel.org, corbet@....net, vsankar@...ovo.com
Cc: linux-doc@...r.kernel.org, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org, Mark Pearson <mpearson-lenovo@...ebb.ca>,
Richie Roy Jayme <rjayme.jp@...il.com>
Subject: Re: [PATCH v3 1/2] HID: intel-ish-hid: loader: Add
PRODUCT_FAMILY-based firmware matching
On Mon, 2025-12-22 at 17:05 +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@...ebb.ca>
> Tested-by: Richie Roy Jayme <rjayme.jp@...il.com>
> Signed-off-by: Vishnu Sankar <vishnuocv@...il.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
> ---
> Changes in v3
> - Removed the duplicate defenition of ISH_FW_FILE_VENDOR_FAMILY_FMT
> ---
> Changes in v2
> - Indent corrected
> - More comments added
> ---
> drivers/hid/intel-ish-hid/ishtp/loader.c | 58
> +++++++++++++++++++++++-
> 1 file changed, 56 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..ffa2042bb316 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/loader.c
> +++ b/drivers/hid/intel-ish-hid/ishtp/loader.c
> @@ -195,13 +195,19 @@ 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"
> #define ISH_FW_FILE_VENDOR_NAME_FMT "intel/ish/ish_%s_%08x_%08x.bin"
> #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)
> @@ -228,6 +234,12 @@ static int _request_ish_firmware(const struct
> firmware **firmware_p,
> * for the given device in the following order, prioritizing custom
> firmware
> * with more precise matching patterns:
> *
> + *
> ish_${fw_generation}_${SYS_VENDOR_CRC32}_$(PRODUCT_FAMILY_CRC32)
> + * _$(PRODUCT_NAME_CRC32)_${PRODUCT_SKU_CRC32}.bin
> + *
> + *
> ish_${fw_generation}_${SYS_VENDOR_CRC32}_$(PRODUCT_FAMILY_CRC32)_${PR
> ODUCT_SKU_CRC32}.bin
> + *
> ish_${fw_generation}_${SYS_VENDOR_CRC32}_$(PRODUCT_FAMILY_CRC32)_$(PR
> ODUCT_NAME_CRC32).bin
> + *
> ish_${fw_generation}_${SYS_VENDOR_CRC32}_$(PRODUCT_FAMILY_CRC32).bin
> *
> ish_${fw_generation}_${SYS_VENDOR_CRC32}_$(PRODUCT_NAME_CRC32)_${PROD
> UCT_SKU_CRC32}.bin
> *
> ish_${fw_generation}_${SYS_VENDOR_CRC32}_${PRODUCT_SKU_CRC32}.bin
> *
> ish_${fw_generation}_${SYS_VENDOR_CRC32}_$(PRODUCT_NAME_CRC32).bin
> @@ -256,8 +268,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,14 +278,55 @@ 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;
> + if (product_family)
> + family_crc = crc32(ISH_CRC_INIT, product_family,
> + strlen(product_family)) ^
> ISH_CRC_XOROUT;
> if (product_name)
> 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;
>
> + /* 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,
> vendor_crc, name_crc, sku_crc);
Powered by blists - more mailing lists