Devicetree
 help / color / mirror / Atom feed
* Re: [PATCHv1 0/2] Convert rx51-battery to IIO API and add DT support
From: Sebastian Reichel @ 2014-02-26 17:51 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Dmitry Eremin-Solenikov, David Woodhouse, Jonathan Cameron,
	Marek Belisko, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Grant Likely, LKML,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Ivajlo Dimitrov
In-Reply-To: <CAHYPw2EeEnDn2FUYSMWbywprmhJmcRRyaTPn1YZK0xPvuiJQFw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2803 bytes --]

Hi Pali,

On Wed, Feb 26, 2014 at 08:40:54AM +0100, Pali Rohár wrote:
> I would like to ask other kernel developers what do you think about
> moving ADC channel numbers from rx51_battery.ko driver code to DT.
> Driver rx51_battery.ko is platform specific for Nokia RX-51 (N900) so
> it is usefull only for this one device.
> 
> Before this patch all driver data (look-up tables, adc channel
> numbers, etc...) were in driver code. Now after this patch adc channel
> numbers were moved to DT. What do you think? It is better to have all
> data in one place (driver code) or some in DT and some in driver code?
> 
> For me it does not make sense to move these numbers to DT, because
> driver is rx51 device specific and chaning it in DT does not make
> sense. And I think it is better to have add driver data in one place
> and not in two...
> 
> Sebastian already wrote me that this is normal to have numbers in DT
> and other code in driver. But I think that driver which can be used
> only in one device (so specified only in one DT file) does not need to
> have configuration (via DT or board files).
>
> Or do you think that driver specified only for one device needs to
> have ADC numbers configuration via DT?

I think the problem is, that you think of ADC channel numbers as
configuration data. This means you think of rx51-battery as an
alternative driver for the twl4030-madc.

I think of rx51-battery as its own platform device, which makes use
of the ADC similar to a button making use of a GPIO chip.
For me the ADC channel numbers are not configuration data, but an
inter-device resource reference, like e.g. GPIO references.
This is exactly the data you would put into the device tree.

Now let's take the rx51-audio device as another example for an n900
specific device (not yet in mainline kernel). It does not need ADC
channels, but GPIO lines:

	sound: n900-audio {
		/* ... some more references ... */
		nokia,tvout-selection-gpio = <&gpio2 8 GPIO_ACTIVE_HIGH>; /* 40 */
		nokia,jack-detection-gpio = <&gpio6 17 GPIO_ACTIVE_HIGH>; /* 177 */
		nokia,eci-switch-gpio = <&gpio6 22 GPIO_ACTIVE_HIGH>; /* 182 */
		nokia,speaker-amplifier-gpio = <&twl_gpio 7 GPIO_ACTIVE_HIGH>;
	};

Since GPIO numbers are not guaranteed to be consistent in DT boot
mode at least the GPIO chip must be referenced. Do you really think
it's a good idea to leave out the exact GPIO number just because
the driver knows it needs the 8th GPIO from the second GPIO chip?
IMHO this is really ugly, since it splits the information, which
GPIO is used into two parts - one living in the DT and one living
in the driver with no advantage at all. So it does make sense to
specify inter-device resources via DT even for platform specific
devices.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [PATCHv2 14/16] ARM: OMAP3: hwmod data: cleanup data for IOMMUs
From: Suman Anna @ 2014-02-26 17:59 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Laurent Pinchart, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	Florian Vaussard
In-Reply-To: <20140226171824.GH11654-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

Tony,

On 02/26/2014 11:18 AM, Tony Lindgren wrote:
> * Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [140213 10:19]:
>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>
>> The irq numbers, ocp address space and device attribute data
>> have all been cleaned up for OMAP3 IOMMUs. All this data is
>> populated via the corresponding dt node.
>>
>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>
> This will need to wait until we've made omap3 to be DT only
> as this will break idling of things for the legacy booting.
>

OK, will drop this and I will adjust Patch 9 to support the legacy boot 
for OMAP3 ISP.

regards
Suman

>
>> ---
>>   arch/arm/mach-omap2/omap_hwmod_3xxx_data.c | 46 ------------------------------
>>   1 file changed, 46 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> index 9c7e23a..d68c131 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
>> @@ -24,7 +24,6 @@
>>   #include "l4_3xxx.h"
>>   #include <linux/platform_data/asoc-ti-mcbsp.h>
>>   #include <linux/platform_data/spi-omap2-mcspi.h>
>> -#include <linux/platform_data/iommu-omap.h>
>>   #include <linux/platform_data/mailbox-omap.h>
>>   #include <plat/dmtimer.h>
>>
>> @@ -2991,83 +2990,39 @@ static struct omap_hwmod_class omap3xxx_mmu_hwmod_class = {
>>
>>   /* mmu isp */
>>
>> -static struct omap_mmu_dev_attr mmu_isp_dev_attr = {
>> -	.da_start	= 0x0,
>> -	.da_end		= 0xfffff000,
>> -	.nr_tlb_entries = 8,
>> -};
>> -
>>   static struct omap_hwmod omap3xxx_mmu_isp_hwmod;
>> -static struct omap_hwmod_irq_info omap3xxx_mmu_isp_irqs[] = {
>> -	{ .irq = 24 + OMAP_INTC_START, },
>> -	{ .irq = -1 }
>> -};
>> -
>> -static struct omap_hwmod_addr_space omap3xxx_mmu_isp_addrs[] = {
>> -	{
>> -		.pa_start	= 0x480bd400,
>> -		.pa_end		= 0x480bd47f,
>> -		.flags		= ADDR_TYPE_RT,
>> -	},
>> -	{ }
>> -};
>>
>>   /* l4_core -> mmu isp */
>>   static struct omap_hwmod_ocp_if omap3xxx_l4_core__mmu_isp = {
>>   	.master		= &omap3xxx_l4_core_hwmod,
>>   	.slave		= &omap3xxx_mmu_isp_hwmod,
>> -	.addr		= omap3xxx_mmu_isp_addrs,
>>   	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>>   };
>>
>>   static struct omap_hwmod omap3xxx_mmu_isp_hwmod = {
>>   	.name		= "mmu_isp",
>>   	.class		= &omap3xxx_mmu_hwmod_class,
>> -	.mpu_irqs	= omap3xxx_mmu_isp_irqs,
>>   	.main_clk	= "cam_ick",
>> -	.dev_attr	= &mmu_isp_dev_attr,
>>   	.flags		= HWMOD_NO_IDLEST,
>>   };
>>
>>   /* mmu iva */
>>
>> -static struct omap_mmu_dev_attr mmu_iva_dev_attr = {
>> -	.da_start	= 0x11000000,
>> -	.da_end		= 0xfffff000,
>> -	.nr_tlb_entries = 32,
>> -};
>> -
>>   static struct omap_hwmod omap3xxx_mmu_iva_hwmod;
>> -static struct omap_hwmod_irq_info omap3xxx_mmu_iva_irqs[] = {
>> -	{ .irq = 28 + OMAP_INTC_START, },
>> -	{ .irq = -1 }
>> -};
>> -
>>   static struct omap_hwmod_rst_info omap3xxx_mmu_iva_resets[] = {
>>   	{ .name = "mmu", .rst_shift = 1, .st_shift = 9 },
>>   };
>>
>> -static struct omap_hwmod_addr_space omap3xxx_mmu_iva_addrs[] = {
>> -	{
>> -		.pa_start	= 0x5d000000,
>> -		.pa_end		= 0x5d00007f,
>> -		.flags		= ADDR_TYPE_RT,
>> -	},
>> -	{ }
>> -};
>> -
>>   /* l3_main -> iva mmu */
>>   static struct omap_hwmod_ocp_if omap3xxx_l3_main__mmu_iva = {
>>   	.master		= &omap3xxx_l3_main_hwmod,
>>   	.slave		= &omap3xxx_mmu_iva_hwmod,
>> -	.addr		= omap3xxx_mmu_iva_addrs,
>>   	.user		= OCP_USER_MPU | OCP_USER_SDMA,
>>   };
>>
>>   static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
>>   	.name		= "mmu_iva",
>>   	.class		= &omap3xxx_mmu_hwmod_class,
>> -	.mpu_irqs	= omap3xxx_mmu_iva_irqs,
>>   	.clkdm_name	= "iva2_clkdm",
>>   	.rst_lines	= omap3xxx_mmu_iva_resets,
>>   	.rst_lines_cnt	= ARRAY_SIZE(omap3xxx_mmu_iva_resets),
>> @@ -3080,7 +3035,6 @@ static struct omap_hwmod omap3xxx_mmu_iva_hwmod = {
>>   			.idlest_idle_bit = OMAP3430_ST_IVA2_SHIFT,
>>   		},
>>   	},
>> -	.dev_attr	= &mmu_iva_dev_attr,
>>   	.flags		= HWMOD_NO_IDLEST,
>>   };
>>
>> --
>> 1.8.5.3
>>

^ permalink raw reply

* [RESEND PATCH v11] video: backlight: gpio-backlight: Add DT support.
From: Denis Carikli @ 2014-02-26 18:04 UTC (permalink / raw)
  To: Lee Jones
  Cc: Bryan Wu, Jingoo Han, Eric Bénard,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Denis Carikli

Cc: Bryan Wu <cooloney-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Jingoo Han <jg1.han-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Cc: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Eric Bénard <eric-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Denis Carikli <denis-fO0SIAKYzcbQT0dZR+AlfA@public.gmane.org>
---
ChangeLog v11->v11-resend:
- Removed the Cc list because it's only one patch.

ChangeLog v10->v11:
- Shrinked the Cc list.
- Fixed the of_match_table warning.

ChangeLog v9->v10:
- Only the respective maintainers, or people who responded to the patch
  were kept in the Cc.
- The unnecessary of_match_ptr in of_match_table was removed.

ChangeLog v8->v9:
- Added Shawn Guo in the Cc list.
- The default-brightness-level is now a boolean default-on property,
  the gpio is only touched if the gpio-backlight driver instance probes.
- The code and documentation was updated accordingly.

ChangeLog v7->v8:
- The default-state was renamed to default-brightness-level.
- default-brightness-level is now mandatory, like for backlight-pwm,
  That way we avoid having to handle the case where it's not set,
  which means that we would need not to set the gpio, but still
  report a brightness value for sysfs, when not all gpio controllers are
  able to read the gpio value.
- switched the default-brightness-level to boolean values (0 or 1) instead
  of using strings ("on", "off", "keep").
- The documentation was updated accordingly.
- The example in the documentation now uses the dts gpio defines.
- The "backlight: gpio_backlight: Use a default state enum." patch was then
  dropped, becuase it is not necessary anymore.

ChangeLog v6->v7:
- removed a compilation warning with the removal of the useless ret declaration.

ChangeLog v5->v6:
- The default state handling was reworked:
  - it's now called default-state, and looks like the gpio-leds default-state.
  - it now has a "keep" option, like for the gpio-leds.
  - that "keep" option is the default when the default-state property is not set.
- The documentation was updated accordingly.

ChangeLog v4->v5:
- The default-brightness property now defaults to 0 in the driver.
- def_value int becomes a bool.
- The check for the gpio validity has been reworked.
---
 .../bindings/video/backlight/gpio-backlight.txt    |   19 +++++++
 drivers/video/backlight/gpio_backlight.c           |   60 +++++++++++++++++---
 2 files changed, 72 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/video/backlight/gpio-backlight.txt

diff --git a/Documentation/devicetree/bindings/video/backlight/gpio-backlight.txt b/Documentation/devicetree/bindings/video/backlight/gpio-backlight.txt
new file mode 100644
index 0000000..a923193
--- /dev/null
+++ b/Documentation/devicetree/bindings/video/backlight/gpio-backlight.txt
@@ -0,0 +1,19 @@
+gpio-backlight bindings
+
+Required properties:
+  - compatible: "gpio-backlight"
+  - gpios: describes the gpio that is used for enabling/disabling the backlight
+    (see GPIO binding[0] for more details).
+
+Optional properties:
+  - default-on: enable the backlight at boot.
+
+[0]: Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+
+	backlight {
+		compatible = "gpio-backlight";
+		gpios = <&gpio3 4 GPIO_ACTIVE_HIGH>;
+		default-on;
+	};
diff --git a/drivers/video/backlight/gpio_backlight.c b/drivers/video/backlight/gpio_backlight.c
index 81fb127..2422614 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -13,6 +13,8 @@
 #include <linux/init.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
 #include <linux/platform_data/gpio_backlight.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
@@ -23,6 +25,7 @@ struct gpio_backlight {
 
 	int gpio;
 	int active;
+	int def_value;
 };
 
 static int gpio_backlight_update_status(struct backlight_device *bl)
@@ -60,6 +63,33 @@ static const struct backlight_ops gpio_backlight_ops = {
 	.check_fb	= gpio_backlight_check_fb,
 };
 
