[U-Boot] [PATCH v3 05/13] ns16550: unify serial_ppc

Simon Glass
2015-11-21T00:27:26Z

Hi Thomas,

On 20 November 2015 at 17:19, Thomas Chou <thomas@wytron.com.tw> wrote:
>
> Hi Simon,
>
> On 2015?11?21? 01:18, Simon Glass wrote:
>>
>> Hi Thomas,
>>
>> On 19 November 2015 at 06:48, Thomas Chou <thomas@wytron.com.tw> wrote:
>>>
>>> Unify serial_ppc, and use the generic binding.
>>>
>>> Signed-off-by: Thomas Chou <thomas@wytron.com.tw>
>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>> ---
>>> arch/powerpc/include/asm/config.h | 4 ++++
>>> drivers/serial/Kconfig | 2 +-
>>> drivers/serial/Makefile | 1 -
>>> drivers/serial/serial_ppc.c | 40 ---------------------------------------
>>> 4 files changed, 5 insertions(+), 42 deletions(-)
>>> delete mode 100644 drivers/serial/serial_ppc.c
>>>
>>> diff --git a/arch/powerpc/include/asm/config.h b/arch/powerpc/include/asm/config.h
>>> index 65496d0..7391066 100644
>>> --- a/arch/powerpc/include/asm/config.h
>>> +++ b/arch/powerpc/include/asm/config.h
>>> @@ -104,4 +104,8 @@
>>> /* All PPC boards must swap IDE bytes */
>>> #define CONFIG_IDE_SWAP_IO
>>>
>>> +#if defined(CONFIG_DM_SERIAL)
>>> +#define CONFIG_SYS_NS16550_CLK get_serial_clock()
>>> +#endif
>>> +
>
>
> I move the get_serial_clock here.
>
>
>>> #endif /* _ASM_CONFIG_H_ */
>>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>>> index 93faa4c..b41f508 100644
>>> --- a/drivers/serial/Kconfig
>>> +++ b/drivers/serial/Kconfig
>>> @@ -198,7 +198,7 @@ config ROCKCHIP_SERIAL
>>> config NS16550_SERIAL
>>> bool "NS16550 UART or compatible"
>>> depends on DM_SERIAL
>>> - default y if X86
>>> + default y if X86 || PPC
>>> help
>>> Support NS16550 UART or compatible with driver model. This can be
>>> enabled in the device tree with the correct input clock frequency.
>>> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
>>> index 9036a8e..9f61113 100644
>>> --- a/drivers/serial/Makefile
>>> +++ b/drivers/serial/Makefile
>>> @@ -8,7 +8,6 @@
>>> ifdef CONFIG_DM_SERIAL
>>> obj-y += serial-uclass.o
>>> obj-$(CONFIG_PL01X_SERIAL) += serial_pl01x.o
>>> -obj-$(CONFIG_PPC) += serial_ppc.o
>>> else
>>> obj-y += serial.o
>>> obj-$(CONFIG_PL010_SERIAL) += serial_pl01x.o
>>> diff --git a/drivers/serial/serial_ppc.c b/drivers/serial/serial_ppc.c
>>> deleted file mode 100644
>>> index 47141c6..0000000
>>> --- a/drivers/serial/serial_ppc.c
>>> +++ /dev/null
>>> @@ -1,40 +0,0 @@
>>> -/*
>>> - * Copyright (c) 2014 Google, Inc
>>> - *
>>> - * SPDX-License-Identifier: GPL-2.0+
>>> - */
>>> -
>>> -#include <common.h>
>>> -#include <dm.h>
>>> -#include <ns16550.h>
>>> -#include <serial.h>
>>> -
>>> -static const struct udevice_id ppc_serial_ids[] = {
>>> - { .compatible = "ns16550" },
>>> - { }
>>> -};
>>> -
>>> -static int ppc_serial_ofdata_to_platdata(struct udevice *dev)
>>> -{
>>> - struct ns16550_platdata *plat = dev_get_platdata(dev);
>>> - int ret;
>>> -
>>> - ret = ns16550_serial_ofdata_to_platdata(dev);
>>> - if (ret)
>>> - return ret;
>>> - plat->clock = get_serial_clock();
>>
>>
>> You are dropping this call. We certainly don't want it for driver
>> model, but I suspect it will break some PPC boards if they don't have
>> the clock-frequency in the device tree. Do they?
>
>
> I moved it to a macro.
> #define CONFIG_SYS_NS16550_CLK get_serial_clock()

Ah, we shouldn't do that with driver model. This should be in some
sort of clock driver. Perhaps we should leave this serial driver along
for now?

Regards,
Simon