On 01/04/2019 08:43, Elaine Zhang wrote: > Based on the TSADC Tshut mode to select pinctrl, > instead of setting pinctrl based on architecture > (Not depends on pinctrl setting by "init" or "default"). > And it requires setting the tshut polarity before select pinctrl.
I'm not sure to fully read the description. Can you rephrase/elaborate the changelog? > Signed-off-by: Elaine Zhang <zhangq...@rock-chips.com> > --- > drivers/thermal/rockchip_thermal.c | 61 > +++++++++++++++++++++++++++++++------- > 1 file changed, 50 insertions(+), 11 deletions(-) > > diff --git a/drivers/thermal/rockchip_thermal.c > b/drivers/thermal/rockchip_thermal.c > index 9c7643d62ed7..faa6c7792155 100644 > --- a/drivers/thermal/rockchip_thermal.c > +++ b/drivers/thermal/rockchip_thermal.c > @@ -34,7 +34,7 @@ > */ > enum tshut_mode { > TSHUT_MODE_CRU = 0, > - TSHUT_MODE_GPIO, > + TSHUT_MODE_OTP, Why do you change the enum name? The impact on the patch is much higher, no ? > }; > > /** > @@ -172,6 +172,9 @@ struct rockchip_thermal_data { > int tshut_temp; > enum tshut_mode tshut_mode; > enum tshut_polarity tshut_polarity; > + struct pinctrl *pinctrl; > + struct pinctrl_state *gpio_state; > + struct pinctrl_state *otp_state; > }; > > /** > @@ -807,7 +810,7 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem > *regs, > u32 val; > > val = readl_relaxed(regs + TSADCV2_INT_EN); > - if (mode == TSHUT_MODE_GPIO) { > + if (mode == TSHUT_MODE_OTP) { > val &= ~TSADCV2_SHUT_2CRU_SRC_EN(chn); > val |= TSADCV2_SHUT_2GPIO_SRC_EN(chn); > } else { > @@ -822,7 +825,7 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem > *regs, > .chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */ > .chn_num = 1, /* one channel for tsadc */ > > - .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */ > + .tshut_mode = TSHUT_MODE_OTP, /* default TSHUT via GPIO give PMIC */ > .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */ > .tshut_temp = 95000, > > @@ -846,7 +849,7 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem > *regs, > .chn_id[SENSOR_CPU] = 0, /* cpu sensor is channel 0 */ > .chn_num = 1, /* one channel for tsadc */ > > - .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */ > + .tshut_mode = TSHUT_MODE_OTP, /* default TSHUT via GPIO give PMIC */ > .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */ > .tshut_temp = 95000, > > @@ -871,7 +874,7 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem > *regs, > .chn_id[SENSOR_GPU] = 2, /* gpu sensor is channel 2 */ > .chn_num = 2, /* two channels for tsadc */ > > - .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */ > + .tshut_mode = TSHUT_MODE_OTP, /* default TSHUT via GPIO give PMIC */ > .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */ > .tshut_temp = 95000, > > @@ -919,7 +922,7 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem > *regs, > .chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */ > .chn_num = 2, /* two channels for tsadc */ > > - .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */ > + .tshut_mode = TSHUT_MODE_OTP, /* default TSHUT via GPIO give PMIC */ > .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */ > .tshut_temp = 95000, > > @@ -944,7 +947,7 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem > *regs, > .chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */ > .chn_num = 2, /* two channels for tsadc */ > > - .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */ > + .tshut_mode = TSHUT_MODE_OTP, /* default TSHUT via GPIO give PMIC */ > .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */ > .tshut_temp = 95000, > > @@ -969,7 +972,7 @@ static void rk_tsadcv2_tshut_mode(int chn, void __iomem > *regs, > .chn_id[SENSOR_GPU] = 1, /* gpu sensor is channel 1 */ > .chn_num = 2, /* two channels for tsadc */ > > - .tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC */ > + .tshut_mode = TSHUT_MODE_OTP, /* default TSHUT via GPIO give PMIC */ > .tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */ > .tshut_temp = 95000, > > @@ -1080,6 +1083,20 @@ static int rockchip_thermal_get_temp(void *_sensor, > int *out_temp) > .set_trips = rockchip_thermal_set_trips, > }; > > +static void thermal_pinctrl_select_otp(struct rockchip_thermal_data *thermal) > +{ > + if (!IS_ERR(thermal->pinctrl) && !IS_ERR_OR_NULL(thermal->otp_state)) > + pinctrl_select_state(thermal->pinctrl, > + thermal->otp_state); > +} > + > +static void thermal_pinctrl_select_gpio(struct rockchip_thermal_data > *thermal) > +{ > + if (!IS_ERR(thermal->pinctrl) && !IS_ERR_OR_NULL(thermal->gpio_state)) > + pinctrl_select_state(thermal->pinctrl, > + thermal->gpio_state); > +} You should not have to create a couple of specific functions just to check the pinctrl pointers are set. The caller should do that. > static int rockchip_configure_from_dt(struct device *dev, > struct device_node *np, > struct rockchip_thermal_data *thermal) > @@ -1103,7 +1120,7 @@ static int rockchip_configure_from_dt(struct device > *dev, > if (of_property_read_u32(np, "rockchip,hw-tshut-mode", &tshut_mode)) { > dev_warn(dev, > "Missing tshut mode property, using default (%s)\n", > - thermal->chip->tshut_mode == TSHUT_MODE_GPIO ? > + thermal->chip->tshut_mode == TSHUT_MODE_OTP ? > "gpio" : "cru"); > thermal->tshut_mode = thermal->chip->tshut_mode; > } else { > @@ -1242,6 +1259,8 @@ static int rockchip_thermal_probe(struct > platform_device *pdev) > return error; > } > > + thermal->chip->control(thermal->regs, false); > + > error = clk_prepare_enable(thermal->clk); > if (error) { > dev_err(&pdev->dev, "failed to enable converter clock: %d\n", > @@ -1267,6 +1286,24 @@ static int rockchip_thermal_probe(struct > platform_device *pdev) > thermal->chip->initialize(thermal->grf, thermal->regs, > thermal->tshut_polarity); > > + if (thermal->tshut_mode == TSHUT_MODE_OTP) { > + thermal->pinctrl = devm_pinctrl_get(&pdev->dev); > + if (IS_ERR(thermal->pinctrl)) > + dev_err(&pdev->dev, "failed to find thermal pinctrl\n"); > + > + thermal->gpio_state = pinctrl_lookup_state(thermal->pinctrl, > + "gpio"); > + if (IS_ERR_OR_NULL(thermal->gpio_state)) > + dev_err(&pdev->dev, "failed to find thermal gpio > state\n"); > + > + thermal->otp_state = pinctrl_lookup_state(thermal->pinctrl, > + "otpout"); > + if (IS_ERR_OR_NULL(thermal->otp_state)) > + dev_err(&pdev->dev, "failed to find thermal otpout > state\n"); What is the meaning for the rest of the code if the lookup fails for any of those ? > + thermal_pinctrl_select_otp(thermal); > + } > + > for (i = 0; i < thermal->chip->chn_num; i++) { > error = rockchip_thermal_register_sensor(pdev, thermal, > &thermal->sensors[i], > @@ -1338,7 +1375,8 @@ static int __maybe_unused > rockchip_thermal_suspend(struct device *dev) > clk_disable(thermal->pclk); > clk_disable(thermal->clk); > > - pinctrl_pm_select_sleep_state(dev); > + if (thermal->tshut_mode == TSHUT_MODE_OTP) > + thermal_pinctrl_select_gpio(thermal); > > return 0; > } > @@ -1383,7 +1421,8 @@ static int __maybe_unused > rockchip_thermal_resume(struct device *dev) > for (i = 0; i < thermal->chip->chn_num; i++) > rockchip_thermal_toggle_sensor(&thermal->sensors[i], true); > > - pinctrl_pm_select_default_state(dev); > + if (thermal->tshut_mode == TSHUT_MODE_OTP) > + thermal_pinctrl_select_otp(thermal); > > return 0; > } > -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog