2022-05-03 01:34:32

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v2] hwmon: (sun4i-lradc) Add driver for LRADC found on Allwinner A13/A20 SoC

Hi,

On Thu, Apr 28, 2022 at 09:09:03PM +0000, Ruslan Zalata wrote:
> 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 <[email protected]>
> ---
> MAINTAINERS | 6 +
> drivers/hwmon/Kconfig | 13 ++
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/sun4i-lradc-hwmon.c | 280 ++++++++++++++++++++++++++++++
> 4 files changed, 300 insertions(+)
> create mode 100644 drivers/hwmon/sun4i-lradc-hwmon.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5e8c2f61176..d9c71e94133 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18861,6 +18861,12 @@ S: Maintained
> F: Documentation/devicetree/bindings/input/allwinner,sun4i-a10-lradc-keys.yaml
> F: drivers/input/keyboard/sun4i-lradc-keys.c
>
> +SUN4I LOW RES ADC HWMON DRIVER
> +M: Ruslan Zalata <[email protected]>
> +L: [email protected]
> +S: Maintained
> +F: drivers/hwmon/sun4i-lradc-hwmon.c
> +
> SUNDANCE NETWORK DRIVER
> M: Denis Kirjanov <[email protected]>
> L: [email protected]
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 68a8a27ab3b..86776488a81 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1691,6 +1691,19 @@ config SENSORS_SIS5595
> This driver can also be built as a module. If so, the module
> will be called sis5595.
>
> +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 must be used at a time.

How do you plan on enforcing that?

I guess a better path forward would be to either register an hwmon
device in the original driver, or convert that driver to iio and use
iio-hwmon.

Maxime


Attachments:
(No filename) (2.41 kB)
signature.asc (235.00 B)
Download all attachments

2022-05-03 21:14:40

by Ruslan Zalata

[permalink] [raw]
Subject: Re: [PATCH v2] hwmon: (sun4i-lradc) Add driver for LRADC found on Allwinner A13/A20 SoC

Hi Maxime,

> I guess a better path forward would be to either register an hwmon
> device in the original driver, or convert that driver to iio and use
> iio-hwmon.

My first idea was to add hwmon to sun4i-lradc-keys.c driver. But soon as
I began hacking the driver I quickly realized that it would be a mess
since keyboard and hwmon belong to two different subsystems. Besides we
would need to invent a way to control which way the driver works (new
bindings?). So I decided that a new independent small driver would be a
better solution.

---
Regards,
Ruslan.

Fabmicro, LLC.

2022-05-04 16:52:04

by Maxime Ripard

[permalink] [raw]
Subject: Re: [PATCH v2] hwmon: (sun4i-lradc) Add driver for LRADC found on Allwinner A13/A20 SoC

Hi,

On Tue, May 03, 2022 at 01:50:39PM +0500, Ruslan Zalata wrote:
> > I guess a better path forward would be to either register an hwmon
> > device in the original driver, or convert that driver to iio and use
> > iio-hwmon.
>
> My first idea was to add hwmon to sun4i-lradc-keys.c driver. But soon as I
> began hacking the driver I quickly realized that it would be a mess since
> keyboard and hwmon belong to two different subsystems.

That's not really an issue in itself. There's plenty of drivers in Linux
that register into two frameworks.

> Besides we would need to invent a way to control which way the driver
> works (new bindings?).

I got confused there, and thought you were adding temperature reading
that is in another ADC on those SoCs, sorry. Yeah, that doesn't make
much sense to have both in the same drivers here.

Maxime


Attachments:
(No filename) (867.00 B)
signature.asc (235.00 B)
Download all attachments