From: Greg KH <gregkh@linuxfoundation.org>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: alsa-devel@alsa-project.org, tiwai@suse.de,
Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
linux-kernel@vger.kernel.org, Hui Wang <hui.wang@canonical.com>,
Vinod Koul <vkoul@kernel.org>,
broonie@kernel.org, srinivas.kandagatla@linaro.org,
jank@cadence.com, slawomir.blauciak@intel.com,
Sanyog Kale <sanyog.r.kale@intel.com>,
Bard liao <yung-chuan.liao@linux.intel.com>,
Rander Wang <rander.wang@linux.intel.com>
Subject: Re: [PATCH 1/8] soundwire: bus_type: add master_device/driver support
Date: Wed, 4 Mar 2020 17:28:37 +0100 [thread overview]
Message-ID: <20200304162837.GA1763256@kroah.com> (raw)
In-Reply-To: <05dbe43c-abf8-9d5a-d808-35bf4defe4ba@linux.intel.com>
On Wed, Mar 04, 2020 at 09:17:07AM -0600, Pierre-Louis Bossart wrote:
>
>
> > Were the above lines agreed or not? Do you see driver for master devices
> > or not? Greg was okay with as well as these patches but I am not okay
> > with the driver part for master, so I would like to see that removed.
> >
> > Different reviewers can have different reasons.. I have given bunch of
> > reasons here, BUT I have not seen a single technical reason why this
> > cannot be done.
>
> With all due respect, I consider Greg as THE reviewer for device/driver
> questions. Your earlier proposal to use platform devices was rejected by
> Greg, and we've lost an entire month in the process, so I am somewhat
> dubious on your proposal not to use a driver.
>
> If you want a technical objection, let me restate what I already mentioned:
>
> If you look at the hierarchy, we have
>
> PCI device -> PCI driver
> soundwire_master_device0
> soundwire_slave(s) -> codec driver
> ...
> soundwire_master_deviceN
> soundwire_slave(s) -> codec driver
>
> You have not explained how I could possibly deal with power management
> without having a driver for the master_device(s). The pm_ops need to be
> inserted in a driver structure, which means we need a driver. And if we need
> a driver, then we might as well have a real driver with .probe .remove
> support, driver_register(), etc.
To weigh in here, yes, you need such a "device" here as it isn't the PCI
device that you can use, you need your own. Just like most other busses
have this (USB has host controller drivers as one example, that create
the "root bus" device that all other USB devices hang off of.) This
"controller device" should hang off of the hardware device be it a
platform/PCI/i2c/spi/serial/whatever type of controller. That's why it
is needed.
> I really don't see what's broken or unnecessary with these patches.
The "wait until something else happens" does seem a bit hacky, odds are
that's not really needed if you are using the driver model correctly,
but soundwire is "odd" in places so maybe that is necessary, I'll defer
to you two on that mess :)
thanks,
greg k-h
next prev parent reply other threads:[~2020-03-04 16:29 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-27 22:31 [PATCH 0/8] soundwire: remove platform devices, add SOF interfaces Pierre-Louis Bossart
2020-02-27 22:31 ` [PATCH 1/8] soundwire: bus_type: add master_device/driver support Pierre-Louis Bossart
2020-02-28 7:32 ` Greg KH
2020-02-28 15:53 ` Pierre-Louis Bossart
2020-03-03 5:41 ` Vinod Koul
2020-03-03 15:23 ` Pierre-Louis Bossart
2020-03-04 9:53 ` Vinod Koul
2020-03-04 15:17 ` Pierre-Louis Bossart
2020-03-04 16:28 ` Greg KH [this message]
2020-03-05 6:46 ` Vinod Koul
2020-03-05 6:36 ` Vinod Koul
2020-03-05 12:46 ` Pierre-Louis Bossart
2020-03-06 5:01 ` Vinod Koul
2020-03-06 15:40 ` Pierre-Louis Bossart
2020-03-11 6:36 ` Vinod Koul
2020-03-11 14:44 ` Pierre-Louis Bossart
2020-03-13 11:50 ` Vinod Koul
2020-03-13 16:54 ` Pierre-Louis Bossart
2020-03-14 9:49 ` Vinod Koul
2020-03-16 19:15 ` Pierre-Louis Bossart
2020-03-20 15:33 ` Vinod Koul
2020-03-20 16:36 ` Pierre-Louis Bossart
2020-03-23 12:16 ` Vinod Koul
2020-02-27 22:32 ` [PATCH 2/8] soundwire: intel: transition to sdw_master_device/driver support Pierre-Louis Bossart
2020-02-28 7:34 ` Greg KH
2020-02-28 16:01 ` Pierre-Louis Bossart
2020-03-03 6:05 ` Vinod Koul
2020-02-27 22:32 ` [PATCH 3/8] soundwire: intel_init: add implementation of sdw_intel_enable_irq() Pierre-Louis Bossart
2020-02-27 22:32 ` [PATCH 4/8] soundwire: intel_init: use EXPORT_SYMBOL_NS Pierre-Louis Bossart
2020-02-27 22:32 ` [PATCH 5/8] soundwire: intel/cadence: merge Soundwire interrupt handlers/threads Pierre-Louis Bossart
2020-02-27 22:32 ` [PATCH 6/8] soundwire: intel: add helpers for link power down and shim wake Pierre-Louis Bossart
2020-02-27 22:32 ` [PATCH 7/8] soundwire: intel: add wake interrupt support Pierre-Louis Bossart
2020-02-27 22:32 ` [PATCH 8/8] soundwire: intel_init: save Slave(s) _ADR info in sdw_intel_ctx Pierre-Louis Bossart
2020-02-28 7:32 ` [PATCH 0/8] soundwire: remove platform devices, add SOF interfaces Greg KH
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=20200304162837.GA1763256@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=hui.wang@canonical.com \
--cc=jank@cadence.com \
--cc=linux-kernel@vger.kernel.org \
--cc=pierre-louis.bossart@linux.intel.com \
--cc=rander.wang@linux.intel.com \
--cc=ranjani.sridharan@linux.intel.com \
--cc=sanyog.r.kale@intel.com \
--cc=slawomir.blauciak@intel.com \
--cc=srinivas.kandagatla@linaro.org \
--cc=tiwai@suse.de \
--cc=vkoul@kernel.org \
--cc=yung-chuan.liao@linux.intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox