| Message ID | cover.1591802243.git.vaibhav.sr@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Enable Greybus Audio codec driver | expand |
On Wed, Jun 10, 2020 at 10:58:24PM +0530, Vaibhav Agarwal wrote: > The existing GB Audio codec driver is dependent on MSM8994 Audio driver. > During the development stage, this dependency was configured due to > various changes involved in MSM Audio driver to enable addtional codec > card and some of the changes proposed in mainline ASoC framework. I'm not sure why you're copying me on a staging driver? I don't recall the base driver having been submitted properly yet.
I love this patchset so much more... Thanks!
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
regards,
dan carpenter
On 10/06/2020 18:37:11+0100, Mark Brown wrote: > On Wed, Jun 10, 2020 at 10:58:24PM +0530, Vaibhav Agarwal wrote: > > The existing GB Audio codec driver is dependent on MSM8994 Audio driver. > > During the development stage, this dependency was configured due to > > various changes involved in MSM Audio driver to enable addtional codec > > card and some of the changes proposed in mainline ASoC framework. > > I'm not sure why you're copying me on a staging driver? I don't recall > the base driver having been submitted properly yet. That was my suggestion, the whole history is that I submitted a patch removing this driver as it was not getting compiled and so didn't go through the componentization. See https://lore.kernel.org/lkml/20200507212912.599433-1-alexandre.belloni@bootlin.com/ My point was that if we were to keep that driver, the goal would be to have it out of staging instead of simply making it compile.
On Wed, Jun 10, 2020 at 06:37:11PM +0100, Mark Brown wrote: > On Wed, Jun 10, 2020 at 10:58:24PM +0530, Vaibhav Agarwal wrote: > > The existing GB Audio codec driver is dependent on MSM8994 Audio driver. > > During the development stage, this dependency was configured due to > > various changes involved in MSM Audio driver to enable addtional codec > > card and some of the changes proposed in mainline ASoC framework. > > I'm not sure why you're copying me on a staging driver? I don't recall > the base driver having been submitted properly yet. Hi Mark, With patch#6 in this series, I'm proposing some of the (dummy) helper APIs required to link DAPM DAI widgets for the GB Audio modules added/removed dynamically. Eventually, I would like to propose relevant changes in snd-soc APIs to enable dynamic linking of DAI widgets for the modules added and remove/free component controls for the module removed. I'm seeking your opinion on the proposed changes. And as per the recommendation I'm sharing the changes with ASoC mailing list as well. Kindly suggest me the preferred way to follow on this thread. -- Regards, Vaibhav
On Wed, Jun 10, 2020 at 11:53:24PM +0530, Vaibhav Agarwal wrote: > With patch#6 in this series, I'm proposing some of the (dummy) helper > APIs required to link DAPM DAI widgets for the GB Audio modules > added/removed dynamically. > Eventually, I would like to propose relevant changes in snd-soc APIs to > enable dynamic linking of DAI widgets for the modules added and > remove/free component controls for the module removed. > I'm seeking your opinion on the proposed changes. And as per the > recommendation I'm sharing the changes with ASoC mailing list as well. These are proposed incremental changes to an out of tree driver that has never been submitted. I don't know what the current code looks like, what it's supposed to be doing or anything like that so I've no idea what's going on or why. > Kindly suggest me the preferred way to follow on this thread. This is effectively out of tree code, until someone submits it properly I'm not sure it's useful to submit incremental patches upstream.
On Wed, Jun 10, 2020 at 08:01:18PM +0200, Alexandre Belloni wrote: > My point was that if we were to keep that driver, the goal would be to > have it out of staging instead of simply making it compile. Yes, definitely - that should be the goal for anything in staging.
On Thu, Jun 11, 2020 at 09:26:16AM +0100, Mark Brown wrote: > On Wed, Jun 10, 2020 at 11:53:24PM +0530, Vaibhav Agarwal wrote: > > > With patch#6 in this series, I'm proposing some of the (dummy) helper > > APIs required to link DAPM DAI widgets for the GB Audio modules > > added/removed dynamically. > > > Eventually, I would like to propose relevant changes in snd-soc APIs to > > enable dynamic linking of DAI widgets for the modules added and > > remove/free component controls for the module removed. > > > I'm seeking your opinion on the proposed changes. And as per the > > recommendation I'm sharing the changes with ASoC mailing list as well. > > These are proposed incremental changes to an out of tree driver that has > never been submitted. I don't know what the current code looks like, > what it's supposed to be doing or anything like that so I've no idea > what's going on or why. > > > Kindly suggest me the preferred way to follow on this thread. > > This is effectively out of tree code, until someone submits it properly > I'm not sure it's useful to submit incremental patches upstream. Thanks for the suggestion Mark. I'll create a separate patchset aligned to the ASoC tree in next ~2 weeks and share them separately. Hi Greg, Do you think the current patchset can be considered for the purpose of resolving componentization issue raised by Alexandre? -- Regards, Vaibhav
On Fri, Jun 12, 2020 at 09:07:24PM +0530, Vaibhav Agarwal wrote: > On Thu, Jun 11, 2020 at 09:26:16AM +0100, Mark Brown wrote: > > > Kindly suggest me the preferred way to follow on this thread. > > This is effectively out of tree code, until someone submits it properly > > I'm not sure it's useful to submit incremental patches upstream. > Thanks for the suggestion Mark. I'll create a separate patchset aligned > to the ASoC tree in next ~2 weeks and share them separately. Great. If there's controversial/complicated design bits to sort out it would probably be good to split them out so the simple stuff can go through more easily.
The existing GB Audio codec driver is dependent on MSM8994 Audio driver. During the development stage, this dependency was configured due to various changes involved in MSM Audio driver to enable addtional codec card and some of the changes proposed in mainline ASoC framework. However, these are not the real dependencies and some of them can be easily removed. The folowing patch series includes the changes to resolve unnecessary depedencies and make the codec driver functional with the latest kernel. Patch 1,2: Incudes jack framework related changes. Patch 3,4,5: Resolves compilation error observed with the latest kernel and also provides helper APIs required to allow dynamic addition/removal of modules. Patch 6: Finally provides config options and related Makefile changes to enable GB Codec driver. Thanks to Alexandre for raising the headsup [1] and motivating me to provide the necessary changes. [1] https://lore.kernel.org/lkml/20200507212912.599433-1-alexandre.belloni@bootlin.com/ Changes from v1 - Include the changes for the review comments suggested by Dan - Rebase to latest staging-next Vaibhav Agarwal (6): staging: greybus: audio: Update snd_jack FW usage as per new APIs staging: greybus: audio: Maintain jack list within GB Audio module staging: greybus: audio: Resolve compilation errors for GB codec module staging: greybus: audio: Resolve compilation error in topology parser staging: greybus: audio: Add helper APIs for dynamic audio modules staging: greybus: audio: Enable GB codec, audio module compilation. drivers/staging/greybus/Kconfig | 14 +- drivers/staging/greybus/Makefile | 6 +- drivers/staging/greybus/audio_codec.c | 178 +++++++++++--------- drivers/staging/greybus/audio_codec.h | 12 +- drivers/staging/greybus/audio_helper.c | 197 +++++++++++++++++++++++ drivers/staging/greybus/audio_helper.h | 17 ++ drivers/staging/greybus/audio_module.c | 15 +- drivers/staging/greybus/audio_topology.c | 128 +++++++-------- 8 files changed, 412 insertions(+), 155 deletions(-) create mode 100644 drivers/staging/greybus/audio_helper.c create mode 100644 drivers/staging/greybus/audio_helper.h base-commit: af7b4801030c07637840191c69eb666917e4135d