Linux-RTC Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] rtc: ds1685: add indirect access method and remove plat_read/plat_write
@ 2019-10-14 21:46 Thomas Bogendoerfer
  2019-10-16  3:31 ` Joshua Kinard
  2019-10-16  8:40 ` Alexandre Belloni
  0 siblings, 2 replies; 3+ messages in thread
From: Thomas Bogendoerfer @ 2019-10-14 21:46 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton, James Hogan, Joshua Kinard,
	Alessandro Zummo, Alexandre Belloni, linux-mips, linux-kernel,
	linux-rtc

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@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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] rtc: ds1685: add indirect access method and remove plat_read/plat_write
  2019-10-14 21:46 [PATCH v2] rtc: ds1685: add indirect access method and remove plat_read/plat_write Thomas Bogendoerfer
@ 2019-10-16  3:31 ` Joshua Kinard
  2019-10-16  8:40 ` Alexandre Belloni
  1 sibling, 0 replies; 3+ messages in thread
From: Joshua Kinard @ 2019-10-16  3:31 UTC (permalink / raw)
  To: Thomas Bogendoerfer, Ralf Baechle, Paul Burton, James Hogan,
	Alessandro Zummo, Alexandre Belloni, linux-mips, linux-kernel,
	linux-rtc

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] rtc: ds1685: add indirect access method and remove plat_read/plat_write
  2019-10-14 21:46 [PATCH v2] rtc: ds1685: add indirect access method and remove plat_read/plat_write Thomas Bogendoerfer
  2019-10-16  3:31 ` Joshua Kinard
@ 2019-10-16  8:40 ` Alexandre Belloni
  1 sibling, 0 replies; 3+ messages in thread
From: Alexandre Belloni @ 2019-10-16  8:40 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: Ralf Baechle, Paul Burton, James Hogan, Joshua Kinard,
	Alessandro Zummo, linux-mips, linux-kernel, linux-rtc

On 14/10/2019 23:46:21+0200, 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@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(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-14 21:46 [PATCH v2] rtc: ds1685: add indirect access method and remove plat_read/plat_write Thomas Bogendoerfer
2019-10-16  3:31 ` Joshua Kinard
2019-10-16  8:40 ` Alexandre Belloni

Linux-RTC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rtc/0 linux-rtc/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-rtc linux-rtc/ https://lore.kernel.org/linux-rtc \
		linux-rtc@vger.kernel.org
	public-inbox-index linux-rtc

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-rtc


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git