devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Icenowy Zheng <icenowy-h8G6r0blFSE@public.gmane.org>
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: mark.rutland-5wv7dgnIgG8@public.gmane.org,
	sean-hENCXIMQXOg@public.gmane.org,
	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
	clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org,
	lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org,
	edu.molinas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org,
	krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	wens-jdAy2FN1RRM@public.gmane.org,
	hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org,
	rask-SivP7zSAdNDZaaYASwVUlg@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	mchehab-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	singhalsimran0-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org,
	knaack.h-Mmb7MZpHnFY@public.gmane.org,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org
Subject: Re: [PATCH 07/16] iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data
Date: Sun, 28 Jan 2018 21:52:36 +0800	[thread overview]
Message-ID: <1927F24D-D3ED-49CF-9B7C-A7C8C873F0CF@aosc.io> (raw)
In-Reply-To: <6ebbb5e0-eb5f-afe4-50e8-eb3cee8ec7fa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>



于 2018年1月28日 GMT+08:00 下午9:46:18, Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 写到:
>
>
>On 28.01.2018 10:02, Jonathan Cameron wrote:
>> On Fri, 26 Jan 2018 16:19:32 +0100
>> Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> 
>>> This patch reworks the driver to support nvmem calibration cells.
>>> The driver checks if the nvmem calibration is supported and reads
>out
>>> the nvmem. At the beginning of the startup process the calibration
>data
>>> is written to the related registers.
>>>
>>> Signed-off-by: Philipp Rossak <embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> 
>> A few minor suggestions inline.
>> 
>> Jonathan
>> 
>>> ---
>>>   drivers/iio/adc/sun4i-gpadc-iio.c | 52
>+++++++++++++++++++++++++++++++++++++++
>>>   include/linux/mfd/sun4i-gpadc.h   |  2 ++
>>>   2 files changed, 54 insertions(+)
>>>
>>> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c
>b/drivers/iio/adc/sun4i-gpadc-iio.c
>>> index bff06f2798e8..7b12666cdd9e 100644
>>> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
>>> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c
>>> @@ -27,6 +27,7 @@
>>>   #include <linux/interrupt.h>
>>>   #include <linux/io.h>
>>>   #include <linux/module.h>
>>> +#include <linux/nvmem-consumer.h>
>>>   #include <linux/of.h>
>>>   #include <linux/of_device.h>
>>>   #include <linux/platform_device.h>
>>> @@ -81,6 +82,7 @@ struct gpadc_data {
>>>   	bool		has_bus_rst;
>>>   	bool		has_mod_clk;
>>>   	int		sensor_count;
>>> +	bool		supports_nvmem;
>>>   };
>>>   
>>>   static const struct gpadc_data sun4i_gpadc_data = {
>>> @@ -94,6 +96,7 @@ static const struct gpadc_data sun4i_gpadc_data =
>{
>>>   	.sample_start = sun4i_gpadc_sample_start,
>>>   	.sample_end = sun4i_gpadc_sample_end,
>>>   	.sensor_count = 1,
>>> +	.supports_nvmem = false,
>>>   };
>>>   
>>>   static const struct gpadc_data sun5i_gpadc_data = {
>>> @@ -107,6 +110,7 @@ static const struct gpadc_data sun5i_gpadc_data
>= {
>>>   	.sample_start = sun4i_gpadc_sample_start,
>>>   	.sample_end = sun4i_gpadc_sample_end,
>>>   	.sensor_count = 1,
>>> +	.supports_nvmem = false,
>>>   };
>>>   
>>>   static const struct gpadc_data sun6i_gpadc_data = {
>>> @@ -120,6 +124,7 @@ static const struct gpadc_data sun6i_gpadc_data
>= {
>>>   	.sample_start = sun4i_gpadc_sample_start,
>>>   	.sample_end = sun4i_gpadc_sample_end,
>>>   	.sensor_count = 1,
>>> +	.supports_nvmem = false,
>>>   };
>>>   
>>>   static const struct gpadc_data sun8i_a33_gpadc_data = {
>>> @@ -130,6 +135,7 @@ static const struct gpadc_data
>sun8i_a33_gpadc_data = {
>>>   	.sample_start = sun4i_gpadc_sample_start,
>>>   	.sample_end = sun4i_gpadc_sample_end,
>>>   	.sensor_count = 1,
>>> +	.supports_nvmem = false,

BTW A33 claims to support calibration data according to the manual.

>>>   };
>>>   
>>>   struct sun4i_gpadc_iio {
>>> @@ -148,6 +154,8 @@ struct sun4i_gpadc_iio {
>>>   	struct clk			*mod_clk;
>>>   	struct reset_control		*reset;
>>>   	int				sensor_id;
>>> +	u32				calibration_data[2];
>>> +	bool				has_calibration_data[2];
>>>   	/* prevents concurrent reads of temperature and ADC */
>>>   	struct mutex			mutex;
>>>   	struct thermal_zone_device	*tzd;
>>> @@ -459,6 +467,17 @@ static int sun4i_gpadc_runtime_suspend(struct
>device *dev)
>>>   	return info->data->sample_end(info);
>>>   }
>>>   
>>> +static void sunxi_calibrate(struct sun4i_gpadc_iio *info)
>>> +{
>>> +	if (info->has_calibration_data[0])
>>> +		regmap_write(info->regmap, SUNXI_THS_CDATA_0_1,
>>> +			info->calibration_data[0]);
>>> +
>>> +	if (info->has_calibration_data[1])
>>> +		regmap_write(info->regmap, SUNXI_THS_CDATA_2_3,
>>> +			info->calibration_data[1]);
>>> +}
>>> +
>>>   static int sun4i_gpadc_sample_start(struct sun4i_gpadc_iio *info)
>>>   {
>>>   	/* clkin = 6MHz */
>>> @@ -481,6 +500,7 @@ static int sun4i_gpadc_sample_start(struct
>sun4i_gpadc_iio *info)
>>>   static int sunxi_ths_sample_start(struct sun4i_gpadc_iio *info)
>>>   {
>>>   	u32 value;
>>> +	sunxi_calibrate(info);
>>>   
>>>   	if (info->data->ctrl0_map)
>>>   		regmap_write(info->regmap, SUNXI_THS_CTRL0,
>>> @@ -602,6 +622,9 @@ static int sun4i_gpadc_probe_dt(struct
>platform_device *pdev,
>>>   	struct resource *mem;
>>>   	void __iomem *base;
>>>   	int ret;
>>> +	struct nvmem_cell *cell;
>>> +	ssize_t cell_size;
>>> +	u64 *cell_data;
>>>   
>>>   	info->data = of_device_get_match_data(&pdev->dev);
>>>   	if (!info->data)
>>> @@ -616,6 +639,35 @@ static int sun4i_gpadc_probe_dt(struct
>platform_device *pdev,
>>>   	if (IS_ERR(base))
>>>   		return PTR_ERR(base);
>>>   
>>> +	info->has_calibration_data[0] = false;
>>> +	info->has_calibration_data[1] = false;
>>> +
>>> +	if (!info->data->supports_nvmem)
>>> +		goto no_nvmem;
>>> +
>>> +	cell = devm_nvmem_cell_get(&pdev->dev, "calibration");
>>> +	if (IS_ERR(cell)) {
>>> +		if (PTR_ERR(cell) == -EPROBE_DEFER)
>>> +			return PTR_ERR(cell);
>> Use a goto here to no_nvmem.
>> 
>> Then you can drop the below else to make things more readable.
>>> +	} else {
>>> +		cell_data = (u64 *)nvmem_cell_read(cell, &cell_size);
>>> +		devm_nvmem_cell_put(&pdev->dev, cell);
>> 
>> I'm really not keen on use of devm for things we are intending
>> to drop almost immediately.  Just use the non managed versions
>> and clean up properly in the error paths (if there are any
>> where it is needed - which there aren't that I can see)
>> 
>
>^^ Ok, I will rework that.
>
>>> +		if (cell_size <= 4) {
>> Is it valid if it is anything other than 4?
>
>For sensors with only one sensor would be also a 2 valid, for those
>with 
>2 sensors ==> 4, with 3 sensors ==> 6 and with and with 4 sensors ==>
>8.
>
>In hardware we have in total to 32bit registers for calibration, thus I
>
>thought it would be a good idea to use always four bytes per register. 
>If two bytes are used, they should be the default value.
>
>But I agree, this needs a rework.
>
>>> +			info->has_calibration_data[0] = true;
>>> +			info->calibration_data[0] = be32_to_cpu(cell_data[0] &
>>> +					GENMASK(31, 0));
>> 
>> Masking isn't needed,  If you want to be paranoid there is the
>lower_32_bits
>> function..
>> 
>^^ Ok, I will rework that.
>>> +		} else if (cell_size <= 8) {
>>> +			info->has_calibration_data[0] = true;
>>> +			info->calibration_data[0] = be32_to_cpu(cell_data[0] &
>>> +					GENMASK(31, 0));
>> 
>> This first block is repeated.  Easy enough to avoid I think...
>> 
>
>^^ Ok, I will rework that.
>
>>> +			info->has_calibration_data[1] = true;
>>> +			info->calibration_data[1] = be32_to_cpu(
>>> +					(cell_data[0] >> 32) & GENMASK(31, 0));
>>> +		}
>>> +	}
>>> +
>>> +no_nvmem:
>>> +
>>>   	info->regmap = devm_regmap_init_mmio(&pdev->dev, base,
>>>   					     &sun4i_gpadc_regmap_config);
>>>   	if (IS_ERR(info->regmap)) {
>>> diff --git a/include/linux/mfd/sun4i-gpadc.h
>b/include/linux/mfd/sun4i-gpadc.h
>>> index 40b4dd9d2405..c251002431bd 100644
>>> --- a/include/linux/mfd/sun4i-gpadc.h
>>> +++ b/include/linux/mfd/sun4i-gpadc.h
>>> @@ -90,6 +90,8 @@
>>>   #define SUNXI_THS_CTRL0					0x00
>>>   #define SUNXI_THS_CTRL2					0x40
>>>   #define SUNXI_THS_FILTER				0x70
>>> +#define SUNXI_THS_CDATA_0_1				0x74
>>> +#define SUNXI_THS_CDATA_2_3				0x78
>>>   #define SUNXI_THS_TDATA0				0x80
>>>   #define SUNXI_THS_TDATA1				0x84
>>>   #define SUNXI_THS_TDATA2				0x88
>> 
>Thanks,
>Philipp
>
>_______________________________________________
>linux-arm-kernel mailing list
>linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
>http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2018-01-28 13:52 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-26 15:19 [PATCH 00/16] IIO-based thermal sensor driver for Allwinner H3 and A83T SoC Philipp Rossak
     [not found] ` <20180126151941.12183-1-embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-01-26 15:19   ` [PATCH 01/16] dt-bindings: update the Allwinner GPADC device tree binding for H3 & A83T Philipp Rossak
2018-01-26 15:19   ` [PATCH 02/16] arm: config: sunxi_defconfig: enable SUN4I_GPADC Philipp Rossak
2018-01-26 15:19   ` [PATCH 03/16] iio: adc: sun4i-gpadc-iio: rename A33-specified registers to contain A33 Philipp Rossak
     [not found]     ` <20180126151941.12183-4-embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-01-28  8:34       ` Jonathan Cameron
2018-01-26 15:19   ` [PATCH 04/16] iio: adc: sun4i-gpadc-iio: rework: sampling start/end code readout reg Philipp Rossak
2018-01-28  8:43     ` Jonathan Cameron
2018-01-28 13:34       ` Philipp Rossak
2018-01-28 13:37         ` Icenowy Zheng
     [not found]           ` <2CC367BA-766A-4384-82EE-9A38BBFB08C2-h8G6r0blFSE@public.gmane.org>
2018-01-28 14:32             ` Philipp Rossak
2018-01-26 15:19   ` [PATCH 05/16] iio: adc: sun4i-gpadc-iio: rework: support clocks and reset Philipp Rossak
2018-01-28  8:50     ` Jonathan Cameron
2018-01-28 11:34       ` Philipp Rossak
2018-01-26 15:19   ` [PATCH 06/16] iio: adc: sun4i-gpadc-iio: rework: support multible sensors Philipp Rossak
2018-01-28  9:08     ` Jonathan Cameron
2018-01-28 14:10       ` Philipp Rossak
2018-01-26 15:19   ` [PATCH 07/16] iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data Philipp Rossak
2018-01-28  9:02     ` Jonathan Cameron
2018-01-28 13:46       ` Philipp Rossak
     [not found]         ` <6ebbb5e0-eb5f-afe4-50e8-eb3cee8ec7fa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-01-28 13:52           ` Icenowy Zheng [this message]
     [not found]             ` <1927F24D-D3ED-49CF-9B7C-A7C8C873F0CF-h8G6r0blFSE@public.gmane.org>
2018-01-28 14:39               ` Philipp Rossak
2018-01-26 15:19   ` [PATCH 08/16] iio: adc: sun4i-gpadc-iio: rework: add interrupt support Philipp Rossak
     [not found]     ` <20180126151941.12183-9-embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-01-28  9:06       ` Jonathan Cameron
2018-01-28 13:48         ` Philipp Rossak
2018-01-26 15:19   ` [PATCH 09/16] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor Philipp Rossak
2018-01-26 15:19   ` [PATCH 10/16] iio: adc: sun4i-gpadc-iio: add support for A83T " Philipp Rossak
     [not found]     ` <20180126151941.12183-11-embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-01-26 17:46       ` [linux-sunxi] " Ondřej Jirman
     [not found]         ` <20180126174602.qolcswhqblgmlad7-9v8tmBix7cb9zxVx7UNMDg@public.gmane.org>
2018-01-27  4:30           ` Philipp Rossak
2018-01-26 15:19   ` [PATCH 11/16] arm: dts: sunxi-h3-h5: add support for the thermal sensor in H3 and H5 Philipp Rossak
2018-01-26 15:19   ` [PATCH 12/16] arm: dts: sun8i: h3: add support for the thermal sensor in H3 Philipp Rossak
2018-01-26 15:19   ` [PATCH 13/16] arm: dts: sun8i: h3: add thermal zone to H3 Philipp Rossak
     [not found]     ` <20180126151941.12183-14-embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-01-26 16:26       ` Samuel Holland
     [not found]         ` <4cfb1faf-a712-c882-1ef1-55845c1bb00e-RkNLwX/CsU9g9hUCZPvPmw@public.gmane.org>
2018-01-26 17:48           ` [linux-sunxi] " Philipp Rossak
2018-01-26 15:19   ` [PATCH 14/16] arm: dts: sun8i: h3: enable H3 sid controller Philipp Rossak
2018-01-26 15:19   ` [PATCH 15/16] arm: dts: sun8i: a83t: add support for the thermal sensor in A83T Philipp Rossak
2018-01-26 15:19   ` [PATCH 16/16] arm: dts: sun8i: a83t: add thermal zone to A83T Philipp Rossak
     [not found]     ` <20180126151941.12183-17-embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-01-26 16:25       ` Samuel Holland
2018-01-26 17:35         ` [linux-sunxi] " Philipp Rossak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1927F24D-D3ED-49CF-9B7C-A7C8C873F0CF@aosc.io \
    --to=icenowy-h8g6r0blfse@public.gmane.org \
    --cc=clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org \
    --cc=devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=edu.molinas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=embed3d-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org \
    --cc=jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=knaack.h-Mmb7MZpHnFY@public.gmane.org \
    --cc=krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org \
    --cc=lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org \
    --cc=mark.rutland-5wv7dgnIgG8@public.gmane.org \
    --cc=maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org \
    --cc=mchehab-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=pmeerw-jW+XmwGofnusTnJN9+BGXg@public.gmane.org \
    --cc=quentin.schulz-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org \
    --cc=rask-SivP7zSAdNDZaaYASwVUlg@public.gmane.org \
    --cc=robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
    --cc=sean-hENCXIMQXOg@public.gmane.org \
    --cc=singhalsimran0-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=wens-jdAy2FN1RRM@public.gmane.org \
    --subject='Re: [PATCH 07/16] iio: adc: sun4i-gpadc-iio: rework: support nvmem calibration data' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).