Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Friday 09 September 2011, Russell King - ARM Linux wrote: That's just twisted and utterly insane - adding more code for precisely zero benefit what so ever. Think about it - the device tree is already creating platform devices for entries in the device tree file. What's the point of having this special ASoC code look up the platform compatible property in a table of strings to find a different string to manually create a device with. Why not just add the bloody device to the DT file in the first place? That's partly what DT is there for - to create platform specific struct devices. Exactly. No driver or (worse) user program should ever need to look at the top-level compatible property. When you want information about a localized part of the system (e.g. the ASoC components), you should look up the information for that component. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Sat, Sep 10, 2011 at 10:37:15PM +0200, Arnd Bergmann wrote: On Friday 09 September 2011, Russell King - ARM Linux wrote: That's just twisted and utterly insane - adding more code for precisely zero benefit what so ever. Think about it - the device tree is already creating platform devices for entries in the device tree file. What's Exactly. No driver or (worse) user program should ever need to look at the top-level compatible property. When you want information about a localized part of the system (e.g. the ASoC components), you should look up the information for that component. Right, and this is the state of the art right now. I can see why people find this inelegant and want to come up with a way of automatically generating all these different devices to describe boards especially with the non-device tree systems, though, so if people want to work on that stuff I don't see a problem with it. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On 9 September 2011 05:29, Mark Brown broo...@opensource.wolfsonmicro.com wrote: Jassi's suggestion was that we should have some magic to automatically generate defaults for the relevant device registrations to sidestep these issues. Perhaps there is some misunderstanding no witchcraft is involved here. To be clear, I suggested moving platform_device definition and registration from 12 board files to some common platform file and use machine_is_xxx() to assign names of those platform devices. Btw, omap_init_audio() is already called from an arch_initcall. Also please note that currently there's no platform_data needed to be passed. If that requirement arise in future, an optional set_asoc_platdata(void *pdata) could be defined beside platform_device creation. While the idea is not absolutely good, imho, it's certainly an improvement over this patch. Or am I overlooking something ? -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Thu, Sep 08, 2011 at 04:59:04PM -0700, Mark Brown wrote: On Fri, Sep 09, 2011 at 12:01:02AM +0100, Russell King - ARM Linux wrote: On Thu, Sep 08, 2011 at 03:47:31PM -0700, Mark Brown wrote: What will happen for device tree is that there will be a device in the device tree for the ASoC board. Sounds like you just solved the machine_is_xxx() problem in ASoC land too there. If you're _already_ going for separate devices to describe the ASoC stuff on the board, then there's no reason that couldn't have already been done to eliminate the machine_is_xxx() usage in ASoC - rather than complaining about machine_is_xxx() not being a very good solution. The problem is that someone has to manually go and add the device to every board that needs one and people find that tedious and slightly inelegant Sheesh. So now you're arguing against your statement above? Please stop wasting my time. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Fri, Sep 09, 2011 at 10:41:56AM +0100, Russell King - ARM Linux wrote: On Thu, Sep 08, 2011 at 04:59:04PM -0700, Mark Brown wrote: The problem is that someone has to manually go and add the device to every board that needs one and people find that tedious and slightly inelegant Sheesh. So now you're arguing against your statement above? Please stop wasting my time. There's two things going on here - what we are doing and what people would like to be able to do. What we are doing is explicitly adding devices, what people would like to do is infer the devices from the board type. Personally I'm totally happy with explicitly adding an audio device, but not everyone is and I do understand where they're coming from. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Fri, Sep 09, 2011 at 09:11:52AM -0700, Mark Brown wrote: On Fri, Sep 09, 2011 at 10:41:56AM +0100, Russell King - ARM Linux wrote: On Thu, Sep 08, 2011 at 04:59:04PM -0700, Mark Brown wrote: The problem is that someone has to manually go and add the device to every board that needs one and people find that tedious and slightly inelegant Sheesh. So now you're arguing against your statement above? Please stop wasting my time. There's two things going on here - what we are doing and what people would like to be able to do. What we are doing is explicitly adding devices, what people would like to do is infer the devices from the board type. Personally I'm totally happy with explicitly adding an audio device, but not everyone is and I do understand where they're coming from. Well, with DT, there won't be any 'board type' anymore. There won't be any 'machine_is_xxx()' to sort it out anymore. Using DT, all that will be history - it's all got to be sorted out by either devices or device properties. So, given that, I don't see the logic of having two methods - it might as well be dealt with by devices and [platform data for non-DT | DT properties], and which then means we have everything working the same way irrespective of what the backing data for the platform actually is. Therefore, as we are heading for DT, I'd definitely say that having machine_is_xxx() outside of arch/arm is a bug, no less and no more. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Fri, Sep 09, 2011 at 08:01:35PM +0100, Russell King - ARM Linux wrote: Well, with DT, there won't be any 'board type' anymore. There won't be any 'machine_is_xxx()' to sort it out anymore. Using DT, all that will be history - it's all got to be sorted out by either devices or device properties. There is a board type - there's a root compatible property for the board which fulfils this purpose - so the situation with and without device tree is essentially the same. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Fri, Sep 09, 2011 at 12:30:11PM -0700, Mark Brown wrote: On Fri, Sep 09, 2011 at 08:01:35PM +0100, Russell King - ARM Linux wrote: Well, with DT, there won't be any 'board type' anymore. There won't be any 'machine_is_xxx()' to sort it out anymore. Using DT, all that will be history - it's all got to be sorted out by either devices or device properties. There is a board type - there's a root compatible property for the board which fulfils this purpose - so the situation with and without device tree is essentially the same. So instead of a table of machine id numbers and soc device strings, you're going to have a table of board 'compatible' strings and soc device strings, and you're going to have to manually create the struct device with that name. That's just twisted and utterly insane - adding more code for precisely zero benefit what so ever. Think about it - the device tree is *already* creating platform devices for entries in the device tree file. What's the point of having this special ASoC code look up the platform compatible property in a table of strings to find a different string to manually create a device with. Why not just _add_ the bloody device to the DT file in the first place? That's partly what DT is there for - to create platform specific struct devices. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Thu, Sep 08, 2011 at 03:29:11PM -0700, Mark Brown wrote: On Thu, 2011-09-08 at 22:28 +0200, Arnd Bergmann wrote: On Thursday 08 September 2011 20:05:48 Mans Rullgard wrote: I had the same thought, but I couldn't find a suitable string anywhere. Are you suggesting an if(machine_is_foo()) cascade in omap_init_audio()? I'll be the first to agree this patch is not particularly pretty. My general feeling is that practically every time someone writes machine_is_*(), they are doing it wrong. There are of course exceptions, but I would strongly recommend to have the initialization calling up from the board file into more general functions instead of having all boards calling the same function which then goes to board specific code again. I have to agree, that seems tasteless. I'd expect something like triggering registration of devices based off walking down a table of machine IDs or something. One other issue to consider here is that we don't want to discourage people from sharing machine drivers while we can so it can't be completely automatic, it This problem has been solved (before DT) for _ages_ through the use of platform devices in the platform support files, registered by the .init_machine callback. Where it went wrong is when ASOC and PCMCIA became rather complicated that you could no longer just pass a platform data structure, but instead needed some complex platform specific code - which started the demand to have mini platform specific chunks of code in drivers/pcmcia/ and sound/. With DT of course, all devices get instantiated from the device tree, so there should not be any more platform specific chunks of code in these locations (ha, it couldn't be solved with platform data so I suspect it will continue to persist, forever unsolved.) -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Thu, Sep 08, 2011 at 03:47:31PM -0700, Mark Brown wrote: On Thu, Sep 08, 2011 at 11:37:20PM +0100, Russell King - ARM Linux wrote: With DT of course, all devices get instantiated from the device tree, so there should not be any more platform specific chunks of code in these locations (ha, it couldn't be solved with platform data so I suspect it will continue to persist, forever unsolved.) That's not the case at all for audio, the PCB schematic for the audio subsystem on a device like a smartphone is a sufficiently interesting piece of hardware to be a device with a driver in its own right. The ASoC machine drivers aren't about instantiating devices, they are about controlling the interrelationships between the various devices in the audio subsystem. What will happen for device tree is that there will be a device in the device tree for the ASoC board. Sounds like you just solved the machine_is_xxx() problem in ASoC land too there. If you're _already_ going for separate devices to describe the ASoC stuff on the board, then there's no reason that couldn't have already been done to eliminate the machine_is_xxx() usage in ASoC - rather than complaining about machine_is_xxx() not being a very good solution. As I said, the problem was solved years ago, and all the component parts have been there also for years. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Thu, Sep 08, 2011 at 11:37:20PM +0100, Russell King - ARM Linux wrote: With DT of course, all devices get instantiated from the device tree, so there should not be any more platform specific chunks of code in these locations (ha, it couldn't be solved with platform data so I suspect it will continue to persist, forever unsolved.) That's not the case at all for audio, the PCB schematic for the audio subsystem on a device like a smartphone is a sufficiently interesting piece of hardware to be a device with a driver in its own right. The ASoC machine drivers aren't about instantiating devices, they are about controlling the interrelationships between the various devices in the audio subsystem. What will happen for device tree is that there will be a device in the device tree for the ASoC board. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Thu, 2011-09-08 at 22:28 +0200, Arnd Bergmann wrote: On Thursday 08 September 2011 20:05:48 Mans Rullgard wrote: I had the same thought, but I couldn't find a suitable string anywhere. Are you suggesting an if(machine_is_foo()) cascade in omap_init_audio()? I'll be the first to agree this patch is not particularly pretty. My general feeling is that practically every time someone writes machine_is_*(), they are doing it wrong. There are of course exceptions, but I would strongly recommend to have the initialization calling up from the board file into more general functions instead of having all boards calling the same function which then goes to board specific code again. I have to agree, that seems tasteless. I'd expect something like triggering registration of devices based off walking down a table of machine IDs or something. One other issue to consider here is that we don't want to discourage people from sharing machine drivers while we can so it can't be completely automatic, it Perhaps some sort of generic machine type based mechanism for instantiating drivers a bit like what's done with DMI might be useful, video will have a similar issue I guess. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Thu, Sep 08, 2011 at 11:47:16PM +0530, Jassi Brar wrote: Can't we do by having omap_init_audio() in arch/arm/mach-omap2/devices.c generate a platform device of name depending upon machine_is_* ? That's not a bad idea. If we were going to do that it shouldn't be OMAP specific, any platform could use it. Though we'd need a way to override it to provide platform data on systems that needs it. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Thursday 08 September 2011 20:05:48 Mans Rullgard wrote: Can't we do by having omap_init_audio() in arch/arm/mach-omap2/devices.c generate a platform device of name depending upon machine_is_* ? I had the same thought, but I couldn't find a suitable string anywhere. Are you suggesting an if(machine_is_foo()) cascade in omap_init_audio()? I'll be the first to agree this patch is not particularly pretty. My general feeling is that practically every time someone writes machine_is_*(), they are doing it wrong. There are of course exceptions, but I would strongly recommend to have the initialization calling up from the board file into more general functions instead of having all boards calling the same function which then goes to board specific code again. Arnd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] ASoC: omap: convert per-board modules to platform drivers
This converts the per-board modules to platform drivers for a device created by in main platform setup. These drivers call snd_soc_register_card() directly instead of going via a soc-audio device and the corresponding driver in soc-core. Signed-off-by: Mans Rullgard mans.rullg...@linaro.org --- Platform device names fixed. N8x0 changed to get clocks before registering card. --- arch/arm/mach-omap2/board-3430sdp.c |6 ++ arch/arm/mach-omap2/board-4430sdp.c |6 ++ arch/arm/mach-omap2/board-am3517evm.c|7 +++ arch/arm/mach-omap2/board-devkit8000.c |6 ++ arch/arm/mach-omap2/board-igep0020.c |6 ++ arch/arm/mach-omap2/board-n8x0.c |6 ++ arch/arm/mach-omap2/board-omap3beagle.c |6 ++ arch/arm/mach-omap2/board-omap3evm.c |7 +++ arch/arm/mach-omap2/board-omap3pandora.c |6 ++ arch/arm/mach-omap2/board-overo.c| 17 ++ arch/arm/mach-omap2/board-rx51.c |6 ++ arch/arm/mach-omap2/board-zoom-peripherals.c |6 ++ sound/soc/omap/am3517evm.c | 55 --- sound/soc/omap/igep0020.c| 52 -- sound/soc/omap/n810.c| 73 -- sound/soc/omap/omap3beagle.c | 55 --- sound/soc/omap/omap3evm.c| 56 +--- sound/soc/omap/omap3pandora.c| 70 +++-- sound/soc/omap/overo.c | 56 --- sound/soc/omap/rx51.c| 55 +-- sound/soc/omap/sdp3430.c | 65 ++- sound/soc/omap/sdp4430.c | 60 + sound/soc/omap/zoom2.c | 68 23 files changed, 509 insertions(+), 241 deletions(-) diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c index 5dac974..4c6a845 100644 --- a/arch/arm/mach-omap2/board-3430sdp.c +++ b/arch/arm/mach-omap2/board-3430sdp.c @@ -470,6 +470,11 @@ static struct twl4030_codec_data sdp3430_codec = { .audio = sdp3430_audio, }; +static struct platform_device sdp3430_soc_audio = { + .name = sdp3430-soc-audio, + .id = -1, +}; + static struct twl4030_platform_data sdp3430_twldata = { .irq_base = TWL4030_IRQ_BASE, .irq_end= TWL4030_IRQ_END, @@ -796,6 +801,7 @@ static void __init omap_3430sdp_init(void) sdp3430_display_init(); enable_board_wakeup_source(); usbhs_init(usbhs_bdata); + platform_device_register(sdp3430_soc_audio); } MACHINE_START(OMAP_3430SDP, OMAP3430 3430SDP board) diff --git a/arch/arm/mach-omap2/board-4430sdp.c b/arch/arm/mach-omap2/board-4430sdp.c index 63de2d3..d73d7e7 100644 --- a/arch/arm/mach-omap2/board-4430sdp.c +++ b/arch/arm/mach-omap2/board-4430sdp.c @@ -276,11 +276,17 @@ static struct platform_device sdp4430_lcd_device = { .id = -1, }; +static struct platform_device sdp4430_soc_audio = { + .name = sdp4430-soc-audio, + .id = -1, +}; + static struct platform_device *sdp4430_devices[] __initdata = { sdp4430_lcd_device, sdp4430_gpio_keys_device, sdp4430_leds_gpio, sdp4430_leds_pwm, + sdp4430_soc_audio, }; static struct omap_lcd_config sdp4430_lcd_config __initdata = { diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-omap2/board-am3517evm.c index 63af417..5d632f6 100644 --- a/arch/arm/mach-omap2/board-am3517evm.c +++ b/arch/arm/mach-omap2/board-am3517evm.c @@ -457,6 +457,11 @@ static void am3517_evm_hecc_init(struct ti_hecc_platform_data *pdata) platform_device_register(am3517_hecc_device); } +static struct platform_device am3517_evm_soc_audio = { + .name = am3517evm-soc-audio, + .id = -1, +}; + static struct omap_board_config_kernel am3517_evm_config[] __initdata = { }; @@ -487,6 +492,8 @@ static void __init am3517_evm_init(void) /* MUSB */ am3517_evm_musb_init(); + + platform_device_register(am3517_evm_soc_audio); } MACHINE_START(OMAP3517EVM, OMAP3517/AM3517 EVM) diff --git a/arch/arm/mach-omap2/board-devkit8000.c b/arch/arm/mach-omap2/board-devkit8000.c index 34956ec..070d4f6 100644 --- a/arch/arm/mach-omap2/board-devkit8000.c +++ b/arch/arm/mach-omap2/board-devkit8000.c @@ -477,6 +477,11 @@ static struct platform_device omap_dm9000_dev = { }, }; +static struct platform_device soc_audio = { + .name = omap3beagle-soc-audio, + .id = -1, +}; + static void __init omap_dm9000_init(void) { unsigned char *eth_addr = omap_dm9000_platdata.dev_addr; @@ -505,6 +510,7 @@ static struct platform_device *devkit8000_devices[] __initdata = { leds_gpio, keys_gpio, omap_dm9000_dev, + soc_audio, };
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On 8 September 2011 20:17, Jassi Brar jaswinder.si...@linaro.org wrote: On 9 September 2011 00:35, Mans Rullgard mans.rullg...@linaro.org wrote: On 8 September 2011 19:17, Jassi Brar jassisinghb...@gmail.com wrote: On Thu, Sep 8, 2011 at 11:04 PM, Mans Rullgard mans.rullg...@linaro.org wrote: This converts the per-board modules to platform drivers for a device created by in main platform setup. These drivers call snd_soc_register_card() directly instead of going via a soc-audio device and the corresponding driver in soc-core. Signed-off-by: Mans Rullgard mans.rullg...@linaro.org --- Platform device names fixed. N8x0 changed to get clocks before registering card. --- arch/arm/mach-omap2/board-3430sdp.c | 6 ++ arch/arm/mach-omap2/board-4430sdp.c | 6 ++ arch/arm/mach-omap2/board-am3517evm.c | 7 +++ arch/arm/mach-omap2/board-devkit8000.c | 6 ++ arch/arm/mach-omap2/board-igep0020.c | 6 ++ arch/arm/mach-omap2/board-n8x0.c | 6 ++ arch/arm/mach-omap2/board-omap3beagle.c | 6 ++ arch/arm/mach-omap2/board-omap3evm.c | 7 +++ arch/arm/mach-omap2/board-omap3pandora.c | 6 ++ arch/arm/mach-omap2/board-overo.c | 17 ++ arch/arm/mach-omap2/board-rx51.c | 6 ++ arch/arm/mach-omap2/board-zoom-peripherals.c | 6 ++ Can't we do by having omap_init_audio() in arch/arm/mach-omap2/devices.c generate a platform device of name depending upon machine_is_* ? I had the same thought, but I couldn't find a suitable string anywhere. Are you suggesting an if(machine_is_foo()) cascade in omap_init_audio()? Yes. And you could assign same names as you do now in board init files. This would still be omap-specific though, but perhaps that can be acceptable for now. Longer term, I guess this is a candidate for device tree. -- Mans Rullgard Multimedia mans.rullg...@linaro.org -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ASoC: omap: convert per-board modules to platform drivers
On Fri, Sep 09, 2011 at 12:01:02AM +0100, Russell King - ARM Linux wrote: On Thu, Sep 08, 2011 at 03:47:31PM -0700, Mark Brown wrote: What will happen for device tree is that there will be a device in the device tree for the ASoC board. Sounds like you just solved the machine_is_xxx() problem in ASoC land too there. If you're _already_ going for separate devices to describe the ASoC stuff on the board, then there's no reason that couldn't have already been done to eliminate the machine_is_xxx() usage in ASoC - rather than complaining about machine_is_xxx() not being a very good solution. The problem is that someone has to manually go and add the device to every board that needs one and people find that tedious and slightly inelegant (especially for device tree where not everyone is entirely happy that it's a good idea to have the node for the board when we already have the top level machine information saying what board we're running on). Jassi's suggestion was that we should have some magic to automatically generate defaults for the relevant device registrations to sidestep these issues. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html