Re: [PATCH V2] hwmon: (tmp102) Force wait for conversion time for the first valid data
On Tue, Dec 01, 2015 at 10:10:21AM -0600, Nishanth Menon wrote: > TMP102 works based on conversions done periodically. However, as per > the TMP102 data sheet[1] the first conversion is triggered immediately > after we program the configuration register. The temperature data > registers do not reflect proper data until the first conversion is > complete (in our case HZ/4). > > The driver currently sets the last_update to be jiffies - HZ, just > after the configuration is complete. When TMP102 driver registers > with the thermal framework, it immediately tries to read the sensor > temperature data. This takes place even before the conversion on the > TMP102 is complete and results in an invalid temperature read. > > Depending on the value read, this may cause thermal framework to > assume that a critical temperature event has occurred and attempts to > shutdown the system. > > Instead of causing an invalid mid-conversion value to be read > erroneously, we mark the last_update to be in-line with the current > jiffies. This allows the tmp102_update_device function to skip update > until the required conversion time is complete. Further, we ensure to > return -EAGAIN result instead of returning spurious temperature (such > as 0C) values to the caller to prevent any wrong decisions made with > such values. NOTE: this allows the read functions not to be blocking > and allows the callers to make the decision if they would like to > block or try again later. At least the current user(thermal) seems to > handle this by retrying later. > > A simpler alternative approach could be to sleep in the probe for the > duration required, but that will result in latency that is undesirable > and delay boot sequence un-necessarily. > > [1] http://www.ti.com/lit/ds/symlink/tmp102.pdf > > Cc: Eduardo Valentin> Reported-by: Aparna Balasubramanian > Reported-by: Elvita Lobo > Reported-by: Yan Liu > Signed-off-by: Nishanth Menon Applied. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hwmon: (tmp102) Force wait for conversion time for the first valid data
On 12/01/2015 06:21 AM, Nishanth Menon wrote: [ ... ] Hint about how the patch will look like: Looks ok (and better). Guenter diff --git a/drivers/hwmon/tmp102.c b/drivers/hwmon/tmp102.c index 65482624ea2c..5289aa0980a8 100644 --- a/drivers/hwmon/tmp102.c +++ b/drivers/hwmon/tmp102.c @@ -58,6 +58,7 @@ struct tmp102 { u16 config_orig; unsigned long last_update; int temp[3]; + bool first_time; }; /* convert left adjusted 13-bit TMP102 register value to milliCelsius */ @@ -93,6 +94,7 @@ static struct tmp102 *tmp102_update_device(struct device *dev) tmp102->temp[i] = tmp102_reg_to_mC(status); } tmp102->last_update = jiffies; + tmp102->first_time = false; } mutex_unlock(>lock); return tmp102; @@ -102,6 +104,12 @@ static int tmp102_read_temp(void *dev, int *temp) { struct tmp102 *tmp102 = tmp102_update_device(dev); + /* Is it too early even to return a conversion? */ + if (tmp102->first_time) { + dev_dbg(dev, "%s: Conversion not ready yet..\n", __func__); + return -EAGAIN; + } + *temp = tmp102->temp[0]; return 0; @@ -114,6 +122,10 @@ static ssize_t tmp102_show_temp(struct device *dev, struct sensor_device_attribute *sda = to_sensor_dev_attr(attr); struct tmp102 *tmp102 = tmp102_update_device(dev); + /* Is it too early even to return a read? */ + if (tmp102->first_time) + return -EAGAIN; + return sprintf(buf, "%d\n", tmp102->temp[sda->index]); } @@ -207,7 +219,9 @@ static int tmp102_probe(struct i2c_client *client, status = -ENODEV; goto fail_restore_config; } - tmp102->last_update = jiffies - HZ; + tmp102->last_update = jiffies; + /* Mark that we are not ready with data until conversion is complete */ + tmp102->first_time = true; mutex_init(>lock); hwmon_dev = hwmon_device_register_with_groups(dev, client->name, -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hwmon: (tmp102) Force wait for conversion time for the first valid data
On 11/30/2015 08:25 PM, Nishanth Menon wrote: TMP102 works based on conversions done periodically. However, as per the TMP102 data sheet[1] the first conversion is triggered immediately after we program the configuration register. The temperature data registers do not reflect proper data until the first conversion is complete (in our case HZ/4). The driver currently sets the last_update to be jiffies - HZ, just after the configuration is complete. When tmp102 driver registers with the thermal framework, it immediately tries to read the sensor temperature data. This takes place even before the conversion on the TMP102 is complete and results in an invalid temperature read. Depending on the value read, this may cause thermal framework to assume that a critical temperature event has occurred and attempts to shutdown the system. Instead of causing an invalid mid-conversion value to be read erroneously, we mark the last_update to be in-line with the current jiffies. This allows the tmp102_update_device function to skip update until the required conversion time is complete. Further, we ensure to return -EAGAIN result instead of returning spurious temperature (such as 0C) values to the caller to prevent any wrong decisions made with such values. A simpler alternative approach could be to sleep in the probe for the duration required, but that will result in latency that is undesirable that can delay boot sequence un-necessarily. A really simpler solution would be to mark when the device is ready to be accessed in the probe function, and go to sleep for the remaining time in the update function if necessary. This would not affect the probe function, avoid the somewhat awkward -EAGAIN, avoid overloading the value cache, and only sleep if necessary and as long as needed. [1] http://www.ti.com/lit/ds/symlink/tmp102.pdf Cc: Eduardo ValentinReported-by: Aparna Balasubramanian Reported-by: Elvita Lobo Reported-by: Yan Liu Signed-off-by: Nishanth Menon --- Example case (from Beagleboard-x15 using an older kernel revision): http://pastebin.ubuntu.com/13591711/ Notice the thermal shutdown trigger: thermal thermal_zone3: critical temperature reached(108 C),shutting down drivers/hwmon/tmp102.c | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/hwmon/tmp102.c b/drivers/hwmon/tmp102.c index 65482624ea2c..145f69108f23 100644 --- a/drivers/hwmon/tmp102.c +++ b/drivers/hwmon/tmp102.c @@ -50,6 +50,9 @@ #define TMP102_TLOW_REG 0x02 #define TMP102_THIGH_REG0x03 +/* TMP102 range is -55 to 150C -> we use -128 as a default invalid value */ +#define TMP102_NOTREADY-128 + This is a bit misleading, and also not correct, since the temperature is stored in milli-degrees C, so a value of -128 reflects -0.128 degreees C. While that value will not be seen in practice, it is still not a good idea to use it for this purpose. Even though the chip temperature range is -55 .. 150 C, that doesn't mean it never returns a value outside that range, for example if nothing is connected to an external sensor or if something is broken. You should use a value outside the value range, ie outside [-128,000 .. 127,999 ] to detect the "not ready" condition. struct tmp102 { struct i2c_client *client; struct device *hwmon_dev; @@ -102,6 +105,12 @@ static int tmp102_read_temp(void *dev, int *temp) { struct tmp102 *tmp102 = tmp102_update_device(dev); + /* Is it too early even to return a conversion? */ + if (tmp102->temp[0] == TMP102_NOTREADY) { + dev_dbg(dev, "%s: Conversion not ready yet..\n", __func__); + return -EAGAIN; Does this cause a hard loop in the calling code, or will the thermal code delay before it reads again ? If it causes a hard loop, it may be better to go to sleep if needed when reading the data, as suggested above. + } + *temp = tmp102->temp[0]; return 0; @@ -114,6 +123,10 @@ static ssize_t tmp102_show_temp(struct device *dev, struct sensor_device_attribute *sda = to_sensor_dev_attr(attr); struct tmp102 *tmp102 = tmp102_update_device(dev); + /* Is it too early even to return a read? */ + if (tmp102->temp[sda->index] == TMP102_NOTREADY) + return -EAGAIN; + return sprintf(buf, "%d\n", tmp102->temp[sda->index]); } @@ -207,7 +220,11 @@ static int tmp102_probe(struct i2c_client *client, status = -ENODEV; goto fail_restore_config; } - tmp102->last_update = jiffies - HZ; + tmp102->last_update = jiffies; + /* Mark that we are not ready with data until conversion is complete */ + tmp102->temp[0] = TMP102_NOTREADY; + tmp102->temp[1] = TMP102_NOTREADY; + tmp102->temp[2] = TMP102_NOTREADY;
Re: [PATCH] watchdog: omap_wdt: fix null pointer dereference
On 11/01/2015 06:20 PM, Peter Robinson wrote: Fix issue from two patches overlapping causing a kernel oops [ 3569.297449] Unable to handle kernel NULL pointer dereference at virtual address 0088 [ 3569.306272] pgd = dc894000 [ 3569.309287] [0088] *pgd= [ 3569.313104] Internal error: Oops: 5 [#1] SMP ARM [ 3569.317986] Modules linked in: ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ebtable_filter ebtable_nat ebtable_broute bridge stp llc ebtables ip6table_security ip6table_raw ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_filter ip6_tables iptable_security iptable_raw iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_mangle musb_dsps cppi41 musb_hdrc phy_am335x udc_core phy_generic phy_am335x_control omap_sham omap_aes omap_rng omap_hwspinlock omap_mailbox hwspinlock_core musb_am335x omap_wdt at24 8250_omap leds_gpio cpufreq_dt smsc davinci_mdio mmc_block ti_cpsw cpsw_common ptp pps_core cpsw_ale davinci_cpdma omap_hsmmc omap_dma mmc_core i2c_dev [ 3569.386293] CPU: 0 PID: 1429 Comm: wdctl Not tainted 4.3.0-0.rc7.git0.1.fc24.armv7hl #1 [ 3569.394740] Hardware name: Generic AM33XX (Flattened Device Tree) [ 3569.401179] task: dbd11a00 ti: dbaac000 task.ti: dbaac000 [ 3569.406917] PC is at omap_wdt_get_timeleft+0xc/0x20 [omap_wdt] [ 3569.413106] LR is at watchdog_ioctl+0x3cc/0x42c [ 3569.417902] pc : []lr : []psr: 600f0013 [ 3569.417902] sp : dbaadf18 ip : 0003 fp : 7f5d3bbe [ 3569.430014] r10: r9 : 0003 r8 : bef21ab8 [ 3569.435535] r7 : dbbc0f7c r6 : dbbc0f18 r5 : bef21ab8 r4 : [ 3569.442427] r3 : r2 : r1 : 8004570a r0 : dbbc0f18 [ 3569.449323] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 3569.456858] Control: 10c5387d Table: 9c894019 DAC: 0051 [ 3569.462927] Process wdctl (pid: 1429, stack limit = 0xdbaac220) [ 3569.469179] Stack: (0xdbaadf18 to 0xdbaae000) [ 3569.473790] df00: bef21ab8 dbf60e38 [ 3569.482441] df20: dc91b840 8004570a bef21ab8 c03988a4 dbaadf48 dc854000 dd313850 [ 3569.491092] df40: ddf033b8 570a dc91b80b dbaadf3c dbf60e38 0020 c0df9250 c0df6c48 [ 3569.499741] df60: dc91b840 8004570a dc91b840 dc91b840 8004570a bef21ab8 0003 [ 3569.508389] df80: c03989d4 bef21b74 7f5d3bad 0003 0036 c020fcc4 dbaac000 [ 3569.517037] dfa0: c020fb00 bef21b74 7f5d3bad 0003 8004570a bef21ab8 0001 [ 3569.525685] dfc0: bef21b74 7f5d3bad 0003 0036 0001 7f5e4eb0 7f5d3bbe [ 3569.534334] dfe0: 7f5e4f10 bef21a3c 7f5d0a54 b6e97e0c a00f0010 0003 [ 3569.543038] [] (omap_wdt_get_timeleft [omap_wdt]) from [] (watchdog_ioctl+0x3cc/0x42c) [ 3569.553266] [] (watchdog_ioctl) from [] (do_vfs_ioctl+0x5bc/0x698) [ 3569.561648] [] (do_vfs_ioctl) from [] (SyS_ioctl+0x54/0x7c) [ 3569.569400] [] (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x3c) [ 3569.577413] Code: e12fff1e e52de004 e8bd4000 e5903060 (e5933088) [ 3569.584089] ---[ end trace cec3039bd3ae610a ]--- Cc: <sta...@vger.kernel.org> # v4.2+ Cc: Guenter Roeck <li...@roeck-us.net> Cc: Lars Poeschel <poesc...@lemonage.de> Signed-off-by: Peter Robinson <pbrobin...@gmail.com> Reviewed-by: Guenter Roeck <li...@roeck-us.net> --- drivers/watchdog/omap_wdt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c index d96bee0..6f17c93 100644 --- a/drivers/watchdog/omap_wdt.c +++ b/drivers/watchdog/omap_wdt.c @@ -205,7 +205,7 @@ static int omap_wdt_set_timeout(struct watchdog_device *wdog, static unsigned int omap_wdt_get_timeleft(struct watchdog_device *wdog) { - struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); + struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); void __iomem *base = wdev->base; u32 value; -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: null pointer dereference in omap_wdt
On 11/01/2015 03:11 PM, Peter Robinson wrote: Hi Lars, I'm seeing the following crash in omap_wdt in omap_wdt_get_timeleft when I just run wdctl on a couple of different TI devices, the beaglebone black/green or panda ES. I'm seeing it on both 4.2.3 and 4.3rc7, looking at git it looks like you added the fuction in June, have you seen this on any of your platforms? I think that struct omap_wdt_dev *wdev = watchdog_get_drvdata(wdog); in omap_wdt_get_timeleft() needs to be struct omap_wdt_dev *wdev = to_omap_wdt_dev(wdog); Looks like there was an overlap between commit d2f78268ba58 ("watchdog: omap: put struct watchdog_device into driver data") and commit 452fafed839e ("watchdog: omap_wdt: implement get_timeleft"). Can someone test this and send a patch to fix the problem ? Thanks, Guenter Peter [ 299.672508] Unable to handle kernel NULL pointer dereference at virtual address 0088 [ 299.681372] pgd = de35c000 [ 299.684385] [0088] *pgd= [ 299.688205] Internal error: Oops: 5 [#1] SMP ARM [ 299.693086] Modules linked in: ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ebtable_filter ebtable_nat ebtable_broute bridge stp llc ebtables ip6table_mangle ip6table_security ip6table_nat nf_conntra ck_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_raw ip6table_filter ip6_tables iptable_mangle iptable_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_raw musb_dsps musb_ hdrc phy_am335x udc_core phy_generic phy_am335x_control cppi41 omap_aes omap_sham omap_hwspinlock hwspinlock_core omap_rng omap_mailbox at24 musb_am335x 8250_omap omap_wdt leds_gpio cpufreq_dt smsc davinci_mdio mmc_block ti_cpsw cpsw_common ptp pps_core cpsw_ale davinci_cpdma omap_hsmmc omap_dma mmc_core i2c_dev [ 299.761395] CPU: 0 PID: 1242 Comm: wdctl Not tainted 4.3.0-0.rc7.git0.1.fc24.armv7hl #1 [ 299.769841] Hardware name: Generic AM33XX (Flattened Device Tree) [ 299.776280] task: de505b00 ti: dcf26000 task.ti: dcf26000 [ 299.782017] PC is at omap_wdt_get_timeleft+0xc/0x20 [omap_wdt] [ 299.788205] LR is at watchdog_ioctl+0x3cc/0x42c [ 299.793001] pc : []lr : []psr: 600f0013 [ 299.793001] sp : dcf27f18 ip : 0003 fp : 7f626bbe [ 299.805114] r10: r9 : 0003 r8 : becb1ab8 [ 299.810634] r7 : dbc0837c r6 : dbc08318 r5 : becb1ab8 r4 : [ 299.817526] r3 : r2 : r1 : 8004570a r0 : dbc08318 [ 299.824423] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 299.831958] Control: 10c5387d Table: 9e35c019 DAC: 0051 [ 299.838026] Process wdctl (pid: 1242, stack limit = 0xdcf26220) [ 299.844280] Stack: (0xdcf27f18 to 0xdcf28000) [ 299.848890] 7f00: becb1ab8 dbf241b8 [ 299.857541] 7f20: dc823840 8004570a becb1ab8 c03988a4 dcf27f48 dcf17100 de352850 [ 299.866191] 7f40: dde86ee0 570a dc82380b dcf27f3c dbf241b8 0020 c0df9250 c0df6c48 [ 299.874840] 7f60: dc823840 8004570a dc823840 dc823840 8004570a becb1ab8 0003 [ 299.883489] 7f80: c03989d4 becb1b74 7f626bad 0003 0036 c020fcc4 dcf26000 [ 299.892137] 7fa0: c020fb00 becb1b74 7f626bad 0003 8004570a becb1ab8 0001 [ 299.900786] 7fc0: becb1b74 7f626bad 0003 0036 0001 7f637eb0 7f626bbe [ 299.909434] 7fe0: 7f637f10 becb1a3c 7f623a54 b6ee5e0c a00f0010 0003 [ 299.918136] [] (omap_wdt_get_timeleft [omap_wdt]) from [] (watchdog_ioctl+0x3cc/0x42c) [ 299.928365] [] (watchdog_ioctl) from [] (do_vfs_ioctl+0x5bc/0x698) [ 299.936747] [] (do_vfs_ioctl) from [] (SyS_ioctl+0x54/0x7c) [ 299.944499] [] (SyS_ioctl) from [] (ret_fast_syscall+0x0/0x3c) [ 299.952511] Code: e12fff1e e52de004 e8bd4000 e5903060 (e5933088) -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: multi_v7_defconfig: Enable PBIAS regulator
On Sun, Sep 06, 2015 at 04:41:10PM -0700, Guenter Roeck wrote: > On Thu, Sep 03, 2015 at 03:25:11PM +0530, Kishon Vijay Abraham I wrote: > > PBIAS regulator is required for MMC module in OMAP2, OMAP3, OMAP4, > > OMAP5 and DRA7 SoCs. Enable it here. > > > > Signed-off-by: Kishon Vijay Abraham I <kis...@ti.com> > > Tested-by: Guenter Roeck <li...@roeck-us.net> > This problem is now causing various runtime failures in mainline. I see the following qemu test failures: arm:beagle:multi_v7_defconfig:omap3-beagle arm:beaglexm:multi_v7_defconfig:omap3-beagle-xm arm:overo:multi_v7_defconfig:omap3-overo-tobi I would expect all omap2/3/4/5 runtime tests with multi_v7_defconfig to fail. Also, FWIW, Fixes: 6a9b2ff07d04 ("mmc: host: omap_hsmmc: return on fatal errors from omap_hsmmc_reg_get") Guenter > > --- > > arch/arm/configs/multi_v7_defconfig |1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm/configs/multi_v7_defconfig > > b/arch/arm/configs/multi_v7_defconfig > > index 5fd8df6..f497c96 100644 > > --- a/arch/arm/configs/multi_v7_defconfig > > +++ b/arch/arm/configs/multi_v7_defconfig > > @@ -403,6 +403,7 @@ CONFIG_REGULATOR_MAX8973=y > > CONFIG_REGULATOR_MAX77686=y > > CONFIG_REGULATOR_MAX77693=m > > CONFIG_REGULATOR_PALMAS=y > > +CONFIG_REGULATOR_PBIAS=y > > CONFIG_REGULATOR_S2MPS11=y > > CONFIG_REGULATOR_S5M8767=y > > CONFIG_REGULATOR_TPS51632=y > > -- > > 1.7.9.5 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ARM: multi_v7_defconfig: Enable PBIAS regulator
On Thu, Sep 03, 2015 at 03:25:11PM +0530, Kishon Vijay Abraham I wrote: > PBIAS regulator is required for MMC module in OMAP2, OMAP3, OMAP4, > OMAP5 and DRA7 SoCs. Enable it here. > > Signed-off-by: Kishon Vijay Abraham I <kis...@ti.com> Tested-by: Guenter Roeck <li...@roeck-us.net> > --- > arch/arm/configs/multi_v7_defconfig |1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/configs/multi_v7_defconfig > b/arch/arm/configs/multi_v7_defconfig > index 5fd8df6..f497c96 100644 > --- a/arch/arm/configs/multi_v7_defconfig > +++ b/arch/arm/configs/multi_v7_defconfig > @@ -403,6 +403,7 @@ CONFIG_REGULATOR_MAX8973=y > CONFIG_REGULATOR_MAX77686=y > CONFIG_REGULATOR_MAX77693=m > CONFIG_REGULATOR_PALMAS=y > +CONFIG_REGULATOR_PBIAS=y > CONFIG_REGULATOR_S2MPS11=y > CONFIG_REGULATOR_S5M8767=y > CONFIG_REGULATOR_TPS51632=y > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -next] smsc911x: Fix crash seen if neither ACPI nor OF is configured or used
On 08/26/2015 10:04 AM, Tony Lindgren wrote: Hi, * Guenter Roeck li...@roeck-us.net [150817 13:48]: Commit 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT) makes the call to smsc911x_probe_config() unconditional, and no longer fails if there is no device node. device_get_phy_mode() is called unconditionally, and if there is no phy node configured returns an error code. This error code is assigned to phy_interface, and interpreted elsewhere in the code as valid phy mode. This in turn causes qemu to crash when running a variant of realview_pb_defconfig. qemu: hardware error: lan9118_read: Bad reg 0x86 Fixes: 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT) Cc: Jeremy Linton jeremy.lin...@arm.com Cc Graeme Gregory graeme.greg...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- drivers/net/ethernet/smsc/smsc911x.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c index 0f21aa3bb537..34f97684506b 100644 --- a/drivers/net/ethernet/smsc/smsc911x.c +++ b/drivers/net/ethernet/smsc/smsc911x.c @@ -2367,12 +2367,17 @@ static const struct smsc911x_ops shifted_smsc911x_ops = { static int smsc911x_probe_config(struct smsc911x_platform_config *config, struct device *dev) { + int phy_interface; u32 width = 0; if (!dev) return -ENODEV; - config-phy_interface = device_get_phy_mode(dev); + phy_interface = device_get_phy_mode(dev); + if (phy_interface 0) + return phy_interface; + + config-phy_interface = phy_interface; device_get_mac_address(dev, config-mac, ETH_ALEN); Looks like this change makes at least omap boards using smsc911x fail with -22 for me in Linux next. Do any of the the device tree configured smsc911x devices actually have a phy configured? Ok, this is more subtle than I thought. Previously, the code would not attempt any devicetree configuration if devicetree was not configured. Now it does. The error return from device_get_phy_mode() isn't the actual problem. Apparently it doesn't really matter if a nonsensical value is assigned to phy_interface. The problem is that the reg-io-width property is obviously not present in the non-dt and non-acpi case. This overwrites the existing platform data configuration and selects 16 bit mode, to which the (simulated) hardware obviously reacts less than enthusiastic. Fixing this properly won't be easy. If the reg-io-width property is not present or wrong, the default register width is 16 bit. Obviously, if neither DT nor ACPI is available, it won't be present. This causes the crash I had observed. Bad part is that there does not seem to be a reliable means to detect that platform data should be used in that situation. Other device_get_XXX functions return -ENXIO if that happens, but not device_property_read_u32(). It is _supposed_ to return it per its API, but it doesn't (it returns -ENODATA). We may need two separate patches, one to fix up device_property_read_u32() to return -ENXIO, and one to fix smsc911x_probe_config() to ignore the error from device_get_phy_mode(), and to bail out if device_property_read_u32() returns -ENXIO. The simpler alternative would be to check the return value from device_property_read_u32() for both -ENXIO and -ENODATA. This would make the code independent of the necessary core changes (which may take a while). I tested this variant, and it works, at least for the non-DT case. Does this make sense ? Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -next] smsc911x: Fix crash seen if neither ACPI nor OF is configured or used
Hi Tony, On 08/26/2015 10:04 AM, Tony Lindgren wrote: Hi, * Guenter Roeck li...@roeck-us.net [150817 13:48]: Commit 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT) makes the call to smsc911x_probe_config() unconditional, and no longer fails if there is no device node. device_get_phy_mode() is called unconditionally, and if there is no phy node configured returns an error code. This error code is assigned to phy_interface, and interpreted elsewhere in the code as valid phy mode. This in turn causes qemu to crash when running a variant of realview_pb_defconfig. qemu: hardware error: lan9118_read: Bad reg 0x86 Fixes: 0b50dc4fc971 (Convert smsc911x to use ACPI as well as DT) Cc: Jeremy Linton jeremy.lin...@arm.com Cc Graeme Gregory graeme.greg...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- drivers/net/ethernet/smsc/smsc911x.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c index 0f21aa3bb537..34f97684506b 100644 --- a/drivers/net/ethernet/smsc/smsc911x.c +++ b/drivers/net/ethernet/smsc/smsc911x.c @@ -2367,12 +2367,17 @@ static const struct smsc911x_ops shifted_smsc911x_ops = { static int smsc911x_probe_config(struct smsc911x_platform_config *config, struct device *dev) { + int phy_interface; u32 width = 0; if (!dev) return -ENODEV; - config-phy_interface = device_get_phy_mode(dev); + phy_interface = device_get_phy_mode(dev); + if (phy_interface 0) + return phy_interface; + + config-phy_interface = phy_interface; device_get_mac_address(dev, config-mac, ETH_ALEN); Looks like this change makes at least omap boards using smsc911x fail with -22 for me in Linux next. What do you see if you revert my patch ? It should assign -22, or its unsigned representation, to phy_interface, which isn't such a good idea either. Do any of the the device tree configured smsc911x devices actually have a phy configured? Good question, and beats me. Looking into the original code, it didn't check for an error return from of_get_phy_mode() either, and thus _would_ dutifully assign the error code to phy_interface. Wonder how was this supposed to work to start with. I'll do some debugging and try to find out what exactly is going on. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -next] smsc911x: Fix crash seen if neither ACPI nor OF is configured or used
Hi Tony, On 08/26/2015 01:16 PM, Tony Lindgren wrote: [ ... ] We may need two separate patches, one to fix up device_property_read_u32() to return -ENXIO, and one to fix smsc911x_probe_config() to ignore the error from device_get_phy_mode(), and to bail out if device_property_read_u32() returns -ENXIO. I guess the device_property_read_u32() change needs to be discussed separately.. So probably best to fix up the regression to smsc911x first. Not sure myself. Jeremy has a point - we don't really know for sure how safe it is to check for -ENODATA (in addition to -ENXIO). Also, fixing device_property_read_u32() turned out to be much easier than I thought. The simpler alternative would be to check the return value from device_property_read_u32() for both -ENXIO and -ENODATA. This would make the code independent of the necessary core changes (which may take a while). I tested this variant, and it works, at least for the non-DT case. Does this make sense ? Yeh I think that would allow fixing up the smsc911x regression while discussing the device_property_read_u32() change. Got a test patch for me to try? You should have two by now to choose from. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] thermal: consistently use int for temperatures
On 07/24/2015 03:11 PM, Pavel Machek wrote: On Fri 2015-07-24 06:59:26, Guenter Roeck wrote: On 07/23/2015 11:29 PM, Sascha Hauer wrote: On Thu, Jul 23, 2015 at 02:07:59PM +0200, Pavel Machek wrote: On Tue 2015-07-21 09:21:32, Sascha Hauer wrote: The thermal code uses int, long and unsigned long for temperatures in different places. Using an unsigned type limits the thermal framework to positive temperatures without need. Also several drivers currently will report temperatures near UINT_MAX for temperatures below 0°C. This will probably immediately shut the machine down due to overtemperature if started below 0°C. 'long' is 64bit on several architectures. This is not needed since INT_MAX °mC is above the melting point of all known materials. Can we do something like typedef millicelsius_t int; ...to document the units? I am not very fond of typedefs and I am not sure this adds any value. I could change it when more people ask for it, but I just sent the new version without this. I thought we are supposed to not introduce new typedefs anyway. You are not supposed to typedef struct, but typedef for millicelsius_t would be ok. And it is your only chance if you want people to pay attention. If you make it int, someone will pass it to long or something else.. Seems to me that would be just lazyness. The same person might use 'long' even if millicelsius_t is defined. A typedef doesn't preclude people from ignoring it. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] thermal: consistently use int for temperatures
On 07/23/2015 11:29 PM, Sascha Hauer wrote: On Thu, Jul 23, 2015 at 02:07:59PM +0200, Pavel Machek wrote: On Tue 2015-07-21 09:21:32, Sascha Hauer wrote: The thermal code uses int, long and unsigned long for temperatures in different places. Using an unsigned type limits the thermal framework to positive temperatures without need. Also several drivers currently will report temperatures near UINT_MAX for temperatures below 0°C. This will probably immediately shut the machine down due to overtemperature if started below 0°C. 'long' is 64bit on several architectures. This is not needed since INT_MAX °mC is above the melting point of all known materials. Can we do something like typedef millicelsius_t int; ...to document the units? I am not very fond of typedefs and I am not sure this adds any value. I could change it when more people ask for it, but I just sent the new version without this. I thought we are supposed to not introduce new typedefs anyway. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH -next] usb: dwc3: Remove unprintable characters from Kconfig
Fix: drivers/usb/dwc3/Kconfig:16:warning: ignoring unsupported character '�' drivers/usb/dwc3/Kconfig:16:warning: ignoring unsupported character '�' Fixes: 88bc9d194ff6 (usb: dwc3: add ULPI interface support) Cc: Heikki Krogerus heikki.kroge...@linux.intel.com Signed-off-by: Guenter Roeck li...@roeck-us.net --- drivers/usb/dwc3/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig index 473bfaa96c30..dede32e809b6 100644 --- a/drivers/usb/dwc3/Kconfig +++ b/drivers/usb/dwc3/Kconfig @@ -13,7 +13,7 @@ if USB_DWC3 config USB_DWC3_ULPI bool Register ULPI PHY Interface - depends on USB_ULPI_BUS=y || USB_ULPI_BUS=USB_DWC3 + depends on USB_ULPI_BUS=y || USB_ULPI_BUS=USB_DWC3 help Select this if you have ULPI type PHY attached to your DWC3 controller. -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1a/3] watchdog: omap: clearify device tree documentation
On 04/24/2015 07:10 PM, Felipe Balbi wrote: On Fri, Apr 24, 2015 at 10:20:50PM +0200, Uwe Kleine-König wrote: ti,hwmods doesn't belong into the compatible section but is a property on it's own. Also reformat the section of required properties to match the usual style of dt binding documents. Signed-off-by: Uwe Kleine-König u.kleine-koe...@pengutronix.de looks great to me, thanks Reviewed-by: Felipe Balbi ba...@ti.com Acked-by: Felipe Balbi ba...@ti.com Reviewed-by: and Acked-by: are mutually exclusive. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1a/3] watchdog: omap: clearify device tree documentation
On 04/26/2015 12:12 PM, Uwe Kleine-König wrote: On Sun, Apr 26, 2015 at 08:29:20AM -0700, Guenter Roeck wrote: On 04/24/2015 07:10 PM, Felipe Balbi wrote: On Fri, Apr 24, 2015 at 10:20:50PM +0200, Uwe Kleine-König wrote: ti,hwmods doesn't belong into the compatible section but is a property on it's own. Also reformat the section of required properties to match the usual style of dt binding documents. Signed-off-by: Uwe Kleine-König u.kleine-koe...@pengutronix.de looks great to me, thanks Reviewed-by: Felipe Balbi ba...@ti.com Acked-by: Felipe Balbi ba...@ti.com Reviewed-by: and Acked-by: are mutually exclusive. Really? Acked means you like it and Reviewed by means you looked deeper into it. Maybe reviewing without liking isn't likely, *shrug*. SubmittingPatches says about Acked-by: ... to signify and record their approval ... has at least reviewed the patch and has indicated acceptance and about Reviewed-by: ... indicates that the patch has been reviewed and found acceptable ... Maybe I should have used the term redundant. *shrugtoo*. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] (gpio-fan): Move the thermal registration after registration is complete
On 04/08/2015 04:23 PM, Nishanth Menon wrote: Thermal framework may already be ready and cooling policies might already be functional when we are attempting to register gpio fan as a cooling device. This can be reproduced by changing probe order in which registration of various modules are done in a system. In such a case, kernel generates an oops since the data structures are not completely populated with the wrong assumption that thermal framework is not yet ready. Fix this by reordering the thermal framework registration to occur after hwmon registration of the fan is complete. Example kernel oops: [ 149.005828] Unable to handle kernel NULL pointer dereference at virtual address 008c [ 149.014369] pgd = ecf48000 [ 149.017204] [008c] *pgd=ac065831, *pte=, *ppte= [ 149.023820] Internal error: Oops: 17 [#1] SMP ARM [ 149.028745] Modules linked in: gpio_fan(+) cpufreq_dt ipv6 evdev leds_gpio led_class omap_wdt phy_omap_usb2 rtc_palmas palmas_pwrbutton tmp102 ti_soc_thermal dwc3_omap thermal_sys extcon rtc_omap rtc_ds1307 hwmon [ 149.048629] CPU: 1 PID: 1183 Comm: modprobe Not tainted 4.0.0-rc7-next-20150407-2-g7a82da074c99 #3 [ 149.058383] Hardware name: Generic DRA74X (Flattened Device Tree) [ 149.064763] task: edec1240 ti: ec0e task.ti: ec0e [ 149.070421] PC is at dev_driver_string+0x0/0x38 [ 149.075165] LR is at __dev_printk+0x24/0x70 [ 149.079540] pc : [c03d6cd0]lr : [c03d72c4]psr: 2013 [ 149.079540] sp : ec0e1c28 ip : edec1240 fp : [ 149.091568] r10: edf3eee0 r9 : r8 : [ 149.097040] r7 : edf3eea0 r6 : 0034 r5 : 0010 r4 : ec0e1c44 [ 149.103871] r3 : ec0e1c4c r2 : ec0e1c44 r1 : c079d800 r0 : 0010 [ 149.110709] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user [ 149.118182] Control: 10c5387d Table: acf4806a DAC: 0015 [ 149.124198] Process modprobe (pid: 1183, stack limit = 0xec0e0218) [ 149.130673] Stack: (0xec0e1c28 to 0xec0e2000) [ 149.135235] 1c20: 6013 c05e2ae0 edf3ec00 ec934a10 c03d73d4 ... [ 149.392230] 1fe0: befe1888 befe1878 00019418 b6ea08f0 8010 0003 [ 149.400798] [c03d6cd0] (dev_driver_string) from [c03d72c4] (__dev_printk+0x24/0x70) [ 149.409193] [c03d72c4] (__dev_printk) from [c03d73d4] (dev_warn+0x34/0x48) [ 149.416767] [c03d73d4] (dev_warn) from [bf0f54fc] (get_fan_speed_index+0x94/0xa4 [gpio_fan]) [ 149.425980] [bf0f54fc] (get_fan_speed_index [gpio_fan]) from [bf0f5524] (gpio_fan_get_cur_state+0x18/0x30 [gpio_fan]) [ 149.437476] [bf0f5524] (gpio_fan_get_cur_state [gpio_fan]) from [bf02767c] (thermal_zone_trip_update+0xe8/0x2a4 [thermal_sys]) [ 149.449794] [bf02767c] (thermal_zone_trip_update [thermal_sys]) from [bf027844] (step_wise_throttle+0xc/0x74 [thermal_sys]) [ 149.461832] [bf027844] (step_wise_throttle [thermal_sys]) from [bf024ff4] (handle_thermal_trip+0x5c/0x188 [thermal_sys]) [ 149.473603] [bf024ff4] (handle_thermal_trip [thermal_sys]) from [bf0256c4] (thermal_zone_device_update+0x94/0x108 [thermal_sys]) [ 149.486104] [bf0256c4] (thermal_zone_device_update [thermal_sys]) from [bf026470] (__thermal_cooling_device_register+0x2e8/0x374 [thermal_sys]) [ 149.499956] [bf026470] (__thermal_cooling_device_register [thermal_sys]) from [bf0f58e4] (gpio_fan_probe+0x350/0x4d0 [gpio_fan]) [ 149.512438] [bf0f58e4] (gpio_fan_probe [gpio_fan]) from [c03db8a0] (platform_drv_probe+0x48/0x98) [ 149.522109] [c03db8a0] (platform_drv_probe) from [c03da30c] (driver_probe_device+0x1b0/0x26c) [ 149.531399] [c03da30c] (driver_probe_device) from [c03da45c] (__driver_attach+0x94/0x98) [ 149.540238] [c03da45c] (__driver_attach) from [c03d8bb0] (bus_for_each_dev+0x54/0x88) [ 149.548814] [c03d8bb0] (bus_for_each_dev) from [c03d9a34] (bus_add_driver+0xdc/0x1d4) [ 149.557381] [c03d9a34] (bus_add_driver) from [c03dac30] (driver_register+0x78/0xf4) [ 149.565765] [c03dac30] (driver_register) from [c0009784] (do_one_initcall+0x80/0x1d8) [ 149.574340] [c0009784] (do_one_initcall) from [c00c2278] (do_init_module+0x5c/0x1b8) [ 149.582833] [c00c2278] (do_init_module) from [c00c3bbc] (load_module+0x1720/0x1dcc) [ 149.591212] [c00c3bbc] (load_module) from [c00c43d0] (SyS_finit_module+0x68/0x6c) [ 149.599418] [c00c43d0] (SyS_finit_module) from [c000f3c0] (ret_fast_syscall+0x0/0x4c) [ 149.607994] Code: 1583 e1a6 e28dd008 e8bd8070 (e590307c) Cc: Eduardo Valentin edubez...@gmail.com Fixes: b5cf88e46bad ((gpio-fan): Add thermal control hooks) Signed-off-by: Nishanth Menon n...@ti.com --- Applied to -next. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Clock Regression in next-20150130 caused by cb75a8fcd14e
On Fri, Jan 30, 2015 at 05:04:44PM -0800, Tony Lindgren wrote: Hi all, Looks like commit cb75a8fcd14e (clk: Add rate constraints to clocks) causes a regression on at least omaps where the serial console either does not show anything, or just prints garbage. Reverting cb75a8fcd14e makes things work again on next-20150130. Any ideas? The patch seems to have some problems. Also see http://www.spinics.net/lists/kernel/msg1916843.html. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: N900 v3.19-rc5 errors
On Fri, Jan 23, 2015 at 02:15:56PM +0100, Pali Rohár wrote: Hello, I'm getting these two errors when booting DT linux v3.19-rc5 in n900 qemu: Hello Pali, can you send me the kernel confguration and qemu command line you are using ? Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor
On 01/18/2015 12:33 PM, Pavel Machek wrote: Pavel, can you look into the omap4 thermal driver to see if it can be used ? After some fixes... yes, it seems to be same hardware. So this should be the way to go, but then we have others claim that it should not be done because the OMAP3 sensors are too unreliable to use for thermal decisions. Not really sure where that leaves us. I am kind of opposed to have similar drivers for similar chips in two different subsystems. Is it possible to add the patch below to the omap thermal driver and not use it for thermal decisions ? Well... noone forces you to enable the driver, and I don't think it will do any thermal decisions on N900 as it is ... so we should be ok. Plus, it seems to work reasonably well (say +- 5 C), so situation does not seem to be as bad as TI claims. Nokia was actually using it in production. Ok, I'll assume that omap3 support will be added to the omap thermal driver. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor
On Sat, Jan 03, 2015 at 10:18:58AM +0100, Pavel Machek wrote: On Mon 2014-12-29 11:04:48, Guenter Roeck wrote: On Mon, Dec 29, 2014 at 07:15:56PM +0100, Pavel Machek wrote: On Mon 2014-12-29 12:01:03, Nishanth Menon wrote: On Mon, Dec 29, 2014 at 11:52 AM, Grazvydas Ignotas nota...@gmail.com wrote: On Fri, Dec 26, 2014 at 2:34 PM, Sebastian Reichel s...@kernel.org wrote: OMAP34xx and OMAP36xx processors contain a register in the syscon area, which can be used to determine the SoCs temperature. This patch provides a DT based driver for the temperature sensor based on an older driver written by Peter De Schrijver for the Nokia N900 and N9. The sensor looks like an earlier iteration of sensors used in newer OMAPs, which are already supported by maybe drivers/thermal/ti-soc-thermal/ , maybe it would make sense to update that driver instead? Just to be clear - OMAP4 is the first time that the sensors were reliable enough to be used. When testing initial version of the patch, they seem to work very well in the omap3 case. Pavel, can you look into the omap4 thermal driver to see if it can be used ? After some fixes... yes, it seems to be same hardware. So this should be the way to go, but then we have others claim that it should not be done because the OMAP3 sensors are too unreliable to use for thermal decisions. Not really sure where that leaves us. I am kind of opposed to have similar drivers for similar chips in two different subsystems. Is it possible to add the patch below to the omap thermal driver and not use it for thermal decisions ? Guenter Signed-off-by: Pavel Machek pa...@ucw.cz diff --git a/drivers/thermal/ti-soc-thermal/Kconfig b/drivers/thermal/ti-soc-thermal/Kconfig index bd4c7be..a49495f 100644 --- a/drivers/thermal/ti-soc-thermal/Kconfig +++ b/drivers/thermal/ti-soc-thermal/Kconfig @@ -21,6 +21,15 @@ config TI_THERMAL This includes trip points definitions, extrapolation rules and CPU cooling device bindings. +config OMAP3_THERMAL + bool Texas Instruments OMAP3 thermal support + depends on TI_SOC_THERMAL + depends on ARCH_OMAP3 + help + If you say yes here you get thermal support for the Texas Instruments + OMAP3 SoC family. The current chip supported are: +- OMAP3430 + config OMAP4_THERMAL bool Texas Instruments OMAP4 thermal support depends on TI_SOC_THERMAL diff --git a/drivers/thermal/ti-soc-thermal/Makefile b/drivers/thermal/ti-soc-thermal/Makefile index 1226b24..2b5b220 100644 --- a/drivers/thermal/ti-soc-thermal/Makefile +++ b/drivers/thermal/ti-soc-thermal/Makefile @@ -2,5 +2,6 @@ obj-$(CONFIG_TI_SOC_THERMAL) += ti-soc-thermal.o ti-soc-thermal-y := ti-bandgap.o ti-soc-thermal-$(CONFIG_TI_THERMAL) += ti-thermal-common.o ti-soc-thermal-$(CONFIG_DRA752_THERMAL) += dra752-thermal-data.o +ti-soc-thermal-$(CONFIG_OMAP3_THERMAL) += omap3-thermal-data.o ti-soc-thermal-$(CONFIG_OMAP4_THERMAL) += omap4-thermal-data.o ti-soc-thermal-$(CONFIG_OMAP5_THERMAL) += omap5-thermal-data.o diff --git a/drivers/thermal/ti-soc-thermal/omap3-thermal-data.c b/drivers/thermal/ti-soc-thermal/omap3-thermal-data.c new file mode 100644 index 000..a79ebf2 --- /dev/null +++ b/drivers/thermal/ti-soc-thermal/omap3-thermal-data.c @@ -0,0 +1,99 @@ +/* + * OMAP3 thermal driver. + * + * Copyright (C) 2011-2012 Texas Instruments Inc. + * Copyright (C) 2014 Pavel Machek pa...@ucw.cz + * + * This software is licensed under the terms of the GNU General Public + * License version 2, as published by the Free Software Foundation, and + * may be copied, distributed, and modified under those terms. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + */ + +#include ti-thermal.h +#include ti-bandgap.h + +/* + * OMAP34XX has one instance of thermal sensor for MPU + * need to describe the individual bit fields + */ +static struct temp_sensor_registers +omap34xx_mpu_temp_sensor_registers = { + .temp_sensor_ctrl = 0, + .bgap_tempsoff_mask = 0, /* Unused, we don't have POWER_SWITCH */ + .bgap_soc_mask = BIT(8), + .bgap_eocz_mask = BIT(7), + .bgap_dtemp_mask = 0x7f, + + .bgap_mode_ctrl = 0, + .mode_ctrl_mask = 0,/* Unused, no MODE_CONFIG */ + + .bgap_efuse = 0, +}; + +/* Thresholds and limits for OMAP34XX MPU temperature sensor */ +static struct temp_sensor_data omap34xx_mpu_temp_sensor_data = { + .min_freq = 32768, + .max_freq = 32768, + .max_temp = -99000, + .min_temp = 99000, + .hyst_val = 5000, +}; + +/* + * Temperature values in milli
Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor
On Mon, Dec 29, 2014 at 07:15:56PM +0100, Pavel Machek wrote: On Mon 2014-12-29 12:01:03, Nishanth Menon wrote: On Mon, Dec 29, 2014 at 11:52 AM, Grazvydas Ignotas nota...@gmail.com wrote: On Fri, Dec 26, 2014 at 2:34 PM, Sebastian Reichel s...@kernel.org wrote: OMAP34xx and OMAP36xx processors contain a register in the syscon area, which can be used to determine the SoCs temperature. This patch provides a DT based driver for the temperature sensor based on an older driver written by Peter De Schrijver for the Nokia N900 and N9. The sensor looks like an earlier iteration of sensors used in newer OMAPs, which are already supported by maybe drivers/thermal/ti-soc-thermal/ , maybe it would make sense to update that driver instead? Just to be clear - OMAP4 is the first time that the sensors were reliable enough to be used. When testing initial version of the patch, they seem to work very well in the omap3 case. Pavel, can you look into the omap4 thermal driver to see if it can be used ? Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] hwmon: Driver for OMAP3 temperature sensor
On Sat, Dec 27, 2014 at 11:35:16PM +0100, Pavel Machek wrote: On Sat 2014-12-27 20:58:25, Pavel Machek wrote: On Fri 2014-12-26 13:34:53, Sebastian Reichel wrote: OMAP34xx and OMAP36xx processors contain a register in the syscon area, which can be used to determine the SoCs temperature. This patch provides a DT based driver for the temperature sensor based on an older driver written by Peter De Schrijver for the Nokia N900 and N9. Signed-off-by: Sebastian Reichel s...@kernel.org --- drivers/hwmon/Kconfig | 8 ++ drivers/hwmon/Makefile | 1 + drivers/hwmon/omap3-temp.c | 307 + 3 files changed, 316 insertions(+) create mode 100644 drivers/hwmon/omap3-temp.c When it hangs, it loops here: do { regmap_read(data-syscon, SYSCON_TEMP_REG,temp_sensor_reg); if ((temp_sensor_reg eocz_mask) == level) return true; printk(=); } while (ktime_us_delta(expire, ktime_get()) 0); And this fixes the hang, and makes level handling more readable. Fix the timeout code, now it actually works. Driver still fails after a while. Signed-off-by: Pavel Machek pa...@ucw.cz diff --git a/drivers/hwmon/omap3-temp.c b/drivers/hwmon/omap3-temp.c index 8a69604..1b8c768 100644 --- a/drivers/hwmon/omap3-temp.c +++ b/drivers/hwmon/omap3-temp.c @@ -130,9 +130,7 @@ static inline bool wait_for_eocz(struct omap3_temp_data *data, ktime_t timeout, expire; u32 temp_sensor_reg, eocz_mask; eocz_mask = BIT(data-hwdata-eocz_bit); - level = 1; - level *= eocz_mask; expire = ktime_add_ns(ktime_get(), max_delay); timeout = ktime_set(0, min_delay); @@ -140,9 +141,9 @@ static inline bool wait_for_eocz(struct omap3_temp_data *data, schedule_hrtimeout(timeout, HRTIMER_MODE_REL); do { regmap_read(data-syscon, SYSCON_TEMP_REG, temp_sensor_reg); - if ((temp_sensor_reg eocz_mask) == level) + if (!!(temp_sensor_reg eocz_mask) == level) return true; - } while (ktime_us_delta(expire, ktime_get()) 0); + } while (ktime_after(expire, ktime_get())); Does this have to be a hard loop, without sleep ? I am a bit concerned that it may loop for more than a ms. Other than that, I assume we'll see an updated version with the coding style issues and hang-up problems fixed. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hwmon: (gpio-fan) Add a shutdown handler to poweroff the fans
On Thu, Dec 04, 2014 at 10:58:56AM -0600, Nishanth Menon wrote: Poweroff the fans when shutting down the system. Else, echo '1' /sys/class/hwmon/hwmon0/fan1_target; poweroff leaves the fan running if the System power off does not drive the gpio expander which might control the fan power supply. Signed-off-by: Nishanth Menon n...@ti.com --- Applied to hwmon-next. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hwmon: (gpio-fan) Allow usage of gpio operations that may sleep
On Thu, Dec 04, 2014 at 10:58:47AM -0600, Nishanth Menon wrote: Certain I2C based GPIO expanders could be used in sleepable context, this results in: [ 115.890569] [ cut here ] [ 115.895422] WARNING: CPU: 0 PID: 1115 at drivers/gpio/gpiolib.c:1370 gpiod_set_raw_value+0x40/0x4c() [ 115.905024] Modules linked in: [ 115.908229] CPU: 0 PID: 1115 Comm: sh Tainted: GW 3.18.0-rc7-next-20141203-dirty #1 [ 115.917461] Hardware name: Generic DRA74X (Flattened Device Tree) [ 115.923876] [c0015368] (unwind_backtrace) from [c00119f4] (show_stack+0x10/0x14) [ 115.932013] [c00119f4] (show_stack) from [c05b78e8] (dump_stack+0x78/0x94) [ 115.939594] [c05b78e8] (dump_stack) from [c003de28] (warn_slowpath_common+0x7c/0xb4) [ 115.948094] [c003de28] (warn_slowpath_common) from [c003de7c] (warn_slowpath_null+0x1c/0x24) [ 115.957315] [c003de7c] (warn_slowpath_null) from [c03461e8] (gpiod_set_raw_value+0x40/0x4c) [ 115.966457] [c03461e8] (gpiod_set_raw_value) from [c04866f4] (set_fan_speed+0x4c/0x64) [ 115.975145] [c04866f4] (set_fan_speed) from [c04868a8] (set_rpm+0x98/0xac) [ 115.982742] [c04868a8] (set_rpm) from [c039fb4c] (dev_attr_store+0x18/0x24) [ 115.990426] [c039fb4c] (dev_attr_store) from [c01b0a28] (sysfs_kf_write+0x4c/0x50) [ 115.998742] [c01b0a28] (sysfs_kf_write) from [c01afe1c] (kernfs_fop_write+0xbc/0x19c) [ 116.007333] [c01afe1c] (kernfs_fop_write) from [c0148cc4] (vfs_write+0xb0/0x1a0) [ 116.015461] [c0148cc4] (vfs_write) from [c0148fbc] (SyS_write+0x44/0x84) [ 116.022881] [c0148fbc] (SyS_write) from [c000e5c0] (ret_fast_syscall+0x0/0x48) [ 116.030833] ---[ end trace 3a0b636123acab82 ]--- So, switch over to sleepable GPIO operations as there is no mandatory need for non-sleepable gpio operations in the fan driver. This allows the fan driver to be used with i2c based gpio expanders such as palmas_gpio. Signed-off-by: Nishanth Menon n...@ti.com Applied to hwmon-next. Do we need this in older kernels ? Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hwmon: (gpio-fan) Allow usage of gpio operations that may sleep
On Thu, Dec 04, 2014 at 01:46:15PM -0600, Nishanth Menon wrote: On 11:05-20141204, Guenter Roeck wrote: On Thu, Dec 04, 2014 at 10:58:47AM -0600, Nishanth Menon wrote: Certain I2C based GPIO expanders could be used in sleepable context, this results in: [ 115.890569] [ cut here ] [ 115.895422] WARNING: CPU: 0 PID: 1115 at drivers/gpio/gpiolib.c:1370 gpiod_set_raw_value+0x40/0x4c() [ 115.905024] Modules linked in: [ 115.908229] CPU: 0 PID: 1115 Comm: sh Tainted: GW 3.18.0-rc7-next-20141203-dirty #1 [ 115.917461] Hardware name: Generic DRA74X (Flattened Device Tree) [ 115.923876] [c0015368] (unwind_backtrace) from [c00119f4] (show_stack+0x10/0x14) [ 115.932013] [c00119f4] (show_stack) from [c05b78e8] (dump_stack+0x78/0x94) [ 115.939594] [c05b78e8] (dump_stack) from [c003de28] (warn_slowpath_common+0x7c/0xb4) [ 115.948094] [c003de28] (warn_slowpath_common) from [c003de7c] (warn_slowpath_null+0x1c/0x24) [ 115.957315] [c003de7c] (warn_slowpath_null) from [c03461e8] (gpiod_set_raw_value+0x40/0x4c) [ 115.966457] [c03461e8] (gpiod_set_raw_value) from [c04866f4] (set_fan_speed+0x4c/0x64) [ 115.975145] [c04866f4] (set_fan_speed) from [c04868a8] (set_rpm+0x98/0xac) [ 115.982742] [c04868a8] (set_rpm) from [c039fb4c] (dev_attr_store+0x18/0x24) [ 115.990426] [c039fb4c] (dev_attr_store) from [c01b0a28] (sysfs_kf_write+0x4c/0x50) [ 115.998742] [c01b0a28] (sysfs_kf_write) from [c01afe1c] (kernfs_fop_write+0xbc/0x19c) [ 116.007333] [c01afe1c] (kernfs_fop_write) from [c0148cc4] (vfs_write+0xb0/0x1a0) [ 116.015461] [c0148cc4] (vfs_write) from [c0148fbc] (SyS_write+0x44/0x84) [ 116.022881] [c0148fbc] (SyS_write) from [c000e5c0] (ret_fast_syscall+0x0/0x48) [ 116.030833] ---[ end trace 3a0b636123acab82 ]--- So, switch over to sleepable GPIO operations as there is no mandatory need for non-sleepable gpio operations in the fan driver. This allows the fan driver to be used with i2c based gpio expanders such as palmas_gpio. Signed-off-by: Nishanth Menon n...@ti.com Applied to hwmon-next. Do we need this in older kernels ? I have'nt had a need for it till recently.. in terms of the sleepable get/set, it seems to have been introduced with commit 7560fa60fcdcdb0da662f6a9fad9064b554ef46c Author: David Brownell davi...@pacbell.net Date: Tue Mar 4 14:28:27 2008 -0800 gpio: linux/gpio.h and no GPIO support here stubs Guessing no one needed it so far.. so probably future should be good enough, I think.. Ok. We can still add it to -stable at a later time if the need arises. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 18/48] mfd: twl4030-power: Register with kernel power-off handler
On 11/10/2014 12:46 AM, Pavel Machek wrote: Hi! @@ -611,7 +611,7 @@ twl4030_power_configure_resources(const struct twl4030_power_data *pdata) * After a successful execution, TWL shuts down the power to the SoC * and all peripherals connected to it. */ -void twl4030_power_off(void) +static void twl4030_power_off(struct power_off_handler_block *this) { int err; @@ -621,6 +621,11 @@ void twl4030_power_off(void) pr_err(TWL4030 Unable to power off\n); } +static struct power_off_handler_block twl4030_power_off_hb = { + .handler = twl4030_power_off, + .priority = POWER_OFF_PRIORITY_LOW, +}; + static bool twl4030_power_use_poweroff(const struct twl4030_power_data *pdata, struct device_node *node) { @@ -839,7 +844,9 @@ static int twl4030_power_probe(struct platform_device *pdev) } /* Board has to be wired properly to use this feature */ - if (twl4030_power_use_poweroff(pdata, node) !pm_power_off) { + if (twl4030_power_use_poweroff(pdata, node)) { + int ret; + /* Default for SEQ_OFFSYNC is set, lets ensure this */ err = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, val, TWL4030_PM_MASTER_CFG_P123_TRANSITION); @@ -856,7 +863,11 @@ static int twl4030_power_probe(struct platform_device *pdev) } } - pm_power_off = twl4030_power_off; + ret = devm_register_power_off_handler(pdev-dev, + twl4030_power_off_hb); + if (ret) + dev_warn(pdev-dev, +Failed to register power-off handler\n); } Could we get rid of the struct power_off_handler_block and guarantee that register_power_off never fails (or print message from the register_power_off...)? That way, your patch would be an cleanup. You could then add priorities if they turn out to be really neccessary, later... Priorities are necessary. We had _that_ discussion before. Priorities solve the problem where multiple handlers are installed, either conditionally or unconditionally. If I take priorities away, a substantial part of the patch set's value gets lost, and I might as well drop it. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 18/48] mfd: twl4030-power: Register with kernel power-off handler
On 11/10/2014 06:09 AM, Guenter Roeck wrote: On 11/10/2014 12:46 AM, Pavel Machek wrote: Hi! @@ -611,7 +611,7 @@ twl4030_power_configure_resources(const struct twl4030_power_data *pdata) * After a successful execution, TWL shuts down the power to the SoC * and all peripherals connected to it. */ -void twl4030_power_off(void) +static void twl4030_power_off(struct power_off_handler_block *this) { int err; @@ -621,6 +621,11 @@ void twl4030_power_off(void) pr_err(TWL4030 Unable to power off\n); } +static struct power_off_handler_block twl4030_power_off_hb = { +.handler = twl4030_power_off, +.priority = POWER_OFF_PRIORITY_LOW, +}; + static bool twl4030_power_use_poweroff(const struct twl4030_power_data *pdata, struct device_node *node) { @@ -839,7 +844,9 @@ static int twl4030_power_probe(struct platform_device *pdev) } /* Board has to be wired properly to use this feature */ -if (twl4030_power_use_poweroff(pdata, node) !pm_power_off) { +if (twl4030_power_use_poweroff(pdata, node)) { +int ret; + /* Default for SEQ_OFFSYNC is set, lets ensure this */ err = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, val, TWL4030_PM_MASTER_CFG_P123_TRANSITION); @@ -856,7 +863,11 @@ static int twl4030_power_probe(struct platform_device *pdev) } } -pm_power_off = twl4030_power_off; +ret = devm_register_power_off_handler(pdev-dev, + twl4030_power_off_hb); +if (ret) +dev_warn(pdev-dev, + Failed to register power-off handler\n); } Could we get rid of the struct power_off_handler_block and guarantee that register_power_off never fails (or print message from the register_power_off...)? That way, your patch would be an cleanup. You could then add priorities if they turn out to be really neccessary, later... Priorities are necessary. We had _that_ discussion before. Priorities solve the problem where multiple handlers are installed, either conditionally or unconditionally. If I take priorities away, a substantial part of the patch set's value gets lost, and I might as well drop it. I have an idea: Instead of dropping the priority, drop power_off_handler_block and add two parameters to register_power_off_handler and devm_register_power_off_handler instead: the priority and a context. At the same time, declare that those two functions must be called with the memory subsystem initialized (register_power_off_handler_simple must be used otherwise). With this change, the registration functions can still fail due to memory allocation errors, but we can get rid of the data structure and simplify the calling code while retaining functionality. I'll explore that for v7. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 09/48] mfd: palmas: Register with kernel power-off handler
Register with kernel power-off handler instead of setting pm_power_off directly. Register with low priority to reflect that the original code only sets pm_power_off if it was not already set. Cc: Samuel Ortiz sa...@linux.intel.com Cc: Lee Jones lee.jo...@linaro.org Cc: linux-omap@vger.kernel.org Acked-by: Lee Jones lee.jo...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- v6: - This patch: No change. Global: Replaced priority defines with enum. v5: - Rebase to v3.18-rc3 v4: - Do not use notifiers but internal functions and data structures to manage the list of power-off handlers. Drop unused parameters from callbacks, and make the power-off function type void v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use define to specify poweroff handler priority - Use devm_register_power_off_handler - Use dev_warn instead of dev_err drivers/mfd/palmas.c | 28 ++-- include/linux/mfd/palmas.h | 3 +++ 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c index 28cb048..60f81ed 100644 --- a/drivers/mfd/palmas.c +++ b/drivers/mfd/palmas.c @@ -19,6 +19,7 @@ #include linux/i2c.h #include linux/interrupt.h #include linux/irq.h +#include linux/pm.h #include linux/regmap.h #include linux/err.h #include linux/mfd/core.h @@ -425,20 +426,17 @@ static void palmas_dt_to_pdata(struct i2c_client *i2c, ti,system-power-controller); } -static struct palmas *palmas_dev; -static void palmas_power_off(void) +static void palmas_power_off(struct power_off_handler_block *this) { + struct palmas *palmas = container_of(this, struct palmas, power_off_hb); unsigned int addr; int ret, slave; - if (!palmas_dev) - return; - slave = PALMAS_BASE_TO_SLAVE(PALMAS_PMU_CONTROL_BASE); addr = PALMAS_BASE_TO_REG(PALMAS_PMU_CONTROL_BASE, PALMAS_DEV_CTRL); ret = regmap_update_bits( - palmas_dev-regmap[slave], + palmas-regmap[slave], addr, PALMAS_DEV_CTRL_DEV_ON, 0); @@ -668,9 +666,16 @@ no_irq: ret = of_platform_populate(node, NULL, NULL, i2c-dev); if (ret 0) { goto err_irq; - } else if (pdata-pm_off !pm_power_off) { - palmas_dev = palmas; - pm_power_off = palmas_power_off; + } else if (pdata-pm_off) { + int ret2; + + palmas-power_off_hb.handler = palmas_power_off; + palmas-power_off_hb.priority = POWER_OFF_PRIORITY_LOW; + ret2 = devm_register_power_off_handler(palmas-dev, + palmas-power_off_hb); + if (ret2) + dev_warn(palmas-dev, +Failed to register power-off handler); } } @@ -698,11 +703,6 @@ static int palmas_i2c_remove(struct i2c_client *i2c) i2c_unregister_device(palmas-i2c_clients[i]); } - if (palmas == palmas_dev) { - pm_power_off = NULL; - palmas_dev = NULL; - } - return 0; } diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h index fb0390a..7a43f7b 100644 --- a/include/linux/mfd/palmas.h +++ b/include/linux/mfd/palmas.h @@ -18,6 +18,7 @@ #include linux/usb/otg.h #include linux/leds.h +#include linux/pm.h #include linux/regmap.h #include linux/regulator/driver.h #include linux/extcon.h @@ -68,6 +69,8 @@ struct palmas { struct i2c_client *i2c_clients[PALMAS_NUM_CLIENTS]; struct regmap *regmap[PALMAS_NUM_CLIENTS]; + struct power_off_handler_block power_off_hb; + /* Stored chip id */ int id; -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 18/48] mfd: twl4030-power: Register with kernel power-off handler
Register with kernel power-off handler instead of setting pm_power_off directly. Register with low priority to reflect that the original code only sets pm_power_off if it was not already set. Make twl4030_power_off static as it is only called from the twl4030-power driver. Drop remove function as it is no longer needed. Cc: Samuel Ortiz sa...@linux.intel.com Cc: Lee Jones lee.jo...@linaro.org Cc: linux-omap@vger.kernel.org Acked-by: Lee Jones lee.jo...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- v6: - This patch: No change. Global: Replaced priority defines with enum. v5: - Rebase to v3.18-rc3 v4: - Do not use notifiers but internal functions and data structures to manage the list of power-off handlers. Drop unused parameters from callbacks, and make the power-off function type void v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use define to specify poweroff handler priority - Use dev_warn instead of dev_err - Use devm_register_power_off_handler - Drop remove function as it is no longer needed. drivers/mfd/twl4030-power.c | 25 +++-- include/linux/i2c/twl.h | 1 - 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c index cf92a6d..88fd33c 100644 --- a/drivers/mfd/twl4030-power.c +++ b/drivers/mfd/twl4030-power.c @@ -25,9 +25,9 @@ */ #include linux/module.h -#include linux/pm.h #include linux/i2c/twl.h #include linux/platform_device.h +#include linux/pm.h #include linux/of.h #include linux/of_device.h @@ -611,7 +611,7 @@ twl4030_power_configure_resources(const struct twl4030_power_data *pdata) * After a successful execution, TWL shuts down the power to the SoC * and all peripherals connected to it. */ -void twl4030_power_off(void) +static void twl4030_power_off(struct power_off_handler_block *this) { int err; @@ -621,6 +621,11 @@ void twl4030_power_off(void) pr_err(TWL4030 Unable to power off\n); } +static struct power_off_handler_block twl4030_power_off_hb = { + .handler = twl4030_power_off, + .priority = POWER_OFF_PRIORITY_LOW, +}; + static bool twl4030_power_use_poweroff(const struct twl4030_power_data *pdata, struct device_node *node) { @@ -839,7 +844,9 @@ static int twl4030_power_probe(struct platform_device *pdev) } /* Board has to be wired properly to use this feature */ - if (twl4030_power_use_poweroff(pdata, node) !pm_power_off) { + if (twl4030_power_use_poweroff(pdata, node)) { + int ret; + /* Default for SEQ_OFFSYNC is set, lets ensure this */ err = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, val, TWL4030_PM_MASTER_CFG_P123_TRANSITION); @@ -856,7 +863,11 @@ static int twl4030_power_probe(struct platform_device *pdev) } } - pm_power_off = twl4030_power_off; + ret = devm_register_power_off_handler(pdev-dev, + twl4030_power_off_hb); + if (ret) + dev_warn(pdev-dev, +Failed to register power-off handler\n); } relock: @@ -870,11 +881,6 @@ relock: return err; } -static int twl4030_power_remove(struct platform_device *pdev) -{ - return 0; -} - static struct platform_driver twl4030_power_driver = { .driver = { .name = twl4030_power, @@ -882,7 +888,6 @@ static struct platform_driver twl4030_power_driver = { .of_match_table = of_match_ptr(twl4030_power_of_match), }, .probe = twl4030_power_probe, - .remove = twl4030_power_remove, }; module_platform_driver(twl4030_power_driver); diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h index 8cfb50f..f8544f1 100644 --- a/include/linux/i2c/twl.h +++ b/include/linux/i2c/twl.h @@ -680,7 +680,6 @@ struct twl4030_power_data { }; extern int twl4030_remove_script(u8 flags); -extern void twl4030_power_off(void); struct twl4030_codec_data { unsigned int digimic_delay; /* in ms */ -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 14/48] mfd: tps80031: Register with kernel power-off handler
Register with kernel power-off handler instead of setting pm_power_off directly. Register with low priority to reflect that the original code only sets pm_power_off if it was not already set. Cc: Samuel Ortiz sa...@linux.intel.com Cc: Lee Jones lee.jo...@linaro.org Cc: linux-omap@vger.kernel.org Acked-by: Lee Jones lee.jo...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- v6: - This patch: No change. Global: Replaced priority defines with enum. v5: - Rebase to v3.18-rc3 v4: - Do not use notifiers but internal functions and data structures to manage the list of power-off handlers. Drop unused parameters from callbacks, and make the power-off function type void v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use define to specify poweroff handler priority - Use dev_warn instead of dev_err drivers/mfd/tps80031.c | 27 +++ include/linux/mfd/tps80031.h | 2 ++ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/mfd/tps80031.c b/drivers/mfd/tps80031.c index ed6c5b0..996fd7c 100644 --- a/drivers/mfd/tps80031.c +++ b/drivers/mfd/tps80031.c @@ -147,7 +147,6 @@ static const struct tps80031_pupd_data tps80031_pupds[] = { [TPS80031_CTLI2C_SCL] = PUPD_DATA(4, 0, BIT(2)), [TPS80031_CTLI2C_SDA] = PUPD_DATA(4, 0, BIT(3)), }; -static struct tps80031 *tps80031_power_off_dev; int tps80031_ext_power_req_config(struct device *dev, unsigned long ext_ctrl_flag, int preq_bit, @@ -209,11 +208,14 @@ int tps80031_ext_power_req_config(struct device *dev, } EXPORT_SYMBOL_GPL(tps80031_ext_power_req_config); -static void tps80031_power_off(void) +static void tps80031_power_off(struct power_off_handler_block *this) { - dev_info(tps80031_power_off_dev-dev, switching off PMU\n); - tps80031_write(tps80031_power_off_dev-dev, TPS80031_SLAVE_ID1, - TPS80031_PHOENIX_DEV_ON, TPS80031_DEVOFF); + struct tps80031 *tps80031 = container_of(this, struct tps80031, +power_off_hb); + + dev_info(tps80031-dev, switching off PMU\n); + tps80031_write(tps80031-dev, TPS80031_SLAVE_ID1, + TPS80031_PHOENIX_DEV_ON, TPS80031_DEVOFF); } static void tps80031_pupd_init(struct tps80031 *tps80031, @@ -501,9 +503,13 @@ static int tps80031_probe(struct i2c_client *client, goto fail_mfd_add; } - if (pdata-use_power_off !pm_power_off) { - tps80031_power_off_dev = tps80031; - pm_power_off = tps80031_power_off; + if (pdata-use_power_off) { + tps80031-power_off_hb.handler = tps80031_power_off; + tps80031-power_off_hb.priority = POWER_OFF_PRIORITY_LOW; + ret = register_power_off_handler(tps80031-power_off_hb); + if (ret) + dev_warn(client-dev, +Failed to register power-off handler\n); } return 0; @@ -523,10 +529,7 @@ static int tps80031_remove(struct i2c_client *client) struct tps80031 *tps80031 = i2c_get_clientdata(client); int i; - if (tps80031_power_off_dev == tps80031) { - tps80031_power_off_dev = NULL; - pm_power_off = NULL; - } + unregister_power_off_handler(tps80031-power_off_hb); mfd_remove_devices(tps80031-dev); diff --git a/include/linux/mfd/tps80031.h b/include/linux/mfd/tps80031.h index 2c75c9c..5716077 100644 --- a/include/linux/mfd/tps80031.h +++ b/include/linux/mfd/tps80031.h @@ -24,6 +24,7 @@ #define __LINUX_MFD_TPS80031_H #include linux/device.h +#include linux/pm.h #include linux/regmap.h /* Pull-ups/Pull-downs */ @@ -513,6 +514,7 @@ struct tps80031 { struct i2c_client *clients[TPS80031_NUM_SLAVES]; struct regmap *regmap[TPS80031_NUM_SLAVES]; struct regmap_irq_chip_data *irq_data; + struct power_off_handler_block power_off_hb; }; struct tps80031_pupd_init_data { -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 35/48] arm: Register with kernel power-off handler
Register with kernel power-off handler instead of setting pm_power_off directly. Always use register_power_off_handler_simple as there is no indication that more than one power-off handler is registered. If the power-off handler only resets the system or puts the CPU in sleep mode, select the fallback priority to indicate that the power-off handler is one of last resort. If the power-off handler powers off the system, select the default priority. Cc: Russell King li...@arm.linux.org.uk Signed-off-by: Guenter Roeck li...@roeck-us.net --- v6: - This patch: No change. Global: Replaced priority defines with enum. v5: - Rebase to v3.18-rc3 v4: - No change v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use defines to specify poweroff handler priorities - Drop changes in arch/arm/mach-at91/setup.c (file removed upstream) arch/arm/kernel/psci.c | 3 ++- arch/arm/mach-at91/board-gsia18s.c | 3 ++- arch/arm/mach-bcm/board_bcm2835.c | 3 ++- arch/arm/mach-cns3xxx/cns3420vb.c | 3 ++- arch/arm/mach-cns3xxx/core.c | 3 ++- arch/arm/mach-highbank/highbank.c | 3 ++- arch/arm/mach-imx/mach-mx31moboard.c | 3 ++- arch/arm/mach-iop32x/em7210.c | 3 ++- arch/arm/mach-iop32x/glantank.c| 3 ++- arch/arm/mach-iop32x/iq31244.c | 3 ++- arch/arm/mach-iop32x/n2100.c | 3 ++- arch/arm/mach-ixp4xx/dsmg600-setup.c | 3 ++- arch/arm/mach-ixp4xx/nas100d-setup.c | 3 ++- arch/arm/mach-ixp4xx/nslu2-setup.c | 3 ++- arch/arm/mach-omap2/board-omap3touchbook.c | 3 ++- arch/arm/mach-orion5x/board-mss2.c | 3 ++- arch/arm/mach-orion5x/dns323-setup.c | 9 ++--- arch/arm/mach-orion5x/kurobox_pro-setup.c | 3 ++- arch/arm/mach-orion5x/ls-chl-setup.c | 3 ++- arch/arm/mach-orion5x/ls_hgl-setup.c | 3 ++- arch/arm/mach-orion5x/lsmini-setup.c | 3 ++- arch/arm/mach-orion5x/mv2120-setup.c | 3 ++- arch/arm/mach-orion5x/net2big-setup.c | 3 ++- arch/arm/mach-orion5x/terastation_pro2-setup.c | 3 ++- arch/arm/mach-orion5x/ts209-setup.c| 3 ++- arch/arm/mach-orion5x/ts409-setup.c| 3 ++- arch/arm/mach-pxa/corgi.c | 3 ++- arch/arm/mach-pxa/mioa701.c| 3 ++- arch/arm/mach-pxa/poodle.c | 3 ++- arch/arm/mach-pxa/spitz.c | 3 ++- arch/arm/mach-pxa/tosa.c | 3 ++- arch/arm/mach-pxa/viper.c | 3 ++- arch/arm/mach-pxa/z2.c | 7 --- arch/arm/mach-pxa/zeus.c | 7 --- arch/arm/mach-s3c24xx/mach-gta02.c | 3 ++- arch/arm/mach-s3c24xx/mach-jive.c | 3 ++- arch/arm/mach-s3c24xx/mach-vr1000.c| 3 ++- arch/arm/mach-s3c64xx/mach-smartq.c| 3 ++- arch/arm/mach-sa1100/generic.c | 3 ++- arch/arm/mach-sa1100/simpad.c | 3 ++- arch/arm/mach-u300/regulator.c | 3 ++- arch/arm/mach-vt8500/vt8500.c | 3 ++- arch/arm/xen/enlighten.c | 3 ++- 43 files changed, 94 insertions(+), 49 deletions(-) diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c index f73891b..a7a2b4a 100644 --- a/arch/arm/kernel/psci.c +++ b/arch/arm/kernel/psci.c @@ -264,7 +264,8 @@ static int psci_0_2_init(struct device_node *np) arm_pm_restart = psci_sys_reset; - pm_power_off = psci_sys_poweroff; + register_power_off_handler_simple(psci_sys_poweroff, + POWER_OFF_PRIORITY_DEFAULT); out_put_node: of_node_put(np); diff --git a/arch/arm/mach-at91/board-gsia18s.c b/arch/arm/mach-at91/board-gsia18s.c index bf5cc55..e628c4a 100644 --- a/arch/arm/mach-at91/board-gsia18s.c +++ b/arch/arm/mach-at91/board-gsia18s.c @@ -521,7 +521,8 @@ static void gsia18s_power_off(void) static int __init gsia18s_power_off_init(void) { - pm_power_off = gsia18s_power_off; + register_power_off_handler_simple(gsia18s_power_off, + POWER_OFF_PRIORITY_DEFAULT); return 0; } diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c index 70f2f39..1d75c76 100644 --- a/arch/arm/mach-bcm/board_bcm2835.c +++ b/arch/arm/mach-bcm/board_bcm2835.c @@ -111,7 +111,8 @@ static void __init bcm2835_init(void) bcm2835_setup_restart(); if (wdt_regs) - pm_power_off = bcm2835_power_off; + register_power_off_handler_simple(bcm2835_power_off, + POWER_OFF_PRIORITY_FALLBACK); bcm2835_init_clocks(); diff --git a/arch/arm/mach
[PATCH v6 17/48] mfd: tps65910: Register with kernel power-off handler
Register with kernel power-off handler instead of setting pm_power_off directly. Register with low priority to reflect that the original code only sets pm_power_off if it was not already set. Cc: Samuel Ortiz sa...@linux.intel.com Cc: Lee Jones lee.jo...@linaro.org Cc: linux-omap@vger.kernel.org Acked-by: Lee Jones lee.jo...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- v6: - This patch: No change. Global: Replaced priority defines with enum. v5: - Rebase to v3.18-rc3 v4: - Do not use notifiers but internal functions and data structures to manage the list of power-off handlers. Drop unused parameters from callbacks, and make the power-off function type void v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use define to specify poweroff handler priority - Use dev_warn instead of dev_err drivers/mfd/tps65910.c | 21 + include/linux/mfd/tps65910.h | 4 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c index 7612d89..b312b92 100644 --- a/drivers/mfd/tps65910.c +++ b/drivers/mfd/tps65910.c @@ -23,6 +23,7 @@ #include linux/irq.h #include linux/irqdomain.h #include linux/mfd/core.h +#include linux/pm.h #include linux/regmap.h #include linux/mfd/tps65910.h #include linux/of.h @@ -437,12 +438,10 @@ struct tps65910_board *tps65910_parse_dt(struct i2c_client *client, } #endif -static struct i2c_client *tps65910_i2c_client; -static void tps65910_power_off(void) +static void tps65910_power_off(struct power_off_handler_block *this) { - struct tps65910 *tps65910; - - tps65910 = dev_get_drvdata(tps65910_i2c_client-dev); + struct tps65910 *tps65910 = container_of(this, struct tps65910, +power_off_hb); if (tps65910_reg_set_bits(tps65910, TPS65910_DEVCTRL, DEVCTRL_PWR_OFF_MASK) 0) @@ -505,9 +504,13 @@ static int tps65910_i2c_probe(struct i2c_client *i2c, tps65910_ck32k_init(tps65910, pmic_plat_data); tps65910_sleepinit(tps65910, pmic_plat_data); - if (pmic_plat_data-pm_off !pm_power_off) { - tps65910_i2c_client = i2c; - pm_power_off = tps65910_power_off; + if (pmic_plat_data-pm_off) { + tps65910-power_off_hb.handler = tps65910_power_off; + tps65910-power_off_hb.priority = POWER_OFF_PRIORITY_LOW; + ret = register_power_off_handler(tps65910-power_off_hb); + if (ret) + dev_warn(i2c-dev, +failed to register power-off handler\n); } ret = mfd_add_devices(tps65910-dev, -1, @@ -527,6 +530,8 @@ static int tps65910_i2c_remove(struct i2c_client *i2c) { struct tps65910 *tps65910 = i2c_get_clientdata(i2c); + unregister_power_off_handler(tps65910-power_off_hb); + tps65910_irq_exit(tps65910); mfd_remove_devices(tps65910-dev); diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h index 6483a6f..a85ad0f 100644 --- a/include/linux/mfd/tps65910.h +++ b/include/linux/mfd/tps65910.h @@ -18,6 +18,7 @@ #define __LINUX_MFD_TPS65910_H #include linux/gpio.h +#include linux/pm.h #include linux/regmap.h /* TPS chip id list */ @@ -905,6 +906,9 @@ struct tps65910 { /* IRQ Handling */ int chip_irq; struct regmap_irq_chip_data *irq_data; + + /* Power-off handling */ + struct power_off_handler_block power_off_hb; }; struct tps65910_platform_data { -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 16/48] mfd: tps6586x: Register with kernel power-off handler
Register with kernel power-off handler instead of setting pm_power_off directly. Register with low priority to reflect that the original code only sets pm_power_off if it was not already set. Cc: Samuel Ortiz sa...@linux.intel.com Cc: Lee Jones lee.jo...@linaro.org Cc: linux-omap@vger.kernel.org Acked-by: Lee Jones lee.jo...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- v6: - This patch: No change. Global: Replaced priority defines with enum. v5: - Rebase to v3.18-rc3 v4: - Do not use notifiers but internal functions and data structures to manage the list of power-off handlers. Drop unused parameters from callbacks, and make the power-off function type void v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use define to specify poweroff handler priority - Use dev_warn instead of dev_err drivers/mfd/tps6586x.c | 25 ++--- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c index 8e1dbc4..f11165f 100644 --- a/drivers/mfd/tps6586x.c +++ b/drivers/mfd/tps6586x.c @@ -25,6 +25,7 @@ #include linux/err.h #include linux/i2c.h #include linux/platform_device.h +#include linux/pm.h #include linux/regmap.h #include linux/of.h @@ -133,6 +134,8 @@ struct tps6586x { u32 irq_en; u8 mask_reg[5]; struct irq_domain *irq_domain; + + struct power_off_handler_block power_off_hb; }; static inline struct tps6586x *dev_to_tps6586x(struct device *dev) @@ -472,13 +475,15 @@ static const struct regmap_config tps6586x_regmap_config = { .cache_type = REGCACHE_RBTREE, }; -static struct device *tps6586x_dev; -static void tps6586x_power_off(void) +static void tps6586x_power_off(struct power_off_handler_block *this) { - if (tps6586x_clr_bits(tps6586x_dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT)) + struct tps6586x *tps6586x = container_of(this, struct tps6586x, +power_off_hb); + + if (tps6586x_clr_bits(tps6586x-dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT)) return; - tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT); + tps6586x_set_bits(tps6586x-dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT); } static void tps6586x_print_version(struct i2c_client *client, int version) @@ -575,9 +580,13 @@ static int tps6586x_i2c_probe(struct i2c_client *client, goto err_add_devs; } - if (pdata-pm_off !pm_power_off) { - tps6586x_dev = client-dev; - pm_power_off = tps6586x_power_off; + if (pdata-pm_off) { + tps6586x-power_off_hb.handler = tps6586x_power_off; + tps6586x-power_off_hb.priority = POWER_OFF_PRIORITY_LOW; + ret = register_power_off_handler(tps6586x-power_off_hb); + if (ret) + dev_warn(client-dev, +failed to register power-off handler\n); } return 0; @@ -594,6 +603,8 @@ static int tps6586x_i2c_remove(struct i2c_client *client) { struct tps6586x *tps6586x = i2c_get_clientdata(client); + unregister_power_off_handler(tps6586x-power_off_hb); + tps6586x_remove_subdevs(tps6586x); mfd_remove_devices(tps6586x-dev); if (client-irq) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 09/48] mfd: palmas: Register with kernel power-off handler
Register with kernel power-off handler instead of setting pm_power_off directly. Register with low priority to reflect that the original code only sets pm_power_off if it was not already set. Cc: Samuel Ortiz sa...@linux.intel.com Cc: Lee Jones lee.jo...@linaro.org Cc: linux-omap@vger.kernel.org Acked-by: Lee Jones lee.jo...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- v5: - Rebase to v3.18-rc3 v4: - Do not use notifiers but internal functions and data structures to manage the list of power-off handlers. Drop unused parameters from callbacks, and make the power-off function type void v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use define to specify poweroff handler priority - Use devm_register_power_off_handler - Use dev_warn instead of dev_err drivers/mfd/palmas.c | 28 ++-- include/linux/mfd/palmas.h | 3 +++ 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/drivers/mfd/palmas.c b/drivers/mfd/palmas.c index 28cb048..60f81ed 100644 --- a/drivers/mfd/palmas.c +++ b/drivers/mfd/palmas.c @@ -19,6 +19,7 @@ #include linux/i2c.h #include linux/interrupt.h #include linux/irq.h +#include linux/pm.h #include linux/regmap.h #include linux/err.h #include linux/mfd/core.h @@ -425,20 +426,17 @@ static void palmas_dt_to_pdata(struct i2c_client *i2c, ti,system-power-controller); } -static struct palmas *palmas_dev; -static void palmas_power_off(void) +static void palmas_power_off(struct power_off_handler_block *this) { + struct palmas *palmas = container_of(this, struct palmas, power_off_hb); unsigned int addr; int ret, slave; - if (!palmas_dev) - return; - slave = PALMAS_BASE_TO_SLAVE(PALMAS_PMU_CONTROL_BASE); addr = PALMAS_BASE_TO_REG(PALMAS_PMU_CONTROL_BASE, PALMAS_DEV_CTRL); ret = regmap_update_bits( - palmas_dev-regmap[slave], + palmas-regmap[slave], addr, PALMAS_DEV_CTRL_DEV_ON, 0); @@ -668,9 +666,16 @@ no_irq: ret = of_platform_populate(node, NULL, NULL, i2c-dev); if (ret 0) { goto err_irq; - } else if (pdata-pm_off !pm_power_off) { - palmas_dev = palmas; - pm_power_off = palmas_power_off; + } else if (pdata-pm_off) { + int ret2; + + palmas-power_off_hb.handler = palmas_power_off; + palmas-power_off_hb.priority = POWER_OFF_PRIORITY_LOW; + ret2 = devm_register_power_off_handler(palmas-dev, + palmas-power_off_hb); + if (ret2) + dev_warn(palmas-dev, +Failed to register power-off handler); } } @@ -698,11 +703,6 @@ static int palmas_i2c_remove(struct i2c_client *i2c) i2c_unregister_device(palmas-i2c_clients[i]); } - if (palmas == palmas_dev) { - pm_power_off = NULL; - palmas_dev = NULL; - } - return 0; } diff --git a/include/linux/mfd/palmas.h b/include/linux/mfd/palmas.h index fb0390a..7a43f7b 100644 --- a/include/linux/mfd/palmas.h +++ b/include/linux/mfd/palmas.h @@ -18,6 +18,7 @@ #include linux/usb/otg.h #include linux/leds.h +#include linux/pm.h #include linux/regmap.h #include linux/regulator/driver.h #include linux/extcon.h @@ -68,6 +69,8 @@ struct palmas { struct i2c_client *i2c_clients[PALMAS_NUM_CLIENTS]; struct regmap *regmap[PALMAS_NUM_CLIENTS]; + struct power_off_handler_block power_off_hb; + /* Stored chip id */ int id; -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 35/48] arm: Register with kernel power-off handler
Register with kernel power-off handler instead of setting pm_power_off directly. Always use register_power_off_handler_simple as there is no indication that more than one power-off handler is registered. If the power-off handler only resets the system or puts the CPU in sleep mode, select the fallback priority to indicate that the power-off handler is one of last resort. If the power-off handler powers off the system, select the default priority. Cc: Russell King li...@arm.linux.org.uk Signed-off-by: Guenter Roeck li...@roeck-us.net --- v5: - Rebase to v3.18-rc3 v4: - No change v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use defines to specify poweroff handler priorities - Drop changes in arch/arm/mach-at91/setup.c (file removed upstream) arch/arm/kernel/psci.c | 3 ++- arch/arm/mach-at91/board-gsia18s.c | 3 ++- arch/arm/mach-bcm/board_bcm2835.c | 3 ++- arch/arm/mach-cns3xxx/cns3420vb.c | 3 ++- arch/arm/mach-cns3xxx/core.c | 3 ++- arch/arm/mach-highbank/highbank.c | 3 ++- arch/arm/mach-imx/mach-mx31moboard.c | 3 ++- arch/arm/mach-iop32x/em7210.c | 3 ++- arch/arm/mach-iop32x/glantank.c| 3 ++- arch/arm/mach-iop32x/iq31244.c | 3 ++- arch/arm/mach-iop32x/n2100.c | 3 ++- arch/arm/mach-ixp4xx/dsmg600-setup.c | 3 ++- arch/arm/mach-ixp4xx/nas100d-setup.c | 3 ++- arch/arm/mach-ixp4xx/nslu2-setup.c | 3 ++- arch/arm/mach-omap2/board-omap3touchbook.c | 3 ++- arch/arm/mach-orion5x/board-mss2.c | 3 ++- arch/arm/mach-orion5x/dns323-setup.c | 9 ++--- arch/arm/mach-orion5x/kurobox_pro-setup.c | 3 ++- arch/arm/mach-orion5x/ls-chl-setup.c | 3 ++- arch/arm/mach-orion5x/ls_hgl-setup.c | 3 ++- arch/arm/mach-orion5x/lsmini-setup.c | 3 ++- arch/arm/mach-orion5x/mv2120-setup.c | 3 ++- arch/arm/mach-orion5x/net2big-setup.c | 3 ++- arch/arm/mach-orion5x/terastation_pro2-setup.c | 3 ++- arch/arm/mach-orion5x/ts209-setup.c| 3 ++- arch/arm/mach-orion5x/ts409-setup.c| 3 ++- arch/arm/mach-pxa/corgi.c | 3 ++- arch/arm/mach-pxa/mioa701.c| 3 ++- arch/arm/mach-pxa/poodle.c | 3 ++- arch/arm/mach-pxa/spitz.c | 3 ++- arch/arm/mach-pxa/tosa.c | 3 ++- arch/arm/mach-pxa/viper.c | 3 ++- arch/arm/mach-pxa/z2.c | 7 --- arch/arm/mach-pxa/zeus.c | 7 --- arch/arm/mach-s3c24xx/mach-gta02.c | 3 ++- arch/arm/mach-s3c24xx/mach-jive.c | 3 ++- arch/arm/mach-s3c24xx/mach-vr1000.c| 3 ++- arch/arm/mach-s3c64xx/mach-smartq.c| 3 ++- arch/arm/mach-sa1100/generic.c | 3 ++- arch/arm/mach-sa1100/simpad.c | 3 ++- arch/arm/mach-u300/regulator.c | 3 ++- arch/arm/mach-vt8500/vt8500.c | 3 ++- arch/arm/xen/enlighten.c | 3 ++- 43 files changed, 94 insertions(+), 49 deletions(-) diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c index f73891b..a7a2b4a 100644 --- a/arch/arm/kernel/psci.c +++ b/arch/arm/kernel/psci.c @@ -264,7 +264,8 @@ static int psci_0_2_init(struct device_node *np) arm_pm_restart = psci_sys_reset; - pm_power_off = psci_sys_poweroff; + register_power_off_handler_simple(psci_sys_poweroff, + POWER_OFF_PRIORITY_DEFAULT); out_put_node: of_node_put(np); diff --git a/arch/arm/mach-at91/board-gsia18s.c b/arch/arm/mach-at91/board-gsia18s.c index bf5cc55..e628c4a 100644 --- a/arch/arm/mach-at91/board-gsia18s.c +++ b/arch/arm/mach-at91/board-gsia18s.c @@ -521,7 +521,8 @@ static void gsia18s_power_off(void) static int __init gsia18s_power_off_init(void) { - pm_power_off = gsia18s_power_off; + register_power_off_handler_simple(gsia18s_power_off, + POWER_OFF_PRIORITY_DEFAULT); return 0; } diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c index 70f2f39..1d75c76 100644 --- a/arch/arm/mach-bcm/board_bcm2835.c +++ b/arch/arm/mach-bcm/board_bcm2835.c @@ -111,7 +111,8 @@ static void __init bcm2835_init(void) bcm2835_setup_restart(); if (wdt_regs) - pm_power_off = bcm2835_power_off; + register_power_off_handler_simple(bcm2835_power_off, + POWER_OFF_PRIORITY_FALLBACK); bcm2835_init_clocks(); diff --git a/arch/arm/mach-cns3xxx/cns3420vb.c b/arch/arm/mach-cns3xxx/cns3420vb.c index 6428bcc7..5c50461
[PATCH v5 18/48] mfd: twl4030-power: Register with kernel power-off handler
Register with kernel power-off handler instead of setting pm_power_off directly. Register with low priority to reflect that the original code only sets pm_power_off if it was not already set. Make twl4030_power_off static as it is only called from the twl4030-power driver. Drop remove function as it is no longer needed. Cc: Samuel Ortiz sa...@linux.intel.com Cc: Lee Jones lee.jo...@linaro.org Cc: linux-omap@vger.kernel.org Acked-by: Lee Jones lee.jo...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- v5: - Rebase to v3.18-rc3 v4: - Do not use notifiers but internal functions and data structures to manage the list of power-off handlers. Drop unused parameters from callbacks, and make the power-off function type void v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use define to specify poweroff handler priority - Use dev_warn instead of dev_err - Use devm_register_power_off_handler - Drop remove function as it is no longer needed. drivers/mfd/twl4030-power.c | 25 +++-- include/linux/i2c/twl.h | 1 - 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/drivers/mfd/twl4030-power.c b/drivers/mfd/twl4030-power.c index cf92a6d..88fd33c 100644 --- a/drivers/mfd/twl4030-power.c +++ b/drivers/mfd/twl4030-power.c @@ -25,9 +25,9 @@ */ #include linux/module.h -#include linux/pm.h #include linux/i2c/twl.h #include linux/platform_device.h +#include linux/pm.h #include linux/of.h #include linux/of_device.h @@ -611,7 +611,7 @@ twl4030_power_configure_resources(const struct twl4030_power_data *pdata) * After a successful execution, TWL shuts down the power to the SoC * and all peripherals connected to it. */ -void twl4030_power_off(void) +static void twl4030_power_off(struct power_off_handler_block *this) { int err; @@ -621,6 +621,11 @@ void twl4030_power_off(void) pr_err(TWL4030 Unable to power off\n); } +static struct power_off_handler_block twl4030_power_off_hb = { + .handler = twl4030_power_off, + .priority = POWER_OFF_PRIORITY_LOW, +}; + static bool twl4030_power_use_poweroff(const struct twl4030_power_data *pdata, struct device_node *node) { @@ -839,7 +844,9 @@ static int twl4030_power_probe(struct platform_device *pdev) } /* Board has to be wired properly to use this feature */ - if (twl4030_power_use_poweroff(pdata, node) !pm_power_off) { + if (twl4030_power_use_poweroff(pdata, node)) { + int ret; + /* Default for SEQ_OFFSYNC is set, lets ensure this */ err = twl_i2c_read_u8(TWL_MODULE_PM_MASTER, val, TWL4030_PM_MASTER_CFG_P123_TRANSITION); @@ -856,7 +863,11 @@ static int twl4030_power_probe(struct platform_device *pdev) } } - pm_power_off = twl4030_power_off; + ret = devm_register_power_off_handler(pdev-dev, + twl4030_power_off_hb); + if (ret) + dev_warn(pdev-dev, +Failed to register power-off handler\n); } relock: @@ -870,11 +881,6 @@ relock: return err; } -static int twl4030_power_remove(struct platform_device *pdev) -{ - return 0; -} - static struct platform_driver twl4030_power_driver = { .driver = { .name = twl4030_power, @@ -882,7 +888,6 @@ static struct platform_driver twl4030_power_driver = { .of_match_table = of_match_ptr(twl4030_power_of_match), }, .probe = twl4030_power_probe, - .remove = twl4030_power_remove, }; module_platform_driver(twl4030_power_driver); diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h index 8cfb50f..f8544f1 100644 --- a/include/linux/i2c/twl.h +++ b/include/linux/i2c/twl.h @@ -680,7 +680,6 @@ struct twl4030_power_data { }; extern int twl4030_remove_script(u8 flags); -extern void twl4030_power_off(void); struct twl4030_codec_data { unsigned int digimic_delay; /* in ms */ -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 14/48] mfd: tps80031: Register with kernel power-off handler
Register with kernel power-off handler instead of setting pm_power_off directly. Register with low priority to reflect that the original code only sets pm_power_off if it was not already set. Cc: Samuel Ortiz sa...@linux.intel.com Cc: Lee Jones lee.jo...@linaro.org Cc: linux-omap@vger.kernel.org Acked-by: Lee Jones lee.jo...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- v5: - Rebase to v3.18-rc3 v4: - Do not use notifiers but internal functions and data structures to manage the list of power-off handlers. Drop unused parameters from callbacks, and make the power-off function type void v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use define to specify poweroff handler priority - Use dev_warn instead of dev_err drivers/mfd/tps80031.c | 27 +++ include/linux/mfd/tps80031.h | 2 ++ 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/drivers/mfd/tps80031.c b/drivers/mfd/tps80031.c index ed6c5b0..996fd7c 100644 --- a/drivers/mfd/tps80031.c +++ b/drivers/mfd/tps80031.c @@ -147,7 +147,6 @@ static const struct tps80031_pupd_data tps80031_pupds[] = { [TPS80031_CTLI2C_SCL] = PUPD_DATA(4, 0, BIT(2)), [TPS80031_CTLI2C_SDA] = PUPD_DATA(4, 0, BIT(3)), }; -static struct tps80031 *tps80031_power_off_dev; int tps80031_ext_power_req_config(struct device *dev, unsigned long ext_ctrl_flag, int preq_bit, @@ -209,11 +208,14 @@ int tps80031_ext_power_req_config(struct device *dev, } EXPORT_SYMBOL_GPL(tps80031_ext_power_req_config); -static void tps80031_power_off(void) +static void tps80031_power_off(struct power_off_handler_block *this) { - dev_info(tps80031_power_off_dev-dev, switching off PMU\n); - tps80031_write(tps80031_power_off_dev-dev, TPS80031_SLAVE_ID1, - TPS80031_PHOENIX_DEV_ON, TPS80031_DEVOFF); + struct tps80031 *tps80031 = container_of(this, struct tps80031, +power_off_hb); + + dev_info(tps80031-dev, switching off PMU\n); + tps80031_write(tps80031-dev, TPS80031_SLAVE_ID1, + TPS80031_PHOENIX_DEV_ON, TPS80031_DEVOFF); } static void tps80031_pupd_init(struct tps80031 *tps80031, @@ -501,9 +503,13 @@ static int tps80031_probe(struct i2c_client *client, goto fail_mfd_add; } - if (pdata-use_power_off !pm_power_off) { - tps80031_power_off_dev = tps80031; - pm_power_off = tps80031_power_off; + if (pdata-use_power_off) { + tps80031-power_off_hb.handler = tps80031_power_off; + tps80031-power_off_hb.priority = POWER_OFF_PRIORITY_LOW; + ret = register_power_off_handler(tps80031-power_off_hb); + if (ret) + dev_warn(client-dev, +Failed to register power-off handler\n); } return 0; @@ -523,10 +529,7 @@ static int tps80031_remove(struct i2c_client *client) struct tps80031 *tps80031 = i2c_get_clientdata(client); int i; - if (tps80031_power_off_dev == tps80031) { - tps80031_power_off_dev = NULL; - pm_power_off = NULL; - } + unregister_power_off_handler(tps80031-power_off_hb); mfd_remove_devices(tps80031-dev); diff --git a/include/linux/mfd/tps80031.h b/include/linux/mfd/tps80031.h index 2c75c9c..5716077 100644 --- a/include/linux/mfd/tps80031.h +++ b/include/linux/mfd/tps80031.h @@ -24,6 +24,7 @@ #define __LINUX_MFD_TPS80031_H #include linux/device.h +#include linux/pm.h #include linux/regmap.h /* Pull-ups/Pull-downs */ @@ -513,6 +514,7 @@ struct tps80031 { struct i2c_client *clients[TPS80031_NUM_SLAVES]; struct regmap *regmap[TPS80031_NUM_SLAVES]; struct regmap_irq_chip_data *irq_data; + struct power_off_handler_block power_off_hb; }; struct tps80031_pupd_init_data { -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 17/48] mfd: tps65910: Register with kernel power-off handler
Register with kernel power-off handler instead of setting pm_power_off directly. Register with low priority to reflect that the original code only sets pm_power_off if it was not already set. Cc: Samuel Ortiz sa...@linux.intel.com Cc: Lee Jones lee.jo...@linaro.org Cc: linux-omap@vger.kernel.org Acked-by: Lee Jones lee.jo...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- v5: - Rebase to v3.18-rc3 v4: - Do not use notifiers but internal functions and data structures to manage the list of power-off handlers. Drop unused parameters from callbacks, and make the power-off function type void v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use define to specify poweroff handler priority - Use dev_warn instead of dev_err drivers/mfd/tps65910.c | 21 + include/linux/mfd/tps65910.h | 4 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c index 7612d89..b312b92 100644 --- a/drivers/mfd/tps65910.c +++ b/drivers/mfd/tps65910.c @@ -23,6 +23,7 @@ #include linux/irq.h #include linux/irqdomain.h #include linux/mfd/core.h +#include linux/pm.h #include linux/regmap.h #include linux/mfd/tps65910.h #include linux/of.h @@ -437,12 +438,10 @@ struct tps65910_board *tps65910_parse_dt(struct i2c_client *client, } #endif -static struct i2c_client *tps65910_i2c_client; -static void tps65910_power_off(void) +static void tps65910_power_off(struct power_off_handler_block *this) { - struct tps65910 *tps65910; - - tps65910 = dev_get_drvdata(tps65910_i2c_client-dev); + struct tps65910 *tps65910 = container_of(this, struct tps65910, +power_off_hb); if (tps65910_reg_set_bits(tps65910, TPS65910_DEVCTRL, DEVCTRL_PWR_OFF_MASK) 0) @@ -505,9 +504,13 @@ static int tps65910_i2c_probe(struct i2c_client *i2c, tps65910_ck32k_init(tps65910, pmic_plat_data); tps65910_sleepinit(tps65910, pmic_plat_data); - if (pmic_plat_data-pm_off !pm_power_off) { - tps65910_i2c_client = i2c; - pm_power_off = tps65910_power_off; + if (pmic_plat_data-pm_off) { + tps65910-power_off_hb.handler = tps65910_power_off; + tps65910-power_off_hb.priority = POWER_OFF_PRIORITY_LOW; + ret = register_power_off_handler(tps65910-power_off_hb); + if (ret) + dev_warn(i2c-dev, +failed to register power-off handler\n); } ret = mfd_add_devices(tps65910-dev, -1, @@ -527,6 +530,8 @@ static int tps65910_i2c_remove(struct i2c_client *i2c) { struct tps65910 *tps65910 = i2c_get_clientdata(i2c); + unregister_power_off_handler(tps65910-power_off_hb); + tps65910_irq_exit(tps65910); mfd_remove_devices(tps65910-dev); diff --git a/include/linux/mfd/tps65910.h b/include/linux/mfd/tps65910.h index 6483a6f..a85ad0f 100644 --- a/include/linux/mfd/tps65910.h +++ b/include/linux/mfd/tps65910.h @@ -18,6 +18,7 @@ #define __LINUX_MFD_TPS65910_H #include linux/gpio.h +#include linux/pm.h #include linux/regmap.h /* TPS chip id list */ @@ -905,6 +906,9 @@ struct tps65910 { /* IRQ Handling */ int chip_irq; struct regmap_irq_chip_data *irq_data; + + /* Power-off handling */ + struct power_off_handler_block power_off_hb; }; struct tps65910_platform_data { -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 16/48] mfd: tps6586x: Register with kernel power-off handler
Register with kernel power-off handler instead of setting pm_power_off directly. Register with low priority to reflect that the original code only sets pm_power_off if it was not already set. Cc: Samuel Ortiz sa...@linux.intel.com Cc: Lee Jones lee.jo...@linaro.org Cc: linux-omap@vger.kernel.org Acked-by: Lee Jones lee.jo...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- v5: - Rebase to v3.18-rc3 v4: - Do not use notifiers but internal functions and data structures to manage the list of power-off handlers. Drop unused parameters from callbacks, and make the power-off function type void v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use define to specify poweroff handler priority - Use dev_warn instead of dev_err drivers/mfd/tps6586x.c | 25 ++--- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c index 8e1dbc4..f11165f 100644 --- a/drivers/mfd/tps6586x.c +++ b/drivers/mfd/tps6586x.c @@ -25,6 +25,7 @@ #include linux/err.h #include linux/i2c.h #include linux/platform_device.h +#include linux/pm.h #include linux/regmap.h #include linux/of.h @@ -133,6 +134,8 @@ struct tps6586x { u32 irq_en; u8 mask_reg[5]; struct irq_domain *irq_domain; + + struct power_off_handler_block power_off_hb; }; static inline struct tps6586x *dev_to_tps6586x(struct device *dev) @@ -472,13 +475,15 @@ static const struct regmap_config tps6586x_regmap_config = { .cache_type = REGCACHE_RBTREE, }; -static struct device *tps6586x_dev; -static void tps6586x_power_off(void) +static void tps6586x_power_off(struct power_off_handler_block *this) { - if (tps6586x_clr_bits(tps6586x_dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT)) + struct tps6586x *tps6586x = container_of(this, struct tps6586x, +power_off_hb); + + if (tps6586x_clr_bits(tps6586x-dev, TPS6586X_SUPPLYENE, EXITSLREQ_BIT)) return; - tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT); + tps6586x_set_bits(tps6586x-dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT); } static void tps6586x_print_version(struct i2c_client *client, int version) @@ -575,9 +580,13 @@ static int tps6586x_i2c_probe(struct i2c_client *client, goto err_add_devs; } - if (pdata-pm_off !pm_power_off) { - tps6586x_dev = client-dev; - pm_power_off = tps6586x_power_off; + if (pdata-pm_off) { + tps6586x-power_off_hb.handler = tps6586x_power_off; + tps6586x-power_off_hb.priority = POWER_OFF_PRIORITY_LOW; + ret = register_power_off_handler(tps6586x-power_off_hb); + if (ret) + dev_warn(client-dev, +failed to register power-off handler\n); } return 0; @@ -594,6 +603,8 @@ static int tps6586x_i2c_remove(struct i2c_client *client) { struct tps6586x *tps6586x = i2c_get_clientdata(client); + unregister_power_off_handler(tps6586x-power_off_hb); + tps6586x_remove_subdevs(tps6586x); mfd_remove_devices(tps6586x-dev); if (client-irq) -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] extcon: gpio: Convert the driver to use gpio desc API's
On Mon, Nov 03, 2014 at 10:32:27AM -0600, Felipe Balbi wrote: From: George Cherian george.cher...@ti.com Convert the driver to use gpiod_* API's. Reviewed-by: Roger Quadros rog...@ti.com Signed-off-by: George Cherian george.cher...@ti.com Signed-off-by: Sekhar Nori nsek...@ti.com Signed-off-by: Felipe Balbi ba...@ti.com --- I think it might be useful and appropriate to explain that and why you remove support for active-low pins, instead of hiding it in a seemingly unrelated patch. Also, since you don't use the platform data flag anymore, you might as well remove it to give out-of-tree users a fair warning. Overall, it might be easier to just revert the patch introducing it (5bfbdc9caa7) before converting extcon to gpiod. I am just glad that we stopped using extcon for other reasons ;-) Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/47] kernel: Add support for power-off handler call chain
On Mon, Nov 03, 2014 at 11:59:35AM -0600, Felipe Balbi wrote: Hi, On Mon, Oct 27, 2014 at 08:55:07AM -0700, Guenter Roeck wrote: Various drivers implement architecture and/or device specific means to remove power from the system. For the most part, those drivers set the global variable pm_power_off to point to a function within the driver. This mechanism has a number of drawbacks. Typically only one means to remove power is supported (at least if pm_power_off is used). At least in theory there can be multiple means to remove power, some of which may be less desirable. For example, one mechanism might power off the entire system through an I/O port or gpio pin, while another might power off a board by disabling its power controller. Other mechanisms may really just execute a restart sequence or drop into the ROM monitor, or put the CPU into sleep mode. Using pm_power_off can also be racy if the function pointer is set from a driver built as module, as the driver may be in the process of being unloaded when pm_power_off is called. If there are multiple power-off handlers in the system, removing a module with such a handler may inadvertently reset the pointer to pm_power_off to NULL, leaving the system with no means to remove power. Introduce a system power-off handler call chain to solve the described problems. This call chain is expected to be executed from the architecture specific machine_power_off() function. Drivers providing system power-off functionality are expected to register with this call chain. By using the priority field in the notifier block, callers can control power-off handler execution sequence and thus ensure that the power-off handler with the optimal capabilities to remove power for a given system is called first. A call chain instead of a single call to the highest priority handler is used to provide fallback: If multiple power-off handlers are installed, all handlers will be called until one actually succeeds to power off the system. Patch 01/47 implements the power-off handler API. Patches 02/47 to 04/47 are cleanup patches to prepare for the move of pm_power_off to a common location. Patches 05/47 to 07/47 remove references to pm_power_off from devicetree bindings descriptions. Patch 08/47 moves the pm_power_off variable from architecture code to kernel/reboot.c. Patches 09/47 to 34/47 convert various drivers to register with the kernel power-off handler instead of setting pm_power_off directly. Patches 35/47 to 46/47 do the same for architecture code. Patch 47/47 finally removes pm_power_off. For the most part, the individual patches include explanations why specific priorities were chosen, at least if the selected priority is not the default priority. Subsystem and architecture maintainers are encouraged to have a look at the selected priorities and suggest improvements. I ran the final code through my normal build and qemu tests. Results are available at http://server.roeck-us.net:8010/builders in the 'poweroff-handler' column. I also built all available configurations for arm, mips, powerpc, m68k, and sh architectures. The series is available in branch poweroff-handler of my repository at git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git. It is based on 3.18-rc2. A note on Cc: In the initial submission I had way too many Cc:, causing the patchset to be treated as spam by many mailers and mailing list handlers, which of course defeated the purpose. This time around I am cutting down the distribution list down significantly. My apologies to anyone I may have failed to copy this time around. you touch every single architecture with this patchset, but you didn't care about Ccing any of the arch-specific mailing lists, like lakml ? What is lakml ? linux-ker...@vger.kernel.org was copied, if that is what you refer to. I don't find a reference to lakml anywhere, sorry. I'll be happy to add it manually if you provide the address. Architecture specific mailing lists were copied on individual patches as long as those mailing lists are reported by the MAINTAINERS file. Please resend with proper people in Cc, IIRC RMK had a few very important comments about the idea behind this series. Felipe, That doesn't work. I tried with rev 1. The result was that the patch series was flagged as spam and got dropped by most mailing lists, as explained above, and I got tagged as spammer by Google and several other mail servers, preventing me from posting _anything_ for several days. Please point me to the comments you refer to above in case I missed them. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 09/47] mfd: palmas: Register with kernel power-off handler
On Mon, Nov 03, 2014 at 11:59:54AM -0600, Felipe Balbi wrote: On Mon, Nov 03, 2014 at 05:56:45PM +, Lee Jones wrote: On Mon, 27 Oct 2014, Guenter Roeck wrote: Register with kernel power-off handler instead of setting pm_power_off directly. Register with low priority to reflect that the original code only sets pm_power_off if it was not already set. Cc: Samuel Ortiz sa...@linux.intel.com Cc: Lee Jones lee.jo...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use define to specify poweroff handler priority - Use devm_register_power_off_handler - Use dev_warn instead of dev_err drivers/mfd/palmas.c | 31 +-- include/linux/mfd/palmas.h | 3 +++ 2 files changed, 20 insertions(+), 14 deletions(-) Acked-by: Lee Jones lee.jo...@linaro.org missed lakml and linux-omap. Felipe, unfortunately, get_maintainer.pl doesn't give a hint that this and the other affected patches should be sent to linux-omap. How am I supposed to know ? Note that linux-ker...@vger.kernel.org was copied on the entire series, if that is what you mean with lakml. linux...@vger.kernel.org was also copied on all patches. Additional mailing lists were only copied for affected architectures to avoid for the patches to be tagged as spam. If there is a list named lakml, I must have missed it, and I seem to be unable to find a reference to it. If that is the case, my apologies, and please provide a link to it. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] extcon: gpio: Convert the driver to use gpio desc API's
On Mon, Nov 03, 2014 at 12:06:02PM -0600, Felipe Balbi wrote: On Mon, Nov 03, 2014 at 10:01:50AM -0800, Guenter Roeck wrote: On Mon, Nov 03, 2014 at 10:32:27AM -0600, Felipe Balbi wrote: From: George Cherian george.cher...@ti.com Convert the driver to use gpiod_* API's. Reviewed-by: Roger Quadros rog...@ti.com Signed-off-by: George Cherian george.cher...@ti.com Signed-off-by: Sekhar Nori nsek...@ti.com Signed-off-by: Felipe Balbi ba...@ti.com --- I think it might be useful and appropriate to explain that and why you remove support for active-low pins, instead of hiding it in a seemingly unrelated patch. removed ? why removed ? gpio descs handle that for you, read the source code. Well, it for sure looks like it. Care to explain that in the patch, as well as how the platform data flag is used ? Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 09/47] mfd: palmas: Register with kernel power-off handler
On Mon, Nov 03, 2014 at 12:43:12PM -0600, Felipe Balbi wrote: On Mon, Nov 03, 2014 at 10:36:53AM -0800, Guenter Roeck wrote: On Mon, Nov 03, 2014 at 11:59:54AM -0600, Felipe Balbi wrote: On Mon, Nov 03, 2014 at 05:56:45PM +, Lee Jones wrote: On Mon, 27 Oct 2014, Guenter Roeck wrote: Register with kernel power-off handler instead of setting pm_power_off directly. Register with low priority to reflect that the original code only sets pm_power_off if it was not already set. Cc: Samuel Ortiz sa...@linux.intel.com Cc: Lee Jones lee.jo...@linaro.org Signed-off-by: Guenter Roeck li...@roeck-us.net --- v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use define to specify poweroff handler priority - Use devm_register_power_off_handler - Use dev_warn instead of dev_err drivers/mfd/palmas.c | 31 +-- include/linux/mfd/palmas.h | 3 +++ 2 files changed, 20 insertions(+), 14 deletions(-) Acked-by: Lee Jones lee.jo...@linaro.org missed lakml and linux-omap. Felipe, unfortunately, get_maintainer.pl doesn't give a hint that this and the other affected patches should be sent to linux-omap. How am I supposed to know ? yeah, just looked and nobody bothered to patch MAINTAINERS when those files were added. I just sent a patch adding them under OMAP SUPPORT. I'll add the omap list as Cc: to the affected patches directly for now. Note that linux-ker...@vger.kernel.org was copied on the entire series, if that is what you mean with lakml. linux...@vger.kernel.org was also copied on all patches. Additional mailing lists were only copied for affected architectures to avoid for the patches to be tagged as spam. If there is a list named lakml, I must have missed it, and I seem to be unable to find a reference to it. If that is the case, my apologies, and please provide a link to it. here it is: Linux ARM Kernel Mailing List linux-arm-ker...@lists.infradead.org Same problem here, though. If there are any ARM specific patches where the arm mailing list was not copied, that was because the dependency is not listed in the MAINTAINERS file. As mentioned before, copying the entire series to all lists touched by one of the patches in the series just doesn't work (and may be considered severe noise by some subscribers of those lists). Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/47] kernel: Add support for power-off handler call chain
On Mon, Nov 03, 2014 at 12:28:29PM -0600, Felipe Balbi wrote: Hi, On Mon, Nov 03, 2014 at 10:22:13AM -0800, Guenter Roeck wrote: On Mon, Nov 03, 2014 at 11:59:35AM -0600, Felipe Balbi wrote: Hi, On Mon, Oct 27, 2014 at 08:55:07AM -0700, Guenter Roeck wrote: Various drivers implement architecture and/or device specific means to remove power from the system. For the most part, those drivers set the global variable pm_power_off to point to a function within the driver. This mechanism has a number of drawbacks. Typically only one means to remove power is supported (at least if pm_power_off is used). At least in theory there can be multiple means to remove power, some of which may be less desirable. For example, one mechanism might power off the entire system through an I/O port or gpio pin, while another might power off a board by disabling its power controller. Other mechanisms may really just execute a restart sequence or drop into the ROM monitor, or put the CPU into sleep mode. Using pm_power_off can also be racy if the function pointer is set from a driver built as module, as the driver may be in the process of being unloaded when pm_power_off is called. If there are multiple power-off handlers in the system, removing a module with such a handler may inadvertently reset the pointer to pm_power_off to NULL, leaving the system with no means to remove power. Introduce a system power-off handler call chain to solve the described problems. This call chain is expected to be executed from the architecture specific machine_power_off() function. Drivers providing system power-off functionality are expected to register with this call chain. By using the priority field in the notifier block, callers can control power-off handler execution sequence and thus ensure that the power-off handler with the optimal capabilities to remove power for a given system is called first. A call chain instead of a single call to the highest priority handler is used to provide fallback: If multiple power-off handlers are installed, all handlers will be called until one actually succeeds to power off the system. Patch 01/47 implements the power-off handler API. Patches 02/47 to 04/47 are cleanup patches to prepare for the move of pm_power_off to a common location. Patches 05/47 to 07/47 remove references to pm_power_off from devicetree bindings descriptions. Patch 08/47 moves the pm_power_off variable from architecture code to kernel/reboot.c. Patches 09/47 to 34/47 convert various drivers to register with the kernel power-off handler instead of setting pm_power_off directly. Patches 35/47 to 46/47 do the same for architecture code. Patch 47/47 finally removes pm_power_off. For the most part, the individual patches include explanations why specific priorities were chosen, at least if the selected priority is not the default priority. Subsystem and architecture maintainers are encouraged to have a look at the selected priorities and suggest improvements. I ran the final code through my normal build and qemu tests. Results are available at http://server.roeck-us.net:8010/builders in the 'poweroff-handler' column. I also built all available configurations for arm, mips, powerpc, m68k, and sh architectures. The series is available in branch poweroff-handler of my repository at git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git. It is based on 3.18-rc2. A note on Cc: In the initial submission I had way too many Cc:, causing the patchset to be treated as spam by many mailers and mailing list handlers, which of course defeated the purpose. This time around I am cutting down the distribution list down significantly. My apologies to anyone I may have failed to copy this time around. you touch every single architecture with this patchset, but you didn't care about Ccing any of the arch-specific mailing lists, like lakml ? What is lakml ? linux-ker...@vger.kernel.org was copied, if that is what you only the greatest mailing list of all time. heh, Linux ARM Kernel Mailing List. Similar to linux-omap, linux-arm-kernel was copied on individual patches if get_maintainer.pl suggested it. I'll be happy to add both lists manually to the entire series for the next version of the patch set if the respective maintainers (Tony and Russell) ask me to do so. Note that this doesn't mean that the patches will actually be accepted by those lists, especially if the lists are moderated for non-subscribers. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message
Re: [PATCH v2 00/20] rtc: omap: fixes and power-off feature
On 10/29/2014 05:34 AM, Johan Hovold wrote: On Tue, Oct 28, 2014 at 03:16:10PM +, Russell King - ARM Linux wrote: On Tue, Oct 28, 2014 at 02:12:57PM +0100, Johan Hovold wrote: That's not what I was trying to refer to. But the patch set explicitly allows for multiple, prioritised power-off handlers, which can power off a board in different ways and with various degrees of success. Specifically, it allows for fallback handlers in case one or more power-off handlers fail. So if we allow for that, what is to prevent the final power-off handler from failing? And should this not be logged by arch code in the same way as failure to restart is? And how is that different from having a set of power-off handlers, and reporting when each individual one fails? Don't you want to know if your primary high priority reboot handler fails, just as much as you want to know if your final last-resort power-off handler fails? Good point. Failed power-off should probably be logged by the power-off call chain implementation (which seems to makes notifier chains a bad fit). Good that I just replaced notifier chain with an open coded implementation. Sure, that is possible, but I would prefer to do that as a follow-up commit, and it should be discussed in the context of the power-off handler patch set. And what about any power-off latencies? Should this always be dealt with in the power-off handler? Again, if it's predictable and high, as in the OMAP RTC case, it should go in the handler. But what if it's just normal bus latencies (peripheral busses, i2c, or whatever people may come up with)? Should there always be a short delay before calling the next handler? That delay would depend on the individual power-off handler, so I think the current implementation works just fine (where power-off handlers implement the delay). We could move the delay into the infrastructure, but it would have to be configurable. I would prefer to consider that as a follow-up patch to not overload the power-off handler patch set with too many changes at the same time. Or different from having no power-off handlers. That is actually quite different, as in that case we call machine_halt instead (via kernel_halt). Here's the x86 code: void machine_power_off(void) { machine_ops.power_off(); } struct machine_ops machine_ops = { .power_off = native_machine_power_off, ... static void native_machine_power_off(void) { if (pm_power_off) { if (!reboot_force) machine_shutdown(); pm_power_off(); } /* A fallback in case there is no PM info available */ tboot_shutdown(TB_SHUTDOWN_HALT); } void tboot_shutdown(u32 shutdown_type) { void (*shutdown)(void); if (!tboot_enabled()) return; See - x86 can very well just fall straight back out of machine_power_off() if there's no pm_power_off() hook and tboot is not enabled. I never doubted that, but is the right thing to do? Not all arches do it that way. And what about the killing of init? Shall we simply consider that a systemd bug? case LINUX_REBOOT_CMD_POWER_OFF: kernel_power_off(); do_exit(0); break; If power-off fails (for whatever reason), do_exit(0) will trigger a panic when called from PID 1. Common handling of that condition - eg to call machine_halt() - might be an option. Separate patch, though. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/20] rtc: omap: fixes and power-off feature
On Wed, Oct 29, 2014 at 02:22:44PM +0100, Johan Hovold wrote: On Wed, Oct 29, 2014 at 01:10:20PM +, Russell King - ARM Linux wrote: On Wed, Oct 29, 2014 at 01:34:18PM +0100, Johan Hovold wrote: On Tue, Oct 28, 2014 at 03:16:10PM +, Russell King - ARM Linux wrote: And how is that different from having a set of power-off handlers, and reporting when each individual one fails? Don't you want to know if your primary high priority reboot handler fails, just as much as you want to know if your final last-resort power-off handler fails? Good point. Failed power-off should probably be logged by the power-off call chain implementation (which seems to makes notifier chains a bad fit). And what about any power-off latencies? Should this always be dealt with in the power-off handler? Again, if it's predictable and high, as in the OMAP RTC case, it should go in the handler. But what if it's just normal bus latencies (peripheral busses, i2c, or whatever people may come up with)? Should there always be a short delay before calling the next handler? If the handler has determined that it has failed, then why delay before trying the next handler? At the point it has decided it has failed, surely that's after it has waited sufficient time to determine that failure? The current handlers we have are not expecting any other handler to be run after they return. My question was whether all these handlers should get a short mdelay added to them (e.g. to compensate for bus latencies) Some of them do add a delay. or if this could be done in the power-off handler (e.g. before printing the error message). That might make sense, but it would have to be configurable, since the delay is platform specific and power-off handler does not know how long to wait (the longest delay I have seen is 10 seconds). Or different from having no power-off handlers. That is actually quite different, as in that case we call machine_halt instead (via kernel_halt). Today, ARM does exactly what x86 does. If there's no power off handler registered, machine_power_off() shuts down other CPUs and returns. No, if there are no power-off handlers registered, kernel/reboot.c will never call machine_power_off: /* Instead of trying to make the power_off code look like * halt when pm_power_off is not set do it the easy way. */ if ((cmd == LINUX_REBOOT_CMD_POWER_OFF) !pm_power_off) cmd = LINUX_REBOOT_CMD_HALT; So in that case on arm, a system-halted message is printed, and we never return to user-space. Some architectures do that, or go into an endless loop. Others do return from machine_power_off. Having a well defined behavior would be nice (such as dumping an error mesasge and calling machine_halt if machine_power_off returns). Only question would be where to put it. kernel_power_off() might be a good place; only problem is that there are direct callers of machine_power_off(). Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/20] rtc: omap: fixes and power-off feature
On Wed, Oct 29, 2014 at 02:35:26PM +0100, Johan Hovold wrote: On Wed, Oct 29, 2014 at 06:20:40AM -0700, Guenter Roeck wrote: On 10/29/2014 05:34 AM, Johan Hovold wrote: On Tue, Oct 28, 2014 at 03:16:10PM +, Russell King - ARM Linux wrote: On Tue, Oct 28, 2014 at 02:12:57PM +0100, Johan Hovold wrote: That's not what I was trying to refer to. But the patch set explicitly allows for multiple, prioritised power-off handlers, which can power off a board in different ways and with various degrees of success. Specifically, it allows for fallback handlers in case one or more power-off handlers fail. So if we allow for that, what is to prevent the final power-off handler from failing? And should this not be logged by arch code in the same way as failure to restart is? And how is that different from having a set of power-off handlers, and reporting when each individual one fails? Don't you want to know if your primary high priority reboot handler fails, just as much as you want to know if your final last-resort power-off handler fails? Good point. Failed power-off should probably be logged by the power-off call chain implementation (which seems to makes notifier chains a bad fit). Good that I just replaced notifier chain with an open coded implementation. Good to hear. Sure, that is possible, but I would prefer to do that as a follow-up commit, and it should be discussed in the context of the power-off handler patch set. Fine with me. And what about any power-off latencies? Should this always be dealt with in the power-off handler? Again, if it's predictable and high, as in the OMAP RTC case, it should go in the handler. But what if it's just normal bus latencies (peripheral busses, i2c, or whatever people may come up with)? Should there always be a short delay before calling the next handler? That delay would depend on the individual power-off handler, so I think the current implementation works just fine (where power-off handlers implement the delay). Some don't, and could possibly unknowingly have been relying on the fact that they could return to user space and be powered off at some later time. With systemd that would have caused a panic. Agreed, but there are two cases to consider: What should be the delay before the next power-off handler is called, and what should the system do if all power-off handlers fail (or if there are none). The current behavior isn't exactly well defined. Ok, with systemd that results in a crash, but I am not really sure if one can or should blame systemd for that. The discussion about systemd and its philosophy should not cloud the fact that power-off behavior isn't exactly well defined. Also consider generic power-off handlers such as gpio-poweroff. It currently hard-codes a three-second delay but the actual delay would really be board specific. A configurable delay would address that. The actually required delay could be provided in platform data or as devicetree property. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 35/47] arm: Register with kernel power-off handler
Register with kernel power-off handler instead of setting pm_power_off directly. Always use register_power_off_handler_simple as there is no indication that more than one power-off handler is registered. If the power-off handler only resets the system or puts the CPU in sleep mode, select the fallback priority to indicate that the power-off handler is one of last resort. If the power-off handler powers off the system, select the default priority. Cc: Russell King li...@arm.linux.org.uk Signed-off-by: Guenter Roeck li...@roeck-us.net --- v3: - Replace poweroff in all newly introduced variables and in text with power_off or power-off as appropriate - Replace POWEROFF_PRIORITY_xxx with POWER_OFF_PRIORITY_xxx v2: - Use defines to specify poweroff handler priorities - Drop changes in arch/arm/mach-at91/setup.c (file removed upstream) arch/arm/kernel/psci.c | 3 ++- arch/arm/mach-at91/board-gsia18s.c | 3 ++- arch/arm/mach-bcm/board_bcm2835.c | 3 ++- arch/arm/mach-cns3xxx/cns3420vb.c | 3 ++- arch/arm/mach-cns3xxx/core.c | 3 ++- arch/arm/mach-highbank/highbank.c | 3 ++- arch/arm/mach-imx/mach-mx31moboard.c | 3 ++- arch/arm/mach-iop32x/em7210.c | 3 ++- arch/arm/mach-iop32x/glantank.c| 3 ++- arch/arm/mach-iop32x/iq31244.c | 3 ++- arch/arm/mach-iop32x/n2100.c | 3 ++- arch/arm/mach-ixp4xx/dsmg600-setup.c | 3 ++- arch/arm/mach-ixp4xx/nas100d-setup.c | 3 ++- arch/arm/mach-ixp4xx/nslu2-setup.c | 3 ++- arch/arm/mach-omap2/board-omap3touchbook.c | 3 ++- arch/arm/mach-orion5x/board-mss2.c | 3 ++- arch/arm/mach-orion5x/dns323-setup.c | 9 ++--- arch/arm/mach-orion5x/kurobox_pro-setup.c | 3 ++- arch/arm/mach-orion5x/ls-chl-setup.c | 3 ++- arch/arm/mach-orion5x/ls_hgl-setup.c | 3 ++- arch/arm/mach-orion5x/lsmini-setup.c | 3 ++- arch/arm/mach-orion5x/mv2120-setup.c | 3 ++- arch/arm/mach-orion5x/net2big-setup.c | 3 ++- arch/arm/mach-orion5x/terastation_pro2-setup.c | 3 ++- arch/arm/mach-orion5x/ts209-setup.c| 3 ++- arch/arm/mach-orion5x/ts409-setup.c| 3 ++- arch/arm/mach-pxa/corgi.c | 3 ++- arch/arm/mach-pxa/mioa701.c| 3 ++- arch/arm/mach-pxa/poodle.c | 3 ++- arch/arm/mach-pxa/spitz.c | 3 ++- arch/arm/mach-pxa/tosa.c | 3 ++- arch/arm/mach-pxa/viper.c | 3 ++- arch/arm/mach-pxa/z2.c | 7 --- arch/arm/mach-pxa/zeus.c | 7 --- arch/arm/mach-s3c24xx/mach-gta02.c | 3 ++- arch/arm/mach-s3c24xx/mach-jive.c | 3 ++- arch/arm/mach-s3c24xx/mach-vr1000.c| 3 ++- arch/arm/mach-s3c64xx/mach-smartq.c| 3 ++- arch/arm/mach-sa1100/generic.c | 3 ++- arch/arm/mach-sa1100/simpad.c | 3 ++- arch/arm/mach-u300/regulator.c | 3 ++- arch/arm/mach-vt8500/vt8500.c | 3 ++- arch/arm/xen/enlighten.c | 3 ++- 43 files changed, 94 insertions(+), 49 deletions(-) diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c index f73891b..a7a2b4a 100644 --- a/arch/arm/kernel/psci.c +++ b/arch/arm/kernel/psci.c @@ -264,7 +264,8 @@ static int psci_0_2_init(struct device_node *np) arm_pm_restart = psci_sys_reset; - pm_power_off = psci_sys_poweroff; + register_power_off_handler_simple(psci_sys_poweroff, + POWER_OFF_PRIORITY_DEFAULT); out_put_node: of_node_put(np); diff --git a/arch/arm/mach-at91/board-gsia18s.c b/arch/arm/mach-at91/board-gsia18s.c index bf5cc55..e628c4a 100644 --- a/arch/arm/mach-at91/board-gsia18s.c +++ b/arch/arm/mach-at91/board-gsia18s.c @@ -521,7 +521,8 @@ static void gsia18s_power_off(void) static int __init gsia18s_power_off_init(void) { - pm_power_off = gsia18s_power_off; + register_power_off_handler_simple(gsia18s_power_off, + POWER_OFF_PRIORITY_DEFAULT); return 0; } diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c index 70f2f39..1d75c76 100644 --- a/arch/arm/mach-bcm/board_bcm2835.c +++ b/arch/arm/mach-bcm/board_bcm2835.c @@ -111,7 +111,8 @@ static void __init bcm2835_init(void) bcm2835_setup_restart(); if (wdt_regs) - pm_power_off = bcm2835_power_off; + register_power_off_handler_simple(bcm2835_power_off, + POWER_OFF_PRIORITY_FALLBACK); bcm2835_init_clocks(); diff --git a/arch/arm/mach-cns3xxx/cns3420vb.c b/arch/arm/mach-cns3xxx/cns3420vb.c index 6428bcc7..5c50461 100644 --- a/arch/arm/mach-cns3xxx
[PATCH v2 35/47] arm: Register with kernel poweroff handler
Register with kernel poweroff handler instead of setting pm_power_off directly. Always use register_power_off_handler_simple as there is no indication that more than one poweroff handler is registered. If the poweroff handler only resets the system or puts the CPU in sleep mode, select the fallback priority to indicate that the poweroff handler is one of last resort. If the poweroff handler powers off the system, select the default priority. Cc: Russell King li...@arm.linux.org.uk Signed-off-by: Guenter Roeck li...@roeck-us.net --- - Use defines to specify poweroff handler priorities - Drop changes in arch/arm/mach-at91/setup.c (file removed upstream) arch/arm/kernel/psci.c | 3 ++- arch/arm/mach-at91/board-gsia18s.c | 3 ++- arch/arm/mach-bcm/board_bcm2835.c | 3 ++- arch/arm/mach-cns3xxx/cns3420vb.c | 3 ++- arch/arm/mach-cns3xxx/core.c | 3 ++- arch/arm/mach-highbank/highbank.c | 3 ++- arch/arm/mach-imx/mach-mx31moboard.c | 3 ++- arch/arm/mach-iop32x/em7210.c | 3 ++- arch/arm/mach-iop32x/glantank.c| 3 ++- arch/arm/mach-iop32x/iq31244.c | 3 ++- arch/arm/mach-iop32x/n2100.c | 3 ++- arch/arm/mach-ixp4xx/dsmg600-setup.c | 3 ++- arch/arm/mach-ixp4xx/nas100d-setup.c | 3 ++- arch/arm/mach-ixp4xx/nslu2-setup.c | 3 ++- arch/arm/mach-omap2/board-omap3touchbook.c | 3 ++- arch/arm/mach-orion5x/board-mss2.c | 3 ++- arch/arm/mach-orion5x/dns323-setup.c | 9 ++--- arch/arm/mach-orion5x/kurobox_pro-setup.c | 3 ++- arch/arm/mach-orion5x/ls-chl-setup.c | 3 ++- arch/arm/mach-orion5x/ls_hgl-setup.c | 3 ++- arch/arm/mach-orion5x/lsmini-setup.c | 3 ++- arch/arm/mach-orion5x/mv2120-setup.c | 3 ++- arch/arm/mach-orion5x/net2big-setup.c | 3 ++- arch/arm/mach-orion5x/terastation_pro2-setup.c | 3 ++- arch/arm/mach-orion5x/ts209-setup.c| 3 ++- arch/arm/mach-orion5x/ts409-setup.c| 3 ++- arch/arm/mach-pxa/corgi.c | 3 ++- arch/arm/mach-pxa/mioa701.c| 3 ++- arch/arm/mach-pxa/poodle.c | 3 ++- arch/arm/mach-pxa/spitz.c | 3 ++- arch/arm/mach-pxa/tosa.c | 3 ++- arch/arm/mach-pxa/viper.c | 3 ++- arch/arm/mach-pxa/z2.c | 7 --- arch/arm/mach-pxa/zeus.c | 7 --- arch/arm/mach-s3c24xx/mach-gta02.c | 3 ++- arch/arm/mach-s3c24xx/mach-jive.c | 3 ++- arch/arm/mach-s3c24xx/mach-vr1000.c| 3 ++- arch/arm/mach-s3c64xx/mach-smartq.c| 3 ++- arch/arm/mach-sa1100/generic.c | 3 ++- arch/arm/mach-sa1100/simpad.c | 3 ++- arch/arm/mach-u300/regulator.c | 3 ++- arch/arm/mach-vt8500/vt8500.c | 3 ++- arch/arm/xen/enlighten.c | 3 ++- 43 files changed, 94 insertions(+), 49 deletions(-) diff --git a/arch/arm/kernel/psci.c b/arch/arm/kernel/psci.c index f73891b..4917c99 100644 --- a/arch/arm/kernel/psci.c +++ b/arch/arm/kernel/psci.c @@ -264,7 +264,8 @@ static int psci_0_2_init(struct device_node *np) arm_pm_restart = psci_sys_reset; - pm_power_off = psci_sys_poweroff; + register_power_off_handler_simple(psci_sys_poweroff, + POWEROFF_PRIORITY_DEFAULT); out_put_node: of_node_put(np); diff --git a/arch/arm/mach-at91/board-gsia18s.c b/arch/arm/mach-at91/board-gsia18s.c index bf5cc55..cb5d1c3 100644 --- a/arch/arm/mach-at91/board-gsia18s.c +++ b/arch/arm/mach-at91/board-gsia18s.c @@ -521,7 +521,8 @@ static void gsia18s_power_off(void) static int __init gsia18s_power_off_init(void) { - pm_power_off = gsia18s_power_off; + register_power_off_handler_simple(gsia18s_power_off, + POWEROFF_PRIORITY_DEFAULT); return 0; } diff --git a/arch/arm/mach-bcm/board_bcm2835.c b/arch/arm/mach-bcm/board_bcm2835.c index 70f2f39..307ebc1 100644 --- a/arch/arm/mach-bcm/board_bcm2835.c +++ b/arch/arm/mach-bcm/board_bcm2835.c @@ -111,7 +111,8 @@ static void __init bcm2835_init(void) bcm2835_setup_restart(); if (wdt_regs) - pm_power_off = bcm2835_power_off; + register_power_off_handler_simple(bcm2835_power_off, + POWEROFF_PRIORITY_FALLBACK); bcm2835_init_clocks(); diff --git a/arch/arm/mach-cns3xxx/cns3420vb.c b/arch/arm/mach-cns3xxx/cns3420vb.c index 6428bcc7..3f48979 100644 --- a/arch/arm/mach-cns3xxx/cns3420vb.c +++ b/arch/arm/mach-cns3xxx/cns3420vb.c @@ -224,7 +224,8 @@ static void __init cns3420_init(void) cns3xxx_ahci_init(); cns3xxx_sdhci_init(); - pm_power_off
Re: [PATCH] watchdog: Fix omap watchdogs to enable the magic close bit
On Tue, Oct 14, 2014 at 12:25:19PM -0700, Tony Lindgren wrote: This allows testing the watchdog easily with distros just by doing pkill -9 watchdog. Reported-by: Thomas Dziedzic gos...@gmail.com Signed-off-by: Tony Lindgren t...@atomide.com Good catch. Reviewed-by: Guenter Roeck li...@roeck-us.net --- a/drivers/watchdog/omap_wdt.c +++ b/drivers/watchdog/omap_wdt.c @@ -189,7 +189,7 @@ static int omap_wdt_set_timeout(struct watchdog_device *wdog, } static const struct watchdog_info omap_wdt_info = { - .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING, + .options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING, .identity = OMAP Watchdog, }; --- a/drivers/watchdog/retu_wdt.c +++ b/drivers/watchdog/retu_wdt.c @@ -94,7 +94,7 @@ static int retu_wdt_set_timeout(struct watchdog_device *wdog, } static const struct watchdog_info retu_wdt_info = { - .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING, + .options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING, .identity = Retu watchdog, }; --- a/drivers/watchdog/twl4030_wdt.c +++ b/drivers/watchdog/twl4030_wdt.c @@ -57,7 +57,7 @@ static int twl4030_wdt_set_timeout(struct watchdog_device *wdt, } static const struct watchdog_info twl4030_wdt_info = { - .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING, + .options = WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE | WDIOF_KEEPALIVEPING, .identity = TWL4030 Watchdog, }; -- To unsubscribe from this list: send the line unsubscribe linux-watchdog in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 02/23] watchdog: omap_wdt: raw read and write endian fix
On 11/15/2013 04:01 PM, Taras Kondratiuk wrote: From: Victor Kamensky victor.kamen...@linaro.org All OMAP IP blocks expect LE data, but CPU may operate in BE mode. Need to use endian neutral functions to read/write h/w registers. I.e instead of __raw_read[lw] and __raw_write[lw] functions code need to use read[lw]_relaxed and write[lw]_relaxed functions. If the first simply reads/writes register, the second will byteswap it if host operates in BE mode. Changes are trivial sed like replacement of __raw_xxx functions with xxx_relaxed variant. Signed-off-by: Victor Kamensky victor.kamen...@linaro.org Signed-off-by: Taras Kondratiuk taras.kondrat...@linaro.org Acked-by: Guenter Roeck li...@roeck-us.net -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] arm: Select NOP_USB_XCEIV if MACH_OMAP3EVM is enabled
arm builds have been failing on and off with arch/arm/mach-omap2/board-omap3evm.c:703: undefined reference to `usb_nop_xceiv_register' for several years. Current arm:allmodconfig build fails for this reason. Problem is that board-omap3evm.c is always build into the kernel. It calls usb_nop_xceiv_register(), which is built with NOP_USB_XCEIV, which in turn can be configured as module. This results in the observed build failure. Fix the problem once and for all by selecting NOP_USB_XCEIV if MACH_OMAP3EVM is enabled. Signed-off-by: Guenter Roeck li...@roeck-us.net --- arch/arm/mach-omap2/Kconfig |1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index b5fb5f7..0bc2cf4 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -265,6 +265,7 @@ config MACH_OMAP3EVM depends on ARCH_OMAP3 default y select OMAP_PACKAGE_CBB + select NOP_USB_XCEIV config MACH_OMAP3517EVM bool OMAP3517/ AM3517 EVM board -- 1.7.9.7 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Build error in torvalds kernel 3.11 for omap2plus
On 09/09/2013 03:55 AM, Anil Kumar wrote: Hi Guenter, On Sun, Sep 8, 2013 at 10:46 PM, Guenter Roeck li...@roeck-us.net wrote: On 09/08/2013 02:02 AM, Russell King - ARM Linux wrote: On Sun, Sep 08, 2013 at 11:34:10AM +0530, Anil Kumar wrote: scripts/kconfig/conf --silentoldconfig Kconfig CHK include/generated/uapi/linux/version.h CHK include/generated/utsrelease.h make[1]: `include/generated/mach-types.h' is up to date. CALLscripts/checksyscalls.sh CHK include/generated/compile.h AS arch/arm/mm/cache-v7.o arch/arm/mm/cache-v7.S: Assembler messages: arch/arm/mm/cache-v7.S:285: Error: garbage following instruction -- `dsb ishst' arch/arm/mm/cache-v7.S:297: Error: garbage following instruction -- `dsb ishst' make[1]: *** [arch/arm/mm/cache-v7.o] Error 1 make: *** [arch/arm/mm] Error 2 You need a later binutils for these instructions - 2.22 works just fine, even with gcc 4.5.x. Since we are at it: Build reference: v3.11-7887-gb409624 Building arm:defconfig ... passed Building arm:allmodconfig ... failed -- Error log: arch/arm/mach-cns3xxx/pcie.c: In function 'cns3xxx_pcie_hw_init': arch/arm/mach-cns3xxx/pcie.c:350:1: warning: the frame size of 1064 bytes is larger than 1024 bytes [-Wframe-larger-than=] arch/arm/kernel/return_address.c:63:2: warning: #warning TODO: return_address should use unwind tables [-Wcpp] arch/arm/kernel/return_address.c:63:2: warning: #warning TODO: return_address should use unwind tables [-Wcpp] /tmp/cce439dZ.s: Assembler messages: /tmp/cce439dZ.s:506: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:512: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:513: Error: selected processor does not support ARM mode `dsb ' /tmp/cce439dZ.s:583: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:589: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:590: Error: selected processor does not support ARM mode `dsb ' make[1]: *** [arch/arm/mach-vexpress/dcscb.o] Error 1 make: *** [arch/arm/mach-vexpress] Error 2 make: *** Waiting for unfinished jobs -- Any solution for this one ? omap2plus passes for me. I have updated my toolchain Assembler (used binutils 2.22 ) with latest one and issue has fixed. Not for this build. As I said, the omap2plus build passes for me. My binutils version is 2.23.1.20121113. Did you try to build the file after make allmodconfig ? Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Build error in torvalds kernel 3.11 for omap2plus
On 09/09/2013 03:51 AM, Russell King - ARM Linux wrote: On Sun, Sep 08, 2013 at 10:16:14AM -0700, Guenter Roeck wrote: Since we are at it: Build reference: v3.11-7887-gb409624 Building arm:defconfig ... passed Building arm:allmodconfig ... failed -- Error log: arch/arm/mach-cns3xxx/pcie.c: In function 'cns3xxx_pcie_hw_init': arch/arm/mach-cns3xxx/pcie.c:350:1: warning: the frame size of 1064 bytes is larger than 1024 bytes [-Wframe-larger-than=] arch/arm/kernel/return_address.c:63:2: warning: #warning TODO: return_address should use unwind tables [-Wcpp] arch/arm/kernel/return_address.c:63:2: warning: #warning TODO: return_address should use unwind tables [-Wcpp] /tmp/cce439dZ.s: Assembler messages: /tmp/cce439dZ.s:506: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:512: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:513: Error: selected processor does not support ARM mode `dsb ' /tmp/cce439dZ.s:583: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:589: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:590: Error: selected processor does not support ARM mode `dsb ' make[1]: *** [arch/arm/mach-vexpress/dcscb.o] Error 1 make: *** [arch/arm/mach-vexpress] Error 2 make: *** Waiting for unfinished jobs -- Any solution for this one ? omap2plus passes for me. gcc version used is arm-poky-linux-gnueabi-gcc (GCC) 4.7.2 from poky 1.3. That's due to: commit e8f9bb1bd6bb93fff773345cc54c42585e0e3ece Author: Nicolas Pitre nicolas.pi...@linaro.org Date: Tue Jul 16 20:59:53 2013 -0400 ARM: vexpress/dcscb: fix cache disabling sequences Unlike real A15/A7's, the RTSM simulation doesn't appear to hit the cache when the CTRL.C bit is cleared. Let's ensure there is no memory access within the disable and flush cache sequence, including to the stack. Signed-off-by: Nicolas Pitre n...@linaro.org which introduces some 'isb' and 'dsb' instructions which are not available on ARMv6 CPUs - however, their 'mcr' equivalents are. Either dcscb needs to be built with an -march=armv7 override, or they need to use the mcr equivalent instructions. Well, I hope it will get fixed one way or another. I don't know enough about arm to fix it myself. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Build error in torvalds kernel 3.11 for omap2plus
On Mon, Sep 09, 2013 at 10:24:42AM -0400, Nicolas Pitre wrote: On Mon, 9 Sep 2013, Guenter Roeck wrote: On 09/09/2013 03:51 AM, Russell King - ARM Linux wrote: On Sun, Sep 08, 2013 at 10:16:14AM -0700, Guenter Roeck wrote: Since we are at it: Build reference: v3.11-7887-gb409624 Building arm:defconfig ... passed Building arm:allmodconfig ... failed -- Error log: arch/arm/mach-cns3xxx/pcie.c: In function 'cns3xxx_pcie_hw_init': arch/arm/mach-cns3xxx/pcie.c:350:1: warning: the frame size of 1064 bytes is larger than 1024 bytes [-Wframe-larger-than=] arch/arm/kernel/return_address.c:63:2: warning: #warning TODO: return_address should use unwind tables [-Wcpp] arch/arm/kernel/return_address.c:63:2: warning: #warning TODO: return_address should use unwind tables [-Wcpp] /tmp/cce439dZ.s: Assembler messages: /tmp/cce439dZ.s:506: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:512: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:513: Error: selected processor does not support ARM mode `dsb ' /tmp/cce439dZ.s:583: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:589: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:590: Error: selected processor does not support ARM mode `dsb ' make[1]: *** [arch/arm/mach-vexpress/dcscb.o] Error 1 make: *** [arch/arm/mach-vexpress] Error 2 make: *** Waiting for unfinished jobs -- Any solution for this one ? omap2plus passes for me. gcc version used is arm-poky-linux-gnueabi-gcc (GCC) 4.7.2 from poky 1.3. That's due to: commit e8f9bb1bd6bb93fff773345cc54c42585e0e3ece Author: Nicolas Pitre nicolas.pi...@linaro.org Date: Tue Jul 16 20:59:53 2013 -0400 ARM: vexpress/dcscb: fix cache disabling sequences Unlike real A15/A7's, the RTSM simulation doesn't appear to hit the cache when the CTRL.C bit is cleared. Let's ensure there is no memory access within the disable and flush cache sequence, including to the stack. Signed-off-by: Nicolas Pitre n...@linaro.org which introduces some 'isb' and 'dsb' instructions which are not available on ARMv6 CPUs - however, their 'mcr' equivalents are. Either dcscb needs to be built with an -march=armv7 override, or they need to use the mcr equivalent instructions. Well, I hope it will get fixed one way or another. I don't know enough about arm to fix it myself. Would you try this patch please: diff --git a/arch/arm/mach-vexpress/Makefile b/arch/arm/mach-vexpress/Makefile index 36ea824712..505e64ab3e 100644 --- a/arch/arm/mach-vexpress/Makefile +++ b/arch/arm/mach-vexpress/Makefile @@ -7,6 +7,8 @@ ccflags-$(CONFIG_ARCH_MULTIPLATFORM) := -I$(srctree)/$(src)/include \ obj-y:= v2m.o obj-$(CONFIG_ARCH_VEXPRESS_CA9X4)+= ct-ca9x4.o obj-$(CONFIG_ARCH_VEXPRESS_DCSCB)+= dcscb.o dcscb_setup.o obj-$(CONFIG_ARCH_VEXPRESS_DCSCB)+= dcscb.o dcscb_setup.o +CFLAGS_dcscb.o += -march=armv7-a obj-$(CONFIG_ARCH_VEXPRESS_TC2_PM) += tc2_pm.o spc.o +CFLAGS_tc2_pm.o += -march=armv7-a obj-$(CONFIG_SMP)+= platsmp.o obj-$(CONFIG_HOTPLUG_CPU)+= hotplug.o That fixes this problem. I'll leave it up to you and Russell to decide if it is the proper fix. Unfortunately, there are more problems. arch/arm/kernel/built-in.o: In function `ret_fast_syscall': /home/groeck/src/linux-stable/arch/arm/kernel/entry-common.S:42: undefined reference to `user_enter' arch/arm/kernel/built-in.o: In function `no_work_pending': /home/groeck/src/linux-stable/arch/arm/kernel/entry-common.S:77: undefined reference to `user_enter' arch/arm/kernel/built-in.o: In function `vector_swi': /home/groeck/src/linux-stable/arch/arm/kernel/entry-common.S:376: undefined reference to `user_exit' arch/arm/kernel/built-in.o: In function `__dabt_usr': /home/groeck/src/linux-stable/arch/arm/kernel/entry-armv.S:365: undefined reference to `user_exit' arch/arm/kernel/built-in.o: In function `__irq_usr': /home/groeck/src/linux-stable/arch/arm/kernel/entry-armv.S:375: undefined reference to `user_exit' arch/arm/kernel/built-in.o: In function `__und_usr': /home/groeck/src/linux-stable/arch/arm/kernel/entry-armv.S:388: undefined reference to `user_exit' arch/arm/kernel/built-in.o: In function `__pabt_usr': /home/groeck/src/linux-stable/arch/arm/kernel/entry-armv.S:662: undefined reference to `user_exit' arch/arm/mach-omap2/built-in.o: In function `omap3_evm_init': /home/groeck/src/linux-stable/arch/arm/mach-omap2/board-omap3evm.c:703: undefined reference to `usb_nop_xceiv_register' The undefined reference to usb_nop_xceiv_register
Re: Build error in torvalds kernel 3.11 for omap2plus
On Mon, Sep 09, 2013 at 09:31:41PM +0100, Russell King - ARM Linux wrote: On Mon, Sep 09, 2013 at 09:03:48AM -0700, Guenter Roeck wrote: Unfortunately, there are more problems. arch/arm/kernel/built-in.o: In function `ret_fast_syscall': /home/groeck/src/linux-stable/arch/arm/kernel/entry-common.S:42: undefined reference to `user_enter' arch/arm/kernel/built-in.o: In function `no_work_pending': /home/groeck/src/linux-stable/arch/arm/kernel/entry-common.S:77: undefined reference to `user_enter' arch/arm/kernel/built-in.o: In function `vector_swi': /home/groeck/src/linux-stable/arch/arm/kernel/entry-common.S:376: undefined reference to `user_exit' arch/arm/kernel/built-in.o: In function `__dabt_usr': /home/groeck/src/linux-stable/arch/arm/kernel/entry-armv.S:365: undefined reference to `user_exit' arch/arm/kernel/built-in.o: In function `__irq_usr': /home/groeck/src/linux-stable/arch/arm/kernel/entry-armv.S:375: undefined reference to `user_exit' arch/arm/kernel/built-in.o: In function `__und_usr': /home/groeck/src/linux-stable/arch/arm/kernel/entry-armv.S:388: undefined reference to `user_exit' arch/arm/kernel/built-in.o: In function `__pabt_usr': /home/groeck/src/linux-stable/arch/arm/kernel/entry-armv.S:662: undefined reference to `user_exit' These are due to ad65782fba50 (context_tracking: Optimize main APIs off case with static key) converting these functions to be inline. No idea what the fix for this is other than reverting the change. Any ideas Frederic? I am all for reverting unless this can be fixed quickly. AFAICS this breaks all arm builds if CONFIG_CONTEXT_TRACKING is enabled. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Build error in torvalds kernel 3.11 for omap2plus
On 09/08/2013 02:02 AM, Russell King - ARM Linux wrote: On Sun, Sep 08, 2013 at 11:34:10AM +0530, Anil Kumar wrote: scripts/kconfig/conf --silentoldconfig Kconfig CHK include/generated/uapi/linux/version.h CHK include/generated/utsrelease.h make[1]: `include/generated/mach-types.h' is up to date. CALLscripts/checksyscalls.sh CHK include/generated/compile.h AS arch/arm/mm/cache-v7.o arch/arm/mm/cache-v7.S: Assembler messages: arch/arm/mm/cache-v7.S:285: Error: garbage following instruction -- `dsb ishst' arch/arm/mm/cache-v7.S:297: Error: garbage following instruction -- `dsb ishst' make[1]: *** [arch/arm/mm/cache-v7.o] Error 1 make: *** [arch/arm/mm] Error 2 You need a later binutils for these instructions - 2.22 works just fine, even with gcc 4.5.x. Since we are at it: Build reference: v3.11-7887-gb409624 Building arm:defconfig ... passed Building arm:allmodconfig ... failed -- Error log: arch/arm/mach-cns3xxx/pcie.c: In function 'cns3xxx_pcie_hw_init': arch/arm/mach-cns3xxx/pcie.c:350:1: warning: the frame size of 1064 bytes is larger than 1024 bytes [-Wframe-larger-than=] arch/arm/kernel/return_address.c:63:2: warning: #warning TODO: return_address should use unwind tables [-Wcpp] arch/arm/kernel/return_address.c:63:2: warning: #warning TODO: return_address should use unwind tables [-Wcpp] /tmp/cce439dZ.s: Assembler messages: /tmp/cce439dZ.s:506: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:512: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:513: Error: selected processor does not support ARM mode `dsb ' /tmp/cce439dZ.s:583: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:589: Error: selected processor does not support ARM mode `isb ' /tmp/cce439dZ.s:590: Error: selected processor does not support ARM mode `dsb ' make[1]: *** [arch/arm/mach-vexpress/dcscb.o] Error 1 make: *** [arch/arm/mach-vexpress] Error 2 make: *** Waiting for unfinished jobs -- Any solution for this one ? omap2plus passes for me. gcc version used is arm-poky-linux-gnueabi-gcc (GCC) 4.7.2 from poky 1.3. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] arm/omap2: Fix build failures
Commit c1d1cd59 (omap_device: remove obsolete pm_lats and early_device code) changed the number of parameters to omap_device_build(), but did not fix all calling code correctly. This causes build failures with some configurations. arch/arm/mach-omap2/devices.c: In function 'omap_init_hdmi_audio': arch/arm/mach-omap2/devices.c:429:2: error: too many arguments to function 'omap_device_build' arch/arm/mach-omap2/omap_device.h:74:25: note: declared here make[2]: [arch/arm/mach-omap2/devices.o] Error 1 (ignored) arch/arm/mach-omap2/sr_device.c: In function 'sr_dev_init': arch/arm/mach-omap2/sr_device.c:155:2: error: too many arguments to function 'omap_device_build' arch/arm/mach-omap2/omap_device.h:74:25: note: declared here make[2]: [arch/arm/mach-omap2/sr_device.o] Error 1 (ignored) arch/arm/mach-omap2/am35xx-emac.c: In function 'omap_davinci_emac_dev_init': arch/arm/mach-omap2/am35xx-emac.c:66:6: error: too many arguments to function 'omap_device_build' arch/arm/mach-omap2/omap_device.h:74:25: note: declared here Signed-off-by: Guenter Roeck li...@roeck-us.net --- arch/arm/mach-omap2/am35xx-emac.c |3 +-- arch/arm/mach-omap2/devices.c |2 +- arch/arm/mach-omap2/sr_device.c |2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/arch/arm/mach-omap2/am35xx-emac.c b/arch/arm/mach-omap2/am35xx-emac.c index a00d391..25b79a2 100644 --- a/arch/arm/mach-omap2/am35xx-emac.c +++ b/arch/arm/mach-omap2/am35xx-emac.c @@ -62,8 +62,7 @@ static int __init omap_davinci_emac_dev_init(struct omap_hwmod *oh, { struct platform_device *pdev; - pdev = omap_device_build(oh-class-name, 0, oh, pdata, pdata_len, -false); + pdev = omap_device_build(oh-class-name, 0, oh, pdata, pdata_len); if (IS_ERR(pdev)) { WARN(1, Can't build omap_device for %s:%s.\n, oh-class-name, oh-name); diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c index 142d9c6..1ec7f05 100644 --- a/arch/arm/mach-omap2/devices.c +++ b/arch/arm/mach-omap2/devices.c @@ -426,7 +426,7 @@ static void __init omap_init_hdmi_audio(void) return; } - pdev = omap_device_build(omap-hdmi-audio-dai, -1, oh, NULL, 0, 0); + pdev = omap_device_build(omap-hdmi-audio-dai, -1, oh, NULL, 0); WARN(IS_ERR(pdev), Can't build omap_device for omap-hdmi-audio-dai.\n); diff --git a/arch/arm/mach-omap2/sr_device.c b/arch/arm/mach-omap2/sr_device.c index bb829e0..d7bc33f 100644 --- a/arch/arm/mach-omap2/sr_device.c +++ b/arch/arm/mach-omap2/sr_device.c @@ -152,7 +152,7 @@ static int __init sr_dev_init(struct omap_hwmod *oh, void *user) sr_data-enable_on_init = sr_enable_on_init; - pdev = omap_device_build(name, i, oh, sr_data, sizeof(*sr_data), 0); + pdev = omap_device_build(name, i, oh, sr_data, sizeof(*sr_data)); if (IS_ERR(pdev)) pr_warning(%s: Could not build omap_device for %s: %s.\n\n, __func__, name, oh-name); -- 1.7.9.7 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation: dt: hwmon: Update for tmp102
On Tue, Aug 14, 2012 at 09:53:32PM +0530, Sourav Poddar wrote: Add Documentation for tmp102 temperature sensor. Cc: Benoit Cousson b-cous...@ti.com Cc: Felipe Balbi ba...@ti.com Cc: Santosh Shilimkar santosh.shilim...@ti.com Signed-off-by: Sourav Poddar sourav.pod...@ti.com --- Documentation/devicetree/bindings/hwmon/tmp102.txt | 11 +++ 1 files changed, 11 insertions(+), 0 deletions(-) create mode 100644 Documentation/devicetree/bindings/hwmon/tmp102.txt diff --git a/Documentation/devicetree/bindings/hwmon/tmp102.txt b/Documentation/devicetree/bindings/hwmon/tmp102.txt new file mode 100644 index 000..df1342b --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/tmp102.txt @@ -0,0 +1,11 @@ +TMP102 Temperature sensor + +Required properties: +- compatible: ti,tmp102 + +Example: + +tmp102@48 { +compatible = ti,tmp102; +reg = 0x48; +}; -- It would be simpler to just add a single line to Documentation/devicetree/bindings/i2c/trivial-devices.txt Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] hwmon: OMAP4460+: On die temperature sensor driver
On Thu, 2011-09-22 at 13:36 -0400, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of on die subsystems. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. The patch depends on the the following series: http://comments.gmane.org/gmane.linux.ports.arm.omap/64436 and http://permalink.gmane.org/gmane.linux.ports.arm.omap/64446 Signed-off-by: Keerthy j-keer...@ti.com Cc: Jean Delvare kh...@linux-fr.org Cc: Guenter Roeck guenter.ro...@ericsson.com Cc: lm-sens...@lm-sensors.org --- Documentation/hwmon/omap_temp_sensor | 25 +++ drivers/hwmon/Kconfig| 11 ++ drivers/hwmon/Makefile |1 + drivers/hwmon/omap4460plus_temp_sensor.c | 244 ++ 4 files changed, 281 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwmon/omap_temp_sensor create mode 100644 drivers/hwmon/omap4460plus_temp_sensor.c Index: linux-omap-2.6/Documentation/hwmon/omap_temp_sensor === --- /dev/null 1970-01-01 00:00:00.0 + +++ linux-omap-2.6/Documentation/hwmon/omap_temp_sensor 2011-09-22 18:57:23.180918857 +0530 @@ -0,0 +1,25 @@ +Kernel driver omap_temp_sensor +== + +Supported chips: + * Texas Instruments OMAP4460 +Prefix: 'omap_temp_sensor' + +Author: +J Keerthy j-keer...@ti.com + +Description +--- + +The Texas Instruments OMAP4 family of chips have a bandgap temperature sensor. +The temperature sensor feature is used to convert the temperature of the device +into a decimal value coded on 10 bits. An internal ADC is used for conversion. +The recommended operating temperatures must be in the range -40 degree Celsius +to 123 degree celsius for standard conversion. +The thresholds are programmable and upon crossing the thresholds an interrupt +is generated. The OMAP temperature sensor has a programmable update rate in +milli seconds. +(Currently the driver programs a default of 2000 milliseconds). + +The driver provides the common sysfs-interface for temperatures (see +Documentation/hwmon/sysfs-interface under Temperatures). Index: linux-omap-2.6/drivers/hwmon/Kconfig === --- linux-omap-2.6.orig/drivers/hwmon/Kconfig 2011-09-22 17:48:38.032575702 +0530 +++ linux-omap-2.6/drivers/hwmon/Kconfig 2011-09-22 19:02:28.744575604 +0530 @@ -323,6 +323,17 @@ This driver can also be built as a module. If so, the module will be called f71805f. +config SENSORS_OMAP_BANDGAP_TEMP_SENSOR + bool OMAP on-die temperature sensor hwmon driver + depends on HWMON ARCH_OMAP OMAP4460PLUS_SCM + help + If you say yes here you get support for hardware + monitoring features of the OMAP on die temperature + sensor. + + Continuous conversion programmable delay + mode is used for temperature conversion. + config SENSORS_F71882FG tristate Fintek F71882FG and compatibles help Index: linux-omap-2.6/drivers/hwmon/Makefile === --- linux-omap-2.6.orig/drivers/hwmon/Makefile2011-09-22 17:48:38.020575728 +0530 +++ linux-omap-2.6/drivers/hwmon/Makefile 2011-09-22 18:57:23.192574712 +0530 @@ -93,6 +93,7 @@ obj-$(CONFIG_SENSORS_MAX6642)+= max6642.o obj-$(CONFIG_SENSORS_MAX6650)+= max6650.o obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o +obj-$(CONFIG_SENSORS_OMAP_BANDGAP_TEMP_SENSOR) += omap4460plus_temp_sensor.o obj-$(CONFIG_SENSORS_NTC_THERMISTOR) += ntc_thermistor.o obj-$(CONFIG_SENSORS_PC87360)+= pc87360.o obj-$(CONFIG_SENSORS_PC87427)+= pc87427.o Index: linux-omap-2.6/drivers/hwmon/omap4460plus_temp_sensor.c === --- /dev/null 1970-01-01 00:00:00.0 + +++ linux-omap-2.6/drivers/hwmon/omap4460plus_temp_sensor.c 2011-09-22 19:02:11.096575567 +0530 @@ -0,0 +1,242 @@ +/* + * + * OMAP4460 Plus bandgap on die sensor hwmon driver. + * + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ + * J Keerthy j-keer...@ti.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Tue, 2011-09-06 at 14:02 -0400, J, KEERTHY wrote: On Thu, Sep 1, 2011 at 10:10 AM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Thu, Sep 01, 2011 at 12:09:14AM -0400, Paul Walmsley wrote: On Wed, 31 Aug 2011, Guenter Roeck wrote: On Wed, Aug 31, 2011 at 08:36:43PM -0400, Paul Walmsley wrote: Hi Some comments. On Wed, 31 Aug 2011, Keerthy wrote: [ ... ] +} + +/* Sysfs hook functions */ These should be conditionally compiled out if sysfs isn't compiled in. The whole point of the hwmon subsystem is to expose hardware monitoring information to userland using sysfs. hwmon without sysfs doesn't make sense. So, if anything, it might make sense to disable the entire hwmon tree if sysfs is disabled. But please no conditionals in the code. Hmm. This IP block is more than just a sensor. It also can interrupt the CPU and/or trigger a GPIO line (to shut down the chip) if the chip temperature crosses some thresholds. On some OMAPs, the thresholds are fixed; on others, they are software-programmable. That functionality shouldn't require sysfs; it's almost closer to an x86 MCE. So based on your comments, it sounds like we should move that part of the code to a different driver, and just leave the basic software thermal monitoring here? Good point. This definitely requires some thought. hwmon is meant to be hw monitoring, as the name says, not thermal management. Maybe this entire driver should be a thermal driver instead ? This driver is not taking any action on THSUT. This is not doing the thermal management. It is a driver exposing configurable temperature thresholds. What sense would it make, then, to keep the driver around even if SYSFS is not defined ? Note that I am not looking at the code right now, but at the suggestion that the driver would do something useful if SYSFS is not defined. Question is what that is, and if that part of it should reside in a hwmon driver. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Wed, Aug 31, 2011 at 08:36:43PM -0400, Paul Walmsley wrote: Hi Some comments. On Wed, 31 Aug 2011, Keerthy wrote: [ ... ] +} + +/* Sysfs hook functions */ These should be conditionally compiled out if sysfs isn't compiled in. The whole point of the hwmon subsystem is to expose hardware monitoring information to userland using sysfs. hwmon without sysfs doesn't make sense. So, if anything, it might make sense to disable the entire hwmon tree if sysfs is disabled. But please no conditionals in the code. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Wed, Aug 31, 2011 at 01:25:10PM -0400, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of the MPU subsystem of the OMAP4. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. Signed-off-by: Keerthy j-keer...@ti.com Cc: Jean Delvare kh...@linux-fr.org Cc: Guenter Roeck guenter.ro...@ericsson.com Cc: lm-sens...@lm-sensors.org --- Documentation/hwmon/omap_temp_sensor | 26 + drivers/hwmon/Kconfig| 11 + drivers/hwmon/Makefile |1 + drivers/hwmon/omap_temp_sensor.c | 881 ++ 4 files changed, 919 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwmon/omap_temp_sensor create mode 100644 drivers/hwmon/omap_temp_sensor.c diff --git a/Documentation/hwmon/omap_temp_sensor b/Documentation/hwmon/omap_temp_sensor new file mode 100644 index 000..357f09a --- /dev/null +++ b/Documentation/hwmon/omap_temp_sensor @@ -0,0 +1,26 @@ +Kernel driver omap_temp_sensor +== + +Supported chips: + * Texas Instruments OMAP4460 +Prefix: 'omap_temp_sensor' + +Author: +J Keerthy j-keer...@ti.com + +Description +--- + +The Texas Instruments OMAP4 family of chips have a bandgap temperature sensor. +The temperature sensor feature is used to convert the temperature of the device +into a decimal value coded on 10 bits. An internal ADC is used for conversion. +The recommended operating temperatures must be in the range -40 degree Celsius +to 123 degree celsius for standard conversion. +The thresholds are programmable and upon crossing the thresholds an interrupt +is generated. The OMAP temperature sensor has a programmable update rate in +milli seconds. +(Currently the driver programs a default of 2000 milliseconds). + +The driver provides the common sysfs-interface for temperatures (see +Documentation/hwmon/sysfs-interface under Temperatures). + diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 5f888f7..9c9cd8b 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -323,6 +323,17 @@ config SENSORS_F71805F This driver can also be built as a module. If so, the module will be called f71805f. +config SENSORS_OMAP_BANDGAP_TEMP_SENSOR + bool OMAP on-die temperature sensor hwmon driver + depends on HWMON ARCH_OMAP OMAP_TEMP_SENSOR + help + If you say yes here you get support for hardware + monitoring features of the OMAP on die temperature + sensor. + + Continuous conversion programmable delay + mode is used for temperature conversion. + config SENSORS_F71882FG tristate Fintek F71882FG and compatibles help diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 28061cf..d0f89f5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -91,6 +91,7 @@ obj-$(CONFIG_SENSORS_MAX6639) += max6639.o obj-$(CONFIG_SENSORS_MAX6642) += max6642.o obj-$(CONFIG_SENSORS_MAX6650) += max6650.o obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o +obj-$(CONFIG_SENSORS_OMAP_BANDGAP_TEMP_SENSOR) += omap_temp_sensor.o obj-$(CONFIG_SENSORS_PC87360) += pc87360.o obj-$(CONFIG_SENSORS_PC87427) += pc87427.o obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o diff --git a/drivers/hwmon/omap_temp_sensor.c b/drivers/hwmon/omap_temp_sensor.c new file mode 100644 index 000..67fa424 --- /dev/null +++ b/drivers/hwmon/omap_temp_sensor.c @@ -0,0 +1,881 @@ +/* + * OMAP4 Temperature sensor driver file + * + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ + * Author: J Keerthy j-keer...@ti.com + * Author: Moiz Sonasath m-sonas...@ti.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA + * + */ + +#include linux/interrupt.h +#include linux/clk.h +#include linux/io.h +#include linux/slab.h +#include linux/init.h +#include plat/omap_device.h +#include linux/kernel.h +#include linux/device.h +#include linux/jiffies.h +#include linux/hwmon.h +#include linux/hwmon-sysfs.h +#include linux/stddef.h +#include linux/sysfs.h
Re: [PATCH 6/6 V4] hwmon: OMAP4: On die temperature sensor driver
On Thu, Sep 01, 2011 at 12:09:14AM -0400, Paul Walmsley wrote: On Wed, 31 Aug 2011, Guenter Roeck wrote: On Wed, Aug 31, 2011 at 08:36:43PM -0400, Paul Walmsley wrote: Hi Some comments. On Wed, 31 Aug 2011, Keerthy wrote: [ ... ] +} + +/* Sysfs hook functions */ These should be conditionally compiled out if sysfs isn't compiled in. The whole point of the hwmon subsystem is to expose hardware monitoring information to userland using sysfs. hwmon without sysfs doesn't make sense. So, if anything, it might make sense to disable the entire hwmon tree if sysfs is disabled. But please no conditionals in the code. Hmm. This IP block is more than just a sensor. It also can interrupt the CPU and/or trigger a GPIO line (to shut down the chip) if the chip temperature crosses some thresholds. On some OMAPs, the thresholds are fixed; on others, they are software-programmable. That functionality shouldn't require sysfs; it's almost closer to an x86 MCE. So based on your comments, it sounds like we should move that part of the code to a different driver, and just leave the basic software thermal monitoring here? Good point. This definitely requires some thought. hwmon is meant to be hw monitoring, as the name says, not thermal management. Maybe this entire driver should be a thermal driver instead ? Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [lm-sensors] [PATCH 6/6 V3] hwmon: OMAP4: On die temperature sensor driver
On Fri, 2011-08-26 at 07:17 -0400, J, KEERTHY wrote: On Thu, Aug 25, 2011 at 9:26 PM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Thu, 2011-08-25 at 10:06 -0400, Guenter Roeck wrote: On Thu, Aug 25, 2011 at 06:30:07AM -0400, J, KEERTHY wrote: On Wed, Aug 24, 2011 at 10:46 PM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Wed, Aug 24, 2011 at 10:37:12AM -0400, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of the MPU subsystem of the OMAP4. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. Signed-off-by: Keerthy j-keer...@ti.com Cc: Jean Delvare kh...@linux-fr.org Cc: Guenter Roeck guenter.ro...@ericsson.com Cc: lm-sens...@lm-sensors.org [ ... ] + } + + mutex_lock(temp_sensor-sensor_mutex); + /* obtain the T cold value */ + t_cold = omap_temp_sensor_readl(temp_sensor, + temp_sensor-registers-bgap_threshold); + t_cold = (t_cold temp_sensor-registers-threshold_tcold_mask) + __ffs(temp_sensor-registers-threshold_tcold_mask); + + if (t_hot t_cold) { + count = -EINVAL; + goto out; Context specific limitations like this one are not a good idea, since it assumes an order of changing max and max_hysteresis which is not necessarily guaranteed by the application making the change, nor is a change order order well defined or even possible. Applications should not have to bother about this. The hardware behavior is like this. As long as the temperature is below t_hot no interrupts are fired. Once the temperature increases above t_hot we get an interrupt. In order to avoid repeated interrupts we mask the t_hot interrupt in the interrupt handler and enable the t_cold interrupts. So we get a t_cold interrupt only when the temperature drops below the t_cold value. Hence setting the t_cold higher than t_hot will mess up the state machine. t_hot value should be greater than t_cold value. One option would be to update t_cold (max_hyst) automatically in that case. Problem here is that you expect the application to know, depending on the current values of (max, max_hyst), how to update both limits in order. That is not a reasonable expectation. One possible solution would be to have a single function to set both t_cold and t_hot, and call it from both initialization code and from the set functions. That would unify all the register setting and interrupt enable code. Regarding the actual values to set, you can (as an example) use the following approach: - If max is set, and t_hot t_cold, adjust t_cold to the new value of t_hot. - if max_hyst is set, and t_cold t_hot, set t_cold to the new value of t_hot. So, in other words, your new unified function would only need the following simple validation: if (t_cold t_hot) t_cold = t_hot; One concern here. There should be a hysteresis difference between the two and can not be equal. I need to add a hysteresis value to t_hot so as to maintain t_hot t_cold condition. Sure, makes sense. Was that in your original patch ? Just wondering. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6 V3] hwmon: OMAP4: On die temperature sensor driver
On Thu, Aug 25, 2011 at 06:30:07AM -0400, J, KEERTHY wrote: On Wed, Aug 24, 2011 at 10:46 PM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Wed, Aug 24, 2011 at 10:37:12AM -0400, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of the MPU subsystem of the OMAP4. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. Signed-off-by: Keerthy j-keer...@ti.com Cc: Jean Delvare kh...@linux-fr.org Cc: Guenter Roeck guenter.ro...@ericsson.com Cc: lm-sens...@lm-sensors.org Partial review only. Too many concerns, and after a while I tend to loose track. --- Documentation/hwmon/omap_temp_sensor | 27 + drivers/hwmon/Kconfig| 11 + drivers/hwmon/Makefile |1 + drivers/hwmon/omap_temp_sensor.c | 933 ++ 4 files changed, 972 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwmon/omap_temp_sensor create mode 100644 drivers/hwmon/omap_temp_sensor.c [ ... ] +/* Sysfs hook functions */ + +static ssize_t show_temp_max(struct device *dev, + struct device_attribute *devattr, char *buf) +{ + struct omap_temp_sensor *temp_sensor = dev_get_drvdata(dev); + int temp; + + temp = omap_temp_sensor_readl(temp_sensor, + temp_sensor-registers-bgap_threshold); + temp = (temp temp_sensor-registers-threshold_thot_mask) + __ffs(temp_sensor-registers-threshold_thot_mask); + + if (temp OMAP_ADC_START_VALUE || temp OMAP_ADC_END_VALUE) { + dev_err(dev, invalid value\n); + return -EDOM; Please don't use EDOM, and drop the error message. The temperature is out of range. Should i use EIO? It is out of range, but not due to a math error, but due to a bad reading from the chip or due to a chip failure. EIO is ok. + } + + temp = adc_to_temp_conversion(temp); + + return snprintf(buf, 16, %d\n, temp); +} + +static ssize_t set_temp_max(struct device *dev, + struct device_attribute *devattr, + const char *buf, size_t count) +{ + struct omap_temp_sensor *temp_sensor = dev_get_drvdata(dev); + longval; + u32 reg_val, t_cold, t_hot, temp; + + if (strict_strtol(buf, 10, val)) { + count = -EINVAL; + goto out; This will try to release the not-acquired lock. I will correct this. + } + + t_hot = temp_to_adc_conversion(val); Bad error return check. Negative on error. Should be if (t_hot 0) return t_hot; Ok. + if ((t_hot OMAP_ADC_START_VALUE || t_hot OMAP_ADC_END_VALUE)) { Unnecessary (( )) This check is unnecessary with the negative check. I will remove. + count = -EINVAL; + goto out; This will try to release the not-acquired lock. I will correct this. + } + + mutex_lock(temp_sensor-sensor_mutex); + /* obtain the T cold value */ + t_cold = omap_temp_sensor_readl(temp_sensor, + temp_sensor-registers-bgap_threshold); + t_cold = (t_cold temp_sensor-registers-threshold_tcold_mask) + __ffs(temp_sensor-registers-threshold_tcold_mask); + + if (t_hot t_cold) { + count = -EINVAL; + goto out; Context specific limitations like this one are not a good idea, since it assumes an order of changing max and max_hysteresis which is not necessarily guaranteed by the application making the change, nor is a change order order well defined or even possible. Applications should not have to bother about this. The hardware behavior is like this. As long as the temperature is below t_hot no interrupts are fired. Once the temperature increases above t_hot we get an interrupt. In order to avoid repeated interrupts we mask the t_hot interrupt in the interrupt handler and enable the t_cold interrupts. So we get a t_cold interrupt only when the temperature drops below the t_cold value. Hence setting the t_cold higher than t_hot will mess up the state machine. t_hot value should be greater than t_cold value. One option would be to update t_cold (max_hyst) automatically in that case. Problem here is that you expect the aplication to know, depending on the current values of (max, max_hyst), how to update both limits in order. That is not a reasonable expectation. + } + + /* write the new t_hot value */ + reg_val = omap_temp_sensor_readl
Re: [PATCH 6/6 V3] hwmon: OMAP4: On die temperature sensor driver
On Thu, 2011-08-25 at 03:24 -0400, Todd Poynor wrote: On Wed, Aug 24, 2011 at 08:07:12PM +0530, Keerthy wrote: ... + temp_sensor-phy_base = ioremap(mem-start, resource_size(mem)); Check NULL return. temp_sensor-phy_base is never iounmapped in error paths or _remove function. ... +static int __devexit omap_temp_sensor_remove(struct platform_device *pdev) +{ + struct omap_temp_sensor *temp_sensor = platform_get_drvdata(pdev); + + hwmon_device_unregister(pdev-dev); Should be hwmon_device_unregister(temp_sensor-hwmon_dev); since that is what was registered, but then the following accesses using temp_sensor-hwmon_dev won't work anymore and should probably use pdev-dev.kobj as parameter instead. Overall the use of temp_sensor-hwmon_dev vs. pdev-dev looks questionable to me. + kobject_uevent(temp_sensor-hwmon_dev-kobj, KOBJ_REMOVE); + sysfs_remove_group(temp_sensor-hwmon_dev-kobj, + omap_temp_sensor_group); + omap_temp_sensor_clk_disable(temp_sensor); + free_irq(temp_sensor-irq, temp_sensor); free_irq before omap_temp_sensor_clk_disable, to avoid ISR attempting to access hardware while unclocked. Also, t_cold and t_hot interrupts should be disabled before calling free_irq (and actually before anything is unregistered). + clk_put(temp_sensor-clock); + dev_set_drvdata(pdev-dev, NULL); + mutex_destroy(temp_sensor-sensor_mutex); + kfree(temp_sensor); + + return 0; +} Todd -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [lm-sensors] [PATCH 6/6 V3] hwmon: OMAP4: On die temperature sensor driver
On Thu, 2011-08-25 at 10:06 -0400, Guenter Roeck wrote: On Thu, Aug 25, 2011 at 06:30:07AM -0400, J, KEERTHY wrote: On Wed, Aug 24, 2011 at 10:46 PM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Wed, Aug 24, 2011 at 10:37:12AM -0400, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of the MPU subsystem of the OMAP4. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. Signed-off-by: Keerthy j-keer...@ti.com Cc: Jean Delvare kh...@linux-fr.org Cc: Guenter Roeck guenter.ro...@ericsson.com Cc: lm-sens...@lm-sensors.org [ ... ] + } + + mutex_lock(temp_sensor-sensor_mutex); + /* obtain the T cold value */ + t_cold = omap_temp_sensor_readl(temp_sensor, + temp_sensor-registers-bgap_threshold); + t_cold = (t_cold temp_sensor-registers-threshold_tcold_mask) + __ffs(temp_sensor-registers-threshold_tcold_mask); + + if (t_hot t_cold) { + count = -EINVAL; + goto out; Context specific limitations like this one are not a good idea, since it assumes an order of changing max and max_hysteresis which is not necessarily guaranteed by the application making the change, nor is a change order order well defined or even possible. Applications should not have to bother about this. The hardware behavior is like this. As long as the temperature is below t_hot no interrupts are fired. Once the temperature increases above t_hot we get an interrupt. In order to avoid repeated interrupts we mask the t_hot interrupt in the interrupt handler and enable the t_cold interrupts. So we get a t_cold interrupt only when the temperature drops below the t_cold value. Hence setting the t_cold higher than t_hot will mess up the state machine. t_hot value should be greater than t_cold value. One option would be to update t_cold (max_hyst) automatically in that case. Problem here is that you expect the application to know, depending on the current values of (max, max_hyst), how to update both limits in order. That is not a reasonable expectation. One possible solution would be to have a single function to set both t_cold and t_hot, and call it from both initialization code and from the set functions. That would unify all the register setting and interrupt enable code. Regarding the actual values to set, you can (as an example) use the following approach: - If max is set, and t_hot t_cold, adjust t_cold to the new value of t_hot. - if max_hyst is set, and t_cold t_hot, set t_cold to the new value of t_hot. So, in other words, your new unified function would only need the following simple validation: if (t_cold t_hot) t_cold = t_hot; [ ... ] + +static int __devinit omap_temp_sensor_probe(struct platform_device *pdev) +{ + struct omap_temp_sensor_pdata *pdata = pdev-dev.platform_data; + struct omap_temp_sensor *temp_sensor; + struct resource *mem; + int ret = 0; + int val, clk_rate; + u32 max_freq, min_freq; + + if (!pdata) { + dev_err(pdev-dev, platform data missing\n); + return -EINVAL; + } + + temp_sensor = kzalloc(sizeof(*temp_sensor), GFP_KERNEL); + if (!temp_sensor) + return -ENOMEM; + You have error messages all over the place, Why not here ? Ok. I will add an error message here. + mutex_init(temp_sensor-sensor_mutex); + + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!mem) { + dev_err(pdev-dev, no mem resource\n); + ret = -ENOMEM; + goto plat_res_err; + } + If you try to get the resource before allocating the memory, you don't have to release the memory if the platform resource is missing. Ok. Here if i fail i am releasing temp_sensor memory which has been allocated. + temp_sensor-irq = platform_get_irq_byname(pdev, thermal_alert); + if (temp_sensor-irq 0) { + dev_err(pdev-dev, get_irq_byname failed\n); + ret = temp_sensor-irq; + goto plat_res_err; + } + + temp_sensor-phy_base = ioremap(mem-start, resource_size(mem)); + temp_sensor-clock = NULL; + temp_sensor-registers = pdata-registers; + temp_sensor-hwmon_dev = pdev-dev; + + if (pdata-max_freq pdata-min_freq
Re: [PATCH 6/6 V3] hwmon: OMAP4: On die temperature sensor driver
On Thu, 2011-08-25 at 12:04 -0400, J, KEERTHY wrote: On Thu, Aug 25, 2011 at 7:36 PM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Thu, Aug 25, 2011 at 06:30:07AM -0400, J, KEERTHY wrote: On Wed, Aug 24, 2011 at 10:46 PM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Wed, Aug 24, 2011 at 10:37:12AM -0400, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of the MPU subsystem of the OMAP4. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. Signed-off-by: Keerthy j-keer...@ti.com Cc: Jean Delvare kh...@linux-fr.org Cc: Guenter Roeck guenter.ro...@ericsson.com Cc: lm-sens...@lm-sensors.org Partial review only. Too many concerns, and after a while I tend to loose track. --- Documentation/hwmon/omap_temp_sensor | 27 + drivers/hwmon/Kconfig| 11 + drivers/hwmon/Makefile |1 + drivers/hwmon/omap_temp_sensor.c | 933 ++ 4 files changed, 972 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwmon/omap_temp_sensor create mode 100644 drivers/hwmon/omap_temp_sensor.c [ ... ] +/* Sysfs hook functions */ + +static ssize_t show_temp_max(struct device *dev, + struct device_attribute *devattr, char *buf) +{ + struct omap_temp_sensor *temp_sensor = dev_get_drvdata(dev); + int temp; + + temp = omap_temp_sensor_readl(temp_sensor, + temp_sensor-registers-bgap_threshold); + temp = (temp temp_sensor-registers-threshold_thot_mask) + __ffs(temp_sensor-registers-threshold_thot_mask); + + if (temp OMAP_ADC_START_VALUE || temp OMAP_ADC_END_VALUE) { + dev_err(dev, invalid value\n); + return -EDOM; Please don't use EDOM, and drop the error message. The temperature is out of range. Should i use EIO? It is out of range, but not due to a math error, but due to a bad reading from the chip or due to a chip failure. EIO is ok. Ok + } + + temp = adc_to_temp_conversion(temp); + + return snprintf(buf, 16, %d\n, temp); +} + +static ssize_t set_temp_max(struct device *dev, + struct device_attribute *devattr, + const char *buf, size_t count) +{ + struct omap_temp_sensor *temp_sensor = dev_get_drvdata(dev); + longval; + u32 reg_val, t_cold, t_hot, temp; + + if (strict_strtol(buf, 10, val)) { + count = -EINVAL; + goto out; This will try to release the not-acquired lock. I will correct this. + } + + t_hot = temp_to_adc_conversion(val); Bad error return check. Negative on error. Should be if (t_hot 0) return t_hot; Ok. + if ((t_hot OMAP_ADC_START_VALUE || t_hot OMAP_ADC_END_VALUE)) { Unnecessary (( )) This check is unnecessary with the negative check. I will remove. + count = -EINVAL; + goto out; This will try to release the not-acquired lock. I will correct this. + } + + mutex_lock(temp_sensor-sensor_mutex); + /* obtain the T cold value */ + t_cold = omap_temp_sensor_readl(temp_sensor, + temp_sensor-registers-bgap_threshold); + t_cold = (t_cold temp_sensor-registers-threshold_tcold_mask) + __ffs(temp_sensor-registers-threshold_tcold_mask); + + if (t_hot t_cold) { + count = -EINVAL; + goto out; Context specific limitations like this one are not a good idea, since it assumes an order of changing max and max_hysteresis which is not necessarily guaranteed by the application making the change, nor is a change order order well defined or even possible. Applications should not have to bother about this. The hardware behavior is like this. As long as the temperature is below t_hot no interrupts are fired. Once the temperature increases above t_hot we get an interrupt. In order to avoid repeated interrupts we mask the t_hot interrupt in the interrupt handler and enable the t_cold interrupts. So we get a t_cold interrupt only when the temperature drops below the t_cold value. Hence setting the t_cold higher than t_hot will mess up the state machine. t_hot value should be greater than t_cold value. One option would be to update t_cold (max_hyst) automatically
Re: [PATCH 6/6 V3] hwmon: OMAP4: On die temperature sensor driver
On Thu, 2011-08-25 at 12:39 -0400, J, KEERTHY wrote: On Thu, Aug 25, 2011 at 9:49 PM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Thu, 2011-08-25 at 12:04 -0400, J, KEERTHY wrote: On Thu, Aug 25, 2011 at 7:36 PM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Thu, Aug 25, 2011 at 06:30:07AM -0400, J, KEERTHY wrote: On Wed, Aug 24, 2011 at 10:46 PM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Wed, Aug 24, 2011 at 10:37:12AM -0400, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of the MPU subsystem of the OMAP4. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. Signed-off-by: Keerthy j-keer...@ti.com Cc: Jean Delvare kh...@linux-fr.org Cc: Guenter Roeck guenter.ro...@ericsson.com Cc: lm-sens...@lm-sensors.org Partial review only. Too many concerns, and after a while I tend to loose track. --- Documentation/hwmon/omap_temp_sensor | 27 + drivers/hwmon/Kconfig| 11 + drivers/hwmon/Makefile |1 + drivers/hwmon/omap_temp_sensor.c | 933 ++ 4 files changed, 972 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwmon/omap_temp_sensor create mode 100644 drivers/hwmon/omap_temp_sensor.c [ ... ] +/* Sysfs hook functions */ + +static ssize_t show_temp_max(struct device *dev, + struct device_attribute *devattr, char *buf) +{ + struct omap_temp_sensor *temp_sensor = dev_get_drvdata(dev); + int temp; + + temp = omap_temp_sensor_readl(temp_sensor, + temp_sensor-registers-bgap_threshold); + temp = (temp temp_sensor-registers-threshold_thot_mask) + __ffs(temp_sensor-registers-threshold_thot_mask); + + if (temp OMAP_ADC_START_VALUE || temp OMAP_ADC_END_VALUE) { + dev_err(dev, invalid value\n); + return -EDOM; Please don't use EDOM, and drop the error message. The temperature is out of range. Should i use EIO? It is out of range, but not due to a math error, but due to a bad reading from the chip or due to a chip failure. EIO is ok. Ok + } + + temp = adc_to_temp_conversion(temp); + + return snprintf(buf, 16, %d\n, temp); +} + +static ssize_t set_temp_max(struct device *dev, + struct device_attribute *devattr, + const char *buf, size_t count) +{ + struct omap_temp_sensor *temp_sensor = dev_get_drvdata(dev); + longval; + u32 reg_val, t_cold, t_hot, temp; + + if (strict_strtol(buf, 10, val)) { + count = -EINVAL; + goto out; This will try to release the not-acquired lock. I will correct this. + } + + t_hot = temp_to_adc_conversion(val); Bad error return check. Negative on error. Should be if (t_hot 0) return t_hot; Ok. + if ((t_hot OMAP_ADC_START_VALUE || t_hot OMAP_ADC_END_VALUE)) { Unnecessary (( )) This check is unnecessary with the negative check. I will remove. + count = -EINVAL; + goto out; This will try to release the not-acquired lock. I will correct this. + } + + mutex_lock(temp_sensor-sensor_mutex); + /* obtain the T cold value */ + t_cold = omap_temp_sensor_readl(temp_sensor, + temp_sensor-registers-bgap_threshold); + t_cold = (t_cold temp_sensor-registers-threshold_tcold_mask) + __ffs(temp_sensor-registers-threshold_tcold_mask); + + if (t_hot t_cold) { + count = -EINVAL; + goto out; Context specific limitations like this one are not a good idea, since it assumes an order of changing max and max_hysteresis which is not necessarily guaranteed by the application making the change, nor is a change order order well defined or even possible. Applications should not have to bother about this. The hardware behavior is like this. As long as the temperature is below t_hot no interrupts are fired. Once the temperature increases above t_hot we get an interrupt. In order to avoid repeated interrupts we mask the t_hot interrupt in the interrupt handler and enable the t_cold interrupts. So we
Re: [PATCH 6/6 V3] hwmon: OMAP4: On die temperature sensor driver
On Wed, Aug 24, 2011 at 10:37:12AM -0400, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of the MPU subsystem of the OMAP4. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. Signed-off-by: Keerthy j-keer...@ti.com Cc: Jean Delvare kh...@linux-fr.org Cc: Guenter Roeck guenter.ro...@ericsson.com Cc: lm-sens...@lm-sensors.org Partial review only. Too many concerns, and after a while I tend to loose track. --- Documentation/hwmon/omap_temp_sensor | 27 + drivers/hwmon/Kconfig| 11 + drivers/hwmon/Makefile |1 + drivers/hwmon/omap_temp_sensor.c | 933 ++ 4 files changed, 972 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwmon/omap_temp_sensor create mode 100644 drivers/hwmon/omap_temp_sensor.c diff --git a/Documentation/hwmon/omap_temp_sensor b/Documentation/hwmon/omap_temp_sensor new file mode 100644 index 000..e01a6d6 --- /dev/null +++ b/Documentation/hwmon/omap_temp_sensor @@ -0,0 +1,27 @@ +Kernel driver omap_temp_sensor +== + +Supported chips: + * Texas Instruments OMAP4460 +Prefix: 'omap_temp_sensor' +Datasheet: http://focus.ti.com/docs/prod/folders/print/tmp102.html + I am a bit lost here. What does the TMP102 have to do with the OMAP4460 ? +Author: +J Keerthy j-keer...@ti.com + +Description +--- + +The Texas Instruments OMAP4 family of chips have a bandgap temperature sensor. +The temperature sensor feature is used to convert the temperature of the device +into a decimal value coded on 10 bits. An internal ADC is used for conversion. +The recommended operating temperatures must be in the range -40 degree Celsius +to 123 degree celsius for standard conversion. +The thresholds are programmable and upon crossing the thresholds an interrupt +is generated. The OMAP temperature sensor has a programmable update rate in +milli seconds. +(Currently the driver programs a default of 2000 milli seconds). + s/milli seconds/milliseconds/ +The driver provides the common sysfs-interface for temperatures (see +Documentation/hwmon/sysfs-interface under Temperatures). + diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 5f888f7..9c9cd8b 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -323,6 +323,17 @@ config SENSORS_F71805F This driver can also be built as a module. If so, the module will be called f71805f. +config SENSORS_OMAP_BANDGAP_TEMP_SENSOR + bool OMAP on-die temperature sensor hwmon driver + depends on HWMON ARCH_OMAP OMAP_TEMP_SENSOR + help + If you say yes here you get support for hardware + monitoring features of the OMAP on die temperature + sensor. + + Continuous conversion programmable delay + mode is used for temperature conversion. + config SENSORS_F71882FG tristate Fintek F71882FG and compatibles help diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 28061cf..d0f89f5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -91,6 +91,7 @@ obj-$(CONFIG_SENSORS_MAX6639) += max6639.o obj-$(CONFIG_SENSORS_MAX6642) += max6642.o obj-$(CONFIG_SENSORS_MAX6650) += max6650.o obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o +obj-$(CONFIG_SENSORS_OMAP_BANDGAP_TEMP_SENSOR) += omap_temp_sensor.o obj-$(CONFIG_SENSORS_PC87360) += pc87360.o obj-$(CONFIG_SENSORS_PC87427) += pc87427.o obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o diff --git a/drivers/hwmon/omap_temp_sensor.c b/drivers/hwmon/omap_temp_sensor.c new file mode 100644 index 000..66fb3a9 --- /dev/null +++ b/drivers/hwmon/omap_temp_sensor.c @@ -0,0 +1,933 @@ +/* + * OMAP4 Temperature sensor driver file + * + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ + * Author: J Keerthy j-keer...@ti.com + * Author: Moiz Sonasath m-sonas...@ti.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA + * + */ + +#include linux/interrupt.h +#include linux/clk.h +#include linux/io.h
Re: [RFC PATCH 6/6 V2] hwmon: OMAP4: On die temperature sensor driver
On Fri, Aug 19, 2011 at 02:04:58AM -0400, J, KEERTHY wrote: On Fri, Aug 19, 2011 at 7:43 AM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Thu, Aug 18, 2011 at 06:52:15AM -0400, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of the MPU subsystem of the OMAP4. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. Signed-off-by: Keerthy j-keer...@ti.com Cc: Jean Delvare kh...@linux-fr.org Cc: Guenter Roeck guenter.ro...@ericsson.com Cc: lm-sens...@lm-sensors.org High level review: - too much and too broad mutex locking. show functions should not need locks at all, set functions only while data is written into registers and into platform data. Ok. I will clean this. - driver is quite noisy. There should definitely not be any log messages if a set parameter is wrong. Show functions already return an error value to the user; a log message indicating the error again just creates noise. For one boolean set during probe (is_efuse_valid), each subsequent show results in a log message if it is false. Some errors result in multiple log messages. A user tries to set an invalid temperature threshold. The user should be notified about this. The invalid temperature will not be set. The user should not be allowed to set an invalid temperature. It is to inform the user about precisely the problem with the parameter. User is notified with -EINVAL. Unless on the console, which is unlikely, the user will likely not notice a message in the kernel log. In some of the samples the bandgap is not trimmed and hence temperature reported will be wrong. So every time a user tries to read he is alerted that the temperatures are not accurate. In the kernel log ? Sorry, that doesn't make sense. You alert the system administrator, not the user. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 6/6 V2] hwmon: OMAP4: On die temperature sensor driver
On Fri, Aug 19, 2011 at 09:01:53AM -0400, J, KEERTHY wrote: On Fri, Aug 19, 2011 at 11:47 AM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Fri, Aug 19, 2011 at 02:04:58AM -0400, J, KEERTHY wrote: On Fri, Aug 19, 2011 at 7:43 AM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Thu, Aug 18, 2011 at 06:52:15AM -0400, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of the MPU subsystem of the OMAP4. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. Signed-off-by: Keerthy j-keer...@ti.com Cc: Jean Delvare kh...@linux-fr.org Cc: Guenter Roeck guenter.ro...@ericsson.com Cc: lm-sens...@lm-sensors.org High level review: - too much and too broad mutex locking. show functions should not need locks at all, set functions only while data is written into registers and into platform data. Ok. I will clean this. - driver is quite noisy. There should definitely not be any log messages if a set parameter is wrong. Show functions already return an error value to the user; a log message indicating the error again just creates noise. For one boolean set during probe (is_efuse_valid), each subsequent show results in a log message if it is false. Some errors result in multiple log messages. A user tries to set an invalid temperature threshold. The user should be notified about this. The invalid temperature will not be set. The user should not be allowed to set an invalid temperature. It is to inform the user about precisely the problem with the parameter. User is notified with -EINVAL. Unless on the console, which is unlikely, the user will likely not notice a message in the kernel log. These messages on console are useful for debug purpose so can i place it under dev_dbg? Only on explicit enabling these messages can be seen. Not really for user errors. The user is already alerted with EINVAL that the parameter was invalid. _Why_ it was invalid should be well defined. Are you really sure you want to have something like user entered 'Hello' as temperature even in a debug log ? The debug log should be to debug driver problems and behavior, not to debug wrong parameters entered by users. In some of the samples the bandgap is not trimmed and hence temperature reported will be wrong. So every time a user tries to read he is alerted that the temperatures are not accurate. In the kernel log ? Sorry, that doesn't make sense. You alert the system administrator, not the user. Ok. Can this be a dev_dbg message? Only for debug purpose this can be useful. Yes, but even then you need the message only once. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 6/6 V2] hwmon: OMAP4: On die temperature sensor driver
On Thu, Aug 18, 2011 at 06:52:15AM -0400, Keerthy wrote: On chip temperature sensor driver. The driver monitors the temperature of the MPU subsystem of the OMAP4. It sends notifications to the user space if the temperature crosses user defined thresholds via kobject_uevent interface. The user is allowed to configure the temperature thresholds vis sysfs nodes exposed using hwmon interface. Signed-off-by: Keerthy j-keer...@ti.com Cc: Jean Delvare kh...@linux-fr.org Cc: Guenter Roeck guenter.ro...@ericsson.com Cc: lm-sens...@lm-sensors.org High level review: - too much and too broad mutex locking. show functions should not need locks at all, set functions only while data is written into registers and into platform data. - driver is quite noisy. There should definitely not be any log messages if a set parameter is wrong. Show functions already return an error value to the user; a log message indicating the error again just creates noise. For one boolean set during probe (is_efuse_valid), each subsequent show results in a log message if it is false. Some errors result in multiple log messages. - Wrong use of EINVAL throughout the driver (EINVAL is Invalid Argument) - excessive ( ) - linear search through a sorted array is very expensive. Consider using a binary search. - temp_to_adc_conversion return code (error if negative) is not checked properly. I am sure there are other problems, but those are difficult to find with all the noise above. Guenter --- Documentation/hwmon/omap_temp_sensor | 27 + drivers/hwmon/Kconfig| 11 + drivers/hwmon/Makefile |1 + drivers/hwmon/omap_temp_sensor.c | 918 ++ 4 files changed, 957 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwmon/omap_temp_sensor create mode 100644 drivers/hwmon/omap_temp_sensor.c diff --git a/Documentation/hwmon/omap_temp_sensor b/Documentation/hwmon/omap_temp_sensor new file mode 100644 index 000..e01a6d6 --- /dev/null +++ b/Documentation/hwmon/omap_temp_sensor @@ -0,0 +1,27 @@ +Kernel driver omap_temp_sensor +== + +Supported chips: + * Texas Instruments OMAP4460 +Prefix: 'omap_temp_sensor' +Datasheet: http://focus.ti.com/docs/prod/folders/print/tmp102.html + +Author: +J Keerthy j-keer...@ti.com + +Description +--- + +The Texas Instruments OMAP4 family of chips have a bandgap temperature sensor. +The temperature sensor feature is used to convert the temperature of the device +into a decimal value coded on 10 bits. An internal ADC is used for conversion. +The recommended operating temperatures must be in the range -40 degree Celsius +to 123 degree celsius for standard conversion. +The thresholds are programmable and upon crossing the thresholds an interrupt +is generated. The OMAP temperature sensor has a programmable update rate in +milli seconds. +(Currently the driver programs a default of 2000 milli seconds). + +The driver provides the common sysfs-interface for temperatures (see +Documentation/hwmon/sysfs-interface under Temperatures). + diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 5f888f7..9c9cd8b 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -323,6 +323,17 @@ config SENSORS_F71805F This driver can also be built as a module. If so, the module will be called f71805f. +config SENSORS_OMAP_BANDGAP_TEMP_SENSOR + bool OMAP on-die temperature sensor hwmon driver + depends on HWMON ARCH_OMAP OMAP_TEMP_SENSOR + help + If you say yes here you get support for hardware + monitoring features of the OMAP on die temperature + sensor. + + Continuous conversion programmable delay + mode is used for temperature conversion. + config SENSORS_F71882FG tristate Fintek F71882FG and compatibles help diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 28061cf..d0f89f5 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -91,6 +91,7 @@ obj-$(CONFIG_SENSORS_MAX6639) += max6639.o obj-$(CONFIG_SENSORS_MAX6642) += max6642.o obj-$(CONFIG_SENSORS_MAX6650) += max6650.o obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o +obj-$(CONFIG_SENSORS_OMAP_BANDGAP_TEMP_SENSOR) += omap_temp_sensor.o obj-$(CONFIG_SENSORS_PC87360) += pc87360.o obj-$(CONFIG_SENSORS_PC87427) += pc87427.o obj-$(CONFIG_SENSORS_PCF8591) += pcf8591.o diff --git a/drivers/hwmon/omap_temp_sensor.c b/drivers/hwmon/omap_temp_sensor.c new file mode 100644 index 000..586a361 --- /dev/null +++ b/drivers/hwmon/omap_temp_sensor.c @@ -0,0 +1,918 @@ +/* + * OMAP4 Temperature sensor driver file + * + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ + * Author: J Keerthy j-keer...@ti.com + * Author: Moiz Sonasath m-sonas...@ti.com
Re: [lm-sensors] [RFC PATCH 6/6] hwmon: OMAP4: On die temperature sensor driver
On Wed, Aug 17, 2011 at 06:37:10AM -0400, J, KEERTHY wrote: [ ... ] Hi Guenter, Any inputs on the driver? Hi Keerthy. I think there was so much feedback that I would just add noise at this time, and stumble over the same issues. I'll look into it after the feedback has been applied. Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 6/6] hwmon: OMAP4: On die temperature sensor driver
On Thu, 2011-08-11 at 09:00 -0400, J, KEERTHY wrote: On Thu, Aug 11, 2011 at 4:06 PM, Felipe Balbi ba...@ti.com wrote: [ ... ] + temp = omap_temp_sensor_readl(temp_sensor, + temp_sensor-registers-bgap_counter); + temp = (temp temp_sensor-registers-counter_mask) + __ffs(temp_sensor-registers-counter_mask); temp = ?? + temp = temp * 1000 / (temp_sensor-clk_rate); temp *= ?? Need to multiply the temp with 1000 before dividing. temp *= evaluates the RHS first and then multiplies LHS. temp *= 1000; temp /= clk_rate; Different coding style. I preferred to do it in a single line. Me too. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [lm-sensors] Query: Omap temperature sensor driver design
On Fri, Apr 01, 2011 at 08:39:43AM -0400, J, KEERTHY wrote: Hello All, I am trying to implement a driver for the OMAP temperature sensor. It is an on chip sensor. The sensor is responsible for reporting the temperature. The sensor has configurable thresholds. The user can configure the thresholds. An interrupt will be generated as soon as there the temperature thresholds are crossed. Two possible approaches for the driver: 1) The entire driver resides in drivers/hwmon directory. The driver containing all the sysfs nodes to be exposed to the user. The interrupt handlers are also part of this driver. The device registration happens in a OMAP arch specific file residing in arch/arm/mach-omap2 directory. 2) The intialization and the interrupt handling done in a separate driver file residing in in arch/arm/mach-omap2 or drivers/misc directory. The device registration happens in a OMAP arch specific file residing in arch/arm/mach-omap2 directory. Only the sysfs nodes will be exposed through a hwmon driver residing in drivers/hwmon. That really depends if it does anything else besides hw monitoring. A hw monitoring driver can support interrupts, and trigger a poll event on an alarm file if it gets one. But it should only perform hw monitoring functionality. If that interrupt triggers other activity, a thermal subsystem driver may be more appropriate. Thanks, Guenter Please suggest the best alternative among the two or if any new design for the requirements is better. -- Regards and Thanks, Keerthy ___ lm-sensors mailing list lm-sens...@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2 v5] hwmon: twl4030: Hwmon Driver for TWL4030 MADC
On Tue, Mar 01, 2011 at 08:42:36AM -0500, Keerthy wrote: This driver exposes the sysfs nodes of the TWL4030 MADC module. All the voltage channel values are expressed in terms of mV. Channel 13 and channel 14 are reserved. There are channels which represent temperature and current the output is represented by celcius and mA respectively. Signed-off-by: Keerthy j-keer...@ti.com Acked-by: Guenter Roeck guenter.ro...@ericsson.com Thanks, Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v3] hwmon: twl4030: Hwmon Driver for TWL4030 MADC
Hi, On Mon, Feb 28, 2011 at 01:05:41AM -0500, Keerthy wrote: This driver exposes the sysfs nodes of the TWL4030 MADC module. All the voltage channel values are expressed in terms of mV. Channel 13 and channel 14 are reserved. There are channels which represent temperature and current the output is represented by celcius and mA respectively. Signed-off-by: Keerthy j-keer...@ti.com --- V3: Corrected channel 15 index V2: Changed the names of the sysfs attributes compliant to current, voltage and temperature attributes. V1: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg44542.html Documentation/hwmon/twl4030-madc-hwmon | 45 + drivers/hwmon/Kconfig | 10 ++ drivers/hwmon/Makefile |1 + drivers/hwmon/twl4030-madc-hwmon.c | 153 4 files changed, 209 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwmon/twl4030-madc-hwmon create mode 100644 drivers/hwmon/twl4030-madc-hwmon.c diff --git a/Documentation/hwmon/twl4030-madc-hwmon b/Documentation/hwmon/twl4030-madc-hwmon new file mode 100644 index 000..ef79843 --- /dev/null +++ b/Documentation/hwmon/twl4030-madc-hwmon @@ -0,0 +1,45 @@ +Kernel driver twl4030-madc += + +Supported chips: + * Texas Instruments TWL4030 + Prefix: 'twl4030-madc' + + +Authors: + J Keerthy j-keer...@ti.com + +Description +--- + +The Texas Instruments TWL4030 is a Power Management and Audio Circuit. Among +other things it contains a 10-bit A/D converter MADC. The converter has 16 +channels which can be used in different modes. + + +See this table for the meaning of the different channels + +Channel Signal +-- +0Battery type(BTYPE) +1BCI: Battery temperature (BTEMP) +2GP analog input +3GP analog input +4GP analog input +5GP analog input +6GP analog input +7GP analog input +8BCI: VBUS voltage(VBUS) +9Backup Battery voltage (VBKP) +10 BCI: Battery charger current (ICHG) +11 BCI: Battery charger voltage (VCHG) +12 BCI: Main battery voltage (VBAT) +13 Reserved +14 Reserved +15 VRUSB Supply/Speaker left/Speaker right polarization level + + +The Sysfs nodes will represent the voltage in the units of mV, +the temperature channel shows the converted temperature in +degree celcius. The Battery charging current channel represents +battery charging current in mA. diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 297bc9a..cb3d895 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -941,6 +941,16 @@ config SENSORS_TMP421 This driver can also be built as a module. If so, the module will be called tmp421. +config SENSORS_TWL4030_MADC + tristate Texas Instruments TWL4030 MADC Hwmon + depends on TWL4030_MADC + help + If you say yes here you get hwmon support for triton + TWL4030-MADC. + + This driver can also be built as a module. If so it will be called + twl4030-madc-hwmon. + config SENSORS_VIA_CPUTEMP tristate VIA CPU temperature sensor depends on X86 diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index dde02d9..bc7d740 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -102,6 +102,7 @@ obj-$(CONFIG_SENSORS_THMC50) += thmc50.o obj-$(CONFIG_SENSORS_TMP102) += tmp102.o obj-$(CONFIG_SENSORS_TMP401) += tmp401.o obj-$(CONFIG_SENSORS_TMP421) += tmp421.o +obj-$(CONFIG_SENSORS_TWL4030_MADC)+= twl4030-madc-hwmon.o obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o obj-$(CONFIG_SENSORS_VIA686A)+= via686a.o obj-$(CONFIG_SENSORS_VT1211) += vt1211.o diff --git a/drivers/hwmon/twl4030-madc-hwmon.c b/drivers/hwmon/twl4030-madc-hwmon.c new file mode 100644 index 000..5eb563f --- /dev/null +++ b/drivers/hwmon/twl4030-madc-hwmon.c @@ -0,0 +1,153 @@ +/* + * + * TWL4030 MADC Hwmon driver-This driver monitors the real time + * conversion of analog signals like battery temperature, + * battery type, battery level etc. User can ask for the conversion on a + * particular channel using the sysfs nodes. + * + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ + * J Keerthy j-keer...@ti.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + *
Re: [PATCH 2/2 v4] mfd: twl4030: Driver for twl4030 madc module
On Mon, Feb 28, 2011 at 04:48:12AM -0500, Keerthy wrote: Introducing a driver for MADC on TWL4030 powerIC. MADC stands for monitoring ADC. This driver monitors the real time conversion of analog signals like battery temperature, battery cuurent etc. Signed-off-by: Keerthy j-keer...@ti.com --- V4: Removed unwanted else from twl4030_madc_read_channels functions. V3: Added for_each_set_bit for efficient bit checking and locking the mutex earlier in the threaded irq handler. V2: Added functions to convert raw voltages to current and temperature. V1: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg44543.html drivers/mfd/Kconfig | 10 + drivers/mfd/Makefile |1 + drivers/mfd/twl4030-madc.c | 812 ++ include/linux/i2c/twl4030-madc.h | 142 +++ 4 files changed, 965 insertions(+), 0 deletions(-) create mode 100644 drivers/mfd/twl4030-madc.c create mode 100644 include/linux/i2c/twl4030-madc.h diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig index fd01836..029e078 100644 --- a/drivers/mfd/Kconfig +++ b/drivers/mfd/Kconfig @@ -167,6 +167,16 @@ config TWL4030_CORE high speed USB OTG transceiver, an audio codec (on most versions) and many other features. +config TWL4030_MADC + tristate Texas Instruments 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 Support power resources on TWL4030 family chips depends on TWL4030_CORE ARM diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile index a54e2c7..2922cc2 100644 --- a/drivers/mfd/Makefile +++ b/drivers/mfd/Makefile @@ -37,6 +37,7 @@ obj-$(CONFIG_TPS6507X)+= tps6507x.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_TWL4030_CODEC)+= twl4030-codec.o obj-$(CONFIG_TWL6030_PWM) += twl6030-pwm.o diff --git a/drivers/mfd/twl4030-madc.c b/drivers/mfd/twl4030-madc.c new file mode 100644 index 000..e533c72 --- /dev/null +++ b/drivers/mfd/twl4030-madc.c @@ -0,0 +1,812 @@ +/* + * + * TWL4030 MADC module driver-This driver monitors the real time + * conversion of analog signals like battery temperature, + * battery type, battery level etc. + * + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/ + * J Keerthy j-keer...@ti.com + * + * Based on twl4030-madc.c + * Copyright (C) 2008 Nokia Corporation + * Mikko Ylinen mikko.k.yli...@nokia.com + * + * Amit Kucheria amit.kuche...@canonical.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA + * + */ + +#include linux/interrupt.h +#include linux/delay.h +#include linux/platform_device.h +#include linux/slab.h +#include linux/i2c/twl.h +#include linux/i2c/twl4030-madc.h + I didn't have a close look, but I suspect that several includes are missing here. See Documentation/SubmitChecklist, item #1. +/* + * struct twl4030_madc_data - a container for madc info + * @dev - pointer to device structure for madc + * @lock - mutex protecting this data structire + * @requests - Array of request struct corresponding to SW1, SW2 and RT + * @imr - Interrupt mask register of MADC + * @isr - Interrupt status register of MADC + */ +struct twl4030_madc_data { + struct device *dev; + struct mutex lock; /* mutex protecting this data structire */ + struct twl4030_madc_request requests[TWL4030_MADC_NUM_METHODS]; + int imr; + int isr; +}; + +static struct twl4030_madc_data *twl4030_madc; + +struct twl4030_prescale_divider_ratios { + s16 numerator; + s16 denominator; +}; + +static const struct twl4030_prescale_divider_ratios +twl4030_divider_ratios[16] = { + {1, 1}, /* CHANNEL 0 No Prescaler */ + {1, 1}, /* CHANNEL 1 No Prescaler */ + {6, 10},/* CHANNEL 2 */ + {6, 10},/* CHANNEL 3 */ +
Re: [PATCH 2/2 v2] hwmon: twl4030: Hwmon Driver for TWL4030 MADC
On Sun, Feb 27, 2011 at 08:03:07PM -0500, Samuel Ortiz wrote: Hi Keerthy, On Thu, Feb 24, 2011 at 08:48:50PM +0530, Keerthy wrote: This driver exposes the sysfs nodes of the TWL4030 MADC module. All the voltage channel values are expressed in terms of mV. Channel 13 and channel 14 are reserved. There are channels which represent temperature and current the output is represented by celcius and mA respectively. It would make sense for me to carry both patches through the MFD tree, but I'd like to get Guenter's ACK before pushing it upstream. Main problem I have with the hwmon part is the sensor index value and how it maps to adc channels. Specifically, channels 13 and 14 are listed as reserved, yet index value 13 is used without further explanation as bit map value passed on to twl4030_madc_conversion() to obtain the value for channel 15. Either this is wrong, or it asks for a detailed explanation. I didn't have time to track down which one it is. And even if it isn't wrong, it doesn't seem to be the right thing to do since it adds a lot of confusion. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] hwmon: twl4030: Hwmon Driver for TWL4030 MADC
On Wed, Feb 16, 2011 at 07:56:57AM -0500, Keerthy wrote: This driver exposes the sysfs nodes of the TWL4030 MADC module. All the channel values are expressed in terms of mV. Channel 13 and channel 14 are reserved. There are channels which represent temperature and current. Even they output raw voltage in mV. Why ? Signed-off-by: Keerthy j-keer...@ti.com --- The previous discussion concluded in keeping the generic ADC functionality and the hwmon separate. The discussion can be found here: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg41805.html Documentation/hwmon/twl4030-madc-hwmon | 45 + drivers/hwmon/Kconfig | 10 ++ drivers/hwmon/Makefile |1 + drivers/hwmon/twl4030-madc-hwmon.c | 155 4 files changed, 211 insertions(+), 0 deletions(-) create mode 100644 Documentation/hwmon/twl4030-madc-hwmon create mode 100644 drivers/hwmon/twl4030-madc-hwmon.c diff --git a/Documentation/hwmon/twl4030-madc-hwmon b/Documentation/hwmon/twl4030-madc-hwmon new file mode 100644 index 000..2cf1cc2 --- /dev/null +++ b/Documentation/hwmon/twl4030-madc-hwmon @@ -0,0 +1,45 @@ +Kernel driver twl4030-madc += + +Supported chips: + * Texas Instruments TWL4030 + Prefix: 'twl4030-madc' + + +Authors: + J Keerthy j-keer...@ti.com + +Description +--- + +The Texas Instruments TWL4030 is a Power Management and Audio Circuit. Among +other things it contains a 10-bit A/D converter MADC. The converter has 16 +channels which can be used in different modes. + + +See this table for the meaning of the different channels + +Channel Signal +-- +0Battery type(BTYPE) +1BCI: Battery temperature (BTEMP) +2GP analog input +3GP analog input +4GP analog input +5GP analog input +6GP analog input +7GP analog input +8BCI: VBUS voltage(VBUS) +9Backup Battery voltage (VBKP) +10 BCI: Battery charger current (ICHG) +11 BCI: Battery charger voltage (VCHG) +12 BCI: Main battery voltage (VBAT) +13 Reserved +14 Reserved +15 VRUSB Supply/Speaker left/Speaker right polarization level + + +The Sysfs nodes will represent the voltage in the units of mV, +the temperature channel shows the converted raw voltage in mV. +The Battery charging current channel represents raw voltage mV. Doesn't really make sense to me to export values known to be current and temperature as voltages. You'll have to add a lot more information for that to make sense. +Channel 13 and 14 are reserved. You already said that above. diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 773e484..11ddde3 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -940,6 +940,16 @@ config SENSORS_TMP421 This driver can also be built as a module. If so, the module will be called tmp421. +config SENSORS_TWL4030_MADC + tristate Texas Instruments TWL4030 MADC Hwmon + depends on TWL4030_MADC + help + This driver provides hwmon support for triton TWL4030-MADC. + This driver can be built as part of kernel or can be built + as a module. Kernel is obvious. Please stick with the usual text This driver can also be built as a module. If so, the module will be called At least please tell users how the module is named. + This driver exposes the various voltage values to + the user space. + Not sure if this provides any value. Either case, it should be before built as module. config SENSORS_VIA_CPUTEMP tristate VIA CPU temperature sensor depends on X86 diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index dde02d9..bc7d740 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -102,6 +102,7 @@ obj-$(CONFIG_SENSORS_THMC50) += thmc50.o obj-$(CONFIG_SENSORS_TMP102) += tmp102.o obj-$(CONFIG_SENSORS_TMP401) += tmp401.o obj-$(CONFIG_SENSORS_TMP421) += tmp421.o +obj-$(CONFIG_SENSORS_TWL4030_MADC)+= twl4030-madc-hwmon.o obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o obj-$(CONFIG_SENSORS_VIA686A)+= via686a.o obj-$(CONFIG_SENSORS_VT1211) += vt1211.o diff --git a/drivers/hwmon/twl4030-madc-hwmon.c b/drivers/hwmon/twl4030-madc-hwmon.c new file mode 100644 index 000..4a61e8a --- /dev/null +++ b/drivers/hwmon/twl4030-madc-hwmon.c @@ -0,0 +1,155 @@ +/* + * + * TWL4030 MADC Hwmon driver-This driver monitors the real time + * conversion of analog signals like battery temperature, + * battery type, battery level etc. User can ask for the conversion on a + * particular channel using the sysfs nodes. + * + * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com/ 2011 ? + * J Keerthy j-keer...@ti.com + * + * This program is free software; you can redistribute it and/or + *
Re: [PATCH 2/2] hwmon: twl4030: Hwmon Driver for TWL4030 MADC
On Wed, Feb 16, 2011 at 12:59:52PM -0500, J, KEERTHY wrote: Hello Guenter, On Wed, Feb 16, 2011 at 9:39 PM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Wed, Feb 16, 2011 at 07:56:57AM -0500, Keerthy wrote: This driver exposes the sysfs nodes of the TWL4030 MADC module. All the channel values are expressed in terms of mV. Channel 13 and channel 14 are reserved. There are channels which represent temperature and current. Even they output raw voltage in mV. Why ? The conversion to current and temperature in case of MADC depends on a register in the battery module. Hence battery driver can expose the converted value. So providing the raw voltage here. Not a good reason. You could create an API to let you retrieve the register values. You could read the respective registers directly. You could provide the register values in platform data. If there will always be just one instance of the driver, you could provide the register values via module parameters. You could present the raw temperature/current values and correct it with sensors3.conf. Either case, I don't think it is a good idea to present known temperatures or currents as voltage. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [lm-sensors] [PATCH v3] hwmon: twl4030: Driver for twl4030 madc module
On Fri, Jan 07, 2011 at 07:12:13AM -0500, J, KEERTHY wrote: On Fri, Jan 7, 2011 at 3:25 AM, Guenter Roeck guenter.ro...@ericsson.com wrote: On Thu, 2011-01-06 at 15:21 -0500, Mark Brown wrote: On Thu, Jan 06, 2011 at 07:04:30AM -0800, Guenter Roeck wrote: On Thu, Jan 06, 2011 at 07:07:13AM -0500, Mark Brown wrote: Why? It's not like hwmon has an unreasonably large core or similar. Because it creates an unnecessary dependency, and because it is not hwmon's responsibility to provide infrastructure for other subsystems or drivers. hwmon isn't really doing anything, though. The *driver* is doing something but it doesn't really impact the core that much. Not that I'm particularly sold on putting the ADC core in here, but total NACK based on that alone seems rather harsh. Possibly. However, I had suggested the following earlier (to the 1st version of the patch): I commented on this a couple of times below - the driver mixes generic ADC reading functions with hwmon functionality. Generic ADC reading functionality should be moved into another driver, possibly to mfd. Obviously that was ignored. Maybe someone is willing to listen this time around. I am sorry for not responding on the generic ADC handling part. Since many other ADC drivers are part of hwmon i thought hwmon was the appropriate place. However I can surely split the generic ADC handling part in mfd and only hardware monitoring part in hwmon as suggested. Other drivers don't _export_ that functionality. Key difference. Sure, the lis3 driver does, but that should not be in hwmon in the first place and is on the way out (if I ever get to do it), and max is just setting a bad example. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [lm-sensors] [PATCH v3] hwmon: twl4030: Driver for twl4030 madc module
On Thu, Jan 06, 2011 at 07:07:13AM -0500, Mark Brown wrote: On Wed, Jan 05, 2011 at 09:33:28PM -0800, Guenter Roeck wrote: [...] +EXPORT_SYMBOL_GPL(twl4030_madc_conversion); [...] +EXPORT_SYMBOL_GPL(twl4030_get_madc_conversion); No symbol export from hwmon drivers. Other parts of the kernel should not depend on HWMON configuration. Why? It's not like hwmon has an unreasonably large core or similar. Because it creates an unnecessary dependency, and because it is not hwmon's responsibility to provide infrastructure for other subsystems or drivers. I would suggest to check if drivers/staging/iio would be a better fit. That does have the problem that it's in staging and constantly churning, though. When I've looked at it it seemed like awfully hard work to use for devices like this. What I've done in some of my drivers is put the ADC core in the MFD core (it's used by both hwmon and power supply function drivers, plus any board specific stuff people do). Fine as well. I think I had suggested that earlier already. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [lm-sensors] [PATCH v3] hwmon: twl4030: Driver for twl4030 madc module
On Thu, 2011-01-06 at 15:21 -0500, Mark Brown wrote: On Thu, Jan 06, 2011 at 07:04:30AM -0800, Guenter Roeck wrote: On Thu, Jan 06, 2011 at 07:07:13AM -0500, Mark Brown wrote: Why? It's not like hwmon has an unreasonably large core or similar. Because it creates an unnecessary dependency, and because it is not hwmon's responsibility to provide infrastructure for other subsystems or drivers. hwmon isn't really doing anything, though. The *driver* is doing something but it doesn't really impact the core that much. Not that I'm particularly sold on putting the ADC core in here, but total NACK based on that alone seems rather harsh. Possibly. However, I had suggested the following earlier (to the 1st version of the patch): I commented on this a couple of times below - the driver mixes generic ADC reading functions with hwmon functionality. Generic ADC reading functionality should be moved into another driver, possibly to mfd. Obviously that was ignored. Maybe someone is willing to listen this time around. I won't let people break modularity just for convenience in a subsystem I am responsible for. And forcing the hwmon subsystem, and with it a specific hwmon driver, to exist just because the adc functionality it provides is needed by some other (most likely unrelated) subsystem / driver _does_ break modularity. Worse, it is completely unnecessary to do so. Other twl4030 functionality was extracted into generic code. twl-core.c, twl4030-codec.c, twl4030-irq.c, twl4030-power.c are all in mfd. I fail to see the problem with mfd/twl4030-adc.c. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] hwmon: twl4030: Driver for twl4030 madc module
On Wed, Jan 05, 2011 at 11:17:37PM -0500, Keerthy wrote: Introducing a driver for MADC on TWL4030 powerIC. MADC stands for monitoring ADC. This driver monitors the real time conversion of analog signals like battery temperature, battery type, battery level etc. User can also ask for the conversion on a particular channel using the sysfs nodes. Tested the conversion through sysfs nodes. Tested with DEBUG_LOCKDEP enabled. Signed-off-by: Keerthy j-keer...@ti.com --- NACK for: [...] +EXPORT_SYMBOL_GPL(twl4030_madc_conversion); [...] +EXPORT_SYMBOL_GPL(twl4030_get_madc_conversion); No symbol export from hwmon drivers. Other parts of the kernel should not depend on HWMON configuration. I would suggest to check if drivers/staging/iio would be a better fit. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] hwmon: twl4030: Driver for twl4030 madc module
On Tue, Nov 09, 2010 at 04:20:57AM -0500, Keerthy wrote: Introducing a driver for MADC on TWL4030 powerIC. MADC stands for monitoring ADC. This driver monitors the real time conversion of analog signals like battery temperature, battery type, battery level etc. User can also ask for the conversion on a particular channel using the sysfs nodes. Signed-off-by: Keerthy j-keer...@ti.com Again, I am not sure if this driver belongs into hwmon, since it is not really a hardware monitoring chip but a generic adc. We'll have to sort that out. Code looks much better than before. Still not a complete review; you should have a much closer look at error handling. I am sure I missed several cases where error returns are ignored. Thanks, Guenter --- Several people have contributed to this driver on the linux-omap list. V2: Error path correction in probe function. Return checks added. the_madc pointer could not be removed. The external drivers will not be knowing device information of the madc. Added another function which takes the channel number alone and returns the channel reading if the caller wants TWL4030_MADC_SW2 method for ADC conversion. IOCTL function is removed. Work struct is completely removed since request_threaded_irq is used. drivers/hwmon/Kconfig|6 + drivers/hwmon/Makefile |1 + drivers/hwmon/twl4030-madc.c | 573 ++ include/linux/i2c/twl4030-madc.h | 118 4 files changed, 698 insertions(+), 0 deletions(-) create mode 100644 drivers/hwmon/twl4030-madc.c create mode 100644 include/linux/i2c/twl4030-madc.h V1: http://www.mail-archive.com/linux-omap@vger.kernel.org/msg34947.html diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index a56f6ad..fef75f2 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1171,6 +1171,12 @@ config SENSORS_MC13783_ADC help Support for the A/D converter on MC13783 PMIC. +config SENSORS_TWL4030_MADC + tristate + depends on TWL4030_CORE + help + This driver provides support for TWL4030-MADC. + Besides adding a description, you might alwo want to move this to the other TI chips. if ACPI comment ACPI drivers diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 2479b3d..a54af22 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -100,6 +100,7 @@ obj-$(CONFIG_SENSORS_THMC50)+= thmc50.o obj-$(CONFIG_SENSORS_TMP102) += tmp102.o obj-$(CONFIG_SENSORS_TMP401) += tmp401.o obj-$(CONFIG_SENSORS_TMP421) += tmp421.o +obj-$(CONFIG_SENSORS_TWL4030_MADC)+= twl4030-madc.o obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o obj-$(CONFIG_SENSORS_VIA686A) += via686a.o obj-$(CONFIG_SENSORS_VT1211) += vt1211.o diff --git a/drivers/hwmon/twl4030-madc.c b/drivers/hwmon/twl4030-madc.c new file mode 100644 index 000..42f7d4a --- /dev/null +++ b/drivers/hwmon/twl4030-madc.c @@ -0,0 +1,573 @@ +/* + * + * TWL4030 MADC module driver-This driver monitors the real time + * conversion of analog signals like battery temperature, + * battery type, battery level etc. User can also ask for the conversion on a + * particular channel using the sysfs nodes. + * + * Copyright (C) 2010 Texas Instruments Inc. + * J Keerthy j-keer...@ti.com + * + * Based on twl4030-madc.c + * Copyright (C) 2008 Nokia Corporation + * Mikko Ylinen mikko.k.yli...@nokia.com + * + * Amit Kucheria amit.kuche...@canonical.com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA + * + */ + +#include linux/init.h +#include linux/interrupt.h +#include linux/kernel.h +#include linux/types.h +#include linux/module.h +#include linux/delay.h +#include linux/fs.h +#include linux/platform_device.h +#include linux/slab.h +#include linux/i2c/twl.h +#include linux/i2c/twl4030-madc.h +#include linux/sysfs.h +#include linux/hwmon.h +#include linux/hwmon-sysfs.h +#include linux/uaccess.h + +struct twl4030_madc_data { + struct device *hwmon_dev; + struct mutex lock; + struct twl4030_madc_request requests[TWL4030_MADC_NUM_METHODS]; + int imr; + int isr; +}; + +static struct twl4030_madc_data *the_madc; + +static ssize_t madc_read(struct device *dev, +
Re: [lm-sensors] [PATCH 1/2] hwmon: twl4030: Driver for twl4030 madc module
Hi, On Mon, Sep 20, 2010 at 06:34:24AM -0400, J, KEERTHY wrote: -Original Message- From: Guenter Roeck [mailto:guenter.ro...@ericsson.com] Sent: Thursday, September 16, 2010 8:48 PM To: J, KEERTHY Cc: linux-omap@vger.kernel.org; lm-sens...@lm-sensors.org; Krishnamoorthy, Balaji T Subject: Re: [lm-sensors] [PATCH 1/2] hwmon: twl4030: Driver for twl4030 madc module [...] +EXPORT_SYMBOL(twl4030_madc_conversion); + If this function is going to be called from external code, it should not really be defined here. I would suggest to move it to a global location such as mfd instead, including all related functions. The existence of this function export indicates that another non-hwmon driver depends on this one, which should not really be the case. Another reason to have a separate common driver instead, and mfd might just be the place for it. Few kernel modules need to perform ADC conversion to measure battery voltage, battery temperature, VBUS voltage via twl4030_madc_conversion. the_madc is needed as those drivers will not have this device pointer. The point I was trying to make is that this function (as well as the ioctl) should not be in this driver in the first place. hwmon is about providing hwmon information, not about providing adc readings to another driver. Or, in other words, hwmon should be a consumer of information from other drivers, not a producer of information to other drivers. There should be a higher level driver (presumably a mfd driver) to provide adc readings to all consumers, ie to all callers of twl4030_madc_conversion(). This driver can provide all data and information needed by more than one driver, and would also be the logical place for the ioctl providing raw adc readings to the user. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [lm-sensors] [PATCH 1/2] hwmon: twl4030: Driver for twl4030 madc module
On Mon, Sep 20, 2010 at 10:09:05AM -0400, Guenter Roeck wrote: Hi, On Mon, Sep 20, 2010 at 06:34:24AM -0400, J, KEERTHY wrote: -Original Message- From: Guenter Roeck [mailto:guenter.ro...@ericsson.com] Sent: Thursday, September 16, 2010 8:48 PM To: J, KEERTHY Cc: linux-omap@vger.kernel.org; lm-sens...@lm-sensors.org; Krishnamoorthy, Balaji T Subject: Re: [lm-sensors] [PATCH 1/2] hwmon: twl4030: Driver for twl4030 madc module [...] +EXPORT_SYMBOL(twl4030_madc_conversion); + If this function is going to be called from external code, it should not really be defined here. I would suggest to move it to a global location such as mfd instead, including all related functions. The existence of this function export indicates that another non-hwmon driver depends on this one, which should not really be the case. Another reason to have a separate common driver instead, and mfd might just be the place for it. Few kernel modules need to perform ADC conversion to measure battery voltage, battery temperature, VBUS voltage via twl4030_madc_conversion. the_madc is needed as those drivers will not have this device pointer. The point I was trying to make is that this function (as well as the ioctl) should not be in this driver in the first place. hwmon is about providing hwmon information, not about providing adc readings to another driver. Or, in other words, hwmon should be a consumer of information from other drivers, not a producer of information to other drivers. There should be a higher level driver (presumably a mfd driver) to provide adc readings to all consumers, ie to all callers of twl4030_madc_conversion(). This driver can provide all data and information needed by more than one driver, and would also be the logical place for the ioctl providing raw adc readings to the user. I just noticed that there already is a mfd core driver for tw4030. You might want to consider moving the functionality to read adc values into that driver. Guenter -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [lm-sensors] [PATCH 2/2] Makefile and Kconfig changes for twl4030-madc driver
On Thu, Sep 16, 2010 at 06:23:32AM -0400, Keerthy wrote: Makefile amd Kconfig Changes for twl4030-madc driver inclusion. Signed-off-by: Keerthy j-keer...@ti.com --- arch/arm/mach-omap2/Kconfig |4 drivers/hwmon/Kconfig |4 drivers/hwmon/Makefile |1 + 3 files changed, 9 insertions(+), 0 deletions(-) diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig index b48bacf..139137b 100644 --- a/arch/arm/mach-omap2/Kconfig +++ b/arch/arm/mach-omap2/Kconfig @@ -170,6 +170,8 @@ config MACH_OMAP_3430SDP depends on ARCH_OMAP3 default y select OMAP_PACKAGE_CBB + select HWMON + select TWL4030_MADC config MACH_NOKIA_N800 bool @@ -206,6 +208,8 @@ config MACH_OMAP_ZOOM3 depends on ARCH_OMAP3 default y select OMAP_PACKAGE_CBP + select HWMON + select TWL4030_MADC config MACH_CM_T35 bool CompuLab CM-T35 module diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig While it may make sense to have the above changes in a separate patch, the two files below should be part of patch #1, since they are logically part of the driver, and the driver itself won't make any sense in the code without being enabled in Kconfig/Makefile. index 4d4d09b..47d66b9 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -39,6 +39,10 @@ config HWMON_DEBUG_CHIP comment Native drivers +config TWL4030_MADC Please use SENSORS_TWL4030_MADC or SENSORS_TWL4030 to be in line with all other definitions. Also, please retain alphabetical order. + tristate + depends on HWMON TWL4030_CORE + HWMON is really redundant here and should be removed. config SENSORS_ABITUGURU tristate Abit uGuru (rev 1 2) depends on X86 EXPERIMENTAL diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index e3c2484..24b3b17 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -16,6 +16,7 @@ obj-$(CONFIG_SENSORS_W83792D) += w83792d.o obj-$(CONFIG_SENSORS_W83793) += w83793.o obj-$(CONFIG_SENSORS_W83781D)+= w83781d.o obj-$(CONFIG_SENSORS_W83791D)+= w83791d.o +obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o Again, please retain alphabetical order. There is a reason for the other sensors to be out of order (see comments in the Makefile), but presumably that does not apply this one. If it does, please add comment/reason indicating that/if this is the case. obj-$(CONFIG_SENSORS_ABITUGURU) += abituguru.o obj-$(CONFIG_SENSORS_ABITUGURU3)+= abituguru3.o -- 1.7.0.4 ___ lm-sensors mailing list lm-sens...@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [lm-sensors] [PATCH 1/2] hwmon: twl4030: Driver for twl4030 madc module
On Thu, Sep 16, 2010 at 06:23:24AM -0400, Keerthy wrote: MADC driver is added under hwmon drivers. This driver monitors the real time conversion of analog signals like battery temperature, battery type, battery level etc. User can also ask for the conversion on a particular channel using the sysfs nodes. Number of comments inline. This is not a complete review; I think there is some cleanup to be done before that is possible. Please run the driver through Lindent. While I understand that people prefer their personal touch, it makes it easier to have a single coding style in the kernel. I commented on this a couple of times below - the driver mixes generic ADC reading functions with hwmon functionality. Generic ADC reading functionality should be moved into another driver, possibly to mfd. Several people have contributed to this driver on the linux-omap list. Signed-off-by: Keerthy j-keer...@ti.com --- drivers/hwmon/twl4030-madc.c | 584 ++ include/linux/i2c/twl4030-madc.h | 118 2 files changed, 702 insertions(+), 0 deletions(-) create mode 100644 drivers/hwmon/twl4030-madc.c create mode 100644 include/linux/i2c/twl4030-madc.h diff --git a/drivers/hwmon/twl4030-madc.c b/drivers/hwmon/twl4030-madc.c new file mode 100644 index 000..b96fccd --- /dev/null +++ b/drivers/hwmon/twl4030-madc.c @@ -0,0 +1,584 @@ +/* It is customary to add a brief description as well as the author here. + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA + * 02110-1301 USA + * + */ + +#include linux/init.h +#include linux/interrupt.h +#include linux/kernel.h +#include linux/types.h +#include linux/module.h +#include linux/delay.h +#include linux/fs.h +#include linux/platform_device.h +#include linux/miscdevice.h +#include linux/slab.h +#include linux/i2c/twl.h +#include linux/i2c/twl4030-madc.h +#include linux/sysfs.h +#include linux/hwmon.h +#include linux/hwmon-sysfs.h + +#include linux/uaccess.h + + Extra blank line. Lindent will remove it. +struct twl4030_madc_data { + struct device *hwmon_dev; + struct mutexlock; + struct work_struct ws; + struct twl4030_madc_request requests[TWL4030_MADC_NUM_METHODS]; + int imr; + int isr; +}; + +static struct twl4030_madc_data *the_madc; + This variable should not exist. See other comments later. +static ssize_t madc_read(struct device *dev, + struct device_attribute *devattr, char *buf) +{ + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); + struct twl4030_madc_request req; + int status; + unsigned long val; + + req.channels = (1 attr-index); + req.method = TWL4030_MADC_SW2; + req.func_cb = NULL; + + val = twl4030_madc_conversion(req); + if (likely(val = 0)) + val = req.rbuf[attr-index]; + else + return val; + status = sprintf(buf, %lu\n, val); + return status; +} + +static +const struct twl4030_madc_conversion_method twl4030_conversion_methods[] = { + [TWL4030_MADC_RT] = { + .sel= TWL4030_MADC_RTSELECT_LSB, + .avg= TWL4030_MADC_RTAVERAGE_LSB, + .rbase = TWL4030_MADC_RTCH0_LSB, + }, + [TWL4030_MADC_SW1] = { + .sel= TWL4030_MADC_SW1SELECT_LSB, + .avg= TWL4030_MADC_SW1AVERAGE_LSB, + .rbase = TWL4030_MADC_GPCH0_LSB, + .ctrl = TWL4030_MADC_CTRL_SW1, + }, + [TWL4030_MADC_SW2] = { + .sel= TWL4030_MADC_SW2SELECT_LSB, + .avg= TWL4030_MADC_SW2AVERAGE_LSB, + .rbase = TWL4030_MADC_GPCH0_LSB, + .ctrl = TWL4030_MADC_CTRL_SW2, + }, +}; + +static int twl4030_madc_read(struct twl4030_madc_data *madc, u8 reg) +{ + int ret; + u8 val; + + ret = twl_i2c_read_u8(TWL4030_MODULE_MADC, val, reg); Structural comment. If there is a i2c master, why don't you have an I2C master driver instead of calling a platform dependent i2c function ? I see this function called from all over the place, so maybe there is a reason. Just looks odd, though. + if (ret) { + dev_dbg(madc-hwmon_dev, unable to