+static int gpio_backlight_probe_dt(struct platform_device *pdev,
+				   struct gpio_backlight *gbl)
+{
+	struct device_node *np = pdev->dev.of_node;
+	enum of_gpio_flags gpio_flags;
+	bool default_on = false;
+
+	gbl->fbdev = NULL;
+	gbl->gpio = of_get_gpio_flags(np, 0, &gpio_flags);
+
+	if (!gpio_is_valid(gbl->gpio)) {
+		if (gbl->gpio != -EPROBE_DEFER) {
+			dev_err(&pdev->dev,
+				"Error: The gpios parameter is missing or invalid.\n");
+		}
+		return gbl->gpio;
+	}
+
+	gbl->active = (gpio_flags & OF_GPIO_ACTIVE_LOW) ? 0 : 1;
+
+	default_on = of_property_read_bool(np, "default-on");
+
+	gbl->def_value = default_on;
+
+	return 0;
+}
+
 static int gpio_backlight_probe(struct platform_device *pdev)
 {
 	struct gpio_backlight_platform_data *pdata =
@@ -67,10 +97,12 @@ static int gpio_backlight_probe(struct platform_device *pdev)
 	struct backlight_properties props;
 	struct backlight_device *bl;
 	struct gpio_backlight *gbl;
+	struct device_node *np = pdev->dev.of_node;
 	int ret;
 
-	if (!pdata) {
-		dev_err(&pdev->dev, "failed to find platform data\n");
+	if (!pdata && !np) {
+		dev_err(&pdev->dev,
+			"failed to find platform data or device tree node.\n");
 		return -ENODEV;
 	}
 
@@ -79,14 +111,22 @@ static int gpio_backlight_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	gbl->dev = &pdev->dev;
-	gbl->fbdev = pdata->fbdev;
-	gbl->gpio = pdata->gpio;
-	gbl->active = pdata->active_low ? 0 : 1;
+
+	if (np) {
+		ret = gpio_backlight_probe_dt(pdev, gbl);
+		if (ret)
+			return ret;
+	} else {
+		gbl->fbdev = pdata->fbdev;
+		gbl->gpio = pdata->gpio;
+		gbl->active = pdata->active_low ? 0 : 1;
+		gbl->def_value = pdata->def_value;
+	}
 
 	ret = devm_gpio_request_one(gbl->dev, gbl->gpio, GPIOF_DIR_OUT |
 				    (gbl->active ? GPIOF_INIT_LOW
 						 : GPIOF_INIT_HIGH),
-				    pdata->name);
+				    pdata ? pdata->name : "backlight");
 	if (ret < 0) {
 		dev_err(&pdev->dev, "unable to request GPIO\n");
 		return ret;
@@ -103,17 +143,23 @@ static int gpio_backlight_probe(struct platform_device *pdev)
 		return PTR_ERR(bl);
 	}
 
-	bl->props.brightness = pdata->def_value;
+	bl->props.brightness = gbl->def_value;
 	backlight_update_status(bl);
 
 	platform_set_drvdata(pdev, bl);
 	return 0;
 }
 
+static struct of_device_id gpio_backlight_of_match[] = {
+	{ .compatible = "gpio-backlight" },
+	{ /* sentinel */ }
+};
+
 static struct platform_driver gpio_backlight_driver = {
 	.driver		= {
 		.name		= "gpio-backlight",
 		.owner		= THIS_MODULE,
+		.of_match_table = gpio_backlight_of_match,
 	},
 	.probe		= gpio_backlight_probe,
 };
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCHv2 10/16] ARM: OMAP2+: use pdata quirks for iommu reset lines
From: Suman Anna @ 2014-02-26 18:04 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	Laurent Pinchart, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	Florian Vaussard
In-Reply-To: <20140226171731.GG11654-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

Hi Tony,

On 02/26/2014 11:17 AM, Tony Lindgren wrote:
> * Suman Anna <s-anna-l0cyMroinI0@public.gmane.org> [140213 10:19]:
>> The OMAP iommu driver performs the reset management for the
>> iommu instances in processor sub-systems using the omap_device
>> API which are currently supplied as platform data ops. Use pdata
>> quirks to maintain the functionality as the OMAP iommu driver
>> gets converted to use DT nodes, until the reset portions are
>> decoupled from omap_hwmod/omap_device into a separate reset
>> driver.
>>
>> This patch adds the pdata quirks for the reset management of
>> iommus within the DSP (OMAP3 & OMAP4) and IPU subsystems (OMAP4).
>>
>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>
> Looks OK, but I suggest you separate out the remaining patches in
> this series into another clean-up series. Then the clean-up series
> can be merged later on as these have a good chance of conflicting
> with other stuff.

Only patches 14 through 16 are cleanup patches. Patches 12 and 13 are 
adding OMAP5 functionality, and Patch11 is fixing up OMAP3 IVA. I would 
have to drop Patch14 and Patch16 until OMAP3 is completely DT, so will 
drop them for now. Let me know if you want me to split out the remaining 
applicable patches that are in arch/arm/ into a separate series.

regards
Suman

>
> Tony
>
>> ---
>>   arch/arm/mach-omap2/pdata-quirks.c | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
>> index 3d5b24d..74e094a 100644
>> --- a/arch/arm/mach-omap2/pdata-quirks.c
>> +++ b/arch/arm/mach-omap2/pdata-quirks.c
>> @@ -16,12 +16,14 @@
>>   #include <linux/wl12xx.h>
>>
>>   #include <linux/platform_data/pinctrl-single.h>
>> +#include <linux/platform_data/iommu-omap.h>
>>
>>   #include "am35xx.h"
>>   #include "common.h"
>>   #include "common-board-devices.h"
>>   #include "dss-common.h"
>>   #include "control.h"
>> +#include "omap_device.h"
>>
>>   struct pdata_init {
>>   	const char *compatible;
>> @@ -92,6 +94,12 @@ static void __init hsmmc2_internal_input_clk(void)
>>   	omap_ctrl_writel(reg, OMAP343X_CONTROL_DEVCONF1);
>>   }
>>
>> +static struct iommu_platform_data omap3_iommu_pdata = {
>> +	.reset_name = "mmu",
>> +	.assert_reset = omap_device_assert_hardreset,
>> +	.deassert_reset = omap_device_deassert_hardreset,
>> +};
>> +
>>   static int omap3_sbc_t3730_twl_callback(struct device *dev,
>>   					   unsigned gpio,
>>   					   unsigned ngpio)
>> @@ -185,6 +193,12 @@ static void __init omap4_panda_legacy_init(void)
>>   	legacy_init_ehci_clk("auxclk3_ck");
>>   	legacy_init_wl12xx(WL12XX_REFCLOCK_38, 0, 53);
>>   }
>> +
>> +static struct iommu_platform_data omap4_iommu_pdata = {
>> +	.reset_name = "mmu_cache",
>> +	.assert_reset = omap_device_assert_hardreset,
>> +	.deassert_reset = omap_device_deassert_hardreset,
>> +};
>>   #endif
>>
>>   #ifdef CONFIG_SOC_OMAP5
>> @@ -240,6 +254,8 @@ struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
>>   #ifdef CONFIG_ARCH_OMAP3
>>   	OF_DEV_AUXDATA("ti,omap3-padconf", 0x48002030, "48002030.pinmux", &pcs_pdata),
>>   	OF_DEV_AUXDATA("ti,omap3-padconf", 0x48002a00, "48002a00.pinmux", &pcs_pdata),
>> +	OF_DEV_AUXDATA("ti,omap2-iommu", 0x5d000000, "5d000000.mmu",
>> +		       &omap3_iommu_pdata),
>>   	/* Only on am3517 */
>>   	OF_DEV_AUXDATA("ti,davinci_mdio", 0x5c030000, "davinci_mdio.0", NULL),
>>   	OF_DEV_AUXDATA("ti,am3517-emac", 0x5c000000, "davinci_emac.0",
>> @@ -248,6 +264,10 @@ struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
>>   #ifdef CONFIG_ARCH_OMAP4
>>   	OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a100040, "4a100040.pinmux", &pcs_pdata),
>>   	OF_DEV_AUXDATA("ti,omap4-padconf", 0x4a31e040, "4a31e040.pinmux", &pcs_pdata),
>> +	OF_DEV_AUXDATA("ti,omap4-iommu", 0x4a066000, "4a066000.mmu",
>> +		       &omap4_iommu_pdata),
>> +	OF_DEV_AUXDATA("ti,omap4-iommu", 0x55082000, "55082000.mmu",
>> +		       &omap4_iommu_pdata),
>>   #endif
>>   	{ /* sentinel */ },
>>   };
>> --
>> 1.8.5.3
>>

^ permalink raw reply

* [PATCH v4 0/8] Modernize pm8921 with irqdomains, regmap, DT
From: Stephen Boyd @ 2014-02-26 18:59 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones
  Cc: linux-kernel, linux-arm-msm, devicetree, Mark Brown,
	linux-arm-kernel

These patches lay the groundwork for converting the pm8921 sub-devices
to devicetree as well as simplify the API by migrating the core code
to use the regmap API instead of the custom pm8xxx read/write wrapper.

Changes since v3:
 * Added irqdomain Kconfig select
 * New patch to loosen Kconfig on PM8921 config
 * Picked up acked-by

Changes since v2:
 * Picked up reviewed-by tags
 * Fixed irqdomain teardown in driver remove
 * No magical 256 constant
 * Renamed domain to irqdomain
 * Clarified kzalloc call
 * Pushed ssbi regmap helpers into header file
 * Fixed whitespace noise in patch 6

Changes since v1:
 * First 3 cleanup patches dropped because they're applied upstream
 * New regmap read/write helpers
 * New patch for DT match table
 * New binding document

Stephen Boyd (8):
  mfd: Move pm8xxx-irq.c contents into only driver that uses it
  mfd: pm8921: Update for genirq changes
  mfd: pm8921: Migrate to irqdomains
  mfd: ssbi: Add regmap read/write helpers
  mfd: pm8921: Use ssbi regmap
  mfd: pm8921: Add DT match table
  mfd: pm8921: Loosen Kconfig dependency
  devicetree: bindings: Document PM8921/8058 PMICs

 .../devicetree/bindings/mfd/qcom,pm8xxx.txt        |  63 +++
 drivers/mfd/Kconfig                                |  15 +-
 drivers/mfd/Makefile                               |   1 -
 drivers/mfd/pm8921-core.c                          | 427 ++++++++++++++++++---
 drivers/mfd/pm8xxx-irq.c                           | 371 ------------------
 include/linux/mfd/pm8xxx/irq.h                     |  59 ---
 include/linux/mfd/pm8xxx/pm8921.h                  |  30 --
 include/linux/ssbi.h                               |  20 +
 8 files changed, 460 insertions(+), 526 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,pm8xxx.txt
 delete mode 100644 drivers/mfd/pm8xxx-irq.c
 delete mode 100644 include/linux/mfd/pm8xxx/irq.h
 delete mode 100644 include/linux/mfd/pm8xxx/pm8921.h

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply

* [PATCH v4 8/8] devicetree: bindings: Document PM8921/8058 PMICs
From: Stephen Boyd @ 2014-02-26 18:59 UTC (permalink / raw)
  To: Samuel Ortiz, Lee Jones
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree
In-Reply-To: <1393441166-32692-1-git-send-email-sboyd@codeaurora.org>

PM8921 and PM8058 are PMICs found paired with MSM8960 and MSM8660
devices respectively. They contain subdevices such as keypads,
RTC, regulators, clocks, etc.

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 .../devicetree/bindings/mfd/qcom,pm8xxx.txt        | 63 ++++++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/qcom,pm8xxx.txt

diff --git a/Documentation/devicetree/bindings/mfd/qcom,pm8xxx.txt b/Documentation/devicetree/bindings/mfd/qcom,pm8xxx.txt
new file mode 100644
index 000000000000..e3fe625ffd58
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/qcom,pm8xxx.txt
@@ -0,0 +1,63 @@
+Qualcomm PM8xxx PMIC multi-function devices
+
+PROPERTIES
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,pm8058"
+		    "qcom,pm8921"
+
+- #address-cells:
+	Usage: required
+	Value type: <u32>
+	Definition: must be 1
+
+- #size-cells:
+	Usage: required
+	Value type: <u32>
+	Definition: must be 0
+
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: specifies the interrupt that indicates a subdevice
+		    has generated an interrupt (summary interrupt). The
+		    format of the specifier is defined by the binding document
+		    describing the node's interrupt parent.
+
+- #interrupt-cells:
+	Usage: required
+	Value type : <u32>
+	Definition: must be 2. Specifies the number of cells needed to encode
+		    an interrupt source. The 1st cell contains the interrupt
+		    number. The 2nd cell is the trigger type and level flags
+		    encoded as follows:
+
+			1 = low-to-high edge triggered
+			2 = high-to-low edge triggered
+			4 = active high level-sensitive
+			8 = active low level-sensitive
+
+- interrupt-controller:
+	Usage: required
+	Value type: <empty>
+	Definition: identifies this node as an interrupt controller
+
+EXAMPLE
+
+	pmicintc: pmic@0 {
+		compatible = "qcom,pm8921";
+		interrupts = <104 8>;
+		#interrupt-cells = <2>;
+		interrupt-controller;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		pwrkey {
+			compatible = "qcom,pm8921-pwrkey";
+			interrupt-parent = <&pmicintc>;
+			interrupts = <50 1>, <51 1>;
+		};
+	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH v3 0/9] Use regmap+devm+DT in pm8xxx input drivers
From: Stephen Boyd @ 2014-02-26 19:05 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, linux-input,
	devicetree

These patches move the pm8xxx input drivers over to use devm_* APIs
and regmap. This breaks the dependency of these drivers on the pm8xxx
specific read/write calls and also simplifies the probe code a bit.
Finally we add devicetree support to these drivers so they can be probed
on the platforms that are supported upstream.

Changes since v2:
 * Rebased to v3.14-rc3

Changes since v1:
 * Picked up Dmitry's version of devm for pwrkey
 * Added DT bindings and parsing patches
 * Dropped patches picked up by Dmitry


Stephen Boyd (9):
  Input: pmic8xxx-pwrkey - Migrate to regmap APIs
  Input: pmic8xxx-keypad - Migrate to devm_* APIs
  Input: pmic8xxx-keypad - Migrate to regmap APIs
  Input: pmic8xxx-pwrkey - Migrate to DT
  Input: pm8xxx-vibrator - Add DT match table
  Input: pmic8xxx-keypad - Migrate to DT
  devicetree: bindings: Document PM8921/8058 keypads
  devicetree: bindings: Document PM8921/8058 power keys
  devicetree: bindings: Document PM8921/8058 vibrators

 .../bindings/input/qcom,pm8xxx-keypad.txt          |  72 +++++
 .../bindings/input/qcom,pm8xxx-pwrkey.txt          |  39 +++
 .../devicetree/bindings/input/qcom,pm8xxx-vib.txt  |  16 ++
 drivers/input/keyboard/pmic8xxx-keypad.c           | 291 ++++++++++-----------
 drivers/input/misc/pm8xxx-vibrator.c               |   8 +
 drivers/input/misc/pmic8xxx-pwrkey.c               |  37 ++-
 include/linux/input/pmic8xxx-keypad.h              |  52 ----
 include/linux/input/pmic8xxx-pwrkey.h              |  31 ---
 8 files changed, 290 insertions(+), 256 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
 create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt
 create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.txt
 delete mode 100644 include/linux/input/pmic8xxx-keypad.h
 delete mode 100644 include/linux/input/pmic8xxx-pwrkey.h

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation


^ permalink raw reply

* [PATCH v3 7/9] devicetree: bindings: Document PM8921/8058 keypads
From: Stephen Boyd @ 2014-02-26 19:06 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree
In-Reply-To: <1393441562-685-1-git-send-email-sboyd@codeaurora.org>

Document the keypad device found on PM8921 and PM8058 PMICs.

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 .../bindings/input/qcom,pm8xxx-keypad.txt          | 72 ++++++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt

diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt b/Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
new file mode 100644
index 000000000000..aa5a9c6cf512
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt
@@ -0,0 +1,72 @@
+Qualcomm PM8xxx PMIC Keypad
+
+PROPERTIES
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,pm8058-keypad"
+		    "qcom,pm8921-keypad"
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: the first interrupt specifies the key sense interrupt
+		    and the second interrupt specifies the key stuck interrupt.
+		    The format of the specifier is defined by the binding
+		    document describing the node's interrupt parent.
+
+- linux,keymap:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: the linux keymap. More information can be found in
+		    input/matrix-keymap.txt.
+
+- keypad,num-rows:
+	Usage: required
+	Value type: <u32>
+	Definition: number of rows in the keymap. More information can be found
+		    in input/matrix-keymap.txt.
+
+- keypad,num-columns:
+	Usage: required
+	Value type: <u32>
+	Definition: number of columns in the keymap. More information can be
+		    found in input/matrix-keymap.txt.
+
+- debounce:
+	Usage: optional
+	Value type: <u32>
+	Definition: time in microseconds that key must be pressed or release
+		    for key sense interrupt to trigger.
+
+- scan-delay:
+	Usage: optional
+	Value type: <u32>
+	Definition: time in microseconds to pause between successive scans
+		    of the matrix array.
+
+- row-hold:
+	Usage: optional
+	Value type: <u32>
+	Definition: time in nanoseconds to pause between scans of each row in
+		    the matrix array.
+
+EXAMPLE
+
+	keypad {
+		compatible = "qcom,pm8921-keypad";
+		interrupt-parent = <&pmicintc>;
+		interrupts = <74 1>, <75 1>;
+		linux,keymap = <
+			MATRIX_KEY(0, 0, KEY_VOLUMEUP)
+			MATRIX_KEY(0, 1, KEY_VOLUMEDOWN)
+			MATRIX_KEY(0, 2, KEY_CAMERA_FOCUS)
+			MATRIX_KEY(0, 3, KEY_CAMERA)
+			>;
+		keypad,num-rows = <1>;
+		keypad,num-columns = <5>;
+		debounce = <15>;
+		scan-delay = <32>;
+		row-hold = <91500>;
+	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH v3 8/9] devicetree: bindings: Document PM8921/8058 power keys
From: Stephen Boyd @ 2014-02-26 19:06 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-kernel, linux-arm-msm, linux-arm-kernel, devicetree
In-Reply-To: <1393441562-685-1-git-send-email-sboyd@codeaurora.org>

Document the power key found on PM8921 and PM8058 PMICs.

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 .../bindings/input/qcom,pm8xxx-pwrkey.txt          | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt

diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt b/Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt
new file mode 100644
index 000000000000..e124d9f33632
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-pwrkey.txt
@@ -0,0 +1,39 @@
+Qualcomm PM8xxx PMIC Power Key
+
+PROPERTIES
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,pm8058-pwrkey"
+		    "qcom,pm8921-pwrkey"
+- interrupts:
+	Usage: required
+	Value type: <prop-encoded-array>
+	Definition: the first interrupt specifies the key release interrupt
+		    and the second interrupt specifies the key press interrupt.
+		    The format of the specifier is defined by the binding
+		    document describing the node's interrupt parent.
+
+- debounce:
+	Usage: optional
+	Value type: <u32>
+	Definition: time in microseconds that key must be pressed or release
+		    for state change interrupt to trigger.
+
+- pull-up:
+	Usage: optional
+	Value type: <empty>
+	Definition: presence of this property indicates that the KPDPWR_N pin
+		    should be configured for pull up.
+
+EXAMPLE
+
+	pwrkey {
+		compatible = "qcom,pm8921-pwrkey";
+		interrupt-parent = <&pmicintc>;
+		interrupts = <50 1>, <51 1>;
+		debounce = <15625>;
+		pull-up;
+	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related

* [PATCH v3 9/9] devicetree: bindings: Document PM8921/8058 vibrators
From: Stephen Boyd @ 2014-02-26 19:06 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-arm-msm, linux-kernel, linux-arm-kernel, devicetree
In-Reply-To: <1393441562-685-1-git-send-email-sboyd@codeaurora.org>

Document the vibration device found on PM8921 and PM8058 PMICs.

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 .../devicetree/bindings/input/qcom,pm8xxx-vib.txt        | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.txt

diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.txt b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.txt
new file mode 100644
index 000000000000..dca1b8872cf1
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.txt
@@ -0,0 +1,16 @@
+Qualcomm PM8xxx PMIC Vibrator
+
+PROPERTIES
+
+- compatible:
+	Usage: required
+	Value type: <string>
+	Definition: must be one of:
+		    "qcom,pm8058-vib"
+		    "qcom,pm8921-vib"
+
+EXAMPLE
+
+	vibrator {
+		compatible = "qcom,pm8058-vib";
+	};
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related

* Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings
From: Laurent Pinchart @ 2014-02-26 19:32 UTC (permalink / raw)
  To: Suman Anna
  Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Tony Lindgren,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kumar Gala,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, Florian Vaussard
In-Reply-To: <530E1E20.9000301-l0cyMroinI0@public.gmane.org>

Hi Suman,

On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
> > On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
> >> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
> >>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
> >>>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> >>>> 
> >>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
> >>>> the standard bindings used by OMAP peripherals, this patch uses a
> >>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
> >>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
> >>>> 
> >>>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
> >>>> [s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error back]
> >>>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
> >>>> ---
> >>>> 
> >>>>  .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 ++++++++++++++
> >>>>  1 file changed, 28 insertions(+)
> >>>>  create mode 100644
> >>>>  Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>> 
> >>>> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file
> >>>> mode 100644
> >>>> index 0000000..116492d
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>> @@ -0,0 +1,28 @@
> >>>> +OMAP2+ IOMMU
> >>>> +
> >>>> +Required properties:
> >>>> +- compatible : Should be one of,
> >>>> +		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
> >>>> +		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
> >>>> +		"ti,dra7-iommu" for DRA7xx IOMMU instances
> >>>> +- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
> >>>> +- reg        : Address space for the configuration registers
> >>>> +- interrupts : Interrupt specifier for the IOMMU instance
> >>>> +- dma-window : IOVA start address and length
> >>> 
> >>> Isn't the dma window more of a system configuration property than a
> >>> hardware property ? How do you expect it to be set?
> >> 
> >> We are setting it based on the addressable range for the MMU.
> > 
> > A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
> > support the full 4GB VA space. Why do you need to restrict it ?
> 
> I should have rephrased it better when I said addressable range. While
> the MMUs are capable of programming the full 4GB space, there are some
> address ranges that are private from the processor view. This window is
> currently used to set the range for the omap-iovmm driver (which only
> OMAP3 ISP is using atm), and there is no point in allowing the
> omap-iovmm driver the full range when the processor could never
> reach/access those addresses.

But the IOMMU VA space is from a device point of view, not from a CPU point of 
view. Could you point me to where those private ranges are documented, in 
order to understand the problem correctly ?

> >> We are reusing the existing defined property and it allows us to get rid
> >> of the IOVA start and end addresses defined in the pre-DT OMAP iommu
> >> platform data.
> >> 
> >>>> +Optional properties:
> >>>> +- ti,#tlb-entries : Number of entries in the translation look-aside
> >>>> buffer. +                    Should be either 8 or 32 (default: 32)
> >>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports
> >>>> throwing
> >>>> +		          back a bus error response on MMU faults.
> >>> 
> >>> Do these features vary per IOMMU instance or per IOMMU model ? In the
> >>> latter case they could be inferred from the compatible string by the
> >>> driver without requiring them to be explicit in DT (whether you want to
> >>> do so is left to you though).
> >> 
> >> Well, these are fixed features given an IOMMU instance, like the OMAP3
> >> ISP is the only one that has 8 TLB entries, all the remaining ones have
> >> 32, and the IPU iommu instances are the only ones that support the bus
> >> error response back. I have no preference to any particular way, and
> >> sure the driver can infer these easily based on unique compatible
> >> strings per subsystem per SoC. I just happened to go with defining
> >> compatible strings per SoC, with the optional properties differentiating
> >> the fixed behavior between different IOMMU instances on that SoC. This
> >> is where I was looking for some inputs/guidance from the DT bindings
> >> maintainers on what is the preferred method.
> > 
> > I think you've made the right choice. I wasn't sure whether those
> > parameters varied across IOMMU instances of compatible devices (from a
> > compatible string point of view) or were constant. As they vary they
> > should be expressed in DT.
>
> Yeah, I wasn't sure if these qualify as features (as per
> Documentation/devicetree/bindings/ABI.txt section II.2).
> 
> regards
> Suman
> 
> >>>> +Example:
> >>>> +	/* OMAP3 ISP MMU */
> >>>> +	mmu_isp: mmu@480bd400 {
> >>>> +		compatible = "ti,omap2-iommu";
> >>>> +		reg = <0x480bd400 0x80>;
> >>>> +		interrupts = <24>;
> >>>> +		ti,hwmods = "mmu_isp";
> >>>> +		ti,#tlb-entries = <8>;
> >>>> +		dma-window = <0 0xfffff000>;
> >>>> +	};

-- 
Regards,

Laurent Pinchart

^ permalink raw reply

* [RFCv4 0/7] Convert twl4030-madc to IIO API and add DT support
From: Sebastian Reichel @ 2014-02-26 20:03 UTC (permalink / raw)
  To: Sebastian Reichel, Marek Belisko, Jonathan Cameron
  Cc: Lee Jones, Samuel Ortiz, Lars-Peter Clausen, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Sebastian Reichel
In-Reply-To: <1393374270-20079-1-git-send-email-sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>

Hi,

This is RFCv4 for converting twl4030-madc to the IIO API and
adding DT support. The patchset compiles and has been tested
on my Nokia N900.

Changes since RFCv3 [0]:
 * Added Acked-by from Lee Jones (MFD subsystem maintainer)
   to twl_i2c_read/write_u16 patches
 * Fixed style issues found by Lee Jones
   This readded the temp variable Jonathan Cameron wanted to
   be removed, but makes the code cleaner - especially once
   the brackets for math for clarity are added.
 * Tested with converted rx51-battery driver on Nokia N900,
   which can be found at [1].

