Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <b9f81a5b-0511-9950-5a20-9e6cbd92d085@redhat.com>
Date:   Fri, 14 Jul 2023 19:10:21 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH] platform/x86: serial-multi-instantiate: Auto detect IRQ
 resource for CSC3551
Content-Language: en-US, nl
To:     David Xu <xuwd1@hotmail.com>, markgross@kernel.org
Cc:     platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org
References: <SY4P282MB18350CD8288687B87FFD2243E037A@SY4P282MB1835.AUSP282.PROD.OUTLOOK.COM>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <SY4P282MB18350CD8288687B87FFD2243E037A@SY4P282MB1835.AUSP282.PROD.OUTLOOK.COM>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

On 7/13/23 15:35, David Xu wrote:
> The current code assumes that the CSC3551(multiple cs35l41) always have
> its interrupt pin connected to GPIO thus the IRQ can be acquired with
> acpi_dev_gpio_irq_get. However on some newer laptop models this is no
> longer the case as they have the CSC3551's interrupt pin connected to
> APIC. This causes smi_i2c_probe to fail on these machines.
> 
> To support these machines, a new macro IRQ_RESOURCE_AUTO was introduced
> for cs35l41 smi_node, and smi_get_irq function was modified so it tries
> to get GPIO irq resource first and if failed, tries to get
> APIC irq resource for cs35l41.
> 
> This patch affects only the cs35l41's probing and brings no negative
> influence on machines that indeed have the cs35l41's interrupt pin
> connected to GPIO.
> 
> Signed-off-by: David Xu <xuwd1@hotmail.com>

Thank you for your patch, I've applied this patch to my fixes
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes

Note it will show up in my fixes branch once I've pushed my
local branch there, which might take a while.

I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.

Regards,

Hans


> ---
>  .../platform/x86/serial-multi-instantiate.c   | 21 +++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/serial-multi-instantiate.c b/drivers/platform/x86/serial-multi-instantiate.c
> index f3dcbdd72fec..2c2abf69f049 100644
> --- a/drivers/platform/x86/serial-multi-instantiate.c
> +++ b/drivers/platform/x86/serial-multi-instantiate.c
> @@ -21,6 +21,7 @@
>  #define IRQ_RESOURCE_NONE	0
>  #define IRQ_RESOURCE_GPIO	1
>  #define IRQ_RESOURCE_APIC	2
> +#define IRQ_RESOURCE_AUTO   3
>  
>  enum smi_bus_type {
>  	SMI_I2C,
> @@ -52,6 +53,18 @@ static int smi_get_irq(struct platform_device *pdev, struct acpi_device *adev,
>  	int ret;
>  
>  	switch (inst->flags & IRQ_RESOURCE_TYPE) {
> +	case IRQ_RESOURCE_AUTO:
> +		ret = acpi_dev_gpio_irq_get(adev, inst->irq_idx);
> +		if (ret > 0) {
> +			dev_dbg(&pdev->dev, "Using gpio irq\n");
> +			break;
> +		}
> +		ret = platform_get_irq(pdev, inst->irq_idx);
> +		if (ret > 0) {
> +			dev_dbg(&pdev->dev, "Using platform irq\n");
> +			break;
> +		}
> +		break;
>  	case IRQ_RESOURCE_GPIO:
>  		ret = acpi_dev_gpio_irq_get(adev, inst->irq_idx);
>  		break;
> @@ -307,10 +320,10 @@ static const struct smi_node int3515_data = {
>  
>  static const struct smi_node cs35l41_hda = {
>  	.instances = {
> -		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
> -		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
> -		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
> -		{ "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 },
> +		{ "cs35l41-hda", IRQ_RESOURCE_AUTO, 0 },
> +		{ "cs35l41-hda", IRQ_RESOURCE_AUTO, 0 },
> +		{ "cs35l41-hda", IRQ_RESOURCE_AUTO, 0 },
> +		{ "cs35l41-hda", IRQ_RESOURCE_AUTO, 0 },
>  		{}
>  	},
>  	.bus_type = SMI_AUTO_DETECT,