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

Simon Glass
2015-02-26T00:55:35Z

Hi Stephen,

On 24 February 2015 at 17:08, Stephen Warren <swarren@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@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.

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