From: Greg KH <gregkh@linuxfoundation.org>
To: Vinod Koul <vkoul@kernel.org>
Cc: Bard Liao <yung-chuan.liao@linux.intel.com>,
alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
tiwai@suse.de, broonie@kernel.org, jank@cadence.com,
srinivas.kandagatla@linaro.org, rander.wang@linux.intel.com,
ranjani.sridharan@linux.intel.com, hui.wang@canonical.com,
pierre-louis.bossart@linux.intel.com, sanyog.r.kale@intel.com,
slawomir.blauciak@intel.com, mengdong.lin@intel.com
Subject: Re: [RFC 1/5] soundwire: bus_type: add sdw_master_device support
Date: Thu, 23 Apr 2020 16:24:51 +0200 [thread overview]
Message-ID: <20200423142451.GA4181720@kroah.com> (raw)
In-Reply-To: <20200420072631.GW72691@vkoul-mobl>
On Mon, Apr 20, 2020 at 12:56:31PM +0530, Vinod Koul wrote:
> Hello Bard,
>
> On 17-04-20, 04:55, Bard Liao wrote:
> > From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> >
> > In the existing SoundWire code, Master Devices are not explicitly
> > represented - only SoundWire Slave Devices are exposed (the use of
> > capital letters follows the SoundWire specification conventions).
> >
> > The SoundWire Master Device provides the clock, synchronization
> > information and command/control channels. When multiple links are
> > supported, a Controller may expose more than one Master Device; they
> > are typically embedded inside a larger audio cluster (be it in an
> > SOC/chipset or an external audio codec), and we need to describe it
> > using the Linux device and driver model. This will allow for
> > configuration functions to account for external dependencies such as
> > power rails, clock sources or wake-up mechanisms. This transition will
> > also allow for better sysfs support without the reference count issues
> > mentioned in the initial reviews.
>
> Well the primary reason for doing sdw_master_device for creating a
> adding sysfs representation.
-ENOPARSE :(
> It *also* helps some vendors due to
> inherent model should not be constructed as the primary approach for the
> sdw_master_device.
No, the PRIMARY reason is "it is the correct thing to do". It's how to
tie into the driver model correctly, without it, crazy things happen as
we have seen.
> > In this patch, we convert the existing code to use an explicit
> > sdw_slave_type, then define a sdw_master_device structure.
>
> Please split that up, we should do the conversions required first and
> then do addition of new things.
Can you really do that in two different steps?
> > +struct device_type sdw_master_type = {
> > + .name = "soundwire_master",
> > + .release = sdw_master_device_release,
> > +};
> > +
> > +/**
> > + * sdw_master_device_add() - create a Linux Master Device representation.
> > + * @parent: the parent Linux device (e.g. a PCI device)
> > + * @fwnode: the parent fwnode (e.g. an ACPI companion device to the parent)
> > + * @link_ops: link-specific ops (optional)
> > + * @link_id: link index as defined by MIPI DisCo specification
> > + * @pdata: private data (e.g. register base, offsets, platform quirks, etc).
> > + *
> > + * The link_ops argument can be NULL, it is only used when link-specific
> > + * initializations and power-management are required.
> > + */
> > +struct sdw_master_device
> > +*sdw_master_device_add(struct device *parent,
> > + struct fwnode_handle *fwnode,
> > + struct sdw_link_ops *link_ops,
> > + int link_id,
> > + void *pdata)
> > +{
> > + struct sdw_master_device *md;
> > + int ret;
> > +
> > + md = kzalloc(sizeof(*md), GFP_KERNEL);
> > + if (!md)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + md->link_id = link_id;
> > + md->pdata = pdata;
> > + md->link_ops = link_ops;
> > +
> > + md->dev.parent = parent;
> > + md->dev.fwnode = fwnode;
> > + md->dev.bus = &sdw_bus_type;
> > + md->dev.type = &sdw_master_type;
> > + md->dev.dma_mask = md->dev.parent->dma_mask;
> > + dev_set_name(&md->dev, "sdw-master-%d", md->link_id);
> > +
> > + if (link_ops && link_ops->driver) {
> > + /*
> > + * A driver is only needed for ASoC integration (need
> > + * driver->name) and for link-specific power management
> > + * w/ a pm_dev_ops structure.
>
> That is not true for everyone, it is only true for Intel, pls call that
> out as well...
Why is it not true for everyone? How else do you get the pm stuff back
to your hardware?
thanks,
greg k-h
next prev parent reply other threads:[~2020-04-23 14:24 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-16 20:55 [RFC 0/5] soundwire: create master device and use it Bard Liao
2020-04-16 20:55 ` [RFC 1/5] soundwire: bus_type: add sdw_master_device support Bard Liao
2020-04-20 7:26 ` Vinod Koul
2020-04-23 14:24 ` Greg KH [this message]
2020-04-28 4:31 ` Vinod Koul
2020-04-28 6:37 ` Greg KH
2020-04-28 6:49 ` Vinod Koul
2020-04-28 6:55 ` Greg KH
2020-04-28 7:51 ` Vinod Koul
[not found] ` <4ecfa01e-4ef4-5368-3a70-2bd57407d2ad@linux.intel.com>
2020-04-30 4:57 ` Vinod Koul
2020-04-16 20:55 ` [RFC 2/5] soundwire: master: use device node pointer from master device Bard Liao
2020-04-16 20:55 ` [RFC 3/5] soundwire: qcom: fix error handling in probe Bard Liao
2020-04-20 7:42 ` Srinivas Kandagatla
2020-04-16 20:55 ` [RFC 4/5] soundwire: qcom: add sdw_master_device support Bard Liao
2020-04-16 20:55 ` [RFC 5/5] soundwire: intel: transition to sdw_master_device Bard Liao
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=20200423142451.GA4181720@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=mengdong.lin@intel.com \
--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;
as well as URLs for NNTP newsgroup(s).