[PATCH 2/2] clk: change clk_ops' -determine_rate() prototype
Clock rates are stored in an unsigned long field, but -determine_rate() (which returns a rounded rate from a requested one) returns a long value (errors are reported using negative error codes), which can lead to long overflow if the clock rate exceed 2Ghz. Change -determine_rate() prototype to return 0 or an error code, and pass the requested rate as a pointer so that it can be adjusted depending on hardware capabilities. Signed-off-by: Boris Brezillon boris.brezil...@free-electrons.com --- CC: Jonathan Corbet cor...@lwn.net CC: Tony Lindgren t...@atomide.com CC: Ralf Baechle r...@linux-mips.org CC: Emilio López emi...@elopez.com.ar CC: Maxime Ripard maxime.rip...@free-electrons.com CC: Tero Kristo t-kri...@ti.com CC: linux-...@vger.kernel.org CC: linux-ker...@vger.kernel.org CC: linux-arm-ker...@lists.infradead.org CC: linux-omap@vger.kernel.org CC: linux-m...@linux-mips.org Documentation/clk.txt | 4 +-- arch/arm/mach-omap2/dpll3xxx.c | 20 +-- arch/arm/mach-omap2/dpll44xx.c | 20 +-- arch/mips/alchemy/common/clock.c| 63 +--- drivers/clk/at91/clk-programmable.c | 28 +-- drivers/clk/bcm/clk-kona.c | 24 - drivers/clk/clk-composite.c | 22 ++-- drivers/clk/clk.c | 72 + drivers/clk/hisilicon/clk-hi3620.c | 22 ++-- drivers/clk/mmp/clk-mix.c | 17 - drivers/clk/qcom/clk-pll.c | 12 --- drivers/clk/qcom/clk-rcg.c | 32 + drivers/clk/qcom/clk-rcg2.c | 55 +++- drivers/clk/sunxi/clk-factors.c | 19 +- drivers/clk/sunxi/clk-sun6i-ar100.c | 18 ++ drivers/clk/sunxi/clk-sunxi.c | 19 +- include/linux/clk-provider.h| 24 ++--- include/linux/clk/ti.h | 24 ++--- 18 files changed, 278 insertions(+), 217 deletions(-) diff --git a/Documentation/clk.txt b/Documentation/clk.txt index fca8b7a..f5cae13 100644 --- a/Documentation/clk.txt +++ b/Documentation/clk.txt @@ -71,8 +71,8 @@ the operations defined in clk.h: int (*round_rate)(struct clk_hw *hw, unsigned long *rate, unsigned long *parent_rate); - long(*determine_rate)(struct clk_hw *hw, - unsigned long rate, + int (*determine_rate)(struct clk_hw *hw, + unsigned long *rate, unsigned long min_rate, unsigned long max_rate, unsigned long *best_parent_rate, diff --git a/arch/arm/mach-omap2/dpll3xxx.c b/arch/arm/mach-omap2/dpll3xxx.c index 7a6fb45..cfa24fd 100644 --- a/arch/arm/mach-omap2/dpll3xxx.c +++ b/arch/arm/mach-omap2/dpll3xxx.c @@ -472,37 +472,37 @@ void omap3_noncore_dpll_disable(struct clk_hw *hw) * Returns a positive clock rate with success, negative error value * in failure. */ -long omap3_noncore_dpll_determine_rate(struct clk_hw *hw, unsigned long rate, - unsigned long min_rate, - unsigned long max_rate, - unsigned long *best_parent_rate, - struct clk_hw **best_parent_clk) +int omap3_noncore_dpll_determine_rate(struct clk_hw *hw, unsigned long *rate, + unsigned long min_rate, + unsigned long max_rate, + unsigned long *best_parent_rate, + struct clk_hw **best_parent_clk) { struct clk_hw_omap *clk = to_clk_hw_omap(hw); struct dpll_data *dd; int ret; - if (!hw || !rate) + if (!hw || !*rate) return -EINVAL; dd = clk-dpll_data; if (!dd) return -EINVAL; - if (__clk_get_rate(dd-clk_bypass) == rate + if (__clk_get_rate(dd-clk_bypass) == *rate (dd-modes (1 DPLL_LOW_POWER_BYPASS))) { *best_parent_clk = __clk_get_hw(dd-clk_bypass); } else { - ret = omap2_dpll_round_rate(hw, rate, best_parent_rate); + ret = omap2_dpll_round_rate(hw, rate, best_parent_rate); if (ret) return ret; *best_parent_clk = __clk_get_hw(dd-clk_ref); } - *best_parent_rate = rate; + *best_parent_rate = *rate; - return rate; + return 0; } /** diff --git a/arch/arm/mach-omap2/dpll44xx.c b/arch/arm/mach-omap2/dpll44xx.c index afd3284..1571d41 100644 --- a/arch/arm/mach-omap2/dpll44xx.c +++
Re: [RFC][PATCH v2 08/13] usb: otg: hub: Notify OTG fsm when A device sets b_hnp_enable
On 17/04/15 05:28, Peter Chen wrote: On Tue, Apr 14, 2015 at 01:41:55PM +0300, Roger Quadros wrote: This is the a_set_b_hnp_enable flag in the OTG state machine diagram and must be set when the A-Host has successfully set the b_hnp_enable feature of the OTG-B-Peripheral attached to it. When this bit changes we kick our OTG FSM to make note of the change and act accordingly. Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/core/hub.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c index d7c3d5a..ab0d498 100644 --- a/drivers/usb/core/hub.c +++ b/drivers/usb/core/hub.c @@ -22,6 +22,7 @@ #include linux/usb/hcd.h #include linux/usb/otg.h #include linux/usb/quirks.h +#include linux/usb/usb-otg.h #include linux/workqueue.h #include linux/mutex.h #include linux/random.h @@ -2270,6 +2271,9 @@ static int usb_enumerate_device_otg(struct usb_device *udev) can't set HNP mode: %d\n, err); bus-b_hnp_enable = 0; +} else { +/* notify OTG fsm about a_set_b_hnp_enable */ +usb_otg_kick_fsm(udev-bus-controller); } } } @@ -4244,8 +4248,13 @@ hub_port_init (struct usb_hub *hub, struct usb_device *udev, int port1, */ if (!hdev-parent) { delay = HUB_ROOT_RESET_TIME; -if (port1 == hdev-bus-otg_port) +if (port1 == hdev-bus-otg_port) { hdev-bus-b_hnp_enable = 0; +#ifdef CONFIG_USB_OTG +/* notify OTG fsm about a_set_b_hnp_enable change */ +usb_otg_kick_fsm(hdev-bus-controller); +#endif +} } /* Some low speed devices have problems with the quick delay, so */ -- 2.1.0 Since the fsm has already kicked, the only thing we need is update fsm.a_set_b_hnp_enable, but this flag is missing at current fsm structure. Every time the input changes, the fsm has to be re-kicked. Instead of fsm.a_set_b_hnp_enable we use fsm-otg-host-b_hnp_enable in the fsm. USB host stack doesn't have access to fsm structure but the OTG state machine has access to hcd. cheers, -roger -- 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: AM335x OMAP2 common clock external fixed-clock registration
On 04/17/2015 05:00 AM, Michael Welling wrote: On Fri, Apr 17, 2015 at 01:23:50AM +0200, Sebastian Hesselbarth wrote: On 17.04.2015 00:09, Michael Welling wrote: On Thu, Apr 16, 2015 at 10:37:19PM +0200, Sebastian Hesselbarth wrote: On 16.04.2015 18:17, Michael Welling wrote: On Thu, Apr 16, 2015 at 07:32:32AM +0300, Tero Kristo wrote: On 04/15/2015 11:51 PM, Michael Welling wrote: On Wed, Apr 15, 2015 at 01:45:53PM -0700, Mike Turquette wrote: On Wed, Apr 15, 2015 at 12:47 PM, Michael Welling mwell...@ieee.org wrote: [...] There is still an issue with the si5351. I had to comment out the clk_put here for the frequency to show up: http://lxr.free-electrons.com/source/drivers/clk/clk-si5351.c#L1133 Ideas? What is the most recent upstream commit that you are based on? I am working from 4.0.0-rc7. 7b43b47373d40d557cd7e1a84a0bd8ebc4d745ab Hmm, I wonder why si5351 calls clk_put immediately after of_clk_get in the first place, as far as I understand this destroys the clock handle, which is still being used later in the code. Not sure how this ever worked. This has been in the code since the initial commit. The reason it worked before may be related with recent rework of clk_put() itself and clk cookies instead of pointers. I lost track on the recent clk subsystem changes here, sorry. However, droping the clk immediately surely isn't right. The thing is, we can remove the clk_put() just because there is no _remove() for that driver. I remember that back in the days the driver was mainlined, clk removal wasn't too easy. FWIW, as soon as _remove() support will be added by someone, we'll have to rethink passing struct clk* by platform_data or at least double-check if we ever used [of_]clk_get() to obtain it. Mind to send a patch removing the clk_put() on !IS_ERR and add a proper error path instead? While of_clk_get() is the only calls that need cleanup on error in si5351_dt_parse() we should probably move that calls to the end of this function. Otherwise we'd also have to cleanup on every of_parse_foo() failure. What would be the proper error path? What cleanup is required? A proper error path would be to release any claimed resource on any error. If you look at the code, the only resources that need to be released are the two clocks in question. So for every error return in the probe function and in the of si5351_dt_parse it needs to clk_put first right? See attached patch to see if we are on the same page. It should be noted that there are more deep rooted issues with the driver that I have noticed. For one the driver behaves differently if the debugging is on and when it is off. I guess you mean #define DEBUG in the driver? Yes. Here is what the kernel reports with debugging off: Do you have any measurement equipment to check what is actually set? Yes, I have an oscilloscope here at my desk. The reported numbers do not always correspond to the actual output in some cases. The ms2 output has appeared to stop working all together sometime whilest testing. I may have to solder a new chip on there. Could misconfiguration damage the chip? root@som3517-som200:~# cat /sys/kernel/debug/clk/clk_summary clock enable_cnt prepare_cntrate accuracy phase ref27002700 0 0 xtal 002700 0 0 pllb 00 59994 0 0 ms0 001249 0 0 clk0 001249 0 0 plla 00 59994 0 0 ms2 00 8219178 0 0 clk2 00 8219178 0 0 ms1 0094117646 0 0 clk1 0094117646 0 0 Here is what the kernel reports with debugging on: clock enable_cnt prepare_cntrate accuracy phase ref27002700 0 0 xtal 002700 0 0 pllb 00 884736000 0 0 ms0 0018432000 0 0 clk0 0018432000 0 0 Is this what you expect for clk0? Yes. plla 00 897023997 0
Re: [RFC][PATCH v2 06/13] usb: hcd: Add hcd add/remove functions for OTG use
On 17/04/15 05:18, Peter Chen wrote: On Tue, Apr 14, 2015 at 01:41:53PM +0300, Roger Quadros wrote: The existing usb_add/remove_hcd() functionality remains unchanged for non-OTG devices. For OTG devices they only register the HCD with the OTG core. Introduce _usb_add/remove_hcd() for use by OTG core. These functions actually add/remove the HCD. Would you please explain why additional _usb_add/remove_hcd are needed? It is to differentiate if the add/remove_hcd was called by the HCD drivers or by the OTG core as we want to behave a bit differently in both cases. When called by HCD drivers, we want to defer the add/remove if it is an OTG device. When called by OTG core, we don't want to defer the add/remove. HCD drivers use usb_add/remove_hcd() OTG core uses _usb_add/remove_hcd() cheers, -roger Peter Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/common/usb-otg.c | 14 +++--- drivers/usb/core/hcd.c | 24 ++-- include/linux/usb/hcd.h | 3 +++ 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/drivers/usb/common/usb-otg.c b/drivers/usb/common/usb-otg.c index e848e08..860e2e7 100644 --- a/drivers/usb/common/usb-otg.c +++ b/drivers/usb/common/usb-otg.c @@ -198,18 +198,18 @@ static int usb_otg_start_host(struct otg_fsm *fsm, int on) otgd-start_host(fsm, on); /* start host */ -usb_add_hcd(otgd-primary_hcd.hcd, otgd-primary_hcd.irqnum, -otgd-primary_hcd.irqflags); +_usb_add_hcd(otgd-primary_hcd.hcd, otgd-primary_hcd.irqnum, + otgd-primary_hcd.irqflags); if (otgd-shared_hcd.hcd) { -usb_add_hcd(otgd-shared_hcd.hcd, -otgd-shared_hcd.irqnum, -otgd-shared_hcd.irqflags); +_usb_add_hcd(otgd-shared_hcd.hcd, + otgd-shared_hcd.irqnum, + otgd-shared_hcd.irqflags); } } else { /* stop host */ if (otgd-shared_hcd.hcd) -usb_remove_hcd(otgd-shared_hcd.hcd); -usb_remove_hcd(otgd-primary_hcd.hcd); +_usb_remove_hcd(otgd-shared_hcd.hcd); +_usb_remove_hcd(otgd-primary_hcd.hcd); /* OTG device operations */ if (otgd-start_host) diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 45a915c..9a9c0f7 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -46,6 +46,7 @@ #include linux/usb.h #include linux/usb/hcd.h #include linux/usb/phy.h +#include linux/usb/usb-otg.h #include usb.h @@ -2622,7 +2623,7 @@ static void usb_put_invalidate_rhdev(struct usb_hcd *hcd) * buffers of consistent memory, register the bus, request the IRQ line, * and call the driver's reset() and start() routines. */ -int usb_add_hcd(struct usb_hcd *hcd, +int _usb_add_hcd(struct usb_hcd *hcd, unsigned int irqnum, unsigned long irqflags) { int retval; @@ -2827,6 +2828,17 @@ err_phy: } return retval; } +EXPORT_SYMBOL_GPL(_usb_add_hcd); + +int usb_add_hcd(struct usb_hcd *hcd, +unsigned int irqnum, unsigned long irqflags) +{ +/* If OTG device, OTG core takes care of adding HCD */ +if (usb_otg_register_hcd(hcd, irqnum, irqflags)) +return _usb_add_hcd(hcd, irqnum, irqflags); + +return 0; +} EXPORT_SYMBOL_GPL(usb_add_hcd); /** @@ -2837,7 +2849,7 @@ EXPORT_SYMBOL_GPL(usb_add_hcd); * Disconnects the root hub, then reverses the effects of usb_add_hcd(), * invoking the HCD's stop() method. */ -void usb_remove_hcd(struct usb_hcd *hcd) +void _usb_remove_hcd(struct usb_hcd *hcd) { struct usb_device *rhdev = hcd-self.root_hub; @@ -2911,6 +2923,14 @@ void usb_remove_hcd(struct usb_hcd *hcd) usb_put_invalidate_rhdev(hcd); } +EXPORT_SYMBOL_GPL(_usb_remove_hcd); + +void usb_remove_hcd(struct usb_hcd *hcd) +{ +/* If OTG device, OTG core takes care of stopping HCD */ +if (usb_otg_unregister_hcd(hcd)) +_usb_remove_hcd(hcd); +} EXPORT_SYMBOL_GPL(usb_remove_hcd); void diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h index 68b1e83..7993ae7 100644 --- a/include/linux/usb/hcd.h +++ b/include/linux/usb/hcd.h @@ -433,7 +433,10 @@ extern void usb_put_hcd(struct usb_hcd *hcd); extern int usb_hcd_is_primary_hcd(struct usb_hcd *hcd); extern int usb_add_hcd(struct usb_hcd *hcd, unsigned int irqnum, unsigned long irqflags); +extern int _usb_add_hcd(struct usb_hcd *hcd, +unsigned int irqnum, unsigned long irqflags); extern void usb_remove_hcd(struct usb_hcd *hcd); +extern void _usb_remove_hcd(struct usb_hcd *hcd); extern int usb_hcd_find_raw_port_number(struct usb_hcd *hcd, int port1); struct
Re: AM335x OMAP2 common clock external fixed-clock registration
On 17.04.2015 04:00, Michael Welling wrote: On Fri, Apr 17, 2015 at 01:23:50AM +0200, Sebastian Hesselbarth wrote: On 17.04.2015 00:09, Michael Welling wrote: On Thu, Apr 16, 2015 at 10:37:19PM +0200, Sebastian Hesselbarth wrote: On 16.04.2015 18:17, Michael Welling wrote: [...] What would be the proper error path? What cleanup is required? A proper error path would be to release any claimed resource on any error. If you look at the code, the only resources that need to be released are the two clocks in question. So for every error return in the probe function and in the of si5351_dt_parse it needs to clk_put first right? Not quite. The driver should clk_put() every clock that it called a [of_]clk_get() for. The thing is that clocks can be passed by platform_data and we never claim them. See attached patch to see if we are on the same page. Adding a .remove() function needs more care because of the pdata passed clocks. I admit that back when the driver was introduced clk_put() wasn't really necessary at all. [...] Here is what the kernel reports with debugging off: Do you have any measurement equipment to check what is actually set? Yes, I have an oscilloscope here at my desk. The reported numbers do not always correspond to the actual output in some cases. is not always correspond close or way off the requested frequency? Stability is an issue that I am aware of. Neither the datasheet nor the appnote were clear about any order the clocks should be set or how long we should wait between changing pll/ms/clkout parameters. SiLabs suggests to configure all clocks at once and never change them later, at least that is what you can read from the public documents. The drivers you mention below can however reveal some steps that have to be taken care of before and after changing parameters. The ms2 output has appeared to stop working all together sometime whilest testing. I may have to solder a new chip on there. Could misconfiguration damage the chip? You should know that a lot of things can damage a chip and misconfiguration is among them, yes. I cannot tell if that is the cause though. [...] Is there any way to try out a less recent kernel, let's say two or three releases before 4.0? Could you provide a specific version that you think has the best chances of working? My guess with 2-3 releases before 4.0 was because somewhere in between clk API must have switched from passing struct clk pointers to clk cookies. [...] From your patch (that you should inline next time please): @@ -1129,11 +1130,21 @@ static int si5351_dt_parse(struct i2c_client *client, return -ENOMEM; pdata-clk_xtal = of_clk_get(np, 0); - if (!IS_ERR(pdata-clk_xtal)) - clk_put(pdata-clk_xtal); - pdata-clk_clkin = of_clk_get(np, 1); - if (!IS_ERR(pdata-clk_clkin)) - clk_put(pdata-clk_clkin); + if (IS_ERR(pdata-clk_xtal)) { + dev_err(client-dev, + xtal clock not speficied\n); + return -ENODEV; + } + + if (variant == SI5351_VARIANT_C) { + pdata-clk_clkin = of_clk_get(np, 1); + if (IS_ERR(pdata-clk_clkin)) { + dev_err(client-dev, + clkin clock not speficied\n); + ret = -ENODEV; + goto err_put_clk_of; + } + } Basically, yes. But as I said, if you move that to the end of si5351_dt_parse() you won't have to add @ -1143,14 +1154,16 @@ static int si5351_dt_parse(struct i2c_client *client, if (num = 2) { dev_err(client-dev, invalid pll %d on pll-source prop\n, num); - return -EINVAL; + ret = -EINVAL; + goto err_put_clk_of; } this to each of the sanity checks. Claiming the clock resources at the end saves you from tearing down the resources on each error above. Another thing is that you now require VARIANT_C devices to always pass both, xtal and clkin. I can live with it until we find somebody who actually uses C-type devices with only one clock input connected. Adding a .remove() function to si5351 definitely needs more care with respect to claimed and pdata passed clocks. I am not sure we should go for it before we haven't ruled out the other issues. Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH v2 01/13] usb: otg-fsm: Add documentation for struct otg_fsm
On 16/04/15 14:32, Peter Chen wrote: On Tue, Apr 14, 2015 at 01:41:48PM +0300, Roger Quadros wrote: struct otg_fsm is the interface to the OTG state machine. Document the input, output and internal state variables. Definations are taken from Table 7-2 and Table 7-4 of the USB OTG EH Specification Rev.2.0 Re-arrange some of the members as per use case for more clarity. Signed-off-by: Roger Quadros rog...@ti.com --- include/linux/usb/otg-fsm.h | 80 + 1 file changed, 73 insertions(+), 7 deletions(-) diff --git a/include/linux/usb/otg-fsm.h b/include/linux/usb/otg-fsm.h index b6ba1bf..c5b74c5 100644 --- a/include/linux/usb/otg-fsm.h +++ b/include/linux/usb/otg-fsm.h @@ -57,37 +57,103 @@ enum otg_fsm_timer { NUM_OTG_FSM_TIMERS, }; -/* OTG state machine according to the OTG spec */ +/** + * struct otg_fsm - OTG state machine according to the OTG spec + * + * OTG hardware Inputs + * + * Common inputs for A and B device What's the intention you leave two spaces at above line? It is just to indicate the subgroup. i.e. 1) common inputs 2) a-device inputs 3) b-device inputs, etc + * @id: TRUE for B-device, FALSE for A-device. + * @adp_change: TRUE when current ADP measurement (n) value, compared to the + * ADP measurement taken at n-2, differs by more than CADP_THR + * @power_up: TRUE when the OTG device first powers up its USB system and + * ADP measurement taken if ADP capable + * + * A-Device state inputs + * @a_srp_det: TRUE if the A-device detects SRP + * @a_vbus_vld: TRUE when VBUS voltage is in regulation + * @b_conn: TRUE if the A-device detects connection from the B-device + * @a_bus_resume: TRUE when the B-device detects that the A-device is signaling + *a resume (K state) + * B-Device state inputs + * @a_bus_suspend: TRUE when the B-device detects that the A-device has put the bus into suspend + * @a_conn: TRUE if the B-device detects a connection from the A-device + * @b_se0_srp: TRUE when the line has been at SE0 for more than the minimum + * time before generating SRP + * @b_ssend_srp: TRUE when the VBUS has been below VOTG_SESS_VLD for more than + * the minimum time before generating SRP + * @b_sess_vld: TRUE when the B-device detects that the voltage on VBUS is + * above VOTG_SESS_VLD + * @test_device: TRUE when the B-device switches to B-Host and detects an OTG test device + * FIXME: must be set by host/hub driver Yes, according to pid/vid pair. + * + * Application inputs (A-Device) + * @a_bus_drop: TRUE when A-device application needs to power down the bus + * @a_bus_req: TRUE when A-device application wants to use the bus. + * FALSE to suspend the bus + * + * Application inputs (B-Device) + * @b_bus_req: TRUE during the time that the Application running on the + * B-device wants to use the bus + * + * Auxilary inputs + * @a_sess_vld: ?? See OTG spec 1.3, and obsolete now. OK. I will mark it obsolete. + * @b_bus_suspend: ?? + * @b_bus_resume: ?? They are used at current otg fsm, just for B-host. These 2 are also present only in otg v1.3 spec and absent in v2.0 spec. b_bus_suspend has been replaced by a_bidl_adis_tmout to transition from a_peripheral to a_wait_bcon. b_bus_resume has been replaced by a_bus_req to transition between a_suspend and a_host. So I'll mark them as obsolete as well. + * + * OTG Output status. Read only for users. updated by otg_ops() helpers + * + * Outputs for Both A and B device + * @drv_vbus: TRUE when A-device is driving VBUS + * @loc_conn: TRUE when the local device has signaled that it is connected to the bus + * @loc_sof:TRUE when the local device is generating activity on the bus + * @adp_prb:TRUE when the local device is in the process of doing ADP probing + * + * Outputs for B-device state + * @adp_sns:TRUE when the B-device is in the process of carrying out ADP sensing + * @data_pulse: TRUE when the B-device is performing data line pulsing + * + * Internal Variables + * + * a_set_b_hnp_en: TRUE when the A-device has successfully set the b_hnp_enable bit in the B-device. + * FIXME: OTG fsm uses otg-host-b_hnp_enable instead a_set_b_hnp_en is for otg 2.0 and it should instead of otg-host-b_hnp_enable at otg fsm. both are the same thing. Just that there is no way for HCD drivers to directly access otg_fsm data structure so we rely on otg-host-b_hnp_enable. + * b_srp_done: TRUE when the B-device has completed initiating SRP + * b_hnp_enable: TRUE when the B-device has accepted the SetFeature(b_hnp_enable) B-device + * FIXME: OTG fsm uses otg-gadget-b_hnp_enable instead Yes + * a_clr_err: Asserted (by application ?) to clear
Patch dmaengine: omap-dma: Fix memory leak when terminating running transfer has been added to the 3.10-stable tree
This is a note to let you know that I've just added the patch titled dmaengine: omap-dma: Fix memory leak when terminating running transfer to the 3.10-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: dmaengine-omap-dma-fix-memory-leak-when-terminating-running-transfer.patch and it can be found in the queue-3.10 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let sta...@vger.kernel.org know about it. From 02d88b735f5a60f04dbf6d051b76e1877a0d0844 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi peter.ujfal...@ti.com Date: Fri, 27 Mar 2015 13:35:52 +0200 Subject: dmaengine: omap-dma: Fix memory leak when terminating running transfer From: Peter Ujfalusi peter.ujfal...@ti.com commit 02d88b735f5a60f04dbf6d051b76e1877a0d0844 upstream. In omap_dma_start_desc the vdesc-node is removed from the virt-dma framework managed lists (to be precise from the desc_issued list). If a terminate_all comes before the transfer finishes the omap_desc will not be freed up because it is not in any of the lists and we stopped the DMA channel so the transfer will not going to complete. There is no special sequence for leaking memory when using cyclic (audio) transfer: with every start and stop of a cyclic transfer the driver leaks struct omap_desc worth of memory. Free up the allocated memory directly in omap_dma_terminate_all() since the framework will not going to do that for us. Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com CC: linux-omap@vger.kernel.org Signed-off-by: Vinod Koul vinod.k...@intel.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org --- drivers/dma/omap-dma.c |1 + 1 file changed, 1 insertion(+) --- a/drivers/dma/omap-dma.c +++ b/drivers/dma/omap-dma.c @@ -487,6 +487,7 @@ static int omap_dma_terminate_all(struct * c-desc is NULL and exit.) */ if (c-desc) { + omap_dma_desc_free(c-desc-vd); c-desc = NULL; /* Avoid stopping the dma twice */ if (!c-paused) Patches currently in stable-queue which might be from peter.ujfal...@ti.com are queue-3.10/dmaengine-omap-dma-fix-memory-leak-when-terminating-running-transfer.patch -- 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 dmaengine: omap-dma: Fix memory leak when terminating running transfer has been added to the 3.14-stable tree
This is a note to let you know that I've just added the patch titled dmaengine: omap-dma: Fix memory leak when terminating running transfer to the 3.14-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: dmaengine-omap-dma-fix-memory-leak-when-terminating-running-transfer.patch and it can be found in the queue-3.14 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let sta...@vger.kernel.org know about it. From 02d88b735f5a60f04dbf6d051b76e1877a0d0844 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi peter.ujfal...@ti.com Date: Fri, 27 Mar 2015 13:35:52 +0200 Subject: dmaengine: omap-dma: Fix memory leak when terminating running transfer From: Peter Ujfalusi peter.ujfal...@ti.com commit 02d88b735f5a60f04dbf6d051b76e1877a0d0844 upstream. In omap_dma_start_desc the vdesc-node is removed from the virt-dma framework managed lists (to be precise from the desc_issued list). If a terminate_all comes before the transfer finishes the omap_desc will not be freed up because it is not in any of the lists and we stopped the DMA channel so the transfer will not going to complete. There is no special sequence for leaking memory when using cyclic (audio) transfer: with every start and stop of a cyclic transfer the driver leaks struct omap_desc worth of memory. Free up the allocated memory directly in omap_dma_terminate_all() since the framework will not going to do that for us. Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com CC: linux-omap@vger.kernel.org Signed-off-by: Vinod Koul vinod.k...@intel.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org --- drivers/dma/omap-dma.c |1 + 1 file changed, 1 insertion(+) --- a/drivers/dma/omap-dma.c +++ b/drivers/dma/omap-dma.c @@ -487,6 +487,7 @@ static int omap_dma_terminate_all(struct * c-desc is NULL and exit.) */ if (c-desc) { + omap_dma_desc_free(c-desc-vd); c-desc = NULL; /* Avoid stopping the dma twice */ if (!c-paused) Patches currently in stable-queue which might be from peter.ujfal...@ti.com are queue-3.14/dmaengine-omap-dma-fix-memory-leak-when-terminating-running-transfer.patch -- 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 dmaengine: omap-dma: Fix memory leak when terminating running transfer has been added to the 3.19-stable tree
This is a note to let you know that I've just added the patch titled dmaengine: omap-dma: Fix memory leak when terminating running transfer to the 3.19-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: dmaengine-omap-dma-fix-memory-leak-when-terminating-running-transfer.patch and it can be found in the queue-3.19 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let sta...@vger.kernel.org know about it. From 02d88b735f5a60f04dbf6d051b76e1877a0d0844 Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi peter.ujfal...@ti.com Date: Fri, 27 Mar 2015 13:35:52 +0200 Subject: dmaengine: omap-dma: Fix memory leak when terminating running transfer From: Peter Ujfalusi peter.ujfal...@ti.com commit 02d88b735f5a60f04dbf6d051b76e1877a0d0844 upstream. In omap_dma_start_desc the vdesc-node is removed from the virt-dma framework managed lists (to be precise from the desc_issued list). If a terminate_all comes before the transfer finishes the omap_desc will not be freed up because it is not in any of the lists and we stopped the DMA channel so the transfer will not going to complete. There is no special sequence for leaking memory when using cyclic (audio) transfer: with every start and stop of a cyclic transfer the driver leaks struct omap_desc worth of memory. Free up the allocated memory directly in omap_dma_terminate_all() since the framework will not going to do that for us. Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com CC: linux-omap@vger.kernel.org Signed-off-by: Vinod Koul vinod.k...@intel.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org --- drivers/dma/omap-dma.c |1 + 1 file changed, 1 insertion(+) --- a/drivers/dma/omap-dma.c +++ b/drivers/dma/omap-dma.c @@ -978,6 +978,7 @@ static int omap_dma_terminate_all(struct * c-desc is NULL and exit.) */ if (c-desc) { + omap_dma_desc_free(c-desc-vd); c-desc = NULL; /* Avoid stopping the dma twice */ if (!c-paused) Patches currently in stable-queue which might be from peter.ujfal...@ti.com are queue-3.19/dmaengine-edma-fix-memory-leak-when-terminating-running-transfers.patch queue-3.19/dmaengine-omap-dma-fix-memory-leak-when-terminating-running-transfer.patch -- 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 dmaengine: edma: fix memory leak when terminating running transfers has been added to the 3.19-stable tree
This is a note to let you know that I've just added the patch titled dmaengine: edma: fix memory leak when terminating running transfers to the 3.19-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: dmaengine-edma-fix-memory-leak-when-terminating-running-transfers.patch and it can be found in the queue-3.19 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let sta...@vger.kernel.org know about it. From 5ca9e7ce6eebec53362ff779264143860ccf68cd Mon Sep 17 00:00:00 2001 From: Petr Kulhavy p...@barix.com Date: Fri, 27 Mar 2015 13:35:51 +0200 Subject: dmaengine: edma: fix memory leak when terminating running transfers From: Petr Kulhavy p...@barix.com commit 5ca9e7ce6eebec53362ff779264143860ccf68cd upstream. If edma_terminate_all() was called while a transfer was running (i.e. after edma_execute() but before edma_callback()) the echan-edesc was not freed. This was due to the fact that a running transfer is on none of the vchan lists: desc_submitted, desc_issued, desc_completed (edma_execute() removes it from the desc_issued list), so the vchan_dma_desc_free_list() called at the end of edma_terminate_all() didn't find it and didn't free it. This bug was found on an AM1808 based hardware (very similar to da850evm, however using the second MMC/SD controller), where intense operations on the SD card wasted the device 128MB RAM within a couple of days. Peter Ujfalusi: The issue is even more severe since it affects cyclic (audio) transfers as well. In this case starting/stopping audio will results memory leak. Signed-off-by: Petr Kulhavy p...@barix.com Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com CC: linux-omap@vger.kernel.org Signed-off-by: Vinod Koul vinod.k...@intel.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org --- drivers/dma/edma.c |7 +++ 1 file changed, 7 insertions(+) --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -258,6 +258,13 @@ static int edma_terminate_all(struct edm */ if (echan-edesc) { int cyclic = echan-edesc-cyclic; + + /* +* free the running request descriptor +* since it is not in any of the vdesc lists +*/ + edma_desc_free(echan-edesc-vdesc); + echan-edesc = NULL; edma_stop(echan-ch_num); /* Move the cyclic channel back to default queue */ Patches currently in stable-queue which might be from p...@barix.com are queue-3.19/dmaengine-edma-fix-memory-leak-when-terminating-running-transfers.patch -- 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.19 062/101] dmaengine: edma: fix memory leak when terminating running transfers
3.19-stable review patch. If anyone has any objections, please let me know. -- From: Petr Kulhavy p...@barix.com commit 5ca9e7ce6eebec53362ff779264143860ccf68cd upstream. If edma_terminate_all() was called while a transfer was running (i.e. after edma_execute() but before edma_callback()) the echan-edesc was not freed. This was due to the fact that a running transfer is on none of the vchan lists: desc_submitted, desc_issued, desc_completed (edma_execute() removes it from the desc_issued list), so the vchan_dma_desc_free_list() called at the end of edma_terminate_all() didn't find it and didn't free it. This bug was found on an AM1808 based hardware (very similar to da850evm, however using the second MMC/SD controller), where intense operations on the SD card wasted the device 128MB RAM within a couple of days. Peter Ujfalusi: The issue is even more severe since it affects cyclic (audio) transfers as well. In this case starting/stopping audio will results memory leak. Signed-off-by: Petr Kulhavy p...@barix.com Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com CC: linux-omap@vger.kernel.org Signed-off-by: Vinod Koul vinod.k...@intel.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org --- drivers/dma/edma.c |7 +++ 1 file changed, 7 insertions(+) --- a/drivers/dma/edma.c +++ b/drivers/dma/edma.c @@ -258,6 +258,13 @@ static int edma_terminate_all(struct edm */ if (echan-edesc) { int cyclic = echan-edesc-cyclic; + + /* +* free the running request descriptor +* since it is not in any of the vdesc lists +*/ + edma_desc_free(echan-edesc-vdesc); + echan-edesc = NULL; edma_stop(echan-ch_num); /* Move the cyclic channel back to default queue */ -- 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.19 063/101] dmaengine: omap-dma: Fix memory leak when terminating running transfer
3.19-stable review patch. If anyone has any objections, please let me know. -- From: Peter Ujfalusi peter.ujfal...@ti.com commit 02d88b735f5a60f04dbf6d051b76e1877a0d0844 upstream. In omap_dma_start_desc the vdesc-node is removed from the virt-dma framework managed lists (to be precise from the desc_issued list). If a terminate_all comes before the transfer finishes the omap_desc will not be freed up because it is not in any of the lists and we stopped the DMA channel so the transfer will not going to complete. There is no special sequence for leaking memory when using cyclic (audio) transfer: with every start and stop of a cyclic transfer the driver leaks struct omap_desc worth of memory. Free up the allocated memory directly in omap_dma_terminate_all() since the framework will not going to do that for us. Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com CC: linux-omap@vger.kernel.org Signed-off-by: Vinod Koul vinod.k...@intel.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org --- drivers/dma/omap-dma.c |1 + 1 file changed, 1 insertion(+) --- a/drivers/dma/omap-dma.c +++ b/drivers/dma/omap-dma.c @@ -978,6 +978,7 @@ static int omap_dma_terminate_all(struct * c-desc is NULL and exit.) */ if (c-desc) { + omap_dma_desc_free(c-desc-vd); c-desc = NULL; /* Avoid stopping the dma twice */ if (!c-paused) -- 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.10 19/34] dmaengine: omap-dma: Fix memory leak when terminating running transfer
3.10-stable review patch. If anyone has any objections, please let me know. -- From: Peter Ujfalusi peter.ujfal...@ti.com commit 02d88b735f5a60f04dbf6d051b76e1877a0d0844 upstream. In omap_dma_start_desc the vdesc-node is removed from the virt-dma framework managed lists (to be precise from the desc_issued list). If a terminate_all comes before the transfer finishes the omap_desc will not be freed up because it is not in any of the lists and we stopped the DMA channel so the transfer will not going to complete. There is no special sequence for leaking memory when using cyclic (audio) transfer: with every start and stop of a cyclic transfer the driver leaks struct omap_desc worth of memory. Free up the allocated memory directly in omap_dma_terminate_all() since the framework will not going to do that for us. Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com CC: linux-omap@vger.kernel.org Signed-off-by: Vinod Koul vinod.k...@intel.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org --- drivers/dma/omap-dma.c |1 + 1 file changed, 1 insertion(+) --- a/drivers/dma/omap-dma.c +++ b/drivers/dma/omap-dma.c @@ -487,6 +487,7 @@ static int omap_dma_terminate_all(struct * c-desc is NULL and exit.) */ if (c-desc) { + omap_dma_desc_free(c-desc-vd); c-desc = NULL; /* Avoid stopping the dma twice */ if (!c-paused) -- 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: AM335x OMAP2 common clock external fixed-clock registration
On Fri, Apr 17, 2015 at 11:12:03AM +0200, Sebastian Hesselbarth wrote: On 17.04.2015 04:00, Michael Welling wrote: On Fri, Apr 17, 2015 at 01:23:50AM +0200, Sebastian Hesselbarth wrote: On 17.04.2015 00:09, Michael Welling wrote: On Thu, Apr 16, 2015 at 10:37:19PM +0200, Sebastian Hesselbarth wrote: On 16.04.2015 18:17, Michael Welling wrote: [...] What would be the proper error path? What cleanup is required? A proper error path would be to release any claimed resource on any error. If you look at the code, the only resources that need to be released are the two clocks in question. So for every error return in the probe function and in the of si5351_dt_parse it needs to clk_put first right? Not quite. The driver should clk_put() every clock that it called a [of_]clk_get() for. The thing is that clocks can be passed by platform_data and we never claim them. I've always said clocks (as in struct clk) should never be passed through platform data. -- 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: [RFC][PATCH v2 06/13] usb: hcd: Add hcd add/remove functions for OTG use
On Fri, 17 Apr 2015, Roger Quadros wrote: On 17/04/15 05:18, Peter Chen wrote: On Tue, Apr 14, 2015 at 01:41:53PM +0300, Roger Quadros wrote: The existing usb_add/remove_hcd() functionality remains unchanged for non-OTG devices. For OTG devices they only register the HCD with the OTG core. Introduce _usb_add/remove_hcd() for use by OTG core. These functions actually add/remove the HCD. Would you please explain why additional _usb_add/remove_hcd are needed? It is to differentiate if the add/remove_hcd was called by the HCD drivers or by the OTG core as we want to behave a bit differently in both cases. When called by HCD drivers, we want to defer the add/remove if it is an OTG device. When called by OTG core, we don't want to defer the add/remove. I don't understand this. Why do you want to defer the add/remove if the device is OTG? Don't host controller drivers expect these things to execute synchronously? For example, what happens if you rmmod the HCD? If the remove call gets deferred, then when it finally runs it will try to call back into HCD code that has already been unloaded from memory! HCD drivers use usb_add/remove_hcd() OTG core uses _usb_add/remove_hcd() How about a more explicit naming scheme? HC drivers use usb_add/remove_hcd() OTG core uses usb_otg_add/remove_hcd() Alan Stern -- 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.14 24/43] dmaengine: omap-dma: Fix memory leak when terminating running transfer
3.14-stable review patch. If anyone has any objections, please let me know. -- From: Peter Ujfalusi peter.ujfal...@ti.com commit 02d88b735f5a60f04dbf6d051b76e1877a0d0844 upstream. In omap_dma_start_desc the vdesc-node is removed from the virt-dma framework managed lists (to be precise from the desc_issued list). If a terminate_all comes before the transfer finishes the omap_desc will not be freed up because it is not in any of the lists and we stopped the DMA channel so the transfer will not going to complete. There is no special sequence for leaking memory when using cyclic (audio) transfer: with every start and stop of a cyclic transfer the driver leaks struct omap_desc worth of memory. Free up the allocated memory directly in omap_dma_terminate_all() since the framework will not going to do that for us. Signed-off-by: Peter Ujfalusi peter.ujfal...@ti.com CC: linux-omap@vger.kernel.org Signed-off-by: Vinod Koul vinod.k...@intel.com Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org --- drivers/dma/omap-dma.c |1 + 1 file changed, 1 insertion(+) --- a/drivers/dma/omap-dma.c +++ b/drivers/dma/omap-dma.c @@ -487,6 +487,7 @@ static int omap_dma_terminate_all(struct * c-desc is NULL and exit.) */ if (c-desc) { + omap_dma_desc_free(c-desc-vd); c-desc = NULL; /* Avoid stopping the dma twice */ if (!c-paused) -- 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] omap: i2c: Add calls for pinctrl state select
From: Pascal Huerst pascal.hue...@gmail.com This adds calls to pinctrl subsystem in order to switch pin states on suspend/resume if you provide a sleep state in DT. Signed-off-by: Pascal Huerst pascal.hue...@gmail.com --- drivers/i2c/busses/i2c-omap.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 0e89419..6b5d4bd 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1423,6 +1423,8 @@ static int omap_i2c_runtime_suspend(struct device *dev) omap_i2c_read_reg(_dev, OMAP_I2C_STAT_REG); } + pinctrl_pm_select_sleep_state(dev); + return 0; } @@ -1431,6 +1433,8 @@ static int omap_i2c_runtime_resume(struct device *dev) struct platform_device *pdev = to_platform_device(dev); struct omap_i2c_dev *_dev = platform_get_drvdata(pdev); + pinctrl_pm_select_default_state(dev); + if (!_dev-regs) return 0; -- 1.9.3 -- 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 errata 430973 on multi platform kernels
On Thu, Apr 16, 2015 at 09:08:58AM -0700, Tony Lindgren wrote: * Sebastian Reichel s...@kernel.org [150415 09:32]: Hi, On Thu, Apr 09, 2015 at 02:48:43PM +0100, Russell King - ARM Linux wrote: On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote: On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote: Works for me. The above needs the following fix folded in to build: --- a/arch/arm/mm/proc-v7.S +++ b/arch/arm/mm/proc-v7.S @@ -532,7 +532,7 @@ __v7_ca9mp_proc_info: __v7_ca8_proc_info: .long 0x410fc080 .long 0xff00 - __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions + __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions .size __v7_ca8_proc_info, . - __v7_ca8_proc_info #endif /* CONFIG_ARM_LPAE */ Thanks, merged into the original patch. Do you want to give me an ack for this, thanks? I tried to test this together with Tony's follow up patch, but I get this after applying the patch to v4.0: sre@earth ~/src/linux [430973-fix] % make -j4 CHK include/config/kernel.release CHK include/generated/uapi/linux/version.h CHK include/generated/utsrelease.h make[1]: 'include/generated/mach-types.h' is up to date. CALLscripts/checksyscalls.sh CHK include/generated/compile.h AS arch/arm/mm/proc-v7.o arch/arm/mm/proc-v7.S: Assembler messages: arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|' arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|' scripts/Makefile.build:294: recipe for target 'arch/arm/mm/proc-v7.o' failed make[1]: *** [arch/arm/mm/proc-v7.o] Error 1 Makefile:947: recipe for target 'arch/arm/mm' failed make: *** [arch/arm/mm] Error 2 make: *** Waiting for unfinished jobs Maybe test the version in Linux next: a6d746789825 (ARM: proc-v7: avoid errata 430973 workaround for non-Cortex A8 CPUs) DONE with your your patch added on top: Tested-By: Sebastian Reichel s...@kernel.org (on N900) I guess we should also drop the CONFIG_ARM_ERRATA_430973 check from pdata-quirks' nokia_n900_legacy_init() and just enable it unconditionally. -- Sebastian signature.asc Description: Digital signature
Re: [RFC/RFT PATCH 2/2] gpio: omap: ensure that runtime pm will disable unused gpio banks
Hi Tony, On 04/16/2015 07:29 PM, Tony Lindgren wrote: * Tony Lindgren t...@atomide.com [150319 16:08]: * Tony Lindgren t...@atomide.com [150307 00:08]: * grygorii.stras...@linaro.org grygorii.stras...@linaro.org [150306 11:27]: From: Grygorii Strashko grygorii.stras...@linaro.org Now there are two points related to Runtime PM usage: 1) bank state doesn't need to be checked in places where Rintime PM is used, bacause Runtime PM maintains its own usage counter: if (!BANK_USED(bank)) pm_runtime_get_sync(bank-dev); so, it's safe to drop such checks. 2) There is a call of pm_runtime_get_sync() in omap_gpio_irq_type(), but no corresponding put. As result, GPIO devices could be powered on forever if at least one GPIO was used as IRQ. Hence, allow powering off GPIO banks by adding missed pm_runtime_put(bank-dev) at the end of omap_gpio_irq_type(). Nice to see this happening, but I think before merging this we need to test to be sure that the pm_runtime calls actually match.. I'm not convinced right now.. We may still have uninitialized entry points similar to 3d009c8c61f9 (gpio: omap: Fix bad device access with setup_irq()). OK so I finally got around testing this along with your bank removal set. Looks like this patch causes a regression at least with n900 keyboard LEDs with off-idle. The LED won't come back on after restore from off-idle. Anyways, now we have something reproducable :) So I'll try to debug it further at some point, might be few days before I get to it. Sorry for the delay, finally got around to this one :) Here's what I came up with, only lightly tested so far. Note that we need to keep the PM runtime per bank, not per GPIO. Will repost a proper patch after some more testing. I've had a opposite idea actually ;) - use Runtime PM on per-gpio basis as it maintains all needed counters inside and we can be sure that PM runtime callbacks will be called once per bank. Also, I've thought about moving the code from omap_enable_gpio_module() into Runtime PM callbacks. So, final goal - get rid of BANK_USED LINE_USED. Were you able to identify broken calls sequence? Also, Pay attention pls, that omap2_gpio_prepare_for/resume_after_idle() will be most probably a NOP now. This should not cause any functional changes, mostly just removal of code that can all be done in omap_enable/disable_gpio_module. 8 - --- a/drivers/gpio/gpio-omap.c +++ b/drivers/gpio/gpio-omap.c @@ -86,6 +86,7 @@ struct gpio_bank { #define BANK_USED(bank) (bank-mod_usage || bank-irq_usage) #define LINE_USED(line, offset) (line (BIT(offset))) +static void omap_reset_gpio(struct gpio_bank *bank, unsigned offset); static void omap_gpio_unmask_irq(struct irq_data *d); static inline struct gpio_bank *omap_irq_data_get_bank(struct irq_data *d) @@ -419,8 +420,16 @@ static int omap_set_gpio_triggering(struct gpio_bank *bank, int gpio, return 0; } -static void omap_enable_gpio_module(struct gpio_bank *bank, unsigned offset) +static void omap_enable_gpio_module(struct gpio_bank *bank, unsigned offset, + bool is_irq) { + unsigned long flags; + + /* PM runtime is per bank, not per GPIO line */ + if (!BANK_USED(bank)) + pm_runtime_get_sync(bank-dev); + + spin_lock_irqsave(bank-lock, flags); if (bank-regs-pinctrl) { void __iomem *reg = bank-base + bank-regs-pinctrl; @@ -438,11 +447,30 @@ static void omap_enable_gpio_module(struct gpio_bank *bank, unsigned offset) writel_relaxed(ctrl, reg); bank-context.ctrl = ctrl; } + + if (is_irq) { + omap_set_gpio_direction(bank, offset, 1); + bank-irq_usage |= BIT(offset); + } else { + omap_set_gpio_triggering(bank, offset, IRQ_TYPE_NONE); + bank-mod_usage |= BIT(offset); + } + spin_unlock_irqrestore(bank-lock, flags); } -static void omap_disable_gpio_module(struct gpio_bank *bank, unsigned offset) +static void omap_disable_gpio_module(struct gpio_bank *bank, unsigned offset, + bool is_irq) { void __iomem *base = bank-base; + unsigned long flags; + + spin_lock_irqsave(bank-lock, flags); + if (is_irq) + bank-irq_usage = ~(BIT(offset)); + else + bank-mod_usage = ~(BIT(offset)); + + omap_reset_gpio(bank, offset); if (bank-regs-wkup_en !LINE_USED(bank-mod_usage, offset) @@ -463,6 +491,11 @@ static void omap_disable_gpio_module(struct gpio_bank *bank, unsigned offset) writel_relaxed(ctrl, reg); bank-context.ctrl = ctrl; } + spin_unlock_irqrestore(bank-lock, flags); + + /* PM runtime is per bank, not per GPIO line */ + if (!BANK_USED(bank)) + pm_runtime_put(bank-dev); }
Re: AM335x OMAP2 common clock external fixed-clock registration
On Fri, Apr 17, 2015 at 11:12:03AM +0200, Sebastian Hesselbarth wrote: On 17.04.2015 04:00, Michael Welling wrote: On Fri, Apr 17, 2015 at 01:23:50AM +0200, Sebastian Hesselbarth wrote: On 17.04.2015 00:09, Michael Welling wrote: On Thu, Apr 16, 2015 at 10:37:19PM +0200, Sebastian Hesselbarth wrote: On 16.04.2015 18:17, Michael Welling wrote: [...] What would be the proper error path? What cleanup is required? A proper error path would be to release any claimed resource on any error. If you look at the code, the only resources that need to be released are the two clocks in question. So for every error return in the probe function and in the of si5351_dt_parse it needs to clk_put first right? Not quite. The driver should clk_put() every clock that it called a [of_]clk_get() for. The thing is that clocks can be passed by platform_data and we never claim them. Should the ones from the platform data be claimed? See attached patch to see if we are on the same page. Adding a .remove() function needs more care because of the pdata passed clocks. I admit that back when the driver was introduced clk_put() wasn't really necessary at all. Is it necessary now? [...] Here is what the kernel reports with debugging off: Do you have any measurement equipment to check what is actually set? Yes, I have an oscilloscope here at my desk. The reported numbers do not always correspond to the actual output in some cases. is not always correspond close or way off the requested frequency? Both. Stability is an issue that I am aware of. Neither the datasheet nor the appnote were clear about any order the clocks should be set or how long we should wait between changing pll/ms/clkout parameters. SiLabs suggests to configure all clocks at once and never change them later, at least that is what you can read from the public documents. The drivers you mention below can however reveal some steps that have to be taken care of before and after changing parameters. The drivers were not provided we wrote them and they do nothing but write the registers with the raw values provided from the SiLab clock generator software. The ms2 output has appeared to stop working all together sometime whilest testing. I may have to solder a new chip on there. Could misconfiguration damage the chip? You should know that a lot of things can damage a chip and misconfiguration is among them, yes. I cannot tell if that is the cause though. Well I did something to the chip. [...] Is there any way to try out a less recent kernel, let's say two or three releases before 4.0? Could you provide a specific version that you think has the best chances of working? My guess with 2-3 releases before 4.0 was because somewhere in between clk API must have switched from passing struct clk pointers to clk cookies. [...] From your patch (that you should inline next time please): @@ -1129,11 +1130,21 @@ static int si5351_dt_parse(struct i2c_client *client, return -ENOMEM; pdata-clk_xtal = of_clk_get(np, 0); - if (!IS_ERR(pdata-clk_xtal)) - clk_put(pdata-clk_xtal); - pdata-clk_clkin = of_clk_get(np, 1); - if (!IS_ERR(pdata-clk_clkin)) - clk_put(pdata-clk_clkin); + if (IS_ERR(pdata-clk_xtal)) { + dev_err(client-dev, + xtal clock not speficied\n); + return -ENODEV; + } + + if (variant == SI5351_VARIANT_C) { + pdata-clk_clkin = of_clk_get(np, 1); + if (IS_ERR(pdata-clk_clkin)) { + dev_err(client-dev, + clkin clock not speficied\n); + ret = -ENODEV; + goto err_put_clk_of; + } + } Basically, yes. But as I said, if you move that to the end of si5351_dt_parse() you won't have to add @ -1143,14 +1154,16 @@ static int si5351_dt_parse(struct i2c_client *client, if (num = 2) { dev_err(client-dev, invalid pll %d on pll-source prop\n, num); - return -EINVAL; + ret = -EINVAL; + goto err_put_clk_of; } this to each of the sanity checks. Claiming the clock resources at the end saves you from tearing down the resources on each error above. Sure I can move it down put it seems counterintuitive to parse parameters that rely on the clocks before they are determined to be specified. Another thing is that you now require VARIANT_C devices to always pass both, xtal and clkin. I can live with it until we find somebody who actually uses C-type devices with only one clock input connected. Okay I did not realize that the C variant could run with one or the other. Adding a .remove() function to si5351 definitely needs more care with respect to claimed and pdata passed clocks. I am not
Re: AM335x OMAP2 common clock external fixed-clock registration
On Fri, Apr 17, 2015 at 11:18:33AM +0100, Russell King - ARM Linux wrote: On Fri, Apr 17, 2015 at 11:12:03AM +0200, Sebastian Hesselbarth wrote: On 17.04.2015 04:00, Michael Welling wrote: On Fri, Apr 17, 2015 at 01:23:50AM +0200, Sebastian Hesselbarth wrote: On 17.04.2015 00:09, Michael Welling wrote: On Thu, Apr 16, 2015 at 10:37:19PM +0200, Sebastian Hesselbarth wrote: On 16.04.2015 18:17, Michael Welling wrote: [...] What would be the proper error path? What cleanup is required? A proper error path would be to release any claimed resource on any error. If you look at the code, the only resources that need to be released are the two clocks in question. So for every error return in the probe function and in the of si5351_dt_parse it needs to clk_put first right? Not quite. The driver should clk_put() every clock that it called a [of_]clk_get() for. The thing is that clocks can be passed by platform_data and we never claim them. I've always said clocks (as in struct clk) should never be passed through platform data. What is the alternative for systems that still use the old platform files? Hypothetically speaking of course. -- 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: AM335x OMAP2 common clock external fixed-clock registration
On Fri, Apr 17, 2015 at 02:06:23PM -0500, Michael Welling wrote: On Fri, Apr 17, 2015 at 11:18:33AM +0100, Russell King - ARM Linux wrote: On Fri, Apr 17, 2015 at 11:12:03AM +0200, Sebastian Hesselbarth wrote: On 17.04.2015 04:00, Michael Welling wrote: On Fri, Apr 17, 2015 at 01:23:50AM +0200, Sebastian Hesselbarth wrote: On 17.04.2015 00:09, Michael Welling wrote: On Thu, Apr 16, 2015 at 10:37:19PM +0200, Sebastian Hesselbarth wrote: On 16.04.2015 18:17, Michael Welling wrote: [...] What would be the proper error path? What cleanup is required? A proper error path would be to release any claimed resource on any error. If you look at the code, the only resources that need to be released are the two clocks in question. So for every error return in the probe function and in the of si5351_dt_parse it needs to clk_put first right? Not quite. The driver should clk_put() every clock that it called a [of_]clk_get() for. The thing is that clocks can be passed by platform_data and we never claim them. I've always said clocks (as in struct clk) should never be passed through platform data. What is the alternative for systems that still use the old platform files? clkdev, which has pre-existed DT. -- 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: AM335x OMAP2 common clock external fixed-clock registration
Quoting Russell King - ARM Linux (2015-04-17 03:18:33) On Fri, Apr 17, 2015 at 11:12:03AM +0200, Sebastian Hesselbarth wrote: On 17.04.2015 04:00, Michael Welling wrote: On Fri, Apr 17, 2015 at 01:23:50AM +0200, Sebastian Hesselbarth wrote: On 17.04.2015 00:09, Michael Welling wrote: On Thu, Apr 16, 2015 at 10:37:19PM +0200, Sebastian Hesselbarth wrote: On 16.04.2015 18:17, Michael Welling wrote: [...] What would be the proper error path? What cleanup is required? A proper error path would be to release any claimed resource on any error. If you look at the code, the only resources that need to be released are the two clocks in question. So for every error return in the probe function and in the of si5351_dt_parse it needs to clk_put first right? Not quite. The driver should clk_put() every clock that it called a [of_]clk_get() for. The thing is that clocks can be passed by platform_data and we never claim them. I've always said clocks (as in struct clk) should never be passed through platform data. +1 And for ccf clock drivers Stephen and I plan to change the behavior of clk_register() at some point so that it returns an error code and not a struct clk. This will make clk_dev the only way to get at a struct clk for users of the ccf implementation. Of course it is still possible to clk_get from some place and pass as platform_data, but every little bit helps. Regards, Mike -- 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-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/6] phy: twl4030-usb: add ABI documentation
On Thu 2015-04-16 18:03:04, NeilBrown wrote: From: NeilBrown ne...@suse.de This driver device one local attribute: vbus. Describe that in Documentation/ABI/testing/sysfs-platform/twl4030-usb. Signed-off-by: NeilBrown n...@brown.name --- .../ABI/testing/sysfs-platform-twl4030-usb |8 1 file changed, 8 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-platform-twl4030-usb diff --git a/Documentation/ABI/testing/sysfs-platform-twl4030-usb b/Documentation/ABI/testing/sysfs-platform-twl4030-usb new file mode 100644 index ..512c51be64ae --- /dev/null +++ b/Documentation/ABI/testing/sysfs-platform-twl4030-usb @@ -0,0 +1,8 @@ +What: /sys/bus/platform/devices/*twl4030-usb/vbus +Description: + Read-only status reporting if VBUS (approx 5V) + is being supplied by the USB bus. + + Possible values: on, off. Would bit be better to have values 0 and 1? Kernel usually does that for booleans... 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 4/6] phy: twl4030-usb: add ABI documentation
On Sat, 18 Apr 2015 00:14:36 +0200 Pavel Machek pa...@ucw.cz wrote: On Thu 2015-04-16 18:03:04, NeilBrown wrote: From: NeilBrown ne...@suse.de This driver device one local attribute: vbus. Describe that in Documentation/ABI/testing/sysfs-platform/twl4030-usb. Signed-off-by: NeilBrown n...@brown.name --- .../ABI/testing/sysfs-platform-twl4030-usb |8 1 file changed, 8 insertions(+) create mode 100644 Documentation/ABI/testing/sysfs-platform-twl4030-usb diff --git a/Documentation/ABI/testing/sysfs-platform-twl4030-usb b/Documentation/ABI/testing/sysfs-platform-twl4030-usb new file mode 100644 index ..512c51be64ae --- /dev/null +++ b/Documentation/ABI/testing/sysfs-platform-twl4030-usb @@ -0,0 +1,8 @@ +What: /sys/bus/platform/devices/*twl4030-usb/vbus +Description: + Read-only status reporting if VBUS (approx 5V) + is being supplied by the USB bus. + + Possible values: on, off. Would bit be better to have values 0 and 1? Kernel usually does that for booleans... 1/ The code already uses on and off, so changing would be an ABI breakage. 2/ No it doesn't. For modules params, the kernel uses Y and N git grep '? on : off' | wc find 172 matches. NeilBrown pgpMBx3gIbOQL.pgp Description: OpenPGP digital signature