All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Stefan Binding <sbinding@opensource.cirrus.com>,
	Mark Brown <broonie@kernel.org>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Len Brown <lenb@kernel.org>, Mark Gross <markgross@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org,
	linux-acpi@vger.kernel.org, platform-driver-x86@vger.kernel.org,
	patches@opensource.cirrus.com
Subject: Re: [PATCH v2 0/6] Support Spi in i2c-multi-instantiate driver
Date: Fri, 10 Dec 2021 17:54:34 +0100	[thread overview]
Message-ID: <8160a1c8-544d-6d95-4f80-224c7a9a9d40@redhat.com> (raw)
In-Reply-To: <20211210154050.3713-1-sbinding@opensource.cirrus.com>

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


  parent reply	other threads:[~2021-12-10 16:54 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-10 15:40 [PATCH v2 0/6] Support Spi in i2c-multi-instantiate driver Stefan Binding
2021-12-10 15:40 ` [PATCH v2 1/6] spi: Export acpi_spi_find_controller_by_adev to be used externally Stefan Binding
2021-12-10 15:40 ` [PATCH v2 2/6] spi: Make spi_alloc_device and spi_add_device public again Stefan Binding
2021-12-10 15:40 ` [PATCH v2 3/6] platform/x86: i2c-multi-instantiate: Move it to drivers/acpi folder Stefan Binding
2021-12-10 17:59   ` Mark Brown
2021-12-15 10:26     ` Hans de Goede
2021-12-10 15:40 ` [PATCH v2 4/6] ACPI: i2c-multi-instantiate: Rename it for a generic bus driver name Stefan Binding
2021-12-10 15:40 ` [PATCH v2 5/6] ACPI: bus-multi-instantiate: Reorganize I2C functions Stefan Binding
2021-12-10 15:40 ` [PATCH v2 6/6] ACPI: bus-multi-instantiate: Add SPI support Stefan Binding
2021-12-21 18:32   ` Hans de Goede
2021-12-21 19:22     ` Hans de Goede
2022-01-10 14:36     ` Stefan Binding
2022-01-11  9:20       ` Hans de Goede
2021-12-10 16:54 ` Hans de Goede [this message]
2021-12-13 11:40   ` [PATCH v2 0/6] Support Spi in i2c-multi-instantiate driver Stefan Binding
2021-12-15 10:22     ` Hans de Goede
2021-12-21 18:16 ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8160a1c8-544d-6d95-4f80-224c7a9a9d40@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=broonie@kernel.org \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=markgross@kernel.org \
    --cc=patches@opensource.cirrus.com \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=sbinding@opensource.cirrus.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.