MIPS: intel: add initial support for Intel MIPS SoCs
Subsystems (4)
v2 cover MIPS: intel: add initial support for Intel MIPS SoCs
This patch series is for adding the support for Intel MIPS interAptiv SoC GRX500 family. It includes CCF support, serial driver optimization and DTS modification.
This patch series is applied on top of linux v4.18-rc7. Basic verification is performed on GRX500 board.
We propose merging this patch series into MIPS Linux tree.
Changes in v2:
- Remove unused _END macros
- Remove the redundant check and not accurate comments
- Replace the get_counter_resolution function with fixed value 2
- Use obj-y and split into per line per .o
- Add EVA mapping description in code comments
- Remove unused include header file
- Do a clean-up for grx500_defconfig
- Rewrite clock driver, add platform clock description details in clock driver.
- Rewrite clock driver's dt-binding document according to Rob Herring's comments.
- Simplify device tree docoment, remove some clock description.
- New patch split from previous patch
- The memory address is changed to @20000000
- Update to obj-$(CONFIG_BUILTIN_DTB) as per commit fca3aa166422
- New patch split from previous patch
- Add the board and chip compatible in dt document
- New patch to reorder the head files according to the coding style.
Hua Ma (3):
MIPS: intel: Add initial support for Intel MIPS SoCs
MIPS: dts: Add initial support for Intel MIPS SoCs
dt-binding: MIPS: Add documentation of Intel MIPS SoCs
Songjun Wu (13):
MIPS: dts: Change upper case to lower case
MIPS: dts: Add aliases node for lantiq danube serial
serial: intel: Get serial id from dts
serial: intel: Change ltq_w32_mask to asc_update_bits
MIPS: lantiq: Unselect SWAP_IO_SPACE when LANTIQ is selected
serial: intel: Use readl/writel instead of ltq_r32/ltq_w32
serial: intel: Rename fpiclk to freqclk
serial: intel: Replace clk_enable/clk_disable with clk generic API
serial: intel: Add CCF support
serial: intel: Support more platform
serial: intel: Reorder the head files
serial: intel: Change init_lqasc to static declaration
dt-bindings: serial: lantiq: Add optional properties for CCF
Yixin Zhu (2):
clk: intel: Add clock driver for Intel MIPS SoCs
dt-bindings: clk: Add documentation of grx500 clock controller
.../devicetree/bindings/clock/intel,grx500-clk.txt | 39 ++ Documentation/devicetree/bindings/mips/intel.txt | 17 + .../devicetree/bindings/serial/lantiq_asc.txt | 15 + arch/mips/Kbuild.platforms | 1 + arch/mips/Kconfig | 30 +- arch/mips/boot/dts/Makefile | 1 + arch/mips/boot/dts/intel-mips/Makefile | 4 + arch/mips/boot/dts/intel-mips/easy350_anywan.dts | 26 ++ arch/mips/boot/dts/intel-mips/xrx500.dtsi | 66 +++ arch/mips/boot/dts/lantiq/danube.dtsi | 42 +- arch/mips/boot/dts/lantiq/easy50712.dts | 18 +- arch/mips/configs/grx500_defconfig | 138 ++++++ .../asm/mach-intel-mips/cpu-feature-overrides.h | 61 +++ arch/mips/include/asm/mach-intel-mips/ioremap.h | 39 ++ arch/mips/include/asm/mach-intel-mips/irq.h | 17 + .../asm/mach-intel-mips/kernel-entry-init.h | 104 +++++ arch/mips/include/asm/mach-intel-mips/spaces.h | 27 ++ arch/mips/include/asm/mach-intel-mips/war.h | 18 + arch/mips/intel-mips/Kconfig | 22 + arch/mips/intel-mips/Makefile | 5 + arch/mips/intel-mips/Platform | 12 + arch/mips/intel-mips/irq.c | 35 ++ arch/mips/intel-mips/prom.c | 172 ++++++++ arch/mips/intel-mips/time.c | 42 ++ drivers/clk/Kconfig | 1 + drivers/clk/Makefile | 3 + drivers/clk/intel/Kconfig | 20 + drivers/clk/intel/Makefile | 7 + drivers/clk/intel/clk-cgu-pll.c | 166 ++++++++ drivers/clk/intel/clk-cgu-pll.h | 34 ++ drivers/clk/intel/clk-cgu.c | 470 +++++++++++++++++++++ drivers/clk/intel/clk-cgu.h | 259 ++++++++++++ drivers/clk/intel/clk-grx500.c | 168 ++++++++ drivers/tty/serial/Kconfig | 2 +- drivers/tty/serial/lantiq.c | 143 ++++--- include/dt-bindings/clock/intel,grx500-clk.h | 69 +++ 36 files changed, 2206 insertions(+), 87 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/intel,grx500-clk.txt create mode 100644 Documentation/devicetree/bindings/mips/intel.txt create mode 100644 arch/mips/boot/dts/intel-mips/Makefile create mode 100644 arch/mips/boot/dts/intel-mips/easy350_anywan.dts create mode 100644 arch/mips/boot/dts/intel-mips/xrx500.dtsi create mode 100644 arch/mips/configs/grx500_defconfig create mode 100644 arch/mips/include/asm/mach-intel-mips/cpu-feature-overrides.h create mode 100644 arch/mips/include/asm/mach-intel-mips/ioremap.h create mode 100644 arch/mips/include/asm/mach-intel-mips/irq.h create mode 100644 arch/mips/include/asm/mach-intel-mips/kernel-entry-init.h create mode 100644 arch/mips/include/asm/mach-intel-mips/spaces.h create mode 100644 arch/mips/include/asm/mach-intel-mips/war.h create mode 100644 arch/mips/intel-mips/Kconfig create mode 100644 arch/mips/intel-mips/Makefile create mode 100644 arch/mips/intel-mips/Platform create mode 100644 arch/mips/intel-mips/irq.c create mode 100644 arch/mips/intel-mips/prom.c create mode 100644 arch/mips/intel-mips/time.c create mode 100644 drivers/clk/intel/Kconfig create mode 100644 drivers/clk/intel/Makefile create mode 100644 drivers/clk/intel/clk-cgu-pll.c create mode 100644 drivers/clk/intel/clk-cgu-pll.h create mode 100644 drivers/clk/intel/clk-cgu.c create mode 100644 drivers/clk/intel/clk-cgu.h create mode 100644 drivers/clk/intel/clk-grx500.c create mode 100644 include/dt-bindings/clock/intel,grx500-clk.h
Signature · 1 line
2.11.0
PATCH v2 1/18 MIPS: intel: Add initial support for Intel MIPS SoCs
From: Hua Ma <[email protected]>
Add initial support for Intel MIPS interAptiv SoCs made by Intel. This series will add support for the grx500 family.
The series allows booting a minimal system using a initramfs.
---
Changes in v2:
- Remove unused _END macros
- Remove the redundant check and not accurate comments
- Replace the get_counter_resolution function with fixed value 2
- Use obj-y and split into per line per .o
- Add EVA mapping description in code comments
- Remove unused include header file
- Do a clean-up for grx500_defconfig
arch/mips/Kbuild.platforms | 1 + arch/mips/Kconfig | 29 ++++ arch/mips/configs/grx500_defconfig | 138 +++++++++++++++++ .../asm/mach-intel-mips/cpu-feature-overrides.h | 61 ++++++++ arch/mips/include/asm/mach-intel-mips/ioremap.h | 39 +++++ arch/mips/include/asm/mach-intel-mips/irq.h | 17 ++ .../asm/mach-intel-mips/kernel-entry-init.h | 104 +++++++++++++ arch/mips/include/asm/mach-intel-mips/spaces.h | 27 ++++ arch/mips/include/asm/mach-intel-mips/war.h | 18 +++ arch/mips/intel-mips/Kconfig | 22 +++ arch/mips/intel-mips/Makefile | 5 + arch/mips/intel-mips/Platform | 12 ++ arch/mips/intel-mips/irq.c | 35 +++++ arch/mips/intel-mips/prom.c | 172 +++++++++++++++++++++ arch/mips/intel-mips/time.c | 42 +++++ 15 files changed, 722 insertions(+) create mode 100644 arch/mips/configs/grx500_defconfig create mode 100644 arch/mips/include/asm/mach-intel-mips/cpu-feature-overrides.h create mode 100644 arch/mips/include/asm/mach-intel-mips/ioremap.h create mode 100644 arch/mips/include/asm/mach-intel-mips/irq.h create mode 100644 arch/mips/include/asm/mach-intel-mips/kernel-entry-init.h create mode 100644 arch/mips/include/asm/mach-intel-mips/spaces.h create mode 100644 arch/mips/include/asm/mach-intel-mips/war.h create mode 100644 arch/mips/intel-mips/Kconfig create mode 100644 arch/mips/intel-mips/Makefile create mode 100644 arch/mips/intel-mips/Platform create mode 100644 arch/mips/intel-mips/irq.c create mode 100644 arch/mips/intel-mips/prom.c create mode 100644 arch/mips/intel-mips/time.c
arch/mips/Kbuild.platforms +1 -0
@@ -12,6 +12,7 @@ platforms += cobalt platforms += dec platforms += emma platforms += generic +platforms += intel-mips platforms += jazz platforms += jz4740 platforms += lantiq
arch/mips/Kconfig +29 -0
@@ -409,6 +409,34 @@ config LANTIQ select ARCH_HAS_RESET_CONTROLLER select RESET_CONTROLLER +config INTEL_MIPS + bool "Intel MIPS interAptiv SoC based platforms" + select BOOT_RAW + select CEVT_R4K + select COMMON_CLK + select CPU_MIPS32_3_5_EVA + select CPU_MIPS32_3_5_FEATURES + select CPU_MIPSR2_IRQ_EI + select CPU_MIPSR2_IRQ_VI + select CSRC_R4K + select DMA_NONCOHERENT + select GENERIC_ISA_DMA + select IRQ_MIPS_CPU + select MFD_CORE + select MFD_SYSCON + select MIPS_CPU_SCACHE + select MIPS_GIC + select SYS_HAS_CPU_MIPS32_R1 + select SYS_HAS_CPU_MIPS32_R2 + select SYS_HAS_CPU_MIPS32_R3_5 + select SYS_SUPPORTS_BIG_ENDIAN + select SYS_SUPPORTS_32BIT_KERNEL + select SYS_SUPPORTS_MIPS_CPS + select SYS_SUPPORTS_MULTITHREADING + select SYS_SUPPORTS_ZBOOT + select TIMER_OF + select USE_OF + config LASAT bool "LASAT Networks platforms" select CEVT_R4K
@@ -1016,6 +1044,7 @@ source "arch/mips/bcm47xx/Kconfig" source "arch/mips/bcm63xx/Kconfig" source "arch/mips/bmips/Kconfig" source "arch/mips/generic/Kconfig" +source "arch/mips/intel-mips/Kconfig" source "arch/mips/jazz/Kconfig" source "arch/mips/jz4740/Kconfig" source "arch/mips/lantiq/Kconfig"
arch/mips/configs/grx500_defconfig new file +138 -0
138 lines not on this page · raw message
arch/mips/include/asm/mach-intel-mips/cpu-feature-overrides.h new file +61 -0
61 lines not on this page · raw message
arch/mips/include/asm/mach-intel-mips/ioremap.h new file +39 -0
@@ -0,0 +1,39 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* Copyright (C) 2014 Lei Chuanhua <[email protected]> +* Copyright (C) 2018 Intel Corporation. +*/ +#ifndef __ASM_MACH_INTEL_MIPS_IOREMAP_H +#define __ASM_MACH_INTEL_MIPS_IOREMAP_H + +#include <linux/types.h> + +static inline phys_addr_t fixup_bigphys_addr (phys_addr_t phys_addr, +phys_addr_t size) +{ +return phys_addr; +} + +/* +* TOP IO Space definition for SSX7 components /PCIe/ToE/Memcpy +* physical 0xa0000000 --> virtual 0xe0000000 +*/ +#define GRX500_TOP_IOREMAP_BASE 0xA0000000 +#define GRX500_TOP_IOREMAP_SIZE 0x20000000 +#define GRX500_TOP_IOREMAP_PHYS_VIRT_OFFSET 0x40000000 + +static inline void __iomem *plat_ioremap (phys_addr_t offset,unsigned long size, +unsigned long flags) +{ +if (offset >=GRX500_TOP_IOREMAP_BASE && + offset < (GRX500_TOP_IOREMAP_BASE +GRX500_TOP_IOREMAP_SIZE )) +return (void __iomem *)(unsigned long ) + (offset +GRX500_TOP_IOREMAP_PHYS_VIRT_OFFSET ); +return NULL ; +} + +static inline int plat_iounmap (const volatile void __iomem *addr) +{ +return (unsigned long )addr >= (unsigned long )GRX500_TOP_IOREMAP_BASE ; +} +#endif /* __ASM_MACH_INTEL_MIPS_IOREMAP_H */
arch/mips/include/asm/mach-intel-mips/irq.h new file +17 -0
@@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* Copyright (C) 2014 Lei Chuanhua <[email protected]> +* Copyright (C) 2018 Intel Corporation. +*/ + +#ifndef __INTEL_MIPS_IRQ_H +#define __INTEL_MIPS_IRQ_H + +#define MIPS_CPU_IRQ_BASE 0 +#define MIPS_GIC_IRQ_BASE (MIPS_CPU_IRQ_BASE + 8) + +#define NR_IRQS 256 + +#include_next <irq.h> + +#endif /* __INTEL_MIPS_IRQ_H */
arch/mips/include/asm/mach-intel-mips/kernel-entry-init.h new file +104 -0
104 lines not on this page · raw message
arch/mips/include/asm/mach-intel-mips/spaces.h new file +27 -0
@@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* Author: Leonid Yegoshin ([email protected]) +* Copyright (C) 2012 MIPS Technologies, Inc. +* Copyright (C) 2014 Lei Chuanhua <[email protected]> +* Copyright (C) 2018 Intel Corporation. +*/ + +#ifndef _ASM_INTEL_MIPS_SPACES_H +#define _ASM_INTEL_MIPS_SPACES_H + +#define PAGE_OFFSET _AC(0x60000000, UL) +#define PHYS_OFFSET _AC(0x20000000, UL) + +/* No Highmem Support */ +#define HIGHMEM_START _AC(0xffff0000, UL) + +#define FIXADDR_TOP ((unsigned long)(long)(int)0xcffe0000) + +#define IO_SIZE _AC(0x10000000, UL) +#define IO_SHIFT _AC(0x10000000, UL) + +/* IO space one */ +#define __pa_symbol (x) __pa(x) + +#include <asm/mach-generic/spaces.h> +#endif /* __ASM_INTEL_MIPS_SPACES_H */
arch/mips/include/asm/mach-intel-mips/war.h new file +18 -0
@@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __ASM_MIPS_MACH_INTEL_MIPS_WAR_H +#define __ASM_MIPS_MACH_INTEL_MIPS_WAR_H + +#define R4600_V1_INDEX_ICACHEOP_WAR 0 +#define R4600_V1_HIT_CACHEOP_WAR 0 +#define R4600_V2_HIT_CACHEOP_WAR 0 +#define R5432_CP0_INTERRUPT_WAR 0 +#define BCM1250_M3_WAR 0 +#define SIBYTE_1956_WAR 0 +#define MIPS4K_ICACHE_REFILL_WAR 0 +#define MIPS_CACHE_SYNC_WAR 0 +#define TX49XX_ICACHE_INDEX_INV_WAR 0 +#define ICACHE_REFILLS_WORKAROUND_WAR 0 +#define R10000_LLSC_WAR 0 +#define MIPS34K_MISSED_ITLB_WAR 0 + +#endif /* __ASM_MIPS_MACH_INTEL_MIPS_WAR_H */
arch/mips/intel-mips/Kconfig new file +22 -0
@@ -0,0 +1,22 @@ +if INTEL_MIPS + +choice + prompt "Built-in device tree" + help + Legacy bootloaders do not pass a DTB pointer to the kernel, so + if a "wrapper" is not being used, the kernel will need to include + a device tree that matches the target board. + + The builtin DTB will only be used if the firmware does not supply + a valid DTB. + +config DTB_INTEL_MIPS_NONE + bool "None" + +config DTB_INTEL_MIPS_GRX500 + bool "Intel MIPS GRX500 Board" + select BUILTIN_DTB + +endchoice + +endif
arch/mips/intel-mips/Makefile new file +5 -0
@@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0 + +obj-y += prom.o +obj-y += irq.o +obj-y += time.o
arch/mips/intel-mips/Platform new file +12 -0
@@ -0,0 +1,12 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# MIPS SoC platform +# + +platform-$(CONFIG_INTEL_MIPS) += intel-mips/ +cflags-$(CONFIG_INTEL_MIPS) += -I$(srctree)/arch/mips/include/asm/mach-intel-mips +ifdef CONFIG_EVA + load-$(CONFIG_INTEL_MIPS) = 0xffffffff60020000 +else + load-$(CONFIG_INTEL_MIPS) = 0xffffffff80020000 +endif
arch/mips/intel-mips/irq.c new file +35 -0
@@ -0,0 +1,35 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (C) 2016 Intel Corporation. +*/ +#include <linux/init.h> +#include <linux/irqchip.h> +#include <linux/of_irq.h> +#include <asm/irq.h> +#include <asm/irq_cpu.h> + +void __init arch_init_irq (void ) +{ +struct device_node *intc_node; + +pr_info ("EIC is %s\n" , cpu_has_veic ?"on" :"off" ); +pr_info ("VINT is %s\n" , cpu_has_vint ?"on" :"off" ); + + intc_node =of_find_compatible_node (NULL ,NULL , +"mti,cpu-interrupt-controller" ); +if (!cpu_has_veic && !intc_node) +mips_cpu_irq_init (); + +irqchip_init (); +} + +int get_c0_perfcount_int (void ) +{ +return gic_get_c0_perfcount_int (); +} +EXPORT_SYMBOL_GPL (get_c0_perfcount_int); + +unsigned int get_c0_compare_int (void ) +{ +return gic_get_c0_compare_int (); +}
arch/mips/intel-mips/prom.c new file +172 -0
172 lines not on this page · raw message
arch/mips/intel-mips/time.c new file +42 -0
42 lines not on this page · raw message
Signature · 1 line
2.11.0
Re: [PATCH v2 01/18] MIPS: intel: Add initial support for Intel MIPS SoCs
Hi Songjun / Hua,
Quoted 6 lines · Songjun Wu
From: Hua Ma <[email protected]>
Add initial support for Intel MIPS interAptiv SoCs made by Intel. This series will add support for the grx500 family.
The series allows booting a minimal system using a initramfs.
Thanks for submitting this - I have some comments below.
arch/mips/Kbuild.platforms platforms += cobalt · line 18
Quoted 12 lines
arch/mips/Kbuild.platforms +1
-0@@ -12,6 +12,7 @@ platforms += cobalt platforms += dec platforms += emma platforms += generic +platforms += intel-mips platforms += jazz platforms += jz4740 platforms += lantiq
Oh EVA, why must you ruin nice things... Ideally I'd be suggesting that we use the generic platform but it doesn't yet have a nice way to deal with non-standard EVA setups.
It would be good if we could make sure that's the only reason for your custom platform though, so that once generic does support EVA we can migrate your system over. Most notably, it would be good to make use of the UHI-specified boot protocol if possible (ie. $r4==-2, $r5==&dtb).
It looks like your prom_init_cmdline() supports multiple boot protocols
- could you clarify which is actually used?
Quoted 16 lines
arch/mips/Kconfig +8
-0@@ -409,6 +409,34 @@ config LANTIQ select ARCH_HAS_RESET_CONTROLLER select RESET_CONTROLLER +config INTEL_MIPS + bool "Intel MIPS interAptiv SoC based platforms" + select BOOT_RAW + select CEVT_R4K + select COMMON_CLK + select CPU_MIPS32_3_5_EVA + select CPU_MIPS32_3_5_FEATURES + select CPU_MIPSR2_IRQ_EI
arch/mips/Kconfig config LANTIQ · line 429
Quoted 10 lines
arch/mips/Kconfig +10
-0+ select CPU_MIPSR2_IRQ_VI + select CSRC_R4K + select DMA_NONCOHERENT + select GENERIC_ISA_DMA + select IRQ_MIPS_CPU + select MFD_CORE + select MFD_SYSCON + select MIPS_CPU_SCACHE + select MIPS_GIC + select SYS_HAS_CPU_MIPS32_R1
For a system based on interAptiv you should never need to build a MIPS32r1 kernel, so you should remove the above select.
Quoted 55 lines
arch/mips/include/asm/mach-intel-mips/cpu-feature-overrides.h new file +49
-0@@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* This file was derived from: include/asm-mips/cpu-features.h +* Copyright (C) 2003, 2004 Ralf Baechle +* Copyright (C) 2004 Maciej W. Rozycki +* Copyright (C) 2018 Intel Corporation. +*/ + +#ifndef __ASM_MACH_INTEL_MIPS_CPU_FEATURE_OVERRIDES_H +#define __ASM_MACH_INTEL_MIPS_CPU_FEATURE_OVERRIDES_H + +#define cpu_has_tlb 1 +#define cpu_has_4kex 1 +#define cpu_has_3k_cache 0 +#define cpu_has_4k_cache 1 +#define cpu_has_tx39_cache 0 +#define cpu_has_sb1_cache 0 +#define cpu_has_fpu 0 +#define cpu_has_32fpr 0 +#define cpu_has_counter 1 +#define cpu_has_watch 1 +#define cpu_has_divec 1 + +#define cpu_has_prefetch 1 +#define cpu_has_ejtag 1 +#define cpu_has_llsc 1 + +#define cpu_has_mips16 0 +#define cpu_has_mdmx 0 +#define cpu_has_mips3d 0 +#define cpu_has_smartmips 0 +#define cpu_has_mmips 0 +#define cpu_has_vz 0 + +#define cpu_has_mips32r1 1 +#define cpu_has_mips32r2 1 +#define cpu_has_mips64r1 0 +#define cpu_has_mips64r2 0 + +#define cpu_has_dsp 1 +#define cpu_has_dsp2 0 +#define cpu_has_mipsmt 1 + +#define cpu_has_vint 1 +#define cpu_has_veic 0 + +#define cpu_has_64bits 0 +#define cpu_has_64bit_zero_reg 0 +#define cpu_has_64bit_gp_regs 0
arch/mips/include/asm/mach-intel-mips/cpu-feature-overrides.h line 59
Quoted 10 lines
arch/mips/include/asm/mach-intel-mips/cpu-feature-overrides.h new file +10
-0+#define cpu_has_64bit_addresses 0 + +#define cpu_has_cm2 1 +#define cpu_has_cm2_l2sync 1 +#define cpu_has_eva 1 +#define cpu_has_tlbinv 1 + +#define cpu_dcache_line_size () 32 +#define cpu_icache_line_size () 32 +#define cpu_scache_line_size () 32
If you rebase atop linux-next or mips-next then you should find that many of these defines are now redundant, especially after removing the SYS_HAS_CPU_MIPS32_R1 select which means your kernel build will always target MIPS32r2.
Due to architectural restrictions on where various options can be present, you should be able to remove:
- cpu_has_4kex
- cpu_has_3k_cache
- cpu_has_4k_cache
- cpu_has_32fpr
- cpu_has_divec
- cpu_has_prefetch
- cpu_has_llsc
cpu_has_mmips defaults to a compile-time zero unless you select CONFIG_SYS_SUPPORTS_MICROMIPS, so please remove that one.
cpu_has_64bit_gp_regs & cpu_has_64bit_addresses both default to zero already for 32bit kernel builds, so please remove those.
cpu_has_cm2 & cpu_has_cm2_l2sync don't exist anywhere in-tree, so please remove those.
Additionally cpu_has_sb1_cache is not used anywhere, or defined by asm/cpu-features.h & seems to just be left over in some platform override files including presumably one you based yours on. Please remove it too.
Also you select CPU_MIPSR2_IRQ_EI but define cpu_has_veic as 0, could you check that mismatch?
Quoted 7 lines
arch/mips/include/asm/mach-intel-mips/irq.h new file +1
-0@@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0 */
arch/mips/include/asm/mach-intel-mips/irq.h line 11
Quoted 10 lines
arch/mips/include/asm/mach-intel-mips/irq.h new file +10
-0+/* +* Copyright (C) 2014 Lei Chuanhua <[email protected]> +* Copyright (C) 2018 Intel Corporation. +*/ + +#ifndef __INTEL_MIPS_IRQ_H +#define __INTEL_MIPS_IRQ_H + +#define MIPS_CPU_IRQ_BASE 0 +#define MIPS_GIC_IRQ_BASE (MIPS_CPU_IRQ_BASE + 8)
These 2 defines are the defaults anyway, so please remove them.
arch/mips/include/asm/mach-intel-mips/irq.h line 13
Quoted 1 line
arch/mips/include/asm/mach-intel-mips/irq.h new file +1
-0+#define NR_IRQS 256
And if you don't actually need this then you could remove irq.h entirely
- do you actually use more than 128 IRQs?
arch/mips/include/asm/mach-intel-mips/spaces.h line 21
Quoted 8 lines
arch/mips/include/asm/mach-intel-mips/spaces.h new file +0
-0% >% >%
arch/mips/include/asm/mach-intel-mips/spaces.h new file +2
-0+#define IO_SIZE _AC(0x10000000, UL) +#define IO_SHIFT _AC(0x10000000, UL)
These IO_ defines don't appear to be used anywhere?
arch/mips/include/asm/mach-intel-mips/spaces.h line 24
Quoted 2 lines
arch/mips/include/asm/mach-intel-mips/spaces.h new file +2
-0+/* IO space one */ +#define __pa_symbol (x) __pa(x)
Can you explain why you need this, rather than the default definition of __pa_symbol()? The comment doesn't seem to describe much of anything.
Quoted 14 lines
arch/mips/include/asm/mach-intel-mips/war.h new file +8
-0@@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __ASM_MIPS_MACH_INTEL_MIPS_WAR_H +#define __ASM_MIPS_MACH_INTEL_MIPS_WAR_H + +#define R4600_V1_INDEX_ICACHEOP_WAR 0 +#define R4600_V1_HIT_CACHEOP_WAR 0 +#define R4600_V2_HIT_CACHEOP_WAR 0 +#define R5432_CP0_INTERRUPT_WAR 0
arch/mips/include/asm/mach-intel-mips/war.h line 18
Quoted 10 lines
arch/mips/include/asm/mach-intel-mips/war.h new file +10
-0+#define BCM1250_M3_WAR 0 +#define SIBYTE_1956_WAR 0 +#define MIPS4K_ICACHE_REFILL_WAR 0 +#define MIPS_CACHE_SYNC_WAR 0 +#define TX49XX_ICACHE_INDEX_INV_WAR 0 +#define ICACHE_REFILLS_WORKAROUND_WAR 0 +#define R10000_LLSC_WAR 0 +#define MIPS34K_MISSED_ITLB_WAR 0 + +#endif /* __ASM_MIPS_MACH_INTEL_MIPS_WAR_H */
Since you need none of these workarounds, you shouldn't need war.h at all.
Quoted 18 lines
arch/mips/intel-mips/Kconfig new file +12
-0@@ -0,0 +1,22 @@ +if INTEL_MIPS + +choice + prompt "Built-in device tree" + help + Legacy bootloaders do not pass a DTB pointer to the kernel, so + if a "wrapper" is not being used, the kernel will need to include + a device tree that matches the target board. + + The builtin DTB will only be used if the firmware does not supply + a valid DTB. +
arch/mips/intel-mips/Kconfig line 22
Quoted 10 lines
arch/mips/intel-mips/Kconfig new file +10
-0+config DTB_INTEL_MIPS_NONE + bool "None" + +config DTB_INTEL_MIPS_GRX500 + bool "Intel MIPS GRX500 Board" + select BUILTIN_DTB + +endchoice + +endif
So do you actually have both styles of bootloader?
Quoted 9 lines
arch/mips/intel-mips/prom.c new file +0
-0% >% >%
arch/mips/intel-mips/prom.c new file +3
-0+static void __init prom_init_cmdline (void ) +{ +int i;
arch/mips/intel-mips/prom.c line 42
Quoted 10 lines
arch/mips/intel-mips/prom.c new file +10
-0+int argc; +char **argv; + +/* +* If u-boot pass parameters, it is ok, however, if without u-boot +* JTAG or other tool has to reset all register value before it goes +* emulation most likely belongs to this category +*/ +if (fw_arg0 ==0 || fw_arg1 ==0 ) +return ;
I don't understand what you're trying to say here, or why this check exists. If you boot with fw_arg0 == fw_arg1 == 0 then you'd just hit the loop below right, and execute zero iterations of it? That seems like it would be fine without this special case.
Quoted 15 lines
arch/mips/intel-mips/prom.c new file +15
-0+/* +* a0: fw_arg0 - the number of string in init cmdline +* a1: fw_arg1 - the address of string in init cmdline +* +* In accordance with the MIPS UHI specification, +* the bootloader can pass the following arguments to the kernel: +* - $a0: -2. +* - $a1: KSEG0 address of the flattened device-tree blob. +*/ +if (fw_arg0 ==-2 ) +return ; + + argc = fw_arg0; + argv = (char **)KSEG1ADDR (fw_arg1); +
arch/mips/intel-mips/prom.c line 68
Quoted 10 lines
arch/mips/intel-mips/prom.c new file +10
-0+ arcs_cmdline[0 ] ='\0' ; + +for (i =0 ; i < argc; i++) { +char *p = (char *)KSEG1ADDR (argv[i]); + +if (argv[i] && *p) { +strlcat (arcs_cmdline, p,sizeof (arcs_cmdline)); +strlcat (arcs_cmdline," " ,sizeof (arcs_cmdline)); + } + }
Why do you need to use kseg1? Why can't the arguments be accessed cached?
Are the arguments passed as virtual or physical addresses? If virtual & we can access them cached then you could replace all of this with a call to fw_init_cmdline().
Quoted 12 lines
arch/mips/intel-mips/prom.c new file +12
-0+static int __init plat_enable_iocoherency (void ) +{ +if (!mips_cps_numiocu (0 )) +return 0 ; + +/* Nothing special needs to be done to enable coherency */ +pr_info ("Coherence Manager IOCU detected\n" ); +/* Second IOCU for MPE or other master access register */ +write_gcr_reg0_base (0xa0000000 ); +write_gcr_reg0_mask (0xf8000000 | CM_GCR_REGn_MASK_CMTGT_IOCU1); +return 1 ; +}
arch/mips/intel-mips/prom.c line 92
Quoted 10 lines
arch/mips/intel-mips/prom.c new file +10
-0+ +static void __init plat_setup_iocoherency (void ) +{ +if (plat_enable_iocoherency () && + coherentio ==IO_COHERENCE_DISABLED ) { +pr_info ("Hardware DMA cache coherency disabled\n" ); +return ; + } +panic ("This kind of IO coherency is not supported!" ); +}
Since you select CONFIG_DMA_NONCOHERENT in Kconfig, coherentio will always equal IO_COHERENCE_DISABLED. That suggests to me that the above 2 functions are probably useless, or at least needlessly convoluted.
arch/mips/intel-mips/prom.c line 172
Quoted 8 lines
arch/mips/intel-mips/prom.c new file +8
-0+static int __init plat_publish_devices (void ) +{ +if (!of_have_populated_dt ()) +return 0 ; +return of_platform_populate (NULL , of_default_bus_match_table,NULL , +NULL ); +} +arch_initcall (plat_publish_devices);
The core DT code calls of_platform_populate() already (see of_platform_default_populate_init()), so you can remove this function.
Thanks,
Paul
Re: [PATCH v2 01/18] MIPS: intel: Add initial support for Intel MIPS SoCs
Quoted 10 lines · Paul Burton
Hi Songjun / Hua,
Quoted 6 lines · Songjun Wu
From: Hua Ma <[email protected]>
Add initial support for Intel MIPS interAptiv SoCs made by Intel. This series will add support for the grx500 family.
The series allows booting a minimal system using a initramfs.
Thanks for submitting this - I have some comments below.
Thanks for the review.
Quoted 7 lines
Quoted 7 lines
arch/mips/Kbuild.platforms +0
-0@@ -12,6 +12,7 @@ platforms += cobalt platforms += dec platforms += emma
Quoted 8 lines
arch/mips/Kbuild.platforms platforms += cobalt · line 18
Quoted 5 lines
arch/mips/Kbuild.platforms +1
-0platforms += generic +platforms += intel-mips platforms += jazz platforms += jz4740 platforms += lantiqOh EVA, why must you ruin nice things... Ideally I'd be suggesting that we use the generic platform but it doesn't yet have a nice way to deal with non-standard EVA setups.
yes, we only support EVA.
Quoted 7 lines
It would be good if we could make sure that's the only reason for your custom platform though, so that once generic does support EVA we can migrate your system over. Most notably, it would be good to make use of the UHI-specified boot protocol if possible (ie. $r4==-2, $r5==&dtb).
It looks like your
prom_init_cmdline()supports multiple boot protocols
- could you clarify which is actually used?
this patch only support build-in dts, we will do a clean up.
Quoted 20 lines
Quoted 20 lines
arch/mips/Kconfig +12
-0@@ -409,6 +409,34 @@ config LANTIQ select ARCH_HAS_RESET_CONTROLLER select RESET_CONTROLLER +config INTEL_MIPS + bool "Intel MIPS interAptiv SoC based platforms" + select BOOT_RAW + select CEVT_R4K + select COMMON_CLK + select CPU_MIPS32_3_5_EVA + select CPU_MIPS32_3_5_FEATURES + select CPU_MIPSR2_IRQ_EI + select CPU_MIPSR2_IRQ_VI + select CSRC_R4K + select DMA_NONCOHERENT + select GENERIC_ISA_DMA
Quoted 8 lines
arch/mips/Kconfig config LANTIQ · line 429
Quoted 6 lines
arch/mips/Kconfig +6
-0+ select IRQ_MIPS_CPU + select MFD_CORE + select MFD_SYSCON + select MIPS_CPU_SCACHE + select MIPS_GIC + select SYS_HAS_CPU_MIPS32_R1For a system based on interAptiv you should never need to build a MIPS32r1 kernel, so you should remove the above select.
will remove.
Quoted 87 lines
Quoted 55 lines
arch/mips/include/asm/mach-intel-mips/cpu-feature-overrides.h new file +49
-0@@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* This file was derived from: include/asm-mips/cpu-features.h +* Copyright (C) 2003, 2004 Ralf Baechle +* Copyright (C) 2004 Maciej W. Rozycki +* Copyright (C) 2018 Intel Corporation. +*/ + +#ifndef __ASM_MACH_INTEL_MIPS_CPU_FEATURE_OVERRIDES_H +#define __ASM_MACH_INTEL_MIPS_CPU_FEATURE_OVERRIDES_H + +#define cpu_has_tlb 1 +#define cpu_has_4kex 1 +#define cpu_has_3k_cache 0 +#define cpu_has_4k_cache 1 +#define cpu_has_tx39_cache 0 +#define cpu_has_sb1_cache 0 +#define cpu_has_fpu 0 +#define cpu_has_32fpr 0 +#define cpu_has_counter 1 +#define cpu_has_watch 1 +#define cpu_has_divec 1 + +#define cpu_has_prefetch 1 +#define cpu_has_ejtag 1 +#define cpu_has_llsc 1 + +#define cpu_has_mips16 0 +#define cpu_has_mdmx 0 +#define cpu_has_mips3d 0 +#define cpu_has_smartmips 0 +#define cpu_has_mmips 0 +#define cpu_has_vz 0 + +#define cpu_has_mips32r1 1 +#define cpu_has_mips32r2 1 +#define cpu_has_mips64r1 0 +#define cpu_has_mips64r2 0 + +#define cpu_has_dsp 1 +#define cpu_has_dsp2 0 +#define cpu_has_mipsmt 1 + +#define cpu_has_vint 1 +#define cpu_has_veic 0 + +#define cpu_has_64bits 0 +#define cpu_has_64bit_zero_reg 0 +#define cpu_has_64bit_gp_regs 0arch/mips/include/asm/mach-intel-mips/cpu-feature-overrides.h line 59
Quoted 10 lines
arch/mips/include/asm/mach-intel-mips/cpu-feature-overrides.h new file +10
-0+#define cpu_has_64bit_addresses 0 + +#define cpu_has_cm2 1 +#define cpu_has_cm2_l2sync 1 +#define cpu_has_eva 1 +#define cpu_has_tlbinv 1 + +#define cpu_dcache_line_size () 32 +#define cpu_icache_line_size () 32 +#define cpu_scache_line_size () 32If you rebase atop linux-next or mips-next then you should find that many of these defines are now redundant, especially after removing the
SYS_HAS_CPU_MIPS32_R1select which means your kernel build will always target MIPS32r2.Due to architectural restrictions on where various options can be present, you should be able to remove:
-
cpu_has_4kex
-cpu_has_3k_cache
-cpu_has_4k_cache
-cpu_has_32fpr
-cpu_has_divec
-cpu_has_prefetch
-cpu_has_llsc
cpu_has_mmipsdefaults to a compile-time zero unless you selectCONFIG_SYS_SUPPORTS_MICROMIPS, so please remove that one.
cpu_has_64bit_gp_regs&cpu_has_64bit_addressesboth default to zero already for 32bit kernel builds, so please remove those.
Quoted 7 lines
cpu_has_cm2&cpu_has_cm2_l2syncdon't exist anywhere in-tree, so please remove those.Additionally
cpu_has_sb1_cacheis not used anywhere, or defined byasm/cpu-features.h& seems to just be left over in some platform override files including presumably one you based yours on. Please remove it too.
Thanks, will remove.
Quoted 2 lines
Also you select
CPU_MIPSR2_IRQ_EIbut definecpu_has_veicas 0, could you check that mismatch?
The hardware does support, but the software does not support.
Quoted 10 lines
Quoted 10 lines
arch/mips/include/asm/mach-intel-mips/irq.h new file +4
-0@@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* Copyright (C) 2014 Lei Chuanhua <[email protected]> +* Copyright (C) 2018 Intel Corporation.
Quoted 8 lines
arch/mips/include/asm/mach-intel-mips/irq.h line 11
Quoted 7 lines
arch/mips/include/asm/mach-intel-mips/irq.h new file +7
-0+*/ + +#ifndef __INTEL_MIPS_IRQ_H +#define __INTEL_MIPS_IRQ_H + +#define MIPS_CPU_IRQ_BASE 0 +#define MIPS_GIC_IRQ_BASE (MIPS_CPU_IRQ_BASE + 8)These 2 defines are the defaults anyway, so please remove them.
Thanks, will remove.
Quoted 3 lines
arch/mips/include/asm/mach-intel-mips/irq.h line 13
Quoted 1 line
arch/mips/include/asm/mach-intel-mips/irq.h new file +1
-0+#define NR_IRQS 256And if you don't actually need this then you could remove
irq.hentirely
- do you actually use more than 128 IRQs?
Yes, the hardware support 256 IRQs.
Quoted 9 lines
arch/mips/include/asm/mach-intel-mips/spaces.h line 21
Quoted 8 lines
arch/mips/include/asm/mach-intel-mips/spaces.h new file +0
-0% >% >%
arch/mips/include/asm/mach-intel-mips/spaces.h new file +2
-0+#define IO_SIZE _AC(0x10000000, UL) +#define IO_SHIFT _AC(0x10000000, UL)These
IO_defines don't appear to be used anywhere?
Thanks, will remove.
Quoted 4 lines
arch/mips/include/asm/mach-intel-mips/spaces.h line 24
Quoted 2 lines
arch/mips/include/asm/mach-intel-mips/spaces.h new file +2
-0+/* IO space one */ +#define __pa_symbol (x) __pa(x)Can you explain why you need this, rather than the default definition of
__pa_symbol()? The comment doesn't seem to describe much of anything.
Thanks, will remove.
Quoted 18 lines
Quoted 18 lines
arch/mips/include/asm/mach-intel-mips/war.h new file +12
-0@@ -0,0 +1,18 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __ASM_MIPS_MACH_INTEL_MIPS_WAR_H +#define __ASM_MIPS_MACH_INTEL_MIPS_WAR_H + +#define R4600_V1_INDEX_ICACHEOP_WAR 0 +#define R4600_V1_HIT_CACHEOP_WAR 0 +#define R4600_V2_HIT_CACHEOP_WAR 0 +#define R5432_CP0_INTERRUPT_WAR 0 +#define BCM1250_M3_WAR 0 +#define SIBYTE_1956_WAR 0 +#define MIPS4K_ICACHE_REFILL_WAR 0 +#define MIPS_CACHE_SYNC_WAR 0
Quoted 8 lines
arch/mips/include/asm/mach-intel-mips/war.h line 18
Quoted 6 lines
arch/mips/include/asm/mach-intel-mips/war.h new file +6
-0+#define TX49XX_ICACHE_INDEX_INV_WAR 0 +#define ICACHE_REFILLS_WORKAROUND_WAR 0 +#define R10000_LLSC_WAR 0 +#define MIPS34K_MISSED_ITLB_WAR 0 + +#endif /* __ASM_MIPS_MACH_INTEL_MIPS_WAR_H */ Since you need none of these workarounds, you shouldn't need
war.hat all.
Thanks, will remove this file.
Quoted 21 lines
Quoted 21 lines
arch/mips/intel-mips/Kconfig new file +15
-0@@ -0,0 +1,22 @@ +if INTEL_MIPS + +choice + prompt "Built-in device tree" + help + Legacy bootloaders do not pass a DTB pointer to the kernel, so + if a "wrapper" is not being used, the kernel will need to include + a device tree that matches the target board. + + The builtin DTB will only be used if the firmware does not supply + a valid DTB. + +config DTB_INTEL_MIPS_NONE + bool "None" +
Quoted 8 lines
arch/mips/intel-mips/Kconfig line 22
Quoted 7 lines
arch/mips/intel-mips/Kconfig new file +7
-0+config DTB_INTEL_MIPS_GRX500 + bool "Intel MIPS GRX500 Board" + select BUILTIN_DTB + +endchoice + +endifSo do you actually have both styles of bootloader?
this patch only support the build-in, will do a clean up.
Quoted 15 lines
Quoted 15 lines
arch/mips/intel-mips/prom.c new file +0
-0% >% >%
arch/mips/intel-mips/prom.c new file +9
-0+static void __init prom_init_cmdline (void ) +{ +int i; +int argc; +char **argv; + +/* +* If u-boot pass parameters, it is ok, however, if without u-boot +* JTAG or other tool has to reset all register value before it goes
Quoted 8 lines
arch/mips/intel-mips/prom.c line 42
Quoted 4 lines
arch/mips/intel-mips/prom.c new file +4
-0+* emulation most likely belongs to this category +*/ +if (fw_arg0 ==0 || fw_arg1 ==0 ) +return ;I don't understand what you're trying to say here, or why this check exists. If you boot with
fw_arg0==fw_arg1== 0 then you'd just hit the loop below right, and execute zero iterations of it? That seems like it would be fine without this special case.
this patch do not support this , will remove.
Quoted 28 lines
Quoted 15 lines
arch/mips/intel-mips/prom.c new file +15
-0+/* +* a0: fw_arg0 - the number of string in init cmdline +* a1: fw_arg1 - the address of string in init cmdline +* +* In accordance with the MIPS UHI specification, +* the bootloader can pass the following arguments to the kernel: +* - $a0: -2. +* - $a1: KSEG0 address of the flattened device-tree blob. +*/ +if (fw_arg0 ==-2 ) +return ; + + argc = fw_arg0; + argv = (char **)KSEG1ADDR (fw_arg1); +arch/mips/intel-mips/prom.c line 68
Quoted 10 lines
arch/mips/intel-mips/prom.c new file +10
-0+ arcs_cmdline[0 ] ='\0' ; + +for (i =0 ; i < argc; i++) { +char *p = (char *)KSEG1ADDR (argv[i]); + +if (argv[i] && *p) { +strlcat (arcs_cmdline, p,sizeof (arcs_cmdline)); +strlcat (arcs_cmdline," " ,sizeof (arcs_cmdline)); + } + }Why do you need to use kseg1? Why can't the arguments be accessed cached?
Quoted 3 lines
Are the arguments passed as virtual or physical addresses? If virtual & we can access them cached then you could replace all of this with a call to
fw_init_cmdline().
this patch only support build-in dts, will remove .
Quoted 17 lines
Quoted 17 lines
arch/mips/intel-mips/prom.c new file +17
-0+static int __init plat_enable_iocoherency (void ) +{ +if (!mips_cps_numiocu (0 )) +return 0 ; + +/* Nothing special needs to be done to enable coherency */ +pr_info ("Coherence Manager IOCU detected\n" ); +/* Second IOCU for MPE or other master access register */ +write_gcr_reg0_base (0xa0000000 ); +write_gcr_reg0_mask (0xf8000000 | CM_GCR_REGn_MASK_CMTGT_IOCU1); +return 1 ; +} + +static void __init plat_setup_iocoherency (void ) +{ +if (plat_enable_iocoherency () && + coherentio ==IO_COHERENCE_DISABLED ) {
Quoted 8 lines
arch/mips/intel-mips/prom.c line 92
Quoted 5 lines
arch/mips/intel-mips/prom.c new file +5
-0+pr_info ("Hardware DMA cache coherency disabled\n" ); +return ; + } +panic ("This kind of IO coherency is not supported!" ); +}Since you select
CONFIG_DMA_NONCOHERENTinKconfig, coherentio will always equalIO_COHERENCE_DISABLED. That suggests to me that the above 2 functions are probably useless, or at least needlessly convoluted.
Thanks, will remove.
Quoted 11 lines
arch/mips/intel-mips/prom.c line 172
Quoted 8 lines
arch/mips/intel-mips/prom.c new file +8
-0+static int __init plat_publish_devices (void ) +{ +if (!of_have_populated_dt ()) +return 0 ; +return of_platform_populate (NULL , of_default_bus_match_table,NULL , +NULL ); +} +arch_initcall (plat_publish_devices);The core DT code calls
of_platform_populate()already (seeof_platform_default_populate_init()), so you can remove this function.
Quoted 2 lines
Thanks,
Paul
Thanks, will remove.
PATCH v2 2/18 clk: intel: Add clock driver for Intel MIPS SoCs
In reply to Songjun WuFrom: Yixin Zhu <[email protected]>
This driver provides PLL clock registration as well as various clock branches, e.g. MUX clock, gate clock, divider clock and so on.
PLLs that provide clock to DDR, CPU and peripherals are shown below:
+---------+
|--->| LCPLL3 0|--PCIe clk-->
XO | +---------+
+-----------|
| +---------+
| | 3|--PAE clk-->
|--->| PLL0B 2|--GSWIP clk-->
| | 1|--DDR clk-->DDR PHY clk-->
| | 0|--CPU1 clk--+ +-----+
| +---------+ |--->0 |
| | MUX |--CPU clk-->
| +---------+ |--->1 |
| | 0|--CPU0 clk--+ +-----+
|--->| PLLOA 1|--SSX4 clk-->
| 2|--NGI clk-->
| 3|--CBM clk-->
+---------+---
Changes in v2:
- Rewrite clock driver, add platform clock description details in clock driver.
drivers/clk/Kconfig | 1 + drivers/clk/Makefile | 3 + drivers/clk/intel/Kconfig | 20 ++ drivers/clk/intel/Makefile | 7 + drivers/clk/intel/clk-cgu-pll.c | 166 ++++++++++ drivers/clk/intel/clk-cgu-pll.h | 34 ++ drivers/clk/intel/clk-cgu.c | 470 +++++++++++++++++++++++++++ drivers/clk/intel/clk-cgu.h | 259 +++++++++++++++ drivers/clk/intel/clk-grx500.c | 168 ++++++++++ include/dt-bindings/clock/intel,grx500-clk.h | 69 ++++ 10 files changed, 1197 insertions(+) create mode 100644 drivers/clk/intel/Kconfig create mode 100644 drivers/clk/intel/Makefile create mode 100644 drivers/clk/intel/clk-cgu-pll.c create mode 100644 drivers/clk/intel/clk-cgu-pll.h create mode 100644 drivers/clk/intel/clk-cgu.c create mode 100644 drivers/clk/intel/clk-cgu.h create mode 100644 drivers/clk/intel/clk-grx500.c create mode 100644 include/dt-bindings/clock/intel,grx500-clk.h
drivers/clk/Kconfig +1 -0
@@ -281,6 +281,7 @@ source "drivers/clk/actions/Kconfig" source "drivers/clk/bcm/Kconfig" source "drivers/clk/hisilicon/Kconfig" source "drivers/clk/imgtec/Kconfig" +source "drivers/clk/intel/Kconfig" source "drivers/clk/keystone/Kconfig" source "drivers/clk/mediatek/Kconfig" source "drivers/clk/meson/Kconfig"
drivers/clk/Makefile +3 -0
@@ -72,6 +72,9 @@ obj-$(CONFIG_ARCH_HISI) += hisilicon/ obj-y += imgtec/ obj-$(CONFIG_ARCH_MXC) += imx/ obj-$(CONFIG_MACH_INGENIC) += ingenic/ +ifeq ($(CONFIG_COMMON_CLK), y) +obj-y +=intel/ +endif obj-$(CONFIG_ARCH_KEYSTONE) += keystone/ obj-$(CONFIG_MACH_LOONGSON32) += loongson1/ obj-y += mediatek/
drivers/clk/intel/Kconfig new file +20 -0
@@ -0,0 +1,20 @@ +# SPDX-License-Identifier: GPL-2.0 +config INTEL_CGU_CLK + depends on COMMON_CLK + depends on INTEL_MIPS || COMPILE_TEST + select MFD_SYSCON + bool "Intel clock controller support" + help + This driver support Intel CGU (Clock Generation Unit). + +choice + prompt "SoC platform selection" + depends on INTEL_CGU_CLK + default INTEL_GRX500_CGU_CLK + +config INTEL_GRX500_CGU_CLK + bool "GRX500 CLK" + help + Clock driver of GRX500 platform. + +endchoice
drivers/clk/intel/Makefile new file +7 -0
@@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0 +# Makefile for intel specific clk + +obj-$(CONFIG_INTEL_CGU_CLK) += clk-cgu.o clk-cgu-pll.o +ifneq ($(CONFIG_INTEL_GRX500_CGU_CLK),) + obj-y += clk-grx500.o +endif
drivers/clk/intel/clk-cgu-pll.c new file +166 -0
166 lines not on this page · raw message
drivers/clk/intel/clk-cgu-pll.h new file +34 -0
@@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* Copyright(c) 2018 Intel Corporation. +* Zhu YiXin <[email protected]> +*/ + +#ifndef __INTEL_CLK_PLL_H +#define __INTEL_CLK_PLL_H + +enum intel_pll_type { + pll_grx500, +}; + +struct intel_pll_rate_table { +unsigned long prate; +unsigned long rate; +unsigned int mult; +unsigned int div; +unsigned int frac; +}; + +struct intel_clk_pll { +struct clk_hw hw; +struct regmap *map; +unsigned int reg; +unsigned long flags; +unsigned int mult; +unsigned int div; +unsigned int frac; +unsigned int table_sz; +const struct intel_pll_rate_table *rate_table; +}; + +#endif /* __INTEL_CLK_PLL_H */
drivers/clk/intel/clk-cgu.c new file +470 -0
470 lines not on this page · raw message
drivers/clk/intel/clk-cgu.h new file +259 -0
259 lines not on this page · raw message
drivers/clk/intel/clk-grx500.c new file +168 -0
168 lines not on this page · raw message
include/dt-bindings/clock/intel,grx500-clk.h new file +69 -0
69 lines not on this page · raw message
Signature · 1 line
2.11.0
Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
Quoted 44 lines · Songjun Wu
From: Yixin Zhu <[email protected]>
This driver provides PLL clock registration as well as various clock branches, e.g. MUX clock, gate clock, divider clock and so on.
PLLs that provide clock to DDR, CPU and peripherals are shown below:
+---------+ |--->| LCPLL3 0|--PCIe clk--> XO | +---------+ +-----------| | +---------+ | | 3|--PAE clk--> |--->| PLL0B 2|--GSWIP clk--> | | 1|--DDR clk-->DDR PHY clk--> | | 0|--CPU1 clk--+ +-----+ | +---------+ |--->0 | | | MUX |--CPU clk--> | +---------+ |--->1 | | | 0|--CPU0 clk--+ +-----+ |--->| PLLOA 1|--SSX4 clk--> | 2|--NGI clk--> | 3|--CBM clk--> +---------+Signed-off-by Yixin Zhu <[email protected]>Signed-off-by Songjun Wu <[email protected]>---
Changes in v2:
- Rewrite clock driver, add platform clock description details in clock driver.drivers/clk/Kconfig | 1 + drivers/clk/Makefile | 3 + drivers/clk/intel/Kconfig | 20 ++ drivers/clk/intel/Makefile | 7 + drivers/clk/intel/clk-cgu-pll.c | 166 ++++++++++ drivers/clk/intel/clk-cgu-pll.h | 34 ++ drivers/clk/intel/clk-cgu.c | 470 +++++++++++++++++++++++++++ drivers/clk/intel/clk-cgu.h | 259 +++++++++++++++ drivers/clk/intel/clk-grx500.c | 168 ++++++++++
Quoted 1 line
include/dt-bindings/clock/intel,grx500-clk.h | 69 ++++
This belongs with the clk binding patch.
Rob
Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
Quoted 44 lines · Rob Herring
Quoted 33 lines · Songjun Wu
From: Yixin Zhu <[email protected]>
This driver provides PLL clock registration as well as various clock branches, e.g. MUX clock, gate clock, divider clock and so on.
PLLs that provide clock to DDR, CPU and peripherals are shown below:
+---------+ |--->| LCPLL3 0|--PCIe clk--> XO | +---------+ +-----------| | +---------+ | | 3|--PAE clk--> |--->| PLL0B 2|--GSWIP clk--> | | 1|--DDR clk-->DDR PHY clk--> | | 0|--CPU1 clk--+ +-----+ | +---------+ |--->0 | | | MUX |--CPU clk--> | +---------+ |--->1 | | | 0|--CPU0 clk--+ +-----+ |--->| PLLOA 1|--SSX4 clk--> | 2|--NGI clk--> | 3|--CBM clk--> +---------+Signed-off-by Yixin Zhu <[email protected]>Signed-off-by Songjun Wu <[email protected]>---
Changes in v2:
- Rewrite clock driver, add platform clock description details in clock driver.Quoted 10 lines
drivers/clk/Kconfig | 1 + drivers/clk/Makefile | 3 + drivers/clk/intel/Kconfig | 20 ++ drivers/clk/intel/Makefile | 7 + drivers/clk/intel/clk-cgu-pll.c | 166 ++++++++++ drivers/clk/intel/clk-cgu-pll.h | 34 ++ drivers/clk/intel/clk-cgu.c | 470 +++++++++++++++++++++++++++ drivers/clk/intel/clk-cgu.h | 259 +++++++++++++++ drivers/clk/intel/clk-grx500.c | 168 ++++++++++ include/dt-bindings/clock/intel,grx500-clk.h | 69 ++++
Quoted 3 lines
This belongs with the clk binding patch.
Rob
Thanks for review.
Will move it to clock binding patch.
Quoted 1 line
Stephen Boyd
maintainerRe: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
In reply to Songjun WuQuoting Songjun Wu (2018-08-02 20:02:21)
Quoted 19 lines
From: Yixin Zhu <[email protected]>
=This driver provides PLL clock registration as well as various clock branches, e.g. MUX clock, gate clock, divider clock and so on. =
PLLs that provide clock to DDR, CPU and peripherals are shown below:
=+---------+ |--->| LCPLL3 0|--PCIe clk--> XO | +---------+ +-----------| | +---------+ | | 3|--PAE clk--> |--->| PLL0B 2|--GSWIP clk--> | | 1|--DDR clk-->DDR PHY clk--> | | 0|--CPU1 clk--+ +-----+
Quoted 8 lines
| +---------+ |--->0 |
| | MUX |--CPU clk-->
| +---------+ |--->1 |
| | 0|--CPU0 clk--+ +-----+
|--->| PLLOA 1|--SSX4 clk-->
| 2|--NGI clk-->
| 3|--CBM clk-->
+---------+Thanks for the picture!
drivers/clk/Makefile obj-$(CONFIG_ARCH_HISI) +=3D hisi=
Quoted 9 lines
=
Signed-off-by Yixin Zhu <[email protected]>Signed-off-by Songjun Wu <[email protected]>drivers/clk/Makefile +0
-0@@ -72,6 +72,9 @@ obj-$(CONFIG_ARCH_HISI) +=3D hisi=
licon/
drivers/clk/Makefile obj-$(CONFIG_ARCH_HISI) +=3D hisi= · line 77
Quoted 6 lines
drivers/clk/Makefile +3
-0obj-y +=3D imgtec/ obj-$(CONFIG_ARCH_MXC) +=3D imx/ obj-$(CONFIG_MACH_INGENIC) +=3D ingenic/ +ifeq ($(CONFIG_COMMON_CLK), y) +obj-y +=3Dintel/ +endif
Why not obj-$(CONFIG_INTEL_CCF) or something like that?
Quoted 9 lines
drivers/clk/Makefile +0
-0obj-$(CONFIG_ARCH_KEYSTONE) +=3D keystone/ obj-$(CONFIG_MACH_LOONGSON32) +=3D loongson1/ obj-y +=3D mediatek/drivers/clk/intel/Kconfig new file +0
-0
drivers/clk/intel/Kconfig line 8
Quoted 8 lines
drivers/clk/intel/Kconfig new file +8
-0@@ -0,0 +1,20 @@ +# SPDX-License-Identifier: GPL-2.0 +config INTEL_CGU_CLK + depends on COMMON_CLK + depends on INTEL_MIPS || COMPILE_TEST + select MFD_SYSCON + bool "Intel clock controller support" + help + This driver support Intel CGU (Clock Generation Unit).
Is it really called a clock generation unit? Or that's just copied from sunxi driver?
drivers/clk/intel/clk-cgu-=
Quoted 26 lines
drivers/clk/intel/Kconfig new file +12
-0+ +choice + prompt "SoC platform selection" + depends on INTEL_CGU_CLK + default INTEL_GRX500_CGU_CLK + +config INTEL_GRX500_CGU_CLK + bool "GRX500 CLK" + help + Clock driver of GRX500 platform. + +endchoicedrivers/clk/intel/Makefile new file +7
-0@@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0 +# Makefile for intel specific clk + +obj-$(CONFIG_INTEL_CGU_CLK) +=3D clk-cgu.o clk-cgu-pll.o +ifneq ($(CONFIG_INTEL_GRX500_CGU_CLK),) + obj-y +=3D clk-grx500.o +endifdrivers/clk/intel/clk-cgu-= renamed from drivers/clk/intel/clk-cgu-pll.c +0
-0
pll.c
drivers/clk/intel/clk-cgu-pll.c line 7
Quoted 12 lines
new file mode 100644
index 000000000000..20759bc27e95drivers/clk/intel/clk-cgu-pll.c new file +7
-0@@ -0,0 +1,166 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (C) 2018 Intel Corporation. +* Zhu YiXin <[email protected]> +*/ + +#include <linux/clk.h>
Is this include used?
drivers/clk/intel/clk-cgu-pll.c
Quoted 12 lines
drivers/clk/intel/clk-cgu-pll.c new file +12
-0+#include <linux/clk-provider.h> +#include <linux/clkdev.h> +#include <linux/mfd/syscon.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/regmap.h> +#include <linux/slab.h> + +#include "clk-cgu-pll.h" +#include "clk-cgu.h" + +#define to_intel_clk_pll (_hw) container_of(_hw, struct intel_clk_pll, h=
w)
Quoted 56 lines
drivers/clk/intel/clk-cgu-pll.c new file +56
-0+ +/* +* Calculate formula: +* rate =3D (prate * mult + (prate * frac) / frac_div) / div +*/ +static unsigned long +intel_pll_calc_rate (unsigned long prate,unsigned int mult, +unsigned int div,unsigned int frac, +unsigned int frac_div) +{ +u64 crate, frate, rate64; + + rate64 =3D prate; + crate =3D rate64 * mult; + +if (frac) { + frate =3D rate64 * frac; +do_div (frate, frac_div); + crate +=3D frate; + } +do_div (crate, div); + +return (unsigned long )crate; +} + +static void +grx500_pll_get_params (struct intel_clk_pll *pll,unsigned int *mult, +unsigned int *frac) +{ + *mult =3D intel_get_clk_val(pll->map, pll->reg, 2, 7); + *frac =3D intel_get_clk_val(pll->map, pll->reg, 9, 21); +} + +static int intel_wait_pll_lock (struct intel_clk_pll *pll,int bit_idx) +{ +unsigned int val; + +return regmap_read_poll_timeout (pll->map, pll->reg, val, + val &BIT (bit_idx),10 ,1000 ); +} + +static unsigned long +intel_grx500_pll_recalc_rate (struct clk_hw *hw,unsigned long prate) +{ + struct intel_clk_pll *pll =3D to_intel_clk_pll(hw); +unsigned int mult, frac; + +grx500_pll_get_params (pll, &mult, &frac); + +return intel_pll_calc_rate (prate, mult,1 , frac,BIT (20 )); +} + +static int intel_grx500_pll_is_enabled (struct clk_hw *hw) +{ + struct intel_clk_pll *pll =3D to_intel_clk_pll(hw); +
drivers/clk/intel/clk-cgu-pll.c
Quoted 10 lines
drivers/clk/intel/clk-cgu-pll.c new file +10
-0+if (intel_wait_pll_lock (pll,1 )) { +pr_err ("%s: pll: %s is not locked!\n" , + __func__,clk_hw_get_name (hw)); +return 0 ; + } + +return intel_get_clk_val (pll->map, pll->reg,1 ,1 ); +} + +const static struct clk_ops intel_grx500_pll_ops =3D {
Should be static const struct ...
drivers/clk/intel/clk-cgu-pll.c
Quoted 11 lines
drivers/clk/intel/clk-cgu-pll.c new file +11
-0+ .recalc_rate =3D intel_grx500_pll_recalc_rate, + .is_enabled =3D intel_grx500_pll_is_enabled, +}; + +static struct clk +*intel_clk_register_pll(struct intel_clk_provider *ctx, + enum intel_pll_type type, const char *cname, + const char *const *pname, u8 num_parents, + unsigned long flags, unsigned int reg, + const struct intel_pll_rate_table *table, + unsigned int mult, unsigned int div, unsigned int=
frac)
Quoted 4 lines
drivers/clk/intel/clk-cgu-pll.c new file +4
-0+{ +struct clk_init_data init; +struct intel_clk_pll *pll; +struct clk_hw *hw;
drivers/clk/intel/clk-cgu-pll.c
Quoted 10 lines
drivers/clk/intel/clk-cgu-pll.c new file +10
-0+int ret, i; + + if (type !=3D pll_grx500) { +pr_err ("%s: pll type %d not supported!\n" , + __func__, type); +return ERR_PTR (-EINVAL ); + } + init.name =3D cname; + init.ops =3D &intel_grx500_pll_ops; + init.flags =3D CLK_IS_BASIC;
Don't use this flag unless you have some reason to need it.
Quoted 28 lines
drivers/clk/intel/clk-cgu-pll.c new file +28
-0+ init.parent_names =3D pname; + init.num_parents =3D num_parents; + + pll =3D kzalloc(sizeof(*pll), GFP_KERNEL); +if (!pll) +return ERR_PTR (-ENOMEM ); + pll->map =3D ctx->map; + pll->reg =3D reg; + pll->flags =3D flags; + pll->mult =3D mult; + pll->div =3D div; + pll->frac =3D frac; + pll->hw.init =3D &init; +if (table) { + for (i =3D 0; table[i].rate !=3D 0; i++) + ; + pll->table_sz =3D i; + pll->rate_table =3D kmemdup(table, i * sizeof(table[0]), +GFP_KERNEL ); +if (!pll->rate_table) { + ret =3D -ENOMEM ; + goto err_free_pll; + } + } + hw =3D &pll->hw; + ret =3D clk_hw_register(NULL, hw); +if (ret) + goto err_free_pll;
drivers/clk/intel/clk-cgu-pll.c
Quoted 10 lines
drivers/clk/intel/clk-cgu-pll.c new file +10
-0+ +return hw->clk; + +err_free_pll: +kfree (pll); +return ERR_PTR (ret); +} + +void intel_clk_register_plls(struct intel_clk_provider *ctx, + struct intel_pll_clk *list, unsigned int nr_=
clk)
drivers/clk/intel/clk-cgu-pll.c
Quoted 6 lines
drivers/clk/intel/clk-cgu-pll.c new file +6
-0+{ + struct clk *clk; + int i; + + for (i =3D 0; i < nr_clk; i++, list++) { + clk =3D intel_clk_register_pll(ctx, list->type, list->nam=
e,
drivers/clk/intel/clk-cgu-=
Quoted 13 lines
drivers/clk/intel/clk-cgu-pll.c new file +12
-0+ list->parent_names, list->num_parents, + list->flags, list->reg, list->rate_table, + list->mult, list->div, list->frac); +if (IS_ERR (clk)) { +pr_err ("%s: failed to register pll: %s\n" , + __func__, list->name); +continue ; + } + +intel_clk_add_lookup (ctx, clk, list->id); + } +}drivers/clk/intel/clk-cgu-= renamed from drivers/clk/intel/clk-cgu-pll.h +0
-0
pll.h
Quoted 61 lines
new file mode 100644
index 000000000000..3e7cff1d5e16drivers/clk/intel/clk-cgu-pll.h new file +34
-0@@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* Copyright(c) 2018 Intel Corporation. +* Zhu YiXin <[email protected]> +*/ + +#ifndef __INTEL_CLK_PLL_H +#define __INTEL_CLK_PLL_H + +enum intel_pll_type { + pll_grx500, +}; + +struct intel_pll_rate_table { +unsigned long prate; +unsigned long rate; +unsigned int mult; +unsigned int div; +unsigned int frac; +}; + +struct intel_clk_pll { +struct clk_hw hw; +struct regmap *map; +unsigned int reg; +unsigned long flags; +unsigned int mult; +unsigned int div; +unsigned int frac; +unsigned int table_sz; +const struct intel_pll_rate_table *rate_table; +}; + +#endif /* __INTEL_CLK_PLL_H */ drivers/clk/intel/clk-cgu.c new file +16
-0@@ -0,0 +1,470 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (C) 2018 Intel Corporation. +* Zhu YiXin <[email protected]> +*/ + +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/clkdev.h> +#include <linux/mfd/syscon.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/regmap.h> +#include <linux/slab.h> + +#include "clk-cgu-pll.h"
drivers/clk/intel/clk-cgu.c line 26
Quoted 10 lines
drivers/clk/intel/clk-cgu.c new file +10
-0+#include "clk-cgu.h" + +#define GATE_HW_REG_STAT (reg) (reg) +#define GATE_HW_REG_EN (reg) ((reg) + 0x4) +#define GATE_HW_REG_DIS (reg) ((reg) + 0x8) + +#define to_intel_clk_mux (_hw) container_of(_hw, struct intel_clk_mux, hw) +#define to_intel_clk_divider (_hw) \ + container_of(_hw, struct intel_clk_divider, hw) +#define to_intel_clk_gate (_hw) container_of(_hw, struct intel_clk_gate, =
hw)
Quoted 28 lines
drivers/clk/intel/clk-cgu.c new file +28
-0+ +void intel_set_clk_val (struct regmap *map,u32 reg,u8 shift, +u8 width,u32 set_val) +{ + u32 mask =3D GENMASK(width + shift, shift); + +regmap_update_bits (map, reg, mask, set_val << shift); +} + +u32 intel_get_clk_val (struct regmap *map,u32 reg,u8 shift, +u8 width) +{ +u32 val; + +if (regmap_read (map, reg, &val)) { +WARN_ONCE (1 ,"Failed to read clk reg: 0x%x\n" , reg); +return 0 ; + } + val >>=3D shift; + val &=3D BIT(width) - 1; + +return val; +} + +void intel_clk_add_lookup (struct intel_clk_provider *ctx, +struct clk *clk,unsigned int id) +{ +pr_debug ("Add clk: %s, id: %u\n" ,__clk_get_name (clk), id);
drivers/clk/intel/clk-cgu.c
Quoted 10 lines
drivers/clk/intel/clk-cgu.c new file +10
-0+if (ctx->clk_data.clks && id) + ctx->clk_data.clks[id] =3D clk; +} + +static struct clk +*intel_clk_register_fixed(struct intel_clk_provider *ctx, + struct intel_clk_branch *list) +{ + if (list->div_flags & CLOCK_FLAG_VAL_INIT) + intel_set_clk_val(ctx->map, list->div_off, list->div_shif=
t,
drivers/clk/intel/clk-cgu.c
Quoted 3 lines
drivers/clk/intel/clk-cgu.c new file +3
-0+ list->div_width, list->div_val); + + return clk_register_fixed_rate(NULL, list->name, list->parent_nam=
es[0],
drivers/clk/intel/clk-cgu.c
Quoted 9 lines
drivers/clk/intel/clk-cgu.c new file +9
-0+ list->flags, list->mux_flags); +} + +static u8 intel_clk_mux_get_parent(struct clk_hw *hw) +{ + struct intel_clk_mux *mux =3D to_intel_clk_mux(hw); + u32 val; + + val =3D intel_get_clk_val(mux->map, mux->reg, mux->shift, mux->wi=
dth);
drivers/clk/intel/clk-cgu.c
Quoted 10 lines
drivers/clk/intel/clk-cgu.c new file +10
-0+return clk_mux_val_to_index (hw,NULL , mux->flags, val); +} + +static int intel_clk_mux_set_parent(struct clk_hw *hw, u8 index) +{ + struct intel_clk_mux *mux =3D to_intel_clk_mux(hw); + u32 val; + + val =3D clk_mux_index_to_val(NULL, mux->flags, index); + intel_set_clk_val(mux->map, mux->reg, mux->shift, mux->width, val=
);
Quoted 47 lines
drivers/clk/intel/clk-cgu.c new file +47
-0+ +return 0 ; +} + +static int intel_clk_mux_determine_rate (struct clk_hw *hw, +struct clk_rate_request *req) +{ + struct intel_clk_mux *mux =3D to_intel_clk_mux(hw); + +return clk_mux_determine_rate_flags (hw, req, mux->flags); +} + +const static struct clk_ops intel_clk_mux_ops =3D { + .get_parent =3D intel_clk_mux_get_parent, + .set_parent =3D intel_clk_mux_set_parent, + .determine_rate =3D intel_clk_mux_determine_rate, +}; + +static struct clk +*intel_clk_register_mux (struct intel_clk_provider *ctx, +struct intel_clk_branch *list) +{ +struct clk_init_data init; +struct clk_hw *hw; +struct intel_clk_mux *mux; + u32 reg =3D list->mux_off; + u8 shift =3D list->mux_shift; + u8 width =3D list->mux_width; + unsigned long cflags =3D list->mux_flags; +int ret; + + mux =3D kzalloc(sizeof(*mux), GFP_KERNEL); +if (!mux) +return ERR_PTR (-ENOMEM ); + + init.name =3D list->name; + init.ops =3D &intel_clk_mux_ops; + init.flags =3D list->flags | CLK_IS_BASIC; + init.parent_names =3D list->parent_names; + init.num_parents =3D list->num_parents; + + mux->map =3D ctx->map; + mux->reg =3D reg; + mux->shift =3D shift; + mux->width =3D width; + mux->flags =3D cflags; + mux->hw.init =3D &init;
drivers/clk/intel/clk-cgu.c
Quoted 10 lines
drivers/clk/intel/clk-cgu.c new file +10
-0+ + hw =3D &mux->hw; + ret =3D clk_hw_register(NULL, hw); +if (ret) { +kfree (mux); +return ERR_PTR (ret); + } + +if (cflags &CLOCK_FLAG_VAL_INIT ) + intel_set_clk_val(ctx->map, reg, shift, width, list->mux_=
val);
Quoted 83 lines
drivers/clk/intel/clk-cgu.c new file +83
-0+ +return hw->clk; +} + +static unsigned long +intel_clk_divider_recalc_rate (struct clk_hw *hw, +unsigned long parent_rate) +{ + struct intel_clk_divider *divider =3D to_intel_clk_divider(hw); +unsigned int val; + + val =3D intel_get_clk_val(divider->map, divider->reg, + divider->shift, divider->width); +return divider_recalc_rate (hw, parent_rate, val, divider->table, + divider->flags, divider->width); +} + +static long +intel_clk_divider_round_rate (struct clk_hw *hw,unsigned long rate, +unsigned long *prate) +{ + struct intel_clk_divider *divider =3D to_intel_clk_divider(hw); + +return divider_round_rate (hw, rate, prate, divider->table, + divider->width, divider->flags); +} + +static int +intel_clk_divider_set_rate (struct clk_hw *hw,unsigned long rate, +unsigned long prate) +{ + struct intel_clk_divider *divider =3D to_intel_clk_divider(hw); +int value; + + value =3D divider_get_val(rate, prate, divider->table, + divider->width, divider->flags); +if (value <0 ) +return value; + +intel_set_clk_val (divider->map, divider->reg, + divider->shift, divider->width, value); + +return 0 ; +} + +const static struct clk_ops intel_clk_divider_ops =3D { + .recalc_rate =3D intel_clk_divider_recalc_rate, + .round_rate =3D intel_clk_divider_round_rate, + .set_rate =3D intel_clk_divider_set_rate, +}; + +static struct clk +*intel_clk_register_divider (struct intel_clk_provider *ctx, +struct intel_clk_branch *list) +{ +struct clk_init_data init; +struct clk_hw *hw; +struct intel_clk_divider *div; + u32 reg =3D list->div_off; + u8 shift =3D list->div_shift; + u8 width =3D list->div_width; + unsigned long cflags =3D list->div_flags; +int ret; + + div =3D kzalloc(sizeof(*div), GFP_KERNEL); +if (!div) +return ERR_PTR (-ENOMEM ); + + init.name =3D list->name; + init.ops =3D &intel_clk_divider_ops; + init.flags =3D list->flags | CLK_IS_BASIC; + init.parent_names =3D &list->parent_names[0 ]; + init.num_parents =3D 1; + + div->map =3D ctx->map; + div->reg =3D reg; + div->shift =3D shift; + div->width =3D width; + div->flags =3D cflags; + div->table =3D list->div_table; + div->hw.init =3D &init; + + hw =3D &div->hw;
drivers/clk/intel/clk-cgu.c
Quoted 10 lines
drivers/clk/intel/clk-cgu.c new file +10
-0+ ret =3D clk_hw_register(NULL, hw); +if (ret) { +pr_err ("%s: register clk: %s failed!\n" , + __func__, list->name); +kfree (div); +return ERR_PTR (ret); + } + +if (cflags &CLOCK_FLAG_VAL_INIT ) + intel_set_clk_val(ctx->map, reg, shift, width, list->div_=
val);
drivers/clk/intel/clk-cgu.c
Quoted 12 lines
drivers/clk/intel/clk-cgu.c new file +12
-0+ +return hw->clk; +} + +static struct clk +*intel_clk_register_fixed_factor(struct intel_clk_provider *ctx, + struct intel_clk_branch *list) +{ + struct clk_hw *hw; + + hw =3D clk_hw_register_fixed_factor(NULL, list->name, + list->parent_names[0], list->fl=
ags,
drivers/clk/intel/clk-cgu.c
Quoted 6 lines
drivers/clk/intel/clk-cgu.c new file +6
-0+ list->mult, list->div); +if (IS_ERR (hw)) +return ERR_CAST (hw); + +if (list->div_flags &CLOCK_FLAG_VAL_INIT ) + intel_set_clk_val(ctx->map, list->div_off, list->div_shif=
t,
Quoted 81 lines
drivers/clk/intel/clk-cgu.c new file +81
-0+ list->div_width, list->div_val); + +return hw->clk; +} + +static int +intel_clk_gate_enable (struct clk_hw *hw) +{ + struct intel_clk_gate *gate =3D to_intel_clk_gate(hw); +unsigned int reg; + +if (gate->flags &GATE_CLK_VT ) { + gate->reg =3D 1; +return 0 ; + } + +if (gate->flags &GATE_CLK_HW ) { + reg =3D GATE_HW_REG_EN(gate->reg); + }else if (gate->flags &GATE_CLK_SW ) { + reg =3D gate->reg; + }else { +pr_err ("%s: gate clk: %s: flag 0x%lx not supported!\n" , + __func__,clk_hw_get_name (hw), gate->flags); +return 0 ; + } + +intel_set_clk_val (gate->map, reg, gate->shift,1 ,1 ); + +return 0 ; +} + +static void +intel_clk_gate_disable (struct clk_hw *hw) +{ + struct intel_clk_gate *gate =3D to_intel_clk_gate(hw); +unsigned int reg; +unsigned int set; + +if (gate->flags &GATE_CLK_VT ) { + gate->reg =3D 0; +return ; + } + +if (gate->flags &GATE_CLK_HW ) { + reg =3D GATE_HW_REG_DIS(gate->reg); + set =3D 1; + }else if (gate->flags &GATE_CLK_SW ) { + reg =3D gate->reg; + set =3D 0; + }else { +pr_err ("%s: gate clk: %s: flag 0x%lx not supported!\n" , + __func__,clk_hw_get_name (hw), gate->flags); +return ; + } + +intel_set_clk_val (gate->map, reg, gate->shift,1 , set); +} + +static int +intel_clk_gate_is_enabled (struct clk_hw *hw) +{ + struct intel_clk_gate *gate =3D to_intel_clk_gate(hw); +unsigned int reg; + +if (gate->flags &GATE_CLK_VT ) +return gate->reg; + +if (gate->flags &GATE_CLK_HW ) { + reg =3D GATE_HW_REG_STAT(gate->reg); + }else if (gate->flags &GATE_CLK_SW ) { + reg =3D gate->reg; + }else { +pr_err ("%s: gate clk: %s: flag 0x%lx not supported!\n" , + __func__,clk_hw_get_name (hw), gate->flags); +return 0 ; + } + +return intel_get_clk_val (gate->map, reg, gate->shift,1 ); +} + +const static struct clk_ops intel_clk_gate_ops =3D {
drivers/clk/intel/clk-cgu.c line 345
Quoted 10 lines
drivers/clk/intel/clk-cgu.c new file +10
-0+ .enable =3D intel_clk_gate_enable, + .disable =3D intel_clk_gate_disable, + .is_enabled =3D intel_clk_gate_is_enabled, +}; + +static struct clk +*intel_clk_register_gate (struct intel_clk_provider *ctx, +struct intel_clk_branch *list) +{ +struct clk_init_data init;
Please init the init struct with { } so that future possible additions to the structure don't require us to hunt this silent corruption down later.
Quoted 23 lines
drivers/clk/intel/clk-cgu.c new file +23
-0+struct clk_hw *hw; +struct intel_clk_gate *gate; + u32 reg =3D list->gate_off; + u8 shift =3D list->gate_shift; + unsigned long cflags =3D list->gate_flags; + const char *pname =3D list->parent_names[0]; +int ret; + + gate =3D kzalloc(sizeof(*gate), GFP_KERNEL); +if (!gate) +return ERR_PTR (-ENOMEM ); + + init.name =3D list->name; + init.ops =3D &intel_clk_gate_ops; + init.flags =3D list->flags | CLK_IS_BASIC; + init.parent_names =3D pname ? &pname : NULL; + init.num_parents =3D pname ? 1 : 0; + + gate->map =3D ctx->map; + gate->reg =3D reg; + gate->shift =3D shift; + gate->flags =3D cflags; + gate->hw.init =3D &init;
drivers/clk/intel/clk-cgu.c line 378
Quoted 10 lines
drivers/clk/intel/clk-cgu.c new file +10
-0+ + hw =3D &gate->hw; + ret =3D clk_hw_register(NULL, hw); +if (ret) { +kfree (gate); +return ERR_PTR (ret); + } + +if (cflags &CLOCK_FLAG_VAL_INIT ) + intel_set_clk_val(ctx->map, reg, shift, 1, list->gate_val=
);
Quoted 4 lines
drivers/clk/intel/clk-cgu.c new file +4
-0+ +return hw->clk; +} +
drivers/clk/intel/clk-cgu.c line 392
Quoted 10 lines
drivers/clk/intel/clk-cgu.c new file +10
-0+void intel_clk_register_branches (struct intel_clk_provider *ctx, +struct intel_clk_branch *list, +unsigned int nr_clk) +{ +struct clk *clk; +unsigned int idx; + + for (idx =3D 0; idx < nr_clk; idx++, list++) { +switch (list->type) { +case intel_clk_fixed:
Please use uppercase for enums.
drivers/clk/intel/clk-cgu.c
Quoted 10 lines
drivers/clk/intel/clk-cgu.c new file +10
-0+ clk =3D intel_clk_register_fixed(ctx, list); +break ; +case intel_clk_mux: + clk =3D intel_clk_register_mux(ctx, list); +break ; +case intel_clk_divider: + clk =3D intel_clk_register_divider(ctx, list); +break ; +case intel_clk_fixed_factor: + clk =3D intel_clk_register_fixed_factor(ctx, list=
);
Quoted 12 lines
drivers/clk/intel/clk-cgu.c new file +12
-0+break ; +case intel_clk_gate: + clk =3D intel_clk_register_gate(ctx, list); +break ; +default : +pr_err ("%s: type: %u not supported!\n" , + __func__, list->type); +return ; + } + +if (IS_ERR (clk)) { + pr_err("%s: register clk: %s, type: %u failed!\n",
drivers/clk/intel/clk-cgu.c
Quoted 10 lines
drivers/clk/intel/clk-cgu.c new file +10
-0+ __func__, list->name, list->type); +return ; + } + +intel_clk_add_lookup (ctx, clk, list->id); + } +} + +struct intel_clk_provider * __init +intel_clk_init(struct device_node *np, struct regmap *map, unsigned int =
nr_clks)
Quoted 27 lines
drivers/clk/intel/clk-cgu.c new file +27
-0+{ +struct intel_clk_provider *ctx; +struct clk **clks; + + ctx =3D kzalloc(sizeof(*ctx), GFP_KERNEL); +if (!ctx) +return ERR_PTR (-ENOMEM ); + + clks =3D kcalloc(nr_clks, sizeof(*clks), GFP_KERNEL); +if (!clks) { +kfree (ctx); +return ERR_PTR (-ENOMEM ); + } + +memset_p ((void **)clks,ERR_PTR (-ENOENT ), nr_clks); + ctx->map =3D map; + ctx->clk_data.clks =3D clks; + ctx->clk_data.clk_num =3D nr_clks; + ctx->np =3D np; + +return ctx; +} + +void __init intel_clk_register_osc (struct intel_clk_provider *ctx, +struct intel_osc_clk *osc, +unsigned int nr_clks) +{
drivers/clk/intel/clk-cgu.c
Quoted 10 lines
drivers/clk/intel/clk-cgu.c new file +10
-0+u32 freq; +struct clk *clk; +int idx; + + for (idx =3D 0; idx < nr_clks; idx++, osc++) { +if (!osc->dt_freq || +of_property_read_u32 (ctx->np, osc->dt_freq, &freq)) + freq =3D osc->def_rate; + + clk =3D clk_register_fixed_rate(NULL, osc->name, NULL, 0,=
freq);
Should come from DT itself.
Quoted 61 lines
drivers/clk/intel/clk-cgu.c new file +9
-0+if (IS_ERR (clk)) { +pr_err ("%s: Failed to register clock: %s\n" , + __func__, osc->name); +return ; + } + +intel_clk_add_lookup (ctx, clk, osc->id); + } +}drivers/clk/intel/clk-cgu.h new file +46
-0@@ -0,0 +1,259 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* Copyright(c) 2018 Intel Corporation. +* Zhu YiXin <[email protected]> +*/ + +#ifndef __INTEL_CLK_H +#define __INTEL_CLK_H + +#define PNAME (x) static const char *const x[] __initconst + +struct intel_clk_mux { +struct clk_hw hw; +struct regmap *map; +unsigned int reg; +u8 shift; +u8 width; +unsigned long flags; +}; + +struct intel_clk_divider { +struct clk_hw hw; +struct regmap *map; +unsigned int reg; +u8 shift; +u8 width; +unsigned long flags; +const struct clk_div_table *table; +}; + +struct intel_clk_gate { +struct clk_hw hw; +struct regmap *map; +unsigned int reg; +u8 shift; +unsigned long flags; +}; + +enum intel_clk_type { + intel_clk_fixed, + intel_clk_mux, + intel_clk_divider, + intel_clk_fixed_factor, + intel_clk_gate, +}; +
drivers/clk/intel/clk-cgu.h line 56
Quoted 10 lines
drivers/clk/intel/clk-cgu.h new file +10
-0+/** +* struct intel_clk_provider +* @map: regmap type base address for register. +* @np: device node +* @clk_data: array of hw clocks and clk number. +*/ +struct intel_clk_provider { +struct regmap *map; +struct device_node *np; +struct clk_onecell_data clk_data;
Please register clk_hw pointers instead of clk pointers with the of provider APIs.
Quoted 5 lines
drivers/clk/intel/clk-cgu.h new file +5
-0+}; + +/** +* struct intel_pll_clk +* @id: plaform specific id of the clock.
drivers/clk/intel/clk-cgu.h line 71
Quoted 10 lines
drivers/clk/intel/clk-cgu.h new file +10
-0+* @name: name of this pll clock. +* @parent_names: name of the parent clock. +* @num_parents: number of parents. +* @flags: optional flags for basic clock. +* @type: platform type of pll. +* @reg: offset of the register. +* @mult: init value of mulitplier. +* @div: init value of divider. +* @frac: init value of fraction. +* @rate_table: table of pll clock rate.
Please drop the full-stop on kernel doc one-liners like this.
drivers/clk/intel/clk-cgu.h line 77
Quoted 6 lines
drivers/clk/intel/clk-cgu.h new file +6
-0+*/ +struct intel_pll_clk { +unsigned int id; +const char *name; +const char *const *parent_names; +u8 num_parents;
Can the PLL have multiple parents?
Quoted 22 lines
drivers/clk/intel/clk-cgu.h new file +22
-0+unsigned long flags; +enum intel_pll_type type; +int reg; +unsigned int mult; +unsigned int div; +unsigned int frac; +const struct intel_pll_rate_table *rate_table; +}; + +#define INTEL_PLL (_id, _type, _name, _pnames, _flags, \ + _reg, _rtable, _mult, _div, _frac) \ + { \ + .id =3D _id, \ + .type =3D _type, \ + .name =3D _name, \ + .parent_names =3D _pnames, \ + .num_parents =3D ARRAY_SIZE(_pnames), \ + .flags =3D _flags, \ + .reg =3D _reg, \ + .rate_table =3D _rtable, \ + .mult =3D _mult, \ + .div =3D _div, \
drivers/clk/intel/clk-cgu.h line 109
Quoted 10 lines
drivers/clk/intel/clk-cgu.h new file +10
-0+ .frac =3D _frac \ + } + +/** +* struct intel_osc_clk +* @id: platform specific id of the clock. +* @name: name of the osc clock. +* @dt_freq: frequency node name in device tree. +* @def_rate: default rate of the osc clock. +* @flags: optional flags for basic clock.
There aren't flags though. I'm very confused by this kernel-doc too. Looks like something that should be done with a fixed rate clk in DT.
Quoted 7 lines
drivers/clk/intel/clk-cgu.h new file +7
-0+*/ +struct intel_osc_clk { +unsigned int id; +const char *name; +const char *dt_freq; +const u32 def_rate; +};
drivers/clk/intel/clk-cgu.h line 126
Quoted 10 lines
drivers/clk/intel/clk-cgu.h new file +10
-0+ +#define INTEL_OSC (_id, _name, _freq, _rate) \ + { \ + .id =3D _id, \ + .name =3D _name, \ + .dt_freq =3D _freq, \ + .def_rate =3D _rate, \ + } + +struct intel_clk_branch {
Seems to be more like intel_clk instead of intel_clk_branch because it does lots of stuff.
Quoted 20 lines
drivers/clk/intel/clk-cgu.h new file +20
-0+unsigned int id; +enum intel_clk_type type; +const char *name; +const char *const *parent_names; +u8 num_parents; +unsigned long flags; +unsigned int mux_off; +u8 mux_shift; +u8 mux_width; +unsigned long mux_flags; +unsigned int mux_val; +unsigned int div_off; +u8 div_shift; +u8 div_width; +unsigned long div_flags; +unsigned int div_val; +const struct clk_div_table *div_table; +unsigned int gate_off; +u8 gate_shift; +unsigned long gate_flags;
drivers/clk/intel/clk-cgu.h line 156
Quoted 10 lines
drivers/clk/intel/clk-cgu.h new file +10
-0+unsigned int gate_val; +unsigned int mult; +unsigned int div; +}; + +/* clock flags definition */ +#define CLOCK_FLAG_VAL_INIT BIT(16) +#define GATE_CLK_HW BIT(17) +#define GATE_CLK_SW BIT(18) +#define GATE_CLK_VT BIT(19)
What does VT mean? Virtual?
Quoted 87 lines
drivers/clk/intel/clk-cgu.h new file +87
-0+ +#define INTEL_MUX (_id, _name, _pname, _f, _reg, \ + _shift, _width, _cf, _v) \ + { \ + .id =3D _id, \ + .type =3D intel_clk_mux, \ + .name =3D _name, \ + .parent_names =3D _pname, \ + .num_parents =3D ARRAY_SIZE(_pname), \ + .flags =3D _f, \ + .mux_off =3D _reg, \ + .mux_shift =3D _shift, \ + .mux_width =3D _width, \ + .mux_flags =3D _cf, \ + .mux_val =3D _v, \ + } + +#define INTEL_DIV (_id, _name, _pname, _f, _reg, \ + _shift, _width, _cf, _v, _dtable) \ + { \ + .id =3D _id, \ + .type =3D intel_clk_divider, \ + .name =3D _name, \ + .parent_names =3D (const char *[]) { _pname }, \ + .num_parents =3D 1, \ + .flags =3D _f, \ + .div_off =3D _reg, \ + .div_shift =3D _shift, \ + .div_width =3D _width, \ + .div_flags =3D _cf, \ + .div_val =3D _v, \ + .div_table =3D _dtable, \ + } + +#define INTEL_GATE (_id, _name, _pname, _f, _reg, \ + _shift, _cf, _v) \ + { \ + .id =3D _id, \ + .type =3D intel_clk_gate, \ + .name =3D _name, \ + .parent_names =3D (const char *[]) { _pname }, \ + .num_parents =3D !_pname ? 0 : 1, \ + .flags =3D _f, \ + .gate_off =3D _reg, \ + .gate_shift =3D _shift, \ + .gate_flags =3D _cf, \ + .gate_val =3D _v, \ + } + +#define INTEL_FIXED (_id, _name, _pname, _f, _reg, \ + _shift, _width, _cf, _freq, _v) \ + { \ + .id =3D _id, \ + .type =3D intel_clk_fixed, \ + .name =3D _name, \ + .parent_names =3D (const char *[]) { _pname }, \ + .num_parents =3D !_pname ? 0 : 1, \ + .flags =3D _f, \ + .div_off =3D _reg, \ + .div_shift =3D _shift, \ + .div_width =3D _width, \ + .div_flags =3D _cf, \ + .div_val =3D _v, \ + .mux_flags =3D _freq, \ + } + +#define INTEL_FIXED_FACTOR (_id, _name, _pname, _f, _reg, \ + _shift, _width, _cf, _v, _m, _d) \ + { \ + .id =3D _id, \ + .type =3D intel_clk_fixed_factor, \ + .name =3D _name, \ + .parent_names =3D (const char *[]) { _pname }, \ + .num_parents =3D 1, \ + .flags =3D _f, \ + .div_off =3D _reg, \ + .div_shift =3D _shift, \ + .div_width =3D _width, \ + .div_flags =3D _cf, \ + .div_val =3D _v, \ + .mult =3D _m, \ + .div =3D _d, \ + } + +void intel_set_clk_val (struct regmap *map,u32 reg,u8 shift, +u8 width,u32 set_val); +u32 intel_get_clk_val (struct regmap *map,u32 reg,u8 shift,u8 width);
drivers/clk/intel/clk-cgu.h line 253
Quoted 10 lines
drivers/clk/intel/clk-cgu.h new file +10
-0+void intel_clk_add_lookup (struct intel_clk_provider *ctx, +struct clk *clk,unsigned int id); +void __init intel_clk_of_add_provider (struct device_node *np, +struct intel_clk_provider *ctx); +struct intel_clk_provider *__init +intel_clk_init (struct device_node *np,struct regmap *map, +unsigned int nr_clks); +void __init intel_clk_register_osc (struct intel_clk_provider *ctx, +struct intel_osc_clk *osc, +unsigned int nr_clks);
Remove __init from headers files. It does nothing.
drivers/clk/intel/clk-cgu.h
Quoted 5 lines
drivers/clk/intel/clk-cgu.h new file +5
-0+void intel_clk_register_branches (struct intel_clk_provider *ctx, +struct intel_clk_branch *list, +unsigned int nr_clk); +void intel_clk_register_plls(struct intel_clk_provider *ctx, + struct intel_pll_clk *list, unsigned int nr_=
clk);
drivers/clk/intel/clk-grx50=
Quoted 2 lines
drivers/clk/intel/clk-cgu.h new file +1
-0+#endif /* __INTEL_CLK_H */ drivers/clk/intel/clk-grx50= renamed from drivers/clk/intel/clk-grx500.c +0
-0
0.c
Quoted 7 lines
new file mode 100644
index 000000000000..5c2546f82579drivers/clk/intel/clk-grx500.c new file +2
-0@@ -0,0 +1,168 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
drivers/clk/intel/clk-grx500.c line 12
Quoted 10 lines
drivers/clk/intel/clk-grx500.c new file +10
-0+* Copyright (C) 2018 Intel Corporation. +* Zhu YiXin <[email protected]> +*/ + +#include <linux/clk-provider.h> +#include <linux/mfd/syscon.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/regmap.h> +#include <linux/spinlock.h>
Used?
Quoted 37 lines
drivers/clk/intel/clk-grx500.c new file +37
-0+#include <dt-bindings/clock/intel,grx500-clk.h> + +#include "clk-cgu-pll.h" +#include "clk-cgu.h" + +#define PLL_DIV_WIDTH 4 + +/* Gate1 clock shift */ +#define G_VCODEC_SHIFT 2 +#define G_DMA0_SHIFT 5 +#define G_USB0_SHIFT 6 +#define G_SPI1_SHIFT 7 +#define G_SPI0_SHIFT 8 +#define G_CBM_SHIFT 9 +#define G_EBU_SHIFT 10 +#define G_SSO_SHIFT 11 +#define G_GPTC0_SHIFT 12 +#define G_GPTC1_SHIFT 13 +#define G_GPTC2_SHIFT 14 +#define G_UART_SHIFT 17 +#define G_CPYTO_SHIFT 20 +#define G_SECPT_SHIFT 21 +#define G_TOE_SHIFT 22 +#define G_MPE_SHIFT 23 +#define G_TDM_SHIFT 25 +#define G_PAE_SHIFT 26 +#define G_USB1_SHIFT 27 +#define G_SWITCH_SHIFT 28 + +/* Gate2 clock shift */ +#define G_PCIE0_SHIFT 1 +#define G_PCIE1_SHIFT 17 +#define G_PCIE2_SHIFT 25 + +/* Register definition */ +#define GRX500_PLL0A_CFG0 0x0004 +#define GRX500_PLL0A_CFG1 0x0008
drivers/clk/intel/clk-grx500.c line 59
Quoted 10 lines
drivers/clk/intel/clk-grx500.c new file +10
-0+#define GRX500_PLL0B_CFG0 0x0034 +#define GRX500_PLL0B_CFG1 0x0038 +#define GRX500_LCPLL_CFG0 0x0094 +#define GRX500_LCPLL_CFG1 0x0098 +#define GRX500_IF_CLK 0x00c4 +#define GRX500_CLK_GSR1 0x0120 +#define GRX500_CLK_GSR2 0x0130 + +static const struct clk_div_table pll_div[] =3D { + {1 ,2 },
Please write it like
{ 1, 2 },instead.
Quoted 9 lines
drivers/clk/intel/clk-grx500.c new file +9
-0+ {2, 3}, + {3, 4}, + {4, 5}, + {5, 6}, + {6, 8}, + {7, 10}, + {8, 12}, + {9, 16}, + {10, 20},
drivers/clk/intel/clk-grx500.c line 78
Quoted 10 lines
drivers/clk/intel/clk-grx500.c new file +10
-0+ {11, 24}, + {12, 32}, + {13, 40}, + {14, 48}, + {15, 64} +}; + +enum grx500_plls { + pll0a, pll0b, pll3, +};
What's the point of the enum?
Quoted 77 lines
drivers/clk/intel/clk-grx500.c new file +77
-0+ +PNAME(pll_p) =3D { "osc" }; +PNAME(cpu_p) =3D { "cpu0", "cpu1" }; + +static struct intel_osc_clk grx500_osc_clks[] __initdata =3D { +INTEL_OSC (CLK_OSC ,"osc" ,"intel,osc-frequency" ,40000000 ), +}; + +static struct intel_pll_clk grx500_pll_clks[] __initdata =3D { + [pll0a] =3D INTEL_PLL(CLK_PLL0A, pll_grx500, "pll0a", + pll_p, 0, GRX500_PLL0A_CFG0, NULL, 0, 0, 0), + [pll0b] =3D INTEL_PLL(CLK_PLL0B, pll_grx500, "pll0b", + pll_p, 0, GRX500_PLL0B_CFG0, NULL, 0, 0, 0), + [pll3] =3D INTEL_PLL(CLK_PLL3, pll_grx500, "pll3", + pll_p, 0, GRX500_LCPLL_CFG0, NULL, 0, 0, 0), +}; + +static struct intel_clk_branch grx500_branch_clks[] __initdata =3D { +INTEL_DIV (CLK_CBM ,"cbm" ,"pll0a" ,0 ,GRX500_PLL0A_CFG1 , +0 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_NGI ,"ngi" ,"pll0a" ,0 ,GRX500_PLL0A_CFG1 , +4 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_SSX4 ,"ssx4" ,"pll0a" ,0 ,GRX500_PLL0A_CFG1 , +8 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_CPU0 ,"cpu0" ,"pll0a" ,0 ,GRX500_PLL0A_CFG1 , +12 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_PAE ,"pae" ,"pll0b" ,0 ,GRX500_PLL0B_CFG1 , +0 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_GSWIP ,"gswip" ,"pll0b" ,0 ,GRX500_PLL0B_CFG1 , +4 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_DDR ,"ddr" ,"pll0b" ,0 ,GRX500_PLL0B_CFG1 , +8 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_CPU1 ,"cpu1" ,"pll0b" ,0 ,GRX500_PLL0B_CFG1 , +12 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_MUX (CLK_CPU ,"cpu" , cpu_p,CLK_SET_RATE_PARENT , +GRX500_PLL0A_CFG1 ,29 ,1 ,0 ,0 ), +INTEL_GATE (GCLK_DMA0 ,"g_dma0" ,NULL ,0 ,GRX500_CLK_GSR1 , +G_DMA0_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_USB0 ,"g_usb0" ,NULL ,0 ,GRX500_CLK_GSR1 , +G_USB0_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_GPTC0 ,"g_gptc0" ,NULL ,0 ,GRX500_CLK_GSR1 , +G_GPTC0_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_GPTC1 ,"g_gptc1" ,NULL ,0 ,GRX500_CLK_GSR1 , +G_GPTC1_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_GPTC2 ,"g_gptc2" ,NULL ,0 ,GRX500_CLK_GSR1 , +G_GPTC2_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_UART ,"g_uart" ,NULL ,0 ,GRX500_CLK_GSR1 , +G_UART_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_PCIE0 ,"g_pcie0" ,NULL ,0 ,GRX500_CLK_GSR2 , +G_PCIE0_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_PCIE1 ,"g_pcie1" ,NULL ,0 ,GRX500_CLK_GSR2 , +G_PCIE1_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_PCIE2 ,"g_pcie2" ,NULL ,0 ,GRX500_CLK_GSR2 , +G_PCIE2_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_I2C ,"g_i2c" ,NULL ,0 ,0 ,0 ,GATE_CLK_VT ,0 ), +INTEL_FIXED (CLK_VOICE ,"voice" ,NULL ,0 ,GRX500_IF_CLK ,14 ,2 , +CLOCK_FLAG_VAL_INIT ,8192000 ,2 ), +INTEL_FIXED_FACTOR (CLK_DDRPHY ,"ddrphy" ,"ddr" ,0 ,0 ,0 , +0 ,0 ,0 ,2 ,1 ), +INTEL_FIXED_FACTOR (CLK_PCIE ,"pcie" ,"pll3" ,0 ,0 ,0 , +0 ,0 ,0 ,1 ,40 ), +}; + +static void __init grx500_clk_init (struct device_node *np) +{ +struct intel_clk_provider *ctx; +struct regmap *map; + + map =3D syscon_node_to_regmap(np); +if (IS_ERR (map)) +return ; + + ctx =3D intel_clk_init(np, map, CLK_NR_CLKS); +if (IS_ERR (ctx)) { +regmap_exit (map); +return ; + }
drivers/clk/intel/clk-grx500.c line 165
Quoted 10 lines
drivers/clk/intel/clk-grx500.c new file +10
-0+ +intel_clk_register_osc (ctx, grx500_osc_clks, +ARRAY_SIZE (grx500_osc_clks)); +intel_clk_register_plls (ctx, grx500_pll_clks, +ARRAY_SIZE (grx500_pll_clks)); +intel_clk_register_branches (ctx, grx500_branch_clks, +ARRAY_SIZE (grx500_branch_clks)); +of_clk_add_provider (np, of_clk_src_onecell_get, &ctx->clk_data); + +pr_debug ("%s clk init done!\n" , __func__);
Yay!!!
drivers/clk/intel/clk-grx500.c line 168
Quoted 3 lines
drivers/clk/intel/clk-grx500.c new file +3
-0+} + +CLK_OF_DECLARE (intel_grx500_cgu,"intel,grx500-cgu" , grx500_clk_init);
Any reason a platform driver can't be used instead of CLK_OF_DECLARE()?
Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
Quoted 25 lines · Stephen Boyd
Quoting Songjun Wu (2018-08-02 20:02:21)
Quoted 16 lines
From: Yixin Zhu <[email protected]>
This driver provides PLL clock registration as well as various clock branches, e.g. MUX clock, gate clock, divider clock and so on.
PLLs that provide clock to DDR, CPU and peripherals are shown below:
+---------+ |--->| LCPLL3 0|--PCIe clk--> XO | +---------+ +-----------| | +---------+ | | 3|--PAE clk--> |--->| PLL0B 2|--GSWIP clk--> | | 1|--DDR clk-->DDR PHY clk--> | | 0|--CPU1 clk--+ +-----+Quoted 8 lines
| +---------+ |--->0 | | | MUX |--CPU clk--> | +---------+ |--->1 | | | 0|--CPU0 clk--+ +-----+ |--->| PLLOA 1|--SSX4 clk--> | 2|--NGI clk--> | 3|--CBM clk--> +---------+
Quoted 1 line
Thanks for the picture!
Thanks for the review.
Quoted 15 lines
Quoted 8 lines
Signed-off-by Yixin Zhu <[email protected]>Signed-off-by Songjun Wu <[email protected]>drivers/clk/Makefile +0
-0drivers/clk/Makefile obj-$(CONFIG_ARCH_HISI) += hisilicon/ · line 77
Quoted 6 lines
drivers/clk/Makefile +3
-0@@ -72,6 +72,9 @@ obj-$(CONFIG_ARCH_HISI) += hisilicon/ obj-y += imgtec/ obj-$(CONFIG_ARCH_MXC) += imx/ obj-$(CONFIG_MACH_INGENIC) += ingenic/ +ifeq ($(CONFIG_COMMON_CLK), y) +obj-y +=intel/ +endifWhy not obj-$(CONFIG_INTEL_CCF) or something like that?
Will use obj-$(CONFIG_INTEL_CCF)
Quoted 11 lines
Quoted 11 lines
drivers/clk/Makefile +0
-0obj-$(CONFIG_ARCH_KEYSTONE) += keystone/ obj-$(CONFIG_MACH_LOONGSON32) += loongson1/ obj-y += mediatek/drivers/clk/intel/Kconfig new file +2
-0@@ -0,0 +1,20 @@ +# SPDX-License-Identifier: GPL-2.0 +config INTEL_CGU_CLK
Quoted 8 lines
drivers/clk/intel/Kconfig line 8
Quoted 6 lines
drivers/clk/intel/Kconfig new file +6
-0+ depends on COMMON_CLK + depends on INTEL_MIPS || COMPILE_TEST + select MFD_SYSCON + bool "Intel clock controller support" + help + This driver support Intel CGU (Clock Generation Unit).Is it really called a clock generation unit? Or that's just copied from sunxi driver?
Yes, It's called clock generation unit(CGU) in our HW chip spec.
Quoted 31 lines
Quoted 31 lines
drivers/clk/intel/Kconfig new file +12
-0+ +choice + prompt "SoC platform selection" + depends on INTEL_CGU_CLK + default INTEL_GRX500_CGU_CLK + +config INTEL_GRX500_CGU_CLK + bool "GRX500 CLK" + help + Clock driver of GRX500 platform. + +endchoicedrivers/clk/intel/Makefile new file +7
-0@@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0 +# Makefile for intel specific clk + +obj-$(CONFIG_INTEL_CGU_CLK) += clk-cgu.o clk-cgu-pll.o +ifneq ($(CONFIG_INTEL_GRX500_CGU_CLK),) + obj-y += clk-grx500.o +endifdrivers/clk/intel/clk-cgu-pll.c new file +0
-0@@ -0,0 +1,166 @@
Quoted 8 lines
drivers/clk/intel/clk-cgu-pll.c line 7
Quoted 7 lines
drivers/clk/intel/clk-cgu-pll.c new file +7
-0+// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (C) 2018 Intel Corporation. +* Zhu YiXin <[email protected]> +*/ + +#include <linux/clk.h> Is this include used?
Not used. Will remove it.
Quoted 71 lines
Quoted 71 lines
drivers/clk/intel/clk-cgu-pll.c new file +71
-0+#include <linux/clk-provider.h> +#include <linux/clkdev.h> +#include <linux/mfd/syscon.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/regmap.h> +#include <linux/slab.h> + +#include "clk-cgu-pll.h" +#include "clk-cgu.h" + +#define to_intel_clk_pll (_hw) container_of(_hw, struct intel_clk_pll, hw) + +/* +* Calculate formula: +* rate = (prate * mult + (prate * frac) / frac_div) / div +*/ +static unsigned long +intel_pll_calc_rate (unsigned long prate,unsigned int mult, +unsigned int div,unsigned int frac, +unsigned int frac_div) +{ +u64 crate, frate, rate64; + + rate64 = prate; + crate = rate64 * mult; + +if (frac) { + frate = rate64 * frac; +do_div (frate, frac_div); + crate += frate; + } +do_div (crate, div); + +return (unsigned long )crate; +} + +static void +grx500_pll_get_params (struct intel_clk_pll *pll,unsigned int *mult, +unsigned int *frac) +{ + *mult =intel_get_clk_val (pll->map, pll->reg,2 ,7 ); + *frac =intel_get_clk_val (pll->map, pll->reg,9 ,21 ); +} + +static int intel_wait_pll_lock (struct intel_clk_pll *pll,int bit_idx) +{ +unsigned int val; + +return regmap_read_poll_timeout (pll->map, pll->reg, val, + val &BIT (bit_idx),10 ,1000 ); +} + +static unsigned long +intel_grx500_pll_recalc_rate (struct clk_hw *hw,unsigned long prate) +{ +struct intel_clk_pll *pll =to_intel_clk_pll (hw); +unsigned int mult, frac; + +grx500_pll_get_params (pll, &mult, &frac); + +return intel_pll_calc_rate (prate, mult,1 , frac,BIT (20 )); +} + +static int intel_grx500_pll_is_enabled (struct clk_hw *hw) +{ +struct intel_clk_pll *pll =to_intel_clk_pll (hw); + +if (intel_wait_pll_lock (pll,1 )) { +pr_err ("%s: pll: %s is not locked!\n" , + __func__,clk_hw_get_name (hw));
Quoted 8 lines
drivers/clk/intel/clk-cgu-pll.c line 85
Quoted 7 lines
drivers/clk/intel/clk-cgu-pll.c new file +7
-0+return 0 ; + } + +return intel_get_clk_val (pll->map, pll->reg,1 ,1 ); +} + +const static struct clk_ops intel_grx500_pll_ops = {Should be static const struct ...
Will update it.
Quoted 18 lines
Quoted 18 lines
drivers/clk/intel/clk-cgu-pll.c new file +18
-0+ .recalc_rate = intel_grx500_pll_recalc_rate, + .is_enabled = intel_grx500_pll_is_enabled, +}; + +static struct clk +*intel_clk_register_pll (struct intel_clk_provider *ctx, +enum intel_pll_type type,const char *cname, +const char *const *pname,u8 num_parents, +unsigned long flags,unsigned int reg, +const struct intel_pll_rate_table *table, +unsigned int mult,unsigned int div,unsigned int frac) +{ +struct clk_init_data init; +struct intel_clk_pll *pll; +struct clk_hw *hw; +int ret, i; + +if (type != pll_grx500) {
Quoted 8 lines
drivers/clk/intel/clk-cgu-pll.c line 110
Quoted 7 lines
drivers/clk/intel/clk-cgu-pll.c new file +7
-0+pr_err ("%s: pll type %d not supported!\n" , + __func__, type); +return ERR_PTR (-EINVAL ); + } + init.name = cname; + init.ops = &intel_grx500_pll_ops; + init.flags =CLK_IS_BASIC ;Don't use this flag unless you have some reason to need it.
Will remove it.
Quoted 442 lines
Quoted 442 lines
drivers/clk/intel/clk-cgu-pll.c new file +56
-0+ init.parent_names = pname; + init.num_parents = num_parents; + + pll =kzalloc (sizeof (*pll),GFP_KERNEL ); +if (!pll) +return ERR_PTR (-ENOMEM ); + pll->map = ctx->map; + pll->reg = reg; + pll->flags = flags; + pll->mult = mult; + pll->div = div; + pll->frac = frac; + pll->hw.init = &init; +if (table) { +for (i =0 ; table[i].rate !=0 ; i++) + ; + pll->table_sz = i; + pll->rate_table =kmemdup (table, i *sizeof (table[0 ]), +GFP_KERNEL ); +if (!pll->rate_table) { + ret = -ENOMEM ; + goto err_free_pll; + } + } + hw = &pll->hw; + ret =clk_hw_register (NULL , hw); +if (ret) + goto err_free_pll; + +return hw->clk; + +err_free_pll: +kfree (pll); +return ERR_PTR (ret); +} + +void intel_clk_register_plls (struct intel_clk_provider *ctx, +struct intel_pll_clk *list,unsigned int nr_clk) +{ +struct clk *clk; +int i; + +for (i =0 ; i < nr_clk; i++, list++) { + clk =intel_clk_register_pll (ctx, list->type, list->name, + list->parent_names, list->num_parents, + list->flags, list->reg, list->rate_table, + list->mult, list->div, list->frac); +if (IS_ERR (clk)) { +pr_err ("%s: failed to register pll: %s\n" , + __func__, list->name); +continue ; + } + +intel_clk_add_lookup (ctx, clk, list->id); + } +}drivers/clk/intel/clk-cgu-pll.h new file +34
-0@@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* Copyright(c) 2018 Intel Corporation. +* Zhu YiXin <[email protected]> +*/ + +#ifndef __INTEL_CLK_PLL_H +#define __INTEL_CLK_PLL_H + +enum intel_pll_type { + pll_grx500, +}; + +struct intel_pll_rate_table { +unsigned long prate; +unsigned long rate; +unsigned int mult; +unsigned int div; +unsigned int frac; +}; + +struct intel_clk_pll { +struct clk_hw hw; +struct regmap *map; +unsigned int reg; +unsigned long flags; +unsigned int mult; +unsigned int div; +unsigned int frac; +unsigned int table_sz; +const struct intel_pll_rate_table *rate_table; +}; + +#endif /* __INTEL_CLK_PLL_H */ drivers/clk/intel/clk-cgu.c new file +340
-0@@ -0,0 +1,470 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (C) 2018 Intel Corporation. +* Zhu YiXin <[email protected]> +*/ + +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/clkdev.h> +#include <linux/mfd/syscon.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/regmap.h> +#include <linux/slab.h> + +#include "clk-cgu-pll.h" +#include "clk-cgu.h" + +#define GATE_HW_REG_STAT (reg) (reg) +#define GATE_HW_REG_EN (reg) ((reg) + 0x4) +#define GATE_HW_REG_DIS (reg) ((reg) + 0x8) + +#define to_intel_clk_mux (_hw) container_of(_hw, struct intel_clk_mux, hw) +#define to_intel_clk_divider (_hw) \ + container_of(_hw, struct intel_clk_divider, hw) +#define to_intel_clk_gate (_hw) container_of(_hw, struct intel_clk_gate, hw) + +void intel_set_clk_val (struct regmap *map,u32 reg,u8 shift, +u8 width,u32 set_val) +{ +u32 mask =GENMASK (width + shift, shift); + +regmap_update_bits (map, reg, mask, set_val << shift); +} + +u32 intel_get_clk_val (struct regmap *map,u32 reg,u8 shift, +u8 width) +{ +u32 val; + +if (regmap_read (map, reg, &val)) { +WARN_ONCE (1 ,"Failed to read clk reg: 0x%x\n" , reg); +return 0 ; + } + val >>= shift; + val &=BIT (width) -1 ; + +return val; +} + +void intel_clk_add_lookup (struct intel_clk_provider *ctx, +struct clk *clk,unsigned int id) +{ +pr_debug ("Add clk: %s, id: %u\n" ,__clk_get_name (clk), id); +if (ctx->clk_data.clks && id) + ctx->clk_data.clks[id] = clk; +} + +static struct clk +*intel_clk_register_fixed (struct intel_clk_provider *ctx, +struct intel_clk_branch *list) +{ +if (list->div_flags &CLOCK_FLAG_VAL_INIT ) +intel_set_clk_val (ctx->map, list->div_off, list->div_shift, + list->div_width, list->div_val); + +return clk_register_fixed_rate (NULL , list->name, list->parent_names[0 ], + list->flags, list->mux_flags); +} + +static u8 intel_clk_mux_get_parent (struct clk_hw *hw) +{ +struct intel_clk_mux *mux =to_intel_clk_mux (hw); +u32 val; + + val =intel_get_clk_val (mux->map, mux->reg, mux->shift, mux->width); +return clk_mux_val_to_index (hw,NULL , mux->flags, val); +} + +static int intel_clk_mux_set_parent (struct clk_hw *hw,u8 index) +{ +struct intel_clk_mux *mux =to_intel_clk_mux (hw); +u32 val; + + val =clk_mux_index_to_val (NULL , mux->flags, index); +intel_set_clk_val (mux->map, mux->reg, mux->shift, mux->width, val); + +return 0 ; +} + +static int intel_clk_mux_determine_rate (struct clk_hw *hw, +struct clk_rate_request *req) +{ +struct intel_clk_mux *mux =to_intel_clk_mux (hw); + +return clk_mux_determine_rate_flags (hw, req, mux->flags); +} + +const static struct clk_ops intel_clk_mux_ops = { + .get_parent = intel_clk_mux_get_parent, + .set_parent = intel_clk_mux_set_parent, + .determine_rate = intel_clk_mux_determine_rate, +}; + +static struct clk +*intel_clk_register_mux (struct intel_clk_provider *ctx, +struct intel_clk_branch *list) +{ +struct clk_init_data init; +struct clk_hw *hw; +struct intel_clk_mux *mux; +u32 reg = list->mux_off; +u8 shift = list->mux_shift; +u8 width = list->mux_width; +unsigned long cflags = list->mux_flags; +int ret; + + mux =kzalloc (sizeof (*mux),GFP_KERNEL ); +if (!mux) +return ERR_PTR (-ENOMEM ); + + init.name = list->name; + init.ops = &intel_clk_mux_ops; + init.flags = list->flags |CLK_IS_BASIC ; + init.parent_names = list->parent_names; + init.num_parents = list->num_parents; + + mux->map = ctx->map; + mux->reg = reg; + mux->shift = shift; + mux->width = width; + mux->flags = cflags; + mux->hw.init = &init; + + hw = &mux->hw; + ret =clk_hw_register (NULL , hw); +if (ret) { +kfree (mux); +return ERR_PTR (ret); + } + +if (cflags &CLOCK_FLAG_VAL_INIT ) +intel_set_clk_val (ctx->map, reg, shift, width, list->mux_val); + +return hw->clk; +} + +static unsigned long +intel_clk_divider_recalc_rate (struct clk_hw *hw, +unsigned long parent_rate) +{ +struct intel_clk_divider *divider =to_intel_clk_divider (hw); +unsigned int val; + + val =intel_get_clk_val (divider->map, divider->reg, + divider->shift, divider->width); +return divider_recalc_rate (hw, parent_rate, val, divider->table, + divider->flags, divider->width); +} + +static long +intel_clk_divider_round_rate (struct clk_hw *hw,unsigned long rate, +unsigned long *prate) +{ +struct intel_clk_divider *divider =to_intel_clk_divider (hw); + +return divider_round_rate (hw, rate, prate, divider->table, + divider->width, divider->flags); +} + +static int +intel_clk_divider_set_rate (struct clk_hw *hw,unsigned long rate, +unsigned long prate) +{ +struct intel_clk_divider *divider =to_intel_clk_divider (hw); +int value; + + value =divider_get_val (rate, prate, divider->table, + divider->width, divider->flags); +if (value <0 ) +return value; + +intel_set_clk_val (divider->map, divider->reg, + divider->shift, divider->width, value); + +return 0 ; +} + +const static struct clk_ops intel_clk_divider_ops = { + .recalc_rate = intel_clk_divider_recalc_rate, + .round_rate = intel_clk_divider_round_rate, + .set_rate = intel_clk_divider_set_rate, +}; + +static struct clk +*intel_clk_register_divider (struct intel_clk_provider *ctx, +struct intel_clk_branch *list) +{ +struct clk_init_data init; +struct clk_hw *hw; +struct intel_clk_divider *div; +u32 reg = list->div_off; +u8 shift = list->div_shift; +u8 width = list->div_width; +unsigned long cflags = list->div_flags; +int ret; + + div =kzalloc (sizeof (*div),GFP_KERNEL ); +if (!div) +return ERR_PTR (-ENOMEM ); + + init.name = list->name; + init.ops = &intel_clk_divider_ops; + init.flags = list->flags |CLK_IS_BASIC ; + init.parent_names = &list->parent_names[0 ]; + init.num_parents =1 ; + + div->map = ctx->map; + div->reg = reg; + div->shift = shift; + div->width = width; + div->flags = cflags; + div->table = list->div_table; + div->hw.init = &init; + + hw = &div->hw; + ret =clk_hw_register (NULL , hw); +if (ret) { +pr_err ("%s: register clk: %s failed!\n" , + __func__, list->name); +kfree (div); +return ERR_PTR (ret); + } + +if (cflags &CLOCK_FLAG_VAL_INIT ) +intel_set_clk_val (ctx->map, reg, shift, width, list->div_val); + +return hw->clk; +} + +static struct clk +*intel_clk_register_fixed_factor (struct intel_clk_provider *ctx, +struct intel_clk_branch *list) +{ +struct clk_hw *hw; + + hw =clk_hw_register_fixed_factor (NULL , list->name, + list->parent_names[0 ], list->flags, + list->mult, list->div); +if (IS_ERR (hw)) +return ERR_CAST (hw); + +if (list->div_flags &CLOCK_FLAG_VAL_INIT ) +intel_set_clk_val (ctx->map, list->div_off, list->div_shift, + list->div_width, list->div_val); + +return hw->clk; +} + +static int +intel_clk_gate_enable (struct clk_hw *hw) +{ +struct intel_clk_gate *gate =to_intel_clk_gate (hw); +unsigned int reg; + +if (gate->flags &GATE_CLK_VT ) { + gate->reg =1 ; +return 0 ; + } + +if (gate->flags &GATE_CLK_HW ) { + reg =GATE_HW_REG_EN (gate->reg); + }else if (gate->flags &GATE_CLK_SW ) { + reg = gate->reg; + }else { +pr_err ("%s: gate clk: %s: flag 0x%lx not supported!\n" , + __func__,clk_hw_get_name (hw), gate->flags); +return 0 ; + } + +intel_set_clk_val (gate->map, reg, gate->shift,1 ,1 ); + +return 0 ; +} + +static void +intel_clk_gate_disable (struct clk_hw *hw) +{ +struct intel_clk_gate *gate =to_intel_clk_gate (hw); +unsigned int reg; +unsigned int set; + +if (gate->flags &GATE_CLK_VT ) { + gate->reg =0 ; +return ; + } + +if (gate->flags &GATE_CLK_HW ) { + reg =GATE_HW_REG_DIS (gate->reg); + set =1 ; + }else if (gate->flags &GATE_CLK_SW ) { + reg = gate->reg; + set =0 ; + }else { +pr_err ("%s: gate clk: %s: flag 0x%lx not supported!\n" , + __func__,clk_hw_get_name (hw), gate->flags); +return ; + } + +intel_set_clk_val (gate->map, reg, gate->shift,1 , set); +} + +static int +intel_clk_gate_is_enabled (struct clk_hw *hw) +{ +struct intel_clk_gate *gate =to_intel_clk_gate (hw); +unsigned int reg; + +if (gate->flags &GATE_CLK_VT ) +return gate->reg; + +if (gate->flags &GATE_CLK_HW ) { + reg =GATE_HW_REG_STAT (gate->reg); + }else if (gate->flags &GATE_CLK_SW ) { + reg = gate->reg; + }else { +pr_err ("%s: gate clk: %s: flag 0x%lx not supported!\n" , + __func__,clk_hw_get_name (hw), gate->flags); +return 0 ; + } + +return intel_get_clk_val (gate->map, reg, gate->shift,1 ); +} + +const static struct clk_ops intel_clk_gate_ops = { + .enable = intel_clk_gate_enable, + .disable = intel_clk_gate_disable, + .is_enabled = intel_clk_gate_is_enabled, +}; +
Quoted 8 lines
drivers/clk/intel/clk-cgu.c line 345
Quoted 5 lines
drivers/clk/intel/clk-cgu.c new file +5
-0+static struct clk +*intel_clk_register_gate (struct intel_clk_provider *ctx, +struct intel_clk_branch *list) +{ +struct clk_init_data init;Please init the init struct with { } so that future possible additions to the structure don't require us to hunt this silent corruption down later.
Will update it.
Quoted 40 lines
Quoted 40 lines
drivers/clk/intel/clk-cgu.c new file +40
-0+struct clk_hw *hw; +struct intel_clk_gate *gate; +u32 reg = list->gate_off; +u8 shift = list->gate_shift; +unsigned long cflags = list->gate_flags; +const char *pname = list->parent_names[0 ]; +int ret; + + gate =kzalloc (sizeof (*gate),GFP_KERNEL ); +if (!gate) +return ERR_PTR (-ENOMEM ); + + init.name = list->name; + init.ops = &intel_clk_gate_ops; + init.flags = list->flags |CLK_IS_BASIC ; + init.parent_names = pname ? &pname :NULL ; + init.num_parents = pname ?1 :0 ; + + gate->map = ctx->map; + gate->reg = reg; + gate->shift = shift; + gate->flags = cflags; + gate->hw.init = &init; + + hw = &gate->hw; + ret =clk_hw_register (NULL , hw); +if (ret) { +kfree (gate); +return ERR_PTR (ret); + } + +if (cflags &CLOCK_FLAG_VAL_INIT ) +intel_set_clk_val (ctx->map, reg, shift,1 , list->gate_val); + +return hw->clk; +} + +void intel_clk_register_branches (struct intel_clk_provider *ctx, +struct intel_clk_branch *list, +unsigned int nr_clk)
Quoted 8 lines
drivers/clk/intel/clk-cgu.c line 392
Quoted 7 lines
drivers/clk/intel/clk-cgu.c new file +7
-0+{ +struct clk *clk; +unsigned int idx; + +for (idx =0 ; idx < nr_clk; idx++, list++) { +switch (list->type) { +case intel_clk_fixed:Please use uppercase for enums.
Will update.
Quoted 63 lines
Quoted 63 lines
drivers/clk/intel/clk-cgu.c new file +62
-0+ clk =intel_clk_register_fixed (ctx, list); +break ; +case intel_clk_mux: + clk =intel_clk_register_mux (ctx, list); +break ; +case intel_clk_divider: + clk =intel_clk_register_divider (ctx, list); +break ; +case intel_clk_fixed_factor: + clk =intel_clk_register_fixed_factor (ctx, list); +break ; +case intel_clk_gate: + clk =intel_clk_register_gate (ctx, list); +break ; +default : +pr_err ("%s: type: %u not supported!\n" , + __func__, list->type); +return ; + } + +if (IS_ERR (clk)) { +pr_err ("%s: register clk: %s, type: %u failed!\n" , + __func__, list->name, list->type); +return ; + } + +intel_clk_add_lookup (ctx, clk, list->id); + } +} + +struct intel_clk_provider *__init +intel_clk_init (struct device_node *np,struct regmap *map,unsigned int nr_clks) +{ +struct intel_clk_provider *ctx; +struct clk **clks; + + ctx =kzalloc (sizeof (*ctx),GFP_KERNEL ); +if (!ctx) +return ERR_PTR (-ENOMEM ); + + clks =kcalloc (nr_clks,sizeof (*clks),GFP_KERNEL ); +if (!clks) { +kfree (ctx); +return ERR_PTR (-ENOMEM ); + } + +memset_p ((void **)clks,ERR_PTR (-ENOENT ), nr_clks); + ctx->map = map; + ctx->clk_data.clks = clks; + ctx->clk_data.clk_num = nr_clks; + ctx->np = np; + +return ctx; +} + +void __init intel_clk_register_osc (struct intel_clk_provider *ctx, +struct intel_osc_clk *osc, +unsigned int nr_clks) +{ +u32 freq; +struct clk *clk; +int idx;
Quoted 8 lines
drivers/clk/intel/clk-cgu.c line 461
Quoted 7 lines
drivers/clk/intel/clk-cgu.c new file +7
-0+ +for (idx =0 ; idx < nr_clks; idx++, osc++) { +if (!osc->dt_freq || +of_property_read_u32 (ctx->np, osc->dt_freq, &freq)) + freq = osc->def_rate; + + clk =clk_register_fixed_rate (NULL , osc->name,NULL ,0 , freq);Should come from DT itself.
Yes. It can be defined as fixed-clock node in device tree.
Do you mean it should be defined in device tree and driver reference it via device tree?
Quoted 65 lines
Quoted 65 lines
drivers/clk/intel/clk-cgu.c new file +9
-0+ iS_ERR(clk)) { +pr_err ("%s: Failed to register clock: %s\n" , + __func__, osc->name); +return ; + } + +intel_clk_add_lookup (ctx, clk, osc->id); + } +}drivers/clk/intel/clk-cgu.h new file +50
-0@@ -0,0 +1,259 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* Copyright(c) 2018 Intel Corporation. +* Zhu YiXin <[email protected]> +*/ + +#ifndef __INTEL_CLK_H +#define __INTEL_CLK_H + +#define PNAME (x) static const char *const x[] __initconst + +struct intel_clk_mux { +struct clk_hw hw; +struct regmap *map; +unsigned int reg; +u8 shift; +u8 width; +unsigned long flags; +}; + +struct intel_clk_divider { +struct clk_hw hw; +struct regmap *map; +unsigned int reg; +u8 shift; +u8 width; +unsigned long flags; +const struct clk_div_table *table; +}; + +struct intel_clk_gate { +struct clk_hw hw; +struct regmap *map; +unsigned int reg; +u8 shift; +unsigned long flags; +}; + +enum intel_clk_type { + intel_clk_fixed, + intel_clk_mux, + intel_clk_divider, + intel_clk_fixed_factor, + intel_clk_gate, +}; + +/** +* struct intel_clk_provider +* @map: regmap type base address for register. +* @np: device node
Quoted 8 lines
drivers/clk/intel/clk-cgu.h line 56
Quoted 6 lines
drivers/clk/intel/clk-cgu.h new file +6
-0+* @clk_data: array of hw clocks and clk number. +*/ +struct intel_clk_provider { +struct regmap *map; +struct device_node *np; +struct clk_onecell_data clk_data;Please register
clk_hwpointers instead of clk pointers with the of provider APIs.
Sorry. I'm not sure I understand you correctly.
If only registering clk_hw pointer, not registering of_provider API, then how to reference it in the user drivers ?
Could you please give me more hints ?
Quoted 8 lines
Quoted 8 lines
drivers/clk/intel/clk-cgu.h new file +8
-0+}; + +/** +* struct intel_pll_clk +* @id: plaform specific id of the clock. +* @name: name of this pll clock. +* @parent_names: name of the parent clock. +* @num_parents: number of parents.
Quoted 8 lines
drivers/clk/intel/clk-cgu.h line 71
Quoted 7 lines
drivers/clk/intel/clk-cgu.h new file +7
-0+* @flags: optional flags for basic clock. +* @type: platform type of pll. +* @reg: offset of the register. +* @mult: init value of mulitplier. +* @div: init value of divider. +* @frac: init value of fraction. +* @rate_table: table of pll clock rate. Please drop the full-stop on kernel doc one-liners like this.
Will update it.
Quoted 8 lines
drivers/clk/intel/clk-cgu.h line 77
Quoted 7 lines
drivers/clk/intel/clk-cgu.h new file +6
-0+*/ +struct intel_pll_clk { +unsigned int id; +const char *name; +const char *const *parent_names; +u8 num_parents;Can the PLL have multiple parents?
Yes. But not in this platform.
The define here make it easy to expand to support new platform.
Quoted 26 lines
Quoted 26 lines
drivers/clk/intel/clk-cgu.h new file +26
-0+unsigned long flags; +enum intel_pll_type type; +int reg; +unsigned int mult; +unsigned int div; +unsigned int frac; +const struct intel_pll_rate_table *rate_table; +}; + +#define INTEL_PLL (_id, _type, _name, _pnames, _flags, \ + _reg, _rtable, _mult, _div, _frac) \ + { \ + .id = _id, \ + .type = _type, \ + .name = _name, \ + .parent_names = _pnames, \ + .num_parents = ARRAY_SIZE(_pnames), \ + .flags = _flags, \ + .reg = _reg, \ + .rate_table = _rtable, \ + .mult = _mult, \ + .div = _div, \ + .frac = _frac \ + } + +/**
Quoted 8 lines
drivers/clk/intel/clk-cgu.h line 109
Quoted 6 lines
drivers/clk/intel/clk-cgu.h new file +6
-0+* struct intel_osc_clk +* @id: platform specific id of the clock. +* @name: name of the osc clock. +* @dt_freq: frequency node name in device tree. +* @def_rate: default rate of the osc clock. +* @flags: optional flags for basic clock. There aren't flags though. I'm very confused by this kernel-doc too. Looks like something that should be done with a fixed rate clk in DT.
Will remove the flags comments.
Quoted 11 lines
Quoted 11 lines
drivers/clk/intel/clk-cgu.h new file +11
-0+*/ +struct intel_osc_clk { +unsigned int id; +const char *name; +const char *dt_freq; +const u32 def_rate; +}; + +#define INTEL_OSC (_id, _name, _freq, _rate) \ + { \ + .id = _id, \
Quoted 8 lines
drivers/clk/intel/clk-cgu.h line 126
Quoted 6 lines
drivers/clk/intel/clk-cgu.h new file +6
-0+ .name = _name, \ + .dt_freq = _freq, \ + .def_rate = _rate, \ + } + +struct intel_clk_branch {Seems to be more like
intel_clkinstead ofintel_clk_branchbecause it does lots of stuff.
Will update.
Quoted 23 lines
Quoted 23 lines
drivers/clk/intel/clk-cgu.h new file +23
-0+unsigned int id; +enum intel_clk_type type; +const char *name; +const char *const *parent_names; +u8 num_parents; +unsigned long flags; +unsigned int mux_off; +u8 mux_shift; +u8 mux_width; +unsigned long mux_flags; +unsigned int mux_val; +unsigned int div_off; +u8 div_shift; +u8 div_width; +unsigned long div_flags; +unsigned int div_val; +const struct clk_div_table *div_table; +unsigned int gate_off; +u8 gate_shift; +unsigned long gate_flags; +unsigned int gate_val; +unsigned int mult; +unsigned int div;
Quoted 8 lines
drivers/clk/intel/clk-cgu.h line 156
Quoted 7 lines
drivers/clk/intel/clk-cgu.h new file +7
-0+}; + +/* clock flags definition */ +#define CLOCK_FLAG_VAL_INIT BIT(16) +#define GATE_CLK_HW BIT(17) +#define GATE_CLK_SW BIT(18) +#define GATE_CLK_VT BIT(19)What does VT mean? Virtual?
Yes. VT means virtual here.
Will change to GATE_CLK_VIRT.
Quoted 90 lines
Quoted 90 lines
drivers/clk/intel/clk-cgu.h new file +90
-0+ +#define INTEL_MUX (_id, _name, _pname, _f, _reg, \ + _shift, _width, _cf, _v) \ + { \ + .id = _id, \ + .type = intel_clk_mux, \ + .name = _name, \ + .parent_names = _pname, \ + .num_parents = ARRAY_SIZE(_pname), \ + .flags = _f, \ + .mux_off = _reg, \ + .mux_shift = _shift, \ + .mux_width = _width, \ + .mux_flags = _cf, \ + .mux_val = _v, \ + } + +#define INTEL_DIV (_id, _name, _pname, _f, _reg, \ + _shift, _width, _cf, _v, _dtable) \ + { \ + .id = _id, \ + .type = intel_clk_divider, \ + .name = _name, \ + .parent_names = (const char *[]) { _pname }, \ + .num_parents = 1, \ + .flags = _f, \ + .div_off = _reg, \ + .div_shift = _shift, \ + .div_width = _width, \ + .div_flags = _cf, \ + .div_val = _v, \ + .div_table = _dtable, \ + } + +#define INTEL_GATE (_id, _name, _pname, _f, _reg, \ + _shift, _cf, _v) \ + { \ + .id = _id, \ + .type = intel_clk_gate, \ + .name = _name, \ + .parent_names = (const char *[]) { _pname }, \ + .num_parents = !_pname ? 0 : 1, \ + .flags = _f, \ + .gate_off = _reg, \ + .gate_shift = _shift, \ + .gate_flags = _cf, \ + .gate_val = _v, \ + } + +#define INTEL_FIXED (_id, _name, _pname, _f, _reg, \ + _shift, _width, _cf, _freq, _v) \ + { \ + .id = _id, \ + .type = intel_clk_fixed, \ + .name = _name, \ + .parent_names = (const char *[]) { _pname }, \ + .num_parents = !_pname ? 0 : 1, \ + .flags = _f, \ + .div_off = _reg, \ + .div_shift = _shift, \ + .div_width = _width, \ + .div_flags = _cf, \ + .div_val = _v, \ + .mux_flags = _freq, \ + } + +#define INTEL_FIXED_FACTOR (_id, _name, _pname, _f, _reg, \ + _shift, _width, _cf, _v, _m, _d) \ + { \ + .id = _id, \ + .type = intel_clk_fixed_factor, \ + .name = _name, \ + .parent_names = (const char *[]) { _pname }, \ + .num_parents = 1, \ + .flags = _f, \ + .div_off = _reg, \ + .div_shift = _shift, \ + .div_width = _width, \ + .div_flags = _cf, \ + .div_val = _v, \ + .mult = _m, \ + .div = _d, \ + } + +void intel_set_clk_val (struct regmap *map,u32 reg,u8 shift, +u8 width,u32 set_val); +u32 intel_get_clk_val (struct regmap *map,u32 reg,u8 shift,u8 width); +void intel_clk_add_lookup (struct intel_clk_provider *ctx, +struct clk *clk,unsigned int id); +void __init intel_clk_of_add_provider(struct device_node *np,
Quoted 8 lines
drivers/clk/intel/clk-cgu.h line 253
Quoted 7 lines
drivers/clk/intel/clk-cgu.h new file +7
-0+ struct intel_clk_provider *ctx); +struct intel_clk_provider *__init +intel_clk_init (struct device_node *np,struct regmap *map, +unsigned int nr_clks); +void __init intel_clk_register_osc (struct intel_clk_provider *ctx, +struct intel_osc_clk *osc, +unsigned int nr_clks);Remove
__initfrom headers files. It does nothing.
Will remove it.
Quoted 18 lines
Quoted 18 lines
drivers/clk/intel/clk-cgu.h new file +6
-0+void intel_clk_register_branches (struct intel_clk_provider *ctx, +struct intel_clk_branch *list, +unsigned int nr_clk); +void intel_clk_register_plls (struct intel_clk_provider *ctx, +struct intel_pll_clk *list,unsigned int nr_clk); +#endif /* __INTEL_CLK_H */ drivers/clk/intel/clk-grx500.c new file +5
-0@@ -0,0 +1,168 @@ +// SPDX-License-Identifier: GPL-2.0 +/* +* Copyright (C) 2018 Intel Corporation. +* Zhu YiXin <[email protected]> +*/
Quoted 8 lines
drivers/clk/intel/clk-grx500.c line 12
Quoted 7 lines
drivers/clk/intel/clk-grx500.c new file +7
-0+ +#include <linux/clk-provider.h> +#include <linux/mfd/syscon.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/regmap.h> +#include <linux/spinlock.h> Used?
Will remove it
Quoted 49 lines
Quoted 37 lines
drivers/clk/intel/clk-grx500.c new file +37
-0+#include <dt-bindings/clock/intel,grx500-clk.h> + +#include "clk-cgu-pll.h" +#include "clk-cgu.h" + +#define PLL_DIV_WIDTH 4 + +/* Gate1 clock shift */ +#define G_VCODEC_SHIFT 2 +#define G_DMA0_SHIFT 5 +#define G_USB0_SHIFT 6 +#define G_SPI1_SHIFT 7 +#define G_SPI0_SHIFT 8 +#define G_CBM_SHIFT 9 +#define G_EBU_SHIFT 10 +#define G_SSO_SHIFT 11 +#define G_GPTC0_SHIFT 12 +#define G_GPTC1_SHIFT 13 +#define G_GPTC2_SHIFT 14 +#define G_UART_SHIFT 17 +#define G_CPYTO_SHIFT 20 +#define G_SECPT_SHIFT 21 +#define G_TOE_SHIFT 22 +#define G_MPE_SHIFT 23 +#define G_TDM_SHIFT 25 +#define G_PAE_SHIFT 26 +#define G_USB1_SHIFT 27 +#define G_SWITCH_SHIFT 28 + +/* Gate2 clock shift */ +#define G_PCIE0_SHIFT 1 +#define G_PCIE1_SHIFT 17 +#define G_PCIE2_SHIFT 25 + +/* Register definition */ +#define GRX500_PLL0A_CFG0 0x0004 +#define GRX500_PLL0A_CFG1 0x0008drivers/clk/intel/clk-grx500.c line 59
Quoted 10 lines
drivers/clk/intel/clk-grx500.c new file +10
-0+#define GRX500_PLL0B_CFG0 0x0034 +#define GRX500_PLL0B_CFG1 0x0038 +#define GRX500_LCPLL_CFG0 0x0094 +#define GRX500_LCPLL_CFG1 0x0098 +#define GRX500_IF_CLK 0x00c4 +#define GRX500_CLK_GSR1 0x0120 +#define GRX500_CLK_GSR2 0x0130 + +static const struct clk_div_table pll_div[] = { + {1 ,2 },Please write it like
Quoted 3 lines
{ 1, 2 },instead.
Will update.
Quoted 12 lines
Quoted 12 lines
drivers/clk/intel/clk-grx500.c new file +12
-0+ {2, 3}, + {3, 4}, + {4, 5}, + {5, 6}, + {6, 8}, + {7, 10}, + {8, 12}, + {9, 16}, + {10, 20}, + {11, 24}, + {12, 32}, + {13, 40},
Quoted 8 lines
drivers/clk/intel/clk-grx500.c line 78
Quoted 7 lines
drivers/clk/intel/clk-grx500.c new file +7
-0+ {14, 48}, + {15, 64} +}; + +enum grx500_plls { + pll0a, pll0b, pll3, +};What's the point of the enum?
Will remove it.
Quoted 89 lines
Quoted 77 lines
drivers/clk/intel/clk-grx500.c new file +77
-0+ +PNAME (pll_p) = {"osc" }; +PNAME (cpu_p) = {"cpu0" ,"cpu1" }; + +static struct intel_osc_clk grx500_osc_clks[]__initdata = { +INTEL_OSC (CLK_OSC ,"osc" ,"intel,osc-frequency" ,40000000 ), +}; + +static struct intel_pll_clk grx500_pll_clks[]__initdata = { + [pll0a] =INTEL_PLL (CLK_PLL0A , pll_grx500,"pll0a" , + pll_p,0 ,GRX500_PLL0A_CFG0 ,NULL ,0 ,0 ,0 ), + [pll0b] =INTEL_PLL (CLK_PLL0B , pll_grx500,"pll0b" , + pll_p,0 ,GRX500_PLL0B_CFG0 ,NULL ,0 ,0 ,0 ), + [pll3] =INTEL_PLL (CLK_PLL3 , pll_grx500,"pll3" , + pll_p,0 ,GRX500_LCPLL_CFG0 ,NULL ,0 ,0 ,0 ), +}; + +static struct intel_clk_branch grx500_branch_clks[]__initdata = { +INTEL_DIV (CLK_CBM ,"cbm" ,"pll0a" ,0 ,GRX500_PLL0A_CFG1 , +0 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_NGI ,"ngi" ,"pll0a" ,0 ,GRX500_PLL0A_CFG1 , +4 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_SSX4 ,"ssx4" ,"pll0a" ,0 ,GRX500_PLL0A_CFG1 , +8 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_CPU0 ,"cpu0" ,"pll0a" ,0 ,GRX500_PLL0A_CFG1 , +12 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_PAE ,"pae" ,"pll0b" ,0 ,GRX500_PLL0B_CFG1 , +0 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_GSWIP ,"gswip" ,"pll0b" ,0 ,GRX500_PLL0B_CFG1 , +4 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_DDR ,"ddr" ,"pll0b" ,0 ,GRX500_PLL0B_CFG1 , +8 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_DIV (CLK_CPU1 ,"cpu1" ,"pll0b" ,0 ,GRX500_PLL0B_CFG1 , +12 ,PLL_DIV_WIDTH ,0 ,0 , pll_div), +INTEL_MUX (CLK_CPU ,"cpu" , cpu_p,CLK_SET_RATE_PARENT , +GRX500_PLL0A_CFG1 ,29 ,1 ,0 ,0 ), +INTEL_GATE (GCLK_DMA0 ,"g_dma0" ,NULL ,0 ,GRX500_CLK_GSR1 , +G_DMA0_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_USB0 ,"g_usb0" ,NULL ,0 ,GRX500_CLK_GSR1 , +G_USB0_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_GPTC0 ,"g_gptc0" ,NULL ,0 ,GRX500_CLK_GSR1 , +G_GPTC0_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_GPTC1 ,"g_gptc1" ,NULL ,0 ,GRX500_CLK_GSR1 , +G_GPTC1_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_GPTC2 ,"g_gptc2" ,NULL ,0 ,GRX500_CLK_GSR1 , +G_GPTC2_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_UART ,"g_uart" ,NULL ,0 ,GRX500_CLK_GSR1 , +G_UART_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_PCIE0 ,"g_pcie0" ,NULL ,0 ,GRX500_CLK_GSR2 , +G_PCIE0_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_PCIE1 ,"g_pcie1" ,NULL ,0 ,GRX500_CLK_GSR2 , +G_PCIE1_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_PCIE2 ,"g_pcie2" ,NULL ,0 ,GRX500_CLK_GSR2 , +G_PCIE2_SHIFT ,GATE_CLK_HW ,0 ), +INTEL_GATE (GCLK_I2C ,"g_i2c" ,NULL ,0 ,0 ,0 ,GATE_CLK_VT ,0 ), +INTEL_FIXED (CLK_VOICE ,"voice" ,NULL ,0 ,GRX500_IF_CLK ,14 ,2 , +CLOCK_FLAG_VAL_INIT ,8192000 ,2 ), +INTEL_FIXED_FACTOR (CLK_DDRPHY ,"ddrphy" ,"ddr" ,0 ,0 ,0 , +0 ,0 ,0 ,2 ,1 ), +INTEL_FIXED_FACTOR (CLK_PCIE ,"pcie" ,"pll3" ,0 ,0 ,0 , +0 ,0 ,0 ,1 ,40 ), +}; + +static void __init grx500_clk_init (struct device_node *np) +{ +struct intel_clk_provider *ctx; +struct regmap *map; + + map =syscon_node_to_regmap (np); +if (IS_ERR (map)) +return ; + + ctx =intel_clk_init (np, map,CLK_NR_CLKS ); +if (IS_ERR (ctx)) { +regmap_exit (map); +return ; + }drivers/clk/intel/clk-grx500.c line 165
Quoted 10 lines
drivers/clk/intel/clk-grx500.c new file +10
-0+ +intel_clk_register_osc (ctx, grx500_osc_clks, +ARRAY_SIZE (grx500_osc_clks)); +intel_clk_register_plls (ctx, grx500_pll_clks, +ARRAY_SIZE (grx500_pll_clks)); +intel_clk_register_branches (ctx, grx500_branch_clks, +ARRAY_SIZE (grx500_branch_clks)); +of_clk_add_provider (np, of_clk_src_onecell_get, &ctx->clk_data); + +pr_debug ("%s clk init done!\n" , __func__);Yay!!!
Quoted 4 lines
drivers/clk/intel/clk-grx500.c line 168
Quoted 3 lines
drivers/clk/intel/clk-grx500.c new file +3
-0+} + +CLK_OF_DECLARE (intel_grx500_cgu,"intel,grx500-cgu" , grx500_clk_init);Any reason a platform driver can't be used instead of
CLK_OF_DECLARE()?
It provides CPU clock which is used in early boot stage.
Stephen Boyd
maintainerRe: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
Quoting yixin zhu (2018-08-08 01:52:20)
Quoted 11 lines · Stephen Boyd
Quoted 10 lines
Quoting Songjun Wu (2018-08-02 20:02:21)
drivers/clk/intel/clk-cgu.c
Quoted 9 lines
drivers/clk/intel/clk-cgu.c new file +9
-0+struct clk *clk; +int idx; + + for (idx =3D 0; idx < nr_clks; idx++, osc++) { +if (!osc->dt_freq || +of_property_read_u32 (ctx->np, osc->dt_freq, &freq)) + freq =3D osc->def_rate; + + clk =3D clk_register_fixed_rate(NULL, osc->name, NULL,=
0, freq);
Quoted 5 lines
Quoted 1 line
Should come from DT itself.
Yes. It can be defined as fixed-clock node in device tree.
Do you mean it should be defined in device tree and driver reference it =via device tree?
Yes the oscillator should be in DT and then the DT node here can call clk_get() or just hardcode the parent name to be what it knows it is. Eventually we'd like to be able to move away from string names for hierarchy descriptions but that's far off. To get there, we would need DT nodes for clock controllers to indicate their clk parents with the clocks and clock-names properties. So for the oscillator, DT would define it and then the driver would eventually have a way to specify that some parent is index 5 or clock name "foo" and then the clk core could figure out the linkage. I haven't written that code yet, but I'll probably do it soon if nobody beats me to it.
Quoted 12 lines
Quoted 4 lines
Quoted 4 lines
+/**
+ * structintel_clk_provider
+ * @map: regmap type base address for register.
+ * @np: device nodeQuoted 8 lines
drivers/clk/intel/clk-cgu.h line 56
Quoted 6 lines
+ * @clk_data: array of hw clocks and clk number.
+ */drivers/clk/intel/clk-cgu.h new file +4
-0+struct intel_clk_provider { +struct regmap *map; +struct device_node *np; +struct clk_onecell_data clk_data;Please register
clk_hwpointers instead of clk pointers with the of provider APIs.
Quoted 2 lines
Sorry.=C2=A0 I'm not sure I understand you correctly.
If only registeringclk_hwpointer,=C2=A0 not registeringof_providerAPI=
, then
Quoted 2 lines
how to reference it in the user drivers ?
Could you please give me more hints ?
Clk provider drivers shouldn't be using clk pointers directly. Usually when that happens something is wrong. So new clk drivers should register clk_hw pointers and pretty much only deal with clk_hw pointers instead of struct clk pointers. You still register an of_provider, but that provider hands out clk_hw pointers so that clk provider drivers aren't tempted to use struct clk pointers.
Quoted 11 lines
=
=
Quoted 7 lines
Quoted 7 lines
+ */
drivers/clk/intel/clk-cgu.h new file +5
-0+struct intel_pll_clk { +unsigned int id; +const char *name; +const char *const *parent_names; +u8 num_parents;
Quoted 4 lines
Quoted 1 line
Can the PLL have multiple parents?
Yes. But not in this platform.
The define here make it easy to expand to support new platform. =
Ok, so it has a mux inside.
Quoted 33 lines
=
Quoted 23 lines
Quoted 23 lines
drivers/clk/intel/clk-cgu.h new file +23
-0+unsigned int id; +enum intel_clk_type type; +const char *name; +const char *const *parent_names; +u8 num_parents; +unsigned long flags; +unsigned int mux_off; +u8 mux_shift; +u8 mux_width; +unsigned long mux_flags; +unsigned int mux_val; +unsigned int div_off; +u8 div_shift; +u8 div_width; +unsigned long div_flags; +unsigned int div_val; +const struct clk_div_table *div_table; +unsigned int gate_off; +u8 gate_shift; +unsigned long gate_flags; +unsigned int gate_val; +unsigned int mult; +unsigned int div;Quoted 8 lines
drivers/clk/intel/clk-cgu.h line 156
Quoted 7 lines
drivers/clk/intel/clk-cgu.h new file +7
-0+}; + +/* clock flags definition */ +#define CLOCK_FLAG_VAL_INIT BIT(16) +#define GATE_CLK_HW BIT(17) +#define GATE_CLK_SW BIT(18) +#define GATE_CLK_VT BIT(19)What does VT mean? Virtual?
Quoted 3 lines
Yes. VT means virtual here.
Will change toGATE_CLK_VIRT.
=
Is it a hardware concept? Or virtualization with hypervisor?
Quoted 7 lines
Quoted 5 lines
drivers/clk/intel/clk-grx500.c line 168
Quoted 4 lines
drivers/clk/intel/clk-grx500.c new file +3
-0+} + +CLK_OF_DECLARE (intel_grx500_cgu,"intel,grx500-cgu" , grx500_clk_init);Any reason a platform driver can't be used instead of
CLK_OF_DECLARE()?It provides CPU clock which is used in early boot stage. =
Ok. What is the CPU clock doing in early boot stage? Some sort of timer frequency? If the driver can be split into two pieces, one to handle the really early stuff that must be in place to get timers up and running and the other to register the rest of the clks that aren't critical from a regular platform driver it would be good. That's preferred model if something is super critical.
Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
Quoted 16 lines · Stephen Boyd
Quoting yixin zhu (2018-08-08 01:52:20)
Quoted 12 lines · Stephen Boyd
Quoted 11 lines
Quoting Songjun Wu (2018-08-02 20:02:21)
drivers/clk/intel/clk-cgu.c line 461
Quoted 9 lines
drivers/clk/intel/clk-cgu.c new file +9
-0+struct clk *clk; +int idx; + +for (idx =0 ; idx < nr_clks; idx++, osc++) { +if (!osc->dt_freq || +of_property_read_u32 (ctx->np, osc->dt_freq, &freq)) + freq = osc->def_rate; + + clk =clk_register_fixed_rate (NULL , osc->name,NULL ,0 , freq);Should come from DT itself.
Quoted 3 lines
Yes. It can be defined as fixed-clock node in device tree. Do you mean it should be defined in device tree and driver reference it via device tree?
Quoted 10 lines
Yes the oscillator should be in DT and then the DT node here can call
clk_get()or just hardcode the parent name to be what it knows it is. Eventually we'd like to be able to move away from string names for hierarchy descriptions but that's far off. To get there, we would need DT nodes for clock controllers to indicate their clk parents with the clocks and clock-names properties. So for the oscillator, DT would define it and then the driver would eventually have a way to specify that some parent is index 5 or clock name "foo" and then the clk core could figure out the linkage. I haven't written that code yet, but I'll probably do it soon if nobody beats me to it.
Thanks. Will update.
Quoted 17 lines
Quoted 13 lines
Quoted 13 lines
drivers/clk/intel/clk-cgu.h line 56
Quoted 11 lines
drivers/clk/intel/clk-cgu.h new file +10
-0+/** +* struct intel_clk_provider +* @map: regmap type base address for register. +* @np: device node +* @clk_data: array of hw clocks and clk number. +*/ +struct intel_clk_provider { +struct regmap *map; +struct device_node *np; +struct clk_onecell_data clk_data;Please register
clk_hwpointers instead of clk pointers with the of provider APIs.Quoted 4 lines
Sorry. I'm not sure I understand you correctly.
If only registeringclk_hwpointer, not registeringof_providerAPI, then how to reference it in the user drivers ?
Could you please give me more hints ?
Quoted 6 lines
Clk provider drivers shouldn't be using clk pointers directly. Usually when that happens something is wrong. So new clk drivers should register
clk_hwpointers and pretty much only deal withclk_hwpointers instead of struct clk pointers. You still register anof_provider, but that provider hands outclk_hwpointers so that clk provider drivers aren't tempted to use struct clk pointers.
Understood. Will update to use clk_hw_onecell_data and change the registration accordingly.
Quoted 44 lines
Quoted 11 lines
Quoted 8 lines
drivers/clk/intel/clk-cgu.h line 77
Quoted 7 lines
+ */
drivers/clk/intel/clk-cgu.h new file +5
-0+struct intel_pll_clk { +unsigned int id; +const char *name; +const char *const *parent_names; +u8 num_parents;Can the PLL have multiple parents?
Yes. But not in this platform.
The define here make it easy to expand to support new platform.Ok, so it has a mux inside.
Quoted 31 lines
Quoted 31 lines
Quoted 20 lines
drivers/clk/intel/clk-cgu.h new file +20
-0+unsigned int id; +enum intel_clk_type type; +const char *name; +const char *const *parent_names; +u8 num_parents; +unsigned long flags; +unsigned int mux_off; +u8 mux_shift; +u8 mux_width; +unsigned long mux_flags; +unsigned int mux_val; +unsigned int div_off; +u8 div_shift; +u8 div_width; +unsigned long div_flags; +unsigned int div_val; +const struct clk_div_table *div_table; +unsigned int gate_off; +u8 gate_shift; +unsigned long gate_flags;drivers/clk/intel/clk-cgu.h line 156
Quoted 10 lines
drivers/clk/intel/clk-cgu.h new file +10
-0+unsigned int gate_val; +unsigned int mult; +unsigned int div; +}; + +/* clock flags definition */ +#define CLOCK_FLAG_VAL_INIT BIT(16) +#define GATE_CLK_HW BIT(17) +#define GATE_CLK_SW BIT(18) +#define GATE_CLK_VT BIT(19)What does VT mean? Virtual?
Quoted 4 lines
Quoted 3 lines
Yes. VT means virtual here.
Will change toGATE_CLK_VIRT.Is it a hardware concept? Or virtualization with hypervisor?
Some peripheral drivers want to use same code cross platforms.
But not all platforms provide HW gate clock. So in this case, clock driver creates
a virtual gate clock to make it work if no HW gate clock in the SoC.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
drivers/clk/intel/clk-grx500.c line 168
Quoted 4 lines
drivers/clk/intel/clk-grx500.c new file +3
-0+} + +CLK_OF_DECLARE (intel_grx500_cgu,"intel,grx500-cgu" , grx500_clk_init);Any reason a platform driver can't be used instead of
CLK_OF_DECLARE()?
Quoted 8 lines
Quoted 2 lines
It provides CPU clock which is used in early boot stage.
Ok. What is the CPU clock doing in early boot stage? Some sort of timer frequency? If the driver can be split into two pieces, one to handle the really early stuff that must be in place to get timers up and running and the other to register the rest of the clks that aren't critical from a regular platform driver it would be good. That's preferred model if something is super critical.
Yes, CPU clock is providing CPU frequency in the early boot stage.
Will put the non-critical clocks in the platform driver.
Quoted 1 line
Stephen Boyd
maintainerRe: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
Quoting Zhu, Yi Xin (2018-08-28 23:56:22)
Quoted 20 lines
=
Quoted 8 lines · Stephen Boyd
Quoting yixin zhu (2018-08-08 01:52:20)
Quoted 7 lines · Stephen Boyd
Quoted 6 lines
drivers/clk/intel/clk-cgu.h line 156
Quoted 5 lines
drivers/clk/intel/clk-cgu.h new file +5
-0+/* clock flags definition */ +#define CLOCK_FLAG_VAL_INIT BIT(16) +#define GATE_CLK_HW BIT(17) +#define GATE_CLK_SW BIT(18) +#define GATE_CLK_VT BIT(19)What does VT mean? Virtual?
Quoted 4 lines
Quoted 3 lines
Yes. VT means virtual here.
Will change toGATE_CLK_VIRT.Is it a hardware concept? Or virtualization with hypervisor?
=
Some peripheral drivers want to use same code cross platforms. =
Quoted 6 lines
But not all platforms provide HW gate clock.=C2=A0 So in this case, clock =
driver creates
=a virtual gate clock to make it work if no HW gate clock in the SoC.
That's not how things are supposed to work. If a clk isn't there in the hardware we don't make them up in software so that the consumer software drivers can keep requesting clks on different platforms. On a different platform, the driver needs to know that the clks aren't there with a different compatible string.
Quoted 8 lines
=
=
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
drivers/clk/intel/clk-cgu-pll.c
Quoted 4 lines
drivers/clk/intel/clk-cgu-pll.c new file +3
-0+} + +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_ini=
t);
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Any reason a platform driver can't be used instead of CLK_OF_DECLARE(=
)?
Quoted 10 lines
Quoted 8 lines
Quoted 2 lines
It provides CPU clock which is used in early boot stage.
Ok. What is the CPU clock doing in early boot stage? Some sort of timer frequency? If the driver can be split into two pieces, one to handle the really early stuff that must be in place to get timers up and running and the other to register the rest of the clks that aren't critical from a regular platform driver it would be good. That's preferred model if something is super critical.
=
Quoted 7 lines
Yes, CPU clock is providing CPU frequency in the early boot stage. =
Will put the non-critical clocks in the platform driver. =
=
Sure the CPU clock is handling frequency, but does that matter for early boot to get going? If timers aren't involved here then it doesn't sound like this needs CLK_OF_DECLARE.
Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
Quoted 19 lines · Stephen Boyd
Quoting Zhu, Yi Xin (2018-08-28 23:56:22)
Quoted 18 lines · Stephen Boyd
Quoted 8 lines
Quoting yixin zhu (2018-08-08 01:52:20)
Quoted 7 lines · Stephen Boyd
Quoted 6 lines
drivers/clk/intel/clk-cgu.h line 156
Quoted 5 lines
drivers/clk/intel/clk-cgu.h new file +5
-0+/* clock flags definition */ +#define CLOCK_FLAG_VAL_INIT BIT(16) +#define GATE_CLK_HW BIT(17) +#define GATE_CLK_SW BIT(18) +#define GATE_CLK_VT BIT(19)What does VT mean? Virtual?
Quoted 4 lines
Quoted 3 lines
Yes. VT means virtual here.
Will change toGATE_CLK_VIRT.Is it a hardware concept? Or virtualization with hypervisor?
Some peripheral drivers want to use same code cross platforms.
But not all platforms provide HW gate clock. So in this case, clock driver creates
Quoted 6 lines
Quoted 1 line
a virtual gate clock to make it work if no HW gate clock in the SoC.
That's not how things are supposed to work. If a clk isn't there in the hardware we don't make them up in software so that the consumer software drivers can keep requesting clks on different platforms. On a different platform, the driver needs to know that the clks aren't there with a different compatible string.
OK. Will remove virtual gate clock.
Quoted 13 lines
Quoted 13 lines
Quoted 13 lines
Quoted 7 lines
Quoted 5 lines
drivers/clk/intel/clk-grx500.c line 168
Quoted 4 lines
drivers/clk/intel/clk-grx500.c new file +3
-0+} + +CLK_OF_DECLARE (intel_grx500_cgu,"intel,grx500-cgu" , grx500_clk_init);Any reason a platform driver can't be used instead of
CLK_OF_DECLARE()?It provides CPU clock which is used in early boot stage.
Ok. What is the CPU clock doing in early boot stage? Some sort of timer frequency? If the driver can be split into two pieces, one to handle the really early stuff that must be in place to get timers up and running and the other to register the rest of the clks that aren't critical from a regular platform driver it would be good. That's preferred model if something is super critical.
Quoted 8 lines
Quoted 5 lines
Yes, CPU clock is providing CPU frequency in the early boot stage.
Will put the non-critical clocks in the platform driver.
Sure the CPU clock is handling frequency, but does that matter for early boot to get going? If timers aren't involved here then it doesn't sound like this needs
CLK_OF_DECLARE.
Yes, timer is involved here.
CPU frequency get by early stage platform code used in clockevent registration.
Quoted 1 line
Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
In reply to Stephen BoydQuoted 4 lines
Quoted 4 lines
Quoted 4 lines
drivers/clk/intel/clk-grx500.c line 168
Quoted 3 lines
drivers/clk/intel/clk-grx500.c new file +3
-0+} + +CLK_OF_DECLARE (intel_grx500_cgu,"intel,grx500-cgu" , grx500_clk_init);Any reason a platform driver can't be used instead of
CLK_OF_DECLARE()?
Quoted 9 lines
Quoted 2 lines
It provides CPU clock which is used in early boot stage.
Ok. What is the CPU clock doing in early boot stage? Some sort of timer frequency? If the driver can be split into two pieces, one to handle the really early stuff that must be in place to get timers up and running and the other to register the rest of the clks that aren't critical from a regular platform driver it would be good. That's preferred model if something is super critical.
Just to make sure my approach is same as you think.
In the driver, there's two clock registrations.
- One through CLK_OF_DECLARE for early stage clocks.
- The other via platform driver for the non-critical clocks.
In the device tree, two clock device nodes are required.
e.g. device tree:
cgu: cgu@16200000 {
compatible = "intel,grx500-clk", "syscon"; reg = <0x16200000 0x200>; #clock-cells = <1>;
};
clk: clk {
compatible = "intel,grx500-cgu";
#clock-cells = <1>;
intel,cgu-syscon = <&cgu>;
};
source code:
CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_init);
static const struct of_device_id of_intel_grx500_cgu_match[] = { { .compatible = "intel,grx500-clk" },
{}
};
static struct platform_driver intel_grx500_clk_driver = { .probe = intel_grx500_clk_probe,
.driver = {
.name = "grx500-cgu",
.of_match_table = of_match_ptr(of_intel_grx500_cgu_match), },
};
static int __init intel_grx500_cgu_init(void)
{
return platform_driver_register(&intel_grx500_clk_driver); }arch_initcall(intel_grx500_cgu_init);
Stephen Boyd
maintainerRe: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
Quoting Zhu, Yi Xin (2018-08-29 03:34:26)
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
drivers/clk/intel/clk-cgu-pll.c
Quoted 3 lines
drivers/clk/intel/clk-cgu-pll.c new file +3
-0+} + +CLK_OF_DECLARE(intel_grx500_cgu, "intel,grx500-cgu", grx500_clk_ini=
t);
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Any reason a platform driver can't be used instead of CLK_OF_DECLARE(=
)?
Quoted 12 lines
Quoted 9 lines
Quoted 2 lines
It provides CPU clock which is used in early boot stage.
Ok. What is the CPU clock doing in early boot stage? Some sort of timer frequency? If the driver can be split into two pieces, one to handle the really early stuff that must be in place to get timers up and running and the other to register the rest of the clks that aren't critical from a regular platform driver it would be good. That's preferred model if something is super critical.
Just to make sure my approach is same as you think. =
Quoted 7 lines
In the driver, there's two clock registrations.
=- One through
CLK_OF_DECLAREfor early stage clocks. =- The other via platform driver for the non-critical clocks.
You can use the same DT node for both parts, no need to split the node into two and use syscon here.
Re: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
Quoted 16 lines · Stephen Boyd
Quoting Zhu, Yi Xin (2018-08-29 03:34:26)
Quoted 15 lines
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
drivers/clk/intel/clk-grx500.c line 168
Quoted 3 lines
drivers/clk/intel/clk-grx500.c new file +3
-0+} + +CLK_OF_DECLARE (intel_grx500_cgu,"intel,grx500-cgu" , grx500_clk_init);Any reason a platform driver can't be used instead of
CLK_OF_DECLARE()?Quoted 9 lines
Quoted 2 lines
It provides CPU clock which is used in early boot stage.
Ok. What is the CPU clock doing in early boot stage? Some sort of timer frequency? If the driver can be split into two pieces, one to handle the really early stuff that must be in place to get timers up and running and the other to register the rest of the clks that aren't critical from a regular platform driver it would be good. That's preferred model if something is super critical.
Just to make sure my approach is same as you think.
Quoted 8 lines
Quoted 5 lines
In the driver, there's two clock registrations.
- One through
CLK_OF_DECLAREfor early stage clocks.- The other via platform driver for the non-critical clocks.
You can use the same DT node for both parts, no need to split the node into two and use syscon here.
Thank you.
Will use CLK_OF_DECLARE_DRIVER to use the same DT node.
Rob Herring
maintainerRe: [PATCH v2 02/18] clk: intel: Add clock driver for Intel MIPS SoCs
In reply to Songjun WuHi, this is an automated email from Rob's (experimental) review bot. I found a couple of common problems with your patch. Please see below.
Quoted 25 lines · Songjun Wu
From: Yixin Zhu <[email protected]>
This driver provides PLL clock registration as well as various clock branches, e.g. MUX clock, gate clock, divider clock and so on.
PLLs that provide clock to DDR, CPU and peripherals are shown below:
+---------+ |--->| LCPLL3 0|--PCIe clk--> XO | +---------+ +-----------| | +---------+ | | 3|--PAE clk--> |--->| PLL0B 2|--GSWIP clk--> | | 1|--DDR clk-->DDR PHY clk--> | | 0|--CPU1 clk--+ +-----+ | +---------+ |--->0 | | | MUX |--CPU clk--> | +---------+ |--->1 | | | 0|--CPU0 clk--+ +-----+ |--->| PLLOA 1|--SSX4 clk--> | 2|--NGI clk--> | 3|--CBM clk--> +---------+
Quoted 2 lines
Signed-off-by Yixin Zhu <[email protected]>Signed-off-by Songjun Wu <[email protected]>
The preferred subject prefix is "dt-bindings: <binding dir>: ...".
Quoted 17 lines
---
Changes in v2:
- Rewrite clock driver, add platform clock description details in clock driver.drivers/clk/Kconfig | 1 + drivers/clk/Makefile | 3 + drivers/clk/intel/Kconfig | 20 ++ drivers/clk/intel/Makefile | 7 + drivers/clk/intel/clk-cgu-pll.c | 166 ++++++++++ drivers/clk/intel/clk-cgu-pll.h | 34 ++ drivers/clk/intel/clk-cgu.c | 470 +++++++++++++++++++++++++++ drivers/clk/intel/clk-cgu.h | 259 +++++++++++++++ drivers/clk/intel/clk-grx500.c | 168 ++++++++++ include/dt-bindings/clock/intel,grx500-clk.h | 69 ++++ 10 files changed, 1197 insertions(+)
Quoted 9 lines
create mode 100644 drivers/clk/intel/Kconfig create mode 100644 drivers/clk/intel/Makefile create mode 100644 drivers/clk/intel/clk-cgu-pll.c create mode 100644 drivers/clk/intel/clk-cgu-pll.h create mode 100644 drivers/clk/intel/clk-cgu.c create mode 100644 drivers/clk/intel/clk-cgu.h create mode 100644 drivers/clk/intel/clk-grx500.c create mode 100644 include/dt-bindings/clock/intel,grx500-clk.h
DT bindings (including binding headers) should be a separate patch. See Documentation/devicetree/bindings/submitting-patches.txt.
PATCH v2 3/18 dt-bindings: clk: Add documentation of grx500 clock controller
In reply to Songjun WuFrom: Yixin Zhu <[email protected]>
This patch adds binding documentation for grx500 clock controller.
---
Changes in v2:
- Rewrite clock driver's dt-binding document according to Rob Herring's comments.
- Simplify device tree docoment, remove some clock description.
.../devicetree/bindings/clock/intel,grx500-clk.txt | 39 ++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/intel,grx500-clk.txt
Documentation/devicetree/bindings/clock/intel,grx500-clk.txt new file +39 -0
@@ -0,0 +1,39 @@ +Device Tree Clock bindings for grx500 PLL controller. + +This binding uses the common clock binding: + Documentation/devicetree/bindings/clock/clock-bindings.txt + +The grx500 clock controller supplies clock to various controllers within the +SoC. + +Required properties for clock node +- compatible: Should be "intel,grx500-cgu". +- reg: physical base address of the controller and length of memory range. +- #clock-cells: should be 1. + +Optional Propteries: +- intel,osc-frequency: frequency of the osc clock. +if missing, driver will use clock rate defined in the driver. + +Example: Clock controller node: + + cgu: cgu@16200000 { + compatible = "intel,grx500-cgu", "syscon"; + reg = <0x16200000 0x200>; + #clock-cells = <1>; + }; + + +Example: UART controller node that consumes the clock generated by clock + controller. + + asc0: serial@16600000 { + compatible = "lantiq,asc"; + reg = <0x16600000 0x100000>; + interrupt-parent = <&gic>; + interrupts = <GIC_SHARED 103 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 105 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 106 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cgu CLK_SSX4>, <&cgu GCLK_UART>; + clock-names = "freq", "asc"; + };
Signature · 1 line
2.11.0
Re: [PATCH v2 03/18] dt-bindings: clk: Add documentation of grx500 clock controller
Quoted 7 lines · Songjun Wu
From: Yixin Zhu <[email protected]>
This patch adds binding documentation for grx500 clock controller.
Signed-off-by YiXin Zhu <[email protected]>Signed-off-by Songjun Wu <[email protected]>
Quoted 8 lines
---
Changes in v2:
- Rewrite clock driver's dt-binding document according to Rob Herring's comments.
- Simplify device tree docoment, remove some clock description..../devicetree/bindings/clock/intel,grx500-clk.txt | 39 ++++++++++++++++++++++
Please match the compatible string: intel,grx500-cgu.txt
Quoted 15 lines
1 file changed, 39 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/intel,grx500-clk.txtDocumentation/devicetree/bindings/clock/intel,grx500-clk.txt new file +6
-0@@ -0,0 +1,39 @@ +Device Tree Clock bindings for grx500 PLL controller. + +This binding uses the common clock binding: + Documentation/devicetree/bindings/clock/clock-bindings.txt + +The grx500 clock controller supplies clock to various controllers within the
Documentation/devicetree/bindings/clock/intel,grx500-clk.txt line 16
Quoted 10 lines
Documentation/devicetree/bindings/clock/intel,grx500-clk.txt new file +10
-0+SoC. + +Required properties for clock node +- compatible: Should be "intel,grx500-cgu". +- reg: physical base address of the controller and length of memory range. +- #clock-cells: should be 1. + +Optional Propteries: +- intel,osc-frequency: frequency of the osc clock. +if missing, driver will use clock rate defined in the driver.
This should use a fixed-clock node instead.
Quoted 26 lines
Documentation/devicetree/bindings/clock/intel,grx500-clk.txt new file +23
-1+ +Example: Clock controller node: + + cgu: cgu@16200000 { + compatible = "intel,grx500-cgu", "syscon"; + reg = <0x16200000 0x200>; + #clock-cells = <1>; + }; + + +Example: UART controller node that consumes the clock generated by clock + controller. + + asc0: serial@16600000 { + compatible = "lantiq,asc"; + reg = <0x16600000 0x100000>; + interrupt-parent = <&gic>; + interrupts = <GIC_SHARED 103 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 105 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 106 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cgu CLK_SSX4>, <&cgu GCLK_UART>; + clock-names = "freq", "asc"; + }; --2.11.0
Re: [PATCH v2 03/18] dt-bindings: clk: Add documentation of grx500 clock controller
Quoted 9 lines · Rob Herring
Quoted 8 lines · Songjun Wu
From: Yixin Zhu <[email protected]>
This patch adds binding documentation for grx500 clock controller.
Signed-off-by YiXin Zhu <[email protected]>Signed-off-by Songjun Wu <[email protected]>---
Quoted 7 lines
Quoted 6 lines
Changes in v2:
- Rewrite clock driver's dt-binding document according to Rob Herring's comments.
- Simplify device tree docoment, remove some clock description..../devicetree/bindings/clock/intel,grx500-clk.txt | 39 ++++++++++++++++++++++Please match the compatible string: intel,grx500-cgu.txt
Will update to use same name.
Quoted 19 lines
Quoted 19 lines
1 file changed, 39 insertions(+)create mode 100644
Documentation/devicetree/bindings/clock/intel,grx500-clk.txtDocumentation/devicetree/bindings/clock/intel,grx500-clk.txt new file +9
-0@@ -0,0 +1,39 @@ +Device Tree Clock bindings for grx500 PLL controller. + +This binding uses the common clock binding: + Documentation/devicetree/bindings/clock/clock-bindings.txt + +The grx500 clock controller supplies clock to various controllers within the +SoC. + +Required properties for clock node
Quoted 8 lines
Documentation/devicetree/bindings/clock/intel,grx500-clk.txt line 16
Quoted 7 lines
Documentation/devicetree/bindings/clock/intel,grx500-clk.txt new file +7
-0+- compatible: Should be "intel,grx500-cgu". +- reg: physical base address of the controller and length of memory range. +- #clock-cells: should be 1. + +Optional Propteries: +- intel,osc-frequency: frequency of the osc clock. +if missing, driver will use clock rate defined in the driver.This should use a fixed-clock node instead.
Yes, This is a fixed clock node registered in driver code.
The frequency of the fixed clock is designed to be overwritten by device tree in case some one verify clock driver in the emulation platform or in some cases frequency other than driver defined one is preferred. These kinds of cases are very rare. But I feel it would be better to have a way to use customized frequency. The frequency defined in device tree will overwritten driver defined frequency before registering fixed-clock node.
Quoted 26 lines
Quoted 26 lines
Documentation/devicetree/bindings/clock/intel,grx500-clk.txt new file +23
-1+ +Example: Clock controller node: + + cgu: cgu@16200000 { + compatible = "intel,grx500-cgu", "syscon"; + reg = <0x16200000 0x200>; + #clock-cells = <1>; + }; + + +Example: UART controller node that consumes the clock generated by clock + controller. + + asc0: serial@16600000 { + compatible = "lantiq,asc"; + reg = <0x16600000 0x100000>; + interrupt-parent = <&gic>; + interrupts = <GIC_SHARED 103 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 105 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 106 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cgu CLK_SSX4>, <&cgu GCLK_UART>; + clock-names = "freq", "asc"; + }; --2.11.0
Re: [PATCH v2 03/18] dt-bindings: clk: Add documentation of grx500 clock controller
Quoted 49 lines · yixin zhu
Quoted 9 lines · Rob Herring
Quoted 8 lines · Songjun Wu
From: Yixin Zhu <[email protected]>
This patch adds binding documentation for grx500 clock controller.
Signed-off-by YiXin Zhu <[email protected]>Signed-off-by Songjun Wu <[email protected]>---
Quoted 7 lines
Quoted 6 lines
Changes in v2:
- Rewrite clock driver's dt-binding document according to Rob Herring's comments.
- Simplify device tree docoment, remove some clock description..../devicetree/bindings/clock/intel,grx500-clk.txt | 39 ++++++++++++++++++++++Please match the compatible string: intel,grx500-cgu.txt
Will update to use same name.
Quoted 19 lines
Quoted 19 lines
1 file changed, 39 insertions(+)create mode 100644
Documentation/devicetree/bindings/clock/intel,grx500-clk.txtDocumentation/devicetree/bindings/clock/intel,grx500-clk.txt new file +9
-0@@ -0,0 +1,39 @@ +Device Tree Clock bindings for grx500 PLL controller. + +This binding uses the common clock binding: + Documentation/devicetree/bindings/clock/clock-bindings.txt + +The grx500 clock controller supplies clock to various controllers within the +SoC. + +Required properties for clock nodeQuoted 8 lines
Documentation/devicetree/bindings/clock/intel,grx500-clk.txt line 16
Quoted 7 lines
Documentation/devicetree/bindings/clock/intel,grx500-clk.txt new file +7
-0+- compatible: Should be "intel,grx500-cgu". +- reg: physical base address of the controller and length of memory range. +- #clock-cells: should be 1. + +Optional Propteries: +- intel,osc-frequency: frequency of the osc clock. +if missing, driver will use clock rate defined in the driver.This should use a fixed-clock node instead.
Quoted 5 lines
Yes, This is a fixed clock node registered in driver code. The frequency of the fixed clock is designed to be overwritten by device tree in case some one verify
clock driver in the emulation platform or in some cases frequency other than driver defined one is preferred.
Emulation platforms often need several hacks that shouldn't be upstream...
Quoted 4 lines
These kinds of cases are very rare. But I feel it would be better to have a way to use customized frequency.
The frequency defined in device tree will overwritten driver defined frequency before registering fixed-clock node.
I don't think you understand what I meant. Add a DT node with "fixed-clock" compatible and have this node refer to it with "clocks" property. The frequency is still in DT, but uses the clock binding.
Rob
PATCH v2 4/18 MIPS: dts: Add initial support for Intel MIPS SoCs
In reply to Songjun WuFrom: Hua Ma <[email protected]>
Add dts files to support Intel MIPS SoCs:
- xrx500.dtsi is the chip dts
- easy350_anywan.dts is the board dts
---
Changes in v2:
- New patch split from previous patch
- The memory address is changed to @20000000
- Update to obj-$(CONFIG_BUILTIN_DTB) as per commit fca3aa166422
arch/mips/boot/dts/Makefile | 1 + arch/mips/boot/dts/intel-mips/Makefile | 4 ++ arch/mips/boot/dts/intel-mips/easy350_anywan.dts | 26 ++++++++++ arch/mips/boot/dts/intel-mips/xrx500.dtsi | 66 ++++++++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 arch/mips/boot/dts/intel-mips/Makefile create mode 100644 arch/mips/boot/dts/intel-mips/easy350_anywan.dts create mode 100644 arch/mips/boot/dts/intel-mips/xrx500.dtsi
arch/mips/boot/dts/Makefile +1 -0
@@ -3,6 +3,7 @@ subdir-y += brcm subdir-y += cavium-octeon subdir-y += img subdir-y += ingenic +subdir-y += intel-mips subdir-y += lantiq subdir-y += mscc subdir-y += mti
arch/mips/boot/dts/intel-mips/Makefile new file +4 -0
@@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0 +dtb-$(CONFIG_DTB_INTEL_MIPS_GRX500) += easy350_anywan.dtb + +obj-$(CONFIG_BUILTIN_DTB) += $(addsuffix .o, $(dtb-y))
arch/mips/boot/dts/intel-mips/easy350_anywan.dts new file +26 -0
@@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; + +#include <dt-bindings/interrupt-controller/mips-gic.h> +#include <dt-bindings/clock/intel,grx500-clk.h> + +#include "xrx500.dtsi" + +/ { + model = "EASY350 ANYWAN (GRX350) Main model"; + compatible = "intel,easy350-anywan"; + + aliases { + serial0 = &asc0; + }; + + chosen { + bootargs = "earlycon=lantiq,0x16600000 clk_ignore_unused"; + stdout-path = "serial0"; + }; + + memory@20000000 { + device_type = "memory"; + reg = <0x20000000 0x0e000000>; + }; +};
arch/mips/boot/dts/intel-mips/xrx500.dtsi new file +66 -0
66 lines not on this page · raw message
Signature · 1 line
2.11.0
Re: [PATCH v2 04/18] MIPS: dts: Add initial support for Intel MIPS SoCs
Quoted 52 lines · Songjun Wu
From: Hua Ma <[email protected]>
Add dts files to support Intel MIPS SoCs:
-xrx500.dtsiis the chip dts
-easy350_anywan.dtsis the board dtsSigned-off-by Hua Ma <[email protected]>Signed-off-by Songjun Wu <[email protected]>---
Changes in v2:
- New patch split from previous patch
- The memory address is changed to @20000000
- Update to obj-$(CONFIG_BUILTIN_DTB) as per commitfca3aa166422arch/mips/boot/dts/Makefile | 1 + arch/mips/boot/dts/intel-mips/Makefile | 4 ++ arch/mips/boot/dts/intel-mips/easy350_anywan.dts | 26 ++++++++++ arch/mips/boot/dts/intel-mips/xrx500.dtsi | 66 ++++++++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 arch/mips/boot/dts/intel-mips/Makefile create mode 100644 arch/mips/boot/dts/intel-mips/easy350_anywan.dts create mode 100644 arch/mips/boot/dts/intel-mips/xrx500.dtsiarch/mips/boot/dts/Makefile +1
-0@@ -3,6 +3,7 @@ subdir-y += brcm subdir-y += cavium-octeon subdir-y += img subdir-y += ingenic +subdir-y += intel-mips subdir-y += lantiq subdir-y += mscc subdir-y += mtiarch/mips/boot/dts/intel-mips/Makefile new file +4
-0@@ -0,0 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0 +dtb-$(CONFIG_DTB_INTEL_MIPS_GRX500) += easy350_anywan.dtb + +obj-$(CONFIG_BUILTIN_DTB) += $(addsuffix .o, $(dtb-y))arch/mips/boot/dts/intel-mips/easy350_anywan.dts new file +0
-0
arch/mips/boot/dts/intel-mips/easy350_anywan.dts line 10
Quoted 10 lines
arch/mips/boot/dts/intel-mips/easy350_anywan.dts new file +10
-0@@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; + +#include <dt-bindings/interrupt-controller/mips-gic.h> +#include <dt-bindings/clock/intel,grx500-clk.h> + +#include "xrx500.dtsi" + +/ { + model = "EASY350 ANYWAN (GRX350) Main model";
Main model can be removed, it does not identify the board.
arch/mips/boot/dts/intel-mips/easy350_anywan.dts line 11
Quoted 1 line
arch/mips/boot/dts/intel-mips/easy350_anywan.dts new file +1
-0+ compatible = "intel,easy350-anywan";
I think this should be
compatible = "intel,easy350-anywan", "intel,xrx500";
Are there different revisions of the EASY350 Anywan board or only of the EASY550 board?There are at least some differences in the power supply on the EASY550 V1 and EASY550 V2 board. I would suggest to be here very specific to make it easier when adding more boards.
arch/mips/boot/dts/intel-mips/easy350_anywan.dts line 18
Quoted 7 lines
arch/mips/boot/dts/intel-mips/easy350_anywan.dts new file +7
-0+ + aliases { + serial0 = &asc0; + }; + + chosen { + bootargs = "earlycon=lantiq,0x16600000 clk_ignore_unused";
What happens when you remove clk_ignore_unused?
If it crashes we should probably define some of the clock to be always active.
Quoted 81 lines
arch/mips/boot/dts/intel-mips/easy350_anywan.dts new file +8
-0+ stdout-path = "serial0"; + }; + + memory@20000000 { + device_type = "memory"; + reg = <0x20000000 0x0e000000>; + }; +};arch/mips/boot/dts/intel-mips/xrx500.dtsi new file +66
-0@@ -0,0 +1,66 @@ +// SPDX-License-Identifier: GPL-2.0 + +/ { + #address-cells = <1>; + #size-cells = <1>; + compatible = "intel,xrx500"; + + cpus { + #address-cells = <1>; + #size-cells = <0>; + + cpu0: cpu@0 { + device_type = "cpu"; + compatible = "mti,interaptiv"; + clocks = <&cgu CLK_CPU>; + reg = <0>; + }; + + cpu1: cpu@1 { + device_type = "cpu"; + compatible = "mti,interaptiv"; + reg = <1>; + }; + }; + + cpu_intc: interrupt-controller { + compatible = "mti,cpu-interrupt-controller"; + + interrupt-controller; + #interrupt-cells = <1>; + }; + + gic: gic@12320000 { + compatible = "mti,gic"; + reg = <0x12320000 0x20000>; + + interrupt-controller; + #interrupt-cells = <3>; + /* + * Declare the interrupt-parent even though the mti,gic + * binding doesn't require it, such that the kernel can + * figure out that cpu_intc is the root interrupt + * controller & should be probed first. + */ + interrupt-parent = <&cpu_intc>; + mti,reserved-ipi-vectors = <56 8>; + }; + + cgu: cgu@16200000 { + compatible = "intel,grx500-cgu", "syscon"; + reg = <0x16200000 0x200>; + #clock-cells = <1>; + }; + + asc0: serial@16600000 { + compatible = "lantiq,asc"; + reg = <0x16600000 0x100000>; + + interrupt-parent = <&gic>; + interrupts = <GIC_SHARED 103 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 105 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 106 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cgu CLK_SSX4>, <&cgu GCLK_UART>; + clock-names = "freq", "asc"; + }; +};
Re: [PATCH v2 04/18] MIPS: dts: Add initial support for Intel MIPS SoCs
Quoted 34 lines · Hauke Mehrtens
Quoted 33 lines · Songjun Wu
From: Hua Ma <[email protected]>
Add dts files to support Intel MIPS SoCs:
-xrx500.dtsiis the chip dts
-easy350_anywan.dtsis the board dtsSigned-off-by Hua Ma <[email protected]>Signed-off-by Songjun Wu <[email protected]>---
Changes in v2:
- New patch split from previous patch
- The memory address is changed to @20000000
- Update to obj-$(CONFIG_BUILTIN_DTB) as per commitfca3aa166422arch/mips/boot/dts/Makefile | 1 + arch/mips/boot/dts/intel-mips/Makefile | 4 ++ arch/mips/boot/dts/intel-mips/easy350_anywan.dts | 26 ++++++++++ arch/mips/boot/dts/intel-mips/xrx500.dtsi | 66 ++++++++++++++++++++++++ 4 files changed, 97 insertions(+)create mode 100644
arch/mips/boot/dts/intel-mips/Makefilecreate mode 100644arch/mips/boot/dts/intel-mips/easy350_anywan.dtscreate mode 100644arch/mips/boot/dts/intel-mips/xrx500.dtsiarch/mips/boot/dts/intel-mips/easy350_anywan.dts new file +3
-0@@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-2.0 +/dts-v1/; +
Quoted 8 lines
arch/mips/boot/dts/intel-mips/easy350_anywan.dts line 10
Quoted 7 lines
arch/mips/boot/dts/intel-mips/easy350_anywan.dts new file +7
-0+#include <dt-bindings/interrupt-controller/mips-gic.h> +#include <dt-bindings/clock/intel,grx500-clk.h> + +#include "xrx500.dtsi" + +/ { + model = "EASY350 ANYWAN (GRX350) Main model";Main model can be removed, it does not identify the board.
Thanks, will remove.
Quoted 8 lines
arch/mips/boot/dts/intel-mips/easy350_anywan.dts line 11
Quoted 1 line
arch/mips/boot/dts/intel-mips/easy350_anywan.dts new file +1
-0+ compatible = "intel,easy350-anywan";I think this should be
compatible = "intel,easy350-anywan", "intel,xrx500";Are there different revisions of the EASY350 Anywan board or only of the EASY550 board?There are at least some differences in the power supply on the EASY550 V1 and EASY550 V2 board. I would suggest to be here very specific to make it easier when adding more boards.
OK, thanks.
Quoted 10 lines
arch/mips/boot/dts/intel-mips/easy350_anywan.dts line 18
Quoted 7 lines
arch/mips/boot/dts/intel-mips/easy350_anywan.dts new file +7
-0+ + aliases { + serial0 = &asc0; + }; + + chosen { + bootargs = "earlycon=lantiq,0x16600000 clk_ignore_unused";What happens when you remove
clk_ignore_unused?
If it crashes we should probably define some of the clock to be always active.
OK, will check and improve if possible.
PATCH v2 5/18 dt-binding: MIPS: Add documentation of Intel MIPS SoCs
In reply to Songjun WuFrom: Hua Ma <[email protected]>
This patch adds binding documentation for the
compatible values of the Intel MIPS SoCs.
---
Changes in v2:
- New patch split from previous patch
- Add the board and chip compatible in dt document
Documentation/devicetree/bindings/mips/intel.txt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 Documentation/devicetree/bindings/mips/intel.txt
Documentation/devicetree/bindings/mips/intel.txt new file +17 -0
@@ -0,0 +1,17 @@ +Intel MIPS SoC device tree bindings + +1, SoCs + +Each device tree must specify a compatible value for the Intel SoC +it uses in the compatible property of the root node. The compatible +value must be one of the following values: + + intel,xrx500 + +2, Boards + +Each device tree must specify a compatible value for the Intel Board +it uses in the compatible property of the root node. The compatible +value must be one of the following values: + + intel,easy350-anywan
Signature · 1 line
2.11.0
Re: [PATCH v2 05/18] dt-binding: MIPS: Add documentation of Intel MIPS SoCs
Quoted 10 lines · Songjun Wu
From: Hua Ma <[email protected]>
This patch adds binding documentation for the
compatible values of the Intel MIPS SoCs.Signed-off-by Hua Ma <[email protected]>Signed-off-by Songjun Wu <[email protected]>---
Quoted 7 lines
Changes in v2:
- New patch split from previous patch
- Add the board and chip compatible in dt documentDocumentation/devicetree/bindings/mips/intel.txt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 Documentation/devicetree/bindings/mips/intel.txt
PATCH v2 6/18 MIPS: dts: Change upper case to lower case
In reply to Songjun WuAll the upper case in unit-address and hex constants are changed to lower case according to the Linux conventions.
---
Changes in v2: None
arch/mips/boot/dts/lantiq/danube.dtsi | 42 ++++++++++++++++----------------- arch/mips/boot/dts/lantiq/easy50712.dts | 14 +++++------ 2 files changed, 28 insertions(+), 28 deletions(-)
arch/mips/boot/dts/lantiq/danube.dtsi +21 -21
@@ -10,12 +10,12 @@ }; }; - biu@1F800000 { + biu@1f800000 { #address-cells = <1>; #size-cells = <1>; compatible = "lantiq,biu", "simple-bus"; - reg = <0x1F800000 0x800000>; - ranges = <0x0 0x1F800000 0x7FFFFF>; + reg = <0x1f800000 0x800000>; + ranges = <0x0 0x1f800000 0x7fffff>; icu0: icu@80200 { #interrupt-cells = <1>;
@@ -24,18 +24,18 @@ reg = <0x80200 0x120>; }; - watchdog@803F0 { + watchdog@803f0 { compatible = "lantiq,wdt"; - reg = <0x803F0 0x10>; + reg = <0x803f0 0x10>; }; }; - sram@1F000000 { + sram@1f000000 { #address-cells = <1>; #size-cells = <1>; compatible = "lantiq,sram"; - reg = <0x1F000000 0x800000>; - ranges = <0x0 0x1F000000 0x7FFFFF>; + reg = <0x1f000000 0x800000>; + ranges = <0x0 0x1f000000 0x7fffff>; eiu0: eiu@101000 { #interrupt-cells = <1>;
@@ -66,41 +66,41 @@ #address-cells = <1>; #size-cells = <1>; compatible = "lantiq,fpi", "simple-bus"; - ranges = <0x0 0x10000000 0xEEFFFFF>; - reg = <0x10000000 0xEF00000>; + ranges = <0x0 0x10000000 0xeefffff>; + reg = <0x10000000 0xef00000>; - gptu@E100A00 { + gptu@e100a00 { compatible = "lantiq,gptu-xway"; - reg = <0xE100A00 0x100>; + reg = <0xe100a00 0x100>; }; - serial@E100C00 { + serial@e100c00 { compatible = "lantiq,asc"; - reg = <0xE100C00 0x400>; + reg = <0xe100c00 0x400>; interrupt-parent = <&icu0>; interrupts = <112 113 114>; }; - dma0: dma@E104100 { + dma0: dma@e104100 { compatible = "lantiq,dma-xway"; - reg = <0xE104100 0x800>; + reg = <0xe104100 0x800>; }; - ebu0: ebu@E105300 { + ebu0: ebu@e105300 { compatible = "lantiq,ebu-xway"; - reg = <0xE105300 0x100>; + reg = <0xe105300 0x100>; }; - pci0: pci@E105400 { + pci0: pci@e105400 { #address-cells = <3>; #size-cells = <2>; #interrupt-cells = <1>; compatible = "lantiq,pci-xway"; bus-range = <0x0 0x0>; ranges = <0x2000000 0 0x8000000 0x8000000 0 0x2000000 /* pci memory */ - 0x1000000 0 0x00000000 0xAE00000 0 0x200000>; /* io space */ + 0x1000000 0 0x00000000 0xae00000 0 0x200000>; /* io space */ reg = <0x7000000 0x8000 /* config space */ - 0xE105400 0x400>; /* pci bridge */ + 0xe105400 0x400>; /* pci bridge */ }; }; };
arch/mips/boot/dts/lantiq/easy50712.dts +7 -7
@@ -52,14 +52,14 @@ }; }; - gpio: pinmux@E100B10 { + gpio: pinmux@e100b10 { compatible = "lantiq,danube-pinctrl"; pinctrl-names = "default"; pinctrl-0 = <&state_default>; #gpio-cells = <2>; gpio-controller; - reg = <0xE100B10 0xA0>; + reg = <0xe100b10 0xa0>; state_default: pinmux { stp {
@@ -82,26 +82,26 @@ }; }; - etop@E180000 { + etop@e180000 { compatible = "lantiq,etop-xway"; - reg = <0xE180000 0x40000>; + reg = <0xe180000 0x40000>; interrupt-parent = <&icu0>; interrupts = <73 78>; phy-mode = "rmii"; mac-address = [ 00 11 22 33 44 55 ]; }; - stp0: stp@E100BB0 { + stp0: stp@e100bb0 { #gpio-cells = <2>; compatible = "lantiq,gpio-stp-xway"; gpio-controller; - reg = <0xE100BB0 0x40>; + reg = <0xe100bb0 0x40>; lantiq,shadow = <0xfff>; lantiq,groups = <0x3>; }; - pci@E105400 { + pci@e105400 { lantiq,bus-clock = <33333333>; interrupt-map-mask = <0xf800 0x0 0x0 0x7>; interrupt-map = <
Signature · 1 line
2.11.0
Re: [PATCH v2 06/18] MIPS: dts: Change upper case to lower case
Quoted 3 lines · Songjun Wu
All the upper case in unit-address and hex constants are changed to lower case according to the Linux conventions.
It is DT conventions, not Linux.
Otherwise,
Quoted 9 lines
Signed-off-by Songjun Wu <[email protected]>---
Changes in v2: None
arch/mips/boot/dts/lantiq/danube.dtsi | 42 ++++++++++++++++----------------- arch/mips/boot/dts/lantiq/easy50712.dts | 14 +++++------ 2 files changed, 28 insertions(+), 28 deletions(-)
PATCH v2 7/18 MIPS: dts: Add aliases node for lantiq danube serial
In reply to Songjun WuPrevious implementation uses a hard-coded register value to check if the current serial entity is the console entity.
Now the lantiq serial driver uses the aliases for the index of the serial port.
The lantiq danube serial dts are updated with aliases to support this.
---
Changes in v2: None
arch/mips/boot/dts/lantiq/danube.dtsi | 2 +- arch/mips/boot/dts/lantiq/easy50712.dts | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-)
arch/mips/boot/dts/lantiq/danube.dtsi +1 -1
@@ -74,7 +74,7 @@ reg = <0xe100a00 0x100>; }; - serial@e100c00 { + asc1: serial@e100c00 { compatible = "lantiq,asc"; reg = <0xe100c00 0x400>; interrupt-parent = <&icu0>;
arch/mips/boot/dts/lantiq/easy50712.dts +4 -0
@@ -4,6 +4,10 @@ /include/ "danube.dtsi" / { + aliases { + serial0 = &asc1; + }; + chosen { bootargs = "console=ttyLTQ0,115200 init=/etc/preinit"; };
Signature · 1 line
2.11.0
PATCH v2 8/18 serial: intel: Get serial id from dts
In reply to Songjun WuGet serial id from dts.
"#ifdef CONFIG_LANTIQ" preprocessor is used because LTQ_EARLY_ASC macro is defined in lantiq_soc.h.lantiq_soc.h is in arch path for legacy product support.
arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.h
If "#ifdef preprocessor" is changed to
"if (IS_ENABLED(CONFIG_LANTIQ))", when CONFIG_LANTIQ is not enabled, code using LTQ_EARLY_ASC is compiled.
Compilation will fail for no LTQ_EARLY_ASC defined.
---
Changes in v2: None
drivers/tty/serial/lantiq.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
drivers/tty/serial/lantiq.c +15 -4
@@ -6,6 +6,7 @@* Copyright (C) 2007 Felix Fietkau <[email protected]> * Copyright (C) 2007 John Crispin <[email protected]> * Copyright (C) 2010 Thomas Langer, <[email protected]> +* Copyright (C) 2018 Intel Corporation. */ #include <linux/slab.h>
@@ -688,7 +689,7 @@ lqasc_probe(struct platform_device *pdev)struct ltq_uart_port *ltq_port;struct uart_port *port;struct resource *mmres, irqres[3 ]; -int line =0 ; +int line;int ret; mmres =platform_get_resource (pdev,IORESOURCE_MEM ,0 );
@@ -699,9 +700,19 @@ lqasc_probe(struct platform_device *pdev)return -ENODEV ; } -/* check if this is the console port */ -if (mmres->start !=CPHYSADDR (LTQ_EARLY_ASC )) - line =1 ; +/* get serial id */ + line =of_alias_get_id (node,"serial" ); +if (line <0 ) { +#ifdef CONFIG_LANTIQ +if (mmres->start ==CPHYSADDR (LTQ_EARLY_ASC )) + line =0 ; +else + line =1 ; +#else +dev_err (&pdev->dev,"failed to get alias id, errno %d\n" , line); +return line; +#endif + }if (lqasc_port[line]) {dev_err (&pdev->dev,"port %d already allocated\n" , line);
Signature · 1 line
2.11.0
Greg Kroah-Hartman
maintainerRe: [PATCH v2 08/18] serial: intel: Get serial id from dts
Quoted 26 lines · Songjun Wu
Get serial id from dts.
"#ifdef
CONFIG_LANTIQ" preprocessor is used becauseLTQ_EARLY_ASCmacro is defined inlantiq_soc.h.lantiq_soc.his in arch path for legacy product support.
arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.hIf "#ifdef preprocessor" is changed to
"if (IS_ENABLED(CONFIG_LANTIQ))", whenCONFIG_LANTIQis not enabled, code usingLTQ_EARLY_ASCis compiled.
Compilation will fail for noLTQ_EARLY_ASCdefined.Signed-off-by Songjun Wu <[email protected]>---
Changes in v2: None
drivers/tty/serial/lantiq.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)drivers/tty/serial/lantiq.c +0
-0
drivers/tty/serial/lantiq.c line 9
Quoted 4 lines
drivers/tty/serial/lantiq.c +1
-0@@ -6,6 +6,7 @@* Copyright (C) 2007 Felix Fietkau <[email protected]> * Copyright (C) 2007 John Crispin <[email protected]> * Copyright (C) 2010 Thomas Langer, <[email protected]> +* Copyright (C) 2018 Intel Corporation.
Your changes here do not warrent the addition of a copyright line, don't you agree? If not, please get a signed-off-by from your corporate lawyer who does this this is warrented when you resend this patch.
thanks,
greg k-h
Re: [PATCH v2 08/18] serial: intel: Get serial id from dts
Quoted 35 lines · Greg Kroah-Hartman
Quoted 26 lines · Songjun Wu
Get serial id from dts.
"#ifdef
CONFIG_LANTIQ" preprocessor is used becauseLTQ_EARLY_ASCmacro is defined inlantiq_soc.h.lantiq_soc.his in arch path for legacy product support.
arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.hIf "#ifdef preprocessor" is changed to
"if (IS_ENABLED(CONFIG_LANTIQ))", whenCONFIG_LANTIQis not enabled, code usingLTQ_EARLY_ASCis compiled.
Compilation will fail for noLTQ_EARLY_ASCdefined.Signed-off-by Songjun Wu <[email protected]>---
Changes in v2: None
drivers/tty/serial/lantiq.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)drivers/tty/serial/lantiq.c +0
-0drivers/tty/serial/lantiq.c line 9
Quoted 4 lines
drivers/tty/serial/lantiq.c +1
-0@@ -6,6 +6,7 @@* Copyright (C) 2007 Felix Fietkau <[email protected]> * Copyright (C) 2007 John Crispin <[email protected]> * Copyright (C) 2010 Thomas Langer, <[email protected]> +* Copyright (C) 2018 Intel Corporation. Your changes here do not warrent the addition of a copyright line, don't you agree? If not, please get a signed-off-by from your corporate lawyer who does this this is warrented when you resend this patch.
Quoted 4 lines
thanks,
greg k-h
Thanks.
The copyright line will be removed when we resend this patch.
Re: [PATCH v2 08/18] serial: intel: Get serial id from dts
In reply to Songjun WuHi Songjun,
Quoted 6 lines · Songjun Wu
Get serial id from dts.
"#ifdef
CONFIG_LANTIQ" preprocessor is used becauseLTQ_EARLY_ASCmacro is defined inlantiq_soc.h.lantiq_soc.his in arch path for legacy product support.
Quoted 8 lines
arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.hIf "#ifdef preprocessor" is changed to
"if (IS_ENABLED(CONFIG_LANTIQ))", whenCONFIG_LANTIQis not enabled, code usingLTQ_EARLY_ASCis compiled.
Compilation will fail for noLTQ_EARLY_ASCdefined.Signed-off-by Songjun Wu <[email protected]>
Thanks for your patch!
Quoted 8 lines
drivers/tty/serial/lantiq.c +1
-3@@ -699,9 +700,19 @@ lqasc_probe(struct platform_device *pdev)return -ENODEV ; } -/* check if this is the console port */ -if (mmres->start !=CPHYSADDR (LTQ_EARLY_ASC )) - line =1 ; +/* get serial id */
drivers/tty/serial/lantiq.c lqasc_probe(struct platform_device *pdev) · line 713
Quoted 10 lines
drivers/tty/serial/lantiq.c +10
-0+ line =of_alias_get_id (node,"serial" ); +if (line <0 ) { +#ifdef CONFIG_LANTIQ +if (mmres->start ==CPHYSADDR (LTQ_EARLY_ASC )) + line =0 ; +else + line =1 ; +#else +dev_err (&pdev->dev,"failed to get alias id, errno %d\n" , line); +return line;
Please note that not providing a fallback here makes life harder when using DT overlays.
See the description of commit 7678f4c20fa7670f ("serial: sh-sci: Add support for dynamic instances") for background info.
drivers/tty/serial/lantiq.c lqasc_probe(struct platform_device *pdev) · line 718
Quoted 5 lines
drivers/tty/serial/lantiq.c +2
-0+#endif + }if (lqasc_port[line]) {dev_err (&pdev->dev,"port %d already allocated\n" , line);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v2 08/18] serial: intel: Get serial id from dts
Quoted 33 lines · Geert Uytterhoeven
Hi Songjun,
Quoted 6 lines · Songjun Wu
Get serial id from dts.
"#ifdef
CONFIG_LANTIQ" preprocessor is used becauseLTQ_EARLY_ASCmacro is defined inlantiq_soc.h.lantiq_soc.his in arch path for legacy product support.Quoted 8 lines
arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.hIf "#ifdef preprocessor" is changed to
"if (IS_ENABLED(CONFIG_LANTIQ))", whenCONFIG_LANTIQis not enabled, code usingLTQ_EARLY_ASCis compiled.
Compilation will fail for noLTQ_EARLY_ASCdefined.Signed-off-by Songjun Wu <[email protected]>Thanks for your patch!
Quoted 14 lines
drivers/tty/serial/lantiq.c +7
-3@@ -699,9 +700,19 @@ lqasc_probe(struct platform_device *pdev)return -ENODEV ; } -/* check if this is the console port */ -if (mmres->start !=CPHYSADDR (LTQ_EARLY_ASC )) - line =1 ; +/* get serial id */ + line =of_alias_get_id (node,"serial" ); +if (line <0 ) { +#ifdef CONFIG_LANTIQ +if (mmres->start ==CPHYSADDR (LTQ_EARLY_ASC )) + line =0 ; +else
Quoted 8 lines
drivers/tty/serial/lantiq.c lqasc_probe(struct platform_device *pdev) · line 713
Quoted 4 lines
drivers/tty/serial/lantiq.c +4
-0+ line =1 ; +#else +dev_err (&pdev->dev,"failed to get alias id, errno %d\n" , line); +return line;Please note that not providing a fallback here makes life harder when using DT overlays.
See the description of commit7678f4c20fa7670f("serial: sh-sci: Add support for dynamic instances") for background info.
Thanks for your comment.
The logic in commit 7678f4c20fa7670f is not suitable here. We need to know which serial instance is used for console. We cannot use dynamic serial instance here.
Re: [PATCH v2 08/18] serial: intel: Get serial id from dts
Hi Songjun,
Quoted 40 lines · Songjun
Quoted 31 lines · Geert Uytterhoeven
Quoted 6 lines · Songjun Wu
Get serial id from dts.
"#ifdef
CONFIG_LANTIQ" preprocessor is used becauseLTQ_EARLY_ASCmacro is defined inlantiq_soc.h.lantiq_soc.his in arch path for legacy product support.Quoted 8 lines
arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.hIf "#ifdef preprocessor" is changed to
"if (IS_ENABLED(CONFIG_LANTIQ))", whenCONFIG_LANTIQis not enabled, code usingLTQ_EARLY_ASCis compiled.
Compilation will fail for noLTQ_EARLY_ASCdefined.Signed-off-by Songjun Wu <[email protected]>Thanks for your patch!
Quoted 14 lines
drivers/tty/serial/lantiq.c +7
-3@@ -699,9 +700,19 @@ lqasc_probe(struct platform_device *pdev)return -ENODEV ; } -/* check if this is the console port */ -if (mmres->start !=CPHYSADDR (LTQ_EARLY_ASC )) - line =1 ; +/* get serial id */ + line =of_alias_get_id (node,"serial" ); +if (line <0 ) { +#ifdef CONFIG_LANTIQ +if (mmres->start ==CPHYSADDR (LTQ_EARLY_ASC )) + line =0 ; +else Quoted 8 lines
drivers/tty/serial/lantiq.c lqasc_probe(struct platform_device *pdev) · line 713
Quoted 4 lines
drivers/tty/serial/lantiq.c +4
-0+ line =1 ; +#else +dev_err (&pdev->dev,"failed to get alias id, errno %d\n" , line); +return line;Please note that not providing a fallback here makes life harder when using DT overlays.
See the description of commit7678f4c20fa7670f("serial: sh-sci: Add support for dynamic instances") for background info.
Quoted 4 lines
Thanks for your comment.
The logic in commit7678f4c20fa7670fis not suitable here. We need to know which serial instance is used for console. We cannot use dynamic serial instance here.
Why does the driver need to use which serial instance is used for the console? Hardcoding that is not an option, as the board DTS may specify the console using chosen/stdout-path.
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v2 08/18] serial: intel: Get serial id from dts
Quoted 43 lines · Geert Uytterhoeven
Hi Songjun,
Quoted 40 lines · Songjun
Quoted 39 lines · Geert Uytterhoeven
Quoted 6 lines · Songjun Wu
Get serial id from dts.
"#ifdef
CONFIG_LANTIQ" preprocessor is used becauseLTQ_EARLY_ASCmacro is defined inlantiq_soc.h.lantiq_soc.his in arch path for legacy product support.Quoted 8 lines
arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.hIf "#ifdef preprocessor" is changed to
"if (IS_ENABLED(CONFIG_LANTIQ))", whenCONFIG_LANTIQis not enabled, code usingLTQ_EARLY_ASCis compiled.
Compilation will fail for noLTQ_EARLY_ASCdefined.Signed-off-by Songjun Wu <[email protected]>Thanks for your patch!
Quoted 8 lines
drivers/tty/serial/lantiq.c +1
-3@@ -699,9 +700,19 @@ lqasc_probe(struct platform_device *pdev)return -ENODEV ; } -/* check if this is the console port */ -if (mmres->start !=CPHYSADDR (LTQ_EARLY_ASC )) - line =1 ; +/* get serial id */ drivers/tty/serial/lantiq.c lqasc_probe(struct platform_device *pdev) · line 713
Quoted 10 lines
drivers/tty/serial/lantiq.c +10
-0+ line =of_alias_get_id (node,"serial" ); +if (line <0 ) { +#ifdef CONFIG_LANTIQ +if (mmres->start ==CPHYSADDR (LTQ_EARLY_ASC )) + line =0 ; +else + line =1 ; +#else +dev_err (&pdev->dev,"failed to get alias id, errno %d\n" , line); +return line;Please note that not providing a fallback here makes life harder when using DT overlays.
See the description of commit7678f4c20fa7670f("serial: sh-sci: Add support for dynamic instances") for background info.
Quoted 7 lines
Quoted 4 lines
Thanks for your comment.
The logic in commit7678f4c20fa7670fis not suitable here. We need to know which serial instance is used for console. We cannot use dynamic serial instance here.Why does the driver need to use which serial instance is used for the console? Hardcoding that is not an option, as the board DTS may specify the console using chosen/stdout-path.
In legacy platform in open source, it only defined asc1 in dts. There's no asc0 in legacy dts.While in the new platform, asc0 is defined in dts. There's no asc1 in new platform dts. To avoid hard code in driver, alias serial0 is used to unified driver code. Actually only one serial is supported in SoC.
aliases {
serial0 = &asc0;
};
chosen {
bootargs = "earlycon clk_ignore_unused";
stdout-path = "serial0";
};
PATCH v2 9/18 serial: intel: Change ltq_w32_mask to asc_update_bits
In reply to Songjun Wultq prefix is platform specific function, asc prefix is more generic.
---
Changes in v2: None
drivers/tty/serial/lantiq.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-)
drivers/tty/serial/lantiq.c +20 -13
@@ -114,6 +114,13 @@ struct ltq_uart_port {unsigned int err_irq; }; +static inline void asc_update_bits (u32 clear,u32 set,void __iomem *reg) +{ +u32 tmp =readl (reg); + +writel ((tmp & ~clear) | set, reg); +} +static inline struct ltq_uart_port *to_ltq_uart_port (struct uart_port *port) {
@@ -164,16 +171,16 @@ lqasc_rx_chars(struct uart_port *port)if (rsr &ASCSTATE_ANY ) {if (rsr &ASCSTATE_PE ) { port->icount.parity++; -ltq_w32_mask (0 ,ASCWHBSTATE_CLRPE , +asc_update_bits (0 ,ASCWHBSTATE_CLRPE , port->membase +LTQ_ASC_WHBSTATE ); }else if (rsr &ASCSTATE_FE ) { port->icount.frame++; -ltq_w32_mask (0 ,ASCWHBSTATE_CLRFE , +asc_update_bits (0 ,ASCWHBSTATE_CLRFE , port->membase +LTQ_ASC_WHBSTATE ); }if (rsr &ASCSTATE_ROE ) { port->icount.overrun++; -ltq_w32_mask (0 ,ASCWHBSTATE_CLRROE , +asc_update_bits (0 ,ASCWHBSTATE_CLRROE , port->membase +LTQ_ASC_WHBSTATE ); }
@@ -253,7 +260,7 @@ lqasc_err_int(int irq, void *_port)struct uart_port *port = (struct uart_port *)_port;spin_lock_irqsave (<q_asc_lock, flags);/* clear any pending interrupts */ -ltq_w32_mask (0 ,ASCWHBSTATE_CLRPE |ASCWHBSTATE_CLRFE | +asc_update_bits (0 ,ASCWHBSTATE_CLRPE |ASCWHBSTATE_CLRFE |ASCWHBSTATE_CLRROE , port->membase +LTQ_ASC_WHBSTATE );spin_unlock_irqrestore (<q_asc_lock, flags);return IRQ_HANDLED ;
@@ -305,7 +312,7 @@ lqasc_startup(struct uart_port *port)clk_enable (ltq_port->clk); port->uartclk =clk_get_rate (ltq_port->fpiclk); -ltq_w32_mask (ASCCLC_DISS |ASCCLC_RMCMASK , (1 <<ASCCLC_RMCOFFSET ), +asc_update_bits (ASCCLC_DISS |ASCCLC_RMCMASK , (1 <<ASCCLC_RMCOFFSET ), port->membase +LTQ_ASC_CLC );ltq_w32 (0 , port->membase +LTQ_ASC_PISEL );
@@ -321,7 +328,7 @@ lqasc_startup(struct uart_port *port)* setting enable bits */ wmb (); -ltq_w32_mask (0 ,ASCCON_M_8ASYNC |ASCCON_FEN |ASCCON_TOEN | +asc_update_bits (0 ,ASCCON_M_8ASYNC |ASCCON_FEN |ASCCON_TOEN |ASCCON_ROEN , port->membase +LTQ_ASC_CON ); retval = request_irq(ltq_port->tx_irq, lqasc_tx_int,
@@ -365,9 +372,9 @@ lqasc_shutdown(struct uart_port *port)free_irq (ltq_port->err_irq, port);ltq_w32 (0 , port->membase +LTQ_ASC_CON ); -ltq_w32_mask (ASCRXFCON_RXFEN ,ASCRXFCON_RXFFLU , +asc_update_bits (ASCRXFCON_RXFEN ,ASCRXFCON_RXFFLU , port->membase +LTQ_ASC_RXFCON ); -ltq_w32_mask (ASCTXFCON_TXFEN ,ASCTXFCON_TXFFLU , +asc_update_bits (ASCTXFCON_TXFEN ,ASCTXFCON_TXFFLU , port->membase +LTQ_ASC_TXFCON );if (!IS_ERR (ltq_port->clk))clk_disable (ltq_port->clk);
@@ -439,7 +446,7 @@ lqasc_set_termios(struct uart_port *port,spin_lock_irqsave (<q_asc_lock, flags);/* set up CON */ -ltq_w32_mask (0 , con, port->membase +LTQ_ASC_CON ); +asc_update_bits (0 , con, port->membase +LTQ_ASC_CON );/* Set baud rate - take a divider of 2 into account */ baud =uart_get_baud_rate (port, new, old,0 , port->uartclk /16 );
@@ -447,19 +454,19 @@ lqasc_set_termios(struct uart_port *port, divisor = divisor /2 -1 ;/* disable the baudrate generator */ -ltq_w32_mask (ASCCON_R ,0 , port->membase +LTQ_ASC_CON ); +asc_update_bits (ASCCON_R ,0 , port->membase +LTQ_ASC_CON );/* make sure the fractional divider is off */ -ltq_w32_mask (ASCCON_FDE ,0 , port->membase +LTQ_ASC_CON ); +asc_update_bits (ASCCON_FDE ,0 , port->membase +LTQ_ASC_CON );/* set up to use divisor of 2 */ -ltq_w32_mask (ASCCON_BRS ,0 , port->membase +LTQ_ASC_CON ); +asc_update_bits (ASCCON_BRS ,0 , port->membase +LTQ_ASC_CON );/* now we can write the new baudrate into the register */ ltq_w32 (divisor, port->membase +LTQ_ASC_BG );/* turn the baudrate generator back on */ -ltq_w32_mask (0 ,ASCCON_R , port->membase +LTQ_ASC_CON ); +asc_update_bits (0 ,ASCCON_R , port->membase +LTQ_ASC_CON );/* enable rx */ ltq_w32 (ASCWHBSTATE_SETREN , port->membase +LTQ_ASC_WHBSTATE );
Signature · 1 line
2.11.0
PATCH v2 10/18 MIPS: lantiq: Unselect SWAP_IO_SPACE when LANTIQ is selected
In reply to Songjun WuSWAP_IO_SPACE macro prevents serial driver /drivers/tty/serial/lantiq.c to use readl/writel to replace ltq_r32/w32 which are SoC or platform specific APIs.
readl/writel are used for this serial driver to support multiple platforms and multiple architectures. The legacy lantiq platform(Danube) enables SWAP_IO_SPACE for supporting PCI due to some hardware bugs.
It's a little-endian bus plus PCI TX/RX swap enable impacted both data and control path for MIPS based platforms. But it is better to let PCI device driver to do endian swap since SWAP_IO_SPACE is a global wide macro which potentially impacts other peripheral like USB. ltq_r32/ltq_w32 is not impacted in other device drivers based on MIPS when SWAP_IO_SPACE is not selected as they use non-byte swapping OS API (__raw_read/__raw_writel).
---
Changes in v2: None
arch/mips/Kconfig | 1 - 1 file changed, 1 deletion(-)
arch/mips/Kconfig +0 -1
@@ -400,7 +400,6 @@ config LANTIQ select SYS_SUPPORTS_VPE_LOADER select SYS_HAS_EARLY_PRINTK select GPIOLIB - select SWAP_IO_SPACE select BOOT_RAW select CLKDEV_LOOKUP select USE_OF
Signature · 1 line
2.11.0
PATCH v2 11/18 serial: intel: Use readl/writel instead of ltq_r32/ltq_w32
In reply to Songjun WuPrevious implementation uses platform-dependent functions ltq_w32()/ltq_r32() to access registers. Those functions are not available for other SoC which uses the same IP.
Change to OS provided readl()/writel() and readb()/writeb(), so that different SoCs can use the same driver.
---
Changes in v2: None
drivers/tty/serial/lantiq.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-)
drivers/tty/serial/lantiq.c +19 -19
@@ -146,7 +146,7 @@ lqasc_start_tx(struct uart_port *port)static void lqasc_stop_rx (struct uart_port *port) { -ltq_w32 (ASCWHBSTATE_CLRREN , port->membase +LTQ_ASC_WHBSTATE ); +writel (ASCWHBSTATE_CLRREN , port->membase +LTQ_ASC_WHBSTATE ); } static int
@@ -155,11 +155,11 @@ lqasc_rx_chars(struct uart_port *port)struct tty_port *tport = &port->state->port;unsigned int ch =0 , rsr =0 , fifocnt; - fifocnt =ltq_r32 (port->membase +LTQ_ASC_FSTAT ) &ASCFSTAT_RXFFLMASK ; + fifocnt =readl (port->membase +LTQ_ASC_FSTAT ) &ASCFSTAT_RXFFLMASK ;while (fifocnt--) {u8 flag =TTY_NORMAL ; - ch =ltq_r8 (port->membase +LTQ_ASC_RBUF ); - rsr = (ltq_r32 (port->membase +LTQ_ASC_STATE ) + ch =readb (port->membase +LTQ_ASC_RBUF ); + rsr = (readl (port->membase +LTQ_ASC_STATE ) &ASCSTATE_ANY ) |UART_DUMMY_UER_RX ;tty_flip_buffer_push (tport); port->icount.rx++;
@@ -219,10 +219,10 @@ lqasc_tx_chars(struct uart_port *port)return ; } -while (((ltq_r32 (port->membase +LTQ_ASC_FSTAT ) & +while (((readl (port->membase +LTQ_ASC_FSTAT ) &ASCFSTAT_TXFREEMASK ) >>ASCFSTAT_TXFREEOFF ) !=0 ) {if (port->x_char) { -ltq_w8 (port->x_char, port->membase +LTQ_ASC_TBUF ); +writeb (port->x_char, port->membase +LTQ_ASC_TBUF ); port->icount.tx++; port->x_char =0 ;continue ;
@@ -231,7 +231,7 @@ lqasc_tx_chars(struct uart_port *port)if (uart_circ_empty (xmit))break ; -ltq_w8 (port->state->xmit.buf[port->state->xmit.tail], +writeb (port->state->xmit.buf[port->state->xmit.tail], port->membase +LTQ_ASC_TBUF ); xmit->tail = (xmit->tail +1 ) & (UART_XMIT_SIZE -1 ); port->icount.tx++;
@@ -247,7 +247,7 @@ lqasc_tx_int(int irq, void *_port)unsigned long flags;struct uart_port *port = (struct uart_port *)_port;spin_lock_irqsave (<q_asc_lock, flags); -ltq_w32 (ASC_IRNCR_TIR , port->membase +LTQ_ASC_IRNCR ); +writel (ASC_IRNCR_TIR , port->membase +LTQ_ASC_IRNCR );spin_unlock_irqrestore (<q_asc_lock, flags);lqasc_start_tx (port);return IRQ_HANDLED ;
@@ -272,7 +272,7 @@ lqasc_rx_int(int irq, void *_port)unsigned long flags;struct uart_port *port = (struct uart_port *)_port;spin_lock_irqsave (<q_asc_lock, flags); -ltq_w32 (ASC_IRNCR_RIR , port->membase +LTQ_ASC_IRNCR ); +writel (ASC_IRNCR_RIR , port->membase +LTQ_ASC_IRNCR );lqasc_rx_chars (port);spin_unlock_irqrestore (<q_asc_lock, flags);return IRQ_HANDLED ;
@@ -282,7 +282,7 @@ static unsigned int lqasc_tx_empty(struct uart_port *port) {int status; - status =ltq_r32 (port->membase +LTQ_ASC_FSTAT ) &ASCFSTAT_TXFFLMASK ; + status =readl (port->membase +LTQ_ASC_FSTAT ) &ASCFSTAT_TXFFLMASK ;return status ?0 :TIOCSER_TEMT ; }
@@ -315,12 +315,12 @@ lqasc_startup(struct uart_port *port)asc_update_bits (ASCCLC_DISS |ASCCLC_RMCMASK , (1 <<ASCCLC_RMCOFFSET ), port->membase +LTQ_ASC_CLC ); -ltq_w32 (0 , port->membase +LTQ_ASC_PISEL ); -ltq_w32 ( +writel (0 , port->membase +LTQ_ASC_PISEL ); +writel ( ((TXFIFO_FL <<ASCTXFCON_TXFITLOFF ) &ASCTXFCON_TXFITLMASK ) |ASCTXFCON_TXFEN |ASCTXFCON_TXFFLU , port->membase +LTQ_ASC_TXFCON ); -ltq_w32 ( +writel ( ((RXFIFO_FL <<ASCRXFCON_RXFITLOFF ) &ASCRXFCON_RXFITLMASK ) |ASCRXFCON_RXFEN |ASCRXFCON_RXFFLU , port->membase +LTQ_ASC_RXFCON );
@@ -352,7 +352,7 @@ lqasc_startup(struct uart_port *port) goto err2; } -ltq_w32 (ASC_IRNREN_RX |ASC_IRNREN_ERR |ASC_IRNREN_TX , +writel (ASC_IRNREN_RX |ASC_IRNREN_ERR |ASC_IRNREN_TX , port->membase +LTQ_ASC_IRNREN );return 0 ;
@@ -371,7 +371,7 @@ lqasc_shutdown(struct uart_port *port)free_irq (ltq_port->rx_irq, port);free_irq (ltq_port->err_irq, port); -ltq_w32 (0 , port->membase +LTQ_ASC_CON ); +writel (0 , port->membase +LTQ_ASC_CON );asc_update_bits (ASCRXFCON_RXFEN ,ASCRXFCON_RXFFLU , port->membase +LTQ_ASC_RXFCON ); asc_update_bits(ASCTXFCON_TXFEN, ASCTXFCON_TXFFLU,
@@ -463,13 +463,13 @@ lqasc_set_termios(struct uart_port *port,asc_update_bits (ASCCON_BRS ,0 , port->membase +LTQ_ASC_CON );/* now we can write the new baudrate into the register */ -ltq_w32 (divisor, port->membase +LTQ_ASC_BG ); +writel (divisor, port->membase +LTQ_ASC_BG );/* turn the baudrate generator back on */ asc_update_bits (0 ,ASCCON_R , port->membase +LTQ_ASC_CON );/* enable rx */ -ltq_w32 (ASCWHBSTATE_SETREN , port->membase +LTQ_ASC_WHBSTATE ); +writel (ASCWHBSTATE_SETREN , port->membase +LTQ_ASC_WHBSTATE );spin_unlock_irqrestore (<q_asc_lock, flags);
@@ -580,10 +580,10 @@ lqasc_console_putchar(struct uart_port *port, int ch)return ;do { - fifofree = (ltq_r32 (port->membase +LTQ_ASC_FSTAT ) + fifofree = (readl (port->membase +LTQ_ASC_FSTAT ) &ASCFSTAT_TXFREEMASK ) >>ASCFSTAT_TXFREEOFF ; }while (fifofree ==0 ); -ltq_w8 (ch, port->membase +LTQ_ASC_TBUF ); +writeb (ch, port->membase +LTQ_ASC_TBUF ); } static void lqasc_serial_port_write(struct uart_port *port, const char *s,
Signature · 1 line
2.11.0
PATCH v2 12/18 serial: intel: Rename fpiclk to freqclk
In reply to Songjun WuRename fpiclk to freqclk.
---
Changes in v2: None
drivers/tty/serial/lantiq.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/tty/serial/lantiq.c +5 -5
@@ -106,7 +106,7 @@ static DEFINE_SPINLOCK(ltq_asc_lock);struct ltq_uart_port {struct uart_port port;/* clock used to derive divider */ -struct clk *fpiclk; +struct clk *freqclk;/* clock gating of the ASC core */ struct clk *clk;unsigned int tx_irq;
@@ -310,7 +310,7 @@ lqasc_startup(struct uart_port *port)if (!IS_ERR (ltq_port->clk))clk_enable (ltq_port->clk); - port->uartclk =clk_get_rate (ltq_port->fpiclk); + port->uartclk =clk_get_rate (ltq_port->freqclk);asc_update_bits (ASCCLC_DISS |ASCCLC_RMCMASK , (1 <<ASCCLC_RMCOFFSET ), port->membase +LTQ_ASC_CLC );
@@ -633,7 +633,7 @@ lqasc_console_setup(struct console *co, char *options)if (!IS_ERR (ltq_port->clk))clk_enable (ltq_port->clk); - port->uartclk =clk_get_rate (ltq_port->fpiclk); + port->uartclk =clk_get_rate (ltq_port->freqclk);if (options)uart_parse_options (options, &baud, &parity, &bits, &flow);
@@ -744,8 +744,8 @@ lqasc_probe(struct platform_device *pdev) port->irq = irqres[0 ].start; port->mapbase = mmres->start; - ltq_port->fpiclk =clk_get_fpi (); -if (IS_ERR (ltq_port->fpiclk)) { + ltq_port->freqclk =clk_get_fpi (); +if (IS_ERR (ltq_port->freqclk)) {pr_err ("failed to get fpi clk\n" );return -ENOENT ; }
Signature · 1 line
2.11.0
PATCH v2 13/18 serial: intel: Replace clk_enable/clk_disable with clk generic API
In reply to Songjun WuThe clk driver has introduced new clock APIs that replace the existing clk_enable and clk_disable.
-clk_enable() APIs is replaced with clk_prepare_enable(). -clk_disable() API is replaced with clk_disable_unprepare().
---
Changes in v2: None
drivers/tty/serial/lantiq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/tty/serial/lantiq.c +3 -3
@@ -309,7 +309,7 @@ lqasc_startup(struct uart_port *port)int retval;if (!IS_ERR (ltq_port->clk)) -clk_enable (ltq_port->clk); +clk_prepare_enable (ltq_port->clk); port->uartclk =clk_get_rate (ltq_port->freqclk); asc_update_bits(ASCCLC_DISS | ASCCLC_RMCMASK, (1 << ASCCLC_RMCOFFSET),
@@ -377,7 +377,7 @@ lqasc_shutdown(struct uart_port *port)asc_update_bits (ASCTXFCON_TXFEN ,ASCTXFCON_TXFFLU , port->membase +LTQ_ASC_TXFCON );if (!IS_ERR (ltq_port->clk)) -clk_disable (ltq_port->clk); +clk_disable_unprepare (ltq_port->clk); } static void
@@ -631,7 +631,7 @@ lqasc_console_setup(struct console *co, char *options) port = <q_port->port;if (!IS_ERR (ltq_port->clk)) -clk_enable (ltq_port->clk); +clk_prepare_enable (ltq_port->clk); port->uartclk =clk_get_rate (ltq_port->freqclk);
Signature · 1 line
2.11.0
PATCH v2 14/18 serial: intel: Add CCF support
In reply to Songjun WuPrevious implementation uses platform-dependent API to get the clock. Those functions are not available for other SoC which uses the same IP. The CCF (Common Clock Framework) have an abstraction based APIs for clock. In future, the platform specific code will be removed when the legacy soc use CCF as well.
Change to use CCF APIs to get clock and rate. So that different SoCs can use the same driver.
---
Changes in v2: None
drivers/tty/serial/lantiq.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
drivers/tty/serial/lantiq.c +11 -0
@@ -26,7 +26,9 @@#include <linux/clk.h> #include <linux/gpio.h> +#ifdef CONFIG_LANTIQ #include <lantiq_soc.h> +#endif #define PORT_LTQ_ASC 111#define MAXPORTS 2
@@ -744,14 +746,23 @@ lqasc_probe(struct platform_device *pdev) port->irq = irqres[0 ].start; port->mapbase = mmres->start; +#if (IS_ENABLED (CONFIG_LANTIQ ) && !IS_ENABLED (CONFIG_COMMON_CLK )) ltq_port->freqclk =clk_get_fpi (); +#else + ltq_port->freqclk =devm_clk_get (&pdev->dev,"freq" ); +#endif +if (IS_ERR (ltq_port->freqclk)) {pr_err ("failed to get fpi clk\n" );return -ENOENT ; }/* not all asc ports have clock gates, lets ignore the return code */ +#if (IS_ENABLED (CONFIG_LANTIQ ) && !IS_ENABLED (CONFIG_COMMON_CLK )) ltq_port->clk =clk_get (&pdev->dev,NULL ); +#else + ltq_port->clk =devm_clk_get (&pdev->dev,"asc" ); +#endif ltq_port->tx_irq = irqres[0 ].start; ltq_port->rx_irq = irqres[1 ].start;
Signature · 1 line
2.11.0
Greg Kroah-Hartman
maintainerRe: [PATCH v2 14/18] serial: intel: Add CCF support
Quoted 21 lines · Songjun Wu
Previous implementation uses platform-dependent API to get the clock. Those functions are not available for other SoC which uses the same IP. The CCF (Common Clock Framework) have an abstraction based APIs for clock. In future, the platform specific code will be removed when the legacy soc use CCF as well.
Change to use CCF APIs to get clock and rate. So that different SoCs can use the same driver.Signed-off-by Songjun Wu <[email protected]>---
Changes in v2: None
drivers/tty/serial/lantiq.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/tty/serial/lantiq.c +0
-0
drivers/tty/serial/lantiq.c line 31
Quoted 6 lines
drivers/tty/serial/lantiq.c +2
-0@@ -26,7 +26,9 @@#include <linux/clk.h> #include <linux/gpio.h> +#ifdef CONFIG_LANTIQ #include <lantiq_soc.h> +#endif
That is never how you do this in Linux, you know better.
Please go and get this patchset reviewed and signed-off-by from other internal Intel kernel developers before resending it next time. It is their job to find and fix your basic errors like this, not ours.
greg k-h
Re: [PATCH v2 14/18] serial: intel: Add CCF support
Quoted 25 lines · Greg Kroah-Hartman
Quoted 24 lines · Songjun Wu
Previous implementation uses platform-dependent API to get the clock. Those functions are not available for other SoC which uses the same IP. The CCF (Common Clock Framework) have an abstraction based APIs for clock. In future, the platform specific code will be removed when the legacy soc use CCF as well.
Change to use CCF APIs to get clock and rate. So that different SoCs can use the same driver.Signed-off-by Songjun Wu <[email protected]>---
Changes in v2: None
drivers/tty/serial/lantiq.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/tty/serial/lantiq.c +0
-0@@ -26,7 +26,9 @@#include <linux/clk.h> #include <linux/gpio.h>
Quoted 8 lines
drivers/tty/serial/lantiq.c line 31
Quoted 3 lines
drivers/tty/serial/lantiq.c +2
-0+#ifdef CONFIG_LANTIQ #include <lantiq_soc.h> +#endif That is never how you do this in Linux, you know better.
Please go and get this patchset reviewed and signed-off-by from other internal Intel kernel developers before resending it next time. It is their job to find and fix your basic errors like this, not ours.
Thank you for your comment.
Actually, we have discussed this issue internally.
We put the reason why we use "#ifdef CONFIG_LANTIQ" preprocessor in commit message in "[PATCH v2 08/18] serial: intel: Get serial id from dts". Please refer the commit message below.
"#ifdef CONFIG_LANTIQ" preprocessor is used because LTQ_EARLY_ASC macro is defined in lantiq_soc.h.lantiq_soc.h is in arch path for legacy product support.
arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.h
If "#ifdef preprocessor" is changed to
"if (IS_ENABLED(CONFIG_LANTIQ))", when CONFIG_LANTIQ is not enabled, code using LTQ_EARLY_ASC is compiled.
Compilation will fail for no LTQ_EARLY_ASC defined.
Greg Kroah-Hartman
maintainerRe: [PATCH v2 14/18] serial: intel: Add CCF support
Quoted 45 lines · Songjun
Quoted 29 lines · Greg Kroah-Hartman
Quoted 21 lines · Songjun Wu
Previous implementation uses platform-dependent API to get the clock. Those functions are not available for other SoC which uses the same IP. The CCF (Common Clock Framework) have an abstraction based APIs for clock. In future, the platform specific code will be removed when the legacy soc use CCF as well.
Change to use CCF APIs to get clock and rate. So that different SoCs can use the same driver.Signed-off-by Songjun Wu <[email protected]>---
Changes in v2: None
drivers/tty/serial/lantiq.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/tty/serial/lantiq.c +0
-0drivers/tty/serial/lantiq.c line 30
Quoted 5 lines
drivers/tty/serial/lantiq.c +2
-0@@ -26,7 +26,9 @@#include <linux/clk.h> #include <linux/gpio.h> +#ifdef CONFIG_LANTIQ #include <lantiq_soc.h> +#endif That is never how you do this in Linux, you know better.
Quoted 3 lines
Please go and get this patchset reviewed and signed-off-by from other internal Intel kernel developers before resending it next time. It is their job to find and fix your basic errors like this, not ours.
Thank you for your comment.
Actually, we have discussed this issue internally.
We put the reason why we use "#ifdefCONFIG_LANTIQ" preprocessor in commit message in "[PATCH v2 08/18] serial: intel: Get serial id from dts". Please refer the commit message below."#ifdef
CONFIG_LANTIQ" preprocessor is used becauseLTQ_EARLY_ASCmacro is defined inlantiq_soc.h.lantiq_soc.his in arch path for legacy product support.
Quoted 6 lines
arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.hIf "#ifdef preprocessor" is changed to
"if (IS_ENABLED(CONFIG_LANTIQ))", whenCONFIG_LANTIQis not enabled, code usingLTQ_EARLY_ASCis compiled.
Compilation will fail for noLTQ_EARLY_ASCdefined.
Sorry, but no. Why is this one tiny driver/chip somehow more "special" than all of the tens of thousands of other devices we support to warrent it getting some sort of special exception to do things differently? What happens to the next device that wants to do it this way?
Our coding style and rules are there for a reason, do not violate them thinking your device is the only one that matters.
Do it properly, again, you all know better than this.
greg k-h
Re: [PATCH v2 14/18] serial: intel: Add CCF support
Quoted 58 lines · Greg Kroah-Hartman
Quoted 45 lines · Songjun
Quoted 29 lines · Greg Kroah-Hartman
Quoted 21 lines · Songjun Wu
Previous implementation uses platform-dependent API to get the clock. Those functions are not available for other SoC which uses the same IP. The CCF (Common Clock Framework) have an abstraction based APIs for clock. In future, the platform specific code will be removed when the legacy soc use CCF as well.
Change to use CCF APIs to get clock and rate. So that different SoCs can use the same driver.Signed-off-by Songjun Wu <[email protected]>---
Changes in v2: None
drivers/tty/serial/lantiq.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/tty/serial/lantiq.c +0
-0drivers/tty/serial/lantiq.c line 30
Quoted 5 lines
drivers/tty/serial/lantiq.c +2
-0@@ -26,7 +26,9 @@#include <linux/clk.h> #include <linux/gpio.h> +#ifdef CONFIG_LANTIQ #include <lantiq_soc.h> +#endif That is never how you do this in Linux, you know better.
Quoted 3 lines
Please go and get this patchset reviewed and signed-off-by from other internal Intel kernel developers before resending it next time. It is their job to find and fix your basic errors like this, not ours.
Thank you for your comment.
Actually, we have discussed this issue internally.
We put the reason why we use "#ifdefCONFIG_LANTIQ" preprocessor in commit message in "[PATCH v2 08/18] serial: intel: Get serial id from dts". Please refer the commit message below."#ifdef
CONFIG_LANTIQ" preprocessor is used becauseLTQ_EARLY_ASCmacro is defined inlantiq_soc.h.lantiq_soc.his in arch path for legacy product support.Quoted 6 lines
arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.hIf "#ifdef preprocessor" is changed to
"if (IS_ENABLED(CONFIG_LANTIQ))", whenCONFIG_LANTIQis not enabled, code usingLTQ_EARLY_ASCis compiled.
Compilation will fail for noLTQ_EARLY_ASCdefined.Sorry, but no. Why is this one tiny driver/chip somehow more "special" than all of the tens of thousands of other devices we support to warrent it getting some sort of special exception to do things differently? What happens to the next device that wants to do it this way?
Quoted 7 lines
Our coding style and rules are there for a reason, do not violate them thinking your device is the only one that matters.
Do it properly, again, you all know better than this.
greg k-h
Hi Greg,
The problem is that the Lantiq SoC code in arch/mips/lantiq does not use the common clock framework, but it uses the clk framework directly. It defines CONFIG_HAVE_CLK and CONFIG_CLKDEV_LOOKUP, but not CONFIG_COMMON_CLK. The xRX500 SoC which is being added here is about 2 generations more recent than the VR9/xRX200 SoC which is the latest which is supported by the code in arch/mips/lantiq. With this new SoC we switched to the common clock framework. This driver is used by the older SoC and also by the new ones because this IP core is pretty similar in all the SoCs.
This patch makes it possible to use it with the legacy lantiq code and also with the common clock framework. I see multiple options to fix this problem.
1. The current approach to have it as a compile variant for a) legacy lantiq arch code without common clock framework and b) support for SoCs using the common clock framework.
2. Convert the lantiq arch code to the common clock framework. This would be a good approach, but it need some efforts.
3. Remove the arch/mips/lantiq code. There are still users of this code.
4. Use the old APIs also for the new xRX500 SoC, I do not like this approach.
5. Move lantiq_soc.h to somewhere in include/linux/ so it is globally available and provide some better wrapper code.
Hauke
Greg Kroah-Hartman
maintainerRe: [PATCH v2 14/18] serial: intel: Add CCF support
Quoted 81 lines · Hauke Mehrtens
Quoted 58 lines · Greg Kroah-Hartman
Quoted 45 lines · Songjun
Quoted 29 lines · Greg Kroah-Hartman
Quoted 21 lines · Songjun Wu
Previous implementation uses platform-dependent API to get the clock. Those functions are not available for other SoC which uses the same IP. The CCF (Common Clock Framework) have an abstraction based APIs for clock. In future, the platform specific code will be removed when the legacy soc use CCF as well.
Change to use CCF APIs to get clock and rate. So that different SoCs can use the same driver.Signed-off-by Songjun Wu <[email protected]>---
Changes in v2: None
drivers/tty/serial/lantiq.c | 11 +++++++++++ 1 file changed, 11 insertions(+)drivers/tty/serial/lantiq.c +0
-0drivers/tty/serial/lantiq.c line 30
Quoted 5 lines
drivers/tty/serial/lantiq.c +2
-0@@ -26,7 +26,9 @@#include <linux/clk.h> #include <linux/gpio.h> +#ifdef CONFIG_LANTIQ #include <lantiq_soc.h> +#endif That is never how you do this in Linux, you know better.
Quoted 3 lines
Please go and get this patchset reviewed and signed-off-by from other internal Intel kernel developers before resending it next time. It is their job to find and fix your basic errors like this, not ours.
Thank you for your comment.
Actually, we have discussed this issue internally.
We put the reason why we use "#ifdefCONFIG_LANTIQ" preprocessor in commit message in "[PATCH v2 08/18] serial: intel: Get serial id from dts". Please refer the commit message below."#ifdef
CONFIG_LANTIQ" preprocessor is used becauseLTQ_EARLY_ASCmacro is defined inlantiq_soc.h.lantiq_soc.his in arch path for legacy product support.Quoted 6 lines
arch/mips/include/asm/mach-lantiq/xway/lantiq_soc.hIf "#ifdef preprocessor" is changed to
"if (IS_ENABLED(CONFIG_LANTIQ))", whenCONFIG_LANTIQis not enabled, code usingLTQ_EARLY_ASCis compiled.
Compilation will fail for noLTQ_EARLY_ASCdefined.Sorry, but no. Why is this one tiny driver/chip somehow more "special" than all of the tens of thousands of other devices we support to warrent it getting some sort of special exception to do things differently? What happens to the next device that wants to do it this way?
Quoted 7 lines
Our coding style and rules are there for a reason, do not violate them thinking your device is the only one that matters.
Do it properly, again, you all know better than this.
greg k-h
Hi Greg,
The problem is that the Lantiq SoC code in
arch/mips/lantiqdoes not use the common clock framework, but it uses the clk framework directly. It definesCONFIG_HAVE_CLKandCONFIG_CLKDEV_LOOKUP, but notCONFIG_COMMON_CLK. The xRX500 SoC which is being added here is about 2 generations more recent than the VR9/xRX200 SoC which is the latest which is supported by the code inarch/mips/lantiq. With this new SoC we switched to the common clock framework. This driver is used by the older SoC and also by the new ones because this IP core is pretty similar in all the SoCs.
This patch makes it possible to use it with the legacy lantiq code and also with the common clock framework. I see multiple options to fix this problem.
Quoted 10 lines
1. The current approach to have it as a compile variant for a) legacy lantiq arch code without common clock framework and b) support for SoCs using the common clock framework.
2. Convert the lantiq arch code to the common clock framework. This would be a good approach, but it need some efforts.
3. Remove thearch/mips/lantiqcode. There are still users of this code.
4. Use the old APIs also for the new xRX500 SoC, I do not like this approach.
5. Movelantiq_soc.hto somewhere ininclude/linux/so it is globally available and provide some better wrapper code.
I don't really care what you do at this point in time, but you all should know better than the crazy #ifdef is not allowed to try to prevent/allow the inclusion of a .h file. Checkpatch might have even warned you about it, right?
So do it correctly, odds are #5 is correct, as that makes it work like any other device in the kernel. You are not unique here.
greg k-h
Re: [PATCH v2 14/18] serial: intel: Add CCF support
Quoted 19 lines · Greg Kroah-Hartman
Quoted 7 lines · Hauke Mehrtens
Quoted 1 line · Greg Kroah-Hartman
On Fri, Aug 03, 2018 at 03:33:38PM +0800, Wu, Songjun wrote:
This patch makes it possible to use it with the legacy lantiq code and also with the common clock framework. I see multiple options to fix this problem.
Quoted 10 lines
1. The current approach to have it as a compile variant for a) legacy lantiq arch code without common clock framework and b) support for SoCs using the common clock framework.
2. Convert the lantiq arch code to the common clock framework. This would be a good approach, but it need some efforts.
3. Remove thearch/mips/lantiqcode. There are still users of this code.
4. Use the old APIs also for the new xRX500 SoC, I do not like this approach.
5. Movelantiq_soc.hto somewhere ininclude/linux/so it is globally available and provide some better wrapper code.
Quoted 7 lines
I don't really care what you do at this point in time, but you all should know better than the crazy #ifdef is not allowed to try to prevent/allow the inclusion of a .h file. Checkpatch might have even warned you about it, right?
So do it correctly, odds are #5 is correct, as that makes it work like any other device in the kernel. You are not unique here.
The best approach here would clearly be 2. We don't want platform specific header files for doing things that should be completely generic.
Converting lantiq to the common-clk framework obviously requires some work, but then again the whole arch/mips/lantiq/clk.c file is fairly short and maybe not that hard to convert.
Quoted 1 line
From looking at
arch/mips/lantiq/xway/sysctrl.c, it appears that you
already use the clkdev lookup mechanism for some devices without using COMMON_CLK, so I would assume that you can also use those for the remaining clks, which would be much simpler. It registers one anonymous clk there as
clkdev_add_pmu("1e100c00.serial", NULL, 0, 0, PMU_ASC1);so why not add replace that with two named clocks and just use the same names in the DT for the newer chip?
Arnd
Re: [PATCH v2 14/18] serial: intel: Add CCF support
Quoted 39 lines · Arnd Bergmann
Quoted 17 lines · Greg Kroah-Hartman
Quoted 6 lines · Hauke Mehrtens
Quoted 1 line · Greg Kroah-Hartman
On Fri, Aug 03, 2018 at 03:33:38PM +0800, Wu, Songjun wrote:
This patch makes it possible to use it with the legacy lantiq code and also with the common clock framework. I see multiple options to fix this problem.
Quoted 10 lines
1. The current approach to have it as a compile variant for a) legacy lantiq arch code without common clock framework and b) support for SoCs using the common clock framework.
2. Convert the lantiq arch code to the common clock framework. This would be a good approach, but it need some efforts.
3. Remove thearch/mips/lantiqcode. There are still users of this code.
4. Use the old APIs also for the new xRX500 SoC, I do not like this approach.
5. Movelantiq_soc.hto somewhere ininclude/linux/so it is globally available and provide some better wrapper code.Quoted 7 lines
I don't really care what you do at this point in time, but you all should know better than the crazy #ifdef is not allowed to try to prevent/allow the inclusion of a .h file. Checkpatch might have even warned you about it, right?
So do it correctly, odds are #5 is correct, as that makes it work like any other device in the kernel. You are not unique here.
The best approach here would clearly be 2. We don't want platform specific header files for doing things that should be completely generic.
Converting lantiq to the common-clk framework obviously requires some work, but then again the whole
arch/mips/lantiq/clk.cfile is fairly short and maybe not that hard to convert.Quoted 1 line
From looking at
arch/mips/lantiq/xway/sysctrl.c, it appears that youalready use the clkdev lookup mechanism for some devices without using
COMMON_CLK, so I would assume that you can also use those for the remaining clks, which would be much simpler. It registers one anonymous clk there as
Quoted 6 lines
clkdev_add_pmu("1e100c00.serial", NULL, 0, 0, PMU_ASC1);so why not add replace that with two named clocks and just use the same names in the DT for the newer chip?
Arnd
We discussed internally and have another solution for this issue. Add one lantiq.h in the serial folder, and use "#ifdef preprocessor" in lantiq.h, also providing no-op stub functions in the #else case, then call those functions unconditionally from lantiq.c to avoid #ifdef in C file.
To support CCF in legacy product is another topic, is not included in this patch.
The implementation is as following：
#ifdef CONFIG_LANTIQ
#include <lantiq_soc.h>
#else
#define LTQ_EARLY_ASC 0
#define CPHYSADDR(_val) 0
static inline struct clk *clk_get_fpi(void)
{
return NULL;
}
#endif
Re: [PATCH v2 14/18] serial: intel: Add CCF support
Hi Songjun,
Quoted 63 lines · Songjun
Quoted 39 lines · Arnd Bergmann
Quoted 17 lines · Greg Kroah-Hartman
Quoted 6 lines · Hauke Mehrtens
Quoted 1 line · Greg Kroah-Hartman
On Fri, Aug 03, 2018 at 03:33:38PM +0800, Wu, Songjun wrote:
This patch makes it possible to use it with the legacy lantiq code and also with the common clock framework. I see multiple options to fix this problem.
Quoted 10 lines
1. The current approach to have it as a compile variant for a) legacy lantiq arch code without common clock framework and b) support for SoCs using the common clock framework.
2. Convert the lantiq arch code to the common clock framework. This would be a good approach, but it need some efforts.
3. Remove thearch/mips/lantiqcode. There are still users of this code.
4. Use the old APIs also for the new xRX500 SoC, I do not like this approach.
5. Movelantiq_soc.hto somewhere ininclude/linux/so it is globally available and provide some better wrapper code.Quoted 7 lines
I don't really care what you do at this point in time, but you all should know better than the crazy #ifdef is not allowed to try to prevent/allow the inclusion of a .h file. Checkpatch might have even warned you about it, right?
So do it correctly, odds are #5 is correct, as that makes it work like any other device in the kernel. You are not unique here.
The best approach here would clearly be 2. We don't want platform specific header files for doing things that should be completely generic.
Converting lantiq to the common-clk framework obviously requires some work, but then again the whole
arch/mips/lantiq/clk.cfile is fairly short and maybe not that hard to convert.Quoted 1 line
From looking at
arch/mips/lantiq/xway/sysctrl.c, it appears that youalready use the clkdev lookup mechanism for some devices without using
COMMON_CLK, so I would assume that you can also use those for the remaining clks, which would be much simpler. It registers one anonymous clk there asQuoted 6 lines
clkdev_add_pmu("1e100c00.serial", NULL, 0, 0, PMU_ASC1);so why not add replace that with two named clocks and just use the same names in the DT for the newer chip?
Arnd
We discussed internally and have another solution for this issue. Add one
lantiq.hin the serial folder, and use "#ifdef preprocessor" inlantiq.h,
also providing no-op stub functions in the #else case, then call those functions
unconditionally fromlantiq.cto avoid #ifdef in C file.To support CCF in legacy product is another topic, is not included in this patch.
The implementation is as following：
#ifdefCONFIG_LANTIQ
#include <lantiq_soc.h>
#else
#defineLTQ_EARLY_ASC0
#defineCPHYSADDR(_val)0
Quoted 5 lines
static inline struct clk
*clk_get_fpi(void){ return NULL; }#endif
Why not use clkdev_add(), as Arnd suggested?
That would be a 3-line patch without introducing a new header file and an ugly #ifdef, which complicates compile coverage testing?
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v2 14/18] serial: intel: Add CCF support
Quoted 71 lines · Geert Uytterhoeven
Hi Songjun,
Quoted 63 lines · Songjun
Quoted 39 lines · Arnd Bergmann
Quoted 17 lines · Greg Kroah-Hartman
Quoted 6 lines · Hauke Mehrtens
Quoted 1 line · Greg Kroah-Hartman
On Fri, Aug 03, 2018 at 03:33:38PM +0800, Wu, Songjun wrote:
This patch makes it possible to use it with the legacy lantiq code and also with the common clock framework. I see multiple options to fix this problem.
Quoted 10 lines
1. The current approach to have it as a compile variant for a) legacy lantiq arch code without common clock framework and b) support for SoCs using the common clock framework.
2. Convert the lantiq arch code to the common clock framework. This would be a good approach, but it need some efforts.
3. Remove thearch/mips/lantiqcode. There are still users of this code.
4. Use the old APIs also for the new xRX500 SoC, I do not like this approach.
5. Movelantiq_soc.hto somewhere ininclude/linux/so it is globally available and provide some better wrapper code.Quoted 7 lines
I don't really care what you do at this point in time, but you all should know better than the crazy #ifdef is not allowed to try to prevent/allow the inclusion of a .h file. Checkpatch might have even warned you about it, right?
So do it correctly, odds are #5 is correct, as that makes it work like any other device in the kernel. You are not unique here.
The best approach here would clearly be 2. We don't want platform specific header files for doing things that should be completely generic.
Converting lantiq to the common-clk framework obviously requires some work, but then again the whole
arch/mips/lantiq/clk.cfile is fairly short and maybe not that hard to convert.Quoted 1 line
From looking at
arch/mips/lantiq/xway/sysctrl.c, it appears that youalready use the clkdev lookup mechanism for some devices without using
COMMON_CLK, so I would assume that you can also use those for the remaining clks, which would be much simpler. It registers one anonymous clk there asQuoted 6 lines
clkdev_add_pmu("1e100c00.serial", NULL, 0, 0, PMU_ASC1);so why not add replace that with two named clocks and just use the same names in the DT for the newer chip?
Arnd
We discussed internally and have another solution for this issue. Add one
lantiq.hin the serial folder, and use "#ifdef preprocessor" inlantiq.h,
also providing no-op stub functions in the #else case, then call those functions
unconditionally fromlantiq.cto avoid #ifdef in C file.To support CCF in legacy product is another topic, is not included in this patch.
The implementation is as following：
#ifdefCONFIG_LANTIQ
#include <lantiq_soc.h>
#else
#defineLTQ_EARLY_ASC0
#defineCPHYSADDR(_val)0Quoted 5 lines
static inline struct clk
*clk_get_fpi(void){ return NULL; }#endif
Quoted 7 lines
Why not use
clkdev_add(), as Arnd suggested?
That would be a 3-line patch without introducing a new header file and an ugly #ifdef, which complicates compile coverage testing?Gr{oetje,eeting}s,
Geert
The reason we add a new head file is also for two macros(LTQ_EARLY_ASC and CPHYSADDR) used by legacy product. We need to provide the no-op stub for these two macro for new product.
Re: [PATCH v2 14/18] serial: intel: Add CCF support
Hi Songjun,
Quoted 70 lines · Songjun
Quoted 69 lines · Geert Uytterhoeven
Quoted 68 lines · Songjun
Quoted 39 lines · Arnd Bergmann
Quoted 17 lines · Greg Kroah-Hartman
Quoted 6 lines · Hauke Mehrtens
Quoted 1 line · Greg Kroah-Hartman
On Fri, Aug 03, 2018 at 03:33:38PM +0800, Wu, Songjun wrote:
This patch makes it possible to use it with the legacy lantiq code and also with the common clock framework. I see multiple options to fix this problem.
Quoted 10 lines
1. The current approach to have it as a compile variant for a) legacy lantiq arch code without common clock framework and b) support for SoCs using the common clock framework.
2. Convert the lantiq arch code to the common clock framework. This would be a good approach, but it need some efforts.
3. Remove thearch/mips/lantiqcode. There are still users of this code.
4. Use the old APIs also for the new xRX500 SoC, I do not like this approach.
5. Movelantiq_soc.hto somewhere ininclude/linux/so it is globally available and provide some better wrapper code.Quoted 7 lines
I don't really care what you do at this point in time, but you all should know better than the crazy #ifdef is not allowed to try to prevent/allow the inclusion of a .h file. Checkpatch might have even warned you about it, right?
So do it correctly, odds are #5 is correct, as that makes it work like any other device in the kernel. You are not unique here.
The best approach here would clearly be 2. We don't want platform specific header files for doing things that should be completely generic.
Converting lantiq to the common-clk framework obviously requires some work, but then again the whole
arch/mips/lantiq/clk.cfile is fairly short and maybe not that hard to convert.Quoted 1 line
From looking at
arch/mips/lantiq/xway/sysctrl.c, it appears that youalready use the clkdev lookup mechanism for some devices without using
COMMON_CLK, so I would assume that you can also use those for the remaining clks, which would be much simpler. It registers one anonymous clk there asQuoted 6 lines
clkdev_add_pmu("1e100c00.serial", NULL, 0, 0, PMU_ASC1);so why not add replace that with two named clocks and just use the same names in the DT for the newer chip?
Arnd
We discussed internally and have another solution for this issue. Add one
lantiq.hin the serial folder, and use "#ifdef preprocessor" inlantiq.h,
also providing no-op stub functions in the #else case, then call those functions
unconditionally fromlantiq.cto avoid #ifdef in C file.To support CCF in legacy product is another topic, is not included in this patch.
The implementation is as following：
#ifdefCONFIG_LANTIQ
#include <lantiq_soc.h>
#else
#defineLTQ_EARLY_ASC0
#defineCPHYSADDR(_val)0static inline struct clk
*clk_get_fpi(void){ return NULL; }#endif
Quoted 8 lines
Quoted 4 lines
Why not use
clkdev_add(), as Arnd suggested?
That would be a 3-line patch without introducing a new header file and an ugly #ifdef, which complicates compile coverage testing?The reason we add a new head file is also for two
macros(LTQ_EARLY_ASC and CPHYSADDR)
used by legacy product. We need to provide the no-op stub for these two macro for new product.
No you don't. The line number should not be obtained by comparing the resource address with a hardcoded base address.
Perhaps the override of port->line should just be removed, as IIRC, the serial core has already filled in that field with the (next available) line number?
Gr{oetje,eeting}s,
Geert
Signature · 5 lines
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected] In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Re: [PATCH v2 14/18] serial: intel: Add CCF support
Quoted 77 lines · Geert Uytterhoeven
Hi Songjun,
Quoted 74 lines · Songjun
Quoted 73 lines · Geert Uytterhoeven
Quoted 63 lines · Songjun
Quoted 39 lines · Arnd Bergmann
Quoted 17 lines · Greg Kroah-Hartman
Quoted 6 lines · Hauke Mehrtens
Quoted 1 line · Greg Kroah-Hartman
On Fri, Aug 03, 2018 at 03:33:38PM +0800, Wu, Songjun wrote:
This patch makes it possible to use it with the legacy lantiq code and also with the common clock framework. I see multiple options to fix this problem.
Quoted 10 lines
1. The current approach to have it as a compile variant for a) legacy lantiq arch code without common clock framework and b) support for SoCs using the common clock framework.
2. Convert the lantiq arch code to the common clock framework. This would be a good approach, but it need some efforts.
3. Remove thearch/mips/lantiqcode. There are still users of this code.
4. Use the old APIs also for the new xRX500 SoC, I do not like this approach.
5. Movelantiq_soc.hto somewhere ininclude/linux/so it is globally available and provide some better wrapper code.Quoted 7 lines
I don't really care what you do at this point in time, but you all should know better than the crazy #ifdef is not allowed to try to prevent/allow the inclusion of a .h file. Checkpatch might have even warned you about it, right?
So do it correctly, odds are #5 is correct, as that makes it work like any other device in the kernel. You are not unique here.
The best approach here would clearly be 2. We don't want platform specific header files for doing things that should be completely generic.
Converting lantiq to the common-clk framework obviously requires some work, but then again the whole
arch/mips/lantiq/clk.cfile is fairly short and maybe not that hard to convert.Quoted 1 line
From looking at
arch/mips/lantiq/xway/sysctrl.c, it appears that youalready use the clkdev lookup mechanism for some devices without using
COMMON_CLK, so I would assume that you can also use those for the remaining clks, which would be much simpler. It registers one anonymous clk there asQuoted 6 lines
clkdev_add_pmu("1e100c00.serial", NULL, 0, 0, PMU_ASC1);so why not add replace that with two named clocks and just use the same names in the DT for the newer chip?
Arnd
We discussed internally and have another solution for this issue. Add one
lantiq.hin the serial folder, and use "#ifdef preprocessor" inlantiq.h,
also providing no-op stub functions in the #else case, then call those functions
unconditionally fromlantiq.cto avoid #ifdef in C file.To support CCF in legacy product is another topic, is not included in this patch.
The implementation is as following：
#ifdefCONFIG_LANTIQ
#include <lantiq_soc.h>
#else
#defineLTQ_EARLY_ASC0
#defineCPHYSADDR(_val)0Quoted 5 lines
static inline struct clk
*clk_get_fpi(void){ return NULL; }#endif
Why not use
clkdev_add(), as Arnd suggested?
That would be a 3-line patch without introducing a new header file and an ugly #ifdef, which complicates compile coverage testing?
Quoted 6 lines
Quoted 4 lines
The reason we add a new head file is also for two
macros(LTQ_EARLY_ASC and CPHYSADDR)
used by legacy product. We need to provide the no-op stub for these two macro for new product.No you don't. The line number should not be obtained by comparing the resource address with a hardcoded base address.
This is the previous code. Now the line number is obtained from dts. We keep this code for the compatibility.
Referring to the conditional-compilation part in coding-style, We add a header file to avoid using “#ifdef” in C file.
Quoted 6 lines
Perhaps the override of
port->lineshould just be removed, as IIRC, the serial core has already filled in that field with the (next available) line number?Gr{oetje,eeting}s,
Geert
Re: [PATCH v2 14/18] serial: intel: Add CCF support
Hi Songjun,
Quoted 9 lines · Songjun
Quoted 7 lines · Geert Uytterhoeven
Quoted 4 lines · Songjun
The reason we add a new head file is also for two
macros(LTQ_EARLY_ASC and CPHYSADDR)
used by legacy product. We need to provide the no-op stub for these two macro for new product.No you don't. The line number should not be obtained by comparing the resource address with a hardcoded base address.
This is the previous code. Now the line number is obtained from dts.
Note that obtaining line numbers from DTS has its own share of problems, when considering DT overlays. I've replied to the patch adding the call to of_alias_get_id().
Quoted 6 lines
We keep this code for the compatibility.
Referring to the conditional-compilation part in coding-style, We add a header file to avoid using “#ifdef” in C file.
Quoted 2 lines
Perhaps the override of
port->lineshould just be removed, as IIRC, the serial core has already filled in that field with the (next available) line number?
Gr{oetje,eeting}s,
Geert--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
PATCH v2 15/18 serial: intel: Support more platform
In reply to Songjun WuSupport more platform.
---
Changes in v2: None
drivers/tty/serial/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/Kconfig +1 -1
@@ -1062,7 +1062,7 @@ config SERIAL_OMAP_CONSOLE config SERIAL_LANTIQ bool "Lantiq serial driver" - depends on LANTIQ + depends on LANTIQ || INTEL_MIPS || COMPILE_TEST select SERIAL_CORE select SERIAL_CORE_CONSOLE select SERIAL_EARLYCON
Signature · 1 line
2.11.0
Greg Kroah-Hartman
maintainerRe: [PATCH v2 15/18] serial: intel: Support more platform
Quoted 4 lines · Songjun Wu
Support more platform.
Signed-off-by Songjun Wu <[email protected]>---
Your changelog text makes no sense, sorry.
greg k-h
Re: [PATCH v2 15/18] serial: intel: Support more platform
Quoted 6 lines · Greg Kroah-Hartman
Quoted 4 lines · Songjun Wu
Support more platform.
Signed-off-by Songjun Wu <[email protected]>---
Your changelog text makes no sense, sorry.
Thanks for your comment.
I will describe it more clearly.
Re: [PATCH v2 15/18] serial: intel: Support more platform
In reply to Songjun WuThe subject line also seems odd, your are changing deps on the lantiq driver, not some (nonexistent) intel serial driver.
Re: [PATCH v2 15/18] serial: intel: Support more platform
Quoted 3 lines · Christoph Hellwig
The subject line also seems odd, your are changing deps on the lantiq driver, not some (nonexistent) intel serial driver.
Your suggestion is reasonable, it will be changed to "serial: lantiq".
PATCH v2 16/18 serial: intel: Reorder the head files
In reply to Songjun WuReorder the head files according to the coding style.
---
Changes in v2:
- New patch to reorder the head files according to the coding style.
drivers/tty/serial/lantiq.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
drivers/tty/serial/lantiq.c +12 -12
@@ -9,22 +9,22 @@* Copyright (C) 2018 Intel Corporation. */ -#include <linux/slab.h> -#include <linux/ioport.h> -#include <linux/init.h> +#include <linux/clk.h> #include <linux/console.h> -#include <linux/sysrq.h> #include <linux/device.h> -#include <linux/tty.h> -#include <linux/tty_flip.h> -#include <linux/serial_core.h> -#include <linux/serial.h> -#include <linux/of_platform.h> +#include <linux/gpio.h> +#include <linux/init.h> +#include <linux/io.h> +#include <linux/ioport.h> #include <linux/of_address.h> #include <linux/of_irq.h> -#include <linux/io.h> -#include <linux/clk.h> -#include <linux/gpio.h> +#include <linux/of_platform.h> +#include <linux/serial.h> +#include <linux/serial_core.h> +#include <linux/slab.h> +#include <linux/sysrq.h> +#include <linux/tty.h> +#include <linux/tty_flip.h> #ifdef CONFIG_LANTIQ #include <lantiq_soc.h>
Signature · 1 line
2.11.0
PATCH v2 17/18 serial: intel: Change init_lqasc to static declaration
In reply to Songjun Wuinit_lqasc() is only used internally, change to static declaration.
---
Changes in v2: None
drivers/tty/serial/lantiq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/tty/serial/lantiq.c +1 -1
@@ -788,7 +788,7 @@ static struct platform_driver lqasc_driver = { }, }; -int __init +static int __init init_lqasc (void ) {int ret;
Signature · 1 line
2.11.0
PATCH v2 18/18 dt-bindings: serial: lantiq: Add optional properties for CCF
In reply to Songjun WuClocks and clock-names are updated in device tree binding.
---
Changes in v2: None
Documentation/devicetree/bindings/serial/lantiq_asc.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
Documentation/devicetree/bindings/serial/lantiq_asc.txt +15 -0
@@ -6,8 +6,23 @@ Required properties: - interrupts: the 3 (tx rx err) interrupt numbers. The interrupt specifier depends on the interrupt-parent interrupt controller. +Optional properties: +- clocks: Should contain frequency clock and gate clock +- clock-names: Should be "freq" and "asc" + Example: +asc0: serial@16600000 { + compatible = "lantiq,asc"; + reg = <0x16600000 0x100000>; + interrupt-parent = <&gic>; + interrupts = <GIC_SHARED 103 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 105 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SHARED 106 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cgu CLK_SSX4>, <&cgu GCLK_UART>; + clock-names = "freq", "asc"; +}; + asc1: serial@e100c00 { compatible = "lantiq,asc"; reg = <0xE100C00 0x400>;
Signature · 1 line
2.11.0
Re: [PATCH v2 18/18] dt-bindings: serial: lantiq: Add optional properties for CCF
Quoted 9 lines · Songjun Wu
Clocks and clock-names are updated in device tree binding.
Signed-off-by Songjun Wu <[email protected]>---
Changes in v2: None
Documentation/devicetree/bindings/serial/lantiq_asc.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+)