| Message ID | 160056201373.9912.14240039764408484073.sendpatchset@octo (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | r8a77961 CMT test setup using UIO | expand |
Hi Magnus, On Sun, Sep 20, 2020 at 3:04 AM Magnus Damm <damm@opensource.se> wrote: > From: Magnus Damm <damm+renesas@opensource.se> > > Device Tree modifications to expose CMT0 via UIO, code to add UIO driver > debug code and also adjust the compat string matching in MODULE_DEVICE_TABLE() > > Signed-off-by: Magnus Damm <damm+renesas@opensource.se> Thanks for your patch! > arch/arm64/boot/dts/renesas/r8a77961.dtsi | 10 ++++++++++ > drivers/uio/uio.c | 3 ++- > drivers/uio/uio_pdrv_genirq.c | 10 ++++++++-- > 3 files changed, 20 insertions(+), 3 deletions(-) Please don't mix DTS and driver changes in a single patch. > --- 0001/arch/arm64/boot/dts/renesas/r8a77961.dtsi > +++ work/arch/arm64/boot/dts/renesas/r8a77961.dtsi 2020-09-20 06:37:14.578864063 +0900 > @@ -453,6 +453,16 @@ > reg = <0 0xe6060000 0 0x50c>; > }; > > + cmt0: timer@e60f0000 { > + compatible = "uio_pdrv_genirq"; Please use an appropriate compatible value, describing the device. DT describes hardware, not software policy. > + reg = <0 0xe60f0000 0 0x1004>; > + interrupts = <GIC_SPI 142 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 303>; > + clock-names = "fck"; > + power-domains = <&sysc R8A77961_PD_ALWAYS_ON>; > + resets = <&cpg 303>; status = "disabled"; ? > + }; While at it, please add the other CMT instances, too. > + > cpg: clock-controller@e6150000 { > compatible = "renesas,r8a77961-cpg-mssr"; > reg = <0 0xe6150000 0 0x1000>; > --- 0001/drivers/uio/uio.c > +++ work/drivers/uio/uio.c 2020-09-20 07:58:51.295172430 +0900 > @@ -11,7 +11,7 @@ > * > * Base Functions > */ > - > +#define DEBUG I don't think this is appropriate for upstream inclusion. > --- 0001/drivers/uio/uio_pdrv_genirq.c > +++ work/drivers/uio/uio_pdrv_genirq.c 2020-09-20 07:58:07.417169667 +0900 > @@ -10,7 +10,7 @@ > * Copyright (C) 2008 by Digi International Inc. > * All rights reserved. > */ > - > +#define DEBUG Likewise. > #include <linux/platform_device.h> > #include <linux/uio_driver.h> > #include <linux/spinlock.h> > @@ -276,7 +282,7 @@ static const struct dev_pm_ops uio_pdrv_ > > #ifdef CONFIG_OF > static struct of_device_id uio_of_genirq_match[] = { > - { /* This is filled with module_parm */ }, > + { .compatible = "uio_pdrv_genirq", }, This does not look like a proper compatible value. > { /* Sentinel */ }, > }; > MODULE_DEVICE_TABLE(of, uio_of_genirq_match); Note that you can bind this driver to your device without modifying this driver, by using one of: 1. modprobe uio_pdrv_genirq of_id=my-compatible-value 2. echo uio_pdrv_genirq > /sys/bus/platform/devices/e60f0000.timer/driver_override echo e60f0000.timer > sys/bus/platform/drivers/uio_pdrv_genirq/bind Gr{oetje,eeting}s, Geert
--- 0001/arch/arm64/boot/dts/renesas/r8a77961.dtsi +++ work/arch/arm64/boot/dts/renesas/r8a77961.dtsi 2020-09-20 06:37:14.578864063 +0900 @@ -453,6 +453,16 @@ reg = <0 0xe6060000 0 0x50c>; }; + cmt0: timer@e60f0000 { + compatible = "uio_pdrv_genirq"; + reg = <0 0xe60f0000 0 0x1004>; + interrupts = <GIC_SPI 142 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 303>; + clock-names = "fck"; + power-domains = <&sysc R8A77961_PD_ALWAYS_ON>; + resets = <&cpg 303>; + }; + cpg: clock-controller@e6150000 { compatible = "renesas,r8a77961-cpg-mssr"; reg = <0 0xe6150000 0 0x1000>; --- 0001/drivers/uio/uio.c +++ work/drivers/uio/uio.c 2020-09-20 07:58:51.295172430 +0900 @@ -11,7 +11,7 @@ * * Base Functions */ - +#define DEBUG #include <linux/module.h> #include <linux/init.h> #include <linux/poll.h> @@ -975,6 +975,7 @@ int __uio_register_device(struct module * FDs at the time of unregister and therefore may not be * freed until they are released. */ + pr_debug("uio request_irq %lu\n", info->irq); ret = request_irq(info->irq, uio_interrupt, info->irq_flags, info->name, idev); if (ret) { --- 0001/drivers/uio/uio_pdrv_genirq.c +++ work/drivers/uio/uio_pdrv_genirq.c 2020-09-20 07:58:07.417169667 +0900 @@ -10,7 +10,7 @@ * Copyright (C) 2008 by Digi International Inc. * All rights reserved. */ - +#define DEBUG #include <linux/platform_device.h> #include <linux/uio_driver.h> #include <linux/spinlock.h> @@ -66,6 +66,8 @@ static irqreturn_t uio_pdrv_genirq_handl * remember the state so we can allow user space to enable it later. */ + pr_debug("irqhandler %d\n", irq); + spin_lock(&priv->lock); if (!__test_and_set_bit(UIO_IRQ_DISABLED, &priv->flags)) disable_irq_nosync(irq); @@ -87,6 +89,8 @@ static int uio_pdrv_genirq_irqcontrol(st * with irq handler on SMP systems. */ + pr_debug("irqcontrol %d\n", irq_on); + spin_lock_irqsave(&priv->lock, flags); if (irq_on) { if (__test_and_clear_bit(UIO_IRQ_DISABLED, &priv->flags)) @@ -172,6 +176,8 @@ static int uio_pdrv_genirq_probe(struct } } + pr_debug("uio irq %lu\n", uioinfo->irq); + if (uioinfo->irq) { struct irq_data *irq_data = irq_get_irq_data(uioinfo->irq); @@ -276,7 +282,7 @@ static const struct dev_pm_ops uio_pdrv_ #ifdef CONFIG_OF static struct of_device_id uio_of_genirq_match[] = { - { /* This is filled with module_parm */ }, + { .compatible = "uio_pdrv_genirq", }, { /* Sentinel */ }, }; MODULE_DEVICE_TABLE(of, uio_of_genirq_match);