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

[Simon Glass]

Hi Thomas,

On 20 November 2015 at 17:19, Thomas Chou <thomas at 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 at wytron.com.tw> wrote:
>>>
>>> Unify serial_ppc, and use the generic binding.
>>>
>>> Signed-off-by: Thomas Chou <thomas at wytron.com.tw>
>>> Reviewed-by: Tom Rini <trini at 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