Hi Simon,
On Fri, May 25, 2018 at 4:42 AM, Simon Glass <s...@chromium.org> wrote:
> Hi Mario,
>
> On 24 May 2018 at 02:42, Mario Six <mario....@gdsys.cc> wrote:
>> Add a clock driver for the MPC83xx architecture.
>>
>> Signed-off-by: Mario Six <mario....@gdsys.cc>
>>
>> ---
>>
>> v2 -> v3:
>> * Added driver files to MAINTAINERS
>>
>> v1 -> v2:
>> * Added binding of sysreset driver
>>
>> ---
>> MAINTAINERS | 3 +
>> arch/powerpc/cpu/mpc83xx/speed.c | 4 +
>> arch/powerpc/include/asm/config.h | 2 +-
>> drivers/clk/Kconfig | 6 +
>> drivers/clk/Makefile | 1 +
>> drivers/clk/mpc83xx_clk.c | 426
>> ++++++++++++++++++++++++++++++++++
>> drivers/clk/mpc83xx_clk.h | 121 ++++++++++
>> include/dt-bindings/clk/mpc83xx-clk.h | 33 +++
>> 8 files changed, 595 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/clk/mpc83xx_clk.c
>> create mode 100644 drivers/clk/mpc83xx_clk.h
>> create mode 100644 include/dt-bindings/clk/mpc83xx-clk.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index b43e4bc179d..ab0f6a0a5d0 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -471,6 +471,9 @@ T: git git://git.denx.de/u-boot-mpc83xx.git
>> F: drivers/ram/mpc83xx_sdram.c
>> F: include/dt-bindings/memory/mpc83xx-sdram.h
>> F: drivers/sysreset/sysreset_mpc83xx.c
>> +F: drivers/clk/mpc83xx_clk.c
>> +F: drivers/clk/mpc83xx_clk.h
>> +F: include/dt-bindings/clk/mpc83xx-clk.h
>> F: arch/powerpc/cpu/mpc83xx/
>> F: arch/powerpc/include/asm/arch-mpc83xx/
>>
>> diff --git a/arch/powerpc/cpu/mpc83xx/speed.c
>> b/arch/powerpc/cpu/mpc83xx/speed.c
>> index f0945281cd2..39bc1c53406 100644
>> --- a/arch/powerpc/cpu/mpc83xx/speed.c
>> +++ b/arch/powerpc/cpu/mpc83xx/speed.c
>> @@ -6,6 +6,8 @@
>> * Copyright (C) 2004-2007 Freescale Semiconductor, Inc.
>> */
>>
>> +#ifndef CONFIG_CLK_MPC83XX
>> +
>> #include <common.h>
>> #include <mpc83xx.h>
>> #include <command.h>
>> @@ -590,3 +592,5 @@ U_BOOT_CMD(clocks, 1, 0, do_clocks,
>> "print clock configuration",
>> " clocks"
>> );
>> +
>> +#endif
>> diff --git a/arch/powerpc/include/asm/config.h
>> b/arch/powerpc/include/asm/config.h
>> index 284cfe21ab0..7bc8f5006ec 100644
>> --- a/arch/powerpc/include/asm/config.h
>> +++ b/arch/powerpc/include/asm/config.h
>> @@ -78,7 +78,7 @@
>> /* All PPC boards must swap IDE bytes */
>> #define CONFIG_IDE_SWAP_IO
>>
>> -#if defined(CONFIG_DM_SERIAL)
>> +#if defined(CONFIG_DM_SERIAL) && !defined(CONFIG_CLK_MPC83XX)
>> /*
>> * TODO: Convert this to a clock driver exists that can give us the UART
>> * clock here.
>> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
>> index edb4ca58ea5..e6ebff0a9d4 100644
>> --- a/drivers/clk/Kconfig
>> +++ b/drivers/clk/Kconfig
>> @@ -98,4 +98,10 @@ config ICS8N3QV01
>> Crystal Oscillator). The output frequency can be programmed via an
>> I2C interface.
>>
>> +config CLK_MPC83XX
>> + bool "Enable MPC83xx clock driver"
>> + depends on CLK
>> + help
>> + Support for the clock driver of the MPC83xx series of SoCs.
>> +
>> endmenu
>> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
>> index 58139b13a89..58f497d3a15 100644
>> --- a/drivers/clk/Makefile
>> +++ b/drivers/clk/Makefile
>> @@ -16,6 +16,7 @@ obj-$(CONFIG_CLK_BCM6345) += clk_bcm6345.o
>> obj-$(CONFIG_CLK_BOSTON) += clk_boston.o
>> obj-$(CONFIG_CLK_EXYNOS) += exynos/
>> obj-$(CONFIG_CLK_HSDK) += clk-hsdk-cgu.o
>> +obj-$(CONFIG_CLK_MPC83XX) += mpc83xx_clk.o
>> obj-$(CONFIG_CLK_RENESAS) += renesas/
>> obj-$(CONFIG_CLK_STM32F) += clk_stm32f.o
>> obj-$(CONFIG_CLK_STM32MP1) += clk_stm32mp1.o
>> diff --git a/drivers/clk/mpc83xx_clk.c b/drivers/clk/mpc83xx_clk.c
>> new file mode 100644
>> index 00000000000..80be597332d
>> --- /dev/null
>> +++ b/drivers/clk/mpc83xx_clk.c
>> @@ -0,0 +1,426 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * (C) Copyright 2017
>> + * Mario Six, Guntermann & Drunck GmbH, mario....@gdsys.cc
>> + */
>> +
>> +#include <common.h>
>> +#include <clk-uclass.h>
>> +#include <dm.h>
>> +#include <dm/lists.h>
>> +#include <dt-bindings/clk/mpc83xx-clk.h>
>> +
>> +#include "mpc83xx_clk.h"
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +static u32 *speed;
>> +
>> +struct mpc83xx_clk_priv {
>> + u32 *speed;
>> +};
>> +
>> +static const char * const names[] = {
>> + [MPC83XX_CLK_CORE] = "Core",
>> + [MPC83XX_CLK_CSB] = "Coherent System Bus",
>> + [MPC83XX_CLK_QE] = "QE",
>> + [MPC83XX_CLK_BRG] = "BRG",
>> + [MPC83XX_CLK_LBIU] = "Local Bus Controller",
>> + [MPC83XX_CLK_LCLK] = "Local Bus",
>> + [MPC83XX_CLK_MEM] = "DDR",
>> + [MPC83XX_CLK_MEM_SEC] = "DDR Secondary",
>> + [MPC83XX_CLK_ENC] = "SEC",
>> + [MPC83XX_CLK_I2C1] = "I2C1",
>> + [MPC83XX_CLK_I2C2] = "I2C2",
>> + [MPC83XX_CLK_TDM] = "TDM",
>> + [MPC83XX_CLK_SDHC] = "SDHC",
>> + [MPC83XX_CLK_TSEC1] = "TSEC1",
>> + [MPC83XX_CLK_TSEC2] = "TSEC2",
>> + [MPC83XX_CLK_USBDR] = "USB DR",
>> + [MPC83XX_CLK_USBMPH] = "USB MPH",
>> + [MPC83XX_CLK_PCIEXP1] = "PCIEXP1",
>> + [MPC83XX_CLK_PCIEXP2] = "PCIEXP2",
>> + [MPC83XX_CLK_SATA] = "SATA",
>> + [MPC83XX_CLK_DMAC] = "DMAC",
>> + [MPC83XX_CLK_PCI] = "PCI",
>> +};
>> +
>> +struct clk_mode {
>> + u8 low;
>> + u8 high;
>> + int type;
>> +};
>> +
>> +const struct clk_mode modes[] = {
>> + [MPC83XX_CLK_CORE] = {0, 0, TYPE_SPECIAL},
>> + [MPC83XX_CLK_CSB] = {0, 0, TYPE_SPECIAL},
>> + [MPC83XX_CLK_QE] = {0, 0, TYPE_SPECIAL},
>> + [MPC83XX_CLK_BRG] = {0, 0, TYPE_SPECIAL},
>> + [MPC83XX_CLK_MEM] = {1, 1, TYPE_SPMR_DIRECT_MULTIPLY },
>> + [MPC83XX_CLK_LBIU] = {0, 0, TYPE_SPMR_DIRECT_MULTIPLY },
>> + [MPC83XX_CLK_LCLK] = {0, 0, TYPE_SPECIAL},
>> + [MPC83XX_CLK_MEM_SEC] = {0, 0, TYPE_SPMR_DIRECT_MULTIPLY }, /* The
>> same as LBIU */
>> +#ifndef CONFIG_MPC8313
>> + [MPC83XX_CLK_TSEC1] = {0, 1, TYPE_SCCR_STANDARD },
>> + [MPC83XX_CLK_TSEC2] = {2, 3, TYPE_SCCR_STANDARD },
>> +#else
>> + [MPC83XX_CLK_TSEC1] = {0, 1, TYPE_SCCR_STANDARD }, /* FIXME: This
>> has separate enable/disable bit! */
>> + [MPC83XX_CLK_TSEC2] = {0, 1, TYPE_SCCR_STANDARD }, /* FIXME: This
>> has separate enable/disable bit! */
>> +#endif
>
> Do you really need all these #ifdefs? It's really unfortunate to have
> a driver which requires build-time #ifdefs. You have the device tree
> which should be enough to determine what to do. What is the issue
> here?
>
The code was mostly copied from the old MPC83xx support, which uses ifdefs to
distinguish between the different types of MPC83xx SoCs, which all have
slightly different memory layout in regards to the clock registers.
It shouldn't be too hard to make the decision depend on the compatible string.
Will be done for v4.
> Regards,
> Simon
Best regards,
Mario
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
