From mboxrd@z Thu Jan 1 00:00:00 1970 From: vw@iommu.org (Wan Zongshun) Date: Tue, 5 Jul 2016 15:38:23 +0800 Subject: [PATCH 1/6] ARM: NUC900: Add nuc970 machine support In-Reply-To: <4806020.41AJ9s5VGu@wuerfel> References: <1466851042-22239-1-git-send-email-vw@iommu.org> <1466851042-22239-2-git-send-email-vw@iommu.org> <4806020.41AJ9s5VGu@wuerfel> Message-ID: <577B63EF.6030906@iommu.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2016?06?29? 23:19, Arnd Bergmann wrote: > On Saturday, June 25, 2016 6:37:17 PM CEST Wan Zongshun wrote: >> NUC970 is a new SoC of Nuvoton nuc900 series, this patch is >> to add machine file support for it. >> >> Signed-off-by: Wan Zongshun > > Nice to see some activity on the port! > >> --- >> arch/arm/mach-w90x900/Kconfig | 25 ++++ >> arch/arm/mach-w90x900/Makefile | 3 + >> .../mach-w90x900/include/mach/nuc970-regs-gcr.h | 56 ++++++++ >> arch/arm/mach-w90x900/mach-nuc970.c | 144 +++++++++++++++++++++ >> 4 files changed, 228 insertions(+) >> create mode 100644 arch/arm/mach-w90x900/include/mach/nuc970-regs-gcr.h >> create mode 100644 arch/arm/mach-w90x900/mach-nuc970.c >> >> diff --git a/arch/arm/mach-w90x900/Kconfig b/arch/arm/mach-w90x900/Kconfig >> index 69bab32..050833e 100644 >> --- a/arch/arm/mach-w90x900/Kconfig >> +++ b/arch/arm/mach-w90x900/Kconfig >> @@ -15,6 +15,21 @@ config CPU_NUC960 >> help >> Support for NUCP960 of Nuvoton NUC900 CPUs. >> >> +config SOC_NUC970 >> + bool >> + select GENERIC_IRQ_CHIP >> + select SOC_BUS >> + select IRQ_DOMAIN >> + select MULTI_IRQ_HANDLER >> + select USE_OF >> + select HAVE_CLK_PREPARE >> + select HAVE_MACH_CLKDEV >> + select COMMON_CLK >> + select NUC900_TIMER >> + help >> + Support for NUCP970 of Nuvoton NUC900 CPUs. >> + > > [style] This looks whitespace damaged, and please sort the line alphabetically. Sure, changed. > > I see you have done this in a way that is basically compatible with > CONFIG_ARCH_MULTIPLATFORM, good. > > What is HAVE_MACH_CLKDEV for? > >> @@ -46,4 +61,14 @@ config MACH_W90N960EVB >> >> endmenu >> >> +menu "NUC970 Machines" >> + >> +config MACH_NUC970EVB >> + bool "Nuvoton NUC970 Evaluation Board" >> + select SOC_NUC970 >> + help >> + Say Y here if you are using the Nuvoton NUC970EVB >> + >> +endmenu > > I'd leave out this entry, with the way have have structured the code (correctly), > there is no need to separate SoC-specific code from board specific code, since > they are the same. > Sure, removed it. >> diff --git a/arch/arm/mach-w90x900/include/mach/nuc970-regs-gcr.h b/arch/arm/mach-w90x900/include/mach/nuc970-regs-gcr.h >> new file mode 100644 >> index 0000000..e7eb653 >> --- /dev/null >> +++ b/arch/arm/mach-w90x900/include/mach/nuc970-regs-gcr.h > > Can you move the new headers to arch/arm/mach-w90x900/ directly? > >> +static int __init nuc900_restart_init(void) >> +{ >> + struct device_node *np; >> + >> + np = of_find_compatible_node(NULL, NULL, "nuvoton,gcr"); >> + wtcr_addr = of_iomap(np, 0); >> + if (!wtcr_addr) >> + return -ENODEV; >> + >> + of_node_put(np); >> + >> + return 0; >> +} > > Is this a watchdog node? If it is, the restart logic should just > move into the watchdog driver. It is not watchdog node, just be global System control register node. > > >> + if (of_machine_is_compatible("nuvoton,nuc970evb")) >> + nuc970_init(); > > What is this for? Currently, no used, remove it. > >> + of_platform_populate(NULL, of_default_bus_match_table, NULL, parent); > > We have actually moved away from using the soc_device as using the parent > for the other devices, just probe them separately. In fact the soc_device > could be handled by a driver in drivers/soc/nuvoton/ Do you think I should add nuc900 soc driver in this folder? If I want to add nuc900 soc driver in drivers/soc/nuvoton/, can I keep my current dts structure no change, or Must I add a new node name soc {}? I went through the code:soc-realview.c for reference, but I have no idea about how to re-structure my dts file to match this type soc driver. > >> +static const char *nuc970_dt_compat[] __initconst = { >> + "nuvoton,nuc970evb", >> + NULL, >> +}; >> + >> +void nuc970_restart(enum reboot_mode mode, const char *cmd) >> +{ >> + if (wtcr_addr) { >> + while (__raw_readl(wtcr_addr + REG_WRPRTR) != 1) { >> + __raw_writel(0x59, wtcr_addr + REG_WRPRTR); >> + __raw_writel(0x16, wtcr_addr + REG_WRPRTR); >> + __raw_writel(0x88, wtcr_addr + REG_WRPRTR); >> + } >> + >> + __raw_writel(1, wtcr_addr + REG_AHBIPRST); >> + } > > Please use writel() instead of __raw_writel(). Does this change apply to all others drivers? or just machine file to use writel()? > >> + soft_restart(0); >> +} >> + >> +DT_MACHINE_START(nuc970_dt, "Nuvoton nuc970 evb") >> + .atag_offset = 0x100, Removed it, thanks! > > The .atag_offset can be removed here. > > Arnd > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wan Zongshun Subject: Re: [PATCH 1/6] ARM: NUC900: Add nuc970 machine support Date: Tue, 5 Jul 2016 15:38:23 +0800 Message-ID: <577B63EF.6030906@iommu.org> References: <1466851042-22239-1-git-send-email-vw@iommu.org> <1466851042-22239-2-git-send-email-vw@iommu.org> <4806020.41AJ9s5VGu@wuerfel> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <4806020.41AJ9s5VGu@wuerfel> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Arnd Bergmann Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Wan Zongshun , Daniel Lezcano , Russell King , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Thomas Gleixner , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: devicetree@vger.kernel.org On 2016=E5=B9=B406=E6=9C=8829=E6=97=A5 23:19, Arnd Bergmann wrote: > On Saturday, June 25, 2016 6:37:17 PM CEST Wan Zongshun wrote: >> NUC970 is a new SoC of Nuvoton nuc900 series, this patch is >> to add machine file support for it. >> >> Signed-off-by: Wan Zongshun > > Nice to see some activity on the port! > >> --- >> arch/arm/mach-w90x900/Kconfig | 25 ++++ >> arch/arm/mach-w90x900/Makefile | 3 + >> .../mach-w90x900/include/mach/nuc970-regs-gcr.h | 56 ++++++++ >> arch/arm/mach-w90x900/mach-nuc970.c | 144 +++++++++= ++++++++++++ >> 4 files changed, 228 insertions(+) >> create mode 100644 arch/arm/mach-w90x900/include/mach/nuc970-regs-= gcr.h >> create mode 100644 arch/arm/mach-w90x900/mach-nuc970.c >> >> diff --git a/arch/arm/mach-w90x900/Kconfig b/arch/arm/mach-w90x900/K= config >> index 69bab32..050833e 100644 >> --- a/arch/arm/mach-w90x900/Kconfig >> +++ b/arch/arm/mach-w90x900/Kconfig >> @@ -15,6 +15,21 @@ config CPU_NUC960 >> help >> Support for NUCP960 of Nuvoton NUC900 CPUs. >> >> +config SOC_NUC970 >> + bool >> + select GENERIC_IRQ_CHIP >> + select SOC_BUS >> + select IRQ_DOMAIN >> + select MULTI_IRQ_HANDLER >> + select USE_OF >> + select HAVE_CLK_PREPARE >> + select HAVE_MACH_CLKDEV >> + select COMMON_CLK >> + select NUC900_TIMER >> + help >> + Support for NUCP970 of Nuvoton NUC900 CPUs. >> + > > [style] This looks whitespace damaged, and please sort the line alpha= betically. Sure, changed. > > I see you have done this in a way that is basically compatible with > CONFIG_ARCH_MULTIPLATFORM, good. > > What is HAVE_MACH_CLKDEV for? > >> @@ -46,4 +61,14 @@ config MACH_W90N960EVB >> >> endmenu >> >> +menu "NUC970 Machines" >> + >> +config MACH_NUC970EVB >> + bool "Nuvoton NUC970 Evaluation Board" >> + select SOC_NUC970 >> + help >> + Say Y here if you are using the Nuvoton NUC970EVB >> + >> +endmenu > > I'd leave out this entry, with the way have have structured the code = (correctly), > there is no need to separate SoC-specific code from board specific co= de, since > they are the same. > Sure, removed it. >> diff --git a/arch/arm/mach-w90x900/include/mach/nuc970-regs-gcr.h b/= arch/arm/mach-w90x900/include/mach/nuc970-regs-gcr.h >> new file mode 100644 >> index 0000000..e7eb653 >> --- /dev/null >> +++ b/arch/arm/mach-w90x900/include/mach/nuc970-regs-gcr.h > > Can you move the new headers to arch/arm/mach-w90x900/ directly? > >> +static int __init nuc900_restart_init(void) >> +{ >> + struct device_node *np; >> + >> + np =3D of_find_compatible_node(NULL, NULL, "nuvoton,gcr"); >> + wtcr_addr =3D of_iomap(np, 0); >> + if (!wtcr_addr) >> + return -ENODEV; >> + >> + of_node_put(np); >> + >> + return 0; >> +} > > Is this a watchdog node? If it is, the restart logic should just > move into the watchdog driver. It is not watchdog node, just be global System control register node. > > >> + if (of_machine_is_compatible("nuvoton,nuc970evb")) >> + nuc970_init(); > > What is this for? Currently, no used, remove it. > >> + of_platform_populate(NULL, of_default_bus_match_table, NULL, paren= t); > > We have actually moved away from using the soc_device as using the pa= rent > for the other devices, just probe them separately. In fact the soc_de= vice > could be handled by a driver in drivers/soc/nuvoton/ Do you think I should add nuc900 soc driver in this folder? If I want to add nuc900 soc driver in drivers/soc/nuvoton/, can I keep=20 my current dts structure no change, or Must I add a new node name soc {= }? I went through the code:soc-realview.c for reference, but I have no ide= a about how to re-structure my dts file to match this type soc driver. > >> +static const char *nuc970_dt_compat[] __initconst =3D { >> + "nuvoton,nuc970evb", >> + NULL, >> +}; >> + >> +void nuc970_restart(enum reboot_mode mode, const char *cmd) >> +{ >> + if (wtcr_addr) { >> + while (__raw_readl(wtcr_addr + REG_WRPRTR) !=3D 1) { >> + __raw_writel(0x59, wtcr_addr + REG_WRPRTR); >> + __raw_writel(0x16, wtcr_addr + REG_WRPRTR); >> + __raw_writel(0x88, wtcr_addr + REG_WRPRTR); >> + } >> + >> + __raw_writel(1, wtcr_addr + REG_AHBIPRST); >> + } > > Please use writel() instead of __raw_writel(). Does this change apply to all others drivers? or just machine file to=20 use writel()? > >> + soft_restart(0); >> +} >> + >> +DT_MACHINE_START(nuc970_dt, "Nuvoton nuc970 evb") >> + .atag_offset =3D 0x100, Removed it, thanks! > > The .atag_offset can be removed here. > > Arnd > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" i= n the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754721AbcGEHia (ORCPT ); Tue, 5 Jul 2016 03:38:30 -0400 Received: from mr213138.mail.yeah.net ([223.252.213.138]:41734 "EHLO mr213138.mail.yeah.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754699AbcGEHi1 (ORCPT ); Tue, 5 Jul 2016 03:38:27 -0400 Subject: Re: [PATCH 1/6] ARM: NUC900: Add nuc970 machine support To: Arnd Bergmann References: <1466851042-22239-1-git-send-email-vw@iommu.org> <1466851042-22239-2-git-send-email-vw@iommu.org> <4806020.41AJ9s5VGu@wuerfel> Cc: devicetree@vger.kernel.org, Wan Zongshun , Daniel Lezcano , Russell King , linux-kernel@vger.kernel.org, Thomas Gleixner , linux-arm-kernel@lists.infradead.org From: Wan Zongshun Message-ID: <577B63EF.6030906@iommu.org> Date: Tue, 5 Jul 2016 15:38:23 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <4806020.41AJ9s5VGu@wuerfel> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-HM-Spam-Status: e1koWUFPN1dZCBgUCR5ZQVZIVU1KSEtLS0pOT09CTklIQkhXWQkOFx4IWU FZKCs9JDU0NT4kMjUkMz46Pz4pQUtVS0A2IyQiPigkMjUkMz46Pz4pQUtVS0ArLykkNTQkMjUkMz 46Pz4pQUtVS0A4NC41LykiJDg1QUtVS0ApPjwyNDUkOigyOkFLVUtAKyk0LTI1OD4kKDM6NTwzOj JBS1VLQD8iNTo2MjgkMiskNTQkMjUkMz46Pz4pQUtVS0A2LjcvMiQpOCsvJD8yPT0#KT41LyQyNS QzPjo*PilBSVVLQDIrJEhLJDYyNS4vPiQ4NS8kSyROS0FLVUtAMiskSiQ2MjUuLz4kODUvJEskSk tBS1VLQDIrJC80PzoiJDg1LyRLJEpLS0FLVUtAMiskTiQ2MjUuLz4kODUvJEskSktBS1VLQDIrJE okMzQuKSQ4NS8kSyRKS0tBS1VLQCguOTE#OC8kLzQ*OiIkODUvJEskSktLQUtVS0AoLjkxPjgvJE 4kNjI1Li8#JDg1LyRLJEpLQUtVS0AoLjkxPjgvJEokMzQuKSQ4NS8kSyRKS0tBS1VLQCg5MSQ3Pj UkT0skI0FLVUtLSktALz4jLyQ3PjUkSklDSyQjQUtVS0tKS0A4NS8kMy82NyQ3PjUkSjAkTjBBS1 VLS0pLQCk#OD4yLT4pJDg1LyROJCNBS1VLS0pLQCM2OjI3PikkNS43N0FLVUtLSktAPz43Mi0#KS Q3PjUkS0FLVUtLSktANzo1PCQ#NUFLVUtLSktAPTUkNjoiJE9KQiQzNzEkSiRLQ0tIS09BS1VISE A9KyQpPiQ9LCQzNzEkS0NLSEtNQVZMVU5AKC45JD5BSlVOTkA9NSQ2OiIkT0pCJDM3MSRJJEtDS0 hLT0FLVUtZBg++ X-HM-Sender-Digest: e1kSHx4VD1lBWUc6NBQ6Cww6TDofTTwaHwlJSxQdF01PFDxVSlVKT01M TEtPSEtPSElDVTMWGhIXVQ0MOxIUFhYOVRQJHEVZV1kMHhlZQR0aFwgeV1kIAVlBTU9NQjdXWRIL WUFZSUpLVUpIVUJMVUpNQ1kG X-HM-Tid: 0a55b9fe62297d8ab669d1019ef Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016年06月29日 23:19, Arnd Bergmann wrote: > On Saturday, June 25, 2016 6:37:17 PM CEST Wan Zongshun wrote: >> NUC970 is a new SoC of Nuvoton nuc900 series, this patch is >> to add machine file support for it. >> >> Signed-off-by: Wan Zongshun > > Nice to see some activity on the port! > >> --- >> arch/arm/mach-w90x900/Kconfig | 25 ++++ >> arch/arm/mach-w90x900/Makefile | 3 + >> .../mach-w90x900/include/mach/nuc970-regs-gcr.h | 56 ++++++++ >> arch/arm/mach-w90x900/mach-nuc970.c | 144 +++++++++++++++++++++ >> 4 files changed, 228 insertions(+) >> create mode 100644 arch/arm/mach-w90x900/include/mach/nuc970-regs-gcr.h >> create mode 100644 arch/arm/mach-w90x900/mach-nuc970.c >> >> diff --git a/arch/arm/mach-w90x900/Kconfig b/arch/arm/mach-w90x900/Kconfig >> index 69bab32..050833e 100644 >> --- a/arch/arm/mach-w90x900/Kconfig >> +++ b/arch/arm/mach-w90x900/Kconfig >> @@ -15,6 +15,21 @@ config CPU_NUC960 >> help >> Support for NUCP960 of Nuvoton NUC900 CPUs. >> >> +config SOC_NUC970 >> + bool >> + select GENERIC_IRQ_CHIP >> + select SOC_BUS >> + select IRQ_DOMAIN >> + select MULTI_IRQ_HANDLER >> + select USE_OF >> + select HAVE_CLK_PREPARE >> + select HAVE_MACH_CLKDEV >> + select COMMON_CLK >> + select NUC900_TIMER >> + help >> + Support for NUCP970 of Nuvoton NUC900 CPUs. >> + > > [style] This looks whitespace damaged, and please sort the line alphabetically. Sure, changed. > > I see you have done this in a way that is basically compatible with > CONFIG_ARCH_MULTIPLATFORM, good. > > What is HAVE_MACH_CLKDEV for? > >> @@ -46,4 +61,14 @@ config MACH_W90N960EVB >> >> endmenu >> >> +menu "NUC970 Machines" >> + >> +config MACH_NUC970EVB >> + bool "Nuvoton NUC970 Evaluation Board" >> + select SOC_NUC970 >> + help >> + Say Y here if you are using the Nuvoton NUC970EVB >> + >> +endmenu > > I'd leave out this entry, with the way have have structured the code (correctly), > there is no need to separate SoC-specific code from board specific code, since > they are the same. > Sure, removed it. >> diff --git a/arch/arm/mach-w90x900/include/mach/nuc970-regs-gcr.h b/arch/arm/mach-w90x900/include/mach/nuc970-regs-gcr.h >> new file mode 100644 >> index 0000000..e7eb653 >> --- /dev/null >> +++ b/arch/arm/mach-w90x900/include/mach/nuc970-regs-gcr.h > > Can you move the new headers to arch/arm/mach-w90x900/ directly? > >> +static int __init nuc900_restart_init(void) >> +{ >> + struct device_node *np; >> + >> + np = of_find_compatible_node(NULL, NULL, "nuvoton,gcr"); >> + wtcr_addr = of_iomap(np, 0); >> + if (!wtcr_addr) >> + return -ENODEV; >> + >> + of_node_put(np); >> + >> + return 0; >> +} > > Is this a watchdog node? If it is, the restart logic should just > move into the watchdog driver. It is not watchdog node, just be global System control register node. > > >> + if (of_machine_is_compatible("nuvoton,nuc970evb")) >> + nuc970_init(); > > What is this for? Currently, no used, remove it. > >> + of_platform_populate(NULL, of_default_bus_match_table, NULL, parent); > > We have actually moved away from using the soc_device as using the parent > for the other devices, just probe them separately. In fact the soc_device > could be handled by a driver in drivers/soc/nuvoton/ Do you think I should add nuc900 soc driver in this folder? If I want to add nuc900 soc driver in drivers/soc/nuvoton/, can I keep my current dts structure no change, or Must I add a new node name soc {}? I went through the code:soc-realview.c for reference, but I have no idea about how to re-structure my dts file to match this type soc driver. > >> +static const char *nuc970_dt_compat[] __initconst = { >> + "nuvoton,nuc970evb", >> + NULL, >> +}; >> + >> +void nuc970_restart(enum reboot_mode mode, const char *cmd) >> +{ >> + if (wtcr_addr) { >> + while (__raw_readl(wtcr_addr + REG_WRPRTR) != 1) { >> + __raw_writel(0x59, wtcr_addr + REG_WRPRTR); >> + __raw_writel(0x16, wtcr_addr + REG_WRPRTR); >> + __raw_writel(0x88, wtcr_addr + REG_WRPRTR); >> + } >> + >> + __raw_writel(1, wtcr_addr + REG_AHBIPRST); >> + } > > Please use writel() instead of __raw_writel(). Does this change apply to all others drivers? or just machine file to use writel()? > >> + soft_restart(0); >> +} >> + >> +DT_MACHINE_START(nuc970_dt, "Nuvoton nuc970 evb") >> + .atag_offset = 0x100, Removed it, thanks! > > The .atag_offset can be removed here. > > Arnd > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > >