[U-Boot] [PATCH v3 5/8] rockchip: rk3399: Add improved pinctrl driver.
David Wu
david.wu at rock-chips.com
Fri Dec 28 12:48:46 UTC 2018
Hi Christoph,
This patch seems is less of code about drive strength, for some modules,
like LCD, Ethernet is still needed.
在 2018/12/27 下午9:13, Christoph Müllner 写道:
> Hi David,
>
> On 12/27/18 1:49 PM, David Wu wrote:
>> Hi Christoph,
>>
>> I once submitted a series of patches that they can support all Socs'
>> Pinctrl and how do you feel about using them.
>
> Thank's for pointing to that.
>
> Your driver looks good, but I don't like the huge amount
> of duplication in it (you have a function rkNNNN_calc_pull_reg_and_bit()
> for each SoC variant, which more or less do all the same).
> Also I prefer to have a generic core driver and SoC specific parts
> in their own C files (to have a slim driver for each SoC, but a maximum
> of code reuse). Also Kconfig entries like PINCTRL_ROCKCHIP_RK3188 don't
> seem to do anything in your driver.
>
> Since this is from Feb 2018:
> May I ask, why you did not continue to bring that mainline?
It seems i have lost them in my mailbox. I'm going to update a new
version in the near future.
>
> My plan is to get the driver in for RK3399 asap and enable it only for
> the RK3399-Q7 board for now (to not mess with other boards).
> During the next merge window I want to move the generic parts into their
> own C files. Other SoC-specific drivers can follow then with their own
> mini-drivers (no code just configuration).
>
> Thanks,
> Christoph
>
>>
>> http://patchwork.ozlabs.org/patch/868849/
>>
>> 在 2018/12/27 上午9:11, Kever Yang 写道:
>>>
>>> Add David to review the pinctrl driver.
>>>
>>> Thanks,
>>> - Kever
>>> On 12/17/2018 09:30 PM, Christoph Muellner wrote:
>>>> The current pinctrl driver for the RK3399 has a range of qulity issues.
>>>> E.g. it only implements the .set_state_simple() callback, it
>>>> does not parse the available pinctrl information from the DTS
>>>> (instead uses hardcoded values), is not flexible enough to cover
>>>> devices without 'interrupt' field in the DTS (e.g. PWM),
>>>> is not written generic enough to make code reusable among other
>>>> rockchip SoCs...
>>>>
>>>> This patch addresses these issues by reimplementing the whole driver
>>>> from scratch using the .set_state() callback.
>>>> The new implementation covers all featurese of the old code
>>>> (i.e. it supports pinmuxing and pullup/pulldown configuration).
>>>>
>>>> This patch has been tested on a RK3399-Q7 SoM (Puma).
>>>>
>>>> Signed-off-by: Christoph Muellner
>>>> <christoph.muellner at theobroma-systems.com>
>>>> ---
>>>>
>>>> Changes in v3: None
>>>> Changes in v2: None
>>>>
>>>> drivers/pinctrl/rockchip/pinctrl_rk3399.c | 226
>>>> ++++++++++++++++++++++++++++++
>>>> 1 file changed, 226 insertions(+)
>>>>
>>>> diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3399.c
>>>> b/drivers/pinctrl/rockchip/pinctrl_rk3399.c
>>>> index bc92dd7c06..ed9828989f 100644
>>>> --- a/drivers/pinctrl/rockchip/pinctrl_rk3399.c
>>>> +++ b/drivers/pinctrl/rockchip/pinctrl_rk3399.c
>>>> @@ -1,6 +1,7 @@
>>>> // SPDX-License-Identifier: GPL-2.0+
>>>> /*
>>>> * (C) Copyright 2016 Rockchip Electronics Co., Ltd
>>>> + * (C) 2018 Theobroma Systems Design und Consulting GmbH
>>>> */
>>>> #include <common.h>
>>>> @@ -14,11 +15,234 @@
>>>> #include <asm/arch/clock.h>
>>>> #include <dm/pinctrl.h>
>>>> +static const u32 RK_GRF_P_PULLUP = 1;
>>>> +static const u32 RK_GRF_P_PULLDOWN = 2;
>>>> +
>>>> struct rk3399_pinctrl_priv {
>>>> struct rk3399_grf_regs *grf;
>>>> struct rk3399_pmugrf_regs *pmugrf;
>>>> + struct rockchip_pin_bank *banks;
>>>> +};
>>>> +
>>>> +/**
>>>> + * Location of pinctrl/pinconf registers.
>>>> + */
>>>> +enum rk_grf_location {
>>>> + RK_GRF,
>>>> + RK_PMUGRF,
>>>> +};
>>>> +
>>>> +/**
>>>> + * @nr_pins: number of pins in this bank
>>>> + * @bank_num: number of the bank, to account for holes
>>>> + * @iomux: array describing the 4 iomux sources of the bank
>>>> + */
>>>> +struct rockchip_pin_bank {
>>>> + u8 nr_pins;
>>>> + enum rk_grf_location grf_location;
>>>> + size_t iomux_offset;
>>>> + size_t pupd_offset;
>>>> };
>>>> +#define PIN_BANK(pins, grf, iomux, pupd) \
>>>> + { \
>>>> + .nr_pins = pins, \
>>>> + .grf_location = grf, \
>>>> + .iomux_offset = iomux, \
>>>> + .pupd_offset = pupd, \
>>>> + }
>>>> +
>>>> +static struct rockchip_pin_bank rk3399_pin_banks[] = {
>>>> + PIN_BANK(16, RK_PMUGRF,
>>>> + offsetof(struct rk3399_pmugrf_regs, gpio0a_iomux),
>>>> + offsetof(struct rk3399_pmugrf_regs, gpio0_p)),
>>>> + PIN_BANK(32, RK_PMUGRF,
>>>> + offsetof(struct rk3399_pmugrf_regs, gpio1a_iomux),
>>>> + offsetof(struct rk3399_pmugrf_regs, gpio1_p)),
>>>> + PIN_BANK(32, RK_GRF,
>>>> + offsetof(struct rk3399_grf_regs, gpio2a_iomux),
>>>> + offsetof(struct rk3399_grf_regs, gpio2_p)),
>>>> + PIN_BANK(32, RK_GRF,
>>>> + offsetof(struct rk3399_grf_regs, gpio3a_iomux),
>>>> + offsetof(struct rk3399_grf_regs, gpio3_p)),
>>>> + PIN_BANK(32, RK_GRF,
>>>> + offsetof(struct rk3399_grf_regs, gpio4a_iomux),
>>>> + offsetof(struct rk3399_grf_regs, gpio4_p)),
>>>> +};
>>>> +
>>>> +static void rk_pinctrl_get_info(uintptr_t base, u32 index, uintptr_t
>>>> *addr,
>>>> + u32 *shift, u32 *mask)
>>>> +{
>>>> + /*
>>>> + * In general we four subsequent 32-bit configuration registers
>>>> + * per bank (e.g. GPIO2A_P, GPIO2B_P, GPIO2C_P, GPIO2D_P).
>>>> + * The configuration for each pin has two bits.
>>>> + *
>>>> + * @base...contains the address to the first register.
>>>> + * @index...defines the pin within the bank (0..31).
>>>> + * @addr...will be the address of the actual register to use
>>>> + */
>>>> +
>>>> + const u32 pins_per_register = 8;
>>>> + const u32 config_bits_per_pin = 2;
>>>> +
>>>> + /* Get the address of the configuration register. */
>>>> + *addr = base + (index / pins_per_register) * sizeof(u32);
>>>> +
>>>> + /* Get the bit offset within the configruation register. */
>>>> + *shift = (index & (pins_per_register - 1)) * config_bits_per_pin;
>>>> +
>>>> + /* Get the (unshifted) mask for the configuration pins. */
>>>> + *mask = ((1 << config_bits_per_pin) - 1);
>>>> +
>>>> + pr_debug("%s: addr=0x%lx, mask=0x%x, shift=0x%x\n",
>>>> + __func__, *addr, *mask, *shift);
>>>> +}
>>>> +
>>>> +static void rk3399_pinctrl_set_pin_iomux(uintptr_t grf_addr,
>>>> + struct rockchip_pin_bank *bank,
>>>> + u32 index, u32 muxval)
>>>> +{
>>>> + uintptr_t iomux_base, addr;
>>>> + u32 shift, mask;
>>>> +
>>>> + iomux_base = grf_addr + bank->iomux_offset;
>>>> + rk_pinctrl_get_info(iomux_base, index, &addr, &shift, &mask);
>>>> +
>>>> + /* Set pinmux register */
>>>> + rk_clrsetreg(addr, mask << shift, muxval << shift);
>>>> +}
>>>> +
>>>> +static void rk3399_pinctrl_set_pin_pupd(uintptr_t grf_addr,
>>>> + struct rockchip_pin_bank *bank,
>>>> + u32 index, int pinconfig)
>>>> +{
>>>> + uintptr_t pupd_base, addr;
>>>> + u32 shift, mask, pupdval;
>>>> +
>>>> + /* Fast path in case there's nothing to do. */
>>>> + if (!pinconfig)
>>>> + return;
>>>> +
>>>> + if (pinconfig & (1 << PIN_CONFIG_BIAS_PULL_UP))
>>>> + pupdval = RK_GRF_P_PULLUP;
>>>> + else if (pinconfig & (1 << PIN_CONFIG_BIAS_PULL_DOWN))
>>>> + pupdval = RK_GRF_P_PULLDOWN;
>>>> + else
>>>> + /* Flag not supported. */
>>>> + pr_warn("%s: Unsupported pinconfig flag: 0x%x\n", __func__,
>>>> + pinconfig);
>>>> + return;
>>>> +
>>>> + pupd_base = grf_addr + (uintptr_t)bank->pupd_offset;
>>>> + rk_pinctrl_get_info(pupd_base, index, &addr, &shift, &mask);
>>>> +
>>>> + /* Set pull-up/pull-down regisrer */
>>>> + rk_clrsetreg(addr, mask << shift, pupdval << shift);
>>>> +}
>>>> +
>>>> +static int rk3399_pinctrl_set_pin(struct udevice *dev, u32 banknum,
>>>> u32 index,
>>>> + u32 muxval, int pinconfig)
>>>> +{
>>>> + struct rk3399_pinctrl_priv *priv = dev_get_priv(dev);
>>>> + struct rockchip_pin_bank *bank = &priv->banks[banknum];
>>>> + uintptr_t grf_addr;
>>>> +
>>>> + pr_debug("%s: 0x%x 0x%x 0x%x 0x%x\n", __func__, banknum, index,
>>>> muxval,
>>>> + pinconfig);
>>>> +
>>>> + if (bank->grf_location == RK_GRF)
>>>> + grf_addr = (uintptr_t)priv->grf;
>>>> + else if (bank->grf_location == RK_PMUGRF)
>>>> + grf_addr = (uintptr_t)priv->pmugrf;
>>>> + else
>>>> + return -EINVAL;
>>>> +
>>>> + rk3399_pinctrl_set_pin_iomux(grf_addr, bank, index, muxval);
>>>> +
>>>> + rk3399_pinctrl_set_pin_pupd(grf_addr, bank, index, pinconfig);
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int rk3399_pinctrl_set_state(struct udevice *dev, struct
>>>> udevice *config)
>>>> +{
>>>> + /*
>>>> + * The order of the fields in this struct must match the order of
>>>> + * the fields in the "rockchip,pins" property.
>>>> + */
>>>> + struct rk_pin {
>>>> + u32 banknum;
>>>> + u32 index;
>>>> + u32 muxval;
>>>> + u32 phandle;
>>>> + } __packed;
>>>> +
>>>> + u32 *fields = NULL;
>>>> + const int fields_per_pin = 4;
>>>> + int num_fields, num_pins;
>>>> + int ret;
>>>> + int size;
>>>> + int i;
>>>> + struct rk_pin *pin;
>>>> +
>>>> + pr_debug("%s: %s\n", __func__, config->name);
>>>> +
>>>> + size = dev_read_size(config, "rockchip,pins");
>>>> + if (size < 0)
>>>> + return -ENODEV;
>>>> +
>>>> + num_fields = size / sizeof(u32);
>>>> + num_pins = num_fields / fields_per_pin;
>>>> +
>>>> + if (num_fields * sizeof(u32) != size ||
>>>> + num_pins * fields_per_pin != num_fields) {
>>>> + printf("Sanity check failed!\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + fields = calloc(num_fields, sizeof(u32));
>>>> + if (!fields)
>>>> + return -ENOMEM;
>>>> +
>>>> + ret = dev_read_u32_array(config, "rockchip,pins", fields,
>>>> num_fields);
>>>> + if (ret) {
>>>> + pr_warn("%s: Failed to read rockchip,pins fields.\n",
>>>> + config->name);
>>>> + goto end;
>>>> + }
>>>> +
>>>> + pin = (struct rk_pin *)fields;
>>>> + for (i = 0; i < num_pins; i++, pin++) {
>>>> + struct udevice *dev_pinconfig;
>>>> + int pinconfig;
>>>> +
>>>> + ret = uclass_get_device_by_phandle_id(UCLASS_PINCONFIG,
>>>> + pin->phandle,
>>>> + &dev_pinconfig);
>>>> + if (ret) {
>>>> + printf("Could not get pinconfig device\n");
>>>> + goto end;
>>>> + }
>>>> +
>>>> + pinconfig = pinctrl_decode_pin_config_dm(dev_pinconfig);
>>>> + if (pinconfig < 0) {
>>>> + printf("Could not parse pinconfig\n");
>>>> + goto end;
>>>> + }
>>>> +
>>>> + ret = rk3399_pinctrl_set_pin(dev, pin->banknum, pin->index,
>>>> + pin->muxval, pinconfig);
>>>> + if (ret) {
>>>> + printf("Could not set pinctrl settings\n");
>>>> + goto end;
>>>> + }
>>>> + }
>>>> +
>>>> +end:
>>>> + free(fields);
>>>> + return ret;
>>>> +}
>>>> +
>>>> static void pinctrl_rk3399_pwm_config(struct rk3399_grf_regs *grf,
>>>> struct rk3399_pmugrf_regs *pmugrf, int pwm_id)
>>>> {
>>>> @@ -468,6 +692,7 @@ static int rk3399_pinctrl_set_state_simple(struct
>>>> udevice *dev,
>>>> }
>>>> static struct pinctrl_ops rk3399_pinctrl_ops = {
>>>> + .set_state = rk3399_pinctrl_set_state,
>>>> .set_state_simple = rk3399_pinctrl_set_state_simple,
>>>> .request = rk3399_pinctrl_request,
>>>> .get_periph_id = rk3399_pinctrl_get_periph_id,
>>>> @@ -481,6 +706,7 @@ static int rk3399_pinctrl_probe(struct udevice *dev)
>>>> priv->grf = syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
>>>> priv->pmugrf = syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);
>>>> debug("%s: grf=%p, pmugrf=%p\n", __func__, priv->grf,
>>>> priv->pmugrf);
>>>> + priv->banks = rk3399_pin_banks;
>>>> return ret;
>>>> }
>>>
>>>
>>>
>>>
>>
>
>
More information about the U-Boot
mailing list