Re: staging:DWC2 USB driver issues

[Pavel Machek]

On Tue 2013-08-27 12:22:59, Matthijs Kooijman wrote:
 Hi Dinh,
 
  Any chance anyone has a similar experience with this DWC2 driver, any
  help will greatly appreciated. Of course, I will go back and verify the
  initialization between the DWC2 and the old driver to see if I can spot
  anything.
 At first glance, the symptoms (getting transaction errors on every
 transaction attempt) looks similar to what I was seeing a while back:
 
 http://comments.gmane.org/gmane.linux.usb.general/86117
 
 However, in my case it seems there was some hardware issue (possibly
 even a hardware fault in a single unit) that caused it, so I doubt this
 will really help you...
 
 
 What hardware are you working with?

Very probably Altera Socfpga Cyclone V board.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: staging:DWC2 USB driver issues

[Pavel Machek]

Hi!

 I was wondering if anyone has come across the problem I am experiencing
 with the staging DWC2 driver. The problem is that the driver is failing
 to detect a device when connected. 
 
 I know that HW works because I have an older version of the driver for
 this IP and it seems to work OK, barring a few issues(DMA, Split
 Transactions), this older driver is able to function adequately on a
 mass storage device.
 
 But the DWC2 driver is not able to detect a device at all, unless I
 force it into HOST mode only. With Host mode only, its able to see a
 device but fails to enumerate correctly. 

What needs to be done  to get DWC2 to detect?

So far I tried checking out your 

g...@github.com:dinh-linux/linux-socfpga.git for-next

tree, and patching it so that VIRT_TO_BUS is configurable. But I
suspect I still need patch for dts?

Thanks,
Pavel 

diff --git a/mm/Kconfig b/mm/Kconfig
index 8028dcc..84bbc9d 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -292,7 +292,7 @@ config NR_QUICK
default 1
 
 config VIRT_TO_BUS
-   bool
+   bool Virt_to_bus
help
  An architecture should select this if it implements the
  deprecated interface virt_to_bus().  All new architectures


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Fix style in s3c-hsotg.c

[Pavel Machek]

Hi!

checkpatch.pl has some valid complaints about style in s3c-hsotg.c :
macro with if should be really enclosed in do {} while, and puts is
going to be slightly faster.

Here's suggested patch. I don't have the hardware, so it is completely
untested.

Signed-off-by: Pavel Machek, pa...@denx.de

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index af22f24..f8e762a 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -2091,12 +2091,14 @@ static void kill_all_requests(struct s3c_hsotg *hsotg,
 }
 
 #define call_gadget(_hs, _entry) \
