Re: [PATCH] clk: ti: fix dual-registration of uart4_ick

[Greg KH]

On Wed, Sep 23, 2015 at 03:48:34PM +0100, Ben Dooks wrote:
> On the OMAP AM3517 platform the uart4_ick gets registered
> twice, causing any power managment to /dev/ttyO3 to fail
> when trying to wake the device up.
> 
> This solves the following oops:
> 
> [] Unhandled fault: external abort on non-linefetch (0x1028) at 0xfa09e008
> [] PC is at serial_omap_pm+0x48/0x15c
> [] LR is at _raw_spin_unlock_irqrestore+0x30/0x5c
> 
> Signed-off-by: Ben Dooks 
> ---
>  drivers/clk/ti/clk-3xxx.c | 1 -
>  1 file changed, 1 deletion(-)



This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.


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

Re: [PATCH 1/3] tty: serial: 8250_omap: do not use RX DMA if pause is not supported

[Greg KH]

On Sat, Aug 08, 2015 at 11:32:05AM +0200, Sebastian Andrzej Siewior wrote:
 On 08/08/2015 02:28 AM, Peter Hurley wrote:
  diff --git a/drivers/tty/serial/8250/8250_omap.c 
  b/drivers/tty/serial/8250/8250_omap.c
  index 0340ee6ba970..07a11e0935e4 100644
  --- a/drivers/tty/serial/8250/8250_omap.c
  +++ b/drivers/tty/serial/8250/8250_omap.c
  @@ -769,7 +771,9 @@ static void omap_8250_rx_dma_flush(struct 
  uart_8250_port *p)
 return;
 }
   
  -  dmaengine_pause(dma-rxchan);
  +  ret = dmaengine_pause(dma-rxchan);
  +  if (WARN_ON_ONCE(ret))
  +  priv-rx_dma_broken = true;
  
  No offense, Sebastian, but it boggles my mind that anyone could defend this
  as solid api design. We're in the middle of an interrupt handler and the
  slave dma driver is /just/ telling us now that it doesn't implement this
  functionality?!!?
 
 This is the best thing I could come up with. But to be fair: This
 happens once _very_ early in the process and is 100% reproducible. The
 WARN_ON should trigger user attention.

Never expect a _user_ to do anything with a WARN_ON except ignore it.

WARN_ON is for developers when something really bad went wrong that you
want to be notified so that you can fix the code to prevent that from
ever happening again.

So this isn't ok, sorry, a user would not know what to do with this at
all except email me asking why the driver is broken because a kernel
oops just happened, when in reality, their hardware is broken in a way
that you already know about when you wrote the patch.

You know better than this.

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

Re: [PATCH] dma: omap-dma: add support for pause of non-cyclic transfers

[Greg KH]

On Fri, Aug 07, 2015 at 10:41:57AM +0200, Sebastian Andrzej Siewior wrote:
 This DMA driver is used by 8250-omap on DRA7-evm. There is one
 requirement that is to pause a transfer. This is currently used on the RX
 side. It is possible that the UART HW aborted the RX (UART's RX-timeout)
 but the DMA controller starts the transfer shortly after.
 Before we can manually purge the FIFO we need to pause the transfer,
 check how many bytes it already received and terminate the transfer
 without it making any progress.
 
 From testing on the TX side it seems that it is possible that we invoke
 pause once the transfer has completed which is indicated by the missing
 CCR_ENABLE bit but before the interrupt has been noticed. In that case the
 interrupt will come even after disabling it.
 
 The AM572x manual says that we have to wait for the CCR_RD_ACTIVE 
 CCR_WR_ACTIVE bits to be gone before programming it again here is the
 drain loop. Also it looks like without the drain the TX-transfer makes
 sometimes progress.
 
 One note: The pause + resume combo is broken because after resume the
 the complete transfer will be programmed again. That means the already
 transferred bytes (until the pause event) will be sent again. This is
 currently not important for my UART user because it does only pause +
 terminate.
 
 Cc: sta...@vger.kernel.org

You don't get a add support patch into the stable tree unless it's a
trivial device id or quirk table addition, please go re-read
Documentation/stable_kernel_rules.txt

thanks,

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

Re: [PATCH 1/1] usb: musb: dsps: fix build on i386 when COMPILE_TEST is set

[Greg KH]

On Fri, Apr 03, 2015 at 10:14:14AM -0500, Felipe Balbi wrote:
 Hi Greg,
 
 On Wed, Mar 25, 2015 at 02:18:37PM -0700, Tony Lindgren wrote:
  Commit 3e457371f436 (usb: musb: Fix fifo reads for dm816x with musb_dsps)
  fixed a USB error on dm816x, but introduced a new build error on i386
  when COMPILE_TEST is set:
  
  drivers/usb/musb/musb_dsps.c: In function ‘dsps_read_fifo32’:
  drivers/usb/musb/musb_dsps.c:624:3: error: implicit declaration of function
  ‘readsl’ [-Werror=implicit-function-declaration]
  readsl(fifo, dst, len  2);
  
  Let's fix this by using ioread32_rep() instead of readsl() as that's
  more portable.
  
  Fixes: 3e457371f436 (usb: musb: Fix fifo reads for dm816x with musb_dsps)
  Reported-by: Fengguang Wu fengguang...@intel.com
  Cc: Bin Liu binml...@gmail.com
  Cc: Brian Hutchinson b.hutch...@gmail.com
  Cc: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
  Signed-off-by: Tony Lindgren t...@atomide.com
 
 Can you apply this one directly ? It fixes a build error on i386.
 
 Signed-off-by: Felipe Balbi ba...@ti.com

Now queued up, thanks.

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

Re: [PATCH] tty/serial: omap: fix !wakeirq message

[Greg KH]

On Mon, Mar 23, 2015 at 05:38:44PM -0500, Doug Kehn wrote:
 When wakeirq is not used/enabled, no wakeirq for uart0 is output for
 all TTY as the log message is generated before the port line is
 initialized.
 
 [0.802656] Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled
 [0.811700] omap_uart 44e09000.serial: no wakeirq for uart0
 [0.812379] 44e09000.serial: ttyO0 at MMIO 0x44e09000 (irq = 88, base_baud 
 = 300) is a OMAP UART0
 [1.503622] console [ttyO0] enabled
 [1.509836] omap_uart 48022000.serial: no wakeirq for uart0
 [1.516118] 48022000.serial: ttyO1 at MMIO 0x48022000 (irq = 89, base_baud 
 = 300) is a OMAP UART1
 [1.527711] omap_uart 48024000.serial: no wakeirq for uart0
 [1.533881] 48024000.serial: ttyO2 at MMIO 0x48024000 (irq = 90, base_baud 
 = 300) is a OMAP UART2
 [1.545324] omap_uart 481a6000.serial: no wakeirq for uart0
 [1.551410] 481a6000.serial: ttyO3 at MMIO 0x481a6000 (irq = 60, base_baud 
 = 300) is a OMAP UART3
 [1.562946] omap_uart 481a8000.serial: no wakeirq for uart0
 [1.569036] 481a8000.serial: ttyO4 at MMIO 0x481a8000 (irq = 61, base_baud 
 = 300) is a OMAP UART4
 
 Fix by moving wakeirq check and dev_info() call to after port line
 initialization and validation.
 
 Patch is against 3.14.36.

That's really old, please make this against a modern kernel, like the
4.0-rc series.

thanks,

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

Re: [PATCH 2/4] usb: dwc3: gadget: Stop TRB preparation after limit is reached

[Greg KH]

On Wed, Jan 14, 2015 at 02:39:55PM +0530, Amit Virdi wrote:
 Alright, I just applied your patches to testing/fixes. I'll start
 testing today and should be able to send a pull request to Greg by the
 end of the week, hopefully.
 
 
 Thanks! Just a small clarification - git failed to send patches to stable
 kernel list again (unfortunately I used the older configuration; so older
 git version - my bad).
 
 Does these patches land up in the stable trees through maintainers or I
 should explicitly cc sta...@vger.kernel.org now?

Please read Documentation/stable_kernel_rules.txt for how to get patches
accepted into the stable trees.

thanks,

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

Re: [PATCH 00/13 v10] omap 8250 based UART + DMA

[Greg KH]

On Mon, Sep 29, 2014 at 08:06:36PM +0200, Sebastian Andrzej Siewior wrote:
 The queue is getting smaller. The highlights of v9…v10
 - the DMA stall Frans Klaver reported which popped up in yocto is gone. It
   also seems that the ack the err-irq even if nothing happened in EDMA
   can be dropped.
 - the RX- and TX-DMA callbacks are now OMAP-only and no bugs flags are
   introduced into the generic DMA code. This also means that there is
   custom IRQ routine in case of DMA.

I've now applied the patches here that I could, if I have missed any,
please let me know and resend.

thanks,

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

Re: RCU bug with v3.17-rc3 ?

[Greg KH]

On Mon, Oct 13, 2014 at 12:43:07PM +0100, Russell King - ARM Linux wrote:
 On Mon, Oct 13, 2014 at 09:11:34AM +, David Laight wrote:
  From: Nathan Lynch
   On 10/10/2014 11:25 AM, Russell King - ARM Linux wrote:
   
Right, so GCC 4.8.{1,2} are totally unsuitable for kernel building (and
it seems that this has been known about for some time.)
   
   Looking at http://gcc.gnu.org/PR58854 it seems that all 4.8.x for x  3
   are affected, as well as 4.9.0.
   
We can blacklist these GCC versions quite easily.  We already have GCC
3.3 blacklisted, and it's trivial to add others.  I would want to 
include
some proper details about the bug, just like the other existing entries
we already have in asm-offsets.c, where we name the functions that the
compiler is known to break where appropriate.
   
   Before blacklisting anything, it's worth considering that simple version
   checks would break existing pre-4.8.3 compilers that have been patched
   for PR58854.  It looks like Yocto and Buildroot issued releases with
   patched 4.8.2 compilers well before the (fixed) 4.8.3 release.  I think
   the most we can reasonably do without breaking some correctly-behaving
   toolchains is to emit a warning.
  
  Is it possible to compile a small code fragment and check the generated
  code for the bug?
  Possibly predicated on the broken version number to avoid false positives.
 
 I don't see how - it looks like it requires an interrupt to occur at an
 opportune moment to provoke the function to fail.  The alternative would
 be to parse the assembly generated by the compiler to determine how it
 is dealing with the stack.
 
 I think the only viable solution here is that:
 
 1. We blacklist the bad compiler versions outright in the kernel.

Yes, please do this, it's what we have done for other buggy compiler
versions, no need to do something different here.

 Remember, it's the distro's choice to fix these buggy compilers, so the
 onus is on _them_ to deal with the mess they've created by doing so.

I totally agree.

Is someone going to send this patch, or do I have to write it myself?

thanks,

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

Re: [PATCH 00/16 v9] omap 8250 based uart + DMA

[Greg KH]

On Wed, Sep 17, 2014 at 11:05:06AM +0200, Sebastian Andrzej Siewior wrote:
 On 2014-09-10 21:29:55 [+0200], Sebastian Andrzej Siewior wrote:
  the diff of v8…v9 is small:
 
 Greg, do you mind taking patches from this series up to [PATCH 05/16]?
 Nobody complained about those so far and it would keep v10 a little smaller.
 I have changes to #6 (the omap driver) and need to do some DMA related
 changes in the following

Now applied, thanks.

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

Re: [PATCH v5 1/2] usb: rename phy to usb_phy in HCD

[Greg KH]

On Fri, Sep 05, 2014 at 01:42:09AM +0400, Sergei Shtylyov wrote:
 From: Antoine Tenart antoine.ten...@free-electrons.com
 
 The USB PHY member of the HCD structure is renamed to 'usb_phy' and
 modifications are done in all drivers accessing it.
 This is in preparation to adding the generic PHY support.
 
 Signed-off-by: Antoine Tenart antoine.ten...@free-electrons.com
 [Sergei: added missing 'drivers/usb/misc/lvstest.c' file, resolved rejects
 caused by patch reordering, updated changelog.]
 Signed-off-by: Sergei Shtylyov sergei.shtyl...@cogentembedded.com
 Acked-by: Alan Stern st...@rowland.harvard.edu
 Acked-by: Felipe Balbi ba...@ti.com
 
 ---
 Changes in version 5:
 - imported the patch from Antoine Tenart's series;
 - added missing 'drivers/usb/misc/lvstest.c' file;
 - resolved rejects caused by patch reordering;
 - refreshed patch;
 - updated changelog.
 
  drivers/usb/chipidea/host.c   |2 +-
  drivers/usb/core/hcd.c|   20 ++--
  drivers/usb/core/hub.c|8 
  drivers/usb/host/ehci-fsl.c   |   16 
  drivers/usb/host/ehci-hub.c   |2 +-
  drivers/usb/host/ehci-msm.c   |4 ++--
  drivers/usb/host/ehci-tegra.c |   16 
  drivers/usb/host/ohci-omap.c  |   20 ++--
  drivers/usb/misc/lvstest.c|8 
  include/linux/usb/hcd.h   |2 +-
  10 files changed, 49 insertions(+), 49 deletions(-)

This doesn't apply to my tree at all anymore, can you refresh it and
resend?

thanks,

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

Re: [PATCH] drivers/misc/ti-st: Load firmware from ti-connectivity directory.

[Greg KH]

On Tue, Jul 22, 2014 at 01:08:38PM +0200, Enric Balletbo i Serra wrote:
 Looks like the default location for TI firmware is inside the ti-connectivity
 directory, to be coherent with other firmware request used by TI drivers, load
 the TIInit firmware from this directory instead of /lib/firmware directly.

Ah, it's this way in the linux-firmware package, I'll go queue this up
now, sorry for the delay.

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

Re: [PATCH] drivers/misc/ti-st: Load firmware from ti-connectivity directory.

[Greg KH]

On Tue, Jul 22, 2014 at 01:08:38PM +0200, Enric Balletbo i Serra wrote:
 Looks like the default location for TI firmware is inside the ti-connectivity
 directory, to be coherent with other firmware request used by TI drivers, load
 the TIInit firmware from this directory instead of /lib/firmware directly.
 
 Signed-off-by: Enric Balletbo i Serra eballe...@iseebcn.com
 ---
  drivers/misc/ti-st/st_kim.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/misc/ti-st/st_kim.c b/drivers/misc/ti-st/st_kim.c
 index 9d3dbb2..fa6a900 100644
 --- a/drivers/misc/ti-st/st_kim.c
 +++ b/drivers/misc/ti-st/st_kim.c
 @@ -244,7 +244,8 @@ static long read_local_version(struct kim_data_s 
 *kim_gdata, char *bts_scr_name)
   if (version  0x8000)
   maj_ver |= 0x0008;
  
 - sprintf(bts_scr_name, TIInit_%d.%d.%d.bts, chip, maj_ver, min_ver);
 + sprintf(bts_scr_name, ti-connectivity/TIInit_%d.%d.%d.bts,
 + chip, maj_ver, min_ver);

Can I get an ack from a ti.com address to verify that this is where the
driver really is?

thanks,

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

Re: Anybody working on tidspbridge?

[Greg KH]

On Thu, Jul 10, 2014 at 08:54:18AM +0300, Ivaylo Dimitrov wrote:
 
 
 On  9.07.2014 10:07, Tony Lindgren wrote:
 * Suman Anna s-a...@ti.com [140708 11:40]:
 Hi Peter,
 
 On 07/08/2014 09:36 AM, Greg KH wrote:
 On Tue, Jul 08, 2014 at 03:03:58PM +0200, Peter Meerwald wrote:
 Hello,
 
 Given the total lack of response here, I suggest just deleting the
 driver.  No one has ever done the real work that is going to be
 required to get this code out of staging.  It has had build errors
 causing it to not even be usable for some kernel versions with no one
 noticing, so I doubt anyone cares about it anymore here.
 
 Cc'ing some more people who might be interested. If no one offers to
 work on the driver in the next couple of days, I'll send a patch to
 remove it.
 
 I'm using the driver (with kernel 3.7) and it works reasonably well for
 me; removing it seems a bit harsh.
 
 Using it is different from being able to maintain the code and move it
 out of the staging tree.  Also, 3.7 is really old and obsolete, not much
 we can do with that kernel version :)
 
 Are you able to work on the code and do the effort needed to get it out
 of the staging tree?  If so, great, if not, we are going to have to
 delete it, sorry.
 
 I agree with Greg here. In fact, the current TODO does not do enough
 justice to the amount of work required to make it even work on the
 latest kernel. Most of the TIers who worked on this driver have moved on
 as Kristina would have figured with her bounced emails. So I do suggest
 that this driver be deleted from the kernel tree. If there are enough
 number of folks using it (not sure how many are out there), it can be
 worked on out-of-tree and brought back in a cleaner fashion rather than
 keeping a broken stale driver in the kernel.
 
 I agree, not much has improved with this driver since it got added into
 staging except just compile fixes.
 
 Regards,
 
 Tony
 
 Well, recently I've sent a couple of patches which implement stuff from TODO
 [1]. However, with the migration to DT, my focus now is to have a
 kernel/userspace that boots at all and this leaves no free cycles for DSP.
 Maybe tidspbridge can be left in staging until DT migration is finished,
 that way me (and maybe other people) will have the time needed to try to
 implement what remains in TODO. Also, keep in mind there will (hopefully) be
 another omap3 end-user device released by the end of the year (Neo900),
 which most probably will gain more developers interested in fixing the DSP
 driver.

I'm really tired of people saying, maybe sometime in the future we will
work on this for this driver.  It's not the first time I've heard it,
and it has _never_ come true.  Honestly, I really don't believe it will
ever happen, given that TI doesn't care about this code at all.

If in the future, someone does want to work on this, a simple revert of
the patch that removes the driver will be all that is needed.  Let's do
that instead of hoping that sometime, someone, somewhere, will do this
work.

thanks,

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

Re: [PATCH] Removes FIXME message in usb.c

[Greg KH]

Ok, this has been fun to watch on lkml for a while now, but really,
please, just stop doing this.  Randomly searching the kernel source for
FIXME lines and just commenting them out, isn't ok.  Almost always,
those lines are there because the original developer really doesn't know
how else to resolve the issue.

So, if the domain-specific-author doesn't have an idea of what to do,
how does someone who is brand-new to the code know better?

If you are looking for a task to do in the kernel, try
drivers/staging/*/TODO for a list.  Or look at the kernel janitor's list
on kernelnewbies.org.  Or try running the kernel and finding something
that is broken for you and fixing that.  Any of those would be better
than randomly deleting FIXME lines.  By doing that, you are just wasting
maintainer's time.  Which is the resource we have the least of at the
moment.

thanks,

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

Re: Anybody working on tidspbridge?

[Greg KH]

On Tue, Jul 08, 2014 at 03:03:58PM +0200, Peter Meerwald wrote:
 Hello,
 
   Given the total lack of response here, I suggest just deleting the
   driver.  No one has ever done the real work that is going to be
   required to get this code out of staging.  It has had build errors
   causing it to not even be usable for some kernel versions with no one
   noticing, so I doubt anyone cares about it anymore here.
  
  Cc'ing some more people who might be interested. If no one offers to
  work on the driver in the next couple of days, I'll send a patch to
  remove it.
 
 I'm using the driver (with kernel 3.7) and it works reasonably well for 
 me; removing it seems a bit harsh

Using it is different from being able to maintain the code and move it
out of the staging tree.  Also, 3.7 is really old and obsolete, not much
we can do with that kernel version :)

Are you able to work on the code and do the effort needed to get it out
of the staging tree?  If so, great, if not, we are going to have to
delete it, sorry.

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

Re: [PATCH] Removes FIXME message in usb.c

[Greg KH]

On Tue, Jul 08, 2014 at 09:58:39PM -0400, Nicholas Krause wrote:
 This patch removes a fixme message in this file:wq for setting the usb 2

I don't think you did this correctly :)

 speed on the board to the correct level. We need to depend on the
 bootloader for doing this as the wires may be shared for the other
 things on the board with the usb chipset.
 
 Signed-off-by: Nicholas Krause xerofo...@gmail.com
 ---
  arch/arm/mach-omap1/usb.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/arch/arm/mach-omap1/usb.c b/arch/arm/mach-omap1/usb.c
 index 4118db5..172245a 100644
 --- a/arch/arm/mach-omap1/usb.c
 +++ b/arch/arm/mach-omap1/usb.c
 @@ -505,7 +505,7 @@ static u32 __init omap1_usb2_init(unsigned nwires, 
 unsigned alt_pingroup)
   omap_cfg_reg(W5_USB2_SE0);
   if (nwires != 3)
   omap_cfg_reg(Y5_USB2_RCV);
 - // FIXME omap_cfg_reg(USB2_SPEED);
 + /* Depend on boatloader for USB speed to be stated for board */

