Re: [PATCH v2 3/4] thermal: mediatek: control buffer enablement tweaks

[Amjad Ouled-Ameur]

Hi Angelo,

On 7/21/22 10:11, AngeloGioacchino Del Regno wrote:
> Il 20/07/22 20:18, Amjad Ouled-Ameur ha scritto:
> > From: Markus Schneider-Pargmann <msp@xxxxxxxxxxxx>
> >
> > Add logic in order to be able to turn on the control buffer on MT8365.
> > This change now allows to have control buffer support for 
> > MTK_THERMAL_V1,
> > and it allows to define the register offset, and mask used to enable it.
> >
> > Signed-off-by: Markus Schneider-Pargmann <msp@xxxxxxxxxxxx>
> > Signed-off-by: Fabien Parent <fparent@xxxxxxxxxxxx>
> > Reviewed-by: Amjad Ouled-Ameur <aouledameur@xxxxxxxxxxxx>
> > Tested-by: Amjad Ouled-Ameur <aouledameur@xxxxxxxxxxxx>
> > ---
> >   drivers/thermal/mtk_thermal.c | 23 +++++++++++++++++------
> >   1 file changed, 17 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/thermal/mtk_thermal.c 
> > b/drivers/thermal/mtk_thermal.c
> > index 216eb0d61c5b..1dc276f8c4f1 100644
> > --- a/drivers/thermal/mtk_thermal.c
> > +++ b/drivers/thermal/mtk_thermal.c
> > @@ -283,6 +283,9 @@ struct mtk_thermal_data {
> >       bool need_switch_bank;
> >       struct thermal_bank_cfg bank_data[MAX_NUM_ZONES];
> >       enum mtk_thermal_version version;
> > +    u32 apmixed_buffer_ctl_reg;
> > +    u32 apmixed_buffer_ctl_mask;
> > +    u32 apmixed_buffer_ctl_set;
> >   };
> >     struct mtk_thermal {
> > @@ -578,6 +581,9 @@ static const struct mtk_thermal_data 
> > mt7622_thermal_data = {
> >       .adcpnp = mt7622_adcpnp,
> >       .sensor_mux_values = mt7622_mux_values,
> >       .version = MTK_THERMAL_V2,
> > +    .apmixed_buffer_ctl_reg = APMIXED_SYS_TS_CON1,
> > +    .apmixed_buffer_ctl_mask = ~0x37,
>
> This is effectively GENMASK(31, 6) | BIT(3) :-)
>
> > +    .apmixed_buffer_ctl_set = 0x1,
>
> ...and this is BIT(0)
Thank you for the catch, will update them appropriately.
> >   }; >
> >   /*
> > @@ -1031,14 +1037,18 @@ static const struct of_device_id 
> > mtk_thermal_of_match[] = {
> >   };
> >   MODULE_DEVICE_TABLE(of, mtk_thermal_of_match);
> >   -static void mtk_thermal_turn_on_buffer(void __iomem *apmixed_base)
> > +static void mtk_thermal_turn_on_buffer(struct mtk_thermal *mt,
> > +                       void __iomem *apmixed_base)
> >   {
> >       int tmp;
>
> Since you're practically restructuring this function, please fix this 
> wrong
> variable type, as this should be a u32.
Sure agreed.


Thank you Angelo for the review.

Regards,

Amjad

> Regards,
> Angelo