* Re: [RFT] PCI changes related to wakeup (was: Re: [linux-pm] ehci_hcd related S3 lockup on ASUS laptops, again)
From: Rafael J. Wysocki @ 2012-05-26 22:06 UTC (permalink / raw)
To: Andrey Rahmatullin
Cc: Steven Rostedt, linux-pm, Alan Stern, ACPI Devel Mailing List
In-Reply-To: <20120526214702.GE5012@belkar.wrar.name>
On Saturday, May 26, 2012, Andrey Rahmatullin wrote:
> On Sat, May 26, 2012 at 11:16:29PM +0200, Rafael J. Wysocki wrote:
> > Andrey, Stephen,
> >
> > We still have problems with this patch in https://bugzilla.kernel.org/show_bug.cgi?id=43278
> > so some more testing will be necessary, I'm afraid.
> >
> > I will send a series of ACPI and PCI patches I have collected so far,
> > that I'd like you to test on top of kernel 3.4.0 with commit
> > 151b61284776 reverted.
> >
> > Please let me know if suspend/wakeup work for you with these patches applied.
> I get the usual freeze on suspending with these patches.
I see.
Please try to unapply [4/4] and see if that helps. If it doesn't,
please try to enable the problematic USB controller to wake up
(e.g. by writing "enabled" to its /sys/power/.../wakeup file before the
suspend) and see if that helps.
Thanks,
Rafael
^ permalink raw reply
* Re: [RFT] PCI changes related to wakeup (was: Re: ehci_hcd related S3 lockup on ASUS laptops, again)
From: Andrey Rahmatullin @ 2012-05-26 22:36 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: ACPI Devel Mailing List, linux-pm, Steven Rostedt
In-Reply-To: <201205270006.01753.rjw@sisk.pl>
[-- Attachment #1.1: Type: text/plain, Size: 697 bytes --]
On Sun, May 27, 2012 at 12:06:01AM +0200, Rafael J. Wysocki wrote:
> > > Andrey, Stephen,
> > >
> > > We still have problems with this patch in https://bugzilla.kernel.org/show_bug.cgi?id=43278
> > > so some more testing will be necessary, I'm afraid.
> > >
> > > I will send a series of ACPI and PCI patches I have collected so far,
> > > that I'd like you to test on top of kernel 3.4.0 with commit
> > > 151b61284776 reverted.
> > >
> > > Please let me know if suspend/wakeup work for you with these patches applied.
> > I get the usual freeze on suspending with these patches.
>
> I see.
>
> Please try to unapply [4/4] and see if that helps.
It helps.
--
WBR, wRAR
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [RFT] PCI changes related to wakeup (was: Re: [linux-pm] ehci_hcd related S3 lockup on ASUS laptops, again)
From: Alan Stern @ 2012-05-26 22:40 UTC (permalink / raw)
To: Andrey Rahmatullin
Cc: Rafael J. Wysocki, Steven Rostedt, linux-pm,
ACPI Devel Mailing List
In-Reply-To: <20120526223611.GF5012@belkar.wrar.name>
On Sun, 27 May 2012, Andrey Rahmatullin wrote:
> On Sun, May 27, 2012 at 12:06:01AM +0200, Rafael J. Wysocki wrote:
> > > > Andrey, Stephen,
> > > >
> > > > We still have problems with this patch in https://bugzilla.kernel.org/show_bug.cgi?id=43278
> > > > so some more testing will be necessary, I'm afraid.
> > > >
> > > > I will send a series of ACPI and PCI patches I have collected so far,
> > > > that I'd like you to test on top of kernel 3.4.0 with commit
> > > > 151b61284776 reverted.
> > > >
> > > > Please let me know if suspend/wakeup work for you with these patches applied.
> > > I get the usual freeze on suspending with these patches.
> >
> > I see.
> >
> > Please try to unapply [4/4] and see if that helps.
> It helps.
Trying to follow the ACPI spec too closely may be a bad idea. We
should try to copy what Windows does (whatever that is!).
Alan Stern
^ permalink raw reply
* Re: [RFT] PCI changes related to wakeup (was: Re: ehci_hcd related S3 lockup on ASUS laptops, again)
From: Rafael J. Wysocki @ 2012-05-26 22:59 UTC (permalink / raw)
To: Alan Stern
Cc: Andrey Rahmatullin, linux-pm, Steven Rostedt,
ACPI Devel Mailing List
In-Reply-To: <Pine.LNX.4.44L0.1205261839180.3174-100000@netrider.rowland.org>
On Sunday, May 27, 2012, Alan Stern wrote:
> On Sun, 27 May 2012, Andrey Rahmatullin wrote:
>
> > On Sun, May 27, 2012 at 12:06:01AM +0200, Rafael J. Wysocki wrote:
> > > > > Andrey, Stephen,
> > > > >
> > > > > We still have problems with this patch in https://bugzilla.kernel.org/show_bug.cgi?id=43278
> > > > > so some more testing will be necessary, I'm afraid.
> > > > >
> > > > > I will send a series of ACPI and PCI patches I have collected so far,
> > > > > that I'd like you to test on top of kernel 3.4.0 with commit
> > > > > 151b61284776 reverted.
> > > > >
> > > > > Please let me know if suspend/wakeup work for you with these patches applied.
> > > > I get the usual freeze on suspending with these patches.
> > >
> > > I see.
> > >
> > > Please try to unapply [4/4] and see if that helps.
> > It helps.
>
> Trying to follow the ACPI spec too closely may be a bad idea. We
> should try to copy what Windows does (whatever that is!).
Well, that's why I made it a separate patch, among other things. :-)
So, do you think we should apply [1/4] and [2/4] and try to work around the
BIOS bug from https://bugzilla.kernel.org/show_bug.cgi?id=43278 (I suppose
we can do that by double checking if the target state returned by ACPI is
in agreement with the capabilities returned by the PCI layer)?
Rafael
^ permalink raw reply
* Re: [RFT] PCI changes related to wakeup (was: Re: [linux-pm] ehci_hcd related S3 lockup on ASUS laptops, again)
From: Alan Stern @ 2012-05-27 16:41 UTC (permalink / raw)
To: Andrey Rahmatullin
Cc: Rafael J. Wysocki, Steven Rostedt, linux-pm,
ACPI Devel Mailing List
In-Reply-To: <20120526223611.GF5012@belkar.wrar.name>
On Sun, 27 May 2012, Andrey Rahmatullin wrote:
> On Sun, May 27, 2012 at 12:06:01AM +0200, Rafael J. Wysocki wrote:
> > > > Andrey, Stephen,
> > > >
> > > > We still have problems with this patch in https://bugzilla.kernel.org/show_bug.cgi?id=43278
> > > > so some more testing will be necessary, I'm afraid.
> > > >
> > > > I will send a series of ACPI and PCI patches I have collected so far,
> > > > that I'd like you to test on top of kernel 3.4.0 with commit
> > > > 151b61284776 reverted.
> > > >
> > > > Please let me know if suspend/wakeup work for you with these patches applied.
> > > I get the usual freeze on suspending with these patches.
> >
> > I see.
> >
> > Please try to unapply [4/4] and see if that helps.
> It helps.
Andrey, can you try out Rafael's patches 1-3 (with 151b... reverted)
and see what happens if the EHCI controllers' power/wakeup sysfs
atributes are first set to "disabled"?
Alan Stern
^ permalink raw reply
* Re: [RFT] PCI changes related to wakeup (was: Re: [linux-pm] ehci_hcd related S3 lockup on ASUS laptops, again)
From: Andrey Rahmatullin @ 2012-05-27 21:17 UTC (permalink / raw)
To: Alan Stern
Cc: Rafael J. Wysocki, Steven Rostedt, linux-pm,
ACPI Devel Mailing List
In-Reply-To: <Pine.LNX.4.44L0.1205271238040.14937-100000@netrider.rowland.org>
[-- Attachment #1: Type: text/plain, Size: 955 bytes --]
On Sun, May 27, 2012 at 12:41:43PM -0400, Alan Stern wrote:
> > > > > Andrey, Stephen,
> > > > >
> > > > > We still have problems with this patch in https://bugzilla.kernel.org/show_bug.cgi?id=43278
> > > > > so some more testing will be necessary, I'm afraid.
> > > > >
> > > > > I will send a series of ACPI and PCI patches I have collected so far,
> > > > > that I'd like you to test on top of kernel 3.4.0 with commit
> > > > > 151b61284776 reverted.
> > > > >
> > > > > Please let me know if suspend/wakeup work for you with these patches applied.
> > > > I get the usual freeze on suspending with these patches.
> > >
> > > I see.
> > >
> > > Please try to unapply [4/4] and see if that helps.
> > It helps.
>
> Andrey, can you try out Rafael's patches 1-3 (with 151b... reverted)
> and see what happens if the EHCI controllers' power/wakeup sysfs
> atributes are first set to "disabled"?
It freezes.
--
WBR, wRAR
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 01/11] ARM: OMAP4: Remove un-used control module headers and defines.
From: Shilimkar, Santosh @ 2012-05-28 9:12 UTC (permalink / raw)
To: Eduardo Valentin
Cc: amit.kucheria, balbi, kishon, kbaidarov, linux-pm, linux-omap,
linux-arm-kernel
In-Reply-To: <1337934361-1606-2-git-send-email-eduardo.valentin@ti.com>
On Fri, May 25, 2012 at 1:55 PM, Eduardo Valentin
<eduardo.valentin@ti.com> wrote:
> From: Santosh Shilimkar <santosh.shilimkar@ti.com>
>
> Most of the OMAP4 control module register defines are not used and
> can be removed. Keep only needed defines and move them to common
> control module header just like other OMAP versions.
>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---
Thanks for picking this up in your series
Regards
Santposh
^ permalink raw reply
* Re: [RFC PATCH 02/11] ARM: OMAP: expose control.h to mach area
From: Shilimkar, Santosh @ 2012-05-28 9:25 UTC (permalink / raw)
To: Eduardo Valentin
Cc: amit.kucheria, balbi, kishon, kbaidarov, linux-pm, linux-omap,
linux-arm-kernel
In-Reply-To: <1337934361-1606-3-git-send-email-eduardo.valentin@ti.com>
On Fri, May 25, 2012 at 1:55 PM, Eduardo Valentin
<eduardo.valentin@ti.com> wrote:
> This patch exposes the definitions under control.h to
> drivers outside the machine code.
>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---
After second thought, this complete header movement needs to avoided.
Drivers should not anyway include something like <mach/control.h>
May be split the control.h header file data into ..
- defines used by mach-omap2/* files which can remain in "control.h"
in existing location.
- common functions/defines used across drivers/*, mach-omap2/*,
plat-omap/*, should
go to include/linux/omap_control.h
- Driver specific defines like thermal, usb etc, should go to
respective drivers file.
What do you think ?
Regards
santosh
^ permalink raw reply
* Re: [RFC PATCH 10/11] omap4: thermal: add basic CPU thermal zone
From: Shilimkar, Santosh @ 2012-05-28 9:33 UTC (permalink / raw)
To: Eduardo Valentin
Cc: amit.kucheria, balbi, kishon, kbaidarov, linux-pm, linux-omap,
linux-arm-kernel
In-Reply-To: <1337934361-1606-11-git-send-email-eduardo.valentin@ti.com>
On Fri, May 25, 2012 at 1:56 PM, Eduardo Valentin
<eduardo.valentin@ti.com> wrote:
> This patch exposes OMAP4 thermal sensor as a thermal zone
> named "cpu". Only thermal creation is done here.
>
> TODO:
>
> - Add cooling bindings
> - Add extrapolation rules
>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---
> drivers/thermal/Kconfig | 12 ++++++
> drivers/thermal/Makefile | 1 +
> drivers/thermal/omap-bandgap.c | 1 +
> drivers/thermal/omap-bandgap.h | 12 ++++++
> drivers/thermal/omap4-thermal.c | 72 +++++++++++++++++++++++++++++++++++++++
> 5 files changed, 98 insertions(+), 0 deletions(-)
> create mode 100644 drivers/thermal/omap4-thermal.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index ffdd240..2e82797 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -39,3 +39,15 @@ config OMAP_BANDGAP
> This includes alert interrupts generation and also the TSHUT
> support.
>
> +config OMAP4_THERMAL
> + bool "Texas Instruments OMAP4 thermal support"
> + depends on OMAP_BANDGAP
> + depends on ARCH_OMAP4
> + help
> + If you say yes here you get thermal support for the Texas Instruments
> + OMAP4 SoC family. The current chip supported are:
> + - OMAP4460
> +
It's more of IP feature than OMAP specific, so something like
config HAVE_BANDGAP_THERMAL_SUPPORT
and then let processor's which support enable it. That OMAP varients
in AMXX etc if needed can make use of it.
If you agree, then rest of the driver also can be cleaned to avoid
omap_* in file names and variables.
Apart from this minor comment, rest of the patch looks fine to me.
Regards
Santosh
^ permalink raw reply
* Re: [RFC PATCH 10/11] omap4: thermal: add basic CPU thermal zone
From: Felipe Balbi @ 2012-05-28 9:48 UTC (permalink / raw)
To: Shilimkar, Santosh
Cc: amit.kucheria, kbaidarov, kishon, balbi, linux-pm, linux-omap,
linux-arm-kernel
In-Reply-To: <CAMQu2gxNa4sPMKPLXGkKz_wkbQF8bTYipo4910p-ui9WNdeVFQ@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 2108 bytes --]
Hi,
On Mon, May 28, 2012 at 03:03:26PM +0530, Shilimkar, Santosh wrote:
> On Fri, May 25, 2012 at 1:56 PM, Eduardo Valentin
> <eduardo.valentin@ti.com> wrote:
> > This patch exposes OMAP4 thermal sensor as a thermal zone
> > named "cpu". Only thermal creation is done here.
> >
> > TODO:
> >
> > - Add cooling bindings
> > - Add extrapolation rules
> >
> > Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> > ---
> > drivers/thermal/Kconfig | 12 ++++++
> > drivers/thermal/Makefile | 1 +
> > drivers/thermal/omap-bandgap.c | 1 +
> > drivers/thermal/omap-bandgap.h | 12 ++++++
> > drivers/thermal/omap4-thermal.c | 72 +++++++++++++++++++++++++++++++++++++++
> > 5 files changed, 98 insertions(+), 0 deletions(-)
> > create mode 100644 drivers/thermal/omap4-thermal.c
> >
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > index ffdd240..2e82797 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -39,3 +39,15 @@ config OMAP_BANDGAP
> > This includes alert interrupts generation and also the TSHUT
> > support.
> >
> > +config OMAP4_THERMAL
> > + bool "Texas Instruments OMAP4 thermal support"
> > + depends on OMAP_BANDGAP
> > + depends on ARCH_OMAP4
> > + help
> > + If you say yes here you get thermal support for the Texas Instruments
> > + OMAP4 SoC family. The current chip supported are:
> > + - OMAP4460
> > +
> It's more of IP feature than OMAP specific, so something like
>
> config HAVE_BANDGAP_THERMAL_SUPPORT
>
> and then let processor's which support enable it. That OMAP varients
> in AMXX etc if needed can make use of it.
Those are just an OMAP in disguise.
> If you agree, then rest of the driver also can be cleaned to avoid
> omap_* in file names and variables.
>
> Apart from this minor comment, rest of the patch looks fine to me.
Then it will appear as a fully generic bandgap driver, which is not the
case. This is really a TI thing, right ?
--
balbi
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 05/11] mfd: omap: control: core system control driver
From: Shilimkar, Santosh @ 2012-05-28 9:54 UTC (permalink / raw)
To: Eduardo Valentin
Cc: amit.kucheria, balbi, kishon, kbaidarov, J Keerthy, linux-pm,
linux-omap, linux-arm-kernel
In-Reply-To: <1337934361-1606-6-git-send-email-eduardo.valentin@ti.com>
On Fri, May 25, 2012 at 1:55 PM, Eduardo Valentin
<eduardo.valentin@ti.com> wrote:
> This patch introduces a MFD core device driver for
> OMAP system control module.
>
> The control module allows software control of
> various static modes supported by the device. It is
> composed of two control submodules: general control
> module and device (padconfiguration) control
> module.
>
> In this patch, the children defined are for:
> . USB-phy pin control
> . Bangap temperature sensor
>
> Device driver is probed with postcore_initcall.
> However, as some of the APIs exposed by this driver
> may be needed in very early init phase, an early init
> class is also available: "early_omap_control".
>
> Signed-off-by: J Keerthy <j-keerthy@ti.com>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---
[..]
> diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
> index ad95c7a..222dbad 100644
> --- a/arch/arm/plat-omap/Kconfig
> +++ b/arch/arm/plat-omap/Kconfig
> @@ -5,6 +5,9 @@ menu "TI OMAP Common Features"
> config ARCH_OMAP_OTG
> bool
>
> +config ARCH_HAS_CONTROL_MODULE
> + bool
> +
Thanks for getting rid of OMAP CONFIG here.
> choice
> prompt "OMAP System Type"
> default ARCH_OMAP2PLUS
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 11e4438..25a66d8 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -795,6 +795,15 @@ config MFD_WL1273_CORE
> driver connects the radio-wl1273 V4L2 module and the wl1273
> audio codec.
>
> +config MFD_OMAP_CONTROL
> + bool "Texas Instruments OMAP System control module"
> + depends on ARCH_HAS_CONTROL_MODULE
> + help
> + This is the core driver for system control module. This driver
> + is responsible for creating the control module mfd child,
> + like USB-pin control, pin muxing, MMC-pbias and DDR IO dynamic
> + change for off mode.
> +
> config MFD_OMAP_USB_HOST
> bool "Support OMAP USBHS core driver"
> depends on USB_EHCI_HCD_OMAP || USB_OHCI_HCD_OMAP3
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 05fa538..00f99d6 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -106,6 +106,7 @@ obj-$(CONFIG_MFD_TPS6586X) += tps6586x.o
> obj-$(CONFIG_MFD_VX855) += vx855.o
> obj-$(CONFIG_MFD_WL1273_CORE) += wl1273-core.o
> obj-$(CONFIG_MFD_CS5535) += cs5535-mfd.o
> +obj-$(CONFIG_MFD_OMAP_CONTROL) += omap-control-core.o
> obj-$(CONFIG_MFD_OMAP_USB_HOST) += omap-usb-host.o
> obj-$(CONFIG_MFD_PM8921_CORE) += pm8921-core.o
> obj-$(CONFIG_MFD_PM8XXX_IRQ) += pm8xxx-irq.o
> diff --git a/drivers/mfd/omap-control-core.c b/drivers/mfd/omap-control-core.c
> new file mode 100644
> index 0000000..7d8d408
> --- /dev/null
> +++ b/drivers/mfd/omap-control-core.c
> @@ -0,0 +1,211 @@
> +/*
> + * OMAP system control module driver file
> + *
> + * Copyright (C) 2011-2012 Texas Instruments Incorporated - http://www.ti.com/
> + * Contacts:
> + * Based on original code written by:
> + * J Keerthy <j-keerthy@ti.com>
> + * Moiz Sonasath <m-sonasath@ti.com>
> + * MFD clean up and re-factoring:
> + * Eduardo Valentin <eduardo.valentin@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + */
> +
> +#include <linux/module.h>
> +#include <linux/export.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/io.h>
> +#include <linux/err.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_address.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/omap_control.h>
> +
> +static struct omap_control *omap_control_module;
> +
> +/**
> + * omap_control_readl: Read a single omap control module register.
> + *
> + * @dev: device to read from.
> + * @reg: register to read.
> + * @val: output with register value.
> + *
> + * returns 0 on success or -EINVAL in case struct device is invalid.
> + */
> +int omap_control_readl(struct device *dev, u32 reg, u32 *val)
> +{
> + struct omap_control *omap_control = dev_get_drvdata(dev);
> +
> + if (!omap_control)
> + return -EINVAL;
> +
> + *val = readl(omap_control->base + reg);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(omap_control_readl);
> +
I might have missed in the last scan, but can you let
function return the register value.
I am guessing, you did this for error case handling. You might
want to stick to read API semantic and just have WARN_ON()
to take care of error case.
> +/**
> + * omap_control_writel: Write a single omap control module register.
> + *
> + * @dev: device to read from.
> + * @val: value to write.
> + * @reg: register to write to.
> + *
> + * returns 0 on success or -EINVAL in case struct device is invalid.
> + */
> +int omap_control_writel(struct device *dev, u32 val, u32 reg)
> +{
> + struct omap_control *omap_control = dev_get_drvdata(dev);
> + unsigned long flags;
> +
> + if (!omap_control)
> + return -EINVAL;
> +
> + spin_lock_irqsave(&omap_control->reg_lock, flags);
> + writel(val, omap_control->base + reg);
> + spin_unlock_irqrestore(&omap_control->reg_lock, flags);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(omap_control_writel);
> +
> +/**
> + * omap_control_get: returns the control module device pinter
> + *
> + * The modules which has to use control module API's to read or write should
> + * call this API to get the control module device pointer.
> + */
> +struct device *omap_control_get(void)
> +{
> + unsigned long flags;
> +
> + if (!omap_control_module)
> + return ERR_PTR(-ENODEV);
> +
> + spin_lock_irqsave(&omap_control_module->reg_lock, flags);
> + omap_control_module->use_count++;
> + spin_unlock_irqrestore(&omap_control_module->reg_lock, flags);
> +
> + return omap_control_module->dev;
> +}
> +EXPORT_SYMBOL_GPL(omap_control_get);
> +
> +/**
> + * omap_control_put: returns the control module device pinter
> + *
> + * The modules which has to use control module API's to read or write should
> + * call this API to get the control module device pointer.
> + */
> +void omap_control_put(struct device *dev)
> +{
> + struct omap_control *omap_control = dev_get_drvdata(dev);
> + unsigned long flags;
> +
> + if (!omap_control)
> + return;
> +
> + spin_lock_irqsave(&omap_control->reg_lock, flags);
> + omap_control->use_count--;
> + spin_unlock_irqrestore(&omap_control_module->reg_lock, flags);
> +}
> +EXPORT_SYMBOL_GPL(omap_control_put);
> +
> +static const struct of_device_id of_omap_control_match[] = {
> + { .compatible = "ti,omap3-control", },
> + { .compatible = "ti,omap4-control", },
> + { .compatible = "ti,omap5-control", },
> + { },
> +};
> +
> +static int __devinit omap_control_probe(struct platform_device *pdev)
> +{
> + struct resource *res;
> + void __iomem *base;
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct omap_control *omap_control;
> +
> + omap_control = devm_kzalloc(dev, sizeof(*omap_control), GFP_KERNEL);
> + if (!omap_control) {
> + dev_err(dev, "not enough memory for omap_control\n");
> + return -ENOMEM;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(dev, "missing memory base resource\n");
> + return -EINVAL;
> + }
> +
> + base = devm_request_and_ioremap(dev, res);
> + if (!base) {
> + dev_err(dev, "ioremap failed\n");
> + return -EADDRNOTAVAIL;
> + }
> +
> + omap_control->base = base;
> + omap_control->dev = dev;
> + spin_lock_init(&omap_control->reg_lock);
> +
> + platform_set_drvdata(pdev, omap_control);
> + omap_control_module = omap_control;
> +
> + return of_platform_populate(np, of_omap_control_match, NULL, dev);
> +}
> +
Will the probe get called on multiple devices and race ?
> +static int __devexit omap_control_remove(struct platform_device *pdev)
> +{
> + struct omap_control *omap_control = platform_get_drvdata(pdev);
> +
> + spin_lock(&omap_control->reg_lock);
> + if (omap_control->use_count > 0) {
> + spin_unlock(&omap_control->reg_lock);
> + dev_err(&pdev->dev, "device removed while still being used\n");
> + return -EBUSY;
> + }
> + spin_unlock(&omap_control->reg_lock);
> +
Do you really need above lock where you are just doing the
register read. smp_rmb(), should be enough, I guess.
Regards
Santosh
^ permalink raw reply
* Re: [RFC PATCH 04/11] OMAP: Add early device for system control module
From: Shilimkar, Santosh @ 2012-05-28 9:58 UTC (permalink / raw)
To: Eduardo Valentin
Cc: b-cousson, kishon, kbaidarov, tony, paul, balbi, amit.kucheria,
linux-pm, linux-arm-kernel, linux-omap, amit.kachhap
In-Reply-To: <1337934361-1606-5-git-send-email-eduardo.valentin@ti.com>
On Fri, May 25, 2012 at 1:55 PM, Eduardo Valentin
<eduardo.valentin@ti.com> wrote:
> This is a way to add an early device for system control module.
> the code is also requesting for driver registration and probing.
> Done at early_initcall because at that time, ioremapping is possible.
>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---
> arch/arm/mach-omap2/devices.c | 29 +++++++++++++++++++++++++++++
> 1 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index 9332673..58cc5c3 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -40,6 +40,35 @@
> #define L3_MODULES_MAX_LEN 12
> #define L3_MODULES 3
>
> +static struct resource control_resources[] = {
> + [0] = {
> + .start = 0x4a002000,
> + .end = 0x4a0027ff,
> + .flags = IORESOURCE_MEM,
> + },
> +};
> +static struct platform_device control_device = {
> + .name = "omap-control-core",
> + .id = 0,
> + .resource = control_resources,
> + .num_resources = ARRAY_SIZE(control_resources),
> +};
> +
> +static struct platform_device *early_devices[] __initdata = {
> + &control_device,
> +};
> +
> +static int __init plat_early_device_setup(void)
> +{
> + early_platform_add_devices(early_devices,
> + ARRAY_SIZE(early_devices));
> + early_platform_driver_register_all("early_omap_control");
> + early_platform_driver_probe("early_omap_control", 1, false);
> +
> + return 0;
> +}
> +early_initcall(plat_early_device_setup);
> +
> static int omap_init_control(void)
> {
> struct omap_hwmod *oh;
Nice !!
Apart from fix from Konstantin and Benoit's DT comment,
patch looks fine to me.
Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH 06/11] OMAP2+: use control module mfd driver in omap_type
From: Shilimkar, Santosh @ 2012-05-28 10:02 UTC (permalink / raw)
To: Cousson, Benoit
Cc: amit.kucheria, balbi, kishon, kbaidarov, linux-pm, linux-omap,
linux-arm-kernel
In-Reply-To: <4FBF80D5.2000600@ti.com>
On Fri, May 25, 2012 at 6:23 PM, Cousson, Benoit <b-cousson@ti.com> wrote:
> Hi Eduardo,
>
>
> On 5/25/2012 10:25 AM, Eduardo Valentin wrote:
>>
>> OMAP system control module can be probed early, then
>> omap_type is safe to use its APIs.
>>
>> TODO: add support for other omap versions
>>
>> Signed-off-by: Eduardo Valentin<eduardo.valentin@ti.com>
>> ---
>> arch/arm/mach-omap2/id.c | 16 +++++++++++++++-
>> 1 files changed, 15 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
>> index 5bb9746..acfd698 100644
>> --- a/arch/arm/mach-omap2/id.c
>> +++ b/arch/arm/mach-omap2/id.c
>> @@ -18,6 +18,7 @@
>> #include<linux/kernel.h>
>> #include<linux/init.h>
>> #include<linux/io.h>
>> +#include<linux/mfd/omap_control.h>
>>
>> #include<asm/cputype.h>
>>
>> @@ -40,8 +41,14 @@ EXPORT_SYMBOL(omap_rev);
>>
>> int omap_type(void)
>> {
>> + struct device *scm;
>> + int ret = 0;
>> u32 val = 0;
>>
>> + scm = omap_control_get();
>> + if (IS_ERR_OR_NULL(scm))
>> + return 0;
>> +
>> if (cpu_is_omap24xx()) {
>
>
> OK, not really related to that patch, but the previous cpu_is_omap24xx makes
> me think of that :-)
>
> What about the omap<X>_check_revision used by cpu_is_XXX?
>
> This call is the very first one to require the control module access in
> order to get the ID_CODE inside the control module.
>
> So far it still use that ugly hard coded phys -> virtual address macro that
> is sued for that.
>
Agree with Benoits comment. One way to deal with this is,
store the register offset with init and then just use it here.
That way you can get rid of all cpu_is_XXXX() from this function.
Regards
Santosh
^ permalink raw reply
* Re: [RFC PATCH 08/11] ARM: OMAP4+: Adding the temperature sensor register set bit fields
From: Shilimkar, Santosh @ 2012-05-28 10:04 UTC (permalink / raw)
To: Eduardo Valentin
Cc: amit.kucheria, balbi, kishon, kbaidarov, Keerthy, linux-pm,
linux-omap, linux-arm-kernel
In-Reply-To: <1337934361-1606-9-git-send-email-eduardo.valentin@ti.com>
On Fri, May 25, 2012 at 1:55 PM, Eduardo Valentin
<eduardo.valentin@ti.com> wrote:
> OMAP4460 specific temperature sensor register bit fields are added.
> Existing OMAP4 entries are renamed to OMAP4430.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> ---
Once you split the header, as mentioned in earlier comment,
you can move those defines in driver file directly.
Regards
Santsoh
^ permalink raw reply
* Re: [RFC PATCH 10/11] omap4: thermal: add basic CPU thermal zone
From: Valentin, Eduardo @ 2012-05-28 10:26 UTC (permalink / raw)
To: balbi
Cc: Shilimkar, Santosh, b-cousson, kishon, kbaidarov, tony, paul,
amit.kucheria, linux-pm, linux-arm-kernel, linux-omap,
amit.kachhap
In-Reply-To: <20120528094850.GS18918@arwen.pp.htv.fi>
Hello Santosh,
On Mon, May 28, 2012 at 12:48 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Mon, May 28, 2012 at 03:03:26PM +0530, Shilimkar, Santosh wrote:
>> On Fri, May 25, 2012 at 1:56 PM, Eduardo Valentin
>> <eduardo.valentin@ti.com> wrote:
>> > This patch exposes OMAP4 thermal sensor as a thermal zone
>> > named "cpu". Only thermal creation is done here.
>> >
>> > TODO:
>> >
>> > - Add cooling bindings
>> > - Add extrapolation rules
>> >
>> > Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
>> > ---
>> > drivers/thermal/Kconfig | 12 ++++++
>> > drivers/thermal/Makefile | 1 +
>> > drivers/thermal/omap-bandgap.c | 1 +
>> > drivers/thermal/omap-bandgap.h | 12 ++++++
>> > drivers/thermal/omap4-thermal.c | 72 +++++++++++++++++++++++++++++++++++++++
>> > 5 files changed, 98 insertions(+), 0 deletions(-)
>> > create mode 100644 drivers/thermal/omap4-thermal.c
>> >
>> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> > index ffdd240..2e82797 100644
>> > --- a/drivers/thermal/Kconfig
>> > +++ b/drivers/thermal/Kconfig
>> > @@ -39,3 +39,15 @@ config OMAP_BANDGAP
>> > This includes alert interrupts generation and also the TSHUT
>> > support.
>> >
>> > +config OMAP4_THERMAL
>> > + bool "Texas Instruments OMAP4 thermal support"
>> > + depends on OMAP_BANDGAP
>> > + depends on ARCH_OMAP4
>> > + help
>> > + If you say yes here you get thermal support for the Texas Instruments
>> > + OMAP4 SoC family. The current chip supported are:
>> > + - OMAP4460
>> > +
>> It's more of IP feature than OMAP specific, so something like
>>
>> config HAVE_BANDGAP_THERMAL_SUPPORT
>>
>> and then let processor's which support enable it. That OMAP varients
>> in AMXX etc if needed can make use of it.
>
> Those are just an OMAP in disguise.
>
>> If you agree, then rest of the driver also can be cleaned to avoid
>> omap_* in file names and variables.
>>
>> Apart from this minor comment, rest of the patch looks fine to me.
>
> Then it will appear as a fully generic bandgap driver, which is not the
> case. This is really a TI thing, right ?
I have to agree with Felipe here. I do see your point to have a name
which is applicable to AMxx, but having such generic naming is not
helping either :-( as this bandgap driver is not supposed to be
generic.
I though of "ti_bandgap.*", but still seams to be misleading..
I guess same applies to usb_phy and scm.
Again, if you have a name that fits better I won't hesitate to use it :-)
>
> --
> balbi
--
Eduardo Valentin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH 02/11] ARM: OMAP: expose control.h to mach area
From: Valentin, Eduardo @ 2012-05-28 10:30 UTC (permalink / raw)
To: Shilimkar, Santosh
Cc: amit.kucheria, balbi, kishon, kbaidarov, linux-pm, linux-omap,
linux-arm-kernel
In-Reply-To: <CAMQu2gxbkFCtkK+NCVksC82-R4SAKKtK2w=zNbrNi9a10s1t1Q@mail.gmail.com>
Hello,
On Mon, May 28, 2012 at 12:25 PM, Shilimkar, Santosh
<santosh.shilimkar@ti.com> wrote:
> On Fri, May 25, 2012 at 1:55 PM, Eduardo Valentin
> <eduardo.valentin@ti.com> wrote:
>> This patch exposes the definitions under control.h to
>> drivers outside the machine code.
>>
>> Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
>> ---
> After second thought, this complete header movement needs to avoided.
> Drivers should not anyway include something like <mach/control.h>
>
> May be split the control.h header file data into ..
> - defines used by mach-omap2/* files which can remain in "control.h"
> in existing location.
OK..
> - common functions/defines used across drivers/*, mach-omap2/*,
> plat-omap/*, should
> go to include/linux/omap_control.h
Right..
> - Driver specific defines like thermal, usb etc, should go to
> respective drivers file.
Indeed.
>
> What do you think ?
I think we are in line. And I believe I saw a similar comment by
Benoit in other email thread.
Having a better thinking of this, it makes sense to have the
definition specific to drivers in the driver scope only, as they are
going to be used only there anyway.
I will drop this patch off and update the remaining changes
accordingly (drop the change in control.h for thermal specific and
move it to omap_bandgap.h).
>
> Regards
> santosh
--
Eduardo Valentin
^ permalink raw reply
* Re: [RFC PATCH 09/11] ARM: OMAP4+: thermal: introduce bandgap temperature sensor
From: Eduardo Valentin @ 2012-05-28 10:55 UTC (permalink / raw)
To: Konstantin Baydarov
Cc: Keerthy, balbi, kishon, amit.kucheria, linux-pm, linux-omap,
linux-arm-kernel
In-Reply-To: <4FBFB5AD.6000005@dev.rtsoft.ru>
Hello Konstantin,
On Fri, May 25, 2012 at 08:39:09PM +0400, Konstantin Baydarov wrote:
> Hi.
> On 05/25/2012 12:25 PM, Eduardo Valentin wrote:
> > In the System Control Module, OMAP supplies a voltage reference
> > and a temperature sensor feature that are gathered in the band
> > gap voltage and temperature sensor (VBGAPTS) module. The band
> > gap provides current and voltage reference for its internal
> > circuits and other analog IP blocks. The analog-to-digital
> > converter (ADC) produces an output value that is proportional
> > to the silicon temperature.
> >
> > This patch provides a platform driver which expose this feature.
> > It is moduled as a MFD child of the System Control Module core
> > MFD driver.
> >
> > This driver provides only APIs to access the device properties,
> > like temperature, thresholds and update rate.
> >
> > Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> > Signed-off-by: Keerthy <j-keerthy@ti.com>
> > ---
> > .../devicetree/bindings/thermal/omap_bandgap.txt | 27 +
> > drivers/thermal/Kconfig | 13 +
> > drivers/thermal/Makefile | 4 +-
> > drivers/thermal/omap-bandgap.c | 1601 ++++++++++++++++++++
> > drivers/thermal/omap-bandgap.h | 63 +
> > 5 files changed, 1707 insertions(+), 1 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/thermal/omap_bandgap.txt
> > create mode 100644 drivers/thermal/omap-bandgap.c
> > create mode 100644 drivers/thermal/omap-bandgap.h
> >
> >
> Add private spin lock in omap-bandgap driver to prevent blocking of
> control module general registers access.
Thanks for sending this out. I will rethink these drivers wrt to locking and will
your proposal.
> I wasn't able to test - I have panda 4430 board.
Right, we definetly need to have it probing on 4430, I will check how we can close on that.
>
> TODO:
> Prevent over-usage of spin_lock/spin_unlock for sequential calls of
> bg_writel().
OK...
>
> Signed-off-by: Konstantin Baydarov <kbaidarov@dev.rtsoft.ru>
>
> Index: omap-thermal/drivers/mfd/omap-control-core.c
> ===================================================================
> --- omap-thermal.orig/drivers/mfd/omap-control-core.c
> +++ omap-thermal/drivers/mfd/omap-control-core.c
> @@ -67,6 +67,19 @@ EXPORT_SYMBOL_GPL(omap_control_readl);
> int omap_control_writel(struct device *dev, u32 val, u32 reg)
> {
> struct omap_control *omap_control = dev_get_drvdata(dev);
> +
> + if (!omap_control)
> + return -EINVAL;
> +
> + writel(val, omap_control->base + reg);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(omap_control_writel);
> +
> +int omap_control_lock_writel(struct device *dev, u32 val, u32 reg)
> +{
> + struct omap_control *omap_control = dev_get_drvdata(dev);
> unsigned long flags;
>
> if (!omap_control)
> @@ -78,7 +91,7 @@ int omap_control_writel(struct device *d
>
> return 0;
> }
> -EXPORT_SYMBOL_GPL(omap_control_writel);
> +EXPORT_SYMBOL_GPL(omap_control_lock_writel);
>
> /**
> * omap_control_get: returns the control module device pinter
> @@ -136,6 +149,9 @@ static int __devinit omap_control_probe(
> struct device_node *np = dev->of_node;
> struct omap_control *omap_control;
>
> + printk("\n\t\t **** omap_control_probe(): enter ");
> + dump_stack();
> +
> omap_control = devm_kzalloc(dev, sizeof(*omap_control), GFP_KERNEL);
> if (!omap_control) {
> dev_err(dev, "not enough memory for omap_control\n");
> Index: omap-thermal/drivers/thermal/omap-bandgap.c
> ===================================================================
> --- omap-thermal.orig/drivers/thermal/omap-bandgap.c
> +++ omap-thermal/drivers/thermal/omap-bandgap.c
> @@ -154,6 +154,7 @@ struct temp_sensor_registers {
> u32 status_cold_mask;
>
> u32 bgap_efuse;
> + spinlock_t bg_reg_lock;
Indeed. I agree we should have this per sensor. Need to revisit the existing mutex as well though.
> };
>
> /**
> @@ -579,6 +580,17 @@ omap5430_adc_to_temp[OMAP5430_ADC_END_VA
> 124600, 124900, 125000, 125000, 125000, 125000,
> };
>
> +static int bg_writel(struct device *dev, u32 val, u32 reg, spinlock_t *lock)
> +{
> + unsigned long flags;
> + int ret;
> +
> + spin_lock_irqsave(lock, flags);
> + ret = omap_control_writel(dev, val, reg);
> + spin_unlock_irqrestore(lock, flags);
In fact this doesn't help much in the bulk writes case. As you have already mentioned,
ideally we should lock, perform the operation, then unlock, instead of several
lock/write/unlock sequences.
> + return ret;
> +}
> +
> static irqreturn_t talert_irq_handler(int irq, void *data)
> {
> struct omap_bandgap *bg_ptr = data;
> @@ -615,7 +627,7 @@ static irqreturn_t talert_irq_handler(in
> ctrl |= tsr->mask_hot_mask;
> }
>
> - r |= omap_control_writel(cdev, ctrl, tsr->bgap_mask_ctrl);
> + r |= bg_writel(cdev, ctrl, tsr->bgap_mask_ctrl, &tsr->bg_reg_lock);
>
> if (r) {
> dev_err(bg_ptr->dev, "failed to ack talert interrupt\n");
> @@ -705,7 +717,7 @@ static int temp_sensor_unmask_interrupts
> reg_val |= tsr->mask_cold_mask;
> else
> reg_val &= ~tsr->mask_cold_mask;
> - err |= omap_control_writel(cdev, reg_val, tsr->bgap_mask_ctrl);
> + err |= bg_writel(cdev, reg_val, tsr->bgap_mask_ctrl, &tsr->bg_reg_lock);
>
> if (err) {
> dev_err(bg_ptr->dev, "failed to unmask interrupts\n");
> @@ -751,14 +763,14 @@ int temp_sensor_configure_thot(struct om
> /* write the new t_cold value */
> reg_val = thresh_val & (~tsr->threshold_tcold_mask);
> reg_val |= cold << __ffs(tsr->threshold_tcold_mask);
> - err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
> + err |= bg_writel(cdev, reg_val, tsr->bgap_threshold, &tsr->bg_reg_lock);
> thresh_val = reg_val;
> }
>
> /* write the new t_hot value */
> reg_val = thresh_val & ~tsr->threshold_thot_mask;
> reg_val |= (t_hot << __ffs(tsr->threshold_thot_mask));
> - err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
> + err |= bg_writel(cdev, reg_val, tsr->bgap_threshold, &tsr->bg_reg_lock);
> if (err) {
> dev_err(bg_ptr->dev, "failed to reprogram thot threshold\n");
> return -EIO;
> @@ -782,7 +794,7 @@ int temp_sensor_init_talert_thresholds(s
> /* write the new t_cold value */
> reg_val = thresh_val & ~tsr->threshold_tcold_mask;
> reg_val |= (t_cold << __ffs(tsr->threshold_tcold_mask));
> - err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
> + err |= bg_writel(cdev, reg_val, tsr->bgap_threshold, &tsr->bg_reg_lock);
> if (err) {
> dev_err(bg_ptr->dev, "failed to reprogram tcold threshold\n");
> return -EIO;
> @@ -793,7 +805,7 @@ int temp_sensor_init_talert_thresholds(s
> /* write the new t_hot value */
> reg_val = thresh_val & ~tsr->threshold_thot_mask;
> reg_val |= (t_hot << __ffs(tsr->threshold_thot_mask));
> - err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
> + err |= bg_writel(cdev, reg_val, tsr->bgap_threshold, &tsr->bg_reg_lock);
> if (err) {
> dev_err(bg_ptr->dev, "failed to reprogram thot threshold\n");
> return -EIO;
> @@ -802,7 +814,7 @@ int temp_sensor_init_talert_thresholds(s
> err = omap_control_readl(cdev, tsr->bgap_mask_ctrl, ®_val);
> reg_val |= tsr->mask_hot_mask;
> reg_val |= tsr->mask_cold_mask;
> - err |= omap_control_writel(cdev, reg_val, tsr->bgap_mask_ctrl);
> + err |= bg_writel(cdev, reg_val, tsr->bgap_mask_ctrl, &tsr->bg_reg_lock);
> if (err) {
> dev_err(bg_ptr->dev, "failed to reprogram thot threshold\n");
> return -EIO;
> @@ -833,14 +845,14 @@ int temp_sensor_configure_tcold(struct o
> /* write the new t_hot value */
> reg_val = thresh_val & (~tsr->threshold_thot_mask);
> reg_val |= hot << __ffs(tsr->threshold_thot_mask);
> - err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
> + err |= bg_writel(cdev, reg_val, tsr->bgap_threshold, &tsr->bg_reg_lock);
> thresh_val = reg_val;
> }
>
> /* write the new t_cold value */
> reg_val = thresh_val & ~tsr->threshold_tcold_mask;
> reg_val |= (t_cold << __ffs(tsr->threshold_tcold_mask));
> - err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
> + err |= bg_writel(cdev, reg_val, tsr->bgap_threshold, &tsr->bg_reg_lock);
> if (err) {
> dev_err(bg_ptr->dev, "failed to reprogram tcold threshold\n");
> return -EIO;
> @@ -861,7 +873,7 @@ static int temp_sensor_configure_tshut_h
> err = omap_control_readl(cdev, tsr->tshut_threshold, ®_val);
> reg_val &= ~tsr->tshut_hot_mask;
> reg_val |= tshut_hot << __ffs(tsr->tshut_hot_mask);
> - err |= omap_control_writel(cdev, reg_val, tsr->tshut_threshold);
> + err |= bg_writel(cdev, reg_val, tsr->tshut_threshold, &tsr->bg_reg_lock);
> if (err) {
> dev_err(bg_ptr->dev, "failed to reprogram tshut thot\n");
> return -EIO;
> @@ -882,7 +894,7 @@ static int temp_sensor_configure_tshut_c
> err = omap_control_readl(cdev, tsr->tshut_threshold, ®_val);
> reg_val &= ~tsr->tshut_cold_mask;
> reg_val |= tshut_cold << __ffs(tsr->tshut_cold_mask);
> - err |= omap_control_writel(cdev, reg_val, tsr->tshut_threshold);
> + err |= bg_writel(cdev, reg_val, tsr->tshut_threshold, &tsr->bg_reg_lock);
> if (err) {
> dev_err(bg_ptr->dev, "failed to reprogram tshut tcold\n");
> return -EIO;
> @@ -903,7 +915,7 @@ static int configure_temp_sensor_counter
> err = omap_control_readl(cdev, tsr->bgap_counter, &val);
> val &= ~tsr->counter_mask;
> val |= counter << __ffs(tsr->counter_mask);
> - err |= omap_control_writel(cdev, val, tsr->bgap_counter);
> + err |= bg_writel(cdev, val, tsr->bgap_counter, &tsr->bg_reg_lock);
> if (err) {
> dev_err(bg_ptr->dev, "failed to reprogram tshut tcold\n");
> return -EIO;
> @@ -1124,7 +1136,7 @@ static int enable_continuous_mode(struct
> tsr = bg_ptr->pdata->sensors[i].registers;
> r = omap_control_readl(cdev, tsr->bgap_mode_ctrl, &val);
> val |= 1 << __ffs(tsr->mode_ctrl_mask);
> - r |= omap_control_writel(cdev, val, tsr->bgap_mode_ctrl);
> + r |= bg_writel(cdev, val, tsr->bgap_mode_ctrl, &tsr->bg_reg_lock);
> if (r)
> dev_err(bg_ptr->dev, "could not save sensor %d\n", i);
> }
> @@ -1342,6 +1354,9 @@ int __devinit omap_bandgap_probe(struct
> u32 val;
>
> tsr = bg_ptr->pdata->sensors[i].registers;
> + /* Initialize register lock */
> + spin_lock_init(&tsr->bg_reg_lock);
> +
> /*
> * check if the efuse has a non-zero value if not
> * it is an untrimmed sample and the temperatures
> @@ -1482,12 +1497,12 @@ omap_bandgap_force_single_read(struct om
> /* Select single conversion mode */
> err = omap_control_readl(cdev, tsr->bgap_mode_ctrl, &temp);
> temp &= ~(1 << __ffs(tsr->mode_ctrl_mask));
> - omap_control_writel(cdev, temp, tsr->bgap_mode_ctrl);
> + bg_writel(cdev, temp, tsr->bgap_mode_ctrl, &tsr->bg_reg_lock);
>
> /* Start of Conversion = 1 */
> err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl, &temp);
> temp |= 1 << __ffs(tsr->bgap_soc_mask);
> - omap_control_writel(cdev, temp, tsr->temp_sensor_ctrl);
> + bg_writel(cdev, temp, tsr->temp_sensor_ctrl, &tsr->bg_reg_lock);
> /* Wait until DTEMP is updated */
> err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl, &temp);
> temp &= (tsr->bgap_dtemp_mask);
> @@ -1498,7 +1513,7 @@ omap_bandgap_force_single_read(struct om
> /* Start of Conversion = 0 */
> err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl, &temp);
> temp &= ~(1 << __ffs(tsr->bgap_soc_mask));
> - err |= omap_control_writel(cdev, temp, tsr->temp_sensor_ctrl);
> + err |= bg_writel(cdev, temp, tsr->temp_sensor_ctrl, &tsr->bg_reg_lock);
>
> return err ? -EIO : 0;
> }
> @@ -1519,20 +1534,20 @@ static int omap_bandgap_restore_ctxt(str
>
> err = omap_control_readl(cdev, tsr->bgap_counter, &val);
> if (val == 0) {
> - err |= omap_control_writel(cdev, rval->bg_threshold,
> - tsr->bgap_threshold);
> - err |= omap_control_writel(cdev, rval->tshut_threshold,
> - tsr->tshut_threshold);
> + err |= bg_writel(cdev, rval->bg_threshold,
> + tsr->bgap_threshold, &tsr->bg_reg_lock);
> + err |= bg_writel(cdev, rval->tshut_threshold,
> + tsr->tshut_threshold, &tsr->bg_reg_lock);
> /* Force immediate temperature measurement and update
> * of the DTEMP field
> */
> omap_bandgap_force_single_read(bg_ptr, i);
> - err |= omap_control_writel(cdev, rval->bg_counter,
> - tsr->bgap_counter);
> - err |= omap_control_writel(cdev, rval->bg_mode_ctrl,
> - tsr->bgap_mode_ctrl);
> - err |= omap_control_writel(cdev, rval->bg_ctrl,
> - tsr->bgap_mask_ctrl);
> + err |= bg_writel(cdev, rval->bg_counter,
> + tsr->bgap_counter, &tsr->bg_reg_lock);
> + err |= bg_writel(cdev, rval->bg_mode_ctrl,
> + tsr->bgap_mode_ctrl, &tsr->bg_reg_lock);
> + err |= bg_writel(cdev, rval->bg_ctrl,
> + tsr->bgap_mask_ctrl, &tsr->bg_reg_lock);
> } else {
> err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl,
> &temp);
> @@ -1543,8 +1558,8 @@ static int omap_bandgap_restore_ctxt(str
> tsr->bgap_mask_ctrl,
> &temp);
> temp |= 1 << __ffs(tsr->mode_ctrl_mask);
> - err |= omap_control_writel(cdev, temp,
> - tsr->bgap_mask_ctrl);
> + err |= bg_writel(cdev, temp,
> + tsr->bgap_mask_ctrl, &tsr->bg_reg_lock);
> }
> }
> if (err)
> Index: omap-thermal/drivers/usb/otg/omap4-usb-phy.c
> ===================================================================
> --- omap-thermal.orig/drivers/usb/otg/omap4-usb-phy.c
> +++ omap-thermal/drivers/usb/otg/omap4-usb-phy.c
> @@ -46,13 +46,13 @@ int omap4_usb_phy_power(struct device *d
> if (on) {
> ret = omap_control_readl(dev, CONTROL_DEV_CONF, &val);
> if (!ret && (val & PHY_PD)) {
> - ret = omap_control_writel(dev, ~PHY_PD,
> + ret = omap_control_lock_writel(dev, ~PHY_PD,
> CONTROL_DEV_CONF);
> /* XXX: add proper documentation for this delay */
> mdelay(200);
> }
> } else {
> - ret = omap_control_writel(dev, PHY_PD, CONTROL_DEV_CONF);
> + ret = omap_control_lock_writel(dev, PHY_PD, CONTROL_DEV_CONF);
> }
>
> return ret;
> @@ -74,7 +74,7 @@ EXPORT_SYMBOL_GPL(omap4_usb_phy_power);
> */
> int omap4_usb_phy_mailbox(struct device *dev, u32 val)
> {
> - return omap_control_writel(dev, val, CONTROL_USBOTGHS_CONTROL);
> + return omap_control_lock_writel(dev, val, CONTROL_USBOTGHS_CONTROL);
> }
> EXPORT_SYMBOL_GPL(omap4_usb_phy_mailbox);
>
> Index: omap-thermal/include/linux/mfd/omap_control.h
> ===================================================================
> --- omap-thermal.orig/include/linux/mfd/omap_control.h
> +++ omap-thermal/include/linux/mfd/omap_control.h
> @@ -43,6 +43,7 @@ struct omap_control {
> #ifdef CONFIG_MFD_OMAP_CONTROL
> extern int omap_control_readl(struct device *dev, u32 reg, u32 *val);
> extern int omap_control_writel(struct device *dev, u32 val, u32 reg);
> +extern int omap_control_lock_writel(struct device *dev, u32 val, u32 reg);
> extern struct device *omap_control_get(void);
> extern void omap_control_put(struct device *dev);
> #else
> @@ -55,6 +56,11 @@ static inline int omap_control_writel(st
> {
> return 0;
> }
> +
> +static inline int omap_control_lock_writel(struct device *dev, u32 val, u32 reg)
> +{
> + return 0;
> +}
>
> static inline struct device *omap_control_get(void)
> {
>
>
^ permalink raw reply
* Re: [RFC PATCH 09/11] ARM: OMAP4+: thermal: introduce bandgap temperature sensor
From: Eduardo Valentin @ 2012-05-28 11:06 UTC (permalink / raw)
To: Cousson, Benoit
Cc: amit.kucheria, kbaidarov, kishon, balbi, Keerthy, linux-pm,
linux-omap, linux-arm-kernel
In-Reply-To: <4FBFAA18.4040103@ti.com>
On Fri, May 25, 2012 at 05:49:44PM +0200, Cousson Benoit wrote:
> On 5/25/2012 10:25 AM, Eduardo Valentin wrote:
> >In the System Control Module, OMAP supplies a voltage reference
> >and a temperature sensor feature that are gathered in the band
> >gap voltage and temperature sensor (VBGAPTS) module. The band
> >gap provides current and voltage reference for its internal
> >circuits and other analog IP blocks. The analog-to-digital
> >converter (ADC) produces an output value that is proportional
> >to the silicon temperature.
> >
> >This patch provides a platform driver which expose this feature.
> >It is moduled as a MFD child of the System Control Module core
> >MFD driver.
> >
> >This driver provides only APIs to access the device properties,
> >like temperature, thresholds and update rate.
> >
> >Signed-off-by: Eduardo Valentin<eduardo.valentin@ti.com>
> >Signed-off-by: Keerthy<j-keerthy@ti.com>
> >---
> > .../devicetree/bindings/thermal/omap_bandgap.txt | 27 +
> > drivers/thermal/Kconfig | 13 +
> > drivers/thermal/Makefile | 4 +-
> > drivers/thermal/omap-bandgap.c | 1601 ++++++++++++++++++++
> > drivers/thermal/omap-bandgap.h | 63 +
> > 5 files changed, 1707 insertions(+), 1 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/thermal/omap_bandgap.txt
> > create mode 100644 drivers/thermal/omap-bandgap.c
> > create mode 100644 drivers/thermal/omap-bandgap.h
> >
> >diff --git a/Documentation/devicetree/bindings/thermal/omap_bandgap.txt b/Documentation/devicetree/bindings/thermal/omap_bandgap.txt
> >new file mode 100644
> >index 0000000..430bcf8
> >--- /dev/null
> >+++ b/Documentation/devicetree/bindings/thermal/omap_bandgap.txt
> >@@ -0,0 +1,27 @@
> >+* Texas Instrument OMAP SCM bandgap bindings
> >+
> >+In the System Control Module, OMAP supplies a voltage reference
> >+and a temperature sensor feature that are gathered in the band
> >+gap voltage and temperature sensor (VBGAPTS) module. The band
> >+gap provides current and voltage reference for its internal
> >+circuits and other analog IP blocks. The analog-to-digital
> >+converter (ADC) produces an output value that is proportional
> >+to the silicon temperature.
> >+
> >+Required properties:
> >+- compatible : Should be:
> >+ - "ti,omap4460-control-bandgap" : for OMAP4460 bandgap
> >+ - "ti,omap5430-control-bandgap" : for OMAP5430 bandgap
> >+- interrupts : this entry should indicate which interrupt line
> >+the talert signal is routed to;
> >+Specific:
> >+- ti,tshut-gpio : this entry should be used to inform which GPIO
> >+line the tshut signal is routed to;
> >+
> >+Example:
> >+
> >+bandgap {
> >+ compatible = "ti,omap4460-control-bandgap";
> >+ interrupts =<0 126 4>; /* talert */
> >+ ti,tshut-gpio =<86>;
> >+};
> >diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> >index 514a691..ffdd240 100644
> >--- a/drivers/thermal/Kconfig
> >+++ b/drivers/thermal/Kconfig
> >@@ -26,3 +26,16 @@ config SPEAR_THERMAL
> > help
> > Enable this to plug the SPEAr thermal sensor driver into the Linux
> > thermal framework
> >+
> >+config OMAP_BANDGAP
> >+ tristate "Texas Instruments OMAP4+ temperature sensor driver"
> >+ depends on THERMAL
> >+ depends on MFD_OMAP_CONTROL
> >+ help
> >+ If you say yes here you get support for the Texas Instruments
> >+ OMAP4460+ on die bandgap temperature sensor support. The register
> >+ set is part of system control module.
> >+
> >+ This includes alert interrupts generation and also the TSHUT
> >+ support.
> >+
> >diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> >index a9fff0b..5ff1af1 100644
> >--- a/drivers/thermal/Makefile
> >+++ b/drivers/thermal/Makefile
> >@@ -3,4 +3,6 @@
> > #
> >
> > obj-$(CONFIG_THERMAL) += thermal_sys.o
> >-obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
> >\ No newline at end of file
> >+obj-$(CONFIG_SPEAR_THERMAL) += spear_thermal.o
> >+obj-$(CONFIG_OMAP_BANDGAP) += omap-thermal.o
> >+omap-thermal-y := omap-bandgap.o
> >diff --git a/drivers/thermal/omap-bandgap.c b/drivers/thermal/omap-bandgap.c
> >new file mode 100644
> >index 0000000..3d5a12b
> >--- /dev/null
> >+++ b/drivers/thermal/omap-bandgap.c
> >@@ -0,0 +1,1601 @@
> >+/*
> >+ * OMAP4 Bandgap temperature sensor driver
> >+ *
> >+ * Copyright (C) 2011-2012 Texas Instruments Incorporated - http://www.ti.com/
> >+ * Author: J Keerthy<j-keerthy@ti.com>
> >+ * Author: Moiz Sonasath<m-sonasath@ti.com>
> >+ * Couple of fixes, DT and MFD adaptation:
> >+ * Eduardo Valentin<eduardo.valentin@ti.com>
> >+ *
> >+ * This program is free software; you can redistribute it and/or
> >+ * modify it under the terms of the GNU General Public License
> >+ * version 2 as published by the Free Software Foundation.
> >+ *
> >+ * This program is distributed in the hope that it will be useful, but
> >+ * WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >+ * General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU General Public License
> >+ * along with this program; if not, write to the Free Software
> >+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> >+ * 02110-1301 USA
> >+ *
> >+ */
> >+
> >+#include<linux/module.h>
> >+#include<linux/export.h>
> >+#include<linux/init.h>
> >+#include<linux/kernel.h>
> >+#include<linux/interrupt.h>
> >+#include<linux/clk.h>
> >+#include<linux/gpio.h>
> >+#include<linux/platform_device.h>
> >+#include<linux/err.h>
> >+#include<linux/types.h>
> >+#include<linux/mutex.h>
> >+#include<linux/reboot.h>
> >+#include<linux/of_platform.h>
> >+#include<linux/of_irq.h>
> >+#include<linux/mfd/omap_control.h>
> >+
> >+#include<mach/control.h>
> >+
> >+#include "omap-bandgap.h"
> >+
> >+/* Offsets from the base of temperature sensor registers */
> >+
> >+#define OMAP4460_TEMP_SENSOR_CTRL_OFFSET 0x32C
> >+#define OMAP4460_BGAP_CTRL_OFFSET 0x378
> >+#define OMAP4460_BGAP_COUNTER_OFFSET 0x37C
> >+#define OMAP4460_BGAP_THRESHOLD_OFFSET 0x380
> >+#define OMAP4460_BGAP_TSHUT_OFFSET 0x384
> >+#define OMAP4460_BGAP_STATUS_OFFSET 0x388
> >+#define OMAP4460_FUSE_OPP_BGAP 0x260
> >+
> >+#define OMAP5430_TEMP_SENSOR_MPU_OFFSET 0x32C
> >+#define OMAP5430_BGAP_CTRL_OFFSET 0x380
> >+#define OMAP5430_BGAP_COUNTER_MPU_OFFSET 0x39C
> >+#define OMAP5430_BGAP_THRESHOLD_MPU_OFFSET 0x384
> >+#define OMAP5430_BGAP_TSHUT_MPU_OFFSET 0x390
> >+#define OMAP5430_BGAP_STATUS_OFFSET 0x3A8
> >+#define OMAP5430_FUSE_OPP_BGAP_MPU 0x1E4
> >+
> >+#define OMAP5430_TEMP_SENSOR_GPU_OFFSET 0x330
> >+#define OMAP5430_BGAP_COUNTER_GPU_OFFSET 0x3A0
> >+#define OMAP5430_BGAP_THRESHOLD_GPU_OFFSET 0x388
> >+#define OMAP5430_BGAP_TSHUT_GPU_OFFSET 0x394
> >+#define OMAP5430_FUSE_OPP_BGAP_GPU 0x1E0
> >+
> >+#define OMAP5430_TEMP_SENSOR_CORE_OFFSET 0x334
> >+#define OMAP5430_BGAP_COUNTER_CORE_OFFSET 0x3A4
> >+#define OMAP5430_BGAP_THRESHOLD_CORE_OFFSET 0x38C
> >+#define OMAP5430_BGAP_TSHUT_CORE_OFFSET 0x398
> >+#define OMAP5430_FUSE_OPP_BGAP_CORE 0x1E8
> >+
> >+#define OMAP4460_TSHUT_HOT 900 /* 122 deg C */
> >+#define OMAP4460_TSHUT_COLD 895 /* 100 deg C */
> >+#define OMAP4460_T_HOT 800 /* 73 deg C */
> >+#define OMAP4460_T_COLD 795 /* 71 deg C */
> >+#define OMAP4460_MAX_FREQ 1500000
> >+#define OMAP4460_MIN_FREQ 1000000
> >+#define OMAP4460_MIN_TEMP -40000
> >+#define OMAP4460_MAX_TEMP 123000
> >+#define OMAP4460_HYST_VAL 5000
> >+#define OMAP4460_ADC_START_VALUE 530
> >+#define OMAP4460_ADC_END_VALUE 932
> >+
> >+#define OMAP5430_MPU_TSHUT_HOT 915
> >+#define OMAP5430_MPU_TSHUT_COLD 900
> >+#define OMAP5430_MPU_T_HOT 800
> >+#define OMAP5430_MPU_T_COLD 795
> >+#define OMAP5430_MPU_MAX_FREQ 1500000
> >+#define OMAP5430_MPU_MIN_FREQ 1000000
> >+#define OMAP5430_MPU_MIN_TEMP -40000
> >+#define OMAP5430_MPU_MAX_TEMP 125000
> >+#define OMAP5430_MPU_HYST_VAL 5000
> >+#define OMAP5430_ADC_START_VALUE 532
> >+#define OMAP5430_ADC_END_VALUE 934
> >+
> >+#define OMAP5430_GPU_TSHUT_HOT 915
> >+#define OMAP5430_GPU_TSHUT_COLD 900
> >+#define OMAP5430_GPU_T_HOT 800
> >+#define OMAP5430_GPU_T_COLD 795
> >+#define OMAP5430_GPU_MAX_FREQ 1500000
> >+#define OMAP5430_GPU_MIN_FREQ 1000000
> >+#define OMAP5430_GPU_MIN_TEMP -40000
> >+#define OMAP5430_GPU_MAX_TEMP 125000
> >+#define OMAP5430_GPU_HYST_VAL 5000
> >+
> >+#define OMAP5430_CORE_TSHUT_HOT 915
> >+#define OMAP5430_CORE_TSHUT_COLD 900
> >+#define OMAP5430_CORE_T_HOT 800
> >+#define OMAP5430_CORE_T_COLD 795
> >+#define OMAP5430_CORE_MAX_FREQ 1500000
> >+#define OMAP5430_CORE_MIN_FREQ 1000000
> >+#define OMAP5430_CORE_MIN_TEMP -40000
> >+#define OMAP5430_CORE_MAX_TEMP 125000
> >+#define OMAP5430_CORE_HYST_VAL 5000
> >+
> >+/**
> >+ * The register offsets and bit fields might change across
> >+ * OMAP versions hence populating them in this structure.
> >+ */
> >+
> >+struct temp_sensor_registers {
> >+ u32 temp_sensor_ctrl;
> >+ u32 bgap_tempsoff_mask;
> >+ u32 bgap_soc_mask;
> >+ u32 bgap_eocz_mask;
> >+ u32 bgap_dtemp_mask;
> >+
> >+ u32 bgap_mask_ctrl;
> >+ u32 mask_hot_mask;
> >+ u32 mask_cold_mask;
> >+
> >+ u32 bgap_mode_ctrl;
> >+ u32 mode_ctrl_mask;
> >+
> >+ u32 bgap_counter;
> >+ u32 counter_mask;
> >+
> >+ u32 bgap_threshold;
> >+ u32 threshold_thot_mask;
> >+ u32 threshold_tcold_mask;
> >+
> >+ u32 tshut_threshold;
> >+ u32 tshut_hot_mask;
> >+ u32 tshut_cold_mask;
> >+
> >+ u32 bgap_status;
> >+ u32 status_clean_stop_mask;
> >+ u32 status_bgap_alert_mask;
> >+ u32 status_hot_mask;
> >+ u32 status_cold_mask;
> >+
> >+ u32 bgap_efuse;
> >+};
> >+
> >+/**
> >+ * The thresholds and limits for temperature sensors.
> >+ */
> >+struct temp_sensor_data {
> >+ u32 tshut_hot;
> >+ u32 tshut_cold;
> >+ u32 t_hot;
> >+ u32 t_cold;
> >+ u32 min_freq;
> >+ u32 max_freq;
> >+ int max_temp;
> >+ int min_temp;
> >+ int hyst_val;
> >+ u32 adc_start_val;
> >+ u32 adc_end_val;
> >+ u32 update_int1;
> >+ u32 update_int2;
> >+};
> >+
> >+/**
> >+ * struct temp_sensor_regval - temperature sensor register values
> >+ * @bg_mode_ctrl: temp sensor control register value
> >+ * @bg_ctrl: bandgap ctrl register value
> >+ * @bg_counter: bandgap counter value
> >+ * @bg_threshold: bandgap threshold register value
> >+ * @tshut_threshold: bandgap tshut register value
> >+ */
> >+struct temp_sensor_regval {
> >+ u32 bg_mode_ctrl;
> >+ u32 bg_ctrl;
> >+ u32 bg_counter;
> >+ u32 bg_threshold;
> >+ u32 tshut_threshold;
> >+};
> >+
> >+/**
> >+ * struct omap_temp_sensor - bandgap temperature sensor platform data
> >+ * @ts_data: pointer to struct with thresholds, limits of temperature sensor
> >+ * @registers: pointer to the list of register offsets and bitfields
> >+ * @regval: temperature sensor register values
> >+ * @domain: the name of the domain where the sensor is located
> >+ */
> >+struct omap_temp_sensor {
> >+ struct temp_sensor_data *ts_data;
> >+ struct temp_sensor_registers *registers;
> >+ struct temp_sensor_regval *regval;
> >+ char *domain;
> >+};
> >+
> >+/**
> >+ * struct omap_bandgap_data - bandgap platform data structure
> >+ * @has_talert: indicates if the chip has talert output line
> >+ * @has_tshut: indicates if the chip has tshut output line
> >+ * @conv_table: Pointer to adc to temperature conversion table
> >+ * @fclock_name: clock name of the functional clock
> >+ * @div_ck_nme: clock name of the clock divisor
> >+ * @sensor_count: count of temperature sensor device in scm
> >+ * @sensors: array of sensors present in this bandgap instance
> >+ * @expose_sensor: callback to export sensor to thermal API
> >+ */
> >+struct omap_bandgap_data {
> >+ bool has_talert;
> >+ bool has_tshut;
> >+ const int *conv_table;
> >+ char *fclock_name;
> >+ char *div_ck_name;
> >+ int sensor_count;
> >+ int (*report_temperature)(struct omap_bandgap *bg_ptr, int id);
> >+ int (*expose_sensor)(struct omap_bandgap *bg_ptr, int id, char *domain);
> >+
> >+ /* this needs to be at the end */
> >+ struct omap_temp_sensor sensors[];
> >+};
> >+
> >+/* TODO: provide data structures for 4430 */
> >+
> >+/*
> >+ * OMAP4460 has one instance of thermal sensor for MPU
> >+ * need to describe the individual bit fields
> >+ */
> >+static struct temp_sensor_registers
> >+omap4460_mpu_temp_sensor_registers = {
> >+ .temp_sensor_ctrl = OMAP4460_TEMP_SENSOR_CTRL_OFFSET,
> >+ .bgap_tempsoff_mask = OMAP4460_BGAP_TEMPSOFF_MASK,
> >+ .bgap_soc_mask = OMAP4460_BGAP_TEMP_SENSOR_SOC_MASK,
> >+ .bgap_eocz_mask = OMAP4460_BGAP_TEMP_SENSOR_EOCZ_MASK,
> >+ .bgap_dtemp_mask = OMAP4460_BGAP_TEMP_SENSOR_DTEMP_MASK,
> >+
> >+ .bgap_mask_ctrl = OMAP4460_BGAP_CTRL_OFFSET,
> >+ .mask_hot_mask = OMAP4460_MASK_HOT_MASK,
> >+ .mask_cold_mask = OMAP4460_MASK_COLD_MASK,
> >+
> >+ .bgap_mode_ctrl = OMAP4460_BGAP_CTRL_OFFSET,
> >+ .mode_ctrl_mask = OMAP4460_SINGLE_MODE_MASK,
> >+
> >+ .bgap_counter = OMAP4460_BGAP_COUNTER_OFFSET,
> >+ .counter_mask = OMAP4460_COUNTER_MASK,
> >+
> >+ .bgap_threshold = OMAP4460_BGAP_THRESHOLD_OFFSET,
> >+ .threshold_thot_mask = OMAP4460_T_HOT_MASK,
> >+ .threshold_tcold_mask = OMAP4460_T_COLD_MASK,
> >+
> >+ .tshut_threshold = OMAP4460_BGAP_TSHUT_OFFSET,
> >+ .tshut_hot_mask = OMAP4460_TSHUT_HOT_MASK,
> >+ .tshut_cold_mask = OMAP4460_TSHUT_COLD_MASK,
> >+
> >+ .bgap_status = OMAP4460_BGAP_STATUS_OFFSET,
> >+ .status_clean_stop_mask = OMAP4460_CLEAN_STOP_MASK,
> >+ .status_bgap_alert_mask = OMAP4460_BGAP_ALERT_MASK,
> >+ .status_hot_mask = OMAP4460_HOT_FLAG_MASK,
> >+ .status_cold_mask = OMAP4460_COLD_FLAG_MASK,
> >+
> >+ .bgap_efuse = OMAP4460_FUSE_OPP_BGAP,
> >+};
> >+
> >+/*
> >+ * OMAP4460 has one instance of thermal sensor for MPU
>
> Bad copy paste.
>
> >+ * need to describe the individual bit fields
> >+ */
> >+static struct temp_sensor_registers
> >+omap5430_mpu_temp_sensor_registers = {
> >+ .temp_sensor_ctrl = OMAP5430_TEMP_SENSOR_MPU_OFFSET,
> >+ .bgap_tempsoff_mask = OMAP5430_BGAP_TEMPSOFF_MASK,
> >+ .bgap_soc_mask = OMAP5430_BGAP_TEMP_SENSOR_SOC_MASK,
> >+ .bgap_eocz_mask = OMAP5430_BGAP_TEMP_SENSOR_EOCZ_MASK,
> >+ .bgap_dtemp_mask = OMAP5430_BGAP_TEMP_SENSOR_DTEMP_MASK,
> >+
> >+ .bgap_mask_ctrl = OMAP5430_BGAP_CTRL_OFFSET,
> >+ .mask_hot_mask = OMAP5430_MASK_HOT_MPU_MASK,
> >+ .mask_cold_mask = OMAP5430_MASK_COLD_MPU_MASK,
> >+
> >+ .bgap_mode_ctrl = OMAP5430_BGAP_COUNTER_MPU_OFFSET,
> >+ .mode_ctrl_mask = OMAP5430_REPEAT_MODE_MASK,
> >+
> >+ .bgap_counter = OMAP5430_BGAP_COUNTER_MPU_OFFSET,
> >+ .counter_mask = OMAP5430_COUNTER_MASK,
> >+
> >+ .bgap_threshold = OMAP5430_BGAP_THRESHOLD_MPU_OFFSET,
> >+ .threshold_thot_mask = OMAP5430_T_HOT_MASK,
> >+ .threshold_tcold_mask = OMAP5430_T_COLD_MASK,
> >+
> >+ .tshut_threshold = OMAP5430_BGAP_TSHUT_MPU_OFFSET,
> >+ .tshut_hot_mask = OMAP5430_TSHUT_HOT_MASK,
> >+ .tshut_cold_mask = OMAP5430_TSHUT_COLD_MASK,
> >+
> >+ .bgap_status = OMAP5430_BGAP_STATUS_OFFSET,
> >+ .status_clean_stop_mask = 0x0,
> >+ .status_bgap_alert_mask = OMAP5430_BGAP_ALERT_MASK,
> >+ .status_hot_mask = OMAP5430_HOT_MPU_FLAG_MASK,
> >+ .status_cold_mask = OMAP5430_COLD_MPU_FLAG_MASK,
> >+
> >+ .bgap_efuse = OMAP5430_FUSE_OPP_BGAP_MPU,
> >+};
> >+
> >+/*
> >+ * OMAP4460 has one instance of thermal sensor for MPU
> >+ * need to describe the individual bit fields
> >+ */
>
> Ditto
>
> >+static struct temp_sensor_registers
> >+omap5430_gpu_temp_sensor_registers = {
> >+ .temp_sensor_ctrl = OMAP5430_TEMP_SENSOR_GPU_OFFSET,
> >+ .bgap_tempsoff_mask = OMAP5430_BGAP_TEMPSOFF_MASK,
> >+ .bgap_soc_mask = OMAP5430_BGAP_TEMP_SENSOR_SOC_MASK,
> >+ .bgap_eocz_mask = OMAP5430_BGAP_TEMP_SENSOR_EOCZ_MASK,
> >+ .bgap_dtemp_mask = OMAP5430_BGAP_TEMP_SENSOR_DTEMP_MASK,
> >+
> >+ .bgap_mask_ctrl = OMAP5430_BGAP_CTRL_OFFSET,
> >+ .mask_hot_mask = OMAP5430_MASK_HOT_MM_MASK,
> >+ .mask_cold_mask = OMAP5430_MASK_COLD_MM_MASK,
> >+
> >+ .bgap_mode_ctrl = OMAP5430_BGAP_COUNTER_GPU_OFFSET,
> >+ .mode_ctrl_mask = OMAP5430_REPEAT_MODE_MASK,
> >+
> >+ .bgap_counter = OMAP5430_BGAP_COUNTER_GPU_OFFSET,
> >+ .counter_mask = OMAP5430_COUNTER_MASK,
> >+
> >+ .bgap_threshold = OMAP5430_BGAP_THRESHOLD_GPU_OFFSET,
> >+ .threshold_thot_mask = OMAP5430_T_HOT_MASK,
> >+ .threshold_tcold_mask = OMAP5430_T_COLD_MASK,
> >+
> >+ .tshut_threshold = OMAP5430_BGAP_TSHUT_GPU_OFFSET,
> >+ .tshut_hot_mask = OMAP5430_TSHUT_HOT_MASK,
> >+ .tshut_cold_mask = OMAP5430_TSHUT_COLD_MASK,
> >+
> >+ .bgap_status = OMAP5430_BGAP_STATUS_OFFSET,
> >+ .status_clean_stop_mask = 0x0,
> >+ .status_bgap_alert_mask = OMAP5430_BGAP_ALERT_MASK,
> >+ .status_hot_mask = OMAP5430_HOT_MM_FLAG_MASK,
> >+ .status_cold_mask = OMAP5430_COLD_MM_FLAG_MASK,
> >+
> >+ .bgap_efuse = OMAP5430_FUSE_OPP_BGAP_GPU,
> >+};
> >+
> >+/*
> >+ * OMAP4460 has one instance of thermal sensor for MPU
> >+ * need to describe the individual bit fields
> >+ */
>
> Ditto
>
> >+static struct temp_sensor_registers
> >+omap5430_core_temp_sensor_registers = {
> >+ .temp_sensor_ctrl = OMAP5430_TEMP_SENSOR_CORE_OFFSET,
> >+ .bgap_tempsoff_mask = OMAP5430_BGAP_TEMPSOFF_MASK,
> >+ .bgap_soc_mask = OMAP5430_BGAP_TEMP_SENSOR_SOC_MASK,
> >+ .bgap_eocz_mask = OMAP5430_BGAP_TEMP_SENSOR_EOCZ_MASK,
> >+ .bgap_dtemp_mask = OMAP5430_BGAP_TEMP_SENSOR_DTEMP_MASK,
> >+
> >+ .bgap_mask_ctrl = OMAP5430_BGAP_CTRL_OFFSET,
> >+ .mask_hot_mask = OMAP5430_MASK_HOT_CORE_MASK,
> >+ .mask_cold_mask = OMAP5430_MASK_COLD_CORE_MASK,
> >+
> >+ .bgap_mode_ctrl = OMAP5430_BGAP_COUNTER_CORE_OFFSET,
> >+ .mode_ctrl_mask = OMAP5430_REPEAT_MODE_MASK,
> >+
> >+ .bgap_counter = OMAP5430_BGAP_COUNTER_CORE_OFFSET,
> >+ .counter_mask = OMAP5430_COUNTER_MASK,
> >+
> >+ .bgap_threshold = OMAP5430_BGAP_THRESHOLD_CORE_OFFSET,
> >+ .threshold_thot_mask = OMAP5430_T_HOT_MASK,
> >+ .threshold_tcold_mask = OMAP5430_T_COLD_MASK,
> >+
> >+ .tshut_threshold = OMAP5430_BGAP_TSHUT_CORE_OFFSET,
> >+ .tshut_hot_mask = OMAP5430_TSHUT_HOT_MASK,
> >+ .tshut_cold_mask = OMAP5430_TSHUT_COLD_MASK,
> >+
> >+ .bgap_status = OMAP5430_BGAP_STATUS_OFFSET,
> >+ .status_clean_stop_mask = 0x0,
> >+ .status_bgap_alert_mask = OMAP5430_BGAP_ALERT_MASK,
> >+ .status_hot_mask = OMAP5430_HOT_CORE_FLAG_MASK,
> >+ .status_cold_mask = OMAP5430_COLD_CORE_FLAG_MASK,
> >+
> >+ .bgap_efuse = OMAP5430_FUSE_OPP_BGAP_CORE,
> >+};
> >+
> >+/* Thresholds and limits for OMAP4460 MPU temperature sensor */
> >+static struct temp_sensor_data omap4460_mpu_temp_sensor_data = {
> >+ .tshut_hot = OMAP4460_TSHUT_HOT,
> >+ .tshut_cold = OMAP4460_TSHUT_COLD,
> >+ .t_hot = OMAP4460_T_HOT,
> >+ .t_cold = OMAP4460_T_COLD,
> >+ .min_freq = OMAP4460_MIN_FREQ,
> >+ .max_freq = OMAP4460_MAX_FREQ,
> >+ .max_temp = OMAP4460_MAX_TEMP,
> >+ .min_temp = OMAP4460_MIN_TEMP,
> >+ .hyst_val = OMAP4460_HYST_VAL,
> >+ .adc_start_val = OMAP4460_ADC_START_VALUE,
> >+ .adc_end_val = OMAP4460_ADC_END_VALUE,
> >+ .update_int1 = 1000,
> >+ .update_int2 = 2000,
> >+};
> >+
> >+/* Thresholds and limits for OMAP5430 MPU temperature sensor */
> >+static struct temp_sensor_data omap5430_mpu_temp_sensor_data = {
> >+ .tshut_hot = OMAP5430_MPU_TSHUT_HOT,
> >+ .tshut_cold = OMAP5430_MPU_TSHUT_COLD,
> >+ .t_hot = OMAP5430_MPU_T_HOT,
> >+ .t_cold = OMAP5430_MPU_T_COLD,
> >+ .min_freq = OMAP5430_MPU_MIN_FREQ,
> >+ .max_freq = OMAP5430_MPU_MAX_FREQ,
> >+ .max_temp = OMAP5430_MPU_MAX_TEMP,
> >+ .min_temp = OMAP5430_MPU_MIN_TEMP,
> >+ .hyst_val = OMAP5430_MPU_HYST_VAL,
> >+ .adc_start_val = OMAP5430_ADC_START_VALUE,
> >+ .adc_end_val = OMAP5430_ADC_END_VALUE,
> >+ .update_int1 = 1000,
> >+ .update_int2 = 2000,
> >+};
> >+
> >+/* Thresholds and limits for OMAP5430 GPU temperature sensor */
> >+static struct temp_sensor_data omap5430_gpu_temp_sensor_data = {
> >+ .tshut_hot = OMAP5430_GPU_TSHUT_HOT,
> >+ .tshut_cold = OMAP5430_GPU_TSHUT_COLD,
> >+ .t_hot = OMAP5430_GPU_T_HOT,
> >+ .t_cold = OMAP5430_GPU_T_COLD,
> >+ .min_freq = OMAP5430_GPU_MIN_FREQ,
> >+ .max_freq = OMAP5430_GPU_MAX_FREQ,
> >+ .max_temp = OMAP5430_GPU_MAX_TEMP,
> >+ .min_temp = OMAP5430_GPU_MIN_TEMP,
> >+ .hyst_val = OMAP5430_GPU_HYST_VAL,
> >+ .adc_start_val = OMAP5430_ADC_START_VALUE,
> >+ .adc_end_val = OMAP5430_ADC_END_VALUE,
> >+ .update_int1 = 1000,
> >+ .update_int2 = 2000,
> >+};
> >+
> >+/* Thresholds and limits for OMAP5430 CORE temperature sensor */
> >+static struct temp_sensor_data omap5430_core_temp_sensor_data = {
> >+ .tshut_hot = OMAP5430_CORE_TSHUT_HOT,
> >+ .tshut_cold = OMAP5430_CORE_TSHUT_COLD,
> >+ .t_hot = OMAP5430_CORE_T_HOT,
> >+ .t_cold = OMAP5430_CORE_T_COLD,
> >+ .min_freq = OMAP5430_CORE_MIN_FREQ,
> >+ .max_freq = OMAP5430_CORE_MAX_FREQ,
> >+ .max_temp = OMAP5430_CORE_MAX_TEMP,
> >+ .min_temp = OMAP5430_CORE_MIN_TEMP,
> >+ .hyst_val = OMAP5430_CORE_HYST_VAL,
> >+ .adc_start_val = OMAP5430_ADC_START_VALUE,
> >+ .adc_end_val = OMAP5430_ADC_END_VALUE,
> >+ .update_int1 = 1000,
> >+ .update_int2 = 2000,
> >+};
> >+
> >+/*
> >+ * Temperature values in milli degree celsius
> >+ * ADC code values from 530 to 923
> >+ */
> >+static const int
> >+omap4460_adc_to_temp[OMAP4460_ADC_END_VALUE - OMAP4460_ADC_START_VALUE + 1] = {
> >+ -40000, -40000, -40000, -40000, -39800, -39400, -39000, -38600, -38200,
> >+ -37800, -37300, -36800, -36400, -36000, -35600, -35200, -34800,
> >+ -34300, -33800, -33400, -33000, -32600, -32200, -31800, -31300,
> >+ -30800, -30400, -30000, -29600, -29200, -28700, -28200, -27800,
> >+ -27400, -27000, -26600, -26200, -25700, -25200, -24800, -24400,
> >+ -24000, -23600, -23200, -22700, -22200, -21800, -21400, -21000,
> >+ -20600, -20200, -19700, -19200, -18800, -18400, -18000, -17600,
> >+ -17200, -16700, -16200, -15800, -15400, -15000, -14600, -14200,
> >+ -13700, -13200, -12800, -12400, -12000, -11600, -11200, -10700,
> >+ -10200, -9800, -9400, -9000, -8600, -8200, -7700, -7200, -6800,
> >+ -6400, -6000, -5600, -5200, -4800, -4300, -3800, -3400, -3000,
> >+ -2600, -2200, -1800, -1300, -800, -400, 0, 400, 800, 1200, 1600,
> >+ 2100, 2600, 3000, 3400, 3800, 4200, 4600, 5100, 5600, 6000, 6400,
> >+ 6800, 7200, 7600, 8000, 8500, 9000, 9400, 9800, 10200, 10600, 11000,
> >+ 11400, 11900, 12400, 12800, 13200, 13600, 14000, 14400, 14800,
> >+ 15300, 15800, 16200, 16600, 17000, 17400, 17800, 18200, 18700,
> >+ 19200, 19600, 20000, 20400, 20800, 21200, 21600, 22100, 22600,
> >+ 23000, 23400, 23800, 24200, 24600, 25000, 25400, 25900, 26400,
> >+ 26800, 27200, 27600, 28000, 28400, 28800, 29300, 29800, 30200,
> >+ 30600, 31000, 31400, 31800, 32200, 32600, 33100, 33600, 34000,
> >+ 34400, 34800, 35200, 35600, 36000, 36400, 36800, 37300, 37800,
> >+ 38200, 38600, 39000, 39400, 39800, 40200, 40600, 41100, 41600,
> >+ 42000, 42400, 42800, 43200, 43600, 44000, 44400, 44800, 45300,
> >+ 45800, 46200, 46600, 47000, 47400, 47800, 48200, 48600, 49000,
> >+ 49500, 50000, 50400, 50800, 51200, 51600, 52000, 52400, 52800,
> >+ 53200, 53700, 54200, 54600, 55000, 55400, 55800, 56200, 56600,
> >+ 57000, 57400, 57800, 58200, 58700, 59200, 59600, 60000, 60400,
> >+ 60800, 61200, 61600, 62000, 62400, 62800, 63300, 63800, 64200,
> >+ 64600, 65000, 65400, 65800, 66200, 66600, 67000, 67400, 67800,
> >+ 68200, 68700, 69200, 69600, 70000, 70400, 70800, 71200, 71600,
> >+ 72000, 72400, 72800, 73200, 73600, 74100, 74600, 75000, 75400,
> >+ 75800, 76200, 76600, 77000, 77400, 77800, 78200, 78600, 79000,
> >+ 79400, 79800, 80300, 80800, 81200, 81600, 82000, 82400, 82800,
> >+ 83200, 83600, 84000, 84400, 84800, 85200, 85600, 86000, 86400,
> >+ 86800, 87300, 87800, 88200, 88600, 89000, 89400, 89800, 90200,
> >+ 90600, 91000, 91400, 91800, 92200, 92600, 93000, 93400, 93800,
> >+ 94200, 94600, 95000, 95500, 96000, 96400, 96800, 97200, 97600,
> >+ 98000, 98400, 98800, 99200, 99600, 100000, 100400, 100800, 101200,
> >+ 101600, 102000, 102400, 102800, 103200, 103600, 104000, 104400,
> >+ 104800, 105200, 105600, 106100, 106600, 107000, 107400, 107800,
> >+ 108200, 108600, 109000, 109400, 109800, 110200, 110600, 111000,
> >+ 111400, 111800, 112200, 112600, 113000, 113400, 113800, 114200,
> >+ 114600, 115000, 115400, 115800, 116200, 116600, 117000, 117400,
> >+ 117800, 118200, 118600, 119000, 119400, 119800, 120200, 120600,
> >+ 121000, 121400, 121800, 122200, 122600, 123000, 123400, 123800, 124200,
> >+ 124600, 124900, 125000, 125000, 125000, 125000
> >+};
> >+
> >+static const int
> >+omap5430_adc_to_temp[OMAP5430_ADC_END_VALUE - OMAP5430_ADC_START_VALUE + 1] = {
> >+ -40000, -40000, -40000, -40000, -39800, -39400, -39000, -38600,
> >+ -38200, -37800, -37300, -36800,
> >+ -36400, -36000, -35600, -35200, -34800, -34300, -33800, -33400, -33000,
> >+ -32600,
> >+ -32200, -31800, -31300, -30800, -30400, -30000, -29600, -29200, -28700,
> >+ -28200, -27800, -27400, -27000, -26600, -26200, -25700, -25200, -24800,
> >+ -24400, -24000, -23600, -23200, -22700, -22200, -21800, -21400, -21000,
> >+ -20600, -20200, -19700, -19200, -9300, -18400, -18000, -17600, -17200,
> >+ -16700, -16200, -15800, -15400, -15000, -14600, -14200, -13700, -13200,
> >+ -12800, -12400, -12000, -11600, -11200, -10700, -10200, -9800, -9400,
> >+ -9000,
> >+ -8600, -8200, -7700, -7200, -6800, -6400, -6000, -5600, -5200, -4800,
> >+ -4300,
> >+ -3800, -3400, -3000, -2600, -2200, -1800, -1300, -800, -400, 0, 400,
> >+ 800,
> >+ 1200, 1600, 2100, 2600, 3000, 3400, 3800, 4200, 4600, 5100, 5600, 6000,
> >+ 6400, 6800, 7200, 7600, 8000, 8500, 9000, 9400, 9800, 10200, 10800,
> >+ 11100,
> >+ 11400, 11900, 12400, 12800, 13200, 13600, 14000, 14400, 14800, 15300,
> >+ 15800,
> >+ 16200, 16600, 17000, 17400, 17800, 18200, 18700, 19200, 19600, 20000,
> >+ 20400,
> >+ 20800, 21200, 21600, 22100, 22600, 23000, 23400, 23800, 24200, 24600,
> >+ 25000,
> >+ 25400, 25900, 26400, 26800, 27200, 27600, 28000, 28400, 28800, 29300,
> >+ 29800,
> >+ 30200, 30600, 31000, 31400, 31800, 32200, 32600, 33100, 33600, 34000,
> >+ 34400,
> >+ 34800, 35200, 35600, 36000, 36400, 36800, 37300, 37800, 38200, 38600,
> >+ 39000,
> >+ 39400, 39800, 40200, 40600, 41100, 41600, 42000, 42400, 42800, 43200,
> >+ 43600,
> >+ 44000, 44400, 44800, 45300, 45800, 46200, 46600, 47000, 47400, 47800,
> >+ 48200,
> >+ 48600, 49000, 49500, 50000, 50400, 50800, 51200, 51600, 52000, 52400,
> >+ 52800,
> >+ 53200, 53700, 54200, 54600, 55000, 55400, 55800, 56200, 56600, 57000,
> >+ 57400,
> >+ 57800, 58200, 58700, 59200, 59600, 60000, 60400, 60800, 61200, 61600,
> >+ 62000,
> >+ 62400, 62800, 63300, 63800, 64200, 64600, 65000, 65400, 65800, 66200,
> >+ 66600,
> >+ 67000, 67400, 67800, 68200, 68700, 69200, 69600, 70000, 70400, 70800,
> >+ 71200,
> >+ 71600, 72000, 72400, 72800, 73200, 73600, 74100, 74600, 75000, 75400,
> >+ 75800,
> >+ 76200, 76600, 77000, 77400, 77800, 78200, 78600, 79000, 79400, 79800,
> >+ 80300,
> >+ 80800, 81200, 81600, 82000, 82400, 82800, 83200, 83600, 84000, 84400,
> >+ 84800,
> >+ 85200, 85600, 86000, 86400, 86800, 87300, 87800, 88200, 88600, 89000,
> >+ 89400,
> >+ 89800, 90200, 90600, 91000, 91400, 91800, 92200, 92600, 93000, 93400,
> >+ 93800,
> >+ 94200, 94600, 95000, 95500, 96000, 96400, 96800, 97200, 97600, 98000,
> >+ 98400,
> >+ 98800, 99200, 99600, 100000, 100400, 100800, 101200, 101600, 102000,
> >+ 102400,
> >+ 102800, 103200, 103600, 104000, 104400, 104800, 105200, 105600, 106100,
> >+ 106600, 107000, 107400, 107800, 108200, 108600, 109000, 109400, 109800,
> >+ 110200, 110600, 111000, 111400, 111800, 112200, 112600, 113000, 113400,
> >+ 113800, 114200, 114600, 115000, 115400, 115800, 116200, 116600, 117000,
> >+ 117400, 117800, 118200, 118600, 119000, 119400, 119800, 120200, 120600,
> >+ 121000, 121400, 121800, 122200, 122600, 123000, 123400, 123800, 124200,
> >+ 124600, 124900, 125000, 125000, 125000, 125000,
> >+};
>
> It might be good to store the per SoC data in dedicated files or at
> least put some #ifdef around to avoid wasting memory for a non-multi
> OMAP boot.
Indeed, I was rewriting this stuff with that in mind when I saw your comment :-)
Idea is to move all this arch dependent data structures to specific files.
Here at omap-bandgap.c we would have essentially the pdriver and the local functions
and APIs. The omapX-thermal.c would contain the specific data structures and
the functions related to the thermal zones, and possibly the policy specifics for
each case. Something like:
drivers/thermal/
|- omap-bandgap.c: Bandgap platform driver specific functions and APIs (CONFIG_OMAP_BANDGAP)
|- omap-bandgap.h: Common data structure definition and defines
|- omap4-thermal.c: OMAP4xx data structures needed by the bandgap driver and the thermal zone definitions (CONFIG_OMAP4_THERMAL)
|- omap5-thermal.c: OMAP54xx data structures needed by the bandgap driver and the thermal zone definitions (CONFIG_OMAP5_THERMAL)
|- omap-thermal.ko: Module binary. It would contain the support depending on what has been selected at compile time
> filename: omap-thermal.ko
author: Texas Instrument Inc.
alias: platform:omap-bandgap
license: GPL v2
description: OMAP4+ bandgap temperature sensor driver
srcversion: 937A9F9230A5025A0E9A268
alias: of:N*T*Cti,omap4460-bandgap*
alias: of:N*T*Cti,omap5430-bandgap*
depends:
intree: Y
vermagic: 3.4.0-rc6-12199-g3e31a0c-dirty SMP preempt mod_unload modversions ARMv7 p2v8
There is gonna happen some ifdefery in omap-bandgap.[c,h] in order to compose the
match_id table based on what has been selected for compilation, but I at least don't see
any other way around.
> >+
> >+static irqreturn_t talert_irq_handler(int irq, void *data)
> >+{
> >+ struct omap_bandgap *bg_ptr = data;
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 t_hot = 0, t_cold = 0, temp, ctrl;
> >+ int i, r;
> >+
> >+ bg_ptr = data;
> >+ /* Read the status of t_hot */
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++) {
> >+ tsr = bg_ptr->pdata->sensors[i].registers;
> >+ r = omap_control_readl(cdev, tsr->bgap_status,&t_hot);
> >+ t_hot&= tsr->status_hot_mask;
> >+
> >+ /* Read the status of t_cold */
> >+ r |= omap_control_readl(cdev, tsr->bgap_status,&t_cold);
> >+ t_cold&= tsr->status_cold_mask;
> >+
> >+ if (!t_cold&& !t_hot)
> >+ continue;
> >+
> >+ r |= omap_control_readl(cdev, tsr->bgap_mask_ctrl,&ctrl);
> >+ /*
> >+ * One TALERT interrupt: Two sources
> >+ * If the interrupt is due to t_hot then mask t_hot and
> >+ * and unmask t_cold else mask t_cold and unmask t_hot
> >+ */
> >+ if (t_hot) {
> >+ ctrl&= ~tsr->mask_hot_mask;
> >+ ctrl |= tsr->mask_cold_mask;
> >+ } else if (t_cold) {
> >+ ctrl&= ~tsr->mask_cold_mask;
> >+ ctrl |= tsr->mask_hot_mask;
> >+ }
> >+
> >+ r |= omap_control_writel(cdev, ctrl, tsr->bgap_mask_ctrl);
> >+
> >+ if (r) {
> >+ dev_err(bg_ptr->dev, "failed to ack talert interrupt\n");
> >+ return IRQ_NONE;
> >+ }
> >+
> >+ /* read temperature */
> >+ r = omap_control_readl(cdev, tsr->temp_sensor_ctrl,&temp);
> >+ temp&= tsr->bgap_dtemp_mask;
> >+
> >+ /* report temperature to whom may concern */
> >+ if (bg_ptr->pdata->report_temperature)
> >+ bg_ptr->pdata->report_temperature(bg_ptr, i);
> >+ }
> >+
> >+ return IRQ_HANDLED;
> >+}
> >+
> >+static irqreturn_t omap_bandgap_tshut_irq_handler(int irq, void *data)
> >+{
> >+ orderly_poweroff(true);
> >+
> >+ return IRQ_HANDLED;
> >+}
> >+
> >+static
> >+int adc_to_temp_conversion(struct omap_bandgap *bg_ptr, int id, int adc_val,
> >+ int *t)
> >+{
> >+ struct temp_sensor_data *ts_data = bg_ptr->pdata->sensors[id].ts_data;
> >+
> >+ /* look up for temperature in the table and return the temperature */
> >+ if (adc_val< ts_data->adc_start_val || adc_val> ts_data->adc_end_val)
> >+ return -ERANGE;
> >+
> >+ *t = bg_ptr->conv_table[adc_val - ts_data->adc_start_val];
> >+
> >+ return 0;
> >+}
> >+
> >+static int temp_to_adc_conversion(long temp, struct omap_bandgap *bg_ptr, int i,
> >+ int *adc)
> >+{
> >+ struct temp_sensor_data *ts_data = bg_ptr->pdata->sensors[i].ts_data;
> >+ int high, low, mid;
> >+
> >+ low = 0;
> >+ high = ts_data->adc_end_val - ts_data->adc_start_val;
> >+ mid = (high + low) / 2;
> >+
> >+ if (temp< bg_ptr->conv_table[high] || temp> bg_ptr->conv_table[high])
> >+ return -EINVAL;
> >+
> >+ while (low< high) {
> >+ if (temp< bg_ptr->conv_table[mid])
> >+ high = mid - 1;
> >+ else
> >+ low = mid + 1;
> >+ mid = (low + high) / 2;
> >+ }
> >+
> >+ *adc = ts_data->adc_start_val + low;
> >+
> >+ return 0;
> >+}
> >+
> >+static int temp_sensor_unmask_interrupts(struct omap_bandgap *bg_ptr, int id,
> >+ u32 t_hot, u32 t_cold)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 temp, reg_val;
> >+ int err;
> >+
> >+ /* Read the current on die temperature */
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+ err = omap_control_readl(cdev, tsr->temp_sensor_ctrl,&temp);
> >+ temp&= tsr->bgap_dtemp_mask;
> >+
> >+ err |= omap_control_readl(cdev, tsr->bgap_mask_ctrl,®_val);
> >+ if (temp< t_hot)
> >+ reg_val |= tsr->mask_hot_mask;
> >+ else
> >+ reg_val&= ~tsr->mask_hot_mask;
> >+
> >+ if (t_cold< temp)
> >+ reg_val |= tsr->mask_cold_mask;
> >+ else
> >+ reg_val&= ~tsr->mask_cold_mask;
> >+ err |= omap_control_writel(cdev, reg_val, tsr->bgap_mask_ctrl);
> >+
> >+ if (err) {
> >+ dev_err(bg_ptr->dev, "failed to unmask interrupts\n");
> >+ return -EIO;
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> >+static
> >+int add_hyst(int adc_val, int hyst_val, struct omap_bandgap *bg_ptr, int i,
> >+ u32 *sum)
> >+{
> >+ int temp, ret;
> >+
> >+ ret = adc_to_temp_conversion(bg_ptr, i, adc_val,&temp);
> >+ if (ret< 0)
> >+ return ret;
> >+
> >+ temp += hyst_val;
> >+
> >+ return temp_to_adc_conversion(temp, bg_ptr, i, sum);
> >+}
> >+
> >+static
> >+int temp_sensor_configure_thot(struct omap_bandgap *bg_ptr, int id, int t_hot)
> >+{
> >+ struct temp_sensor_data *ts_data = bg_ptr->pdata->sensors[id].ts_data;
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 thresh_val, reg_val;
> >+ int cold, err;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+
> >+ /* obtain the T cold value */
> >+ err = omap_control_readl(cdev, tsr->bgap_threshold,&thresh_val);
> >+ cold = (thresh_val& tsr->threshold_tcold_mask)>>
> >+ __ffs(tsr->threshold_tcold_mask);
> >+ if (t_hot<= cold) {
> >+ /* change the t_cold to t_hot - 5000 millidegrees */
> >+ err |= add_hyst(t_hot, -ts_data->hyst_val, bg_ptr, id,&cold);
> >+ /* write the new t_cold value */
> >+ reg_val = thresh_val& (~tsr->threshold_tcold_mask);
> >+ reg_val |= cold<< __ffs(tsr->threshold_tcold_mask);
> >+ err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
> >+ thresh_val = reg_val;
> >+ }
> >+
> >+ /* write the new t_hot value */
> >+ reg_val = thresh_val& ~tsr->threshold_thot_mask;
> >+ reg_val |= (t_hot<< __ffs(tsr->threshold_thot_mask));
> >+ err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
> >+ if (err) {
> >+ dev_err(bg_ptr->dev, "failed to reprogram thot threshold\n");
> >+ return -EIO;
> >+ }
> >+
> >+ return temp_sensor_unmask_interrupts(bg_ptr, id, t_hot, cold);
> >+}
> >+
> >+static
> >+int temp_sensor_init_talert_thresholds(struct omap_bandgap *bg_ptr, int id,
> >+ int t_hot, int t_cold)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 reg_val, thresh_val;
> >+ int err;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+ err = omap_control_readl(cdev, tsr->bgap_threshold,&thresh_val);
> >+
> >+ /* write the new t_cold value */
> >+ reg_val = thresh_val& ~tsr->threshold_tcold_mask;
> >+ reg_val |= (t_cold<< __ffs(tsr->threshold_tcold_mask));
> >+ err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
> >+ if (err) {
> >+ dev_err(bg_ptr->dev, "failed to reprogram tcold threshold\n");
> >+ return -EIO;
> >+ }
> >+
> >+ err = omap_control_readl(cdev, tsr->bgap_threshold,&thresh_val);
> >+
> >+ /* write the new t_hot value */
> >+ reg_val = thresh_val& ~tsr->threshold_thot_mask;
> >+ reg_val |= (t_hot<< __ffs(tsr->threshold_thot_mask));
> >+ err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
> >+ if (err) {
> >+ dev_err(bg_ptr->dev, "failed to reprogram thot threshold\n");
> >+ return -EIO;
> >+ }
> >+
> >+ err = omap_control_readl(cdev, tsr->bgap_mask_ctrl,®_val);
> >+ reg_val |= tsr->mask_hot_mask;
> >+ reg_val |= tsr->mask_cold_mask;
> >+ err |= omap_control_writel(cdev, reg_val, tsr->bgap_mask_ctrl);
> >+ if (err) {
> >+ dev_err(bg_ptr->dev, "failed to reprogram thot threshold\n");
> >+ return -EIO;
> >+ }
> >+
> >+ return err;
> >+}
> >+
> >+static
> >+int temp_sensor_configure_tcold(struct omap_bandgap *bg_ptr, int id,
> >+ int t_cold)
> >+{
> >+ struct temp_sensor_data *ts_data = bg_ptr->pdata->sensors[id].ts_data;
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 thresh_val, reg_val;
> >+ int hot, err;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+ /* obtain the T cold value */
> >+ err = omap_control_readl(cdev, tsr->bgap_threshold,&thresh_val);
> >+ hot = (thresh_val& tsr->threshold_thot_mask)>>
> >+ __ffs(tsr->threshold_thot_mask);
> >+
> >+ if (t_cold>= hot) {
> >+ /* change the t_hot to t_cold + 5000 millidegrees */
> >+ err |= add_hyst(t_cold, ts_data->hyst_val, bg_ptr, id,&hot);
> >+ /* write the new t_hot value */
> >+ reg_val = thresh_val& (~tsr->threshold_thot_mask);
> >+ reg_val |= hot<< __ffs(tsr->threshold_thot_mask);
> >+ err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
> >+ thresh_val = reg_val;
> >+ }
> >+
> >+ /* write the new t_cold value */
> >+ reg_val = thresh_val& ~tsr->threshold_tcold_mask;
> >+ reg_val |= (t_cold<< __ffs(tsr->threshold_tcold_mask));
> >+ err |= omap_control_writel(cdev, reg_val, tsr->bgap_threshold);
> >+ if (err) {
> >+ dev_err(bg_ptr->dev, "failed to reprogram tcold threshold\n");
> >+ return -EIO;
> >+ }
> >+
> >+ return temp_sensor_unmask_interrupts(bg_ptr, id, hot, t_cold);
> >+}
> >+
> >+static int temp_sensor_configure_tshut_hot(struct omap_bandgap *bg_ptr,
> >+ int id, int tshut_hot)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 reg_val;
> >+ int err;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+ err = omap_control_readl(cdev, tsr->tshut_threshold,®_val);
> >+ reg_val&= ~tsr->tshut_hot_mask;
> >+ reg_val |= tshut_hot<< __ffs(tsr->tshut_hot_mask);
> >+ err |= omap_control_writel(cdev, reg_val, tsr->tshut_threshold);
> >+ if (err) {
> >+ dev_err(bg_ptr->dev, "failed to reprogram tshut thot\n");
> >+ return -EIO;
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> >+static int temp_sensor_configure_tshut_cold(struct omap_bandgap *bg_ptr,
> >+ int id, int tshut_cold)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 reg_val;
> >+ int err;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+ err = omap_control_readl(cdev, tsr->tshut_threshold,®_val);
> >+ reg_val&= ~tsr->tshut_cold_mask;
> >+ reg_val |= tshut_cold<< __ffs(tsr->tshut_cold_mask);
> >+ err |= omap_control_writel(cdev, reg_val, tsr->tshut_threshold);
> >+ if (err) {
> >+ dev_err(bg_ptr->dev, "failed to reprogram tshut tcold\n");
> >+ return -EIO;
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> >+static int configure_temp_sensor_counter(struct omap_bandgap *bg_ptr, int id,
> >+ u32 counter)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 val;
> >+ int err;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+ err = omap_control_readl(cdev, tsr->bgap_counter,&val);
> >+ val&= ~tsr->counter_mask;
> >+ val |= counter<< __ffs(tsr->counter_mask);
> >+ err |= omap_control_writel(cdev, val, tsr->bgap_counter);
> >+ if (err) {
> >+ dev_err(bg_ptr->dev, "failed to reprogram tshut tcold\n");
> >+ return -EIO;
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> >+/* Exposed APIs */
> >+/**
> >+ * omap_bandgap_read_thot() - reads sensor current thot
> >+ * @bg_ptr - pointer to bandgap instance
> >+ * @id - sensor id
> >+ * @thot - resulting current thot value
> >+ *
> >+ * returns 0 on success or the proper error code
> >+ */
> >+int omap_bandgap_read_thot(struct omap_bandgap *bg_ptr, int id,
> >+ int *thot)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 temp;
> >+ int ret;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+ ret = omap_control_readl(cdev, tsr->bgap_threshold,&temp);
> >+ temp = (temp& tsr->threshold_thot_mask)>>
> >+ __ffs(tsr->threshold_thot_mask);
> >+ ret |= adc_to_temp_conversion(bg_ptr, id, temp,&temp);
> >+ if (ret) {
> >+ dev_err(bg_ptr->dev, "failed to read thot\n");
> >+ return -EIO;
> >+ }
> >+
> >+ *thot = temp;
> >+
> >+ return 0;
> >+}
> >+
> >+/**
> >+ * omap_bandgap_write_thot() - sets sensor current thot
> >+ * @bg_ptr - pointer to bandgap instance
> >+ * @id - sensor id
> >+ * @val - desired thot value
> >+ *
> >+ * returns 0 on success or the proper error code
> >+ */
> >+int omap_bandgap_write_thot(struct omap_bandgap *bg_ptr, int id, int val)
> >+{
> >+ struct temp_sensor_data *ts_data = bg_ptr->pdata->sensors[id].ts_data;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 t_hot;
> >+ int ret;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+
> >+ if (val< ts_data->min_temp + ts_data->hyst_val)
> >+ return -EINVAL;
> >+ ret = temp_to_adc_conversion(val, bg_ptr, id,&t_hot);
> >+ if (ret< 0)
> >+ return ret;
> >+
> >+ mutex_lock(&bg_ptr->bg_mutex);
> >+ temp_sensor_configure_thot(bg_ptr, id, t_hot);
> >+ mutex_unlock(&bg_ptr->bg_mutex);
> >+
> >+ return 0;
> >+}
> >+
> >+/**
> >+ * omap_bandgap_read_tcold() - reads sensor current tcold
> >+ * @bg_ptr - pointer to bandgap instance
> >+ * @id - sensor id
> >+ * @tcold - resulting current tcold value
> >+ *
> >+ * returns 0 on success or the proper error code
> >+ */
> >+int omap_bandgap_read_tcold(struct omap_bandgap *bg_ptr, int id,
> >+ int *tcold)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 temp;
> >+ int ret;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+ ret = omap_control_readl(cdev, tsr->bgap_threshold,&temp);
> >+ temp = (temp& tsr->threshold_tcold_mask)
> >+ >> __ffs(tsr->threshold_tcold_mask);
> >+ ret |= adc_to_temp_conversion(bg_ptr, id, temp,&temp);
> >+ if (ret)
> >+ return -EIO;
> >+
> >+ *tcold = temp;
> >+
> >+ return 0;
> >+}
> >+
> >+/**
> >+ * omap_bandgap_write_tcold() - sets the sensor tcold
> >+ * @bg_ptr - pointer to bandgap instance
> >+ * @id - sensor id
> >+ * @val - desired tcold value
> >+ *
> >+ * returns 0 on success or the proper error code
> >+ */
> >+int omap_bandgap_write_tcold(struct omap_bandgap *bg_ptr, int id, int val)
> >+{
> >+ struct temp_sensor_data *ts_data = bg_ptr->pdata->sensors[id].ts_data;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 t_cold;
> >+ int ret;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+ if (val> ts_data->max_temp + ts_data->hyst_val)
> >+ return -EINVAL;
> >+
> >+ ret = temp_to_adc_conversion(val, bg_ptr, id,&t_cold);
> >+ if (ret< 0)
> >+ return ret;
> >+
> >+ mutex_lock(&bg_ptr->bg_mutex);
> >+ temp_sensor_configure_tcold(bg_ptr, id, t_cold);
> >+ mutex_unlock(&bg_ptr->bg_mutex);
> >+
> >+ return 0;
> >+}
> >+
> >+/**
> >+ * omap_bandgap_read_update_interval() - read the sensor update interval
> >+ * @bg_ptr - pointer to bandgap instance
> >+ * @id - sensor id
> >+ * @interval - resulting update interval in miliseconds
> >+ *
> >+ * returns 0 on success or the proper error code
> >+ */
> >+int omap_bandgap_read_update_interval(struct omap_bandgap *bg_ptr, int id,
> >+ int *interval)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 time;
> >+ int ret;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+ ret = omap_control_readl(cdev, tsr->bgap_counter,&time);
> >+ if (ret)
> >+ return ret;
> >+ time = (time& tsr->counter_mask)>> __ffs(tsr->counter_mask);
> >+ time = time * 1000 / bg_ptr->clk_rate;
> >+
> >+ *interval = time;
> >+
> >+ return 0;
> >+}
> >+
> >+/**
> >+ * omap_bandgap_write_update_interval() - set the update interval
> >+ * @bg_ptr - pointer to bandgap instance
> >+ * @id - sensor id
> >+ * @interval - desired update interval in miliseconds
> >+ *
> >+ * returns 0 on success or the proper error code
> >+ */
> >+int omap_bandgap_write_update_interval(struct omap_bandgap *bg_ptr,
> >+ int id, u32 interval)
> >+{
> >+ interval = interval * bg_ptr->clk_rate / 1000;
> >+ mutex_lock(&bg_ptr->bg_mutex);
> >+ configure_temp_sensor_counter(bg_ptr, id, interval);
> >+ mutex_unlock(&bg_ptr->bg_mutex);
> >+
> >+ return 0;
> >+}
> >+
> >+/**
> >+ * omap_bandgap_read_temperature() - report current temperature
> >+ * @bg_ptr - pointer to bandgap instance
> >+ * @id - sensor id
> >+ * @temperature - resulting temperature
> >+ *
> >+ * returns 0 on success or the proper error code
> >+ */
> >+int omap_bandgap_read_temperature(struct omap_bandgap *bg_ptr, int id,
> >+ int *temperature)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 temp;
> >+ int ret;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+ ret = omap_control_readl(cdev, tsr->temp_sensor_ctrl,&temp);
> >+ temp&= tsr->bgap_dtemp_mask;
> >+
> >+ ret |= adc_to_temp_conversion(bg_ptr, id, temp,&temp);
> >+ if (ret)
> >+ return -EIO;
> >+
> >+ *temperature = temp;
> >+
> >+ return 0;
> >+}
> >+
> >+/**
> >+ * enable_continuous_mode() - One time enabling of continuous conversion mode
> >+ * @bg_ptr - pointer to scm instance
> >+ */
> >+static int enable_continuous_mode(struct omap_bandgap *bg_ptr)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ int i, r;
> >+ u32 val;
> >+
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++) {
> >+ tsr = bg_ptr->pdata->sensors[i].registers;
> >+ r = omap_control_readl(cdev, tsr->bgap_mode_ctrl,&val);
> >+ val |= 1<< __ffs(tsr->mode_ctrl_mask);
> >+ r |= omap_control_writel(cdev, val, tsr->bgap_mode_ctrl);
> >+ if (r)
> >+ dev_err(bg_ptr->dev, "could not save sensor %d\n", i);
> >+ }
> >+
> >+ return r ? -EIO : 0;
> >+}
> >+
> >+static int omap_bandgap_tshut_init(struct omap_bandgap *bg_ptr,
> >+ struct platform_device *pdev)
> >+{
> >+ int gpio_nr = bg_ptr->tshut_gpio;
> >+ int status;
> >+
> >+ /* Request for gpio_86 line */
> >+ status = gpio_request(gpio_nr, "tshut");
> >+ if (status< 0) {
> >+ dev_err(bg_ptr->dev,
> >+ "Could not request for TSHUT GPIO:%i\n", 86);
> >+ return status;
> >+ }
> >+ status = gpio_direction_input(gpio_nr);
> >+ if (status) {
> >+ dev_err(bg_ptr->dev,
> >+ "Cannot set input TSHUT GPIO %d\n", gpio_nr);
> >+ return status;
> >+ }
> >+
> >+ status = request_irq(gpio_to_irq(gpio_nr),
> >+ omap_bandgap_tshut_irq_handler,
> >+ IRQF_TRIGGER_RISING, "tshut",
> >+ NULL);
> >+ if (status) {
> >+ gpio_free(gpio_nr);
> >+ dev_err(bg_ptr->dev, "request irq failed for TSHUT");
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> >+static int omap_bandgap_talert_init(struct omap_bandgap *bg_ptr,
> >+ struct platform_device *pdev)
> >+{
> >+ int ret;
> >+
> >+ bg_ptr->irq = platform_get_irq(pdev, 0);
> >+ if (bg_ptr->irq< 0) {
> >+ dev_err(&pdev->dev, "get_irq failed\n");
> >+ return bg_ptr->irq;
> >+ }
> >+ ret = request_threaded_irq(bg_ptr->irq, NULL,
> >+ talert_irq_handler,
> >+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> >+ "talert", bg_ptr);
> >+ if (ret) {
> >+ dev_err(&pdev->dev, "Request threaded irq failed.\n");
> >+ return ret;
> >+ }
> >+
> >+ return 0;
> >+}
> >+
> >+static const struct omap_bandgap_data omap4460_data = {
> >+ .has_talert = true,
> >+ .has_tshut = true,
> >+ .fclock_name = "bandgap_ts_fclk",
> >+ .div_ck_name = "div_ts_ck",
>
> None of these clock data should be there ideally. You should ensure
> that the proper device alias will be there using clkdev entries.
>
> Except that with DT, it will not work without the clock DT binding :-(
>
> I think Rob posted a latest update based on CCF... but for the
> moment we are stuck :-(
>
> >+ .conv_table = omap4460_adc_to_temp,
> >+ .sensors = {
> >+ {
> >+ .registers =&omap4460_mpu_temp_sensor_registers,
> >+ .ts_data =&omap4460_mpu_temp_sensor_data,
> >+ .domain = "cpu",
> >+ },
> >+ },
> >+ .sensor_count = 1,
> >+};
> >+
> >+static const struct omap_bandgap_data omap5430_data = {
> >+ .has_talert = true,
> >+ .has_tshut = true,
> >+ .fclock_name = "ts_clk_div_ck",
> >+ .div_ck_name = "ts_clk_div_ck",
> >+ .conv_table = omap5430_adc_to_temp,
> >+ .sensors = {
> >+ {
> >+ .registers =&omap5430_mpu_temp_sensor_registers,
> >+ .ts_data =&omap5430_mpu_temp_sensor_data,
> >+ .domain = "cpu",
> >+ },
> >+ {
> >+ .registers =&omap5430_gpu_temp_sensor_registers,
> >+ .ts_data =&omap5430_gpu_temp_sensor_data,
> >+ .domain = "gpu",
> >+ },
> >+ {
> >+ .registers =&omap5430_core_temp_sensor_registers,
> >+ .ts_data =&omap5430_core_temp_sensor_data,
> >+ .domain = "core",
> >+ },
> >+ },
> >+ .sensor_count = 3,
>
> It can probably be replaced by a sizeof.
>
> >+};
> >+
> >+static const struct of_device_id of_omap_bandgap_match[] = {
> >+ /*
> >+ * TODO: Add support to 4430
> >+ * { .compatible = "ti,omap4430-bandgap", .data = , },
> >+ */
> >+ {
> >+ .compatible = "ti,omap4460-bandgap",
> >+ .data = (void *)&omap4460_data,
>
> No need to cast toward a void *.
>
> >+ },
> >+ {
> >+ .compatible = "ti,omap5430-bandgap",
> >+ .data = (void *)&omap5430_data,
> >+ },
> >+ /* Sentinel */
> >+ { },
> >+};
> >+
> >+static struct omap_bandgap *omap_bandgap_build(struct platform_device *pdev)
> >+{
> >+ struct device_node *node = pdev->dev.of_node;
> >+ const struct of_device_id *of_id;
> >+ struct omap_bandgap *bg_ptr;
>
> bg_ptr is not a super name.
>
> >+ u32 prop;
> >+
> >+ /* just for the sake */
> >+ if (!node) {
> >+ dev_err(&pdev->dev, "no platform information available\n");
> >+ return ERR_PTR(-EINVAL);
> >+ }
>
> Not needed, just do the of_match_device here directly.
>
> >+
> >+ bg_ptr = devm_kzalloc(&pdev->dev, sizeof(struct omap_bandgap),
> >+ GFP_KERNEL);
> >+ if (!bg_ptr) {
> >+ dev_err(&pdev->dev, "Unable to allocate mem for driver ref\n");
> >+ return ERR_PTR(-ENOMEM);
> >+ }
> >+
> >+ of_id = of_match_device(of_omap_bandgap_match,&pdev->dev);
> >+ if (of_id)
> >+ bg_ptr->pdata = of_id->data;
>
> Nit: This is not really pdata anymore, so you should maybe remove
> the "p" to avoid confusion.
>
> >+
> >+ if (bg_ptr->pdata->has_tshut) {
> >+ if (of_property_read_u32(node, "ti,tshut-gpio",&prop)< 0) {
> >+ dev_err(&pdev->dev, "missing tshut gpio in device tree\n");
> >+ return ERR_PTR(-EINVAL);
> >+ }
> >+ bg_ptr->tshut_gpio = prop;
> >+ if (!gpio_is_valid(bg_ptr->tshut_gpio)) {
> >+ dev_err(&pdev->dev, "invalid gpio for tshut (%d)\n",
> >+ bg_ptr->tshut_gpio);
> >+ return ERR_PTR(-EINVAL);
> >+ }
> >+ }
> >+
> >+ return bg_ptr;
> >+}
> >+
> >+static
> >+int __devinit omap_bandgap_probe(struct platform_device *pdev)
> >+{
> >+ struct device *cdev = pdev->dev.parent;
> >+ struct omap_bandgap *bg_ptr;
> >+ int clk_rate, ret = 0, i;
> >+
> >+ if (!cdev) {
> >+ dev_err(&pdev->dev, "no omap control ref in our parent\n");
> >+ return -EINVAL;
> >+ }
> >+
> >+ bg_ptr = omap_bandgap_build(pdev);
> >+ if (IS_ERR_OR_NULL(bg_ptr)) {
> >+ dev_err(&pdev->dev, "failed to fetch platform data\n");
> >+ return PTR_ERR(bg_ptr);
> >+ }
> >+
> >+ if (bg_ptr->pdata->has_talert) {
>
> Nit2: Yeah, in fact instead of pdata, "conf" or "settings" seems to
> be more representative of what this structure really contain.
>
> >+ ret = omap_bandgap_talert_init(bg_ptr, pdev);
> >+ if (ret) {
> >+ dev_err(&pdev->dev, "failed to initialize Talert IRQ\n");
> >+ return ret;
> >+ }
> >+ }
> >+
> >+ if (bg_ptr->pdata->has_tshut) {
> >+ ret = omap_bandgap_tshut_init(bg_ptr, pdev);
> >+ if (ret) {
> >+ dev_err(&pdev->dev,
> >+ "failed to initialize system tshut IRQ\n");
> >+ goto free_talert;
> >+ }
> >+ }
> >+
> >+ bg_ptr->fclock = clk_get(NULL, bg_ptr->pdata->fclock_name);
>
> That's not good to get a clock without using the local dev alias.
> But because of lack of clock DT binding yet, I'm not sure we have
> the choice.
>
> >+ ret = IS_ERR_OR_NULL(bg_ptr->fclock);
> >+ if (ret) {
> >+ dev_err(&pdev->dev, "failed to request fclock reference\n");
> >+ goto free_irqs;
> >+ }
> >+
> >+ bg_ptr->div_clk = clk_get(NULL, bg_ptr->pdata->div_ck_name);
> >+ ret = IS_ERR_OR_NULL(bg_ptr->div_clk);
> >+ if (ret) {
> >+ dev_err(&pdev->dev,
> >+ "failed to request div_ts_ck clock ref\n");
> >+ goto free_irqs;
> >+ }
> >+
> >+ bg_ptr->conv_table = bg_ptr->pdata->conv_table;
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++) {
> >+ struct temp_sensor_registers *tsr;
> >+ u32 val;
> >+
> >+ tsr = bg_ptr->pdata->sensors[i].registers;
> >+ /*
> >+ * check if the efuse has a non-zero value if not
> >+ * it is an untrimmed sample and the temperatures
> >+ * may not be accurate
> >+ */
> >+ ret = omap_control_readl(cdev, tsr->bgap_efuse,&val);
> >+ if (ret || !val)
> >+ dev_info(&pdev->dev,
> >+ "Non-trimmed BGAP, Temp not accurate\n");
> >+ }
> >+
> >+ clk_rate = clk_round_rate(bg_ptr->div_clk,
> >+ bg_ptr->pdata->sensors[0].ts_data->max_freq);
> >+ if (clk_rate< bg_ptr->pdata->sensors[0].ts_data->min_freq ||
> >+ clk_rate == 0xffffffff) {
> >+ ret = -ENODEV;
> >+ goto put_clks;
> >+ }
> >+
> >+ ret = clk_set_rate(bg_ptr->div_clk, clk_rate);
> >+ if (ret) {
> >+ dev_err(&pdev->dev, "Cannot set clock rate\n");
> >+ goto put_clks;
> >+ }
> >+
> >+ bg_ptr->clk_rate = clk_rate;
> >+ clk_enable(bg_ptr->fclock);
> >+
> >+ mutex_init(&bg_ptr->bg_mutex);
> >+ bg_ptr->dev =&pdev->dev;
> >+ platform_set_drvdata(pdev, bg_ptr);
> >+
> >+ /* 1 clk cycle */
>
> What does that mean exactly?
>
> Regards,
> Benoit
>
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++)
> >+ configure_temp_sensor_counter(bg_ptr, i, 1);
> >+
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++) {
> >+ struct temp_sensor_data *ts_data;
> >+
> >+ ts_data = bg_ptr->pdata->sensors[i].ts_data;
> >+
> >+ temp_sensor_init_talert_thresholds(bg_ptr, i,
> >+ ts_data->t_hot,
> >+ ts_data->t_cold);
> >+ temp_sensor_configure_tshut_hot(bg_ptr, i,
> >+ ts_data->tshut_hot);
> >+ temp_sensor_configure_tshut_cold(bg_ptr, i,
> >+ ts_data->tshut_cold);
> >+ }
> >+
> >+ enable_continuous_mode(bg_ptr);
> >+
> >+ /* Set .250 seconds time as default counter */
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++)
> >+ configure_temp_sensor_counter(bg_ptr, i,
> >+ bg_ptr->clk_rate / 4);
> >+
> >+ /* Every thing is good? Then expose the sensors */
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++) {
> >+ char *domain;
> >+
> >+ domain = bg_ptr->pdata->sensors[i].domain;
> >+ if (bg_ptr->pdata->expose_sensor)
> >+ bg_ptr->pdata->expose_sensor(bg_ptr, i, domain);
> >+ }
> >+
> >+ return 0;
> >+
> >+put_clks:
> >+ clk_disable(bg_ptr->fclock);
> >+ clk_put(bg_ptr->fclock);
> >+ clk_put(bg_ptr->div_clk);
> >+free_irqs:
> >+ free_irq(gpio_to_irq(bg_ptr->tshut_gpio), NULL);
> >+ gpio_free(bg_ptr->tshut_gpio);
> >+free_talert:
> >+ free_irq(bg_ptr->irq, bg_ptr);
> >+
> >+ return ret;
> >+}
> >+
> >+static
> >+int __devexit omap_bandgap_remove(struct platform_device *pdev)
> >+{
> >+ struct omap_bandgap *bg_ptr = platform_get_drvdata(pdev);
> >+
> >+ clk_disable(bg_ptr->fclock);
> >+ clk_put(bg_ptr->fclock);
> >+ clk_put(bg_ptr->div_clk);
> >+ free_irq(bg_ptr->irq, bg_ptr);
> >+ free_irq(gpio_to_irq(bg_ptr->tshut_gpio), NULL);
> >+ gpio_free(bg_ptr->tshut_gpio);
> >+
> >+ return 0;
> >+}
> >+
> >+#ifdef CONFIG_PM
> >+static int omap_bandgap_save_ctxt(struct omap_bandgap *bg_ptr)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ int err = 0;
> >+ int i;
> >+
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++) {
> >+ struct temp_sensor_registers *tsr;
> >+ struct temp_sensor_regval *rval;
> >+
> >+ rval = bg_ptr->pdata->sensors[i].regval;
> >+ tsr = bg_ptr->pdata->sensors[i].registers;
> >+
> >+ err = omap_control_readl(cdev, tsr->bgap_mode_ctrl,
> >+ &rval->bg_mode_ctrl);
> >+ err |= omap_control_readl(cdev, tsr->bgap_mask_ctrl,
> >+ &rval->bg_ctrl);
> >+ err |= omap_control_readl(cdev, tsr->bgap_counter,
> >+ &rval->bg_counter);
> >+ err |= omap_control_readl(cdev, tsr->bgap_threshold,
> >+ &rval->bg_threshold);
> >+ err |= omap_control_readl(cdev, tsr->tshut_threshold,
> >+ &rval->tshut_threshold);
> >+
> >+ if (err)
> >+ dev_err(bg_ptr->dev, "could not save sensor %d\n", i);
> >+ }
> >+
> >+ return err ? -EIO : 0;
> >+}
> >+
> >+static int
> >+omap_bandgap_force_single_read(struct omap_bandgap *bg_ptr, int id)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 temp = 0, counter = 1000;
> >+ int err;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+ /* Select single conversion mode */
> >+ err = omap_control_readl(cdev, tsr->bgap_mode_ctrl,&temp);
> >+ temp&= ~(1<< __ffs(tsr->mode_ctrl_mask));
> >+ omap_control_writel(cdev, temp, tsr->bgap_mode_ctrl);
> >+
> >+ /* Start of Conversion = 1 */
> >+ err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl,&temp);
> >+ temp |= 1<< __ffs(tsr->bgap_soc_mask);
> >+ omap_control_writel(cdev, temp, tsr->temp_sensor_ctrl);
> >+ /* Wait until DTEMP is updated */
> >+ err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl,&temp);
> >+ temp&= (tsr->bgap_dtemp_mask);
> >+ while ((temp == 0)&& --counter) {
> >+ err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl,&temp);
> >+ temp&= (tsr->bgap_dtemp_mask);
> >+ }
> >+ /* Start of Conversion = 0 */
> >+ err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl,&temp);
> >+ temp&= ~(1<< __ffs(tsr->bgap_soc_mask));
> >+ err |= omap_control_writel(cdev, temp, tsr->temp_sensor_ctrl);
> >+
> >+ return err ? -EIO : 0;
> >+}
> >+
> >+static int omap_bandgap_restore_ctxt(struct omap_bandgap *bg_ptr)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ int i, err = 0;
> >+ u32 temp = 0;
> >+
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++) {
> >+ struct temp_sensor_registers *tsr;
> >+ struct temp_sensor_regval *rval;
> >+ u32 val;
> >+
> >+ rval = bg_ptr->pdata->sensors[i].regval;
> >+ tsr = bg_ptr->pdata->sensors[i].registers;
> >+
> >+ err = omap_control_readl(cdev, tsr->bgap_counter,&val);
> >+ if (val == 0) {
> >+ err |= omap_control_writel(cdev, rval->bg_threshold,
> >+ tsr->bgap_threshold);
> >+ err |= omap_control_writel(cdev, rval->tshut_threshold,
> >+ tsr->tshut_threshold);
> >+ /* Force immediate temperature measurement and update
> >+ * of the DTEMP field
> >+ */
> >+ omap_bandgap_force_single_read(bg_ptr, i);
> >+ err |= omap_control_writel(cdev, rval->bg_counter,
> >+ tsr->bgap_counter);
> >+ err |= omap_control_writel(cdev, rval->bg_mode_ctrl,
> >+ tsr->bgap_mode_ctrl);
> >+ err |= omap_control_writel(cdev, rval->bg_ctrl,
> >+ tsr->bgap_mask_ctrl);
> >+ } else {
> >+ err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl,
> >+ &temp);
> >+ temp&= (tsr->bgap_dtemp_mask);
> >+ if (temp == 0) {
> >+ omap_bandgap_force_single_read(bg_ptr, i);
> >+ err |= omap_control_readl(cdev,
> >+ tsr->bgap_mask_ctrl,
> >+ &temp);
> >+ temp |= 1<< __ffs(tsr->mode_ctrl_mask);
> >+ err |= omap_control_writel(cdev, temp,
> >+ tsr->bgap_mask_ctrl);
> >+ }
> >+ }
> >+ if (err)
> >+ dev_err(bg_ptr->dev, "could not save sensor %d\n", i);
> >+ }
> >+
> >+ return err ? -EIO : 0;
> >+}
> >+
> >+static int omap_bandgap_suspend(struct device *dev)
> >+{
> >+ struct omap_bandgap *bg_ptr = dev_get_drvdata(dev);
> >+ int err;
> >+
> >+ err = omap_bandgap_save_ctxt(bg_ptr);
> >+ clk_disable(bg_ptr->fclock);
> >+
> >+ return err;
> >+}
> >+
> >+static int omap_bandgap_resume(struct device *dev)
> >+{
> >+ struct omap_bandgap *bg_ptr = dev_get_drvdata(dev);
> >+
> >+ clk_enable(bg_ptr->fclock);
> >+
> >+ return omap_bandgap_restore_ctxt(bg_ptr);
> >+}
> >+static const struct dev_pm_ops omap_bandgap_dev_pm_ops = {
> >+ SET_SYSTEM_SLEEP_PM_OPS(omap_bandgap_suspend,
> >+ omap_bandgap_resume)
> >+};
> >+
> >+#define DEV_PM_OPS (&omap_bandgap_dev_pm_ops)
> >+#else
> >+#define DEV_PM_OPS NULL
> >+#endif
> >+
> >+static struct platform_driver omap_bandgap_sensor_driver = {
> >+ .probe = omap_bandgap_probe,
> >+ .remove = omap_bandgap_remove,
> >+ .driver = {
> >+ .name = "omap-bandgap",
> >+ .pm = DEV_PM_OPS,
> >+ .of_match_table = of_omap_bandgap_match,
> >+ },
> >+};
> >+
> >+module_platform_driver(omap_bandgap_sensor_driver);
> >+early_platform_init("early_omap_temperature",&omap_bandgap_sensor_driver);
> >+
> >+MODULE_DESCRIPTION("OMAP4+ bandgap temperature sensor driver");
> >+MODULE_LICENSE("GPL v2");
> >+MODULE_ALIAS("platform:omap-bandgap");
> >+MODULE_AUTHOR("Texas Instrument Inc.");
> >diff --git a/drivers/thermal/omap-bandgap.h b/drivers/thermal/omap-bandgap.h
> >new file mode 100644
> >index 0000000..12e0d6b
> >--- /dev/null
> >+++ b/drivers/thermal/omap-bandgap.h
> >@@ -0,0 +1,63 @@
> >+/*
> >+ * OMAP4 Bandgap temperature sensor driver
> >+ *
> >+ * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/
> >+ * Contact:
> >+ * Eduardo Valentin<eduardo.valentin@ti.com>
> >+ *
> >+ * This program is free software; you can redistribute it and/or
> >+ * modify it under the terms of the GNU General Public License
> >+ * version 2 as published by the Free Software Foundation.
> >+ *
> >+ * This program is distributed in the hope that it will be useful, but
> >+ * WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >+ * General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU General Public License
> >+ * along with this program; if not, write to the Free Software
> >+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> >+ * 02110-1301 USA
> >+ *
> >+ */
> >+#ifndef __OMAP_BANDGAP_H
> >+#define __OMAP_BANDGAP_H
> >+
> >+struct omap_bandgap_data;
> >+
> >+/**
> >+ * struct omap_bandgap - bandgap device structure
> >+ * @dev: device pointer
> >+ * @pdata: platform data with sensor data
> >+ * @fclock: pointer to functional clock of temperature sensor
> >+ * @div_clk: pointer to parent clock of temperature sensor fclk
> >+ * @conv_table: Pointer to adc to temperature conversion table
> >+ * @bg_mutex: Mutex for sysfs, irq and PM
> >+ * @irq: MPU Irq number for thermal alert
> >+ * @tshut_gpio: GPIO where Tshut signal is routed
> >+ * @clk_rate: Holds current clock rate
> >+ */
> >+struct omap_bandgap {
> >+ struct device *dev;
> >+ const struct omap_bandgap_data *pdata;
> >+ struct clk *fclock;
> >+ struct clk *div_clk;
> >+ const int *conv_table;
> >+ struct mutex bg_mutex; /* Mutex for irq and PM */
> >+ int irq;
> >+ int tshut_gpio;
> >+ u32 clk_rate;
> >+};
> >+
> >+int omap_bandgap_read_thot(struct omap_bandgap *bg_ptr, int id, int *thot);
> >+int omap_bandgap_write_thot(struct omap_bandgap *bg_ptr, int id, int val);
> >+int omap_bandgap_read_tcold(struct omap_bandgap *bg_ptr, int id, int *tcold);
> >+int omap_bandgap_write_tcold(struct omap_bandgap *bg_ptr, int id, int val);
> >+int omap_bandgap_read_update_interval(struct omap_bandgap *bg_ptr, int id,
> >+ int *interval);
> >+int omap_bandgap_write_update_interval(struct omap_bandgap *bg_ptr, int id,
> >+ u32 interval);
> >+int omap_bandgap_read_temperature(struct omap_bandgap *bg_ptr, int id,
> >+ int *temperature);
> >+
> >+#endif
>
^ permalink raw reply
* Re: [RFC PATCH 09/11] ARM: OMAP4+: thermal: introduce bandgap temperature sensor
From: Eduardo Valentin @ 2012-05-28 11:16 UTC (permalink / raw)
To: Cousson, Benoit
Cc: amit.kucheria, kbaidarov, kishon, balbi, Keerthy, linux-pm,
linux-omap, linux-arm-kernel
In-Reply-To: <4FBFAA18.4040103@ti.com>
Hello again,
On Fri, May 25, 2012 at 05:49:44PM +0200, Cousson Benoit wrote:
> On 5/25/2012 10:25 AM, Eduardo Valentin wrote:
<big cut>
> >+
> >+static const struct omap_bandgap_data omap4460_data = {
> >+ .has_talert = true,
> >+ .has_tshut = true,
> >+ .fclock_name = "bandgap_ts_fclk",
> >+ .div_ck_name = "div_ts_ck",
>
> None of these clock data should be there ideally. You should ensure
> that the proper device alias will be there using clkdev entries.
In fact, it is a shame that it would be needed to have this entries there :-(
>
> Except that with DT, it will not work without the clock DT binding :-(
>
> I think Rob posted a latest update based on CCF... but for the
> moment we are stuck :-(
OK. But would it work for BG as well as it seams to be a special case?
>
> >+ .conv_table = omap4460_adc_to_temp,
> >+ .sensors = {
> >+ {
> >+ .registers =&omap4460_mpu_temp_sensor_registers,
> >+ .ts_data =&omap4460_mpu_temp_sensor_data,
> >+ .domain = "cpu",
> >+ },
> >+ },
> >+ .sensor_count = 1,
> >+};
> >+
> >+static const struct omap_bandgap_data omap5430_data = {
> >+ .has_talert = true,
> >+ .has_tshut = true,
> >+ .fclock_name = "ts_clk_div_ck",
> >+ .div_ck_name = "ts_clk_div_ck",
> >+ .conv_table = omap5430_adc_to_temp,
> >+ .sensors = {
> >+ {
> >+ .registers =&omap5430_mpu_temp_sensor_registers,
> >+ .ts_data =&omap5430_mpu_temp_sensor_data,
> >+ .domain = "cpu",
> >+ },
> >+ {
> >+ .registers =&omap5430_gpu_temp_sensor_registers,
> >+ .ts_data =&omap5430_gpu_temp_sensor_data,
> >+ .domain = "gpu",
> >+ },
> >+ {
> >+ .registers =&omap5430_core_temp_sensor_registers,
> >+ .ts_data =&omap5430_core_temp_sensor_data,
> >+ .domain = "core",
> >+ },
> >+ },
> >+ .sensor_count = 3,
>
> It can probably be replaced by a sizeof.
ARRAY_SIZE prob, will check.
>
> >+};
> >+
> >+static const struct of_device_id of_omap_bandgap_match[] = {
> >+ /*
> >+ * TODO: Add support to 4430
> >+ * { .compatible = "ti,omap4430-bandgap", .data = , },
> >+ */
> >+ {
> >+ .compatible = "ti,omap4460-bandgap",
> >+ .data = (void *)&omap4460_data,
>
> No need to cast toward a void *.
In this case, there is a need, because the omap4460_data is const but the .data field isn't. So I need to force it.
>
> >+ },
> >+ {
> >+ .compatible = "ti,omap5430-bandgap",
> >+ .data = (void *)&omap5430_data,
> >+ },
> >+ /* Sentinel */
> >+ { },
> >+};
> >+
> >+static struct omap_bandgap *omap_bandgap_build(struct platform_device *pdev)
> >+{
> >+ struct device_node *node = pdev->dev.of_node;
> >+ const struct of_device_id *of_id;
> >+ struct omap_bandgap *bg_ptr;
>
> bg_ptr is not a super name.
Got a better name? Just don't want a long one to avoid code bending at 80th column...
>
> >+ u32 prop;
> >+
> >+ /* just for the sake */
> >+ if (!node) {
> >+ dev_err(&pdev->dev, "no platform information available\n");
> >+ return ERR_PTR(-EINVAL);
> >+ }
>
> Not needed, just do the of_match_device here directly.
Indeed...
>
> >+
> >+ bg_ptr = devm_kzalloc(&pdev->dev, sizeof(struct omap_bandgap),
> >+ GFP_KERNEL);
> >+ if (!bg_ptr) {
> >+ dev_err(&pdev->dev, "Unable to allocate mem for driver ref\n");
> >+ return ERR_PTR(-ENOMEM);
> >+ }
> >+
> >+ of_id = of_match_device(of_omap_bandgap_match,&pdev->dev);
> >+ if (of_id)
> >+ bg_ptr->pdata = of_id->data;
>
> Nit: This is not really pdata anymore, so you should maybe remove
> the "p" to avoid confusion.
OK...
>
> >+
> >+ if (bg_ptr->pdata->has_tshut) {
> >+ if (of_property_read_u32(node, "ti,tshut-gpio",&prop)< 0) {
> >+ dev_err(&pdev->dev, "missing tshut gpio in device tree\n");
> >+ return ERR_PTR(-EINVAL);
> >+ }
> >+ bg_ptr->tshut_gpio = prop;
> >+ if (!gpio_is_valid(bg_ptr->tshut_gpio)) {
> >+ dev_err(&pdev->dev, "invalid gpio for tshut (%d)\n",
> >+ bg_ptr->tshut_gpio);
> >+ return ERR_PTR(-EINVAL);
> >+ }
> >+ }
> >+
> >+ return bg_ptr;
> >+}
> >+
> >+static
> >+int __devinit omap_bandgap_probe(struct platform_device *pdev)
> >+{
> >+ struct device *cdev = pdev->dev.parent;
> >+ struct omap_bandgap *bg_ptr;
> >+ int clk_rate, ret = 0, i;
> >+
> >+ if (!cdev) {
> >+ dev_err(&pdev->dev, "no omap control ref in our parent\n");
> >+ return -EINVAL;
> >+ }
> >+
> >+ bg_ptr = omap_bandgap_build(pdev);
> >+ if (IS_ERR_OR_NULL(bg_ptr)) {
> >+ dev_err(&pdev->dev, "failed to fetch platform data\n");
> >+ return PTR_ERR(bg_ptr);
> >+ }
> >+
> >+ if (bg_ptr->pdata->has_talert) {
>
> Nit2: Yeah, in fact instead of pdata, "conf" or "settings" seems to
> be more representative of what this structure really contain.
conf looks good to me.
>
> >+ ret = omap_bandgap_talert_init(bg_ptr, pdev);
> >+ if (ret) {
> >+ dev_err(&pdev->dev, "failed to initialize Talert IRQ\n");
> >+ return ret;
> >+ }
> >+ }
> >+
> >+ if (bg_ptr->pdata->has_tshut) {
> >+ ret = omap_bandgap_tshut_init(bg_ptr, pdev);
> >+ if (ret) {
> >+ dev_err(&pdev->dev,
> >+ "failed to initialize system tshut IRQ\n");
> >+ goto free_talert;
> >+ }
> >+ }
> >+
> >+ bg_ptr->fclock = clk_get(NULL, bg_ptr->pdata->fclock_name);
>
> That's not good to get a clock without using the local dev alias.
> But because of lack of clock DT binding yet, I'm not sure we have
> the choice.
In fact I didn't touch the clk data on purpose and left the clock handling
as is. On my side I didn't know how the clock struct would look like with DT,
so, I didn't mess with it.
Do you have a reference to check the work in progress for clock DT ?
>
> >+ ret = IS_ERR_OR_NULL(bg_ptr->fclock);
> >+ if (ret) {
> >+ dev_err(&pdev->dev, "failed to request fclock reference\n");
> >+ goto free_irqs;
> >+ }
> >+
> >+ bg_ptr->div_clk = clk_get(NULL, bg_ptr->pdata->div_ck_name);
> >+ ret = IS_ERR_OR_NULL(bg_ptr->div_clk);
> >+ if (ret) {
> >+ dev_err(&pdev->dev,
> >+ "failed to request div_ts_ck clock ref\n");
> >+ goto free_irqs;
> >+ }
> >+
> >+ bg_ptr->conv_table = bg_ptr->pdata->conv_table;
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++) {
> >+ struct temp_sensor_registers *tsr;
> >+ u32 val;
> >+
> >+ tsr = bg_ptr->pdata->sensors[i].registers;
> >+ /*
> >+ * check if the efuse has a non-zero value if not
> >+ * it is an untrimmed sample and the temperatures
> >+ * may not be accurate
> >+ */
> >+ ret = omap_control_readl(cdev, tsr->bgap_efuse,&val);
> >+ if (ret || !val)
> >+ dev_info(&pdev->dev,
> >+ "Non-trimmed BGAP, Temp not accurate\n");
> >+ }
> >+
> >+ clk_rate = clk_round_rate(bg_ptr->div_clk,
> >+ bg_ptr->pdata->sensors[0].ts_data->max_freq);
> >+ if (clk_rate< bg_ptr->pdata->sensors[0].ts_data->min_freq ||
> >+ clk_rate == 0xffffffff) {
> >+ ret = -ENODEV;
> >+ goto put_clks;
> >+ }
> >+
> >+ ret = clk_set_rate(bg_ptr->div_clk, clk_rate);
> >+ if (ret) {
> >+ dev_err(&pdev->dev, "Cannot set clock rate\n");
> >+ goto put_clks;
> >+ }
> >+
> >+ bg_ptr->clk_rate = clk_rate;
> >+ clk_enable(bg_ptr->fclock);
> >+
> >+ mutex_init(&bg_ptr->bg_mutex);
> >+ bg_ptr->dev =&pdev->dev;
> >+ platform_set_drvdata(pdev, bg_ptr);
> >+
> >+ /* 1 clk cycle */
>
> What does that mean exactly?
That's indeed a good question. I guess by default we configure the bandgap to
wait only 1 cycle before it goes to the next read round, if in continuous mode.
That should get overwritten when we have, for instance, some policy initialized
and changing the update rate based on the temperature level that the sensor is.
These decisions would go under omapXX-thermal.c, BTW. Check my reply on your
suggestion for data structure split.
>
> Regards,
> Benoit
>
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++)
> >+ configure_temp_sensor_counter(bg_ptr, i, 1);
> >+
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++) {
> >+ struct temp_sensor_data *ts_data;
> >+
> >+ ts_data = bg_ptr->pdata->sensors[i].ts_data;
> >+
> >+ temp_sensor_init_talert_thresholds(bg_ptr, i,
> >+ ts_data->t_hot,
> >+ ts_data->t_cold);
> >+ temp_sensor_configure_tshut_hot(bg_ptr, i,
> >+ ts_data->tshut_hot);
> >+ temp_sensor_configure_tshut_cold(bg_ptr, i,
> >+ ts_data->tshut_cold);
> >+ }
> >+
> >+ enable_continuous_mode(bg_ptr);
> >+
> >+ /* Set .250 seconds time as default counter */
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++)
> >+ configure_temp_sensor_counter(bg_ptr, i,
> >+ bg_ptr->clk_rate / 4);
> >+
> >+ /* Every thing is good? Then expose the sensors */
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++) {
> >+ char *domain;
> >+
> >+ domain = bg_ptr->pdata->sensors[i].domain;
> >+ if (bg_ptr->pdata->expose_sensor)
> >+ bg_ptr->pdata->expose_sensor(bg_ptr, i, domain);
> >+ }
> >+
> >+ return 0;
> >+
> >+put_clks:
> >+ clk_disable(bg_ptr->fclock);
> >+ clk_put(bg_ptr->fclock);
> >+ clk_put(bg_ptr->div_clk);
> >+free_irqs:
> >+ free_irq(gpio_to_irq(bg_ptr->tshut_gpio), NULL);
> >+ gpio_free(bg_ptr->tshut_gpio);
> >+free_talert:
> >+ free_irq(bg_ptr->irq, bg_ptr);
> >+
> >+ return ret;
> >+}
> >+
> >+static
> >+int __devexit omap_bandgap_remove(struct platform_device *pdev)
> >+{
> >+ struct omap_bandgap *bg_ptr = platform_get_drvdata(pdev);
> >+
> >+ clk_disable(bg_ptr->fclock);
> >+ clk_put(bg_ptr->fclock);
> >+ clk_put(bg_ptr->div_clk);
> >+ free_irq(bg_ptr->irq, bg_ptr);
> >+ free_irq(gpio_to_irq(bg_ptr->tshut_gpio), NULL);
> >+ gpio_free(bg_ptr->tshut_gpio);
> >+
> >+ return 0;
> >+}
> >+
> >+#ifdef CONFIG_PM
> >+static int omap_bandgap_save_ctxt(struct omap_bandgap *bg_ptr)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ int err = 0;
> >+ int i;
> >+
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++) {
> >+ struct temp_sensor_registers *tsr;
> >+ struct temp_sensor_regval *rval;
> >+
> >+ rval = bg_ptr->pdata->sensors[i].regval;
> >+ tsr = bg_ptr->pdata->sensors[i].registers;
> >+
> >+ err = omap_control_readl(cdev, tsr->bgap_mode_ctrl,
> >+ &rval->bg_mode_ctrl);
> >+ err |= omap_control_readl(cdev, tsr->bgap_mask_ctrl,
> >+ &rval->bg_ctrl);
> >+ err |= omap_control_readl(cdev, tsr->bgap_counter,
> >+ &rval->bg_counter);
> >+ err |= omap_control_readl(cdev, tsr->bgap_threshold,
> >+ &rval->bg_threshold);
> >+ err |= omap_control_readl(cdev, tsr->tshut_threshold,
> >+ &rval->tshut_threshold);
> >+
> >+ if (err)
> >+ dev_err(bg_ptr->dev, "could not save sensor %d\n", i);
> >+ }
> >+
> >+ return err ? -EIO : 0;
> >+}
> >+
> >+static int
> >+omap_bandgap_force_single_read(struct omap_bandgap *bg_ptr, int id)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ struct temp_sensor_registers *tsr;
> >+ u32 temp = 0, counter = 1000;
> >+ int err;
> >+
> >+ tsr = bg_ptr->pdata->sensors[id].registers;
> >+ /* Select single conversion mode */
> >+ err = omap_control_readl(cdev, tsr->bgap_mode_ctrl,&temp);
> >+ temp&= ~(1<< __ffs(tsr->mode_ctrl_mask));
> >+ omap_control_writel(cdev, temp, tsr->bgap_mode_ctrl);
> >+
> >+ /* Start of Conversion = 1 */
> >+ err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl,&temp);
> >+ temp |= 1<< __ffs(tsr->bgap_soc_mask);
> >+ omap_control_writel(cdev, temp, tsr->temp_sensor_ctrl);
> >+ /* Wait until DTEMP is updated */
> >+ err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl,&temp);
> >+ temp&= (tsr->bgap_dtemp_mask);
> >+ while ((temp == 0)&& --counter) {
> >+ err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl,&temp);
> >+ temp&= (tsr->bgap_dtemp_mask);
> >+ }
> >+ /* Start of Conversion = 0 */
> >+ err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl,&temp);
> >+ temp&= ~(1<< __ffs(tsr->bgap_soc_mask));
> >+ err |= omap_control_writel(cdev, temp, tsr->temp_sensor_ctrl);
> >+
> >+ return err ? -EIO : 0;
> >+}
> >+
> >+static int omap_bandgap_restore_ctxt(struct omap_bandgap *bg_ptr)
> >+{
> >+ struct device *cdev = bg_ptr->dev->parent;
> >+ int i, err = 0;
> >+ u32 temp = 0;
> >+
> >+ for (i = 0; i< bg_ptr->pdata->sensor_count; i++) {
> >+ struct temp_sensor_registers *tsr;
> >+ struct temp_sensor_regval *rval;
> >+ u32 val;
> >+
> >+ rval = bg_ptr->pdata->sensors[i].regval;
> >+ tsr = bg_ptr->pdata->sensors[i].registers;
> >+
> >+ err = omap_control_readl(cdev, tsr->bgap_counter,&val);
> >+ if (val == 0) {
> >+ err |= omap_control_writel(cdev, rval->bg_threshold,
> >+ tsr->bgap_threshold);
> >+ err |= omap_control_writel(cdev, rval->tshut_threshold,
> >+ tsr->tshut_threshold);
> >+ /* Force immediate temperature measurement and update
> >+ * of the DTEMP field
> >+ */
> >+ omap_bandgap_force_single_read(bg_ptr, i);
> >+ err |= omap_control_writel(cdev, rval->bg_counter,
> >+ tsr->bgap_counter);
> >+ err |= omap_control_writel(cdev, rval->bg_mode_ctrl,
> >+ tsr->bgap_mode_ctrl);
> >+ err |= omap_control_writel(cdev, rval->bg_ctrl,
> >+ tsr->bgap_mask_ctrl);
> >+ } else {
> >+ err |= omap_control_readl(cdev, tsr->temp_sensor_ctrl,
> >+ &temp);
> >+ temp&= (tsr->bgap_dtemp_mask);
> >+ if (temp == 0) {
> >+ omap_bandgap_force_single_read(bg_ptr, i);
> >+ err |= omap_control_readl(cdev,
> >+ tsr->bgap_mask_ctrl,
> >+ &temp);
> >+ temp |= 1<< __ffs(tsr->mode_ctrl_mask);
> >+ err |= omap_control_writel(cdev, temp,
> >+ tsr->bgap_mask_ctrl);
> >+ }
> >+ }
> >+ if (err)
> >+ dev_err(bg_ptr->dev, "could not save sensor %d\n", i);
> >+ }
> >+
> >+ return err ? -EIO : 0;
> >+}
> >+
> >+static int omap_bandgap_suspend(struct device *dev)
> >+{
> >+ struct omap_bandgap *bg_ptr = dev_get_drvdata(dev);
> >+ int err;
> >+
> >+ err = omap_bandgap_save_ctxt(bg_ptr);
> >+ clk_disable(bg_ptr->fclock);
> >+
> >+ return err;
> >+}
> >+
> >+static int omap_bandgap_resume(struct device *dev)
> >+{
> >+ struct omap_bandgap *bg_ptr = dev_get_drvdata(dev);
> >+
> >+ clk_enable(bg_ptr->fclock);
> >+
> >+ return omap_bandgap_restore_ctxt(bg_ptr);
> >+}
> >+static const struct dev_pm_ops omap_bandgap_dev_pm_ops = {
> >+ SET_SYSTEM_SLEEP_PM_OPS(omap_bandgap_suspend,
> >+ omap_bandgap_resume)
> >+};
> >+
> >+#define DEV_PM_OPS (&omap_bandgap_dev_pm_ops)
> >+#else
> >+#define DEV_PM_OPS NULL
> >+#endif
> >+
> >+static struct platform_driver omap_bandgap_sensor_driver = {
> >+ .probe = omap_bandgap_probe,
> >+ .remove = omap_bandgap_remove,
> >+ .driver = {
> >+ .name = "omap-bandgap",
> >+ .pm = DEV_PM_OPS,
> >+ .of_match_table = of_omap_bandgap_match,
> >+ },
> >+};
> >+
> >+module_platform_driver(omap_bandgap_sensor_driver);
> >+early_platform_init("early_omap_temperature",&omap_bandgap_sensor_driver);
> >+
> >+MODULE_DESCRIPTION("OMAP4+ bandgap temperature sensor driver");
> >+MODULE_LICENSE("GPL v2");
> >+MODULE_ALIAS("platform:omap-bandgap");
> >+MODULE_AUTHOR("Texas Instrument Inc.");
> >diff --git a/drivers/thermal/omap-bandgap.h b/drivers/thermal/omap-bandgap.h
> >new file mode 100644
> >index 0000000..12e0d6b
> >--- /dev/null
> >+++ b/drivers/thermal/omap-bandgap.h
> >@@ -0,0 +1,63 @@
> >+/*
> >+ * OMAP4 Bandgap temperature sensor driver
> >+ *
> >+ * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/
> >+ * Contact:
> >+ * Eduardo Valentin<eduardo.valentin@ti.com>
> >+ *
> >+ * This program is free software; you can redistribute it and/or
> >+ * modify it under the terms of the GNU General Public License
> >+ * version 2 as published by the Free Software Foundation.
> >+ *
> >+ * This program is distributed in the hope that it will be useful, but
> >+ * WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >+ * General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU General Public License
> >+ * along with this program; if not, write to the Free Software
> >+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
> >+ * 02110-1301 USA
> >+ *
> >+ */
> >+#ifndef __OMAP_BANDGAP_H
> >+#define __OMAP_BANDGAP_H
> >+
> >+struct omap_bandgap_data;
> >+
> >+/**
> >+ * struct omap_bandgap - bandgap device structure
> >+ * @dev: device pointer
> >+ * @pdata: platform data with sensor data
> >+ * @fclock: pointer to functional clock of temperature sensor
> >+ * @div_clk: pointer to parent clock of temperature sensor fclk
> >+ * @conv_table: Pointer to adc to temperature conversion table
> >+ * @bg_mutex: Mutex for sysfs, irq and PM
> >+ * @irq: MPU Irq number for thermal alert
> >+ * @tshut_gpio: GPIO where Tshut signal is routed
> >+ * @clk_rate: Holds current clock rate
> >+ */
> >+struct omap_bandgap {
> >+ struct device *dev;
> >+ const struct omap_bandgap_data *pdata;
> >+ struct clk *fclock;
> >+ struct clk *div_clk;
> >+ const int *conv_table;
> >+ struct mutex bg_mutex; /* Mutex for irq and PM */
> >+ int irq;
> >+ int tshut_gpio;
> >+ u32 clk_rate;
> >+};
> >+
> >+int omap_bandgap_read_thot(struct omap_bandgap *bg_ptr, int id, int *thot);
> >+int omap_bandgap_write_thot(struct omap_bandgap *bg_ptr, int id, int val);
> >+int omap_bandgap_read_tcold(struct omap_bandgap *bg_ptr, int id, int *tcold);
> >+int omap_bandgap_write_tcold(struct omap_bandgap *bg_ptr, int id, int val);
> >+int omap_bandgap_read_update_interval(struct omap_bandgap *bg_ptr, int id,
> >+ int *interval);
> >+int omap_bandgap_write_update_interval(struct omap_bandgap *bg_ptr, int id,
> >+ u32 interval);
> >+int omap_bandgap_read_temperature(struct omap_bandgap *bg_ptr, int id,
> >+ int *temperature);
> >+
> >+#endif
>
---
Eduardo
^ permalink raw reply
* Re: [RFC PATCH 08/11] ARM: OMAP4+: Adding the temperature sensor register set bit fields
From: Eduardo Valentin @ 2012-05-28 11:17 UTC (permalink / raw)
To: Cousson, Benoit
Cc: amit.kucheria, kbaidarov, kishon, balbi, Keerthy, linux-pm,
linux-omap, linux-arm-kernel
In-Reply-To: <4FBFA196.2030503@ti.com>
On Fri, May 25, 2012 at 05:13:26PM +0200, Cousson Benoit wrote:
> On 5/25/2012 10:25 AM, Eduardo Valentin wrote:
> >OMAP4460 specific temperature sensor register bit fields are added.
> >Existing OMAP4 entries are renamed to OMAP4430.
> >
> >Signed-off-by: Keerthy<j-keerthy@ti.com>
> >Signed-off-by: Eduardo Valentin<eduardo.valentin@ti.com>
> >---
> > arch/arm/mach-omap2/include/mach/control.h | 116 ++++++++++++++++++++++++++++
>
> Now that we do have a dedicated driver for the bandgap, I guess it
> will be better to put these defines inside the driver directly.
Agreed
>
> Benoit
>
> > 1 files changed, 116 insertions(+), 0 deletions(-)
> >
> >diff --git a/arch/arm/mach-omap2/include/mach/control.h b/arch/arm/mach-omap2/include/mach/control.h
> >index cf42764..171b504 100644
> >--- a/arch/arm/mach-omap2/include/mach/control.h
> >+++ b/arch/arm/mach-omap2/include/mach/control.h
> >@@ -230,6 +230,122 @@
> > /* OMAP44xx control McBSP padconf */
> > #define OMAP4_CTRL_MODULE_PAD_CORE_CONTROL_MCBSPLP 0x061c
> >
> >+/* TEMP_SENSOR OMAP4430 */
> >+#define OMAP4430_BGAP_TEMPSOFF_SHIFT 12
> >+#define OMAP4430_BGAP_TEMPSOFF_MASK (1<< 12)
> >+#define OMAP4430_BGAP_TSHUT_SHIFT 11
> >+#define OMAP4430_BGAP_TSHUT_MASK (1<< 11)
> >+#define OMAP4430_BGAP_TEMP_SENSOR_CONTCONV_SHIFT 10
> >+#define OMAP4430_BGAP_TEMP_SENSOR_CONTCONV_MASK (1<< 10)
> >+#define OMAP4430_BGAP_TEMP_SENSOR_SOC_SHIFT 9
> >+#define OMAP4430_BGAP_TEMP_SENSOR_SOC_MASK (1<< 9)
> >+#define OMAP4430_BGAP_TEMP_SENSOR_EOCZ_SHIFT 8
> >+#define OMAP4430_BGAP_TEMP_SENSOR_EOCZ_MASK (1<< 8)
> >+#define OMAP4430_BGAP_TEMP_SENSOR_DTEMP_SHIFT 0
> >+#define OMAP4430_BGAP_TEMP_SENSOR_DTEMP_MASK (0xff<< 0)
> >+
> >+/* TEMP_SENSOR OMAP4460 */
> >+#define OMAP4460_BGAP_TEMPSOFF_SHIFT 13
> >+#define OMAP4460_BGAP_TEMPSOFF_MASK (1<< 13)
> >+#define OMAP4460_BGAP_TEMP_SENSOR_SOC_SHIFT 11
> >+#define OMAP4460_BGAP_TEMP_SENSOR_SOC_MASK (1<< 11)
> >+#define OMAP4460_BGAP_TEMP_SENSOR_EOCZ_SHIFT 10
> >+#define OMAP4460_BGAP_TEMP_SENSOR_EOCZ_MASK (1<< 10)
> >+#define OMAP4460_BGAP_TEMP_SENSOR_DTEMP_SHIFT 0
> >+#define OMAP4460_BGAP_TEMP_SENSOR_DTEMP_MASK (0x3ff<< 0)
> >+
> >+/* BANDGAP_CTRL */
> >+#define OMAP4460_SINGLE_MODE_SHIFT 31
> >+#define OMAP4460_SINGLE_MODE_MASK (1<< 31)
> >+#define OMAP4460_MASK_HOT_SHIFT 1
> >+#define OMAP4460_MASK_HOT_MASK (1<< 1)
> >+#define OMAP4460_MASK_COLD_SHIFT 0
> >+#define OMAP4460_MASK_COLD_MASK (1<< 0)
> >+
> >+/* BANDGAP_COUNTER */
> >+#define OMAP4460_COUNTER_SHIFT 0
> >+#define OMAP4460_COUNTER_MASK (0xffffff<< 0)
> >+
> >+/* BANDGAP_THRESHOLD */
> >+#define OMAP4460_T_HOT_SHIFT 16
> >+#define OMAP4460_T_HOT_MASK (0x3ff<< 16)
> >+#define OMAP4460_T_COLD_SHIFT 0
> >+#define OMAP4460_T_COLD_MASK (0x3ff<< 0)
> >+
> >+/* TSHUT_THRESHOLD */
> >+#define OMAP4460_TSHUT_HOT_SHIFT 16
> >+#define OMAP4460_TSHUT_HOT_MASK (0x3ff<< 16)
> >+#define OMAP4460_TSHUT_COLD_SHIFT 0
> >+#define OMAP4460_TSHUT_COLD_MASK (0x3ff<< 0)
> >+
> >+/* BANDGAP_STATUS */
> >+#define OMAP4460_CLEAN_STOP_SHIFT 3
> >+#define OMAP4460_CLEAN_STOP_MASK (1<< 3)
> >+#define OMAP4460_BGAP_ALERT_SHIFT 2
> >+#define OMAP4460_BGAP_ALERT_MASK (1<< 2)
> >+#define OMAP4460_HOT_FLAG_SHIFT 1
> >+#define OMAP4460_HOT_FLAG_MASK (1<< 1)
> >+#define OMAP4460_COLD_FLAG_SHIFT 0
> >+#define OMAP4460_COLD_FLAG_MASK (1<< 0)
> >+
> >+/* TEMP_SENSOR OMAP5430 */
> >+#define OMAP5430_BGAP_TEMP_SENSOR_SOC_SHIFT 12
> >+#define OMAP5430_BGAP_TEMP_SENSOR_SOC_MASK (1<< 12)
> >+#define OMAP5430_BGAP_TEMPSOFF_SHIFT 11
> >+#define OMAP5430_BGAP_TEMPSOFF_MASK (1<< 11)
> >+#define OMAP5430_BGAP_TEMP_SENSOR_EOCZ_SHIFT 10
> >+#define OMAP5430_BGAP_TEMP_SENSOR_EOCZ_MASK (1<< 10)
> >+#define OMAP5430_BGAP_TEMP_SENSOR_DTEMP_SHIFT 0
> >+#define OMAP5430_BGAP_TEMP_SENSOR_DTEMP_MASK (0x3ff<< 0)
> >+
> >+/* BANDGAP_CTRL */
> >+#define OMAP5430_MASK_HOT_CORE_SHIFT 5
> >+#define OMAP5430_MASK_HOT_CORE_MASK (1<< 5)
> >+#define OMAP5430_MASK_COLD_CORE_SHIFT 4
> >+#define OMAP5430_MASK_COLD_CORE_MASK (1<< 4)
> >+#define OMAP5430_MASK_HOT_MM_SHIFT 3
> >+#define OMAP5430_MASK_HOT_MM_MASK (1<< 3)
> >+#define OMAP5430_MASK_COLD_MM_SHIFT 2
> >+#define OMAP5430_MASK_COLD_MM_MASK (1<< 2)
> >+#define OMAP5430_MASK_HOT_MPU_SHIFT 1
> >+#define OMAP5430_MASK_HOT_MPU_MASK (1<< 1)
> >+#define OMAP5430_MASK_COLD_MPU_SHIFT 0
> >+#define OMAP5430_MASK_COLD_MPU_MASK (1<< 0)
> >+
> >+/* BANDGAP_COUNTER */
> >+#define OMAP5430_REPEAT_MODE_SHIFT 31
> >+#define OMAP5430_REPEAT_MODE_MASK (1<< 31)
> >+#define OMAP5430_COUNTER_SHIFT 0
> >+#define OMAP5430_COUNTER_MASK (0xffffff<< 0)
> >+
> >+/* BANDGAP_THRESHOLD */
> >+#define OMAP5430_T_HOT_SHIFT 16
> >+#define OMAP5430_T_HOT_MASK (0x3ff<< 16)
> >+#define OMAP5430_T_COLD_SHIFT 0
> >+#define OMAP5430_T_COLD_MASK (0x3ff<< 0)
> >+
> >+/* TSHUT_THRESHOLD */
> >+#define OMAP5430_TSHUT_HOT_SHIFT 16
> >+#define OMAP5430_TSHUT_HOT_MASK (0x3ff<< 16)
> >+#define OMAP5430_TSHUT_COLD_SHIFT 0
> >+#define OMAP5430_TSHUT_COLD_MASK (0x3ff<< 0)
> >+
> >+/* BANDGAP_STATUS */
> >+#define OMAP5430_BGAP_ALERT_SHIFT 31
> >+#define OMAP5430_BGAP_ALERT_MASK (1<< 31)
> >+#define OMAP5430_HOT_CORE_FLAG_SHIFT 5
> >+#define OMAP5430_HOT_CORE_FLAG_MASK (1<< 5)
> >+#define OMAP5430_COLD_CORE_FLAG_SHIFT 4
> >+#define OMAP5430_COLD_CORE_FLAG_MASK (1<< 4)
> >+#define OMAP5430_HOT_MM_FLAG_SHIFT 3
> >+#define OMAP5430_HOT_MM_FLAG_MASK (1<< 3)
> >+#define OMAP5430_COLD_MM_FLAG_SHIFT 2
> >+#define OMAP5430_COLD_MM_FLAG_MASK (1<< 2)
> >+#define OMAP5430_HOT_MPU_FLAG_SHIFT 1
> >+#define OMAP5430_HOT_MPU_FLAG_MASK (1<< 1)
> >+#define OMAP5430_COLD_MPU_FLAG_SHIFT 0
> >+#define OMAP5430_COLD_MPU_FLAG_MASK (1<< 0)
> >+
> > /* AM35XX only CONTROL_GENERAL register offsets */
> > #define AM35XX_CONTROL_MSUSPENDMUX_6 (OMAP2_CONTROL_GENERAL + 0x0038)
> > #define AM35XX_CONTROL_DEVCONF2 (OMAP2_CONTROL_GENERAL + 0x0310)
>
^ permalink raw reply
* Re: [RFC PATCH 08/11] ARM: OMAP4+: Adding the temperature sensor register set bit fields
From: Eduardo Valentin @ 2012-05-28 11:18 UTC (permalink / raw)
To: Shilimkar, Santosh
Cc: Eduardo Valentin, b-cousson, kishon, kbaidarov, tony, paul, balbi,
amit.kucheria, linux-pm, linux-arm-kernel, linux-omap,
amit.kachhap, Keerthy
In-Reply-To: <CAMQu2gweS9O7+n3_J=7W3CeDOBgMML+z+BXKafqChhEZd3bmDQ@mail.gmail.com>
On Mon, May 28, 2012 at 03:34:28PM +0530, Shilimkar, Santosh wrote:
> On Fri, May 25, 2012 at 1:55 PM, Eduardo Valentin
> <eduardo.valentin@ti.com> wrote:
> > OMAP4460 specific temperature sensor register bit fields are added.
> > Existing OMAP4 entries are renamed to OMAP4430.
> >
> > Signed-off-by: Keerthy <j-keerthy@ti.com>
> > Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> > ---
> Once you split the header, as mentioned in earlier comment,
> you can move those defines in driver file directly.
Agreed, this patch is gonna be dropped and the definition will be part of patch introducing the bandgap driver
>
> Regards
> Santsoh
^ permalink raw reply
* Re: [RFC PATCH 06/11] OMAP2+: use control module mfd driver in omap_type
From: Eduardo Valentin @ 2012-05-28 11:24 UTC (permalink / raw)
To: Shilimkar, Santosh
Cc: amit.kucheria, kbaidarov, kishon, balbi, linux-pm, linux-omap,
linux-arm-kernel
In-Reply-To: <CAMQu2gyjJdJZKAf+TJwQfQ2BgAXe_s-soTcs7Am6O4+yy=7cLQ@mail.gmail.com>
Hello,
On Mon, May 28, 2012 at 03:32:50PM +0530, Shilimkar, Santosh wrote:
> On Fri, May 25, 2012 at 6:23 PM, Cousson, Benoit <b-cousson@ti.com> wrote:
> > Hi Eduardo,
> >
> >
> > On 5/25/2012 10:25 AM, Eduardo Valentin wrote:
> >>
> >> OMAP system control module can be probed early, then
> >> omap_type is safe to use its APIs.
> >>
> >> TODO: add support for other omap versions
> >>
> >> Signed-off-by: Eduardo Valentin<eduardo.valentin@ti.com>
> >> ---
> >> arch/arm/mach-omap2/id.c | 16 +++++++++++++++-
> >> 1 files changed, 15 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/id.c b/arch/arm/mach-omap2/id.c
> >> index 5bb9746..acfd698 100644
> >> --- a/arch/arm/mach-omap2/id.c
> >> +++ b/arch/arm/mach-omap2/id.c
> >> @@ -18,6 +18,7 @@
> >> #include<linux/kernel.h>
> >> #include<linux/init.h>
> >> #include<linux/io.h>
> >> +#include<linux/mfd/omap_control.h>
> >>
> >> #include<asm/cputype.h>
> >>
> >> @@ -40,8 +41,14 @@ EXPORT_SYMBOL(omap_rev);
> >>
> >> int omap_type(void)
> >> {
> >> + struct device *scm;
> >> + int ret = 0;
> >> u32 val = 0;
> >>
> >> + scm = omap_control_get();
> >> + if (IS_ERR_OR_NULL(scm))
> >> + return 0;
> >> +
> >> if (cpu_is_omap24xx()) {
> >
> >
> > OK, not really related to that patch, but the previous cpu_is_omap24xx makes
> > me think of that :-)
> >
> > What about the omap<X>_check_revision used by cpu_is_XXX?
> >
> > This call is the very first one to require the control module access in
> > order to get the ID_CODE inside the control module.
> >
> > So far it still use that ugly hard coded phys -> virtual address macro that
> > is sued for that.
> >
> Agree with Benoits comment. One way to deal with this is,
> store the register offset with init and then just use it here.
>
> That way you can get rid of all cpu_is_XXXX() from this function.
I see. I need to check how this storing would look like.
Probably we can do the storing when the early device gets probed.
>
> Regards
> Santosh
^ permalink raw reply
* Re: [RFC PATCH 05/11] mfd: omap: control: core system control driver
From: Eduardo Valentin @ 2012-05-28 11:35 UTC (permalink / raw)
To: Cousson, Benoit
Cc: amit.kucheria, kbaidarov, kishon, balbi, J Keerthy, linux-pm,
linux-omap, linux-arm-kernel
In-Reply-To: <4FBF8078.9050809@ti.com>
Hello,
On Fri, May 25, 2012 at 02:52:08PM +0200, Cousson Benoit wrote:
> On 5/25/2012 10:25 AM, Eduardo Valentin wrote:
> >This patch introduces a MFD core device driver for
> >OMAP system control module.
> >
> >The control module allows software control of
> >various static modes supported by the device. It is
> >composed of two control submodules: general control
> >module and device (padconfiguration) control
> >module.
> >
> >In this patch, the children defined are for:
> >. USB-phy pin control
> >. Bangap temperature sensor
> >
> >Device driver is probed with postcore_initcall.
> >However, as some of the APIs exposed by this driver
> >may be needed in very early init phase, an early init
> >class is also available: "early_omap_control".
> >
> >Signed-off-by: J Keerthy<j-keerthy@ti.com>
> >Signed-off-by: Kishon Vijay Abraham I<kishon@ti.com>
> >Signed-off-by: Eduardo Valentin<eduardo.valentin@ti.com>
> >---
> > .../devicetree/bindings/mfd/omap_control.txt | 44 ++++
> > arch/arm/mach-omap2/Kconfig | 1 +
> > arch/arm/plat-omap/Kconfig | 3 +
> > drivers/mfd/Kconfig | 9 +
> > drivers/mfd/Makefile | 1 +
> > drivers/mfd/omap-control-core.c | 211 ++++++++++++++++++++
> > include/linux/mfd/omap_control.h | 69 +++++++
> > 7 files changed, 338 insertions(+), 0 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/mfd/omap_control.txt
> > create mode 100644 drivers/mfd/omap-control-core.c
> > create mode 100644 include/linux/mfd/omap_control.h
> >
> >diff --git a/Documentation/devicetree/bindings/mfd/omap_control.txt b/Documentation/devicetree/bindings/mfd/omap_control.txt
> >new file mode 100644
> >index 0000000..46d5e7e
> >--- /dev/null
> >+++ b/Documentation/devicetree/bindings/mfd/omap_control.txt
> >@@ -0,0 +1,44 @@
> >+* Texas Instrument OMAP System Control Module (SCM) bindings
> >+
> >+The control module allows software control of various static modes supported by
> >+the device. The control module controls the settings of various device modules
> >+through register configuration and internal signals. It also controls the pad
> >+configuration, pin functional multiplexing, and the routing of internal signals
> >+(such as PRCM signals or DMA requests) to output pins configured for hardware
> >+observability.
> >+
> >+Required properties:
> >+- compatible : Should be:
> >+ - "ti,omap3-control" for OMAP3 support
> >+ - "ti,omap4-control" for OMAP4 support
> >+ - "ti,omap5-control" for OMAP5 support
> >+
> >+OMAP specific properties:
> >+- ti,hwmods: Name of the hwmod associated to the control module:
> >+ Should be "ctrl_module_core";
> >+
> >+Sub-nodes:
> >+- bandgap : contains the bandgap node
> >+
> >+ The bindings details of individual bandgap device can be found in:
> >+ Documentation/devicetree/bindings/thermal/omap_bandgap.txt
> >+
> >+- usb : contains the usb phy pin control node
> >+
> >+ The only required property for this child is:
> >+ - compatible = "ti,omap4-control-usb";
> >+
> >+Examples:
> >+
> >+ctrl_module_core: ctrl_module_core@4a002000 {
> >+ compatible = "ti,omap4-control";
> >+ ti,hwmods = "ctrl_module_core";
> >+ bandgap {
> >+ compatible = "ti,omap4460-bandgap";
> >+ interrupts =<0 126 4>; /* talert */
> >+ ti,tshut-gpio =<86>; /* tshut */
> >+ };
> >+ usb {
> >+ compatible = "ti,omap4-usb-phy";
> >+ };
> >+};
> >diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> >index a2b946d..7dfe5e1 100644
> >--- a/arch/arm/mach-omap2/Kconfig
> >+++ b/arch/arm/mach-omap2/Kconfig
> >@@ -52,6 +52,7 @@ config ARCH_OMAP4
> > select PL310_ERRATA_727915
> > select ARM_ERRATA_720789
> > select ARCH_HAS_OPP
> >+ select ARCH_HAS_CONTROL_MODULE
> > select PM_OPP if PM
> > select USB_ARCH_HAS_EHCI if USB_SUPPORT
> > select ARM_CPU_SUSPEND if PM
> >diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
> >index ad95c7a..222dbad 100644
> >--- a/arch/arm/plat-omap/Kconfig
> >+++ b/arch/arm/plat-omap/Kconfig
> >@@ -5,6 +5,9 @@ menu "TI OMAP Common Features"
> > config ARCH_OMAP_OTG
> > bool
> >
> >+config ARCH_HAS_CONTROL_MODULE
> >+ bool
> >+
> > choice
> > prompt "OMAP System Type"
> > default ARCH_OMAP2PLUS
> >diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> >index 11e4438..25a66d8 100644
> >--- a/drivers/mfd/Kconfig
> >+++ b/drivers/mfd/Kconfig
> >@@ -795,6 +795,15 @@ config MFD_WL1273_CORE
> > driver connects the radio-wl1273 V4L2 module and the wl1273
> > audio codec.
> >
> >+config MFD_OMAP_CONTROL
> >+ bool "Texas Instruments OMAP System control module"
> >+ depends on ARCH_HAS_CONTROL_MODULE
> >+ help
> >+ This is the core driver for system control module. This driver
> >+ is responsible for creating the control module mfd child,
> >+ like USB-pin control, pin muxing, MMC-pbias and DDR IO dynamic
> >+ change for off mode.
> >+
> > config MFD_OMAP_USB_HOST
> > bool "Support OMAP USBHS core driver"
> > depends on USB_EHCI_HCD_OMAP || USB_OHCI_HCD_OMAP3
> >diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> >index 05fa538..00f99d6 100644
> >--- a/drivers/mfd/Makefile
> >+++ b/drivers/mfd/Makefile
> >@@ -106,6 +106,7 @@ obj-$(CONFIG_MFD_TPS6586X) += tps6586x.o
> > obj-$(CONFIG_MFD_VX855) += vx855.o
> > obj-$(CONFIG_MFD_WL1273_CORE) += wl1273-core.o
> > obj-$(CONFIG_MFD_CS5535) += cs5535-mfd.o
> >+obj-$(CONFIG_MFD_OMAP_CONTROL) += omap-control-core.o
> > obj-$(CONFIG_MFD_OMAP_USB_HOST) += omap-usb-host.o
> > obj-$(CONFIG_MFD_PM8921_CORE) += pm8921-core.o
> > obj-$(CONFIG_MFD_PM8XXX_IRQ) += pm8xxx-irq.o
> >diff --git a/drivers/mfd/omap-control-core.c b/drivers/mfd/omap-control-core.c
> >new file mode 100644
> >index 0000000..7d8d408
> >--- /dev/null
> >+++ b/drivers/mfd/omap-control-core.c
> >@@ -0,0 +1,211 @@
> >+/*
> >+ * OMAP system control module driver file
> >+ *
> >+ * Copyright (C) 2011-2012 Texas Instruments Incorporated - http://www.ti.com/
> >+ * Contacts:
> >+ * Based on original code written by:
> >+ * J Keerthy<j-keerthy@ti.com>
> >+ * Moiz Sonasath<m-sonasath@ti.com>
> >+ * MFD clean up and re-factoring:
> >+ * Eduardo Valentin<eduardo.valentin@ti.com>
> >+ *
> >+ * This program is free software; you can redistribute it and/or
> >+ * modify it under the terms of the GNU General Public License
> >+ * version 2 as published by the Free Software Foundation.
> >+ *
> >+ * This program is distributed in the hope that it will be useful, but
> >+ * WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >+ * General Public License for more details.
> >+ *
> >+ */
> >+
> >+#include<linux/module.h>
> >+#include<linux/export.h>
> >+#include<linux/platform_device.h>
> >+#include<linux/slab.h>
> >+#include<linux/io.h>
> >+#include<linux/err.h>
> >+#include<linux/of_platform.h>
> >+#include<linux/of_address.h>
> >+#include<linux/mfd/core.h>
> >+#include<linux/mfd/omap_control.h>
> >+
> >+static struct omap_control *omap_control_module;
>
> Mmm, we can have up to 4 control module instances in OMAP4.
>
> Well, I'm not sure it worth considering them as separate devices. Is
> that your plan as well?
At least for now I was focusing on the ctrl_module_core ...
>
> But since they all have different base address, it will be trick to
> handle them with only a single entry.
Indeed. We can always add the support latter on.
I am not sure what would be the best way to handle those instances though,
and how they are going to expose APIs. Would need to have an instance bound
to API set?
>
> >+
> >+/**
> >+ * omap_control_readl: Read a single omap control module register.
> >+ *
> >+ * @dev: device to read from.
> >+ * @reg: register to read.
> >+ * @val: output with register value.
> >+ *
> >+ * returns 0 on success or -EINVAL in case struct device is invalid.
> >+ */
> >+int omap_control_readl(struct device *dev, u32 reg, u32 *val)
> >+{
> >+ struct omap_control *omap_control = dev_get_drvdata(dev);
> >+
> >+ if (!omap_control)
> >+ return -EINVAL;
> >+
> >+ *val = readl(omap_control->base + reg);
> >+
> >+ return 0;
> >+}
> >+EXPORT_SYMBOL_GPL(omap_control_readl);
> >+
> >+/**
> >+ * omap_control_writel: Write a single omap control module register.
> >+ *
> >+ * @dev: device to read from.
> >+ * @val: value to write.
> >+ * @reg: register to write to.
> >+ *
> >+ * returns 0 on success or -EINVAL in case struct device is invalid.
> >+ */
> >+int omap_control_writel(struct device *dev, u32 val, u32 reg)
> >+{
> >+ struct omap_control *omap_control = dev_get_drvdata(dev);
> >+ unsigned long flags;
> >+
> >+ if (!omap_control)
> >+ return -EINVAL;
> >+
> >+ spin_lock_irqsave(&omap_control->reg_lock, flags);
> >+ writel(val, omap_control->base + reg);
> >+ spin_unlock_irqrestore(&omap_control->reg_lock, flags);
> >+
> >+ return 0;
> >+}
> >+EXPORT_SYMBOL_GPL(omap_control_writel);
> >+
> >+/**
> >+ * omap_control_get: returns the control module device pinter
>
> typo
K
>
> >+ *
> >+ * The modules which has to use control module API's to read or write should
> >+ * call this API to get the control module device pointer.
> >+ */
> >+struct device *omap_control_get(void)
> >+{
> >+ unsigned long flags;
> >+
> >+ if (!omap_control_module)
> >+ return ERR_PTR(-ENODEV);
> >+
> >+ spin_lock_irqsave(&omap_control_module->reg_lock, flags);
> >+ omap_control_module->use_count++;
> >+ spin_unlock_irqrestore(&omap_control_module->reg_lock, flags);
>
> Don't we do have some better way to increment atomically a variable
> in Linux.
Yeah we have, atomic API. In general I think the SCM/bangap/phy APIs
need to be revisited WRT locking, in general.
>
> >+
> >+ return omap_control_module->dev;
> >+}
> >+EXPORT_SYMBOL_GPL(omap_control_get);
> >+
> >+/**
> >+ * omap_control_put: returns the control module device pinter
> >+ *
> >+ * The modules which has to use control module API's to read or write should
> >+ * call this API to get the control module device pointer.
> >+ */
> >+void omap_control_put(struct device *dev)
> >+{
> >+ struct omap_control *omap_control = dev_get_drvdata(dev);
> >+ unsigned long flags;
> >+
> >+ if (!omap_control)
> >+ return;
> >+
> >+ spin_lock_irqsave(&omap_control->reg_lock, flags);
> >+ omap_control->use_count--;
> >+ spin_unlock_irqrestore(&omap_control_module->reg_lock, flags);
> >+}
> >+EXPORT_SYMBOL_GPL(omap_control_put);
> >+
> >+static const struct of_device_id of_omap_control_match[] = {
> >+ { .compatible = "ti,omap3-control", },
> >+ { .compatible = "ti,omap4-control", },
> >+ { .compatible = "ti,omap5-control", },
> >+ { },
> >+};
> >+
> >+static int __devinit omap_control_probe(struct platform_device *pdev)
> >+{
> >+ struct resource *res;
> >+ void __iomem *base;
> >+ struct device *dev =&pdev->dev;
> >+ struct device_node *np = dev->of_node;
> >+ struct omap_control *omap_control;
>
> Maybe omap_control_data instead? At least if this is drvdata only.
> If this is supposed to be the *handle* to the control module
> instance, it should be fine.
That's suppose to be the phandle :-)
>
> >+
> >+ omap_control = devm_kzalloc(dev, sizeof(*omap_control), GFP_KERNEL);
> >+ if (!omap_control) {
> >+ dev_err(dev, "not enough memory for omap_control\n");
> >+ return -ENOMEM;
> >+ }
> >+
> >+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >+ if (!res) {
> >+ dev_err(dev, "missing memory base resource\n");
> >+ return -EINVAL;
> >+ }
> >+
> >+ base = devm_request_and_ioremap(dev, res);
> >+ if (!base) {
> >+ dev_err(dev, "ioremap failed\n");
> >+ return -EADDRNOTAVAIL;
> >+ }
> >+
> >+ omap_control->base = base;
> >+ omap_control->dev = dev;
> >+ spin_lock_init(&omap_control->reg_lock);
> >+
> >+ platform_set_drvdata(pdev, omap_control);
> >+ omap_control_module = omap_control;
> >+
> >+ return of_platform_populate(np, of_omap_control_match, NULL, dev);
> >+}
> >+
> >+static int __devexit omap_control_remove(struct platform_device *pdev)
> >+{
> >+ struct omap_control *omap_control = platform_get_drvdata(pdev);
> >+
> >+ spin_lock(&omap_control->reg_lock);
> >+ if (omap_control->use_count> 0) {
> >+ spin_unlock(&omap_control->reg_lock);
> >+ dev_err(&pdev->dev, "device removed while still being used\n");
> >+ return -EBUSY;
> >+ }
> >+ spin_unlock(&omap_control->reg_lock);
> >+
> >+ iounmap(omap_control->base);
> >+ platform_set_drvdata(pdev, NULL);
> >+
> >+ return 0;
> >+}
> >+
> >+static struct platform_driver omap_control_driver = {
> >+ .probe = omap_control_probe,
> >+ .remove = __devexit_p(omap_control_remove),
> >+ .driver = {
> >+ .name = "omap-control-core",
> >+ .owner = THIS_MODULE,
> >+ .of_match_table = of_omap_control_match,
> >+ },
> >+};
> >+
> >+static int __init omap_control_init(void)
> >+{
> >+ return platform_driver_register(&omap_control_driver);
> >+}
> >+postcore_initcall_sync(omap_control_init);
> >+
> >+static void __exit omap_control_exit(void)
> >+{
> >+ platform_driver_unregister(&omap_control_driver);
> >+}
> >+module_exit(omap_control_exit);
> >+early_platform_init("early_omap_control",&omap_control_driver);
> >+
> >+MODULE_DESCRIPTION("OMAP system control module driver");
> >+MODULE_LICENSE("GPL v2");
> >+MODULE_ALIAS("platform:omap-control-core");
> >+MODULE_AUTHOR("Texas Instruments Inc.");
> >diff --git a/include/linux/mfd/omap_control.h b/include/linux/mfd/omap_control.h
> >new file mode 100644
> >index 0000000..7a33eda
> >--- /dev/null
> >+++ b/include/linux/mfd/omap_control.h
> >@@ -0,0 +1,69 @@
> >+/*
> >+ * OMAP system control module header file
> >+ *
> >+ * Copyright (C) 2011-2012 Texas Instruments Incorporated - http://www.ti.com/
> >+ * Contact:
> >+ * J Keerthy<j-keerthy@ti.com>
> >+ * Moiz Sonasath<m-sonasath@ti.com>
> >+ * Abraham, Kishon Vijay<kishon@ti.com>
> >+ * Eduardo Valentin<eduardo.valentin@ti.com>
> >+ *
> >+ * This program is free software; you can redistribute it and/or
> >+ * modify it under the terms of the GNU General Public License
> >+ * version 2 as published by the Free Software Foundation.
> >+ *
> >+ * This program is distributed in the hope that it will be useful, but
> >+ * WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> >+ * General Public License for more details.
> >+ *
> >+ */
> >+
> >+#ifndef __DRIVERS_OMAP_CONTROL_H
> >+#define __DRIVERS_OMAP_CONTROL_H
> >+
> >+#include<linux/err.h>
> >+
> >+/**
> >+ * struct system control module - scm device structure
> >+ * @dev: device pointer
> >+ * @base: Base of the temp I/O
> >+ * @reg_lock: protect omap_control structure
> >+ * @use_count: track API users
> >+ */
> >+struct omap_control {
> >+ struct device *dev;
>
> Do you really need the dev?
> You API is device based and not omap_control based, so it should not
> be needed.
>
> I guess we should be consistent here. We can store the devices and
> used a device based API or store the omap_control and thus expose a
> omap_control API.
Yeah, this API structure is left over of the previous driver.
The omap_control_get returns the SCM device reference
and the users of SCM use it as parameter for the SCM APIs.
We need to have in mind that, for SCM, the users are:
a. Its children (USB phy, BG, etc)
b. Non children users (mach code)
The refcounting and the locking needs to take care of both I'd say.
The struct dev was just a way to pass the SCM phandle.
>
> Regards,
> Benoit
^ permalink raw reply
* Re: [RFC PATCH 05/11] mfd: omap: control: core system control driver
From: Eduardo Valentin @ 2012-05-28 11:42 UTC (permalink / raw)
To: Shilimkar, Santosh
Cc: amit.kucheria, kbaidarov, kishon, balbi, J Keerthy, linux-pm,
linux-omap, linux-arm-kernel
In-Reply-To: <CAMQu2gy_rgLidHWy7xxZBiVD4wfFuTvA+wC7joogrcFptwsx8w@mail.gmail.com>
Hello,
On Mon, May 28, 2012 at 03:24:01PM +0530, Shilimkar, Santosh wrote:
> On Fri, May 25, 2012 at 1:55 PM, Eduardo Valentin
> <eduardo.valentin@ti.com> wrote:
> > This patch introduces a MFD core device driver for
> > OMAP system control module.
> >
> > The control module allows software control of
> > various static modes supported by the device. It is
> > composed of two control submodules: general control
> > module and device (padconfiguration) control
> > module.
> >
> > In this patch, the children defined are for:
> > . USB-phy pin control
> > . Bangap temperature sensor
> >
> > Device driver is probed with postcore_initcall.
> > However, as some of the APIs exposed by this driver
> > may be needed in very early init phase, an early init
> > class is also available: "early_omap_control".
> >
> > Signed-off-by: J Keerthy <j-keerthy@ti.com>
> > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> > Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
> > ---
>
> [..]
> > diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
> > index ad95c7a..222dbad 100644
> > --- a/arch/arm/plat-omap/Kconfig
> > +++ b/arch/arm/plat-omap/Kconfig
> > @@ -5,6 +5,9 @@ menu "TI OMAP Common Features"
> > config ARCH_OMAP_OTG
> > bool
> >
> > +config ARCH_HAS_CONTROL_MODULE
> > + bool
> > +
> Thanks for getting rid of OMAP CONFIG here.
OK. ARCH_HAS_CONTROL_MODULE is a bit too generic though..
>
> > choice
> > prompt "OMAP System Type"
> > default ARCH_OMAP2PLUS
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 11e4438..25a66d8 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -795,6 +795,15 @@ config MFD_WL1273_CORE
> > driver connects the radio-wl1273 V4L2 module and the wl1273
> > audio codec.
> >
> > +config MFD_OMAP_CONTROL
> > + bool "Texas Instruments OMAP System control module"
> > + depends on ARCH_HAS_CONTROL_MODULE
> > + help
> > + This is the core driver for system control module. This driver
> > + is responsible for creating the control module mfd child,
> > + like USB-pin control, pin muxing, MMC-pbias and DDR IO dynamic
> > + change for off mode.
> > +
> > config MFD_OMAP_USB_HOST
> > bool "Support OMAP USBHS core driver"
> > depends on USB_EHCI_HCD_OMAP || USB_OHCI_HCD_OMAP3
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index 05fa538..00f99d6 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -106,6 +106,7 @@ obj-$(CONFIG_MFD_TPS6586X) += tps6586x.o
> > obj-$(CONFIG_MFD_VX855) += vx855.o
> > obj-$(CONFIG_MFD_WL1273_CORE) += wl1273-core.o
> > obj-$(CONFIG_MFD_CS5535) += cs5535-mfd.o
> > +obj-$(CONFIG_MFD_OMAP_CONTROL) += omap-control-core.o
> > obj-$(CONFIG_MFD_OMAP_USB_HOST) += omap-usb-host.o
> > obj-$(CONFIG_MFD_PM8921_CORE) += pm8921-core.o
> > obj-$(CONFIG_MFD_PM8XXX_IRQ) += pm8xxx-irq.o
> > diff --git a/drivers/mfd/omap-control-core.c b/drivers/mfd/omap-control-core.c
> > new file mode 100644
> > index 0000000..7d8d408
> > --- /dev/null
> > +++ b/drivers/mfd/omap-control-core.c
> > @@ -0,0 +1,211 @@
> > +/*
> > + * OMAP system control module driver file
> > + *
> > + * Copyright (C) 2011-2012 Texas Instruments Incorporated - http://www.ti.com/
> > + * Contacts:
> > + * Based on original code written by:
> > + * J Keerthy <j-keerthy@ti.com>
> > + * Moiz Sonasath <m-sonasath@ti.com>
> > + * MFD clean up and re-factoring:
> > + * Eduardo Valentin <eduardo.valentin@ti.com>
> > + *
> > + * This program is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU General Public License
> > + * version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * General Public License for more details.
> > + *
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/export.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/slab.h>
> > +#include <linux/io.h>
> > +#include <linux/err.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/of_address.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/mfd/omap_control.h>
> > +
> > +static struct omap_control *omap_control_module;
> > +
> > +/**
> > + * omap_control_readl: Read a single omap control module register.
> > + *
> > + * @dev: device to read from.
> > + * @reg: register to read.
> > + * @val: output with register value.
> > + *
> > + * returns 0 on success or -EINVAL in case struct device is invalid.
> > + */
> > +int omap_control_readl(struct device *dev, u32 reg, u32 *val)
> > +{
> > + struct omap_control *omap_control = dev_get_drvdata(dev);
> > +
> > + if (!omap_control)
> > + return -EINVAL;
> > +
> > + *val = readl(omap_control->base + reg);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(omap_control_readl);
> > +
> I might have missed in the last scan, but can you let
> function return the register value.
Why?
>
> I am guessing, you did this for error case handling. You might
> want to stick to read API semantic and just have WARN_ON()
> to take care of error case.
Yeah, that was for error handling and to do not confuse register
values with error values.
>
> > +/**
> > + * omap_control_writel: Write a single omap control module register.
> > + *
> > + * @dev: device to read from.
> > + * @val: value to write.
> > + * @reg: register to write to.
> > + *
> > + * returns 0 on success or -EINVAL in case struct device is invalid.
> > + */
> > +int omap_control_writel(struct device *dev, u32 val, u32 reg)
> > +{
> > + struct omap_control *omap_control = dev_get_drvdata(dev);
> > + unsigned long flags;
> > +
> > + if (!omap_control)
> > + return -EINVAL;
> > +
> > + spin_lock_irqsave(&omap_control->reg_lock, flags);
> > + writel(val, omap_control->base + reg);
> > + spin_unlock_irqrestore(&omap_control->reg_lock, flags);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(omap_control_writel);
> > +
> > +/**
> > + * omap_control_get: returns the control module device pinter
> > + *
> > + * The modules which has to use control module API's to read or write should
> > + * call this API to get the control module device pointer.
> > + */
> > +struct device *omap_control_get(void)
> > +{
> > + unsigned long flags;
> > +
> > + if (!omap_control_module)
> > + return ERR_PTR(-ENODEV);
> > +
> > + spin_lock_irqsave(&omap_control_module->reg_lock, flags);
> > + omap_control_module->use_count++;
> > + spin_unlock_irqrestore(&omap_control_module->reg_lock, flags);
> > +
> > + return omap_control_module->dev;
> > +}
> > +EXPORT_SYMBOL_GPL(omap_control_get);
> > +
> > +/**
> > + * omap_control_put: returns the control module device pinter
> > + *
> > + * The modules which has to use control module API's to read or write should
> > + * call this API to get the control module device pointer.
> > + */
> > +void omap_control_put(struct device *dev)
> > +{
> > + struct omap_control *omap_control = dev_get_drvdata(dev);
> > + unsigned long flags;
> > +
> > + if (!omap_control)
> > + return;
> > +
> > + spin_lock_irqsave(&omap_control->reg_lock, flags);
> > + omap_control->use_count--;
> > + spin_unlock_irqrestore(&omap_control_module->reg_lock, flags);
> > +}
> > +EXPORT_SYMBOL_GPL(omap_control_put);
> > +
> > +static const struct of_device_id of_omap_control_match[] = {
> > + { .compatible = "ti,omap3-control", },
> > + { .compatible = "ti,omap4-control", },
> > + { .compatible = "ti,omap5-control", },
> > + { },
> > +};
> > +
> > +static int __devinit omap_control_probe(struct platform_device *pdev)
> > +{
> > + struct resource *res;
> > + void __iomem *base;
> > + struct device *dev = &pdev->dev;
> > + struct device_node *np = dev->of_node;
> > + struct omap_control *omap_control;
> > +
> > + omap_control = devm_kzalloc(dev, sizeof(*omap_control), GFP_KERNEL);
> > + if (!omap_control) {
> > + dev_err(dev, "not enough memory for omap_control\n");
> > + return -ENOMEM;
> > + }
> > +
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + if (!res) {
> > + dev_err(dev, "missing memory base resource\n");
> > + return -EINVAL;
> > + }
> > +
> > + base = devm_request_and_ioremap(dev, res);
> > + if (!base) {
> > + dev_err(dev, "ioremap failed\n");
> > + return -EADDRNOTAVAIL;
> > + }
> > +
> > + omap_control->base = base;
> > + omap_control->dev = dev;
> > + spin_lock_init(&omap_control->reg_lock);
> > +
> > + platform_set_drvdata(pdev, omap_control);
> > + omap_control_module = omap_control;
> > +
> > + return of_platform_populate(np, of_omap_control_match, NULL, dev);
> > +}
> > +
>
> Will the probe get called on multiple devices and race ?
It depends. If we decide to have an early device for scm, then the probe will
be called more than once. If not, then only once.
>
> > +static int __devexit omap_control_remove(struct platform_device *pdev)
> > +{
> > + struct omap_control *omap_control = platform_get_drvdata(pdev);
> > +
> > + spin_lock(&omap_control->reg_lock);
> > + if (omap_control->use_count > 0) {
> > + spin_unlock(&omap_control->reg_lock);
> > + dev_err(&pdev->dev, "device removed while still being used\n");
> > + return -EBUSY;
> > + }
> > + spin_unlock(&omap_control->reg_lock);
> > +
> Do you really need above lock where you are just doing the
> register read. smp_rmb(), should be enough, I guess.
It is locking the use counter not a register..
>
> Regards
> Santosh
^ permalink raw reply
* Re: [RFC PATCH 05/11] mfd: omap: control: core system control driver
From: Shilimkar, Santosh @ 2012-05-28 13:15 UTC (permalink / raw)
To: eduardo.valentin
Cc: amit.kucheria, balbi, kishon, kbaidarov, J Keerthy, linux-pm,
linux-omap, linux-arm-kernel
In-Reply-To: <20120528114225.GH3923@besouro>
On Mon, May 28, 2012 at 5:12 PM, Eduardo Valentin
<eduardo.valentin@ti.com> wrote:
> Hello,
>
> On Mon, May 28, 2012 at 03:24:01PM +0530, Shilimkar, Santosh wrote:
>> On Fri, May 25, 2012 at 1:55 PM, Eduardo Valentin
>> <eduardo.valentin@ti.com> wrote:
>> > This patch introduces a MFD core device driver for
>> > OMAP system control module.
>> >
>> > The control module allows software control of
>> > various static modes supported by the device. It is
>> > composed of two control submodules: general control
>> > module and device (padconfiguration) control
>> > module.
>> >
>> > In this patch, the children defined are for:
>> > . USB-phy pin control
>> > . Bangap temperature sensor
>> >
>> > Device driver is probed with postcore_initcall.
>> > However, as some of the APIs exposed by this driver
>> > may be needed in very early init phase, an early init
>> > class is also available: "early_omap_control".
>> >
>> > Signed-off-by: J Keerthy <j-keerthy@ti.com>
>> > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
>> > Signed-off-by: Eduardo Valentin <eduardo.valentin@ti.com>
>> > ---
>>
>> [..]
>> > diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
>> > index ad95c7a..222dbad 100644
>> > --- a/arch/arm/plat-omap/Kconfig
>> > +++ b/arch/arm/plat-omap/Kconfig
>> > @@ -5,6 +5,9 @@ menu "TI OMAP Common Features"
>> > config ARCH_OMAP_OTG
>> > bool
>> >
>> > +config ARCH_HAS_CONTROL_MODULE
>> > + bool
>> > +
>> Thanks for getting rid of OMAP CONFIG here.
>
> OK. ARCH_HAS_CONTROL_MODULE is a bit too generic though..
>
>>
>> > choice
>> > prompt "OMAP System Type"
>> > default ARCH_OMAP2PLUS
>> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
>> > index 11e4438..25a66d8 100644
>> > --- a/drivers/mfd/Kconfig
>> > +++ b/drivers/mfd/Kconfig
>> > @@ -795,6 +795,15 @@ config MFD_WL1273_CORE
>> > driver connects the radio-wl1273 V4L2 module and the wl1273
>> > audio codec.
>> >
>> > +config MFD_OMAP_CONTROL
>> > + bool "Texas Instruments OMAP System control module"
>> > + depends on ARCH_HAS_CONTROL_MODULE
>> > + help
>> > + This is the core driver for system control module. This driver
>> > + is responsible for creating the control module mfd child,
>> > + like USB-pin control, pin muxing, MMC-pbias and DDR IO dynamic
>> > + change for off mode.
>> > +
>> > config MFD_OMAP_USB_HOST
>> > bool "Support OMAP USBHS core driver"
>> > depends on USB_EHCI_HCD_OMAP || USB_OHCI_HCD_OMAP3
>> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
>> > index 05fa538..00f99d6 100644
>> > --- a/drivers/mfd/Makefile
>> > +++ b/drivers/mfd/Makefile
>> > @@ -106,6 +106,7 @@ obj-$(CONFIG_MFD_TPS6586X) += tps6586x.o
>> > obj-$(CONFIG_MFD_VX855) += vx855.o
>> > obj-$(CONFIG_MFD_WL1273_CORE) += wl1273-core.o
>> > obj-$(CONFIG_MFD_CS5535) += cs5535-mfd.o
>> > +obj-$(CONFIG_MFD_OMAP_CONTROL) += omap-control-core.o
>> > obj-$(CONFIG_MFD_OMAP_USB_HOST) += omap-usb-host.o
>> > obj-$(CONFIG_MFD_PM8921_CORE) += pm8921-core.o
>> > obj-$(CONFIG_MFD_PM8XXX_IRQ) += pm8xxx-irq.o
>> > diff --git a/drivers/mfd/omap-control-core.c b/drivers/mfd/omap-control-core.c
>> > new file mode 100644
>> > index 0000000..7d8d408
>> > --- /dev/null
>> > +++ b/drivers/mfd/omap-control-core.c
>> > @@ -0,0 +1,211 @@
>> > +/*
>> > + * OMAP system control module driver file
>> > + *
>> > + * Copyright (C) 2011-2012 Texas Instruments Incorporated - http://www.ti.com/
>> > + * Contacts:
>> > + * Based on original code written by:
>> > + * J Keerthy <j-keerthy@ti.com>
>> > + * Moiz Sonasath <m-sonasath@ti.com>
>> > + * MFD clean up and re-factoring:
>> > + * Eduardo Valentin <eduardo.valentin@ti.com>
>> > + *
>> > + * This program is free software; you can redistribute it and/or
>> > + * modify it under the terms of the GNU General Public License
>> > + * version 2 as published by the Free Software Foundation.
>> > + *
>> > + * This program is distributed in the hope that it will be useful, but
>> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
>> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> > + * General Public License for more details.
>> > + *
>> > + */
>> > +
>> > +#include <linux/module.h>
>> > +#include <linux/export.h>
>> > +#include <linux/platform_device.h>
>> > +#include <linux/slab.h>
>> > +#include <linux/io.h>
>> > +#include <linux/err.h>
>> > +#include <linux/of_platform.h>
>> > +#include <linux/of_address.h>
>> > +#include <linux/mfd/core.h>
>> > +#include <linux/mfd/omap_control.h>
>> > +
>> > +static struct omap_control *omap_control_module;
>> > +
>> > +/**
>> > + * omap_control_readl: Read a single omap control module register.
>> > + *
>> > + * @dev: device to read from.
>> > + * @reg: register to read.
>> > + * @val: output with register value.
>> > + *
>> > + * returns 0 on success or -EINVAL in case struct device is invalid.
>> > + */
>> > +int omap_control_readl(struct device *dev, u32 reg, u32 *val)
>> > +{
>> > + struct omap_control *omap_control = dev_get_drvdata(dev);
>> > +
>> > + if (!omap_control)
>> > + return -EINVAL;
>> > +
>> > + *val = readl(omap_control->base + reg);
>> > +
>> > + return 0;
>> > +}
>> > +EXPORT_SYMBOL_GPL(omap_control_readl);
>> > +
>> I might have missed in the last scan, but can you let
>> function return the register value.
>
> Why?
>
Because thats how typical read 1 value kind of functions
look like..
>>
>> I am guessing, you did this for error case handling. You might
>> want to stick to read API semantic and just have WARN_ON()
>> to take care of error case.
>
> Yeah, that was for error handling and to do not confuse register
> values with error values.
>
No strong opinion if you like it that way but it just appeared odd to
me from a caller perspective.
>>
>> > +/**
>> > + * omap_control_writel: Write a single omap control module register.
>> > + *
>> > + * @dev: device to read from.
>> > + * @val: value to write.
>> > + * @reg: register to write to.
>> > + *
>> > + * returns 0 on success or -EINVAL in case struct device is invalid.
>> > + */
>> > +int omap_control_writel(struct device *dev, u32 val, u32 reg)
>> > +{
>> > + struct omap_control *omap_control = dev_get_drvdata(dev);
>> > + unsigned long flags;
>> > +
>> > + if (!omap_control)
>> > + return -EINVAL;
>> > +
>> > + spin_lock_irqsave(&omap_control->reg_lock, flags);
>> > + writel(val, omap_control->base + reg);
>> > + spin_unlock_irqrestore(&omap_control->reg_lock, flags);
>> > +
>> > + return 0;
>> > +}
>> > +EXPORT_SYMBOL_GPL(omap_control_writel);
>> > +
>> > +/**
>> > + * omap_control_get: returns the control module device pinter
>> > + *
>> > + * The modules which has to use control module API's to read or write should
>> > + * call this API to get the control module device pointer.
>> > + */
>> > +struct device *omap_control_get(void)
>> > +{
>> > + unsigned long flags;
>> > +
>> > + if (!omap_control_module)
>> > + return ERR_PTR(-ENODEV);
>> > +
>> > + spin_lock_irqsave(&omap_control_module->reg_lock, flags);
>> > + omap_control_module->use_count++;
>> > + spin_unlock_irqrestore(&omap_control_module->reg_lock, flags);
>> > +
>> > + return omap_control_module->dev;
>> > +}
>> > +EXPORT_SYMBOL_GPL(omap_control_get);
>> > +
>> > +/**
>> > + * omap_control_put: returns the control module device pinter
>> > + *
>> > + * The modules which has to use control module API's to read or write should
>> > + * call this API to get the control module device pointer.
>> > + */
>> > +void omap_control_put(struct device *dev)
>> > +{
>> > + struct omap_control *omap_control = dev_get_drvdata(dev);
>> > + unsigned long flags;
>> > +
>> > + if (!omap_control)
>> > + return;
>> > +
>> > + spin_lock_irqsave(&omap_control->reg_lock, flags);
>> > + omap_control->use_count--;
>> > + spin_unlock_irqrestore(&omap_control_module->reg_lock, flags);
>> > +}
>> > +EXPORT_SYMBOL_GPL(omap_control_put);
>> > +
>> > +static const struct of_device_id of_omap_control_match[] = {
>> > + { .compatible = "ti,omap3-control", },
>> > + { .compatible = "ti,omap4-control", },
>> > + { .compatible = "ti,omap5-control", },
>> > + { },
>> > +};
>> > +
>> > +static int __devinit omap_control_probe(struct platform_device *pdev)
>> > +{
>> > + struct resource *res;
>> > + void __iomem *base;
>> > + struct device *dev = &pdev->dev;
>> > + struct device_node *np = dev->of_node;
>> > + struct omap_control *omap_control;
>> > +
>> > + omap_control = devm_kzalloc(dev, sizeof(*omap_control), GFP_KERNEL);
>> > + if (!omap_control) {
>> > + dev_err(dev, "not enough memory for omap_control\n");
>> > + return -ENOMEM;
>> > + }
>> > +
>> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > + if (!res) {
>> > + dev_err(dev, "missing memory base resource\n");
>> > + return -EINVAL;
>> > + }
>> > +
>> > + base = devm_request_and_ioremap(dev, res);
>> > + if (!base) {
>> > + dev_err(dev, "ioremap failed\n");
>> > + return -EADDRNOTAVAIL;
>> > + }
>> > +
>> > + omap_control->base = base;
>> > + omap_control->dev = dev;
>> > + spin_lock_init(&omap_control->reg_lock);
>> > +
>> > + platform_set_drvdata(pdev, omap_control);
>> > + omap_control_module = omap_control;
>> > +
>> > + return of_platform_populate(np, of_omap_control_match, NULL, dev);
>> > +}
>> > +
>>
>> Will the probe get called on multiple devices and race ?
>
> It depends. If we decide to have an early device for scm, then the probe will
> be called more than once. If not, then only once.
>
OK. But in either case it won't race so that's fine.
>>
>> > +static int __devexit omap_control_remove(struct platform_device *pdev)
>> > +{
>> > + struct omap_control *omap_control = platform_get_drvdata(pdev);
>> > +
>> > + spin_lock(&omap_control->reg_lock);
>> > + if (omap_control->use_count > 0) {
>> > + spin_unlock(&omap_control->reg_lock);
>> > + dev_err(&pdev->dev, "device removed while still being used\n");
>> > + return -EBUSY;
>> > + }
>> > + spin_unlock(&omap_control->reg_lock);
>> > +
>> Do you really need above lock where you are just doing the
>> register read. smp_rmb(), should be enough, I guess.
>
> It is locking the use counter not a register..
>
Yes I know. But is just read and you can do away and
hence you can avoid the lock.
Regards
Santosh
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox