lkml.org 
[lkml]   [2015]   [Jun]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectarnRe: [PATCH v2 1/5] ARM: rcm-k1879xb1: Add support for K1879XB1 SoC
Date
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


\
 
 \ /
  Last update: 2015-06-30 23:21    [W:0.071 / U:0.876 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site