[0] https://lkml.org/lkml/2014/2/25/627
[1] https://lkml.org/lkml/2014/2/25/639

-- Sebastian

Sebastian Reichel (7):
  mfd: twl4030-madc: Use managed resources
  mfd: twl4030-madc: Add DT support and convert to IIO framework
  mfd: twl4030-madc: Cleanup driver
  mfd: twl-core: Add twl_i2c_read/write_u16
  mfd: twl4030-madc: Use twl_i2c_read/write_u16 for 16 bit registers
  Documentation: DT: Document twl4030-madc binding
  mfd: twl4030-madc: Move driver to drivers/iio/adc

 .../devicetree/bindings/iio/adc/twl4030-madc.txt   |  24 ++
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/{mfd => iio/adc}/twl4030-madc.c            | 281 +++++++++++++--------
 drivers/mfd/Kconfig                                |  10 -
 drivers/mfd/Makefile                               |   1 -
 include/linux/i2c/twl.h                            |  12 +
 include/linux/i2c/twl4030-madc.h                   |   2 +-
 8 files changed, 227 insertions(+), 114 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/twl4030-madc.txt
 rename drivers/{mfd => iio/adc}/twl4030-madc.c (77%)

-- 
1.8.5.3

^ permalink raw reply

* [RFCv4 1/7] mfd: twl4030-madc: Use managed resources
From: Sebastian Reichel @ 2014-02-26 20:03 UTC (permalink / raw)
  To: Sebastian Reichel, Marek Belisko, Jonathan Cameron
  Cc: Lee Jones, Samuel Ortiz, Lars-Peter Clausen, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
	linux-kernel, devicetree, linux-iio, Sebastian Reichel
In-Reply-To: <1393444990-28140-1-git-send-email-sre@debian.org>

Update twl4030-madc driver to use managed resources.

Signed-off-by: Sebastian Reichel <sre@debian.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/twl4030-madc.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/twl4030-madc.c b/drivers/mfd/twl4030-madc.c
index 4c583e4..5458561 100644
--- a/drivers/mfd/twl4030-madc.c
+++ b/drivers/mfd/twl4030-madc.c
@@ -702,14 +702,14 @@ static int twl4030_madc_probe(struct platform_device *pdev)
 {
 	struct twl4030_madc_data *madc;
 	struct twl4030_madc_platform_data *pdata = dev_get_platdata(&pdev->dev);
-	int ret;
+	int irq, ret;
 	u8 regval;
 
 	if (!pdata) {
 		dev_err(&pdev->dev, "platform_data not available\n");
 		return -EINVAL;
 	}
-	madc = kzalloc(sizeof(*madc), GFP_KERNEL);
+	madc = devm_kzalloc(&pdev->dev, sizeof(*madc), GFP_KERNEL);
 	if (!madc)
 		return -ENOMEM;
 
@@ -726,7 +726,7 @@ static int twl4030_madc_probe(struct platform_device *pdev)
 	    TWL4030_MADC_ISR1 : TWL4030_MADC_ISR2;
 	ret = twl4030_madc_set_power(madc, 1);
 	if (ret < 0)
-		goto err_power;
+		return ret;
 	ret = twl4030_madc_set_current_generator(madc, 0, 1);
 	if (ret < 0)
 		goto err_current_generator;
@@ -770,7 +770,9 @@ static int twl4030_madc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, madc);
 	mutex_init(&madc->lock);
-	ret = request_threaded_irq(platform_get_irq(pdev, 0), NULL,
+
+	irq = platform_get_irq(pdev, 0);
+	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
 				   twl4030_madc_threaded_irq_handler,
 				   IRQF_TRIGGER_RISING, "twl4030_madc", madc);
 	if (ret) {
@@ -783,9 +785,6 @@ err_i2c:
 	twl4030_madc_set_current_generator(madc, 0, 0);
 err_current_generator:
 	twl4030_madc_set_power(madc, 0);
-err_power:
-	kfree(madc);
-
 	return ret;
 }
 
@@ -793,10 +792,8 @@ static int twl4030_madc_remove(struct platform_device *pdev)
 {
 	struct twl4030_madc_data *madc = platform_get_drvdata(pdev);
 
-	free_irq(platform_get_irq(pdev, 0), madc);
 	twl4030_madc_set_current_generator(madc, 0, 0);
 	twl4030_madc_set_power(madc, 0);
-	kfree(madc);
 
 	return 0;
 }
-- 
1.8.5.3

^ permalink raw reply related

* [RFCv4 2/7] mfd: twl4030-madc: Add DT support and convert to IIO framework
From: Sebastian Reichel @ 2014-02-26 20:03 UTC (permalink / raw)
  To: Sebastian Reichel, Marek Belisko, Jonathan Cameron
  Cc: Lee Jones, Samuel Ortiz, Lars-Peter Clausen, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
	linux-kernel, devicetree, linux-iio, Sebastian Reichel
In-Reply-To: <1393444990-28140-1-git-send-email-sre@debian.org>

This converts twl4030-madc module to use the Industrial IO ADC
framework and adds device tree support.

Signed-off-by: Sebastian Reichel <sre@debian.org>
---
 drivers/mfd/twl4030-madc.c | 127 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 118 insertions(+), 9 deletions(-)

diff --git a/drivers/mfd/twl4030-madc.c b/drivers/mfd/twl4030-madc.c
index 5458561..37cb3ad 100644
--- a/drivers/mfd/twl4030-madc.c
+++ b/drivers/mfd/twl4030-madc.c
@@ -47,11 +47,14 @@
 #include <linux/gfp.h>
 #include <linux/err.h>
 
+#include <linux/iio/iio.h>
+
 /*
  * struct twl4030_madc_data - a container for madc info
  * @dev - pointer to device structure for madc
  * @lock - mutex protecting this data structure
  * @requests - Array of request struct corresponding to SW1, SW2 and RT
+ * @use_second_irq - IRQ selection (main or co-processor)
  * @imr - Interrupt mask register of MADC
  * @isr - Interrupt status register of MADC
  */
@@ -59,10 +62,73 @@ struct twl4030_madc_data {
 	struct device *dev;
 	struct mutex lock;	/* mutex protecting this data structure */
 	struct twl4030_madc_request requests[TWL4030_MADC_NUM_METHODS];
+	bool use_second_irq;
 	int imr;
 	int isr;
 };
 
+static int twl4030_madc_read(struct iio_dev *iio_dev,
+			     const struct iio_chan_spec *chan,
+			     int *val, int *val2, long mask)
+{
+	struct twl4030_madc_data *madc = iio_priv(iio_dev);
+	struct twl4030_madc_request req;
+	int ret;
+
+	req.method = madc->use_second_irq ? TWL4030_MADC_SW2 : TWL4030_MADC_SW1;
+
+	req.channels = BIT(chan->channel);
+	req.active = false;
+	req.func_cb = NULL;
+	req.type = TWL4030_MADC_WAIT;
+	req.raw = !(mask == IIO_CHAN_INFO_PROCESSED);
+	req.do_avg = (mask == IIO_CHAN_INFO_AVERAGE_RAW);
+
+	ret = twl4030_madc_conversion(&req);
+	if (ret < 0)
+		return ret;
+
+	*val = req.rbuf[chan->channel];
+
+	return IIO_VAL_INT;
+}
+
+static const struct iio_info twl4030_madc_iio_info = {
+	.read_raw = &twl4030_madc_read,
+	.driver_module = THIS_MODULE,
+};
+
+#define TWL4030_ADC_CHANNEL(_channel, _type, _name, _mask) {	\
+	.type = _type,					\
+	.channel = _channel,				\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |  \
+			      BIT(IIO_CHAN_INFO_AVERAGE_RAW) | \
+			      _mask,			\
+	.datasheet_name = _name,			\
+	.indexed = 1,					\
+}
+
+static const struct iio_chan_spec twl4030_madc_iio_channels[] = {
+	TWL4030_ADC_CHANNEL(0, IIO_VOLTAGE, "ADCIN0", 0),
+	TWL4030_ADC_CHANNEL(1, IIO_TEMP, "ADCIN1",
+		BIT(IIO_CHAN_INFO_PROCESSED)),
+	TWL4030_ADC_CHANNEL(2, IIO_VOLTAGE, "ADCIN2", 0),
+	TWL4030_ADC_CHANNEL(3, IIO_VOLTAGE, "ADCIN3", 0),
+	TWL4030_ADC_CHANNEL(4, IIO_VOLTAGE, "ADCIN4", 0),
+	TWL4030_ADC_CHANNEL(5, IIO_VOLTAGE, "ADCIN5", 0),
+	TWL4030_ADC_CHANNEL(6, IIO_VOLTAGE, "ADCIN6", 0),
+	TWL4030_ADC_CHANNEL(7, IIO_VOLTAGE, "ADCIN7", 0),
+	TWL4030_ADC_CHANNEL(8, IIO_VOLTAGE, "ADCIN8", 0),
+	TWL4030_ADC_CHANNEL(9, IIO_VOLTAGE, "ADCIN9", 0),
+	TWL4030_ADC_CHANNEL(10, IIO_CURRENT, "ADCIN10",
+		BIT(IIO_CHAN_INFO_PROCESSED)),
+	TWL4030_ADC_CHANNEL(11, IIO_VOLTAGE, "ADCIN11", 0),
+	TWL4030_ADC_CHANNEL(12, IIO_VOLTAGE, "ADCIN12", 0),
+	TWL4030_ADC_CHANNEL(13, IIO_VOLTAGE, "ADCIN13", 0),
+	TWL4030_ADC_CHANNEL(14, IIO_VOLTAGE, "ADCIN14", 0),
+	TWL4030_ADC_CHANNEL(15, IIO_VOLTAGE, "ADCIN15", 0),
+};
+
 static struct twl4030_madc_data *twl4030_madc;
 
 struct twl4030_prescale_divider_ratios {
@@ -702,28 +768,51 @@ static int twl4030_madc_probe(struct platform_device *pdev)
 {
 	struct twl4030_madc_data *madc;
 	struct twl4030_madc_platform_data *pdata = dev_get_platdata(&pdev->dev);
+	struct device_node *np = pdev->dev.of_node;
 	int irq, ret;
 	u8 regval;
+	struct iio_dev *iio_dev = NULL;
 
-	if (!pdata) {
+	if (!pdata && !np) {
 		dev_err(&pdev->dev, "platform_data not available\n");
 		return -EINVAL;
 	}
-	madc = devm_kzalloc(&pdev->dev, sizeof(*madc), GFP_KERNEL);
-	if (!madc)
+
+	iio_dev = devm_iio_device_alloc(&pdev->dev,
+					sizeof(struct twl4030_madc_data));
+	if (!iio_dev) {
+		dev_err(&pdev->dev, "failed allocating iio device\n");
 		return -ENOMEM;
+	}
 
+	madc = iio_priv(iio_dev);
 	madc->dev = &pdev->dev;
+	madc->use_second_irq = false;
+
+	iio_dev->name = dev_name(&pdev->dev);
+	iio_dev->dev.parent = &pdev->dev;
+	iio_dev->dev.of_node = pdev->dev.of_node;
+	iio_dev->info = &twl4030_madc_iio_info;
+	iio_dev->modes = INDIO_DIRECT_MODE;
+	iio_dev->channels = twl4030_madc_iio_channels;
+	iio_dev->num_channels = 16;
 
 	/*
 	 * Phoenix provides 2 interrupt lines. The first one is connected to
 	 * the OMAP. The other one can be connected to the other processor such
 	 * as modem. Hence two separate ISR and IMR registers.
 	 */
-	madc->imr = (pdata->irq_line == 1) ?
-	    TWL4030_MADC_IMR1 : TWL4030_MADC_IMR2;
-	madc->isr = (pdata->irq_line == 1) ?
-	    TWL4030_MADC_ISR1 : TWL4030_MADC_ISR2;
+	if (pdata && pdata->irq_line != 1)
+		madc->use_second_irq = true;
+	else
+		madc->use_second_irq = of_property_read_bool(np,
+				       "ti,system-uses-second-madc-irq");
+
+	madc->imr = madc->use_second_irq ? TWL4030_MADC_IMR2 :
+					   TWL4030_MADC_IMR1;
+	madc->isr = madc->use_second_irq ? TWL4030_MADC_ISR2 :
+					   TWL4030_MADC_ISR1;
+
 	ret = twl4030_madc_set_power(madc, 1);
 	if (ret < 0)
 		return ret;
@@ -768,7 +857,7 @@ static int twl4030_madc_probe(struct platform_device *pdev)
 		}
 	}
 
-	platform_set_drvdata(pdev, madc);
+	platform_set_drvdata(pdev, iio_dev);
 	mutex_init(&madc->lock);
 
 	irq = platform_get_irq(pdev, 0);
@@ -780,7 +869,15 @@ static int twl4030_madc_probe(struct platform_device *pdev)
 		goto err_i2c;
 	}
 	twl4030_madc = madc;
