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: <8d976e9c-8fea-8a67-1331-7993705c60d9@redhat.com>
Date:   Wed, 28 Sep 2022 11:52:51 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.1
Subject: Re: [PATCH v2 0/2] platform/x86: dell: Add new dell-wmi-ddv driver
Content-Language: en-US, nl
To:     Armin Wolf <W_Armin@gmx.de>, markgross@kernel.org
Cc:     rafael@kernel.org, lenb@kernel.org, hmh@hmh.eng.br,
        matan@svgalib.org, corentin.chary@gmail.com, jeremy@system76.com,
        productdev@system76.com, mario.limonciello@amd.com,
        pobrn@protonmail.com, coproscefalo@gmail.com,
        platform-driver-x86@vger.kernel.org, linux-acpi@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20220927204521.601887-1-W_Armin@gmx.de>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <20220927204521.601887-1-W_Armin@gmx.de>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi,

On 9/27/22 22:45, Armin Wolf wrote:
> This patch series adds a new driver for a WMI interface found in
> many newer Dell machines. This interface allows to read battery
> properties like temperature and the ePPID (Dell-specific), while
> also providing fan and thermal sensor information.
> 
> The interface does support multiple batteries which are indentified
> by an "index", which appears to be the battery ACPI UID. Since
> the interface also appears to omit any bounts checking of the
> index, the ACPI battery hook mechanism is used to discover batteries.
> 
> Since the information returned when querying fan/thermal sensor
> information is currently unknown, a debugfs entry is created to
> allow for easier reverse engineering. The interface is likely
> to be replaced by a proper hwmon interface in the future.
> 
> Since the driver can potentially be instantiated multiple times,
> the ACPI battery hook mechanism had to be extended.
> 
> The first patch passes a pointer to the battery hook to the
> hook callbacks, so that they can access instance-specific data
> with container_of().
> 
> The second patch finally adds the new driver. It was called
> dell-wmi-ddv since the interface is called "DDV" by Dell software,
> likely meaning "Dell Data Vault".
> 
> The driver was tested, together with the changes made to the
> ACPI battery driver, on a Dell Inspiron 3505. Other drivers
> already using the battery hook mechanism where changed as well,
> but could only be compile-tested due to missing hardware.
> 
> ---
> Changes in v2:
> - Significantly lower the amount of changes being made to the
> acpi battery driver
> - drop unnecessary ABI description of the temp attribute
> - return 0 when a unsupported battery is found to avoid being
> unloaded
> 
> Armin Wolf (2):
>   ACPI: battery: Pass battery hook pointer to hook callbacks
>   platform/x86: dell: Add new dell-wmi-ddv driver

Thanks.

The new version looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

for the series.

Rafael, from my POV this can be merged through either your
tree or mine. Feel free to merge this through your tree,
or please give your Ack for merging through mine
(assuming you are ok with the changes of course).

Regards,

Hans