[alsa-devel] [PATCH 1/7] [RFC] OMAP: MCBSP: hwmod database for 2xxx devices

[Cousson, Benoit]

Hi Charu,

On 10/8/2010 8:20 AM, Varadarajan, Charulatha wrote:
>
>
>> From: Peter Ujfalusi [mailto:peter.ujfalusi at nokia.com]
>> Sent: Wednesday, October 06, 2010 12:47 PM
>> To: Varadarajan, Charulatha
>> Cc: linux-omap at vger.kernel.org; alsa-devel at alsa-project.org; Kamat,
>> Nishant; Datta, Shubhrajyoti; Basak, Partha; Girdwood, Liam;
>> jhnikula at gmail.com; broonie at opensource.wolfsonmicro.com; ABRAHAM, KISHON
>> VIJAY
>> Subject: Re: [PATCH 1/7] [RFC] OMAP: MCBSP: hwmod database for 2xxx
>> devices
>>
>> Hello,
>
> Thanks for the quick response.
>
>>
>> On Wednesday 06 October 2010 10:01:28 ext Varadarajan, Charulatha wrote:
>>> This patch series is targeted to implement mcbsp driver in
>>> hwmod way and to make use of pm_runtime APIs.
>>>
>>> This patch series is tested on OMAP3&  4 and yet to be tested
>>> on OMAP2.
>>>
>>> There are few clarifications required so that the next patch series
>>> can be implemented after aligning.
>>>
>>> 1. Audio layer is making use of mcbsp and it's dma base addresses and
>>> is closely coupled with omap-mcbsp.
>>> This can be handled either by
>>> a. providing an API with which Audio layer can get these addresses.
>>> (or)
>>> b. move the plat-omap/mcbsp.c and mach-omap2/mcbsp.c to sound/soc/omap/
>>> [1]
>>>
>>> Option (a) would only be a workaround to handle the situation. As
>>> audio is the only user for mcbsp, option (b) is better. If option(b)
>>> is agreed upon, the same can be addressed on top of the mcbsp hwmod
>>> series.
>>
>> it is true that at the moment only audio is using the McBSP ports, but
>> McBSP is
>> really flexible, it can run for example in SPI mode, and it can be
>> configured to
>> use other serial protocols.
>
> Yes.
>
>> I would go with option c.
>> Since ASoC is moving to multi-component (the conversion is already in
>> linux-
>> next), this means that the sound/soc/omap/omap-mcbsp, omap-pcm drivers are
>> platform drivers.
>> So if the plat-omap/mcbsp would register the platform device for McBSP
>> clients
>> (we have only ASoC client at the moment), and use platform data to pass
>> the
>> needed information to the McBSP client driver, than we do not need new API.
>
> Sorry I am confused.
>
> With hwmod implementation, there is a device register code for mcbsp
> devices in mach-omap2/mcbsp.c and a probe in plat-omap/mcbsp.c. The base
> address, dma info are not part of pdata and are available to the driver
> only after probe. I do not understand how the multi-component design in
> ASOC can avoid the new API.
>
> Also with this multi-component approach in ASOC, two device
> registrations happens for a single mcbsp device with two different
> rames ("omap-mcbsp-dai.id"&  "omap-mcbsp.id"). Please explain if this
> what is expected?
>
>> We still need to modify the ASoC drivers to make use of this platform data,
>> but
>> at least we are going to keep the door open for others to use the McBSP
>> ports
>> for other than audio.
>
> Agreed. But the current omap-mcbsp driver cannot work standalone for
> OMAP3/4 due to the issues stated below:
> 1. omap_mcbsp_pollwrite and omap_mcbsp_pollread functions access McBSP
> registers as 16-bit. But in OMAP3/4, McBSP registers (DRR_REG and DXR_REG)
> are limited to 32-bit data accesses and hence poll mode would not work [x].
> 2. DMA transfers would also not work as it requires a patch similar to [y].
>
> Patches [x]&  [y] were rejected as there are no users other than asoc.
> If it is not agreed to move omap-mcbsp driver to asoc layer, we need to
> get the omap-mcbsp driver working as a standalone driver. Otherwise it
> is of no use keeping the mcbsp driver in plat-omap.
>
> Once [x]&  [y] patches are upstreamed, audio layer needs to be modified
> to make use of omap-mcbsp APIs rather than Audio layer calling dma
> APIs directly to transfer data.
>
> Coming back to the original question. Either we need to fix the broken
> legacy mcbsp driver or move the omap-mcbsp driver completely to asoc
> layer. What do you say?
>
> [x] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg19531.html
> [y] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg04085.html
>>
>>> 2. Sidetone feature is available only in OMAP3 (McBSP2&3) which has
>>> different base address and sys configs compared to it's mcbsp port.
>>> Hence the mcbsp is considered as a single device with two hwmods
>>> for McBSP2&3 devices in OMAP3.
>>
>> Sounds fair enough.
>
> Thanks.
>
>>
>>> 3. Autoidle needs to be disabled for sidetone before enabling the
>> sidetone
>>> feature. There was a design proposed by Kishon [2] to add an API in
>> hwmod
>>> to modify the autoidle bit but was not agreed upon. How do we handle
>> this
>>> situation where the device has to disable or enable the autoidle bit at
>>> runtime?
>>
>> Yeah, this is really annoying problem. The McBSP ST should block autoidle
>> from
>> McBSP side, but it does not.
>> If you can not get through the proposed API, we should consider to switch
>> the
>> corresponding McBSP port to NoIdle, when the ST is in use (and restore the
>> idle
>> mode, when the ST has been disabled).
>> When McBSP is in NoIdle the interface clock is not going to be gated, so
>> ST
>> block will be running without a problem (ST needs the iface clock for
>> operation)
>>
>> What do you think?
>
> I think it might not be possible to handle this, as the clocks are the same for ST and mcbsp port. pm_runtime APIs are not called during ST enable/disable as clocks are already enabled while enabling mcbsp port. Hence the idle bit change cannot happen even if the oh->flags are modified runtime during ST enable/disable.

This is mainly because of the 2 hwmods for 1 device implementation, 
which make perfect sense in theory, but because of that bug, becomes 
tricky to handle it in the fmwk.

FYI, SDMA and MUSB are as well requiring some sysconfig change during 
runtime. So a couple of API will have to be implemented.
I guess we don't have the choice. Now, it is just a matter of having the 
right implementation.
I'll comment on that patch directly.

Benoit


>
>>>
>>> [1] https://patchwork.kernel.org/patch/225582/
>>> [2] https://patchwork.kernel.org/patch/134371/
>>>
>>> We would resend the same patch series by including alsa mailing list
>>> (alsa-devel at alsa-project.org)
>>>
>>> <<snip>>
>>
>> --
>> Péter
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html