Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: * Eliad Peller el...@wizery.com [150309 14:03]: On Mon, Mar 9, 2015 at 9:50 PM, Arnd Bergmann a...@arndb.de wrote: On Monday 09 March 2015 17:36:42 Eliad Peller wrote: --- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts +++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts @@ -64,4 +64,13 @@ vmmc-supply = lbep5clwmc_wlen; bus-width = 4; non-removable; + + #address-cells = 1; + #size-cells = 0; + wlcore: wlcore@2 { + compatible = ti,wl1835; + reg = 2; + interrupt-parent = gpio5; + interrupts = 8 IRQ_TYPE_NONE; + }; Why IRQ_TYPE_NONE? i simply mirrored the current board file (which only sets the irq number). I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. Eliad. -- 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: ARM: OMPA4+: is it expected dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); to fail?
On Fri, Mar 06, 2015 at 11:47:48PM +0200, grygorii.stras...@linaro.org wrote: Hi Russell, On 03/05/2015 10:17 PM, Russell King - ARM Linux wrote: On Thu, Mar 05, 2015 at 08:55:07PM +0200, grygorii.stras...@linaro.org wrote: The dma_coerce_mask_and_coherent() will fail in case 'Example 3' and succeed in cases 1,2. dma-mapping.c -- __dma_supported() if (sizeof(mask) != sizeof(dma_addr_t) == true for all OMAP4+ mask (dma_addr_t)~0 == true for DMA_BIT_MASK(64) dma_to_pfn(dev, ~0) max_pfn) { == true only for Example 3 Hmm, I think this may make more sense to be max_pfn - 1 here, as that would be better suited to our intention. The result of dma_to_pfn(dev, ~0) is the maximum PFN which we could address via DMA, but we're comparing it with the maximum PFN in the system plus 1 - so we need to subtract one from it. Ok. I'll try it. Any news on this - I think it is a real off-by-one bug which we should fix in any case. Thanks. -- FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up according to speedtest.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
Re: [PATCH 1/2] pinctrl: bindings: pinctrl: Add support for TI's IODelay configuration
On Wed, Mar 4, 2015 at 1:00 AM, Nishanth Menon n...@ti.com wrote: +Configuration definition follows similar model as the pinctrl-single: +The groups of pin configuration are defined under pinctrl-single,pins + +dra7_iodelay_core { + mmc2_iodelay_3v3_conf: mmc2_iodelay_3v3_conf { + pinctrl-single,pins = + 0x18c (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A19_IN */ + 0x1a4 (A_DELAY(265) | G_DELAY(360)) /* CFG_GPMC_A20_IN */ + 0x1b0 (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A21_IN */ + 0x1bc (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A22_IN */ + 0x1c8 (A_DELAY(287) | G_DELAY(420)) /* CFG_GPMC_A23_IN */ + 0x1d4 (A_DELAY(144) | G_DELAY(240)) /* CFG_GPMC_A24_IN */ + 0x1e0 (A_DELAY(0) | G_DELAY(0)) /* CFG_GPMC_A25_IN */ + 0x1ec (A_DELAY(120) | G_DELAY(0)) /* CFG_GPMC_A26_IN */ + 0x1f8 (A_DELAY(120) | G_DELAY(180)) /* CFG_GPMC_A27_IN */ + 0x360 (A_DELAY(0) | G_DELAY(0)) /* CFG_GPMC_CS1_IN */ + ; + }; +}; But wait. The promise when we merged pinctrl-single was that this driver was to be used when the system had a one-register-per-pin layout and it was easy to do device trees based on that. We were very reluctant to accept that even though we didn't even have the generic pin control bindings in place, the argument being that the driver should know the detailed register layout, it should not be described in the device tree. We eventually caved in and accepted it as an exception. Since this pin controller is not using pinctrl-single it does not enjoy that privilege and you need to explain why this pin controller cannot use the generic bindings like so many other pin controllers have since started to do. (It is in the same SoC is not an acceptable argument.) What is wrong with this: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt We can add generic delay bindings to the list, it even seems like a good idea to do so, as it is likely something that will come up in other hardware and will be needed for ACPI etc going forward. Yours, Linus Walleij -- 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] pinctrl: Introduce TI IOdelay configuration driver
On Wed, Mar 4, 2015 at 1:00 AM, Nishanth Menon n...@ti.com wrote: SoC family such as DRA7 family of processors have, in addition to the regular muxing of pins (as done by pinctrl-single), an additional hardware module called IODelay which is also expected to be configured. This IODelay module has it's own register space that is independent of the control module. It is advocated strongly in TI's official documentation considering the existing design of the DRA7 family of processors during mux or IODelay reconfiguration, there is a potential for a significant glitch which may cause functional impairment to certain hardware. It is hence recommended to do as little of muxing as absolutely necessary without I/O isolation (which can only be done in initial stages of bootloader). NOTE: with the system wide I/O isolation scheme present in DRA7 SoC family, it is not reasonable to do stop all I/O operations for every such pad configuration scheme. So, we will let it glitch when used in this mode. Even with the above limitation, certain functionality such as MMC has mandatory need for IODelay reconfiguration requirements, depending on speed of transfer. In these cases, with careful examination of usecase involved, the expected glitch can be controlled such that it does not impact functionality. In short, IODelay module support as a padconf driver being introduced here is not expected to do SoC wide I/O Isolation and is meant for a limited subset of IODelay configuration requirements that need to be dynamic and whose glitchy behavior will not cause functionality failure for that interface. Signed-off-by: Nishanth Menon n...@ti.com Signed-off-by: Lokesh Vutla lokeshvu...@ti.com Pending the conclusion of my remark on the bindings that this should use the generic pin config bindings... here are some normal review comments. +config PINCTRL_TI_IODELAY + bool TI IODelay Module pinconf driver + depends on OF + select PINCONF + select GENERIC_PINCONF + select REGMAP_MMIO + help + Say Y here to support Texas Instruments' IODelay pinconf driver. + IODelay module is used for the DRA7 SoC family. This driver is in + addition to PINCTRL_SINGLE which controls the mux. If the driver is in addition to PINCTRL_SINGLE, then it should depend on it. +/* Should I change this? Abuse? */ +#define IODELAY_MUX_PINS_NAME pinctrl-single,pins Yeah this is abuse and is why you should use the generic bindings with just pins instead, and use the generic pinconf parameters to set them up. + * struct ti_iodelay_conf_vals - Description of each configuration parameters. + * @offset:Configuration register offset + * @a_delay: Agnostic Delay (in ps) + * @g_delay: Gnostic Delay (in ps) + */ +struct ti_iodelay_conf_vals { + u16 offset; + u16 a_delay; + u16 g_delay; +}; So I don't think it's a good idea to get the register offset from the device tree. The driver should know these. +/** + * struct ti_iodelay_reg_values - Computed io_reg configuration values (see TRM) + * @coarse_ref_count: Coarse reference count + * @coarse_delay_count:Coarse delay count + * @fine_ref_count:Fine reference count + * @fine_delay_count: Fine Delay count + * @ref_clk_period:Reference Clock period + * @cdpe: Coarse delay parameter + * @fdpe: Fine delay parameter + */ +struct ti_iodelay_reg_values { + u16 coarse_ref_count; If you're doing reference counting, use kref. linux/kref.h + u16 coarse_delay_count; + + u16 fine_ref_count; Dito. + u16 fine_delay_count; + + u16 ref_clk_period; + + u32 cdpe; + u32 fdpe; +}; +/** + * struct ti_iodelay_pin_name - name of the pins + * @name: name + */ +struct ti_iodelay_pin_name { + char name[IODELAY_REG_NAME_LEN]; +}; What is this? The pin control subsystem already has a struct for naming pins. struct pinctrl_pin_desc. Are you reimplementing core functionality? The generic pin config reference pins by name, and since you obviously like to encode these into you data you can just as well use that method for this too. Again I think this is a side effect from using the pinctrl-single concepts, if you think of this as some other pinctrl driver I think this will fall out nicely. +/** + * struct ti_iodelay_device - Represents information for a IOdelay instance + * @dev: device pointer + * @reg_base: Remapped virtual address + * @regmap: Regmap for this IOdelay instance + * @pctl: Pinctrl device + * @desc: pinctrl descriptor for pctl + * @pa: pinctrl pin wise description + * @names: names of the pins + * @groups: list of pinconf groups for iodelay instance + * @ngroups: number of groups in the list + * @mutex: mutex to protect group list modification + * @reg_data: Register definition data for the IODelay instance + * @reg_init_conf_values: Initial
Re: [PATCH v2 6/7] drm/tilcdc: Force building of DRM_TILCDC_SLAVE_COMPAT
On 06/03/15 17:36, Jyri Sarha wrote: If I read Documentation/kbuild/makefiles.txt section 3.6 right, this patch should not be needed. However, without this patch the objects needed for DRM_TILCDC_SLAVE_COMPAT are not linked, if DRM_TILCDC is built as module. Signed-off-by: Jyri Sarha jsa...@ti.com --- drivers/gpu/drm/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 2c239b9..ad7f10f 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -60,6 +60,7 @@ obj-$(CONFIG_DRM_RCAR_DU) += rcar-du/ obj-$(CONFIG_DRM_SHMOBILE) +=shmobile/ obj-$(CONFIG_DRM_OMAP) += omapdrm/ obj-$(CONFIG_DRM_TILCDC) += tilcdc/ +obj-$(CONFIG_DRM_TILCDC_SLAVE_COMPAT) += tilcdc/ obj-$(CONFIG_DRM_QXL) += qxl/ obj-$(CONFIG_DRM_BOCHS) += bochs/ obj-$(CONFIG_DRM_MSM) += msm/ I don't think it's a good idea to enter the directory twice. Maybe it works fine, I don't know, but I'd suggest just using obj-y above. Tomi signature.asc Description: OpenPGP digital signature
Re: [PATCH v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
* Arnd Bergmann a...@arndb.de [150310 07:11]: On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote: On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: * Eliad Peller el...@wizery.com [150309 14:03]: On Mon, Mar 9, 2015 at 9:50 PM, Arnd Bergmann a...@arndb.de wrote: On Monday 09 March 2015 17:36:42 Eliad Peller wrote: --- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts +++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts @@ -64,4 +64,13 @@ vmmc-supply = lbep5clwmc_wlen; bus-width = 4; non-removable; + + #address-cells = 1; + #size-cells = 0; + wlcore: wlcore@2 { + compatible = ti,wl1835; + reg = 2; + interrupt-parent = gpio5; + interrupts = 8 IRQ_TYPE_NONE; + }; Why IRQ_TYPE_NONE? i simply mirrored the current board file (which only sets the irq number). I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. Right, that was my idea. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it. Unfortunately, there is still one machine that uses a board file in combination with this wl12xx: da850-evm calls wl12xx_set_platform_data() from a legacy board file without DT. We have DT support for this file as well, but if I remember correctly, it is somewhat incomplete and we don't want to remove the file yet. Adding Sekhar and Kevin to Cc here for confirmation. Oops I forgot about the omap3-sbc-t3730, so yes we have to keep the platform data a little bit longer. But nothing stopping us moving all the other ones to use a proper device tree based configuration. Regards, Tony -- 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] phy: Add a driver for dm816x USB PHY
On Mon, Mar 9, 2015 at 4:41 PM, Tony Lindgren t...@atomide.com wrote: * Bin Liu binml...@gmail.com [150309 14:35]: On Mon, Mar 9, 2015 at 4:26 PM, Tony Lindgren t...@atomide.com wrote: * Felipe Balbi ba...@ti.com [150309 14:21]: On Mon, Mar 09, 2015 at 04:17:29PM -0500, Bin Liu wrote: On Mon, Mar 9, 2015 at 4:13 PM, Felipe Balbi ba...@ti.com wrote: On Mon, Mar 09, 2015 at 04:11:28PM -0500, Bin Liu wrote: Hi, On Mon, Mar 9, 2015 at 3:51 PM, Tony Lindgren t...@atomide.com wrote: Add a minimal driver for dm816x USB. Otherwise we can just use the existing musb_am335x and musb_dsps on dm816x. dm816x has the almost identical usbss as that in am335x, we should be able to adopt musb_am335x and musb_dsps for dm816x, and dm814x too? Tony's using the same musb glue layers, this is just a phy driver, right ? Can the current am335x phy driver be adopted too? I remember it is under drivers/usb/phy/. Tony will be best to answer, but according to our IRC discussions, it's too different. Tony ? Well we should check again between dm814x and am335x documentation against the $subject driver as the dm816x docs were buggy and I may have gotten a wrong idea initially. Will it make our life a little easier by comparing the usb drivers for dm81xx and am335x in previous TI kernel releases? I cam dig out the source code if that helps. Yeah that probably helps if you've dealt with dm814x earlier :) Well, don't expect much from me about dm814x ;), I did not read its trm nor errata, but only booted the board a few times trying to compare the MUSB behaviours while debugging issues on am335x. Ok, the previous TI released 3.2 kernel for am335x is at [1], and 2.6.37 kernel dm81xx is at [2]. Regards, -Bin. [1] http://arago-project.org/git/projects/?p=linux-am33x.git;a=shortlog;h=refs/heads/v3.2-staging [2] http://arago-project.org/git/projects/?p=linux-omap3.git;a=shortlog;h=refs/heads/TI81XXPSP_04.04.00.01 Just don't look at the dm816x trm, only the errata pdf works for dm816x phy. Note that we still are missing basic support for dm814x in mainline, I'm planning to tackle that at some point but I don't know when I'm going to get to it.. Regards, Tony -- 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 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tue, Mar 10, 2015 at 4:11 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote: On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. Right, that was my idea. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it. i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution) the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well. Unfortunately, there is still one machine that uses a board file in combination with this wl12xx: da850-evm calls wl12xx_set_platform_data() from a legacy board file without DT. ditto. Eliad. -- 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 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tuesday 10 March 2015 07:41 PM, Arnd Bergmann wrote: On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote: On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: * Eliad Peller el...@wizery.com [150309 14:03]: On Mon, Mar 9, 2015 at 9:50 PM, Arnd Bergmann a...@arndb.de wrote: On Monday 09 March 2015 17:36:42 Eliad Peller wrote: --- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts +++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts @@ -64,4 +64,13 @@ vmmc-supply = lbep5clwmc_wlen; bus-width = 4; non-removable; + + #address-cells = 1; + #size-cells = 0; + wlcore: wlcore@2 { + compatible = ti,wl1835; + reg = 2; + interrupt-parent = gpio5; + interrupts = 8 IRQ_TYPE_NONE; + }; Why IRQ_TYPE_NONE? i simply mirrored the current board file (which only sets the irq number). I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. Right, that was my idea. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it. Unfortunately, there is still one machine that uses a board file in combination with this wl12xx: da850-evm calls wl12xx_set_platform_data() from a legacy board file without DT. We have DT support for this file as well, but if I remember correctly, it is somewhat incomplete and we don't want to remove the file yet. Adding Sekhar and Kevin to Cc here for confirmation. Thats true, we do not want to drop legacy booting for DA850 EVM yet. But if its coming in the way of code consolidation, I do not mind dropping WLAN support for that board. Anyone who really needs it can add support to the DA850 EVM DT file. Thanks, Sekhar -- 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 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote: On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: * Eliad Peller el...@wizery.com [150309 14:03]: On Mon, Mar 9, 2015 at 9:50 PM, Arnd Bergmann a...@arndb.de wrote: On Monday 09 March 2015 17:36:42 Eliad Peller wrote: --- a/arch/arm/boot/dts/omap3-igep0030-rev-g.dts +++ b/arch/arm/boot/dts/omap3-igep0030-rev-g.dts @@ -64,4 +64,13 @@ vmmc-supply = lbep5clwmc_wlen; bus-width = 4; non-removable; + + #address-cells = 1; + #size-cells = 0; + wlcore: wlcore@2 { + compatible = ti,wl1835; + reg = 2; + interrupt-parent = gpio5; + interrupts = 8 IRQ_TYPE_NONE; + }; Why IRQ_TYPE_NONE? i simply mirrored the current board file (which only sets the irq number). I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. Right, that was my idea. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it. Unfortunately, there is still one machine that uses a board file in combination with this wl12xx: da850-evm calls wl12xx_set_platform_data() from a legacy board file without DT. We have DT support for this file as well, but if I remember correctly, it is somewhat incomplete and we don't want to remove the file yet. Adding Sekhar and Kevin to Cc here for confirmation. Arnd -- 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] i2c: omap: implement bus recovery
On Mon, Mar 09, 2015 at 11:39:17AM -0500, Felipe Balbi wrote: On Thu, Feb 19, 2015 at 12:06:49PM -0600, Felipe Balbi wrote: If either SCL or SDA are stuck low, we need to recover the bus using the procedure described on section 3.1.16 of the I2C specification. Note that we're trying to implement the procedure exactly as described by that section. First we check which line is stuck low, then implement one or the other procedure. If SDA recovery procedure fails, we reset our IP in an attempt to make it work. Signed-off-by: Felipe Balbi ba...@ti.com --- Tested with AM437x IDK, AM437x SK, BeagleBoneBlack and Beagle X15 with 1000 iterations of i2cdetect on all available buses. That said, I couldn't get any device to hold the bus busy so I could see this working. If anybody has any good way of forcing a condition so that we need bus recovery, I'd be glad to look at. ping any comments here ?? Anybody at all -- balbi signature.asc Description: Digital signature
[PATCH 7/8] s390: time: Provide read_boot_clock64() and read_persistent_clock64()
From: Xunlei Pang pang.xun...@linaro.org As part of addressing y2038 problem for in-kernel uses, this patch converts read_boot_clock() to read_boot_clock64() and read_persistent_clock() to read_persistent_clock64() using timespec64. Since S390 is a 64bit architecture, also rename some timespec to timespec64 in time.c and the related references. Signed-off-by: Xunlei Pang pang.xun...@linaro.org --- arch/s390/include/asm/timex.h | 4 ++-- arch/s390/kernel/debug.c | 4 ++-- arch/s390/kernel/time.c | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/arch/s390/include/asm/timex.h b/arch/s390/include/asm/timex.h index 98eb2a5..8b08be3 100644 --- a/arch/s390/include/asm/timex.h +++ b/arch/s390/include/asm/timex.h @@ -108,10 +108,10 @@ int get_sync_clock(unsigned long long *clock); void init_cpu_timer(void); unsigned long long monotonic_clock(void); -void tod_to_timeval(__u64, struct timespec *); +void tod_to_timeval(__u64, struct timespec64 *); static inline -void stck_to_timespec(unsigned long long stck, struct timespec *ts) +void stck_to_timespec64(unsigned long long stck, struct timespec64 *ts) { tod_to_timeval(stck - TOD_UNIX_EPOCH, ts); } diff --git a/arch/s390/kernel/debug.c b/arch/s390/kernel/debug.c index c1f21ac..20ff016 100644 --- a/arch/s390/kernel/debug.c +++ b/arch/s390/kernel/debug.c @@ -1457,14 +1457,14 @@ int debug_dflt_header_fn(debug_info_t * id, struct debug_view *view, int area, debug_entry_t * entry, char *out_buf) { - struct timespec time_spec; + struct timespec64 time_spec; char *except_str; unsigned long caller; int rc = 0; unsigned int level; level = entry-id.fields.level; - stck_to_timespec(entry-id.stck, time_spec); + stck_to_timespec64(entry-id.stck, time_spec); if (entry-id.fields.exception) except_str = *; diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c index 20660dd..e4df144 100644 --- a/arch/s390/kernel/time.c +++ b/arch/s390/kernel/time.c @@ -76,7 +76,7 @@ unsigned long long monotonic_clock(void) } EXPORT_SYMBOL(monotonic_clock); -void tod_to_timeval(__u64 todval, struct timespec *xt) +void tod_to_timeval(__u64 todval, struct timespec64 *xt) { unsigned long long sec; @@ -181,12 +181,12 @@ static void timing_alert_interrupt(struct ext_code ext_code, static void etr_reset(void); static void stp_reset(void); -void read_persistent_clock(struct timespec *ts) +void read_persistent_clock64(struct timespec64 *ts) { tod_to_timeval(get_tod_clock() - TOD_UNIX_EPOCH, ts); } -void read_boot_clock(struct timespec *ts) +void read_boot_clock64(struct timespec64 *ts) { tod_to_timeval(sched_clock_base_cc - TOD_UNIX_EPOCH, ts); } -- 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 6/8] ARM: time: Provide read_boot_clock64() and read_persistent_clock64()
From: Xunlei Pang pang.xun...@linaro.org As part of addressing y2038 problem for in-kernel uses, this patch converts read_boot_clock() to read_boot_clock64() and read_persistent_clock() to read_persistent_clock64() using timespec64 by converting clock_access_fn to use timespec64. Signed-off-by: Xunlei Pang pang.xun...@linaro.org --- arch/arm/include/asm/mach/time.h| 3 +-- arch/arm/kernel/time.c | 6 +++--- arch/arm/plat-omap/counter_32k.c| 10 +- drivers/clocksource/tegra20_timer.c | 10 +- 4 files changed, 6 insertions(+), 23 deletions(-) diff --git a/arch/arm/include/asm/mach/time.h b/arch/arm/include/asm/mach/time.h index 90c12e1..0f79e4d 100644 --- a/arch/arm/include/asm/mach/time.h +++ b/arch/arm/include/asm/mach/time.h @@ -12,8 +12,7 @@ extern void timer_tick(void); -struct timespec; -typedef void (*clock_access_fn)(struct timespec *); +typedef void (*clock_access_fn)(struct timespec64 *); extern int register_persistent_clock(clock_access_fn read_boot, clock_access_fn read_persistent); diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c index 0cc7e58..a66e37e 100644 --- a/arch/arm/kernel/time.c +++ b/arch/arm/kernel/time.c @@ -76,7 +76,7 @@ void timer_tick(void) } #endif -static void dummy_clock_access(struct timespec *ts) +static void dummy_clock_access(struct timespec64 *ts) { ts-tv_sec = 0; ts-tv_nsec = 0; @@ -85,12 +85,12 @@ static void dummy_clock_access(struct timespec *ts) static clock_access_fn __read_persistent_clock = dummy_clock_access; static clock_access_fn __read_boot_clock = dummy_clock_access;; -void read_persistent_clock(struct timespec *ts) +void read_persistent_clock64(struct timespec64 *ts) { __read_persistent_clock(ts); } -void read_boot_clock(struct timespec *ts) +void read_boot_clock64(struct timespec64 *ts) { __read_boot_clock(ts); } diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c index d422e36..aee17ff 100644 --- a/arch/arm/plat-omap/counter_32k.c +++ b/arch/arm/plat-omap/counter_32k.c @@ -70,14 +70,6 @@ static void omap_read_persistent_clock64(struct timespec64 *ts) *ts = persistent_ts; } -static void omap_read_persistent_clock(struct timespec *ts) -{ - struct timespec64 ts64; - - omap_read_persistent_clock64(ts64); - *ts = timespec64_to_timespec(ts64); -} - /** * omap_init_clocksource_32k - setup and register counter 32k as a * kernel clocksource @@ -118,7 +110,7 @@ int __init omap_init_clocksource_32k(void __iomem *vbase) } sched_clock_register(omap_32k_read_sched_clock, 32, 32768); - register_persistent_clock(NULL, omap_read_persistent_clock); + register_persistent_clock(NULL, omap_read_persistent_clock64); pr_info(OMAP clocksource: 32k_counter at 32768 Hz\n); return 0; diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c index 7fc7fb9..8547742 100644 --- a/drivers/clocksource/tegra20_timer.c +++ b/drivers/clocksource/tegra20_timer.c @@ -141,14 +141,6 @@ static void tegra_read_persistent_clock64(struct timespec64 *ts) *ts = persistent_ts; } -static void tegra_read_persistent_clock(struct timespec *ts) -{ - struct timespec ts64; - - tegra_read_persistent_clock64(ts64); - *ts = timespec64_to_timespec(ts64); -} - static unsigned long tegra_delay_timer_read_counter_long(void) { return readl(timer_reg_base + TIMERUS_CNTR_1US); @@ -259,7 +251,7 @@ static void __init tegra20_init_rtc(struct device_node *np) else clk_prepare_enable(clk); - register_persistent_clock(NULL, tegra_read_persistent_clock); + register_persistent_clock(NULL, tegra_read_persistent_clock64); } CLOCKSOURCE_OF_DECLARE(tegra20_rtc, nvidia,tegra20-rtc, tegra20_init_rtc); -- 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 5/8] ARM: tegra: clock: Provide y2038-safe tegra_read_persistent_clock() replacement
From: Xunlei Pang pang.xun...@linaro.org As part of addressing y2038 problem for in-kernel uses, this patch adds the y2038-safe tegra_read_persistent_clock64() using timespec64. Because we rely on some subsequent changes to convert arm multiarch support, tegra_read_persistent_clock() will be removed then. Signed-off-by: Xunlei Pang pang.xun...@linaro.org --- drivers/clocksource/tegra20_timer.c | 21 ++--- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/clocksource/tegra20_timer.c b/drivers/clocksource/tegra20_timer.c index d2616ef..7fc7fb9 100644 --- a/drivers/clocksource/tegra20_timer.c +++ b/drivers/clocksource/tegra20_timer.c @@ -51,7 +51,7 @@ static void __iomem *timer_reg_base; static void __iomem *rtc_base; -static struct timespec persistent_ts; +static struct timespec64 persistent_ts; static u64 persistent_ms, last_persistent_ms; static struct delay_timer tegra_delay_timer; @@ -120,26 +120,33 @@ static u64 tegra_rtc_read_ms(void) } /* - * tegra_read_persistent_clock - Return time from a persistent clock. + * tegra_read_persistent_clock64 - Return time from a persistent clock. * * Reads the time from a source which isn't disabled during PM, the * 32k sync timer. Convert the cycles elapsed since last read into - * nsecs and adds to a monotonically increasing timespec. + * nsecs and adds to a monotonically increasing timespec64. * Care must be taken that this funciton is not called while the * tegra_rtc driver could be executing to avoid race conditions * on the RTC shadow register */ -static void tegra_read_persistent_clock(struct timespec *ts) +static void tegra_read_persistent_clock64(struct timespec64 *ts) { u64 delta; - struct timespec *tsp = persistent_ts; last_persistent_ms = persistent_ms; persistent_ms = tegra_rtc_read_ms(); delta = persistent_ms - last_persistent_ms; - timespec_add_ns(tsp, delta * NSEC_PER_MSEC); - *ts = *tsp; + timespec64_add_ns(persistent_ts, delta * NSEC_PER_MSEC); + *ts = persistent_ts; +} + +static void tegra_read_persistent_clock(struct timespec *ts) +{ + struct timespec ts64; + + tegra_read_persistent_clock64(ts64); + *ts = timespec64_to_timespec(ts64); } static unsigned long tegra_delay_timer_read_counter_long(void) -- 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 0/8] Add y2038 safe replacements for read_boot_clock(), read_persistent_clock() and update_persistent_clock()
From: Xunlei Pang pang.xun...@linaro.org read_boot_clock(), read_persistent_clock() and update_persistent_clock() all use timespec which may have y2038 problem, thus we are planning on converting all of them to use timespec64. The approach we're using is: 1) First of all, add the __weak implementaion of xxx_clock64() which uses xxx_clock() and timespec64. Let's take read_persistent_clock() as an example. We can create its y2038-safe version below: void __weak read_persistent_clock64(struct timespec64 *ts64) { struct timespec ts; read_persistent_clock(ts); *ts64 = timespec_to_timespec64(ts); } Then, replace all the call sites of xxx_clock() with xxx_clock64() except the one used by xxx_clock64(). 2) Convert every architecture specific xxx_clock() to xxx_clock64() one by one. At this point, we can convert the three functions at a time if needed, because most time they're correlated. 3) Remove xxx_clock() after all its architecture specific implementations have been converted to use corresponding y2038 safe xxx_clock64(). This patchset firstly finished the step1 for all the three functions, then focused on read_boot_clock() which is simple to go on with step2 and step3 as a whole show. As for read_persistent_clock() and update_persistent_clock() which are more complex, requiring many efforts to convert every architecture specific implementation gradually. The approach used here is Suggested-by: Arnd Bergmann a...@arndb.de Xunlei Pang (8): time: Add y2038 safe read_boot_clock64() time: Add y2038 safe read_persistent_clock64() time: Add y2038 safe update_persistent_clock64() ARM: OMAP: 32k counter: Provide y2038-safe omap_read_persistent_clock() replacement ARM: tegra: clock: Provide y2038-safe tegra_read_persistent_clock() replacement ARM: time: Provide read_boot_clock64() and read_persistent_clock64() s390: time: Provide read_boot_clock64() and read_persistent_clock64() time: Remove read_boot_clock() arch/arm/include/asm/mach/time.h| 3 +-- arch/arm/kernel/time.c | 6 +++--- arch/arm/plat-omap/counter_32k.c| 18 ++ arch/mips/lasat/sysctl.c| 4 ++-- arch/s390/include/asm/timex.h | 4 ++-- arch/s390/kernel/debug.c| 4 ++-- arch/s390/kernel/time.c | 6 +++--- drivers/clocksource/tegra20_timer.c | 15 +++ drivers/rtc/systohc.c | 2 +- include/linux/timekeeping.h | 4 +++- kernel/time/ntp.c | 13 - kernel/time/timekeeping.c | 31 --- 12 files changed, 58 insertions(+), 52 deletions(-) -- 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 8/8] time: Remove read_boot_clock()
From: Xunlei Pang pang.xun...@linaro.org Now we have all the read_boot_clock64() for all implementations, it's time to remove read_boot_clock() completely from the kernel. Signed-off-by: Xunlei Pang pang.xun...@linaro.org --- read_persistent_clock() and update_persistent_clock() are way more complex, so we will deal with them gradually in extra patchsets. include/linux/timekeeping.h | 1 - kernel/time/timekeeping.c | 14 +++--- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h index a7fa96b..72631e8 100644 --- a/include/linux/timekeeping.h +++ b/include/linux/timekeeping.h @@ -263,7 +263,6 @@ static inline bool has_persistent_clock(void) extern void read_persistent_clock(struct timespec *ts); extern void read_persistent_clock64(struct timespec64 *ts); -extern void read_boot_clock(struct timespec *ts); extern void read_boot_clock64(struct timespec64 *ts); extern int update_persistent_clock(struct timespec now); extern int update_persistent_clock64(struct timespec64 now); diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 0e5a696..d0ca908 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -1066,28 +1066,20 @@ void __weak read_persistent_clock64(struct timespec64 *ts64) } /** - * read_boot_clock - Return time of the system start. + * read_boot_clock64 - Return time of the system start. * * Weak dummy function for arches that do not yet support it. * Function to read the exact time the system has been started. - * Returns a timespec with tv_sec=0 and tv_nsec=0 if unsupported. + * Returns a timespec64 with tv_sec=0 and tv_nsec=0 if unsupported. * * XXX - Do be sure to remove it once all arches implement it. */ -void __weak read_boot_clock(struct timespec *ts) +void __weak read_boot_clock64(struct timespec64 *ts) { ts-tv_sec = 0; ts-tv_nsec = 0; } -void __weak read_boot_clock64(struct timespec64 *ts64) -{ - struct timespec ts; - - read_boot_clock(ts); - *ts64 = timespec_to_timespec64(ts); -} - /* * timekeeping_init - Initializes the clocksource and common timekeeping values */ -- 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] ARM: OMAP2+: omap_hwmod: Introduce ti,no-init dt property
Grygorii, On 03/10/2015 12:36 PM, Grygorii Strashko wrote: Hi Dave, On 03/06/2015 07:45 PM, Tony Lindgren wrote: * Dave Gerlach d-gerl...@ti.com [150306 09:28]: On 03/05/2015 06:41 PM, Tony Lindgren wrote: * Tony Lindgren t...@atomide.com [150305 12:24]: * Dave Gerlach d-gerl...@ti.com [150305 11:53]: On 03/05/2015 12:49 PM, Tony Lindgren wrote: * Paul Walmsley p...@pwsan.com [150305 10:16]: On Thu, 5 Mar 2015, Dave Gerlach wrote: Introduce a dt property, ti,no-init, that prevents hwmod initialization. Even if a dt node is marked as disabled, hwmod still at least enables the hwmod and programs the sysconfig before attempting to idle it at boot. If an IP has been disabled by the hardware configuration on a platform, this will cause a hang due to writing to inactive registers. This property prevents that from happening by marking the hwmod as _HWMOD_STATE_DISABLED during init. I'm kind of wondering if hwmod should even touch a device if it's marked as disabled in the DT. Tony, what do you think? Well nothing happens if a device is status = disabled. No dev entry gets created for it at all and hwmod won't have any data for the device populated. The only way hwmod code could see that device if the device gets it's data from the legacy omap_hwmod_*_data.c instead of DT. We still need this for the sysconfig programming, correct? hwmod programs that regardless of dt status and then idles the IP, Well hwmod does not even know about the IP IO addresses if it's marked with status = disabled.. Which IP are you having problems with? which is why I needed the ti,no-init for the epos evm. It isn't just a matter of we shouldnt write to it because we don't want to use it; we can't write to it because the module is held off so it causes an external abort if we do. Well hard to say not knowing which module this is.. Pretty much all the modules have drivers and the driver just does pm_runtime_get() on it? Heh OK this thread is about the RTC driver, so I assume that's the problem :) So if you set the rtc to status = disabled how can the hwmod code do anything as AFAIK it won't even get the rtc IO address? Or am I missing something here? Perhaps I am mistaken, but from what I understand, all hwmods have _init and _setup called on them, and all hwmods read the IO address regardless of DT status at this point with _init_mpu_rt_base. In _setup, _setup_reset gets called which calls _enable for the hwmod, and this calls both _enable_sysc and _update_sysc_cache which touch the sysconfig register of the IP. Oh OK, I think you're right. I was thinking of omap_device_build_from_dt(), sorry. Looks like the hwmod IO address data does get populated even for status = disabled although the dev entry won't get created and omap_device_build_from_dt() never gets called. Normally this is fine regardless of whether or not we are using an IP because the module will wake up for a moment, have it's sysc programmed, and then be put back to sleep later, potentially never to be woken again if we bind no driver for it, which is fine for 99% of cases. In the case of am43x epos evm, you can take the same piece of silicon that will boot happily on the gp evm with the rtc hwmod in place and it will hang during boot on the epos evm because the board uses a pin to hold the RTC IP in reset. There is no way to detect this in software, the module can be turned on as normal using the clk_ctrl, but if you touch any of the IP registers you get an abort. OK sounds like some dts property is needed to signal this. As I understand, there is device RTC and this device has status 'disabled', so there is reasonable question why do we need to touch it at all? The HWMOD is some kind of SoC description, which was needed before DT. Now with DT in place it seems unreasonable to touch any IP block which: - are not defined in DT - has status 'disabled'. in above cases the u-boot has to dial with IP block if it's invisible for Kernel. The HWMOD framework intended to reset and put in some predefined states IPs which are visible for the Kernel, so then proper driver can start working with IP or IP will be kept in some low-power state if there is no driver. Currently we still need this hwmod behavior as not all IPs are in an ideal state for PM by default. Not sure if you saw my last email in this thread but I gave an example for USB hwmod: If no USB driver is bound even just letting omap_device idle it using the USB_ CLKCTRL in the PRCM will not work properly, because the USB on am33xx expects it's SYSCONFIG STANDBY_MODE to be software supervised, the default state (SMART_STANDBY) does not let the IP go into standby so the IP gets stuck in a partially off state but does not actually shut down as expected. So we need to be able to touch the dt nodes even if they are marked as disabled because we need to get the IO address to have access to the SYSCONFIG register. I
Re: [PATCH 1/2] pinctrl: bindings: pinctrl: Add support for TI's IODelay configuration
On 03/10/2015 10:33 AM, Tony Lindgren wrote: * Linus Walleij linus.wall...@linaro.org [150310 03:39]: On Wed, Mar 4, 2015 at 1:00 AM, Nishanth Menon n...@ti.com wrote: +Configuration definition follows similar model as the pinctrl-single: +The groups of pin configuration are defined under pinctrl-single,pins + +dra7_iodelay_core { + mmc2_iodelay_3v3_conf: mmc2_iodelay_3v3_conf { + pinctrl-single,pins = + 0x18c (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A19_IN */ + 0x1a4 (A_DELAY(265) | G_DELAY(360)) /* CFG_GPMC_A20_IN */ + 0x1b0 (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A21_IN */ + 0x1bc (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A22_IN */ + 0x1c8 (A_DELAY(287) | G_DELAY(420)) /* CFG_GPMC_A23_IN */ + 0x1d4 (A_DELAY(144) | G_DELAY(240)) /* CFG_GPMC_A24_IN */ + 0x1e0 (A_DELAY(0) | G_DELAY(0)) /* CFG_GPMC_A25_IN */ + 0x1ec (A_DELAY(120) | G_DELAY(0)) /* CFG_GPMC_A26_IN */ + 0x1f8 (A_DELAY(120) | G_DELAY(180)) /* CFG_GPMC_A27_IN */ + 0x360 (A_DELAY(0) | G_DELAY(0)) /* CFG_GPMC_CS1_IN */ + ; + }; +}; But wait. The promise when we merged pinctrl-single was that this driver was to be used when the system had a one-register-per-pin layout and it was easy to do device trees based on that. We were very reluctant to accept that even though we didn't even have the generic pin control bindings in place, the argument being that the driver should know the detailed register layout, it should not be described in the device tree. We eventually caved in and accepted it as an exception. Hey let's get few things straight here. There's nothing stopping the driver from knowing a detailed register layout with the pinctrl-single binding. This can be very easily done based on the compatible flag and match data as needed and check the values provided. And let's also recap the reasons for the pinctrl-single binding. The the main reason for the pinctrl-single binding is to avoid mapping individual register bits to device tree compatible string properties. Imagine doing that for hundreds of similar registers. Believe me, I tried using device tree properties initially and it just sucked big time. For larger amounts of dts data, it's best to stick to numeric values and phandles in the device tree data and rely on the preprocessor for getting the values right. Finally, we don't want to build databases into the kernel drivers for every SoC. We certainly have plenty fo those already. Since this pin controller is not using pinctrl-single it does not enjoy that privilege and you need to explain why this pin controller cannot use the generic bindings like so many other pin controllers have since started to do. (It is in the same SoC is not an acceptable argument.) What is wrong with this: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt Nishanth, care to explain your reasons for using pinctrl-single binding here vs the generic binding? Is the amount of string parsing with the data an issue here? Wrong option chosen, I suppose :( - alright, lets discuss the alternative. We can add generic delay bindings to the list, it even seems like a good idea to do so, as it is likely something that will come up in other hardware and will be needed for ACPI etc going forward. We certainly need to make setting delays (with values) generic, no doubt about that. If the large amount of data is not an issue here, we could maybe set each iodelay register as a separate device? Then the binding could be just along the interrupts-extended type binding: foo = bar 0x18c A_DELAY(0) G_DELAY(120); Where the 0x18c is the instance offset of the register within the ti,dra7-iodelay compatible controller. if mmc2_pins_default point at pins for mmc pin configuration for control_core (pinctrl-single), are you proposing the following? mmc2_pins_default: mmc2_pins_default { pinctrl-single,pins = 0x9c (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a23.mmc2_clk */ 0xb0 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_cs1.mmc2_cmd */ 0xa0 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a24.mmc2_dat0 */ 0xa4 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a25.mmc2_dat1 */ 0xa8 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a26.mmc2_dat2 */ 0xac (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a27.mmc2_dat3 */ 0x8c (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a19.mmc2_dat4 */ 0x90 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a20.mmc2_dat5 */
Re: [PATCH 1/2] pinctrl: bindings: pinctrl: Add support for TI's IODelay configuration
* Nishanth Menon n...@ti.com [150310 10:25]: On 03/10/2015 10:33 AM, Tony Lindgren wrote: * Linus Walleij linus.wall...@linaro.org [150310 03:39]: On Wed, Mar 4, 2015 at 1:00 AM, Nishanth Menon n...@ti.com wrote: +Configuration definition follows similar model as the pinctrl-single: +The groups of pin configuration are defined under pinctrl-single,pins + +dra7_iodelay_core { + mmc2_iodelay_3v3_conf: mmc2_iodelay_3v3_conf { + pinctrl-single,pins = + 0x18c (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A19_IN */ + 0x1a4 (A_DELAY(265) | G_DELAY(360)) /* CFG_GPMC_A20_IN */ + 0x1b0 (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A21_IN */ + 0x1bc (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A22_IN */ + 0x1c8 (A_DELAY(287) | G_DELAY(420)) /* CFG_GPMC_A23_IN */ + 0x1d4 (A_DELAY(144) | G_DELAY(240)) /* CFG_GPMC_A24_IN */ + 0x1e0 (A_DELAY(0) | G_DELAY(0)) /* CFG_GPMC_A25_IN */ + 0x1ec (A_DELAY(120) | G_DELAY(0)) /* CFG_GPMC_A26_IN */ + 0x1f8 (A_DELAY(120) | G_DELAY(180)) /* CFG_GPMC_A27_IN */ + 0x360 (A_DELAY(0) | G_DELAY(0)) /* CFG_GPMC_CS1_IN */ + ; + }; +}; But wait. The promise when we merged pinctrl-single was that this driver was to be used when the system had a one-register-per-pin layout and it was easy to do device trees based on that. We were very reluctant to accept that even though we didn't even have the generic pin control bindings in place, the argument being that the driver should know the detailed register layout, it should not be described in the device tree. We eventually caved in and accepted it as an exception. Hey let's get few things straight here. There's nothing stopping the driver from knowing a detailed register layout with the pinctrl-single binding. This can be very easily done based on the compatible flag and match data as needed and check the values provided. And let's also recap the reasons for the pinctrl-single binding. The the main reason for the pinctrl-single binding is to avoid mapping individual register bits to device tree compatible string properties. Imagine doing that for hundreds of similar registers. Believe me, I tried using device tree properties initially and it just sucked big time. For larger amounts of dts data, it's best to stick to numeric values and phandles in the device tree data and rely on the preprocessor for getting the values right. Finally, we don't want to build databases into the kernel drivers for every SoC. We certainly have plenty fo those already. Since this pin controller is not using pinctrl-single it does not enjoy that privilege and you need to explain why this pin controller cannot use the generic bindings like so many other pin controllers have since started to do. (It is in the same SoC is not an acceptable argument.) What is wrong with this: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt Nishanth, care to explain your reasons for using pinctrl-single binding here vs the generic binding? Is the amount of string parsing with the data an issue here? Wrong option chosen, I suppose :( - alright, lets discuss the alternative. Heh well now we know :) We can add generic delay bindings to the list, it even seems like a good idea to do so, as it is likely something that will come up in other hardware and will be needed for ACPI etc going forward. We certainly need to make setting delays (with values) generic, no doubt about that. If the large amount of data is not an issue here, we could maybe set each iodelay register as a separate device? Then the binding could be just along the interrupts-extended type binding: foo = bar 0x18c A_DELAY(0) G_DELAY(120); Where the 0x18c is the instance offset of the register within the ti,dra7-iodelay compatible controller. if mmc2_pins_default point at pins for mmc pin configuration for control_core (pinctrl-single), are you proposing the following? mmc2_pins_default: mmc2_pins_default { pinctrl-single,pins = 0x9c (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a23.mmc2_clk */ 0xb0 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_cs1.mmc2_cmd */ 0xa0 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a24.mmc2_dat0 */ 0xa4 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a25.mmc2_dat1 */ 0xa8 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a26.mmc2_dat2 */ 0xac (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a27.mmc2_dat3 */
Re: ARM: OMPA4+: is it expected dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); to fail?
Hi Arnd, On 03/09/2015 11:33 PM, Arnd Bergmann wrote: On Thursday 05 March 2015 20:55:07 grygorii.stras...@linaro.org wrote: Hi All, Now I can see very interesting behavior related to dma_coerce_mask_and_coherent() and friends which I'd like to explain and clarify. Below is set of questions I have (why - I explained below): - Is expected dma_coerce_mask_and_coherent(DMA_BIT_MASK(64)) and friends to fail on 32 bits HW? No. dma_coerce_mask_and_coherent() is meant to ignore the actual mask. It's usually considered a bug to use this function for that reason. - What is expected value for max_pfn: max_phys_pfn or max_phys_pfn + 1? - What is expected value for struct memblock_region-size: mem_range_size or mem_range_size - 1? - What is expected value to be returned by memblock_end_of_DRAM(): @base + @size(max_phys_addr + 1) or @base + @size - 1(max_phys_addr)? I'm working with BeaglBoard-X15 (AM572x/DRA7xx) board and have following code in OMAP ASOC driver which is failed SOMETIMES during the boot with error -EIO. === to omap-pcm.c: omap_pcm_new() { ... ret = dma_coerce_mask_and_coherent(card-dev, DMA_BIT_MASK(64)); ^^ failed sometimes if (ret) return ret; } The code should be fixed to use dma_set_mask_and_coherent(), which is expected to fail if the bus is incapable of addressing all RAM within the mask. I'd be very appreciated for any comments/clarification on questions I've listed at the beginning of my e-mail - there are no patches from my side as I'd like to understand expected behavior of the kernel first (especially taking into account that any memblock changes might affect on at least half of arches). Is the device you have actually 64-bit capable? Is the bus it is connected to 64-bit wide? As I mentioned before - The device was fixed by switching to use 32 bit mask The issue with omap-pcm was simply fixed by using DMA_BIT_MASK(32), . Does the dma-ranges property of the parent bus reflect the correct address width? dma-ranges is not used and all devices are created with default mask DMA_BIT_MASK(32); My goal was to clarify above questions (first of all), because on my HW I can see different values of max_pfn, max_mapnr and memblock configuration depending on CONFIG_ARM_LPAE=n|y and when RAM is defined as: start = 0x8000 size = 0x8000. (and also between kernels 3.14 and LKML). Looks like such RAM configuration is a corner case, which is not always handled as expected (and how is it expected to be handled?). For example: before commit ARM: 8025/1: Get rid of meminfo - registered RAM start = 0x8000 size = 0x8000 will be adjusted by arm_add_memory() and final RAM configuration will be start = 0x8000 size = 0x7000 after this commit: - code will try to register start = 0x8000 size = 0x8000, but memblock will adjust it to start = 0x8000 size = 0x7fff. -- regards, -grygorii -- 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] pinctrl: bindings: pinctrl: Add support for TI's IODelay configuration
On 03/10/2015 05:39 AM, Linus Walleij wrote: On Wed, Mar 4, 2015 at 1:00 AM, Nishanth Menon n...@ti.com wrote: +Configuration definition follows similar model as the pinctrl-single: +The groups of pin configuration are defined under pinctrl-single,pins + +dra7_iodelay_core { + mmc2_iodelay_3v3_conf: mmc2_iodelay_3v3_conf { + pinctrl-single,pins = + 0x18c (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A19_IN */ + 0x1a4 (A_DELAY(265) | G_DELAY(360)) /* CFG_GPMC_A20_IN */ + 0x1b0 (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A21_IN */ + 0x1bc (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A22_IN */ + 0x1c8 (A_DELAY(287) | G_DELAY(420)) /* CFG_GPMC_A23_IN */ + 0x1d4 (A_DELAY(144) | G_DELAY(240)) /* CFG_GPMC_A24_IN */ + 0x1e0 (A_DELAY(0) | G_DELAY(0)) /* CFG_GPMC_A25_IN */ + 0x1ec (A_DELAY(120) | G_DELAY(0)) /* CFG_GPMC_A26_IN */ + 0x1f8 (A_DELAY(120) | G_DELAY(180)) /* CFG_GPMC_A27_IN */ + 0x360 (A_DELAY(0) | G_DELAY(0)) /* CFG_GPMC_CS1_IN */ + ; + }; +}; But wait. The promise when we merged pinctrl-single was that this driver was to be used when the system had a one-register-per-pin layout and it was easy to do device trees based on that. We were very reluctant to accept that even though we didn't even have the generic pin control bindings in place, the argument being that the driver should know the detailed register layout, it should not be described in the device tree. We eventually caved in and accepted it as an exception. Since this pin controller is not using pinctrl-single it does not enjoy that privilege and you need to explain why this pin controller cannot use the generic bindings like so many other pin controllers have since started to do. (It is in the same SoC is not an acceptable argument.) What is wrong with this: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt We can add generic delay bindings to the list, it even seems like a good idea to do so, as it is likely something that will come up in other hardware and will be needed for ACPI etc going forward. Let me try to explain how the hardware works in this instance - it is a quirk that we had'nt understood as being mandatory until very recently. Apologies on pointing to TRM. Unfortunately, understanding this kind of needs us to understand the hardware a little deeper. http://www.ti.com/lit/ug/spruhz6/spruhz6.pdf 18.5.2 CTRL_MODULE_CORE registers (page 4040) - mux starts from CTRL_CORE_PAD_GPMC_AD0 (page 4390) This is the basic support needed for DRA7 family of SoC handled by pinctrl-single. a single register for a single pin - write the mux mode, internal pull, wakeup capability etc. handled today as part of pinctrl-single compatible ti,dra7-padconf. This works for almost 95%+ of the pins. few pins need tweaking of the delay parameters to address per die, operational and board(rarely) characteristics. Here is where it gets a little complicated. 18.6.1 IODELAYCONFIG Module Instance Summary (page 4798) - starts at CFG_RMII_MHZ_50_CLK_IN. These registers are programmed per 18.4.6.1.4 Manual IO Timing Modes (page 4004). Initial sequence of recalibration involves IO isolation - process involving isolating every DRA7 pin from the external world - The only logical place to do this is obviously as part of bootloader. Doing this from kernel can be more than rationally complicated (IO isolation for doing recalibration while a video playback or coprocessor like DSP is active is just plain ridiculous in complexity). The calibrated values then are read for programming these iodelay registers per pin as described in the Section 18.4.6.1.4 Manual IO Timing Modes (page 4005). In summary: - This does not really control traditional points of pinctrl control such as mux mode, pull direction etc. It is, in short, a tweaking of delay paths from the IP to the external pin. - Most pins do not need iodelay register programming. The ones that do may have upto 3 other registers that may need programming (IN, OUT, OUTEN) - Unlike pinctrl-single where a value is read from dts and programmed straight to the register, programming iodelay involves taking two parameter(a_delay and g_delay) per iodelay register, value for the registers computed based on two other parameters(cdpe, fdpe). Where cdpe and fdpe are computed based on recalibration sequence generated values programmed in register fields for ref_count, delay_count and ref_clk_period. - This is also why pinctrl-single wont work here - it is not a copy from dts to register sequence, it is a compute from dts to register sequence. - The recalibration parameters ref_count, delay_count and ref_clk_period
Re: ARM: OMPA4+: is it expected dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64)); to fail?
Hi Russell, On 03/10/2015 01:05 PM, Russell King - ARM Linux wrote: On Fri, Mar 06, 2015 at 11:47:48PM +0200, grygorii.stras...@linaro.org wrote: On 03/05/2015 10:17 PM, Russell King - ARM Linux wrote: On Thu, Mar 05, 2015 at 08:55:07PM +0200, grygorii.stras...@linaro.org wrote: The dma_coerce_mask_and_coherent() will fail in case 'Example 3' and succeed in cases 1,2. dma-mapping.c -- __dma_supported() if (sizeof(mask) != sizeof(dma_addr_t) == true for all OMAP4+ mask (dma_addr_t)~0 == true for DMA_BIT_MASK(64) dma_to_pfn(dev, ~0) max_pfn) { == true only for Example 3 Hmm, I think this may make more sense to be max_pfn - 1 here, as that would be better suited to our intention. The result of dma_to_pfn(dev, ~0) is the maximum PFN which we could address via DMA, but we're comparing it with the maximum PFN in the system plus 1 - so we need to subtract one from it. Ok. I'll try it. Any news on this - I think it is a real off-by-one bug which we should fix in any case. Sorry for delay, there was a day-off on my side. As per my test results - with above change dma_coerce_mask_and_coherent(DMA_BIT_MASK(64)) and friends will succeed always. === Test results: Test case 1: Input data: - RAM: start = 0x8000 size = 0x8000 - CONFIG_ARM_LPAE=n and sizeof(phys_addr_t) = 4 a) NO changes: memory registered within memblock as: memory.cnt = 0x1 memory[0x0] [0x008000-0x00fffe], 0x7fff bytes flags: 0x0 max_pfn = 0xF max_mapnr = 0x7 dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); -- succeeded b) with change in __dma_supported(): if (sizeof(mask) != sizeof(dma_addr_t) mask (dma_addr_t)~0 - dma_to_pfn(dev, ~0) max_pfn) { + dma_to_pfn(dev, ~0) (max_pfn - 1)) { if (warn) { memory registered within memblock as: memory.cnt = 0x1 memory[0x0] [0x008000-0x00fffe], 0x7fff bytes flags: 0x0 max_pfn = 0xF max_mapnr = 0x7 dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); -- succeeded Test case 2: Input data: - RAM: start = 0x8000 size = 0x8000 - CONFIG_ARM_LPAE=y and sizeof(phys_addr_t) = 8 a) NO changes: memory registered within memblock as: memory.cnt = 0x1 memory[0x0] [0x008000-0x00], 0x8000 bytes flags: 0x0 max_pfn = 0x10 max_mapnr = 0x8 dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); -- failed [5.468470] asoc-simple-card sound@0: Coherent DMA mask 0x is larger than dma_addr_t allows [5.478706] asoc-simple-card sound@0: Driver did not use or check the return value from dma_set_coherent_mask()? [5.496620] davinci-mcasp 48468000.mcasp: ASoC: pcm constructor failed: -5 [5.503844] asoc-simple-card sound@0: ASoC: can't create pcm davinci-mcasp.0-tlv320aic3x-hifi :-5 b) with change in __dma_supported(): if (sizeof(mask) != sizeof(dma_addr_t) mask (dma_addr_t)~0 - dma_to_pfn(dev, ~0) max_pfn) { + dma_to_pfn(dev, ~0) (max_pfn - 1)) { if (warn) { memory registered within memblock as: memory.cnt = 0x1 memory[0x0] [0x008000-0x00], 0x8000 bytes flags: 0x0 max_pfn = 0x10 max_mapnr = 0x8 dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)); -- succeeded regards, -grygorii -- regards, -grygorii -- 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 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tue, Mar 10, 2015 at 6:18 PM, Tony Lindgren t...@atomide.com wrote: * Eliad Peller el...@wizery.com [150310 09:11]: On Tue, Mar 10, 2015 at 5:52 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 16:31:33 Eliad Peller wrote: On Tue, Mar 10, 2015 at 4:11 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote: On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. Right, that was my idea. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it. i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution) the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well. What is missing for that binding then? I keep getting confused here, but I thought that they share the implementation that looks at the platform data. they both get the same wl12xx_platform_data struct, but only wl12xx cares about the clocks-related fields. the bindings i added parses only the irq. (Luca tried previously to upstream wl12xx DT support along with the required clock DT changes, but got some rejections, mainly wrt. clock stuff. e.g. http://thread.gmane.org/gmane.linux.kernel/1520752 that's why i preferred starting with easier wl18xx bindings only) I believe we did not have clock bindings back then, now it's simple to get the clock. If it's some internal clock to the wl12xx, then that's a different story, it should be just hidden behind a compatible flag then. i'm really not familiar with the common clock framework, but there still doesn't seem to be a way to determine whether a clock is XTAL or not (which is what Luca's patch was about). should we use compatible flag in such case? i'm not sure it sounds right. anyway, i'd really prefer, if possible, starting with the wl18xx bindings, and defer the wl12xx complications to later on. (i also need to find some wl12xx card in order to actually test the patches once i'll have them...) we do have to make sure these wl18xx bindings are future-compatible with the wl12xx ones, but i think the current bindings are pretty much standard (and are actually a subset of the bindings needed by wl12xx), so it should be fine. Eliad. -- 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 3/3] ARM: dts: igep00x0: add wl18xx bindings
Hello Tony, On Tue, Mar 10, 2015 at 6:35 PM, Tony Lindgren t...@atomide.com wrote: we do have to make sure these wl18xx bindings are future-compatible with the wl12xx ones, but i think the current bindings are pretty much standard (and are actually a subset of the bindings needed by wl12xx), so it should be fine. Well how about add just the parsing of the clock and assume it's always WL12XX_REFCLOCK_38 for now as that's the only thing we're currently using. Then we can add a property or compatible value if using something else as needed. What do you mean by parsing here? IIUC there isn't a clock driver for these clocks and are setup directly in the drivers/net/wireless/ti/wl12xx/main.c driver. So you can't make the WLAN chip dev node a consumer of these clocks by adding a phandle to a clock provider and clock specifiers since there isn't a provider to be referenced in the DT. Or did I misunderstand? Regards, Tony Best regards, Javier -- 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 1/8] time: Add y2038 safe read_boot_clock64()
From: Xunlei Pang pang.xun...@linaro.org As part of addressing in-kernel y2038 issues, this patch adds read_boot_clock64() and replaces all the call sites of read_boot_clock() with this function. This is a __weak implementation, which simply calls the existing y2038 unsafe read_boot_clock(). This allows architecture specific implementations to be converted independently, and eventually the y2038 unsafe read_boot_clock() can be removed after all its architecture specific implementations have been converted to read_boot_clock64(). Suggested-by: Arnd Bergmann a...@arndb.de Signed-off-by: Xunlei Pang pang.xun...@linaro.org --- include/linux/timekeeping.h | 1 + kernel/time/timekeeping.c | 11 +-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h index 3eaae47..d53c522 100644 --- a/include/linux/timekeeping.h +++ b/include/linux/timekeeping.h @@ -263,6 +263,7 @@ static inline bool has_persistent_clock(void) extern void read_persistent_clock(struct timespec *ts); extern void read_boot_clock(struct timespec *ts); +extern void read_boot_clock64(struct timespec64 *ts); extern int update_persistent_clock(struct timespec now); diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 91db941..7080c21 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -1072,6 +1072,14 @@ void __weak read_boot_clock(struct timespec *ts) ts-tv_nsec = 0; } +void __weak read_boot_clock64(struct timespec64 *ts64) +{ + struct timespec ts; + + read_boot_clock(ts); + *ts64 = timespec_to_timespec64(ts); +} + /* * timekeeping_init - Initializes the clocksource and common timekeeping values */ @@ -1093,8 +1101,7 @@ void __init timekeeping_init(void) } else if (now.tv_sec || now.tv_nsec) persistent_clock_exist = true; - read_boot_clock(ts); - boot = timespec_to_timespec64(ts); + read_boot_clock64(boot); if (!timespec64_valid_strict(boot)) { pr_warn(WARNING: Boot clock returned invalid value!\n Check your CMOS/BIOS settings.\n); -- 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 2/8] time: Add y2038 safe read_persistent_clock64()
From: Xunlei Pang pang.xun...@linaro.org As part of addressing in-kernel y2038 issues, this patch adds read_persistent_clock64() and replaces all the call sites of read_persistent_clock() with this function. This is a __weak implementation, which simply calls the existing y2038 unsafe read_persistent_clock(). This allows architecture specific implementations to be converted independently, and eventually the y2038 unsafe read_persistent_clock() can be removed after all its architecture specific implementations have been converted to read_persistent_clock64(). Suggested-by: Arnd Bergmann a...@arndb.de Signed-off-by: Xunlei Pang pang.xun...@linaro.org --- arch/mips/lasat/sysctl.c| 4 ++-- include/linux/timekeeping.h | 1 + kernel/time/timekeeping.c | 22 -- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/arch/mips/lasat/sysctl.c b/arch/mips/lasat/sysctl.c index 3b7f65c..cf9b4633 100644 --- a/arch/mips/lasat/sysctl.c +++ b/arch/mips/lasat/sysctl.c @@ -75,11 +75,11 @@ static int rtctmp; int proc_dolasatrtc(struct ctl_table *table, int write, void *buffer, size_t *lenp, loff_t *ppos) { - struct timespec ts; + struct timespec64 ts; int r; if (!write) { - read_persistent_clock(ts); + read_persistent_clock64(ts); rtctmp = ts.tv_sec; /* check for time 0 and set to 0 */ if (rtctmp 0) diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h index d53c522..ff56a0f 100644 --- a/include/linux/timekeeping.h +++ b/include/linux/timekeeping.h @@ -262,6 +262,7 @@ static inline bool has_persistent_clock(void) } extern void read_persistent_clock(struct timespec *ts); +extern void read_persistent_clock64(struct timespec64 *ts); extern void read_boot_clock(struct timespec *ts); extern void read_boot_clock64(struct timespec64 *ts); extern int update_persistent_clock(struct timespec now); diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 7080c21..0e5a696 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -1057,6 +1057,14 @@ void __weak read_persistent_clock(struct timespec *ts) ts-tv_nsec = 0; } +void __weak read_persistent_clock64(struct timespec64 *ts64) +{ + struct timespec ts; + + read_persistent_clock(ts); + *ts64 = timespec_to_timespec64(ts); +} + /** * read_boot_clock - Return time of the system start. * @@ -1089,10 +1097,8 @@ void __init timekeeping_init(void) struct clocksource *clock; unsigned long flags; struct timespec64 now, boot, tmp; - struct timespec ts; - read_persistent_clock(ts); - now = timespec_to_timespec64(ts); + read_persistent_clock64(now); if (!timespec64_valid_strict(now)) { pr_warn(WARNING: Persistent clock returned invalid value!\n Check your CMOS/BIOS settings.\n); @@ -1163,7 +1169,7 @@ static void __timekeeping_inject_sleeptime(struct timekeeper *tk, * timekeeping_inject_sleeptime64 - Adds suspend interval to timeekeeping values * @delta: pointer to a timespec64 delta value * - * This hook is for architectures that cannot support read_persistent_clock + * This hook is for architectures that cannot support read_persistent_clock64 * because their RTC/persistent clock is only accessible when irqs are enabled. * * This function should only be called by rtc_resume(), and allows @@ -1210,12 +1216,10 @@ void timekeeping_resume(void) struct clocksource *clock = tk-tkr.clock; unsigned long flags; struct timespec64 ts_new, ts_delta; - struct timespec tmp; cycle_t cycle_now, cycle_delta; bool suspendtime_found = false; - read_persistent_clock(tmp); - ts_new = timespec_to_timespec64(tmp); + read_persistent_clock64(ts_new); clockevents_resume(); clocksource_resume(); @@ -1291,10 +1295,8 @@ int timekeeping_suspend(void) unsigned long flags; struct timespec64 delta, delta_delta; static struct timespec64old_delta; - struct timespec tmp; - read_persistent_clock(tmp); - timekeeping_suspend_time = timespec_to_timespec64(tmp); + read_persistent_clock64(timekeeping_suspend_time); /* * On some systems the persistent_clock can not be detected at -- 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 3/8] time: Add y2038 safe update_persistent_clock64()
From: Xunlei Pang pang.xun...@linaro.org As part of addressing in-kernel y2038 issues, this patch adds update_persistent_clock64() and replaces all the call sites of update_persistent_clock() with this function. This is a __weak implementation, which simply calls the existing y2038 unsafe update_persistent_clock(). This allows architecture specific implementations to be converted independently, and eventually y2038-unsafe update_persistent_clock() can be removed after all its architecture specific implementations have been converted to update_persistent_clock64(). Suggested-by: Arnd Bergmann a...@arndb.de Signed-off-by: Xunlei Pang pang.xun...@linaro.org --- drivers/rtc/systohc.c | 2 +- include/linux/timekeeping.h | 1 + kernel/time/ntp.c | 13 - 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/rtc/systohc.c b/drivers/rtc/systohc.c index eb71872..ef3c07a 100644 --- a/drivers/rtc/systohc.c +++ b/drivers/rtc/systohc.c @@ -11,7 +11,7 @@ * rtc_set_ntp_time - Save NTP synchronized time to the RTC * @now: Current time of day * - * Replacement for the NTP platform function update_persistent_clock + * Replacement for the NTP platform function update_persistent_clock64 * that stores time for later retrieval by rtc_hctosys. * * Returns 0 on successful RTC update, -ENODEV if a RTC update is not diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h index ff56a0f..a7fa96b 100644 --- a/include/linux/timekeeping.h +++ b/include/linux/timekeeping.h @@ -266,6 +266,7 @@ extern void read_persistent_clock64(struct timespec64 *ts); extern void read_boot_clock(struct timespec *ts); extern void read_boot_clock64(struct timespec64 *ts); extern int update_persistent_clock(struct timespec now); +extern int update_persistent_clock64(struct timespec64 now); #endif diff --git a/kernel/time/ntp.c b/kernel/time/ntp.c index 0f60b08..42d1bc7 100644 --- a/kernel/time/ntp.c +++ b/kernel/time/ntp.c @@ -459,6 +459,16 @@ out: return leap; } +#ifdef CONFIG_GENERIC_CMOS_UPDATE +int __weak update_persistent_clock64(struct timespec64 now64) +{ + struct timespec now; + + now = timespec64_to_timespec(now64); + return update_persistent_clock(now); +} +#endif + #if defined(CONFIG_GENERIC_CMOS_UPDATE) || defined(CONFIG_RTC_SYSTOHC) static void sync_cmos_clock(struct work_struct *work); @@ -494,8 +504,9 @@ static void sync_cmos_clock(struct work_struct *work) if (persistent_clock_is_local) adjust.tv_sec -= (sys_tz.tz_minuteswest * 60); #ifdef CONFIG_GENERIC_CMOS_UPDATE - fail = update_persistent_clock(timespec64_to_timespec(adjust)); + fail = update_persistent_clock64(adjust); #endif + #ifdef CONFIG_RTC_SYSTOHC if (fail == -ENODEV) fail = rtc_set_ntp_time(adjust); -- 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 4/8] ARM: OMAP: 32k counter: Provide y2038-safe omap_read_persistent_clock() replacement
From: Xunlei Pang pang.xun...@linaro.org As part of addressing y2038 problem for in-kernel uses, this patch adds the y2038-safe omap_read_persistent_clock64() using timespec64. Because we rely on some subsequent changes to convert arm multiarch support, omap_read_persistent_clock() will be removed then. Also remove the needless spinlock, because read_persistent_clock() doesn't run simultaneously. Signed-off-by: Xunlei Pang pang.xun...@linaro.org --- arch/arm/plat-omap/counter_32k.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/arch/arm/plat-omap/counter_32k.c b/arch/arm/plat-omap/counter_32k.c index 61b4d70..d422e36 100644 --- a/arch/arm/plat-omap/counter_32k.c +++ b/arch/arm/plat-omap/counter_32k.c @@ -44,24 +44,20 @@ static u64 notrace omap_32k_read_sched_clock(void) } /** - * omap_read_persistent_clock - Return time from a persistent clock. + * omap_read_persistent_clock64 - Return time from a persistent clock. * * Reads the time from a source which isn't disabled during PM, the * 32k sync timer. Convert the cycles elapsed since last read into - * nsecs and adds to a monotonically increasing timespec. + * nsecs and adds to a monotonically increasing timespec64. */ -static struct timespec persistent_ts; +static struct timespec64 persistent_ts; static cycles_t cycles; static unsigned int persistent_mult, persistent_shift; -static DEFINE_SPINLOCK(read_persistent_clock_lock); -static void omap_read_persistent_clock(struct timespec *ts) +static void omap_read_persistent_clock64(struct timespec64 *ts) { unsigned long long nsecs; cycles_t last_cycles; - unsigned long flags; - - spin_lock_irqsave(read_persistent_clock_lock, flags); last_cycles = cycles; cycles = sync32k_cnt_reg ? readl_relaxed(sync32k_cnt_reg) : 0; @@ -69,11 +65,17 @@ static void omap_read_persistent_clock(struct timespec *ts) nsecs = clocksource_cyc2ns(cycles - last_cycles, persistent_mult, persistent_shift); - timespec_add_ns(persistent_ts, nsecs); + timespec64_add_ns(persistent_ts, nsecs); *ts = persistent_ts; +} + +static void omap_read_persistent_clock(struct timespec *ts) +{ + struct timespec64 ts64; - spin_unlock_irqrestore(read_persistent_clock_lock, flags); + omap_read_persistent_clock64(ts64); + *ts = timespec64_to_timespec(ts64); } /** -- 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 v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
Hello Eliad, On Wed, Mar 11, 2015 at 1:28 AM, Javier Martinez Canillas jav...@dowhile0.org wrote: On Mon, Mar 9, 2015 at 4:36 PM, Eliad Peller el...@wizery.com wrote: --- a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts +++ b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts @@ -42,4 +42,13 @@ vmmc-supply = lbep5clwmc_wlen; Something I forgot to mention is that this vmmc-supply is a DT hack since the WLAN SDIO chip needs a WL_EN signal to be asserted as a part of the chip power sequencing. But there wasn't a DT binding to describe that so most boards use the same hack which is to define a fake fixed-regulator with an enable GPIO just to toggle that pin. But now the MMC subsystem has support for power sequence providers and the pwrseq-simple driver (Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt) has support for reset GPIOs so you may want to change that as well. In fact, the pwrseq-simple also has support for an external clock (and can be extended to support more than one) so if the clocks are not internal to the wl12xx, maybe these should be defined in the pwrseq-simple dev node assuming that there is a clock driver for them. Best regards, Javier -- 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 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tue, Mar 10, 2015 at 5:52 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 16:31:33 Eliad Peller wrote: On Tue, Mar 10, 2015 at 4:11 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote: On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. Right, that was my idea. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it. i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution) the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well. What is missing for that binding then? I keep getting confused here, but I thought that they share the implementation that looks at the platform data. they both get the same wl12xx_platform_data struct, but only wl12xx cares about the clocks-related fields. the bindings i added parses only the irq. (Luca tried previously to upstream wl12xx DT support along with the required clock DT changes, but got some rejections, mainly wrt. clock stuff. e.g. http://thread.gmane.org/gmane.linux.kernel/1520752 that's why i preferred starting with easier wl18xx bindings only) Eliad. -- 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 2/2] ARM: dts: am33xx: Move wkup_m3 node to soc node and add ranges
* Suman Anna s-a...@ti.com [150309 16:59]: On 03/05/2015 10:57 AM, Tony Lindgren wrote: * Suman Anna s-a...@ti.com [150305 08:47]: On 03/05/2015 09:40 AM, Tony Lindgren wrote: * Dave Gerlach d-gerl...@ti.com [150304 20:14]: Dave, Looks like the commit message disappeared during your patch preparation. Signed-off-by: Suman Anna s-a...@ti.com Signed-off-by: Dave Gerlach d-gerl...@ti.com --- arch/arm/boot/dts/am33xx.dtsi | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index acd3705..086415c 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -77,10 +77,23 @@ */ soc { compatible = ti,omap-infra; +#address-cells = 1; +#size-cells = 1; +ranges = 0x0 0x44d0 0x4000, + 0x8 0x44d8 0x2000; + I think putting the ranges here will cause issues for adding ranges for anything else. How about do something like this instead (untested): ocp { l4_wkup: l4_wkup@44c0 { compatible = am335-l4-wkup, simple-bus; ranges = 0 0x44c0 0x3f; wkup_m3: wkup_m3@44d0 { compatible = ti,am3353-wkup-m3; reg = 0x100 0x4000, /* M3 UMEM */ 0x18 0x2000; /* M3 DMEM */ ti,hwmods = wkup_m3; ti,pm-firmware = am335x-pm-firmware.elf; }; ... }; }; That way we can start moving also the other l4_wkup components there eventuallly without having to redo the ranges again for wkup_m3. You can also look at how the scm_conf was done for dm816x.dtsi for an example, and the recent large set of patches posted by Tero. I have taken a look at both the above. The L4_WKUP range includes the PRCM, Control Module, as well as a few peripherals like DMTimer0, UART0 etc. What all do we want to move here eventually? Well eventually all the children of L4_WKUP, but that can be done slowly as some of the drivers have weird hacks and may not work properly if moved around. For example, anything with reg entries for something like SCM area will break as that's not going to be in the L4_WKUP area ny longer :p And that's actually good as it will protect us from spaghetti code automatically later on for new code. Depending on that, we may have to use 2 address cells like in Tero's PRCM cleanup series rather than the single cell translation used by you in dm816x.dtsi so that we can retain the relative addresses w.r.t the existing node bases in the derivative child nodes. Hmm OK, care to paste a dts snippet example for that? Regards, Tony -- 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] mfd: menelaus: rmove incorrect __exit markups
On Mon, 09 Mar 2015, Dmitry Torokhov wrote: Even if bus is not hot-pluggable, the devices can be unbound from the driver via sysfs, so we should not be using __exit annotations on remove() methods. The only exception is drivers registered with platform_driver_probe() which specifically disables sysfs bind/unbind attributes. Signed-off-by: Dmitry Torokhov dmitry.torok...@gmail.com --- drivers/mfd/menelaus.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Applied, thanks. diff --git a/drivers/mfd/menelaus.c b/drivers/mfd/menelaus.c index 9f01aef..5269ff2 100644 --- a/drivers/mfd/menelaus.c +++ b/drivers/mfd/menelaus.c @@ -1259,7 +1259,7 @@ fail: return err; } -static int __exit menelaus_remove(struct i2c_client *client) +static int menelaus_remove(struct i2c_client *client) { struct menelaus_chip*menelaus = i2c_get_clientdata(client); @@ -1280,7 +1280,7 @@ static struct i2c_driver menelaus_i2c_driver = { .name = DRIVER_NAME, }, .probe = menelaus_probe, - .remove = __exit_p(menelaus_remove), + .remove = menelaus_remove, .id_table = menelaus_id, }; -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- 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 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tuesday 10 March 2015 07:28:05 Tony Lindgren wrote: Oops I forgot about the omap3-sbc-t3730, so yes we have to keep the platform data a little bit longer. But nothing stopping us moving all the other ones to use a proper device tree based configuration. For all I can tell, the t3730 board file does not support wl12xx at the moment, only the dts file does. If we do what Sekhar suggested and drop wl12xx support from the DA850 EVM board file, we can fix all wlcore users. Arnd -- 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 3/3] ARM: dts: igep00x0: add wl18xx bindings
* Eliad Peller el...@wizery.com [150310 09:11]: On Tue, Mar 10, 2015 at 5:52 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 16:31:33 Eliad Peller wrote: On Tue, Mar 10, 2015 at 4:11 PM, Arnd Bergmann a...@arndb.de wrote: On Tuesday 10 March 2015 13:00:19 Eliad Peller wrote: On Tue, Mar 10, 2015 at 12:49 AM, Tony Lindgren t...@atomide.com wrote: I was expecting you to remove all calls to legacy_init_wl12xx from this file, including the ones for wl12xx aside from the wl18xx ones you removed, but if that's enough to clean out the platform_data handling from the wlcore driver, it's good enough as a start. not sure i'm following - can you elaborate? i'll summarize the way i see it. please correct me if i'm wrong. both wl18xx and wl12xx use the platform data to get the irq number. wl12xx (only) also needs some additional clock definitions to be passed. there's currently some issue with specifying some the of clock sources, so i preferred starting only with (the simpler) wl18xx bindings. for platforms with wl18xx, we can remove the pdata-quirk, as all the data (i.e. irq) can be passed by the new DT bindings. however, for platforms with wl12xx, we still need to pass the clock definitions (along with the irq), so we have to keep legacy_init_wl12xx for the time being (and that's also why we have to currently keep the platform_data handling in the wlcore driver) do you have something else in mind? I think what Arnd is saying is we've now removed all the wl12xx using legacy platforms, so all of them can boot with just data from dts. Right, that was my idea. I don't think that's the case (unless i'm missing something). e.g. there's still pdata-quirk for compulab,omap3-sbc-t3730 which initializes wl12xx device. This one is just like the igep0030, as Tony was saying: the board boots from device tree already, so now that we have a binding for it, we can remove the wl12xx_set_platform_data() for it. i think the wl12xx_set_platform_data() name created some confusion - it is used to pass platform data for both wl12xx and wl18xx devices. (this confusion is all around the wlcore driver as well, due to the code evolution) the binding i added is for wl18xx only (there is no wl12xx binding yet). the remaining boards, AFAICT, have wl12xx (rather than wl18xx) cards. so i don't see how we can remove these wl12xx_set_platform_data() calls before we have wl12xx bindings in-place as well. What is missing for that binding then? I keep getting confused here, but I thought that they share the implementation that looks at the platform data. they both get the same wl12xx_platform_data struct, but only wl12xx cares about the clocks-related fields. the bindings i added parses only the irq. (Luca tried previously to upstream wl12xx DT support along with the required clock DT changes, but got some rejections, mainly wrt. clock stuff. e.g. http://thread.gmane.org/gmane.linux.kernel/1520752 that's why i preferred starting with easier wl18xx bindings only) I believe we did not have clock bindings back then, now it's simple to get the clock. If it's some internal clock to the wl12xx, then that's a different story, it should be just hidden behind a compatible flag then. Regards, Tony -- 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] ARM: OMAP2+: omap_hwmod: Introduce ti,no-init dt property
Tony, On 03/06/2015 11:45 AM, Tony Lindgren wrote: * Dave Gerlach d-gerl...@ti.com [150306 09:28]: On 03/05/2015 06:41 PM, Tony Lindgren wrote: * Tony Lindgren t...@atomide.com [150305 12:24]: * Dave Gerlach d-gerl...@ti.com [150305 11:53]: On 03/05/2015 12:49 PM, Tony Lindgren wrote: * Paul Walmsley p...@pwsan.com [150305 10:16]: On Thu, 5 Mar 2015, Dave Gerlach wrote: Introduce a dt property, ti,no-init, that prevents hwmod initialization. Even if a dt node is marked as disabled, hwmod still at least enables the hwmod and programs the sysconfig before attempting to idle it at boot. If an IP has been disabled by the hardware configuration on a platform, this will cause a hang due to writing to inactive registers. This property prevents that from happening by marking the hwmod as _HWMOD_STATE_DISABLED during init. I'm kind of wondering if hwmod should even touch a device if it's marked as disabled in the DT. Tony, what do you think? Well nothing happens if a device is status = disabled. No dev entry gets created for it at all and hwmod won't have any data for the device populated. The only way hwmod code could see that device if the device gets it's data from the legacy omap_hwmod_*_data.c instead of DT. We still need this for the sysconfig programming, correct? hwmod programs that regardless of dt status and then idles the IP, Well hwmod does not even know about the IP IO addresses if it's marked with status = disabled.. Which IP are you having problems with? which is why I needed the ti,no-init for the epos evm. It isn't just a matter of we shouldnt write to it because we don't want to use it; we can't write to it because the module is held off so it causes an external abort if we do. Well hard to say not knowing which module this is.. Pretty much all the modules have drivers and the driver just does pm_runtime_get() on it? Heh OK this thread is about the RTC driver, so I assume that's the problem :) So if you set the rtc to status = disabled how can the hwmod code do anything as AFAIK it won't even get the rtc IO address? Or am I missing something here? Perhaps I am mistaken, but from what I understand, all hwmods have _init and _setup called on them, and all hwmods read the IO address regardless of DT status at this point with _init_mpu_rt_base. In _setup, _setup_reset gets called which calls _enable for the hwmod, and this calls both _enable_sysc and _update_sysc_cache which touch the sysconfig register of the IP. Oh OK, I think you're right. I was thinking of omap_device_build_from_dt(), sorry. Looks like the hwmod IO address data does get populated even for status = disabled although the dev entry won't get created and omap_device_build_from_dt() never gets called. Normally this is fine regardless of whether or not we are using an IP because the module will wake up for a moment, have it's sysc programmed, and then be put back to sleep later, potentially never to be woken again if we bind no driver for it, which is fine for 99% of cases. In the case of am43x epos evm, you can take the same piece of silicon that will boot happily on the gp evm with the rtc hwmod in place and it will hang during boot on the epos evm because the board uses a pin to hold the RTC IP in reset. There is no way to detect this in software, the module can be turned on as normal using the clk_ctrl, but if you touch any of the IP registers you get an abort. OK sounds like some dts property is needed to signal this. So we need to prevent this from happening but of course we can't selectively choose when the rtc hwmod gets added based on which board we are using so it seemed a DT flag was appropriate to indicate that we do not want to init the rtc IP at all only on this board. Without this flag in place but with the rtc hwmod added, the am43x-epos-evm fails booting with an imprecise abort. OK thanks for explaining it. I'm fine with this patch, Paul may have other issues. The other option would be to use status = disabled to not touch the device at all like Paul suggested. I wonder if that's going to break PM on some devices though.. Well the initial programming of the SYSCONFIG register is pretty important for certain modules, just take AM33xx USB hwmod for example. If no USB driver is bound even just letting omap_device idle it using the USB_ CLKCTRL in the PRCM after no driver gets bound will not work properly, because the USB on am33xx expects it's SYSCONFIG STANDBY_MODE to be software supervised, the default state (SMART_STANDBY) does not let the IP go into standby so the IP gets stuck in a partially off state but does not actually shut down as expected. So I don't think we can change this without consequences elsewhere. Regards, Dave Regards, Tony -- 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
Re: [PATCH 1/2] pinctrl: bindings: pinctrl: Add support for TI's IODelay configuration
On 03/10/2015 12:31 PM, Tony Lindgren wrote: * Nishanth Menon n...@ti.com [150310 10:25]: On 03/10/2015 10:33 AM, Tony Lindgren wrote: * Linus Walleij linus.wall...@linaro.org [150310 03:39]: On Wed, Mar 4, 2015 at 1:00 AM, Nishanth Menon n...@ti.com wrote: +Configuration definition follows similar model as the pinctrl-single: +The groups of pin configuration are defined under pinctrl-single,pins + +dra7_iodelay_core { + mmc2_iodelay_3v3_conf: mmc2_iodelay_3v3_conf { + pinctrl-single,pins = + 0x18c (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A19_IN */ + 0x1a4 (A_DELAY(265) | G_DELAY(360)) /* CFG_GPMC_A20_IN */ + 0x1b0 (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A21_IN */ + 0x1bc (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A22_IN */ + 0x1c8 (A_DELAY(287) | G_DELAY(420)) /* CFG_GPMC_A23_IN */ + 0x1d4 (A_DELAY(144) | G_DELAY(240)) /* CFG_GPMC_A24_IN */ + 0x1e0 (A_DELAY(0) | G_DELAY(0)) /* CFG_GPMC_A25_IN */ + 0x1ec (A_DELAY(120) | G_DELAY(0)) /* CFG_GPMC_A26_IN */ + 0x1f8 (A_DELAY(120) | G_DELAY(180)) /* CFG_GPMC_A27_IN */ + 0x360 (A_DELAY(0) | G_DELAY(0)) /* CFG_GPMC_CS1_IN */ + ; + }; +}; But wait. The promise when we merged pinctrl-single was that this driver was to be used when the system had a one-register-per-pin layout and it was easy to do device trees based on that. We were very reluctant to accept that even though we didn't even have the generic pin control bindings in place, the argument being that the driver should know the detailed register layout, it should not be described in the device tree. We eventually caved in and accepted it as an exception. Hey let's get few things straight here. There's nothing stopping the driver from knowing a detailed register layout with the pinctrl-single binding. This can be very easily done based on the compatible flag and match data as needed and check the values provided. And let's also recap the reasons for the pinctrl-single binding. The the main reason for the pinctrl-single binding is to avoid mapping individual register bits to device tree compatible string properties. Imagine doing that for hundreds of similar registers. Believe me, I tried using device tree properties initially and it just sucked big time. For larger amounts of dts data, it's best to stick to numeric values and phandles in the device tree data and rely on the preprocessor for getting the values right. Finally, we don't want to build databases into the kernel drivers for every SoC. We certainly have plenty fo those already. Since this pin controller is not using pinctrl-single it does not enjoy that privilege and you need to explain why this pin controller cannot use the generic bindings like so many other pin controllers have since started to do. (It is in the same SoC is not an acceptable argument.) What is wrong with this: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt Nishanth, care to explain your reasons for using pinctrl-single binding here vs the generic binding? Is the amount of string parsing with the data an issue here? Wrong option chosen, I suppose :( - alright, lets discuss the alternative. Heh well now we know :) We can add generic delay bindings to the list, it even seems like a good idea to do so, as it is likely something that will come up in other hardware and will be needed for ACPI etc going forward. We certainly need to make setting delays (with values) generic, no doubt about that. If the large amount of data is not an issue here, we could maybe set each iodelay register as a separate device? Then the binding could be just along the interrupts-extended type binding: foo = bar 0x18c A_DELAY(0) G_DELAY(120); Where the 0x18c is the instance offset of the register within the ti,dra7-iodelay compatible controller. if mmc2_pins_default point at pins for mmc pin configuration for control_core (pinctrl-single), are you proposing the following? mmc2_pins_default: mmc2_pins_default { pinctrl-single,pins = 0x9c (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a23.mmc2_clk */ 0xb0 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_cs1.mmc2_cmd */ 0xa0 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a24.mmc2_dat0 */ 0xa4 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a25.mmc2_dat1 */ 0xa8 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a26.mmc2_dat2 */ 0xac (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a27.mmc2_dat3 */ 0x8c (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /*
Re: [PATCH 1/2] pinctrl: bindings: pinctrl: Add support for TI's IODelay configuration
On 03/10/2015 01:33 PM, Nishanth Menon wrote: On 03/10/2015 12:31 PM, Tony Lindgren wrote: * Nishanth Menon n...@ti.com [150310 10:25]: On 03/10/2015 10:33 AM, Tony Lindgren wrote: * Linus Walleij linus.wall...@linaro.org [150310 03:39]: On Wed, Mar 4, 2015 at 1:00 AM, Nishanth Menon n...@ti.com wrote: +Configuration definition follows similar model as the pinctrl-single: +The groups of pin configuration are defined under pinctrl-single,pins + +dra7_iodelay_core { + mmc2_iodelay_3v3_conf: mmc2_iodelay_3v3_conf { + pinctrl-single,pins = + 0x18c (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A19_IN */ + 0x1a4 (A_DELAY(265) | G_DELAY(360)) /* CFG_GPMC_A20_IN */ + 0x1b0 (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A21_IN */ + 0x1bc (A_DELAY(0) | G_DELAY(120)) /* CFG_GPMC_A22_IN */ + 0x1c8 (A_DELAY(287) | G_DELAY(420)) /* CFG_GPMC_A23_IN */ + 0x1d4 (A_DELAY(144) | G_DELAY(240)) /* CFG_GPMC_A24_IN */ + 0x1e0 (A_DELAY(0) | G_DELAY(0)) /* CFG_GPMC_A25_IN */ + 0x1ec (A_DELAY(120) | G_DELAY(0)) /* CFG_GPMC_A26_IN */ + 0x1f8 (A_DELAY(120) | G_DELAY(180)) /* CFG_GPMC_A27_IN */ + 0x360 (A_DELAY(0) | G_DELAY(0)) /* CFG_GPMC_CS1_IN */ + ; + }; +}; But wait. The promise when we merged pinctrl-single was that this driver was to be used when the system had a one-register-per-pin layout and it was easy to do device trees based on that. We were very reluctant to accept that even though we didn't even have the generic pin control bindings in place, the argument being that the driver should know the detailed register layout, it should not be described in the device tree. We eventually caved in and accepted it as an exception. Hey let's get few things straight here. There's nothing stopping the driver from knowing a detailed register layout with the pinctrl-single binding. This can be very easily done based on the compatible flag and match data as needed and check the values provided. And let's also recap the reasons for the pinctrl-single binding. The the main reason for the pinctrl-single binding is to avoid mapping individual register bits to device tree compatible string properties. Imagine doing that for hundreds of similar registers. Believe me, I tried using device tree properties initially and it just sucked big time. For larger amounts of dts data, it's best to stick to numeric values and phandles in the device tree data and rely on the preprocessor for getting the values right. Finally, we don't want to build databases into the kernel drivers for every SoC. We certainly have plenty fo those already. Since this pin controller is not using pinctrl-single it does not enjoy that privilege and you need to explain why this pin controller cannot use the generic bindings like so many other pin controllers have since started to do. (It is in the same SoC is not an acceptable argument.) What is wrong with this: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt Nishanth, care to explain your reasons for using pinctrl-single binding here vs the generic binding? Is the amount of string parsing with the data an issue here? Wrong option chosen, I suppose :( - alright, lets discuss the alternative. Heh well now we know :) We can add generic delay bindings to the list, it even seems like a good idea to do so, as it is likely something that will come up in other hardware and will be needed for ACPI etc going forward. We certainly need to make setting delays (with values) generic, no doubt about that. If the large amount of data is not an issue here, we could maybe set each iodelay register as a separate device? Then the binding could be just along the interrupts-extended type binding: foo = bar 0x18c A_DELAY(0) G_DELAY(120); Where the 0x18c is the instance offset of the register within the ti,dra7-iodelay compatible controller. if mmc2_pins_default point at pins for mmc pin configuration for control_core (pinctrl-single), are you proposing the following? mmc2_pins_default: mmc2_pins_default { pinctrl-single,pins = 0x9c (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a23.mmc2_clk */ 0xb0 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_cs1.mmc2_cmd */ 0xa0 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a24.mmc2_dat0 */ 0xa4 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a25.mmc2_dat1 */ 0xa8 (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a26.mmc2_dat2 */ 0xac (PIN_INPUT_PULLUP | MANUAL_MODE | MUX_MODE1) /* gpmc_a27.mmc2_dat3 */ 0x8c
Re: [PATCH v2 2/2] ARM: dts: am33xx: Move wkup_m3 node to soc node and add ranges
Tony, On 03/10/2015 11:09 AM, Tony Lindgren wrote: * Suman Anna s-a...@ti.com [150309 16:59]: On 03/05/2015 10:57 AM, Tony Lindgren wrote: * Suman Anna s-a...@ti.com [150305 08:47]: On 03/05/2015 09:40 AM, Tony Lindgren wrote: * Dave Gerlach d-gerl...@ti.com [150304 20:14]: Dave, Looks like the commit message disappeared during your patch preparation. Signed-off-by: Suman Anna s-a...@ti.com Signed-off-by: Dave Gerlach d-gerl...@ti.com --- arch/arm/boot/dts/am33xx.dtsi | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi index acd3705..086415c 100644 --- a/arch/arm/boot/dts/am33xx.dtsi +++ b/arch/arm/boot/dts/am33xx.dtsi @@ -77,10 +77,23 @@ */ soc { compatible = ti,omap-infra; +#address-cells = 1; +#size-cells = 1; +ranges = 0x0 0x44d0 0x4000, + 0x8 0x44d8 0x2000; + I think putting the ranges here will cause issues for adding ranges for anything else. How about do something like this instead (untested): ocp { l4_wkup: l4_wkup@44c0 { compatible = am335-l4-wkup, simple-bus; ranges = 0 0x44c0 0x3f; wkup_m3: wkup_m3@44d0 { compatible = ti,am3353-wkup-m3; reg = 0x100 0x4000, /* M3 UMEM */ 0x18 0x2000; /* M3 DMEM */ ti,hwmods = wkup_m3; ti,pm-firmware = am335x-pm-firmware.elf; }; ... }; }; That way we can start moving also the other l4_wkup components there eventuallly without having to redo the ranges again for wkup_m3. You can also look at how the scm_conf was done for dm816x.dtsi for an example, and the recent large set of patches posted by Tero. I have taken a look at both the above. The L4_WKUP range includes the PRCM, Control Module, as well as a few peripherals like DMTimer0, UART0 etc. What all do we want to move here eventually? Well eventually all the children of L4_WKUP, but that can be done slowly as some of the drivers have weird hacks and may not work properly if moved around. For example, anything with reg entries for something like SCM area will break as that's not going to be in the L4_WKUP area ny longer :p And that's actually good as it will protect us from spaghetti code automatically later on for new code. Depending on that, we may have to use 2 address cells like in Tero's PRCM cleanup series rather than the single cell translation used by you in dm816x.dtsi so that we can retain the relative addresses w.r.t the existing node bases in the derivative child nodes. Hmm OK, care to paste a dts snippet example for that? Suman and I have been looking at this together, so I can comment here. An implementation like this is what Suman is referring to: + l4_wkup: l4_wkup@44c0 { + compatible = am335-l4-wkup, simple-bus; + #address-cells = 2; + #size-cells = 1; + ranges = 0 0 0x44c0 0x10, +1 0x0 0x44d0 0x4000, +2 0x8 0x44d8 0x2000; + + wkup_m3: wkup_m3@1,0 { + compatible = ti,am3353-wkup-m3; + reg = 1 0x0 0x4000, /* M3 UMEM */ + 2 0x8 0x2000; /* M3 DMEM */ + + ti,hwmods = wkup_m3; + ti,pm-firmware = am335x-pm-firmware.elf; + }; + }; + The of_* layer automatically translates everything so the pdata-quirks can still match based on wkup_m3@44d0. The existing wkup_m3_rproc driver works almost entirely as is with this, all cpu addresses are read and mapped correctly but the driver no longer will read the actual device addresses correctly which we need for understanding where to load the firmware sections. These device addresses are being read directly using of_get_address, which reads the first value in the reg entries which is 1 and 2 now for UMEM and DMEM. We would need some sort of change there also to get the proper 0x0 and 0x8 device address values. Just advancing the pointer returned by of_get_address does the trick but this doesn't seem like the cleanest solution. Regards, Dave Regards, Tony -- 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 3/3] ARM: dts: igep00x0: add wl18xx bindings
On Tuesday 10 March 2015 10:35:50 Tony Lindgren wrote: * Eliad Peller el...@wizery.com [150310 10:01]: i'm really not familiar with the common clock framework, but there still doesn't seem to be a way to determine whether a clock is XTAL or not (which is what Luca's patch was about). should we use compatible flag in such case? i'm not sure it sounds right. anyway, i'd really prefer, if possible, starting with the wl18xx bindings, and defer the wl12xx complications to later on. (i also need to find some wl12xx card in order to actually test the patches once i'll have them...) we do have to make sure these wl18xx bindings are future-compatible with the wl12xx ones, but i think the current bindings are pretty much standard (and are actually a subset of the bindings needed by wl12xx), so it should be fine. Well how about add just the parsing of the clock and assume it's always WL12XX_REFCLOCK_38 for now as that's the only thing we're currently using. Then we can add a property or compatible value if using something else as needed. We have two exceptions: static void __init omap3_zoom_legacy_init(void) { legacy_init_wl12xx(WL12XX_REFCLOCK_26, 0, 162); } static void __init omap4_sdp_legacy_init(void) { legacy_init_wl12xx(WL12XX_REFCLOCK_26, WL12XX_TCXOCLOCK_26, 53); } Where do those clocks come from? If these are always fixed-rate clocks coming from an external clock provider, using a separate compatible string in the clock provider would seem reasonable to me, but we can do that once we have to: none of the machines we support use an XTAL clock input. Arnd -- 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 v4 6/6] power: twl4030_madc_battery: Add missing MODULE_ALIAS
Without MODULE_ALIAS twl4030_madc_battery won't get loaded automatically. Signed-off-by: Marek Belisko ma...@goldelico.com --- drivers/power/twl4030_madc_battery.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/power/twl4030_madc_battery.c b/drivers/power/twl4030_madc_battery.c index 3e0005d..d0753f1 100644 --- a/drivers/power/twl4030_madc_battery.c +++ b/drivers/power/twl4030_madc_battery.c @@ -355,3 +355,4 @@ module_platform_driver(twl4030_madc_battery_driver); MODULE_LICENSE(GPL); MODULE_AUTHOR(Lukas Märdian lu...@goldelico.com); MODULE_DESCRIPTION(twl4030_madc battery driver); +MODULE_ALIAS(platform:twl4030_madc_battery); -- 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 v4 5/6] power: twl4030_madc_battery: Add of_twl4030_madc_match to MODULE_DEVICE_TABLE
When twl_4030_madc_battery is build as module, MODULE_DEVICE_TABLE allow the module to be auto-loaded since the module will match the devices instantiated from device tree. Signed-off-by: Marek Belisko ma...@goldelico.com --- drivers/power/twl4030_madc_battery.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/power/twl4030_madc_battery.c b/drivers/power/twl4030_madc_battery.c index 4bcb4a9..3e0005d 100644 --- a/drivers/power/twl4030_madc_battery.c +++ b/drivers/power/twl4030_madc_battery.c @@ -252,6 +252,8 @@ static const struct of_device_id of_twl4030_madc_match[] = { {}, }; +MODULE_DEVICE_TABLE(of, of_twl4030_madc_match); + #else static struct twl4030_madc_bat_platform_data * twl4030_madc_dt_probe(struct platform_device *pdev) -- 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 v4 2/6] power: twl4030_madc_battery: Add device tree support
Signed-off-by: Marek Belisko ma...@goldelico.com --- drivers/power/twl4030_madc_battery.c | 81 1 file changed, 81 insertions(+) diff --git a/drivers/power/twl4030_madc_battery.c b/drivers/power/twl4030_madc_battery.c index 6af28b5..4bcb4a9 100644 --- a/drivers/power/twl4030_madc_battery.c +++ b/drivers/power/twl4030_madc_battery.c @@ -20,6 +20,7 @@ #include linux/i2c/twl4030-madc.h #include linux/power/twl4030_madc_battery.h #include linux/iio/consumer.h +#include linux/of.h struct twl4030_madc_battery { struct power_supply psy; @@ -183,6 +184,82 @@ static int twl4030_cmp(const void *a, const void *b) ((struct twl4030_madc_bat_calibration *)a)-voltage; } +#ifdef CONFIG_OF +static struct twl4030_madc_bat_platform_data * + twl4030_madc_dt_probe(struct platform_device *pdev) +{ + struct twl4030_madc_bat_platform_data *pdata; + struct device_node *np = pdev-dev.of_node; + int ret; + int i, proplen; + + pdata = devm_kzalloc(pdev-dev, + sizeof(struct twl4030_madc_bat_platform_data), + GFP_KERNEL); + if (!pdata) + return ERR_PTR(-ENOMEM); + + ret = of_property_read_u32(np, capacity-uah, pdata-capacity); + if (ret != 0) + return ERR_PTR(-EINVAL); + + /* parse and prepare charging data */ + proplen = of_property_count_u32_elems(np, ti,volt-to-capacity-charging-map); + if (proplen 0) { + dev_warn(pdev-dev, No 'ti,volt-to-capacity-charging-map' property found\n); + return ERR_PTR(-EINVAL); + } + + pdata-charging = devm_kzalloc(pdev-dev, + sizeof(struct twl4030_madc_bat_calibration) * (proplen / 2), + GFP_KERNEL); + + for (i = 0; i proplen / 2; i++) { + of_property_read_u32_index(np, ti,volt-to-capacity-charging-map, + i * 2, + (u32 *)pdata-charging[i].voltage); + of_property_read_u32_index(np, ti,volt-to-capacity-charging-map, + i * 2 + 1, + (u32 *)pdata-charging[i].level); + } + + /* parse and prepare discharging data */ + proplen = of_property_count_u32_elems(np, + ti,volt-to-capacity-discharging-map); + if (proplen 0) { + dev_warn(pdev-dev, No 'ti,volt-to-capacity-discharging-map' property found\n); + return ERR_PTR(-EINVAL); + } + + pdata-discharging = devm_kzalloc(pdev-dev, + sizeof(struct twl4030_madc_bat_calibration) * (proplen / 2), + GFP_KERNEL); + + for (i = 0; i proplen / 2; i++) { + of_property_read_u32_index(np, ti,volt-to-capacity-discharging-map, + i * 2, + (u32 *)pdata-discharging[i].voltage); + of_property_read_u32_index(np, ti,volt-to-capacity-discharging-map, + i * 2 + 1, + (u32 *)pdata-discharging[i].level); + } + + return pdata; +} + +static const struct of_device_id of_twl4030_madc_match[] = { + { .compatible = ti,twl4030-madc-battery, }, + {}, +}; + +#else +static struct twl4030_madc_bat_platform_data * + twl4030_madc_dt_probe(struct platform_device *pdev) +{ + return ERR_PTR(-ENODEV); +} +#endif + static int twl4030_madc_battery_probe(struct platform_device *pdev) { struct twl4030_madc_battery *twl4030_madc_bat; @@ -194,6 +271,9 @@ static int twl4030_madc_battery_probe(struct platform_device *pdev) if (!twl4030_madc_bat) return -ENOMEM; + if (!pdata) + pdata = twl4030_madc_dt_probe(pdev); + twl4030_madc_bat-psy.name = twl4030_battery; twl4030_madc_bat-psy.type = POWER_SUPPLY_TYPE_BATTERY; twl4030_madc_bat-psy.properties = twl4030_madc_bat_props; @@ -263,6 +343,7 @@ static int twl4030_madc_battery_remove(struct platform_device *pdev) static struct platform_driver twl4030_madc_battery_driver = { .driver = { .name = twl4030_madc_battery, + .of_match_table = of_match_ptr(of_twl4030_madc_match), }, .probe = twl4030_madc_battery_probe, .remove = twl4030_madc_battery_remove, -- 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 v4 1/6] power: twl4030-madc-battery: Convert to iio consumer.
Because of added iio error handling private data allocation was converted to managed to simplify code. Signed-off-by: Marek Belisko ma...@goldelico.com Reviewed-By: Sebastian Reichel s...@debian.org --- drivers/power/twl4030_madc_battery.c | 99 +++- 1 file changed, 64 insertions(+), 35 deletions(-) diff --git a/drivers/power/twl4030_madc_battery.c b/drivers/power/twl4030_madc_battery.c index 7ef445a..6af28b5 100644 --- a/drivers/power/twl4030_madc_battery.c +++ b/drivers/power/twl4030_madc_battery.c @@ -19,10 +19,14 @@ #include linux/sort.h #include linux/i2c/twl4030-madc.h #include linux/power/twl4030_madc_battery.h +#include linux/iio/consumer.h struct twl4030_madc_battery { struct power_supply psy; struct twl4030_madc_bat_platform_data *pdata; + struct iio_channel *channel_temp; + struct iio_channel *channel_ichg; + struct iio_channel *channel_vbat; }; static enum power_supply_property twl4030_madc_bat_props[] = { @@ -38,43 +42,34 @@ static enum power_supply_property twl4030_madc_bat_props[] = { POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW, }; -static int madc_read(int index) +static int madc_read(struct iio_channel *channel) { - struct twl4030_madc_request req; - int val; + int val, err; + err = iio_read_channel_processed(channel, val); + if (err 0) + return err; - req.channels = index; - req.method = TWL4030_MADC_SW2; - req.type = TWL4030_MADC_WAIT; - req.do_avg = 0; - req.raw = false; - req.func_cb = NULL; - - val = twl4030_madc_conversion(req); - if (val 0) - return val; - - return req.rbuf[ffs(index) - 1]; + return val; } -static int twl4030_madc_bat_get_charging_status(void) +static int twl4030_madc_bat_get_charging_status(struct twl4030_madc_battery *bt) { - return (madc_read(TWL4030_MADC_ICHG) 0) ? 1 : 0; + return (madc_read(bt-channel_ichg) 0) ? 1 : 0; } -static int twl4030_madc_bat_get_voltage(void) +static int twl4030_madc_bat_get_voltage(struct twl4030_madc_battery *bt) { - return madc_read(TWL4030_MADC_VBAT); + return madc_read(bt-channel_vbat); } -static int twl4030_madc_bat_get_current(void) +static int twl4030_madc_bat_get_current(struct twl4030_madc_battery *bt) { - return madc_read(TWL4030_MADC_ICHG) * 1000; + return madc_read(bt-channel_ichg) * 1000; } -static int twl4030_madc_bat_get_temp(void) +static int twl4030_madc_bat_get_temp(struct twl4030_madc_battery *bt) { - return madc_read(TWL4030_MADC_BTEMP) * 10; + return madc_read(bt-channel_temp) * 10; } static int twl4030_madc_bat_voltscale(struct twl4030_madc_battery *bat, @@ -84,7 +79,7 @@ static int twl4030_madc_bat_voltscale(struct twl4030_madc_battery *bat, int i, res = 0; /* choose charging curve */ - if (twl4030_madc_bat_get_charging_status()) + if (twl4030_madc_bat_get_charging_status(bat)) calibration = bat-pdata-charging; else calibration = bat-pdata-discharging; @@ -119,23 +114,23 @@ static int twl4030_madc_bat_get_property(struct power_supply *psy, switch (psp) { case POWER_SUPPLY_PROP_STATUS: if (twl4030_madc_bat_voltscale(bat, - twl4030_madc_bat_get_voltage()) 95) + twl4030_madc_bat_get_voltage(bat)) 95) val-intval = POWER_SUPPLY_STATUS_FULL; else { - if (twl4030_madc_bat_get_charging_status()) + if (twl4030_madc_bat_get_charging_status(bat)) val-intval = POWER_SUPPLY_STATUS_CHARGING; else val-intval = POWER_SUPPLY_STATUS_DISCHARGING; } break; case POWER_SUPPLY_PROP_VOLTAGE_NOW: - val-intval = twl4030_madc_bat_get_voltage() * 1000; + val-intval = twl4030_madc_bat_get_voltage(bat) * 1000; break; case POWER_SUPPLY_PROP_TECHNOLOGY: val-intval = POWER_SUPPLY_TECHNOLOGY_LION; break; case POWER_SUPPLY_PROP_CURRENT_NOW: - val-intval = twl4030_madc_bat_get_current(); + val-intval = twl4030_madc_bat_get_current(bat); break; case POWER_SUPPLY_PROP_PRESENT: /* assume battery is always present */ @@ -143,23 +138,23 @@ static int twl4030_madc_bat_get_property(struct power_supply *psy, break; case POWER_SUPPLY_PROP_CHARGE_NOW: { int percent = twl4030_madc_bat_voltscale(bat, - twl4030_madc_bat_get_voltage()); + twl4030_madc_bat_get_voltage(bat)); val-intval = (percent *
[PATCH v4 3/6] Documentation: DT: Document twl4030-madc-battery bindings
Signed-off-by: Marek Belisko ma...@goldelico.com --- .../bindings/power_supply/twl4030_madc_battery.txt | 43 ++ 1 file changed, 43 insertions(+) create mode 100644 Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt diff --git a/Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt b/Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt new file mode 100644 index 000..d3dd9d8 --- /dev/null +++ b/Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt @@ -0,0 +1,43 @@ +twl4030_madc_battery + +Required properties: + - compatible : ti,twl4030-madc-battery + - capacity-uah : battery capacity in uAh + - ti,volt-to-capacity-charging-map : list of voltage(mV):level(%) values + for charging calibration (see example) + - ti,volt-to-capacity-discharging-map : list of voltage(mV):level(%) values + for discharging calibration (see example) + - io-channels: Should contain IIO channel specifiers + for each element in io-channel-names. +- io-channel-names: Should contain the following values: + * temp - The ADC channel for temperature reading + * ichg - The ADC channel for battery charging status + * vbat - The ADC channel to measure the battery voltage + +Example: + madc-battery { + compatible = ti,twl4030-madc-battery; + capacity-uah = 120; + ti,volt-to-capacity-charging-map = 4200 100, + 4100 75, + 4000 55, + 3900 25, + 3800 5, + 3700 2, + 3600 1, + 3300 0; + + ti,volt-to-capacity-discharging-map = 4200 100 + 4100 95, + 4000 70, + 3800 50, + 3700 10, + 3600 5, + 3300 0; + io-channels = twl_madc 1, + twl_madc 10, + twl_madc 12; + io-channel-names = temp, + ichg, + vbat; + }; -- 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 v4 4/6] ARM: dts: omap3-gta04: Add battery support
Added battery support for gta04 devices. Signed-off-by: Marek Belisko ma...@goldelico.com --- arch/arm/boot/dts/omap3-gta04.dtsi | 30 ++ 1 file changed, 30 insertions(+) diff --git a/arch/arm/boot/dts/omap3-gta04.dtsi b/arch/arm/boot/dts/omap3-gta04.dtsi index fb3a696..cbf515a 100644 --- a/arch/arm/boot/dts/omap3-gta04.dtsi +++ b/arch/arm/boot/dts/omap3-gta04.dtsi @@ -49,6 +49,32 @@ ti,codec = twl_audio; }; + battery { + compatible = ti,twl4030-madc-battery; + capacity-uah = 120; + ti,volt-to-capacity-charging-map = 4200 100, +4100 75, +4000 55, +3900 25, +3800 5, +3700 2, +3600 1, +3300 0; + ti,volt-to-capacity-discharging-map = 4200 100, + 4100 95, + 4000 70, + 3800 50, + 3700 10, + 3600 5, + 3300 0; + io-channels = twl_madc 1, + twl_madc 10, + twl_madc 12; + io-channel-names = temp, + ichg, + vbat; + }; + spi_lcd { compatible = spi-gpio; #address-cells = 0x1; @@ -518,3 +544,7 @@ mcbsp2 { status = okay; }; + +twl_madc { + ti,system-uses-second-madc-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 v4 0/6] Convert twl4030_madc_battery to IIO consumer and add DT aupport
This patches adding support for twl4030_madc_battery to use twl4030_madc iio framework + DT support. Patches was tested on gta04 board. twl4030_madc_battery driver is converted in first patch to iio consumer and in next patches is added support for devicetree + some small fixes for module autoloading. Changes from V3: - added ti prefix for volt-to-capacity-discharging/charging-map properties Changes from V2: - fix property names - add MODULE_DEVICE_TABLE and MODULE_ALIAS Changes from V1: - use iio_read_channel_processed instead iio_read_channel_processed - convert probe to use devm_ as part of adding error handling for iio - free iio channels on error and on module removal [1] - https://lkml.org/lkml/2014/2/26/482dd Marek Belisko (6): power: twl4030-madc-battery: Convert to iio consumer. power: twl4030_madc_battery: Add device tree support Documentation: DT: Document twl4030-madc-battery bindings ARM: dts: omap3-gta04: Add battery support power: twl4030_madc_battery: Add of_twl4030_madc_match to MODULE_DEVICE_TABLE power: twl4030_madc_battery: Add missing MODULE_ALIAS .../bindings/power_supply/twl4030_madc_battery.txt | 43 + arch/arm/boot/dts/omap3-gta04.dtsi | 30 drivers/power/twl4030_madc_battery.c | 183 + 3 files changed, 221 insertions(+), 35 deletions(-) create mode 100644 Documentation/devicetree/bindings/power_supply/twl4030_madc_battery.txt -- 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: RGB LED control (was Re: advanced LED controllers)
On Mon 2015-03-09 11:50:56, Måns Rullgård wrote: Geert Uytterhoeven ge...@linux-m68k.org writes: Hi Pavel, On Sun, Mar 8, 2015 at 9:57 PM, Pavel Machek pa...@ucw.cz wrote: Ok, so I played with RGB LED a bit, and we have quite a gap in documentation: what 50% brightness means is non-trivial and very important in case we want to do smooth blinking and color transitions. Signed-off-by: Pavel Machek pa...@ucw.cz diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led index 3646ec8..649d7a6 100644 --- a/Documentation/ABI/testing/sysfs-class-led +++ b/Documentation/ABI/testing/sysfs-class-led @@ -8,6 +8,11 @@ Description: non-zero brightness settings. The value is between 0 and /sys/class/leds/led/max_brightness. + If LED supports continuous brightness settings, 50% brightness + should correspond to 50% brightness perceived by human, in a similar + manner pixel brightness on monitor does (not 50% PWM). How many drivers do it right? How many don't? For those that don't, perhaps we handle the conversion between perceived and pwm in the core, e.g. by adding a new flag to led_classdev.flags to indicate the need for conversion? Some LED controllers do the right thing in hardware, so any adjustment done in the core needs to be optional. Do you have example controller that gets it right, btw? Bryan, can you apply the documentation patch so we can start fixing the drivers? Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.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 v5 3/3] ARM: dts: igep00x0: add wl18xx bindings
Hello Eliad, On Mon, Mar 9, 2015 at 4:36 PM, Eliad Peller el...@wizery.com wrote: Add wl18xx (wilink8) bindings to omap3-igep0030-rev-g and omap3-igep0020-rev-f dts files, instead of defining the platform data through the pdata-quirks. The patch was compile-tested only. You should look at MAINTAINERS or use ./scripts/get_maintainer.pl to know who should be cc'ed. Specially for this kind of patches where you are not able to test on the actual platform. Added Enric to cc as well since he also maintains the IGEP boards an has access to the hw too. Signed-off-by: Eliad Peller el...@wizery.com --- arch/arm/boot/dts/omap3-igep0020-rev-f.dts | 9 + arch/arm/boot/dts/omap3-igep0030-rev-g.dts | 9 + arch/arm/mach-omap2/pdata-quirks.c | 2 -- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts index cc8bd0c..8e5b44e 100644 --- a/arch/arm/boot/dts/omap3-igep0020-rev-f.dts +++ b/arch/arm/boot/dts/omap3-igep0020-rev-f.dts @@ -42,4 +42,13 @@ vmmc-supply = lbep5clwmc_wlen; bus-width = 4; non-removable; + + #address-cells = 1; + #size-cells = 0; + wlcore: wlcore@2 { + compatible = ti,wl1835; + reg = 2; + interrupt-parent = gpio6; + interrupts = 17 IRQ_TYPE_NONE; As Arnd said, it seems this should be IRQ_TYPE_LEVEL_HIGH to match what the platform code is currently doing. Best regards, Javier -- 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 2/3] wl18xx: add basic device-tree support
Hello Eliad, On Mon, Mar 9, 2015 at 4:36 PM, Eliad Peller el...@wizery.com wrote: When running with device-tree, we no longer have a board file that can set up the platform data for wlcore. Allow this data to be passed from DT. For now, parse only the irq used. Other (optional) properties can be added later on. Signed-off-by: Ido Yariv i...@wizery.com Signed-off-by: Eliad Peller el...@wizery.com --- I see this is a v5 but I don't know what was changed from prior revisions. It would be good if the patches had a versions history. drivers/net/wireless/ti/wlcore/sdio.c | 80 --- 1 file changed, 74 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/ti/wlcore/sdio.c b/drivers/net/wireless/ti/wlcore/sdio.c index d3dd7bf..ee556ac 100644 --- a/drivers/net/wireless/ti/wlcore/sdio.c +++ b/drivers/net/wireless/ti/wlcore/sdio.c @@ -34,6 +34,8 @@ #include linux/wl12xx.h #include linux/pm_runtime.h #include linux/printk.h +#include linux/of.h +#include linux/of_irq.h #include wlcore.h #include wl12xx_80211.h @@ -214,6 +216,69 @@ static struct wl1271_if_operations sdio_ops = { .set_block_size = wl1271_sdio_set_block_size, }; +#ifdef CONFIG_OF +static const struct of_device_id wlcore_sdio_of_match_table[] = { + { .compatible = ti,wl1801 }, + { .compatible = ti,wl1805 }, + { .compatible = ti,wl1807 }, + { .compatible = ti,wl1831 }, + { .compatible = ti,wl1835 }, + { .compatible = ti,wl1837 }, + { } +}; + +static struct wl12xx_platform_data *wlcore_probe_of(struct device *dev) +{ + struct device_node *np = dev-of_node; + struct wl12xx_platform_data *pdata; + + if (!np || !of_match_node(wlcore_sdio_of_match_table, np)) + return NULL; + + pdata = kzalloc(sizeof(*pdata), GFP_KERNEL); + if (!pdata) + return NULL; + + pdata-irq = irq_of_parse_and_map(np, 0); + if (!pdata-irq) { + dev_err(dev, No irq in platform data\n); + kfree(pdata); + return NULL; + } + + return pdata; +} +#else +static struct wl12xx_platform_data *wlcore_probe_of(struct device *dev) +{ + return NULL; +} +#endif + +static struct wl12xx_platform_data * +wlcore_get_platform_data(struct device *dev) +{ + struct wl12xx_platform_data *pdata; + + /* first, look for DT data */ I thought it was the opposite, that platform data should over-rule DT. That way you can still use the data filled in arch/arm/mach-omap2/pdata-quirks.c even after the driver supports your new DT binding. + pdata = wlcore_probe_of(dev); + if (pdata) + return pdata; + + /* if not found - fallback to static platform data */ + pdata = wl12xx_get_platform_data(); + if (!IS_ERR(pdata)) + return kmemdup(pdata, sizeof(*pdata), GFP_KERNEL); + + dev_err(dev, No platform data set\n); + return NULL; +} + +static void wlcore_del_platform_data(struct wl12xx_platform_data *pdata) +{ + kfree(pdata); +} + This function seems to be an unnecessary, why not just call kfree() directly? Or better, maybe the resource-managed devm_*() functions can be used so the data doesn't have to be explicitly freed? Best regards, Javier -- 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