Really?  Is that correct?  WHy?

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

Re: [PATCH] [media] staging: allow omap4iss to be modular

[Greg KH]

On Thu, Jun 12, 2014 at 04:15:32PM +0200, Arnd Bergmann wrote:
 On Thursday 12 June 2014 16:12:17 Laurent Pinchart wrote:
   From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
   From: Arnd Bergmann a...@arndb.de
   Date: Tue, 21 Jan 2014 09:32:43 +0100
   Subject: [PATCH] [media] staging: tighten omap4iss dependencies
   
   The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
   of which can be loadable modules. This causes build failures
   if we want the camera driver to be built-in.
   
   This can be solved by turning the option into tristate,
   which unfortunately causes another problem, because the
   driver incorrectly calls a platform-internal interface
   for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
   
   Instead, this patch just forbids the invalid configurations
   and ensures that the driver can only be built if all its
   dependencies are built-in.
   
   Signed-off-by: Arnd Bergmann a...@arndb.de
  
  Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
  
  Should I take this in my tree for v3.17 or would you like to fast-track it ?
  
 
 I'd actually like to see it in 3.15 as a stable backport if possible,

It's not stable material, sorry.

 but definitely in 3.16. What is the normal path for staging/media
 but fix patches?

Through Mauro's tree.

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

Re: [PATCH] [media] staging: allow omap4iss to be modular

[Greg KH]

On Thu, Jun 12, 2014 at 04:28:39PM +0200, Arnd Bergmann wrote:
 On Thursday 12 June 2014 07:25:15 Greg KH wrote:
  On Thu, Jun 12, 2014 at 04:15:32PM +0200, Arnd Bergmann wrote:
   On Thursday 12 June 2014 16:12:17 Laurent Pinchart wrote:
 From 3a965f4fd5a6b3ef4a66aa4e7c916cfd34fd5706 Mon Sep 17 00:00:00 2001
 From: Arnd Bergmann a...@arndb.de
 Date: Tue, 21 Jan 2014 09:32:43 +0100
 Subject: [PATCH] [media] staging: tighten omap4iss dependencies
 
 The OMAP4 camera support depends on I2C and VIDEO_V4L2, both
 of which can be loadable modules. This causes build failures
 if we want the camera driver to be built-in.
 
 This can be solved by turning the option into tristate,
 which unfortunately causes another problem, because the
 driver incorrectly calls a platform-internal interface
 for omap4_ctrl_pad_readl/omap4_ctrl_pad_writel.
 
 Instead, this patch just forbids the invalid configurations
 and ensures that the driver can only be built if all its
 dependencies are built-in.
 
 Signed-off-by: Arnd Bergmann a...@arndb.de

Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com

Should I take this in my tree for v3.17 or would you like to fast-track 
it ?

   
   I'd actually like to see it in 3.15 as a stable backport if possible,
  
  It's not stable material, sorry.
 
 To clarify, I was talking about second version of the patch,
 not the original one. It just does this:
 
   config VIDEO_OMAP4
bool OMAP 4 Camera support
  - depends on VIDEO_V4L2  VIDEO_V4L2_SUBDEV_API  I2C  ARCH_OMAP4
  + depends on VIDEO_V4L2=y  VIDEO_V4L2_SUBDEV_API  I2C=y  
  ARCH_OMAP4
select VIDEOBUF2_DMA_CONTIG
---help---
  Driver for an OMAP 4 ISS controller.
 
 which enforces that configurations that cannot be compiled
 will not be selectable in Kconfig, so we can have allmodconfig
 working. I thought that was ok for -stable.

Ah, yes, that one works, sorry, I was thinking of the first one.

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

Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

[Greg KH]

On Thu, May 29, 2014 at 02:26:55PM +0530, Satish Patel wrote:
 
 
 On 5/29/2014 12:14 AM, Greg KH wrote:
 On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:
 +/**
 + * struct sc_phy - The basic smart card phy structure
 + *
 + * @dev: phy device
 + * @pdata: pointer to phy's private data structure
 + * @set_config: called to set phy's configuration
 + * @get_config: called to get phy's configuration
 + * @activate_card: perform smart card activation
 + * @deactivate_card: perform smart card de-activation
 + * @warm_reset: execute smart card warm reset sequence
 + * @register_card_activity_cb: register call back to phy device.
 + * This call back will be called on card insert or remove event
 + *
 + * smart card controller uses this interface to communicate with
 + * smart card via phy.Some smart card phy has multiple slots for
 + * cards. This inerface also enables controller to communicate with
 + * one or more smart card connected over phy.
 + */
 +struct sc_phy {
 +   /* phy's device pointer */
 +   struct device *dev;
 
 So this is the parent, right?  Why not embed a struct device into this
 structure as well, further streaching out the device tree.
 
 Do you mean to use dev-p for pdata ?

No, use the device itself.

 I have kept it outside, to give easeness/pragmatic focal for new phy driver
 development. Driver can make this pointer null and use above pointer.

Ick, no, that's not how the driver model works.  Each device in the
system needs a struct device, don't try to chain off of an existing
device like this.  Make it a real one.

 +
 +   /* phy's private data */
 +   void *pdata;
 
 If you do the above, then this pointer is not needed.
 
 
 +
 +   /* notify data, passed by interface user as a part of
 +* register_notify API. Data should be passed back when
 +* notification raised to the interface user
 +*/
 +   void *notify_data;
 
 What makes this different from the pdata?
 pdata is phy's private data, while notify_data is something phy will send to
 smart card controller on some event, like card remove/insert/timeout etc..

That doesn't make much sense to me, why not just put that in the notify
function callback itself?

Please either use the existing phy layer of the kernel, or make a real
subsystem here, don't try to put a tiny shim on top of an existing
struct device for this driver, that's not how subsystems in Linux are
done.

thanks,

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

Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

[Greg KH]

On Thu, May 29, 2014 at 08:47:31AM -0500, Rob Herring wrote:
 On Thu, May 29, 2014 at 3:34 AM, Satish Patel satish.pa...@ti.com wrote:
 
 
  On 5/29/2014 12:23 AM, Greg KH wrote:
 
  On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:
 
  SmartCard controller uses this interface to communicate with
  SmartCard via PHY
 
  Some SmartCard PHY has multiple slots for cards.
  This inerface also enables controller to communicate
  with one or more SmartCard connected over phy.
 
  interface structure includes following APIs
  - set/get config
  - activate/deactivate smart card
  - warm reset
  - register_notify (for card insert/remove/overheat)
  - unregister_notify
 
  Signed-off-by: Satish Patel satish.pa...@ti.com
  ---
Documentation/sc_phy.txt |  171
  ++
include/linux/sc_phy.h   |  136 
2 files changed, 307 insertions(+)
create mode 100644 Documentation/sc_phy.txt
create mode 100644 include/linux/sc_phy.h
 
 
  These are .h files, but where is the api functions that use
  these structures defined at?
 
  This is like template/wrappers, smart card phy driver will write API
  functions. And smartcard controller will call these functions.
  With proposed approach, smartcard controller can communicate with any smart
  card phy (TI/NxP) without change in code. Using DT entry smartcard and PHY
  will gets connected with each other.
  Refer diagram given @Documentation/sc_phy.txt.
 
 
  confused,
 
 I believe the api Greg is wondering about is the notifier which as I
 commented is not a good design.

That, and the fact that if this really is an api, there are no .c
files for it like a normal api is in the kernel.

 There is now a phy subsystem. I don't know if it has what you need,
 but you should look at it to determine if it will work or could be
 extended to work.

I agree.  Satish, what's wrong with our existing phy layer?

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

Re: [PATCH v3 2/5] misc: tda8026: Add NXP TDA8026 PHY driver

[Greg KH]

On Thu, May 29, 2014 at 02:07:59PM +0530, Satish Patel wrote:
 
 
 On 5/29/2014 12:14 AM, Greg KH wrote:
 On Wed, May 28, 2014 at 02:27:14PM +0530, Satish Patel wrote:
 TDA8026 is a SmartCard PHY from NXP.
 
 The PHY interfaces with the main processor over the
 I2C interface and acts as a slave device.
 
 The driver also exposes the phy interface
 (defined@include/linux/sc_phy.h) for SmartCard controller.
 Controller uses this interface to communicate with smart card
 inserted to the phy's slot.
 
 Note: gpio irq is not validated as I do not have device with that.
 I have validated interrupt with dedicated interrupt line on my device.
 
 Signed-off-by: Satish Patel satish.pa...@ti.com
 ---
   Documentation/devicetree/bindings/misc/tda8026.txt |   19 +
   drivers/misc/Kconfig   |7 +
   drivers/misc/Makefile  |1 +
   drivers/misc/tda8026.c | 1258 
  
   4 files changed, 1285 insertions(+)
   create mode 100644 Documentation/devicetree/bindings/misc/tda8026.txt
   create mode 100644 drivers/misc/tda8026.c
 
 diff --git a/Documentation/devicetree/bindings/misc/tda8026.txt 
 b/Documentation/devicetree/bindings/misc/tda8026.txt
 new file mode 100644
 index 000..f115c9c
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/misc/tda8026.txt
 @@ -0,0 +1,19 @@
 +TDA8026 smart card slot interface
 +
 +This is an i2c based smart card interface device forming the electrical
 +interface between a microcontroller and smart cards. This device supports
 +asynchronous cards (micro controller-based IC cards) as well as synchronous
 +cards (mainly memory cards)
 +
 +Required properties:
 +- compatible: nxp,tda8026
 +- shutdown-gpio = GPIO pin mapping for SDWNN pin
 +- reg = i2c interface address
 +
 +
 +Example:
 +tda8026: tda8026@48 {
 +compatible = nxp,tda8026;
 +reg = 0x48;
 +shutdown-gpio = gpio5 19 GPIO_ACTIVE_HIGH;/* Bank5, pin19 */
 +};
 diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
 index 8baff0e..80b21d7 100644
 --- a/drivers/misc/Kconfig
 +++ b/drivers/misc/Kconfig
 @@ -515,6 +515,13 @@ config SRAM
   the genalloc API. It is supposed to be used for small on-chip SRAM
   areas found on many SoCs.
 
 +config NXP_TDA8026_PHY
 +tristate NXP PHY Driver for Smart Card PHY
 +depends on I2C=y
 +help
 +  If you say yes here you get support for the TDA8026 Smart card 
 PHY
 + with I2C interface.
 +
   source drivers/misc/c2port/Kconfig
   source drivers/misc/eeprom/Kconfig
   source drivers/misc/cb710/Kconfig
 diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
 index 7eb4b69..f262c0b 100644
 --- a/drivers/misc/Makefile
 +++ b/drivers/misc/Makefile
 @@ -55,3 +55,4 @@ obj-$(CONFIG_SRAM)+= sram.o
   obj-y += mic/
   obj-$(CONFIG_GENWQE)  += genwqe/
   obj-$(CONFIG_ECHO)+= echo/
 +obj-$(CONFIG_NXP_TDA8026_PHY)  += tda8026.o
 diff --git a/drivers/misc/tda8026.c b/drivers/misc/tda8026.c
 new file mode 100644
 index 000..38df33e
 --- /dev/null
 +++ b/drivers/misc/tda8026.c
 @@ -0,0 +1,1258 @@
 +/*
 + * tda8026.c - TDA8026 PHY driver for NXP Smart card PHY
 + *
 + *
 + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
 + *
 + * 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 version 2.
 + *
 + * This program is distributed as is WITHOUT ANY WARRANTY of any
 + * kind, whether express or implied; without even the implied warranty
 + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +
 +#include linux/module.h
 +#include linux/moduleparam.h
 +#include linux/interrupt.h
 +#include linux/init.h
 +#include linux/slab.h
 +#include linux/gpio.h
 +#include linux/i2c.h
 +#include linux/mfd/core.h
 +#include linux/notifier.h
 +#include linux/sc_phy.h
 +#include linux/of_gpio.h
 +#include linux/of_device.h
 +#include linux/delay.h
 
 I think you just broke the build if this driver is enabled now right?
 
 Not good :(
 Before sending, I have applied these patches to v3.15-rc7 and build with
 both the option ti-usim  tda8026 as module, as well as part of kernel. Any
 specific tree you would like me to rebase these patches against.

Did you try applying the patches one-by-one and building afterwards
between each one?  In looking at this further, I think it will work, but
please test and make sure.  You can not break the build with any
individual patch.

thanks,

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

Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

[Greg KH]

On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:
 +/**
 + * struct sc_phy - The basic smart card phy structure
 + *
 + * @dev: phy device
 + * @pdata: pointer to phy's private data structure
 + * @set_config: called to set phy's configuration
 + * @get_config: called to get phy's configuration
 + * @activate_card: perform smart card activation
 + * @deactivate_card: perform smart card de-activation
 + * @warm_reset: execute smart card warm reset sequence
 + * @register_card_activity_cb: register call back to phy device.
 + * This call back will be called on card insert or remove event
 + *
 + * smart card controller uses this interface to communicate with
 + * smart card via phy.Some smart card phy has multiple slots for
 + * cards. This inerface also enables controller to communicate with
 + * one or more smart card connected over phy.
 + */
 +struct sc_phy {
 + /* phy's device pointer */
 + struct device *dev;

So this is the parent, right?  Why not embed a struct device into this
structure as well, further streaching out the device tree.

 +
 + /* phy's private data */
 + void *pdata;

If you do the above, then this pointer is not needed.


 +
 + /* notify data, passed by interface user as a part of
 +  * register_notify API. Data should be passed back when
 +  * notification raised to the interface user
 +  */
 + void *notify_data;

What makes this different from the pdata?


 +
 + int (*set_config)(struct sc_phy *phy, u8 slot,
 + enum sc_phy_config attr, int value);
 + int (*get_config)(struct sc_phy *phy, u8 slot, enum
 + sc_phy_config attr);
 + int (*activate_card)(struct sc_phy *phy, u8 slot);
 + int (*deactivate_card)(struct sc_phy *phy, u8 slot);
 + int (*get_syncatr)(struct sc_phy *phy, u8 slot, u8 len, char *atr);
 + int (*warm_reset)(struct sc_phy *phy, u8 slot);
 + int (*register_notify)(struct sc_phy *phy,
 +  struct notifier_block *nb, void *notify_data);
 + int (*unregister_notify)(struct sc_phy *phy,
 + struct  notifier_block *nb);
 +};
 +
 +#endif /* __SC_PHY_H__ */
 -- 
 1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] misc: tda8026: Add NXP TDA8026 PHY driver

[Greg KH]

On Wed, May 28, 2014 at 02:27:14PM +0530, Satish Patel wrote:
 TDA8026 is a SmartCard PHY from NXP.
 
 The PHY interfaces with the main processor over the
 I2C interface and acts as a slave device.
 
 The driver also exposes the phy interface
 (defined@include/linux/sc_phy.h) for SmartCard controller.
 Controller uses this interface to communicate with smart card
 inserted to the phy's slot.
 
 Note: gpio irq is not validated as I do not have device with that.
 I have validated interrupt with dedicated interrupt line on my device.
 
 Signed-off-by: Satish Patel satish.pa...@ti.com
 ---
  Documentation/devicetree/bindings/misc/tda8026.txt |   19 +
  drivers/misc/Kconfig   |7 +
  drivers/misc/Makefile  |1 +
  drivers/misc/tda8026.c | 1258 
 
  4 files changed, 1285 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/misc/tda8026.txt
  create mode 100644 drivers/misc/tda8026.c
 
 diff --git a/Documentation/devicetree/bindings/misc/tda8026.txt 
 b/Documentation/devicetree/bindings/misc/tda8026.txt
 new file mode 100644
 index 000..f115c9c
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/misc/tda8026.txt
 @@ -0,0 +1,19 @@
 +TDA8026 smart card slot interface
 +
 +This is an i2c based smart card interface device forming the electrical
 +interface between a microcontroller and smart cards. This device supports
 +asynchronous cards (micro controller-based IC cards) as well as synchronous
 +cards (mainly memory cards)
 +
 +Required properties:
 +- compatible: nxp,tda8026
 +- shutdown-gpio = GPIO pin mapping for SDWNN pin
 +- reg = i2c interface address
 +
 +
 +Example:
 +tda8026: tda8026@48 {
 +  compatible = nxp,tda8026;
 +  reg = 0x48;
 +  shutdown-gpio = gpio5 19 GPIO_ACTIVE_HIGH;/* Bank5, pin19 */
 +  };
 diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
 index 8baff0e..80b21d7 100644
 --- a/drivers/misc/Kconfig
 +++ b/drivers/misc/Kconfig
 @@ -515,6 +515,13 @@ config SRAM
 the genalloc API. It is supposed to be used for small on-chip SRAM
 areas found on many SoCs.
  
 +config NXP_TDA8026_PHY
 +tristate NXP PHY Driver for Smart Card PHY
 +depends on I2C=y
 +help
 +  If you say yes here you get support for the TDA8026 Smart card PHY
 +   with I2C interface.
 +
  source drivers/misc/c2port/Kconfig
  source drivers/misc/eeprom/Kconfig
  source drivers/misc/cb710/Kconfig
 diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
 index 7eb4b69..f262c0b 100644
 --- a/drivers/misc/Makefile
 +++ b/drivers/misc/Makefile
 @@ -55,3 +55,4 @@ obj-$(CONFIG_SRAM)  += sram.o
  obj-y+= mic/
  obj-$(CONFIG_GENWQE) += genwqe/
  obj-$(CONFIG_ECHO)   += echo/
 +obj-$(CONFIG_NXP_TDA8026_PHY)+= tda8026.o
 diff --git a/drivers/misc/tda8026.c b/drivers/misc/tda8026.c
 new file mode 100644
 index 000..38df33e
 --- /dev/null
 +++ b/drivers/misc/tda8026.c
 @@ -0,0 +1,1258 @@
 +/*
 + * tda8026.c - TDA8026 PHY driver for NXP Smart card PHY
 + *
 + *
 + * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/
 + *
 + * 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 version 2.
 + *
 + * This program is distributed as is WITHOUT ANY WARRANTY of any
 + * kind, whether express or implied; without even the implied warranty
 + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +
 +#include linux/module.h
 +#include linux/moduleparam.h
 +#include linux/interrupt.h
 +#include linux/init.h
 +#include linux/slab.h
 +#include linux/gpio.h
 +#include linux/i2c.h
 +#include linux/mfd/core.h
 +#include linux/notifier.h
 +#include linux/sc_phy.h
 +#include linux/of_gpio.h
 +#include linux/of_device.h
 +#include linux/delay.h

I think you just broke the build if this driver is enabled now right?

Not good :(

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

Re: [PATCH v3 1/5] sc_phy:SmartCard(SC) PHY interface to SC controller

[Greg KH]

On Wed, May 28, 2014 at 02:27:13PM +0530, Satish Patel wrote:
 SmartCard controller uses this interface to communicate with
 SmartCard via PHY
 
 Some SmartCard PHY has multiple slots for cards.
 This inerface also enables controller to communicate
 with one or more SmartCard connected over phy.
 
 interface structure includes following APIs
 - set/get config
 - activate/deactivate smart card
 - warm reset
 - register_notify (for card insert/remove/overheat)
 - unregister_notify
 
 Signed-off-by: Satish Patel satish.pa...@ti.com
 ---
  Documentation/sc_phy.txt |  171 
 ++
  include/linux/sc_phy.h   |  136 
  2 files changed, 307 insertions(+)
  create mode 100644 Documentation/sc_phy.txt
  create mode 100644 include/linux/sc_phy.h

These are .h files, but where is the api functions that use
these structures defined at?

confused,

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

Re: [PATCH v2 0/5] Smart Card(SC) interface, TI USIM NxP SC phy driver

[Greg KH]

On Fri, May 16, 2014 at 09:14:34AM +0530, Satish Patel wrote:
 
 On 1/30/2014 6:35 PM, Greg KH wrote:
 On Thu, Jan 30, 2014 at 11:22:48AM +0530, Satish Patel wrote:
 On 1/20/2014 10:03 AM, Satish Patel wrote:
 Changes from v1:
 * RFC(v1) comments are fixed
 
 ** removed gpio_to_irq as GPIO controller process  cell from DT and
 give it to DT node
 ** comments on documentation
 ** few other comments on null checks are resolved
 
 * BWT timing configuration is added to ti-usim driver
 
 v1 cover letter link#
 https://lkml.org/lkml/2014/1/6/250
 
 Satish Patel (5):
sc_phy:SmartCard(SC) PHY interface to SC controller
misc: tda8026: Add NXP TDA8026 PHY driver
char: ti-usim: Add driver for USIM module on AM43xx
ARM: dts: AM43xx: DT entries added for ti-usim
ARM: dts: AM43xx-epos-evm: DT entries  for ti-usim and phy
 
   Documentation/devicetree/bindings/misc/tda8026.txt |   19 +
   .../devicetree/bindings/ti-usim/ti-usim.txt|   31 +
   Documentation/sc_phy.txt   |  171 ++
   arch/arm/boot/dts/am4372.dtsi  |   10 +
   arch/arm/boot/dts/am43x-epos-evm.dts   |   43 +
   drivers/char/Kconfig   |7 +
   drivers/char/Makefile  |1 +
   drivers/char/ti-usim-hw.h  |  863 +
   drivers/char/ti-usim.c | 1859 
  
   drivers/misc/Kconfig   |7 +
   drivers/misc/Makefile  |1 +
   drivers/misc/tda8026.c | 1255 +
   include/linux/sc_phy.h |  132 ++
   include/linux/ti-usim.h|   98 +
   14 files changed, 4497 insertions(+), 0 deletions(-)
   create mode 100644 Documentation/devicetree/bindings/misc/tda8026.txt
   create mode 100644 Documentation/devicetree/bindings/ti-usim/ti-usim.txt
   create mode 100644 Documentation/sc_phy.txt
   create mode 100644 drivers/char/ti-usim-hw.h
   create mode 100644 drivers/char/ti-usim.c
   create mode 100644 drivers/misc/tda8026.c
   create mode 100644 include/linux/sc_phy.h
   create mode 100644 include/linux/ti-usim.h
 Any comments on this patch series ?
 
 If not,
 Can you accept these patches for next merge window
 It's the middle of this merge window, and I can't accept any patches
 until after 3.14-rc1 is out, at which point I'll start to work on my
 patch backlog.
 Are these to be consider for next submission ? Or you want me to start
 review cycle one more time.

I don't have them in my queue, so please resend.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/13] extcon: major rework

[Greg KH]

On Mon, Apr 14, 2014 at 01:46:11PM +0200, Robert Baldyga wrote:
 This patchset adds many improvements to extcon class driver and extcon
 provider drivers. It changes extcon API to faster and safer by replaceing
 function taking extcon and cable names with functions working with
 structures representing this objects.

What is faster exactly?  How did you measure this?  What benchmarks
can be run to test this claim?

thanks,

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

Re: omap-serial: use mctrl_gpio helpers

[Greg KH]

On Thu, Apr 24, 2014 at 11:24:30AM +0200, yegorsli...@googlemail.com wrote:
 This patch is based on Richard Genoud' patch adding generic GPIO support [1]
 and my patch adding get_direction() functionality to OMAP's GPIO driver [2].
 
 So far RTS/DTR are working both as modem control outputs and RTS as RS-485 
 2-wire controller.
 
 CTS/DSR/DSR/RI are generating interrupts.
 
 Richard, Greg, what is the status of [1]? It seems like it will be available 
 in 3.16 first?

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

Re: [PATCH 01/11] tty: serial: add missing braces

[Greg KH]

On Tue, Apr 22, 2014 at 09:22:56AM -0500, Felipe Balbi wrote:
 Hi,
 
 On Thu, Mar 20, 2014 at 02:29:57PM -0500, Felipe Balbi wrote:
  per CodingStyle we should have those braces, no
  functional changes.
  
  Signed-off-by: Felipe Balbi ba...@ti.com
 
 Greg, do you want me to refresh and resend this series ?

If there are conflicts with my current tree, sure, I'd love to have
that.  I should be trying to dig out from my patch backlog for 3.16-rc1
by the end of this week...

thanks,

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

Re: [PATCH 05/10] usb: gadget: only GPL drivers in the gadget and phy framework

[Greg KH]

On Mon, Apr 21, 2014 at 10:56:47AM -0500, Felipe Balbi wrote:
 We only support GPL drivers in the USB Gadget Framework,
 it sounds correct to make all exported symbols GPL too.
 
 Signed-off-by: Felipe Balbi ba...@ti.com

Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 03/10] usb: phy: mv-u3d: switch over to writel/readl

[Greg KH]

On Mon, Apr 21, 2014 at 10:56:45AM -0500, Felipe Balbi wrote:
 by removing the _relaxed suffix, we can build
 this driver in other architectures.

Odd, why was the _relaxed variants used here at all?  Someone trying to
optimize something ahead of time?

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

Re: [RFC] possible removal of omap-serial

[Greg KH]

On Fri, Mar 21, 2014 at 12:36:53PM -0500, Felipe Balbi wrote:
 Hi,
 
 On Fri, Mar 21, 2014 at 10:10:30AM -0700, Tony Lindgren wrote:
On the support side, I'm not looking forward to this for beagle/panda
users.  We've already converted them once from ttySx - ttyOx back in
2.6.33/2.6.34? days.  That was an irc/email/u-boot/kernel nightmare...
   
   that's exactly why we're talking about ways to maintain backwards
   compatibility here. But I'm more interested in finding a way to switch
   over to ttyS and have a symlink to ttyO, that way a simple debootstrap
   (or any other ARM distro minimal rootfs) would work out of the box,
   without any changes, just like in normal systems.
  
  Yeah let's not break things. The symlink solution won't work for kernel
 
 again. Let's not break things again.
 
  console output so it needs to be dealt with some other way.
 
 it would if kernel itself would create the symlink, but that might not
 be doable in a clean way.

The kernel isn't going to create the symlink, sorry, no.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] possible removal of omap-serial

[Greg KH]

On Thu, Mar 20, 2014 at 06:52:10PM -0500, Felipe Balbi wrote:
 Hi folks,
 
 I've been toying with the idea of removing
 drivers/tty/serial/omap-serial.c since that's, to put it bluntly, an
 ungly copy of 8250 driver.
 
 The original concern was wrt suspend/resume but I think it'd be a far
 better approach to implement runtime PM in 8250 and write a rather small
 8250-omap.c glue (much like 8250-acorn.c or 8250-dw.c) just to get the
 OMAP-specific details out of the way.
 
 The question I have is: omap-serial.c calls the serial devnodes ttyO\d,
 instead of ttyS\d so removing omap-serial.c would have a direct impact
 in userland. I wonder if it's an acceptable regression considering
 we'd be able to reuse 8250 gaining proper Flow Control support, proper
 DMA support, years and years of bug-fixes, etc.

Breaking device node names is a contentious issue for serial ports, I
don't think you can do that :(

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

Re: [RFC] possible removal of omap-serial

[Greg KH]

On Thu, Mar 20, 2014 at 08:37:29PM -0500, Felipe Balbi wrote:
 On Thu, Mar 20, 2014 at 08:35:57PM -0500, Felipe Balbi wrote:
  Hi,
  
  On Thu, Mar 20, 2014 at 05:12:28PM -0700, Greg KH wrote:
   On Thu, Mar 20, 2014 at 06:52:10PM -0500, Felipe Balbi wrote:
Hi folks,

I've been toying with the idea of removing
drivers/tty/serial/omap-serial.c since that's, to put it bluntly, an
ungly copy of 8250 driver.

The original concern was wrt suspend/resume but I think it'd be a far
better approach to implement runtime PM in 8250 and write a rather small
8250-omap.c glue (much like 8250-acorn.c or 8250-dw.c) just to get the
OMAP-specific details out of the way.

The question I have is: omap-serial.c calls the serial devnodes ttyO\d,
instead of ttyS\d so removing omap-serial.c would have a direct impact
in userland. I wonder if it's an acceptable regression considering
we'd be able to reuse 8250 gaining proper Flow Control support, proper
DMA support, years and years of bug-fixes, etc.
   
   Breaking device node names is a contentious issue for serial ports, I
   don't think you can do that :(
  
  would an upstream udev rule creating a symbolic link from ttyO to ttyS
  be enough ?
  
  I didn't test this yet but I guess this is enough (?)
  
  KERNEL==ttyO[0-9], GROUP=dialout, SYMLINK+=ttyS
 
 or actually it should be to other way around, ttyS would be the real
 device:
 
 KERNEL==ttyS[0-9], GROUP=dialout, SYMLINK+=ttyO

As udev rules don't ship with the kernel, this might be tough to do :(

Might be easier to make the 8250 driver handle different names like
Alan said.

greg k-h

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

Re: [PATCH v4 1/2] usb: musb: dsps, debugfs files

[Greg KH]

On Tue, Feb 18, 2014 at 10:20:54AM -0600, Felipe Balbi wrote:
 On Fri, Jan 17, 2014 at 10:22:35AM +0100, Markus Pargmann wrote:
  debugfs files to show the contents of important dsps registers.
  
  Signed-off-by: Markus Pargmann m...@pengutronix.de
  ---
   drivers/usb/musb/musb_dsps.c | 54 
  
   1 file changed, 54 insertions(+)
  
  diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
  index 593d3c9..d0a97d6 100644
  --- a/drivers/usb/musb/musb_dsps.c
  +++ b/drivers/usb/musb/musb_dsps.c
  @@ -46,6 +46,8 @@
   #include linux/of_irq.h
   #include linux/usb/of.h
   
  +#include linux/debugfs.h
  +
   #include musb_core.h
   
   static const struct of_device_id musb_dsps_of_match[];
  @@ -137,6 +139,26 @@ struct dsps_glue {
  unsigned long last_timer;/* last timer data for each instance */
   
  struct dsps_context context;
  +   struct debugfs_regset32 regset;
  +   struct dentry *dbgfs_root;
  +};
  +
  +static const struct debugfs_reg32 dsps_musb_regs[] = {
  +   { revision,   0x00 },
  +   { control,0x14 },
  +   { status, 0x18 },
  +   { eoi,0x24 },
  +   { intr0_stat, 0x30 },
  +   { intr1_stat, 0x34 },
  +   { intr0_set,  0x38 },
  +   { intr1_set,  0x3c },
  +   { txmode, 0x70 },
  +   { rxmode, 0x74 },
  +   { autoreq,0xd0 },
  +   { srpfixtime, 0xd4 },
  +   { tdown,  0xd8 },
  +   { phy_utmi,   0xe0 },
  +   { mode,   0xe8 },
   };
   
   static void dsps_musb_try_idle(struct musb *musb, unsigned long timeout)
  @@ -369,6 +391,30 @@ out:
  return ret;
   }
   
  +static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue *glue)
  +{
  +   struct dentry *root;
  +   struct dentry *file;
  +   char buf[128];
  +
  +   sprintf(buf, %s.dsps, dev_name(musb-controller));
  +   root = debugfs_create_dir(buf, NULL);
  +   if (!root)
 
 wrong, you should be using IS_ERR()

!root is fine, IS_ERR() will fail if CONFIG_DEBUGFS is not enabled.

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

Re: [PATCH v4 1/2] usb: musb: dsps, debugfs files

[Greg KH]

On Tue, Feb 18, 2014 at 11:03:35AM -0600, Felipe Balbi wrote:
 On Tue, Feb 18, 2014 at 08:59:11AM -0800, Greg KH wrote:
  On Tue, Feb 18, 2014 at 10:20:54AM -0600, Felipe Balbi wrote:
   On Fri, Jan 17, 2014 at 10:22:35AM +0100, Markus Pargmann wrote:
debugfs files to show the contents of important dsps registers.

Signed-off-by: Markus Pargmann m...@pengutronix.de
---
 drivers/usb/musb/musb_dsps.c | 54 

 1 file changed, 54 insertions(+)

diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
index 593d3c9..d0a97d6 100644
--- a/drivers/usb/musb/musb_dsps.c
+++ b/drivers/usb/musb/musb_dsps.c
@@ -46,6 +46,8 @@
 #include linux/of_irq.h
 #include linux/usb/of.h
 
+#include linux/debugfs.h
+
 #include musb_core.h
 
 static const struct of_device_id musb_dsps_of_match[];
@@ -137,6 +139,26 @@ struct dsps_glue {
unsigned long last_timer;/* last timer data for each 
instance */
 
struct dsps_context context;
+   struct debugfs_regset32 regset;
+   struct dentry *dbgfs_root;
+};
+
+static const struct debugfs_reg32 dsps_musb_regs[] = {
+   { revision,   0x00 },
+   { control,0x14 },
+   { status, 0x18 },
+   { eoi,0x24 },
+   { intr0_stat, 0x30 },
+   { intr1_stat, 0x34 },
+   { intr0_set,  0x38 },
+   { intr1_set,  0x3c },
+   { txmode, 0x70 },
+   { rxmode, 0x74 },
+   { autoreq,0xd0 },
+   { srpfixtime, 0xd4 },
+   { tdown,  0xd8 },
+   { phy_utmi,   0xe0 },
+   { mode,   0xe8 },
 };
 
 static void dsps_musb_try_idle(struct musb *musb, unsigned long 
timeout)
@@ -369,6 +391,30 @@ out:
return ret;
 }
 
+static int dsps_musb_dbg_init(struct musb *musb, struct dsps_glue 
*glue)
+{
+   struct dentry *root;
+   struct dentry *file;
+   char buf[128];
+
+   sprintf(buf, %s.dsps, dev_name(musb-controller));
+   root = debugfs_create_dir(buf, NULL);
+   if (!root)
   
   wrong, you should be using IS_ERR()
  
  !root is fine, IS_ERR() will fail if CONFIG_DEBUGFS is not enabled.
 
 in that case, files will be created on parent directory right ?

If, for some reason, creating a directory fails and then creating a file
would succeed, yes, that will happen.

 If we pass a ERR_PTR(-ENODEV), otoh, we will try to dereference it in
 __create_file().

No, because -ENODEV will only happen if debugfs is not enabled, so no
dereference will ever happen.

Don't worry about checking return values from debugfs, it shouldn't be
needed.

thanks,

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

Re: [PATCH 0/2] usb: musb: host: fixes for 3.14-rc

[Greg KH]

On Tue, Feb 04, 2014 at 03:25:47PM +0200, Roger Quadros wrote:
 Hi Greg,
 
 Patch 1 fixes SuperSpeed hub enumeration on beaglebone.
 Patch 2 fixes remote-wakeup resume on beaglebone.
 
 Felipe has Acked the 1st patch but still needs to Ack the 2nd one.
 
 Patches are based on 3.14-rc1

Why wouldn't these come into my tree from Felipe like normal?  Why
should I take them?

confused,

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

Re: [PATCH v2 0/5] Smart Card(SC) interface, TI USIM NxP SC phy driver

[Greg KH]

On Thu, Jan 30, 2014 at 11:22:48AM +0530, Satish Patel wrote:
 
 On 1/20/2014 10:03 AM, Satish Patel wrote:
  Changes from v1:
  * RFC(v1) comments are fixed
 
  ** removed gpio_to_irq as GPIO controller process  cell from DT and
  give it to DT node
  ** comments on documentation
  ** few other comments on null checks are resolved
 
  * BWT timing configuration is added to ti-usim driver
 
  v1 cover letter link#
  https://lkml.org/lkml/2014/1/6/250
 
  Satish Patel (5):
sc_phy:SmartCard(SC) PHY interface to SC controller
misc: tda8026: Add NXP TDA8026 PHY driver
char: ti-usim: Add driver for USIM module on AM43xx
ARM: dts: AM43xx: DT entries added for ti-usim
ARM: dts: AM43xx-epos-evm: DT entries  for ti-usim and phy
 
   Documentation/devicetree/bindings/misc/tda8026.txt |   19 +
   .../devicetree/bindings/ti-usim/ti-usim.txt|   31 +
   Documentation/sc_phy.txt   |  171 ++
   arch/arm/boot/dts/am4372.dtsi  |   10 +
   arch/arm/boot/dts/am43x-epos-evm.dts   |   43 +
   drivers/char/Kconfig   |7 +
   drivers/char/Makefile  |1 +
   drivers/char/ti-usim-hw.h  |  863 +
   drivers/char/ti-usim.c | 1859 
  
   drivers/misc/Kconfig   |7 +
   drivers/misc/Makefile  |1 +
   drivers/misc/tda8026.c | 1255 +
   include/linux/sc_phy.h |  132 ++
   include/linux/ti-usim.h|   98 +
   14 files changed, 4497 insertions(+), 0 deletions(-)
   create mode 100644 Documentation/devicetree/bindings/misc/tda8026.txt
   create mode 100644 Documentation/devicetree/bindings/ti-usim/ti-usim.txt
   create mode 100644 Documentation/sc_phy.txt
   create mode 100644 drivers/char/ti-usim-hw.h
   create mode 100644 drivers/char/ti-usim.c
   create mode 100644 drivers/misc/tda8026.c
   create mode 100644 include/linux/sc_phy.h
   create mode 100644 include/linux/ti-usim.h
 Any comments on this patch series ?
 
 If not,
 Can you accept these patches for next merge window

It's the middle of this merge window, and I can't accept any patches
until after 3.14-rc1 is out, at which point I'll start to work on my
patch backlog.

thanks,

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

Re: [PATCH v7 09/10] usb: dwc3: omap: manage usb_otg_ss_refclk960m clock

[Greg KH]

On Fri, Oct 04, 2013 at 01:46:08PM +0300, Roger Quadros wrote:
 Greg,
 
 On 10/03/2013 06:41 PM, Greg KH wrote:
  On Thu, Oct 03, 2013 at 05:54:14PM +0300, Roger Quadros wrote:
  On 10/03/2013 03:29 PM, Felipe Balbi wrote:
  Hi,
 
  On Wed, Oct 02, 2013 at 04:41:53PM +0300, Roger Quadros wrote:
  On 10/02/2013 04:11 PM, Felipe Balbi wrote:
  On Mon, Sep 23, 2013 at 04:11:30PM +0300, Roger Quadros wrote:
  Hi Felipe,
 
  On 09/18/2013 03:49 PM, Roger Quadros wrote:
  usb_otg_ss_refclk960m is an optional functional clock to the
  UBS_OTG_SS module. So manage it in the driver.
 
 
  Just realized that usb_otg_ss_refclk960m is in fact functional clock 
  to the 
  PHY and not USB_OTG_SS module. The name is misleading.
 
  So please ignore patch 9 and 10.
 
  ignored. All others are fine, right ?
 
  Yes. But I might have to rebase this on top of Phy framework stuff.
 
  alright, Greg already took the PHY framework, so maybe we need to just
  give him my Acked-by and he takes the patches directly as I don't have
  PYH framework.
 
  OK. I'll resend this series to Greg in a while.
  
  It looks like you did, but forgot Felipe's ack :(
  
 I was under the impression that Felipe will explicitly give his Ack there.
 Should I resend with his Acked-bys?

No need, I took the patches yesterday, you should have seen the
automated emails, right?

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

Re: [PATCH v7 09/10] usb: dwc3: omap: manage usb_otg_ss_refclk960m clock

[Greg KH]

On Thu, Oct 03, 2013 at 05:54:14PM +0300, Roger Quadros wrote:
 On 10/03/2013 03:29 PM, Felipe Balbi wrote:
  Hi,
  
  On Wed, Oct 02, 2013 at 04:41:53PM +0300, Roger Quadros wrote:
  On 10/02/2013 04:11 PM, Felipe Balbi wrote:
  On Mon, Sep 23, 2013 at 04:11:30PM +0300, Roger Quadros wrote:
  Hi Felipe,
 
  On 09/18/2013 03:49 PM, Roger Quadros wrote:
  usb_otg_ss_refclk960m is an optional functional clock to the
  UBS_OTG_SS module. So manage it in the driver.
 
 
  Just realized that usb_otg_ss_refclk960m is in fact functional clock 
  to the 
  PHY and not USB_OTG_SS module. The name is misleading.
 
  So please ignore patch 9 and 10.
 
  ignored. All others are fine, right ?
 
  Yes. But I might have to rebase this on top of Phy framework stuff.
  
  alright, Greg already took the PHY framework, so maybe we need to just
  give him my Acked-by and he takes the patches directly as I don't have
  PYH framework.
  
 OK. I'll resend this series to Greg in a while.

It looks like you did, but forgot Felipe's ack :(
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v12 0/7] PHY framework

[Greg KH]

On Fri, Sep 27, 2013 at 11:53:24AM +0530, Kishon Vijay Abraham I wrote:
 Added a generic PHY framework that provides a set of APIs for the PHY drivers
 to create/destroy a PHY and APIs for the PHY users to obtain a reference to
 the PHY with or without using phandle.
 
 This framework will be of use only to devices that uses external PHY (PHY
 functionality is not embedded within the controller).
 
 The intention of creating this framework is to bring the phy drivers spread
 all over the Linux kernel to drivers/phy to increase code re-use and to
 increase code maintainability.
 
 Comments to make PHY as bus wasn't done because PHY devices can be part of
 other bus and making a same device attached to multiple bus leads to bad
 design.
 
 If the PHY driver has to send notification on connect/disconnect, the PHY
 driver should make use of the extcon framework. Using this susbsystem
 to use extcon framwork will have to be analysed.
 
 You can find this patch series @
 git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git testing
 
 I'll create a new branch *next* once this patch series is finalized. All the
 PHY driver development that depends on PHY framework can be based on this
 branch.
 
 Did USB enumeration testing in panda and beagle after applying [1] (needed for
 non-dt)

All now applied to my usb-next branch.  Thanks for redoing this many
times and sticking with it.

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

Re: [PATCH v11 2/8] usb: phy: omap-usb2: use the new generic PHY framework

[Greg KH]

On Wed, Aug 21, 2013 at 11:16:07AM +0530, Kishon Vijay Abraham I wrote:
 Used the generic PHY framework API to create the PHY. Now the power off and
 power on are done in omap_usb_power_off and omap_usb_power_on respectively.
 The omap-usb2 driver is also moved to driver/phy.
 
 However using the old USB PHY library cannot be completely removed
 because OTG is intertwined with PHY and moving to the new framework
 will break OTG. Once we have a separate OTG state machine, we
 can get rid of the USB PHY library.
 
 Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
 Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
 Acked-by: Felipe Balbi ba...@ti.com
 ---
  drivers/phy/Kconfig   |   12 +
  drivers/phy/Makefile  |1 +
  drivers/{usb = }/phy/phy-omap-usb2.c |   45 
 ++---
  drivers/usb/phy/Kconfig   |   10 
  drivers/usb/phy/Makefile  |1 -
  5 files changed, 54 insertions(+), 15 deletions(-)
  rename drivers/{usb = }/phy/phy-omap-usb2.c (88%)

I tried to apply this to my USB branch, but it fails.

Kishon, you were going to refresh this patch series, right?  Please do,
because as-is, I can't take it.

thanks,

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

Re: [PATCH v11 0/8] PHY framework

[Greg KH]

On Fri, Sep 20, 2013 at 11:04:40AM +0530, Kishon Vijay Abraham I wrote:
 Hi Greg,
 
 On Tuesday 17 September 2013 09:11 PM, Felipe Balbi wrote:
  On Wed, Sep 04, 2013 at 02:27:06PM +0530, Kishon Vijay Abraham I wrote:
  On Tuesday 03 September 2013 09:20 PM, Greg KH wrote:
  On Tue, Sep 03, 2013 at 08:55:23PM +0530, Kishon Vijay Abraham I wrote:
  Hi Greg,
 
  On Wednesday 28 August 2013 12:50 AM, Felipe Balbi wrote:
  Hi,
 
  On Mon, Aug 26, 2013 at 01:44:49PM +0530, Kishon Vijay Abraham I wrote:
  On Wednesday 21 August 2013 11:16 AM, Kishon Vijay Abraham I wrote:
  Added a generic PHY framework that provides a set of APIs for the PHY 
  drivers
  to create/destroy a PHY and APIs for the PHY users to obtain a 
  reference to
  the PHY with or without using phandle.
 
  This framework will be of use only to devices that uses external PHY 
  (PHY
  functionality is not embedded within the controller).
 
  The intention of creating this framework is to bring the phy drivers 
  spread
  all over the Linux kernel to drivers/phy to increase code re-use and 
  to
  increase code maintainability.
 
  Comments to make PHY as bus wasn't done because PHY devices can be 
  part of
  other bus and making a same device attached to multiple bus leads to 
  bad
  design.
 
  If the PHY driver has to send notification on connect/disconnect, the 
  PHY
  driver should make use of the extcon framework. Using this susbsystem
  to use extcon framwork will have to be analysed.
 
  You can find this patch series @
  git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git 
  testing
 
  Looks like there are not further comments on this series. Can you take 
  this in
  your misc tree?
 
  Do you want me to queue these for you ? There are quite a few users for
  this framework already and I know of at least 2 others which will show
  up for v3.13.
 
  Can you queue this patch series? There are quite a few users already for 
  this
  framework.
 
  It will have to wait for 3.13 as the merge window for new features has
  been closed for a week or so.  Sorry, I'll queue this up after 3.12-rc1
  is out.
 
  Alright, thanks.
  
  Just a gentle ping on this one...
 
 Let me know if you want me to rebase this patch series on the latest mainline 
 HEAD.

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

Re: [PATCHv4] ARM: OMAP2+: am335x-bone*: add DT for BeagleBone Black

[Greg KH]

On Mon, Sep 09, 2013 at 03:45:50PM +0200, Koen Kooi wrote:
 The BeagleBone Black is basically a regular BeagleBone with eMMC and HDMI 
 added,
 so create a common dtsi both can use.
 
 IMPORTANT: booting the existing am335x-bone.dts will blow up the HDMI 
 transceiver 
 after a dozen boots with an uSD card inserted because LDO will be at 3.3V 
 instead
 of 1.8. 
 
 MMC support for AM335x still isn't in, so only the LDO change has been added.
 
 Signed-off-by: Koen Kooi k...@dominion.thruhere.net
 Tested-by: Tom Rini tr...@ti.com
 Tested-by: Matt Porter matt.por...@linaro.org
 ---

formletter

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

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

Re: [PATCH v11 0/8] PHY framework

[Greg KH]

On Tue, Sep 03, 2013 at 08:55:23PM +0530, Kishon Vijay Abraham I wrote:
 Hi Greg,
 
 On Wednesday 28 August 2013 12:50 AM, Felipe Balbi wrote:
  Hi,
  
  On Mon, Aug 26, 2013 at 01:44:49PM +0530, Kishon Vijay Abraham I wrote:
  On Wednesday 21 August 2013 11:16 AM, Kishon Vijay Abraham I wrote:
  Added a generic PHY framework that provides a set of APIs for the PHY 
  drivers
  to create/destroy a PHY and APIs for the PHY users to obtain a reference 
  to
  the PHY with or without using phandle.
 
  This framework will be of use only to devices that uses external PHY (PHY
  functionality is not embedded within the controller).
 
  The intention of creating this framework is to bring the phy drivers 
  spread
  all over the Linux kernel to drivers/phy to increase code re-use and to
  increase code maintainability.
 
  Comments to make PHY as bus wasn't done because PHY devices can be part of
  other bus and making a same device attached to multiple bus leads to bad
  design.
 
  If the PHY driver has to send notification on connect/disconnect, the PHY
  driver should make use of the extcon framework. Using this susbsystem
  to use extcon framwork will have to be analysed.
 
  You can find this patch series @
  git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git testing
 
  Looks like there are not further comments on this series. Can you take 
  this in
  your misc tree?
  
  Do you want me to queue these for you ? There are quite a few users for
  this framework already and I know of at least 2 others which will show
  up for v3.13.
 
 Can you queue this patch series? There are quite a few users already for this
 framework.

It will have to wait for 3.13 as the merge window for new features has
been closed for a week or so.  Sorry, I'll queue this up after 3.12-rc1
is out.

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

Re: [PATCH] serial: omap: Fix IRQ handling return value

[Greg KH]

On Tue, Aug 27, 2013 at 07:30:19AM -0700, Kevin Hilman wrote:
 Greg,
 
 
 On Tue, Aug 20, 2013 at 8:44 AM, Kevin Hilman khil...@linaro.org wrote:
  +Felipe
 
  On Wed, Jul 17, 2013 at 6:29 AM, Alexander Savchenko
  oleksandr.savche...@ti.com wrote:
  From: Ruchika Kharwar ruch...@ti.com
 
  Ensure the Interrupt handling routine return IRQ_HANDLED vs
  IRQ_NONE.
 
  Why?
 
  By unconditionally returning IRQ_HANDLED, this patch will surely break
  systems where the UART IRQ is shared with other platforms.
 
  I just noticed this patch when bisecting a related problem.  Why
  wasn't this Cc'd to linux-omap where OMAP users might have been more
  likely to see it?
 
  Greg, without a better justification in the changelog, I think this
  patch should be dropped from tty-next.
 
 Can you drop this from tty-next please?
 
 The authors aren't responding (one of the ti.com addresses bounced)
 and this fix is most is not correct.

Yes, sorry, behind on my pending tty patch queue.  I'll try to get to it
this week.

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

Re: [PATCH v2] serial: OMAP: add RS485 support

[Greg KH]

On Tue, Aug 13, 2013 at 12:54:47PM +0200, Javier Martinez Canillas wrote:
 On Tue, Aug 13, 2013 at 12:22 PM, Mark Jackson mpfj-l...@newflow.co.uk 
 wrote:
  On 12/08/13 23:56, Greg KH wrote:
  On Sun, Aug 11, 2013 at 02:56:50PM +0100, Mark Jackson wrote:
  This patch adds RS485 support to the OMAP serial driver, as
  defined in:-
 
  Documentation/devicetree/bindings/serial/rs485.txt
 
  When a UART transmitter is connected to (eg) a RS485 driver, it is
  necessary to turn the driver on/off as quickly as possible.  This is
  best achieved in the serial driver itself (rather than in userspace
  where the latency can be quite large).
 
  This patch allows a GPIO pin to be defined (via DT) that controls
  the enabling of the driver at the start of a message, and disables
  the driver when the message has been completed.
 
  When RS485 is disabled, the RTS pin is set to on.
 
  Signed-off-by: Mark Jackson m...@newflow.co.uk
  ---
  Changes in v2:
  - Fix incorrect logic in serial_omap_config_rs485()
 
   drivers/tty/serial/omap-serial.c |  178 
  ++
   1 file changed, 178 insertions(+)
 
  This doesn't apply to my tty-next branch:
checking file drivers/tty/serial/omap-serial.c
Hunk #1 FAILED at 40.
Hunk #2 succeeded at 162 (offset 6 lines).
Hunk #3 succeeded at 280 (offset 5 lines).
Hunk #4 succeeded at 378 (offset 6 lines).
Hunk #5 succeeded at 1312 (offset 8 lines).
Hunk #6 succeeded at 1405 (offset 8 lines).
Hunk #7 succeeded at 1528 (offset 10 lines).
Hunk #8 FAILED at 1638.
Hunk #9 succeeded at 1705 (offset 6 lines).
2 out of 9 hunks FAILED
 
  so I can't apply it, sorry.
 
  It was applied on top of Linus' tree, if that makes any difference.
 
  I'll rebase onto yours and re-post the patch.
  --
 
 Hi Mark,
 
 I've seen several attempts to add RS485 support to the omap serial
 driver and it is always nack-ed. There seems to be concerns about
 controlling the RTS by software when RS485 is not supported by the
 UART hardware. Please refer to [1] and [2] for more information.

Hm, but we do support RS485 now in the kernel, look at
Documentation/serial/serial-rs485.txt.  Why can't this driver support it
as well if the serial core already does?

thanks,

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

Re: [PATCH v2] serial: OMAP: add RS485 support

[Greg KH]

On Tue, Aug 13, 2013 at 12:07:01PM +0100, Mark Jackson wrote:
 On 13/08/13 11:54, Javier Martinez Canillas wrote:
 
 snip
 
  
  Hi Mark,
  
  I've seen several attempts to add RS485 support to the omap serial
  driver and it is always nack-ed. There seems to be concerns about
  controlling the RTS by software when RS485 is not supported by the
  UART hardware. Please refer to [1] and [2] for more information.
 
 H ... okay, I guess I'll just have to maintain our own code (as
 did the OP in [1]).

I don't want to have that happen, that's just foolish.  I'd rather merge
it so that everyone can benifit, and so you don't have to spend extra
work maintaining this out-of-tree.

thanks,

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

Re: [PATCH v2] serial: OMAP: add RS485 support

[Greg KH]

On Tue, Aug 13, 2013 at 11:22:54AM +0100, Mark Jackson wrote:
 On 12/08/13 23:56, Greg KH wrote:
  On Sun, Aug 11, 2013 at 02:56:50PM +0100, Mark Jackson wrote:
  This patch adds RS485 support to the OMAP serial driver, as
  defined in:-
 
  Documentation/devicetree/bindings/serial/rs485.txt
 
  When a UART transmitter is connected to (eg) a RS485 driver, it is
  necessary to turn the driver on/off as quickly as possible.  This is
  best achieved in the serial driver itself (rather than in userspace
  where the latency can be quite large).
 
  This patch allows a GPIO pin to be defined (via DT) that controls
  the enabling of the driver at the start of a message, and disables
  the driver when the message has been completed.
 
  When RS485 is disabled, the RTS pin is set to on.
 
  Signed-off-by: Mark Jackson m...@newflow.co.uk
  ---
  Changes in v2:
  - Fix incorrect logic in serial_omap_config_rs485()
 
   drivers/tty/serial/omap-serial.c |  178 
  ++
   1 file changed, 178 insertions(+)
  
  This doesn't apply to my tty-next branch:
  checking file drivers/tty/serial/omap-serial.c
  Hunk #1 FAILED at 40.
  Hunk #2 succeeded at 162 (offset 6 lines).
  Hunk #3 succeeded at 280 (offset 5 lines).
  Hunk #4 succeeded at 378 (offset 6 lines).
  Hunk #5 succeeded at 1312 (offset 8 lines).
  Hunk #6 succeeded at 1405 (offset 8 lines).
  Hunk #7 succeeded at 1528 (offset 10 lines).
  Hunk #8 FAILED at 1638.
  Hunk #9 succeeded at 1705 (offset 6 lines).
  2 out of 9 hunks FAILED
  
  so I can't apply it, sorry.
 
 It was applied on top of Linus' tree, if that makes any difference.

Ah, that makes sense.  I should have tried a --3way merge to see if it
worked, sorry about that.  Let me go try it again...

Nope, a few merge errors that I don't know how to resolve, sorry.

 I'll rebase onto yours and re-post the patch.

That would be great, please do.

thanks,

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

Re: [PATCH v2] serial: OMAP: add RS485 support

[Greg KH]

On Sun, Aug 11, 2013 at 02:56:50PM +0100, Mark Jackson wrote:
 This patch adds RS485 support to the OMAP serial driver, as
 defined in:-
 
 Documentation/devicetree/bindings/serial/rs485.txt
 
 When a UART transmitter is connected to (eg) a RS485 driver, it is
 necessary to turn the driver on/off as quickly as possible.  This is
 best achieved in the serial driver itself (rather than in userspace
 where the latency can be quite large).
 
 This patch allows a GPIO pin to be defined (via DT) that controls
 the enabling of the driver at the start of a message, and disables
 the driver when the message has been completed.
 
 When RS485 is disabled, the RTS pin is set to on.
 
 Signed-off-by: Mark Jackson m...@newflow.co.uk
 ---
 Changes in v2:
 - Fix incorrect logic in serial_omap_config_rs485()
 
  drivers/tty/serial/omap-serial.c |  178 
 ++
  1 file changed, 178 insertions(+)

This doesn't apply to my tty-next branch:
checking file drivers/tty/serial/omap-serial.c
Hunk #1 FAILED at 40.
Hunk #2 succeeded at 162 (offset 6 lines).
Hunk #3 succeeded at 280 (offset 5 lines).
Hunk #4 succeeded at 378 (offset 6 lines).
Hunk #5 succeeded at 1312 (offset 8 lines).
Hunk #6 succeeded at 1405 (offset 8 lines).
Hunk #7 succeeded at 1528 (offset 10 lines).
Hunk #8 FAILED at 1638.
Hunk #9 succeeded at 1705 (offset 6 lines).
2 out of 9 hunks FAILED

so I can't apply it, sorry.

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

Re: Reg: USB: ehci-omap: Suspend the controller during idle.

[Greg KH]

On Tue, Aug 06, 2013 at 06:25:32PM +0530, Bharathraj Nagaraju wrote:
   Dear All,
 
   We are working on omap4470 based device,kernel-3.0.31 is running on this.

Have you tried the 3.10 kernel for this?  I think you might find that
this already is resolved there, as lots of pm USB work has happened in
the past 2+ years since 3.0 was released.

thanks,

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

Re: [RESEND PATCH v10 1/8] drivers: phy: add generic PHY framework

[Greg KH]

On Fri, Jul 26, 2013 at 06:19:16PM +0530, Kishon Vijay Abraham I wrote:
 +static int phy_get_id(void)
 +{
 + int ret;
 + int id;
 +
 + ret = ida_pre_get(phy_ida, GFP_KERNEL);
 + if (!ret)
 + return -ENOMEM;
 +
 + ret = ida_get_new(phy_ida, id);
 + if (ret  0)
 + return ret;
 +
 + return id;
 +}

ida_simple_get() instead?  And if you do that, you can get rid of this
function entirely.

thanks,

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

Re: [PATCH 2/2] arm: omap: remove *.auto* from device names given in usb_bind_phy

[Greg KH]

On Tue, Jul 30, 2013 at 11:15:20AM +0300, Felipe Balbi wrote:
   look at Greg's and my reply to that email.
  
  but finally Greg agreed to what Tomasz proposed no?
 
 that's not what I see in the thread. I see Greg agreed to regulator's
 own IDs being sequentially created, but he mentions device names can
 change.

And that no one should _ever_ rely on them to be a specific name, the
bus is responsible for creating the id, it could be random and
everything should work just fine.

thanks,

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 08:48:24PM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
  On Tue, 23 Jul 2013, Tomasz Figa wrote:
  
  On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
  Hi Alan,
  
  Thanks for helping to clarify the issues here.
  
  Okay.  Are PHYs _always_ platform devices?
 
  They can be i2c, spi or any other device types as well.
  
  In those other cases, presumably there is no platform data associated
  with the PHY since it isn't a platform device.  Then how does the
  kernel know which controller is attached to the PHY?  Is this spelled
  out in platform data associated with the PHY's i2c/spi/whatever parent?
 
 Yes. I think we could use i2c_board_info for passing platform data.
  
 PHY.  Currently this information is represented by name or 
  ID
 strings embedded in platform data.
 
  right. It's embedded in the platform data of the controller.
 
  It must also be embedded in the PHY's platform data somehow.
  Otherwise, how would the kernel know which PHY to use?
 
  By using a PHY lookup as Stephen and I suggested in our previous
  replies. Without any extra data in platform data. (I have even posted a
  code example.)
  
  I don't understand, because I don't know what a PHY lookup does.
 
 It is how the PHY framework finds a PHY, when the controller (say USB)requests
 a PHY from the PHY framework.
  
  In this case, it doesn't matter where the platform_device structures
  are created or where the driver source code is.  Let's take a simple
  example.  Suppose the system design includes a PHY named foo.  Then
  the board file could contain:
 
  struct phy_info { ... } phy_foo;
  EXPORT_SYMBOL_GPL(phy_foo);
 
  and a header file would contain:
 
  extern struct phy_info phy_foo;
 
  The PHY supplier could then call phy_create(phy_foo), and the PHY
  client could call phy_find(phy_foo).  Or something like that; make up
  your own structure tags and function names.
 
  It's still possible to have conflicts, but now two PHYs with the same
  name (or a misspelled name somewhere) will cause an error at link
  time.
 
  This is incorrect, sorry. First of all it's a layering violation - you
  export random driver-specific symbols from one driver to another. Then
  
  No, that's not what I said.  Neither the PHY driver nor the controller
  driver exports anything to the other.  Instead, both drivers use data
  exported by the board file.
 
 I think instead we can use the same data while creating the platform data of
 the controller and the PHY.
 The PHY driver while creating the PHY (using PHY framework) will also pass the
 *data* it actually got from the platform data to the framework.
 The PHY user driver (USB), while requesting for the PHY (from the PHY
 framework) will pass the *data* it got from its platform data.
 The PHY framework can do a comparison of the *data* pointers it has and return
 the appropriate PHY to the controller.
  
  imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and
  there are two types of consumer drivers (e.g. USB host controllers). Now
  consider following mapping:
 
  SoC   PHY consumer
  A PHY1HOST1
  B PHY1HOST2
  C PHY2HOST1
  D PHY2HOST2
 
  So we have to be able to use any of the PHYs with any of the host
  drivers. This means you would have to export symbol with the same name
  from both PHY drivers, which obviously would not work in this case,
  because having both drivers enabled (in a multiplatform aware
  configuration) would lead to linking conflict.
  
  You're right; the scheme was too simple.  Instead, the board file must
  export two types of data structures, one for PHYs and one for
  controllers.  Like this:
  
  struct phy_info {
  /* Info for the controller attached to this PHY */
  struct controller_info  *hinfo;
  };
  
  struct controller_info {
  /* Info for the PHY which this controller is attached to */
  struct phy_info *pinfo;
  };
  
  The board file for SoC A would contain:
  
  struct phy_info phy1 = {host1);
  EXPORT_SYMBOL(phy1);
  struct controller_info host1 = {phy1};
  EXPORT_SYMBOL(host1);
  
  The board file for SoC B would contain:
  
  struct phy_info phy1 = {host2);
  EXPORT_SYMBOL(phy1);
  struct controller_info host2 = {phy1};
  EXPORT_SYMBOL(host2);
 
 I meant something like this
 struct phy_info {
   const char *name;
 };
 
 struct phy_platform_data {
   .
   .
   struct phy_info *info;
 };
 
 struct usb_controller_platform_data {
   .
   .
   struct phy_info *info;
 };
 
 struct phy_info phy_info;
 
 While creating the phy device
   struct phy_platform_data phy_data;
   phy_data.info = info;
   platform_device_add_data(pdev, phy_data, sizeof(*phy_data))
   platform_device_add();
 
 While creating the controller device
   struct usb_controller_platform_data controller_data;
   controller_data.info = info;
   

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 09:58:34PM +0530, Kishon Vijay Abraham I wrote:
 Hi Greg,
 
 On Tuesday 23 July 2013 09:48 PM, Greg KH wrote:
  On Tue, Jul 23, 2013 at 08:48:24PM +0530, Kishon Vijay Abraham I wrote:
  Hi,
 
  On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
  On Tue, 23 Jul 2013, Tomasz Figa wrote:
 
  On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
  Hi Alan,
 
  Thanks for helping to clarify the issues here.
 
  Okay.  Are PHYs _always_ platform devices?
 
  They can be i2c, spi or any other device types as well.
 
  In those other cases, presumably there is no platform data associated
  with the PHY since it isn't a platform device.  Then how does the
  kernel know which controller is attached to the PHY?  Is this spelled
  out in platform data associated with the PHY's i2c/spi/whatever parent?
 .
 .
 snip
 .
 .
 
 static struct phy *phy_lookup(void *priv) {
 .
 .
 if (phy-priv==priv) //instead of string comparison, we'll use 
  pointer
 return phy;
 }
 
  PHY driver should be like
 phy_create((dev, ops, pdata-info);
 
  The controller driver would do
 phy_get(dev, NULL, pdata-info);
 
  Now the PHY framework will check for a match of *priv* pointer and return 
  the PHY.
 
  I think this should be possible?
  
  Ick, no.  Why can't you just pass the pointer to the phy itself?  If you
  had a priv pointer to search from, then you could have just passed the
  original phy pointer in the first place, right?
  
  The issue is that a string name is not going to scale at all, as it
  requires hard-coded information that will change over time (as the
  existing clock interface is already showing.)
  
  Please just pass the real phy pointer around, that's what it is there
  for.  Your board binding logic/code should be able to handle this, as
  it somehow was going to do the same thing with a name.
 
 The problem is the board file won't have the *phy* pointer. *phy* pointer is
 created at a much later time when the phy driver is probed.

Ok, then save it then, as no one could have used it before then, right?

All I don't want to see is any get by name/void * functions in the
api, as that way is fragile and will break, as people have already
shown.

Just pass the real pointer around.  If that is somehow a problem, then
something larger is a problem with how board devices are tied together :)