+
+	ret = iio_device_register(iio_dev);
+	if (ret) {
+		dev_dbg(&pdev->dev, "could not register iio device\n");
+		goto err_i2c;
+	}
+
 	return 0;
+
 err_i2c:
 	twl4030_madc_set_current_generator(madc, 0, 0);
 err_current_generator:
@@ -790,20 +887,32 @@ err_current_generator:
 
 static int twl4030_madc_remove(struct platform_device *pdev)
 {
-	struct twl4030_madc_data *madc = platform_get_drvdata(pdev);
+	struct iio_dev *iio_dev = platform_get_drvdata(pdev);
+	struct twl4030_madc_data *madc = iio_priv(iio_dev);
 
 	twl4030_madc_set_current_generator(madc, 0, 0);
 	twl4030_madc_set_power(madc, 0);
 
+	iio_device_unregister(iio_dev);
+
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id twl_madc_of_match[] = {
+	{.compatible = "ti,twl4030-madc", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, twl_madc_of_match);
+#endif
+
 static struct platform_driver twl4030_madc_driver = {
 	.probe = twl4030_madc_probe,
 	.remove = twl4030_madc_remove,
 	.driver = {
 		   .name = "twl4030_madc",
 		   .owner = THIS_MODULE,
+		   .of_match_table = of_match_ptr(twl_madc_of_match),
 		   },
 };
 
-- 
1.8.5.3

^ permalink raw reply related

* [RFCv4 3/7] mfd: twl4030-madc: Cleanup driver
From: Sebastian Reichel @ 2014-02-26 20:03 UTC (permalink / raw)
  To: Sebastian Reichel, Marek Belisko, Jonathan Cameron
  Cc: Lee Jones, Samuel Ortiz, Lars-Peter Clausen, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
	linux-kernel, devicetree, linux-iio, Sebastian Reichel
In-Reply-To: <1393444990-28140-1-git-send-email-sre@debian.org>

Some style fixes in twl4030-madc driver.

Reported-by: Jonathan Cameron <jic23@kernel.org>
Signed-off-by: Sebastian Reichel <sre@debian.org>
---
 drivers/mfd/twl4030-madc.c       | 106 ++++++++++++++++++---------------------
 include/linux/i2c/twl4030-madc.h |   2 +-
 2 files changed, 51 insertions(+), 57 deletions(-)

diff --git a/drivers/mfd/twl4030-madc.c b/drivers/mfd/twl4030-madc.c
index 37cb3ad..c90013e 100644
--- a/drivers/mfd/twl4030-madc.c
+++ b/drivers/mfd/twl4030-madc.c
@@ -49,7 +49,7 @@
 
 #include <linux/iio/iio.h>
 
-/*
+/**
  * struct twl4030_madc_data - a container for madc info
  * @dev - pointer to device structure for madc
  * @lock - mutex protecting this data structure
@@ -63,8 +63,8 @@ struct twl4030_madc_data {
 	struct mutex lock;	/* mutex protecting this data structure */
 	struct twl4030_madc_request requests[TWL4030_MADC_NUM_METHODS];
 	bool use_second_irq;
-	int imr;
-	int isr;
+	u8 imr;
+	u8 isr;
 };
 
 static int twl4030_madc_read(struct iio_dev *iio_dev,
@@ -157,17 +157,16 @@ twl4030_divider_ratios[16] = {
 };
 
 
-/*
- * Conversion table from -3 to 55 degree Celcius
- */
-static int therm_tbl[] = {
-30800,	29500,	28300,	27100,
-26000,	24900,	23900,	22900,	22000,	21100,	20300,	19400,	18700,	17900,
-17200,	16500,	15900,	15300,	14700,	14100,	13600,	13100,	12600,	12100,
-11600,	11200,	10800,	10400,	10000,	9630,	9280,	8950,	8620,	8310,
-8020,	7730,	7460,	7200,	6950,	6710,	6470,	6250,	6040,	5830,
-5640,	5450,	5260,	5090,	4920,	4760,	4600,	4450,	4310,	4170,
-4040,	3910,	3790,	3670,	3550
+/* Conversion table from -3 to 55 degrees Celcius */
+static int twl4030_therm_tbl[] = {
+	30800,	29500,	28300,	27100,
+	26000,	24900,	23900,	22900,	22000,	21100,	20300,	19400,	18700,
+	17900,	17200,	16500,	15900,	15300,	14700,	14100,	13600,	13100,
+	12600,	12100,	11600,	11200,	10800,	10400,	10000,	9630,	9280,
+	8950,	8620,	8310,	8020,	7730,	7460,	7200,	6950,	6710,
+	6470,	6250,	6040,	5830,	5640,	5450,	5260,	5090,	4920,
+	4760,	4600,	4450,	4310,	4170,	4040,	3910,	3790,	3670,
+	3550
 };
 
 /*
@@ -199,7 +198,7 @@ const struct twl4030_madc_conversion_method twl4030_conversion_methods[] = {
 			      },
 };
 
-/*
+/**
  * Function to read a particular channel value.
  * @madc - pointer to struct twl4030_madc_data
  * @reg - lsb of ADC Channel
@@ -229,7 +228,7 @@ static int twl4030_madc_channel_raw_read(struct twl4030_madc_data *madc, u8 reg)
 }
 
 /*
- * Return battery temperature
+ * Return battery temperature in degrees Celsius
  * Or < 0 on failure.
  */
 static int twl4030battery_temperature(int raw_volt)
@@ -238,18 +237,18 @@ static int twl4030battery_temperature(int raw_volt)
 	int temp, curr, volt, res, ret;
 
 	volt = (raw_volt * TEMP_STEP_SIZE) / TEMP_PSR_R;
-	/* Getting and calculating the supply current in micro ampers */
+	/* Getting and calculating the supply current in micro amperes */
 	ret = twl_i2c_read_u8(TWL_MODULE_MAIN_CHARGE, &val,
 		REG_BCICTL2);
 	if (ret < 0)
 		return ret;
+
 	curr = ((val & TWL4030_BCI_ITHEN) + 1) * 10;
 	/* Getting and calculating the thermistor resistance in ohms */
 	res = volt * 1000 / curr;
 	/* calculating temperature */
 	for (temp = 58; temp >= 0; temp--) {
-		int actual = therm_tbl[temp];
-
+		int actual = twl4030_therm_tbl[temp];
 		if ((actual - res) >= 0)
 			break;
 	}
@@ -271,11 +270,12 @@ static int twl4030battery_current(int raw_volt)
 	else /* slope of 0.88 mV/mA */
 		return (raw_volt * CURR_STEP_SIZE) / CURR_PSR_R2;
 }
+
 /*
  * Function to read channel values
  * @madc - pointer to twl4030_madc_data struct
  * @reg_base - Base address of the first channel
- * @Channels - 16 bit bitmap. If the bit is set, channel value is read
+ * @Channels - 16 bit bitmap. If the bit is set, channel's value is read
  * @buf - The channel values are stored here. if read fails error
  * @raw - Return raw values without conversion
  * value is stored
@@ -286,17 +286,17 @@ static int twl4030_madc_read_channels(struct twl4030_madc_data *madc,
 				      long channels, int *buf,
 				      bool raw)
 {
-	int count = 0, count_req = 0, i;
+	int count = 0;
+	int i;
 	u8 reg;
 
 	for_each_set_bit(i, &channels, TWL4030_MADC_MAX_CHANNELS) {
-		reg = reg_base + 2 * i;
+		u8 reg = reg_base + (2 * i);
 		buf[i] = twl4030_madc_channel_raw_read(madc, reg);
 		if (buf[i] < 0) {
-			dev_err(madc->dev,
-				"Unable to read register 0x%X\n", reg);
-			count_req++;
-			continue;
+			dev_err(madc->dev, "Unable to read register 0x%X\n",
+				reg);
+			return buf[i];
 		}
 		if (raw) {
 			count++;
@@ -307,7 +307,7 @@ static int twl4030_madc_read_channels(struct twl4030_madc_data *madc,
 			buf[i] = twl4030battery_current(buf[i]);
 			if (buf[i] < 0) {
 				dev_err(madc->dev, "err reading current\n");
-				count_req++;
+				return buf[i];
 			} else {
 				count++;
 				buf[i] = buf[i] - 750;
@@ -317,7 +317,7 @@ static int twl4030_madc_read_channels(struct twl4030_madc_data *madc,
 			buf[i] = twl4030battery_temperature(buf[i]);
 			if (buf[i] < 0) {
 				dev_err(madc->dev, "err reading temperature\n");
-				count_req++;
+				return buf[i];
 			} else {
 				buf[i] -= 3;
 				count++;
@@ -338,8 +338,6 @@ static int twl4030_madc_read_channels(struct twl4030_madc_data *madc,
 				twl4030_divider_ratios[i].numerator);
 		}
 	}
-	if (count_req)
-		dev_err(madc->dev, "%d channel conversion failed\n", count_req);
 
 	return count;
 }
@@ -363,13 +361,13 @@ static int twl4030_madc_enable_irq(struct twl4030_madc_data *madc, u8 id)
 			madc->imr);
 		return ret;
 	}
+
 	val &= ~(1 << id);
 	ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, val, madc->imr);
 	if (ret) {
 		dev_err(madc->dev,
 			"unable to write imr register 0x%X\n", madc->imr);
 		return ret;
-
 	}
 
 	return 0;
@@ -432,7 +430,7 @@ static irqreturn_t twl4030_madc_threaded_irq_handler(int irq, void *_madc)
 			continue;
 		ret = twl4030_madc_disable_irq(madc, i);
 		if (ret < 0)
-			dev_dbg(madc->dev, "Disable interrupt failed%d\n", i);
+			dev_dbg(madc->dev, "Disable interrupt failed %d\n", i);
 		madc->requests[i].result_pending = 1;
 	}
 	for (i = 0; i < TWL4030_MADC_NUM_METHODS; i++) {
@@ -514,21 +512,17 @@ static int twl4030_madc_start_conversion(struct twl4030_madc_data *madc,
 {
 	const struct twl4030_madc_conversion_method *method;
 	int ret = 0;
+
+	if (conv_method != TWL4030_MADC_SW1 && conv_method != TWL4030_MADC_SW2)
+		return -ENOTSUPP;
+
 	method = &twl4030_conversion_methods[conv_method];
-	switch (conv_method) {
-	case TWL4030_MADC_SW1:
-	case TWL4030_MADC_SW2:
-		ret = twl_i2c_write_u8(TWL4030_MODULE_MADC,
-				       TWL4030_MADC_SW_START, method->ctrl);
-		if (ret) {
-			dev_err(madc->dev,
-				"unable to write ctrl register 0x%X\n",
-				method->ctrl);
-			return ret;
-		}
-		break;
-	default:
-		break;
+	ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, TWL4030_MADC_SW_START,
+			       method->ctrl);
+	if (ret) {
+		dev_err(madc->dev, "unable to write ctrl register 0x%X\n",
+			method->ctrl);
+		return ret;
 	}
 
 	return 0;
@@ -625,8 +619,8 @@ int twl4030_madc_conversion(struct twl4030_madc_request *req)
 				       ch_lsb, method->avg);
 		if (ret) {
 			dev_err(twl4030_madc->dev,
-				"unable to write sel reg 0x%X\n",
-				method->sel + 1);
+				"unable to write avg reg 0x%X\n",
+				method->avg);
 			goto out;
 		}
 	}
@@ -667,10 +661,6 @@ out:
 }
 EXPORT_SYMBOL_GPL(twl4030_madc_conversion);
 
-/*
- * Return channel value
- * Or < 0 on failure.
- */
 int twl4030_get_madc_conversion(int channel_no)
 {
 	struct twl4030_madc_request req;
@@ -705,6 +695,7 @@ static int twl4030_madc_set_current_generator(struct twl4030_madc_data *madc,
 					      int chan, int on)
 {
 	int ret;
+	int regmask;
 	u8 regval;
 
 	ret = twl_i2c_read_u8(TWL_MODULE_MAIN_CHARGE,
@@ -714,10 +705,13 @@ static int twl4030_madc_set_current_generator(struct twl4030_madc_data *madc,
 			TWL4030_BCI_BCICTL1);
 		return ret;
 	}
+
+	regmask = chan ? TWL4030_BCI_ITHEN : TWL4030_BCI_TYPEN;
 	if (on)
-		regval |= chan ? TWL4030_BCI_ITHEN : TWL4030_BCI_TYPEN;
+		regval |= regmask;
 	else
-		regval &= chan ? ~TWL4030_BCI_ITHEN : ~TWL4030_BCI_TYPEN;
+		regval &= ~regmask;
+
 	ret = twl_i2c_write_u8(TWL_MODULE_MAIN_CHARGE,
 			       regval, TWL4030_BCI_BCICTL1);
 	if (ret) {
@@ -732,7 +726,7 @@ static int twl4030_madc_set_current_generator(struct twl4030_madc_data *madc,
 /*
  * Function that sets MADC software power on bit to enable MADC
  * @madc - pointer to twl4030_madc_data struct
- * @on - Enable or disable MADC software powen on bit.
+ * @on - Enable or disable MADC software power on bit.
  * returns error if i2c read/write fails else 0
  */
 static int twl4030_madc_set_power(struct twl4030_madc_data *madc, int on)
@@ -795,7 +789,7 @@ static int twl4030_madc_probe(struct platform_device *pdev)
 	iio_dev->info = &twl4030_madc_iio_info;
 	iio_dev->modes = INDIO_DIRECT_MODE;
 	iio_dev->channels = twl4030_madc_iio_channels;
-	iio_dev->num_channels = 16;
+	iio_dev->num_channels = ARRAY_SIZE(twl4030_madc_iio_channels);
 
 	/*
 	 * Phoenix provides 2 interrupt lines. The first one is connected to
diff --git a/include/linux/i2c/twl4030-madc.h b/include/linux/i2c/twl4030-madc.h
index 01f5951..1c0134d 100644
--- a/include/linux/i2c/twl4030-madc.h
+++ b/include/linux/i2c/twl4030-madc.h
@@ -44,7 +44,7 @@ struct twl4030_madc_conversion_method {
 
 struct twl4030_madc_request {
 	unsigned long channels;
-	u16 do_avg;
+	bool do_avg;
 	u16 method;
 	u16 type;
 	bool active;
-- 
1.8.5.3

^ permalink raw reply related

* [RFCv4 4/7] mfd: twl-core: Add twl_i2c_read/write_u16
From: Sebastian Reichel @ 2014-02-26 20:03 UTC (permalink / raw)
  To: Sebastian Reichel, Marek Belisko, Jonathan Cameron
  Cc: Lee Jones, Samuel Ortiz, Lars-Peter Clausen, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
	linux-kernel, devicetree, linux-iio, Sebastian Reichel
In-Reply-To: <1393444990-28140-1-git-send-email-sre@debian.org>

Add a simple twl_i2c_read/write_u16 wrapper over
the twl_i2c_read/write, which is similar to the
twl_i2c_read/write_u8 wrapper.

Signed-off-by: Sebastian Reichel <sre@debian.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 include/linux/i2c/twl.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
index ade1c06..d2b1670 100644
--- a/include/linux/i2c/twl.h
+++ b/include/linux/i2c/twl.h
@@ -195,6 +195,18 @@ static inline int twl_i2c_read_u8(u8 mod_no, u8 *val, u8 reg) {
 	return twl_i2c_read(mod_no, val, reg, 1);
 }
 
+static inline int twl_i2c_write_u16(u8 mod_no, u16 val, u8 reg) {
+	val = cpu_to_le16(val);
+	return twl_i2c_write(mod_no, (u8*) &val, reg, 2);
+}
+
+static inline int twl_i2c_read_u16(u8 mod_no, u16 *val, u8 reg) {
+	int ret;
+	ret = twl_i2c_read(mod_no, (u8*) val, reg, 2);
+	*val = le16_to_cpu(*val);
+	return ret;
+}
+
 int twl_get_type(void);
 int twl_get_version(void);
 int twl_get_hfclk_rate(void);
-- 
1.8.5.3

^ permalink raw reply related

* [RFCv4 5/7] mfd: twl4030-madc: Use twl_i2c_read/write_u16 for 16 bit registers
From: Sebastian Reichel @ 2014-02-26 20:03 UTC (permalink / raw)
  To: Sebastian Reichel, Marek Belisko, Jonathan Cameron
  Cc: Lee Jones, Samuel Ortiz, Lars-Peter Clausen, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
	linux-kernel, devicetree, linux-iio, Sebastian Reichel
In-Reply-To: <1393444990-28140-1-git-send-email-sre@debian.org>

Simplify reading and writing of 16 bit TWL registers in the
driver by using twl_i2c_read_u16 and twl_i2c_write_u16.

Signed-off-by: Sebastian Reichel <sre@debian.org>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mfd/twl4030-madc.c | 39 ++++++++-------------------------------
 1 file changed, 8 insertions(+), 31 deletions(-)

diff --git a/drivers/mfd/twl4030-madc.c b/drivers/mfd/twl4030-madc.c
index c90013e..d918670 100644
--- a/drivers/mfd/twl4030-madc.c
+++ b/drivers/mfd/twl4030-madc.c
@@ -206,25 +206,19 @@ const struct twl4030_madc_conversion_method twl4030_conversion_methods[] = {
  */
 static int twl4030_madc_channel_raw_read(struct twl4030_madc_data *madc, u8 reg)
 {
-	u8 msb, lsb;
+	u16 val;
 	int ret;
 	/*
 	 * For each ADC channel, we have MSB and LSB register pair. MSB address
 	 * is always LSB address+1. reg parameter is the address of LSB register
 	 */
-	ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &msb, reg + 1);
+	ret = twl_i2c_read_u16(TWL4030_MODULE_MADC, &val, reg);
 	if (ret) {
-		dev_err(madc->dev, "unable to read MSB register 0x%X\n",
-			reg + 1);
-		return ret;
-	}
-	ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, &lsb, reg);
-	if (ret) {
-		dev_err(madc->dev, "unable to read LSB register 0x%X\n", reg);
+		dev_err(madc->dev, "unable to read register 0x%X\n", reg);
 		return ret;
 	}
 
-	return (int)(((msb << 8) | lsb) >> 6);
+	return (int)(val >> 6);
 }
 
 /*
@@ -573,7 +567,6 @@ static int twl4030_madc_wait_conversion_ready(struct twl4030_madc_data *madc,
 int twl4030_madc_conversion(struct twl4030_madc_request *req)
 {
 	const struct twl4030_madc_conversion_method *method;
-	u8 ch_msb, ch_lsb;
 	int ret;
 
 	if (!req || !twl4030_madc)
@@ -589,37 +582,21 @@ int twl4030_madc_conversion(struct twl4030_madc_request *req)
 		ret = -EBUSY;
 		goto out;
 	}
-	ch_msb = (req->channels >> 8) & 0xff;
-	ch_lsb = req->channels & 0xff;
 	method = &twl4030_conversion_methods[req->method];
 	/* Select channels to be converted */
-	ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, ch_msb, method->sel + 1);
+	ret = twl_i2c_write_u16(TWL4030_MODULE_MADC, req->channels, method->sel);
 	if (ret) {
 		dev_err(twl4030_madc->dev,
-			"unable to write sel register 0x%X\n", method->sel + 1);
-		goto out;
-	}
-	ret = twl_i2c_write_u8(TWL4030_MODULE_MADC, ch_lsb, method->sel);
-	if (ret) {
-		dev_err(twl4030_madc->dev,
-			"unable to write sel register 0x%X\n", method->sel + 1);
+			"unable to write sel register 0x%X\n", method->sel);
 		goto out;
 	}
 	/* Select averaging for all channels if do_avg is set */
 	if (req->do_avg) {
-		ret = twl_i2c_write_u8(TWL4030_MODULE_MADC,
-				       ch_msb, method->avg + 1);
+		ret = twl_i2c_write_u16(TWL4030_MODULE_MADC, req->channels,
+				       method->avg);
 		if (ret) {
 			dev_err(twl4030_madc->dev,
 				"unable to write avg register 0x%X\n",
-				method->avg + 1);
-			goto out;
-		}
-		ret = twl_i2c_write_u8(TWL4030_MODULE_MADC,
-				       ch_lsb, method->avg);
-		if (ret) {
-			dev_err(twl4030_madc->dev,
-				"unable to write avg reg 0x%X\n",
 				method->avg);
 			goto out;
 		}
-- 
1.8.5.3

^ permalink raw reply related

* [RFCv4 6/7] Documentation: DT: Document twl4030-madc binding
From: Sebastian Reichel @ 2014-02-26 20:03 UTC (permalink / raw)
  To: Sebastian Reichel, Marek Belisko, Jonathan Cameron
  Cc: Lee Jones, Samuel Ortiz, Lars-Peter Clausen, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
	linux-kernel, devicetree, linux-iio, Sebastian Reichel
In-Reply-To: <1393444990-28140-1-git-send-email-sre@debian.org>

Add devicetree binding documentation for twl4030-madc
analog digital converter.

Signed-off-by: Sebastian Reichel <sre@debian.org>
---
 .../devicetree/bindings/iio/adc/twl4030-madc.txt   | 24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/twl4030-madc.txt

diff --git a/Documentation/devicetree/bindings/iio/adc/twl4030-madc.txt b/Documentation/devicetree/bindings/iio/adc/twl4030-madc.txt
new file mode 100644
index 0000000..6bdd214
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/twl4030-madc.txt
@@ -0,0 +1,24 @@
+* TWL4030 Monitoring Analog to Digital Converter (MADC)
+
+The MADC subsystem in the TWL4030 consists of a 10-bit ADC
+combined with a 16-input analog multiplexer.
+
+Required properties:
+  - compatible: Should contain "ti,twl4030-madc".
+  - interrupts: IRQ line for the MADC submodule.
+  - #io-channel-cells: Should be set to <1>.
+
+Optional properties:
+  - ti,system-uses-second-madc-irq: boolean, set if the second madc irq register
+				    should be used, which is intended to be used
+				    by Co-Processors (e.g. a modem).
+
+Example:
+
+&twl {
+	madc {
+		compatible = "ti,twl4030-madc";
+		interrupts = <3>;
+		#io-channel-cells = <1>;
+	};
+};
-- 
1.8.5.3

^ permalink raw reply related

* [RFCv4 7/7] mfd: twl4030-madc: Move driver to drivers/iio/adc
From: Sebastian Reichel @ 2014-02-26 20:03 UTC (permalink / raw)
  To: Sebastian Reichel, Marek Belisko, Jonathan Cameron
  Cc: Lee Jones, Samuel Ortiz, Lars-Peter Clausen, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Grant Likely,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA, Sebastian Reichel
In-Reply-To: <1393444990-28140-1-git-send-email-sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>

This is a driver for an A/D converter, which belongs into
drivers/iio/adc.

Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
Acked-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/iio/adc/Kconfig                 | 10 ++++++++++
 drivers/iio/adc/Makefile                |  1 +
 drivers/{mfd => iio/adc}/twl4030-madc.c |  0
 drivers/mfd/Kconfig                     | 10 ----------
 drivers/mfd/Makefile                    |  1 -
 5 files changed, 11 insertions(+), 11 deletions(-)
 rename drivers/{mfd => iio/adc}/twl4030-madc.c (100%)

diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 2209f28..427f75c 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -183,6 +183,16 @@ config TI_AM335X_ADC
 	  Say yes here to build support for Texas Instruments ADC
 	  driver which is also a MFD client.
 
+config TWL4030_MADC
+	tristate "TWL4030 MADC (Monitoring A/D Converter)"
+	depends on TWL4030_CORE
+	help
+	This driver provides support for Triton TWL4030-MADC. The
+	driver supports both RT and SW conversion methods.
+
+	This driver can also be built as a module. If so, the module will be
+	called twl4030-madc.
+
 config TWL6030_GPADC
 	tristate "TWL6030 GPADC (General Purpose A/D Converter) Support"
 	depends on TWL4030_CORE
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index ba9a10a..9acf2df 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -20,5 +20,6 @@ obj-$(CONFIG_MCP3422) += mcp3422.o
 obj-$(CONFIG_NAU7802) += nau7802.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
 obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
+obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
 obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
 obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
diff --git a/drivers/mfd/twl4030-madc.c b/drivers/iio/adc/twl4030-madc.c
similarity index 100%
rename from drivers/mfd/twl4030-madc.c
rename to drivers/iio/adc/twl4030-madc.c
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 49bb445..23a8a51 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -935,16 +935,6 @@ config TWL4030_CORE
 	  high speed USB OTG transceiver, an audio codec (on most
 	  versions) and many other features.
 
-config TWL4030_MADC
-	tristate "TI TWL4030 MADC"
-	depends on TWL4030_CORE
-	help
-	This driver provides support for triton TWL4030-MADC. The
-	driver supports both RT and SW conversion methods.
-
-	This driver can be built as a module. If so it will be
-	named twl4030-madc
-
 config TWL4030_POWER
 	bool "TI TWL4030 power resources"
 	depends on TWL4030_CORE && ARM
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 5aea5ef..c8eb0bc 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -71,7 +71,6 @@ obj-$(CONFIG_MFD_TPS80031)	+= tps80031.o
 obj-$(CONFIG_MENELAUS)		+= menelaus.o
 
 obj-$(CONFIG_TWL4030_CORE)	+= twl-core.o twl4030-irq.o twl6030-irq.o
-obj-$(CONFIG_TWL4030_MADC)      += twl4030-madc.o
 obj-$(CONFIG_TWL4030_POWER)    += twl4030-power.o
 obj-$(CONFIG_MFD_TWL4030_AUDIO)	+= twl4030-audio.o
 obj-$(CONFIG_TWL6040_CORE)	+= twl6040.o
-- 
1.8.5.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings
From: Suman Anna @ 2014-02-26 20:23 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA, Tony Lindgren,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Rob Herring,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kumar Gala,
	linux-omap-u79uwXL29TY76Z2rM5mHXA, Florian Vaussard
In-Reply-To: <8258385.uITvkIdEvn@avalon>

Hi Laurent,

>
> On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
>> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
>>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
>>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
>>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
>>>>>> From: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>>
>>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
>>>>>> the standard bindings used by OMAP peripherals, this patch uses a
>>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
>>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
>>>>>>
>>>>>> Signed-off-by: Florian Vaussard <florian.vaussard-p8DiymsW2f8@public.gmane.org>
>>>>>> [s-anna-l0cyMroinI0@public.gmane.org: split bindings document, add dra7 and bus error back]
>>>>>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>>>>>> ---
>>>>>>
>>>>>>   .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 ++++++++++++++
>>>>>>   1 file changed, 28 insertions(+)
>>>>>>   create mode 100644
>>>>>>   Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file
>>>>>> mode 100644
>>>>>> index 0000000..116492d
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
>>>>>> @@ -0,0 +1,28 @@
>>>>>> +OMAP2+ IOMMU
>>>>>> +
>>>>>> +Required properties:
>>>>>> +- compatible : Should be one of,
>>>>>> +		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
>>>>>> +		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
>>>>>> +		"ti,dra7-iommu" for DRA7xx IOMMU instances
>>>>>> +- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
>>>>>> +- reg        : Address space for the configuration registers
>>>>>> +- interrupts : Interrupt specifier for the IOMMU instance
>>>>>> +- dma-window : IOVA start address and length
>>>>>
>>>>> Isn't the dma window more of a system configuration property than a
>>>>> hardware property ? How do you expect it to be set?
>>>>
>>>> We are setting it based on the addressable range for the MMU.
>>>
>>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
>>> support the full 4GB VA space. Why do you need to restrict it ?
>>
>> I should have rephrased it better when I said addressable range. While
>> the MMUs are capable of programming the full 4GB space, there are some
>> address ranges that are private from the processor view. This window is
>> currently used to set the range for the omap-iovmm driver (which only
>> OMAP3 ISP is using atm), and there is no point in allowing the
>> omap-iovmm driver the full range when the processor could never
>> reach/access those addresses.
>
> But the IOMMU VA space is from a device point of view, not from a CPU point of
> view. Could you point me to where those private ranges are documented, in
> order to understand the problem correctly ?

Yes, they are indeed from the device perspective. I meant DSP and/or IPU 
by processor.

For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP 
Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external 
addressable range starts from 0x11000000.

regards
Suman

>
>>>> We are reusing the existing defined property and it allows us to get rid
>>>> of the IOVA start and end addresses defined in the pre-DT OMAP iommu
>>>> platform data.
>>>>
>>>>>> +Optional properties:
>>>>>> +- ti,#tlb-entries : Number of entries in the translation look-aside
>>>>>> buffer. +                    Should be either 8 or 32 (default: 32)
>>>>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports
>>>>>> throwing
>>>>>> +		          back a bus error response on MMU faults.
>>>>>
>>>>> Do these features vary per IOMMU instance or per IOMMU model ? In the
>>>>> latter case they could be inferred from the compatible string by the
>>>>> driver without requiring them to be explicit in DT (whether you want to
>>>>> do so is left to you though).
>>>>
>>>> Well, these are fixed features given an IOMMU instance, like the OMAP3
>>>> ISP is the only one that has 8 TLB entries, all the remaining ones have
>>>> 32, and the IPU iommu instances are the only ones that support the bus
>>>> error response back. I have no preference to any particular way, and
>>>> sure the driver can infer these easily based on unique compatible
>>>> strings per subsystem per SoC. I just happened to go with defining
>>>> compatible strings per SoC, with the optional properties differentiating
>>>> the fixed behavior between different IOMMU instances on that SoC. This
>>>> is where I was looking for some inputs/guidance from the DT bindings
>>>> maintainers on what is the preferred method.
>>>
>>> I think you've made the right choice. I wasn't sure whether those
>>> parameters varied across IOMMU instances of compatible devices (from a
>>> compatible string point of view) or were constant. As they vary they
>>> should be expressed in DT.
>>
>> Yeah, I wasn't sure if these qualify as features (as per
>> Documentation/devicetree/bindings/ABI.txt section II.2).
>>
>> regards
>> Suman
>>
>>>>>> +Example:
>>>>>> +	/* OMAP3 ISP MMU */
>>>>>> +	mmu_isp: mmu@480bd400 {
>>>>>> +		compatible = "ti,omap2-iommu";
>>>>>> +		reg = <0x480bd400 0x80>;
>>>>>> +		interrupts = <24>;
>>>>>> +		ti,hwmods = "mmu_isp";
>>>>>> +		ti,#tlb-entries = <8>;
>>>>>> +		dma-window = <0 0xfffff000>;
>>>>>> +	};
>

^ permalink raw reply

* Re: [PATCHv2 03/16] Documentation: dt: add OMAP iommu bindings
From: Laurent Pinchart @ 2014-02-26 20:36 UTC (permalink / raw)
  To: Suman Anna
  Cc: Joerg Roedel, Tony Lindgren, Florian Vaussard, iommu, devicetree,
	linux-omap, linux-arm-kernel, Mark Rutland, Kumar Gala,
	Rob Herring
In-Reply-To: <530E4D27.4010205@ti.com>

Hi Suman,

On Wednesday 26 February 2014 14:23:03 Suman Anna wrote:
> > On Wednesday 26 February 2014 11:02:24 Suman Anna wrote:
> >> On 02/25/2014 08:13 PM, Laurent Pinchart wrote:
> >>> On Tuesday 25 February 2014 17:02:35 Suman Anna wrote:
> >>>> On 02/25/2014 03:26 PM, Laurent Pinchart wrote:
> >>>>> On Thursday 13 February 2014 12:15:34 Suman Anna wrote:
> >>>>>> From: Florian Vaussard <florian.vaussard@epfl.ch>
> >>>>>> 
> >>>>>> This patch adds the iommu bindings for all OMAP2+ SoCs. Apart from
> >>>>>> the standard bindings used by OMAP peripherals, this patch uses a
> >>>>>> 'dma-window' (already used by Tegra SMMU) and adds two OMAP custom
> >>>>>> bindings - 'ti,#tlb-entries' and 'ti,iommu-bus-err-back'.
> >>>>>> 
> >>>>>> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> >>>>>> [s-anna@ti.com: split bindings document, add dra7 and bus error back]
> >>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
> >>>>>> ---
> >>>>>> 
> >>>>>>  .../devicetree/bindings/iommu/ti,omap-iommu.txt    | 28 ++++++++++++
> >>>>>>  1 file changed, 28 insertions(+) 
> >>>>>>  create mode 100644
> >>>>>>  Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>>>> 
> >>>>>> diff --git
> >>>>>> a/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>>>> b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt new file
> >>>>>> mode 100644
> >>>>>> index 0000000..116492d
> >>>>>> --- /dev/null
> >>>>>> +++ b/Documentation/devicetree/bindings/iommu/ti,omap-iommu.txt
> >>>>>> @@ -0,0 +1,28 @@
> >>>>>> +OMAP2+ IOMMU
> >>>>>> +
> >>>>>> +Required properties:
> >>>>>> +- compatible : Should be one of,
> >>>>>> +		"ti,omap2-iommu" for OMAP2/OMAP3 IOMMU instances
> >>>>>> +		"ti,omap4-iommu" for OMAP4/OMAP5 IOMMU instances
> >>>>>> +		"ti,dra7-iommu" for DRA7xx IOMMU instances
> >>>>>> +- ti,hwmods  : Name of the hwmod associated with the IOMMU instance
> >>>>>> +- reg        : Address space for the configuration registers
> >>>>>> +- interrupts : Interrupt specifier for the IOMMU instance
> >>>>>> +- dma-window : IOVA start address and length
> >>>>> 
> >>>>> Isn't the dma window more of a system configuration property than a
> >>>>> hardware property ? How do you expect it to be set?
> >>>> 
> >>>> We are setting it based on the addressable range for the MMU.
> >>> 
> >>> A quick look at the ISP and IVA IOMMUs in the OMAP3 shows that both
> >>> support the full 4GB VA space. Why do you need to restrict it ?
> >> 
> >> I should have rephrased it better when I said addressable range. While
> >> the MMUs are capable of programming the full 4GB space, there are some
> >> address ranges that are private from the processor view. This window is
> >> currently used to set the range for the omap-iovmm driver (which only
> >> OMAP3 ISP is using atm), and there is no point in allowing the
> >> omap-iovmm driver the full range when the processor could never
> >> reach/access those addresses.
> > 
> > But the IOMMU VA space is from a device point of view, not from a CPU
> > point of view. Could you point me to where those private ranges are
> > documented, in order to understand the problem correctly ?
> 
> Yes, they are indeed from the device perspective. I meant DSP and/or IPU
> by processor.
> 
> For example on OMAP3, you can refer to Table 2-9 in section 2.4.5 "DSP
> Subsystem Memory Space Mapping" of the OMAP36xx TRM, and the external
> addressable range starts from 0x11000000.

OK, so it looks more like a property of the IOMMU master than a property of 
the IOMMU itself. It would be better to express it as such, but I wonder how 
that could be done, and if it would be worth it in this case.

As not all masters (the OMAP3 ISP doesn't for instance) have restrictions 
regarding the VA range they can address, should this property be at least made 
optional ?

> >>>> We are reusing the existing defined property and it allows us to get
> >>>> rid of the IOVA start and end addresses defined in the pre-DT OMAP
> >>>> iommu platform data.
> >>>> 
> >>>>>> +Optional properties:
> >>>>>> +- ti,#tlb-entries : Number of entries in the translation look-aside
> >>>>>> buffer. +                    Should be either 8 or 32 (default: 32)
> >>>>>> +- ti,iommu-bus-err-back : Indicates the IOMMU instance supports
> >>>>>> throwing
> >>>>>> +		          back a bus error response on MMU faults.
> >>>>> 
> >>>>> Do these features vary per IOMMU instance or per IOMMU model ? In the
> >>>>> latter case they could be inferred from the compatible string by the
> >>>>> driver without requiring them to be explicit in DT (whether you want
> >>>>> to do so is left to you though).
> >>>> 
> >>>> Well, these are fixed features given an IOMMU instance, like the OMAP3
> >>>> ISP is the only one that has 8 TLB entries, all the remaining ones have
> >>>> 32, and the IPU iommu instances are the only ones that support the bus
> >>>> error response back. I have no preference to any particular way, and
> >>>> sure the driver can infer these easily based on unique compatible
> >>>> strings per subsystem per SoC. I just happened to go with defining
> >>>> compatible strings per SoC, with the optional properties
> >>>> differentiating the fixed behavior between different IOMMU instances on
> >>>> that SoC. This is where I was looking for some inputs/guidance from the
> >>>> DT bindings maintainers on what is the preferred method.
> >>> 
> >>> I think you've made the right choice. I wasn't sure whether those
> >>> parameters varied across IOMMU instances of compatible devices (from a
> >>> compatible string point of view) or were constant. As they vary they
> >>> should be expressed in DT.
> >> 
> >> Yeah, I wasn't sure if these qualify as features (as per
> >> Documentation/devicetree/bindings/ABI.txt section II.2).
> >> 
> >> regards
> >> Suman
> >> 
> >>>>>> +Example:
> >>>>>> +	/* OMAP3 ISP MMU */
> >>>>>> +	mmu_isp: mmu@480bd400 {
> >>>>>> +		compatible = "ti,omap2-iommu";
> >>>>>> +		reg = <0x480bd400 0x80>;
> >>>>>> +		interrupts = <24>;
> >>>>>> +		ti,hwmods = "mmu_isp";
> >>>>>> +		ti,#tlb-entries = <8>;
> >>>>>> +		dma-window = <0 0xfffff000>;
> >>>>>> +	};

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH RESEND v10 3/4] PHY: add APM X-Gene SoC 15Gbps Multi-purpose PHY driver
From: Loc Ho @ 2014-02-26 20:45 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Tejun Heo, Olof Johansson, Arnd Bergmann,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	Don Dutile, Jon Masters, patches-qTEPVZfXA3Y@public.gmane.org,
	Tuan Phan, Suman Tripathi
In-Reply-To: <530DB713.6000701-l0cyMroinI0@public.gmane.org>

Hi,

>>
>> +config PHY_XGENE
>> +       tristate "APM X-Gene 15Gbps PHY support"
>> +       depends on ARM64 || COMPILE_TEST
>> +       select GENERIC_PHY
>
>
> depends on HAS_IOMEM and CONFIG_OF..

I will make it depends as "HAS_IOMEM && OF && (ARM64 || COMPILE_TEST)

>>
>> +/* PLL Clock Macro Unit (CMU) CSR accessing from SDS indirectly */
>> +#define CMU_REG0                       0x00000
>> +#define  CMU_REG0_PLL_REF_SEL_MASK     0x00002000
>> +#define  CMU_REG0_PLL_REF_SEL_SET(dst, src)    \
>> +               (((dst) & ~0x00002000) | (((u32)(src) << 0xd) & 0x00002000))
>
>
> using decimals for shift value would be better. No strong feeling though.

I will change to integer value.

>> +/*
>> + * For chip earlier than A3 version, enable this flag.
>> + * To enable, pass boot argument phy_xgene.preA3Chip=1
>> + */
>> +static int preA3Chip;
>> +MODULE_PARM_DESC(preA3Chip, "Enable pre-A3 chip support (1=enable 0=disable)");
>> +module_param_named(preA3Chip, preA3Chip, int, 0444);
>
>
> Do we need to have module param for this? I mean we can differentiate between
> different chip versions in dt data only.

This is only required for the short term. Once all the pre-A3 system
are replaced, there isn't an need for this. For those who still has an
pre-A3 silicon system, this would provide an short term solution for
them. DT isn't quite correct here. This is an global thing. I guess I
can OR all node. If it is still better to put in the DT, let me know
and I will move it.

>> +
>> +static void sds_wr(void __iomem *csr_base, u32 indirect_cmd_reg,
>> +                  u32 indirect_data_reg, u32 addr, u32 data)
>> +{
>> +       u32 val;
>> +       u32 cmd;
>> +
>> +       cmd = CFG_IND_WR_CMD_MASK | CFG_IND_CMD_DONE_MASK;
>> +       cmd = CFG_IND_ADDR_SET(cmd, addr);
>
>
> This looks hacky. If 'CFG_IND_WR_CMD_MASK | CFG_IND_CMD_DONE_MASK' should be set then it should be part of the second argument. From the macro 'CFG_IND_ADDR_SET' the first argument should be more like the current value present in the register right? I feel the macro (CFG_IND_ADDR_SET) is not used in the way it is intended to.

The macro XXX_SET is intended to update an field within the register.
The update field is returned. The first assignment lines are setting
another field. Those two lines can be written as:

cmd = 0;
cmd |= CFG_IND_WR_CMD_MASK;            ==> Set the CMD bit
cmd |= CFG_IND_CMD_DONE_MASK;        ==> Set the DONE bit
cmd = CFG_IND_ADDR_SET(cmd, addr);    ===> Set the field ADDR
(multiple bit) to value from addr

Is this clear?

>> +       ctx->dev = &pdev->dev;
>> +       platform_set_drvdata(pdev, ctx);
>> +
>> +       phy_provider = devm_of_phy_provider_register(ctx->dev,
>> +                                                    xgene_phy_xlate);
>> +       if (IS_ERR(phy_provider)) {
>> +               rc = PTR_ERR(phy_provider);
>> +               goto error;
>> +       }
>
>
> phy_provider_register should be the last step in registering the PHY with the framework.

I will move to after devm_phy_create. BTW, other driver is before.

-Loc
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v8 06/14] CLK: TI: OMAP3: Get rid of unused USB Host dummy clocks
From: Mike Turquette @ 2014-02-26 20:56 UTC (permalink / raw)
  To: Roger Quadros, tony, bcousson, lee.jones
  Cc: balbi, nm, khilman, linux-omap, linux-arm-kernel, linux-kernel,
	devicetree, linux-usb, Tero Kristo
In-Reply-To: <530C6323.90205@ti.com>

Quoting Roger Quadros (2014-02-25 01:32:19)
> Hi Mike,
> 
> On 02/25/2014 10:43 AM, Mike Turquette wrote:
> > Quoting Roger Quadros (2014-02-20 03:40:01)
> >> The OMAP USB Host MFD driver no longer expects these non-existing
> >> clocks from the OMAP3 platform, so get rid of them.
> > 
> > Looks good to me.
> 
> Is it OK if I squash this patch with [1] and take it through the MFD tree?
> Keeping them separate could break functionality if both don't go in together.

Acked-by: Mike Turquette <mturquette@linaro.org>

> 
> [1] - http://article.gmane.org/gmane.linux.ports.arm.kernel/303266
> 
> cheers,
> -roger
> 
> > 
> >>
> >> CC: Tero Kristo <t-kristo@ti.com>
> >> CC: Mike Turquette <mturquette@linaro.org>
> >> Signed-off-by: Roger Quadros <rogerq@ti.com>
> >> ---
> >>  arch/arm/mach-omap2/cclock3xxx_data.c | 4 ----
> >>  drivers/clk/ti/clk-3xxx.c             | 4 ----
> >>  2 files changed, 8 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/cclock3xxx_data.c b/arch/arm/mach-omap2/cclock3xxx_data.c
> >> index 3b05aea..4299a55 100644
> >> --- a/arch/arm/mach-omap2/cclock3xxx_data.c
> >> +++ b/arch/arm/mach-omap2/cclock3xxx_data.c
> >> @@ -3495,10 +3495,6 @@ static struct omap_clk omap3xxx_clks[] = {
> >>         CLK(NULL,       "dss_tv_fck",   &dss_tv_fck),
> >>         CLK(NULL,       "dss_96m_fck",  &dss_96m_fck),
> >>         CLK(NULL,       "dss2_alwon_fck",       &dss2_alwon_fck),
> >> -       CLK(NULL,       "utmi_p1_gfclk",        &dummy_ck),
> >> -       CLK(NULL,       "utmi_p2_gfclk",        &dummy_ck),
> >> -       CLK(NULL,       "xclk60mhsp1_ck",       &dummy_ck),
> >> -       CLK(NULL,       "xclk60mhsp2_ck",       &dummy_ck),
> >>         CLK(NULL,       "init_60m_fclk",        &dummy_ck),
> >>         CLK(NULL,       "gpt1_fck",     &gpt1_fck),
> >>         CLK(NULL,       "aes2_ick",     &aes2_ick),
> >> diff --git a/drivers/clk/ti/clk-3xxx.c b/drivers/clk/ti/clk-3xxx.c
> >> index d323023..0d1750a 100644
> >> --- a/drivers/clk/ti/clk-3xxx.c
> >> +++ b/drivers/clk/ti/clk-3xxx.c
> >> @@ -130,10 +130,6 @@ static struct ti_dt_clk omap3xxx_clks[] = {
> >>         DT_CLK(NULL, "dss_tv_fck", "dss_tv_fck"),
> >>         DT_CLK(NULL, "dss_96m_fck", "dss_96m_fck"),
> >>         DT_CLK(NULL, "dss2_alwon_fck", "dss2_alwon_fck"),
> >> -       DT_CLK(NULL, "utmi_p1_gfclk", "dummy_ck"),
> >> -       DT_CLK(NULL, "utmi_p2_gfclk", "dummy_ck"),
> >> -       DT_CLK(NULL, "xclk60mhsp1_ck", "dummy_ck"),
> >> -       DT_CLK(NULL, "xclk60mhsp2_ck", "dummy_ck"),
> >>         DT_CLK(NULL, "init_60m_fclk", "dummy_ck"),
> >>         DT_CLK(NULL, "gpt1_fck", "gpt1_fck"),
> >>         DT_CLK(NULL, "aes2_ick", "aes2_ick"),
> >> -- 
> >> 1.8.3.2
> >>
> 

^ permalink raw reply

* Re: [PATCH 3/4] ARM: dts: OMAP4: Add IOMMU nodes
From: Laurent Pinchart @ 2014-02-26 21:05 UTC (permalink / raw)
  To: Suman Anna
  Cc: Benoit Cousson, Tony Lindgren, Florian Vaussard, devicetree,
	iommu, linux-omap, linux-arm-kernel
In-Reply-To: <1392315776-33197-4-git-send-email-s-anna@ti.com>

Hi Suman,

Thank you for the patch.

On Thursday 13 February 2014 12:22:55 Suman Anna wrote:
> From: Florian Vaussard <florian.vaussard@epfl.ch>
> 
> Add the IOMMU nodes for the DSP and IPU subsystems. The external
> address space for DSP starts at 0x20000000 in OMAP4 compared to
> 0x11000000 in OMAP3, and the addresses beyond 0xE0000000 are
> private address space for the Cortex-M3 cores in the IPU subsystem.
> The MMU within the IPU sub-system also supports a bus error back
> capability, not available on the DSP MMU.
> 
> Signed-off-by: Florian Vaussard <florian.vaussard@epfl.ch>
> [s-anna@ti.com: dma-window updates and bus error back addition]
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
>  arch/arm/boot/dts/omap4.dtsi | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index d3f8a6e..1885f90 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -461,6 +461,23 @@
>  			dma-names = "tx", "rx";
>  		};
> 
> +		mmu_dsp: mmu@4a066000 {
> +			compatible = "ti,omap4-iommu";
> +			reg = <0x4a066000 0xff>;
> +			interrupts = <GIC_SPI 28 IRQ_TYPE_LEVEL_HIGH>;
> +			ti,hwmods = "mmu_dsp";
> +			dma-window = <0x20000000 0xdffff000>;
> +		};
> +
> +		mmu_ipu: mmu@55082000 {
> +			compatible = "ti,omap4-iommu";
> +			reg = <0x55082000 0xff>;
> +			interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
> +			ti,hwmods = "mmu_ipu";
> +			dma-window = <0 0xdffff000>;

I'm not too familiar with the M3 MPU in the OMAP4, but doesn't its memory map 
also include other reserved regions, such as 0x55040000- 0x5505ffff to access 
the ISS ?

> +			ti,iommu-bus-err-back;
> +		};
> +
>  		wdt2: wdt@4a314000 {
>  			compatible = "ti,omap4-wdt", "ti,omap3-wdt";
>  			reg = <0x4a314000 0x80>;

-- 
Regards,

Laurent Pinchart


^ permalink raw reply

* Re: [PATCH] ARM: tegra: add device tree for SHIELD
From: Arend van Spriel @ 2014-02-26 21:10 UTC (permalink / raw)
  To: Alexandre Courbot, Stephen Warren, Thierry Reding, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Russell King
  Cc: devicetree@vger.kernel.org, linux-tegra@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <530D731B.5050305@nvidia.com>

On 02/26/2014 05:52 AM, Alexandre Courbot wrote:
> On 02/25/2014 06:52 PM, Arend van Spriel wrote:
>> On 02/25/2014 03:13 AM, Alexandre Courbot wrote:
>>>>
>>>>> +    /* Wifi */
>>>>> +    sdhci@78000000 {
>>>>> +        status = "okay";
>>>>> +        bus-width = <4>;
>>>>> +        broken-cd;
>>>>> +        keep-power-in-suspend;
>>>>> +        cap-sdio-irq;
>>>>
>>>> Is non-removable better than broken-cd, or are they entirely unrelated?
>>>
>>> They are unrelated actually. With non-removable the driver expects the
>>> device to always be there since boot, and does not check for the card to
>>> be removed/added after boot. broken-cd indicates there is no CD line and
>>> the device should be polled regularly.
>>>
>>> For the Wifi chip, non-removable would be the correct setting
>>> hardware-wise, but there is a trap: the chip has its reset line asserted
>>> at boot-time, and you need to set GPIO 229 to de-assert it. Only after
>>> that will the device be detected on the SDIO bus. Since it lacks a CD
>>> line, it must be polled, hence the broken-cd property.
>>>
>>> This also raises another, redundant problem with DT bindings: AFAIK we
>>> currently have no way to let the system know the device will only appear
>>> after a given GPIO is set. It would also be nice to be able to give some
>>> parameters to the Wifi driver through the DT (like the OOB interrupt).
>>> Right now the Wifi chip is brought up by exporting the GPIO and writing
>>> to it from user-space, and the OOB interrupt is not used.
>>
>> Hi Alexandre,
>>
>> I recently posted a proposal for brcmfmac DT binding [1]. I did receive
>> some comments, but it would be great if you (and/or others involved) had
>> a look at it as well and give me some feedback. DT work still needs to
>> grow on me.
> 
> Hi Arend, (and thanks again for all the help with getting the chip to
> work!)
> 
> Great, I'm not subscribed to the devicetree list and so have missed this
> thread, but I'm glad to see it.
> 
> I don't think I have much to add to the comments you already received
> there. I'd need it to reference the 32K clock (which I currently
> force-enable manually), the OOB interrupt, and the reset pin as a GPIO
> (as for SHIELD the device needs to be put out of reset using an
> active-low GPIO before anything can happen). That last property could be
> optional as I suspect most designs won't use it.
> 
> Getting the device out of reset should be done before the bus probes the
> non-removable device, so I wonder how this would fit wrt. the DT
> power-on sequencing series by Olof. Something tells me this could rather
> be a property of the bus, but physically speaking the pin is connected
> to the wifi chip, so... Maybe we could get the platform driver to ask
> the bus to probe again after enabling power/getting the device out of
> reset?

Actually, brcmfmac provides a platform driver and a sdio driver. At the
end of the platform probe it registers the sdio driver, which will
trigger the bus to probe again. I am not sure how that would relate to
the DT power-on sequencing you mentioned.

Regards,
Arend

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox