Re: Appended DTB files for multi-machine kernels

2013-07-05 Thread Magnus Damm
Hi Arnd,

On Fri, Jul 5, 2013 at 6:34 AM, Arnd Bergmann a...@arndb.de wrote:
 On Thursday 04 July 2013, Mark Brown wrote:
 On Thu, Jul 04, 2013 at 06:56:24PM +0200, Daniel Mack wrote:

  Unless I missed some recent discussion, this case is not easy to handle.
  Yes, I know that these kind of things should be handled by a
  next-generation bootloader, but in our case, we want to avoid a loader
  update of already shipped hardware by all means.

 There was some discussion about appending multiple DTBs recently.  I
 can't actually recall anything about it though so that's not an entirely
 helpful thing...

 Yes, it keeps coming up, I think by now everybody agreed it's a good
 idea to extend the ATAGS compat mode, but so far nobody has implemented
 it. Magnus Damm was very interested in this feature last year and
 was planning to work on it, but I don't know how far he got.

Correct, I was and still am a bit interested in it. I did however
never get around to write any code. My apologies if that stalled
anyone.

With mach-shmobile we're now getting to a point where most of our
boards are using DT_MACHINE and appended DTB instead of the MACHINE
and mach-type. Because of that I feel that the moment has sort of
passed for me.

Regardless of this nearly all our boot loaders pass mach-type so it
would be nice to match on that instead of the appended DTB.

Thanks,

/ magnus
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3 5/7] ARM: shmobile: r8a7740: add DT nodes for three DMAC instance

2013-07-03 Thread Magnus Damm
Hi Guennadi,

On Wed, Jul 3, 2013 at 3:23 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 Hi Magnus

 On Wed, 3 Jul 2013, Magnus Damm wrote:

 Hi Guennadi,

 On Wed, Jun 26, 2013 at 11:40 PM, Guennadi Liakhovetski
 g.liakhovet...@gmx.de wrote:
  This patch adds Device Tree support for the three DMA controller instances
  on r8a7740 in a DMA multiplexer node.
 
  Signed-off-by: Guennadi Liakhovetski g.liakhovetski+rene...@gmail.com
  ---
   arch/arm/boot/dts/r8a7740.dtsi |   62 
  
   arch/arm/mach-shmobile/setup-r8a7740.c |   12 ++
   2 files changed, 74 insertions(+), 0 deletions(-)

 Thanks for your work on this.

  --- a/arch/arm/mach-shmobile/setup-r8a7740.c
  +++ b/arch/arm/mach-shmobile/setup-r8a7740.c
  @@ -996,7 +996,19 @@ void __init r8a7740_add_early_devices(void)
 
   #ifdef CONFIG_USE_OF
 
  +static struct of_dev_auxdata r8a7740_dmac_auxdata[] = {
  +   OF_DEV_AUXDATA(renesas,shdma, 0xfe008020, sh-dma-engine.0,
  +  dma_platform_data),
  +   OF_DEV_AUXDATA(renesas,shdma, 0xfe018020, sh-dma-engine.1,
  +  dma_platform_data),
  +   OF_DEV_AUXDATA(renesas,shdma, 0xfe028020, sh-dma-engine.2,
  +  dma_platform_data),
  +   { }
  +};
  +
   static const struct of_dev_auxdata r8a7740_auxdata_lookup[] __initconst = 
  {
  +   OF_DEV_AUXDATA(renesas,shdma-mux, 0, shdma-of.0,
  +  r8a7740_dmac_auxdata),
  { }
   };

 Uhm, what is the reason for adding AUXDATA? For all other cases we
 have clearly separated the DT reference bits from the C version
 without using AUXDATA. With that approach we can use the default NULL
 callbacks for -init_machine(). Now with this patch we're going in the
 totally different direction...

 Why can't you use DT only for these?

 Short answer - because that's how the SH DMA driver, currently in the
 next tree ready for Linus to be pulled, is implemented. We had enough
 time to discuss this back then, I think, it is a pity this question is
 raised only now.

