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

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

[-- 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 --]