from:"Jassi Brar"

Re: [PATCH v3 1/3] mailbox/omap: Add ti,mbox-send-noirq quirk to fix AM33xx CPU Idle

2015-10-21 Thread Jassi Brar

On 22 October 2015 at 05:35, Suman Anna  wrote:

>>   Anyways I am OK too, if you guys want to fix it with a platform
>> specific quirk. Let me know I'll pick this patch.
>
> I haven't gotten a chance to try #1, and I won't be able to look at it
> atleast for another month. I suggest that you go ahead and pick this
> patch up, as a quirk is needed in one form or the other for #2, and #1
> is anyways a bigger change that will affect all our IPC stacks across
> all pairs of MPU - remote processors on different SoCs.
>
Yeah that is the reason I offered to pick this patch as such.
OK I'll take this patch.
--
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/3] mailbox/omap: Add ti,mbox-send-noirq quirk to fix AM33xx CPU Idle

2015-10-20 Thread Jassi Brar

On Wed, Sep 23, 2015 at 5:44 AM, Dave Gerlach  wrote:
> The mailbox framework controls the transmission queue and requires
> either its controller implementations or clients to run the state
> machine for the Tx queue. The OMAP mailbox controller uses a Tx-ready
> interrupt as the equivalent of a Tx-done interrupt to run this Tx
> queue state-machine.
>
> The WkupM3 processor on AM33xx and AM43xx SoCs is used to offload
> certain PM tasks, like doing the necessary operations for Device
> PM suspend/resume or for entering lower c-states during cpuidle.
>
> The CPUIdle on AM33xx requires the messages to be sent without
> having to trigger the Tx-ready interrupts, as the interrupt
> would immediately terminate the CPUIdle operation. Support for
> this has been added by introducing a DT quirk, "ti,mbox-send-noirq"
> and using it to modify the normal OMAP mailbox controller behavior
> on the sub-mailboxes used to communicate with the WkupM3 remote
> processor. This also requires the wkup_m3_ipc driver to adjust
> its mailbox usage logic to run the Tx state machine.
>
Probably Suman already updated you on what was discussed about this
patch at Connect-SFO. My suggestion was to drive TX poll-based (I
know, I know...) because the "interrupt-driven" is just an impression,
it is not really. You get the interrupt as soon as you enable it
because it is the "FIFO not-full" interrupt which is always because
there is always space left after writing to the fifo. The cons are
that it buffers messages hidden from the client while abusing the irq.
If you guys could break away from the "interrupt-driven" illusion and
use polling which it actually is, everything falls into place and you
avoid the "ti,mbox-send-noirq" quirk.

  Anyways I am OK too, if you guys want to fix it with a platform
specific quirk. Let me know I'll pick this patch.

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

Re: [PATCH v4] mailbox/omap: adapt to the new mailbox framework

2014-11-26 Thread Jassi Brar

On 4 November 2014 at 04:35, Suman Anna s-a...@ti.com wrote:
 The OMAP mailbox driver and its existing clients (remoteproc
 for OMAP4+) are adapted to use the generic mailbox framework.

 The main changes for the adaptation are:
   - The tasklet used for Tx is replaced with the state machine from
 the generic mailbox framework. The workqueue used for processing
 the received messages stays intact for minimizing the effects on
 the OMAP mailbox clients.
   - The existing exported client API, omap_mbox_get, omap_mbox_put and
 omap_mbox_send_msg are deleted, as the framework provides equivalent
 functionality. A OMAP-specific omap_mbox_request_channel is added
 though to support non-DT way of requesting mailboxes.
   - The OMAP mailbox driver is integrated with the mailbox framework
 through the proper implementations of mbox_chan_ops, except for
 .last_tx_done and .peek_data. The OMAP mailbox driver does not need
 these ops, as it is completely interrupt driven.
   - The OMAP mailbox driver uses a custom of_xlate controller ops that
 allows phandles for the pargs specifier instead of indexing to avoid
 any channel registration order dependencies.
   - The new framework does not support multiple clients operating on a
 single channel, so the reference counting logic is simplified.
   - The remoteproc driver (current client) is adapted to use the new API.
 The notifier callbacks used within this client is replaced with the
 regular callbacks from the newer framework.
   - The exported OMAP mailbox API are limited to omap_mbox_save_ctx,
 omap_mbox_restore_ctx, omap_mbox_enable_irq  omap_mbox_disable_irq,
 with the signature modified to take in the new mbox_chan handle instead
 of the OMAP specific omap_mbox handle. The first 2 will be removed when
 the OMAP mailbox driver is adapted to runtime_pm. The other exported
 API omap_mbox_request_channel will be removed once existing legacy
 users are converted to DT.

 Cc: Jassi Brar jassisinghb...@gmail.com
 Cc: Ohad Ben-Cohen o...@wizery.com
 Signed-off-by: Suman Anna s-a...@ti.com
 ---
 v3-v4: No code changes, switched the example to use the DSP node instead of
 WkupM3 in the bindings document  minor commit description changes. Other than
 that, this is a repost of the driver adaptation patch [1] from the OMAP 
 Mailbox
 framework adaptation series [2]. This patch is intended for the 3.19 merge 
 window,
 all the dependent patches in [2] are merged as of 3.18-rc2. The DTS patch in 
 [2]
 will be posted separately.

 [1] http://marc.info/?l=linux-omapm=141038453917790w=2
 [2] http://marc.info/?l=linux-omapm=141038447817775w=2

  .../devicetree/bindings/mailbox/omap-mailbox.txt   |  23 ++
  drivers/mailbox/omap-mailbox.c | 346 
 -
  drivers/remoteproc/omap_remoteproc.c   |  51 +--
  include/linux/omap-mailbox.h   |  16 +-
  4 files changed, 256 insertions(+), 180 deletions(-)

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

Re: [PATCH v4] mailbox/omap: adapt to the new mailbox framework

2014-11-24 Thread Jassi Brar

On 4 November 2014 at 04:35, Suman Anna s-a...@ti.com wrote:
 The OMAP mailbox driver and its existing clients (remoteproc
 for OMAP4+) are adapted to use the generic mailbox framework.

 The main changes for the adaptation are:
   - The tasklet used for Tx is replaced with the state machine from
 the generic mailbox framework. The workqueue used for processing
 the received messages stays intact for minimizing the effects on
 the OMAP mailbox clients.
   - The existing exported client API, omap_mbox_get, omap_mbox_put and
 omap_mbox_send_msg are deleted, as the framework provides equivalent
 functionality. A OMAP-specific omap_mbox_request_channel is added
 though to support non-DT way of requesting mailboxes.
   - The OMAP mailbox driver is integrated with the mailbox framework
 through the proper implementations of mbox_chan_ops, except for
 .last_tx_done and .peek_data. The OMAP mailbox driver does not need
 these ops, as it is completely interrupt driven.
   - The OMAP mailbox driver uses a custom of_xlate controller ops that
 allows phandles for the pargs specifier instead of indexing to avoid
 any channel registration order dependencies.
   - The new framework does not support multiple clients operating on a
 single channel, so the reference counting logic is simplified.
   - The remoteproc driver (current client) is adapted to use the new API.
 The notifier callbacks used within this client is replaced with the
 regular callbacks from the newer framework.
   - The exported OMAP mailbox API are limited to omap_mbox_save_ctx,
 omap_mbox_restore_ctx, omap_mbox_enable_irq  omap_mbox_disable_irq,
 with the signature modified to take in the new mbox_chan handle instead
 of the OMAP specific omap_mbox handle. The first 2 will be removed when
 the OMAP mailbox driver is adapted to runtime_pm. The other exported
 API omap_mbox_request_channel will be removed once existing legacy
 users are converted to DT.

 Cc: Jassi Brar jassisinghb...@gmail.com
 Cc: Ohad Ben-Cohen o...@wizery.com
 Signed-off-by: Suman Anna s-a...@ti.com
 ---
 v3-v4: No code changes, switched the example to use the DSP node instead of
 WkupM3 in the bindings document  minor commit description changes. Other than
 that, this is a repost of the driver adaptation patch [1] from the OMAP 
 Mailbox
 framework adaptation series [2]. This patch is intended for the 3.19 merge 
 window,
 all the dependent patches in [2] are merged as of 3.18-rc2. The DTS patch in 
 [2]
 will be posted separately.

 [1] http://marc.info/?l=linux-omapm=141038453917790w=2
 [2] http://marc.info/?l=linux-omapm=141038447817775w=2

  .../devicetree/bindings/mailbox/omap-mailbox.txt   |  23 ++
  drivers/mailbox/omap-mailbox.c | 346 
 -
  drivers/remoteproc/omap_remoteproc.c   |  51 +--
  include/linux/omap-mailbox.h   |  16 +-
  4 files changed, 256 insertions(+), 180 deletions(-)

 diff --git a/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt 
 b/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
 index 48edc4b..d1a0433 100644
 --- a/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
 +++ b/Documentation/devicetree/bindings/mailbox/omap-mailbox.txt
 @@ -43,6 +43,9 @@ Required properties:
 device. The format is dependent on which interrupt
 controller the OMAP device uses
  - ti,hwmods:   Name of the hwmod associated with the mailbox
 +- #mbox-cells: Common mailbox binding property to identify the number
 +   of cells required for the mailbox specifier. Should be
 +   1
  - ti,mbox-num-users:   Number of targets (processor devices) that the mailbox
 device can interrupt
  - ti,mbox-num-fifos:   Number of h/w fifo queues within the mailbox IP block
 @@ -72,6 +75,18 @@ data that represent the following:
  Cell #3 (usr_id)  - mailbox user id for identifying the interrupt line
  associated with generating a tx/rx fifo interrupt.

 +Mailbox Users:
 +==
 +A device needing to communicate with a target processor device should specify
 +them using the common mailbox binding properties, mboxes and the optional
 +mbox-names (please see 
 Documentation/devicetree/bindings/mailbox/mailbox.txt
 +for details). Each value of the mboxes property should contain a phandle to 
 the
 +mailbox controller device node and an args specifier that will be the 
 phandle to
 +the intended sub-mailbox child node to be used for communication. The 
 equivalent
 +mbox-names property value can be used to give a name to the communication 
 channel
 +to be used by the client user.
 +
 +
  Example:
  

 @@ -81,6 +96,7 @@ mailbox: mailbox@4a0f4000 {
 reg = 0x4a0f4000 0x200;
 interrupts = GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH;
 ti,hwmods

Re: [GIT PULL] mailbox driver framework for v3.10 merge window

2013-05-02 Thread Jassi Brar

On 3 May 2013 03:39, Suman Anna s-a...@ti.com wrote:
 Hi Arnd,

 On 04/28/2013 11:07 PM, Jassi Brar wrote:

 Not to mean only talk and no do, I prepared another proposal that
 addressed all the concerns shared so far in the RFC
 http://www.spinics.net/lists/kernel/msg1523873.html (its not ready for
 primetime yet)  And also converted the PL320 driver to the new API,
 unlike the pulled patchset which leaves that out in the cold.
 http://www.spinics.net/lists/kernel/msg1523874.html

 Yes, the current code is mainly interrupt-driven and supports only
 non-atomic context. We have a need to support atomic contexts and ipc
 controllers that do not have interrupt-based triggering.

Supporting poll and client driven TX and atomic context is going to
need big chunks of changes which we can avoid by doing them already.
Plus a bottleneck with PL320, as Mark pointed out they can't afford
any bigger latency, which will come from RX via notifier path.

 As Jassi
 pointed out, the RFC is not ready yet and there are still some
 contention points that needs to be sorted out (especially to maintain
 OMAP mailbox functionality).

Apart from a few checkpatch fixes, a missing timer delete call and
some testing with dummy client and controller drivers for various
usecases, it's ready from my side. It worked at least as good as your
API in our internal testing.

Please do let me know which OMAP functionality are you worried about,
I believe it could all still be done above this api.


 Now, we could either call it (effectively the TI's framework with
 quirks for STE) as the Common API and then dismantle and convert it
 patch by patch (authors and I seem to agree many things need to be
 changed and implemented).
   OR we do it reasonably right the first time even if that means yet
 another release. IMHO we should go for slow but steady thing.

 I think it is your call here, either of the above approaches would
 definitely involve some rework on the framework as well as both the OMAP
  ST mailboxes.

I am interested to know what changes do you anticipate in my proposed
API. Not to mean it's perfect, but I thought I provided for all
practical use-cases.

Yes, TI and STE would need re-work, but then as of now they are their
own APIs upstream. And even with your proposal they would still need
to be changed if we are to implement the desired features. What about
PL320?

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

Re: [GIT PULL] mailbox driver framework for v3.10 merge window

2013-04-28 Thread Jassi Brar

Hello Arnd,

On 9 April 2013 16:25, Arnd Bergmann a...@arndb.de wrote:
 On Thursday 04 April 2013, Anna, Suman wrote:
 OMAP and ST-Ericsson platforms are both using mailbox to communicate with 
 some coprocessors. This series creates a consolidated framework, living 
 under drivers/mailbox.
 The changes mainly contain:
 - create a mailbox framework independent from OMAP h/w
 - creates dbx500 mailbox driver for ST-Ericsson platforms
 - move the omap mailbox out of plat-omap/mach-omapX  adapting to the new 
 framework.
 - minor bug fixes in mailbox code

 Pulled into a new next/mailbox branch, to keep it separate from the
 existing subsystems.

I am going to be a heavy user of the Mailbox API. And I have reviewed
this API quite in detail. I pointed out many aspects that might have
worked for TI's usage but are not going to be work on many platforms
(including one of mine).  Suman and Loic also acknowledged most (if
not all) from 'generic' POV.  Here is the thread ...
   http://www.spinics.net/lists/arm-kernel/msg239433.html

Not to mean only talk and no do, I prepared another proposal that
addressed all the concerns shared so far in the RFC
http://www.spinics.net/lists/kernel/msg1523873.html (its not ready for
primetime yet)  And also converted the PL320 driver to the new API,
unlike the pulled patchset which leaves that out in the cold.
http://www.spinics.net/lists/kernel/msg1523874.html

Now, we could either call it (effectively the TI's framework with
quirks for STE) as the Common API and then dismantle and convert it
patch by patch (authors and I seem to agree many things need to be
changed and implemented).
  OR we do it reasonably right the first time even if that means yet
another release. IMHO we should go for slow but steady thing.

Hi Suman, Hi Loic,
   Please feel free to object to anything you think I might have
misrepresented. I am OK if you are not sure about my implementation
either - we could co-work on a new one.

Regards,
-Jassi
--
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 23/23] mfd: omap-usb-host: Don't spam console on clk_set_parent failure

2012-12-13 Thread Jassi Brar

On 5 December 2012 19:43, Roger Quadros rog...@ti.com wrote:
 On 12/05/2012 03:42 PM, Sergei Shtylyov wrote:
 Hello.

 On 04-12-2012 18:31, Roger Quadros wrote:

 clk_set_parent is expected to fail on OMAP3 platforms. We don't
 consider that as fatal so don't spam console.

 Signed-off-by: Roger Quadros rog...@ti.com
 ---
   drivers/mfd/omap-usb-host.c |   18 +-
   1 files changed, 9 insertions(+), 9 deletions(-)

 diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
 index 0bb54393..e5257dc 100644
 --- a/drivers/mfd/omap-usb-host.c
 +++ b/drivers/mfd/omap-usb-host.c
 @@ -657,32 +657,32 @@ static int __devinit usbhs_omap_probe(struct
 platform_device *pdev)
   }

   if (is_ehci_phy_mode(pdata-port_mode[0])) {
 -/* for OMAP3 , the clk set paretn fails */
 +/* for OMAP3, clk_set_parent fails */
   ret = clk_set_parent(omap-utmi_clk[0],
   omap-xclk60mhsp1_ck);
   if (ret != 0)
 -dev_err(dev, xclk60mhsp1_ck set parent
 -failed error:%d\n, ret);
 +dev_dbg(dev, xclk60mhsp1_ck set parent failed : %d\n,
 +ret);
   } else if (is_ehci_tll_mode(pdata-port_mode[0])) {
   ret = clk_set_parent(omap-utmi_clk[0],
   omap-init_60m_fclk);
   if (ret != 0)
 -dev_err(dev, init_60m_fclk set parent
 -failed error:%d\n, ret);
 +dev_dbg(dev, P0 init_60m_fclk set parent failed: %d\n,
 +ret);
   }

   if (is_ehci_phy_mode(pdata-port_mode[1])) {
   ret = clk_set_parent(omap-utmi_clk[1],
   omap-xclk60mhsp2_ck);
   if (ret != 0)
 -dev_err(dev, xclk60mhsp2_ck set parent
 -failed error:%d\n, ret);
 +dev_dbg(dev, xclk60mhsp2_ck set parent failed : %d\n,
 +ret);
   } else if (is_ehci_tll_mode(pdata-port_mode[1])) {
   ret = clk_set_parent(omap-utmi_clk[1],
   omap-init_60m_fclk);
   if (ret != 0)
 -dev_err(dev, init_60m_fclk set parent
 -failed error:%d\n, ret);
 +dev_dbg(dev, P1 init_60m_fclk set parent failed: %d\n,
 +ret);

Hm, you sometimes put a space before colon in the error message and
 sometimes not. Inconsistent. :-)


 That was because it fit in 80 characters without the space. I'm not sure
 what is more important, fitting in 80 or consistency in the print
 message. Maybe i should have removed the spaces everywhere so that it is
 consistent as well. :)

prints are one thing where breaking the 80 column rule is acceptable.
Otherwise it fails grep'ing
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-12-11 Thread Jassi Brar

On Mon, Dec 10, 2012 at 3:18 PM, Roger Quadros rog...@ti.com wrote:
 On 12/06/2012 04:34 PM, Jassi Brar wrote:

 Yes, this is where we can think of a generic PHY driver to make sure thy
 PHY has necessary resources enabled. In the Panda case it would be the
 PHY clock and RESET gpio.

 I wonder if ehci-omap should assume, besides regulator, a clock might
 also be need to setup for the transceiver chip.
 Maybe usbhs_bdata in board-omap4panda.c should have
 .reset_gpio_port[0]  = GPIO_HUB_NRESET ?


 Just like the regulator, reset_gpio_port has nothing to do with OMAP
 EHCI. So we would want to get rid of that too from the OMAP USB driver.

Looking at the code I realized we already book resources only for
populated ports. Saving power from LAN9514 chip would come from a
separate solution. So, for now when our transceiver, USB3320, has
simple hardwired configuration probably just platform_data/DT would
do. When we come across some programmable transceiver (like USB3503
over i2c), that might warrant a separate PHY driver. Still I don't
think we could have a 'generic' phy driver.


 The LAN95xx chip still needs to be powered up and the PHY driver isn't
 the right place for that (though it could be done there as a hack). The
 closest we can get to emulating right USB behavior is to map the
 SET/CLEAR PORT_FEATURE of the root hub port to the regulator that powers
 the LAN95xx.

 This way, we still don't fall in the dynamically probed space, so we
 could still provide the regulator information to the ehci hub via
 platform data and handle the regulator in ehci_hub_control
 (Set/ClearPortFeature). I'm looking at this as a software workaround for
 all platforms not implementing the EHCI set port power feature
 correctly. The same could be repeated for other HCDs as well if required.

 IMHO it's not a matter of platforms not implementing EHCI set port
 power feature correctly, we should think about onboard devices
 connected to onboard non-root hubs. Setups like LAN9514 on Panda (HSIC
 too ?) that don't run on VBUS of USB, would like their local supply to
 be treated as if it came from the parent hub's port  i.e, tie together
 the USB's set port power control with their OOB regulated power supply
 (U9 on PandaBoard).


 On Pandaboards we are still talking about root hub ports.

 Do you know any of such platforms which power their USB devices out of
 band for _non_ root hub ports?

I don't know of any. But I do believe we shouldn't discount that scenario.
IIANW lan9514 doesn't take in VBUS because it wants to avoid 5V-3.3V
regulating. What if someone designs an omap4 platform with 3
high-speed devices and the same concern in mind ?

 Assuming they do exist, the only solution is to match platform data for
 dynamically probed devices and deal with the regulators in the hub/port
 driver. Something like Andy already proposed.

Yes, but I doubt if that is the only implementation.
One USB specific solution could be to abstract out OOB port power
control in, say, port-power.c which constructs a regulator topology
mapped directly onto onboard devices', from a generic DT binding,
platform_data, ACPI whatever. And then catch any
set/clear_port_feature(POWER) call to enable/disable the regulator
corresponding to that port. Where each regulator could be a
board-specific virtual one, that does all that is necessary (like
clock/reg hierarchy setup) to power up the device.

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

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

2012-12-06 Thread Jassi Brar

On Wed, Dec 5, 2012 at 7:39 PM, Roger Quadros rog...@ti.com wrote:
Hi Jassi,

On 12/01/2012 09:49 AM, Jassi Brar wrote:
On Tue, Nov 27, 2012 at 10:32 PM, Greg KH gre...@linuxfoundation.org wrote:
On Tue, Nov 27, 2012 at 11:30:11AM -0500, Alan Stern wrote:

We should have a more generic solution in a more central location, like
the device core.

For example, each struct platform_device could have a list of resources
to be enabled when the device is bound to a driver and disabled when
the device is unbound. Such a list could include regulators, clocks,
and whatever else people can invent.

In this case, when it is created the ehci-omap.0 platform device could
get an entry pointing to the LAN95xx's regulator. Then the regulator
would automatically be turned on when the platform device is bound to
the ehci-omap driver.

How does this sound?

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

Isn't enabling/disabling of clocks and regulators what
dev.probe()/remove() of any driver already does?
If we mean only board specific setup/teardown, still it would mean
having the device core to do an extra 'probe' call when the current
probe() is already meant to do whatever it takes to bring the device
up. To my untrained eyes, it seem like messing with the
probe()/remove()'s semantics.
IMHO, if we really must implement something like that, may be we
should employ some 'broadcast mechanism' so that anything anywhere in
kernel knows which new device has been probed()/removed()
successfully. I haven't thought exactly how because I am not sure even
that would be the right way to approach PandaBoard's problem.

Looking at Figure 15 – Panda USBB1 Interface Block Diagram of
http://pandaboard.org/sites/default/files/board_reference/pandaboard-es-b/panda-es-b-manual.pdf
gives me visions ...

Actually the EHCI controller within OMAP provides the root hub with 3
ports but no PHY. One of them is connected to the USB3320 which is just
a ULPI PHY.

a) Enable REFCLK (FREF_CLK3_OUT)
b) Reset itself by cycling RESETB (GPIO_62)
c) Create one ehci-omap platform device

c) is not appropriate. ehci-omap must be created before usb3320.

