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

[Simon Glass]

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.

Regards,
Simon