[U-Boot] [PATCH 1/9] ARM: tegra: pinmux: add note re: drive group field defines

[Simon Glass]

Hi Stephen,

On 24 February 2015 at 17:08, Stephen Warren <swarren at wwwdotorg.org> wrote:
> On 02/24/2015 04:44 PM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 24 February 2015 at 14:08, Stephen Warren <swarren at wwwdotorg.org>
>> wrote:
>>>
>>> From: Stephen Warren <swarren at nvidia.com>
>>>
>>> Tegra's drive group registers have a remarkably inconsistent layout. The
>>> current U-Boot driver doesn't take this into account at all. Add a
>>> comment to describe the issue, so at least anyone debugging the driver
>>> will be aware of this. To solve this, we'd need to add a per-drive-group
>>> data structure describing the layout for the individual register. Since
>>> we don't set up too many drive groups in U-Boot at present, this
>>> hopefully isn't causing too much practical issue. Still, we probably need
>>> to fix this sometime.
>>>
>>> Wth Tegra210, the register layout becomes almost entirely consistent, so
>>> this problem partially solves itself over time.
>>>
>>> Signed-off-by: Stephen Warren <swarren at nvidia.com>
>>> ---
>>>   arch/arm/cpu/tegra-common/pinmux-common.c | 15 +++++++++++++++
>>>   1 file changed, 15 insertions(+)
>>>
>>> diff --git a/arch/arm/cpu/tegra-common/pinmux-common.c
>>> b/arch/arm/cpu/tegra-common/pinmux-common.c
>>> index 64baed45d591..0bef6e246357 100644
>>> --- a/arch/arm/cpu/tegra-common/pinmux-common.c
>>> +++ b/arch/arm/cpu/tegra-common/pinmux-common.c
>>> @@ -347,6 +347,21 @@ void pinmux_config_pingrp_table(const struct
>>> pmux_pingrp_config *config,
>>>   #define SCHMT_SHIFT    3
>>>   #define LPMD_SHIFT     4
>>>   #define LPMD_MASK      (3 << LPMD_SHIFT)
>>> +/*
>>> + * Note that the following DRV* and SLW* defines are accurate for many
>>> drive
>>
>>
>> Do you mean 'accurate'? That seems like a good thing so wonder if this
>> should be reworded a bit compared to your commit message.
>
>
> I think "accurate" is correct; the defines accurately describe the layout of
> the register in many cases, but not all. In cases where the register layout
> is different, the defines are not accurate for those cases. I suppose
> "correct" and "incorrect" would be suitable synonyms for "accurate" and "not
> accurate" if you wish.

That's OK, I suppose I was expecting the comment to say that it is
accurate for many but not all. But it's not important.

>
>
>>> + * groups on many SoCs. We really need a per-group data structure to
>>> solve
>>> + * this, since the fields are in different positions/sizes in different
>>> + * registers (for different groups).
>>> + *
>>> + * On Tegra30/114/124, the DRV*_SHIFT values vary.
>>> + * On Tegra30, the SLW*_SHIFT values vary.
>>> + * On Tegra30/114/124/210, the DRV*_MASK values vary, although the
>>> values
>>> + *   below are wide enough to cover the widest fields, and hopefully
>>> don't
>>> + *   interfere with any other fields.
>>> + * On Tegra30, the SLW*_MASK values vary, but we can't use a value
>>> that's
>>> + *   wide enough to cover all cases, since that would cause the field to
>>> + *   overlap with other fields in the narrower cases.
>>> + */
>>>   #define DRVDN_SHIFT    12
>>>   #define DRVDN_MASK     (0x7F << DRVDN_SHIFT)
>>>   #define DRVUP_SHIFT    20
>>> --
>>> 1.9.1
>
>

Regards,
Simon