(or in appropriate order if the above isn't)
That way insmod/rmmod'ing the USB3320C driver would power-up/down the
whole subsystem.

Yes, this is where we can think of a generic PHY driver to make sure thy
PHY has necessary resources enabled. In the Panda case it would be the
PHY clock and RESET gpio.

I wonder if ehci-omap should assume, besides regulator, a clock might
also be need to setup for the transceiver chip.
Maybe usbhs_bdata in board-omap4panda.c should have
.reset_gpio_port[0] = GPIO_HUB_NRESET ?

The LAN95xx chip still needs to be powered up and the PHY driver isn't
the right place for that (though it could be done there as a hack). The
closest we can get to emulating right USB behavior is to map the
SET/CLEAR PORT_FEATURE of the root hub port to the regulator that powers
the LAN95xx.

This way, we still don't fall in the dynamically probed space, so we
could still provide the regulator information to the ehci hub via
platform data and handle the regulator in ehci_hub_control
(Set/ClearPortFeature). I'm looking at this as a software workaround for
all platforms not implementing the EHCI set port power feature
correctly. The same could be repeated for other HCDs as well if required.

IMHO it's not a matter of platforms not implementing EHCI set port
power feature correctly, we should think about onboard devices
connected to onboard non-root hubs. Setups like LAN9514 on Panda (HSIC
too ?) that don't run on VBUS of USB, would like their local supply to
be treated as if it came from the parent hub's port i.e, tie together
the USB's set port power control with their OOB regulated power supply
(U9 on PandaBoard).

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

Re: [RFC PATCH 1/5] Device Power: introduce power controller

2012-12-06 Thread Jassi Brar

On 6 December 2012 18:48, Ming Lei tom.leim...@gmail.com wrote:
 On Thu, Dec 6, 2012 at 11:46 AM, Jassi Brar jaswinder.si...@linaro.org 
 wrote:
 The patch can make usb port deal with the 'power controller' only, and make 
 it
 avoid to deal with regulators/clocks/... directly.

 I am curious too, except for clocks and voltage supplies (regulators),
 what other external resources does a chip need ?

 For example, one indicator LED which doesn't connect to the same power
 domain might need to be triggered after the power switch state is changed.

Hmm.. I am not sure if we could call an indicator LED a resource that
a chip needs to be functional. Isn't how mmc core manages the
indicator led, a better way?

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

Re: [RFC PATCH 1/5] Device Power: introduce power controller

2012-12-05 Thread Jassi Brar

On 6 December 2012 06:57, Ming Lei tom.leim...@gmail.com wrote:
 On Thu, Dec 6, 2012 at 12:49 AM, Roger Quadros rog...@ti.com wrote:
 On 12/03/2012 05:00 AM, Ming Lei wrote:
 On Mon, Dec 3, 2012 at 12:02 AM, Andy Green andy.gr...@linaro.org wrote:
 On 02/12/12 23:01, the mail apparently from Ming Lei included:

 Power controller is an abstract on simple power on/off switch.

 One power controller can bind to more than one device, which
 provides power logically, for example, we can think one usb port
 in hub provides power to the usb device attached to the port, even
 though the power is supplied actually by other ways, eg. the usb
 hub is a self-power device. From hardware view, more than one
 device can share one power domain, and power controller can power
 on if one of these devices need to provide power, and power off if
 all these devices don't need to provide power.


 What stops us using struct regulator here?  If you have child regulators
 supplied by a parent supply, isn't that the right semantic already without
 introducing a whole new thing?  Apologies if I missed the point.

 There are two purposes:

 One is to hide the implementation details of the power controller because
 the user doesn't care how it is implemented, maybe clock, regulator, gpio
 and other platform dependent stuffs involved, so the patch simplify the 
 usage
 from the view of users.


 Which user are you talking about?

 Here it is the usb port device.

 At least, there are many boards which have hardwired and self-powered usb
 devices, so in theory they can benefits from the power controller.  Maybe
 only regulator and clock can't be covered completely for other boards.

 The patch can make usb port deal with the 'power controller' only, and make it
 avoid to deal with regulators/clocks/... directly.

I am curious too, except for clocks and voltage supplies (regulators),
what other external resources does a chip need ?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2012-12-01 Thread Jassi Brar

On Sat, Dec 1, 2012 at 2:07 PM, Andy Green andy.gr...@linaro.org wrote:
On 12/01/2012 03:49 PM, the mail apparently from Jassi Brar included:

On Tue, Nov 27, 2012 at 10:32 PM, Greg KH gre...@linuxfoundation.org
wrote:

On Tue, Nov 27, 2012 at 11:30:11AM -0500, Alan Stern wrote:

We should have a more generic solution in a more central location, like
the device core.

How does this sound?

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

Isn't enabling/disabling of clocks and regulators what
dev.probe()/remove() of any driver already does?

The particular issue this tries to address is stuff that is not part of the
generic driver function, but which is tied to the device instance by
hardware connection choices on the physical platform. External clocks,
regulators and mux settings needed to make the device instance work on the
board may all be out of scope for the generic driver, even when the generic
driver has a SoC-specific part as in this case.

So no, enabling regulators, clock and mux it has no idea about because the
dependency only exists as a board feature is not the job of probe / remove
in drivers already.

Yeah, I called that board specific setup/teardown. Which is quite
common in ASoC, where an soc has an I2S controller which itself
doesn't do much without some third party CODEC chip onboard which has
board specific OOB control. The CODEC setup usually involves
occasional PLL configuring besides setting reference clock(s) and
regulator supplies.

If we mean only board specific setup/teardown, still it would mean
having the device core to do an extra 'probe' call when the current
probe() is already meant to do whatever it takes to bring the device
up. To my untrained eyes, it seem like messing with the
probe()/remove()'s semantics.

It's in the way of automating and simplifying code that would be repeated in
each driver probe routine according to what you're suggesting. In fact the
pre-probe activity happens at the start of the actual probe code, and post
remove at the end of the actual remove, there is no duplicated activity.
It's just a helper.

I am not calling that code duplication. Just that device core
shouldn't be bothered to first setup board specific stuff and then
platform/soc specific stuff. Esp when both calls would be contiguous.
dev.probe() should simply get things up and running.

IMHO, if we really must implement something like that, may be we
should employ some 'broadcast mechanism' so that anything anywhere in
kernel knows which new device has been probed()/removed()
successfully. I haven't thought exactly how because I am not sure even
that would be the right way to approach PandaBoard's problem.

I think this stuff is a bit more multifacted than you're giving it credit
for, and indeed this thread has gone right into this aspect. Say we hooked
to a device creation notifier, we still must identify the correct device, in
the case the device is on a dynamic bus. Hence the discussion about device
paths. Just throwing a notifier at it itself doesn't scratch the problem.
The stuff I completed yesterday does solve this dynamic matching issue
inexpensively.

Maybe your try#2 will brainwash me to fall in line :)

Looking at Figure 15 – Panda USBB1 Interface Block Diagram of

http://pandaboard.org/sites/default/files/board_reference/pandaboard-es-b/panda-es-b-manual.pdf
gives me visions ...

1) OMAP doesn't provide the USB root-hub, but only ULPIPHY. It is
USB3320C chip that employs OMAP's ULPIPHY to provide the root-hub. So
we should have a platform device for USB3320C, the probe() of which
should simply
a) Enable REFCLK (FREF_CLK3_OUT)
b) Reset itself by cycling RESETB (GPIO_62)
c) Create one ehci-omap platform device
(or in appropriate order if the above isn't)
That way insmod/rmmod'ing the USB3320C driver would power-up/down the
whole subsystem.

First there's the issue that Panda has the same signal to reset the ULPI PHY
and the SMSC device, and that we must operate that reset after powering the
SMSC device. The only nice way to do that is to arrange for the reset to
happen once for both, at a time after the SMSC chip is powered, so we have
no need to interrupt ULPI operation or touch the reset subsequently, until
next powerup of the ULPI PHY + SMSC.

That is why tying foreign

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

2012-11-30 Thread Jassi Brar

On Tue, Nov 27, 2012 at 10:32 PM, Greg KH gre...@linuxfoundation.org wrote:
On Tue, Nov 27, 2012 at 11:30:11AM -0500, Alan Stern wrote:

We should have a more generic solution in a more central location, like
the device core.

How does this sound?

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

Looking at Figure 15 – Panda USBB1 Interface Block Diagram of
http://pandaboard.org/sites/default/files/board_reference/pandaboard-es-b/panda-es-b-manual.pdf
gives me visions ...

2) Just like the user has to manually power-on/off any externally
powered hub connected to a PC, maybe we should implement a user
controlled 'soft' switch (reacting to UDEV events from ehci-omap?) to
emulate LAN9514 power-on/off. I do realize it would be cool to have it
automatically toggle in kernel when we (de)power the hub but isn't
that outside of scope of Linux USB implementation?

The above solution isn't most optimal for Panda but it seems a design
more consistent with what we already have. Or so do I think.

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

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

2012-09-25 Thread Jassi Brar

On 19 September 2012 17:29, Felipe Balbi ba...@ti.com wrote:

 this is what I mean, actually. If we remove pm_runtime_get_sync() in
 exchange for pm_runtime_set_active() before pm_runtime_enable(), it
 works on PandaBoard, but breaks BeagleBoard.

Perhaps it suggests that OMAP4 (PandaBoard) serial port is already
activated but OMAP3 (BeagleBoard) isn't. So we need to reflect that
either by doing pm_runtime_set_active() only on OMAP4 or by bringing
up the OMAP3 too before the pm_runtime_set_active() call.
BTW I understand get_sync(), set_active() and enable() are for
different purposes, they can't be traded for one another.
--
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: How to disable message Uncompressing linux... on kernel start?

2012-09-18 Thread Jassi Brar

On 18 September 2012 12:09, Maximilian Schwerin
maximilian.schwe...@tigris.de wrote:
 Hi all,

 I have been convinced that my patch for disabling the message
 Uncompressing linux... on kernel start was not all that good an idea.
 As the problem still remains an issue for me and I'd like to find a fix
 for everyone I'd like to ask for pointers where or how to fix this.

 To sum up my problem: I'm using the primary serial port on an OMAP3 as a
 debug console for development. In production we set the u-boot silent
 option which disables all kernel logging to this serial port. The only
 kernel message that remains is Uncompressing linux... at the very
 beginning. As we connect an external device via this port in production
 environments this is not acceptable.

Perhaps adding config choice under Kernel low-level debugging port,
to select the UART port to direct the early prints to, would help.
Like some platforms already do. See DEBUG_CLPS711X_UART1/2 ,
DEBUG_MSM_UART1/2/3 etc.
--
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: How to disable message Uncompressing linux... on kernel start?

2012-09-18 Thread Jassi Brar

On 18 September 2012 15:37, Jassi Brar jaswinder.si...@linaro.org wrote:
 On 18 September 2012 12:09, Maximilian Schwerin
 maximilian.schwe...@tigris.de wrote:
 Hi all,

 I have been convinced that my patch for disabling the message
 Uncompressing linux... on kernel start was not all that good an idea.
 As the problem still remains an issue for me and I'd like to find a fix
 for everyone I'd like to ask for pointers where or how to fix this.

 To sum up my problem: I'm using the primary serial port on an OMAP3 as a
 debug console for development. In production we set the u-boot silent
 option which disables all kernel logging to this serial port. The only
 kernel message that remains is Uncompressing linux... at the very
 beginning. As we connect an external device via this port in production
 environments this is not acceptable.

 Perhaps adding config choice under Kernel low-level debugging port,
 to select the UART port to direct the early prints to, would help.
 Like some platforms already do. See DEBUG_CLPS711X_UART1/2 ,
 DEBUG_MSM_UART1/2/3 etc.

On a second thought, I think we need a new config independent of DEBUG_LL.
Something like what Samsung code does with symbol
S3C_LOWLEVEL_UART_PORT in arch/arm/plat-samsung/Kconfig
--
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: How to disable message Uncompressing linux... on kernel start?

2012-09-18 Thread Jassi Brar

  On 18 September 2012 12:09, Maximilian Schwerin
  maximilian.schwe...@tigris.de wrote:
  Hi all,
 
  I have been convinced that my patch for disabling the message
  Uncompressing linux... on kernel start was not all that
 good an idea.
  As the problem still remains an issue for me and I'd like
 to find a fix
  for everyone I'd like to ask for pointers where or how to fix this.
 
  To sum up my problem: I'm using the primary serial port on
 an OMAP3 as a
  debug console for development. In production we set the
 u-boot silent
  option which disables all kernel logging to this serial
 port. The only
  kernel message that remains is Uncompressing linux... at the very
  beginning. As we connect an external device via this port
 in production
  environments this is not acceptable.
 
  Perhaps adding config choice under Kernel low-level
 debugging port,
  to select the UART port to direct the early prints to, would help.
  Like some platforms already do. See DEBUG_CLPS711X_UART1/2 ,
  DEBUG_MSM_UART1/2/3 etc.

 On a second thought, I think we need a new config independent
 of DEBUG_LL.
 Something like what Samsung code does with symbol
 S3C_LOWLEVEL_UART_PORT in arch/arm/plat-samsung/Kconfig

 In my case I need to be able to disable it completely.

The idea is to direct the output to some other (unused) UART port.

Also if one doesn't select DEBUG_LL, I wonder if it makes sense to
expect the Uncompressing   message, which too is just dumping
raw data onto UART. So another option might be to simply use
CONFIG_DEBUG_LL instead of CONFIG_DEBUG_DECOMPRESS_KERNEL ?
--
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] OMAPDSS: Add timings for ChiMei G121S1-L01/L02 and G121X1-L01 LCD displays

2012-07-31 Thread Jassi Brar

On 31 July 2012 13:21, Tomi Valkeinen tomi.valkei...@ti.com wrote:

 2) Have the configuration for countless panels specified in the DT data

Why should a DT blob for a board contain more than 1 panel configuration?
--
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] OMAPDSS: Add timings for ChiMei G121S1-L01/L02 and G121X1-L01 LCD displays

2012-07-31 Thread Jassi Brar

On 31 July 2012 13:44, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2012-07-31 at 13:33 +0530, Jassi Brar wrote:
 On 31 July 2012 13:21, Tomi Valkeinen tomi.valkei...@ti.com wrote:

  2) Have the configuration for countless panels specified in the DT data
 
 Why should a DT blob for a board contain more than 1 panel configuration?

 I meant the DT data generally, for all boards.

If you mean : Why have the configuration (those 15 integers) of the
panel on a board specified in board.dtb?
Well, that is an important purpose of DT - moving board specific
parameters, on which a generic code works, out of kernel (I am
refraining from preaching the goodness of that).
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAPDSS: Add timings for ChiMei G121S1-L01/L02 and G121X1-L01 LCD displays

2012-07-31 Thread Jassi Brar

On 31 July 2012 14:12, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2012-07-31 at 13:57 +0530, Jassi Brar wrote:
 On 31 July 2012 13:44, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Tue, 2012-07-31 at 13:33 +0530, Jassi Brar wrote:
  On 31 July 2012 13:21, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 
   2) Have the configuration for countless panels specified in the DT data
  
  Why should a DT blob for a board contain more than 1 panel configuration?
 
  I meant the DT data generally, for all boards.
 
 If you mean : Why have the configuration (those 15 integers) of the
 panel on a board specified in board.dtb?
 Well, that is an important purpose of DT - moving board specific
 parameters, on which a generic code works, out of kernel (I am
 refraining from preaching the goodness of that).

 Sure. But panel's unconfigurable properties are not board specific
 parameters, they are panel's internal stuff. It doesn't matter to which
 board I attach Acme Foo-123 panel, the panel timings are still the same.

It's not about the panel, it's about the board. For the generic driver
in the kernel , the 'panel' is just a set of 15 integer values.
There's no Acme Foo-123 or Acme Bar-123.  In fact, the _only_
purpose of the panel's name string in the driver is to pick the
correct set of 15 integers. With DT, the name string would be
unnecessary.

Consider two panels ABC_123 and XYZ_321 having identical
parameters but different internals.
Would you have duplicate elements in the generic_dpi_panels[] array ?
Because the 'panels' are different.
Or would you simply assume the XYZ_Board has the panel 'ABC_123'?
Because after all it's the parameters that matter.

In short, we should see a 'panel' simply as a set of 15 integers.
--
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] OMAPDSS: Add timings for ChiMei G121S1-L01/L02 and G121X1-L01 LCD displays

2012-07-31 Thread Jassi Brar

On 31 July 2012 15:27, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2012-07-31 at 14:27 +0530, Jassi Brar wrote:
 On 31 July 2012 14:12, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Tue, 2012-07-31 at 13:57 +0530, Jassi Brar wrote:
  On 31 July 2012 13:44, Tomi Valkeinen tomi.valkei...@ti.com wrote:
   On Tue, 2012-07-31 at 13:33 +0530, Jassi Brar wrote:
   On 31 July 2012 13:21, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  
2) Have the configuration for countless panels specified in the DT 
data
   
   Why should a DT blob for a board contain more than 1 panel 
   configuration?
  
   I meant the DT data generally, for all boards.
  
  If you mean : Why have the configuration (those 15 integers) of the
  panel on a board specified in board.dtb?
  Well, that is an important purpose of DT - moving board specific
  parameters, on which a generic code works, out of kernel (I am
  refraining from preaching the goodness of that).
 
  Sure. But panel's unconfigurable properties are not board specific
  parameters, they are panel's internal stuff. It doesn't matter to which
  board I attach Acme Foo-123 panel, the panel timings are still the same.
 
 It's not about the panel, it's about the board. For the generic driver
 in the kernel , the 'panel' is just a set of 15 integer values.
 There's no Acme Foo-123 or Acme Bar-123.  In fact, the _only_
 purpose of the panel's name string in the driver is to pick the
 correct set of 15 integers. With DT, the name string would be
 unnecessary.

 Yes, the panel's name is used to probe the correct config. If we had
 panels that could be asked which panel are you we could use that, but
 with dummy panels we need to manually give the identifier (name) so that
 the driver can do the probe.

 Consider two panels ABC_123 and XYZ_321 having identical
 parameters but different internals.
 Would you have duplicate elements in the generic_dpi_panels[] array ?
 Because the 'panels' are different.
 Or would you simply assume the XYZ_Board has the panel 'ABC_123'?
 Because after all it's the parameters that matter.

 I would duplicate the elements. Or, if we have lots of panels having the
 exact same parameters, we could have an array of names instead of just a
 name.

 In short, we should see a 'panel' simply as a set of 15 integers.

 Ok, I see. You mean that the 15 integers define the panel, so, in a
 sense, the 15 integers is the name/identifier for the panel.

 It would technically work, of course. But I do disagree with it:

 1) I still don't see why you say it's board related. The properties in
 question are properties of the panel, told in the panel specs, and
 programmed when using the panel. No matter where the panel is used, the
 same properties should be used.

 2) As I see it, describing non-configurable device hardware properties
 in the DT data is the wrong way. The driver should either probe the
 properties or an ID from the device, or the ID of the device should be
 given to the driver (a bit like what can be done with i2c).

 3) Moving the data to DT would make any future changes more difficult.
 Say, we could (probably should) add some regulator handling to the
 driver, because usually panels need power to operate. Currently we just
 presume the powers are always on.

 Adding this is easy with the current approach. Adding it if the data is
 in DT would be difficult, if not impossible, as all the board out there
 could already be using the old DT format which doesn't have the
 regulator data. Even in the best case all the boards out there would not
 be able to use the regulator stuff.


 Academic issues aside, what is the issue with the current approach in
 practice? How would the DT approach make it better? Both approaches work
 just fine, afaik. The current approach requires some maintenance from
 me, but that's rather minor.

 Anyway, even if I don't see the point, I'm not strictly against your
 approach. If everybody thinks it's much better, it's fine for me.

No, I don't insist. Its only I in 'everybody' here, so please do it
only if you see any merit in passing panel parameters via DT. I only
wanted to share my opinion and so I did.

Best Regards.
-jassi
--
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] OMAPDSS: DISPC: Use msleep instead of blocking mdelay

2012-07-24 Thread Jassi Brar

We have no reason to block in the error handler workqueue, so use msleep.

Signed-off-by: Jassi Brar jaswinder.si...@linaro.org
---
 drivers/video/omap2/dss/dispc.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/dispc.c b/drivers/video/omap2/dss/dispc.c
index c56a51a..0e3ad44 100644
--- a/drivers/video/omap2/dss/dispc.c
+++ b/drivers/video/omap2/dss/dispc.c
@@ -3492,7 +3492,7 @@ static void dispc_error_worker(struct work_struct *work)
ovl-name);
dispc_ovl_enable(ovl-id, false);
dispc_mgr_go(ovl-manager-id);
-   mdelay(50);
+   msleep(50);
}
}
 
@@ -3524,7 +3524,7 @@ static void dispc_error_worker(struct work_struct *work)
}
 
dispc_mgr_go(mgr-id);
-   mdelay(50);
+   msleep(50);
 
if (enable)
dssdev-driver-enable(dssdev);
-- 
1.7.4.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] OMAPDSS: Add timings for ChiMei G121S1-L01/L02 and G121X1-L01 LCD displays

2012-07-20 Thread Jassi Brar

On 20 July 2012 13:41, Archit Taneja a0393...@ti.com wrote:
 On Tuesday 17 July 2012 09:57 PM, Jassi Brar wrote:

 Add timings for ChiMei G121S1-L01/L02 and G121X1-L01 LCD displays.

 Display panels are board specific and there is no limit to the number
 of panels that could be connected to omap dss.
   Does it make sense to get panel params via DT? Or at least have them
 come from board file? (esp when there is hardly a panel shared by two
 boards, and some panels aren't even used by any board in mainline)


 A panel specific param should stay in the panel driver, it's something which
 is specific to the panel and not the platform it is in

Yes it is board specific, but no it should not stay in the driver. The
driver simply needs one compatible set of 15 numbers to do its job.

Let me explain my point in detail
The array generic_dpi_panels[] is but a limited list of compatible
configurations of a 'generic' panel. Each occupying ~20 lines in
kernel. There would be dozens of supported panels that exist but are
not listed in this array, and countless more that are possible to
manufacture. If I submit a 2000 lines patch with only 100 such
configurations you would have no reason to reject other than I know
what you mean :)

 It's true that currently omap platforms don't share the same panels, but
 there is no stopping us to do that. We could remove the default panel and
 attach a new one, even though we won't upstream non default panels in the
 DT/board file, it would be always easier to make this change in software if
 most of the panel specific info stays in the panel driver.

You mean you want to hardcode parameters in the driver instead of
modifying the dtb that you pass to the kernel?

 Also, 2 platforms of different SoC's may use the same panel. Currently the
 panel drivers are SoC specific, but there is work being done between
 different display maintainers so that the same panel driver works across
 different SoCs.

Doesn't that make the case for DT/platform_data even stronger?

Of course you as a maintainer have the final say. I am out of ways to
explain my point.

Cheers.
--
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] OMAPDSS: DPI: Get panel configuration from platform data

2012-07-20 Thread Jassi Brar

Instead of harcoding in the driver some of potentially countless sets
of parameters that could define a panel, have the board provide the
parameters to the panel driver.

Signed-off-by: Jassi Brar jaswinder.si...@linaro.org
---
 arch/arm/mach-omap2/board-2430sdp.c  |4 +-
 arch/arm/mach-omap2/board-am3517evm.c|5 +-
 arch/arm/mach-omap2/board-apollon.c  |4 +-
 arch/arm/mach-omap2/board-cm-t35.c   |5 +-
 arch/arm/mach-omap2/board-devkit8000.c   |4 +-
 arch/arm/mach-omap2/board-h4.c   |3 +-
 arch/arm/mach-omap2/board-ldp.c  |4 +-
 arch/arm/mach-omap2/board-overo.c|4 +-
 drivers/video/omap2/displays/panel-generic-dpi.c |  467 +-
 include/video/omap-panel-generic-dpi.h   |   15 +-
 10 files changed, 44 insertions(+), 471 deletions(-)

diff --git a/arch/arm/mach-omap2/board-2430sdp.c 
b/arch/arm/mach-omap2/board-2430sdp.c
index 99ca6ba..0cc58c3 100644
--- a/arch/arm/mach-omap2/board-2430sdp.c
+++ b/arch/arm/mach-omap2/board-2430sdp.c
@@ -125,7 +125,9 @@ static void sdp2430_panel_disable_lcd(struct 
omap_dss_device *dssdev)
 }
 
 static struct panel_generic_dpi_data sdp2430_panel_data = {
-   .name   = nec_nl2432dr22-11b,
+   .pcfg = {{240, 320, 5400, 3, 3, 39, 1, 2, 7,}, 0, 0,
+   (OMAP_DSS_LCD_TFT | OMAP_DSS_LCD_IVS | OMAP_DSS_LCD_IHS),
+   0, 0,},
.platform_enable= sdp2430_panel_enable_lcd,
.platform_disable   = sdp2430_panel_disable_lcd,
 };
diff --git a/arch/arm/mach-omap2/board-am3517evm.c 
b/arch/arm/mach-omap2/board-am3517evm.c
index 18f6010..afd87b5 100644
--- a/arch/arm/mach-omap2/board-am3517evm.c
+++ b/arch/arm/mach-omap2/board-am3517evm.c
@@ -176,7 +176,10 @@ static void am3517_evm_panel_disable_lcd(struct 
omap_dss_device *dssdev)
 }
 
 static struct panel_generic_dpi_data lcd_panel = {
-   .name   = sharp_lq,
+   .pcfg = {{480, 272, 9000, 42, 3, 2, 11, 3, 2,}, 0, 0,
+   (OMAP_DSS_LCD_TFT | OMAP_DSS_LCD_IVS |
+   OMAP_DSS_LCD_IHS | OMAP_DSS_LCD_IEO),
+   50, 100,},
.platform_enable= am3517_evm_panel_enable_lcd,
.platform_disable   = am3517_evm_panel_disable_lcd,
 };
diff --git a/arch/arm/mach-omap2/board-apollon.c 
b/arch/arm/mach-omap2/board-apollon.c
index 502c31e..e9c6f5a 100644
--- a/arch/arm/mach-omap2/board-apollon.c
+++ b/arch/arm/mach-omap2/board-apollon.c
@@ -261,7 +261,9 @@ static struct omap_usb_config apollon_usb_config __initdata 
= {
 };
 
 static struct panel_generic_dpi_data apollon_panel_data = {
-   .name   = apollon,
+   .pcfg = {{480, 272, 6250, 41, 2, 2, 10, 2, 2,}, 0, 0,
+   (OMAP_DSS_LCD_TFT | OMAP_DSS_LCD_IVS | OMAP_DSS_LCD_IHS),
+   0, 0,},
 };
 
 static struct omap_dss_device apollon_lcd_device = {
diff --git a/arch/arm/mach-omap2/board-cm-t35.c 
b/arch/arm/mach-omap2/board-cm-t35.c
index ded100c..f5a8ce4 100644
--- a/arch/arm/mach-omap2/board-cm-t35.c
+++ b/arch/arm/mach-omap2/board-cm-t35.c
@@ -228,7 +228,10 @@ static void cm_t35_panel_disable_tv(struct omap_dss_device 
*dssdev)
 }
 
 static struct panel_generic_dpi_data lcd_panel = {
-   .name   = toppoly_tdo35s,
+   .pcfg = {{480, 640, 26000, 8, 104, 8, 2, 4, 2,}, 0, 0,
+   (OMAP_DSS_LCD_TFT | OMAP_DSS_LCD_IVS | OMAP_DSS_LCD_IHS |
+   OMAP_DSS_LCD_IPC | OMAP_DSS_LCD_ONOFF),
+   0, 0,},
.platform_enable= cm_t35_panel_enable_lcd,
.platform_disable   = cm_t35_panel_disable_lcd,
 };
diff --git a/arch/arm/mach-omap2/board-devkit8000.c 
b/arch/arm/mach-omap2/board-devkit8000.c
index 6567c1c..397f52d 100644
--- a/arch/arm/mach-omap2/board-devkit8000.c
+++ b/arch/arm/mach-omap2/board-devkit8000.c
@@ -128,7 +128,9 @@ static struct regulator_consumer_supply 
devkit8000_vio_supply[] = {
 };
 
 static struct panel_generic_dpi_data lcd_panel = {
-   .name   = innolux_at070tn83,
+   .pcfg = {{800, 480, 4, 48, 1, 1, 3, 12, 25,}, 0, 0x28,
+   (OMAP_DSS_LCD_TFT | OMAP_DSS_LCD_IVS | OMAP_DSS_LCD_IHS),
+   0, 0,},
.platform_enable= devkit8000_panel_enable_lcd,
.platform_disable   = devkit8000_panel_disable_lcd,
 };