thanks,

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
  Ick, no.  Why can't you just pass the pointer to the phy itself?  If you
  had a priv pointer to search from, then you could have just passed the
  original phy pointer in the first place, right?
 
 IMHO it would be better if you provided some code example, but let's try to 
 check if I understood you correctly.

It's not my code that I want to have added, so I don't have to write
examples, I just get to complain about the existing stuff :)

 8
 
 [Board file]
 
 static struct phy my_phy;
 
 static struct platform_device phy_pdev = {
   /* ... */
   .platform_data = my_phy;
   /* ... */
 };
 
 static struct platform_device phy_pdev = {
   /* ... */
   .platform_data = my_phy;
   /* ... */
 };
 
 [Provider driver]
 
 struct phy *phy = pdev-dev.platform_data;
 
 ret = phy_create(phy);
 
 [Consumer driver]
 
 struct phy *phy = pdev-dev.platform_data;
 
 ret = phy_get(pdev-dev, phy);
 
 8
 
 Is this what you mean?

No.  Well, kind of.  What's wrong with using the platform data structure
unique to the board to have the pointer?

For example (just randomly picking one), the ata-pxa driver would change
include/linux/platform_data/ata-pxa.h to have a phy pointer in it:

struct phy;

struct  pata_pxa_pdata {
/* PXA DMA DREQ0:2 pin */
uint32_tdma_dreq;
/* Register shift */
uint32_treg_shift;
/* IRQ flags */
uint32_tirq_flags;
/* PHY */
struct phy  *phy;
};

Then, when you create the platform, set the phy* pointer with a call to
phy_create().  Then you can use that pointer wherever that plaform data
is available (i.e. whereever platform_data is at).

  The issue is that a string name is not going to scale at all, as it
  requires hard-coded information that will change over time (as the
  existing clock interface is already showing.)
 
 I fully agree that a simple, single string will not scale even in some, not 
 so uncommon cases, but there is already a lot of existing lookup solutions 
 over the kernel and so there is no point in introducing another one.

I'm trying to get _rid_ of lookup solutions and just use a real
pointer, as you should.  I'll go tackle those other ones after this one
is taken care of, to show how the others should be handled as well.

  Please just pass the real phy pointer around, that's what it is there
  for.  Your board binding logic/code should be able to handle this, as
  it somehow was going to do the same thing with a name.
 
 It's technically correct, but quality of this solution isn't really nice, 
 because it's a layering violation (at least if I understood what you mean). 
 This is because you need to have full definition of struct phy in board file 
 and a structure that is used as private data in PHY core comes from 
 platform code.

No, just a pointer, you don't need the full structure until you get to
some .c code that actually manipulates the phy itself, for all other
places, you are just dealing with a pointer and a structure you never
reference.

Does that make more sense?

thanks,

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 06:44:56PM +0100, Mark Brown wrote:
 On Tue, Jul 23, 2013 at 10:37:11AM -0700, Greg KH wrote:
  On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
 
   I fully agree that a simple, single string will not scale even in some, 
   not 
   so uncommon cases, but there is already a lot of existing lookup 
   solutions 
   over the kernel and so there is no point in introducing another one.
 
  I'm trying to get _rid_ of lookup solutions and just use a real
  pointer, as you should.  I'll go tackle those other ones after this one
  is taken care of, to show how the others should be handled as well.
 
 What are the problems you are seeing with doing things with lookups?

You don't know the id of the device you are looking up, due to
multiple devices being in the system (dynamic ids, look back earlier in
this thread for details about that.)

 Having to write platform data for everything gets old fast and the code
 duplication is pretty tedious...

Adding a single pointer is tedious?  Where is the name that you are
going to lookup going to come from?  That code doesn't write itself...

thanks,

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 07:48:11PM +0200, Tomasz Figa wrote:
 On Tuesday 23 of July 2013 10:37:11 Greg KH wrote:
  On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
Ick, no.  Why can't you just pass the pointer to the phy itself?  If
you
had a priv pointer to search from, then you could have just passed
the
original phy pointer in the first place, right?
   
   IMHO it would be better if you provided some code example, but let's
   try to check if I understood you correctly.
  
  It's not my code that I want to have added, so I don't have to write
  examples, I just get to complain about the existing stuff :)
 
 Still, I think that some small code snippets illustrating the idea are 
 really helpful.
 
   8
   
   
   [Board file]
   
   static struct phy my_phy;
   
   static struct platform_device phy_pdev = {
   
 /* ... */
 .platform_data = my_phy;
 /* ... */
   
   };
   
   static struct platform_device phy_pdev = {
   
 /* ... */
 .platform_data = my_phy;
 /* ... */
   
   };
   
   [Provider driver]
   
   struct phy *phy = pdev-dev.platform_data;
   
   ret = phy_create(phy);
   
   [Consumer driver]
   
   struct phy *phy = pdev-dev.platform_data;
   
   ret = phy_get(pdev-dev, phy);
   
   ---
   -8
   
   Is this what you mean?
  
  No.  Well, kind of.  What's wrong with using the platform data structure
  unique to the board to have the pointer?
  
  For example (just randomly picking one), the ata-pxa driver would change
  include/linux/platform_data/ata-pxa.h to have a phy pointer in it:
  
  struct phy;
  
  struct  pata_pxa_pdata {
  /* PXA DMA DREQ0:2 pin */
  uint32_tdma_dreq;
  /* Register shift */
  uint32_treg_shift;
  /* IRQ flags */
  uint32_tirq_flags;
  /* PHY */
  struct phy  *phy;
  };
  
  Then, when you create the platform, set the phy* pointer with a call to
  phy_create().  Then you can use that pointer wherever that plaform data
  is available (i.e. whereever platform_data is at).
 
 Hmm? So, do you suggest to call phy_create() from board file? What phy_ops 
 struct and other hardware parameters would it take?
 
The issue is that a string name is not going to scale at all, as it
requires hard-coded information that will change over time (as the
existing clock interface is already showing.)
   
   I fully agree that a simple, single string will not scale even in some,
   not so uncommon cases, but there is already a lot of existing lookup
   solutions over the kernel and so there is no point in introducing
   another one.
  I'm trying to get _rid_ of lookup solutions and just use a real
  pointer, as you should.  I'll go tackle those other ones after this one
  is taken care of, to show how the others should be handled as well.
 
 There was a reason for introducing lookup solutions. The reason was that in 
 board file there is no way to get a pointer to something that is going to be 
 created much later in time. We don't do time travel ;-).
 
Please just pass the real phy pointer around, that's what it is
there
for.  Your board binding logic/code should be able to handle this,
as
it somehow was going to do the same thing with a name.
   
   It's technically correct, but quality of this solution isn't really
   nice, because it's a layering violation (at least if I understood what
   you mean). This is because you need to have full definition of struct
   phy in board file and a structure that is used as private data in PHY
   core comes from platform code.
  
  No, just a pointer, you don't need the full structure until you get to
  some .c code that actually manipulates the phy itself, for all other
  places, you are just dealing with a pointer and a structure you never
  reference.
  
  Does that make more sense?
 
 Well, to the point that I think I now understood your suggestion. 
 Unfortunately the suggestion alone isn't really something that can be done, 
 considering how driver core and generic frameworks work.

Ok, given that I seem to be totally confused as to exactly how the
board-specific frameworks work, I'll take your word for it.

But again, I will not accept lookup by name type solutions, when the
name is dynamic and will change.  Because you are using a name, you
can deal with a pointer, putting it _somewhere_ in your board-specific
data structures, as you are going to need to store it anyway (hint, you
had to get that name from somewhere, right?)

And maybe the way that these generic frameworks are created is wrong,
given that you don't feel that a generic pointer can be passed to the
needed devices.  That seems like a huge problem, one that has already
been pointed out is causing issues with other subsystems.

So maybe they need to be fixed?

thanks,

greg k-h

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote:
  You don't know the id of the device you are looking up, due to
  multiple devices being in the system (dynamic ids, look back earlier in
  this thread for details about that.)
 
 I got copied in very late so don't have most of the thread I'm afraid, 
 I did try looking at web archives but didn't see a clear problem
 statement.  In any case this is why the APIs doing lookups do the
 lookups in the context of the requesting device - devices ask for
 whatever name they use locally.

What do you mean by locally?

The problem with the api was that the phy core wanted a id and a name to
create a phy, and then later other code was doing a lookup based on
the name and id (mushed together), because it knew that this device
was the one it wanted.

Just like the clock api, which, for multiple devices, has proven to
cause problems.  I don't want to see us accept an api that we know has
issues in it now, I'd rather us fix it up properly.

Subsystems should be able to create ids how ever they want to, and not
rely on the code calling them to specify the names of the devices that
way, otherwise the api is just too fragile.

I think, that if you create a device, then just carry around the pointer
to that device (in this case a phy) and pass it to whatever other code
needs it.  No need to do lookups on known names or anything else, just
normal pointers, with no problems for multiple devices, busses, or
naming issues.

   Having to write platform data for everything gets old fast and the code
   duplication is pretty tedious...
 
  Adding a single pointer is tedious?  Where is the name that you are
  going to lookup going to come from?  That code doesn't write itself...
 
 It's adding platform data in the first place that gets tedious - and of
 course there's also DT and ACPI to worry about, it's not just a case of
 platform data and then you're done.  Pushing the lookup into library
 code means that drivers don't have to worry about any of this stuff.

I agree, so just pass around the pointer to the phy and all is good.  No
need to worry about DT or ACPI or anything else.

 For most of the APIs doing this there is a clear and unambiguous name in
 the hardware that can be used (and for hardware process reasons is
 unlikely to get changed).  The major exception to this is the clock API
 since it is relatively rare to have clear, segregated IP level
 information for IPs baked into larger chips.  The other APIs tend to be
 establishing chip to chip links.

The clock api is having problems with multiple names due to dynamic
devices from what I was told.  I want to prevent the PHY interface from
having that same issue.

thanks,

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 10:07:52PM +0200, Tomasz Figa wrote:
 On Tuesday 23 of July 2013 12:44:23 Greg KH wrote:
  On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote:
You don't know the id of the device you are looking up, due to
multiple devices being in the system (dynamic ids, look back earlier
in
this thread for details about that.)
   
   I got copied in very late so don't have most of the thread I'm afraid,
   I did try looking at web archives but didn't see a clear problem
   statement.  In any case this is why the APIs doing lookups do the
   lookups in the context of the requesting device - devices ask for
   whatever name they use locally.
  
  What do you mean by locally?
  
  The problem with the api was that the phy core wanted a id and a name to
  create a phy, and then later other code was doing a lookup based on
  the name and id (mushed together), because it knew that this device
  was the one it wanted.
  
  Just like the clock api, which, for multiple devices, has proven to
  cause problems.  I don't want to see us accept an api that we know has
  issues in it now, I'd rather us fix it up properly.
  
  Subsystems should be able to create ids how ever they want to, and not
  rely on the code calling them to specify the names of the devices that
  way, otherwise the api is just too fragile.
  
  I think, that if you create a device, then just carry around the pointer
  to that device (in this case a phy) and pass it to whatever other code
  needs it.  No need to do lookups on known names or anything else,
  just normal pointers, with no problems for multiple devices, busses, or
  naming issues.
 
 PHY object is not a device, it is something that a device driver creates 
 (one or more instances of) when it is being probed.

But you created a 'struct device' for it, so I think of it as a device
be it virtual or real :)

 You don't have a clean way to export this PHY object to other driver,
 other than keeping this PHY on a list inside PHY core with some
 well-known ID (e.g. device name + consumer port name/index, like in
 regulator core) and then to use this well-known ID inside consumer
 driver as a lookup key passed to phy_get();
 
 Actually I think for PHY case, exactly the same way as used for
 regulators might be completely fine:
 
 1. Each PHY would have some kind of platform, non-unique name, that is 
 just used to print some messages (like the platform/board name of a 
 regulator).
 2. Each PHY would have an array of consumers. Consumer specifier would 
 consist of consumer device name and consumer port name - just like in 
 regulator subsystem.
 3. PHY driver receives an array of, let's say, phy_init_data inside its 
 platform data that it would use to register its PHYs.
 4. Consumer drivers would have constant consumer port names and wouldn't 
 receive any information about PHYs from platform code.
 
 Code example:
 
 [Board file]
 
 static const struct phy_consumer_data usb_20_phy0_consumers[] = {
   {
   .devname = foo-ehci,
   .port = usbphy,
   },
 };
 
 static const struct phy_consumer_data usb_20_phy1_consumers[] = {
   {
   .devname = foo-otg,
   .port = otgphy,
   },
 };
 
 static const struct phy_init_data my_phys[] = {
   {
   .name = USB 2.0 PHY 0,
   .consumers = usb_20_phy0_consumers,
   .num_consumers = ARRAY_SIZE(usb_20_phy0_consumers),
   },
   {
   .name = USB 2.0 PHY 1,
   .consumers = usb_20_phy1_consumers,
   .num_consumers = ARRAY_SIZE(usb_20_phy1_consumers),
   },
   { }
 };
 
 static const struct platform_device usb_phy_pdev = {
   .name = foo-usbphy,
   .id = -1,
   .dev = {
   .platform_data = my_phys,
   },
 };
 
 [PHY driver]
 
 static int foo_usbphy_probe(pdev)
 {
   struct foo_usbphy *foo;
   struct phy_init_data *init_data = pdev-dev.platform_data;
   /* ... */
   // for each PHY in init_data {
   phy_register(foo-phy[i], init_data[i]);
   // }
   /* ... */
 }
 
 [EHCI driver]
 
 static int foo_ehci_probe(pdev)
 {
   struct phy *phy;
   /* ... */
   phy = phy_get(pdev-dev, usbphy);
   /* ... */
 }
 
 [OTG driver]
 
 static int foo_otg_probe(pdev)
 {
   struct phy *phy;
   /* ... */
   phy = phy_get(pdev-dev, otgphy);
   /* ... */
 }

That's not so bad, as long as you let the phy core use whatever name it
wants for the device when it registers it with sysfs.  Use the name you
are requesting as a tag or some such hint as to what the phy can be
looked up by.

Good luck handling duplicate tags :)

thanks,

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Tue, Jul 23, 2013 at 11:05:48PM +0200, Tomasz Figa wrote:
  That's not so bad, as long as you let the phy core use whatever name it
  wants for the device when it registers it with sysfs.
 
 Yes, in regulator core consumer names are completely separated from this. 
 Regulator core simply assigns a sequential integer ID to each regulator 
 and registers /sys/class/regulator/regulator.ID for each regulator.

Yes, that's fine.

  Use the name you
  are requesting as a tag or some such hint as to what the phy can be
  looked up by.
  
  Good luck handling duplicate tags :)
 
 The tag alone is not a key. Lookup key consists of two components, 
 consumer device name and consumer tag. What kind of duplicate tags can be 
 a problem here?

Ok, I didn't realize it looked at both parts, that makes sense, thanks.

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Mon, Jul 22, 2013 at 12:55:18PM +0530, Kishon Vijay Abraham I wrote:
  The issue (or one of the issues) in this discussion is that 
  Greg does not like the idea of using names or IDs to associate
  PHYs with controllers, because they are too prone to
  duplications or other errors.  Pointers are more reliable.
  
  But pointers to what?  Since the only data known to be 
  available to both the PHY driver and controller driver is the
  platform data, the obvious answer is a pointer to platform data
  (either for the PHY or for the controller, or maybe both).
 
 hmm.. it's not going to be simple though as the platform device for the PHY 
 and
 controller can be created in entirely different places. e.g., in some cases 
 the
 PHY device is a child of some mfd core device (the device will be created in
 drivers/mfd) and the controller driver (usually) is created in board file. I
 guess then we have to come up with something to share a pointer in two
 different files.

What's wrong with using the platform_data structure that is unique to
all boards (see include/platform_data/ for examples)?  Isn't that what
this structure is there for?

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Sun, Jul 21, 2013 at 01:12:07PM +0200, Tomasz Figa wrote:
 On Sunday 21 of July 2013 16:37:33 Kishon Vijay Abraham I wrote:
  Hi,
  
  On Sunday 21 July 2013 04:01 PM, Tomasz Figa wrote:
   Hi,
   
   On Saturday 20 of July 2013 19:59:10 Greg KH wrote:
   On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote:
   On Sat, 20 Jul 2013, Greg KH wrote:
   That should be passed using platform data.
   
   Ick, don't pass strings around, pass pointers.  If you have
   platform
   data you can get to, then put the pointer there, don't use a
   name.
   
   I don't think I understood you here :-s We wont have phy pointer
   when we create the device for the controller no?(it'll be done in
   board file). Probably I'm missing something.
   
   Why will you not have that pointer?  You can't rely on the name
   as
   the device id will not match up, so you should be able to rely on
   the pointer being in the structure that the board sets up, right?
   
   Don't use names, especially as ids can, and will, change, that is
   going
   to cause big problems.  Use pointers, this is C, we are supposed to
   be
   doing that :)
   
   Kishon, I think what Greg means is this:  The name you are using
   must
   be stored somewhere in a data structure constructed by the board
   file,
   right?  Or at least, associated with some data structure somehow.
   Otherwise the platform code wouldn't know which PHY hardware
   corresponded to a particular name.
   
   Greg's suggestion is that you store the address of that data
   structure
   in the platform data instead of storing the name string.  Have the
   consumer pass the data structure's address when it calls phy_create,
   instead of passing the name.  Then you don't have to worry about two
   PHYs accidentally ending up with the same name or any other similar
   problems.
   
   Close, but the issue is that whatever returns from phy_create()
   should
   then be used, no need to call any find functions, as you can just
   use
   the pointer that phy_create() returns.  Much like all other class api
   functions in the kernel work.
   
   I think there is a confusion here about who registers the PHYs.
   
   All platform code does is registering a platform/i2c/whatever device,
   which causes a driver (located in drivers/phy/) to be instantiated.
   Such drivers call phy_create(), usually in their probe() callbacks,
   so platform_code has no way (and should have no way, for the sake of
   layering) to get what phy_create() returns.

