| Message ID | 20171205181448.18513-2-afd@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 05, 2017 at 12:14:46PM -0600, Andrew F. Davis wrote: > Platform ASoC drivers are a lot like ASoC CODEC drivers in that they > both are independent pieces of a sound device, or "machine". Platform > drivers should be free of CODEC specifics and visa-versa. Both are then > used by the the "machine" driver to form the complete sound device. > This forms a hierarchy that makes it natural to group platform drivers > into their own directory, much like we group CODEC drivers already. This seems like a step backwards, and your current patch set only does this for the TI drivers anyway. Currently we have things split up by IP holder, with machine drivers that have some platform specifics grouped together with the matching IP drivers that may also share some platform specific considerations. This would group all the IP drivers together and separate them from the machine drivers they work in concert with for no obvious benefit. If you want to make a common directory for TI stuff do that, there's no need to mess up all the other platforms to do that though.
On 12/06/2017 06:39 AM, Mark Brown wrote: > On Tue, Dec 05, 2017 at 12:14:46PM -0600, Andrew F. Davis wrote: >> Platform ASoC drivers are a lot like ASoC CODEC drivers in that they >> both are independent pieces of a sound device, or "machine". Platform >> drivers should be free of CODEC specifics and visa-versa. Both are then >> used by the the "machine" driver to form the complete sound device. >> This forms a hierarchy that makes it natural to group platform drivers >> into their own directory, much like we group CODEC drivers already. > > This seems like a step backwards, and your current patch set only does > this for the TI drivers anyway. Currently we have things split up by IP > holder, with machine drivers that have some platform specifics grouped > together with the matching IP drivers that may also share some platform > specific considerations. This would group all the IP drivers together > and separate them from the machine drivers they work in concert with for > no obvious benefit. > My current patches are an RFC with a couple examples, so I only moved two TI platforms that I was familiar with, others would follow. Your wording seems a bit inconsistent to me, what do you mean by "IP drivers", CODEC or SoC internal IP? For clarity I'll try to use only the three driver type labels: codec, platform, and machine. This is all in Documentation/sound/soc/overview.rst which I'm sure you are familiar with as you seem to have had a hand in writing it. Anyway, I'm working under the assumption that we should try to enforce a logical separation between component drivers: codec drivers should be agnostic to what machine they are placed, platform drivers should do the same and not make special arrangements to work with one machine in particular. Machine drivers on the other hand will need to dig into specifics of the codec and platform drivers that they use and connect. With this in mind I do not see any reason not to have platform drivers in a platforms/ directory just like we do with codecs/. In case there was any confusion, I still want to keep the platform drivers' files all grouped in directories by IP holder, just moved under this platforms/. This has the benefit of reducing exactly what you are talking about, platform drivers working in concert with machine drivers, instead of the other way around. This isn't only confusing to me, but other first time ASoC devs: https://stackoverflow.com/questions/20110801/asoc-drivers-which-files-are-platform-machine-and-codec-drivers even the answerer seems to assume there is a sound/soc/platforms/, for the same reason we have sound/soc/codecs/. > If you want to make a common directory for TI stuff do that, there's no > need to mess up all the other platforms to do that though. > Do you mean sounds/soc/ti/{platforms,machines}/ ? I could do this, but I don't see how what I've done in my example patches has any effect on other IP holders, they are free to migrate as the please.
On Wed, Dec 06, 2017 at 10:06:34AM -0600, Andrew F. Davis wrote: > Your wording seems a bit inconsistent to me, what do you mean by "IP > drivers", CODEC or SoC internal IP? For clarity I'll try to use only the > three driver type labels: codec, platform, and machine. This is all in > Documentation/sound/soc/overview.rst which I'm sure you are familiar > with as you seem to have had a hand in writing it. IPs in SoCs. > Anyway, I'm working under the assumption that we should try to enforce a > logical separation between component drivers: codec drivers should be > agnostic to what machine they are placed, platform drivers should do the > same and not make special arrangements to work with one machine in > particular. Machine drivers on the other hand will need to dig into > specifics of the codec and platform drivers that they use and connect. Machine and drivers for SoC internal stuff tend to be bound fairly closely together, simiarly the various drivers for an IP on a SoC often know things about each other for various reasons. > With this in mind I do not see any reason not to have platform drivers > in a platforms/ directory just like we do with codecs/. In case there > was any confusion, I still want to keep the platform drivers' files all > grouped in directories by IP holder, just moved under this platforms/. Moving everything around is at the very least going to be a pain for anyone doing backports and anyone actively working on patches, splitting the machine drivers from the rest of the drivers for systems based on that SoC means it's going to be a little harder for people to find relevant system specific machine drivers. Generic machine drivers are already split out. Any benefits seem very weak here and it's an awfully disruptive change. > This has the benefit of reducing exactly what you are talking about, > platform drivers working in concert with machine drivers, instead of the > other way around. What I am saying is that they go together very closely. Moving the code around isn't going to change that. > This isn't only confusing to me, but other first time ASoC devs: > https://stackoverflow.com/questions/20110801/asoc-drivers-which-files-are-platform-machine-and-codec-drivers I rather suspect the confusion they're having is more to do with the fact that the documentation isn't very good than it is to do with where the files are. > even the answerer seems to assume there is a > sound/soc/platforms/, for the same reason we have sound/soc/codecs/. Or possibly just because they're not very familiar with what they're talking about here. > > If you want to make a common directory for TI stuff do that, there's no > > need to mess up all the other platforms to do that though. > Do you mean sounds/soc/ti/{platforms,machines}/ ? I could do this, but I > don't see how what I've done in my example patches has any effect on > other IP holders, they are free to migrate as the please. Oh, right. Your commit message sounded like you wanted to dump everything into a single directory for all SoC side drivers which doesn't seem like an obviously useful thing, my best guess had been that you were trying to get all the TI drivers into one directory. I don't see a pressing need to do that, but I can see it might potentially be causing issues for people. If we were going to do this reshuffling then we *really* shouldn't be doing it randomly for only a few vendors. Doing it inconsistently is not going to make anything clearer.
On 12/06/2017 11:29 AM, Mark Brown wrote: > On Wed, Dec 06, 2017 at 10:06:34AM -0600, Andrew F. Davis wrote: > >> Your wording seems a bit inconsistent to me, what do you mean by "IP >> drivers", CODEC or SoC internal IP? For clarity I'll try to use only the >> three driver type labels: codec, platform, and machine. This is all in >> Documentation/sound/soc/overview.rst which I'm sure you are familiar >> with as you seem to have had a hand in writing it. > > IPs in SoCs. > >> Anyway, I'm working under the assumption that we should try to enforce a >> logical separation between component drivers: codec drivers should be >> agnostic to what machine they are placed, platform drivers should do the >> same and not make special arrangements to work with one machine in >> particular. Machine drivers on the other hand will need to dig into >> specifics of the codec and platform drivers that they use and connect. > > Machine and drivers for SoC internal stuff tend to be bound fairly > closely together, simiarly the various drivers for an IP on a SoC often > know things about each other for various reasons. > This is the problem, we don't want them to be so tightly bound, and luckily, for the most part they are not. Even a complex and history rich platform like OMAP was rather trivial to split from its various machine drivers. >> With this in mind I do not see any reason not to have platform drivers >> in a platforms/ directory just like we do with codecs/. In case there >> was any confusion, I still want to keep the platform drivers' files all >> grouped in directories by IP holder, just moved under this platforms/. > > Moving everything around is at the very least going to be a pain for > anyone doing backports and anyone actively working on patches, splitting > the machine drivers from the rest of the drivers for systems based on > that SoC means it's going to be a little harder for people to find > relevant system specific machine drivers. Generic machine drivers are > already split out. > > Any benefits seem very weak here and it's an awfully disruptive change. > While it may still be a small pain for people currently building patches against the old structure, for back-ports, git seems to handle these renames well and follows the file patches back to the old location. >> This has the benefit of reducing exactly what you are talking about, >> platform drivers working in concert with machine drivers, instead of the >> other way around. > > What I am saying is that they go together very closely. Moving the code > around isn't going to change that. > Not at first, but this partition will discourage future machine-platform mash-ups (like omap-hdmi-audio.c, yuck). My end-goal here is to start trimming the needed machine drivers and replacing them with generics, a couple OMAP machine drivers do nothing that couldn't be done with the "asoc-simple-card" driver. With the machine drivers split out form the platform drivers it becomes easier to target them. >> This isn't only confusing to me, but other first time ASoC devs: >> https://stackoverflow.com/questions/20110801/asoc-drivers-which-files-are-platform-machine-and-codec-drivers > > I rather suspect the confusion they're having is more to do with the > fact that the documentation isn't very good than it is to do with where > the files are. > The documentation wont have to explain why platform and machine drivers are mashed together in top-level directories if we fix this here :) >> even the answerer seems to assume there is a >> sound/soc/platforms/, for the same reason we have sound/soc/codecs/. > > Or possibly just because they're not very familiar with what they're > talking about here. > That does seems to be the case, but the assumption was still that a partitioning should exist. >>> If you want to make a common directory for TI stuff do that, there's no >>> need to mess up all the other platforms to do that though. > >> Do you mean sounds/soc/ti/{platforms,machines}/ ? I could do this, but I >> don't see how what I've done in my example patches has any effect on >> other IP holders, they are free to migrate as the please. > > Oh, right. Your commit message sounded like you wanted to dump > everything into a single directory for all SoC side drivers which > doesn't seem like an obviously useful thing, my best guess had been that > you were trying to get all the TI drivers into one directory. I don't > see a pressing need to do that, but I can see it might potentially be > causing issues for people. > I don't have any need to group the TI platforms (Davinci / OMAP) right now, but I *have* been thinking about grouping the TI CODECs, they share a lot of code that could be factored out if they were stored in their own space sound/soc/codecs/ti/. Plus it would make it easy to add myself as a reviewer for them (I seem to be getting a lot of internal support requests for these drivers these days). That can be a re-org for another day, unless you would like me to post an RFC with what I had in mind? > If we were going to do this reshuffling then we *really* shouldn't be > doing it randomly for only a few vendors. Doing it inconsistently is > not going to make anything clearer. > I can send patches for rest of the vendors if you would like to see that and what the end result would look like.
On Wed, Dec 06, 2017 at 12:13:22PM -0600, Andrew F. Davis wrote: > On 12/06/2017 11:29 AM, Mark Brown wrote: > > Machine and drivers for SoC internal stuff tend to be bound fairly > > closely together, simiarly the various drivers for an IP on a SoC often > > know things about each other for various reasons. > This is the problem, we don't want them to be so tightly bound, and > luckily, for the most part they are not. Even a complex and history rich > platform like OMAP was rather trivial to split from its various machine > drivers. Anything new that can is already getting pushed into using the existing generic cards. New machine drivers are only for things where that's not possible. > > What I am saying is that they go together very closely. Moving the code > > around isn't going to change that. > Not at first, but this partition will discourage future machine-platform > mash-ups (like omap-hdmi-audio.c, yuck). It's not a pressing problem. > My end-goal here is to start trimming the needed machine drivers and > replacing them with generics, a couple OMAP machine drivers do nothing > that couldn't be done with the "asoc-simple-card" driver. With the > machine drivers split out form the platform drivers it becomes easier to > target them. We need to preserve old bindings to ensure DT compatiblity, the easiest way to do that is to keep old machine drivers around. There are plenty of older drivers that wouldn't be accepted now but would at least need replacing with a compatibility layer that adapts the bindings onto one of the generic drivers. That adaption layer would definitely be useful (basically a big table of platform data) but it'd take time to implement it. > I don't have any need to group the TI platforms (Davinci / OMAP) right > now, but I *have* been thinking about grouping the TI CODECs, they share > a lot of code that could be factored out if they were stored in their > own space sound/soc/codecs/ti/. Plus it would make it easy to add myself You can share code easily enough without moving anything, just make a library like the arizona drivers did. > as a reviewer for them (I seem to be getting a lot of internal support > requests for these drivers these days). That can be a re-org for another > day, unless you would like me to post an RFC with what I had in mind? Wouldn't a few regexps in the MAINTAINERS file cover it? We've already got a bunch of vendors doing this. > > If we were going to do this reshuffling then we *really* shouldn't be > > doing it randomly for only a few vendors. Doing it inconsistently is > > not going to make anything clearer. > I can send patches for rest of the vendors if you would like to see that > and what the end result would look like. I'm not convinced this is a good idea.
On 12/06/2017 12:42 PM, Mark Brown wrote: > On Wed, Dec 06, 2017 at 12:13:22PM -0600, Andrew F. Davis wrote: >> On 12/06/2017 11:29 AM, Mark Brown wrote: > >>> Machine and drivers for SoC internal stuff tend to be bound fairly >>> closely together, simiarly the various drivers for an IP on a SoC often >>> know things about each other for various reasons. > >> This is the problem, we don't want them to be so tightly bound, and >> luckily, for the most part they are not. Even a complex and history rich >> platform like OMAP was rather trivial to split from its various machine >> drivers. > > Anything new that can is already getting pushed into using the existing > generic cards. New machine drivers are only for things where that's not > possible. > >>> What I am saying is that they go together very closely. Moving the code >>> around isn't going to change that. > >> Not at first, but this partition will discourage future machine-platform >> mash-ups (like omap-hdmi-audio.c, yuck). > > It's not a pressing problem. > >> My end-goal here is to start trimming the needed machine drivers and >> replacing them with generics, a couple OMAP machine drivers do nothing >> that couldn't be done with the "asoc-simple-card" driver. With the >> machine drivers split out form the platform drivers it becomes easier to >> target them. > > We need to preserve old bindings to ensure DT compatiblity, the easiest > way to do that is to keep old machine drivers around. There are plenty > of older drivers that wouldn't be accepted now but would at least need > replacing with a compatibility layer that adapts the bindings onto one > of the generic drivers. That adaption layer would definitely be useful > (basically a big table of platform data) but it'd take time to implement > it. > We then should at least start depreciating them now so that someday we can drop that stuff. Isolating them would be the first step. >> I don't have any need to group the TI platforms (Davinci / OMAP) right >> now, but I *have* been thinking about grouping the TI CODECs, they share >> a lot of code that could be factored out if they were stored in their >> own space sound/soc/codecs/ti/. Plus it would make it easy to add myself > > You can share code easily enough without moving anything, just make a > library like the arizona drivers did. > The lack of organization bugs me, this is why directories exist. >> as a reviewer for them (I seem to be getting a lot of internal support >> requests for these drivers these days). That can be a re-org for another >> day, unless you would like me to post an RFC with what I had in mind? > > Wouldn't a few regexps in the MAINTAINERS file cover it? We've already > got a bunch of vendors doing this. > pcm* tas* tlv* twl* It's messy how many prefixes we have :/ >>> If we were going to do this reshuffling then we *really* shouldn't be >>> doing it randomly for only a few vendors. Doing it inconsistently is >>> not going to make anything clearer. > >> I can send patches for rest of the vendors if you would like to see that >> and what the end result would look like. > > I'm not convinced this is a good idea. >
On Wed, Dec 06, 2017 at 12:49:39PM -0600, Andrew F. Davis wrote: > On 12/06/2017 12:42 PM, Mark Brown wrote: > > We need to preserve old bindings to ensure DT compatiblity, the easiest > > way to do that is to keep old machine drivers around. There are plenty > > of older drivers that wouldn't be accepted now but would at least need > > replacing with a compatibility layer that adapts the bindings onto one > > of the generic drivers. That adaption layer would definitely be useful > > (basically a big table of platform data) but it'd take time to implement > > it. > We then should at least start depreciating them now so that someday we > can drop that stuff. Isolating them would be the first step. Moving the drivers around is not going to help with that. For users the drivers are not deprecated until someone actually steps up and makes something that allows the generic drivers to handle the old bindings and moves them over, at which point we'd just remove things that have been converted. We can't just tell them not to use something without providing an alternative. For developers they're just going to end up with the simpler machine drivers sitting next to a bunch of machine drivers that reasonably exist which I'm not sure clarifies anything. It's an orthogonal problem. > > Wouldn't a few regexps in the MAINTAINERS file cover it? We've already > > got a bunch of vendors doing this. > pcm* > tas* > tlv* > twl* > It's messy how many prefixes we have :/ Eh, not that bad. And easy enough to do anyway.
On 12/06/2017 01:27 PM, Mark Brown wrote: > On Wed, Dec 06, 2017 at 12:49:39PM -0600, Andrew F. Davis wrote: >> On 12/06/2017 12:42 PM, Mark Brown wrote: > >>> We need to preserve old bindings to ensure DT compatiblity, the easiest >>> way to do that is to keep old machine drivers around. There are plenty >>> of older drivers that wouldn't be accepted now but would at least need >>> replacing with a compatibility layer that adapts the bindings onto one >>> of the generic drivers. That adaption layer would definitely be useful >>> (basically a big table of platform data) but it'd take time to implement >>> it. > >> We then should at least start depreciating them now so that someday we >> can drop that stuff. Isolating them would be the first step. > > Moving the drivers around is not going to help with that. For users the > drivers are not deprecated until someone actually steps up and makes > something that allows the generic drivers to handle the old bindings and > moves them over, at which point we'd just remove things that have been > converted. We can't just tell them not to use something without > providing an alternative. For developers they're just going to end up > with the simpler machine drivers sitting next to a bunch of machine > drivers that reasonably exist which I'm not sure clarifies anything. > It's an orthogonal problem. > It helps in that people would see an organized set of drivers to target, "this folder here (sound/soc/machines/) should be reduced as much as possible". Otherwise it is just, "go dig around for the machine drivers burred together with platform drivers, which file is which type? Who knows!?" I will help too, but there needs to be just a bit of organization first. >>> Wouldn't a few regexps in the MAINTAINERS file cover it? We've already >>> got a bunch of vendors doing this. > >> pcm* >> tas* >> tlv* >> twl* > >> It's messy how many prefixes we have :/ > > Eh, not that bad. And easy enough to do anyway. > And it is easy enough to group them into a directory by something like vendor, same as everyone else, including your very own sound/soc/. Yet codecs/ is a giant 400+ file blob, by C file count it is the largest directory in the whole kernel... Top 10: C files count: folder 130: drivers/staging/comedi/drivers 145: drivers/gpio 159: drivers/watchdog 164: drivers/rtc 165: drivers/hwmon 172: net/netfilter 175: drivers/acpi/acpica 194: lib 201: drivers/mfd 239: sound/soc/codecs Maybe a bit of grouping wouldn't be so bad
diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig index d22758165496..b8064e8b2bdf 100644 --- a/sound/soc/Kconfig +++ b/sound/soc/Kconfig @@ -75,6 +75,9 @@ source "sound/soc/ux500/Kconfig" source "sound/soc/xtensa/Kconfig" source "sound/soc/zte/Kconfig" +# Supported platforms +source "sound/soc/platforms/Kconfig" + # Supported codecs source "sound/soc/codecs/Kconfig" diff --git a/sound/soc/Makefile b/sound/soc/Makefile index 5327f4d6c668..af578273548c 100644 --- a/sound/soc/Makefile +++ b/sound/soc/Makefile @@ -24,6 +24,7 @@ obj-$(CONFIG_SND_SOC_ACPI) += snd-soc-acpi.o obj-$(CONFIG_SND_SOC) += snd-soc-core.o obj-$(CONFIG_SND_SOC) += codecs/ obj-$(CONFIG_SND_SOC) += generic/ +obj-$(CONFIG_SND_SOC) += platforms/ obj-$(CONFIG_SND_SOC) += adi/ obj-$(CONFIG_SND_SOC) += amd/ obj-$(CONFIG_SND_SOC) += atmel/ diff --git a/sound/soc/platforms/Kconfig b/sound/soc/platforms/Kconfig new file mode 100644 index 000000000000..620d1f292246 --- /dev/null +++ b/sound/soc/platforms/Kconfig @@ -0,0 +1,7 @@ +# +# SoC Platform Support Configuration +# + +menu "Platform drivers" + +endmenu diff --git a/sound/soc/platforms/Makefile b/sound/soc/platforms/Makefile new file mode 100644 index 000000000000..f66554cd5c45 --- /dev/null +++ b/sound/soc/platforms/Makefile @@ -0,0 +1 @@ +# SPDX-License-Identifier: GPL-2.0
Platform ASoC drivers are a lot like ASoC CODEC drivers in that they both are independent pieces of a sound device, or "machine". Platform drivers should be free of CODEC specifics and visa-versa. Both are then used by the the "machine" driver to form the complete sound device. This forms a hierarchy that makes it natural to group platform drivers into their own directory, much like we group CODEC drivers already. With this change, we will leave machine drivers in the top-level ASoC directory and have CODECs and Platforms below. The machine drivers may also be moved at some point into a grouping directory to further enforce the logical separation intended by the ASoC framework. Create the initial directory, Kconfig, and Makefile here. Signed-off-by: Andrew F. Davis <afd@ti.com> --- sound/soc/Kconfig | 3 +++ sound/soc/Makefile | 1 + sound/soc/platforms/Kconfig | 7 +++++++ sound/soc/platforms/Makefile | 1 + 4 files changed, 12 insertions(+) create mode 100644 sound/soc/platforms/Kconfig create mode 100644 sound/soc/platforms/Makefile