Received-SPF: pass (google.com: domain of linux-kernel+bounces-190372-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <568162b8-563e-4ec7-8f31-bd16f329b245@web.de>
Date: Mon, 27 May 2024 11:45:34 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 3/3] drm/loongson: Add dummy gpu driver as a
 subcomponent
To: Sui Jingfeng <sui.jingfeng@linux.dev>,
 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
 Maxime Ripard <mripard@kernel.org>, Thomas Zimmermann <tzimmermann@suse.de>,
 dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org
References: <20240526195826.109008-1-sui.jingfeng@linux.dev>
 <20240526195826.109008-4-sui.jingfeng@linux.dev>
Content-Language: en-GB
From: Markus Elfring <Markus.Elfring@web.de>
In-Reply-To: <20240526195826.109008-4-sui.jingfeng@linux.dev>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
UI-OutboundReport: notjunk:1;M01:P0:NYLnz5L0av4=;5CfIPCZ4HKcxCI2iE3OBU+9r8Bt
 w5qKMdMZfB67NYxwP5F1YmER+TqhJqLpK6o3i50no16Ges0MRsRcxMC3HUhKeMiqgrnmZSHkk
 fHzHpaEk6Cr0yXYSuRgdicvOer7cIAJW91+8rgiQPqX0Ary0ci/LAHwxIllS5Z3NO/s2CufuP
 0iv4MwA2Uw4uFRU7az5H/dvVzI6MHe11dQ82PCD8ltsJuZ1wDAwdDapX4oALNzDrEuMrBWkvd
 uHi8d7AYgEwcmssD07f9MiiHUFwgcTb0iZ+ZI2eglFxqGR3UGAtUn3SBRy81Sy7+XYrPlZY6r
 w+n5GD+ZgiAS1f85rq/ABDBRdn2sIzIKebfQIEzmr6SHqX3n0HcD5izWZxne4MXNxDw8KqMIt
 0OS7gfL4nirEwf2ElcWKV5uPDgbpkHa530WofBRh+CSbcVqMTYAR+ljji8eWJ1atLmuz/fwza
 FWZtQUw3uikcg/Xd+05b5B+wO+9RKDg7/nz4umi1z1CL3MVm+bLZwGyy1rabDzGx6UwWWWp0A
 Euau3irXtpGFPwACV/ApUq/uqdljH+YyaBmmzRnNIfCOxHQe5RnQeqaUpyY/Qff31A/KWR1Ft
 LD1R2h7W4ApeREWynbYTfQGdck9Yo/aEUOskFLPLRO9dDliETE+4X3yMrrh0IYDGnHGVuQ4FI
 UJ97DTe2CbktgSf1nLFfh7jjhTDbx7chHyZrY1AtFN3r5VRf5DkuQCJifHnFW9Bc8sIqrYwTd
 Bd44se47iXPe/+dANwGYsL+/lb0C90+TpNG616vpjOD97T3DZMa0RUF9d78aJfahQeo0xWwnk
 71koy2ilJNryfj+61v34WbPNAM5lMEwA8Ozl77HYetsO8=

=E2=80=A6
> loose coupling, but still be able to works togather to provide a unified

  use loose?          should?          work together?            an?


=E2=80=A6
> Add a dummy driver for the GPU, it functional as a subcomponent as well.

                                     is?


Please improve your change descriptions considerably.


=E2=80=A6
> +++ b/drivers/gpu/drm/loongson/loongson_module.c
> @@ -29,8 +29,15 @@ static int __init loongson_module_init(void)
>  	if (ret)
>  		return ret;
>
> +	ret =3D pci_register_driver(&loong_gpu_pci_driver);
> +	if (ret) {
> +		platform_driver_unregister(&lsdc_output_port_platform_driver);
> +		return ret;
> +	}
> +
>  	ret =3D pci_register_driver(&lsdc_pci_driver);
>  	if (ret) {
> +		pci_unregister_driver(&loong_gpu_pci_driver);
>  		platform_driver_unregister(&lsdc_output_port_platform_driver);
>  		return ret;
>  	}

How do you think about to use another goto chain for common exception hand=
ling?
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+go=
to+chain+when+leaving+a+function+on+error+when+using+and+releasing+resourc=
es

Would you become interested in the application of scope-based resource man=
agement here?


> @@ -43,6 +50,8 @@ static void __exit loongson_module_exit(void)
>  {
>  	pci_unregister_driver(&lsdc_pci_driver);
>
> +	pci_unregister_driver(&loong_gpu_pci_driver);
> +
>  	platform_driver_unregister(&lsdc_output_port_platform_driver);
>  }

I suggest to avoid blank lines for this function implementation.

Regards,
Markus