All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Stefan Binding <sbinding@opensource.cirrus.com>
Cc: Mark Brown <broonie@kernel.org>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Len Brown <lenb@kernel.org>, Hans de Goede <hdegoede@redhat.com>,
	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
Subject: Re: [PATCH 3/3] platform/x86: Support Spi in i2c-multi-instantiate driver
Date: Fri, 3 Dec 2021 13:35:44 +0200	[thread overview]
Message-ID: <YaoBEKJtuAb9xUAk@smile.fi.intel.com> (raw)
In-Reply-To: <20211202162421.7628-3-sbinding@opensource.cirrus.com>

On Thu, Dec 02, 2021 at 04:24:21PM +0000, Stefan Binding wrote:
> Add support for spi bus in i2c-multi-instantiate driver
> and rename it for a multiple purpose driver name
> By adding spi support into this driver enables devices
> to use the same _HID string for i2c and spi uses and
> minimize the support for two drivers doing the same thing
> for different busses

Please take care about periods at the end of sentences.
But this is minor in comparison to the following issues:

 - you enable this for existing I²C multi-instantiate devices,
   are you sure it is fine?

 - continuing above, how can you guarantee that the same ID would
   be used I²C and SPI versions of the same chip and not, let's say,
   for UART?

 - or other way around, how do we know that the same component will
   have the same ID for different bus types? (Yes, I understand that
   this is logically appropriate assumption, but you never know OEMs
   and others in their ways to (ab)use ACPI specifications and common
   sense)

 - if we even go this way, it should be under drivers/acpi


-- 
With Best Regards,
Andy Shevchenko



  parent reply	other threads:[~2021-12-03 11:36 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 [this message]
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
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=YaoBEKJtuAb9xUAk@smile.fi.intel.com \
    --to=andriy.shevchenko@intel.com \
    --cc=broonie@kernel.org \
    --cc=hdegoede@redhat.com \
    --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.