Re: [PATCH v2 01/10] usb: dwc2/gadget: report disconnect event from 'end session' irq
Hello,
On 2014-10-25 03:23, Paul Zimmerman wrote:
From: Marek Szyprowski [mailto:m.szyprow...@samsung.com]
Sent: Monday, October 20, 2014 3:46 AM
This patch adds a call to s3c_hsotg_disconnect() from 'end session'
interrupt (GOTGINT_SES_END_DET) to correctly notify gadget subsystem
about unplugged usb cable. 'disconnected' interrupt (DISCONNINT) might
look a bit more suitable for this event, but it is asserted only in
host mode, so in device mode we need to use something else.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/usb/dwc2/gadget.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 7b5856fadd93..119c8a3effc2 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2279,6 +2279,12 @@ irq_retry:
dev_info(hsotg-dev, OTGInt: %08x\n, otgint);
writel(otgint, hsotg-regs + GOTGINT);
+
+ if (otgint GOTGINT_SES_END_DET) {
+ if (hsotg-gadget.speed != USB_SPEED_UNKNOWN)
+ s3c_hsotg_disconnect(hsotg);
+ hsotg-gadget.speed = USB_SPEED_UNKNOWN;
+ }
}
if (gintsts GINTSTS_SESSREQINT) {
Does this mean we can get rid of the call to s3c_hsotg_disconnect in
s3c_hsotg_process_control after a SET_ADDRESS is received? If not,
then s3c_hsotg_disconnect will be called twice, once here after the
disconnect, and once again after the reconnect and SET_ADDRESS.
Nope, we cannot get rid of that call, because it is needed got get gadget
operational when device has been re-enumerated. The simplest way to
reproduce
such case is to connect dwc2 to externally powered hub and then unplug cable
between hub and usb host. In such case the 'end session' interrupt is not
asserted, so the additional s3c_hsotg_disconnect() call before setting new
address is needed. I will rework this patch and add a code to ensures that
the gadget-disconnect() will be called only once.
Best regards
--
Marek Szyprowski, PhD
Samsung RD Institute Poland
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] regulator: max77686: Add external GPIO control
On pią, 2014-10-31 at 12:31 +0900, Alexandre Courbot wrote:
On Fri, Oct 31, 2014 at 12:03 AM, Krzysztof Kozlowski
k.kozlow...@samsung.com wrote:
On czw, 2014-10-30 at 22:56 +0900, Alexandre Courbot wrote:
Hi, and thanks for bringing this issue to us!
On Wed, Oct 29, 2014 at 7:49 PM, Javier Martinez Canillas
javier.marti...@collabora.co.uk wrote:
[adding Linus and Alexandre to the cc list]
Hello Krzysztof,
On 10/29/2014 11:42 AM, Krzysztof Kozlowski wrote:
On wto, 2014-10-28 at 13:11 +0100, Krzysztof Kozlowski wrote:
On wto, 2014-10-28 at 09:52 +0100, Krzysztof Kozlowski wrote:
On pon, 2014-10-27 at 21:03 +0100, Javier Martinez Canillas wrote:
Hello Krzysztof,
On 10/27/2014 04:03 PM, Krzysztof Kozlowski wrote:
@@ -85,6 +91,9 @@ struct max77686_data {
struct max77686_regulator_data *regulators;
int num_regulators;
+ /* Array of size num_regulators with GPIOs for external
control. */
+ int *ext_control_gpio;
+
The integer-based GPIO API is deprecated in favor of the
descriptor-based GPIO
interface (Documentation/gpio/consumer.txt). Could you please use
the later?
Sure, I can. Please have in mind that regulator core still accepts
old
GPIO so I will have to use desc_to_gpio(). That should work... and
should be future-ready.
It seems I was too hasty... I think usage of the new gpiod API implies
completely different bindings.
The gpiod_get() gets GPIO from a device level, not from given sub-node
pointer. This means that you cannot have DTS like this:
ldo21_reg: ldo21 {
regulator-compatible = LDO21;
regulator-name = VTF_2.8V;
regulator-min-microvolt = 280;
regulator-max-microvolt = 280;
ec-gpio = gpy2 0 0;
};
ldo22_reg: ldo22 {
regulator-compatible = LDO22;
regulator-name = VMEM_VDD_2.8V;
regulator-min-microvolt = 280;
regulator-max-microvolt = 280;
ec-gpio = gpk0 2 0;
};
I could put GPIOs in device node:
max77686_pmic@09 {
compatible = maxim,max77686;
interrupt-parent = gpx0;
interrupts = 7 0;
reg = 0x09;
#clock-cells = 1;
ldo21-gpio = gpy2 0 0;
ldo22-gpio = gpk0 2 0;
ldo21_reg: ldo21 {
regulator-compatible = LDO21;
regulator-name = VTF_2.8V;
regulator-min-microvolt = 280;
regulator-max-microvolt = 280;
};
ldo22_reg: ldo22 {
regulator-compatible = LDO22;
regulator-name = VMEM_VDD_2.8V;
regulator-min-microvolt = 280;
regulator-max-microvolt = 280;
};
This would work but I don't like it. The properties of a regulator are
above the node configuring that regulator.
Any ideas?
Continuing talking to myself... I found another problem - GPIO cannot be
requested more than once (-EBUSY). In case of this driver (and board:
Trats2) one GPIO is connected to regulators. The legacy GPIO API and
regulator core handle this.
With new GPIO API I would have to implement some additional steps in
such case...
So there are 2 issues:
1. Cannot put GPIO property in regulator node.
For this problem you will probably want to use the
dev(m)_get_named_gpiod_from_child() function from the following patch:
https://lkml.org/lkml/2014/10/6/529
It should reach -next soon now.
Thanks! Probably I would switch to top level gpios property anyway
(other reasons) but it would be valuable in some cases to specify them
per child node.
Mmm, but doesn't it make more sense to have them in the child nodes?
Yes, it makes more sense. Using old way of parsing regulators from DT it
was straightforward.
However new DT style parsing (regulator_of_get_init_data()) does the
basic parsing stuff and this removes a lot of code from driver. The
driver no longer parses all regulaotrs but the regulator core does it.
Unfortunately regulator core does not parse custom bindings (like such
GPIOs) so I would have to iterate once again through all regulators just
to find gpios property.
It is simpler just to do something like (5 regulators can be controlled
by GPIO):
max77686_pmic_dt_parse_gpio_control(struct platform_device *pdev, *gpio)
{
gpio[MAX77686_LDO20] = of_get_named_gpio(np, ldo20-gpios, 0);
gpio[MAX77686_LDO21] = of_get_named_gpio(np, ldo21-gpios, 0);
gpio[MAX77686_LDO22] = of_get_named_gpio(np, ldo22-gpios, 0);
gpio[MAX77686_BUCK8] = of_get_named_gpio(np, buck8-gpios, 0);
gpio[MAX77686_BUCK9] = of_get_named_gpio(np, buck9-gpios, 0);
}
Besides if the bindings of this driver have already been published,
I'm afraid you will have to maintain backward compability.
These are new bindings. Driver exists but I am adding new
[PATCH v3] usb: dwc2/gadget: report disconnect event from 'end session' irq
This patch adds a call to s3c_hsotg_disconnect() from 'end session'
interrupt (GOTGINT_SES_END_DET) to correctly notify gadget subsystem
about unplugged usb cable. 'disconnected' interrupt (DISCONNINT) might
look a bit more suitable for this event, but it is asserted only in
host mode, so in device mode we need to use something else.
Additional check has been added in s3c_hsotg_disconnect() function
to ensure that the event is reported only once after successful device
enumeration.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/usb/dwc2/core.h | 1 +
drivers/usb/dwc2/gadget.c | 10 ++
2 files changed, 11 insertions(+)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 55c90c53f2d6..b42df32e7737 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -212,6 +212,7 @@ struct s3c_hsotg {
struct usb_gadget gadget;
unsigned intsetup;
unsigned long last_rst;
+ unsigned intaddress;
struct s3c_hsotg_ep *eps;
};
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index fcd2bb55ccca..6304efba11aa 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1114,6 +1114,7 @@ static void s3c_hsotg_process_control(struct s3c_hsotg
*hsotg,
DCFG_DEVADDR_SHIFT) DCFG_DEVADDR_MASK;
writel(dcfg, hsotg-regs + DCFG);
+ hsotg-address = ctrl-wValue;
dev_info(hsotg-dev, new address %d\n, ctrl-wValue);
ret = s3c_hsotg_send_reply(hsotg, ep0, NULL, 0);
@@ -2031,6 +2032,10 @@ static void s3c_hsotg_disconnect(struct s3c_hsotg *hsotg)
{
unsigned ep;
+ if (!hsotg-address)
+ return;
+
+ hsotg-address = 0;
for (ep = 0; ep hsotg-num_of_eps; ep++)
kill_all_requests(hsotg, hsotg-eps[ep], -ESHUTDOWN, true);
@@ -2290,6 +2295,11 @@ irq_retry:
dev_info(hsotg-dev, OTGInt: %08x\n, otgint);
writel(otgint, hsotg-regs + GOTGINT);
+
+ if (otgint GOTGINT_SES_END_DET) {
+ s3c_hsotg_disconnect(hsotg);
+ hsotg-gadget.speed = USB_SPEED_UNKNOWN;
+ }
}
if (gintsts GINTSTS_SESSREQINT) {
--
1.9.2
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] PM / Domains: Fix race conditions during boot
On 24 October 2014 18:12, Kevin Hilman khil...@kernel.org wrote: Ulf Hansson ulf.hans...@linaro.org writes: Changes in v3: -Rework the entire intermediate step which was suggested in v2. That means solving the race condition, but also cope with PM domains that are initialized in powered off state. Changes in v2: -Added some acks. -Updated commit messages. -Included a wider audience of the patchset. V1 was lacking SoC maintainers. Here are link to the first patchset, were some discussion started. http://marc.info/?l=linux-pmm=141208104729597w=2 There may be more than one device in a PM domain which then will be probed at different points in time. Depending on timing and runtime PM support, in for the device related driver/subsystem, a PM domain may be advised to power off after a successful probe sequence. A general requirement for a device within a PM domain, is that the PM domain must stay powered during the probe sequence. To cope with such requirement, let's add two new APIs, dev_pm_domain_get|put(). These APIs are intended to be invoked from subsystem-level code and the calls between get/put needs to be balanced. dev_pm_domain_get(), tells the PM domain that it needs to increase a usage count and to keep supplying power. dev_pm_domain_put(), does the opposite. I'm confused. Why arent' pm_runtime_get*() and pm_runtime_put*() working? See, below. What's not explained here (or what I'm not understanding) is why a PM domain is powering off if it has active devices. It doesn't. The problem is that using pm_runtime_get_sync() in this path is not working. Now, I failed to include some of the important information from previous discussions around this patchset. Let me iterate the patchset with better commit messages, but let's first continue this thread. Here are some of the previous discussion: http://marc.info/?l=linux-pmm=141270897014653w=2 http://marc.info/?l=linux-pmm=141208104729597w=2 Below is a summary of why I think pm_runtime_get_sync() isn't working for us. 1) It's bad practice to use pm_runtime_get_sync() in the -probe() path, to bring your resources to full power. The consequence would be a driver that requires CONFIG_PM_RUNTIME to be even functional, which just isn't acceptable. Drivers that behaves well within this context, follows the runtime PM documentation/recommendation. They use pm_runtime_set_active() during -probe() to reflect that their devices are fully powered and capable of handling I/O. You may also have a look at these discussions which also touches this topic, but within a context of another patchset. https://lkml.org/lkml/2014/10/23/95 2) Another good example why pm_runtime_get_sync() is a bad solution to our problem, is the amba bus. Before it invokes the driver's -probe() callback it does the following. - enable bus clock - pm_runtime_get_noresume() - pm_runtime_set_active() - pm_runtime_enable() For these scenarios a pm_runtime_get_sync() from any of amba driver's -probe() callback wouldn't have any effect, since the device is already active. In other words, the resources needs to be manually enabled. Kind regards Uffe -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/9] drivercore / platform: Keep PM domain powered during -probe()
On 30 October 2014 21:47, Kevin Hilman khil...@kernel.org wrote: Ulf Hansson ulf.hans...@linaro.org writes: To sucessfully probe some devices their corresponding PM domains may need to be powered. Isn't that what pm_runtime_get*() is supposed to be doing? Why isn't that working? Let stay at the discussion of the cover letter patch. I just replied to this there. Kind regards Uffe -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/9] drivercore / platform: Keep PM domain powered during -probe()
On 31 October 2014 01:07, Dmitry Torokhov dmitry.torok...@gmail.com wrote: On Thu, Oct 30, 2014 at 01:47:27PM -0700, Kevin Hilman wrote: Ulf Hansson ulf.hans...@linaro.org writes: To sucessfully probe some devices their corresponding PM domains may need to be powered. Isn't that what pm_runtime_get*() is supposed to be doing? Why isn't that working? Also, I do not understand why we placing device into a power domain only when we probe it. Why if I unbind device from its driver (or do not have a driver for it) it disappears from its power domain? To me power domain and having driver bound to a device are 2 orthogonal concepts. That's a different discussion, I don't think we want to go there within this context. Kind regards Uffe -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 0/2] clk: samsung: Add clock controller driver for Exynos4415 SoC
On 28/10/14 05:09, Kukjin Kim wrote: Sylwester Nawrocki wrote: On 27/10/14 02:11, Chanwoo Choi wrote: Chanwoo Choi (2): clk: samsung: exynos4415: Add clocks using common clock framework clk: samsung: Document binding for Exynos4415 clock controller [...] Can you please provide a topic branch for clk-exynos4415? Because exynos4415 dt file needs inclusion of following header files. +#include dt-bindings/clock/exynos4415.h +#include dt-bindings/clock/exynos-audss-clk.h Hi Kukjin, AFAICS there is more clk patches that will be a dependency for the dts changes, for instance recent exynos7 clock controller driver addition. Unfortunately the patchset is written so almost every patch touches header file at include/dt-bindings/clock/. I'm inclined to send the collected exynos4415 and exynos7 patches to Mike in a pull request and then he could make a topic branch which could then be pulled as a dependency for dts. Alternatively you could use my branch which can be found at: git://linuxtv.org/snawrocki/samsung.git for-v3.19-exynos-clk What would be your preference, Mike ? -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 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 10/10] usb: dwc2/gadget: rework suspend/resume code to correctly restore gadget state
Hello,
On 2014-10-27 08:18, Marek Szyprowski wrote:
On 2014-10-25 03:16, Paul Zimmerman wrote:
From: Marek Szyprowski [mailto:m.szyprow...@samsung.com]
Sent: Monday, October 20, 2014 3:46 AM
Suspend/resume code assumed that the gadget was always enabled and
connected to usb bus. This means that the actual state of the gadget
(soft-enabled/disabled or connected/disconnected) was not correctly
preserved on suspend/resume cycle. This patch fixes this issue.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/usb/dwc2/core.h | 4 +++-
drivers/usb/dwc2/gadget.c | 43
+++
2 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index bf015ab3b44c..3648b76a18b4 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -210,7 +210,9 @@ struct s3c_hsotg {
u8 ctrl_buff[8];
struct usb_gadget gadget;
-unsigned intsetup;
+unsigned intsetup:1;
+unsigned intconnected:1;
+unsigned intenabled:1;
unsigned long last_rst;
struct s3c_hsotg_ep *eps;
};
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0d34cfc71bfb..c6c6cf982c90 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2925,6 +2925,8 @@ static int s3c_hsotg_udc_start(struct
usb_gadget *gadget,
spin_lock_irqsave(hsotg-lock, flags);
s3c_hsotg_init(hsotg);
s3c_hsotg_core_init_disconnected(hsotg);
+hsotg-enabled = 1;
+hsotg-connected = 0;
spin_unlock_irqrestore(hsotg-lock, flags);
dev_info(hsotg-dev, bound driver %s\n, driver-driver.name);
@@ -2961,6 +2963,8 @@ static int s3c_hsotg_udc_stop(struct
usb_gadget *gadget,
hsotg-driver = NULL;
hsotg-gadget.speed = USB_SPEED_UNKNOWN;
+hsotg-enabled = 0;
+hsotg-connected = 0;
spin_unlock_irqrestore(hsotg-lock, flags);
@@ -2999,11 +3003,14 @@ static int s3c_hsotg_pullup(struct
usb_gadget *gadget, int is_on)
dev_dbg(hsotg-dev, %s: is_on: %d\n, __func__, is_on);
spin_lock_irqsave(hsotg-lock, flags);
+
if (is_on) {
clk_enable(hsotg-clk);
+hsotg-connected = 1;
s3c_hsotg_core_connect(hsotg);
} else {
s3c_hsotg_core_disconnect(hsotg);
+hsotg-connected = 0;
clk_disable(hsotg-clk);
}
@@ -3652,16 +3659,18 @@ static int s3c_hsotg_suspend(struct
platform_device *pdev, pm_message_t state)
dev_info(hsotg-dev, suspending usb gadget %s\n,
hsotg-driver-driver.name);
-spin_lock_irqsave(hsotg-lock, flags);
-s3c_hsotg_core_disconnect(hsotg);
-s3c_hsotg_disconnect(hsotg);
-hsotg-gadget.speed = USB_SPEED_UNKNOWN;
-spin_unlock_irqrestore(hsotg-lock, flags);
+if (hsotg-enabled) {
Hmm. Are you sure it's safe to check -enabled outside of the spinlock?
What happens if s3c_hsotg_udc_stop() runs right after this, before the
spinlock is taken, and disables stuff? Sure, it's a tiny window, but
still...
This code is called only in system suspend path, when userspace has
been already frozen.
udc_stop() can be called only as a result of userspace action, so it
is imho safe to
assume that the above code doesn't need additional locking.
However if you prefer the version with explicit locking, I will send v3
in a few minutes.
Best regards
--
Marek Szyprowski, PhD
Samsung RD Institute Poland
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/2] usb: dwc2/gadget: add mutex to serialize init/deinit calls
This patch adds mutex, which protects initialization and
deinitialization procedures against suspend/resume methods.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/usb/dwc2/core.h | 1 +
drivers/usb/dwc2/gadget.c | 20
2 files changed, 21 insertions(+)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 9f77b4d1c5ff..58732a9a0019 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -187,6 +187,7 @@ struct s3c_hsotg {
struct s3c_hsotg_plat*plat;
spinlock_t lock;
+ struct mutexinit_mutex;
void __iomem*regs;
int irq;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index d8dda39c9e16..a2e4272a904e 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -21,6 +21,7 @@
#include linux/platform_device.h
#include linux/dma-mapping.h
#include linux/debugfs.h
+#include linux/mutex.h
#include linux/seq_file.h
#include linux/delay.h
#include linux/io.h
@@ -2908,6 +2909,7 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
return -EINVAL;
}
+ mutex_lock(hsotg-init_mutex);
WARN_ON(hsotg-driver);
driver-driver.bus = NULL;
@@ -2933,9 +2935,12 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
dev_info(hsotg-dev, bound driver %s\n, driver-driver.name);
+ mutex_unlock(hsotg-init_mutex);
+
return 0;
err:
+ mutex_unlock(hsotg-init_mutex);
hsotg-driver = NULL;
return ret;
}
@@ -2957,6 +2962,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,
if (!hsotg)
return -ENODEV;
+ mutex_lock(hsotg-init_mutex);
+
/* all endpoints should be shutdown */
for (ep = 1; ep hsotg-num_of_eps; ep++)
s3c_hsotg_ep_disable(hsotg-eps[ep].ep);
@@ -2974,6 +2981,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,
clk_disable(hsotg-clk);
+ mutex_unlock(hsotg-init_mutex);
+
return 0;
}
@@ -3002,6 +3011,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget,
int is_on)
dev_dbg(hsotg-dev, %s: is_on: %d\n, __func__, is_on);
+ mutex_lock(hsotg-init_mutex);
spin_lock_irqsave(hsotg-lock, flags);
if (is_on) {
clk_enable(hsotg-clk);
@@ -3013,6 +3023,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget,
int is_on)
hsotg-gadget.speed = USB_SPEED_UNKNOWN;
spin_unlock_irqrestore(hsotg-lock, flags);
+ mutex_unlock(hsotg-init_mutex);
return 0;
}
@@ -3507,6 +3518,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
}
spin_lock_init(hsotg-lock);
+ mutex_init(hsotg-init_mutex);
hsotg-irq = ret;
@@ -3652,6 +3664,8 @@ static int s3c_hsotg_suspend(struct platform_device
*pdev, pm_message_t state)
unsigned long flags;
int ret = 0;
+ mutex_lock(hsotg-init_mutex);
+
if (hsotg-driver)
dev_info(hsotg-dev, suspending usb gadget %s\n,
hsotg-driver-driver.name);
@@ -3674,6 +3688,8 @@ static int s3c_hsotg_suspend(struct platform_device
*pdev, pm_message_t state)
clk_disable(hsotg-clk);
}
+ mutex_unlock(hsotg-init_mutex);
+
return ret;
}
@@ -3683,7 +3699,9 @@ static int s3c_hsotg_resume(struct platform_device *pdev)
unsigned long flags;
int ret = 0;
+ mutex_lock(hsotg-init_mutex);
if (hsotg-driver) {
+
dev_info(hsotg-dev, resuming usb gadget %s\n,
hsotg-driver-driver.name);
@@ -3699,6 +3717,8 @@ static int s3c_hsotg_resume(struct platform_device *pdev)
s3c_hsotg_core_connect(hsotg);
spin_unlock_irqrestore(hsotg-lock, flags);
+ mutex_unlock(hsotg-init_mutex);
+
return ret;
}
--
1.9.2
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/2] usb: dwc2/gadget: rework suspend/resume code to correctly restore gadget state
Suspend/resume code assumed that the gadget was always enabled and
connected to usb bus. This means that the actual state of the gadget
(soft-enabled/disabled or connected/disconnected) was not correctly
preserved on suspend/resume cycle. This patch fixes this issue.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/usb/dwc2/core.h | 4 +++-
drivers/usb/dwc2/gadget.c | 41 +
2 files changed, 28 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 58732a9a0019..a1aa9ecf052e 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -211,7 +211,9 @@ struct s3c_hsotg {
u8 ctrl_buff[8];
struct usb_gadget gadget;
- unsigned intsetup;
+ unsigned intsetup:1;
+ unsigned intconnected:1;
+ unsigned intenabled:1;
unsigned long last_rst;
unsigned intaddress;
struct s3c_hsotg_ep *eps;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index a2e4272a904e..e61bb1c4275d 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2931,6 +2931,8 @@ static int s3c_hsotg_udc_start(struct usb_gadget *gadget,
spin_lock_irqsave(hsotg-lock, flags);
s3c_hsotg_init(hsotg);
s3c_hsotg_core_init_disconnected(hsotg);
+ hsotg-enabled = 1;
+ hsotg-connected = 0;
spin_unlock_irqrestore(hsotg-lock, flags);
dev_info(hsotg-dev, bound driver %s\n, driver-driver.name);
@@ -2972,6 +2974,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,
hsotg-driver = NULL;
hsotg-gadget.speed = USB_SPEED_UNKNOWN;
+ hsotg-enabled = 0;
+ hsotg-connected = 0;
spin_unlock_irqrestore(hsotg-lock, flags);
@@ -3015,9 +3019,11 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget,
int is_on)
spin_lock_irqsave(hsotg-lock, flags);
if (is_on) {
clk_enable(hsotg-clk);
+ hsotg-connected = 1;
s3c_hsotg_core_connect(hsotg);
} else {
s3c_hsotg_core_disconnect(hsotg);
+ hsotg-connected = 0;
clk_disable(hsotg-clk);
}
@@ -3670,16 +3676,18 @@ static int s3c_hsotg_suspend(struct platform_device
*pdev, pm_message_t state)
dev_info(hsotg-dev, suspending usb gadget %s\n,
hsotg-driver-driver.name);
- spin_lock_irqsave(hsotg-lock, flags);
- s3c_hsotg_core_disconnect(hsotg);
- s3c_hsotg_disconnect(hsotg);
- hsotg-gadget.speed = USB_SPEED_UNKNOWN;
- spin_unlock_irqrestore(hsotg-lock, flags);
+ if (hsotg-enabled) {
+ int ep;
- s3c_hsotg_phy_disable(hsotg);
+ spin_lock_irqsave(hsotg-lock, flags);
+ if (hsotg-connected)
+ s3c_hsotg_core_disconnect(hsotg);
+ s3c_hsotg_disconnect(hsotg);
+ hsotg-gadget.speed = USB_SPEED_UNKNOWN;
+ spin_unlock_irqrestore(hsotg-lock, flags);
+
+ s3c_hsotg_phy_disable(hsotg);
- if (hsotg-driver) {
- int ep;
for (ep = 0; ep hsotg-num_of_eps; ep++)
s3c_hsotg_ep_disable(hsotg-eps[ep].ep);
@@ -3705,18 +3713,19 @@ static int s3c_hsotg_resume(struct platform_device
*pdev)
dev_info(hsotg-dev, resuming usb gadget %s\n,
hsotg-driver-driver.name);
+ if (hsotg-enabled) {
clk_enable(hsotg-clk);
ret = regulator_bulk_enable(ARRAY_SIZE(hsotg-supplies),
- hsotg-supplies);
- }
-
- s3c_hsotg_phy_enable(hsotg);
+ hsotg-supplies);
- spin_lock_irqsave(hsotg-lock, flags);
- s3c_hsotg_core_init_disconnected(hsotg);
- s3c_hsotg_core_connect(hsotg);
- spin_unlock_irqrestore(hsotg-lock, flags);
+ s3c_hsotg_phy_enable(hsotg);
+ spin_lock_irqsave(hsotg-lock, flags);
+ s3c_hsotg_core_init_disconnected(hsotg);
+ if (hsotg-connected)
+ s3c_hsotg_core_connect(hsotg);
+ spin_unlock_irqrestore(hsotg-lock, flags);
+ }
mutex_unlock(hsotg-init_mutex);
return ret;
--
1.9.2
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] regulator: max77686: Add external GPIO control
On Fri, Oct 31, 2014 at 08:51:38AM +0100, Krzysztof Kozlowski wrote: However new DT style parsing (regulator_of_get_init_data()) does the basic parsing stuff and this removes a lot of code from driver. The driver no longer parses all regulaotrs but the regulator core does it. Unfortunately regulator core does not parse custom bindings (like such GPIOs) so I would have to iterate once again through all regulators just to find gpios property. We could always add a callback for the driver to handle any custom properties... one of the advantages of an OS like Linux is that we can improve the core code. signature.asc Description: Digital signature
Re: [PATCH 6/8] regulator: max77686: Add external GPIO control
On pią, 2014-10-31 at 10:32 +, Mark Brown wrote: On Fri, Oct 31, 2014 at 08:51:38AM +0100, Krzysztof Kozlowski wrote: However new DT style parsing (regulator_of_get_init_data()) does the basic parsing stuff and this removes a lot of code from driver. The driver no longer parses all regulaotrs but the regulator core does it. Unfortunately regulator core does not parse custom bindings (like such GPIOs) so I would have to iterate once again through all regulators just to find gpios property. We could always add a callback for the driver to handle any custom properties... one of the advantages of an OS like Linux is that we can improve the core code. Then I'll add it. Mark, what device should be assigned to config.dev during registration of regulators? The regulator driver's device or its parent (MFD main driver)? Various drivers do this differently. Best regards, Krzysztof -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] regulator: max77686: Add external GPIO control
On Fri, Oct 31, 2014 at 12:45:47PM +0100, Krzysztof Kozlowski wrote: Then I'll add it. Mark, what device should be assigned to config.dev during registration of regulators? The regulator driver's device or its parent (MFD main driver)? Various drivers do this differently. Normally the parent device. signature.asc Description: Digital signature
Re: [PATCH v3 01/14] mfd: max77686/802: Map regulator driver to its own of_node
On Thu, Oct 30, 2014 at 12:20:40PM +0100, Krzysztof Kozlowski wrote: Add of_compatible fields for max77686 and max77802 regulator drivers. The driver's node should be the same as voltage-regulators node. This simplifies parsing of regulators init data from DTS. No, this is broken. You're introducing an ABI break that conveys no additional information, I can't see any reason why this should make it simpler to parse init data (you've certainly not articulated one in the changelog here) but even if it did you are changing the ABI incompatibly and convenience isn't a good reason to do that. I'm getting very frustrated with what's going on with these drivers, there seem to be a lot of rather large sets of patches spawning lots of discussion but also frequent review problems and very little actually getting merged (look at the set of changes in the past few merge windows for example). There's something going wrong here. signature.asc Description: Digital signature
Re: [PATCH v6 1/3] regulator: max77686: Add suspend disable for some LDOs
On Wed, Oct 29, 2014 at 12:14:52PM +0100, Krzysztof Kozlowski wrote:
Some LDOs of Maxim 77686 PMIC support disabling during system suspend
(LDO{2,6,7,8,10,11,12,14,15,16}). This was already implemented as part
of set_suspend_mode function. In that case the mode was one of:
Applied, thanks.
signature.asc
Description: Digital signature
Re: [PATCH v3 01/14] mfd: max77686/802: Map regulator driver to its own of_node
On pią, 2014-10-31 at 12:23 +, Mark Brown wrote: On Thu, Oct 30, 2014 at 12:20:40PM +0100, Krzysztof Kozlowski wrote: Add of_compatible fields for max77686 and max77802 regulator drivers. The driver's node should be the same as voltage-regulators node. This simplifies parsing of regulators init data from DTS. No, this is broken. You're introducing an ABI break that conveys no additional information, I can't see any reason why this should make it simpler to parse init data (you've certainly not articulated one in the changelog here) but even if it did you are changing the ABI incompatibly and convenience isn't a good reason to do that. The ABI won't be broken - both drivers would work fine with old and new DTB. However I agree that I should justify this more... Javier and you explained me using parent's device for rdev-dev so I think this change won't be needed and I'll just drop it. Thank you for feedback. I'm getting very frustrated with what's going on with these drivers, there seem to be a lot of rather large sets of patches spawning lots of discussion but also frequent review problems and very little actually getting merged (look at the set of changes in the past few merge windows for example). There's something going wrong here. If I over-spammed you, then I am deeply sorry. Best regards, Krzysztof -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 0/2] Fine tune USB 3.0 PHY on exynos5420
This series is tested with V3 of Heikki's patches for simpliefied phy lookup table: [PATCHv3 0/6] phy: simplified phy lookup [1] on 'usb-next' branch. V4 of this series is giving some issue, which i have already pointed out in the patch: [PATCHv4 2/6] phy: improved lookup method Changes since v6: - Dropped the changes for adding additional phy_calibrate() callback. - Added phy_init() and phy_power_on() sequence in xhci-plat driver; NOTE: both phy_init() and phy_power_on() will now require PHY's 'init_count' and 'power_count' to be reset to '0' so that we can actually re-initialize the phy. Though this has already been pointed out in discussion for the previous patch-series. [2] - Refactored return codes and error handling in cr_port functions as pointed out by Felipe. Changes since v5: - Assigned NULL to hcd-gen_phy in error path in xhci-plat.c, so that we don't need to check for IS_ERR() while calibrating the PHYs in core/hcd.c - Removed extra empty lines in register definitions in exynos5-usbdrd phy driver. - Added write access for EXYNOS5_DRD_PHYREG0 register before any crport_handshake() call as suggested by Jingoo Han. - Renamed member 'calibrate' to 'phy_exynos_calibrate' of struct exynos5_usbdrd_phy_drvdata. Changes since v4: - Rebased on latest patches by Heikki. - Took care of handling -EPROBE_DEFER error number while getting PHY in xhci plat. Changes from v3: - Modified error message as per review comments from Julius. Changes since v2: - Removed any check for DWC3 in xhci-plat for getting usb2-phy and usb3-phy, in order to make it more generic. - Moved the phy_calibration calls to core/hcd.c to enable a more generic solution for issues of calibrating the PHYs. Changes since v1: - Using 'gen_phy' member of 'hcd' instead of declaring more variables to hold phys. - Added a check for compatible match for 'Synopsys-dwc3' controller, since the 'gen_phy' member of 'hcd' already gets the 'usb' PHY in core/hcd.c; but XHCI on Synopsys-dwc3 doesn't need that, instead two separate PHYs for UTMI+ and PIPE3 for the two HCDs (main hcd and shared hcd). - Restructured the code in 'xhci_plat_setup()' and 'xhci_plat_resume()' to use hcd-gen_phy directly. Also added the check for Synopsys's DWC3 controller while trying to calibrate the PHY. Explanation for the need of this patch-series: The DWC3-exynos eXtensible host controller present on Exynos5420/5800 SoCs is quirky. The PHY serving this controller operates at High-Speed by default, so it detects even Super-speed devices as high-speed ones. Certain PHY parameters like Tx LOS levels and Boost levels need to be calibrated further post initialization of xHCI controller, to get SuperSpeed operations working. [1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg710094.html [2] https://lkml.org/lkml/2014/9/2/170; (to be specific https://lkml.org/lkml/2014/9/10/132) Vivek Gautam (2): usb: host: xhci-plat: Get PHYs for xhci's hcds phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800 drivers/phy/phy-exynos5-usbdrd.c | 219 +++--- drivers/usb/host/xhci-plat.c | 74 + 2 files changed, 277 insertions(+), 16 deletions(-) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 2/2] phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800
Adding phy calibration sequence for USB 3.0 DRD PHY present on Exynos5420/5800 systems. This calibration facilitates setting certain PHY parameters viz. the Loss-of-Signal (LOS) Detector Threshold Level, as well as Tx-Vboost-Level for Super-Speed operations. Additionally we also set proper time to wait for RxDetect measurement, for desired PHY reference clock, so as to solve issue with enumeration of few USB 3.0 devices, like Samsung SUM-TSB16S 3.0 USB drive on the controller. We are using CR_port for this purpose to send required data to override the LOS values. On testing with USB 3.0 devices on USB 3.0 port present on SMDK5420, and peach-pit boards should see following message: usb 2-1: new SuperSpeed USB device number 2 using xhci-hcd and without this patch, should see below shown message: usb 1-1: new high-speed USB device number 2 using xhci-hcd [Also removed unnecessary extra lines in the register macro definitions] Signed-off-by: Vivek Gautam gautam.vi...@samsung.com --- drivers/phy/phy-exynos5-usbdrd.c | 219 +++--- 1 file changed, 203 insertions(+), 16 deletions(-) diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c index b3ca3bc..1a63634 100644 --- a/drivers/phy/phy-exynos5-usbdrd.c +++ b/drivers/phy/phy-exynos5-usbdrd.c @@ -37,13 +37,11 @@ /* EXYNOS5: USB 3.0 DRD PHY registers */ #define EXYNOS5_DRD_LINKSYSTEM 0x04 - #define LINKSYSTEM_FLADJ_MASK (0x3f 1) #define LINKSYSTEM_FLADJ(_x) ((_x) 1) #define LINKSYSTEM_XHCI_VERSION_CONTROLBIT(27) #define EXYNOS5_DRD_PHYUTMI0x08 - #define PHYUTMI_OTGDISABLE BIT(6) #define PHYUTMI_FORCESUSPEND BIT(1) #define PHYUTMI_FORCESLEEP BIT(0) @@ -51,26 +49,20 @@ #define EXYNOS5_DRD_PHYPIPE0x0c #define EXYNOS5_DRD_PHYCLKRST 0x10 - #define PHYCLKRST_EN_UTMISUSPEND BIT(31) - #define PHYCLKRST_SSC_REFCLKSEL_MASK (0xff 23) #define PHYCLKRST_SSC_REFCLKSEL(_x)((_x) 23) - #define PHYCLKRST_SSC_RANGE_MASK (0x03 21) #define PHYCLKRST_SSC_RANGE(_x)((_x) 21) - #define PHYCLKRST_SSC_EN BIT(20) #define PHYCLKRST_REF_SSP_EN BIT(19) #define PHYCLKRST_REF_CLKDIV2 BIT(18) - #define PHYCLKRST_MPLL_MULTIPLIER_MASK (0x7f 11) #define PHYCLKRST_MPLL_MULTIPLIER_100MHZ_REF (0x19 11) #define PHYCLKRST_MPLL_MULTIPLIER_50M_REF (0x32 11) #define PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF(0x68 11) #define PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF(0x7d 11) #define PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF (0x02 11) - #define PHYCLKRST_FSEL_UTMI_MASK (0x7 5) #define PHYCLKRST_FSEL_PIPE_MASK (0x7 8) #define PHYCLKRST_FSEL(_x) ((_x) 5) @@ -78,46 +70,68 @@ #define PHYCLKRST_FSEL_PAD_24MHZ (0x2a 5) #define PHYCLKRST_FSEL_PAD_20MHZ (0x31 5) #define PHYCLKRST_FSEL_PAD_19_2MHZ (0x38 5) - #define PHYCLKRST_RETENABLEN BIT(4) - #define PHYCLKRST_REFCLKSEL_MASK (0x03 2) #define PHYCLKRST_REFCLKSEL_PAD_REFCLK (0x2 2) #define PHYCLKRST_REFCLKSEL_EXT_REFCLK (0x3 2) - #define PHYCLKRST_PORTRESETBIT(1) #define PHYCLKRST_COMMONONNBIT(0) #define EXYNOS5_DRD_PHYREG00x14 +#define PHYREG0_SSC_REF_CLK_SELBIT(21) +#define PHYREG0_SSC_RANGE BIT(20) +#define PHYREG0_CR_WRITE BIT(19) +#define PHYREG0_CR_READBIT(18) +#define PHYREG0_CR_DATA_IN(_x) ((_x) 2) +#define PHYREG0_CR_CAP_DATABIT(1) +#define PHYREG0_CR_CAP_ADDRBIT(0) + #define EXYNOS5_DRD_PHYREG10x18 +#define PHYREG1_CR_DATA_OUT(_x)((_x) 1) +#define PHYREG1_CR_ACK BIT(0) #define EXYNOS5_DRD_PHYPARAM0 0x1c - #define PHYPARAM0_REF_USE_PAD BIT(31) #define PHYPARAM0_REF_LOSLEVEL_MASK(0x1f 26) #define PHYPARAM0_REF_LOSLEVEL (0x9 26) #define EXYNOS5_DRD_PHYPARAM1 0x20 - #define PHYPARAM1_PCS_TXDEEMPH_MASK(0x1f 0) #define PHYPARAM1_PCS_TXDEEMPH (0x1c) #define EXYNOS5_DRD_PHYTERM0x24 #define EXYNOS5_DRD_PHYTEST0x28 - #define PHYTEST_POWERDOWN_SSP BIT(3) #define PHYTEST_POWERDOWN_HSP BIT(2) #define EXYNOS5_DRD_PHYADP 0x2c #define EXYNOS5_DRD_PHYUTMICLKSEL 0x30 - #define PHYUTMICLKSEL_UTMI_CLKSEL BIT(2) #define EXYNOS5_DRD_PHYRESUME 0x34
[PATCH v7 1/2] usb: host: xhci-plat: Get PHYs for xhci's hcds
The host controller by itself may sometimes need to handle PHY
and re-initialize it to re-configure some of the PHY parameters
to get full support out of the PHY controller.
Therefore, facilitate getting the two possible PHYs, viz.
USB 2.0 type (UTMI+) and USB 3.0 type (PIPE3), and initialize them.
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
---
drivers/usb/host/xhci-plat.c | 74 ++
1 file changed, 74 insertions(+)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 3d78b0c..5207d5b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -16,6 +16,7 @@
#include linux/module.h
#include linux/of.h
#include linux/platform_device.h
+#include linux/phy/phy.h
#include linux/slab.h
#include linux/usb/xhci_pdriver.h
@@ -129,10 +130,41 @@ static int xhci_plat_probe(struct platform_device *pdev)
goto put_hcd;
}
+ /* Get possile USB 2.0 type PHY (UTMI+) available with xhci */
+ hcd-phy = devm_phy_get(pdev-dev, usb2-phy);
+ if (IS_ERR(hcd-phy)) {
+ ret = PTR_ERR(hcd-phy);
+ if (ret == -EPROBE_DEFER) {
+ goto disable_clk;
+ } else if (ret != -ENOSYS ret != -ENODEV) {
+ hcd-phy = NULL;
+ dev_warn(pdev-dev,
+Error retrieving usb2 phy: %d\n, ret);
+ }
+ }
+
ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (ret)
goto disable_clk;
+ /*
+* Initialize and power-on USB 2.0 PHY
+* FIXME: Isn't this a hacky way of initializing the PHY again ?
+* xhci's parent would have already initialized the PHY, but we
+* wanna do it again.
+*/
+ hcd-phy-init_count = 0;
+ ret = phy_init(hcd-phy);
+ if (ret)
+ goto dealloc_usb2_hcd;
+
+ hcd-phy-power_count = 0;
+ ret = phy_power_on(hcd-phy);
+ if (ret) {
+ phy_exit(hcd-phy);
+ goto dealloc_usb2_hcd;
+ }
+
device_wakeup_enable(hcd-self.controller);
/* USB 2.0 roothub is stored in the platform_device now. */
@@ -158,12 +190,41 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (HCC_MAX_PSA(xhci-hcc_params) = 4)
xhci-shared_hcd-can_do_streams = 1;
+ /* Get possile USB 3.0 type PHY (PIPE3) available with xhci */
+ xhci-shared_hcd-phy = devm_phy_get(pdev-dev, usb3-phy);
+ if (IS_ERR(xhci-shared_hcd-phy)) {
+ ret = PTR_ERR(xhci-shared_hcd-phy);
+ if (ret == -EPROBE_DEFER) {
+ goto put_usb3_hcd;
+ } else if (ret != -ENOSYS ret != -ENODEV) {
+ xhci-shared_hcd-phy = NULL;
+ dev_warn(pdev-dev,
+Error retrieving usb3 phy: %d\n, ret);
+ }
+ }
+
ret = usb_add_hcd(xhci-shared_hcd, irq, IRQF_SHARED);
if (ret)
goto put_usb3_hcd;
+ /* Initialize and power-on USB 3.0 PHY */
+ xhci-shared_hcd-phy-init_count = 0;
+ ret = phy_init(xhci-shared_hcd-phy);
+ if (ret)
+ goto dealloc_usb3_hcd;
+
+ xhci-shared_hcd-phy-power_count = 0;
+ ret = phy_power_on(xhci-shared_hcd-phy);
+ if (ret) {
+ phy_exit(xhci-shared_hcd-phy);
+ goto dealloc_usb3_hcd;
+ }
+
return 0;
+dealloc_usb3_hcd:
+ usb_remove_hcd(xhci-shared_hcd);
+
put_usb3_hcd:
usb_put_hcd(xhci-shared_hcd);
@@ -186,9 +247,15 @@ static int xhci_plat_remove(struct platform_device *dev)
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct clk *clk = xhci-clk;
+ phy_power_off(xhci-shared_hcd-phy);
+ phy_exit(xhci-shared_hcd-phy);
+
usb_remove_hcd(xhci-shared_hcd);
usb_put_hcd(xhci-shared_hcd);
+ phy_power_off(hcd-phy);
+ phy_exit(hcd-phy);
+
usb_remove_hcd(hcd);
if (!IS_ERR(clk))
clk_disable_unprepare(clk);
@@ -204,6 +271,8 @@ static int xhci_plat_suspend(struct device *dev)
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ phy_exit(hcd-phy);
+
return xhci_suspend(xhci);
}
@@ -211,6 +280,11 @@ static int xhci_plat_resume(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ int ret;
+
+ ret = phy_init(hcd-phy);
+ if (ret)
+ return ret;
return xhci_resume(xhci, 0);
}
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: dts: Fix CLK_UART_ISP_SCLK clock assignment in exynos4x12.dtsi
Assign proper FIMC-IS UART gate clock in the device DT node and not use the SRC_MASK gate. This fixes regression introduced in commit a37c82a3b3c0910019abfd22a97be1f (clk: samsung: exynos4: Remove SRC_MASK_ISP gates). Without this change exynos4 fimc-is driver fails to probe with an error log: [1.842447] ERROR: could not get clock /camera/fimc-is@1200:uart(13) [1.848529] exynos4-fimc-is 1200.fimc-is: failed to get clock: uart [1.855275] exynos4-fimc-is: probe of 1200.fimc-is failed with error -2 Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com --- arch/arm/boot/dts/exynos4x12.dtsi |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/exynos4x12.dtsi b/arch/arm/boot/dts/exynos4x12.dtsi index 861bb91..dd98768 100644 --- a/arch/arm/boot/dts/exynos4x12.dtsi +++ b/arch/arm/boot/dts/exynos4x12.dtsi @@ -224,7 +224,7 @@ clock CLK_DIV_ISP0,clock CLK_DIV_ISP1, clock CLK_DIV_MCUISP0, clock CLK_DIV_MCUISP1, -clock CLK_SCLK_UART_ISP, +clock CLK_UART_ISP_SCLK, clock CLK_ACLK200, clock CLK_DIV_ACLK200, clock CLK_ACLK400_MCUISP, clock CLK_DIV_ACLK400_MCUISP; -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/2] usb: host: xhci-plat: Get PHYs for xhci's hcds
Hello.
On 10/31/2014 4:26 PM, Vivek Gautam wrote:
The host controller by itself may sometimes need to handle PHY
and re-initialize it to re-configure some of the PHY parameters
to get full support out of the PHY controller.
Therefore, facilitate getting the two possible PHYs, viz.
USB 2.0 type (UTMI+) and USB 3.0 type (PIPE3), and initialize them.
Signed-off-by: Vivek Gautam gautam.vi...@samsung.com
---
drivers/usb/host/xhci-plat.c | 74 ++
1 file changed, 74 insertions(+)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 3d78b0c..5207d5b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
[...]
@@ -129,10 +130,41 @@ static int xhci_plat_probe(struct platform_device *pdev)
goto put_hcd;
}
+ /* Get possile USB 2.0 type PHY (UTMI+) available with xhci */
+ hcd-phy = devm_phy_get(pdev-dev, usb2-phy);
+ if (IS_ERR(hcd-phy)) {
+ ret = PTR_ERR(hcd-phy);
+ if (ret == -EPROBE_DEFER) {
+ goto disable_clk;
+ } else if (ret != -ENOSYS ret != -ENODEV) {
Asking to be a *switch* statement instead...
+ hcd-phy = NULL;
+ dev_warn(pdev-dev,
+Error retrieving usb2 phy: %d\n, ret);
+ }
+ }
+
[...]
@@ -158,12 +190,41 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (HCC_MAX_PSA(xhci-hcc_params) = 4)
xhci-shared_hcd-can_do_streams = 1;
+ /* Get possile USB 3.0 type PHY (PIPE3) available with xhci */
+ xhci-shared_hcd-phy = devm_phy_get(pdev-dev, usb3-phy);
+ if (IS_ERR(xhci-shared_hcd-phy)) {
+ ret = PTR_ERR(xhci-shared_hcd-phy);
+ if (ret == -EPROBE_DEFER) {
+ goto put_usb3_hcd;
+ } else if (ret != -ENOSYS ret != -ENODEV) {
Likewise...
+ xhci-shared_hcd-phy = NULL;
+ dev_warn(pdev-dev,
+Error retrieving usb3 phy: %d\n, ret);
+ }
+ }
+
[...]
@@ -204,6 +271,8 @@ static int xhci_plat_suspend(struct device *dev)
struct usb_hcd *hcd = dev_get_drvdata(dev);
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ phy_exit(hcd-phy);
Hm, in the suspend() method?
+
return xhci_suspend(xhci);
}
[...]
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] drm/exynos: Replace repeated declaration by include drm/drmP.h
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
Re-declare struct is not a good practice, let's use the original
drm declarations.
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/exynos/exynos_drm_drv.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 3905e30..7806981 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -15,6 +15,7 @@
#ifndef _EXYNOS_DRM_DRV_H_
#define _EXYNOS_DRM_DRV_H_
+#include drm/drmP.h
#include linux/module.h
#define MAX_CRTC 3
@@ -36,9 +37,6 @@
#define wait_for(COND, MS) _wait_for(COND, MS)
-struct drm_device;
-struct drm_connector;
-
/* This enumerates device type. */
enum exynos_drm_device_type {
EXYNOS_DEVICE_TYPE_NONE,
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/9] drm/exynos: Replace repeated declarations by #include exynos_drm_drv.h
From: Gustavo Padovan gustavo.pado...@collabora.co.uk Re-declare struct is not a good practice, let's use the original drm and exynos declarations. Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk --- drivers/gpu/drm/exynos/exynos_drm_crtc.h | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.h b/drivers/gpu/drm/exynos/exynos_drm_crtc.h index 690dcdd..e353d35 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h @@ -15,10 +15,7 @@ #ifndef _EXYNOS_DRM_CRTC_H_ #define _EXYNOS_DRM_CRTC_H_ -struct drm_device; -struct drm_crtc; -struct exynos_drm_manager; -struct exynos_drm_overlay; +#include exynos_drm_drv.h int exynos_drm_crtc_create(struct exynos_drm_manager *manager); int exynos_drm_crtc_enable_vblank(struct drm_device *dev, int pipe); -- 1.9.3 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 7/9] drm/exynos: Save up space using bool var as bitfields
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
Save a few bytes by compiling them all in the same byte.
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/exynos/exynos_drm_drv.h | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 9e4a7e1..f77e6aa 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -66,10 +66,10 @@ enum exynos_drm_output_type {
* @dma_addr: array of bus(accessed by dma) address to the memory region
* allocated for a overlay.
* @zpos: order of overlay layer(z position).
- * @default_win: a window to be enabled.
- * @color_key: color key on or off.
* @index_color: if using color key feature then this value would be used
* as index color.
+ * @default_win: a window to be enabled.
+ * @color_key: color key on or off.
* @local_path: in case of lcd type, local path mode on or off.
* @transparency: transparency on or off.
* @activated: activated or not.
@@ -97,13 +97,13 @@ struct exynos_drm_overlay {
uint32_t pixel_format;
dma_addr_t dma_addr[MAX_FB_BUFFER];
int zpos;
-
- bool default_win;
- bool color_key;
unsigned int index_color;
- bool local_path;
- bool transparency;
- bool activated;
+
+ bool default_win:1;
+ bool color_key:1;
+ bool local_path:1;
+ bool transparency:1;
+ bool activated:1;
};
/*
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/9] drm/exynos: remove leftover hdmi function declarations
From: Gustavo Padovan gustavo.pado...@collabora.co.uk They are not implemented anywhere, so wipe them out. Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk --- drivers/gpu/drm/exynos/exynos_drm_drv.h | 11 --- 1 file changed, 11 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h index e762cbb..262a459 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h @@ -294,17 +294,6 @@ int exynos_drm_device_subdrv_remove(struct drm_device *dev); int exynos_drm_subdrv_open(struct drm_device *dev, struct drm_file *file); void exynos_drm_subdrv_close(struct drm_device *dev, struct drm_file *file); -/* - * this function registers exynos drm hdmi platform device. It ensures only one - * instance of the device is created. - */ -int exynos_platform_device_hdmi_register(void); - -/* - * this function unregisters exynos drm hdmi platform device if it exists. - */ -void exynos_platform_device_hdmi_unregister(void); - #ifdef CONFIG_DRM_EXYNOS_IPP int exynos_platform_device_ipp_register(void); void exynos_platform_device_ipp_unregister(void); -- 1.9.3 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/9] drm/exynos: remove uneeded declaration of struct dma_iommu_mapping
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
It is not even used in this header anymore.
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/exynos/exynos_drm_iommu.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.h
b/drivers/gpu/drm/exynos/exynos_drm_iommu.h
index 72376d4..35d2588 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_iommu.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_iommu.h
@@ -40,7 +40,6 @@ static inline bool is_drm_iommu_supported(struct drm_device
*drm_dev)
#else
-struct dma_iommu_mapping;
static inline int drm_create_iommu_mapping(struct drm_device *drm_dev)
{
return 0;
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 8/9] drm/exynos: update documentation to reflect code changes
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
Description of the @create_connector callback was missing,
and the @manager was no longer needed.
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/exynos/exynos_drm_drv.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index f77e6aa..e762cbb 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -110,6 +110,7 @@ struct exynos_drm_overlay {
* Exynos DRM Display Structure.
* - this structure is common to analog tv, digital tv and lcd panel.
*
+ * @create_connector: initialize and register a new connector
* @remove: cleans up the display for removal
* @mode_fixup: fix mode data comparing to hw specific display mode.
* @mode_set: convert drm_display_mode to hw specific display mode and
@@ -262,8 +263,6 @@ struct exynos_drm_private {
* @dev: pointer to device object for subdrv device driver.
* @drm_dev: pointer to drm_device and this pointer would be set
* when sub driver calls exynos_drm_subdrv_register().
- * @manager: subdrv has its own manager to control a hardware appropriately
- * and we can access a hardware drawing on this manager.
* @probe: this callback would be called by exynos drm driver after
* subdrv is registered to it.
* @remove: this callback is used to release resources created
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/9] drm/exynos: remove extra declaration of struct exynos_drm_manager
From: Gustavo Padovan gustavo.pado...@collabora.co.uk The struct is defined in the same file, declare it here is just unnecessary. Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk --- drivers/gpu/drm/exynos/exynos_drm_encoder.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.h b/drivers/gpu/drm/exynos/exynos_drm_encoder.h index b7a1620..26305d8 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.h +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.h @@ -14,8 +14,6 @@ #ifndef _EXYNOS_DRM_ENCODER_H_ #define _EXYNOS_DRM_ENCODER_H_ -struct exynos_drm_manager; - void exynos_drm_encoder_setup(struct drm_device *dev); struct drm_encoder *exynos_drm_encoder_create(struct drm_device *dev, struct exynos_drm_display *mgr, -- 1.9.3 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/9] drm/exynos: remove extra declaration of struct exynos_overlay
From: Gustavo Padovan gustavo.pado...@collabora.co.uk The struct is defined in the same file, declare it here is just unnecessary Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk --- drivers/gpu/drm/exynos/exynos_drm_drv.h | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h index 3c81c4b..3905e30 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h @@ -37,7 +37,6 @@ #define wait_for(COND, MS) _wait_for(COND, MS) struct drm_device; -struct exynos_drm_overlay; struct drm_connector; /* This enumerates device type. */ -- 1.9.3 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 6/9] drm/exynos: remove unused wait_for macro
From: Gustavo Padovan gustavo.pado...@collabora.co.uk
This is a leftover, all code using this macro have been removed/
changed already.
Signed-off-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
---
drivers/gpu/drm/exynos/exynos_drm_drv.h | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h
b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 7806981..9e4a7e1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -23,20 +23,6 @@
#define MAX_FB_BUFFER 4
#define DEFAULT_ZPOS -1
-#define _wait_for(COND, MS) ({ \
- unsigned long timeout__ = jiffies + msecs_to_jiffies(MS); \
- int ret__ = 0; \
- while (!(COND)) { \
- if (time_after(jiffies, timeout__)) { \
- ret__ = -ETIMEDOUT; \
- break; \
- } \
- } \
- ret__; \
-})
-
-#define wait_for(COND, MS) _wait_for(COND, MS)
-
/* This enumerates device type. */
enum exynos_drm_device_type {
EXYNOS_DEVICE_TYPE_NONE,
--
1.9.3
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 03/12] drm/bridge: Add helper functions for drm_bridge
On Wed, Oct 29, 2014 at 10:14:37AM +0100, Thierry Reding wrote:
On Wed, Oct 29, 2014 at 09:57:02AM +0100, Daniel Vetter wrote:
That makes the entire thing a bit non-trivial, which is why I think it
would be better as some generic helper. Which then gets embedded or
instantiated for specific cases, like dtdrm_panel or dtdrm_bridge.
Or maybe even acpidrm_bridge, who knows ;-)
I worry a little about type safety. How will this generic helper know
what registry to look in for? Or conversely, if all these resources are
added to a single registry how do you know that they're of the correct
type? Failing to ensure this could cause situations where you're asking
for a panel and get a bridge in return because you've wrongly wired it
up in device tree for example.
But perhaps if both the registry and the device parts are turned into
helpers we could still have a single core implementation and then
instantiate that for each type, something roughly like this:
struct registry {
struct list_head list;
struct mutex lock;
};
struct registry_record {
struct list_head list;
struct module *owner;
struct kref *ref;
struct device *dev;
};
int registry_add(struct registry *registry, struct registry_record
*record)
{
...
try_module_get(record-owner);
...
}
struct registry_record *registry_find_by_of_node(struct registry
*registry,
struct device_node *np)
{
...
kref_get(...);
...
}
That way it should be possible to embed these into other structures,
like so:
struct drm_panel {
struct registry_record record;
...
};
static struct registry drm_panels;
int drm_panel_add(struct drm_panel *panel)
{
return registry_add(drm_panels, panel-record);
}
struct drm_panel *of_drm_panel_find(struct device_node *np)
{
struct registry_record *record;
record = registry_find_by_of_node(drm_panels, np);
return container_of(record, struct drm_panel, record);
}
Is that what you had in mind?
Yeah I've thought that we should instantiate using macros even, so that we
have per-type registries. So you'd smash the usual set of
DECLARE/DEFINE_AUX_DEV_REGISTRY into headers/source files, and they'd take
a (name, key, value) tripled. For the example here(of_drm_panel, struct
device_node *, struct drm_panel *) or similar. I might be hand-waving over
a few too many details though ;-)
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 03/12] drm/bridge: Add helper functions for drm_bridge
On Thu, Oct 30, 2014 at 10:09:28AM +, Russell King - ARM Linux wrote: On Thu, Oct 30, 2014 at 11:01:02AM +0100, Andrzej Hajda wrote: On 10/29/2014 10:14 AM, Thierry Reding wrote: On Wed, Oct 29, 2014 at 09:57:02AM +0100, Daniel Vetter wrote: I think we nee try_get_module for the code and kref on the actual data structures. Agreed, that should do the trick. We'd probably need some sort of logic to also make operations return something like -ENODEV when the underlying device has disappeared. I think David had introduced something similar for DRM device not so long ago? If the underlying device disappears it would be good to receive notification anyway to trigger DRM HPD event. And if we have the notification, we can release references to the device smoothly. We do not need to play tricky games with krefs, zombie data and module refcounting. Any solution which thinks it needs to lock modules into the core is fundamentally broken. It totally misses the point. While you can lock a module into the core using try_get_module(), that doesn't stop the device itself being unbound from a driver. Soo many people have fallen into that trap. They write their device driver, along with some kind of framework which they make use try_get_module(). They think its safe. When you then echo the device name into the driver's unbind sysfs file, all hell breaks loose and the kernel oopses. try_get_module is /totally/ useless for ensuring that things stick around. The reality is that you can't make devices stick around. Once that remove callback from the driver layer is called, that's it, the device _is_ going away whether you like it or not. You can't stop it. It's no good returning -EBUSY, because the remove return code is ignored. What's more scarey is when you consider that in a real hotplug situation, when the remove callback is called, the device has /already/ gone. So please, stop thinking that try_get_module() has some magic solution. Any solution to device lifetimes using try_get_module() totally misses the problem, and is just mere obfuscation and actually a bug in itself. We need proper refcounting ofc, but we also need to make sure that as long as the thing is around the text section for the final unref (at least that) doesn't go poof. I'd prefer if the framework takes care of that detail and drivers could just supply a THIS_MODULE at the right place. But fully agree on your overall point that try_get_module alone is pure snake oil. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V7 03/12] drm/bridge: Add helper functions for drm_bridge
On Wed, Oct 29, 2014 at 10:16:49AM +0100, Thierry Reding wrote:
On Wed, Oct 29, 2014 at 08:51:27AM +0100, Daniel Vetter wrote:
On Tue, Oct 28, 2014 at 03:29:47PM +0100, Thierry Reding wrote:
On Mon, Oct 27, 2014 at 11:20:31PM +0100, Daniel Vetter wrote:
On Mon, Oct 27, 2014 at 8:58 PM, Sean Paul seanp...@chromium.org
wrote:
@@ -660,8 +662,11 @@ struct drm_bridge_funcs {
* @driver_private: pointer to the bridge driver's internal context
*/
struct drm_bridge {
- struct drm_device *dev;
+ struct device *dev;
Please don't rename the -dev pointer into drm. Because _all_ the
other
drm structures still call it -dev. Also, can't we use struct
device_node
here like we do in the of helpers Russell added? See 7e435aad38083
I think this is modeled after the naming in drm_panel, FWIW. However,
seems reasonable to keep the device_node instead.
Hm, indeed. Tbh I vote to rename drm_panel-drm to -dev and like with
drm_crtc drop the struct device and go directly to a struct
device_node. Since we don't really need the sturct device, the only
thing we care about is the of_node. For added bonus wrap an #ifdef
CONFIG_OF around all the various struct device_node in drm_foo.h.
Should be all fairly simple to pull off with cocci.
Thierry?
The struct device * is in DRM panel because there's nothing device tree
specific about the concept. Having a struct device_node * instead would
indicate that it can only be used with a device tree, whereas the
framework doesn't care the tiniest bit what type of device we have.
While the trend clearly is to use more device tree, I don't think we
should make it impossible for anybody else to use these frameworks.
There are other advantages to keeping a struct device *, like having
access to the proper device and its name. Also you get access to the
device_node * via dev-of_node anyway. I don't see any advantage in
switching to just a struct device_node *, only disadvantages.
Well the idea is to make the lookup key specific, and conditional on
#CONFIG_OF. If there's going to be another neat way to enumerate platform
devices then I think we should add that, too. Or maybe we should have a
void *platform_data or so.
The reason I really don't want a struct device * in core drm structures is
that two releases down the road people will have found tons of really
great ways to abuse them and re-create a midlayer. DRM core really should
only care about the sw objects and not be hw specific at all. Heck there's
not even an requirement to have any piece of actual hw, you could write a
completely fake drm driver (for e.g. testing like the new v4l driver).
Tbh I wonder a bit why we even have this registery embedded into the core
drm objects. Essentially the only thing you're doing is a list that maps
some platform specific key onto some subsystem specific driver structure
or fails the lookup. So instead of putting all these low-level details
into drm core structures can't we just have a generic hashtable/list for
this, plus some static inline helpers that cast the void * you get into
the one you want?
I also get the feeling that this really should be in the driver core (like
the component helpers), and that we should think a lot harder about
lifetimes and refcounting (see my other reply on that).
Yes, that sounds very useful indeed. Also see my reply to yours. =)
Just replying here with some of the irc discussions we've had. Since
drm_bridge/panel isn't a core drm interface object exposed to userspace
it's less critical. I still think that wasting a few brain cycles to have
a clear separation between the abstract interface object and how to bind
and unbind the pieces together is worthwhile, even though the cost when
getting it wrong is much less severe than in the case of a mandatory piece
of core infrastructure.
I think in general the recent armsoc motivated drm infrastructure lacks a
bit in attention to these details. E.g. the cma helpers are built into the
drm.ko module, but clearly are auxilliary library code. So they should be
pulled out and the headers clean up, so that we have a clear separation
between core and helpers. Otherwise someone will sooner or later screw up
and insert a helper depency into the core, and then we've started with the
midlayer mess. Same goes with drm_bridge/panel, which didn't even bother
to have separate headers from the core modeset header drm_crtc.h.
So would be great if someone could invest a bit of time into cleaning this
up. Writing proper api docs also helps a lot with achieving a clean and
sensible split ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to
Re: [PATCH V7 03/12] drm/bridge: Add helper functions for drm_bridge
On Wed, Oct 29, 2014 at 10:09:04AM +0100, Andrzej Hajda wrote: On 10/29/2014 08:58 AM, Daniel Vetter wrote: On Tue, Oct 28, 2014 at 04:05:34PM +0100, Thierry Reding wrote: On Tue, Oct 28, 2014 at 08:16:44PM +0530, Ajay kumar wrote: On Tue, Oct 28, 2014 at 8:11 PM, Thierry Reding thierry.red...@gmail.com wrote: On Tue, Oct 28, 2014 at 03:19:36PM +0100, Daniel Vetter wrote: On Tue, Oct 28, 2014 at 1:28 PM, Ajay kumar ajayn...@gmail.com wrote: On Tue, Oct 28, 2014 at 3:31 PM, Daniel Vetter dan...@ffwll.ch wrote: [...] Hm, if you do this can you pls also update drm_panel accordingly? It shouldn't be a lot of fuzz and would make things around drm+dt more consistent. Are you talking about using struct device_node instead of struct device? I guess you have misplaced the comment under the wrong section! Yeah, that should have been one up ;-) Like I said earlier, I don't think dropping struct device * in favour of struct device_node * is a good idea. I am not sure about drm_panel. But, I am not really doing anything with the struct device pointer in case of bridge. So, just wondering if it is really needed? I think it's useful to have it just to send the right message. DRM panel and DRM bridge aren't specific to device tree. They are completely generic and can work with any type of device, whether it was instantiated from the device tree or some other infrastructure. Dropping struct device * will make it effectively useless on anything but DT. I don't think we should strive for that, even if only DT-enabled platforms currently use them. See my other reply, but I now think we should put neither into drm structures. This find me the driver structures for this device problem looks really generic, and it has nothing to do with the drm structures and concepts like bridges/panels at all. It shouldn't be in there at all. Adding it looks very much like reintroducing the drm midlayer that we just finally made obsolete, just not at the top-level (struct drm_device) but at a bunch of leaf nodes. I expect all the same issues though. And I'm definitely not looking to de-midlayer more stuff that we're just adding. Imo this should be solved as a separate helper thing, maybe in the driver core akin to the component helpers from Russell. -Daniel As I understand you want something generic to look for panels, bridges, whatever and, like components, it should allow to safe hot plug/unplug. I have proposed such thing few months ago [1]. Have you looked at it? [1]: https://lkml.org/lkml/2014/4/30/345 Yeah I think I've looked but wasn't convinced. I do agree though that we should definitely aim for something in the driver core. Since if Greg doesn't like it it's not suddenly better if we just hide it in the drm subsystem. This really smells like a generic issue - after all we already have a two implementations with bridgespanels already. So maybe we need to augment the component framework with the possibility to add additional devices later on at runtime, or something similar. Not really sure since I don't have actual practice with these issues since i915 doesn't (yet) have these problems. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL] Samsung clock changes for 3.19
Hi Mike, I've collected Exynos clk patches in this pull request, as Tomasz has been busy recently. This includes addition of clock controller drivers for Exynos4415 and Exynos7 SoCs and related refactoring of the Samsung common clk API. As I mentioned in my other e-mail, it might be sensible to put these patches into a separate topic branch, so Kukjin can pull it as a dependency for the related dts changes. There are also few Samsung clk cleanup patches not included here, I thought I'd send them separately. The following changes since commit f114040e3ea6e07372334ade75d1ee0775c355e1: Linux 3.18-rc1 (2014-10-19 18:08:38 -0700) are available in the git repository at: git://linuxtv.org/snawrocki/samsung.git for-v3.19/exynos-clk for you to fetch changes up to 932e98224d5602be17ed61d0e057e9326f12b59d: clk: samsung: exynos7: add gate clock for ADC block (2014-10-31 10:45:54 +0100) Abhilash Kesavan (1): clk: samsung: exynos7: add gate clock for ADC block Chanwoo Choi (2): clk: samsung: Document binding for Exynos4415 clock controller clk: samsung: exynos4415: Add clocks using common clock framework Naveen Krishna Ch (8): clk: samsung: add support for 145xx and 1460x PLLs clk: samsung: Factor out the common code to clk.c clk: samsung: Add fixed_factor_clocks field to struct exynos_cmu_info clk: samsung: add initial clock support for Exynos7 SoC clk: samsung: exynos7: add clocks for I2C block clk: samsung: exynos7: add clocks for MMC block clk: samsung: exynos7: add clocks for RTC block clk: samsung: exynos7: add gate clocks for WDT, TMU and PWM blocks .../devicetree/bindings/clock/exynos4415-clock.txt | 38 + .../devicetree/bindings/clock/exynos7-clock.txt| 93 ++ drivers/clk/samsung/Makefile |2 + drivers/clk/samsung/clk-exynos4415.c | 1142 drivers/clk/samsung/clk-exynos5260.c | 185 +--- drivers/clk/samsung/clk-exynos7.c | 743 + drivers/clk/samsung/clk-pll.c | 25 +- drivers/clk/samsung/clk-pll.h |4 + drivers/clk/samsung/clk.c | 98 ++ drivers/clk/samsung/clk.h | 37 + include/dt-bindings/clock/exynos4415.h | 360 ++ include/dt-bindings/clock/exynos7-clk.h| 92 ++ 12 files changed, 2655 insertions(+), 164 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/exynos4415-clock.txt create mode 100644 Documentation/devicetree/bindings/clock/exynos7-clock.txt create mode 100644 drivers/clk/samsung/clk-exynos4415.c create mode 100644 drivers/clk/samsung/clk-exynos7.c create mode 100644 include/dt-bindings/clock/exynos4415.h create mode 100644 include/dt-bindings/clock/exynos7-clk.h -- Regards, Sylwester -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: exynos5420/arndale-octa: imprecise external aborts on exynos_defconfig
On Wed, Sep 17, 2014 at 5:39 PM, Kevin Hilman khil...@kernel.org wrote: Thomas Abraham ta.oma...@gmail.com writes: On Thu, Sep 11, 2014 at 12:16 AM, Kevin Hilman khil...@kernel.org wrote: Tyler Baker tyler.ba...@linaro.org writes: Exynos5420-based Arndale octa boards have recently started failing boot tests due to imprecise external aborts. This only appears to happen when using exynos_defconfig and boots fine with multi_v7_defconfig. The issue seems to be intermittent, so is not reliably reproducable and difficult to bisect. Here are a few boot logs from recent mainline/linux-next kernels that are failing: FYI, I'm seeing the same periodic aborts. For example, here's my boot of next-20140910: http://images.armcloud.us/kernel-ci/next/next-20140910/arm-exynos_defconfig/boot-exynos5420-arndale-octa.html However, my userspace is much simpler and doesn't seem to cause a panic, so my boot tests report passing. (I should fixup my scripts so these imprecise aborts are reported as a FAIL.) I'm glad you pointed out that it happens only with exynos_defconfig and not multi_v7_defconfig because I noticed that too. I haven't had the time to track it any further than that, so maybe the exynos folks can help track it down from here. Thanks for reporting this, Kevin Hi Tyler, Kevin, From the bootlog you have shared, [1.060016] CPU4: failed to come online [2.070031] CPU5: failed to come online [3.080049] CPU6: failed to come online [4.090066] CPU7: failed to come online [4.090099] Brought up 4 CPUs [4.090109] SMP: Total of 4 processors activated. [4.090119] CPU: WARNING: CPU(s) started in wrong/inconsistent modes (primary CPU mode 0x13) [4.090128] CPU: This may indicate a broken bootloader or firmware. Would it be possible to set max cpus to 1, disable switcher and try again. I don't have a arndale octa board but I have tested mainline kernel with smdk5420 board. It boots all eight CPUs, switcher works fine and there are no imprecise aborts seen. Sorry for the delay, I'm travelling this week. FWIW, the same CPU boot failures you hilight above are happening on multi_v7_defconfig[1] which is not getting the imprecise abort. This is only happening on exynos_defconfig[2], so I'm curious why you think the switcher or NR_CPUS might be the issues. Anyways, I narrowed this down a bit and discovered it's CONFIG_EXYNOS5420_MCPM=y that's the root cause. If I use exynos_defconfig and then disable that option, I don't get any more imprecise aborts. These imprecise aborts are still happening, and preventing running full userspace. I'm going to send a patch to disable this CONFIG_EXYNOS5420_MCPM until someone can figure out what's going on. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 01/14] mfd: max77686/802: Map regulator driver to its own of_node
On Fri, Oct 31, 2014 at 02:07:54PM +0100, Krzysztof Kozlowski wrote: On pią, 2014-10-31 at 12:23 +, Mark Brown wrote: I'm getting very frustrated with what's going on with these drivers, there seem to be a lot of rather large sets of patches spawning lots of discussion but also frequent review problems and very little actually getting merged (look at the set of changes in the past few merge windows for example). There's something going wrong here. If I over-spammed you, then I am deeply sorry. It's not just about volume, it's also about the fact that so little of the mail around these drivers (not just from you) is translating into code that gets merged. signature.asc Description: Digital signature
RE: [PATCH v3] usb: dwc2/gadget: report disconnect event from 'end session' irq
From: Marek Szyprowski [mailto:m.szyprow...@samsung.com]
Sent: Friday, October 31, 2014 1:04 AM
To: linux-...@vger.kernel.org; linux-samsung-soc@vger.kernel.org
Cc: Marek Szyprowski; Kyungmin Park; Robert Baldyga; Paul Zimmerman;
Krzysztof Kozlowski; Felipe Balbi
Subject: [PATCH v3] usb: dwc2/gadget: report disconnect event from 'end
session' irq
This patch adds a call to s3c_hsotg_disconnect() from 'end session'
interrupt (GOTGINT_SES_END_DET) to correctly notify gadget subsystem
about unplugged usb cable. 'disconnected' interrupt (DISCONNINT) might
look a bit more suitable for this event, but it is asserted only in
host mode, so in device mode we need to use something else.
Additional check has been added in s3c_hsotg_disconnect() function
to ensure that the event is reported only once after successful device
enumeration.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/usb/dwc2/core.h | 1 +
drivers/usb/dwc2/gadget.c | 10 ++
2 files changed, 11 insertions(+)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 55c90c53f2d6..b42df32e7737 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -212,6 +212,7 @@ struct s3c_hsotg {
struct usb_gadget gadget;
unsigned intsetup;
unsigned long last_rst;
+ unsigned intaddress;
struct s3c_hsotg_ep *eps;
};
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index fcd2bb55ccca..6304efba11aa 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -1114,6 +1114,7 @@ static void s3c_hsotg_process_control(struct s3c_hsotg
*hsotg,
DCFG_DEVADDR_SHIFT) DCFG_DEVADDR_MASK;
writel(dcfg, hsotg-regs + DCFG);
+ hsotg-address = ctrl-wValue;
dev_info(hsotg-dev, new address %d\n, ctrl-wValue);
ret = s3c_hsotg_send_reply(hsotg, ep0, NULL, 0);
@@ -2031,6 +2032,10 @@ static void s3c_hsotg_disconnect(struct s3c_hsotg
*hsotg)
{
unsigned ep;
+ if (!hsotg-address)
+ return;
+
+ hsotg-address = 0;
for (ep = 0; ep hsotg-num_of_eps; ep++)
kill_all_requests(hsotg, hsotg-eps[ep], -ESHUTDOWN, true);
@@ -2290,6 +2295,11 @@ irq_retry:
dev_info(hsotg-dev, OTGInt: %08x\n, otgint);
writel(otgint, hsotg-regs + GOTGINT);
+
+ if (otgint GOTGINT_SES_END_DET) {
+ s3c_hsotg_disconnect(hsotg);
+ hsotg-gadget.speed = USB_SPEED_UNKNOWN;
+ }
}
if (gintsts GINTSTS_SESSREQINT) {
I don't think this is right. The host can send control requests to
the device before it sends a SetAddress to change from the default
address of 0. So if a GOTGINT_SES_END_DET happens before the
SetAddress, it will be ignored.
Or am I missing something?
--
Paul
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3 1/2] usb: dwc2/gadget: add mutex to serialize init/deinit calls
From: Marek Szyprowski [mailto:m.szyprow...@samsung.com]
Sent: Friday, October 31, 2014 3:13 AM
This patch adds mutex, which protects initialization and
deinitialization procedures against suspend/resume methods.
Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
---
drivers/usb/dwc2/core.h | 1 +
drivers/usb/dwc2/gadget.c | 20
2 files changed, 21 insertions(+)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 9f77b4d1c5ff..58732a9a0019 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -187,6 +187,7 @@ struct s3c_hsotg {
struct s3c_hsotg_plat*plat;
spinlock_t lock;
+ struct mutexinit_mutex;
void __iomem*regs;
int irq;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index d8dda39c9e16..a2e4272a904e 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -21,6 +21,7 @@
#include linux/platform_device.h
#include linux/dma-mapping.h
#include linux/debugfs.h
+#include linux/mutex.h
#include linux/seq_file.h
#include linux/delay.h
#include linux/io.h
@@ -2908,6 +2909,7 @@ static int s3c_hsotg_udc_start(struct usb_gadget
*gadget,
return -EINVAL;
}
+ mutex_lock(hsotg-init_mutex);
WARN_ON(hsotg-driver);
driver-driver.bus = NULL;
@@ -2933,9 +2935,12 @@ static int s3c_hsotg_udc_start(struct usb_gadget
*gadget,
dev_info(hsotg-dev, bound driver %s\n, driver-driver.name);
+ mutex_unlock(hsotg-init_mutex);
+
return 0;
err:
+ mutex_unlock(hsotg-init_mutex);
hsotg-driver = NULL;
return ret;
}
@@ -2957,6 +2962,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,
if (!hsotg)
return -ENODEV;
+ mutex_lock(hsotg-init_mutex);
+
/* all endpoints should be shutdown */
for (ep = 1; ep hsotg-num_of_eps; ep++)
s3c_hsotg_ep_disable(hsotg-eps[ep].ep);
@@ -2974,6 +2981,8 @@ static int s3c_hsotg_udc_stop(struct usb_gadget *gadget,
clk_disable(hsotg-clk);
+ mutex_unlock(hsotg-init_mutex);
+
return 0;
}
@@ -3002,6 +3011,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget,
int is_on)
dev_dbg(hsotg-dev, %s: is_on: %d\n, __func__, is_on);
+ mutex_lock(hsotg-init_mutex);
spin_lock_irqsave(hsotg-lock, flags);
if (is_on) {
clk_enable(hsotg-clk);
@@ -3013,6 +3023,7 @@ static int s3c_hsotg_pullup(struct usb_gadget *gadget,
int is_on)
hsotg-gadget.speed = USB_SPEED_UNKNOWN;
spin_unlock_irqrestore(hsotg-lock, flags);
+ mutex_unlock(hsotg-init_mutex);
return 0;
}
@@ -3507,6 +3518,7 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
}
spin_lock_init(hsotg-lock);
+ mutex_init(hsotg-init_mutex);
hsotg-irq = ret;
@@ -3652,6 +3664,8 @@ static int s3c_hsotg_suspend(struct platform_device
*pdev, pm_message_t state)
unsigned long flags;
int ret = 0;
+ mutex_lock(hsotg-init_mutex);
+
if (hsotg-driver)
dev_info(hsotg-dev, suspending usb gadget %s\n,
hsotg-driver-driver.name);
@@ -3674,6 +3688,8 @@ static int s3c_hsotg_suspend(struct platform_device
*pdev, pm_message_t state)
clk_disable(hsotg-clk);
}
+ mutex_unlock(hsotg-init_mutex);
+
return ret;
}
@@ -3683,7 +3699,9 @@ static int s3c_hsotg_resume(struct platform_device
*pdev)
unsigned long flags;
int ret = 0;
+ mutex_lock(hsotg-init_mutex);
if (hsotg-driver) {
+
dev_info(hsotg-dev, resuming usb gadget %s\n,
hsotg-driver-driver.name);
@@ -3699,6 +3717,8 @@ static int s3c_hsotg_resume(struct platform_device
*pdev)
s3c_hsotg_core_connect(hsotg);
spin_unlock_irqrestore(hsotg-lock, flags);
+ mutex_unlock(hsotg-init_mutex);
+
return ret;
}
Hmm. I can't find any other UDC driver that uses a mutex in its
suspend/resume functions. Can you explain why this is needed only
for dwc2?
--
Paul
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] ARM: exynos_defconfig: disable CONFIG_EXYNOS5420_MCPM; not stable
From: Kevin Hilman khil...@linaro.org The option CONFIG_EXYNOS5420_MCPM is causing imprecise external aborts during boot testing, causing various userspace startup failures. Disable until it has gotten more testing. Cc: Kukjin Kim kgene@samsung.com, Cc: Javier Martinez Canillas javier.marti...@collabora.co.uk, Cc: Sachin Kamat sachin.ka...@samsung.com, Cc: Doug Anderson diand...@chromium.org, Cc: Bartlomiej Zolnierkiewicz b.zolnier...@samsung.com, Cc: Krzysztof Kozlowski k.kozlow...@samsung.com, Cc: Tushar Behera tushar.beh...@linaro.org, Cc: sta...@vger.kernel.org # v3.17+ Signed-off-by: Kevin Hilman khil...@linaro.org --- This has been reported by a few people[1], but not investigated or fixed, so it's time to disable this feature until it can be fixed. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/288344.html arch/arm/configs/exynos_defconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/configs/exynos_defconfig b/arch/arm/configs/exynos_defconfig index 72058b8a6f4d..a250dcbf34cd 100644 --- a/arch/arm/configs/exynos_defconfig +++ b/arch/arm/configs/exynos_defconfig @@ -10,7 +10,7 @@ CONFIG_MODULE_UNLOAD=y CONFIG_PARTITION_ADVANCED=y CONFIG_ARCH_EXYNOS=y CONFIG_ARCH_EXYNOS3=y -CONFIG_EXYNOS5420_MCPM=y +CONFIG_EXYNOS5420_MCPM=n CONFIG_SMP=y CONFIG_BIG_LITTLE=y CONFIG_BL_SWITCHER=y -- 2.1.0 -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 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 1/1] ARM: exynos_defconfig: Enable options for display panel support
Hello Kukjin, On Mon, Aug 25, 2014 at 10:45 AM, Javier Martinez Canillas javier.marti...@collabora.co.uk wrote: Many Exynos devices have a display panel. Most of them just have a simple panel while others have more complex configurations that requires an embedded DisplayPort (eDP) to LVDS bridges. This patch enables the following features to be built in the kernel image to suport both setups: - Direct Rendering Manager (DRM) - DRM bridge registration and lookup framework - Parade ps8622/ps8625 eDP/LVDS bridge - NXP ptn3460 eDP/LVDS bridge - Exynos Fully Interactive Mobile Display controller (FIMD) - Panel registration and lookup framework - Simple panels - Backlight LCD device support Signed-off-by: Javier Martinez Canillas javier.marti...@collabora.co.uk Tested-by: Kevin Hilman khil...@linaro.org --- This patch is needed to have display working on many Exynos boards. $subject enables the config option for the ps8622/ps8625 eDP/LVDS bridge driver but this has not landed in mainline yet. Ajay will re-spin a new revision of his series that adds this driver though, after addressing some issues pointed out on a previous version. The Kconfig symbol name (DRM_PS8622) won't change though so maybe is not crazy to pick the patch as is since CONFIG_DRM_PS8622 will have to be enabled later when the bridge driver lands. Or do you want me to re-spin $subject removing that option? Best regards, Javier -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] PM / Domains: Fix race conditions during boot
On Friday, October 31, 2014 10:16:14 AM Ulf Hansson wrote: On 24 October 2014 18:12, Kevin Hilman khil...@kernel.org wrote: Ulf Hansson ulf.hans...@linaro.org writes: Changes in v3: -Rework the entire intermediate step which was suggested in v2. That means solving the race condition, but also cope with PM domains that are initialized in powered off state. Changes in v2: -Added some acks. -Updated commit messages. -Included a wider audience of the patchset. V1 was lacking SoC maintainers. Here are link to the first patchset, were some discussion started. http://marc.info/?l=linux-pmm=141208104729597w=2 There may be more than one device in a PM domain which then will be probed at different points in time. Depending on timing and runtime PM support, in for the device related driver/subsystem, a PM domain may be advised to power off after a successful probe sequence. A general requirement for a device within a PM domain, is that the PM domain must stay powered during the probe sequence. To cope with such requirement, let's add two new APIs, dev_pm_domain_get|put(). These APIs are intended to be invoked from subsystem-level code and the calls between get/put needs to be balanced. dev_pm_domain_get(), tells the PM domain that it needs to increase a usage count and to keep supplying power. dev_pm_domain_put(), does the opposite. I'm confused. Why arent' pm_runtime_get*() and pm_runtime_put*() working? See, below. What's not explained here (or what I'm not understanding) is why a PM domain is powering off if it has active devices. It doesn't. The problem is that using pm_runtime_get_sync() in this path is not working. Now, I failed to include some of the important information from previous discussions around this patchset. Let me iterate the patchset with better commit messages, but let's first continue this thread. Here are some of the previous discussion: http://marc.info/?l=linux-pmm=141270897014653w=2 http://marc.info/?l=linux-pmm=141208104729597w=2 Below is a summary of why I think pm_runtime_get_sync() isn't working for us. 1) It's bad practice to use pm_runtime_get_sync() in the -probe() path, Honestly, I'm no longer amused. to bring your resources to full power. The consequence would be a driver that requires CONFIG_PM_RUNTIME to be even functional, which just isn't acceptable. Sorry, but this is utter nonsense. CONFIG_PM_RUNTIME unset means no runtime PM at all, so all drivers can expect everything they need to be always on. If that is not the case, then someone is doing runtime PM behind the scenes and therefore cheating. Or in different words, for CONFIG_PM_RUNTIME unset bus types, platforms etc must ensure that everything is on from the drivers' perspective. If that is the case, then calling pm_runtime_get_sync() from -probe for CONFIG_PM_RUNTIME unset simply doesn't matter. Now, for CONFIG_PM_RUNTIME enabled, if power domains are in use, doing pm_runtime_get_sync() from -probe is the only way the driver can ensure in a non-racy way that the device will be accessible going forward. Why? Simply because the probing need not happen during system initialization. It very well may take places when the other devices in the same domain have beein in use for quite a while and have been using runtime PM (in which case the domain may go off at any time unless it is explicityly prevented from doing that). One thing that you may be missing is that, for CONFIG_PM_RUNTIME set, runtime PM has to be either enabled or disabled for all devices in one domain (and if it is disabled, then the domain needs to be always on for all practical purposes). Otherwise you can't just make all of them happy at the same time. The documentation doesn't cover this, because it had been written before we even started to consider power domains. Drivers that behaves well within this context, follows the runtime PM documentation/recommendation. So please go to Documentation/power/runtime_pm.txt and read it again. Quote: If the default initial runtime PM status of the device (i.e. 'suspended') reflects the actual state of the device, its bus type's or its driver's -probe() callback will likely need to wake it up using one of the PM core's helper functions described in Section 4. In that case, pm_runtime_resume() should be used. Of course, for this purpose the device's runtime PM has to be enabled earlier by calling pm_runtime_enable(). So how is this in agreement with what you're saying, I wonder? They use pm_runtime_set_active() during -probe() to reflect that their devices are fully powered and capable of handling I/O. And how the heck can a driver (whose device belongs to a power domain) be sure that the device is fully powered and capable of handling I/O duing -probe()? You may also have a look at these discussions which also touches this topic, but within a
Re: [PATCH v3 4/9] drivercore / platform: Keep PM domain powered during -probe()
On Friday, October 31, 2014 10:23:00 AM Ulf Hansson wrote: On 31 October 2014 01:07, Dmitry Torokhov dmitry.torok...@gmail.com wrote: On Thu, Oct 30, 2014 at 01:47:27PM -0700, Kevin Hilman wrote: Ulf Hansson ulf.hans...@linaro.org writes: To sucessfully probe some devices their corresponding PM domains may need to be powered. Isn't that what pm_runtime_get*() is supposed to be doing? Why isn't that working? Also, I do not understand why we placing device into a power domain only when we probe it. Why if I unbind device from its driver (or do not have a driver for it) it disappears from its power domain? To me power domain and having driver bound to a device are 2 orthogonal concepts. That's a different discussion, I don't think we want to go there within this context. Why don't we, exactly? Rafael -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
pm_runtime_enable() in -probe() (was: Re: [PATCH v3 0/9] PM / Domains: Fix race conditions during boot)
On Saturday, November 01, 2014 01:20:38 AM Rafael J. Wysocki wrote: On Friday, October 31, 2014 10:16:14 AM Ulf Hansson wrote: On 24 October 2014 18:12, Kevin Hilman khil...@kernel.org wrote: [cut] 1) It's bad practice to use pm_runtime_get_sync() in the -probe() path, Honestly, I'm no longer amused. to bring your resources to full power. The consequence would be a driver that requires CONFIG_PM_RUNTIME to be even functional, which just isn't acceptable. Sorry, but this is utter nonsense. CONFIG_PM_RUNTIME unset means no runtime PM at all, so all drivers can expect everything they need to be always on. If that is not the case, then someone is doing runtime PM behind the scenes and therefore cheating. Or in different words, for CONFIG_PM_RUNTIME unset bus types, platforms etc must ensure that everything is on from the drivers' perspective. If that is the case, then calling pm_runtime_get_sync() from -probe for CONFIG_PM_RUNTIME unset simply doesn't matter. Now, for CONFIG_PM_RUNTIME enabled, if power domains are in use, doing pm_runtime_get_sync() from -probe is the only way the driver can ensure in a non-racy way that the device will be accessible going forward. Why? Simply because the probing need not happen during system initialization. It very well may take places when the other devices in the same domain have beein in use for quite a while and have been using runtime PM (in which case the domain may go off at any time unless it is explicityly prevented from doing that). One thing that you may be missing is that, for CONFIG_PM_RUNTIME set, runtime PM has to be either enabled or disabled for all devices in one domain (and if it is disabled, then the domain needs to be always on for all practical purposes). Otherwise you can't just make all of them happy at the same time. The documentation doesn't cover this, because it had been written before we even started to consider power domains. Drivers that behaves well within this context, follows the runtime PM documentation/recommendation. So please go to Documentation/power/runtime_pm.txt and read it again. Quote: If the default initial runtime PM status of the device (i.e. 'suspended') reflects the actual state of the device, its bus type's or its driver's -probe() callback will likely need to wake it up using one of the PM core's helper functions described in Section 4. In that case, pm_runtime_resume() should be used. Of course, for this purpose the device's runtime PM has to be enabled earlier by calling pm_runtime_enable(). All that said there is a logical error related to calling pm_runtime_enable() and its derivatives from -probe() that I've been overlooking pretty much from the start. Namely, really_probe() sets dev-driver to drv before calling -probe() for either the bus type or the driver itself, so if the driver's probe calls pm_runtime_enable(), it will execute the driver's own runtime PM resume callback before the driver can check whether or not it wants to handle the device in the first place. That doesn't sound quite right to me. This means we need a different mechanism to ensure that the device will be accessible to the driver and/or the bus type in -probe(). At one point we had pm_runtime_get_sync() before really_probe() in driver_proble_device() IIRC, but people complained about it, so we dropped it and put a barrier in there instead, but that's not sufficient. We need to explicitly ensure that the device won't be powered off while -probe() is in progress *but* we need to avoid calling the driver's runtime PM resume callback before -probe() returns. Alan, Kevin, ideas? Rafael -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: pm_runtime_enable() in -probe() (was: Re: [PATCH v3 0/9] PM / Domains: Fix race conditions during boot)
On Saturday, November 01, 2014 02:08:57 AM Rafael J. Wysocki wrote: On Saturday, November 01, 2014 01:20:38 AM Rafael J. Wysocki wrote: On Friday, October 31, 2014 10:16:14 AM Ulf Hansson wrote: On 24 October 2014 18:12, Kevin Hilman khil...@kernel.org wrote: [cut] 1) It's bad practice to use pm_runtime_get_sync() in the -probe() path, Honestly, I'm no longer amused. to bring your resources to full power. The consequence would be a driver that requires CONFIG_PM_RUNTIME to be even functional, which just isn't acceptable. Sorry, but this is utter nonsense. CONFIG_PM_RUNTIME unset means no runtime PM at all, so all drivers can expect everything they need to be always on. If that is not the case, then someone is doing runtime PM behind the scenes and therefore cheating. Or in different words, for CONFIG_PM_RUNTIME unset bus types, platforms etc must ensure that everything is on from the drivers' perspective. If that is the case, then calling pm_runtime_get_sync() from -probe for CONFIG_PM_RUNTIME unset simply doesn't matter. Now, for CONFIG_PM_RUNTIME enabled, if power domains are in use, doing pm_runtime_get_sync() from -probe is the only way the driver can ensure in a non-racy way that the device will be accessible going forward. Why? Simply because the probing need not happen during system initialization. It very well may take places when the other devices in the same domain have beein in use for quite a while and have been using runtime PM (in which case the domain may go off at any time unless it is explicityly prevented from doing that). One thing that you may be missing is that, for CONFIG_PM_RUNTIME set, runtime PM has to be either enabled or disabled for all devices in one domain (and if it is disabled, then the domain needs to be always on for all practical purposes). Otherwise you can't just make all of them happy at the same time. The documentation doesn't cover this, because it had been written before we even started to consider power domains. Drivers that behaves well within this context, follows the runtime PM documentation/recommendation. So please go to Documentation/power/runtime_pm.txt and read it again. Quote: If the default initial runtime PM status of the device (i.e. 'suspended') reflects the actual state of the device, its bus type's or its driver's -probe() callback will likely need to wake it up using one of the PM core's helper functions described in Section 4. In that case, pm_runtime_resume() should be used. Of course, for this purpose the device's runtime PM has to be enabled earlier by calling pm_runtime_enable(). All that said there is a logical error related to calling pm_runtime_enable() and its derivatives from -probe() that I've been overlooking pretty much from the start. Namely, really_probe() sets dev-driver to drv before calling -probe() for either the bus type or the driver itself, so if the driver's probe calls pm_runtime_enable(), it will execute the driver's own runtime PM resume callback before the driver can check whether or not it wants to handle the device in the first place. That doesn't sound quite right to me. This means we need a different mechanism to ensure that the device will be accessible to the driver and/or the bus type in -probe(). At one point we had pm_runtime_get_sync() before really_probe() in driver_proble_device() IIRC, but people complained about it, so we dropped it and put a barrier in there instead, but that's not sufficient. So we actually had pm_runtime_get_noresume() before the barrier, but then we dropped it due to complaints. We need to explicitly ensure that the device won't be powered off while -probe() is in progress *but* we need to avoid calling the driver's runtime PM resume callback before -probe() returns. I wonder, then, if we just need to do pm_runtime_get_sync() instead of the barrier and then pm_runtime_put() instead of pm_request_idle() in driver_probe_device()? Rafael -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] regulator: max77686: Add external GPIO control
On Fri, Oct 31, 2014 at 4:51 PM, Krzysztof Kozlowski
k.kozlow...@samsung.com wrote:
On pią, 2014-10-31 at 12:31 +0900, Alexandre Courbot wrote:
On Fri, Oct 31, 2014 at 12:03 AM, Krzysztof Kozlowski
k.kozlow...@samsung.com wrote:
On czw, 2014-10-30 at 22:56 +0900, Alexandre Courbot wrote:
Hi, and thanks for bringing this issue to us!
On Wed, Oct 29, 2014 at 7:49 PM, Javier Martinez Canillas
javier.marti...@collabora.co.uk wrote:
[adding Linus and Alexandre to the cc list]
Hello Krzysztof,
On 10/29/2014 11:42 AM, Krzysztof Kozlowski wrote:
On wto, 2014-10-28 at 13:11 +0100, Krzysztof Kozlowski wrote:
On wto, 2014-10-28 at 09:52 +0100, Krzysztof Kozlowski wrote:
On pon, 2014-10-27 at 21:03 +0100, Javier Martinez Canillas wrote:
Hello Krzysztof,
On 10/27/2014 04:03 PM, Krzysztof Kozlowski wrote:
@@ -85,6 +91,9 @@ struct max77686_data {
struct max77686_regulator_data *regulators;
int num_regulators;
+ /* Array of size num_regulators with GPIOs for external
control. */
+ int *ext_control_gpio;
+
The integer-based GPIO API is deprecated in favor of the
descriptor-based GPIO
interface (Documentation/gpio/consumer.txt). Could you please use
the later?
Sure, I can. Please have in mind that regulator core still accepts
old
GPIO so I will have to use desc_to_gpio(). That should work... and
should be future-ready.
It seems I was too hasty... I think usage of the new gpiod API implies
completely different bindings.
The gpiod_get() gets GPIO from a device level, not from given sub-node
pointer. This means that you cannot have DTS like this:
ldo21_reg: ldo21 {
regulator-compatible = LDO21;
regulator-name = VTF_2.8V;
regulator-min-microvolt = 280;
regulator-max-microvolt = 280;
ec-gpio = gpy2 0 0;
};
ldo22_reg: ldo22 {
regulator-compatible = LDO22;
regulator-name = VMEM_VDD_2.8V;
regulator-min-microvolt = 280;
regulator-max-microvolt = 280;
ec-gpio = gpk0 2 0;
};
I could put GPIOs in device node:
max77686_pmic@09 {
compatible = maxim,max77686;
interrupt-parent = gpx0;
interrupts = 7 0;
reg = 0x09;
#clock-cells = 1;
ldo21-gpio = gpy2 0 0;
ldo22-gpio = gpk0 2 0;
ldo21_reg: ldo21 {
regulator-compatible = LDO21;
regulator-name = VTF_2.8V;
regulator-min-microvolt = 280;
regulator-max-microvolt = 280;
};
ldo22_reg: ldo22 {
regulator-compatible = LDO22;
regulator-name = VMEM_VDD_2.8V;
regulator-min-microvolt = 280;
regulator-max-microvolt = 280;
};
This would work but I don't like it. The properties of a regulator are
above the node configuring that regulator.
Any ideas?
Continuing talking to myself... I found another problem - GPIO cannot
be
requested more than once (-EBUSY). In case of this driver (and board:
Trats2) one GPIO is connected to regulators. The legacy GPIO API and
regulator core handle this.
With new GPIO API I would have to implement some additional steps in
such case...
So there are 2 issues:
1. Cannot put GPIO property in regulator node.
For this problem you will probably want to use the
dev(m)_get_named_gpiod_from_child() function from the following patch:
https://lkml.org/lkml/2014/10/6/529
It should reach -next soon now.
Thanks! Probably I would switch to top level gpios property anyway
(other reasons) but it would be valuable in some cases to specify them
per child node.
Mmm, but doesn't it make more sense to have them in the child nodes?
Yes, it makes more sense. Using old way of parsing regulators from DT it
was straightforward.
However new DT style parsing (regulator_of_get_init_data()) does the
basic parsing stuff and this removes a lot of code from driver. The
driver no longer parses all regulaotrs but the regulator core does it.
Unfortunately regulator core does not parse custom bindings (like such
GPIOs) so I would have to iterate once again through all regulators just
to find gpios property.
It is simpler just to do something like (5 regulators can be controlled
by GPIO):
max77686_pmic_dt_parse_gpio_control(struct platform_device *pdev, *gpio)
{
gpio[MAX77686_LDO20] = of_get_named_gpio(np, ldo20-gpios, 0);
gpio[MAX77686_LDO21] = of_get_named_gpio(np, ldo21-gpios, 0);
gpio[MAX77686_LDO22] = of_get_named_gpio(np, ldo22-gpios, 0);
gpio[MAX77686_BUCK8] = of_get_named_gpio(np, buck8-gpios, 0);
gpio[MAX77686_BUCK9] = of_get_named_gpio(np, buck9-gpios, 0);
}
It is simpler from the driver's perspective, but if I understand
correctly DT bindings are not
