All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Andy Shevchenko <andriy.shevchenko@intel.com>,
	Stefan Binding <sbinding@opensource.cirrus.com>
Cc: Mark Brown <broonie@kernel.org>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Len Brown <lenb@kernel.org>, Mark Gross <markgross@kernel.org>,
	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,
	Lucas Tanure <tanureal@opensource.cirrus.com>
Subject: Re: [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()"
Date: Fri, 3 Dec 2021 12:14:28 +0100	[thread overview]
Message-ID: <a1f546c2-5c63-573a-c032-603c792f3f7c@redhat.com> (raw)
In-Reply-To: <Yan6JVpS50keP2Pl@smile.fi.intel.com>

Hi,

On 12/3/21 12:06, Andy Shevchenko wrote:
> On Thu, Dec 02, 2021 at 04:24:19PM +0000, Stefan Binding wrote:
>> From: Lucas Tanure <tanureal@opensource.cirrus.com>
>>
>> Revert commit bdc7ca008e1f ("spi: Remove unused function
>> spi_busnum_to_master()")
>> This function is needed for the spi version of i2c multi
>> instantiate driver.
> 
> I understand the intention, but I have no clue from this series (it lacks of
> proper cover letter, it lacks of much better and justified commit message in
> the patch 3) what is the actual issue. Without these to be provided it's no go
> for the series. Please, provide much better description what is the actual
> issue you are trying to solve (from patch 3 my guts telling me that this can
> be achieved differently without this code being involved).

Yes I assume that eventually there will be a follow-up which will
actually add some new ACPI HIDs to the new bus-multi-instantiate.c file ?

Can we please have (some of) those patches as part of the next
version, so that we can see how you will actually use this?

Also I'm wondering is this actually about ACPI device's having multiple
SpiSerialBusV2 resources in a single _CRS resource list ?

Or do you plan to use this for devices with only a single
I2cSerialBusV2 or SpiSerialBusV2 resource to e.g. share IRQ lookup
code between the 2 cases ?

If you plan to use this for devices with only a single
I2cSerialBusV2 or SpiSerialBusV2 resource, then I'm going to have to
nack this.

Each ACPI HID which needs to be handled in this code also needs an
entry in the i2c_multi_instantiate_ids[] list in drivers/acpi/scan.c
which is code which ends up loaded on every single ACPI system, so
we really only want to add HIDs there for the special case of having
multiple I2cSerialBusV2 or SpiSerialBusV2 resources in a single
ACPI Device / ACPI fwnode.

If you are looking to use this as a way to share code for other reasons
(which is a good goal to strive for!) please find another way, such
as e.g. adding some helper functions to drivers/gpio/gpiolib-acpi.c
(note there already are a couple of helpers there which you may use).

Regards,

Hans


  reply	other threads:[~2021-12-03 11:14 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-02 16:24 [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" Stefan Binding
2021-12-02 16:24 ` [PATCH 2/3] spi: Make spi_alloc_device and spi_add_device public again Stefan Binding
2021-12-02 16:24 ` [PATCH 3/3] platform/x86: Support Spi in i2c-multi-instantiate driver Stefan Binding
2021-12-03 11:22   ` Hans de Goede
2021-12-03 11:35   ` Andy Shevchenko
2021-12-02 16:53 ` [PATCH 1/3] spi: Revert "spi: Remove unused function spi_busnum_to_master()" Mark Brown
2021-12-03 11:06 ` Andy Shevchenko
2021-12-03 11:14   ` Hans de Goede [this message]
2021-12-10 18:10     ` Lucas tanure
2021-12-10 18:22       ` 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=a1f546c2-5c63-573a-c032-603c792f3f7c@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=andriy.shevchenko@intel.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 \
    --cc=tanureal@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.