Re: [PATCH 2/2] pinctrl: spacemit: k3: add K3 pin support

Yixun Lan <[email protected]>
Newsgroups org.u-boot-project.lists.u-boot
Message-ID <[email protected]>
Hi Bo,

On 14:55 Thu 27 Aug     , Bo Gan wrote:
> Hi Yixun,
> 
> On 8/20/26 06:01, Yixun Lan wrote:
> > Based on upstream Linux:
> >    7412311c4655 ("pinctrl: spacemit: k3: add initial pin support")
> >    3f20bdf71518 ("pinctrl: spacemit: k3: adjust drive strength and schmitter trigger")
> > 
> > K3 uses a different pin-to-register mapping than K1, so add a
> > K3-specific pin_to_reg path and pin/IO power-domain tables. Drive
> > strength expands to 4 bits with larger mA tables, and the shared
> > pinctrl data gains per-SoC drive_mask and drive-strength tables so
> > K1 and K3 can live in the same driver.
> > 
> > Signed-off-by: Yixun Lan <[email protected]>
> > ---
> >   drivers/pinctrl/spacemit/Kconfig      |   4 +-
> >   drivers/pinctrl/spacemit/pinctrl-k1.c | 291 ++++++++++++++++++++++++++++++----
> >   2 files changed, 259 insertions(+), 36 deletions(-)
> > 
> > diff --git a/drivers/pinctrl/spacemit/Kconfig b/drivers/pinctrl/spacemit/Kconfig
> > index 6aab89e160c..87cba336923 100644
> > --- a/drivers/pinctrl/spacemit/Kconfig
> > +++ b/drivers/pinctrl/spacemit/Kconfig
> > @@ -1,8 +1,8 @@
> >   config PINCTRL_SPACEMIT_K1
> > -	bool "Spacemit K1 SoC pinctrl driver"
> > +	bool "Spacemit K1/K3 SoC pinctrl driver"
> >   	depends on PINCTRL_GENERIC && DM
> >   	help
> > -	  Supports pin multiplexing control on Spacemit K1 SoCs.
> > +	  Supports pin multiplexing control on Spacemit K1 and K3 SoCs.
> >   
> >   	  The driver is controlled by a device tree node which contains both
> >   	  the GPIO definitions and pin control functions for each available
> > diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.c b/drivers/pinctrl/spacemit/pinctrl-k1.c
> > index 469c41c40c8..1eb20fd2956 100644
> > --- a/drivers/pinctrl/spacemit/pinctrl-k1.c
> > +++ b/drivers/pinctrl/spacemit/pinctrl-k1.c
> > @@ -30,8 +30,10 @@
> >   #define PAD_EDGE_CLEAR		BIT(6)
> >   #define PAD_SLEW_RATE		GENMASK(12, 11)
> >   #define PAD_SLEW_RATE_EN	BIT(7)
> > -#define PAD_SCHMITT		GENMASK(9, 8)
> > -#define PAD_DRIVE		GENMASK(12, 10)
> > +#define PAD_SCHMITT_K1		GENMASK(9, 8)
> > +#define PAD_DRIVE_K1		GENMASK(12, 10)
> > +#define PAD_SCHMITT_K3		BIT(8)
> > +#define PAD_DRIVE_K3		GENMASK(12, 9)
> >   #define PAD_PULLDOWN		BIT(13)
> >   #define PAD_PULLUP		BIT(14)
> >   #define PAD_PULL_EN		BIT(15)
> > @@ -43,6 +45,11 @@
> >   #define IO_PWR_DOMAIN_MMC_Kx	0x1c
> >   #define IO_PWR_DOMAIN_QSPI_K1	0x20
> >   
> > +#define IO_PWR_DOMAIN_GPIO1_K3  0x04
> > +#define IO_PWR_DOMAIN_GPIO5_K3  0x10
> > +#define IO_PWR_DOMAIN_GPIO4_K3  0x20
> > +#define IO_PWR_DOMAIN_QSPI_K3   0x2c
> > +
> >   #define IO_PWR_DOMAIN_V18EN	BIT(2)
> >   
> >   #define APBC_ASFAR		0x50
> > @@ -68,6 +75,12 @@ struct spacemit_pin_io {
> >   	unsigned int	reserved : 8;
> >   };
> >   
> > +struct spacemit_pin_drv_strength {
> > +	unsigned int	val : 8;
> > +	unsigned int	ma : 16;
> > +	unsigned int	reserved : 8;
> > +};
> > +
> >   struct spacemit_pinctrl_data {
> >   	struct spacemit_pin_io *io_pins;
> >   	int nr_io_pins;
> > @@ -78,6 +91,12 @@ struct spacemit_pinctrl_data {
> >   	int (*get_functions)(struct udevice *dev);
> >   	int (*get_io_type)(struct udevice *dev, unsigned int pin);
> >   	unsigned int (*pin_to_io_pd_offset)(unsigned int pin);
> > +
> > +	u32 drive_mask;
> > +	struct spacemit_pin_drv_strength *ds_1v8_tbl;
> > +	int ds_1v8_tbl_num;
> > +	struct spacemit_pin_drv_strength *ds_3v3_tbl;
> > +	int ds_3v3_tbl_num;
> >   };
> >   
> >   struct spacemit_pinctrl_priv {
> > @@ -92,12 +111,6 @@ struct spacemit_pin_mux_config {
> >   	u32				config;
> >   };
> >   
> > -struct spacemit_pin_drv_strength {
> > -	unsigned int	val : 8;
> > -	unsigned int	ma : 16;
> > -	unsigned int	reserved : 8;
> > -};
> > -
> >   static char pin_name[PINNAME_SIZE];
> >   
> >   /* External: IO voltage via external source, can be 1.8V or 3.3V */
> > @@ -280,35 +293,17 @@ static inline u32 spacemit_get_ds_ma(struct spacemit_pin_drv_strength *tbl,
> >   	return 0;
> >   }
> >   
> > -static inline u8 spacemit_get_drive_strength(enum spacemit_pin_io_type type,
> > +static inline u8 spacemit_get_drive_strength(struct spacemit_pinctrl_data *data,
> > +					     enum spacemit_pin_io_type type,
> >   					     u32 ma)
> >   {
> >   	switch (type) {
> >   	case IO_TYPE_1V8:
> > -		return spacemit_get_ds_value(spacemit_ds_1v8_tbl,
> > -					     ARRAY_SIZE(spacemit_ds_1v8_tbl),
> > -					     ma);
> > -	case IO_TYPE_3V3:
> > -		return spacemit_get_ds_value(spacemit_ds_3v3_tbl,
> > -					     ARRAY_SIZE(spacemit_ds_3v3_tbl),
> > -					     ma);
> > -	default:
> > -		return 0;
> > -	}
> > -}
> > -
> > -static inline u32 spacemit_get_drive_strength_ma(enum spacemit_pin_io_type type,
> > -						 u32 value)
> > -{
> > -	switch (type) {
> > -	case IO_TYPE_1V8:
> > -		return spacemit_get_ds_ma(spacemit_ds_1v8_tbl,
> > -					  ARRAY_SIZE(spacemit_ds_1v8_tbl),
> > -					  value & 0x6);
> > +		return spacemit_get_ds_value(data->ds_1v8_tbl,
> > +					     data->ds_1v8_tbl_num, ma);
> >   	case IO_TYPE_3V3:
> > -		return spacemit_get_ds_ma(spacemit_ds_3v3_tbl,
> > -					  ARRAY_SIZE(spacemit_ds_3v3_tbl),
> > -					  value);
> > +		return spacemit_get_ds_value(data->ds_3v3_tbl,
> > +					     data->ds_3v3_tbl_num, ma);
> >   	default:
> >   		return 0;
> >   	}
> > @@ -549,8 +544,9 @@ static int spacemit_pinconf_set(struct udevice *dev, unsigned int pin_selector,
> >   			dev_err(dev, "Invalid IO type (%d)\n", io_type);
> >   			return -EINVAL;
> >   		}
> > -		ds = spacemit_get_drive_strength(io_type, argument);
> > -		clrsetbits_le32(addr, PAD_DRIVE, FIELD_PREP(PAD_DRIVE, ds));
> > +		ds = spacemit_get_drive_strength(data, io_type, argument);
> > +		clrsetbits_le32(addr, data->drive_mask,
> > +				field_prep(data->drive_mask, ds));
> >   		break;
> >   	case PIN_CONFIG_POWER_SOURCE:
> >   		return spacemit_set_pin_power_source(dev, pin_selector, argument);
> > @@ -704,12 +700,239 @@ static const struct spacemit_pinctrl_data k1_pinctrl_data = {
> >   	.get_functions	= k1_get_functions,
> >   	.get_io_type	= k1_get_io_type,
> >   	.pin_to_io_pd_offset = spacemit_k1_pin_to_io_pd_offset,
> > +	.drive_mask	= PAD_DRIVE_K1,
> > +	.ds_1v8_tbl	= spacemit_ds_1v8_tbl,
> > +	.ds_1v8_tbl_num	= ARRAY_SIZE(spacemit_ds_1v8_tbl),
> > +	.ds_3v3_tbl	= spacemit_ds_3v3_tbl,
> > +	.ds_3v3_tbl_num	= ARRAY_SIZE(spacemit_ds_3v3_tbl),
> > +};
> > +
> 
> Can the following table be simplified using static function and switch
> cases? As your other series mentioned, the SPL has very strict size limit,
> so we'd better not define this huge data array.
> 
I think it's possible, will try to implement, and see how much space it
can save.. thanks for the suggestion

> > +static struct spacemit_pin_io k3_io_pins[] = {
> > +	{ 0, IO_TYPE_EXTERNAL, 0, },
> > +	{ 1, IO_TYPE_EXTERNAL, 0, },
..
> 

-- 
Yixun Lan (dlan)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.