Yes, indeed, in the same way it is a pity that this turns out to be
the only user of AUXDATA.

 A while ago I read, that it is ok to pass SoC-specific device properties
 via auxdata, as opposed to board-specific ones, that have to go via DT. On
 shmobile this is also easy to implement - all devices, initialised from
 setup-soc.c, i.e. having no board-specific configuration, and defined in
 soc.dtsi can just take their configuration from the same platform data,
 as in .c case. Another motivation for this is, that we want to get rid of
 board-name.c files, but we'll always have setup-soc.c files. With that
 in mind I implemented DMAC DT bindings and that also allowed us to so
 quickly get them accepted into the mainline. If we needed to initialise
 DMAC completely from DT, mainlining would certainly take much longer.

Neither DMAC nor DT have ever been rushing, so I'm not sure why you
are doing that now. Both those are about correctness. We are fine
without DMAC and without DT for a majority of the cases. So let's get
it right instead of half-right.

Sorry to say this, but from my perspective the AUXDATA solution above
seems to be a rushed short term solution to a more long term problem.
Actually, I'm not sure how you ever came to think that was a good
idea.

Please show how you plan on going from AUXDATA to DT only. I am fine
with something similar to the PFC implementation that has SoC-specifc
bits written in C outside arch/arm, but I'd rather not use AUXDATA for
future code.

Thanks,

/ magnus
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3 5/7] ARM: shmobile: r8a7740: add DT nodes for three DMAC instance

2013-07-02 Thread Magnus Damm
Hi Guennadi,

On Wed, Jun 26, 2013 at 11:40 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 This patch adds Device Tree support for the three DMA controller instances
 on r8a7740 in a DMA multiplexer node.

 Signed-off-by: Guennadi Liakhovetski g.liakhovetski+rene...@gmail.com
 ---
  arch/arm/boot/dts/r8a7740.dtsi |   62 
 
  arch/arm/mach-shmobile/setup-r8a7740.c |   12 ++
  2 files changed, 74 insertions(+), 0 deletions(-)

Thanks for your work on this.

 --- a/arch/arm/mach-shmobile/setup-r8a7740.c
 +++ b/arch/arm/mach-shmobile/setup-r8a7740.c
 @@ -996,7 +996,19 @@ void __init r8a7740_add_early_devices(void)

  #ifdef CONFIG_USE_OF

 +static struct of_dev_auxdata r8a7740_dmac_auxdata[] = {
 +   OF_DEV_AUXDATA(renesas,shdma, 0xfe008020, sh-dma-engine.0,
 +  dma_platform_data),
 +   OF_DEV_AUXDATA(renesas,shdma, 0xfe018020, sh-dma-engine.1,
 +  dma_platform_data),
 +   OF_DEV_AUXDATA(renesas,shdma, 0xfe028020, sh-dma-engine.2,
 +  dma_platform_data),
 +   { }
 +};
 +
  static const struct of_dev_auxdata r8a7740_auxdata_lookup[] __initconst = {
 +   OF_DEV_AUXDATA(renesas,shdma-mux, 0, shdma-of.0,
 +  r8a7740_dmac_auxdata),
 { }
  };

Uhm, what is the reason for adding AUXDATA? For all other cases we
have clearly separated the DT reference bits from the C version
without using AUXDATA. With that approach we can use the default NULL
callbacks for -init_machine(). Now with this patch we're going in the
totally different direction...

Why can't you use DT only for these?

Cheers,

/ magnus
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [RFC] early init and DT platform devices allocation/registration

2013-06-26 Thread Magnus Damm
Hi Lorenzo,