diff --git a/arch/arm/mach-omap2/board-h4.c b/arch/arm/mach-omap2/board-h4.c
index 876becf..4f62bad 100644
--- a/arch/arm/mach-omap2/board-h4.c
+++ b/arch/arm/mach-omap2/board-h4.c
@@ -205,7 +205,8 @@ static struct platform_device *h4_devices[] __initdata = {
 };
 
 static struct panel_generic_dpi_data h4_panel_data = {
-   .name   = h4,
+   .pcfg = {{240, 320, 6250, 15, 15, 60, 1, 1, 1,}, 0, 0,
+   OMAP_DSS_LCD_TFT, 0, 0,},
 };
 
 static struct omap_dss_device h4_lcd_device = {
diff --git a/arch/arm/mach-omap2/board-ldp.c b/arch/arm/mach

Re: [PATCH] OMAPDSS: Add timings for ChiMei G121S1-L01/L02 and G121X1-L01 LCD displays

2012-07-20 Thread Jassi Brar

On 20 July 2012 18:14, Archit Taneja a0393...@ti.com wrote:
 On Friday 20 July 2012 05:43 PM, Jassi Brar wrote:

 It's true that currently omap platforms don't share the same panels, but
 there is no stopping us to do that. We could remove the default panel and
 attach a new one, even though we won't upstream non default panels in the
 DT/board file, it would be always easier to make this change in software
 if
 most of the panel specific info stays in the panel driver.

 You mean you want to hardcode parameters in the driver instead of
 modifying the dtb that you pass to the kernel?

 I meant that, but if we go with your approach, which sort of makes sense
 now, it would be in the dtb file.

In dtb or via platform_data until we have common DT bindings. I have
sent a patch doing it via platform_data, which maybe taken as such or
after making it prettier to taste.

Thanks.
--
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] OMAPDSS: Add timings for ChiMei G121S1-L01/L02 and G121X1-L01 LCD displays

2012-07-17 Thread Jassi Brar

[CC'ing OMAPDSS matinainer]

On 17 July 2012 19:31, Raphael Assenat r...@8d.com wrote:
 Add timings for ChiMei G121S1-L01/L02 and G121X1-L01 LCD displays.

Display panels are board specific and there is no limit to the number
of panels that could be connected to omap dss.
 Does it make sense to get panel params via DT? Or at least have them
come from board file? (esp when there is hardly a panel shared by two
boards, and some panels aren't even used by any board in mainline)
--
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: 3.5-rc3: PM/DSS broken (was vdd_mpu_iva warnings)

2012-07-03 Thread Jassi Brar

On 3 July 2012 16:29, Archit Taneja a0393...@ti.com wrote:

 I was going through Tomi's queue for the 3.6 merge window:

 git://gitorious.org/linux-omap-dss2/linux.git master

 There is a commit called:

 2b8501d777346ce1d4fe99167e9b3c0e42aae7a8

 OMAPDSS: Use PM notifiers for system suspend

 The commit message mentions the issue you see, and seems to resolve it.
 Could you give this a try?

 Tomi is out on vacation, and I don't know why this wasn't intended for the
 3.5-rcs, maybe there are still some discussion going on about this?

 Copping Jassi who was involved with this commit.

I understand, we settled on that patch finally.


 The error returned from the runtime_resume call is -ENOACCES, which by
 looking at the code, is returned by rpm_resume() when:

 dev-power.disable_depth  0

 I don't know what that means, does someone have any idea?

That is because during suspend PM on DISPC is disabled, so any attempt
to enable it reports ACCESS error. I suggested a mechanism, but Tomi
instead wanted to suspend early and resume late the panels via
notifiers to avoid that scenario.
--
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] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-28 Thread Jassi Brar

On 28 June 2012 12:11, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Wed, 2012-06-27 at 20:23 +0530, Jassi Brar wrote:
 On 27 June 2012 13:43, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 
  I don't like it at all that omapdss disables and enables the panels in
  omapdss's suspend/resume hooks. But I'm not sure how this should work...
  Should panel drivers each have their own suspend/resume hooks, and
  handle it themselves? Or should the call to suspend/resume come from
  upper layers, like omapfb or omapdrm.
 
  I made a prototype patch a few weeks ago to move the suspend to omapfb,
  and it feels better than the current one, but I'm still not sure...
 
 IIUC, I have similar opinion.
 Each panel having its own suspend/resume sounds like inter-dependency 
 trouble.

 What do you mean with that? If we just consider omapdss and the panel
 drivers, I see no dependency trouble. Panels are independent of each
 other, and omapdss is supposed to handle any locking  refcounting
 related to multiple panels already, as from omapdss's point of view
 panel suspend is the same as panel disable.

 And if we take omapfb/omapdrm into equation, well, in any case it
 couldn't be any worse than the current one where suspend is handled by
 omapdss.

I just anticipate it not trivial keeping  omap_dss_device.state in
sync with omap_dss_driver.suspend/resume
when the latter is made system suspend/resume and former still
affected by hdmi_panel_disable/enable.
Perhaps .disable and .suspend would need merging?

But as I said, it 'sounds' like.  It all may be straight forward - you
would know better.


 I too would prefer suspend/resume propagating from omap-fb/drm, which
 imho fits better with the notion of a linux device(omapdss is only
 backend). Though I don't have strong feelings about how core then take
 various panels up/down optimally.

 One one hand, I see the combination of omapdss (or the output side of
 omapdss) and a panel as a whole entity. I mean, if you just load omapdss
 and a panel driver, but no omapfb/omapdrm, you already have a working
 panel. You don't _need_ omapfb/omapdrm there. And in that sense it'd
 make sense if the panels did handle their own suspend/resume.

Well, I would think if omapdss+panels (backend) serve none other
omapfb/drm, they should simply lie dormant hogging no resources if the
omapfb/drm driver isn't loaded (if that isn't already the case).
Also one usually has tighter control over a private subsystem by
having only one point of intervention by system as compared to two
or more.

 But then, in real life it may be just simpler if omapfb/omapdrm handles
 the suspend.

Yeah, being a simpler option is always there.
--
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 3/3] OMAPDSS: HDMI: Cache EDID

2012-06-28 Thread Jassi Brar

On 28 June 2012 13:18, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Wed, 2012-06-27 at 19:35 +0530, jaswinder.si...@linaro.org wrote:
 From: Jassi Brar jaswinder.si...@linaro.org

 We can easily keep track of latest EDID from the display and hence avoid
 expensive EDID re-reads over I2C.
 This could also help some cheapo displays that provide EDID reliably only
 immediately after asserting HPD and not later.
 Even with good displays, there is something in OMAPDSS that apparantly
 messes up DDC occasionally while EDID is being read, giving the
   operation stopped when reading edid error.

 Btw, this is in nitpicking area, but what editor do you use? I find it
 difficult to read text that is not formatted properly =). At least vim
 formats text nicely with its formating commands.

Indeed a nitpick :)
I use vim and, iirc, checkpatch.pl gave 0 warning. Perhaps my poor
cmoposition. Please do tell how I could I make it more appealing to
you ?

 --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
 +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
 @@ -243,10 +243,13 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data 
 *ip_data)

       hpd = gpio_get_value(ip_data-hpd_gpio);

 -     if (hpd)
 +     if (hpd) {
               r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
 -     else
 +     } else {
 +             /* Invalidate EDID Cache */
 +             ip_data-edid_len = 0;
               r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON);
 +     }

 There's a problem with this patch, which leaves a wrong EDID in the
 cache: if you first have the cable connected and hdmi is enabled, you
 then turn off the HDMI display device via sysfs, we do not go to
 hdmi_check_hpd_state at all. The next time hdmi is enabled, we only get
 the plug-in event, and thus EDID cache is never invalidated.

If the hdmi cable is not replugged during that period, I don't see why
would you want the EDID invalidated ?
--
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 3/3] OMAPDSS: HDMI: Cache EDID

2012-06-28 Thread Jassi Brar

On 28 June 2012 15:44, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Thu, 2012-06-28 at 15:18 +0530, Jassi Brar wrote:

  --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
  +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
  @@ -243,10 +243,13 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data 
  *ip_data)
 
        hpd = gpio_get_value(ip_data-hpd_gpio);
 
  -     if (hpd)
  +     if (hpd) {
                r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
  -     else
  +     } else {
  +             /* Invalidate EDID Cache */
  +             ip_data-edid_len = 0;
                r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON);
  +     }
 
  There's a problem with this patch, which leaves a wrong EDID in the
  cache: if you first have the cable connected and hdmi is enabled, you
  then turn off the HDMI display device via sysfs, we do not go to
  hdmi_check_hpd_state at all. The next time hdmi is enabled, we only get
  the plug-in event, and thus EDID cache is never invalidated.
 
 If the hdmi cable is not replugged during that period, I don't see why
 would you want the EDID invalidated ?

 I wasn't very clear with my comment.

 When the display device is disabled, we're not listening to the hpd
 interrupt anymore. So when it's disabled, the cable can be replugged and
 the monitor changed, and the driver won't know about it. And so it'll
 return the old EDID for the new monitor.

If that could be a problem, then we already have some problem with
current omapdss.

I think among the first things, while enabling HDMI, should be to see
if there is really some display connected on the port i.e, HPD
asserted. Only if ti_hdmi_4xxx_detect() returned true, should we
proceed otherwise wait for HPQ irq.

Unconditionally invalidating edid really seems like a regression - we
impose atleast 50ms (edid read) as extra cost on
hdmi_check_hpd_state(), which kills half the purpose of this patch.
--
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 3/3] OMAPDSS: HDMI: Cache EDID

2012-06-28 Thread Jassi Brar

On 28 June 2012 16:17, Jassi Brar jaswinder.si...@linaro.org wrote:
 On 28 June 2012 15:44, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Thu, 2012-06-28 at 15:18 +0530, Jassi Brar wrote:

  --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
  +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
  @@ -243,10 +243,13 @@ static int hdmi_check_hpd_state(struct 
  hdmi_ip_data *ip_data)
 
        hpd = gpio_get_value(ip_data-hpd_gpio);
 
  -     if (hpd)
  +     if (hpd) {
                r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
  -     else
  +     } else {
  +             /* Invalidate EDID Cache */
  +             ip_data-edid_len = 0;
                r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON);
  +     }
 
  There's a problem with this patch, which leaves a wrong EDID in the
  cache: if you first have the cable connected and hdmi is enabled, you
  then turn off the HDMI display device via sysfs, we do not go to
  hdmi_check_hpd_state at all. The next time hdmi is enabled, we only get
  the plug-in event, and thus EDID cache is never invalidated.
 
 If the hdmi cable is not replugged during that period, I don't see why
 would you want the EDID invalidated ?

 I wasn't very clear with my comment.

 When the display device is disabled, we're not listening to the hpd
 interrupt anymore. So when it's disabled, the cable can be replugged and
 the monitor changed, and the driver won't know about it. And so it'll
 return the old EDID for the new monitor.

 If that could be a problem, then we already have some problem with
 current omapdss.

 I think among the first things, while enabling HDMI, should be to see
 if there is really some display connected on the port i.e, HPD
 asserted. Only if ti_hdmi_4_detect() returned true, should we
 proceed otherwise wait for HPQ irq.

 Unconditionally invalidating edid really seems like a regression - we
 impose atleast 50ms (edid read) as extra cost on
 hdmi_check_hpd_state(), which kills half the purpose of this patch.

Sorry a correction. Reading detect() won't work. I suggest we keep HPD
IRQ enabled for the lifetime of the driver.
--
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 3/3] OMAPDSS: HDMI: Cache EDID

2012-06-28 Thread Jassi Brar

On 28 June 2012 16:40, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Thu, 2012-06-28 at 16:28 +0530, Jassi Brar wrote:
 
 Sorry a correction. Reading detect() won't work. I suggest we keep HPD
 IRQ enabled for the lifetime of the driver.

 Ok, I see. But that's not acceptable. It would require us to keep the
 TPD12S015 always powered and enabled. Even if you're not interested in
 using HDMI at all.

 I think we need to differentiate between HDMI PHY enable and HDMI
5V+,HPD enable [1]... currently they are clubbed together in
omap_dss_device.platform_enable.  AFAIK, at least with TPD12S015, they
can be controlled independently and PHY enabling is actually the main
source of power consumption if no display is connected.

By 'lifetime' I mean when the end-user selects some option to the
effect of Automatically detect and configure display over HDMI 
and then we simply enable the HDMI 5V+/HPD,   HDMI-PHY would be
enabled only when we actually detect HPD asserted. If a device doesn't
have a port or the user doesn't have a display, neither would be ever
enabled. I mean we should provide a way to make it platform dependent.


 [1]  Thanks to Andy and his crappy TV, he found clubbing enabling PHY
with 5V+ application comes in the way of detecting cheapo displays
that take ~700ms before asserting HPD i.e, making EDID available. See
how we don't leave it to a HDMI display to take it's own time before
asserting HPD - omapdss_hdmi_display_enable/disable pairs don't care
for that.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID

2012-06-28 Thread Jassi Brar

On 28 June 2012 17:33, Andy Green andy.gr...@linaro.org wrote:
 On 06/28/12 19:10, the mail apparently from Tomi Valkeinen included:

 On Thu, 2012-06-28 at 16:28 +0530, Jassi Brar wrote:

 On 28 June 2012 16:17, Jassi Brar jaswinder.si...@linaro.org wrote:

 On 28 June 2012 15:44, Tomi Valkeinen tomi.valkei...@ti.com wrote:

 On Thu, 2012-06-28 at 15:18 +0530, Jassi Brar wrote:


 --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
 +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
 @@ -243,10 +243,13 @@ static int hdmi_check_hpd_state(struct
 hdmi_ip_data *ip_data)

       hpd = gpio_get_value(ip_data-hpd_gpio);

 -     if (hpd)
 +     if (hpd) {
               r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
 -     else
 +     } else {
 +             /* Invalidate EDID Cache */
 +             ip_data-edid_len = 0;
               r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON);
 +     }


 There's a problem with this patch, which leaves a wrong EDID in the
 cache: if you first have the cable connected and hdmi is enabled, you
 then turn off the HDMI display device via sysfs, we do not go to
 hdmi_check_hpd_state at all. The next time hdmi is enabled, we only
 get
 the plug-in event, and thus EDID cache is never invalidated.

 If the hdmi cable is not replugged during that period, I don't see why
 would you want the EDID invalidated ?


 I wasn't very clear with my comment.

 When the display device is disabled, we're not listening to the hpd
 interrupt anymore. So when it's disabled, the cable can be replugged
 and
 the monitor changed, and the driver won't know about it. And so it'll
 return the old EDID for the new monitor.

 If that could be a problem, then we already have some problem with
 current omapdss.

 I think among the first things, while enabling HDMI, should be to see
 if there is really some display connected on the port i.e, HPD
 asserted. Only if ti_hdmi_4_detect() returned true, should we
 proceed otherwise wait for HPQ irq.

 Unconditionally invalidating edid really seems like a regression - we
 impose atleast 50ms (edid read) as extra cost on
 hdmi_check_hpd_state(), which kills half the purpose of this patch.

 Sorry a correction. Reading detect() won't work. I suggest we keep HPD
 IRQ enabled for the lifetime of the driver.


 Ok, I see. But that's not acceptable. It would require us to keep the
 TPD12S015 always powered and enabled. Even if you're not interested in
 using HDMI at all.

 For this to work like you want we need a bigger restructuring of HDMI
 and partly omapdss also. Currently we have just one big enabled or
 disabled state. We need multiple states. Probably something like:

 - disabled, everything totally off
 - low power, hotplug detection enabled
 - powered on, but no video
 - video enabled

 Been long in my mind, but I'm not very familiar with HDMI so I could get
 my simple prototypes to work when I tried something like this once.


 That doesn't sound too hard since the difference between the first three
 states at the HDMI chip is just whether the two gpio controlling it are 00,
 10 or 11.

 If Jassi's alright with it we might have a go at implementing this, but can
 you define a bit more about how we logically tell DSS that we want to, eg,
 disable HDMI totally?

A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
probe and disable during remove.
HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
HDMI_PHY on/off.
The user selecting Autodetect and Configure option would then equate
to (un)loading of the HDMI driver.
Not to mean a trivial job.
--
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 3/3] OMAPDSS: HDMI: Cache EDID

2012-06-28 Thread Jassi Brar

On 28 June 2012 19:01, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Thu, 2012-06-28 at 18:43 +0530, Jassi Brar wrote:
 On 28 June 2012 17:33, Andy Green andy.gr...@linaro.org wrote:

  If Jassi's alright with it we might have a go at implementing this, but can
  you define a bit more about how we logically tell DSS that we want to, eg,
  disable HDMI totally?
 
 A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
 probe and disable during remove.

 The problem with this is a feature of omapdss: we can have multiple
 displays for the same output, of which only one can be enabled at the
 same time. What this means is that you shouldn't (and in some cases
 can't) allocate or enable resources in probe that may be shared, because
 then the driver for both displays would try to allocate the same
 resource.

 Sure, this is not a problem for the HDMI configuration we are using now,
 but it's still against the panel model we have. Thus we should allocate
 resources only when the panel device is turned on, and release them when
 it's disabled.

 I do think the model is slightly broken, but that's what we have now.
 And I'm also not even sure how it should be fixed...

I won't press further with my Utopian ideas, but I think we need to
segregate 5V/HPD enabling from PHY enabling somehow. Because that is
already failing slow but otherwise perfectly legit displays (like
Andy's HPD taking 700ms TV)


 And also, as I said earlier, if you keep it enabled all the time, it'll
 eat power even if the user is never going to use HDMI.

 On a desktop I guess the power consumption wouldn't be an issue, but I
 do feel a bit uneasy about it on an embedded device.

As I said, it should be platform dependent. If a device doesn't have
HDMI port, the board file would not even have platform_enable. And if
it has, some user action should enable it while 'making the device
ready for new display'.
IOW, how do you envision an OMAP4 based tablet with HDMI port react to
display connections ?


 HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
 HDMI_PHY on/off.
 The user selecting Autodetect and Configure option would then equate
 to (un)loading of the HDMI driver.

 HDMI cannot be currently compiled as a separate module. Although I think
 you can detach a device and a driver, achieving the same. Is that what
 you meant with unloading?

Yeah, I meant something to the effect of bringing HDMI driver to life.


 By the way, when the device is in system suspend, we surely won't detect
 the HPD even if we kept the HPD always enabled. So there we'll miss the
 HPD interrupt anyway, and the EDID cache would be invalid.

If omapdss already handles the possibility of display changed during
suspend, I think we should be good :)
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] OMAPDSS: HDMI: Cache EDID

2012-06-28 Thread Jassi Brar

On 28 June 2012 20:44, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Thu, 2012-06-28 at 18:43 +0530, Jassi Brar wrote:

 A quick reaction of my guts say, we simply enable 5V/HPD_IRQ during
 probe and disable during remove.
 HDMI enable/disable via /sysfs/ and HPD (de)assertion, switch only
 HDMI_PHY on/off.
 The user selecting Autodetect and Configure option would then equate
 to (un)loading of the HDMI driver.
 Not to mean a trivial job.

 One more thing I realized while thinking about this:

 While it could be argued that the power draw from having the tpd12s015
 always enabled is very small, I think it could matter. If you consider a
 phone with HDMI output, it's likely that the phone is locked 99% of the
 time. When the phone is locked, there's no need to keep the HDMI HPD
 enabled. So this could add to a considerable amount of power wasted, if
 the HPD was always enabled.

 At least I can't figure out a reason why one would want the HPD to work
 when the phone is locked. Also, I have never used the HDMI output on my
 phone, so I'd be glad if it was totally powered off if it gave me more
 standby hours =).

Of course, I don't suggest imposing any hard rule here.
All I suggest is make it platform dependent and provide a way from
user-space too to enable/disable HPD.
--
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 3/3] OMAPDSS: HDMI: Cache EDID

2012-06-28 Thread Jassi Brar

On 28 June 2012 20:57, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Thu, 2012-06-28 at 20:44 +0530, Jassi Brar wrote:

 it has, some user action should enable it while 'making the device
 ready for new display'.
 IOW, how do you envision an OMAP4 based tablet with HDMI port react to
 display connections ?

 I guess this was covered in my mail about the phone's HDMI. If the
 tablet is unlocked, and I plug in a HDMI cable, I expect the device to
 do something. Either clone the display, or perhaps ask me what I want to
 do.

 So yes, HPD would be always enabled, when the tablet is active
 (unlocked).

OK, somehow I was under impression you didn't wanna spare even the 5V+ floating.
Though there could also be some option in settings to enable 5/HPD
only when the user is about the connect a display... so that the
activate window is even narrowed down. Anyway... I am glad we are in
sync.

  By the way, when the device is in system suspend, we surely won't detect
  the HPD even if we kept the HPD always enabled. So there we'll miss the
  HPD interrupt anyway, and the EDID cache would be invalid.
 
 If omapdss already handles the possibility of display changed during
 suspend, I think we should be good :)

 Hmm I'm not sure I understand what you mean. I was referring to your
 patch, which invalidated the EDID cache only on HPD interrupt when the
 cable is unplugged. And we'd miss that interrupt when the board is in
 system suspend, even if we otherwise kept the HPD interrupt always
 enabled.

I meant before stale-edid, we face potential problem of omapdss
behaving badly to the displays switched during suspend ?
--
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 3/3] OMAPDSS: HDMI: Cache EDID

2012-06-28 Thread Jassi Brar

On 28 June 2012 21:18, Jassi Brar jaswinder.si...@linaro.org wrote:
 On 28 June 2012 20:57, Tomi Valkeinen tomi.valkei...@ti.com wrote:

  By the way, when the device is in system suspend, we surely won't detect
  the HPD even if we kept the HPD always enabled. So there we'll miss the
  HPD interrupt anyway, and the EDID cache would be invalid.
 
 If omapdss already handles the possibility of display changed during
 suspend, I think we should be good :)

 Hmm I'm not sure I understand what you mean. I was referring to your
 patch, which invalidated the EDID cache only on HPD interrupt when the
 cable is unplugged. And we'd miss that interrupt when the board is in
 system suspend, even if we otherwise kept the HPD interrupt always
 enabled.

 I meant before stale-edid, we face potential problem of omapdss
 behaving badly to the displays switched during suspend ?

OK, I think I get now what you mean. We do need to invalidate
edid-cache in the suspend path, irrespective of how omapdss behaves.

Thanks.
--
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] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-27 Thread Jassi Brar

On 27 June 2012 11:28, Tomi Valkeinen tomi.valkei...@ti.com wrote:

 It doesn't matter how omapdss is organized, -EACCES _is_ an error. It
 tells us that something unexpected happened, and we should react to it
 somehow.

  $ git show 5025ce070
Exactly how omapdss is organised is the reason -EBUSY isn't an error there :)
Otherwise, omapdss should panic that somehow 'imbalance' has been
introduced in rpm.



 Sure, in the current omapdss neither is a breaking problem, because 1)
 the matching dispc_runtime_put() is called also with runtime PM
 disabled, and thus we don't decrease the use count, and 2) the HW
 happens to be already enabled. But that's just by luck, and tomorrow
 omapdss could be different.

It's no 'luck', but it's because today omapdss takes proper care of PM
enable/disable and get/put.
Rather, if tomorrow that stops working, it would hint that we managed
to screw up the balance.
Because if omapdss suspended and disabled PM on DISPC, and still HDMI
attempted to access dss regs, that clearly means HDMI hasn't been duly
made aware of the DISPC status.

Just as preemption and suspend/resume don't introduce any race in
locking, RPM won't introduce new imbalance in get/put of omapdss.

I am afraid, we won't reach any eureka moment on this, so I would
leave us to our conditions. This patch and discussion made me look
deep into rpm, I thank you for that and for your patience.

Cheers!
--
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] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-27 Thread Jassi Brar

On 27 June 2012 13:43, Tomi Valkeinen tomi.valkei...@ti.com wrote:

 I don't like it at all that omapdss disables and enables the panels in
 omapdss's suspend/resume hooks. But I'm not sure how this should work...
 Should panel drivers each have their own suspend/resume hooks, and
 handle it themselves? Or should the call to suspend/resume come from
 upper layers, like omapfb or omapdrm.

 I made a prototype patch a few weeks ago to move the suspend to omapfb,
 and it feels better than the current one, but I'm still not sure...

IIUC, I have similar opinion.
Each panel having its own suspend/resume sounds like inter-dependency trouble.
I too would prefer suspend/resume propagating from omap-fb/drm, which
imho fits better with the notion of a linux device(omapdss is only
backend). Though I don't have strong feelings about how core then take
various panels up/down optimally.

BR.
--
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] OMAPDSS: Use PM notifiers for system suspend

2012-06-27 Thread Jassi Brar

On 27 June 2012 20:04, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 The current way how omapdss handles system suspend and resume is that
 omapdss device (a platform device, which is not part of the device
 hierarchy of the DSS HW devices, like DISPC and DSI, or panels.) uses
 the suspend and resume callbacks from platform_driver to handle system
 suspend. It does this by disabling all enabled panels on suspend, and
 resuming the previously disabled panels on resume.

 This presents a few problems.

 One is that as omapdss device is not related to the panel devices or the
 DSS HW devices, there's no ordering in the suspend process. This means
 that suspend could be first ran for DSS HW devices and panels, and only
 then for omapdss device. Currently this is not a problem, as DSS HW
 devices and panels do not handle suspend.

 Another, more pressing problem, is that when suspending or resuming, the
 runtime PM functions return -EACCES as runtime PM is disabled during
 system suspend. This causes the driver to print warnings, and operations
 to fail as they think that they failed to bring up the HW.

 This patch changes the omapdss suspend handling to use PM notifiers,
 which are called before suspend and after resume. This way we have a
 normally functioning system when we are suspending and resuming the
 panels.

 This patch, I believe, creates a problem that somebody could enable or
 disable a panel between PM_SUSPEND_PREPARE and the system suspend, and
 similarly the other way around in resume. I choose to ignore the problem
 for now, as it sounds rather unlikely, and if it happens, it's not
 fatal.

 In the long run the system suspend handling of omapdss and panels should
 be thought out properly. The current approach feels rather hacky.
 Perhaps the panel drivers should handle system suspend, or the users of
 omapdss (omapfb, omapdrm) should handle system suspend.

 Note that after this patch we could probably revert
 0eaf9f52e94f756147dbfe1faf1f77a02378dbf9 (OMAPDSS: use sync versions of
 pm_runtime_put).

