| Message ID | 20220422170920.401914-1-sebastian.reichel@collabora.com |
|---|---|
| Headers | show |
| Series | Basic RK3588 Support | expand |
On Fri, Apr 22, 2022 at 7:09 PM Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > From: Jianqun Xu <jay.xu@rock-chips.com> > > Add V2.1 rockchip gpio controller type, which is part of the > RK3588 SoC. > > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com> > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> This looks like something that can just be merged to Bartosz tree as there are no dependencies on the other patches. Yours, Linus Walleij
Hi Sebastian, thanks for the patches, On Fri, Apr 22, 2022 at 7:09 PM Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > pinctrl/rockchip: add rk3588 support > pinctrl/rockchip: add error handling for pull/drive register getters I expect I can just apply these two to the pinctrl tree after giving Heiko some time to look at it? (And he is happy with them) No dependencies I think? > Sebastian Reichel (6): > dt-bindings: pinctrl: rockchip: add rk3588 I just applied this patch since it is trivial and evidently correct. Yours, Linus Walleij
On Fri, Apr 22, 2022 at 7:14 PM Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > From: Jianqun Xu <jay.xu@rock-chips.com> > > This adds the pin controller data for rk3588. > > Signed-off-by: Shengfei Xu <xsf@rock-chips.com> > Signed-off-by: Damon Ding <damon.ding@rock-chips.com> > Signed-off-by: Steven Liu <steven.liu@rock-chips.com> > Signed-off-by: Jon Lin <jon.lin@rock-chips.com> > Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com> > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com> > [port from vendor tree merging all fixes] > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Please funnel this through the SoC tree. Yours, Linus Walleij
Am Freitag, 22. April 2022, 19:09:13 CEST schrieb Sebastian Reichel: > Add error handling for the pull and driver register getters in preparation > for RK3588 support. > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> Reviewed-by: Heiko Stübner <heiko@sntech.de> > --- > drivers/pinctrl/pinctrl-rockchip.c | 168 ++++++++++++++++++----------- > drivers/pinctrl/pinctrl-rockchip.h | 4 +- > 2 files changed, 109 insertions(+), 63 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c > index a1b598b86aa9..012cd2f0d85b 100644 > --- a/drivers/pinctrl/pinctrl-rockchip.c > +++ b/drivers/pinctrl/pinctrl-rockchip.c > @@ -986,9 +986,9 @@ static int rockchip_set_mux(struct rockchip_pin_bank *bank, int pin, int mux) > #define PX30_PULL_PINS_PER_REG 8 > #define PX30_PULL_BANK_STRIDE 16 > > -static void px30_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int px30_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1008,6 +1008,8 @@ static void px30_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > *reg += ((pin_num / PX30_PULL_PINS_PER_REG) * 4); > *bit = (pin_num % PX30_PULL_PINS_PER_REG); > *bit *= PX30_PULL_BITS_PER_PIN; > + > + return 0; > } > > #define PX30_DRV_PMU_OFFSET 0x20 > @@ -1016,9 +1018,9 @@ static void px30_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > #define PX30_DRV_PINS_PER_REG 8 > #define PX30_DRV_BANK_STRIDE 16 > > -static void px30_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int px30_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1038,6 +1040,8 @@ static void px30_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > *reg += ((pin_num / PX30_DRV_PINS_PER_REG) * 4); > *bit = (pin_num % PX30_DRV_PINS_PER_REG); > *bit *= PX30_DRV_BITS_PER_PIN; > + > + return 0; > } > > #define PX30_SCHMITT_PMU_OFFSET 0x38 > @@ -1077,9 +1081,9 @@ static int px30_calc_schmitt_reg_and_bit(struct rockchip_pin_bank *bank, > #define RV1108_PULL_BITS_PER_PIN 2 > #define RV1108_PULL_BANK_STRIDE 16 > > -static void rv1108_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rv1108_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1098,6 +1102,8 @@ static void rv1108_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > *reg += ((pin_num / RV1108_PULL_PINS_PER_REG) * 4); > *bit = (pin_num % RV1108_PULL_PINS_PER_REG); > *bit *= RV1108_PULL_BITS_PER_PIN; > + > + return 0; > } > > #define RV1108_DRV_PMU_OFFSET 0x20 > @@ -1106,9 +1112,9 @@ static void rv1108_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > #define RV1108_DRV_PINS_PER_REG 8 > #define RV1108_DRV_BANK_STRIDE 16 > > -static void rv1108_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rv1108_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1128,6 +1134,8 @@ static void rv1108_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > *reg += ((pin_num / RV1108_DRV_PINS_PER_REG) * 4); > *bit = pin_num % RV1108_DRV_PINS_PER_REG; > *bit *= RV1108_DRV_BITS_PER_PIN; > + > + return 0; > } > > #define RV1108_SCHMITT_PMU_OFFSET 0x30 > @@ -1184,9 +1192,9 @@ static int rk3308_calc_schmitt_reg_and_bit(struct rockchip_pin_bank *bank, > #define RK2928_PULL_PINS_PER_REG 16 > #define RK2928_PULL_BANK_STRIDE 8 > > -static void rk2928_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk2928_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1196,13 +1204,15 @@ static void rk2928_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > *reg += (pin_num / RK2928_PULL_PINS_PER_REG) * 4; > > *bit = pin_num % RK2928_PULL_PINS_PER_REG; > + > + return 0; > }; > > #define RK3128_PULL_OFFSET 0x118 > > -static void rk3128_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3128_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1212,6 +1222,8 @@ static void rk3128_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > *reg += ((pin_num / RK2928_PULL_PINS_PER_REG) * 4); > > *bit = pin_num % RK2928_PULL_PINS_PER_REG; > + > + return 0; > } > > #define RK3188_PULL_OFFSET 0x164 > @@ -1220,9 +1232,9 @@ static void rk3128_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > #define RK3188_PULL_BANK_STRIDE 16 > #define RK3188_PULL_PMU_OFFSET 0x64 > > -static void rk3188_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3188_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1252,12 +1264,14 @@ static void rk3188_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > *bit = 7 - (pin_num % RK3188_PULL_PINS_PER_REG); > *bit *= RK3188_PULL_BITS_PER_PIN; > } > + > + return 0; > } > > #define RK3288_PULL_OFFSET 0x140 > -static void rk3288_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3288_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1281,6 +1295,8 @@ static void rk3288_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > *bit = (pin_num % RK3188_PULL_PINS_PER_REG); > *bit *= RK3188_PULL_BITS_PER_PIN; > } > + > + return 0; > } > > #define RK3288_DRV_PMU_OFFSET 0x70 > @@ -1289,9 +1305,9 @@ static void rk3288_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > #define RK3288_DRV_PINS_PER_REG 8 > #define RK3288_DRV_BANK_STRIDE 16 > > -static void rk3288_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3288_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1315,13 +1331,15 @@ static void rk3288_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > *bit = (pin_num % RK3288_DRV_PINS_PER_REG); > *bit *= RK3288_DRV_BITS_PER_PIN; > } > + > + return 0; > } > > #define RK3228_PULL_OFFSET 0x100 > > -static void rk3228_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3228_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1332,13 +1350,15 @@ static void rk3228_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > > *bit = (pin_num % RK3188_PULL_PINS_PER_REG); > *bit *= RK3188_PULL_BITS_PER_PIN; > + > + return 0; > } > > #define RK3228_DRV_GRF_OFFSET 0x200 > > -static void rk3228_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3228_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1349,13 +1369,15 @@ static void rk3228_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > > *bit = (pin_num % RK3288_DRV_PINS_PER_REG); > *bit *= RK3288_DRV_BITS_PER_PIN; > + > + return 0; > } > > #define RK3308_PULL_OFFSET 0xa0 > > -static void rk3308_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3308_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1366,13 +1388,15 @@ static void rk3308_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > > *bit = (pin_num % RK3188_PULL_PINS_PER_REG); > *bit *= RK3188_PULL_BITS_PER_PIN; > + > + return 0; > } > > #define RK3308_DRV_GRF_OFFSET 0x100 > > -static void rk3308_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3308_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1383,14 +1407,16 @@ static void rk3308_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > > *bit = (pin_num % RK3288_DRV_PINS_PER_REG); > *bit *= RK3288_DRV_BITS_PER_PIN; > + > + return 0; > } > > #define RK3368_PULL_GRF_OFFSET 0x100 > #define RK3368_PULL_PMU_OFFSET 0x10 > > -static void rk3368_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3368_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1414,14 +1440,16 @@ static void rk3368_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > *bit = (pin_num % RK3188_PULL_PINS_PER_REG); > *bit *= RK3188_PULL_BITS_PER_PIN; > } > + > + return 0; > } > > #define RK3368_DRV_PMU_OFFSET 0x20 > #define RK3368_DRV_GRF_OFFSET 0x200 > > -static void rk3368_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3368_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1445,15 +1473,17 @@ static void rk3368_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > *bit = (pin_num % RK3288_DRV_PINS_PER_REG); > *bit *= RK3288_DRV_BITS_PER_PIN; > } > + > + return 0; > } > > #define RK3399_PULL_GRF_OFFSET 0xe040 > #define RK3399_PULL_PMU_OFFSET 0x40 > #define RK3399_DRV_3BITS_PER_PIN 3 > > -static void rk3399_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3399_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1479,11 +1509,13 @@ static void rk3399_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > *bit = (pin_num % RK3188_PULL_PINS_PER_REG); > *bit *= RK3188_PULL_BITS_PER_PIN; > } > + > + return 0; > } > > -static void rk3399_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3399_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > int drv_num = (pin_num / 8); > @@ -1500,6 +1532,8 @@ static void rk3399_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > *bit = (pin_num % 8) * 3; > else > *bit = (pin_num % 8) * 2; > + > + return 0; > } > > #define RK3568_PULL_PMU_OFFSET 0x20 > @@ -1508,9 +1542,9 @@ static void rk3399_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > #define RK3568_PULL_PINS_PER_REG 8 > #define RK3568_PULL_BANK_STRIDE 0x10 > > -static void rk3568_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3568_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1531,6 +1565,8 @@ static void rk3568_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > *bit = (pin_num % RK3568_PULL_PINS_PER_REG); > *bit *= RK3568_PULL_BITS_PER_PIN; > } > + > + return 0; > } > > #define RK3568_DRV_PMU_OFFSET 0x70 > @@ -1539,9 +1575,9 @@ static void rk3568_calc_pull_reg_and_bit(struct rockchip_pin_bank *bank, > #define RK3568_DRV_PINS_PER_REG 2 > #define RK3568_DRV_BANK_STRIDE 0x40 > > -static void rk3568_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > - int pin_num, struct regmap **regmap, > - int *reg, u8 *bit) > +static int rk3568_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > + int pin_num, struct regmap **regmap, > + int *reg, u8 *bit) > { > struct rockchip_pinctrl *info = bank->drvdata; > > @@ -1562,6 +1598,8 @@ static void rk3568_calc_drv_reg_and_bit(struct rockchip_pin_bank *bank, > *bit = (pin_num % RK3568_DRV_PINS_PER_REG); > *bit *= RK3568_DRV_BITS_PER_PIN; > } > + > + return 0; > } > > static int rockchip_perpin_drv_list[DRV_TYPE_MAX][8] = { > @@ -1584,7 +1622,9 @@ static int rockchip_get_drive_perpin(struct rockchip_pin_bank *bank, > u8 bit; > int drv_type = bank->drv[pin_num / 8].drv_type; > > - ctrl->drv_calc_reg(bank, pin_num, ®map, ®, &bit); > + ret = ctrl->drv_calc_reg(bank, pin_num, ®map, ®, &bit); > + if (ret) > + return ret; > > switch (drv_type) { > case DRV_TYPE_IO_1V8_3V0_AUTO: > @@ -1664,7 +1704,9 @@ static int rockchip_set_drive_perpin(struct rockchip_pin_bank *bank, > dev_dbg(dev, "setting drive of GPIO%d-%d to %d\n", > bank->bank_num, pin_num, strength); > > - ctrl->drv_calc_reg(bank, pin_num, ®map, ®, &bit); > + ret = ctrl->drv_calc_reg(bank, pin_num, ®map, ®, &bit); > + if (ret) > + return ret; > if (ctrl->type == RK3568) { > rmask_bits = RK3568_DRV_BITS_PER_PIN; > ret = (1 << (strength + 1)) - 1; > @@ -1777,7 +1819,9 @@ static int rockchip_get_pull(struct rockchip_pin_bank *bank, int pin_num) > if (ctrl->type == RK3066B) > return PIN_CONFIG_BIAS_DISABLE; > > - ctrl->pull_calc_reg(bank, pin_num, ®map, ®, &bit); > + ret = ctrl->pull_calc_reg(bank, pin_num, ®map, ®, &bit); > + if (ret) > + return ret; > > ret = regmap_read(regmap, reg, &data); > if (ret) > @@ -1824,7 +1868,9 @@ static int rockchip_set_pull(struct rockchip_pin_bank *bank, > if (ctrl->type == RK3066B) > return pull ? -EINVAL : 0; > > - ctrl->pull_calc_reg(bank, pin_num, ®map, ®, &bit); > + ret = ctrl->pull_calc_reg(bank, pin_num, ®map, ®, &bit); > + if (ret) > + return ret; > > switch (ctrl->type) { > case RK2928: > diff --git a/drivers/pinctrl/pinctrl-rockchip.h b/drivers/pinctrl/pinctrl-rockchip.h > index 91f10279d084..4992a048acbc 100644 > --- a/drivers/pinctrl/pinctrl-rockchip.h > +++ b/drivers/pinctrl/pinctrl-rockchip.h > @@ -230,10 +230,10 @@ struct rockchip_pin_ctrl { > struct rockchip_mux_route_data *iomux_routes; > u32 niomux_routes; > > - void (*pull_calc_reg)(struct rockchip_pin_bank *bank, > + int (*pull_calc_reg)(struct rockchip_pin_bank *bank, > int pin_num, struct regmap **regmap, > int *reg, u8 *bit); > - void (*drv_calc_reg)(struct rockchip_pin_bank *bank, > + int (*drv_calc_reg)(struct rockchip_pin_bank *bank, > int pin_num, struct regmap **regmap, > int *reg, u8 *bit); > int (*schmitt_calc_reg)(struct rockchip_pin_bank *bank, >
22.04.2022 20:09, Sebastian Reichel пишет: > From: Yifeng Zhao <yifeng.zhao@rock-chips.com> > > The reset function build in the SDHCI will not reset the logic > circuit related to the tuning function, which may cause data > reading errors. Resetting the complete SDHCI controller through > the reset controller fixes the issue. > > Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com> > [rebase] > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > --- > drivers/mmc/host/sdhci-of-dwcmshc.c | 28 +++++++++++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c > index bac874ab0b33..d95ae6ca1256 100644 > --- a/drivers/mmc/host/sdhci-of-dwcmshc.c > +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c > @@ -15,6 +15,7 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_device.h> > +#include <linux/reset.h> > #include <linux/sizes.h> > > #include "sdhci-pltfm.h" > @@ -63,6 +64,7 @@ > struct rk3568_priv { > /* Rockchip specified optional clocks */ > struct clk_bulk_data rockchip_clks[RK3568_MAX_CLKS]; > + struct reset_control *reset; > u8 txclk_tapnum; > }; > > @@ -255,6 +257,23 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock > sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN); > } > > +static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask) > +{ > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host); > + struct rk35xx_priv *priv = dwc_priv->priv; > + > + if (mask & SDHCI_RESET_ALL) { > + if (!IS_ERR_OR_NULL(priv->reset)) { priv->reset can't be a error ptr since probe fails on error. > + reset_control_assert(priv->reset); > + udelay(1); > + reset_control_deassert(priv->reset); > + } > + } > + > + sdhci_reset(host, mask); > +} > + > static const struct sdhci_ops sdhci_dwcmshc_ops = { > .set_clock = sdhci_set_clock, > .set_bus_width = sdhci_set_bus_width, > @@ -269,7 +288,7 @@ static const struct sdhci_ops sdhci_dwcmshc_rk3568_ops = { > .set_bus_width = sdhci_set_bus_width, > .set_uhs_signaling = dwcmshc_set_uhs_signaling, > .get_max_clock = sdhci_pltfm_clk_get_max_clock, > - .reset = sdhci_reset, > + .reset = rk35xx_sdhci_reset, > .adma_write_desc = dwcmshc_adma_write_desc, > }; > > @@ -292,6 +311,13 @@ static int dwcmshc_rk3568_init(struct sdhci_host *host, struct dwcmshc_priv *dwc > int err; > struct rk3568_priv *priv = dwc_priv->priv; > > + priv->reset = devm_reset_control_array_get_exclusive(mmc_dev(host->mmc)); The devm_reset_control_array_get_exclusive() never returns NULL. The devm_reset_control_array_get_optional_exclusive(() may return NULL if reset is missing in DT, perhaps that's what you actually want? > + if (IS_ERR_OR_NULL(priv->reset)) { > + err = PTR_ERR(priv->reset); NULL isn't a error > + dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err); dev_err_probe()?
On 22/04/22 20:09, Sebastian Reichel wrote: > From: Yifeng Zhao <yifeng.zhao@rock-chips.com> > > The reset function build in the SDHCI will not reset the logic > circuit related to the tuning function, which may cause data > reading errors. Resetting the complete SDHCI controller through > the reset controller fixes the issue. > > Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com> > [rebase] > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci-of-dwcmshc.c | 28 +++++++++++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c > index bac874ab0b33..d95ae6ca1256 100644 > --- a/drivers/mmc/host/sdhci-of-dwcmshc.c > +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c > @@ -15,6 +15,7 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_device.h> > +#include <linux/reset.h> > #include <linux/sizes.h> > > #include "sdhci-pltfm.h" > @@ -63,6 +64,7 @@ > struct rk3568_priv { > /* Rockchip specified optional clocks */ > struct clk_bulk_data rockchip_clks[RK3568_MAX_CLKS]; > + struct reset_control *reset; > u8 txclk_tapnum; > }; > > @@ -255,6 +257,23 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock > sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN); > } > > +static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask) > +{ > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host); > + struct rk35xx_priv *priv = dwc_priv->priv; > + > + if (mask & SDHCI_RESET_ALL) { > + if (!IS_ERR_OR_NULL(priv->reset)) { > + reset_control_assert(priv->reset); > + udelay(1); > + reset_control_deassert(priv->reset); > + } > + } > + > + sdhci_reset(host, mask); > +} > + > static const struct sdhci_ops sdhci_dwcmshc_ops = { > .set_clock = sdhci_set_clock, > .set_bus_width = sdhci_set_bus_width, > @@ -269,7 +288,7 @@ static const struct sdhci_ops sdhci_dwcmshc_rk3568_ops = { > .set_bus_width = sdhci_set_bus_width, > .set_uhs_signaling = dwcmshc_set_uhs_signaling, > .get_max_clock = sdhci_pltfm_clk_get_max_clock, > - .reset = sdhci_reset, > + .reset = rk35xx_sdhci_reset, > .adma_write_desc = dwcmshc_adma_write_desc, > }; > > @@ -292,6 +311,13 @@ static int dwcmshc_rk3568_init(struct sdhci_host *host, struct dwcmshc_priv *dwc > int err; > struct rk3568_priv *priv = dwc_priv->priv; > > + priv->reset = devm_reset_control_array_get_exclusive(mmc_dev(host->mmc)); > + if (IS_ERR_OR_NULL(priv->reset)) { > + err = PTR_ERR(priv->reset); > + dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err); > + return err; > + } > + > priv->rockchip_clks[0].id = "axi"; > priv->rockchip_clks[1].id = "block"; > priv->rockchip_clks[2].id = "timer";
On 22/04/22 20:09, Sebastian Reichel wrote: > Prepare driver for rk3588 support by renaming the internal data > structures. > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci-of-dwcmshc.c | 46 ++++++++++++++--------------- > 1 file changed, 23 insertions(+), 23 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c > index d95ae6ca1256..54ae0268e002 100644 > --- a/drivers/mmc/host/sdhci-of-dwcmshc.c > +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c > @@ -56,14 +56,14 @@ > #define DLL_LOCK_WO_TMOUT(x) \ > ((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \ > (((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0)) > -#define RK3568_MAX_CLKS 3 > +#define RK35xx_MAX_CLKS 3 > > #define BOUNDARY_OK(addr, len) \ > ((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1))) > > -struct rk3568_priv { > +struct rk35xx_priv { > /* Rockchip specified optional clocks */ > - struct clk_bulk_data rockchip_clks[RK3568_MAX_CLKS]; > + struct clk_bulk_data rockchip_clks[RK35xx_MAX_CLKS]; > struct reset_control *reset; > u8 txclk_tapnum; > }; > @@ -178,7 +178,7 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock > { > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host); > - struct rk3568_priv *priv = dwc_priv->priv; > + struct rk35xx_priv *priv = dwc_priv->priv; > u8 txclk_tapnum = DLL_TXCLK_TAPNUM_DEFAULT; > u32 extra, reg; > int err; > @@ -283,7 +283,7 @@ static const struct sdhci_ops sdhci_dwcmshc_ops = { > .adma_write_desc = dwcmshc_adma_write_desc, > }; > > -static const struct sdhci_ops sdhci_dwcmshc_rk3568_ops = { > +static const struct sdhci_ops sdhci_dwcmshc_rk35xx_ops = { > .set_clock = dwcmshc_rk3568_set_clock, > .set_bus_width = sdhci_set_bus_width, > .set_uhs_signaling = dwcmshc_set_uhs_signaling, > @@ -298,18 +298,18 @@ static const struct sdhci_pltfm_data sdhci_dwcmshc_pdata = { > .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN, > }; > > -static const struct sdhci_pltfm_data sdhci_dwcmshc_rk3568_pdata = { > - .ops = &sdhci_dwcmshc_rk3568_ops, > +static const struct sdhci_pltfm_data sdhci_dwcmshc_rk35xx_pdata = { > + .ops = &sdhci_dwcmshc_rk35xx_ops, > .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN | > SDHCI_QUIRK_BROKEN_TIMEOUT_VAL, > .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN | > SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN, > }; > > -static int dwcmshc_rk3568_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv) > +static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv) > { > int err; > - struct rk3568_priv *priv = dwc_priv->priv; > + struct rk35xx_priv *priv = dwc_priv->priv; > > priv->reset = devm_reset_control_array_get_exclusive(mmc_dev(host->mmc)); > if (IS_ERR_OR_NULL(priv->reset)) { > @@ -321,14 +321,14 @@ static int dwcmshc_rk3568_init(struct sdhci_host *host, struct dwcmshc_priv *dwc > priv->rockchip_clks[0].id = "axi"; > priv->rockchip_clks[1].id = "block"; > priv->rockchip_clks[2].id = "timer"; > - err = devm_clk_bulk_get_optional(mmc_dev(host->mmc), RK3568_MAX_CLKS, > + err = devm_clk_bulk_get_optional(mmc_dev(host->mmc), RK35xx_MAX_CLKS, > priv->rockchip_clks); > if (err) { > dev_err(mmc_dev(host->mmc), "failed to get clocks %d\n", err); > return err; > } > > - err = clk_bulk_prepare_enable(RK3568_MAX_CLKS, priv->rockchip_clks); > + err = clk_bulk_prepare_enable(RK35xx_MAX_CLKS, priv->rockchip_clks); > if (err) { > dev_err(mmc_dev(host->mmc), "failed to enable clocks %d\n", err); > return err; > @@ -350,7 +350,7 @@ static int dwcmshc_rk3568_init(struct sdhci_host *host, struct dwcmshc_priv *dwc > static const struct of_device_id sdhci_dwcmshc_dt_ids[] = { > { > .compatible = "rockchip,rk3568-dwcmshc", > - .data = &sdhci_dwcmshc_rk3568_pdata, > + .data = &sdhci_dwcmshc_rk35xx_pdata, > }, > { > .compatible = "snps,dwcmshc-sdhci", > @@ -373,7 +373,7 @@ static int dwcmshc_probe(struct platform_device *pdev) > struct sdhci_pltfm_host *pltfm_host; > struct sdhci_host *host; > struct dwcmshc_priv *priv; > - struct rk3568_priv *rk_priv = NULL; > + struct rk35xx_priv *rk_priv = NULL; > const struct sdhci_pltfm_data *pltfm_data; > int err; > u32 extra; > @@ -428,8 +428,8 @@ static int dwcmshc_probe(struct platform_device *pdev) > host->mmc_host_ops.request = dwcmshc_request; > host->mmc_host_ops.hs400_enhanced_strobe = dwcmshc_hs400_enhanced_strobe; > > - if (pltfm_data == &sdhci_dwcmshc_rk3568_pdata) { > - rk_priv = devm_kzalloc(&pdev->dev, sizeof(struct rk3568_priv), GFP_KERNEL); > + if (pltfm_data == &sdhci_dwcmshc_rk35xx_pdata) { > + rk_priv = devm_kzalloc(&pdev->dev, sizeof(struct rk35xx_priv), GFP_KERNEL); > if (!rk_priv) { > err = -ENOMEM; > goto err_clk; > @@ -437,7 +437,7 @@ static int dwcmshc_probe(struct platform_device *pdev) > > priv->priv = rk_priv; > > - err = dwcmshc_rk3568_init(host, priv); > + err = dwcmshc_rk35xx_init(host, priv); > if (err) > goto err_clk; > } > @@ -454,7 +454,7 @@ static int dwcmshc_probe(struct platform_device *pdev) > clk_disable_unprepare(pltfm_host->clk); > clk_disable_unprepare(priv->bus_clk); > if (rk_priv) > - clk_bulk_disable_unprepare(RK3568_MAX_CLKS, > + clk_bulk_disable_unprepare(RK35xx_MAX_CLKS, > rk_priv->rockchip_clks); > free_pltfm: > sdhci_pltfm_free(pdev); > @@ -466,14 +466,14 @@ static int dwcmshc_remove(struct platform_device *pdev) > struct sdhci_host *host = platform_get_drvdata(pdev); > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host); > - struct rk3568_priv *rk_priv = priv->priv; > + struct rk35xx_priv *rk_priv = priv->priv; > > sdhci_remove_host(host, 0); > > clk_disable_unprepare(pltfm_host->clk); > clk_disable_unprepare(priv->bus_clk); > if (rk_priv) > - clk_bulk_disable_unprepare(RK3568_MAX_CLKS, > + clk_bulk_disable_unprepare(RK35xx_MAX_CLKS, > rk_priv->rockchip_clks); > sdhci_pltfm_free(pdev); > > @@ -486,7 +486,7 @@ static int dwcmshc_suspend(struct device *dev) > struct sdhci_host *host = dev_get_drvdata(dev); > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host); > - struct rk3568_priv *rk_priv = priv->priv; > + struct rk35xx_priv *rk_priv = priv->priv; > int ret; > > ret = sdhci_suspend_host(host); > @@ -498,7 +498,7 @@ static int dwcmshc_suspend(struct device *dev) > clk_disable_unprepare(priv->bus_clk); > > if (rk_priv) > - clk_bulk_disable_unprepare(RK3568_MAX_CLKS, > + clk_bulk_disable_unprepare(RK35xx_MAX_CLKS, > rk_priv->rockchip_clks); > > return ret; > @@ -509,7 +509,7 @@ static int dwcmshc_resume(struct device *dev) > struct sdhci_host *host = dev_get_drvdata(dev); > struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host); > - struct rk3568_priv *rk_priv = priv->priv; > + struct rk35xx_priv *rk_priv = priv->priv; > int ret; > > ret = clk_prepare_enable(pltfm_host->clk); > @@ -523,7 +523,7 @@ static int dwcmshc_resume(struct device *dev) > } > > if (rk_priv) { > - ret = clk_bulk_prepare_enable(RK3568_MAX_CLKS, > + ret = clk_bulk_prepare_enable(RK35xx_MAX_CLKS, > rk_priv->rockchip_clks); > if (ret) > return ret;
On 22/04/22 20:09, Sebastian Reichel wrote: > From: Yifeng Zhao <yifeng.zhao@rock-chips.com> > > Add support for RK3588's DWCMSHC controller, which is used for > providing the rootfs on the RK3588 evaluation board. > > Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com> > [port from vendor BSP] > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> One comment otherwise: Acked-by: Adrian Hunter <adrian.hunter@intel.com> > --- > drivers/mmc/host/sdhci-of-dwcmshc.c | 113 +++++++++++++++++++++++----- > 1 file changed, 96 insertions(+), 17 deletions(-) > > diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c > index 54ae0268e002..bc365767e66c 100644 > --- a/drivers/mmc/host/sdhci-of-dwcmshc.c > +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c > @@ -31,6 +31,7 @@ > /* Offset inside the vendor area 1 */ > #define DWCMSHC_HOST_CTRL3 0x8 > #define DWCMSHC_EMMC_CONTROL 0x2c > +#define DWCMSHC_CARD_IS_EMMC BIT(0) > #define DWCMSHC_ENHANCED_STROBE BIT(8) > #define DWCMSHC_EMMC_ATCTRL 0x40 > > @@ -39,7 +40,7 @@ > #define DWCMSHC_EMMC_DLL_RXCLK 0x804 > #define DWCMSHC_EMMC_DLL_TXCLK 0x808 > #define DWCMSHC_EMMC_DLL_STRBIN 0x80c > -#define DLL_STRBIN_TAPNUM_FROM_SW BIT(24) > +#define DECMSHC_EMMC_DLL_CMDOUT 0x810 > #define DWCMSHC_EMMC_DLL_STATUS0 0x840 > #define DWCMSHC_EMMC_DLL_START BIT(0) > #define DWCMSHC_EMMC_DLL_LOCKED BIT(8) > @@ -48,11 +49,21 @@ > #define DWCMSHC_EMMC_DLL_START_POINT 16 > #define DWCMSHC_EMMC_DLL_INC 8 > #define DWCMSHC_EMMC_DLL_DLYENA BIT(27) > -#define DLL_TXCLK_TAPNUM_DEFAULT 0x8 > -#define DLL_STRBIN_TAPNUM_DEFAULT 0x8 > +#define DLL_TXCLK_TAPNUM_DEFAULT 0x10 > +#define DLL_TXCLK_TAPNUM_90_DEGREES 0xA > #define DLL_TXCLK_TAPNUM_FROM_SW BIT(24) > +#define DLL_STRBIN_TAPNUM_DEFAULT 0x8 > +#define DLL_STRBIN_TAPNUM_FROM_SW BIT(24) > +#define DLL_STRBIN_DELAY_NUM_SEL BIT(26) > +#define DLL_STRBIN_DELAY_NUM_OFFSET 16 > +#define DLL_STRBIN_DELAY_NUM_DEFAULT 0x16 > #define DLL_RXCLK_NO_INVERTER 1 > #define DLL_RXCLK_INVERTER 0 > +#define DLL_CMDOUT_TAPNUM_90_DEGREES 0x8 > +#define DLL_CMDOUT_TAPNUM_FROM_SW BIT(24) > +#define DLL_CMDOUT_SRC_CLK_NEG BIT(28) > +#define DLL_CMDOUT_EN_SRC_CLK_NEG BIT(29) > + > #define DLL_LOCK_WO_TMOUT(x) \ > ((((x) & DWCMSHC_EMMC_DLL_LOCKED) == DWCMSHC_EMMC_DLL_LOCKED) && \ > (((x) & DWCMSHC_EMMC_DLL_TIMEOUT) == 0)) > @@ -61,10 +72,16 @@ > #define BOUNDARY_OK(addr, len) \ > ((addr | (SZ_128M - 1)) == ((addr + len - 1) | (SZ_128M - 1))) > > +enum dwcmshc_rk_type { > + DWCMSHC_RK3568, > + DWCMSHC_RK3588, > +}; > + > struct rk35xx_priv { > /* Rockchip specified optional clocks */ > struct clk_bulk_data rockchip_clks[RK35xx_MAX_CLKS]; > struct reset_control *reset; > + enum dwcmshc_rk_type devtype; > u8 txclk_tapnum; > }; > > @@ -133,7 +150,9 @@ static void dwcmshc_request(struct mmc_host *mmc, struct mmc_request *mrq) > static void dwcmshc_set_uhs_signaling(struct sdhci_host *host, > unsigned int timing) > { > - u16 ctrl_2; > + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host); > + struct dwcmshc_priv *priv = sdhci_pltfm_priv(pltfm_host); > + u16 ctrl, ctrl_2; > > ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2); > /* Select Bus Speed Mode for host */ > @@ -151,8 +170,15 @@ static void dwcmshc_set_uhs_signaling(struct sdhci_host *host, > else if ((timing == MMC_TIMING_UHS_DDR50) || > (timing == MMC_TIMING_MMC_DDR52)) > ctrl_2 |= SDHCI_CTRL_UHS_DDR50; > - else if (timing == MMC_TIMING_MMC_HS400) > + else if (timing == MMC_TIMING_MMC_HS400) { > + /* set CARD_IS_EMMC bit to enable Data Strobe for HS400 */ > + ctrl = sdhci_readw(host, priv->vendor_specific_area1 + DWCMSHC_EMMC_CONTROL); > + ctrl |= DWCMSHC_CARD_IS_EMMC; > + sdhci_writew(host, ctrl, priv->vendor_specific_area1 + DWCMSHC_EMMC_CONTROL); > + > ctrl_2 |= DWCMSHC_CTRL_HS400; > + } > + > sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2); > } > > @@ -185,17 +211,11 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock > > host->mmc->actual_clock = 0; > > - /* > - * DO NOT TOUCH THIS SETTING. RX clk inverter unit is enabled > - * by default, but it shouldn't be enabled. We should anyway > - * disable it before issuing any cmds. > - */ > - extra = DWCMSHC_EMMC_DLL_DLYENA | > - DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL; > - sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK); > - > - if (clock == 0) > + if (clock == 0) { > + /* Disable interface clock at initial state. */ > + sdhci_set_clock(host, clock); > return; > + } > > /* Rockchip platform only support 375KHz for identify mode */ > if (clock <= 400000) > @@ -213,9 +233,21 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock > extra &= ~BIT(0); > sdhci_writel(host, extra, reg); > > - if (clock <= 400000) { > - /* Disable DLL to reset sample clock */ > + if (clock <= 52000000) { > + /* Disable DLL and reset both of sample and drive clock */ > sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_CTRL); > + sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_RXCLK); > + sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK); > + sdhci_writel(host, 0, DECMSHC_EMMC_DLL_CMDOUT); > + /* > + * Before switching to hs400es mode, the driver will enable > + * enhanced strobe first. PHY needs to configure the parameters > + * of enhanced strobe first. > + */ > + extra = DWCMSHC_EMMC_DLL_DLYENA | > + DLL_STRBIN_DELAY_NUM_SEL | > + DLL_STRBIN_DELAY_NUM_DEFAULT << DLL_STRBIN_DELAY_NUM_OFFSET; > + sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN); > return; > } > > @@ -224,6 +256,15 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock > udelay(1); > sdhci_writel(host, 0x0, DWCMSHC_EMMC_DLL_CTRL); > > + /* > + * We shouldn't set DLL_RXCLK_NO_INVERTER for identify mode but > + * we must set it in higher speed mode. > + */ > + extra = DWCMSHC_EMMC_DLL_DLYENA; > + if (priv->devtype == DWCMSHC_RK3568) > + extra |= DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL; > + sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK); > + > /* Init DLL settings */ > extra = 0x5 << DWCMSHC_EMMC_DLL_START_POINT | > 0x2 << DWCMSHC_EMMC_DLL_INC | > @@ -246,8 +287,20 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock > host->mmc->ios.timing == MMC_TIMING_MMC_HS400) > txclk_tapnum = priv->txclk_tapnum; > > + if ((priv->devtype == DWCMSHC_RK3588) && host->mmc->ios.timing == MMC_TIMING_MMC_HS400) { > + txclk_tapnum = DLL_TXCLK_TAPNUM_90_DEGREES; > + > + extra = DLL_CMDOUT_SRC_CLK_NEG | > + DLL_CMDOUT_EN_SRC_CLK_NEG | > + DWCMSHC_EMMC_DLL_DLYENA | > + DLL_CMDOUT_TAPNUM_90_DEGREES | > + DLL_CMDOUT_TAPNUM_FROM_SW; > + sdhci_writel(host, extra, DECMSHC_EMMC_DLL_CMDOUT); > + } > + > extra = DWCMSHC_EMMC_DLL_DLYENA | > DLL_TXCLK_TAPNUM_FROM_SW | > + DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL | > txclk_tapnum; > sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK); > > @@ -347,7 +400,25 @@ static int dwcmshc_rk35xx_init(struct sdhci_host *host, struct dwcmshc_priv *dwc > return 0; > } > > +static void dwcmshc_rk35xx_postinit(struct sdhci_host *host, struct dwcmshc_priv *dwc_priv) > +{ > + /* > + * Don't support highspeed bus mode with low clk speed as we > + * cannot use DLL for this condition. > + */ > + if (host->mmc->f_max <= 52000000) { > + dev_info(mmc_dev(host->mmc), "Disabling HS200/HS400, frequency too low (%d)\n", > + host->mmc->f_max); > + host->mmc->caps2 &= ~(MMC_CAP2_HS200 | MMC_CAP2_HS400); > + host->mmc->caps &= ~(MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR); Ideally, this should be done before mmc_add_host(), for example by using sdhci_setup_host() + __sdhci_add_host() instead of sdhci_add_host(), and putting dwcmshc_rk35xx_postinit() between sdhci_setup_host() and __sdhci_add_host(). > + } > +} > + > static const struct of_device_id sdhci_dwcmshc_dt_ids[] = { > + { > + .compatible = "rockchip,rk3588-dwcmshc", > + .data = &sdhci_dwcmshc_rk35xx_pdata, > + }, > { > .compatible = "rockchip,rk3568-dwcmshc", > .data = &sdhci_dwcmshc_rk35xx_pdata, > @@ -435,6 +506,11 @@ static int dwcmshc_probe(struct platform_device *pdev) > goto err_clk; > } > > + if (of_device_is_compatible(pdev->dev.of_node, "rockchip,rk3588-dwcmshc")) > + rk_priv->devtype = DWCMSHC_RK3588; > + else > + rk_priv->devtype = DWCMSHC_RK3568; > + > priv->priv = rk_priv; > > err = dwcmshc_rk35xx_init(host, priv); > @@ -448,6 +524,9 @@ static int dwcmshc_probe(struct platform_device *pdev) > if (err) > goto err_clk; > > + if (rk_priv) > + dwcmshc_rk35xx_postinit(host, priv); > + > return 0; > > err_clk:
Le vendredi 22 avril 2022 à 19:09 +0200, Sebastian Reichel a écrit : > From: Elaine Zhang <zhangqing@rock-chips.com> > > Add RK3588 PLL support including calculation of PLL parameters > for arbitrary frequencies. > > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > [rebase and partially rewrite code] > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > --- > drivers/clk/rockchip/clk-pll.c | 287 ++++++++++++++++++++++++++++++++- > drivers/clk/rockchip/clk.h | 18 +++ > 2 files changed, 304 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c > index f7827b3b7fc1..010e47eb51b8 100644 > --- a/drivers/clk/rockchip/clk-pll.c > +++ b/drivers/clk/rockchip/clk-pll.c > @@ -15,6 +15,7 @@ > #include <linux/iopoll.h> > #include <linux/regmap.h> > #include <linux/clk.h> > +#include <linux/units.h> > #include "clk.h" > > #define PLL_MODE_MASK 0x3 > @@ -47,6 +48,67 @@ struct rockchip_clk_pll { > #define to_rockchip_clk_pll_nb(nb) \ > container_of(nb, struct rockchip_clk_pll, clk_nb) > > +static int > +rockchip_rk3588_get_pll_settings(struct rockchip_clk_pll *pll, > + unsigned long fin_hz, > + unsigned long fout_hz, > + struct rockchip_pll_rate_table *rate_table) > +{ > + u64 fvco_min = 2250 * HZ_PER_MHZ, fvco_max = 4500 * HZ_PER_MHZ; > + u64 fout_min = 37 * HZ_PER_MHZ, fout_max = 4500 * HZ_PER_MHZ; > + u32 p, m, s; > + u64 fvco, fref, fout, ffrac; > + > + if (fin_hz == 0 || fout_hz == 0 || fout_hz == fin_hz) > + return -EINVAL; > + > + if (fout_hz > fout_max || fout_hz < fout_min) > + return -EINVAL; > + > + if (fin_hz / HZ_PER_MHZ * HZ_PER_MHZ == fin_hz && > + fout_hz / HZ_PER_MHZ * HZ_PER_MHZ == fout_hz) { > + for (s = 0; s <= 6; s++) { > + fvco = fout_hz << s; > + if (fvco < fvco_min || fvco > fvco_max) > + continue; > + for (p = 2; p <= 4; p++) { > + for (m = 64; m <= 1023; m++) { > + if (fvco == m * fin_hz / p) { > + rate_table->p = p; > + rate_table->m = m; > + rate_table->s = s; > + rate_table->k = 0; > + return 0; > + } > + } > + } > + } > + } else { > + fout = (fout_hz / HZ_PER_MHZ) * HZ_PER_MHZ; > + ffrac = (fout_hz % HZ_PER_MHZ); > + for (s = 0; s <= 6; s++) { > + fvco = fout << s; > + if (fvco < fvco_min || fvco > fvco_max) > + continue; > + for (p = 1; p <= 4; p++) { > + for (m = 64; m <= 1023; m++) { > + if (fvco == m * fin_hz / p) { > + rate_table->p = p; > + rate_table->m = m; > + rate_table->s = s; > + fref = fin_hz / p; > + fout = (ffrac << s) * 65535; > + rate_table->k = fout / fref; > + return 0; > + } > + } > + } > + } > + } > + > + return -EINVAL; > +} > + > static const struct rockchip_pll_rate_table *rockchip_get_pll_settings( > struct rockchip_clk_pll *pll, unsigned long rate) > { > @@ -68,6 +130,14 @@ static long rockchip_pll_round_rate(struct clk_hw *hw, > const struct rockchip_pll_rate_table *rate_table = pll->rate_table; > int i; > > + if (pll->type == pll_rk3588 || pll->type == pll_rk3588_core) { > + long parent_rate = prate ? *prate : 24 * HZ_PER_MHZ; > + struct rockchip_pll_rate_table pll_settings; > + > + if (rockchip_rk3588_get_pll_settings(pll, parent_rate, drate, &pll_settings) >= 0) > + return pll_settings.rate; > + } > + I was reading some of previous backlog on why these formula have never been mainlines [0]. It looks like so far the statu quo was adopted. But if that was to change, I think this implementation is not aligned with the intent. If my understanding is right, the rate_table[] (if it exists) should be looked up first and the formula use as a fallback. [0] https://patchwork.kernel.org/project/linux-rockchip/patch/1470144852-20708-1-git-send-email-zhengxing@rock-chips.com/#19548765 > /* Assumming rate_table is in descending order */ > for (i = 0; i < pll->rate_count; i++) { > if (drate >= rate_table[i].rate) > @@ -842,6 +912,212 @@ static const struct clk_ops rockchip_rk3399_pll_clk_ops = { > .init = rockchip_rk3399_pll_init, > }; > > +/** > + * PLL used in RK3588 > + */ > + > +#define RK3588_PLLCON(i) (i * 0x4) > +#define RK3588_PLLCON0_M_MASK 0x3ff > +#define RK3588_PLLCON0_M_SHIFT 0 > +#define RK3588_PLLCON1_P_MASK 0x3f > +#define RK3588_PLLCON1_P_SHIFT 0 > +#define RK3588_PLLCON1_S_MASK 0x7 > +#define RK3588_PLLCON1_S_SHIFT 6 > +#define RK3588_PLLCON2_K_MASK 0xffff > +#define RK3588_PLLCON2_K_SHIFT 0 > +#define RK3588_PLLCON1_PWRDOWN BIT(13) > +#define RK3588_PLLCON6_LOCK_STATUS BIT(15) > + > +static int rockchip_rk3588_pll_wait_lock(struct rockchip_clk_pll *pll) > +{ > + u32 pllcon; > + int ret; > + > + /* > + * Lock time typical 250, max 500 input clock cycles @24MHz > + * So define a very safe maximum of 1000us, meaning 24000 cycles. > + */ > + ret = readl_relaxed_poll_timeout(pll->reg_base + RK3588_PLLCON(6), > + pllcon, > + pllcon & RK3588_PLLCON6_LOCK_STATUS, > + 0, 1000); > + if (ret) > + pr_err("%s: timeout waiting for pll to lock\n", __func__); > + > + return ret; > +} > + > +static void rockchip_rk3588_pll_get_params(struct rockchip_clk_pll *pll, > + struct rockchip_pll_rate_table *rate) > +{ > + u32 pllcon; > + > + pllcon = readl_relaxed(pll->reg_base + RK3588_PLLCON(0)); > + rate->m = ((pllcon >> RK3588_PLLCON0_M_SHIFT) & RK3588_PLLCON0_M_MASK); > + > + pllcon = readl_relaxed(pll->reg_base + RK3588_PLLCON(1)); > + rate->p = ((pllcon >> RK3588_PLLCON1_P_SHIFT) & RK3588_PLLCON1_P_MASK); > + rate->s = ((pllcon >> RK3588_PLLCON1_S_SHIFT) & RK3588_PLLCON1_S_MASK); > + > + pllcon = readl_relaxed(pll->reg_base + RK3588_PLLCON(2)); > + rate->k = ((pllcon >> RK3588_PLLCON2_K_SHIFT) & RK3588_PLLCON2_K_MASK); > +} > + > +static unsigned long rockchip_rk3588_pll_recalc_rate(struct clk_hw *hw, unsigned long prate) > +{ > + struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > + struct rockchip_pll_rate_table cur; > + u64 rate64 = prate, postdiv; > + > + rockchip_rk3588_pll_get_params(pll, &cur); > + > + rate64 *= cur.m; > + do_div(rate64, cur.p); > + > + if (cur.k) { > + /* fractional mode */ > + u64 frac_rate64 = prate * cur.k; > + > + postdiv = cur.p * 65535; > + do_div(frac_rate64, postdiv); > + rate64 += frac_rate64; > + } > + rate64 = rate64 >> cur.s; > + > + return (unsigned long)rate64; > +} > + > +static int rockchip_rk3588_pll_set_params(struct rockchip_clk_pll *pll, > + const struct rockchip_pll_rate_table *rate) > +{ > + const struct clk_ops *pll_mux_ops = pll->pll_mux_ops; > + struct clk_mux *pll_mux = &pll->pll_mux; > + struct rockchip_pll_rate_table cur; > + int rate_change_remuxed = 0; > + int cur_parent; > + int ret; > + > + pr_debug("%s: rate settings for %lu p: %d, m: %d, s: %d, k: %d\n", > + __func__, rate->rate, rate->p, rate->m, rate->s, rate->k); > + > + rockchip_rk3588_pll_get_params(pll, &cur); > + cur.rate = 0; > + > + if (pll->type == pll_rk3588) { > + cur_parent = pll_mux_ops->get_parent(&pll_mux->hw); > + if (cur_parent == PLL_MODE_NORM) { > + pll_mux_ops->set_parent(&pll_mux->hw, PLL_MODE_SLOW); > + rate_change_remuxed = 1; > + } > + } > + > + /* set pll power down */ > + writel(HIWORD_UPDATE(RK3588_PLLCON1_PWRDOWN, > + RK3588_PLLCON1_PWRDOWN, 0), > + pll->reg_base + RK3399_PLLCON(1)); > + > + /* update pll values */ > + writel_relaxed(HIWORD_UPDATE(rate->m, RK3588_PLLCON0_M_MASK, RK3588_PLLCON0_M_SHIFT), > + pll->reg_base + RK3399_PLLCON(0)); > + > + writel_relaxed(HIWORD_UPDATE(rate->p, RK3588_PLLCON1_P_MASK, RK3588_PLLCON1_P_SHIFT) | > + HIWORD_UPDATE(rate->s, RK3588_PLLCON1_S_MASK, RK3588_PLLCON1_S_SHIFT), > + pll->reg_base + RK3399_PLLCON(1)); > + > + writel_relaxed(HIWORD_UPDATE(rate->k, RK3588_PLLCON2_K_MASK, RK3588_PLLCON2_K_SHIFT), > + pll->reg_base + RK3399_PLLCON(2)); > + > + /* set pll power up */ > + writel(HIWORD_UPDATE(0, RK3588_PLLCON1_PWRDOWN, 0), > + pll->reg_base + RK3588_PLLCON(1)); > + > + /* wait for the pll to lock */ > + ret = rockchip_rk3588_pll_wait_lock(pll); > + if (ret) { > + pr_warn("%s: pll update unsuccessful, trying to restore old params\n", > + __func__); > + rockchip_rk3588_pll_set_params(pll, &cur); > + } > + > + if ((pll->type == pll_rk3588) && rate_change_remuxed) > + pll_mux_ops->set_parent(&pll_mux->hw, PLL_MODE_NORM); > + > + return ret; > +} > + > +static int rockchip_rk3588_pll_set_rate(struct clk_hw *hw, unsigned long drate, > + unsigned long prate) > +{ > + struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > + struct rockchip_pll_rate_table rate; > + unsigned long old_rate = rockchip_rk3588_pll_recalc_rate(hw, prate); > + > + pr_debug("%s: changing %s from %lu to %lu with a parent rate of %lu\n", > + __func__, __clk_get_name(hw->clk), old_rate, drate, prate); > + > + if (rockchip_rk3588_get_pll_settings(pll, prate, drate, &rate) < 0) { > + pr_err("%s: Invalid rate : %lu for pll clk %s\n", __func__, > + drate, __clk_get_name(hw->clk)); > + return -EINVAL; > + } > + > + return rockchip_rk3588_pll_set_params(pll, &rate); > +} > + > +static int rockchip_rk3588_pll_enable(struct clk_hw *hw) > +{ > + struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > + > + writel(HIWORD_UPDATE(0, RK3588_PLLCON1_PWRDOWN, 0), > + pll->reg_base + RK3588_PLLCON(1)); > + rockchip_rk3588_pll_wait_lock(pll); > + > + return 0; > +} > + > +static void rockchip_rk3588_pll_disable(struct clk_hw *hw) > +{ > + struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > + > + writel(HIWORD_UPDATE(RK3588_PLLCON1_PWRDOWN, RK3588_PLLCON1_PWRDOWN, 0), > + pll->reg_base + RK3588_PLLCON(1)); > +} > + > +static int rockchip_rk3588_pll_is_enabled(struct clk_hw *hw) > +{ > + struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > + u32 pllcon = readl(pll->reg_base + RK3588_PLLCON(1)); > + > + return !(pllcon & RK3588_PLLCON1_PWRDOWN); > +} > + > +static int rockchip_rk3588_pll_init(struct clk_hw *hw) > +{ > + struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > + > + if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE)) > + return 0; > + > + return 0; > +} > + > +static const struct clk_ops rockchip_rk3588_pll_clk_norate_ops = { > + .recalc_rate = rockchip_rk3588_pll_recalc_rate, > + .enable = rockchip_rk3588_pll_enable, > + .disable = rockchip_rk3588_pll_disable, > + .is_enabled = rockchip_rk3588_pll_is_enabled, > +}; > + > +static const struct clk_ops rockchip_rk3588_pll_clk_ops = { > + .recalc_rate = rockchip_rk3588_pll_recalc_rate, > + .round_rate = rockchip_pll_round_rate, > + .set_rate = rockchip_rk3588_pll_set_rate, > + .enable = rockchip_rk3588_pll_enable, > + .disable = rockchip_rk3588_pll_disable, > + .is_enabled = rockchip_rk3588_pll_is_enabled, > + .init = rockchip_rk3588_pll_init, > +}; > + > /* > * Common registering of pll clocks > */ > @@ -890,7 +1166,8 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, > if (pll_type == pll_rk3036 || > pll_type == pll_rk3066 || > pll_type == pll_rk3328 || > - pll_type == pll_rk3399) > + pll_type == pll_rk3399 || > + pll_type == pll_rk3588) > pll_mux->flags |= CLK_MUX_HIWORD_MASK; > > /* the actual muxing is xin24m, pll-output, xin32k */ > @@ -957,6 +1234,14 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, > else > init.ops = &rockchip_rk3399_pll_clk_ops; > break; > + case pll_rk3588: > + case pll_rk3588_core: > + if (!pll->rate_table) > + init.ops = &rockchip_rk3588_pll_clk_norate_ops; > + else > + init.ops = &rockchip_rk3588_pll_clk_ops; > + init.flags = flags; > + break; > default: > pr_warn("%s: Unknown pll type for pll clk %s\n", > __func__, name); > diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h > index 6aece7f07a7d..bf7c8d082fde 100644 > --- a/drivers/clk/rockchip/clk.h > +++ b/drivers/clk/rockchip/clk.h > @@ -221,6 +221,8 @@ enum rockchip_pll_type { > pll_rk3066, > pll_rk3328, > pll_rk3399, > + pll_rk3588, > + pll_rk3588_core, > }; > > #define RK3036_PLL_RATE(_rate, _refdiv, _fbdiv, _postdiv1, \ > @@ -253,6 +255,15 @@ enum rockchip_pll_type { > .nb = _nb, \ > } > > +#define RK3588_PLL_RATE(_rate, _p, _m, _s, _k) \ > +{ \ > + .rate = _rate##U, \ > + .p = _p, \ > + .m = _m, \ > + .s = _s, \ > + .k = _k, \ > +} > + > /** > * struct rockchip_clk_provider - information about clock provider > * @reg_base: virtual address for the register base. > @@ -288,6 +299,13 @@ struct rockchip_pll_rate_table { > unsigned int dsmpd; > unsigned int frac; > }; > + struct { > + /* for RK3588 */ > + unsigned int m; > + unsigned int p; > + unsigned int s; > + unsigned int k; > + }; > }; > }; > > -- > 2.35.1 > >
On Fri, Apr 22, 2022 at 7:09 PM Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > Add error handling for the pull and driver register getters in preparation > for RK3588 support. > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> Patch applied. Yours, Linus Walleij
On Fri, Apr 22, 2022 at 7:09 PM Sebastian Reichel <sebastian.reichel@collabora.com> wrote: > From: Jianqun Xu <jay.xu@rock-chips.com> > > Add pinctrl support for RK3588. > > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com> > [merged in downstream fixes, simplified register lookup logic for better > maintanence at the cost of a bit more static const memory and fixed some > incorrect registers] > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> Patch applied, unless Heiko does some loud protesting it stays in the tree. Yours, Linus Walleij
Am Mittwoch, 27. April 2022, 15:36:17 CEST schrieb Nicolas Dufresne: > Le vendredi 22 avril 2022 à 19:09 +0200, Sebastian Reichel a écrit : > > From: Elaine Zhang <zhangqing@rock-chips.com> > > > > Add RK3588 PLL support including calculation of PLL parameters > > for arbitrary frequencies. > > > > Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com> > > [rebase and partially rewrite code] > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > > --- > > drivers/clk/rockchip/clk-pll.c | 287 ++++++++++++++++++++++++++++++++- > > drivers/clk/rockchip/clk.h | 18 +++ > > 2 files changed, 304 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c > > index f7827b3b7fc1..010e47eb51b8 100644 > > --- a/drivers/clk/rockchip/clk-pll.c > > +++ b/drivers/clk/rockchip/clk-pll.c > > @@ -15,6 +15,7 @@ > > #include <linux/iopoll.h> > > #include <linux/regmap.h> > > #include <linux/clk.h> > > +#include <linux/units.h> > > #include "clk.h" > > > > #define PLL_MODE_MASK 0x3 > > @@ -47,6 +48,67 @@ struct rockchip_clk_pll { > > #define to_rockchip_clk_pll_nb(nb) \ > > container_of(nb, struct rockchip_clk_pll, clk_nb) > > > > +static int > > +rockchip_rk3588_get_pll_settings(struct rockchip_clk_pll *pll, > > + unsigned long fin_hz, > > + unsigned long fout_hz, > > + struct rockchip_pll_rate_table *rate_table) > > +{ > > + u64 fvco_min = 2250 * HZ_PER_MHZ, fvco_max = 4500 * HZ_PER_MHZ; > > + u64 fout_min = 37 * HZ_PER_MHZ, fout_max = 4500 * HZ_PER_MHZ; > > + u32 p, m, s; > > + u64 fvco, fref, fout, ffrac; > > + > > + if (fin_hz == 0 || fout_hz == 0 || fout_hz == fin_hz) > > + return -EINVAL; > > + > > + if (fout_hz > fout_max || fout_hz < fout_min) > > + return -EINVAL; > > + > > + if (fin_hz / HZ_PER_MHZ * HZ_PER_MHZ == fin_hz && > > + fout_hz / HZ_PER_MHZ * HZ_PER_MHZ == fout_hz) { > > + for (s = 0; s <= 6; s++) { > > + fvco = fout_hz << s; > > + if (fvco < fvco_min || fvco > fvco_max) > > + continue; > > + for (p = 2; p <= 4; p++) { > > + for (m = 64; m <= 1023; m++) { > > + if (fvco == m * fin_hz / p) { > > + rate_table->p = p; > > + rate_table->m = m; > > + rate_table->s = s; > > + rate_table->k = 0; > > + return 0; > > + } > > + } > > + } > > + } > > + } else { > > + fout = (fout_hz / HZ_PER_MHZ) * HZ_PER_MHZ; > > + ffrac = (fout_hz % HZ_PER_MHZ); > > + for (s = 0; s <= 6; s++) { > > + fvco = fout << s; > > + if (fvco < fvco_min || fvco > fvco_max) > > + continue; > > + for (p = 1; p <= 4; p++) { > > + for (m = 64; m <= 1023; m++) { > > + if (fvco == m * fin_hz / p) { > > + rate_table->p = p; > > + rate_table->m = m; > > + rate_table->s = s; > > + fref = fin_hz / p; > > + fout = (ffrac << s) * 65535; > > + rate_table->k = fout / fref; > > + return 0; > > + } > > + } > > + } > > + } > > + } > > + > > + return -EINVAL; > > +} > > + > > static const struct rockchip_pll_rate_table *rockchip_get_pll_settings( > > struct rockchip_clk_pll *pll, unsigned long rate) > > { > > @@ -68,6 +130,14 @@ static long rockchip_pll_round_rate(struct clk_hw *hw, > > const struct rockchip_pll_rate_table *rate_table = pll->rate_table; > > int i; > > > > + if (pll->type == pll_rk3588 || pll->type == pll_rk3588_core) { > > + long parent_rate = prate ? *prate : 24 * HZ_PER_MHZ; > > + struct rockchip_pll_rate_table pll_settings; > > + > > + if (rockchip_rk3588_get_pll_settings(pll, parent_rate, drate, &pll_settings) >= 0) > > + return pll_settings.rate; > > + } > > + > > I was reading some of previous backlog on why these formula have never been > mainlines [0]. It looks like so far the statu quo was adopted. But if that was > to change, I think this implementation is not aligned with the intent. If my > understanding is right, the rate_table[] (if it exists) should be looked up > first and the formula use as a fallback. real life products like the Chromebooks using rk3288 and rk3399 had a lot of fun with PLL rates (jitter and whatnot) and even had the underlying parameters of some rates changed (same rate but using different parameters to achive it) to reduce effects. When doing this rate-table + dynamic calculation you never really know which one will be taken I guess. When you hit the exact rate as in the table you might get an optimized one but when round-rate ends up like 1kHz above, you would then get a non-optimized calculated one. So I'm personally really in favor of just sticking with a curated rate-table. Heiko > [0] https://patchwork.kernel.org/project/linux-rockchip/patch/1470144852-20708-1-git-send-email-zhengxing@rock-chips.com/#19548765 > > > /* Assumming rate_table is in descending order */ > > for (i = 0; i < pll->rate_count; i++) { > > if (drate >= rate_table[i].rate) > > @@ -842,6 +912,212 @@ static const struct clk_ops rockchip_rk3399_pll_clk_ops = { > > .init = rockchip_rk3399_pll_init, > > }; > > > > +/** > > + * PLL used in RK3588 > > + */ > > + > > +#define RK3588_PLLCON(i) (i * 0x4) > > +#define RK3588_PLLCON0_M_MASK 0x3ff > > +#define RK3588_PLLCON0_M_SHIFT 0 > > +#define RK3588_PLLCON1_P_MASK 0x3f > > +#define RK3588_PLLCON1_P_SHIFT 0 > > +#define RK3588_PLLCON1_S_MASK 0x7 > > +#define RK3588_PLLCON1_S_SHIFT 6 > > +#define RK3588_PLLCON2_K_MASK 0xffff > > +#define RK3588_PLLCON2_K_SHIFT 0 > > +#define RK3588_PLLCON1_PWRDOWN BIT(13) > > +#define RK3588_PLLCON6_LOCK_STATUS BIT(15) > > + > > +static int rockchip_rk3588_pll_wait_lock(struct rockchip_clk_pll *pll) > > +{ > > + u32 pllcon; > > + int ret; > > + > > + /* > > + * Lock time typical 250, max 500 input clock cycles @24MHz > > + * So define a very safe maximum of 1000us, meaning 24000 cycles. > > + */ > > + ret = readl_relaxed_poll_timeout(pll->reg_base + RK3588_PLLCON(6), > > + pllcon, > > + pllcon & RK3588_PLLCON6_LOCK_STATUS, > > + 0, 1000); > > + if (ret) > > + pr_err("%s: timeout waiting for pll to lock\n", __func__); > > + > > + return ret; > > +} > > + > > +static void rockchip_rk3588_pll_get_params(struct rockchip_clk_pll *pll, > > + struct rockchip_pll_rate_table *rate) > > +{ > > + u32 pllcon; > > + > > + pllcon = readl_relaxed(pll->reg_base + RK3588_PLLCON(0)); > > + rate->m = ((pllcon >> RK3588_PLLCON0_M_SHIFT) & RK3588_PLLCON0_M_MASK); > > + > > + pllcon = readl_relaxed(pll->reg_base + RK3588_PLLCON(1)); > > + rate->p = ((pllcon >> RK3588_PLLCON1_P_SHIFT) & RK3588_PLLCON1_P_MASK); > > + rate->s = ((pllcon >> RK3588_PLLCON1_S_SHIFT) & RK3588_PLLCON1_S_MASK); > > + > > + pllcon = readl_relaxed(pll->reg_base + RK3588_PLLCON(2)); > > + rate->k = ((pllcon >> RK3588_PLLCON2_K_SHIFT) & RK3588_PLLCON2_K_MASK); > > +} > > + > > +static unsigned long rockchip_rk3588_pll_recalc_rate(struct clk_hw *hw, unsigned long prate) > > +{ > > + struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > > + struct rockchip_pll_rate_table cur; > > + u64 rate64 = prate, postdiv; > > + > > + rockchip_rk3588_pll_get_params(pll, &cur); > > + > > + rate64 *= cur.m; > > + do_div(rate64, cur.p); > > + > > + if (cur.k) { > > + /* fractional mode */ > > + u64 frac_rate64 = prate * cur.k; > > + > > + postdiv = cur.p * 65535; > > + do_div(frac_rate64, postdiv); > > + rate64 += frac_rate64; > > + } > > + rate64 = rate64 >> cur.s; > > + > > + return (unsigned long)rate64; > > +} > > + > > +static int rockchip_rk3588_pll_set_params(struct rockchip_clk_pll *pll, > > + const struct rockchip_pll_rate_table *rate) > > +{ > > + const struct clk_ops *pll_mux_ops = pll->pll_mux_ops; > > + struct clk_mux *pll_mux = &pll->pll_mux; > > + struct rockchip_pll_rate_table cur; > > + int rate_change_remuxed = 0; > > + int cur_parent; > > + int ret; > > + > > + pr_debug("%s: rate settings for %lu p: %d, m: %d, s: %d, k: %d\n", > > + __func__, rate->rate, rate->p, rate->m, rate->s, rate->k); > > + > > + rockchip_rk3588_pll_get_params(pll, &cur); > > + cur.rate = 0; > > + > > + if (pll->type == pll_rk3588) { > > + cur_parent = pll_mux_ops->get_parent(&pll_mux->hw); > > + if (cur_parent == PLL_MODE_NORM) { > > + pll_mux_ops->set_parent(&pll_mux->hw, PLL_MODE_SLOW); > > + rate_change_remuxed = 1; > > + } > > + } > > + > > + /* set pll power down */ > > + writel(HIWORD_UPDATE(RK3588_PLLCON1_PWRDOWN, > > + RK3588_PLLCON1_PWRDOWN, 0), > > + pll->reg_base + RK3399_PLLCON(1)); > > + > > + /* update pll values */ > > + writel_relaxed(HIWORD_UPDATE(rate->m, RK3588_PLLCON0_M_MASK, RK3588_PLLCON0_M_SHIFT), > > + pll->reg_base + RK3399_PLLCON(0)); > > + > > + writel_relaxed(HIWORD_UPDATE(rate->p, RK3588_PLLCON1_P_MASK, RK3588_PLLCON1_P_SHIFT) | > > + HIWORD_UPDATE(rate->s, RK3588_PLLCON1_S_MASK, RK3588_PLLCON1_S_SHIFT), > > + pll->reg_base + RK3399_PLLCON(1)); > > + > > + writel_relaxed(HIWORD_UPDATE(rate->k, RK3588_PLLCON2_K_MASK, RK3588_PLLCON2_K_SHIFT), > > + pll->reg_base + RK3399_PLLCON(2)); > > + > > + /* set pll power up */ > > + writel(HIWORD_UPDATE(0, RK3588_PLLCON1_PWRDOWN, 0), > > + pll->reg_base + RK3588_PLLCON(1)); > > + > > + /* wait for the pll to lock */ > > + ret = rockchip_rk3588_pll_wait_lock(pll); > > + if (ret) { > > + pr_warn("%s: pll update unsuccessful, trying to restore old params\n", > > + __func__); > > + rockchip_rk3588_pll_set_params(pll, &cur); > > + } > > + > > + if ((pll->type == pll_rk3588) && rate_change_remuxed) > > + pll_mux_ops->set_parent(&pll_mux->hw, PLL_MODE_NORM); > > + > > + return ret; > > +} > > + > > +static int rockchip_rk3588_pll_set_rate(struct clk_hw *hw, unsigned long drate, > > + unsigned long prate) > > +{ > > + struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > > + struct rockchip_pll_rate_table rate; > > + unsigned long old_rate = rockchip_rk3588_pll_recalc_rate(hw, prate); > > + > > + pr_debug("%s: changing %s from %lu to %lu with a parent rate of %lu\n", > > + __func__, __clk_get_name(hw->clk), old_rate, drate, prate); > > + > > + if (rockchip_rk3588_get_pll_settings(pll, prate, drate, &rate) < 0) { > > + pr_err("%s: Invalid rate : %lu for pll clk %s\n", __func__, > > + drate, __clk_get_name(hw->clk)); > > + return -EINVAL; > > + } > > + > > + return rockchip_rk3588_pll_set_params(pll, &rate); > > +} > > + > > +static int rockchip_rk3588_pll_enable(struct clk_hw *hw) > > +{ > > + struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > > + > > + writel(HIWORD_UPDATE(0, RK3588_PLLCON1_PWRDOWN, 0), > > + pll->reg_base + RK3588_PLLCON(1)); > > + rockchip_rk3588_pll_wait_lock(pll); > > + > > + return 0; > > +} > > + > > +static void rockchip_rk3588_pll_disable(struct clk_hw *hw) > > +{ > > + struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > > + > > + writel(HIWORD_UPDATE(RK3588_PLLCON1_PWRDOWN, RK3588_PLLCON1_PWRDOWN, 0), > > + pll->reg_base + RK3588_PLLCON(1)); > > +} > > + > > +static int rockchip_rk3588_pll_is_enabled(struct clk_hw *hw) > > +{ > > + struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > > + u32 pllcon = readl(pll->reg_base + RK3588_PLLCON(1)); > > + > > + return !(pllcon & RK3588_PLLCON1_PWRDOWN); > > +} > > + > > +static int rockchip_rk3588_pll_init(struct clk_hw *hw) > > +{ > > + struct rockchip_clk_pll *pll = to_rockchip_clk_pll(hw); > > + > > + if (!(pll->flags & ROCKCHIP_PLL_SYNC_RATE)) > > + return 0; > > + > > + return 0; > > +} > > + > > +static const struct clk_ops rockchip_rk3588_pll_clk_norate_ops = { > > + .recalc_rate = rockchip_rk3588_pll_recalc_rate, > > + .enable = rockchip_rk3588_pll_enable, > > + .disable = rockchip_rk3588_pll_disable, > > + .is_enabled = rockchip_rk3588_pll_is_enabled, > > +}; > > + > > +static const struct clk_ops rockchip_rk3588_pll_clk_ops = { > > + .recalc_rate = rockchip_rk3588_pll_recalc_rate, > > + .round_rate = rockchip_pll_round_rate, > > + .set_rate = rockchip_rk3588_pll_set_rate, > > + .enable = rockchip_rk3588_pll_enable, > > + .disable = rockchip_rk3588_pll_disable, > > + .is_enabled = rockchip_rk3588_pll_is_enabled, > > + .init = rockchip_rk3588_pll_init, > > +}; > > + > > /* > > * Common registering of pll clocks > > */ > > @@ -890,7 +1166,8 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, > > if (pll_type == pll_rk3036 || > > pll_type == pll_rk3066 || > > pll_type == pll_rk3328 || > > - pll_type == pll_rk3399) > > + pll_type == pll_rk3399 || > > + pll_type == pll_rk3588) > > pll_mux->flags |= CLK_MUX_HIWORD_MASK; > > > > /* the actual muxing is xin24m, pll-output, xin32k */ > > @@ -957,6 +1234,14 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, > > else > > init.ops = &rockchip_rk3399_pll_clk_ops; > > break; > > + case pll_rk3588: > > + case pll_rk3588_core: > > + if (!pll->rate_table) > > + init.ops = &rockchip_rk3588_pll_clk_norate_ops; > > + else > > + init.ops = &rockchip_rk3588_pll_clk_ops; > > + init.flags = flags; > > + break; > > default: > > pr_warn("%s: Unknown pll type for pll clk %s\n", > > __func__, name); > > diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h > > index 6aece7f07a7d..bf7c8d082fde 100644 > > --- a/drivers/clk/rockchip/clk.h > > +++ b/drivers/clk/rockchip/clk.h > > @@ -221,6 +221,8 @@ enum rockchip_pll_type { > > pll_rk3066, > > pll_rk3328, > > pll_rk3399, > > + pll_rk3588, > > + pll_rk3588_core, > > }; > > > > #define RK3036_PLL_RATE(_rate, _refdiv, _fbdiv, _postdiv1, \ > > @@ -253,6 +255,15 @@ enum rockchip_pll_type { > > .nb = _nb, \ > > } > > > > +#define RK3588_PLL_RATE(_rate, _p, _m, _s, _k) \ > > +{ \ > > + .rate = _rate##U, \ > > + .p = _p, \ > > + .m = _m, \ > > + .s = _s, \ > > + .k = _k, \ > > +} > > + > > /** > > * struct rockchip_clk_provider - information about clock provider > > * @reg_base: virtual address for the register base. > > @@ -288,6 +299,13 @@ struct rockchip_pll_rate_table { > > unsigned int dsmpd; > > unsigned int frac; > > }; > > + struct { > > + /* for RK3588 */ > > + unsigned int m; > > + unsigned int p; > > + unsigned int s; > > + unsigned int k; > > + }; > > }; > > }; > > > >
Hi, Am Freitag, 29. April 2022, 00:55:52 CEST schrieb Linus Walleij: > On Fri, Apr 22, 2022 at 7:09 PM Sebastian Reichel > <sebastian.reichel@collabora.com> wrote: > > > From: Jianqun Xu <jay.xu@rock-chips.com> > > > > Add pinctrl support for RK3588. > > > > Signed-off-by: Jianqun Xu <jay.xu@rock-chips.com> > > [merged in downstream fixes, simplified register lookup logic for better > > maintanence at the cost of a bit more static const memory and fixed some > > incorrect registers] > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com> > > Patch applied, unless Heiko does some loud protesting it stays > in the tree. I'm never loud ;-) Patch looked ok overall as well. I guess at some future point someone will need to refactor the small "if rk3588" but that can be done when another slightly different user appears and we have a feeling what the common approach could look like. Heiko