thermal/drivers/mediatek/lvts_thermal: Remove redundant code in lvts_ctrl_configure

[PATCH] thermal/drivers/mediatek/lvts_thermal: Remove redundant code in lvts_ctrl_configure

Posted by Pin-yen Lin 1 year, 10 months ago

The removed variable assignment is never written to the register, so it
has no effect on the device behavior.  Mediatek has confirmed that it
is not required to initialize this register for current platforms, so
remove this segment to avoid confusion.

Signed-off-by: Pin-yen Lin <treapking@chromium.org>

---

 drivers/thermal/mediatek/lvts_thermal.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index fd4bd650c77a..48d2f8ba3f18 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -985,12 +985,6 @@ static int lvts_ctrl_configure(struct device *dev, struct lvts_ctrl *lvts_ctrl)
 	value = LVTS_TSSEL_CONF;
 	writel(value, LVTS_TSSEL(lvts_ctrl->base));
 
-	/*
-	 * LVTS_CALSCALE : ADC voltage round
-	 */
-	value = 0x300;
-	value = LVTS_CALSCALE_CONF;
-
 	/*
 	 * LVTS_MSRCTL0 : Sensor filtering strategy
 	 *
-- 
2.44.0.478.gd926399ef9-goog

Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: Remove redundant code in lvts_ctrl_configure

Posted by AngeloGioacchino Del Regno 1 year, 10 months ago

Il 10/04/24 02:07, Pin-yen Lin ha scritto:
> The removed variable assignment is never written to the register, so it
> has no effect on the device behavior.  Mediatek has confirmed that it
> is not required to initialize this register for current platforms, so
> remove this segment to avoid confusion.
> 
> Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> 

Added Nicolas Pitre to the loop;

Nicolas, since you're pushing support for those, can you please check if CALSCALE
is used/useful/necessary on MT8186/88 before I give a R-b to this commit?

P.S.: Can anyone from MediaTek please confirm if the 0x300 value is right for this
       register? I'd still like Linux to properly initialize the registers, even if
       this could technically be a default value.


> ---
> 
>   drivers/thermal/mediatek/lvts_thermal.c | 6 ------
>   1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
> index fd4bd650c77a..48d2f8ba3f18 100644
> --- a/drivers/thermal/mediatek/lvts_thermal.c
> +++ b/drivers/thermal/mediatek/lvts_thermal.c
> @@ -985,12 +985,6 @@ static int lvts_ctrl_configure(struct device *dev, struct lvts_ctrl *lvts_ctrl)
>   	value = LVTS_TSSEL_CONF;
>   	writel(value, LVTS_TSSEL(lvts_ctrl->base));
>   
> -	/*
> -	 * LVTS_CALSCALE : ADC voltage round
> -	 */
> -	value = 0x300;
> -	value = LVTS_CALSCALE_CONF;

