diff mbox series

[U-Boot] pinctrl: rockchip: Add 32bit writing function for rk3288 gpio0 pinctrl

Message ID 20190212115151.2351-1-david.wu@rock-chips.com
State Accepted
Commit 502980914b2d6f9ee85a823aa3ef9ead76c0b7f2
Delegated to: Philipp Tomsich
Headers show
Series [U-Boot] pinctrl: rockchip: Add 32bit writing function for rk3288 gpio0 pinctrl | expand

Commit Message

David Wu Feb. 12, 2019, 11:51 a.m. UTC
There are no higher 16 writing corresponding bits for pmu_gpio0's
iomux/drive/pull at rk3288, need to read the value from register
firstly. Add the flag to distinguish it from normal registers.

Signed-off-by: David Wu <david.wu@rock-chips.com>
---
 drivers/pinctrl/rockchip/pinctrl-rk3288.c     | 17 ++++++--
 .../pinctrl/rockchip/pinctrl-rockchip-core.c  | 39 ++++++++++++++-----
 drivers/pinctrl/rockchip/pinctrl-rockchip.h   | 33 ++++++++++++++++
 3 files changed, 76 insertions(+), 13 deletions(-)

Comments

Philipp Tomsich Feb. 12, 2019, 11:55 a.m. UTC | #1
> On 12.02.2019, at 12:51, David Wu <david.wu@rock-chips.com> wrote:
> 
> There are no higher 16 writing corresponding bits for pmu_gpio0's
> iomux/drive/pull at rk3288, need to read the value from register
> firstly. Add the flag to distinguish it from normal registers.
> 
> Signed-off-by: David Wu <david.wu@rock-chips.com>
> ---
> drivers/pinctrl/rockchip/pinctrl-rk3288.c     | 17 ++++++--
> .../pinctrl/rockchip/pinctrl-rockchip-core.c  | 39 ++++++++++++++-----
> drivers/pinctrl/rockchip/pinctrl-rockchip.h   | 33 ++++++++++++++++
> 3 files changed, 76 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pinctrl/rockchip/pinctrl-rk3288.c b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
> index 60585f3208..8b6ce11a63 100644
> --- a/drivers/pinctrl/rockchip/pinctrl-rk3288.c
> +++ b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
> @@ -92,10 +92,19 @@ static void rk3288_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank,
> }
> 
> static struct rockchip_pin_bank rk3288_pin_banks[] = {
> -	PIN_BANK_IOMUX_FLAGS(0, 24, "gpio0", IOMUX_SOURCE_PMU,
> -					     IOMUX_SOURCE_PMU,
> -					     IOMUX_SOURCE_PMU,
> -					     IOMUX_UNROUTED
> +	PIN_BANK_IOMUX_DRV_PULL_FLAGS(0, 24, "gpio0",
> +				      IOMUX_SOURCE_PMU | IOMUX_WRITABLE_32BIT,
> +				      IOMUX_SOURCE_PMU | IOMUX_WRITABLE_32BIT,
> +				      IOMUX_SOURCE_PMU | IOMUX_WRITABLE_32BIT,
> +				      IOMUX_UNROUTED,
> +				      DRV_TYPE_WRITABLE_32BIT,
> +				      DRV_TYPE_WRITABLE_32BIT,
> +				      DRV_TYPE_WRITABLE_32BIT,
> +				      0,
> +				      PULL_TYPE_WRITABLE_32BIT,
> +				      PULL_TYPE_WRITABLE_32BIT,
> +				      PULL_TYPE_WRITABLE_32BIT,
> +				      0
> 			    ),
> 	PIN_BANK_IOMUX_FLAGS(1, 32, "gpio1", IOMUX_UNROUTED,
> 					     IOMUX_UNROUTED,
> diff --git a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
> index b84b079064..ce935656f0 100644
> --- a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
> +++ b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
> @@ -228,7 +228,13 @@ static int rockchip_set_mux(struct rockchip_pin_bank *bank, int pin, int mux)
> 		}
> 	}
> 
> -	data = (mask << (bit + 16));
> +	if (mux_type & IOMUX_WRITABLE_32BIT) {
> +		regmap_read(regmap, reg, &data);
> +		data &= ~(mask << bit);
> +	} else {
> +		data = (mask << (bit + 16));
> +	}
> +

This can not be optimised out by the compiler (for all the other platforms that don’t need it).
Please structure this (and the entire driver) to not pull in unneeded baggage that is only used
by one or a few devices.

> 	data |= (mux & mask) << bit;
> 	ret = regmap_write(regmap, reg, data);
> 
> @@ -252,7 +258,8 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
> 	int reg, ret, i;
> 	u32 data, rmask_bits, temp;
> 	u8 bit;
> -	int drv_type = bank->drv[pin_num / 8].drv_type;
> +	/* Where need to clean the special mask for rockchip_perpin_drv_list */
> +	int drv_type = bank->drv[pin_num / 8].drv_type & (~DRV_TYPE_IO_MASK);
> 
> 	debug("setting drive of GPIO%d-%d to %d\n", bank->bank_num,
> 	      pin_num, strength);
> @@ -324,10 +331,15 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
> 		return -EINVAL;
> 	}
> 
> -	/* enable the write to the equivalent lower bits */
> -	data = ((1 << rmask_bits) - 1) << (bit + 16);
> -	data |= (ret << bit);
> +	if (bank->drv[pin_num / 8].drv_type & DRV_TYPE_WRITABLE_32BIT) {
> +		regmap_read(regmap, reg, &data);
> +		data &= ~(((1 << rmask_bits) - 1) << bit);
> +	} else {
> +		/* enable the write to the equivalent lower bits */
> +		data = ((1 << rmask_bits) - 1) << (bit + 16);
> +	}
> 
> +	data |= (ret << bit);
> 	ret = regmap_write(regmap, reg, data);
> 	return ret;
> }
> @@ -375,7 +387,11 @@ static int rockchip_set_pull(struct rockchip_pin_bank *bank,
> 	case RK3288:
> 	case RK3368:
> 	case RK3399:
> -		pull_type = bank->pull_type[pin_num / 8];
> +		/*
> +		 * Where need to clean the special mask for
> +		 * rockchip_pull_list.
> +		 */
> +		pull_type = bank->pull_type[pin_num / 8] & (~PULL_TYPE_IO_MASK);
> 		ret = -EINVAL;
> 		for (i = 0; i < ARRAY_SIZE(rockchip_pull_list[pull_type]);
> 			i++) {
> @@ -390,10 +406,15 @@ static int rockchip_set_pull(struct rockchip_pin_bank *bank,
> 			return ret;
> 		}
> 
> -		/* enable the write to the equivalent lower bits */
> -		data = ((1 << ROCKCHIP_PULL_BITS_PER_PIN) - 1) << (bit + 16);
> -		data |= (ret << bit);
> +		if (bank->pull_type[pin_num / 8] & PULL_TYPE_WRITABLE_32BIT) {
> +			regmap_read(regmap, reg, &data);
> +			data &= ~(((1 << ROCKCHIP_PULL_BITS_PER_PIN) - 1) << bit);
> +		} else {
> +			/* enable the write to the equivalent lower bits */
> +			data = ((1 << ROCKCHIP_PULL_BITS_PER_PIN) - 1) << (bit + 16);
> +		}
> 
> +		data |= (ret << bit);
> 		ret = regmap_write(regmap, reg, data);
> 		break;
> 	default:
> diff --git a/drivers/pinctrl/rockchip/pinctrl-rockchip.h b/drivers/pinctrl/rockchip/pinctrl-rockchip.h
> index bc809630c1..5a6849c996 100644
> --- a/drivers/pinctrl/rockchip/pinctrl-rockchip.h
> +++ b/drivers/pinctrl/rockchip/pinctrl-rockchip.h
> @@ -26,6 +26,7 @@ enum rockchip_pinctrl_type {
> #define IOMUX_SOURCE_PMU	BIT(2)
> #define IOMUX_UNROUTED		BIT(3)
> #define IOMUX_WIDTH_3BIT	BIT(4)
> +#define IOMUX_WRITABLE_32BIT	BIT(5)
> 
> /**
>  * Defined some common pins constants
> @@ -49,6 +50,9 @@ struct rockchip_iomux {
> 	int				offset;
> };
> 
> +#define DRV_TYPE_IO_MASK		GENMASK(31, 16)
> +#define DRV_TYPE_WRITABLE_32BIT		BIT(31)
> +
> /**
>  * enum type index corresponding to rockchip_perpin_drv_list arrays index.
>  */
> @@ -61,6 +65,9 @@ enum rockchip_pin_drv_type {
> 	DRV_TYPE_MAX
> };
> 
> +#define PULL_TYPE_IO_MASK		GENMASK(31, 16)
> +#define PULL_TYPE_WRITABLE_32BIT	BIT(31)
> +
> /**
>  * enum type index corresponding to rockchip_pull_list arrays index.
>  */
> @@ -200,6 +207,32 @@ struct rockchip_pin_bank {
> 		},							\
> 	}
> 
> +#define PIN_BANK_IOMUX_DRV_PULL_FLAGS(id, pins, label, iom0, iom1,	\
> +				      iom2, iom3, drv0, drv1, drv2,	\
> +				      drv3, pull0, pull1, pull2,	\
> +				      pull3)				\
> +	{								\
> +		.bank_num	= id,					\
> +		.nr_pins	= pins,					\
> +		.name		= label,				\
> +		.iomux		= {					\
> +			{ .type = iom0, .offset = -1 },			\
> +			{ .type = iom1, .offset = -1 },			\
> +			{ .type = iom2, .offset = -1 },			\
> +			{ .type = iom3, .offset = -1 },			\
> +		},							\
> +		.drv		= {					\
> +			{ .drv_type = drv0, .offset = -1 },		\
> +			{ .drv_type = drv1, .offset = -1 },		\
> +			{ .drv_type = drv2, .offset = -1 },		\
> +			{ .drv_type = drv3, .offset = -1 },		\
> +		},							\
> +		.pull_type[0] = pull0,					\
> +		.pull_type[1] = pull1,					\
> +		.pull_type[2] = pull2,					\
> +		.pull_type[3] = pull3,					\
> +	}
> +
> #define PIN_BANK_IOMUX_FLAGS_DRV_FLAGS_OFFSET_PULL_FLAGS(id, pins,	\
> 					      label, iom0, iom1, iom2,  \
> 					      iom3, drv0, drv1, drv2,   \
> -- 
> 2.19.1
> 
> 
>
Philipp Tomsich March 29, 2019, 2:40 p.m. UTC | #2
David,

I am applying this one as a last-minute fix for 2019.4.
However, I’ll still need the v2 for the next cycle, as I my review comments still apply.

Thanks,
Philipp.

> On 12.02.2019, at 12:55, Philipp Tomsich <philipp.tomsich@theobroma-systems.com> wrote:
> 
> 
> 
>> On 12.02.2019, at 12:51, David Wu <david.wu@rock-chips.com> wrote:
>> 
>> There are no higher 16 writing corresponding bits for pmu_gpio0's
>> iomux/drive/pull at rk3288, need to read the value from register
>> firstly. Add the flag to distinguish it from normal registers.
>> 
>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>> ---
>> drivers/pinctrl/rockchip/pinctrl-rk3288.c     | 17 ++++++--
>> .../pinctrl/rockchip/pinctrl-rockchip-core.c  | 39 ++++++++++++++-----
>> drivers/pinctrl/rockchip/pinctrl-rockchip.h   | 33 ++++++++++++++++
>> 3 files changed, 76 insertions(+), 13 deletions(-)
>> 
>> diff --git a/drivers/pinctrl/rockchip/pinctrl-rk3288.c b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
>> index 60585f3208..8b6ce11a63 100644
>> --- a/drivers/pinctrl/rockchip/pinctrl-rk3288.c
>> +++ b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
>> @@ -92,10 +92,19 @@ static void rk3288_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank,
>> }
>> 
>> static struct rockchip_pin_bank rk3288_pin_banks[] = {
>> -	PIN_BANK_IOMUX_FLAGS(0, 24, "gpio0", IOMUX_SOURCE_PMU,
>> -					     IOMUX_SOURCE_PMU,
>> -					     IOMUX_SOURCE_PMU,
>> -					     IOMUX_UNROUTED
>> +	PIN_BANK_IOMUX_DRV_PULL_FLAGS(0, 24, "gpio0",
>> +				      IOMUX_SOURCE_PMU | IOMUX_WRITABLE_32BIT,
>> +				      IOMUX_SOURCE_PMU | IOMUX_WRITABLE_32BIT,
>> +				      IOMUX_SOURCE_PMU | IOMUX_WRITABLE_32BIT,
>> +				      IOMUX_UNROUTED,
>> +				      DRV_TYPE_WRITABLE_32BIT,
>> +				      DRV_TYPE_WRITABLE_32BIT,
>> +				      DRV_TYPE_WRITABLE_32BIT,
>> +				      0,
>> +				      PULL_TYPE_WRITABLE_32BIT,
>> +				      PULL_TYPE_WRITABLE_32BIT,
>> +				      PULL_TYPE_WRITABLE_32BIT,
>> +				      0
>> 			    ),
>> 	PIN_BANK_IOMUX_FLAGS(1, 32, "gpio1", IOMUX_UNROUTED,
>> 					     IOMUX_UNROUTED,
>> diff --git a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
>> index b84b079064..ce935656f0 100644
>> --- a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
>> +++ b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
>> @@ -228,7 +228,13 @@ static int rockchip_set_mux(struct rockchip_pin_bank *bank, int pin, int mux)
>> 		}
>> 	}
>> 
>> -	data = (mask << (bit + 16));
>> +	if (mux_type & IOMUX_WRITABLE_32BIT) {
>> +		regmap_read(regmap, reg, &data);
>> +		data &= ~(mask << bit);
>> +	} else {
>> +		data = (mask << (bit + 16));
>> +	}
>> +
> 
> This can not be optimised out by the compiler (for all the other platforms that don’t need it).
> Please structure this (and the entire driver) to not pull in unneeded baggage that is only used
> by one or a few devices.
> 
>> 	data |= (mux & mask) << bit;
>> 	ret = regmap_write(regmap, reg, data);
>> 
>> @@ -252,7 +258,8 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
>> 	int reg, ret, i;
>> 	u32 data, rmask_bits, temp;
>> 	u8 bit;
>> -	int drv_type = bank->drv[pin_num / 8].drv_type;
>> +	/* Where need to clean the special mask for rockchip_perpin_drv_list */
>> +	int drv_type = bank->drv[pin_num / 8].drv_type & (~DRV_TYPE_IO_MASK);
>> 
>> 	debug("setting drive of GPIO%d-%d to %d\n", bank->bank_num,
>> 	      pin_num, strength);
>> @@ -324,10 +331,15 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
>> 		return -EINVAL;
>> 	}
>> 
>> -	/* enable the write to the equivalent lower bits */
>> -	data = ((1 << rmask_bits) - 1) << (bit + 16);
>> -	data |= (ret << bit);
>> +	if (bank->drv[pin_num / 8].drv_type & DRV_TYPE_WRITABLE_32BIT) {
>> +		regmap_read(regmap, reg, &data);
>> +		data &= ~(((1 << rmask_bits) - 1) << bit);
>> +	} else {
>> +		/* enable the write to the equivalent lower bits */
>> +		data = ((1 << rmask_bits) - 1) << (bit + 16);
>> +	}
>> 
>> +	data |= (ret << bit);
>> 	ret = regmap_write(regmap, reg, data);
>> 	return ret;
>> }
>> @@ -375,7 +387,11 @@ static int rockchip_set_pull(struct rockchip_pin_bank *bank,
>> 	case RK3288:
>> 	case RK3368:
>> 	case RK3399:
>> -		pull_type = bank->pull_type[pin_num / 8];
>> +		/*
>> +		 * Where need to clean the special mask for
>> +		 * rockchip_pull_list.
>> +		 */
>> +		pull_type = bank->pull_type[pin_num / 8] & (~PULL_TYPE_IO_MASK);
>> 		ret = -EINVAL;
>> 		for (i = 0; i < ARRAY_SIZE(rockchip_pull_list[pull_type]);
>> 			i++) {
>> @@ -390,10 +406,15 @@ static int rockchip_set_pull(struct rockchip_pin_bank *bank,
>> 			return ret;
>> 		}
>> 
>> -		/* enable the write to the equivalent lower bits */
>> -		data = ((1 << ROCKCHIP_PULL_BITS_PER_PIN) - 1) << (bit + 16);
>> -		data |= (ret << bit);
>> +		if (bank->pull_type[pin_num / 8] & PULL_TYPE_WRITABLE_32BIT) {
>> +			regmap_read(regmap, reg, &data);
>> +			data &= ~(((1 << ROCKCHIP_PULL_BITS_PER_PIN) - 1) << bit);
>> +		} else {
>> +			/* enable the write to the equivalent lower bits */
>> +			data = ((1 << ROCKCHIP_PULL_BITS_PER_PIN) - 1) << (bit + 16);
>> +		}
>> 
>> +		data |= (ret << bit);
>> 		ret = regmap_write(regmap, reg, data);
>> 		break;
>> 	default:
>> diff --git a/drivers/pinctrl/rockchip/pinctrl-rockchip.h b/drivers/pinctrl/rockchip/pinctrl-rockchip.h
>> index bc809630c1..5a6849c996 100644
>> --- a/drivers/pinctrl/rockchip/pinctrl-rockchip.h
>> +++ b/drivers/pinctrl/rockchip/pinctrl-rockchip.h
>> @@ -26,6 +26,7 @@ enum rockchip_pinctrl_type {
>> #define IOMUX_SOURCE_PMU	BIT(2)
>> #define IOMUX_UNROUTED		BIT(3)
>> #define IOMUX_WIDTH_3BIT	BIT(4)
>> +#define IOMUX_WRITABLE_32BIT	BIT(5)
>> 
>> /**
>> * Defined some common pins constants
>> @@ -49,6 +50,9 @@ struct rockchip_iomux {
>> 	int				offset;
>> };
>> 
>> +#define DRV_TYPE_IO_MASK		GENMASK(31, 16)
>> +#define DRV_TYPE_WRITABLE_32BIT		BIT(31)
>> +
>> /**
>> * enum type index corresponding to rockchip_perpin_drv_list arrays index.
>> */
>> @@ -61,6 +65,9 @@ enum rockchip_pin_drv_type {
>> 	DRV_TYPE_MAX
>> };
>> 
>> +#define PULL_TYPE_IO_MASK		GENMASK(31, 16)
>> +#define PULL_TYPE_WRITABLE_32BIT	BIT(31)
>> +
>> /**
>> * enum type index corresponding to rockchip_pull_list arrays index.
>> */
>> @@ -200,6 +207,32 @@ struct rockchip_pin_bank {
>> 		},							\
>> 	}
>> 
>> +#define PIN_BANK_IOMUX_DRV_PULL_FLAGS(id, pins, label, iom0, iom1,	\
>> +				      iom2, iom3, drv0, drv1, drv2,	\
>> +				      drv3, pull0, pull1, pull2,	\
>> +				      pull3)				\
>> +	{								\
>> +		.bank_num	= id,					\
>> +		.nr_pins	= pins,					\
>> +		.name		= label,				\
>> +		.iomux		= {					\
>> +			{ .type = iom0, .offset = -1 },			\
>> +			{ .type = iom1, .offset = -1 },			\
>> +			{ .type = iom2, .offset = -1 },			\
>> +			{ .type = iom3, .offset = -1 },			\
>> +		},							\
>> +		.drv		= {					\
>> +			{ .drv_type = drv0, .offset = -1 },		\
>> +			{ .drv_type = drv1, .offset = -1 },		\
>> +			{ .drv_type = drv2, .offset = -1 },		\
>> +			{ .drv_type = drv3, .offset = -1 },		\
>> +		},							\
>> +		.pull_type[0] = pull0,					\
>> +		.pull_type[1] = pull1,					\
>> +		.pull_type[2] = pull2,					\
>> +		.pull_type[3] = pull3,					\
>> +	}
>> +
>> #define PIN_BANK_IOMUX_FLAGS_DRV_FLAGS_OFFSET_PULL_FLAGS(id, pins,	\
>> 					      label, iom0, iom1, iom2,  \
>> 					      iom3, drv0, drv1, drv2,   \
>> -- 
>> 2.19.1
>> 
>> 
>> 
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de <mailto:U-Boot@lists.denx.de>
> https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot>
David Wu April 1, 2019, 10:06 a.m. UTC | #3
Hi Philipp,

在 2019/3/29 下午10:40, Philipp Tomsich 写道:
> David,
> 
> I am applying this one as a last-minute fix for 2019.4.
> However, I’ll still need the v2 for the next cycle, as I my review 
> comments still apply.
> 

Sorry, there was a lot of things in March.
I think I could push v2 at this week.

> Thanks,
> Philipp.
> 
>> On 12.02.2019, at 12:55, Philipp Tomsich 
>> <philipp.tomsich@theobroma-systems.com 
>> <mailto:philipp.tomsich@theobroma-systems.com>> wrote:
>>
>>
>>
>>> On 12.02.2019, at 12:51, David Wu <david.wu@rock-chips.com 
>>> <mailto:david.wu@rock-chips.com>> wrote:
>>>
>>> There are no higher 16 writing corresponding bits for pmu_gpio0's
>>> iomux/drive/pull at rk3288, need to read the value from register
>>> firstly. Add the flag to distinguish it from normal registers.
>>>
>>> Signed-off-by: David Wu <david.wu@rock-chips.com 
>>> <mailto:david.wu@rock-chips.com>>
>>> ---
>>> drivers/pinctrl/rockchip/pinctrl-rk3288.c     | 17 ++++++--
>>> .../pinctrl/rockchip/pinctrl-rockchip-core.c  | 39 ++++++++++++++-----
>>> drivers/pinctrl/rockchip/pinctrl-rockchip.h   | 33 ++++++++++++++++
>>> 3 files changed, 76 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/drivers/pinctrl/rockchip/pinctrl-rk3288.c 
>>> b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
>>> index 60585f3208..8b6ce11a63 100644
>>> --- a/drivers/pinctrl/rockchip/pinctrl-rk3288.c
>>> +++ b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
>>> @@ -92,10 +92,19 @@ static void rk3288_calc_drv_reg_and_bit(struct 
>>> rockchip_pin_bank *bank,
>>> }
>>>
>>> static struct rockchip_pin_bank rk3288_pin_banks[] = {
>>> -PIN_BANK_IOMUX_FLAGS(0, 24, "gpio0", IOMUX_SOURCE_PMU,
>>> -    IOMUX_SOURCE_PMU,
>>> -    IOMUX_SOURCE_PMU,
>>> -    IOMUX_UNROUTED
>>> +PIN_BANK_IOMUX_DRV_PULL_FLAGS(0, 24, "gpio0",
>>> +     IOMUX_SOURCE_PMU | IOMUX_WRITABLE_32BIT,
>>> +     IOMUX_SOURCE_PMU | IOMUX_WRITABLE_32BIT,
>>> +     IOMUX_SOURCE_PMU | IOMUX_WRITABLE_32BIT,
>>> +     IOMUX_UNROUTED,
>>> +     DRV_TYPE_WRITABLE_32BIT,
>>> +     DRV_TYPE_WRITABLE_32BIT,
>>> +     DRV_TYPE_WRITABLE_32BIT,
>>> +     0,
>>> +     PULL_TYPE_WRITABLE_32BIT,
>>> +     PULL_TYPE_WRITABLE_32BIT,
>>> +     PULL_TYPE_WRITABLE_32BIT,
>>> +     0
>>>    ),
>>> PIN_BANK_IOMUX_FLAGS(1, 32, "gpio1", IOMUX_UNROUTED,
>>>     IOMUX_UNROUTED,
>>> diff --git a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c 
>>> b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
>>> index b84b079064..ce935656f0 100644
>>> --- a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
>>> +++ b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
>>> @@ -228,7 +228,13 @@ static int rockchip_set_mux(struct 
>>> rockchip_pin_bank *bank, int pin, int mux)
>>> }
>>> }
>>>
>>> -data = (mask << (bit + 16));
>>> +if (mux_type & IOMUX_WRITABLE_32BIT) {
>>> +regmap_read(regmap, reg, &data);
>>> +data &= ~(mask << bit);
>>> +} else {
>>> +data = (mask << (bit + 16));
>>> +}
>>> +
>>
>> This can not be optimised out by the compiler (for all the other 
>> platforms that don’t need it).
>> Please structure this (and the entire driver) to not pull in unneeded 
>> baggage that is only used
>> by one or a few devices.
>>
>>> data |= (mux & mask) << bit;
>>> ret = regmap_write(regmap, reg, data);
>>>
>>> @@ -252,7 +258,8 @@ static int rockchip_set_drive_perpin(struct 
>>> rockchip_pin_bank *bank,
>>> int reg, ret, i;
>>> u32 data, rmask_bits, temp;
>>> u8 bit;
>>> -int drv_type = bank->drv[pin_num / 8].drv_type;
>>> +/* Where need to clean the special mask for rockchip_perpin_drv_list */
>>> +int drv_type = bank->drv[pin_num / 8].drv_type & (~DRV_TYPE_IO_MASK);
>>>
>>> debug("setting drive of GPIO%d-%d to %d\n", bank->bank_num,
>>>      pin_num, strength);
>>> @@ -324,10 +331,15 @@ static int rockchip_set_drive_perpin(struct 
>>> rockchip_pin_bank *bank,
>>> return -EINVAL;
>>> }
>>>
>>> -/* enable the write to the equivalent lower bits */
>>> -data = ((1 << rmask_bits) - 1) << (bit + 16);
>>> -data |= (ret << bit);
>>> +if (bank->drv[pin_num / 8].drv_type & DRV_TYPE_WRITABLE_32BIT) {
>>> +regmap_read(regmap, reg, &data);
>>> +data &= ~(((1 << rmask_bits) - 1) << bit);
>>> +} else {
>>> +/* enable the write to the equivalent lower bits */
>>> +data = ((1 << rmask_bits) - 1) << (bit + 16);
>>> +}
>>>
>>> +data |= (ret << bit);
>>> ret = regmap_write(regmap, reg, data);
>>> return ret;
>>> }
>>> @@ -375,7 +387,11 @@ static int rockchip_set_pull(struct 
>>> rockchip_pin_bank *bank,
>>> case RK3288:
>>> case RK3368:
>>> case RK3399:
>>> -pull_type = bank->pull_type[pin_num / 8];
>>> +/*
>>> +* Where need to clean the special mask for
>>> +* rockchip_pull_list.
>>> +*/
>>> +pull_type = bank->pull_type[pin_num / 8] & (~PULL_TYPE_IO_MASK);
>>> ret = -EINVAL;
>>> for (i = 0; i < ARRAY_SIZE(rockchip_pull_list[pull_type]);
>>> i++) {
>>> @@ -390,10 +406,15 @@ static int rockchip_set_pull(struct 
>>> rockchip_pin_bank *bank,
>>> return ret;
>>> }
>>>
>>> -/* enable the write to the equivalent lower bits */
>>> -data = ((1 << ROCKCHIP_PULL_BITS_PER_PIN) - 1) << (bit + 16);
>>> -data |= (ret << bit);
>>> +if (bank->pull_type[pin_num / 8] & PULL_TYPE_WRITABLE_32BIT) {
>>> +regmap_read(regmap, reg, &data);
>>> +data &= ~(((1 << ROCKCHIP_PULL_BITS_PER_PIN) - 1) << bit);
>>> +} else {
>>> +/* enable the write to the equivalent lower bits */
>>> +data = ((1 << ROCKCHIP_PULL_BITS_PER_PIN) - 1) << (bit + 16);
>>> +}
>>>
>>> +data |= (ret << bit);
>>> ret = regmap_write(regmap, reg, data);
>>> break;
>>> default:
>>> diff --git a/drivers/pinctrl/rockchip/pinctrl-rockchip.h 
>>> b/drivers/pinctrl/rockchip/pinctrl-rockchip.h
>>> index bc809630c1..5a6849c996 100644
>>> --- a/drivers/pinctrl/rockchip/pinctrl-rockchip.h
>>> +++ b/drivers/pinctrl/rockchip/pinctrl-rockchip.h
>>> @@ -26,6 +26,7 @@ enum rockchip_pinctrl_type {
>>> #define IOMUX_SOURCE_PMUBIT(2)
>>> #define IOMUX_UNROUTEDBIT(3)
>>> #define IOMUX_WIDTH_3BITBIT(4)
>>> +#define IOMUX_WRITABLE_32BITBIT(5)
>>>
>>> /**
>>> * Defined some common pins constants
>>> @@ -49,6 +50,9 @@ struct rockchip_iomux {
>>> intoffset;
>>> };
>>>
>>> +#define DRV_TYPE_IO_MASKGENMASK(31, 16)
>>> +#define DRV_TYPE_WRITABLE_32BITBIT(31)
>>> +
>>> /**
>>> * enum type index corresponding to rockchip_perpin_drv_list arrays index.
>>> */
>>> @@ -61,6 +65,9 @@ enum rockchip_pin_drv_type {
>>> DRV_TYPE_MAX
>>> };
>>>
>>> +#define PULL_TYPE_IO_MASKGENMASK(31, 16)
>>> +#define PULL_TYPE_WRITABLE_32BITBIT(31)
>>> +
>>> /**
>>> * enum type index corresponding to rockchip_pull_list arrays index.
>>> */
>>> @@ -200,6 +207,32 @@ struct rockchip_pin_bank {
>>> },\
>>> }
>>>
>>> +#define PIN_BANK_IOMUX_DRV_PULL_FLAGS(id, pins, label, iom0, iom1,\
>>> +     iom2, iom3, drv0, drv1, drv2,\
>>> +     drv3, pull0, pull1, pull2,\
>>> +     pull3)\
>>> +{\
>>> +.bank_num= id,\
>>> +.nr_pins= pins,\
>>> +.name= label,\
>>> +.iomux= {\
>>> +{ .type = iom0, .offset = -1 },\
>>> +{ .type = iom1, .offset = -1 },\
>>> +{ .type = iom2, .offset = -1 },\
>>> +{ .type = iom3, .offset = -1 },\
>>> +},\
>>> +.drv= {\
>>> +{ .drv_type = drv0, .offset = -1 },\
>>> +{ .drv_type = drv1, .offset = -1 },\
>>> +{ .drv_type = drv2, .offset = -1 },\
>>> +{ .drv_type = drv3, .offset = -1 },\
>>> +},\
>>> +.pull_type[0] = pull0,\
>>> +.pull_type[1] = pull1,\
>>> +.pull_type[2] = pull2,\
>>> +.pull_type[3] = pull3,\
>>> +}
>>> +
>>> #define PIN_BANK_IOMUX_FLAGS_DRV_FLAGS_OFFSET_PULL_FLAGS(id, pins,\
>>>      label, iom0, iom1, iom2,  \
>>>      iom3, drv0, drv1, drv2,   \
>>> --
>>> 2.19.1
>>>
>>>
>>>
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot@lists.denx.de <mailto:U-Boot@lists.denx.de>
>> https://lists.denx.de/listinfo/u-boot
>
diff mbox series

Patch

diff --git a/drivers/pinctrl/rockchip/pinctrl-rk3288.c b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
index 60585f3208..8b6ce11a63 100644
--- a/drivers/pinctrl/rockchip/pinctrl-rk3288.c
+++ b/drivers/pinctrl/rockchip/pinctrl-rk3288.c
@@ -92,10 +92,19 @@  static void rk3288_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank,
 }
 
 static struct rockchip_pin_bank rk3288_pin_banks[] = {
-	PIN_BANK_IOMUX_FLAGS(0, 24, "gpio0", IOMUX_SOURCE_PMU,
-					     IOMUX_SOURCE_PMU,
-					     IOMUX_SOURCE_PMU,
-					     IOMUX_UNROUTED
+	PIN_BANK_IOMUX_DRV_PULL_FLAGS(0, 24, "gpio0",
+				      IOMUX_SOURCE_PMU | IOMUX_WRITABLE_32BIT,
+				      IOMUX_SOURCE_PMU | IOMUX_WRITABLE_32BIT,
+				      IOMUX_SOURCE_PMU | IOMUX_WRITABLE_32BIT,
+				      IOMUX_UNROUTED,
+				      DRV_TYPE_WRITABLE_32BIT,
+				      DRV_TYPE_WRITABLE_32BIT,
+				      DRV_TYPE_WRITABLE_32BIT,
+				      0,
+				      PULL_TYPE_WRITABLE_32BIT,
+				      PULL_TYPE_WRITABLE_32BIT,
+				      PULL_TYPE_WRITABLE_32BIT,
+				      0
 			    ),
 	PIN_BANK_IOMUX_FLAGS(1, 32, "gpio1", IOMUX_UNROUTED,
 					     IOMUX_UNROUTED,
diff --git a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
index b84b079064..ce935656f0 100644
--- a/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
+++ b/drivers/pinctrl/rockchip/pinctrl-rockchip-core.c
@@ -228,7 +228,13 @@  static int rockchip_set_mux(struct rockchip_pin_bank *bank, int pin, int mux)
 		}
 	}
 
-	data = (mask << (bit + 16));
+	if (mux_type & IOMUX_WRITABLE_32BIT) {
+		regmap_read(regmap, reg, &data);
+		data &= ~(mask << bit);
+	} else {
+		data = (mask << (bit + 16));
+	}
+
 	data |= (mux & mask) << bit;
 	ret = regmap_write(regmap, reg, data);
 
@@ -252,7 +258,8 @@  static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 	int reg, ret, i;
 	u32 data, rmask_bits, temp;
 	u8 bit;
-	int drv_type = bank->drv[pin_num / 8].drv_type;
+	/* Where need to clean the special mask for rockchip_perpin_drv_list */
+	int drv_type = bank->drv[pin_num / 8].drv_type & (~DRV_TYPE_IO_MASK);
 
 	debug("setting drive of GPIO%d-%d to %d\n", bank->bank_num,
 	      pin_num, strength);
@@ -324,10 +331,15 @@  static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank,
 		return -EINVAL;
 	}
 
-	/* enable the write to the equivalent lower bits */
-	data = ((1 << rmask_bits) - 1) << (bit + 16);
-	data |= (ret << bit);
+	if (bank->drv[pin_num / 8].drv_type & DRV_TYPE_WRITABLE_32BIT) {
+		regmap_read(regmap, reg, &data);
+		data &= ~(((1 << rmask_bits) - 1) << bit);
+	} else {
+		/* enable the write to the equivalent lower bits */
+		data = ((1 << rmask_bits) - 1) << (bit + 16);
+	}
 
+	data |= (ret << bit);
 	ret = regmap_write(regmap, reg, data);
 	return ret;
 }
@@ -375,7 +387,11 @@  static int rockchip_set_pull(struct rockchip_pin_bank *bank,
 	case RK3288:
 	case RK3368:
 	case RK3399:
-		pull_type = bank->pull_type[pin_num / 8];
+		/*
+		 * Where need to clean the special mask for
+		 * rockchip_pull_list.
+		 */
+		pull_type = bank->pull_type[pin_num / 8] & (~PULL_TYPE_IO_MASK);
 		ret = -EINVAL;
 		for (i = 0; i < ARRAY_SIZE(rockchip_pull_list[pull_type]);
 			i++) {
@@ -390,10 +406,15 @@  static int rockchip_set_pull(struct rockchip_pin_bank *bank,
 			return ret;
 		}
 
-		/* enable the write to the equivalent lower bits */
-		data = ((1 << ROCKCHIP_PULL_BITS_PER_PIN) - 1) << (bit + 16);
-		data |= (ret << bit);
+		if (bank->pull_type[pin_num / 8] & PULL_TYPE_WRITABLE_32BIT) {
+			regmap_read(regmap, reg, &data);
+			data &= ~(((1 << ROCKCHIP_PULL_BITS_PER_PIN) - 1) << bit);
+		} else {
+			/* enable the write to the equivalent lower bits */
+			data = ((1 << ROCKCHIP_PULL_BITS_PER_PIN) - 1) << (bit + 16);
+		}
 
+		data |= (ret << bit);
 		ret = regmap_write(regmap, reg, data);
 		break;
 	default:
diff --git a/drivers/pinctrl/rockchip/pinctrl-rockchip.h b/drivers/pinctrl/rockchip/pinctrl-rockchip.h
index bc809630c1..5a6849c996 100644
--- a/drivers/pinctrl/rockchip/pinctrl-rockchip.h
+++ b/drivers/pinctrl/rockchip/pinctrl-rockchip.h
@@ -26,6 +26,7 @@  enum rockchip_pinctrl_type {
 #define IOMUX_SOURCE_PMU	BIT(2)
 #define IOMUX_UNROUTED		BIT(3)
 #define IOMUX_WIDTH_3BIT	BIT(4)
+#define IOMUX_WRITABLE_32BIT	BIT(5)
 
 /**
  * Defined some common pins constants
@@ -49,6 +50,9 @@  struct rockchip_iomux {
 	int				offset;
 };
 
+#define DRV_TYPE_IO_MASK		GENMASK(31, 16)
+#define DRV_TYPE_WRITABLE_32BIT		BIT(31)
+
 /**
  * enum type index corresponding to rockchip_perpin_drv_list arrays index.
  */
@@ -61,6 +65,9 @@  enum rockchip_pin_drv_type {
 	DRV_TYPE_MAX
 };
 
+#define PULL_TYPE_IO_MASK		GENMASK(31, 16)
+#define PULL_TYPE_WRITABLE_32BIT	BIT(31)
+
 /**
  * enum type index corresponding to rockchip_pull_list arrays index.
  */
@@ -200,6 +207,32 @@  struct rockchip_pin_bank {
 		},							\
 	}
 
+#define PIN_BANK_IOMUX_DRV_PULL_FLAGS(id, pins, label, iom0, iom1,	\
+				      iom2, iom3, drv0, drv1, drv2,	\
+				      drv3, pull0, pull1, pull2,	\
+				      pull3)				\
+	{								\
+		.bank_num	= id,					\
+		.nr_pins	= pins,					\
+		.name		= label,				\
+		.iomux		= {					\
+			{ .type = iom0, .offset = -1 },			\
+			{ .type = iom1, .offset = -1 },			\
+			{ .type = iom2, .offset = -1 },			\
+			{ .type = iom3, .offset = -1 },			\
+		},							\
+		.drv		= {					\
+			{ .drv_type = drv0, .offset = -1 },		\
+			{ .drv_type = drv1, .offset = -1 },		\
+			{ .drv_type = drv2, .offset = -1 },		\
+			{ .drv_type = drv3, .offset = -1 },		\
+		},							\
+		.pull_type[0] = pull0,					\
+		.pull_type[1] = pull1,					\
+		.pull_type[2] = pull2,					\
+		.pull_type[3] = pull3,					\
+	}
+
 #define PIN_BANK_IOMUX_FLAGS_DRV_FLAGS_OFFSET_PULL_FLAGS(id, pins,	\
 					      label, iom0, iom1, iom2,  \
 					      iom3, drv0, drv1, drv2,   \