Why not put pointers in the platform data structure that can hold these
pointers?  I thought that is why we created those structures in the
first place.  If not, what are they there for?

   IMHO we need a lookup method for PHYs, just like for clocks,
   regulators, PWMs or even i2c busses because there are complex cases
   when passing just a name using platform data will not work. I would
   second what Stephen said [1] and define a structure doing things in a
   DT-like way.
   
   Example;
   
   [platform code]
   
   static const struct phy_lookup my_phy_lookup[] = {
   
 PHY_LOOKUP(s3c-hsotg.0, otg, samsung-usbphy.1, phy.2),
  
  The only problem here is that if *PLATFORM_DEVID_AUTO* is used while
  creating the device, the ids in the device name would change and
  PHY_LOOKUP wont be useful.
 
 I don't think this is a problem. All the existing lookup methods already 
 use ID to identify devices (see regulators, clkdev, PWMs, i2c, ...). You 
 can simply add a requirement that the ID must be assigned manually, 
 without using PLATFORM_DEVID_AUTO to use PHY lookup.

And I'm saying that this idea, of using a specific name and id, is
frought with fragility and will break in the future in various ways when
devices get added to systems, making these strings constantly have to be
kept up to date with different board configurations.

People, NEVER, hardcode something like an id.  The fact that this
happens today with the clock code, doesn't make it right, it makes the
clock code wrong.  Others have already said that this is wrong there as
well, as systems change and dynamic ids get used more and more.

Let's not repeat the same mistakes of the past just because we refuse to
learn from them...

So again, the find a phy by a string functions should be removed, the
device id should be automatically created by the phy core just to make
things unique in sysfs, and no driver code should _ever_ be reliant on
the number that is being created, and the pointer to the phy structure
should be used everywhere instead.

With those types of changes, I will consider merging this subsystem, but
without them, sorry, I will not.

thanks,

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Sun, Jul 21, 2013 at 12:22:48PM +0200, Sascha Hauer wrote:
 On Sat, Jul 20, 2013 at 07:59:10PM -0700, Greg KH wrote:
  On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote:
   On Sat, 20 Jul 2013, Greg KH wrote:
   
 That should be passed using platform data.
 
 Ick, don't pass strings around, pass pointers.  If you have platform
 data you can get to, then put the pointer there, don't use a name.
 
 I don't think I understood you here :-s We wont have phy pointer
 when we create the device for the controller no?(it'll be done in
 board file). Probably I'm missing something.

Why will you not have that pointer?  You can't rely on the name as the
device id will not match up, so you should be able to rely on the
pointer being in the structure that the board sets up, right?

Don't use names, especially as ids can, and will, change, that is going
to cause big problems.  Use pointers, this is C, we are supposed to be
doing that :)
   
   Kishon, I think what Greg means is this:  The name you are using must
   be stored somewhere in a data structure constructed by the board file,
   right?  Or at least, associated with some data structure somehow.  
   Otherwise the platform code wouldn't know which PHY hardware
   corresponded to a particular name.
   
   Greg's suggestion is that you store the address of that data structure 
   in the platform data instead of storing the name string.  Have the 
   consumer pass the data structure's address when it calls phy_create, 
   instead of passing the name.  Then you don't have to worry about two 
   PHYs accidentally ending up with the same name or any other similar 
   problems.
  
  Close, but the issue is that whatever returns from phy_create() should
  then be used, no need to call any find functions, as you can just use
  the pointer that phy_create() returns.  Much like all other class api
  functions in the kernel work.
 
 I think the problem here is to connect two from the bus structure
 completely independent devices. Several frameworks (ASoC, soc-camera)
 had this problem and this wasn't solved until the advent of devicetrees
 and their phandles.
 phy_create might be called from the probe function of some i2c device
 (the phy device) and the resulting pointer is then needed in some other
 platform devices (the user of the phy) probe function.
 The best solution we have right now is implemented in the clk framework
 which uses a string matching of the device names in clk_get() (at least
 in the non-dt case).

I would argue that clocks are wrong here as well, as others have already
pointed out.

What's wrong with the platform_data structure, why can't that be used
for this?

Or, if not, we can always add pointers to the platform device structure,
or even the main 'struct device' as well, that's what it is there for.

thanks,

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Sat, Jul 20, 2013 at 08:49:32AM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Saturday 20 July 2013 05:20 AM, Greg KH wrote:
 On Fri, Jul 19, 2013 at 12:06:01PM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Friday 19 July 2013 11:59 AM, Greg KH wrote:
 On Fri, Jul 19, 2013 at 11:25:44AM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Friday 19 July 2013 11:13 AM, Greg KH wrote:
 On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote:
 +   ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
 
 Your naming is odd, no phy anywhere in it?  You rely on the sender 
 to
 never send a duplicate name.id pair?  Why not create your own ids 
 based
 on the number of phys in the system, like almost all other classes 
 and
 subsystems do?
 
 hmm.. some PHY drivers use the id they provide to perform some of 
 their
 internal operation as in [1] (This is used only if a single PHY 
 provider
 implements multiple PHYS). Probably I'll add an option like 
 PLATFORM_DEVID_AUTO
 to give the PHY drivers an option to use auto id.
 
 [1] -
 http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html
 
 No, who cares about the id?  No one outside of the phy core ever 
 should,
 because you pass back the only pointer that they really do care about,
 if they need to do anything with the device.  Use that, and then you 
 can
 
 hmm.. ok.
 
 rip out all of the search for a phy by a string logic, as that's not
 
 Actually this is needed for non-dt boot case. In the case of dt boot, 
 we use a
 phandle by which the controller can get a reference to the phy. But in 
 the case
 of non-dt boot, the controller can get a reference to the phy only by 
 label.
 
 I don't understand.  They registered the phy, and got back a pointer to
 it.  Why can't they save it in their local structure to use it again
 later if needed?  They should never have to ask for the device, as the
 
 One is a *PHY provider* driver which is a driver for some PHY device. 
 This will
 use phy_create to create the phy.
 The other is a *PHY consumer* driver which might be any controller driver 
 (can
 be USB/SATA/PCIE). The PHY consumer will use phy_get to get a reference 
 to the
 phy (by *phandle* in the case of dt boot and *label* in the case of 
 non-dt boot).
 device id might be unknown if there are multiple devices in the system.
 
 I agree with you on the device id part. That need not be known to the PHY 
 driver.
 
 How does a consumer know which label to use in a non-dt system if
 there are multiple PHYs in the system?
 
 That should be passed using platform data.
 
 Ick, don't pass strings around, pass pointers.  If you have platform
 data you can get to, then put the pointer there, don't use a name.
 
 I don't think I understood you here :-s We wont have phy pointer
 when we create the device for the controller no?(it'll be done in
 board file). Probably I'm missing something.

Why will you not have that pointer?  You can't rely on the name as the
device id will not match up, so you should be able to rely on the
pointer being in the structure that the board sets up, right?

Don't use names, especially as ids can, and will, change, that is going
to cause big problems.  Use pointers, this is C, we are supposed to be
doing that :)

thanks,

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Sat, Jul 20, 2013 at 10:32:26PM -0400, Alan Stern wrote:
 On Sat, 20 Jul 2013, Greg KH wrote:
 
   That should be passed using platform data.
   
   Ick, don't pass strings around, pass pointers.  If you have platform
   data you can get to, then put the pointer there, don't use a name.
   
   I don't think I understood you here :-s We wont have phy pointer
   when we create the device for the controller no?(it'll be done in
   board file). Probably I'm missing something.
  
  Why will you not have that pointer?  You can't rely on the name as the
  device id will not match up, so you should be able to rely on the
  pointer being in the structure that the board sets up, right?
  
  Don't use names, especially as ids can, and will, change, that is going
  to cause big problems.  Use pointers, this is C, we are supposed to be
  doing that :)
 
 Kishon, I think what Greg means is this:  The name you are using must
 be stored somewhere in a data structure constructed by the board file,
 right?  Or at least, associated with some data structure somehow.  
 Otherwise the platform code wouldn't know which PHY hardware
 corresponded to a particular name.
 
 Greg's suggestion is that you store the address of that data structure 
 in the platform data instead of storing the name string.  Have the 
 consumer pass the data structure's address when it calls phy_create, 
 instead of passing the name.  Then you don't have to worry about two 
 PHYs accidentally ending up with the same name or any other similar 
 problems.

Close, but the issue is that whatever returns from phy_create() should
then be used, no need to call any find functions, as you can just use
the pointer that phy_create() returns.  Much like all other class api
functions in the kernel work.

thanks,

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Fri, Jul 19, 2013 at 11:25:44AM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Friday 19 July 2013 11:13 AM, Greg KH wrote:
  On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote:
  +  ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
 
  Your naming is odd, no phy anywhere in it?  You rely on the sender to
  never send a duplicate name.id pair?  Why not create your own ids based
  on the number of phys in the system, like almost all other classes and
  subsystems do?
 
  hmm.. some PHY drivers use the id they provide to perform some of their
  internal operation as in [1] (This is used only if a single PHY provider
  implements multiple PHYS). Probably I'll add an option like 
  PLATFORM_DEVID_AUTO
  to give the PHY drivers an option to use auto id.
 
  [1] -
  http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html
 
  No, who cares about the id?  No one outside of the phy core ever should,
  because you pass back the only pointer that they really do care about,
  if they need to do anything with the device.  Use that, and then you can
 
  hmm.. ok.
 
  rip out all of the search for a phy by a string logic, as that's not
 
  Actually this is needed for non-dt boot case. In the case of dt boot, we 
  use a
  phandle by which the controller can get a reference to the phy. But in the 
  case
  of non-dt boot, the controller can get a reference to the phy only by 
  label.
  
  I don't understand.  They registered the phy, and got back a pointer to
  it.  Why can't they save it in their local structure to use it again
  later if needed?  They should never have to ask for the device, as the
 
 One is a *PHY provider* driver which is a driver for some PHY device. This 
 will
 use phy_create to create the phy.
 The other is a *PHY consumer* driver which might be any controller driver (can
 be USB/SATA/PCIE). The PHY consumer will use phy_get to get a reference to the
 phy (by *phandle* in the case of dt boot and *label* in the case of non-dt 
 boot).
  device id might be unknown if there are multiple devices in the system.
 
 I agree with you on the device id part. That need not be known to the PHY 
 driver.

How does a consumer know which label to use in a non-dt system if
there are multiple PHYs in the system?

Do you have any drivers that are non-dt using this yet?

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Fri, Jul 19, 2013 at 12:06:01PM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Friday 19 July 2013 11:59 AM, Greg KH wrote:
  On Fri, Jul 19, 2013 at 11:25:44AM +0530, Kishon Vijay Abraham I wrote:
  Hi,
 
  On Friday 19 July 2013 11:13 AM, Greg KH wrote:
  On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote:
  +ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
 
  Your naming is odd, no phy anywhere in it?  You rely on the sender 
  to
  never send a duplicate name.id pair?  Why not create your own ids 
  based
  on the number of phys in the system, like almost all other classes and
  subsystems do?
 
  hmm.. some PHY drivers use the id they provide to perform some of their
  internal operation as in [1] (This is used only if a single PHY 
  provider
  implements multiple PHYS). Probably I'll add an option like 
  PLATFORM_DEVID_AUTO
  to give the PHY drivers an option to use auto id.
 
  [1] -
  http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html
 
  No, who cares about the id?  No one outside of the phy core ever should,
  because you pass back the only pointer that they really do care about,
  if they need to do anything with the device.  Use that, and then you can
 
  hmm.. ok.
 
  rip out all of the search for a phy by a string logic, as that's not
 
  Actually this is needed for non-dt boot case. In the case of dt boot, we 
  use a
  phandle by which the controller can get a reference to the phy. But in 
  the case
  of non-dt boot, the controller can get a reference to the phy only by 
  label.
 
  I don't understand.  They registered the phy, and got back a pointer to
  it.  Why can't they save it in their local structure to use it again
  later if needed?  They should never have to ask for the device, as the
 
  One is a *PHY provider* driver which is a driver for some PHY device. This 
  will
  use phy_create to create the phy.
  The other is a *PHY consumer* driver which might be any controller driver 
  (can
  be USB/SATA/PCIE). The PHY consumer will use phy_get to get a reference to 
  the
  phy (by *phandle* in the case of dt boot and *label* in the case of non-dt 
  boot).
  device id might be unknown if there are multiple devices in the system.
 
  I agree with you on the device id part. That need not be known to the PHY 
  driver.
  
  How does a consumer know which label to use in a non-dt system if
  there are multiple PHYs in the system?
 
 That should be passed using platform data.

Ick, don't pass strings around, pass pointers.  If you have platform
data you can get to, then put the pointer there, don't use a name.

thanks,

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

Re: [PATCH v9 1/8] drivers: phy: add generic PHY framework

[Greg KH]

On Thu, Jul 18, 2013 at 11:33:17AM +0530, Kishon Vijay Abraham I wrote:
  Wanted to group all the PHY drivers to be used by different subsystems
  (SATA/USB/PCIE/HDMI/VIDEO) into a single entity. There were some comments 
  in my
  initial version [3] on using a bus_type instead of class but then it was
  decided to go with class itself.
 
  [3] - http://lkml.indiana.edu/hypermail/linux/kernel/1302.2/01389.html
  
  Ok, but what does the class usage get you?
 
 hmm.. actually I use class only to iterate through the list of devices in 
 *phy*
 class which could very well be implemented using list. Just that I wont have a
 /sys/class/phy/ entry to find the list of phys added in the system. I dont
 think I want to add any other stuff to expose to the user space at this point
 of time.
  
  When modifying/adding new sysfs stuff, you need a Documentation/ABI/
  entry as well.
 
  I'm not actually adding any new sysfs entry other than what a 
  *class_create*
  must have created. Do I need to add one for that?
  
  If you are not creating anything in sysfs at all, why use the driver
  model?  (hint, I think you need to do something here to justify it...)
 
 Well.. it helps me to use pm_runtime to enable clocks utilizing the
 parent-child relationship.

Ok, that's a good reason for this, nevermind then.  Care to send the
latest patches you have in emails so I can review them?

thanks,

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Thu, Jul 18, 2013 at 12:16:10PM +0530, Kishon Vijay Abraham I wrote:
 +struct phy_provider *__of_phy_provider_register(struct device *dev,
 + struct module *owner, struct phy * (*of_xlate)(struct device *dev,
 + struct of_phandle_args *args));
 +struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
 + struct module *owner, struct phy * (*of_xlate)(struct device *dev,
 + struct of_phandle_args *args))
 +
 +__of_phy_provider_register and __devm_of_phy_provider_register can be used to
 +register the phy_provider and it takes device, owner and of_xlate as
 +arguments. For the dt boot case, all PHY providers should use one of the 
 above
 +2 APIs to register the PHY provider.

Why do you have __ for the prefix of a public function?  Is that really
the way that OF handles this type of thing?

 --- /dev/null
 +++ b/drivers/phy/Kconfig
 @@ -0,0 +1,13 @@
 +#
 +# PHY
 +#
 +
 +menuconfig GENERIC_PHY
 + tristate PHY Subsystem
 + help
 +   Generic PHY support.
 +
 +   This framework is designed to provide a generic interface for PHY
 +   devices present in the kernel. This layer will have the generic
 +   API by which phy drivers can create PHY using the phy framework and
 +   phy users can obtain reference to the PHY.

Again, please reverse this.  The drivers that use it should select it,
not depend on it, which will then enable this option.  I will never know
if I need to enable it, and based on your follow-on patches, if I don't,
drivers that were working just fine, now disappeared from my build,
which isn't nice, and a pain to notice and fix up.

 +/**
 + * phy_create() - create a new phy
 + * @dev: device that is creating the new phy
 + * @id: id of the phy
 + * @ops: function pointers for performing phy operations
 + * @label: label given to the phy
 + *
 + * Called to create a phy using phy framework.
 + */
 +struct phy *phy_create(struct device *dev, u8 id, const struct phy_ops *ops,
 + const char *label)
 +{
 + int ret;
 + struct phy *phy;
 +
 + if (!dev) {
 + dev_WARN(dev, no device provided for PHY\n);
 + ret = -EINVAL;
 + goto err0;
 + }
 +
 + phy = kzalloc(sizeof(*phy), GFP_KERNEL);
 + if (!phy) {
 + ret = -ENOMEM;
 + goto err0;
 + }
 +
 + device_initialize(phy-dev);
 + mutex_init(phy-mutex);
 +
 + phy-dev.class = phy_class;
 + phy-dev.parent = dev;
 + phy-dev.of_node = dev-of_node;
 + phy-id = id;
 + phy-ops = ops;
 + phy-label = kstrdup(label, GFP_KERNEL);
 +
 + ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);

Your naming is odd, no phy anywhere in it?  You rely on the sender to
never send a duplicate name.id pair?  Why not create your own ids based
on the number of phys in the system, like almost all other classes and
subsystems do?

 +static inline int phy_pm_runtime_get(struct phy *phy)
 +{
 + if (WARN(IS_ERR(phy), Invalid PHY reference\n))
 + return -EINVAL;

Why would phy ever not be valid and a error pointer?  And why dump the
stack if that happens, that seems really extreme.

 +
 + if (!pm_runtime_enabled(phy-dev))
 + return -ENOTSUPP;
 +
 + return pm_runtime_get(phy-dev);
 +}

