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)