I would think only DISPC should need the sync version.

 But as I said, this patch may be temporary, so let's
 leave the sync version still in place.

 Signed-off-by: Tomi Valkeinen tomi.valkei...@ti.com
 Reported-by: Jassi Brar jaswinder.si...@linaro.org

Please feel free to add

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

Re: [PATCH] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-26 Thread Jassi Brar

On 26 June 2012 12:49, Tomi Valkeinen tomi.valkei...@ti.com wrote:
On Mon, 2012-06-25 at 22:36 +0530, Jassi Brar wrote:

Normally if the driver does dispc_runtime_get() and dispc_read_reg(),
the first call will enable the HW so the reg read works.

But if the pm_runtime is disabled, say, during system suspend, with your
patch dispc_runtime_get() will just return 0 without doing anything, and
the dispc_read_reg() will crash because the HW is disabled (because
nobody enabled it).

Hmm, I am not sure if new calls would/should be made to dispc.c after
the system has suspended and before resumed. That is, anything other
than from runtime_resume/suspend callbacks of DSS, DISPC, HDMI, VENC
and RFBI, which rightly don't touch any dss reg but only
enable/disable a clock.

They do touch the registers. For example, dispc's callbacks save and
restore the registers. The HW should be fully functional during the
callbacks. The point of the callbacks is to suspend/resume the HW in
question, which of course requires accessing the HW.

DISPC being held by HDMI, VENC and RFBI would be the last to suspend
and first to resume. And it won't have its registers touched between
dispc_runtime_suspend() and dispc_runtime_resume(), which seems ok to
me (?)
HDMI, VENC and RFBI directly fooling around with DISPC regs would have
been a problem, which isn't the case.

As we know, a subsystem should make sure any active work is cleared
out before suspending and set some flag so that nothing runs until it
has resumed. I don't say we can't crash the system with this patch,
but then we would be violating rules of suspend-resume.

Let's go back a bit. I feel like I'm missing some pieces of information,
as I still don't quite grasp the problem.

In the patch you said this fixes an issue with HDMI. Can you tell more
about the problem? What code path is being run? Any error messages?

I tried system suspend with omap4-sdp and panda, with 3.5-rc2, but
neither board seems to wake up from the suspend. Does it work for you?

Something non-omapdss in vanilla breaks suspend/resume.
Without this patch I see the upstream's display broken after the
suspend attempt.
$ echo mem /sys/power/state

I work on TILT tree, which has suspend/resume working after some more
local patches.

http://git.linaro.org/gitweb?p=people/andygreen/kernel-tilt.git;a=shortlog;h=refs/heads/tilt-3.4

I don't have SDP so not sure, but it should simply be testable with
Panda4460 and the omap4plus_defconfig there. Please feel free to ask
if you have any issue checking that out.

Thanks.
--
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] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-26 Thread Jassi Brar

On 26 June 2012 14:37, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2012-06-26 at 14:02 +0530, Jassi Brar wrote:

 Something non-omapdss in vanilla breaks suspend/resume.

 I was able to reproduce (probably) the same issue with omap3 overo. Does
 this looks familiar:

 [ 2267.140197] [ cut here ]
 [ 2267.145172] WARNING: at drivers/video/omap2/dss/dispc.c:377 
 dispc_runtime_get+0x60/0x7c [omapdss]
 ()
 [ 2267.154846] Modules linked in: omapfb panel_generic_dpi omapdss cfbimgblt 
 cfbfillrect cfbcopyarea
  [last unloaded: omapdss]
 [ 2267.166595] [c001b61c] (unwind_backtrace+0x0/0xf0) from [c0040238] 
 (warn_slowpath_common+0x4c
 /0x64)
 [ 2267.176605] [c0040238] (warn_slowpath_common+0x4c/0x64) from 
 [c004026c] (warn_slowpath_null+0
 x1c/0x24)
 [ 2267.186859] [c004026c] (warn_slowpath_null+0x1c/0x24) from [bf0d7918] 
 (dispc_runtime_get+0x60
 /0x7c [omapdss])
 [ 2267.197814] [bf0d7918] (dispc_runtime_get+0x60/0x7c [omapdss]) from 
 [bf0e3148] (omapdss_dpi_d
 isplay_enable+0x48/0x230 [omapdss])
 [ 2267.210479] [bf0e3148] (omapdss_dpi_display_enable+0x48/0x230 [omapdss]) 
 from [bf110034] (gen
 eric_dpi_panel_check_timings+0x30/0x7c [panel_generic_dpi])
 [ 2267.225311] [bf110034] (generic_dpi_panel_check_timings+0x30/0x7c 
 [panel_generic_dpi]) from [b
 f11008c] (generic_dpi_panel_resume+0xc/0x1c [panel_generic_dpi])
 [ 2267.240722] [bf11008c] (generic_dpi_panel_resume+0xc/0x1c 
 [panel_generic_dpi]) from [bf0de654
 ] (dss_resume_device+0x28/0x40 [omapdss])
 [ 2267.253936] [bf0de654] (dss_resume_device+0x28/0x40 [omapdss]) from 
 [c02bfb94] (bus_for_each_
 dev+0x50/0x7c)
 [ 2267.264678] [c02bfb94] (bus_for_each_dev+0x50/0x7c) from [c02c287c] 
 (platform_pm_resume+0x2c/
 0x50)
 [ 2267.274566] [c02c287c] (platform_pm_resume+0x2c/0x50) from [c02c6da8] 
 (dpm_run_callback.clone
 .7+0x30/0xb0)
 [ 2267.285186] [c02c6da8] (dpm_run_callback.clone.7+0x30/0xb0) from 
 [c02c7b2c] (device_resume+0x
 c8/0x188)
 [ 2267.295471] [c02c7b2c] (device_resume+0xc8/0x188) from [c02c7f54] 
 (dpm_resume+0xfc/0x21c)
 [ 2267.304534] [c02c7f54] (dpm_resume+0xfc/0x21c) from [c02c8208] 
 (dpm_resume_end+0xc/0x18)
 [ 2267.313507] [c02c8208] (dpm_resume_end+0xc/0x18) from [c007fbcc] 
 (suspend_devices_and_enter+0
 x15c/0x2d0)
 [ 2267.323913] [c007fbcc] (suspend_devices_and_enter+0x15c/0x2d0) from 
 [c007fecc] (pm_suspend+0x
 18c/0x208)
 [ 2267.334259] [c007fecc] (pm_suspend+0x18c/0x208) from [c007f170] 
 (state_store+0x120/0x134)
 [ 2267.343292] [c007f170] (state_store+0x120/0x134) from [c0262850] 
 (kobj_attr_store+0x14/0x20)
 [ 2267.352661] [c0262850] (kobj_attr_store+0x14/0x20) from [c0169b6c] 
 (sysfs_write_file+0x100/0x
 184)
 [ 2267.362457] [c0169b6c] (sysfs_write_file+0x100/0x184) from [c0109008] 
 (vfs_write+0xb4/0x148)
 [ 2267.371795] [c0109008] (vfs_write+0xb4/0x148) from [c0109290] 
 (sys_write+0x40/0x70)
 [ 2267.380310] [c0109290] (sys_write+0x40/0x70) from [c0013d60] 
 (ret_fast_syscall+0x0/0x3c)
 [ 2267.389282] ---[ end trace 54fe7eea726ac84d ]---
 [ 2267.394592] dpm_run_callback(): platform_pm_resume+0x0/0x50 returns -13
 [ 2267.401641] PM: Device omapdss failed to resume: error -13

Seems similar, but I only tested OMAP4 HDMI.
thanks.
--
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] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-26 Thread Jassi Brar

On 26 June 2012 17:33, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2012-06-26 at 15:27 +0530, Jassi Brar wrote:

 Seems similar, but I only tested OMAP4 HDMI.

 Would something like this one below work for you? It fixes the issues on
 my overo board.

I think this should work too (I will get to test it only tomorrow).

Though I don't think it'll fix stack spew when run without
CONFIG_PM_RUNTIME. Maybe we could simply remove the WARN_ON in the
xxx_runtime_put() as Alan noted?

-j

 Instead of using omapdss device's suspend/resume callbacks, this one
 uses PM notifier calls which happen before suspend and after resume.

 I still think the suspend handling is wrong, omapdss shouldn't be
 enabling and disabling panel devices like that, but this one should
 remove the biggest issues with the current suspend method.

  Tomi

 diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
 index 5066eee..c35a248 100644
 --- a/drivers/video/omap2/dss/core.c
 +++ b/drivers/video/omap2/dss/core.c
 @@ -32,6 +32,7 @@
  #include linux/io.h
  #include linux/device.h
  #include linux/regulator/consumer.h
 +#include linux/suspend.h

  #include video/omapdss.h

 @@ -201,6 +202,30 @@ int dss_debugfs_create_file(const char *name, void 
 (*write)(struct seq_file *))
  #endif /* CONFIG_DEBUG_FS  CONFIG_OMAP2_DSS_DEBUG_SUPPORT */

  /* PLATFORM DEVICE */
 +static int omap_dss_pm_notif(struct notifier_block *b, unsigned long v, void 
 *d)
 +{
 +       DSSDBG(pm notif %lu\n, v);
 +
 +       switch (v)
 +       {
 +       case PM_SUSPEND_PREPARE:
 +               DSSDBG(suspending displays\n);
 +               return dss_suspend_all_devices();
 +
 +       case PM_POST_SUSPEND:
 +               DSSDBG(resuming displays\n);
 +               return dss_resume_all_devices();
 +
 +       default:
 +               return 0;
 +       }
 +}
 +
 +static struct notifier_block omap_dss_pm_notif_block =
 +{
 +       .notifier_call = omap_dss_pm_notif,
 +};
 +
  static int __init omap_dss_probe(struct platform_device *pdev)
  {
        struct omap_dss_board_info *pdata = pdev-dev.platform_data;
 @@ -224,6 +249,8 @@ static int __init omap_dss_probe(struct platform_device 
 *pdev)
        else if (pdata-default_device)
                core.default_display_name = pdata-default_device-name;

 +       register_pm_notifier(omap_dss_pm_notif_block);
 +
        return 0;

  err_debugfs:
 @@ -233,6 +260,8 @@ err_debugfs:

  static int omap_dss_remove(struct platform_device *pdev)
  {
 +       unregister_pm_notifier(omap_dss_pm_notif_block);
 +
        dss_uninitialize_debugfs();

        dss_uninit_overlays(pdev);
 @@ -247,25 +276,9 @@ static void omap_dss_shutdown(struct platform_device 
 *pdev)
        dss_disable_all_devices();
  }

 -static int omap_dss_suspend(struct platform_device *pdev, pm_message_t state)
 -{
 -       DSSDBG(suspend %d\n, state.event);
 -
 -       return dss_suspend_all_devices();
 -}
 -
 -static int omap_dss_resume(struct platform_device *pdev)
 -{
 -       DSSDBG(resume\n);
 -
 -       return dss_resume_all_devices();
 -}
 -
  static struct platform_driver omap_dss_driver = {
        .remove         = omap_dss_remove,
        .shutdown       = omap_dss_shutdown,
 -       .suspend        = omap_dss_suspend,
 -       .resume         = omap_dss_resume,
        .driver         = {
                .name   = omapdss,
                .owner  = THIS_MODULE,




-- 
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106  -
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog
--
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] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-26 Thread Jassi Brar

On 26 June 2012 20:38, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2012-06-26 at 20:19 +0530, Jassi Brar wrote:
 On 26 June 2012 17:33, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Tue, 2012-06-26 at 15:27 +0530, Jassi Brar wrote:
 
  Seems similar, but I only tested OMAP4 HDMI.
 
  Would something like this one below work for you? It fixes the issues on
  my overo board.
 
 I think this should work too (I will get to test it only tomorrow).

 Though I don't think it'll fix stack spew when run without
 CONFIG_PM_RUNTIME. Maybe we could simply remove the WARN_ON in the
 xxx_runtime_put() as Alan noted?

 Yes, that's a different issue. I'll look at that also.

Well, my patch took care of that also. But I agree, that could be
added separately as well.
--
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] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-26 Thread Jassi Brar

On 26 June 2012 20:41, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2012-06-26 at 20:39 +0530, Jassi Brar wrote:
 On 26 June 2012 20:38, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Tue, 2012-06-26 at 20:19 +0530, Jassi Brar wrote:
  On 26 June 2012 17:33, Tomi Valkeinen tomi.valkei...@ti.com wrote:
   On Tue, 2012-06-26 at 15:27 +0530, Jassi Brar wrote:
  
   Seems similar, but I only tested OMAP4 HDMI.
  
   Would something like this one below work for you? It fixes the issues on
   my overo board.
  
  I think this should work too (I will get to test it only tomorrow).
 
  Though I don't think it'll fix stack spew when run without
  CONFIG_PM_RUNTIME. Maybe we could simply remove the WARN_ON in the
  xxx_runtime_put() as Alan noted?
 
  Yes, that's a different issue. I'll look at that also.
 
 Well, my patch took care of that also. But I agree, that could be
 added separately as well.

 Well, I don't agree that your patch is correct =). I don't think it's
 right to skip runtime get and put when pm_runtime_enabled() returns
 false.

While I think your patch is simpler and achieve the same, I also think
your fears about this patch are unfounded.

A quick snack for thought...

 But if pm_runtime_get_sync() returns an error, it means the HW has not
 been resumed successfully, and is not operational,

Not always. The HW could be in RPM_ACTIVE state while PM on it could
be disabled, if the returned error is -EACCESS.   And
pm_runtime_enabled() only catches a potential -EACCESS.


BTW, I just tested your patch and it worked for me as well. But as
suspected, it doesn't help the stack spew of CONFIG_PM_RUNTIME:=n

So I understand, I only need to resend the other three patches ?

Thanks.
--
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] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-26 Thread Jassi Brar

On 27 June 2012 00:14, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Tue, 2012-06-26 at 22:31 +0530, Jassi Brar wrote:

  But if pm_runtime_get_sync() returns an error, it means the HW has not
  been resumed successfully, and is not operational,
 
 Not always. The HW could be in RPM_ACTIVE state while PM on it could
 be disabled, if the returned error is -EACCESS.   And
 pm_runtime_enabled() only catches a potential -EACCESS.

 True. But the HW could also be in disabled state. And that would lead to
 a crash when accessing the registers.

 It is not a fatal error if pm_runtime_get returns -EACCES, but we sure
 shouldn't ignore it (or avoid it with pm_runtime_enabled()), but handle
 it. In some rare cases it could be ok to get -EACCES, but that's a
 special case, not standard.

You are mixing up generic concepts with what we have in omapdss.
Believe me, I do understand it's bad to proceed without caring for
returned _errors_.
The way omapdss is organized -EACCESS is _not_ an  error, it just
denotes PM is disabled on the device and that DISPC is in RPM_ACTIVE
is backed by the fact that HDMI always hold a reference between
resume-suspend and DISPC goes to suspend last and resume first.


 BTW, I just tested your patch and it worked for me as well. But as
 suspected, it doesn't help the stack spew of CONFIG_PM_RUNTIME:=n

 So I understand, I only need to resend the other three patches ?

 Yes, please.

OK, will do today later.

Regards.
--
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] OMAPDSS: HDMI: Cache EDID

2012-06-25 Thread Jassi Brar

On 25 June 2012 12:05, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Sat, 2012-06-23 at 13:36 +0530, jaswinder.si...@linaro.org wrote:
 From: Jassi Brar jaswinder.si...@linaro.org

 We can easily keep track of latest EDID from the display and hence avoid
 expensive EDID re-reads over I2C.
 This could also help some cheapo displays that provide EDID reliably only
 immediately after asserting HPD and not later.
 Even with good displays, there is something in OMAPDSS that apparantly
 messes up DDC occasionally while EDID is being read, giving the
   operation stopped when reading edid error.

 Signed-off-by: Jassi Brar jaswinder.si...@linaro.org
 ---
  drivers/video/omap2/dss/hdmi.c            |    1 +
  drivers/video/omap2/dss/ti_hdmi.h         |    2 ++
  drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |   23 ---
  3 files changed, 23 insertions(+), 3 deletions(-)

 diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
 index 900e621..0a8c825 100644
 --- a/drivers/video/omap2/dss/hdmi.c
 +++ b/drivers/video/omap2/dss/hdmi.c
 @@ -764,6 +764,7 @@ static int __init omapdss_hdmihw_probe(struct 
 platform_device *pdev)
       hdmi.ip_data.core_av_offset = HDMI_CORE_AV;
       hdmi.ip_data.pll_offset = HDMI_PLLCTRL;
       hdmi.ip_data.phy_offset = HDMI_PHY;
 +     hdmi.ip_data.edid_len = 0; /* Invalidate EDID Cache */
       mutex_init(hdmi.ip_data.lock);

 Your HDMI patches seem to depend on each other. Please post them as a
 proper patch series, instead of each one separately.

They don't depend functionality wise. Any fix can be accepted
regardless of others.
I deliberately avoided a series, because revision of just one could
require resending 3, otherwise
perfectly OK, patches. I just wanted to limit the noise.
I understand, 'git am' might complain but I think that should be trivial to fix.
I am perfectly OK to resend as a patch series, if you want.

       hdmi_panel_init();
 diff --git a/drivers/video/omap2/dss/ti_hdmi.h 
 b/drivers/video/omap2/dss/ti_hdmi.h
 index cc292b8..4735860 100644
 --- a/drivers/video/omap2/dss/ti_hdmi.h
 +++ b/drivers/video/omap2/dss/ti_hdmi.h
 @@ -178,6 +178,8 @@ struct hdmi_ip_data {
       /* ti_hdmi_4xxx_ip private data. These should be in a separate struct 
 */
       int hpd_gpio;
       struct mutex lock;
 +     u8 edid_cached[256];
 +     unsigned edid_len;
  };
  int ti_hdmi_4xxx_phy_enable(struct hdmi_ip_data *ip_data);
  void ti_hdmi_4xxx_phy_disable(struct hdmi_ip_data *ip_data);
 diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c 
 b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
 index 04acca9..2633ade 100644
 --- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
 +++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
 @@ -243,10 +243,13 @@ static int hdmi_check_hpd_state(struct hdmi_ip_data 
 *ip_data)

       hpd = gpio_get_value(ip_data-hpd_gpio);

 -     if (hpd)
 +     if (hpd) {
               r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_TXON);
 -     else
 +     } else {
 +             /* Invalidate EDID Cache */
 +             ip_data-edid_len = 0;
               r = hdmi_set_phy_pwr(ip_data, HDMI_PHYPWRCMD_LDOON);
 +     }

       if (r) {
               DSSERR(Failed to %s PHY TX power\n,
 @@ -454,6 +457,11 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data *ip_data,
  {
       int r, l;

 +     if (ip_data-edid_len) {
 +             memcpy(edid, ip_data-edid_cached, ip_data-edid_len);
 +             return ip_data-edid_len;
 +     }
 +
       if (len  128)
               return -EINVAL;

 @@ -474,12 +482,21 @@ int ti_hdmi_4xxx_read_edid(struct hdmi_ip_data 
 *ip_data,
               l += 128;
       }

 +     ip_data-edid_len = l;
 +     memcpy(ip_data-edid_cached, edid, l);
 +
       return l;
  }

  bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
  {
 -     return gpio_get_value(ip_data-hpd_gpio);
 +     if (gpio_get_value(ip_data-hpd_gpio))
 +             return true;
 +
 +     /* Invalidate EDID Cache */
 +     ip_data-edid_len = 0;
 +
 +     return false;

 Why is this needed? The HPD interrupt should handle this already. And if
 the HPD interrupt doesn't work for some reason, this won't work either,
 as the user can plug and unplug his HDMI monitors a thousand times
 between two detect calls.

I thought it is almost impossible to unplug-plug cycle the HDMI cable
even twice a second, let alone 1000 times against the 10Hz .detect()
poll  :)Or you mean some relay controlled HDMI switching
mechanism?

 Anyways, that is not the point of this patch. This patch only aims to
avoid un-ncessary EDID reads while doing its best to make sure we
invalidate EDID 'as soon as possible'.

Thanks.
--
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] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-25 Thread Jassi Brar

On 25 June 2012 11:50, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Sat, 2012-06-23 at 13:36 +0530, jaswinder.si...@linaro.org wrote:

  Currenlty HDMI fails to come up in the suspend-resume path.
 This patch helps that real-world scenario.

 What is the problem there? It'd be good to explain the problem in the
 patch description. Does the pm_runtime_get return -EACCES?

Yes, it returns -EACCESS because RPM on devices is disabled during the
period from suspend-start to resume-finished.
--
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] OMAPDSS: HDMI: Cache EDID

2012-06-25 Thread Jassi Brar

On 25 June 2012 13:41, Tomi Valkeinen tomi.valkei...@ti.com wrote:

 I am perfectly OK to resend as a patch series, if you want.

 Yes please.

OK, will do.

   bool ti_hdmi_4xxx_detect(struct hdmi_ip_data *ip_data)
   {
  -     return gpio_get_value(ip_data-hpd_gpio);
  +     if (gpio_get_value(ip_data-hpd_gpio))
  +             return true;
  +
  +     /* Invalidate EDID Cache */
  +     ip_data-edid_len = 0;
  +
  +     return false;
 
  Why is this needed? The HPD interrupt should handle this already. And if
  the HPD interrupt doesn't work for some reason, this won't work either,
  as the user can plug and unplug his HDMI monitors a thousand times
  between two detect calls.
 
 I thought it is almost impossible to unplug-plug cycle the HDMI cable
 even twice a second, let alone 1000 times against the 10Hz .detect()
 poll  :)    Or you mean some relay controlled HDMI switching
 mechanism?

 omapdss doesn't call the detect function, so it can't presume it's used
 in some certain frequency. Also, last time I tested omapdrm, I think
 detect was called once in 5 secs or so.

It's not omapdss.  It's the DRM stack, via the omapdrm, that polls
every 10 secs (not 5).
Sorry I said 10Hz instead of 1/10Hz.

  Anyways, that is not the point of this patch. This patch only aims to
 avoid un-ncessary EDID reads while doing its best to make sure we
 invalidate EDID 'as soon as possible'.

 I'm not sure I understand your point. If the HPD interrupt works
 properly, EDID is invalidated there, and the code in detect is not
 needed. And if the HPD interrupt doesn't work (although I don't see why
 it wouldn't), the code in detect doesn't work. In either case it's not
 correct.

Well, the idea was to tie edid-cache invalidating with de-asserted
HPD, wherever we read HPD. I will drop it.

thnx
--
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] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-25 Thread Jassi Brar

On 25 June 2012 15:00, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Mon, 2012-06-25 at 14:19 +0530, Jassi Brar wrote:
 On 25 June 2012 11:50, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Sat, 2012-06-23 at 13:36 +0530, jaswinder.si...@linaro.org wrote:
 
   Currenlty HDMI fails to come up in the suspend-resume path.
  This patch helps that real-world scenario.
 
  What is the problem there? It'd be good to explain the problem in the
  patch description. Does the pm_runtime_get return -EACCES?
 
 Yes, it returns -EACCESS because RPM on devices is disabled during the
 period from suspend-start to resume-finished.

 So... You didn't answer my first comment, how can the code work?

Sorry, don't know why I thought I didn't miss anything.

 The driver needs to enable the HW and the call to pm_runtime_get() is
 skipped. Won't this lead to crash as the DSS registers are accessed
 without the HW in enabled state?

Hmm...  how does the extant code in hdmi driver ensures DSS is up and running ?
While it does sound important even to my limited knowledge of OMAPDSS,
I see rpm of HDMI, VENC and RFBI only dependent on DISPC, not DSS.

And for DISPC these drivers already hold a reference in their display
enable/resume and keep it until disable/suspend. So we don't lose
DISPC anytime it is really required.

 And what happens if the pm_runtime_get() call is skipped, but 
 pm_runtime_put() is not?

Not sure in what newly introduced scenario by this patch, because
get/put both check for pm_enabled before proceeding. Am I overlooking
something?

thnx
--
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] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-25 Thread Jassi Brar

On 25 June 2012 17:35, Grazvydas Ignotas nota...@gmail.com wrote:
 On Mon, Jun 25, 2012 at 9:20 AM, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Sat, 2012-06-23 at 13:36 +0530, jaswinder.si...@linaro.org wrote:

  Currenlty HDMI fails to come up in the suspend-resume path.
 This patch helps that real-world scenario.

 What is the problem there? It'd be good to explain the problem in the
 patch description. Does the pm_runtime_get return -EACCES?

 On slightly different but related issue, currently OMAPDSS always
 spits lots of backtraces when it's compiled without CONFIG_PM_RUNTIME,
 because pm_runtime_put* always return -ENOSYS without
 CONFIG_PM_RUNTIME. So something like this patch proposes is needed, or
 maybe WARN_ON should check for -ENOSYS, I don't know..

I didn't check, but this patch should already fix that I think ?

IMHO, for omapdss, we need not differentiate between -ENOSYS and
-EACCESS because anyway the ultimate functions dispc_runtime_resume()
and dispc_runtime_suspend()  can't report failure (they always return
0).

-j
--
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] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-25 Thread Jassi Brar

On 25 June 2012 18:11, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Mon, 2012-06-25 at 17:57 +0530, Jassi Brar wrote:
 On 25 June 2012 15:00, Tomi Valkeinen tomi.valkei...@ti.com wrote:

  The driver needs to enable the HW and the call to pm_runtime_get() is
  skipped. Won't this lead to crash as the DSS registers are accessed
  without the HW in enabled state?
 
 Hmm...  how does the extant code in hdmi driver ensures DSS is up and 
 running ?
 While it does sound important even to my limited knowledge of OMAPDSS,
 I see rpm of HDMI, VENC and RFBI only dependent on DISPC, not DSS.

 DSS device is parent to all the DSS subdevices. So when a subdevice is
 enabled, DSS device is enabled first.

 But anyway, I wasn't referring to the DSS part of OMAPDSS, but to
 omapdss generally. If we do this:

 /* this is skipped, if runtime PM is disabled */
 dispc_runtime_get();