+do { \
if ((_hs)-gadget.speed != USB_SPEED_UNKNOWN  \
(_hs)-driver  (_hs)-driver-_entry) { \
spin_unlock(_hs-lock); \
(_hs)-driver-_entry((_hs)-gadget); \
spin_lock(_hs-lock); \
}
+} while (0)
 
 /**
  * s3c_hsotg_disconnect - disconnect service
@@ -3204,7 +3206,7 @@ static int state_show(struct seq_file *seq, void *v)
   readl(regs + GNPTXSTS),
   readl(regs + GRXSTSR));
 
-   seq_printf(seq, \nEndpoint status:\n);
+   seq_puts(seq, \nEndpoint status:\n);
 
for (idx = 0; idx  15; idx++) {
u32 in, out;
@@ -3221,7 +3223,7 @@ static int state_show(struct seq_file *seq, void *v)
seq_printf(seq, , DIEPTSIZ=0x%08x, DOEPTSIZ=0x%08x,
   in, out);
 
-   seq_printf(seq, \n);
+   seq_puts(seq, \n);
}
 
return 0;
@@ -3255,7 +3257,7 @@ static int fifo_show(struct seq_file *seq, void *v)
u32 val;
int idx;
 
-   seq_printf(seq, Non-periodic FIFOs:\n);
+   seq_puts(seq, Non-periodic FIFOs:\n);
seq_printf(seq, RXFIFO: Size %d\n, readl(regs + GRXFSIZ));
 
val = readl(regs + GNPTXFSIZ);
@@ -3263,7 +3265,7 @@ static int fifo_show(struct seq_file *seq, void *v)
   val  GNPTXFSIZ_NPTxFDep_SHIFT,
   val  GNPTXFSIZ_NPTxFStAddr_MASK);
 
-   seq_printf(seq, \nPeriodic TXFIFOs:\n);
+   seq_puts(seq, \nPeriodic TXFIFOs:\n);
 
for (idx = 1; idx = 15; idx++) {
val = readl(regs + DPTXFSIZn(idx));
@@ -3334,7 +3336,7 @@ static int ep_show(struct seq_file *seq, void *v)
   readl(regs + DIEPTSIZ(index)),
   readl(regs + DOEPTSIZ(index)));
 
-   seq_printf(seq, \n);
+   seq_puts(seq, \n);
seq_printf(seq, mps %d\n, ep-ep.maxpacket);
seq_printf(seq, total_data=%ld\n, ep-total_data);
 
@@ -3345,7 +3347,7 @@ static int ep_show(struct seq_file *seq, void *v)
 
list_for_each_entry(req, ep-queue, queue) {
if (--show_limit  0) {
-   seq_printf(seq, not showing more requests...\n);
+   seq_puts(seq, not showing more requests...\n);
break;
}
 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Fix style in s3c-hsotg.c

[Pavel Machek]

On Tue 2013-09-17 10:42:30, Felipe Balbi wrote:
 Hi,
 
 On Mon, Sep 02, 2013 at 03:58:32PM +0200, Pavel Machek wrote:
  Hi!
  
  checkpatch.pl has some valid complaints about style in s3c-hsotg.c :
  macro with if should be really enclosed in do {} while, and puts is
  going to be slightly faster.
  
  Here's suggested patch. I don't have the hardware, so it is completely
  untested.
  
  Signed-off-by: Pavel Machek, pa...@denx.de
 
 this is not how you send a patch, please read
 Documentation/SubmittingPatches

Have you considered possibility that this is how you nudge maintainer
into fix their coding style?

Thanks,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Fix style in s3c-hsotg.c

[Pavel Machek]

On Tue 2013-09-17 20:45:39, Felipe Balbi wrote:
 On Wed, Sep 18, 2013 at 12:04:27AM +0200, Pavel Machek wrote:
  On Tue 2013-09-17 10:42:30, Felipe Balbi wrote:
   Hi,
   
   On Mon, Sep 02, 2013 at 03:58:32PM +0200, Pavel Machek wrote:
Hi!

checkpatch.pl has some valid complaints about style in s3c-hsotg.c :
macro with if should be really enclosed in do {} while, and puts is
going to be slightly faster.

Here's suggested patch. I don't have the hardware, so it is completely
untested.

Signed-off-by: Pavel Machek, pa...@denx.de
   
   this is not how you send a patch, please read
   Documentation/SubmittingPatches
  
  Have you considered possibility that this is how you nudge maintainer
  into fix their coding style?
 
 cute...
 
 Seriously though, read that file, you're commit log has garbage in it
 which shouldn't go to git history.

Run git log on SubmittingPatches.

Then, instead of telling me what to read, run checkpatch on your
files. You can either fix them yourself, or use my patch as a
basis. Note there's missing } or something, so it probably will not
compile, see the other mail. So you actually will have to modify that
patch. Stripping Hi! from it should not be that hard, neither should
be stripping note that patch is untested when you actually test
it. And as you are the maintainer, it is your job.

No, I'll not polish patch for hardware I don't have and have little
interest in.  wanted to help you, but according to your first reply,
you do not really want help.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-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 1/4] usb: musb: Call atomic_notifier_call_chain when status is changed

[Pavel Machek]

Hi!

   So will you do that? Or it is needed to resend this one line
   hunk again in new email again?
 
  new patch, new email
 
  Guys, WHY ARE YOU SO STUPID AND ARROGANT?
 
  Sorry but, need to copy full isolated patch/hunk from one mail to
  another is hassling. So what you want from me? Do all those non
  sense working only because yesterday you had bad day? Or what?
...
 
 Hi Pali,
 
 There is no need to be rude.
 
 Felipe asked you to do the split since he believes that the notifier
 chain call for musb xceiv and the twl-phy notifier head init should
 be done in two separate patches.

Actually, there is need to be rude, because Felipe fails to act as
maintainer. Instead of fixing bugs in his code, he bounces bugfix
patches, points people to random READMEs and wastes everyones time.
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-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 1/4] usb: musb: Call atomic_notifier_call_chain when status is changed

[Pavel Machek]

Hi!

So will you do that? Or it is needed to resend this one line
hunk again in new email again?
  
   new patch, new email
  
   Guys, WHY ARE YOU SO STUPID AND ARROGANT?
  
   Sorry but, need to copy full isolated patch/hunk from one mail to
   another is hassling. So what you want from me? Do all those non
   sense working only because yesterday you had bad day? Or what?
...
  Actually, there is need to be rude, because Felipe fails to act as
  maintainer. Instead of fixing bugs in his code, he bounces bugfix
  patches, points people to random READMEs and wastes everyones time.
 
 I don't know what are you talking about (if that happened in another
 thread then I need more context). Felipe is not bouncing any bugfix

Take a look here:

https://lkml.org/lkml/2013/9/17/286

I clearly state that patch can not be tested as required for proper
submission, but offer patch anyway. I get irrelevant boilerplate on
patch format.

 but just asked to split the patch in two since the patch was solving
 two separate issues so is way better to have it in two separate
 patches for the reasons I explained before.
 
 So, as far as I can tell Felipe did exactly what I would expect from a
 maintainer. He took the time to review the patches sent to him and

I'd expect maintainer to, well, maintain code. It means actually
fixing bugs in his code, when he's pointed at them.

 gave feedback. If the sender doesn't want to take his feedback into
 account and prefer to send pretty insulting emails instead that is his
 choice but I would say that is this not the greatest approach to get
 your code merged (to say the least).

Clearly not. But Pali found bug in code Felipe should
maintain. Instead of thank you for bug report, I applied this one
line of your code to fix it, Pali got new patch, new email for his
efforts. That is how you train dogs, not how you should treat kernel
contributors.

Now, it is possible that Felipe just has problems with english, as he
called me piece of wood in https://lkml.org/lkml/2013/9/17/476 , but
he appears more arogant than usual over email.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-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 1/4] usb: musb: Call atomic_notifier_call_chain when status is changed

[Pavel Machek]

Hi!

  gave feedback. If the sender doesn't want to take his feedback into
  account and prefer to send pretty insulting emails instead that is his
  choice but I would say that is this not the greatest approach to get
  your code merged (to say the least).
 
 Clearly not. But Pali found bug in code Felipe should
 maintain. Instead of thank you for bug report, I applied this one
 line of your code to fix it, Pali got new patch, new email for his
 efforts. That is how you train dogs, not how you should treat kernel
 contributors.
 
 Now, it is possible that Felipe just has problems with english, as he
 called me piece of wood in https://lkml.org/lkml/2013/9/17/476 , but
 he appears more arogant than usual over email.

Actually he called me piece of wood with garbage in it. I guess I
have right to be offended. I'm baby in the next email. Hmm.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Fix style in s3c-hsotg.c

[Pavel Machek]

Hi!

   No, I'll not polish patch for hardware I don't have and have little
   interest in.  wanted to help you, but according to your first reply,
   you do not really want help.
  
  that's your call. Now how about you stop being such a baby and go fix
  your mistakes to start with ? Just because I'm the maintainer of the
  gadget framework, doesn't mean I'm the maintainer of s3c-hsotc.c file.
  Maintainer != author too, btw.
  
  Anyway, I got much better stuff to do than babysitting grown ups.
 
 ..even if it could have been communicated in a gentler way.
 
 Anyway s3c-hsotg.c is a driver for our hardware so I'm going to pick this
 patch up, polish it and then re-submit it later. Thanks.

Here's version that should compile. Thanks,
Pavel

Signed-off-by: Pavel Machek pa...@ucw.cz

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index d69b36a..c8cdde0 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -2092,12 +2092,14 @@ static void kill_all_requests(struct s3c_hsotg *hsotg,
 }
 
 #define call_gadget(_hs, _entry) \
+do { \
if ((_hs)-gadget.speed != USB_SPEED_UNKNOWN  \
(_hs)-driver  (_hs)-driver-_entry) { \
spin_unlock(_hs-lock); \
(_hs)-driver-_entry((_hs)-gadget); \
spin_lock(_hs-lock); \
-   }
+   } \
+} while (0)
 
 /**
  * s3c_hsotg_disconnect - disconnect service
@@ -3205,7 +3207,7 @@ static int state_show(struct seq_file *seq, void *v)
   readl(regs + GNPTXSTS),
   readl(regs + GRXSTSR));
 
-   seq_printf(seq, \nEndpoint status:\n);
+   seq_puts(seq, \nEndpoint status:\n);
 
for (idx = 0; idx  15; idx++) {
u32 in, out;
@@ -3222,7 +3224,7 @@ static int state_show(struct seq_file *seq, void *v)
seq_printf(seq, , DIEPTSIZ=0x%08x, DOEPTSIZ=0x%08x,
   in, out);
 
-   seq_printf(seq, \n);
+   seq_puts(seq, \n);
}
 
return 0;
@@ -3256,7 +3258,7 @@ static int fifo_show(struct seq_file *seq, void *v)
u32 val;
int idx;
 
-   seq_printf(seq, Non-periodic FIFOs:\n);
+   seq_puts(seq, Non-periodic FIFOs:\n);
seq_printf(seq, RXFIFO: Size %d\n, readl(regs + GRXFSIZ));
 
val = readl(regs + GNPTXFSIZ);
@@ -3264,7 +3266,7 @@ static int fifo_show(struct seq_file *seq, void *v)
   val  GNPTXFSIZ_NPTxFDep_SHIFT,
   val  GNPTXFSIZ_NPTxFStAddr_MASK);
 
-   seq_printf(seq, \nPeriodic TXFIFOs:\n);
+   seq_puts(seq, \nPeriodic TXFIFOs:\n);
 
for (idx = 1; idx = 15; idx++) {
val = readl(regs + DPTXFSIZn(idx));
@@ -3335,7 +3337,7 @@ static int ep_show(struct seq_file *seq, void *v)
   readl(regs + DIEPTSIZ(index)),
   readl(regs + DOEPTSIZ(index)));
 
-   seq_printf(seq, \n);
+   seq_puts(seq, \n);
seq_printf(seq, mps %d\n, ep-ep.maxpacket);
seq_printf(seq, total_data=%ld\n, ep-total_data);
 
@@ -3346,7 +3348,7 @@ static int ep_show(struct seq_file *seq, void *v)
 
list_for_each_entry(req, ep-queue, queue) {
if (--show_limit  0) {
-   seq_printf(seq, not showing more requests...\n);
+   seq_puts(seq, not showing more requests...\n);
break;
}
 


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-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 usb 1/2] usb: musb: Add missing ATOMIC_INIT_NOTIFIER_HEAD

[Pavel Machek]

On Wed 2013-09-25 15:33:33, Felipe Balbi wrote:
 Hi,
 
 On Wed, Sep 25, 2013 at 10:17:38AM +0200, Pali Rohár wrote:
  On Wednesday 18 September 2013 19:03:33 Pali Rohár wrote:
   twl-phy.notifier is not initalized
   
   Signed-off-by: Pali Rohár pali.ro...@gmail.com
   
   diff --git a/drivers/usb/phy/phy-twl4030-usb.c
   b/drivers/usb/phy/phy-twl4030-usb.c index 8f78d2d..efe6155
   100644
   --- a/drivers/usb/phy/phy-twl4030-usb.c
   +++ b/drivers/usb/phy/phy-twl4030-usb.c
   @@ -705,6 +705,8 @@ static int twl4030_usb_probe(struct
   platform_device *pdev) if (device_create_file(pdev-dev,
   dev_attr_vbus)) dev_warn(pdev-dev, could not create sysfs
   file\n);
   
   + ATOMIC_INIT_NOTIFIER_HEAD(twl-phy.notifier);
   +
 /* Our job is to use irqs and status from the power module
  * to keep the transceiver disabled when nothing's
   connected. *
  
  I sent above patch week ago. Did you already included it?
 
 look at my testing branch.

Felipe prefers you to go MAINTAINERS file for

git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git

entry. After few clicks, you'll find out your two patches in 

https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/log/?h=testing

. So yes, it seems that after 10+ flames Felipe was _not_ lazy to
send, Thanks for a patch mail was too much work.

Apparently, we'll need  Documentation/HowToBeGoodMaintainer file...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-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] usb: gadget: nokia: Add mass storage driver to g_nokia

[Pavel Machek]

On Mon 2013-01-21 10:05:06, Felipe Balbi wrote:
 Hi,
 
 On Sun, Jan 20, 2013 at 11:17:31AM +0100, Pali Rohár wrote:
  On Sunday 20 January 2013 10:25:37 Felipe Balbi wrote:
   On Sun, Jan 20, 2013 at 03:58:13AM +0100, Pali Rohár wrote:
Signed-off-by: Pali Rohár pali.ro...@gmail.com
   
   NAK for two reasons:
   
   a) the original Nokia kernel used a separate g_file_storage
   gadget to use Mass Storage mode, use that
   
   b) there is no commit log
  
  Reason why add mass storage mode to g_nokia is to avoid switching 
  between g_{file,mass}_storage and g_nokia and to have one gadget 
  driver for Nokia N900. It is better to have usb network and mass 
  storage mode in one driver (and not to unload  load another).
  
  I tested this patch with 3.8-rc3 kernel on Nokia N900 and usb 
  network with mass storage mode working without problems.
 
 Doesn't matter, in this case this is something which nokia wrote to
 carry on their Maemo/MeeGo devices so unless someone from Nokia says
 this is how they want to use nokia.c from now on, I can't simply risk
 breaking all other users for your own convenience.

Nokia is unlikely to continue linux development, sorry.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: power_state: remove it from usb

[Pavel Machek]

On Thu 2008-02-21 16:50:36, Alan Stern wrote:
 On Thu, 21 Feb 2008, Pavel Machek wrote:
 
  power_state is scheduled for removal, and it is used only write-only
  by USB. Remove it.
 
 Unfortunately some of the things you changed turn out not to be 
 write-only.  (You also missed a usage of power.power_state in a 
 comment!)  

I might have missed comments, but where I missed read? It compiled
after I removed the field from the core...?

 I'll post a more complete patch soon. 

Thanks!
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
-
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: musb: Fix obex in g_nokia.ko causing kernel panic

[Pavel Machek]

Hi!

  From: Felipe Balbi ba...@ti.com
 
 This patch, which is present in 3.14-rc4 as 30a70b026 (usb: musb: fix
 obex in g_nokia.ko causing kernel panic), breaks USB gadget support
 on my Pandaboard.  Bisecting points to this commit, reverting it makes
 USB gadget support work again.  The problem is that this patch deletes
 the call which turns on the PHY.
 
 Config is attached.

Can you try adding 

static int omap2430_musb_init(struct musb *musb)
{
...
+   phy_power_on(musb-phy);
return 0;
...
}   


?
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] PM / Runtime: Allow to inactivate devices during system suspend

[Pavel Machek]

On Wed 2013-11-20 11:52:05, Ulf Hansson wrote:
 On 19 November 2013 16:35, Alan Stern st...@rowland.harvard.edu wrote:
  On Tue, 19 Nov 2013, Ulf Hansson wrote:
 
  At the moment, system PM is already affecting behaviour of runtime PM
  since it is preventing runtime suspend during system suspend.
 
  Sure.  And that behavior is documented.  In any case, it's a bug for
  drivers to depend on runtime suspend for carrying out a system suspend.
 
 Why do you say that?

Because that's the way it is?

 A significant amount of drivers should be able to cope with only the
 runtime PM callbacks, if only the PM core have respected the drivers
 in the way my RFC proposes.

So what? It is not as additional callback is huge burden -- code is
the same -- and we do want option of disabling runtime PM.

Don't make system suspend dependend on runtime PM.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] PM / Runtime: Allow to inactivate devices during system suspend

[Pavel Machek]

On Wed 2013-11-20 11:58:31, Alan Stern wrote:
 On Wed, 20 Nov 2013, Ulf Hansson wrote:
 
  Do note that, the intent with my RFC is also to simplify for drivers
  in general using runtime PM. No matter how you put it, the
  consequences of preventing runtime suspend during system suspend are
  causing confusions.
 
 Then fix the confusion.  Explain peoples' mistakes to them.

Exactly. Way to solve confusion is not make core behave as part of
drivers apparently think it should.

And yes, you have a list of broken drivers... thanks for that.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: BUG: usb: obex in g_nokia.ko causing kernel panic

[Pavel Machek]

On Tue 2013-11-26 18:17:49, Sebastian Reichel wrote:
 On Tue, Nov 26, 2013 at 06:10:22PM +0100, Pali Rohár wrote:
  On Tuesday 19 November 2013 11:51:12 Pali Rohár wrote:
   For a long time (since 3.5 or 3.8? - I do not remember) obex
   subdriver in g_nokia usb gadget module causing kernel panic
   after module is loaded on Nokia N900. I do not know where is
   problem and due to immediatelly kernel crash when loading
   driver I was not able to see any dmesg output. Now I was able
   to store something into mtd log and here is crash backtrace:
   [...]
  
  Hi! can you look at this problem?
 
 So it worked at some time?
 Have you tried to git bisect the problem?

You are a cruel person :-). N900 historically boots about half of the
time, trying to do git bisect there is a nice weekend project... for
about next century or so.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 1/3] pm: make PM macros more smart

[Pavel Machek]

On Thu 2013-12-12 21:18:23, David Cohen wrote:
 This patch makes SET_SYSTEM_SLEEP_PM_OPS() and SET_RUNTIME_PM_OPS() more
 smart.
 
 Despite those macros check for '#ifdef CONFIG_PM_SLEEP/RUNTIME' to avoid
 setting the callbacks when such #ifdef's aren't defined, they don't
 handle compiler to avoid messages like that:
 
 drivers/usb/host/xhci-plat.c:200:12: warning: ???xhci_plat_suspend??? defined 
 but not used [-Wunused-function]
 drivers/usb/host/xhci-plat.c:208:12: warning: ???xhci_plat_resume??? defined 
 but not used [-Wunused-function]
 
 As result, those macros get rid of #ifdef's when setting callbacks but
 not when implementing them.
 
 With this patch, drivers using SET_*_PM_OPS() macros don't need to #ifdef
 the callbacks implementation as well.

Well... Interesting trickery, but it means that resulting kernel
will be bigge due to the dead functions no?

That may be acceptable tradeoff, but I guess its worth discussing...
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH 1/3] pm: make PM macros more smart

[Pavel Machek]

On Sun 2013-12-15 11:25:08, David Cohen wrote:
 On Sun, Dec 15, 2013 at 06:51:12PM +0100, Pavel Machek wrote:
  On Thu 2013-12-12 21:18:23, David Cohen wrote:
   This patch makes SET_SYSTEM_SLEEP_PM_OPS() and SET_RUNTIME_PM_OPS() more
   smart.
   
   Despite those macros check for '#ifdef CONFIG_PM_SLEEP/RUNTIME' to avoid
   setting the callbacks when such #ifdef's aren't defined, they don't
   handle compiler to avoid messages like that:
   
   drivers/usb/host/xhci-plat.c:200:12: warning: ???xhci_plat_suspend??? 
   defined but not used [-Wunused-function]
   drivers/usb/host/xhci-plat.c:208:12: warning: ???xhci_plat_resume??? 
   defined but not used [-Wunused-function]
   
   As result, those macros get rid of #ifdef's when setting callbacks but
   not when implementing them.
   
   With this patch, drivers using SET_*_PM_OPS() macros don't need to #ifdef
   the callbacks implementation as well.
  
  Well... Interesting trickery, but it means that resulting kernel
  will be bigge due to the dead functions no?
 
 Actually, it doesn't get bigger. Before sending the patch I did this
 dummy test app:
 
 
 #include stdio.h
 
 #define USE_IT_OR_LOOSE_IT(fn)  ((void *)((unsigned long)(fn) - (unsigned 
 long)(fn)))
 
 #ifdef MAKE_ME_NULL
 static int func1(int a)
 {
 printf(Hey!!\n);
 return 0;
 }
 #endif

I thought that point of this patch series was getting rid of the
#ifdefs around the function...? Now I'm confused.

 struct global_data {
 int (*func)(int);
 };
 
 static struct global_data gd = {
 #ifdef MAKE_ME_NULL
 .func = USE_IT_OR_LOOSE_IT(func1),

If you have ifdef around the function, why do you need magic here? Why
not

.func = func1

?

Basically the warning tells you that you want the ifdef around the
function, too... (Otherwise you waste space). That seems like good
warning.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-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 v5] leds: USB: HID: Add support for MSI GT683R led panels

[Pavel Machek]

On Thu 2014-06-12 23:34:12, Janne Kanniainen wrote:
 This driver adds support for USB controlled led panels that exists in
 MSI GT683R laptop


Hi!

 --- /dev/null
 +++ b/Documentation/ABI/testing/sysfs-class-hid-driver-gt683r
 @@ -0,0 +1,10 @@
 +What:/sys/class/hidraw/hidraw/device/state
 +Date:Jun 2014
 +KernelVersion:   3.15
 +Contact: Janne Kanniainen janne.kanniai...@gmail.com
 +Description:
 + Set the mode of LEDs
 +
 + 0 - normal
 + 1 - audio
 + 2 - breathing

THat's some strange interface. Don't we normally use led triggers for this?

And the mode of the LED should really be in /sys/class/leds, not in hidraw 
somewhere...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-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 1/3] usb: gadget: function: phonet: balance usb_ep_disable calls

[Pavel Machek]

On Tue 2015-02-03 13:18:59, Felipe Balbi wrote:
 On Tue, Feb 03, 2015 at 05:17:28PM +0100, Pali Rohár wrote:
  On Tuesday 03 February 2015 16:43:45 Felipe Balbi wrote:
   Hi,
   
   On Tue, Feb 03, 2015 at 04:31:51PM +0100, Pali Rohár wrote:
On Tuesday 03 February 2015 00:15:19 Felipe Balbi wrote:
 f_phonet's -set_alt() method will call usb_ep_disable()
 potentially on an endpoint which is already disabled.
 That's something the gadget/function driver must
 guarantee that it's always balanced.
 
 In order to balance the calls, just make sure the endpoint
 was enabled before by means of checking the validity of
 driver_data.
 
 Reported-by: Pali Rohár pali.ro...@gmail.com
 Signed-off-by: Felipe Balbi ba...@ti.com
 ---

Your patches cause that kernel does not print any error
message to n900 screen anymore and reboot device in 10
seconds. I did not loaded any external modules.
   
In qemu I see this crash in early boot:
   alright, so n900's working fine. I'll wait until you debug
   qemu a little more, thank you
  
  NO! It does not working, see . It break n900 totally!
 
 settle down a bit more. I don't have the HW you have and things are
 working fine on boards I _do_ have, there's not much more I can do to
 help without you doing your homework. Debug a bit more and bring more
 information as to what's going on, until then you're on your own.

I'm not sure what you are smoking, but Pali is doing more then enough
of his homework. No, it is not okay for you to break n900, and it is
not okay for you to break qemu.

In fact, you should do _your_ homework and install n900 qemu now. It
is not Pali's homework to debug your stuff for you.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-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 1/3] usb: gadget: function: phonet: balance usb_ep_disable calls

[Pavel Machek]

  In current state I review all 3 patches as:
  
  Rejected-by: Pali Rohár pali.ro...@gmail.com
  [It breaks booting Nokia N900 device]
 
 next step, figure why it's broken. Working just fine here
 on AM335x which has the same musb IP.

Why is broken? That is easy. You send 3 patches which broke
it.
   
   Actually when I reverted only that patch which adds line:
   
   pm_runtime_irq_safe(musb-controller)
   
   then early boot crash disappeared.
   
   But other two patches did not fixed support for external .ko
   gadget modules. State is same -- crash after modprobe.
  
  Here is crash from qemu when musb is compiled into kernel:
  
  [0.641662] Unable to handle kernel NULL pointer dereference at virtual 
  address 
  [0.642211] pgd = c0004000
...
  [0.672882] ---[ end Kernel panic - not syncing: Attempted to kill init! 
  exitcode=0x000b
  [0.672882]
  
  Reason why it crashes is because when function
  omap2430_runtime_resume() is called pointer to functions 
  musb_readl and musb_writel are both NULL. And so NULL pointer
  dereference.
 
 https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/nextid=1861a2c60351a390272b3395f4d88480cdfd9e58


So you commit a fix to your internal branch without telling Pali, who
is actively debugging same problem at the same time, and then you just
send him url without at least saying sorry?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-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 1/3] usb: gadget: function: phonet: balance usb_ep_disable calls

[Pavel Machek]

On Fri 2015-02-20 08:39:06, Felipe Balbi wrote:
 On Fri, Feb 20, 2015 at 09:27:51AM +0100, Pavel Machek wrote:
In current state I review all 3 patches as:

Rejected-by: Pali Rohár pali.ro...@gmail.com
[It breaks booting Nokia N900 device]
   
   next step, figure why it's broken. Working just fine here
   on AM335x which has the same musb IP.
  
  Why is broken? That is easy. You send 3 patches which broke
  it.
 
 Actually when I reverted only that patch which adds line:
 
 pm_runtime_irq_safe(musb-controller)
 
 then early boot crash disappeared.
 
 But other two patches did not fixed support for external .ko
 gadget modules. State is same -- crash after modprobe.

Here is crash from qemu when musb is compiled into kernel:

[0.641662] Unable to handle kernel NULL pointer dereference at 
virtual address 
[0.642211] pgd = c0004000
  ...
[0.672882] ---[ end Kernel panic - not syncing: Attempted to kill 
init! exitcode=0x000b
[0.672882]

Reason why it crashes is because when function
omap2430_runtime_resume() is called pointer to functions 
musb_readl and musb_writel are both NULL. And so NULL pointer
dereference.
   
   https://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=testing/nextid=1861a2c60351a390272b3395f4d88480cdfd9e58
  
  
  So you commit a fix to your internal branch without telling Pali, who
  is actively debugging same problem at the same time, and then you just
  send him url without at least saying sorry?
 
 grow up... why do I have to say sorry for fixing a problem ? Why do I
 have to let you know I committed something to my branch ? It's the

Because that's polite thing to do when you break someone elses system?

I see politeness is not your great strength.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-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 0/2] Allow twl4030_charger to find phy reliably.

[Pavel Machek]

On Tue 2015-02-24 15:01:29, NeilBrown wrote:
 The twl4030_charger is physically paired with the twl4030 USB phy,
 so the drivers need to be able to reliably find each other.
 
 twl4030_charger currently uses usb_get_phy(), which works if there is
 only one phy to choose from, but is not reliable in more complex
 configurations.
 
 These patches add a new interface to allow a phy to be found given a
 device node, and then use that interface in twl4030_charger so that
 it finds its sibling in the devicetree, and gets the phy associated
 with that.

Apart for dt documentation...

Acked-by: Pavel Machek pa...@ucw.cz

 --
 Signature

Smiley

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-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 v4 1/5] gadget: Introduce the notifier functions

[Pavel Machek]

HI!

> > +   int ret;
> > +
> > +   mutex_lock(>lock);
> > +   ret = raw_notifier_chain_unregister(>nh, nb);
> 
> Greg, this is the kind of thing I wanted to avoid adding more of.
> 
> I was wondering if you would accept subsystems using kdbus for
> this sort of notification. I'm okay waiting for kdbus for another
> couple merge windows (if we have to) before that's merged, but
> if we take this raw notifier approach now, we will end up having
> to support it forever.
> 
> Also, because soon enough we will have to support USB Power Delivery
> with Type C connector, this is bound to change in the coming months.
> 
> Frankly, I wanted all of this to be decided in userland with the
> kernel just providing notification and basic safety checks (we don't
> want to allow a bogus userspace daemon frying anybody's devices).
> 
> How would you feel about that ?

So init=/bin/bash boot no longer provides machine that charges itself?

That would be bad. Traditionally, hardware controls battery charging,
and if hardware needs some help, we should do it in kernel, to mask
the difference from userspace.
Pavel
--
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: [Device-mainlining] [PATCH v2 2/3] gadget: Introduce the usb charger framework

[Pavel Machek]

Hi!

> On 08/16/2015 08:03 PM, Baolin Wang via device-mainlining wrote:
> > On 14 August 2015 at 23:27, Greg KH  wrote:
> >> On Fri, Aug 14, 2015 at 05:47:45PM +0800, Baolin Wang wrote:
> >>> + *
> >>> + * This program is free software; you can redistribute it and/or modify
> >>> + * it under the terms of the GNU General Public License as published by
> >>> + * the Free Software Foundation; either version 2 of the License, or
> >>> + * (at your option) any later version.
> >>
> >> I have to ask, do you really mean "any later version"?
> >>
> > 
> > I think I did not get your point, could you explain it detailedly?
> 
> The full kernel is licensed under v2 of the GPL only, not "any later version".
> See the second paragraph at the top of the COPYING file in the root directory
> of the kernel source tree. There are differences on individual files, but
> having this file allow "any later version" makes it different from much of
> rest of the kernel.
> 
> Unless you have a specific reason to allow greater-than-V2 GPL licensing on 
> this
> file, you should change the licensing clause.  The following wording appears
> to be pretty popular:
> 
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> * published by the Free Software Foundation.

Please, keep it V2 or later, if you can. It makes sharing code with U-Boot (for
example) easier.

There's long tradition of "V2 or later" code in the kernel.

Best regards,
Pavel
--
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 v4 1/5] gadget: Introduce the notifier functions

[Pavel Machek]

Hi!

> > What's the advantage of pushing this to userspace?  By the time we
> > provide enough discoverability to enable userspace to configure itself
> > it seems like we'd have enough information to do the job anyway.
> 
> you're going to be dealing with a setup where each vendor does one thing
> differently. Some will have it all in the SoC part of a single IP (dwc3 can be
> configured that way), some will push it out to companion IC, some might even 
> use
> some mostly discrete setup and so on...
> 
> We're gonna be dealing with a decision that involves information from multiple
> subsystems (USB, regulator, hwmon, power supply to name a few).
> 
> We tried doing it all in the kernel back in N800, N810 and N900/N9 days and 
> it's
> just plain difficult. To make matters worse, N900 had two USB PHYs, one for
> actual runtime use and another just for detecting the charger type on the 
> other
> end.

N900 does charging from kernel these days.

Not being able to boot generic distribution would be regression there...


Pavel
--
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: [Device-mainlining] [PATCH v2 2/3] gadget: Introduce the usb charger framework

[Pavel Machek]

Hi!

> > > * This program is free software; you can redistribute it and/or modify
> > > * it under the terms of the GNU General Public License version 2 as
> > > * published by the Free Software Foundation.
> > 
> > Please, keep it V2 or later, if you can. It makes sharing code with U-Boot 
> > (for
> > example) easier.
> > 
> > There's long tradition of "V2 or later" code in the kernel.
> 
> And there's even longer "v2 only" tradition as well.  It's up to the
> person / company submitting the code to pick the license for it, let
> them make the decision please.

You wrote:

>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>
>I have to ask, do you really mean "any later version"?

Given that you are quite high-level maintainer, that does not sound exactly
like "let them make the decision". It sounds like a pressure to change the
license to the one you liked. WHy did you have to ask?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 1/2] usb: configfs: allow UDC binding rule configured as binding to *any* UDC

[Pavel Machek]

On Tue 2016-05-03 11:04:24, changbin...@intel.com wrote:
> From: "Du, Changbin" 
> 
> On most platforms, there is only one device controller available.
> In this case, we desn't care the UDC's name. So let's ignore the
> name by setting 'UDC' to 'any'. And also we can change UDC name
> at any time if it is not binded (no need set to "" first).

making "any" special does not look like a good idea. What if it really
is "any"?

Return nothing instead, not even \n?

> Signed-off-by: Du, Changbin 
> Signed-off-by: Du, Changbin 

I don't think this is how you should sign it off.

Best regards,

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCHv2] usb: USB Type-C Connector Class

[Pavel Machek]

On Thu 2016-05-19 15:44:54, Heikki Krogerus wrote:
> The purpose of this class is to provide unified interface for user
> space to get the status and basic information about USB Type-C
> Connectors in the system, control data role swapping, and when USB PD
> is available, also power role swapping and Alternate Modes.
> 
> Signed-off-by: Heikki Krogerus 
> ---
>  drivers/usb/Kconfig |   2 +
>  drivers/usb/Makefile|   2 +
>  drivers/usb/type-c/Kconfig  |   7 +
>  drivers/usb/type-c/Makefile |   1 +
>  drivers/usb/type-c/typec.c  | 957 
> 

For consistency, should this be either type-c/type-c.c or
typec/typec.c?

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> >>The "color" attribute would contain "R G B" values. Setting the "color"
> >>attribute of any of the three LED class devices would affect brightness
> >>properties (i.e. constituent colors) of the remaining two ones.
> >>It would result in disabling any active triggers and writing all the
> >>three color settings to the RGB LED controller at one go.
> >
> >Having one attribute across three devices is rather ugly. And we'll
> >need to solve the pattern issue one day.
> >
> >What's tricky about patterns is that you need to control 3 (or more)
> >leds at a time. Problem you are trying to solve here is ... control of
> >3 leds, at the same time.
> >
> >So let's solve them together.
> 
> OK, now I've got your point. So we'd need to have a means for defining
> patterns. The interface could be located at /sys/class/leds/patterns.
> 
> We'd need to have a flexible way for defining LED class devices involved
> in a pattern. Since we cannot guarantee no space in a LED class device
> name, then a single attribute containing space separated list is not an
> option. We'd have to create a predefined set of attributes that would
> contain LED class device name. Predefined implies that it would be
> a fixed number, i.e. either some attributes would always remain unused
> or, which is even worse, we could run out of free attributes for some
> use cases.

There's a better solution: make pattern behave as a trigger for leds
it controls.

So we'd have

/sys/class/leds/patterns/lp5523

then we'd have

/sys/class/leds/lp5523::red/trigger = "lp5523:1"
/sys/class/leds/lp5523::green/trigger = "lp5523:2"
/sys/class/leds/lp5523::blue/trigger = "lp5523:3"

(or something similar, I'd have to boot the n900 to see the exact
names).

That means that we don't need space-separated lists. (And actually
gives us more flexibility; Maemo for example used the pattern engine
not for RGB led, but for 6 keyboard backlight leds.)

> The same constraints would appear if we wanted to be able to define
> more than one pattern.

We'd like to have more than one pattern _engine_, but it should be
enough to have one pattern per pattern engine at a time. 

> It would be best to work out more flexible solution. I wonder if
> ioctl interface isn't the only option.

Well, there's configs, which is more flexible, but...

Best regards,

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> > We would probably need additional op in the LED core : color_set.
> > 
> > Having the color set to nonzero value would signify the the three LED
> > class devices are in sync and that setting a trigger on any of them
> > applies to the remaining two ones. It would have to be considered
> > whether existing triggers could be made compatible with synchronized
> > RGB LED class devices.
> > 
> > I'm curious what do you think about the idea.
> > 
> > Pavel, Heiner, others?
> > 
> Exposing "coupled LED devices" as separate LED devices most likely is ok
> when accessed from user space as the name of the led_classdev's indicates
> that they belong together.
> But how about a trigger wanting to set a RGB LED to a specific color?
> (That's not available yet but one possible use case for RGB LED's)
> A trigger is bound to a led_classdev currently. In addition we'd need
> to introduce some kind of super_led_classdev having links to the respective
> R/G/B led_classdev's (+ trigger functions dealing with this 
> super_led_classdev).
> 
> These changes / extensions are not needed if a RGB LED is exposed as one
> led_classdev, just with flag LED_DEV_CAP_RGB set.
> OK, we'd still have to change the sysfs interface as obviously setting
> hue/sat/brightness via one "brightness" attribute is not acceptable.
> However this constraint might not affect the kernel-internal trigger API
> (usage of parameter brightness in led_trigger_event).

Your proposal would break existing hardware. We already have RGB LEDs
exposed as three LEDs. It is too late to change interface there.

> I see Pavel's point that there might be different types of multi-color LED's.
> At least we have:
> - multi-color LED's where each single LED is visible even if all are switched 
> on
> - multi-color LED's like RGB LED's where you usually just see a
> uniform color

Well, I suggest we ignore that distinction. Yes, I can see different
colors coming from different directions, but the LED was clearly
designed to look like single light. 

> Last but not least regarding the patterns:
> Something like proposed by Pavel is e.g. (partially) supported by the blink(1)
> firmware. That would be an example of such a "hardware-accelerated" pattern.
> 
> As I see it the current blinking support then would be one special case of a 
> pattern.
> As a consequence once having pattern support we might be able to switch users 
> of blinking
> to pattern and remove the blinking support.

No, you can't remove existing blinking support, due to backwards
compatibility reasons.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> >Lets say we have
> >
> >/sys/class/pattern/lp5533::0
> >/sys/class/pattern/software::0
> >
> >/sys/class/led/n900::red ; default trigger "lp5533::0:0"
> >/sys/class/led/n900::green ; default trigger "lp5533::0:1"
> >/sys/class/led/n900::blue ; default trigger "lp5533::0:2"
> >
> >Normally, pattern would correspond to one RGB LED. We could have
> >attribute "/sys/class/pattern/lp5533::0/color" containing R,G,B for
> >this pattern.
> >>
> >>Could you give an example on how to set a color for RGB LED using
> >>this interface? Would it be compatible with LED triggers?
> >>Where the "pattern" class would be implemented?
> >
> >Well, 'echo "50 60 70" > /sys/class/pattern/lp5533::0/color' should
> >set the color for the led. 'echo "trigger-name" > trigger' would set
> >the trigger, probably just toggling between LED off and set color for
> >the old triggers.
> >
> >Where to implement the patterns is different question, but for example
> >drivers/leds/pattern?
> 
> I'd rather leave the pattern issue for now, since it seems to be
> different from the problem Heiner was trying to solve with his LED RGB
> extension. Moreover, hardware patterns are device specific and it could
> be hard to propose a generic interface.

Well, RGB leds are basically useless without pattern support. And I
believe we can do generic interface.

> Drivers can always expose their custom sysfs attributes for configuring
> the patterns.
> 
> Regardless of the above, some of your considerations brought me an idea
> on how to add generic and backwards compatible support for setting RGB
> color at one go.
> 
> Currently LED class drivers of RGB LED controllers expose three LED
> class devices - one per R, G and B color component. I propose that
> such drivers set LED_DEV_CAP_RGB flag for each LED class device they
> register. LED core, seeing the flag, would create a generic "color"
> sysfs attribute for each of the three LED class devices.
> 
> The "color" attribute would contain "R G B" values. Setting the "color"
> attribute of any of the three LED class devices would affect brightness
> properties (i.e. constituent colors) of the remaining two ones.
> It would result in disabling any active triggers and writing all the
> three color settings to the RGB LED controller at one go.

Having one attribute across three devices is rather ugly. And we'll
need to solve the pattern issue one day.

What's tricky about patterns is that you need to control 3 (or more)
leds at a time. Problem you are trying to solve here is ... control of
3 leds, at the same time.

So let's solve them together.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> >>>What's tricky about patterns is that you need to control 3 (or more)
> >>>leds at a time. Problem you are trying to solve here is ... control of
> >>>3 leds, at the same time.
> >>>
> >>>So let's solve them together.
> >>
> >>OK, now I've got your point. So we'd need to have a means for defining
> >>patterns. The interface could be located at /sys/class/leds/patterns.
> >>
> >>We'd need to have a flexible way for defining LED class devices involved
> >>in a pattern. Since we cannot guarantee no space in a LED class device
> >>name, then a single attribute containing space separated list is not an
> >>option. We'd have to create a predefined set of attributes that would
> >>contain LED class device name. Predefined implies that it would be
> >>a fixed number, i.e. either some attributes would always remain unused
> >>or, which is even worse, we could run out of free attributes for some
> >>use cases.
> >
> >There's a better solution: make pattern behave as a trigger for leds
> >it controls.
> >
> >So we'd have
> >
> >/sys/class/leds/patterns/lp5523
> >
> >then we'd have
> >
> >/sys/class/leds/lp5523::red/trigger = "lp5523:1"
> >/sys/class/leds/lp5523::green/trigger = "lp5523:2"
> >/sys/class/leds/lp5523::blue/trigger = "lp5523:3"
> >
> >(or something similar, I'd have to boot the n900 to see the exact
> >names).
> 
> How about implementing patterns as a specific typer of triggers?
> Let's say we have ledtrig-rgb-pattern:

Well, we'd need ledtrig-rgb-pattern-1, ledtrig-rgb-pattern-2, ... , as we
can have more than one rgb led. But yes.

> After setting a trigger following sysfs attribute would appear
> in a LED class device sysfs interface:
> 
> $cat /sys/class/lp5523::red/rgb_color
> red green blue [none]
> 
> $echo "red" > /sys/class/leds/lp5523::red/rgb_color
> 
> and similarly
> 
> $echo "green" > /sys/class/leds/lp5523::green/rgb_color
> $echo "blue" > /sys/class/leds/lp5523::blue/rgb_color

Yes, that would work -- selecting channels from the pattern.

> Similar approach could be applied for blink patterns:
> There could be additional attributes provided for defining
> the position in a blink sequence, or/and blink period.

For patterns, I'd suggest array of (r g b time) values.

Pattern engines can do stuff like "slowly turn LED from off to red, then switch 
color to
white, then slowly turn it to yellow, then turn it off at once" with defined 
speeds
for "slowly" and option of either linear on non-linear brightness ramping.

The last option might be a bit too much, but I believe we should support the 
rest.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v4 1/5] gadget: Introduce the notifier functions

[Pavel Machek]

Hi!

> >> Also, because soon enough we will have to support USB Power Delivery
> >> with Type C connector, this is bound to change in the coming months.
> >> 
> >> Frankly, I wanted all of this to be decided in userland with the
> >> kernel just providing notification and basic safety checks (we don't
> >> want to allow a bogus userspace daemon frying anybody's devices).
> >> 
> >> How would you feel about that ?
> >
> > So init=/bin/bash boot no longer provides machine that charges itself?
> >
> > That would be bad. Traditionally, hardware controls battery charging,
> > and if hardware needs some help, we should do it in kernel, to mask
> > the difference from userspace.
> 
> this is a very valid point which I hadn't considered :-)
> 
> Seems like kernel it is, no matter how easy or how difficult it gets.

Basically yes. Note that for some bells & whistles, you can require userland 
help.

For example it would probably be ok if it charged slower than usual... but I 
guess
that does not really help in this case.

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v7 1/4] gadget: Introduce the usb charger framework

[Pavel Machek]

Hi!

> > It's your HW :-) You tell me if it's really necessary. But, hey, if you
> > get enumerated @500mA, this is the host telling you it _CAN_ give you
> > 500mA. In that case, why wouldn't you ?

Dunno, perhaps not to drain battery in host too quickly?
Or perhaps you are charging from external battery?

> >>> why RW ? Who's going to use these ? Also, you're not documenting this
> >>> new sysfs file.
> >>
> >> Cause we have show and store operation for SDP type. If users want to
> >> know or set the SDP current, they can use the sysfs file.
> >> I'll add the documentation for it.
> >
> > but why would the user change it ? Here's the thing: you have a few
> > posibilities for this:
> >
> > a) you are connected to a dedicated charger
> >
> > In this case, you can get up to 2000mA depending on the charger.
> >
> > If $this charger can give you or not 2000mA is not detectable,
> > so what do charging ICs do ? They slowly increase the attached
> > load accross VBUS/GND and measure VBUS value. When IC notices
> > VBUS dropping bit, step back to previous load.
> >
> > This means you will always charger with maximum rating of DCP.
> >
> > Why would user change this ? More is unsafe, less is just
> > stupid.

Less is not neccessarily stupid. First, it is useful for debugging, second, you
don't know how much this charger can give you. You measured you can get 1.8A,
but the note on the charger says 1.5A. You may want to go with 1.5A.

Also, there are several incompatible standards for detecting "dedicated 
charger". IIRC
iPhone has different one from iPad. So it is quite important to be able to 
control this manually.


> > d) you are connected to a standard port and get enumerated with your
> > 100mA configuration.
> >
> > you *know* 100mA is okay. So you connect a 100mA load and get it
> > over with.
> >
> > This means you will always charger with maximum rating for this
> > SDP.
> >
> > Why would user change this ? More is unsafe, less is just
> > stupid.

I've needed to override 100mA default many times. Maybe it is unsafe,
but it is useful.

(And with USB 5V connected directly to pretty beefy PC power supply...
it is sometimes safer than it looks).

Plus, again, useful for debugging.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v7 1/4] gadget: Introduce the usb charger framework

[Pavel Machek]

Hi!

> >>> +#define DEFAULT_SDP_CUR_LIMIT(500 - DEFAULT_CUR_PROTECT)
> >>
> >> According to the spec we should always be talking about unit loads (1
> >> unit load is 100mA for HS/FS/LS and 150mA for SS). Also, this will not
> >> work for SS capable ports and SS gadgets (we have quite a few of them,
> >> actually). You're missing the opportunity of charging at 900mA.
> >
> > I follow the DCP/SDP/CDP/ACA type's default current limitation and
> > user can set them what they want.
> 
> no, the user CANNOT set it to what they want. If you get enumerated
> @100mA and the user just decides to set it to 2000mA, s/he could even
> melt the USB connector. The kernel _must_ prevent such cases.

root should be allowed to do that.

Very often, you want to charge using 1.8A from an old desktop PC.

N900 will simply not charge from .5A.

> a) you are connected to a dedicated charger
> 
>   In this case, you can get up to 2000mA depending on the charger.
> 
>   If $this charger can give you or not 2000mA is not detectable,
>   so what do charging ICs do ? They slowly increase the attached
>   load accross VBUS/GND and measure VBUS value. When IC notices
>   VBUS dropping bit, step back to previous load.
> 
>   This means you will always charger with maximum rating of DCP.
> 
>   Why would user change this ? More is unsafe, less is just
>   stupid.

Actually, less is not stupid. Charging li-ion battery from li-ion battery might
be stupid. Imagine I'm on train, with device like N900 (50% battery) and power 
bank
(3Ah). I'm actively using the device. If I let it charge at full current, I'll 
waste
energy. If I limit current to approximately the power consumption, it will run 
the
powerbank empty, first, then empty the internal battery, maximizing total time I
can use the device.

Best regards,

Pavel

--
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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> >As I see it the current blinking support then would be one special case of a 
> >pattern.
> >As a consequence once having pattern support we might be able to switch 
> >users of blinking
> >to pattern and remove the blinking support.
> 
> Let's split patterns related discussion into a separate thread.
> It would be best if it began with a patch.

Lets design userland interface first, then decide how to implement it
in the kernel. Patches are useless at this point.

And actually... without patterns, existing interface works just
fine. Even if you can't "atomically" write values to three different
files, operation is so fast that user will not see the intermediate
state, anyway. So solving the "atomic" issue without solving the rest
is pretty much useless.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!
On Wed 2016-03-30 09:57:38, Jacek Anaszewski wrote:
> Hi Heiner and Pavel,
> 
> On 03/29/2016 10:38 PM, Heiner Kallweit wrote:
> >Am 29.03.2016 um 12:02 schrieb Pavel Machek:
> >>Hi!
> >>
> >>First, please Cc me on RGB color support.
> >>
> >>>Add generic support for RGB Color LED's.
> >>>
> >>>Basic idea is to use enum led_brightness also for the hue and saturation
> >>>color components.This allows to implement the color extension w/o
> >>>changes to struct led_classdev.
> >>>
> >>>Select LEDS_RGB to enable building drivers using the RGB extension.
> >>>
> >>>Flag LED_SET_HUE_SAT allows to specify that hue / saturation
> >>>should be overridden even if the provided values are zero.
> >>>
> >>>Some examples for writing values to /sys/class/leds//brightness:
> >>>(now also hex notation can be used)
> >>>
> >>>255 -> set full brightness and keep existing color if set
> >>>0 -> switch LED off but keep existing color so that it can be restored
> >>>  if the LED is switched on again later
> >>>0x100 -> switch LED off and set also hue and saturation to 0
> >>>0x00 -> set full brightness, full saturation and set hue to 0
> >>>(red)
> >>
> >>Umm, that's rather strange interface -- and three values in single sysfs
> >>file is actually forbidden.
> >>
> >>Plus, it is very much unlike existing interfaces for RGB LEDs, which
> >>we already have supported in the tree. (At least nokia N900 and Sony
> >>motion controller already contain supported three-color LEDs).
> >>
> >>Now... yes, there's work to be done for the 3-color LEDs. Currently,
> >>they are treated as three different LEDs. (Which makes some sense, you
> >>can use "battery charging" trigger for LED, and CPU activity trigger
> >>for green, for example). It would be good to have some kind of
> >>grouping, so that userspace can tell "these 3 leds are actually
> >>combined into one light".
Hi!

> >At first thanks for the review comments.
> >Treating the three physical LEDs of a RGB LED as separate LED devices
> >might have been implemented due to the lack of alternatives.
> >With one trigger controlling the red LED and another controlling the green
> >LED we may end up with a yellow light. Not sure whether this is what we want.
> >
> >One driver for this extension was the idea of triggers using color
> >to visualize states etc.
> >Therefore it's not only about userspace controlling the color.
> >As a trigger is bound to a led_classdev we need a led_classdev
> >representing a RGB LED device.
> >
> >And ok: If required the sysfs interface can be splitted into separate
> >attributes for hue, saturation, and (existing) brightness.
> 
> It would have the same downsides as in case of having r, g and b in
> separate attributes, i.e. - problems with setting LED colour in
> a consistent way. This way LED blinking in whatever colour couldn't
> be supported reliably. It was one of your primary rationale standing
> behind this design, if I remember correctly. Second - what about
> triggers? We've had a long discussion about it and this design turned
> out to be most fitting.

Are on/off triggers really that useful for a LED that can produce 16
million colors?

I believe we should support patterns for RGB LEDs. Something like
[ (time, r, g, b), ... ] . Ok, what about this one?

Lets say we have

/sys/class/pattern/lp5533::0
/sys/class/pattern/software::0

/sys/class/led/n900::red ; default trigger "lp5533::0:0"
/sys/class/led/n900::green ; default trigger "lp5533::0:1"
/sys/class/led/n900::blue ; default trigger "lp5533::0:2"

Normally, pattern would correspond to one RGB LED. We could have
attribute "/sys/class/pattern/lp5533::0/color" containing R,G,B for
this pattern. Then we could have normal "trigger" mechanism, working
with the color used. Probably recognizing "none" for manual control,
and "pattern" for pattern control. (Pattern would be controlled as
described above).

> It's hard to address these requirements by having the settings in
> separate attributes, due to synchronization issues, and LED trigger
> mechanism specificity.
> 
> There is a question whether we can bend the sysfs "one value per sysfs
> file" rule down to RGB LEDs needs.
> 
> Of course other brilliant ideas on how to approach the problem are
> more than expected.

linux-api sounds like interesting idea, please cc me if you do that.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> >>The main drawback is that you can't set the colour at one go,
> >>but have to set brightness of each LED class device (R,G,B)
> >>separately. It incurs delays between setting each colour component.
> >
> >Yeah. Well, on some hardware, that's just the way it is. If the leds
> >are separate devices on i2c, you can't really set them in one go.
> 
> Delays can occur even if the LEDs are controlled by the same device.
> Brightness of each LED class device is set with separate system
> call and there will be always some time shift between particular I2C
> transmissions that set the brightness for each sub-LED.
> 
> If the three sub-LEDs were controlled by a single LED class device
> then we could setup the brightness of each sub-LED with single I2C
> transmission.

Ok, well, yes, maybe you could.

You can still do that with the proposed interface, but yes, it will be
trickier.

OTOH proposed interface will also help with the hardware pattern
support, will work with existing leds, and matches the way hardware
works. So I believe it is worth it.

Best regards,

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> >>>pavel@duo:~$ ls -1 /sys/class/leds/
> >>>tpacpi:green:batt
> >>>tpacpi:orange:batt
> >>>
> >>>This is physically 2 leds but hidden under one indicator, so you got
> >>>"off", "green", "orange" and "green+orange".
> >>
> >>That's a good example. As long as you can recognize green+orange as
> >>separate lights/colors
> >>(w/o magnifying glass) I wouldn't call it "a LED with multiple colors"
> >>but "multiple
> >>LED devices".
> >
> >Well, that's how it is currently handled. But for the user, it looks
> >as a LED with multiple colors.
> >
> >>In my use case we talk about RGB LEDs like the commonly used 5050 SMD RGB 
> >>LEDs.
> >>And it's not only about using a handful of discrete colors but about
> >>displaying any arbitrary
> >>color.
> >>So far the kernel exposes the physical RGB LEDs as separate LEDs only
> >>and I can't use
> >>a trigger to e.g. set "magenta with 50% brightness".
> >
> >Why not?
> >
> >What do you do if you want to display magenta on your LCD?
> >
> >You compute RGB values, then display them.
> 
> The main drawback is that you can't set the colour at one go,
> but have to set brightness of each LED class device (R,G,B)
> separately. It incurs delays between setting each colour component.

Yeah. Well, on some hardware, that's just the way it is. If the leds
are separate devices on i2c, you can't really set them in one go.

But some hardware has hardware pattern controls, and it can set them
atomically. 

> It is also impossible to set arbitrary colour from a trigger.
> Similarly blinking with arbitrarily chosen colour from RGB palette
> is impossible if separate colour components are treated as
> separate LEDs.

Yes, see the proposal in the other mail. I believe we should have
separate R, G, B LED devices, and separate pattern controller.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> >>It would have the same downsides as in case of having r, g and b in
> >>separate attributes, i.e. - problems with setting LED colour in
> >>a consistent way. This way LED blinking in whatever colour couldn't
> >>be supported reliably. It was one of your primary rationale standing
> >>behind this design, if I remember correctly. Second - what about
> >>triggers? We've had a long discussion about it and this design turned
> >>out to be most fitting.
> >
> >Are on/off triggers really that useful for a LED that can produce 16
> >million colors?
> >
> >I believe we should support patterns for RGB LEDs. Something like
> >[ (time, r, g, b), ... ] . Ok, what about this one?
> >
> >Lets say we have
> >
> >/sys/class/pattern/lp5533::0
> >/sys/class/pattern/software::0
> >
> >/sys/class/led/n900::red ; default trigger "lp5533::0:0"
> >/sys/class/led/n900::green ; default trigger "lp5533::0:1"
> >/sys/class/led/n900::blue ; default trigger "lp5533::0:2"
> >
> >Normally, pattern would correspond to one RGB LED. We could have
> >attribute "/sys/class/pattern/lp5533::0/color" containing R,G,B for
> >this pattern.
> 
> This involves the same issue you were opposed to: three values per
> sysfs attribute.

And solves a lot of other things. Like actually being backwards
compatible.

And yes, it involves three values in a file, but now it is array of
led brightnesses, and that might actually be acceptable. (At least the
values have uniform meaning).

Plus, it is not "issue you were opposed to" it is "something that is
not permitted by sysfs maintainers".

Thanks,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> It would have the same downsides as in case of having r, g and b in
> separate attributes, i.e. - problems with setting LED colour in
> a consistent way. This way LED blinking in whatever colour couldn't
> be supported reliably. It was one of your primary rationale standing
> behind this design, if I remember correctly. Second - what about
> triggers? We've had a long discussion about it and this design turned
> out to be most fitting.
> >>>
> >>>Are on/off triggers really that useful for a LED that can produce 16
> >>>million colors?
> >>>
> >>>I believe we should support patterns for RGB LEDs. Something like
> >>>[ (time, r, g, b), ... ] . Ok, what about this one?
> >>>
> >>>Lets say we have
> >>>
> >>>/sys/class/pattern/lp5533::0
> >>>/sys/class/pattern/software::0
> >>>
> >>>/sys/class/led/n900::red ; default trigger "lp5533::0:0"
> >>>/sys/class/led/n900::green ; default trigger "lp5533::0:1"
> >>>/sys/class/led/n900::blue ; default trigger "lp5533::0:2"
> >>>
> >>>Normally, pattern would correspond to one RGB LED. We could have
> >>>attribute "/sys/class/pattern/lp5533::0/color" containing R,G,B for
> >>>this pattern.
> 
> Could you give an example on how to set a color for RGB LED using
> this interface? Would it be compatible with LED triggers?
> Where the "pattern" class would be implemented?

Well, 'echo "50 60 70" > /sys/class/pattern/lp5533::0/color' should
set the color for the led. 'echo "trigger-name" > trigger' would set
the trigger, probably just toggling between LED off and set color for
the old triggers.

Where to implement the patterns is different question, but for example
drivers/leds/pattern?

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> > First, please Cc me on RGB color support.
> > 
> >> Add generic support for RGB Color LED's.
> >>
> >> Basic idea is to use enum led_brightness also for the hue and saturation
> >> color components.This allows to implement the color extension w/o
> >> changes to struct led_classdev.
> >>
> >> Select LEDS_RGB to enable building drivers using the RGB extension.
> >>
> >> Flag LED_SET_HUE_SAT allows to specify that hue / saturation
> >> should be overridden even if the provided values are zero.
> >>
> >> Some examples for writing values to /sys/class/leds//brightness:
> >> (now also hex notation can be used)
> >>
> >> 255 -> set full brightness and keep existing color if set
> >> 0 -> switch LED off but keep existing color so that it can be restored
> >>  if the LED is switched on again later
> >> 0x100 -> switch LED off and set also hue and saturation to 0
> >> 0x00 -> set full brightness, full saturation and set hue to 0
> >> (red)
> > 
> > Umm, that's rather strange interface -- and three values in single sysfs
> > file is actually forbidden.
> > 
> > Plus, it is very much unlike existing interfaces for RGB LEDs, which
> > we already have supported in the tree. (At least nokia N900 and Sony
> > motion controller already contain supported three-color LEDs).
> > 
> > Now... yes, there's work to be done for the 3-color LEDs. Currently,
> > they are treated as three different LEDs. (Which makes some sense, you
> > can use "battery charging" trigger for LED, and CPU activity trigger
> > for green, for example). It would be good to have some kind of
> > grouping, so that userspace can tell "these 3 leds are actually
> > combined into one light".
> > 
> At first thanks for the review comments.
> Treating the three physical LEDs of a RGB LED as separate LED devices
> might have been implemented due to the lack of alternatives.

To be fair... they _are_ separate LED devices. In N900 case, you can
even see light comming from slightly different places if you look closely.

> With one trigger controlling the red LED and another controlling the green
> LED we may end up with a yellow light. Not sure whether this is what
> we want.

Well, it should be understandable for most people.

> One driver for this extension was the idea of triggers using color
> to visualize states etc.
> Therefore it's not only about userspace controlling the color.
> As a trigger is bound to a led_classdev we need a led_classdev
> representing a RGB LED device.
> 
> And ok: If required the sysfs interface can be splitted into separate
> attributes for hue, saturation, and (existing) brightness.

Required.

Ok, so:

a) Do we want RGB leds to be handled by existing subsystem, or do we
need separate layer on top of that?

b) Does RGB make sense, or HSV? RGB is quite widely used in graphics,
and it is what hardware implements. (But we'd need to do gamma
correction).

c) My hardware has "acceleration engine", LED is independend from
CPU. That's rather big deal. Does yours? It seems to be quite common,
at least in cellphones.

Ideally, I'd like to have "triggers", but different ones. As in: if
charging, do yellow " .xX" pattern. If fully charged, do green steady
light. If message is waiting, do blue " x x" pattern. If none of
above, do slow white blinking. (Plus priorities of events). But that's
quite different from existing support...)

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> > One driver for this extension was the idea of triggers using color
> > to visualize states etc.
> > Therefore it's not only about userspace controlling the color.
> > As a trigger is bound to a led_classdev we need a led_classdev
> > representing a RGB LED device.
> > 
> > And ok: If required the sysfs interface can be splitted into separate
> > attributes for hue, saturation, and (existing) brightness.
> 
> Required.
> 
> Ok, so:
> 
> a) Do we want RGB leds to be handled by existing subsystem, or do we
> need separate layer on top of that?

And subquestion: if using existing subsystem, should the RGB led be
one led, or three?

Kernel currently uses three leds for one RGB led, and even before
that, there were leds such as "charging::yellow", "charging::green"
that were as close as leds in RGB module are.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

First, please Cc me on RGB color support.

> Add generic support for RGB Color LED's.
> 
> Basic idea is to use enum led_brightness also for the hue and saturation
> color components.This allows to implement the color extension w/o
> changes to struct led_classdev.
> 
> Select LEDS_RGB to enable building drivers using the RGB extension.
> 
> Flag LED_SET_HUE_SAT allows to specify that hue / saturation
> should be overridden even if the provided values are zero.
> 
> Some examples for writing values to /sys/class/leds//brightness:
> (now also hex notation can be used)
> 
> 255 -> set full brightness and keep existing color if set
> 0 -> switch LED off but keep existing color so that it can be restored
>  if the LED is switched on again later
> 0x100 -> switch LED off and set also hue and saturation to 0
> 0x00 -> set full brightness, full saturation and set hue to 0
> (red)

Umm, that's rather strange interface -- and three values in single sysfs
file is actually forbidden.

Plus, it is very much unlike existing interfaces for RGB LEDs, which
we already have supported in the tree. (At least nokia N900 and Sony
motion controller already contain supported three-color LEDs).

Now... yes, there's work to be done for the 3-color LEDs. Currently,
they are treated as three different LEDs. (Which makes some sense, you
can use "battery charging" trigger for LED, and CPU activity trigger
for green, for example). It would be good to have some kind of
grouping, so that userspace can tell "these 3 leds are actually
combined into one light".

Second, we should define that LED brightness has similar gamma to the
monitor, so that expected colors are displayed when user requests
them.

(And then.. I guess we should talk about more advanced stuff, like
hardware that can drive the LED changes independently of the main
CPU.)

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 2/4] leds: core: add color LED sysfs extension

[Pavel Machek]

On Tue 2016-03-01 22:28:00, Heiner Kallweit wrote:
> Extend brightness sysfs property handling to deal with monochrome
> and color mode as well.
> 
> Signed-off-by: Heiner Kallweit 
> ---
> v2:
> - split from patch 1
> v3:
> - moved one change (led_is_off) to patch 1
> v4:
> - changed printf format string to %#.6x
> v5:
> - no changes
> ---
>  drivers/leds/led-class.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 007a5b3..8a3748a 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -32,7 +32,10 @@ static ssize_t brightness_show(struct device *dev,
>   /* no lock needed for this */
>   led_update_brightness(led_cdev);
>  
> - return sprintf(buf, "%u\n", led_cdev->brightness);
> + if (led_cdev->brightness > LED_FULL)
> + return sprintf(buf, "%#.6x\n", led_cdev->brightness);
> + else
> + return sprintf(buf, "%u\n", led_cdev->brightness);
>  }
>  
>  static ssize_t brightness_store(struct device *dev,

No... I don't think you should change interface for existing file like
this.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 4/4] leds: core: add support for RGB LED's

