[PATCH v8 4/9] davinci: McASP configuration for Omapl138-Hawkboard

[Victor Rodriguez]

On Mon, Nov 15, 2010 at 9:47 AM, Sergei Shtylyov <sshtylyov at mvista.com> wrote:
> Hello.
>
> Nori, Sekhar wrote:
>
>>>>> This patch defines Pin Mux configuration for MacASP
>>>>> used on the Hawkboard-L138 system in order to add Audio support
>>>>> Signed-off-by: Victor Rodriguez<victor.rodriguez at sasken.com>
>>>>> Tested-by: Rene Gonzalez<renegs.2378 at gmail.com>
>>>>> diff --git a/arch/arm/mach-davinci/da850.c
>>>>> b/arch/arm/mach-davinci/da850.c
>>>>> index 63916b9..f033a0a 100644
>>>>> --- a/arch/arm/mach-davinci/da850.c
>>>>> +++ b/arch/arm/mach-davinci/da850.c
>>>>> @@ -591,7 +591,7 @@ const short da850_cpgmac_pins[] __initdata = {
>>>>>  const short da850_mcasp_pins[] __initdata = {
>>>>>       DA850_AHCLKX, DA850_ACLKX, DA850_AFSX,
>>>>>       DA850_AHCLKR, DA850_ACLKR, DA850_AFSR, DA850_AMUTE,
>>>>> -     DA850_AXR_11, DA850_AXR_12,
>>>>> +     DA850_AXR_11, DA850_AXR_12, DA850_AXR_13, DA850_AXR_14,
>
>>>> Looks like I missed pointing this out previously, but extending
>>>> this list to take care of all boards will not be right since
>>>> (for example) AXR13 and AXR14 pins could be used for different
>>>> purpose on different boards.
>
>>>    This is correct as the list in da850.c is a *generic* module's pin
>>> list.
>>> If the board needs less pins (and the pins it does not use for McASP are
>>> used
>>> differently), it should define its own pin list.
>
>>>> The right way would be to make this a per-board list. Since it
>>>> is marked __initdata, that wouldn't lead to bloat.
>
>>>    This patch is correct anyway. Unless DA850 EVM board can't use these
>>> pins
>>> for McASP -- but in this case the corresponding board file needs the
>>> specific
>>> pin list added.
>
>> Okay. I guess you are saying we will keep adding pins to the generic list
>> as long as *all* supported boards don't get a conflict
>
>   No, I don't care about conflicts as long as da850.c is concerned.
>
>> and if we run into
>> a conflict we will spawn separate list for the affected board.
>
>   Yes.
>
>> The only issue I see with this approach is it puts too much burden on the
>> developer to verify that none of the supported boards break.
>
>   That should be verified by the owners of that board. They shouldn't even
> have used the generic pin lists in the first place. The conception behind
> pin lists in da850.c was celarly misunderstood when DA850 EVM code was
> developed.
>
>> Since it is highly unlikely that any board will need all the McASP pins,
>> the generic list will likely remain unused.
>
>   At least for now, Hawkboard will use all the defined pins -- I don't know
> if there are any more undefined yet...
>
>> It might just be easier to
>> start using board specific lists right away. This is especially true for
>> McASP where usage of pins across boards will likely vary widely.
>
>   Well, that depends on how complete the new generic McASP pin list is. If
> it's still not, then board specific pin list looks preferable indeed...
>
>> Thanks,
>> Sekhar
>
> WBR, Sergei
>

HI Sergei and Sekhar

Thanks for check the patch

What I can do if you agree with this change is to leave da850.c as it
is, and declare

static short hawk_mcasp_pins[] __initdata = {
	DA850_AHCLKX, DA850_ACLKX, DA850_AFSX,
	DA850_AHCLKR, DA850_ACLKR, DA850_AFSR, DA850_AMUTE,
	DA850_AXR_11, DA850_AXR_12, DA850_AXR_13, DA850_AXR_14,
	-1
};

on the hawkboard file and call it insted of da850_mcasp_pins.

	ret = davinci_cfg_reg_list(hawk_mcasp_pins);
	if (ret)
		pr_warning("%s: mcasp mux setup failed: %d\n", __func__, ret);

Please tell me if you agree with this change, I think is better
because I do not touch any other file besides my board file.

Thanks

Regards

Victor Rodriguez