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

[kishon <a0393678@ti.com>]

ping..

On Friday 08 October 2010 11:50 AM, Varadarajan, Charulatha wrote:
>
>
>> -----Original Message-----
>> From: Peter Ujfalusi [mailto:peter.ujfalusi@nokia.com]
>> Sent: Wednesday, October 06, 2010 12:47 PM
>> To: Varadarajan, Charulatha
>> Cc: linux-omap@vger.kernel.org; alsa-devel@alsa-project.org; Kamat,
>> Nishant; Datta, Shubhrajyoti; Basak, Partha; Girdwood, Liam;
>> jhnikula@gmail.com; broonie@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.
>
>>>
>>> [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@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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html