[Pavel Machek]

On Tue 2016-03-01 22:36:12, Heiner Kallweit wrote:
> Export a function to convert HSV color values to RGB.
> It's intended to be called by drivers for RGB LEDs.
> 
> Signed-off-by: Heiner Kallweit 
> ---
> v2:
> - move hsv -> rgb conversion to separate file
> - remove flag LED_DEV_CAP_RGB
> v3:
> - call led_hsv_to_rgb only if LED_DEV_CAP_HSV is set
>   This is needed in cases when we have monochrome and color LEDs
>   as well in a system.
> v4:
> - Export led_hsv_to_rgb and let the device driver call it instead
>   of doing the conversion in the core
> v5:
> - don't ignore led_cdev->brightness_get silently if LED_DEV_CAP_RGB
>   is set but warn
> ---
>  drivers/leds/led-class.c|  7 +++
>  drivers/leds/led-rgb-core.c | 36 
>  include/linux/leds.h|  8 
>  3 files changed, 51 insertions(+)
> 
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 8a3748a..a4b144e 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -193,6 +193,13 @@ int led_classdev_register(struct device *parent, struct 
> led_classdev *led_cdev)
>   char name[64];
>   int ret;
>  
> + /*
> +  * for now reading back the color is not supported as multiple
> +  * HSV -> RGB -> HSV conversions may distort the color due to
> +  * rounding issues in the conversion algorithm
> +  */
> + WARN_ON(led_cdev->flags & LED_DEV_CAP_RGB && led_cdev->brightness_get);
> +

