Re: [PATCH v2] usb: dwc3: pci: make better use of gpiod API
On Tue, Jun 23, 2015 at 02:25:06PM +0200, Uwe Kleine-König wrote: Since 39b2bbe3d715 (gpio: add flags argument to gpiod_get*() functions) which appeared in v3.17-rc1, the gpiod_get* functions take an additional parameter that allows to specify direction and initial value for output. Use this additional parameter and the _optional variant to simplify the driver and improve error handling. Also expand the comment to explain why it's not sensible to switch to devm_gpiod_get and why the gpiod_put is also necessary. Furthermore this is one caller less that stops us making the flags argument to gpiod_get*() mandatory. Signed-off-by: Uwe Kleine-König u.kleine-koe...@pengutronix.de Tested-by: Heikki Krogerus heikki.kroge...@linux.intel.com Thanks, -- heikki -- 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: pci: make better use of gpiod API
On Fri, Jun 12, 2015 at 09:10:19AM +0200, Uwe Kleine-König wrote:
Since 39b2bbe3d715 (gpio: add flags argument to gpiod_get*() functions)
which appeared in v3.17-rc1, the gpiod_get* functions take an additional
parameter that allows to specify direction and initial value for output.
Furthermore there is devm_gpiod_get_optional which is designed to get
optional gpios. Also use devm_gpiod_get* because otherwise the gpio
might be grabbed by a different driver.
These gpios are later given to the phy driver. We handle them in this
quirk separately because otherwise the phy can not be probed as it
may still be in reset.
So we are just turning the phy on here, and then releasing the gpios
so the phy driver can take over.
diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
index 27e4fc896e9d..7e308730f955 100644
--- a/drivers/usb/dwc3/dwc3-pci.c
+++ b/drivers/usb/dwc3/dwc3-pci.c
@@ -84,18 +84,19 @@ static int dwc3_pci_quirks(struct pci_dev *pdev)
acpi_dwc3_byt_gpios);
/* These GPIOs will turn on the USB2 PHY */
- gpio = gpiod_get(pdev-dev, cs);
- if (!IS_ERR(gpio)) {
- gpiod_direction_output(gpio, 0);
- gpiod_set_value_cansleep(gpio, 1);
- gpiod_put(gpio);
- }
+ gpio = devm_gpiod_get_optional(pdev-dev, cs, GPIOD_OUT_LOW);
+ if (IS_ERR(gpio))
+ return PTR_ERR(gpio);
+
+ gpiod_set_value_cansleep(gpio, 1);
+
+ gpio = devm_gpiod_get_optional(pdev-dev, reset,
+GPIOD_OUT_LOW);
+ if (IS_ERR(gpio))
+ return PTR_ERR(gpio);
- gpio = gpiod_get(pdev-dev, reset);
- if (!IS_ERR(gpio)) {
- gpiod_direction_output(gpio, 0);
+ if (gpio) {
gpiod_set_value_cansleep(gpio, 1);
- gpiod_put(gpio);
So we still need to release the gpio here, or we break the platforms
where we have these gpios.
Thanks,
--
heikki
--
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 12/13] tty: serial: 8250: omap: add custom irq handling
On Mon, Sep 29, 2014 at 08:06:48PM +0200, Sebastian Andrzej Siewior wrote: We have (or will have) custom DMA callbacks in the omap driver due to the different behaviour in the RX and TX case. To make this work we need a few changes in the IRQ handler to invoke the rx_handler again after the manual mode or retry the tx_handler again before falling back to the manual mode. Heikki didn't want to see the extra hacks in the generic / default irq handler and Peter wasn't too happy about an OMAP-only IRQ handler. The way I planned it is to use this extra IRQ routine only in DMA case. If Peter dislike this approach then I hope Heikki doesn't block changes in the default IRQ handler :) Cc: Heikki Krogerus heikki.kroge...@linux.intel.com Cc: Peter Hurley pe...@hurleysoftware.com Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de Looks good to me. Reviewed-by: Heikki Krogerus heikki.kroge...@linux.intel.com -- heikki -- 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 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx
On Fri, Sep 19, 2014 at 12:58:44PM +0200, Sebastian Andrzej Siewior wrote: On 09/19/2014 12:22 PM, Heikki Krogerus wrote: Couldn't you just replace the handle_irq with a custom irq routine in the omap driver like you did with set_termios? Where you would do those tricks and/or call serial8250_handle_irq()? Tricks within serial8250_handle_irq(), see [0]. It is not a lot but still. I could provide my own handle irq, just asking what you would prefer. [0] http://git.breakpoint.cc/cgit/bigeasy/linux.git/commit/?h=uart_v10_pre2id=f26f161d998ee4a84a0aa6ddff69a435c25f204d The 8250_core changes in that patch #10 only modify serial8250_handle_irg right? Correct. However there is another change due to the RX_DMA_BUG. A while ago you said that this RX_DMA_BUG thing might be something that other SoC could use, too. If you ask me now for my own irq routine it would make sense to move RX bug handling into omap specific code as well. Well, there are no other SoCs at the moment that would need it, and it's actually possible that there never will be. So yes, just handle that also in the omap specific code. Thanks, -- heikki -- 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 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx
On Wed, Sep 17, 2014 at 06:34:45PM +0200, Sebastian Andrzej Siewior wrote: On 09/11/2014 01:42 PM, Sebastian Andrzej Siewior wrote: We should add hooks like tx_dma and rx_dma to struct uart_8250_dma so that the probe drivers can replace serial8250_tx_dma and seria8250_rx_dma, like I think Alan already suggested. Okay. Wasn't aware that Alan already suggested that. I also need a watchdog timer for TX since it seems that on omap3 the DMA engine suddenly forgets to continue with DMA… If this is really what we want, I would need to refactor a few things… Let's keep serial8250_tx_dma/rx_dma as the default, and not add any quirks to them. Only if there is a very common case should it be handled in those. The case of RX req needing to be sent before data in FIFO maybe one of those, but I'm no sure. keep in mind that both (RX TX bugs/hacks) need also a bit of handling in the 8250-core so it works together (like the tx_err member so we fall back to manual xmit) Done. I've kept the RX workarounds in the 8250_dma and moved the TX part into the omap driver. I needed to add the 8250_core pieces of patch #10 [0]. Now If you say, couldn't this done in an other way then I could move the RX workarounds pieces to the omap driver as well as the interrupt routine. Any preferences? [0] [PATCH 10/16] tty: serial: 8250_dma: optimize the xmit path due to UART_BUG_DMA_TX Couldn't you just replace the handle_irq with a custom irq routine in the omap driver like you did with set_termios? Where you would do those tricks and/or call serial8250_handle_irq()? The 8250_core changes in that patch #10 only modify serial8250_handle_irg right? Cheers, -- heikki -- 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 09/16] tty: serial: 8250_dma: Add a TX trigger workaround for AM33xx
On Wed, Sep 10, 2014 at 09:30:04PM +0200, Sebastian Andrzej Siewior wrote:
At least on AM335x the following problem exists: Even if the TX FIFO is
empty and a TX transfer is programmed (and started) the UART does not
trigger the DMA transfer.
After $TRESHOLD number of bytes have been written to the FIFO manually the
UART reevaluates the whole situation and decides that now there is enough
room in the FIFO and so the transfer begins.
This problem has not been seen on DRA7 or beagle board xm (OMAP3). I am not
sure if this is UART-IP core specific or DMA engine.
The workaround is to use a threshold of one byte, program the DMA
transfer minus one byte and then to put the first byte into the FIFO to
kick start the transfer.
v7…v8:
- fix the problem when get invoked and the FIFO is full.
Reviewed-by: Tony Lindgren t...@atomide.com
Tested-by: Tony Lindgren t...@atomide.com
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
drivers/tty/serial/8250/8250.h | 3 +++
drivers/tty/serial/8250/8250_dma.c | 39
+++---
include/uapi/linux/serial_reg.h| 1 +
3 files changed, 40 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index fbed1636e9c4..09489b391568 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -82,6 +82,9 @@ struct serial8250_config {
#define UART_BUG_PARITY (1 4)/* UART mishandles parity if
FIFO enabled */
#define UART_BUG_DMA_RX (1 5)/* UART needs DMA RX req before
there is
data in FIFO */
+#define UART_BUG_DMA_TX (1 6)/* UART needs one byte in FIFO
for
+kickstart */
I don't think we should go ahead with this patch. I'm pretty sure
this is AM335 specific problem, or at least limited to only few
platforms. And I don't think we should take any more BUG flags.
We should add hooks like tx_dma and rx_dma to struct uart_8250_dma so
that the probe drivers can replace serial8250_tx_dma and
seria8250_rx_dma, like I think Alan already suggested.
Let's keep serial8250_tx_dma/rx_dma as the default, and not add any
quirks to them. Only if there is a very common case should it be
handled in those. The case of RX req needing to be sent before data in
FIFO maybe one of those, but I'm no sure.
#define PROBE_RSA(1 0)
#define PROBE_ANY(~0)
diff --git a/drivers/tty/serial/8250/8250_dma.c
b/drivers/tty/serial/8250/8250_dma.c
index 3674900a1f14..48dc57aad0dd 100644
--- a/drivers/tty/serial/8250/8250_dma.c
+++ b/drivers/tty/serial/8250/8250_dma.c
@@ -83,6 +83,7 @@ int serial8250_tx_dma(struct uart_8250_port *p)
struct uart_8250_dma*dma = p-dma;
struct circ_buf *xmit = p-port.state-xmit;
struct dma_async_tx_descriptor *desc;
+ unsigned intskip_byte = 0;
int ret;
if (uart_tx_stopped(p-port) || dma-tx_running ||
@@ -91,10 +92,40 @@ int serial8250_tx_dma(struct uart_8250_port *p)
dma-tx_size = CIRC_CNT_TO_END(xmit-head, xmit-tail, UART_XMIT_SIZE);
+ if (p-bugs UART_BUG_DMA_TX) {
+ u8 tx_lvl;
+
+ /*
+ * We need to put the first byte into the FIFO in order to start
+ * the DMA transfer. For transfers smaller than four bytes we
+ * don't bother doing DMA at all. It seem not matter if there
+ * are still bytes in the FIFO from the last transfer (in case
+ * we got here directly from __dma_tx_complete()). Bytes leaving
+ * the FIFO seem not to trigger the DMA transfer. It is really
+ * the byte that we put into the FIFO.
+ * If the FIFO is already full then we most likely got here from
+ * __dma_tx_complete(). And this means the DMA engine just
+ * completed its work. We don't have to wait the complete 86us
+ * at 115200,8n1 but around 60us (not to mention lower
+ * baudrates). So in that case we take the interrupt and try
+ * again with an empty FIFO.
+ */
+ tx_lvl = serial_in(p, UART_OMAP_TX_LVL);
+ if (tx_lvl == p-tx_loadsz) {
+ ret = -EBUSY;
+ goto err;
+ }
+ if (dma-tx_size 4) {
+ ret = -EINVAL;
+ goto err;
+ }
+ skip_byte = 1;
+ }
+
desc = dmaengine_prep_slave_single(dma-txchan,
-dma-tx_addr + xmit-tail,
-dma-tx_size, DMA_MEM_TO_DEV,
-DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
+ dma-tx_addr + xmit-tail + skip_byte,
+
Re: [PATCH 05/16] tty: serial: 8250_core: remove UART_IER_RDI in serial8250_stop_rx()
On Wed, Sep 10, 2014 at 09:30:00PM +0200, Sebastian Andrzej Siewior wrote: serial8250_do_startup() adds UART_IER_RDI and UART_IER_RLSI to ier. serial8250_stop_rx() should remove both. This is what the serial-omap driver has been doing and is now moved to the 8250-core since it does no look to be *that* omap specific. Cc: heikki.kroge...@linux.intel.com Looks good to me. FWIW... Reviewed-by: Heikki Krogerus heikki.kroge...@linux.intel.com Reviewed-by: Tony Lindgren t...@atomide.com Tested-by: Tony Lindgren t...@atomide.com Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de --- drivers/tty/serial/8250/8250_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c index ac88e66df65d..139f3d2b8aa9 100644 --- a/drivers/tty/serial/8250/8250_core.c +++ b/drivers/tty/serial/8250/8250_core.c @@ -1390,7 +1390,7 @@ static void serial8250_stop_rx(struct uart_port *port) serial8250_rpm_get(up); - up-ier = ~UART_IER_RLSI; + up-ier = ~(UART_IER_RLSI | UART_IER_RDI); up-port.read_status_mask = ~UART_LSR_DR; serial_port_out(port, UART_IER, up-ier); -- 2.1.0 Thanks, -- heikki -- 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/15] tty: serial: Add 8250-core based omap driver
Hi,
On Fri, Aug 15, 2014 at 07:42:33PM +0200, Sebastian Andrzej Siewior wrote:
diff --git a/drivers/tty/serial/8250/8250_core.c
b/drivers/tty/serial/8250/8250_core.c
index cc90c19..ab003b6 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -264,6 +264,12 @@ static const struct serial8250_config uart_config[] = {
.fcr= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
.flags = UART_CAP_FIFO | UART_CAP_AFE,
},
+ [PORT_OMAP_16750] = {
+ .name = OMAP,
+ .fifo_size = 64,
+ .tx_loadsz = 64,
+ .flags = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP,
+ },
I don't think you need this. Reasons below...
[PORT_TEGRA] = {
.name = Tegra,
.fifo_size = 32,
@@ -1390,6 +1396,8 @@ static void serial8250_stop_rx(struct uart_port *port)
serial8250_rpm_get(up);
up-ier = ~UART_IER_RLSI;
+ if (port-type == PORT_OMAP_16750)
+ up-ier = ~UART_IER_RDI;
I don't see any reason why this could not be always cleared regardless
of the type:
up-ier = ~(UART_IER_RLSI | UART_IRE_RDI);
up-port.read_status_mask = ~UART_LSR_DR;
serial_port_out(port, UART_IER, up-ier);
diff --git a/drivers/tty/serial/8250/8250_omap.c
b/drivers/tty/serial/8250/8250_omap.c
new file mode 100644
index 000..368e9d8
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -0,0 +1,911 @@
+/*
+ * 8250-core based driver for the OMAP internal UART
+ *
+ * Copyright (C) 2014 Sebastian Andrzej Siewior
+ *
+ */
[cut]
+static int omap8250_probe(struct platform_device *pdev)
+{
+ struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ struct resource *irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+ struct omap8250_priv *priv;
+ struct uart_8250_port up;
+ int ret;
+ void __iomem *membase;
+
+ if (!regs || !irq) {
+ dev_err(pdev-dev, missing registers or irq\n);
+ return -EINVAL;
+ }
+
+ priv = devm_kzalloc(pdev-dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv) {
+ dev_err(pdev-dev, unable to allocate private data\n);
+ return -ENOMEM;
+ }
+ membase = devm_ioremap_nocache(pdev-dev, regs-start,
+ resource_size(regs));
+ if (!membase)
+ return -ENODEV;
+
+ memset(up, 0, sizeof(up));
+ up.port.dev = pdev-dev;
+ up.port.mapbase = regs-start;
+ up.port.membase = membase;
+ up.port.irq = irq-start;
+ /*
+ * It claims to be 16C750 compatible however it is a little different.
+ * It has EFR and has no FCR7_64byte bit. The AFE which it claims to
+ * have is enabled via EFR instead of MCR.
+ */
+ up.port.type = PORT_OMAP_16750;
Since you are not calling serial8250_do_set_termios, 8250_core.c newer
overrides the FCR set in this driver. However, if the FCR is a
problem, since Yoshihiro added the member for it to struct
uart_8250_port (commit aef9a7bd9), just make it possible for the probe
drivers to provide also it's value:
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -2829,7 +2829,9 @@ static void serial8250_config_port(struct uart_port
*port, int flags)
port-handle_irq = exar_handle_irq;
register_dev_spec_attr_grp(up);
- up-fcr = uart_config[up-port.type].fcr;
+
+ if (!up-fcr)
+ up-fcr = uart_config[up-port.type].fcr;
}
static int
So instead of using PORT_OMAP_16750:
up.port.type = PORT_16750;
up.capabilities = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP;
and the fcr if needed.
up.fcr = ...
+ up.port.iotype = UPIO_MEM32;
+ up.port.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_PORT | UPF_FIXED_TYPE |
+ UPF_SOFT_FLOW | UPF_HARD_FLOW;
+ up.port.private_data = priv;
+
+ up.port.regshift = 2;
+ up.port.fifosize = 64;
You don't need to set the fifosize unless you want to replace the
value you get from uart_config array.
+ up.port.set_termios = omap_8250_set_termios;
+ up.port.pm = omap_8250_pm;
+ up.port.startup = omap_8250_startup;
+ up.port.shutdown = omap_8250_shutdown;
+ up.port.throttle = omap_8250_throttle;
+ up.port.unthrottle = omap_8250_unthrottle;
+
+ if (pdev-dev.of_node) {
+ up.port.line = of_alias_get_id(pdev-dev.of_node, serial);
+ of_property_read_u32(pdev-dev.of_node, clock-frequency,
+ up.port.uartclk);
+ priv-wakeirq = irq_of_parse_and_map(pdev-dev.of_node, 1);
+ } else {
+ up.port.line = pdev-id;
+ }
+
+ if (up.port.line 0) {
+ dev_err(pdev-dev, failed to get alias/pdev id, errno %d\n,
+
Re: [PATCH 06/15] tty: serial: 8250_dma: handle error on TX submit
On Fri, Aug 15, 2014 at 07:42:34PM +0200, Sebastian Andrzej Siewior wrote:
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index b161eee..02e82dc 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -85,6 +85,7 @@ struct uart_8250_port {
unsigned char mcr_force; /* mask of forced bits */
unsigned char cur_iotype; /* Running I/O type */
unsigned char rpm_tx_active;
+ unsigned char dma_tx_err;
Why not make this member of uart_8250_dma instead?
--
heikki
--
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 6/6] tty: serial: Add 8250-core based omap driver
On Wed, Jul 09, 2014 at 07:49:37PM +0200, Sebastian Andrzej Siewior wrote:
diff --git a/drivers/tty/serial/8250/8250_core.c
b/drivers/tty/serial/8250/8250_core.c
index bf06a4c..1cbfc8c 100644
--- a/drivers/tty/serial/8250/8250_core.c
+++ b/drivers/tty/serial/8250/8250_core.c
@@ -263,6 +263,12 @@ static const struct serial8250_config uart_config[] = {
.fcr= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
.flags = UART_CAP_FIFO | UART_CAP_AFE,
},
+ [PORT_OMAP_16750] = {
+ .name = OMAP,
+ .fifo_size = 64,
+ .tx_loadsz = 64,
+ .flags = UART_CAP_FIFO | UART_CAP_EFR | UART_CAP_SLEEP,
+ },
[PORT_TEGRA] = {
.name = Tegra,
.fifo_size = 32,
@@ -1340,6 +1346,8 @@ static void serial8250_stop_rx(struct uart_port *port)
pm_runtime_get_sync(port-dev);
up-ier = ~UART_IER_RLSI;
+ if (port-type == PORT_OMAP_16750)
+ up-ier = ~UART_IER_RDI;
up-port.read_status_mask = ~UART_LSR_DR;
serial_port_out(port, UART_IER, up-ier);
Alan couldn't UART_IER_RDI be always cleared here with all port types?
Actually, shouldn't it be?
Then the custom port type PORT_OMAP_16750 would not be needed.
--
heikki
--
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 RFC 3/4] xhci: Tune PHY for the DWC3-Exynos host controller
Hi,
On Tue, Apr 15, 2014 at 06:24:11PM +0530, Vivek Gautam wrote:
I had seen your patches in the mailing list, but i don't see any
updated version of these patches.
Are you planning to work on this above mentioned patch-series any time soon ?
I'm sorry, I forgot this completely. I have not prepared new version
of those patches as the drivers I need them for are not ready yet, but
I guess I can also use this case as justification for them.
Or, should i try to find a different approach for handling the phy
from the host controller (child of DWC3 in this case, which has the
phys).
Well, there is now an issue with the lookup method I'm suggesting in
this case. Since the device ID is now generated automatically for
xhci-hcd in dwc3, we don't know the xhci-hcd device name before
platform_device_add(), and that is too late. I don't see why the
device could not be named when platform_device_alloc() is called, so I
think I'll suggest something like the attached patch to fix this
issue.
In any case, I'll send an updated version of the phy patches soon.
Thanks,
--
heikki
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index e714709..13f8edb 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -169,11 +169,47 @@ struct platform_object {
*/
void platform_device_put(struct platform_device *pdev)
{
- if (pdev)
- put_device(pdev-dev);
+ if (!pdev)
+ return;
+
+ if (pdev-id_auto) {
+ ida_simple_remove(platform_devid_ida, pdev-id);
+ pdev-id = PLATFORM_DEVID_AUTO;
+ }
+
+ put_device(pdev-dev);
}
EXPORT_SYMBOL_GPL(platform_device_put);
+static int pdev_set_name(struct platform_device *pdev)
+{
+ int ret;
+
+ switch (pdev-id) {
+ default:
+ dev_set_name(pdev-dev, %s.%d, pdev-name, pdev-id);
+ break;
+ case PLATFORM_DEVID_NONE:
+ dev_set_name(pdev-dev, %s, pdev-name);
+ break;
+ case PLATFORM_DEVID_AUTO:
+ /*
+ * Automatically allocated device ID. We mark it as such so
+ * that we remember it must be freed, and we append a suffix
+ * to avoid namespace collision with explicit IDs.
+ */
+ ret = ida_simple_get(platform_devid_ida, 0, 0, GFP_KERNEL);
+ if (ret 0)
+ return ret;
+ pdev-id = ret;
+ pdev-id_auto = true;
+ dev_set_name(pdev-dev, %s.%d.auto, pdev-name, pdev-id);
+ break;
+ }
+
+ return 0;
+}
+
static void platform_device_release(struct device *dev)
{
struct platform_object *pa = container_of(dev, struct platform_object,
@@ -206,6 +242,10 @@ struct platform_device *platform_device_alloc(const char *name, int id)
device_initialize(pa-pdev.dev);
pa-pdev.dev.release = platform_device_release;
arch_setup_pdev_archdata(pa-pdev);
+ if (pdev_set_name(pa-pdev)) {
+ kfree(pa);
+ return NULL;
+ }
}
return pa ? pa-pdev : NULL;
@@ -286,28 +326,6 @@ int platform_device_add(struct platform_device *pdev)
pdev-dev.bus = platform_bus_type;
- switch (pdev-id) {
- default:
- dev_set_name(pdev-dev, %s.%d, pdev-name, pdev-id);
- break;
- case PLATFORM_DEVID_NONE:
- dev_set_name(pdev-dev, %s, pdev-name);
- break;
- case PLATFORM_DEVID_AUTO:
- /*
- * Automatically allocated device ID. We mark it as such so
- * that we remember it must be freed, and we append a suffix
- * to avoid namespace collision with explicit IDs.
- */
- ret = ida_simple_get(platform_devid_ida, 0, 0, GFP_KERNEL);
- if (ret 0)
- goto err_out;
- pdev-id = ret;
- pdev-id_auto = true;
- dev_set_name(pdev-dev, %s.%d.auto, pdev-name, pdev-id);
- break;
- }
-
for (i = 0; i pdev-num_resources; i++) {
struct resource *p, *r = pdev-resource[i];
@@ -350,7 +368,6 @@ int platform_device_add(struct platform_device *pdev)
release_resource(r);
}
- err_out:
return ret;
}
EXPORT_SYMBOL_GPL(platform_device_add);
@@ -370,11 +387,6 @@ void platform_device_del(struct platform_device *pdev)
if (pdev) {
device_del(pdev-dev);
- if (pdev-id_auto) {
- ida_simple_remove(platform_devid_ida, pdev-id);
- pdev-id = PLATFORM_DEVID_AUTO;
- }
-
for (i = 0; i pdev-num_resources; i++) {
struct resource *r = pdev-resource[i];
unsigned long type = resource_type(r);
@@ -392,8 +404,15 @@ EXPORT_SYMBOL_GPL(platform_device_del);
*/
int platform_device_register(struct platform_device *pdev)
{
+ int ret;
+
device_initialize(pdev-dev);
arch_setup_pdev_archdata(pdev);
+
+ ret = pdev_set_name(pdev);
+ if (ret)
+ return ret;
+
return platform_device_add(pdev);
}
EXPORT_SYMBOL_GPL(platform_device_register);
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi, On Tue, Jan 28, 2014 at 10:30:36AM -0600, Felipe Balbi wrote: On Tue, Jan 28, 2014 at 05:32:30PM +0200, Heikki Krogerus wrote: On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: For the controller drivers the PHYs are just a resource like any other. The controller drivers can't have any responsibility of them. They should not care if PHY drivers are available for them or not, or even if the PHY framework is available or not. huh? If memory isn't available you don't continue probing, right ? If your IORESOURCE_MEM is missing, you also don't continue probing, if your IRQ line is missing, you bail too. Those are also nothing but resources to the driver, what you're asking here is to treat PHY as a _different_ resource; which might be fine, but we need to make sure we don't continue probing when a PHY is missing in a platform that certainly needs a PHY. Yes, true. In my head I was comparing the PHY only to resources like gpios, clocks, dma channels, etc. that are often optional to the drivers. But I really want to see the argument against using no-op. As far as I could see, everybody needs a PHY driver one way or another, some platforms just haven't sent any PHY driver upstream and have their own hacked up solution to avoid using the PHY layer. Not true in our case. Platforms using Intel's SoCs and chip sets may or may not have controllable USB PHY. Quite often they don't. The Baytrails have usually ULPI PHY for USB2, but that does not mean they provide any vendor specific functions or any need for a driver in any case. that's different from what I heard. I don't know where you got that impression, but it's not true. The Baytrail SoCs for example don't have internal USB PHYs, which means the manufacturers using it can select what they want. So we have boards where PHY driver(s) is needed and boards where it isn't. alright, that explains it ;-) So you have external USB2 and USB3 PHYs ? You have an external PIPE3 interface ? That's quite an achievement, kudos to your HW designers. Getting timing closure on PIPE3 is a difficult task. No, only the USB2 PHY is external. I'm giving you wrong information, I'm sorry about that. Need to concentrate on what I'm writing. snip This is really good to get. We have some projects where we are dealing with more embedded environments, like IVI, where the kernel should be stripped of everything useless. Since the PHYs are autonomous, we should be able to disable the PHY libraries/frameworks. hmmm, in that case it's a lot easier to treat. We can use ERR_PTR(-ENXIO) as an indication that the framework is disabled, or something like that. The difficult is really reliably supporting e.g. OMAP5 (which won't work without a PHY) and your BayTrail with autonomous PHYs. What can we use as an indication ? OMAP has it's own glue driver, so shouldn't it depend on the PHY layer? I mean, I need to know that a particular platform depends on a PHY driver before I decide to return -EPROBE_DEFER or just assume the PHY isn't needed ;-) I don't think dwc3 (core) should care about that. The PHY layer needs to tell us that. If the PHY driver that the platform depends is not available yet, the PHY layer returns -EPROBE_DEFER and dwc3 ends up returning -EPROBE_DEFER. snip I think our goals are the same. I just want to also minimize the need for any platform specific extra work from the upper layers regarding the PHYs. I'll agree to that, but let's not apply patches until we're 100% sure there will be no regressions. Looking at $subject, I don't think it satisfies that condition ;-) Agreed. Let's think this through carefully. Cheers, -- heikki -- 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/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi, On Mon, Jan 27, 2014 at 10:05:20AM -0600, Felipe Balbi wrote: Why would you need to know if the PHY drivers are needed or not explicitly in your controller driver? because, one way or another, they all do need it. Except for quirky ones like AM437x where a USB3 IP was hardwired into USB2-only mode, so it really lacks a USB3 PHY. The Baytrail board I deal with has completely autonomous PHYs. But my question is, why would you need to care about the PHYs in your controller driver? Why can't you leave the responsibility of them to the upper layers and trust what they tell you? If there is no USB3 PHY for dwc3 then, the driver gets something like -ENODEV and just continues. There is no need to know about the details. For the controller drivers the PHYs are just a resource like any other. The controller drivers can't have any responsibility of them. They should not care if PHY drivers are available for them or not, or even if the PHY framework is available or not. But I really want to see the argument against using no-op. As far as I could see, everybody needs a PHY driver one way or another, some platforms just haven't sent any PHY driver upstream and have their own hacked up solution to avoid using the PHY layer. Not true in our case. Platforms using Intel's SoCs and chip sets may or may not have controllable USB PHY. Quite often they don't. The Baytrails have usually ULPI PHY for USB2, but that does not mean they provide any vendor specific functions or any need for a driver in any case. that's different from what I heard. I don't know where you got that impression, but it's not true. The Baytrail SoCs for example don't have internal USB PHYs, which means the manufacturers using it can select what they want. So we have boards where PHY driver(s) is needed and boards where it isn't. The problem is that we just don't always know all the details about the platform. If the PHY is ULPI PHY, we can do runtime detection, but we can't even rely on always having ULPI. Are we talking about the old USB PHY library or the new PHY framework with the no-op PHY driver? Well, in any case, I don't understand what is the purpose of the no-op PHY driver. What are you drying to achieve with that? I'm trying to avoid supporting 500 different combinations for a single driver. We already support old USB PHY layer and generic PHY layer, now they both need to be made optional. This is really good to get. We have some projects where we are dealing with more embedded environments, like IVI, where the kernel should be stripped of everything useless. Since the PHYs are autonomous, we should be able to disable the PHY libraries/frameworks. But I still don't understand what is the benefit in the no-op? You really need to explain this. What you have now in dwc3 is expectations regarding the PHY, which put a lot of pressure on upper layers to satisfy the driver. The no-op is just an extra thing that you have to provide when you fail to determine if the system requires a PHY driver or not, or if you know that it doesn't, plus an additional check for the case where the PHY lib/framework is not enabled. I don't see the value in it. The old USB PHY layer also provides a no-op, now called phy-generic, which is actually pretty useful. On top of all that, I'm sure you'll subscribe to the idea of preventing dwc3 from becoming another MUSB which supports several different configurations and none work correctly. I much prefer to take baby steps which are well thought-out and very well exercised, so I'll be very pedantic about proof of testing. I think our goals are the same. I just want to also minimize the need for any platform specific extra work from the upper layers regarding the PHYs. Thanks, -- heikki -- 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/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi Felipe, On Tue, Jan 21, 2014 at 08:47:25AM -0600, Felipe Balbi wrote: On Tue, Jan 21, 2014 at 03:41:38PM +0530, Kishon Vijay Abraham I wrote: Since PHYs for dwc3 is optional (not all SoCs that have DWC3 use PHYs), do not return from probe if the USB PHY library returns -ENODEV as that this isn't correct, they all have PHYs, some of them might not be controllable. indicates the platform does not have PHY. not really, that indicates the current platform tried to grab a PHY and the PHY doesn't exist. If there's anybody with a non-controllable PHY and someone gives me a really good reason for not using the generic no-op PHY, then we should add a flag and we could: if (!likely(dwc-flags DWC3_USB2PHY_DRIVER_NOT_NEEDED)) dwc3_grab_phys(dwc); Why would you need to know if the PHY drivers are needed or not explicitly in your controller driver? But I really want to see the argument against using no-op. As far as I could see, everybody needs a PHY driver one way or another, some platforms just haven't sent any PHY driver upstream and have their own hacked up solution to avoid using the PHY layer. Not true in our case. Platforms using Intel's SoCs and chip sets may or may not have controllable USB PHY. Quite often they don't. The Baytrails have usually ULPI PHY for USB2, but that does not mean they provide any vendor specific functions or any need for a driver in any case. Are we talking about the old USB PHY library or the new PHY framework with the no-op PHY driver? Well, in any case, I don't understand what is the purpose of the no-op PHY driver. What are you drying to achieve with that? Thanks, -- heikki -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy
On Wed, Jan 15, 2014 at 07:41:55PM +0530, Kishon Vijay Abraham I wrote: The point I'm trying to make is that we won't 'always' know the device names in advance. In which cases do we not know the device names, and please note, cases where we would need to use the lookup? The normal cases we would use it are.. 1) A driver creating a child device, like dwc3 when it creates the xhci. There the parent just delivers the phys it has to the child, so both the device name and the phys are known. 2) Platform code. Hopefully we can get rid of the platform code one day, but in any case, when we use it, we always know at least how many users a phy has, and if there is only a singe user, we can rely con_id to do the matching. Though I still don't really see much risk in using the controller device name also there. The lookup table should in any case be made in the place where the phy devices are created, so the phy name is definitely always known. 3) Phys part of something like mfd. This is in practice the same as the platform code case. For example, with twl we always know there is only one user for the phy, which is musb. So we can just use musb's con_id if it's to scary to use musb-hdrc.0. In any other case, you get the phy from DT, and there is no need to use the lookup when linking the phys and the users. Btw how are you planning to differentiate between multiple controllers of the same type with con_id? You don't rely on the con_id alone in those cases. You then use the device name. The con_id is just one parameter you can use to do the matching. Maybe I'm missing the actual intention of this patch. Do you face problem if the PHY's have to know about it's consumers in ACPI? snip I would rather leave the way it is modelled now. Do you mean, leave it in the platform code? Why? We would reduce the platform code by moving it to the mfd driver. Or did I misunderstood something? In this case, the lookup table will be used only for non-dt boot so don't see much use in moving to mfd driver. I meant if the new method is not solving any problem then I would prefer the way it is modelled now where the PHY's know about it's consumers. OK, so that's what you meant. Things like that init_data promotes use of platform data in the drivers, something that we really should get rid of. Is that not a problem to you? The phy drivers simply should not need to care about the consumers. They should not need to care about anything DT, ACPI, platform specific if it's possible, and here there is nothing preventing it. Let me clarify.. The plan is that ultimately the drivers (meaning all the drivers, not just phy) don't need to care about things like DT properties, ACPI properties or anything DT, ACPI, platform specific. We will have generic driver properties and capabilities and the upper layers will take care of delivering the correct values from the source at hand. The drivers can be made truly platform agnostic. That is the goal, and we should make everything new by keeping that in mind. Many existing frameworks are being converted to that direction, like the gpio framework and DMA Engine API. The way you force the phy drivers the deliver the consumers is taking a step backwards. Btw where does device gets created in ACPI boot? drivers/acpi/acpi_platform.c Thanks, -- heikki -- 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/5] phy: add support for indexed lookup
Hi Kishon, And happy new year.. On Tue, Jan 07, 2014 at 07:10:36PM +0530, Kishon Vijay Abraham I wrote: /** - * phy_get() - lookup and obtain a reference to a phy. + * phy_get_index() - obtain a phy based on index NAK. It still takes a 'char' argument and the name is misleading. Btw are you replacing phy_get() or adding a new API in addition to phy_get()? Additional API. The phy_get() would in practice act as a wrapper after In this patch it looks like you've replaced phy_get(). this. It could actually be just a #define macro in the include file. The function naming I just copied straight from gpiolib.c. I did not have the imagination for anything fancier. I would like to be able to use some function like phy_get_index() and be able to deliver it both the name and the index. With DT you guys will always be able to use the name (and the string will always supersede the index if we do it like this), but with ACPI, and possibly the platform lookup tables, the index can be used... I think in that case, we should drop the 'string' from phy_get_index since we have the other API to handle that? I don't know about ACPI, but is it not possible to use strings with ACPI? No unfortunately. We just have what the ACPI tables provide. The PHYs would be child device entries under the controller and we can only get handle to them based on the index. I think I'll skip this patch from this set. Let's wait until we have an actual ACPI DSDT describing some PHYs. Thanks, -- heikki -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy
On Tue, Jan 07, 2014 at 06:31:52PM +0530, Kishon Vijay Abraham I wrote: In any case, having two device names to deal with does not add any more risk. These associations should always be made in the place where the phy device is created so you will always know it's device name. huh.. we should also know the 'controller' device name while defining these associations and in some cases the controller device and phy device are created in entirely different places. If you don't want to use the controller device name to do the matching, we can use the con_id string as well. I believe the lookup method I use in this set just needs a small change. Is that OK with you? Normally the platform code creates these associations in the same place it creates the platform devices, so you definitely know what the device names will be. In this case it's actually created in drivers/mfd/twl-core.c, so I do need to update this and move the lookup table there. We can still deliver the user name as platform data, though I believe it's always musb. Maybe we could actually skip that and just hard code the name? I would rather leave the way it is modelled now. Do you mean, leave it in the platform code? Why? We would reduce the platform code by moving it to the mfd driver. Or did I misunderstood something? Hope I'm not forgetting something we have talked before my vacation. Thanks, -- heikki -- 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/5] phy: add support for indexed lookup
Hi Kishon,
On Mon, Dec 16, 2013 at 04:32:58PM +0530, Kishon Vijay Abraham I wrote:
On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
Removes the need for the consumer drivers requesting the
phys to provide name for the phy. This should ease the use
of the framework considerable when using only one phy, which
is usually the case when except with USB, but it can also
be useful with multiple phys.
If index has to be used with multiple PHYs, the controller should be aware of
the order in which it is populated in dt data. That's not good.
The Idea is not to replace the name based lookup. Just to provide
possibility for index based lookup.
With ACPI, if we get the device entries for PHYs, the order will be
fixed and we will not have any other reference to the phys. In case
of USB, the first one should always be USB2 PHY and the second the
USB3 PHY.
This will also reduce noise from the framework when there is
no phy by changing warnings to debug messages.
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
---
drivers/phy/phy-core.c | 106
++--
include/linux/phy/phy.h | 14 +++
2 files changed, 89 insertions(+), 31 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 1102aef..99dc046 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res,
void *match_data)
return res == match_data;
}
-static struct phy *phy_lookup(struct device *device, const char *con_id)
+static struct phy *phy_lookup(struct device *device, const char *con_id,
+ unsigned int idx)
{
unsigned int count;
struct phy *phy;
@@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device,
const char *con_id)
count = phy-init_data-num_consumers;
consumers = phy-init_data-consumers;
while (count--) {
+ /* index must always match exactly */
+ if (!con_id)
+ if (idx != count)
+ continue;
if (!strcmp(consumers-dev_name, dev_name(device))
!strcmp(consumers-port, con_id)) {
class_dev_iter_exit(iter);
@@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
/**
* of_phy_get() - lookup and obtain a reference to a phy by phandle
* @dev: device that requests this phy
- * @index: the index of the phy
+ * @con_id: name of the phy from device's point of view
+ * @idx: the index of the phy if name is not used
*
* Returns the phy associated with the given phandle value,
* after getting a refcount to it or -ENODEV if there is no such phy or
@@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
* not yet loaded. This function uses of_xlate call back function provided
* while registering the phy_provider to find the phy instance.
*/
-static struct phy *of_phy_get(struct device *dev, int index)
+static struct phy *of_phy_get(struct device *dev, const char *con_id,
+ unsigned int idx)
{
int ret;
struct phy_provider *phy_provider;
struct phy *phy = NULL;
struct of_phandle_args args;
+ int index;
+
+ if (!con_id)
+ index = idx;
+ else
+ index = of_property_match_string(dev-of_node, phy-names,
+con_id);
ret = of_parse_phandle_with_args(dev-of_node, phys, #phy-cells,
index, args);
@@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev,
struct of_phandle_args
EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
/**
- * phy_get() - lookup and obtain a reference to a phy.
+ * phy_get_index() - obtain a phy based on index
NAK. It still takes a 'char' argument and the name is misleading.
Btw are you replacing phy_get() or adding a new API in addition to phy_get()?
Additional API. The phy_get() would in practice act as a wrapper after
this. It could actually be just a #define macro in the include file.
The function naming I just copied straight from gpiolib.c. I did not
have the imagination for anything fancier.
I would like to be able to use some function like phy_get_index() and
be able to deliver it both the name and the index. With DT you guys
will always be able to use the name (and the string will always
supersede the index if we do it like this), but with ACPI, and possibly
the platform lookup tables, the index can be used...
phy2 = phy_get_index(dev, usb2-phy, 0);
phy3 = phy_get_index(dev, usb3-phy, 1);
* @dev: device that requests this phy
* @con_id: name of the phy from device's point of view
+ * @idx: index of the phy to obtain in the consumer
*
- * Returns the phy driver, after
Re: [PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy
Hi,
On Mon, Dec 16, 2013 at 04:34:35PM +0530, Kishon Vijay Abraham I wrote:
On Monday 09 December 2013 08:38 PM, Heikki Krogerus wrote:
Provide a complete association for the phy and it's user
(musb) with the new phy_lookup_table.
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
---
arch/arm/mach-omap2/twl-common.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-omap2/twl-common.c
b/arch/arm/mach-omap2/twl-common.c
index b0d54da..972430b 100644
--- a/arch/arm/mach-omap2/twl-common.c
+++ b/arch/arm/mach-omap2/twl-common.c
@@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
}
#if defined(CONFIG_ARCH_OMAP3)
-struct phy_consumer consumers[] = {
- PHY_CONSUMER(musb-hdrc.0, usb),
-};
-
-struct phy_init_data init_data = {
- .consumers = consumers,
- .num_consumers = ARRAY_SIZE(consumers),
+static struct phy_lookup_table twl4030_usb_lookup = {
+ .dev_id = musb-hdrc.0,
+ .table = PHY_LOOKUP(phy-twl4030_usb.0, NULL),
};
had a similar approach during the initial version of phy framework [1] (see
phy_bind) but changed it later [2] . Here you should know the device names of
two devices and even if one of them started using DEVID_AUTO, it'll start
breaking. Such an approach needs to reviewed carefully.
If somebody creates a regression by changing something like the
platform device id without checking all the users, he deserves to get
into big trouble. I don't see why an individual framework should
provide protection against such cases.
In any case, having two device names to deal with does not add any
more risk. These associations should always be made in the place where
the phy device is created so you will always know it's device name.
Normally the platform code creates these associations in the same
place it creates the platform devices, so you definitely know what the
device names will be.
In this case it's actually created in drivers/mfd/twl-core.c, so I do
need to update this and move the lookup table there. We can still
deliver the user name as platform data, though I believe it's always
musb. Maybe we could actually skip that and just hard code the name?
The name of the phy we will of course know as the platform device is
created there and then, so the two device names don't create any more
risk even in this case.
Thanks,
--
heikki
--
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 RFC 1/4] phy: Add provision for tuning phy.
Hi, On Wed, Dec 11, 2013 at 12:08:04PM +0530, Vivek Gautam wrote: On Tue, Dec 10, 2013 at 7:31 PM, Heikki Krogerus I think setup instead of tune is much more clear and reusable. I think setup will look more like first time setting up the phy, which is rather served by init callback. This i thought would serve the purpose of over-riding certain PHY parameters, which would not have been possible at init time. Please correct my thinking if i am unable to understand your point here. OK, sorry I was not clear on this. I'm thinking the same, that this is something that is called later, for example when the controller is ready. Some ULPI phys need to be initialized, but since the controller provides the interface, it's usually not possible during init time. This hook could be used in that case as well. All I'm saying is that tune is a poor expression. You will need to add a comment explaining what the hook does in any case, so you'll have something like this is something that is called when the controller is ready or something similar. That will make it clear what it's meant for. Thanks, -- heikki -- 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 04/10] usb: dwc3: use quirks to know if a particualr platform doesn't have PHY
Hi, On Mon, Dec 09, 2013 at 11:26:04AM +0200, Heikki Krogerus wrote: Can you guys explain why is something like this needed? Like with clocks and gpios, the device drivers shouldn't need to care any more if the platform has the phys or not. -ENODEV tells you your platform Shouldn't we report if a particular platform needs a PHY and not able to get it. How will a user know if a particular controller is not working because it's not able to get and initialize the PHYs? Don't you think in such cases it's better to fail (and return from probe) because the controller will not work anyway without the PHY? My point is that you do not need to separately tell this to the driver like you do with the quirks (if you did, then you would need to fix your framework and not hack the drivers). Like I said, ENODEV tells you that there is no phy on this platform for you, allowing you to safely continue. If your phy driver is not loaded, the framework already returns EPROBE_DEFER, right. Any other right. but that doesn't consider broken dt data. With quirks we'll able to tell if a controller in a particular platform has PHY or not without depending on the dt data. Broken dt data? What kind of scenario are you thinking here? Do you mean case where the dt does not describe the phy on a platform that depends on it? Shouldn't that problem be fixed in the dt and not hacked in the drivers? Or are you thinking about something else? Is there a case where something like that is actually happening? I'm guessing I'm not getting an answer to this one. Look, this patch will not work with ACPI enumerated devices. We will have a platform providing a single ACPI id, but there is a whole bunch of boards based on it and we have no way of telling which of them need/have phys to deal with and which ones don't. Thanks, -- heikki -- 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 RFC 1/4] phy: Add provision for tuning phy.
Hi again, On Wed, Dec 11, 2013 at 02:02:43PM +0530, Vivek Gautam wrote: On Wed, Dec 11, 2013 at 1:39 PM, Heikki Krogerus heikki.kroge...@linux.intel.com wrote: On Wed, Dec 11, 2013 at 12:08:04PM +0530, Vivek Gautam wrote: On Tue, Dec 10, 2013 at 7:31 PM, Heikki Krogerus I think setup instead of tune is much more clear and reusable. I think setup will look more like first time setting up the phy, which is rather served by init callback. This i thought would serve the purpose of over-riding certain PHY parameters, which would not have been possible at init time. Please correct my thinking if i am unable to understand your point here. OK, sorry I was not clear on this. I'm thinking the same, that this is something that is called later, for example when the controller is ready. Some ULPI phys need to be initialized, but since the controller provides the interface, it's usually not possible during init time. This hook could be used in that case as well. All I'm saying is that tune is a poor expression. You will need to add a comment explaining what the hook does in any case, so you'll have something like this is something that is called when the controller is ready or something similar. That will make it clear what it's meant for. Ok, i think i should have kept a comment atleast :-( I will add proper comments above, and as suggested in the mail by Kishon, may be name it calibrate ? What do you think ? Sure, I'm fine with that. Thanks, -- heikki -- 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 RFC 3/4] xhci: Tune PHY for the DWC3-Exynos host controller
Hi,
On Tue, Dec 10, 2013 at 04:25:25PM +0530, Vivek Gautam wrote:
@@ -170,6 +189,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
}
/*
+ * The parent of the xhci-plat device may pass in a PHY via
+ * platform data. If it exists, store it in our struct usb_hcd
+ * so that we can use it later.
+ */
+ phy_generic = dev_get_platdata(pdev-dev);
+ if (phy_generic)
+ xhci-shared_hcd-phy_generic = *phy_generic;
Getting the handle to the phy from platform data like this is not
going to work for long. It should be possible to get it normally with
phy_get(). It's not going to be possible to get the handle from the
platform data like this if the xhci-hcd platform device is created
from ACPI or DT. You are also not considering case where you have two
phys.
Vivek, I have made a patch set for the phy framework allowing
associations between the phys and their users to be made in same way
gpios and clk make them. With those you should be able to create a
lookup entry to the phy framework in drivers/usb/dwc3/host.c. Then we
could use phy_get() here already. Please check them. Subject of the
thread:
phy: remove the need for the phys to know about their users
The lookup table can then be added in drivers/usb/dwc3/host.c with
something like this:
int dwc3_host_init(struct dwc3 *dwc)
{
struct platform_device *xhci;
struct phy_lookup_table *table;
...
table-dev_id = dev_name(xhci-dev);
if (dwc-usb2_generic_phy)
table-table[0].phy_name =
dev_name(dwc-usb2_generic_phy-dev);
if (dwc-usb3_generic_phy)
table-table[1].phy_name =
dev_name(dwc-usb3_generic_phy-dev);
phy_add_lookup_table(table);
...
Br,
--
heikki
--
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 RFC 1/4] phy: Add provision for tuning phy.
Hi,
On Tue, Dec 10, 2013 at 04:25:23PM +0530, Vivek Gautam wrote:
Some PHY controllers may need to tune PHY post-initialization,
so that the PHY consumers can call phy-tuning at appropriate
point of time.
Signed-off-by: vivek Gautam gautam.vi...@samsung.com
---
drivers/phy/phy-core.c | 20
include/linux/phy/phy.h |7 +++
2 files changed, 27 insertions(+), 0 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 03cf8fb..68dbb90 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -239,6 +239,26 @@ out:
}
EXPORT_SYMBOL_GPL(phy_power_off);
+int phy_tune(struct phy *phy)
+{
+ int ret = -ENOTSUPP;
+
+ mutex_lock(phy-mutex);
+ if (phy-ops-tune) {
+ ret = phy-ops-tune(phy);
+ if (ret 0) {
+ dev_err(phy-dev, phy tuning failed -- %d\n, ret);
+ goto out;
+ }
+ }
+
+out:
+ mutex_unlock(phy-mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_tune);
I think setup instead of tune is much more clear and reusable.
Thanks,
--
heikki
--
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 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
Hi, On Fri, Dec 06, 2013 at 02:15:30PM -0600, Felipe Balbi wrote: On Tue, Dec 03, 2013 at 12:39:50PM +0200, Heikki Krogerus wrote: On Thu, Oct 17, 2013 at 09:54:26AM -0500, Felipe Balbi wrote: On Wed, Oct 16, 2013 at 04:27:26PM +0300, Roger Quadros wrote: On 10/16/2013 04:10 PM, Kishon Vijay Abraham I wrote: Do you know if there are users of dwc3 other than exynos5250 and omap5? If not, we should get rid of the old USB PHY altogether. Intel's Baytrail, at least. But they don't use DT. I don't see any use for the generic-phy when the device is enumerated from PCI. If dwc3 can live without phys, there should not be any problem dropping the old USB PHY support. yeah, I don't want to drop PHY support, I want to require everybody to provide a PHY, otherwise we have too many options to handle and that's never clean. I have to clarify in case there was a misunderstanding. When I said generic-phy I meant drivers/usb/phy/phy-generic.c and I was not talking about Kishon's new generic PHY framework. So I was only saying that if the dwc3-pci.c is the only thing that needs the old usb phy support, then there should not be any problem dropping the support for it. Thanks, -- heikki -- 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 04/10] usb: dwc3: use quirks to know if a particualr platform doesn't have PHY
Hi, On Mon, Dec 09, 2013 at 12:43:37PM +0530, Kishon Vijay Abraham I wrote: On Thursday 05 December 2013 01:28 PM, Heikki Krogerus wrote: On Thu, Dec 05, 2013 at 12:04:46PM +0530, Kishon Vijay Abraham I wrote: On Wednesday 04 December 2013 08:10 PM, Heikki Krogerus wrote: On Mon, Nov 25, 2013 at 03:31:24PM +0530, Kishon Vijay Abraham I wrote: There can be systems which does not have an external phy, so get phy only if no quirks are added that indicates the PHY is not present. Introduced two quirk flags to indicate the *absence* of usb2 phy and usb3 phy. Also remove checking if return value is -ENXIO since it's now changed to always enable usb_phy layer. Can you guys explain why is something like this needed? Like with clocks and gpios, the device drivers shouldn't need to care any more if the platform has the phys or not. -ENODEV tells you your platform Shouldn't we report if a particular platform needs a PHY and not able to get it. How will a user know if a particular controller is not working because it's not able to get and initialize the PHYs? Don't you think in such cases it's better to fail (and return from probe) because the controller will not work anyway without the PHY? My point is that you do not need to separately tell this to the driver like you do with the quirks (if you did, then you would need to fix your framework and not hack the drivers). Like I said, ENODEV tells you that there is no phy on this platform for you, allowing you to safely continue. If your phy driver is not loaded, the framework already returns EPROBE_DEFER, right. Any other right. but that doesn't consider broken dt data. With quirks we'll able to tell if a controller in a particular platform has PHY or not without depending on the dt data. Broken dt data? What kind of scenario are you thinking here? Do you mean case where the dt does not describe the phy on a platform that depends on it? Shouldn't that problem be fixed in the dt and not hacked in the drivers? Or are you thinking about something else? Is there a case where something like that is actually happening? Br, -- heikki -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] phy: remove the need for the phys to know about their users
Hi, This replaces the consumer init_data structures with a lookup table that contains complete associations with the phys and their users, removing the need for the phy drivers themselves to care about their users even when not using DT. The lookup method is copied from the way the gpio descriptor lookup is now handled in gpiolib.c. Heikki Krogerus (5): phy: unify the phy name parameters phy: add support for indexed lookup phy: improved lookup method arm: omap3: twl: use the new lookup method with usb phy phy: remove the old lookup method arch/arm/mach-omap2/twl-common.c| 15 ++- drivers/phy/phy-core.c | 209 ++-- drivers/phy/phy-exynos-dp-video.c | 2 +- drivers/phy/phy-exynos-mipi-video.c | 2 +- drivers/phy/phy-omap-usb2.c | 2 +- drivers/phy/phy-twl4030-usb.c | 4 +- include/linux/phy/phy.h | 86 ++- 7 files changed, 221 insertions(+), 99 deletions(-) -- 1.8.5.1 -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] phy: remove the old lookup method
The users of the old method are now converted to the new one.
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
---
Documentation/phy.txt | 100 ++--
drivers/phy/phy-core.c | 46 ++---
drivers/phy/phy-exynos-dp-video.c | 2 +-
drivers/phy/phy-exynos-mipi-video.c | 2 +-
drivers/phy/phy-omap-usb2.c | 2 +-
drivers/phy/phy-twl4030-usb.c | 4 +-
include/linux/phy/phy.h | 36 ++---
7 files changed, 73 insertions(+), 119 deletions(-)
diff --git a/Documentation/phy.txt b/Documentation/phy.txt
index 0103e4b..cfff1a2 100644
--- a/Documentation/phy.txt
+++ b/Documentation/phy.txt
@@ -53,17 +53,13 @@ unregister the PHY.
The PHY driver should create the PHY in order for other peripheral controllers
to make use of it. The PHY framework provides 2 APIs to create the PHY.
-struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
-struct phy_init_data *init_data);
-struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops,
- struct phy_init_data *init_data);
+struct phy *phy_create(struct device *dev, const struct phy_ops *ops);
+struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops);
The PHY drivers can use one of the above 2 APIs to create the PHY by passing
-the device pointer, phy ops and init_data.
+the device pointer, phy ops.
phy_ops is a set of function pointers for performing PHY operations such as
-init, exit, power_on and power_off. *init_data* is mandatory to get a reference
-to the PHY in the case of non-dt boot. See section *Board File Initialization*
-on how init_data should be used.
+init, exit, power_on and power_off.
Inorder to dereference the private data (in phy_ops), the phy provider driver
can use phy_set_drvdata() after creating the PHY and use phy_get_drvdata() in
@@ -74,16 +70,21 @@ phy_ops to get back the private data.
Before the controller can make use of the PHY, it has to get a reference to
it. This framework provides the following APIs to get a reference to the PHY.
-struct phy *phy_get(struct device *dev, const char *string);
-struct phy *devm_phy_get(struct device *dev, const char *string);
+struct phy *phy_get(struct device *dev, const char *con_id);
+struct phy *devm_phy_get(struct device *dev, const char *con_id);
phy_get and devm_phy_get can be used to get the PHY. In the case of dt boot,
-the string arguments should contain the phy name as given in the dt data and
+the con_id arguments should contain the phy name as given in the dt data and
in the case of non-dt boot, it should contain the label of the PHY.
The only difference between the two APIs is that devm_phy_get associates the
device with the PHY using devres on successful PHY get. On driver detach,
release function is invoked on the the devres data and devres data is freed.
+Indexed lookup is also possible when device has multiple PHYs attached to it.
+The framework provides variants for phy_get() functions called phy_get_index()
+and devm_phy_get_index(). They take the same parameters as do the normal
+phy_get() plus the index number of the PHY as the last parameter.
+
5. Releasing a reference to the PHY
When the controller no longer needs the PHY, it has to release the reference
@@ -123,42 +124,63 @@ There are exported APIs like phy_pm_runtime_get,
phy_pm_runtime_get_sync,
phy_pm_runtime_put, phy_pm_runtime_put_sync, phy_pm_runtime_allow and
phy_pm_runtime_forbid for performing PM operations.
-8. Board File Initialization
+8. Platform Data binding
-Certain board file initialization is necessary in order to get a reference
-to the PHY in the case of non-dt boot.
-Say we have a single device that implements 3 PHYs that of USB, SATA and PCIe,
-then in the board file the following initialization should be done.
+PHY's are mapped by the means of tables of lookups, containing instances of the
+phy_lookup structure. Two macros are defined to help declaring such mappings:
-struct phy_consumer consumers[] = {
- PHY_CONSUMER(dwc3.0, usb),
- PHY_CONSUMER(pcie.0, pcie),
- PHY_CONSUMER(sata.0, sata),
-};
-PHY_CONSUMER takes 2 parameters, first is the device name of the controller
-(PHY consumer) and second is the port name.
+ PHY_LOOKUP(phy_name, con_id)
+ PHY_LOOKUP_IDX(phy_name, con_id, idx)
+
+where
-struct phy_init_data init_data = {
- .consumers = consumers,
- .num_consumers = ARRAY_SIZE(consumers),
+ - phy_name identifiers of the phy device
+ - con_id is the name of the PHY from the device point of view. It can be
NULL.
+ - idx is the index of the PHY within the function.
+
+A lookup table can then be defined as follows, with an empty entry defining its
+end:
+
+struct phy_lookup_table phys_table = {
+ .dev_id = usb_controller.0,
+ .table = {
+ PHY_LOOKUP_IDX(phy-usb2phy.0, usb2-phy, 0),
+ PHY_LOOKUP_IDX(phy-usb3phy
[PATCH 2/5] phy: add support for indexed lookup
Removes the need for the consumer drivers requesting the
phys to provide name for the phy. This should ease the use
of the framework considerable when using only one phy, which
is usually the case when except with USB, but it can also
be useful with multiple phys.
This will also reduce noise from the framework when there is
no phy by changing warnings to debug messages.
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
---
drivers/phy/phy-core.c | 106 ++--
include/linux/phy/phy.h | 14 +++
2 files changed, 89 insertions(+), 31 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 1102aef..99dc046 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -53,7 +53,8 @@ static int devm_phy_match(struct device *dev, void *res, void
*match_data)
return res == match_data;
}
-static struct phy *phy_lookup(struct device *device, const char *con_id)
+static struct phy *phy_lookup(struct device *device, const char *con_id,
+ unsigned int idx)
{
unsigned int count;
struct phy *phy;
@@ -67,6 +68,10 @@ static struct phy *phy_lookup(struct device *device, const
char *con_id)
count = phy-init_data-num_consumers;
consumers = phy-init_data-consumers;
while (count--) {
+ /* index must always match exactly */
+ if (!con_id)
+ if (idx != count)
+ continue;
if (!strcmp(consumers-dev_name, dev_name(device))
!strcmp(consumers-port, con_id)) {
class_dev_iter_exit(iter);
@@ -242,7 +247,8 @@ EXPORT_SYMBOL_GPL(phy_power_off);
/**
* of_phy_get() - lookup and obtain a reference to a phy by phandle
* @dev: device that requests this phy
- * @index: the index of the phy
+ * @con_id: name of the phy from device's point of view
+ * @idx: the index of the phy if name is not used
*
* Returns the phy associated with the given phandle value,
* after getting a refcount to it or -ENODEV if there is no such phy or
@@ -250,12 +256,20 @@ EXPORT_SYMBOL_GPL(phy_power_off);
* not yet loaded. This function uses of_xlate call back function provided
* while registering the phy_provider to find the phy instance.
*/
-static struct phy *of_phy_get(struct device *dev, int index)
+static struct phy *of_phy_get(struct device *dev, const char *con_id,
+ unsigned int idx)
{
int ret;
struct phy_provider *phy_provider;
struct phy *phy = NULL;
struct of_phandle_args args;
+ int index;
+
+ if (!con_id)
+ index = idx;
+ else
+ index = of_property_match_string(dev-of_node, phy-names,
+con_id);
ret = of_parse_phandle_with_args(dev-of_node, phys, #phy-cells,
index, args);
@@ -348,38 +362,36 @@ struct phy *of_phy_simple_xlate(struct device *dev,
struct of_phandle_args
EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
/**
- * phy_get() - lookup and obtain a reference to a phy.
+ * phy_get_index() - obtain a phy based on index
* @dev: device that requests this phy
* @con_id: name of the phy from device's point of view
+ * @idx: index of the phy to obtain in the consumer
*
- * Returns the phy driver, after getting a refcount to it; or
- * -ENODEV if there is no such phy. The caller is responsible for
- * calling phy_put() to release that count.
+ * This variant of phy_get() allows to access PHYs other than the first
+ * defined one for functions that define several PHYs.
*/
-struct phy *phy_get(struct device *dev, const char *con_id)
+struct phy *phy_get_index(struct device *dev, const char *con_id,
+ unsigned int idx)
{
- int index = 0;
struct phy *phy = NULL;
- if (con_id == NULL) {
- dev_WARN(dev, missing string\n);
- return ERR_PTR(-EINVAL);
+ if (dev-of_node) {
+ dev_dbg(dev, using device tree for PHY lookup\n);
+ phy = of_phy_get(dev, con_id, idx);
}
- if (dev-of_node) {
- index = of_property_match_string(dev-of_node, phy-names,
-con_id);
- phy = of_phy_get(dev, index);
- if (IS_ERR(phy)) {
- dev_err(dev, unable to find phy\n);
- return phy;
- }
- } else {
- phy = phy_lookup(dev, con_id);
- if (IS_ERR(phy)) {
- dev_err(dev, unable to find phy\n);
- return phy;
- }
+ /**
+* Either we are not using DT, or their lookup did not return
+* a result. In that case, use platform lookup
[PATCH 4/5] arm: omap3: twl: use the new lookup method with usb phy
Provide a complete association for the phy and it's user
(musb) with the new phy_lookup_table.
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
---
arch/arm/mach-omap2/twl-common.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
index b0d54da..972430b 100644
--- a/arch/arm/mach-omap2/twl-common.c
+++ b/arch/arm/mach-omap2/twl-common.c
@@ -91,18 +91,13 @@ void __init omap_pmic_late_init(void)
}
#if defined(CONFIG_ARCH_OMAP3)
-struct phy_consumer consumers[] = {
- PHY_CONSUMER(musb-hdrc.0, usb),
-};
-
-struct phy_init_data init_data = {
- .consumers = consumers,
- .num_consumers = ARRAY_SIZE(consumers),
+static struct phy_lookup_table twl4030_usb_lookup = {
+ .dev_id = musb-hdrc.0,
+ .table = PHY_LOOKUP(phy-twl4030_usb.0, NULL),
};
static struct twl4030_usb_data omap3_usb_pdata = {
.usb_mode = T2_USB_MODE_ULPI,
- .init_data = init_data,
};
static int omap3_batt_table[] = {
@@ -225,8 +220,10 @@ void __init omap3_pmic_get_config(struct
twl4030_platform_data *pmic_data,
}
/* Common platform data configurations */
- if (pdata_flags TWL_COMMON_PDATA_USB !pmic_data-usb)
+ if (pdata_flags TWL_COMMON_PDATA_USB !pmic_data-usb) {
+ phy_add_lookup_table(twl4030_usb_lookup);
pmic_data-usb = omap3_usb_pdata;
+ }
if (pdata_flags TWL_COMMON_PDATA_BCI !pmic_data-bci)
pmic_data-bci = omap3_bci_pdata;
--
1.8.5.1
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] phy: improved lookup method
Removes the need for the phys to be aware of their users
even when not using DT. The method is copied from gpiolib.c.
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
---
drivers/phy/phy-core.c | 91 -
include/linux/phy/phy.h | 48 ++
2 files changed, 138 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 99dc046..05792d0 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -25,6 +25,7 @@
static struct class *phy_class;
static DEFINE_MUTEX(phy_provider_mutex);
static LIST_HEAD(phy_provider_list);
+static LIST_HEAD(phy_lookup_list);
static DEFINE_IDA(phy_ida);
static void devm_phy_release(struct device *dev, void *res)
@@ -85,6 +86,94 @@ static struct phy *phy_lookup(struct device *device, const
char *con_id,
return ERR_PTR(-ENODEV);
}
+/**
+ * phy_add_table() - register PHY/device association to the lookup list
+ * @table: association to register
+ */
+void phy_add_lookup_table(struct phy_lookup_table *table)
+{
+ mutex_lock(phy_provider_mutex);
+ list_add_tail(table-list, phy_lookup_list);
+ mutex_unlock(phy_provider_mutex);
+}
+
+/**
+ * phy_add_table() - remove PHY/device association from the lookup list
+ * @table: association to be removed
+ */
+void phy_del_lookup_table(struct phy_lookup_table *table)
+{
+ mutex_lock(phy_provider_mutex);
+ list_del(table-list);
+ mutex_unlock(phy_provider_mutex);
+}
+
+static struct phy *find_phy_by_name(const char *name)
+{
+ struct class_dev_iter iter;
+ struct phy *phy = NULL;
+ struct device *dev;
+
+ class_dev_iter_init(iter, phy_class, NULL, NULL);
+ while ((dev = class_dev_iter_next(iter))) {
+ if (!strcmp(dev_name(dev), name)) {
+ phy = to_phy(dev);
+ break;
+ }
+ }
+ class_dev_iter_exit(iter);
+
+ return phy;
+}
+
+static struct phy_lookup_table *phy_get_lookup_table(struct device *dev)
+{
+ const char *dev_id = dev ? dev_name(dev) : NULL;
+ struct phy_lookup_table *table;
+
+ mutex_lock(phy_provider_mutex);
+ list_for_each_entry(table, phy_lookup_list, list)
+ if (!strcmp(table-dev_id, dev_id))
+ goto out;
+ table = NULL;
+out:
+ mutex_unlock(phy_provider_mutex);
+ return table;
+}
+
+static struct phy *phy_find(struct device *dev, const char *con_id,
+ unsigned int idx)
+{
+ struct phy_lookup_table *table;
+ struct phy_lookup *p;
+ struct phy *phy;
+
+ table = phy_get_lookup_table(dev);
+ if (!table)
+ /* fall-back to the old lookup method for now */
+ return phy_lookup(dev, con_id, idx);
+
+ for (p = table-table[0]; p-phy_name; p++) {
+ /* index must always match exactly */
+ if (idx != p-idx)
+ continue;
+
+ /* If the lookup entry has a con_id, require exact match */
+ if (p-con_id (!con_id || strcmp(p-con_id, con_id)))
+ continue;
+
+ phy = find_phy_by_name(p-phy_name);
+ if (!phy) {
+ dev_warn(dev, no PHY by the name %s\n, p-phy_name);
+ return ERR_PTR(-ENODEV);
+ }
+
+ return phy;
+ }
+
+ return ERR_PTR(-ENODEV);
+}
+
static struct phy_provider *of_phy_provider_lookup(struct device_node *node)
{
struct phy_provider *phy_provider;
@@ -386,7 +475,7 @@ struct phy *phy_get_index(struct device *dev, const char
*con_id,
*/
if (!phy || IS_ERR(phy)) {
dev_dbg(dev, using lookup tables for PHY lookup);
- phy = phy_lookup(dev, con_id, idx);
+ phy = phy_find(dev, con_id, idx);
}
if (IS_ERR(phy)) {
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 43d1a23..cca363a 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -98,6 +98,54 @@ struct phy_init_data {
.port = _port,\
}
+/**
+ * struct phy_lookup - Lookup entry for associating PHYs
+ * @phy_name: device name of the PHY
+ * @con_id: name of the PHY from device's point of view
+ * @idx: index of the PHY when name is not used
+ */
+struct phy_lookup {
+ const char *phy_name;
+ const char *con_id;
+ unsigned int idx;
+};
+
+/**
+ * struct phy_lookup_table - association of PHYs to specific device
+ * @list: entry in the lookup list
+ * @dev_id: the name of the device
+ * @table: table of PHYs attached to this device
+ */
+struct phy_lookup_table {
+ struct list_head list;
+ const char *dev_id;
+ struct phy_lookup table[];
+};
+
+/**
+ * Simple definition of a single PHY under a consumer
+ */
+#define PHY_LOOKUP
[PATCH 1/5] phy: unify the phy name parameters
Replace string and port that are used as phy name
parameter for various functions with con_id which is
commonly used in other frameworks.
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
---
drivers/phy/phy-core.c | 22 ++
include/linux/phy/phy.h | 8
2 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 03cf8fb..1102aef 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -53,7 +53,7 @@ static int devm_phy_match(struct device *dev, void *res, void
*match_data)
return res == match_data;
}
-static struct phy *phy_lookup(struct device *device, const char *port)
+static struct phy *phy_lookup(struct device *device, const char *con_id)
{
unsigned int count;
struct phy *phy;
@@ -68,7 +68,7 @@ static struct phy *phy_lookup(struct device *device, const
char *port)
consumers = phy-init_data-consumers;
while (count--) {
if (!strcmp(consumers-dev_name, dev_name(device))
- !strcmp(consumers-port, port)) {
+ !strcmp(consumers-port, con_id)) {
class_dev_iter_exit(iter);
return phy;
}
@@ -350,33 +350,32 @@ EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
/**
* phy_get() - lookup and obtain a reference to a phy.
* @dev: device that requests this phy
- * @string: the phy name as given in the dt data or the name of the controller
- * port for non-dt case
+ * @con_id: name of the phy from device's point of view
*
* Returns the phy driver, after getting a refcount to it; or
* -ENODEV if there is no such phy. The caller is responsible for
* calling phy_put() to release that count.
*/
-struct phy *phy_get(struct device *dev, const char *string)
+struct phy *phy_get(struct device *dev, const char *con_id)
{
int index = 0;
struct phy *phy = NULL;
- if (string == NULL) {
+ if (con_id == NULL) {
dev_WARN(dev, missing string\n);
return ERR_PTR(-EINVAL);
}
if (dev-of_node) {
index = of_property_match_string(dev-of_node, phy-names,
- string);
+con_id);
phy = of_phy_get(dev, index);
if (IS_ERR(phy)) {
dev_err(dev, unable to find phy\n);
return phy;
}
} else {
- phy = phy_lookup(dev, string);
+ phy = phy_lookup(dev, con_id);
if (IS_ERR(phy)) {
dev_err(dev, unable to find phy\n);
return phy;
@@ -395,14 +394,13 @@ EXPORT_SYMBOL_GPL(phy_get);
/**
* devm_phy_get() - lookup and obtain a reference to a phy.
* @dev: device that requests this phy
- * @string: the phy name as given in the dt data or phy device name
- * for non-dt case
+ * @con_id: name of the phy from device's point of view
*
* Gets the phy using phy_get(), and associates a device with it using
* devres. On driver detach, release function is invoked on the devres data,
* then, devres data is freed.
*/
-struct phy *devm_phy_get(struct device *dev, const char *string)
+struct phy *devm_phy_get(struct device *dev, const char *con_id)
{
struct phy **ptr, *phy;
@@ -410,7 +408,7 @@ struct phy *devm_phy_get(struct device *dev, const char
*string)
if (!ptr)
return ERR_PTR(-ENOMEM);
- phy = phy_get(dev, string);
+ phy = phy_get(dev, con_id);
if (!IS_ERR(phy)) {
*ptr = phy;
devres_add(dev, ptr);
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..d67dcbf 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -127,8 +127,8 @@ int phy_init(struct phy *phy);
int phy_exit(struct phy *phy);
int phy_power_on(struct phy *phy);
int phy_power_off(struct phy *phy);
-struct phy *phy_get(struct device *dev, const char *string);
-struct phy *devm_phy_get(struct device *dev, const char *string);
+struct phy *phy_get(struct device *dev, const char *con_id);
+struct phy *devm_phy_get(struct device *dev, const char *con_id);
void phy_put(struct phy *phy);
void devm_phy_put(struct device *dev, struct phy *phy);
struct phy *of_phy_simple_xlate(struct device *dev,
@@ -199,12 +199,12 @@ static inline int phy_power_off(struct phy *phy)
return -ENOSYS;
}
-static inline struct phy *phy_get(struct device *dev, const char *string)
+static inline struct phy *phy_get(struct device *dev, const char *con_id)
{
return ERR_PTR(-ENOSYS);
}
-static inline struct phy *devm_phy_get(struct device *dev, const char *string)
+static inline struct phy *devm_phy_get(struct device *dev, const char *con_id)
{
return
Re: [PATCH v3 04/10] usb: dwc3: use quirks to know if a particualr platform doesn't have PHY
Hi guys, Kishon, sorry I did not see this v3 set. On Mon, Nov 25, 2013 at 03:31:24PM +0530, Kishon Vijay Abraham I wrote: There can be systems which does not have an external phy, so get phy only if no quirks are added that indicates the PHY is not present. Introduced two quirk flags to indicate the *absence* of usb2 phy and usb3 phy. Also remove checking if return value is -ENXIO since it's now changed to always enable usb_phy layer. Can you guys explain why is something like this needed? Like with clocks and gpios, the device drivers shouldn't need to care any more if the platform has the phys or not. -ENODEV tells you your platform does not have them, so go ahead and continue when getting the phy. I just tested removing the phy registering from dwc3-pci.c and all I had to do was to add IS_ERR(phy) checks where the old usb phys were used and everything worked fine? What am I missing? Br, -- heikki -- 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 04/10] usb: dwc3: use quirks to know if a particualr platform doesn't have PHY
Hi, On Thu, Dec 05, 2013 at 12:04:46PM +0530, Kishon Vijay Abraham I wrote: On Wednesday 04 December 2013 08:10 PM, Heikki Krogerus wrote: On Mon, Nov 25, 2013 at 03:31:24PM +0530, Kishon Vijay Abraham I wrote: There can be systems which does not have an external phy, so get phy only if no quirks are added that indicates the PHY is not present. Introduced two quirk flags to indicate the *absence* of usb2 phy and usb3 phy. Also remove checking if return value is -ENXIO since it's now changed to always enable usb_phy layer. Can you guys explain why is something like this needed? Like with clocks and gpios, the device drivers shouldn't need to care any more if the platform has the phys or not. -ENODEV tells you your platform Shouldn't we report if a particular platform needs a PHY and not able to get it. How will a user know if a particular controller is not working because it's not able to get and initialize the PHYs? Don't you think in such cases it's better to fail (and return from probe) because the controller will not work anyway without the PHY? My point is that you do not need to separately tell this to the driver like you do with the quirks (if you did, then you would need to fix your framework and not hack the drivers). Like I said, ENODEV tells you that there is no phy on this platform for you, allowing you to safely continue. If your phy driver is not loaded, the framework already returns EPROBE_DEFER, right. Any other error when getting the phy you can consider critical. They are the errors telling you that you do need a phy on this platform, but something actually went wrong when getting it. Those quirks should always be avoided, and I don't see any use for them here. Thanks Kishon Thanks, -- heikki -- 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 1/7] usb: dwc3: get usb_phy only if the platform indicates the presence of PHY's
Hi, On Thu, Oct 17, 2013 at 09:54:26AM -0500, Felipe Balbi wrote: On Wed, Oct 16, 2013 at 04:27:26PM +0300, Roger Quadros wrote: On 10/16/2013 04:10 PM, Kishon Vijay Abraham I wrote: Do you know if there are users of dwc3 other than exynos5250 and omap5? If not, we should get rid of the old USB PHY altogether. Intel's Baytrail, at least. But they don't use DT. I don't see any use for the generic-phy when the device is enumerated from PCI. If dwc3 can live without phys, there should not be any problem dropping the old USB PHY support. Br, -- heikki -- 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 2/7] usb: dwc3: adapt dwc3 core to use Generic PHY Framework
Hi Kishon,
On Wed, Oct 16, 2013 at 01:24:12AM +0530, Kishon Vijay Abraham I wrote:
+ count = of_property_match_string(node, phy-names, usb2-phy);
+ if (count = 0 || (pdata pdata-usb2_generic_phy)) {
+ dwc-usb2_generic_phy = devm_phy_get(dev, usb2-phy);
+ if (IS_ERR(dwc-usb2_generic_phy)) {
+ dev_err(dev, no usb2 phy configured yet);
+ return PTR_ERR(dwc-usb2_generic_phy);
+ }
+ dwc-usb2_phy = NULL;
+ }
+
+ count = of_property_match_string(node, phy-names, usb3-phy);
+ if (count = 0 || (pdata pdata-usb3_generic_phy)) {
+ dwc-usb3_generic_phy = devm_phy_get(dev, usb3-phy);
+ if (IS_ERR(dwc-usb3_generic_phy)) {
+ dev_err(dev, no usb3 phy configured yet);
+ return PTR_ERR(dwc-usb3_generic_phy);
+ }
+ dwc-usb3_phy = NULL;
+ }
Is there some specific reason for these checks? The driver should not
need to care about the platform (DT, ACPI, platform based).
Just get the phys and check the return value. In case ERR_PTR(-ENODEV)
leave the phy as NULL and let the driver continue normally. With other
errors you make the dwc3 probe fail.
Thanks,
--
heikki
--
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: rx51: initialize isp1704 properly
Hi Felipe, On Wed, Dec 07, 2011 at 09:56:56PM +0200, Felipe Contreras wrote: From: Heikki Krogerus kro...@gmail.com I would prefer that you change this so this is from you, like it actually is. You can mention me in the commit message if you like. Without this USB networking doesn't work as the link is never detected. What this changes is, it leaves the transceiver powered by default instead of setting it into power off mode. So not only USB networking is affected by this. If isp1704_charger is not enabled with RX51, USB does not work at all. Please change the comment. Thanks, -- heikki -- 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/2] RX-51: Enable isp1704 power on/off
Hi, On Wed, Mar 30, 2011 at 08:08:54AM +0300, ext Heikki Krogerus wrote: On Tue, Mar 29, 2011 at 04:28:00PM +0300, Kalle Jokiniemi wrote: The isp1704 usb tranceiver is used for charging and can be disabled when not in use. Provide the powering routine to the driver via platform data. Also changed the indent of .name variable in rx51_charger_device definition to use tabs same way as the new .dev variable indent. Put this in the same patch since the indent fix is only needed when there are multiple members in the struct definition. Loosely based on earlier patches from Heikki Krogerus in Nokia N900 maemo kernel. Signed-off-by: Kalle Jokiniemi kalle.jokini...@nokia.com Cc: Heikki Krogerus heikki.kroge...@nokia.com Acked-By: Heikki Krogerus heikki.kroge...@nokia.com Before Anton takes these, this one needs ack from Tony. -- heikki -- 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/2] isp1704_charger: allow board specific powering routine
Hi, On Tue, Mar 29, 2011 at 04:27:59PM +0300, Kalle Jokiniemi wrote: The ISP1704/1707 chip can be put to full power down state by asserting the CHIP_SEL line. This patch enables platform or board specific hooks to put the device into power down mode in case not needed. This patch is a preparation for enabling this powering routine in n900 (rx-51) devices. Thanks to Heikki Krogerus for helping out with the patch. Signed-off-by: Kalle Jokiniemi kalle.jokini...@nokia.com Cc: Heikki Krogerus heikki.kroge...@nokia.com Acked-By: Heikki Krogerus heikki.kroge...@nokia.com --- drivers/power/isp1704_charger.c | 22 ++ include/linux/power/isp1704_charger.h | 29 + 2 files changed, 51 insertions(+), 0 deletions(-) create mode 100644 include/linux/power/isp1704_charger.h -- heikki -- 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/2] RX-51: Enable isp1704 power on/off
On Tue, Mar 29, 2011 at 04:28:00PM +0300, Kalle Jokiniemi wrote: The isp1704 usb tranceiver is used for charging and can be disabled when not in use. Provide the powering routine to the driver via platform data. Also changed the indent of .name variable in rx51_charger_device definition to use tabs same way as the new .dev variable indent. Put this in the same patch since the indent fix is only needed when there are multiple members in the struct definition. Loosely based on earlier patches from Heikki Krogerus in Nokia N900 maemo kernel. Signed-off-by: Kalle Jokiniemi kalle.jokini...@nokia.com Cc: Heikki Krogerus heikki.kroge...@nokia.com Acked-By: Heikki Krogerus heikki.kroge...@nokia.com -- heikki -- 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 1/2] isp1704_charger: allow board specific powering routine
Hi,
Add Anton Vorontsov cbouatmai...@gmail.com to your v3. This will
need ack from him, or this needs to go to him. In this case I guess we
are only dealing with RX51 stuff, so maybe this should go to Tony.
On Mon, Mar 28, 2011 at 09:51:38AM +0300, Kalle Jokiniemi wrote:
The ISP1704/1707 chip can be put to full power down
state by asserting the CHIP_SEL line. This patch enables
platform or board specific hooks to put the device into
power down mode in case not needed.
These patches are preparatio for enabling this powering
routine in n900 (rx-51) devices.
Thanks to Heikki Krogerus for helping out with the patch.
Signed-off-by: Kalle Jokiniemi kalle.jokini...@nokia.com
Cc: Heikki Krogerus heikki.kroge...@nokia.com
---
drivers/power/isp1704_charger.c | 26 ++
include/linux/power/isp1704_charger.h | 29 +
2 files changed, 55 insertions(+), 0 deletions(-)
create mode 100644 include/linux/power/isp1704_charger.h
diff --git a/drivers/power/isp1704_charger.c b/drivers/power/isp1704_charger.c
index 2ad9b14..c796b9f 100644
--- a/drivers/power/isp1704_charger.c
+++ b/drivers/power/isp1704_charger.c
@@ -33,6 +33,7 @@
#include linux/usb/ulpi.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
+#include linux/power/isp1704_charger.h
/* Vendor specific Power Control register */
#define ISP1704_PWR_CTRL 0x3d
@@ -63,6 +64,7 @@ struct isp1704_charger {
charmodel[8];
unsignedpresent:1;
unsignedonline:1;
+ unsignedinit_done;
Do we need this?
unsignedcurrent_max;
/* temp storage variables */
@@ -71,6 +73,18 @@ struct isp1704_charger {
};
/*
+ * Disable/enable the power from the isp1704 if a function for it
+ * has been provided with platform data.
+ */
+static void isp1704_charger_set_power(struct isp1704_charger *isp, bool on)
+{
+ struct isp1704_charger_data *board = isp-dev-platform_data;
+
+ if (board-set_power)
+ board-set_power(on);
+}
+
+/*
* Determine is the charging port DCP (dedicated charger) or CDP (Host/HUB
* chargers).
*
@@ -222,6 +236,9 @@ static void isp1704_charger_work(struct work_struct *data)
mutex_lock(lock);
+ if (event != USB_EVENT_NONE)
+ isp1704_charger_set_power(isp, 1);
+
switch (event) {
case USB_EVENT_VBUS:
isp-online = true;
@@ -269,6 +286,9 @@ static void isp1704_charger_work(struct work_struct *data)
*/
if (isp-otg-gadget)
usb_gadget_disconnect(isp-otg-gadget);
+ /* If we're initialized, we can power down the isp */
+ if (isp-init_done)
+ isp1704_charger_set_power(isp, 0);
break;
case USB_EVENT_ENUMERATED:
if (isp-present)
@@ -394,6 +414,8 @@ static int __devinit isp1704_charger_probe(struct
platform_device *pdev)
isp-dev = pdev-dev;
platform_set_drvdata(pdev, isp);
+ isp1704_charger_set_power(isp, 1);
+
ret = isp1704_test_ulpi(isp);
if (ret 0)
goto fail1;
@@ -437,8 +459,11 @@ static int __devinit isp1704_charger_probe(struct
platform_device *pdev)
if ((ret ULPI_INT_VBUS_VALID) !isp-otg-default_a) {
isp-event = USB_EVENT_VBUS;
schedule_work(isp-work);
+ } else {
+ isp1704_charger_set_power(isp, 0);
}
I think the transceiver can be powered down even if we just scheduled
the work. This will basically cause hw reset on the transceiver which
is only a good thing IMO. Drop the else condition.
+ isp-init_done = 1;
return 0;
fail2:
power_supply_unregister(isp-psy);
@@ -459,6 +484,7 @@ static int __devexit isp1704_charger_remove(struct
platform_device *pdev)
otg_unregister_notifier(isp-otg, isp-nb);
power_supply_unregister(isp-psy);
otg_put_transceiver(isp-otg);
+ isp1704_charger_set_power(isp, 0);
kfree(isp);
return 0;
--
heikki
--
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] OMAP3: rx51: specify phy_power for usb tranceiver
Hi,
On Mon, Mar 21, 2011 at 03:50:20PM +0200, Kalle Jokiniemi wrote:
This patch allows the ISP1707 USB tranceiver on Nokia
N900 to be disabled when usb cable is disconnected.
This saves approximately 14mA of battery current.
Patch based on work done by Heikki Krogerus on N900
maemo kernel.
Signed-off-by: Kalle Jokiniemi kalle.jokini...@nokia.com
Cc: Heikki Krogerus heikki.kroge...@nokia.com
---
arch/arm/mach-omap2/board-rx51-peripherals.c | 32
++
1 files changed, 32 insertions(+), 0 deletions(-)
snip
+static void __init rx51_xceiv_init(void)
+{
+ if (gpio_request(RX51_USB_TRANSCEIVER_RST_GPIO, NULL) 0)
+ BUG();
+ gpio_direction_output(RX51_USB_TRANSCEIVER_RST_GPIO, 1);
+}
+
+static int rx51_xceiv_power(struct device *dev, int iD, int on)
+{
+ unsigned long timeout;
+
+ if (!on) {
+ /* Let musb go stdby before powering down the transceiver */
+ timeout = jiffies + msecs_to_jiffies(100);
+ while (!time_after(jiffies, timeout))
+ if (omap2_cm_read_mod_reg(CORE_MOD, CM_IDLEST1)
+ OMAP3430ES2_ST_HSOTGUSB_STDBY_MASK)
+ break;
+ if (!(omap2_cm_read_mod_reg(CORE_MOD, CM_IDLEST1)
+ OMAP3430ES2_ST_HSOTGUSB_STDBY_MASK))
+ WARN(1, could not put musb to sleep\n);
+ }
+ gpio_set_value(RX51_USB_TRANSCEIVER_RST_GPIO, on);
+
+ return 0;
+}
The busy loop is not needed, and not what we want. We need to be able
to toggle the CHIP_SEL even if the USB block is not IDLE or STDBY.
The guys have already pointed out some issues with this code, so
please rework these functions.
static struct twl4030_usb_data rx51_usb_data = {
.usb_mode = T2_USB_MODE_ULPI,
+ .phy_power = rx51_xceiv_power,
};
This is not the right place for this. The gpio controls isp1707, not
the twl4030_usb. You have the rx51_charger_device platform device for
isp1707. Add them there.
--
heikki
--
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/2] USB: twl4030-usb: do board specific phy_power up/down
Hi,
On Mon, Mar 21, 2011 at 03:50:19PM +0200, Kalle Jokiniemi wrote:
In case some board has special powering sequences for
the USB tranceiver, call those during __twl4030_phy_power
calls.
This is a preparation patch to allow powering down the
ISP1707 USB serial tranceiver in Nokia N900.
Signed-off-by: Kalle Jokiniemi kalle.jokini...@nokia.com
---
drivers/usb/otg/twl4030-usb.c |9 -
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/otg/twl4030-usb.c b/drivers/usb/otg/twl4030-usb.c
index 6ca505f..dea99b1 100644
--- a/drivers/usb/otg/twl4030-usb.c
+++ b/drivers/usb/otg/twl4030-usb.c
@@ -348,13 +348,20 @@ static void twl4030_i2c_access(struct twl4030_usb *twl,
int on)
static void __twl4030_phy_power(struct twl4030_usb *twl, int on)
{
- u8 pwr = twl4030_usb_read(twl, PHY_PWR_CTRL);
+ u8 pwr;
+ struct twl4030_usb_data *board = twl-dev-platform_data;
+
+ pwr = twl4030_usb_read(twl, PHY_PWR_CTRL);
if (on)
pwr = ~PHY_PWR_PHYPWD;
else
pwr |= PHY_PWR_PHYPWD;
+ /* do board specific power up/down, if available */
+ if (board-phy_power)
+ board-phy_power(twl-dev, 0, on);
+
WARN_ON(twl4030_usb_write_verify(twl, PHY_PWR_CTRL, pwr) 0);
}
Wrong driver. We are controlling isp1707. The driver is
drivers/power/isp1704_charger.c.
--
heikki
--
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: twl4030-usb: fix isp1707 xceiver powering
Hi Kalle, Missed the cover letter so I already commented the patches. On Mon, Mar 21, 2011 at 03:50:18PM +0200, Kalle Jokiniemi wrote: These two patches introduce phy_power calls form board files to twl4030-usb. This fixes a problem in Nokia N900, where the ISP1707 serial tranceiver did not get disabled during phy power down. Based on patches used on n900 maemo kernel, mainly from Heikki Krogerus. Commentstesting appreciated. Basic test done on MeeGo + linux usb that the ISP powering happens correctly. Also simple data transfer tests done with backported patches on MeeGo n900 kernel. Cross posting to linux-omap, but I propose that we'd push these to linux-usb. Patches based on linux-usb master. Kalle Jokiniemi (2): USB: twl4030-usb: do board specific phy_power up/down OMAP3: rx51: specify phy_power for usb tranceiver arch/arm/mach-omap2/board-rx51-peripherals.c | 32 ++ drivers/usb/otg/twl4030-usb.c|9 ++- 2 files changed, 40 insertions(+), 1 deletions(-) I do like the idea of getting the support for power down mode of isp1707 in RX51 but... If I understood correctly, meego has the isp1704_charger driver from mainline, so we don't need to involve twl4030-usb when powering the isp1707 like we did in maemo kernel. -- heikki -- 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] OMAP3: rx51: specify phy_power for usb tranceiver
Hi,
On Tue, Mar 22, 2011 at 12:24:27PM +0200, Jokiniemi Kalle (Nokia-MS/Tampere)
wrote:
+static int rx51_xceiv_power(struct device *dev, int iD, int on)
+{
+unsigned long timeout;
+
+if (!on) {
+/* Let musb go stdby before powering down the
transceiver */
+timeout = jiffies + msecs_to_jiffies(100);
+while (!time_after(jiffies, timeout))
+if (omap2_cm_read_mod_reg(CORE_MOD,
CM_IDLEST1)
+
OMAP3430ES2_ST_HSOTGUSB_STDBY_MASK)
+break;
+if (!(omap2_cm_read_mod_reg(CORE_MOD, CM_IDLEST1)
+ OMAP3430ES2_ST_HSOTGUSB_STDBY_MASK))
+WARN(1, could not put musb to sleep\n);
+}
+gpio_set_value(RX51_USB_TRANSCEIVER_RST_GPIO, on);
+
+return 0;
+}
The busy loop is not needed, and not what we want. We need to be able
to toggle the CHIP_SEL even if the USB block is not IDLE or STDBY.
I basically just took what was in the maemo kernel. Was there some reason
originally to include the busyloop? Do I get it now correctly that the ISP is
only needed active when we are charging?
OK, my comment was incorrect. The gpio is clearly set regardless of the
outcome of the loop. To answer your question, we only need the ISP to
be active when _detecting_ the charger and obviously when USB is in
use.
We had problems hitting off-mode if we switched the transceiver off
before the controller had entered STDBY. I think this needs to be
tested again. The code in drivers/usb/musb/omap2430.c is quite
different then what we had in maemo kernel. It could be that there is
no need for the loop anymore. If you want to play it safe, fix it and
leave it there.
--
heikki
--
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/1] power_supply: twl4030_charger modification
On Sat, Feb 26, 2011 at 08:10:17PM +0200, ext Grazvydas Ignotas wrote: On Fri, Feb 25, 2011 at 2:12 PM, Felipe Balbi ba...@ti.com wrote: On Fri, Feb 25, 2011 at 01:56:35PM +0200, Heikki Krogerus wrote: Hi guys, Grazvydas, I don't have hardware to test this with. If this is OK to you, could you test it? Sure, replied to your patch. Thanks! Felipe, was the idea that you take this with the atomic notifier support for otg patches? I guess this is how it needs to be done in order to keep things bisectable, right? Or does this simply go to Anton? We can't change patches anymore :-( So as long as it goes on the merge window, we should be ok, hopefully. Talking about notifiers, do we have away to find the amount of current host can provide yet? I haven't followed USB development closely lately. There is nothing to support this yet. I'm only assuming the ENUMERATED event will be used for this. Now with the atomic notifiers it should be OK for the controller drivers to send this notification always when usb_gadget_vbus_draw() is called. But Felipe is working with the udc class. Maybe he has some other method in mind? For now I think we could send the ENUMERATED event from all the controller drivers. Felipe? -- heikki -- 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] twl4030_charger: Make the driver atomic notifier safe
On Sat, Feb 26, 2011 at 07:53:03PM +0200, ext Grazvydas Ignotas wrote: On Fri, Feb 25, 2011 at 1:56 PM, Heikki Krogerus heikki.kroge...@nokia.com wrote: This queues work from the otg notification where the i2c operations can be safely made. Needed for atomic otg notifiers. Signed-off-by: Heikki Krogerus heikki.kroge...@nokia.com Cc: Grazvydas Ignotas nota...@gmail.com Cc: Anton Vorontsov cbouatmai...@gmail.com seems to work fine. Tested-by: Grazvydas Ignotas nota...@gmail.com Anton, can you take this? -- heikki -- 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.6.39 12/12] usb: otg: notifier: switch to atomic notifier
Hi Felipe, On Thu, Feb 17, 2011 at 02:38:49PM +0200, ext Felipe Balbi wrote: most of our notifications, will be called from IRQ context, so an atomic notifier suits the job better. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/usb/otg/ab8500-usb.c|6 +++--- drivers/usb/otg/nop-usb-xceiv.c |2 +- drivers/usb/otg/twl4030-usb.c |6 +++--- drivers/usb/otg/twl6030-usb.c |8 include/linux/usb/otg.h |6 +++--- 5 files changed, 14 insertions(+), 14 deletions(-) You should fix drivers/usb/otg/ab8500-usb.c as well. Also, drivers/power/twl4030_charger.c will need to be made atomic notifier safe. -- heikki -- 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.6.39 12/12] usb: otg: notifier: switch to atomic notifier
On Fri, Feb 25, 2011 at 12:46:53PM +0200, ext Heikki Krogerus wrote: Hi Felipe, On Thu, Feb 17, 2011 at 02:38:49PM +0200, ext Felipe Balbi wrote: most of our notifications, will be called from IRQ context, so an atomic notifier suits the job better. Signed-off-by: Felipe Balbi ba...@ti.com --- drivers/usb/otg/ab8500-usb.c|6 +++--- drivers/usb/otg/nop-usb-xceiv.c |2 +- drivers/usb/otg/twl4030-usb.c |6 +++--- drivers/usb/otg/twl6030-usb.c |8 include/linux/usb/otg.h |6 +++--- 5 files changed, 14 insertions(+), 14 deletions(-) You should fix drivers/usb/otg/ab8500-usb.c as well. Sorry, ignore that :). drivers/power/twl4030_charger.c will need to be made atomic notifier safe. Still valid I belive. -- heikki -- 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.6.39 12/12] usb: otg: notifier: switch to atomic notifier
On Fri, Feb 25, 2011 at 12:53:17PM +0200, ext Felipe Balbi wrote: drivers/power/twl4030_charger.c will need to be made atomic notifier safe. oops, that thing is doing i2c transfers. Would you care to send a patch since you have that hardware easily available ?? I can make a patch for this if you like, but I don't have the hw. -- heikki -- 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.6.39 12/12] usb: otg: notifier: switch to atomic notifier
On Fri, Feb 25, 2011 at 01:24:00PM +0200, ext Felipe Balbi wrote: On Fri, Feb 25, 2011 at 01:07:41PM +0200, Heikki Krogerus wrote: On Fri, Feb 25, 2011 at 12:53:17PM +0200, ext Felipe Balbi wrote: drivers/power/twl4030_charger.c will need to be made atomic notifier safe. oops, that thing is doing i2c transfers. Would you care to send a patch since you have that hardware easily available ?? I can make a patch for this if you like, but I don't have the hw. Aha, I got confused with the isp1710, sorry. If you can cook a patch please do and Cc Grazvydas Ignotas nota...@gmail.com when you send it ;-) OK, np. -- heikki -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/1] power_supply: twl4030_charger modification
Hi guys, Grazvydas, I don't have hardware to test this with. If this is OK to you, could you test it? Felipe, was the idea that you take this with the atomic notifier support for otg patches? I guess this is how it needs to be done in order to keep things bisectable, right? Or does this simply go to Anton? Heikki Krogerus (1): twl4030_charger: Make the driver atomic notifier safe drivers/power/twl4030_charger.c | 25 +++-- 1 files changed, 19 insertions(+), 6 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] twl4030_charger: Make the driver atomic notifier safe
This queues work from the otg notification where the
i2c operations can be safely made. Needed for atomic otg
notifiers.
Signed-off-by: Heikki Krogerus heikki.kroge...@nokia.com
Cc: Grazvydas Ignotas nota...@gmail.com
Cc: Anton Vorontsov cbouatmai...@gmail.com
---
drivers/power/twl4030_charger.c | 25 +++--
1 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/power/twl4030_charger.c b/drivers/power/twl4030_charger.c
index ff1f423..92c16e1 100644
--- a/drivers/power/twl4030_charger.c
+++ b/drivers/power/twl4030_charger.c
@@ -71,8 +71,11 @@ struct twl4030_bci {
struct power_supply usb;
struct otg_transceiver *transceiver;
struct notifier_block otg_nb;
+ struct work_struct work;
int irq_chg;
int irq_bci;
+
+ unsigned long event;
};
/*
@@ -258,14 +261,11 @@ static irqreturn_t twl4030_bci_interrupt(int irq, void
*arg)
return IRQ_HANDLED;
}
-static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
- void *priv)
+static void twl4030_bci_usb_work(struct work_struct *data)
{
- struct twl4030_bci *bci = container_of(nb, struct twl4030_bci, otg_nb);
+ struct twl4030_bci *bci = container_of(data, struct twl4030_bci, work);
- dev_dbg(bci-dev, OTG notify %lu\n, val);
-
- switch (val) {
+ switch (bci-event) {
case USB_EVENT_VBUS:
case USB_EVENT_CHARGER:
twl4030_charger_enable_usb(bci, true);
@@ -274,6 +274,17 @@ static int twl4030_bci_usb_ncb(struct notifier_block *nb,
unsigned long val,
twl4030_charger_enable_usb(bci, false);
break;
}
+}
+
+static int twl4030_bci_usb_ncb(struct notifier_block *nb, unsigned long val,
+ void *priv)
+{
+ struct twl4030_bci *bci = container_of(nb, struct twl4030_bci, otg_nb);
+
+ dev_dbg(bci-dev, OTG notify %lu\n, val);
+
+ bci-event = val;
+ schedule_work(bci-work);
return NOTIFY_OK;
}
@@ -466,6 +477,8 @@ static int __init twl4030_bci_probe(struct platform_device
*pdev)
goto fail_bci_irq;
}
+ INIT_WORK(bci-work, twl4030_bci_usb_work);
+
bci-transceiver = otg_get_transceiver();
if (bci-transceiver != NULL) {
bci-otg_nb.notifier_call = twl4030_bci_usb_ncb;
--
1.7.1
--
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] power_supply: add isp1704 charger detection driver
Hi,
On Wed, Aug 18, 2010 at 03:42:37PM +0200, ext Anton Vorontsov wrote:
On Wed, Aug 18, 2010 at 04:01:05PM +0300, Krogerus Heikki
(EXT-Teleca/Helsinki) wrote:
NXP ISP1704 is Battery Charging Specification 1.0 compliant USB
transceiver. This adds a power supply driver for ISP1704 and
ISP1707 USB transceivers.
Signed-off-by: Heikki Krogerus ext-heikki.kroge...@nokia.com
---
I like this driver (very). A few, mostly cosmetic comments down
below.
[...]
+config CHARGER_ISP1704
+ tristate ISP1704 USB Charger Detection
+ select NOP_USB_XCEIV if ! MACH_NOKIA_RX51
+ help
! MACH_NOKIA_RX51 looks wrong here. You probably don't actually
need this if you fix platform device registration issue (see the
very bottom of this email).
[...]
+/*
+ * ISP1704 detects PS/2 adapters as charger. To make sure the detected
charger
+ * is actually a dedicated charger, the following steps need to be taken.
+ */
+static inline int isp1704_charger_verify(struct isp1704_charger *isp)
+{
+u8 r, ret = 0;
Please put variable declarations on separate lines, plus,
'ret' should probably be int, i.e.
int ret;
u8 r;
OK
+
+/* Reset the transceiver */
+r = otg_io_read(isp-otg, ULPI_FUNC_CTRL);
+r |= ULPI_FUNC_CTRL_RESET;
+otg_io_write(isp-otg, ULPI_FUNC_CTRL, r);
+usleep_range(1000, 2000);
+
+/* Set normal mode */
+r = ~(ULPI_FUNC_CTRL_RESET | ULPI_FUNC_CTRL_OPMODE_MASK);
+otg_io_write(isp-otg, ULPI_FUNC_CTRL, r);
+
+/* Clear the DP and DM pull-down bits */
+r = ULPI_OTG_CTRL_DP_PULLDOWN | ULPI_OTG_CTRL_DM_PULLDOWN;
+otg_io_write(isp-otg, ULPI_CLR(ULPI_OTG_CTRL), r);
+
+/* Enable strong pull-up on DP (1.5K) and reset */
+r = ULPI_FUNC_CTRL_TERMSELECT | ULPI_FUNC_CTRL_RESET;
+otg_io_write(isp-otg, ULPI_SET(ULPI_FUNC_CTRL), r);
+usleep_range(1000, 2000);
+
+/* Read the line state */
+if (otg_io_read(isp-otg, ULPI_DEBUG)) {
+/* Is it a charger or PS/2 connection */
+
+/* Enable weak pull-up resistor on DP */
+otg_io_write(isp-otg, ULPI_SET(ISP1704_PWR_CTRL),
+ISP1704_PWR_CTRL_DP_WKPU_EN);
+
+/* Disable strong pull-up on DP (1.5K) */
+otg_io_write(isp-otg, ULPI_CLR(ULPI_FUNC_CTRL),
+ULPI_FUNC_CTRL_TERMSELECT);
+
+/* Enable weak pull-down resistor on DM */
+otg_io_write(isp-otg, ULPI_SET(ULPI_OTG_CTRL),
+ULPI_OTG_CTRL_DM_PULLDOWN);
+
+/* It's a charger if the line states are clear */
+if (!(otg_io_read(isp-otg, ULPI_DEBUG)))
+ret = 1;
+
+/* Disable weak pull-up resistor on DP */
+otg_io_write(isp-otg, ULPI_CLR(ISP1704_PWR_CTRL),
+ISP1704_PWR_CTRL_DP_WKPU_EN);
+} else {
+ret = 1;
+
+/* Disable strong pull-up on DP (1.5K) */
+otg_io_write(isp-otg, ULPI_CLR(ULPI_FUNC_CTRL),
+ULPI_FUNC_CTRL_TERMSELECT);
+}
How about
if (!otg_io_read(isp-otg, ULPI_DEBUG)) {
/* Disable strong pull-up on DP (1.5K) */
otg_io_write(isp-otg, ULPI_CLR(ULPI_FUNC_CTRL),
ULPI_FUNC_CTRL_TERMSELECT);
return 1;
}
the rest
Saves indentation level.
Yes, makes sense.
+return ret;
+}
+
+static inline int isp1704_charger_detect(struct isp1704_charger *isp)
+{
+unsigned long timeout;
+u8 r;
+int vdat;
+
+/* set SW control bit in PWR_CTRL register */
+otg_io_write(isp-otg, ISP1704_PWR_CTRL,
+ISP1704_PWR_CTRL_SWCTRL);
+
+/* enable manual charger detection */
+r = (ISP1704_PWR_CTRL_SWCTRL | ISP1704_PWR_CTRL_DPVSRC_EN);
+otg_io_write(isp-otg, ULPI_SET(ISP1704_PWR_CTRL), r);
+usleep_range(1000, 2000);
+
+timeout = jiffies + msecs_to_jiffies(300);
+while (!time_after(jiffies, timeout)) {
I guess it is possible that vdat might become uninitialized
if the code never hits the loop. The time between
timeout = jiffies + msecs_to_jiffies(300);
and
while (!time_after(jiffies, timeout)) {
is undefined.
So do while loop it is.
+/* Check if there is a charger */
+vdat = !!(otg_io_read(isp-otg, ISP1704_PWR_CTRL)
+ ISP1704_PWR_CTRL_VDAT_DET);
Technically, there is no need for !!.
VDAT_DET is the fifth bit, and since vdat is returned, it would end up
being the value for isp-present. However..
+if (vdat)
+break;
+}
+return vdat;
+}
+
+static void isp1704_charger_work(struct work_struct *data)
+{
+struct isp1704_charger *isp =
+container_of(data, struct isp1704_charger, work);
+
+/* FIXME Only supporting dedicated chargers even though isp1704 can
+ * detect HUB
Re: [PATCH] power_supply: add isp1704 charger detection driver
Hi,
On Wed, Aug 18, 2010 at 04:55:28PM +0200, Quadros Roger (Nokia-MS/Helsinki)
wrote:
On 08/18/2010 04:01 PM, Krogerus Heikki (EXT-Teleca/Helsinki) wrote:
NXP ISP1704 is Battery Charging Specification 1.0 compliant USB
transceiver. This adds a power supply driver for ISP1704 and
ISP1707 USB transceivers.
snip
+static inline int isp1704_test_ulpi(struct isp1704_charger *isp)
+{
+ int vendor, product, i;
+ int ret = -ENODEV;
+
+ /* Test ULPI interface */
+ ret = otg_io_write(isp-otg, ULPI_SCRATCH, 0xaa);
+ if (ret 0)
+ return ret;
+ ret = otg_io_read(isp-otg, ULPI_SCRATCH);
+ if (ret 0)
+ return ret;
+ if (ret != 0xaa)
+ return -ENODEV;
+ /* Verify the product and vendor id matches */
+ vendor = otg_io_read(isp-otg, ULPI_VENDOR_ID_LOW);
+ vendor |= otg_io_read(isp-otg, ULPI_VENDOR_ID_HIGH) 8;
+ if (vendor != NXP_VENDOR_ID)
+ return -ENODEV;
+ for (i = 0; i ARRAY_SIZE(isp170x_id); i++) {
+ product = otg_io_read(isp-otg, ULPI_PRODUCT_ID_LOW);
+ product |= otg_io_read(isp-otg, ULPI_PRODUCT_ID_HIGH) 8;
product id should be read outside the for loop. This way you will save
unnecessary otg_io_reads. product id won't change anyways.
Good point. Fixing.
+ if (product == isp170x_id[i]) {
+ sprintf(isp-model, isp%x, product);
+ return product;
+ }
+ }
+
+ dev_err(isp-dev, product id %x not matching known ids, product);
+
+ return -ENODEV;
+}
snip
+static struct platform_driver isp1704_charger_driver = {
+ .driver = {
+ .name = isp1704_charger,
+ },
+ .probe = isp1704_charger_probe,
+ .remove = __devexit_p(isp1704_charger_remove),
+};
+
+static struct platform_device *isp1704_device;
+
+static int __init isp1704_charger_init(void)
+{
+ int ret = 0;
+
+ ret = platform_driver_register(isp1704_charger_driver);
+ if (ret)
+ return ret;
+
+ isp1704_device = platform_device_register_simple(isp1704_charger,
+ 0, NULL, 0);
This platform_device_register should be done in the rx51 board file. i.e.
board-rx51-peripherals.c
OK. I'll fix this.
Thanks,
--
heikki
--
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 2/2] omap: rx51: add support for USB chargers
On Thu, Aug 19, 2010 at 02:40:42PM +0200, ext Anton Vorontsov wrote: On Thu, Aug 19, 2010 at 03:09:37PM +0300, Krogerus Heikki (EXT-Teleca/Helsinki) wrote: @@ -909,5 +913,6 @@ void __init rx51_peripherals_init(void) spi_register_board_info(rx51_peripherals_spi_board_info, ARRAY_SIZE(rx51_peripherals_spi_board_info)); omap2_hsmmc_init(mmc); +platform_device_register(rx51_charger_device); Maybe platform_device_register_simple(isp1704_charger, -1, NULL, 0); I guess it's enough in this case. Ameya, Roger, any objections? -- heikki -- 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
