linux-kernel - Re: [RFC PATCH] rtc/nxp: add FTM alarm driver as the wakeup source

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170127000329.jekcc3a3fxim4mbb@piout.net>
Date:   Fri, 27 Jan 2017 01:03:29 +0100
From:   Alexandre Belloni <alexandre.belloni@...e-electrons.com>
To:     Meng Yi <meng.yi@....com>
Cc:     a.zummo@...ertech.it, rtc-linux@...glegroups.com,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] rtc/nxp: add FTM alarm driver as the wakeup source

Hi,

On 24/10/2016 at 09:42:16 +0800, Meng Yi wrote:
> For the platforms including LS1021A, LS1043A that has the
> flextimer module, implementing alarm functions within RTC
> subsystem to wakeup the system when system going to sleep.
> Only Ftm0 can be used to wakeup the system.
> 
> Signed-off-by: Meng Yi <meng.yi@....com>
> ---
>  drivers/rtc/Kconfig       |   9 ++
>  drivers/rtc/Makefile      |   1 +
>  drivers/rtc/rtc-nxp-ftm.c | 255 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 265 insertions(+)
>  create mode 100644 drivers/rtc/rtc-nxp-ftm.c
> 
> +static const struct of_device_id nxp_ftm_rtc_of_match[] = {
> +	{
> +		.compatible	= "fsl,ftm-clock",
> +	}, {
> +		/* sentinel */
> +	}
> +};
> +MODULE_DEVICE_TABLE(of, nxp_ftm_rtc_of_match);
> 

I think the patch is generally fine but I think you will get an issue
later if anybody is trying to use an FTM to actually do something else
(e.g. PWM, counter, quad decoder,...).

I'm asking because there is exactly this issue on atmel SoCs.

I think we may be missing a proper subsystem.

> +static int nxp_ftm_rtc_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct resource *r;
> +	int irq;
> +	int ret;
> +
> +	rtc.alarm_freq = (u32)FIXED_FREQ_CLK / (u32)MAX_FREQ_DIV;
> +
> +	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!r)
> +		return -ENODEV;
> +
> +	rtc.base = devm_ioremap_resource(&pdev->dev, r);
> +	if (IS_ERR(rtc.base))
> +		return PTR_ERR(rtc.base);
> +
> +	irq = irq_of_parse_and_map(np, 0);
> +	if (irq <= 0) {
> +		pr_err("ftm: unable to get IRQ from DT, %d\n", irq);
> +		return -EINVAL;
> +	}
> +
> +	rtc.endian = of_property_read_bool(np, "big-endian");
> +
Doesn't the platform know the endianness, is it really necessary to have
that in the device tree?


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com