Backtrace is useless here, you may want to add some ()s and you don't
really want user to be causing messages in syslog this easily.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> > pavel@duo:~$ ls -1 /sys/class/leds/
> > tpacpi:green:batt
> > tpacpi:orange:batt
> >
> > This is physically 2 leds but hidden under one indicator, so you got
> > "off", "green", "orange" and "green+orange".
> 
> That's a good example. As long as you can recognize green+orange as
> separate lights/colors
> (w/o magnifying glass) I wouldn't call it "a LED with multiple colors"
> but "multiple
> LED devices".

Well, that's how it is currently handled. But for the user, it looks
as a LED with multiple colors.

> In my use case we talk about RGB LEDs like the commonly used 5050 SMD RGB 
> LEDs.
> And it's not only about using a handful of discrete colors but about
> displaying any arbitrary
> color.
> So far the kernel exposes the physical RGB LEDs as separate LEDs only
> and I can't use
> a trigger to e.g. set "magenta with 50% brightness".

Why not?

What do you do if you want to display magenta on your LCD?

You compute RGB values, then display them.

What would you do for the LEDs? Same thing.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> > To be fair... they _are_ separate LED devices. In N900 case, you can
> > even see light comming from slightly different places if you look closely.
> > 
> I mainly work with encapsulated USB HID LED devices like Thingm blink(1).
> Due to the diffuse plastic cover you don't see the individual LEDs on the 
> chip.

Yeah, so on N900, you can't really see the individual LEDs, either.
But white is not uniform white.

On PS/3 motion controller (another device that is already supported),
diffusing works a bit better. 

> > Required.
> > 
> > Ok, so:
> > 
> > a) Do we want RGB leds to be handled by existing subsystem, or do we
> > need separate layer on top of that?
> > 
> > b) Does RGB make sense, or HSV? RGB is quite widely used in graphics,
> > and it is what hardware implements. (But we'd need to do gamma
> > correction).
> > 
> HSV has the charme that the current monochrome V-only is a subset.
> Therefore the current API can be used also with color LEDs.
> However there might be good reasons for using RGB too.

Yes, nice, but we already have RGB LED support in kernel, and it looks
different from what is proposed here. And quite incompatible. 

> > c) My hardware has "acceleration engine", LED is independend from
> > CPU. That's rather big deal. Does yours? It seems to be quite common,
> > at least in cellphones.
> > 
> Devices like blink(1) support storing and re-playing patterns, fading etc.
> 
> > Ideally, I'd like to have "triggers", but different ones. As in: if
> > charging, do yellow " .xX" pattern. If fully charged, do green steady
> > light. If message is waiting, do blue " x x" pattern. If none of
> > above, do slow white blinking. (Plus priorities of events). But that's
> > quite different from existing support...)
> > 
> I think for this a separate layer would be helpful.
> Your primary intention is to e.g. display "charging" on the RGB LED
> device. Most likely you don't want to split yellow into its red + green
> component and then write to the respective (sub-)LEDs.
> Also just think of the case that later you might decide that orange
> is nicer than yellow.

Well, so what about keeping existing red/green/blue LED devices (to
stay backward compatible) and then add separate device that links to
these, and controls patterns and colors?

Small complication is that (at least on N900) the pattern capability
can control keyboard backlight LEDs, too. It has nine channels, and
you select 3 channels that are connected to pattern generator.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> >Ideally, I'd like to have "triggers", but different ones. As in: if
> >charging, do yellow " .xX" pattern. If fully charged, do green steady
> >light. If message is waiting, do blue " x x" pattern. If none of
> >above, do slow white blinking. (Plus priorities of events). But that's
> >quite different from existing support...)
> 
> Please note that HSV colour scheme allows to neatly project monochrome
> brightness semantics on the RGB realm. I.e. you can have fixed
> hue and saturation, and by changing the brightness component a perceived
> colour intensity can be altered.

Yes, that's nice, but it is incompatible with already existing RGB
support in kernel. Plus, echoing hue into file called brightness is
extremely ugly, and it violates sysfs rules.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v8 1/4] gadget: Introduce the usb charger framework

[Pavel Machek]

Hi!

> +/*
> + * Sysfs attributes:
> + *
> + * These sysfs attributes are used for showing and setting different type
> + * (SDP/DCP/CDP/ACA) chargers' current limitation.
> + */
> +static ssize_t sdp_limit_show(struct device *dev,
> +   struct device_attribute *attr,
> +   char *buf)
> +{
> + struct usb_charger *uchger = dev_to_uchger(dev);
> +
> + return sprintf(buf, "%d\n", uchger->cur_limit.sdp_cur_limit);
> +}
> +

Sysfs attributes... can we get appropriate documentation for them?

Thanks,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v7 1/4] gadget: Introduce the usb charger framework

[Pavel Machek]

Hi!

> >> > a) you are connected to a dedicated charger
> >> >
> >> > In this case, you can get up to 2000mA depending on the charger.
> >> >
> >> > If $this charger can give you or not 2000mA is not detectable,
> >> > so what do charging ICs do ? They slowly increase the attached
> >> > load accross VBUS/GND and measure VBUS value. When IC notices
> >> > VBUS dropping bit, step back to previous load.
> >> >
> >> > This means you will always charger with maximum rating of DCP.
> >> >
> >> > Why would user change this ? More is unsafe, less is just
> >> > stupid.
> >
> > Less is not neccessarily stupid. First, it is useful for debugging, second, 
> > you
> > don't know how much this charger can give you. You measured you can get 
> > 1.8A,
> > but the note on the charger says 1.5A. You may want to go with 1.5A.
> >
> > Also, there are several incompatible standards for detecting
> > "dedicated charger". IIRC iPhone has different one from iPad. So it is
> > quite important to be able to control this manually.
> 
> manually ??? Hell no! Charger IC should be able to do this no
> problem. I would be surprised if there's any charger IC out there which
> blindly connects a 1.8A load from the start. What these ICs do is that
> they slowly increment the load and check voltage level. They'll continue
> to do that up to the maximum you listed (1.8A, let's say). As soon as
> voltage drops a bit, charger IC knows that it use previous load.

As I explained, if the note on the wall charger says 1.5A, you want to
do 1.5A, not 1.8A. You can measure voltage on the charger, but you
don't know its temperature.

> >> > d) you are connected to a standard port and get enumerated with your
> >> > 100mA configuration.
> >> >
> >> > you *know* 100mA is okay. So you connect a 100mA load and get it
> >> > over with.
> >> >
> >> > This means you will always charger with maximum rating for this
> >> > SDP.
> >> >
> >> > Why would user change this ? More is unsafe, less is just
> >> > stupid.
> >
> > I've needed to override 100mA default many times. Maybe it is unsafe,
> > but it is useful.
> 
> still unsafe. If you really wanna do that, you're welcome to removing
> safety margins from your own kernel, but we're definitely not going to
> ship this to millions of users.

Not more unsafe than loading wall chargers with "lets see how much we
can get out of it" algorithm. Plus actually required to charge your
machines in useful way. So it is important that common API
exists. Whether it gets enalbed at production is different question.

Unfortunately, there's more than one standard for detecting charger,
so manual control will probably be required.

> > (And with USB 5V connected directly to pretty beefy PC power supply...
> > it is sometimes safer than it looks).
> 
> you're not considering the thermal dissipation on the USB connector
> itself. Many of them might not use good metals because they assume the
> maximum power dissipated is 500mA * 5V = 2.5W. If you try to draw more,
> you could, literally, melt the connector.

If you are dissipating 2.5W at the connector, you are doing something
very wrong. You should not be short-circuiting your USB... even when
the ports are usually designed to survive that.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v7 1/4] gadget: Introduce the usb charger framework

[Pavel Machek]

Hi!

> >> >>> +#define DEFAULT_SDP_CUR_LIMIT(500 - DEFAULT_CUR_PROTECT)
> >> >>
> >> >> According to the spec we should always be talking about unit loads (1
> >> >> unit load is 100mA for HS/FS/LS and 150mA for SS). Also, this will not
> >> >> work for SS capable ports and SS gadgets (we have quite a few of them,
> >> >> actually). You're missing the opportunity of charging at 900mA.
> >> >
> >> > I follow the DCP/SDP/CDP/ACA type's default current limitation and
> >> > user can set them what they want.
> >> 
> >> no, the user CANNOT set it to what they want. If you get enumerated
> >> @100mA and the user just decides to set it to 2000mA, s/he could even
> >> melt the USB connector. The kernel _must_ prevent such cases.
> >
> > root should be allowed to do that.
> 
> root should not be allowed to put user at risk. The usb connector is a
> path to earth ground in most (all?) desktop computers. Allowing root to
> put that connector in a situation where it could melt the connector and
> short mains should never be allowed.

Be sure that hardware people put reasonable precautions...

> > Very often, you want to charge using 1.8A from an old desktop PC.
> 
> if that old desktop's port is not a charging port, you shouldn't be
> allowed to do that. Not ever.

Yes, Felipe just decided that I should not be able to charge my N900
in useful way.

> > N900 will simply not charge from .5A.
> 
> it used to with original maemo userspace/kernel.

Yeah, at about 10% per night.

> >> a) you are connected to a dedicated charger
> >> 
> >>In this case, you can get up to 2000mA depending on the charger.
> >> 
> >>If $this charger can give you or not 2000mA is not detectable,
> >>so what do charging ICs do ? They slowly increase the attached
> >>load accross VBUS/GND and measure VBUS value. When IC notices
> >>VBUS dropping bit, step back to previous load.
> >> 
> >>This means you will always charger with maximum rating of DCP.
> >> 
> >>Why would user change this ? More is unsafe, less is just
> >>stupid.
> >
> > Actually, less is not stupid. Charging li-ion battery from li-ion battery 
> > might
> > be stupid. Imagine I'm on train, with device like N900 (50% battery) and 
> > power bank
> > (3Ah). I'm actively using the device. If I let it charge at full current, 
> > I'll waste
> > energy. If I limit current to approximately the power consumption, it will 
> > run the
> > powerbank empty, first, then empty the internal battery, maximizing total 
> > time I
> > can use the device.
> 
> why would you waste energy ? What the charger chip would do is charge
> battery to maximum then just to maintenance charge from that point
> on. Where is energy being wasted other than normal heat dissipation ?

Physics 101, of course wasted energy goes to heat. Lets not waste
energy by charging li-ion from li-ion when it is not required.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v7 1/4] gadget: Introduce the usb charger framework

[Pavel Machek]

On Mon 2016-04-18 13:55:17, Felipe Balbi wrote:
> 
> Hi,
> 
> Felipe Balbi  writes:
> >> But cellphone user knows what he connected his charger to, and that's
> >> why it is useful to be able to lower the current. Even when you said
> >> "less is just stupid" I demonstrated it is not, at least in case when
> 
> and btw, you haven't demonstrated anything. You merely stated that it
> isn't without references or numbers, or any source of trustworthy
> information. I'm not really into 'believing'.

You are not really into reading, either, it seems. Or into the
electronics. Or into physics.

You seem to understand that charging li-ion from li-ion produces too
much heat. (And yes, it does, DC-DC convertors are not 100%
effective). Converting energy into heat is not a good idea.

If you need numbers or references to understand basic physics, you'll
need to google it yourself, I'm afraid. 

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v7 1/4] gadget: Introduce the usb charger framework

[Pavel Machek]

On Mon 2016-04-18 13:30:54, Felipe Balbi wrote:
> 
> Hi,
> 
> Pavel Machek <pa...@ucw.cz> writes:
> >> > Very often, you want to charge using 1.8A from an old desktop PC.
> >> 
> >> if that old desktop's port is not a charging port, you shouldn't be
> >> allowed to do that. Not ever.
> >
> > Yes, Felipe just decided that I should not be able to charge my N900
> > in useful way.
> 
> you can do whatever you want with *your* kernel binary, but we're not
> gonna ship something potentially dangerous. If that PC port is telling
> you it can only allow 100mA, you should *not* be allowed to overcome
> that limitation from the device side, sorry.

Yes, and if your cpu tells you it can only run on 2GHz, you should not
be able to run it on 2.1GHz. And you should not be able to use
non-VESA VGA modes, because you could overheat coils in the monitor.

You are shipping something potentially dangerous already, because you
know, USB-A-to-micro-B cables with D+/D- connected to simulate charger
are actually very common. The world did not end, yet, so it is clearly
not as bad as you make it be.

