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

Simon Glass
2015-02-24T23:44:36Z

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.

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