lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4994384.31r3eYUQgx@workhorse>
Date: Mon, 17 Mar 2025 09:28:55 +0100
From: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
To: Jonas Karlman <jonas@...boo.se>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
 Daniel Lezcano <daniel.lezcano@...aro.org>, Zhang Rui <rui.zhang@...el.com>,
 Lukasz Luba <lukasz.luba@....com>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
 Heiko Stuebner <heiko@...ech.de>, Ye Zhang <ye.zhang@...k-chips.com>,
 devicetree@...r.kernel.org, linux-pm@...r.kernel.org,
 Sebastian Reichel <sebastian.reichel@...labora.com>,
 linux-kernel@...r.kernel.org, linux-rockchip@...ts.infradead.org,
 kernel@...labora.com, linux-arm-kernel@...ts.infradead.org
Subject:
 Re: [PATCH v3 3/6] thermal: rockchip: Support RK3576 SoC in the thermal
 driver

On Saturday, 15 March 2025 09:20:07 Central European Standard Time Jonas 
Karlman wrote:
> Hi Nicolas,

Hi Jonas,

> 
> On 2025-02-28 21:06, Nicolas Frattaroli wrote:
> > From: Ye Zhang <ye.zhang@...k-chips.com>
> > 
> > The RK3576 SoC has six TS-ADC channels: TOP, BIG_CORE, LITTLE_CORE,
> > DDR, NPU and GPU.
> > 
> > Signed-off-by: Ye Zhang <ye.zhang@...k-chips.com>
> > [ported to mainline, reworded commit message]
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
> > ---
> > 
> >  drivers/thermal/rockchip_thermal.c | 42
> >  ++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
> > 
> > diff --git a/drivers/thermal/rockchip_thermal.c
> > b/drivers/thermal/rockchip_thermal.c index
> > a8ad85feb68fbb7ec8d79602b16c47838ecb3c00..bec1930bebd87859a7e519cfc9f05e1
> > 0b1c31e87 100644 --- a/drivers/thermal/rockchip_thermal.c
> > +++ b/drivers/thermal/rockchip_thermal.c
> > @@ -1061,6 +1061,22 @@ static void rk_tsadcv3_tshut_mode(int chn, void
> > __iomem *regs,> 
> >  	writel_relaxed(val_cru, regs + TSADCV3_HSHUT_CRU_INT_EN);
> >  
> >  }
> > 
> > +static void rk_tsadcv4_tshut_mode(int chn, void __iomem *regs,
> > +				  enum tshut_mode mode)
> > +{
> > +	u32 val_gpio, val_cru;
> > +
> > +	if (mode == TSHUT_MODE_GPIO) {
> > +		val_gpio = TSADCV2_INT_SRC_EN(chn) | 
TSADCV2_INT_SRC_EN_MASK(chn);
> > +		val_cru = TSADCV2_INT_SRC_EN_MASK(chn);
> > +	} else {
> > +		val_cru = TSADCV2_INT_SRC_EN(chn) | 
TSADCV2_INT_SRC_EN_MASK(chn);
> > +		val_gpio = TSADCV2_INT_SRC_EN_MASK(chn);
> > +	}
> > +	writel_relaxed(val_gpio, regs + TSADCV3_HSHUT_GPIO_INT_EN);
> > +	writel_relaxed(val_cru, regs + TSADCV3_HSHUT_CRU_INT_EN);
> > +}
> 
> This function is identical to rk_tsadcv3_tshut_mode() in mainline.
> 
> Should the v3 function be renamed to v4 in mainline to match vendor
> kernel to avoid confusion?

Good catch. Yes, I'll add a patch to rename the function before introducing 
new changes in v4, and get rid of the duplicate function.

> 
> > +
> > 
> >  static const struct rockchip_tsadc_chip px30_tsadc_data = {
> >  
> >  	/* cpu, gpu */
> >  	.chn_offset = 0,
> > 
> > @@ -1284,6 +1300,28 @@ static const struct rockchip_tsadc_chip
> > rk3568_tsadc_data = {> 
> >  	},
> >  
> >  };
> > 
> > +static const struct rockchip_tsadc_chip rk3576_tsadc_data = {
> > +	/* top, big_core, little_core, ddr, npu, gpu */
> > +	.chn_offset = 0,
> > +	.chn_num = 6, /* six channels for tsadc */
> > +	.tshut_mode = TSHUT_MODE_GPIO, /* default TSHUT via GPIO give PMIC 
*/
> > +	.tshut_polarity = TSHUT_LOW_ACTIVE, /* default TSHUT LOW ACTIVE */
> > +	.tshut_temp = 95000,
> 
> Here the default is GPIO and 95 deg, in DT node the default is override
> to CRU and 120 deg.
> 
> Any reason that is not the default here?

No reason, other than that this is what most Rockchip SoCs seem to do. RK3588 
does the same thing. The hardware power-on-reset state is to not have any 
tshut, so whatever the "default" should be is entirely made up by the driver 
in either case.

For the sake of being consistent, I'll keep it the same in v4, as RK3588 does. 
Otherwise, we'll have RK3576 and RK3588 do different things. If someone wants 
to change the default, then ideally this would be done in a follow-up series 
to make it consistent for all SoCs.

If that's alright with you, then I'll send out a v4.

> 
> Regards,
> Jonas
> 

Regards,
Nicolas Frattaroli

> > +	.initialize = rk_tsadcv8_initialize,
> > +	.irq_ack = rk_tsadcv4_irq_ack,
> > +	.control = rk_tsadcv4_control,
> > +	.get_temp = rk_tsadcv4_get_temp,
> > +	.set_alarm_temp = rk_tsadcv3_alarm_temp,
> > +	.set_tshut_temp = rk_tsadcv3_tshut_temp,
> > +	.set_tshut_mode = rk_tsadcv4_tshut_mode,
> > +	.table = {
> > +		.id = rk3588_code_table,
> > +		.length = ARRAY_SIZE(rk3588_code_table),
> > +		.data_mask = TSADCV4_DATA_MASK,
> > +		.mode = ADC_INCREMENT,
> > +	},
> > +};
> > +
> > 
> >  static const struct rockchip_tsadc_chip rk3588_tsadc_data = {
> >  
> >  	/* top, big_core0, big_core1, little_core, center, gpu, npu */
> >  	.chn_offset = 0,
> > 
> > @@ -1342,6 +1380,10 @@ static const struct of_device_id
> > of_rockchip_thermal_match[] = {> 
> >  		.compatible = "rockchip,rk3568-tsadc",
> >  		.data = (void *)&rk3568_tsadc_data,
> >  	
> >  	},
> > 
> > +	{
> > +		.compatible = "rockchip,rk3576-tsadc",
> > +		.data = (void *)&rk3576_tsadc_data,
> > +	},
> > 
> >  	{
> >  	
> >  		.compatible = "rockchip,rk3588-tsadc",
> >  		.data = (void *)&rk3588_tsadc_data,





Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