As a side note, I believe that the original author wanted to write, instead...

	value = LVTS_CALSCALE_CONF;
	writel(value, LVTS_CALSCALE(lvts_ctrl->base);

Cheers,
Angelo

> -
>   	/*
>   	 * LVTS_MSRCTL0 : Sensor filtering strategy
>   	 *

Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: Remove redundant code in lvts_ctrl_configure

Posted by Chang-Yi Lin (林長億) 9 months, 1 week ago

On Wed, 2024-04-10 at 11:55 +0200, AngeloGioacchino Del Regno wrote:
> Il 10/04/24 02:07, Pin-yen Lin ha scritto:
> > The removed variable assignment is never written to the register,
> > so it
> > has no effect on the device behavior.  Mediatek has confirmed that
> > it
> > is not required to initialize this register for current platforms,
> > so
> > remove this segment to avoid confusion.
> > 
> > Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> > 
> 
> Added Nicolas Pitre to the loop;
> 
> Nicolas, since you're pushing support for those, can you please check
> if CALSCALE
> is used/useful/necessary on MT8186/88 before I give a R-b to this
> commit?
> 
> P.S.: Can anyone from MediaTek please confirm if the 0x300 value is
> right for this
>        register? I'd still like Linux to properly initialize the
> registers, even if
>        this could technically be a default value.
> 
> 
> > ---
> > 
> >   drivers/thermal/mediatek/lvts_thermal.c | 6 ------
> >   1 file changed, 6 deletions(-)
> > 
> > diff --git a/drivers/thermal/mediatek/lvts_thermal.c
> > b/drivers/thermal/mediatek/lvts_thermal.c
> > index fd4bd650c77a..48d2f8ba3f18 100644
> > --- a/drivers/thermal/mediatek/lvts_thermal.c
> > +++ b/drivers/thermal/mediatek/lvts_thermal.c
> > @@ -985,12 +985,6 @@ static int lvts_ctrl_configure(struct device
> > *dev, struct lvts_ctrl *lvts_ctrl)
> >   	value = LVTS_TSSEL_CONF;
> >   	writel(value, LVTS_TSSEL(lvts_ctrl->base));
> >   
> > -	/*
> > -	 * LVTS_CALSCALE : ADC voltage round
> > -	 */
> > -	value = 0x300;
> > -	value = LVTS_CALSCALE_CONF;
> 
> As a side note, I believe that the original author wanted to write,
> instead...
> 
> 	value = LVTS_CALSCALE_CONF;
> 	writel(value, LVTS_CALSCALE(lvts_ctrl->base);
> 
> Cheers,
> Angelo
> 
> > -
> >   	/*
> >   	 * LVTS_MSRCTL0 : Sensor filtering strategy
> >   	 *
> 
> 
Base on xiaojun.zheng
     The register is for PTP function, and in MT8192 it's not required
to do such setting any more

Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: Remove redundant code in lvts_ctrl_configure

Posted by Pin-yen Lin 8 months, 4 weeks ago

Hi Chang-Yi,

On Wed, Apr 30, 2025 at 4:26 AM Chang-Yi Lin (林長億)
<ds_chang-yi.lin@mediatek.com> wrote:
>
> On Wed, 2024-04-10 at 11:55 +0200, AngeloGioacchino Del Regno wrote:
> > Il 10/04/24 02:07, Pin-yen Lin ha scritto:
> > > The removed variable assignment is never written to the register,
> > > so it
> > > has no effect on the device behavior.  Mediatek has confirmed that
> > > it
> > > is not required to initialize this register for current platforms,
> > > so
> > > remove this segment to avoid confusion.
> > >
> > > Signed-off-by: Pin-yen Lin <treapking@chromium.org>
> > >
> >
> > Added Nicolas Pitre to the loop;
> >
> > Nicolas, since you're pushing support for those, can you please check
> > if CALSCALE
> > is used/useful/necessary on MT8186/88 before I give a R-b to this
> > commit?
> >
> > P.S.: Can anyone from MediaTek please confirm if the 0x300 value is
> > right for this
> >        register? I'd still like Linux to properly initialize the
> > registers, even if
> >        this could technically be a default value.
> >
> >
> > > ---
> > >
> > >   drivers/thermal/mediatek/lvts_thermal.c | 6 ------
> > >   1 file changed, 6 deletions(-)
> > >
> > > diff --git a/drivers/thermal/mediatek/lvts_thermal.c
> > > b/drivers/thermal/mediatek/lvts_thermal.c
> > > index fd4bd650c77a..48d2f8ba3f18 100644
> > > --- a/drivers/thermal/mediatek/lvts_thermal.c
> > > +++ b/drivers/thermal/mediatek/lvts_thermal.c
> > > @@ -985,12 +985,6 @@ static int lvts_ctrl_configure(struct device
> > > *dev, struct lvts_ctrl *lvts_ctrl)
> > >   value = LVTS_TSSEL_CONF;
> > >   writel(value, LVTS_TSSEL(lvts_ctrl->base));
> > >
> > > -/*
> > > - * LVTS_CALSCALE : ADC voltage round
> > > - */
> > > -value = 0x300;
> > > -value = LVTS_CALSCALE_CONF;
> >
> > As a side note, I believe that the original author wanted to write,
> > instead...
> >
> > value = LVTS_CALSCALE_CONF;
> > writel(value, LVTS_CALSCALE(lvts_ctrl->base);
> >
> > Cheers,
> > Angelo
> >
> > > -
> > >   /*
> > >    * LVTS_MSRCTL0 : Sensor filtering strategy
> > >    *
> >
> >
> Base on xiaojun.zheng
>      The register is for PTP function, and in MT8192 it's not required
> to do such setting any more

This driver is now also used by mt7988, mt8188, and mt8195. Could you
confirm if the PTP function are not required for all these SoCs, so we
are safe to remove this?

As a side note, Mediatek explained PTP in an offline discussion with us:
PTPOD stands for PTP (Performance Thermal Power) OD (Overdrive).
PTPOD is a technique to let the HW runs at the same frequency but with
lower operating voltage to save power.

Regards,
Pin-yen
>
>
> ************* MEDIATEK Confidentiality Notice
>  ********************
> The information contained in this e-mail message (including any
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be
> conveyed only to the designated recipient(s). Any use, dissemination,
> distribution, printing, retaining or copying of this e-mail (including its
> attachments) by unintended recipient(s) is strictly prohibited and may
> be unlawful. If you are not an intended recipient of this e-mail, or believe
>
> that you have received this e-mail in error, please notify the sender
> immediately (by replying to this e-mail), delete any and all copies of
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!

Re: [PATCH] thermal/drivers/mediatek/lvts_thermal: Remove redundant code in lvts_ctrl_configure

Posted by Chen-Yu Tsai 1 year, 10 months ago

On Wed, Apr 10, 2024 at 8:07 AM Pin-yen Lin <treapking@chromium.org> wrote:
>
> The removed variable assignment is never written to the register, so it
> has no effect on the device behavior.  Mediatek has confirmed that it
> is not required to initialize this register for current platforms, so
> remove this segment to avoid confusion.
>
> Signed-off-by: Pin-yen Lin <treapking@chromium.org>

Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>