On Wed, Jun 26, 2013 at 2:07 AM, Lorenzo Pieralisi
lorenzo.pieral...@arm.com wrote:
 On Tue, Jun 25, 2013 at 03:56:22PM +0100, Stephen Warren wrote:
 On 06/25/2013 05:45 AM, Grant Likely wrote:
  On Mon, Jun 24, 2013 at 5:33 PM, Lorenzo Pieralisi
  lorenzo.pieral...@arm.com wrote:
  Hi all,
 
  I am dealing with a lingering problem related to init and probing of 
  platform
  devices early (before initcalls) in the kernel boot process. The problem,
  which is nothing new, is related to how platform devices are created in 
  the
  kernel from DT and when they become available. Platform devices are 
  created
  through (assuming any legacy static initialization is removed from the 
  kernel)
 
  of_platform_populate()
 
  at arch_initcall time on ARM. This is a problem for drivers that need to 
  be
  probed and initialized before arch_initcall (ie early_initcall) because
  the corresponding platform_device has not been created yet.

 What's the use-case here; why does the driver /have/ to be
 allocated/probed so early? Can't drivers all be allocated from DT in the
 usual fashion, and any dependencies resolved using deferred probe? In
 almost all cases, that should work.

 The driver must be initialized in order to boot secondary CPUs, so
 very very early, its initialization cannot be deferred.
 Otherwise we would end up with MCPM back-end having to add ad-hoc
 kludges to boot secondary CPUs, and then replace the early function calls
 to the power controller drivers when the power controller has been
 properly initialized. Feasible, but really horrible.

How about moving part of the SMP initialization to later during boot?

In particular I mean the bring up of the secondary cores. Just booting
with a single CPU for a bit longer is certainly doable because it is
today possible to bring secondary CPU cores up from user space via the
hotplug interface. Below is some old prototype code that I hacked up a
while ago - note that I copied the original comment about that this
should be done in user space. =)

I'd like to have secondary CPU cores initialized later so I can use
the regular device model (DT or platform device) to describe one or
more power domain hardware devices. These power domains control the
CPU cores, so there is a dependency on the SMP bring up code...

Cheers,

/ magnus

--- 0001/include/linux/smp.h
+++ work/include/linux/smp.h2013-05-21 20:08:42.0 +0900
@@ -124,6 +124,7 @@ void smp_prepare_boot_cpu(void);
 extern unsigned int setup_max_cpus;
 extern void __init setup_nr_cpu_ids(void);
 extern void __init smp_init(void);
+extern void __init smp_late_init(void);

 #else /* !SMP */

--- 0001/init/main.c
+++ work/init/main.c2013-05-21 20:09:34.0 +0900
@@ -334,6 +334,7 @@ static void __init smp_init(void)

 static inline void setup_nr_cpu_ids(void) { }
 static inline void smp_prepare_cpus(unsigned int maxcpus) { }
+static inline void smp_late_init(void) { }
 #endif

 /*
@@ -879,6 +880,8 @@ static noinline void __init kernel_init_

do_basic_setup();

+   smp_late_init();
+
/* Open the /dev/console on the rootfs, this should never fail */
if (sys_open((const char __user *) /dev/console, O_RDWR, 0)  0)
pr_err(Warning: unable to open an initial console.\n);
--- 0001/kernel/smp.c
+++ work/kernel/smp.c   2013-05-21 20:07:06.0 +0900
@@ -549,6 +549,20 @@ void __init smp_init(void)
if (!cpu_online(cpu))
cpu_up(cpu);
}
+}
+
+/* Called by boot processor late during boot to activate the rest. */
+void __init smp_late_init(void)
+{
+   unsigned int cpu;
+
+   /* FIXME: This should be done in userspace --RR */
+   for_each_present_cpu(cpu) {
+   if (num_online_cpus() = setup_max_cpus)
+   break;
+   if (!cpu_online(cpu))
+   cpu_up(cpu);
+   }

/* Any cleanup work */
printk(KERN_INFO Brought up %ld CPUs\n, (long)num_online_cpus());


Cheers,

/ magnus
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH 1/2] ARM: shmobile: irqpin: add a DT property to enable masking on parent

2013-06-11 Thread Magnus Damm
On Fri, May 24, 2013 at 6:13 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 To disable spurious interrupts, that get triggered on certain hardware, the
 irqpin driver masks them on the parent interrupt controller. To specify
 such broken devices a .control_parent parameter can be provided in the
 platform data. In the DT case we need a property, to do the same.

 Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
 ---

 These two patches simply split the earlier ARM: shmobile: irqpin: fix
 handling of spurious interrupts in DT case patch into two parts.
 Otherwise no change.

