linux-kernel - Re: [PATCH v2 0/6] Support Spi in i2c-multi-instantiate driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8160a1c8-544d-6d95-4f80-224c7a9a9d40@redhat.com>
Date:   Fri, 10 Dec 2021 17:54:34 +0100
From:   Hans de Goede <hdegoede@...hat.com>
To:     Stefan Binding <sbinding@...nsource.cirrus.com>,
        Mark Brown <broonie@...nel.org>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>, Mark Gross <markgross@...nel.org>
Cc:     linux-kernel@...r.kernel.org, linux-spi@...r.kernel.org,
        linux-acpi@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        patches@...nsource.cirrus.com
Subject: Re: [PATCH v2 0/6] Support Spi in i2c-multi-instantiate driver

Hi Stefan,

On 12/10/21 16:40, Stefan Binding wrote:
> Add support for SPI bus in the ic2-multi-instantiate driver as
> upcoming laptops will need to multi instantiate SPI devices from
> a single device node, which has multiple SpiSerialBus entries at
> the ACPI table.
> 
> With the new SPI support, i2c-multi-instantiate becomes
> bus-multi-instantiate and is moved to the ACPI folder.
> 
> The intention is to support the SPI bus by re-using the current
> I2C multi instantiate, instead of creating a new SPI multi
> instantiate, to make it possible for peripherals that can be
> controlled by I2C or SPI to have the same HID at the ACPI table.
> 
> The new driver (Bus multi instantiate, bmi) checks for the
> hard-coded bus type and returns -ENODEV in case of zero devices
> found for that bus. In the case of automatic bus detection, 
> the driver will give preference to I2C.
> 
> The expectation is for a device node in the ACPI table to have
> multiple I2cSerialBus only or multiple SpiSerialBus only, not
> a mix of both; and for the case where there are both entries in
> one device node, only the I2C ones would be probed.
> 
> This new bus multi instantiate will be used in CS35L41 HDA new
> driver, being upstreamed:
> https://lkml.org/lkml/2021/11/23/723

Unfortunately you never really answered my questions about v1
of this series:

https://lore.kernel.org/platform-driver-x86/a1f546c2-5c63-573a-c032-603c792f3f7c@redhat.com/

So looking at the linked CS35L41 HDA series there is a single
ACPI device node with a HID of CLSA0100 which describes
two CS35L41 amplifiers connected over I2C ?

I assume you are doing this work because there are also designs
where there is a similar CLSA0100 ACPI device which also describes
two CS35L41 amplifiers but then connected over SPI ?

It would really help if you can:

1. Answer my questions from v1
2. Provide a concrete example of a device where these changes will
be necessary to make things work, preferably with a link to an
actual ACPI DSDT of that device.

Until you can better clarify why this is necessary, this series
gets a nack from me. The i2c-mult-instantiate code is a hack to
deal with some rather sub-optimal choices made in DSDTs used on
devices shipped with Windows and unless absolutely necessary
I would rather not see this get expanded to SPI.

Regards,

Hans