> >> >> a) you are connected to a dedicated charger
> >> >> 
> >> >> In this case, you can get up to 2000mA depending on the charger.
> >> >> 
> >> >> If $this charger can give you or not 2000mA is not detectable,
> >> >> so what do charging ICs do ? They slowly increase the attached
> >> >> load accross VBUS/GND and measure VBUS value. When IC notices
> >> >> VBUS dropping bit, step back to previous load.
> >> >> 
> >> >> This means you will always charger with maximum rating of DCP.
> >> >> 
> >> >> Why would user change this ? More is unsafe, less is just
> >> >> stupid.
> >> >
> >> > Actually, less is not stupid. Charging li-ion battery from li-ion 
> >> > battery might
> >> > be stupid. Imagine I'm on train, with device like N900 (50% battery) and 
> >> > power bank
> >> > (3Ah). I'm actively using the device. If I let it charge at full 
> >> > current, I'll waste
> >> > energy. If I limit current to approximately the power consumption, it 
> >> > will run the
> >> > powerbank empty, first, then empty the internal battery, maximizing 
> >> > total time I
> >> > can use the device.
> >> 
> >> why would you waste energy ? What the charger chip would do is charge
> >> battery to maximum then just to maintenance charge from that point
> >> on. Where is energy being wasted other than normal heat dissipation ?
> >
> > Physics 101, of course wasted energy goes to heat. Lets not waste
> > energy by charging li-ion from li-ion when it is not required.
> 
> your cellphone has no means to know that it's connected to a Li-Ion
> battery. We don't have visibility on what we're connected to, just how
> much it can source.

But cellphone user knows what he connected his charger to, and that's
why it is useful to be able to lower the current. Even when you said
"less is just stupid" I demonstrated it is not, at least in case when
your power source is a battery.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v7 1/4] gadget: Introduce the usb charger framework

[Pavel Machek]

Hi!

> >> manually ??? Hell no! Charger IC should be able to do this no
> >> problem. I would be surprised if there's any charger IC out there which
> >> blindly connects a 1.8A load from the start. What these ICs do is that
> >> they slowly increment the load and check voltage level. They'll continue
> >> to do that up to the maximum you listed (1.8A, let's say). As soon as
> >> voltage drops a bit, charger IC knows that it use previous load.
> >
> > As I explained, if the note on the wall charger says 1.5A, you want to
> > do 1.5A, not 1.8A. You can measure voltage on the charger, but you
> > don't know its temperature.
> 
> phone can't read what it says in the wall charger, nor can it know that
> it's connected charger ABC and not charger XYZ. Think of the user
> experience. You can't expect users to tell you "okay phone, the charger
> reads that maximum is 1.5A, so please don't go over that."

Of course, we may do something sensible by default. But manual
controls should still be present. You called them "stupid" but they
are not.

Note that just because you detected wall charger does not even mean
you are connected to wall charger. See the link below.

> >> still unsafe. If you really wanna do that, you're welcome to removing
> >> safety margins from your own kernel, but we're definitely not going to
> >> ship this to millions of users.
> >
> > Not more unsafe than loading wall chargers with "lets see how much we
> > can get out of it" algorithm. Plus actually required to charge your
> 
> it actually _is_ more unsafe. You could burn mother boards with that. If
> host tells you it only has 100mA power budget left, why are you trying
> to get more ?

No, you can't burn motherboard like that... You can force emergency
shutdowns, which is also bad, but... There are many devices that break
this aspect of USB protocol.

https://www.kickstarter.com/projects/1785889318/doubbletime-charging-cable-full-battery-in-1-2-the

> > Unfortunately, there's more than one standard for detecting charger,
> > so manual control will probably be required.
> 
> n900 never had manual control of anything. It was just using information
> given by the battery IC, charger IC and twl4030 madc.

Manual control of n900 charging is done by:

echo 1800 > /sys/class/power_supply/bq24150a-0/current_limit

> >> > (And with USB 5V connected directly to pretty beefy PC power supply...
> >> > it is sometimes safer than it looks).
> >> 
> >> you're not considering the thermal dissipation on the USB connector
> >> itself. Many of them might not use good metals because they assume the
> >> maximum power dissipated is 500mA * 5V = 2.5W. If you try to draw more,
> >> you could, literally, melt the connector.
> >
> > If you are dissipating 2.5W at the connector, you are doing something
> > very wrong. You should not be short-circuiting your USB... even when
> > the ports are usually designed to survive that.
> 
> yes. You shouldn't. You also shouldn't go over that limit. If you have a
> 500mA total power budget, we should not let anybody try to draw more
> because we have no control over what's on the side of the wire.

They already can go over the limit, for example using cable linked
above. I have several such cables here. I also have various wall
supplies that are not detected as a wall supply by N900. So I either
have to remember and connect them with the "special" cable, or
(easier) use the override above to get useful charging.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v7 1/4] gadget: Introduce the usb charger framework

[Pavel Machek]

Hi!

On Mon 2016-04-18 10:59:23, David Laight wrote:
> From: Pavel Machek
> > Sent: 18 April 2016 11:40
> ...
> > > >> > Actually, less is not stupid. Charging li-ion battery from li-ion 
> > > >> > battery might
> > > >> > be stupid. Imagine I'm on train, with device like N900 (50% battery) 
> > > >> > and power bank
> > > >> > (3Ah). I'm actively using the device. If I let it charge at full 
> > > >> > current, I'll waste
> > > >> > energy. If I limit current to approximately the power consumption, 
> > > >> > it will run the
> > > >> > powerbank empty, first, then empty the internal battery, maximizing 
> > > >> > total time I
> > > >> > can use the device.
> > > >>
> > > >> why would you waste energy ? What the charger chip would do is charge
> > > >> battery to maximum then just to maintenance charge from that point
> > > >> on. Where is energy being wasted other than normal heat dissipation ?
> > > >
> > > > Physics 101, of course wasted energy goes to heat. Lets not waste
> > > > energy by charging li-ion from li-ion when it is not required.
> > >
> > > your cellphone has no means to know that it's connected to a Li-Ion
> > > battery. We don't have visibility on what we're connected to, just how
> > > much it can source.
> > 
> > But cellphone user knows what he connected his charger to, and that's
> > why it is useful to be able to lower the current. Even when you said
> > "less is just stupid" I demonstrated it is not, at least in case when
> > your power source is a battery.
> 
> It reality you may want the phone/tablet to be configurable to take power
> from USB, but disable the li-ion charging circuit.
> That will maximise the time you get when running from an external battery.
> I connect my tablet to the 1A output (which discharges the internal battery
> slowly) rather than the 2A one (which will charge it with some
> cables).

Yes, being able to power device from external without charging the
battery is useful, too.

But I'd still like to control individual currents, too. If I have
power bank and two devices, I may want to select which one charges
faster.

Best regards,   
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v7 1/4] gadget: Introduce the usb charger framework

[Pavel Machek]

Hi!

> > Of course, we may do something sensible by default. But manual
> > controls should still be present. You called them "stupid" but they
> > are not.
> >
> > Note that just because you detected wall charger does not even mean
> > you are connected to wall charger. See the link below.
> 
> that's a horrible product. So what ? If you want to use that, be my
> guest. Just, again, don't ask for support when things start falling
> apart ;-)

So you have USB spec? So what? There are many such products out there,
and I have at least two such cables here.

They did not cause end of the world, yet, and they are actually very useful.

> >> >> still unsafe. If you really wanna do that, you're welcome to removing
> >> >> safety margins from your own kernel, but we're definitely not going to
> >> >> ship this to millions of users.
> >> >
> >> > Not more unsafe than loading wall chargers with "lets see how much we
> >> > can get out of it" algorithm. Plus actually required to charge your
> >> 
> >> it actually _is_ more unsafe. You could burn mother boards with that. If
> >> host tells you it only has 100mA power budget left, why are you trying
> >> to get more ?
> >
> > No, you can't burn motherboard like that... You can force emergency
> > shutdowns, which is also bad, but... There are many devices that break
> > this aspect of USB protocol.
> >
> > https://www.kickstarter.com/projects/1785889318/doubbletime-charging-cable-full-battery-in-1-2-the
> 
> we can't prevent people from coming up with bad devices/cables/whatever,
> right ? But we can make sure that overcoming a 500mA power budget on a
> USB 2.0 port will not be allowed.

No, you can't, because you don't know if you are connected to USB 2.0
port. (Because non-compliant cables exist).

> >> > Unfortunately, there's more than one standard for detecting charger,
> >> > so manual control will probably be required.
> >> 
> >> n900 never had manual control of anything. It was just using information
> >> given by the battery IC, charger IC and twl4030 madc.
> >
> > Manual control of n900 charging is done by:
> >
> > echo 1800 > /sys/class/power_supply/bq24150a-0/current_limit
> 
> yes, that's fine. And if you're connected to a dedicated charger (DCP)
> which follows USB Battery Charger specification, we *know* that it
> should, per the spec, source up to 2A, so this is fine.

BTW have you ever seen such USB-compliant dedicated charger? I have
more than 5 chargers here, and not one of them is 2A. (Most are .5A,
some are 1A, one is 1.2A).

> However, if you're connected to SDP (regular PC port), which has a power
> budget of 500mA per-port (meaning, that if you're behind a bus powered
> hub, you can't even get 500mA), then this write() of yours should be
> invalid. It should *not* be a successful write() as that creates an
> unsafe and potentially dangerous scenario for the user.

Yes, USB is "potentially dangerous", because noone follows the
specs. Fortunately, everyone knows (except you?) that noone follows
the specs, so the hardware can deal with that, and they include
(poly?) fuses where neccessary.

> > They already can go over the limit, for example using cable linked
> > above. I have several such cables here. I also have various wall
> 
> people can use unsupported cable assemblies if they want, but you can't
> expect kernel to support you.

Then we won't have useful charging support in kernel.

> > supplies that are not detected as a wall supply by N900. So I either
> > have to remember and connect them with the "special" cable, or
> > (easier) use the override above to get useful charging.
> 
> those supplies are not supported by N900. N900 was designed with USB
> Battery Chaging specification in mind and Nokia is not around anymore to
> give you SW updates. Sorry, but that's not the kernel's fault.
> 
> The point is the following: there are a handful of people who would
> *know* how to fiddle with these limits, many would not. The vast
> majority would not. And, considering this is something completely out of
> spec, and, again, potentially dangerous to the user, we are not going to
> support it.

You speak about dangerous where little danger exist; number of
non-compliant cables and USB devices is very high (take your power
bank. Does it really limit to .5A when charging from computer?) and we
should support them, not cry "dangerous" and force everyone to come
with their own "solutions".

> You may use your hacked up cables, not a problem. I did that myself
> during N900 development (though I was using a lab power supply with a 2A
> current limit sourcing 5V) to test port type detection and charging
> algorithm. But that's really not something any company (or this
> community) will support.

Fortunately, that's not your decision and community already decided
the other way.
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 

Re: [PATCH v7 1/4] gadget: Introduce the usb charger framework

[Pavel Machek]

On Mon 2016-04-18 14:42:58, Felipe Balbi wrote:
> 
> Hi,
> 
> Pavel Machek <pa...@ucw.cz> writes:
> > On Mon 2016-04-18 13:55:17, Felipe Balbi wrote:
> >> 
> >> Hi,
> >> 
> >> Felipe Balbi <ba...@kernel.org> writes:
> >> >> But cellphone user knows what he connected his charger to, and that's
> >> >> why it is useful to be able to lower the current. Even when you said
> >> >> "less is just stupid" I demonstrated it is not, at least in case when
> >> 
> >> and btw, you haven't demonstrated anything. You merely stated that it
> >> isn't without references or numbers, or any source of trustworthy
> >> information. I'm not really into 'believing'.
> >
> > You are not really into reading, either, it seems. Or into the
> > electronics. Or into physics.
> >
> > You seem to understand that charging li-ion from li-ion produces too
> > much heat. (And yes, it does, DC-DC convertors are not 100%
> > effective). Converting energy into heat is not a good idea.
> >
> > If you need numbers or references to understand basic physics, you'll
> > need to google it yourself, I'm afraid. 
> 
> still doesn't change the fact that you haven't demonstrated anything. If
> you can't be helpful and/or constructive, you may also go away and use
> whatever unsafe setup you want to use.

So you still claim that it is "stupid" to charge at anything lower
than maximum allowed current?

Even after I described the example with device running off power bank
on a train? Would you explain your reasoning?
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v5 1/4] leds: core: add generic support for RGB Color LED's

[Pavel Machek]

Hi!

> >>How about implementing patterns as a specific typer of triggers?
> >>Let's say we have ledtrig-rgb-pattern:
> >
> >Well, we'd need ledtrig-rgb-pattern-1, ledtrig-rgb-pattern-2, ... , as we
> >can have more than one rgb led. But yes.
> 
> Triggers can have many listeners, i.e. led_trigger_event() sets
> brightness on all LED class devices registered on given trigger.
> We could have led_trigger_rgb_event() that would set brightness
> on all groups-of-three LEDs registered on given rgb-trigger.

I do not understand that.

> I agree that ledtrig-rgb-pattern-1, ledtrig-rgb-pattern-2, etc. would
> be also needed to add a capability of setting different colors on
> different LED devices.

Ok.

> >For patterns, I'd suggest array of (r g b time) values.
> >
> >Pattern engines can do stuff like "slowly turn LED from off to red, then 
> >switch color to
> >white, then slowly turn it to yellow, then turn it off at once" with defined 
> >speeds
> >for "slowly" and option of either linear on non-linear brightness ramping.
> >
> >The last option might be a bit too much, but I believe we should support the 
> >rest.
> 
> Yes, that's an interesting idea. It also turns out that trigger based
> patterns could be also used for defining generic patterns for a group
> of monochrome LEDs.

Yes, controlling monochrome LEDs synchronously is another task for
patterns. Actually, N900 uses that to control 6 keyboard backlight
LEDs synchronously... and yes, it would be somehow nice to preserve
this functionality.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regression - SATA disks behind USB ones on v4.8-rc1, breaking boot. [Re: Who reordered my disks (probably v4.8-rc1 problem)]

[Pavel Machek]

