[PATCH] ARM: Add hwmon driver for LRADC found on Allwinner A13/A20 SoCs

[Ruslan Zalata <rz@fabmicro.ru>]

Jernej,

Are these CHECKs ok to pass ?

CHECK: Alignment should match open parenthesis
#314: FILE: drivers/hwmon/sun4i-lradc-hwmon.c:231:
+       lradc->base = devm_ioremap_resource(dev,
+                       platform_get_resource(pdev, IORESOURCE_MEM, 0));

CHECK: Alignment should match open parenthesis
#319: FILE: drivers/hwmon/sun4i-lradc-hwmon.c:236:
+       error = devm_request_irq(dev, platform_get_irq(pdev, 0),
+                       sun4i_lradc_irq, 0, "sun4i-lradc-hwmon", lradc);

total: 0 errors, 0 warnings, 2 checks, 307 lines checked

Aligning above by paranthesis will make code look ugly and exceed 75 
chars limit. Please advise.

---
Regards,
Ruslan.

Fabmicro, LLC.

On 2022-04-28 02:12, Jernej Škrabec wrote:
> Dne sreda, 27. april 2022 ob 23:01:58 CEST je Ruslan Zalata napisal(a):
>> Dear Jernej and all.
>> 
>> I used sendmail instead of "git send-email" and it did the whole this
>> wrong - several separate emails were sent. I'm sorry for that, will
>> stick to git command next time.
>> 
>> I read the guide of course and I fed the patch through
>> scripts/checkpatch.pl many times in an effort to elaborate all the
>> errors and warnings. The only warning left is this:
>> 
>> WARNING: added, moved or deleted file(s), does MAINTAINERS need
>> updating?
>> #61:
>> new file mode 100644
>> 
>> total: 0 errors, 1 warnings, 304 lines checked
> 
> Did you used --strict? Anyway, you have multiple consecutive empty 
> lines in
> several places. Only one empty line is allowed.
> 
>> 
>> Since this is my first attempt to submit my code I'm quite confused 
>> whom
>> should I add as a maintainer for this driver. :)
> 
> Usually yourself.
> 
>> 
>> PS: What's wrong with the subject ?
> 
> Too long and ARM is not a proper tag. Check git history of 
> drivers/hwmon
> folder for ideas.
> 
> Best regards,
> Jernej
> 
>> 
>> ---
>> Regards,
>> Ruslan.
>> 
>> Fabmicro, LLC.
>> 
>> On 2022-04-28 01:23, Jernej Škrabec wrote:
>> > Hi Ruslan!
>> >
>> > Dne sreda, 27. april 2022 ob 21:25:12 CEST je Ruslan Zalata napisal(a):
>> >> Some Allwinner SoCs like A13, A20 or T2 are equipped with two-channel
>> >> low rate (6 bit) ADC that is often used for extra keys. There's a
>> >> driver
>> >> for that already implementing standard input device, but it has these
>> >> limitations: 1) it cannot be used for general ADC data equisition, and
>> >> 2) it uses only one LRADC channel of two available.
>> >>
>> >> This driver provides basic hwmon interface to both channels of LRADC
>> >> on
>> >> such Allwinner SoCs.
>> >>
>> >> Signed-off-by: Ruslan Zalata <rz@fabmicro.ru>
>> >
>> > Before even going to check actual driver, please read
>> > https://www.kernel.org/
>> > doc/html/latest/process/submitting-patches.html in detail.
>> >
>> > Just few basic things to fix first:
>> > 1. subject doesn't conform to rules
>> > 2. send patch to maintainers and mailing lists (use
>> > scripts/get_maintainer.pl)
>> > 3. code has style issues (multiple empty lines). Run
>> > scripts/checkpatch.pl --
>> > strict on your patch before submission and fix all reported issues.
>> > 4. you sent same patch two times. If it's same, it should be marked
>> > with
>> > RESEND, if not, it should be versioned (V2, V3, etc.) with short
>> > changelog
>> > after --- line (as explained in above link)
>> > 5. it's customary to add new entry in MAINTAINERS when adding new
>> > driver
>> >
>> > Best regards,
>> > Jernej
>> >
>> >> ---
>> >>
>> >>  drivers/hwmon/Kconfig             |  13 ++
>> >>  drivers/hwmon/Makefile            |   1 +
>> >>  drivers/hwmon/sun4i-lradc-hwmon.c | 278
>> >>
>> >> ++++++++++++++++++++++++++++++
>> >>
>> >>  3 files changed, 292 insertions(+)
>> >>  create mode 100644 drivers/hwmon/sun4i-lradc-hwmon.c
>> >>
>> >> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> >> index 68a8a27ab3b..b7732a9e992 100644
>> >> --- a/drivers/hwmon/Kconfig
>> >> +++ b/drivers/hwmon/Kconfig
>> >> @@ -521,6 +521,19 @@ config I8K
>> >>
>> >>  	  Say Y if you intend to run userspace programs that use this
>> >
>> > interface.
>> >
>> >>  	  Say N otherwise.
>> >>
>> >> +config SENSORS_SUN4I_LRADC
>> >> +	tristate "Allwinner A13/A20 LRADC hwmon"
>> >> +	depends on ARCH_SUNXI && (!KEYBOARD_SUN4I_LRADC)
>> >> +	help
>> >> +	  Say y here to support the LRADC found in Allwinner A13/A20 SoCs.
>> >> +	  Both channels are supported.
>> >> +
>> >> +	  This driver can also be built as module. If so, the module
>> >> +	  will be called sun4i-lradc-hwmon.
>> >> +
>> >> +	  This option is not compatible with KEYBOARD_SUN4I_LRADC, one
>> >> +	  of these should be used at a time.
>> >> +
>> >>
>> >>  config SENSORS_DA9052_ADC
>> >>
>> >>  	tristate "Dialog DA9052/DA9053 ADC"
>> >>  	depends on PMIC_DA9052
>> >>
>> >> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> >> index 8a03289e2aa..43bed6fff27 100644
>> >> --- a/drivers/hwmon/Makefile
>> >> +++ b/drivers/hwmon/Makefile
>> >> @@ -30,6 +30,7 @@ obj-$(CONFIG_SENSORS_AD7314)	+= ad7314.o
>> >>
>> >>  obj-$(CONFIG_SENSORS_AD7414)	+= ad7414.o
>> >>  obj-$(CONFIG_SENSORS_AD7418)	+= ad7418.o
>> >>  obj-$(CONFIG_SENSORS_ADC128D818) += adc128d818.o
>> >>
>> >> +obj-$(CONFIG_SENSORS_SUN4I_LRADC) += sun4i-lradc-hwmon.o
>> >>
>> >>  obj-$(CONFIG_SENSORS_ADCXX)	+= adcxx.o
>> >>  obj-$(CONFIG_SENSORS_ADM1021)	+= adm1021.o
>> >>  obj-$(CONFIG_SENSORS_ADM1025)	+= adm1025.o
>> >>
>> >> diff --git a/drivers/hwmon/sun4i-lradc-hwmon.c
>> >> b/drivers/hwmon/sun4i-lradc-hwmon.c new file mode 100644
>> >> index 00000000000..1947b716dbf
>> >> --- /dev/null
>> >> +++ b/drivers/hwmon/sun4i-lradc-hwmon.c
>> >> @@ -0,0 +1,278 @@
>> >> +// SPDX-License-Identifier: GPL-2.0-or-later
>> >> +/*
>> >> + * Allwinner sun4i (A13/A20) LRADC hwmon driver
>> >> + *
>> >> + * Copyright (C) 2022 Fabmicro, LLC., Tyumen, Russia.
>> >> + * Copyright (C) 2022 Ruslan Zalata <rz@fabmicro.ru>
>> >> + */
>> >> +
>> >> +#include <linux/err.h>
>> >> +#include <linux/init.h>
>> >> +#include <linux/input.h>
>> >> +#include <linux/interrupt.h>
>> >> +#include <linux/module.h>
>> >> +#include <linux/of_platform.h>
>> >> +#include <linux/platform_device.h>
>> >> +#include <linux/regulator/consumer.h>
>> >> +#include <linux/slab.h>
>> >> +#include <linux/hwmon.h>
>> >> +#include <linux/hwmon-sysfs.h>
>> >> +
>> >> +#define LRADC_CTRL		0x00
>> >> +#define LRADC_INTC		0x04
>> >> +#define LRADC_INTS		0x08
>> >> +#define LRADC_DATA0		0x0c
>> >> +#define LRADC_DATA1		0x10
>> >> +
>> >> +/* LRADC_CTRL bits */
>> >> +#define FIRST_CONVERT_DLY(x)	((x) << 24) /* 8 bits */
>> >> +#define CHAN_SELECT(x)		((x) << 22) /* 2 bits */
>> >> +#define CONTINUE_TIME_SEL(x)	((x) << 16) /* 4 bits */
>> >> +#define KEY_MODE_SEL(x)		((x) << 12) /* 2 bits */
>> >> +#define LEVELA_B_CNT(x)		((x) << 8)  /* 4 bits */
>> >> +#define HOLD_KEY_EN(x)		((x) << 7)
>> >> +#define HOLD_EN(x)		((x) << 6)
>> >> +#define LEVELB_VOL(x)		((x) << 4)  /* 2 bits */
>> >> +#define SAMPLE_RATE(x)		((x) << 2)  /* 2 bits */
>> >> +#define ENABLE(x)		((x) << 0)
>> >> +
>> >> +/* LRADC_INTC and LRADC_INTS bits */
>> >> +#define CHAN1_KEYUP_IRQ		BIT(12)
>> >> +#define CHAN1_ALRDY_HOLD_IRQ	BIT(11)
>> >> +#define CHAN1_HOLD_IRQ		BIT(10)
>> >> +#define	CHAN1_KEYDOWN_IRQ	BIT(9)
>> >> +#define CHAN1_DATA_IRQ		BIT(8)
>> >> +#define CHAN0_KEYUP_IRQ		BIT(4)
>> >> +#define CHAN0_ALRDY_HOLD_IRQ	BIT(3)
>> >> +#define CHAN0_HOLD_IRQ		BIT(2)
>> >> +#define	CHAN0_KEYDOWN_IRQ	BIT(1)
>> >> +#define CHAN0_DATA_IRQ		BIT(0)
>> >> +
>> >> +
>> >> +struct lradc_variant {
>> >> +	u32 bits;
>> >> +	u32 resolution;
>> >> +	u32 vref;
>> >> +};
>> >> +
>> >> +struct lradc_variant variant_sun4i_a10_lradc = {
>> >> +	.bits = 0x3f,
>> >> +	.resolution = 63,
>> >> +	.vref = 2000000,
>> >> +};
>> >> +
>> >> +struct lradc_variant variant_sun8i_a83t_r_lradc = {
>> >> +	.bits = 0x3f,
>> >> +	.resolution = 63,
>> >> +	.vref = 2000000,
>> >> +};
>> >> +
>> >> +static const struct of_device_id sun4i_lradc_of_match[];
>> >> +
>> >> +
>> >> +struct sun4i_lradc_data {
>> >> +	struct device *dev;
>> >> +	struct device *hwmon_dev;
>> >> +	void __iomem *base;
>> >> +	const struct lradc_variant *variant;
>> >> +	u32 in[2];
>> >> +	u32 in_mvolts[2];
>> >> +};
>> >> +
>> >> +
>> >> +static ssize_t lradc_in_mvolts_show(struct device *dev,
>> >> +			struct device_attribute *attr, char *buf)
>> >> +{
>> >> +	struct sun4i_lradc_data *data = dev_get_drvdata(dev);
>> >> +	int nr = to_sensor_dev_attr_2(attr)->nr;
>> >> +
>> >> +	if (IS_ERR(data))
>> >> +		return PTR_ERR(data);
>> >> +
>> >> +	return sprintf(buf, "%d\n", data->in_mvolts[nr]);
>> >> +}
>> >> +
>> >> +static ssize_t lradc_in_show(struct device *dev,
>> >> +		struct device_attribute *attr, char *buf)
>> >> +{
>> >> +	struct sun4i_lradc_data *data = dev_get_drvdata(dev);
>> >> +	int nr = to_sensor_dev_attr_2(attr)->nr;
>> >> +
>> >> +	if (IS_ERR(data))
>> >> +		return PTR_ERR(data);
>> >> +
>> >> +	return sprintf(buf, "%d\n", data->in[nr]);
>> >> +}
>> >> +
>> >> +
>> >> +static umode_t lradc_is_visible(struct kobject *kobj,
>> >> +		struct attribute *attr, int index)
>> >> +{
>> >> +	return attr->mode;
>> >> +}
>> >> +
>> >> +
>> >> +
>> >> +static SENSOR_DEVICE_ATTR_2_RO(in0_input, lradc_in, 0, 0);
>> >> +static SENSOR_DEVICE_ATTR_2_RO(in0_input_mvolts, lradc_in_mvolts, 0,
>> >> 1);
>> >> +
>> >> +static SENSOR_DEVICE_ATTR_2_RO(in1_input, lradc_in, 1, 0);
>> >> +static SENSOR_DEVICE_ATTR_2_RO(in1_input_mvolts, lradc_in_mvolts, 1,
>> >> 1);
>> >> +
>> >> +static struct attribute *lradc_attrs[] = {
>> >> +	&sensor_dev_attr_in0_input.dev_attr.attr,
>> >> +	&sensor_dev_attr_in0_input_mvolts.dev_attr.attr,
>> >> +	&sensor_dev_attr_in1_input.dev_attr.attr,
>> >> +	&sensor_dev_attr_in1_input_mvolts.dev_attr.attr,
>> >> +	NULL
>> >> +};
>> >> +
>> >> +static const struct attribute_group lradc_group = {
>> >> +	.attrs = lradc_attrs,
>> >> +	.is_visible = lradc_is_visible,
>> >> +};
>> >> +__ATTRIBUTE_GROUPS(lradc);
>> >> +
>> >> +
>> >> +static irqreturn_t sun4i_lradc_irq(int irq, void *dev_id)
>> >> +{
>> >> +	struct sun4i_lradc_data *lradc = dev_id;
>> >> +	u32 ints;
>> >> +
>> >> +	ints  = readl(lradc->base + LRADC_INTS);
>> >> +
>> >> +	if (ints & CHAN0_DATA_IRQ) {
>> >> +		lradc->in[0] = readl(lradc->base + LRADC_DATA0) &
>> >> +			lradc->variant->bits;
>> >> +		lradc->in_mvolts[0] = lradc->in[0] * lradc->variant-
>> >> vref /
>> >> +			lradc->variant->resolution / 1000;
>> >> +	}
>> >> +
>> >> +	if (ints & CHAN1_DATA_IRQ) {
>> >> +		lradc->in[1] = readl(lradc->base + LRADC_DATA1) &
>> >> +			lradc->variant->bits;
>> >> +		lradc->in_mvolts[1] = lradc->in[1] * lradc->variant-
>> >> vref /
>> >> +			lradc->variant->resolution / 1000;
>> >> +	}
>> >> +
>> >> +	writel(ints, lradc->base + LRADC_INTS);
>> >> +
>> >> +	return IRQ_HANDLED;
>> >> +}
>> >> +
>> >> +static int sun4i_lradc_open(struct device *dev)
>> >> +{
>> >> +	struct sun4i_lradc_data *lradc = dev_get_drvdata(dev);
>> >> +
>> >> +	writel(FIRST_CONVERT_DLY(2) | CHAN_SELECT(3) | LEVELA_B_CNT(1) |
>> >> +		HOLD_EN(1) | KEY_MODE_SEL(2) | SAMPLE_RATE(3) |
>> >
>> > ENABLE(1),
>> >
>> >> +		lradc->base + LRADC_CTRL);
>> >> +
>> >> +	writel(CHAN0_DATA_IRQ | CHAN1_DATA_IRQ, lradc->base + LRADC_INTC);
>> >> +
>> >> +	return 0;
>> >> +}
>> >> +
>> >> +static void sun4i_lradc_close(struct device *dev)
>> >> +{
>> >> +	struct sun4i_lradc_data *lradc = dev_get_drvdata(dev);
>> >> +
>> >> +	writel(FIRST_CONVERT_DLY(2) | LEVELA_B_CNT(1) | HOLD_EN(1) |
>> >> +		SAMPLE_RATE(2), lradc->base + LRADC_CTRL);
>> >> +	writel(0, lradc->base + LRADC_INTC);
>> >> +}
>> >> +
>> >> +static int sun4i_lradc_resume(struct platform_device *pdev)
>> >> +{
>> >> +	struct device *dev = &pdev->dev;
>> >> +
>> >> +	return sun4i_lradc_open(dev);
>> >> +}
>> >> +
>> >> +static int sun4i_lradc_suspend(struct platform_device *pdev,
>> >> pm_message_t
>> >> state) +{
>> >> +	struct device *dev = &pdev->dev;
>> >> +
>> >> +	sun4i_lradc_close(dev);
>> >> +	return 0;
>> >> +}
>> >> +
>> >> +static int sun4i_lradc_remove(struct platform_device *pdev)
>> >> +{
>> >> +	struct device *dev = &pdev->dev;
>> >> +
>> >> +	sun4i_lradc_close(dev);
>> >> +	return 0;
>> >> +}
>> >> +
>> >> +static int sun4i_lradc_probe(struct platform_device *pdev)
>> >> +{
>> >> +	struct sun4i_lradc_data *lradc;
>> >> +	struct device *dev = &pdev->dev;
>> >> +	int error;
>> >> +
>> >> +	lradc = devm_kzalloc(dev, sizeof(struct sun4i_lradc_data),
>> >
>> > GFP_KERNEL);
>> >
>> >> +	if (!lradc)
>> >> +		return -ENOMEM;
>> >> +
>> >> +	dev_set_drvdata(dev, lradc);
>> >> +
>> >> +	lradc->variant = of_device_get_match_data(&pdev->dev);
>> >> +	if (!lradc->variant) {
>> >> +		dev_err(&pdev->dev, "Missing '%s' or '%s' variant\n",
>> >> +			sun4i_lradc_of_match[0].compatible,
>> >> +			sun4i_lradc_of_match[1].compatible);
>> >> +		return -EINVAL;
>> >> +	}
>> >> +
>> >> +	lradc->dev = dev;
>> >> +	lradc->hwmon_dev = devm_hwmon_device_register_with_groups(dev,
>> >
>> > "lradc",
>> >
>> >> +
>> >
>> > lradc, lradc_groups);
>> >
>> >> +	if (IS_ERR(lradc->hwmon_dev)) {
>> >> +		error = PTR_ERR(lradc->hwmon_dev);
>> >> +		return error;
>> >> +	}
>> >> +
>> >> +	lradc->base = devm_ioremap_resource(dev,
>> >> +			platform_get_resource(pdev, IORESOURCE_MEM,
>> >
>> > 0));
>> >
>> >> +	if (IS_ERR(lradc->base))
>> >> +		return PTR_ERR(lradc->base);
>> >> +
>> >> +	error = devm_request_irq(dev, platform_get_irq(pdev, 0),
>> >> +			sun4i_lradc_irq, 0, "sun4i-lradc-hwmon",
>> >
>> > lradc);
>> >
>> >> +	if (error)
>> >> +		return error;
>> >> +
>> >> +	error = sun4i_lradc_open(dev);
>> >> +	if (error)
>> >> +		return error;
>> >> +
>> >> +	return 0;
>> >> +}
>> >> +
>> >> +static const struct of_device_id sun4i_lradc_of_match[] = {
>> >> +	{ .compatible = "allwinner,sun4i-a10-lradc-keys", .data =
>> >> &variant_sun4i_a10_lradc}, +	{ .compatible =
>> >> "allwinner,sun8i-a83t-r-lradc", .data = &variant_sun8i_a83t_r_lradc},
>> >> +
>> >
>> > {
>> >
>> >> /* sentinel */ }
>> >> +};
>> >> +MODULE_DEVICE_TABLE(of, sun4i_lradc_of_match);
>> >> +
>> >> +static struct platform_driver sun4i_lradc_driver = {
>> >> +	.driver = {
>> >> +		.name	= "sun4i-lradc-hwmon",
>> >> +		.of_match_table = of_match_ptr(sun4i_lradc_of_match),
>> >> +	},
>> >> +	.probe	= sun4i_lradc_probe,
>> >> +	.remove	= sun4i_lradc_remove,
>> >> +	.resume	= sun4i_lradc_resume,
>> >> +	.suspend = sun4i_lradc_suspend,
>> >> +};
>> >> +
>> >> +module_platform_driver(sun4i_lradc_driver);
>> >> +
>> >> +
>> >> +
>> >> +
>> >> +MODULE_DESCRIPTION("Allwinner A13/A20 LRADC hwmon driver");
>> >> +MODULE_AUTHOR("Ruslan Zalata <rz@fabmicro.ru>");
>> >> +MODULE_LICENSE("GPL");