[v2,0/6] Enable Greybus Audio codec driver
mbox series

Message ID cover.1591802243.git.vaibhav.sr@gmail.com
Headers show
Series
  • Enable Greybus Audio codec driver
Related show

Message

Vaibhav Agarwal June 10, 2020, 5:28 p.m. UTC
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

Comments

Mark Brown June 10, 2020, 5:37 p.m. UTC | #1
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.
Dan Carpenter June 10, 2020, 5:51 p.m. UTC | #2
I love this patchset so much more...  Thanks!

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter
Alexandre Belloni June 10, 2020, 6:01 p.m. UTC | #3
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.
Vaibhav Agarwal June 10, 2020, 6:23 p.m. UTC | #4
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
Mark Brown June 11, 2020, 8:26 a.m. UTC | #5
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.
Mark Brown June 11, 2020, 8:41 a.m. UTC | #6
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.
Vaibhav Agarwal June 12, 2020, 3:37 p.m. UTC | #7
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
Mark Brown June 12, 2020, 4:06 p.m. UTC | #8
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.