I'm fine with this portion of the patch series. Thanks for your help!

Acked-by: Magnus Damm d...@opensource.se
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v4 15/21] ARM: shmobile: armadillo-reference: Move st1232 reset GPIO to DT

2013-05-24 Thread Magnus Damm
Hi Linus,

On Fri, May 24, 2013 at 5:59 PM, Linus Walleij linus.wall...@linaro.org wrote:
 On Tue, May 21, 2013 at 2:14 PM, Laurent Pinchart
 laurent.pinchart+rene...@ideasonboard.com wrote:

 Reference the st1232 reset GPIO from the device tree and remove it from
 board code.

 Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com

 +++ b/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts
 @@ -43,6 +43,7 @@
 interrupts = 2 0; /* IRQ10: hwirq 2 on irqpin1 */
 pinctrl-0 = st1232_pins;
 pinctrl-names = default;
 +   gpios = pfc 166 1;

 Just as a random example this becomes:

 gpios = pfc 166 GPIO_ACTIVE_LOW;

 Which is WAAAY more readable.

Yes, agreed, even grumpy me starts thinking this looks good. =)

 +++ b/arch/arm/mach-shmobile/board-armadillo800eva-reference.c
 @@ -158,7 +158,6 @@ clock_error:
   */
  static void __init eva_init(void)
  {
 -
 r8a7740_clock_init(MD_CK0 | MD_CK2);
 eva_clock_init();

 @@ -171,12 +170,6 @@ static void __init eva_init(void)

 r8a7740_add_standard_devices_dt();

 -   /*
 -* Touchscreen
 -* TODO: Move reset GPIO over to .dts when we can reference it
 -*/
 -   gpio_request_one(166, GPIOF_OUT_INIT_HIGH, NULL); /* TP_RST_B */
 -

 Not really my business but I guess the driver is already augmented to pick its
 GPIO from the device tree in this case? And are you willingly breaking
 non-DT boots or something?

Regarding dependency handing of driver and board code, I believe
Laurent has us covered.

As for breaking non-DT boot, under mach-shmobile the code for DT
-reference board support is DT only so judging by the file name this
kind of change should be fine.

Thanks,

/ magnus
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v3] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

2013-04-09 Thread Magnus Damm
Hi Guennadi,

On Mon, Apr 8, 2013 at 5:08 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 Most Renesas irqpin controllers have 4-bit sense fields, however, some
 have different widths. This patch adds a DT binding to optionally
 specify such non-standard values.

 Signed-off-by: Guennadi Liakhovetski g.liakhovetski+rene...@gmail.com
 ---

 v3: move the code to a common location, where device configuration
 parameters are retrieved

Thanks for rearranging the code, this looks good to me.

Acked-by: Magnus Damm d...@opensource.se

To be clear, I prefer your approach over a per-SoC compatible string.

In general I think a per-SoC compatible string is nice in theory, but
I don't think it is correct to use it to describe a change in a IP
block that just happens to included in the SoC. Instead the version of
the IP block shall be used with the compatible value. In some cases it
may not be easy to retrieve such a version.

The per-SoC compatible string may look good but they come with at
least two drawbacks. Either
1) the driver has to be updated for each new SoC even though the
device IP the driver is handling hasn't changed which leads to
1.1) more need for pointless per-SoC compatible string patches to be
merged and tracked and back ported
and
1.2) less chance of running a standard distro lacking per-SoC
compatible string but has actual code for support
or
2) to ship soon the per-SoC DT will use SoC compatible strings
matching other SoC names which works but is even more confusing.

For the INTC irqpin case I believe this approach with a single
property is the best.

Thanks,

/ magnus
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH v2] irqchip: renesas-intc-irqpin: DT binding for sense bitfield width

2013-04-07 Thread Magnus Damm
Hi Guennadi,