I hope you do realize that there is difference between PM is disabled
on a device
and the device is in some low-power state.   pm_runtime_enabled()
checks for the former.
So under this light...

 /* this accesses a register, but the HW is disabled? */
 dispc_read_reg(FOO);

 the H/W is already always enabled if CONFIG_PM_RUNTIME is not defined.

If CONFIG_PM_RUNTIME is indeed defined,  pm_runtime_enabled() will
always return true after pm_runtime_enable()  unless someone disables
it explicitly - omapdss or the RPM stack(during suspend/resume).
OMAPDSS never does so in the lifetime of a driver.  So the only period
in which pm_runtime_enabled() returns false, is when the platform is
suspending, suspended or resuming.


  And what happens if the pm_runtime_get() call is skipped, but 
  pm_runtime_put() is not?
 
 Not sure in what newly introduced scenario by this patch, because
 get/put both check for pm_enabled before proceeding. Am I overlooking
 something?

 Currently (for example) dispc_runtime_get/put call
 pm_runtime_get/put_sync. When somebody uses dispc_runtime_get, the same
 somebody knows it needs to call dispc_runtime_put later.

 Now, what happens if dispc_runtime_get is called when runtime PM is
 disabled (i.e. pm_runtime_get_sync is skipped), but runtime PM is
 enabled later when that somebody calls dispc_runtime_put (i.e.
 pm_runtime_put_sync is _not_ skipped)?

As I said, for omapdss, PM is disabled (not device deactivated) only
during rpm suspend/resume.
And it should be no different than any lock protected section
preempted by suspend-resume before reaching its end.
--
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] OMAPDSS: Check if RPM enabled before trying to change state

2012-06-25 Thread Jassi Brar

On 25 June 2012 19:19, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Mon, 2012-06-25 at 19:01 +0530, Jassi Brar wrote:

  /* this accesses a register, but the HW is disabled? */
  dispc_read_reg(FOO);
 
  the H/W is already always enabled if CONFIG_PM_RUNTIME is not defined.

 If CONFIG_PM_RUNTIME is indeed defined,  pm_runtime_enabled() will
 always return true after pm_runtime_enable()  unless someone disables
 it explicitly - omapdss or the RPM stack(during suspend/resume).
 OMAPDSS never does so in the lifetime of a driver.  So the only period
 in which pm_runtime_enabled() returns false, is when the platform is
 suspending, suspended or resuming.

 Right. So what happens in my example above?

 Normally if the driver does dispc_runtime_get() and dispc_read_reg(),
 the first call will enable the HW so the reg read works.

 But if the pm_runtime is disabled, say, during system suspend, with your
 patch dispc_runtime_get() will just return 0 without doing anything, and
 the dispc_read_reg() will crash because the HW is disabled (because
 nobody enabled it).

Hmm, I am not sure if new calls would/should be made to dispc.c after
the system has suspended and before resumed. That is, anything other
than from runtime_resume/suspend callbacks of DSS, DISPC, HDMI, VENC
and RFBI, which rightly don't touch any dss reg but only
enable/disable a clock.
As we know, a subsystem should make sure any active work is cleared
out before suspending and set some flag so that nothing runs until it
has resumed. I don't say we can't crash the system with this patch,
but then we would be violating rules of suspend-resume.



 As I said, for omapdss, PM is disabled (not device deactivated) only
 during rpm suspend/resume.
 And it should be no different than any lock protected section
 preempted by suspend-resume before reaching its end.

 I'm not sure if I understand... If the driver does dispc_runtime_get()
 while the PM is disabled, say, during system resume, dispc_runtime_get()
 will do nothing and return 0. The driver thinks it succeeded, and will
 call dispc_runtime_put() later.

 Calling the dispc_runtime_put() could happen very soon, while runtime PM
 is still disabled, in which case everything works fine. But there's no
 rule to say dispc_runtime_put() has to be called very soon after
 dispc_runtime_get(). The driver might as well call put later, when
 runtime PM is enabled.

 This would end up with a pm_runtime_put call without a matching
 pm_runtime_get call.

Again, we need to see if there is really some situation where
something new is attempted before the subsystem has resumed. If there
is indeed, maybe omapdss need to flag it's suspended and prevent such
thing until it has resumed.
Btw, even without this patch, when dispc_runtime_get() does return
error under rpm disabled, we disturb the dev.power.usage_count balance
by not calling dispc_runtime_put()
This patch doesn't make everything perfect, but only improve upon the
current situation.

thnx
--
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] OMAPDSS: HDMI: Discard phy_tx_enabled member

2012-06-18 Thread Jassi Brar

On 18 June 2012 13:41, Tomi Valkeinen tomi.valkei...@ti.com wrote:

 Explicitly maintaining HDMI phy power state using a flag is prone to
 race and un-necessary when we have a zero-cost alternative of checking
 the state before trying to set it.

 Why would reading the value from the register be any less racy than
 keeping it in memory?

Racy in the sense that h/w doesn't always hop states according to what
a state variable would expect it to.
Also in this case, phy_tx_enabled modification is unprotected in
ti_hdmi_4xxx_phy_disable().

BTW, coming to think about it, I am not sure what we need the
spin_lock_irqsave() protection for in hdmi_check_hpd_state() ?  It
can't control HPD gpio state change and hdmi_set_phy_pwr() seems too
expensive and is already unprotected elsewhere.

 And reading from memory is probably much faster
 than reading from an HDMI register, so I'm not sure what you mean with
 zero-cost.

Zero-cost in terms of space and bother :)

 But I guess it is simpler, so in that sense the patch is ok. But please
 revise the description.

OK, will do.

Thanks.
--
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] OMAPDSS: HDMI: Discard phy_tx_enabled member

2012-06-18 Thread Jassi Brar

On 18 June 2012 16:24, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Mon, 2012-06-18 at 15:42 +0530, Jassi Brar wrote:

 BTW, coming to think about it, I am not sure what we need the
 spin_lock_irqsave() protection for in hdmi_check_hpd_state() ?  It
 can't control HPD gpio state change and hdmi_set_phy_pwr() seems too
 expensive and is already unprotected elsewhere.

 It's needed when enabling the hdmi output. In phy_enable() the irq is
 requested first, and then the phy_enable() runs hdmi_check_hpd_state().
 So there's a chance to run hdmi_check_hpd_state() from both
 hpd-interrupt and phy_enable() at the same time.

 The hdmi_set_phy_pwr() is not called in many places, but I think there's
 indeed a problem there. It is called after free_irq(), but I think
 (guess) the irq handler could still be running after free_irq. So those
 should be protected by the same spinlock too.

You know TI HDMI better than I do, so I assume your concerns are valid.
So preferably I would move request_threaded_irq() to after
hdmi_check_hpd_state() in ti_hdmi_4xxx_phy_enable()  and convert the
spin_lock_irqsave() in hdmi_check_hpd_state() to some mutex (we don't
want irqs disabled so long as it takes for phy to power on/off).
--
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] OMAPDSS: HDMI: Discard phy_tx_enabled member

2012-06-18 Thread Jassi Brar

On 18 June 2012 17:54, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Mon, 2012-06-18 at 17:16 +0530, Jassi Brar wrote:

 So preferably I would move request_threaded_irq() to after
 hdmi_check_hpd_state() in ti_hdmi_4xxx_phy_enable()  and convert the

 No, you can't move the check. If you move it, the HPD state could change
 between the check and the request_irq, and we'd miss it.

Wouldn't we then get an irq, and hence another hdmi_check_hpd_state(), for that?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAPDSS: HDMI: Discard phy_tx_enabled member

2012-06-18 Thread Jassi Brar

On 18 June 2012 18:41, Tomi Valkeinen tomi.valkei...@ti.com wrote:
 On Mon, 2012-06-18 at 18:37 +0530, Jassi Brar wrote:
 On 18 June 2012 17:54, Tomi Valkeinen tomi.valkei...@ti.com wrote:
  On Mon, 2012-06-18 at 17:16 +0530, Jassi Brar wrote:

  So preferably I would move request_threaded_irq() to after
  hdmi_check_hpd_state() in ti_hdmi_4xxx_phy_enable()  and convert the
 
  No, you can't move the check. If you move it, the HPD state could change
  between the check and the request_irq, and we'd miss it.
 
 Wouldn't we then get an irq, and hence another hdmi_check_hpd_state(), for 
 that?

 No, if we haven't requested the irq yet. So what could happen:

 - initially the cable is unplugged
 - ti_hdmi_4xxx_phy_enable() calls hdmi_check_hpd_state(), nothing is
 done as cable is unplugged
 - cable plugged in
 - ti_hdmi_4xxx_phy_enable() calls request_irq. No irq raised, as the
 cable's state doesn't change.

 We wouldn't know that cable is actually plugged in at that point.

I see, you mean physically (un)plugging the cable could race with phy_enable.

OK, I'll revise the changelog for this patch and submit another patch
converting the spinlock to mutex.

Thanks.
--
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] of: Add generic device tree DMA helpers

2012-06-13 Thread Jassi Brar

On 14 June 2012 04:02, Jon Hunter jon-hun...@ti.com wrote:
 On 06/08/2012 07:04 PM, Arnd Bergmann wrote:

 As I said previously, I think just encoding the direction but not
 the client specific ID (meaning we would have to disambiguate
 the more complex cases that Stephen mentioned using a dma-names
 property) would be the best because it keeps the common case simple,
 but I could live with other things going in there too.

 Ok, so you are saying that there are some DMA controllers where there is
 no channel/request ID and only a direction is needed? So an even simpler
 case than what I had imagined.

I am curious to know which DMA controllers don't need any client ID ?
unless it is Mem-Mem (for which even specifying direction is
meaningless).
Rather, I think specifying direction is even lesser useful, since
usually the client's ID imply the (bi/uni)direction.
--
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] of: Add generic device tree DMA helpers

2012-05-18 Thread Jassi Brar

On 18 May 2012 01:02, Stephen Warren swar...@wwwdotorg.org wrote:

 Now, the DMA node for an on-SoC DMAC would be in soc.dtsi. Typically,
 the DMAC is connected to many on-SoC devices, and hence soc.dtsi would
 need to specify the routing information for all those devices to avoid
 duplicating it in every board.dts. Now, if you have some DMA requests
 that go off-SoC, the board.dts file might want to add to the routing
 table to indicate what clients connect to those DMA requests. However,
 there's no way in the device tree compiler right now to add to a
 property; you can only completely replace it. That would entail
 duplicating the entire routing information from soc.dtsi into each
 board.dts that wanted to add to it - a bad situation. Splitting the
 routing information into chunks in the client nodes avoids this issue
 entirely.

 As already noted by Russell, the dma setup is different than irq or
gpio which deliberately lend to off-SoC attachments. Without fpga'ing,
I find it hard to imagine request-signals going off-SoC.

I only see the issue of different boards sporting a different sub-sets
of clients i.e, a map entry may or may exist for a board but it would
be same for two boards that have it.
 Maybe we could somehow separate out the chan-map and private-data
into board.dts ?
Even if it has to stay in soc.dts, we still have the benefit of having
dmac agnostic client nodes.

cheers!
--
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] of: Add generic device tree DMA helpers

2012-05-16 Thread Jassi Brar

Hi Jon,

On 16 May 2012 06:41, Jon Hunter jon-hun...@ti.com wrote:
 On 05/04/2012 02:01 PM, Jassi Brar wrote:

 +       i2c1: i2c@1 {
 +               ...
 +               dma = sdma 2 1 sdma 3 2;
 +               ...
 +       };

 I see this requires a client driver to specify a particular req_line on a
 particular dma controller. I am not sure if this is most optimal.

 Actually, no. The phandle in the DT specifies the DMA controller to use.
 Then the client simply asks for a channel with a particular property,
 for example, DMA_MEM_TO_DEV (ie. TX) and the channel information is return.

See below.

 I think such client-req_line map should be provided to the dmac controller
 driver via its dt node in some format. The dmac driver could then populate
 a dma_chan, or similar, only for that req_line and not for the unused one
 which otherwise could also have served the same client.

 Ideally the I2C driver should simply ask, say, a channel for TX and another
 for RX, everything else should already be setup via dmac's dt nodes.

 Yes that is the intention here.

But the client is required to specify the dmac that would serve it.
Which is more
than simply asking for some suitable channel.

If you read the whole exchange between I and Stephen, we converged on a
scheme of clients' node having nothing to specify and DMAC told all about
every client in one palce. Which resembles closer to reality and is
much simpler.
I already started on a patchset and should be able submit for review
in a day or two.
--
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] of: Add generic device tree DMA helpers

2012-05-16 Thread Jassi Brar

On 16 May 2012 21:31, Jon Hunter jon-hun...@ti.com wrote:
 On 05/16/2012 08:15 AM, Jon Hunter wrote:
 Hi Jassi,

 On 05/16/2012 07:37 AM, Jassi Brar wrote:
 Hi Jon,

 On 16 May 2012 06:41, Jon Hunter jon-hun...@ti.com wrote:
 On 05/04/2012 02:01 PM, Jassi Brar wrote:

 +       i2c1: i2c@1 {
 +               ...
 +               dma = sdma 2 1 sdma 3 2;
 +               ...
 +       };

 I see this requires a client driver to specify a particular req_line on a
 particular dma controller. I am not sure if this is most optimal.

 Actually, no. The phandle in the DT specifies the DMA controller to use.
 Then the client simply asks for a channel with a particular property,
 for example, DMA_MEM_TO_DEV (ie. TX) and the channel information is return.

 See below.

 I think such client-req_line map should be provided to the dmac 
 controller
 driver via its dt node in some format. The dmac driver could then populate
 a dma_chan, or similar, only for that req_line and not for the unused one
 which otherwise could also have served the same client.

 Ideally the I2C driver should simply ask, say, a channel for TX and 
 another
 for RX, everything else should already be setup via dmac's dt nodes.

 Yes that is the intention here.

 But the client is required to specify the dmac that would serve it.
 Which is more
 than simply asking for some suitable channel.

 No this is not the case with what I propose. The client knows nothing
 about the dmac.

 By the way, I do see your point. You wish to describe the all the
 mappings available to all dma controllers and then set a mapping in the
 device tree. Where as I am simply setting a mapping and do not list all
 other possibilities (assuming that there some).

 What is still unclear to me, is if you use this token approach how
 readable is the device-tree? For example, if you have a client that can
 use one of two dmac and for each dmac the request/channel number is
 different, then by using a global token how can I determine what the
 options available for this client are?

Simple - you/client need not know about any option at all :)

Client driver would simply request some channel and if it
doesn't get it, it bails out.

It would be the DMACs' DT node that would contain that info.

 Take your example ...

 mmc1: mmc@13002000 {
        ...
        dma_tx = 891   //some platform-wide unique value
        dma_rx = 927   //some platform-wide unique value
        ...
 };

 DMAC's Node:-

 pdma2: pdma@1080 {
         ...
        dma_map = 891, 7,       // Map mmc1_tx onto i/f 7
                  927, 8,       // Map mmc1_rx onto i/f 8
        ...
 };

 But now I have another dmac which has the following options ...

 pdma1: pdma@1000 {
         ...
        dma_map = 598, 2,       // Map mmc1_tx onto i/f 2
                  230, 3,       // Map mmc1_rx onto i/f 3
        ...
 };

No, rather the pdma1 node should look like

 pdma1: pdma@1000 {
         ...
        dma_map = 891, 2,       // Map mmc1_tx onto i/f 2
                   927, 3,       // Map mmc1_rx onto i/f 3
        ...
};

Because the tokens 891 and 927 are came from the client's node/driver.

After the DMAC driver has probed both pdma-0  1, it would
know that MMC1 could be served by 2 DMACs. And basically
its the dmac driver that should be making about routing decisions,
not the client.

Btw, everything remains same, only we have now decided to not
use tokens but phandle+req_sig_ids instead.
--
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] of: Add generic device tree DMA helpers

2012-05-16 Thread Jassi Brar

On 16 May 2012 21:45, Stephen Warren swar...@wwwdotorg.org wrote:
 On 05/16/2012 10:01 AM, Jon Hunter wrote:
 ...
 By the way, I do see your point. You wish to describe the all the
 mappings available to all dma controllers and then set a mapping in the
 device tree. Where as I am simply setting a mapping and do not list all
 other possibilities (assuming that there some).

 What is still unclear to me, is if you use this token approach how
 readable is the device-tree? For example, if you have a client that can
 use one of two dmac and for each dmac the request/channel number is
 different, then by using a global token how can I determine what the
 options available for this client are?

 Take your example ...

 mmc1: mmc@13002000 {
         ...
         dma_tx = 891   //some platform-wide unique value
         dma_rx = 927   //some platform-wide unique value
         ...
 };

 I believe those properties (in the DMA client) should be completely
 omitted; there's no need for them.

 Also, we definitely should not be using some platform-wide unique
 value, but rather the phandle of the DMA client, plus some
 client-defined client channel ID. ...

 (oh, and - rather than _ is idiomatic for DT property names)

 DMAC's Node:-

 pdma2: pdma@1080 {
          ...
       dma_map = 891, 7,       // Map mmc1_tx onto i/f 7
                 927, 8,       // Map mmc1_rx onto i/f 8
       ...
 };

 So this would become:

 pdma2: pdma@1080 {
        ...
        dma-map =
                ... entries for channels 0.. 6
                mmc1, 0,       // Map mmc1_tx onto i/f 7
                mmc1, 1,       // Map mmc1_rx onto i/f 8
                ... ;
        ...
 };

 This (a) follows existing DT practice of using phandle + specifier, and
 (b) makes it easy to know exactly what clients you're talking about,
 since all you need to do is search for the label mmc1 throughout the DT.

 But now I have another dmac which has the following options ...

 pdma1: pdma@1000 {
          ...
       dma_map = 598, 2,       // Map mmc1_tx onto i/f 2
                 230, 3,       // Map mmc1_rx onto i/f 3
       ...
 };

 Which would become something very similar:

 pdma1: pdma@1000 {
        ...
        dma-map =
                ... entries for channels 0.. 1
                mmc1, 0,       // Map mmc1_tx onto i/f 2
                mmc1, 1,       // Map mmc1_rx onto i/f 3
                ... ;
        ...
 };

 Note that dma-map here is describing the list of DMA requests that the
 DMA controller knows about. As far as the binding goes, these are
 irrelevant to channels; only the driver for the DMAC knows whether it
 needs to use a specific channel ID to service a particular DMA request
 signal, or not.


OK, my guts feel people might be interested in what's cooking on
my side. I started with the binding text first and then would write
code based upon that approach.

The following might be tweaked as I look deeper into client and DMAC
drivers while deciding upon what the helper functions should be optimally...

 8 


Generic binding to provide a way to provide the client-channel map and
other dmac specific parameters to the dma controller driver

DMA Model:-
  Only the most common characteristics of a dma setup are assumed
in this binding.
Client: Some h/w controller that could request a DMA controller in
the system to perform data transfer on its behalf. Example SPI, MMC,
I2S etc.
DMAC: A DMA Controller instance. Example, PL330, PL08X, SDMA etc.

 The assumed model of the DMAC, in this binding, has P peripheral
interfaces (P request signals) that could request a data transfer
and C physical channels that actually do the data transfers, hence,
at most C out of P peripherals could be served by the DMAC at any
point of time. Usually C := P, but not always. Usually, any of the
physical channels could be employed by the DMAC driver to serve any
client.
 The DMAC driver identifies a client by its i/f number, 'peri_id'
on the given DMAC. For example, TX for SPI has 7th while RX_TX
(half-duplex) for MMC has 10th peripheral interface (request-signal)
on a given DMAC. Usually, any of the physical channels could be
employed by the DMAC driver to serve a client.

* DMA Controller

Required property:

- #map-cells: Number of elements in each chan-map entry.
At least 3 elements are required by this binding.

- chan-map: List of entries that specify clients' 'peri_id'.
and also possibly DMAC specific per-client data.
The first element of each entry being the client's
phandle. The second the direction of data transfer
w.r.t the client 1 for RX, 2 for TX and  3 for RX|TX.
The third the 'peri_id' of the client's request signal
on the DMAC.

Optional properties:

- #dma-peri-ifs: P, usually the DMAC driver would simply assume the

Re: [PATCH V3 1/2] of: Add generic device tree DMA helpers

2012-05-16 Thread Jassi Brar

On 16 May 2012 22:42, Jon Hunter jon-hun...@ti.com wrote:

 What is still unclear to me, is if you use this token approach how
 readable is the device-tree? For example, if you have a client that can
 use one of two dmac and for each dmac the request/channel number is
 different, then by using a global token how can I determine what the
 options available for this client are?

 Simple - you/client need not know about any option at all :)

 Client driver would simply request some channel and if it
 doesn't get it, it bails out.

 It would be the DMACs' DT node that would contain that info.

 Yes, but what if I am doing some custom application and want to modify
 the mapping that is being used? So I just wanted to make sure it is easy
 to understand assuming that you understand what your h/w is capable of.

Any scenario when a client would want to choose which dma controller
it runs on?

Because when we say a client could be provided a channel on any of the
two given dmacs, it implies that the client wouldn't feel any difference.
--
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] of: Add generic device tree DMA helpers

2012-05-16 Thread Jassi Brar

On 17 May 2012 01:12, Arnd Bergmann a...@arndb.de wrote:

 Generic binding to provide a way to provide the client-channel map and
 other dmac specific parameters to the dma controller driver

 DMA Model:-
   Only the most common characteristics of a dma setup are assumed
 in this binding.
 Client: Some h/w controller that could request a DMA controller in
 the system to perform data transfer on its behalf. Example SPI, MMC,
 I2S etc.
 DMAC: A DMA Controller instance. Example, PL330, PL08X, SDMA etc.

  The assumed model of the DMAC, in this binding, has P peripheral
 interfaces (P request signals) that could request a data transfer
 and C physical channels that actually do the data transfers, hence,
 at most C out of P peripherals could be served by the DMAC at any
 point of time. Usually C := P, but not always. Usually, any of the
 physical channels could be employed by the DMAC driver to serve any
 client.
  The DMAC driver identifies a client by its i/f number, 'peri_id'
 on the given DMAC. For example, TX for SPI has 7th while RX_TX
 (half-duplex) for MMC has 10th peripheral interface (request-signal)
 on a given DMAC. Usually, any of the physical channels could be
 employed by the DMAC driver to serve a client.

Btw, just to be clear... this is not _my_ setup.
The dma controller manuals might use different terms but underneath
most(if not all) dma controllers fit this model. At lease every dmac
on Samsung SoCs and ARM's PL330 and PL08x does.
In fact, I have trouble imaging some dmac not conforming to this model...
but then again it could be just me.


 I'm still anything but convinced by this model. Basically it's the
 exact opposite of what we do for every other subsystem (irq, pinctrl,
 regulator, gpio, ...), where the device using some infrastructure
 contains the information about who provides it, whereas here you
 move all the information into the device that provides the functionality,
 and have no handle in the device using it by which the driver can
 identify it.

The idea was that a client shouldn't need to know/tell which dma controller
serves it or which peripheral interface of a dma controller.
I think in future third-party device IPs, like ARM's Primecell, are only gonna
get more common so it makes even more sense.


 I believe that it can work and that it solves the problem you are
 faced with at minimal complexity, but you put the burden of this
 complexity on everybody who does not have this issue, and make
 the general binding confusing and harder to read.

I am sorry if I gave you the wrong impression, but I didn't intend to just
scratch my personal itch. I truly believed I and Stephen reached a generic
solution i.e, as much as it could be.


 It also adds much more data to the device tree (in source and binary form)
 because you need to describe every device using a dma controller
 and have a label to reference it.

I don't see why one can't add entries to chan-map as and when more
clients appear for the DMAC ? It should be perfectly ok to specify less
than the maximum possible clients in chan-map.
Requiring labels - yes, doesn't sound very nice, but many nodes already do.

 More importantly, you make it very hard to add devices in a board file
 to a dma controller that already has descriptions for some channels,
 because you cannot easily extend the chan-map unless you rewrite all of it.

I am not sure I understand the point.

 We really need something simpler than this for the common case.
 I have already made suggestions for how to make it still possible
 to cover the corner case of multiple dma engines connected to the
 same slave, which would keep the complexity local to those devices
 that actually need it.

Thanks $DEITY, I posted a preview. Maybe I should put it on hold.
And thank you for speaking up immediately. Really!
--
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] of: Add generic device tree DMA helpers

2012-05-16 Thread Jassi Brar

On 17 May 2012 05:29, Stephen Warren swar...@wwwdotorg.org wrote:

 Generic binding to provide a way to provide the client-channel map and
 other dmac specific parameters to the dma controller driver

 DMA Model:-
   Only the most common characteristics of a dma setup are assumed
 in this binding.
 Client: Some h/w controller that could request a DMA controller in
 the system to perform data transfer on its behalf. Example SPI, MMC,
 I2S etc.
 DMAC: A DMA Controller instance. Example, PL330, PL08X, SDMA etc.

  The assumed model of the DMAC, in this binding, has P peripheral
 interfaces (P request signals) that could request a data transfer
 and C physical channels that actually do the data transfers, hence,
 at most C out of P peripherals could be served by the DMAC at any
 point of time. Usually C := P, but not always. Usually, any of the
 physical channels could be employed by the DMAC driver to serve any
 client.
  The DMAC driver identifies a client by its i/f number, 'peri_id'
 on the given DMAC. For example, TX for SPI has 7th while RX_TX
 (half-duplex) for MMC has 10th peripheral interface (request-signal)
 on a given DMAC. Usually, any of the physical channels could be
 employed by the DMAC driver to serve a client.

 I'm still anything but convinced by this model. Basically it's the
 exact opposite of what we do for every other subsystem (irq, pinctrl,
 regulator, gpio, ...), where the device using some infrastructure
 contains the information about who provides it, whereas here you
 move all the information into the device that provides the functionality,
 and have no handle in the device using it by which the driver can
 identify it.

 Yes, I guess this is backwards. But, the HW is a little different too;
 GPIOs (and probably interrupts) don't have multiple places they could
 come from.

 The problem is that if we did something like this in the DMA client:

 dma-reqs = dmac1 DMAC1_DMA_REQ_6 dmac1 DMAC1_DMA_REQ_8;

 how do we know if the client is emitting 2 DMA request signals that get
 routed to two different inputs on the DMA controller, or whether this is
 two alternatives for the same signal.

