Re: [RFC PATCH 1/5] WIP: gnss: merge MTK driver into UBX driver

Wolfram Sang
2023-06-12T11:43:03Z

[-- Attachment #1: Type: text/plain, Size: 3154 bytes --]

On Tue, May 23, 2023 at 08:43:06AM +0200, Wolfram Sang wrote:
> This is a proof-of-concept how easy it is to merge those two drivers as
> they are extremly similar. The differences can be abstracted away
> easily. Further work (renaming from 'ubx' to something more generic,
> removing the MTK driver, ...) is postponed until there is agrement that
> merging these drivers is actually wanted. I vote for it, though. I have
> updates to the UBX driver which do make sense for the MTK driver as
> well. Code saving is also a plus. We can always fork into a specific
> driver again if we'd ever need that.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Johan, just from a gut feeling or a hi level glimpse: is this merging of
the driver worth pursuing?

Thanks and all the best,

Wolfram

> ---
> drivers/gnss/ubx.c | 30 +++++++++++++++++++++++++-----
> 1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gnss/ubx.c b/drivers/gnss/ubx.c
> index c951be202ca2..c01e1e875727 100644
> --- a/drivers/gnss/ubx.c
> +++ b/drivers/gnss/ubx.c
> @@ -63,12 +63,31 @@ static const struct gnss_serial_ops ubx_gserial_ops = {
> .set_power = ubx_set_power,
> };
>
> +struct ubx_info {
> + enum gnss_type type;
> + char *v_bckp_name;
> +};
> +
> +const struct ubx_info ubx_info_ubx = {
> + .type = GNSS_TYPE_UBX,
> + .v_bckp_name = "v-bckp",
> +};
> +
> +const struct ubx_info ubx_info_mtk = {
> + .type = GNSS_TYPE_MTK,
> + .v_bckp_name = "vbackup",
> +};
> +
> static int ubx_probe(struct serdev_device *serdev)
> {
> + const struct ubx_info *info = of_device_get_match_data(&serdev->dev);
> struct gnss_serial *gserial;
> struct ubx_data *data;
> int ret;
>
> + if (!info)
> + return -ENOENT;
> +
> gserial = gnss_serial_allocate(serdev, sizeof(*data));
> if (IS_ERR(gserial)) {
> ret = PTR_ERR(gserial);
> @@ -77,7 +96,7 @@ static int ubx_probe(struct serdev_device *serdev)
>
> gserial->ops = &ubx_gserial_ops;
>
> - gserial->gdev->type = GNSS_TYPE_UBX;
> + gserial->gdev->type = info->type;
>
> data = gnss_serial_get_drvdata(gserial);
>
> @@ -87,7 +106,7 @@ static int ubx_probe(struct serdev_device *serdev)
> goto err_free_gserial;
> }
>
> - data->v_bckp = devm_regulator_get_optional(&serdev->dev, "v-bckp");
> + data->v_bckp = devm_regulator_get_optional(&serdev->dev, info->v_bckp_name);
> if (IS_ERR(data->v_bckp)) {
> ret = PTR_ERR(data->v_bckp);
> if (ret == -ENODEV)
> @@ -130,9 +149,10 @@ static void ubx_remove(struct serdev_device *serdev)
>
> #ifdef CONFIG_OF
> static const struct of_device_id ubx_of_match[] = {
> - { .compatible = "u-blox,neo-6m" },
> - { .compatible = "u-blox,neo-8" },
> - { .compatible = "u-blox,neo-m8" },
> + { .compatible = "u-blox,neo-6m", .data = &ubx_info_ubx, },
> + { .compatible = "u-blox,neo-8", .data = &ubx_info_ubx, },
> + { .compatible = "u-blox,neo-m8", .data = &ubx_info_ubx, },
> + { .compatible = "globaltop,pa6h", .data = &ubx_info_mtk },
> {},
> };
> MODULE_DEVICE_TABLE(of, ubx_of_match);
> --
> 2.35.1
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]