From mboxrd@z Thu Jan 1 00:00:00 1970 From: arnd@arndb.de (Arnd Bergmann) Date: Tue, 30 Jun 2015 23:01:49 +0200 Subject: arnRe: [PATCH v2 1/5] ARM: rcm-k1879xb1: Add support for K1879XB1 SoC In-Reply-To: <1435677307-6526-2-git-send-email-andrew@ncrmnt.org> References: <1435677307-6526-1-git-send-email-andrew@ncrmnt.org> <1435677307-6526-2-git-send-email-andrew@ncrmnt.org> Message-ID: <1814286.cdle2p4xha@wuerfel> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tuesday 30 June 2015 18:15:03 Andrew Andrianov wrote: > --- a/arch/arm/Makefile > +++ b/arch/arm/Makefile > @@ -185,6 +185,7 @@ machine-$(CONFIG_ARCH_ORION5X) += orion5x > machine-$(CONFIG_ARCH_PICOXCELL) += picoxcell > machine-$(CONFIG_ARCH_PXA) += pxa > machine-$(CONFIG_ARCH_QCOM) += qcom > +machine-$(CONFIG_ARCH_RCM_K1879XB1) += rcm-k1879xb1 Could this be a bit shorter, e.g. 'mach-rcm' or 'mach-k1879xb1'? The directories contain very few files for new platforms these days, so we just tend to lump everything together by manufacturer. > diff --git a/arch/arm/mach-rcm-k1879xb1/board-dt.c b/arch/arm/mach-rcm-k1879xb1/board-dt.c > new file mode 100644 > index 0000000..48d3835 > --- /dev/null > +++ b/arch/arm/mach-rcm-k1879xb1/board-dt.c > @@ -0,0 +1,145 @@ > +/* > + * arch/arm/mach-rcm-k1879/board-dt.c Generally speaking, please avoid comments that only contain the file names. Also, 'board-dt' is not a particularly useful name if that is used to handle all machines. Maybe call it the same as the platform. > + > +static void __iomem *k1879_sctl_base(void) > +{ > + return (void __iomem *)RCM_K1879_SCTL_VIRT_BASE; > +} Please don't hardcode virtual addresses like this. Instead read it from DT at boot time using of_iomap or similar. > +static void k1879_level_irq_i2c0_fixup(unsigned int irq, struct irq_desc *desc) > +{ > + writel(1, k1879_mif_base() + RCM_K1879_MIF_I2C_INT_STAT); > + handle_level_irq(irq, desc); > +} > + > +static void k1879_level_irq_i2c1_fixup(unsigned int irq, struct irq_desc *desc) > +{ > + writel(1 << 0, k1879_sctl_base() + RCM_K1879_SCTL_INT_P_OUT); > + handle_level_irq(irq, desc); > +} > + > +static void k1879_level_irq_i2c2_fixup(unsigned int irq, struct irq_desc *desc) > +{ > + writel(1 << 1, k1879_sctl_base() + RCM_K1879_SCTL_INT_P_OUT); > + handle_level_irq(irq, desc); > +} > + > +static void k1879_level_irq_i2c3_fixup(unsigned int irq, struct irq_desc *desc) > +{ > + writel(1 << 2, k1879_sctl_base() + RCM_K1879_SCTL_INT_P_OUT); > + handle_level_irq(irq, desc); > +} > + > +static void (*k1879_i2c_fixups[])(unsigned int irq, struct irq_desc *desc) = { > + k1879_level_irq_i2c0_fixup, > + k1879_level_irq_i2c1_fixup, > + k1879_level_irq_i2c2_fixup, > + k1879_level_irq_i2c3_fixup > +}; It looks to me like this could be implemented as a nested irq domain with its own irqchip driver rather than a hack in platform code. > +static void __init k1879_dt_mach_init(void) > +{ > + struct device_node *np; > + > + of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); > + k1879_mif = ioremap(RCM_K1879_MIF_PHYS_BASE, > + RCM_K1879_MIF_SIZE); > + BUG_ON(!k1879_mif); > + > + np = of_find_compatible_node(NULL, NULL, "arm,sp805"); > + WARN_ON(!np); > + if (np) > + k1879_wdt = of_iomap(np, 0); Please move this into the watchdog driver as a reboot handler. > + /* Setup i2c interrupt fixups */ > + setup_i2c_fixups(); > + > + /* I2C0 (Internal, HDMI) needs some extra love */ > + do { > + void __iomem *mif; > + > + mif = k1879_mif_base(); > + writel(1, mif + RCM_K1879_MIF_I2C_INT_TYPE_ENA); > + writel(1, mif + RCM_K1879_MIF_I2C_INT_TYPE); > + writel(1, mif + RCM_K1879_MIF_I2C_INT_ENA); > + } while (0); > +} It would be nice if this could be moved to some driver as well, ideally the one that already maps k1879_mif_base. Does this need to be done really early at boot time? > diff --git a/arch/arm/mach-rcm-k1879xb1/hardware.h b/arch/arm/mach-rcm-k1879xb1/hardware.h > new file mode 100644 > index 0000000..2977ed7 > --- /dev/null > +++ b/arch/arm/mach-rcm-k1879xb1/hardware.h > @@ -0,0 +1,48 @@ > +/* > + * arch/arm/mach-rcm-k1879/board-dt.c ;-) Arnd From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752640AbbF3VCU (ORCPT ); Tue, 30 Jun 2015 17:02:20 -0400 Received: from mout.kundenserver.de ([212.227.126.130]:62818 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751643AbbF3VCN (ORCPT ); Tue, 30 Jun 2015 17:02:13 -0400 From: Arnd Bergmann To: Andrew Andrianov Cc: Pawel Moll , Mark Rutland , "Rafael J. Wysocki" , Daniel Lezcano , Ian Campbell , Kumar Gala , Russell King , Rob Herring , Pavel Shevchenko , Andrew Andrianov , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Subject: arnRe: [PATCH v2 1/5] ARM: rcm-k1879xb1: Add support for K1879XB1 SoC Date: Tue, 30 Jun 2015 23:01:49 +0200 Message-ID: <1814286.cdle2p4xha@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <1435677307-6526-2-git-send-email-andrew@ncrmnt.org> References: <1435677307-6526-1-git-send-email-andrew@ncrmnt.org> <1435677307-6526-2-git-send-email-andrew@ncrmnt.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:BGB51xXlh3ymEeqWuv8F2WSNixHyRkln8WavHzhdzxV5ndUAdvY tkMB3MKYKspBuhcMfJoJx4EXim6Ww46/O1jsIhXfuDTCjcBCWdxPIOiNFhtvKHWqkTjsceU JkdDyzzJUxtXOyRrJTpKwc0EizHF+RKRaV2n7LEp4GDgI0WtbGf+BlCdKBhIXbqEfNuPr3f PEraYgAUL78FEguRkw/og== X-UI-Out-Filterresults: notjunk:1;V01:K0:z/f/dITSszY=:u59R2j0gUJ6YnHCaDu/s48 4oOf37QBstMWq/rcGbLMjyKz8Tv0Lh+yukQULoHErJmmYhf5idXw83AVtC2Tc1UDCmTocPIRd l2CVdj2sIB+UjBo07szWWHGqj/ItrVvnP7xB0t2qj3d6aRr2JqQdRDdmYSun0+47cSyHYIkrq eHCNGXZfy6jDbRPnRB7X9P4ojFvU3OQs9jzWxVTty56HUhaIL6i+igu8GV6bOdNWkX5omPqIT OIJBOXJUUO7JP1Ou2IbU672vU2yOK6GmXLenRUT+jMJSvzFsBkp7bZ7LVEZ+CZCKnft3cwRk9 Dzaqk/kIFm3hu/GxUDJCq1+LrMox2DcDNWeJ6LmdAkONpGWjusfFaoxoewJ482RmLLYliGj5D 8HBp6oL66+wRbiBro8LaIS81dDKsGAZ972n4QFLDVifY0SgpIKqVUtrdUU1TNI7+EVVygn+A6 gehREDV7K+y5w/8kSJ2ioy5UUeNJU8Zha3rmHkjn/BOo/CxxT0amzY3E6wMS1gCwzjv51F22R 0/VESG6uJB8R0MxA88/GuZL7oBZcJhBHoDpaDzbCS32da9Dfq6WTpQKhB3FIACB6pEAh28ZYn HUs8TtJ4RjPU5IFdFPzupZdsrj46J8Qvwnu7d6ePzGupFaRIoa56I0i2WBF3Q2HxVCZ1bSYAD Zgw+RyNWZAF1zaiRA9mPc/2QVQeHP5WG3lC6bEtrL+I5pcA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 30 June 2015 18:15:03 Andrew Andrianov wrote: > --- a/arch/arm/Makefile > +++ b/arch/arm/Makefile > @@ -185,6 +185,7 @@ machine-$(CONFIG_ARCH_ORION5X) += orion5x > machine-$(CONFIG_ARCH_PICOXCELL) += picoxcell > machine-$(CONFIG_ARCH_PXA) += pxa > machine-$(CONFIG_ARCH_QCOM) += qcom > +machine-$(CONFIG_ARCH_RCM_K1879XB1) += rcm-k1879xb1 Could this be a bit shorter, e.g. 'mach-rcm' or 'mach-k1879xb1'? The directories contain very few files for new platforms these days, so we just tend to lump everything together by manufacturer. > diff --git a/arch/arm/mach-rcm-k1879xb1/board-dt.c b/arch/arm/mach-rcm-k1879xb1/board-dt.c > new file mode 100644 > index 0000000..48d3835 > --- /dev/null > +++ b/arch/arm/mach-rcm-k1879xb1/board-dt.c > @@ -0,0 +1,145 @@ > +/* > + * arch/arm/mach-rcm-k1879/board-dt.c Generally speaking, please avoid comments that only contain the file names. Also, 'board-dt' is not a particularly useful name if that is used to handle all machines. Maybe call it the same as the platform. > + > +static void __iomem *k1879_sctl_base(void) > +{ > + return (void __iomem *)RCM_K1879_SCTL_VIRT_BASE; > +} Please don't hardcode virtual addresses like this. Instead read it from DT at boot time using of_iomap or similar. > +static void k1879_level_irq_i2c0_fixup(unsigned int irq, struct irq_desc *desc) > +{ > + writel(1, k1879_mif_base() + RCM_K1879_MIF_I2C_INT_STAT); > + handle_level_irq(irq, desc); > +} > + > +static void k1879_level_irq_i2c1_fixup(unsigned int irq, struct irq_desc *desc) > +{ > + writel(1 << 0, k1879_sctl_base() + RCM_K1879_SCTL_INT_P_OUT); > + handle_level_irq(irq, desc); > +} > + > +static void k1879_level_irq_i2c2_fixup(unsigned int irq, struct irq_desc *desc) > +{ > + writel(1 << 1, k1879_sctl_base() + RCM_K1879_SCTL_INT_P_OUT); > + handle_level_irq(irq, desc); > +} > + > +static void k1879_level_irq_i2c3_fixup(unsigned int irq, struct irq_desc *desc) > +{ > + writel(1 << 2, k1879_sctl_base() + RCM_K1879_SCTL_INT_P_OUT); > + handle_level_irq(irq, desc); > +} > + > +static void (*k1879_i2c_fixups[])(unsigned int irq, struct irq_desc *desc) = { > + k1879_level_irq_i2c0_fixup, > + k1879_level_irq_i2c1_fixup, > + k1879_level_irq_i2c2_fixup, > + k1879_level_irq_i2c3_fixup > +}; It looks to me like this could be implemented as a nested irq domain with its own irqchip driver rather than a hack in platform code. > +static void __init k1879_dt_mach_init(void) > +{ > + struct device_node *np; > + > + of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); > + k1879_mif = ioremap(RCM_K1879_MIF_PHYS_BASE, > + RCM_K1879_MIF_SIZE); > + BUG_ON(!k1879_mif); > + > + np = of_find_compatible_node(NULL, NULL, "arm,sp805"); > + WARN_ON(!np); > + if (np) > + k1879_wdt = of_iomap(np, 0); Please move this into the watchdog driver as a reboot handler. > + /* Setup i2c interrupt fixups */ > + setup_i2c_fixups(); > + > + /* I2C0 (Internal, HDMI) needs some extra love */ > + do { > + void __iomem *mif; > + > + mif = k1879_mif_base(); > + writel(1, mif + RCM_K1879_MIF_I2C_INT_TYPE_ENA); > + writel(1, mif + RCM_K1879_MIF_I2C_INT_TYPE); > + writel(1, mif + RCM_K1879_MIF_I2C_INT_ENA); > + } while (0); > +} It would be nice if this could be moved to some driver as well, ideally the one that already maps k1879_mif_base. Does this need to be done really early at boot time? > diff --git a/arch/arm/mach-rcm-k1879xb1/hardware.h b/arch/arm/mach-rcm-k1879xb1/hardware.h > new file mode 100644 > index 0000000..2977ed7 > --- /dev/null > +++ b/arch/arm/mach-rcm-k1879xb1/hardware.h > @@ -0,0 +1,48 @@ > +/* > + * arch/arm/mach-rcm-k1879/board-dt.c ;-) Arnd