date:20160731

Re: [v3,2/6] power: add power sequence library

2016-07-31 Thread Peter Chen

On Fri, Jul 29, 2016 at 01:06:48PM -0700, Matthias Kaehlcke wrote:
> Hi Peter,
> 
> Thanks for your work on this, a few comments inline
> 
> 
> On 07/20/2016 02:40 AM, Peter Chen wrote:
> 
> >...
> >
> >+static int pwrseq_generic_on(struct device_node *np, struct pwrseq *pwrseq)
> >+{
> >
> >...
> >
> >+if (gpiod_reset) {
> >+u32 duration_us = 50;
> >+
> >+of_property_read_u32(np, "reset-duration-us",
> >+_us);
> >+usleep_range(duration_us, duration_us + 10);
> The end of the range could allow for more margin. Also consider busy
> looping for very short delays as in
> http://lxr.free-electrons.com/source/drivers/regulator/core.c#L2062

Thanks, I will change it.

> >...
> >
> >+static int pwrseq_generic_get(struct device_node *np, struct pwrseq *pwrseq)
> >+{
> >+struct pwrseq_generic *pwrseq_gen = to_generic_pwrseq(pwrseq);
> >+enum of_gpio_flags flags;
> >+int reset_gpio, ret = 0;
> >+
> >+pwrseq_gen->clk = of_clk_get_by_name(np, NULL);
> This only gets the first of potentially multiple clocks, is that intended?

Since it is ran before the driver's probe, we thought one clock for
power sequence is enough. If your case really needs several clocks
to be enabled before your device can be found by bus, let me know.
I will add support for it. But what are the name for clocks, since
it is generic library? "gen1, gen2 and gen3"?

-- 

Best Regards,
Peter Chento 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: pwc over musb: 100% frame drop (lost) on high resolution stream

2016-07-31 Thread Matwey V. Kornilov

Hello,

I've also just found that the same commit breaks cpufreq on BeagleBone Black :)

So, probably without HCD_BH flag musb works correctly only at 1Ghz CPU
frequency, which is unlisted and being set to 720Mhz by cpufreq driver
(as it did whet there was cpufreq driver).

2016-07-29 21:01 GMT+03:00 Matwey V. Kornilov :
> Hello,
>
> I've found that the following commit fixes the issue:
>
> commit 7694ca6e1d6f01122f05039b81f70f64b1ec4063
> Author: Viresh Kumar 
> Date:   Fri Apr 22 16:58:42 2016 +0530
>
> cpufreq: omap: Use generic platdev driver
>
> The cpufreq-dt-platdev driver supports creation of cpufreq-dt platform
> device now, reuse that and remove similar code from platform code.
>
>
> 2016-07-28 19:16 GMT+03:00 Matwey V. Kornilov :
>> Hello,
>>
>> I've just bisected commit, which fixed the issue in v4.7
>>
>> commit 9fa64d6424adabf0e3a546ae24d01a62a927b342
>> Merge: f55532a febce40
>> Author: Rafael J. Wysocki 
>> Date:   Sat Apr 2 01:07:17 2016 +0200
>>
>> Merge back intel_pstate fixes for v4.6.
>>
>> * pm-cpufreq:
>>   intel_pstate: Avoid extra invocation of intel_pstate_sample()
>>   intel_pstate: Do not set utilization update hook too early
>>
>> Unfortunately, intel_pstate branch doesn't have
>> f551e13529833e052f75ec628a8af7b034af20f9 applied.
>> I'll try to bisect this branch with the patch manually applied.
>>
>> 2016-07-27 20:34 GMT+03:00 Matwey V. Kornilov :
>>> Hello,
>>>
>>> I've just biseced commit, which introduced this issue
>>>
>>> commit f551e13529833e052f75ec628a8af7b034af20f9
>>> Author: Bin Liu 
>>> Date:   Mon Apr 25 15:53:30 2016 -0500
>>>
>>> Revert "usb: musb: musb_host: Enable HCD_BH flag to handle urb
>>> return in bottom half"
>>>
>>> I have not checked yet, if it was intentionnaly fixed.
>>>
>>> 2016-07-23 22:24 GMT+03:00 Matwey V. Kornilov :
 2016-07-20 21:56 GMT+03:00 Matwey V. Kornilov :
> 2016-07-20 18:06 GMT+03:00 Bin Liu :
>> Hi,
>>
>> On Wed, Jul 20, 2016 at 05:44:56PM +0300, Matwey V. Kornilov wrote:
>>> 2016-07-20 17:13 GMT+03:00 Bin Liu :
>>> > Hi,
>>> >
>>> > On Wed, Jul 20, 2016 at 09:09:42AM +0300, Matwey V. Kornilov wrote:
>>> >> 2016-07-20 0:34 GMT+03:00 Bin Liu :
>>> >> > Hi,
>>> >> >
>>> >> > On Wed, Jul 20, 2016 at 12:25:44AM +0300, Matwey V. Kornilov wrote:
>>> >> >> 2016-07-19 23:56 GMT+03:00 Bin Liu :
>>> >> >> > Hi,
>>> >> >> >
>>> >> >> > On Tue, Jul 19, 2016 at 11:21:17PM +0300, mat...@sai.msu.ru 
>>> >> >> > wrote:
>>> >> >> >> Hello,
>>> >> >> >>
>>> >> >> >> I have Philips SPC 900 camera (0471:0329) connected to my 
>>> >> >> >> AM335x based BeagleBoneBlack SBC.
>>> >> >> >> I am sure that both of them are fine and work properly.
>>> >> >> >> I am running Linux 4.6.4 (my kernel config is available at 
>>> >> >> >> https://clck.ru/A2kQs ) and I've just discovered, that there 
>>> >> >> >> is an issue with frame transfer when high resolution formats 
>>> >> >> >> are used.
>>> >> >> >>
>>> >> >> >> The issue is the following. I use simple v4l2 example tool 
>>> >> >> >> (taken from API docs), which source code is available at 
>>> >> >> >> http://pastebin.com/grcNXxfe
>>> >> >> >>
>>> >> >> >> When I use (see line 488) 640x480 frames
>>> >> >> >>
>>> >> >> >> fmt.fmt.pix.width   = 640;
>>> >> >> >> fmt.fmt.pix.height  = 480;
>>> >> >> >>
>>> >> >> >> then I get "select timeout" and don't get any frames.
>>> >> >> >>
>>> >> >> >> When I use 320x240 frames
>>> >> >> >>
>>> >> >> >> fmt.fmt.pix.width   = 320;
>>> >> >> >> fmt.fmt.pix.height  = 240;
>>> >> >> >>
>>> >> >> >> then about 60% frames are missed. An example outpout of 
>>> >> >> >> ./a.out -f is available at https://yadi.sk/d/aRka8xWPtSc4y
>>> >> >> >> It looks like there are pauses between bulks of frames (frame 
>>> >> >> >> counter and timestamp as returned from v4l2 API):
>>> >> >> >>
>>> >> >> >> 3 3705.142553
>>> >> >> >> 8 3705.342533
>>> >> >> >> 13 3705.542517
>>> >> >> >> 110 3708.776208
>>> >> >> >> 115 3708.976190
>>> >> >> >> 120 3709.176169
>>> >> >> >> 125 3709.376152
>>> >> >> >> 130 3709.576144
>>> >> >> >> 226 3712.807848
>>> >> >> >>
>>> >> >> >> When I use tiny 160x120 frames
>>> >> >> >>
>>> >> >> >> fmt.fmt.pix.width   = 160;
>>> >> >> >> fmt.fmt.pix.height  = 120;
>>> >> >> >>
>>> >> >> >> then more frames are received. See output example at 
>>> >> >> >> https://yadi.sk/d/DedBmH6ftSc9t
>>> >> >> >> That is why I thought that everything

Re: [PATCH 2/3] phy: sun4i: add support for A64 usb phy

2016-07-31 Thread Hans de Goede


Hi,

On 31-07-16 16:50, Chen-Yu Tsai wrote:


FYI: H3 USB PHY support is not complete. USB0 PHY is not supported, and
it does not work. I did a preliminary comparison of this PHY driver and
the code in Allwinner's SDK. There are some bits missing.


Right that is a known issue, I believe someone was working on an
otg support patch series for the H3 though ?

Regards,

Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] phy: sun4i: add support for A64 usb phy

2016-07-31 Thread Chen-Yu Tsai

Hi,

On Sun, Jul 31, 2016 at 10:39 PM, Hans de Goede  wrote:
> Hi,
>
> On 31-07-16 13:25, Icenowy Zheng wrote:
>>
>> There's something unknown in the pmu part.
>>
>> Signed-off-by: Icenowy Zheng 
>
>
> Cool, I really like the work you're doing on A64 support,
> keep up the good work!
>
>> ---
>>  drivers/phy/phy-sun4i-usb.c | 21 +++--
>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
>> index 0a45bc6..6f94369 100644
>> --- a/drivers/phy/phy-sun4i-usb.c
>> +++ b/drivers/phy/phy-sun4i-usb.c
>> @@ -97,6 +97,7 @@ enum sun4i_usb_phy_type {
>> sun6i_a31_phy,
>> sun8i_a33_phy,
>> sun8i_h3_phy,
>> +   sun50i_a64_phy,
>>  };
>>
>>  struct sun4i_usb_phy_cfg {
>> @@ -180,8 +181,9 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy
>> *phy, u32 addr, u32 data,
>>
>> mutex_lock(_data->mutex);
>>
>> -   if (phy_data->cfg->type == sun8i_a33_phy) {
>> -   /* A33 needs us to set phyctl to 0 explicitly */
>> +   if (phy_data->cfg->type == sun8i_a33_phy ||
>> +   phy_data->cfg->type == sun50i_a64_phy) {
>> +   /* A33 or A64 needs us to set phyctl to 0 explicitly */
>> writel(0, phyctl);
>> }
>>
>
> Maybe add a "bool explicitly_0_phyctl;" to sun4i_usb_phy_cfg ?
>
> Note I'm not sure of this myself, feel free to keep this as is,
> we can always introduce such a bool when we get a 3th SoC which
> needs it.
>
>> @@ -264,6 +266,12 @@ static int sun4i_usb_phy_init(struct phy *_phy)
>> val = readl(phy->pmu + REG_PMU_UNK_H3);
>> writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
>> } else {
>> +   /* A64 needs also this unknown bit */
>> +   if (data->cfg->type == sun50i_a64_phy) {
>> +   val = readl(phy->pmu + REG_PMU_UNK_H3);
>> +   writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
>> +   }
>> +
>> /* Enable USB 45 Ohm resistor calibration */
>> if (phy->index == 0)
>> sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01,
>> 1);
>
>
> Erm, as pointed out thus duplicates code from the H3 code path, I believe
> that you should add a "bool needs_h3_pmu_unk_poke;" to sun4i_usb_phy_cfg
> and then change this bit of the code to:
>
> if (data->cfg->needs_h3_pmu_unk_poke) {
> val = readl(phy->pmu + REG_PMU_UNK_H3);
> writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
> }
>
> if (data->cfg->type == sun8i_h3_phy) {
> if (phy->index == 0) {
> val = readl(data->base + REG_PHY_UNK_H3);
> writel(val & ~1, data->base + REG_PHY_UNK_H3);
> }
> } else {
> ... (original code)
> }
>
> That seems like a cleaner solution to me.
>
> And do not forget to set the needs_h3_pmu_unk_poke for the h3!
>
> I would not add it to the sun4i_usb_phy_cfg structs for the
> other type SoCs, if part of the struct is initialized the
> rest will get set to 0 by the compiler and I believe that
> it things will be more readable without an explicit:
>
> .needs_h3_pmu_unk_poke = false
>
> Everywhere.
>

FYI: H3 USB PHY support is not complete. USB0 PHY is not supported, and
it does not work. I did a preliminary comparison of this PHY driver and
the code in Allwinner's SDK. There are some bits missing.

ChenYu

>
> Thanks & Regards,
>
> Hans
>
>
>
>
>
>> @@ -762,6 +770,14 @@ static const struct sun4i_usb_phy_cfg sun8i_h3_cfg =
>> {
>> .dedicated_clocks = true,
>>  };
>>
>> +static const struct sun4i_usb_phy_cfg sun50i_a64_cfg = {
>> +   .num_phys = 2,
>> +   .type = sun50i_a64_phy,
>> +   .disc_thresh = 3,
>> +   .phyctl_offset = REG_PHYCTL_A33,
>> +   .dedicated_clocks = true,
>> +};
>> +
>>  static const struct of_device_id sun4i_usb_phy_of_match[] = {
>> { .compatible = "allwinner,sun4i-a10-usb-phy", .data =
>> _a10_cfg },
>> { .compatible = "allwinner,sun5i-a13-usb-phy", .data =
>> _a13_cfg },
>> @@ -770,6 +786,7 @@ static const struct of_device_id
>> sun4i_usb_phy_of_match[] = {
>> { .compatible = "allwinner,sun8i-a23-usb-phy", .data =
>> _a23_cfg },
>> { .compatible = "allwinner,sun8i-a33-usb-phy", .data =
>> _a33_cfg },
>> { .compatible = "allwinner,sun8i-h3-usb-phy", .data =
>> _h3_cfg },
>> +   { .compatible = "allwinner,sun50i-a64-usb-phy", .data =
>> _a64_cfg},
>> { },
>>  };
>>  MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] phy: sun4i: add support for A64 usb phy

2016-07-31 Thread Hans de Goede


Hi,

On 31-07-16 13:25, Icenowy Zheng wrote:

There's something unknown in the pmu part.

Signed-off-by: Icenowy Zheng 


Cool, I really like the work you're doing on A64 support,
keep up the good work!


---
 drivers/phy/phy-sun4i-usb.c | 21 +++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/phy-sun4i-usb.c b/drivers/phy/phy-sun4i-usb.c
index 0a45bc6..6f94369 100644
--- a/drivers/phy/phy-sun4i-usb.c
+++ b/drivers/phy/phy-sun4i-usb.c
@@ -97,6 +97,7 @@ enum sun4i_usb_phy_type {
sun6i_a31_phy,
sun8i_a33_phy,
sun8i_h3_phy,
+   sun50i_a64_phy,
 };

 struct sun4i_usb_phy_cfg {
@@ -180,8 +181,9 @@ static void sun4i_usb_phy_write(struct sun4i_usb_phy *phy, 
u32 addr, u32 data,

mutex_lock(_data->mutex);

-   if (phy_data->cfg->type == sun8i_a33_phy) {
-   /* A33 needs us to set phyctl to 0 explicitly */
+   if (phy_data->cfg->type == sun8i_a33_phy ||
+   phy_data->cfg->type == sun50i_a64_phy) {
+   /* A33 or A64 needs us to set phyctl to 0 explicitly */
writel(0, phyctl);
}



Maybe add a "bool explicitly_0_phyctl;" to sun4i_usb_phy_cfg ?

Note I'm not sure of this myself, feel free to keep this as is,
we can always introduce such a bool when we get a 3th SoC which
needs it.


@@ -264,6 +266,12 @@ static int sun4i_usb_phy_init(struct phy *_phy)
val = readl(phy->pmu + REG_PMU_UNK_H3);
writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
} else {
+   /* A64 needs also this unknown bit */
+   if (data->cfg->type == sun50i_a64_phy) {
+   val = readl(phy->pmu + REG_PMU_UNK_H3);
+   writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
+   }
+
/* Enable USB 45 Ohm resistor calibration */
if (phy->index == 0)
sun4i_usb_phy_write(phy, PHY_RES45_CAL_EN, 0x01, 1);


Erm, as pointed out thus duplicates code from the H3 code path, I believe
that you should add a "bool needs_h3_pmu_unk_poke;" to sun4i_usb_phy_cfg
and then change this bit of the code to:

if (data->cfg->needs_h3_pmu_unk_poke) {
val = readl(phy->pmu + REG_PMU_UNK_H3);
writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
}

if (data->cfg->type == sun8i_h3_phy) {
if (phy->index == 0) {
val = readl(data->base + REG_PHY_UNK_H3);
writel(val & ~1, data->base + REG_PHY_UNK_H3);
}
} else {
... (original code)
}

That seems like a cleaner solution to me.

And do not forget to set the needs_h3_pmu_unk_poke for the h3!

I would not add it to the sun4i_usb_phy_cfg structs for the
other type SoCs, if part of the struct is initialized the
rest will get set to 0 by the compiler and I believe that
it things will be more readable without an explicit:

.needs_h3_pmu_unk_poke = false

Everywhere.


Thanks & Regards,

Hans





@@ -762,6 +770,14 @@ static const struct sun4i_usb_phy_cfg sun8i_h3_cfg = {
.dedicated_clocks = true,
 };

+static const struct sun4i_usb_phy_cfg sun50i_a64_cfg = {
+   .num_phys = 2,
+   .type = sun50i_a64_phy,
+   .disc_thresh = 3,
+   .phyctl_offset = REG_PHYCTL_A33,
+   .dedicated_clocks = true,
+};
+
 static const struct of_device_id sun4i_usb_phy_of_match[] = {
{ .compatible = "allwinner,sun4i-a10-usb-phy", .data = _a10_cfg },
{ .compatible = "allwinner,sun5i-a13-usb-phy", .data = _a13_cfg },
@@ -770,6 +786,7 @@ static const struct of_device_id sun4i_usb_phy_of_match[] = 
{
{ .compatible = "allwinner,sun8i-a23-usb-phy", .data = _a23_cfg },
{ .compatible = "allwinner,sun8i-a33-usb-phy", .data = _a33_cfg },
{ .compatible = "allwinner,sun8i-h3-usb-phy", .data = _h3_cfg },
+   { .compatible = "allwinner,sun50i-a64-usb-phy", .data = 
_a64_cfg},
{ },
 };
 MODULE_DEVICE_TABLE(of, sun4i_usb_phy_of_match);


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

Re: [PATCH 2/3] phy: sun4i: add support for A64 usb phy

2016-07-31 Thread Amit Tomer

Hello ,

> @@ -264,6 +266,12 @@ static int sun4i_usb_phy_init(struct phy *_phy)
> val = readl(phy->pmu + REG_PMU_UNK_H3);
> writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
> } else {
> +   /* A64 needs also this unknown bit */
> +   if (data->cfg->type == sun50i_a64_phy) {
> +   val = readl(phy->pmu + REG_PMU_UNK_H3);
> +   writel(val & ~2, phy->pmu + REG_PMU_UNK_H3);
> +   }
> +

This bit is also set for H3, shall we reuse it or we should enclose it
in else-if part ?

Thanks
Amit.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [v3,2/6] power: add power sequence library

Re: pwc over musb: 100% frame drop (lost) on high resolution stream

Re: [PATCH 2/3] phy: sun4i: add support for A64 usb phy

Re: [PATCH 2/3] phy: sun4i: add support for A64 usb phy

Re: [PATCH 2/3] phy: sun4i: add support for A64 usb phy

Re: [PATCH 2/3] phy: sun4i: add support for A64 usb phy

6 matches

Site Navigation

Mail list logo

Footer information