FWIW, I wouldn't lose sleep over the possibility of redundancy on same DMAC.
If a client's request signal is routed to 2 inputs, they are usually
on different DMACs.
--
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] of: Add generic device tree DMA helpers

2012-05-12 Thread Jassi Brar

On 12 May 2012 05:21, Stephen Warren swar...@wwwdotorg.org wrote:
 On 05/11/2012 03:06 PM, Jassi Brar wrote:
 On 12 May 2012 00:58, Stephen Warren swar...@wwwdotorg.org wrote:
 On 05/10/2012 01:59 PM, Jassi Brar wrote:
 ...
 client0: i2s {
   /* has 2 DMA request output signals: 0, 1 */
 };

 client1: spdif {
   /* has 2 DMA request signals: 0, 1 */
 };

 Do we also need to somehow tag these signals for the client to
 differentiate between TX and RX channel ?

 Yes, the client's DT binding would certainly need to describe how many
 DMA request signals its HW generates, and give a unique ID to each. The
 driver would need to request a DMA channel for a specific one of its DMA
 requests.

 Did I read give a unique ID to each correctly ?

 It'd be unique relative to that individual device or DT node, not at any
 larger scope.

OK.

 Could you please take some time out to jot down an example of how a
 typical client's dma specifier should look.

 With this proposal, I'm not sure that the client DT node would need any
 DMA information at all, at least nothing that identifies which DMA
 controllers, channels, or requests are required to service this
 node/device's DMA requests - that routing information is all represented
 in the DMA controller itself.

 (I think Arnd made the following point earlier in this thread):

 If you did need to put any other information in DT, then that probably
 would go in the DMA client node, since it'd presumably be the same
 irrespective of which DMA controller got used. However, that information
 presumably wouldn't be needed in DT at all, since the driver would know
 it, since it'd be a facet of the HW.

 Note: I'm thinking things like DMA physical address (presumably an
 offset from the reg property), DMA access size (presumably a fixed
 property of the HW), DMA burst size (presumably a property of the HW,
 although at least some HW can be programmed to raise the DMA request
 signal with a varying number of FIFO entries free, so not fixed), etc.

Yeah, neither did I think a client node should tell more than IDs of
its channels
- which we now decide to simply mention in its binding.

How do I know if you or someone is interested in reworking the patchset
or want me to give it a try?

Thanks,
Jassi
--
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] of: Add generic device tree DMA helpers

2012-05-11 Thread Jassi Brar

On 12 May 2012 00:58, Stephen Warren swar...@wwwdotorg.org wrote:
 On 05/10/2012 01:59 PM, Jassi Brar wrote:

 I think arbitrary numerical tokens are a reasonable price to pay for the
 robustness and simplicity they bring.

 I have to disagree here.

 Using phandle+ID is almost as simple, and much more flexible. Global IDs
 have a number of disadvantages:

 a) You have to somehow keep them unique. Even with just a single .dts
 file, that's going to be a little painful since there's no central table
 of these IDs.

 What if the DT is composed of a bunch of chunks that represent pluggable
 boards, which may be mixed/matched together depending on what the user
 actually plugged in? Then, you have to be very careful to keep the n
 different files' numbering ranges segregated, or conflicts will occur.

You might want to revisit this point after working out the finer details of what
you propose for a client's specifier :)


 Well, I doubt if there would ever be enough such platforms to warrant a
 new generic framework. For now, I would leave that to be a matter between
 the dmac driver and its DT node.

 Similarly let every dmac, being unique, require DT data in it's own custom
 format - I don't believe we can find a generic DT format for every kind of
 dmac that does exist or would exist. (For ex, you found a way for RMK's
 mux'ed req_lines, but what about assigning priorities to clients which is
 possible with PL08X dmacs but not most others?)

 Good question. Again thought that sounds a little like policy, so
 perhaps should be negotiated at runtime rather than described in DT?

As much as we love everything to be runtime configurable, I am afraid it
can't be. We can't add new calls to the dmaengine api for simply
supporting specific features of various dmacs (priority in this case)
Because that would mean ideally every client going through the mostly
useless ritual of querying/setting those features and most dmacs
just 'pretending' to support some, except for the rare ones that actually do.
So as much as these sound like policy, they would usually be directly
hardcoded via dt for the machine or deducted by the dmac driver.


 client0: i2s {
   /* has 2 DMA request output signals: 0, 1 */
 };

 client1: spdif {
   /* has 2 DMA request signals: 0, 1 */
 };

 Do we also need to somehow tag these signals for the client to
 differentiate between TX and RX channel ?

 Yes, the client's DT binding would certainly need to describe how many
 DMA request signals its HW generates, and give a unique ID to each. The
 driver would need to request a DMA channel for a specific one of its DMA
 requests.

Did I read give a unique ID to each correctly ?
Could you please take some time out to jot down an example of how a
typical client's dma specifier should look.

FWIW, I think I can live with what you propose. Let us go for the kill.

Thanks.
--
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] of: Add generic device tree DMA helpers

2012-05-10 Thread Jassi Brar

On 10 May 2012 22:30, Stephen Warren swar...@wwwdotorg.org wrote:
 On 05/09/2012 03:38 PM, Jassi Brar wrote:

 One point is about 'qos' here something like bandwidth allocation.
 If the dmac driver knew up-front the max possible clients that could be
 active simultaneously, it could divide the bandwidth accordingly.
 Again, the example of PL330 employing 2physical channels for better
 service - LLI (you got it right), where even 1 physical channel would
 also have served only not as reliably. I believe there would be
 more such scenarios.

 QoS seems like policy to me, whereas DT is more about describing the HW.
 Is DT the correct place to describe QoS policy?

 I guess you are talking more about deriving policy from the description
 of HW, i.e. how many client described in DT.

Yeah, that's what I meant.

 However, for some reason that seems dangerous to me; what if clients
 can be instantiated some other way?

The other way could be hotplug ?
Anyway in those machines every channel would be populated
and dmac driver would always account for the all-ports-plugged case.


 For a 1:1 mapping (or 1:n mapping in HW with static selection specified
 in the DT) between DMA client and DMA controller, perhaps the controller
 can indeed make QoS decisions based on which (how many) clients are
 connected to it.

 However, if a DMA client can be serviced by more than 1 DMA engine, and
 the decision as to which DMA engine to use occurs at run-time by the DMA
 driver core, rather than being statically configured in the DT, then the
 DMA controller drivers cannot know ahead of time which will be used

I think the dmac driver would make use of the routing flexibility to sustain its
'qos', and not the other way around (decide qos based on which one of the
two dmacs would provide a channel to a client in future).
Anyways, so far only Samsung SoCs seem to have that flexibility/redundancy
and I have never had anyone asking for that runtime decision making.


 The minor difference being, you use the {request-signal, phandle} pair
 to find the right channel, I used only 'token'.

 That's a pretty big difference, I think.

 In your proposal, every token was globally unique (well, within the 1 DT
 file). I'd rather not see any more global numbering schemes.

Probably my shallow experience, but globally unique, within a file sounds
like an oxymoron :)
I think arbitrary numerical tokens are a reasonable price to pay for the
robustness and simplicity they bring.


 Now, DMA requests are signals /from/ a DMA client to a DMA controller
 (send more data please, or pull more data please). Hence, I assert
 that the phandle should refer to the DMA client, not the DMA controller.

OK, though we may just want to convey information about the h/w setup
from the s/w POV, rather than info to simulate the h/w  ;)
For ex, the dma api and controller drivers don't really care about
the fact that the client's driver must set some bit to trigger operation,
whereas some simulator would need to care about that.

Anyways, I am OK with whatever works well and make things simpler.



 Also note that, a client's dma specifier becomes perfectly general
 and future-proof

    client1: spdif {
           dma_tx = 278
           dma_rx = 723
     };

 otherwise the following representation

     client1: spdif {
                dma = sdma 2 1 sdma 3 2;
      };

 could break with some weird dma setups (IIANW Russell already finds
 it wouldn't work for him).

 To solve Russell's HW, we need some way of representing the mux directly
 in DT irrespective of how the DMA controller or DMA client specify what
 they're connected to. Anything else isn't representing the HW in DT.

 Also, who knows how to control the mux? We need that to be fully
 general, and so the mux itself really needs some kind of driver which
 the DMA core or DMA controller can call into when the channel is
 allocated in order to set up the mux. Right now, Russell's driver calls
 in the a platform-/board-provided callback, but we should really
 architect a generic driver framework for this.

Well, I doubt if there would ever be enough such platforms to warrant a
new generic framework. For now, I would leave that to be a matter between
the dmac driver and its DT node.
Similarly let every dmac, being unique, require DT data in it's own custom
format - I don't believe we can find a generic DT format for every kind of
dmac that does exist or would exist. (For ex, you found a way for RMK's
mux'ed req_lines, but what about assigning priorities to clients which is
possible with PL08X dmacs but not most others?)

So, I would strive only to make clients' dma specifier generic.

 client0: i2s {
   /* has 2 DMA request output signals: 0, 1 */
 };

 client1: spdif {
   /* has 2 DMA request signals: 0, 1 */
 };

Do we also need to somehow tag these signals for the client to
differentiate between TX and RX channel ?
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body

Re: [PATCH V3 1/2] of: Add generic device tree DMA helpers

2012-05-09 Thread Jassi Brar

On 10 May 2012 00:40, Stephen Warren swar...@wwwdotorg.org wrote:
 On 05/08/2012 01:09 PM, Jassi Brar wrote:

 There is important difference between providing data via clients
 during runtime vs providing info about every client to the dmac driver
 at one go during its probe.

 I certainly see that there is a difference.

 I don't understand why it's an important difference; I think everything
 needs to be handled at run-time no matter what:

 Just because there is information in the DT that this client DT node
 uses this channel/request-ID/... of this DMA controller doesn't mean
 that the driver for the client is going to be loaded, or that SW running
 on the system is going to cause that driver to actually be opened and do
 any DMA. As such, any resource allocation rules etc. must be evaluated
 only if/when a driver actually requests/uses a DMA channel, so having
 all the information up front doesn't seem like a theoretically
 possible thing anyway.

One point is about 'qos' here something like bandwidth allocation.
If the dmac driver knew up-front the max possible clients that could be
active simultaneously, it could divide the bandwidth accordingly.
Again, the example of PL330 employing 2physical channels for better
service - LLI (you got it right), where even 1 physical channel would
also have served only not as reliably. I believe there would be
more such scenarios.

Another minor benefit could be that the dmac driver populate only
as many struct dma_chan as there are actually clients on the
machine (and this population has to be done during probe).
It could mean ~8 instead of ~40 channels populated on a machine.
Note, a PL330 dmac can have 32 channels, OMAP's has 128

Most importantly, I just think it's a cleaner approach.


 Very similar to this, in order to support your point that a given client
 could potentially use a channel from either of 2 different DMA
 controller instances, you don't know until run-time which controller
 will be used, so can't have up-front information in this scenario
 either, even if the DMA does actually take place.

Sorry, perhaps I wasn't clear enough.
A client sees _no_ difference between the two channels that could serve it.
And it can't start using two, if two are available.
Client just needs one suitable channel on whichever dmac that might be.
If the channel for a client is to be switched runtime, that decision
should lie with the dmac driver, not the client. And I am not really
after implementing that runtime decision support.


 Solving (b) seems to require something very roughly like:

 dma-controller@0 {
    compatible = foo,dmac-xxx;
    dma-requests = 
                client0 0 // DMAC req 0 is client0's 0th req output
                client0 1
                client1 0
                client1 1
                client2 0
                ...;
 };

 dma-controller@1 {
    compatible = bar,dmac-yyy;
    dma-requests = 
                // Supports some client modules, but not the same ones
                client0 0
                client1 0
                client3 0
                ...;
 };

 client0: i2s {
    /* has 2 DMA request output signals: 0, 1 */
 };

 client1: spdif {
    /* has 2 DMA request signals: 0, 1 */
 };

 client2: foo {
    /* has 1 DMA request signal: 0 */
 };

 client3: bar {
    /* has 1 DMA request signal: 0 */
 };

 and then having the core DMA code have an API like:

 channel = dma_request(clients_dma_req_output_id)

 which looks in each DMA controller's dma-requests list for the client's
 phandle and matching dma_req_output_id.

Almost what I proposed in my third post in this thread !!

The minor difference being, you use the {request-signal, phandle} pair
to find the right channel, I used only 'token'.

Please note, with this method we specify info about every client at one
go and via dmac's DT node - hence those benefits.

Also note that, a client's dma specifier becomes perfectly general
and future-proof
   client1: spdif {
  dma_tx = 278
  dma_rx = 723
};

otherwise the following representation
client1: spdif {
   dma = sdma 2 1 sdma 3 2;
 };
could break with some weird dma setups (IIANW Russell already finds
it wouldn't work for him).

I am arguing for (b) for the above mentioned reasons, and not because
I want to have clients switching channels in runtime.

Thanks,
Jassi
--
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] of: Add generic device tree DMA helpers

2012-05-08 Thread Jassi Brar

On 8 May 2012 22:05, Stephen Warren swar...@wwwdotorg.org wrote:

 The data doesn't need to be part of the DMA controller node in order for
 the DMA controller driver to be the entity interpreting it.

I rather say, if the dma controller driver is the entity going to interpret the
data, why not provide the data directly through its node at one go?

There is important difference between providing data via clients
during runtime vs providing info about every client to the dmac driver
at one go during its probe.


 The advantage here is that:

 * The specifier is stored in the client, not the IRQ/GPIO/DMA
 controller's node, so it's located right at the usage site, which
 typically makes working out what resources a client uses easier.

A client doesn't really need some n'th channel of some m'th dma controller.
A client simply needs one channel for transmitting data whichever
platform it be.
So {n,m}_channel isn't really a required resource, dma_tx is, which I  still
specify in the client's node.
  The fact that this information is simply fwd by the client as such to the dmac
(via utility code), makes the ritual even more pointless.


 This also keeps client-specific information out of the provider node,
 allowing it to be fully generic.

A typical Samsung SoC has 2 peripheral DMA controllers and
about 40 possible clients. Of which ~20 clients could be served by either of
the two dmacs. Of course hardly ever a machine has 10 clients. It would be
desirable if the dma driver doesn't populate the unused 54(32+32-10) channels,
 presumably also reserving limited resources for each, on the machine.

Consider this example ...
A PL330 instance has 8 physical channels and 32 client i/f.
So only 8 client reqs could be active at any time.
PL330 doesn't provide a way to program a true LLI transfer(very useful for
audio) using a single physical channel. However, we could emulate
true LLI if we employ 2 physical channels for one audio channel request.

Obviously, if a machine had 7 or lesser clients, one would freely employ
2 physical channels for audio and yet not worry about starving other clients.
How would we achieve this if the dmac driver was initialized as fully
generic ?


 That encoding(channel_id) would be dma
 controller specific and if we also manage to contain it within fixed number
 of bytes we could also have common helpers for fetching it,

 I don't think there's any need for it to fit into a fixed number of  bytes

Yeah, I realized that soon after posting.

I think I have run out of ways to explain myself better. FWIW, I won't
object to whatever mechanism folks decide after knowing my concerns.

Best Regards,
-Jassi
--
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] of: Add generic device tree DMA helpers

2012-05-07 Thread Jassi Brar

On 7 May 2012 21:23, Stephen Warren swar...@wwwdotorg.org wrote:
 On 05/05/2012 11:10 AM, Jassi Brar wrote:

 Hmm... there ought to be a way by which a client is handed a random 'token'
 via its dt node and similarly the dmac informed which channel (and with what
 capabilities) to allocate should it see a request coming with that token.

 I think that's the whole point of the patch.

 However, the token needs to be some driver-specific struct, since the
 required information may be more than just a single channel or request
 ID in general.

Well, what I call 'token' could just as well be some numerical hash of
what you call 'driver-specific struct'.
And I never thought we could do without h/w specific information,
just that peculiarities lie with the dmac controllers and that's where
the discerning should happen.

Our opinions differ in that, I believe client side shouldn't need to
parse/decode the h/w specific parameters from data gotten via DT
(what I call 'token' and you 'driver-specific struct') - because that is
the key to having common client drivers working with different dma
controllers. And yes, I don't think we could find a future proof generic
and simple enough representation of dma resource specification in a
DT node :)

Instead, I suggest we encode the finer details of each channel-request
in node of the dma controller matched against the 'token' assigned
to the respective clients. That encoding(channel_id) would be dma
controller specific and if we also manage to contain it within fixed number
of bytes we could also have common helpers for fetching it, though
it still would need to be decoded by dmac controller driver - which I think
we can't do without, considering the variety of dma floating around.


 That way dmac and client drivers using DT could do away with the filter_fn.

 Roughly speaking (I am not very well versed with DT syntax)

 Client Node:-

 mmc1: mmc@13002000 {
         ...
         dma_tx = 891   //some platform-wide unique value
         dma_rx = 927   //some platform-wide unique value
         ...
  };

 I believe we specifically don't want to introduce any global concept of
 DMA channel ID, either within the kernel, or at the device tree level.

I don't think these tokens have to be global.
They just need to be common for .dts that define client and dmac nodes
- which would usually be the same file. They just have to be unique across
all dma clients on a given machine.
--
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] of: Add generic device tree DMA helpers

2012-05-05 Thread Jassi Brar

On 5 May 2012 00:53, Arnd Bergmann a...@arndb.de wrote:
 On Friday 04 May 2012, Jassi Brar wrote:
 I see this requires a client driver to specify a particular req_line on a
 particular dma controller. I am not sure if this is most optimal.

 I think such client-req_line map should be provided to the dmac controller
 driver via its dt node in some format. The dmac driver could then populate
 a dma_chan, or similar, only for that req_line and not for the unused one
 which otherwise could also have served the same client.

 Ideally the I2C driver should simply ask, say, a channel for TX and another
 for RX, everything else should already be setup via dmac's dt nodes.

 If I understand you correctly, you think we can generalize the dma-request
 data in a way that we don't need to pass a specific dma engine phandle.
 I very much doubt this is possible, given how different the requirements
 are for each of the engines we support. You really need to pass a specific
 phandle so that we can find the code that can interpret the data in a
 meaningful way.

Hmm... there ought to be a way by which a client is handed a random 'token'
via its dt node and similarly the dmac informed which channel (and with what
capabilities) to allocate should it see a request coming with that token.

That way dmac and client drivers using DT could do away with the filter_fn.

Roughly speaking (I am not very well versed with DT syntax)

Client Node:-

mmc1: mmc@13002000 {
...
dma_tx = 891   //some platform-wide unique value
dma_rx = 927   //some platform-wide unique value
...
 };


Say the PL330 chapter of SoC's trm specifies that channel_id for
MMC1_TX is 7 and MMC1_RX is 8 on second instance of PL330.

DMAC's Node:-

pdma2: pdma@1080 {
 ...
dma_map = 891, 7,   //Map mmc1_tx onto i/f 7
  927, 8,  //Map mmc1_rx onto i/f 8
  487, 9,  // Map other dma clients
...
};

The dma_map could also encode features like duplex, priority etc
depending on the needs of the client and capability of the dmac.
(The channel_id could very well be an composite value specifying
more than one parameter basically a value private to the dmac driver)

As a positive side effect, the dmac controller could populate channels only
for which some client exists on the machine.

Also the decision to map a client onto one of two or more supporting dmacs
is made here - token 927 wouldn't appear in dma_map of pdma1 even
if it also could do RX for MMC1.

The solution seems so simple that I am already cringing at the thought
of having overlooked something fundamental. Please clarify :)
--
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] of: Add generic device tree DMA helpers

2012-05-04 Thread Jassi Brar

On 1 May 2012 02:47, Jon Hunter jon-hun...@ti.com wrote:
 From: Jon Hunter jon-hun...@ti.com

 This is based upon the work by Benoit Cousson [1] and Nicolas Ferre [2]
 to add some basic helpers to retrieve a DMA controller device_node and the
 DMA request/channel information.

 Aim of DMA helpers
 - The purpose of device-tree (as far as I understand), is to describe the
  capabilites of the hardware. Thinking about DMA controllers purely from the
  context of the hardware to begin with, we can describe a device in terms of
  a DMA controller as follows ...
        1. Number of DMA controllers
        2. Number of channels (maybe physical or logical)
        3. Mapping of DMA requests signals to DMA controller
        4. Number of DMA interrupts
        5. Mapping of DMA interrupts to channels
 - With the above in mind the aim of the DT DMA helper functions is to extract
  the above information from the DT and provide to the appropriate driver.
  However, due to the vast number of DMA controllers and not all are using a
  common driver (such as DMA Engine) it has been seen that this is not a
  trivial task.

Sorry for being slow, but so far I thought DT is only to provide _h/w_
specific info
to the _controller_ drivers. It was not supposed to provide any info
pertaining to
some API (dmaengine in this case).

And I believe this is one of few situations where we are better off
not generalizing
the interface - pass controller specific info in the controller
driver's specified format.
Generalizing only seems to complicate things here, when we anyway have machine
specific dtb, which could always have clients requesting and the
controllers given
dma's info in controller driver's specific format.

Perhaps I am overlooking the need to generalize. If you think so, please help me
understand by pointing out some use case for it.

Thanks.
--
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] of: Add generic device tree DMA helpers

2012-05-04 Thread Jassi Brar

On 4 May 2012 20:47, Jon Hunter jon-hun...@ti.com wrote:
 Hi Jassi,

 On 05/04/2012 01:56 AM, Jassi Brar wrote:
 On 1 May 2012 02:47, Jon Hunter jon-hun...@ti.com wrote:
 From: Jon Hunter jon-hun...@ti.com

 This is based upon the work by Benoit Cousson [1] and Nicolas Ferre [2]
 to add some basic helpers to retrieve a DMA controller device_node and the
 DMA request/channel information.

 Aim of DMA helpers
 - The purpose of device-tree (as far as I understand), is to describe the
  capabilites of the hardware. Thinking about DMA controllers purely from the
  context of the hardware to begin with, we can describe a device in terms of
  a DMA controller as follows ...
        1. Number of DMA controllers
        2. Number of channels (maybe physical or logical)
        3. Mapping of DMA requests signals to DMA controller
        4. Number of DMA interrupts
        5. Mapping of DMA interrupts to channels
 - With the above in mind the aim of the DT DMA helper functions is to 
 extract
  the above information from the DT and provide to the appropriate driver.
  However, due to the vast number of DMA controllers and not all are using a
  common driver (such as DMA Engine) it has been seen that this is not a
  trivial task.

 Sorry for being slow, but so far I thought DT is only to provide _h/w_
 specific info
 to the _controller_ drivers. It was not supposed to provide any info
 pertaining to
 some API (dmaengine in this case).

 And I believe this is one of few situations where we are better off
 not generalizing
 the interface - pass controller specific info in the controller
 driver's specified format.
 Generalizing only seems to complicate things here, when we anyway have 
 machine
 specific dtb, which could always have clients requesting and the
 controllers given
 dma's info in controller driver's specific format.

 Perhaps I am overlooking the need to generalize. If you think so, please 
 help me
 understand by pointing out some use case for it.

 No not really, your points are valid. From reading the previous
 discussions one of the items that was clearly lacking was the ability to
 represent and identify a device having more than one DMA controller. In
 other words, when you request the DMA resource, how do you identify
 which DMA controller in the system that resource belongs too. With DMA
 engine there are ways we can do this.

Well, if we really can't have dmac drivers make 'intelligent' runtime decision
of request mapping on more than one capable controllers, we still can
make it simpler than the proposed scheme.

+   i2c1: i2c@1 {
+   ...
+   dma = sdma 2 1 sdma 3 2;
+   ...
+   };

I see this requires a client driver to specify a particular req_line on a
particular dma controller. I am not sure if this is most optimal.

I think such client-req_line map should be provided to the dmac controller
driver via its dt node in some format. The dmac driver could then populate
a dma_chan, or similar, only for that req_line and not for the unused one
which otherwise could also have served the same client.

Ideally the I2C driver should simply ask, say, a channel for TX and another
for RX, everything else should already be setup via dmac's dt nodes.

Channel properties like duplex, priority etc specified in client's dt node
doesn't seem really necessary - the client's driver is able to adjust
according to the capability of the channel the dmaengine api is able
to provide and the client driver can't really do anything if it saw txrx
specified in it's dt node while it doesn't yet support full-duplex (say).

Having spilled my guts and realizing the fact that the Maradonas and Peles
of the game seem to have already bought the scheme, I am afraid it only
points to my not yet diagnosed adhd  :o
--
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] of: Add generic device tree DMA helpers

2012-03-19 Thread Jassi Brar

On Mon, Mar 19, 2012 at 9:41 PM, Arnd Bergmann a...@arndb.de wrote:

 Secondly, there are platforms (Samsung) where peripherals are connected
 to more than one DMA controller, and either DMA controller can be used -
 I'm told by Jassi that there's reasons why you'd want to select one or
 other as the target at runtime.

 How common is that? If there are only a few drivers that have this
 requirement, we could have this represented in the driver binding, by
 listing multiple channels and giving the device the choice.

All Samsung SoCs (last ~4yrs) with PL330 have some peripherals
that could be served by more than one PL330 instance.
The requirement is not from any driver and it is nothing Samsung specific.
Any platform with 1:n client:dmac might want to do stuff like serving
a new channel request from an already active DMAC, rather than
invoking otherwise fully idle one.
Similarly for Mem-Mem, the PL330 distributes its cycles between
active channels, so a platform might want channel allocation uniformly
across PL330 instances.

