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

[Sergei Shtylyov]

Hello.

Victor Rodriguez 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,

    No, please don't.

> 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.

    No, it's not really better. The generic list in da850.c should be more 
complete, regardless... Ideally, you should go thru the DA850 manual and put in 
that list all McASP pins that aren't already there. Then you can use your own 
pin list if that *complete* pin list can't be used on your board.

> Thanks

> Regards

> Victor Rodriguez

WBR, Sergei