On Wed, Oct 11, 2023, at 20:48, Peter Griffin wrote:
> Add serial driver data for Google Tensor gs101 SoC.
>
> Signed-off-by: Peter Griffin <peter.griffin@xxxxxxxxxx>
Reviewed-by: Arnd Bergmann <arnd@xxxxxxxx>
While the patch is now correct, I would point out a few
improvements we could make on top:
> +static const struct s3c24xx_serial_drv_data gs101_serial_drv_data = {
> + EXYNOS_COMMON_SERIAL_DRV_DATA(),
> + /* rely on samsung,uart-fifosize DT property for fifosize */
> + .fifosize = { 0 },
> +};
> +
> #define EXYNOS4210_SERIAL_DRV_DATA (&exynos4210_serial_drv_data)
> #define EXYNOS5433_SERIAL_DRV_DATA (&exynos5433_serial_drv_data)
> #define EXYNOS850_SERIAL_DRV_DATA (&exynos850_serial_drv_data)
> +#define GS101_SERIAL_DRV_DATA (&gs101_serial_drv_data)
Since this is now actually correct for any Exynos variant that
has the FIFO size listed in the DT, we could use a variable/macro
name that leads itself to being used by future chips.
There is also the question of whether we want to address the
ordering bug for the other SoC types. The way I understand it,
the .fifosize array logic is wrong because it relies on having
a particular alias for each of the ports to match the entry in
the array. For the exynosautov9, this would be trivially fixed
by using the same data as gs101 (since it already lists the
correct size in DT), but for the other ones we'd need a different
logic.
> @@ -2688,6 +2696,9 @@ static const struct platform_device_id
> s3c24xx_serial_driver_ids[] = {
> }, {
> .name = "artpec8-uart",
> .driver_data = (kernel_ulong_t)ARTPEC8_SERIAL_DRV_DATA,
> + }, {
> + .name = "gs101-uart",
> + .driver_data = (kernel_ulong_t)GS101_SERIAL_DRV_DATA,
> },
> { },
> };
I just noticed that the platform_device_id array is currently
only used for mach-crag6410, since everything else uses DT
based probing. s3c64xx is scheduled for removal in early 2024
(though no patch has been sent), and we can probably just
remove all the atags/platform_device based code when that happens.
Arnd