-jassi
--
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: [alsa-devel] [PATCH v4 - resend 00/13] OMAP4: ASoC: Support for PandaBoard family

2012-01-27 Thread Jassi Brar

Hi Sebastien,

On 28 January 2012 04:21, Guiriec, Sebastien s-guir...@ti.com wrote:

 Hi Peter,
     I tried the patchset on my 4430 Panda. Playback works ok, but not 
 Capture.
 All is get is a hiss.
    Is capture supposed to work ? If yes, any amixer settings that need
 to be done?

 Hi Jassi,

 With the next update the PandaBoard 4430 is working fine.
 cpu_is_omap4430() macro is not define. We should use cpu_is_omap443x()
 macro in order to select correctly PandaBoard and not PandaBoardES
 board. Due to this error the heaset Mic pin is not enable so record is not
 working (use for Panda 4430 recording).

 diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-
 omap2/board-omap4panda.c
 index 5f27387..6a397e9 100644
 --- a/arch/arm/mach-omap2/board-omap4panda.c
 +++ b/arch/arm/mach-omap2/board-omap4panda.c
 @@ -580,7 +580,7 @@ void omap4_panda_display_init(void)

  static void omap4_panda_init_rev(void)
  {
 -       if (cpu_is_omap4430()) {
 +       if (cpu_is_omap443x()) {
                /* PandaBoard 4430 */
                /* ASoC audio configuration */
                panda_abe_audio_data.card_name = PandaBoard;

Thanks, the patch make it work for Panda-4430 too.

Thanks Peter for the amixer settings.

Cheers,
Jassi
--
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: [alsa-devel] [PATCH v4 - resend 00/13] OMAP4: ASoC: Support for PandaBoard family

2012-01-25 Thread Jassi Brar

On 24 January 2012 17:22, Peter Ujfalusi peter.ujfal...@ti.com wrote:
 Hello,

 the following series will add ASoC support for PandaBoards.
 PandaBoards have different audio routings compared to SDP4430/Blaze boards, 
 but
 the differences not that big to justify a new ASoC machine driver.

 The v3 series did not made it to 3.3 kernel.
 This set contains all the patches needed to enable the audio support on the
 PandaBoard family.
 The machine driver has been converted to a generic OMAP4 driver which can
 support wide range of machines using OMAP4 with twl6040 codec.

 I have remove the hardcoded MCLK clock frequency use in the machine driver
 (there can be devices with different MCLK configuration). This changed
 patch 04, 05, and 12. Since this does not affect the functionality I kept the
 acks for these patches. Patch 11 did not existed in the v3 series.

Hi Peter,
   I tried the patchset on my 4430 Panda. Playback works ok, but not Capture.
All is get is a hiss.
  Is capture supposed to work ? If yes, any amixer settings that need
to be done?

Thanks,
-Jassi
--
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/2] OMAPDSS: HDMI: Rid hw_params of extra argument

2011-11-22 Thread Jassi Brar

Signed-off-by: Jassi Brar jaswinder.si...@linaro.org
---
 drivers/video/omap2/dss/hdmi.c |   13 ++---
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 3262f0f..d3eae98 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -554,8 +554,7 @@ void omapdss_hdmi_display_disable(struct omap_dss_device 
*dssdev)
 #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
 
-static int hdmi_audio_hw_params(struct hdmi_ip_data *ip_data,
-   struct snd_pcm_substream *substream,
+static int hdmi_audio_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *params,
struct snd_soc_dai *dai)
 {
@@ -609,7 +608,7 @@ static int hdmi_audio_hw_params(struct hdmi_ip_data 
*ip_data,
return -EINVAL;
}
 
-   err = hdmi_config_audio_acr(ip_data, params_rate(params), n, cts);
+   err = hdmi_config_audio_acr(hdmi.ip_data, params_rate(params), n, 
cts);
if (err  0)
return err;
 
@@ -625,8 +624,8 @@ static int hdmi_audio_hw_params(struct hdmi_ip_data 
*ip_data,
audio_dma.mode = HDMI_AUDIO_TRANSF_DMA;
audio_dma.fifo_threshold = 0x20; /* in number of samples */
 
-   hdmi_wp_audio_config_dma(ip_data, audio_dma);
-   hdmi_wp_audio_config_format(ip_data, audio_format);
+   hdmi_wp_audio_config_dma(hdmi.ip_data, audio_dma);
+   hdmi_wp_audio_config_format(hdmi.ip_data, audio_format);
 
/*
 * I2S config
@@ -670,7 +669,7 @@ static int hdmi_audio_hw_params(struct hdmi_ip_data 
*ip_data,
/* Use parallel audio interface */
core_cfg.en_parallel_aud_input = true;
 
-   hdmi_core_audio_config(ip_data, core_cfg);
+   hdmi_core_audio_config(hdmi.ip_data, core_cfg);
 
/*
 * Configure packet
@@ -684,7 +683,7 @@ static int hdmi_audio_hw_params(struct hdmi_ip_data 
*ip_data,
aud_if_cfg.db5_downmix_inh = false;
aud_if_cfg.db5_lsv = 0;
 
-   hdmi_core_audio_infoframe_config(ip_data, aud_if_cfg);
+   hdmi_core_audio_infoframe_config(hdmi.ip_data, aud_if_cfg);
return 0;
 }
 
-- 
1.7.4.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 2/2] OMAPDSS: HDMI: Indirect trigger to IP specific version

2011-11-22 Thread Jassi Brar

Signed-off-by: Jassi Brar jaswinder.si...@linaro.org
---
 drivers/video/omap2/dss/hdmi.c|6 ++
 drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c |2 +-
 drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h |2 +-
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index d3eae98..9822951 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -687,6 +687,12 @@ static int hdmi_audio_hw_params(struct snd_pcm_substream 
*substream,
return 0;
 }
 
+static int hdmi_audio_trigger(struct snd_pcm_substream *substream,
+   int cmd, struct snd_soc_dai *dai)
+{
+   return hdmi_4xxx_audio_trigger(hdmi.ip_data, substream, cmd, dai);
+}
+
 static int hdmi_audio_startup(struct snd_pcm_substream *substream,
  struct snd_soc_dai *dai)
 {
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c 
b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
index e1a6ce5..4b71093 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.c
@@ -1204,7 +1204,7 @@ int hdmi_config_audio_acr(struct hdmi_ip_data *ip_data,
return 0;
 }
 
-int hdmi_audio_trigger(struct hdmi_ip_data *ip_data,
+int hdmi_4xxx_audio_trigger(struct hdmi_ip_data *ip_data,
struct snd_pcm_substream *substream, int cmd,
struct snd_soc_dai *dai)
 {
diff --git a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h 
b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h
index 2040956..a975f17 100644
--- a/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h
+++ b/drivers/video/omap2/dss/ti_hdmi_4xxx_ip.h
@@ -576,7 +576,7 @@ struct hdmi_core_audio_config {
 
 #if defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI) || \
defined(CONFIG_SND_OMAP_SOC_OMAP4_HDMI_MODULE)
-int hdmi_audio_trigger(struct hdmi_ip_data *ip_data,
+int hdmi_4xxx_audio_trigger(struct hdmi_ip_data *ip_data,
struct snd_pcm_substream *substream, int cmd,
struct snd_soc_dai *dai);
 int hdmi_config_audio_acr(struct hdmi_ip_data *ip_data,
-- 
1.7.4.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] DMAEngine: Define generic transfer request api

2011-09-15 Thread Jassi Brar

On 15 September 2011 12:01, Barry Song 21cn...@gmail.com wrote:
 2011/9/13 Barry Song 21cn...@gmail.com:
 2011/9/13 Jassi Brar jaswinder.si...@linaro.org:
 On 13 September 2011 13:16, Barry Song 21cn...@gmail.com wrote:
 if test pass, to the patch, and even for the moment, to the API's idea
 Acked-by: Barry Song baohua.s...@csr.com

 one issue i noticed is with a device_prep_dma_genxfer, i don't need
 device_prep_slave_sg any more,
 Yeah, the damengine would need to adapt to the fact that these
 interleaved tranfers could be Mem-Mem as well as Mem-Dev
 (even though yours could be only one type, but some dmacs could
 do both).

 How about:

       BUG_ON(dma_has_cap(DMA_MEMCPY, device-cap_mask) 
 -               !device-device_prep_dma_memcpy);
 +               !device-device_prep_dma_memcpy 
 +               !device-device_prep_dma_genxfer);

        BUG_ON(dma_has_cap(DMA_SLAVE, device-cap_mask) 
  -               !device-device_prep_slave_sg);
 +               !device-device_prep_slave_sg 
 +               !device-device_prep_dma_genxfer);

 Seems ok, but please modify in a way you think is best and submit a patch
 on top of this new api. Then it'll be easier to evaluate everything.

 i think it should be handled by this patch but not a new one.

 and i also think xfer_template is a bad name for a structure which is
 an API. i'd like to add namespace for it and rename it to dma_genxfer.
 or have any good suggestion?
I think xfer_template is better - which stresses the usage as having prepared
templates of transfers and only change src/dst address before submitting.
'device_prep_dma_genxfer' is the API which is already named so.

 i'd like to send this together with BUG_ON(dma_has_cap(DMA_SLAVE,
 device-cap_mask) !device-device_prep_dma_genxfer) as v2.
Is there no change other than skipping check for SLAVE when using this api ?
--
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] DMAEngine: Define generic transfer request api

2011-09-13 Thread Jassi Brar

On 13 September 2011 13:16, Barry Song 21cn...@gmail.com wrote:
 if test pass, to the patch, and even for the moment, to the API's idea
 Acked-by: Barry Song baohua.s...@csr.com

 one issue i noticed is with a device_prep_dma_genxfer, i don't need
 device_prep_slave_sg any more,
Yeah, the damengine would need to adapt to the fact that these
interleaved tranfers could be Mem-Mem as well as Mem-Dev
(even though yours could be only one type, but some dmacs could
do both).

 How about:

       BUG_ON(dma_has_cap(DMA_MEMCPY, device-cap_mask) 
 -               !device-device_prep_dma_memcpy);
 +               !device-device_prep_dma_memcpy 
 +               !device-device_prep_dma_genxfer);

        BUG_ON(dma_has_cap(DMA_SLAVE, device-cap_mask) 
  -               !device-device_prep_slave_sg);
 +               !device-device_prep_slave_sg 
 +               !device-device_prep_dma_genxfer);

Seems ok, but please modify in a way you think is best and submit a patch
on top of this new api. Then it'll be easier to evaluate everything.

thanks.
--
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] DMAEngine: Define generic transfer request api

2011-09-12 Thread Jassi Brar

On 12 September 2011 21:56, Barry Song 21cn...@gmail.com wrote:
 Define a new api that could be used for doing fancy data transfers
 like interleaved to contiguous copy and vice-versa.
 Traditional SG_list based transfers tend to be very inefficient in
 such cases as where the interleave and chunk are only a few bytes,
 which call for a very condensed api to convey pattern of the transfer.

 This api supports all 4 variants of scatter-gather and contiguous transfer.
 Besides, it could also represent common operations like
        device_prep_dma_{cyclic, memset, memcpy}
 and maybe some more that I am not sure of.

 Of course, neither can this api help transfers that don't lend to DMA by
 nature, i.e, scattered tiny read/writes with no periodic pattern.

 Signed-off-by: Jassi Brar jaswinder.si...@linaro.org

 anyway, this API needs a real user to prove why it needs to exist.

 prima2 can be the 1st(?, 2nd if TI uses) user of this API. let's try
 to see what the driver will be with this api. Then we might figure out
 more about what it should be.

Did you discover any issue with the api?
Because only three days ago you said
{
Jassi, you might think my reply as an ACK to [PATCH] DMAEngine:
Define generic transfer request api.
}

The api met your requirements easily not because I know them already,
but because I designed the api to be as generic as practically possible.
--
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] ASoC: omap: convert per-board modules to platform drivers

2011-09-09 Thread Jassi Brar

On 9 September 2011 05:29, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:
 Jassi's suggestion was that we should have some magic to
 automatically generate defaults for the relevant device registrations to
 sidestep these issues.
Perhaps there is some misunderstanding no witchcraft is involved here.
To be clear, I suggested moving platform_device definition and registration
from 12 board files to some common platform file and use machine_is_xxx()
to assign names of those platform devices. Btw, omap_init_audio() is already
called from an arch_initcall.
Also please note that currently there's no platform_data needed to be passed.
If that requirement arise in future, an optional set_asoc_platdata(void *pdata)
could be defined beside platform_device creation.
While the idea is not absolutely good, imho, it's certainly an improvement
over this patch. Or am I overlooking something ?
--
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] DMAEngine: Define generic transfer request api

2011-08-19 Thread Jassi Brar

On 19 August 2011 22:58, Koul, Vinod vinod.k...@intel.com wrote:
 On Fri, 2011-08-19 at 21:16 +0530, Jassi Brar wrote:
 On 19 August 2011 19:49, Linus Walleij linus.ml.wall...@gmail.com wrote:
  2011/8/19 Koul, Vinod vinod.k...@intel.com:
  On Tue, 2011-08-16 at 15:06 +0200, Linus Walleij wrote:
  On Tue, Aug 16, 2011 at 2:56 PM, Koul, Vinod vinod.k...@intel.com 
  wrote:
 
  I think Sundaram is in the position of doing some heavy work on
  using one or the other of the API:s, and I think he is better
  suited than anyone else of us to select what scheme to use,
  in the end he's going to write the first code using the API.
 
  And Unfortunately TI folks don't seem to care about this discussion :(
  Haven't seen anything on this from them, or on previous RFC by Jassi
 
  Well if there is no code usig the API then there is no rush
  in merging it either I guess. Whenever someone (TI or
  Samsung) cook some driver patches they can choose their
  approach.
 
 No, it's not a matter of choice.
 If that were the case, Sundaram already proposed a TI specific
 flag. Why wait for him to tell his choice again?

 You might, but I can't molest my sensibility to believe that a Vendor
 specific flag could be better than a generic solution.
 Not here at least, where the overhead due to generality is not much.
 (though I can trim some 'futuristic' members from the 'struct xfer_template')
 Who said anything about adding a vendor flag solution,
Not you, but to whom I replied - LinusW   See https://lkml.org/lkml/2011/7/11/74

 since TI are potential users of the API it would good to know i this fits 
 there needs
 are not.
I am super-interested to hear from TI guys.
The generic api here rather supports the case Sundaram projected as
'most' general case. Look at the figure at end of
https://lkml.org/lkml/2011/6/9/343

 Maintainers might wait as long as they want, but there should never
 be an option to have vendor specific hacks.
 to me API looks decent after reading some specs of DMACs which support
 this mode. Pls send updated patch along with one driver which uses it.
 Should be good to go...
That has been one problem with DMAEngine. People patch the API as they need
stuff, rather than having had solid thought-out API that could be
extended consistently.
For ex, we ended up having _ten_ device_prep_dma_* callbacks, where as
we could have done having had originally 1 (or maybe 2)  'generic'
prepare callback with a 'enum dma_transaction_type op' argument.
IMO it's rather good that I designed this API for a theoretical
highly-capable DMAC,
and not just the DMAC I've worked on - which would have constrained
the api in future
for other DMACs.
--
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] DMAEngine: Define generic transfer request api

2011-08-12 Thread Jassi Brar

Define a new api that could be used for doing fancy data transfers
like interleaved to contiguous copy and vice-versa.
Traditional SG_list based transfers tend to be very inefficient in
such cases as where the interleave and chunk are only a few bytes,
which call for a very condensed api to convey pattern of the transfer.

This api supports all 4 variants of scatter-gather and contiguous transfer.
Besides, it could also represent common operations like
device_prep_dma_{cyclic, memset, memcpy}
and maybe some more that I am not sure of.

Of course, neither can this api help transfers that don't lend to DMA by
nature, i.e, scattered tiny read/writes with no periodic pattern.

Signed-off-by: Jassi Brar jaswinder.si...@linaro.org
---
 include/linux/dmaengine.h |   73 +
 1 files changed, 73 insertions(+), 0 deletions(-)

diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 8fbf40e..74f3ae0 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -76,6 +76,76 @@ enum dma_transaction_type {
 /* last transaction type for creation of the capabilities mask */
 #define DMA_TX_TYPE_END (DMA_CYCLIC + 1)
 
+/**
+ * Generic Transfer Request
+ * 
+ * A chunk is collection of contiguous bytes to be transfered.
+ * The gap(in bytes) between two chunks is called inter-chunk-gap(ICG).
+ * ICGs may or maynot change between chunks.
+ * A FRAME is the smallest series of contiguous {chunk,icg} pairs,
+ *  that when repeated an integral number of times, specifies the transfer.
+ * A transfer template is specification of a Frame, the number of times
+ *  it is to be repeated and other per-transfer attributes.
+ *
+ * Practically, a client driver would have ready a template for each
+ *  type of transfer it is going to need during its lifetime and
+ *  set only 'src_start' and 'dst_start' before submitting the requests.
+ *
+ *
+ *  |  Frame-1|   Frame-2   | ~ |   Frame-'numf'  |
+ *  |==.===...=...|==.===...=...| ~ |==.===...=...|
+ *
+ *==  Chunk size
+ *... ICG
+ */
+
+/**
+ * struct data_chunk - Element of scatter-gather list that makes a frame.
+ * @size: Number of bytes to read from source.
+ *   size_dst := fn(op, size_src), so doesn't mean much for destination.
+ * @icg: Number of bytes to jump after last src/dst address of this
+ *  chunk and before first src/dst address for next chunk.
+ *  Ignored for dst(assumed 0), if dst_inc is true and dst_sgl is false.
+ *  Ignored for src(assumed 0), if src_inc is true and src_sgl is false.
+ */
+struct data_chunk {
+   size_t size;
+   size_t icg;
+};
+
+/**
+ * struct xfer_template - Template to convey DMAC the transfer pattern
+ *  and attributes.
+ * @op: The operation to perform on source data before writing it on
+ *  to destination address.
+ * @src_start: Bus address of source for the first chunk.
+ * @dst_start: Bus address of destination for the first chunk.
+ * @src_inc: If the source address increments after reading from it.
+ * @dst_inc: If the destination address increments after writing to it.
+ * @src_sgl: If the 'icg' of sgl[] applies to Source (scattered read).
+ * Otherwise, source is read contiguously (icg ignored).
+ * Ignored if src_inc is false.
+ * @dst_sgl: If the 'icg' of sgl[] applies to Destination (scattered write).
+ * Otherwise, destination is filled contiguously (icg ignored).
+ * Ignored if dst_inc is false.
+ * @frm_irq: If the client expects DMAC driver to do callback after each frame.
+ * @numf: Number of frames in this template.
+ * @frame_size: Number of chunks in a frame i.e, size of sgl[].
+ * @sgl: Array of {chunk,icg} pairs that make up a frame.
+ */
+struct xfer_template {
+   enum dma_transaction_type op;
+   dma_addr_t src_start;
+   dma_addr_t dst_start;
+   bool src_inc;
+   bool dst_inc;
+   bool src_sgl;
+   bool dst_sgl;
+   bool frm_irq;
+   size_t numf;
+   size_t frame_size;
+   struct data_chunk sgl[0];
+};
 
 /**
  * enum dma_ctrl_flags - DMA flags to augment operation preparation,
@@ -432,6 +502,7 @@ struct dma_tx_state {
  * @device_prep_dma_cyclic: prepare a cyclic dma operation suitable for audio.
  * The function takes a buffer of size buf_len. The callback function will
  * be called after period_len bytes have been transferred.
+ * @device_prep_dma_genxfer: Transfer expression in a generic way.
  * @device_control: manipulate all pending operations on a channel, returns
  * zero or error code
  * @device_tx_status: poll for transaction completion, the optional
@@ -496,6 +567,8 @@ struct dma_device {
struct dma_async_tx_descriptor *(*device_prep_dma_cyclic)(
struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
size_t period_len, enum dma_data_direction direction

Re: [PATCH v4] usb: musb: Enable DMA mode1 RX for USB-Mass-Storage

2011-07-26 Thread Jassi Brar

On Tue, Jul 26, 2011 at 8:36 PM, Felipe Balbi ba...@ti.com wrote:
 Hi,

 On Wed, Jul 20, 2011 at 09:48:53AM -0700, Pandita, Vikram wrote:
  This *today happens to be only UMS* is my exact point here.
  Can you guarantee no other function driver will ever expect only
  full packet xfers and treat short as errors ?
 

 We are trying to test if short_not_ok may not be needed at all. But
 all gadgets need to be tested on MUSB for that.
 We will need wider help from MUSB maintainer/author(anand g) to
 determine if removing short_not_ok is fine on MUSB for _all_ gadgets.

 To be safe we only enable for UMS use case today that is definitely
 working fine.

 Time for Maintainer/author to pitch in !!

 I'm thinking on allowing this patch to go in if nobody has really strong
 arguments against it. The real fix, though, would need a bigger re-write
 of the endpoint IRQ handling and that would need more time to write and
 stabilize. Together with the huge amount of HW issues MUSB has, it's
 quite a task (been there, done that).

Please note that I never objected to the _code_.

I just think if the _comments_ could be made UMS agnostic... for ex if it works
for other protocols just fine(quite possible) in future the reader
might wonder what
is UMS specific about the code snippet.

Please feel free to go ahead and apply the patch.

thnx
--
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] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration

2011-07-23 Thread Jassi Brar

On Mon, Jul 18, 2011 at 1:21 PM, Raju, Sundaram sunda...@ti.com wrote:

 Maybe a new api to pass fixed-format variable-length encoded message
 to the DMAC drivers?
 Which could be interpreted by DMAC drivers to extract all the needed xfer
 parameters from the 'header' section and instructions to program the xfers
 in the DMAC from the variable length body of the 'message' buffer.
 It might sound complicated but we can have helpers to make the job easy.
 Btw, the regular single/sg-list xfers could also be expressed by this method.

 Do you expect this variable length body of the message to be DMAC
 independent? I don't think so. In that case how is this different from what
 we have here already?
Yes, this whould be DMAC independent.

 If it can be DMAC independent, can you illustrate more on how this can
 be done? But the point to note is, if this can be made DMAC independent
 then the control command we have also can be made DMAC independent
 by generalizing the configuration structure passed to it.
The 'header' I suggest, would in fact be a structure body, only an extra pointer
would point to the 'instructions' to convey actual location and sizes
of mico-xfers.
I don't think it is possible to have general definition of such transfers fully
within a structure. If I understand what you ask.

I have just posted an RFC. I kept the terms same so that it is easier
to understand.
Please have a look. You are CC'ed too.

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

Re: [PATCH v4] usb: musb: Enable DMA mode1 RX for USB-Mass-Storage

2011-07-19 Thread Jassi Brar

On Wed, Jul 20, 2011 at 10:41 AM, Vikram Pandita vikram.pand...@ti.com wrote:
 From: Anand Gadiyar gadi...@ti.com

 This patch enables the DMA mode1 RX support.
 This feature is enabled based on the short_not_ok flag passed from
 gadget drivers.

 This will result in a thruput performance gain of around
 40% for USB mass-storage/mtp use cases.

 Signed-off-by: Anand Gadiyar gadi...@ti.com
 Signed-off-by: Moiz Sonasath m-sonas...@ti.com
 Signed-off-by: Vikram Pandita vikram.pand...@ti.com
 Tested-by: Vikram Pandita vikram.pand...@ti.com
 ---
 v1 - initial push
 v2 - fixed whitespace issues as per Sergei Shtylyovsshtyl...@mvista.com 
 comments
 v3 - restor authorship to Anand Gadiyar gadi...@ti.com
 v4 - adding my signed-off as per Kevin Hilman khil...@ti.com

  drivers/usb/musb/musb_gadget.c |   68 ---
  1 files changed, 42 insertions(+), 26 deletions(-)

 @@ -683,6 +684,18 @@ static void rxstate(struct musb *musb, struct 
 musb_request *req)

        if (csr  MUSB_RXCSR_RXPKTRDY) {
                len = musb_readw(epio, MUSB_RXCOUNT);
 +
 +               /*
 +                * Enable Mode 1 for RX transfers only for mass-storage
 +                * use-case, based on short_not_ok flag which is set only
 +                * from file_storage and f_mass_storage drivers
 +                */
 +
 +               if (request-short_not_ok  len == musb_ep-packet_sz)
 +                       use_mode_1 = 1;
 +               else
 +                       use_mode_1 = 0;
 +
There is nothing UMS class specific in this patch...
(request-short_not_ok  len == musb_ep-packet_sz) may not be the
signature of, and only of, Mass Storage Functions. So maybe removing
the UMS mention from
comment and change-log is a good idea.
You might want to add is-ep-type-bulk-out check to the condition
though, so that it doesn't affect
cases that you didn't verify.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: musb: Enable DMA mode1 RX for USB-Mass-Storage

2011-07-19 Thread Jassi Brar

On Wed, Jul 20, 2011 at 11:15 AM, Pandita, Vikram vikram.pand...@ti.com wrote:
 On Tue, Jul 19, 2011 at 10:34 PM, Jassi Brar jassisinghb...@gmail.com wrote:

 On Wed, Jul 20, 2011 at 10:41 AM, Vikram Pandita vikram.pand...@ti.com 
 wrote:
  From: Anand Gadiyar gadi...@ti.com
 
  This patch enables the DMA mode1 RX support.
  This feature is enabled based on the short_not_ok flag passed from
  gadget drivers.
 
  This will result in a thruput performance gain of around
  40% for USB mass-storage/mtp use cases.
 
  Signed-off-by: Anand Gadiyar gadi...@ti.com
  Signed-off-by: Moiz Sonasath m-sonas...@ti.com
  Signed-off-by: Vikram Pandita vikram.pand...@ti.com
  Tested-by: Vikram Pandita vikram.pand...@ti.com
  ---
  v1 - initial push
  v2 - fixed whitespace issues as per Sergei Shtylyovsshtyl...@mvista.com 
  comments
  v3 - restor authorship to Anand Gadiyar gadi...@ti.com
  v4 - adding my signed-off as per Kevin Hilman khil...@ti.com
 
   drivers/usb/musb/musb_gadget.c |   68 
  ---
   1 files changed, 42 insertions(+), 26 deletions(-)

  @@ -683,6 +684,18 @@ static void rxstate(struct musb *musb, struct 
  musb_request *req)
 
         if (csr  MUSB_RXCSR_RXPKTRDY) {
                 len = musb_readw(epio, MUSB_RXCOUNT);
  +
  +               /*
  +                * Enable Mode 1 for RX transfers only for mass-storage
  +                * use-case, based on short_not_ok flag which is set only
  +                * from file_storage and f_mass_storage drivers
  +                */
  +
  +               if (request-short_not_ok  len == musb_ep-packet_sz)
  +                       use_mode_1 = 1;
  +               else
  +                       use_mode_1 = 0;
  +
 There is nothing UMS class specific in this patch...
 (request-short_not_ok  len == musb_ep-packet_sz) may not be the
 signature of, and only of, Mass Storage Functions. So maybe removing
 the UMS mention from
 comment and change-log is a good idea.

 Have you grepped the code in drivers/usb/gadget/*.*
 only UMS sets this flag today and hence the use of this flag.

 As i understand, on UMS, CSW/data/CBW  - the data part size is a known
 size and to be safe that mode=1 dma is not stuck up,
 the mode is enabled only for the gadget driver that sets short_not_ok
 flag - and that today happens to be only UMS.

This *today happens to be only UMS* is my exact point here.
Can you guarantee no other function driver will ever expect only
full packet xfers and treat short as errors ?


 You might want to add is-ep-type-bulk-out check to the condition
 though, so that it doesn't affect
 cases that you didn't verify.

 TX path (IN host), already uses the mode=1 DMA and hence the comment
 is not valid.
 This patch just also enables mode=1 on RX path.
Well, then no need for the ep-type check.
--
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] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration

2011-07-12 Thread Jassi Brar

On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij linus.wall...@linaro.org wrote:
 On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar jassisinghb...@gmail.com wrote:

 1) Striding, in one form or other, is supported by other DMACs as well.
   The number will only increase in future.
   Are we to add  VENDOR_DMA_STRIDE_CONFIG for each case ?

 If we are sure about this and striding will work in a similar way on all
 then let's have the enum named DMA_STRIDE_CONFIG and move the
 passed-in struct to linux/dmaengine.h) then?

 Would that be:

 struct dma_stride_config {
    u32 read_bytes;
    u32 skip_bytes;
 };

 Or something more complex?
Well, I am not sure if striding needs any special treatment at all.
Why not have client drivers prepare and submit sg-list.
Let the DMAC drivers interpret/parse the sg-list and program it
as strides if the h/w supports it.
If anything, we should make preparation and submission of sg-list
as efficient as possible.

 2) As Dan noted, client drivers are going to have ifdef hackery in
 order to be common
  to other SoCs.

 Don't think so, why? This is a runtime config entirely, and I just illustrated
 in mail to Dan how that can be handled by falling back to a sglist I believe?
Runtime decision isn't neat either.
What if a client driver is common to 'N' SoCs each with different DMACs ?
We would need a switch construct !
--
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] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration

2011-07-12 Thread Jassi Brar

On Tue, Jul 12, 2011 at 5:01 PM, Raju, Sundaram sunda...@ti.com wrote:
 -Original Message-
 From: Jassi Brar [mailto:jassisinghb...@gmail.com]
 Sent: Tuesday, July 12, 2011 4:51 PM
 To: Linus Walleij
 Cc: Raju, Sundaram; linux-arm-ker...@lists.infradead.org; linux-
 ker...@vger.kernel.org; davinci-linux-open-sou...@linux.davincidsp.com;
 li...@arm.linux.org.uk; dan.j.willi...@intel.com; linux-omap@vger.kernel.org
 Subject: Re: [PATCH] dmaengine: add dma_ctrl_cmd to pass buffer stride
 configuration

 On Tue, Jul 12, 2011 at 3:33 PM, Linus Walleij linus.wall...@linaro.org
 wrote:
  On Tue, Jul 12, 2011 at 6:17 AM, Jassi Brar jassisinghb...@gmail.com
 wrote:

  1) Striding, in one form or other, is supported by other DMACs as well.
    The number will only increase in future.
    Are we to add  VENDOR_DMA_STRIDE_CONFIG for each case ?

  If we are sure about this and striding will work in a similar way on all
  then let's have the enum named DMA_STRIDE_CONFIG and move the
  passed-in struct to linux/dmaengine.h) then?

  Would that be:

  struct dma_stride_config {
     u32 read_bytes;
     u32 skip_bytes;
  };

  Or something more complex?
 Well, I am not sure if striding needs any special treatment at all.
 Why not have client drivers prepare and submit sg-list.
 Let the DMAC drivers interpret/parse the sg-list and program it
 as strides if the h/w supports it.
 If anything, we should make preparation and submission of sg-list
 as efficient as possible.
 Jassi,

 sg_lists describe only a bunch of disjoint buffers. But what if the
 DMAC can skip and read the bytes within each of the buffers in
 the sg_list? (like TI EDMAC and TI SDMAC)
 How can that information be passed to the offload
 engine driver from the client?

OK, I overlooked.
We do need something new to handle these ultra-fine-grained sg-lists.
But still we shouldn't add SoC specific API to the common sub-systems.

Maybe a new api to pass fixed-format variable-length encoded message
to the DMAC drivers?
Which could be interpreted by DMAC drivers to extract all the needed xfer
parameters from the 'header' section and instructions to program the xfers
in the DMAC from the variable length body of the 'message' buffer.
It might sound complicated but we can have helpers to make the job easy.
Btw, the regular single/sg-list xfers could also be expressed by this method.
--
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] dmaengine: add dma_ctrl_cmd to pass buffer stride configuration

2011-07-11 Thread Jassi Brar

On Sun, Jul 10, 2011 at 8:33 PM, Sundaram Raju sunda...@ti.com wrote:
 Added new dma_ctrl_cmd TI_DMA_STRIDE_CONFIG to pass the TI DMA
 controller specific configurations on how a buffer must be walked
 through and how data is picked for transfer based on a specified
 pattern over the channel.

 The configuration passed is specific to the TI DMA controller used.

 Signed-off-by: Sundaram Raju sunda...@ti.com
 ---
  include/linux/dmaengine.h |    5 +
  1 files changed, 5 insertions(+), 0 deletions(-)

 diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
 index eee7add..51dadc4 100644
 --- a/include/linux/dmaengine.h
 +++ b/include/linux/dmaengine.h
 @@ -123,6 +123,10 @@ enum dma_ctrl_flags {
  * command.
  * @FSLDMA_EXTERNAL_START: this command will put the Freescale DMA controller
  * into external start mode.
 + * @TI_DMA_STRIDE_CONFIG: this command is only implemented by TI DMA
 + * controllers that need to pass special configuration on how to walk through
 + * the buffer to pick up data in a specified pattern to be transferred in
 + * the channel.
  */
  enum dma_ctrl_cmd {
        DMA_TERMINATE_ALL,
 @@ -130,6 +134,7 @@ enum dma_ctrl_cmd {
        DMA_RESUME,
        DMA_SLAVE_CONFIG,
        FSLDMA_EXTERNAL_START,
 +       TI_DMA_STRIDE_CONFIG,
  };
IMHO this isn't very correct.

1) Striding, in one form or other, is supported by other DMACs as well.
   The number will only increase in future.
   Are we to add  VENDOR_DMA_STRIDE_CONFIG for each case ?

2) As Dan noted, client drivers are going to have ifdef hackery in
order to be common
 to other SoCs.

3) TI may not have just one DMAC IP used in all the SoCs. So if you want
  vendor specific defines anyway, please atleast also add DMAC version to it.
  Something like
DMA_SLAVE_CONFIG,
FSLDMA_EXTERNAL_START,
 +   TI_DMA_v1_STRIDE_CONFIG,
--
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

[PATCHv2] OMAP4: PANDA, SDP: Fix EHCI regulator supply

2011-06-27 Thread Jassi Brar

VUSB is a fixed level line and hence have no set_voltage
callback in regulator ops, but has apply_uV set to true.
As a result it fails to register with the regulator core.
Remove setting apply_uV.

Also, assign name to VUSB supply, without which regulator core
fails to find it and assigns the default 'dummy' regulator to
the ehci-omap device.

Signed-off-by: Jassi Brar jaswinder.si...@linaro.org
---
 arch/arm/mach-omap2/board-4430sdp.c|7 ++-
 arch/arm/mach-omap2/board-omap4panda.c |7 ++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/board-4430sdp.c 
b/arch/arm/mach-omap2/board-4430sdp.c
index 63de2d3..9493cd3 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -504,16 +504,21 @@ static struct regulator_init_data sdp4430_vdac = {
},
 };
 
+static struct regulator_consumer_supply sdp4430_vusb_supply[] = {
+   REGULATOR_SUPPLY(hsusb0, ehci-omap.0),
+};
+
 static struct regulator_init_data sdp4430_vusb = {
.constraints = {
.min_uV = 330,
.max_uV = 330,
-   .apply_uV   = true,
.valid_modes_mask   = REGULATOR_MODE_NORMAL
| REGULATOR_MODE_STANDBY,
.valid_ops_mask  =  REGULATOR_CHANGE_MODE
| REGULATOR_CHANGE_STATUS,
},
+   .num_consumer_supplies  = ARRAY_SIZE(sdp4430_vusb_supply),
+   .consumer_supplies  = sdp4430_vusb_supply,
 };
 
 static struct regulator_init_data sdp4430_clk32kg = {
diff --git a/arch/arm/mach-omap2/board-omap4panda.c 
b/arch/arm/mach-omap2/board-omap4panda.c
index d4f9879..2beb0d5 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -362,16 +362,21 @@ static struct regulator_init_data omap4_panda_vdac = {
},
 };
 
+static struct regulator_consumer_supply omap4_panda_vusb_supply[] = {
+   REGULATOR_SUPPLY(hsusb0, ehci-omap.0),
+};
+
 static struct regulator_init_data omap4_panda_vusb = {
.constraints = {
.min_uV = 330,
.max_uV = 330,
-   .apply_uV   = true,
.valid_modes_mask   = REGULATOR_MODE_NORMAL
| REGULATOR_MODE_STANDBY,
.valid_ops_mask  =  REGULATOR_CHANGE_MODE
| REGULATOR_CHANGE_STATUS,
},
+   .num_consumer_supplies  = ARRAY_SIZE(omap4_panda_vusb_supply),
+   .consumer_supplies  = omap4_panda_vusb_supply,
 };
 
 static struct regulator_init_data omap4_panda_clk32kg = {
-- 
1.7.4.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] OMAP4: PANDA, SDP: Fix EHCI regulator supply

2011-06-24 Thread Jassi Brar

VUSB is a fixed level line and hence have no set_voltage
callback in regulator ops, but has apply_uV set to true.
As a result it fails to register with the regulator core.
Remove setting apply_uV.

Also, assign name to VUSB supply, without which regulator core
fails to find it and assigns the default 'dummy' regulator to
the ehci-omap device.

Signed-off-by: Jassi Brar jaswinder.si...@linaro.org
---
 arch/arm/mach-omap2/board-4430sdp.c|6 +-
 arch/arm/mach-omap2/board-omap4panda.c |6 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/board-4430sdp.c 
b/arch/arm/mach-omap2/board-4430sdp.c
index 63de2d3..1ec60be 100644
--- a/arch/arm/mach-omap2/board-4430sdp.c
+++ b/arch/arm/mach-omap2/board-4430sdp.c
@@ -504,16 +504,20 @@ static struct regulator_init_data sdp4430_vdac = {
},
 };
 
+static struct regulator_consumer_supply sdp4430_vusb_supply =
+   REGULATOR_SUPPLY(hsusb0, ehci-omap.0);
+
 static struct regulator_init_data sdp4430_vusb = {
.constraints = {
.min_uV = 330,
.max_uV = 330,
-   .apply_uV   = true,
.valid_modes_mask   = REGULATOR_MODE_NORMAL
| REGULATOR_MODE_STANDBY,
.valid_ops_mask  =  REGULATOR_CHANGE_MODE
| REGULATOR_CHANGE_STATUS,
},
+   .num_consumer_supplies  = 1,
+   .consumer_supplies  = sdp4430_vusb_supply,
 };
 
 static struct regulator_init_data sdp4430_clk32kg = {
diff --git a/arch/arm/mach-omap2/board-omap4panda.c 
b/arch/arm/mach-omap2/board-omap4panda.c
index d4f9879..7429f7e 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -362,16 +362,20 @@ static struct regulator_init_data omap4_panda_vdac = {
},
 };
 
+static struct regulator_consumer_supply omap4_panda_vusb_supply =
+   REGULATOR_SUPPLY(hsusb0, ehci-omap.0);
+
 static struct regulator_init_data omap4_panda_vusb = {
.constraints = {
.min_uV = 330,
.max_uV = 330,
-   .apply_uV   = true,
.valid_modes_mask   = REGULATOR_MODE_NORMAL
| REGULATOR_MODE_STANDBY,
.valid_ops_mask  =  REGULATOR_CHANGE_MODE
| REGULATOR_CHANGE_STATUS,
},
+   .num_consumer_supplies  = 1,
+   .consumer_supplies  = omap4_panda_vusb_supply,
 };
 
 static struct regulator_init_data omap4_panda_clk32kg = {
-- 
1.7.4.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] regulator: twl: Add 'fixed' set_voltage callback

2011-06-23 Thread Jassi Brar

Define dummy set_voltage callback for fixed lines,
without which voltage constraints fail to apply.

Signed-off-by: Jassi Brar jaswinder.si...@linaro.org
---
 drivers/regulator/twl-regulator.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/twl-regulator.c 
b/drivers/regulator/twl-regulator.c
index 87fe0f7..8223e49 100644
--- a/drivers/regulator/twl-regulator.c
+++ b/drivers/regulator/twl-regulator.c
@@ -591,10 +591,18 @@ static int twlfixed_get_voltage(struct regulator_dev 
*rdev)
return info-min_mV * 1000;
 }
 