On Sun 2016-08-14 17:34:18, Tom Yan wrote:
> Since when it is expected that SATA disks will always be probed before
> USB disks? We can't guarantee that even if we make sure all ata
> drivers are loaded before usb-storage/uas. That's why we need
> consistent namings (e.g. /dev/disk/by-id/*).

Since SATA support was merged, certainly since v2.4, and from way
before /dev/disk/by-id existed.

People may not run udev, and you can't use /dev/disk/by-id on kernel
command line.

Pavel

> On 14 August 2016 at 17:20, Pavel Machek <pa...@ucw.cz> wrote:
> > Hi!
> >
> >> It seems that in v4.8-rc0, /dev/sdX got reordered, and now USB devices
> >> are probed before SATA drivers. That is pretty anti-social. It
> >> broke my boot on my primary machine, and unfortunately due to BIOS
> >> problems (keyboard does not work when connected through a hub) it is
> >> less fun than it should be.
> >
> > If you know which commit caused the reordering, that would be helpful.
> >
> > v4.1 seems to be ok: SATA disk is sda, as expected.
> >
> > v4.4 seems to be ok: SATA disk is sda, as expected.
> >
> > I'll test v4.6 next.
> >
> > v4.8-rc1: SATA disk is sde, behind USB card readers. Not helpful.
> >
> > Best regards,
> > 
> > Pavel
> > --
> > (english) http://www.livejournal.com/~pavelmachek
> > (cesky, pictures) 
> > http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Regression - SATA disks behind USB ones on v4.8-rc1, breaking boot. [Re: Who reordered my disks (probably v4.8-rc1 problem)]

[Pavel Machek]

Hi!

> It seems that in v4.8-rc0, /dev/sdX got reordered, and now USB devices
> are probed before SATA drivers. That is pretty anti-social. It
> broke my boot on my primary machine, and unfortunately due to BIOS
> problems (keyboard does not work when connected through a hub) it is
> less fun than it should be.

If you know which commit caused the reordering, that would be helpful.

v4.1 seems to be ok: SATA disk is sda, as expected.

v4.4 seems to be ok: SATA disk is sda, as expected.

I'll test v4.6 next.

v4.8-rc1: SATA disk is sde, behind USB card readers. Not helpful.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regression - SATA disks behind USB ones on v4.8-rc1, breaking boot. [Re: Who reordered my disks (probably v4.8-rc1 problem)]

[Pavel Machek]

On Sun 2016-08-14 11:20:44, Pavel Machek wrote:
> Hi!
> 
> > It seems that in v4.8-rc0, /dev/sdX got reordered, and now USB devices
> > are probed before SATA drivers. That is pretty anti-social. It
> > broke my boot on my primary machine, and unfortunately due to BIOS
> > problems (keyboard does not work when connected through a hub) it is
> > less fun than it should be.
> 
> If you know which commit caused   the reordering, that would be helpful.
> 
> v4.1 seems to be ok: SATA disk is sda, as expected.
> 
> v4.4 seems to be ok: SATA disk is sda, as expected.

v4.6 seems to be ok.

v4.7 SATA disk is sde, behind USB card readers. Not helpful.

Ouch. So we have 'stable' kernel with this. Much more fun :-(.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regression - SATA disks behind USB ones on v4.8-rc1, breaking boot. [Re: Who reordered my disks (probably v4.8-rc1 problem)]

[Pavel Machek]

Hi!

On Sun 2016-08-14 18:17:39, Tom Yan wrote:
> On 14 August 2016 at 18:07, Tom Yan <tom.t...@gmail.com> wrote:
> > On 14 August 2016 at 18:01, Pavel Machek <pa...@ucw.cz> wrote:
> >>
> >> Since SATA support was merged, certainly since v2.4, and from way
> >> before /dev/disk/by-id existed.
> >
> > I have no idea how "SATA before USB" had been done in the past (if it
> > was ever a thing in the kernel), but that has not been the case since
> > at least v3.0 AFAIR.

It is the case in v4.6. We had change hda->sda for SATA drives long
time ago, it was stable since that.

> > No, but you can always use root=PARTUUID=, that's built into the
> > kernel. (root=UUID= requires udev or so though).
> 
> Silly me. root=UUID= has nothing to do with udev, but `blkid` in
> util-linux. At least that's how it's done in Arch/mkinitcpio.

I'd rather not mess with initrd, and initrd was not required in the
past.

kernel-parameters.txt only mentions UUID= in connection with
resume. Is the documentation correct?
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regression - SATA disks behind USB ones on v4.8-rc1, breaking boot. [Re: Who reordered my disks (probably v4.8-rc1 problem)]

[Pavel Machek]

On Sun 2016-08-14 10:56:54, Alan Stern wrote:
> On Sun, 14 Aug 2016, Pavel Machek wrote:
> 
> > > That being said, it would be great if the original reporter could use
> > > 'git bisect' and let the linux-usb and linux-scsi mailing list know what
> > > the offending patch is, and we can take it from there.
> > 
> > Original reporter is me :-(.
> > 
> > Yes, I can do bisect, if required. I'd like some kind of confirmation
> > that it happens on other systems...
> 
> Can you post a kernel log from one of those unsuccessful boots
> (netconsole or something or the sort)?

Even scrollback does not seem to work well on modern kernels. So
sorry, no easy way to get a kernel log :-(.
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regression - SATA disks behind USB ones on v4.8-rc1, breaking boot. [Re: Who reordered my disks (probably v4.8-rc1 problem)]

[Pavel Machek]

On Sun 2016-08-14 12:14:38, Pavel Machek wrote:
> On Sun 2016-08-14 11:20:44, Pavel Machek wrote:
> > Hi!
> > 
> > > It seems that in v4.8-rc0, /dev/sdX got reordered, and now USB devices
> > > are probed before SATA drivers. That is pretty anti-social. It
> > > broke my boot on my primary machine, and unfortunately due to BIOS
> > > problems (keyboard does not work when connected through a hub) it is
> > > less fun than it should be.
> > 
> > If you know which commit caused the reordering, that would be helpful.
> > 
> > v4.1 seems to be ok: SATA disk is sda, as expected.
> > 
> > v4.4 seems to be ok: SATA disk is sda, as expected.
> 
> v4.6 seems to be ok.

v4.7-rc1 seems to be ok?! 9956d572
v4.7-rc3 seems still to be ok.
v4.7-rc4: still ok.
v4.7-rc5: ok.
v4.7-rc6: problem

Bisecting now, 8 steps to go. As it was introduced between -rc5 and
-rc6, bisect should not be too scary.

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regression - SATA disks behind USB ones on v4.8-rc1, breaking boot. [Re: Who reordered my disks (probably v4.8-rc1 problem)]

[Pavel Machek]

Hi!

> > > > I have no idea how "SATA before USB" had been done in the past (if it
> > > > was ever a thing in the kernel), but that has not been the case since
> > > > at least v3.0 AFAIR.
> > > > 
> > > > > 
> > > > > People may not run udev, and you can't use /dev/disk/by-id on kernel
> > > > > command line.
> > > > > 
> > > > 
> > > > No, but you can always use root=PARTUUID=, that's built into the
> > > > kernel. (root=UUID= requires udev or so though).
> > > 
> > > Silly me. root=UUID= has nothing to do with udev, but `blkid` in
> > > util-linux. At least that's how it's done in Arch/mkinitcpio.
> > 
> > The rule is "don't break working systems", not "but we are allowed to break
> > systems, see it says here not to depend on this"
> > 
> > Drive ordering has been stable since the 0.1 kernel [1]
> 
> Drive probing order of USB has always been non-deterministic, so while I
> agree that it is not good to break existing systems at all, perhaps this
> is on the edge of what works vs. doesn't work?

Yeah, USB order is known to be random. But root=/dev/sda (when sda is
on SATA) is very old, and it would be good to keep it.

> I know my USB drives always seem to come up in random order, which is
> why tools like udev were invented :)
> 
> > It takes a lot longer to detect USB drives, why in the world would they be
> > detected before hard-wired drives?
> 
> Depends, some hard-wired drives take much longer to find than USB ones.
> 
> That being said, it would be great if the original reporter could use
> 'git bisect' and let the linux-usb and linux-scsi mailing list know what
> the offending patch is, and we can take it from there.

Original reporter is me :-(.

Yes, I can do bisect, if required. I'd like some kind of confirmation
that it happens on other systems...

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regression - SATA disks behind USB ones on v4.8-rc1, breaking boot. [Re: Who reordered my disks (probably v4.8-rc1 problem)]

[Pavel Machek]

On Sun 2016-08-14 18:06:53, Pavel Machek wrote:
> On Sun 2016-08-14 12:14:38, Pavel Machek wrote:
> > On Sun 2016-08-14 11:20:44, Pavel Machek wrote:
> > > Hi!
> > > 
> > > > It seems that in v4.8-rc0, /dev/sdX got reordered, and now USB devices
> > > > are probed before SATA drivers. That is pretty anti-social. It
> > > > broke my boot on my primary machine, and unfortunately due to BIOS
> > > > problems (keyboard does not work when connected through a hub) it is
> > > > less fun than it should be.
> > > 
> > > If you know which commit caused   the reordering, that would be helpful.
> > > 
> > > v4.1 seems to be ok: SATA disk is sda, as expected.
> > > 
> > > v4.4 seems to be ok: SATA disk is sda, as expected.
> > 
> > v4.6 seems to be ok.
> 
> v4.7-rc1 seems to be ok?! 9956d572
> v4.7-rc3 seems still to be ok.
> v4.7-rc4: still ok.
> v4.7-rc5: ok.
> v4.7-rc6: problem
> 
> Bisecting now, 8 steps to go. As it was introduced between -rc5 and
> -rc6, bisect should not be too scary.

Huh. Scary.

I bisected it to

[ba34a65324259082dc6d2924cb82d562db1c6a6b] drm/i915: Avoid early
timeout during AUX transfers

pavel@amd:/data/l/linux$ git bisect log
# bad: [a99cde438de0c4c0cecc1d1af1a55a75b10bfdef] Linux 4.7-rc6
# good: [a8ef0490c3583c3a83fa52ebe60abb8fd7d3c2a4] Merge commit
'4c2e07c6a29e0129e975727b9f57eede813eea85' into bisect-v4.7
git bisect start 'a99cde438de0c4c0cecc1d1af1a55a75b10bfdef'
'a8ef0490c3583c3a83fa52ebe60abb8fd7d3c2a4'
# good: [4c2e07c6a29e0129e975727b9f57eede813eea85] Linux 4.7-rc5
git bisect good 4c2e07c6a29e0129e975727b9f57eede813eea85
# good: [751ad819b0bf443ad8963eb7bfbd533e6a463973] Merge tag
'mac80211-for-davem-2016-06-29-v2' of
git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211
git bisect good 751ad819b0bf443ad8963eb7bfbd533e6a463973
# good: [aa7a6c8e5252ba28f36a8f87b9acd6a726aa3ae5] Merge tag
'iommu-fixes-v4.7-rc5' of
git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu
git bisect good aa7a6c8e5252ba28f36a8f87b9acd6a726aa3ae5
# good: [dbdc3bb74faeec5fd92e28c15c945045d5aab426] Merge tag
'acpi-4.7-rc6' of
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
git bisect good dbdc3bb74faeec5fd92e28c15c945045d5aab426
# good: [467ce7693f5111c11daeb75e02eba2ab9ee6f334] Merge tag
'spi-fix-v4.7-rc5' of
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi
git bisect good 467ce7693f5111c11daeb75e02eba2ab9ee6f334
# bad: [88c087109b5fedaf9b61839d08543fdaf9d5ec39] Merge tag
'drm-intel-fixes-2016-06-30' of
git://anongit.freedesktop.org/drm-intel into drm-fixes
git bisect bad 88c087109b5fedaf9b61839d08543fdaf9d5ec39
# bad: [cab103274352721b77fc5923a631fc63350bef8e] drm/i915: Fix
missing unlock on error in i915_ppgtt_info()
git bisect bad cab103274352721b77fc5923a631fc63350bef8e
# good: [fa7c13e5b1e2076b0ec716ed584ab76f9e65b7a6] drm/i915/hsw: Avoid
early timeout during LCPLL disable/restore
git bisect good fa7c13e5b1e2076b0ec716ed584ab76f9e65b7a6
# bad: [42482b4546336ecd93acdd95e435bafd7a4c581c] drm/i915: Add more
Kabylake PCI IDs.
git bisect bad 42482b4546336ecd93acdd95e435bafd7a4c581c
pavel@amd:/data/l/linux$

I reverted ba34a65324259082dc6d2924cb82d562db1c6a6b on top of
4.8-rc1+, and it started booting.

Now the question is... what is going on there...?
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Is it ok if ModemManager process is killed AFTER network-interface is brought up and IP-Address assigned?

[Pavel Machek]

Hi!

> > We are using Sierra's USB-to-WWAN driver on Ubuntu-14 for Sierra's
> > MC8090 modem, and we have a requirement wherein we need to have access
> > to the modem-serial-port (from our user-application that is).
> > 
> > Right now, we see that /usr/sbin/ModemManager is always connected to
> > /dev/ttyUSB3 (which means we cannot connect to the port from our
> > application at the same time, or even if we can, received-data will be
> > at best inconsistent).
> > 
> > 
> > We are thinking of the following ::
> > 
> > * Initially, let nmcli and ModemManager do their work, and let them
> > bring the WWAN interface up.
> > 
> > * Once this happens, we permanently-down the ModemManager from our
> > application-binary, thereby freeing up /dev/ttyUSB3.
> > 
> > * Thereafter, we are free to connect to /dev/ttyUSB3 from our
> > application, thereby using features like SMS-notification (+CMTI),
> > signal-strength (+CSQ), etc.
> > 
> > 
> > 
> > Does our approach make sense?
> > We will be grateful to any help.
> 
> Why not ask the modem manager team about this?  The kernel doesn't care
> what you do with the device links :)

Arguably, kernel is doing its job pretty badly when it comes to modems.

1) it does not hide differences between different modems (nokia modem is network
protocol, some modems are single tty, some modems are multiple ttys, different 
AT
command sets...)

2) it does not allow userland applications to share a GSM modem in a reasonable 
way.
Being able to read signal strength while PPP is connected would be nice, for 
example.

Anyway, when kernel fails to do its job, there's often userland daemon that can 
do it.

ofono seems to be the cannonical one there, and it seems to do the job rather 
well.

So... I guess the original author should take a look at ofono.


Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 V2] leds: trigger: Introduce an USB port trigger

[Pavel Machek]

On Mon 2016-07-18 08:55:52, Rafał Miłecki wrote:
> On 18 July 2016 at 07:53, Peter Chen  wrote:
> > On Mon, Jul 18, 2016 at 07:57:34AM +0200, Rafał Miłecki wrote:
> >> On 18 July 2016 at 07:40, Peter Chen  wrote:
> >> > On Mon, Jul 18, 2016 at 06:44:49AM +0200, Rafał Miłecki wrote:
> >> >> On 18 July 2016 at 04:31, Peter Chen  wrote:
> >> >> > On Fri, Jul 15, 2016 at 11:10:45PM +0200, Rafał Miłecki wrote:
> >> >> >> +
> >> >> >> +usbport trigger:
> >> >> >> +- usb-ports : List of USB ports that usbport should observed for 
> >> >> >> turning on a
> >> >> >> +   given LED.
> >> >> >> +
> >> >> >
> >> >> > %s/should/should be
> >> >>
> >> >> Thanks.
> >> >>
> >> >>
> >> >> >> diff --git a/drivers/leds/trigger/ledtrig-usbport.c 
> >> >> >> b/drivers/leds/trigger/ledtrig-usbport.c
> >> >> >> new file mode 100644
> >> >> >> index 000..97b064c
> >> >> >> --- /dev/null
> >> >> >> +++ b/drivers/leds/trigger/ledtrig-usbport.c
> >> >> >> @@ -0,0 +1,206 @@
> >> >> >> +/*
> >> >> >> + * USB port LED trigger
> >> >> >> + *
> >> >> >> + * Copyright (C) 2016 Rafał Miłecki 
> >> >> >> + *
> >> >> >> + * This program is free software; you can redistribute it and/or 
> >> >> >> modify
> >> >> >> + * it under the terms of the GNU General Public License as 
> >> >> >> published by
> >> >> >> + * the Free Software Foundation; either version 2 of the License, 
> >> >> >> or (at
> >> >> >> + * your option) any later version.
> >> >> >> + */
> >> >> >
> >
> > In your COPYRIGHT, it says "or any later version". But afaik, It should
> > not be on GPL v3.
> 
> If one picks my code, modifies it, relicenses it using GPL v3, we
> can't include it anymore. It's the same story as with BSD systems and
> their BSD licenses. If one picks e.g. BSD 3-clause licensed code,
> makes it GPL, releases it (e.g. by putting in Linux), BSD can't use it
> anymore.
> 
> Still, this license is GPL v2 compatible and as is it can be included
> in the Linux.

Anything GPLv2 compatible is fair game for the kernel. GPLv2+ is very
common for the kernel.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 V2] leds: trigger: Introduce an USB port trigger

[Pavel Machek]

Hi!

> > @@ -0,0 +1,206 @@
> > +/*
> > + * USB port LED trigger
> > + *
> > + * Copyright (C) 2016 Rafał Miłecki 
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or (at
> > + * your option) any later version.
> > + */
> 
> GPL v2 only.

No, you are not going to tell people which license they can use for
their kernel code.

GPL v2+ is perfectly fine for the kernel.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCHv2] usb: USB Type-C Connector Class

[Pavel Machek]

Hi!

> > With these boards, you will not see anything on the screen that is
> > attached to a Type-C connector until the OS has booted to the point
> > where it has negotiated the power contract and entered a mode.
> > 
> > If the system has BIOS/FW/EC capable of negotiating the power contract
> > and enter a mode, but where we still are expected to take over the
> > whole TCPM in OS, I think the connection will be reset.
> 
> Think about a DP over type C display with a USB PD power brick on a
> daisy chain.
> If the host needs more than 15W or more than 5V, a reset is suicide.
> 
> And losing earlyprintk hurts a lot.
> This means we need USB PD statically in the kernel. And a kernel
> based policy that brings up all displays.

Yes please.

Of course, even that will hurt, because I guess that means printk()
after USB, and that means late in the boot process, but better late in
kernel than in initrd.

(And yes, N900 has display pretty late in the bootprocess, and yes, it
is very annoying.)

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: v4.10-rc6 boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

Hi!

> > Hmm. I moved keyboard between USB ports, and now 4.10-rc6 no longer
> > boots. v4.6 works ok. Let me try with keyboard unplugged... no, I
> > could not get it to work. I believe v4.9 and some v4.10-rc's worked,
> > but I'll have to double check.
> 
> But all the kernel versions worked when the keyboard was plugged into
> its original USB port?