On Fri, Apr 5, 2013 at 6:33 PM, Guennadi Liakhovetski
g.liakhovet...@gmx.de wrote:
 Most Renesas irqpin controllers have 4-bit sense fields, however, some
 have different widths. This patch adds a DT binding to optionally
 specify such non-standard values.

 Signed-off-by: Guennadi Liakhovetski g.liakhovetski+rene...@gmail.com
 ---

 v2: is no longer based on an earlier ARM: shmobile: irqpin: fix handling
 of spurious interrupts in DT case patch, which needs more work.

  .../interrupt-controller/renesas,intc-irqpin.txt   |   13 +
  drivers/irqchip/irq-renesas-intc-irqpin.c  |5 +
  2 files changed, 18 insertions(+), 0 deletions(-)
  create mode 100644 
 Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt

 diff --git 
 a/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
  
 b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
 new file mode 100644
 index 000..c6f09b7
 --- /dev/null
 +++ 
 b/Documentation/devicetree/bindings/interrupt-controller/renesas,intc-irqpin.txt
 @@ -0,0 +1,13 @@
 +DT bindings for the R-/SH-Mobile irqpin controller
 +
 +Required properties:
 +
 +- compatible: has to be renesas,intc-irqpin
 +- #interrupt-cells: has to be 2
 +
 +Optional properties:
 +
 +- any properties, listed in interrupts.txt in this directory, and any 
 standard
 +  resource allocation properties
 +- sense-bitfield-width: width of a single sense bitfield in the SENSE 
 register,
 +  if different from the default 4 bits
 diff --git a/drivers/irqchip/irq-renesas-intc-irqpin.c 
 b/drivers/irqchip/irq-renesas-intc-irqpin.c
 index 5a68e5a..7a02dfe 100644
 --- a/drivers/irqchip/irq-renesas-intc-irqpin.c
 +++ b/drivers/irqchip/irq-renesas-intc-irqpin.c
 @@ -18,6 +18,7 @@
   */

  #include linux/init.h
 +#include linux/of.h
  #include linux/platform_device.h
  #include linux/spinlock.h
  #include linux/interrupt.h
 @@ -429,6 +430,10 @@ static int intc_irqpin_probe(struct platform_device 
 *pdev)
 }
 }

 +   if (!pdata)
 +   of_property_read_u32(pdev-dev.of_node, 
 sense-bitfield-width,
 +p-config.sense_bitfield_width);
 +

Thanks for remaking this patch into V2. I am OK with the actual code,
but I wonder why you don't keep the above together with the rest of
the configuration setup code somewhere below the comment /* deal with
driver instance configuration */.

Cheers,

/ magnus
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH] net: sh_eth: Add support of device tree probe

2013-02-06 Thread Magnus Damm
Hey Simon, Iwamatsu-san,

On Wed, Feb 6, 2013 at 11:00 AM, Simon Horman
horms+rene...@verge.net.au wrote:
 From: Nobuhiro Iwamatsu nobuhiro.iwamatsu...@renesas.com

 This adds support of device tree probe for Renesas sh-ether driver.

 Signed-off-by: Nobuhiro Iwamatsu nobuhiro.iwamatsu...@renesas.com

Thanks for your work on this

 +++ b/Documentation/devicetree/bindings/net/sh_ether.txt
 @@ -0,0 +1,43 @@
 +* Renesas Electronics SuperH EMAC
 +
 +This file provides information, what the device node
 +for the sh_eth interface contains.
 +
 +Required properties:
 +- compatible:   renesas,sh-eth;
 +- interrupt-parent: The phandle for the interrupt controller that
 +services interrupts for this device.
 +- reg:  Offset and length of the register set for the
 +device.
 +- interrupts:   Interrupt mapping for the sh_eth interrupt
 +sources (vector id).
 +- phy-mode:  String, operation mode of the PHY interface.
 +- sh-eth,edmac-endian:  String, endian of sh_eth dmac.
 +- sh-eth,register-type: String, register type of sh_eth.
 +Please select gigabit, fast-sh4 or
 +fast-sh3-sh2.
 +Please select little or big.
 +- sh-eth,endian:String, endian of sh_eth dmac.
 +- sh-eth,phy-id:PHY id.
 +
 +Optional properties:
 +- local-mac-address : 6 bytes, mac address


 +- sh-eth,no-ether-link: set link control by software. When device 
 does
 +not support ether-link, set.
 +- sh-etn,ether-link-active-low: set link check method.
 +When detection of link is treated as 
 active-low,
 +set.
 +- sh-etn,needs-init:Initialization flag.
 +When initialize device in probing device, 
 set.

