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

Stephen Warren
2015-02-25T00:08:15Z

On 02/24/2015 04:44 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 24 February 2015 at 14:08, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@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@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.

>> + * 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