Received-SPF: pass (google.com: domain of linux-kernel+bounces-8353-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 21 Dec 2023 14:41:27 +0200
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
Cc: hdegoede@redhat.com, lee@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] mfd: intel-lpss: Switch to generalized quirk table
Message-ID: <ZYQydxWHmKSQgZN4@smile.fi.intel.com>
References: <20231220073148.19402-1-alex.vinarskis@gmail.com>
 <20231220205621.8575-1-alex.vinarskis@gmail.com>
 <20231220205621.8575-2-alex.vinarskis@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231220205621.8575-2-alex.vinarskis@gmail.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo

On Wed, Dec 20, 2023 at 09:56:20PM +0100, Aleksandrs Vinarskis wrote:
> Introduce generic quirk table, and port existing walkaround for select
> Microsoft devices to it. This is a preparation for
> QUIRK_CLOCK_DIVIDER_UNITY.

Thank you for the update!
Some nit-picks below, after addressing them feel free to add
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

...

> -	{ }

> +	{}

No need to change here.

Also I'm not sure about "}, {" style. Lee as the maintainer can clarify
on this.

...

>  	const struct intel_lpss_platform_info *data = (void *)id->driver_data;
>  	struct intel_lpss_platform_info *info;
> +	const struct pci_device_id *quirk_pci_info;
>  	int ret;

I would preserve reversed xmas tree order:

	const struct intel_lpss_platform_info *data = (void *)id->driver_data;
	const struct pci_device_id *quirk_pci_info;
	struct intel_lpss_platform_info *info;
	int ret;

...

> +	quirk_pci_info = pci_match_id(quirk_ids, pdev);
> +	if (quirk_pci_info)
> +		info->quirks = quirk_pci_info->driver_data;

Can you confirm that there is no compiler warning if you compile for 32-bit?

...

> +/*
> + * Some DSDTs have an unused GEXP ACPI device conflicting with I2C4 resources
> + * Set to ignore resource conflicts with ACPI declared SystemMemory regions

Please, mind the periods at the ends of sentences.

> + */

...

> +	unsigned int quirks;

Depending on the above (32-bit compilation) this might be converted to
unsigned long.

-- 
With Best Regards,
Andy Shevchenko