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 <tbogendoer...@suse.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 <ku...@gentoo.org>
Reviewed-by: Joshua Kinard <ku...@gentoo.org>

Reply via email to