Re: [PATCH 2/4] common: add wolfvision board code library

[Sascha Hauer <s.hauer@pengutronix.de>]

On Fri, Apr 05, 2024 at 04:04:43PM +0200, Michael Riesch wrote:
> Add board code library for all WolfVision boards.
> 
> Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net>
> ---
>  common/boards/Kconfig              |   3 +
>  common/boards/Makefile             |   1 +
>  common/boards/wolfvision/Makefile  |   2 +
>  common/boards/wolfvision/common.c  | 145 +++++++++++++++++++++++++++++++++++++
>  include/boards/wolfvision/common.h |  32 ++++++++
>  5 files changed, 183 insertions(+)
> 
> diff --git a/common/boards/Kconfig b/common/boards/Kconfig
> index f6d4a56f88..a2a51155ea 100644
> --- a/common/boards/Kconfig
> +++ b/common/boards/Kconfig
> @@ -14,3 +14,6 @@ config BOARD_PHYTEC_SOM_IMX8M_DETECTION
>  config BOARD_TQ
>  	select CRC_ITU_T
>  	bool
> +
> +config BOARD_WOLFVISION
> +	bool
> diff --git a/common/boards/Makefile b/common/boards/Makefile
> index 147c36643d..3f8ac57b2f 100644
> --- a/common/boards/Makefile
> +++ b/common/boards/Makefile
> @@ -3,3 +3,4 @@
>  obj-$(CONFIG_BOARD_QEMU_VIRT)	+= qemu-virt/
>  obj-$(CONFIG_BOARD_PHYTEC_SOM_DETECTION) += phytec/
>  obj-$(CONFIG_BOARD_TQ) += tq/
> +obj-$(CONFIG_BOARD_WOLFVISION) += wolfvision/
> diff --git a/common/boards/wolfvision/Makefile b/common/boards/wolfvision/Makefile
> new file mode 100644
> index 0000000000..b2be4b73f4
> --- /dev/null
> +++ b/common/boards/wolfvision/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +obj-pbl-y += common.o
> diff --git a/common/boards/wolfvision/common.c b/common/boards/wolfvision/common.c
> new file mode 100644
> index 0000000000..188931c24c
> --- /dev/null
> +++ b/common/boards/wolfvision/common.c
> @@ -0,0 +1,145 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Common board code functions WolfVision boards.
> + *
> + * Copyright (C) 2024 WolfVision GmbH.
> + */
> +
> +#include <common.h>
> +#include <aiodev.h>
> +#include <net.h>
> +#include <state.h>
> +
> +#include <boards/wolfvision/common.h>
> +
> +#define WV_RK3568_HWID_TOLERANCE 50
> +
> +int wolfvision_apply_overlay(const struct wv_overlay *overlay, char **files)
> +{
> +	int ret;
> +
> +	if (overlay->filename) {
> +		if (*files) {
> +			char *old = *files;
> +			*files = basprintf("%s %s", old, overlay->filename);
> +			free(old);
> +		} else {
> +			*files = basprintf("%s", overlay->filename);
> +		}
> +	}
> +
> +	if (overlay->data) {
> +		struct device_node *node =
> +			of_unflatten_dtb(overlay->data, INT_MAX);
> +
> +		if (!node) {

of_unflatten_dtb returns an error pointer in case of failure.

> +			pr_err("Cannot unflatten dtbo\n");

Please provide a pt_fmt() macro to give the messages a meaningful
prefix.

> +			return -EINVAL;
> +		}
> +
> +		ret = of_overlay_apply_tree(of_get_root_node(), node);
> +
> +		of_delete_node(node);
> +
> +		if (ret) {
> +			pr_err("Cannot apply overlay: %s\n", strerror(-ret));

Should be "%pe\n", ERR_PTR(ret)

> +			return ret;
> +		}
> +
> +		of_clk_init();
> +		of_probe();
> +	}
> +
> +	return 0;
> +}
> +
> +int wolfvision_register_ethaddr(void)
> +{
> +	struct device_node *eth0;
> +	struct state *state;
> +	char mac[ETH_ALEN];
> +	int ret;
> +
> +	ret = of_device_ensure_probed_by_alias("state");
> +	if (ret)
> +		return ret;
> +
> +	state = state_by_name("state");
> +	if (!state)
> +		return -ENOENT;
> +
> +	ret = state_read_mac(state, "mac-address", mac);
> +	if (ret)
> +		return ret;
> +
> +	if (!is_valid_ether_addr(mac))
> +		return -EINVAL;
> +
> +	eth0 = of_find_node_by_alias(of_get_root_node(), "ethernet0");
> +	if (eth0)
> +		of_eth_register_ethaddr(eth0, mac);
> +
> +	return 0;
> +}
> +
> +int wolfvision_rk3568_get_hwid(int chan_idx)
> +{
> +	const int values[WV_RK3568_HWID_MAX] = {
> +		0,    112,  225,  337,	450,  562,  675,  787,	900,
> +		1012, 1125, 1237, 1350, 1462, 1575, 1687, 1800,
> +	};
> +	struct aiochannel *chan;
> +	int ret, hwid, voltage;
> +	char *chan_name;
> +
> +	chan_name = basprintf("saradc.in_value%d_mV", chan_idx);
> +	chan = aiochannel_by_name(chan_name);
> +	free(chan_name);
> +	if (IS_ERR(chan))
> +		return PTR_ERR(chan);
> +
> +	ret = aiochannel_get_value(chan, &voltage);
> +	if (ret)
> +		return ret;

I just recently added aiochannel_name_get_value() which combines
aiochannel_by_name() and aiochannel_get_value(), you can use this here.

> +
> +	for (hwid = 0; hwid < ARRAY_SIZE(values); hwid++)
> +		if (abs(voltage - values[hwid]) < WV_RK3568_HWID_TOLERANCE)
> +			return hwid;
> +
> +	return -EINVAL;
> +};
> +
> +int wolfvision_rk3568_detect_hw(const struct wv_rk3568_extension *extensions,
> +				int num_extensions, char **overlays)
> +{
> +	int i, hwid, ret;
> +
> +	ret = of_device_ensure_probed_by_alias("saradc");
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < num_extensions; i++) {
> +		const struct wv_rk3568_extension *extension = &extensions[i];
> +		const struct wv_overlay *overlay;
> +
> +		ret = wolfvision_rk3568_get_hwid(extension->adc_chan);
> +		if (ret < 0) {
> +			pr_warning("Could not retrieve %s HWID (%d)\n",
> +				   extension->name, ret);
> +			return ret;

Maybe better continue with the next entry rather than returning an
error?

> +		}
> +
> +		hwid = ret;
> +		overlay = &extension->overlays[hwid];
> +		if (overlay->name) {
> +			pr_info("Detected %s %s\n", overlay->name,
> +				extension->name);
> +			wolfvision_apply_overlay(overlay, overlays);
> +		} else {
> +			pr_warning("Detected unknown %s HWID %d\n",
> +				   extension->name, hwid);
> +		}
> +	}
> +
> +	return 0;
> +}

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |