All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] pinctrl: rockchip: Add 32bit writing function for rk3288 gpio0 pinctrl
@ 2019-02-12 11:51 David Wu
  2019-02-12 11:55 ` Philipp Tomsich
  0 siblings, 1 reply; 4+ messages in thread
From: David Wu @ 2019-02-12 11:51 UTC (permalink / raw)
  To: u-boot

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));
+	}
+
 	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

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] pinctrl: rockchip: Add 32bit writing function for rk3288 gpio0 pinctrl
  2019-02-12 11:51 [U-Boot] [PATCH] pinctrl: rockchip: Add 32bit writing function for rk3288 gpio0 pinctrl David Wu
@ 2019-02-12 11:55 ` Philipp Tomsich
  2019-03-29 14:40   ` Philipp Tomsich
  0 siblings, 1 reply; 4+ messages in thread
From: Philipp Tomsich @ 2019-02-12 11:55 UTC (permalink / raw)
  To: u-boot



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

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] pinctrl: rockchip: Add 32bit writing function for rk3288 gpio0 pinctrl
  2019-02-12 11:55 ` Philipp Tomsich
@ 2019-03-29 14:40   ` Philipp Tomsich
  2019-04-01 10:06     ` David Wu
  0 siblings, 1 reply; 4+ messages in thread
From: Philipp Tomsich @ 2019-03-29 14:40 UTC (permalink / raw)
  To: u-boot

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 at lists.denx.de <mailto:U-Boot@lists.denx.de>
> https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot>

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] pinctrl: rockchip: Add 32bit writing function for rk3288 gpio0 pinctrl
  2019-03-29 14:40   ` Philipp Tomsich
@ 2019-04-01 10:06     ` David Wu
  0 siblings, 0 replies; 4+ messages in thread
From: David Wu @ 2019-04-01 10:06 UTC (permalink / raw)
  To: u-boot

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 at lists.denx.de <mailto:U-Boot@lists.denx.de>
>> https://lists.denx.de/listinfo/u-boot
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-04-01 10:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-12 11:51 [U-Boot] [PATCH] pinctrl: rockchip: Add 32bit writing function for rk3288 gpio0 pinctrl David Wu
2019-02-12 11:55 ` Philipp Tomsich
2019-03-29 14:40   ` Philipp Tomsich
2019-04-01 10:06     ` David Wu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.