Aha. So it looks difference is probably in "where is keyboard plugged
in" but in "reboot" vs. "cold boot". I did not do a cold boot in quite
a while :-(.

Booting to grub, then hitting ctrl-alt-del is enough to make it work. Ouch.

It happens with current Linus' tree.

"usbcore.nousb usbcore.nousb=1" on kernel command line does not
help. So maybe it is not USB after all. (Sorry).

> Last message I can see on console is something like:
>
> pci :00:02.0: Video device with shadowed ROM at [mem 
> 0x000c-0x000d]

If I do the reboot dance with 4.10-rc6, messages around the hang are:

[0.281948] RPC: Registered named UNIX socket transport module.
[0.282009] RPC: Registered udp transport module.
[0.282068] RPC: Registered tcp transport module.
[0.282126] RPC: Registered tcp NFSv4.1 backchannel transport
module.
[0.282205] pci :00:02.0: Video device with shadowed ROM at
[mem 0x000c-0x000d]
[0.316266] PCI: CLS 64 bytes, default 64
[0.316330] PCI-DMA: Using software bounce buffering for IO
(SWIOTLB)
[0.316395] software IO TLB [mem 0xb987e000-0xbd87e000] (64MB)
mapped at [8800b987e000-8800bd87dfff]
[0.317912] futex hash table entries: 1024 (order: 5, 131072 bytes)
[0.318646] workingset: timestamp_bits=62 max_order=20
bucket_order=0

So I guess this is something with PCI subsystem?

Ideas welcome...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc6 boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

Hi!

> > > Hmm. I moved keyboard between USB ports, and now 4.10-rc6 no longer
> > > boots. v4.6 works ok. Let me try with keyboard unplugged... no, I
> > > could not get it to work. I believe v4.9 and some v4.10-rc's worked,
> > > but I'll have to double check.
> > 
> > But all the kernel versions worked when the keyboard was plugged into
> > its original USB port?
> 
> Aha. So it looks difference is probably in "where is keyboard plugged
> in" but in "reboot" vs. "cold boot". I did not do a cold boot in quite
> a while :-(.
> 
> Booting to grub, then hitting ctrl-alt-del is enough to make it work. Ouch.
> 
> It happens with current Linus' tree.

v4.10-rc6-feb3 : broken
v4.9 : ok
(v4.6 : ok)

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

v4.10-rc6 boot regression on Intel desktop, maybe related to EHCI hadnoff?

[Pavel Machek]

Hi!

Hmm. I moved keyboard between USB ports, and now 4.10-rc6 no longer
boots. v4.6 works ok. Let me try with keyboard unplugged... no, I
could not get it to work. I believe v4.9 and some v4.10-rc's worked,
but I'll have to double check.

Machine is small Intel desktop:

00:00.0 Host bridge: Intel Corporation 4 Series Chipset DRAM Controller (rev 03)
00:01.0 PCI bridge: Intel Corporation 4 Series Chipset PCI Express Root Port 
(rev 03)
00:02.0 VGA compatible controller: Intel Corporation 4 Series Chipset 
Integrated Graphics Controller (rev 03)
00:02.1 Display controller: Intel Corporation 4 Series Chipset Integrated 
Graphics Controller (rev 03)
00:1b.0 Audio device: Intel Corporation NM10/ICH7 Family High Definition Audio 
Controller (rev 01)
00:1c.0 PCI bridge: Intel Corporation NM10/ICH7 Family PCI Express Port 1 (rev 
01)
00:1c.1 PCI bridge: Intel Corporation NM10/ICH7 Family PCI Express Port 2 (rev 
01)
00:1d.0 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI Controller 
#1 (rev 01)
00:1d.1 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI Controller 
#2 (rev 01)
00:1d.2 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI Controller 
#3 (rev 01)
00:1d.3 USB controller: Intel Corporation NM10/ICH7 Family USB UHCI Controller 
#4 (rev 01)
00:1d.7 USB controller: Intel Corporation NM10/ICH7 Family USB2 EHCI Controller 
(rev 01)
00:1e.0 PCI bridge: Intel Corporation 82801 PCI Bridge (rev e1)
00:1f.0 ISA bridge: Intel Corporation 82801GB/GR (ICH7 Family) LPC Interface 
Bridge (rev 01)
00:1f.1 IDE interface: Intel Corporation 82801G (ICH7 Family) IDE Controller 
(rev 01)
00:1f.2 IDE interface: Intel Corporation NM10/ICH7 Family SATA Controller [IDE 
mode] (rev 01)
00:1f.3 SMBus: Intel Corporation NM10/ICH7 Family SMBus Controller (rev 01)
03:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 
PCI Express Gigabit Ethernet Controller (rev 03)

Last message I can see on console is something like:

pci :00:02.0: Video device with shadowed ROM at [mem 0x000c-0x000d]

...and then blinking cursor. According to dmesg, v4.6 kernel prints
this just after this message:

pci :00:1d.7: EHCI: BIOS handoff failed (BIOS bug?) 01010001

Any ideas? Let me try to update to current Linus' tree. I guess I
could try to boot with CONFIG_USB=n, but that will be pretty useless
configuration.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc6 boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

On Fri 2017-02-03 16:59:05, Alan Stern wrote:
> On Fri, 3 Feb 2017, Pavel Machek wrote:
> 
> > Hi!
> > 
> > > > > Hmm. I moved keyboard between USB ports, and now 4.10-rc6 no longer
> > > > > boots. v4.6 works ok. Let me try with keyboard unplugged... no, I
> > > > > could not get it to work. I believe v4.9 and some v4.10-rc's worked,
> > > > > but I'll have to double check.
> > > > 
> > > > But all the kernel versions worked when the keyboard was plugged into
> > > > its original USB port?
> > > 
> > > Aha. So it looks difference is probably in "where is keyboard plugged
> > > in" but in "reboot" vs. "cold boot". I did not do a cold boot in quite
> > > a while :-(.
> > > 
> > > Booting to grub, then hitting ctrl-alt-del is enough to make it work. 
> > > Ouch.
> > > 
> > > It happens with current Linus' tree.
> > 
> > v4.10-rc6-feb3 : broken
> > v4.9 : ok
> > (v4.6 : ok)
> 
> All I can suggest is git bisect.  :-(  But I agree that the problem is 
> unlikely to be in the USB layer.

Yep. I'm hoping PCI people speak up adding printks there should be
possibility, too.

(And I guess I should remove you and usb people from the cc-list... in
the next mails).

(I verified it happens with 32bit configuration, too, FWIW).

Thanks,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.9 to v4.10 regression: oops when USB cable is plugged in.

[Pavel Machek]

Hi!

> On Fri, Jan 27, 2017 at 10:55:12PM +0100, Pavel Machek wrote:
> > Ok, I can try. But so far even -rc1 is a lot of fun. But... I consider
> > phone calls core feature of a phone. I'd very much like to get that to
> > work. Unfortunately, that means real-time audio, and a lot of
> > fun. Plus, as it is touchscreen device, it needs a GUI. 
> 
> Talking about testing - kernelci's n900 is broken?
> 
> https://kernelci.org/boot/omap3-n900/

The N900 regressions are now more of "audio mixers no longer work"
than "it fails to boot" kind -- thus difficult to test automatically,
I'm afraid.
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.9 to v4.10 regression: oops when USB cable is plugged in.

[Pavel Machek]

On Fri 2017-02-03 08:30:48, Tony Lindgren wrote:
> * Pavel Machek <pa...@ucw.cz> [170203 00:00]:
> > Hi!
> > 
> > > On Fri, Jan 27, 2017 at 10:55:12PM +0100, Pavel Machek wrote:
> > > > Ok, I can try. But so far even -rc1 is a lot of fun. But... I consider
> > > > phone calls core feature of a phone. I'd very much like to get that to
> > > > work. Unfortunately, that means real-time audio, and a lot of
> > > > fun. Plus, as it is touchscreen device, it needs a GUI. 
> > > 
> > > Talking about testing - kernelci's n900 is broken?
> > > 
> > > https://kernelci.org/boot/omap3-n900/
> > 
> > The N900 regressions are now more of "audio mixers no longer work"
> > than "it fails to boot" kind -- thus difficult to test automatically,
> > I'm afraid.
> 
> Talking about audio testing, next now has commit 9834ffd1ecc3
> ("ASoC: omap-mcbsp: Add PM QoS support for McBSP to prevent glitches"),
> care to check if that also fixes the n900 audio glitch issues you
> reported earlier?

Yes, audio was choppy when I tried to use n900 as an mp3
player. Hmm. I re-checked v4.9, and it seems to work ok now. I'll
speak up if it re-appears.

Thanks,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH V2 1/2] dt-bindings: leds: document new led-triggers property

[Pavel Machek]

Hi!

> Is this possible to mix various entries in a list assigned to single
> property?
> Let's say:
> trigger-sources =
> <_port1>,
> <_port1>,
> < 1 GPIO_ACTIVE_HIGH>;

Actually... I'm not sure I like the "multiple sources". It is somehow
justified for ohci/ehci_port, because they... represent single
physical port. Could we introduce something for the physical port into
the DTS, too?

> >>>According to my knowledge all elements in the list are elements
> >>>of one array, no matter if they are comma separated or space separated
> >>>in "<>" brackets. DT maintainer would have to confirm that though.
> >>
> >>This matches my knowledge as well.
> >
> >Having that, we would be limited to a list of fixed size
> >tuples IMHO.
> 
> That sounds OK. Now I spent some time thinking about this I think it can work.
> First of all we may need something like #sources-cells to extend our property
> in the future.
> Secondly it should be possible to detect type of phandle node by trying things
> one by one. We should be e.g. able to check is phandle is for GPIO by trying
> of_find_gpiochip_by_xlate.

I am not sure if variable-length parameters here is good idea. Would
be nice to keep it simple... Having the led representing voltage on
gpio line is somehow strange to me. I'd rather have dts explaining
what that voltage means ("it is battery charging signal") and than
have led connected to that...
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

v4.9 to v4.10 regression: oops when USB cable is plugged in.

[Pavel Machek]

Hi!

v4.9 was ok (this is annoying enought that I'd notice).

v4.10-rc5 is not. (And yes, I probably 

[0.00] Booting Linux on physical CPU 0x0
[0.00] Linux version 4.10.0-rc5-142127-g41f2839-dirty (pavel@amd) (gcc 
version 4.7.2 (GCC) ) #222 Mon Jan 23 15:13:11 CET 2017
[0.00] CPU: ARMv7 Processor [411fc083] revision 3 (ARMv7), cr=10c5387d
[0.00] CPU: PIPT / VIPT nonaliasing data cache, VIPT nonaliasing 
instruction cache
[0.00] OF: fdt:Machine model: Nokia N900
[0.00] cma: Reserved 16 MiB at 0x8e80
[0.00] Memory policy: Data cache writeback
[0.00] On node 0 totalpages: 65280
[0.00] free_area_init_node: node 0, pgdat c0b4b848, node_mem_map 
cfcf1000
[0.00]   Normal zone: 512 pages used for memmap
[0.00]   Normal zone: 0 pages reserved
[0.00]   Normal zone: 65280 pages, LIFO batch:15
[0.00] CPU: All CPU(s) started in SVC mode.
[0.00] OMAP3430/3530 ES3.1 (l2cache iva sgx neon isp)
[0.00] pcpu-alloc: s0 r0 d32768 u32768 alloc=1*32768
[0.00] pcpu-alloc: [0] 0 
[0.00] Built 1 zonelists in Zone order, mobility grouping on.  Total 
pages: 64768
[0.00] Kernel command line: root=/dev/mmcblk0p1 rootwait no-omap-wd 
no-ext-wd
[0.00] PID hash table entries: 1024 (order: 0, 4096 bytes)
[0.00] Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
[0.00] Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
[0.00] Memory: 231196K/261120K available (6144K kernel code, 305K 
rwdata, 2048K rodata, 1024K init, 313K bss, 13540K reserved, 16384K 
cma-reserved, 0K highmem)
[0.00] Virtual kernel memory layout:
[0.00] vector  : 0x - 0x1000   (   4 kB)
[0.00] fixmap  : 0xffc0 - 0xfff0   (3072 kB)
[0.00] vmalloc : 0xd080 - 0xff80   ( 752 MB)
[0.00] lowmem  : 0xc000 - 0xd000   ( 256 MB)
[0.00] pkmap   : 0xbfe0 - 0xc000   (   2 MB)
[0.00] modules : 0xbf00 - 0xbfe0   (  14 MB)
[0.00]   .text : 0xc0008000 - 0xc070   (7136 kB)
[0.00]   .init : 0xc0a0 - 0xc0b0   (1024 kB)
[0.00]   .data : 0xc0b0 - 0xc0b4c480   ( 306 kB)
[0.00].bss : 0xc0b4e000 - 0xc0b9c5f4   ( 314 kB)
[0.00] NR_IRQS:16 nr_irqs:16 16
[0.00] IRQ: Found an INTC at 0xfa20 (revision 4.0) with 96 
interrupts
[0.00] Clocking rate (Crystal/Core/MPU): 19.2/332/500 MHz
[0.00] OMAP clockevent source: timer1 at 32768 Hz
[0.00] clocksource: 32k_counter: mask: 0x max_cycles: 
0x, max_idle_ns: 58327039986419 ns
[0.00] sched_clock: 32 bits at 32kHz, resolution 30517ns, wraps every 
6553584741ns
[0.30] OMAP clocksource: 32k_counter at 32768 Hz
[0.000885] Console: colour dummy device 80x30
[0.001708] console [tty0] enabled
[0.001770] Calibrating delay loop... 496.43 BogoMIPS (lpj=2482176)
[0.047760] pid_max: default: 32768 minimum: 301
[0.047943] Security Framework initialized
[0.048065] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes)
[0.048126] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes)
[0.049377] CPU: Testing write buffer coherency: ok
[0.050384] Setting up static identity map for 0x8010 - 0x80100070
[0.054046] devtmpfs: initialized
[0.092803] VFP support v0.3: implementor 41 architecture 3 part 30 variant 
c rev 1
[0.093353] clocksource: jiffies: mask: 0x max_cycles: 0x, 
max_idle_ns: 1911260446275 ns
[0.094879] pinctrl core: initialized pinctrl subsystem
[0.096954] NET: Registered protocol family 16
[0.101165] DMA: preallocated 256 KiB pool for atomic coherent allocations
[0.128479] omap_hwmod: mcbsp2_sidetone using broken dt data from mcbsp
[0.129394] omap_hwmod: mcbsp3_sidetone using broken dt data from mcbsp
[0.175903] cpuidle: using governor menu
[0.176574] Reprogramming SDRC clock to 33200 Hz
[0.182952] gpio gpiochip0: (gpio): added GPIO chardev (254:0)
[0.183471] gpiochip_setup_dev: registered GPIOs 0 to 31 on device: 
gpiochip0 (gpio)
[0.186126] OMAP GPIO hardware version 2.5
[0.187164] gpio gpiochip1: (gpio): added GPIO chardev (254:1)
[0.187927] gpiochip_setup_dev: registered GPIOs 32 to 63 on device: 
gpiochip1 (gpio)
[0.191314] gpio gpiochip2: (gpio): added GPIO chardev (254:2)
[0.191925] gpiochip_setup_dev: registered GPIOs 64 to 95 on device: 
gpiochip2 (gpio)
[0.195404] gpio gpiochip3: (gpio): added GPIO chardev (254:3)
[0.195922] gpiochip_setup_dev: registered GPIOs 96 to 127 on device: 
gpiochip3 (gpio)
[0.199218] gpio gpiochip4: (gpio): added GPIO chardev (254:4)
[0.199737] gpiochip_setup_dev: registered GPIOs 128 to 159 on device: 
gpiochip4 (gpio)
[0.203033] gpio gpiochip5: (gpio): added GPIO chardev (254:5)
[

Re: v4.9 to v4.10 regression: oops when USB cable is plugged in.

[Pavel Machek]

Hi!
On Mon 2017-01-23 14:44:54, Tony Lindgren wrote:
> * Pavel Machek <pa...@ucw.cz> [170123 14:26]:
> > [25392.239837] Unhandled fault: external abort on non-linefetch (0x1028) at 
> > 0xfa0ab060
> > [25392.239868] pgd = c0004000
> > [25392.239898] [fa0ab060] *pgd=48011452(bad)
> > [25392.239929] Internal error: : 1028 [#1] ARM
> > [25392.239929] Modules linked in:
> > [25392.239959] CPU: 0 PID: 24322 Comm: kworker/0:1 Not tainted 
> > 4.10.0-rc5-142127-g41f2839-dirty #222
> > [25392.239990] Hardware name: Nokia RX-51 board
> > [25392.240020] Workqueue: events musb_irq_work
> > [25392.240051] task: cd44d5c0 task.stack: cd308000
> > [25392.240051] PC is at musb_default_readb+0x0/0xc
> > [25392.240081] LR is at musb_irq_work+0x1c/0x1b0
> 
> OK I'm pretty sure the patch I posted few days ago fixes
> this. Can you please test patch "[PATCH] usb: musb: Fix
> external abort on non-linefetch for musb_irq_work()"?

Can I get the copy of the patch?

http://www.spinics.net/lists/linux-usb/msg152542.html

...but it is html mangled with no obvious way to unmangle it.

> I was able to hit that only once so far, do you hit it
> every time with your built-in g_ether .config?

I get it "way too often", like once a day. I don't yet know how to hit
it reliably :-(.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.9 to v4.10 regression: oops when USB cable is plugged in.

[Pavel Machek]

Hi!

> > > Can I get the copy of the patch?
> > > 
> > > http://www.spinics.net/lists/linux-usb/msg152542.html
> > > 
> > > ...but it is html mangled with no obvious way to unmangle it.
> 
> Bounced it to you. FYI, patchwork.kernel.org should have it too, the
> "mbox" option there works the best.

Thanks, applied. Problem did not reappear so far.

> > > > I was able to hit that only once so far, do you hit it
> > > > every time with your built-in g_ether .config?
> > > 
> > > I get it "way too often", like once a day. I don't yet know how to hit
> > > it reliably :-(.
> 
> OK, well let's hope the patch linked above fixes it. At some point the
> number of musb fixes should just start going down if I'm predicting right :)

Fingers crossed :-).

> Anyways, hitting these issues during late -rc cycle is too late. We
> really should have some n900 usability testing for core features with
> Linux next on at least weekly basis.

Ok, I can try. But so far even -rc1 is a lot of fun. But... I consider
phone calls core feature of a phone. I'd very much like to get that to
work. Unfortunately, that means real-time audio, and a lot of
fun. Plus, as it is touchscreen device, it needs a GUI. 

> I've noticed that testing with Linux next is way less effort than chasing
> bugs every -rc cycle when it's too late. For about past four months or so
> next has been usable for me with only occasional minor issues that get
> fixed within a day or two.

Well, I am carrying some patches... but maybe mainline is now good
enough to be useable without them?

No promises. I really should find more time for N900
development. Hmm. Or more developers. Where are all those Eudyptula
Challenge people when you need them?

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

On Thu 2017-02-16 12:21:13, Linus Torvalds wrote:
> On Thu, Feb 16, 2017 at 12:06 PM, Pavel Machek <pa...@ucw.cz> wrote:
> >
> > Hmm, that would explain problems at boot _and_ problems during
> > suspend/resume.
> 
> I've committed the revert, and I'm just assuming that the revert also
> fixed your suspend/resume issues, but I wanted to just double-check
> that since it's only implied, no staed explicitly..

So boot issue is fixed, but it hung on resume, again. v4.9 worked
ok. Display is restored when it hangs on resume, but mouse is dead; I
guess that means there should be some chance to get debugging messages
during the resume.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

On Thu 2017-02-23 17:28:26, Frederic Weisbecker wrote:
> On Tue, Feb 14, 2017 at 08:27:43PM +0100, Pavel Machek wrote:
> > On Tue 2017-02-14 18:59:56, Pavel Machek wrote:
> > > Hi!
> > > 
> > > > > > > Hmm. I moved keyboard between USB ports, and now 4.10-rc6 no 
> > > > > > > longer
> > > > > > > boots. v4.6 works ok. Let me try with keyboard unplugged... no, I
> > > > > > > could not get it to work. I believe v4.9 and some v4.10-rc's 
> > > > > > > worked,
> > > > > > > but I'll have to double check.
> > > > > > 
> > > > > > But all the kernel versions worked when the keyboard was plugged 
> > > > > > into
> > > > > > its original USB port?
> > > > > 
> > > > > Aha. So it looks difference is probably in "where is keyboard plugged
> > > > > in" but in "reboot" vs. "cold boot". I did not do a cold boot in quite
> > > > > a while :-(.
> > > > > 
> > > > > Booting to grub, then hitting ctrl-alt-del is enough to make it work. 
> > > > > Ouch.
> > > > > 
> > > > > It happens with current Linus' tree.
> > > > 
> > > > v4.10-rc6-feb3 : broken
> > > > v4.9 : ok
> > > > (v4.6 : ok)
> > > 
> > > Hmm. It hangs during PCI fixups, and it hangs in v4.10-rc8, too.   
> > > 
> > > With debug patch below, I get
> > > 
> > > ...1d.7: PCI fixup... pass 2
> > > ...1d.7: PCI fixup... pass 3
> > > ...1d.7: PCI fixup... pass 3 done
> > > 
> > > ...followed by hang. So yes, it looks USB related.
> > > 
> > > (Sometimes it hangs with some kind backtrace involving secondary CPU
> > > startup, unfortunately useful info is off screen at that point).
> > 
> > Forgot to say, 1d.7 is EHCI controller.
> > 
> > 00:1d.7 USB controller: Intel Corporation NM10/ICH7 Family USB2 EHCI
> > Controller (rev 01)
> 
> Ok, I should have access soon to a EeePc 1015CX (which seem to have this 
> controller).
> I hope I'll be able to reproduce the issue there. If not, I'm sorry but I'll 
> have to
> burden you again :-)

Go through more mails. It is only reproducible after cold boot. .. so
I doubt it will be easy to reproduce on another machine.

Now... I do have serial port, and I even might have serial cable
somewhere, but Giving how sensitive it is, it is probably going to
go away with console on ttyS...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

On Fri 2017-02-17 17:37:47, Thomas Gleixner wrote:
> On Fri, 17 Feb 2017, Frederic Weisbecker wrote:
> > On Thu, Feb 16, 2017 at 08:34:45PM +0100, Thomas Gleixner wrote:
> > > On Thu, 16 Feb 2017, Frederic Weisbecker wrote:
> > > > On Thu, Feb 16, 2017 at 10:20:14AM -0800, Linus Torvalds wrote:
> > > > > On Thu, Feb 16, 2017 at 10:13 AM, Frederic Weisbecker
> > > > >  wrote:
> > > > > >
> > > > > > I haven't followed the discussion but this patch has a known issue 
> > > > > > which is fixed
> > > > > > with:
> > > > > > 7bdb59f1ad474bd7161adc8f923cdef10f2638d1
> > > > > > "tick/nohz: Fix possible missing clock reprog after tick soft 
> > > > > > restart"
> > > > > >
> > > > > > I hope this fixes your issue.
> > > > > 
> > > > > No, Pavel saw the problem with rc8 too, which already has that fix.
> > > > > 
> > > > > So I think we'll just need to revert that original patch (and that
> > > > > means that we have to revert the commit you point to as well, since
> > > > > that ->next_tick field was added by the original commit).
> > > > 
> > > > Aw too bad, but indeed that late we don't have the choice.
> > > 
> > > Hint: Look for CPU hotplug interaction of these patches. I bet something
> > > becomes stale when the CPU goes down and does not get reset when it comes
> > > back online.
> > 
> > Indeed I should check that. But Pavel is seeing this on boot, where the
> 
> I don't think so. He observed it on suspend resume and by doing hotplug
> operations in a loop. But I might be wrong as usual.

These are different bugs.

On x60, I see failures doing hotplug/unplug in a loop, or lot of
suspends. Someone seen it in v4.8-stable etc. Old bug. Rare to hit.

Desktop machine was failing to boot, and had some fun with
suspend/resume too. Boot hang was reproducible with right
procedure. (Hard poweroff, cold boot.). That one was introduced in
4.10-rc cycle.


Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

On Tue 2017-02-14 18:59:56, Pavel Machek wrote:
> Hi!
> 
> > > > > Hmm. I moved keyboard between USB ports, and now 4.10-rc6 no longer
> > > > > boots. v4.6 works ok. Let me try with keyboard unplugged... no, I
> > > > > could not get it to work. I believe v4.9 and some v4.10-rc's worked,
> > > > > but I'll have to double check.
> > > > 
> > > > But all the kernel versions worked when the keyboard was plugged into
> > > > its original USB port?
> > > 
> > > Aha. So it looks difference is probably in "where is keyboard plugged
> > > in" but in "reboot" vs. "cold boot". I did not do a cold boot in quite
> > > a while :-(.
> > > 
> > > Booting to grub, then hitting ctrl-alt-del is enough to make it work. 
> > > Ouch.
> > > 
> > > It happens with current Linus' tree.
> > 
> > v4.10-rc6-feb3 : broken
> > v4.9 : ok
> > (v4.6 : ok)
> 
> Hmm. It hangs during PCI fixups, and it hangs in v4.10-rc8, too.   
> 
> With debug patch below, I get
> 
> ...1d.7: PCI fixup... pass 2
> ...1d.7: PCI fixup... pass 3
> ...1d.7: PCI fixup... pass 3 done
> 
> ...followed by hang. So yes, it looks USB related.
> 
> (Sometimes it hangs with some kind backtrace involving secondary CPU
> startup, unfortunately useful info is off screen at that point).

Forgot to say, 1d.7 is EHCI controller.

00:1d.7 USB controller: Intel Corporation NM10/ICH7 Family USB2 EHCI
Controller (rev 01)

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

Hi!

> > > > Hmm. I moved keyboard between USB ports, and now 4.10-rc6 no longer
> > > > boots. v4.6 works ok. Let me try with keyboard unplugged... no, I
> > > > could not get it to work. I believe v4.9 and some v4.10-rc's worked,
> > > > but I'll have to double check.
> > > 
> > > But all the kernel versions worked when the keyboard was plugged into
> > > its original USB port?
> > 
> > Aha. So it looks difference is probably in "where is keyboard plugged
> > in" but in "reboot" vs. "cold boot". I did not do a cold boot in quite
> > a while :-(.
> > 
> > Booting to grub, then hitting ctrl-alt-del is enough to make it work. Ouch.
> > 
> > It happens with current Linus' tree.
> 
> v4.10-rc6-feb3 : broken
> v4.9 : ok
> (v4.6 : ok)

Hmm. It hangs during PCI fixups, and it hangs in v4.10-rc8, too.   

With debug patch below, I get

...1d.7: PCI fixup... pass 2
...1d.7: PCI fixup... pass 3
...1d.7: PCI fixup... pass 3 done

...followed by hang. So yes, it looks USB related.

(Sometimes it hangs with some kind backtrace involving secondary CPU
startup, unfortunately useful info is off screen at that point).

Any ideas?
Pavel

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 1800bef..060ad79 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3510,6 +3510,8 @@ void pci_fixup_device(enum pci_fixup_pass pass, struct 
pci_dev *dev)
 {
struct pci_fixup *start, *end;
 
+   dev_info(>dev, "PCI fixup device %p, pass %d\n", dev, pass);
+
switch (pass) {
case pci_fixup_early:
start = __start_pci_fixups_early;
@@ -3558,6 +3560,7 @@ void pci_fixup_device(enum pci_fixup_pass pass, struct 
pci_dev *dev)
return;
}
pci_do_fixups(dev, start, end);
+   dev_info(>dev, "PCI fixup device %p, pass %d, done\n", dev, pass);
 }
 EXPORT_SYMBOL(pci_fixup_device);
 


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

Hi!

On Wed 2017-02-15 15:34:27, Linus Torvalds wrote:
> On Wed, Feb 15, 2017 at 3:20 PM, Pavel Machek <pa...@ucw.cz> wrote:
> > 4.10-rc4 broken
> > 4.10-rc3 ok
> 
> Hmm. If those actually end up being reliable, then there's not a whole
> lot in between them wrt PCI or USB.
> 
> What looked like the most likely candidate seems to be xhci-specific, though.
> 
> But maybe it's something that isn't directly in drivers/{pci,usb}/ and
> just interacts badly.

Ok. I _hope_ my tests are ok. Bisect log so far is:

pavel@half:/data/l/linux$ git bisect log
# bad: [49def1853334396f948dcb4cedb9347abb318df5] Linux 4.10-rc4
# good: [a121103c922847ba5010819a3f250f1f7fc84ab8] Linux 4.10-rc3
git bisect start 'v4.10-rc4' 'v4.10-rc3'
# good: [557ed56cc75e0a33c15ba438734a280bac23bd32] Merge tag
'sound-4.10-rc4' of
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound
git bisect good 557ed56cc75e0a33c15ba438734a280bac23bd32
# good: [f4d3935e4f4884ba80561db5549394afb8eef8f7] Merge branch
'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
git bisect good f4d3935e4f4884ba80561db5549394afb8eef8f7
# bad: [83346fbc07d267de777e2597552f785174ad0373] Merge branch
'x86-urgent-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect bad 83346fbc07d267de777e2597552f785174ad0373
# good: [18e7a45af91acdde99d3aa1372cc40e1f8142f7b] perf/x86: Reject
non sampling events with precise_ip
git bisect good 18e7a45af91acdde99d3aa1372cc40e1f8142f7b
# good: [84936118bdf37bda513d4a361c38181a216427e0] x86/unwind: Disable
KASAN checks for non-current tasks
git bisect good 84936118bdf37bda513d4a361c38181a216427e0
# good: [79078c53baabee12dfefb0cfe00ca94cb2c35570] Merge branch
'perf-urgent-for-linus' of
git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
git bisect good 79078c53baabee12dfefb0cfe00ca94cb2c35570
# good: [695085b4bc7603551db0b3da897b8bf9893ca218] x86/tsc: Add the
Intel Denverton Processor to native_calibrate_tsc()
git bisect good 695085b4bc7603551db0b3da897b8bf9893ca218

I should go now, but I should be able to finish it today.

Best regards,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

On Thu 2017-02-16 12:21:13, Linus Torvalds wrote:
> On Thu, Feb 16, 2017 at 12:06 PM, Pavel Machek <pa...@ucw.cz> wrote:
> >
> > Hmm, that would explain problems at boot _and_ problems during
> > suspend/resume.
> 
> I've committed the revert, and I'm just assuming that the revert also
> fixed your suspend/resume issues, but I wanted to just double-check
> that since it's only implied, no staed explicitly..

Thanks!

I don't yet know if suspend/resume issues are fixed. Those are somehow
tricky to reproduce -- fun stuff does not happen on every suspend. I
should know within a week or so...

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

On Thu 2017-02-16 20:34:45, Thomas Gleixner wrote:
> On Thu, 16 Feb 2017, Frederic Weisbecker wrote:
> > On Thu, Feb 16, 2017 at 10:20:14AM -0800, Linus Torvalds wrote:
> > > On Thu, Feb 16, 2017 at 10:13 AM, Frederic Weisbecker
> > >  wrote:
> > > >
> > > > I haven't followed the discussion but this patch has a known issue 
> > > > which is fixed
> > > > with:
> > > > 7bdb59f1ad474bd7161adc8f923cdef10f2638d1
> > > > "tick/nohz: Fix possible missing clock reprog after tick soft 
> > > > restart"
> > > >
> > > > I hope this fixes your issue.
> > > 
> > > No, Pavel saw the problem with rc8 too, which already has that fix.
> > > 
> > > So I think we'll just need to revert that original patch (and that
> > > means that we have to revert the commit you point to as well, since
> > > that ->next_tick field was added by the original commit).

(I already said that elsewhere, but yes, revert of 7bdb59f1ad474b and
24b91e360ef5 fixes boot problems for me. Hmm, and 24b9 was marked for
stable... I don't know how to contact all the stable maintainers, but
probably it should not go to stable just yet...) 

> > Aw too bad, but indeed that late we don't have the choice.
> 
> Hint: Look for CPU hotplug interaction of these patches. I bet something
> becomes stale when the CPU goes down and does not get reset when it comes
> back online.

Hmm, that would explain problems at boot _and_ problems during
suspend/resume.

Note that this can be used to test the hotplug...

 cd /sys/devices/system/cpu/cpu1
 while true; do echo 0 > online; echo 1 > online; done

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

Hi!

> > > 4.10-rc4 broken
> > > 4.10-rc3 ok
> > 
> > Hmm. If those actually end up being reliable, then there's not a whole
> > lot in between them wrt PCI or USB.
> > 
> > What looked like the most likely candidate seems to be xhci-specific, 
> > though.
> > 
> > But maybe it's something that isn't directly in drivers/{pci,usb}/ and
> > just interacts badly.
> 
> Ok. I _hope_ my tests are ok. Bisect log so far is:

And the winner is:

pavel@half:/data/l/linux$ git bisect bad
24b91e360ef521a2808771633d76ebc68bd5604b is the first bad commit
commit 24b91e360ef521a2808771633d76ebc68bd5604b
Author: Frederic Weisbecker 
Date:   Wed Jan 4 15:12:04 2017 +0100

nohz: Fix collision between tick and other hrtimers


Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

On Thu 2017-02-16 18:25:35, Pavel Machek wrote:
> Hi!
> 
> > > > 4.10-rc4 broken
> > > > 4.10-rc3 ok
> > > 
> > > Hmm. If those actually end up being reliable, then there's not a whole
> > > lot in between them wrt PCI or USB.
> > > 
> > > What looked like the most likely candidate seems to be xhci-specific, 
> > > though.
> > > 
> > > But maybe it's something that isn't directly in drivers/{pci,usb}/ and
> > > just interacts badly.
> > 
> > Ok. I _hope_ my tests are ok. Bisect log so far is:
> 
> And the winner is:
> 
> pavel@half:/data/l/linux$ git bisect bad
> 24b91e360ef521a2808771633d76ebc68bd5604b is the first bad commit
> commit 24b91e360ef521a2808771633d76ebc68bd5604b
> Author: Frederic Weisbecker <fweis...@gmail.com>
> Date:   Wed Jan 4 15:12:04 2017 +0100
> 
> nohz: Fix collision between tick and other hrtimers
> 

I had to revert 7bdb59f1ad474bd7161adc8f923cdef10f2638d1, too,
otherwise -rc8 does not compile.

With 24b91e360ef521a28087716 and 7bdb59f1ad474 reverted, it seems to
boot ok. (I did few tries.)

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

On Tue 2017-02-14 11:12:26, Linus Torvalds wrote:
> On Feb 14, 2017 9:59 AM, "Pavel Machek" <pa...@ucw.cz> wrote:
> 
> Hi!
> 
> > >
> > > Booting to grub, then hitting ctrl-alt-del is enough to make it work.
> Ouch.
> > >
> > > It happens with current Linus' tree.
> >
> > v4.10-rc6-feb3 : broken
> > v4.9 : ok
> 
> I wonder if you could bisect it now that you've figured out the rules for
> when it breaks...

I guess that's what I'll need to do. It is my main machine, so it is a
bit painful.

Anyway, it seems that "nosmp" makes it hang at similar place, but
makes it hang reliably, reboot or cold poweroff. So I guess that's
what I'll use for bisection -- should be possible to do automatically
that way.

> I don't think I've seen any similar reports, so we don't have a lot of
> clues to go by otherwise, I think.

:-(.
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: v4.10-rc8 (-rc6) boot regression on Intel desktop, does not boot after cold boots, boots after reboot

[Pavel Machek]

On Wed 2017-02-15 18:23:03, Pavel Machek wrote:
> On Tue 2017-02-14 11:12:26, Linus Torvalds wrote:
> > On Feb 14, 2017 9:59 AM, "Pavel Machek" <pa...@ucw.cz> wrote:
> > 
> > Hi!
> > 
> > > >
> > > > Booting to grub, then hitting ctrl-alt-del is enough to make it work.
> > Ouch.
> > > >
> > > > It happens with current Linus' tree.
> > >
> > > v4.10-rc6-feb3 : broken
> > > v4.9 : ok
> > 
> > I wonder if you could bisect it now that you've figured out the rules for
> > when it breaks...
> 
> I guess that's what I'll need to do. It is my main machine, so it is a
> bit painful.
> 
> Anyway, it seems that "nosmp" makes it hang at similar place, but
> makes it hang reliably, reboot or cold poweroff. So I guess that's
> what I'll use for bisection -- should be possible to do automatically
> that way.

I was mistaken. "nosmp" does not seem to make the hang reliable.

my-4.10-r8+ broken
4.10-rc8 broken
4.10-rc4 broken
4.10-rc3 ok
4.10-rc2 ok?

I started bisect, 168 revisions to go.

Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH V5] leds: trigger: Introduce a USB port trigger

[Pavel Machek]

On Fri 2016-09-09 13:31:10, Rafał Miłecki wrote:
> On 9 September 2016 at 13:05, Greg KH  wrote:
> > On Fri, Sep 09, 2016 at 05:34:40PM +0800, Peter Chen wrote:
> >> On Thu, Sep 08, 2016 at 06:08:24PM +0200, Rafał Miłecki wrote:
> >> > From: Rafał Miłecki 
> >> >
> >> > This commit adds a new trigger responsible for turning on LED when USB
> >> > device gets connected to the selected USB port. This can can useful for
> >> > various home routers that have USB port(s) and a proper LED telling user
> >> > a device is connected.
> >> >
> >> > The trigger gets its documentation file but basically it just requires
> >> > enabling it and selecting USB ports (e.g. echo 1 > ports/usb1-1).
> >> >
> >> > There was a long discussion on design of this driver. Its current state
> >> > is a result of picking them most adjustable solution as others couldn't
> >> > handle all cases.
> >> >
> >> > 1) It wasn't possible for the driver to register separated trigger for
> >> >each USB port. Some physical USB ports are handled by more than one
> >> >controller and so by more than one USB port. E.g. USB 2.0 physical
> >> >port may be handled by OHCI's port and EHCI's port.
> >> >It's also not possible to assign more than 1 trigger to a single LED
> >> >and implementing such feature would be tricky due to syncing triggers
> >> >and sysfs conflicts with old triggers.
> >> >
> >> > 2) Another idea was to register trigger per USB hub. This wouldn't allow
> >> >handling devices with multiple USB LEDs and controllers (hubs)
> >> >controlling more than 1 physical port. It's common for hubs to have
> >> >few ports and each may have its own LED.
> >> >
> >> > This final trigger is highly flexible. It allows selecting any USB ports
> >> > for any LED. It was also modified (compared to the initial version) to
> >> > allow choosing ports rather than having user /guess/ proper names. It
> >> > was successfully tested on SmartRG SR400ac which has 3 USB LEDs,
> >> > 2 physical ports and 3 controllers.
> >> >
> >> > Another planned feature is support for LED reacting to the USB activity.
> >> > This can be implemented with another sysfs file for setting mode. The
> >> > default mode wouldn't change so there won't be ABI breakage and such
> >> > feature can be safely implemented later.
> >> >
> >>
> >> It has such driver at: drivers/usb/common/led.c
> >
> > Ugh, I thought I had seen something like this before...
> >
> > Rafał, can you just use this in-kernel code instead?
> 
> I really don't think I can because of all the reasons I carefully
> listed in the commit message.
> 
> Have you took a look at that simple driver? It does nothing I need.
> Its design doesn't allow implementing features I clearly listed in the
> commit message.

In any case, the new driver should probably go near the old one, at
the very least.
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 v16 0/4] Introduce usb charger framework to deal with the usb gadget power negotation

[Pavel Machek]

Hi!

> > That's not actually 100% clear to me - for what the wm831x is doing it
> > probably *does* want the higher limit.  This is a system inflow limit
> > (as it should be for this), at least the charger will adapt to voltage
> > variations though other users in the system are much less likely to do
> > so.
> 
> Interesting ... I hadn't considered that possibility.
> 
> As long as the current remains below the maximum, the charger will
> reduce the voltage towards 2V as load increases.  Somewhere before it
> gets there, the system will not be able to make use of the power as the
> voltage will be too low to be usable. So that will naturally limit the
> current being drawn.
> 
> Not having very much electrical engineering background, I cannot say for
> sure what will happen, but it seems likely that once the voltage drops
> much below 4.75V, the charger won't be operating at peak efficiency,
> which would be a waste.
> I can easily imagine that the hardware would switch off at some voltage
> level, rather than just making do with what is there.
> So I'm skeptical of this approach, but I'm open to being corrected by
> someone more knowledgeable than I.

Devices I seen charge down to ~4.2V. This is useful thing to play
with:

dx.com: 406496
1" USB Current & Voltage Detector Tester Meter w/ Red LED Display -
Blue

Best regards,
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 V5] leds: trigger: Introduce a USB port trigger

[Pavel Machek]

On Thu 2016-09-15 15:33:19, Rafał Miłecki wrote:
> On 15 September 2016 at 14:56, Pavel Machek <pa...@ucw.cz> wrote:
> > On Fri 2016-09-09 13:31:10, Rafał Miłecki wrote:
> >> On 9 September 2016 at 13:05, Greg KH <gre...@linuxfoundation.org> wrote:
> >> > On Fri, Sep 09, 2016 at 05:34:40PM +0800, Peter Chen wrote:
> >> >> On Thu, Sep 08, 2016 at 06:08:24PM +0200, Rafał Miłecki wrote:
> >> >> > From: Rafał Miłecki <ra...@milecki.pl>
> >> >> >
> >> >> > This commit adds a new trigger responsible for turning on LED when USB
> >> >> > device gets connected to the selected USB port. This can can useful 
> >> >> > for
> >> >> > various home routers that have USB port(s) and a proper LED telling 
> >> >> > user
> >> >> > a device is connected.
> >> >> >
> >> >> > The trigger gets its documentation file but basically it just requires
> >> >> > enabling it and selecting USB ports (e.g. echo 1 > ports/usb1-1).
> >> >> >
> >> >> > There was a long discussion on design of this driver. Its current 
> >> >> > state
> >> >> > is a result of picking them most adjustable solution as others 
> >> >> > couldn't
> >> >> > handle all cases.
> >> >> >
> >> >> > 1) It wasn't possible for the driver to register separated trigger for
> >> >> >each USB port. Some physical USB ports are handled by more than one
> >> >> >controller and so by more than one USB port. E.g. USB 2.0 physical
> >> >> >port may be handled by OHCI's port and EHCI's port.
> >> >> >It's also not possible to assign more than 1 trigger to a single 
> >> >> > LED
> >> >> >and implementing such feature would be tricky due to syncing 
> >> >> > triggers
> >> >> >and sysfs conflicts with old triggers.
> >> >> >
> >> >> > 2) Another idea was to register trigger per USB hub. This wouldn't 
> >> >> > allow
> >> >> >handling devices with multiple USB LEDs and controllers (hubs)
> >> >> >controlling more than 1 physical port. It's common for hubs to have
> >> >> >few ports and each may have its own LED.
> >> >> >
> >> >> > This final trigger is highly flexible. It allows selecting any USB 
> >> >> > ports
> >> >> > for any LED. It was also modified (compared to the initial version) to
> >> >> > allow choosing ports rather than having user /guess/ proper names. It
> >> >> > was successfully tested on SmartRG SR400ac which has 3 USB LEDs,
> >> >> > 2 physical ports and 3 controllers.
> >> >> >
> >> >> > Another planned feature is support for LED reacting to the USB 
> >> >> > activity.
> >> >> > This can be implemented with another sysfs file for setting mode. The
> >> >> > default mode wouldn't change so there won't be ABI breakage and such
> >> >> > feature can be safely implemented later.
> >> >> >
> >> >>
> >> >> It has such driver at: drivers/usb/common/led.c
> >> >
> >> > Ugh, I thought I had seen something like this before...
> >> >
> >> > Rafał, can you just use this in-kernel code instead?
> >>
> >> I really don't think I can because of all the reasons I carefully
> >> listed in the commit message.
> >>
> >> Have you took a look at that simple driver? It does nothing I need.
> >> Its design doesn't allow implementing features I clearly listed in the
> >> commit message.
> >
> > In any case, the new driver should probably go near the old one, at
> > the very least.
> 
> I can do that. Anyone objects?

