| 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
| ||
|
Date: Tue, 15 Oct 2019 23:31:46 -0400
From: Joshua Kinard <kumba@...too.org>
To: Thomas Bogendoerfer <tbogendoerfer@...e.de>,
Ralf Baechle <ralf@...ux-mips.org>,
Paul Burton <paul.burton@...s.com>,
James Hogan <jhogan@...nel.org>,
Alessandro Zummo <a.zummo@...ertech.it>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-rtc@...r.kernel.org
Subject: Re: [PATCH v2] rtc: ds1685: add indirect access method and remove
plat_read/plat_write
On 10/14/2019 17:46, Thomas Bogendoerfer wrote:
> SGI Octane (IP30) doesn't have RTC register directly mapped into CPU
> address space, but accesses RTC registers with an address and data
> register. This is now supported by additional access functions, which
> are selected by a new field in platform data. Removed plat_read/plat_write
> since there is no user and their usage could introduce lifetime issue,
> when functions are placed in different modules.
>
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@...e.de>
> ---
> Changes in v2:
>
> - check if rtc->read and rtc->write are setup
> - spell out indirect in function names and explain difference
> between standard and indirect functions
>
> arch/mips/sgi-ip32/ip32-platform.c | 2 +-
> drivers/rtc/rtc-ds1685.c | 78 +++++++++++++++++++++++++-------------
> include/linux/rtc/ds1685.h | 8 ++--
> 3 files changed, 58 insertions(+), 30 deletions(-)
>
> diff --git a/arch/mips/sgi-ip32/ip32-platform.c b/arch/mips/sgi-ip32/ip32-platform.c
> index 5a2a82148d8d..c3909bd8dd1a 100644
> --- a/arch/mips/sgi-ip32/ip32-platform.c
> +++ b/arch/mips/sgi-ip32/ip32-platform.c
> @@ -115,7 +115,7 @@ ip32_rtc_platform_data[] = {
> .bcd_mode = true,
> .no_irq = false,
> .uie_unsupported = false,
> - .alloc_io_resources = true,
> + .access_type = ds1685_reg_direct,
> .plat_prepare_poweroff = ip32_prepare_poweroff,
> },
> };
> diff --git a/drivers/rtc/rtc-ds1685.c b/drivers/rtc/rtc-ds1685.c
> index 349a8d1caca1..98d06b3ee913 100644
> --- a/drivers/rtc/rtc-ds1685.c
> +++ b/drivers/rtc/rtc-ds1685.c
> @@ -31,7 +31,10 @@
>
>
> /* ----------------------------------------------------------------------- */
> -/* Standard read/write functions if platform does not provide overrides */
> +/*
> + * Standard read/write
> + * all registers are mapped in CPU address space
> + */
>
> /**
> * ds1685_read - read a value from an rtc register.
> @@ -59,6 +62,35 @@ ds1685_write(struct ds1685_priv *rtc, int reg, u8 value)
> }
> /* ----------------------------------------------------------------------- */
>
> +/*
> + * Indirect read/write functions
> + * access happens via address and data register mapped in CPU address space
> + */
> +
> +/**
> + * ds1685_indirect_read - read a value from an rtc register.
> + * @rtc: pointer to the ds1685 rtc structure.
> + * @reg: the register address to read.
> + */
> +static u8
> +ds1685_indirect_read(struct ds1685_priv *rtc, int reg)
> +{
> + writeb(reg, rtc->regs);
> + return readb(rtc->data);
> +}
> +
> +/**
> + * ds1685_indirect_write - write a value to an rtc register.
> + * @rtc: pointer to the ds1685 rtc structure.
> + * @reg: the register address to write.
> + * @value: value to write to the register.
> + */
> +static void
> +ds1685_indirect_write(struct ds1685_priv *rtc, int reg, u8 value)
> +{
> + writeb(reg, rtc->regs);
> + writeb(value, rtc->data);
> +}
>
> /* ----------------------------------------------------------------------- */
> /* Inlined functions */
> @@ -1062,42 +1094,36 @@ ds1685_rtc_probe(struct platform_device *pdev)
> if (!rtc)
> return -ENOMEM;
>
> - /*
> - * Allocate/setup any IORESOURCE_MEM resources, if required. Not all
> - * platforms put the RTC in an easy-access place. Like the SGI Octane,
> - * which attaches the RTC to a "ByteBus", hooked to a SuperIO chip
> - * that sits behind the IOC3 PCI metadevice.
> - */
> - if (pdata->alloc_io_resources) {
> + /* Setup resources and access functions */
> + switch (pdata->access_type) {
> + case ds1685_reg_direct:
> + rtc->regs = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(rtc->regs))
> + return PTR_ERR(rtc->regs);
> + rtc->read = ds1685_read;
> + rtc->write = ds1685_write;
> + break;
> + case ds1685_reg_indirect:
> rtc->regs = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(rtc->regs))
> return PTR_ERR(rtc->regs);
> + rtc->data = devm_platform_ioremap_resource(pdev, 1);
> + if (IS_ERR(rtc->data))
> + return PTR_ERR(rtc->data);
> + rtc->read = ds1685_indirect_read;
> + rtc->write = ds1685_indirect_write;
> + break;
> }
>
> + if (!rtc->read || !rtc->write)
> + return -ENXIO;
> +
> /* Get the register step size. */
> if (pdata->regstep > 0)
> rtc->regstep = pdata->regstep;
> else
> rtc->regstep = 1;
>
> - /* Platform read function, else default if mmio setup */
> - if (pdata->plat_read)
> - rtc->read = pdata->plat_read;
> - else
> - if (pdata->alloc_io_resources)
> - rtc->read = ds1685_read;
> - else
> - return -ENXIO;
> -
> - /* Platform write function, else default if mmio setup */
> - if (pdata->plat_write)
> - rtc->write = pdata->plat_write;
> - else
> - if (pdata->alloc_io_resources)
> - rtc->write = ds1685_write;
> - else
> - return -ENXIO;
> -
> /* Platform pre-shutdown function, if defined. */
> if (pdata->plat_prepare_poweroff)
> rtc->prepare_poweroff = pdata->plat_prepare_poweroff;
> diff --git a/include/linux/rtc/ds1685.h b/include/linux/rtc/ds1685.h
> index 101c7adc05a2..67ee9d20cc5a 100644
> --- a/include/linux/rtc/ds1685.h
> +++ b/include/linux/rtc/ds1685.h
> @@ -42,6 +42,7 @@
> struct ds1685_priv {
> struct rtc_device *dev;
> void __iomem *regs;
> + void __iomem *data;
> u32 regstep;
> int irq_num;
> bool bcd_mode;
> @@ -70,12 +71,13 @@ struct ds1685_rtc_platform_data {
> const bool bcd_mode;
> const bool no_irq;
> const bool uie_unsupported;
> - const bool alloc_io_resources;
> - u8 (*plat_read)(struct ds1685_priv *, int);
> - void (*plat_write)(struct ds1685_priv *, int, u8);
> void (*plat_prepare_poweroff)(void);
> void (*plat_wake_alarm)(void);
> void (*plat_post_ram_clear)(void);
> + enum {
> + ds1685_reg_direct,
> + ds1685_reg_indirect
> + } access_type;
> };
>
>
> -- 2.16.4
>
Acked-by: Joshua Kinard <kumba@...too.org>
Reviewed-by: Joshua Kinard <kumba@...too.org>
Powered by blists - more mailing lists