From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-22.5 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 147D7C433DB for ; Sat, 16 Jan 2021 18:02:45 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BD35E22510 for ; Sat, 16 Jan 2021 18:02:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727839AbhAPSC2 (ORCPT ); Sat, 16 Jan 2021 13:02:28 -0500 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:30301 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727284AbhAPSC1 (ORCPT ); Sat, 16 Jan 2021 13:02:27 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1610820060; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=VcTYlJfv01axpJK4vnd7O3uwpWJGDHk4puaBt63QC6I=; b=OkX8tEIb0v+5kmOpi2KOX31Rf3jWQeRF8RBeb0BnBOCKI9y2T2ZwbF2CLA+Bw78k6i0hO+ D7Hk5DVFyPjT4YDcIAfu+52X4RdEIj/iAkjGtJp32/N1XI17OJ/+1PoLefuE0Bz4uQxbmB 8grZFhqUD++V1VrHxvm7k/tJ+4RbUA8= Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-222-kgBcD1TqMzaNhTqZt9rFHA-1; Sat, 16 Jan 2021 09:46:45 -0500 X-MC-Unique: kgBcD1TqMzaNhTqZt9rFHA-1 Received: by mail-ed1-f72.google.com with SMTP id x13so5421914edi.7 for ; Sat, 16 Jan 2021 06:46:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=VcTYlJfv01axpJK4vnd7O3uwpWJGDHk4puaBt63QC6I=; b=fdQRXlfPoCMuTRG2S+xrFH/yVbUzgwHFUs7xQjez/CinU2ZnX/VC3shnP5BRLTkKc3 aEwgHW9RrAcWzPHaFpTNi2EUHwhCOXCpwzCg01frdalo1+uIdcG9X/XfJakZVbP2FpdS 5PzKeUEjTTeTZUQDdgNBYDQzr2+eSwaDT+QTsNVtNK0DKRCNhfxqrA/huKn6W41b6cXw uNcBGIYCsI35T92WkweBxcxZE21ZDCsX1UOPL1F61OZ5rcNu3yRBV+ULFFWMMyfBFNOf NtvC2VRu6iqPuefRepoHKX22a0jY5I3deot24pgmuwmyafsgWJKd53b8K2/oqFo95EVR DXxg== X-Gm-Message-State: AOAM530LZ9ycPbOgCdGB0c0hrO7J3/qtIt2AVxfmqnfK712mx5iHQGfw LuFpvTT0s0bn/4du70ip0R8m3bC4n/q4O8MoFhkay09ZlmvloAFUbDCWGUz5kX8/EUdM45Ly3J/ DEFdsrQS8vZcEhzrs3UtgwjqT X-Received: by 2002:a17:906:804c:: with SMTP id x12mr11998502ejw.42.1610808404324; Sat, 16 Jan 2021 06:46:44 -0800 (PST) X-Google-Smtp-Source: ABdhPJwh2dzEePXou8ehRPVsaKmmllVB6qkpljg6ffqyogAis6ElOP2KdZ5P145rlzQU4nWJvJd3WQ== X-Received: by 2002:a17:906:804c:: with SMTP id x12mr11998494ejw.42.1610808404115; Sat, 16 Jan 2021 06:46:44 -0800 (PST) Received: from x1.localdomain (2001-1c00-0c1e-bf00-37a3-353b-be90-1238.cable.dynamic.v6.ziggo.nl. [2001:1c00:c1e:bf00:37a3:353b:be90:1238]) by smtp.gmail.com with ESMTPSA id p16sm7478518edw.44.2021.01.16.06.46.43 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 16 Jan 2021 06:46:43 -0800 (PST) Subject: Re: [PATCH 03/14] mfd: arizona: Add support for ACPI enumeration of WM5102 connected over SPI To: Andy Shevchenko Cc: Lee Jones , MyungJoo Ham , Chanwoo Choi , Cezary Rojewski , Pierre-Louis Bossart , Liam Girdwood , Jie Yang , Mark Brown , patches@opensource.cirrus.com, Linux Kernel Mailing List , ALSA Development Mailing List , Christian Hartmann References: <20201227211232.117801-1-hdegoede@redhat.com> <20201227211232.117801-4-hdegoede@redhat.com> From: Hans de Goede Message-ID: <3946c0c5-6ace-47f9-9a3e-182bf6615d1f@redhat.com> Date: Sat, 16 Jan 2021 15:46:42 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Thank you for the review. On 12/28/20 3:14 PM, Andy Shevchenko wrote: > On Sun, Dec 27, 2020 at 11:16 PM Hans de Goede wrote: >> >> The Intel Bay Trail (x86/ACPI) based Lenovo Yoga Tablet 2 series use >> a WM5102 codec connected over SPI. >> >> Add support for ACPI enumeration to arizona-spi so that arizona-spi can >> bind to the codec on these tablets. >> >> This is loosely based on an earlier attempt (for Android-x86) at this by >> Christian Hartmann, combined with insights in things like the speaker GPIO >> from the android-x86 android port for the Lenovo Yoga Tablet 2 1051F/L [1]. > > Few nitpicks here and there, but the most important bit that hits me > is device_get_match_data(). > >> [1] https://github.com/Kitsune2222/Android_Yoga_Tablet_2-1051F_Kernel >> >> Cc: Christian Hartmann >> Signed-off-by: Hans de Goede >> --- >> drivers/mfd/arizona-spi.c | 120 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 120 insertions(+) >> >> diff --git a/drivers/mfd/arizona-spi.c b/drivers/mfd/arizona-spi.c >> index 704f214d2614..bcdbd72fefb5 100644 >> --- a/drivers/mfd/arizona-spi.c >> +++ b/drivers/mfd/arizona-spi.c >> @@ -7,7 +7,10 @@ >> * Author: Mark Brown >> */ >> >> +#include >> #include >> +#include >> +#include >> #include >> #include >> #include >> @@ -15,11 +18,119 @@ >> #include >> #include >> #include >> +#include >> >> #include >> >> #include "arizona.h" >> >> +#ifdef CONFIG_ACPI >> +const struct acpi_gpio_params reset_gpios = { 1, 0, false }; >> +const struct acpi_gpio_params ldoena_gpios = { 2, 0, false }; >> + >> +static const struct acpi_gpio_mapping arizona_acpi_gpios[] = { >> + { "reset-gpios", &reset_gpios, 1, }, >> + { "wlf,ldoena-gpios", &ldoena_gpios, 1 }, >> + { } >> +}; >> + >> +/* >> + * The ACPI resources for the device only describe external GPIO-s. They do >> + * not provide mappings for the GPIO-s coming from the Arizona codec itself. >> + */ >> +static const struct gpiod_lookup arizona_soc_gpios[] = { >> + { "arizona", 2, "wlf,spkvdd-ena", 0, GPIO_ACTIVE_HIGH }, >> + { "arizona", 4, "wlf,micd-pol", 0, GPIO_ACTIVE_LOW }, >> +}; >> + >> +/* >> + * The AOSP 3.5 mm Headset: Accessory Specification gives the following values: >> + * Function A Play/Pause: 0 ohm >> + * Function D Voice assistant: 135 ohm >> + * Function B Volume Up 240 ohm >> + * Function C Volume Down 470 ohm >> + * Minimum Mic DC resistance 1000 ohm >> + * Minimum Ear speaker impedance 16 ohm >> + * Note the first max value below must be less then the min. speaker impedance, >> + * to allow CTIA/OMTP detection to work. The other max values are the closest >> + * value from extcon-arizona.c:arizona_micd_levels halfway 2 button resistances. >> + */ >> +static const struct arizona_micd_range arizona_micd_aosp_ranges[] = { >> + { .max = 11, .key = KEY_PLAYPAUSE }, >> + { .max = 186, .key = KEY_VOICECOMMAND }, >> + { .max = 348, .key = KEY_VOLUMEUP }, >> + { .max = 752, .key = KEY_VOLUMEDOWN }, >> +}; >> + >> +static void arizona_spi_acpi_remove_lookup(void *lookup) >> +{ >> + gpiod_remove_lookup_table(lookup); >> +} >> + >> +static int arizona_spi_acpi_probe(struct arizona *arizona) >> +{ >> + struct gpiod_lookup_table *lookup; >> + int i, ret; >> + >> + /* Add mappings for the 2 ACPI declared GPIOs used for reset and ldo-ena */ >> + devm_acpi_dev_add_driver_gpios(arizona->dev, arizona_acpi_gpios); >> + >> + /* Add lookups for the SoCs own GPIOs used for micdet-polarity and spkVDD-enable */ >> + lookup = devm_kzalloc(arizona->dev, >> + struct_size(lookup, table, ARRAY_SIZE(arizona_soc_gpios) + 1), >> + GFP_KERNEL); >> + if (!lookup) >> + return -ENOMEM; >> + >> + lookup->dev_id = dev_name(arizona->dev); > >> + for (i = 0; i < ARRAY_SIZE(arizona_soc_gpios); i++) >> + lookup->table[i] = arizona_soc_gpios[i]; > > Would memcpy() do the same at one pass? True, fixed for v2. >> + gpiod_add_lookup_table(lookup); >> + ret = devm_add_action_or_reset(arizona->dev, arizona_spi_acpi_remove_lookup, lookup); >> + if (ret) >> + return ret; > >> + /* Enable 32KHz clock from SoC to codec for jack-detect */ >> + acpi_evaluate_object(ACPI_HANDLE(arizona->dev), "CLKE", NULL, NULL); > > No error check? The codec will still work without the 32KHz clk, but we will loose jack-detect functionality. I expect the ACPI call to already print some error, but to make sure something is logged and to clarify what any previous logged ACPI errors are about I've added code to log a warning when this fails for v2. > >> + /* >> + * Some DSDTs wrongly declare the IRQ trigger-type as IRQF_TRIGGER_FALLING >> + * The IRQ line will stay low when a new IRQ event happens between reading >> + * the IRQ status flags and acknowledging them. When the IRQ line stays >> + * low like this the IRQ will never trigger again when its type is set >> + * to IRQF_TRIGGER_FALLING. Correct the IRQ trigger-type to fix this. >> + */ >> + arizona->pdata.irq_flags = IRQF_TRIGGER_LOW; >> + >> + /* Wait 200 ms after jack insertion */ >> + arizona->pdata.micd_detect_debounce = 200; >> + >> + /* Use standard AOSP values for headset-button mappings */ >> + arizona->pdata.micd_ranges = arizona_micd_aosp_ranges; >> + arizona->pdata.num_micd_ranges = ARRAY_SIZE(arizona_micd_aosp_ranges); >> + >> + return 0; >> +} >> + >> +static const struct acpi_device_id arizona_acpi_match[] = { >> + { >> + .id = "WM510204", >> + .driver_data = WM5102, >> + }, >> + { >> + .id = "WM510205", >> + .driver_data = WM5102, >> + }, > >> + { }, > > No need for comma here. Fixed for v2. > >> +}; >> +MODULE_DEVICE_TABLE(acpi, arizona_acpi_match); >> +#else > >> +static void arizona_spi_acpi_probe(struct arizona *arizona) >> +{ >> +} > > Can be one line? I find it more readable as is. > >> +#endif >> + >> static int arizona_spi_probe(struct spi_device *spi) >> { >> const struct spi_device_id *id = spi_get_device_id(spi); >> @@ -30,6 +141,8 @@ static int arizona_spi_probe(struct spi_device *spi) >> >> if (spi->dev.of_node) >> type = arizona_of_get_type(&spi->dev); >> + else if (ACPI_COMPANION(&spi->dev)) >> + type = (unsigned long)acpi_device_get_match_data(&spi->dev); > > Can we rather get rid of these and use device_get_match_data() directly? That is a good idea, I'll add a nw preparation patch to v2 replacing the custom arizona_of_get_type() helper with device_get_match_data(). >> else >> type = id->driver_data; >> >> @@ -75,6 +188,12 @@ static int arizona_spi_probe(struct spi_device *spi) >> arizona->dev = &spi->dev; >> arizona->irq = spi->irq; >> >> + if (ACPI_COMPANION(&spi->dev)) { > > has_acpi_companion() ? Fixed for v2. >> + ret = arizona_spi_acpi_probe(arizona); >> + if (ret) >> + return ret; >> + } >> + >> return arizona_dev_init(arizona); >> } >> >> @@ -102,6 +221,7 @@ static struct spi_driver arizona_spi_driver = { >> .name = "arizona", >> .pm = &arizona_pm_ops, >> .of_match_table = of_match_ptr(arizona_of_match), >> + .acpi_match_table = ACPI_PTR(arizona_acpi_match), >> }, >> .probe = arizona_spi_probe, >> .remove = arizona_spi_remove, >> -- >> 2.28.0 >> > > Regards, Hans From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-20.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 182C0C433E0 for ; Sat, 16 Jan 2021 14:47:58 +0000 (UTC) Received: from alsa0.perex.cz (alsa0.perex.cz [77.48.224.243]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3641022B45 for ; Sat, 16 Jan 2021 14:47:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3641022B45 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=alsa-devel-bounces@alsa-project.org Received: from alsa1.perex.cz (alsa1.perex.cz [207.180.221.201]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by alsa0.perex.cz (Postfix) with ESMTPS id 5E82417E1; Sat, 16 Jan 2021 15:47:02 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa0.perex.cz 5E82417E1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=alsa-project.org; s=default; t=1610808472; bh=HzY53iaMDkxlCrkaIL6qPiKVcTOdDyl+ydbCWpCclKg=; h=Subject:To:References:From:Date:In-Reply-To:Cc:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=Yoxv5iatfv1ne5CYHuJggjLC/oYxWns9gEP64LLVSfhlkopocddLgeLlRNXMVLxKk dp+aTx5jaJ4Qy3jOX24vTPSN3QLK5zzYFNEcpGkw1f//C6GW1VG+hwwLrCRgSPi/jO 0g7HNR+MMjBv27OCwr2utV5BjK2dMBl+A3JpKsA0= Received: from alsa1.perex.cz (localhost.localdomain [127.0.0.1]) by alsa1.perex.cz (Postfix) with ESMTP id DB06CF80134; Sat, 16 Jan 2021 15:47:01 +0100 (CET) Received: by alsa1.perex.cz (Postfix, from userid 50401) id EC152F8025E; Sat, 16 Jan 2021 15:46:58 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by alsa1.perex.cz (Postfix) with ESMTPS id B314CF80134 for ; Sat, 16 Jan 2021 15:46:50 +0100 (CET) DKIM-Filter: OpenDKIM Filter v2.11.0 alsa1.perex.cz B314CF80134 Authentication-Results: alsa1.perex.cz; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="LmIIXxKj" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1610808409; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=VcTYlJfv01axpJK4vnd7O3uwpWJGDHk4puaBt63QC6I=; b=LmIIXxKj15ck/0ztCJ881Anor8KygPAbv67C7lwJaQEcLrbbmz1Ob4dyRzYGVef4BLoMCB 4JuLDwaMtotX2L3AeOAmuqbg3M/1/ulb/P51n1XtOqyD1YjjHJO9SJbeLTf/h1YRwpB0tm SeXulEsVexO44gnBvuRjB4sD1fl4UPs= Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-485-rZ63W15iMIqVklS8zUkboQ-1; Sat, 16 Jan 2021 09:46:45 -0500 X-MC-Unique: rZ63W15iMIqVklS8zUkboQ-1 Received: by mail-ed1-f72.google.com with SMTP id a24so135731eda.14 for ; Sat, 16 Jan 2021 06:46:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=VcTYlJfv01axpJK4vnd7O3uwpWJGDHk4puaBt63QC6I=; b=WBsJ/UytyUSDmk9sTKNoHv8tBCly5BTIgf95uHiWaHgcCRBmWu/xJFw0bK8taxd+WB hG1Ak5MRM5UM73lbsfyantUwslbwgINudRQC3XCs0PYhkEwqvkJ2THgjISlYuDMG6SKB VJHxUAIAtWpcMSK/Aax3Edh0PqN5v8C9hAso2NjoADHDvpjTgDalNqR++bJRtfUzERun o/Gm3si0UsPrn7QtbNn9KDZRyameF3Q3R9yBHSGMc4s7E7MZRgbLWjMOr9b2Odfup3Z8 fDhloDylUhtY1i0XP1QVf6WM6D67STX5NjhJRPmNq5YNYxALxdPHyjHxn7U8L10DDrm3 o0vw== X-Gm-Message-State: AOAM531A+VAFcPFzEMJW+dfFtB+5oAbVswf398IPaUKj0BBGqvh+FgXM l6mhaaBfoOrSwISfxAhbLBh62TijwTkAiPpPqkHIzVHG3mHGF+dbtvHQ9OV+CRwR1PKWkULoiNt SbvE62f5ZpYjRyGdv2+vzZU0= X-Received: by 2002:a17:906:804c:: with SMTP id x12mr11998508ejw.42.1610808404334; Sat, 16 Jan 2021 06:46:44 -0800 (PST) X-Google-Smtp-Source: ABdhPJwh2dzEePXou8ehRPVsaKmmllVB6qkpljg6ffqyogAis6ElOP2KdZ5P145rlzQU4nWJvJd3WQ== X-Received: by 2002:a17:906:804c:: with SMTP id x12mr11998494ejw.42.1610808404115; Sat, 16 Jan 2021 06:46:44 -0800 (PST) Received: from x1.localdomain (2001-1c00-0c1e-bf00-37a3-353b-be90-1238.cable.dynamic.v6.ziggo.nl. [2001:1c00:c1e:bf00:37a3:353b:be90:1238]) by smtp.gmail.com with ESMTPSA id p16sm7478518edw.44.2021.01.16.06.46.43 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 16 Jan 2021 06:46:43 -0800 (PST) Subject: Re: [PATCH 03/14] mfd: arizona: Add support for ACPI enumeration of WM5102 connected over SPI To: Andy Shevchenko References: <20201227211232.117801-1-hdegoede@redhat.com> <20201227211232.117801-4-hdegoede@redhat.com> From: Hans de Goede Message-ID: <3946c0c5-6ace-47f9-9a3e-182bf6615d1f@redhat.com> Date: Sat, 16 Jan 2021 15:46:42 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 MIME-Version: 1.0 In-Reply-To: Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=hdegoede@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Cc: Cezary Rojewski , Christian Hartmann , ALSA Development Mailing List , patches@opensource.cirrus.com, Mark Brown , Jie Yang , Pierre-Louis Bossart , Linux Kernel Mailing List , Liam Girdwood , Chanwoo Choi , MyungJoo Ham , Lee Jones X-BeenThere: alsa-devel@alsa-project.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: "Alsa-devel mailing list for ALSA developers - http://www.alsa-project.org" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: "Alsa-devel" Hi, Thank you for the review. On 12/28/20 3:14 PM, Andy Shevchenko wrote: > On Sun, Dec 27, 2020 at 11:16 PM Hans de Goede wrote: >> >> The Intel Bay Trail (x86/ACPI) based Lenovo Yoga Tablet 2 series use >> a WM5102 codec connected over SPI. >> >> Add support for ACPI enumeration to arizona-spi so that arizona-spi can >> bind to the codec on these tablets. >> >> This is loosely based on an earlier attempt (for Android-x86) at this by >> Christian Hartmann, combined with insights in things like the speaker GPIO >> from the android-x86 android port for the Lenovo Yoga Tablet 2 1051F/L [1]. > > Few nitpicks here and there, but the most important bit that hits me > is device_get_match_data(). > >> [1] https://github.com/Kitsune2222/Android_Yoga_Tablet_2-1051F_Kernel >> >> Cc: Christian Hartmann >> Signed-off-by: Hans de Goede >> --- >> drivers/mfd/arizona-spi.c | 120 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 120 insertions(+) >> >> diff --git a/drivers/mfd/arizona-spi.c b/drivers/mfd/arizona-spi.c >> index 704f214d2614..bcdbd72fefb5 100644 >> --- a/drivers/mfd/arizona-spi.c >> +++ b/drivers/mfd/arizona-spi.c >> @@ -7,7 +7,10 @@ >> * Author: Mark Brown >> */ >> >> +#include >> #include >> +#include >> +#include >> #include >> #include >> #include >> @@ -15,11 +18,119 @@ >> #include >> #include >> #include >> +#include >> >> #include >> >> #include "arizona.h" >> >> +#ifdef CONFIG_ACPI >> +const struct acpi_gpio_params reset_gpios = { 1, 0, false }; >> +const struct acpi_gpio_params ldoena_gpios = { 2, 0, false }; >> + >> +static const struct acpi_gpio_mapping arizona_acpi_gpios[] = { >> + { "reset-gpios", &reset_gpios, 1, }, >> + { "wlf,ldoena-gpios", &ldoena_gpios, 1 }, >> + { } >> +}; >> + >> +/* >> + * The ACPI resources for the device only describe external GPIO-s. They do >> + * not provide mappings for the GPIO-s coming from the Arizona codec itself. >> + */ >> +static const struct gpiod_lookup arizona_soc_gpios[] = { >> + { "arizona", 2, "wlf,spkvdd-ena", 0, GPIO_ACTIVE_HIGH }, >> + { "arizona", 4, "wlf,micd-pol", 0, GPIO_ACTIVE_LOW }, >> +}; >> + >> +/* >> + * The AOSP 3.5 mm Headset: Accessory Specification gives the following values: >> + * Function A Play/Pause: 0 ohm >> + * Function D Voice assistant: 135 ohm >> + * Function B Volume Up 240 ohm >> + * Function C Volume Down 470 ohm >> + * Minimum Mic DC resistance 1000 ohm >> + * Minimum Ear speaker impedance 16 ohm >> + * Note the first max value below must be less then the min. speaker impedance, >> + * to allow CTIA/OMTP detection to work. The other max values are the closest >> + * value from extcon-arizona.c:arizona_micd_levels halfway 2 button resistances. >> + */ >> +static const struct arizona_micd_range arizona_micd_aosp_ranges[] = { >> + { .max = 11, .key = KEY_PLAYPAUSE }, >> + { .max = 186, .key = KEY_VOICECOMMAND }, >> + { .max = 348, .key = KEY_VOLUMEUP }, >> + { .max = 752, .key = KEY_VOLUMEDOWN }, >> +}; >> + >> +static void arizona_spi_acpi_remove_lookup(void *lookup) >> +{ >> + gpiod_remove_lookup_table(lookup); >> +} >> + >> +static int arizona_spi_acpi_probe(struct arizona *arizona) >> +{ >> + struct gpiod_lookup_table *lookup; >> + int i, ret; >> + >> + /* Add mappings for the 2 ACPI declared GPIOs used for reset and ldo-ena */ >> + devm_acpi_dev_add_driver_gpios(arizona->dev, arizona_acpi_gpios); >> + >> + /* Add lookups for the SoCs own GPIOs used for micdet-polarity and spkVDD-enable */ >> + lookup = devm_kzalloc(arizona->dev, >> + struct_size(lookup, table, ARRAY_SIZE(arizona_soc_gpios) + 1), >> + GFP_KERNEL); >> + if (!lookup) >> + return -ENOMEM; >> + >> + lookup->dev_id = dev_name(arizona->dev); > >> + for (i = 0; i < ARRAY_SIZE(arizona_soc_gpios); i++) >> + lookup->table[i] = arizona_soc_gpios[i]; > > Would memcpy() do the same at one pass? True, fixed for v2. >> + gpiod_add_lookup_table(lookup); >> + ret = devm_add_action_or_reset(arizona->dev, arizona_spi_acpi_remove_lookup, lookup); >> + if (ret) >> + return ret; > >> + /* Enable 32KHz clock from SoC to codec for jack-detect */ >> + acpi_evaluate_object(ACPI_HANDLE(arizona->dev), "CLKE", NULL, NULL); > > No error check? The codec will still work without the 32KHz clk, but we will loose jack-detect functionality. I expect the ACPI call to already print some error, but to make sure something is logged and to clarify what any previous logged ACPI errors are about I've added code to log a warning when this fails for v2. > >> + /* >> + * Some DSDTs wrongly declare the IRQ trigger-type as IRQF_TRIGGER_FALLING >> + * The IRQ line will stay low when a new IRQ event happens between reading >> + * the IRQ status flags and acknowledging them. When the IRQ line stays >> + * low like this the IRQ will never trigger again when its type is set >> + * to IRQF_TRIGGER_FALLING. Correct the IRQ trigger-type to fix this. >> + */ >> + arizona->pdata.irq_flags = IRQF_TRIGGER_LOW; >> + >> + /* Wait 200 ms after jack insertion */ >> + arizona->pdata.micd_detect_debounce = 200; >> + >> + /* Use standard AOSP values for headset-button mappings */ >> + arizona->pdata.micd_ranges = arizona_micd_aosp_ranges; >> + arizona->pdata.num_micd_ranges = ARRAY_SIZE(arizona_micd_aosp_ranges); >> + >> + return 0; >> +} >> + >> +static const struct acpi_device_id arizona_acpi_match[] = { >> + { >> + .id = "WM510204", >> + .driver_data = WM5102, >> + }, >> + { >> + .id = "WM510205", >> + .driver_data = WM5102, >> + }, > >> + { }, > > No need for comma here. Fixed for v2. > >> +}; >> +MODULE_DEVICE_TABLE(acpi, arizona_acpi_match); >> +#else > >> +static void arizona_spi_acpi_probe(struct arizona *arizona) >> +{ >> +} > > Can be one line? I find it more readable as is. > >> +#endif >> + >> static int arizona_spi_probe(struct spi_device *spi) >> { >> const struct spi_device_id *id = spi_get_device_id(spi); >> @@ -30,6 +141,8 @@ static int arizona_spi_probe(struct spi_device *spi) >> >> if (spi->dev.of_node) >> type = arizona_of_get_type(&spi->dev); >> + else if (ACPI_COMPANION(&spi->dev)) >> + type = (unsigned long)acpi_device_get_match_data(&spi->dev); > > Can we rather get rid of these and use device_get_match_data() directly? That is a good idea, I'll add a nw preparation patch to v2 replacing the custom arizona_of_get_type() helper with device_get_match_data(). >> else >> type = id->driver_data; >> >> @@ -75,6 +188,12 @@ static int arizona_spi_probe(struct spi_device *spi) >> arizona->dev = &spi->dev; >> arizona->irq = spi->irq; >> >> + if (ACPI_COMPANION(&spi->dev)) { > > has_acpi_companion() ? Fixed for v2. >> + ret = arizona_spi_acpi_probe(arizona); >> + if (ret) >> + return ret; >> + } >> + >> return arizona_dev_init(arizona); >> } >> >> @@ -102,6 +221,7 @@ static struct spi_driver arizona_spi_driver = { >> .name = "arizona", >> .pm = &arizona_pm_ops, >> .of_match_table = of_match_ptr(arizona_of_match), >> + .acpi_match_table = ACPI_PTR(arizona_acpi_match), >> }, >> .probe = arizona_spi_probe, >> .remove = arizona_spi_remove, >> -- >> 2.28.0 >> > > Regards, Hans