I did not have time to study your patches in detail. IIRC there was
something like "directory full of usb IDs". I'm not sure that's a way
to go.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 RFC V3.5] leds: trigger: Introduce an USB port trigger

[Pavel Machek]

On Thu 2016-08-25 11:04:38, Jacek Anaszewski wrote:
> On 08/25/2016 10:29 AM, Rafał Miłecki wrote:
> >On 25 August 2016 at 10:03, Jacek Anaszewski  
> >wrote:
> >>On 08/24/2016 07:52 PM, Rafał Miłecki wrote:
> >>>
> >>>From: Rafał Miłecki 
> >>>
> >>>This commit adds a new trigger responsible for turning on LED when USB
> >>>device gets connected to the specified USB port. This can can useful for
> >>>various home routers that have USB port(s) and a proper LED telling user
> >>>a device is connected.
> >>>
> >>>The trigger gets its documentation file but basically it just requires
> >>>specifying USB port in a Linux format (e.g. echo 1-1 > new_port).
> >>>
> >>>During work on this trigger there was a plan to add DT bindings for it,
> >>>but there wasn't an agreement on the format yet. This can be worked on
> >>>later, a sysfs interface is needed anyway for platforms not using DT.
> >>>
> >>>Signed-off-by: Rafał Miłecki 
> >>>---
> >>>V2: Trying to add DT support, idea postponed as it will take more time
> >>>to discuss the bindings.
> >>>V3: Fix typos in commit and Documentation (thanks Jacek!)
> >>>Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
> >>>Check if there is USB device connected after adding new USB port
> >>>Fix memory leak or two
> >>>V3.5: Fix e-mail address (thanks Matthias)
> >>>  Simplify conditions in usbport_trig_notify (thx Matthias)
> >>>  Make "ports" a subdirectory with file per port, to match one value
> >>>  per file sysfs rule (thanks Greg)
> >>>  As "ports" couldn't be used for adding and removing ports anymore,
> >>>  there are now "new_port" and "remove_port". Having them makes this
> >>>  API also common with e.g. pci and usb buses.
> >>
> >>
> >>Now writing new_port with "1-1" produces a file named "1-1" in the
> >>ports directory with 000 permissions. I think that what Greg had
> >>on mind by referring to "one value per file" rule was a set of
> >>files representing ports, like "1-1 1-2 2-1", and each file should be
> >>readable/writeable.
> >>
> >>For instance "echo 1 > 1-1" would enable the trigger for the port 1-1
> >>and "echo 0 > 1-1" would disable it. The problem is that we don't know
> >>the number of required ports at compilation time and the sysfs
> >>attributes would have to be dynamically created on driver instantiation.
> >>What is more, as the USB ports can dynamically appear/disappear in the
> >>system, the files would have to be created/removed accordingly during
> >>LED class device lifetime, which is not the best design for the sysfs
> >>interface I think.

Could we add a flag to the USB port, instead? That way, USB code would
take care of creating the enable file in its own directory.

Is per-port control needed? Would just global control be sufficient?

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 RFC V3.5] leds: trigger: Introduce an USB port trigger

[Pavel Machek]

On Thu 2016-08-25 20:48:04, Jacek Anaszewski wrote:
> On 08/25/2016 04:30 PM, Alan Stern wrote:
> >On Thu, 25 Aug 2016, Jacek Anaszewski wrote:
> >
> >>I'd see it as follows:
> >>
> >>#cat available_ports
> >>#1-1 1-2 2-1
> >>
> >>#echo "1-1" > new_port
> >>
> >>#cat observed_ports
> >>#1-1
> >>
> >>#echo "2-1" > new_port
> >>
> >>#cat observed_ports
> >>#1-1 2-1
> >>
> >>We've already had few discussions about the sysfs designs trying
> >>to break the one-value-per-file rule for LED class device, and
> >>there was always strong resistance against.
> >
> >This scheme has multiple values in both the available_ports and
> >observed_ports files.  :-(  Not that I have any better suggestions...
> 
> Right, I forgot to add a note here, that this follows space
> separated list pattern similarly as in case of triggers attribute.
> Of course other suggestions are welcome.
> 
> Also a description of the device connected to the port would be a nice
> feature, however I am not certain about the feasibility thereof.
> >>>
> >>>What kind of description do you mean? Where should it be used / where
> >>>should it appear?
> >>>
> >>
> >>Product name/symbol. Actually it should be USB subsystem responsibility
> >>to provide the means for querying the product name by port id, if it
> >>is possible at all.
> >
> > cat /sys/bus/usb/devices/PORT/product
> > cat /sys/bus/usb/devices/PORT/manufacturer
> >
> >These will work if there is a device registered under PORT.
> 
> I've found only idProduct and idVendor files. They indeed uniquely
> identify the device, but the numbers are not human readable.

Actually, they don't. They identify device _type_. If you have two
mice of the same type connected, they'll have same idProduct /
idVendor values.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-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 RFC V3.5] leds: trigger: Introduce an USB port trigger

[Pavel Machek]

Hi!

> >2) Having "ports" subdir with RW files, one per each existing physical port
> >In this situation we don't need "new_port" or "remove_port". If we
> >want port to be observable we just do:
> >echo 1 > 1-1
> >Implementing this solution needs reading more details from USB subsystem.
> 
> The situation here is clear IMO - the number of USB ports in the system
> can change dynamically. I'm not sure if this can be handled easily with
> sysfs, where we usually expose an interface for known set of settings.
> struct attribute arrays are usually defined statically at the compile
> time and filled with the variables, that are created with DEVICE_ATTR
> macro.

sysfs already exposes current view of all usb devices. Just use it.

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html