linux-kernel - RE: [PATCH v4 1/2] thermal: Add thermal driver for Sunplus SP7021

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] [day] [month] [year] [list]

Message-ID: <eb063f1a287049a1942625b846c0764d@sphcmbx02.sunplus.com.tw>
Date:   Tue, 1 Mar 2022 08:04:23 +0000
From:   Lh Kuo 郭力豪 <lh.Kuo@...plus.com>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>,
        Li-hao Kuo <lhjeff911@...il.com>,
        "rafael@...nel.org" <rafael@...nel.org>,
        "amitk@...nel.org" <amitk@...nel.org>,
        "rui.zhang@...el.com" <rui.zhang@...el.com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     Wells Lu 呂芳騰 <wells.lu@...plus.com>
Subject: RE: [PATCH v4 1/2] thermal: Add thermal driver for Sunplus SP7021

Hi Mr Daniel Lezcano:

Thank you for your review 

> Please check the headers above. Some are not necessary (eg cpufreq,
> delay, rtc ...) AFAICT
> 
> > +#define DISABLE_THREMAL		(BIT(31) | BIT(15))
> > +#define ENABLE_THREMAL		BIT(31)
> > +#define SP_THREMAL_MASK		GENMASK(10, 0)
> 
> THREMAL ?
> 
> s/THREMAL/THERMAL/ ?

Sorry, I'll correct it in next commit.

> > +static char *sp7021_otp_coef_read(struct device *dev, ssize_t *len)
> > +{
> > +	char *ret = NULL;
> > +	struct nvmem_cell *c = nvmem_cell_get(dev, "calib");
> > +
> > +	if (IS_ERR_OR_NULL(c))
> > +		return NULL;
> > +
> > +	ret = nvmem_cell_read(c, len);
> > +	nvmem_cell_put(c);
> 
> That is wrong. Please refer to the documentation driver-api/nvmem.rst
> 
> nvmem_cell_put() must be called when the pointer returned by
> nvmem_cell_read() is no longer used.
>

Sorry for that. I don't know which part is wrong.
nvmem_cell_put() has been called after nvmem_cell_read()
I am coding according to the example.


> duplicate line
> 
> > +	sp_data->otp_temp0 = FIELD_GET(SP_THREMAL_MASK, sp_data->otp_temp0);
> > +	sp_data->otp_temp1 = (otp_v[1] >> 3) | (otp_v[2] << 5);
> > +	sp_data->otp_temp1 = FIELD_GET(SP_THREMAL_MASK, sp_data->otp_temp1);
> > +	if (sp_data->otp_temp0 == 0)
> > +		sp_data->otp_temp0 = TEMP_OTP_BASE;
> 
> Can you add a comment explaining how is stored the coef in the nvcell,
> so we can understand the above actions ?
> 
>

Is the comment below ok?

/*
 *When remanufacturing, the 35 degree T_CODE will be read and stored in nvcell.
 *TEMP_RATE is the SP7021 temperature slope.
 *T_CODE : 11 digits in total
*/