This, and the other inline functions in this .h file seem huge, why are
they inline and not in the .c file?  There's no speed issues, and it
should save space overall in the .c file.  Please move them.


 +static inline int phy_init(struct phy *phy)
 +{
 + int ret;
 +
 + ret = phy_pm_runtime_get_sync(phy);
 + if (ret  0  ret != -ENOTSUPP)
 + return ret;
 +
 + mutex_lock(phy-mutex);
 + if (phy-init_count++ == 0  phy-ops-init) {
 + ret = phy-ops-init(phy);
 + if (ret  0) {
 + dev_err(phy-dev, phy init failed -- %d\n, ret);
 + goto out;
 + }
 + }
 +
 +out:
 + mutex_unlock(phy-mutex);
 + phy_pm_runtime_put(phy);
 + return ret;
 +}
 +
 +static inline int phy_exit(struct phy *phy)
 +{
 + int ret;
 +
 + ret = phy_pm_runtime_get_sync(phy);
 + if (ret  0  ret != -ENOTSUPP)
 + return ret;
 +
 + mutex_lock(phy-mutex);
 + if (--phy-init_count == 0  phy-ops-exit) {
 + ret = phy-ops-exit(phy);
 + if (ret  0) {
 + dev_err(phy-dev, phy exit failed -- %d\n, ret);
 + goto out;
 + }
 + }
 +
 +out:
 + mutex_unlock(phy-mutex);
 + phy_pm_runtime_put(phy);
 + return ret;
 +}
 +
 +static inline int phy_power_on(struct phy *phy)
 +{
 + int ret = -ENOTSUPP;
 +
 + ret = phy_pm_runtime_get_sync(phy);
 + if (ret  0  ret != -ENOTSUPP)
 + return ret;
 +
 + mutex_lock(phy-mutex);
 + if (phy-power_count++ == 0  phy-ops-power_on) {
 + ret = phy-ops-power_on(phy);
 + if (ret  0) {
 + dev_err(phy-dev, phy 

Re: [PATCH 02/15] usb: phy: omap-usb2: use the new generic PHY framework

[Greg KH]

On Thu, Jul 18, 2013 at 12:16:11PM +0530, Kishon Vijay Abraham I wrote:
 Used the generic PHY framework API to create the PHY. Now the power off and
 power on are done in omap_usb_power_off and omap_usb_power_on respectively.
 
 However using the old USB PHY library cannot be completely removed
 because OTG is intertwined with PHY and moving to the new framework
 will break OTG. Once we have a separate OTG state machine, we
 can get rid of the USB PHY library.
 
 Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
 Reviewed-by: Sylwester Nawrocki s.nawro...@samsung.com
 Acked-by: Felipe Balbi ba...@ti.com
 ---
  drivers/usb/phy/Kconfig |1 +
  drivers/usb/phy/phy-omap-usb2.c |   45 
 +++
  2 files changed, 42 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
 index 3622fff..cc55993 100644
 --- a/drivers/usb/phy/Kconfig
 +++ b/drivers/usb/phy/Kconfig
 @@ -75,6 +75,7 @@ config OMAP_CONTROL_USB
  config OMAP_USB2
   tristate OMAP USB2 PHY Driver
   depends on ARCH_OMAP2PLUS
 + depends on GENERIC_PHY
   select OMAP_CONTROL_USB
   help
 Enable this to support the transceiver that is part of SOC. This
 diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
 index 844ab68..751b30f 100644
 --- a/drivers/usb/phy/phy-omap-usb2.c
 +++ b/drivers/usb/phy/phy-omap-usb2.c
 @@ -28,6 +28,7 @@
  #include linux/pm_runtime.h
  #include linux/delay.h
  #include linux/usb/omap_control_usb.h
 +#include linux/phy/phy.h
  
  /**
   * omap_usb2_set_comparator - links the comparator present in the sytem with
 @@ -119,10 +120,36 @@ static int omap_usb2_suspend(struct usb_phy *x, int 
 suspend)
   return 0;
  }
  
 +static int omap_usb_power_off(struct phy *x)
 +{
 + struct omap_usb *phy = phy_get_drvdata(x);
 +
 + omap_control_usb_phy_power(phy-control_dev, 0);
 +
 + return 0;
 +}
 +
 +static int omap_usb_power_on(struct phy *x)
 +{
 + struct omap_usb *phy = phy_get_drvdata(x);
 +
 + omap_control_usb_phy_power(phy-control_dev, 1);
 +
 + return 0;
 +}
 +
 +static struct phy_ops ops = {
 + .power_on   = omap_usb_power_on,
 + .power_off  = omap_usb_power_off,
 + .owner  = THIS_MODULE,
 +};
 +
  static int omap_usb2_probe(struct platform_device *pdev)
  {
   struct omap_usb *phy;
 + struct phy  *generic_phy;
   struct usb_otg  *otg;
 + struct phy_provider *phy_provider;
  
   phy = devm_kzalloc(pdev-dev, sizeof(*phy), GFP_KERNEL);
   if (!phy) {
 @@ -144,6 +171,11 @@ static int omap_usb2_probe(struct platform_device *pdev)
   phy-phy.otg= otg;
   phy-phy.type   = USB_PHY_TYPE_USB2;
  
 + phy_provider = devm_of_phy_provider_register(phy-dev,
 + of_phy_simple_xlate);
 + if (IS_ERR(phy_provider))
 + return PTR_ERR(phy_provider);
 +
   phy-control_dev = omap_get_control_dev();
   if (IS_ERR(phy-control_dev)) {
   dev_dbg(pdev-dev, Failed to get control device\n);
 @@ -159,6 +191,15 @@ static int omap_usb2_probe(struct platform_device *pdev)
   otg-start_srp  = omap_usb_start_srp;
   otg-phy= phy-phy;
  
 + platform_set_drvdata(pdev, phy);
 + pm_runtime_enable(phy-dev);
 +
 + generic_phy = devm_phy_create(phy-dev, 0, ops, omap-usb2);
 + if (IS_ERR(generic_phy))
 + return PTR_ERR(generic_phy);

So, if I have two of these controllers in my system, I can't create the
second phy because the name for it will be identical to the first?
That's why the phy core should handle the id, and not rely on the
drivers to set it, as they have no idea how many they have in the
system.

thanks,

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Thu, Jul 18, 2013 at 02:29:52PM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Thursday 18 July 2013 12:50 PM, Greg KH wrote:
  On Thu, Jul 18, 2013 at 12:16:10PM +0530, Kishon Vijay Abraham I wrote:
  +struct phy_provider *__of_phy_provider_register(struct device *dev,
  +  struct module *owner, struct phy * (*of_xlate)(struct device *dev,
  +  struct of_phandle_args *args));
  +struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
  +  struct module *owner, struct phy * (*of_xlate)(struct device *dev,
  +  struct of_phandle_args *args))
  +
  +__of_phy_provider_register and __devm_of_phy_provider_register can be 
  used to
  +register the phy_provider and it takes device, owner and of_xlate as
  +arguments. For the dt boot case, all PHY providers should use one of the 
  above
  +2 APIs to register the PHY provider.
  
  Why do you have __ for the prefix of a public function?  Is that really
  the way that OF handles this type of thing?
 
 I have a macro of_phy_provider_register/devm_of_phy_provider_register that
 calls these functions and should be used by the PHY drivers. Probably I should
 make a mention of it in the Documentation.

Yes, mention those as you never want to be calling __* functions
directly, right?

  +  ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
  
  Your naming is odd, no phy anywhere in it?  You rely on the sender to
  never send a duplicate name.id pair?  Why not create your own ids based
  on the number of phys in the system, like almost all other classes and
  subsystems do?
 
 hmm.. some PHY drivers use the id they provide to perform some of their
 internal operation as in [1] (This is used only if a single PHY provider
 implements multiple PHYS). Probably I'll add an option like 
 PLATFORM_DEVID_AUTO
 to give the PHY drivers an option to use auto id.
 
 [1] -
 http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html

No, who cares about the id?  No one outside of the phy core ever should,
because you pass back the only pointer that they really do care about,
if they need to do anything with the device.  Use that, and then you can
rip out all of the search for a phy by a string logic, as that's not
needed either.  Just stick to the pointer, it's easier, and safer that
way.

  +static inline int phy_pm_runtime_get(struct phy *phy)
  +{
  +  if (WARN(IS_ERR(phy), Invalid PHY reference\n))
  +  return -EINVAL;
  
  Why would phy ever not be valid and a error pointer?  And why dump the
  stack if that happens, that seems really extreme.
 
 hmm.. there might be cases where the same controller in one soc needs PHY
 control and in some other soc does not need PHY control. In such cases, we
 might get error pointer here.
 I'll change WARN to dev_err.

I still don't understand.  You have control over the code that calls
these functions, just ensure that they pass in a valid pointer, it's
that simple.  Or am I missing something?

thanks,

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

Re: [PATCH 01/15] drivers: phy: add generic PHY framework

[Greg KH]

On Fri, Jul 19, 2013 at 11:07:10AM +0530, Kishon Vijay Abraham I wrote:
  +ret = dev_set_name(phy-dev, %s.%d, dev_name(dev), id);
 
  Your naming is odd, no phy anywhere in it?  You rely on the sender to
  never send a duplicate name.id pair?  Why not create your own ids based
  on the number of phys in the system, like almost all other classes and
  subsystems do?
 
  hmm.. some PHY drivers use the id they provide to perform some of their
  internal operation as in [1] (This is used only if a single PHY provider
  implements multiple PHYS). Probably I'll add an option like 
  PLATFORM_DEVID_AUTO
  to give the PHY drivers an option to use auto id.
 
  [1] -
  http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html
  
  No, who cares about the id?  No one outside of the phy core ever should,
  because you pass back the only pointer that they really do care about,
  if they need to do anything with the device.  Use that, and then you can
 
 hmm.. ok.
 
  rip out all of the search for a phy by a string logic, as that's not
 
 Actually this is needed for non-dt boot case. In the case of dt boot, we use a
 phandle by which the controller can get a reference to the phy. But in the 
 case
 of non-dt boot, the controller can get a reference to the phy only by label.

I don't understand.  They registered the phy, and got back a pointer to
it.  Why can't they save it in their local structure to use it again
later if needed?  They should never have to ask for the device, as the
device id might be unknown if there are multiple devices in the system.

Or am I missing something?

thanks,

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

Re: [PATCH v9 1/8] drivers: phy: add generic PHY framework

[Greg KH]

On Wed, Jun 26, 2013 at 05:17:29PM +0530, Kishon Vijay Abraham I wrote:
 +menuconfig GENERIC_PHY
 + tristate PHY Subsystem
 + help
 +   Generic PHY support.
 +
 +   This framework is designed to provide a generic interface for PHY
 +   devices present in the kernel. This layer will have the generic
 +   API by which phy drivers can create PHY using the phy framework and
 +   phy users can obtain reference to the PHY.

Shouldn't this be something that other drivers select?  How will anyone
know if they need this or not?

 --- /dev/null
 +++ b/drivers/phy/phy-core.c
 @@ -0,0 +1,544 @@
 +/*
 + * phy-core.c  --  Generic Phy framework.
 + *
 + * Copyright (C) 2013 Texas Instruments
 + *
 + * Author: Kishon Vijay Abraham I kis...@ti.com
 + *
 + * 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.

You really mean any later version (I have to ask)?

 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program.  If not, see http://www.gnu.org/licenses/.

Are these two paragraphs needed?  This isn't a program, and they got a
copy of the GPL already with the kernel.

 +static struct class *phy_class;

Why do you need a class?

When modifying/adding new sysfs stuff, you need a Documentation/ABI/
entry as well.

thanks,

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

Re: [PATCH v9 1/8] drivers: phy: add generic PHY framework

[Greg KH]

On Wed, Jul 17, 2013 at 03:02:59PM +0530, Kishon Vijay Abraham I wrote:
 Hi,
 
 On Wednesday 17 July 2013 11:59 AM, Greg KH wrote:
  On Wed, Jun 26, 2013 at 05:17:29PM +0530, Kishon Vijay Abraham I wrote:
  +menuconfig GENERIC_PHY
  +  tristate PHY Subsystem
  +  help
  +Generic PHY support.
  +
  +This framework is designed to provide a generic interface for PHY
  +devices present in the kernel. This layer will have the generic
  +API by which phy drivers can create PHY using the phy framework and
  +phy users can obtain reference to the PHY.
  
  Shouldn't this be something that other drivers select?  How will anyone
  know if they need this or not?
 
 All the PHY drivers should go here. So only if *GENERIC_PHY* is enabled those
 PHY drivers can be selected like in [1].
 The PHY consumer driver should ideally use *depends on* in their Kconfig.
 
 [1] -
 http://archive.arm.linux.org.uk/lurker/message/20130628.134308.4a8f7668.ca.html

No, switch it around the other way.  How would I even _know_ that I need
to enable the generic PHY subsystem in the first place?  How can I
determine that I need this for my hardware?  You need to give hints to
someone who doesn't even know what a PHY is, otherwise they will always
disable it and move on.

  --- /dev/null
  +++ b/drivers/phy/phy-core.c
  @@ -0,0 +1,544 @@
  +/*
  + * phy-core.c  --  Generic Phy framework.
  + *
  + * Copyright (C) 2013 Texas Instruments
  + *
  + * Author: Kishon Vijay Abraham I kis...@ti.com
  + *
  + * 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.
  
  You really mean any later version (I have to ask)?
 
 That was copied from somewhere :-s

So, is that what you really mean to have for this code?

  + * This program is distributed in the hope that it will be useful,
  + * but WITHOUT ANY WARRANTY; without even the implied warranty of
  + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  + * GNU General Public License for more details.
  + *
  + * You should have received a copy of the GNU General Public License
  + * along with this program.  If not, see http://www.gnu.org/licenses/.
  
  Are these two paragraphs needed?  This isn't a program, and they got a
  copy of the GPL already with the kernel.
 
 hmm.. I can remove that.
  
  +static struct class *phy_class;
  
  Why do you need a class?
 
 Wanted to group all the PHY drivers to be used by different subsystems
 (SATA/USB/PCIE/HDMI/VIDEO) into a single entity. There were some comments in 
 my
 initial version [3] on using a bus_type instead of class but then it was
 decided to go with class itself.
 
 [3] - http://lkml.indiana.edu/hypermail/linux/kernel/1302.2/01389.html

Ok, but what does the class usage get you?

  When modifying/adding new sysfs stuff, you need a Documentation/ABI/
  entry as well.
 
 I'm not actually adding any new sysfs entry other than what a *class_create*
 must have created. Do I need to add one for that?

If you are not creating anything in sysfs at all, why use the driver
model?  (hint, I think you need to do something here to justify it...)

thanks,

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

Re: [PATCH v2 0/2] USB: PHY: Improve PHY selection logic

[Greg KH]

On Fri, May 31, 2013 at 02:29:01PM +0300, Roger Quadros wrote:
 Hi,
 
 Improve Kconfig so that the relevant PHY driver can be explicitely
 selected by the controller driver instead of relying on the user
 to do so.
 
 Detailed description in patch 1.

Felipe needs to take these, not I.

thanks,

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

Re: [PATCH 1/1] USB: ehci-omap: Reset dma_mask pointer on probe

[Greg KH]

On Thu, May 23, 2013 at 05:46:44PM +0300, Roger Quadros wrote:
 On 05/23/2013 05:11 PM, Alan Stern wrote:
  On Thu, 23 May 2013, Roger Quadros wrote:
  
  Device tree probed devices don't get dma_mask set. Previously
  we were setting the dma_mask pointer only if it was NULL.
  However, the address of 'omap_ehci_dma_mask' would change
  each time the module is unloaded and loaded back thus causing
  the devices dma_mask pointer to be invalid on the next load.
 
  This will cause page faults if any driver tries to access the
  old dma_mask pointer.
 
  Unconditionally re-setting the dma_mask pointer fixes this problem.
  
  diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
  index 3d1491b..b33e306 100644
  --- a/drivers/usb/host/ehci-omap.c
  +++ b/drivers/usb/host/ehci-omap.c
  @@ -146,8 +146,7 @@ static int ehci_hcd_omap_probe(struct platform_device 
  *pdev)
  * Since shared usb code relies on it, set it here for now.
  * Once we have dma capability bindings this can go away.
  */
  -  if (!pdev-dev.dma_mask)
  -  pdev-dev.dma_mask = omap_ehci_dma_mask;
  +  pdev-dev.dma_mask = omap_ehci_dma_mask;
  
  Is this the solution that people have agreed on?  There has been a lot 
  of discussion on this topic.  In particular, there has been talk about 
  fixing it in the DT core.
 
 Fixing it in DT core would be best.

Then please do that.

thanks,

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

Re: [PATCHv5 1/2] driver: tty: serial: Move uart_console def to core header file.

[Greg KH]

On Wed, May 15, 2013 at 08:13:09PM +0530, Sourav Poddar wrote:
 Hi Greg,
 On Saturday 27 April 2013 03:48 AM, Greg KH wrote:
 On Fri, Apr 26, 2013 at 03:03:07PM -0700, Kevin Hilman wrote:
 Hi Greg,
 
 Sourav Poddarsourav.pod...@ti.com  writes:
 
 Move uart_console definition to serial core header file, so that it can 
 be
 used by serial drivers.
 Get rid of the uart_console defintion from mpc52xx_uart driver.
 
 Cc: Santosh Shilimkarsantosh.shilim...@ti.com
 Cc: Felipe Balbiba...@ti.com
 Cc: Rajendra nayakrna...@ti.com
 Signed-off-by: Sourav Poddarsourav.pod...@ti.com
 Reviewed-by: Felipe Balbiba...@ti.com
 Can you queue up this patch (and 2/2)?  Once these are in, I will queue
 up the corresponding arch/arm changes which can go independently.
 
 And feel free to add
 
 Reviewed-by: Kevin Hilmankhil...@linaro.org
 Tested-by: Kevin Hilmankhil...@linaro.org
 I will do so after 3.10-rc1 is out, it is _way_ too late in the
 development cycle for me to add these to my trees right now, considering
 they are pretty much closed at the moment.
 
 Do you want me to rebase this patches on top of 3.10-rc1?

That would be great to have, thanks.

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

Re: [PATCH] ARM : omap : remove __init for _enable_preprogram

[Greg KH]

On Tue, May 14, 2013 at 06:07:01PM +0200, jp.franc...@cynove.com wrote:
 _enable_preprogram is marked as __init, but is called from _enable which is 
 not.
 This results in oops once init is freed.
 Fix this by removing the __init marker.
 
 Signed-off-by: Jean-Philippe François jp.franc...@cynove.com

formletter

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read Documentation/stable_kernel_rules.txt
for how to do this properly.

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

Re: [PATCHv5 1/2] driver: tty: serial: Move uart_console def to core header file.

[Greg KH]

On Fri, Apr 26, 2013 at 03:03:07PM -0700, Kevin Hilman wrote:
 Hi Greg,
 
 Sourav Poddar sourav.pod...@ti.com writes:
 
  Move uart_console definition to serial core header file, so that it can be
  used by serial drivers.
  Get rid of the uart_console defintion from mpc52xx_uart driver.
 
  Cc: Santosh Shilimkar santosh.shilim...@ti.com
  Cc: Felipe Balbi ba...@ti.com
  Cc: Rajendra nayak rna...@ti.com
  Signed-off-by: Sourav Poddar sourav.pod...@ti.com
  Reviewed-by: Felipe Balbi ba...@ti.com
 
 Can you queue up this patch (and 2/2)?  Once these are in, I will queue
 up the corresponding arch/arm changes which can go independently.
 
 And feel free to add
 
 Reviewed-by: Kevin Hilman khil...@linaro.org
 Tested-by: Kevin Hilman khil...@linaro.org

I will do so after 3.10-rc1 is out, it is _way_ too late in the
development cycle for me to add these to my trees right now, considering
they are pretty much closed at the moment.

thanks,

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

Re: [PATCH] OMAP/serial: Revert bad fix of Rx FIFO threshold granularity

[Greg KH]

On Wed, Apr 03, 2013 at 08:16:18PM +, Paul Walmsley wrote:
 Hi Greg, Alexey,
 
 On Wed, 3 Apr 2013, Alexey Pelykh wrote:
 
  From: Alexey Pelykh alexey.pel...@gmail.com
  
  Partially reverts 1776fd059c40907297d6c26c51876575d63fd9e2
  that introduced regression reported by Paul Walmsley.
  
  This commit restores setting granularity in SCR register
  and adds note about comments below being inconsistent with
  actual code.
  
  Signed-off-by: Alexey Pelykh alexey.pel...@gmail.com
  Cc: Paul Walmsley p...@pwsan.com
  Cc: Kevin Hilman khil...@linaro.org
  Cc: Felipe Balbi ba...@ti.com
  Cc: linux-ser...@vger.kernel.org 
  Cc: linux-arm-ker...@lists.infradead.org 
  Cc: linux-omap@vger.kernel.org 
 
 Tested-by: Paul Walmsley p...@pwsan.com
 
 Thanks for the fast response and fix, Alexey.
 
 Greg, is it possible for you to take this for v3.9-rc fixes?

Will do, thanks.

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

Re: [PATCHv2 09/12] staging: ti-soc-thermal: fix several kernel-doc warnings and error

[Greg KH]

On Tue, Mar 19, 2013 at 10:54:25AM -0400, Eduardo Valentin wrote:
 This patch updates the documentation to remove
 all warnings and errors reported by scripts/kernel-doc.
 Most are missing arguments due to wrong format.
 
 Cc: Nishanth Menon n...@ti.com
 
 Signed-off-by: Eduardo Valentin eduardo.valen...@ti.com

In the future, don't put empty lines between the Cc: and signed-off-by:
lines please, I had to hand-edit all of these to fix that...

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

Re: [PATCH 00/50] staging: omap-thermal: several code refactoring

[Greg KH]

On Sat, Mar 16, 2013 at 08:46:03AM -0400, Eduardo Valentin wrote:
 Hello Dan,
 
 On 16-03-2013 05:05, Dan Carpenter wrote:
 I've reviewed this set.
 
 I hate to make people redo whole patchset sets, and I hate
 re-reviewing code.  Obviously, I don't really like the bunny hop
 patches and I'm trying to discourage that going forward.  ;P  But
 I wouldn't say it's a Redo the whole thing kind of problem.
 
 Could just resend patch 33 and 47?  You should probably be able to
 redo those without changing the rest.
 
 I could of course change them if the comment is better clarified. As
 I mentioned as reply to one of your comments, those changes are
 following what is suggested in CodingStyle file.
 
 I can of course send a diff on top of 33, to fix the introduce bug.
 
 For 47, I'm not sure the comment is fully applicable.

As I've taken all of these already (sorry Dan, I was fast and I didn't
review them as well as you did), you will have to just send incremental
patches on top of the whole series in order for me to be able to apply
them.

thanks,

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

Re: [PATCH 0/8] memory: emif: miscellaneous bug fixes for EMIF driver

[Greg KH]

On Mon, Mar 11, 2013 at 10:35:57AM +0530, Lokesh Vutla wrote:
 This series resolves a few minor issues for EMIF driver.
 
 Tested all patches on OMAP4430-sdp.
 Patch : memory: emif: setup LP settings on freq update
 is tested on a local tree, since freq update cannot be
 tested on mainline.

Can you please resend these with all of the requested changes made, and
acks added?

thanks,

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

Re: [GIT PULL] usb: bug fixes for ehci-omap for 3.9

[Greg KH]

On Thu, Feb 14, 2013 at 02:14:11PM +0200, Roger Quadros wrote:
 Hi Greg,
 
 This pull request contains 2 bug fixes for ehci-omap for 3.9 as well
 as older kernels. It is based on 3.8-rc6.
 
 Please pull. Thanks.
 
 The following changes since commit 88b62b915b0b7e25870eb0604ed9a92ba4bfc9f7:
 
   Linux 3.8-rc6 (2013-02-01 12:08:14 +1100)
 
 are available in the git repository at:
   git://github.com/rogerq/linux.git usbhost17-for-usb

Sorry, I don't take git pulls, please just send these to me as patches
in email.

thanks,

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

Re: [RFC PATCH 1/5] drivers : introduce device_path api

[Greg KH]

On Tue, Nov 27, 2012 at 11:30:11AM -0500, Alan Stern wrote:
 On Tue, 27 Nov 2012, Ming Lei wrote:
 
  IMO, all matches mean the devices are inside the ehci-omap bus, so
  the direct/simple way is to enable/disable the regulators in the probe() and
  remove() of ehci-omap controller driver.
 
 When this discussion started, Felipe argued against such an approach.  
 He pointed out that the same chip could be used in other platforms, and
 then the code added to ehci-omap.c would have to be duplicated in
 several other places.
 
 We should have a more generic solution in a more central location, like 
 the device core.
 
 For example, each struct platform_device could have a list of resources
 to be enabled when the device is bound to a driver and disabled when
 the device is unbound.  Such a list could include regulators, clocks,
 and whatever else people can invent.
 
 In this case, when it is created the ehci-omap.0 platform device could
 get an entry pointing to the LAN95xx's regulator.  Then the regulator 
 would automatically be turned on when the platform device is bound to 
 the ehci-omap driver.
 
 How does this sound?

That sounds much better, and it might come in handy for other types of
devices than platform devices, so feel free to put this on the core
'struct device' instead if needed.

thanks,

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

Re: [RFC PATCH 1/5] drivers : introduce device_path api

[Greg KH]

On Mon, Nov 26, 2012 at 12:45:34PM +, Andy Green wrote:
 +struct device_path list;
 +DEFINE_MUTEX(lock);

Those are some very descriptive global variables you just created :(

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

Re: [RFC PATCH 1/5] drivers : introduce device_path api

[Greg KH]

On Mon, Nov 26, 2012 at 12:45:34PM +, Andy Green wrote:
 This adds a small optional API into drivers/base which deals with generating,
 matching and registration of wildcard device paths.
 
 From a struct device * you can generate a string like
 
  /platform/usbhs_omap/ehci-omap.0/usb1/1-1
 
 which enapsulates the path of the device's connection to the board.
 
 These can be used to match up other assets, for example struct regulators,
 that have been registed elsewhere with a device instance that is probed
 asynchronously from the other board assets.
 
 If your device is on a bus, as it probably is, the device path will feature
 redundant bus indexes that do not contain information about the connectivity.

Huh?  A bus index does show the connectivity, well, one type of
connectivity, but perhaps it's not the one _you_ happen to want at the
moment.  Which is fine, but I don't see why you want to try to figure
this out using the device path in the first place, surely you have some
other way that the hardware can describe itself to the kernel as to
where it needs to be hooked up to?

 For example if more than one driver can generate devices on the same bus,
 then the ordering of device probing will change the path, despite the
 connectivity remains the same.

That's an expected thing, I don't see the issue here.

 For that reason, to get a deterministic path for matching, wildcards are
 allowed.  If your target device has the path

Wait, no, why would you want a deterministic path and have that
hard-coded into the kernel here?  You can't rely on that any more than
userspace can, so let's not start making the mistake that lots of
userspace programmers originally did when they started using sysfs
please.  We have learned from our past mistakes.

  /platform/usbhs_omap/ehci-omap.0/usb1/1-1
 
 generated, in the asset you wish to match with it you can instead use
 
 /platform/usbhs_omap/ehci-omap.0/usb*/*-1
 
 in order to only leave the useful, invariant parts of the path to match
 against.
 
 To avoid having to adapt every kind of search by string api to also use
 the wildcards, the api takes the approach you can register your wildcard,
 and if a matching path is generated for a device, the wildcard itself is
 handed back as the device path instead.
 
 So if your board code or a (not yet done) DT binding registers this wildcard
 
  /platform/usbhs_omap/ehci-omap.0/usb*

Device tree lists sysfs paths in it's descriptions?  That's news to me.

 and the usb hub driver asks to generate its device path
 
   device_path_generate(dev, name, sizeof name);
 
 that is actually
 
  /platform/usbhs_omap/ehci-omap.0/usb1
 
 then what will be returned is
 
  /platform/usbhs_omap/ehci-omap.0/usb*
 
 providing the same literal string for ehci-omap.0 hub no matter how many\
 usb buses have been registered before.
 
 This wildcard string can then be matched to regulators or other string-
 searchable assets intended for the device on that hardware path.

Ah, here's the root of your problem, right?  You need a way for your
hardware to tell the kernel that you have a regulator attached to a
specific device?  Using the device path and hard-coding it into the
kernel is not the proper way to solve this, sorry.  Use some other
unique way to describe the hardware, surely the hardware designers
couldn't have been that foolish not to provide this [1]?

thanks,

greg k-h

[1] Yeah, I know I'm being hopeful here, and probably wrong, but then
you need to push back.  We are not helpless here.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH 3/5] usb: hub: add device_path regulator control to generic hub

[Greg KH]

On Mon, Nov 26, 2012 at 12:45:45PM +, Andy Green wrote:
 This adds the config option to associate a regulator with each hub,
 when the hub on a specific, interesting device path appears, then
 the regular is powered while the logical hub exists.
 
 Signed-off-by: Andy Green andy.gr...@linaro.org
 ---
  drivers/usb/core/Kconfig |   10 ++
  drivers/usb/core/hub.c   |   26 +-
  2 files changed, 35 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/usb/core/Kconfig b/drivers/usb/core/Kconfig
 index f70c1a1..4a91eb1 100644
 --- a/drivers/usb/core/Kconfig
 +++ b/drivers/usb/core/Kconfig
 @@ -95,3 +95,13 @@ config USB_OTG_BLACKLIST_HUB
 and software costs by not supporting external hubs.  So
 are Embedded Hosts that don't offer OTG support.
  
 +config USB_HUB_DEVICE_PATH_REGULATOR
 + bool Support generic regulators at hubs
 + select DEVICEPATH
 + depends on USB
 + default n
 + help
 +   Allows you to use the device_path APIs to associate kernel regulators
 +   with dynamically probed USB hubs, so the regulators are enabled
 +   as the appropriate hub instance gets created and disabled as it
 +   is destroyed.

Even if I did like the device_path code (which I do not), this needs to
be rewritten to actually make sense to a user who would have to pick
this option, not a kernel developer.

 diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
 index a815fd2..49ebb5e 100644
 --- a/drivers/usb/core/hub.c
 +++ b/drivers/usb/core/hub.c
 @@ -26,6 +26,7 @@
  #include linux/mutex.h
  #include linux/freezer.h
  #include linux/random.h
 +#include linux/regulator/consumer.h
  
  #include asm/uaccess.h
  #include asm/byteorder.h
 @@ -54,7 +55,9 @@ struct usb_hub {
   struct usb_device   *hdev;
   struct kref kref;
   struct urb  *urb;   /* for interrupt polling pipe */
 -
 +#ifdef CONFIG_USB_HUB_DEVICE_PATH_REGULATOR

No #ifdefs in .c files, if at all possible please.

thanks,

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

Re: [RFC PATCH 1/5] drivers : introduce device_path api

[Greg KH]

On Mon, Nov 26, 2012 at 11:22:06AM -0800, Greg KH wrote:
 On Mon, Nov 26, 2012 at 12:45:34PM +, Andy Green wrote:
  This adds a small optional API into drivers/base which deals with 
  generating,
  matching and registration of wildcard device paths.
  
  From a struct device * you can generate a string like
  
   /platform/usbhs_omap/ehci-omap.0/usb1/1-1
  
  which enapsulates the path of the device's connection to the board.
  
  These can be used to match up other assets, for example struct regulators,
  that have been registed elsewhere with a device instance that is probed
  asynchronously from the other board assets.
  
  If your device is on a bus, as it probably is, the device path will feature
  redundant bus indexes that do not contain information about the 
  connectivity.
 
 Huh?  A bus index does show the connectivity, well, one type of
 connectivity, but perhaps it's not the one _you_ happen to want at the
 moment.  Which is fine, but I don't see why you want to try to figure
 this out using the device path in the first place, surely you have some
 other way that the hardware can describe itself to the kernel as to
 where it needs to be hooked up to?
 
  For example if more than one driver can generate devices on the same bus,
  then the ordering of device probing will change the path, despite the
  connectivity remains the same.
 
 That's an expected thing, I don't see the issue here.
 
  For that reason, to get a deterministic path for matching, wildcards are
  allowed.  If your target device has the path
 
 Wait, no, why would you want a deterministic path and have that
 hard-coded into the kernel here?  You can't rely on that any more than
 userspace can, so let's not start making the mistake that lots of
 userspace programmers originally did when they started using sysfs
 please.  We have learned from our past mistakes.
 
   /platform/usbhs_omap/ehci-omap.0/usb1/1-1

Oh, and further proof of this, there are patches floating around to drop
the platform name from the sys/drivers/ tree, so your driver just
broke if that goes through, showing you really don't want to be
hard-coding sysfs paths in any type of logic.

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

Re: [RFC PATCH 1/5] drivers : introduce device_path api

[Greg KH]

On Mon, Nov 26, 2012 at 04:07:38PM -0500, Alan Stern wrote:
 On Mon, 26 Nov 2012, Greg KH wrote:
 
  Ah, here's the root of your problem, right?  You need a way for your
  hardware to tell the kernel that you have a regulator attached to a
  specific device?  Using the device path and hard-coding it into the
  kernel is not the proper way to solve this, sorry.  Use some other
  unique way to describe the hardware, surely the hardware designers
  couldn't have been that foolish not to provide this [1]?
 
 As far as I know, the kernel has no other way to describe devices.
 
 What about using partial matches?  In this example, instead of doing a 
 wildcard match against
 
   /platform/usbhs_omap/ehci-omap.0/usb*
 
 (which would fail if the platform part of the path changes), suppose 
 the string ehci-omap.0/usb* could be associated with the usbhs_omap 
 component somehow.  Or even better, the string usb* could be 
 associated with the ehci-omap.0 device.

Yes, all you really care about here is the ehci-omap.0 device, so why
even search for this, you know where the device is, you just created
it :)

 Then the path-matching code could restrict its attention to that part
 of the path and to devices below the specified one.  Naming changes
 wouldn't be an issue, because the changes themselves would be made in
 the same source file that contains the partial path string.
 
 
 On the other hand, this may be way more general than we really need.  
 For this particular case, all we need to do is take special action the
 first time any device is registered below the ehci-omap.0 platform
 device.  There ought to be a more direct way to accomplish this,
 without using string pattern-matching or sysfs pathnames (and without 
 adding overhead every time a device is added or deleted).
 
 I don't know if such an approach would be sufficiently general for 
 future requirements, but it would solve the problem at hand.

And given that this is a very specific problem, and the changes are only
needed for one piece of problem hardware, I suggest keeping the existing
code that implements it.  It's smaller, more specific to the exact
platform, and we don't end up getting stuck with device paths to
describe hardware that might change in the future in ways we do not
anticipate (i.e. the platform change.)

thanks,

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

Re: [PATCH] Revert serial: omap: fix software flow control

[Greg KH]

On Wed, Nov 07, 2012 at 10:56:59AM +0100, Andreas Bießmann wrote:
 On 16.10.2012 16:09, Felipe Balbi wrote:
  This reverts commit 957ee7270d632245b43f6feb0e70d9a5e9ea6cf6
  (serial: omap: fix software flow control).
  
  As Russell has pointed out, that commit isn't fixing
  Software Flow Control at all, and it actually makes
  it even more broken.
  
  It was agreed to revert this commit and use Russell's
  latest UART patches instead.
  
  Cc: Russell King li...@arm.linux.org.uk
  Signed-off-by: Felipe Balbi ba...@ti.com
 
 since 957ee7270d632245b43f6feb0e70d9a5e9ea6cf6 made it into stable (at
 least 3.4) I think it would be good decision to also apply this revert
 to stable until a working solution exists.

Now queued up for the stable releases, thanks.

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

Re: [PATCH] drm/omap: use omapdss low level API

[Greg KH]

On Thu, Nov 15, 2012 at 06:00:58PM -0600, Rob Clark wrote:
 This patch changes the omapdrm KMS to bypass the omapdss compat
 layer and use the core omapdss API directly.  This solves some layering
 issues that would cause unpin confusion vs GO bit status, because we
 would not know whether a particular pageflip or overlay update has hit
 the screen or not.  Now instead we explicitly manage the GO bits in
 dispc and handle the vblank/framedone interrupts ourself so that we
 always know which buffers are being scanned out at any given time, and
 so on.
 
 As an added bonus, we no longer leave the last overlay buffer pinned
 when the display is disabled, and have been able to add the previously
 missing vblank event handling.
 
 Signed-off-by: Rob Clark robdcl...@gmail.com
 ---
 Note: this patch applies on top of staging-next plus the OMAPDSS:
 create compat layer patch series:
 
 http://comments.gmane.org/gmane.linux.ports.arm.omap/89435

Hm, I can't take that patch set in staging-next, so how should this go
in?

thanks,

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

Re: [PATCH] usb: dwc3: core: move dwc3_cache_hwparams before dwc3_alloc_event_buffers

[Greg KH]

On Fri, Nov 16, 2012 at 10:17:19AM +0200, Felipe Balbi wrote:
 Hi,
 
 On Fri, Nov 16, 2012 at 12:07:54PM +0530, Kishon Vijay Abraham I wrote:
  commit 392142 moved event buffer allocation out of dwc3_core_init() but
  event buffer allocation uses the cached copy of hwparams to determine
  the number of event buffers and the caching is done in dwc3_core_init.
  So moved dwc3_cache_hwparams function before dwc3_alloc_event_buffers so
  that dwc3_alloc_event_buffers sees the correct number of event buffers.
  
  Signed-off-by: Kishon Vijay Abraham I kis...@ti.com
 
 My bad, what a regression I caused :-)
 
 Greg, can you take this one ?

Yes, will do.

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

Re: [PATCH] Revert serial: omap: fix software flow control

[Greg KH]

On Wed, Oct 24, 2012 at 01:02:55PM +0300, Felipe Balbi wrote:
 Hi Greg,
 
 On Tue, Oct 16, 2012 at 08:13:59AM -0700, Tony Lindgren wrote:
  * Felipe Balbi ba...@ti.com [121016 07:16]:
   This reverts commit 957ee7270d632245b43f6feb0e70d9a5e9ea6cf6
   (serial: omap: fix software flow control).
   
   As Russell has pointed out, that commit isn't fixing
   Software Flow Control at all, and it actually makes
   it even more broken.
   
   It was agreed to revert this commit and use Russell's
   latest UART patches instead.
   
   Cc: Russell King li...@arm.linux.org.uk
   Signed-off-by: Felipe Balbi ba...@ti.com
  
  This seems like the best way to go for the -rc series:
  
  Acked-by: Tony Lindgren t...@atomide.com
 
 Any chance you can pick this one up for v3.7-rc3 ?

Now queued up, sorry for the delay.

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

Re: [balbi-usb:merge-result-for-greg 59/99] webcam.c:(.text+0x14afec): undefined reference to `config_ep_by_speed'

[Greg KH]

On Mon, Oct 15, 2012 at 09:49:20AM +0200, Sebastian Andrzej Siewior wrote:
 On 10/12/2012 06:39 PM, Fengguang Wu wrote:
 Hi Sebastian,
 
 Hi Fengguang,
 
 FYI, kernel build failed on
 
 tree:   git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git 
 merge-result-for-greg
 head:   23953bde3e4d6aa8780dc054f6ad9882ac63f4f4
 commit: 721e2e91945bc2520d57d795dfe1b502ecec567c [59/99] usb: gadget: 
 libcomposite: move composite.c into libcomposite
 config: i386-randconfig-b199 (attached as .config)
 
 All error/warnings:
 
 drivers/built-in.o: In function `uvc_function_set_alt':
 
 Yes. I accidentally broke this and this still fails in 3.7-rc1. Greg
 has a patch for this in his tree (usb: gadget: Make webcam gadget
 select USB_LIBCOMPOSITE) which fixes this.

Yes, now queued up in my tree.

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

Re: The USB is not working with SM502 Graphics, but working with Intel Pc graphics

[Greg KH]

On Wed, Sep 26, 2012 at 03:09:03PM +0530, Thirumalesha N wrote:
 Dear Sir/Madam,
  
               I'm testing PMC type SM502 based Graphics card which has usb,
 audio and display interfaces. 
 
              I'm using kernel version linux-2.6.21 with redhat enterprise 5.5
 OS.

As you are testing RHEL, please work with Red Hat for this issue, they
are the only ones tha tcan support it, we can't.  Especially as you are
paying for that support, you might as well use it :)

Good luck,

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

Re: [PATCH] drivers: phy: add generic PHY framework

[Greg KH]

On Wed, Sep 26, 2012 at 08:31:15PM +0530, Kishon Vijay Abraham I wrote:
 The PHY framework provides a set of API's for the PHY drivers to
 create/destroy a PHY and API's for the PHY users to obtain a reference to the
 PHY with or without using phandle. To obtain a reference to the PHY without
 using phandle, the platform specfic intialization code (say from board file)
 should have already called phy_bind with the binding information. The binding
 information consists of phy's device name, phy user device name and an index.
 The index is used when the same phy user binds to mulitple phys.
 
 PHY drivers should create the PHY by passing phy_descriptor that has
 describes the PHY (label, type etc..) and ops like init, exit, suspend, 
 resume,
 poweron, shutdown.

Do you have an example driver that uses this new framework?

How does it look in sysfs?  You need to add Documentation/ABI/ entries
for the sysfs files you created as well.

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

Re: [PATCH v3 0/4] lis3: lis3lv02d_i2c: Add device tree support

[Greg KH]

On Mon, Sep 17, 2012 at 12:53:18PM +0530, AnilKumar Ch wrote:
 Adds device tree support to lis3lv02d_i2c driver. Along with this
 DT init is moved from core driver to individual drivers, with the
 current implementation some pdata is missing in lis3lv02d_i2c driver.
 
 Also adds platform data for lis331dlh driver to am335x-EVM. These
 patches were tested on AM335x-EVM.

None of these patches apply to any tree that I know of, what did you do
them against?  Please redo them against the char-next branch of
linux-next and resend them if you wish to see them applied.

thanks,

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

Re: [RFT/PATCH] serial: omap: prevent resume if device is not suspended.

[Greg KH]

On Tue, Sep 25, 2012 at 01:52:03PM +0530, Poddar, Sourav wrote:
 Hi Greg,
 
 Ping on this?

It was a RFT patch, with a huge thread.  What's the resolution here?
Did people figure out the real problem here or not?  If so, care to
resend the proper patch so I know what to apply?

thanks,

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

Re: [PATCH v3 0/4] lis3: lis3lv02d_i2c: Add device tree support

[Greg KH]

On Wed, Sep 26, 2012 at 01:34:50PM -0700, Andrew Morton wrote:
 On Wed, 26 Sep 2012 13:24:00 -0700
 Greg KH gre...@linuxfoundation.org wrote:
 
  On Mon, Sep 17, 2012 at 12:53:18PM +0530, AnilKumar Ch wrote:
   Adds device tree support to lis3lv02d_i2c driver. Along with this
   DT init is moved from core driver to individual drivers, with the
   current implementation some pdata is missing in lis3lv02d_i2c driver.
   
   Also adds platform data for lis331dlh driver to am335x-EVM. These
   patches were tested on AM335x-EVM.
  
  None of these patches apply to any tree that I know of, what did you do
  them against?  Please redo them against the char-next branch of
  linux-next and resend them if you wish to see them applied.
 
 Presumably they're against eariler patches whcih you don't have.  I'm
 presently sitting on
 
 drivers-misc-lis3lv02d-add-generic-dt-matching-code.patch
 drivers-misc-lis3lv02d-add-generic-dt-matching-code-fix.patch
 drivers-misc-lis3lv02d-lis3lv02d_spic-add-dt-matching-table-passthru-code.patch
 drivers-misc-lis3lv02d-lis3lv02d_spic-add-dt-matching-table-passthru-code-fix.patch
 drivers-misc-lis3lv02d-lis3lv02d_spic-add-dt-matching-table-passthru-code-fix-fix.patch
 drivers-misc-lis3lv02d-remove-lis3lv02d-driver-dt-init.patch
 drivers-misc-lis3lv02d-lis3lv02d_spic-add-lis3lv02d-device-tree-init.patch
 drivers-misc-lis3lv02d-lis3lv02d_spic-add-lis3lv02d-device-tree-init-fix.patch
 drivers-misc-lis3lv02d-lis3lv02d_i2cc-add-lis3lv02d-device-tree-init.patch
 drivers-misc-lis3lv02d-lis3lv02d_i2cc-add-lis3lv02d-device-tree-init-fix.patch
 drivers-misc-lis3lv02d-fix-some-comments-specific-to-lis331dlh-driver.patch

Care to send them to me so that I can get them to Linus for the 3.7
merge window?  I do keep the char-misc tree for these types of things :)

thanks,

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