Hi Magnus,
On Sun, Sep 20, 2020 at 3:04 AM Magnus Damm <damm@xxxxxxxxxxxxx> wrote:
> From: Magnus Damm <damm+renesas@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxx>
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
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
