2020-06-10 17:36:44

by Vaibhav Agarwal

[permalink] [raw]
Subject: [PATCH v2 0/6] Enable Greybus Audio codec driver

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/[email protected]/

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
--
2.26.2


2020-06-10 17:39:30

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Enable Greybus Audio codec driver

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.


Attachments:
(No filename) (484.00 B)
signature.asc (499.00 B)
Download all attachments

2020-06-10 20:18:08

by Alexandre Belloni

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Enable Greybus Audio codec driver

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/[email protected]/

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.

--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2020-06-10 20:18:09

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Enable Greybus Audio codec driver

I love this patchset so much more... Thanks!

Reviewed-by: Dan Carpenter <[email protected]>

regards,
dan carpenter

2020-06-10 21:10:39

by Vaibhav Agarwal

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Enable Greybus Audio codec driver

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

2020-06-11 08:31:24

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Enable Greybus Audio codec driver

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.


Attachments:
(No filename) (1.02 kB)
signature.asc (499.00 B)
Download all attachments

2020-06-11 08:45:56

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Enable Greybus Audio codec driver

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.


Attachments:
(No filename) (277.00 B)
signature.asc (499.00 B)
Download all attachments

2020-06-12 15:42:04

by Vaibhav Agarwal

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Enable Greybus Audio codec driver

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

2020-06-12 16:08:36

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] Enable Greybus Audio codec driver

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.


Attachments:
(No filename) (658.00 B)
signature.asc (499.00 B)
Download all attachments