+static int twlfixed_set_voltage(struct regulator_dev *rdev,
+   int min_uV, int max_uV, unsigned *selector)
+{
+   *selector = min_uV / 1000;
+   return 0;
+}
+
 static struct regulator_ops twl4030fixed_ops = {
.list_voltage   = twlfixed_list_voltage,
 
.get_voltage= twlfixed_get_voltage,
+   .set_voltage= twlfixed_set_voltage,
 
.enable = twl4030reg_enable,
.disable= twl4030reg_disable,
@@ -609,6 +617,7 @@ static struct regulator_ops twl6030fixed_ops = {
.list_voltage   = twlfixed_list_voltage,
 
.get_voltage= twlfixed_get_voltage,
+   .set_voltage= twlfixed_set_voltage,
 
.enable = twl6030reg_enable,
.disable= twl6030reg_disable,
-- 
1.7.4.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] mfd: omap: Restore TLL initialization

2011-06-23 Thread Jassi Brar

The commit
  7e6502d577106fb5b202bbaac64c5f1b065 'mfd: Add omap-usbhs runtime PM support'
besides moving to RPM, removes necessary TLL initialization as well.

Restore the TLL initialization, without which device detection fails.

Signed-off-by: Jassi Brar jaswinder.si...@linaro.org
---
 drivers/mfd/omap-usb-host.c |   42 ++
 1 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 8552195..12ca77b 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -640,6 +640,7 @@ static int usbhs_enable(struct device *dev)
struct usbhs_omap_platform_data *pdata = omap-platdata;
unsigned long   flags = 0;
int ret = 0;
+   unsigned long   timeout;
unsignedreg;
 
dev_dbg(dev, starting TI HSUSB Controller\n);
@@ -676,6 +677,47 @@ static int usbhs_enable(struct device *dev)
omap-usbhs_rev = usbhs_read(omap-uhh_base, OMAP_UHH_REVISION);
dev_dbg(dev, OMAP UHH_REVISION 0x%x\n, omap-usbhs_rev);
 
+   /* perform TLL soft reset, and wait until reset is complete */
+   usbhs_write(omap-tll_base, OMAP_USBTLL_SYSCONFIG,
+   OMAP_USBTLL_SYSCONFIG_SOFTRESET);
+
+   /* Wait for TLL reset to complete */
+   timeout = jiffies + msecs_to_jiffies(1000);
+   while (!(usbhs_read(omap-tll_base, OMAP_USBTLL_SYSSTATUS)
+OMAP_USBTLL_SYSSTATUS_RESETDONE)) {
+   cpu_relax();
+
+   if (time_after(jiffies, timeout)) {
+   dev_dbg(dev, operation timed out\n);
+   ret = -EINVAL;
+   goto err_tll;
+   }
+   }
+   dev_dbg(dev, TLL RESET DONE\n);
+
+   /* (13) = no idle mode only for initial debugging */
+   usbhs_write(omap-tll_base, OMAP_USBTLL_SYSCONFIG,
+   OMAP_USBTLL_SYSCONFIG_ENAWAKEUP |
+   OMAP_USBTLL_SYSCONFIG_SIDLEMODE |
+   OMAP_USBTLL_SYSCONFIG_AUTOIDLE);
+
+   /* Put UHH in NoIdle/NoStandby mode */
+   reg = usbhs_read(omap-uhh_base, OMAP_UHH_SYSCONFIG);
+   if (is_omap_usbhs_rev1(omap)) {
+   reg |= (OMAP_UHH_SYSCONFIG_ENAWAKEUP
+   | OMAP_UHH_SYSCONFIG_SIDLEMODE
+   | OMAP_UHH_SYSCONFIG_CACTIVITY
+   | OMAP_UHH_SYSCONFIG_MIDLEMODE);
+   reg = ~OMAP_UHH_SYSCONFIG_AUTOIDLE;
+   } else if (is_omap_usbhs_rev2(omap)) {
+   reg = ~OMAP4_UHH_SYSCONFIG_IDLEMODE_CLEAR;
+   reg |= OMAP4_UHH_SYSCONFIG_NOIDLE;
+   reg = ~OMAP4_UHH_SYSCONFIG_STDBYMODE_CLEAR;
+   reg |= OMAP4_UHH_SYSCONFIG_NOSTDBY;
+   }
+
+   usbhs_write(omap-uhh_base, OMAP_UHH_SYSCONFIG, reg);
+
reg = usbhs_read(omap-uhh_base, OMAP_UHH_HOSTCONFIG);
/* setup ULPI bypass and burst configurations */
reg |= (OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN
-- 
1.7.4.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: [RFC] dmaengine: add new api for preparing simple slave transfer

2011-06-09 Thread Jassi Brar

On Thu, Jun 9, 2011 at 6:09 PM, Raju, Sundaram sunda...@ti.com wrote:

 Generic buffer description:
 A generic buffer can be split into number of frames which contain number of 
 chunks inside them. The frames need not be contiguous, nor do the chunks 
 inside a frame.

        ---
        | Chunk 0 |ICG| Chunk 1 |ICG| ... |ICG| Chunk n |       Frame 0
        ---
        |                       Inter Frame Gap                     |
        ---
        | Chunk 0 |ICG| Chunk 1 |ICG| ... |ICG| Chunk n |       Frame 1
        ---
        |                       Inter Frame Gap                     |
        ---
        |                                                              
      |
        ---
        |                       Inter Frame Gap                     |
        ---
        | Chunk 0 |ICG| Chunk 1 |ICG| ... |ICG| Chunk n |       Frame m
        ---

IIUC the above figure, the work done by DMA controller remains the
same, either by
passing this as a transfer of the new type or as a normal sg-list  - unless
the DMAC driver attempts to reorder the transfers or the DMAC h/w
natively supports
some form of sg-list.
For DMACs, that have no special support, different representation
wouldn't make a
difference.
And if the DMAC does support the kind of fancy scatter-gather, it
should be possible for
the dma api driver to analyze the submitted 'normal' sg-list and
program the transfers at one go.
Besides, it should be possible to have a 'template' sequence of
requests prepared
already because usually, for above mentioned scenario, the parameters
don't change across
items in a list.
--
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: [spi-devel-general] [PATCH 1/3] [SPI] [OMAP] Add OMAP spi100k driver

2009-12-06 Thread jassi brar

On Mon, Dec 7, 2009 at 1:48 PM, Cory Maccarrone darkstar6...@gmail.com wrote:
 This change adds the OMAP SPI 100k driver created by
 Fabrice Crohas fcro...@gmail.com.  This SPI bus is found on
 OMAP7xx-series smartphones, and for many, the touchscreen is
 attached to this bus.

 The lion's share of the work was done by Fabrice on this driver --
 I am merely porting it from the Linwizard project on his behalf.

 Signed-off-by: Cory Maccarrone darkstar6...@gmail.com
 ---
  drivers/spi/Kconfig         |    6 +
  drivers/spi/Makefile        |    1 +
  drivers/spi/omap_spi_100k.c |  642 
 +++
  3 files changed, 649 insertions(+), 0 deletions(-)
  create mode 100644 drivers/spi/omap_spi_100k.c

 diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
 index 4b6f7cb..7ef9b12 100644
 --- a/drivers/spi/Kconfig
 +++ b/drivers/spi/Kconfig
 @@ -164,6 +164,12 @@ config SPI_OMAP24XX
          SPI master controller for OMAP24xx/OMAP34xx Multichannel SPI
          (McSPI) modules.

 +config SPI_OMAP_100K
 +       tristate OMAP SPI 100K
 +       depends on SPI_MASTER  (ARCH_OMAP850 || ARCH_OMAP730)
 +       help
 +         OMAP SPI 100K master controller for omap7xx boards.
 +
  config SPI_ORION
        tristate Orion SPI master (EXPERIMENTAL)
        depends on PLAT_ORION  EXPERIMENTAL
 diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
 index 21a1182..55f670d 100644
 --- a/drivers/spi/Makefile
 +++ b/drivers/spi/Makefile
 @@ -22,6 +22,7 @@ obj-$(CONFIG_SPI_LM70_LLP)            += spi_lm70llp.o
  obj-$(CONFIG_SPI_PXA2XX)               += pxa2xx_spi.o
  obj-$(CONFIG_SPI_OMAP_UWIRE)           += omap_uwire.o
  obj-$(CONFIG_SPI_OMAP24XX)             += omap2_mcspi.o
 +obj-$(CONFIG_SPI_OMAP_100K)            += omap_spi_100k.o
  obj-$(CONFIG_SPI_ORION)                        += orion_spi.o
  obj-$(CONFIG_SPI_PL022)                        += amba-pl022.o
  obj-$(CONFIG_SPI_MPC52xx_PSC)          += mpc52xx_psc_spi.o
 diff --git a/drivers/spi/omap_spi_100k.c b/drivers/spi/omap_spi_100k.c
 new file mode 100644
 index 000..d0ebfa8
 --- /dev/null
 +++ b/drivers/spi/omap_spi_100k.c
 @@ -0,0 +1,642 @@
 +/*
 + * OMAP7xx SPI 100k controller driver
 + * Author: Fabrice Crohas fcro...@gmail.com
 + * from original omap1_mcspi driver
 + *
 + * Copyright (C) 2005, 2006 Nokia Corporation
 + * Author:      Samuel Ortiz samuel.or...@nokia.com and
 + *              Juha Yrj�l� juha.yrj...@nokia.com
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 + *
 + */
 +#include linux/kernel.h
 +#include linux/init.h
 +#include linux/interrupt.h
 +#include linux/module.h
 +#include linux/device.h
 +#include linux/delay.h
 +#include linux/platform_device.h
 +#include linux/err.h
 +#include linux/clk.h
 +#include linux/io.h
 +#include linux/gpio.h
 +
 +#include linux/spi/spi.h
 +
 +#include plat/clock.h
 +
 +#define OMAP1_SPI100K_MAX_FREQ          4800
 +
 +#define ICR_SPITAS      (OMAP7XX_ICR_BASE + 0x12)
 +
 +#define SPI_SETUP1      0x00
 +#define SPI_SETUP2      0x02
 +#define SPI_CTRL        0x04
 +#define SPI_STATUS      0x06
 +#define SPI_TX_LSB      0x08
 +#define SPI_TX_MSB      0x0a
 +#define SPI_RX_LSB      0x0c
 +#define SPI_RX_MSB      0x0e
 +
 +#define SPI_SETUP1_INT_READ_ENABLE      (1UL  5)
 +#define SPI_SETUP1_INT_WRITE_ENABLE     (1UL  4)
 +#define SPI_SETUP1_CLOCK_DIVISOR(x)     ((x)  1)
 +#define SPI_SETUP1_CLOCK_ENABLE         (1UL  0)
 +
 +#define SPI_SETUP2_ACTIVE_EDGE_FALLING  (0UL  0)
 +#define SPI_SETUP2_ACTIVE_EDGE_RISING   (1UL  0)
 +#define SPI_SETUP2_NEGATIVE_LEVEL       (0UL  5)
 +#define SPI_SETUP2_POSITIVE_LEVEL       (1UL  5)
 +#define SPI_SETUP2_LEVEL_TRIGGER        (0UL  10)
 +#define SPI_SETUP2_EDGE_TRIGGER         (1UL  10)
 +
 +#define SPI_CTRL_SEN(x)                 ((x)  7)
 +#define SPI_CTRL_WORD_SIZE(x)           (((x) - 1)  2)
 +#define SPI_CTRL_WR                     (1UL  1)
 +#define SPI_CTRL_RD                     (1UL  0)
 +
 +#define SPI_STATUS_WE                   (1UL  1)
 +#define SPI_STATUS_RD                   (1UL  0)
 +
 +#define WRITE 0
 +#define READ  1
 +
 +
 +/* use PIO for small transfers, avoiding DMA setup/teardown overhead and
 + * cache operations; better heuristics consider wordsize and bitrate.
 + */
 +#define DMA_MIN_BYTES                   8
 +
 +struct

98 matches

Mail list logo