I believe there is a spelling mistake here with sh-etn instead of sh-eth.

Also, I am happy when DT is used to describe hardware information, but
in the case of needs-init it more looks like software policy.

Thanks,

/ magnus
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss


Re: [PATCH 3/8] ARM: shmobile: Add support OF of INTC for sh7372

2013-01-11 Thread Magnus Damm
Hi Mark,

Thanks for your feedback!

On Wed, Jan 9, 2013 at 8:17 PM, Mark Rutland mark.rutl...@arm.com wrote:
 On Wed, Jan 09, 2013 at 06:30:02AM +, Simon Horman wrote:
 From: Nobuhiro Iwamatsu nobuhiro.iwamatsu...@renesas.com

 This CPU has four interrupt controllers (INTCA, pins-High and pins-Low).
 This supports these.
 Note: This supports DT of INTCA only.

 Cc: Magnus Damm d...@opensource.se
 Signed-off-by: Nobuhiro Iwamatsu nobuhiro.iwamatsu...@renesas.com
 Signed-off-by: Simon Horman horms+rene...@verge.net.au

 ---

 v9
 * Update compatible string to use '-' instead of '_'
 ---
  arch/arm/mach-shmobile/include/mach/common.h |1 +
  arch/arm/mach-shmobile/intc-sh7372.c |  113 
 --
  2 files changed, 89 insertions(+), 25 deletions(-)

 diff --git a/arch/arm/mach-shmobile/include/mach/common.h 
 b/arch/arm/mach-shmobile/include/mach/common.h
 index d0a5790..223250b 100644
 --- a/arch/arm/mach-shmobile/include/mach/common.h
 +++ b/arch/arm/mach-shmobile/include/mach/common.h
 @@ -18,6 +18,7 @@ extern int shmobile_enter_wfi(struct cpuidle_device *dev,
 struct cpuidle_driver *drv, int index);
  extern void shmobile_cpuidle_set_driver(struct cpuidle_driver *drv);

 +extern void sh7372_init_irq_of(void);
  extern void sh7372_init_irq(void);
  extern void sh7372_map_io(void);
  extern void sh7372_add_early_devices(void);
 diff --git a/arch/arm/mach-shmobile/intc-sh7372.c 
 b/arch/arm/mach-shmobile/intc-sh7372.c
 index a91caad..bbde18d 100644
 --- a/arch/arm/mach-shmobile/intc-sh7372.c
 +++ b/arch/arm/mach-shmobile/intc-sh7372.c
 @@ -16,6 +16,8 @@
   * along with this program; if not, write to the Free Software
   * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  
 USA
   */
 +#define pr_fmt(fmt) intc:  fmt
 +
  #include linux/kernel.h
  #include linux/init.h
  #include linux/interrupt.h
 @@ -551,23 +553,28 @@ static void intcs_demux(unsigned int irq, struct 
 irq_desc *desc)

  static void __iomem *intcs_ffd2;
  static void __iomem *intcs_ffd5;
 -
 -void __init sh7372_init_irq(void)
 +static void __iomem *intca_e694;
 +static void __iomem *intca_e695;
 +
 +static void __init sh7372_init_intc(resource_size_t intca0_start,
 + resource_size_t intca1_start,
 + resource_size_t intcs0_start,
 + resource_size_t intcs1_start,
 + unsigned short vect)
  {
   void __iomem *intevtsa;
   int n;

 - intcs_ffd2 = ioremap_nocache(0xffd2, PAGE_SIZE);
 - intevtsa = intcs_ffd2 + 0x100;
 - intcs_ffd5 = ioremap_nocache(0xffd5, PAGE_SIZE);
 + intca_e694 = IOMEM(intca0_start);
 + intca_e695 = IOMEM(intca1_start);

 - register_intc_controller(intca_desc);
 - register_intc_controller(intca_irq_pins_lo_desc);
 - register_intc_controller(intca_irq_pins_hi_desc);
 - register_intc_controller(intcs_desc);
 + intcs_ffd2 = ioremap_nocache(intcs0_start, PAGE_SIZE);
 + intevtsa = intcs_ffd2 + 0x100;
 + intcs_ffd5 = ioremap_nocache(intcs1_start, PAGE_SIZE);

 I think you need to check the return value of ioremap_nocache (it looks like 
 it
 could return NULL), but I'm not certain of its semantics.

I guess we could, but I suppose it is somewhat unclear how we shall
proceed in the case of failure.

But regardless, this is not really changing any logic in the current code.


   /* setup dummy cascade chip for INTCS */
 - n = evt2irq(0xf80);
 + n = evt2irq(vect);
 +
   irq_alloc_desc_at(n, numa_node_id());
   irq_set_chip_and_handler_name(n, dummy_irq_chip,
 handle_level_irq, level);
 @@ -581,6 +588,65 @@ void __init sh7372_init_irq(void)
   iowrite16(0, intcs_ffd2 + 0x104);
  }

 +#ifdef CONFIG_OF
 +static unsigned short intevtsa_vect;
 +
 +#define INTC_RES_MAX 2
 +static struct {
 + struct intc_desc intc_desc;
 + struct resource intc_res[INTC_RES_MAX];
 +} intc_data __initdata;
 +
 +static int __init intc_of_init(struct device_node *np,
 +struct device_node *parent)
 +{
 + int ret, i;
 +
 + memset(intc_data, 0, sizeof(intc_data));
 +
 + for (i = 0; i  INTC_RES_MAX; i++) {
 + ret = of_address_to_resource(np, i, intc_data.intc_res[i]);
 + if (ret  0)
 + break;
 + }
 +
 + intc_data.intc_desc.name = (char *)of_node_full_name(np);
 + intc_data.intc_desc.resource = intc_data.intc_res;
 + intc_data.intc_desc.num_resources = i;
 +
 + ret = of_sh_intc_get_intc(np, intc_data.intc_desc);
 + if (ret)
 + return ret;
 +
 + of_sh_intc_get_intevtsa_vect(np, intevtsa_vect);
 +
 + register_intc_controller(intc_data.intc_desc);
 + return 0;
 +}

 You seem to have the same code for r8a7740. They should be consolidated.

