[U-Boot] [PATCH v2 29/56] rockchip: rk3368: grf: use shifted-constants

[Dr. Philipp Tomsich]

> On 01 Aug 2017, at 11:49, Simon Glass <sjg at chromium.org> wrote:
> 
> On 28 July 2017 at 02:53, Dr. Philipp Tomsich
> <philipp.tomsich at theobroma-systems.com> wrote:
>> 
>>> On 28 Jul 2017, at 05:38, Simon Glass <sjg at chromium.org> wrote:
>>> 
>>> Hi Philipp,
>>> 
>>> On 26 July 2017 at 04:40, Philipp Tomsich
>>> <philipp.tomsich at theobroma-systems.com> wrote:
>>>> The RK3368 GRF header was still defines with a shifted-mask but with
>>>> non-shifted function selectors for the IOMUX defines.  As the RK3368
>>>> support is still fresh enough to allow a quick change, we do this now
>>>> before having more code use this.
>>>> 
>>>> Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
>>>> 
>>>> ---
>>>> 
>>>> Changes in v2:
>>>> - dropped the RK3368_ prefix for the GRF constants
>>>> 
>>>> arch/arm/include/asm/arch-rockchip/grf_rk3368.h | 441 ++++++++++++------------
>>>> drivers/pinctrl/rockchip/pinctrl_rk3368.c       |   9 +-
>>>> 2 files changed, 226 insertions(+), 224 deletions(-)
>>>> 
>>>> diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3368.h b/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
>>>> index a438f5d..1966960 100644
>>>> --- a/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
>>>> +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3368.h
>>>> @@ -1,4 +1,6 @@
>>>> -/* (C) Copyright 2016 Rockchip Electronics Co., Ltd
>>>> +/*
>>>> + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
>>>> + * (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH
>>>> *
>>>> * SPDX-License-Identifier:     GPL-2.0+
>>>> */
>>>> @@ -100,315 +102,318 @@ check_member(rk3368_pmu_grf, os_reg[0], 0x200);
>>>> 
>>>> /*GRF_GPIO0C_IOMUX*/
>>>> enum {
>>>> -       GPIO0C7_SHIFT           = 14,
>>>> -       GPIO0C7_MASK            = 3 << GPIO0C7_SHIFT,
>>>> -       GPIO0C7_GPIO            = 0,
>>>> -       GPIO0C7_LCDC_D19,
>>>> -       GPIO0C7_TRACE_D9,
>>>> -       GPIO0C7_UART1_RTSN,
>>>> -
>>>> -       GPIO0C6_SHIFT           = 12,
>>>> -       GPIO0C6_MASK            = 3 << GPIO0C6_SHIFT,
>>>> +       GPIO0C7_MASK           = GENMASK(15, 14),
>>>> +       GPIO0C7_GPIO           = 0,
>>>> +       GPIO0C7_LCDC_D19        = (1 << 14),
>>>> +       GPIO0C7_TRACE_D9        = (2 << 14),
>>>> +       GPIO0C7_UART1_RTSN      = (3 << 14),
>>> 
>>> Can we keep the SHIFT macros so we(e.g.) don't have to open-code the
>>> '14' each time?
>> 
>> In fact I wanted this to stick out, so we are motivated to move towards
>> a model of set_pin_function(BANK_C, PIN_7, UART1) where the
>> shift-value can then calculated from the pin# within the bank (and the
>> function id is looked up from a config-table for this pin).
> 
> That sounds like a separate issue to me. The problem here is that it
> it isn't clear where the '14' comes from and what it is common with.
> Well, not clear enough to permit using an editor's search/replace
> function.

Concern understood. I’ll revise.

Regards,
Philipp.