The plan is to consolidate part of this code. There is also a chained

Re: [PATCH 1/8] SH: intc: Add support OF for INTC

2013-01-11 Thread Magnus Damm
Hi Mark,

On Wed, Jan 9, 2013 at 8:53 PM, Mark Rutland mark.rutl...@arm.com wrote:
 Hi,

 Thanks for updating the text, this is far easier to read than previously.

 However, I'm still concerned by how complex the binding seems. As I don't have
 any familiarity with the device, I don't know whether that's just an artifact
 of the hardware or something that can be cleared up.

Iwamatsu-san wrote this binding based on our C version of the INTC
tables. And I wrote the original INTC table code based on perhaps 30+
data sheet. They code was initially designed to allow people to input
data straight off the data sheet - this so we could support a wide
range of slightly different interrupt controllers.

 I think the approach used by the binding needs some serious review before this
 should be merged. It seems far more complex than any existing interrupt
 controller binding. Without a dts example for a complete board (complete with
 devices wired up to the interrupt controller), it's difficult to judge how 
 this
 will work in practice.

Feel free to review the code, but I am not sure why anyone would case
about this Renesas specific legacy interrupt controller. If I were to
chose how cycles should be spent then I think it is better to try to
come up with power domain DT bindings for all SoC vendors.

Also, there are the DT board code queued up that makes use of this controller.

Thanks,

/ magnus
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss